feat(harness): report categorized post-script failures on issue/PR#38
feat(harness): report categorized post-script failures on issue/PR#38ifireball wants to merge 1 commit into
Conversation
Port fullsend-ai/fullsend#2921: add shared post-failure-report.sh with categorized failure comments, output sanitization, and workflow run links. post-code.sh posts detailed failures on the issue; post-fix.sh posts equivalent comments on the PR. Extend post-code-test.sh and post-fix-test.sh. Signed-off-by: Barak Korren <bkorren@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
🤖 Finished Review · ✅ Success · Started 7:39 AM UTC · Completed 7:54 AM UTC |
PR Summary by QodoReport categorized post-script failures on issues/PRs (shared failure reporter)
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1. ISSUE_NUMBER in ::warning unsanitized
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| echo "::warning::Posting failure comment to issue #${ISSUE_NUMBER}..." | ||
| gh issue comment "${ISSUE_NUMBER}" \ | ||
| --repo "${REPO_FULL_NAME}" \ | ||
| --body "${body}" 2>/dev/null || \ | ||
| echo "::warning::Failed to post error comment to issue #${ISSUE_NUMBER}" | ||
| } | ||
|
|
||
| report_post_failure_to_pr() { | ||
| local exit_code="${1:-$?}" | ||
|
|
||
| if [ "${POST_FAILURE_REPORTED}" = "true" ]; then | ||
| return 0 | ||
| fi | ||
| POST_FAILURE_REPORTED=true | ||
|
|
||
| _post_failure_ensure_token | ||
|
|
||
| local category="${POST_FAILURE_CATEGORY:-post-script-error}" | ||
| local detail="${POST_FAILURE_DETAIL:-Post-fix script failed before push or PR update completed.}" | ||
| local body | ||
| # PR_NUMBER and REPO_FULL_NAME are required by post-fix.sh before sourcing. | ||
| # shellcheck disable=SC2153 | ||
| body="$(build_post_failure_comment \ | ||
| "fix" "${exit_code}" "${category}" "${detail}" \ | ||
| "${REPO_FULL_NAME}" "/fs-fix")" | ||
|
|
||
| echo "::warning::Posting failure comment to PR #${PR_NUMBER}..." | ||
| gh pr comment "${PR_NUMBER}" \ | ||
| --repo "${REPO_FULL_NAME}" \ | ||
| --body "${body}" 2>/dev/null || \ | ||
| echo "::warning::Failed to post error comment to PR #${PR_NUMBER}" |
There was a problem hiding this comment.
1. issue_number in ::warning unsanitized 📜 Skill insight ⛨ Security
The new failure-reporting library emits GitHub Actions workflow commands (::warning::...) with interpolated values that are not sanitized. Unsanitized interpolation can allow workflow-command injection via ::, encoded newlines (%0A/%0D), or control characters.
Agent Prompt
## Issue description
`scripts/lib/post-failure-report.sh` prints GitHub Actions workflow commands (e.g., `::warning::...`) that include interpolated variables (e.g., `ISSUE_NUMBER`, `PR_NUMBER`) without sanitizing each variable for workflow-command injection vectors (`::`, `%0A/%0D`, ANSI/control chars).
## Issue Context
Compliance requires that **every interpolated value** in a GitHub Actions workflow command be individually sanitized, not just some values or only "likely safe" values.
## Fix Focus Areas
- scripts/lib/post-failure-report.sh[193-223]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| SCRIPT_DIR_POST="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| # shellcheck source=lib/post-failure-report.sh | ||
| source "${SCRIPT_DIR_POST}/lib/post-failure-report.sh" | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Configuration | ||
| # --------------------------------------------------------------------------- |
There was a problem hiding this comment.
2. Protected scripts/ paths changed 📜 Skill insight § Compliance
This PR modifies protected governance/infrastructure paths under scripts/, which must always be flagged for required human review and must not be auto-approved. These changes affect post-script behavior and failure reporting in automation.
| if [ -n "${sanitized_detail}" ]; then | ||
| detail_block="$(cat <<EOF | ||
|
|
||
| **Details:** | ||
| \`\`\` | ||
| ${sanitized_detail} | ||
| \`\`\` | ||
| EOF |
There was a problem hiding this comment.
3. Markdown fence injection 🐞 Bug ≡ Correctness
build_post_failure_comment embeds sanitized failure output inside a triple-backtick fenced block without escaping embedded ``` sequences, so certain command outputs can prematurely terminate the fence and corrupt the rendered issue/PR comment. This is reachable because post-code.sh/post-fix.sh pass raw command output (push/pre-commit/etc.) into the failure detail that becomes the comment body.
Agent Prompt
### Issue description
`build_post_failure_comment()` builds a Markdown comment that wraps `${sanitized_detail}` in a triple-backtick fenced code block. If `${sanitized_detail}` contains ``` (or an unmatched fence-like sequence), the fence can be closed early and the rest of the comment renders incorrectly.
### Issue Context
Although the content is token-redacted, it is still untrusted command output (e.g., git push / pre-commit output) and may contain arbitrary text.
### Fix Focus Areas
- scripts/lib/post-failure-report.sh[118-166]
### Suggested fix
Update `build_post_failure_comment()` to avoid fenced blocks entirely by rendering details as an indented code block:
- Build `detail_block` by prefixing every line of `${sanitized_detail}` (including empty lines) with 4 spaces, e.g. via `sed 's/^/ /'`.
- Keep the existing token/PEM redaction and line truncation.
This removes the possibility of fence-termination by embedded backticks and preserves readability in GitHub-flavored Markdown.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ReviewVerdict: comment — one medium-severity defense-in-depth improvement and several low-severity observations. No blocking issues. This PR extracts failure reporting into a well-structured shared library ( Findings1.
|
fullsend-ai-review
Bot
added
the
requires-manual-review
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
I think we learned in the fix for the prioritize agent's CSMA lib fix that the post scripts of agents cannot reach a lib script. Right?
2 participants
Summary
scripts/lib/post-failure-report.shwith categorized failure comments, output sanitization, and workflow run linkspost-code.shposts detailed failure comments on the issue (push rejected, pre-commit blocked, secret scan, PR creation failed, etc.)post-fix.shposts equivalent failure comments on the PR (previously silent on many post-script failures)post-code-test.sh/post-fix-test.shcoverage for failure reporting, sanitization, and push categorizationPorts fullsend-ai/fullsend#2921.
Depends on #73 — the new/extended tests in this PR will not run in CI until the script-test workflow lands.
Test plan
bash scripts/post-code-test.shbash scripts/post-fix-test.shMade with Cursor