gha_echo/sanitize_gha_log_output doesn't strip ANSI escape or other control characters

[ggallen]

What happens

_sanitize_workflow_value (in scripts/lib/post-failure-report.lib.sh), the function underlying gha_echo/sanitize_gha_log_output/sanitize_comment_workflow_commands, strips literal :: and percent-encoded %0A/%0a/%0D/%0d sequences, but does not strip ANSI escape sequences (\x1b[...) or other control characters:

_sanitize_workflow_value() {
  local value="$1"
  value="${value//::/}"
  value="${value//%0A/}"
  value="${value//%0a/}"
  value="${value//%0D/}"
  value="${value//%0d/}"
  printf '%s' "${value}"
}

This function is the shared sanitization primitive for every gha_echo call across the codebase (post-code.sh, post-fix.sh, post-review.sh, post-retro.sh, pr-assignee.lib.sh, relabel-retrigger.lib.sh, etc.) — any of these that echo externally-sourced text (API error bodies, review content, etc.) through gha_echo inherit this gap.

What should happen

_sanitize_workflow_value (or a variant used specifically where externally-sourced text is involved) should also strip ANSI/CSI escape sequences and other non-printable control characters, consistent with defense-in-depth for GitHub Actions workflow-command log injection.

Context

Flagged during review of #469 (scripts/lib/relabel-retrigger.lib.sh's retrigger_via_label, which passes gh CLI stderr output through gha_echo). Not fixed as part of that PR since the gap is in shared infrastructure used by many other call sites, not something introduced there — a local, one-off fix in a single consumer would leave every other gha_echo caller with the same gap and create inconsistent sanitization behavior across the codebase. In practice, gh CLI disables ANSI color output when stderr isn't a TTY (which is the case for all these redirected-to-file/variable call sites), so the practical exposure is low, but the sanitizer's documented contract (stripping workflow-command-injection vectors) doesn't fully match its implementation.

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 3:30 PM UTC · Completed 3:34 PM UTC
Commit: d72d648 · View workflow run →

[fullsend-ai-triage]

Triage Summary

Severity: Low | Category: Security (defense-in-depth)

Problem

_sanitize_workflow_value in scripts/lib/post-failure-report.lib.sh strips :: and percent-encoded %0A/%0D sequences but does not strip ANSI/CSI escape sequences or other non-printable control characters. Since this function is the shared sanitization primitive for gha_echo, sanitize_gha_log_output, and sanitize_comment_workflow_commands, every caller that passes externally-sourced text inherits this gap.

Root Cause

The function was written to handle known workflow-command injection vectors but did not account for ANSI escape sequences or other control characters.

Recommended Fix

Add ANSI/CSI escape sequence stripping (pattern: \x1b\[[0-9;]*[A-Za-z]) and non-printable control character removal (0x00–0x08, 0x0B–0x0C, 0x0E–0x1F) to _sanitize_workflow_value. Apply the same treatment in sanitize_comment_workflow_commands. Keep the approach minimal and consistent with the existing bash-native style.

Proposed Test Case

# ANSI color codes should be stripped
input=$'\x1b[31mred text\x1b[0m'
expected='red text'
actual=$(_sanitize_workflow_value "$input")
[[ "$actual" == "$expected" ]] || fail "ANSI not stripped"

# Non-printable control chars should be stripped
input=$'hello\x01world\x07end'
expected='helloworldend'
actual=$(_sanitize_workflow_value "$input")
[[ "$actual" == "$expected" ]] || fail "Control chars not stripped"

# Existing stripping still works (regression)
input='::error::msg%0Aline2'
actual=$(_sanitize_workflow_value "$input")
[[ "$actual" != *'::'* && "$actual" != *'%0A'* ]] || fail "Regression"

Notes

Practical exposure is low — gh CLI disables ANSI color output when stderr is not a TTY — but the sanitizer's contract should match its implementation for defense-in-depth consistency.

---

Labels: This is a security defense-in-depth gap in shared infrastructure (sanitization function). The 'security' label applies because the issue concerns log injection vectors. The 'enhancement' label applies as this extends existing sanitization coverage.

[fullsend-ai-prioritize]

RICE Priority Score: 0.9

Score breakdown

Dimension	Score	Reasoning
Reach	1	The affected function (_sanitize_workflow_value) is shared infrastructure underlying gha_echo across many scripts (post-code.sh, post-fix.sh, post-review.sh, post-retro.sh, pr-assignee.lib.sh, relabel-retrigger.lib.sh, etc.), giving it broad theoretical reach. However, practical exposure is low since gh CLI disables ANSI when stderr isn't a TTY, so few users would actually encounter the gap. Score reflects platform-wide shared code with low real-world triggering.
Impact	0.5	This is a defense-in-depth security improvement, not an active vulnerability. The issue author and triage both note 'practical exposure is low.' The gh CLI naturally avoids producing ANSI in the relevant contexts, providing a de facto workaround. The impact per affected user is minimal — it closes a theoretical log-injection vector rather than fixing an observed problem.
Confidence	0.9	Very high confidence. The issue is exceptionally well-described with the exact function, its callers, the gap, a proposed fix approach, and concrete test cases. The triage summary confirms root cause and scope. There is minimal ambiguity about what needs to change.
Effort	0.5	Simple, well-scoped change. The fix involves adding ANSI/CSI escape stripping (a sed or bash pattern) and control character removal to one function in one file, possibly with a parallel change in sanitize_comment_workflow_commands. The triage provides a clear implementation approach and test cases. This is a small, targeted bash change.

Formula: (1 x 0.5 x 0.9) / 0.5 = 0.9