Skip to content

NO-JIRA: fix(ci): prevent Claude from breaking push credentials in fork PRs#8831

Open
bryan-cox wants to merge 1 commit into
openshift:mainfrom
bryan-cox:fix-ci-push-credentials
Open

NO-JIRA: fix(ci): prevent Claude from breaking push credentials in fork PRs#8831
bryan-cox wants to merge 1 commit into
openshift:mainfrom
bryan-cox:fix-ci-push-credentials

Conversation

@bryan-cox

@bryan-cox bryan-cox commented Jun 25, 2026

Copy link
Copy Markdown
Member

Summary

  • Add credential verification step after checkout to fail fast if the GitHub App token is broken
  • Expose PUSH_TOKEN, PR_REPO, and PR_BRANCH env vars to the Claude step so it has the correct token for fork pushes (distinct from GH_TOKEN)
  • Add .claude/rules/ci-git-push.md instructing Claude to never modify git remote URLs in CI

Context

When the address-review-comments workflow runs on a PR from hypershift-community/hypershift, actions/checkout configures the credential helper with a GitHub App token via persist-credentials: true. Claude was overwriting the remote URL with $GH_TOKEN (github.token) when push attempts timed out, destroying the working credential config. All subsequent pushes failed with "Invalid username or token".

See PR #8211 comment for the failed run.

Test plan

  • Trigger /address-review-comments on a PR from hypershift-community/hypershift and verify push succeeds
  • Verify the Verify push credentials step passes in the workflow logs
  • Verify same-repo PRs (from openshift/hypershift forks) still work

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Improved pull request workflow support for safer and more reliable Git pushes during automated runs.
    • Added workflow checks to validate push access before continuing.
  • Bug Fixes

    • Reduced push failures by handling authentication issues and timeouts more robustly.
    • Improved consistency when pushing changes from CI by using the correct branch context and credentials.

The address-review-comments workflow generates a GitHub App token for
pushing to hypershift-community/hypershift fork PRs. actions/checkout
configures the credential helper via persist-credentials. However,
Claude was overwriting the remote URL with $GH_TOKEN on push timeouts,
destroying the working credential config and causing all subsequent
pushes to fail with auth errors.

Add a verification step after checkout to fail fast if credentials are
broken, expose PUSH_TOKEN/PR_REPO/PR_BRANCH env vars so Claude has the
correct token available, and add a Claude rule file that instructs it
to never modify the git remote URL in CI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The PR adds a new CI rule file for git push behavior in GitHub Actions, covering remote URL handling, branch selection, timeout retries, and credential-helper reconfiguration on authentication failures. The reusable PR workflow now verifies access to the origin remote after checkout and passes PR_REPO, PR_BRANCH, and PUSH_TOKEN into the Run Claude step.

Sequence Diagram(s)

sequenceDiagram
  participant Workflow as reusable-claude-on-pr workflow
  participant Origin as origin remote
  participant RunClaude as Run Claude step

  Workflow->>Origin: git ls-remote --exit-code origin HEAD
  Origin-->>Workflow: exit code
  Workflow->>RunClaude: set PR_REPO, PR_BRANCH, PUSH_TOKEN
Loading

Possibly related PRs

Suggested reviewers

  • cblecker
  • csrwng
  • jparrill
🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed Only workflow/docs files changed; the touched files contain no Ginkgo titles or dynamic test-name patterns.
Test Structure And Quality ✅ Passed Not applicable: this PR only changes a CI workflow and a Claude rule file; no Ginkgo tests or test helpers were added or modified.
Topology-Aware Scheduling Compatibility ✅ Passed Only CI workflow/docs changed; no deployment manifests, operators, or controllers were modified, so topology-aware scheduling rules don’t apply.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No Ginkgo e2e tests were added; the PR only changes workflow/docs and contains no IPv4-assuming or external-network test code.
No-Weak-Crypto ✅ Passed The PR only adds docs/workflow logic; no MD5/SHA1/DES/RC4/3DES/Blowfish, ECB, custom crypto, or secret comparisons were introduced.
Container-Privileges ✅ Passed No privileged container/K8s settings are present in the changed workflow or rule file; the PR only adds env vars and a credential check.
No-Sensitive-Data-In-Logs ✅ Passed No new logs print secrets or PII; the workflow only echoes the origin URL, branch/repo metadata, and generic status text.
Title check ✅ Passed The title accurately summarizes the main CI fix for preserving push credentials in fork PRs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot requested review from Nirshal and cblecker June 25, 2026 12:50
@openshift-ci openshift-ci Bot added the area/ai Indicates the PR includes changes related to AI - Claude agents, Cursor rules, etc. label Jun 25, 2026
@openshift-ci

