From 750c9f93c2cf24733762a811db80e21c4da59edb Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 17 Aug 2026 12:11:27 -0700 Subject: [PATCH] Fix CodeQL alerts: workflow permissions and shell command construction 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. --- .github/workflows/cd.yml | 2 ++ .github/workflows/stale.yml | 3 +++ .github/workflows/triage.yml | 2 ++ lib/packs.rb | 12 ++++++------ lib/packs/private.rb | 10 ++++++---- spec/packs/private/cli_spec.rb | 4 ++-- 6 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 4d244f7..cf7425d 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -8,6 +8,8 @@ on: jobs: call-workflow-from-shared-config: + permissions: + contents: write uses: rubyatscale/shared-config/.github/workflows/cd.yml@main secrets: inherit diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 0287d52..2696450 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -5,4 +5,7 @@ on: - cron: '0 0 * * *' jobs: call-workflow-from-shared-config: + permissions: + issues: write + pull-requests: write uses: rubyatscale/shared-config/.github/workflows/stale.yml@main diff --git a/.github/workflows/triage.yml b/.github/workflows/triage.yml index 74bb1d9..7c492ee 100644 --- a/.github/workflows/triage.yml +++ b/.github/workflows/triage.yml @@ -6,4 +6,6 @@ on: - opened jobs: call-workflow-from-shared-config: + permissions: + issues: write uses: rubyatscale/shared-config/.github/workflows/triage.yml@main diff --git a/lib/packs.rb b/lib/packs.rb index 0f189e1..e35e9bf 100644 --- a/lib/packs.rb +++ b/lib/packs.rb @@ -33,27 +33,27 @@ def self.start_interactive_mode! sig { returns(T::Boolean) } def self.update if Packs.config.use_pks - Private.system_with('bin/pks update') + Private.system_with(['bin/pks', 'update']) else - Private.system_with('bin/packwerk update-todo') + Private.system_with(['bin/packwerk', 'update-todo']) end end sig { returns(T::Boolean) } def self.validate if Packs.config.use_pks - Private.system_with('bin/pks validate') + Private.system_with(['bin/pks', 'validate']) else - Private.system_with('bin/packwerk validate') + Private.system_with(['bin/packwerk', 'validate']) end end sig { params(files: T::Array[String]).returns(T::Boolean) } def self.check(files) if Packs.config.use_pks - Private.system_with("bin/pks check #{files.join(' ')}") + Private.system_with(['bin/pks', 'check', *files]) else - Private.system_with("bin/packwerk check #{files.join(' ')}") + Private.system_with(['bin/packwerk', 'check', *files]) end end diff --git a/lib/packs/private.rb b/lib/packs/private.rb index 1cb9db3..9d02504 100644 --- a/lib/packs/private.rb +++ b/lib/packs/private.rb @@ -783,10 +783,12 @@ def self.exit_with(code) exit code end - # This function exists to give us something to stub in test - sig { params(command: String).returns(T::Boolean) } - def self.system_with(command) - T.cast(system(command), T::Boolean) + # This function exists to give us something to stub in test. + # Each argv entry is passed to `system` as a separate argument, so the + # command is executed directly and never interpreted by a shell. + sig { params(argv: T::Array[String]).returns(T::Boolean) } + def self.system_with(argv) + T.cast(T.unsafe(Kernel).system(*argv), T::Boolean) end end diff --git a/spec/packs/private/cli_spec.rb b/spec/packs/private/cli_spec.rb index f6f8536..4a935d0 100644 --- a/spec/packs/private/cli_spec.rb +++ b/spec/packs/private/cli_spec.rb @@ -92,7 +92,7 @@ def expect_failure it 'exits successfully' do expect_success expect(Packs.const_get(:Private)).to receive(:system_with).with( - 'bin/packwerk validate' + ['bin/packwerk', 'validate'] ).and_return(true) described_class.start(['validate']) end @@ -102,7 +102,7 @@ def expect_failure it 'exits unsuccessfully' do expect_failure expect(Packs.const_get(:Private)).to receive(:system_with).with( - 'bin/packwerk validate' + ['bin/packwerk', 'validate'] ).and_return(false) described_class.start(['validate']) end