Skip to content

Commit fe67290

Browse files
committed
fix(workflows): contain tool-package entry resolution via realpath + npm-name gate
Realpath containment mirrors vendor workflow-definition-loader: rejects interchange.tools escapes through file or directory symlinks. Gate dep names with ToolPackagePinName so keys like ../../evil cannot escape node_modules. Adds 3 regression tests.
1 parent bcd5df7 commit fe67290

2 files changed

Lines changed: 85 additions & 2 deletions

File tree

‎src/workflows/tool-packages.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,56 @@ describe("resolveWorkflowToolPackages", () => {
170170
).rejects.toThrow(/escapes its directory/);
171171
});
172172

173+
test("rejects an interchange.tools entry that escapes via a file symlink", async () => {
174+
const workflowDir = await makeWorkflow(root, {
175+
[TOOL_NAME]: "workspace:*",
176+
});
177+
const toolDir = await layOutTool(workflowDir, {
178+
name: TOOL_NAME,
179+
version: "1.2.3",
180+
interchange: { tools: "./dist/tools.js" },
181+
});
182+
const outside = join(root, "evil.js");
183+
await writeFile(outside, "export const tools = [];\n");
184+
await rm(join(toolDir, "dist", "tools.js"));
185+
await symlink(outside, join(toolDir, "dist", "tools.js"));
186+
await expect(
187+
resolveWorkflowToolPackages(workflowDir, [TOOL_NAME]),
188+
).rejects.toThrow(/escapes its directory via a symlink/);
189+
});
190+
191+
test("rejects an interchange.tools entry that escapes via a directory symlink", async () => {
192+
const workflowDir = await makeWorkflow(root, {
193+
[TOOL_NAME]: "workspace:*",
194+
});
195+
const outsideDir = join(root, "evil-dir");
196+
await mkdir(outsideDir, { recursive: true });
197+
await writeFile(
198+
join(outsideDir, "tools.js"),
199+
"export const tools = [];\n",
200+
);
201+
const toolDir = join(workflowDir, "node_modules", TOOL_NAME);
202+
await mkdir(toolDir, { recursive: true });
203+
await writeJSON(join(toolDir, "package.json"), {
204+
name: TOOL_NAME,
205+
version: "1.2.3",
206+
interchange: { tools: "./linked/tools.js" },
207+
});
208+
await symlink(outsideDir, join(toolDir, "linked"));
209+
await expect(
210+
resolveWorkflowToolPackages(workflowDir, [TOOL_NAME]),
211+
).rejects.toThrow(/escapes its directory via a symlink/);
212+
});
213+
214+
test("rejects a tool package name that is not a valid npm package name", async () => {
215+
const workflowDir = await makeWorkflow(root, {
216+
[TOOL_NAME]: "workspace:*",
217+
});
218+
await expect(
219+
resolveWorkflowToolPackages(workflowDir, ["../../evil"]),
220+
).rejects.toThrow(/not a valid npm package name/);
221+
});
222+
173223
test("rejects a resolved directory whose package.json names another package", async () => {
174224
const workflowDir = await makeWorkflow(root, {
175225
[TOOL_NAME]: "workspace:*",

‎src/workflows/tool-packages.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { readFile, realpath } from "node:fs/promises";
2-
import { join } from "node:path";
2+
import { join, sep } from "node:path";
33
import { ArkErrors, type } from "arktype";
44

55
import { isContainedEntryPath, PackageJSON } from "@intx/types/package-json";
6+
import { ToolPackagePinName } from "@intx/types/tool-packages";
67

78
// A workflow package declares the tool packages its steps need as npm
89
// dependencies (CL-4473). Resolution reads the laid-out `node_modules/` tree,
@@ -52,6 +53,15 @@ async function resolveOne(
5253
declared: ReadonlySet<string>,
5354
name: string,
5455
): Promise<WorkflowToolPackage> {
56+
// A declared dep key is joined onto `node_modules/` below, so the name must
57+
// satisfy npm's package-name rules first — otherwise a key like
58+
// `../../evil` resolves outside the workflow package. Same rule the hub
59+
// enforces on tool-package pins (`ToolPackagePinName`).
60+
if (ToolPackagePinName(name) instanceof ArkErrors) {
61+
throw new Error(
62+
`tool package ${JSON.stringify(name)} is not a valid npm package name`,
63+
);
64+
}
5565
if (!declared.has(name)) {
5666
throw new Error(
5767
`tool package ${JSON.stringify(name)} is not a dependency of the workflow package at ${workflowDir}; declare it as an npm dep`,
@@ -89,11 +99,34 @@ async function resolveOne(
8999
`tool package ${JSON.stringify(name)} at ${dir} declares an "interchange.tools" entry ${JSON.stringify(entryRel)} that escapes its directory`,
90100
);
91101
}
102+
// Realpath containment, mirroring the vendored workflow-definition-loader:
103+
// the string check above rejects `..`/absolute paths, and this check
104+
// rejects an escape through a file or directory symlink inside the package.
105+
// Both sides are realpath'd so the comparison holds under a symlinked
106+
// parent (e.g. macOS `/tmp` -> `/private/tmp`).
107+
const entryAbs = join(dir, entryRel);
108+
let realDir: string;
109+
let realEntry: string;
110+
try {
111+
realDir = await realpath(dir);
112+
realEntry = await realpath(entryAbs);
113+
} catch (cause) {
114+
throw new Error(
115+
`tool package ${JSON.stringify(name)} at ${dir} declares an "interchange.tools" entry ${JSON.stringify(entryRel)} that could not be resolved`,
116+
{ cause },
117+
);
118+
}
119+
const containmentRoot = realDir.endsWith(sep) ? realDir : realDir + sep;
120+
if (realEntry !== realDir && !realEntry.startsWith(containmentRoot)) {
121+
throw new Error(
122+
`tool package ${JSON.stringify(name)} at ${dir} declares an "interchange.tools" entry ${JSON.stringify(entryRel)} that escapes its directory via a symlink`,
123+
);
124+
}
92125
return {
93126
name,
94127
version: parsed.version,
95128
dir,
96-
toolsEntry: join(dir, entryRel),
129+
toolsEntry: entryAbs,
97130
};
98131
}
99132

0 commit comments

Comments
 (0)