fix(npm-audit-autofix): add head_ref input to orchestrator and validate in autofix#86
Conversation
…te in autofix CodeRabbit follow-up on PR #85: 1. build-test-publish.yml passed github.head_ref directly to security-scan-source.yml, breaking workflow_call callers that don't get github.head_ref populated. Added head_ref as an explicit input with fallback: inputs.head_ref || github.head_ref. 2. npm-audit-autofix.yml declared head_ref as required but GitHub Actions does not validate that required inputs are non-empty strings. Added an explicit validate head_ref step that fails early with a clear error if the value is empty.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTwo GitHub Actions workflow files are updated to improve Changeshead_ref input wiring and validation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/build-test-publish.yml (3)
227-229:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
actionlintas the first job in this workflow.Line 227 starts with
lint, but this workflow is required to runactionlintbefore any other job.As per coding guidelines, "Run actionlint as the first job in build-test-publish.yml before any other jobs to catch invalid workflows early."
🤖 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/build-test-publish.yml around lines 227 - 229, The build-test-publish.yml workflow is missing the actionlint job as the first job. Add a new actionlint job at the beginning of the jobs section, before the existing lint job. This actionlint job should validate the workflow syntax early in the CI pipeline. Ensure that actionlint runs before any other jobs (including the current lint job) to catch invalid workflows early as per the coding guidelines.Source: Coding guidelines
1-4:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing required workflow token permissions (
id-tokenandsecurity-events).This workflow has no explicit
permissionsblock, so it does not satisfy the required OIDC/trusted publishing and SARIF upload permissions.Suggested patch
name: universal workflow +permissions: + contents: read + id-token: write + security-events: write + on: workflow_call:As per coding guidelines, "GitHub Actions workflows MUST include
id-token: writepermission for OIDC" and "MUST includesecurity-events: writepermission to enable SARIF uploads," while applying minimal permissions by default.🤖 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/build-test-publish.yml around lines 1 - 4, The workflow named "universal workflow" is missing explicit permissions configuration required for OIDC and SARIF uploads. Add a permissions block at the root level of the workflow (after the on: workflow_call section) that grants id-token: write permission for OIDC/trusted publishing and security-events: write permission for SARIF uploads. Apply these permissions with minimal scope by only including what is required for the workflow's operations.Source: Coding guidelines
226-420:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDefine explicit job timeouts to prevent runaway workflow runs.
Jobs in this workflow currently omit
timeout-minutes; the repository policy requires optimized timeouts per job complexity.As per coding guidelines, "Set optimized timeouts for jobs based on complexity (5-60 minutes depending on tool and task complexity) to prevent runaway builds."
🤖 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/build-test-publish.yml around lines 226 - 420, Add timeout-minutes configuration to each job definition in the workflow to prevent runaway builds. For simple jobs like lint, security_scan_source, and post_publish_verification, use 10-15 minutes. For moderate complexity jobs like test_and_build, security_scan_artifacts, and the various publish jobs (publish_docker_image, publish_npm_libraries, publish_python_libraries, publish_firefox_extension, release_android_apk, release_github, publish_crates_io), use 20-30 minutes. For the set_git_tag and summarize jobs, use 10 minutes since they are lightweight. Add the timeout-minutes field directly under each job name at the same indentation level as the if, needs, uses, and with fields.Source: Coding guidelines
.github/workflows/npm-audit-autofix.yml (1)
52-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd required workflow permissions for OIDC and SARIF uploads.
This job defines write scopes for repo/PR operations, but the workflow still omits required
id-token: writeandsecurity-events: write.As per coding guidelines, "GitHub Actions workflows MUST include
id-token: writepermission for OIDC" and "MUST includesecurity-events: writepermission to enable SARIF uploads for security scanning."🤖 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/npm-audit-autofix.yml around lines 52 - 55, The permissions block in the npm-audit-autofix workflow is missing two required permissions for OIDC and SARIF uploads. Add `id-token: write` permission to enable OIDC authentication and `security-events: write` permission to enable SARIF uploads for security scanning. Both permissions should be added to the existing permissions object alongside the current `contents: write` and `pull-requests: write` entries.Source: Coding guidelines
🤖 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.
Inline comments:
In @.github/workflows/npm-audit-autofix.yml:
- Around line 64-69: The validate head_ref step directly expands the
inputs.head_ref template variable inside a shell condition, which creates a
command injection risk if the branch name contains malicious shell characters.
Instead of directly expanding the template within the shell script, define
inputs.head_ref as an environment variable in the step's env section first, then
reference that environment variable in the shell condition with proper quoting.
This ensures the template expansion happens in the GitHub Actions context rather
than within shell code execution.
---
Outside diff comments:
In @.github/workflows/build-test-publish.yml:
- Around line 227-229: The build-test-publish.yml workflow is missing the
actionlint job as the first job. Add a new actionlint job at the beginning of
the jobs section, before the existing lint job. This actionlint job should
validate the workflow syntax early in the CI pipeline. Ensure that actionlint
runs before any other jobs (including the current lint job) to catch invalid
workflows early as per the coding guidelines.
- Around line 1-4: The workflow named "universal workflow" is missing explicit
permissions configuration required for OIDC and SARIF uploads. Add a permissions
block at the root level of the workflow (after the on: workflow_call section)
that grants id-token: write permission for OIDC/trusted publishing and
security-events: write permission for SARIF uploads. Apply these permissions
with minimal scope by only including what is required for the workflow's
operations.
- Around line 226-420: Add timeout-minutes configuration to each job definition
in the workflow to prevent runaway builds. For simple jobs like lint,
security_scan_source, and post_publish_verification, use 10-15 minutes. For
moderate complexity jobs like test_and_build, security_scan_artifacts, and the
various publish jobs (publish_docker_image, publish_npm_libraries,
publish_python_libraries, publish_firefox_extension, release_android_apk,
release_github, publish_crates_io), use 20-30 minutes. For the set_git_tag and
summarize jobs, use 10 minutes since they are lightweight. Add the
timeout-minutes field directly under each job name at the same indentation level
as the if, needs, uses, and with fields.
In @.github/workflows/npm-audit-autofix.yml:
- Around line 52-55: The permissions block in the npm-audit-autofix workflow is
missing two required permissions for OIDC and SARIF uploads. Add `id-token:
write` permission to enable OIDC authentication and `security-events: write`
permission to enable SARIF uploads for security scanning. Both permissions
should be added to the existing permissions object alongside the current
`contents: write` and `pull-requests: write` entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53bda121-809b-4f5e-b14d-fb35878adbcc
📒 Files selected for processing (2)
.github/workflows/build-test-publish.yml.github/workflows/npm-audit-autofix.yml
Direct template expansion of inputs.head_ref inside a shell condition is a command injection risk. Move the value into an env variable and reference that instead.
1 participant
Summary
CodeRabbit follow-up on PR #85:
build-test-publish.yml:github.head_refwas hardcoded when passing tosecurity-scan-source.yml. Forworkflow_callcallersgithub.head_refis always empty, so callers had no way to provide the branch name. Addedhead_refas an explicit optional input with fallbackinputs.head_ref || github.head_ref— directpull_requesttriggers continue to work unchanged.npm-audit-autofix.yml:head_refwas declaredrequired: truebut GitHub Actions only validates that the key exists, not that the value is non-empty. An empty string would silently produce malformed branch names (audit-fix/-<sha>). Added an explicitvalidate head_refstep that fails early with a clear::error::annotation.Test plan
actionlintpasses on all changed filesworkflow_callcan now passhead_ref: ${{ github.head_ref }}explicitly and it flows correctly through the chainhead_reftonpm-audit-autofix.ymlnow fails immediately with a descriptive error instead of silently producing a broken branch nameSummary by CodeRabbit