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
64 changes: 62 additions & 2 deletions apps/server/src/provider/providerMaintenanceRunner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import * as Schema from "effect/Schema";
import * as Sink from "effect/Sink";
import * as Stream from "effect/Stream";
import { HttpClient, HttpClientResponse } from "effect/unstable/http";
import { ChildProcessSpawner } from "effect/unstable/process";
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";

import { ProviderRegistry, type ProviderRegistryShape } from "./Services/ProviderRegistry.ts";
import * as ProviderMaintenanceRunner from "./providerMaintenanceRunner.ts";
Expand Down Expand Up @@ -125,6 +125,7 @@ function mockSpawnerLayer(
handler: (
command: string,
args: ReadonlyArray<string>,
options: ChildProcess.CommandOptions,
) => {
readonly stdout?: string;
readonly stderr?: string;
Expand All @@ -138,9 +139,32 @@ function mockSpawnerLayer(
const childProcess = command as unknown as {
readonly command: string;
readonly args: ReadonlyArray<string>;
readonly options: ChildProcess.CommandOptions;
};
return Effect.succeed(mockHandle(handler(childProcess.command, childProcess.args)));
return Effect.succeed(
mockHandle(handler(childProcess.command, childProcess.args, childProcess.options)),
);
}),
);
}

function withProcessPlatform<A, E, R>(
platform: NodeJS.Platform,
effect: Effect.Effect<A, E, R>,
): Effect.Effect<A, E, R> {
return Effect.acquireUseRelease(
Effect.sync(() => {
const descriptor = Object.getOwnPropertyDescriptor(process, "platform");
Object.defineProperty(process, "platform", { value: platform });
return descriptor;
}),
() => effect,
(descriptor) =>
Effect.sync(() => {
if (descriptor) {
Object.defineProperty(process, "platform", descriptor);
}
}),
);
}

Expand Down Expand Up @@ -319,6 +343,42 @@ describe("providerMaintenanceRunner", () => {
},
);

it.effect("runs provider update commands through a shell on Windows", () => {
const calls: Array<{
command: string;
args: ReadonlyArray<string>;
shell: ChildProcess.CommandOptions["shell"];
}> = [];
return withProcessPlatform(
"win32",
Effect.gen(function* () {
const { registry } = yield* makeRegistry(baseProvider);
const runner = yield* makeTestRunner(registry);

const result = yield* runner.updateProvider(CODEX_DRIVER);

assert.deepStrictEqual(calls, [
{
command: "npm",
args: ["install", "-g", "@openai/codex@latest"],
shell: true,
},
]);
assert.strictEqual(result.providers[0]?.updateState?.status, "succeeded");
}),
).pipe(
Effect.provide(
Layer.mergeAll(
latestVersionHttpClient("0.0.0"),
mockSpawnerLayer((command, args, options) => {
calls.push({ command, args, shell: options.shell });
return { stdout: "updated" };
}),
),
),
);
});

it.effect("updates a single provider instance without touching sibling instances", () => {
const calls: Array<{ command: string; args: ReadonlyArray<string> }> = [];
return Effect.gen(function* () {
Expand Down
8 changes: 7 additions & 1 deletion apps/server/src/provider/providerMaintenanceRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ const runProviderMaintenanceCommandWithSpawner = Effect.fn("ProviderMaintenanceR
const collectCommandResult = Effect.fn("ProviderMaintenanceRunner.collectCommandResult")(
function* () {
const child = yield* input.spawner
.spawn(ChildProcess.make(input.command, [...input.args]))
.spawn(
ChildProcess.make(
input.command,
[...input.args],
process.platform === "win32" ? { shell: true } : {},
),
)
Comment on lines +79 to +85
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most minimal fix for update command resolution on Windows. This lets Windows resolve shims like npm.cmd, bun.cmd, and other package-manager/provider launchers.

The injection risk is limited, because command and args are generated internally in by providerMaintanance.ts, not accepted directly from user input. That said, the concern is valid as a hardening issue: using a shell broadly expands parsing/quoting semantics and increases risk if future update capabilities become externally influenced.

.pipe(
Effect.mapError(
(cause) =>
Expand Down
Loading