Repository navigation
fix(safe-yaml): deep nesting and control characters are named refusals, not a 500 (#3324) - #3418
Merged
Merged
Conversation
) load_hardened_yaml let two failure classes escape its HardenedYamlError contract, so deploy_system (which catches only ManifestError/ValueError) answered an unnamed 500: - Control characters: PyYAML's Reader checks printability in __init__, and `loader(text)` sat above the try that maps YAMLError -> `{kind}_yaml_invalid`. Construction now sits inside the try, and the finally only disposes an instance that was actually built. - Deep nesting: no depth bound, so ~330 flow levels (6 KB) or 500 block levels (~125 KB), both under the byte cap, exhausted the stack. A compose_node depth gate (DEFAULT_MAX_DEPTH = 64, per-call `max_depth`) now refuses `{kind}_too_deep` before recursing. An `except RecursionError` backstop maps recursion the gate cannot see (a `<<` merge chain walked by flatten_mapping) to the same code. The agent-server copy is byte-identical (Invariant #5). Tests: removed the two #3324 strict-xfail markers in TestDocumentShape (the deep test now pins `k_too_deep`); new ent314 tests cover the 64/65 boundary, block form, per-call max_depth, the manifest codes through parse_manifest, and the merge-chain budget + backstop. Before the fix they failed on RecursionError / ReaderError (and on the missing API), and they pass after it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… leaf (#3324) Review I1: the compose_node counter incremented for every node, so DEFAULT_MAX_DEPTH = 64 admitted only 63 collection levels for any document ending in a scalar. Count only mapping/sequence starts; add a leaf-bearing 64/65 boundary test for flow and block style. Both copies byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the measured figure (#3324) Review I2: measured on PyYAML 6.0.3 with the compose_node override, 4 frames per level, ~265 at depth 64, ~725 of 1000 left for the caller (a 64-level doc parses with the caller 725 frames deep). Comment only; both copies byte-identical. 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)
webmixgamer
added a commit
that referenced
this pull request
Oct 9, 2026
Brings the branch up to ed59049 (12 commits, incl. #3406 public-link sessions, #3422 `user-invocable:` in the library, #3418 safe_yaml). One conflict, in docs/user-docs/automation/skills-and-playbooks.md: this PR's prose kept, with the `user-invocable:` key spelling (the hyphen is the only spelling the agent server reads). The Skills flow's Run table now names the listing field and the frontmatter key apart. PublicChat.vue merged cleanly with #3406 (this PR's "/ for skills" placeholder kept). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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 #3324 — 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/safe_yaml.pyand its agent-server mirrordocker/base-image/agent_server/safe_yaml.py, byte-identical (Invariant #5):try, so PyYAML'sReaderError(NUL, other C0 controls, DEL, U+FFFE) becomes{kind}_yaml_invalidinstead of escaping. A failed construction cannot turn into anUnboundLocalError.compose_nodeoverride refuses a document with{kind}_too_deepbefore recursing pastDEFAULT_MAX_DEPTH = 64nested collections. Flow nesting ([[[…) and block nesting are both covered; block nesting also crashed under the byte cap and was not in the issue.RecursionErroris caught and raised as{kind}_too_deep, for recursion the counter cannot see (merge-key chains).deploy_systemneeds no change:ManifestErrorsubclassesHardenedYamlError, so it now answers a named 400.Tests: the two
#3324strict-xfail markers inTestDocumentShapeare removed; new cases intest_ent314_hardened_yaml.pycover the 64 / 65 boundary with a scalar leaf in flow and block style, deep block nesting, merge chains, and a manifest refused withmanifest_too_deep/manifest_yaml_invalid.Rulings carried (orchestrator, on the operator's behalf — plan file)
except RecursionErrorbackstop plus tests; no override of PyYAML'sflatten_mapping— as recommended.Review + security
/reviewreading pass (claude-fable-5-1, report-only): MERGEABLE, no critical findings. Every exit ofload_hardened_yamlis a named error or a return, confirmed by running the branch copy; the counter is restored in afinally; 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.
cmpof the two copies is empty after each commit.git merge-treeis clean againstdevf6bedcd and against #3413's head.Tests
690 passed across the 15 files. Not run here: the full unit island (CI).
Before merge
tests/unit/test_ec_input_hardening_edges.py; a test-merge of the two heads is clean, in either order.Handoffs
{kind}_too_deep. The existing byte and alias limits and their codes are unchanged.🤖 Generated with Claude Code