fix(security): harden shell arguments and workflow permissions#488
Conversation
commit: |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fea7308. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR hardens shell-mode command execution to prevent argument injection by ensuring environment-derived values are passed as positional parameters (not parsed as shell source), and updates GitHub Actions workflows to explicitly scope GITHUB_TOKEN permissions.
Changes:
- Updated
defaultExecutorto run shell-mode commands via/bin/sh -c 'exec "$@"'with the executable/args passed as positional parameters. - Added a regression test that asserts shell metacharacters remain literal in shell-mode execution.
- Added explicit workflow
permissionsblocks for CI and Sentry release workflows.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/utils/command.ts |
Reworks shell-mode execution to avoid shell parsing of argument values while preserving readable logging. |
src/utils/__tests__/command.test.ts |
Adds a real-shell regression test covering common shell metacharacters and quoting cases. |
CHANGELOG.md |
Documents the security hardening and workflow permission adjustments. |
.github/workflows/sentry.yml |
Sets explicit contents: read token permissions for the Sentry release workflow. |
.github/workflows/ci.yml |
Sets explicit token permissions for CI (currently too restrictive for existing steps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fea7308 to
c8e3e67
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:11
- With workflow-level
permissions: contents: read, theGITHUB_TOKENwill not be allowed to post/update PR comments. This workflow runsnpx pkg-pr-new publish --comment=updateonpull_request, which will typically require write permissions (commonlyissues: writeand/orpull-requests: write) and will start failing with 403s, changing existing behavior.
permissions:
contents: read
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:11
- The workflow now restricts
GITHUB_TOKENtocontents: read, but this job runsnpx pkg-pr-new publish --comment=update(line 57), which posts/updates a pull-request comment via the GitHub API and typically requiresissues: writepermissions. With the current permissions block, that step will likely start failing consistently, changing CI behavior (even ifcontinue-on-error: truekeeps the job green).
permissions:
contents: read
CodeQL reports three security problems on
main: environment-derived valuesreach a shell command string, and the CI and Sentry workflows inherit whatever
repository permissions are configured for
GITHUB_TOKEN. The command path cangive metacharacters executable meaning, while inherited workflow permissions
can silently become broader when repository defaults change.
Shell-mode requests now execute through a fixed
/usr/bin/env --relay and passthe executable and arguments only as argv. Dynamic values never become shell
source, and
--ensures an executable name beginning with a dash cannot beconsumed as an
envoption. The CI and Sentry workflows now explicitly limitrepository access to
contents: read;pkg-pr-newcontinues to post throughits installed GitHub App.
Direct command execution is unchanged. All current shell-mode callers invoke
external programs, so their successful behavior is unchanged; missing programs
also remain an exit-127 command response instead of becoming a rejected spawn.
Shell builtins are intentionally not part of this relay. CI package previews,
cache use, and artifact uploads retain their existing behavior.
Real-host regression tests verify that command substitution, backticks,
separators, variable references, quotes, and a leading-dash executable remain
literal argv, and that missing executables preserve the existing failure
contract. The local unit suite, typecheck, build, lint, and formatting checks
all pass. GitHub's Ubuntu build/test job, CodeQL analyses, Semgrep, Warden
security review, dependency review, secret scan, and package preview also pass.