Skip to content

Commit eba6f74

Browse files
vmrh21claude
andcommitted
fix: address all CodeRabbit review comments on PR 101
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>
1 parent 60d6f70 commit eba6f74

2 files changed

Lines changed: 92 additions & 47 deletions

File tree

workflows/cve-fixer/.claude/commands/cve.find.md

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,23 +59,30 @@ Report: artifacts/cve-fixer/find/cve-issues-20260226-145018.md
5959
```bash
6060
JIRA_BASE_URL="https://redhat.atlassian.net"
6161
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
62-
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
63-
--connect-timeout 10 --max-time 15 \
64-
-H "Authorization: Basic ${AUTH}" \
65-
-H "Content-Type: application/json" \
66-
"${JIRA_BASE_URL}/rest/api/3/myself")
62+
63+
# Retry once on network failure (curl exit code 000 = timeout/no response)
64+
for ATTEMPT in 1 2; do
65+
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
66+
--connect-timeout 10 --max-time 15 \
67+
-H "Authorization: Basic ${AUTH}" \
68+
-H "Content-Type: application/json" \
69+
"${JIRA_BASE_URL}/rest/api/3/myself")
70+
[ "$TEST_RESPONSE" != "000" ] && break
71+
echo "⚠️ Network timeout on attempt ${ATTEMPT}, retrying..."
72+
sleep 3
73+
done
6774
```
6875

6976
- **HTTP 200** → credentials valid, proceed
70-
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
77+
- **HTTP 401** → credentials missing or invalid. Note: `/rest/api/3/myself` returns 401 for all authentication failures — there is no separate 403 for this endpoint. Only now inform the user:
7178
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
7279
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
80+
7381
```bash
7482
export JIRA_API_TOKEN="your-token-here"
7583
export JIRA_EMAIL="your-email@redhat.com"
7684
```
77-
- **HTTP 403** → token valid but insufficient permissions — inform user
78-
- **Other / timeout** → network issue — inform user and retry once
85+
- **HTTP 000 after retry** → persistent network issue — inform user and stop
7986

8087
**Do NOT pre-check env vars with `[ -z "$JIRA_API_TOKEN" ]` and stop.** The variables may be available to the API call even if not visible to the shell check (e.g. Ambient secrets injection).
8188

workflows/cve-fixer/.claude/commands/cve.fix.md

Lines changed: 77 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,10 @@ Summary:
151151
- If `active_release_branches` is empty, target `default_branch` only
152152

