diff --git a/.env.example b/.env.example index bfd61b36..706b402a 100644 --- a/.env.example +++ b/.env.example @@ -7,7 +7,17 @@ KEY_ENCRYPTION_KEY=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= # one to leave alone until somebody has decided otherwise: the trail is append-only and nothing else # can remove a row, so this is the only way it ever shrinks. # AUDIT_RETENTION_DAYS=365 +# Two names for one number, and they have to agree. +# +# The server reads PORT (server/src/index.ts). scripts/start.sh reads SERVER_PORT, because it also +# has to know where the app should proxy and which port to report free -- and docs/configuration.md +# documents SERVER_PORT as the setting. Only PORT shipped here, so moving the server by editing this +# line left the script still looking at 3001: it found whatever else was there, accepted the first +# 200 as proof, and failed several stages later parsing that stranger's HTML as JSON. +# +# Change both, or neither. PORT=3001 +SERVER_PORT=3001 TENANT_PACKAGE_DIR=../examples/fintech # What this deployment calls itself, when more than one shares an Intelligence project. A copy of a # deployment made for development uses the same project key, and threads are listed per Bot with @@ -230,8 +240,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/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c79e8f1..7fa0f51f 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. @@ -149,6 +149,13 @@ jobs: # A browser inside every replica of a replicated API. refuses "an embedded browser across several replicas" \ --set server.embeddedComputer=true --set server.replicaCount=2 + # A Bot's egress proxy on a port the computer's own network policy does not allow. The + # variables reach the computer through extraEnv, so nothing else notices that the policy + # then refuses to let it be reached. + refuses "an egress proxy the network policy blocks" \ + --set networkPolicy.enabled=true \ + --set computers.extraEnv[0].name=EGRESS_PROXY_DEFAULT \ + --set-string computers.extraEnv[0].value=http://proxy.internal:3128 test: name: tests 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 b90ea999..a7cced0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,78 @@ 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`. + +### 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. +### 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 computer that was suspended once suspends again + +Scale-to-zero worked once per Bot. A computer suspended, resumed, used and then left alone again was +offered for suspension on every sweep after that and never suspended, and stayed awake until the next +day. Nothing reported it, because a sweep that offers work and suspends nothing looks exactly like a +fleet that is busy. + +The queue keys a suspension on the Bot id and keeps the finished row so that a late offer of the same +key collides with it rather than running the work twice. Both are right. What was wrong is that the +finished row was kept for a day, which is the window the other half of the sweep needs: a suspension +that keeps failing is held back that long before anything tries it again. One number could not be +both, so there are now two, and a finished suspension is kept for the idle window instead. That is +the same clock the offer runs on, so a Bot cannot come back round as idle until its row has gone. + +Nothing to configure, and the sweep already runs on a schedule. A deployment where each Bot has its +own computer stops paying for browsers that were used once. +### A Bot's egress proxy is reachable on Kubernetes, or the install is refused + +The chart named no egress variable anywhere, so a Helm deployment read the per-Bot proxy settings +nowhere and every Bot went out directly. They were always settable through `computers.extraEnv`, +which reaches the computer in both the shared and the sandbox arrangement, but nothing in the chart +or its README said so, and a setting whose whole purpose is to give a security team a per-Bot +address is not one to leave undocumented. + +The other half is that setting it was not enough. A computer is allowed 80 and 443 to public +addresses and nothing else, which is almost no proxies: they sit on a private address, or on 3128 or +8080. So a proxy the network policy provably blocks is now refused at `helm install`, naming +`networkPolicy.computerExtraEgress`, rather than found later as a Bot that fails on every page. +Nothing changes for a deployment that sets no proxy, or one that already opened a path to it. + ### 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 @@ -136,6 +208,31 @@ unknown session means "no opinion" and skips the generation check, so on exactly shape it was written for, the check that stops a ref from a replaced computer resolving against a live one was silently absent. It now asks the supervisor when it does not know, by listing rather than by ensuring, so asking never starts a computer that had stopped. +### A routing trail says why a message was not routed, not only that it was not + +Every untagged message writes a `channel.routed` row, and that row carried `fallback: true` for two +completely different situations: the router answering honestly that no specialist was a confident +match, which is the feature working, and the router not answering at all, which is an endpoint that is +down. Both read identically, so a deployment whose router had stopped working looked like one whose +messages were simply hard to route. + +That is not hypothetical. The intent router spent an unknown period 404'ing on every deployment that +set `OPENAI_BASE_URL`, because a `/v1` was appended to a URL that already had one. It was fixed in +0.0.3, whose own note says untagged messages "silently stopped being routed and nothing said why". + +The row now carries `undecided`, naming the cause: `unreachable`, `unparsed`, `off-roster`, +`unconfident`, or `one-candidate` — and `null` when the router did decide. Named values rather than a +sentence, because the useful question is how often, and a count needs something to group by. + +Two smaller corrections came with it. A message routed to the only coworker that can reach the system +it names kept that as its reason and threw the cause away, so a router that had been down for a week +produced rows reading like reach-based routing working as intended; the cause now survives that path. +And an answer containing no JSON at all — a model replying in prose — was recorded as the router +naming a coworker off the roster, which sends whoever reads it to look at their roster rather than at +the model. It is now reported as unparsed, which is what it is. + +Nothing changes about where a message goes. Every routing decision is the same decision it was. + ### Notion joins the connector catalogue Notion is now a governed MCP connector, reached through Notion's own hosted server on the @@ -163,6 +260,81 @@ 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. +### 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. +### Screens without a conversation stop polling for a Bot that does not exist + +Every surface asks which components its Bot holds, and asks again every few seconds so a revoked +grant leaves an open conversation quickly. The Bot it asked about was whichever one the surface +declared — and on a screen with no conversation at all, that was the placeholder id the routing +holder falls back to, which no package registers and the server answers 404 for. An admin page left +open polled a guaranteed miss every five seconds, indefinitely. + +Nothing looked wrong. The screen rendered, because an absent grant list and an empty one draw the +same. The cost was the noise: a request log where the same 404 repeats forever is one where the 404 +that matters is invisible. + +The grant queries now wait for a surface to declare a real Bot, and simply do not run while the +placeholder holds. Conversation surfaces — the Bot page, channels — declare one and are unchanged. + ### Knowledge searches instead of guessing A package can say which of its skills each coworker gets, and the fintech example gives Knowledge the @@ -214,6 +386,60 @@ 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. +### 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 @@ -459,6 +685,44 @@ one they are, so those match too. No configuration changes and nothing is stored differently; a deployment that was already on the light theme sees no difference at all. +### `start.sh` refuses a port that answers but is not OpenBot + +The startup checks asked whether a port answered, and treated that as proof the port belonged to this +stack. Those are not the same claim. Any single-page app serves its index.html for every path it does +not recognise, so an unrelated dashboard on a default port answers `200` to `/api/capabilities` as +readily as this server does. + +The cost was not a wrong answer, it was a wrong answer three stages later. `require_free_or_ours` +reported "already up", so the server was never started; `wait_for` then printed a green +"server ready"; and the run failed at stage 3 inside `json.loads`, parsing that stranger's HTML. The +error names `char 0`, which reads like an empty response rather than a `<`, so the visible symptom +pointed nowhere near the port. + +Each surface is now asked for something only it can produce: a `licenseStatus` field for the server, +its own `` for the app, `/health` for the compose services. When a check gives up it says +whether the process failed to start or the port belongs to something else. + +The root cause was in `.env.example`, and is fixed there too. The server reads `PORT`, this script +reads `SERVER_PORT`, `docs/configuration.md` documents `SERVER_PORT` as the setting, and only `PORT` +shipped. Moving the server by editing that one line left the script still looking at 3001. Both names +are now present, next to each other, saying they have to agree. + +**A run may now stop where it used to continue.** That is the point: it stops at the port that is +wrong, naming it, rather than several steps later on a parse error. + +### `docker compose up -d` configures the same stack `scripts/start.sh` does + +`SUPERVISOR_TOKEN` and `COMPUTER_TOKEN` defaulted to the empty string in `docker-compose.yml`, so +which stack you got depended on how you brought it up. `scripts/start.sh` resolves both to their +`openbot-dev-*` defaults and exports them before calling compose. A plain `docker compose up -d` — +which this project's own shutdown notes tell you to use — passed an empty string instead. + +`agent-computer` refuses to start without one, so that half failed loudly. The supervisor half was +the quiet one: the server kept the token it started with while the supervisor held an empty string, +and every call between them was refused at the door. + +Both now carry the same defaults `start.sh` applies, as `COMPUTER_IMAGE` already did two lines down. +A value set in `.env` still wins, and a deployment should set one. ## 0.0.4 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<StreamData>({ } 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<StreamData>({ 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<StreamData>({ 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<StreamData>({ const startedAt = Date.now(); try { - session.control.assertBotMayAct(); const target = await currentPage(botId); const detail = await performAction( session, @@ -932,10 +951,6 @@ serve<StreamData>({ 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(); + } + }); +}); 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 ( <motion.div animate={{ opacity: 1, transform: "translateY(0px)" }} @@ -160,6 +196,7 @@ function ChannelRow({ : undefined } pinned={channel.pinned} + unread={unread} /> </motion.div> ); 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({ </div> <div className="flex-col min-w-0 flex-1"> <div className="flex flex-row items-center justify-between gap-2"> - <span className="text-[14px] tracking-[-1%] truncate"> + <span + className={`text-[14px] tracking-[-1%] truncate ${ + unread ? "font-medium" : "" + }`} + > {name} </span> <div className="text-[12px] text-muted-foreground/70"> @@ -123,6 +129,10 @@ export const Channel = memo(function Channel({ <span className="min-w-0 flex-1 truncate text-[12px] leading-4 text-muted-foreground"> {lastMessage} </span> + {unread ? ( + /* State about the message beats state about the row, so it sits first. */ + <span className="size-2 shrink-0 rounded-full bg-primary" /> + ) : null} {pinned ? ( <IconPinFilled className="size-3 shrink-0 text-muted-foreground/70" /> ) : 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<ChannelPage> | 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/lib/copilot/active-bot.tsx b/app/src/lib/copilot/active-bot.tsx index b66da3aa..dfb8dda2 100644 --- a/app/src/lib/copilot/active-bot.tsx +++ b/app/src/lib/copilot/active-bot.tsx @@ -73,3 +73,24 @@ export function useActiveBotHolder(): BotHolder { export function useActiveBotId(): string { return useContext(ActiveBotValueContext)?.botId ?? DEFAULT_BOT_ID; } + +/** + * The active Bot when a surface has declared one, and undefined while the placeholder holds. + * + * The placeholder exists so a handler always has something to route with, but it is not a Bot: no + * package registers an agent by that name, and the server answers 404 for it. A grant query handed + * the placeholder therefore polls a guaranteed miss on its own interval — every few seconds, on + * every screen without a conversation — and the constant failing request is noise that would bury a + * real 404 the day one matters. A query given undefined instead simply does not run. + * + * The name is already reserved in practice: an agents.yaml entry called "default" would be + * indistinguishable from the placeholder in every handler that reads the holder. + */ +export function declaredBotId(botId: string): string | undefined { + return botId === DEFAULT_BOT_ID ? undefined : botId; +} + +/** As useActiveBotId, for callers that should do nothing while the placeholder holds. */ +export function useDeclaredBotId(): string | undefined { + return declaredBotId(useActiveBotId()); +} diff --git a/app/src/lib/copilot/gallery-tools.tsx b/app/src/lib/copilot/gallery-tools.tsx index 26b39a2a..b421df73 100644 --- a/app/src/lib/copilot/gallery-tools.tsx +++ b/app/src/lib/copilot/gallery-tools.tsx @@ -9,7 +9,7 @@ import { decideComponent, type GrantedComponent, } from "@/lib/components/queries"; -import { useActiveBotId } from "@/lib/copilot/active-bot"; +import { useActiveBotId, useDeclaredBotId } from "@/lib/copilot/active-bot"; import { GALLERY_COMPONENTS, type GalleryComponent, @@ -32,7 +32,10 @@ export function GalleryTools() { // Active Bot comes from the route/channel surface currently driving the provider. const grantsFor = useActiveBotId(); - const { data: granted } = useQuery(agentComponentsQueryOptions(grantsFor)); + // See sandboxed-tools: the placeholder Bot is not fetchable, so the grant query waits for a + // surface to declare a real one. + const declared = useDeclaredBotId(); + const { data: granted } = useQuery(agentComponentsQueryOptions(declared)); const held = useMemo( () => new Map( diff --git a/app/src/lib/copilot/sandboxed-tools.tsx b/app/src/lib/copilot/sandboxed-tools.tsx index 5b570c01..f69aa43f 100644 --- a/app/src/lib/copilot/sandboxed-tools.tsx +++ b/app/src/lib/copilot/sandboxed-tools.tsx @@ -12,7 +12,7 @@ import { decideComponent, type GrantedComponent, } from "@/lib/components/queries"; -import { useActiveBotId } from "@/lib/copilot/active-bot"; +import { useActiveBotId, useDeclaredBotId } from "@/lib/copilot/active-bot"; import { type PublishedSandboxed, publishedSandboxedQueryOptions, @@ -24,8 +24,11 @@ import { */ export function SandboxedTools() { const botId = useActiveBotId(); + // Grants are fetched only once a surface has declared its Bot; the placeholder is not one the + // server knows, and asking would 404 on every poll. + const declared = useDeclaredBotId(); const { data: published } = useQuery(publishedSandboxedQueryOptions()); - const { data: granted } = useQuery(agentComponentsQueryOptions(botId)); + const { data: granted } = useQuery(agentComponentsQueryOptions(declared)); const held = new Map( (granted ?? []).map((component: GrantedComponent) => [ 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 ( <SidebarProvider /* - * The same 340px the app and Settings use. A rail that changes width as you cross into admin - * makes the whole frame look like it moved. + * The same 300px Settings uses: these two rails hold short nav labels, not the roster's + * two-line previews, so they earn less width than the app shell's 340px. */ style={ { - "--sidebar-width": "340px", + "--sidebar-width": "300px", "--sidebar-width-mobile": "20rem", } as React.CSSProperties } diff --git a/app/src/routes/_authed/settings/route.tsx b/app/src/routes/_authed/settings/route.tsx index 9aae2f3e..93e3d935 100644 --- a/app/src/routes/_authed/settings/route.tsx +++ b/app/src/routes/_authed/settings/route.tsx @@ -10,12 +10,12 @@ function RouteComponent() { return ( <SidebarProvider /* - * The same 340px the app shell uses. Settings is a different screen, not a different product, - * and a rail that changes width on the way in makes the whole frame look like it moved. + * The same 300px admin uses: these two rails hold short nav labels, not the roster's + * two-line previews, so they earn less width than the app shell's 340px. */ style={ { - "--sidebar-width": "340px", + "--sidebar-width": "300px", "--sidebar-width-mobile": "20rem", } as React.CSSProperties } diff --git a/app/tests/channel-menu-mutations.test.ts b/app/tests/channel-menu-mutations.test.ts index 5ccad1e1..b0cd4791 100644 --- a/app/tests/channel-menu-mutations.test.ts +++ b/app/tests/channel-menu-mutations.test.ts @@ -1,9 +1,11 @@ import { afterEach, expect, test } from "bun:test"; -import type { QueryClient } from "@tanstack/react-query"; +import { QueryClient, type InfiniteData } from "@tanstack/react-query"; import { deleteChannelMutationOptions, + markChannelReadMutationOptions, setChannelPinnedMutationOptions, } from "../src/lib/channels/mutations"; +import { channelKeys, type ChannelPage } from "../src/lib/channels/queries"; const realFetch = globalThis.fetch; @@ -86,3 +88,87 @@ test("a refused delete surfaces the server's sentence", async () => { "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<ChannelPage>); + 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<InfiniteData<ChannelPage>>( + 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<ChannelPage>); + const options = markChannelReadMutationOptions(queryClient); + + options.onMutate?.("channel-1"); + + const patched = queryClient.getQueryData<InfiniteData<ChannelPage>>( + 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>): 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/app/tests/declared-bot.test.ts b/app/tests/declared-bot.test.ts new file mode 100644 index 00000000..5b7b2244 --- /dev/null +++ b/app/tests/declared-bot.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, test } from "bun:test"; +import { declaredBotId } from "../src/lib/copilot/active-bot"; + +/** + * The placeholder Bot id is a routing convenience, not a Bot. Anything that would ask the server + * about it must be told there is nothing to ask about. + */ + +describe("declaredBotId", () => { + test("returns undefined for the placeholder", () => { + expect(declaredBotId("default")).toBeUndefined(); + }); + + test("passes a declared Bot through", () => { + expect(declaredBotId("general-assistant")).toBe("general-assistant"); + }); + + test("passes a Bot through even when the placeholder is its prefix", () => { + expect(declaredBotId("default-2")).toBe("default-2"); + }); +}); diff --git a/charts/openbot/README.md b/charts/openbot/README.md index e783de07..b43a71c3 100644 --- a/charts/openbot/README.md +++ b/charts/openbot/README.md @@ -216,6 +216,32 @@ the policy on with an external database and no `networkPolicy.extraEgress` is re enforcing cluster it would fence the API off from its own database, which reads as the database being down. +A Bot's computer is allowed 80 and 443 to public addresses and nothing else, which is what stops a +browser reaching the cluster, the database, or the cloud's credential endpoint. A per-Bot egress +proxy is therefore two settings rather than one: the variable that names it, and the rule that lets +the computer reach it. + +```yaml +computers: + extraEnv: + - name: EGRESS_PROXY_DEFAULT + value: http://proxy.internal:3128 + - name: EGRESS_PROXY_SALES_BOT + value: http://sales.proxy.internal:3128 +networkPolicy: + computerExtraEgress: + - to: + - ipBlock: + cidr: 10.4.0.0/16 + ports: + - port: 3128 + protocol: TCP +``` + +`EGRESS_PROXY_DEFAULT` covers every Bot and `EGRESS_PROXY_<BOT>` names one, with the Bot's id +upper-cased and anything unusual replaced. Naming a proxy the policy provably blocks is refused at +install rather than found as a browser that fails on every page. + ## Upgrades Migrations run as a `pre-install,pre-upgrade` Job, so no replica ever serves in front of a schema it 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/templates/validation.yaml b/charts/openbot/templates/validation.yaml index 7f16e1b0..8d34c137 100644 --- a/charts/openbot/templates/validation.yaml +++ b/charts/openbot/templates/validation.yaml @@ -247,3 +247,31 @@ This template renders nothing. {{- end }} {{- end }} {{- end }} + +{{- /* + A Bot's egress proxy, against the network policy that decides whether it can be reached. + + `EGRESS_PROXY_DEFAULT` and `EGRESS_PROXY_<BOT>` are how a Bot gets a stable outbound address, and + on Kubernetes they arrive through `computers.extraEnv`. The computer's own policy allows 80 and + 443 to public addresses and nothing else, which is most of the internet and almost no proxies: + they sit on a private address, or on 3128 or 8080, or both. Setting one without opening a path to + it produces a Bot whose every page fails, which reads as a broken browser rather than as a network + rule, so it is refused here instead. + + Only what the value provably shows: a port that is not 80 or 443, or a literal private address. + A private DNS name on 443 is blocked by the `ipBlock` rules just the same and cannot be told from + a public one at template time, so this misses that case rather than guessing at it. A proxy + supplied through `extraEnvFrom` is not readable here at all. +*/}} +{{- if and .Values.networkPolicy.enabled (not .Values.networkPolicy.computerExtraEgress) }} +{{- range .Values.computers.extraEnv }} +{{- if hasPrefix "EGRESS_PROXY" (.name | default "") }} +{{- $value := .value | default "" | trim | trimSuffix "/" }} +{{- $port := regexFind ":[0-9]+$" $value | trimPrefix ":" }} +{{- $private := regexMatch "(^|//)(10\\.|127\\.|169\\.254\\.|192\\.168\\.|172\\.(1[6-9]|2[0-9]|3[01])\\.)" $value }} +{{- if or (and $port (not (has $port (list "80" "443")))) $private }} +{{- fail (printf "%s names a proxy the computer's network policy blocks, so every page a Bot opens would fail. Name it in networkPolicy.computerExtraEgress, or turn networkPolicy.enabled off." .name) }} +{{- end }} +{{- end }} +{{- end }} +{{- end }} diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index da8e7596..65f98420 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -212,6 +212,11 @@ computers: nodeSelector: {} tolerations: [] + # Free-form additions, and where a Bot's outbound address is set. `EGRESS_PROXY_DEFAULT` covers + # every Bot and `EGRESS_PROXY_<BOT>` names one, upper-cased with anything unusual replaced, so + # `sales-bot` reads `EGRESS_PROXY_SALES_BOT`. Both reach the computer whichever mode it runs in. + # With `networkPolicy.enabled` the proxy also has to be named in + # `networkPolicy.computerExtraEgress`, or the computer cannot reach it. extraEnv: [] # `mode: sandbox` only. Where the per-Bot computers are created and what may create them. @@ -365,8 +370,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/docker-compose.yml b/docker-compose.yml index e2e7cfbc..e89f4f93 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -61,9 +61,24 @@ 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_<BOT>` 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:-} + # + # The default matches the one scripts/start.sh applies when .env leaves this blank, which + # .env.example does. Without it the two paths disagree: the script exports a value and brings + # the stack up working, while a plain `docker compose up -d` -- which this project's own + # shutdown notes tell you to use -- hands the container an empty string instead. Reaches only + # a loopback-bound port; a deployment sets a real value in .env. + COMPUTER_TOKEN: ${COMPUTER_TOKEN:-openbot-dev-computer-token} # How many Bots may hold a running browser at once, and how long an untouched one is kept. # A few hundred MB each, so on a deployment with many Bots these are the difference between # a container that holds steady and one that is killed for memory. Defaults are 8 and 30 @@ -143,13 +158,20 @@ 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 # services on the network from calling those verbs. - SUPERVISOR_TOKEN: ${SUPERVISOR_TOKEN:-} + # Defaulted for the same reason as agent-computer's above: so `docker compose up -d` by hand + # configures the same stack scripts/start.sh does, rather than a silently unauthenticated one. + SUPERVISOR_TOKEN: ${SUPERVISOR_TOKEN:-openbot-dev-supervisor-token} # Handed to every computer this creates, so the server and the computers share one secret. - COMPUTER_TOKEN: ${COMPUTER_TOKEN:-} + COMPUTER_TOKEN: ${COMPUTER_TOKEN:-openbot-dev-computer-token} COMPUTER_IMAGE: ${COMPUTER_IMAGE:-openbot-agent-computer:latest} # Which deployment the computers it creates belong to, so two stacks on one Docker host never # derive the same container and volume names for the same Bot. 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 `<provider>` is `google`, `microsoft` or `okta`. - `WORKSPACE_DIR` - `PROFILES_DIR` - `COMPUTER_BOT_ID` -- `EGRESS_PROXY_DEFAULT` -- `EGRESS_PROXY_<BOT_ID>` +- `EGRESS_PROXY_DEFAULT` (in `egress.env`, see below) +- `EGRESS_PROXY_<BOT_ID>` (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_<BOT_ID>` +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/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. 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.` : ""), diff --git a/scripts/start.sh b/scripts/start.sh index 20536095..54831826 100755 --- a/scripts/start.sh +++ b/scripts/start.sh @@ -111,21 +111,65 @@ holder() { lsof -nP -iTCP:"$1" -sTCP:LISTEN -Fcn 2>/dev/null | awk '/^c/{c=substr($0,2)} /^n/{print c" ("substr($0,2)")"; exit}' || true } +# Does whatever holds this port answer as OpenBot, rather than merely answer? +# +# `curl -f` proves something is listening and returned 2xx. That is not the same claim, and the gap +# between them is not academic: any single-page app serves its own index.html for every path it does +# not recognise, so an unrelated dashboard sitting on a default port answers 200 to +# `/api/capabilities` as readily as this server does. +# +# When that happened here the cost was not a wrong answer, it was a wrong answer three stages later. +# `require_free_or_ours` reported "already up", the server was therefore never started, `wait_for` +# printed a green "server ready", and the run died at stage 3 in `json.loads` on a mouthful of HTML — +# a JSON parse error standing in for "that port belongs to something else". +# +# So each surface is asked for something only it can produce. +identifies_as_openbot() { + local port="$1" name="$2" + case "$name" in + # A field of this server's own payload. A stray 200 does not carry it. + server) + curl -fsS --max-time 3 "http://localhost:$port/api/copilotkit/info" 2>/dev/null \ + | grep -q '"licenseStatus"' + ;; + # The app is static HTML with nothing to interrogate, so its title is the identity available. + app) + curl -fsS --max-time 3 "http://localhost:$port/" 2>/dev/null \ + | grep -qi '<title>[^<]*OpenBot' + ;; + # Compose services on dedicated loopback ports, answering a route named for this stack. + *) + curl -fsS --max-time 3 "http://localhost:$port/health" >/dev/null 2>&1 + ;; + esac +} + require_free_or_ours() { local port="$1" name="$2" who who="$(holder "$port")" [ -z "$who" ] && return 0 - if curl -fsS --max-time 3 "http://localhost:$port/health" >/dev/null 2>&1 \ - || curl -fsS --max-time 3 "http://localhost:$port/api/capabilities" >/dev/null 2>&1 \ - || curl -fsS --max-time 3 "http://localhost:$port/" >/dev/null 2>&1; then + if identifies_as_openbot "$port" "$name"; then info " $name: already up on $port ($who)" return 0 fi - red " $name: port $port is held by something else: $who" + red " $name: port $port is held by something that is not OpenBot: $who" red " Re-run with ${name^^}_PORT=<free port>, or stop that process yourself." exit 1 } +# As wait_for, but satisfied only by OpenBot answering, not by anything answering. +wait_for_openbot() { + local port="$1" name="$2" tries="${3:-40}" + for _ in $(seq 1 "$tries"); do + identifies_as_openbot "$port" "$name" && { green " $name ready"; return 0; } + sleep 1 + done + red " $name never answered as OpenBot on port $port" + red " Either it failed to start, or that port belongs to another process." + red " Log: $LOGS/${name}.log" + exit 1 +} + wait_for() { local url="$1" name="$2" tries="${3:-40}" for _ in $(seq 1 "$tries"); do @@ -214,7 +258,7 @@ if [ "$SECRETS_ROTATED" = "true" ]; then pkill -f "bun --env-file=../.env src/index.ts" >/dev/null 2>&1 || true sleep 1 fi -if ! curl -fsS --max-time 3 "http://localhost:$SERVER_PORT/api/capabilities" >/dev/null 2>&1; then +if ! identifies_as_openbot "$SERVER_PORT" server; then if [ "$ONE_COMPUTER_EACH" = "true" ]; then (cd server && PORT="$SERVER_PORT" \ COMPUTER_SUPERVISOR_URL="http://localhost:$SUPERVISOR_PORT" \ @@ -225,7 +269,7 @@ if ! curl -fsS --max-time 3 "http://localhost:$SERVER_PORT/api/capabilities" >/d (cd server && PORT="$SERVER_PORT" bun --env-file=../.env src/index.ts >"$LOGS/server.log" 2>&1 &) fi fi -wait_for "http://localhost:$SERVER_PORT/api/capabilities" "server" +wait_for_openbot "$SERVER_PORT" server info "3/4 Runtime health" INFO="$(curl -fsS --max-time 8 "http://localhost:$SERVER_PORT/api/copilotkit/info")" @@ -246,10 +290,10 @@ PY info "4/4 App" require_free_or_ours "$APP_PORT" app -if ! curl -fsS --max-time 3 "http://localhost:$APP_PORT/" >/dev/null 2>&1; then +if ! identifies_as_openbot "$APP_PORT" app; then (cd app && bun run dev --port "$APP_PORT" --strictPort >"$LOGS/app.log" 2>&1 &) fi -wait_for "http://localhost:$APP_PORT/" "app" +wait_for_openbot "$APP_PORT" app cat <<EOF 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/scripts/cull-idle-computers.ts b/server/scripts/cull-idle-computers.ts index 626e55ae..43183ac3 100644 --- a/server/scripts/cull-idle-computers.ts +++ b/server/scripts/cull-idle-computers.ts @@ -71,6 +71,20 @@ try { const purged = await queue.purge({ kind: CULL_KIND, olderThanMs: 24 * 60 * 60 * 1000, + /* + * A FINISHED SUSPENSION IS KEPT FOR THE IDLE WINDOW, NOT FOR A DAY. + * + * The key is the Bot id, so the row left behind by a suspension is what the next one collides + * with: a computer resumed, used, and left alone again was offered every sweep and swallowed + * every time, and stayed awake until the row aged out a day later. Nobody saw it, because a + * sweep that offers work and suspends nothing looks exactly like a fleet that is busy. + * + * The idle window is the right length because it is the same clock the offer runs on: a Bot + * cannot qualify as idle again until this long after its last action, by which point its row has + * gone. A day is still right for the other half, where the window is the backoff before a + * suspension that keeps failing is tried again. + */ + finishedOlderThanMs: config.computer.idleAfterMs, }); /* * And the screenshots, which had a reaper and nothing calling it. 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<void>; + /** Stamp the caller's own membership as read now. Throws ChannelNotFoundError for a non-member. */ + markRead(actor: AgentActor, channelId: string): Promise<void>; /** * 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/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/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/src/plugins/catalogue.ts b/server/src/plugins/catalogue.ts index 420148a5..f4fd2d3e 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"; @@ -263,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; } @@ -330,6 +348,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 +439,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 +476,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/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<void> { + /* + * 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/src/routing/classify.ts b/server/src/routing/classify.ts index 5ae457b0..0286fa7c 100644 --- a/server/src/routing/classify.ts +++ b/server/src/routing/classify.ts @@ -34,6 +34,35 @@ export type RoutingCandidate = { reaches?: readonly string[]; }; +/** + * Why the router did not decide, when it did not. + * + * `fallback` says a message did not reach a coworker by an inferred match. It does not say whether + * that was the router declining or the router failing, and those are different facts about a + * deployment: "no specialist was a confident match" is the feature working, and "the router was + * unreachable" is an endpoint that is down. Both landed on the same boolean and the same sentence. + * + * That mattered once already. #178 found the intent router appending `/v1` to a `OPENAI_BASE_URL` + * that already carried one, so every call 404'd on every deployment that set the variable — and its + * own changelog entry says untagged messages "silently stopped being routed and nothing said why". + * The URL is fixed. The blindness that let it go unnoticed is this field. + * + * Named rather than free text so a trail can be counted: "this deployment routed nothing by inference + * for a week" is a question `select ... where payload->>'undecided' = 'unreachable'` answers, and a + * sentence is not. + */ +export type RoutingUndecided = + /** The model call threw. An endpoint being down, a bad key, a gateway 404. */ + | "unreachable" + /** It answered, and the answer was not JSON this could read. */ + | "unparsed" + /** It named a coworker that is not on the roster it was given. */ + | "off-roster" + /** It answered honestly that it was not sure. The feature working, not failing. */ + | "unconfident" + /** Nothing to decide between, so no call was made. */ + | "one-candidate"; + export type RoutingDecision = { agentId: string; name: string; @@ -41,6 +70,14 @@ export type RoutingDecision = { reason: string; /** True when this is the default rather than an inferred match: an honest "we were not sure". */ fallback: boolean; + /** + * Why it was not decided, or null when it was. + * + * Survives the reach-based answer below. Landing on the one coworker that can reach the system a + * message names is a good outcome and says nothing about whether the router answered, so replacing + * this with that would hide exactly the failure it exists to count. + */ + undecided: RoutingUndecided | null; }; /** Below this the match is a guess, and a guess should defer to the default rather than surprise. */ @@ -138,7 +175,10 @@ export function createIntentRouter(deps: { defaultId: string, ): Promise<RoutingDecision> { const byId = new Map(candidates.map((c) => [c.id, c])); - const fallback = (reason: string): RoutingDecision => { + const fallback = ( + reason: string, + undecided: RoutingUndecided, + ): RoutingDecision => { /* * Before the default, ask whether the message named a system only one coworker can reach. * @@ -157,18 +197,33 @@ export function createIntentRouter(deps: { name: reachable.name, reason: `the only coworker that can reach ${reachable.system}`, fallback: true, + // Carried through. Reach answered where the message went; it did not answer whether the + // router did, and a router that has been down for a week must not read as this. + undecided, }; } const chosen = byId.get(defaultId) ?? candidates[0]; return chosen - ? { agentId: chosen.id, name: chosen.name, reason, fallback: true } + ? { + agentId: chosen.id, + name: chosen.name, + reason, + fallback: true, + undecided, + } : // No roster at all is a misconfiguration, not a routing outcome; surface the default id. - { agentId: defaultId, name: defaultId, reason, fallback: true }; + { + agentId: defaultId, + name: defaultId, + reason, + fallback: true, + undecided, + }; }; // Nothing to decide between: one coworker, or none but the default. if (candidates.length <= 1) { - return fallback("the only coworker available"); + return fallback("the only coworker available", "one-candidate"); } let raw: string; @@ -177,17 +232,34 @@ export function createIntentRouter(deps: { } catch { return fallback( "sent to your default while the router was unreachable", + "unreachable", ); } let parsed: { agentId?: unknown; reason?: unknown; confidence?: unknown }; + // The model is asked for bare JSON, but tolerate a fenced or padded answer. Named `jsonPart` + // rather than `match`, which is the roster lookup a few lines below. + const jsonPart = raw.match(/\{[\s\S]*\}/); + /* + * An answer with no object in it at all is unparsed, not off-roster. + * + * This used to fall through as `{}`, so a model replying in prose — "I think Risk Analyst is + * best" — reached the roster check, found no id, and was recorded as the router having named a + * coworker that does not exist. That points whoever reads it at their roster, when what is + * wrong is that the model is not answering in the format it was asked for. + */ + if (!jsonPart) { + return fallback( + "sent to your default; the router's answer did not parse", + "unparsed", + ); + } try { - // The model is asked for bare JSON, but tolerate a fenced or padded answer. - const match = raw.match(/\{[\s\S]*\}/); - parsed = match ? JSON.parse(match[0]) : {}; + parsed = JSON.parse(jsonPart[0]); } catch { return fallback( "sent to your default; the router's answer did not parse", + "unparsed", ); } @@ -197,6 +269,7 @@ export function createIntentRouter(deps: { // A returned id that is not on the roster is the dangerous case: never act on it. return fallback( "sent to your default; the router named no coworker on your roster", + "off-roster", ); } const confidence = @@ -204,6 +277,7 @@ export function createIntentRouter(deps: { if (confidence < MIN_CONFIDENCE) { return fallback( "sent to your default; no specialist was a confident match", + "unconfident", ); } @@ -211,7 +285,14 @@ export function createIntentRouter(deps: { typeof parsed.reason === "string" && parsed.reason.trim() ? parsed.reason.trim() : `matches ${match.name}`; - return { agentId: match.id, name: match.name, reason, fallback: false }; + // The router answered, on the roster, confidently. The only path where nothing was undecided. + return { + agentId: match.id, + name: match.name, + reason, + fallback: false, + undecided: null, + }; }, // Split out so the prompt-build + call is one seam the tests can leave alone. diff --git a/server/src/routing/routes.ts b/server/src/routing/routes.ts index 5d523c20..e8ae0720 100644 --- a/server/src/routing/routes.ts +++ b/server/src/routing/routes.ts @@ -4,7 +4,11 @@ import type { AuditStore } from "../audit"; import { recordAuditEvent } from "../audit"; import type { AppVariables } from "../auth/guards"; import type { AgentProfileStore } from "../agents/profile-store"; -import type { IntentRouter, RoutingCandidate } from "./classify"; +import type { + IntentRouter, + RoutingCandidate, + RoutingUndecided, +} from "./classify"; const DEV_ACTOR_EMAIL = "dev@openbot.local"; @@ -68,6 +72,14 @@ export function createRoutingRoutes( fallback: boolean, viaMention: boolean, candidates: readonly string[], + /* + * Why the router did not decide, when it did not. + * + * On the row rather than only in the sentence, because this is the field a deployment counts. A + * router that has been unreachable for a week produced rows that read like ordinary + * "no confident match" ones, which is how #178 went unnoticed for as long as it did. + */ + undecided: RoutingUndecided | null, ): Promise<void> { if (!auditStore) return; await recordAuditEvent(auditStore, { @@ -75,7 +87,7 @@ export function createRoutingRoutes( targetType: "agent", targetId: chosen, ...(actorUserId ? { actorUserId } : {}), - payload: { chosen, reason, fallback, viaMention, candidates }, + payload: { chosen, reason, fallback, viaMention, candidates, undecided }, }); } @@ -126,7 +138,16 @@ export function createRoutingRoutes( * @zopeVaibhav had this right in #134. */ const reason = "named by the person asking"; - await record(actorId(actor), chosen.id, reason, false, true, [chosen.id]); + // The person chose. Nothing was left to the router, so nothing about it was undecided. + await record( + actorId(actor), + chosen.id, + reason, + false, + true, + [chosen.id], + null, + ); return context.json({ agentId: chosen.id, name: chosen.name, @@ -164,6 +185,7 @@ export function createRoutingRoutes( decision.fallback, false, candidates.map((c) => c.id), + decision.undecided, ); return context.json({ @@ -171,6 +193,7 @@ export function createRoutingRoutes( name: decision.name, reason: decision.reason, fallback: decision.fallback, + undecided: decision.undecided, viaMention: false, }); }); diff --git a/server/src/work/queue.ts b/server/src/work/queue.ts index ce30b41b..30aa7ba5 100644 --- a/server/src/work/queue.ts +++ b/server/src/work/queue.ts @@ -87,10 +87,18 @@ export type WorkQueue = { * * Both kinds of done: finished, and given up on. An item at its attempt cap is not finished and was * reaped by nothing, so its key stayed occupied for ever and the work could never be offered again. + * + * TWO KINDS OF DONE, TWO WINDOWS. They are the same length only by coincidence. A finished row is + * kept so a late offer of the same key collides with it, which needs to outlast a sweep; a row that + * gave up is kept because the window is also the backoff before anything tries again, which wants + * to be long. Kept as one number, a queue whose work repeats has to choose which of those to be + * wrong about. `finishedOlderThanMs` defaults to `olderThanMs`, so a caller that has only one + * answer keeps the behaviour it had. */ purge: (input: { kind: string; olderThanMs: number; + finishedOlderThanMs?: number; maxAttempts?: number; }) => Promise<number>; }; @@ -250,15 +258,21 @@ export function createWorkQueue(database: Database): WorkQueue { return Boolean(released); }, - async purge({ kind, olderThanMs, maxAttempts = DEFAULT_MAX_ATTEMPTS }) { + async purge({ + kind, + olderThanMs, + finishedOlderThanMs = olderThanMs, + maxAttempts = DEFAULT_MAX_ATTEMPTS, + }) { const cutoff = fromNow(-olderThanMs); + const finishedCutoff = fromNow(-finishedOlderThanMs); const gone = await database .delete(workItems) .where( and( eq(workItems.kind, kind), or( - lt(workItems.finishedAt, cutoff), + lt(workItems.finishedAt, finishedCutoff), /* * AND THE ONES THAT GAVE UP, which is the half this forgot. * 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(); diff --git a/server/tests/computer-culler.integration.test.ts b/server/tests/computer-culler.integration.test.ts index 6f8555e0..22092d77 100644 --- a/server/tests/computer-culler.integration.test.ts +++ b/server/tests/computer-culler.integration.test.ts @@ -219,4 +219,57 @@ describe("suspending computers nobody is using", () => { // Each exactly once, which is the point of claiming rather than sweeping. expect(new Set(stopped).size).toBe(stopped.length); }); + + /* + * The second idle window, which is the one that pays for scale-to-zero. + * + * A computer is suspended once and then resumed, used, and left alone again. Every sweep after the + * first was offering work that the finished row silently swallowed, so the Bot stayed awake until + * that row aged out a day later. The queue is right to keep the row: it is what stops the same key + * running twice. It is the retention window for a finished suspension that has to be the idle + * window rather than a day, which is what the sweep below passes. + */ + test("a computer used again after it was suspended is suspended again", async () => { + const botId = botOf("recycled"); + const { provider, stopped } = providerWith([ + { botId, status: "running", url: "http://c" }, + ]); + const at = (iso: string) => () => new Date(iso); + const sweep = async (whenIso: string) => { + const options = { + database, + queue, + provider, + idleAfterMs, + owner: "replica-1", + now: at(whenIso), + }; + await offerIdleComputers(options); + const report = await suspendClaimedComputers(options); + /* + * What the CronJob does at the end of every sweep. Zero here rather than the idle window + * because these rows are finished seconds ago in real time and `purge` reads the database's + * clock, not this test's: the window being separate from the give-up one is the property + * under test, not its length. + */ + await queue.purge({ + kind: CULL_KIND, + olderThanMs: 24 * 60 * 60_000, + finishedOlderThanMs: 0, + }); + return report; + }; + + await database.insert(auditEvents).values(ran(botId, minutesAgo(60))); + expect((await sweep("2026-08-24T12:00:00Z")).suspended).toEqual([botId]); + + // Somebody comes back at 13:00, and it is quiet again by 14:00. + await database + .insert(auditEvents) + .values(ran(botId, new Date("2026-08-24T13:00:00Z"))); + const second = await sweep("2026-08-24T14:00:00Z"); + + expect(second.suspended).toEqual([botId]); + expect(stopped).toEqual([botId, botId]); + }); }); diff --git a/server/tests/cull-sweep-wiring.test.ts b/server/tests/cull-sweep-wiring.test.ts new file mode 100644 index 00000000..5c3d4164 --- /dev/null +++ b/server/tests/cull-sweep-wiring.test.ts @@ -0,0 +1,40 @@ +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +/** + * What the sweep passes, asserted against the script's own source. + * + * The script cannot be imported to be tested: it opens a database and a computer provider at the top + * level and runs a sweep as a side effect of loading. So the retention window it hands `purge` is the + * one line in this fix that nothing executes, and every test around it passes the window itself, + * which means they would all still be green with this line deleted. + * + * That is the failure this whole change is about, one level up: a value the caller never names + * reaches nothing, and the sweep goes on looking like it worked. Read as text, the way + * `tests/compose.test.ts` pins the variables Compose has to name, because the alternative is a fix + * whose only load-bearing line is the untested one. + * + * WHAT THIS CANNOT SEE. It proves the argument is written and where its value comes from, not that + * the sweep behaves. `computer-culler.integration.test.ts` owns the behaviour and runs the real + * queue against a real database; this is only the wire between the two. + */ +const script = readFileSync( + join(import.meta.dir, "..", "scripts", "cull-idle-computers.ts"), + "utf8", +); + +test("the cull sweep keeps a finished suspension for the configured idle window", () => { + expect(script).toContain("finishedOlderThanMs: config.computer.idleAfterMs"); +}); + +/** + * And that the other half still gets its day. + * + * The two windows exist because they are different questions. A sweep that passed the idle window for + * both would turn a suspension that keeps failing into one retried every few minutes, which is the + * regression this fix could most easily cause and the one nothing else would report. + */ +test("and gives a suspension that gave up the longer window before it is tried again", () => { + expect(script).toContain("olderThanMs: 24 * 60 * 60 * 1000"); +}); diff --git a/server/tests/plugin-catalogue.test.ts b/server/tests/plugin-catalogue.test.ts index e98a6213..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"; /** @@ -342,7 +344,155 @@ 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."); }); }); + +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<never>, + 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 () => { diff --git a/server/tests/routing-classify.test.ts b/server/tests/routing-classify.test.ts index 9407879b..f801f5f6 100644 --- a/server/tests/routing-classify.test.ts +++ b/server/tests/routing-classify.test.ts @@ -269,3 +269,130 @@ describe("falling back to somebody who can actually answer", () => { expect(decision.fallback).toBe(false); }); }); + +/** + * Why a message was not routed, as a thing a deployment can count. + * + * `fallback` says an inferred match did not happen. It does not say whether the router declined or + * the router failed, and only one of those is a deployment with something wrong with it. #178 was + * exactly that: the router 404'd on every deployment that set `OPENAI_BASE_URL`, and its changelog + * says untagged messages "silently stopped being routed and nothing said why". These pin the field + * that answers it. + */ +describe("saying why a message was not routed", () => { + test("a confident match leaves nothing undecided", async () => { + const decision = await withAnswer( + JSON.stringify({ + agentId: "risk-analyst", + reason: "fraud", + confidence: 0.9, + }), + ).route("is this transaction fraud", ROSTER, "general-assistant"); + + expect(decision.fallback).toBe(false); + expect(decision.undecided).toBeNull(); + }); + + test("an unreachable router is named as unreachable", async () => { + const decision = await throwing().route( + "anything", + ROSTER, + "general-assistant", + ); + expect(decision.undecided).toBe("unreachable"); + }); + + test("an answer that does not parse is named as unparsed", async () => { + const decision = await withAnswer('{ "agentId": }').route( + "anything", + ROSTER, + "general-assistant", + ); + expect(decision.undecided).toBe("unparsed"); + }); + + test("prose with no JSON in it is unparsed, not off-roster", async () => { + /* + * A model answering in sentences is a model not following the format. Recorded as off-roster it + * reads as a roster problem, and whoever investigates goes and looks at their roster. + */ + const decision = await withAnswer("I think Risk Analyst is best.").route( + "anything", + ROSTER, + "general-assistant", + ); + expect(decision.undecided).toBe("unparsed"); + }); + + test("an id that is not on the roster is named as off-roster", async () => { + const decision = await withAnswer( + JSON.stringify({ agentId: "somebody-else", confidence: 0.9 }), + ).route("anything", ROSTER, "general-assistant"); + expect(decision.undecided).toBe("off-roster"); + }); + + test("an honest low confidence is named as unconfident, not as a failure", async () => { + // The one cause that is the feature working. Counting it with the failures would make the + // number useless, which is the whole reason this is a named cause rather than a boolean. + const decision = await withAnswer( + JSON.stringify({ agentId: "risk-analyst", confidence: 0.2 }), + ).route("anything", ROSTER, "general-assistant"); + expect(decision.undecided).toBe("unconfident"); + }); + + test("a roster of one is named as one-candidate, and asks nothing", async () => { + const decision = await throwing().route( + "anything", + [ROSTER[0] as RoutingCandidate], + "general-assistant", + ); + expect(decision.undecided).toBe("one-candidate"); + }); + + test("the reach answer does not hide that the router never answered", async () => { + /* + * THE CASE THIS EXISTS FOR. Landing on the only coworker that can reach Google Drive is a good + * outcome, and it says nothing about whether the router was up. Before this the reach sentence + * replaced the failure entirely, so a deployment whose router had been down for a week produced + * rows that read exactly like reach-based routing working as intended. + */ + const reaching: RoutingCandidate[] = [ + { ...(ROSTER[0] as RoutingCandidate) }, + { + ...(ROSTER[1] as RoutingCandidate), + reaches: ["google-drive"], + }, + ]; + + const decision = await throwing().route( + "find the PRD in google drive", + reaching, + "general-assistant", + ); + + // Still routed by reach, and still a fallback — both unchanged. + expect(decision.agentId).toBe("knowledge"); + expect(decision.reason).toContain("google-drive"); + expect(decision.fallback).toBe(true); + // And the router failure survives it. + expect(decision.undecided).toBe("unreachable"); + }); + + test("reach after a low-confidence answer says unconfident, not unreachable", async () => { + // The two are told apart on the reach path as well, or the count is wrong wherever reach fires. + const reaching: RoutingCandidate[] = [ + { ...(ROSTER[0] as RoutingCandidate) }, + { + ...(ROSTER[1] as RoutingCandidate), + reaches: ["google-drive"], + }, + ]; + + const decision = await withAnswer( + JSON.stringify({ agentId: "knowledge", confidence: 0.1 }), + ).route("find the PRD in google drive", reaching, "general-assistant"); + + expect(decision.agentId).toBe("knowledge"); + expect(decision.undecided).toBe("unconfident"); + }); +}); diff --git a/server/tests/routing-routes.test.ts b/server/tests/routing-routes.test.ts index 974996b2..b974d136 100644 --- a/server/tests/routing-routes.test.ts +++ b/server/tests/routing-routes.test.ts @@ -4,7 +4,7 @@ import { Hono } from "hono"; import type { AgentProfileStore } from "../src/agents/profile-store"; import type { AuditStore } from "../src/audit"; import type { AppVariables } from "../src/auth/guards"; -import type { IntentRouter } from "../src/routing/classify"; +import type { IntentRouter, RoutingUndecided } from "../src/routing/classify"; import { createRoutingRoutes } from "../src/routing/routes"; /** @@ -47,7 +47,7 @@ type Recorded = { payload: Record<string, unknown>; }; -function app(options: { routed?: string } = {}) { +function app(options: { routed?: string; undecided?: RoutingUndecided } = {}) { const written: Recorded[] = []; /** Every call the router was asked to make, so "never asked" is an assertion and not a hope. */ const asked: string[] = []; @@ -72,7 +72,8 @@ function app(options: { routed?: string } = {}) { agentId: chosen, name: ROSTER.find((a) => a.id === chosen)?.name ?? chosen, reason: "matches what it is for", - fallback: false, + fallback: options.undecided !== undefined, + undecided: options.undecided ?? null, }; }, } as unknown as IntentRouter; @@ -185,3 +186,47 @@ describe("recording which coworker a message went to", () => { expect(asked).toEqual(["hello"]); }); }); + +/** + * Why it was not decided, on the row rather than only in the sentence. + * + * The reason is prose for a person. This is the field a deployment counts, and counting is the point: + * a router that has been unreachable for a week is invisible until somebody can ask how often. + */ +describe("recording why a message was not routed", () => { + test("an unreachable router is on the row, not only in the sentence", async () => { + const { server, written } = app({ undecided: "unreachable" }); + + await post(server, { text: "what is our PTO policy" }); + + expect(written[0]?.payload).toMatchObject({ + chosen: "knowledge", + fallback: true, + undecided: "unreachable", + }); + }); + + test("a decided routing records no cause", async () => { + const { server, written } = app(); + + await post(server, { text: "what is our PTO policy" }); + + expect(written[0]?.payload).toMatchObject({ + fallback: false, + undecided: null, + }); + }); + + test("a coworker the person named records no cause either", async () => { + // Nothing was left to the router, so there is nothing about it to have failed. Recording a cause + // here would count a person's own choice as a routing failure. + const { server, written } = app(); + + await post(server, { text: "hello", agentId: "risk-analyst" }); + + expect(written[0]?.payload).toMatchObject({ + viaMention: true, + undecided: null, + }); + }); +}); 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, ); diff --git a/server/tests/work-queue.integration.test.ts b/server/tests/work-queue.integration.test.ts index a7397e6a..14b7d847 100644 --- a/server/tests/work-queue.integration.test.ts +++ b/server/tests/work-queue.integration.test.ts @@ -323,4 +323,53 @@ describe("claiming durable work", () => { expect(row?.attempts).toBe(3); expect(row?.why).toBe("the cluster said no"); }); + + /* + * The two kinds of done, on their own clocks. + * + * They were one number, which a queue whose work repeats cannot afford: a finished row only has to + * outlast a sweep, and a row that gave up is kept because that window is also the backoff before + * anything tries the work again. Sharing it meant the culler either wedged its own key for a day + * or retried a broken suspension every few minutes. + */ + test("a finished row and one that gave up are swept on separate windows", async () => { + await queue.offer({ kind, key: "done" }); + await queue.claim({ kind, owner: "replica-1", leaseMs: 30_000 }); + await queue.finish({ kind, key: "done", owner: "replica-1" }); + + await queue.offer({ kind, key: "gave-up" }); + for (let attempt = 0; attempt < 3; attempt += 1) { + await queue.claim({ + kind, + owner: "replica-1", + leaseMs: 30_000, + maxAttempts: 3, + }); + await queue.release({ + kind, + key: "gave-up", + owner: "replica-1", + delayMs: 0, + reason: "the cluster said no", + }); + } + + // The finished one goes on its own short window; the one that gave up keeps its long one. + expect( + await queue.purge({ + kind, + olderThanMs: 60_000, + finishedOlderThanMs: 0, + maxAttempts: 3, + }), + ).toBe(1); + const [left] = await database + .select({ key: workItems.key }) + .from(workItems) + .where(eq(workItems.kind, kind)); + expect(left?.key).toBe("gave-up"); + + // And omitting it leaves both halves on the one window, which is what every other caller does. + expect(await queue.purge({ kind, olderThanMs: 0, maxAttempts: 3 })).toBe(1); + }); }); 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_<BOT>` 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"); +});