computer: reject an empty workspace artifacts session id

The artifacts wrapper rejects an empty session id so a blank value
cannot quietly turn one tenant's client into a client over every
repository in the namespace. The Workspace path read an empty id as
an absent one instead, which walked around that check: a caller
deriving the session id from an unset variable or a missing request
field got the namespace-wide client rather than an error.

An empty id now reaches the wrapper and fails there, as it did
before the session id became optional. Omitting the id, or passing
null, remains the way to ask for a client over the whole namespace.

Author: aron-cf

docs/15_artifacts_interface.md

@@ -313,6 +313,12 @@ explicit `artifacts: { binding, sessionId: null }`, the opt-out for
 a workspace that has a session id but wants artifacts across every
 session — `workspace.artifacts` spans the namespace.
 
+An empty session id fails here the same way it fails at
+`createArtifact`: the constructor throws `InvalidSessionIdError`.
+A workspace whose session id comes from an unset variable or a
+missing request field stops rather than handing that tenant a
+client over every other tenant's repositories.
+
 When `artifacts` is omitted from `Workspace`, the command still
 exists, but operations fail with a clear "Workspace Artifacts binding
 is not configured" error.

packages/computer/src/workspace.test.ts

@@ -2,7 +2,7 @@ import { SQLiteTestStorage } from "@cloudflare/dofs/testing";
 import { describe, expect, it, vi } from "vitest";
 
 import { FakeArtifactsBinding } from "../tests/utilities/fake-artifacts-binding.js";
-import { createArtifact } from "./artifacts/index.js";
+import { createArtifact, InvalidSessionIdError } from "./artifacts/index.js";
 import type { BackendHandle, WorkspaceBackend } from "./backend.js";
 import { createGitClient } from "./git/index.js";
 import type { WorkspaceModuleBackend } from "./runtime/types.js";
@@ -817,6 +817,27 @@ describe("Workspace backend selection", () => {
       expect(await ws.artifacts.list()).toHaveLength(1);
     });
 
+    it("rejects an empty workspace session id instead of widening the client", () => {
+      // A session id derived from an unset variable or a missing
+      // request field must not turn into namespace-wide access.
+      const binding = new FakeArtifactsBinding();
+      expect(
+        () => new Workspace({ storage: makeStorage(), sessionId: "", artifacts: { binding } }),
+      ).toThrow(InvalidSessionIdError);
+    });
+
+    it("rejects an empty artifacts session id", () => {
+      const binding = new FakeArtifactsBinding();
+      expect(
+        () =>
+          new Workspace({
+            storage: makeStorage(),
+            sessionId: "sess1",
+            artifacts: { binding, sessionId: "" },
+          }),
+      ).toThrow(InvalidSessionIdError);
+    });
+
     it("reports no session id when no binding is configured", async () => {
       const ws = new Workspace({ storage: makeStorage() });
       expect(ws.artifacts.sessionId).toBeUndefined();

packages/computer/src/workspace.ts

@@ -183,6 +183,11 @@ export interface WorkspaceOptions {
   // and reaches every repository, including those other sessions
   // own. Scope it unless the caller is meant to administer the
   // namespace.
+  //
+  // An empty session id is a construction error rather than a way
+  // to ask for the namespace-wide client, so an id that came out
+  // blank by accident cannot widen one tenant's client to all of
+  // them.
   artifacts?: {
     binding: Artifacts;
     sessionId?: string | null;
@@ -1335,16 +1340,17 @@ function positiveRetryOption(value: number | undefined, fallback: number, name:
  * artifacts-specific one when the caller set it, otherwise the
  * workspace's own, and undefined for a namespace-wide client.
  *
- * `WorkspaceOptions.sessionId` documents the empty string as its
- * unset value, so an empty id here means the workspace has no
- * session to scope by rather than a session named "". The wrapper
- * itself is stricter: it rejects an empty id outright, since a
- * caller reaching it directly has no such convention.
+ * An empty id is passed through rather than read as an absent one.
+ * A caller that derives the session id from an unset variable or a
+ * missing request field gets the wrapper's `InvalidSessionIdError`
+ * at construction instead of a client that reaches every
+ * repository in the namespace. Ask for that client by omitting the
+ * id or passing `null`.
  */
 function artifactsSessionId(options: WorkspaceOptions): string | null | undefined {
   const configured = options.artifacts?.sessionId;
   if (configured !== undefined) return configured;
-  return options.sessionId === "" ? undefined : options.sessionId;
+  return options.sessionId;
 }
 
 function createDisabledArtifactsClient(): ArtifactClient {