fix(ci): Make breaking changes checks compare against the merge-base#2924
fix(ci): Make breaking changes checks compare against the merge-base#2924kensimon wants to merge 2 commits into
Conversation
It currently compares against main for breaking changes, but that means if new fields were added to main since the branch was created, it incorrectly shows them as being removed in the PR branch. This updates it to check against $(git merge-base HEAD origin/main) so that it sees the actual diffs introduced in this branch since it started.
WalkthroughThe CI workflows update protobuf and OpenAPI breaking-change checks to compare against a computed merge-base with ChangesWorkflow comparison base updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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
🤖 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/ci.yaml:
- Around line 994-997: The checkout step currently leaves the GitHub token
persisted in the workspace, which unnecessarily exposes credentials to later
steps. Update the existing actions/checkout usage in the CI workflow to disable
persisted credentials for this job, and keep authentication limited to the
explicit git fetch that follows if the repository is private. Locate the
Checkout code step in the workflow and apply the fix there, preserving the
current fetch-depth behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 89b0a22c-3240-4ba4-a31d-b2ae345a55b6
📒 Files selected for processing (1)
.github/workflows/ci.yaml
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 # Required: Buf needs the history to compare against main | ||
| fetch-depth: 0 # Required to compute the merge-base against main |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable persistent checkout credentials for this job.
actions/checkout writes the repo token into .git/config by default. This job only needs credentials for the explicit git fetch, so leaving the token available to every later step unnecessarily widens the exfiltration surface. Set persist-credentials: false here, and if the repo is private, scope authentication to the single fetch command instead of the whole workspace.
Suggested change
- name: Checkout code
uses: actions/checkout@v4
with:
+ persist-credentials: false
fetch-depth: 0 # Required to compute the merge-base against mainAs per path instructions, GitHub Actions workflow reviews should cover secret handling; zizmor also flags this checkout step for persisted credentials.
📝 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: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| fetch-depth: 0 # Required: Buf needs the history to compare against main | |
| fetch-depth: 0 # Required to compute the merge-base against main | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false | |
| fetch-depth: 0 # Required to compute the merge-base against main |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 994-997: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 995-995: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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.yaml around lines 994 - 997, The checkout step
currently leaves the GitHub token persisted in the workspace, which
unnecessarily exposes credentials to later steps. Update the existing
actions/checkout usage in the CI workflow to disable persisted credentials for
this job, and keep authentication limited to the explicit git fetch that follows
if the repository is private. Locate the Checkout code step in the workflow and
apply the fix there, preserving the current fetch-depth behavior.
Sources: Path instructions, Linters/SAST tools
There was a problem hiding this comment.
Filed #2925 for this, out of scope for this PR.
kensimon
changed the title
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-06-26 17:42:23 UTC | Commit: 78b27e9 |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
2 participants
For both the proto-breaking-changes and openapi-breaking-changes checks, they currently compare against main for breaking changes, but that means if new fields/etc were added to main since the branch was created, they incorrectly show them as being removed in the PR branch. This updates it to check against $(git merge-base HEAD origin/main) so that it only sees the actual diffs introduced in this branch since it started.
Related issues
#2923
Type of Change
Breaking Changes
Testing
Additional Notes