fix(security): remove inert curl-pipe deny rules from settings template#55
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The two curl-pipe deny rules in global/settings.template.json were inert: Claude Code only treats ":*" as a trailing wildcard at the END of a specifier, so trailing text after it makes the colon a literal and the rule matches no command that ever runs. They looked like a security control but enforced nothing, and `forge init` copied them verbatim into users' settings.json. The intended protection already exists and genuinely enforces at global/guards/protect-paths.sh (a PreToolUse Bash hook) — the mechanism the Claude Code docs recommend for constraining piped commands. This removes the dead rules and tightens that guard to also catch a no-space pipe and a zsh target, while still allowing legit pipes such as shellcheck. Adds test/settings_template.test.js as a regression guard: no Bash rule may embed a shell operator in its specifier, ":*" must be pattern-terminal, and the two removed rules must stay gone.
18722e5 to
e05ca7d
Compare
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.
What & why
A usage audit flagged the two
curl-pipe deny rules inglobal/settings.template.jsonas malformed. Verification confirmed it: ClaudeCode only honors
:*as a trailing wildcard at the end of aBash(...)specifier — any text after it turns the colon into a literal, so these rules
matched no command that ever runs. They looked like a security control and
enforced nothing, and
forge initcopied them verbatim into every user's~/.claude/settings.json.The intended protection already exists and genuinely enforces at
global/guards/protect-paths.sh— a PreToolUse Bash hook that emits apermissionDecision:"deny", which is the mechanism the Claude Code docsrecommend for constraining piped commands.
Changes
protect-paths.shpipe-to-shell detection: was a space-onlysubstring match; now a boundary-aware regex that also catches a no-space pipe
and a
zshtarget, while still allowing legit pipes such asshellcheck.test/settings_template.test.js— a regression guard: noBashrulemay embed a shell operator in its specifier,
:*must be pattern-terminal,and the two removed rules must stay gone.
Note for reviewers
No enforcement is lost — the removed rules were dead; the real guard is
unchanged in behavior except for being broadened. Verified the guard blocks
| sh, no-space pipe, andzshtargets, and still allowsshellcheckandplain
curl.Checklist
npm testpasses — 663/666; the one failure (imagine.test.jsworktreesandbox) is pre-existing and unrelated (proven by stashing this diff)
npm run checkpasses for the changed files (pre-existingbiome.jsondeprecation + warnings in other files are untouched)
fix:)CHANGELOG.mdupdated under## [Unreleased]node src/cli.js docs checkpassesRisk & rollback
previous guard match; no behavioral loss either way.
Extra checks
npm run typecheckpasses