Skip to content

Commit aba88fe

Browse files
pgondhi987pgondhi987
committed
fix: fail closed cwd and yarn package shorthands
1 parent 71bfe80 commit aba88fe

5 files changed

Lines changed: 206 additions & 31 deletions

‎src/infra/exec-allow-always-persistence.test.ts‎

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,30 @@ describe("resolveAllowAlwaysPersistenceDecision", () => {
6161
});
6262
});
6363

64+
it("persists pnpm cwd exec approvals against the inner executable", async () => {
65+
const dir = makeTempDir();
66+
makeExecutable(dir, "pnpm");
67+
const tsxPath = makeExecutable(dir, "tsx");
68+
const env = makePathEnv(dir);
69+
const command = "pnpm -C ./package exec -- tsx ./run.ts";
70+
const plan = await planShellAuthorization({ command, cwd: dir, env });
71+
72+
const decision = resolveAllowAlwaysPersistenceDecision({
73+
segments: plannedSegments(plan),
74+
commandText: command,
75+
cwd: dir,
76+
env,
77+
platform: process.platform,
78+
authorizationPlan: plan,
79+
});
80+
81+
expect(decision).toEqual({
82+
kind: "patterns",
83+
commandText: command,
84+
patterns: [expect.objectContaining({ pattern: tsxPath })],
85+
});
86+
});
87+
6488
it.each(["env --", "nice"])(
6589
"persists dispatch-wrapped package-manager exec approvals against the inner executable: %s",
6690
async (wrapper) => {
@@ -321,31 +345,32 @@ describe("resolveAllowAlwaysPersistenceDecision", () => {
321345
});
322346
});
323347

324-
it("keeps yarn run as a script-runner command instead of delegated exec", async () => {
325-
const dir = makeTempDir();
326-
const yarnPath = makeExecutable(dir, "yarn");
327-
for (const executable of ["sh", "echo"]) {
328-
makeExecutable(dir, executable);
329-
}
330-
const env = makePathEnv(dir);
331-
const command = "yarn run sh -c 'echo warmup-ok'";
332-
const plan = await planShellAuthorization({ command, cwd: dir, env });
348+
it.each(["yarn run sh -c 'echo warmup-ok'", "yarn sh -c 'echo warmup-ok'"])(
349+
"keeps yarn script or bin fallback carriers one-shot: %s",
350+
async (command) => {
351+
const dir = makeTempDir();
352+
makeExecutable(dir, "yarn");
353+
for (const executable of ["sh", "echo"]) {
354+
makeExecutable(dir, executable);
355+
}
356+
const env = makePathEnv(dir);
357+
const plan = await planShellAuthorization({ command, cwd: dir, env });
333358

334-
const decision = resolveAllowAlwaysPersistenceDecision({
335-
segments: plannedSegments(plan),
336-
commandText: command,
337-
cwd: dir,
338-
env,
339-
platform: process.platform,
340-
authorizationPlan: plan,
341-
});
359+
const decision = resolveAllowAlwaysPersistenceDecision({
360+
segments: plannedSegments(plan),
361+
commandText: command,
362+
cwd: dir,
363+
env,
364+
platform: process.platform,
365+
authorizationPlan: plan,
366+
});
342367

343-
expect(decision).toEqual({
344-
kind: "patterns",
345-
commandText: command,
346-
patterns: [expect.objectContaining({ pattern: yarnPath })],
347-
});
348-
});
368+
expect(decision).toEqual({
369+
kind: "one-shot",
370+
reasons: expect.arrayContaining(["no-reusable-pattern"]),
371+
});
372+
},
373+
);
349374

350375
it.each(["env --", "nice"])(
351376
"keeps dispatch-wrapped package-manager shell carriers one-shot: %s",

‎src/infra/exec-approvals-allow-always.test.ts‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ $0 \\"$1\\"" touch {marker}`,
10961096
{ command: "pnpm run build -- node", executable: "pnpm" },
10971097
{ command: "pnpm test -- node", executable: "pnpm" },
10981098
{ command: "pnpm install", executable: "pnpm" },
1099-
{ command: "yarn run build -- node", executable: "yarn" },
1099+
{ command: "yarn install", executable: "yarn" },
11001100
])(
11011101
"keeps exec-like arguments on known non-exec package-manager subcommands allowlisted: $command",
11021102
async ({ command, executable }) => {
@@ -1152,6 +1152,71 @@ $0 \\"$1\\"" touch {marker}`,
11521152
).toBe(true);
11531153
});
11541154

