fix(security-review): batch template-discovery grep so review.sh --scanners-only stops overflowing argv on the monorepo
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.
This commit is contained in:
parent
f7bfa5baf0
commit
bdafb7bbf1
1 changed files with 12 additions and 2 deletions
|
|
@ -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)"
|
||||
|
|
|
|||
Reference in a new issue