Skip to content

fix(capture-kit): linear email-label check in repeated-nav detection - #3365

Merged
thymikee merged 1 commit into
mainfrom
security/2026-10-10-redos-email-label
Oct 10, 2026
Merged

thymikee merged 1 commit into
mainfrom
security/2026-10-10-redos-email-label

Conversation

@thymikee

@thymikee thymikee commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Summary

Fixes code-scanning alert #47 (js/polynomial-redos, high). isEmailLikeLabel in repeated-nav-subtree.ts ran /\S+@\S+\.\S+/ on accessibility text from the app under test. This runs during snapshot presentation, so a long label with the wrong shape (for example '@'.repeat(50_000)) could stall every snapshot.

The label has already been trimmed and its whitespace collapsed to single spaces. The fix splits it on ' ' and checks each token in linear time. A token matches when it has an @ at index ≥ 1 and a . after that @, with at least one character between the @ and the . and at least one after the .. This is the same rule the regex applied to one whitespace-free run, so results don't change. There's no length cap and no suppression comment.

2 files touched: the source module and its colocated test.

Validation

Tested commit: e42bc0c.

  • pnpm check:affected --run: all runnable checks passed (302 files, 2361 tests).
  • New table test pins each accepted and rejected case against the original regex, then checks the detector's result.
  • New adversarial cases ('@'.repeat(50_000), 'a@'.repeat(25_000), and others) finish in milliseconds. On the same '@'.repeat(50_000) input the old regex had not finished after more than 2 minutes.
  • Local fuzz over 300k random short strings drawn from a@. found no mismatch with the old regex.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

Replace the polynomially backtracking /\S+@\S+\.\S+/ with a per-token
indexOf/lastIndexOf check on the already whitespace-normalized label.
Results are unchanged. Resolves code-scanning alert #47.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

View guided diff | Turn on auto-fix | Re-trigger cubic

@github-actions

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.15 MB 5.15 MB +87 B
Package (unpacked) 5.15 MB 5.15 MB +87 B
Package (download) 1.55 MB 1.55 MB +15 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.9 ms 28.3 ms -0.6 ms
CLI --help 83.4 ms 83.3 ms -0.2 ms

@thymikee

Copy link
Copy Markdown
Member Author

I found no problems in e42bc0c. The linear email-label check in repeated-nav detection reads as equivalent to the old regex, and the colocated test covers the slow-input case.

CI is green: all 19 checks pass, and the diff touches only repeated-nav-subtree.ts and its test. There are no conflicts, and nothing else stands between this and merge.

I did not run the tests or the old regex locally. My read rests on the code and the CI result. The 300k-string fuzz and the 2-minute old-regex run are your numbers, and I did not reproduce them. The <1_000 ms timing assertions are loose against the roughly millisecond cost of the new check. They could still flake under heavy CI load, but I would not change them now.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 10, 2026
@thymikee
thymikee merged commit 4037805 into main Oct 10, 2026
19 checks passed
@thymikee
thymikee deleted the security/2026-10-10-redos-email-label branch October 10, 2026 11:32
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-10 11:33 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant