From 2703864572647cbf4331813f607796b977a53396 Mon Sep 17 00:00:00 2001 From: mptyl Date: Wed, 5 Aug 2026 04:49:19 +0200 Subject: [PATCH] fix: reject executable pi configuration --- backend/src/pi/list-models.ts | 10 ++ backend/src/pi/managed-config.ts | 87 ++++++++++++++++ backend/src/pi/management.ts | 10 +- backend/src/pi/provider-smoke.ts | 80 ++++++--------- backend/test/list-models.test.ts | 131 +++++++++++++++++++++++++ backend/test/pi-management.test.ts | 51 ++++++++++ backend/test/pi-provider-smoke.test.ts | 130 +++++++++++++++++++++++- docs/contracts/thothctl-pi.md | 12 +++ docs/general/pi-configuration.md | 24 ++++- 9 files changed, 479 insertions(+), 56 deletions(-) create mode 100644 backend/src/pi/managed-config.ts diff --git a/backend/src/pi/list-models.ts b/backend/src/pi/list-models.ts index 7d68d16b..6616221f 100644 --- a/backend/src/pi/list-models.ts +++ b/backend/src/pi/list-models.ts @@ -6,6 +6,10 @@ import { loadPiEnabledModels, type PiEnabledModelsResult, } from "./enabled-models.js"; +import { + readConfiguredPiAgentFile, + validateDeclarativePiConfig, +} from "./managed-config.js"; export interface PiModel { provider: string; @@ -23,6 +27,7 @@ interface Opts { ttlMs?: number; nowMs?: () => number; loadEnabledModels?: () => PiEnabledModelsResult; + readModelsStore?: () => string | undefined; warn?: (message: string) => void; } @@ -39,6 +44,11 @@ export function createPiModelLister(cfg: AppConfig, opts: Opts = {}): () => Prom let cache: { at: number; models: PiModel[] } | null = null; return async function listModels(): Promise { + const managedModels = opts.readModelsStore + ? opts.readModelsStore() + : readConfiguredPiAgentFile("models.json", true); + if (managedModels !== undefined) validateDeclarativePiConfig(managedModels); + if (cache && now() - cache.at < ttlMs) return cache.models; const enabled = (opts.loadEnabledModels diff --git a/backend/src/pi/managed-config.ts b/backend/src/pi/managed-config.ts new file mode 100644 index 00000000..2d1cbfde --- /dev/null +++ b/backend/src/pi/managed-config.ts @@ -0,0 +1,87 @@ +import { + closeSync, constants, fstatSync, lstatSync, openSync, readFileSync, +} from "node:fs"; +import { homedir } from "node:os"; +import { join } from "node:path"; + +const MAX_AGENT_CONFIG_BYTES = 1024 * 1024; + +export const PI_MANAGED_CONFIG_ERROR_CODE = "PI_MANAGED_CONFIG_INVALID"; +export const PI_MANAGED_CONFIG_ERROR_MESSAGE = "Pi provider/model configuration is invalid"; + +export class PiManagedConfigError extends Error { + readonly code = PI_MANAGED_CONFIG_ERROR_CODE; + + constructor() { + super(PI_MANAGED_CONFIG_ERROR_MESSAGE); + } +} + +export function isPiManagedConfigError(error: unknown): boolean { + return Boolean( + error && typeof error === "object" + && (error as { code?: unknown }).code === PI_MANAGED_CONFIG_ERROR_CODE, + ); +} + +export function parsePiConfigJson(raw: string): unknown { + try { + return JSON.parse(raw); + } catch { + throw new PiManagedConfigError(); + } +} + +/** Reject every Pi shell-backed configuration value, including unknown future nested fields. */ +export function assertDeclarativePiConfig(value: unknown): void { + const pending: unknown[] = [value]; + while (pending.length > 0) { + const current = pending.pop(); + if (typeof current === "string") { + if (current.startsWith("!")) throw new PiManagedConfigError(); + continue; + } + if (Array.isArray(current)) { + for (const nested of current) pending.push(nested); + continue; + } + if (current && typeof current === "object") { + for (const nested of Object.values(current as Record)) pending.push(nested); + } + } +} + +export function validateDeclarativePiConfig(raw: string): void { + assertDeclarativePiConfig(parsePiConfigJson(raw)); +} + +export function readConfiguredPiAgentFile(name: "auth.json"): string; +export function readConfiguredPiAgentFile(name: "models.json", optional: true): string | undefined; +export function readConfiguredPiAgentFile( + name: "auth.json" | "models.json", + optional = false, +): string | undefined { + const configuredAgentDir = process.env.PI_CODING_AGENT_DIR ?? join(homedir(), ".pi", "agent"); + const path = join(configuredAgentDir, name); + let fd: number | undefined; + try { + const before = lstatSync(path); + if (!before.isFile() || before.isSymbolicLink() || before.size > MAX_AGENT_CONFIG_BYTES) { + throw new PiManagedConfigError(); + } + fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW); + const opened = fstatSync(fd); + if (!opened.isFile() || opened.size > MAX_AGENT_CONFIG_BYTES + || before.dev !== opened.dev || before.ino !== opened.ino) { + throw new PiManagedConfigError(); + } + return readFileSync(fd, "utf8"); + } catch (error) { + if (optional && (error as NodeJS.ErrnoException)?.code === "ENOENT") return undefined; + throw new PiManagedConfigError(); + } finally { + if (fd !== undefined) { + try { closeSync(fd); } catch { /* preserve the stable validation outcome */ } + } + } +} diff --git a/backend/src/pi/management.ts b/backend/src/pi/management.ts index 38030b02..86bc8e35 100644 --- a/backend/src/pi/management.ts +++ b/backend/src/pi/management.ts @@ -7,6 +7,10 @@ import { type Settings, } from "../settings/settings-store.js"; import type { PiModel } from "./list-models.js"; +import { + PI_MANAGED_CONFIG_ERROR_MESSAGE, + isPiManagedConfigError, +} from "./managed-config.js"; import { createPiProviderSmoke, type PiProviderSmoke } from "./provider-smoke.js"; const execFile = promisify(nodeExecFile); @@ -105,7 +109,10 @@ export function createPiManagement(config: AppConfig, deps: PiManagementDeps): P let listed: PiModel[]; try { listed = await deps.listModels(); - } catch { + } catch (error) { + if (isPiManagedConfigError(error)) { + throw new PiManagementError("pi_management_unavailable", PI_MANAGED_CONFIG_ERROR_MESSAGE); + } throw new PiManagementError("pi_management_unavailable", "Pi model choices are unavailable"); } const models: Array<{ provider: string; id: string }> = []; @@ -287,6 +294,7 @@ function isTimeout(error: unknown): boolean { } function stableMessage(error: unknown, fallback: string): string { + if (isPiManagedConfigError(error)) return PI_MANAGED_CONFIG_ERROR_MESSAGE; return error instanceof PiManagementError ? error.message : fallback; } diff --git a/backend/src/pi/provider-smoke.ts b/backend/src/pi/provider-smoke.ts index 57c3c8d8..976b0e3c 100644 --- a/backend/src/pi/provider-smoke.ts +++ b/backend/src/pi/provider-smoke.ts @@ -1,9 +1,6 @@ import { spawn as nodeSpawn, type ChildProcessWithoutNullStreams } from "node:child_process"; -import { - closeSync, constants, fstatSync, lstatSync, mkdirSync, mkdtempSync, openSync, - readFileSync, rmSync, writeFileSync, -} from "node:fs"; -import { homedir, tmpdir } from "node:os"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import type { AppConfig } from "../config.js"; import { secretValue } from "../config/secret-bundle.js"; @@ -12,9 +9,15 @@ import { RpcClient } from "../rpc/rpc-client.js"; import { loadPiAuthProviders } from "./auth-providers.js"; import { buildPiChildEnv, canonicalPiProvider } from "./provider-credentials.js"; import type { PiReasoning } from "./management.js"; +import { + PiManagedConfigError, + isPiManagedConfigError, + parsePiConfigJson, + readConfiguredPiAgentFile, + validateDeclarativePiConfig, +} from "./managed-config.js"; const SMOKE_PROMPT = "Provider health check. Reply with exactly OK."; -const MAX_AGENT_CONFIG_BYTES = 1024 * 1024; const SMOKE_ARGS = [ "--mode", "rpc", "--no-session", @@ -82,14 +85,14 @@ export function createPiProviderSmoke( mkdirSync(isolatedAgentDir, { mode: 0o700 }); if (configuredAuthProviders.has(canonicalProvider)) { const authStore = selectedProviderAuthStore( - options.readAuthStore?.() ?? readConfiguredAgentFile("auth.json"), + options.readAuthStore?.() ?? readConfiguredPiAgentFile("auth.json"), canonicalProvider, ); - writeFileSync(join(isolatedAgentDir, "auth.json"), authStore, { mode: 0o600, flag: "wx" }); + writeDeclarativeAgentConfig(join(isolatedAgentDir, "auth.json"), authStore); } const configuredModels = options.readModelsStore ? options.readModelsStore() - : readConfiguredAgentFile("models.json", true); + : readConfiguredPiAgentFile("models.json", true); if (configuredModels !== undefined) { const modelsStore = selectedProviderModelsStore( configuredModels, @@ -97,9 +100,7 @@ export function createPiProviderSmoke( model, ); if (modelsStore !== undefined) { - writeFileSync(join(isolatedAgentDir, "models.json"), modelsStore, { - mode: 0o600, flag: "wx", - }); + writeDeclarativeAgentConfig(join(isolatedAgentDir, "models.json"), modelsStore); } } env.PI_CODING_AGENT_DIR = isolatedAgentDir; @@ -126,6 +127,7 @@ export function createPiProviderSmoke( ]); } catch (error) { if (isProviderTimeout(error)) throw providerTimeout(); + if (isPiManagedConfigError(error)) throw new PiManagedConfigError(); throw providerFailure(); } finally { if (timer) clearTimeout(timer); @@ -167,43 +169,19 @@ function messageUsesTool(message: any): boolean { && message.content.some((content: any) => content?.type === "toolCall")); } -function readConfiguredAgentFile(name: "auth.json"): string; -function readConfiguredAgentFile(name: "models.json", optional: true): string | undefined; -function readConfiguredAgentFile( - name: "auth.json" | "models.json", - optional = false, -): string | undefined { - const configuredAgentDir = process.env.PI_CODING_AGENT_DIR ?? join(homedir(), ".pi", "agent"); - const path = join(configuredAgentDir, name); - let fd: number | undefined; - try { - const before = lstatSync(path); - if (!before.isFile() || before.isSymbolicLink() || before.size > MAX_AGENT_CONFIG_BYTES) { - throw providerFailure(); - } - fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW); - const opened = fstatSync(fd); - if (!opened.isFile() || opened.size > MAX_AGENT_CONFIG_BYTES - || before.dev !== opened.dev || before.ino !== opened.ino) { - throw providerFailure(); - } - return readFileSync(fd, "utf8"); - } catch (error) { - if (optional && (error as NodeJS.ErrnoException)?.code === "ENOENT") return undefined; - throw providerFailure(); - } finally { - if (fd !== undefined) { - try { closeSync(fd); } catch { /* preserve the sanitized smoke outcome */ } - } - } +function writeDeclarativeAgentConfig(path: string, raw: string): void { + validateDeclarativePiConfig(raw); + writeFileSync(path, raw, { mode: 0o600, flag: "wx" }); } function selectedProviderAuthStore(raw: string, provider: string): string { - const parsed: unknown = JSON.parse(raw); - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) throw providerFailure(); + const parsed = parsePiConfigJson(raw); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new PiManagedConfigError(); + } const entry = Object.entries(parsed as Record) .find(([key]) => key.trim().toLowerCase() === provider); - if (!entry) throw providerFailure(); + if (!entry) throw new PiManagedConfigError(); return JSON.stringify({ [entry[0]]: entry[1] }); } @@ -212,18 +190,20 @@ const PROVIDER_CONFIG_FIELDS = [ ] as const; function selectedProviderModelsStore(raw: string, provider: string, model: string): string | undefined { - const parsed: unknown = JSON.parse(raw); - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) throw providerFailure(); + const parsed = parsePiConfigJson(raw); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new PiManagedConfigError(); + } const providers = (parsed as { providers?: unknown }).providers; if (!providers || typeof providers !== "object" || Array.isArray(providers)) { - throw providerFailure(); + throw new PiManagedConfigError(); } const entry = Object.entries(providers as Record) .find(([key]) => key.trim().toLowerCase() === provider); if (!entry) return undefined; const providerConfig = entry[1]; if (!providerConfig || typeof providerConfig !== "object" || Array.isArray(providerConfig)) { - throw providerFailure(); + throw new PiManagedConfigError(); } const source = providerConfig as Record; const selected: Record = {}; @@ -231,7 +211,7 @@ function selectedProviderModelsStore(raw: string, provider: string, model: strin if (Object.hasOwn(source, field)) selected[field] = source[field]; } if (Object.hasOwn(source, "models")) { - if (!Array.isArray(source.models)) throw providerFailure(); + if (!Array.isArray(source.models)) throw new PiManagedConfigError(); let selectedModel: unknown; for (const candidate of source.models) { if (candidate && typeof candidate === "object" && !Array.isArray(candidate) @@ -244,7 +224,7 @@ function selectedProviderModelsStore(raw: string, provider: string, model: strin if (Object.hasOwn(source, "modelOverrides")) { const overrides = source.modelOverrides; if (!overrides || typeof overrides !== "object" || Array.isArray(overrides)) { - throw providerFailure(); + throw new PiManagedConfigError(); } if (Object.hasOwn(overrides, model)) { selected.modelOverrides = { [model]: (overrides as Record)[model] }; diff --git a/backend/test/list-models.test.ts b/backend/test/list-models.test.ts index 84699025..3f615a0c 100644 --- a/backend/test/list-models.test.ts +++ b/backend/test/list-models.test.ts @@ -20,6 +20,9 @@ function enabled(...ids: string[]) { return () => ({ ids, warnings: [], source: "/test/settings.json" }); } +const noManagedModels = { readModelsStore: () => undefined }; +const MANAGED_CONFIG_ERROR = "Pi provider/model configuration is invalid"; + test("createPiModelLister returns mapped PiModel[] from get_available_models", async () => { const script = scriptWith([ { provider: "zai", id: "glm-5.2", name: "GLM 5.2", reasoning: true, extra: "ignored" }, @@ -27,6 +30,7 @@ test("createPiModelLister returns mapped PiModel[] from get_available_models", a ]); try { const lister = createPiModelLister(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + ...noManagedModels, loadEnabledModels: enabled("zai/glm-5.2", "anthropic/claude-opus-4-8"), spawnFn: () => spawn("node", [FAKE, script]) as any, }); @@ -45,6 +49,7 @@ test("createPiModelLister caches within ttl (spawns once for two calls)", async try { let spawns = 0; const lister = createPiModelLister(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + ...noManagedModels, loadEnabledModels: enabled("zai/glm-5.2"), spawnFn: () => { spawns++; return spawn("node", [FAKE, script]) as any; }, ttlMs: 10_000, @@ -69,6 +74,7 @@ test("production model-list spawn preserves PATH and passes the portable data ro PI_BIN: "/usr/local/bin/pi", THT_DATA_ROOT: "/data", }), { + ...noManagedModels, loadEnabledModels: enabled("test/unavailable"), spawnFn: (...args: any[]) => { calls.push(args); @@ -101,6 +107,7 @@ test("model-list spawn scrubs ambient provider credentials and generic secret me process.env.CLOUDFLARE_ACCOUNT_ID = "must-not-leak"; try { const lister = createPiModelLister(loadConfig({ PI_BIN: "/usr/local/bin/pi" }), { + ...noManagedModels, loadEnabledModels: enabled("test/unavailable"), spawnFn: (...args: any[]) => { calls.push(args); @@ -135,6 +142,7 @@ test("model listing does not require PI_PROVIDER or read the generic credential" PI_BIN: "/usr/local/bin/pi", THT_MODEL_API_KEY_FILE: "/missing-and-must-not-be-read", }), { + ...noManagedModels, loadEnabledModels: enabled("zai/glm-5.2"), spawnFn: (...args: any[]) => { calls.push(args); @@ -158,6 +166,7 @@ test("returns only enabled available models in enabledModels order", async () => ]); try { const lister = createPiModelLister(loadConfig({}), { + ...noManagedModels, loadEnabledModels: enabled( "zai/glm-5.2", "deepseek/deepseek-v4-flash", @@ -179,6 +188,7 @@ test("empty enabled model scope fails closed without spawning Pi", async () => { let spawns = 0; const warnings: string[] = []; const lister = createPiModelLister(loadConfig({}), { + ...noManagedModels, loadEnabledModels: () => ({ ids: [], warnings: ["scope invalid"] }), warn: (message) => warnings.push(message), spawnFn: () => { spawns += 1; throw new Error("must not spawn"); }, @@ -195,6 +205,7 @@ test("warns and returns empty when enabled identifiers are unavailable", async ( const warnings: string[] = []; try { const lister = createPiModelLister(loadConfig({}), { + ...noManagedModels, loadEnabledModels: enabled("zai/glm-5.2"), warn: (message) => warnings.push(message), spawnFn: () => spawn("node", [FAKE, script]) as any, @@ -205,3 +216,123 @@ test("warns and returns empty when enabled identifiers are unavailable", async ( rmSync(path.dirname(script), { recursive: true, force: true }); } }); + +const executableModelsConfigCases: Array<[string, unknown]> = [ + ["nested provider headers", { + providers: { + selected: { + headers: { Authorization: "!sensitive-header-command /private/header-path" }, + }, + }, + }], + ["provider apiKey", { + providers: { + selected: { apiKey: "!sensitive-api-key-command /private/key-path" }, + }, + }], + ["selected model objects", { + providers: { + selected: { + models: [{ id: "model", name: "!sensitive-model-command /private/model-path" }], + }, + }, + }], + ["selected model overrides", { + providers: { + selected: { + modelOverrides: { + model: { headers: { "X-Override": "!sensitive-override-command /private/override-path" } }, + }, + }, + }, + }], + ["nested arrays", { + providers: { + selected: { + compat: { nested: ["literal", { value: "!sensitive-array-command /private/array-path" }] }, + }, + }, + }], +]; + +// Catches Task 8 model discovery delegating raw managed models.json values to Pi. Pi 0.80.3 +// executes leading-! values at request time, so the complete managed store must be rejected before +// it can become an authoritative source of API choices. +test.each(executableModelsConfigCases)( + "managed models ingestion rejects executable strings in %s", + async (_name, modelsConfig) => { + let spawns = 0; + const lister = createPiModelLister(loadConfig({}), { + loadEnabledModels: enabled("selected/model"), + readModelsStore: () => JSON.stringify(modelsConfig), + spawnFn: () => { + spawns += 1; + throw new Error("unsafe model-list spawn"); + }, + }); + + let caught: unknown; + try { + await lister(); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toBe(MANAGED_CONFIG_ERROR); + expect(String(caught)).not.toMatch(/sensitive|private|command|path/i); + expect(spawns).toBe(0); + }, +); + +// Selection-time smoke filtering intentionally ignores unrelated providers, but the full +// installation-owned models.json is invalid at the model-choice ingestion boundary. +test("managed models ingestion rejects an executable string in an unrelated provider", async () => { + let spawns = 0; + const lister = createPiModelLister(loadConfig({}), { + loadEnabledModels: enabled("selected/model"), + readModelsStore: () => JSON.stringify({ + providers: { + selected: { models: [{ id: "model" }] }, + unrelated: { apiKey: "!sensitive-unrelated-command /private/unrelated-path" }, + }, + }), + spawnFn: () => { + spawns += 1; + throw new Error("unsafe model-list spawn"); + }, + }); + + await expect(lister()).rejects.toThrow(MANAGED_CONFIG_ERROR); + expect(spawns).toBe(0); +}); + +test("managed models ingestion revalidates the store before serving a cached choice", async () => { + const script = scriptWith([ + { provider: "selected", id: "model", name: "Selected model", reasoning: true }, + ]); + let managedModels = JSON.stringify({ + providers: { selected: { models: [{ id: "model" }] } }, + }); + let spawns = 0; + try { + const lister = createPiModelLister(loadConfig({}), { + loadEnabledModels: enabled("selected/model"), + readModelsStore: () => managedModels, + spawnFn: () => { + spawns += 1; + return spawn("node", [FAKE, script]) as any; + }, + ttlMs: 10_000, + }); + + await expect(lister()).resolves.toHaveLength(1); + managedModels = JSON.stringify({ + providers: { selected: { headers: { Authorization: "!new-unsafe-value" } } }, + }); + + await expect(lister()).rejects.toThrow(MANAGED_CONFIG_ERROR); + expect(spawns).toBe(1); + } finally { + rmSync(path.dirname(script), { recursive: true, force: true }); + } +}); diff --git a/backend/test/pi-management.test.ts b/backend/test/pi-management.test.ts index 563aed43..493e8ddb 100644 --- a/backend/test/pi-management.test.ts +++ b/backend/test/pi-management.test.ts @@ -73,6 +73,32 @@ test("options expose only closed provider, model, and reasoning choices", async }); }); +// Catches raw managed models.json validation details being collapsed into an ambiguous model-list +// failure or escaping through the Pi Management options API. +test("options report invalid managed model configuration with a stable sanitized error", async () => { + const service = createPiManagement(configFor(), { + execute: successfulExec([]), + listModels: async () => { + throw Object.assign( + new Error("!sensitive-command /private/models.json raw-secret"), + { code: "PI_MANAGED_CONFIG_INVALID" }, + ); + }, + }); + + let caught: unknown; + try { + await service.options(); + } catch (error) { + caught = error; + } + expect(caught).toMatchObject({ + code: "pi_management_unavailable", + message: "Pi provider/model configuration is invalid", + }); + expect(String(caught)).not.toMatch(/sensitive|private|models\.json|secret/i); +}); + // Catches configuration writes that accept whitespace, unknown choices, or extra free-form fields // before reaching the durable installation settings file. test("config rejects invalid free-form values before writing settings", async () => { @@ -186,6 +212,31 @@ test("smoke fails closed and sanitizes configured-provider authentication errors expect(JSON.stringify(result)).not.toMatch(/raw-expired-token|raw-provider-output/); }); +// Catches selected auth/models validation failures being downgraded to a generic provider error +// or exposing the rejected command, path, or secret through POST /pi-management/test. +test("smoke reports invalid managed provider configuration with a stable sanitized error", async () => { + const service = createPiManagement(configFor(), { + execute: successfulExec([]), + listModels: async () => supportedModels, + readSettings: () => ({ provider: "zai", model: "glm-5.2", thinking: "medium" }), + smokeProvider: async () => { + throw Object.assign( + new Error("!sensitive-command /private/models.json raw-secret"), + { code: "PI_MANAGED_CONFIG_INVALID" }, + ); + }, + now: () => new Date("2026-08-05T10:00:00.000Z"), + }); + + const result = await service.test(); + expect(result).toEqual({ + ready: false, + message: "Pi provider/model configuration is invalid", + checkedAt: "2026-08-05T10:00:00.000Z", + }); + expect(JSON.stringify(result)).not.toMatch(/sensitive|private|models\.json|secret/i); +}); + // Catches separate per-phase timeouts that allow a later provider turn to exceed the one // end-to-end Pi Management smoke budget. test("smoke applies one deadline across version and a hung provider turn", async () => { diff --git a/backend/test/pi-provider-smoke.test.ts b/backend/test/pi-provider-smoke.test.ts index 08260a8c..b96499cc 100644 --- a/backend/test/pi-provider-smoke.test.ts +++ b/backend/test/pi-provider-smoke.test.ts @@ -24,6 +24,26 @@ function rpcChild(onCommand: (command: any, emit: (message: unknown) => void) => return child; } +function successfulProviderChild() { + return 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({ + type: "message_end", + message: { role: "assistant", stopReason: "stop", content: "must-not-be-returned" }, + }); + emit({ + type: "agent_end", + messages: [{ role: "assistant", stopReason: "stop", content: "must-not-be-returned" }], + }); + } + }); +} + +const MANAGED_CONFIG_ERROR = "Pi provider/model configuration is invalid"; + // Catches an isolated smoke agent that copies auth.json but drops the selected custom // provider/model from models.json, causing set_model to fail before the real request. test("provider smoke reaches the selected custom provider from an isolated models.json", async () => { @@ -61,23 +81,30 @@ test("provider smoke reaches the selected custom provider from an isolated model providers: { "custom-openai": { baseUrl: "https://selected.invalid/v1", + apiKey: "$CUSTOM_OPENAI_API_KEY", api: "openai-completions", + headers: { "X-Literal-Bang": "$!literal-value" }, models: [{ id: "selected-model", name: "Selected model", reasoning: true }], }, }, }); + expect(JSON.parse(readFileSync(`${isolatedAgentDir}/auth.json`, "utf8"))).toEqual({ + "custom-openai": { type: "api_key", key: "${CUSTOM_OPENAI_API_KEY}" }, + }); return child; }, authProviders: () => new Set(["custom-openai"]), readAuthStore: () => JSON.stringify({ - "custom-openai": { type: "api_key", key: "test-only" }, - unrelated: { type: "api_key", key: "must-not-enter-isolated-context" }, + "custom-openai": { type: "api_key", key: "${CUSTOM_OPENAI_API_KEY}" }, + unrelated: { type: "api_key", key: "!must-not-run-or-enter-isolated-context" }, }), readModelsStore: () => JSON.stringify({ providers: { "custom-openai": { baseUrl: "https://selected.invalid/v1", + apiKey: "$CUSTOM_OPENAI_API_KEY", api: "openai-completions", + headers: { "X-Literal-Bang": "$!literal-value" }, models: [ { id: "selected-model", name: "Selected model", reasoning: true }, { id: "unrelated-model", name: "Must not enter isolated context" }, @@ -101,6 +128,105 @@ test("provider smoke reaches the selected custom provider from an isolated model expect(isolatedAgentDir && existsSync(dirname(isolatedAgentDir))).toBe(false); }); +const selectedExecutableConfigCases: Array<{ + name: string; + selectedAuth?: unknown; + selectedProvider: Record; +}> = [ + { + name: "selected auth credential", + selectedAuth: { + type: "api_key", + key: "!sensitive-credential-command /private/credential-path", + }, + selectedProvider: {}, + }, + { + name: "selected provider apiKey", + selectedProvider: { + apiKey: "!sensitive-api-key-command /private/key-path", + }, + }, + { + name: "nested selected-provider headers", + selectedProvider: { + headers: { Authorization: "!sensitive-header-command /private/header-path" }, + }, + }, + { + name: "selected model object", + selectedProvider: { + models: [{ + id: "selected-model", + name: "!sensitive-model-command /private/model-path", + }], + }, + }, + { + name: "selected model override", + selectedProvider: { + modelOverrides: { + "selected-model": { + headers: { "X-Override": "!sensitive-override-command /private/override-path" }, + }, + }, + }, + }, + { + name: "array nested in selected provider configuration", + selectedProvider: { + compat: { + nested: ["literal", { value: "!sensitive-array-command /private/array-path" }], + }, + }, + }, +]; + +// Catches a defense that validates only known top-level fields or waits until after the isolated +// Pi process starts. Every selected value crossing into auth.json/models.json must be declarative. +test.each(selectedExecutableConfigCases)( + "provider smoke rejects executable config in $name before isolated Pi spawn", + async ({ selectedAuth, selectedProvider }) => { + const child = successfulProviderChild(); + const spawnFn = vi.fn(() => child); + const smoke = createPiProviderSmoke(loadConfig({}), { + spawnFn, + authProviders: () => new Set(["custom-openai"]), + readAuthStore: () => JSON.stringify({ + "custom-openai": selectedAuth ?? { type: "api_key", key: "test-only" }, + unrelated: { type: "api_key", key: "!must-not-contaminate-selected-provider" }, + }), + readModelsStore: () => JSON.stringify({ + providers: { + "custom-openai": { + baseUrl: "https://selected.invalid/v1", + api: "openai-completions", + models: [{ id: "selected-model", name: "Selected model" }], + ...selectedProvider, + }, + unrelated: { + apiKey: "!must-not-contaminate-selected-provider", + models: [{ id: "unrelated-model" }], + }, + }, + }), + }); + + let caught: unknown; + try { + await smoke({ + provider: "custom-openai", model: "selected-model", reasoning: "medium", timeoutMs: 750, + }); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toBe(MANAGED_CONFIG_ERROR); + expect(String(caught)).not.toMatch(/sensitive|private|command|path/i); + expect(spawnFn).not.toHaveBeenCalled(); + }, +); + // 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 () => { diff --git a/docs/contracts/thothctl-pi.md b/docs/contracts/thothctl-pi.md index a5306635..decd0f60 100644 --- a/docs/contracts/thothctl-pi.md +++ b/docs/contracts/thothctl-pi.md @@ -36,6 +36,18 @@ file and verifies the absent/default state. Empty prior files are supported. The the actual host path from `PI_AUTH_FILE`; credentials remain in that protected host file and must never be passed as flags. +Installation-managed Pi provider/model configuration is declarative only. Any JSON value beginning +with `!` is rejected recursively in the complete `models.json` before it can supply management +choices, and the exact selected provider/model and credential payload is checked again before the +isolated smoke files are written. The API returns only the fixed +`Pi provider/model configuration is invalid` message; rejected commands, paths, and secrets are +never included. Use `$NAME`/`${NAME}` environment references in `models.json`, or omit `apiKey` and +provide the selected credential through the protected `PI_AUTH_FILE`, `THT_MODEL_API_KEY_FILE`, or +`THT_SECRETS_FILE` contract. A literal leading exclamation mark uses Pi's `$!` escape. Direct +secret-file references are not a `models.json` feature: ThothII converts its managed key source to +the provider-native child environment, while `PI_AUTH_FILE` is mounted as Pi's protected credential +store. + ## Supported Compose entry points and current image Use `thothctl start`, `stop`, `status`, `logs`, and `doctor` for ordinary installation lifecycle diff --git a/docs/general/pi-configuration.md b/docs/general/pi-configuration.md index ecc2606b..d8db2eae 100644 --- a/docs/general/pi-configuration.md +++ b/docs/general/pi-configuration.md @@ -4,9 +4,11 @@ Pi (il coding agent che orchestra il workflow NL→SQL) può risolvere un `provi ## Credenziali nel backend container -In produzione configurare una sola sorgente generica, `THT_MODEL_API_KEY_FILE`, come secret file -assoluto e non il valore della chiave. `PiProcessManager` rilegge e valida il file per ogni processo, -normalizza il provider selezionato e passa al solo child Pi la variabile nativa appropriata +In produzione configurare una sola sorgente generica: il bundle `THT_SECRETS_FILE` con la voce +`THT_MODEL_API_KEY` (predefinito della distribuzione unificata), oppure +`THT_MODEL_API_KEY_FILE` come secret file assoluto. Non mettere il valore della chiave in `.env`. +`PiProcessManager` rilegge e valida la sorgente per ogni processo, normalizza il provider +selezionato e passa al solo child Pi la variabile nativa appropriata (`ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`, `ZAI_API_KEY`, ecc.). Il percorso generico, le chiavi di provider non selezionati e il vecchio `PI_PROVIDER_API_KEY` vengono rimossi dall'ambiente del child. Provider locali come `ollama`, `lmstudio` e `aritmolab` continuano senza chiave; un provider @@ -37,6 +39,22 @@ Pi viene distribuito con un elenco di modelli già noti (`models.generated.js` d Per un endpoint **OpenAI-compatible** che non è tra i built-in — ma che non richiede nessuna logica di trasporto speciale — basta *dichiararlo*: baseUrl, apiKey, lista modelli. Questo file esiste **solo a livello utente**: non c'è un equivalente project-level (un `./.pi/models.json` non viene letto). +Nelle installazioni gestite da ThothII il file deve essere interamente dichiarativo. ThothII +rifiuta ricorsivamente qualsiasi valore JSON che inizi con `!`, anche dentro `headers`, `models`, +`modelOverrides`, `compat`, array o campi non ancora conosciuti. Pi 0.80.3 tratterebbe quel prefisso +come un comando shell al momento della richiesta; questa forma non è ammessa né dall'elenco gestito +dei modelli né dallo smoke isolato. L'errore restituito è fisso e non include comando, percorso o +secret. + +Per i secret usare un riferimento ambiente come `"$ZAI_API_KEY"` o `"${ZAI_API_KEY}"`. Il backend +può popolare la variabile nativa del solo provider selezionato leggendo +`THT_MODEL_API_KEY_FILE`, oppure dal bundle `THT_SECRETS_FILE` (`THT_MODEL_API_KEY`); in alternativa +le credenziali possono arrivare dal file protetto montato con `PI_AUTH_FILE`, omettendo `apiKey` da +`models.json`. Non esiste una sintassi di riferimento diretto a un secret file dentro +`models.json`: con le sorgenti `THT_MODEL_*` il file viene letto da ThothII e trasformato nella +variabile ambiente del child Pi; `PI_AUTH_FILE` viene invece montato come archivio credenziali Pi +protetto. Per un punto esclamativo letterale iniziale, la sintassi Pi dichiarativa è `$!`, non `!`. + Esempio reale in uso su questa macchina — GLM (provider `zai`): ```json