From 89900c20cfd904d283e783eb2eb9dc79d6134410 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 01:18:23 -0700 Subject: [PATCH 1/5] feat(cards): approve Linear, Sentry and Atlassian links; teach the card rule - Frontend and server share one approved-host rule: exact linear.app and sentry.io, plus one-label *.atlassian.net. HTTPS only, no credentials, no custom port. - commonly_post_message description carries the list-results card rule, with the catalogue tuple read from the package's catalogue file. - Presence test keeps hex and rgb literals out of uiCard.css, so cards follow the v2 dark tokens. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../unit/services/uiCardValidation.test.js | 13 ++++++++++++ backend/services/uiCardValidation.ts | 20 +++++++++++++------ commonly-mcp/__tests__/tools.test.mjs | 8 ++++++++ commonly-mcp/src/tools.js | 2 +- .../v2/__tests__/v2-layout-invariants.test.ts | 9 +++++++++ .../v2/cards/__tests__/uiCardSource.test.ts | 9 +++++++++ frontend/src/v2/cards/cardLinks.ts | 11 ++++++++-- 7 files changed, 63 insertions(+), 9 deletions(-) diff --git a/backend/__tests__/unit/services/uiCardValidation.test.js b/backend/__tests__/unit/services/uiCardValidation.test.js index 5c0468f296..45cbe569ef 100644 --- a/backend/__tests__/unit/services/uiCardValidation.test.js +++ b/backend/__tests__/unit/services/uiCardValidation.test.js @@ -22,6 +22,19 @@ describe('untrusted UI card write validation', () => { it.each(['system', 'approval', null, 1])('refuses non-agent message type %s without a payload', (type) => { expect(() => validateAgentCardPayload(type, undefined, 'hello')).toThrow('text or card'); }); + it('accepts the Linear, Sentry and Atlassian site links the renderer opens', () => { + for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x']) { + expect(() => validate(card(`root = Text("hello")\nunused = Action(@OpenUrl("${url}"))`))).not.toThrow(); + } + }); + it.each([ + 'https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', + 'https://sentry.io.evil.com', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', + ])('refuses the unapproved link %s', (url) => { + expect(() => validate(card(`root = Text("hello")\nunused = Action(@OpenUrl("${url}"))`))).toThrow(expect.objectContaining({ + statusCode: 400, code: 'invalid_ui_card', + })); + }); it('accepts a complete known component without evaluating it', () => { expect(() => validate()).not.toThrow(); expect(() => validate(card('root = Text("Query Mutation @Run are just quoted text")'))).not.toThrow(); diff --git a/backend/services/uiCardValidation.ts b/backend/services/uiCardValidation.ts index f00bfacfa2..0f3fbc4d79 100644 --- a/backend/services/uiCardValidation.ts +++ b/backend/services/uiCardValidation.ts @@ -18,6 +18,18 @@ export const UI_CARD_SOURCE_CONNECTORS: readonly string[] = Object.freeze( [...catalogueRev2.$defs.Source.properties.connector.enum], ); +// Mirrors frontend/src/v2/cards/cardLinks.ts: the same hosts the renderer opens, +// so a card the server accepts is one the client will not render as unavailable. +const UI_LINK_HOSTS = new Set(['commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io']); +const ATLASSIAN_SITE = /^[a-z0-9-]+\.atlassian\.net$/; +export function isApprovedCardLink(raw: string): boolean { + try { + const url = new URL(raw); + return url.protocol === 'https:' && !url.username && !url.password && !url.port + && (UI_LINK_HOSTS.has(url.hostname) || ATLASSIAN_SITE.test(url.hostname)); + } catch { return false; } +} + export const UI_CARD_MAX_BYTES = 32 * 1024; export const UI_CARD_MAX_EXPANDED_NODES = 2048; const bracketTokens = tokenize('()[]{}'); @@ -82,8 +94,7 @@ function validatePresentation(root: ElementNode): void { const { props } = node; if (node.typeName === 'Source') sources += 1; if (node.typeName === 'Link') { - const url = new URL(props.url as string); - if (url.protocol !== 'https:' || url.username || url.password) refuse('Link requires a credential-free https URL'); + if (!isApprovedCardLink(String(props.url))) refuse('Link requires an approved credential-free https URL'); if (props.as !== 'text') buttons += 1; if (props.as === 'primary') primary += 1; } @@ -282,10 +293,7 @@ function validateCardSource(source: string, rev: CardRevision, serverStamped: bo if (node.name === 'OpenUrl') { const [url] = node.args; if (node.args.length !== 1 || url?.k !== 'Str') refuse('OpenUrl requires a literal https URL'); - const parsed = new URL(url.v); - if (parsed.protocol !== 'https:' || parsed.username || parsed.password) { - refuse('OpenUrl requires a literal https URL'); - } + if (!isApprovedCardLink(url.v)) refuse('OpenUrl requires an approved literal https URL'); } } else if (node.k === 'Ref' && ['Query', 'Mutation', 'Run', 'Action', 'OpenUrl'].includes(node.n)) { refuse(`UI card capability is not allowed: ${node.n}`); diff --git a/commonly-mcp/__tests__/tools.test.mjs b/commonly-mcp/__tests__/tools.test.mjs index 2dc4d62ea9..3aec88f6b1 100644 --- a/commonly-mcp/__tests__/tools.test.mjs +++ b/commonly-mcp/__tests__/tools.test.mjs @@ -89,6 +89,14 @@ describe('commonly_post_message', () => { expect(description).toContain(payload.catalogHash); }); + it('teaches every runtime the list-results card rule, with the live catalogue tuple', () => { + const description = byName.commonly_post_message.description; + const live = JSON.parse(readFileSync(new URL('../../backend/config/uiCardCatalogueRevision.json', import.meta.url), 'utf8')); + expect(description).toContain('CARD RULE: when a linear.*, sentry.* or atlassian.* list tool'); + expect(description).toContain(`"catalogId":"${live.catalogId}"`); + expect(description).toContain(`"catalogHash":"${live.catalogHash}"`); + }); + it('POSTs to the right path with body', async () => { const fetchSpy = installFetch(async () => okResponse({ id: 'm1' })); const result = await byName.commonly_post_message.call({ diff --git a/commonly-mcp/src/tools.js b/commonly-mcp/src/tools.js index a8874604b0..04b08c127a 100644 --- a/commonly-mcp/src/tools.js +++ b/commonly-mcp/src/tools.js @@ -203,7 +203,7 @@ export const buildTools = (config) => { }, { name: 'commonly_post_message', - description: 'Post a chat message into a pod as this agent. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true to send despite them.\n\nA pod is a CHAT ROOM a human may scroll, not a report surface. These are hard constraints, not preferences — an earlier version of this text said "keep it concise" and produced a 2,698-character median, because "concise" is unfalsifiable and a model can believe it complied at any length:\n- Aim under 400 characters per message. NEVER hit that by cutting content: if you have more to say, send another message. Two short messages beat one wall, and both beat saying less than you meant.\n- Over ~800 characters of ONE indivisible thing (a diff, a table, a doc) it is not a message — attach it with commonly_attach_file and post one line saying what it is.\n- Post the RESULT, not your reasoning. The thinking earned the answer; it is not the answer. Reasoning belongs in a PR body or a doc.\n- Never open with a bold sentence. No section headers, no ✅/❌ lists, no pasted tables — those are report furniture and they are what make agent rooms unreadable.\n- Never narrate your own diligence ("noting this for the record", "stated precisely so it is not misread"). Delete those sentences entirely.\n- One idea per message. A second header means it should be two messages or a linked document.\n- Cap 3 messages per minute AND 3 in a row. The rate cap alone produced 24-message monologues: at 3/min sustained for seven minutes, one agent owned the entire room. A rate bounds how FAST you talk, never how LONG you hold the floor.\n- Before a 4th consecutive message with nobody else having spoken, STOP. Not "wrap up" — stop. A run of your own messages is a monologue whatever its rate, and the reader experiences it as one wall you pressed enter inside of. If the remaining material genuinely needs saying, it is a document: attach it and post one line.\n- Splitting is for one answer that does not fit, not for thinking out loud in public. Three messages answering one question is fine. Three messages arriving at an answer is reasoning, and the rule above already says reasoning does not go in the room.\n\nReply to what was actually said. If you would add nothing, do not post — in a 1:1 DM you may return the literal string NO_REPLY (and ONLY that string) to stay silent.\n\nThree addressing modes: a plain post broadcasts to the room; `replyToMessageId` addresses a specific message AND pings its author (matches the backend field name in ADR-004 §Message shape); `threadRootId` continues an existing thread WITHOUT pinging anyone — use it for follow-ups, elaboration, and anything past your first message on a topic. Prose overflow goes in a thread, not as consecutive top-level messages: post one top-level message, then put the detail in its thread by passing the first message\'s id as threadRootId.' + '\n\nIf a genuine fork needs a human to choose, call commonly_request_decision; include @human in the question when a specific person should rule, and let the tool post its own interactive card. Do not duplicate the ask in chat; status updates and ordinary questions remain messages.', + description: 'Post a chat message into a pod as this agent. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true to send despite them.\n\nA pod is a CHAT ROOM a human may scroll, not a report surface. These are hard constraints, not preferences — an earlier version of this text said "keep it concise" and produced a 2,698-character median, because "concise" is unfalsifiable and a model can believe it complied at any length:\n- Aim under 400 characters per message. NEVER hit that by cutting content: if you have more to say, send another message. Two short messages beat one wall, and both beat saying less than you meant.\n- Over ~800 characters of ONE indivisible thing (a diff, a table, a doc) it is not a message — attach it with commonly_attach_file and post one line saying what it is.\n- Post the RESULT, not your reasoning. The thinking earned the answer; it is not the answer. Reasoning belongs in a PR body or a doc.\n- Never open with a bold sentence. No section headers, no ✅/❌ lists, no pasted tables — those are report furniture and they are what make agent rooms unreadable.\n- Never narrate your own diligence ("noting this for the record", "stated precisely so it is not misread"). Delete those sentences entirely.\n- One idea per message. A second header means it should be two messages or a linked document.\n- Cap 3 messages per minute AND 3 in a row. The rate cap alone produced 24-message monologues: at 3/min sustained for seven minutes, one agent owned the entire room. A rate bounds how FAST you talk, never how LONG you hold the floor.\n- Before a 4th consecutive message with nobody else having spoken, STOP. Not "wrap up" — stop. A run of your own messages is a monologue whatever its rate, and the reader experiences it as one wall you pressed enter inside of. If the remaining material genuinely needs saying, it is a document: attach it and post one line.\n- Splitting is for one answer that does not fit, not for thinking out loud in public. Three messages answering one question is fine. Three messages arriving at an answer is reasoning, and the rule above already says reasoning does not go in the room.\n\nReply to what was actually said. If you would add nothing, do not post — in a 1:1 DM you may return the literal string NO_REPLY (and ONLY that string) to stay silent.\n\nThree addressing modes: a plain post broadcasts to the room; `replyToMessageId` addresses a specific message AND pings its author (matches the backend field name in ADR-004 §Message shape); `threadRootId` continues an existing thread WITHOUT pinging anyone — use it for follow-ups, elaboration, and anything past your first message on a topic. Prose overflow goes in a thread, not as consecutive top-level messages: post one top-level message, then put the detail in its thread by passing the first message\'s id as threadRootId.' + '\n\nCARD RULE: when a linear.*, sentry.* or atlassian.* list tool returns records, post them as ONE card, not bullet prose. Set messageType "card"; content is the same list as plain text (the fallback every reader sees); payload is {"kind":"ui","v":1,"lang":"openui@1","catalogId":"' + UI_CARD_CATALOGUE.revision.catalogId + '","catalogRev":' + UI_CARD_CATALOGUE.revision.catalogRev + ',"catalogHash":"' + UI_CARD_CATALOGUE.revision.catalogHash + '","source":""}. Call commonly_get_ui_card_catalogue first for the grammar. Links in a card must be credential-free https on an approved host.' + '\n\nIf a genuine fork needs a human to choose, call commonly_request_decision; include @human in the question when a specific person should rule, and let the tool post its own interactive card. Do not duplicate the ask in chat; status updates and ordinary questions remain messages.', inputSchema: reqWith({ podId: STRING, content: STRING, diff --git a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts index c8ce18ebb6..31b22f9ba2 100644 --- a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts +++ b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts @@ -5375,6 +5375,15 @@ test('the card Source line (TASK-607) uses v2 tokens only, one value per token, expect(css).toMatch(/--m:var\(--v2-text-muted\);--s:var\(--v2-text-secondary\)/); }); +test('OpenUI card styles take every colour from v2 tokens, so cards follow dark mode (TASK-336)', () => { + const css = read('../cards/uiCard.css'); + // Attribute selectors name the bundled brand marks' own fills, so they are stripped first. + const colourless = css.replace(/\[[^\]]*\]/g, ''); + expect(colourless).not.toMatch(/#[0-9a-fA-F]{3,8}\b/); + expect(colourless).not.toMatch(/\b(rgba?|hsla?)\(/); + expect(css).toContain('--b:var(--v2-border)'); +}); + describe('v2 board on a phone (TASK-576)', () => { test('the board head is the frame\'s one-row 52px top bar, title taking the free width', () => { // S4 (d215 .mtop): back, title, search, New task in one row; the title ellipsizes. diff --git a/frontend/src/v2/cards/__tests__/uiCardSource.test.ts b/frontend/src/v2/cards/__tests__/uiCardSource.test.ts index 50b9e793d1..957e1c5c6c 100644 --- a/frontend/src/v2/cards/__tests__/uiCardSource.test.ts +++ b/frontend/src/v2/cards/__tests__/uiCardSource.test.ts @@ -17,6 +17,15 @@ test('permits only exact approved HTTPS hosts without credentials or ports', () } }); +test('permits the Linear, Sentry and Atlassian site hosts and refuses lookalikes', () => { + for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x']) { + expect(cardHref(url)?.href).toBe(url); + } + for (const url of ['https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', 'https://sentry.io.evil.com', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', 'https://a.b.atlassian.net']) { + expect(cardHref(url)).toBeNull(); + } +}); + test.each([ 'OpenUrl("javascript:alert(1)")', 'Action(OpenUrl("javascript:alert(1)"))', diff --git a/frontend/src/v2/cards/cardLinks.ts b/frontend/src/v2/cards/cardLinks.ts index edb4f310f2..a8692eabb8 100644 --- a/frontend/src/v2/cards/cardLinks.ts +++ b/frontend/src/v2/cards/cardLinks.ts @@ -1,9 +1,16 @@ -export const UI_LINK_HOSTS = new Set(['commonly.me', 'docs.commonly.me', 'github.com']); +export const UI_LINK_HOSTS = new Set([ + 'commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io', +]); +// One suffix rule: a single-label Atlassian Cloud site, e.g. acme.atlassian.net. +const ATLASSIAN_SITE = /^[a-z0-9-]+\.atlassian\.net$/; +export function isCardHost(hostname: string): boolean { + return UI_LINK_HOSTS.has(hostname) || ATLASSIAN_SITE.test(hostname); +} export function cardHref(raw: unknown): URL | null { if (typeof raw !== 'string') return null; try { const url = new URL(raw); return url.protocol === 'https:' && !url.username && !url.password && !url.port - && UI_LINK_HOSTS.has(url.hostname) ? url : null; + && isCardHost(url.hostname) ? url : null; } catch { return null; } } From d78fb23c611d5902a54a484b8f21cedb27df7928 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 03:15:45 -0800 Subject: [PATCH 2/5] fix(cards): approve the staging host the seed card links to The staging seed's rev-1 card links https://staging.commonly.me/v2. The new server host check refused it, and the renderer never opened it either, so the seed button had not worked on staging. Add the exact host to both sets. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- backend/services/uiCardValidation.ts | 2 +- frontend/src/v2/cards/__tests__/uiCardSource.test.ts | 2 +- frontend/src/v2/cards/cardLinks.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/services/uiCardValidation.ts b/backend/services/uiCardValidation.ts index 0f3fbc4d79..54161a0882 100644 --- a/backend/services/uiCardValidation.ts +++ b/backend/services/uiCardValidation.ts @@ -20,7 +20,7 @@ export const UI_CARD_SOURCE_CONNECTORS: readonly string[] = Object.freeze( // Mirrors frontend/src/v2/cards/cardLinks.ts: the same hosts the renderer opens, // so a card the server accepts is one the client will not render as unavailable. -const UI_LINK_HOSTS = new Set(['commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io']); +const UI_LINK_HOSTS = new Set(['commonly.me', 'staging.commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io']); const ATLASSIAN_SITE = /^[a-z0-9-]+\.atlassian\.net$/; export function isApprovedCardLink(raw: string): boolean { try { diff --git a/frontend/src/v2/cards/__tests__/uiCardSource.test.ts b/frontend/src/v2/cards/__tests__/uiCardSource.test.ts index 957e1c5c6c..85d6d2f12b 100644 --- a/frontend/src/v2/cards/__tests__/uiCardSource.test.ts +++ b/frontend/src/v2/cards/__tests__/uiCardSource.test.ts @@ -18,7 +18,7 @@ test('permits only exact approved HTTPS hosts without credentials or ports', () }); test('permits the Linear, Sentry and Atlassian site hosts and refuses lookalikes', () => { - for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x']) { + for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x', 'https://staging.commonly.me/v2']) { expect(cardHref(url)?.href).toBe(url); } for (const url of ['https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', 'https://sentry.io.evil.com', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', 'https://a.b.atlassian.net']) { diff --git a/frontend/src/v2/cards/cardLinks.ts b/frontend/src/v2/cards/cardLinks.ts index a8692eabb8..02d108a76b 100644 --- a/frontend/src/v2/cards/cardLinks.ts +++ b/frontend/src/v2/cards/cardLinks.ts @@ -1,5 +1,5 @@ export const UI_LINK_HOSTS = new Set([ - 'commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io', + 'commonly.me', 'staging.commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io', ]); // One suffix rule: a single-label Atlassian Cloud site, e.g. acme.atlassian.net. const ATLASSIAN_SITE = /^[a-z0-9-]+\.atlassian\.net$/; From 88cd720f92459b649dd663d3da19abac24686b23 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 03:16:04 -0800 Subject: [PATCH 3/5] test(cards): pin the host rule on the Link presentation component A revert of the Link guard to HTTPS-only would have left the suite green. Add Link cases: approved hosts accepted, an unapproved HTTPS host refused. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- backend/__tests__/unit/services/uiCardValidation.test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/backend/__tests__/unit/services/uiCardValidation.test.js b/backend/__tests__/unit/services/uiCardValidation.test.js index 45cbe569ef..b4e208fab1 100644 --- a/backend/__tests__/unit/services/uiCardValidation.test.js +++ b/backend/__tests__/unit/services/uiCardValidation.test.js @@ -27,6 +27,14 @@ describe('untrusted UI card write validation', () => { expect(() => validate(card(`root = Text("hello")\nunused = Action(@OpenUrl("${url}"))`))).not.toThrow(); } }); + it('applies the same host rule to the Link presentation component', () => { + const link = (url) => card(`root = Card("Links", [Link("Open", "${url}", "button")])`); + expect(() => validate(link('https://linear.app/x'))).not.toThrow(); + expect(() => validate(link('https://staging.commonly.me/v2'))).not.toThrow(); + expect(() => validate(link('https://evil.example/x'))).toThrow(expect.objectContaining({ + statusCode: 400, code: 'invalid_ui_card', + })); + }); it.each([ 'https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', 'https://sentry.io.evil.com', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', From 2a141973f4ba409e67ccecc8936d9d1143175602 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 03:39:49 -0800 Subject: [PATCH 4/5] feat(cards): approve single-label Sentry org subdomains Real Sentry issue links sit on the org subdomain, so the bare sentry.io entry refused every Sentry card. Extend the suffix rule to .sentry.io, the same single-label shape as .atlassian.net. Deeper names stay refused on both client and server. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- backend/__tests__/unit/services/uiCardValidation.test.js | 4 ++-- backend/services/uiCardValidation.ts | 4 ++-- frontend/src/v2/cards/__tests__/uiCardSource.test.ts | 4 ++-- frontend/src/v2/cards/cardLinks.ts | 7 ++++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/backend/__tests__/unit/services/uiCardValidation.test.js b/backend/__tests__/unit/services/uiCardValidation.test.js index b4e208fab1..2592f065d3 100644 --- a/backend/__tests__/unit/services/uiCardValidation.test.js +++ b/backend/__tests__/unit/services/uiCardValidation.test.js @@ -23,7 +23,7 @@ describe('untrusted UI card write validation', () => { expect(() => validateAgentCardPayload(type, undefined, 'hello')).toThrow('text or card'); }); it('accepts the Linear, Sentry and Atlassian site links the renderer opens', () => { - for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x']) { + for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x', 'https://my-organization.sentry.io/issues/123/']) { expect(() => validate(card(`root = Text("hello")\nunused = Action(@OpenUrl("${url}"))`))).not.toThrow(); } }); @@ -37,7 +37,7 @@ describe('untrusted UI card write validation', () => { }); it.each([ 'https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', - 'https://sentry.io.evil.com', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', + 'https://sentry.io.evil.com', 'https://a.b.sentry.io/issues/1/', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', ])('refuses the unapproved link %s', (url) => { expect(() => validate(card(`root = Text("hello")\nunused = Action(@OpenUrl("${url}"))`))).toThrow(expect.objectContaining({ statusCode: 400, code: 'invalid_ui_card', diff --git a/backend/services/uiCardValidation.ts b/backend/services/uiCardValidation.ts index 54161a0882..6e517cb889 100644 --- a/backend/services/uiCardValidation.ts +++ b/backend/services/uiCardValidation.ts @@ -21,12 +21,12 @@ export const UI_CARD_SOURCE_CONNECTORS: readonly string[] = Object.freeze( // Mirrors frontend/src/v2/cards/cardLinks.ts: the same hosts the renderer opens, // so a card the server accepts is one the client will not render as unavailable. const UI_LINK_HOSTS = new Set(['commonly.me', 'staging.commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io']); -const ATLASSIAN_SITE = /^[a-z0-9-]+\.atlassian\.net$/; +const SITE_SUFFIX = /^[a-z0-9-]+\.(atlassian\.net|sentry\.io)$/; export function isApprovedCardLink(raw: string): boolean { try { const url = new URL(raw); return url.protocol === 'https:' && !url.username && !url.password && !url.port - && (UI_LINK_HOSTS.has(url.hostname) || ATLASSIAN_SITE.test(url.hostname)); + && (UI_LINK_HOSTS.has(url.hostname) || SITE_SUFFIX.test(url.hostname)); } catch { return false; } } diff --git a/frontend/src/v2/cards/__tests__/uiCardSource.test.ts b/frontend/src/v2/cards/__tests__/uiCardSource.test.ts index 85d6d2f12b..2b281410ef 100644 --- a/frontend/src/v2/cards/__tests__/uiCardSource.test.ts +++ b/frontend/src/v2/cards/__tests__/uiCardSource.test.ts @@ -18,10 +18,10 @@ test('permits only exact approved HTTPS hosts without credentials or ports', () }); test('permits the Linear, Sentry and Atlassian site hosts and refuses lookalikes', () => { - for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x', 'https://staging.commonly.me/v2']) { + for (const url of ['https://linear.app/x', 'https://sentry.io/x', 'https://acme.atlassian.net/x', 'https://staging.commonly.me/v2', 'https://my-organization.sentry.io/issues/123/']) { expect(cardHref(url)?.href).toBe(url); } - for (const url of ['https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', 'https://sentry.io.evil.com', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', 'https://a.b.atlassian.net']) { + for (const url of ['https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', 'https://sentry.io.evil.com', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', 'https://a.b.atlassian.net', 'https://a.b.sentry.io/issues/1/']) { expect(cardHref(url)).toBeNull(); } }); diff --git a/frontend/src/v2/cards/cardLinks.ts b/frontend/src/v2/cards/cardLinks.ts index 02d108a76b..35482e19f8 100644 --- a/frontend/src/v2/cards/cardLinks.ts +++ b/frontend/src/v2/cards/cardLinks.ts @@ -1,10 +1,11 @@ export const UI_LINK_HOSTS = new Set([ 'commonly.me', 'staging.commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io', ]); -// One suffix rule: a single-label Atlassian Cloud site, e.g. acme.atlassian.net. -const ATLASSIAN_SITE = /^[a-z0-9-]+\.atlassian\.net$/; +// Suffix rules: a single-label Atlassian Cloud site (acme.atlassian.net) and a single-label +// Sentry org (my-organization.sentry.io). Nothing deeper. +const SITE_SUFFIX = /^[a-z0-9-]+\.(atlassian\.net|sentry\.io)$/; export function isCardHost(hostname: string): boolean { - return UI_LINK_HOSTS.has(hostname) || ATLASSIAN_SITE.test(hostname); + return UI_LINK_HOSTS.has(hostname) || SITE_SUFFIX.test(hostname); } export function cardHref(raw: unknown): URL | null { if (typeof raw !== 'string') return null; From 500166619b187a08a318bce78b6dcf6ef955d748 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 07:03:56 -0800 Subject: [PATCH 5/5] fix(cards): degrade unapproved link hosts to the text fallback A host-only refusal rejected the whole agent message, so the text fallback never posted. Scheme, credential and port faults still refuse the card; an unknown host now returns a degrade signal and both bot routes post the non-empty text as a plain message. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: kai Agent-Model: claude/claude-haiku-5-5/default --- .../unit/services/uiCardValidation.test.js | 18 +++++--- backend/routes/agentsRuntime.ts | 14 ++++--- backend/services/uiCardValidation.ts | 41 ++++++++++++++----- 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/backend/__tests__/unit/services/uiCardValidation.test.js b/backend/__tests__/unit/services/uiCardValidation.test.js index 2592f065d3..e0ecb2ec1f 100644 --- a/backend/__tests__/unit/services/uiCardValidation.test.js +++ b/backend/__tests__/unit/services/uiCardValidation.test.js @@ -31,18 +31,26 @@ describe('untrusted UI card write validation', () => { const link = (url) => card(`root = Card("Links", [Link("Open", "${url}", "button")])`); expect(() => validate(link('https://linear.app/x'))).not.toThrow(); expect(() => validate(link('https://staging.commonly.me/v2'))).not.toThrow(); - expect(() => validate(link('https://evil.example/x'))).toThrow(expect.objectContaining({ - statusCode: 400, code: 'invalid_ui_card', - })); + // An unknown host degrades the card to its text fallback instead of dropping the message. + expect(validate(link('https://evil.example/x'))).toBe(true); }); it.each([ 'https://linear.app.evil.com', 'https://evilatlassian.net', 'https://atlassian.net.evil.com', - 'https://sentry.io.evil.com', 'https://a.b.sentry.io/issues/1/', 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', - ])('refuses the unapproved link %s', (url) => { + 'https://sentry.io.evil.com', 'https://a.b.sentry.io/issues/1/', + ])('degrades a card with an unapproved host %s to the text fallback', (url) => { + expect(validate(card(`root = Text("hello")\nunused = Action(@OpenUrl("${url}"))`))).toBe(true); + expect(validate(card(`root = Card("Links", [Link("Open", "${url}", "button")])`))).toBe(true); + }); + it.each([ + 'http://linear.app/x', 'https://user:pw@linear.app', 'https://linear.app:8443', + ])('still refuses the scheme, credential or port fault %s', (url) => { expect(() => validate(card(`root = Text("hello")\nunused = Action(@OpenUrl("${url}"))`))).toThrow(expect.objectContaining({ statusCode: 400, code: 'invalid_ui_card', })); }); + it('returns false for a card whose links are all approved', () => { + expect(validate(card('root = Text("hello")\nunused = Action(@OpenUrl("https://linear.app/x"))'))).toBe(false); + }); it('accepts a complete known component without evaluating it', () => { expect(() => validate()).not.toThrow(); expect(() => validate(card('root = Text("Query Mutation @Run are just quoted text")'))).not.toThrow(); diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index d8b1dce3ba..4e63bd8bcd 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -1613,9 +1613,11 @@ router.post( return res.status(403).json({ message: 'Bot not installed in this pod' }); } - const { content, metadata, messageType, payload } = req.body || {}; - validateAgentCardPayload(messageType, payload, payload !== undefined + const { content, metadata, payload: cardPayload } = req.body || {}; + const hostFallback = validateAgentCardPayload(req.body?.messageType, cardPayload, cardPayload !== undefined ? AgentMessageService.sanitizeAgentContent(content) : content); + const messageType = hostFallback ? 'text' : req.body?.messageType; + const payload = hostFallback ? undefined : cardPayload; const wouldPost = messageType === 'card' || Boolean(AgentMessageService.sanitizeAgentContent(content)); if (wouldPost && req.body?.continueAnyway !== true) { const hold = await AgentSendFreshness.unseenMessages(user._id, podId); @@ -2308,12 +2310,12 @@ router.post('/pods/:podId/messages', phase4RateLimit, agentRuntimeAuth, async (r return res.status(403).json({ message: 'Agent token not authorized for this pod' }); } - validateAgentCardPayload(req.body?.messageType, req.body?.payload, req.body?.payload !== undefined + const hostFallback = validateAgentCardPayload(req.body?.messageType, req.body?.payload, req.body?.payload !== undefined ? AgentMessageService.sanitizeAgentContent(req.body?.content) : req.body?.content); // A total-match silence or runtime artifact is not a post. Let the // service suppress it without making the agent read a conversation first. - const wouldPost = req.body?.messageType === 'card' + const wouldPost = (!hostFallback && req.body?.messageType === 'card') || Boolean(AgentMessageService.sanitizeAgentContent(req.body?.content)); if (wouldPost && req.body?.continueAnyway !== true) { const agentUser = req.agentUser || await AgentIdentityService.getOrCreateAgentUser(installation.agentName, { @@ -2325,8 +2327,10 @@ router.post('/pods/:podId/messages', phase4RateLimit, agentRuntimeAuth, async (r } const { - content, metadata, messageType, replyToMessageId, threadRootId, payload, + content, metadata, replyToMessageId, threadRootId, payload: cardPayload, } = req.body || {}; + const messageType = hostFallback ? 'text' : req.body?.messageType; + const payload = hostFallback ? undefined : cardPayload; const result = await AgentMessageService.postMessage({ agentName: installation.agentName, instanceId: installation.instanceId || 'default', diff --git a/backend/services/uiCardValidation.ts b/backend/services/uiCardValidation.ts index 6e517cb889..9c649a2b4b 100644 --- a/backend/services/uiCardValidation.ts +++ b/backend/services/uiCardValidation.ts @@ -22,12 +22,14 @@ export const UI_CARD_SOURCE_CONNECTORS: readonly string[] = Object.freeze( // so a card the server accepts is one the client will not render as unavailable. const UI_LINK_HOSTS = new Set(['commonly.me', 'staging.commonly.me', 'docs.commonly.me', 'github.com', 'linear.app', 'sentry.io']); const SITE_SUFFIX = /^[a-z0-9-]+\.(atlassian\.net|sentry\.io)$/; -export function isApprovedCardLink(raw: string): boolean { - try { - const url = new URL(raw); - return url.protocol === 'https:' && !url.username && !url.password && !url.port - && (UI_LINK_HOSTS.has(url.hostname) || SITE_SUFFIX.test(url.hostname)); - } catch { return false; } +/** Scheme, credential and port faults refuse the card; an unknown host is the only UnapprovedHostError. */ +function assertApprovedCardLink(raw: string, refusal: string): void { + let url: URL; + try { url = new URL(raw); } catch { refuse(refusal); } + if (url.protocol !== 'https:' || url.username || url.password || url.port) refuse(refusal); + if (!UI_LINK_HOSTS.has(url.hostname) && !SITE_SUFFIX.test(url.hostname)) { + refuseHost(`${refusal} (host is not approved)`); + } } export const UI_CARD_MAX_BYTES = 32 * 1024; @@ -94,7 +96,7 @@ function validatePresentation(root: ElementNode): void { const { props } = node; if (node.typeName === 'Source') sources += 1; if (node.typeName === 'Link') { - if (!isApprovedCardLink(String(props.url))) refuse('Link requires an approved credential-free https URL'); + assertApprovedCardLink(String(props.url), 'Link requires an approved credential-free https URL'); if (props.as !== 'text') buttons += 1; if (props.as === 'primary') primary += 1; } @@ -137,6 +139,13 @@ function refuse(message: string): never { throw new UICardValidationError(message); } +/** A link to a host the renderer does not open. Agent cards fall back to text for this alone. */ +export class UnapprovedHostError extends UICardValidationError {} + +function refuseHost(message: string): never { + throw new UnapprovedHostError(message); +} + /** Bound reference substitution before lang-core allocates a materialized tree. */ function validateExpansionBudget(expressions: Map): void { type Cost = { nodes: number; depth: number }; @@ -176,13 +185,23 @@ function validateExpansionBudget(expressions: Map): void { } } -/** Validate untrusted CAP input, never privileged kernel-composed payloads. */ +/** + * Validate untrusted CAP input, never privileged kernel-composed payloads. + * Returns true when the card was refused only for an unapproved link host, so + * the route posts the text fallback instead of dropping the whole message. + */ export function validateAgentCardPayload( messageType: unknown, payload: unknown, content: unknown, -): void { - validateCardPayload(messageType, payload, content, false); +): boolean { + try { + validateCardPayload(messageType, payload, content, false); + return false; + } catch (error) { + if (error instanceof UnapprovedHostError) return true; + throw error; + } } function validateCardPayload( @@ -293,7 +312,7 @@ function validateCardSource(source: string, rev: CardRevision, serverStamped: bo if (node.name === 'OpenUrl') { const [url] = node.args; if (node.args.length !== 1 || url?.k !== 'Str') refuse('OpenUrl requires a literal https URL'); - if (!isApprovedCardLink(url.v)) refuse('OpenUrl requires an approved literal https URL'); + assertApprovedCardLink(url.v, 'OpenUrl requires an approved literal https URL'); } } else if (node.k === 'Ref' && ['Query', 'Mutation', 'Run', 'Action', 'OpenUrl'].includes(node.n)) { refuse(`UI card capability is not allowed: ${node.n}`);