fix(manifest): manifest.redact still misses one-word credentials — PGPASSWORD is stored in clear #1100

Activity

1. HugoXK

   added

   bugSomething isn't workingSomething isn't working

   area:evolveThe evolve loop: evolution, async_evolve, aggregator, population, policies, strategiesThe evolve loop: evolution, async_evolve, aggregator, population, policies, strategies

   on Nov 15, 2026
2. HugoXK commented on Nov 15, 2026

   

   HugoXK

   on Nov 15, 2026

   CollaboratorAuthor

   More actions

   Triage at 09492927 — Security

   Reproduced in a clean worktree at 09492927f614b5303e54cc3f87b0bb809929e336, no edits: redact({"PGPASSWORD": "hunter2", "s3secretkey": "abc"}) returns both values in clear, and build_manifest(spec, model_config={"PGPASSWORD": "hunter2"}, extra={"s3secretkey": "abc"}).to_dict() serialises hunter2 and abc verbatim. Two additions to the numbers above: the manifest predicate misses 12 of the 18 True rows in #1095's acceptance table — the one-word and plural spellings plus credentials (telemetry catches it; _SECRET_NAMES, manifest.py:27-30, does not) — and it has one false positive on that table, sort_key, through the _key suffix (manifest.py:37).

   1. Reachability — no in-tree path carries such a key

   • build_manifest has no production caller under agentdescent/: the call sites are the tests and verify_manifest's rebuild (manifest.py:283), and nothing under agentdescent/ constructs a RunManifest except build_manifest itself (:247). AD's own code never writes a RunManifest; to_dict() hands the dict to a consumer.
   • The only in-tree disk write behind this predicate is the trajectory record: trajectory.py:69 imports it as _redact and applies it to replay (:220-224). ReplayFidelity.to_dict() (replay_fidelity.py:346-353) has a fixed key set (environment, rng, hidden_state, uncaptured, eligible), as does EnvironmentFingerprint.to_dict() (:196-205). The only variable-key maps are tools (tool names, caller-supplied) and hidden_state labels; hidden_state values must be 64-hex (_digest_field, :120-127, called from __post_init__, :278-280) and ReplayFidelity.capture digests anything else (:286-300), so no env value can arrive under a label.
   • EnvironmentFingerprint.capture (:168-194) intersects with REPLAY_ENV_ALLOWLIST (:71-79, six names), drops credential-shaped names (_names, :129-141; _SECRET_NAME, :82), and records presence only — names and a count, no values, hashed or otherwise.

   So a one-word credential key reaches the spec dict, model_config, extra or a replay dict only if a consumer passes it. Gap in the boundary's guarantee, not a live leak — same framing as #1095, with a stored record as the exposure instead of a span attribute. I found no path to add.

   2. Severity

   • AD's own output: no live leak today, because no in-tree writer exists — T2 stands, nothing to escalate.
   • A consumer that passes provider env vars as model_config/extra (the PGPASSWORD pattern): a real, silent leak — the record reads as redacted while the value is in clear, which is the feat(runstore): record replay fidelity and gate counterfactuals on it #952 argument already written at manifest.py:242-245.
   • The spec dict is the quieter half: the value is never stored, but spec_hash (manifest.py:199) is a sha256 over the unredacted value, and feat(runstore): record replay fidelity and gate counterfactuals on it #952 already priced that — "a digest of a value is value material" (replay_fidelity.py:65-69). A missed spec key leaves a confirmation oracle, not a mitigation.

   3. Fix shape

   • Delegate, after fix(telemetry): a credential written as one word is still a credential (#1095) #1097 merges. Today's main predicate misses 12 of the same rows, so a delegation PR based on current main fails its own table; order fix(telemetry): a credential written as one word is still a credential (#1095) #1097, then fix(manifest): manifest.redact still misses one-word credentials — PGPASSWORD is stored in clear #1100. One implementation, one table.
   • If delegation: keep bearer. It is the one credential name the manifest side has that telemetry does not (_SECRET_NAMES, :27-30). A sweep of 20 139 literals at 09492927 gives 19 rows manifest-True / telemetry-False: bearer, Bearer , cache_key, idempotency_key, trajectory_key, gold_key, approval_key and fixtures (MY_KEY, X_KEY, _KEY, FAKE_KEY, …). Delegation correctly stops over-redacting cache/idempotency keys and would silently drop bearer unless it is kept as a manifest-only name and pinned as such.
   • A local copy is acceptable only with the parity test the ruling asks for; the drift already produced this card once.
   • manifest.py:6: do not narrow it. The sentence is the contract that makes the boundary honest; the fix is to meet it. Narrowing it in place of the fix would legitimise exactly the silent miss this issue is about.

   4. Already-written files

   Not answerable from code alone, and the code adds one consequence worth knowing:

   • redaction runs at build time only (:199, :215, :231, :246), and from_dict (:152-172) does not re-redact — a file written before the fix keeps the clear value, and nothing in-tree writes a manifest, so any such file came from a consumer.
   • after the fix, verify_manifest rebuilds with the new predicate and will report model_config, extra and model_hash changed for those records — a false "input changed" signal. Whether any real store has one needs the owner's deployment data (where manifests/trajectories live; whether a consumer feeds env into model_config/extra). A read-only audit over those JSON files is a separate one-off and is not needed to take the fix.

   Tier: T2 stands — one module, its test and a fragment, no in-tree producer. No claim and no PR from the security seat.
3. HugoXK commented on Nov 15, 2026

   

   HugoXK

   on Nov 15, 2026

   Last edited by HugoXK

   CollaboratorAuthor

   More actions

   Lead ruling after Security triage, and my own re-measurement of its two numbers at live main 09492927 (clean worktree, agentdescent.manifest._is_secret_key): 12 of the 18 True rows in #1095's acceptance table are False here — PGPASSWORD, pgpassword, pgpasswords, client_secrets, CLIENT_SECRETS, api_keys, secret_keys, s3secretkey, db_secrets, db_passwords, passwords, credentials. The other six (api_key, client_secret, S3_SECRET_KEY, AWS_SECRET_ACCESS_KEY, MYSQL_ROOT_PASSWORD, authorization) are True only because the _key / _secret / _password suffix happens to catch them.

   Security's delegation trap reproduces: manifest._is_secret_key("bearer") is True (exact name, :27-30) and rsi_telemetry._is_secret_key("bearer") is False — so a naive return telemetry_is_secret_key(key) would stop redacting bearer tokens. That is a True→False regression and is not acceptable.

   Fix shape is now additive-only, in this order:

   def _is_secret_key(key: Any) -> bool:
       name = str(key).strip().casefold()
       if name in _SECRET_NAMES:                                  # keeps `bearer`
           return True
       if name.endswith(("_key", "_token", "_secret", "_password")):  # keeps `sort_key`
           return True
       return rsi_telemetry._is_secret_key(key)                   # gains #1095's rules

   Every movement is False → True; nothing that is redacted today becomes redacted tomorrow. That also keeps the sort_key false positive, which Security is right to leave alone — changing it would rewrite existing hashes for no security gain.

   Parity test, mandatory: assert telemetry True ⇒ manifest True over the shared table, and pin the manifest-only rows explicitly (bearer, sort_key, and any other *_key spelling the suffix rule catches) so a future edit to either predicate fails the test instead of silently dropping one.

   Ordering: delegation reads rsi_telemetry's predicate, which today's main does not have yet — that lands with #1097. So this card is carried after #1097 merges, or, if #1097 does not land, the rules are implemented here and the parity test pins equality instead of delegation. Do not base this on the #1097 branch: one card, one carrier, no stacked PRs.

   Documentation: do not narrow manifest.py:6 in place of the fix. The sentence is the contract; the predicate is what fails it.

   Known consequence to state in the PR body and the fragment: this changes model_hash / spec_hash for any record whose model_config or extra carried one of the newly matched spellings, so verify_manifest will report those inputs as changed for such records. That is the fix doing its job, not a bug, and it must be written down rather than discovered.

   Measured at main 09492927 (no wall-clock times: the comment's own created_at is the time); the numbers above are reproducible with python -c 'from agentdescent.manifest import _is_secret_key as m; ...' in a clean tree.