Fix pre-existing CI: bound wire decoder recursion; unblock Smoke + parse gates#64
Merged
Merged
Conversation
…rse gates
Two unrelated, pre-existing CI failures on main:
parse-gate (vcltotal-parse):
- The "total, never-panics" wire decoder recursed without bound, so a
deeply-nested adversarial stream (thousands of nested Compare/Aggregate
tags, or TList in the schema codec) overflowed the native stack and
aborted the process — a crash that violates the decoder's totality
contract. Bound decode depth via a DepthGuard with MAX_DEPTH=128 (the
cap serde_json uses for the same reason), returning the new
WireError::TooDeep. Regression test covers both dec_expr and
dec_vqltype: a 200k-deep input now returns Err, never overflows.
- The roundtrip property tests intermittently overflowed the default
2 MiB test-thread stack: proptest's recursive expr()/statement()
strategies recurse far enough during value generation for some RNG
seeds (the generated values are shallow, depth ~6 — harness overhead,
not a deep AST). Run the parse-gate test step with RUST_MIN_STACK set
to 32 MiB so the property tests are robust.
backend-matrix (Smoke matrix — all per-prover jobs):
- echidna-client is a standalone workspace with no committed Cargo.lock
(its echidna-core path-dep only exists once the sibling is cloned), so
`cargo test --locked` failed before building ("cannot create the lock
file ... because --locked was passed"). Drop --locked so dependencies
resolve fresh against the just-cloned sibling.
Validated: full vcltotal-parse suite (lib + gate + parse + wire +
conformance) green at RUST_MIN_STACK=32 MiB; clippy --all-targets
-D warnings clean; the totality regression test passes on the default
2 MiB stack with no overflow.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017nyxs8RgqZa72PzrTu3L75
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
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.
What
Fixes two pre-existing, unrelated CI failures on
main(surfaced while CI ran on the now-merged hook PR #63). Both were investigated and reproduced locally.This branch contains only these fixes (4 files); the SessionStart hook from #63 is already merged.
1.
parse-gate—vcltotal-parsestack overflowsTwo distinct causes, both fixed:
a) The wire decoder was not actually total.
from_wire/from_wire_op/from_wire_schemaare documented "total, never panics", butdec_expr(anddec_vqltypein the schema codec) recursed without any depth bound. A deeply-nested adversarial stream — thousands of nestedCompare/Aggregatetags, or nestedTList— overflows the native stack and aborts the process (SIGABRT). A crash is not a totalOk/Err, so this violated the trusted-boundary contract (and is a DoS vector). The existing*_total_on_garbagefuzz tests didn't catch it because ≤2048 random bytes rarely form a deep valid prefix.Fix: bound decode recursion with a
DepthGuard+MAX_DEPTH = 128(the same cap serde_json uses), returning a newWireError::TooDeep. This mirrors the module's existing anti-DoS stance on untrusted counts (never pre-allocated), now extended to untrusted nesting. New regression test feeds a 200k-deep stream to bothfrom_wireandfrom_wire_schemaand assertsErr(TooDeep)with no overflow on the default 2 MiB stack.b) The roundtrip property tests intermittently overflowed the 2 MiB test stack.
proptest's recursiveexpr()/statement()strategies recurse far enough during value generation for some RNG seeds to exhaust the default test-thread stack — flaky, seed-dependent, and the cause of the intermittent red. The generated values themselves are shallow (measured max depth 6), so this is proptest harness overhead, not a deep AST, and is independent of (a).Fix: run the parse-gate test step with
RUST_MIN_STACK=32 MiB. (Verified: 8 MiB already clears it across repeated runs; 32 MiB is comfortable headroom.)2.
backend-matrix— everySmoke (<prover>)job fails at setupAll ~105 jobs failed identically before running any prover logic:
echidna-clientis a standalone workspace with no committedCargo.lock— its transitiveechidna-corepath-dep only exists once the sibling repo is cloned (the step just above), so a lockfile can't be committed and--lockedcan never be satisfied.Fix: drop
--lockedfrom that onecargo testinvocation so deps resolve fresh against the just-cloned sibling.Validation
vcltotal-parsesuite (lib +gate+parse+wire+conformance, 36 tests) green atRUST_MIN_STACK=32 MiBcargo clippy --manifest-path src/interface/parse/Cargo.toml --all-targets --locked -- -D warningsclean (the gate's SPARK-grade lint set)Note
The
backend-matrix(Smoke) change can't be exercised locally — those jobs clone theechidnasibling, which isn't available here — but the edit directly removes the exact--lockedcondition that produced the error.🤖 Generated with Claude Code
https://claude.ai/code/session_017nyxs8RgqZa72PzrTu3L75
Generated by Claude Code