OCPBUGS-100279: isolate Claude from push credentials in PR workflows - #9214
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@bryan-cox: This pull request references Jira Issue OCPBUGS-100279, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe workflows now instruct Claude not to push changes. The reusable workflow disables persisted checkout credentials, removes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Claude
participant PRRepository
participant PRComment
GitHubActions->>Claude: Run in bare mode
Claude-->>GitHubActions: Return changes without pushing
GitHubActions->>PRRepository: Authenticate and force-push HEAD
GitHubActions->>PRComment: Report push failure when push fails
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
approved
![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/workflows/reusable-claude-on-pr.yaml:
- Around line 137-152: Separate the Claude execution from the credentialed push:
run Claude in an uncredentialed workspace, export only its changes as a patch
artifact, and remove shared workspace access to PUSH_TOKEN. Replace the current
Push changes step with a dependent fresh job that checks out the target branch,
applies the patch, disables Git hooks, and performs the existing push and
failure-comment flow using PUSH_TOKEN.
In
`@control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.go`:
- Around line 519-521: Update the test setup around runtime.NewScheme in the
globalps test to check the errors returned by corev1.AddToScheme and
appsv1.AddToScheme, failing the test immediately with the registration error
instead of discarding it. Keep the existing scheme registration order and
fake-client setup unchanged.
In
`@control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go`:
- Around line 182-185: The DaemonSet no-op check in globalps.go must not rely
only on config seed and volume count; compare all controller-managed fields or
allow CreateOrUpdate to detect differences, including rolling-update strategy
and readiness probe. In
control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go
lines 182-185, remove or expand the early return accordingly. In
control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.go
lines 542-555, add a legacy DaemonSet fixture with matching seed and volumes but
missing the new fields, and assert reconciliation invokes CreateOrUpdate.
- Around line 181-188: The Get call in the existing daemon set lookup must not
suppress unexpected errors. Update the error handling around c.Get in the global
pull-secret reconciliation flow to continue on apierrors.IsNotFound(err), but
return a wrapped error for all other errors; preserve the existing early return
when the fetched object matches the expected labels and volume count.
🪄 Autofix
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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 30825d57-641d-415c-86d3-14e3009b7c68
📒 Files selected for processing (7)
.github/workflows/rebase.yaml.github/workflows/restructure-commits.yaml.github/workflows/reusable-claude-on-pr.yamlcontrol-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/globalps/setup.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/globalps/setup_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Please upload reports for the commit 6be67a5 to get more accurate results. Additional details and impacted files@@ Coverage Diff @@
## main #9214 +/- ##
==========================================
+ Coverage 44.95% 44.96% +0.01%
==========================================
Files 778 778
Lines 97434 97467 +33
==========================================
+ Hits 43797 43830 +33
Misses 50615 50615
Partials 3022 3022
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Add --bare to the Claude invocation to prevent loading CLAUDE.md, AGENTS.md, and pre-commit hooks from fork checkouts, eliminating context file and hook injection vectors. Separate Claude execution from push credentials by setting persist-credentials: false, removing GH_TOKEN from the Claude step, and adding a dedicated post-Claude push step. Update caller prompts to remove push instructions since the workflow now handles pushing. OCPBUGS-100279 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0f82a27 to
6be67a5
Compare
|
@bryan-cox: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@bryan-cox: Jira Issue OCPBUGS-100279: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-100279 has been moved to the MODIFIED state. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, celebdor The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
What this PR does / why we need it:
Hardens the
reusable-claude-on-pr.yamlGitHub Actions workflow against context file injection and credential exposure when operating on fork PRs.The workflow previously checked out fork PR content with
persist-credentials: trueand passedGH_TOKEN(a write-scoped token) to the Claude step's environment. An external PR author could craft maliciousCLAUDE.md/AGENTS.mdfiles or pre-commit hooks that Claude would load and execute with Bash access, gaining access to the repo write token and GCP WIF credentials.Changes:
--bareto the Claude invocation — prevents loadingCLAUDE.md/AGENTS.mdfrom the fork checkout and skips hooks, eliminating context file and pre-commit hook injection vectors. Skills still resolve via/skill-nameso/restructure-commitscontinues to work.persist-credentials: false— prevents the checkout action from storing the write token in.git/config.GH_TOKENfrom the Claude step's env — Claude no longer has access to a write-scoped token during execution.rebase.yaml,restructure-commits.yaml) — remove push instructions from Claude prompts since the workflow now handles pushing.Which issue(s) this PR fixes:
Fixes https://redhat.atlassian.net/browse/OCPBUGS-100279
Special notes for your reviewer:
The severity was reassessed to CVSS 4.2 / Medium (down from 8.0 / High) — see the detailed assessment. This is CI tooling, not product code.
Checklist:
Always review AI generated responses prior to use.
Generated with Claude Code via openshift-developer plugin
Summary by CodeRabbit