WEBDIENSTE/ECC
1
0
Fork 0
mirror of https://github.com/affaan-m/ECC.git synced 2026-10-20 15:01:46 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: guard literal Git hook redirects and env split arguments

Browse Source 
Carry forward bounded literal include/includeIf configuration capabilities
and env -S/--split-string words from PRs #2858 and #2897. Charge array
copy and expansion work before allocation to preserve the shared quota.

Verified 731 hook cases and 60 real Git/config and registered-route
controls. Dynamic remainder and original full source PRs remain held.

Co-authored-by: Nguyen Thanh Dat <ntdat812.dev@gmail.com>
Co-authored-by: Davemaina1 <dmain7015@gmail.com>
This commit is contained in:
haelyra
2026-10-09 20:41:09 -04:00
co-authored by Nguyen Thanh Dat Davemaina1
parent 8384d609a0
commit 993b5f230f
2 changed files with 148 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/hooks/block-no-verify.js
+30 -5
View File
@@ -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;
@@ -358,6 +367,22 @@ 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);
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));
tests/hooks/block-no-verify.test.js
+118
View File
@@ -1994,6 +1994,124 @@ 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++;
}
console.log('─'.repeat(50));
console.log(`Passed: ${passed} Failed: ${failed}`);