docs(scripts): stop overstating what the process-substitution gap readmits - #2085
Conversation
Unverified batch preserved from a session that ended before its findings were falsified against origin/main. Touches check-contract-slice-prune, the shell-portability test suite, and the portability token list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gap class excludes `(` and `)` alongside the separators, but the comment claimed only separators. Record the exclusion, the converse catch it buys, and the process-substitution blind spot it costs — a tradeoff the date/stat/mktemp tokens already carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…call Narrowing the sed command gap to stop at a bare paren also stopped it at a process substitution, which is not a command of its own -- the shell folds it into the word it touches. `sed -f <(gen) -i "$f"` therefore went undetected where the pre-narrowing gap caught it. Readmit exactly that shape; a `)` closing a real subshell still ends the gap, so the converse catch it bought is kept. The `--in-place` unit block was building its local token from the pre-narrowing pattern, so none of its assertions could tell old behavior from new. Point it at the shipped pattern and give both sed tokens separator- and process-substitution-scoping cases, including two backed by the real token list so a hand-copied pattern cannot drift unnoticed again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dmits `[^)]*` cannot balance parens, so a nested process substitution still stops the gap at the inner `)`. The comment claimed the alternative readmits that shape "and nothing else"; record the residual false negative instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba8ed8fad7
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keeps this branch's corrected comment — the process-substitution readmission is not recursive, so a nested one still goes unreported — and takes main's removal of the obsolete standalone `sed -Ei` token, which the cluster token now covers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Warning Automated security review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
The check is red on purpose. It certifies that a security pass ran, and this one did not complete, so it cannot report success. Where this check is required, merging is blocked until a review actually finishes. Re-run the job to retry the review; a new push also retries it only if the caller's Re-running does NOT help for every class:
|
|
Warning Automated review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-run the job to retry the review. A new push re-triggers this lane only if the caller's |
|
Filed the quoted-command-word gap as #2091 so it survives this PR's resolution — it is a real coverage limit in the gate's design (an ERE cannot see through shell quote removal), just not one this comment-only change introduced or touched. The thread here is resolved on that basis. |
A one-comment correction to
scripts/shell-portability-tokens.txt:245, which #2064 merged beforethis fix landed. Comment-only — no token, no behavior, no test changes.
What is wrong on main
The sed command-gap comment claims
[<>]\([^)]*\)"readmits exactly that shape and nothing else".It does readmit a process substitution, but
[^)]*is not recursive, so a process substitutionwhose body itself contains one —
sed -f <(diff <(a) <(b)) -i "$f"— stops the gap at the INNER)and goes unreported.An ERE cannot balance parentheses; catching that shape needs real parsing, which this grep-level
tripwire does not attempt. So the residual is a false NEGATIVE on a rare shape, which is the
direction this gate already accepts everywhere else. The defect is the comment asserting a
completeness the regex does not have — a reader trusting it would not think to check.
Provenance
Found by the fresh-context verifier on #2064 while auditing the very fix that introduced the
sentence. #2064 merged with the overstatement still in place; this carries the correction forward.
Prose asserting more than the artifact does has been a recurring defect class on this sweep — it
drew CHALLENGE on
context-guardtwice and appeared again asdisk-hygiene's R1 — so it is worthcorrecting rather than leaving as a harmless-looking comment.
Related
Follow-up to #2064. Discharges no review thread; the four findings #2064 addressed are resolved.
No linked issue