Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-birds-tumble.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
type: Fixed
pr: 5874
---
**The secret-read guard reads past sudo's `-a`/`-c` and doas's `-a` values again** — `sudo -c ls cat .env`, `sudo --auth-type echo cat .env` and `doas -a ls cat .env` are denied again, as they were in 1.18.0; the wrapper table now lists those value-taking options from sudo(8) and doas(1). (#5852)
11 changes: 8 additions & 3 deletions hooks/gsd-secret-read-guard.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,11 @@ const NON_SECRET_ENV_SUFFIXES = new Set(['example', 'sample', 'template', 'dist'
// stdbuf(1), setsid(1) and bash's `command`/`exec`. A wrong entry fails open
// (the option's value becomes the command word), so keep it to the man pages.
const WRAPPERS = new Map([
['sudo', { short: 'CDghpRrTtUu', long: ['--close-from', '--chdir', '--group', '--host', '--prompt', '--chroot', '--role', '--command-timeout', '--type', '--other-user', '--user'] }],
['doas', { short: 'Cu' }],
['sudo', { short: 'aCcDghpRrTtUu', long: ['--auth-type', '--close-from', '--login-class', '--chdir', '--group', '--host', '--prompt', '--chroot', '--role', '--command-timeout', '--type', '--other-user', '--user'],
// No-value long options, so an exact one never prefix-matches a longer
// valued one (`--login` vs `--login-class`).
flags: ['--askpass', '--background', '--bell', '--edit', '--help', '--list', '--login', '--non-interactive', '--preserve-env', '--preserve-groups', '--remove-timestamp', '--reset-timestamp', '--set-home', '--shell', '--stdin', '--validate', '--version'] }],
['doas', { short: 'aCu' }],
['env', { short: 'uCSaP', long: ['--unset', '--chdir', '--split-string', '--argv0'], assigns: true }],
['nice', { short: 'n', long: ['--adjustment'] }],
['nohup', {}],
Expand Down Expand Up @@ -886,7 +889,9 @@ const GROUPING_SEPS = new Set(['(', ')', '{', '}']);
function wrapperOptionWidth(word, spec) {
if (word.startsWith('--')) {
if (word.includes('=')) return 1;
// getopt_long accepts any unambiguous prefix (`--adj 5`).
// getopt_long takes an exact match over any longer option it prefixes
// (`sudo --login` is not `--login-class`), else any unambiguous prefix (`--adj 5`).
if ((spec.flags || []).includes(word)) return 1;
return word.length > 2 && (spec.long || []).some((l) => l.startsWith(word)) ? 2 : 1;
}
const valued = spec.short || '';
Expand Down
58 changes: 58 additions & 0 deletions tests/gsd-secret-read-guard.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,8 @@ describe('gsd-secret-read-guard: nested shell behind a wrapper with flags (#5627
'command -p', 'command --', 'time -p', 'time -f %e',
'stdbuf -oL', 'stdbuf -i0 -oL -eL', 'stdbuf --output=L', 'setsid', 'setsid -w', 'exec', 'exec -a name',
'nohup', 'sudo -u root', 'sudo -E', 'sudo --', 'sudo -u root -g wheel', 'doas -u root',
'sudo -c staff', 'sudo --login-class staff', 'sudo -a bsdauth', 'sudo --auth-type bsdauth', 'doas -a passwd',
'sudo --login', 'sudo --login -u root',
'nice -n 5 timeout -s KILL 5 env -i', 'sudo -n timeout 5 nohup',
];
for (const w of wrappers) {
Expand Down Expand Up @@ -1237,6 +1239,62 @@ describe('gsd-secret-read-guard: nested shell behind a wrapper with flags (#5627
});
});

describe('gsd-secret-read-guard: sudo/doas value options the wrapper table missed (#5852)', () => {
// Oracle: sudo(8) documents `-a type, --auth-type=type` and `-c class,
// --login-class=class`; doas(1) documents `-a style`. Each takes the next
// word as its value, so the command is the word after it. When that value
// happens to be a non-reading command name (or `timeout`), reading it as the
// command word let the read through; 1.18.0 denied all of these.
const reads = [
'sudo -c ls cat .env', 'sudo --login-class ls cat .env',
'sudo -a echo cat .env', 'sudo --auth-type echo cat .env',
'sudo -c timeout cat .env', 'doas -a ls cat .env',
'sudo -cls cat .env', 'doas -als cat .env',
"sudo -c ls bash -c 'cat .env'", "doas -a ls bash -c 'cat .env'",
];
for (const cmd of reads) {
test(`blocks ${JSON.stringify(cmd)}`, () => {
assertBlocked(runHook(bash(cmd)), cmd, { tool: 'Bash', path: '.env' });
});
}

// `--login` (sudo's -i) takes no value. It must not prefix-match the valued
// `--login-class`, or `cat` becomes its value and `.env` the command word.
const stillBlocked = [
'sudo -u root cat .env', 'doas -u root cat .env', 'sudo -c staff cat .env', 'sudo -E cat .env',
'sudo --login cat .env', "sudo --login bash -c 'cat .env'", 'sudo --login -u root cat .env',
];
for (const cmd of stillBlocked) {
test(`still blocks ${JSON.stringify(cmd)}`, () => {
assertBlocked(runHook(bash(cmd)), cmd, { tool: 'Bash', path: '.env' });
});
}

// The option consumes exactly one word: the command after the value is
// resolved, and a no-value option (-n, -E, -H) still resolves to the next word.
const allows = ['sudo -c staff ls .env', 'doas -a passwd ls .env', 'sudo --auth-type bsdauth ls .env', 'sudo -n ls .env', 'sudo -E -H ls .env', 'sudo --login ls .env'];
for (const cmd of allows) {
test(`allows ${JSON.stringify(cmd)}`, () => {
assertAllowed(runHook(bash(cmd)), cmd);
});
}

// The xargs-pipe check resolves the UPSTREAM command with resolveCommand
// (xargs's own sub-command word is taken as is), so the wrapper sits upstream.
for (const cmd of ['sudo -c staff echo .env | xargs cat', 'sudo --login echo .env | xargs cat']) {
test(`the xargs-pipe check resolves the upstream command: ${JSON.stringify(cmd)}`, () => {
assertBlocked(runHook(bash(cmd)), cmd, { tool: 'Bash', path: '.env' });
});
}

test('the debugger-spawn check resolves the command after sudo -c', () => {
assertBlocked(
runHook(bash('sudo -c ls claude --agent gsd-debugger -p hi')),
'sudo -c', { code: 'debugger-cli-spawn', tool: 'Bash' },
);
});
});

describe('gsd-secret-read-guard: bundled short-flag clusters (#5046)', () => {
// operandNamesSecret is the parser: for a single-dash word it checks the
// value after the run of option characters, in addition to the historical
Expand Down
Loading