Skip to content

Commit 489c55f

Browse files
committed
fix: prevent command injection from workspace config, gate workspace mise binaries
Security hardening for values controlled by the opened repository (task names, tool names, env vars, mise.binPath): - Pass all mise invocations as argv arrays (no shell interpolation); - Ask for user approval before running a mise binary located inside the workspace (e.g. set via a committed .vscode/settings.json). - Fix setupTaskFile containment check and validate new task names - Escape values interpolated into trusted markdown tooltips (command: link injection) and restrict tool website links to http(s)
1 parent 9ac024c commit 489c55f

25 files changed

Lines changed: 1986 additions & 204 deletions

.vscode-test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,20 @@ module.exports = defineConfig([
5959
timeout: 60_000,
6060
},
6161
},
62+
{
63+
label: "command-injection",
64+
files: "src/e2e-tests/command-injection/*.e2e.ts",
65+
workspaceFolder: path.join(fixturesPath, "command-injection-workspace"),
66+
env: {
67+
MISE_CEILING_PATHS: fixturesPath,
68+
MISE_LOCKED: "0",
69+
MISE_TRUSTED_CONFIG_PATHS: fixturesPath,
70+
},
71+
mocha: {
72+
require: ["tsx/cjs"],
73+
timeout: 60_000,
74+
},
75+
},
6276
{
6377
label: "monorepo",
6478
files: "src/e2e-tests/monorepo/*.e2e.ts",

package.json

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@
188188
"markdownDescription": "Path to the mise binary (automatically detected on startup).\n\nIf set to `mise` (default), it will use `mise` available in `PATH`.\n\nRelative paths (e.g. `./bin/mise`) and `${workspaceFolder}` variables are resolved against the workspace folders.\n\nSee https://mise.jdx.dev/getting-started.html to install mise.",
189189
"default": "mise"
190190
},
191+
"mise.skipWorkspaceBinaryApproval": {
192+
"type": "boolean",
193+
"default": false,
194+
"scope": "machine",
195+
"markdownDescription": "Run a mise binary located inside the workspace without asking for approval first.\n\n**This turns off a security check.** A repository can point `mise.binPath` at a program it ships, and with this enabled that program runs as soon as you open the project. Useful if you rely on a committed launcher such as the one written by `mise generate bootstrap -l -w`.\n\nYou still have to trust the folder in VS Code first: the extension does not run in an untrusted workspace at all. This setting is machine-scoped, so a project cannot enable it for you."
196+
},
191197
"mise.miseEnv": {
192198
"order": 3,
193199
"type": "string",
@@ -676,6 +682,17 @@
676682
"title": "Mise: Open Menu",
677683
"enablement": "!isWeb"
678684
},
685+
{
686+
"command": "mise.reviewWorkspaceBinary",
687+
"title": "Mise: Review the workspace mise binary",
688+
"icon": "$(shield)",
689+
"enablement": "!isWeb"
690+
},
691+
{
692+
"command": "mise.revokeWorkspaceBinaryApprovals",
693+
"title": "Mise: Revoke approved workspace mise binaries",
694+
"enablement": "!isWeb"
695+
},
679696
{
680697
"command": "mise.openMissingToolsMenu",
681698
"title": "Mise: Open Missing Tools Menu",

src/commands.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,6 @@ export const MISE_HIDE_ENV_VARIABLE_VALUE = "mise.hideEnvVariableValue";
4747
export const MISE_DISPLAY_PATH = "mise.displayPath";
4848
export const MISE_DOCTOR = "mise.doctor";
4949
export const MISE_ENABLE_AUTO_CONFIGURATION = "mise.enableAutoConfiguration";
50+
export const MISE_REVIEW_WORKSPACE_BINARY = "mise.reviewWorkspaceBinary";
51+
export const MISE_REVOKE_WORKSPACE_BINARIES =
52+
"mise.revokeWorkspaceBinaryApprovals";

src/configuration.ts

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export const CONFIGURATION_FLAGS = {
3636
customBinaryExtensions: "customBinaryExtensions",
3737
customFolderExtensions: "customFolderExtensions",
3838
enableTaskSymbolProvider: "enableTaskSymbolProvider",
39+
skipWorkspaceBinaryApproval: "skipWorkspaceBinaryApproval",
3940
} as const;
4041

4142
const getExtensionConfig = () => {
@@ -52,21 +53,21 @@ export const getConfOrElse = <T>(
5253
export const getIgnoreList = (): string[] => {
5354
return getConfOrElse(
5455
CONFIGURATION_FLAGS.configureExtensionsAutomaticallyIgnoreList,
55-
[],
56+
["biomejs.biome", "oxc.oxc-vscode"],
5657
);
5758
};
5859

5960
export const getIncludeList = (): string[] => {
6061
return getConfOrElse(
6162
CONFIGURATION_FLAGS.configureExtensionsAutomaticallyIncludeList,
62-
[],
63+
["all"],
6364
);
6465
};
6566

6667
export const shouldConfigureExtensionsAutomatically = (): boolean => {
6768
return getConfOrElse(
6869
CONFIGURATION_FLAGS.configureExtensionsAutomatically,
69-
true,
70+
false,
7071
);
7172
};
7273

@@ -85,7 +86,7 @@ export const shouldUseShims = () => {
8586
export const shouldUseSymLinks = () => {
8687
return getConfOrElse(
8788
CONFIGURATION_FLAGS.configureExtensionsUseSymLinks,
88-
true,
89+
false,
8990
);
9091
};
9192

@@ -132,6 +133,54 @@ export const getConfiguredBinPath = (): string | undefined => {
132133
);
133134
};
134135

136+
export type BinPathSource =
137+
| "workspaceFolder"
138+
| "workspace"
139+
| "global"
140+
| "default";
141+
142+
/**
143+
* Where `mise.binPath` is set. The setting is `window` scoped, so a repository
144+
* can ship it in its own `.vscode/settings.json`: telling the two apart is what
145+
* lets the user know whether they chose the binary or the project did.
146+
*/
147+
export const getBinPathSource = (): {
148+
source: BinPathSource;
149+
value: string | undefined;
150+
} => {
151+
const inspection = getExtensionConfig().inspect<string>(
152+
CONFIGURATION_FLAGS.binPath,
153+
);
154+
155+
if (inspection?.workspaceFolderValue !== undefined) {
156+
return {
157+
source: "workspaceFolder",
158+
value: inspection.workspaceFolderValue,
159+
};
160+
}
161+
if (inspection?.workspaceValue !== undefined) {
162+
return { source: "workspace", value: inspection.workspaceValue };
163+
}
164+
if (inspection?.globalValue !== undefined) {
165+
return { source: "global", value: inspection.globalValue };
166+
}
167+
return { source: "default", value: inspection?.defaultValue };
168+
};
169+
170+
/**
171+
* Whether the approval prompt for a workspace mise binary is turned off. The
172+
* setting is machine scoped, so a repository cannot enable it for the user.
173+
*/
174+
export const shouldSkipWorkspaceBinaryApproval = (): boolean => {
175+
return getConfOrElse(CONFIGURATION_FLAGS.skipWorkspaceBinaryApproval, false);
176+
};
177+
178+
/** Whether `mise.binPath` comes from settings committed to the project */
179+
export const isBinPathSetByWorkspace = (): boolean => {
180+
const { source } = getBinPathSource();
181+
return source === "workspace" || source === "workspaceFolder";
182+
};
183+
135184
export const updateBinPath = async (binPath: string) => {
136185
logger.info(`Updating bin path to: ${binPath}`);
137186

@@ -218,11 +267,11 @@ export const shouldShowNotificationIfMissingTools = () => {
218267
};
219268

220269
export const isTeraAutoCompletionEnabled = () => {
221-
return getConfOrElse(CONFIGURATION_FLAGS.teraAutoCompletion, false);
270+
return getConfOrElse(CONFIGURATION_FLAGS.teraAutoCompletion, true);
222271
};
223272

224273
export const getCommandTTLCacheSeconds = () => {
225-
return getConfOrElse(CONFIGURATION_FLAGS.commandTTLCacheSeconds, 1);
274+
return getConfOrElse(CONFIGURATION_FLAGS.commandTTLCacheSeconds, 2);
226275
};
227276

228277
export const shouldAutoDetectMiseBinPath = () => {
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
import * as assert from "node:assert";
2+
import { exec } from "node:child_process";
3+
import { readdir, readFile, rm, writeFile } from "node:fs/promises";
4+
import * as path from "node:path";
5+
import { promisify } from "node:util";
6+
import * as vscode from "vscode";
7+
import { MiseService } from "../../miseService";
8+
9+
const execAsync = promisify(exec);
10+
11+
/**
12+
* Task and tool names come from the repository configuration, so a repository
13+
* that gets opened in vscode fully controls them. None of them may ever reach a
14+
* shell as anything other than a single literal argument.
15+
*
16+
* Every name in the fixture carries a payload creating a `pwned-*` file in the
17+
* workspace: the assertions below check that no such file is ever created,
18+
* while the task itself still runs and prints its marker.
19+
*/
20+
suite("Command Injection Test Suite", function () {
21+
this.timeout(60_000);
22+
23+
const TASK_DQ = 'inj-dq"; touch pwned-dq #';
24+
const TASK_SQ = "inj-sq'; touch pwned-sq #";
25+
const TASK_SUB = "inj-sub$(touch pwned-sub)`touch pwned-bt`";
26+
const TASK_SPACES = "inj spaces & touch pwned-amp | name";
27+
28+
const INJECTING_TASKS = [
29+
{ name: TASK_DQ, marker: "marker-dq" },
30+
{ name: TASK_SQ, marker: "marker-sq" },
31+
{ name: TASK_SUB, marker: "marker-sub" },
32+
{ name: TASK_SPACES, marker: "marker-spaces" },
33+
];
34+
35+
let workspaceRoot: string;
36+
let miseService: MiseService;
37+
38+
// the service only reads workspaceState from the context
39+
const fakeContext = {
40+
workspaceState: { get: () => undefined },
41+
} as unknown as vscode.ExtensionContext;
42+
43+
const listPwnedFiles = async () =>
44+
(await readdir(workspaceRoot)).filter((name) => name.startsWith("pwned"));
45+
46+
const assertNoInjection = async (context: string) => {
47+
assert.deepEqual(
48+
await listPwnedFiles(),
49+
[],
50+
`${context} executed an injected command`,
51+
);
52+
};
53+
54+
suiteSetup(async () => {
55+
workspaceRoot = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath ?? "";
56+
assert.ok(workspaceRoot, "Workspace root should be available");
57+
58+
miseService = new MiseService(fakeContext);
59+
await miseService.initializeMisePath();
60+
assert.ok(
61+
miseService.getMiseBinaryPath(),
62+
"mise binary should be resolved",
63+
);
64+
});
65+
66+
teardown(async () => {
67+
for (const name of await listPwnedFiles()) {
68+
await rm(path.join(workspaceRoot, name), { force: true });
69+
}
70+
});
71+
72+
test("the fixture exposes the task names an attacker would use", async () => {
73+
const tasks = await miseService.getTasks();
74+
75+
assert.deepEqual(
76+
tasks.map((task) => task.name).sort(),
77+
[TASK_SPACES, TASK_DQ, TASK_SQ, TASK_SUB, "echo-ok"].sort(),
78+
);
79+
});
80+
81+
test("getTaskInfo resolves hostile task names instead of running them", async () => {
82+
for (const { name } of INJECTING_TASKS) {
83+
const taskInfo = await miseService.getTaskInfo(name);
84+
85+
assert.ok(taskInfo, `task info should be returned for ${name}`);
86+
assert.equal(taskInfo?.name, name);
87+
await assertNoInjection(`getTaskInfo(${name})`);
88+
}
89+
});
90+
91+
test("the command sent to the terminal runs the task and nothing else", async function () {
92+
if (process.platform === "win32") {
93+
// the generated command targets powershell, sh cannot evaluate it
94+
this.skip();
95+
}
96+
97+
for (const { name, marker } of INJECTING_TASKS) {
98+
const command = miseService.createMiseCommand(["run", name]);
99+
assert.ok(command, "a command should be built");
100+
101+
const { stdout } = await execAsync(command as string, {
102+
cwd: workspaceRoot,
103+
});
104+
105+
assert.ok(
106+
stdout.includes(marker),
107+
`expected ${marker} in the output of ${name}, got: ${stdout}`,
108+
);
109+
await assertNoInjection(`run '${name}'`);
110+
}
111+
});
112+
113+
test("task arguments are passed through without reaching the shell", async function () {
114+
if (process.platform === "win32") {
115+
this.skip();
116+
}
117+
118+
const command = miseService.createMiseCommand([
119+
"run",
120+
"echo-ok",
121+
"--",
122+
"$(touch pwned-arg)",
123+
]);
124+
assert.ok(command, "a command should be built");
125+
126+
const { stdout } = await execAsync(command as string, {
127+
cwd: workspaceRoot,
128+
});
129+
130+
assert.ok(stdout.includes("marker-ok"), `unexpected output: ${stdout}`);
131+
await assertNoInjection("run with an injected argument");
132+
});
133+
134+
test("tasks executed through the vscode tasks api stay a single argument", async () => {
135+
const tasks = await vscode.tasks.fetchTasks({ type: "mise" });
136+
137+
for (const { name } of INJECTING_TASKS) {
138+
const task = tasks.find((t) => t.name === name);
139+
assert.ok(task, `task ${name} should be provided to vscode`);
140+
141+
const exitCode = await runVsCodeTask(task as vscode.Task);
142+
143+
assert.equal(exitCode, 0, `task ${name} should succeed`);
144+
await assertNoInjection(`vscode task '${name}'`);
145+
}
146+
});
147+
148+
test("tool actions running in a terminal quote their arguments", async () => {
149+
// mise exits non-zero on the unknown setting, the point is that the
150+
// payload in the name must not be executed by the task shell
151+
await miseService.runMiseToolActionInConsole(
152+
["settings", "get", "x; touch pwned-console #"],
153+
"injection-test",
154+
);
155+
156+
await assertNoInjection("runMiseToolActionInConsole");
157+
});
158+
159+
test("tool lookups do not execute hostile tool names", async () => {
160+
const toolName = "x; touch pwned-tool #";
161+
162+
assert.equal(await miseService.which(toolName), undefined);
163+
await assertNoInjection("which");
164+
165+
await miseService.binPaths(toolName).catch(() => []);
166+
await assertNoInjection("binPaths");
167+
168+
// mise rejects the unknown setting instead of the payload being run
169+
await assert.rejects(() =>
170+
miseService.getSetting("x; touch pwned-setting #"),
171+
);
172+
await assertNoInjection("getSetting");
173+
});
174+
175+
test("environment variables are written verbatim, without being evaluated", async () => {
176+
const envFilePath = path.join(workspaceRoot, "mise.injection.toml");
177+
await writeFile(envFilePath, "");
178+
179+
try {
180+
await miseService.miseSetEnv({
181+
filePath: envFilePath,
182+
name: "INJECTED",
183+
value: '$(touch pwned-env) "; touch pwned-env2 #',
184+
});
185+
186+
const content = await readFile(envFilePath, "utf8");
187+
assert.ok(
188+
content.includes("touch pwned-env"),
189+
`the value should be stored as written, got: ${content}`,
190+
);
191+
await assertNoInjection("miseSetEnv");
192+
} finally {
193+
await rm(envFilePath, { force: true });
194+
}
195+
});
196+
});
197+
198+
function runVsCodeTask(task: vscode.Task): Promise<number | undefined> {
199+
return new Promise<number | undefined>((resolve, reject) => {
200+
const disposable = vscode.tasks.onDidEndTaskProcess((event) => {
201+
if (event.execution.task === task) {
202+
disposable.dispose();
203+
resolve(event.exitCode);
204+
}
205+
});
206+
207+
vscode.tasks.executeTask(task).then(undefined, (error) => {
208+
disposable.dispose();
209+
reject(error);
210+
});
211+
});
212+
}

0 commit comments

Comments
 (0)