fix(gateguard): detect dd by command word, not by text match

Review on #2829 found the regex fix widened a false positive: matching
`\bdd\s+if=` against the whole flattened line gated `echo dd if=/dev/zero`
and `grep dd if=/dev/zero file`, neither of which runs dd. That class was
already present before — `echo dd if=x` matched the old arm too — but the
boundary fix extended it to the slash and dot spellings, so the arm now
decides on text position rather than on what is being executed.

dd moves to isDestructiveDd(tokens), next to isDestructiveRm and
isDestructiveGit, and DESTRUCTIVE_SQL_DD goes back to SQL only. The
per-segment loop already tokenizes every executable body, so the check runs
where the command word is known.

This resolves four things the text match could not:

  dd if=/dev/zero of=/dev/sda    was allowed  -> denied  (the reported bug)
  sudo dd if=/dev/zero           was allowed  -> denied
  dd of=/dev/sda if=/dev/zero    was allowed  -> denied  (operands are order-free)
  echo dd if=x                   was denied   -> allowed (pre-existing false positive)

Leading sudo/doas/env, their flags, and VAR=value assignment prefixes are
skipped so a wrapped invocation still resolves to dd; flags are only skipped
once a wrapper has been seen, so the scan cannot walk into an unrelated
command's arguments.

Tests: 6 fail on upstream main, 155 pass with this change.

Refs #2642

scripts/hooks/gateguard-fact-force.js

@@ -53,11 +53,13 @@ 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.
-// 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;
+// `dd if=` used to be a fourth arm here. Matching it as text could not work:
+// the arm ended in `=`, so the shared trailing \b required the NEXT character
+// to be a word character and `dd if=/dev/zero` slipped through while
+// `echo dd if=x` — which runs no dd at all — was gated. The boundary decided
+// the verdict instead of the command position, so dd moved to isDestructiveDd()
+// alongside the other token-based detectors (#2642).
+const DESTRUCTIVE_SQL = /\b(drop\s+table|delete\s+from|truncate)\b/i;
 
 // Operator-supplied additional destructive patterns. Lazily compiled from
 // `GATEGUARD_BASH_EXTRA_DESTRUCTIVE` (regex source) on first use, then
@@ -357,6 +359,7 @@ function isDestructiveQuoteAware(raw, depth = 0) {
     if (tokens.length === 0) continue;
     if (isDestructiveRm(tokens)) return true;
     if (isDestructiveGit(tokens)) return true;
+    if (isDestructiveDd(tokens)) return true;
     if (isDestructiveFindExec(tokens.join(' '))) return true;
     const base = commandBasename(tokens[0]);
     if (SHELL_WRAPPERS.has(base)) {
@@ -384,6 +387,52 @@ function commandBasename(token) {
     .toLowerCase();
 }
 
+/**
+ * Detect a `dd` invocation carrying an `if=` operand.
+ *
+ * Token-based rather than a regex arm because the verdict has to depend on
+ * `dd` being the command, not on `dd if=` appearing anywhere in the line:
+ * `echo dd if=/dev/zero` executes nothing. dd operands are order-free, so
+ * `dd of=/dev/sda if=/dev/zero` counts too — a text pattern anchored on
+ * `dd\s+if=` missed that spelling entirely.
+ *
+ * Leading `sudo` / `doas` / `env`, their flags, and `VAR=value` assignment
+ * prefixes are skipped so `sudo dd if=/dev/zero` stays the dd invocation it is.
+ *
+ * @param {string[]} tokens
+ * @returns {boolean}
+ */
+function isDestructiveDd(tokens) {
+  let index = 0;
+  let sawWrapper = false;
+  while (index < tokens.length) {
+    const token = tokens[index];
+    const name = commandBasename(token);
+    if (name === 'sudo' || name === 'doas' || name === 'env') {
+      sawWrapper = true;
+      index += 1;
+      continue;
+    }
+    // `FOO=bar dd if=…` and `env FOO=bar dd if=…` both put assignments before
+    // the command word. `dd`'s own operands are never reached here: the loop
+    // stops at the first token that is neither a wrapper nor an assignment.
+    if (/^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) {
+      index += 1;
+      continue;
+    }
+    // Only skip flags once a wrapper has been seen, so this cannot walk past
+    // an unrelated command's arguments.
+    if (sawWrapper && token.startsWith('-')) {
+      index += 1;
+      continue;
+    }
+    break;
+  }
+
+  if (index >= tokens.length || commandBasename(tokens[index]) !== 'dd') return false;
+  return tokens.slice(index + 1).some(operand => /^if=/i.test(operand));
+}
+
 /**
  * Detect `rm` invocations that recursively force-delete files. Handles
  * combined (`-rf`, `-fr`, `-Rf`) and split (`-r -f`) flag forms.
@@ -681,9 +730,11 @@ function isDestructiveBash(command) {
   // after quoting AND subshell delimiters are normalized so phrases
   // inside `$(...)` or backticks are also caught.
   const raw = String(command || '');
+  // Keep main's heredoc stripping: a phrase inside a heredoc body is data, not a
+  // command. dd is no longer part of this regex — see DESTRUCTIVE_SQL.
   const executable = stripHeredocBodies(raw);
   const flattened = explodeSubshells(stripQuotedStrings(executable));
-  if (DESTRUCTIVE_SQL_DD.test(flattened)) return true;
+  if (DESTRUCTIVE_SQL.test(flattened)) return true;
 
   // Operator-supplied additional destructive patterns. Same scope as the
   // built-in SQL/dd regex: matched against the quote-stripped, subshell-
@@ -710,7 +761,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.test(stripped)) return true;
     if (extra && extra.test(stripped)) return true;
     const tokens = tokenize(segment);
     if (isDestructiveRm(tokens)) return true;

tests/hooks/gateguard-fact-force.test.js

@@ -267,7 +267,11 @@ function runTests() {
   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'
+    'dd if="/dev/zero" of=/dev/sda',
+    // Wrapped invocations must still resolve to the dd command word.
+    'sudo dd if=/dev/zero of=/dev/sda',
+    // dd operands are order-free; a text pattern anchored on `dd if=` missed this.
+    'dd of=/dev/sda if=/dev/zero'
   ]) {
     clearState();
     if (
@@ -294,7 +298,16 @@ function runTests() {
    * 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']) {
+  for (const command of [
+    'echo add if=1',
+    'echo truncated output',
+    'git status',
+    // `dd if=` as another command's argument runs no dd at all. The old text
+    // match gated these; the command-word check is what keeps them out.
+    'echo dd if=/dev/zero',
+    'grep dd if=/dev/zero file',
+    'echo dd if=x'
+  ]) {
     clearState();
     if (
       test(`does not gate as destructive: ${command}`, () => {