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
13 changes: 13 additions & 0 deletions DEVLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@

> Append-only session log. Read at session start. Update at session end.

## 2026-06-18 — Safe URL scheme handling for rich content links

Fixed AXI-85 by centralizing renderable/external URL validation and applying it to rich markdown links, link attachments, and existing link-attachment open/preview surfaces.

Fixes:
- Added `src/lib/url-safety.ts` to allow only `http:`/`https:` external URLs and intentional internal app paths beginning with `/` for rendered markdown navigation.
- Updated `MarkdownWithAttachments` so unsafe markdown link schemes such as `javascript:` and `data:` render as inert text, while `https://…`, internal `/w/...` links, and existing http(s)-only forge-link chips continue to work.
- Enforced http(s)-only link attachment URLs in the tRPC attachment router, MCP `attachments.attachLink`, and storage helper; also guarded existing link attachment chip/lightbox/canvas open and iframe-preview paths against legacy unsafe values.
- Added regression tests for shared URL safety, markdown rendering behavior, tRPC attachLink rejection, and MCP attachLink rejection.

Verify:
`env -u OPENAI_API_KEY pnpm exec vitest run tests/unit/url-safety.test.ts tests/unit/markdown-url-safety.test.ts src/server/services/__tests__/mcp.test.ts src/server/routers/__tests__/attachment.test.ts --reporter=verbose --testNamePattern "url safety|MarkdownWithAttachments URL safety|attachments.attachLink accepts only http\\(s\\) external URLs|rejects non-http"`, `pnpm typecheck`, `pnpm lint`, `env -u OPENAI_API_KEY pnpm test`.

## 2026-06-18 — Quick-access Mission Control and bounded Command Center

Refined the floating activity dock after the live Command Center screenshots
Expand Down
9 changes: 6 additions & 3 deletions src/app/(app)/w/[slug]/canvas/[canvasId]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {
type InspectorSelection,
} from "@/components/canvas/canvas-selection-inspector";
import { computeSnap } from "@/lib/canvas-snap-guides";
import { isSafeExternalUrl } from "@/lib/url-safety";
import { useCanvasUndoStack } from "@/lib/canvas-undo";
import {
CanvasEntityCreator,
Expand Down Expand Up @@ -6014,7 +6015,9 @@ function AttachmentPreviewCardBody({ node }: { node: HydratedNode }) {
},
);
const previewKind = canvasKindForAttachment(meta);
const url = isLink ? meta.externalUrl ?? null : dl.data?.url ?? null;
const safeExternalUrl =
meta.externalUrl && isSafeExternalUrl(meta.externalUrl) ? meta.externalUrl : null;
const url = isLink ? safeExternalUrl : dl.data?.url ?? null;
// Header — filename + mime chip. Padding mirrors the legacy card body.
return (
<div className="flex min-w-0 flex-col gap-1.5">
Expand All @@ -6039,8 +6042,8 @@ function AttachmentPreviewCardBody({ node }: { node: HydratedNode }) {
height={Math.max(120, node.height - 60)}
onExpand={() => {
if (isLink) {
if (meta.externalUrl) {
window.open(meta.externalUrl, "_blank", "noopener,noreferrer");
if (safeExternalUrl) {
window.open(safeExternalUrl, "_blank", "noopener,noreferrer");
}
return;
}
Expand Down
7 changes: 4 additions & 3 deletions src/components/attachments/attachment-chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from "lucide-react";
import { trpc } from "@/lib/trpc";
import { cn } from "@/lib/utils";
import { isSafeExternalUrl } from "@/lib/url-safety";
import { useAttachmentLightbox } from "@/components/attachments/attachment-lightbox";

/**
Expand All @@ -32,7 +33,7 @@ export function LinkFavicon({
let origin = "";
let host = "";
try {
if (url) {
if (url && isSafeExternalUrl(url)) {
const parsed = new URL(url);
origin = parsed.origin;
host = parsed.hostname.replace(/^www\./i, "").toLowerCase();
Expand Down Expand Up @@ -110,7 +111,7 @@ function isLinkAttachment(a: AttachmentChipData): boolean {

/** Best-effort hostname extraction for LINK chips' trailing slot. */
function safeHostname(url: string | null | undefined): string {
if (!url) return "";
if (!url || !isSafeExternalUrl(url)) return "";
try {
return new URL(url).hostname.replace(/^www\./i, "");
} catch {
Expand Down Expand Up @@ -142,7 +143,7 @@ export function AttachmentChip({
// is "open the page in a new tab". Lightbox would render an
// iframe-via-presigned-url that doesn't apply here.
const href = attachment.externalUrl ?? "";
if (href) {
if (href && isSafeExternalUrl(href)) {
window.open(href, "_blank", "noopener,noreferrer");
}
return;
Expand Down
12 changes: 10 additions & 2 deletions src/components/attachments/attachment-lightbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { useModalBehavior } from "@/components/ui/modal/use-modal-behavior";
import { trpc } from "@/lib/trpc";
import { cn } from "@/lib/utils";
import { MOTION } from "@/lib/motion";
import { isSafeExternalUrl } from "@/lib/url-safety";
import { LinkFavicon } from "@/components/attachments/attachment-chip";

/**
Expand Down Expand Up @@ -192,7 +193,11 @@ function Lightbox({
retry: false,
},
);
const url = isLink ? (current?.externalUrl ?? "") : data?.url;
const url = isLink
? current?.externalUrl && isSafeExternalUrl(current.externalUrl)
? current.externalUrl
: ""
: data?.url;

// Delete plumbing — confirm modal stacks on top of the lightbox.
const [confirmOpen, setConfirmOpen] = React.useState(false);
Expand Down Expand Up @@ -499,7 +504,10 @@ function Preview({
}

function LinkPreview({ attachment }: { attachment: AttachmentLite }) {
const href = attachment.externalUrl ?? "";
const href =
attachment.externalUrl && isSafeExternalUrl(attachment.externalUrl)
? attachment.externalUrl
: "";
let host = "";
try {
if (href) host = new URL(href).hostname.replace(/^www\./i, "");
Expand Down
5 changes: 3 additions & 2 deletions src/components/attachments/issue-attachments-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { Button } from "@/components/ui/button";
import { Confirm } from "@/components/ui/modal";
import { trpc } from "@/lib/trpc";
import { cn } from "@/lib/utils";
import { isSafeExternalUrl } from "@/lib/url-safety";
import { useWorkspace } from "@/hooks/use-workspace";
import { useAttachmentLightbox } from "@/components/attachments/attachment-lightbox";
import { LinkFavicon } from "@/components/attachments/attachment-chip";
Expand Down Expand Up @@ -494,7 +495,7 @@ function AttachmentTile({
const openTile = () => {
if (isLink) {
const href = attachment.externalUrl ?? "";
if (href) window.open(href, "_blank", "noopener,noreferrer");
if (href && isSafeExternalUrl(href)) window.open(href, "_blank", "noopener,noreferrer");
return;
}
lightbox.open({
Expand Down Expand Up @@ -633,7 +634,7 @@ function isPdf(mime: string): boolean {
}

function linkAttachmentMeta(url: string | null): { host: string; path: string } {
if (!url) return { host: "", path: "" };
if (!url || !isSafeExternalUrl(url)) return { host: "", path: "" };
try {
const parsed = new URL(url);
const host = parsed.hostname.replace(/^www\./i, "");
Expand Down
37 changes: 18 additions & 19 deletions src/components/markdown/attachment-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { YouTubeEmbed } from "@/components/embeds/youtube-embed";
import { GithubEmbed } from "@/components/embeds/github-embed";
import { LoomEmbed } from "@/components/embeds/loom-embed";
import { FigmaEmbed } from "@/components/embeds/figma-embed";
import { isSafeExternalUrl, toRenderableHref } from "@/lib/url-safety";

/**
* Rich body renderer for descriptions, comments, artifacts, and notes.
Expand Down Expand Up @@ -663,49 +664,47 @@ function renderInlineSegs(
inlineList={inlineList}
/>
);
if (s.type === "linkChip")
if (s.type === "linkChip") {
if (!isSafeExternalUrl(s.url)) {
return <Fragment key={key}>{s.label || s.url}</Fragment>;
}
return <InlineForgeLink key={key} label={s.label} url={s.url} />;
}
if (s.type === "issueRef")
return <InlineIssueRef key={key} issueKey={s.key} />;
if (s.type === "mention")
return <InlineAgentMention key={key} profileKey={s.profileKey} />;
if (s.type === "mdLink") {
// External markdown link — open in a new tab. Internal absolute
// paths (e.g. `/w/foo/issues/abc`) get an in-app Link.
if (/^https?:\/\//i.test(s.url)) {
// Explicit allowlist: http(s) links open in a new tab, and internal
// app paths (e.g. `/w/foo/issues/abc`) get an in-app Link. Everything
// else renders as inert text so `javascript:` / `data:` never become
// clickable browser-controlled hrefs.
const renderable = toRenderableHref(s.url);
if (renderable?.kind === "external") {
return (
<a
key={key}
href={s.url}
href={renderable.href}
target="_blank"
rel="noopener noreferrer"
className="text-ember underline-offset-2 hover:underline"
>
{s.label || s.url}
{s.label || renderable.href}
</a>
);
}
if (s.url.startsWith("/")) {
if (renderable?.kind === "internal") {
return (
<Link
key={key}
href={s.url}
href={renderable.href}
className="text-ember underline-offset-2 hover:underline"
>
{s.label || s.url}
{s.label || renderable.href}
</Link>
);
}
// Other schemes (mailto:, etc.) render as plain anchors.
return (
<a
key={key}
href={s.url}
className="text-ember underline-offset-2 hover:underline"
>
{s.label || s.url}
</a>
);
return <Fragment key={key}>{s.label || s.url}</Fragment>;
}
if (s.type === "url")
return (
Expand Down
49 changes: 49 additions & 0 deletions src/lib/url-safety.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
export type RenderableHrefKind = "external" | "internal";

export interface RenderableHref {
href: string;
kind: RenderableHrefKind;
}

const SAFE_EXTERNAL_PROTOCOLS = new Set(["http:", "https:"]);

export function isSafeExternalUrl(value: string): boolean {
const trimmed = value.trim();
if (!trimmed) return false;
try {
const parsed = new URL(trimmed);
return SAFE_EXTERNAL_PROTOCOLS.has(parsed.protocol);
} catch {
return false;
}
}

export function assertSafeExternalUrl(value: string): string {
const trimmed = value.trim();
if (!isSafeExternalUrl(trimmed)) {
throw new Error("External links must use http or https URLs.");
}
return trimmed;
}

export function safeExternalUrlMessage(): string {
return "External links must use http or https URLs.";
}

export function isSafeInternalAppPath(value: string): boolean {
const trimmed = value.trim();
if (!trimmed.startsWith("/")) return false;
// Avoid protocol-relative URLs (`//example.com`) and backslash variants
// that browsers can treat as host-relative navigations.
if (trimmed.startsWith("//") || trimmed.startsWith("/\\")) return false;
if (/^\/%5c/i.test(trimmed)) return false;
return true;
}

export function toRenderableHref(value: string): RenderableHref | null {
const trimmed = value.trim();
if (!trimmed) return null;
if (isSafeInternalAppPath(trimmed)) return { href: trimmed, kind: "internal" };
if (isSafeExternalUrl(trimmed)) return { href: trimmed, kind: "external" };
return null;
}
22 changes: 22 additions & 0 deletions src/server/routers/__tests__/attachment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,28 @@ async function putBytes(url: string, body: Buffer, mimeType: string) {
}

describe("attachmentRouter", () => {
it("rejects non-http(s) link attachment URLs", async () => {
const { caller, fixture } = await setup();
const issue = await createIssue(fixture);

await expect(
caller.attachLink({
targetType: "issue",
targetId: issue.id,
url: "javascript:alert(1)",
title: "Bad",
}),
).rejects.toThrow(/http or https/i);
await expect(
caller.attachLink({
targetType: "issue",
targetId: issue.id,
url: "data:text/html,<h1>x</h1>",
title: "Bad",
}),
).rejects.toThrow(/http or https/i);
});

it("allows chat-message uploads for the owning human thread", async () => {
const { caller, fixture } = await setup();
const prisma = getPrisma();
Expand Down
14 changes: 11 additions & 3 deletions src/server/routers/attachment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import { z } from "zod";
import { TRPCError } from "@trpc/server";
import { EventKind } from "@prisma/client";
import type { PrismaClient } from "@prisma/client";
import {
assertSafeExternalUrl,
isSafeExternalUrl,
safeExternalUrlMessage,
} from "@/lib/url-safety";
import { router, workspaceProcedure, adminProcedure } from "@/server/trpc";
import { recordChange } from "@/server/audit";
import {
Expand Down Expand Up @@ -74,7 +79,9 @@ const finalizeInput = z.object({
const attachLinkInput = z.object({
targetType: targetTypeSchema,
targetId: z.string().cuid(),
url: z.string().url().max(2048),
url: z.string().max(2048).refine(isSafeExternalUrl, {
message: safeExternalUrlMessage(),
}),
title: z.string().max(255).optional(),
});

Expand Down Expand Up @@ -175,13 +182,14 @@ export const attachmentRouter = router({
.input(attachLinkInput)
.mutation(async ({ ctx, input }) => {
await assertTargetInWorkspace(ctx, input.targetType, input.targetId);
const safeUrl = assertSafeExternalUrl(input.url);
// Caller didn't provide a label → try to scrape <title> off the
// target page so the chip is meaningful. Fail soft: any error
// leaves resolvedTitle null and createLinkAttachment falls back
// to hostname.
let resolvedTitle: string | null = input.title ?? null;
if (resolvedTitle === null) {
const meta = await fetchLinkMetadata(input.url);
const meta = await fetchLinkMetadata(safeUrl);
if (meta.title) resolvedTitle = meta.title;
}
let row: Awaited<ReturnType<typeof createLinkAttachment>>;
Expand All @@ -190,7 +198,7 @@ export const attachmentRouter = router({
workspaceId: ctx.workspaceId,
targetType: input.targetType,
targetId: input.targetId,
url: input.url,
url: safeUrl,
title: resolvedTitle,
});
} catch (err) {
Expand Down
47 changes: 47 additions & 0 deletions src/server/services/__tests__/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,53 @@ function envelopeOf<T>(value: unknown): { data: T[]; nextCursor?: string } {
}

describe("mcp tool registry", () => {
it("attachments.attachLink accepts only http(s) external URLs", async () => {
const f = await createWorkspaceFixture({ keyPrefix: "LNK" });
fixtures.push(f);
const issue = await createIssue(f, { title: "link target" });
const { ctx } = buildMcpCtx(f);

const link = (await call(
"attachments.attachLink",
{
targetType: "issue",
targetId: issue.id,
url: "https://example.com/runbook",
title: "Runbook",
},
ctx,
)) as { externalUrl: string; kind: string };
expect(link).toMatchObject({
externalUrl: "https://example.com/runbook",
kind: "LINK",
});

await expect(
call(
"attachments.attachLink",
{
targetType: "issue",
targetId: issue.id,
url: "javascript:alert(1)",
title: "Bad",
},
ctx,
),
).rejects.toThrow(/http or https/i);
await expect(
call(
"attachments.attachLink",
{
targetType: "issue",
targetId: issue.id,
url: "data:text/html,<h1>x</h1>",
title: "Bad",
},
ctx,
),
).rejects.toThrow(/http or https/i);
});

it("registers >= 30 tools spanning the new primitives", () => {
const names = Object.keys(mcpTools);
expect(names.length).toBeGreaterThanOrEqual(30);
Expand Down
Loading
Loading