Summary
Found while fixing #2451 (Greptile review, round 3 on PR #2557).
normalize-ifs.mjs now correctly recognizes ${IFS:+ }/${IFS+ } (bash's alternate-value expansion, restricted to all-whitespace substituted content) as producing a literal space, the same as the whole-variable ${IFS}/$IFS form. But this operator's behavior has nothing to do with the variable being named IFS specifically — bash's ${VAR:+word} substitutes word whenever VAR is set and non-null (the normally-true case for almost any commonly-set variable), regardless of what VAR's own value actually is.
This means ${HOME:+ }, ${PWD:+ }, ${PATH:+ }, or any other variable that's normally set in a shell session works identically to ${IFS:+ } for this specific purpose — none of them reference IFS's value at all, they just use an arbitrary set variable as the condition and inject a literal space as the substituted "alternate" text.
Why this is a separate problem from #2451
normalize-ifs.mjs is (by design and by name) scoped to recognizing IFS-specific obfuscation. A per-variable-name normalizer like this one can never fully close the "any normally-set variable + :+/+ with a whitespace alternate value" class, since the variable name in the bypass is not fixed — an attacker (or a sufficiently obfuscation-aware agent) could pick any variable known to be set in the target shell.
Suggested fix
This needs a fundamentally different detection strategy than name-based normalization, e.g.:
- A regex matching the general SHAPE
${<any-identifier>:+<all-whitespace>} / ${<any-identifier>+<all-whitespace>} regardless of the specific variable name (broader coverage, but still name-shape-based rather than semantically evaluating whether that variable is actually set at execution time — a variable that's NOT normally set would make this a false positive, though for guard-purposes a false positive here is the safe direction to err in).
- Or, a more fundamental rethink of guard-git.sh's approach: instead of static regex scanning of command TEXT, actually invoke a shell dry-run/AST parse to see the real resulting argv bash would produce, sidestepping the entire class of "does this obfuscation technique produce whitespace" heuristics.
The first option is a bounded, incremental improvement; the second is a much larger architectural change (matching CLAUDE.md's "prioritize best architecture" guidance if pursued) that would also close the ${IFS/pattern/replacement} and ${IFS@Q}-style gaps noted in normalize-ifs.mjs's own comments, and any other future bash expansion trick, without needing per-case regex patches.
Related
Summary
Found while fixing #2451 (Greptile review, round 3 on PR #2557).
normalize-ifs.mjsnow correctly recognizes${IFS:+ }/${IFS+ }(bash's alternate-value expansion, restricted to all-whitespace substituted content) as producing a literal space, the same as the whole-variable${IFS}/$IFSform. But this operator's behavior has nothing to do with the variable being namedIFSspecifically — bash's${VAR:+word}substituteswordwheneverVARis set and non-null (the normally-true case for almost any commonly-set variable), regardless of whatVAR's own value actually is.This means
${HOME:+ },${PWD:+ },${PATH:+ }, or any other variable that's normally set in a shell session works identically to${IFS:+ }for this specific purpose — none of them reference IFS's value at all, they just use an arbitrary set variable as the condition and inject a literal space as the substituted "alternate" text.Why this is a separate problem from #2451
normalize-ifs.mjsis (by design and by name) scoped to recognizing IFS-specific obfuscation. A per-variable-name normalizer like this one can never fully close the "any normally-set variable +:+/+with a whitespace alternate value" class, since the variable name in the bypass is not fixed — an attacker (or a sufficiently obfuscation-aware agent) could pick any variable known to be set in the target shell.Suggested fix
This needs a fundamentally different detection strategy than name-based normalization, e.g.:
${<any-identifier>:+<all-whitespace>}/${<any-identifier>+<all-whitespace>}regardless of the specific variable name (broader coverage, but still name-shape-based rather than semantically evaluating whether that variable is actually set at execution time — a variable that's NOT normally set would make this a false positive, though for guard-purposes a false positive here is the safe direction to err in).The first option is a bounded, incremental improvement; the second is a much larger architectural change (matching CLAUDE.md's "prioritize best architecture" guidance if pursued) that would also close the
${IFS/pattern/replacement}and${IFS@Q}-style gaps noted innormalize-ifs.mjs's own comments, and any other future bash expansion trick, without needing per-case regex patches.Related
normalize-ifs.mjs's own doc comment documents this as a known, accepted gap pending this follow-up.