ci: finish PR verification hardening#472
Conversation
# Conflicts: # docs/branch-review-ledger.md
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes pin Supabase CLI caching in CI, make Semgrep advisory while expanding its scan scope, and add automated checks for these workflow contracts. A branch review ledger entry records the associated completed review. ChangesWorkflow hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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: 2
🤖 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/sast.yml:
- Around line 25-27: Move continue-on-error: true from the Semgrep job
definition to the specific Semgrep scan step, keeping checkout, setup, and
container initialization failures blocking while allowing only scan findings to
be advisory.
In `@scripts/check-github-action-pins.mjs`:
- Around line 68-95: Refactor the checks in the script’s CI and SAST validation
blocks to inspect parsed YAML structure or explicitly scoped job/step sections
instead of raw includes and broad regexes. Verify Supabase version, cache
identity/key, and cache-hit condition on the intended CI steps, and require
`continue-on-error: true` under `jobs.semgrep`; also validate that `src`,
`worker`, `scripts`, and `supabase/functions` are actual Semgrep target paths
rather than arbitrary text or comments.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 233a51e3-988f-4f46-8d7a-ea146f542207
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/sast.ymldocs/branch-review-ledger.mdscripts/check-github-action-pins.mjs
| semgrep: | ||
| name: Semgrep | ||
| continue-on-error: true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant workflow file with line numbers.
if [ -f .github/workflows/sast.yml ]; then
wc -l .github/workflows/sast.yml
sed -n '1,220p' .github/workflows/sast.yml | cat -n
else
echo "File not found: .github/workflows/sast.yml"
exit 1
fiRepository: BigSimmo/Database
Length of output: 2298
🌐 Web query:
GitHub Actions job-level continue-on-error failed step checkout container initialization workflow passes without running later steps
💡 Result:
In GitHub Actions, the continue-on-error: true attribute is designed to prevent a job from failing when a specific step fails, allowing subsequent steps in that job to execute [1][2]. If your workflow appears to stop after a step with continue-on-error: true fails, it is likely due to one of the following reasons: 1. Step Outcome vs. Conclusion: When continue-on-error is set to true, the step's outcome will reflect the actual failure (e.g., 'failure'), but the step's conclusion will be 'success' because the error was suppressed [3][4]. If you have subsequent steps that use conditional logic (e.g., if: failure()), they will not run because GitHub Actions evaluates them based on the step's conclusion [3][4]. To detect the failure in later steps, you should check the outcome property instead: if: steps.<step_id>.outcome == 'failure' [2][4]. 2. Container Initialization Issues: If the failure occurs during "container initialization" (e.g., pulling an image, mounting volumes, or setting up the container environment), it may represent a platform-level or infrastructure error that occurs outside the scope of individual step execution. In some cases, if the runner itself cannot initialize the environment for subsequent steps, the job may terminate despite the continue-on-error flag [5]. 3. Nested Action Failure: continue-on-error generally works at the immediate step level [5]. If the failure occurs deep within a nested composite action or a sub-action that itself calls other actions, the continue-on-error flag on the outer step may not successfully mask failures occurring in certain internal processes (such as post-actions or deep-level node scripts) [5]. To troubleshoot, inspect the logs for any error messages occurring after the failed step, as these often indicate if the runner aborted the job due to infrastructure constraints rather than a simple step failure [5].
Citations:
- 1: https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions
- 2: https://stackoverflow.com/questions/73706004/mark-a-github-actions-workflow-as-success-even-if-a-step-fails
- 3: Wrong behaviour when combining 'continue-on-error' and 'failure()' in subsequent steps actions/toolkit#1034
- 4: Wrong behaviour when combining 'continue-on-error' and 'failure()' in subsequent steps actions/toolkit#1034
- 5:
continue-on-errorin an action doesn't suppress an error from a node action called by an action it calls actions/runner#3510
🌐 Web query:
GitHub Actions jobs.<job_id>.continue-on-error job-level semantics step failures checkout failure workflow conclusion
💡 Result:
In GitHub Actions, the continue-on-error property behaves differently depending on whether it is applied to a specific step or an entire job [1]. For steps, jobs.<job_id>.steps[*].continue-on-error allows a job to continue executing subsequent steps even if the specific step fails [2]. When a step with continue-on-error: true fails, the step's outcome is failure, but its final conclusion is recorded as success [3][4]. This allows the job to proceed, effectively masking the failure at the step level [2]. For jobs, jobs.<job_id>.continue-on-error prevents the entire workflow run from failing if the specific job fails [1]. When set to true, the workflow run is permitted to pass even if that job fails [1]. It is important to note that using continue-on-error does not make the job "succeed" in a technical sense; it primarily affects the workflow's overall completion status [5][4]. Because the job itself may still report a failure internally, conditional expressions like if: failure() or needs.<job_id>.result may behave unexpectedly if you rely on them to detect the "true" outcome of a job [6][5]. The job's internal failure status remains, which can impact dependent jobs if they are configured to run based on the outcome of the job that used continue-on-error [5][7]. If a job fails or is skipped, all downstream jobs that depend on it are typically skipped unless they use conditional expressions to continue [7].
Citations:
- 1: https://github.com/github/docs/blob/main/content/actions/reference/workflows-and-actions/workflow-syntax.md
- 2: https://docs.cloudbees.com/docs/cloudbees-unify/latest/dsl-syntax/jobs-job-id-steps-continue-on-error
- 3: https://docs.github.com/en/enterprise-server@3.2/actions/learn-github-actions/contexts
- 4: Please support something like "allow-failure" for a given job actions/runner#2347
- 5:
continue-on-errorshould not returnsuccessinneeds_job_resultactions/toolkit#1739 - 6: Wrong behaviour when combining 'continue-on-error' and 'failure()' in subsequent steps actions/toolkit#1034
- 7: https://docs.github.com/actions/using-jobs/using-jobs-in-a-workflow
Move continue-on-error to the Semgrep scan step. At the job level, checkout or container setup failures also become advisory, so the workflow can pass without ever running Semgrep.
🤖 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/sast.yml around lines 25 - 27, Move continue-on-error:
true from the Semgrep job definition to the specific Semgrep scan step, keeping
checkout, setup, and container initialization failures blocking while allowing
only scan findings to be advisory.
| const ciWorkflowPath = path.join(workflowDir, "ci.yml"); | ||
| const ciWorkflow = readFileSync(ciWorkflowPath, "utf8"); | ||
| const requiredCiFragments = [ | ||
| `SUPABASE_CLI_VERSION: ${expectedSupabaseCliVersion}`, | ||
| "version: ${{ env.SUPABASE_CLI_VERSION }}", | ||
| "id: supabase-docker-cache", | ||
| "supabase-docker-${{ runner.os }}-cli-${{ env.SUPABASE_CLI_VERSION }}-", | ||
| "if: success() && steps.supabase-docker-cache.outputs.cache-hit != 'true'", | ||
| ]; | ||
|
|
||
| for (const fragment of requiredCiFragments) { | ||
| if (!ciWorkflow.includes(fragment)) { | ||
| failures.push(`ci.yml: missing required pinned Supabase/cache contract: ${fragment}`); | ||
| } | ||
| } | ||
|
|
||
| if (/\bversion:\s*latest\b/.test(ciWorkflow)) { | ||
| failures.push("ci.yml: required workflow tooling must not use version: latest."); | ||
| } | ||
|
|
||
| const sastWorkflowPath = path.join(workflowDir, "sast.yml"); | ||
| const sastWorkflow = readFileSync(sastWorkflowPath, "utf8"); | ||
| if (!/^\s{4}continue-on-error:\s*true\s*$/m.test(sastWorkflow)) { | ||
| failures.push("sast.yml: Semgrep must remain advisory while it depends on mutable registry rules."); | ||
| } | ||
| if (!/src worker scripts supabase\/functions/.test(sastWorkflow)) { | ||
| failures.push("sast.yml: Semgrep must scan Supabase Edge Function source."); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make the workflow-contract checks structural and job-scoped.
The current includes and broad regex checks can pass a broken workflow when required fragments appear in comments, unrelated steps, or another job. In particular, the continue-on-error check does not require jobs.semgrep, and the scan-path check does not require the path to be an actual Semgrep target. Parse the YAML or scope each assertion to the named job/step and actual values.
🤖 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 `@scripts/check-github-action-pins.mjs` around lines 68 - 95, Refactor the
checks in the script’s CI and SAST validation blocks to inspect parsed YAML
structure or explicitly scoped job/step sections instead of raw includes and
broad regexes. Verify Supabase version, cache identity/key, and cache-hit
condition on the intended CI steps, and require `continue-on-error: true` under
`jobs.semgrep`; also validate that `src`, `worker`, `scripts`, and
`supabase/functions` are actual Semgrep target paths rather than arbitrary text
or comments.
1 participant
Summary
Why
Most of the original PR-check hardening task was already merged by PR #454. These were the three reviewed improvements still absent from current main; this PR deliberately excludes all superseded changes.
Areas touched
.github/workflows/ci.yml,.github/workflows/sast.yml,scripts/check-github-action-pins.mjs, and the branch review ledger.Verification
npm run check:github-actions— passed before and after merging current main.npm run check:ci-scope— passed before and after merging current main.git diff --check— passed.Checks not run
Migration replay and hosted Semgrep were not run locally because they are provider/container workflows. The PR workflows are the authoritative verification.
Risk and governance
No application, schema, or production-data behavior changes. The Supabase CLI is made deterministic, cache writes become conservative, and Semgrep remains visible without making mutable external rule registries a merge blocker.