From 8b857313f8ee17d218ed263736ce31a0fd5e6717 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:27:58 -0500 Subject: [PATCH 01/10] Refuse the shell and a workspace write while a person holds the wheel `assertBotMayAct` was called in the navigate handler and the four action handlers, and nowhere else. `/exec` and `/files/write` were not covered, so a Bot could keep running commands and rewriting its workspace underneath somebody who had taken the browser at a login wall. The shell arrived after the wheel existed and was never wired to it. That is the property this codebase states outright. control.ts says every acting call from the Bot is refused while a person holds control, and the README says Bot actions are refused rather than queued. Both were false for the most powerful path the product exposes, and the server could not cover for it: control lives in this process, so those two call sites were the whole of the enforcement. The decision moves to `actsOnTheComputer` in authorisation.ts and is asked once by the dispatcher, after the session resolves. A per-handler check is the thing the next endpoint forgets, which is exactly how the shell came to be missing one; a list the dispatcher consults has to be added to instead. It lives beside the other path decision rather than in index.ts because that file imports Playwright at module scope, so a decision left there cannot be tested without Chrome. Reading stays open. `/files/read` and `/files/list` are not acting, and a Bot that has just been stopped still needs to say what it was doing. The two in-handler guards and their now-dead ControlError branches come out with it, so there is one place that answers this and not three. --- agent-computer/src/authorisation.ts | 27 +++++++++++ agent-computer/src/index.ts | 37 ++++++++++----- agent-computer/tests/authorisation.test.ts | 54 +++++++++++++++++++++- 3 files changed, 106 insertions(+), 12 deletions(-) diff --git a/agent-computer/src/authorisation.ts b/agent-computer/src/authorisation.ts index 667f2d44..cbeabb10 100644 --- a/agent-computer/src/authorisation.ts +++ b/agent-computer/src/authorisation.ts @@ -47,3 +47,30 @@ export function offeredToken(headers: Headers, url: URL): string { export function isOpenPath(pathname: string): boolean { return pathname === "/health"; } + +/** + * Which paths act on the computer, and so are refused while a person holds the wheel. + * + * One list, asked once per request, rather than a check inside each handler. The shell is the reason: + * `/exec` arrived after the wheel existed and was never given the guard the page paths had, so a Bot + * could keep running commands and writing files underneath somebody who had taken the browser at a + * login wall. A per-handler check is exactly the thing the next endpoint forgets, which is how that + * happened; a list the dispatcher consults is one an endpoint has to be added to. + * + * Reading is not acting. `/files/read` and `/files/list` stay open so a Bot that has been stopped can + * still read its own notes and explain what it was doing, which is the answer the person handing the + * wheel back usually wants. + */ +const ACTING_PATHS = new Set([ + "/navigate", + "/click", + "/type", + "/key", + "/scroll", + "/exec", + "/files/write", +]); + +export function actsOnTheComputer(pathname: string): boolean { + return ACTING_PATHS.has(pathname); +} diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index b4762635..5d7177a6 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -1,7 +1,12 @@ import { serve } from "bun"; import type { Page } from "playwright"; import { parseAriaSnapshot, type SnapshotElement } from "./aria-snapshot"; -import { isOpenPath, matchesToken, offeredToken } from "./authorisation"; +import { + actsOnTheComputer, + isOpenPath, + matchesToken, + offeredToken, +} from "./authorisation"; import { isPlainBotId } from "./bot-id"; import { type Control, @@ -485,6 +490,26 @@ serve({ } const session = sessionFor(botId); + /* + * The wheel, asked once for everything that acts. + * + * Refused here rather than inside each handler because the handler that forgets is the whole + * defect: the shell shipped without this check and ran commands underneath a person who had taken + * the browser at a login wall. `actsOnTheComputer` is the list, and a new acting endpoint is + * refused by being added to it rather than by remembering to repeat this. + */ + if (actsOnTheComputer(url.pathname)) { + try { + session.control.assertBotMayAct(); + } catch (error) { + // A person holding the wheel is not a failure of the action; the Bot should wait and say so. + if (error instanceof ControlError) { + return json({ error: error.message, humanHasControl: true }, 409); + } + throw error; + } + } + if (url.pathname === "/stream") { /* * The socket carries the Bot in the query because it cannot do it in a header. Every other call here names @@ -696,7 +721,6 @@ serve({ const startedAt = Date.now(); try { - session.control.assertBotMayAct(); const target = await currentPage(botId); await target.goto(body.url, { waitUntil: "domcontentloaded", @@ -715,10 +739,6 @@ serve({ elapsedMs: Date.now() - startedAt, }); } catch (error) { - // A person holding the wheel is not a failed navigation; the Bot should wait. - if (error instanceof ControlError) { - return json({ error: error.message, humanHasControl: true }, 409); - } // The page is the Bot's working surface, so a failed navigation is reported rather than // thrown: the transcript needs to say what happened, and the browser stays usable. return json( @@ -893,7 +913,6 @@ serve({ const startedAt = Date.now(); try { - session.control.assertBotMayAct(); const target = await currentPage(botId); const detail = await performAction( session, @@ -932,10 +951,6 @@ serve({ if (error instanceof StaleSnapshotError) { return json({ error: error.message, stale: true }, 409); } - // 409 as well, and for the same reason: nothing is broken, the caller simply has to wait. - if (error instanceof ControlError) { - return json({ error: error.message, humanHasControl: true }, 409); - } return json({ error: describe(error, "The action failed.") }, 502); } } diff --git a/agent-computer/tests/authorisation.test.ts b/agent-computer/tests/authorisation.test.ts index 9ec6521b..34fcfdc4 100644 --- a/agent-computer/tests/authorisation.test.ts +++ b/agent-computer/tests/authorisation.test.ts @@ -1,5 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { isOpenPath, matchesToken, offeredToken } from "../src/authorisation"; +import { + actsOnTheComputer, + isOpenPath, + matchesToken, + offeredToken, +} from "../src/authorisation"; /** * The check that stands in front of a Bot's browser. @@ -88,3 +93,50 @@ describe("what an unauthenticated caller may reach", () => { } }); }); + +/** + * Which paths the wheel stops. + * + * A person takes the wheel at a login wall precisely because they no longer want the Bot acting, and + * `control.ts` states the property outright: "While a person holds control every acting call from the + * Bot is refused". That was true of the page from the start and untrue of the shell, which arrived + * later (#62) and was never wired to the wheel, so a Bot could run a command and rewrite the + * workspace underneath somebody mid-sign-in. + * + * The list lives here, beside the other path decision, rather than in `index.ts`, for the reason the + * header of this file gives: a decision next to `chromium.launch()` cannot be tested without Chrome. + * + * Reading is not acting. `/files/read` and `/files/list` stay open so a Bot waiting to be handed the + * wheel back can still say what it was doing. + */ +describe("what the wheel stops while a person is driving", () => { + test("every path that acts on the computer, the shell and a workspace write included", () => { + for (const path of [ + "/navigate", + "/click", + "/type", + "/key", + "/scroll", + "/exec", + "/files/write", + ]) { + expect(actsOnTheComputer(path)).toBeTrue(); + } + }); + + test("reading, looking and the handover itself are not acting", () => { + for (const path of [ + "/files/read", + "/files/list", + "/snapshot", + "/screenshot", + "/health", + "/control", + "/control/take", + "/control/release", + "/stream", + ]) { + expect(actsOnTheComputer(path)).toBeFalse(); + } + }); +}); From a15c5d2b22d4674b0f48d3ccd673ab0036f09d98 Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:38:22 -0500 Subject: [PATCH 02/10] Say in the changelog that the wheel now stops the shell A deployment behaves differently afterwards: an action that used to run during a takeover is refused, so it belongs here rather than only in the commit. --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b90ea999..131d3843 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,21 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Taking the wheel stops the Bot's shell, not just its clicks + +While a person held the wheel the Bot was refused on the page, and not in the shell. `/exec` and a +workspace write went through, so a Bot could keep running commands and rewriting its `/workspace` +underneath somebody who had taken the browser at a login wall. The guard existed and covered +navigation and the four page actions; the shell arrived later and was never wired to it. + +Every acting path now asks the same question in one place, so the property the documentation states +is the property the computer has. Reading is deliberately not acting: `/files/read` and +`/files/list` still answer while a person drives, because a Bot that has just been stopped still has +to be able to say what it was doing. + +Nothing to configure. A Bot that acts during a takeover gets the refusal it already got for a click, +and the trail records the attempt and the failure the same way. + ### A finished turn shows the page it opened, not the one open now Reopening a conversation made every past turn fetch the screen as it is now, so an answer about From 291bae6dc7b79821d10bc2719a22bcc995af613a Mon Sep 17 00:00:00 2001 From: Guido Vizoso Date: Wed, 26 Aug 2026 10:30:59 -0300 Subject: [PATCH 03/10] Show a dot on a channel a Bot has spoken in unseen (#259) * Give a membership a memory of when its channel was last read * Stamp the caller's membership read, and say so in the roster * Let a member say they have read a channel * Carry the read marker to the app and let it be stamped * Draw a dot on a channel a Bot has spoken in unseen * Mark a channel read the moment it is the one on screen * Narrow the admin and settings rails to the width their labels earn * Keep a fast clock elsewhere from turning mark-read into a storm * Say in the changelog what the unread dot is and is not * Stamp a read against the message clock, not just this one * Assert the clamped stamp without reaching through an optional chain * Regenerate the read-marker migration behind the queue and the frames * Refuse to mark a deleted channel read, matching the pin --- CHANGELOG.md | 10 + .../components/app-sidebar/app-sidebar.tsx | 39 +- app/src/components/app-sidebar/channel.tsx | 12 +- app/src/lib/channels/mutations.ts | 56 +- app/src/lib/channels/queries.ts | 2 + .../_authed/_app/channel/$channelId.tsx | 44 +- app/src/routes/_authed/admin/route.tsx | 6 +- app/src/routes/_authed/settings/route.tsx | 6 +- app/tests/channel-menu-mutations.test.ts | 88 +- app/tests/channel-order.test.ts | 1 + app/tests/channel-unread.test.ts | 62 + server/drizzle/0019_channel_read_marker.sql | 1 + server/drizzle/meta/0019_snapshot.json | 2583 +++++++++++++++++ server/drizzle/meta/_journal.json | 7 + server/src/channels/routes.ts | 50 + server/src/db/schema/core.ts | 5 + .../channel-activity.integration.test.ts | 1 + server/tests/channel-routes.test.ts | 163 ++ 18 files changed, 3123 insertions(+), 13 deletions(-) create mode 100644 app/tests/channel-unread.test.ts create mode 100644 server/drizzle/0019_channel_read_marker.sql create mode 100644 server/drizzle/meta/0019_snapshot.json diff --git a/CHANGELOG.md b/CHANGELOG.md index b90ea999..7bdc7671 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A channel a Bot has spoken in unseen shows a dot + +The sidebar marks a channel when a Bot has said something since you last had it open: a dot beside +the preview, the name a touch heavier. Opening the channel clears it, your own messages never set +it, and the channel you are looking at never shows it. The marker is yours alone — per member, on +the membership row like the pin — so one person reading does not clear anybody else's dot. + +The deployment gains one nullable column, via migration `0019`. + + ### A finished turn shows the page it opened, not the one open now Reopening a conversation made every past turn fetch the screen as it is now, so an answer about diff --git a/app/src/components/app-sidebar/app-sidebar.tsx b/app/src/components/app-sidebar/app-sidebar.tsx index 68c0676d..99946b00 100644 --- a/app/src/components/app-sidebar/app-sidebar.tsx +++ b/app/src/components/app-sidebar/app-sidebar.tsx @@ -13,7 +13,12 @@ import { useQuery, useQueryClient, } from "@tanstack/react-query"; -import { Link, type LinkOptions, useNavigate } from "@tanstack/react-router"; +import { + Link, + type LinkOptions, + useNavigate, + useParams, +} from "@tanstack/react-router"; import { AnimatePresence, motion, useReducedMotion } from "motion/react"; import type * as React from "react"; import { useState } from "react"; @@ -123,6 +128,30 @@ export function pinnedFirst(channels: ChannelSummary[]): ChannelSummary[] { return [...channels].sort((a, b) => Number(b.pinned) - Number(a.pinned)); } +/** + * Whether a Bot has said something this member has not had on screen yet. + * + * A Bot's message, and only a Bot's: your own message carries a null agent id and reading your own + * words needs no marker. ISO-8601 strings compare correctly as strings, which is the same bet the + * server's recency sort already makes. + */ +export function hasUnseenActivity(channel: ChannelSummary): boolean { + if (channel.lastMessageAgentId === null || channel.lastMessageAt === null) { + return false; + } + return ( + channel.lastReadAt === null || channel.lastMessageAt > channel.lastReadAt + ); +} + +/** Unseen activity somewhere you are not looking. The open channel never shows the dot. */ +export function isUnread( + channel: ChannelSummary, + openChannelId: string | undefined, +): boolean { + return channel.id !== openChannelId && hasUnseenActivity(channel); +} + /** * A roster row that can animate. * @@ -138,6 +167,13 @@ function ChannelRow({ animateOrder: boolean; }) { const shouldReduceMotion = useReducedMotion(); + // Whether this row is unread, as a boolean, for the same reason `Channel` computes `isOpen` + // that way: navigating re-renders the rows whose answer changed, not the whole roster. + const unread = useParams({ + strict: false, + select: (params) => + isUnread(channel, (params as { channelId?: string }).channelId), + }); return ( ); diff --git a/app/src/components/app-sidebar/channel.tsx b/app/src/components/app-sidebar/channel.tsx index f4fa2edf..18733576 100644 --- a/app/src/components/app-sidebar/channel.tsx +++ b/app/src/components/app-sidebar/channel.tsx @@ -42,6 +42,7 @@ export const Channel = memo(function Channel({ lastMessage, lastMessageAt, pinned, + unread, }: { channelId: string; participantIds: string[]; @@ -49,6 +50,7 @@ export const Channel = memo(function Channel({ lastMessage?: string; lastMessageAt?: string; pinned: boolean; + unread: boolean; }) { const queryClient = useQueryClient(); const navigate = useNavigate(); @@ -112,7 +114,11 @@ export const Channel = memo(function Channel({
- + {name}
@@ -123,6 +129,10 @@ export const Channel = memo(function Channel({ {lastMessage} + {unread ? ( + /* State about the message beats state about the row, so it sits first. */ + + ) : null} {pinned ? ( ) : null} diff --git a/app/src/lib/channels/mutations.ts b/app/src/lib/channels/mutations.ts index cf116ed8..2d10c4e4 100644 --- a/app/src/lib/channels/mutations.ts +++ b/app/src/lib/channels/mutations.ts @@ -1,6 +1,10 @@ -import { mutationOptions, type QueryClient } from "@tanstack/react-query"; +import { + mutationOptions, + type InfiniteData, + type QueryClient, +} from "@tanstack/react-query"; import { client, tryClient } from "@/lib/client"; -import { type AgentChannel, channelKeys } from "./queries"; +import { type AgentChannel, type ChannelPage, channelKeys } from "./queries"; /** * Start a new channel with one or more coworkers. @@ -66,6 +70,54 @@ export function setChannelPinnedMutationOptions(queryClient: QueryClient) { }); } +/** + * Stamp a channel read for this member, patching the cache before the wire answers. + * + * Patched in onMutate rather than refetched on success: the dot must clear the instant the channel + * opens, not a round-trip later. No rollback on failure and no invalidation — a mark-read that did + * not land is a dot that returns on the next refetch, which is the truth reasserting itself, and a + * refetch here would race the socket's own patches for nothing. + */ +export function markChannelReadMutationOptions(queryClient: QueryClient) { + return mutationOptions({ + mutationFn: async (channelId: string) => { + await client(`/api/channels/${channelId}/read`, { + method: "PUT", + fallback: "Could not mark this channel read", + }); + }, + onMutate: (channelId) => { + const now = new Date().toISOString(); + queryClient.setQueryData( + channelKeys.list(), + (data: InfiniteData | undefined) => + data && { + ...data, + pages: data.pages.map((page) => ({ + ...page, + channels: page.channels.map((row) => + row.id === channelId + ? { + ...row, + /* + * The later of now and the row's own lastMessageAt: lastMessageAt comes from + * another clock, and a marker stamped "now" by a clock running behind it + * would leave the row still reading as unseen — and the dot still lit. + */ + lastReadAt: + row.lastMessageAt && row.lastMessageAt > now + ? row.lastMessageAt + : now, + } + : row, + ), + })), + }, + ); + }, + }); +} + /** Soft-delete a channel for everyone in it. The server keeps the transcript; the roster forgets. */ export function deleteChannelMutationOptions(queryClient: QueryClient) { return mutationOptions({ diff --git a/app/src/lib/channels/queries.ts b/app/src/lib/channels/queries.ts index 2c292946..6da66bf1 100644 --- a/app/src/lib/channels/queries.ts +++ b/app/src/lib/channels/queries.ts @@ -26,6 +26,8 @@ export type ChannelSummary = AgentChannel & { createdAt: string; /** Whether this member pinned the channel. Pinned channels sort first in the roster. */ pinned: boolean; + /** ISO-8601 when this member last had the channel open, or null for never. The caller's, only. */ + lastReadAt: string | null; }; export const channelKeys = { diff --git a/app/src/routes/_authed/_app/channel/$channelId.tsx b/app/src/routes/_authed/_app/channel/$channelId.tsx index d9030a43..1c282746 100644 --- a/app/src/routes/_authed/_app/channel/$channelId.tsx +++ b/app/src/routes/_authed/_app/channel/$channelId.tsx @@ -1,10 +1,16 @@ import { IconDeviceDesktop, IconSettings } from "@tabler/icons-react"; -import { useQuery } from "@tanstack/react-query"; +import { + useInfiniteQuery, + useMutation, + useQuery, + useQueryClient, +} from "@tanstack/react-query"; import { createFileRoute } from "@tanstack/react-router"; import { motion, useReducedMotion } from "motion/react"; import { useEffect, useRef } from "react"; import { z } from "zod"; import { AgentProfile } from "@/components/agents/agent-profile"; +import { hasUnseenActivity } from "@/components/app-sidebar/app-sidebar"; import { ChannelAvatar } from "@/components/channels/avatar"; import { ChannelChat } from "@/components/channels/channel-chat"; import { ActivityLog } from "@/components/computer/activity-log"; @@ -12,7 +18,12 @@ import { ComputerView } from "@/components/computer/computer-view"; import { useNeedsYou } from "@/components/computer/needs-you"; import { DetailPanel } from "@/components/layout/detail-panel"; import { Button } from "@/components/ui/button"; -import { type AgentChannel, channelQueryOptions } from "@/lib/channels/queries"; +import { markChannelReadMutationOptions } from "@/lib/channels/mutations"; +import { + type AgentChannel, + channelListQueryOptions, + channelQueryOptions, +} from "@/lib/channels/queries"; import { onComputerActivity } from "@/lib/copilot/computer-activity"; const chatSearchSchema = z.object({ @@ -77,6 +88,35 @@ function RouteComponent() { /** Only polled while the screen is closed; the screen panel polls control itself. */ const needsYou = useNeedsYou(agentId, !isWatching); + const queryClient = useQueryClient(); + const markRead = useMutation(markChannelReadMutationOptions(queryClient)); + /* + * This channel's roster summary, read out of the same infinite query the sidebar renders. + * The detail query deliberately knows nothing about activity; the roster is where the socket + * keeps lastMessageAt live, so it is the one honest source for "has something new been said". + */ + const roster = useInfiniteQuery(channelListQueryOptions()); + const summary = roster.data?.find((row) => row.id === channelId); + + /* + * Opening the channel marks it read; the Bot replying while it is open marks it read again. + * One effect covers both: the dep changes on navigation and on every activity patch, and the + * unseen check keeps it from writing a row per render. No dependency on the mutation object — + * its identity changes per render and the effect must not re-fire for that. + * + * Keyed on primitives, deliberately. The optimistic mark-read patch changes the summary OBJECT's + * identity without changing these values, so an object dep would re-fire the effect on its own + * write — and when lastMessageAt sits ahead of this browser's clock (another device wrote it), + * that re-fire loops into a PUT per render. Primitives hold still under the patch: one PUT. + */ + const unseen = summary !== undefined && hasUnseenActivity(summary); + const markReadMutate = markRead.mutate; + useEffect(() => { + if (unseen) { + markReadMutate(channelId); + } + }, [channelId, unseen, markReadMutate]); + /* * Needs-you prompts auto-open the screen panel, because the prompt with the reason on it — the * amber "the assistant needs you" row, and the masked field for a credential — is drawn on the diff --git a/app/src/routes/_authed/admin/route.tsx b/app/src/routes/_authed/admin/route.tsx index d83cfe74..e92175c3 100644 --- a/app/src/routes/_authed/admin/route.tsx +++ b/app/src/routes/_authed/admin/route.tsx @@ -19,12 +19,12 @@ function RouteComponent() { return ( { "This channel is defined by the deployment package, so it cannot be deleted here.", ); }); + +test("marking read PUTs the read route and patches lastReadAt in place", async () => { + const seen = capturingFetch(204, undefined); + const queryClient = new QueryClient(); + queryClient.setQueryData(channelKeys.list(), { + pages: [ + { + channels: [ + { + id: "channel-1", + name: "Assistant channel", + agentIds: ["agent-1"], + threadId: "thread-1", + active: true, + lastMessage: "hello", + lastMessageAt: "2026-08-25T12:00:00.000Z", + lastMessageAgentId: "agent-1", + createdAt: "2026-08-25T11:00:00.000Z", + pinned: false, + lastReadAt: null, + }, + ], + nextCursor: null, + }, + ], + pageParams: [""], + } satisfies InfiniteData); + const options = markChannelReadMutationOptions(queryClient); + + options.onMutate?.("channel-1"); + await options.mutationFn?.("channel-1"); + + expect(seen).toHaveLength(1); + expect(seen[0]?.url).toBe("/api/channels/channel-1/read"); + expect(seen[0]?.init?.method).toBe("PUT"); + const patched = queryClient.getQueryData>( + channelKeys.list(), + ); + // The dot clears from the cache before the wire answered, and nothing was invalidated: + // there is no onSuccess to queue a refetch that would race the socket's own patches. + expect(patched?.pages[0]?.channels[0]?.lastReadAt).not.toBeNull(); + expect(options.onSuccess).toBeUndefined(); +}); + +test("a message stamped by a clock ahead of ours still reads as seen after marking", async () => { + capturingFetch(204, undefined); + const queryClient = new QueryClient(); + const futureLastMessageAt = new Date(Date.now() + 60_000).toISOString(); + queryClient.setQueryData(channelKeys.list(), { + pages: [ + { + channels: [ + { + id: "channel-1", + name: "Assistant channel", + agentIds: ["agent-1"], + threadId: "thread-1", + active: true, + lastMessage: "hello", + lastMessageAt: futureLastMessageAt, + lastMessageAgentId: "agent-1", + createdAt: "2026-08-25T11:00:00.000Z", + pinned: false, + lastReadAt: null, + }, + ], + nextCursor: null, + }, + ], + pageParams: [""], + } satisfies InfiniteData); + const options = markChannelReadMutationOptions(queryClient); + + options.onMutate?.("channel-1"); + + const patched = queryClient.getQueryData>( + channelKeys.list(), + ); + const row = patched?.pages[0]?.channels[0]; + // A reader's clock running behind the writer's must not leave the row still reading as unseen: + // the patched lastReadAt has to catch up to (or pass) lastMessageAt, not just "now". + expect(row?.lastReadAt).not.toBeNull(); + expect((row?.lastReadAt as string) >= futureLastMessageAt).toBe(true); +}); diff --git a/app/tests/channel-order.test.ts b/app/tests/channel-order.test.ts index 3544a81a..ad283696 100644 --- a/app/tests/channel-order.test.ts +++ b/app/tests/channel-order.test.ts @@ -15,6 +15,7 @@ function channel(id: string, pinned: boolean): ChannelSummary { lastMessageAgentId: null, createdAt: "2024-01-01T00:00:00.000Z", pinned, + lastReadAt: null, }; } diff --git a/app/tests/channel-unread.test.ts b/app/tests/channel-unread.test.ts new file mode 100644 index 00000000..40192e3e --- /dev/null +++ b/app/tests/channel-unread.test.ts @@ -0,0 +1,62 @@ +import { expect, test } from "bun:test"; +import { + hasUnseenActivity, + isUnread, +} from "../src/components/app-sidebar/app-sidebar"; +import type { ChannelSummary } from "../src/lib/channels/queries"; + +/** A minimal but fully-typed summary, so tests build real objects rather than casts. */ +function channel(overrides: Partial): ChannelSummary { + return { + id: "channel-1", + name: "Assistant channel", + agentIds: ["agent-1"], + threadId: "thread-1", + active: true, + lastMessage: "hello", + lastMessageAt: "2026-08-25T12:00:00.000Z", + lastMessageAgentId: "agent-1", + createdAt: "2026-08-25T11:00:00.000Z", + pinned: false, + lastReadAt: null, + ...overrides, + }; +} + +test("a Bot message in a never-opened channel is unseen", () => { + expect(hasUnseenActivity(channel({}))).toBe(true); +}); + +test("a Bot message newer than the read marker is unseen", () => { + expect( + hasUnseenActivity(channel({ lastReadAt: "2026-08-25T11:30:00.000Z" })), + ).toBe(true); +}); + +test("a read marker after the last message means nothing is unseen", () => { + expect( + hasUnseenActivity(channel({ lastReadAt: "2026-08-25T12:30:00.000Z" })), + ).toBe(false); +}); + +test("your own last message never counts as unseen", () => { + expect(hasUnseenActivity(channel({ lastMessageAgentId: null }))).toBe(false); +}); + +test("a silent channel has nothing unseen", () => { + expect( + hasUnseenActivity( + channel({ + lastMessage: null, + lastMessageAt: null, + lastMessageAgentId: null, + }), + ), + ).toBe(false); +}); + +test("the open channel is never unread, however unseen its activity", () => { + expect(isUnread(channel({}), "channel-1")).toBe(false); + expect(isUnread(channel({}), "channel-2")).toBe(true); + expect(isUnread(channel({}), undefined)).toBe(true); +}); diff --git a/server/drizzle/0019_channel_read_marker.sql b/server/drizzle/0019_channel_read_marker.sql new file mode 100644 index 00000000..81128352 --- /dev/null +++ b/server/drizzle/0019_channel_read_marker.sql @@ -0,0 +1 @@ +ALTER TABLE "channel_memberships" ADD COLUMN "last_read_at" timestamp with time zone; \ No newline at end of file diff --git a/server/drizzle/meta/0019_snapshot.json b/server/drizzle/meta/0019_snapshot.json new file mode 100644 index 00000000..e899ec13 --- /dev/null +++ b/server/drizzle/meta/0019_snapshot.json @@ -0,0 +1,2583 @@ +{ + "id": "9f46b81a-bfe4-4c29-ab95-e08d00506767", + "prevId": "aa5ec39b-170c-495b-b4a9-e08ed0fd643d", + "version": "7", + "dialect": "postgresql", + "tables": { + "public.accounts": { + "name": "accounts", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "account_id": { + "name": "account_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "provider_id": { + "name": "provider_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "issuer": { + "name": "issuer", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "access_token": { + "name": "access_token", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "refresh_token": { + "name": "refresh_token", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "id_token": { + "name": "id_token", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "access_token_expires_at": { + "name": "access_token_expires_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "refresh_token_expires_at": { + "name": "refresh_token_expires_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "scope": { + "name": "scope", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "password": { + "name": "password", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "accounts_provider_account_idx": { + "name": "accounts_provider_account_idx", + "columns": [ + { + "expression": "provider_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "account_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "accounts_user_id_users_id_fk": { + "name": "accounts_user_id_users_id_fk", + "tableFrom": "accounts", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.agents": { + "name": "agents", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "type": { + "name": "type", + "type": "agent_type", + "typeSchema": "public", + "primaryKey": false, + "notNull": true + }, + "configuration": { + "name": "configuration", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "package_id": { + "name": "package_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "override": { + "name": "override", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "agents_package_id_deployment_packages_id_fk": { + "name": "agents_package_id_deployment_packages_id_fk", + "tableFrom": "agents", + "tableTo": "deployment_packages", + "columnsFrom": ["package_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.audit_events": { + "name": "audit_events", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "actor_user_id": { + "name": "actor_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "event_type": { + "name": "event_type", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "target_type": { + "name": "target_type", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "target_id": { + "name": "target_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "payload": { + "name": "payload", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "audit_events_created_at_idx": { + "name": "audit_events_created_at_idx", + "columns": [ + { + "expression": "created_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "audit_events_type_time_idx": { + "name": "audit_events_type_time_idx", + "columns": [ + { + "expression": "event_type", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "created_at", + "isExpression": false, + "asc": false, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": false, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "audit_events_actor_time_idx": { + "name": "audit_events_actor_time_idx", + "columns": [ + { + "expression": "actor_user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "created_at", + "isExpression": false, + "asc": false, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": false, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "audit_events_target_time_idx": { + "name": "audit_events_target_time_idx", + "columns": [ + { + "expression": "target_type", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "target_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "created_at", + "isExpression": false, + "asc": false, + "nulls": "last" + }, + { + "expression": "id", + "isExpression": false, + "asc": false, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.channel_agents": { + "name": "channel_agents", + "schema": "", + "columns": { + "channel_id": { + "name": "channel_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "agent_id": { + "name": "agent_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "channel_agents_channel_id_channels_id_fk": { + "name": "channel_agents_channel_id_channels_id_fk", + "tableFrom": "channel_agents", + "tableTo": "channels", + "columnsFrom": ["channel_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "channel_agents_agent_id_agents_id_fk": { + "name": "channel_agents_agent_id_agents_id_fk", + "tableFrom": "channel_agents", + "tableTo": "agents", + "columnsFrom": ["agent_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "channel_agents_channel_id_agent_id_pk": { + "name": "channel_agents_channel_id_agent_id_pk", + "columns": ["channel_id", "agent_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.channel_memberships": { + "name": "channel_memberships", + "schema": "", + "columns": { + "channel_id": { + "name": "channel_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "pinned_at": { + "name": "pinned_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "last_read_at": { + "name": "last_read_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "channel_memberships_channel_id_channels_id_fk": { + "name": "channel_memberships_channel_id_channels_id_fk", + "tableFrom": "channel_memberships", + "tableTo": "channels", + "columnsFrom": ["channel_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "channel_memberships_user_id_users_id_fk": { + "name": "channel_memberships_user_id_users_id_fk", + "tableFrom": "channel_memberships", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "channel_memberships_channel_id_user_id_pk": { + "name": "channel_memberships_channel_id_user_id_pk", + "columns": ["channel_id", "user_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.channels": { + "name": "channels", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "suggested_prompts": { + "name": "suggested_prompts", + "type": "text[]", + "primaryKey": false, + "notNull": true, + "default": "'{}'" + }, + "allowed_groups": { + "name": "allowed_groups", + "type": "text[]", + "primaryKey": false, + "notNull": true, + "default": "'{}'" + }, + "package_id": { + "name": "package_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "override": { + "name": "override", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, + "last_message": { + "name": "last_message", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "last_message_at": { + "name": "last_message_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "last_message_agent_id": { + "name": "last_message_agent_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "deleted_at": { + "name": "deleted_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "channels_recent_activity_idx": { + "name": "channels_recent_activity_idx", + "columns": [ + { + "expression": "COALESCE(\"last_message_at\", \"created_at\") DESC", + "asc": true, + "isExpression": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "channels_package_id_deployment_packages_id_fk": { + "name": "channels_package_id_deployment_packages_id_fk", + "tableFrom": "channels", + "tableTo": "deployment_packages", + "columnsFrom": ["package_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + }, + "channels_last_message_agent_id_agents_id_fk": { + "name": "channels_last_message_agent_id_agents_id_fk", + "tableFrom": "channels", + "tableTo": "agents", + "columnsFrom": ["last_message_agent_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.credentials": { + "name": "credentials", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "kind": { + "name": "kind", + "type": "credential_kind", + "typeSchema": "public", + "primaryKey": false, + "notNull": true + }, + "provider": { + "name": "provider", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "encrypted_value": { + "name": "encrypted_value", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "key_id": { + "name": "key_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "metadata": { + "name": "metadata", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "revoked_at": { + "name": "revoked_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "credentials_active_key_idx": { + "name": "credentials_active_key_idx", + "columns": [ + { + "expression": "kind", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "provider", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "key_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "where": "\"credentials\".\"revoked_at\" IS NULL", + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.deployment_packages": { + "name": "deployment_packages", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "tenant_id": { + "name": "tenant_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "source_path": { + "name": "source_path", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "checksum": { + "name": "checksum", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "loaded_at": { + "name": "loaded_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "deployment_packages_tenant_id_unique": { + "name": "deployment_packages_tenant_id_unique", + "nullsNotDistinct": false, + "columns": ["tenant_id"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.intelligence_channel_mappings": { + "name": "intelligence_channel_mappings", + "schema": "", + "columns": { + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "channel_id": { + "name": "channel_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "thread_id": { + "name": "thread_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "intelligence_channel_mappings_thread_idx": { + "name": "intelligence_channel_mappings_thread_idx", + "columns": [ + { + "expression": "thread_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "intelligence_channel_mappings_user_id_users_id_fk": { + "name": "intelligence_channel_mappings_user_id_users_id_fk", + "tableFrom": "intelligence_channel_mappings", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "intelligence_channel_mappings_channel_id_channels_id_fk": { + "name": "intelligence_channel_mappings_channel_id_channels_id_fk", + "tableFrom": "intelligence_channel_mappings", + "tableTo": "channels", + "columnsFrom": ["channel_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "intelligence_channel_mappings_user_id_channel_id_pk": { + "name": "intelligence_channel_mappings_user_id_channel_id_pk", + "columns": ["user_id", "channel_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.revoked_access": { + "name": "revoked_access", + "schema": "", + "columns": { + "email": { + "name": "email", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "revoked_at": { + "name": "revoked_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "revoked_by": { + "name": "revoked_by", + "type": "text", + "primaryKey": false, + "notNull": true + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.sessions": { + "name": "sessions", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "token": { + "name": "token", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "expires_at": { + "name": "expires_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true + }, + "ip_address": { + "name": "ip_address", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "user_agent": { + "name": "user_agent", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "sessions_user_id_users_id_fk": { + "name": "sessions_user_id_users_id_fk", + "tableFrom": "sessions", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "sessions_token_unique": { + "name": "sessions_token_unique", + "nullsNotDistinct": false, + "columns": ["token"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.sso_providers": { + "name": "sso_providers", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "issuer": { + "name": "issuer", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "oidc_config": { + "name": "oidc_config", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "saml_config": { + "name": "saml_config", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "provider_id": { + "name": "provider_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "organization_id": { + "name": "organization_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "domain": { + "name": "domain", + "type": "text", + "primaryKey": false, + "notNull": true + } + }, + "indexes": {}, + "foreignKeys": { + "sso_providers_user_id_users_id_fk": { + "name": "sso_providers_user_id_users_id_fk", + "tableFrom": "sso_providers", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "sso_providers_provider_id_unique": { + "name": "sso_providers_provider_id_unique", + "nullsNotDistinct": false, + "columns": ["provider_id"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.user_roles": { + "name": "user_roles", + "schema": "", + "columns": { + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "role": { + "name": "role", + "type": "role", + "typeSchema": "public", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "user_roles_user_id_users_id_fk": { + "name": "user_roles_user_id_users_id_fk", + "tableFrom": "user_roles", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "user_roles_user_id_role_pk": { + "name": "user_roles_user_id_role_pk", + "columns": ["user_id", "role"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.users": { + "name": "users", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "image": { + "name": "image", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "email_verified": { + "name": "email_verified", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "groups": { + "name": "groups", + "type": "text[]", + "primaryKey": false, + "notNull": true, + "default": "'{}'" + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": { + "users_email_unique": { + "name": "users_email_unique", + "nullsNotDistinct": false, + "columns": ["email"] + } + }, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.verifications": { + "name": "verifications", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "identifier": { + "name": "identifier", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "value": { + "name": "value", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "expires_at": { + "name": "expires_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.action_policy": { + "name": "action_policy", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "mode": { + "name": "mode", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "deny": { + "name": "deny", + "type": "text[]", + "primaryKey": false, + "notNull": true + }, + "allow": { + "name": "allow", + "type": "text[]", + "primaryKey": false, + "notNull": true + }, + "updated_by": { + "name": "updated_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.computer_page_frame": { + "name": "computer_page_frame", + "schema": "", + "columns": { + "computer_id": { + "name": "computer_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "tool_call_id": { + "name": "tool_call_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "frame": { + "name": "frame", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "captured_at": { + "name": "captured_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "computer_page_frame_captured_idx": { + "name": "computer_page_frame_captured_idx", + "columns": [ + { + "expression": "captured_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": { + "computer_page_frame_computer_id_tool_call_id_pk": { + "name": "computer_page_frame_computer_id_tool_call_id_pk", + "columns": ["computer_id", "tool_call_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.computer_snapshot": { + "name": "computer_snapshot", + "schema": "", + "columns": { + "computer_id": { + "name": "computer_id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "snapshot_id": { + "name": "snapshot_id", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "elements": { + "name": "elements", + "type": "jsonb", + "primaryKey": false, + "notNull": true + }, + "taken_at": { + "name": "taken_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "session": { + "name": "session", + "type": "text", + "primaryKey": false, + "notNull": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.agent_preferences": { + "name": "agent_preferences", + "schema": "", + "columns": { + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "agent_id": { + "name": "agent_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "hidden_at": { + "name": "hidden_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + } + }, + "indexes": {}, + "foreignKeys": { + "agent_preferences_user_id_users_id_fk": { + "name": "agent_preferences_user_id_users_id_fk", + "tableFrom": "agent_preferences", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "agent_preferences_agent_id_agents_id_fk": { + "name": "agent_preferences_agent_id_agents_id_fk", + "tableFrom": "agent_preferences", + "tableTo": "agents", + "columnsFrom": ["agent_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "agent_preferences_user_id_agent_id_pk": { + "name": "agent_preferences_user_id_agent_id_pk", + "columns": ["user_id", "agent_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.agent_profiles": { + "name": "agent_profiles", + "schema": "", + "columns": { + "agent_id": { + "name": "agent_id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "role_description": { + "name": "role_description", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "avatar_seed": { + "name": "avatar_seed", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "visibility": { + "name": "visibility", + "type": "agent_visibility", + "typeSchema": "public", + "primaryKey": false, + "notNull": true + }, + "callback_token_hash": { + "name": "callback_token_hash", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "callback_token_issued_at": { + "name": "callback_token_issued_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "deleted_at": { + "name": "deleted_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "agent_profiles_visibility_deleted_idx": { + "name": "agent_profiles_visibility_deleted_idx", + "columns": [ + { + "expression": "visibility", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "deleted_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "agent_profiles_agent_id_agents_id_fk": { + "name": "agent_profiles_agent_id_agents_id_fk", + "tableFrom": "agent_profiles", + "tableTo": "agents", + "columnsFrom": ["agent_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "agent_profiles_owner_user_id_users_id_fk": { + "name": "agent_profiles_owner_user_id_users_id_fk", + "tableFrom": "agent_profiles", + "tableTo": "users", + "columnsFrom": ["owner_user_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.component_exclusions": { + "name": "component_exclusions", + "schema": "", + "columns": { + "component_name": { + "name": "component_name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "agent_id": { + "name": "agent_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "withheld_by": { + "name": "withheld_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "component_exclusions_component_name_components_name_fk": { + "name": "component_exclusions_component_name_components_name_fk", + "tableFrom": "component_exclusions", + "tableTo": "components", + "columnsFrom": ["component_name"], + "columnsTo": ["name"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "component_exclusions_agent_id_agents_id_fk": { + "name": "component_exclusions_agent_id_agents_id_fk", + "tableFrom": "component_exclusions", + "tableTo": "agents", + "columnsFrom": ["agent_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "component_exclusions_component_name_agent_id_pk": { + "name": "component_exclusions_component_name_agent_id_pk", + "columns": ["component_name", "agent_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.component_functions": { + "name": "component_functions", + "schema": "", + "columns": { + "component_name": { + "name": "component_name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "function_name": { + "name": "function_name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "granted_by": { + "name": "granted_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "component_functions_component_name_components_name_fk": { + "name": "component_functions_component_name_components_name_fk", + "tableFrom": "component_functions", + "tableTo": "components", + "columnsFrom": ["component_name"], + "columnsTo": ["name"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "component_functions_component_name_function_name_pk": { + "name": "component_functions_component_name_function_name_pk", + "columns": ["component_name", "function_name"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.components": { + "name": "components", + "schema": "", + "columns": { + "name": { + "name": "name", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "kind": { + "name": "kind", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "draft_description": { + "name": "draft_description", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "published_description": { + "name": "published_description", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "published": { + "name": "published", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "published_at": { + "name": "published_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "updated_by": { + "name": "updated_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.mcp_servers": { + "name": "mcp_servers", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "vendor": { + "name": "vendor", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "provenance": { + "name": "provenance", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "'first-party'" + }, + "credential_id": { + "name": "credential_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "tools_refreshed_at": { + "name": "tools_refreshed_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "last_error": { + "name": "last_error", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "added_by": { + "name": "added_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "mcp_servers_credential_id_credentials_id_fk": { + "name": "mcp_servers_credential_id_credentials_id_fk", + "tableFrom": "mcp_servers", + "tableTo": "credentials", + "columnsFrom": ["credential_id"], + "columnsTo": ["id"], + "onDelete": "restrict", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.mcp_tools": { + "name": "mcp_tools", + "schema": "", + "columns": { + "server_id": { + "name": "server_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "input_schema": { + "name": "input_schema", + "type": "jsonb", + "primaryKey": false, + "notNull": true, + "default": "'{}'::jsonb" + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": { + "mcp_tools_server_id_mcp_servers_id_fk": { + "name": "mcp_tools_server_id_mcp_servers_id_fk", + "tableFrom": "mcp_tools", + "tableTo": "mcp_servers", + "columnsFrom": ["server_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "mcp_tools_server_id_name_pk": { + "name": "mcp_tools_server_id_name_pk", + "columns": ["server_id", "name"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.mcp_user_credentials": { + "name": "mcp_user_credentials", + "schema": "", + "columns": { + "server_id": { + "name": "server_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "credential_id": { + "name": "credential_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "scope": { + "name": "scope", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "connected_at": { + "name": "connected_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "mcp_user_credentials_user_idx": { + "name": "mcp_user_credentials_user_idx", + "columns": [ + { + "expression": "user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "mcp_user_credentials_server_id_mcp_servers_id_fk": { + "name": "mcp_user_credentials_server_id_mcp_servers_id_fk", + "tableFrom": "mcp_user_credentials", + "tableTo": "mcp_servers", + "columnsFrom": ["server_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "mcp_user_credentials_user_id_users_id_fk": { + "name": "mcp_user_credentials_user_id_users_id_fk", + "tableFrom": "mcp_user_credentials", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "mcp_user_credentials_credential_id_credentials_id_fk": { + "name": "mcp_user_credentials_credential_id_credentials_id_fk", + "tableFrom": "mcp_user_credentials", + "tableTo": "credentials", + "columnsFrom": ["credential_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "mcp_user_credentials_server_id_user_id_pk": { + "name": "mcp_user_credentials_server_id_user_id_pk", + "columns": ["server_id", "user_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.plugin_grants": { + "name": "plugin_grants", + "schema": "", + "columns": { + "kind": { + "name": "kind", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "ref": { + "name": "ref", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "agent_id": { + "name": "agent_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "granted_by": { + "name": "granted_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "plugin_grants_agent_idx": { + "name": "plugin_grants_agent_idx", + "columns": [ + { + "expression": "agent_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "plugin_grants_agent_id_agents_id_fk": { + "name": "plugin_grants_agent_id_agents_id_fk", + "tableFrom": "plugin_grants", + "tableTo": "agents", + "columnsFrom": ["agent_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "plugin_grants_kind_ref_agent_id_pk": { + "name": "plugin_grants_kind_ref_agent_id_pk", + "columns": ["kind", "ref", "agent_id"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.sandboxed_components": { + "name": "sandboxed_components", + "schema": "", + "columns": { + "name": { + "name": "name", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "draft_description": { + "name": "draft_description", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "draft_html": { + "name": "draft_html", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "draft_css": { + "name": "draft_css", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "draft_js_functions": { + "name": "draft_js_functions", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "draft_argument_schema": { + "name": "draft_argument_schema", + "type": "jsonb", + "primaryKey": false, + "notNull": true, + "default": "'{}'::jsonb" + }, + "published_description": { + "name": "published_description", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "published_html": { + "name": "published_html", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "published_css": { + "name": "published_css", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "published_js_functions": { + "name": "published_js_functions", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "published_argument_schema": { + "name": "published_argument_schema", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, + "sample_arguments": { + "name": "sample_arguments", + "type": "jsonb", + "primaryKey": false, + "notNull": true, + "default": "'{}'::jsonb" + }, + "revision": { + "name": "revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + }, + "published": { + "name": "published", + "type": "boolean", + "primaryKey": false, + "notNull": true, + "default": false + }, + "published_at": { + "name": "published_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "authored_by": { + "name": "authored_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.skill_tools": { + "name": "skill_tools", + "schema": "", + "columns": { + "skill_id": { + "name": "skill_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "ref": { + "name": "ref", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "declared_by": { + "name": "declared_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "skill_tools_ref_idx": { + "name": "skill_tools_ref_idx", + "columns": [ + { + "expression": "ref", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "skill_tools_skill_id_skills_id_fk": { + "name": "skill_tools_skill_id_skills_id_fk", + "tableFrom": "skill_tools", + "tableTo": "skills", + "columnsFrom": ["skill_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "skill_tools_skill_id_ref_pk": { + "name": "skill_tools_skill_id_ref_pk", + "columns": ["skill_id", "ref"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.skills": { + "name": "skills", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "slug": { + "name": "slug", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "summary": { + "name": "summary", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "instructions": { + "name": "instructions", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "origin": { + "name": "origin", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "'yours'" + }, + "installed_by": { + "name": "installed_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "skills_slug_key": { + "name": "skills_slug_key", + "columns": [ + { + "expression": "slug", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "skills_owner_idx": { + "name": "skills_owner_idx", + "columns": [ + { + "expression": "owner_user_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "skills_owner_user_id_users_id_fk": { + "name": "skills_owner_user_id_users_id_fk", + "tableFrom": "skills", + "tableTo": "users", + "columnsFrom": ["owner_user_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, + "public.work_items": { + "name": "work_items", + "schema": "", + "columns": { + "kind": { + "name": "kind", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "key": { + "name": "key", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "run_at": { + "name": "run_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "claimed_by": { + "name": "claimed_by", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "lease_until": { + "name": "lease_until", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "attempts": { + "name": "attempts", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + }, + "finished_at": { + "name": "finished_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "last_error": { + "name": "last_error", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "payload": { + "name": "payload", + "type": "jsonb", + "primaryKey": false, + "notNull": true, + "default": "'{}'::jsonb" + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "work_items_claimable_idx": { + "name": "work_items_claimable_idx", + "columns": [ + { + "expression": "kind", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "run_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": { + "work_items_kind_key_pk": { + "name": "work_items_kind_key_pk", + "columns": ["kind", "key"] + } + }, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + } + }, + "enums": { + "public.agent_type": { + "name": "agent_type", + "schema": "public", + "values": ["built_in", "remote_ag_ui"] + }, + "public.credential_kind": { + "name": "credential_kind", + "schema": "public", + "values": [ + "model", + "connector", + "agent", + "mcp", + "mcp_oauth_client", + "mcp_user_token" + ] + }, + "public.role": { + "name": "role", + "schema": "public", + "values": ["admin", "user"] + }, + "public.agent_visibility": { + "name": "agent_visibility", + "schema": "public", + "values": ["public", "private"] + } + }, + "schemas": {}, + "sequences": {}, + "roles": {}, + "policies": {}, + "views": {}, + "_meta": { + "columns": {}, + "schemas": {}, + "tables": {} + } +} diff --git a/server/drizzle/meta/_journal.json b/server/drizzle/meta/_journal.json index 8b13de0c..b2ebb2ae 100644 --- a/server/drizzle/meta/_journal.json +++ b/server/drizzle/meta/_journal.json @@ -134,6 +134,13 @@ "when": 1787688017645, "tag": "0018_page_frames", "breakpoints": true + }, + { + "idx": 19, + "version": "7", + "when": 1787744867526, + "tag": "0019_channel_read_marker", + "breakpoints": true } ] } diff --git a/server/src/channels/routes.ts b/server/src/channels/routes.ts index 0a1061d7..0d10a13a 100644 --- a/server/src/channels/routes.ts +++ b/server/src/channels/routes.ts @@ -51,6 +51,8 @@ export type ChannelSummary = AgentChannel & { createdAt: Date; /** Whether the caller pinned this channel. A pin is per-member, so this is the caller's, only. */ pinned: boolean; + /** When the caller last had this channel open, or null for never. The caller's, only. */ + lastReadAt: Date | null; }; /** What a client that ran an agent reports back about the message it just saw. */ @@ -152,6 +154,8 @@ export type ChannelStore = { channelId: string, pinned: boolean, ): Promise; + /** Stamp the caller's own membership as read now. Throws ChannelNotFoundError for a non-member. */ + markRead(actor: AgentActor, channelId: string): Promise; /** * Hide the channel for every member. Soft: the row and the thread survive, every read filters. * Throws ChannelNotFoundError for a non-member and ChannelPackageOwnedError for a channel the @@ -366,6 +370,7 @@ export function createChannelStore( lastMessageAgentId: channels.lastMessageAgentId, createdAt: channels.createdAt, pinnedAt: channelMemberships.pinnedAt, + lastReadAt: channelMemberships.lastReadAt, }) .from(channels) .innerJoin( @@ -423,6 +428,7 @@ export function createChannelStore( lastMessageAgentId: row.lastMessageAgentId, createdAt: row.createdAt, pinned: row.pinnedAt !== null, + lastReadAt: row.lastReadAt, }); } return { channels: [...summaries.values()], nextCursor }; @@ -482,6 +488,39 @@ export function createChannelStore( ); }, + async markRead(actor, channelId) { + const updated = await database + .update(channelMemberships) + .set({ + /* + * The later of this clock and the channel's own last-message stamp. last_message_at is + * written from the reporting browser's clock and is not bounded; a marker stamped + * plainly "now" by a server running behind it would leave the row reading as unseen for + * every member, re-lighting the dot on each refetch until wall clock catches up. + */ + lastReadAt: sql`greatest(now(), coalesce((select ${channels.lastMessageAt} from ${channels} where ${channels.id} = ${channelMemberships.channelId}), now()))`, + }) + .where( + and( + eq(channelMemberships.channelId, channelId), + eq(channelMemberships.userId, actor.id), + // A deleted channel is not there to read. The same guard `setPinned` carries, for the + // same reason: the row is gone from every roster, so nothing about it is markable. + exists( + database + .select({ one: sql`1` }) + .from(channels) + .where( + and(eq(channels.id, channelId), isNull(channels.deletedAt)), + ), + ), + ), + ) + .returning({ channelId: channelMemberships.channelId }); + // Not a member, or no such channel: the same answer either way, matching setPinned. + if (updated.length === 0) throw new ChannelNotFoundError(channelId); + }, + async softDelete(actor, channelId) { await database.transaction( async (transaction) => { @@ -874,6 +913,15 @@ export function createChannelRoutes( } }); + routes.put("/:channelId/read", requireUser, async (context) => { + try { + await store.markRead(context.var.actor, context.req.param("channelId")); + return context.body(null, 204); + } catch (error) { + return mapStoreError(context, error); + } + }); + routes.delete("/:channelId", requireUser, async (context) => { const channelId = context.req.param("channelId"); try { @@ -922,6 +970,8 @@ function channelSummaryDto(channel: ChannelSummary) { lastMessageAgentId: channel.lastMessageAgentId, createdAt: channel.createdAt.toISOString(), pinned: channel.pinned, + // Serialised as ISO-8601 like lastMessageAt, so the browser can compare the two as strings. + lastReadAt: channel.lastReadAt?.toISOString() ?? null, }; } diff --git a/server/src/db/schema/core.ts b/server/src/db/schema/core.ts index 617ba97f..f8869b26 100644 --- a/server/src/db/schema/core.ts +++ b/server/src/db/schema/core.ts @@ -308,6 +308,11 @@ export const channelMemberships = pgTable( * one person's marker, and the membership row is already the per-member half of a channel. */ pinnedAt: timestamp("pinned_at", { withTimezone: true }), + /** + * When this member last had the channel open, or null for never. On the membership like the + * pin: reading is one person's act, and the unread marker it feeds is that person's alone. + */ + lastReadAt: timestamp("last_read_at", { withTimezone: true }), createdAt: createdAt(), }, (table) => [primaryKey({ columns: [table.channelId, table.userId] })], diff --git a/server/tests/channel-activity.integration.test.ts b/server/tests/channel-activity.integration.test.ts index 5b0b155c..a592eb88 100644 --- a/server/tests/channel-activity.integration.test.ts +++ b/server/tests/channel-activity.integration.test.ts @@ -228,6 +228,7 @@ describe("channel activity", () => { lastMessageAt: at, createdAt: expect.any(Date), pinned: false, + lastReadAt: null, }, ]); }); diff --git a/server/tests/channel-routes.test.ts b/server/tests/channel-routes.test.ts index a43275b0..7019b1f9 100644 --- a/server/tests/channel-routes.test.ts +++ b/server/tests/channel-routes.test.ts @@ -79,6 +79,9 @@ function fakeStore( async setPinned(receivedActor, id, pinned) { calls.push(["setPinned", receivedActor, id, pinned]); }, + async markRead(receivedActor, id) { + calls.push(["markRead", receivedActor, id]); + }, async softDelete(receivedActor, id) { calls.push(["softDelete", receivedActor, id]); }, @@ -360,6 +363,45 @@ describe("channel routes", () => { expect(store.calls).toEqual([]); }); + test("marks read through the authenticated actor and answers 204", async () => { + const store = fakeStore(); + const response = await appFor(store).request( + "http://openbot.test/channel-1/read", + { method: "PUT" }, + ); + + expect(response.status).toBe(204); + expect(store.calls).toEqual([["markRead", actor, "channel-1"]]); + }); + + test("maps an unknown channel to 404 for marking read", async () => { + const store = fakeStore({ + markRead: async () => { + throw new ChannelNotFoundError("channel-1"); + }, + }); + const response = await appFor(store).request( + "http://openbot.test/channel-1/read", + { method: "PUT" }, + ); + + expect(response.status).toBe(404); + expect(await json(response)).toEqual({ error: "Channel not found." }); + }); + + test("keeps authentication in front of marking read", async () => { + const store = fakeStore(); + const denied: MiddlewareHandler<{ Variables: AppVariables }> = (context) => + Promise.resolve(context.json({ error: "denied" }, 401)); + const response = await appFor(store, denied).request( + "http://openbot.test/channel-1/read", + { method: "PUT" }, + ); + + expect(response.status).toBe(401); + expect(store.calls).toEqual([]); + }); + test("deletes through the authenticated actor and answers 204", async () => { const store = fakeStore(); const response = await appFor(store).request( @@ -1115,6 +1157,127 @@ describe("channel pinning", () => { }); }); +describe("channel read markers", () => { + // Two members of one channel, which is what a per-member marker has to be tested against. + async function sharedChannel() { + const reader = await createPersistentUser(); + const other = await createPersistentUser(); + const agentId = await createPersistentAgent({ + name: "Shared readable agent", + owner: reader, + visibility: "public", + }); + const created = await persistentStore.create(reader, [agentId]); + createdChannelIds.push(created.id); + // The store only creates the creator's membership; give the other user one directly, + // plus the thread mapping the list join requires. + await database.insert(channelMemberships).values({ + channelId: created.id, + userId: other.id, + }); + await database.insert(intelligenceChannelMappings).values({ + userId: other.id, + channelId: created.id, + // thread_id is globally unique; the reader's own mapping row already claimed + // created.threadId, so the other member's row needs one of its own. + threadId: randomUUID(), + }); + return { reader, other, channelId: created.id }; + } + + test("stamps last_read_at on the caller's own membership only", async () => { + const { reader, other, channelId } = await sharedChannel(); + + await persistentStore.markRead(reader, channelId); + + const rows = await database + .select({ + userId: channelMemberships.userId, + lastReadAt: channelMemberships.lastReadAt, + }) + .from(channelMemberships) + .where(eq(channelMemberships.channelId, channelId)); + expect( + rows.find((row) => row.userId === reader.id)?.lastReadAt, + ).not.toBeNull(); + expect(rows.find((row) => row.userId === other.id)?.lastReadAt).toBeNull(); + }); + + test("the list carries the caller's lastReadAt and nobody else's", async () => { + const { reader, other, channelId } = await sharedChannel(); + + await persistentStore.markRead(reader, channelId); + + const forReader = await persistentStore.list(reader); + const forOther = await persistentStore.list(other); + expect( + forReader.channels.find((channel) => channel.id === channelId) + ?.lastReadAt, + ).not.toBeNull(); + expect( + forOther.channels.find((channel) => channel.id === channelId)?.lastReadAt, + ).toBeNull(); + }); + + test("refuses to mark read a channel the caller is not a member of", async () => { + const { channelId } = await sharedChannel(); + const outsider = await createPersistentUser(); + + await expect( + persistentStore.markRead(outsider, channelId), + ).rejects.toBeInstanceOf(ChannelNotFoundError); + }); + + test("stamps a read no earlier than the channel's own last-message clock", async () => { + const { reader, channelId } = await sharedChannel(); + // last_message_at is written from the reporting browser's clock and is not bounded; simulate + // one running ahead of the server so a plain "now" stamp would still read as unseen. + const future = new Date(Date.now() + 60_000); + await database + .update(channels) + .set({ lastMessageAt: future }) + .where(eq(channels.id, channelId)); + + await persistentStore.markRead(reader, channelId); + + const [row] = await database + .select({ lastReadAt: channelMemberships.lastReadAt }) + .from(channelMemberships) + .where( + and( + eq(channelMemberships.channelId, channelId), + eq(channelMemberships.userId, reader.id), + ), + ); + expect(row?.lastReadAt).not.toBeNull(); + expect(row?.lastReadAt?.getTime() ?? 0).toBeGreaterThanOrEqual( + future.getTime(), + ); + }); + + test("refuses to mark a soft-deleted channel read, mirroring setPinned", async () => { + const { reader, channelId } = await sharedChannel(); + + await persistentStore.softDelete(reader, channelId); + + await expect( + persistentStore.markRead(reader, channelId), + ).rejects.toBeInstanceOf(ChannelNotFoundError); + + const [row] = await database + .select({ lastReadAt: channelMemberships.lastReadAt }) + .from(channelMemberships) + .where( + and( + eq(channelMemberships.channelId, channelId), + eq(channelMemberships.userId, reader.id), + ), + ); + // The membership row outlives the channel, but its marker was never stamped. + expect(row?.lastReadAt).toBeNull(); + }); +}); + describe("channel soft delete", () => { test("hides a deleted channel from list and get", async () => { const actor = await createPersistentUser(); From b94138548a3f55869434cf22a49d6b1f0e5fb278 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:13:25 -0700 Subject: [PATCH 04/10] Update azure/setup-helm action to v5 (#260) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c79e8f1..a5550af0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - uses: azure/setup-helm@b9e51907a09c216f16ebe8536097933489208112 # v4.3.0 + - uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5.0.1 with: version: v3.19.0 # For the coherence check below, which is a Bun script like everything else here. From a4549bee80a495db8deb73a815ad6f85e5236710 Mon Sep 17 00:00:00 2001 From: Vaibhav Zope <121456155+zopeVaibhav@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:43:41 +0530 Subject: [PATCH 05/10] Say what a strict content-security-policy has to allow (#225) --- docs/deployment.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/deployment.md b/docs/deployment.md index 830827d0..14164094 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -135,3 +135,9 @@ which makes them the shortest path from nothing to a running deployment. **The image is 5.3 GB**, most of it the Playwright base, which ships Firefox and WebKit alongside the Chromium we use. Deleting them afterwards does not help, because the bytes still ship in the layer below. Building Chromium-only onto a slim base would cut this substantially and is not done yet. + +**A strict content-security-policy needs a hash or a nonce.** `app/index.html` runs a small inline +script that decides the theme before the first paint. Nothing in this repo sends a CSP header, so it +works as shipped; a deployment that adds one at its proxy has to allow that script explicitly, or +`script-src` blocks it and the page renders with the wrong theme until the app boots. A `'sha256-'` +hash of the script body is the version that survives a rebuild without a per-request nonce. From 951d20f62e2dbc2b097ff32c57da07b85cb18401 Mon Sep 17 00:00:00 2001 From: Vaibhav Zope <121456155+zopeVaibhav@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:43:44 +0530 Subject: [PATCH 06/10] Point the test at the database the project actually has (#234) --- server/tests/server-side-tools.integration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tests/server-side-tools.integration.test.ts b/server/tests/server-side-tools.integration.test.ts index d0fbffd4..eb5c58bc 100644 --- a/server/tests/server-side-tools.integration.test.ts +++ b/server/tests/server-side-tools.integration.test.ts @@ -31,7 +31,7 @@ import { TEST_POOL } from "./support/database"; const database = createDatabase( process.env.DATABASE_URL ?? - "postgres://openkai:openkai@localhost:5432/openkai", + "postgres://openbot:openbot@localhost:5432/openbot", TEST_POOL, ); From c0638c7ad19b818ca90ed513653b1994ba35a83f Mon Sep 17 00:00:00 2001 From: Vaibhav Zope <121456155+zopeVaibhav@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:45:19 +0530 Subject: [PATCH 07/10] Let the API reach Intelligence and sign-in when a NetworkPolicy is on (#257) --- CHANGELOG.md | 19 ++++++++ charts/openbot/templates/networkpolicy.yaml | 50 +++++++++++++++++++-- charts/openbot/values.yaml | 10 +++++ scripts/check-rendered-chart.ts | 28 ++++++++++++ 4 files changed, 103 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bdc7671..98fc7192 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,25 @@ the membership row like the pin — so one person reading does not clear anybody The deployment gains one nullable column, via migration `0019`. +### The API can reach Intelligence and sign-in when a NetworkPolicy is on + +`networkPolicy.enabled` wrote a rule for the API server that named DNS, the database and the Bots' +computers, and nothing on 443. On a cluster that enforces policy the server could therefore reach +neither CopilotKit Intelligence, nor an identity provider, nor a Bot: nobody could sign in and no +conversation ran. Two of the five shipped `ci/` targets turn the policy on, and on GKE enforcement is +the default and cannot be switched off. + +Nothing said so. The pod passed every probe and stayed Ready, because `/health` answers from a +literal, so the first evidence was a timeout to a hostname that read as the internet being down. + +The API now reaches HTTP and HTTPS everywhere outside the cluster's private ranges, in every +`computers.mode` rather than only `sandbox`, cut by the same exception list the computers' own policy +uses. It still cannot address another pod, a node, or a cloud metadata endpoint. + +`mode: sandbox` had been working only because a rule meant for the Kubernetes API server carried no +destination and so permitted everything. That rule now covers the API server alone, and +`networkPolicy.kubernetesApiCidr` narrows it to your cluster's service range; left empty it stays as +it was, because a chart cannot know that range. ### A finished turn shows the page it opened, not the one open now diff --git a/charts/openbot/templates/networkpolicy.yaml b/charts/openbot/templates/networkpolicy.yaml index 973a093e..d188a428 100644 --- a/charts/openbot/templates/networkpolicy.yaml +++ b/charts/openbot/templates/networkpolicy.yaml @@ -7,8 +7,16 @@ Off by default, because a NetworkPolicy on a cluster with no CNI that enforces o that silently does nothing, and on a cluster that does enforce one a wrong rule is an outage. A deployment that turns this on is saying it knows which of the two it has. -Egress deliberately allows DNS and the database, and nothing else without being asked: a Bot's -computer reaching the open internet is the computers' own policy, not the API's. +Egress allows DNS, the database, the computers, and HTTP and HTTPS to everywhere that is not the +cluster's own private network. THAT LAST ONE IS NOT A CONCESSION, it is what this pod does: sign-in +goes to an identity provider, every conversation goes to Intelligence, and every run goes to a Bot +at an address somebody registered. All three are hostnames rather than CIDRs, and a NetworkPolicy +cannot match a hostname, so there is no narrower rule to write. Leaving it out did not fence the API +off, it stopped the product working, and only in `computers.mode: sandbox` did a rule meant for the +Kubernetes API server quietly cover for it. + +The private ranges stay cut out by exception, the way the computers' policy does it, so this is +still a pod that cannot address another pod, a node, or a cloud metadata endpoint. */}} apiVersion: networking.k8s.io/v1 kind: NetworkPolicy @@ -62,9 +70,43 @@ spec: - port: 4100 protocol: TCP {{- end }} + {{- /* + Intelligence, the identity provider, and every Bot: all of them, and all outside the cluster. + + Written as an exception list rather than as a destination list because the destinations are + hostnames the deployment configures and a NetworkPolicy matches addresses. The same shape the + computers' policy below already uses, for the same reason. + */}} + - to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + # The cluster and everything else on the private network, including the database. + - 10.0.0.0/8 + - 172.16.0.0/12 + - 192.168.0.0/16 + # Link-local, which is where every cloud keeps the endpoint that hands out credentials. + - 169.254.0.0/16 + ports: + - port: 80 + protocol: TCP + - port: 443 + protocol: TCP {{- if eq .Values.computers.mode "sandbox" }} - {{- /* The API server, which is where a per-Bot computer is asked for. */}} - - ports: + {{- /* + The Kubernetes API server, which is where a per-Bot computer is asked for. + + Its own rule because it sits on the private network the rule above cuts out, so nothing else + here reaches it. Unscoped unless a deployment says otherwise: the API server answers on a + ClusterIP from the service range, and a chart cannot know that range at template time. Name it + in `networkPolicy.kubernetesApiCidr` and this narrows to it. + */}} + - {{- with .Values.networkPolicy.kubernetesApiCidr }} + to: + - ipBlock: + cidr: {{ . }} + {{- end }} + ports: - port: 443 protocol: TCP - port: 6443 diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index da8e7596..dbb42f81 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -365,8 +365,18 @@ httpRoute: networkPolicy: enabled: false # Where the API may reach out to. A deployment with a managed database adds its CIDR here. + # + # It already reaches HTTP and HTTPS everywhere outside the cluster's private ranges, because that + # is where Intelligence, sign-in and the Bots are. This is for anything on the private side. extraEgress: [] extraIngress: [] + # `computers.mode: sandbox` only. The service range the Kubernetes API server answers on, so the + # rule that lets the API ask for a Bot's computer can name it instead of being left open. + # + # Empty means unscoped, which is the only thing a chart can do by default: the range is the + # cluster's, not the release's. `kubectl get svc kubernetes -o jsonpath='{.spec.clusterIP}'` shows + # which one yours is on; on EKS it is usually 172.20.0.0/16, on GKE and kubeadm 10.96.0.0/12. + kubernetesApiCidr: "" # Where a Bot's computer may reach beyond the public internet. A deployment whose Bots must reach # an internal site adds it here, one address at a time, rather than reopening the private ranges. computerExtraEgress: [] diff --git a/scripts/check-rendered-chart.ts b/scripts/check-rendered-chart.ts index 12e68145..dc5de112 100644 --- a/scripts/check-rendered-chart.ts +++ b/scripts/check-rendered-chart.ts @@ -127,6 +127,33 @@ for (const [name, keys] of written) { } } +/** + * A policy that fences the API off from the services it cannot work without. + * + * The same question as the Secret one above, asked of the other thing a render can be internally + * wrong about: this chart requires CopilotKit Intelligence and an identity provider, reaches both + * over HTTPS at hostnames, and also writes the rule that says where the API may go. Those two had + * never been compared. The server's egress named DNS, the database and the computers, so on any + * cluster that enforces policy nobody could sign in and no conversation ran — and the pod stayed + * Ready throughout, because `/health` answers from a literal. + * + * Asked of the rendered object rather than the template, because the rule that covered for this was + * conditional on `computers.mode` and only one mode ever had it. + */ +const serverPolicy = documents.find( + (document) => + /^kind:\s*NetworkPolicy\s*$/m.test(document) && + /app\.kubernetes\.io\/component:\s*server/.test(document), +); +if (serverPolicy) { + const egress = serverPolicy.split(/^\s{2}egress:\s*$/m)[1] ?? ""; + if (!/port:\s*443\b/.test(egress)) { + problems.push( + "The server's NetworkPolicy has no egress on 443, so the API cannot reach Intelligence or an identity provider. Nothing would report it: /health answers from a literal and every probe reads it.", + ); + } +} + if (problems.length > 0) { for (const problem of problems) console.error(`::error::${problem}`); process.exit(1); @@ -134,6 +161,7 @@ if (problems.length > 0) { console.log( `${documents.length} objects, ${demands.length} secret keys demanded, and every required one is written.` + + (serverPolicy ? " The server's egress reaches 443." : "") + (skippedOptional > 0 ? ` ${skippedOptional} optional key${skippedOptional === 1 ? " was" : "s were"} not checked.` : ""), From cbab27edce060ddf2b9462c47922f31f7b85bf0c Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:16:25 -0500 Subject: [PATCH 08/10] Refuse a credential written into the rest of the address (#230) Refusing a credential in the userinfo closed one spelling and left the two beside it open. A token in the query string or the fragment was accepted, and addCustomServer writes the address it was given into mcp_servers.url and into the configuration.changed audit payload verbatim. Redaction keys on the field name and url is not a sensitive one, so the secret landed in an append-only row in clear text, which is the disclosure the userinfo rule exists to prevent. The name is read rather than matched against a list. An exact-name version of this rule refused token and accepted auth_token, api_token, x-api-key and X-Amz-Signature, and an operator has no way to know which spellings the check happens to hold. Reading the name over-refuses in one direction on purpose: a misread parameter costs a rename, a missed one cannot be deleted afterwards. The fragment is split at the first question mark first, because a hash route or an OAuth-style callback puts a path in front of the parameters and reading the whole fragment as one query string turns all of it into a single name that matches nothing. metadata.goog is refused too, by asking the list browsing already uses rather than keeping a second copy here. It is Google's own short alias for the metadata server and it carries a dot and none of the suffixes this check lists, so it read as an ordinary vendor name, while the long spelling was refused only incidentally by the .internal test. --- CHANGELOG.md | 26 +++++++ server/src/computer/target.ts | 20 ++++- server/src/plugins/catalogue.ts | 98 +++++++++++++++++++++++ server/tests/plugin-catalogue.test.ts | 108 ++++++++++++++++++++++++++ 4 files changed, 248 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98fc7192..c16b435c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -243,6 +243,32 @@ this port has to reach it another way**, which is what publishing it on every in This does not reach back in time. A deployment that has been running with the two on one network should assume a Bot could have read or written the database, and look at the trail with that in mind. +### A credential in an MCP server address is refused in the query and the fragment too + +Refusing `https://user:token@vendor.example/mcp` closed the userinfo spelling of a credential in the +address and left the two obvious ones open. `?token=`, `?api_key=` and their neighbours were still +accepted, and the address is stored and named in the trail exactly as given: audit redaction keys on +the field name, `url` is not a sensitive one, so the secret was written to `mcp_servers` and to an +append-only audit row in clear text. That is the same disclosure the userinfo rule exists to prevent, +one character away. + +A parameter whose name reads as a credential is now refused, in the query string and in the fragment, +and the refusal points at the token field without repeating what was typed. The name is read rather +than matched against a list, so `?auth_token=`, `?x-api-key=` and `?X-Amz-Signature=` are refused +alongside `?token=`: a rule that only catches the spellings somebody thought of reads as a guard +while behaving like a gap. The test is on the parameter name rather than on the presence of a query, +because vendors route and version with parameters and a floor that refused every one of them would +be one an operator works around instead of with. `https://mcp.example.com/mcp?workspace=acme&version=2` +is unaffected, and so is an ordinary fragment. A credential written into the *path* is still +accepted: it is indistinguishable from a route, and at least one hosted provider addresses servers +that way. **A deployment where somebody has put a credential in an address should treat it as +disclosed and rotate it**, for the same reason as before: the audit row cannot be deleted. + +`metadata.goog` is refused too. It is Google's own short name for the metadata server, published +beside `metadata.google.internal`, and it carries a dot and none of the suffixes this check lists, so +it read as an ordinary vendor name. The long spelling was only ever refused incidentally, by the +`.internal` rule. Both are now named, so the address this check was written for is refused on purpose +rather than by luck. ### Name the private addresses an agent may live at diff --git a/server/src/computer/target.ts b/server/src/computer/target.ts index a0874141..0304b9a9 100644 --- a/server/src/computer/target.ts +++ b/server/src/computer/target.ts @@ -39,6 +39,20 @@ const NEVER_ALLOWED_HOSTNAMES = new Set([ "100.100.100.200", ]); +/** + * Is this the address of a cloud metadata service? + * + * Exported because the same question is asked outside browsing: an MCP server address an + * administrator types is refused on the same grounds, and the answer has to come from one list. + * Two copies drift, and the copy that misses an alias is the one that lets a credential endpoint + * through. + * + * Canonicalises first, so the trailing-dot and IPv6 spellings are seen through here as well. + */ +export function isNeverAllowedHostname(hostname: string): boolean { + return NEVER_ALLOWED_HOSTNAMES.has(canonicalHostname(hostname.toLowerCase())); +} + /** Hostnames inside the deployment. Reachable only when a deployment opts in. */ const INTERNAL_HOSTNAMES = new Set([ "localhost", @@ -204,9 +218,7 @@ export function checkComputerAddress(raw: string): TargetVerdict { // Canonicalised for the same reason navigation is: the address reaches a fetch either way, so the // spellings that gate has to see through are the spellings this one has to see through. - if ( - NEVER_ALLOWED_HOSTNAMES.has(canonicalHostname(url.hostname.toLowerCase())) - ) { + if (isNeverAllowedHostname(url.hostname)) { return { allowed: false, reason: @@ -244,7 +256,7 @@ export function checkNavigationTarget( const hostname = canonicalHostname(url.hostname.toLowerCase()); // Checked before the opt-in, so no configuration can reach it. - if (NEVER_ALLOWED_HOSTNAMES.has(hostname)) { + if (isNeverAllowedHostname(hostname)) { return { allowed: false, reason: diff --git a/server/src/plugins/catalogue.ts b/server/src/plugins/catalogue.ts index 420148a5..e6ca47b0 100644 --- a/server/src/plugins/catalogue.ts +++ b/server/src/plugins/catalogue.ts @@ -26,6 +26,9 @@ * These are where this deployment sends a person's authorization code and receives the refresh * token that stands in for their access, so they are a reviewed source contract too. */ +// The one place browsing and this check agree on: the addresses that hold the deployment's own +// cloud credentials. `target.ts` imports nothing itself, so asking it here adds no dependency. +import { isNeverAllowedHostname } from "../computer/target"; // Type-only, so naming the transport here creates no import cycle with the registry that resolves it. import type { TransportKind } from "./transport"; @@ -330,6 +333,61 @@ export function classifyTool( return entry.writeTools.includes(toolName) ? "write" : "read"; } +/** + * Words that make a parameter name a credential, wherever they appear in it. + * + * A containment test rather than a list of exact names, because the exact-name version of this rule + * refused `?token=` and accepted `?auth_token=`, `?api_token=`, `?session_token=` and every other + * spelling one word away. An operator has no way to know which of those the check happens to hold, + * so a rule that only refuses the names somebody thought of reads as a guard while behaving like a + * gap. + * + * Not shared with `sensitiveKeys` in `audit.ts`: that module reaches the database and this function + * deliberately imports nothing that does. The two also want different contents, since audit redacts + * `content`, `prompt` and `result`, which are payload field names and mean nothing here. + */ +const CREDENTIAL_WORDS = [ + "token", + "secret", + "password", + "passwd", + "credential", + "signature", + "bearer", +]; + +/** + * Names that are a credential on their own but are too short to contain safely. + * + * `sig` is the reason this list is separate from the one above: "design" contains it. These are + * compared whole, so an ordinary word carrying the same three letters is left alone. + */ +const CREDENTIAL_NAMES = new Set([ + "auth", + "authorization", + "pass", + "pwd", + "sig", +]); + +/** + * Does this parameter name say it holds a credential? + * + * Names are compared with their separators dropped, so `api_key`, `apiKey` and `x-api-key` are one + * question rather than three. A name ending in "key" is a credential and a name merely containing it + * is not, which is what keeps `keyword` and `monkey` apart; "author" is likewise not "auth". + * + * It over-refuses in one direction on purpose. A parameter this rule misreads costs an operator a + * rename, and one it misses is written to an append-only audit row that cannot be deleted. + */ +function readsAsCredential(name: string): boolean { + const normalized = name.replaceAll(/[^a-zA-Z0-9]/g, "").toLowerCase(); + if (CREDENTIAL_NAMES.has(normalized) || normalized.endsWith("key")) { + return true; + } + return CREDENTIAL_WORDS.some((word) => normalized.includes(word)); +} + /** * Is this a URL an administrator may point the deployment at? * @@ -366,6 +424,32 @@ export function customUrlRefusal(raw: string): string | null { return "Put the credential in the token field rather than in the address."; } + /* + * The query is the other half of the same hole, and the fragment is the half after that. + * + * No host rule below reads either one, and both are stored and audited with the rest of the + * string, so a token written here is as durable and as readable as one written into the userinfo. + * The fragment never reaches the server at all, which is why it is not a request-forgery concern + * and is still a disclosure one: what this rule is about is where the string ends up, not where + * the request goes. + * + * The test is on the parameter name rather than on the presence of a query, because vendors + * legitimately route and version with parameters. A floor that refused every one of them would be + * one an operator works around rather than with, and an ordinary `#section` is left alone for the + * same reason. + */ + const hash = url.hash.replace(/^#/, ""); + const marker = hash.indexOf("?"); + const fragment = + marker === -1 ? [hash] : [hash.slice(0, marker), hash.slice(marker + 1)]; + const named = [ + ...url.searchParams.keys(), + ...fragment.flatMap((part) => [...new URLSearchParams(part).keys()]), + ]; + if (named.some(readsAsCredential)) { + return "Put the credential in the token field rather than in the address."; + } + // A trailing dot is the root-anchored spelling of the same name and resolves to the same place, so // they are stripped here rather than added to each comparison below. Without it "localhost." // misses the equality test, "vault.internal." misses the suffix tests, and "database." picks up @@ -377,6 +461,20 @@ export function customUrlRefusal(raw: string): string | null { if (host.includes(":") || /^[0-9.]+$/.test(host)) { return "Give a hostname rather than an IP address."; } + /* + * The cloud metadata endpoint, by name rather than by luck. + * + * `metadata.goog` is Google's own short alias for it, published beside `metadata.google.internal`, + * and it carries a dot and none of the suffixes below, so it read as an ordinary vendor name. The + * long spelling was refused only incidentally, by the `.internal` test. + * + * Asked of the list browsing already uses rather than a second copy here. That list holds the + * aliases somebody has already had to think about, including the ones Alibaba and ECS answer on, + * and a new alias added there should not have to be remembered here as well. + */ + if (isNeverAllowedHostname(host)) { + return "That address holds this deployment's own cloud credentials."; + } if (host === "localhost" || host.endsWith(".localhost")) { return "That address is local to the deployment."; } diff --git a/server/tests/plugin-catalogue.test.ts b/server/tests/plugin-catalogue.test.ts index e98a6213..b0acfe29 100644 --- a/server/tests/plugin-catalogue.test.ts +++ b/server/tests/plugin-catalogue.test.ts @@ -342,6 +342,114 @@ describe("a URL an administrator typed", () => { expect(refusal).not.toContain("oauth"); }); + test("a credential in the query string is refused", () => { + // The same harm as the userinfo case above, reached through the other part of the URL no host + // rule looks at. addCustomServer writes the string it was given into mcp_servers.url and into + // the configuration.changed audit payload, audit redaction keys on the field name, and "url" is + // not a sensitive name, so a token here sits in an append-only trail in clear text. + expect( + customUrlRefusal("https://mcp.example.com/mcp?token=sk-live-abcdef"), + ).not.toBeNull(); + expect( + customUrlRefusal("https://mcp.example.com/mcp?api_key=SECRET"), + ).not.toBeNull(); + expect( + customUrlRefusal("https://mcp.example.com/mcp?access_token=SECRET"), + ).not.toBeNull(); + expect( + customUrlRefusal("https://mcp.example.com/mcp?client_secret=SECRET"), + ).not.toBeNull(); + }); + + test("the names a credential is actually given are refused too", () => { + // The first version of this rule listed exact names, which is a corner of the class rather than + // the class: every one of these was accepted while `?token=` was refused, and an operator does + // not know which spelling the check happens to hold. The match reads the name for what it says. + for (const name of [ + "auth_token", + "api_token", + "apiToken", + "access_key", + "secret_key", + "private_key", + "session_token", + "x-api-key", + "subscription-key", + "X-Amz-Signature", + "bearer", + "pwd", + ]) { + expect( + customUrlRefusal(`https://mcp.example.com/mcp?${name}=s3cret`), + ).not.toBeNull(); + } + }); + + test("an ordinary query parameter is still accepted", () => { + // The rule reads the parameter name, not the presence of a query, because vendors route and + // version with parameters. Refusing every query string would make this floor an outage rather + // than a guard, and an operator who cannot add a working server will find a way around it. + expect( + customUrlRefusal("https://mcp.example.com/mcp?workspace=acme&version=2"), + ).toBeNull(); + // The near misses, which are what a rule that reads names rather than matching them exactly has + // to get right: "keyword" is not a key and "author" is not auth. + expect( + customUrlRefusal("https://mcp.example.com/mcp?keyword=x&author=jane"), + ).toBeNull(); + }); + + test("refusing a credential in the query does not repeat it", () => { + // Same property as the userinfo refusal: this string is rendered to an administrator and can + // reach a log, so it must not carry the secret it exists to reject. + const refusal = customUrlRefusal( + "https://mcp.example.com/mcp?token=s3cret", + ); + expect(refusal).not.toBeNull(); + expect(refusal).not.toContain("s3cret"); + expect(refusal).not.toContain("mcp.example.com"); + }); + + test("a credential in the fragment is refused too", () => { + // The fragment never leaves the browser, but that is not the harm here. addCustomServer stores + // and audits the whole string, so a secret written after the hash is as durable and as readable + // as one in the query. Refusing one and not the other would leave the same bypass a character + // away. + expect( + customUrlRefusal("https://mcp.example.com/mcp#token=s3cret"), + ).not.toBeNull(); + // The shapes a fragment is actually written in. A hash route or an OAuth-style callback puts a + // path before the question mark, and reading the whole fragment as one query string turns all + // of it into a single name that matches nothing. + expect( + customUrlRefusal("https://mcp.example.com/mcp#/callback?token=s3cret"), + ).not.toBeNull(); + expect( + customUrlRefusal("https://mcp.example.com/mcp#!/x?token=s3cret"), + ).not.toBeNull(); + expect( + customUrlRefusal("https://mcp.example.com/mcp#token%3Ds3cret"), + ).not.toBeNull(); + // An ordinary fragment is not a credential and is left alone. + expect(customUrlRefusal("https://mcp.example.com/mcp#section")).toBeNull(); + }); + + test("the short name for the cloud metadata endpoint is refused", () => { + // metadata.goog is Google's own alias for the metadata server, published beside + // metadata.google.internal and 169.254.169.254. It carries a dot and none of the suffixes + // above, so it read as an ordinary vendor name, while the long spelling was caught only + // incidentally by the .internal test. + expect( + customUrlRefusal("https://metadata.goog/computeMetadata/v1/"), + ).not.toBeNull(); + expect( + customUrlRefusal("https://metadata.goog./computeMetadata/v1/"), + ).not.toBeNull(); + expect( + customUrlRefusal("https://METADATA.GOOG/computeMetadata/v1/"), + ).not.toBeNull(); + }); + test("nonsense is refused rather than thrown", () => { expect(customUrlRefusal("not a url")).toBe("That is not a URL."); }); From 8f68eaa42bfb51f6a22247070bb997d25d988c1e Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:21:27 -0500 Subject: [PATCH 09/10] Spend an MCP token only for its own server, and only at its own address (#238) * Point a curated MCP server only at a credential of its own kind Adding a server by URL checks which credential it is being pointed at. Adding one from the catalogue took the same field from the same request and stored it unread, so a credential of any kind could be attached to a curated server and spent by the refresh that runs before the add returns. The reach is narrower than the path beside it and worth saying so. The column is a foreign key, so an id naming nothing was already refused by the database, and the one entry in the catalogue is reached with each person's own account, whose OAuth client is registered through its own call and sent to a pinned address. What was reachable is a credential of the wrong kind being accepted and spent on behalf of somebody who never agreed to it, a malformed id arriving as a database error where a refusal belongs, and the whole shape returning with the first deployment-bearer entry a fork re-adds, which the catalogue invites. Which kind an entry takes is decided beside the entry, because it is a property of the vendor's auth rather than of the request. Both add paths then ask one function the same question, so a credential that does not exist and one of the wrong kind are still refused in the same words and the endpoint cannot be asked which ids are real. The curated route maps that refusal to a 400 rather than letting it surface as a 500. Re-adding a curated server no longer clears the credential it points at. That column holds the OAuth client registering one put there, and a re-add to change an instance host said nothing about it while clearing it anyway, leaving the row orphaned and everybody who had connected told there is no client registered. * Spend an MCP token only for its own server, and only at its own address Attaching a credential to a server is the one place this deployment accepts a reference to a stored secret rather than the secret itself. Everywhere else the value arrives in the request that stores it, and the id it gets is nobody's to choose: storeAgentAuth mints its own row from the key an administrator typed. So this is the field where which secret and which address can be made to disagree, and the add is what settles it, because refreshTools runs before the call returns and sends what it decrypts to the URL from the same request. Both ways they could disagree are now refused. A credential has to belong to the server it is attached to, which the vault already records: storeMcpToken sets the provider to the server it mints for and is the only way the plugins screen makes one, so nothing a deployment can reach through the UI is refused by this. And a server that already holds a credential cannot be re-added at a different address, which is the case a check on ownership cannot see: the token does belong to that server, and only the address moved. The second is why the first is not enough alone. Both delivered a stored token to a host the caller named, before any Bot, grant or policy check existed, and a stored credential is otherwise unreadable by design. Refused rather than repaired, because both harmless readings are served by something else. Correcting a title or retrying an interrupted add sends the same URL and is untouched, a server holding no credential can still be re-addressed, and moving one that does means removing it and adding it again with the token the new address is meant to have. Curated servers are unaffected: their URL comes from the catalogue rather than the request, and an instance hostname is matched against the vendor's anchored pattern before anything is stored. The upsert test from #214 now mints its own token. It had reused one credential across two server ids, which is a shape storeMcpToken cannot produce. * Show a dot on a channel a Bot has spoken in unseen (#259) * Give a membership a memory of when its channel was last read * Stamp the caller's membership read, and say so in the roster * Let a member say they have read a channel * Carry the read marker to the app and let it be stamped * Draw a dot on a channel a Bot has spoken in unseen * Mark a channel read the moment it is the one on screen * Narrow the admin and settings rails to the width their labels earn * Keep a fast clock elsewhere from turning mark-read into a storm * Say in the changelog what the unread dot is and is not * Stamp a read against the message clock, not just this one * Assert the clamped stamp without reaching through an optional chain * Regenerate the read-marker migration behind the queue and the frames * Refuse to mark a deleted channel read, matching the pin * Update azure/setup-helm action to v5 (#260) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Say what a strict content-security-policy has to allow (#225) * Point the test at the database the project actually has (#234) * Let the API reach Intelligence and sign-in when a NetworkPolicy is on (#257) * Refuse a credential written into the rest of the address (#230) Refusing a credential in the userinfo closed one spelling and left the two beside it open. A token in the query string or the fragment was accepted, and addCustomServer writes the address it was given into mcp_servers.url and into the configuration.changed audit payload verbatim. Redaction keys on the field name and url is not a sensitive one, so the secret landed in an append-only row in clear text, which is the disclosure the userinfo rule exists to prevent. The name is read rather than matched against a list. An exact-name version of this rule refused token and accepted auth_token, api_token, x-api-key and X-Amz-Signature, and an operator has no way to know which spellings the check happens to hold. Reading the name over-refuses in one direction on purpose: a misread parameter costs a rename, a missed one cannot be deleted afterwards. The fragment is split at the first question mark first, because a hash route or an OAuth-style callback puts a path in front of the parameters and reading the whole fragment as one query string turns all of it into a single name that matches nothing. metadata.goog is refused too, by asking the list browsing already uses rather than keeping a second copy here. It is Google's own short alias for the metadata server and it carries a dot and none of the suffixes this check lists, so it read as an ordinary vendor name, while the long spelling was refused only incidentally by the .internal test. --------- Co-authored-by: Guido Vizoso Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Vaibhav Zope <121456155+zopeVaibhav@users.noreply.github.com> Co-authored-by: David McKay --- CHANGELOG.md | 68 ++++ server/src/plugins/catalogue.ts | 15 + server/src/plugins/routes.ts | 7 +- server/src/plugins/store.ts | 195 +++++++++-- server/tests/plugin-catalogue.test.ts | 42 +++ ...gin-credential-binding.integration.test.ts | 328 ++++++++++++++++++ ...gin-curated-credential.integration.test.ts | 261 ++++++++++++++ .../tests/plugin-routes.integration.test.ts | 268 ++++++++++++++ server/tests/plugin-routes.test.ts | 105 ++++++ server/tests/plugin-store.integration.test.ts | 18 +- 10 files changed, 1279 insertions(+), 28 deletions(-) create mode 100644 server/tests/plugin-credential-binding.integration.test.ts create mode 100644 server/tests/plugin-curated-credential.integration.test.ts create mode 100644 server/tests/plugin-routes.integration.test.ts create mode 100644 server/tests/plugin-routes.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c16b435c..805817b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -192,6 +192,46 @@ read that as a stolen token and revoke the whole connection. Every plugin call t token now locks the credential's vault row for the length of the exchange, so a second replica waits rather than races, and the rotated token is written back in the same transaction that held the lock. Nothing to configure; a connection just stops going stale under concurrent traffic. + +### An MCP token is spent only by its own server, and only at the address it was given + +Pointing a server at a credential is the one place this deployment takes a reference to a stored +secret rather than the secret itself. Everywhere else, the value was typed into the same request that +stores it: a Bot's key is minted from what an administrator pasted and the id it gets is nobody's to +choose. So this is the one field where which secret and which address could be made to disagree, and +the add settles the disagreement by spending the credential: the tool refresh runs before the call +returns and sends what it decrypts to the URL from that same request. + +Two ways they could disagree, and both are now refused. A server could be pointed at any `mcp` +credential in the vault, including one minted for a different vendor, so a token given to one server +was deliverable to another. And re-adding a server with a different URL rewrote the address while +keeping the credential, so the same token could be sent somewhere else entirely with no +cross-server trick at all: the token really did belong to that server, and only the address moved. + +The second is why the first was not enough on its own. A credential now has to belong to the server +it is attached to, and a server that already holds one cannot be re-added at a different address. +Correcting a title or retrying an interrupted add sends the same URL and is unaffected. A server +holding no credential can still be re-addressed, because there is nothing to misdirect. Moving a +server that does hold one means removing it and adding it again with the token the new address is +meant to have, which is the honest description of what has happened anyway. + +This matters more than "an administrator could misconfigure something". A stored credential cannot +be read back by anybody, by design: the credentials screen answers that a credential exists and +never what it is. These two shapes were the way around that, so a deployment where somebody has +used them should treat the credentials involved as disclosed and rotate them. + +A token also stops outliving the server it was minted for. Re-adding a server without naming a +credential used to clear the pointer while leaving the credential live, and removing a server retires +its token by reading it off that pointer, so a cleared one meant the token survived its server and +could be attached to a freshly created one at any address, where there was no longer a stored address +to compare against. Three ordinary acts in a row and the binding above stopped meaning anything. The +pointer now survives a re-add that names none, removal therefore finds and retires it, and a retired +credential is refused rather than quietly attached to fail on its next call. + +Curated servers keep working as they did. Their URL comes from the catalogue rather than the +request, and a per-instance hostname is matched against the vendor's own anchored pattern before +anything is stored, so re-adding one cannot point it at an address of the caller's choosing. + ### Knowledge searches instead of guessing A package can say which of its skills each coworker gets, and the fintech example gives Knowledge the @@ -269,6 +309,34 @@ beside `metadata.google.internal`, and it carries a dot and none of the suffixes it read as an ordinary vendor name. The long spelling was only ever refused incidentally, by the `.internal` rule. Both are now named, so the address this check was written for is refused on purpose rather than by luck. +### A curated MCP server is pointed at its own kind of credential too + +Adding a server by URL was made to check which credential it is being pointed at. Adding one from the +catalogue, the other half of the same screen, took the same field from the same request and stored it +unread, so a credential of any kind could be attached to a curated server and spent by the refresh +that runs before the add returns. + +Worth being plain about the reach, because it is narrower than the path beside it. The column is a +foreign key, so an id naming nothing was already refused by the database, and the one entry in the +catalogue is reached with each person's own Google account, whose OAuth client is registered through +its own call and sent to an address pinned in code. Nothing could be delivered to an address a caller +chose. What was reachable was a credential of the wrong kind being accepted and spent on behalf of +somebody who never agreed to it, and a malformed id arriving as a database error rather than as a +refusal. + +The rule now comes from the entry: a server the deployment holds one token for takes that token, and +a server answered as the person asking takes no credential when it is added, because its client +arrives through the call that mints it. Both add paths ask the same question in the same words, so a +credential that does not exist and one of the wrong kind are still refused identically and the +endpoint cannot be used to ask which ids are real. Adding a curated server the way the admin screen +does is unchanged. + +Adding a curated server that is already there no longer clears the credential it points at. The +column holds the OAuth client that registering one put there, and re-adding the server to change an +instance host said nothing about that client, but cleared it anyway: the credential row was left +behind with nothing pointing at it and nothing to revoke it, and everybody who had connected their +account was told the deployment has no client registered. A re-add that names no credential now +leaves the one that is there alone. ### Name the private addresses an agent may live at diff --git a/server/src/plugins/catalogue.ts b/server/src/plugins/catalogue.ts index e6ca47b0..f4fd2d3e 100644 --- a/server/src/plugins/catalogue.ts +++ b/server/src/plugins/catalogue.ts @@ -266,6 +266,21 @@ const PATTERNS = new Map( ]), ); +/** + * Which kind of credential this entry's server record may be pointed at, or null when it takes none + * from the caller. + * + * Beside the entry rather than at the call site, because it is a property of the vendor's auth and + * not of the request. `deployment-bearer` is the only kind that means "one token this deployment + * holds for this server", which is what `mcp` names in the vault. A `user-oauth` server is answered + * with the asker's own grant and its OAuth client is registered through its own call, which mints + * the credential itself, so an id offered when the server is added is never the right one whatever + * kind it names. A server needing no credential takes none. + */ +export function serverCredentialKind(entry: CatalogueEntry): "mcp" | null { + return entry.auth.kind === "deployment-bearer" ? "mcp" : null; +} + export function catalogueEntry(key: string): CatalogueEntry | null { return BY_KEY.get(key) ?? null; } diff --git a/server/src/plugins/routes.ts b/server/src/plugins/routes.ts index 0c62c9ee..30d73294 100644 --- a/server/src/plugins/routes.ts +++ b/server/src/plugins/routes.ts @@ -202,7 +202,12 @@ export function createPluginRoutes( }); return context.json({ server }); } catch (error) { - if (error instanceof CatalogueEntryUnknownError) { + // A refused credential is the administrator's mistake to correct, so it comes back as a + // refusal with its reason rather than as a 500 the way an unmapped throw would. + if ( + error instanceof CatalogueEntryUnknownError || + error instanceof CustomServerRefusedError + ) { return context.json({ error: error.message }, 400); } throw error; diff --git a/server/src/plugins/store.ts b/server/src/plugins/store.ts index 61a27974..958c29f9 100644 --- a/server/src/plugins/store.ts +++ b/server/src/plugins/store.ts @@ -32,6 +32,7 @@ import { classifyTool, customUrlRefusal, resolveServerUrl, + serverCredentialKind, } from "./catalogue"; import { McpServerError } from "./mcp"; import { registerDynamicClient } from "./oauth"; @@ -1373,6 +1374,84 @@ export function createPluginStore(options: PluginStoreOptions) { } } + /** + * The credential a server is being pointed at is of the kind that server can spend. + * + * Both add paths dereference the pointer before they return, so this is checked where the pointer + * is accepted rather than where it is used. `mcp` is the only kind that answers "this server's own + * token". A `mcp_user_token` is one person's grant and a `mcp_oauth_client` identifies the + * deployment to a vendor; spending either here uses a credential on behalf of somebody who never + * agreed to it, which is the same objection `POST /api/admin/credentials` already makes when it + * refuses to mint those two by hand. + * + * The shape is checked before the lookup because `credentials.id` is a `uuid` column, so a value + * that is not one makes the query itself fail rather than return no rows, and the caller gets a + * database error where a refusal belongs. + * + * One message for both "wrong kind" and "no such credential", deliberately. A caller who can tell + * those apart can ask this endpoint which credential ids are real. + */ + async function requireCredentialOfKind( + serverTitle: string, + serverId: string, + credentialId: string, + kind: "mcp" | null, + ): Promise { + /* + * A server that takes no credential when it is added is refused here rather than at the caller, + * so that offering an id is one question with one answer wherever it is asked. The wording says + * what is true of both kinds that reach it: a `user-oauth` server's client arrives through the + * call that mints it, and a server needing no credential has nothing to be given. + */ + if (!kind) { + throw new CustomServerRefusedError( + `${serverTitle} takes no credential when it is added.`, + ); + } + + const looksLikeId = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test( + credentialId, + ); + /* + * Live, as well as the right kind and the right owner. + * + * A revoked credential cannot be decrypted, so attaching one only ever produced a server that + * fails on its next call. Refusing it here says so at the moment somebody can still act on it, + * and it closes the case where a token was retired precisely because it should stop being used. + */ + const [named] = looksLikeId + ? await database + .select({ + kind: credentialRows.kind, + provider: credentialRows.provider, + }) + .from(credentialRows) + .where( + and( + eq(credentialRows.id, credentialId), + isNull(credentialRows.revokedAt), + ), + ) + : []; + + /* + * Whose it is, as well as what it is. + * + * `provider` is the server a token was minted for: `storeMcpToken` sets it to the server id and + * is the only way the plugins screen makes one. Without this, any `mcp` row in the vault could + * be attached to any server, and since the refresh spends it against that server's address, a + * token given to one vendor was deliverable to another. Reading a credential back is otherwise + * impossible by design, so this closes the one field that accepts a reference to a secret rather + * than the secret itself. + */ + if (named?.kind !== kind || named.provider !== serverId) { + throw new CustomServerRefusedError( + "That is not a credential this server can use. Add the server's own token instead.", + ); + } + } + async function requireServer(serverId: string) { const [row] = await database .select() @@ -1411,6 +1490,27 @@ export function createPluginStore(options: PluginStoreOptions) { const resolved = resolveServerUrl(input.key, input.instanceHost); if (!resolved) throw new CatalogueEntryUnknownError(input.key); + /* + * The pointer is checked here for the same reason it is on the path below: the refresh that + * runs before this returns dereferences whatever it names. + * + * What that reaches is narrower on this path, because the URL is the catalogue's rather than + * the caller's, so a credential cannot be delivered to an address somebody chose. That is a + * property of today's catalogue rather than of this function: the one entry it holds is + * `user-oauth`, and the catalogue's own comment invites a fork to re-add the vendors that were + * taken out. The first `deployment-bearer` entry restores the full shape, so the check belongs + * here now rather than in the review that re-adds one. + */ + const credentialId = input.credentialId?.trim() || undefined; + if (credentialId) { + await requireCredentialOfKind( + resolved.entry.title, + resolved.entry.key, + credentialId, + serverCredentialKind(resolved.entry), + ); + } + await database .insert(mcpServers) .values({ @@ -1418,14 +1518,24 @@ export function createPluginStore(options: PluginStoreOptions) { title: resolved.entry.title, vendor: resolved.entry.vendor, url: resolved.url, - credentialId: input.credentialId ?? null, + credentialId: credentialId ?? null, addedBy: input.by, }) .onConflictDoUpdate({ target: mcpServers.id, set: { url: resolved.url, - credentialId: input.credentialId ?? null, + /* + * Left alone when the caller sends none, rather than cleared. + * + * `registerOAuthClient` keeps the client it minted in this column, and adding the server + * again to change an instance host is not a statement about that client. Clearing it + * orphaned the credential row, which nothing then revokes, and told everybody who had + * connected that the deployment has no OAuth client registered. There is no longer a way + * to hand it back through this call either, since a `user-oauth` entry now refuses a + * credential id, so the pointer has to survive here. + */ + ...(credentialId ? { credentialId } : {}), addedBy: input.by, updatedAt: new Date(), }, @@ -1504,30 +1614,51 @@ export function createPluginStore(options: PluginStoreOptions) { * One message for both "wrong kind" and "no such credential", deliberately. A caller who can * tell those apart can ask this endpoint which credential ids are real. */ + /* + * A credential is spent at the address it was given to, or not spent. + * + * Adding a server that is already here rewrites its URL, and the refresh that follows sends + * whatever credential it holds to the new one, in the same call. That is the same disclosure + * as naming another server's token and it needs no trick at all: the token really does belong + * to this server, and only the address moved. A check on whose credential it is cannot see it, + * which is why this rule is here and not folded into that one. + * + * Refused rather than repaired, because the two harmless readings of the request are both + * served by something else. Correcting a title or retrying an interrupted add sends the same + * URL and is unaffected, and genuinely moving a server means the vendor is at a new address, + * where the honest act is to remove it and add it again with the token that address is + * supposed to hold. + * + * Only this path. A curated server's URL comes from the catalogue rather than the request, so + * the most a caller can influence is an instance hostname, and that is matched against the + * vendor's own anchored pattern before anything is stored. Re-adding one cannot point it at an + * address of the caller's choosing, which is the whole of what this refuses. + */ const credentialId = input.credentialId?.trim() || undefined; + const [existing] = await database + .select({ url: mcpServers.url, credentialId: mcpServers.credentialId }) + .from(mcpServers) + .where(eq(mcpServers.id, input.id)); + + if ( + existing && + existing.url !== input.url && + (existing.credentialId || credentialId) + ) { + throw new CustomServerRefusedError( + `${input.id} is already here at a different address and holds a credential. Remove it and add it again, with the token the new address is meant to have.`, + ); + } + if (credentialId) { - /* - * The shape is checked before the lookup because `credentials.id` is a `uuid` column, so a - * value that is not one makes the query itself fail rather than return no rows, and the - * caller gets a database error where a refusal belongs. The same was true of the foreign key - * before this guard existed. - */ - const looksLikeId = - /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test( - credentialId, - ); - const [named] = looksLikeId - ? await database - .select({ kind: credentialRows.kind }) - .from(credentialRows) - .where(eq(credentialRows.id, credentialId)) - : []; - - if (named?.kind !== "mcp") { - throw new CustomServerRefusedError( - "That is not a credential this server can use. Add the server's own token instead.", - ); - } + // Always `mcp`: a server added by URL is reached with the one token the deployment holds for + // it, whatever the vendor is, because nothing here knows the vendor. + await requireCredentialOfKind( + input.title, + input.id, + credentialId, + "mcp", + ); } await database @@ -1546,7 +1677,21 @@ export function createPluginStore(options: PluginStoreOptions) { set: { title: input.title, url: input.url, - credentialId: credentialId ?? null, + /* + * Kept when the caller names none, rather than cleared, for a reason beyond tidiness. + * + * Clearing it left the credential live with nothing pointing at it, and `removeServer` + * retires a token by reading it off the row: with the pointer gone it revoked nothing + * and deleted the server, so the token outlived the server it was minted for. It could + * then be attached to a freshly created server at any address, because the rule above + * compares against a row that no longer existed. Three ordinary acts, and the address + * this server was entrusted to stopped meaning anything. + * + * So the pointer survives, `removeServer` finds it, and a removed server's token is + * dead rather than loose. Detaching a token without removing the server is not a thing + * this endpoint does, and nothing asks it to. + */ + ...(credentialId ? { credentialId } : {}), addedBy: input.by, updatedAt: new Date(), }, diff --git a/server/tests/plugin-catalogue.test.ts b/server/tests/plugin-catalogue.test.ts index b0acfe29..9ba4dfd6 100644 --- a/server/tests/plugin-catalogue.test.ts +++ b/server/tests/plugin-catalogue.test.ts @@ -1,11 +1,13 @@ import { describe, expect, test } from "bun:test"; import { CATALOGUE, + type CatalogueEntry, catalogueEntry, classifyTool, customUrlRefusal, hostAdmissible, resolveServerUrl, + serverCredentialKind, } from "../src/plugins/catalogue"; /** @@ -454,3 +456,43 @@ describe("a URL an administrator typed", () => { expect(customUrlRefusal("not a url")).toBe("That is not a URL."); }); }); + +describe("which credential a curated server is given", () => { + /** + * A synthetic entry, because the catalogue holds one vendor today and it is `user-oauth`. + * + * The shared-token branch is the one a fork re-enables when it puts a removed vendor back, which + * is the case this rule exists for, so it is exercised here rather than left to be discovered + * then. The other side of the same argument is why the entry is written out in full rather than + * spread from a real one: what is under test is the auth kind deciding the answer. + */ + const sharedToken: CatalogueEntry = { + key: "shared-token-vendor", + title: "Vendor", + vendor: "Vendor", + summary: "A server the deployment holds one token for.", + host: "https://mcp.vendor.example", + path: "/mcp", + auth: { kind: "deployment-bearer" }, + writeTools: [], + docsUrl: "https://vendor.example/docs", + }; + + test("a shared-token server takes the deployment's own token for it", () => { + expect(serverCredentialKind(sharedToken)).toBe("mcp"); + }); + + test("a server reached as the asker takes no credential from the caller", () => { + // Its OAuth client arrives through registerOAuthClient, which mints the credential itself. An id + // offered here is therefore never the right one, whatever kind it names. + const drive = catalogueEntry("google-drive"); + expect(drive?.auth.kind).toBe("user-oauth"); + expect(serverCredentialKind(drive as CatalogueEntry)).toBeNull(); + }); + + test("a server that needs no credential takes none", () => { + expect( + serverCredentialKind({ ...sharedToken, auth: { kind: "none" } }), + ).toBeNull(); + }); +}); diff --git a/server/tests/plugin-credential-binding.integration.test.ts b/server/tests/plugin-credential-binding.integration.test.ts new file mode 100644 index 00000000..1427b6f8 --- /dev/null +++ b/server/tests/plugin-credential-binding.integration.test.ts @@ -0,0 +1,328 @@ +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + test, +} from "bun:test"; +import { randomUUID } from "node:crypto"; +import { eq, inArray, like } from "drizzle-orm"; +import { createAuditStore } from "../src/audit"; +import { encryptSecret } from "../src/credentials"; +import { createDatabase } from "../src/db/client"; +import { credentials, mcpServers, mcpTools } from "../src/db/schema"; +import { + CustomServerRefusedError, + createPluginStore, +} from "../src/plugins/store"; +import { TEST_POOL } from "./support/database"; + +/** + * Which address a stored credential may be spent against, and whose it has to be. + * + * Pointing a server at a credential is the one place this deployment accepts a *reference* to a + * secret rather than the secret itself. Everywhere else that a stored value is spent, the value was + * typed into the same request that stores it: `storeAgentAuth` mints its own row from the key an + * administrator pasted and hands back an id nobody chose. So this is the field where "which secret" + * and "which address" can be made to disagree, and the add is what settles the disagreement, because + * the refresh runs before it returns and sends what it decrypts to the URL from that same request. + * + * Two rules, and the second is the one that matters. Naming another server's token was accepted, so + * a credential could be spent by a server it was never given to. And re-adding a server with a + * different URL rewrote the address while keeping the credential, so the same token could be sent + * somewhere else entirely without any cross-server trick at all. Closing only the first leaves the + * second, which is why they are one question here rather than two. + */ + +const database = createDatabase( + process.env.DATABASE_URL ?? + "postgres://openbot:openbot@localhost:5432/openbot", + TEST_POOL, +); + +const KEY = `${"x".repeat(43)}=`; +const tag = randomUUID().slice(0, 8); +const serverId = `binding-${tag}`; +const otherServerId = `binding-other-${tag}`; +const ownCredentialId = randomUUID(); +const otherCredentialId = randomUUID(); +const OWN_TOKEN = `sk-own-${tag}`; +const OTHER_TOKEN = `sk-other-${tag}`; +const LEGITIMATE_URL = "https://legit.vendor.example/mcp"; +const CHOSEN_URL = "https://collector.attacker.example/mcp"; + +const store = createPluginStore({ + database, + auditStore: createAuditStore(database), + credentials: { + readSecret: async (id: string) => { + const [row] = await database + .select({ + encryptedValue: credentials.encryptedValue, + revokedAt: credentials.revokedAt, + }) + .from(credentials) + .where(eq(credentials.id, id)); + return row ?? null; + }, + create: async () => { + throw new Error("this suite does not write credentials"); + }, + /** + * A real revoke, unlike the other suites here, because the chain below turns on whether removing + * a server actually retires its token. Stubbing this to throw would make the test prove nothing + * about the case it exists for. + */ + revoke: async (id: string) => { + await database + .update(credentials) + .set({ revokedAt: new Date() }) + .where(eq(credentials.id, id)); + }, + } as never, + encryptionKey: KEY, + policy: () => ({ mode: "enforce", deny: [], allow: ["true"] }), +}); + +/** + * What left the deployment, so a refusal can be shown to have stopped the send rather than reported + * on it afterwards. The vendors here do not exist, so a real request would fail anyway; what this + * captures is whether one was attempted at all, and what it carried. + */ +let sent: { url: string; authorization: string | null }[] = []; +const realFetch = globalThis.fetch; + +beforeAll(async () => { + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const request = + input instanceof Request ? input : new Request(input as string, init); + sent.push({ + url: request.url, + authorization: request.headers.get("authorization"), + }); + return new Response("{}", { status: 500 }); + }) as typeof fetch; + + const encrypted = async (value: string) => encryptSecret(KEY, value); + await database.insert(credentials).values([ + { + id: ownCredentialId, + kind: "mcp", + // How `storeMcpToken` records whose token this is: the server it was minted for. + provider: serverId, + keyId: `mcp-${serverId}`, + encryptedValue: await encrypted(OWN_TOKEN), + metadata: {}, + }, + { + id: otherCredentialId, + kind: "mcp", + provider: otherServerId, + keyId: `mcp-${otherServerId}`, + encryptedValue: await encrypted(OTHER_TOKEN), + metadata: {}, + }, + ]); +}); + +afterEach(() => { + sent = []; +}); + +afterAll(async () => { + globalThis.fetch = realFetch; + await database.delete(mcpTools).where(like(mcpTools.serverId, `binding-%`)); + await database.delete(mcpServers).where(like(mcpServers.id, `binding-%`)); + await database + .delete(credentials) + .where(inArray(credentials.id, [ownCredentialId, otherCredentialId])); +}); + +async function storedUrl(id: string) { + const [row] = await database + .select({ url: mcpServers.url }) + .from(mcpServers) + .where(eq(mcpServers.id, id)); + return row?.url ?? null; +} + +describe("a credential is spent only by the server it belongs to", () => { + test("another server's token is refused", async () => { + await expect( + store.addCustomServer({ + id: serverId, + title: "Collector", + url: CHOSEN_URL, + credentialId: otherCredentialId, + by: "admin@example.com", + }), + ).rejects.toBeInstanceOf(CustomServerRefusedError); + + // The refusal is the whole point only if it happens before the send. + expect(sent).toEqual([]); + expect(await storedUrl(serverId)).toBeNull(); + }); + + test("the server's own token is accepted", async () => { + const added = await store.addCustomServer({ + id: serverId, + title: "Collector", + url: LEGITIMATE_URL, + credentialId: ownCredentialId, + by: "admin@example.com", + }); + + expect(added.id).toBe(serverId); + expect(await storedUrl(serverId)).toBe(LEGITIMATE_URL); + // This is the case the field exists for, so the token does go out, to the address it was given. + expect(sent[0]?.url).toContain("legit.vendor.example"); + expect(sent[0]?.authorization).toContain(OWN_TOKEN); + }); +}); + +describe("a credential is spent only at the address it was given", () => { + test("re-adding the server at a different address is refused", async () => { + // The case a check on whose credential it is cannot see: the token really does belong to this + // server. What changed is where the server points, and the add would spend the credential + // against the new address in the same call. + await expect( + store.addCustomServer({ + id: serverId, + title: "Collector", + url: CHOSEN_URL, + credentialId: ownCredentialId, + by: "admin@example.com", + }), + ).rejects.toBeInstanceOf(CustomServerRefusedError); + + expect(sent).toEqual([]); + expect(await storedUrl(serverId)).toBe(LEGITIMATE_URL); + }); + + test("re-adding it at the address it already has still works", async () => { + // Adding twice is not an attack and must stay ordinary: it is how a title is corrected and how + // an interrupted add is retried. + const added = await store.addCustomServer({ + id: serverId, + title: "Collector, renamed", + url: LEGITIMATE_URL, + credentialId: ownCredentialId, + by: "admin@example.com", + }); + + expect(added.title).toBe("Collector, renamed"); + expect(await storedUrl(serverId)).toBe(LEGITIMATE_URL); + }); + + test("a server holding no credential can still be re-addressed", async () => { + // Nothing to misdirect, so nothing to refuse. The rule is about spending a secret somewhere it + // was not entrusted to, not about URLs being immutable. + const openServerId = `binding-open-${tag}`; + await store.addCustomServer({ + id: openServerId, + title: "Open", + url: LEGITIMATE_URL, + by: "admin@example.com", + }); + + const moved = await store.addCustomServer({ + id: openServerId, + title: "Open", + url: CHOSEN_URL, + by: "admin@example.com", + }); + + expect(moved.id).toBe(openServerId); + expect(await storedUrl(openServerId)).toBe(CHOSEN_URL); + expect(sent.every((call) => call.authorization === null)).toBe(true); + }); +}); + +/** + * The way a token used to outlive the server it belonged to, and become spendable again. + * + * Three ordinary administrative acts in a row, none of them suspicious on its own. This is the shape + * that makes "a credential belongs to its server" and "a server keeps its address" both true and + * still not enough: the address rule only fires when a row is already here, so anything that gets + * the row out of the way while the token stays live reopens the same door. + */ +describe("a token does not outlive the server it was given to", () => { + const holderId = `binding-holder-${tag}`; + const holderCredentialId = randomUUID(); + const HOLDER_TOKEN = `sk-holder-${tag}`; + + beforeAll(async () => { + await database.insert(credentials).values({ + id: holderCredentialId, + kind: "mcp", + provider: holderId, + keyId: `mcp-${holderId}`, + encryptedValue: await encryptSecret(KEY, HOLDER_TOKEN), + metadata: {}, + }); + }); + + afterAll(async () => { + // The server row first: it holds a foreign key onto the credential, so the other order is + // refused by the database rather than by anything this suite is testing. + await database.delete(mcpTools).where(eq(mcpTools.serverId, holderId)); + await database.delete(mcpServers).where(eq(mcpServers.id, holderId)); + await database + .delete(credentials) + .where(eq(credentials.id, holderCredentialId)); + }); + + test("re-adding without a token keeps the one the server already holds", async () => { + // Clearing it was the first link: the row stops naming the credential, so nothing later knows + // the credential belongs to anything, and nothing retires it. + await store.addCustomServer({ + id: holderId, + title: "Holder", + url: LEGITIMATE_URL, + credentialId: holderCredentialId, + by: "admin@example.com", + }); + + await store.addCustomServer({ + id: holderId, + title: "Holder, renamed", + url: LEGITIMATE_URL, + by: "admin@example.com", + }); + + const [row] = await database + .select({ credentialId: mcpServers.credentialId }) + .from(mcpServers) + .where(eq(mcpServers.id, holderId)); + expect(row?.credentialId).toBe(holderCredentialId); + }); + + test("removing the server retires its token", async () => { + await store.removeServer(holderId, "admin@example.com"); + + const [row] = await database + .select({ revokedAt: credentials.revokedAt }) + .from(credentials) + .where(eq(credentials.id, holderCredentialId)); + expect(row?.revokedAt).not.toBeNull(); + }); + + test("a retired token cannot be attached to a server again", async () => { + // The end of the chain. Even with the row gone, so the address rule has nothing to compare + // against, the credential itself is no longer spendable. + sent = []; + + await expect( + store.addCustomServer({ + id: holderId, + title: "Holder", + url: CHOSEN_URL, + credentialId: holderCredentialId, + by: "admin@example.com", + }), + ).rejects.toBeInstanceOf(CustomServerRefusedError); + + expect(sent).toEqual([]); + }); +}); diff --git a/server/tests/plugin-curated-credential.integration.test.ts b/server/tests/plugin-curated-credential.integration.test.ts new file mode 100644 index 00000000..65c9171c --- /dev/null +++ b/server/tests/plugin-curated-credential.integration.test.ts @@ -0,0 +1,261 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { randomUUID } from "node:crypto"; +import { eq, inArray } from "drizzle-orm"; +import { createAuditStore } from "../src/audit"; +import { encryptSecret } from "../src/credentials"; +import { createDatabase } from "../src/db/client"; +import { credentials, mcpServers, mcpTools } from "../src/db/schema"; +import { CATALOGUE, serverCredentialKind } from "../src/plugins/catalogue"; +import { + CustomServerRefusedError, + createPluginStore, +} from "../src/plugins/store"; +import { TEST_POOL } from "./support/database"; + +/** + * Which credential a curated server is allowed to be pointed at. + * + * `addCustomServer` was given this rule and `addServer`, one function above it, was not: it takes the + * same `credentialId` from the same administrator's request and stored it unread. The two paths are + * a pair, and a guard on one of them is a guard on the path somebody happened to look at. + * + * What is reachable today is narrower than the custom case and worth stating rather than dressing + * up. `mcp_servers.credential_id` is a real foreign key, so an id naming nothing is refused by the + * database, and the one entry in the catalogue is `user-oauth`, whose client is registered through + * `registerOAuthClient` and sent to a pinned vendor address. What is left is a credential of the + * wrong kind being accepted and spent, a malformed id arriving as a database error where a refusal + * belongs, and the whole hole reopening the moment a fork re-adds a `deployment-bearer` vendor, + * which the catalogue's own comment invites. + */ + +const database = createDatabase( + process.env.DATABASE_URL ?? + "postgres://openbot:openbot@localhost:5432/openbot", + TEST_POOL, +); + +const store = createPluginStore({ + database, + auditStore: createAuditStore(database), + credentials: { + readSecret: async () => null, + create: async () => { + throw new Error("this suite does not write credentials"); + }, + revoke: async () => { + throw new Error("this suite does not revoke credentials"); + }, + }, + encryptionKey: "x".repeat(44), + policy: () => ({ mode: "enforce", deny: [], allow: ["true"] }), +}); + +/** The catalogue key under test. Real, because which credential it takes is a property of the entry. */ +const serverId = "google-drive"; +const suffix = randomUUID().slice(0, 8); +const deploymentCredentialId = randomUUID(); +const personalCredentialId = randomUUID(); +const oauthClientCredentialId = randomUUID(); + +/** + * Whether this deployment already had the server, and what it pointed at. + * + * The id is a real catalogue key rather than a suite-scoped one, so on a database somebody is using + * it is their configured server. It is removed only when this suite is what created it, and left + * pointing where it pointed before when it is not. + */ +let existing: { credentialId: string | null } | null = null; + +beforeAll(async () => { + const [row] = await database + .select({ credentialId: mcpServers.credentialId }) + .from(mcpServers) + .where(eq(mcpServers.id, serverId)); + existing = row ?? null; + + const encrypted = await encryptSecret(`${"A".repeat(43)}=`, "not-read-here"); + await database.insert(credentials).values([ + { + id: deploymentCredentialId, + kind: "mcp", + provider: serverId, + keyId: `mcp-${serverId}-${suffix}`, + encryptedValue: encrypted, + metadata: {}, + }, + { + id: oauthClientCredentialId, + kind: "mcp_oauth_client", + provider: serverId, + keyId: `oauth-client-${serverId}-${suffix}`, + encryptedValue: encrypted, + metadata: {}, + }, + { + id: personalCredentialId, + kind: "mcp_user_token", + provider: serverId, + // For a user token the key is the person, which is what makes one pickable by name from the + // administrator's own credential list. + keyId: `user_someone_else_${suffix}`, + encryptedValue: encrypted, + metadata: {}, + }, + ]); +}); + +afterAll(async () => { + if (existing) { + await database + .update(mcpServers) + .set({ credentialId: existing.credentialId }) + .where(eq(mcpServers.id, serverId)); + } else { + await database.delete(mcpTools).where(eq(mcpTools.serverId, serverId)); + await database.delete(mcpServers).where(eq(mcpServers.id, serverId)); + } + await database + .delete(credentials) + .where( + inArray(credentials.id, [ + deploymentCredentialId, + oauthClientCredentialId, + personalCredentialId, + ]), + ); +}); + +describe("a curated server may only be pointed at its own kind of credential", () => { + test("somebody else's connector token is refused, and nothing is written", async () => { + await expect( + store.addServer({ + key: serverId, + credentialId: personalCredentialId, + by: "admin@example.com", + }), + ).rejects.toBeInstanceOf(CustomServerRefusedError); + + // The refusal has to stop the write, not merely report on it: a row here is a pointer the next + // refresh dereferences. + const rows = await database + .select({ id: mcpServers.id }) + .from(mcpServers) + .where(eq(mcpServers.id, serverId)); + expect(rows).toHaveLength(existing ? 1 : 0); + }); + + test("a deployment token is refused for a vendor reached as the person asking", async () => { + // The right kind for a shared-token server and the wrong thing entirely for this one. Drive is + // answered with each person's own grant, and the deployment's OAuth client is registered through + // its own call, so there is no credential for this path to be given at all. + await expect( + store.addServer({ + key: serverId, + credentialId: deploymentCredentialId, + by: "admin@example.com", + }), + ).rejects.toBeInstanceOf(CustomServerRefusedError); + }); + + test("a malformed credential id is a refusal rather than a database error", async () => { + // `credentials.id` is a uuid column, so a value that is not one makes the query itself fail and + // the administrator gets a 500 where a refusal belongs. The same was true of the custom path + // before its shape check, and it is the reason that check reads the shape before the lookup. + const refused = store + .addServer({ + key: serverId, + credentialId: "not-a-uuid", + by: "admin@example.com", + }) + .catch((error: Error) => error); + expect(await refused).toBeInstanceOf(CustomServerRefusedError); + }); + + test("adding it again leaves the registered OAuth client where it was", async () => { + /* + * `registerOAuthClient` keeps the client it minted in this column, and adding the server again + * to change an instance host says nothing about that client. Clearing it orphaned a credential + * row that nothing revokes and told everybody who had connected that the deployment has no + * client registered, and there is no way to hand it back through this call now that a + * `user-oauth` entry refuses a credential id. + */ + await store.addServer({ key: serverId, by: "admin@example.com" }); + await database + .update(mcpServers) + .set({ credentialId: oauthClientCredentialId }) + .where(eq(mcpServers.id, serverId)); + + await store.addServer({ key: serverId, by: "admin@example.com" }); + + const [row] = await database + .select({ credentialId: mcpServers.credentialId }) + .from(mcpServers) + .where(eq(mcpServers.id, serverId)); + expect(row?.credentialId).toBe(oauthClientCredentialId); + + // Put it back, so the case below reads the column this suite left rather than this one. + await database + .update(mcpServers) + .set({ credentialId: null }) + .where(eq(mcpServers.id, serverId)); + }); + + test("adding the server without a credential still works", async () => { + // The case that must keep passing, so the refusals above are a rule and not a wall. This is also + // how the admin screen adds this vendor: it sends no credential and registers the OAuth client + // afterwards. + const added = await store.addServer({ + key: serverId, + by: "admin@example.com", + }); + expect(added.id).toBe(serverId); + + const [row] = await database + .select({ credentialId: mcpServers.credentialId }) + .from(mcpServers) + .where(eq(mcpServers.id, serverId)); + expect(row?.credentialId).toBeNull(); + }); +}); + +/** + * Every entry the catalogue actually holds, asked the same question. + * + * The shared-token branch cannot be reached today: the catalogue is frozen in code and its one entry + * is reached as the person asking. Rather than add a seam to this store so a test can invent an + * entry, the check is written over whatever the catalogue contains, so the branch starts being + * exercised the moment somebody re-adds one of the vendors that were taken out. That is the review + * where it matters, and this is the test that will be sitting there when it happens. + */ +describe("every curated entry is asked which credential it takes", () => { + test("the catalogue's own entries decide it, whatever they are", async () => { + expect(CATALOGUE.length).toBeGreaterThan(0); + + for (const entry of CATALOGUE) { + const kind = serverCredentialKind(entry); + + if (kind === null) { + // Takes none from the caller, so any id is refused, including one of the right kind. + await expect( + store.addServer({ + key: entry.key, + credentialId: deploymentCredentialId, + by: "admin@example.com", + }), + ).rejects.toBeInstanceOf(CustomServerRefusedError); + continue; + } + + // A shared-token entry takes the deployment's token for that server and nothing else. The + // fixture credential belongs to a different server, so it is refused on ownership, which is + // the branch a wrong pointer would take. + await expect( + store.addServer({ + key: entry.key, + credentialId: deploymentCredentialId, + by: "admin@example.com", + }), + ).rejects.toBeInstanceOf(CustomServerRefusedError); + } + }); +}); diff --git a/server/tests/plugin-routes.integration.test.ts b/server/tests/plugin-routes.integration.test.ts new file mode 100644 index 00000000..e25aab01 --- /dev/null +++ b/server/tests/plugin-routes.integration.test.ts @@ -0,0 +1,268 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { randomUUID } from "node:crypto"; +import { eq, inArray } from "drizzle-orm"; +import { createApp } from "../src/app"; +import { createAuditStore } from "../src/audit"; +import { loadConfig } from "../src/config"; +import { encryptSecret } from "../src/credentials"; +import { createDatabase } from "../src/db/client"; +import { credentials, mcpServers, mcpTools } from "../src/db/schema"; +import { createPluginStore } from "../src/plugins/store"; +import { TEST_POOL } from "./support/database"; +import { testEnvironment } from "./support/environment"; + +/** + * The whole path an administrator's request actually takes, with nothing stubbed between the request + * and the row. + * + * The two halves are covered on their own: the store's refusals against a real database, and the + * route's mapping of them against a stubbed store. Both passing does not prove the pair is wired + * together, and the failure that would live in the gap is quiet in exactly the way that matters: a + * refusal that reaches the browser as a 500 reads as a broken deployment rather than a correctable + * mistake, and a refusal that stops short of the write leaves a row pointing at a credential the + * next refresh spends. So this asks the question end to end and then looks in the table. + */ + +const database = createDatabase( + process.env.DATABASE_URL ?? + "postgres://openbot:openbot@localhost:5432/openbot", + TEST_POOL, +); + +const store = createPluginStore({ + database, + auditStore: createAuditStore(database), + credentials: { + // Never read: Drive's tool list is in this deployment's own code, so the add path here reaches + // no vault. Loud rather than absent, so a call that starts reaching one is named. + readSecret: async () => { + throw new Error("this suite does not read credentials"); + }, + create: async () => { + throw new Error("this suite does not write credentials"); + }, + revoke: async () => { + throw new Error("this suite does not revoke credentials"); + }, + }, + encryptionKey: "x".repeat(44), + policy: () => ({ mode: "enforce", deny: [], allow: ["true"] }), +}); + +const ADMIN = { + id: "admin-1", + email: "admin@openbot.test", + name: "An Administrator", + image: null, +}; + +function request( + body: unknown, + role: "admin" | "user" = "admin", + path = "/api/plugins/servers", +) { + const app = createApp( + loadConfig(testEnvironment()), + { + handler: () => new Response(null, { status: 204 }), + api: { getSession: async () => ({ user: ADMIN }) }, + } as never, + { rolesForUser: async () => [role] }, + // Positions 4-14 are the other stores; the real one is 15, pluginStore. + ...(Array.from({ length: 11 }) as never[]), + store as never, + ); + + return app.request(`http://openbot.test${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +const serverId = "google-drive"; +const suffix = randomUUID().slice(0, 8); +const personalCredentialId = randomUUID(); +const customServerId = `route-custom-${suffix}`; +const foreignCredentialId = randomUUID(); +const ownCredentialId = randomUUID(); + +/** What this deployment already had, so a database somebody is using is left as it was found. */ +let existing: { credentialId: string | null } | null = null; + +beforeAll(async () => { + const [row] = await database + .select({ credentialId: mcpServers.credentialId }) + .from(mcpServers) + .where(eq(mcpServers.id, serverId)); + existing = row ?? null; + + const encrypted = await encryptSecret(`${"A".repeat(43)}=`, "not-read-here"); + await database.insert(credentials).values([ + { + id: personalCredentialId, + kind: "mcp_user_token", + provider: serverId, + keyId: `user_someone_else_${suffix}`, + encryptedValue: encrypted, + metadata: {}, + }, + { + id: foreignCredentialId, + kind: "mcp", + // Minted for a different server, which is what makes it somebody else's to spend. + provider: `route-elsewhere-${suffix}`, + keyId: `mcp-elsewhere-${suffix}`, + encryptedValue: encrypted, + metadata: {}, + }, + { + id: ownCredentialId, + kind: "mcp", + provider: customServerId, + keyId: `mcp-${customServerId}`, + encryptedValue: encrypted, + metadata: {}, + }, + ]); +}); + +afterAll(async () => { + if (existing) { + await database + .update(mcpServers) + .set({ credentialId: existing.credentialId }) + .where(eq(mcpServers.id, serverId)); + } else { + await database.delete(mcpTools).where(eq(mcpTools.serverId, serverId)); + await database.delete(mcpServers).where(eq(mcpServers.id, serverId)); + } + await database.delete(mcpTools).where(eq(mcpTools.serverId, customServerId)); + await database.delete(mcpServers).where(eq(mcpServers.id, customServerId)); + await database + .delete(credentials) + .where( + inArray(credentials.id, [ + personalCredentialId, + foreignCredentialId, + ownCredentialId, + ]), + ); +}); + +async function serverRow() { + const [row] = await database + .select({ credentialId: mcpServers.credentialId }) + .from(mcpServers) + .where(eq(mcpServers.id, serverId)); + return row ?? null; +} + +describe("adding a curated server over HTTP", () => { + test("a credential of the wrong kind is refused, and nothing is written", async () => { + const before = await serverRow(); + + const response = await request({ + key: serverId, + credentialId: personalCredentialId, + }); + + // Not a 500. An administrator who picked the wrong row is told what to do about it. + expect(response.status).toBe(400); + expect((await response.json()).error).toContain( + "takes no credential when it is added", + ); + + // And the refusal stopped the write rather than reporting on it. + expect(await serverRow()).toEqual(before); + }); + + test("a malformed credential id is refused the same way, not as a database error", async () => { + const response = await request({ key: serverId, credentialId: "nonsense" }); + + expect(response.status).toBe(400); + }); + + test("the add the admin screen makes still works and writes the row", async () => { + const response = await request({ key: serverId }); + + expect(response.status).toBe(200); + expect((await response.json()).server.id).toBe(serverId); + // Whatever the column held before, not null: an add that names no credential leaves a registered + // OAuth client alone, so asserting null here would pass on a fresh database and fail on the one + // deployment shape that behaviour exists for. + expect(await serverRow()).toEqual({ + credentialId: existing?.credentialId ?? null, + }); + }); + + test("somebody who is not an administrator is refused before the store", async () => { + const response = await request({ key: serverId }, "user"); + + expect(response.status).toBe(403); + }); +}); + +/** + * The same two rules, asked over HTTP against the real store. + * + * Both are refusals an administrator has to be able to act on, so what they must never be is a 500: + * "something went wrong" sends somebody to look at the deployment when the answer is to pick a + * different token or remove the server first. + */ +describe("adding a server by URL over HTTP", () => { + const custom = "/api/plugins/servers/custom"; + + test("another server's token is refused rather than spent", async () => { + const response = await request( + { + id: customServerId, + title: "Collector", + url: "https://collector.attacker.example/mcp", + credentialId: foreignCredentialId, + }, + "admin", + custom, + ); + + expect(response.status).toBe(400); + + const rows = await database + .select({ id: mcpServers.id }) + .from(mcpServers) + .where(eq(mcpServers.id, customServerId)); + expect(rows).toHaveLength(0); + }); + + test("re-addressing a server that holds a token is refused", async () => { + const added = await request( + { + id: customServerId, + title: "Collector", + url: "https://legit.vendor.example/mcp", + credentialId: ownCredentialId, + }, + "admin", + custom, + ); + expect(added.status).toBe(200); + + const moved = await request( + { + id: customServerId, + title: "Collector", + url: "https://collector.attacker.example/mcp", + credentialId: ownCredentialId, + }, + "admin", + custom, + ); + expect(moved.status).toBe(400); + + const [row] = await database + .select({ url: mcpServers.url }) + .from(mcpServers) + .where(eq(mcpServers.id, customServerId)); + expect(row?.url).toBe("https://legit.vendor.example/mcp"); + }); +}); diff --git a/server/tests/plugin-routes.test.ts b/server/tests/plugin-routes.test.ts new file mode 100644 index 00000000..cace693d --- /dev/null +++ b/server/tests/plugin-routes.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, test } from "bun:test"; +import { createApp } from "../src/app"; +import { loadConfig } from "../src/config"; +import { + CatalogueEntryUnknownError, + CustomServerRefusedError, +} from "../src/plugins/store"; +import { testEnvironment } from "./support/environment"; + +/** + * What a refused add looks like to the administrator who made it. + * + * The store's refusals are tested where they are decided. What is worth pinning here is the mapping, + * because an unmapped throw leaves the route on its default path: the refusal becomes a 500, the + * screen says something went wrong, and a correctable mistake reads as a broken deployment. The + * curated route mapped one refusal and not the other, which is exactly the shape that is invisible + * until somebody hits it. + */ + +const ADMIN = { + id: "admin-1", + email: "admin@openbot.test", + name: "An Administrator", + image: null, +}; + +function appWith( + addServer: () => Promise, + role: "admin" | "user" = "admin", +) { + const store = { + addServer, + // Every read the plugins surface makes on its way to the route under test. + listServers: async () => [], + listSkills: async () => [], + listGrants: async () => [], + }; + + const app = createApp( + loadConfig(testEnvironment()), + { + handler: () => new Response(null, { status: 204 }), + api: { getSession: async () => ({ user: ADMIN }) }, + } as never, + { rolesForUser: async () => [role] }, + // Positions 4-14 are the other stores; `store` is 15, pluginStore. + ...(Array.from({ length: 11 }) as never[]), + store as never, + ); + + return (body: unknown) => + app.request("http://openbot.test/api/plugins/servers", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +describe("adding a curated server", () => { + test("a refused credential comes back as a refusal with its reason", async () => { + const request = appWith(async () => { + throw new CustomServerRefusedError( + "That is not a credential this server can use. Add the server's own token instead.", + ); + }); + + const response = await request({ + key: "google-drive", + credentialId: "11111111-1111-1111-1111-111111111111", + }); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ + error: + "That is not a credential this server can use. Add the server's own token instead.", + }); + }); + + test("an unknown catalogue key still comes back the same way", async () => { + const request = appWith(async () => { + throw new CatalogueEntryUnknownError("nope"); + }); + + expect((await request({ key: "nope" })).status).toBe(400); + }); + + test("a failure that is not a refusal is not dressed up as one", async () => { + // The must-not case. Mapping every throw to 400 would tell an administrator to correct their + // input when the database is down, and would hide a real fault behind a message about + // credentials. + const request = appWith(async () => { + throw new Error("the database is unreachable"); + }); + + expect((await request({ key: "google-drive" })).status).toBe(500); + }); + + test("somebody who is not an administrator cannot add one at all", async () => { + const request = appWith(async () => { + throw new Error("the store must not be reached"); + }, "user"); + + expect((await request({ key: "google-drive" })).status).toBe(403); + }); +}); diff --git a/server/tests/plugin-store.integration.test.ts b/server/tests/plugin-store.integration.test.ts index 00fe637b..646c108c 100644 --- a/server/tests/plugin-store.integration.test.ts +++ b/server/tests/plugin-store.integration.test.ts @@ -2279,6 +2279,12 @@ describe("a custom server may only be pointed at its own kind of credential", () const deploymentCredentialId = randomUUID(); const personalCredentialId = randomUUID(); const oauthClientCredentialId = randomUUID(); + /** + * The upsert case gets its own token, because a credential names the server it was minted for and + * that case adds a second server id. Sharing one row across two ids is a shape `storeMcpToken` + * cannot produce: it sets the provider to the server it is minting for, every time. + */ + const upsertCredentialId = randomUUID(); const customServerId = `custom-cred-${suffix}`; const madeServerIds: string[] = []; @@ -2306,6 +2312,14 @@ describe("a custom server may only be pointed at its own kind of credential", () encryptedValue: encrypted, metadata: {}, }, + { + id: upsertCredentialId, + kind: "mcp", + provider: `${customServerId}-upsert`, + keyId: `${customServerId}-upsert`, + encryptedValue: encrypted, + metadata: {}, + }, { id: oauthClientCredentialId, kind: "mcp_oauth_client", @@ -2463,7 +2477,7 @@ describe("a custom server may only be pointed at its own kind of credential", () id, title: "Collector", url: "https://collector.example/mcp", - credentialId: deploymentCredentialId, + credentialId: upsertCredentialId, by: "admin@example.com", }); @@ -2481,7 +2495,7 @@ describe("a custom server may only be pointed at its own kind of credential", () .select({ credentialId: mcpServers.credentialId }) .from(mcpServers) .where(eq(mcpServers.id, id)); - expect(row?.credentialId).toBe(deploymentCredentialId); + expect(row?.credentialId).toBe(upsertCredentialId); }); test("a custom server with no credential at all still works", async () => { From a46b5f91d2462adeaa3fe556ddca64e24bcf9eeb Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:22:16 -0500 Subject: [PATCH 10/10] Carry the per-Bot egress proxy as far as the process that reads it (#250) * Carry the per-Bot egress proxy as far as the process that reads it `EGRESS_PROXY_DEFAULT` and `EGRESS_PROXY_` are documented in .env.example and docs/configuration.md, and neither reached any process. docker-compose.yml named no EGRESS variable and had no `env_file`, and Compose hands a container only what those two blocks name. So the shared computer resolved every Bot to null and went out directly, and in the supervisor arrangement the supervisor's own environment held none either, leaving its EGRESS_PROXY passthrough with nothing to forward into the computers it creates. Nothing said so. The operator sets a proxy, the stack starts, the browser leaves by the host, and the Computers screen reports "Leaves directly" because it is reading the same empty environment. For a setting whose stated purpose is to give a security team a per-Bot address for network rules, silently doing nothing is the worst of the available failures. A file rather than more `environment:` entries because `EGRESS_PROXY_` is derived from a Bot's id, so there is no fixed set of names to write out here. A file of its own rather than .env because that one holds the deployment's secrets, and the container driving a browser and running a Bot's shell is deliberately given what it needs and not the rest. It is optional, since going out directly is the ordinary case and must still start, and gitignored, because a proxy URL can carry a password. The all-in-one image was never affected: its s6 service runs under `with-contenv` and inherits the container's environment, which is the mechanism this restores for Compose. * Show a dot on a channel a Bot has spoken in unseen (#259) * Give a membership a memory of when its channel was last read * Stamp the caller's membership read, and say so in the roster * Let a member say they have read a channel * Carry the read marker to the app and let it be stamped * Draw a dot on a channel a Bot has spoken in unseen * Mark a channel read the moment it is the one on screen * Narrow the admin and settings rails to the width their labels earn * Keep a fast clock elsewhere from turning mark-read into a storm * Say in the changelog what the unread dot is and is not * Stamp a read against the message clock, not just this one * Assert the clamped stamp without reaching through an optional chain * Regenerate the read-marker migration behind the queue and the frames * Refuse to mark a deleted channel read, matching the pin * Update azure/setup-helm action to v5 (#260) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Say what a strict content-security-policy has to allow (#225) * Point the test at the database the project actually has (#234) * Let the API reach Intelligence and sign-in when a NetworkPolicy is on (#257) * Refuse a credential written into the rest of the address (#230) Refusing a credential in the userinfo closed one spelling and left the two beside it open. A token in the query string or the fragment was accepted, and addCustomServer writes the address it was given into mcp_servers.url and into the configuration.changed audit payload verbatim. Redaction keys on the field name and url is not a sensitive one, so the secret landed in an append-only row in clear text, which is the disclosure the userinfo rule exists to prevent. The name is read rather than matched against a list. An exact-name version of this rule refused token and accepted auth_token, api_token, x-api-key and X-Amz-Signature, and an operator has no way to know which spellings the check happens to hold. Reading the name over-refuses in one direction on purpose: a misread parameter costs a rename, a missed one cannot be deleted afterwards. The fragment is split at the first question mark first, because a hash route or an OAuth-style callback puts a path in front of the parameters and reading the whole fragment as one query string turns all of it into a single name that matches nothing. metadata.goog is refused too, by asking the list browsing already uses rather than keeping a second copy here. It is Google's own short alias for the metadata server and it carries a dot and none of the suffixes this check lists, so it read as an ordinary vendor name, while the long spelling was refused only incidentally by the .internal test. * Spend an MCP token only for its own server, and only at its own address (#238) * Point a curated MCP server only at a credential of its own kind Adding a server by URL checks which credential it is being pointed at. Adding one from the catalogue took the same field from the same request and stored it unread, so a credential of any kind could be attached to a curated server and spent by the refresh that runs before the add returns. The reach is narrower than the path beside it and worth saying so. The column is a foreign key, so an id naming nothing was already refused by the database, and the one entry in the catalogue is reached with each person's own account, whose OAuth client is registered through its own call and sent to a pinned address. What was reachable is a credential of the wrong kind being accepted and spent on behalf of somebody who never agreed to it, a malformed id arriving as a database error where a refusal belongs, and the whole shape returning with the first deployment-bearer entry a fork re-adds, which the catalogue invites. Which kind an entry takes is decided beside the entry, because it is a property of the vendor's auth rather than of the request. Both add paths then ask one function the same question, so a credential that does not exist and one of the wrong kind are still refused in the same words and the endpoint cannot be asked which ids are real. The curated route maps that refusal to a 400 rather than letting it surface as a 500. Re-adding a curated server no longer clears the credential it points at. That column holds the OAuth client registering one put there, and a re-add to change an instance host said nothing about it while clearing it anyway, leaving the row orphaned and everybody who had connected told there is no client registered. * Spend an MCP token only for its own server, and only at its own address Attaching a credential to a server is the one place this deployment accepts a reference to a stored secret rather than the secret itself. Everywhere else the value arrives in the request that stores it, and the id it gets is nobody's to choose: storeAgentAuth mints its own row from the key an administrator typed. So this is the field where which secret and which address can be made to disagree, and the add is what settles it, because refreshTools runs before the call returns and sends what it decrypts to the URL from the same request. Both ways they could disagree are now refused. A credential has to belong to the server it is attached to, which the vault already records: storeMcpToken sets the provider to the server it mints for and is the only way the plugins screen makes one, so nothing a deployment can reach through the UI is refused by this. And a server that already holds a credential cannot be re-added at a different address, which is the case a check on ownership cannot see: the token does belong to that server, and only the address moved. The second is why the first is not enough alone. Both delivered a stored token to a host the caller named, before any Bot, grant or policy check existed, and a stored credential is otherwise unreadable by design. Refused rather than repaired, because both harmless readings are served by something else. Correcting a title or retrying an interrupted add sends the same URL and is untouched, a server holding no credential can still be re-addressed, and moving one that does means removing it and adding it again with the token the new address is meant to have. Curated servers are unaffected: their URL comes from the catalogue rather than the request, and an instance hostname is matched against the vendor's anchored pattern before anything is stored. The upsert test from #214 now mints its own token. It had reused one credential across two server ids, which is a shape storeMcpToken cannot produce. * Show a dot on a channel a Bot has spoken in unseen (#259) * Give a membership a memory of when its channel was last read * Stamp the caller's membership read, and say so in the roster * Let a member say they have read a channel * Carry the read marker to the app and let it be stamped * Draw a dot on a channel a Bot has spoken in unseen * Mark a channel read the moment it is the one on screen * Narrow the admin and settings rails to the width their labels earn * Keep a fast clock elsewhere from turning mark-read into a storm * Say in the changelog what the unread dot is and is not * Stamp a read against the message clock, not just this one * Assert the clamped stamp without reaching through an optional chain * Regenerate the read-marker migration behind the queue and the frames * Refuse to mark a deleted channel read, matching the pin * Update azure/setup-helm action to v5 (#260) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Say what a strict content-security-policy has to allow (#225) * Point the test at the database the project actually has (#234) * Let the API reach Intelligence and sign-in when a NetworkPolicy is on (#257) * Refuse a credential written into the rest of the address (#230) Refusing a credential in the userinfo closed one spelling and left the two beside it open. A token in the query string or the fragment was accepted, and addCustomServer writes the address it was given into mcp_servers.url and into the configuration.changed audit payload verbatim. Redaction keys on the field name and url is not a sensitive one, so the secret landed in an append-only row in clear text, which is the disclosure the userinfo rule exists to prevent. The name is read rather than matched against a list. An exact-name version of this rule refused token and accepted auth_token, api_token, x-api-key and X-Amz-Signature, and an operator has no way to know which spellings the check happens to hold. Reading the name over-refuses in one direction on purpose: a misread parameter costs a rename, a missed one cannot be deleted afterwards. The fragment is split at the first question mark first, because a hash route or an OAuth-style callback puts a path in front of the parameters and reading the whole fragment as one query string turns all of it into a single name that matches nothing. metadata.goog is refused too, by asking the list browsing already uses rather than keeping a second copy here. It is Google's own short alias for the metadata server and it carries a dot and none of the suffixes this check lists, so it read as an ordinary vendor name, while the long spelling was refused only incidentally by the .internal test. --------- Co-authored-by: Guido Vizoso Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Vaibhav Zope <121456155+zopeVaibhav@users.noreply.github.com> Co-authored-by: David McKay --------- Co-authored-by: Guido Vizoso Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Vaibhav Zope <121456155+zopeVaibhav@users.noreply.github.com> Co-authored-by: David McKay --- .env.example | 9 +++++++-- .gitignore | 2 ++ CHANGELOG.md | 21 +++++++++++++++++++++ docker-compose.yml | 14 ++++++++++++++ docs/configuration.md | 22 ++++++++++++++++++++-- tests/compose.test.ts | 32 ++++++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index bfd61b36..e749f1eb 100644 --- a/.env.example +++ b/.env.example @@ -230,8 +230,13 @@ COMPUTER_TOKEN= # # This is attribution, not anonymity, and it is not a boundary by itself: it gives a security team a # per-Bot address for network rules alongside AGENT_COMPUTER_POLICY. -# EGRESS_PROXY_DEFAULT=http://user:password@proxy.internal:8080 -# EGRESS_PROXY_SALES_BOT=http://sales.proxy.internal:8080 +# +# These go in `egress.env` beside this file, NOT here. The names are per-Bot, so Compose cannot +# list them the way it lists every variable below, and it hands a container only what it is told to. +# In `.env` they reach no process and the browser goes out directly with nothing saying so. +# +# EGRESS_PROXY_DEFAULT=http://user:password@proxy.internal:8080 +# EGRESS_PROXY_SALES_BOT=http://sales.proxy.internal:8080 # The managed coworker AG-UI endpoint. Optional: use an HTTP(S) URL, and set MANAGED_AGENT_TOKEN diff --git a/.gitignore b/.gitignore index f1e6ae03..bfc1237c 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,8 @@ docs/plans/ .env .env.* !.env.example +# Per-Bot egress proxies. Carries credentials in the URL, like .env does. +egress.env node_modules/ **/dist/ app/src/lib/generated/application-config.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 805817b0..350328b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -231,6 +231,27 @@ credential is refused rather than quietly attached to fail on its next call. Curated servers keep working as they did. Their URL comes from the catalogue rather than the request, and a per-instance hostname is matched against the vendor's own anchored pattern before anything is stored, so re-adding one cannot point it at an address of the caller's choosing. +### A configured egress proxy reaches the browser that uses it + +`EGRESS_PROXY_DEFAULT` and `EGRESS_PROXY_` were documented as the way to give a Bot a stable +outbound address, and Compose passed neither to anything. `docker-compose.yml` named no egress +variable and had no `env_file`, so the shared computer resolved every Bot to no proxy and went out +directly, and under the supervisor the same emptiness meant there was nothing to forward into the +computers it creates. + +The failure was silent, which for a setting whose purpose is to give a security team a per-Bot +address for network rules is the worst of the available failures. The stack started, the browser +left by the host, and the Computers screen reported "Leaves directly" because it was reading the +same empty environment. + +They now live in `egress.env`, which both the computer and the supervisor are given. A file rather +than more `environment:` entries because `EGRESS_PROXY_` is derived from a Bot's id and there +is no fixed set of names to list; a file of its own rather than `.env` because that one holds the +deployment's secrets and the container running a browser and a Bot's shell is deliberately not +given them. It is optional, so a deployment with no proxy is unchanged, and gitignored, because a +proxy URL can carry a password. + +**Move these two out of `.env` and into `egress.env`.** In `.env` they reach no process. ### Knowledge searches instead of guessing diff --git a/docker-compose.yml b/docker-compose.yml index e2e7cfbc..b420e933 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -61,6 +61,15 @@ services: # Loopback only. This process drives a browser holding real logins; COMPUTER_TOKEN is the # request control, and loopback keeps the surface off routed networks. - "127.0.0.1:${COMPUTER_PORT:-4100}:4100" + # Per-Bot egress, in a file of its own because the names are not knowable here. + # + # `EGRESS_PROXY_` is derived from the Bot's id, so there is no fixed list to write out the + # way COMPUTER_TOKEN is. Not `.env`: that holds the deployment's secrets, and this container + # drives a browser and runs a Bot's shell, so it is given what it needs and not the rest. + # Optional, because going out directly is the ordinary case and must still start. + env_file: + - path: ./egress.env + required: false environment: # The secret every caller must present. The container refuses to start without it. COMPUTER_TOKEN: ${COMPUTER_TOKEN:-} @@ -143,6 +152,11 @@ services: build: context: . dockerfile: supervisor/Dockerfile + # The same file, because this process does not read these itself: it forwards every EGRESS_PROXY + # key out of its own environment into each computer it creates, so it has to be given them first. + env_file: + - path: ./egress.env + required: false environment: PORT: "4300" # Shared with the API server. The Bot-level verb set is the boundary; this token keeps other diff --git a/docs/configuration.md b/docs/configuration.md index be7ff85b..a8291d3c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -191,8 +191,8 @@ where `` is `google`, `microsoft` or `okta`. - `WORKSPACE_DIR` - `PROFILES_DIR` - `COMPUTER_BOT_ID` -- `EGRESS_PROXY_DEFAULT` -- `EGRESS_PROXY_` +- `EGRESS_PROXY_DEFAULT` (in `egress.env`, see below) +- `EGRESS_PROXY_` (in `egress.env`, see below) - `COMPUTER_SHELL_ENV` A command on the computer inherits PATH, locale and terminal names, and the proxy variables, not @@ -200,6 +200,24 @@ the rest of the process environment. Userinfo is stripped from a proxy URL, so a `HTTP_PROXY` is not in `env`. `COMPUTER_SHELL_ENV` is a comma-separated list of extra names to pass. Naming a secret or a credentialed proxy there is an operator's decision; the default does not. +### Per-Bot egress + +The two egress variables live in `egress.env` at the repository root, not in `.env`. `EGRESS_PROXY_` +is derived from a Bot's id, so there is no fixed set of names for Compose to list the way it lists +every other variable, and Compose passes a container only the names it is given. A file of its own +rather than `.env` because that one holds the deployment's secrets and neither the browser container +nor the supervisor is given those. + +```sh +# egress.env +EGRESS_PROXY_DEFAULT=http://user:password@proxy.internal:8080 +EGRESS_PROXY_SALES_BOT=http://sales.proxy.internal:8080 +``` + +The file is optional and gitignored. Without it every Bot's browser goes out directly, which is the +default. Both the shared computer and the supervisor are given it: the computer resolves its own +proxy from these names, and the supervisor forwards them into each computer it creates. + The supervisor also reads: - `COMPUTER_IMAGE` diff --git a/tests/compose.test.ts b/tests/compose.test.ts index ac18455f..b7261b3e 100644 --- a/tests/compose.test.ts +++ b/tests/compose.test.ts @@ -121,3 +121,35 @@ test("runs migrations after PostgreSQL becomes healthy", () => { expect(compose).toContain("condition: service_healthy"); expect(compose).toContain('"drizzle-kit", "migrate"'); }); + +/** + * Per-Bot egress reaches the processes that read it. + * + * `EGRESS_PROXY_` and `EGRESS_PROXY_DEFAULT` are resolved from `process.env` by the computer + * itself (`agent-computer/src/egress.ts`), and the supervisor forwards every `EGRESS_PROXY` key out + * of its own environment into each computer it creates (`supervisor/src/index.ts`). Compose gives a + * container only what its `environment:` and `env_file:` blocks name, and for a long time neither + * named these, so an operator who configured a proxy per the documentation got a browser that went + * out directly and no error saying so. + * + * A file rather than `environment:` entries because the names are per-Bot and therefore not knowable + * here, and a file of its own rather than `.env` because that one holds the deployment's secrets and + * the browser container is deliberately not given them. + */ +test("carries per-Bot egress into the computer and the supervisor", () => { + const compose = readFileSync( + join(import.meta.dir, "..", "docker-compose.yml"), + "utf8", + ); + + // Both halves: the shared computer reads them itself, and the supervisor passes them on. + const services = compose.split(/^ {2}(?=\S)/m); + for (const name of ["agent-computer:", "supervisor:"]) { + const service = services.find((block) => block.startsWith(name)); + expect(service).toBeDefined(); + expect(service).toContain("egress.env"); + } + + // Optional, because a deployment with no proxy is the ordinary case and must still start. + expect(compose).toContain("required: false"); +});