Skip to content

Commit f62586a

Browse files
committed
chore: harden CVE remediation summary rendering
Address review findings: escape backslashes before pipes in table cells, fail loudly when the issues artifact is missing or malformed, exercise note truncation in the tests, and scope the always() assertion to the report step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VsjLedntEDA6nHFmfYYHAQ
1 parent 5c5d1f3 commit f62586a

2 files changed

Lines changed: 42 additions & 7 deletions

File tree

.github/scripts/render-cve-remediation-summary.sh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,14 @@ NOTES_DIR="${2:?notes directory is required}"
1313

1414
MAX_NOTE_LENGTH=500
1515

16+
if ! jq -e 'type == "array"' "$ISSUES_FILE" >/dev/null 2>&1; then
17+
echo "issues file '$ISSUES_FILE' is missing or is not a JSON array" >&2
18+
exit 1
19+
fi
20+
1621
escape_table_cell() {
17-
printf '%s' "${1//|/\\|}"
22+
local escaped="${1//\\/\\\\}"
23+
printf '%s' "${escaped//|/\\|}"
1824
}
1925

2026
echo '| Linear issue | Priority | Status | Title | Notes |'

.github/scripts/test-cve-remediation.sh

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -378,25 +378,46 @@ cat > "$RENDER_ISSUES_FILE" <<'EOF'
378378
"priority": 4,
379379
"status": "Backlog",
380380
"statusType": "backlog"
381+
},
382+
{
383+
"id": "issue-35",
384+
"identifier": "SOU-35",
385+
"title": "[sourcebot-dev/example] CVE-35: truncated long note",
386+
"url": "https://linear.app/sourcebot/issue/SOU-35/test",
387+
"priority": 5,
388+
"status": "Backlog",
389+
"statusType": "backlog"
381390
}
382391
]
383392
EOF
384393
printf 'Opened PR #99 upgrading foo to 1.2.3.\n' > "$RENDER_NOTES_DIR/SOU-31.md"
385-
printf 'No patched | release exists yet;\nre-check tomorrow.\n' > "$RENDER_NOTES_DIR/SOU-32.md"
394+
cat > "$RENDER_NOTES_DIR/SOU-32.md" <<'EOF'
395+
No patched | release \| exists yet;
396+
re-check tomorrow.
397+
EOF
386398
printf 'Skipped: dependency already patched on main.\n' > "$RENDER_NOTES_DIR/issue-34.md"
399+
LONG_NOTE=$(printf 'a%.0s' $(seq 1 600))
400+
printf '%s\n' "$LONG_NOTE" > "$RENDER_NOTES_DIR/SOU-35.md"
387401

388402
EXPECTED_SUMMARY='| Linear issue | Priority | Status | Title | Notes |
389403
| --- | ---: | --- | --- | --- |
390404
| [SOU-31](https://linear.app/sourcebot/issue/SOU-31/test) | 1 | Todo | [sourcebot-dev/example] CVE-31: note keyed by identifier | Opened PR #99 upgrading foo to 1.2.3. |
391-
| [SOU-32](https://linear.app/sourcebot/issue/SOU-32/test) | 2 | Backlog | [sourcebot-dev/example] CVE-32: pipe \| in title | No patched \| release exists yet; re-check tomorrow. |
405+
| [SOU-32](https://linear.app/sourcebot/issue/SOU-32/test) | 2 | Backlog | [sourcebot-dev/example] CVE-32: pipe \| in title | No patched \| release \\\| exists yet; re-check tomorrow. |
392406
| [SOU-33](https://linear.app/sourcebot/issue/SOU-33/test) | 3 | Backlog | [sourcebot-dev/example] CVE-33: no note | |
393407
| [SOU-34](https://linear.app/sourcebot/issue/SOU-34/test) | 4 | Backlog | [sourcebot-dev/example] CVE-34: note keyed by UUID | Skipped: dependency already patched on main. |'
408+
EXPECTED_SUMMARY+="
409+
| [SOU-35](https://linear.app/sourcebot/issue/SOU-35/test) | 5 | Backlog | [sourcebot-dev/example] CVE-35: truncated long note | ${LONG_NOTE:0:500}… |"
394410

395411
assert_equals \
396-
"renders the results table with flattened, escaped agent notes" \
412+
"renders the results table with flattened, escaped, truncated agent notes" \
397413
"$("$RENDER_SCRIPT" "$RENDER_ISSUES_FILE" "$RENDER_NOTES_DIR")" \
398414
"$EXPECTED_SUMMARY"
399415

416+
if "$RENDER_SCRIPT" "$RENDER_NOTES_DIR/does-not-exist.json" "$RENDER_NOTES_DIR" >/dev/null 2>&1; then
417+
echo "FAIL: fails loudly instead of rendering an empty table when the issues file is missing"
418+
exit 1
419+
fi
420+
400421
assert_json \
401422
"maps Linear on-call assignees to GitHub reviewers" \
402423
"$(jq -c . "$REVIEWER_MAP_FILE")" \
@@ -478,15 +499,23 @@ assert_workflow_not_contains \
478499
assert_workflow_contains \
479500
"hands issue metadata to the remediation job through an artifact" \
480501
'name: cve-remediation-issues'
502+
assert_workflow_contains \
503+
"uploads the issue metadata from the discovery job" \
504+
'name: Upload issue metadata for the remediation summary'
505+
assert_workflow_contains \
506+
"downloads the issue metadata in the remediation job" \
507+
'name: Download issue metadata'
481508
assert_workflow_contains \
482509
"prompts Claude with the notes location for the run summary" \
483510
'cve-remediation-notes/<LINEAR-IDENTIFIER>.md'
484511
assert_workflow_contains \
485512
"renders the remediation results table after the agent runs" \
486513
'.cve-remediation-workflow/.github/scripts/render-cve-remediation-summary.sh'
487-
assert_workflow_contains \
488-
"reports remediation results even when the agent step fails" \
489-
'if: always()'
514+
if ! grep -A1 -- '- name: Report remediation results' "$WORKFLOW_FILE" | grep -Fq 'if: always()'; then
515+
echo "FAIL: reports remediation results even when the agent step fails"
516+
echo "Expected the 'Report remediation results' step to run with 'if: always()'"
517+
exit 1
518+
fi
490519
assert_workflow_contains \
491520
"includes the agent notes column in the results table" \
492521
'| Linear issue | Priority | Status | Title | Notes |' \

0 commit comments

Comments
 (0)