Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 73 additions & 10 deletions extensions/shared/child-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<SourceInfo, "path">) => {
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;
}
Expand Down Expand Up @@ -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<string>(),
project: new Set<string>(),
};
// 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);
}
}
Expand All @@ -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);
Expand Down Expand Up @@ -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<ToolInfo, "name" | "sourceInfo">[];
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);
},
);
}

Expand Down
4 changes: 4 additions & 0 deletions extensions/subagents/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions extensions/workflows/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
127 changes: 118 additions & 9 deletions tests/extensions/shared/child-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import test from "node:test";
import { fileURLToPath, pathToFileURL } from "node:url";
import {
createAgentSession,
createSyntheticSourceInfo,
DefaultPackageManager,
DefaultResourceLoader,
defineTool,
Expand Down Expand Up @@ -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;
}
});
});

Expand Down Expand Up @@ -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/,
);
});
}
});
Expand Down Expand Up @@ -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(`<sdk:${name}>`, { 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/,
);
});
Loading
Loading