Refuse the shell and a workspace write while a person holds the wheel

.env.example

@@ -230,8 +230,13 @@ COMPUTER_TOKEN=
 #
 # This is attribution, not anonymity, and it is not a boundary by itself: it gives a security team a
 # per-Bot address for network rules alongside AGENT_COMPUTER_POLICY.
-# EGRESS_PROXY_DEFAULT=http://user:password@proxy.internal:8080
-# EGRESS_PROXY_SALES_BOT=http://sales.proxy.internal:8080
+#
+# These go in `egress.env` beside this file, NOT here. The names are per-Bot, so Compose cannot
+# list them the way it lists every variable below, and it hands a container only what it is told to.
+# In `.env` they reach no process and the browser goes out directly with nothing saying so.
+#
+#   EGRESS_PROXY_DEFAULT=http://user:password@proxy.internal:8080
+#   EGRESS_PROXY_SALES_BOT=http://sales.proxy.internal:8080
 
 
 # The managed coworker AG-UI endpoint. Optional: use an HTTP(S) URL, and set MANAGED_AGENT_TOKEN

.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.

.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

CHANGELOG.md

@@ -8,6 +8,49 @@ 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 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
@@ -163,6 +206,67 @@ 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_<BOT>` 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_<BOT>` is derived from a Bot's id and there
+is no fixed set of names to list; a file of its own rather than `.env` because that one holds the
+deployment's secrets and the container running a browser and a Bot's shell is deliberately not
+given them. It is optional, so a deployment with no proxy is unchanged, and gitignored, because a
+proxy URL can carry a password.
+
+**Move these two out of `.env` and into `egress.env`.** In `.env` they reach no process.
+
 ### Knowledge searches instead of guessing
 
 A package can say which of its skills each coworker gets, and the fintech example gives Knowledge the
@@ -214,6 +318,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
 

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);
+}

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);
       }
     }