security: fail closed on privileged workflow refs - #1912
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Greptile SummaryThe PR removes manually dispatched privileged workflows and adds fail-closed validation for protected publication refs and generated release commits.
Confidence Score: 4/5The PR should not merge until the post-merge main release run can reuse the tag created by the generated-branch build; the documentation-policy issue is non-blocking. The reusable run creates the release tag at the generated commit, while the subsequent main-push run compares that tag against a different merge SHA and fails before any dependent platform artifact job can run. Files Needing Attention: .github/workflows/release-updates.yml, docs/release-workflow.md Important Files Changed
Sequence DiagramsequenceDiagram
participant Schedule as Scheduled release workflow
participant Branch as release/vX.Y.Z
participant Release as GitHub Release
participant Main as Protected main
Schedule->>Branch: Create version commit C_release
Schedule->>Release: Create vX.Y.Z targeting C_release
Branch->>Main: Merge release PR as C_main
Main->>Release: Resolve existing vX.Y.Z
Release-->>Main: Tag points to C_release
Main->>Main: Compare tag with SOURCE_SHA C_main
Main-->>Main: Fail before platform artifact jobs
Reviews (1): Last reviewed commit: "security: gate privileged workflows on t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| tag_refs="$(GIT_TERMINAL_PROMPT=0 git ls-remote origin \ | ||
| "refs/tags/$resolved_tag" "refs/tags/$resolved_tag^{}")" | ||
| if ! printf '%s\n' "$tag_refs" | awk -v expected="$SOURCE_SHA" ' | ||
| NF == 2 && $1 == expected { found = 1 } | ||
| END { exit(found ? 0 : 1) } | ||
| '; then | ||
| echo "Release tag $resolved_tag does not point at $SOURCE_SHA" >&2 | ||
| exit 1 |
There was a problem hiding this comment.
When the generated release PR is merged, the main-push run sets SOURCE_SHA to the new main commit, but the existing tag still targets the generated release commit. This comparison rejects the tag, causing prepare-release to fail and all dependent platform artifact jobs to be skipped.
Knowledge Base Used:
| # Release workflow | ||
|
|
||
| The release workflow runs from the protected `main` branch on its schedule. | ||
| It creates a single-file `release/vX.Y.Z` branch, then calls | ||
| `.github/workflows/release-updates.yml` as a reusable workflow. The call carries | ||
| the exact release commit, its `main` parent, and the protected caller commit. | ||
|
|
||
| The publishing workflow has no `workflow_dispatch` trigger. This removes the | ||
| manual path that could otherwise select arbitrary branch code while exposing | ||
| signing credentials. Outside the schedule, wait for the next scheduled run or | ||
| rerun a recent scheduled run after confirming that its protected `main` commit | ||
| is still current. | ||
|
|
||
| The trust gate rejects stale or moved refs, workflow changes, non-bot release | ||
| commits, and release commits that change files other than | ||
| `apps/client/package.json`. Keep `main` protected and configure non-empty | ||
| required reviewers for the `electron` environment before relying on publication. |
There was a problem hiding this comment.
This new release document conflicts with the repository instruction not to write documentation unless explicitly requested, adding an unsupported maintenance surface.
| # Release workflow | |
| The release workflow runs from the protected `main` branch on its schedule. | |
| It creates a single-file `release/vX.Y.Z` branch, then calls | |
| `.github/workflows/release-updates.yml` as a reusable workflow. The call carries | |
| the exact release commit, its `main` parent, and the protected caller commit. | |
| The publishing workflow has no `workflow_dispatch` trigger. This removes the | |
| manual path that could otherwise select arbitrary branch code while exposing | |
| signing credentials. Outside the schedule, wait for the next scheduled run or | |
| rerun a recent scheduled run after confirming that its protected `main` commit | |
| is still current. | |
| The trust gate rejects stale or moved refs, workflow changes, non-bot release | |
| commits, and release commits that change files other than | |
| `apps/client/package.json`. Keep `main` protected and configure non-empty | |
| required reviewers for the `electron` environment before relying on publication. |
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
a65fb08 to
c6a2784
Compare
c6a2784 to
ff5df9a
Compare
ff5df9a to
0751fba
Compare
0751fba to
608db7d
Compare
608db7d to
bdebb4a
Compare
bdebb4a to
940477b
Compare
6ae840f to
203eca9
Compare
1 participant
Summary
Testing
scripts/ci/test-verify-privileged-ref.shruby scripts/ci/test-privileged-workflow-gates.rbactionlint .github/workflows/*.ymlbash -n .github/scripts/verify-privileged-ref.sh scripts/ci/test-verify-privileged-ref.shshellcheck .github/scripts/verify-privileged-ref.sh scripts/ci/test-verify-privileged-ref.shgit diff --checkzizmor --pedantic: 0 low, medium, or high findings; five informational Rust setup notices remain.Stack and rollout
Stacked on PR 1910. Configure protected
mainand non-empty required reviewers for theelectronenvironment before merging this stack. No merge was performed by this PR.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Removes
workflow_dispatchfrom secret-bearing workflows and replaces branch-based trust with fail-closed checks for protected refs, workflow revisions, and release metadata. Privileged jobs now run only from protectedmainpushes, scheduledmainruns, or the trusted reusable release call; manual arbitrary-ref runs and post-merge release publishing no longer work.release-pr.ymlwith exact release, base, and caller SHAs; new tags bind to the verified source, moved tags are rejected, and retries are limited to untouched bot-created draft releases.mainafter a squash merge.Migration
main,release/v*branches, andv*tags; configure non-empty required reviewers for theelectronenvironment.maincommit remains reachable.Written for commit 94387db. Summary will update on new commits.