From 174f854b966b6d196f8f03d843ed5223b7d47111 Mon Sep 17 00:00:00 2001 From: mptyl Date: Wed, 5 Aug 2026 01:19:46 +0200 Subject: [PATCH] fix: isolate pi provider smoke --- backend/src/pi/provider-smoke.ts | 89 ++++++++++++++++++++- backend/test/pi-management.test.ts | 5 +- backend/test/pi-provider-smoke.test.ts | 104 +++++++++++++++++++++++-- 3 files changed, 188 insertions(+), 10 deletions(-) diff --git a/backend/src/pi/provider-smoke.ts b/backend/src/pi/provider-smoke.ts index 6b321ef3..fc8400d1 100644 --- a/backend/src/pi/provider-smoke.ts +++ b/backend/src/pi/provider-smoke.ts @@ -1,4 +1,7 @@ import { spawn as nodeSpawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { homedir, tmpdir } from "node:os"; +import { join } from "node:path"; import type { AppConfig } from "../config.js"; import { secretValue } from "../config/secret-bundle.js"; import { clearPrincipalEnvironment } from "../auth/principal.js"; @@ -7,7 +10,18 @@ import { loadPiAuthProviders } from "./auth-providers.js"; import { buildPiChildEnv, canonicalPiProvider } from "./provider-credentials.js"; import type { PiReasoning } from "./management.js"; -const SMOKE_PROMPT = "Provider health check only. Reply with exactly OK without using tools."; +const SMOKE_PROMPT = "Provider health check. Reply with exactly OK."; +const SMOKE_ARGS = [ + "--mode", "rpc", + "--no-session", + "--no-tools", + "--no-extensions", + "--no-skills", + "--no-prompt-templates", + "--no-themes", + "--no-context-files", + "--no-approve", +] as const; export interface PiProviderSmokeRequest { provider: string; @@ -25,6 +39,7 @@ interface ProviderSmokeOptions { options: { cwd: string; env: NodeJS.ProcessEnv }, ) => ChildProcessWithoutNullStreams; authProviders?: () => ReadonlySet; + readAuthStore?: () => string; } export function createPiProviderSmoke( @@ -36,23 +51,43 @@ export function createPiProviderSmoke( return async ({ provider, model, reasoning, timeoutMs }): Promise => { let child: ChildProcessWithoutNullStreams | undefined; + let isolatedRoot: string | undefined; let timer: NodeJS.Timeout | undefined; try { const canonicalProvider = canonicalPiProvider(provider); if (!canonicalProvider || timeoutMs <= 0) throw providerFailure(); + const configuredAuthProviders = authProviders(); const env = buildPiChildEnv({ provider: canonicalProvider, - authProviders: authProviders(), + authProviders: configuredAuthProviders, credentialValue: secretValue(config, "THT_MODEL_API_KEY"), credentialFile: config.modelApiKeyFile, }); clearPrincipalEnvironment(env); delete env.THT_DATA_ROOT; - if (config.dataRoot !== undefined) env.THT_DATA_ROOT = config.dataRoot; + delete env.THT_SESSION; + delete env.THT_AUTHOR; + delete env.THT_CONFIG; + delete env.PI_CODING_AGENT_SESSION_DIR; - child = spawnFn(config.piBin, ["--mode", "rpc"], { cwd: config.harnessDir, env }); + isolatedRoot = mkdtempSync(join(tmpdir(), "thothii-pi-smoke-")); + const isolatedCwd = join(isolatedRoot, "work"); + const isolatedAgentDir = join(isolatedRoot, "agent"); + mkdirSync(isolatedCwd, { mode: 0o700 }); + mkdirSync(isolatedAgentDir, { mode: 0o700 }); + if (configuredAuthProviders.has(canonicalProvider)) { + const authStore = selectedProviderAuthStore( + options.readAuthStore?.() ?? readConfiguredAuthStore(), + canonicalProvider, + ); + writeFileSync(join(isolatedAgentDir, "auth.json"), authStore, { mode: 0o600, flag: "wx" }); + } + env.PI_CODING_AGENT_DIR = isolatedAgentDir; + + child = spawnFn(config.piBin, [...SMOKE_ARGS], { cwd: isolatedCwd, env }); child.stderr.resume(); const rpc = new RpcClient(child); + const capabilityGuard = failOnUnexpectedCapabilities(rpc); const turn = async (): Promise => { requireSuccessfulResponse(await rpc.request({ type: "set_model", provider: canonicalProvider, modelId: model, @@ -64,6 +99,7 @@ export function createPiProviderSmoke( }; await Promise.race([ turn(), + capabilityGuard, new Promise((_resolve, reject) => { timer = setTimeout(() => reject(providerTimeout()), timeoutMs); }), @@ -76,10 +112,55 @@ export function createPiProviderSmoke( if (child) { try { child.kill(); } catch { /* preserve the sanitized smoke outcome */ } } + if (isolatedRoot) { + try { rmSync(isolatedRoot, { recursive: true, force: true, maxRetries: 2 }); } catch { + /* preserve the sanitized smoke outcome; the OS temp directory remains isolated */ + } + } } }; } +function failOnUnexpectedCapabilities(rpc: RpcClient): Promise { + return new Promise((_resolve, reject) => { + rpc.on("event", (event) => { + if (isUnexpectedCapabilityEvent(event)) reject(providerFailure()); + }); + }); +} + +function isUnexpectedCapabilityEvent(event: any): boolean { + const type = typeof event?.type === "string" ? event.type : ""; + if (type.startsWith("tool_") || type.startsWith("toolcall_") || type.startsWith("extension_")) { + return true; + } + const updateType = event?.assistantMessageEvent?.type; + if (typeof updateType === "string" && updateType.startsWith("toolcall_")) return true; + if (Array.isArray(event?.toolResults) && event.toolResults.length > 0) return true; + if (messageUsesTool(event?.message)) return true; + return Array.isArray(event?.messages) && event.messages.some(messageUsesTool); +} + +function messageUsesTool(message: any): boolean { + return message?.role === "toolResult" || message?.stopReason === "toolUse" + || (Array.isArray(message?.content) + && message.content.some((content: any) => content?.type === "toolCall")); +} + +function readConfiguredAuthStore(): string { + const configuredAgentDir = process.env.PI_CODING_AGENT_DIR ?? join(homedir(), ".pi", "agent"); + return readFileSync(join(configuredAgentDir, "auth.json"), "utf8"); +} + +function selectedProviderAuthStore(raw: string, provider: string): string { + const parsed: unknown = JSON.parse(raw); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) throw providerFailure(); + const entry = Object.entries(parsed as Record) + .find(([key]) => key.trim().toLowerCase() === provider); + if (!entry) throw providerFailure(); + return JSON.stringify({ [entry[0]]: entry[1] }); +} + function waitForProviderTurn(rpc: RpcClient, child: ChildProcessWithoutNullStreams): Promise { return new Promise((resolve, reject) => { let failed = false; diff --git a/backend/test/pi-management.test.ts b/backend/test/pi-management.test.ts index 14d3e21a..563aed43 100644 --- a/backend/test/pi-management.test.ts +++ b/backend/test/pi-management.test.ts @@ -47,7 +47,10 @@ test("status parses only a Pi version from a fixed execFile argument array", asy config: { provider: "zai", model: "glm-5.2", reasoning: "medium" }, checkedAt: "2026-08-05T10:00:00.000Z", }); - expect(calls).toEqual([{ command: "/usr/local/bin/pi", args: ["--version"], timeout: 750 }]); + expect(calls).toHaveLength(1); + expect(calls[0]).toMatchObject({ command: "/usr/local/bin/pi", args: ["--version"] }); + expect(calls[0].timeout).toBeGreaterThan(0); + expect(calls[0].timeout).toBeLessThanOrEqual(750); }); // Catches an options response that leaks provider metadata or lets callers choose model IDs that diff --git a/backend/test/pi-provider-smoke.test.ts b/backend/test/pi-provider-smoke.test.ts index e48d8718..d2ac069a 100644 --- a/backend/test/pi-provider-smoke.test.ts +++ b/backend/test/pi-provider-smoke.test.ts @@ -1,8 +1,12 @@ import { EventEmitter } from "node:events"; -import { expect, test, vi } from "vitest"; +import { existsSync, readFileSync, readdirSync } from "node:fs"; +import { dirname } from "node:path"; +import { afterEach, expect, test, vi } from "vitest"; import { loadConfig } from "../src/config.js"; import { createPiProviderSmoke } from "../src/pi/provider-smoke.js"; +afterEach(() => vi.unstubAllEnvs()); + function rpcChild(onCommand: (command: any, emit: (message: unknown) => void) => void) { const child: any = new EventEmitter(); child.stdout = new EventEmitter(); @@ -20,10 +24,17 @@ function rpcChild(onCommand: (command: any, emit: (message: unknown) => void) => return child; } -// Catches a provider smoke implementation that merely selects a model, leaks generated output, -// or fails to terminate its ephemeral Pi process after a real model turn. -test("provider smoke selects the configured model and completes a fixed output-discarding turn", async () => { +// Catches a provider smoke process that runs from the trusted harness or leaves Pi tools, +// extensions, skills, context files, templates, themes, or session persistence enabled. +test("provider smoke makes one configured request from an isolated no-capability Pi process", async () => { + vi.stubEnv("THT_DATA_ROOT", "/mounted-workflow-state"); + vi.stubEnv("THT_SESSION", "mounted-session-id"); + vi.stubEnv("THT_AUTHOR", "mounted-author"); + vi.stubEnv("THT_CONFIG", "/mounted-workflow-state/config.yaml"); + vi.stubEnv("PI_CODING_AGENT_DIR", "/home/thoth/.pi/agent"); + vi.stubEnv("PI_CODING_AGENT_SESSION_DIR", "/mounted-session-state"); const commands: any[] = []; + const spawns: any[][] = []; const child = rpcChild((command, emit) => { commands.push(command); if (command.type === "set_model" || command.type === "set_thinking_level") { @@ -43,14 +54,47 @@ test("provider smoke selects the configured model and completes a fixed output-d const smoke = createPiProviderSmoke(loadConfig({ THT_HARNESS_DIR: "/app/harness", PI_BIN: "/usr/local/bin/pi", + THT_DATA_ROOT: "/mounted-workflow-state", }), { - spawnFn: () => child, + spawnFn: (...args) => { + spawns.push(args); + expect(args[2].cwd).not.toBe("/app/harness"); + expect(readdirSync(args[2].cwd)).toEqual([]); + expect(args[2].env.PI_CODING_AGENT_DIR).not.toBe("/home/thoth/.pi/agent"); + expect(readdirSync(args[2].env.PI_CODING_AGENT_DIR)).toEqual(["auth.json"]); + expect(JSON.parse(readFileSync(`${args[2].env.PI_CODING_AGENT_DIR}/auth.json`, "utf8"))) + .toEqual({ zai: { type: "api_key", key: "test-only" } }); + return child; + }, authProviders: () => new Set(["zai"]), + readAuthStore: () => JSON.stringify({ + zai: { type: "api_key", key: "test-only" }, + deepseek: { type: "api_key", key: "must-not-enter-isolated-context" }, + }), }); await expect(smoke({ provider: "zai", model: "glm-5.2", reasoning: "medium", timeoutMs: 750, })).resolves.toBeUndefined(); + expect(spawns).toHaveLength(1); + expect(spawns[0][0]).toBe("/usr/local/bin/pi"); + expect(spawns[0][1]).toEqual([ + "--mode", "rpc", + "--no-session", + "--no-tools", + "--no-extensions", + "--no-skills", + "--no-prompt-templates", + "--no-themes", + "--no-context-files", + "--no-approve", + ]); + expect(spawns[0][2].env).not.toHaveProperty("THT_DATA_ROOT"); + expect(spawns[0][2].env).not.toHaveProperty("THT_SESSION"); + expect(spawns[0][2].env).not.toHaveProperty("THT_AUTHOR"); + expect(spawns[0][2].env).not.toHaveProperty("THT_CONFIG"); + expect(spawns[0][2].env).not.toHaveProperty("PI_CODING_AGENT_SESSION_DIR"); + expect(existsSync(dirname(spawns[0][2].cwd))).toBe(false); expect(commands.map(({ id: _id, ...command }) => command)).toEqual([ { type: "set_model", provider: "zai", modelId: "glm-5.2" }, { type: "set_thinking_level", level: "medium" }, @@ -59,6 +103,55 @@ test("provider smoke selects the configured model and completes a fixed output-d expect(child.kill).toHaveBeenCalledOnce(); }); +const unexpectedToolEvents = [ + { + name: "streamed tool call", + event: { + type: "message_update", + assistantMessageEvent: { type: "toolcall_start", contentIndex: 0 }, + }, + }, + { + name: "tool execution", + event: { type: "tool_execution_start", toolCallId: "tool-1", toolName: "read" }, + }, + { + name: "completed message tool call", + event: { + type: "message_end", + message: { role: "assistant", stopReason: "toolUse", content: [{ type: "toolCall" }] }, + }, + }, + { + name: "turn tool result", + event: { type: "turn_end", toolResults: [{ role: "toolResult" }] }, + }, +]; + +// Catches Pi/provider regressions that surface a tool capability despite the fixed no-tools argv; +// accepting agent_end after any such event could hide a mounted-state read or mutation. +test.each(unexpectedToolEvents)("provider smoke fails closed on an unexpected $name event", async ({ event }) => { + const child = rpcChild((command, emit) => { + if (command.type === "set_model" || command.type === "set_thinking_level") { + emit({ type: "response", id: command.id, success: true }); + } + if (command.type === "prompt") { + emit(event); + emit({ type: "agent_end", messages: [] }); + } + }); + const smoke = createPiProviderSmoke(loadConfig({}), { + spawnFn: () => child, + authProviders: () => new Set(["zai"]), + readAuthStore: () => '{"zai":{"type":"api_key","key":"test-only"}}', + }); + + await expect(smoke({ + provider: "zai", model: "glm-5.2", reasoning: "medium", timeoutMs: 750, + })).rejects.toThrow("Pi provider smoke check failed"); + expect(child.kill).toHaveBeenCalledOnce(); +}); + // Catches provider errors that are accepted as a successful health check or returned with raw // credential/output diagnostics. test("provider smoke rejects a failed model turn with a stable non-secret error", async () => { @@ -80,6 +173,7 @@ test("provider smoke rejects a failed model turn with a stable non-secret error" const smoke = createPiProviderSmoke(loadConfig({}), { spawnFn: () => child, authProviders: () => new Set(["zai"]), + readAuthStore: () => '{"zai":{"type":"api_key","key":"test-only"}}', }); let caught: unknown;