Repository navigation
fix(sanitizer): the depth cap fails closed — nested secrets no longer escape redaction (#3312) - #3413
Merged
Merged
Conversation
…3312) sanitize_dict / sanitize_list returned the raw subtree once depth exceeded max_depth, so a secret nested 12+ containers deep (a stream-json tool input with ~6 levels of its own nesting) was persisted unredacted. Past the cap the subtree is now replaced with REDACTION_PLACEHOLDER; recursion stays bounded. sanitize_json_string (both copies) and the agent's sanitize_subprocess_line also catch RecursionError from json.loads on pathologically deep input and fall back to the linear sanitize_text pass instead of raising. Tests: strict-xfail removed from test_secret_below_max_depth_is_still_redacted; new 2000-level dict/list cases and a 200k-deep JSON string case (backend edges + agent copy, incl. sanitize_subprocess_line); both test_max_depth_protection now assert the secret is absent; the property test drops its depth<=11 assume. Mutation: with the fix reverted, 6 backend tests (both S12 params, both deep-object params, deep-JSON, backend max_depth_protection) and 5 agent tests (max_depth_protection + the 4 TestDepthFailsClosed cases) went red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trinity-ability
approved these changes
Oct 9, 2026
trinity-ability
left a comment
trinity-ability
left a comment
Contributor
There was a problem hiding this comment.
merge-train: batch validated on train/20261009-0735 (#3424)
vybe
added a commit
that referenced
this pull request
Oct 9, 2026
…s, not a 500 (#3324) (#3418) Fixes #3324 — independent fix, one of ten small bug fixes in the trinity-pm `chain-easy-1008` run. Base is `dev`. **Draft until the operator merges.** ## What `src/backend/utils/safe_yaml.py` and its agent-server mirror `docker/base-image/agent_server/safe_yaml.py`, byte-identical (Invariant #5): - **Control characters:** the loader is constructed inside the `try`, so PyYAML's `ReaderError` (NUL, other C0 controls, DEL, U+FFFE) becomes `{kind}_yaml_invalid` instead of escaping. A failed construction cannot turn into an `UnboundLocalError`. - **Depth:** a counter in the existing `compose_node` override refuses a document with `{kind}_too_deep` before recursing past `DEFAULT_MAX_DEPTH = 64` nested collections. Flow nesting (`[[[…`) and block nesting are both covered; block nesting also crashed under the byte cap and was not in the issue. - **Backstop:** `RecursionError` is caught and raised as `{kind}_too_deep`, for recursion the counter cannot see (merge-key chains). `deploy_system` needs no change: `ManifestError` subclasses `HardenedYamlError`, so it now answers a named 400. Tests: the two `#3324` strict-xfail markers in `TestDocumentShape` are removed; new cases in `test_ent314_hardened_yaml.py` cover the 64 / 65 boundary with a scalar leaf in flow and block style, deep block nesting, merge chains, and a manifest refused with `manifest_too_deep` / `manifest_yaml_invalid`. ## Rulings carried (orchestrator, on the operator's behalf — plan file) - T1: the `except RecursionError` backstop plus tests; no override of PyYAML's `flatten_mapping` — as recommended. - T2: depth limit 64 — as recommended. The deepest document the loader reads today is 9–11 levels; the crash point was about 329. ## Review + security `/review` reading pass (claude-fable-5-1, report-only): **MERGEABLE**, no critical findings. Every exit of `load_hardened_yaml` is a named error or a return, confirmed by running the branch copy; the counter is restored in a `finally`; the branch loader parses all 102 tracked YAML files with zero refusals; every caller catches the error type or broader and none branches on a closed set of codes. `/cso --diff` (diff-scoped): no finding. Fixed after review: - `462797e9` — I1: the counter counted the scalar leaf, so "64" meant 63 for a document ending in a scalar. It now counts nested mappings and sequences only: 64 with a scalar leaf parse, 65 are refused. - `8dfa40a3` — I2: the comment's frame arithmetic now states the measured figure (4 frames per level after the fix above, about 265 frames at depth 64). Not fixed, on purpose: I3, two comments in the sibling properties test that still mention the removed xfail — left out to keep the merge surface with #3413 small. Evidence sweep after the fix (claude-opus-5-5): **GREEN**. All 15 unit files that use the loader ran shuffled (seed 12345), one file per process, every run exit 0. `cmp` of the two copies is empty after each commit. `git merge-tree` is clean against `dev` f6bedcd and against #3413's head. ## Tests 690 passed across the 15 files. Not run here: the full unit island (CI). ## Before merge - The agent-server copy changes, so agents pick it up with the next base-image rebuild. - #3413 edits a different class in `tests/unit/test_ec_input_hardening_edges.py`; a test-merge of the two heads is clean, in either order. ## Handoffs - New reason codes callers may now see: `{kind}_too_deep`. The existing byte and alias limits and their codes are unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3312 — independent fix, one of ten small bug fixes in the trinity-pm
chain-easy-1008run. Base isdev. Draft until the operator merges.What
src/backend/utils/credential_sanitizer.pyanddocker/base-image/agent_server/utils/credential_sanitizer.py: pastmax_depth,sanitize_dictandsanitize_listreturn the redaction placeholder instead of the raw subtree. The cap now fails closed.sanitize_json_string(both copies) and the agent'ssanitize_subprocess_linecatchRecursionErrorfromjson.loadsand fall back to the text sanitizer, so very deep input no longer raises.test_secret_below_max_depth_is_still_redactedis removed; new cases cover 2,000-level structures and a 200k-deep JSON string on both copies; bothtest_max_depth_protectiontests now assert the secret is gone (they only checked the type before); the property test holds at any depth.Rulings carried (orchestrator, on the operator's behalf — plan file)
RecursionErrorcatches ship here, not in a follow-up — as recommended; AC 2 needs them.Review + security
/reviewreading pass (claude-fable-5-1, report-only): MERGEABLE, no blocking findings. The fix was proven by mutation: thedevcopies leak a secret nested 14 deep and raise on 200k-deep JSON; the branch copies redact, identically in both files. The four touched function bodies are token-identical across the copies;sanitize_textand the #3335 logic are byte-identical todev; every reader of the sanitized structure indexes at most five levels, and the placeholder first appears at level 11./cso --diff: no findings (no Docker pass, no independent verifier — coverage gaps recorded in the review file).Evidence sweep (claude-opus-5-5): GREEN. 22 test files reference
credential_sanitizer; 1,029 passed shuffled with seed 12345. Two reds fail identically ondevand are not from this branch:test_ent615_token_free_remotes.py::…no_repository_as_traversable_and_absent, andtest_cb_probe_execution_close.py(needs a live backend).git merge-treeagainstdevf6bedcd is clean.Tests
Targeted: 571 + 12 + 60 passed on the engineer. Not run here: the full unit island (CI).
Before merge
Handoffs
json.loads(claude_code.py:372,codex_runtime.py:1282,headless_executor) do not catchRecursionError, so a very deep stream-json line is dropped, not leaked. Outside this issue's acceptance criteria.🤖 Generated with Claude Code