Skip to content
Open
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
88 changes: 87 additions & 1 deletion extensions/shared/child-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,17 +550,103 @@ export function effectiveChildToolAllowlist(tools?: readonly string[]) {
);
}

export type ChildToolDescriptor = {
name: string;
sourceInfo?: {
path?: string;
source?: string;
baseDir?: string;
scope?: string;
origin?: string;
};
};

export interface ChildToolInheritanceOptions {
availableTools?: readonly ChildToolDescriptor[];
cwd?: string;
agentDir?: string;
}

function isChildToolDescriptorList(
options: unknown,
): options is readonly ChildToolDescriptor[] {
return Array.isArray(options);
}

/**
* Checks whether a tool originates from a package that is blocked from child sessions.
* Currently, pi-intercom packages are blocked (via blockedPackageSources) to avoid
* process.env session cross-wiring in concurrent child sessions (#128).
*/
export function isBlockedChildTool(
tool: ChildToolDescriptor,
options: { cwd?: string; agentDir?: string } = {},
) {
if (!tool.sourceInfo) return false;
const { source, baseDir, path: toolFilePath } = tool.sourceInfo;
if (
source === "builtin" ||
source === "sdk" ||
(toolFilePath && toolFilePath.startsWith("<"))
) {
return false;
}
const isPiIntercomPackage = createPiIntercomPackageMatcher({
cwd: options.cwd ?? process.cwd(),
agentDir: options.agentDir ?? getAgentDir(),
});
const candidatePath = baseDir ?? toolFilePath;
try {
return isPiIntercomPackage(source ?? "", candidatePath);
} catch {
if (
source &&
(source === "npm:pi-intercom" || source.includes("pi-intercom"))
) {
return true;
}
if (candidatePath && candidatePath.includes("pi-intercom")) {
return true;
}
return false;
}
}

/** 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.
* Tools registered by packages that are blocked from child sessions (e.g. pi-intercom)
* are dynamically dropped during inheritance when availableTools metadata is provided.
*/
export function inheritedChildToolAllowlist(
parentTools: readonly string[],
roleTools?: readonly string[],
options?: ChildToolInheritanceOptions | readonly ChildToolDescriptor[],
) {
const allowed = roleTools === undefined ? undefined : new Set(roleTools);
const optionsObj: ChildToolInheritanceOptions = isChildToolDescriptorList(
options,
)
? { availableTools: options }
: (options ?? {});

const blockedTools = new Set<string>();
if (optionsObj.availableTools) {
for (const tool of optionsObj.availableTools) {
if (
isBlockedChildTool(tool, {
cwd: optionsObj.cwd,
agentDir: optionsObj.agentDir,
})
) {
blockedTools.add(tool.name);
}
}
}

return effectiveChildToolAllowlist([...new Set(parentTools)])!.filter(
(name) => allowed === undefined || allowed.has(name),
(name) =>
!blockedTools.has(name) && (allowed === undefined || allowed.has(name)),
);
}

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
78 changes: 78 additions & 0 deletions tests/extensions/shared/child-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1575,3 +1575,81 @@ test("child delegation inherits active tools and custom restrictions only narrow
assert.deepEqual(inheritedChildToolAllowlist(parent, []), []);
assert.deepEqual(inheritedChildToolAllowlist([], ["bash"]), []);
});

test("tools from blocked packages like pi-intercom are dynamically excluded from child allowlist and pass preflight", async () => {
// Simulate a parent session where pi-intercom is active alongside native tools and third-party tools
const parentActiveTools = [
"read",
"bash",
"edit",
"intercom",
"intercom_git",
"weather",
];
const availableTools = [
{ name: "read", sourceInfo: { source: "builtin" } },
{ name: "bash", sourceInfo: { source: "builtin" } },
{ name: "edit", sourceInfo: { source: "builtin" } },
{ name: "weather", sourceInfo: { source: "npm:pi-weather" } },
{ name: "intercom", sourceInfo: { source: "npm:pi-intercom" } },
{
name: "intercom_git",
sourceInfo: {
source: "git:https://github.com/nicobailon/pi-intercom",
},
},
];

// 1. CHILD_EXCLUDED_TOOL_NAMES must NOT include community tool names
assert.equal(
(CHILD_EXCLUDED_TOOL_NAMES as readonly string[]).includes("intercom"),
false,
"CHILD_EXCLUDED_TOOL_NAMES must remain strictly for OpenPI package tools",
);

// 2. Inherited allowlist dynamically drops tools from blocked packages
const inherited = inheritedChildToolAllowlist(parentActiveTools, undefined, {
availableTools,
});
assert.deepEqual(inherited, ["read", "bash", "edit", "weather"]);
assert.equal(inherited.includes("intercom"), false);
assert.equal(inherited.includes("intercom_git"), false);
assert.equal(inherited.includes("weather"), true);

// 3. An explicit role allowlist naming a blocked package tool must also drop it
const explicitNarrowed = inheritedChildToolAllowlist(
parentActiveTools,
["read", "intercom", "weather"],
{ availableTools },
);
assert.deepEqual(explicitNarrowed, ["read", "weather"]);

// 4. Array shorthand for options works identically
const arrayShorthand = inheritedChildToolAllowlist(
parentActiveTools,
undefined,
availableTools,
);
assert.deepEqual(arrayShorthand, ["read", "bash", "edit", "weather"]);

// 5. Child tool policy constructed from inherited tools has no blocked tools
const policy = childToolPolicy(inherited);
assert.equal(policy.tools?.includes("intercom"), false);

// 6. bindChildSessionExtensions preflight must pass with the sanitized inherited allowlist
const mockChildSession = {
async bindExtensions() {},
getActiveToolNames: () => ["read", "bash", "edit", "weather"],
getAllTools: () => [
{ name: "read" },
{ name: "bash" },
{ name: "edit" },
{ name: "weather" },
],
setActiveToolsByName(_names: string[]) {},
};

await assert.doesNotReject(
bindChildSessionExtensions(mockChildSession, inherited),
);
});
Loading