fix(session-flow): bound the JWT, URL, email and private-key redaction regexes

[kyle-sexton]

Summary

The running-retro observer's ledger redaction and save_point.py's secret-shape scan took seconds to minutes on adversarial lines: 20 s for the JWT rule, up to 86 s for the URL rule, up to 80 s for the email rule, and 37 s for the observer's private-key rule. They now finish in milliseconds, and realistic secrets are still redacted.

Fix

Each pattern started at a word boundary or header and then ran an unbounded run. On a long line with no terminator, every start position scanned to the end of the line and backtracked, so the scan was quadratic (OWASP ReDoS). Each run is now bounded, as #5935 did for the ghs_ rule:

• Cap the JWT header at 512 characters (as the ghs_ rule does) and the URL scheme at 64, in both observer.py and save_point.py. The longest scheme in the IANA registry is 36.
• A JWT or ghs_<APPID>_<JWT> token whose header exceeds 512 characters (for example one carrying an x5c chain, RFC 7515 §4.1.6) matches a linear fallback. The JWT fallback, \beyJ[A-Za-z0-9_-]{513}[A-Za-z0-9_.-]*, sits right after the JWT rule, and the ghs_ fallback, \bghs_[0-9]+_eyJ[A-Za-z0-9_-]{513}[A-Za-z0-9_.-]*, right after the ghs_ rule.
  • observer.py redacts the whole token, payload and signature included, and save_point.py still flags it with the same label.
  • Nothing follows the trailing *, so neither fallback backtracks, and a matching run is consumed in one match.
  • The JWT fallback runs after the JWT rule because a long payload segment also starts eyJ.
  • The ghs_ fallback is needed because _ is a word character, so no word boundary precedes a ghs_ token's eyJ and neither JWT rule can match it.
• Cap the email local part at 64 and the domain at 255, the RFC 5321 §4.5.3.1 limits, in observer.py.
• In observer.py, cap the private-key label at 64 and stop the body at the next -----BEGIN and at 16384 characters.
  • An 8192-bit RSA key is about 6.4–6.5 KB as PKCS#8, PKCS#1 or OpenSSH (generated locally with openssl genpkey and ssh-keygen), so 16384 is over twice that.
  • A 16384 cap alone still took 2.3 s on a repeated header; stopping at the next -----BEGIN makes the scan linear.
  • The body still allows -, because legacy encrypted PEM headers such as DEK-Info: DES-EDE3-CBC,… contain it (RFC 7468, RFC 1421).
• save_point.py's header-only private-key rule is already linear; its timing test now covers the repeated-header input.
• Residual coverage change, rare: a truncated key block with no -----END followed by a complete block now redacts only the complete block.
• The test fixtures build their fake secrets from split literals, and .gitleaksignore carries commit-bound entries for the two fake JWT lines in this PR's first commit (the repository's documented mechanism for intentional findings).
• session-flow 0.48.5 with a CHANGELOG entry.

Verification