openshift-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/needs-area labels Jun 25, 2026
@bryan-cox bryan-cox changed the title fix(ci): prevent Claude from breaking push credentials in fork PRs NO-JIRA: fix(ci): prevent Claude from breaking push credentials in fork PRs Jun 25, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 25, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@bryan-cox: This pull request explicitly references no jira issue.

Details

In response to this:

Summary

  • Add credential verification step after checkout to fail fast if the GitHub App token is broken
  • Expose PUSH_TOKEN, PR_REPO, and PR_BRANCH env vars to the Claude step so it has the correct token for fork pushes (distinct from GH_TOKEN)
  • Add .claude/rules/ci-git-push.md instructing Claude to never modify git remote URLs in CI

Context

When the address-review-comments workflow runs on a PR from hypershift-community/hypershift, actions/checkout configures the credential helper with a GitHub App token via persist-credentials: true. Claude was overwriting the remote URL with $GH_TOKEN (github.token) when push attempts timed out, destroying the working credential config. All subsequent pushes failed with "Invalid username or token".

See PR #8211 comment for the failed run.

Test plan

  • Trigger /address-review-comments on a PR from hypershift-community/hypershift and verify push succeeds
  • Verify the Verify push credentials step passes in the workflow logs
  • Verify same-repo PRs (from openshift/hypershift forks) still work

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

  • Improved pull request workflow support for safer and more reliable Git pushes during automated runs.

  • Added workflow checks to validate push access before continuing.

  • Bug Fixes

  • Reduced push failures by handling authentication issues and timeouts more robustly.

  • Improved consistency when pushing changes from CI by using the correct branch context and credentials.

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.

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.03%. Comparing base (c4fd7dc) to head (d5e6af5).
⚠️ Report is 59 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8831      +/-   ##
==========================================
- Coverage   42.11%   42.03%   -0.09%     
==========================================
  Files         767      769       +2     
  Lines       95069    96852    +1783     
==========================================
+ Hits        40039    40709     +670     
- Misses      52216    53329    +1113     
  Partials     2814     2814              

see 25 files with indirect coverage changes

Flag Coverage Δ
cmd-support 35.63% <ø> (+0.17%) ⬆️
cpo-hostedcontrolplane 44.88% <ø> (+0.35%) ⬆️
cpo-other 44.94% <ø> (+0.68%) ⬆️
hypershift-operator 50.19% <ø> (-1.73%) ⬇️
other 31.69% <ø> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/reusable-claude-on-pr.yaml:
- Around line 83-88: The “Verify push credentials” step only validates read
access via git ls-remote in the workflow, so it can succeed even when later git
push fails; update this check in the reusable-claude-on-pr workflow to verify
actual push authorization using the same origin remote, and keep stderr visible
so failures provide diagnostics. Use the existing “Verify push credentials” step
and its git remote/origin setup to replace the read-only probe with a
push-oriented validation.
🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: a0892273-85b0-4bb5-a258-d3562e6db500

📥 Commits

Reviewing files that changed from the base of the PR and between 652dbf2 and d5e6af5.

📒 Files selected for processing (2)
  • .claude/rules/ci-git-push.md
  • .github/workflows/reusable-claude-on-pr.yaml

Comment on lines +83 to +88
- name: Verify push credentials
run: |
echo "Remote URL: $(git remote get-url origin)"
echo "Credential helper configured by checkout — testing ls-remote..."
git ls-remote --exit-code origin HEAD > /dev/null 2>&1
echo "Push credentials verified."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Verify push authorization, not just fetch authorization

Line 87 uses git ls-remote, which only checks read access, so this can pass while later git push still fails (the exact failure mode this PR targets). Also, redirecting stderr to /dev/null removes useful failure diagnostics.

Suggested fix
       - name: Verify push credentials
         run: |
           echo "Remote URL: $(git remote get-url origin)"
-          echo "Credential helper configured by checkout — testing ls-remote..."
-          git ls-remote --exit-code origin HEAD > /dev/null 2>&1
+          echo "Credential helper configured by checkout — testing push authorization (dry-run)..."
+          git push --dry-run origin "HEAD:${{ steps.pr.outputs.branch }}"
           echo "Push credentials verified."
📝 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.

