fix(gate): gate-integrity hotfix (step 0 — all 5 bypasses closed) - #482
Merged
Conversation
step 0.1 of the gate-integrity hotfix. the actor recorded on every proposal and audit event came from client-controlled input: X-Vouch-Agent on /rpc (jsonl_server._agent) and VOUCH_AGENT env on /mcp (server._agent). because the self-approval gate compares actor strings, one bearer token could propose as "alice" and approve as "bob" and the approval sailed through — the review gate was forgeable by anyone holding a single token. both _agent() resolvers now prefer trust.current().auth_subject when the request is authenticated, returning a stable token:<subject> identity that the client cannot override. the header/env fallback survives only for tokenless loopback/dev, which is trusted by design. mirrors web/server.py, where the token's label already wins over any client-supplied name. the exploit is pinned by a test: propose-as-alice then approve-as-bob under one token is now caught as forbidden_self_approval. /mcp callers also stop collapsing to a single env identity — distinct tokens are distinct actors, so cross-review becomes possible and attribution is real.
step 0.2 of the gate-integrity hotfix. kb.import_apply writes bundle members (claims/pages/decided) straight to disk — a parallel path past proposals.approve() — and it was reachable by agents on MCP and JSONL. two stopgaps until gated import lands (roadmap 8.2): - import_check now refuses any bundle carrying decided/ members, scanned directly from the manifest (not a self-reported safety flag a hand-crafted bundle could lie about), so import_apply raises on them for every caller including the CLI. decided/ holds approved decisions; importing it would land approved claims/pages with no receiving-side proposal. - kb.import_apply is dropped from the agent surfaces — the MCP tool, the JSONL handler + HANDLERS entry, and the METHODS list — and from hot_memory's exclusion map. it survives only as the human `vouch import apply` CLI command. the read-only kb.import_check stays available on every surface. exports still include decided/ (a faithful snapshot); the guard is on the write side. capabilities/handlers parity holds.
…rfaces step 0.4 of the gate-integrity hotfix. kb.export took a client-supplied out_path and wrote a tarball there (creating parent dirs), and kb.import_check/export_check read a client-supplied bundle_path — so a remote caller on /rpc or /mcp could clobber or read an arbitrary file (out_path=../../etc/cron.d/x, bundle_path=/etc/passwd). new KBStore.resolve_under_root is the containment half of read_under_root (resolve + is_relative_to, no open), and bundle.fenced_bundle_path applies it only when trust.current().remote — so both server surfaces confine the path to the project root while local cli and stdio stay unfenced, since a human choosing where to write a backup is not a threat. wired into _h_export / _h_export_check / _h_import_check and the matching mcp tools, one shared helper so the surfaces cannot drift. tests pin both directions: a remote ../escaped.tar.gz export is refused and writes nothing, a within-root export still works, and a remote import_check at /etc/passwd is refused instead of reading it.
… origins step 0.3 of the gate-integrity hotfix. the console /proxy bridge forwarded to whatever host X-Vouch-Target named and copied the caller's Authorization header onto the forwarded request — an open relay that leaks the bearer token to an arbitrary host and an SSRF primitive into internal services (including cloud metadata endpoints). the loopback client-guard limited who could call it, not where it forwarded. build_console_app now takes allowed_targets (serve origins, scheme://host:port) and _target_allowed enforces it: with an allowlist the target origin must be one of the configured origins; with none, the target must be loopback — the safe default for the local console, never an arbitrary host. serve_console threads the allowlist through. existing loopback-upstream proxy tests are unaffected; a non-loopback target is now refused with forbidden_target.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
step 0.5 of the gate-integrity hotfix, and the last one. capture ingested raw session content with zero secret scanning, and the append-only audit log plus git make a pasted credential permanent — the only masking in tree was on log field names, not content values. new secrets.mask_secrets runs before observations reach the gitignored buffer, so a pasted key never becomes a committed session fact. it is deliberately conservative: curated high-precision patterns (aws/github/openai/anthropic/ slack/google tokens, jwts, private-key blocks, key=value assignments) rather than raw entropy, which would shred git shas and uuids; contains_secret exposes the same check. wired into capture.observe (summary + cmd), masked before dedup so dedup compares masked text. lifecycle.redact + `vouch redact <claim_id>` are the backstop for a secret that already reached a durable claim: mask the text, mark it REDACTED (drops from retrieval), audit a claim.redact event. it rewrites the current tree only — the append-only audit log and git history are untouched, so a real leak must still be rotated. docs/security/git-retention.md says so plainly and gives the filter-repo purge steps. with this, step 0 (gate integrity) is complete: the review gate is no longer forgeable by actor spoofing, an import bypass, an ssrf proxy, arbitrary-path writes, or a leaked credential.
plind-junior
marked this pull request as ready for review
July 15, 2026 04:14
Member
Author
|
step 0.5 landed: capture-time secret masking ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
step 0 of the fidelity-pivot roadmap: the gate-integrity hotfix. before the
review gate can be made mechanical and the human taken out of the loop (phase
d), the gate must not be forgeable. this closes four live bypasses; the fifth
item (capture-time secret masking +
vouch redact) lands in a follow-up.0.1 — actor binding. the proposal/audit actor came from client-controlled
input (X-Vouch-Agent on /rpc, VOUCH_AGENT env on /mcp), so one bearer token
could propose as "alice" and approve as "bob" and the self-approval gate waved
it through. both _agent() resolvers now bind to trust.current().auth_subject
when authenticated; the header/env survive only for tokenless loopback/dev.
pinned by an exploit test.
0.2 — import_apply bypass. kb.import_apply writes bundle members
(claims/pages/decided) straight to disk, a parallel path past
proposals.approve(). it is dropped from the agent surfaces (mcp tool, jsonl
handler, METHODS) and kept as the human-only CLI; import_check now refuses any
bundle carrying decided/ members, scanned from the manifest rather than a
spoofable safety flag. read-only import_check stays on every surface.
0.3 — console /proxy allowlist. the bridge forwarded to any X-Vouch-Target
host with the caller's Authorization header — a token-forwarding SSRF relay.
it is now loopback-only by default, with an allowlist for configured serve
origins.
0.4 — export/import path fencing. kb.export wrote to a client-supplied
out_path and import/export_check read a client-supplied bundle_path — remote
arbitrary write/read. both server surfaces now confine the path to the project
root on remote trust via one shared helper; local cli/stdio stay unfenced.
gate green throughout: pytest (minus embeddings), mypy, ruff. every fix is
test-first with the vulnerability pinned. draft until 0.5 lands and step 1
(fleet concurrency) is decided.