mirror of
https://github.com/affaan-m/ECC.git
synced 2026-09-29 21:15:16 +02:00
fix(gateguard): deny dd whose input path is not word-initial
DESTRUCTIVE_SQL_DD shared one trailing \b across every alternation arm. `dd\s+if=` ends in `=`, and a \b after a non-word character only holds when the NEXT character is a word character, so the arm matched `dd if=x` and missed every path starting with `/`, `.` or a quote: dd if=/dev/zero of=/dev/sda allowed dd if=./disk.img of=/dev/sdb allowed dd if="/dev/zero" of=/dev/sda allowed The word-boundary suffix now applies only to the arms that end in a word character. The split is what keeps the widening bounded: dropping the trailing \b outright would let `truncate` match `truncated`, and dropping the leading \b would let `dd if=` match inside `add if=`. Both are covered. This is the half of #2642 that survived the structural findGitSubcommand() parser, which already handles `git checkout -- .`. Not addressed here: `dd of=/dev/sda if=/dev/zero` with the operands reversed is still allowed, before and after, because the pattern requires `if=` immediately after `dd`. That is a different defect from the boundary bug and widening a P0 gate's pattern shape is a maintainer call. Refs #2642
This commit is contained in:
@@ -53,7 +53,11 @@ const ECC_ENABLE_VALUES = new Set(['1', 'true', 'on', 'enabled', 'enable', 'yes'
|
||||
// phrases without shell-flag ordering concerns. Quoted strings are
|
||||
// stripped before this regex runs so a commit message mentioning
|
||||
// "drop table" no longer triggers a false positive.
|
||||
const DESTRUCTIVE_SQL_DD = /\b(drop\s+table|delete\s+from|truncate|dd\s+if=)\b/i;
|
||||
// The trailing \b applies only to the arms that end in a word character.
|
||||
// `dd\s+if=` ends in `=`, so a shared \b demanded that the NEXT character be a
|
||||
// word character and the disk-wipe spellings slipped through: `dd if=/dev/zero`
|
||||
// and `dd if=./img` were allowed while `dd if=x` was denied (#2642).
|
||||
const DESTRUCTIVE_SQL_DD = /\b(?:drop\s+table|delete\s+from|truncate)\b|\bdd\s+if=/i;
|
||||
|
||||
// Operator-supplied additional destructive patterns. Lazily compiled from
|
||||
// `GATEGUARD_BASH_EXTRA_DESTRUCTIVE` (regex source) on first use, then
|
||||
|
||||
@@ -255,6 +255,67 @@ function runTests() {
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
// --- Test 4b: dd targets that do not start with a word character ---
|
||||
/**
|
||||
* #2642: DESTRUCTIVE_SQL_DD carried one trailing \b across every alternation
|
||||
* arm. `dd\s+if=` ends in `=`, so that \b demanded the NEXT character be a
|
||||
* word character: `dd if=x` was denied while the disk-wipe spelling
|
||||
* `dd if=/dev/zero of=/dev/sda` and the relative `dd if=./img` were allowed.
|
||||
* These run through the real hook, since the report is specifically that the
|
||||
* published hook lets the slash-prefixed form through.
|
||||
*/
|
||||
for (const command of [
|
||||
'dd if=/dev/zero of=/dev/sda',
|
||||
'dd if=./disk.img of=/dev/sdb',
|
||||
'dd if="/dev/zero" of=/dev/sda'
|
||||
]) {
|
||||
clearState();
|
||||
if (
|
||||
test(`denies dd whose input path is not word-initial: ${command}`, () => {
|
||||
const result = runBashHook({ tool_name: 'Bash', tool_input: { command } });
|
||||
const output = parseOutput(result.stdout);
|
||||
assert.ok(output, 'hook should produce JSON output');
|
||||
assert.ok(output.hookSpecificOutput, 'hook should return a permission decision');
|
||||
assert.strictEqual(
|
||||
output.hookSpecificOutput.permissionDecision,
|
||||
'deny',
|
||||
`${command} must be gated as destructive`
|
||||
);
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Destructive'));
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
}
|
||||
|
||||
// --- Test 4c: widening the dd arm must not gate ordinary commands ---
|
||||
/**
|
||||
* The fix drops the trailing \b only from the dd arm, so the arms that end in
|
||||
* a word character keep theirs. Without that split, `truncated` would match
|
||||
* `truncate`, and `add if=` would match `dd if=`.
|
||||
*/
|
||||
for (const command of ['echo add if=1', 'echo truncated output', 'git status']) {
|
||||
clearState();
|
||||
if (
|
||||
test(`does not gate as destructive: ${command}`, () => {
|
||||
// Prime the session so the separate first-command routine gate cannot
|
||||
// be mistaken for a destructive denial.
|
||||
runBashHook({ tool_name: 'Bash', tool_input: { command: 'printf ready' } });
|
||||
const result = runBashHook({ tool_name: 'Bash', tool_input: { command } });
|
||||
const output = parseOutput(result.stdout);
|
||||
if (output && output.hookSpecificOutput) {
|
||||
const reason = output.hookSpecificOutput.permissionDecisionReason || '';
|
||||
assert.ok(
|
||||
output.hookSpecificOutput.permissionDecision !== 'deny' || !reason.includes('Destructive'),
|
||||
`${command} must not be gated as destructive`
|
||||
);
|
||||
}
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
}
|
||||
|
||||
// --- Test 5: denies first routine Bash, allows second ---
|
||||
clearState();
|
||||
if (
|
||||
|
||||
Reference in New Issue
Block a user