fix(ingest): bundle devil's-advocate followups #230 + #232 + #233

docs/research-brief-compliance-audit-2026-05-06.md

@@ -416,6 +416,7 @@ This is **the highest-novelty surface for bicameral-mcp.** The system is an LLM
 - **LLM-06** [P1 — narrowed scope] — **Skill content drift between server release and a future remote-skill-loading channel.** Original P0 framing was overstated (per Kilo #2): in the current install model, the operator already trusted the supply chain at `pip install` / `uv tool install` / `pipx install` time, and skills ship co-located with server code in the same wheel — there's no separate channel to compromise without compromising the wheel itself, which is covered by SOC2-03 (signed releases) and OWASP-01 (SBOM). The scenario where LLM-06 has independent value is a **future remote-skill-loading or marketplace feature** (none today): when skills could be pulled from a registry distinct from the server wheel, signing the skill payload separately becomes load-bearing. Remediation when that scope opens: cosign-signed `skills/MANIFEST.toml`, per-file SHA-256 verification at copy time. Until then, the gate is "don't ship remote skill-loading without first activating signed manifests" — a design constraint, not a runtime defect.
 - **LLM-07** [P1] — **`source_ref` redaction default is `full` (verbatim) per #209.** This is a known issue tracked separately. Remediation: ship #209 (refine regex + flip default to `redacted`).
 - **LLM-08** [P2] — **`bicameral.ingest` has no rate limit.** A runaway agent can flood the ledger. Remediation: token-bucket rate limit per session_id; declare server-side enforcement.
+  - **Limitations (#230 Finding 2)** — The token-bucket gate slows BURST consumption per session but does not bound aggregate throughput. Default config (burst=10, refill=1/s, size cap=1 MiB) admits ~70 ingests in any 60-second window and 1 MiB/s sustained — ~86 GiB/day worst case under a runaway agent (model regression, prompt-injection-hijacked re-ingest cycle, dev-time infinite-loop bug). Not a security crisis (the size cap bounds per-payload damage; in-process registry resets on server restart) but a real operator-side disaster for ledger-writer churn + disk pressure. Stricter aggregate enforcement (sliding-window cross-session bound) is deferred to the team-server-activation track which has cross-developer correlation needs that single-session token-bucket cannot satisfy.
 - **LLM-09** [P1] — **`ratify`, `link_commit`, `set_decision_level` fire without human-in-loop on agent-initiated calls.** These are state-changing decisions. Remediation: declare each tool's HITL requirement deterministically; gate the destructive ones with **out-of-band operator confirmation** (same shape as LLM-05 — `AskUserQuestion` when host supports it, terminal prompt fallback otherwise). Agent-supplied `confirm`-style parameters are not security gates here either.
 - **LLM-11** [P0/M, all deployments] — **Cross-tool config-file modification surface** (per Kilo #9). `setup_wizard._install_hooks` modifies `.claude/settings.json` (Claude Code's host-config) and the analogous Cursor / Codex configs. A compromised bicameral-mcp install can therefore inject arbitrary Claude Code hook commands that fire as the operator at hook-trigger-time (PreToolUse, PostToolUse, SessionEnd). This is **distinct from LLM-06** (skill content): LLM-06 is text the agent reads, LLM-11 is shell commands the host runs. A signed-skills-manifest gate doesn't cover this. Remediation: ship a signed `hooks-manifest.json` separately (cosign-signed at release); `setup_wizard` verifies the manifest before writing to `.claude/settings.json`. The manifest is the second supply-chain leg distinct from skills + wheel.
 

handlers/ingest.py

@@ -2,6 +2,19 @@
 
 Thin orchestration: validate payload, resolve symbols, ingest into ledger, then sync.
 Auto-grounding removed in caller-LLM binding flow (v0.5.1+).
+
+Limitations — aggregate-rate worst case (#230 Finding 2):
+  The token-bucket rate gate slows BURST consumption per session but does
+  not bound aggregate throughput across time. Default config (burst=10,
+  refill=1/s, size cap=1 MiB) admits ~70 ingests in any 60-second window
+  and 1 MiB/s sustained, which works out to ~86 GiB/day in the worst case
+  (runaway agent loop, model regression producing infinite tool calls,
+  prompt-injection-hijacked re-ingest cycle, dev-time infinite-loop bug).
+  Not a security crisis — the size cap bounds per-payload damage and the
+  in-process registry is reset on server restart — but it IS an operator-
+  side disaster (ledger writer churn + disk pressure). Stricter aggregate
+  enforcement (sliding-window cross-session bound) is deferred to the
+  team-server-activation track.
 """
 
 from __future__ import annotations
@@ -14,6 +27,13 @@
 from datetime import UTC
 
 import preflight_telemetry
+
+# #232 Finding 1: cross-module use of context.py's private truthy frozenset
+# is intentional — it's the canonical vocabulary for BICAMERAL_* env-var
+# toggles (1/true/yes/on, case-insensitive). Renaming to a public alias is
+# out of scope here; #232 acceptance only requires vocabulary parity across
+# the existing toggle reads.
+from context import _GUIDED_MODE_TRUTHY
 from contracts import (
     BriefEnvelope,
     BriefGap,
@@ -62,8 +82,21 @@ def _check_payload_size(payload: dict, max_bytes: int) -> None:
     every field the agent might supply, language-agnostic, single
     comparison. Pure: no telemetry side-effect; the wrapping try/except
     in ``handle_ingest`` records the refusal event before re-raising.
+
+    #232 Finding 2: a payload that is not JSON-serializable (circular ref,
+    deeply nested object, opaque type whose ``__str__`` raises) would
+    previously leak ``ValueError`` / ``TypeError`` / ``RecursionError``
+    past the gate to the MCP boundary's generic exception handler.
+    Translate to ``_IngestRefused('malformed_payload', ...)`` at the same
+    boundary as the other refusals — closes the fail-open path.
     """
-    size = len(json.dumps(payload, default=str).encode("utf-8"))
+    try:
+        size = len(json.dumps(payload, default=str).encode("utf-8"))
+    except (ValueError, TypeError, RecursionError) as exc:
+        raise _IngestRefused(
+            "malformed_payload",
+            detail=f"payload is not JSON-serializable: {type(exc).__name__}",
+        ) from exc
     if size > max_bytes:
         raise _IngestRefused(
             "size_limit_exceeded",
@@ -109,9 +142,19 @@ def take(self) -> bool:
 def _check_rate_limit(session_id: str, burst: int, refill_per_sec: float) -> None:
     """Raise ``_IngestRefused('rate_limit_exceeded', ...)`` when the bucket
     for ``session_id`` has no tokens. Disabled entirely by setting
-    ``BICAMERAL_INGEST_RATE_LIMIT_DISABLE=1`` (local debugging knob).
+    ``BICAMERAL_INGEST_RATE_LIMIT_DISABLE`` to a truthy value (1/true/yes/on,
+    case-insensitive — see ``context._GUIDED_MODE_TRUTHY``).
+
+    #230 Finding 1: the refusal detail does NOT include ``session_id``. The
+    raw session UUID is process-fingerprinting state that surrounding
+    telemetry writers hash via per-install salt; emitting it raw at the
+    MCP boundary (which the agent then relays into operator-visible context)
+    is inconsistent with that posture. Operators get the bucket params
+    they need to tune ``.bicameral/config.yaml``; the session UUID is not
+    action-relevant here.
     """
-    if os.getenv("BICAMERAL_INGEST_RATE_LIMIT_DISABLE", "").strip() == "1":
+    env_val = os.getenv("BICAMERAL_INGEST_RATE_LIMIT_DISABLE", "").strip().lower()
+    if env_val in _GUIDED_MODE_TRUTHY:
         return
     with _RATE_LIMIT_REGISTRY_LOCK:
         bucket = _RATE_LIMIT_REGISTRY.get(session_id)
@@ -121,9 +164,7 @@ def _check_rate_limit(session_id: str, burst: int, refill_per_sec: float) -> Non
     if not bucket.take():
         raise _IngestRefused(
             "rate_limit_exceeded",
-            detail=(
-                f"session {session_id} bucket empty (burst={burst}, refill={refill_per_sec}/s)"
-            ),
+            detail=f"bucket empty (burst={burst}, refill={refill_per_sec}/s)",
         )
 
 

plan-216-ingest-size-and-rate-limit.md

@@ -15,7 +15,7 @@
   home: handlers/ingest.py
 
 **boundaries**:
-- limitations: rate-limit state is in-process (per-server-restart); a malicious agent restart-looping has bigger problems than this gate. Token-bucket-burst defaults are tuned for single-user developer-tool workflow shape; team-server activation may want stricter sliding-window enforcement (revisit then). **Per-developer isolation update (post-#231)**: bucket scoping is now per-developer (salted-email-hash key) when `git config user.email` is available; falls back to process-wide single bucket only in test/CI mode. Two developers on the same install get distinct buckets — runaway loop on developer-A doesn't affect developer-B.
+- limitations: rate-limit state is in-process (per-server-restart); a malicious agent restart-looping has bigger problems than this gate. Token-bucket-burst defaults are tuned for single-user developer-tool workflow shape; team-server activation may want stricter sliding-window enforcement (revisit then). **Per-developer isolation update (post-#231)**: bucket scoping is now per-developer (salted-email-hash key) when `git config user.email` is available; falls back to process-wide single bucket only in test/CI mode. Two developers on the same install get distinct buckets — runaway loop on developer-A doesn't affect developer-B. **Aggregate-rate worst case (post-#230 Finding 2)**: the gate slows BURST consumption per session but does not bound aggregate. Default config admits ~70 ingests in any 60-second window and 1 MiB/s sustained — ~86 GiB/day worst case under a runaway agent. Not a security crisis (size cap bounds per-payload damage); operator-side disaster for ledger-writer churn. Stricter aggregate enforcement (sliding-window cross-session bound) deferred to the team-server-activation track.
 - non_goals: do not implement LLM-01 (prompt-injection canary scan — already filed as #212), LLM-04 (PII/secret/PHI/PAN detect-and-refuse — already filed as #213), or any other epic #216 sub-task. Do not extend rate-limit beyond `bicameral.ingest`. Do not add adversarial-human threat-model coverage (out of scope per LLM-08's deployment trigger).
 - exclusions: not modifying `IngestPayload` Pydantic schema. Not changing `_normalize_payload` semantics. Not adding telemetry counters to the outbound `telemetry.py` relay (size + rate counters land in local `~/.bicameral/preflight_events.jsonl` only).