fix(lint): enforce SHA pinning for externals, warnings for internals#170
Merged
fix(lint): enforce SHA pinning for externals, warnings for internals#170
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughBumped reusable workflow refs in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
lerian-studio
added
size/XS
🔍 Lint Analysis
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…unpinned-tag Pinned Actions Check: - External actions must be pinned by commit SHA (40-64 hex chars) - Internal actions (LerianStudio/) accept any semver ref including pre-releases - Internal also allows develop/main branch refs CodeQL Reporter: - Suppress actions/unpinned-tag findings from PR comments — handled by our own pinned-actions check with org-aware logic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lint/pinned-actions/action.yml`:
- Line 61: The semver prerelease part of the regex in the conditional that
validates refs (the printf/grep line) is too permissive and allows invalid forms
while rejecting valid hyphenated identifiers; update the prerelease pattern in
that regex (the one currently using [a-zA-Z0-9.]+) to the stricter RFC-like
pattern [0-9A-Za-z-]+(\.[0-9A-Za-z-]+)* so prerelease identifiers like beta-1
are accepted and malformed sequences like .. are rejected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ae2f0007-b179-487c-b926-e71eb8bb5c3a
📒 Files selected for processing (1)
src/lint/pinned-actions/action.yml
Fetch warning-level annotations from all checks (not just failures) and display them in a collapsible "⚠️ Warnings" section in the PR lint comment. This surfaces internal pinned-actions warnings (e.g., pre-release tags) without cluttering the main summary table.
lerian-studio
added
size/S
bedatty
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lint/pinned-actions/action.yml`:
- Around line 67-68: The SHA validation is case-sensitive and rejects uppercase
hex; update the grep check that inspects "$ref" (the line using grep -Eq
'^[0-9a-f]{40,64}$') to allow uppercase hex as well by using a character class
that includes A-F (e.g., [0-9a-fA-F]) so both lowercase and uppercase SHAs pass
validation.
- Around line 66-72: The validation currently rejects any external action not
pinned to a full hex SHA (the block that checks printf '%s\n' "$ref" | grep -Eq
'^[0-9a-f]{40,64}$' and emits the "::error ... External action not pinned by
SHA" message), but your repo policy allows third-party actions pinned to
semantic-version tags; update the logic to treat valid semver tags as
acceptable: expand the conditional to also accept refs that match semver/tag
patterns (e.g. /^v[0-9]+(\.[0-9]+){0,2}([-.+][0-9A-Za-z.-]+)?$/ or other
approved tag forms), so "$ref" and "$normalized" that are semver tags bypass the
SHA-only error, and adjust the error text to reflect the policy (mention
acceptable formats) while still counting violations via the violations variable.
In `@src/security/codeql-reporter/action.yml`:
- Around line 128-133: Replace the hardcoded SUPPRESSED_RULES array with a
configurable input: add a new action input (e.g., suppressed-rules) defaulting
to "actions/unpinned-tag" and read it at runtime
(process.env.SUPPRESSED_RULES_INPUT) in the script that builds the report; parse
the string as a comma-separated list (trim entries and ignore empties) to
produce SUPPRESSED_RULES and then apply it when filtering readSarifFiles() so
callers can override the default suppression when the pinned-actions check is
not present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 35156765-5e38-4ed3-95b1-5dab2f7be9ba
📒 Files selected for processing (2)
src/lint/pinned-actions/action.ymlsrc/security/codeql-reporter/action.yml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/notify/pr-lint-reporter/action.yml`:
- Around line 224-255: The warnings block duplicates the failure rendering
logic; extract the grouping-and-formatting logic into a single helper (e.g.,
renderAnnotationsGroupedByFile or formatAnnotationSection) that accepts an array
of annotations and returns the markdown string, and then replace the duplicated
code in the warnings flow (the block using jobWarningAnnotations, byFile, and
iterating w.start_line) and the failure rendering flow to call that helper;
ensure the helper preserves existing behavior (group by a.path or '__general__',
render `**\`file\`**`, include line info when start_line exists, and emit plain
messages for '__general__') so both sections remain identical.
- Around line 162-166: The current call to
github.rest.actions.listJobsForWorkflowRun only fetches the first page (default
30) and truncates jobs; replace this with a paginated fetch (e.g., use
github.paginate or an explicit paging loop) to collect all pages into a single
jobs array before doing annotation lookup. Specifically, change the usage around
listJobsForWorkflowRun / const { data: { jobs } } to call
github.paginate(github.rest.actions.listJobsForWorkflowRun, { owner:
context.repo.owner, repo: context.repo.repo, run_id: context.runId }) (or
equivalent paging loop) and assign the flattened result to the jobs variable so
subsequent annotation processing (which iterates jobs) covers all workflow jobs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 72e910f7-a7da-4f7c-96b6-0586520a1b82
📒 Files selected for processing (1)
src/notify/pr-lint-reporter/action.yml
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/security/codeql-reporter/action.yml (1)
128-133:⚠️ Potential issue | 🟠 MajorMake suppressed SARIF rules configurable.
Line 133 filters
actions/unpinned-tagbeforefindings-count,has-findings, and thefail-on-findingsgate are computed. Any caller that does not also run the org-aware pinned-actions check will silently under-report real findings and can pass the gate incorrectly.Proposed fix
inputs: github-token: description: GitHub token with pull-requests:write and issues:write permissions required: true sarif-path: description: Path to the CodeQL SARIF output directory required: false default: '../results' languages: description: Comma-separated list of languages analyzed (for display purposes) required: true fail-on-findings: description: 'Fail the step with exit code 1 when security findings are detected (true/false)' required: false default: 'false' + suppressed-rules: + description: 'Comma-separated SARIF rule IDs to suppress from the PR report' + required: false + default: 'actions/unpinned-tag' @@ env: SARIF_PATH: ${{ inputs.sarif-path }} LANGUAGES: ${{ inputs.languages }} + SUPPRESSED_RULES_INPUT: ${{ inputs.suppressed-rules }} @@ - const SUPPRESSED_RULES = ['actions/unpinned-tag']; + const SUPPRESSED_RULES = (process.env.SUPPRESSED_RULES_INPUT || '') + .split(',') + .map((rule) => rule.trim()) + .filter(Boolean);As per coding guidelines, every composite action must be self-contained with sensible defaults in
action.yml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/security/codeql-reporter/action.yml` around lines 128 - 133, The SARIF suppression list (SUPPRESSED_RULES) is hard-coded and applied before computing findings-count/has-findings/fail-on-findings, which can under-report results; change action.yml to expose a new string input (e.g., suppressed_rules) with a sensible default that includes "actions/unpinned-tag", parse that input where readSarifFiles() is called, and remove the static SUPPRESSED_RULES constant; also ensure filtering using the parsed suppressed_rules is deferred so that findings-count, has-findings, and fail-on-findings are computed from the unfiltered findings and the suppression is only applied when generating the final report (reference SUPPRESSED_RULES, readSarifFiles(), findings filter, and the findings-count/has-findings/fail-on-findings gates).src/notify/pr-lint-reporter/action.yml (1)
162-166:⚠️ Potential issue | 🟠 MajorPaginate the workflow job fetch before annotation lookup.
Line 162 still loads only one page of jobs. Now that this block collects annotations for both failed and successful checks, later jobs in larger runs will be skipped entirely, so both the failures section and the new warnings section can be incomplete.
Proposed fix
- const { data: { jobs } } = await github.rest.actions.listJobsForWorkflowRun({ - owner: context.repo.owner, - repo: context.repo.repo, - run_id: context.runId, - }); + const jobs = await github.paginate(github.rest.actions.listJobsForWorkflowRun, { + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.runId, + per_page: 100, + });GitHub REST API `actions.listJobsForWorkflowRun` pagination: does a single call return only one page of jobs, and is `github.paginate(github.rest.actions.listJobsForWorkflowRun, { owner, repo, run_id, per_page: 100 })` the correct way to collect all jobs for a workflow run?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/notify/pr-lint-reporter/action.yml` around lines 162 - 166, The code currently calls github.rest.actions.listJobsForWorkflowRun and only retrieves a single page of jobs (assigned to jobs), which misses later jobs in large runs; replace the single-page call with GitHub REST pagination by using github.paginate against github.rest.actions.listJobsForWorkflowRun (pass the same parameters owner, repo, run_id and per_page: 100) to collect all jobs before you iterate for annotations so both failures and warnings include every job; update references to the resulting jobs collection where annotations are looked up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/notify/pr-lint-reporter/action.yml`:
- Around line 162-166: The code currently calls
github.rest.actions.listJobsForWorkflowRun and only retrieves a single page of
jobs (assigned to jobs), which misses later jobs in large runs; replace the
single-page call with GitHub REST pagination by using github.paginate against
github.rest.actions.listJobsForWorkflowRun (pass the same parameters owner,
repo, run_id and per_page: 100) to collect all jobs before you iterate for
annotations so both failures and warnings include every job; update references
to the resulting jobs collection where annotations are looked up.
In `@src/security/codeql-reporter/action.yml`:
- Around line 128-133: The SARIF suppression list (SUPPRESSED_RULES) is
hard-coded and applied before computing
findings-count/has-findings/fail-on-findings, which can under-report results;
change action.yml to expose a new string input (e.g., suppressed_rules) with a
sensible default that includes "actions/unpinned-tag", parse that input where
readSarifFiles() is called, and remove the static SUPPRESSED_RULES constant;
also ensure filtering using the parsed suppressed_rules is deferred so that
findings-count, has-findings, and fail-on-findings are computed from the
unfiltered findings and the suppression is only applied when generating the
final report (reference SUPPRESSED_RULES, readSarifFiles(), findings filter, and
the findings-count/has-findings/fail-on-findings gates).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ba818843-dfa1-4596-8836-e5ea588873f6
📒 Files selected for processing (2)
src/notify/pr-lint-reporter/action.ymlsrc/security/codeql-reporter/action.yml
bedatty
dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot]
All comments addressed in commit c8385d4.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub Actions Shared Workflows
Description
Pin pr-validation composite refs to
v1.20.0-beta.1and overhaul the action pinning enforcement:LerianStudio/) accepts any semver ref including pre-releases anddevelop/mainbranchesactions/unpinned-tagfindings from PR comments — handled by our own pinned-actions check with org-aware logicType of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBREAKING CHANGE: Callers must update their configuration after this PRBreaking Changes
External actions pinned by tag (e.g.,
@v6) will now fail the Pinned Actions Check. They must be pinned by commit SHA with a version comment (e.g.,@abc123 # v6). Internal (LerianStudio/) actions are unaffected.Testing
@developor the beta tagCaller repo / workflow run:
Related Issues
Summary by CodeRabbit
Chores
Bug Fixes
New Features