test(redact): cover the value-run ReDoS shape and assert on scaling - #15
Merged
Merged
Conversation
#16 made config scanning linear and covered the case where a large opaque payload sits on its own line, away from the secret keyword. It did not cover the shape that actually took a gateway down: the unbroken run being the VALUE the secret key is assigned to. Adds four regression tests: - test_long_undotted_value_completes_fast — `token=<32 KB>`, the production shape. ~95s before the linear rewrite. - test_undotted_value_scaling_is_not_quadratic — asserts on the growth RATE rather than a budget at one fixed size, so a reintroduced quadratic is caught while still cheap instead of only once catastrophic. - test_long_undotted_secret_still_redacted — linear scanning must not stop a real long secret being masked. - test_midtoken_keyword_still_not_matched — a key that merely embeds a keyword must stay untouched, guarding the candidate-scanning rewrite against widening what counts as a config key. Tests only. The pattern fix this branch originally carried is dropped as redundant: #16 already makes both shapes linear (`token=<64 KB>` went from ~6 minutes to 3ms), so re-adding a key-start lookbehind on top would change a security-sensitive regex for no measured gain. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BenSheridanEdwards
force-pushed
the
fix/redact-cfg-dotted-redos
branch
from
August 21, 2026 19:18
34143db to
832a31a
Compare
This was referenced Aug 22, 2026
6 tasks
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.
Closes #17
What changed since this PR was opened
This originally carried a one-line
(?<![A-Za-z0-9_.\-])key-start guard on_CFG_DOTTED_RE. #16 has since landed and makes config scanning linear by a broader route, which conflicted with this branch.The pattern fix is dropped as redundant. I measured #16 against the shape that actually wedged the gateway before deciding:
mainwith #16token=<8 KB>token=<16 KB>token=<32 KB>token=<64 KB>#16 fully covers it. Re-adding a lookbehind on top would change a security-sensitive regex for no measured gain, so this PR is now tests only (+63/-0, one file).
What this adds
#16 added one timing test, where a large opaque payload sits on its own line away from the secret keyword. The shape that took Elon's gateway down for 35 minutes is different: the unbroken run is the value the secret key is assigned to. That was uncovered, as was any assertion on scaling behaviour.
Four tests in
TestConfigKeyRedosResistance:test_long_undotted_value_completes_fast—token=<32 KB>, the production shape. Cost ~95s before the linear rewrite; this fails against the pre-fix(redact): keep opaque payload scanning linear #16 pattern at 94.97s against a 2.0s budget.test_undotted_value_scaling_is_not_quadratic— asserts on the growth rate, not a budget at one fixed size. Every other timing test here only fails once a regression is already catastrophic at its chosen size; this one catches a reintroduced quadratic while it is still cheap.test_long_undotted_secret_still_redacted— linear scanning must not stop a real long secret being masked.test_midtoken_keyword_still_not_matched— a key that merely embeds a keyword (value_xtoken=) must stay untouched, guarding the candidate-scanning rewrite against widening what counts as a config key.Why it is worth keeping after #16
The bug is fixed; the coverage gap that let it ship is not.
TestConfigKeyRedosResistanceexisted before this incident and every test in it used dotted runs, which an earlier possessive-quantifier rewrite had already fixed. The undotted shape was never exercised, so the class looked like protection it was not providing. These tests close that gap and add a rate assertion so the next regression surfaces early rather than as a wedged gateway.Tests
78 passedintests/agent/test_redact.pyagainstmainwith #16. Suite runtime is now 1.5s, down from ~56s before #16.Context
Full incident write-up, including the
samplestack trace and the frozen-subsystem timestamps, is in #17. Upstream carries the same bug class, fixed there by NousResearch#91672.🤖 Generated with Claude Code