• New timing tests fail before the fix (12–94 s against a 1 s budget for the JWT, URL and email cases; 42.7 s for the repeated private-key header) and finish in under 0.1 s after, including 'eyJ' + 'A'*600000, ('eyJ' + 'A'*600) * 1000 and ('ghs_1_eyJ' + 'A'*600) * 1000.
• Coverage tests confirm that realistic JWTs (including 512-, 513- and 4000-character headers, and a 2000-character payload), ghs_ tokens with 513- and 4000-character headers, compound-scheme and 64-character-scheme URLs, emails at the RFC limits, and a 4096-bit-sized PEM (plain, RSA and OPENSSH labels) are still redacted or flagged; real generated 4096- and 8192-bit keys are fully redacted. The long-header cases fail with only the source change stashed.
• All 10 session-flow Python test modules pass (run with GIT_CONFIG_GLOBAL=/dev/null, because the test VM's global url.insteadOf rewrite breaks the unrelated test_new_origin_falls_back_to_directory_name on main too). hop_chain.test.sh, tidy_work.test.sh and observer.test.sh pass.
• gitleaks git over this PR's commits: no leaks. scripts/run-ruff.sh check, typos, markdownlint and all four check-changelog-parity.sh modes pass.
• The GitGuardian check (not a required check) flags the two fake JWT test strings in this PR's first commit (bd90c553c). They spell FAKE and were split in a later commit, so the squash commit on main carries no such literal; the incidents can be marked as test credentials in the GitGuardian dashboard. Rewriting the branch history is not done here.

Related

Closes #5951
Follows #5935.

  

[gitguardian]

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
37850296	Triggered	JSON Web Token	bd90c55	plugins/session-flow/scripts/tests/test_save_point.py	View secret
37850296	Triggered	JSON Web Token	bd90c55	plugins/session-flow/skills/running-retro/scripts/test_observer.py	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secrets safely. Learn here the best practices.
3. Revoke and rotate these secrets.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-04T02:21:14.385420Z	0e471e5	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 8m 10s —— View job

---

/review:code-review — code-review lane

• Gather context (PR diff, changed files, custom instructions)
• Invoke /review:code-review and follow its instructions for this PR
• Post findings as inline PR comments (anchored to changed lines)
• Post summary comment with overview / cross-file findings

No code-review-lane findings.

What I checked

Scoped to the 6 changed files (observer.py, save_point.py, their two test
files, plugin.json, CHANGELOG.md). I traced the backtracking behavior of
each rewritten pattern by hand (Python's re backtracking semantics, not just
reading the diff) rather than trusting the stated fix:

• JWT/URL/email bounds ({10,512} header, {0,63} scheme, {1,64}/{1,255}
  email parts): the quadratic blowup in the original patterns came from an
  unbounded leading quantifier whose character class fully overlaps the
  adversarial input with no terminator in reach, forcing a full-length
  backtrack from every one of O(n) start positions. Bounding that leading
  run caps the backtrack per start position at a constant, so total cost is
  O(n·constant). I also checked whether the remaining unbounded segments in
  the JWT pattern ([A-Za-z0-9_-]{10,} for payload/signature) reopen the same
  class of blowup — they don't, because . is excluded from that character
  class, so the greedy consumption and the required literal . are disjoint
  and the quantifier can't backtrack across a span it already determined
  contains no .. The number of "eyJ" occurrences that can reach that
  expensive tail-scan is bounded by the 512-char header cap relative to each
  literal . in the text, and those tail-scans partition the string rather
  than overlapping, so the worst case stays linear.
• Private-key body (stop at next -----BEGIN or 16384 chars): checked
  that back-to-back complete PEM blocks still redact independently (the lazy
  body quantifier finds its own block's END before reaching the next
  BEGIN), and that the single-occurrence adversarial cases in the new tests
  (repeated header, repeated -----END, unterminated label) each cost at
  most O(cap) since only one BEGIN is present.
• Diff-only concerns (comment accuracy, version bump, changelog entry, test
  parametrization/id alignment, string literal construction) all checked out.

Any finding bearing on whether a secret still gets redacted is a credential-
exposure question, and REVIEW.md's lane split sends that to
claude-security-review, which runs in this repo
(.github/workflows/claude-security-review-hosted.yml) — so it's intentionally
left for that lane rather than duplicated here.

Not independently verified

I could not execute Python in this sandbox (every python3/pytest
invocation was denied approval), so the timing numbers, the 238/46-pass
counts, and the ruff/typos/markdownlint/changelog-parity results in the PR
description are author-claimed, not independently confirmed. The regex
behavior above is verified by manual trace of the engine semantics, not by
running the new tests. If this matters, update this lane's --allowedTools
to permit python3/pytest so a future run can execute the suite directly.

[claude]

Claude finished @kyle-sexton's task in 6m 19s —— View job

---

Security review (/review:security-review)

• Gather context: PR diff (gh pr diff 6151), the six changed files, the GitGuardian comment
• Analyze the ReDoS fix's correctness and look for residual catastrophic-backtracking paths
• Check how SECRET_SHAPES / _REDACTIONS are consumed (gate vs. warn vs. defense-in-depth) to size any coverage-regression risk
• Verify the GitGuardian "JWT" flags
• Post findings

Scope: only the 6 files this PR touches (plugin.json, CHANGELOG.md, save_point.py + its test, observer.py + its test). No GitHub Actions files changed, so nothing deferred to zizmor. No CLAUDE.md/AGENTS.md/rules/skill/agent body is touched, so the instruction-surface-deletion lens doesn't apply here.

No CRITICAL or IMPORTANT issues found. This is a well-reasoned ReDoS fix; walking through the new patterns by hand confirms the bound actually caps the backtracking rather than just moving it:

• Private-key body (observer.py L911-915): the (?!-----BEGIN) lookahead means a second -----BEGIN short-circuits the lazy body scan immediately instead of running it out to the 16384 cap, so a text with many adjacent headers stays O(n) rather than O(n·16384).
• JWT/URL (observer.py L910-912, save_point.py L65-68): the later {10,} groups are still unbounded, but since greedy runs stop at the first non-class character, a costly full backtrack only happens when a group's run reaches end-of-string with no terminator — and the number of distinct start positions that can reach that deep is itself bounded by the (now-512) cap on the first group. So this doesn't reopen an O(n²) path.

SUGGESTION — disclosed, non-blocking: the -----BEGIN-stop change means a truncated key block (a -----BEGIN... with no matching -----END, e.g. from output that got cut off) followed later by a complete block no longer gets redacted for the truncated portion — only the complete block matches. observer.py L911-915. Concretely: a session transcript containing -----BEGIN RSA PRIVATE KEY-----\n<partial base64, no END>...later...-----BEGIN RSA PRIVATE KEY-----\n<full key>\n-----END RSA PRIVATE KEY----- would land the partial block's bytes in the ledger unredacted. The PR body already discloses this tradeoff and it mirrors the precedent set in #5935. Severity is low because this redaction hop is explicitly "defense in depth over the -p run's semantic pass" (observer.py L902-904), not the only control, and save_point.py's SECRET_SHAPES scan is WARN-only, not a blocking gate. No action required; flagging for visibility only.

GitGuardian's two "JWT" flags, verified as false positives: both are synthetic test fixtures whose header/payload/signature literally decode to {"alg":"HS256"} / {"sub":"FAKE"} / FAKEsignatureNOTreal — test_save_point.py L513, test_observer.py L897. Shape-matching tripped the scanner; there's no real credential here and no action needed.

Job run link: not available — this session's sandbox blocks shell variable expansion and env, so the GITHUB_RUN_ID couldn't be read, and I don't fabricate URLs. Branch: cursor/redaction-regex-bounds-e44b → main.

[github-actions]

claude-security-review has reviewed this pull request through 0e471e5; a later push is reviewed from there.

[github-actions]

claude-review has reviewed this pull request through 0e471e5; a later push is reviewed from there.