Skip to content
Merged
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
6 changes: 6 additions & 0 deletions packages/core/src/task-detail/taskCreationHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ export interface ITaskCreationHost {
): Promise<string[]>;
setProvisioningActive(taskId: string): void;
clearProvisioning(taskId: string): void;
confirmEnvironmentSetup(args: {
repoPath: string;
environmentId: string;
name: string;
script: string;
}): Promise<boolean>;
dispatchSetupAction(args: SetupActionDispatch): void;
track(event: string, props?: Record<string, unknown>): void;
importClaudeCliSession(args: {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/task-detail/taskCreationSaga.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/task-detail/taskCreationSaga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 28 additions & 0 deletions packages/ui/src/features/environments/EnvironmentSelector.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<EnvironmentSelector repoPath="/repo" value={null} onChange={onChange} />,
);

expect(onChange).not.toHaveBeenCalled();
});
});
8 changes: 2 additions & 6 deletions packages/ui/src/features/environments/EnvironmentSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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";
Expand Down
58 changes: 58 additions & 0 deletions packages/ui/src/features/task-detail/taskCreationHostImpl.test.ts
Original file line number Diff line number Diff line change
@@ -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",
);
});
});
17 changes: 17 additions & 0 deletions packages/ui/src/features/task-detail/taskCreationHostImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
// 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
Expand Down
Loading