1155+
it("rejects stale pnpm allow-always entries for cwd implicit exec shorthands", async () => {
1156+
if (process.platform === "win32") {
1157+
return;
1158+
}
1159+
const dir = makeTempDir();
1160+
const pnpmPath = makeExecutable(dir, "pnpm");
1161+
makeExecutable(dir, "eslint");
1162+
const env = makePathEnv(dir);
1163+
const safeBins = resolveSafeBins(undefined);
1164+
1165+
const result = await evaluateShellAllowlistWithAuthorization({
1166+
command: "pnpm -C ./package eslint .",
1167+
allowlist: [{ pattern: pnpmPath, source: "allow-always" }],
1168+
safeBins,
1169+
cwd: dir,
1170+
env,
1171+
platform: process.platform,
1172+
});
1173+
1174+
expect(result.allowlistSatisfied).toBe(false);
1175+
expect(result.segmentAllowlistEntries).toEqual([null]);
1176+
expect(
1177+
requiresExecApproval({
1178+
ask: "on-miss",
1179+
security: "allowlist",
1180+
analysisOk: result.analysisOk,
1181+
allowlistSatisfied: result.allowlistSatisfied,
1182+
}),
1183+
).toBe(true);
1184+
});
1185+
1186+
it.each(["yarn run eslint .", "yarn eslint ."])(
1187+
"rejects stale yarn allow-always entries for script or bin fallback: %s",
1188+
async (command) => {
1189+
if (process.platform === "win32") {
1190+
return;
1191+
}
1192+
const dir = makeTempDir();
1193+
const yarnPath = makeExecutable(dir, "yarn");
1194+
makeExecutable(dir, "eslint");
1195+
const env = makePathEnv(dir);
1196+
const safeBins = resolveSafeBins(undefined);
1197+
1198+
const result = await evaluateShellAllowlistWithAuthorization({
1199+
command,
1200+
allowlist: [{ pattern: yarnPath, source: "allow-always" }],
1201+
safeBins,
1202+
cwd: dir,
1203+
env,
1204+
platform: process.platform,
1205+
});
1206+
1207+
expect(result.allowlistSatisfied).toBe(false);
1208+
expect(result.segmentAllowlistEntries).toEqual([null]);
1209+
expect(
1210+
requiresExecApproval({
1211+
ask: "on-miss",
1212+
security: "allowlist",
1213+
analysisOk: result.analysisOk,
1214+
allowlistSatisfied: result.allowlistSatisfied,
1215+
}),
1216+
).toBe(true);
1217+
},
1218+
);
1219+
11551220
it("requires bound args for package-manager shell script carriers", async () => {
11561221
if (process.platform === "win32") {
11571222
return;
@@ -1267,6 +1332,16 @@ $0 \\"$1\\"" touch {marker}`,
12671332
});
12681333
expect(inner.allowlistSatisfied).toBe(true);
12691334

1335+
const pnpmCwdInner = await evaluateShellAllowlistWithAuthorization({
1336+
command: "pnpm -C ./package exec -- tsx ./run.ts",
1337+
allowlist: [{ pattern: tsxPath, source: "allow-always" }],
1338+
safeBins,
1339+
cwd: dir,
1340+
env,
1341+
platform: process.platform,
1342+
});
1343+
expect(pnpmCwdInner.allowlistSatisfied).toBe(true);
1344+
12701345
const npmInner = await evaluateShellAllowlistWithAuthorization({
12711346
command: "npm --loglevel=silent exec -- tsx ./run.ts",
12721347
allowlist: [{ pattern: tsxPath, source: "allow-always" }],

‎src/infra/package-manager-exec-wrapper.ts‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ export const PNPM_OPTIONS_WITH_VALUE = new Set([
3535
"--stream",
3636
"--test-pattern",
3737
"--workspace-concurrency",
38-
"-C",
3938
]);
4039

40+
export const PNPM_CASE_SENSITIVE_OPTIONS_WITH_VALUE = new Set(["-C"]);
41+
4142
export const PNPM_FLAG_OPTIONS = new Set([
4243
"--aggregate-output",
4344
"--color",
@@ -88,6 +89,46 @@ const YARN_FLAG_OPTIONS = new Set(["--immutable", "--silent", "-s"]);
8889
const YARN_DLX_OPTIONS_WITH_VALUE = new Set(["--package", "-p"]);
8990
const YARN_DLX_FLAG_OPTIONS = new Set(["--quiet", "-q"]);
9091
const YARN_EXEC_SUBCOMMANDS = new Set(["exec", "dlx"]);
92+
const YARN_BUILTIN_NON_EXEC_SUBCOMMANDS = new Set([
93+
"add",
94+
"audit",
95+
"autoclean",
96+
"bin",
97+
"cache",
98+
"check",
99+
"config",
100+
"create",
101+
"dedupe",
102+
"generate-lock-entry",
103+
"global",
104+
"help",
105+
"import",
106+
"info",
107+
"init",
108+
"install",
109+
"licenses",
110+
"link",
111+
"list",
112+
"login",
113+
"logout",
114+
"outdated",
115+
"owner",
116+
"pack",
117+
"policies",
118+
"prune",
119+
"publish",
120+
"remove",
121+
"self-update",
122+
"tag",
123+
"team",
124+
"unlink",
125+
"upgrade",
126+
"upgrade-interactive",
127+
"version",
128+
"versions",
129+
"why",
130+
"workspace",
131+
]);
91132

92133
function normalizeOptionFlag(token: string): string {
93134
return normalizeLowercaseStringOrEmpty(parseInlineOptionToken(token).name);
@@ -183,11 +224,16 @@ function unwrapPnpmExecInvocation(argv: string[]): string[] | null {
183224
}
184225
return null;
185226
}
186-
const flag = normalizeOptionFlag(token);
227+
const parsedOption = parseInlineOptionToken(token);
228+
const flag = normalizeLowercaseStringOrEmpty(parsedOption.name);
187229
if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) {
188230
idx += token.includes("=") ? 1 : 2;
189231
continue;
190232
}
233+
if (PNPM_CASE_SENSITIVE_OPTIONS_WITH_VALUE.has(parsedOption.name)) {
234+
idx += token.includes("=") ? 1 : 2;
235+
continue;
236+
}
191237
if (PNPM_FLAG_OPTIONS.has(flag)) {
192238
idx += 1;
193239
continue;
@@ -212,14 +258,19 @@ function unwrapPnpmDlxInvocation(argv: string[]): string[] | null {
212258
if (!token.startsWith("-")) {
213259
return argv.slice(idx);
214260
}
215-
const flag = normalizeOptionFlag(token);
261+
const parsedOption = parseInlineOptionToken(token);
262+
const flag = normalizeLowercaseStringOrEmpty(parsedOption.name);
216263
if (flag === "-c" || flag === "--shell-mode") {
217264
return null;
218265
}
219266
if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) {
220267
idx += token.includes("=") ? 1 : 2;
221268
continue;
222269
}
270+
if (PNPM_CASE_SENSITIVE_OPTIONS_WITH_VALUE.has(parsedOption.name)) {
271+
idx += token.includes("=") ? 1 : 2;
272+
continue;
273+
}
223274
if (PNPM_FLAG_OPTIONS.has(flag)) {
224275
idx += 1;
225276
continue;
@@ -398,6 +449,7 @@ export function resolveKnownPackageManagerExecInvocation(
398449
}
399450
const firstSubcommand = firstSubcommandAfterOptions(argv, {
400451
optionsWithValue: new Set([...PNPM_OPTIONS_WITH_VALUE, ...PNPM_DLX_OPTIONS_WITH_VALUE]),
452+
caseSensitiveOptionsWithValue: PNPM_CASE_SENSITIVE_OPTIONS_WITH_VALUE,
401453
flagOptions: PNPM_FLAG_OPTIONS,
402454
});
403455
const detectedKnownExec = PNPM_EXEC_SUBCOMMANDS.has(firstSubcommand ?? "");
@@ -420,8 +472,13 @@ export function resolveKnownPackageManagerExecInvocation(
420472
optionsWithValue: new Set([...YARN_OPTIONS_WITH_VALUE, ...YARN_DLX_OPTIONS_WITH_VALUE]),
421473
flagOptions: new Set([...YARN_FLAG_OPTIONS, ...YARN_DLX_FLAG_OPTIONS]),
422474
});
423-
return YARN_EXEC_SUBCOMMANDS.has(firstSubcommand ?? "") ||
424-
(firstSubcommand === null && containsSubcommandToken(argv.slice(1), YARN_EXEC_SUBCOMMANDS))
475+
const detectedKnownExec = YARN_EXEC_SUBCOMMANDS.has(firstSubcommand ?? "");
476+
const hiddenKnownExec =
477+
firstSubcommand === null && containsSubcommandToken(argv.slice(1), YARN_EXEC_SUBCOMMANDS);
478+
const implicitRunOrBin =
479+
firstSubcommand !== null &&
480+
(firstSubcommand === "run" || !YARN_BUILTIN_NON_EXEC_SUBCOMMANDS.has(firstSubcommand));
481+
return detectedKnownExec || hiddenKnownExec || implicitRunOrBin
425482
? { kind: "unsafe-exec" }
426483
: { kind: "not-exec" };
427484
}

‎src/node-host/invoke-system-run-plan.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,13 @@ describe("hardenApprovedExecutionPaths", () => {
590590
initialBody: 'console.log("SAFE");\n',
591591
expectedArgvIndex: 5,
592592
},
593+
{
594+
name: "pnpm cwd exec tsx file",
595+
argv: ["pnpm", "-C", "./package", "exec", "tsx", "./run.ts"],
596+
scriptName: "run.ts",
597+
initialBody: 'console.log("SAFE");\n',
598+
expectedArgvIndex: 5,
599+
},
593600
{
594601
name: "pnpm js shim exec tsx file",
595602
argv: ["./pnpm.js", "exec", "tsx", "./run.ts"],

‎src/node-host/invoke-system-run-plan.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { sameFileIdentity } from "../infra/fs-safe-advanced.js";
2323
import { parseInlineOptionToken } from "../infra/inline-option-token.js";
2424
import {
2525
normalizePackageManagerExecToken,
26+
PNPM_CASE_SENSITIVE_OPTIONS_WITH_VALUE,
2627
PNPM_DLX_OPTIONS_WITH_VALUE,
2728
PNPM_FLAG_OPTIONS,
2829
PNPM_OPTIONS_WITH_VALUE,
@@ -803,11 +804,16 @@ function pnpmDlxInvocationNeedsFailClosedBinding(argv: string[], cwd: string | u
803804
}
804805
return pnpmDlxTailNeedsFailClosedBinding(argv.slice(idx + 1), cwd);
805806
}
806-
const flag = normalizeOptionFlag(token);
807+
const parsedOption = parseInlineOptionToken(token);
808+
const flag = normalizeLowercaseStringOrEmpty(parsedOption.name);
807809
if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) {
808810
idx += token.includes("=") ? 1 : 2;
809811
continue;
810812
}
813+
if (PNPM_CASE_SENSITIVE_OPTIONS_WITH_VALUE.has(parsedOption.name)) {
814+
idx += token.includes("=") ? 1 : 2;
815+
continue;
816+
}
811817
if (PNPM_FLAG_OPTIONS.has(flag)) {
812818
idx += 1;
813819
continue;
@@ -832,14 +838,19 @@ function pnpmDlxTailNeedsFailClosedBinding(argv: string[], cwd: string | undefin
832838
if (!token.startsWith("-")) {
833839
return pnpmDlxTailMayNeedStableBinding(argv.slice(idx), cwd);
834840
}
835-
const flag = normalizeOptionFlag(token);
841+
const parsedOption = parseInlineOptionToken(token);
842+
const flag = normalizeLowercaseStringOrEmpty(parsedOption.name);
836843
if (flag === "-c" || flag === "--shell-mode") {
837844
return false;
838845
}
839846
if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) {
840847
idx += token.includes("=") ? 1 : 2;
841848
continue;
842849
}
850+
if (PNPM_CASE_SENSITIVE_OPTIONS_WITH_VALUE.has(parsedOption.name)) {
851+
idx += token.includes("=") ? 1 : 2;
852+
continue;
853+
}
843854
if (PNPM_FLAG_OPTIONS.has(flag)) {
844855
idx += 1;
845856
continue;

0 commit comments

Comments
 (0)