From fd3a09045ecab7de72cdb12a56bbf6ada0174b8a Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Fri, 24 Jul 2026 13:12:27 -0700 Subject: [PATCH] confirm repo-provided setup script before running --- .../core/src/task-detail/taskCreationHost.ts | 6 ++ .../src/task-detail/taskCreationSaga.test.ts | 1 + .../core/src/task-detail/taskCreationSaga.ts | 10 +++- .../environments/EnvironmentSelector.test.tsx | 28 +++++++++ .../environments/EnvironmentSelector.tsx | 8 +-- .../task-detail/taskCreationHostImpl.test.ts | 58 +++++++++++++++++++ .../task-detail/taskCreationHostImpl.ts | 17 ++++++ 7 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 packages/ui/src/features/environments/EnvironmentSelector.test.tsx create mode 100644 packages/ui/src/features/task-detail/taskCreationHostImpl.test.ts diff --git a/packages/core/src/task-detail/taskCreationHost.ts b/packages/core/src/task-detail/taskCreationHost.ts index 92aefe9d1c..2d29814552 100644 --- a/packages/core/src/task-detail/taskCreationHost.ts +++ b/packages/core/src/task-detail/taskCreationHost.ts @@ -128,6 +128,12 @@ export interface ITaskCreationHost { ): Promise; setProvisioningActive(taskId: string): void; clearProvisioning(taskId: string): void; + confirmEnvironmentSetup(args: { + repoPath: string; + environmentId: string; + name: string; + script: string; + }): Promise; dispatchSetupAction(args: SetupActionDispatch): void; track(event: string, props?: Record): void; importClaudeCliSession(args: { diff --git a/packages/core/src/task-detail/taskCreationSaga.test.ts b/packages/core/src/task-detail/taskCreationSaga.test.ts index 5558ab6580..edc3f8bca2 100644 --- a/packages/core/src/task-detail/taskCreationSaga.test.ts +++ b/packages/core/src/task-detail/taskCreationSaga.test.ts @@ -29,6 +29,7 @@ const mockHost = vi.hoisted(() => ({ uploadRunAttachments: vi.fn(), setProvisioningActive: vi.fn(), clearProvisioning: vi.fn(), + confirmEnvironmentSetup: vi.fn(async () => true), dispatchSetupAction: vi.fn(), importClaudeCliSession: vi.fn(), deleteClaudeCliImport: vi.fn(), diff --git a/packages/core/src/task-detail/taskCreationSaga.ts b/packages/core/src/task-detail/taskCreationSaga.ts index 2e3d0c5bad..f7648b44f6 100644 --- a/packages/core/src/task-detail/taskCreationSaga.ts +++ b/packages/core/src/task-detail/taskCreationSaga.ts @@ -637,9 +637,17 @@ export class TaskCreationSaga extends Saga< ): void { this.deps.host .getEnvironment({ repoPath, id: environmentId }) - .then((env) => { + .then(async (env) => { if (!env?.setup?.script) return; + const approved = await this.deps.host.confirmEnvironmentSetup({ + repoPath, + environmentId, + name: env.name, + script: env.setup.script, + }); + if (!approved) return; + this.deps.host.dispatchSetupAction({ taskId, command: env.setup.script, diff --git a/packages/ui/src/features/environments/EnvironmentSelector.test.tsx b/packages/ui/src/features/environments/EnvironmentSelector.test.tsx new file mode 100644 index 0000000000..279aa8d5cf --- /dev/null +++ b/packages/ui/src/features/environments/EnvironmentSelector.test.tsx @@ -0,0 +1,28 @@ +import { render } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +const useEnvironments = vi.fn(); + +vi.mock("./useEnvironments", () => ({ + useEnvironments: (repoPath: string | null) => useEnvironments(repoPath), +})); + +import { EnvironmentSelector } from "./EnvironmentSelector"; + +describe("EnvironmentSelector", () => { + it("never selects a repo-provided environment on its own", () => { + useEnvironments.mockReturnValue({ + data: [ + { id: "env-1", name: "Malicious" }, + { id: "env-2", name: "Other" }, + ], + }); + const onChange = vi.fn(); + + render( + , + ); + + expect(onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/ui/src/features/environments/EnvironmentSelector.tsx b/packages/ui/src/features/environments/EnvironmentSelector.tsx index 8f61735daa..db1fcc5a3f 100644 --- a/packages/ui/src/features/environments/EnvironmentSelector.tsx +++ b/packages/ui/src/features/environments/EnvironmentSelector.tsx @@ -10,7 +10,7 @@ import { ComboboxListFooter, ComboboxTrigger, } from "@posthog/quill"; -import { useEffect, useRef, useState } from "react"; +import { useRef, useState } from "react"; import { useEnvironments } from "./useEnvironments"; interface EnvironmentSelectorProps { @@ -35,11 +35,7 @@ export function EnvironmentSelector({ const { data: environments = [] } = useEnvironments(repoPath); - useEffect(() => { - if (value === null && environments.length > 0) { - onChange(environments[0].id); - } - }, [value, environments, onChange]); + // Never auto-select: environments are repo-provided and run setup scripts. const selectedEnvironment = environments.find((env) => env.id === value); const displayText = selectedEnvironment?.name ?? "No environment"; diff --git a/packages/ui/src/features/task-detail/taskCreationHostImpl.test.ts b/packages/ui/src/features/task-detail/taskCreationHostImpl.test.ts new file mode 100644 index 0000000000..8916cf2274 --- /dev/null +++ b/packages/ui/src/features/task-detail/taskCreationHostImpl.test.ts @@ -0,0 +1,58 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@posthog/di/container", () => ({ + resolveService: vi.fn(), +})); + +vi.mock("../../shell/analytics", () => ({ + track: vi.fn(), + captureException: vi.fn(), +})); + +import { TrpcTaskCreationHost } from "./taskCreationHostImpl"; + +const args = { + repoPath: "/repo", + environmentId: "env-1", + name: "Dev", + script: "npm run setup", +}; + +describe("TrpcTaskCreationHost.confirmEnvironmentSetup", () => { + const host = new TrpcTaskCreationHost(); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it.each([ + { answer: true, expected: true }, + { answer: false, expected: false }, + ])( + "returns $expected when the user answers $answer", + async ({ answer, expected }) => { + vi.spyOn(window, "confirm").mockReturnValue(answer); + + await expect(host.confirmEnvironmentSetup(args)).resolves.toBe(expected); + }, + ); + + it("prompts on every run so an earlier answer cannot be reused", async () => { + const confirmSpy = vi.spyOn(window, "confirm").mockReturnValue(true); + + await host.confirmEnvironmentSetup(args); + await host.confirmEnvironmentSetup(args); + + expect(confirmSpy).toHaveBeenCalledTimes(2); + }); + + it("warns that the script can execute other files in the repository", async () => { + const confirmSpy = vi.spyOn(window, "confirm").mockReturnValue(false); + + await host.confirmEnvironmentSetup(args); + + expect(confirmSpy.mock.calls[0][0]).toContain( + "can execute other files in the repository", + ); + }); +}); diff --git a/packages/ui/src/features/task-detail/taskCreationHostImpl.ts b/packages/ui/src/features/task-detail/taskCreationHostImpl.ts index e4d99882e7..6862976fe6 100644 --- a/packages/ui/src/features/task-detail/taskCreationHostImpl.ts +++ b/packages/ui/src/features/task-detail/taskCreationHostImpl.ts @@ -194,6 +194,23 @@ export class TrpcTaskCreationHost implements ITaskCreationHost { useProvisioningStore.getState().clear(taskId); } + async confirmEnvironmentSetup(args: { + repoPath: string; + environmentId: string; + name: string; + script: string; + }): Promise { + // Asked every run, never remembered: the script text does not determine + // what the command does, so a stored approval for "npm run setup" would + // still hold after the repo changes what that runs. + return window.confirm( + `The environment "${args.name}" in ${args.repoPath} wants to run this ` + + `setup script on your machine:\n\n${args.script}\n\n` + + `It runs with your permissions and can execute other files in the ` + + `repository. Run it?`, + ); + } + dispatchSetupAction(args: SetupActionDispatch): void { const actionId = `setup-${args.taskId}-${Date.now()}`; usePanelLayoutStore