From c2e1978876f4d2bbf86493014cc368feead4b608 Mon Sep 17 00:00:00 2001 From: L4XB Date: Wed, 9 Sep 2026 22:34:41 +0200 Subject: [PATCH] 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(