From c2e1978876f4d2bbf86493014cc368feead4b608 Mon Sep 17 00:00:00 2001 From: L4XB Date: Wed, 9 Sep 2026 22:34:41 +0200 Subject: [PATCH 01/11] fix(gateguard): match destructive SQL passed to a database client in a quoted argument The DESTRUCTIVE_SQL_DD phrases (drop table, delete from, truncate) were only ever tested against quote-stripped text, so a real invocation such as `psql -c "drop table users"` or `mysql -e "delete from sessions"`, which must quote the statement, was always allowed. Only the unquoted spelling, which the shell would split into separate arguments, was caught. Keep the quote-stripped scan for the generic case (a commit message that mentions "drop table" still passes), and additionally test the SQL phrases against the unquoted argument text when the segment's executable is a known database client (psql, mysql, mariadb, sqlite3, duckdb, clickhouse-client, mongosh, ...). The check lives in the quote-aware pass, so it also applies behind `sh -c` and in chained segments. Fixes #3024 --- scripts/hooks/gateguard-fact-force.js | 37 ++++++++++++++++++++++++ tests/hooks/gateguard-fact-force.test.js | 33 +++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index bfd2b11c6..e6fdd7267 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -359,6 +359,34 @@ function quoteAwareSegments(input) { const SHELL_WRAPPERS = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh']); +// Interactive database clients that take a SQL statement as an argument +// (`psql -c "..."`, `mysql -e "..."`, `sqlite3 db.sqlite "..."`). A real +// SQL invocation is always quoted, so the generic quote-stripped scan can +// never see it; for these executables the destructive-SQL phrases are +// matched against the (unquoted) argument text instead. +const SQL_CLIENTS = new Set([ + 'psql', + 'pgcli', + 'mysql', + 'mariadb', + 'mycli', + 'sqlite3', + 'sqlite', + 'litecli', + 'duckdb', + 'usql', + 'sqlcmd', + 'clickhouse-client', + 'clickhouse', + 'cockroach', + 'mongosh', + 'mongo', + 'cqlsh', + 'trino', + 'presto', + 'beeline', +]); + /** * Quote-aware destructive check: catches quoted command words, newline * separators, quoted `find -exec`, and `sh -c`/`bash -c` wrappers that evade @@ -376,6 +404,15 @@ function isDestructiveQuoteAware(raw, depth = 0) { if (isDestructiveGit(tokens)) return true; if (isDestructiveFindExec(tokens.join(' '))) return true; const base = commandBasename(tokens[0]); + // SQL passed to a database client lives inside a quoted argument, which + // the generic scan strips on purpose (a commit message that mentions + // "drop table" must not trip the gate). The tokens here carry the + // unquoted argument text, so test the SQL phrases on them when the + // executable is a known SQL client — `git commit -m "drop table ..."` + // still has no SQL client in argv0. + if (SQL_CLIENTS.has(base) && DESTRUCTIVE_SQL_DD.test(tokens.slice(1).join(' '))) { + return true; + } if (SHELL_WRAPPERS.has(base)) { const ci = tokens.indexOf('-c'); if (ci !== -1 && tokens[ci + 1] && isDestructiveQuoteAware(tokens[ci + 1], depth + 1)) { diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 495928691..e0cc22e46 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1520,6 +1520,39 @@ function runTests() { passed++; else failed++; + if ( + test('denies destructive SQL passed to a database client in a quoted argument', () => { + expectDestructiveDeny('psql -c "drop table users"', 'psql -c drop table'); + expectDestructiveDeny("psql -c 'truncate audit_log'", 'psql -c truncate'); + expectDestructiveDeny("psql --command='truncate audit_log'", 'psql --command= truncate'); + expectDestructiveDeny('mysql -e "delete from sessions"', 'mysql -e delete from'); + expectDestructiveDeny('sqlite3 app.db "drop table users"', 'sqlite3 positional drop table'); + expectDestructiveDeny('/usr/local/bin/psql -h db -U app -c "DROP TABLE users"', 'psql with path and flags'); + }) + ) + passed++; + else failed++; + + if ( + test('denies destructive SQL to a database client behind a shell wrapper or chain', () => { + expectDestructiveDeny('sh -c "psql -c \'drop table users\'"', 'psql inside sh -c'); + expectDestructiveDeny('echo migrating && mysql -e "truncate sessions"', 'mysql in second segment'); + }) + ) + passed++; + else failed++; + + if ( + test('allows non-destructive SQL and SQL phrases outside a database client', () => { + expectAllow('psql -c "select count(*) from users"', 'psql select'); + expectAllow('psql -c "delete_from_queue()"', 'psql identifier, not a phrase'); + expectAllow('git commit -m "refactor: drop table indirection"', 'drop table in commit message'); + expectAllow('echo "psql -c \'drop table users\'"', 'echo of a SQL command string'); + }) + ) + passed++; + else failed++; + if ( test('allows destructive SQL prose inside a quoted heredoc', () => { expectAllow( From a0f22b0b6794cfb0cf681bbb8dc8e7e08ef21680 Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 10 Sep 2026 08:42:59 +0200 Subject: [PATCH 02/11] fix(gateguard): classify SQL clients behind prefixes and combined shell flags - Strip execution prefixes (`env NAME=v`, `sudo -u user`, `command`, `nice -n N`, `timeout DURATION`, `stdbuf`, `ionice`, `setsid`, ...) with their options before argv0 is classified, so `env PGHOST=db psql -c "drop table users"` is gated like the bare client. - Recognise a shell's `-c` inside a short-option cluster (`bash -lc`, `sh -ec`) when recursing into the command string. - Skip filename-like words (`truncate.db`, `-f truncate.sql`, `--file=x.sql`) in the SQL phrase scan, both for client arguments and for the flattened command text; `dd if=disk.img` stays matched. --- scripts/hooks/gateguard-fact-force.js | 120 +++++++++++++++++++++-- tests/hooks/gateguard-fact-force.test.js | 20 ++++ 2 files changed, 134 insertions(+), 6 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index e6fdd7267..67405d707 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -359,6 +359,112 @@ function quoteAwareSegments(input) { const SHELL_WRAPPERS = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh']); +// Programs that only run the command following them. They are stripped +// (together with their own options and `NAME=value` assignments) before +// argv0 is classified, so `env PGHOST=db psql -c "..."`, `sudo -u postgres +// psql -c "..."` and `timeout 30 mysql -e "..."` are treated like the bare +// client. `valueOptions` consume the next token; `positionals` are leading +// operands of the prefix itself (`timeout DURATION`, `chroot NEWROOT`). +const EXECUTION_PREFIXES = { + env: { assignments: true, valueOptions: ['-u', '--unset', '-C', '--chdir', '-S', '--split-string'] }, + sudo: { + valueOptions: [ + '-u', '--user', '-g', '--group', '-C', '--close-from', '-D', '--chdir', '-h', '--host', + '-p', '--prompt', '-r', '--role', '-t', '--type', '-T', '--command-timeout', '-U', '--other-user', + ], + }, + doas: { valueOptions: ['-u', '-C'] }, + command: {}, + exec: { valueOptions: ['-a'] }, + nice: { valueOptions: ['-n', '--adjustment'] }, + nohup: {}, + time: {}, + timeout: { valueOptions: ['-s', '--signal', '-k', '--kill-after'], positionals: 1 }, + stdbuf: { valueOptions: ['-i', '-o', '-e', '--input', '--output', '--error'] }, + ionice: { valueOptions: ['-c', '--class', '-n', '--classdata', '-p', '--pid'] }, + setsid: {}, + unshare: {}, + busybox: {}, + runuser: { valueOptions: ['-u', '--user', '-g', '--group', '-G', '--supp-group'] }, + chroot: { positionals: 1 }, + taskset: { valueOptions: ['-c', '--cpu-list'], positionals: 1, positionalUnlessValueOption: true }, +}; + +/** + * Drop leading execution prefixes (`env`, `sudo`, `nice`, ...) with their + * options so the real command sits at index 0. + * + * @param {string[]} tokens + * @returns {string[]} + */ +function stripExecutionPrefixes(tokens) { + let rest = tokens; + for (let guard = 0; guard < 8 && rest.length > 0; guard++) { + const spec = EXECUTION_PREFIXES[commandBasename(rest[0])]; + if (!spec) break; + const valueOptions = new Set(spec.valueOptions || []); + let i = 1; + let consumedValueOption = false; + while (i < rest.length) { + const token = rest[i]; + if (token === '--') { + i++; + break; + } + if (spec.assignments && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) { + i++; + continue; + } + if (token.startsWith('-') && token.length > 1) { + if (valueOptions.has(token)) { + i += 2; + consumedValueOption = true; + } else { + i++; + } + continue; + } + break; + } + let positionals = spec.positionals || 0; + if (spec.positionalUnlessValueOption && consumedValueOption) positionals = 0; + rest = rest.slice(i + positionals); + } + return rest; +} + +// A short-option cluster of a shell that includes `c` (`-c`, `-lc`, `-ec`, +// `-xc`): the next token is the command string the shell executes. +const SHELL_COMMAND_OPTION = /^-[A-Za-z]*c[A-Za-z]*$/; + +// A bare word that carries a path separator or ends in a file extension is +// a filename (`truncate.db`, `psql -f truncate.sql`, `--file=x.sql`), not a +// SQL phrase. `if=...` is kept so `dd if=disk.img` still matches the dd +// pattern. Used on single arguments and on flattened command text. +const FILE_LIKE_ARGUMENT = /^(?!if=)\S*(?:[\\/]\S*|\.[A-Za-z0-9]{1,8})$/; +const FILE_LIKE_TOKEN = /(^|\s)(?!if=)\S*(?:[\\/]\S*|\.[A-Za-z0-9]{1,8})(?=\s|$)/g; + +/** + * Argument text of a SQL client invocation with filename-like tokens removed. + * + * @param {string[]} args + * @returns {string} + */ +function sqlArgumentText(args) { + return args.filter(arg => !FILE_LIKE_ARGUMENT.test(arg)).join(' '); +} + +/** + * Flattened command text with filename-like words removed, for the SQL/dd + * phrase regex. + * + * @param {string} text + * @returns {string} + */ +function stripFileLikeTokens(text) { + return text.replace(FILE_LIKE_TOKEN, '$1'); +} + // Interactive database clients that take a SQL statement as an argument // (`psql -c "..."`, `mysql -e "..."`, `sqlite3 db.sqlite "..."`). A real // SQL invocation is always quoted, so the generic quote-stripped scan can @@ -403,19 +509,21 @@ function isDestructiveQuoteAware(raw, depth = 0) { if (isDestructiveRm(tokens)) return true; if (isDestructiveGit(tokens)) return true; if (isDestructiveFindExec(tokens.join(' '))) return true; - const base = commandBasename(tokens[0]); + const command = stripExecutionPrefixes(tokens); + if (command.length === 0) continue; + const base = commandBasename(command[0]); // SQL passed to a database client lives inside a quoted argument, which // the generic scan strips on purpose (a commit message that mentions // "drop table" must not trip the gate). The tokens here carry the // unquoted argument text, so test the SQL phrases on them when the // executable is a known SQL client — `git commit -m "drop table ..."` // still has no SQL client in argv0. - if (SQL_CLIENTS.has(base) && DESTRUCTIVE_SQL_DD.test(tokens.slice(1).join(' '))) { + if (SQL_CLIENTS.has(base) && DESTRUCTIVE_SQL_DD.test(sqlArgumentText(command.slice(1)))) { return true; } if (SHELL_WRAPPERS.has(base)) { - const ci = tokens.indexOf('-c'); - if (ci !== -1 && tokens[ci + 1] && isDestructiveQuoteAware(tokens[ci + 1], depth + 1)) { + const ci = command.findIndex((token, index) => index > 0 && SHELL_COMMAND_OPTION.test(token)); + if (ci !== -1 && command[ci + 1] && isDestructiveQuoteAware(command[ci + 1], depth + 1)) { return true; } } @@ -737,7 +845,7 @@ function isDestructiveBash(command) { const raw = String(command || ''); const executable = stripHeredocBodies(raw); const flattened = explodeSubshells(stripQuotedStrings(executable)); - if (DESTRUCTIVE_SQL_DD.test(flattened)) return true; + if (DESTRUCTIVE_SQL_DD.test(stripFileLikeTokens(flattened))) return true; // Operator-supplied additional destructive patterns. Same scope as the // built-in SQL/dd regex: matched against the quote-stripped, subshell- @@ -764,7 +872,7 @@ function isDestructiveBash(command) { const segments = bodies.flatMap(splitCommandSegments); for (const segment of segments) { const stripped = stripQuotedStrings(segment); - if (DESTRUCTIVE_SQL_DD.test(stripped)) return true; + if (DESTRUCTIVE_SQL_DD.test(stripFileLikeTokens(stripped))) return true; if (extra && extra.test(stripped)) return true; const tokens = tokenize(segment); if (isDestructiveRm(tokens)) return true; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index e0cc22e46..e2a1fa9c8 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1536,18 +1536,38 @@ function runTests() { if ( test('denies destructive SQL to a database client behind a shell wrapper or chain', () => { expectDestructiveDeny('sh -c "psql -c \'drop table users\'"', 'psql inside sh -c'); + expectDestructiveDeny('bash -lc \'psql -c "drop table users"\'', 'psql inside bash -lc'); + expectDestructiveDeny('sh -ec \'mysql -e "delete from sessions"\'', 'mysql inside sh -ec'); expectDestructiveDeny('echo migrating && mysql -e "truncate sessions"', 'mysql in second segment'); }) ) passed++; else failed++; + if ( + test('denies destructive SQL to a database client behind an execution prefix', () => { + expectDestructiveDeny('env PGHOST=db psql -c "drop table users"', 'env assignment + psql'); + expectDestructiveDeny('env psql -c "drop table users"', 'bare env + psql'); + expectDestructiveDeny('sudo -u postgres psql -c "truncate audit_log"', 'sudo -u + psql'); + expectDestructiveDeny('command mysql -e "delete from sessions"', 'command + mysql'); + expectDestructiveDeny('nice -n 10 psql -c "drop table users"', 'nice -n + psql'); + expectDestructiveDeny('timeout 30 mysql -e "truncate sessions"', 'timeout DURATION + mysql'); + expectDestructiveDeny('sudo env PGHOST=db psql -c "drop table users"', 'stacked prefixes'); + }) + ) + passed++; + else failed++; + if ( test('allows non-destructive SQL and SQL phrases outside a database client', () => { expectAllow('psql -c "select count(*) from users"', 'psql select'); expectAllow('psql -c "delete_from_queue()"', 'psql identifier, not a phrase'); expectAllow('git commit -m "refactor: drop table indirection"', 'drop table in commit message'); expectAllow('echo "psql -c \'drop table users\'"', 'echo of a SQL command string'); + expectAllow('sqlite3 truncate.db "select 1"', 'filename that contains a SQL word'); + expectAllow('psql -f truncate.sql', 'script filename via -f'); + expectAllow('psql --file=./sql/truncate.sql', 'script path via --file='); + expectAllow('env PGHOST=db psql -c "select 1"', 'prefixed non-destructive SQL'); }) ) passed++; From 9968419155734f52718a82e9eb3eb3e49340a9b4 Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 10 Sep 2026 08:57:15 +0200 Subject: [PATCH 03/11] fix(gateguard): scan env -S payloads, model GNU time value options, unbounded prefix stacking --- scripts/hooks/gateguard-fact-force.js | 39 +++++++++++++++++++----- tests/hooks/gateguard-fact-force.test.js | 5 +++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 67405d707..66163c5dc 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -366,7 +366,12 @@ const SHELL_WRAPPERS = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh']); // client. `valueOptions` consume the next token; `positionals` are leading // operands of the prefix itself (`timeout DURATION`, `chroot NEWROOT`). const EXECUTION_PREFIXES = { - env: { assignments: true, valueOptions: ['-u', '--unset', '-C', '--chdir', '-S', '--split-string'] }, + env: { + assignments: true, + valueOptions: ['-u', '--unset', '-C', '--chdir'], + // `env -S ''` splits the string into a command line and runs it. + commandStringOptions: ['-S', '--split-string'], + }, sudo: { valueOptions: [ '-u', '--user', '-g', '--group', '-C', '--close-from', '-D', '--chdir', '-h', '--host', @@ -378,7 +383,7 @@ const EXECUTION_PREFIXES = { exec: { valueOptions: ['-a'] }, nice: { valueOptions: ['-n', '--adjustment'] }, nohup: {}, - time: {}, + time: { valueOptions: ['-f', '--format', '-o', '--output'] }, timeout: { valueOptions: ['-s', '--signal', '-k', '--kill-after'], positionals: 1 }, stdbuf: { valueOptions: ['-i', '-o', '-e', '--input', '--output', '--error'] }, ionice: { valueOptions: ['-c', '--class', '-n', '--classdata', '-p', '--pid'] }, @@ -392,17 +397,22 @@ const EXECUTION_PREFIXES = { /** * Drop leading execution prefixes (`env`, `sudo`, `nice`, ...) with their - * options so the real command sits at index 0. + * options so the real command sits at index 0. Every recognised prefix is + * removed, however many are stacked; each round drops at least one token, + * so the loop terminates. Option payloads that are themselves command lines + * (`env -S ''`) are returned separately for a recursive scan. * * @param {string[]} tokens - * @returns {string[]} + * @returns {{ command: string[], commandStrings: string[] }} */ function stripExecutionPrefixes(tokens) { let rest = tokens; - for (let guard = 0; guard < 8 && rest.length > 0; guard++) { + const commandStrings = []; + while (rest.length > 0) { const spec = EXECUTION_PREFIXES[commandBasename(rest[0])]; if (!spec) break; const valueOptions = new Set(spec.valueOptions || []); + const commandStringOptions = spec.commandStringOptions || []; let i = 1; let consumedValueOption = false; while (i < rest.length) { @@ -416,7 +426,16 @@ function stripExecutionPrefixes(tokens) { continue; } if (token.startsWith('-') && token.length > 1) { - if (valueOptions.has(token)) { + const inline = commandStringOptions.find( + opt => token.startsWith(opt) && token.length > opt.length && (opt.length === 2 || token.charAt(opt.length) === '=') + ); + if (inline) { + commandStrings.push(token.slice(inline.length + (opt => (opt.length === 2 ? 0 : 1))(inline))); + i++; + } else if (commandStringOptions.includes(token)) { + if (rest[i + 1] !== undefined) commandStrings.push(rest[i + 1]); + i += 2; + } else if (valueOptions.has(token)) { i += 2; consumedValueOption = true; } else { @@ -430,7 +449,7 @@ function stripExecutionPrefixes(tokens) { if (spec.positionalUnlessValueOption && consumedValueOption) positionals = 0; rest = rest.slice(i + positionals); } - return rest; + return { command: rest, commandStrings }; } // A short-option cluster of a shell that includes `c` (`-c`, `-lc`, `-ec`, @@ -509,7 +528,11 @@ function isDestructiveQuoteAware(raw, depth = 0) { if (isDestructiveRm(tokens)) return true; if (isDestructiveGit(tokens)) return true; if (isDestructiveFindExec(tokens.join(' '))) return true; - const command = stripExecutionPrefixes(tokens); + const { command, commandStrings } = stripExecutionPrefixes(tokens); + // `env -S ''` executes the string as a command line of its own. + for (const commandString of commandStrings) { + if (isDestructiveQuoteAware(commandString, depth + 1)) return true; + } if (command.length === 0) continue; const base = commandBasename(command[0]); // SQL passed to a database client lives inside a quoted argument, which diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index e2a1fa9c8..1c561cc75 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1553,6 +1553,11 @@ function runTests() { expectDestructiveDeny('nice -n 10 psql -c "drop table users"', 'nice -n + psql'); expectDestructiveDeny('timeout 30 mysql -e "truncate sessions"', 'timeout DURATION + mysql'); expectDestructiveDeny('sudo env PGHOST=db psql -c "drop table users"', 'stacked prefixes'); + expectDestructiveDeny('env -S \'psql -c "drop table users"\'', 'env -S command string'); + expectDestructiveDeny('env --split-string=\'mysql -e "delete from sessions"\'', 'env --split-string= command string'); + expectDestructiveDeny('env time -f "%E" psql -c "drop table users"', 'GNU time -f FORMAT + psql'); + expectDestructiveDeny('/usr/bin/time -o /tmp/t.log psql -c "truncate audit_log"', 'GNU time -o FILE + psql'); + expectDestructiveDeny(`${'env '.repeat(12)}psql -c "drop table users"`, 'twelve stacked prefixes'); }) ) passed++; From 8a933ad9d6653d13965713f2533ad1e182927c23 Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 10 Sep 2026 09:16:24 +0200 Subject: [PATCH 04/11] fix(gateguard): scan runuser command strings, build commandStrings immutably --- scripts/hooks/gateguard-fact-force.js | 13 +++++++++---- tests/hooks/gateguard-fact-force.test.js | 3 +++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 66163c5dc..9d2e98564 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -390,7 +390,11 @@ const EXECUTION_PREFIXES = { setsid: {}, unshare: {}, busybox: {}, - runuser: { valueOptions: ['-u', '--user', '-g', '--group', '-G', '--supp-group'] }, + runuser: { + valueOptions: ['-u', '--user', '-g', '--group', '-G', '--supp-group'], + // `runuser -c ''` hands the string to a shell. + commandStringOptions: ['-c', '--command', '--session-command'], + }, chroot: { positionals: 1 }, taskset: { valueOptions: ['-c', '--cpu-list'], positionals: 1, positionalUnlessValueOption: true }, }; @@ -407,7 +411,7 @@ const EXECUTION_PREFIXES = { */ function stripExecutionPrefixes(tokens) { let rest = tokens; - const commandStrings = []; + let commandStrings = []; while (rest.length > 0) { const spec = EXECUTION_PREFIXES[commandBasename(rest[0])]; if (!spec) break; @@ -430,10 +434,11 @@ function stripExecutionPrefixes(tokens) { opt => token.startsWith(opt) && token.length > opt.length && (opt.length === 2 || token.charAt(opt.length) === '=') ); if (inline) { - commandStrings.push(token.slice(inline.length + (opt => (opt.length === 2 ? 0 : 1))(inline))); + const payload = token.slice(inline.length + (inline.length === 2 ? 0 : 1)); + commandStrings = [...commandStrings, payload]; i++; } else if (commandStringOptions.includes(token)) { - if (rest[i + 1] !== undefined) commandStrings.push(rest[i + 1]); + if (rest[i + 1] !== undefined) commandStrings = [...commandStrings, rest[i + 1]]; i += 2; } else if (valueOptions.has(token)) { i += 2; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 1c561cc75..035b23c03 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1558,6 +1558,9 @@ function runTests() { expectDestructiveDeny('env time -f "%E" psql -c "drop table users"', 'GNU time -f FORMAT + psql'); expectDestructiveDeny('/usr/bin/time -o /tmp/t.log psql -c "truncate audit_log"', 'GNU time -o FILE + psql'); expectDestructiveDeny(`${'env '.repeat(12)}psql -c "drop table users"`, 'twelve stacked prefixes'); + expectDestructiveDeny('runuser -u postgres -c \'psql -c "drop table users"\'', 'runuser -c command string'); + expectDestructiveDeny('runuser -u postgres --command=\'psql -c "truncate audit_log"\'', 'runuser --command= command string'); + expectDestructiveDeny('runuser -u postgres --session-command \'mysql -e "delete from sessions"\'', 'runuser --session-command'); }) ) passed++; From 093be8fcf9e210851c5f991913d2942f4b84eeed Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 10 Sep 2026 09:27:08 +0200 Subject: [PATCH 05/11] fix(gateguard): consume runuser -s/--shell values before the command string --- scripts/hooks/gateguard-fact-force.js | 2 +- tests/hooks/gateguard-fact-force.test.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 9d2e98564..e0b44c972 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -391,7 +391,7 @@ const EXECUTION_PREFIXES = { unshare: {}, busybox: {}, runuser: { - valueOptions: ['-u', '--user', '-g', '--group', '-G', '--supp-group'], + valueOptions: ['-u', '--user', '-g', '--group', '-G', '--supp-group', '-s', '--shell'], // `runuser -c ''` hands the string to a shell. commandStringOptions: ['-c', '--command', '--session-command'], }, diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 035b23c03..69e355f62 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1561,6 +1561,8 @@ function runTests() { expectDestructiveDeny('runuser -u postgres -c \'psql -c "drop table users"\'', 'runuser -c command string'); expectDestructiveDeny('runuser -u postgres --command=\'psql -c "truncate audit_log"\'', 'runuser --command= command string'); expectDestructiveDeny('runuser -u postgres --session-command \'mysql -e "delete from sessions"\'', 'runuser --session-command'); + expectDestructiveDeny('runuser -u postgres -s /usr/local/bin/fish -c \'psql -c "drop table users"\'', 'runuser -s SHELL -c command string'); + expectDestructiveDeny('runuser --shell=/usr/local/bin/fish -u postgres -c \'psql -c "drop table users"\'', 'runuser --shell= -c command string'); }) ) passed++; From e2a69ff5dfa5479d826643e1f16703f9969f43d3 Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 10 Sep 2026 22:16:01 +0200 Subject: [PATCH 06/11] fix(gateguard): fail closed on unmodelled wrapper options and past the nesting limit Three gaps from review: - stripExecutionPrefixes skipped an unknown wrapper option without knowing whether it consumed a value, so `chroot --userspec nobody:nogroup / psql -c "drop table users"` left `/` as the command and the SQL-client check never ran. chroot's --userspec/--groups are modelled now, and an unmodelled option marks the boundary as uncertain, in which case every later token is tried as the command start. - isDestructiveQuoteAware returned false past five nesting levels, so a deep enough wrapper chain hid its payload from the scanner; it now treats an uninspected payload as destructive. - sqlArgumentText dropped only space-free filename-like tokens, so `psql -f "drop table.sql"` was denied for its script name; -f/--file values (separate and attached) are excluded before the SQL phrases are matched. Tests cover runuser --shell with a separate value, chroot with --userspec and with an unmodelled option, the script-with-space allow case, a script plus destructive -c, and the nesting limit. --- scripts/hooks/gateguard-fact-force.js | 79 ++++++++++++++++++------ tests/hooks/gateguard-fact-force.test.js | 20 ++++++ 2 files changed, 79 insertions(+), 20 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index e0b44c972..04e60eb81 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -395,7 +395,7 @@ const EXECUTION_PREFIXES = { // `runuser -c ''` hands the string to a shell. commandStringOptions: ['-c', '--command', '--session-command'], }, - chroot: { positionals: 1 }, + chroot: { valueOptions: ['--userspec', '--groups'], positionals: 1 }, taskset: { valueOptions: ['-c', '--cpu-list'], positionals: 1, positionalUnlessValueOption: true }, }; @@ -412,6 +412,10 @@ const EXECUTION_PREFIXES = { function stripExecutionPrefixes(tokens) { let rest = tokens; let commandStrings = []; + // Set once a wrapper option this table does not model is skipped: it may + // or may not have consumed a value, so where the command starts is no + // longer certain and the caller scans every later token (fail closed). + let inconclusive = false; while (rest.length > 0) { const spec = EXECUTION_PREFIXES[commandBasename(rest[0])]; if (!spec) break; @@ -444,6 +448,8 @@ function stripExecutionPrefixes(tokens) { i += 2; consumedValueOption = true; } else { + // `--opt=value` carries its value; a bare unknown option may not. + if (!(token.startsWith('--') && token.includes('='))) inconclusive = true; i++; } continue; @@ -454,7 +460,7 @@ function stripExecutionPrefixes(tokens) { if (spec.positionalUnlessValueOption && consumedValueOption) positionals = 0; rest = rest.slice(i + positionals); } - return { command: rest, commandStrings }; + return { command: rest, commandStrings, inconclusive }; } // A short-option cluster of a shell that includes `c` (`-c`, `-lc`, `-ec`, @@ -465,6 +471,8 @@ const SHELL_COMMAND_OPTION = /^-[A-Za-z]*c[A-Za-z]*$/; // a filename (`truncate.db`, `psql -f truncate.sql`, `--file=x.sql`), not a // SQL phrase. `if=...` is kept so `dd if=disk.img` still matches the dd // pattern. Used on single arguments and on flattened command text. +// Options of the SQL clients whose next argument is a script file, not SQL. +const SQL_FILE_OPTIONS = new Set(['-f', '--file']); const FILE_LIKE_ARGUMENT = /^(?!if=)\S*(?:[\\/]\S*|\.[A-Za-z0-9]{1,8})$/; const FILE_LIKE_TOKEN = /(^|\s)(?!if=)\S*(?:[\\/]\S*|\.[A-Za-z0-9]{1,8})(?=\s|$)/g; @@ -475,7 +483,20 @@ const FILE_LIKE_TOKEN = /(^|\s)(?!if=)\S*(?:[\\/]\S*|\.[A-Za-z0-9]{1,8})(?=\s|$) * @returns {string} */ function sqlArgumentText(args) { - return args.filter(arg => !FILE_LIKE_ARGUMENT.test(arg)).join(' '); + const kept = []; + for (let i = 0; i < args.length; i++) { + const arg = args[i]; + // A script path given to the client (`-f FILE`, `--file FILE`, `-fFILE`, + // `--file=FILE`) is a filename even when it contains spaces. + if (SQL_FILE_OPTIONS.has(arg)) { + i++; + continue; + } + if (arg.startsWith('--file=') || (arg.startsWith('-f') && arg.length > 2 && !arg.startsWith('--'))) continue; + if (FILE_LIKE_ARGUMENT.test(arg)) continue; + kept.push(arg); + } + return kept.join(' '); } /** @@ -527,33 +548,51 @@ const SQL_CLIENTS = new Set([ * @returns {boolean} */ function isDestructiveQuoteAware(raw, depth = 0) { - if (depth > 4) return false; + // Past the nesting limit the payload is not inspected at all, so it is + // treated as destructive rather than waved through unseen (fail closed). + if (depth > 4) return true; for (const tokens of quoteAwareSegments(raw)) { if (tokens.length === 0) continue; if (isDestructiveRm(tokens)) return true; if (isDestructiveGit(tokens)) return true; if (isDestructiveFindExec(tokens.join(' '))) return true; - const { command, commandStrings } = stripExecutionPrefixes(tokens); + const { command, commandStrings, inconclusive } = stripExecutionPrefixes(tokens); // `env -S ''` executes the string as a command line of its own. for (const commandString of commandStrings) { if (isDestructiveQuoteAware(commandString, depth + 1)) return true; } if (command.length === 0) continue; - const base = commandBasename(command[0]); - // SQL passed to a database client lives inside a quoted argument, which - // the generic scan strips on purpose (a commit message that mentions - // "drop table" must not trip the gate). The tokens here carry the - // unquoted argument text, so test the SQL phrases on them when the - // executable is a known SQL client — `git commit -m "drop table ..."` - // still has no SQL client in argv0. - if (SQL_CLIENTS.has(base) && DESTRUCTIVE_SQL_DD.test(sqlArgumentText(command.slice(1)))) { - return true; + // With an unmodelled wrapper option the command boundary is uncertain, + // so every later token is tried as the command start. + const starts = inconclusive ? command.map((_, index) => index) : [0]; + for (const start of starts) { + if (isDestructiveClientCommand(command.slice(start), depth)) return true; } - if (SHELL_WRAPPERS.has(base)) { - const ci = command.findIndex((token, index) => index > 0 && SHELL_COMMAND_OPTION.test(token)); - if (ci !== -1 && command[ci + 1] && isDestructiveQuoteAware(command[ci + 1], depth + 1)) { - return true; - } + } + return false; +} + +/** + * SQL passed to a database client lives inside a quoted argument, which the + * generic scan strips on purpose (a commit message that mentions "drop + * table" must not trip the gate). The tokens here carry the unquoted + * argument text, so the SQL phrases are tested on them when argv0 is a known + * SQL client — `git commit -m "drop table ..."` still has no client in + * argv0. A shell wrapper hands its `-c` string to a recursive scan. + * + * @param {string[]} command tokens with execution prefixes removed + * @param {number} depth recursion guard, forwarded to nested scans + * @returns {boolean} + */ +function isDestructiveClientCommand(command, depth) { + const base = commandBasename(command[0]); + if (SQL_CLIENTS.has(base) && DESTRUCTIVE_SQL_DD.test(sqlArgumentText(command.slice(1)))) { + return true; + } + if (SHELL_WRAPPERS.has(base)) { + const ci = command.findIndex((token, index) => index > 0 && SHELL_COMMAND_OPTION.test(token)); + if (ci !== -1 && command[ci + 1] && isDestructiveQuoteAware(command[ci + 1], depth + 1)) { + return true; } } return false; @@ -1517,4 +1556,4 @@ function run(rawInput) { return rawInput; // allow } -module.exports = { classifyDestructiveCommand, run }; +module.exports = { classifyDestructiveCommand, run, isDestructiveQuoteAware }; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 69e355f62..316b82a20 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1563,6 +1563,12 @@ function runTests() { expectDestructiveDeny('runuser -u postgres --session-command \'mysql -e "delete from sessions"\'', 'runuser --session-command'); expectDestructiveDeny('runuser -u postgres -s /usr/local/bin/fish -c \'psql -c "drop table users"\'', 'runuser -s SHELL -c command string'); expectDestructiveDeny('runuser --shell=/usr/local/bin/fish -u postgres -c \'psql -c "drop table users"\'', 'runuser --shell= -c command string'); + expectDestructiveDeny( + `runuser --shell /usr/local/bin/fish -u postgres -c 'psql -c "drop table users"'`, + 'runuser --shell separate value + -c command string' + ); + expectDestructiveDeny('chroot --userspec nobody:nogroup / psql -c "drop table users"', 'chroot --userspec VALUE + psql'); + expectDestructiveDeny('chroot --made-up-flag value / psql -c "drop table users"', 'unmodelled wrapper option, fail closed'); }) ) passed++; @@ -1577,12 +1583,26 @@ function runTests() { expectAllow('sqlite3 truncate.db "select 1"', 'filename that contains a SQL word'); expectAllow('psql -f truncate.sql', 'script filename via -f'); expectAllow('psql --file=./sql/truncate.sql', 'script path via --file='); + expectAllow('psql -f "drop table.sql"', 'script filename with a space via -f'); + expectAllow('psql --file "drop table.sql"', 'script filename with a space via --file'); + expectAllow('chroot --userspec nobody:nogroup / ls -la', 'chroot --userspec with a harmless command'); expectAllow('env PGHOST=db psql -c "select 1"', 'prefixed non-destructive SQL'); }) ) passed++; else failed++; + if ( + test('denies a script plus a destructive statement and fails closed past the nesting limit', () => { + expectDestructiveDeny('psql -f schema.sql -c "drop table users"', 'script file plus destructive -c'); + const { isDestructiveQuoteAware } = require(hookScript); + assert.strictEqual(isDestructiveQuoteAware('ls -la', 5), true, 'sixth nesting level is not inspected and fails closed'); + assert.strictEqual(isDestructiveQuoteAware('ls -la', 4), false, 'fifth nesting level is still inspected'); + }) + ) + passed++; + else failed++; + if ( test('allows destructive SQL prose inside a quoted heredoc', () => { expectAllow( From f35470e9b8859ed04f225cb084e00c0776430802 Mon Sep 17 00:00:00 2001 From: L4XB Date: Wed, 23 Sep 2026 15:15:55 +0200 Subject: [PATCH 07/11] fix(gateguard): unwrap the runners and shell forms the SQL-client check still misses #3107 resolves sudo, doas and env before matching a SQL client, and recurses into sh -c. Six shapes still reached a client unseen and were allowed: bash -lc "psql -c 'drop table users'" a -c inside a flag cluster env -S 'psql -c "drop table users"' the command inside one argument env --split-string='...' (same) env time -f "%E" psql -c "..." time, nice, timeout, nohup, nice -n 10 psql -c "..." stdbuf, ionice, setsid, timeout 30 psql -c "..." command, exec as runners Unwrap those runners with their options and positionals, stacked without a fixed limit, scan env -S payloads as command lines, and accept a -c inside a short-option cluster. Past the shell recursion limit the check now fails closed: five levels of sh -c used to hide a drop table. --- scripts/hooks/gateguard-fact-force.js | 86 ++++++++++++++++++++++-- tests/hooks/gateguard-fact-force.test.js | 39 +++++++++++ 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 6756a0b79..587e48739 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -421,17 +421,34 @@ const SUDO_VALUE_FLAGS = new Set([ '--command-timeout', ]); +// Programs that only run the command after them. `valueOptions` consume the +// next token; `positionals` are leading operands of the program itself +// (`timeout DURATION`). sudo, doas and env keep their own handling below. +const RUNNER_PREFIXES = { + command: {}, + exec: { valueOptions: ['-a'] }, + ionice: { valueOptions: ['-c', '--class', '-n', '--classdata', '-p', '--pid'] }, + nice: { valueOptions: ['-n', '--adjustment'] }, + nohup: {}, + setsid: {}, + stdbuf: { valueOptions: ['-i', '-o', '-e', '--input', '--output', '--error'] }, + time: { valueOptions: ['-f', '--format', '-o', '--output'] }, + timeout: { valueOptions: ['-s', '--signal', '-k', '--kill-after'], positionals: 1 } +}; + /** * Advance past `sudo`/`doas`/`env` wrappers including their flags and - * `VAR=value` assignments, so `sudo -u postgres psql ...` and - * `env PGUSER=postgres psql ...` still resolve to the real command. + * `VAR=value` assignments, and past the runners in {@link RUNNER_PREFIXES}, + * so `sudo -u postgres psql ...`, `env PGUSER=postgres psql ...` and + * `timeout 30 psql ...` still resolve to the real command. Wrappers stack + * without a fixed limit: every one consumes at least one token. * * @param {string[]} tokens dequoted tokens for one segment * @returns {number} index of the real command token */ function unwrapLeadWrappers(tokens) { let index = 0; - for (let guard = 0; guard < 4; guard += 1) { + for (let guard = 0; guard < tokens.length; guard += 1) { if (index >= tokens.length) return index; const base = commandBasename(tokens[index]); if (base === 'sudo' || base === 'doas') { @@ -487,11 +504,63 @@ function unwrapLeadWrappers(tokens) { } continue; } + const runner = RUNNER_PREFIXES[base]; + if (runner) { + const valueOptions = runner.valueOptions || []; + index += 1; + while (index < tokens.length) { + const arg = tokens[index]; + if (arg === '--') { + index += 1; + break; + } + if (arg === '-' || !arg.startsWith('-')) break; + index += valueOptions.includes(arg) ? 2 : 1; + } + index += runner.positionals || 0; + continue; + } break; } return index; } +/** + * Command lines that `env -S` / `env --split-string` split and run. The + * whole command hides inside one argument (`env -S 'psql -c "..."'`), so + * each payload is scanned as a command line of its own. + * + * @param {string[]} tokens dequoted tokens for one segment + * @returns {string[]} the command lines, possibly none + */ +function envSplitStrings(tokens) { + const payloads = []; + for (let i = 0; i < tokens.length; i += 1) { + if (commandBasename(tokens[i]) !== 'env') continue; + for (let j = i + 1; j < tokens.length; j += 1) { + const arg = tokens[j]; + if (arg === '-S' || arg === '--split-string') { + if (j + 1 < tokens.length) payloads.push(tokens.slice(j + 1).join(' ')); + break; + } + if (arg.startsWith('--split-string=')) { + payloads.push([arg.slice('--split-string='.length), ...tokens.slice(j + 1)].join(' ')); + break; + } + if (/^-S./.test(arg)) { + payloads.push([arg.slice(2), ...tokens.slice(j + 1)].join(' ')); + break; + } + if (arg === '-u' || arg === '--unset' || arg === '-C' || arg === '--chdir') { + j += 1; + continue; + } + if (!arg.startsWith('-') && !/^[A-Za-z_][A-Za-z0-9_]*=/.test(arg)) break; + } + } + return payloads; +} + /** * Detect destructive SQL passed as (possibly quoted) arguments to a known * SQL client. Operates on dequoted tokens from `quoteAwareSegments`, so @@ -513,22 +582,29 @@ function isDestructiveSqlClient(tokens) { * separators, quoted `find -exec`, and `sh -c`/`bash -c` wrappers that evade * the quote-stripping path (GHSA-4v57-ph3x-gf55). * + * Past the recursion limit the nested command is not visible, so the check + * fails closed: a guard that allowed it would be bypassed by one more level. + * * @param {string} raw * @param {number} [depth] recursion guard for shell -c wrappers * @returns {boolean} */ function isDestructiveQuoteAware(raw, depth = 0) { - if (depth > 4) return false; + if (depth > 4) return true; for (const tokens of quoteAwareSegments(raw)) { if (tokens.length === 0) continue; if (isDestructiveRm(tokens)) return true; if (isDestructiveGit(tokens)) return true; if (isDestructiveSqlClient(tokens)) return true; if (isDestructiveFindExec(tokens.join(' '))) return true; + for (const payload of envSplitStrings(tokens)) { + if (isDestructiveQuoteAware(payload, depth + 1)) return true; + } const wi = unwrapLeadWrappers(tokens); const base = wi < tokens.length ? commandBasename(tokens[wi]) : ''; if (SHELL_WRAPPERS.has(base)) { - const ci = tokens.indexOf('-c', wi); + // `-c`, or a short-option cluster that includes it (`-lc`, `-ec`). + const ci = tokens.findIndex((token, i) => i > wi && /^-[A-Za-z]*c[A-Za-z]*$/.test(token)); if (ci !== -1 && tokens[ci + 1] && isDestructiveQuoteAware(tokens[ci + 1], depth + 1)) { return true; } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 9e6a18334..2496182ff 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1539,6 +1539,45 @@ function runTests() { passed++; else failed++; + if ( + test('denies destructive SQL through runners and shell flag clusters', () => { + expectDestructiveDeny('bash -lc "psql -c \'drop table users\'"', 'bash -lc psql'); + expectDestructiveDeny('sh -ec "psql -c \'drop table users\'"', 'sh -ec psql'); + expectDestructiveDeny('env -S \'psql -c "drop table users"\'', 'env -S psql'); + expectDestructiveDeny('env --split-string=\'psql -c "drop table users"\'', 'env --split-string psql'); + expectDestructiveDeny('env time -f "%E" psql -c "drop table users"', 'env time -f psql'); + expectDestructiveDeny('nice -n 10 psql -c "drop table users"', 'nice psql'); + expectDestructiveDeny('timeout 30 psql -c "drop table users"', 'timeout psql'); + expectDestructiveDeny('nohup mysql -e "delete from sessions"', 'nohup mysql'); + expectDestructiveDeny('sudo nice -n 5 timeout -s KILL 30 psql -c "drop table users"', 'stacked runners psql'); + }) + ) + passed++; + else failed++; + + if ( + test('denies a command nested past the shell -c recursion limit', () => { + // Each level quotes the one inside it. Past the limit the command is not + // visible, so the check fails closed instead of allowing it. + let command = 'psql -c "drop table users"'; + for (let level = 0; level < 6; level += 1) command = `sh -c ${JSON.stringify(command)}`; + expectDestructiveDeny(command, 'sh -c nested six deep'); + }) + ) + passed++; + else failed++; + + if ( + test('allows harmless commands under the runners and flag clusters', () => { + expectAllow('bash -lc "psql -c \'select count(*) from users\'"', 'bash -lc select'); + expectAllow('nice -n 10 psql -c "select 1"', 'nice select'); + expectAllow('timeout 30 git status', 'timeout git status'); + expectAllow('env -S \'psql -c "select 1"\'', 'env -S select'); + }) + ) + passed++; + else failed++; + if ( test('allows SQL string literals and non-SQL clients mentioning SQL', () => { expectAllow('psql -c "SELECT \'drop table\' FROM audit_log"', 'SQL string literal'); From 64e004a9625cc8752edc9bd43f712cb8a743ef3a Mon Sep 17 00:00:00 2001 From: L4XB Date: Wed, 23 Sep 2026 21:04:38 +0200 Subject: [PATCH 08/11] fix(hooks): keep the arguments after env -S whole GNU env -S splits only its own argument; the ones after it reach the command unchanged. Joining them into the split string let a quoted ';' inside a later argument read as a command separator, so env -S 'echo' '; psql -c "drop table users"' was denied. Each later argument is now escaped back into a single literal word, which still leaves env -S 'psql -c' 'drop table users' denied. --- scripts/hooks/gateguard-fact-force.js | 13 ++++++++++--- tests/hooks/gateguard-fact-force.test.js | 2 ++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 587e48739..48a9d466d 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -533,6 +533,13 @@ function unwrapLeadWrappers(tokens) { * @param {string[]} tokens dequoted tokens for one segment * @returns {string[]} the command lines, possibly none */ +// `env -S` splits only its own argument; later arguments reach the command +// whole. Escaping every character makes the classifier read each one back as a +// single literal word, so a `;` inside one cannot become a separator. +function asLiteralWord(token) { + return token === '' ? "''" : token.replace(/[\s\S]/gu, ch => `\\${ch}`); +} + function envSplitStrings(tokens) { const payloads = []; for (let i = 0; i < tokens.length; i += 1) { @@ -540,15 +547,15 @@ function envSplitStrings(tokens) { for (let j = i + 1; j < tokens.length; j += 1) { const arg = tokens[j]; if (arg === '-S' || arg === '--split-string') { - if (j + 1 < tokens.length) payloads.push(tokens.slice(j + 1).join(' ')); + if (j + 1 < tokens.length) payloads.push([tokens[j + 1], ...tokens.slice(j + 2).map(asLiteralWord)].join(' ')); break; } if (arg.startsWith('--split-string=')) { - payloads.push([arg.slice('--split-string='.length), ...tokens.slice(j + 1)].join(' ')); + payloads.push([arg.slice('--split-string='.length), ...tokens.slice(j + 1).map(asLiteralWord)].join(' ')); break; } if (/^-S./.test(arg)) { - payloads.push([arg.slice(2), ...tokens.slice(j + 1)].join(' ')); + payloads.push([arg.slice(2), ...tokens.slice(j + 1).map(asLiteralWord)].join(' ')); break; } if (arg === '-u' || arg === '--unset' || arg === '-C' || arg === '--chdir') { diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 2496182ff..dd8488085 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1545,6 +1545,7 @@ function runTests() { expectDestructiveDeny('sh -ec "psql -c \'drop table users\'"', 'sh -ec psql'); expectDestructiveDeny('env -S \'psql -c "drop table users"\'', 'env -S psql'); expectDestructiveDeny('env --split-string=\'psql -c "drop table users"\'', 'env --split-string psql'); + expectDestructiveDeny('env -S \'psql -c\' \'drop table users\'', 'env -S psql with the SQL as a later argument'); expectDestructiveDeny('env time -f "%E" psql -c "drop table users"', 'env time -f psql'); expectDestructiveDeny('nice -n 10 psql -c "drop table users"', 'nice psql'); expectDestructiveDeny('timeout 30 psql -c "drop table users"', 'timeout psql'); @@ -1573,6 +1574,7 @@ function runTests() { expectAllow('nice -n 10 psql -c "select 1"', 'nice select'); expectAllow('timeout 30 git status', 'timeout git status'); expectAllow('env -S \'psql -c "select 1"\'', 'env -S select'); + expectAllow('env -S \'echo\' \'; psql -c "drop table users"\'', 'env -S echo with a literal argument'); }) ) passed++; From bbe5d784201b9b67dca0ad092f673e2b5b4db046 Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 24 Sep 2026 22:39:44 +0200 Subject: [PATCH 09/11] fix(gateguard): read the command line su runs, and unwrap taskset `su postgres -c "psql -c 'drop table users'"` resolved to `su`, so the SQL-client check never saw psql. The value of su's -c/--command (or of a short-option cluster ending in c, as in -lc) is now scanned as a command line of its own, like the value of sh -c. The scan continues past `--`, since su hands the arguments after it to the target shell, which runs a -c there too. taskset joins the runners: its -c is a flag and the CPU mask is one positional, so `taskset -c 0 psql ...` resolves to psql. --- scripts/hooks/gateguard-fact-force.js | 24 ++++++++++++++++++++++++ tests/hooks/gateguard-fact-force.test.js | 8 ++++++++ 2 files changed, 32 insertions(+) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 48a9d466d..d7106c6dd 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -359,6 +359,25 @@ function quoteAwareSegments(input) { const SHELL_WRAPPERS = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh']); +/** + * The command line `su` runs through the target user's shell: the value of + * `-c`/`--command`, or of a short-option cluster ending in `c` (`-lc`). `su` + * passes the arguments after `--` to that shell, which runs a `-c` there too, + * so the scan does not stop at `--`. + * + * @param {string[]} tokens dequoted tokens for one segment + * @param {number} start index of the `su` token + * @returns {string|null} + */ +function suCommandLine(tokens, start) { + for (let i = start + 1; i < tokens.length; i += 1) { + const arg = tokens[i]; + if (arg.startsWith('--command=')) return arg.slice('--command='.length); + if (arg === '--command' || /^-[A-Za-z]*c$/.test(arg)) return i + 1 < tokens.length ? tokens[i + 1] : null; + } + return null; +} + /** * SQL clients whose `-c`/`-e`/positional arguments carry SQL statements. * Quoted SQL (e.g. `psql -c "drop table users"`) is invisible to the @@ -432,6 +451,7 @@ const RUNNER_PREFIXES = { nohup: {}, setsid: {}, stdbuf: { valueOptions: ['-i', '-o', '-e', '--input', '--output', '--error'] }, + taskset: { positionals: 1 }, time: { valueOptions: ['-f', '--format', '-o', '--output'] }, timeout: { valueOptions: ['-s', '--signal', '-k', '--kill-after'], positionals: 1 } }; @@ -616,6 +636,10 @@ function isDestructiveQuoteAware(raw, depth = 0) { return true; } } + if (base === 'su') { + const commandLine = suCommandLine(tokens, wi); + if (commandLine && isDestructiveQuoteAware(commandLine, depth + 1)) return true; + } } return false; } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index dd8488085..8dddc21c5 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1551,6 +1551,12 @@ function runTests() { expectDestructiveDeny('timeout 30 psql -c "drop table users"', 'timeout psql'); expectDestructiveDeny('nohup mysql -e "delete from sessions"', 'nohup mysql'); expectDestructiveDeny('sudo nice -n 5 timeout -s KILL 30 psql -c "drop table users"', 'stacked runners psql'); + expectDestructiveDeny('sudo su postgres -c "psql -c \'drop table users\'"', 'sudo su -c psql'); + expectDestructiveDeny('su - postgres -c "psql -c \'drop table users\'"', 'su - user -c psql'); + expectDestructiveDeny('su -lc "psql -c \'drop table users\'" postgres', 'su -lc psql'); + expectDestructiveDeny('su --command="psql -c \'drop table users\'" postgres', 'su --command= psql'); + expectDestructiveDeny('su postgres -- -c "psql -c \'drop table users\'"', 'su -c after --'); + expectDestructiveDeny('taskset -c 0 psql -c "drop table users"', 'taskset -c psql'); }) ) passed++; @@ -1573,6 +1579,8 @@ function runTests() { expectAllow('bash -lc "psql -c \'select count(*) from users\'"', 'bash -lc select'); expectAllow('nice -n 10 psql -c "select 1"', 'nice select'); expectAllow('timeout 30 git status', 'timeout git status'); + expectAllow('su postgres -c "psql -c \'select 1\'"', 'su -c select'); + expectAllow('taskset -c 0 git status', 'taskset git status'); expectAllow('env -S \'psql -c "select 1"\'', 'env -S select'); expectAllow('env -S \'echo\' \'; psql -c "drop table users"\'', 'env -S echo with a literal argument'); }) From 2df1aa7a92667423c827aa99ff397c247c843cfb Mon Sep 17 00:00:00 2001 From: L4XB Date: Thu, 24 Sep 2026 23:11:54 +0200 Subject: [PATCH 10/11] fix(gateguard): check every command su is given, not only the first `su postgres -c "echo ok" -c "psql -c 'drop table users'"` runs the last -c, but only the first was scanned, so the drop went through. Every -c/--command value is now scanned as a command line of its own. --- scripts/hooks/gateguard-fact-force.js | 29 +++++++++++++++--------- tests/hooks/gateguard-fact-force.test.js | 2 ++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index d7106c6dd..329b3ae16 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -360,22 +360,28 @@ function quoteAwareSegments(input) { const SHELL_WRAPPERS = new Set(['sh', 'bash', 'zsh', 'dash', 'ksh']); /** - * The command line `su` runs through the target user's shell: the value of - * `-c`/`--command`, or of a short-option cluster ending in `c` (`-lc`). `su` - * passes the arguments after `--` to that shell, which runs a `-c` there too, - * so the scan does not stop at `--`. + * The command lines `su` may run through the target user's shell: the value of + * every `-c`/`--command`, or of a short-option cluster ending in `c` (`-lc`). + * `su` runs only the last one, so each is checked rather than guessing which + * wins. `su` passes the arguments after `--` to that shell, which runs a `-c` + * there too, so the scan does not stop at `--`. * * @param {string[]} tokens dequoted tokens for one segment * @param {number} start index of the `su` token - * @returns {string|null} + * @returns {string[]} */ -function suCommandLine(tokens, start) { +function suCommandLines(tokens, start) { + const commandLines = []; for (let i = start + 1; i < tokens.length; i += 1) { const arg = tokens[i]; - if (arg.startsWith('--command=')) return arg.slice('--command='.length); - if (arg === '--command' || /^-[A-Za-z]*c$/.test(arg)) return i + 1 < tokens.length ? tokens[i + 1] : null; + if (arg.startsWith('--command=')) { + commandLines.push(arg.slice('--command='.length)); + } else if ((arg === '--command' || /^-[A-Za-z]*c$/.test(arg)) && i + 1 < tokens.length) { + commandLines.push(tokens[i + 1]); + i += 1; + } } - return null; + return commandLines; } /** @@ -637,8 +643,9 @@ function isDestructiveQuoteAware(raw, depth = 0) { } } if (base === 'su') { - const commandLine = suCommandLine(tokens, wi); - if (commandLine && isDestructiveQuoteAware(commandLine, depth + 1)) return true; + for (const commandLine of suCommandLines(tokens, wi)) { + if (isDestructiveQuoteAware(commandLine, depth + 1)) return true; + } } } return false; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 8dddc21c5..32404f81a 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1556,6 +1556,8 @@ function runTests() { expectDestructiveDeny('su -lc "psql -c \'drop table users\'" postgres', 'su -lc psql'); expectDestructiveDeny('su --command="psql -c \'drop table users\'" postgres', 'su --command= psql'); expectDestructiveDeny('su postgres -- -c "psql -c \'drop table users\'"', 'su -c after --'); + expectDestructiveDeny('su postgres -c "echo ok" -c "psql -c \'drop table users\'"', 'su runs its last -c'); + expectDestructiveDeny('su -c "psql -c \'drop table users\'" -c "echo ok" postgres', 'su -c with a later harmless -c'); expectDestructiveDeny('taskset -c 0 psql -c "drop table users"', 'taskset -c psql'); }) ) From f838085274be4f35ba0c6ec7cf82912320b5fec4 Mon Sep 17 00:00:00 2001 From: L4XB Date: Mon, 5 Oct 2026 15:56:53 +0200 Subject: [PATCH 11/11] fix(gateguard): reach SQL clients behind shell flag clusters, su and taskset main resolves SQL clients behind env -S and its launcher grammars, but a destructive statement still passed under four forms: - a shell -c inside a short-option cluster (bash -lc, sh -ec), since only an exact -c was looked up; - su, whose -c / --command / -lc value runs through the target user's shell, before or after --; - taskset, whose mask or CPU list precedes the command, like timeout's duration; - a command nested past the shell -c recursion limit, which was allowed instead of denied. The first three are checked like sh -c; past the limit the check now fails closed. --- scripts/hooks/gateguard-fact-force.js | 49 +++++++++++++++++++++--- tests/hooks/gateguard-fact-force.test.js | 40 +++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index b23055b72..aae94c081 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -581,6 +581,10 @@ const DD_LAUNCHER_OPTIONS = { setsid: { values: new Set(), optional: new Set(), flags: new Set(['-c', '--ctty', '-f', '--fork', '-w', '--wait']) + }, + taskset: { + values: new Set(), optional: new Set(), + flags: new Set(['-a', '--all-tasks', '-c', '--cpu-list']) } }; @@ -633,8 +637,9 @@ function ddLauncherCommandIndex(argv, index, name) { } index += consumesNext ? 2 : 1; } - // timeout's duration is data, followed by exactly one executable position. - return name === 'timeout' ? index + 1 : index; + // timeout's duration and taskset's mask or CPU list are data, followed by + // exactly one executable position. + return name === 'timeout' || name === 'taskset' ? index + 1 : index; } /** @@ -767,6 +772,30 @@ function unwrapLeadWrappers(tokens, allowShellBuiltins = true, allowDdLaunchers return []; } +/** + * The command lines `su` may run through the target user's shell: the value of + * every `-c`/`--command`, or of a short-option cluster ending in `c` (`-lc`). + * `su` runs only the last one, so each is checked rather than guessing which + * wins. `su` passes the arguments after `--` to that shell, which runs a `-c` + * there too, so the scan does not stop at `--`. + * + * @param {string[]} argv command argv starting at `su` + * @returns {string[]} + */ +function suCommandLines(argv) { + const commandLines = []; + for (let i = 1; i < argv.length; i += 1) { + const arg = argv[i]; + if (arg.startsWith('--command=')) { + commandLines.push(arg.slice('--command='.length)); + } else if ((arg === '--command' || /^-[A-Za-z]*c$/.test(arg)) && i + 1 < argv.length) { + commandLines.push(argv[i + 1]); + i += 1; + } + } + return commandLines; +} + /** * Detect destructive SQL passed as (possibly quoted) arguments to a known * SQL client. Operates on dequoted tokens from `quoteAwareSegments`, so @@ -787,12 +816,15 @@ function isDestructiveSqlClient(tokens) { * separators, quoted `find -exec`, and `sh -c`/`bash -c` wrappers that evade * the quote-stripping path (GHSA-4v57-ph3x-gf55). * + * Past the recursion limit the nested command is not visible, so the check + * fails closed: a guard that allowed it would be bypassed by one more level. + * * @param {string} raw * @param {number} [depth] recursion guard for shell -c wrappers * @returns {boolean} */ function isDestructiveQuoteAware(raw, depth = 0) { - if (depth > 4) return false; + if (depth > 4) return true; // The outer command was preprocessed already; shell -c introduces a new // program whose literal heredoc data must also stay outside execution. const executable = depth === 0 ? raw : stripHeredocBodies(raw); @@ -805,12 +837,19 @@ function isDestructiveQuoteAware(raw, depth = 0) { if (isDestructiveSqlClient(tokens)) return true; if (isDestructiveFindExec(tokens)) return true; const argv = unwrapLeadWrappers(tokens, true, true); - if (SHELL_WRAPPERS.has(commandBasename(argv[0]))) { - const ci = argv.indexOf('-c', 1); + const base = commandBasename(argv[0]); + if (SHELL_WRAPPERS.has(base)) { + // `-c`, or a short-option cluster that includes it (`-lc`, `-ec`). + const ci = argv.findIndex((arg, i) => i > 0 && /^-[A-Za-z]*c[A-Za-z]*$/.test(arg)); if (ci !== -1 && argv[ci + 1] && isDestructiveQuoteAware(argv[ci + 1], depth + 1)) { return true; } } + if (base === 'su') { + for (const commandLine of suCommandLines(argv)) { + if (isDestructiveQuoteAware(commandLine, depth + 1)) return true; + } + } } } return false; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 3bd3ba797..fc7b6570a 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -2195,6 +2195,46 @@ function runTests() { passed++; else failed++; + if ( + test('denies destructive SQL through shell flag clusters, su and taskset', () => { + expectDestructiveDeny('bash -lc "psql -c \'drop table users\'"', 'bash -lc psql'); + expectDestructiveDeny('sh -ec "psql -c \'drop table users\'"', 'sh -ec psql'); + expectDestructiveDeny('sudo su postgres -c "psql -c \'drop table users\'"', 'sudo su -c psql'); + expectDestructiveDeny('su - postgres -c "psql -c \'drop table users\'"', 'su - user -c psql'); + expectDestructiveDeny('su -lc "psql -c \'drop table users\'" postgres', 'su -lc psql'); + expectDestructiveDeny('su --command="psql -c \'drop table users\'" postgres', 'su --command= psql'); + expectDestructiveDeny('su postgres -- -c "psql -c \'drop table users\'"', 'su -c after --'); + expectDestructiveDeny('su postgres -c "echo ok" -c "psql -c \'drop table users\'"', 'su runs its last -c'); + expectDestructiveDeny('su -c "psql -c \'drop table users\'" -c "echo ok" postgres', 'su -c with a later harmless -c'); + expectDestructiveDeny('taskset -c 0 psql -c "drop table users"', 'taskset -c psql'); + expectDestructiveDeny('taskset 0x3 psql -c "drop table users"', 'taskset mask psql'); + }) + ) + passed++; + else failed++; + + if ( + test('denies a command nested past the shell -c recursion limit', () => { + // Each level quotes the one inside it. Past the limit the command is not + // visible, so the check fails closed instead of allowing it. + let command = 'psql -c "drop table users"'; + for (let level = 0; level < 6; level += 1) command = `sh -c ${JSON.stringify(command)}`; + expectDestructiveDeny(command, 'sh -c nested six deep'); + }) + ) + passed++; + else failed++; + + if ( + test('allows harmless commands under shell flag clusters, su and taskset', () => { + expectAllow('bash -lc "psql -c \'select count(*) from users\'"', 'bash -lc select'); + expectAllow('su postgres -c "psql -c \'select 1\'"', 'su -c select'); + expectAllow('taskset -c 0 git status', 'taskset git status'); + }) + ) + passed++; + else failed++; + if ( test('allows SQL string literals and non-SQL clients mentioning SQL', () => { expectAllow('psql -c "SELECT \'drop table\' FROM audit_log"', 'SQL string literal');