NO-JIRA: fix Claude WIF test workflow for ARC runners#8598
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@bryan-cox: This pull request explicitly references no jira issue. 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. |
openshift-ci
Bot
added
area/api
bryan-cox
force-pushed
the
test-arc-runners
branch
from
2f52041 to
41f8c0f
Compare
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Codecov Report✅ All modified and coverable lines are covered by tests. Please upload reports for the commit acebf15 to get more accurate results. Additional details and impacted files@@ Coverage Diff @@
## main #8598 +/- ##
=======================================
Coverage 40.61% 40.61%
=======================================
Files 755 755
Lines 93227 93227
=======================================
Hits 37864 37864
Misses 52640 52640
Partials 2723 2723
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates the GitHub Actions test workflow: the PR metadata lookup now uses a direct Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
bryan-cox
force-pushed
the
test-arc-runners
branch
from
41f8c0f to
7993c4c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/claude-wif-test.yaml (1)
28-29: 💤 Low valueConsider using
Bearertoken format for consistency.The Authorization header uses
tokenformat, while the repo's established pattern (as seen in.github/workflows/docs-deploy.yaml:59-65) usesBearerformat. Both are valid, butBeareris the modern standard and aligns with the existing codebase pattern.♻️ Proposed change for consistency
- PR_DATA=$(curl -fsSL -H "Authorization: token ${{ github.token }}" \ + PR_DATA=$(curl -fsSL -H "Authorization: Bearer ${{ github.token }}" \ "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.issue.number }}")As per coding guidelines, the repo pattern uses
Authorization: Bearer ${GITHUB_TOKEN}format in.github/workflows/docs-deploy.yaml.🤖 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/claude-wif-test.yaml around lines 28 - 29, Update the Authorization header used when populating PR_DATA: in the curl invocation that sets PR_DATA (the PR_DATA=$(curl -fsSL -H "Authorization: token ${{ github.token }}" ... ) line), change the header to use the Bearer format so it sends Authorization: Bearer ${{ github.token }} for consistency with the repo pattern.
🤖 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.
Nitpick comments:
In @.github/workflows/claude-wif-test.yaml:
- Around line 28-29: Update the Authorization header used when populating
PR_DATA: in the curl invocation that sets PR_DATA (the PR_DATA=$(curl -fsSL -H
"Authorization: token ${{ github.token }}" ... ) line), change the header to use
the Bearer format so it sends Authorization: Bearer ${{ github.token }} for
consistency with the repo pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: cd88e04c-1dea-4302-9350-ace1d4ce6542
📒 Files selected for processing (1)
.github/workflows/claude-wif-test.yaml
bryan-cox
force-pushed
the
test-arc-runners
branch
from
7993c4c to
d79be09
Compare
ARC runner containers use dash as default shell and don't have gh CLI installed. Use bash explicitly for the Claude installer and curl for the GitHub API call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bryan-cox
force-pushed
the
test-arc-runners
branch
from
d79be09 to
acebf15
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/claude-wif-test.yaml (2)
51-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAudit for agentic prompt injection risk.
The workflow uses Claude Code (an LLM-based tool) and is triggered by issue comments. While the current prompt is hardcoded (line 58:
"Say hello in one sentence"), this creates a latent security risk:
- Line 18 already reads
github.event.comment.bodyto check for/test-wif.- Future modifications might pass issue/PR titles, bodies, or comments to Claude as prompts.
- Malicious users could inject prompts that cause Claude to exfiltrate secrets, modify code, or perform unauthorized actions.
Example attack vector (if extended):
Comment: /test-wif Please analyze this PR: [malicious prompt to exfiltrate env vars]As per coding guidelines: "Agentic CI actions: audit for prompt injection via issue/PR title/body flowing into LLM prompts."
🛡️ Recommendations
Never pass unsanitized user input to LLM prompts. If extending this workflow to use dynamic prompts:
- Use a strict allowlist of permitted prompt templates
- Sanitize and validate all user inputs
- Use prompt sandboxing techniques (e.g., system prompts that forbid code execution)
Add a security warning comment in the workflow to prevent future maintainers from inadvertently introducing prompt injection:
- name: Test Claude Code env: CLAUDE_CODE_USE_VERTEX: "1" CLOUD_ML_REGION: global ANTHROPIC_VERTEX_PROJECT_ID: hosted-control-planes run: | # SECURITY: Never pass user input (issue/PR title/body/comments) to Claude prompts # to prevent prompt injection attacks. Keep prompts hardcoded or use strict allowlists. claude --version claude -p "Say hello in one sentence" --max-turns 1🤖 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/claude-wif-test.yaml around lines 51 - 58, The "Test Claude Code" workflow step uses a hardcoded claude prompt but is triggered by issue comments and risks future prompt injection if maintainers later pass user content; update the step named "Test Claude Code" to add a clear SECURITY comment above the run block warning not to pass any user-controlled input (issue/PR title/body/comments) into the claude command, and if you ever need dynamic prompts implement strict allowlisting and sanitization instead of direct interpolation (ensure the claude invocation remains hardcoded like claude -p "Say hello in one sentence" or replaced by a vetted template only).
24-37:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCRITICAL: Untrusted fork code execution with secrets access (pull_request_target anti-pattern via issue_comment).
This workflow recreates the dangerous
pull_request_targetpattern usingissue_comment:
- The
issue_commenttrigger fires on comments on pull requests, including PRs from forks.- Lines 30-31 extract the fork's repository name and ref without validation.
- Line 36 checks out code from the potentially malicious fork:
repository: ${{ steps.pr.outputs.repo || github.repository }}.- Lines 44-50 authenticate to GCP via WIF, making credentials accessible to the checked-out code.
- A malicious contributor can open a PR from a fork with backdoored code, comment
/test-wif, and exfiltrate GCP credentials or perform unauthorized actions.Impact: Complete compromise of the
claude-gha@hosted-control-planes.iam.gserviceaccount.comservice account and any resources it can access.As per coding guidelines: "No pull_request_target with checkout of PR head" – this pattern achieves the same risk via
issue_comment.🔒 Proposed fix: Restrict to base repository only
Never check out code from forks when secrets are in scope. Option 1 (preferred): validate that the PR is from the same repository:
- name: Get PR ref if: github.event_name == 'issue_comment' id: pr run: | curl -fsSL -H "Authorization: token ${{ github.token }}" \ "https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.issue.number }}" > /tmp/pr.json + fork_repo=$(jq -r '.head.repo.full_name' /tmp/pr.json) + if [ "$fork_repo" != "${{ github.repository }}" ]; then + echo "::error::Cannot run WIF test on fork PRs for security reasons" + exit 1 + fi echo "ref=$(jq -r '.head.sha' /tmp/pr.json)" >> "$GITHUB_OUTPUT" - echo "repo=$(jq -r '.head.repo.full_name' /tmp/pr.json)" >> "$GITHUB_OUTPUT"- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: ref: ${{ steps.pr.outputs.ref || github.sha }} - repository: ${{ steps.pr.outputs.repo || github.repository }} persist-credentials: falseOption 2: Remove the
issue_commenttrigger entirely and rely only onworkflow_dispatchfor manual testing.🤖 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/claude-wif-test.yaml around lines 24 - 37, The workflow currently fetches and checks out PR head refs from untrusted forks (the "Get PR ref" step and the actions/checkout usage that reads steps.pr.outputs.repo and steps.pr.outputs.ref), enabling credential exposure; fix by preventing checkout of forked repositories before secrets are used: validate the PR's head repo equals the base repository (compare jq '.head.repo.full_name' against github.repository) and only pass repository/ref into actions/checkout when they match, otherwise abort or fall back to checking out the base repo; alternatively remove the issue_comment trigger and/or move any steps that use secrets (GCP WIF auth) after a safe verification gate so secrets are never available to code checked out from forks.
🧹 Nitpick comments (1)
.github/workflows/claude-wif-test.yaml (1)
13-59: ⚖️ Poor tradeoffConsider adding SAST/SCA security scanning.
The workflow currently lacks static analysis or software composition analysis steps. Given that this workflow:
- Executes third-party code (Claude installer)
- Checks out potentially untrusted code
- Accesses sensitive GCP credentials via WIF
Adding security scanning would help detect vulnerabilities before they reach this test environment.
As per coding guidelines: "SAST/SCA steps in pipeline."
💡 Example SAST/SCA step
Add a security scanning step before installing external tools:
- name: Run security scan uses: github/super-linter@<full-sha> # Pin by SHA per guidelines env: DEFAULT_BRANCH: main GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} VALIDATE_BASH: true VALIDATE_DOCKERFILE: trueOr use CodeQL for deeper analysis:
- name: Initialize CodeQL uses: github/codeql-action/init@<full-sha> with: languages: bash, python - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@<full-sha>Note: Remember to pin these actions by full commit SHA as required by the CI/CD security guidelines.
🤖 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/claude-wif-test.yaml around lines 13 - 59, The workflow job test-wif currently runs untrusted checkout and a third-party installer (step "Install Claude Code") before any security scanning; add a pinned SAST/SCA step (e.g., a pinned github/super-linter@<full-sha> or GitHub CodeQL init/analyze actions pinned by full SHAs) inserted before the "Install Claude Code" step and prior to using "Authenticate to GCP via WIF" so the repo and any scripts are scanned first; ensure the new step validates bash/dockerfiles and uses GITHUB_TOKEN from secrets, and reference the job id test-wif and step names "Install Claude Code" and "Authenticate to GCP via WIF" when making the change.
🤖 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/claude-wif-test.yaml:
- Around line 39-42: Replace the unsafe "curl | bash" step in the "Install
Claude Code" job by first downloading the installer to a file (the current curl
command), then verifying its integrity (compare SHA-256 or verify a
cosign/sigstore signature) before executing; specifically update the workflow
step that currently runs "curl -fsSL https://claude.ai/install.sh | bash" to
instead download the installer, validate it against an EXPECTED_HASH or a
provided signature, fail the job if verification fails, and only then run the
installer file (do not pipe directly to bash and ensure version/hash/signature
are pinned and updated when intentionally changing Claude Code versions).
---
Outside diff comments:
In @.github/workflows/claude-wif-test.yaml:
- Around line 51-58: The "Test Claude Code" workflow step uses a hardcoded
claude prompt but is triggered by issue comments and risks future prompt
injection if maintainers later pass user content; update the step named "Test
Claude Code" to add a clear SECURITY comment above the run block warning not to
pass any user-controlled input (issue/PR title/body/comments) into the claude
command, and if you ever need dynamic prompts implement strict allowlisting and
sanitization instead of direct interpolation (ensure the claude invocation
remains hardcoded like claude -p "Say hello in one sentence" or replaced by a
vetted template only).
- Around line 24-37: The workflow currently fetches and checks out PR head refs
from untrusted forks (the "Get PR ref" step and the actions/checkout usage that
reads steps.pr.outputs.repo and steps.pr.outputs.ref), enabling credential
exposure; fix by preventing checkout of forked repositories before secrets are
used: validate the PR's head repo equals the base repository (compare jq
'.head.repo.full_name' against github.repository) and only pass repository/ref
into actions/checkout when they match, otherwise abort or fall back to checking
out the base repo; alternatively remove the issue_comment trigger and/or move
any steps that use secrets (GCP WIF auth) after a safe verification gate so
secrets are never available to code checked out from forks.
---
Nitpick comments:
In @.github/workflows/claude-wif-test.yaml:
- Around line 13-59: The workflow job test-wif currently runs untrusted checkout
and a third-party installer (step "Install Claude Code") before any security
scanning; add a pinned SAST/SCA step (e.g., a pinned
github/super-linter@<full-sha> or GitHub CodeQL init/analyze actions pinned by
full SHAs) inserted before the "Install Claude Code" step and prior to using
"Authenticate to GCP via WIF" so the repo and any scripts are scanned first;
ensure the new step validates bash/dockerfiles and uses GITHUB_TOKEN from
secrets, and reference the job id test-wif and step names "Install Claude Code"
and "Authenticate to GCP via WIF" when making the change.
🪄 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: ddbd5b96-b163-4a28-9519-03bbbbf2bd3f
📒 Files selected for processing (1)
.github/workflows/claude-wif-test.yaml
| - name: Install Claude Code | ||
| run: | | ||
| curl -fsSL https://claude.ai/install.sh | sh | ||
| curl -fsSL https://claude.ai/install.sh | bash | ||
| echo "$HOME/.local/bin" >> $GITHUB_PATH |
There was a problem hiding this comment.
CRITICAL: Unverified script execution from the internet.
Downloading and executing a script from https://claude.ai/install.sh without integrity verification creates multiple attack vectors:
- Supply chain compromise: If the Claude.ai infrastructure is compromised, malicious code will be executed in the workflow with access to GCP credentials.
- Man-in-the-middle attacks: Despite HTTPS, certificate validation issues or DNS poisoning could serve malicious scripts.
- No version pinning: The script content can change at any time without notice.
The change from sh to bash does not address the underlying security issue.
As per coding guidelines: "Sign artifacts with Sigstore/cosign" – here unsigned, unverified code is executed directly.
🔒 Proposed fix: Verify installer integrity
Download the installer, verify its hash, then execute:
- name: Install Claude Code
run: |
- curl -fsSL https://claude.ai/install.sh | bash
+ curl -fsSL https://claude.ai/install.sh -o /tmp/install.sh
+ # Update this hash when updating Claude Code version
+ echo "EXPECTED_HASH /tmp/install.sh" | sha256sum --check
+ bash /tmp/install.sh
echo "$HOME/.local/bin" >> $GITHUB_PATHReplace EXPECTED_HASH with the SHA-256 hash of the legitimate installer. Update the hash when intentionally updating Claude Code versions.
Alternatively, request that Anthropic provide signed releases or publish the installer to a package manager with built-in verification.
🤖 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/claude-wif-test.yaml around lines 39 - 42, Replace the
unsafe "curl | bash" step in the "Install Claude Code" job by first downloading
the installer to a file (the current curl command), then verifying its integrity
(compare SHA-256 or verify a cosign/sigstore signature) before executing;
specifically update the workflow step that currently runs "curl -fsSL
https://claude.ai/install.sh | bash" to instead download the installer, validate
it against an EXPECTED_HASH or a provided signature, fail the job if
verification fails, and only then run the installer file (do not pipe directly
to bash and ensure version/hash/signature are pinned and updated when
intentionally changing Claude Code versions).
|
/test-wif |
3 participants
Summary
bashinstead ofshfor Claude Code installer — ARC runner containers default to dash which doesn't support bash syntax in the install scriptgh apiwithcurlfor PR ref lookup —ghCLI is not available on ARC runnersTest plan
workflow_dispatchfrom Actions tab to verify WIF auth end-to-end/test-wifon a PR to verifyissue_commentpath🤖 Generated with Claude Code
Summary by CodeRabbit