Skip to content

fix(capture): make the write-gate bypass rule an allowlist (#365 follow-up) - #380

Merged
cdeust merged 3 commits into
mainfrom
fix/capture-origin-allowlist
Aug 7, 2026
Merged

fix(capture): make the write-gate bypass rule an allowlist (#365 follow-up)#380
cdeust merged 3 commits into
mainfrom
fix/capture-origin-allowlist

Conversation

@cdeust

@cdeust cdeust commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Follow-up to #375, implementing the review finding. Refs #365.

What was wrong

The origin control was a denylist:

_ORIGINS_REFUSED_CONTENT_BYPASS = frozenset({ORIGIN_NETWORK})

Classification already failed safe — an unrecognised tool lands in UNKNOWN rather than being promoted to LOCAL_ACTION. But the security decision built on top of it failed open: anything not literally webfetch or websearch could claim a content-derived bypass.

The trigger is not hypothetical. _NETWORK_TOOLS hardcodes two names against a host whose tool surface changes. Add a third off-machine tool, or rename one, and its content classifies UNKNOWN and silently regains the bypass — the control stopping with no failing test and no signal. That is the same shape as the issue #375 closes.

Now an allowlist of {DELIBERATE, LOCAL_ACTION}. An unclassified origin is refused, so the cost of a missing classification is a rejected write instead of a trusted one.

Two things the change forced out

ORIGIN_DELIBERATE was dead. Defined, documented in the vocabulary, produced by nothing since it was written. Under an allowlist a direct remember would resolve UNKNOWN and lose the content bypass it has always had, so the value now has a producer: a remember carrying no producing tool is the user asking directly.

The condition is the absence of a tool name, not an UNKNOWN classification. Those differ exactly where it counts — a named-but-unrecognised tool also classifies UNKNOWN, and promoting that to DELIBERATE would reinstate the fail-open this PR removes. My first draft got this wrong and test_capture_origin_persistence.py::test_unrecognised_tool_persists_unknown_not_a_guess caught it.

The issue #147 ordering broke. A deliberate write's reason degraded from bypass_error to the generic bypass_write_class_deliberate, masking the diagnostic that test exists to protect. Since such a write bypasses either way, refusing it the specific label changed no outcome and only destroyed information. The origin rule now governs whether content may buy a bypass, not how an already-granted one is labelled.

The honest cost

The original design note said UNKNOWN was chosen so "adding this parameter changes no existing caller's behaviour". Under an allowlist that is no longer true, and seven tests in test_write_gate.py were silently relying on it. They now state which channel produced their content — which is what they were asserting all along: error-shaped content from a local tool bypasses, not error-shaped content from anywhere.

That is the trade in one line: the permissive default was doing real work, and now every caller has to mean it.

Completion Ledger

§ Item Evidence
A1 Happy paths pytest -k "gate or capture or remember or origin or write_class"453 passed, 13 subtests
A2 Edge cases Absent tool, named-but-unrecognised tool, empty string, each origin value, deliberate write class at network origin, important/critical tag at network origin
A3 Failure paths Refusal asserted directly: UNKNOWN refused, unrecognised origin refused, network refused; each with its own test
A4 Trust boundary The whole change. Origin comes from the channel, never from content; the promotion to DELIBERATE keys on tool absence and is additionally gated on a write class the auto-capture hook pins out-of-band
A5 Invariants Stated at the use site: an unclassified origin is refused, and the cost of a missing classification is a rejected write
A6 Idempotency Pure functions over inputs
B1–B3 Concurrency N/A
C1–C3 Resources / perf N/A — set membership
D1 Injection class N/A
D2 Untrusted data The point of the PR. A future or renamed off-machine tool can no longer buy a content-derived bypass by being unclassified
D3 Secrets None touched
E1 API compatibility may_bypass_write_gate_on_content keeps its signature; the semantics invert, which is the intended behaviour change
E2 Downstream consumers write_gate.determine_bypass and handlers/remember; both updated and covered. Seven test call sites made explicit
E3 Persisted data capture_origin column now records deliberate for direct writes where it recorded unknown; more accurate, no migration needed
E4 Cross-platform Pure Python
F1 Signals The specific bypass reason is preserved for deliberate writes rather than masked
F2 Degraded modes Refusal is explicit, not silent: determine_bypass returns (False, None) and the gate reports a rejection
G1 Path→test ledger Allowlist membership, UNKNOWN refusal, unrecognised-origin refusal, absent-tool promotion, unrecognised-tool non-promotion, deliberate-keeps-specific-reason
G2 Regression test test_unrecognised_tool_persists_unknown_not_a_guess is the regression test for the flaw in my own first draft
G3 Determinism Slice re-run three times; 453 each time
G4 Negative assertions Named-but-unrecognised is not promoted; UNKNOWN may not bypass; a caller naming no origin gets (False, None)
G5 Full gate ruff check . clean · ruff format --check . clean (1283 files)
H1–H3 Standards, readability Each decision states the failure it prevents at the use site
H4 CHANGELOG Deferred to review: #375's entry describes the denylist. Say the word and I amend it rather than adding a second entry
H5 Commit hygiene One commit
H6 CI green Required before merge
H7 Boy-scout (§14) Found and fixed a dead vocabulary value (ORIGIN_DELIBERATE) that predates this PR

cdeust and others added 3 commits August 7, 2026 11:45
…ow-up)

Follow-up to #375, per review. The origin control was a denylist:

    _ORIGINS_REFUSED_CONTENT_BYPASS = frozenset({ORIGIN_NETWORK})

so anything not literally named webfetch or websearch could claim a
content-derived bypass. Classification already failed safe — an unrecognised
tool lands in UNKNOWN rather than being promoted to LOCAL_ACTION — but the
security decision built on it failed open.

The trigger is not hypothetical: _NETWORK_TOOLS hardcodes two names against a
host whose tool surface changes. A third off-machine tool, or a rename,
classifies UNKNOWN and silently regains the bypass — the control stopping with
no failing test and no signal, which is the shape of the issue #375 closes.

Inverted to an allowlist of {DELIBERATE, LOCAL_ACTION}. An unclassified origin
is refused; the cost of a missing classification is a rejected write instead of
a trusted one.

Two things fell out of it.

ORIGIN_DELIBERATE was defined, documented in the vocabulary and produced by
NOTHING — dead since it was written. Under an allowlist a direct `remember`
would resolve UNKNOWN and lose the content bypass it has always had, so the
value now has a producer: a `remember` carrying no producing tool is the user
asking directly. The condition is the ABSENCE of a tool name, not an UNKNOWN
classification — those differ exactly where it counts, since a named-but-
unrecognised tool also classifies UNKNOWN and promoting that would reinstate
the fail-open just removed. tests_py/infrastructure/
test_capture_origin_persistence.py caught that in the first draft.

The issue #147 ordering (deliberate never novelty-rejected, but a specific
content reason still wins over the generic one) broke: a deliberate write's
reason degraded from bypass_error to bypass_write_class_deliberate. Such a
write bypasses either way, so refusing it the specific label changed no
outcome and only destroyed a diagnostic. The origin rule now governs whether
content may BUY a bypass, not how an already-granted one is labelled.

