diff --git a/services/runner/docker/Dockerfile.gh b/services/runner/docker/Dockerfile.gh index 91fed149b5..cc2a953491 100644 --- a/services/runner/docker/Dockerfile.gh +++ b/services/runner/docker/Dockerfile.gh @@ -69,6 +69,15 @@ ENV NODE_ENV=production \ EXPOSE 8765 +# Pi's agent dir (the published deployment sets PI_CODING_AGENT_DIR=/pi-agent) is where the runner +# installs the permission-enforcing extension at the start of every run. The runtime user `node` +# (uid 1000) cannot create a directory at the container root, so create it here and hand it to +# `node` before dropping privileges. Without this the install fails with EACCES and — before the +# fail-closed guard — the policy silently did nothing. The runner also routes managed/no-key runs +# through a per-run temp dir the runtime user owns, so this is the fast path for the default dir, +# not the only safeguard. +RUN mkdir -p /pi-agent && chown node:node /pi-agent + USER node # Call the local tsx binary directly to avoid pnpm/corepack HOME writes when the diff --git a/services/runner/src/engines/sandbox_agent.ts b/services/runner/src/engines/sandbox_agent.ts index 8ab19905fa..4e396df19c 100644 --- a/services/runner/src/engines/sandbox_agent.ts +++ b/services/runner/src/engines/sandbox_agent.ts @@ -91,6 +91,7 @@ import { buildPiExtensionEnv, configurePiSessionWorkspace, configurePiSkillSnapshot, + PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE, prepareLocalPiAssets, resolvePiSkillSnapshot, uploadSystemPromptToSandbox, @@ -297,7 +298,8 @@ function shouldSuppressPausedToolCallUpdate( pause: PendingApprovalPauseController, ): boolean { const frame = update as - { sessionUpdate?: unknown; toolCallId?: unknown } | undefined; + | { sessionUpdate?: unknown; toolCallId?: unknown } + | undefined; const kind = frame?.sessionUpdate; if (kind !== "tool_call" && kind !== "tool_call_update") return false; const toolCallId = @@ -644,7 +646,8 @@ export interface SessionEnvironment { } export type AcquireEnvironmentResult = - { ok: true; env: SessionEnvironment } | { ok: false; error: string }; + | { ok: true; env: SessionEnvironment } + | { ok: false; error: string }; /** * Sign the session's durable mount up front so keep-alive can build a pool key (the mount's @@ -856,7 +859,18 @@ export async function acquireEnvironment( } // undefined is fine: the local provider runs its own resolution and errors clearly. const binaryPath = (deps.resolveDaemonBinary ?? resolveDaemonBinary)(); - let runAgentDir = prepareLocalPiAssets({ plan, env, log: logger }); + const localPiAssets = prepareLocalPiAssets({ plan, env, log: logger }); + let runAgentDir = localPiAssets.dir; + // Fail closed (Decision 2): when the policy could gate a Pi built-in tool but the permission + // extension did not install, the run must stop rather than run those tools unprotected. Recorded + // here (the install ran above) and thrown inside the try below so the engine's own catch turns it + // into `{ ok: false, error }` and a visible error frame. `builtinGatingActive` false means + // allow-everything, where the extension is not needed and a failed install is harmless. + const localBuiltinGatingUnenforceable = + plan.isPi && + !plan.isDaytona && + plan.builtinGatingActive && + !localPiAssets.extensionInstalled; // A local Claude subscription run reads and writes the operator's read-write mounted login // DIRECTLY: `buildDaemonEnv` already carried `CLAUDE_CONFIG_DIR` (the mount) into the daemon env, @@ -1065,7 +1079,17 @@ export async function acquireEnvironment( ); return; } - runAgentDir = prepareLocalPiAssets({ plan, env, log: logger }); + // Discarding `.extensionInstalled` here is safe, and a fail-closed throw here would be + // unsound anyway (both callers wrap this in a mount try/catch that logs and continues, so a + // throw could not stop the run). Reachability: managed/none local Pi runs always created a + // throwaway dir in the first prepareLocalPiAssets call, so `environment.runAgentDir` is set + // for them and they returned above — only the subscription (runtime_provided) path reaches + // this re-prep. That path installs into the SAME operator mount the first call already + // installed into, and the fail-closed gating check right after that first call stopped the + // run when the install was required but failed. So by the time this runs, either enforcement + // is not needed (policy allows everything) or the extension file is already on disk from the + // verified first install; a transient failure here cannot remove it. + runAgentDir = prepareLocalPiAssets({ plan, env, log: logger }).dir; environment.runAgentDir = runAgentDir; }; @@ -1213,6 +1237,11 @@ export async function acquireEnvironment( }; try { + // Fail closed before any sandbox/mount infra spins up: a local Pi run whose policy could gate a + // built-in tool cannot proceed without the permission extension installed (Decision 2). + if (localBuiltinGatingUnenforceable) { + throw new Error(PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE); + } // Persist events in-process so a follow-up turn can resume by session id. const persist = deps.createPersist?.() ?? new InMemorySessionPersistDriver(); @@ -1328,11 +1357,18 @@ export async function acquireEnvironment( // cannot be delivered (fail loud — this path requires it). let internalToolMcp: ToolMcpAssets | undefined; if (plan.isDaytona) { - await (deps.prepareDaytonaPiAssets ?? prepareDaytonaPiAssets)({ + const daytonaExtensionInstalled = await ( + deps.prepareDaytonaPiAssets ?? prepareDaytonaPiAssets + )({ sandbox: environment.sandbox, plan: { ...plan, skillDirs: [] }, log: logger, }); + // Fail closed (Decision 2): same guarantee as the local path. A genuine upload failure on the + // Daytona sandbox stops the run rather than running Pi's built-in tools unprotected. + if (plan.isPi && plan.builtinGatingActive && !daytonaExtensionInstalled) { + throw new Error(PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE); + } if (!plan.isPi && plan.executableToolSpecs.length > 0) { internalToolMcp = await ( deps.uploadToolMcpAssets ?? uploadToolMcpAssets @@ -1356,9 +1392,9 @@ export async function acquireEnvironment( const storeEndpoint = environment.mountCreds.endpoint; const endpoint = storeReachableFromSandbox(storeEndpoint) ? undefined - : (await (deps.discoverTunnelEndpoint ?? discoverTunnelEndpoint)({ + : ((await (deps.discoverTunnelEndpoint ?? discoverTunnelEndpoint)({ log: logger, - })) ?? undefined; + })) ?? undefined); const canMount = storeReachableFromSandbox(storeEndpoint) || !!endpoint; if ( canMount && @@ -1411,9 +1447,9 @@ export async function acquireEnvironment( const storeEndpoint = environment.agentMountCreds.endpoint; const endpoint = storeReachableFromSandbox(storeEndpoint) ? undefined - : (await (deps.discoverTunnelEndpoint ?? discoverTunnelEndpoint)({ + : ((await (deps.discoverTunnelEndpoint ?? discoverTunnelEndpoint)({ log: logger, - })) ?? undefined; + })) ?? undefined); const canMount = storeReachableFromSandbox(storeEndpoint) || !!endpoint; const mountPath = agentMountDir; if ( diff --git a/services/runner/src/engines/sandbox_agent/daytona.ts b/services/runner/src/engines/sandbox_agent/daytona.ts index 33bae19514..52cea644ab 100644 --- a/services/runner/src/engines/sandbox_agent/daytona.ts +++ b/services/runner/src/engines/sandbox_agent/daytona.ts @@ -154,20 +154,26 @@ export interface PrepareDaytonaPiAssetsInput { /** * Push the Pi login fallback, Agenta extension, forced skills, system prompts, and optional - * Pi CLI install into a Daytona sandbox. + * Pi CLI install into a Daytona sandbox. Reports whether the permission extension installed so the + * caller can fail the run closed when the policy could gate a Pi built-in tool. A non-Pi run needs + * no extension, so it reports `true` (nothing to enforce here). */ export async function prepareDaytonaPiAssets({ sandbox, plan, log = () => {}, -}: PrepareDaytonaPiAssetsInput): Promise { - if (!plan.isPi) return; +}: PrepareDaytonaPiAssetsInput): Promise { + if (!plan.isPi) return true; // A Daytona run never receives the runner's own Pi login: subscription (runtime_provided) auth // is rejected for Daytona in buildRunPlan, and a managed run authenticates from the vault keys // in `daytonaEnvVars`. The runner therefore uploads only the inert Agenta extension, forced // skills, and system prompts — never a personal `auth.json` (interface.md section 6). - await uploadPiExtensionToSandbox(sandbox, DAYTONA_PI_DIR, log); + const extensionInstalled = await uploadPiExtensionToSandbox( + sandbox, + DAYTONA_PI_DIR, + log, + ); if (plan.skillDirs.length > 0) { await uploadSkillsToSandbox(sandbox, DAYTONA_PI_DIR, plan.skillDirs, log); } @@ -185,6 +191,7 @@ export async function prepareDaytonaPiAssets({ log( `[timing] stage=pi_install ms=${Math.round(Date.now() - piInstallStartedAt)} sandbox=${sandbox?.sandboxId ?? "-"} session=-`, ); + return extensionInstalled; } /** diff --git a/services/runner/src/engines/sandbox_agent/pi-assets.ts b/services/runner/src/engines/sandbox_agent/pi-assets.ts index 11dbb7297b..a8ee344530 100644 --- a/services/runner/src/engines/sandbox_agent/pi-assets.ts +++ b/services/runner/src/engines/sandbox_agent/pi-assets.ts @@ -243,11 +243,32 @@ export async function materializeDaytonaPiSkillSnapshot( } } -// The bundled Agenta Pi extension (tracing + tools). Built by `pnpm run build:extension` -// and baked into the image; installed into Pi's agent dir so Pi loads it on every run. -export const EXTENSION_BUNDLE = - process.env.SANDBOX_AGENT_EXTENSION_BUNDLE ?? - join(PKG_ROOT, "dist", "extensions", "agenta.js"); +// The bundled Agenta Pi extension (tracing + tools + permission gating). Built by +// `pnpm run build:extension` and baked into the image; installed into Pi's agent dir so Pi loads +// it on every run. Resolved lazily (a function, not a module-level const) so +// `SANDBOX_AGENT_EXTENSION_BUNDLE` is honored at call time — tests point it at a fixture, and it +// mirrors `toolMcpBundlePath()` in tool-mcp-assets.ts. The override selects code, so it is trusted +// deployment configuration, never run or request configuration. +export function extensionBundlePath(): string { + return ( + process.env.SANDBOX_AGENT_EXTENSION_BUNDLE ?? + join(PKG_ROOT, "dist", "extensions", "agenta.js") + ); +} + +/** + * Thrown when the policy could gate a Pi built-in tool (`builtinGatingActive`) but the Agenta + * permission extension could not be installed, so Pi would run its built-in tools with NO policy + * enforcement. Fail closed: stop the run rather than run unprotected. Mirrors + * `TOOL_MCP_UNAVAILABLE_MESSAGE` in tool-mcp-assets.ts — a named message the engine's own catch + * turns into `{ ok: false, error }` and a visible error frame. Single line so `conciseError` + * (which keeps only the first line) surfaces it verbatim. + */ +export const PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE = + "The agent could not enforce its permission policy: the permission component failed to install, " + + "so no built-in tool could be gated. The run was stopped so no tool ran outside your policy. " + + "Ask your deployment operator to make the runner's Pi agent directory writable, or rebuild and " + + "republish the runner image."; /** * Env the Agenta Pi extension reads. Tool env contains only public metadata plus the @@ -331,23 +352,28 @@ export function writeOtlpAuthFile( } } -/** Install the extension bundle into a local Pi agent dir's extensions/. Best-effort. */ +/** + * Install the extension bundle into a local Pi agent dir's extensions/. Reports whether the + * install succeeded so the caller can fail closed when the policy needs it (a missing bundle or + * an unwritable dir returns false rather than silently proceeding with no enforcement). + */ export function installPiExtensionLocal( agentDir: string, log: Log = () => {}, -): void { - if (!existsSync(EXTENSION_BUNDLE)) { - log( - `pi extension bundle missing at ${EXTENSION_BUNDLE} (run build:extension)`, - ); - return; +): boolean { + const bundle = extensionBundlePath(); + if (!existsSync(bundle)) { + log(`pi extension bundle missing at ${bundle} (run build:extension)`); + return false; } try { const dir = join(agentDir, "extensions"); mkdirSync(dir, { recursive: true }); - copyFileSync(EXTENSION_BUNDLE, join(dir, "agenta.js")); + copyFileSync(bundle, join(dir, "agenta.js")); + return true; } catch (err) { log(`pi extension install skipped: ${(err as Error).message}`); + return false; } } @@ -404,33 +430,52 @@ export async function uploadSystemPromptToSandbox( } } -/** Upload the extension bundle into a Daytona sandbox's Pi extensions dir. Best-effort. */ +/** + * Upload the extension bundle into a Daytona sandbox's Pi extensions dir. Reports whether the + * upload succeeded so the caller can fail closed when the policy needs enforcement (a missing + * bundle or a failed upload returns false rather than silently proceeding with no enforcement). + */ export async function uploadPiExtensionToSandbox( sandbox: any, agentDir: string, log: Log = () => {}, -): Promise { - if (!existsSync(EXTENSION_BUNDLE)) return; +): Promise { + const bundle = extensionBundlePath(); + if (!existsSync(bundle)) { + log(`pi extension bundle missing at ${bundle} (run build:extension)`); + return false; + } try { const dir = `${agentDir}/extensions`; await sandbox.mkdirFs({ path: dir }); await sandbox.writeFsFile( { path: `${dir}/agenta.js` }, - readFileSync(EXTENSION_BUNDLE, "utf-8"), + readFileSync(bundle, "utf-8"), ); + return true; } catch (err) { log(`pi extension upload skipped: ${(err as Error).message}`); + return false; } } +export interface PreparedLocalAgentDir { + /** The throwaway per-run agent dir the runtime user owns (under the system temp path). */ + dir: string; + /** False when the Agenta permission extension could not be installed into it. */ + extensionInstalled: boolean; +} + /** * Seed a throwaway local Pi agent dir from `sourceAgentDir` and install the Agenta extension - * into it. + * into it. The temp dir lives under the system temp path, which the runtime user owns, so the + * install never depends on the operator-configured `PI_CODING_AGENT_DIR` being writable. Reports + * whether the extension install succeeded. */ export function prepareLocalAgentDir( sourceAgentDir: string, log: Log = () => {}, -): string { +): PreparedLocalAgentDir { const dir = mkdtempSync(join(tmpdir(), "agenta-pi-agentdir-")); for (const name of ["auth.json", "settings.json"]) { const src = join(sourceAgentDir, name); @@ -440,8 +485,8 @@ export function prepareLocalAgentDir( log(`agent-dir seed skipped for ${name}: ${(err as Error).message}`); } } - installPiExtensionLocal(dir, log); - return dir; + const extensionInstalled = installPiExtensionLocal(dir, log); + return { dir, extensionInstalled }; } export interface PrepareLocalPiAssetsInput { @@ -460,10 +505,23 @@ export interface PrepareLocalPiAssetsInput { log?: Log; } +export interface PrepareLocalPiAssetsResult { + /** + * The THROWAWAY per-run dir when one was created — `undefined` means "nothing here for the caller + * to delete" (the caller `rmSync`s `dir` at teardown, so the operator's own login must never be + * returned). + */ + dir: string | undefined; + /** + * False when the Agenta permission extension could not be installed for this run. The caller + * fails the run closed when the policy could gate a Pi built-in tool (`builtinGatingActive`). + */ + extensionInstalled: boolean; +} + /** - * Prepare local Pi's agent dir assets and return the THROWAWAY per-run dir when one was created — - * `undefined` means "nothing here for the caller to delete" (the caller `rmSync`s whatever it gets - * back at teardown, so the operator's own login must never be returned). + * Prepare local Pi's agent dir assets and report the throwaway dir plus whether the permission + * extension installed. * * Two shapes: * @@ -471,10 +529,13 @@ export interface PrepareLocalPiAssetsInput { * mounted login, exactly like a normal local Pi install. Pi refreshes its OAuth token mid-run and * writes the new one back into its agent dir; a per-run copy would throw that refresh away, so * once the provider rotated the refresh token the next run would fail and the operator would have - * to log in by hand. Returns `undefined` so teardown cannot delete the mount. - * - Managed / none: no credential to preserve, so skills and system prompts still get an isolated - * per-run copy (returned, and deleted at teardown). A plain run with neither only installs the - * inert Agenta extension into the configured shared dir. + * to log in by hand. Returns `dir: undefined` so teardown cannot delete the mount. The extension + * still installs into the mount; a non-writable mount reports `extensionInstalled: false`. + * - Managed / none: no credential to preserve, so every run (plain, or with skills / a system + * prompt) gets an isolated per-run copy under the system temp path the runtime user owns, and the + * extension installs there. This removes the fail-open that shipped when the configured + * `PI_CODING_AGENT_DIR` (e.g. `/pi-agent` on the published image) was not writable by the runtime + * user: the install no longer depends on that directory being writable. * * Tradeoff (interface.md section 6): concurrent local subscription runs share the one agent dir, * the same way two local `pi` sessions do. This path is single-trusted-operator only. @@ -483,14 +544,16 @@ export function prepareLocalPiAssets({ plan, env, log = () => {}, -}: PrepareLocalPiAssetsInput): string | undefined { - if (!plan.isPi || plan.isDaytona) return undefined; +}: PrepareLocalPiAssetsInput): PrepareLocalPiAssetsResult { + // Not a local Pi run: nothing to install here, so enforcement is not this path's concern. + if (!plan.isPi || plan.isDaytona) + return { dir: undefined, extensionInstalled: true }; // buildRunPlan already rejected a local runtime_provided run with no configured // PI_CODING_AGENT_DIR, so `sourcePiAgentDir` here IS the operator's mount. if (plan.credentialMode === "runtime_provided") { const agentDir = plan.sourcePiAgentDir; - installPiExtensionLocal(agentDir, log); + const extensionInstalled = installPiExtensionLocal(agentDir, log); if (plan.hasSystemPrompt) { writeSystemPromptLocal( agentDir, @@ -501,32 +564,25 @@ export function prepareLocalPiAssets({ } env.PI_CODING_AGENT_DIR = agentDir; // Deliberately NOT returned as a throwaway: this is the operator's login, not a temp dir. - return undefined; + return { dir: undefined, extensionInstalled }; } - if (plan.skillDirs.length > 0 || plan.hasSystemPrompt) { - const runAgentDir = prepareLocalAgentDir(plan.sourcePiAgentDir, log); - if (plan.hasSystemPrompt) { - writeSystemPromptLocal( - runAgentDir, - plan.systemPrompt, - plan.appendSystemPrompt, - log, - ); - } - env.PI_CODING_AGENT_DIR = runAgentDir; - return runAgentDir; - } - - if (process.env.PI_CODING_AGENT_DIR) { - installPiExtensionLocal(process.env.PI_CODING_AGENT_DIR, log); - } else { - // unset here means this run has no Agenta extension (tracing + tools); warn so it's visible. - log( - "PI_CODING_AGENT_DIR is unset; plain local Pi run has no Agenta extension installed", + // Managed / none: always route through a throwaway per-run dir the runtime user owns, so the + // extension install never depends on the configured PI_CODING_AGENT_DIR being writable. + const { dir: runAgentDir, extensionInstalled } = prepareLocalAgentDir( + plan.sourcePiAgentDir, + log, + ); + if (plan.hasSystemPrompt) { + writeSystemPromptLocal( + runAgentDir, + plan.systemPrompt, + plan.appendSystemPrompt, + log, ); } - return undefined; + env.PI_CODING_AGENT_DIR = runAgentDir; + return { dir: runAgentDir, extensionInstalled }; } /** Upload materialized skill dirs into a Daytona sandbox's Pi `skills/` user scope. */ diff --git a/services/runner/tests/setup/hermetic-env.ts b/services/runner/tests/setup/hermetic-env.ts index cf23385c76..810706486d 100644 --- a/services/runner/tests/setup/hermetic-env.ts +++ b/services/runner/tests/setup/hermetic-env.ts @@ -6,10 +6,29 @@ * * A test that wants either is free to set it back with vi.stubEnv. */ +import { writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + import { beforeEach } from "vitest"; import { resetRunnerConfigCache } from "../../src/config/runner-config.ts"; +// The permission-enforcing Pi extension bundle is built by `pnpm run build:extension` into +// `dist/`, which CI does NOT run before the unit suite (only dev builds it). Since the runner now +// FAILS CLOSED when that extension cannot install, a local Pi run whose policy could gate a +// built-in tool would throw in CI purely because the bundle file is absent. Point the bundle at a +// tiny stub file so the install succeeds by default and tests exercise the real gating logic, not +// a missing-artifact accident. A test that wants the install to FAIL overrides this env var (and +// restores it). Its contents are irrelevant — nothing loads it in a unit test. +const STUB_EXTENSION_BUNDLE = join(tmpdir(), "agenta-runner-test-extension.js"); +writeFileSync( + STUB_EXTENSION_BUNDLE, + "// test stub extension bundle\n", + "utf-8", +); +process.env.SANDBOX_AGENT_EXTENSION_BUNDLE = STUB_EXTENSION_BUNDLE; + const SCRUBBED = [ "AGENTA_INSECURE_EGRESS_ALLOWED", "AGENTA_WEBHOOKS_ALLOW_INSECURE", @@ -37,5 +56,7 @@ for (const name of SCRUBBED) delete process.env[name]; // runner config so the next `loadRunnerConfig()` re-parses the scrubbed environment. beforeEach(() => { for (const name of SCRUBBED) delete process.env[name]; + // Restore the stub bundle so a prior test's override (to force a failed install) cannot leak. + process.env.SANDBOX_AGENT_EXTENSION_BUNDLE = STUB_EXTENSION_BUNDLE; resetRunnerConfigCache(); }); diff --git a/services/runner/tests/unit/pi-permission-failclosed.test.ts b/services/runner/tests/unit/pi-permission-failclosed.test.ts new file mode 100644 index 0000000000..7c5eaae714 --- /dev/null +++ b/services/runner/tests/unit/pi-permission-failclosed.test.ts @@ -0,0 +1,191 @@ +/** + * Session-setup fail-closed behavior (Decision 2 of the permission-failopen plan). + * + * `acquireEnvironment` must STOP a local Pi run whose permission policy could gate a built-in tool + * when the Agenta permission extension did not install — rather than running those tools with no + * enforcement (the fail-open the plan fixes). When the policy is allow-everything the extension is + * not needed, so a failed install is harmless and the run continues. + * + * Covered for BOTH sandboxes: the local path forces a failed install by pointing + * SANDBOX_AGENT_EXTENSION_BUNDLE at a path that does not exist (the shared hermetic setup otherwise + * points it at a real stub); the Daytona path injects a `prepareDaytonaPiAssets` that reports a + * failed upload. No live Pi/sandbox is started: each gating-active case fails at the guard, and + * each allow-everything case is stopped with an injected sentinel at the next acquire stage, + * proving it got past the guard. + * + * Run: pnpm test (or: pnpm exec vitest run tests/unit/pi-permission-failclosed.test.ts) + */ +import { describe, it, afterEach } from "vitest"; +import assert from "node:assert/strict"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import type { AgentRunRequest } from "../../src/protocol.ts"; +import type { SandboxAgentDeps } from "../../src/engines/sandbox_agent.ts"; +import { acquireEnvironment } from "../../src/engines/sandbox_agent.ts"; +import { PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE } from "../../src/engines/sandbox_agent/pi-assets.ts"; +import { resetRunnerConfigCache } from "../../src/config/runner-config.ts"; + +const MISSING_BUNDLE = join(tmpdir(), "agenta-missing-extension-bundle.js"); + +function forceFailedInstall(): void { + process.env.SANDBOX_AGENT_EXTENSION_BUNDLE = MISSING_BUNDLE; +} + +afterEach(() => { + // The hermetic setup's beforeEach restores the stub bundle, but keep this explicit too. + delete process.env.SANDBOX_AGENT_EXTENSION_BUNDLE; +}); + +describe("acquireEnvironment fail-closed on a missing Pi permission extension", () => { + it("stops a local Pi run with ok:false and the named message when gating is active and the install failed", async () => { + forceFailedInstall(); + + const request: AgentRunRequest = { + harness: "pi_core", + messages: [{ role: "user", content: "run a shell command" }], + // default "deny" makes builtinGatingActive true, so the extension is required. + permissions: { default: "deny" }, + } as AgentRunRequest; + + const result = await acquireEnvironment(request, {}); + + assert.equal(result.ok, false); + if (result.ok) return; + assert.equal(result.error, PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE); + }); + + it("continues an allow-everything local Pi run even when the install failed (extension not needed)", async () => { + forceFailedInstall(); + + const SENTINEL = "pi-failclosed-test-reached-sandbox-start"; + + const request: AgentRunRequest = { + harness: "pi_core", + messages: [{ role: "user", content: "hello" }], + // default "allow" with no built-in rules => builtinGatingActive false => no enforcement need. + permissions: { default: "allow" }, + } as AgentRunRequest; + + const result = await acquireEnvironment(request, { + // The run must get PAST the fail-closed guard; stop it the instant it reaches sandbox startup + // so we neither hit real infra nor mistake an early abort for the guard firing. + startSandboxAgent: (async () => { + throw new Error(SENTINEL); + }) as typeof import("sandbox-agent").SandboxAgent.start, + }); + + assert.equal(result.ok, false); + if (result.ok) return; + // Reaching the sandbox-start sentinel proves the guard did NOT stop the run — the failed + // install was correctly treated as harmless under an allow-everything policy. + assert.match(result.error, new RegExp(SENTINEL)); + assert.notEqual(result.error, PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE); + }); +}); + +// --- Daytona path: the same guarantee for the remote-sandbox extension upload ------------- // + +/** Minimal fake Daytona sandbox that records teardown calls; no live Daytona is touched. */ +function fakeDaytonaSandbox() { + const teardown = { destroyed: 0, disposed: 0 }; + const sandbox: any = { + sandboxId: "sbx-failclosed-test", + async destroySession() {}, + async destroySandbox() { + teardown.destroyed += 1; + }, + async dispose() { + teardown.disposed += 1; + }, + }; + return { sandbox, teardown }; +} + +/** + * Deps that carry a Daytona Pi run exactly up to the extension-upload seam with fakes: the + * provider/daemon/cwd shims never touch infra, `startSandboxAgent` returns the fake sandbox, and + * `prepareDaytonaPiAssets` reports the upload outcome under test. + */ +function daytonaDeps( + sandbox: any, + extensionInstalled: boolean, + extra: Partial = {}, +): SandboxAgentDeps { + return { + createDaytonaCwd: () => "/home/sandbox/agenta-failclosed-test", + resolveSkillDirs: () => ({ skills: [], cleanup: () => {} }), + buildDaemonEnv: () => ({}), + resolveDaemonBinary: () => "/bin/sandbox-agent", + buildSandboxProvider: (() => ({}) as any) as any, + startSandboxAgent: (async () => sandbox) as any, + prepareDaytonaPiAssets: (async () => extensionInstalled) as any, + ...extra, + }; +} + +describe("acquireEnvironment fail-closed on a failed Daytona extension upload", () => { + // The hermetic setup scrubs the Daytona env before every test; enable the provider (with a + // provisioning credential) per test and drop the memoized config so the run plan accepts it. + function enableDaytona(): void { + process.env.AGENTA_RUNNER_ENABLED_SANDBOX_PROVIDERS = "local,daytona"; + process.env.AGENTA_RUNNER_DAYTONA_API_KEY = "test-key"; + resetRunnerConfigCache(); + } + + it("stops a Daytona Pi run with ok:false, the named message, and sandbox teardown when gating is active and the upload failed", async () => { + enableDaytona(); + const { sandbox, teardown } = fakeDaytonaSandbox(); + + const request: AgentRunRequest = { + harness: "pi_core", + sandbox: "daytona", + messages: [{ role: "user", content: "run a shell command" }], + permissions: { default: "deny" }, + } as AgentRunRequest; + + const result = await acquireEnvironment( + request, + daytonaDeps(sandbox, false), + ); + + assert.equal(result.ok, false); + if (result.ok) return; + assert.equal(result.error, PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE); + // The half-built environment must not leak: the engine's catch tears the sandbox down. + assert.equal(teardown.destroyed, 1); + assert.equal(teardown.disposed, 1); + }); + + it("continues an allow-everything Daytona Pi run even when the upload failed (extension not needed)", async () => { + enableDaytona(); + const { sandbox } = fakeDaytonaSandbox(); + + const SENTINEL = "pi-failclosed-test-reached-prepare-workspace"; + + const request: AgentRunRequest = { + harness: "pi_core", + sandbox: "daytona", + messages: [{ role: "user", content: "hello" }], + permissions: { default: "allow" }, + } as AgentRunRequest; + + const result = await acquireEnvironment( + request, + daytonaDeps(sandbox, false, { + // The run must get PAST the fail-closed check (which sits right after the upload); stop + // it at the next acquire stage, workspace preparation, so no further infra is touched. + prepareWorkspace: (async () => { + throw new Error(SENTINEL); + }) as any, + }), + ); + + assert.equal(result.ok, false); + if (result.ok) return; + // Reaching the workspace sentinel proves the check did NOT stop the run — the failed upload + // was correctly treated as harmless under an allow-everything policy. + assert.match(result.error, new RegExp(SENTINEL)); + assert.notEqual(result.error, PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE); + }); +}); diff --git a/services/runner/tests/unit/sandbox-agent-orchestration.test.ts b/services/runner/tests/unit/sandbox-agent-orchestration.test.ts index 35d3ee5fbb..db7ad02bb1 100644 --- a/services/runner/tests/unit/sandbox-agent-orchestration.test.ts +++ b/services/runner/tests/unit/sandbox-agent-orchestration.test.ts @@ -394,7 +394,7 @@ describe("runSandboxAgent orchestration", () => { it("creates the configured Pi transcript directory inside a Daytona cwd", async () => { const { calls, deps } = fakeHarness(); - deps.prepareDaytonaPiAssets = (async () => {}) as any; + deps.prepareDaytonaPiAssets = (async () => true) as any; const result = await runSandboxAgent( { @@ -482,6 +482,7 @@ describe("runSandboxAgent orchestration", () => { let agentDirSkillCount = -1; deps.prepareDaytonaPiAssets = (async ({ plan }: any) => { agentDirSkillCount = plan.skillDirs.length; + return true; }) as any; const result = await runSandboxAgent( diff --git a/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts b/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts index 952c15406c..dddcabc200 100644 --- a/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts +++ b/services/runner/tests/unit/sandbox-agent-pi-assets.test.ts @@ -58,10 +58,7 @@ describe("Pi session workspace", () => { const env: Record = {}; assert.equal( - configurePiSessionWorkspace( - { isPi: false, cwd: "/work/session-1" }, - env, - ), + configurePiSessionWorkspace({ isPi: false, cwd: "/work/session-1" }, env), undefined, ); assert.equal(env.PI_CODING_AGENT_SESSION_DIR, undefined); @@ -357,10 +354,11 @@ describe("prepareLocalAgentDir", () => { writeFileSync(join(source, "auth.json"), '{"token":"x"}', "utf-8"); writeFileSync(join(source, "settings.json"), '{"model":"gpt"}', "utf-8"); - const runDir = prepareLocalAgentDir(source); + const { dir: runDir, extensionInstalled } = prepareLocalAgentDir(source); dirs.push(runDir); assert.notEqual(runDir, source); + assert.equal(extensionInstalled, true); assert.equal( readFileSync(join(runDir, "auth.json"), "utf-8"), '{"token":"x"}', @@ -371,17 +369,28 @@ describe("prepareLocalAgentDir", () => { ); assert.equal(existsSync(join(runDir, "skills")), false); }); -}); - -describe("prepareLocalPiAssets (PI_CODING_AGENT_DIR guard)", () => { - const ENV_VAR = "PI_CODING_AGENT_DIR"; - const previous = process.env[ENV_VAR]; - afterEach(() => { - if (previous === undefined) delete process.env[ENV_VAR]; - else process.env[ENV_VAR] = previous; + it("reports failure when the extension cannot be installed (bundle missing)", () => { + const source = tempDir("agenta-pi-source-nobundle-"); + const previous = process.env.SANDBOX_AGENT_EXTENSION_BUNDLE; + process.env.SANDBOX_AGENT_EXTENSION_BUNDLE = join( + tmpdir(), + "agenta-nonexistent-extension-bundle.js", + ); + try { + const { dir: runDir, extensionInstalled } = prepareLocalAgentDir(source); + dirs.push(runDir); + assert.equal(extensionInstalled, false); + assert.equal(existsSync(join(runDir, "extensions", "agenta.js")), false); + } finally { + if (previous === undefined) + delete process.env.SANDBOX_AGENT_EXTENSION_BUNDLE; + else process.env.SANDBOX_AGENT_EXTENSION_BUNDLE = previous; + } }); +}); +describe("prepareLocalPiAssets (managed/none routes through a throwaway dir)", () => { const plainPiPlan = { isPi: true, isDaytona: false, @@ -392,35 +401,43 @@ describe("prepareLocalPiAssets (PI_CODING_AGENT_DIR guard)", () => { sourcePiAgentDir: "/unused", }; - it("logs a clear warning when a plain local Pi run has no PI_CODING_AGENT_DIR", () => { - delete process.env[ENV_VAR]; - const logs: string[] = []; + it("installs the extension into a per-run temp dir it owns, independent of PI_CODING_AGENT_DIR", () => { + const env: Record = {}; - const runDir = prepareLocalPiAssets({ + const { dir: runDir, extensionInstalled } = prepareLocalPiAssets({ plan: plainPiPlan, - env: {}, - log: (msg) => logs.push(msg), + env, }); - assert.equal(runDir, undefined); - assert.ok( - logs.some((m) => m.includes("PI_CODING_AGENT_DIR is unset")), - `expected a PI_CODING_AGENT_DIR warning, got: ${JSON.stringify(logs)}`, + assert.ok(runDir, "a plain local Pi run gets a throwaway per-run dir"); + assert.notEqual(runDir, "/unused"); + assert.equal(env.PI_CODING_AGENT_DIR, runDir); + assert.equal(extensionInstalled, true); + assert.equal( + existsSync(join(runDir as string, "extensions", "agenta.js")), + true, ); + dirs.push(runDir as string); }); - it("installs the extension silently (no warning) when PI_CODING_AGENT_DIR is set", () => { - const dir = tempDir("agenta-pi-configured-dir-"); - process.env[ENV_VAR] = dir; - const logs: string[] = []; - - prepareLocalPiAssets({ - plan: plainPiPlan, - env: {}, - log: (msg) => logs.push(msg), - }); - - assert.ok(!logs.some((m) => m.includes("PI_CODING_AGENT_DIR is unset"))); + it("reports extensionInstalled=false when the extension could not be installed", () => { + const previous = process.env.SANDBOX_AGENT_EXTENSION_BUNDLE; + process.env.SANDBOX_AGENT_EXTENSION_BUNDLE = join( + tmpdir(), + "agenta-nonexistent-extension-bundle.js", + ); + try { + const { dir: runDir, extensionInstalled } = prepareLocalPiAssets({ + plan: plainPiPlan, + env: {}, + }); + if (runDir) dirs.push(runDir); + assert.equal(extensionInstalled, false); + } finally { + if (previous === undefined) + delete process.env.SANDBOX_AGENT_EXTENSION_BUNDLE; + else process.env.SANDBOX_AGENT_EXTENSION_BUNDLE = previous; + } }); }); @@ -555,7 +572,7 @@ describe("prepareLocalPiAssets (runtime_provided runs out of the mount, read-wri const mount = tempDir("agenta-pi-subscription-mount-"); writeFileSync(join(mount, "auth.json"), '{"token":"live"}', "utf-8"); - const runDir = prepareLocalPiAssets({ + const { dir: runDir } = prepareLocalPiAssets({ plan: subscriptionPlan(mount, { skillDirs: [], hasSystemPrompt: true, @@ -574,7 +591,7 @@ describe("prepareLocalPiAssets (runtime_provided runs out of the mount, read-wri writeFileSync(join(source, "auth.json"), '{"token":"managed"}', "utf-8"); const env: Record = {}; - const runDir = prepareLocalPiAssets({ + const { dir: runDir } = prepareLocalPiAssets({ plan: subscriptionPlan(source, { credentialMode: "env", hasSystemPrompt: true, diff --git a/services/runner/tests/unit/sandbox-lifecycle.test.ts b/services/runner/tests/unit/sandbox-lifecycle.test.ts index 541d653a04..ab9799ef73 100644 --- a/services/runner/tests/unit/sandbox-lifecycle.test.ts +++ b/services/runner/tests/unit/sandbox-lifecycle.test.ts @@ -123,7 +123,7 @@ function fakeSandbox(sandboxId: string | undefined, opts: FakeOpts = {}) { return sandbox; }) as any, prepareWorkspace: (async () => ({ cleanup: async () => {} })) as any, - prepareDaytonaPiAssets: async () => {}, + prepareDaytonaPiAssets: async () => true, discoverTunnelEndpoint: async () => null, probeCapabilities: async () => ({ @@ -351,7 +351,11 @@ describe("remote sandbox reconnect ladder", () => { await runSandboxAgent(daytonaRequest, undefined, undefined, deps); - assert.equal(calls.starts.length, 1, "no delete-and-rebuild, a single reconnect"); + assert.equal( + calls.starts.length, + 1, + "no delete-and-rebuild, a single reconnect", + ); assert.equal(calls.starts[0].sandboxId, "sbx-trusted"); assert.ok( !calls.logs.some((message) => message.includes("compatibility teardown")),