From bdafb7bbf1bf92fd1d17971c55b33f34204937c6 Mon Sep 17 00:00:00 2001
From: Adam Moussa <adam@seahavenind.com>
Date: Tue, 23 Jun 2026 15:39:03 -0400
Subject: [PATCH] fix(security-review): batch template-discovery grep so
 review.sh --scanners-only stops overflowing argv on the monorepo
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The cfn-lint template-discovery step in review.sh piped the repo's whole
matched-file list into 'xargs -I{} sh -c "grep -l {}"'. On the orchestrator
monorepo — especially from a deep worktree path, where every matched path is a
long absolute path — xargs -I{} packs all paths into one assembled command and
aborts with 'xargs: command line cannot be assembled, too long'. The subprocess
exits non-zero having emitted ZERO findings, so the global pre-push hook BLOCKS
every push (agents were working around it with --no-verify).

Fix: switch the grep stage to NUL-delimited, un-batched xargs
(find ... -print0 | xargs -0 grep -lE ...). xargs -0 (no -I) splits the input
across multiple grep invocations, so the argv never exceeds ARG_MAX; grep -l
reports the same matching files as the old per-file grep, and -print0/-0 is safe
for paths with spaces/newlines. The first 'xargs -I{} find {}' is kept (find
needs the start path before its expression) and is bounded by the scope-path
count, so it is not an overflow source. Trailing '|| true' preserves the old
no-match/no-files semantics (TPLS = list-of-templates or empty, never fails).

Purely an argv-batching fix: the scanned file set, findings, and exit codes are
unchanged. Verified exit-code-identical: clean tree -> exit 0 (PASS); planted
GitHub PAT + RSA private key -> exit 1 (BLOCK, gitleaks high); planted CFN
template with a cfn-lint error on a deeply-nested path -> exit 1 (cfn-lint flags
it, no overflow). The previously-overflowing command now completes clean.
---
 security-review/review.sh | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/security-review/review.sh b/security-review/review.sh
index 6575ec5..73d294a 100755
--- a/security-review/review.sh
+++ b/security-review/review.sh
@@ -45,8 +45,18 @@ note_missing() { echo "  [MISSING] $1 — not run. Install: $2" >&2; }
 if command -v cfn-lint >/dev/null; then
   # Prune generated/vendored trees (cdk.out, node_modules, …): scanning synthesized output is
   # wrong and, on CDK repos, explodes the arg list / stalls the scanners.
-  TPLS="$(scope_paths | xargs -I{} find {} \( -type d \( -name cdk.out -o -name node_modules -o -name .git -o -name .claude -o -name .aws-sam -o -name .venv -o -name venv -o -name dist -o -name build \) -prune \) -o \( -type f \( -name '*.yaml' -o -name '*.yml' \) -print \) 2>/dev/null \
-          | xargs -I{} sh -c 'grep -lE "AWSTemplateFormatVersion|Transform: *AWS::Serverless" "{}" 2>/dev/null || true')"
+  # `find -print0 | xargs -0 grep -lE` (was `… | xargs -I{} sh -c 'grep -l "{}"'`): the grep stage
+  # is the one that overflows. `xargs -I{}` packs every matched path — long, absolute, deep-worktree
+  # paths on this monorepo — into one assembled command and dies with "command line cannot be
+  # assembled, too long", emitting zero findings and blocking the push. NUL-delimited `xargs -0 grep`
+  # splits across invocations transparently (batches by ARG_MAX, never overflows), is safe for paths
+  # with spaces/newlines, and `grep -l` reports the same matching files as the old per-file grep.
+  # The first `xargs -I{} find {}` keeps the start path first (find needs it before the expression)
+  # and is bounded by the scope-path count, so it is not an overflow risk. `--no-run-if-empty` is
+  # GNU-only, so the trailing `|| true` absorbs grep's exit 1 on no-match / no-files, matching the
+  # old per-file `… || true` so TPLS is "list of templates, or empty" and never fails the gate.
+  TPLS="$(scope_paths | xargs -I{} find {} \( -type d \( -name cdk.out -o -name node_modules -o -name .git -o -name .claude -o -name .aws-sam -o -name .venv -o -name venv -o -name dist -o -name build \) -prune \) -o \( -type f \( -name '*.yaml' -o -name '*.yml' \) -print0 \) 2>/dev/null \
+          | xargs -0 grep -lE "AWSTemplateFormatVersion|Transform: *AWS::Serverless" 2>/dev/null || true)"
   if [ -n "$TPLS" ]; then
     # shellcheck disable=SC2086
     RAW="$(cfn-lint -f json $TPLS 2>/dev/null || true)"