refactor(ci): extract cora review into composite action#46
Conversation
- Move Infisical identity-id to secret (INFISICAL_IDENTITY_ID) - Remove 2>&1 redirect (was mixing stderr into SARIF JSON) - Remove || true (was masking failures) - Guard SARIF upload with hashFiles check
- Create .github/actions/cora-review/action.yml - Configurable: base-branch, severity, infisical settings - Handles: SARIF upload, PR comment (update existing), blocking check - Suppresses cargo build noise (2>/dev/null) - No GITHUB_STEP_SUMMARY branding pollution - Slim ci.yml: cora-review job now 5 lines (uses composite action) - Fix duplicate 'error' in blocking check python script
|
Warning Review limit reached
More reviews will be available in 53 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR creates a reusable GitHub composite action for cora AI code review that fetches Infisical secrets, runs the review tool to generate SARIF, uploads results to Code Scanning, posts PR comments summarizing findings, and enforces a blocking gate for critical errors. The CI workflow is refactored to use this action with explicit permissions, replacing prior inline steps. ChangesCora Review Action and CI Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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}
🔍 Cora AI Code Review❌ Blocked — critical issues found. 🔴 Error (2)
Review powered by cora-cli · BYOK · MIT |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/actions/cora-review/action.yml:
- Around line 55-60: The Upload SARIF to GitHub Code Scanning step can run on a
zero-byte cora-results.sarif because hashFiles returns non-empty for an existing
empty file; add a preceding check step (e.g., "Check SARIF file") that tests
size and/or JSON validity (run test -s cora-results.sarif && jq -e .
cora-results.sarif >/dev/null) and sets an output like sarif_valid=true/false,
then change the Upload SARIF step's if to depend on that output (e.g., if:
steps.check-sarif.outputs.sarif_valid == 'true') so upload-sarif only runs when
cora-results.sarif is non-empty and valid JSON.
- Around line 62-67: In the "Post PR comment" step using
actions/github-script@v7, remove the misleading env GH_TOKEN and wire the
provided inputs.github-token into the action by setting with.github-token: ${{
inputs.github-token }} so the Octokit client actually uses the input token; then
update the dedup logic that currently checks c.user.login ===
'github-actions[bot]' to a token-agnostic check (for example compare
c.user.login to github.actor or check c.user.type === 'Bot' and a unique comment
marker in c.body) so prior comments posted with non-default tokens are detected
and updated.
- Around line 44-53: In the GitHub Action shell step that runs "cargo build
--release" and "./target/release/cora review" remove the trailing "|| true" from
the "./target/release/cora review" invocation so process failures propagate and
the job fails instead of producing an empty/missing cora-results.sarif; also
stop swallowing errors by removing the "2>/dev/null" redirections on the
build/review commands (retain output redirection to "cora-results.sarif" but let
stderr surface) so failures in cargo build or the cora review binary are visible
and cause CI to fail.
In @.github/workflows/ci.yml:
- Around line 73-77: Top-level workflow permissions currently grant write scopes
(security-events: write, pull-requests: write, id-token: write) which are
inherited by all jobs; remove those write-scoped entries and restrict the
top-level permissions to only minimal read (e.g., contents: read) so jobs like
check, fmt, clippy, test, build do not get elevated access, then add any
required write permissions at the job level (for example ensure the cora-review
job keeps its specific write permissions) so only jobs that need write scopes
receive them.
🪄 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: 72580e48-6d55-434f-bf79-c31f7732d07c
📒 Files selected for processing (2)
.github/actions/cora-review/action.yml.github/workflows/ci.yml
| - name: Run cora review | ||
| shell: bash | ||
| run: | | ||
| cargo build --release 2>/dev/null | ||
| ./target/release/cora review \ | ||
| --base ${{ inputs.base-branch }} \ | ||
| --format sarif \ | ||
| --severity ${{ inputs.severity }} \ | ||
| --quiet \ | ||
| > cora-results.sarif 2>/dev/null || true |
There was a problem hiding this comment.
Remove || true so CI fails on cora/build execution errors instead of reporting “✅ No issues found”.
With cargo build ... 2>/dev/null and || true on the ./target/release/cora review command, crashes/build failures can leave cora-results.sarif missing/empty; downstream steps treat that as “no issues” and the blocking gate is skipped.
🐛 Proposed fix to fail loudly on build/run errors
- name: Run cora review
shell: bash
run: |
- cargo build --release 2>/dev/null
- ./target/release/cora review \
- --base ${{ inputs.base-branch }} \
- --format sarif \
- --severity ${{ inputs.severity }} \
- --quiet \
- > cora-results.sarif 2>/dev/null || true
+ set -euo pipefail
+ cargo build --release --quiet
+ # Don't fail here on non-zero exit: cora returns non-zero when findings exist.
+ # Capture the exit code and only fail on unexpected (execution) errors.
+ set +e
+ ./target/release/cora review \
+ --base "${{ inputs.base-branch }}" \
+ --format sarif \
+ --severity "${{ inputs.severity }}" \
+ --quiet \
+ > cora-results.sarif 2>cora-stderr.log
+ rc=$?
+ set -e
+ # Treat a missing/empty SARIF as an execution failure.
+ if [ ! -s cora-results.sarif ]; then
+ echo "::error::cora did not produce a SARIF report (exit $rc)."
+ cat cora-stderr.log || true
+ exit 1
+ fi🤖 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/actions/cora-review/action.yml around lines 44 - 53, In the GitHub
Action shell step that runs "cargo build --release" and "./target/release/cora
review" remove the trailing "|| true" from the "./target/release/cora review"
invocation so process failures propagate and the job fails instead of producing
an empty/missing cora-results.sarif; also stop swallowing errors by removing the
"2>/dev/null" redirections on the build/review commands (retain output
redirection to "cora-results.sarif" but let stderr surface) so failures in cargo
build or the cora review binary are visible and cause CI to fail.
| - name: Upload SARIF to GitHub Code Scanning | ||
| if: always() && hashFiles('cora-results.sarif') != '' | ||
| uses: github/codeql-action/upload-sarif@v4 | ||
| with: | ||
| sarif_file: cora-results.sarif | ||
| category: cora-review |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does GitHub Actions hashFiles() return an empty string for a zero-byte (empty) file, or only when no files match the pattern?
💡 Result:
Yes—hashFiles returns an empty string when no files match the pattern. However, for a zero-byte (empty) file that matches the pattern, hashFiles returns the hash of the empty file content (not an empty string). Evidence: - GitHub’s docs/runner behavior documents that hashFiles may return an empty string when the pattern does not match any file. This is described/added via a documentation change request explaining “if the path does not match any known path, an empty string would be returned” and references the runner issue for that behavior [1][2]. - In the runner implementation, hashFiles collects files by the provided glob patterns; if the search yields no matches, the function’s return value becomes empty (i.e., it does not compute a content hash) [3]. So the “empty string” return is tied to “no matching files,” not “matched file has 0 bytes.” (A zero-byte file is still a matched file, so it’s hashed; the empty file’s content hash would be the hash of an empty byte sequence, not blank.) [3][1]
Citations:
- 1: Added explanation about hashFiles returning an empty string github/docs#15719
- 2: hashFiles() may output empty string when patterns are not matched actions/runner#556
- 3: https://github.com/actions/runner/blob/main/src/Runner.Worker/Expressions/HashFilesFunction.cs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/actions/cora-review/action.yml"
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,140p'Repository: ajianaz/cora-cli
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/actions/cora-review/action.yml"
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,140p'Repository: ajianaz/cora-cli
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/actions/cora-review/action.yml"
echo "== Relevant section: $FILE =="
# print with line numbers using cat -n (nl unavailable)
cat -n "$FILE" | sed -n '35,90p'Repository: ajianaz/cora-cli
Length of output: 2566
Fix hashFiles guard so empty SARIF doesn’t get uploaded
The step > cora-results.sarif 2>/dev/null || true will still create cora-results.sarif as a zero-byte file when the review command fails. hashFiles('cora-results.sarif') returns a hash for a matched (but empty) file, and only returns '' when the glob matches no files—so if: ... && hashFiles('cora-results.sarif') != '' can pass and upload-sarif will try to upload invalid/empty JSON. Gate the upload on file size (e.g., test -s cora-results.sarif) and/or JSON validity via a preceding check step, then use that output in the upload step’s if.
🤖 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/actions/cora-review/action.yml around lines 55 - 60, The Upload
SARIF to GitHub Code Scanning step can run on a zero-byte cora-results.sarif
because hashFiles returns non-empty for an existing empty file; add a preceding
check step (e.g., "Check SARIF file") that tests size and/or JSON validity (run
test -s cora-results.sarif && jq -e . cora-results.sarif >/dev/null) and sets an
output like sarif_valid=true/false, then change the Upload SARIF step's if to
depend on that output (e.g., if: steps.check-sarif.outputs.sarif_valid ==
'true') so upload-sarif only runs when cora-results.sarif is non-empty and valid
JSON.
| - name: Post PR comment | ||
| if: always() && github.event_name == 'pull_request' | ||
| uses: actions/github-script@v7 | ||
| env: | ||
| GH_TOKEN: ${{ inputs.github-token }} | ||
| with: |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does actions/github-script v7 use the github-token input (default github.token) to authenticate the github Octokit client, and does it read a GH_TOKEN env var?
💡 Result:
Yes. actions/github-script v7 uses its github-token input (defaulting to the workflow’s github.token / default GITHUB_TOKEN) to authenticate the pre-authenticated Octokit client it exposes to your script.[1][2][3][4] No evidence that v7 reads a GH_TOKEN environment variable. The documented/declared default is github-token = ${{ github.token }} (not GH_TOKEN), and the README describes using the workflow’s GITHUB_TOKEN by default, with an explicit github-token input for a different token.[2][3][4] What this means: - If you do not set with: github-token, github-script v7’s Octokit client will use the default workflow token (github.token / GITHUB_TOKEN scoped to the current repo).[2][3] - Setting with: github-token overrides that token (e.g., a PAT from secrets).[1][2] - GH_TOKEN is not mentioned as an input source for github-script’s authentication in the v7 docs/action.yml; only github-token / github.token is.[4][2]
Citations:
- 1: https://github.com/marketplace/actions/github-script?version=v7.0.0
- 2: https://github.com/actions/github-script
- 3: https://docs.github.com/actions/reference/authentication-in-a-workflow
- 4: https://github.com/actions/github-script/blob/main/action.yml
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/actions/cora-review/action.yml"
echo "== File = $FILE =="
nl -ba "$FILE" | sed -n '1,220p'Repository: ajianaz/cora-cli
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/actions/cora-review/action.yml"
echo "== Show relevant sections from $FILE =="
echo "-- Lines 40-90 --"
cat -n "$FILE" | sed -n '40,90p'
echo "-- Lines 105-155 --"
cat -n "$FILE" | sed -n '105,155p'
echo "-- Inputs section (if any) --"
cat -n "$FILE" | sed -n '1,60p'Repository: ajianaz/cora-cli
Length of output: 7003
Wire inputs.github-token into actions/github-script@v7 (remove unused env.GH_TOKEN)
actions/github-script@v7 authenticates its Octokit client from with.github-token (defaulting to github.token); it does not read env.GH_TOKEN. As written, inputs.github-token has no effect on the PR-comment API calls, and the GH_TOKEN env var is misleading. Also, the existing dedup check hardcodes c.user.login === 'github-actions[bot]', so once token wiring is corrected, non-default tokens would likely cause the update logic to miss prior comments.
🔧 Proposed fix
- name: Post PR comment
if: always() && github.event_name == 'pull_request'
uses: actions/github-script@v7
- env:
- GH_TOKEN: ${{ inputs.github-token }}
with:
+ github-token: ${{ inputs.github-token }}
script: |
const fs = require('fs');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Post PR comment | |
| if: always() && github.event_name == 'pull_request' | |
| uses: actions/github-script@v7 | |
| env: | |
| GH_TOKEN: ${{ inputs.github-token }} | |
| with: | |
| - name: Post PR comment | |
| if: always() && github.event_name == 'pull_request' | |
| uses: actions/github-script@v7 | |
| with: | |
| github-token: ${{ inputs.github-token }} |
🤖 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/actions/cora-review/action.yml around lines 62 - 67, In the "Post PR
comment" step using actions/github-script@v7, remove the misleading env GH_TOKEN
and wire the provided inputs.github-token into the action by setting
with.github-token: ${{ inputs.github-token }} so the Octokit client actually
uses the input token; then update the dedup logic that currently checks
c.user.login === 'github-actions[bot]' to a token-agnostic check (for example
compare c.user.login to github.actor or check c.user.type === 'Bot' and a unique
comment marker in c.body) so prior comments posted with non-default tokens are
detected and updated.
| permissions: | ||
| contents: read | ||
| security-events: write | ||
| pull-requests: write | ||
| id-token: write |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
ls -la "$FILE" || true
echo
echo "== Tail/head around permissions sections =="
# Show top-level permissions and nearby lines
sed -n '1,140p' "$FILE" | nl -ba | sed -n '1,140p'
echo
echo "== Permissions blocks with line numbers =="
rg -n "^[[:space:]]*permissions:" -n "$FILE" || true
echo
echo "== Extract full permissions block(s) =="
# crude extraction: from each "permissions:" line, print until next non-indented top-level key
# Use awk to print blocks starting at permissions
awk '
BEGIN{inblock=0}
/^[[:space:]]*permissions:/ {
inblock=1; print "----"; print NR ":" $0; next
}
inblock==1 {
# stop when encountering a line that looks like a new top-level key (no leading spaces) or a job id
if ($0 ~ /^[^[:space:]]/ && $0 !~ /^permissions:/) { inblock=0; next }
if ($0 ~ /^[[:space:]]*$/) { print NR ":" $0; next }
# keep indented permission lines
if ($0 ~ /^[[:space:]]+[a-zA-Z0-9_-]+:[[:space:]]*(read|write|)?/ || $0 ~ /^[[:space:]]+[a-zA-Z0-9_-]+:[[:space:]]*(true|false)?/) { print NR ":" $0; next }
# also keep comments/blank lines inside block
if ($0 ~ /^[[:space:]]*`#/`) { print NR ":" $0; next }
# otherwise keep for a bit; heuristic
print NR ":" $0
}
' "$FILE"
echoRepository: ajianaz/cora-cli
Length of output: 278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
echo "== File: $FILE =="
wc -l "$FILE"
echo
echo "== Top-level permissions (line-numbered) =="
awk '
BEGIN{p=0}
/^[[:space:]]*permissions:[[:space:]]*$/ {p=1; print "----"; print FNR ":" $0; next}
p==1 {
if ($0 ~ /^[^[:space:]]/ && $0 !~ /^permissions:/) {p=0}
else { print FNR ":" $0 }
}
' "$FILE"
echo
echo "== All occurrences of 'permissions:' (line-numbered) =="
awk '/^[[:space:]]*permissions:/ {print FNR ":" $0}' "$FILE"
echo
echo "== Excerpt around lines 1-120 (to locate jobs/permissions context) =="
awk 'NR>=1 && NR<=120 {printf "%5d: %s\n", NR, $0}' "$FILE" | sed -n '1,120p'Repository: ajianaz/cora-cli
Length of output: 3582
Remove write-scoped workflow permissions so other jobs don’t inherit elevated access.
Top-level permissions in .github/workflows/ci.yml still grant security-events: write, pull-requests: write, and id-token: write to all jobs (check, fmt, clippy, test, build), so the least-privilege improvement is incomplete even though cora-review has its own job-level permissions.
🔐 Proposed fix
permissions:
contents: read
- security-events: write
- pull-requests: write
- id-token: write
...
cora-review:
name: Cora Review
runs-on: ubuntu-latest
needs: [check, fmt, clippy, test]
if: github.event_name == 'pull_request'
permissions:
contents: read
security-events: write
pull-requests: write
id-token: write🤖 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/ci.yml around lines 73 - 77, Top-level workflow
permissions currently grant write scopes (security-events: write, pull-requests:
write, id-token: write) which are inherited by all jobs; remove those
write-scoped entries and restrict the top-level permissions to only minimal read
(e.g., contents: read) so jobs like check, fmt, clippy, test, build do not get
elevated access, then add any required write permissions at the job level (for
example ensure the cora-review job keeps its specific write permissions) so only
jobs that need write scopes receive them.
- Remove || true (was masking failures silently) - Remove 2>/dev/null from cargo build (show build errors) - Keep 2>/dev/null on cora output only (prevent stderr corrupting SARIF) - Add echo for review completion status
| - name: Fetch LLM secrets from Infisical | ||
| uses: Infisical/secrets-action@v1.0.9 | ||
| with: | ||
| method: 'oidc' |
| uses: Infisical/secrets-action@v1.0.9 | ||
| with: | ||
| method: 'oidc' | ||
| identity-id: ${{ inputs.infisical-identity-id }} |
2 participants
Summary
Extract the cora-review CI job into a reusable composite action.
Before (ci.yml)
After (ci.yml)
Changes
.github/actions/cora-review/action.yml— composite action with all review logicbase-branch,severity, infisical settingsGITHUB_STEP_SUMMARYbranding pollution'error'check in blocking python scriptCloses
Closes #45 (supersedes the secret fix — identity-id already in secret)
Summary by CodeRabbit