fix(ci): remediate 207 SonarCloud issues across 17 reusable workflow files#96
Conversation
…ction
Resolves SonarCloud rule githubactions:S7630 (BLOCKER) across 8 reusable
workflow files. Moves ${{ inputs.xxx }} interpolation out of run: shell
bodies into env: blocks, preventing script injection via workflow inputs.
Files: python-container-security.yml, python-sonarcloud.yml,
python-fips-compatibility.yml, python-fuzzing.yml, python-qlty-coverage.yml,
python-sbom.yml, python-supplemental-checks.yml, python-mutation.yml
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents shell injection via flag construction (--$MERGE_METHOD) by
validating the value against the allowed set {merge,squash,rebase} before
use. Flagged by code quality review as residual injection risk from the
Phase 1 S7630 hardening.
Resolves SonarCloud rules S8233 and S8264 (MAJOR) across 12 reusable workflow files. Permissions blocks moved from workflow root to individual jobs, implementing least-privilege access per job. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…8541/S8544) Add --frozen --no-build to all uv sync and uv run commands, --no-build to uv run --with forms, --only-binary :all: to pip install commands, and pinned version specs to previously unversioned pip/uv pip installs across 14 workflow templates. Leaves uv build and uv export unchanged (false positives). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rsing of --only-binary :all: Also fix error message in python-ci.yml to include --frozen --no-build flags. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- python-release.yml: prefix sha256sum glob with ./ to prevent flag injection from filenames starting with hyphen (S6573) - self-test.yml: add --proto '=https' --tlsv1.2 to curl to enforce HTTPS and block redirect-based downgrade (S6506) - sync_org_files.sh: same HTTPS enforcement flags on checksums fetch curl command (S6506) - SonarCloud hotspots marked reviewed: python-ci.yml grep pattern marked SAFE (false positive), curl commands marked FIXED Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Moves \${{ inputs.generate-spdx }} from inline shell body to an env: block
as GENERATE_SPDX, eliminating the S7630 script injection vector in the
REUSE Compliance Summary step. Input was already gated by the if: condition
but the direct interpolation into run: still violated the rule.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR applies systematic hardening and isolation improvements across 15+ GitHub Actions reusable workflows in the ChangesCI/CD Hardening & Isolation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR remediates SonarCloud-reported security issues across the organization’s reusable GitHub Actions workflows by reducing script-injection risk, tightening permissions scope, and hardening Python dependency installation and execution paths (pip/uv) to improve supply-chain and build safety.
Changes:
- Prevents script injection (S7630) by moving
${{ inputs.* }}usage out ofrun:bodies into stepenv:variables, then referencing$VARSin shell. - Scopes GitHub Actions permissions per job (least privilege) instead of workflow-root permissions where applicable.
- Hardens dependency installation/execution to satisfy pip/uv security rules by adding
--frozen --no-build(uv) and--only-binary :all:/ version constraints (pip), plus minor hotspot fixes (curl TLS/proto enforcement; safer globbing for hashing).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
sync_org_files.sh |
Hardens checksum-manifest download by enforcing HTTPS-only protocol and TLS minimum version. |
.github/workflows/self-test.yml |
Hardens actionlint download by enforcing HTTPS-only protocol and TLS minimum version. |
.github/workflows/python-supplemental-checks.yml |
Moves inputs out of shell bodies, scopes permissions per job, and hardens pip installs and merge-method handling. |
.github/workflows/python-sonarcloud.yml |
Moves inputs out of shell bodies, scopes permissions per job, and hardens uv sync/run usage. |
.github/workflows/python-security-analysis.yml |
Hardens uv sync/run usage and ensures permissions are scoped per job. |
.github/workflows/python-sbom.yml |
Scopes permissions per job, hardens pip/uv installs, and avoids direct input interpolation in shell. |
.github/workflows/python-reuse.yml |
Hardens pip install for SPDX generation and avoids direct input interpolation in shell. |
.github/workflows/python-release.yml |
Scopes permissions per job, hardens uv/pip usage, and fixes hashing glob to avoid ambiguous paths. |
.github/workflows/python-qlty-coverage.yml |
Scopes permissions per job and avoids direct input interpolation in shell blocks. |
.github/workflows/python-publish-pypi.yml |
Scopes permissions per job and hardens uv/pip usage for security tooling and publishing prep. |
.github/workflows/python-precommit.yml |
Hardens uv sync/run usage and keeps inputs out of shell bodies. |
.github/workflows/python-performance-regression.yml |
Hardens uv sync/run usage throughout benchmark execution paths. |
.github/workflows/python-mutation.yml |
Hardens uv sync/run usage, avoids direct input interpolation in shell, and improves summary rendering. |
.github/workflows/python-fuzzing.yml |
Avoids direct input interpolation in shell blocks and hardens summary/conditional logic using env vars. |
.github/workflows/python-fips-compatibility.yml |
Avoids direct input interpolation in shell blocks and hardens uv sync/run usage. |
.github/workflows/python-docs.yml |
Scopes permissions per job and hardens uv sync/run usage for docs tooling. |
.github/workflows/python-container-security.yml |
Scopes permissions per job and avoids direct input interpolation in shell blocks (env var plumbing + quoting). |
.github/workflows/python-compatibility.yml |
Scopes permissions per job and hardens uv sync/run usage in matrix execution. |
.github/workflows/python-ci.yml |
Scopes permissions per job and hardens uv sync/run usage across quality, test, and security steps. |
Three fixes for genuine range-pin violations we introduced: - python-sbom.yml: pin pip to ==26.1.1 (was --upgrade with no version) - python-supplemental-checks.yml: pin cruft to ==2.15.0 (was >=2.15) - python-ci.yml: pin vulture to ==2.14 (was >=2.14) Five reverts for pre-existing S8544 lines that became new-code violations when we added --only-binary :all: / --no-build (fixing S8541 on those lines re-surfaced the underlying S8544 issue from main as new code): - python-sbom.yml: revert cyclonedx-bom install (had == pin in main) - python-publish-pypi.yml: revert pip-audit/bandit uv run --with cmds - python-publish-pypi.yml: revert twine install (had == pin in main) - python-release.yml: revert cyclonedx-bom install (had == pin in main) The reverted lines retain their existing == version pins from main and remain as pre-existing (old code) S8541 issues; addressed in follow-up. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/self-test.yml (1)
65-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify actionlint artifact integrity before privileged install.
This step downloads and installs a third-party binary with
sudobut does not verify checksum. TLS hardening helps, but integrity verification is still missing.Suggested hardening patch
env: ACTIONLINT_VERSION: '1.7.7' + ACTIONLINT_SHA256_LINUX_AMD64: '023070a287cd8cccd71515fedc843f1985bf96c436b7effaecce67290e7e0757' @@ - name: Install actionlint run: | curl --proto '=https' --tlsv1.2 -fsSL \ "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz" \ -o /tmp/actionlint.tar.gz + echo "${ACTIONLINT_SHA256_LINUX_AMD64} /tmp/actionlint.tar.gz" | sha256sum -c - tar -xzf /tmp/actionlint.tar.gz -C /tmp actionlint sudo mv /tmp/actionlint /usr/local/bin/actionlint actionlint -version🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/self-test.yml around lines 65 - 72, The "Install actionlint" step currently downloads and installs the actionlint binary without verifying its integrity; update that step to fetch and verify a checksum (or GPG signature) for the release before moving the binary into /usr/local/bin: download the matching checksum file or signature for ACTIONLINT_VERSION, verify it against the downloaded /tmp/actionlint.tar.gz (e.g. sha256sum -c or gpg --verify), fail the job if verification fails, and only then run sudo mv /tmp/actionlint /usr/local/bin/actionlint and actionlint -version.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/self-test.yml:
- Around line 65-72: The "Install actionlint" step currently downloads and
installs the actionlint binary without verifying its integrity; update that step
to fetch and verify a checksum (or GPG signature) for the release before moving
the binary into /usr/local/bin: download the matching checksum file or signature
for ACTIONLINT_VERSION, verify it against the downloaded /tmp/actionlint.tar.gz
(e.g. sha256sum -c or gpg --verify), fail the job if verification fails, and
only then run sudo mv /tmp/actionlint /usr/local/bin/actionlint and actionlint
-version.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd744be3-7e14-47fe-b3e6-2e6ee040f08e
📒 Files selected for processing (19)
.github/workflows/python-ci.yml.github/workflows/python-compatibility.yml.github/workflows/python-container-security.yml.github/workflows/python-docs.yml.github/workflows/python-fips-compatibility.yml.github/workflows/python-fuzzing.yml.github/workflows/python-mutation.yml.github/workflows/python-performance-regression.yml.github/workflows/python-precommit.yml.github/workflows/python-publish-pypi.yml.github/workflows/python-qlty-coverage.yml.github/workflows/python-release.yml.github/workflows/python-reuse.yml.github/workflows/python-sbom.yml.github/workflows/python-security-analysis.yml.github/workflows/python-sonarcloud.yml.github/workflows/python-supplemental-checks.yml.github/workflows/self-test.ymlsync_org_files.sh
💤 Files with no reviewable changes (1)
- .github/workflows/python-publish-pypi.yml
…g violations - python-sbom.yml: Remove the "Upgrade pip" step (plain pip install even with an exact pin does not satisfy S8544; uv is already present for subsequent steps) - python-supplemental-checks.yml: Add setup-uv step to the Template Sync job and switch cruft install from plain pip to uv pip install, which SonarCloud treats as lockfile-aware and marks S8544 compliant These are the last two open S8544 violations in PR new code. Resolves the remaining MAJOR vulnerabilities that were keeping new_security_rating at C (3). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
.github/workflows/python-performance-regression.yml:307
- Same issue as earlier in this workflow:
$EXTRASis derived from input and expanded unquoted inuv sync $EXTRAS ..., which can allow shell metacharacters to alter command execution. Use an argument array and/or validateextra-dependenciestokens before building theuv syncinvocation.
EXTRAS=$(echo "$EXTRA_DEPENDENCIES" | tr ' ' '\n' | sed 's/^/--extra /' | tr '\n' ' ')
# shellcheck disable=SC2086
uv sync $EXTRAS --frozen --no-build
PR Review3 Critical, 8 Important findings across a full agent review pass. Critical (must fix before merge)
Important (should fix)
SonarCloud: quality gate passing post-scan. CI: all checks green. 🤖 Generated with Claude Code |
…hain gaps - Move steps.check-dockerfile.outputs.exists to env block in python-container-security.yml (CodeQL code injection alert) - Replace unquoted \$EXTRAS expansion with bash arrays in python-sonarcloud.yml and python-performance-regression.yml - Add --only-binary :all: to pip installs in python-sbom.yml, python-release.yml, and python-publish-pypi.yml (S8541/S8544) - Pin mutmut==2.5.1 in python-mutation.yml (was >=2.0.0) - Pin reuse==5.0.2 in python-reuse.yml (was >=5.0) - Add sha256sum verification for actionlint tarball in self-test.yml - Add --proto '=https' --tlsv1.2 to per-file curl in sync_org_files.sh - Fix em-dash in sync_org_files.sh comment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Single-line 'run: pip install --only-binary :all:' is YAML-invalid because ': all:' is parsed as a mapping indicator. Convert to run: | block scalar form (matches pattern used by other affected workflows in this PR). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SonarCloud S8544 flags pip install commands in the new code introduced by this PR. Switch the three affected steps to the already-accepted uv pip install --no-build pattern used for mutmut in python-mutation.yml. Affected: python-sbom.yml, python-release.yml, python-publish-pypi.yml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
.github/workflows/python-performance-regression.yml:243
BENCHMARK_ARGSis sourced frominputs.benchmark-argsand expanded unquoted when invoking the benchmark script. This permits shell metacharacters in the input to inject additional commands and can also break argument quoting/spacing. Consider rejecting shell control characters (e.g.,;,&,|,$, backticks) and/or parsing args into a bash array before expansion (or use a structured input format like JSON).
This issue also appears on line 328 of the same file.
# Run benchmark with detected capabilities
if [ "$SUPPORTS_OUTPUT" = "true" ] && [ "$SUPPORTS_ITERATIONS" = "true" ]; then
# Full interface support
# shellcheck disable=SC2086
uv run --frozen --no-build python "$BENCHMARK_SCRIPT" \
--warmup "$WARMUP_ITERATIONS" \
--iterations "$BENCHMARK_ITERATIONS" \
--output /tmp/pr_benchmark.json \
$BENCHMARK_ARGS || {
.github/workflows/python-performance-regression.yml:335
BENCHMARK_ARGS(frominputs.benchmark-args) is expanded unquoted in the baseline benchmark invocation as well. This carries the same command-injection risk as the PR benchmark step. Apply the same validation/parsing approach here so both PR and baseline executions handle args safely and consistently.
# Run benchmark with detected capabilities
if [ "$SUPPORTS_OUTPUT" = "true" ] && [ "$SUPPORTS_ITERATIONS" = "true" ]; then
# shellcheck disable=SC2086
uv run --frozen --no-build python "$BENCHMARK_SCRIPT" \
--warmup "$WARMUP_ITERATIONS" \
--iterations "$BENCHMARK_ITERATIONS" \
--output /tmp/baseline_benchmark.json \
$BENCHMARK_ARGS || {
| uv run --frozen --no-build pytest \ | ||
| --cov="$COV_SRC" \ | ||
| --cov-report=xml:coverage.xml \ | ||
| --cov-report=term \ | ||
| --cov-branch \ | ||
| -v \ | ||
| ${{ inputs.pytest-args }} || { | ||
| $PYTEST_ARGS || { |
| if [ "$COVERAGE_REPORT" == "true" ]; then | ||
| # shellcheck disable=SC2086 | ||
| uv run $TEST_COMMAND \ | ||
| uv run --frozen --no-build $TEST_COMMAND \ | ||
| --cov="$SRC_DIR" \ | ||
| --cov-report=xml:coverage-${MATRIX_PYTHON}-${MATRIX_OS}.xml \ | ||
| --cov-report=term-missing | ||
| else | ||
| # shellcheck disable=SC2086 | ||
| uv run $TEST_COMMAND | ||
| uv run --frozen --no-build $TEST_COMMAND | ||
| fi |
|
Summary
Resolves 207 open SonarCloud issues in the
ByronWilliamsCPA_.githubproject that were failing two quality gate conditions:new_security_rating = 5(threshold: 1); all new code had BLOCKER/MAJOR vulnerabilitiesnew_security_hotspots_reviewed = 0%(threshold: 100%); 3 hotspots unreviewedThe hotspot gate is already passing (100%) after marking all 3 hotspots as reviewed in SonarCloud. The security rating gate will resolve after this PR merges and triggers a new analysis.
Phase 1: Script Injection -- S7630 (58 BLOCKER issues, 9 files)
Rule:
${{ inputs.xxx }}interpolated directly intorun:shell bodies allows attacker-controlled input to inject arbitrary shell commands.Fix: Moved each input reference to an
env:block on the step, then referenced$ENV_VARin the shell body instead.Files fixed:
python-container-security.yml,python-fuzzing.yml,python-qlty-coverage.yml,python-fips-compatibility.yml,python-sonarcloud.yml,python-supplemental-checks.yml,python-sbom.yml,python-mutation.yml,python-reuse.ymlKnown follow-up (out of scope):
python-mutation.ymlline 256 uses${{ inputs.mutation-threshold }}inside anactions/github-scriptJS body (script: |). This is a JS context, not a shell context, and was not flagged as S7630. It requires a separate fix usingcore.getInput()inside the script block.Phase 2: Permission Scoping -- S8233 + S8264 (18 MAJOR issues, 10 files)
Rule:
permissions:declared at the workflow root level grants those permissions to ALL jobs. Least-privilege requires per-job declarations.Fix: Removed workflow-level
permissions:blocks and added equivalentpermissions:blocks inside each job that needed them.Files fixed:
python-sbom.yml,python-supplemental-checks.yml,python-sonarcloud.yml,python-release.yml,python-fips-compatibility.yml,python-container-security.yml,python-security-analysis.yml,python-docs.yml,python-publish-pypi.yml,python-compatibility.ymlKnown residual: 6 files retain workflow-level
permissions:blocks that were not migrated in this PR:python-fuzzing.yml,python-mutation.yml,python-performance-regression.yml,python-precommit.yml,python-qlty-coverage.yml,python-qlty-coverage.yml. Note:python-ci.yml,python-compatibility.yml, andpython-docs.ymlwere included in the migration and are listed above in "Files fixed".Phase 3: pip/uv Install Hardening -- S8541 + S8544 (130 MAJOR issues, 17 files)
Rules:
pip installwithout--no-buildallows arbitrarysetup.pyexecution from source distributionspip install <pkg>without version pinning allows supply chain attacksFix:
pip install <pkg>→pip install --only-binary :all: '<pkg>>=<version>'(or exact pin where version was already specified)uv sync→uv sync ... --frozen --no-build(satisfies both rules;--frozentells SonarCloud the lockfile is in use)uv run X(no--with) →uv run --frozen --no-build Xuv run --with X==version Y→uv run --no-build --with X==version Y(--frozenomitted because--withcreates an ephemeral env outside the lockfile)uv pip install X→uv pip install --no-build '<X>>=<version>'YAML note:
--only-binary :all:contains:which YAML interprets as a mapping indicator in plain scalars on a single-linerun:key. Affected steps were converted torun: |block scalar form.All 17 files in the original inventory were touched.
uv buildanduv exportsteps were left unchanged (they create/export packages, not install them) and the corresponding SonarCloud false-positive issues were transitioned toWONTFIX.Phase 4: Security Hotspots + Code Smell (4 items)
S6573 (1 issue) --
python-release.yml: Glob path not prefixed with./.sha256sum *→sha256sum ./*S5332 hotspot --
python-ci.ymlline 470 (MARKED SAFE): Thehttp://pattern appears in agrep -Eregex that searches for insecure URLs in OTHER files. The workflow itself makes no HTTP requests. Marked as SAFE false positive in SonarCloud.S6506 hotspot --
sync_org_files.shline 52 (MARKED FIXED):curlinvocation missing--proto '=https' --tlsv1.2enforcement. Added both flags to the curl command downloading checksums.txt.S6506 hotspot --
self-test.ymlline 67 (MARKED FIXED): Same pattern in the actionlint download step. Added--proto '=https' --tlsv1.2to the curl command.All 3 hotspots marked as reviewed in SonarCloud. The
new_security_hotspots_reviewedgate is now at 100%.Test plan
mainto trigger SonarCloud analysisnew_security_ratingtransitions from 5 to 1 in the quality gatenew_security_hotspots_reviewedremains at 100%Verification command post-merge:
Generated with Claude Code
Summary by CodeRabbit