mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(careful): replace sed masking with quote-aware awk scanner (#1060)
The sed-based -m content masker had two bypass paths that capy-ai identified:
1. Single-quoted message containing a double-quote boundary:
git commit -m 'note -m "' && rm -rf build && echo '"'
The double-quote pass matched '-m "' inside the single-quoted string,
swallowing '&& rm -rf build && echo '; the rm vanished from CMD_MEDIUM.
2. Double-quoted message with a command substitution:
git commit -m "$(rm -rf build)"
sed replaced the content including the $(...), so the substitution
— which executes in the shell — was masked away.
Replace the sed substitution with a quote-aware awk scanner that:
- Walks the command string tracking single-quote, double-quote, and
backslash state, so it only treats a delimiter as a message boundary
when the shell would.
- Masks single-quoted messages (always literal) and double-quoted messages
that contain no $, backtick, or backslash (safe to mask — no expansion).
- Leaves double-quoted messages with $, backtick, or backslash unmasked
so command substitutions stay visible to the MEDIUM checks.
- Handles -m, -am, --message, and --message= forms.
Four new tests added as requested (alongside the original six):
- single-quoted message with embedded double-quote + chained rm → warns
- commit message with $(rm -rf build) → warns
- git push --force chained after commit → warns
- git commit -am "drop table notes" → silent (message masked)
This commit is contained in:
1 parent
c4427ae146
commit
8e6328c9ce
2 files changed
+113
-8
No files matched your search
@@ -224,17 +224,76 @@ esac
|
||||
WARN=""
|
||||
PATTERN=""
|
||||
|
||||
# Mask quoted -m argument content in git commit commands so that descriptive
|
||||
# commit messages (e.g. "fix: prevent drop table in migration") do not trigger
|
||||
# false positives. Real destructive commands outside the quoted string still
|
||||
# match because the rest of the command is preserved. Fall back to $CMD when
|
||||
# sed is unavailable.
|
||||
# Mask quoted -m/-am/--message argument content in git commit commands so that
|
||||
# descriptive commit messages do not trigger false positives. Uses a
|
||||
# quote-aware awk scanner that tracks shell quoting state so it cannot swallow
|
||||
# real commands that follow a commit via &&/; (e.g. `git commit -m 'note -m "'
|
||||
# && rm -rf build` keeps the rm visible). Double-quoted messages containing $,
|
||||
# backtick or backslash are left unmasked so command substitutions remain
|
||||
# visible to the MEDIUM checks. Falls back to $CMD when awk is unavailable.
|
||||
CMD_MEDIUM="$CMD"
|
||||
CMD_MEDIUM_LOWER="$CMD_LOWER"
|
||||
if printf '%s' "$CMD" | grep -qE '(^|[;&|][[:space:]]*)git[[:space:]]+commit[[:space:]]' 2>/dev/null; then
|
||||
_cmd_masked=$(printf '%s' "$CMD" \
|
||||
| sed 's/-m[[:space:]]*"[^"]*"/-m ""/g
|
||||
s/-m[[:space:]]*'"'"'[^'"'"']*'"'"'/-m '"'"''"'"'/g' 2>/dev/null) \
|
||||
_AWK_MASK_PROG=$(cat <<'_AWKEOF'
|
||||
function handle_msg(s, n, i, j, safe) {
|
||||
if (i > n) return i
|
||||
if (substr(s, i, 1) == "'") {
|
||||
result = result "'"
|
||||
i++
|
||||
while (i <= n && substr(s, i, 1) != "'") i++
|
||||
result = result "'"
|
||||
if (i <= n) i++
|
||||
} else if (substr(s, i, 1) == "\"") {
|
||||
j = i+1; safe = 1
|
||||
while (j <= n && substr(s, j, 1) != "\"") {
|
||||
if (substr(s,j,1) == "$" || substr(s,j,1) == "`" || substr(s,j,1) == "\\") safe = 0
|
||||
j++
|
||||
}
|
||||
if (safe) { result = result "\"\""; i = j+1 }
|
||||
else {
|
||||
result = result substr(s, i, j-i)
|
||||
if (j <= n) result = result "\""
|
||||
i = j+1
|
||||
}
|
||||
}
|
||||
return i
|
||||
}
|
||||
{
|
||||
n = length($0); sq = 0; dq = 0; bs = 0; i = 1
|
||||
while (i <= n) {
|
||||
c = substr($0, i, 1)
|
||||
if (bs) { result = result c; bs = 0; i++; continue }
|
||||
if (!sq && dq && c == "\\") { result = result c; bs = 1; i++; continue }
|
||||
if (!dq && c == "'") { sq = !sq; result = result c; i++; continue }
|
||||
if (!sq && c == "\"") { dq = !dq; result = result c; i++; continue }
|
||||
if (sq || dq) { result = result c; i++; continue }
|
||||
rest = substr($0, i)
|
||||
if (rest ~ /^--message=/) {
|
||||
result = result "--message="; i += 10
|
||||
i = handle_msg($0, n, i); continue
|
||||
}
|
||||
if (rest ~ /^--message([ \t]|$)/) {
|
||||
result = result "--message"; i += 9
|
||||
while (i <= n && substr($0,i,1) ~ /[ \t]/) { result = result substr($0,i,1); i++ }
|
||||
i = handle_msg($0, n, i); continue
|
||||
}
|
||||
if (rest ~ /^-am([ \t]|$)/) {
|
||||
result = result "-am"; i += 3
|
||||
while (i <= n && substr($0,i,1) ~ /[ \t]/) { result = result substr($0,i,1); i++ }
|
||||
i = handle_msg($0, n, i); continue
|
||||
}
|
||||
if (rest ~ /^-m([ \t]|$)/) {
|
||||
result = result "-m"; i += 2
|
||||
while (i <= n && substr($0,i,1) ~ /[ \t]/) { result = result substr($0,i,1); i++ }
|
||||
i = handle_msg($0, n, i); continue
|
||||
}
|
||||
result = result c; i++
|
||||
}
|
||||
printf "%s", result
|
||||
}
|
||||
_AWKEOF
|
||||
)
|
||||
_cmd_masked=$(printf '%s' "$CMD" | awk "$_AWK_MASK_PROG" 2>/dev/null) \
|
||||
&& CMD_MEDIUM="$_cmd_masked" || true
|
||||
CMD_MEDIUM_LOWER=$(printf '%s' "$CMD_MEDIUM" | tr '[:upper:]' '[:lower:]')
|
||||
fi
|
||||
|
||||
@@ -377,6 +377,52 @@ describe('check-careful.sh', () => {
|
||||
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
|
||||
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('SQL TRUNCATE');
|
||||
});
|
||||
|
||||
// Quote-aware awk scanner: a single-quoted message whose content looks like
|
||||
// a flag boundary must not swallow real commands that follow via &&.
|
||||
test('warns when rm follows a single-quoted message that embeds a double-quote', () => {
|
||||
const { exitCode, output } = runHook(
|
||||
CAREFUL_SCRIPT,
|
||||
carefulInput(`git commit -m 'note -m "' && rm -rf build && echo '"'`),
|
||||
);
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
|
||||
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
|
||||
});
|
||||
|
||||
// A double-quoted message containing $(...) is not masked — the substitution
|
||||
// content remains visible to the MEDIUM checks.
|
||||
test('warns when a commit message contains a command substitution with rm', () => {
|
||||
const { exitCode, output } = runHook(
|
||||
CAREFUL_SCRIPT,
|
||||
carefulInput('git commit -m "$(rm -rf build)"'),
|
||||
);
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
|
||||
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
|
||||
});
|
||||
|
||||
// A force-push chained after a commit must still warn even after the message is masked.
|
||||
test('warns on force-push chained after git commit', () => {
|
||||
const { exitCode, output } = runHook(
|
||||
CAREFUL_SCRIPT,
|
||||
carefulInput('git commit -m "x" && git push --force'),
|
||||
);
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
|
||||
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('force-push');
|
||||
});
|
||||
|
||||
// -am shorthand: the message is masked, so "drop table" in the message text
|
||||
// does not trigger the SQL DROP warning.
|
||||
test('does not warn on drop-table text in -am commit message', () => {
|
||||
const { exitCode, output } = runHook(
|
||||
CAREFUL_SCRIPT,
|
||||
carefulInput('git commit -am "drop table notes"'),
|
||||
);
|
||||
expect(exitCode).toBe(0);
|
||||
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// --- Shell obfuscation ---
|
||||
|
||||
Reference in new issue
Block a user