Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 27 additions & 46 deletions workflows/cve-fixer/.claude/commands/cve.find.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,59 +51,40 @@ Report: artifacts/cve-fixer/find/cve-issues-20260226-145018.md
/cve.find "AI Evaluations" trustyai-ragas
```

2. **Check JIRA API Token (REQUIRED - User Setup)**
- **This is the ONLY thing the user must configure manually before proceeding**
2. **Verify Jira Access**

- Check if JIRA_API_TOKEN and JIRA_EMAIL are set:
```bash
if [ -z "$JIRA_API_TOKEN" ]; then
echo "ERROR: JIRA_API_TOKEN is not set"
else
echo "JIRA_API_TOKEN is set"
fi
if [ -z "$JIRA_EMAIL" ]; then
echo "ERROR: JIRA_EMAIL is not set"
else
echo "JIRA_EMAIL is set"
fi
```

- **If JIRA_API_TOKEN or JIRA_EMAIL is NOT set or empty**:
- **STOP here and inform the user they need to set up both variables first**
- Provide instructions:

**Step 1: Generate a Jira API Token**
- Go to https://id.atlassian.com/manage-profile/security/api-tokens
- Click "Create API token"
- Give it a name and copy the token

**Step 2: Export both environment variables**
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
To make it persistent, add to `~/.bashrc` or `~/.zshrc`:
```bash
echo 'export JIRA_API_TOKEN="your-token-here"' >> ~/.bashrc
echo 'export JIRA_EMAIL="your-email@redhat.com"' >> ~/.bashrc
source ~/.bashrc
```

- **After user sets the variables, verify they're exported correctly** using the check script above
- Should output: "JIRA_API_TOKEN is set" and "JIRA_EMAIL is set"

- **Only proceed to the next steps if both JIRA_API_TOKEN and JIRA_EMAIL are set**
Secrets may be injected by the Ambient session, a secrets manager, or an MCP server — do NOT rely solely on bash env var checks. Instead, attempt a lightweight test API call and let the response determine whether credentials are available.

```bash
JIRA_BASE_URL="https://redhat.atlassian.net"
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
--connect-timeout 10 --max-time 15 \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
"${JIRA_BASE_URL}/rest/api/3/myself")
```

- **HTTP 200** → credentials valid, proceed
- **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"
```
Comment on lines +69 to +75
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 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).

- **HTTP 403** → token valid but insufficient permissions — inform user
- **Other / timeout** → network issue — inform user and retry once
Comment on lines +58 to +77
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Implement retry logic for timeout handling.

Line 77 documents that timeout/network errors should "retry once," but the test API call in lines 58-66 lacks retry flags. This creates a documentation-implementation gap.

🔄 Proposed fix to add retry logic
 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")
🧰 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 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.


**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).
Comment on lines +58 to +79
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

What is the recommended Jira REST API v3 endpoint for testing authentication credentials, and what HTTP status codes does it return for valid credentials, invalid credentials, and insufficient permissions?

💡 Result:

The recommended Jira REST API v3 endpoint for testing authentication credentials is GET /rest/api/3/myself. This endpoint returns details for the current authenticated user and requires only "Permission to access Jira". HTTP status codes: - Valid credentials: 200 OK - Invalid credentials: 401 Unauthorized - Insufficient permissions: This endpoint has minimal permissions required (access Jira), so insufficient permissions would also return 401 Unauthorized if the user lacks basic access. Alternative endpoint for permission checks: GET /rest/api/3/mypermissions, which returns the user's permissions and can be used to verify more granular access. It returns 200 OK for valid authenticated users. Note: There is no dedicated "health check" or ping endpoint solely for auth testing in official docs; /myself is commonly used as it confirms successful authentication and basic access.

Citations:


Reconsider the HTTP 403 status code—/rest/api/3/myself returns 401 for all authentication failures.

The endpoint is correct for credentials validation. However, the /rest/api/3/myself endpoint returns HTTP 401 for both missing/invalid credentials and insufficient permissions. The current code handles HTTP 403 as a separate "insufficient permissions" case, but this endpoint does not return 403 per official Jira REST API v3 documentation.

Consolidate error handling to treat 401 responses uniformly, or verify that a different endpoint is intended for the 403 case.

🧰 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 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).


3. **Query Jira for CVE Issues**

a. Set up variables:
a. Set up variables (AUTH already set from Step 2):
```bash
COMPONENT_NAME="[from step 1]"
JIRA_BASE_URL="https://redhat.atlassian.net"
JIRA_EMAIL="${JIRA_EMAIL}"
JIRA_API_TOKEN="${JIRA_API_TOKEN}"
# Jira Cloud uses Basic Auth: base64(email:api-token)
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
# AUTH already constructed in Step 2 — reuse it
Comment on lines +83 to +87
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add blank line before fenced code block for markdown compliance.

Markdownlint reports a missing blank line before the code block at line 84 per MD031.

📝 Proposed formatting fix
 3. **Query Jira for CVE Issues**
 
    a. Set up variables (AUTH already set from Step 2):
+
    ```bash
    COMPONENT_NAME="[from step 1]"
    JIRA_BASE_URL="https://redhat.atlassian.net"
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 84-84: 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 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.

```

b. Construct JQL query and execute API call:
Expand Down
140 changes: 114 additions & 26 deletions workflows/cve-fixer/.claude/commands/cve.fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,34 +140,122 @@ Summary:
- The CVE scan in Step 5 acts as the safety net — it will skip repos where the CVE doesn't exist
- Log a warning: "⚠️ Could not extract container from summary — processing all component repos"

**3.3: For each target repo, gather:**
- Repository name (e.g., "opendatahub-io/odh-dashboard")
- Default branch (e.g., "main")
- Active release branches (e.g., ["v2.29.0-fixes", "v2.28.0-fixes", "rhoai-3.0"])
- Primary target branch for CVE fixes (from `cve_fix_workflow.primary_target`)
- Backport targets from `cve_fix_workflow`
- Repository type (monorepo vs single package)
- Repo type: upstream or downstream (from `repo_type` field, defaults to upstream if absent)

**Multi-repo strategy**: When a container chain has upstream, midstream, and downstream repos:
- Fix upstream first, then apply the same fix to midstream and downstream
- Each repo gets its own clone, branch, PR, and verification cycle
- Steps 4 through 11 are repeated for EACH repository in the list
**3.3: For each target repo, determine target branches:**

The branches to fix depend on `repo_type`:

- **`upstream` or `midstream`**: target `default_branch` only (e.g., `main`)
- Fixes flow forward from there — no backports needed at this level
- **`downstream`**: target `default_branch` AND every branch in `active_release_branches`
- Each branch gets its own separate PR — never combine multiple branches in one PR
- If `active_release_branches` is empty, target `default_branch` only

```bash
# Determine target branches per repo
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
Comment on lines +155 to +159
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

TARGET_BRANCHES needs deduplication to avoid duplicate PR attempts.

If ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH, this creates duplicate targets and can trigger redundant PR creation/push conflicts.

Suggested doc fix
-  if [ "$REPO_TYPE" = "downstream" ]; then
-    TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
-  else
-    TARGET_BRANCHES=("$DEFAULT_BRANCH")
-  fi
+  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[@]}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
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 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.

```

**Example for llm-d inference-scheduler:**
```
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
```

**Multi-repo + multi-branch strategy**:
- Fix upstream repos first, then midstream, then downstream
- For downstream: Steps 4 through 11 repeat for EACH branch independently
- Each branch produces its own PR with its own fix branch (e.g., `fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`)
- Never combine fixes for multiple branches into a single PR

Comment on lines +174 to 179
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Branch naming rule is still inconsistent and can collide across target branches.

This section says branch names include the target branch, but the canonical naming rule later still omits it (fix/cve-...-<package>-attempt-1). That can overwrite/reuse the same fix branch name for multiple base branches.

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 - 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.

4. **Clone or Use Existing Repository**
- Always use `/tmp` for repository operations with unique dirs per repo
- For each repo, extract `REPO_ORG` and `REPO_NAME` from `github_url`, set `REPO_DIR="/tmp/${REPO_ORG}/${REPO_NAME}"`
- If `$REPO_DIR` exists, `cd` into it; otherwise `mkdir -p "/tmp/${REPO_ORG}"`, `git clone` the URL, `cd` in, and `git checkout` the target branch
- **Configure git credentials** immediately after clone (needed for push):
1. `gh auth setup-git` (if `gh` is authenticated)
2. Else set `credential.helper` using `$GITHUB_TOKEN` or `$GH_TOKEN`
3. Else switch remote to SSH if `~/.ssh/id_rsa` or `id_ed25519` exists
4. Else warn: no credentials configured, push will fail
- **Multi-repo example**:
```bash
# Upstream: /tmp/opendatahub-io/models-as-a-service (branch: main)
# Downstream: /tmp/red-hat-data-services/models-as-a-service (branch: rhoai-3.0)
```

**4.0: GitHub Authentication Setup**

Use a **Classic Personal Access Token (PAT)** with `repo` scope. This is the most reliable option across all repo types (upstream external orgs, midstream ODH, downstream RHDS):

```bash
# Recommended: Classic PAT with repo scope
# Generate at: https://github.com/settings/tokens (classic)
# Required scopes: repo (full control of private repositories)
export GITHUB_TOKEN="ghp_your_classic_pat_here"
gh auth login --with-token <<< "$GITHUB_TOKEN"
```

**Why Classic PAT?**
- Fine-grained PATs require org-level approval for `red-hat-data-services`, `opendatahub-io` etc.
- The Ambient Code GitHub App only covers repos where it is installed — it will NOT work for upstream repos like `llm-d/*`, `eval-hub/*`, `trustyai-explainability/*`
- Classic PAT with `repo` scope works immediately for any repo you are a member of

**4.1: Clone and detect write access**

For each repo:
```bash
REPO_ORG=$(echo "$GITHUB_URL" | sed 's|https://github.com/||' | cut -d/ -f1)
REPO_NAME=$(echo "$GITHUB_URL" | sed 's|https://github.com/||' | cut -d/ -f2)
REPO_FULL="${REPO_ORG}/${REPO_NAME}"
REPO_DIR="/tmp/${REPO_ORG}/${REPO_NAME}"

# Clone the repo
mkdir -p "/tmp/${REPO_ORG}"
gh repo clone "$REPO_FULL" "$REPO_DIR" -- --depth=1 2>/dev/null || \
git clone "https://github.com/${REPO_FULL}.git" "$REPO_DIR"

# Check if user has write (push) access
PUSH_ACCESS=$(gh api repos/${REPO_FULL} --jq '.permissions.push' 2>/dev/null)
```

**4.2: Fork fallback if no write access**

If `PUSH_ACCESS` is `false` or the push fails:
```bash
# Create a fork under the authenticated user's account
gh repo fork "$REPO_FULL" --clone=false

FORK_USER=$(gh api user --jq '.login')
FORK_REPO="${FORK_USER}/${REPO_NAME}"

# Add fork as a remote
cd "$REPO_DIR"
git remote add fork "https://github.com/${FORK_REPO}.git"

# Push fix branch to fork, PR targets the original repo
git push fork "$FIX_BRANCH"
gh pr create --repo "$REPO_FULL" --head "${FORK_USER}:${FIX_BRANCH}" \
--base "$TARGET_BRANCH" --title "..." --body "..."
```

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

**4.3: Branch loop**

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)

**Example for downstream with 4 active branches:**
```bash
# Clone once: /tmp/red-hat-data-services/llm-d-inference-scheduler
# User has write access → push directly, 5 PRs:
# PR 1 → base: main
# PR 2 → base: rhoai-3.3
# PR 3 → base: rhoai-3.4
# PR 4 → base: rhoai-3.4-ea.1
# PR 5 → base: rhoai-3.4-ea.2

# Clone once: /tmp/llm-d/llm-d-inference-scheduler
# User has NO write access → fork + 1 PR:
# PR 1 → head: <user>/llm-d-inference-scheduler:fix/... → base: main
```

4.5. **Load Global Fix Guidance from `.cve-fix/` Folder**
- Runs ONCE after all repos are cloned, BEFORE any fixes. Builds a global knowledge base from `.cve-fix/` folders across all cloned repos.
Expand Down
Loading