Merge pull request #3484 from affaan-m/integration/ecc-043-20261009-verified

fix: guard literal Git include redirects and env split arguments

scripts/hooks/block-no-verify.js

@@ -25,7 +25,15 @@ let raw = '';
 // so we normalize the candidate token to lowercase before matching.
 // See https://git-scm.com/docs/git-config — "The variable names are
 // case-insensitive."
-const GIT_CONFIG_KEY_PREFIX = 'core.hookspath=';
+function isHookRedirectConfigKey(key) {
+  const normalized = key.toLowerCase();
+  return normalized === 'core.hookspath' || normalized === 'include.path'
+    || /^includeif\..*\.path$/.test(normalized);
+}
+
+function isHookRedirectSetting(setting) {
+  return isHookRedirectConfigKey(setting.split('=', 1)[0]);
+}
 
 const COMMIT_OPTIONS_WITH_VALUE = new Set([
   '-m',
@@ -189,7 +197,7 @@ function gitEnvironmentOverride(environment, budget) {
       const key = environment.get(`GIT_CONFIG_KEY_${i}`);
       if (key === undefined || !environment.has(`GIT_CONFIG_VALUE_${i}`)) { complete = false; break; }
       budget.spend(key.length + 1);
-      override ||= key.toLowerCase() === 'core.hookspath';
+      override ||= isHookRedirectConfigKey(key);
     }
     if (complete && override) return true;
   }
@@ -201,7 +209,7 @@ function gitEnvironmentOverride(environment, budget) {
     for (const command of scanShell(parameters, budget).commands) {
       for (const word of command.words) {
         budget.spend(word.value.length + 1);
-        if (word.value.toLowerCase().startsWith(GIT_CONFIG_KEY_PREFIX)) return true;
+        if (isHookRedirectSetting(word.value)) return true;
       }
     }
   }
@@ -219,8 +227,9 @@ function checkGitWords(words, budget, start = 0, environmentOverride = false) {
     if (value === '-c' || value === '--config-env') {
       const setting = words[index + 1]?.value || '';
       budget.spend(setting.length + 1);
-      override ||= setting.toLowerCase().startsWith(GIT_CONFIG_KEY_PREFIX);
-    } else if (value.toLowerCase().startsWith(`-c${GIT_CONFIG_KEY_PREFIX}`) || value.toLowerCase().startsWith(`--config-env=${GIT_CONFIG_KEY_PREFIX}`)) override = true;
+      override ||= isHookRedirectSetting(setting);
+    } else if (value.startsWith('-c')) override ||= isHookRedirectSetting(value.slice(2));
+    else if (value.startsWith('--config-env=')) override ||= isHookRedirectSetting(value.slice('--config-env='.length));
     if (GIT_GLOBAL_VALUES.has(value)) index++;
   }
   const command = words[index]?.value;
@@ -257,6 +266,23 @@ function assignmentValues(prior, operand, append, dynamic, budget) {
   return [...values];
 }
 
+class UnsupportedEnvSplitEscape extends Error {}
+
+function validateEnvSplitEscapes(payload, budget) {
+  let quote = null;
+  for (let index = 0; index < payload.length; index++) {
+    budget.spend();
+    const character = payload[index];
+    // env -S has its own escapes. Only literal single-quoted backslashes
+    // (excluding env's \\ and \' escapes) agree with the shell scanner.
+    if (character === '\\' && (quote !== "'" || payload[index + 1] === '\\' || payload[index + 1] === "'")) {
+      throw new UnsupportedEnvSplitEscape();
+    }
+    if (character === "'" && quote !== '"') quote = quote === "'" ? null : "'";
+    else if (character === '"' && quote !== "'") quote = quote === '"' ? null : '"';
+  }
+}
+
 // Only explicit option grammars remove wrapper operands. Unknown launchers are
 // opaque/conservative, never guessed from a name found among data arguments.
 function executableWords(words, budget, inherited = new Map(), callerValues = inherited) {
@@ -358,6 +384,23 @@ function executableWords(words, budget, inherited = new Map(), callerValues = in
         const flag = words[i].value;
         budget.spend(flag.length + 1);
         if (flag === '--') { i++; break; }
+        if (env && (flag === '-S' || flag === '--split-string'
+          || flag.startsWith('--split-string=') || flag.startsWith('-S'))) {
+          const separated = flag === '-S' || flag === '--split-string';
+          const payload = separated ? words[i + 1]?.value || ''
+            : flag.slice(flag.startsWith('--split-string=') ? '--split-string='.length : 2);
+          budget.spend(payload.length + 1);
+          validateEnvSplitEscapes(payload, budget);
+          const scan = scanShell(payload, budget);
+          // Only literal word splitting is understood here. Keep complex env
+          // syntax opaque rather than pretending to execute it as a shell.
+          if (scan.commands.length !== 1 || scan.commands[0].nested.length
+            || scan.commands[0].redirects.length || scan.commands[0].words.some(word => word.dynamic)) return suffix(i - 1);
+          // Pay for both slices and the expanded array before allocating them.
+          budget.spend(2 * words.length + scan.commands[0].words.length);
+          words = [...words.slice(0, i), ...scan.commands[0].words, ...words.slice(i + (separated ? 2 : 1))];
+          continue;
+        }
         if (env && (flag === '-i' || flag === '--ignore-environment')) resetEnvironment();
         if (env && (flag === '-u' || flag === '--unset')) resetEnvironment(words[i + 1]?.value || '');
         else if (env && flag.startsWith('--unset=')) resetEnvironment(flag.slice('--unset='.length));
@@ -701,6 +744,9 @@ function checkCommand(input) {
       }
     }
   } catch (error) {
+    if (error instanceof UnsupportedEnvSplitEscape) {
+      return { blocked: true, reason: 'BLOCKED: Unsupported env split-string escape; hook-bypass safety could not be established.' };
+    }
     if (!(error instanceof RangeError)) throw error;
     return { blocked: true, reason: 'BLOCKED: Shell analysis work budget exceeded; hook-bypass safety could not be established.' };
   }

tests/hooks/block-no-verify.test.js

@@ -1994,6 +1994,154 @@ else {
   }
 }
 
