fix(shell): close pgrep self-kill bypasses for short patterns and nice wrapper

Two bypasses in detectSelfKillCommand let a self-targeting pgrep slip
through and were encoded as expected-not-blocked in tests:

1. isSubstringSelfTarget's `length < 3` guard assumed short pgrep
   patterns cannot match a self process name. But pgrep matches the
   pattern as an ERE against the process name, so 'no' (len 2) matches
   'node' and 'e' (len 1) matches 'node'/'qwen' — both are real
   self-kills. Replace the length heuristic with an empty-string guard
   and rely on the actual substring check. 'pgrep a'/'pgrep ab' stay
   allowed (no self name contains them).

2. killCommandTargetsSelf skipped any $(...) inner command starting
   with 'nice -', so 'kill -9 $(nice -n10 pgrep node)' was not blocked
   even though 'nice -n10 pgrep node' executes 'pgrep node'. The skip
   had no false-positive benefit (unwrapExecutionPrefixes already
   handles nice + its -n value); removing it closes the gap. Benign
   'nice ...' without a self-targeting pgrep still returns false.

Tests updated to assert these are blocked.
This commit is contained in:
jinjing.zzj 2026-07-08 20:33:13 +08:00
parent 2fd6c9cdf9
commit d8df832e99
2 changed files with 20 additions and 11 deletions

View file

@ -827,9 +827,18 @@ describe('detectSelfKillCommand', () => {
expect(detectSelfKillCommand('kill -9 $(pgrep qwen-cod)')).toBe(true);
});
it('does not false-positive on very short pgrep patterns', () => {
expect(detectSelfKillCommand('kill -9 $(pgrep no)')).toBe(false);
it('blocks short pgrep patterns that substring-match a self name', () => {
// "no" is a substring of "node": pgrep treats the pattern as an ERE
// matched against the process name, so `pgrep no` matches "node"
// (which is qwen-code's own process) and must be blocked.
expect(detectSelfKillCommand('kill -9 $(pgrep no)')).toBe(true);
expect(detectSelfKillCommand('kill -9 $(pgrep -f no)')).toBe(true);
expect(detectSelfKillCommand('kill -9 $(pgrep e)')).toBe(true);
});
it('does not false-positive on short patterns that no self name contains', () => {
expect(detectSelfKillCommand('kill -9 $(pgrep a)')).toBe(false);
expect(detectSelfKillCommand('kill -9 $(pgrep ab)')).toBe(false);
});
it('handles xargs with additional options like -L, -d, -a', () => {
@ -984,9 +993,13 @@ describe('detectSelfKillCommand', () => {
);
});
it('nice -n10 inside command substitution (regex bypass)', () => {
expect(detectSelfKillCommand('kill -9 $(nice -n10 pgrep node)')).toBe(
false,
it('detects pgrep wrapped by nice with a niceness value', () => {
// `nice -n10 pgrep node` and `nice -n 10 pgrep node` both execute
// `pgrep node`, which lists qwen-code's own PID, so the surrounding
// kill must be blocked.
expect(detectSelfKillCommand('kill -9 $(nice -n10 pgrep node)')).toBe(true);
expect(detectSelfKillCommand('kill -9 $(nice -n 10 pgrep node)')).toBe(
true,
);
});

View file

@ -878,8 +878,8 @@ const SELF_NAMES_LIST = [
];
function isSubstringSelfTarget(value: string): boolean {
if (!value) return false;
if (isBareSelfProcessName(value)) return true;
if (value.length < 3) return false;
const lower = value.toLowerCase();
return SELF_NAMES_LIST.some((name) => name.includes(lower));
}
@ -998,8 +998,7 @@ function pgrepCommandTargetsSelf(command: string): boolean {
const tokens = unwrapExecutionPrefixes(parsed);
const root = normalizeExecutableName(tokens[0] ?? '');
return (
(root === 'pgrep' || root === 'pidof') &&
pgrepTargetsSelf(tokens, root)
(root === 'pgrep' || root === 'pidof') && pgrepTargetsSelf(tokens, root)
);
});
}
@ -1092,9 +1091,6 @@ function killCommandTargetsSelf(segment: string): boolean {
}
for (const match of segment.matchAll(/\$\(([^()]*)\)|<\s*<\(([^()]*)\)/g)) {
const innerCommand = match[1] ?? match[2] ?? '';
if (/^\s*nice\s+-/i.test(innerCommand)) {
continue;
}
if (pgrepCommandTargetsSelf(innerCommand)) {
return true;
}