fix: enforce work product external url policy
This commit is contained in:
1 parent
c779fc2858
commit
6c757f2081
7 files changed
+456
-30
No files matched your search
@@ -0,0 +1,211 @@
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createAgentToken, initializeAgentTokenSigningKey } from "../../src/main/agent-auth";
|
||||
import { createApiServer } from "../../src/main/api-server";
|
||||
import { AppDatabase } from "../../src/main/database";
|
||||
import { createTestApiServerCallbacks } from "../support/api-server-callbacks";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
async function createTempDir(prefix: string) {
|
||||
const dir = await mkdtemp(join(tmpdir(), prefix));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
async function createHarness() {
|
||||
const profileDir = await createTempDir("agentcompany-work-product-url-");
|
||||
initializeAgentTokenSigningKey(profileDir);
|
||||
const db = new AppDatabase(profileDir);
|
||||
db.init();
|
||||
db.core.connectors.updateConnectorHealth({
|
||||
id: "openai-codex",
|
||||
status: "ready",
|
||||
authState: "authenticated",
|
||||
version: "test",
|
||||
lastError: null,
|
||||
});
|
||||
|
||||
const companyId = db.core.company.saveCompany({
|
||||
name: "URL Policy Co",
|
||||
description: "Tests work product URL trust policy",
|
||||
status: "active",
|
||||
});
|
||||
const workspaceId = db.core.workspaces.saveWorkspace({
|
||||
companyId,
|
||||
name: "Primary",
|
||||
localPath: profileDir,
|
||||
repoUrl: "",
|
||||
repoRef: "",
|
||||
isPrimary: true,
|
||||
});
|
||||
const agentId = db.core.agents.saveAgent({
|
||||
companyId,
|
||||
name: "Reporter",
|
||||
role: "Researcher",
|
||||
title: "Reporter",
|
||||
status: "active",
|
||||
reportsTo: null,
|
||||
connectorId: "openai-codex",
|
||||
workspaceId,
|
||||
model: null,
|
||||
capabilities: "Writes reports",
|
||||
budgetMonthlyUsd: 100,
|
||||
});
|
||||
const taskId = db.core.tasks.saveTask({
|
||||
companyId,
|
||||
title: "Publish external report",
|
||||
description: "Attach a report link.",
|
||||
assigneeAgentId: agentId,
|
||||
workspaceId,
|
||||
priority: "medium",
|
||||
status: "in_progress",
|
||||
});
|
||||
const run = db.core.runs.createRun({
|
||||
companyId,
|
||||
taskId,
|
||||
agentId,
|
||||
workspaceId,
|
||||
connectorId: "openai-codex",
|
||||
});
|
||||
const token = createAgentToken(agentId, companyId, run.id);
|
||||
const server = createApiServer(db, createTestApiServerCallbacks());
|
||||
const port = await server.start();
|
||||
|
||||
return {
|
||||
db,
|
||||
server,
|
||||
companyId,
|
||||
taskId,
|
||||
baseUrl: `http://127.0.0.1:${port}`,
|
||||
headers: {
|
||||
Authorization: `Bearer ${token}`,
|
||||
"X-Agent-Company-Run-Id": run.id,
|
||||
"Content-Type": "application/json; charset=utf-8",
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("work product URL API policy", () => {
|
||||
it("rejects blank URLs during work product creation", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
const response = await fetch(`${harness.baseUrl}/api/issues/${harness.taskId}/work-products`, {
|
||||
method: "POST",
|
||||
headers: harness.headers,
|
||||
body: JSON.stringify({
|
||||
type: "link",
|
||||
provider: "external",
|
||||
title: "Blank URL",
|
||||
url: " ",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(harness.db.queryAll("select id from work_products")).toHaveLength(0);
|
||||
} finally {
|
||||
harness.server.stop();
|
||||
harness.db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects custom schemes during work product creation", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
const response = await fetch(`${harness.baseUrl}/api/issues/${harness.taskId}/work-products`, {
|
||||
method: "POST",
|
||||
headers: harness.headers,
|
||||
body: JSON.stringify({
|
||||
type: "link",
|
||||
provider: "external",
|
||||
title: "Local scheme",
|
||||
url: "vscode://file/etc/passwd",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(harness.db.queryAll("select id from work_products")).toHaveLength(0);
|
||||
} finally {
|
||||
harness.server.stop();
|
||||
harness.db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects credential-bearing URLs during work product creation", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
const response = await fetch(`${harness.baseUrl}/api/issues/${harness.taskId}/work-products`, {
|
||||
method: "POST",
|
||||
headers: harness.headers,
|
||||
body: JSON.stringify({
|
||||
type: "link",
|
||||
provider: "external",
|
||||
title: "Credential URL",
|
||||
url: "https://user:pass@example.com/reports/q1",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
expect(harness.db.queryAll("select id from work_products")).toHaveLength(0);
|
||||
} finally {
|
||||
harness.server.stop();
|
||||
harness.db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("stores http links for later confirmation during open", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
const response = await fetch(`${harness.baseUrl}/api/issues/${harness.taskId}/work-products`, {
|
||||
method: "POST",
|
||||
headers: harness.headers,
|
||||
body: JSON.stringify({
|
||||
type: "link",
|
||||
provider: "external",
|
||||
title: "External report",
|
||||
url: " https://example.com/reports/q1 ",
|
||||
}),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(201);
|
||||
const row = harness.db.queryAll<{ url: string }>("select url from work_products limit 1")[0];
|
||||
expect(row?.url).toBe("https://example.com/reports/q1");
|
||||
} finally {
|
||||
harness.server.stop();
|
||||
harness.db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects custom schemes during work product updates", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
const id = harness.db.core.documents.createWorkProduct({
|
||||
companyId: harness.companyId,
|
||||
taskId: harness.taskId,
|
||||
type: "link",
|
||||
provider: "external",
|
||||
title: "External report",
|
||||
url: "https://example.com/reports/q1",
|
||||
});
|
||||
|
||||
const response = await fetch(`${harness.baseUrl}/api/work-products/${id}`, {
|
||||
method: "PATCH",
|
||||
headers: harness.headers,
|
||||
body: JSON.stringify({ url: "file:///etc/passwd" }),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
const row = harness.db.queryAll<{ url: string }>("select url from work_products where id = ? limit 1", id)[0];
|
||||
expect(row?.url).toBe("https://example.com/reports/q1");
|
||||
} finally {
|
||||
harness.server.stop();
|
||||
harness.db.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,22 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
classifyExternalUrl,
|
||||
normalizeExternalHttpUrl,
|
||||
} from "../../src/main/external-url-policy";
|
||||
|
||||
describe("external URL policy", () => {
|
||||
it("normalizes http and https URLs", () => {
|
||||
expect(normalizeExternalHttpUrl(" https://github.com/agentcompany ")).toBe("https://github.com/agentcompany");
|
||||
expect(normalizeExternalHttpUrl("http://example.com/path?q=1")).toBe("http://example.com/path?q=1");
|
||||
});
|
||||
|
||||
it("blocks custom schemes and credential-bearing URLs", () => {
|
||||
expect(classifyExternalUrl("vscode://file/etc/passwd")).toMatchObject({ kind: "blocked" });
|
||||
expect(classifyExternalUrl("https://user:pass@example.com/report")).toMatchObject({ kind: "blocked" });
|
||||
});
|
||||
|
||||
it("separates trusted hosts from confirmation hosts", () => {
|
||||
expect(classifyExternalUrl("https://github.com/acme/repo")).toMatchObject({ kind: "trusted", host: "github.com" });
|
||||
expect(classifyExternalUrl("https://example.com/report")).toMatchObject({ kind: "confirm", host: "example.com" });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,122 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { IPC_CHANNELS } from "../../src/shared/contracts";
|
||||
|
||||
const electron = vi.hoisted(() => ({
|
||||
openExternal: vi.fn<() => Promise<void>>(async () => {}),
|
||||
openPath: vi.fn<() => Promise<string>>(async () => ""),
|
||||
showMessageBox: vi.fn<() => Promise<{ response: number }>>(async () => ({ response: 0 })),
|
||||
}));
|
||||
|
||||
vi.mock("electron", () => ({
|
||||
dialog: { showMessageBox: electron.showMessageBox },
|
||||
nativeTheme: { themeSource: "system" },
|
||||
shell: {
|
||||
openExternal: electron.openExternal,
|
||||
openPath: electron.openPath,
|
||||
},
|
||||
}));
|
||||
|
||||
import { registerCoreReadHandlers } from "../../src/main/ipc-core-read-handlers";
|
||||
import type { AppDatabase } from "../../src/main/database";
|
||||
import type { RegisterCoreHandlersDependencies } from "../../src/main/ipc-core-common";
|
||||
|
||||
const companyId = "11111111-1111-4111-8111-111111111111";
|
||||
const workProductId = "22222222-2222-4222-8222-222222222222";
|
||||
|
||||
function registerOpenWorkProductHandler(url: string) {
|
||||
const handlers = new Map<string, (...args: unknown[]) => Promise<unknown> | unknown>();
|
||||
const db = {
|
||||
core: {
|
||||
documents: {
|
||||
resolveWorkProductTarget: vi.fn(() => ({ kind: "url", value: url })),
|
||||
updateWorkProduct: vi.fn(),
|
||||
},
|
||||
settings: {
|
||||
canOpenPath: vi.fn(() => true),
|
||||
},
|
||||
},
|
||||
projection: {
|
||||
snapshot: {
|
||||
listRendererSnapshot: vi.fn(() => ({})),
|
||||
},
|
||||
},
|
||||
} as unknown as AppDatabase;
|
||||
const deps = {
|
||||
registerHandle: (channel, handler) => handlers.set(channel, handler as (...args: unknown[]) => Promise<unknown> | unknown),
|
||||
db,
|
||||
ensureCompanyExists: () => null,
|
||||
publishDomainChanged: vi.fn(),
|
||||
checkForUpdates: vi.fn(async () => true),
|
||||
resetAfterRestore: vi.fn(),
|
||||
listConnectorModels: vi.fn(async () => []),
|
||||
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
|
||||
} satisfies Pick<
|
||||
RegisterCoreHandlersDependencies,
|
||||
| "registerHandle"
|
||||
| "db"
|
||||
| "ensureCompanyExists"
|
||||
| "publishDomainChanged"
|
||||
| "checkForUpdates"
|
||||
| "resetAfterRestore"
|
||||
| "listConnectorModels"
|
||||
| "logger"
|
||||
>;
|
||||
registerCoreReadHandlers(deps);
|
||||
const handler = handlers.get(IPC_CHANNELS.openWorkProduct);
|
||||
if (!handler) {
|
||||
throw new Error("openWorkProduct handler was not registered.");
|
||||
}
|
||||
return handler;
|
||||
}
|
||||
|
||||
async function openUrl(url: string) {
|
||||
const handler = registerOpenWorkProductHandler(url);
|
||||
return handler({}, { companyId, id: workProductId });
|
||||
}
|
||||
|
||||
describe("openWorkProduct external URL policy", () => {
|
||||
it("opens trusted hosts without confirmation", async () => {
|
||||
electron.openExternal.mockClear();
|
||||
electron.showMessageBox.mockClear();
|
||||
|
||||
const result = await openUrl("https://github.com/acme/repo");
|
||||
|
||||
expect(result).toEqual({ ok: true, data: true });
|
||||
expect(electron.showMessageBox).not.toHaveBeenCalled();
|
||||
expect(electron.openExternal).toHaveBeenCalledWith("https://github.com/acme/repo");
|
||||
});
|
||||
|
||||
it("asks before opening untrusted http hosts", async () => {
|
||||
electron.openExternal.mockClear();
|
||||
electron.showMessageBox.mockClear();
|
||||
electron.showMessageBox.mockResolvedValueOnce({ response: 0 });
|
||||
|
||||
const result = await openUrl("https://example.com/report");
|
||||
|
||||
expect(result).toEqual({ ok: true, data: true });
|
||||
expect(electron.showMessageBox).toHaveBeenCalledTimes(1);
|
||||
expect(electron.openExternal).toHaveBeenCalledWith("https://example.com/report");
|
||||
});
|
||||
|
||||
it("blocks custom schemes before shell handoff", async () => {
|
||||
electron.openExternal.mockClear();
|
||||
electron.showMessageBox.mockClear();
|
||||
|
||||
const result = await openUrl("vscode://file/etc/passwd");
|
||||
|
||||
expect(result).toMatchObject({ ok: false, error: { code: "OPEN_EXTERNAL_URL_FORBIDDEN" } });
|
||||
expect(electron.showMessageBox).not.toHaveBeenCalled();
|
||||
expect(electron.openExternal).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks credential-bearing URLs before shell handoff", async () => {
|
||||
electron.openExternal.mockClear();
|
||||
electron.showMessageBox.mockClear();
|
||||
|
||||
const result = await openUrl("https://user:pass@example.com/report");
|
||||
|
||||
expect(result).toMatchObject({ ok: false, error: { code: "OPEN_EXTERNAL_URL_FORBIDDEN" } });
|
||||
expect(electron.showMessageBox).not.toHaveBeenCalled();
|
||||
expect(electron.openExternal).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user