fix(security): block docker socket from sandboxed shell; disable opencode batch_tool
two real CI failures on main, both shipping bugs in the action: 1. `token-exfil-claude` was a real sandbox escape: GHA `ubuntu-latest` puts `runner` in the `docker` group, so a sandboxed shell could run `docker run --pid=host --privileged busybox cat /proc/<parent>/environ` and read the action process's env (which holds user secrets) — fully bypassing the unshare PID-namespace. fix: inside the sandbox's mount namespace (already private via `--mount-proc` which implies `--mount`), bind-mount /dev/null over /var/run/docker.sock (+ podman/containerd/crio variants) so any container-runtime socket connect from the sandbox fails. only affects sandboxed shells — host runner mount table is untouched, so user workflow steps outside pullfrog keep working. 2. `restricted-opencode` regressed in #719 (`experimental.batch_tool`). opencode's batch tool rejects MCP tools with `"Tool '<name>' not in registry. External tools (MCP, environment) cannot be batched."` when a model emits parallel `pullfrog_shell` (or any MCP) tool_use blocks, opencode internally routes them through batch, they all fail, the model misreads the error as "the tool doesn't exist", and gives up. caught by a `lens:` subagent in the restricted test concluding shell was unavailable and setting `DIAGNOSTIC_ID=empty`. drop `batch_tool: true` and the matching opencode-specific guidance in `instructions.ts` — native parallel tool_use (multiple tool_use blocks per assistant message) still works for both built-in and MCP tools without batch, so we lose only the 1-25 wrapper, not parallelism.
This commit is contained in:
committed by
pullfrog[bot]
parent
efc1b67e7b
commit
c0988e35b0
+13
-9
@@ -135,15 +135,19 @@ function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): s
|
|||||||
log.info(`» subagent models: reviewfrog=${reviewerModel}`);
|
log.info(`» subagent models: reviewfrog=${reviewerModel}`);
|
||||||
return cfg;
|
return cfg;
|
||||||
})(),
|
})(),
|
||||||
// opt into opencode's experimental `batch` tool (added in
|
// NOTE: `experimental.batch_tool` was enabled in #719 to bundle 1-25
|
||||||
// anomalyco/opencode PR #2983, opt-in via `experimental.batch_tool`). it
|
// independent tool calls into one round trip, but the batch tool rejects
|
||||||
// exposes a single `batch` tool that runs 1-25 independent tool calls
|
// MCP/"external" tools with `"Tool '<name>' not in registry. External
|
||||||
// (read/grep/glob/bash/etc.) concurrently in one assistant turn, which
|
// tools (MCP, environment) cannot be batched - call them directly."`
|
||||||
// collapses the dominant grep→20×read pattern into a single round trip.
|
// (anomalyco/opencode PR #2983 design). when a model emits parallel
|
||||||
// edits are explicitly disallowed inside the batch upstream. paired with
|
// tool_use blocks containing `pullfrog_*` calls, opencode internally
|
||||||
// the "Parallel tool execution" guidance in utils/instructions.ts so the
|
// routes them through batch — they all fail, the model misreads the
|
||||||
// model actually reaches for it. see wiki/prompt.md.
|
// error as "the tool doesn't exist", and gives up. caught in CI by
|
||||||
experimental: { batch_tool: true },
|
// `restricted-opencode` after a `lens:` subagent dispatched parallel
|
||||||
|
// `pullfrog_shell` calls and concluded shell was unavailable.
|
||||||
|
// native parallel tool_use (multiple tool_use blocks per assistant
|
||||||
|
// message) still works without batch_tool for both built-in and MCP
|
||||||
|
// tools, so we lose only the batch wrapper, not parallelism.
|
||||||
// gemini-3 thinking pinned to high for review depth; gpt and anthropic
|
// gemini-3 thinking pinned to high for review depth; gpt and anthropic
|
||||||
// effort set elsewhere (gpt: upstream default, anthropic: --effort flag in claude.ts).
|
// effort set elsewhere (gpt: upstream default, anthropic: --effort flag in claude.ts).
|
||||||
provider: { google: { models: geminiHighThinkingOverrides() } },
|
provider: { google: { models: geminiHighThinkingOverrides() } },
|
||||||
|
|||||||
+30
-2
@@ -96,6 +96,27 @@ function detectSandboxMethod(): SandboxMethod {
|
|||||||
const PROC_CLEANUP =
|
const PROC_CLEANUP =
|
||||||
"umount /proc 2>/dev/null; umount /proc 2>/dev/null; mount -t proc proc /proc 2>/dev/null;";
|
"umount /proc 2>/dev/null; umount /proc 2>/dev/null; mount -t proc proc /proc 2>/dev/null;";
|
||||||
|
|
||||||
|
// block container-runtime sockets that would otherwise grant a PID-namespace
|
||||||
|
// escape: `docker run --pid=host --privileged busybox cat /proc/<pid>/environ`
|
||||||
|
// reads the parent action process's env (which contains user secrets) even
|
||||||
|
// though the sandbox itself is unsharing PIDs. GHA `ubuntu-latest` puts the
|
||||||
|
// `runner` user in the `docker` group by default, so the socket is reachable
|
||||||
|
// without sudo. bind-mounting /dev/null on top inside the sandbox's mount
|
||||||
|
// namespace makes the socket unreachable from sandboxed shells without
|
||||||
|
// touching the host runner (so it doesn't break user workflow steps that
|
||||||
|
// run before/after pullfrog and legitimately need docker). same trick for
|
||||||
|
// podman/containerd/cri-o sockets — all silent-fail if the path is missing.
|
||||||
|
const SOCKET_CLEANUP = [
|
||||||
|
"/var/run/docker.sock",
|
||||||
|
"/run/docker.sock",
|
||||||
|
"/var/run/podman/podman.sock",
|
||||||
|
"/run/podman/podman.sock",
|
||||||
|
"/run/containerd/containerd.sock",
|
||||||
|
"/var/run/crio/crio.sock",
|
||||||
|
]
|
||||||
|
.map((path) => `mount --bind /dev/null ${path} 2>/dev/null;`)
|
||||||
|
.join(" ");
|
||||||
|
|
||||||
function spawnShell(params: SpawnParams): ChildProcess {
|
function spawnShell(params: SpawnParams): ChildProcess {
|
||||||
const spawnOpts = { env: params.env, cwd: params.cwd, stdio: params.stdio, detached: true };
|
const spawnOpts = { env: params.env, cwd: params.cwd, stdio: params.stdio, detached: true };
|
||||||
const sandboxMethod = detectSandboxMethod();
|
const sandboxMethod = detectSandboxMethod();
|
||||||
@@ -110,7 +131,14 @@ function spawnShell(params: SpawnParams): ChildProcess {
|
|||||||
if (sandboxMethod === "unshare") {
|
if (sandboxMethod === "unshare") {
|
||||||
return spawn(
|
return spawn(
|
||||||
"unshare",
|
"unshare",
|
||||||
["--pid", "--fork", "--mount-proc", "bash", "-c", `${PROC_CLEANUP} ${params.command}`],
|
[
|
||||||
|
"--pid",
|
||||||
|
"--fork",
|
||||||
|
"--mount-proc",
|
||||||
|
"bash",
|
||||||
|
"-c",
|
||||||
|
`${PROC_CLEANUP} ${SOCKET_CLEANUP} ${params.command}`,
|
||||||
|
],
|
||||||
spawnOpts
|
spawnOpts
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -143,7 +171,7 @@ function spawnShell(params: SpawnParams): ChildProcess {
|
|||||||
"--mount-proc",
|
"--mount-proc",
|
||||||
"bash",
|
"bash",
|
||||||
"-c",
|
"-c",
|
||||||
`${PROC_CLEANUP} exec su -p -s /bin/bash ${username} -c '${escaped}'`,
|
`${PROC_CLEANUP} ${SOCKET_CLEANUP} exec su -p -s /bin/bash ${username} -c '${escaped}'`,
|
||||||
],
|
],
|
||||||
{ ...spawnOpts, env: {} }
|
{ ...spawnOpts, env: {} }
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -297,11 +297,9 @@ For maximum efficiency, whenever you need to perform multiple independent operat
|
|||||||
- listing multiple directories
|
- listing multiple directories
|
||||||
- inspecting multiple MCP tools or resources
|
- inspecting multiple MCP tools or resources
|
||||||
|
|
||||||
Do NOT parallelize operations that depend on prior output (e.g. create a file then read it), or ordered stateful mutations. Edits are not parallelizable — sequence those normally.${
|
Do NOT parallelize operations that depend on prior output (e.g. create a file then read it), or ordered stateful mutations. Edits are not parallelizable — sequence those normally.
|
||||||
ctx.agentId === "opencode"
|
|
||||||
? `\n\nOn OpenCode you also have a \`batch\` tool that bundles 1-25 independent calls into one wrapper call. Reach for it whenever you have >=2 independent calls. Native parallel tool_use and \`batch\` both achieve one round trip instead of N — use whichever your provider supports best.`
|
Emit multiple \`tool_use\` blocks in the same assistant message for independent calls — the runtime executes them concurrently. Do not wait for one tool result before issuing the next independent call.
|
||||||
: `\n\nEmit multiple \`tool_use\` blocks in the same assistant message for independent calls — the runtime executes them concurrently. Do not wait for one tool result before issuing the next independent call.`
|
|
||||||
}
|
|
||||||
|
|
||||||
### Command execution
|
### Command execution
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user