Seven tests in test_write_gate.py were silently relying on the permissive
default and now state which channel produced their content — which is what
they were asserting all along: error-shaped content FROM A LOCAL TOOL
bypasses, not error-shaped content from anywhere.

Verified:
  pytest -k "gate or capture or remember or origin or write_class"
    -> 453 passed, 13 subtests passed
  ruff check / format --check -> clean

Refs #365

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#375's entry described the denylist it shipped: "network-origin content is
refused the two content-derived bypasses" and "unrecognised tools classify as
unknown rather than trusted, so a newly added tool is visibly unclassified".
Both are now wrong in the same direction — unknown is refused, not merely
visible — so the release notes would have contradicted the code.

Restated as what it is: an allowlist of {deliberate, local_action}, with the
reason a denylist was abandoned (it fails open the moment the host adds or
renames a network tool) and the distinction between a tool that was NAMED but
is unrecognised and no tool being named at all.

docs/mcp-tools.md's write-path description carried the same denylist framing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cdeust

cdeust commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

Ledger row H4 updated: the CHANGELOG is no longer deferred.

#375's entry described the denylist it shipped — "network-origin content is refused the two content-derived bypasses" and "unrecognised tools classify as unknown rather than trusted, so a newly added tool is visibly unclassified". Both were wrong in the same direction once this landed, since unknown is now refused rather than merely visible, so the release notes would have contradicted the code.

Amended in place rather than appended to, so there is one account of the behaviour instead of two that disagree. It now states the allowlist, why the denylist was abandoned (it fails open the moment the host adds or renames a network tool), and the distinction between a tool that was named but unrecognised and no tool named at all.

docs/mcp-tools.md's write-path description carried the same denylist framing and is reconciled too.

@cdeust
cdeust merged commit 577a898 into main Aug 7, 2026
21 checks passed
@cdeust
cdeust deleted the fix/capture-origin-allowlist branch August 7, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant