Skip to content

Fix CodeQL alerts: workflow permissions and shell command construction - #190

Merged
dduugg merged 1 commit into
mainfrom
fix-code-scanning-alerts
Aug 17, 2026
Merged

Fix CodeQL alerts: workflow permissions and shell command construction#190
dduugg merged 1 commit into
mainfrom
fix-code-scanning-alerts

Conversation

@dduugg

@dduugg dduugg commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Adds explicit least-privilege GITHUB_TOKEN permissions to the three
reusable-workflow callers that had none (alerts #4, #2, #1), matching the
job-level style already used in ci.yml and codeql.yml:

  • cd.yml -> contents: write. shared-config's cd.yml checks out with
    persisted credentials and runs discourse/publish-rubygems-action
    (rake release does a raw git push of the version tag) followed by
    gh release create, both of which need write access to contents.
  • stale.yml -> issues: write + pull-requests: write. shared-config's
    stale.yml runs actions/stale, which comments on and closes both stale
    issues and stale pull requests.
  • triage.yml -> issues: write. shared-config's triage.yml runs
    gh issue edit --add-label triage.

Fixes the two rb/shell-command-constructed-from-input alerts (#5, #6) in
Packs.check, where the caller-supplied file list was interpolated into a
single string handed to Kernel#system and therefore to /bin/sh. A path
containing a space, quote, semicolon or backtick would have been
re-interpreted by the shell.

Private.system_with now takes the argv array and splats it into system,
so the command is exec'd directly and never goes through a shell. All six
callers in lib/packs.rb and the two specs that stub the seam are updated.
The sig is params(argv: T::Array[String]); T.unsafe is needed only because
Sorbet cannot type-check a splat of statically unknown size.

srb tc, rspec (119 examples) and rubocop all pass; actionlint is clean on
the workflow files.

Alerts resolved

  • #6 rb/shell-command-constructed-from-input (medium) — lib/packs.rb:56
  • #5 rb/shell-command-constructed-from-input (medium) — lib/packs.rb:54
  • #4 actions/missing-workflow-permissions (medium) — .github/workflows/cd.yml:11
  • #2 actions/missing-workflow-permissions (medium) — .github/workflows/stale.yml:8
  • #1 actions/missing-workflow-permissions (medium) — .github/workflows/triage.yml:9

Verification

  • Every job in every flagged workflow now has an effective permissions: block (cross-checked by parsing the YAML against the alert list).
  • actionlint output is byte-identical to main — no new findings introduced.
  • Test suite, type check and linter all pass locally.
  • Regex/argv rewrites were fuzz-checked for exact equivalence against the originals.
  • codeql.yml untouched.
  • I bumped the gem version (or don't need to) 💎

Version not bumped — Packs.check no longer routes through a shell, so a maintainer should decide whether this warrants a release.

Adds explicit least-privilege GITHUB_TOKEN permissions to the three
reusable-workflow callers that had none (alerts #4, #2, #1), matching the
job-level style already used in ci.yml and codeql.yml:

- cd.yml -> contents: write. shared-config's cd.yml checks out with
  persisted credentials and runs discourse/publish-rubygems-action
  (rake release does a raw git push of the version tag) followed by
  gh release create, both of which need write access to contents.
- stale.yml -> issues: write + pull-requests: write. shared-config's
  stale.yml runs actions/stale, which comments on and closes both stale
  issues and stale pull requests.
- triage.yml -> issues: write. shared-config's triage.yml runs
  gh issue edit --add-label triage.

Fixes the two rb/shell-command-constructed-from-input alerts (#5, #6) in
Packs.check, where the caller-supplied file list was interpolated into a
single string handed to Kernel#system and therefore to /bin/sh. A path
containing a space, quote, semicolon or backtick would have been
re-interpreted by the shell.

Private.system_with now takes the argv array and splats it into system,
so the command is exec'd directly and never goes through a shell. All six
callers in lib/packs.rb and the two specs that stub the seam are updated.
The sig is params(argv: T::Array[String]); T.unsafe is needed only because
Sorbet cannot type-check a splat of statically unknown size.

srb tc, rspec (119 examples) and rubocop all pass; actionlint is clean on
the workflow files.
@dduugg
dduugg requested a review from a team as a code owner August 17, 2026 19:34
@github-project-automation github-project-automation Bot moved this to Triage in Modularity Aug 17, 2026
@dduugg
dduugg merged commit 3de670c into main Aug 17, 2026
9 checks passed
@dduugg
dduugg deleted the fix-code-scanning-alerts branch August 17, 2026 19:40
@github-project-automation github-project-automation Bot moved this from Triage to Done in Modularity Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant