diff --git a/extensions/shared/child-session.ts b/extensions/shared/child-session.ts index 61a9d3ce..1b0b0cb2 100644 --- a/extensions/shared/child-session.ts +++ b/extensions/shared/child-session.ts @@ -11,6 +11,8 @@ import { type ResolvedPaths, type SessionShutdownEvent, SettingsManager, + type SourceInfo, + type ToolInfo, } from "@earendil-works/pi-coding-agent"; import { OPENPI_OWNER_SOURCE_PATHS, @@ -167,6 +169,47 @@ function createPiIntercomPackageMatcher(options: { }; } +/** Use Pi's scoped package identity for both resources and inherited tools. + * A local single-file source must be resolved as a file, not as its containing + * baseDir: only the file lookup walks up to the owning package manifest. + */ +function createBlockedChildPackagePolicy(options: { + cwd: string; + agentDir: string; +}) { + const packageManager = new DefaultPackageManager({ + ...options, + settingsManager: SettingsManager.inMemory(), + }); + const matches = createPiIntercomPackageMatcher(options); + return (sourceInfo: Omit) => { + if ( + !sourceInfo || + !sourceInfo.source || + !["package", "top-level"].includes(sourceInfo.origin) || + !["user", "project", "temporary"].includes(sourceInfo.scope) + ) { + throw new Error( + "Cannot verify child package identity: missing source metadata", + ); + } + if (sourceInfo.origin !== "package" || sourceInfo.scope === "temporary") { + return false; + } + const installedPath = + packageManager.getInstalledPath(sourceInfo.source, sourceInfo.scope) ?? + sourceInfo.baseDir; + // A canonical blocked source can be denied even if it has disappeared. + if (matches(sourceInfo.source, installedPath)) return true; + if (!installedPath) { + throw new Error( + `Cannot verify child package identity from ${sourceInfo.source}`, + ); + } + return false; + }; +} + function packageSourceValue(source: PackageSource) { return typeof source === "string" ? source : source.source; } @@ -351,13 +394,16 @@ function blockedPackageSources( resolvedPaths: ResolvedPaths, options: { cwd: string; agentDir: string }, ) { - const isPiIntercomPackage = createPiIntercomPackageMatcher(options); + const isBlocked = createBlockedChildPackagePolicy(options); const blocked = { user: new Set(), project: new Set(), }; + // Configured packages can be absent in offline mode. Match their canonical + // identity without demanding loaded-tool provenance from an unloaded package. + const matches = createPiIntercomPackageMatcher(options); for (const configured of packageManager.listConfiguredPackages()) { - if (isPiIntercomPackage(configured.source, configured.installedPath)) { + if (matches(configured.source, configured.installedPath)) { blocked[configured.scope].add(configured.source); } } @@ -370,11 +416,7 @@ function blockedPackageSources( ]; for (const resource of resources) { const { metadata } = resource; - if ( - metadata.origin !== "package" || - metadata.scope === "temporary" || - !isPiIntercomPackage(metadata.source, metadata.baseDir) - ) { + if (metadata.scope === "temporary" || !isBlocked(metadata)) { continue; } blocked[metadata.scope].add(metadata.source); @@ -552,15 +594,36 @@ export function effectiveChildToolAllowlist(tools?: readonly string[]) { /** Project the parent's active surface into a child; a role can only narrow it. * Active tools are a visibility choice, not a filesystem/network sandbox. - * Inactive tools are not implicitly activated by delegation. + * Inactive tools are not implicitly activated by delegation. Pi supplies the + * provenance for every inherited tool; unverifiable identities stop startup. */ export function inheritedChildToolAllowlist( parentTools: readonly string[], - roleTools?: readonly string[], + roleTools: readonly string[] | undefined, + options: { + availableTools: readonly Pick[]; + cwd: string; + }, ) { const allowed = roleTools === undefined ? undefined : new Set(roleTools); + const available = new Map( + options.availableTools.map((tool) => [tool.name, tool]), + ); + const isBlocked = createBlockedChildPackagePolicy({ + cwd: options.cwd, + agentDir: getAgentDir(), + }); return effectiveChildToolAllowlist([...new Set(parentTools)])!.filter( - (name) => allowed === undefined || allowed.has(name), + (name) => { + if (allowed && !allowed.has(name)) return false; + const tool = available.get(name); + if (!tool) { + throw new Error( + `Cannot verify child tool provenance for ${JSON.stringify(name)}`, + ); + } + return !isBlocked(tool.sourceInfo); + }, ); } diff --git a/extensions/subagents/index.ts b/extensions/subagents/index.ts index f753c77b..e5331422 100644 --- a/extensions/subagents/index.ts +++ b/extensions/subagents/index.ts @@ -940,6 +940,10 @@ export default function ( const childTools = inheritedChildToolAllowlist( pi.getActiveTools(), requestedChildTools, + { + availableTools: pi.getAllTools(), + cwd: ctx.cwd, + }, ); // Read at spawn time so `/openpi-setup` changes affect the next child // without reloading this extension. Undefined preserves parent-model diff --git a/extensions/workflows/index.ts b/extensions/workflows/index.ts index ff0f5a9c..3d96e27e 100644 --- a/extensions/workflows/index.ts +++ b/extensions/workflows/index.ts @@ -1647,6 +1647,10 @@ export default function workflows( const childTools = inheritedChildToolAllowlist( pi.getActiveTools(), agentType?.tools, + { + availableTools: pi.getAllTools(), + cwd: ctx.cwd, + }, ); if ( opts.working_dir !== undefined && diff --git a/tests/extensions/shared/child-session.test.ts b/tests/extensions/shared/child-session.test.ts index ff49e6c6..34db7291 100644 --- a/tests/extensions/shared/child-session.test.ts +++ b/tests/extensions/shared/child-session.test.ts @@ -16,6 +16,7 @@ import test from "node:test"; import { fileURLToPath, pathToFileURL } from "node:url"; import { createAgentSession, + createSyntheticSourceInfo, DefaultPackageManager, DefaultResourceLoader, defineTool, @@ -888,6 +889,76 @@ test("child resources exclude pi-intercom npm, Git, and local packages without m for (const marker of executionMarkers) { assert.equal(await readFile(marker, "utf8"), "executed"); } + + // Exercise inheritance with Pi's actual parent metadata, including the + // nested single-file package and an unrelated tool also named intercom. + const parentLoader = new DefaultResourceLoader({ + cwd, + agentDir, + settingsManager: SettingsManager.create(cwd, agentDir, { + projectTrusted: true, + }), + }); + await parentLoader.reload(); + const { session: parent } = await createAgentSession({ + cwd, + agentDir, + resourceLoader: parentLoader, + sessionManager: SessionManager.inMemory(cwd), + }); + const previousAgentDir = process.env.PI_CODING_AGENT_DIR; + process.env.PI_CODING_AGENT_DIR = agentDir; + try { + await parent.bindExtensions({ mode: "print" }); + const parentTools = parent.getActiveToolNames(); + for (const name of packageToolNames) + assert.ok(parentTools.includes(name)); + const options = { cwd, availableTools: parent.getAllTools() }; + const inherited = inheritedChildToolAllowlist( + parentTools, + undefined, + options, + ); + for (const name of packageToolNames) + assert.equal(inherited.includes(name), false); + assert.ok( + inherited.includes("intercom"), + "unrelated same-name tool survives", + ); + assert.ok(inherited.includes("ordinary_manifestless")); + assert.ok(inherited.includes("project_intercom_path_fixture")); + assert.deepEqual( + inheritedChildToolAllowlist( + parentTools, + ["read", "intercom_single_file"], + options, + ), + ["read"], + ); + const { session: child } = await createAgentSession({ + cwd, + agentDir, + resourceLoader: trusted.loader, + settingsManager: trusted.settingsManager, + sessionManager: SessionManager.inMemory(cwd), + ...childToolPolicy(inherited), + }); + try { + await bindChildSessionExtensions(child, inherited); + assert.deepEqual( + child.getActiveToolNames().sort(), + [...inherited].sort(), + ); + assert.deepEqual(parent.getActiveToolNames(), parentTools); + } finally { + await shutdownAndDisposeChildSession(child); + } + } finally { + await shutdownAndDisposeChildSession(parent); + if (previousAgentDir === undefined) + delete process.env.PI_CODING_AGENT_DIR; + else process.env.PI_CODING_AGENT_DIR = previousAgentDir; + } }); }); @@ -1188,6 +1259,25 @@ test("unverifiable local package identities fail closed before factory execution /Cannot verify child package identity/, ); await assert.rejects(readFile(executionMarker)); + assert.throws( + () => + inheritedChildToolAllowlist(["fixture"], undefined, { + cwd, + availableTools: [ + { + name: "fixture", + sourceInfo: { + path: path.join(packageDir, "extensions", "index.ts"), + source: packageDir, + baseDir: packageDir, + scope: "user", + origin: "package", + }, + }, + ], + }), + /Cannot verify child package identity/, + ); }); } }); @@ -1558,20 +1648,39 @@ test("git-info exclusion: ENOENT degrades, other errors fail closed", async () = test("child delegation inherits active tools and custom restrictions only narrow", () => { const parent = ["read", "bash", "web_search", "workflow", "subagent_spawn"]; - assert.deepEqual(inheritedChildToolAllowlist(parent), [ + const options = { + cwd: process.cwd(), + availableTools: parent.map((name) => ({ + name, + sourceInfo: createSyntheticSourceInfo(``, { source: "sdk" }), + })), + }; + assert.deepEqual(inheritedChildToolAllowlist(parent, undefined, options), [ "read", "bash", "web_search", ]); assert.deepEqual( - inheritedChildToolAllowlist(parent, [ - "read", - "rg", - "web_search", - "workflow", - ]), + inheritedChildToolAllowlist( + parent, + ["read", "rg", "web_search", "workflow"], + options, + ), ["read", "web_search"], ); - assert.deepEqual(inheritedChildToolAllowlist(parent, []), []); - assert.deepEqual(inheritedChildToolAllowlist([], ["bash"]), []); + assert.deepEqual(inheritedChildToolAllowlist(parent, [], options), []); + assert.deepEqual(inheritedChildToolAllowlist([], ["bash"], options), []); + assert.throws( + () => inheritedChildToolAllowlist(["missing"], undefined, options), + /Cannot verify child tool provenance/, + ); + assert.throws( + () => + inheritedChildToolAllowlist(["read"], undefined, { + cwd: process.cwd(), + // Verify the runtime boundary even if a broken caller violates Pi's type. + availableTools: [{ name: "read", sourceInfo: undefined! }], + }), + /Cannot verify child package identity/, + ); }); diff --git a/tests/extensions/subagents/index.test.ts b/tests/extensions/subagents/index.test.ts index d4620e78..57764e08 100644 --- a/tests/extensions/subagents/index.test.ts +++ b/tests/extensions/subagents/index.test.ts @@ -18,6 +18,18 @@ import subagents, { } from "../../../extensions/subagents/index.ts"; import { projectResult } from "../../../extensions/subagents/src/result-artifact.ts"; +import { + type AgentSession, + createAgentSession, + DefaultResourceLoader, + SessionManager, + SettingsManager, +} from "@earendil-works/pi-coding-agent"; +import { shutdownAndDisposeChildSession } from "../../../extensions/shared/child-session.ts"; +import { makePiBackend } from "../../../extensions/subagents/src/backends/pi.ts"; +import { __setSubagentTestBackends } from "../../../extensions/subagents/src/runtime.ts"; +import { createPiAgentSessionHarness } from "../../support/pi-agent-session-harness.ts"; + initTheme("dark", false); const emptySessionManager = { getBranch: () => [] }; @@ -1010,3 +1022,146 @@ test("session_start re-registers agent types for its cwd and live trust decision ); }); }); + +test("ordinary and typed Direct spawns inherit real single-file package provenance", async () => { + await withTempDir(async (cwd) => { + const agentDir = process.env.PI_CODING_AGENT_DIR!; + const packageDir = path.join(agentDir, "local-package-checkout"); + const source = path.join(packageDir, "extensions", "intercom.ts"); + await mkdir(path.dirname(source), { recursive: true }); + await writeFile( + path.join(packageDir, "package.json"), + JSON.stringify({ name: "pi-intercom", version: "0.0.0" }), + ); + await writeFile( + source, + `export default function (pi) { + pi.registerTool({ name: "intercom", label: "fixture", description: "fixture", + parameters: { type: "object", properties: {} }, + async execute() { return { content: [{ type: "text", text: "ok" }] }; } + }); + }`, + ); + await writeFile( + path.join(agentDir, "settings.json"), + JSON.stringify({ packages: [source] }), + ); + const settingsManager = SettingsManager.create(cwd, agentDir); + const loader = new DefaultResourceLoader({ + cwd, + agentDir, + settingsManager, + }); + await loader.reload(); + const { session: parent } = await createAgentSession({ + cwd, + agentDir, + resourceLoader: loader, + settingsManager, + sessionManager: SessionManager.inMemory(cwd), + }); + const model = { + provider: "fixture", + id: "model", + name: "fixture", + api: "openai-completions", + baseUrl: "http://127.0.0.1:1", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 8192, + maxTokens: 100, + } as NonNullable; + let prompts = 0; + __setSubagentTestBackends([ + makePiBackend({ + sessionFactory: async (options) => { + assert.deepEqual(options?.tools, ["read"]); + assert.equal( + options?.resourceLoader + ?.getExtensions() + .extensions.some((extension) => extension.tools.has("intercom")), + false, + ); + const harness = createPiAgentSessionHarness({ + model, + activeTools: ["read"], + prompt: async (_text, child) => { + prompts++; + child.emitAssistant("intercom boundary verified"); + }, + }); + return { session: harness.session }; + }, + }), + ]); + const tools = new Map< + string, + { execute: (...args: unknown[]) => Promise } + >(); + const hooks = new Map unknown>(); + const pi = { + events: { on() {}, emit() {} }, + on(name: string, handler: (...args: unknown[]) => unknown) { + hooks.set(name, handler); + }, + registerTool(tool: { + name: string; + execute: (...args: unknown[]) => Promise; + }) { + tools.set(tool.name, tool); + }, + registerCommand() {}, + registerMessageRenderer() {}, + registerEntryRenderer() {}, + appendEntry() {}, + sendMessage() {}, + setActiveTools() {}, + getActiveTools: () => parent.getActiveToolNames(), + getAllTools: () => parent.getAllTools(), + getThinkingLevel: () => "off", + } as unknown as ExtensionAPI; + const ctx = { + cwd, + hasUI: false, + isProjectTrusted: () => false, + model, + getContextUsage: () => undefined, + modelRegistry: { find: () => model, getAll: () => [model] }, + } as unknown as ExtensionContext; + try { + await parent.bindExtensions({ mode: "print" }); + parent.setActiveToolsByName(["read", "intercom"]); + subagents(pi); + for (const agent_type of [undefined, "advisor"]) { + const spawned = (await tools.get("subagent_spawn")!.execute( + "spawn", + { + prompt: "inspect", + name: agent_type ?? "ordinary", + ...(agent_type ? { agent_type } : {}), + }, + undefined, + undefined, + ctx, + )) as { details: { id: string } }; + const waited = await tools + .get("subagent_wait")! + .execute( + "wait", + { ids: [spawned.details.id] }, + undefined, + undefined, + ctx, + ); + assert.match(JSON.stringify(waited), /intercom boundary verified/); + assert.deepEqual(parent.getActiveToolNames(), ["read", "intercom"]); + } + assert.equal(prompts, 2); + } finally { + await hooks.get("session_shutdown")?.({}, ctx); + __setSubagentTestBackends(undefined); + await shutdownAndDisposeChildSession(parent); + } + }); +}); diff --git a/tests/extensions/subagents/prompt.test.ts b/tests/extensions/subagents/prompt.test.ts index c3f51151..1c388a50 100644 --- a/tests/extensions/subagents/prompt.test.ts +++ b/tests/extensions/subagents/prompt.test.ts @@ -1,3 +1,4 @@ +import { createSyntheticSourceInfo } from "@earendil-works/pi-coding-agent"; /** Model-facing strings that carry a behavioral contract, not just wording. */ import assert from "node:assert/strict"; @@ -128,7 +129,15 @@ test("investigator roles describe the same inherited capability that spawn repor assert.match(description, /\[inherited-tools\]/); assert.doesNotMatch(description, /read-only/i, name); - const inherited = inheritedChildToolAllowlist(parentTools, role.tools); + const inherited = inheritedChildToolAllowlist(parentTools, role.tools, { + cwd: process.cwd(), + availableTools: parentTools.map((name) => ({ + name, + sourceInfo: createSyntheticSourceInfo(``, { + source: "sdk", + }), + })), + }); assert.deepEqual(inherited, expectedTools); assert.deepEqual(effectiveChildToolAllowlist(parentTools), expectedTools); const result = buildSubagentSpawnResult({ diff --git a/tests/extensions/subagents/startup-worktree.test.ts b/tests/extensions/subagents/startup-worktree.test.ts index cf08fe03..5d32b714 100644 --- a/tests/extensions/subagents/startup-worktree.test.ts +++ b/tests/extensions/subagents/startup-worktree.test.ts @@ -17,6 +17,7 @@ import type { ExtensionContext, AgentSession, } from "@earendil-works/pi-coding-agent"; +import { createSyntheticSourceInfo } from "@earendil-works/pi-coding-agent"; import subagents from "../../../extensions/subagents/index.ts"; import { SpawnError } from "../../../extensions/subagents/src/domain.ts"; import { makePiBackend } from "../../../extensions/subagents/src/backends/pi.ts"; @@ -114,6 +115,14 @@ for (const interrupted of [true, false]) { registerMessageRenderer() {}, registerEntryRenderer() {}, getActiveTools: () => ["read"], + getAllTools: () => [ + { + name: "read", + sourceInfo: createSyntheticSourceInfo("", { + source: "builtin", + }), + }, + ], setActiveTools() {}, getThinkingLevel: () => "off", } as unknown as ExtensionAPI; diff --git a/tests/extensions/workflows/execute.e2e.test.ts b/tests/extensions/workflows/execute.e2e.test.ts index c0aad385..62e77346 100644 --- a/tests/extensions/workflows/execute.e2e.test.ts +++ b/tests/extensions/workflows/execute.e2e.test.ts @@ -27,6 +27,14 @@ import type { ExtensionContext, ToolDefinition, } from "@earendil-works/pi-coding-agent"; +import { + createAgentSession, + createSyntheticSourceInfo, + DefaultResourceLoader, + SessionManager, + SettingsManager, +} from "@earendil-works/pi-coding-agent"; +import { shutdownAndDisposeChildSession } from "../../../extensions/shared/child-session.ts"; import { SPINNER_INTERVAL_MS } from "../../../extensions/shared/spinner.ts"; import { reclaimWorktree } from "../../../extensions/shared/worktree.ts"; import { persistWorkflowJson } from "../../../extensions/workflows/artifacts.ts"; @@ -119,6 +127,11 @@ const pi = { }, getThinkingLevel: () => "off", getActiveTools: () => [...activeTools], + getAllTools: () => + activeTools.map((name) => ({ + name, + sourceInfo: createSyntheticSourceInfo(``, { source: "sdk" }), + })), setActiveTools(names: string[]) { activeTools = [...names]; }, @@ -1873,3 +1886,91 @@ for (const dirty of [false, true]) __setWorkflowTestLifecycleHooks(undefined); } }); + +test("Workflow agent inherits real parent package provenance and excludes intercom", async () => { + const packageDir = join(agentDir, "local-intercom-fixture"); + mkdirSync(packageDir, { recursive: true }); + writeFileSync( + join(packageDir, "package.json"), + JSON.stringify({ + name: "pi-intercom", + version: "0.0.0", + pi: { extensions: ["./index.ts"] }, + }), + ); + writeFileSync( + join(packageDir, "index.ts"), + `export default function (pi) { + pi.registerTool({ name: "intercom", label: "fixture", description: "fixture", + parameters: { type: "object", properties: {} }, + async execute() { return { content: [{ type: "text", text: "ok" }] }; } + }); + }`, + ); + const settingsPath = join(agentDir, "settings.json"); + const previousSettings = existsSync(settingsPath) + ? readFileSync(settingsPath) + : undefined; + const previousTools = [...activeTools]; + const previousGetAllTools = pi.getAllTools; + let parent: AgentSession | undefined; + let prompts = 0; + try { + writeFileSync(settingsPath, JSON.stringify({ packages: [packageDir] })); + const settingsManager = SettingsManager.create(repoDir, agentDir); + const loader = new DefaultResourceLoader({ + cwd: repoDir, + agentDir, + settingsManager, + }); + await loader.reload(); + ({ session: parent } = await createAgentSession({ + cwd: repoDir, + agentDir, + resourceLoader: loader, + settingsManager, + sessionManager: SessionManager.inMemory(repoDir), + })); + await parent.bindExtensions({ mode: "print" }); + parent.setActiveToolsByName(["read", "intercom"]); + const parentSession = parent; + activeTools = parentSession.getActiveToolNames(); + pi.getAllTools = () => parentSession.getAllTools(); + __setWorkflowTestAgentSessionFactory(async (options) => { + assert.deepEqual(options?.tools, ["read"]); + assert.equal( + options?.resourceLoader + ?.getExtensions() + .extensions.some((extension) => extension.tools.has("intercom")), + false, + ); + return { + session: fakeAgentSession( + "intercom boundary verified", + undefined, + () => prompts++, + ), + }; + }); + const result = (await workflow.execute( + "intercom-inherit", + { + script: 'return await agent("inspect", { agent_type: "explorer" });', + wait: true, + }, + undefined, + undefined, + ctx, + )) as { content: Array<{ text: string }> }; + assert.match(result.content[0]!.text, /intercom boundary verified/); + assert.equal(prompts, 1); + assert.deepEqual(parentSession.getActiveToolNames(), ["read", "intercom"]); + } finally { + __setWorkflowTestAgentSessionFactory(undefined); + pi.getAllTools = previousGetAllTools; + activeTools = previousTools; + if (parent) await shutdownAndDisposeChildSession(parent); + if (previousSettings) writeFileSync(settingsPath, previousSettings); + else rmSync(settingsPath, { force: true }); + } +});