+// Contributor carry-forward regressions for #2858 and #2897.
+for (const [name, command, expected] of [
+  [
+    "blocks include.path config override",
+    "git -c include.path=/tmp/evil.conf commit -m x",
+    2
+  ],
+  [
+    "blocks quoted include.path config",
+    "git \"-c\" \"include.path=/tmp/evil.conf\" push origin main",
+    2
+  ],
+  [
+    "blocks inline include.path config",
+    "git -cinclude.path=/tmp/evil.conf commit -m x",
+    2
+  ],
+  [
+    "blocks include.path config-env",
+    "git --config-env=include.path=EVIL commit -m x",
+    2
+  ],
+  [
+    "blocks separated include.path config-env",
+    "git --config-env include.path=EVIL commit -m x",
+    2
+  ],
+  [
+    "blocks case-insensitive include.path",
+    "git -c INCLUDE.PATH=/tmp/evil.conf commit -m x",
+    2
+  ],
+  [
+    "blocks conditional include capability",
+    "git -c includeIf.gitdir:**/.path=/tmp/evil.conf commit -m x",
+    2
+  ],
+  [
+    "blocks counted include.path environment",
+    "GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=include.path GIT_CONFIG_VALUE_0=/tmp/evil.conf git commit -m x",
+    2
+  ],
+  [
+    "blocks encoded include.path parameters",
+    "GIT_CONFIG_PARAMETERS=\"'include.path'='/tmp/evil.conf'\" git commit -m x",
+    2
+  ],
+  [
+    "allows unrelated config value",
+    "git -c user.name=ECC commit -m x",
+    0
+  ],
+  [
+    "allows include setting outside protected command",
+    "git -c include.path=/tmp/evil.conf status",
+    0
+  ],
+  [
+    "blocks env long inline split-string",
+    "env --split-string='git commit --no-verify -m x'",
+    2
+  ],
+  [
+    "blocks env split-string after ignore-environment",
+    "env -i --split-string='git commit --no-verify -m x'",
+    2
+  ],
+  [
+    "blocks env separated long split-string",
+    "env --split-string 'git commit --no-verify -m x'",
+    2
+  ],
+  [
+    "blocks env attached short split-string",
+    "env -S'git commit --no-verify -m x'",
+    2
+  ],
+  [
+    "blocks appended argv after env split-string",
+    "env --split-string='git commit' --no-verify -m x",
+    2
+  ],
+  [
+    "allows quoted bypass text passed through env printf",
+    "env --split-string=\"printf '%s' 'git commit --no-verify -m x'\"",
+    0
+  ]
+]) {
+  if (test(name, () => {
+    const result = runHook({ tool_input: { command } });
+    assert.strictEqual(result.code, expected, result.stderr);
+  })) passed++; else failed++;
+}
+
+for (const n of [100, 1000, 4000]) {
+  for (const tail of ['git status', 'git commit --no-verify']) {
+    if (test(`literal env split copies stay within the input quota at ${n}: ${tail}`, () => {
+      const command = 'env ' + '-S -i '.repeat(n) + tail;
+      const quota = 24 * (command.length + 1) + 4096;
+      const result = countedClassification(command);
+      assert.strictEqual(result.result.exitCode, 2, JSON.stringify(result));
+      assert.match(result.result.stderr, /work budget/);
+      assert.ok(result.copiedElements <= quota, JSON.stringify(result));
+      assert.ok(result.copiedElements <= result.spent, JSON.stringify(result));
+      // The rejected charge may exceed the quota, but allocation must not run.
+      assert.ok(result.spent >= quota && result.spent <= quota + 2 * (2 * n + 4) + 1, JSON.stringify(result));
+    })) passed++; else failed++;
+  }
+}
+for (const n of [0, 1, 2, 4]) {
+  if (test(`shallow literal env splits allow harmless argv at ${n}`, () => {
+    const command = 'env ' + '-S -i '.repeat(n) + 'git status';
+    const result = countedClassification(command);
+    assert.strictEqual(result.result.exitCode, 0, JSON.stringify(result));
+    assert.ok(result.copiedElements <= result.spent, JSON.stringify(result));
+  })) passed++; else failed++;
+}
+
+const quoteSplitPayload = payload => `'${payload.replace(/'/g, `'\\''`)}'`;
+const escapedSplitCases = [
+  ['unquoted escaped separators', 'git\\_commit\\_--no-verify', 2],
+  ['single-quoted literal separators', "'git\\_commit\\_--no-verify'", 0],
+  ['double-quoted escaped separators', '"git\\_commit\\_--no-verify"', 2],
+  ['mixed quoted escaped separators', 'git\\_commit\\_"--no-verify"', 2],
+  ['unsupported stop escape', 'printf ok\\c ignored', 2],
+  ['unsupported double-quoted stop escape', 'printf "ok\\c" ignored', 2],
+  ['single-quoted stop data', "printf '%s' 'ok\\c'", 0],
+  ['single-quoted separator data', "printf '%s' 'git\\_commit\\_--no-verify'", 0],
+  ['single-quoted newline format', "printf '%s\\n' 'git commit --no-verify'", 0],
+  ['unsupported newline escape', 'printf ok\\n', 2],
+  ['unsupported tab escape', 'printf ok\\t', 2],
+  ['unsupported escaped backslash', 'printf ok\\\\_', 2],
+  ['unsupported single-quoted backslash escape', "printf '%s' 'ok\\\\_'", 2],
+  ['unsupported single-quoted apostrophe escape', "printf '%s' 'ok\\'data'", 2],
+];
+for (const [name, payload, expected] of escapedSplitCases) {
+  for (const option of ['-S ', '-S', '--split-string=', '--split-string ']) {
+    if (test(`env escape boundary ${name}: ${option}`, () => {
+      const command = 'env ' + option + quoteSplitPayload(payload);
+      for (const input of [command, JSON.stringify({ tool_input: { command } })]) {
+        const result = runHook(input);
+        assert.strictEqual(result.code, expected, result.stderr);
+        if (expected === 2) assert.match(result.stderr, /Unsupported env split-string escape/);
+      }
+    })) passed++; else failed++;
+  }
+}
+
 console.log('─'.repeat(50));
 console.log(`Passed: ${passed}  Failed: ${failed}`);