Repository navigation
Identity hook: the three readings SonarCloud flagged are string readings in one pass - #249
Conversation
…ngs in one pass (#239) The identity git var gives, a credential in a URL and an scp-like remote were read by regexes that backtrack super-linearly (python:S8786): a remote of 200,000 characters with an @ in every other one took the old pattern 41 seconds, and an identity of 40,000 " <" nineteen. Each is read by string methods now, in one pass, with the same answers at every edge the regexes had, held by a test of those edges and a wall-clock test. The hook's remaining cost on a long command is Python's shlex and its passes over the command, within its own budget. A Boundary row on decision 0040; four mutants. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 12 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… shard let it survive The edge test had no name holding " <", so a reading that takes the first " <" for the email's start gave the same answers; a name with one, and an email with a bare <, are edges now, read as the greedy regex read them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…x's digit, after Muse's read After a run that no @ ended, the search went on from the run's end, so a :// nested in it, as in https://a://user:secret@host, kept its credential where the old regex took it out; the search goes on from just after the :// now. The identity's digits are decimal digits, as \d had them, not a superscript, and an identity holds one line. Two mutants, six in all. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Why
"Identity hook: three super-linear regexes bounded, so a long command cannot stall the guard (SonarCloud S8786)" (issue 239). SonarCloud flagged three regexes in
tools/hooks/identity.pyas super-linear: the one that reads the identitygit vargives, the one that takes a credential out of a URL before a sentence shows it, and the one that tells a URL or an scp-like remote from a remote's name. The hook is a mistake guard (decisions 0040, 0041) that reads every command a session is about to run; a reading that backtracks on a long or repetitive word stalls the session for a harmless command.What changed
_ident,_without_credentialand_urlish, replace the regexes, with the same answers at every edge the regexes had: an empty name, a<the email holds, a://@with nothing before the@, an@or a:after the first/. Git strips<and>from a name and an email, so the last<and the last>bound the email as the greedy regex did.@in every other one took the old pattern 41 seconds, and 10 seconds at 100,000; an identity of 40,000<took 19 seconds. Each reading is now a pass. The wall-clock test holds the three readings under a second at those sizes, and the whole hook under five seconds on a 20,000-character remote. Past that, the hook's cost on a long command is Python'sshlexand the hook's several passes over the command, within the hook's own 10-second budget and itscommand_unreadablerefusal; that cost is older than this issue and is not changed here.Regression corpus
hook_regressions.py(the reviewers' attack inputs and the ordinary corpus) and the 10,722-command fresh corpus, main b324f93 against this head, at the first head and again after the fix round: 40 attack inputs, 0 regressions; 8,842 corpus commands, 0 newly refused; and of the 10,722 fresh commands, 0 newly refused and 0 newly passed, with no exception on either side. The readings give the regexes' answers on every command the corpora hold.Measured
make verifypassed on agit archiveexport of the head, run alone. The six mutants were killed by hand on scratch copies with their named tests (the filtered run cannot selecttools/hooks/identity.pyalone: it holds more than 40 mutants).Who read it
Muse Wonder Full read the first head, 1247383, as the Maintainer's ruling of 2026-10-20 has it for a hook pull request, and found three things, each in the one fix round the helper rule allows: a blocker, real, where a
://nested in a run that no@ended was skipped, sohttps://a://user:secret@hostkept its credential that the old regex took out (the search goes on from just after each://now, with the case in the credential test); therpartitionmutant unkilled for want of a name holding<(the same survivor CI's shard 7 showed; the edge is in); andisdigitwider than the regex's\d, taking a superscript (isdecimalnow, and one line only). Muse did not read the fix round. The Claude review and the security pass found no defect: the readings take no new input and give no new output, and a credential is still taken out of every sentence.Closes #239.
🤖 Generated with Claude Code