feat: create separate PRs per branch for downstream repos in cve.fix#101
Merged
bobbravo2 merged 8 commits intoambient-code:mainfrom Apr 7, 2026
Merged
Conversation
- upstream/midstream: PR against default_branch only (main) - downstream: separate PR for default_branch + each active_release_branch Each branch is independently fixed and gets its own PR — never combined. Fix branch naming includes the target branch for clarity, e.g.: fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1 No mapping file changes needed — logic derived from existing repo_type, default_branch, and active_release_branches fields. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthroughcve.fix now derives TARGET_BRANCHES from repo_type, authenticates via GitHub Classic PAT ( Changes
Sequence Diagram(s)sequenceDiagram
participant WF as Workflow
participant GH as GitHub CLI/API (gh)
participant Git as Local Git
participant Origin as Origin Repo
participant Fork as Fork Repo
participant Jira as Jira API
WF->>GH: gh auth login (Classic PAT)
WF->>Git: git clone --depth=1 /tmp/<org>/<repo>
WF->>GH: gh api -> check can_push?
alt has write access
WF->>Git: for each TARGET_BRANCH: git checkout && git pull
WF->>Git: apply fixes, commit, create branch
Git->>Origin: git push branch
WF->>GH: create PR to Origin
else no write access
WF->>Fork: push branch to Fork
WF->>GH: create PR from Fork -> Origin
end
WF->>Jira: if CVE not found -> add VEX justification or create human-review artifact
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 163-172: Update the fenced code blocks in
workflows/cve-fixer/.claude/commands/cve.fix.md to satisfy MD031/MD040 by adding
explicit language identifiers (e.g., ```text for the plain table and ```bash for
the shell block) and ensuring there is a blank line before and after each fenced
block; locate the blocks using the existing fence markers (``` and ```bash) and
the shown content ("upstream llm-d/..." table and the "# Clone once:" shell
example) and insert the missing blank lines and the language tag on the opening
fence for each block.
- Around line 174-179: The branch naming guidance is inconsistent: the
multi-branch example uses a target-branch token (e.g.,
fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1) but the canonical rule later
omits it (fix/cve-...-<package>-attempt-1), risking collisions across base
branches; update the canonical naming rule to include a target-branch
placeholder (e.g., <target-branch> or the actual base branch name) and make both
the example and the canonical pattern consistent so every fix branch (symbols:
fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-N) uniquely identifies the
base branch and prevents reuse/overwrite when repeating steps 4–11 for each
branch.
- Around line 183-191: The current flow that "Clone the repo once" then loops
over TARGET_BRANCHES (steps 5–11) can leak state between branch iterations;
update the procedure so each target branch runs in an isolated workspace: for
each branch in TARGET_BRANCHES either (a) create a fresh clone (git clone into a
new REPO_DIR per branch) or (b) perform a strict reset (git fetch --all; git
checkout <branch>; git reset --hard origin/<branch>; git clean -fdx) before
applying fixes, and ensure git credentials configuration (gh auth setup-git /
credential.helper / SSH fallback) is applied for each cloned workspace so
pushes/PRs cannot be affected by artifacts, uncommitted changes, or lockfile
drift from previous iterations.
- Around line 155-159: The TARGET_BRANCHES array construction can include
duplicates if ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; change the logic
that sets TARGET_BRANCHES (the branch assembly around TARGET_BRANCHES,
DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES) to deduplicate entries before use —
for example, build TARGET_BRANCHES by iterating DEFAULT_BRANCH and each element
of ACTIVE_RELEASE_BRANCHES and only appending branches not already present (or
use a temporary associative set) so TARGET_BRANCHES contains unique branch names
and avoids duplicate PR attempts.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dcde3137-0bcb-4cf1-9ef6-55b5f3a67393
📒 Files selected for processing (1)
workflows/cve-fixer/.claude/commands/cve.fix.md
Ambient sessions inject secrets differently — JIRA_API_TOKEN and JIRA_EMAIL may be available to API calls even when not visible to bash [ -z "$VAR" ] checks, causing false "missing credentials" errors. Replace the upfront env var check with a lightweight GET /myself test call: proceed on 200, only complain on 401/403. Never hard-stop just because the shell check returns empty. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.find.md`:
- Around line 83-87: Insert a single blank line before the fenced code block in
the "a. Set up variables (AUTH already set from Step 2):" list item so the
markdown has a blank line immediately before the ```bash fence (fixing MD031).
Locate the fenced block in workflows/cve-fixer/.claude/commands/cve.find.md
under the "COMPONENT_NAME" setup section and add the blank line above the
```bash line to make the block compliant.
- Around line 69-75: The fenced code block starting with "```bash" containing
the export lines currently lacks surrounding blank lines (MD031); add one blank
line immediately before the opening "```bash" and one blank line immediately
after the closing "```" so the code block is separated from the surrounding list
text (update the block in workflows/cve-fixer/.claude/commands/cve.find.md where
the export JIRA_API_TOKEN/JIRA_EMAIL lines appear).
- Around line 58-79: The curl healthcheck using JIRA_BASE_URL and TEST_RESPONSE
should not treat HTTP 403 as "insufficient permissions" because
/rest/api/3/myself returns 401 for both auth failures and permission issues;
update the logic that branches on TEST_RESPONSE to consolidate 401 handling (log
both missing/invalid creds and insufficient permissions together and instruct
the user to check API token/email and permissions) and remove or disable the
separate 403 branch, or alternatively replace the endpoint with a
permissions-specific endpoint if you really need to distinguish 403; keep the
existing note about not pre-checking env vars (do not add a [ -z
"$JIRA_API_TOKEN" ] check).
- Around line 58-77: The documentation says to "retry once" on network/timeout
but the TEST_RESPONSE curl call lacks retry options; update the curl invocation
that sets TEST_RESPONSE (the command using JIRA_BASE_URL and AUTH) to perform a
single retry on transient failures by adding curl retry flags (e.g., --retry 1
plus an appropriate --retry-delay and --retry-connrefused) while preserving
existing timeouts (--connect-timeout, --max-time) and the exit-code capture (-s
-o /dev/null -w "%{http_code}"); ensure the adjusted curl still returns an HTTP
status code into TEST_RESPONSE so the subsequent HTTP 200/401/403/Other handling
continues to work.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f7d15b0-6ed9-419e-b03a-15674de9f5c6
📒 Files selected for processing (1)
workflows/cve-fixer/.claude/commands/cve.find.md
Comment on lines
+69
to
+75
| - **HTTP 401** → credentials missing or invalid. Only now inform the user: | ||
| - Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets | ||
| - If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export: | ||
| ```bash | ||
| export JIRA_API_TOKEN="your-token-here" | ||
| export JIRA_EMAIL="your-email@redhat.com" | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add blank lines around fenced code blocks for markdown compliance.
Markdownlint reports missing blank lines around the code block per MD031. Add a blank line before line 72 and after line 75 to improve formatting consistency.
📝 Proposed formatting fix
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
+
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
+
- **HTTP 403** → token valid but insufficient permissions — inform user🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 72-72: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 75-75: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 69 - 75, The
fenced code block starting with "```bash" containing the export lines currently
lacks surrounding blank lines (MD031); add one blank line immediately before the
opening "```bash" and one blank line immediately after the closing "```" so the
code block is separated from the surrounding list text (update the block in
workflows/cve-fixer/.claude/commands/cve.find.md where the export
JIRA_API_TOKEN/JIRA_EMAIL lines appear).
…access repos - Recommend Classic PAT with repo scope over fine-grained PAT or Ambient Code GitHub App (neither works reliably for cross-org repos like llm-d/*, eval-hub/*, trustyai-explainability/*) - Add write access check: use gh api repos/X --jq '.permissions.push' before attempting to push - Add fork fallback: if no write access, create fork, push fix branch to fork, and open PR targeting the original repo with head: user:branch This handles all upstream external org repos automatically Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
workflows/cve-fixer/.claude/commands/cve.fix.md (4)
153-160:⚠️ Potential issue | 🟠 MajorDeduplicate
TARGET_BRANCHESbefore processing to prevent duplicate PR attempts.If
ACTIVE_RELEASE_BRANCHEScontainsDEFAULT_BRANCH, this list can duplicate targets and cause redundant PR/push attempts.Suggested doc fix
if [ "$REPO_TYPE" = "downstream" ]; then TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}") else TARGET_BRANCHES=("$DEFAULT_BRANCH") fi + +# Deduplicate while preserving order +UNIQUE_TARGET_BRANCHES=() +for b in "${TARGET_BRANCHES[@]}"; do + [ -z "$b" ] && continue + [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b") +done +TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 153 - 160, The TARGET_BRANCHES assignment can produce duplicates when ACTIVE_RELEASE_BRANCHES includes DEFAULT_BRANCH; update the logic that builds TARGET_BRANCHES (the block using REPO_TYPE, DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES) to deduplicate entries before further processing — for example, after composing TARGET_BRANCHES, iterate and rebuild a new array using a seen set (declare -A seen) to skip already-added branch names so each branch appears only once; ensure downstream consumers of TARGET_BRANCHES use the deduplicated array.
162-173:⚠️ Potential issue | 🟡 MinorFix fenced code block formatting for markdownlint compliance (MD031/MD040).
This fence is missing a language identifier and blank-line framing.
Suggested doc fix
- ``` + + ```text upstream llm-d/llm-d-inference-scheduler → PR against: main midstream opendatahub-io/llm-d-inference-scheduler → PR against: main downstream red-hat-data-services/llm-d-inference-scheduler → PRs against: - main - rhoai-3.3 - rhoai-3.4 - rhoai-3.4-ea.1 - rhoai-3.4-ea.2 ``` +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 162 - 173, The fenced code block for the "llm-d inference-scheduler" example is missing a language identifier and blank-line framing; update the markdown by adding a blank line before and after the triple-backtick fence and include a language token (e.g., "text") after the opening ``` so the block around the "upstream llm-d/llm-d-inference-scheduler ..." example is fenced as ```text ... ``` to satisfy MD031/MD040.
206-207:⚠️ Potential issue | 🟠 MajorSingle-clone branch loop still contradicts per-target isolated execution.
Current guidance still uses one clone + checkout loop, which risks state leakage across target branches and conflicts with the PR objective of independent clone→fix→test→PR per branch.
Suggested doc fix
-REPO_DIR="/tmp/${REPO_ORG}/${REPO_NAME}" +REPO_DIR_BASE="/tmp/${REPO_ORG}" ... -Steps 5–11 run in a loop over `TARGET_BRANCHES` — for each branch: -- `git checkout <branch>` and `git pull` to ensure it is up to date -- Apply fix and create PR targeting that branch (from fork if no direct push access) +Steps 5–11 run independently per `TARGET_BRANCH` — for each branch: +- Create branch-specific workspace: `REPO_DIR="${REPO_DIR_BASE}/${REPO_NAME}-${TARGET_BRANCH}"` +- Clone/fetch in that workspace +- Checkout the target branch in that workspace +- Apply fix, run tests, and create PR for that target branch onlyAlso applies to: 239-243, 246-258
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 206 - 207, The workflow currently sets a single REPO_DIR and then iterates branches using checkout loops, which causes state leakage; modify the flow so each target branch performs an independent clone into its own temporary directory (create a fresh REPO_DIR per-target inside the per-target loop), perform git checkout, run the fix/test/PR sequence, and then clean up that temp clone; update mentions of REPO_DIR, the per-target loop, and any checkout logic to reflect per-target isolated clone→checkout→fix→test→PR execution and ensure cleanup after each target.
174-178:⚠️ Potential issue | 🟠 MajorBranch naming guidance is still inconsistent with canonical naming section.
These lines correctly include
<target-branch>in examples, but the canonical pattern later still omits it (fix/cve-...-<package>-attempt-1), which can collide across bases.Suggested doc fix
-- Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-attempt-1` +- Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-<target-branch>-attempt-1`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 174 - 178, Update the canonical branch-naming pattern to include the <target-branch> token so it matches the earlier examples; specifically change the canonical example `fix/cve-...-<package>-attempt-1` to the consistent pattern used above `fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-1` (or equivalent ordering used in the Multi-repo + multi-branch examples) and update any other occurrences in the document so all examples and the canonical section use the same `<target-branch>`-inclusive format to avoid cross-branch collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 153-160: The TARGET_BRANCHES assignment can produce duplicates
when ACTIVE_RELEASE_BRANCHES includes DEFAULT_BRANCH; update the logic that
builds TARGET_BRANCHES (the block using REPO_TYPE, DEFAULT_BRANCH and
ACTIVE_RELEASE_BRANCHES) to deduplicate entries before further processing — for
example, after composing TARGET_BRANCHES, iterate and rebuild a new array using
a seen set (declare -A seen) to skip already-added branch names so each branch
appears only once; ensure downstream consumers of TARGET_BRANCHES use the
deduplicated array.
- Around line 162-173: The fenced code block for the "llm-d inference-scheduler"
example is missing a language identifier and blank-line framing; update the
markdown by adding a blank line before and after the triple-backtick fence and
include a language token (e.g., "text") after the opening ``` so the block
around the "upstream llm-d/llm-d-inference-scheduler ..." example is fenced as
```text ... ``` to satisfy MD031/MD040.
- Around line 206-207: The workflow currently sets a single REPO_DIR and then
iterates branches using checkout loops, which causes state leakage; modify the
flow so each target branch performs an independent clone into its own temporary
directory (create a fresh REPO_DIR per-target inside the per-target loop),
perform git checkout, run the fix/test/PR sequence, and then clean up that temp
clone; update mentions of REPO_DIR, the per-target loop, and any checkout logic
to reflect per-target isolated clone→checkout→fix→test→PR execution and ensure
cleanup after each target.
- Around line 174-178: Update the canonical branch-naming pattern to include the
<target-branch> token so it matches the earlier examples; specifically change
the canonical example `fix/cve-...-<package>-attempt-1` to the consistent
pattern used above `fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-1` (or
equivalent ordering used in the Multi-repo + multi-branch examples) and update
any other occurrences in the document so all examples and the canonical section
use the same `<target-branch>`-inclusive format to avoid cross-branch
collisions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c2120c28-59ea-4e75-8e7d-6748971ae08e
📒 Files selected for processing (1)
workflows/cve-fixer/.claude/commands/cve.fix.md
cve.fix: when both scan and package check find no CVE evidence: - Auto-determine VEX justification where possible: 1. Component not Present — package not in any manifest (auto) 2. Vulnerable Code not Present — package at safe version (auto) 3. Vulnerable Code not in Execute Path — govulncheck call graph (auto, Go only) 4. Vulnerable Code cannot be Controlled by Adversary — human judgment 5. Inline Mitigations already Exist — human judgment - For cases 1-3: add Jira comment with justification + evidence, do not auto-close - For cases 4-5: log in artifacts/vex-needs-human-review-CVE-*.md, skip Jira comment - In interactive mode: prompt user to select from all 5 options cve.find: add --ignore-vex flag to exclude issues already closed with VEX justification (Not a Bug / Obsolete / Won't Fix) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
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 (1)
workflows/cve-fixer/.claude/commands/cve.find.md (1)
36-46:⚠️ Potential issue | 🟠 MajorPersist
--ignore-vexas a boolean alongside--ignore-resolved.Step 1 introduces
--ignore-vex, but the storage note still only documents--ignore-resolved. That leaves Step 3’sIGNORE_VEXbranch under-specified.Suggested doc patch
- - Store the `--ignore-resolved` flag as a boolean for use in step 3 + - Store `--ignore-resolved` and `--ignore-vex` flags as booleans for use in step 3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 36 - 46, The docs mention the --ignore-vex flag but never instruct storing it; update the behavior notes so --ignore-vex is persisted as a boolean just like --ignore-resolved (e.g., document IGNORE_VEX alongside IGNORE_RESOLVED), and clarify Step 3’s IGNORE_VEX branch to read that boolean when filtering results; ensure the instruction still forbids using AskUserQuestion or presenting options and that component prompting remains unchanged.
♻️ Duplicate comments (6)
workflows/cve-fixer/.claude/commands/cve.find.md (2)
72-76:⚠️ Potential issue | 🟡 MinorFix MD031 fenced-block spacing in changed list items.
The fenced
bashblocks here still need blank lines before/after to satisfy markdownlint.Also applies to: 84-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 72 - 76, Add a blank line before and after each fenced bash code block that contains the export lines (the block starting with the URL "https://id.atlassian.com/manage-profile/security/api-tokens" and the block with "export JIRA_API_TOKEN" / "export JIRA_EMAIL") so the fenced blocks have an empty line above and below to satisfy MD031; update both occurrences (the block containing the export lines and the later duplicate block) accordingly.
62-78:⚠️ Potential issue | 🟠 MajorAuth probe logic for
/myselfis still inconsistent and retry is undocumented in command behavior.This block still treats HTTP 403 separately and says “retry once” without curl retry flags. Either consolidate 401 handling for
/myself, or switch endpoint if you need permission-specific branching.Suggested doc patch
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \ --connect-timeout 10 --max-time 15 \ + --retry 1 --retry-delay 2 --retry-connrefused \ -H "Authorization: Basic ${AUTH}" \ -H "Content-Type: application/json" \ "${JIRA_BASE_URL}/rest/api/3/myself") @@ - - **HTTP 403** → token valid but insufficient permissions — inform user - - **Other / timeout** → network issue — inform user and retry once + - **Other / timeout** → network issue — inform user (probe includes one retry)For Jira Cloud REST API v3, what are the documented response codes for GET /rest/api/3/myself, specifically whether 403 is expected versus 401 for auth/access failures?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 62 - 78, The auth probe using GET "${JIRA_BASE_URL}/rest/api/3/myself" is inconsistent: it documents a special HTTP 403 case and “retry once” but the curl invocation (TEST_RESPONSE) has no retry behavior and the endpoint may not reliably differentiate 401 vs 403 for auth issues; update the logic around TEST_RESPONSE to either consolidate 401/403 handling for auth failures or switch to a permission-specific endpoint if you need to detect permission vs credential problems, and explicitly implement a single retry (using curl retry flags or a coded retry loop) when handling network/timeout responses so the documented “retry once” matches actual behavior; ensure messages reference TEST_RESPONSE and the "/rest/api/3/myself" probe so maintainers can find and update the probe logic.workflows/cve-fixer/.claude/commands/cve.fix.md (4)
239-258:⚠️ Potential issue | 🟠 Major“Clone once + branch loop” conflicts with independent branch execution.
This section still describes shared workspace iteration, which can leak state between branch runs and contradicts the per-target independent cycle.
Suggested doc patch
- Steps 5–11 run in a loop over `TARGET_BRANCHES` — for each branch: - - `git checkout <branch>` and `git pull` to ensure it is up to date - - Apply fix and create PR targeting that branch (from fork if no direct push access) + Steps 5–11 run independently per `TARGET_BRANCH` in isolated workspaces: + - Create branch-specific clone dir (e.g., `/tmp/${REPO_ORG}/${REPO_NAME}-${TARGET_BRANCH}`) + - Checkout target branch in that clone, apply fix, test, and create PR + - Do not reuse working tree state across target branches🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 239 - 258, The documentation section "4.3: Branch loop" currently describes a single cloned workspace iterating over TARGET_BRANCHES and may leak state; update the text to reflect the intended independent-per-branch execution model by removing or clarifying the "Clone once" example and the shared workspace loop (steps 5–11) and instead describe that each TARGET_BRANCH run should perform its own checkout/pull/apply-create-PR cycle in an isolated workspace (or explicitly document the alternative "clone once" mode as a separate, stateful option), referencing the TARGET_BRANCHES loop and the checkout/pull/apply PR steps to ensure readers know to avoid shared-state side effects between branch runs.
174-178:⚠️ Potential issue | 🟠 MajorMake branch naming rule consistent with multi-branch behavior.
This section requires branch-qualified fix names, but the canonical rule later (Line 573) still omits
<target-branch>, which can collide across base branches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 174 - 178, The branch naming guidance is inconsistent: the multi-branch section requires branch-qualified names (example `fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`) but the canonical naming rule later omits the `<target-branch>` component, risking name collisions; update the canonical rule to include `<target-branch>` (or a `<base-branch>` token) so it matches the multi-branch example and ensure all references to the canonical rule and examples use the same format including the branch qualifier.
153-160:⚠️ Potential issue | 🟠 MajorDeduplicate
TARGET_BRANCHESto prevent duplicate PR attempts.If
ACTIVE_RELEASE_BRANCHEScontainsDEFAULT_BRANCH, this creates duplicate targets.Suggested doc patch
if [ "$REPO_TYPE" = "downstream" ]; then TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}") else TARGET_BRANCHES=("$DEFAULT_BRANCH") fi + +# Deduplicate while preserving order +UNIQUE_TARGET_BRANCHES=() +for b in "${TARGET_BRANCHES[@]}"; do + [ -z "$b" ] && continue + [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b") +done +TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 153 - 160, The TARGET_BRANCHES assignment can yield duplicates when ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; update the logic that builds TARGET_BRANCHES (the branch selection block using TARGET_BRANCHES, DEFAULT_BRANCH, and ACTIVE_RELEASE_BRANCHES) to deduplicate entries before using them — for example, after composing the array, filter it to unique values (using an associative "seen" map loop or a stdout-unique transform like sort -u/awk) and reassign the cleaned list back to TARGET_BRANCHES so no duplicate PR targets are attempted.
163-172:⚠️ Potential issue | 🟡 MinorFix markdown fences (MD031/MD040) in changed examples.
These blocks still need language tags and blank-line framing around fenced blocks.
Also applies to: 473-481
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 163 - 172, The fenced code examples lack language tags and surrounding blank lines, which triggers MD031/MD040; update each triple-backtick block (e.g., the block starting with "upstream llm-d/llm-d-inference-scheduler → PR against: main" and the other occurrence around the content referenced as 473-481) to add an appropriate language tag (like ```text or ```bash) immediately after the opening backticks and ensure there is a blank line before and after the fenced block so the markdown linter accepts the examples; apply this change consistently to all similar fenced blocks in the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 439-443: Update the output/reporting docs to reference the new VEX
artifact names and flows instead of the old "already-fixed" wording: replace
references to `artifacts/cve-fixer/fixes/vex-justified-CVE-YYYY-XXXXX.md` and
any mentions of the old “already-fixed” flow with the corresponding new artifact
names (`vex-justified-*`, `vex-needs-human-review-*`) and update the
printed/report messages (e.g., the "✅ CVE-YYYY-XXXXX not present..." line) and
the bullet list for the auto-determined justification cases to explicitly
document writing the correct VEX artifact and leaving closure to a human
reviewer.
---
Outside diff comments:
In `@workflows/cve-fixer/.claude/commands/cve.find.md`:
- Around line 36-46: The docs mention the --ignore-vex flag but never instruct
storing it; update the behavior notes so --ignore-vex is persisted as a boolean
just like --ignore-resolved (e.g., document IGNORE_VEX alongside
IGNORE_RESOLVED), and clarify Step 3’s IGNORE_VEX branch to read that boolean
when filtering results; ensure the instruction still forbids using
AskUserQuestion or presenting options and that component prompting remains
unchanged.
---
Duplicate comments:
In `@workflows/cve-fixer/.claude/commands/cve.find.md`:
- Around line 72-76: Add a blank line before and after each fenced bash code
block that contains the export lines (the block starting with the URL
"https://id.atlassian.com/manage-profile/security/api-tokens" and the block with
"export JIRA_API_TOKEN" / "export JIRA_EMAIL") so the fenced blocks have an
empty line above and below to satisfy MD031; update both occurrences (the block
containing the export lines and the later duplicate block) accordingly.
- Around line 62-78: The auth probe using GET
"${JIRA_BASE_URL}/rest/api/3/myself" is inconsistent: it documents a special
HTTP 403 case and “retry once” but the curl invocation (TEST_RESPONSE) has no
retry behavior and the endpoint may not reliably differentiate 401 vs 403 for
auth issues; update the logic around TEST_RESPONSE to either consolidate 401/403
handling for auth failures or switch to a permission-specific endpoint if you
need to detect permission vs credential problems, and explicitly implement a
single retry (using curl retry flags or a coded retry loop) when handling
network/timeout responses so the documented “retry once” matches actual
behavior; ensure messages reference TEST_RESPONSE and the "/rest/api/3/myself"
probe so maintainers can find and update the probe logic.
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 239-258: The documentation section "4.3: Branch loop" currently
describes a single cloned workspace iterating over TARGET_BRANCHES and may leak
state; update the text to reflect the intended independent-per-branch execution
model by removing or clarifying the "Clone once" example and the shared
workspace loop (steps 5–11) and instead describe that each TARGET_BRANCH run
should perform its own checkout/pull/apply-create-PR cycle in an isolated
workspace (or explicitly document the alternative "clone once" mode as a
separate, stateful option), referencing the TARGET_BRANCHES loop and the
checkout/pull/apply PR steps to ensure readers know to avoid shared-state side
effects between branch runs.
- Around line 174-178: The branch naming guidance is inconsistent: the
multi-branch section requires branch-qualified names (example
`fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`) but the canonical naming
rule later omits the `<target-branch>` component, risking name collisions;
update the canonical rule to include `<target-branch>` (or a `<base-branch>`
token) so it matches the multi-branch example and ensure all references to the
canonical rule and examples use the same format including the branch qualifier.
- Around line 153-160: The TARGET_BRANCHES assignment can yield duplicates when
ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; update the logic that builds
TARGET_BRANCHES (the branch selection block using TARGET_BRANCHES,
DEFAULT_BRANCH, and ACTIVE_RELEASE_BRANCHES) to deduplicate entries before using
them — for example, after composing the array, filter it to unique values (using
an associative "seen" map loop or a stdout-unique transform like sort -u/awk)
and reassign the cleaned list back to TARGET_BRANCHES so no duplicate PR targets
are attempted.
- Around line 163-172: The fenced code examples lack language tags and
surrounding blank lines, which triggers MD031/MD040; update each triple-backtick
block (e.g., the block starting with "upstream llm-d/llm-d-inference-scheduler
→ PR against: main" and the other occurrence around the content referenced as
473-481) to add an appropriate language tag (like ```text or ```bash)
immediately after the opening backticks and ensure there is a blank line before
and after the fenced block so the markdown linter accepts the examples; apply
this change consistently to all similar fenced blocks in the document.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4dda33f7-a5a1-4ee0-a166-ae7ab194816a
📒 Files selected for processing (2)
workflows/cve-fixer/.claude/commands/cve.find.mdworkflows/cve-fixer/.claude/commands/cve.fix.md
When a vulnerable package is not found in application manifests (requirements.txt, go.mod, package.json), check if it comes from the container base image (Dockerfile.konflux FROM line): - Use skopeo list-tags to check for newer base image tags - If newer tag available: update FROM line → real PR with note to verify the fix is included in the new tag before merging - If no newer tag: add Jira comment asking base image team (e.g. AIPCC for quay.io/aipcc/*) to release updated image → no PR created - If no Dockerfile found: fall back to transitive dependency guidance This covers both the general "base image CVE" case and the AIPCC-specific scenario with a single generic mechanism — no AIPCC-specific handling needed. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
workflows/cve-fixer/.claude/commands/cve.fix.md (3)
153-160:⚠️ Potential issue | 🟠 Major
TARGET_BRANCHESstill needs deduplication to prevent duplicate PR attempts.If
DEFAULT_BRANCHappears inACTIVE_RELEASE_BRANCHES(e.g., both are "main"), the array will contain duplicates, causing redundant PR creation or push conflicts.🛡️ Proposed fix to deduplicate TARGET_BRANCHES
if [ "$REPO_TYPE" = "downstream" ]; then TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}") else TARGET_BRANCHES=("$DEFAULT_BRANCH") fi + + # Deduplicate while preserving order + UNIQUE_TARGET_BRANCHES=() + for b in "${TARGET_BRANCHES[@]}"; do + [[ -z "$b" ]] && continue + [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b") + done + TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 153 - 160, TARGET_BRANCHES can contain duplicates when DEFAULT_BRANCH also appears in ACTIVE_RELEASE_BRANCHES; update the assignment logic for TARGET_BRANCHES (the variable) to deduplicate entries after assembling from DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES: build the initial list as currently done, then iterate and push each branch into a new deduped array only if not already present (use a shell associative map or membership check) so TARGET_BRANCHES ends up with unique branch names.
628-628:⚠️ Potential issue | 🟠 MajorBranch naming rule still omits target branch, contradicting multi-branch examples.
Line 177 shows
fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1(includes target branch), but this canonical rule omits it:fix/cve-YYYY-XXXXX-<package-name>-attempt-1. When processing multiple target branches, this creates name collisions.📝 Proposed fix
- - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-attempt-1` + - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-<target-branch>-attempt-1` + - Example: `fix/cve-2025-12345-urllib3-main-attempt-1` + - Example: `fix/cve-2025-12345-urllib3-rhoai-3.4-attempt-1`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` at line 628, Update the canonical branch-naming rule to include the target branch to avoid collisions: change the rule string from "fix/cve-YYYY-XXXXX-<package-name>-attempt-1" to a pattern that includes the target branch (for example matching the existing example "fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1"); ensure any validation or generation logic that uses the symbol/name pattern (branch naming rule, branch creation logic) is updated to accept and produce the target-branch token so multiple target branches produce distinct branch names.
163-172:⚠️ Potential issue | 🟡 MinorFenced code block still missing language tag and blank lines.
The past review marked this as addressed in commit 177e6d0, but the fenced block at line 163 still lacks a language identifier and blank-line framing (MD040/MD031).
📝 Proposed fix
+ - ``` + ```text upstream llm-d/llm-d-inference-scheduler → PR against: main midstream opendatahub-io/llm-d-inference-scheduler → PR against: main downstream red-hat-data-services/llm-d-inference-scheduler → PRs against: - main - rhoai-3.3 - rhoai-3.4 - rhoai-3.4-ea.1 - rhoai-3.4-ea.2 ``` +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 163 - 172, The fenced code block containing the lines starting with "upstream llm-d/llm-d-inference-scheduler" is missing a language tag and surrounding blank lines; update the block to use a language identifier (e.g., ```text) and ensure there's a blank line before the opening ```text and a blank line after the closing ``` so the block complies with MD040/MD031 rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 511-516: The curl payload is built by directly interpolating
${COMMENT}, creating unsafe JSON; instead generate a properly escaped JSON body
(for example using jq or another JSON encoder) and pipe it into curl so you
don't interpolate ${COMMENT} into the -d string. Replace the inline -d
"{\"body\": \"${COMMENT}\"}" usage in the block that constructs AUTH and calls
curl (refer to variables AUTH, COMMENT, JIRA_BASE_URL, JIRA_KEY) with a safe
encoder step such as: create the JSON via jq -n --arg body "$COMMENT"
'{body:$body}' (or equivalent), then pass it to curl with --data `@-` (or -d `@-`)
so the comment is JSON-escaped and newline/quote characters cannot break the
payload.
- Around line 423-424: The pipeline currently assigns AVAILABLE_TAGS from the
skopeo command without checking failure; capture skopeo's exit status (or run it
in a subshell storing both output and exit code) when populating AVAILABLE_TAGS,
and if skopeo fails (non-zero exit) log a clear error and fail/exit the job (or
set a distinct error flag) instead of continuing with an empty AVAILABLE_TAGS;
update the code that references AVAILABLE_TAGS to rely on this failure handling
so you don't treat an empty list from a failed skopeo call as "no newer tags
available."
- Around line 246-258: Split the combined example into two clearly labeled
scenarios so operators don't confuse repositories and steps: update the block in
workflows/cve-fixer/.claude/commands/cve.fix.md to (1) label and describe
"Scenario 1: User has write access" (or "Push directly to origin") with the
/tmp/red-hat-data-services/llm-d-inference-scheduler clone and its five PR
targets (main, rhoai-3.3, rhoai-3.4, rhoai-3.4-ea.1, rhoai-3.4-ea.2), and (2)
label and describe "Scenario 2: User lacks write access (upstream repo)" (or
"Push to fork") with the /tmp/llm-d/llm-d-inference-scheduler clone and the
single fork-based PR example (head: <user>/llm-d-inference-scheduler:fix/... →
base: main); ensure each scenario is wrapped in its own fenced bash block and
update the inline comments to "Push directly to origin, create 5 PRs" and "Push
to fork, create 1 PR" respectively.
- Around line 483-485: The script calls an undefined helper version_is_safe when
evaluating PACKAGE_VERSION against CVE_AFFECTED_RANGE which will crash at
runtime; add a version_is_safe function (or replace the call) that accepts
(pkg_version, affected_range) and returns success (0) when the package version
is not in the affected range and failure (non‑zero) when it is, then keep the
existing logic that sets VEX_JUSTIFICATION="Vulnerable Code not Present" and
VEX_EVIDENCE using PACKAGE, PACKAGE_VERSION and CVE_AFFECTED_RANGE; reference
the symbol version_is_safe and ensure it is available before the conditional
that uses PACKAGE_VERSION and CVE_AFFECTED_RANGE so the branch executes
correctly.
- Around line 427-428: The awk comparison '$0 > cur' used to compute NEWER_TAGS
from AVAILABLE_TAGS compares strings lexicographically and breaks for semantic
versions; replace that step with a version-aware filter: sort AVAILABLE_TAGS
with sort -V and then select only entries that are semantically greater than
CURRENT_TAG (or run a short Python snippet that uses packaging.version.parse or
similar to compare each tag to CURRENT_TAG) so NEWER_TAGS contains only tags
with higher semantic versions; update the code that references NEWER_TAGS,
AVAILABLE_TAGS and CURRENT_TAG accordingly.
- Around line 488-490: The existing conditional that uses SCAN_OUTPUT and
PACKAGE in the elif branch wrongly checks for "No vulnerabilities found" and
will never match the intended scenario; update the conditional in the
conditional block that sets VEX_JUSTIFICATION/VEX_EVIDENCE to instead detect a
govulncheck informational vulnerability with no call stacks by using: check that
SCAN_OUTPUT contains "Vulnerability #" and the "${PACKAGE}" string and that it
does NOT contain "Call stacks" (e.g., replace the elif with: elif echo
"$SCAN_OUTPUT" | grep -q "Vulnerability #" && echo "$SCAN_OUTPUT" | grep -q
"${PACKAGE}" && ! echo "$SCAN_OUTPUT" | grep -q "Call stacks"; then), or
alternatively change the scan invocation to emit JSON (-json) and parse the JSON
output for findings without call traces for more robust detection.
---
Duplicate comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 153-160: TARGET_BRANCHES can contain duplicates when
DEFAULT_BRANCH also appears in ACTIVE_RELEASE_BRANCHES; update the assignment
logic for TARGET_BRANCHES (the variable) to deduplicate entries after assembling
from DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES: build the initial list as
currently done, then iterate and push each branch into a new deduped array only
if not already present (use a shell associative map or membership check) so
TARGET_BRANCHES ends up with unique branch names.
- Line 628: Update the canonical branch-naming rule to include the target branch
to avoid collisions: change the rule string from
"fix/cve-YYYY-XXXXX-<package-name>-attempt-1" to a pattern that includes the
target branch (for example matching the existing example
"fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1"); ensure any validation or
generation logic that uses the symbol/name pattern (branch naming rule, branch
creation logic) is updated to accept and produce the target-branch token so
multiple target branches produce distinct branch names.
- Around line 163-172: The fenced code block containing the lines starting with
"upstream llm-d/llm-d-inference-scheduler" is missing a language tag and
surrounding blank lines; update the block to use a language identifier (e.g.,
```text) and ensure there's a blank line before the opening ```text and a blank
line after the closing ``` so the block complies with MD040/MD031 rules.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d3979fd-07c5-4300-9082-c28e67feab4e
📒 Files selected for processing (1)
workflows/cve-fixer/.claude/commands/cve.fix.md
cve.fix.md: - Deduplicate TARGET_BRANCHES to prevent processing DEFAULT_BRANCH twice - Fix branch naming: include target branch in fix branch name to avoid collisions (fix/cve-...-urllib3-rhoai-3.4-attempt-1) - Replace shared-dir branch loop with isolated git worktree per branch to prevent cross-branch state contamination - Fix version_is_safe: replace undefined function with sort -V comparison - Fix govulncheck condition: check for Informational section (not "No vulnerabilities found") for execute path detection - Fix unsafe JSON in curl: use jq -n --arg to safely encode Jira comment - Add skopeo error handling: check exit code, warn and skip on failure - Fix semantic version comparison: use sort -V + awk instead of awk string compare - Clarify fork vs direct-push as two separate repo examples (not sequential) cve.find.md: - Remove HTTP 403 case: /rest/api/3/myself only returns 401 for all auth failures - Add retry loop: attempt auth test call twice before giving up on network timeout Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Add VEX and base image artifact types to Output section: vex-justified-*, vex-needs-human-review-*, base-image-pending-* so operators know what artifacts to expect for each scenario - Fix MD031 markdown violations in cve.find.md: add blank lines before fenced code blocks in Step 3a and 3b Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Remove llm-d specific example from Step 3.3 — workflow should be component-agnostic and read all repos/branches from the mapping file - Simplify branch logic: create PRs against default_branch + ALL active_release_branches for every repo, regardless of repo_type (mapping file is the source of truth, not repo_type heuristics) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
bobbravo2
approved these changes
Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Updates
cve.fixto target branches differently based onrepo_type:default_branchonly (e.g.main)default_branch+ each branch inactive_release_branchesEach branch gets its own independent clone → fix → test → PR cycle. Branches are never combined into a single PR.
Example for
red-hat-data-services/llm-d-inference-schedulerwith active branchesrhoai-3.3,rhoai-3.4,rhoai-3.4-ea.1,rhoai-3.4-ea.2:mainrhoai-3.3rhoai-3.4rhoai-3.4-ea.1rhoai-3.4-ea.2No mapping file changes required — logic is derived from existing
repo_type,default_branch, andactive_release_branchesfields.Test plan
/cve.fix llm-dwith an inference-scheduler CVE — verify upstream gets 1 PR (main only) and downstream gets 5 PRs (main + 4 release branches)fix/cve-YYYY-XXXXX-urllib3-rhoai-3.4-attempt-1)🤖 Generated with Claude Code