Repository navigation
fix(#5054): retire the unscoped Bash(npx gsd-core *) allow rule - #5269
Conversation
The Claude installer pre-authorized `npx gsd-core *`, but the project ships as `@opengsd/gsd-core` and the unscoped npm name belongs to a different owner. The rule let Claude Code run whatever is published under that name without asking. GSD itself never runs the bare form. Move the rule from GSD_CLAUDE_ALLOW_PERMISSIONS to GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS, so the existing open-gsd#2278 retirement filter removes it on the next install or upgrade (byte-equal only) and uninstall keeps cleaning it up. GEMINI.md now gives the scoped `npx @opengsd/gsd-core@latest --antigravity --global` command, and the USER-GUIDE permission list drops the rule and explains the retirement. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… claim Add a --claude --local upgrade row: a pre-open-gsd#5054 settings.local.json carrying the unscoped rule loses it and keeps the user's own rule. It fails on the base installer. The guards also catch `--yes`, and the USER-GUIDE note now says which entries are kept instead of promising that every other permission is untouched. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Before opening, I ran one Codex review of the whole diff against the merge-base. Here's what it found and what I did with each. Acted on (
Rejected, with evidence
Noted, not acted on (out of this issue's scope)
Verification: the full local suite ran before the follow-up commit: 44,414 tests, 44,378 pass, 2 fail. Both failures reproduce on a clean |
trek-e
left a comment
There was a problem hiding this comment.
Automated maintainer review of head f70cc9a833. Zero-tolerance policy: every finding at any severity must be resolved before approval.
Review — fix(#5054): retire the unscoped Bash(npx gsd-core *) allow rule
Head f70cc9a (3 commits; branch fix/5054-unscoped-npx-allow-rule). mergeable=MERGEABLE, mergeStateStatus=BLOCKED (live gh view this turn). Non-success checks: only SKIPPED (auto-merge, inert CI, emitted-baseline artifact); no failures.
Summary
Moves Bash(npx gsd-core *) from GSD_CLAUDE_ALLOW_PERMISSIONS to GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS (bin/install.js:201,238-242), reusing the existing #2278 retirement filter (install.js:285-287) and the uninstall filter (install.js:9099). GEMINI.md:28 uses the scoped command. USER-GUIDE updated. Root-cause fix at the owning seam; no special-casing.
Acceptance checklist (issue #5054, "Suggested fix")
- Remove rule from ALLOW list (or scope it) -- MET (install.js:201-210).
- On upgrade remove old rule from existing settings.local.json -- MET (legacy filter; tests for --claude --global and --claude --local).
- Update GEMINI.md and other docs to scoped form -- MET (GEMINI.md:28; repo-wide grep of docs/ gsd-core/ commands/ agents/ bin/ src/ README finds no remaining bare npx/bunx/pnpx gsd-core|gsd-tools invocations outside CHANGELOG and the retirement notes).
- "Consider" maintainers taking unscoped npm names -- registry action, explicitly out of scope in PR body; not a diff criterion.
Findings
- Minor | judgment | CONTEXT.md:307 (Installer Module entry): lists
GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS #2278as only the Write->Edit retirement; this PR widens that list to retireBash(npx gsd-core *)(#5054) and the entry describes the filter semantics. Fix: add "; also retires the unscopedBash(npx gsd-core *)rule (#5054)" to that clause. (CONTEXT.md glossary is a PR gate per project memory.)
No other findings.
Root cause / tests
Root cause: rule predates @opengsd rebrand, merge is additive-only so constant change alone would leave it in existing installs; PR routes through the retirement filter. Regression tests assert persisted settings.json / settings.local.json on disk via real installer runs (global + local), order preservation, byte-equal-only removal (4 near-miss rules survive), uninstall removal, GEMINI.md line check (CRLF-safe split, markdown not source, no source-grep rule hit). Fail-first plausible and claimed (10 failing on base). No wall-clock assertions; timeouts bounded (SCOPED_INSTALL_TIMEOUT_MS). No parser/budget so fast-check not required. Boundary limit-1/limit/limit+1 n/a.
Security
Removes a standing pre-authorization; no new reads/argv/subprocess. Byte-equal removal intentionally drops a user-authored identical string (disclosed in PR body).
Blast radius
codegraph_impact mergeClaudePermissions (index from main checkout, not this worktree): 9 symbols -- finishInstall, installAllRuntimes, install-regressions/install/cline-install/configured-entrypoint-validation/worktree-baseref-install tests. Behavior change limited to Claude permissions.allow; exports unchanged.
Co-change gaps
- CONTEXT.md installer entry (above).
- Changeset .changeset/wise-voles-wave.md: type Fixed, pr 5269, bold lead + symptom explanation, trailing (#5054) -- OK. Fixed is docs-exempt but docs updated anyway. No references/ or workflows/ files added (no INVENTORY change); no shipped workflow growth (no Emitted-Drift-Ack trailer needed). No parallel-surface constant duplicates found.
- PR body follows fix template,
Fixes #5054, not draft, conventional title/commits, branch name valid. Prior feedback: no reviews; contributor self-review comment items addressed in 2d97baf.
Behind-by / rebase
0 behind origin/next (fetched this turn), 3 ahead; git merge-tree --write-tree clean.
Verdict
changes-requested (single Minor: CONTEXT.md; zero-tolerance policy). Otherwise substance clean.
Memtrace Evidence
Memtrace quota was exhausted for this review run, so the graph-backed checks were NOT executed; this is stated rather than reported as passed:
get_impact: not run (quota). Blast radius above comes from codegraph impact/callers where present.get_symbol_context: not run (quota). Callers/callees were read via codegraph where present.recall_decision: not run (quota). The recorded-decision gate was NOT evaluated; this is not a finding that no decision governs the change.find_code_review_issues: not run (quota). Deterministic AST/YAML/cross-module pass did not execute; every finding above is judgment-sourced unless marked deterministic.
…NTEXT entry GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS now also retires the unscoped `Bash(npx gsd-core *)` rule; the Installer Module entry said it covered only the open-gsd#2278 Write->Edit retirement. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Addressed the one Minor from the 2026-10-09 review.
Head is now |
trek-e
left a comment
There was a problem hiding this comment.
Review of head e8f8fd461929 — approve
Summary. Retires the unscoped Bash(npx gsd-core *) allow rule by moving it into GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS, so install no longer adds it and merge/uninstall strip it by byte-equal match.
Prior finding. The Minor from the earlier review (the CONTEXT.md:307 Installer Module entry must record the retired rule) is resolved: the clause now says GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS #2278 also retires the unscoped rule (#5054). [verified on disk]
Classification and gates. #5054 carries bug + confirmed-bug; bug-fix track. Fixes #5054, conventional title, fix template, branch fix/5054-…. Changeset .changeset/wise-voles-wave.md is type Fixed, pr: 5269, bold lead, trailing (#5054). USER-GUIDE and GEMINI.md updated. No references/ or workflows/ additions, so no INVENTORY change is needed.
Root cause. The rule lived in the active list and was re-added on every install. It now lives only in the legacy list, and the same filter drives merge and uninstall (bin/install.js:238-242, :285-287). Idempotent: filter, then add-if-missing. Only byte-equal entries are removed, so user-authored near-miss rules are preserved (covered by tests).
Security. Removes a standing pre-authorization; adds no reads or argv. Disclosed trade-off: a user-authored string identical to the retired rule is also removed. Judgment: acceptable.
Tests. Assertions read persisted settings.json / settings.local.json after real installer runs. No wall-clock, no source-grep. I did not re-execute the fail-first demonstration (local node --test is not permitted); on the base the installer adds the rule, so the absence assertion cannot pass there.
CI. Verified on this exact head: Required tests, test (ubuntu-latest, 24) and shards 1–3, macOS and Windows conformance, smoke, lint-tests, docs-lint, changeset-lint all SUCCESS. Only test (inert CI), auto-merge and the emitted-baseline artifact job are SKIPPED. mergeable: MERGEABLE.
CodeGraph Evidence
Memtrace unavailable: monthly quota exhausted, so get_impact, get_symbol_context, recall_decision and find_code_review_issues were not run. The recorded-decision check was not evaluated, and this is not a claim that no decision governs.
codegraph_impact GSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS: 15 symbols. Inbin/install.js:mergeClaudePermissions(:266),finishInstall(:13524),uninstall(:8283). The rest are install-related test files. No consumer outside the installer.codegraph_callers mergeClaudePermissions:finishInstall(bin/install.js:13524) andtests/install-regressions.test.cjs.- Finding: the change is confined to the installer's permission-merge and uninstall paths, and both consume the same list.
Findings
None. Blocker 0, Major 0, Minor 0, Nit 0.
Verdict
Approved.
Fix PR
Linked Issue
Fixes #5054
What was broken
Every Claude Code install and upgrade wrote
Bash(npx gsd-core *)intopermissions.allow. GSD ships as@opengsd/gsd-core, and the unscopedgsd-corenpm name belongs to a different owner, so Claude Code would run whatever that owner (or anyone who takes over their account) publishes next, without asking.GEMINI.mdalso told Antigravity users to run the unscoped form.What this fix does
Bash(npx gsd-core *)moves fromGSD_CLAUDE_ALLOW_PERMISSIONStoGSD_CLAUDE_LEGACY_ALLOW_PERMISSIONS. That list already feeds the #2278 retirement filter inmergeClaudePermissions, so a fresh install never writes the rule, the next install or upgrade removes it from an existing settings file (byte-equal match only, every other entry kept in order), and uninstall still removes it.GEMINI.mdnow givesnpx @opengsd/gsd-core@latest --antigravity --global, matchingdocs/how-to/install-on-your-runtime.md.docs/USER-GUIDE.mddrops the rule from the documented permission list and explains why it is removed.Root cause
The rule was added in #768 / #819, before the rebrand to the
@opengsdscope (#126). It stayed in the current allow list after the unscoped name stopped pointing at this project, and the merge only ever adds entries from that list, so changing the constant alone would have left it in every existing install.Testing
How I verified the fix
describe('#5054 regression: …')block intests/install-regressions.test.cjs:npx/bunx/pnpx, with or without-y);Bash(npx @opengsd/gsd-core *),Bash(npx -y --package=@opengsd/gsd-core*),Bash(npx gsd-core)andBash(npx gsd-core:*)written by a user all survive;--claude --globalinstall over asettings.jsonthat already carries the rule removes it and keeps the user'sBash(git *);--claude --localdoes the same for.claude/settings.local.json, which is where local installs have written permissions since before enh(install): pre-populate settings.json permissions.allow/deny for gsd's own tool calls (Claude Code) #768 added them;GEMINI.mdhas no install line running the unscoped package (it is scoped toGEMINI.md, the one file the acceptance criteria name, soinstall-regressionsstays out of the docs-guard lane).Read(.planning/*); the uninstall test now seeds the old rule and asserts uninstall removes it.GEMINI.mdguard failed on the old file with'> npx gsd-core --antigravity --global', and the local-upgrade row fails against the basebin/install.js. All 85 tests intests/install-regressions.test.cjspass with the fix.--claude --global --config-dir <sandbox>, HOME sandboxed) over a copy of my own~/.claude/settings.json, which had the rule from an earlier install.permissions.allowwent from 8 entries to 7. The only removed entry wasBash(npx gsd-core *), nothing was added, and the order of the rest was unchanged.npm run lint:ciexits 0. Full suite (Node 24.19.0, macOS, TMPDIR unset, GSD_STRICT_LIVE_CONFIG_GUARD=1): 44,414 tests, 44,378 pass, 2 fail, 33 skipped. Both failures are environmental on this Mac and reproduce on a clean upstream/next checkout: runtime-launcher-parity (K2) and effort-local-install bug(effort): effort sync and resolve-execution read ~/.claude/agents on project-local installs — sync is a silent no-op and drift is reported as in-sync #4988 (/tmp vs /private/tmp).Regression test added?
Platforms tested
Runtimes tested
Checklist
Fixes #NNN— PR will be auto-closed if missingconfirmed-buglabelnpm test).changeset/fragment added if this is a user-facing fix (npm run changeset -- --type Fixed --pr <NNN> --body "...") — orno-changeloglabel appliedBreaking changes
Intended behavior change: Claude Code now asks before running
npx gsd-core …. GSD's own update path is unaffected, since it already usesnpx -y --package=@opengsd/gsd-core@<tag> -- gsd-core. Like the #2278 and #4221 retirements, the match is byte-equal, so a user who hand-wrote the identicalBash(npx gsd-core *)string loses it too. The install manifest never recorded permission strings, so ownership can't be told apart.Out of scope, per the triage brief: taking over the unscoped
gsd-core/gsd-toolsnpm names. That is a registry action, and it is the only thing that also protects installs that never upgrade.🤖 Generated with Claude Code