153153
```bash
154-
# Determine target branches per repo
154+
# Determine target branches per repo — deduplicate to avoid processing DEFAULT_BRANCH twice
155155
if [ "$REPO_TYPE" = "downstream" ]; then
156-
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
156+
ALL_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
157+
TARGET_BRANCHES=($(printf '%s\n' "${ALL_BRANCHES[@]}" | awk '!seen[$0]++'))
157158
else
158159
TARGET_BRANCHES=("$DEFAULT_BRANCH")
159160
fi
@@ -174,7 +175,9 @@ Summary:
174175
**Multi-repo + multi-branch strategy**:
175176
- Fix upstream repos first, then midstream, then downstream
176177
- For downstream: Steps 4 through 11 repeat for EACH branch independently
177-
- Each branch produces its own PR with its own fix branch (e.g., `fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`)
178+
- Each branch produces its own fix branch including the target branch name to avoid collisions:
179+
`fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-1`
180+
e.g. `fix/cve-2025-66418-urllib3-rhoai-3.4-attempt-1`
178181
- Never combine fixes for multiple branches into a single PR
179182

180183
4. **Clone or Use Existing Repository**
@@ -236,25 +239,41 @@ Summary:
236239

237240
This is common for upstream repos (`llm-d/*`, `eval-hub/*`, `trustyai-explainability/*`) where the user doesn't have direct write access.
238241

239-
**4.3: Branch loop**
242+
**4.3: Branch loop — isolated worktree per branch**
240243

241-
Steps 5–11 run in a loop over `TARGET_BRANCHES` — for each branch:
242-
- `git checkout <branch>` and `git pull` to ensure it is up to date
243-
- Apply fix and create PR targeting that branch (from fork if no direct push access)
244+
To prevent cross-branch contamination (uncommitted files, lockfile drift, tool artifacts),
245+
use a separate worktree for each target branch rather than switching branches in the same dir:
244246

245-
**Example for downstream with 4 active branches:**
246247
```bash
247-
# Clone once: /tmp/red-hat-data-services/llm-d-inference-scheduler
248-
# User has write access → push directly, 5 PRs:
249-
# PR 1 → base: main
250-
# PR 2 → base: rhoai-3.3
251-
# PR 3 → base: rhoai-3.4
252-
# PR 4 → base: rhoai-3.4-ea.1
253-
# PR 5 → base: rhoai-3.4-ea.2
254-
255-
# Clone once: /tmp/llm-d/llm-d-inference-scheduler
256-
# User has NO write access → fork + 1 PR:
257-
# PR 1 → head: <user>/llm-d-inference-scheduler:fix/... → base: main
248+
for TARGET_BRANCH in "${TARGET_BRANCHES[@]}"; do
249+
BRANCH_DIR="/tmp/${REPO_ORG}/${REPO_NAME}-${TARGET_BRANCH//\//-}"
250+
git -C "$REPO_DIR" worktree add "$BRANCH_DIR" "$TARGET_BRANCH"
251+
cd "$BRANCH_DIR"
252+
git pull origin "$TARGET_BRANCH"
253+
# Steps 5–11 run here — fix, test, push, PR
254+
FIX_BRANCH="fix/cve-${CVE_ID}-${PACKAGE}-${TARGET_BRANCH//\//-}-attempt-1"
255+
git -C "$REPO_DIR" worktree remove "$BRANCH_DIR" --force # cleanup after PR
256+
done
257+
```
258+
259+
Each worktree is fully isolated — no shared index or working tree state between branches.
260+
261+
**Example output for downstream with write access (5 separate PRs):**
262+
263+
```
264+
Repo: red-hat-data-services/llm-d-inference-scheduler
265+
├── worktree: main → fix branch: fix/cve-2025-66418-urllib3-main-attempt-1
266+
├── worktree: rhoai-3.3 → fix branch: fix/cve-2025-66418-urllib3-rhoai-3.3-attempt-1
267+
├── worktree: rhoai-3.4 → fix branch: fix/cve-2025-66418-urllib3-rhoai-3.4-attempt-1
268+
├── worktree: rhoai-3.4-ea.1 → fix branch: fix/cve-2025-66418-urllib3-rhoai-3.4-ea.1-attempt-1
269+
└── worktree: rhoai-3.4-ea.2 → fix branch: fix/cve-2025-66418-urllib3-rhoai-3.4-ea.2-attempt-1
270+
```
271+
272+
**Example output for upstream with no write access (fork, 1 PR):**
273+
274+
```
275+
Repo: llm-d/llm-d-inference-scheduler (no write access → fork)
276+
└── worktree: main → push to <user>/llm-d-inference-scheduler → PR targeting llm-d/... main
258277
```
259278

260279
4.5. **Load Global Fix Guidance from `.cve-fix/` Folder**
@@ -420,11 +439,18 @@ Summary:
420439
echo "Checking for newer tags of: $IMAGE_REF (current: $CURRENT_TAG)"
421440

422441
# List available tags (works for quay.io, registry.access.redhat.com, etc.)
423-
AVAILABLE_TAGS=$(skopeo list-tags "docker://${IMAGE_REF}" 2>/dev/null | \
424-
jq -r '.Tags[]' | sort -V)
442+
SKOPEO_OUTPUT=$(skopeo list-tags "docker://${IMAGE_REF}" 2>&1)
443+
SKOPEO_EXIT=$?
444+
if [ $SKOPEO_EXIT -ne 0 ]; then
445+
echo "⚠️ skopeo list-tags failed for ${IMAGE_REF}: ${SKOPEO_OUTPUT}"
446+
echo "⚠️ Skipping base image update check — treating as no newer tag available"
447+
continue
448+
fi
449+
AVAILABLE_TAGS=$(echo "$SKOPEO_OUTPUT" | jq -r '.Tags[]' 2>/dev/null | sort -V)
425450

426-
# Find tags newer than current
427-
NEWER_TAGS=$(echo "$AVAILABLE_TAGS" | awk -v cur="$CURRENT_TAG" '$0 > cur')
451+
# Find tags newer than current using sort -V (semantic version aware)
452+
NEWER_TAGS=$(printf '%s\n' "$AVAILABLE_TAGS" | sort -V | \
453+
awk -v cur="$CURRENT_TAG" 'found{print} $0==cur{found=1}')
428454
done
429455
```
430456

@@ -479,15 +505,23 @@ Summary:
479505
VEX_JUSTIFICATION="Component not Present"
480506
VEX_EVIDENCE="Package '${PACKAGE}' not found in any dependency manifest (requirements.txt, go.mod, package.json)"
481507
482-
# Check 2: Vulnerable Code not Present (package exists at safe version)
483-
elif [ -n "$PACKAGE_VERSION" ] && version_is_safe "$PACKAGE_VERSION" "$CVE_AFFECTED_RANGE"; then
484-
VEX_JUSTIFICATION="Vulnerable Code not Present"
485-
VEX_EVIDENCE="Package '${PACKAGE}' present at version ${PACKAGE_VERSION} which is not in the affected range"
508+
# Check 2: Vulnerable Code not Present — package present but at a non-vulnerable version.
509+
# PACKAGE_VERSION is set during the manifest check in Step 5.2.1.
510+
# Compare using sort -V: if the installed version sorts after the last affected version, it is safe.
511+
elif [ -n "$PACKAGE_VERSION" ] && [ -n "$CVE_FIXED_VERSION" ]; then
512+
HIGHER=$(printf '%s\n' "$PACKAGE_VERSION" "$CVE_FIXED_VERSION" | sort -V | tail -1)
513+
if [ "$HIGHER" = "$PACKAGE_VERSION" ] && [ "$PACKAGE_VERSION" != "$CVE_FIXED_VERSION" ]; then
514+
VEX_JUSTIFICATION="Vulnerable Code not Present"
515+
VEX_EVIDENCE="Package '${PACKAGE}' present at version ${PACKAGE_VERSION} which is >= fixed version ${CVE_FIXED_VERSION}"
516+
fi
486517
487-
# Check 3: Vulnerable Code not in Execute Path (Go govulncheck call graph)
488-
elif echo "$SCAN_OUTPUT" | grep -q "No vulnerabilities found" && echo "$SCAN_OUTPUT" | grep -q "${PACKAGE}"; then
518+
# Check 3: Vulnerable Code not in Execute Path (Go only — govulncheck call graph analysis)
519+
# govulncheck prints an "Informational" block when a module is in the dep tree but the
520+
# vulnerable symbol is not reachable. Look for the package name in Informational output.
521+
elif echo "$SCAN_OUTPUT" | grep -q "Informational" && \
522+
echo "$SCAN_OUTPUT" | grep -A5 "Informational" | grep -qi "${PACKAGE}"; then
489523
VEX_JUSTIFICATION="Vulnerable Code not in Execute Path"
490-
VEX_EVIDENCE="govulncheck found module ${PACKAGE} in dependency tree but confirmed vulnerable symbol is not called in code path"
524+
VEX_EVIDENCE="govulncheck found module ${PACKAGE} in dependency tree but reported it as Informational — vulnerable symbol is not called in the code path"
491525
fi
492526
```
493527
@@ -498,21 +532,25 @@ Summary:
498532
- Print: "✅ CVE-YYYY-XXXXX not present in [repo]. VEX justification added to [JIRA-KEY]: [justification]"
499533
500534
```bash
501-
# Add Jira comment with VEX justification
502-
COMMENT="*VEX Justification (auto-detected by CVE fixer workflow)*\n\n"
503-
COMMENT+="*Justification:* ${VEX_JUSTIFICATION}\n"
504-
COMMENT+="*Evidence:* ${VEX_EVIDENCE}\n"
505-
COMMENT+="*Repository:* ${REPO_FULL}\n"
506-
COMMENT+="*Branch:* ${TARGET_BRANCH}\n"
507-
COMMENT+="*Scan date:* $(date -u +%Y-%m-%dT%H:%M:%SZ)\n\n"
508-
COMMENT+="This issue can be closed as 'Not a Bug / ${VEX_JUSTIFICATION}' if the above evidence is satisfactory."
535+
# Add Jira comment with VEX justification — use jq to safely build JSON (avoids injection)
536+
COMMENT_TEXT="VEX Justification (auto-detected by CVE fixer workflow)
537+
538+
Justification: ${VEX_JUSTIFICATION}
539+
Evidence: ${VEX_EVIDENCE}
540+
Repository: ${REPO_FULL}
541+
Branch: ${TARGET_BRANCH}
542+
Scan date: $(date -u +%Y-%m-%dT%H:%M:%SZ)
543+
544+
This issue can be closed as 'Not a Bug / ${VEX_JUSTIFICATION}' if the above evidence is satisfactory."
545+
546+
COMMENT_JSON=$(jq -n --arg body "$COMMENT_TEXT" '{"body": $body}')
509547
510548
# Post comment via Jira API
511549
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
512550
curl -s -X POST \
513551
-H "Authorization: Basic ${AUTH}" \
514552
-H "Content-Type: application/json" \
515-
-d "{\"body\": \"${COMMENT}\"}" \
553+
-d "$COMMENT_JSON" \
516554
"${JIRA_BASE_URL}/rest/api/3/issue/${JIRA_KEY}/comment"
517555
```
518556

0 commit comments

Comments
 (0)