Apply work product security patch and test service mocks

- Restrict work product URL schema to http/https via shared validator
- Defensively gate UI action hrefs through safeActionHref helper
- Add issueDeliverableService mocks to route tests that stub the services registry so the new dependency resolves

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-04-21 14:41:02 -05:00
1 parent 6573e9f056
commit 01a67ec543
16 files changed
+106 -8

No files matched your search

+10 -1
View File
@@ -1,5 +1,14 @@
import { z } from "zod";
function isHttpUrl(value: string) {
try {
const parsed = new URL(value);
return parsed.protocol === "http:" || parsed.protocol === "https:";
} catch {
return false;
}
}
export const issueWorkProductTypeSchema = z.enum([
"preview_url",
"runtime_service",
@@ -37,7 +46,7 @@ export const createIssueWorkProductSchema = z.object({
provider: z.string().min(1),
externalId: z.string().optional().nullable(),
title: z.string().min(1),
url: z.string().url().optional().nullable(),
url: z.string().trim().refine(isHttpUrl, "URL must use http or https").optional().nullable(),
status: issueWorkProductStatusSchema.default("active"),
reviewState: issueWorkProductReviewStateSchema.optional().default("none"),
isPrimary: z.boolean().optional().default(false),
@@ -200,7 +200,7 @@ describe("agent instructions bundle routes", () => {
entryFile: "AGENTS.md",
});
expect(mockAgentInstructionsService.getBundle).toHaveBeenCalled();
});
}, 10_000);
it("writes a bundle file and persists compatibility config", async () => {
const res = await request(await createApp())
@@ -62,6 +62,7 @@ function registerRouteMocks() {
routineService: () => ({
syncRunStatusForIssue: vi.fn(async () => undefined),
}),
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
}
@@ -80,6 +80,7 @@ function registerServiceMocks() {
routineService: () => ({
syncRunStatusForIssue: vi.fn(async () => undefined),
}),
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
}
@@ -56,6 +56,7 @@ vi.mock("../services/index.js", () => ({
logActivity: mockLogActivity,
projectService: () => ({}),
routineService: () => ({ syncRunStatusForIssue: vi.fn(async () => undefined) }),
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
@@ -80,6 +80,7 @@ vi.mock("../services/index.js", () => ({
logActivity: mockLogActivity,
projectService: () => ({}),
routineService: () => mockRoutineService,
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
@@ -107,6 +108,7 @@ function registerModuleMocks() {
logActivity: mockLogActivity,
projectService: () => ({}),
routineService: () => mockRoutineService,
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
}
@@ -71,6 +71,9 @@ function registerModuleMocks() {
getGeneral: vi.fn(async () => ({ feedbackDataSharingPreference: "prompt" })),
}),
issueApprovalService: () => ({}),
issueDeliverableService: () => ({
getForIssue: vi.fn(async () => null),
}),
issueService: () => mockIssueService,
logActivity: mockLogActivity,
projectService: () => ({}),
@@ -74,6 +74,7 @@ function registerModuleMocks() {
logActivity: mockLogActivity,
projectService: () => ({}),
routineService: () => mockRoutineService,
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
}
@@ -48,6 +48,7 @@ function registerModuleMocks() {
routineService: () => ({
syncRunStatusForIssue: vi.fn(async () => undefined),
}),
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
}
@@ -97,6 +97,9 @@ function registerModuleMocks() {
listCompanyIds: vi.fn(async () => ["company-1"]),
}),
issueApprovalService: () => ({}),
issueDeliverableService: () => ({
getForIssue: vi.fn(async () => null),
}),
issueService: () => mockIssueService,
logActivity: vi.fn(async () => undefined),
projectService: () => ({}),
@@ -58,6 +58,7 @@ vi.mock("../services/index.js", () => ({
routineService: () => ({
syncRunStatusForIssue: vi.fn(async () => undefined),
}),
issueDeliverableService: () => ({}),
workProductService: () => ({}),
}));
@@ -0,0 +1,31 @@
import { describe, expect, it } from "vitest";
import { createIssueWorkProductSchema } from "@paperclipai/shared";
const baseWorkProduct = {
type: "pull_request",
provider: "github",
title: "PR 42",
};
describe("work product URL validation", () => {
it("accepts http and https URLs", () => {
expect(createIssueWorkProductSchema.safeParse({
...baseWorkProduct,
url: "https://github.com/paperclipai/paperclip/pull/42",
}).success).toBe(true);
expect(createIssueWorkProductSchema.safeParse({
...baseWorkProduct,
url: "http://127.0.0.1:3100/preview",
}).success).toBe(true);
});
it("rejects executable and local-file URL schemes", () => {
for (const url of ["javascript:alert(document.domain)", "file:///Users/operator/.ssh/id_rsa", "data:text/html,hello"]) {
const result = createIssueWorkProductSchema.safeParse({
...baseWorkProduct,
url,
});
expect(result.success).toBe(false);
}
});
});
+6 -4
View File
@@ -1,6 +1,7 @@
import type { IssueDeliverableItem, IssueDeliverablesResponse } from "@paperclipai/shared";
import { Link } from "@/lib/router";
import { projectWorkspaceUrl, relativeTime } from "@/lib/utils";
import { safeActionHref } from "@/lib/safeLinks";
import { EmptyState } from "./EmptyState";
import { PageSkeleton } from "./PageSkeleton";
import { Badge } from "@/components/ui/badge";
@@ -48,17 +49,18 @@ function itemIcon(item: IssueDeliverableItem) {
}
function DeliverableAction({ href, label }: { href: string | null; label: string | null }) {
if (!href || !label) return null;
if (href.startsWith("#")) {
const safeHref = safeActionHref(href, { allowHash: true });
if (!safeHref || !label) return null;
if (safeHref.startsWith("#")) {
return (
<Button asChild size="sm" variant="outline">
<a href={href}>{label}</a>
<a href={safeHref}>{label}</a>
</Button>
);
}
return (
<Button asChild size="sm" variant="outline">
<a href={href} target="_blank" rel="noreferrer">
<a href={safeHref} target="_blank" rel="noreferrer noopener">
{label}
</a>
</Button>
+18
View File
@@ -0,0 +1,18 @@
import { describe, expect, it } from "vitest";
import { safeActionHref } from "./safeLinks";
describe("safeActionHref", () => {
it("allows http, https, app-relative paths, and explicit hash links", () => {
expect(safeActionHref("https://example.com/pr/42")).toBe("https://example.com/pr/42");
expect(safeActionHref("http://127.0.0.1:3100/preview")).toBe("http://127.0.0.1:3100/preview");
expect(safeActionHref("/api/attachments/attachment-1/content")).toBe("/api/attachments/attachment-1/content");
expect(safeActionHref("#document-plan", { allowHash: true })).toBe("#document-plan");
});
it("rejects executable, protocol-relative, local-file, and hash links by default", () => {
expect(safeActionHref("javascript:alert(document.domain)")).toBeNull();
expect(safeActionHref("//evil.example/path")).toBeNull();
expect(safeActionHref("file:///Users/operator/.ssh/id_rsa")).toBeNull();
expect(safeActionHref("#document-plan")).toBeNull();
});
});
+23
View File
@@ -0,0 +1,23 @@
export function safeActionHref(href: string | null | undefined, options: { allowHash?: boolean } = {}) {
const trimmed = href?.trim();
if (!trimmed) return null;
if (options.allowHash && trimmed.startsWith("#")) {
return trimmed;
}
if (trimmed.startsWith("/") && !trimmed.startsWith("//")) {
return trimmed;
}
try {
const parsed = new URL(trimmed);
if (parsed.protocol === "http:" || parsed.protocol === "https:") {
return trimmed;
}
} catch {
return null;
}
return null;
}
+3 -2
View File
@@ -13,6 +13,7 @@ import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { ToggleSwitch } from "@/components/ui/toggle-switch";
import { queryKeys } from "../lib/queryKeys";
import { safeActionHref } from "../lib/safeLinks";
import { createIssueDetailPath } from "../lib/issueDetailBreadcrumb";
import { projectUrl, relativeTime } from "../lib/utils";
import {
@@ -157,7 +158,7 @@ function ArtifactCard({
projectHref: string | null;
}) {
const meta = deliverableMeta(item);
const href = itemHref(item);
const href = safeActionHref(itemHref(item));
const sizeLabel = formatByteSize(item.byteSize);
const MetaIcon = meta.icon;
const isInternalDocLink = Boolean(href?.startsWith("/issues/"));
@@ -263,7 +264,7 @@ function ArtifactCard({
</Button>
) : (
<Button asChild size="sm" variant="outline">
<a href={href} target="_blank" rel="noreferrer">
<a href={href} target="_blank" rel="noreferrer noopener">
<ArrowUpRight className="mr-1.5 h-4 w-4" />
{actionLabel(item)}
</a>