NO-JIRA: add address-review-comments workflow - #8621
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. |
📝 WalkthroughWalkthroughWorkflow renamed to "Address Review Comments" and retargeted to run on created issue comments starting with Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub
participant Workflow as "Address Review Comments Workflow"
participant PRMeta as "Get PR ref step"
participant Checkout as "actions/checkout"
participant Claude as "Claude Code (utils:address-reviews)"
GitHub->>Workflow: issue_comment created (startsWith '/address-review-comments')
Workflow->>Workflow: check author_association in (MEMBER, OWNER, COLLABORATOR)
Workflow->>PRMeta: fetch PR JSON
PRMeta-->>Workflow: outputs branch=head.ref, repo=head.repo.full_name
Workflow->>Checkout: checkout using branch/repo (persist-credentials:true, fetch-depth:0)
Workflow->>Claude: run claude -p "/utils:address-reviews $PR_NUMBER" --max-turns 100
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 |
|
[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
ac30daf to
bd38c97
Compare
openshift-ci
Bot
added
the
needs-rebase
bd38c97 to
cafaec6
Compare
bryan-cox
changed the title
cafaec6 to
668c73c
Compare
openshift-ci
Bot
removed
the
needs-rebase
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/address-review-comments.yaml:
- Around line 35-37: The checkout step currently uses the PR head ref
(steps.pr.outputs.ref) with persist-credentials: true, which gives untrusted PR
code write access; modify the workflow to avoid checking out the untrusted
repo/ref with persisted credentials: either (A) checkout the base branch instead
and fetch the PR changes without persist-credentials, (B) move any git push or
sensitive operations into a separate job that checks out only trusted code (base
branch) and runs with persist-credentials: true, or (C) add explicit fork
detection using steps.pr.outputs.repo/actor and fail or skip the job for forked
PRs; update the checkout action usage accordingly to remove persist-credentials:
true for untrusted checkouts.
- Around line 64-68: The workflow invokes the agentic command
"/utils:address-reviews" with PR_NUMBER and uses claude with "--max-turns 100",
which exposes risk of prompt injection from untrusted PR review comments; update
the workflow to (1) fetch and filter/sanitize review comment content before
handing data to the agent (e.g., strip markdown, remove code blocks, and reject
suspicious tokens) rather than piping raw comments, (2) gate execution to
trusted author_associations only (check the PR review author_association and
only run for e.g., "OWNER","MEMBER","COLLABORATOR"), and (3) reduce interactive
power by lowering "--max-turns" and adding a human-in-the-loop checkpoint for
any write actions or high-risk suggestions produced by the claude invocation
(i.e., require manual approval before applying changes).
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f259b9ab-900c-41a7-8f8f-b2748d0464ff
📒 Files selected for processing (1)
.github/workflows/address-review-comments.yaml
668c73c to
5feecc6
Compare
|
/area ci-tooling |
openshift-ci
Bot
added
area/ci-tooling
5feecc6 to
76eeb34
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/address-review-comments.yaml (1)
64-73:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUntrusted review text still reaches Claude with write permissions.
The trusted
author_associationcheck only applies to the slash-command comment./utils:address-reviewsthen operates on PR review comments, which can still contain attacker-controlled prompt text. With repository write access, that remains a high-risk agentic CI path.Filter or trust-bound the review comments before they reach Claude, or add a human approval checkpoint before any write action. As per coding guidelines, "Agentic CI actions: audit for prompt injection via issue/PR title/body flowing into LLM prompts".
🤖 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/address-review-comments.yaml around lines 64 - 73, The workflow currently pipes raw PR review comments into the Claude agent via the "/utils:address-reviews" slash-command (invoked by claude -p "/utils:address-reviews $PR_NUMBER") after only validating the comment author_association, which allows untrusted review text to reach an LLM with write-capable credentials (GH_TOKEN); modify the CI step so review text is pre-filtered or explicitly trusted before being passed to Claude: either (a) fetch PR reviews in the runner and validate/trust only reviews from allowed associations or specific reviewers and sanitize/whitelist content before calling "/utils:address-reviews", or (b) require a manual approval job (human_in_the_loop) that gates the claude invocation; ensure the claude invocation and the "/utils:address-reviews" input are only executed after the trust check and do not run with GH_TOKEN write permissions unless approved.
🤖 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/address-review-comments.yaml:
- Around line 25-31: The PR step currently writes head.sha to the output key
"ref", causing actions/checkout to land on a detached HEAD; update the curl/jq
block in the job with id "pr" so it stores the branch name by using .head.ref
(not .head.sha) for the "ref" output and keep "repo" from .head.repo.full_name,
and make the same change in the duplicated block referenced around lines 38-43;
ensure the checkout step (actions/checkout) uses the "ref" output to check out
the PR branch rather than a commit SHA.
- Line 16: The job currently uses contains(github.event.comment.body,
'/address-review-comments') which can match incidental mentions; update the
condition to only trigger on an intentional slash command by using
startsWith(github.event.comment.body, '/address-review-comments') or by parsing
github.event.comment.body to extract the first token and compare equality to
'/address-review-comments' so only comments beginning with that exact command
fire; change the contains(...) expression to startsWith(...) or equivalent
parsing logic referencing github.event.comment.body in the workflow.
---
Duplicate comments:
In @.github/workflows/address-review-comments.yaml:
- Around line 64-73: The workflow currently pipes raw PR review comments into
the Claude agent via the "/utils:address-reviews" slash-command (invoked by
claude -p "/utils:address-reviews $PR_NUMBER") after only validating the comment
author_association, which allows untrusted review text to reach an LLM with
write-capable credentials (GH_TOKEN); modify the CI step so review text is
pre-filtered or explicitly trusted before being passed to Claude: either (a)
fetch PR reviews in the runner and validate/trust only reviews from allowed
associations or specific reviewers and sanitize/whitelist content before calling
"/utils:address-reviews", or (b) require a manual approval job
(human_in_the_loop) that gates the claude invocation; ensure the claude
invocation and the "/utils:address-reviews" input are only executed after the
trust check and do not run with GH_TOKEN write permissions unless approved.
🪄 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: df6f5468-39e6-4de3-9fa7-025bb7890184
📒 Files selected for processing (1)
.github/workflows/address-review-comments.yaml
03193d5 to
517cd07
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/address-review-comments.yaml (1)
31-37:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid checking out the PR repo with persisted write credentials.
steps.pr.outputs.repocan point at an external fork, butpersist-credentials: truestill writes this workflow's write-scoped token into that checkout. That recreates the same trust boundary aspull_request_target: untrusted PR contents are being processed in a workspace that now holds base-repo write creds and later receives WIF auth. Please either fail fork PRs before checkout or split the write path into a separate job that never checks out the PR head with persisted credentials.As per coding guidelines, "No pull_request_target with checkout of PR head".
🤖 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/address-review-comments.yaml around lines 31 - 37, The workflow currently checks out the PR head using actions/checkout with persist-credentials: true and repository: ${{ steps.pr.outputs.repo }}, which can be an external fork and leaks write-scoped tokens; update the checkout step (the actions/checkout invocation) to not persist credentials for untrusted forks (set persist-credentials: false) or add logic to fail/stop when steps.pr.outputs.repo indicates a fork before any checkout, or refactor so that the job that uses ${{ steps.pr.outputs.repo }} only performs a read-only checkout and any write operations run in a separate job that checks out the base repo with persist-credentials: true; ensure the change targets the actions/checkout usage and the persist-credentials setting referenced in the diff.
🤖 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/address-review-comments.yaml:
- Around line 13-21: The job "address-review-comments" can run concurrently for
the same PR and race when amending/rebasing branches; add a concurrency block to
serialize runs per PR by using a PR-scoped group (e.g. group:
address-review-comments-${{ github.event.pull_request.number }}) and set
cancel-in-progress: false so new triggers are queued rather than running in
parallel; place this concurrency stanza at the "address-review-comments" job
level to ensure only one agent run operates on a given PR branch at a time.
---
Duplicate comments:
In @.github/workflows/address-review-comments.yaml:
- Around line 31-37: The workflow currently checks out the PR head using
actions/checkout with persist-credentials: true and repository: ${{
steps.pr.outputs.repo }}, which can be an external fork and leaks write-scoped
tokens; update the checkout step (the actions/checkout invocation) to not
persist credentials for untrusted forks (set persist-credentials: false) or add
logic to fail/stop when steps.pr.outputs.repo indicates a fork before any
checkout, or refactor so that the job that uses ${{ steps.pr.outputs.repo }}
only performs a read-only checkout and any write operations run in a separate
job that checks out the base repo with persist-credentials: true; ensure the
change targets the actions/checkout usage and the persist-credentials setting
referenced in the diff.
🪄 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: e0a83810-fc97-4a25-bb65-ccee3dceee0b
📒 Files selected for processing (1)
.github/workflows/address-review-comments.yaml
The container: directive requires containerMode on ARC runners which needs anyuid SCC and a long-running process. Replace it by installing Claude and cloning ai-helpers directly in workflow steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
517cd07 to
ab030b0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/address-review-comments.yaml (1)
50-60: ⚡ Quick winConsider pinning external dependencies for supply chain security.
Two supply chain risks worth noting:
Line 52: The
curl | bashpattern executes code without integrity verification. Ifclaude.aiwere compromised, malicious code would run with workflow permissions.Line 57: The
ai-helpersclone uses--depth 1on the default branch without pinning to a specific commit or tag. A compromised commit could affect all subsequent runs.Suggested mitigations:
- For Claude Code: verify a checksum or pin to a versioned release URL if available
- For ai-helpers: pin to a specific commit SHA (e.g.,
git clone --depth 1 --branch <tag-or-sha>)These are hardening recommendations rather than blockers, given both sources are controlled (Anthropic official domain, internal org repo).
♻️ Example: Pin ai-helpers to a specific commit
- git clone --depth 1 https://github.com/openshift-eng/ai-helpers.git "$GITHUB_WORKSPACE/ai-helpers" + git clone --depth 1 --branch v1.0.0 https://github.com/openshift-eng/ai-helpers.git "$GITHUB_WORKSPACE/ai-helpers"Or with a commit SHA:
- git clone --depth 1 https://github.com/openshift-eng/ai-helpers.git "$GITHUB_WORKSPACE/ai-helpers" + git clone https://github.com/openshift-eng/ai-helpers.git "$GITHUB_WORKSPACE/ai-helpers" + cd "$GITHUB_WORKSPACE/ai-helpers" && git checkout <known-good-sha>As per coding guidelines: "Pin actions by full SHA, not tag" — extending this principle to cloned repositories improves reproducibility and supply chain integrity.
🤖 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/address-review-comments.yaml around lines 50 - 60, Replace the unsecured curl|bash install and unpinned git clone with verifiable, pinned sources: for the Claude install step (the curl -fsSL https://claude.ai/install.sh | bash invocation) change to fetching a specific release or installer and validating its checksum/signature before executing; for the ai-helpers setup (the git clone --depth 1 ... "$GITHUB_WORKSPACE/ai-helpers") clone a specific commit SHA or tag (use --branch <tag-or-sha> and --depth 1) so the repository is pinned and reproducible, and update the workflow to fail if checksum/signature or the exact commit cannot be verified.
🤖 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/address-review-comments.yaml:
- Around line 50-60: Replace the unsecured curl|bash install and unpinned git
clone with verifiable, pinned sources: for the Claude install step (the curl
-fsSL https://claude.ai/install.sh | bash invocation) change to fetching a
specific release or installer and validating its checksum/signature before
executing; for the ai-helpers setup (the git clone --depth 1 ...
"$GITHUB_WORKSPACE/ai-helpers") clone a specific commit SHA or tag (use --branch
<tag-or-sha> and --depth 1) so the repository is pinned and reproducible, and
update the workflow to fail if checksum/signature or the exact commit cannot be
verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 68533541-5bfb-48c7-9b62-ccbfd7a7c21f
📒 Files selected for processing (1)
.github/workflows/address-review-comments.yaml
| cat /tmp/claude-output.txt | ||
| exit 1 | ||
| fi | ||
| claude -p "/utils:address-reviews $PR_NUMBER" --model claude-opus-4-6 --max-turns 100 |
There was a problem hiding this comment.
do we want to pin a specific effort and --allowedTools?
There was a problem hiding this comment.
Good call. The model is already pinned via --model claude-opus-4-6. Adding --allowedTools "Bash Read Write Edit Grep Glob WebFetch" to match what we have in the Prow review-agent job in openshift/release.
AI-assisted response via Claude Code
|
@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. |
|
|
||
| - name: Install Claude Code | ||
| run: | | ||
| curl -fsSL https://claude.ai/install.sh | bash |
There was a problem hiding this comment.
should we pin a specific version?
have you considered https://github.com/anthropics/claude-code-action
There was a problem hiding this comment.
Good suggestion on both counts.
Version pinning: The install script (curl | bash) always gets latest. We could pin by downloading a specific release tarball or using the claude-code-action which supports version pinning natively. Will address as a follow-up.
claude-code-action: Yes — it supports Vertex AI + WIF natively and handles installation, tool configuration, and structured outputs. It would replace our manual install + plugin setup steps. The main question is whether it supports loading ai-helpers marketplace plugins (the utils:address-reviews command). Will investigate and potentially migrate to it as a follow-up.
AI-assisted response via Claude Code
|
I now have all the evidence needed. Here is the complete analysis: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryBoth failures are caused by corrupted entries in the shared Go build cache ( Root CauseThe ARC (Actions Runner Controller) runner pods in the The shared read-only cache layer contains corrupted or stale entries with invalid checksums. When The corruption likely occurred during a cache population phase (e.g., a prior job writing to the shared cache while being terminated mid-write, a cache race between concurrent writers, or a PV sync issue). Key indicators:
Recommendations
Evidence
|
|
/retest |
|
/lgtm |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
|
/verified later @bryan-cox |
openshift-ci-robot
added
verified-later
verified
|
@bryan-cox: This PR has been marked to be verified later by 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. |
|
/retest |
9f16502
into
openshift:main
3 participants
Summary
address-review-commentsworkflow triggered by/address-review-commentson PRsutils:address-reviewsai-helpers pluginclaude-wif-testsmoke test workflow with a real toolChanges
claude-wif-test.yamltoaddress-review-comments.yaml/address-review-commentscomment on a PR (MEMBER/OWNER/COLLABORATOR only)startsWithinstead ofcontainsto prevent accidental triggers from quoted mentionshead.ref) instead of SHA to avoid detached HEADworkflow_dispatch(no PR context without a comment trigger)fetch-depth: 0for full git history (rebasing, commit amending)contents: writeandpull-requests: writepersist-credentialsfor git push (credentials scoped to job lifetime)$GITHUB_WORKSPACEfor plugin file accessutilsplugin (containsaddress-reviewscommand)Test plan
/address-review-commentson a PR with review comments🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores