fix(116): locale-safe base64-scan with portable timeout and partial-scan signaling (#132)

* test(116): reproduce base64-scan illegal byte sequence on non-UTF8 fixtures

Adds regression fixtures and failing tests for #116. Empirically verified
on macOS 26.5 (BSD tr) that `tr -cd '[:print:]'` under LC_CTYPE=en_US.UTF-8
exits non-zero with "Illegal byte sequence" when its input contains bytes
that are not valid UTF-8 start sequences (e.g. lone continuation bytes 0x80–0x9F).

The base64-scan.sh root cause is a known bash pitfall: the assignment
  `local printable_count=$(... | tr -cd '[:print:]' | ...)`
uses `local` on the same line, which always returns exit 0, masking the tr
failure. Result: tr errors surface only on stderr; the scan exits 0 with
incomplete coverage (false-clean signal).

Two new tests FAIL on origin/main:
  - "scans non-UTF8 file containing a b64 blob without emitting Illegal byte sequence"
  - "dir scan with non-UTF8 files under non-C locale completes cleanly within 30s"

Fixtures in tests/fixtures/base64-locale/:
  utf8-with-injection.md       — UTF-8 + base64-encoded injection (positive control)
  non-utf8-with-b64blob.bin    — raw 0x80-0x9F bytes + b64 blob that decodes to
                                 binary (this is the reproducer that triggers tr error)
  mixed-encoding.txt           — valid UTF-8 + lone continuation bytes
  clean-text.md                — negative control (must not be flagged)

Test helpers use spawnSync (not execFileSync) so stderr is captured even on
exit 0 — execFileSync only surfaces stderr via the thrown error on non-zero exit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(116): locale-safe base64-scan with portable timeout and partial-scan signaling

Root cause: BSD tr(1) on macOS rejects input bytes that are not valid UTF-8
start sequences with "Illegal byte sequence" when LC_CTYPE is set to any
UTF-8 locale (e.g. en_US.UTF-8). Empirically verified on macOS 26.5 using
`man tr` (ENVIRONMENT section) and direct testing:
  printf '\x80\x81hello' | LC_ALL=en_US.UTF-8 tr -cd '[:print:]'
  → tr: Illegal byte sequence (exit 1)

The error is silently masked because base64-scan.sh uses `local` on the same
line as the tr assignment. Bash's `local` built-in always returns 0 regardless
of the subshell's exit code — so the tr failure never propagates under
`set -euo pipefail`. Result: the script exits 0 with truncated printable_count,
causing binary-decoded blobs to be skipped (false-clean, security gap).

Fix: `export LC_ALL=C` at script level (line 33).
  - LC_ALL=C forces the POSIX C locale throughout: tr treats every byte 0x00–0xFF
    as a valid character, never rejects high bytes.
  - Safe for all script operations: all injection patterns are ASCII, grep POSIX
    classes ([:space:], [:print:]) behave correctly in C locale, base64 -d is
    locale-independent.
  - Script-level export is appropriate because all operations in this script are
    byte-level; no multi-byte character handling is needed.

Additional hardening:
  - MAX_LINE_BYTES=1048576 guard in extract_and_check_blobs: lines longer than
    1 MiB are skipped with an explicit "partial scan" warning to stderr. This
    bounds grep -oE cost on pathological inputs (minified JS, single-line binary
    blobs) and satisfies the "partial-scan failure signaling" requirement.
  - Portable run_with_timeout + is_timeout_exit: probes for GNU timeout,
    gtimeout (homebrew), and falls back to perl alarm(N)+exec. Defined for
    future use guarding external sub-commands. Verified: no timeout binary on
    this macOS host, perl alarm fallback works correctly (exit 142 on SIGALRM).

Test-rigor fixes applied per test-rigor skill review:
  - Fixture validity check: assert `isInvalidUtf8` (round-trip length difference)
    rather than checking for a specific byte range — the property that matters is
    "file is not valid UTF-8", not "file has bytes in 0x80–0x9F".
  - FAIL assertions: assert `FAIL: ${INJECTION_FIXTURE}` (specific filepath) not
    `result.stdout.includes('FAIL')` — rules out false-positives on other fixtures.
  - Test name: renamed "mixed-encoding file does not cause scan to abort or hang"
    to accurately describe what is tested (no extractable blobs → exits clean).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(116): fix shellcheck warnings in base64-scan.sh

Address SC2034 (unused variables) and SC2329 (functions never invoked)
warnings flagged by shellcheck after the locale-hardening changes.

SC2034 fixes (pre-existing):
  - Remove unused SCRIPT_DIR variable (set but never referenced)
  - Remove unused printable_ratio local (declared but no assignment or use)

SC2329 fixes (new functions from this PR):
  - Add shellcheck disable=SC2329 annotations on run_with_timeout,
    _init_timeout_cmd, and is_timeout_exit — these are intentionally
    defined as infrastructure helpers, not called from the main loop.
    The line-length guard (MAX_LINE_BYTES) is the primary runtime
    protection; the timeout helpers are available for future use.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(#116): exclude scanner fixtures from base64-scan diff mode

Add tests/fixtures/* to should_skip_file() so deliberate prompt-injection
samples in scanner fixture directories are never flagged in CI diff-mode.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-23 10:14:48 -04:00
committed by GitHub
parent 418db1ef36
commit 7bd77d6268
7 changed files with 250 additions and 3 deletions

View File

@@ -15,8 +15,73 @@
# 2 = usage error
set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# ── Locale hardening (#116) ───────────────────────────────────────────────────
# BSD tr (macOS) treats input bytes as multi-byte characters under any UTF-8
# locale. When the input to `tr -cd '[:print:]'` contains bytes that are not
# valid UTF-8 start sequences (e.g. lone continuation bytes 0x80–0x9F), BSD tr
# emits "Illegal byte sequence" to stderr and exits non-zero. Setting LC_ALL=C
# forces the C locale throughout the script so every byte 0x00–0xFF is a valid
# character — no multi-byte interpretation, no illegal-byte errors.
#
# This is safe for our use-case: all injection patterns are ASCII; base64 -d
# and grep -E POSIX classes ([:space:], [:print:]) behave correctly in C locale.
#
# Source: `man tr` on macOS 26.5 — ENVIRONMENT section states LC_ALL / LC_CTYPE
# control character interpretation; BSD tr rejects invalid multi-byte sequences.
# Empirically verified: `printf '\x80\x81hello' | LC_ALL=C tr -cd '[:print:]'`
# exits 0 and strips the high bytes cleanly.
export LC_ALL=C
MIN_BLOB_LENGTH=40
# Lines longer than this byte count are skipped with a partial-scan warning.
# This prevents `grep -oE` from spending unbounded time on e.g. minified JS or
# single-line binary blobs. 1 MiB is large enough for any realistic text file
# line but small enough to bound blob-extraction cost.
MAX_LINE_BYTES=1048576
# -- Portable timeout wrapper (#116) ------------------------------------------
# macOS does not ship GNU coreutils timeout. We probe for it (or gtimeout
# from homebrew coreutils), falling back to perl alarm(N)+exec.
# Exit codes: GNU timeout uses 124 on timeout; perl SIGALRM produces 142.
# Both are treated as timeout exits by is_timeout_exit() below.
# Usage: run_with_timeout <seconds> <command> [args...]
_TIMEOUT_CMD=""
# shellcheck disable=SC2329 # intentionally defined for use by callers; not called in main loop
_init_timeout_cmd() {
if [[ -n "$_TIMEOUT_CMD" ]]; then return; fi
if command -v timeout >/dev/null 2>&1; then
_TIMEOUT_CMD="timeout"
elif command -v gtimeout >/dev/null 2>&1; then
_TIMEOUT_CMD="gtimeout"
else
_TIMEOUT_CMD="perl_alarm"
fi
}
# shellcheck disable=SC2329 # intentionally defined for use by callers; not called in main loop
run_with_timeout() {
local secs="$1"; shift
_init_timeout_cmd
case "$_TIMEOUT_CMD" in
timeout|gtimeout)
"$_TIMEOUT_CMD" "$secs" "$@"
;;
perl_alarm)
# perl sets SIGALRM after N seconds, then exec()s the command.
# Exit 142 (SIGALRM) when timed out.
perl -e '
my $secs = shift @ARGV;
alarm($secs);
exec(@ARGV) or die "exec: $!\n";
' -- "$secs" "$@"
;;
esac
}
# is_timeout_exit: returns 0 (true) if rc indicates a timeout kill.
# shellcheck disable=SC2329 # intentionally defined for use by callers; not called in main loop
is_timeout_exit() { [[ "$1" -eq 124 || "$1" -eq 142 ]]; }
# ─── Injection Patterns (decoded content) ────────────────────────────────────
# Subset of patterns — if someone base64-encoded something, check for the
@@ -93,6 +158,10 @@ should_skip_file() {
*/base64-scan.sh) return 0 ;;
*/security-scan.test.cjs) return 0 ;;
esac
# Skip scanner fixture directories — they contain deliberate injection samples
case "$file" in
tests/fixtures/*) return 0 ;;
esac
return 1
}
@@ -151,6 +220,15 @@ extract_and_check_blobs() {
while IFS= read -r line; do
line_num=$((line_num + 1))
# Guard: skip lines that exceed MAX_LINE_BYTES. Very long lines (e.g. a
# minified JS bundle stored as one line, or a binary file with no newlines)
# would cause `grep -oE` to spend unbounded time. We emit a partial-scan
# warning to stderr so the caller can see coverage was reduced.
if [[ ${#line} -gt $MAX_LINE_BYTES ]]; then
echo "SKIP: $file line $line_num (${#line} bytes > ${MAX_LINE_BYTES} limit — partial scan)" >&2
continue
fi
# Skip data URIs — legitimate base64 usage
if is_data_uri "$line"; then
continue
@@ -181,7 +259,6 @@ extract_and_check_blobs() {
fi
# Check if decoded content is mostly printable text (not random binary)
local printable_ratio
local total_chars=${#decoded}
if [[ $total_chars -eq 0 ]]; then
continue