Suggested change
- name: Verify push credentials
run: |
echo "Remote URL: $(git remote get-url origin)"
echo "Credential helper configured by checkout — testing ls-remote..."
git ls-remote --exit-code origin HEAD > /dev/null 2>&1
echo "Push credentials verified."
- name: Verify push credentials
run: |
echo "Remote URL: $(git remote get-url origin)"
echo "Credential helper configured by checkout — testing push authorization (dry-run)..."
git push --dry-run origin "HEAD:${{ steps.pr.outputs.branch }}"
echo "Push credentials verified."
🤖 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/reusable-claude-on-pr.yaml around lines 83 - 88, The
“Verify push credentials” step only validates read access via git ls-remote in
the workflow, so it can succeed even when later git push fails; update this
check in the reusable-claude-on-pr workflow to verify actual push authorization
using the same origin remote, and keep stderr visible so failures provide
diagnostics. Use the existing “Verify push credentials” step and its git
remote/origin setup to replace the read-only probe with a push-oriented
validation.

@openshift-ci

openshift-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

@bryan-cox: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@hypershift-jira-solve-ci

Copy link
Copy Markdown

Test Failure Analysis Complete

Job Information

  • Prow Job: codecov/project (GitHub check, not a Prow CI job)
  • Build ID: Check Run 83442208250
  • PR: #8831NO-JIRA: fix(ci): prevent Claude from breaking push credentials in fork PRs
  • Base Commit: c4fd7dc (coverage: 42.11%)
  • Head Commit: d5e6af5 (coverage: 42.03%)

Test Failure Analysis

Error

codecov/project: 42.03% (-0.09%) compared to c4fd7dc

Summary

The codecov/project check failed because overall project coverage dropped by 0.09% (42.11% → 42.03%). This is not caused by PR #8831 itself — the PR only modifies .claude/rules/ci-git-push.md and .github/workflows/reusable-claude-on-pr.yaml, neither of which contains Go code. Codecov confirms "All modified and coverable lines are covered by tests." The coverage drop comes from 59 other commits merged to main since the PR's base commit, which added 1,783 new lines of Go code with only 670 covered (+1,113 uncovered). Codecov's default auto project target requires coverage not to decrease relative to the base, so the accumulated uncovered code from other merges triggered the failure on this PR.

Root Cause

The failure is a false positive — the PR does not cause any coverage regression.

Codecov's default project-level check uses an "auto" target, which means the PR's head coverage must be ≥ the base commit's coverage. The codecov.yml in the repo does not define a coverage.status.project section, so Codecov falls back to this default "do not decrease" policy.

The PR branch includes 59 commits from main that were merged after the base commit c4fd7dc. These commits added +1,783 lines of Go code but only +670 hits (covered lines) vs +1,113 misses (uncovered lines). This net addition of undertested code across 25 indirectly-changed files is what drove overall coverage from 42.11% down to 42.03%.

Since the PR itself adds zero coverable lines (only a Markdown rule file and a YAML workflow file), it has no ability to influence the coverage percentage in either direction. The check is penalizing this PR for code coverage debt introduced by unrelated merges to main.

Recommendations
  1. This check can be safely ignored for merge purposes — the PR introduces no Go code and Codecov itself confirms all modified coverable lines are covered.

  2. Rebase the PR onto latest main — this will update the base commit to a more recent one, which already includes the coverage-decreasing commits. After rebase, the coverage delta should be ~0.00% and the check should pass.

  3. Long-term: Configure an explicit coverage threshold — add a coverage.status.project section to codecov.yml to prevent false positives on non-code PRs:

    coverage:
      status:
        project:
          default:
            target: auto
            threshold: 0.5%  # Allow up to 0.5% drop to reduce noise
  4. Address the indirect coverage debt — the 25 files with indirect coverage changes (flagged in the Codecov report) include new code on main that lacks test coverage, particularly in the hypershift-operator flag which dropped 1.73%.

Evidence
Evidence Detail
PR changed files .claude/rules/ci-git-push.md, .github/workflows/reusable-claude-on-pr.yaml (no Go code)
Codecov patch check ✅ Passed — "All modified and coverable lines are covered by tests"
Codecov project check ❌ Failed — 42.03% (-0.09%) vs base 42.11%
Coverage diff +1,783 lines, +670 hits, +1,113 misses, ±0 partials
Stale base report "Report is 59 commits behind head on main"
codecov.yml threshold config None — no coverage.status.project section; defaults to auto (no decrease allowed)
hypershift-operator flag Dropped 1.73% (50.19%) — largest indirect regression
Base commit for comparison c4fd7dc (42.11% coverage, 95,069 lines)
Head commit d5e6af5 (42.03% coverage, 96,852 lines)

@cblecker

Copy link
Copy Markdown
Member

This sounds like something that could be fixed in the environment with positive language as opposed to rules that tell it not to do those things that may be brittle/ignored.

Do you have an example of this happening?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/ai Indicates the PR includes changes related to AI - Claude agents, Cursor rules, etc. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants