Skip to content

Commit 1b8f24f

Browse files
vmrh21claude
andcommitted
fix: address all CodeRabbit comments on PR 104
cve.fix.md: - Search 3 (broad package name): document false-positive trade-off so workflow uses judgment before skipping on common package names - Post-fix scan: check exit code before grepping output — if scanner failed to run, treat as inconclusive and block PR creation - Automerge: add non-empty PR_URL guard before calling gh pr merge - Jira comment: add MCP-first note (use MCP tool if available) onboard.md: - Jira validation: use MCP-first pattern (select:mcp__mcp-atlassian__) with curl fallback; also fix shell injection in JQL encoding - Python heredoc: pass all variables as argv[] instead of interpolating shell vars into Python code — prevents injection from component names containing quotes, backslashes, or newlines Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 7a88384 commit 1b8f24f

2 files changed

Lines changed: 53 additions & 18 deletions

File tree

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -633,8 +633,11 @@ This issue can be closed as 'Not a Bug / ${VEX_JUSTIFICATION}' if the above evid
633633
--jq '[.[] | select(.author.login | test("dependabot|renovate|renovate-bot"; "i"))] | .[0]' \
634634
2>/dev/null)
635635
636-
# If no bot PR, still check for any open PR bumping this package
637-
# (a human may have already opened a fix PR without mentioning the CVE)
636+
# Search 3: broad package name search to catch human-opened PRs.
637+
# Note: for common package names (e.g. "requests", "yaml") this may
638+
# produce false positives. If the matched PR title does not appear
639+
# related to the CVE (e.g. it's a feature PR that happens to mention
640+
# the package), do NOT skip — use judgment before skipping.
638641
if [ -z "$EXISTING_PR" ] || [ "$EXISTING_PR" = "null" ]; then
639642
EXISTING_PR=$(gh pr list --repo "$REPO_FULL" --state open --base "$TARGET_BRANCH" \
640643
--search "$PACKAGE" --json number,title,url --jq '.[0]' 2>/dev/null)
@@ -1096,10 +1099,18 @@ This issue can be closed as 'Not a Bug / ${VEX_JUSTIFICATION}' if the above evid
10961099
POST_SCAN_OUTPUT=$(npm audit --json 2>&1)
10971100
```
10981101

1099-
**Check result:**
1102+
**Check result — verify scan ran successfully before trusting output:**
11001103

11011104
```bash
1102-
if echo "$POST_SCAN_OUTPUT" | grep -q "$CVE_ID"; then
1105+
SCAN_EXIT_CODE=$?
1106+
1107+
# If scanner failed (tool missing, network issue, toolchain error), treat as
1108+
# inconclusive — do NOT assume CVE is fixed. Log and skip PR creation.
1109+
if [ $SCAN_EXIT_CODE -ne 0 ] && [ -z "$POST_SCAN_OUTPUT" ]; then
1110+
echo "⚠️ Post-fix scan failed to run (exit code ${SCAN_EXIT_CODE}). Cannot verify fix."
1111+
echo "⚠️ Skipping PR creation for safety — manual verification required."
1112+
CVE_STILL_PRESENT=true # treat as still present to block PR creation
1113+
elif echo "$POST_SCAN_OUTPUT" | grep -q "$CVE_ID"; then
11031114
CVE_STILL_PRESENT=true
11041115
else
11051116
CVE_STILL_PRESENT=false
@@ -1110,7 +1121,10 @@ This issue can be closed as 'Not a Bug / ${VEX_JUSTIFICATION}' if the above evid
11101121

11111122
**If CVE is still present** (`CVE_STILL_PRESENT=true`) → **do NOT create a PR**:
11121123
- Print: "❌ CVE-YYYY-XXXXX still detected after fix attempt in [repo] ([branch]). Fix was insufficient."
1113-
- Add a Jira comment:
1124+
- Add a Jira comment using MCP if available, otherwise curl:
1125+
1126+
If `mcp__mcp-atlassian__jira_search` was used in Step 2, use the corresponding
1127+
Jira comment MCP tool. Otherwise fall back to curl:
11141128

11151129
```bash
11161130
COMMENT_TEXT="Automated fix attempted but CVE still detected after applying changes.
@@ -1231,10 +1245,12 @@ EOF
12311245
--title "Security: Fix CVE-YYYY-XXXXX (<package-name>)" \
12321246
--body "$PR_BODY")
12331247
1234-
# Enable automerge if --automerge flag was passed
1235-
if [ "$AUTOMERGE" = "true" ]; then
1248+
# Enable automerge if --automerge flag was passed and PR was created successfully
1249+
if [ "$AUTOMERGE" = "true" ] && [ -n "$PR_URL" ] && [ "$PR_URL" != "null" ]; then
12361250
gh pr merge --auto --squash "$PR_URL"
12371251
echo "✅ Automerge enabled on $PR_URL — will merge when checks pass"
1252+
elif [ "$AUTOMERGE" = "true" ] && [ -z "$PR_URL" ]; then
1253+
echo "⚠️ Automerge skipped — PR URL is empty (PR creation may have failed)"
12381254
fi
12391255
```
12401256
- Capture the PR URL from the command output

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

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,17 @@ used in Jira — this is validated against the Jira API during onboarding.
3030

3131
2. **Validate Jira Component Name**
3232

33-
Confirm the component exists in Jira and has CVE issues:
33+
Use MCP if available (preferred), otherwise fall back to curl.
34+
Follow the same MCP-first pattern as `cve.find`:
35+
- Try `ToolSearch: select:mcp__mcp-atlassian__jira_search` first
36+
- If found, use it to search: `component = "${COMPONENT_NAME}" AND labels = SecurityTracking`
37+
- If not found, fall back to curl:
3438

3539
```bash
3640
JIRA_BASE_URL="https://redhat.atlassian.net"
3741
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
38-
ENCODED=$(python3 -c "import urllib.parse; print(urllib.parse.quote('component = \"${COMPONENT_NAME}\" AND labels = SecurityTracking'))")
42+
JQL="component = \"${COMPONENT_NAME}\" AND labels = SecurityTracking"
43+
ENCODED=$(python3 -c "import urllib.parse, sys; print(urllib.parse.quote(sys.argv[1]))" "$JQL")
3944

4045
RESULT=$(curl -s -X GET --connect-timeout 10 --max-time 15 \
4146
-H "Authorization: Basic ${AUTH}" \
@@ -152,24 +157,38 @@ used in Jira — this is validated against the Jira API during onboarding.
152157

153158
MAPPING_FILE="workflows/cve-fixer/component-repository-mappings.json"
154159

155-
# Insert new component using Python (preserves JSON formatting)
156-
python3 - <<PYEOF
157-
import json
160+
# Write the new component JSON to a temp file first to avoid shell injection
161+
# (component names or JSON values may contain quotes, backslashes, or newlines)
162+
echo "$NEW_COMPONENT_JSON" > /tmp/new_component.json
158163

159-
with open("${MAPPING_FILE}") as f:
164+
TODAY=$(date +%Y-%m-%d)
165+
166+
python3 - "$MAPPING_FILE" "$COMPONENT_NAME" /tmp/new_component.json "$TODAY" <<'PYEOF'
167+
import json, sys
168+
169+
mapping_file = sys.argv[1]
170+
component_name = sys.argv[2]
171+
new_component_file = sys.argv[3]
172+
today = sys.argv[4]
173+
174+
with open(mapping_file) as f:
160175
data = json.load(f)
161176
162-
# Add new component
163-
data["components"]["${COMPONENT_NAME}"] = ${NEW_COMPONENT_JSON}
177+
with open(new_component_file) as f:
178+
new_component = json.load(f)
164179
165-
# Update metadata
166-
data["metadata"]["last_updated"] = "$(date +%Y-%m-%d)"
180+
data["components"][component_name] = new_component
181+
data["metadata"]["last_updated"] = today
167182
168-
with open("${MAPPING_FILE}", "w") as f:
183+
with open(mapping_file, "w") as f:
169184
json.dump(data, f, indent=2, ensure_ascii=False)
170185
f.write("\n")
186+
187+
print(f"Added component: {component_name}")
171188
PYEOF
172189
190+
rm -f /tmp/new_component.json
191+
173192
# Validate JSON
174193
python3 -m json.tool "$MAPPING_FILE" > /dev/null && echo "✅ JSON valid"
175194

0 commit comments

Comments
 (0)