Review fixes for Cloudflare sandboxes

AGENTS.md

@@ -13,7 +13,7 @@ It owns:
 - Template records, async template builds, and their provider images
 - Inline provisioning in `POST /hosts`
 - Provider VM creation and deletion (exe.dev, AWS, Hetzner, Exoscale,
-  local Docker, Docker Sandboxes)
+  Cloudflare, local Docker, Docker Sandboxes)
 - Tailscale auth key creation, device discovery, and cleanup
 - SSH host key scanning and `known_hosts` material
 - An SSH gateway for hosts of gateway providers (`python -m gateway.server`)

deploy/cloudflare/src/index.ts

@@ -5,16 +5,24 @@ interface Env {
   SANDBOXES: DurableObjectNamespace<Sandbox>;
 }
 
+const INSTANCES = ["lite", "standard-1", "standard-2", "standard-3", "standard-4"] as const;
+
 interface SandboxRequest {
   image: string;
-  instance: "lite" | "standard-1" | "standard-2" | "standard-3" | "standard-4";
+  instance: (typeof INSTANCES)[number];
   script: string;
   label: string;
 }
 
 const INACTIVITY_MS = 5 * 60 * 1000;
 const ALARM_MS = 60 * 1000;
-const INSTANCES = new Set(["lite", "standard-1", "standard-2", "standard-3", "standard-4"]);
+const encoder = new TextEncoder();
+
+function isAuthorized(request: Request, token: string): boolean {
+  const supplied = encoder.encode(request.headers.get("Authorization") ?? "");
+  const expected = encoder.encode(`Bearer ${token}`);
+  return supplied.byteLength === expected.byteLength && crypto.subtle.timingSafeEqual(supplied, expected);
+}
 
 export class Sandbox extends DurableObject<Env> {
   constructor(ctx: DurableObjectState, env: Env) {
@@ -32,10 +40,6 @@ export class Sandbox extends DurableObject<Env> {
       await this.ctx.storage.deleteAlarm();
       return Response.json({ status: "deleted" });
     }
-    if (request.method === "GET") {
-      const status = await this.ctx.storage.get<string>("status");
-      return Response.json({ status: container.running ? status : "stopped" });
-    }
     if (request.method !== "PUT") {
       return new Response(null, { status: 405 });
     }
@@ -48,7 +52,7 @@ export class Sandbox extends DurableObject<Env> {
     }
     if (
       !body || typeof body.image !== "string" || !Object.hasOwn(container.images, body.image) ||
-      !INSTANCES.has(body.instance) || typeof body.script !== "string" ||
+      !INSTANCES.includes(body.instance) || typeof body.script !== "string" ||
       body.script.length > 65536 || typeof body.label !== "string" ||
       body.label.length > 64
     ) {
@@ -109,20 +113,16 @@ export class Sandbox extends DurableObject<Env> {
 
   async alarm(): Promise<void> {
     const status = await this.ctx.storage.get<string>("status");
-    if (status === "active" || status === "creating") {
-      if (this.ctx.container!.running) {
-        await this.ctx.container!.setInactivityTimeout(INACTIVITY_MS);
-        await this.ctx.storage.setAlarm(Date.now() + ALARM_MS);
-      } else {
-        await this.ctx.storage.put("status", "stopped");
-      }
+    if (this.ctx.container!.running && (status === "active" || status === "creating")) {
+      await this.ctx.container!.setInactivityTimeout(INACTIVITY_MS);
+      await this.ctx.storage.setAlarm(Date.now() + ALARM_MS);
     }
   }
 }
 
 export default {
   async fetch(request: Request, env: Env): Promise<Response> {
-    if (!env.DRUKBOX_TOKEN || request.headers.get("Authorization") !== `Bearer ${env.DRUKBOX_TOKEN}`) {
+    if (!env.DRUKBOX_TOKEN || !isAuthorized(request, env.DRUKBOX_TOKEN)) {
       return new Response(null, { status: 401 });
     }
     const url = new URL(request.url);

deploy/cloudflare/tests/sandbox.test.ts

@@ -121,6 +121,13 @@ test("delete is repeatable and a later alarm cannot restart the VM", async () =>
 test("a stopped VM is not silently recreated", async () => {
   values.set("status", "active");
   await sandbox().alarm();
-  expect(values.get("status")).toBe("stopped");
+  expect(storage.setAlarm).not.toHaveBeenCalled();
   expect(container.start).not.toHaveBeenCalled();
 });
+
+test("a wrong token of the same length is rejected", async () => {
+  const response = await worker.fetch(new Request("https://worker.test/health", {
+    headers: { Authorization: "Bearer test-tokem" },
+  }), env as never);
+  expect(response.status).toBe(401);
+});

deploy/cloudflare/tests/setup.ts

@@ -0,0 +1,6 @@
+import { timingSafeEqual } from "node:crypto";
+
+Object.assign(crypto.subtle, {
+  timingSafeEqual: (a: ArrayBufferView, b: ArrayBufferView) =>
+    timingSafeEqual(new Uint8Array(a.buffer, a.byteOffset, a.byteLength), new Uint8Array(b.buffer, b.byteOffset, b.byteLength)),
+});

deploy/cloudflare/vitest.config.ts

@@ -2,5 +2,5 @@ import { defineConfig } from "vitest/config";
 
 export default defineConfig({
   resolve: { alias: { "cloudflare:workers": new URL("./tests/workers.ts", import.meta.url).pathname } },
-  test: { include: ["tests/**/*.test.ts"] },
+  test: { include: ["tests/**/*.test.ts"], setupFiles: ["tests/setup.ts"] },
 });

docs/cloudflare.md

@@ -74,7 +74,7 @@ It does not test capacity or tailnet SSH.
 ## Verify
 
 ```bash
-uv run pytest src/providers/cloudflare/tests src/hosts/tests/test_userspace_bootstrap.py
+uv run pytest src/providers/cloudflare/tests src/hosts/tests/test_bootstrap.py
 cd deploy/cloudflare
 npm ci
 npm run check

src/hosts/scripts/sandbox_bootstrap.sh

@@ -41,7 +41,7 @@ if [[ -d /run/systemd/system ]]; then
 else
   run_privileged install -d -m 755 /var/run/tailscale
   run_privileged sh -c 'nohup tailscaled --tun=userspace-networking --state=mem: </dev/null >/var/log/tailscaled.log 2>&1 &'
-  for attempt in {1..60}; do
+  for _ in {1..60}; do
     if [[ -S /var/run/tailscale/tailscaled.sock ]]; then
       break
     fi

src/hosts/tests/test_bootstrap.py

@@ -1,3 +1,12 @@
+import asyncio
+import os
+import sys
+from collections.abc import Iterator
+from pathlib import Path
+from tempfile import TemporaryDirectory
+
+import pytest
+
 from hosts.service import _SANDBOX_BOOTSTRAP_SCRIPT as SCRIPT
 
 
@@ -36,3 +45,65 @@ def test_bootstrap_script_uses_privileged_service_operations() -> None:
     assert "sudo -n" in SCRIPT
     assert "run_privileged install -d" in SCRIPT
     assert "run_privileged systemctl enable --now tailscaled.service" in SCRIPT
+
+
+@pytest.fixture
+def short_path() -> Iterator[Path]:
+    # A Unix socket path is capped near 108 bytes; pytest's short_path can exceed it.
+    with TemporaryDirectory(prefix="dbx-", dir="/tmp") as directory:
+        yield Path(directory)
+
+
+@pytest.mark.parametrize("systemd", [False, True])
+async def test_bootstrap_starts_tailscale_and_enables_ssh(short_path: Path, systemd: bool):
+    commands = short_path / "bin"
+    commands.mkdir()
+    log = short_path / "commands.log"
+    socket_path = short_path / "var/run/tailscale/tailscaled.sock"
+    scripts = {
+        "sudo": '#!/bin/sh\nshift\nexec "$@"\n',
+        "systemctl": f'#!/bin/sh\necho "systemctl $*" >> {log}\n',
+        "tailscale": f'#!/bin/sh\necho "tailscale $*" >> {log}\n',
+        "jq": "#!/bin/sh\ncat >/dev/null\nexit 1\n",
+        "tailscaled": (
+            f"#!{sys.executable}\nimport socket, sys\n"
+            f"with open({str(log)!r}, 'a') as log:\n"
+            "    log.write('tailscaled ' + ' '.join(sys.argv[1:]) + '\\n')\n"
+            f"connection = socket.socket(socket.AF_UNIX)\nconnection.bind({str(socket_path)!r})\n"
+        ),
+    }
+    for name, contents in scripts.items():
+        path = commands / name
+        path.write_text(contents)
+        path.chmod(0o755)
+    (short_path / "var/log").mkdir(parents=True)
+    if systemd:
+        (short_path / "run/systemd/system").mkdir(parents=True)
+    script = SCRIPT.replace("/var/", f"{short_path}/var/").replace(
+        "[[ -d /run/systemd/system ]]", f"[[ -d {short_path}/run/systemd/system ]]"
+    )
+    process = await asyncio.create_subprocess_exec(
+        "bash",
+        "-c",
+        script,
+        env={
+            **os.environ,
+            "PATH": f"{commands}:{os.environ['PATH']}",
+            "TAILSCALE_AUTHKEY": "test-key",
+            "TAILSCALE_HOSTNAME": "sb-test",
+        },
+        stdout=asyncio.subprocess.PIPE,
+        stderr=asyncio.subprocess.PIPE,
+    )
+    stdout, stderr = await asyncio.wait_for(process.communicate(), timeout=10)
+    assert process.returncode == 0, (stdout, stderr)
+    operations = log.read_text()
+    assert "--ssh" in operations
+    assert "--hostname=sb-test" in operations
+    assert (short_path / "var/lib/sandbox/bootstrap.done").exists()
+    if systemd:
+        assert "systemctl enable --now tailscaled.service" in operations
+        assert "--tun=userspace-networking" not in operations
+    else:
+        assert "--tun=userspace-networking --state=mem:" in operations
+        assert "systemctl" not in operations

src/providers/cloudflare/api.py

@@ -23,7 +23,7 @@ async def create_sandbox(
         await self.request(
             "PUT",
             f"sandboxes/{name}",
-            status="active",
+            expected="active",
             json={
                 "image": image,
                 "instance": instance,
@@ -33,17 +33,17 @@ async def create_sandbox(
         )
 
     async def delete_sandbox(self, name: str) -> None:
-        await self.request("DELETE", f"sandboxes/{name}", status="deleted")
+        await self.request("DELETE", f"sandboxes/{name}", expected="deleted")
 
     async def diagnose(self) -> str:
-        await self.request("GET", "health", status="ok")
+        await self.request("GET", "health", expected="ok")
         return "Cloudflare Worker authentication succeeded"
 
     async def aclose(self) -> None:
         await self.client.aclose()
 
     async def request(
-        self, method: str, path: str, *, status: str, json: dict[str, str] | None = None
+        self, method: str, path: str, *, expected: str, json: dict[str, str] | None = None
     ) -> None:
         try:
             response = await self.client.request(method, path, json=json)
@@ -58,7 +58,7 @@ async def request(
                 f"Cloudflare Worker rejected request: HTTP {response.status_code}"
             )
         try:
-            if response.status_code not in {200, 201} or response.json()["status"] != status:
+            if response.status_code not in {200, 201} or response.json()["status"] != expected:
                 raise ProviderTransportError("Unexpected Cloudflare Worker response")
         except (ValueError, KeyError, TypeError) as exc:
             raise ProviderTransportError("Invalid Cloudflare Worker response") from exc

src/providers/cloudflare/provider.py

@@ -52,16 +52,13 @@ async def create_vm(
     ) -> VMCreateResult:
         if not setup_script:
             raise ProviderCommandError("Cloudflare requires TAILSCALE_ENABLED=true")
-        size = instance_type or self.settings.instance_type
-        if size not in {"lite", "standard-1", "standard-2", "standard-3", "standard-4"}:
-            raise ProviderCommandError("Unsupported Cloudflare instance_type")
-        try:
-            script = environment.get_cloud_init(setup_script, env)
-        except ValueError as exc:
-            raise ProviderCommandError(str(exc)) from exc
         try:
             await self.api.create_sandbox(
-                name, image=image, instance=size, script=script, label=self.service_label
+                name,
+                image=image,
+                instance=instance_type or self.settings.instance_type,
+                script=environment.get_cloud_init(setup_script, env),
+                label=self.service_label,
             )
         except ProviderAuthError as exc:
             raise ProviderCommandError("Cloudflare Worker authentication failed") from exc

src/providers/cloudflare/tests/test_provider.py

@@ -54,12 +54,13 @@ async def test_requires_tailscale_before_any_request(provider: CloudflareProvide
 
 
 @respx.mock
-async def test_rejects_unknown_size(provider: CloudflareProvider):
-    with pytest.raises(ProviderCommandError, match="instance_type"):
+async def test_unknown_size_is_rejected_by_the_worker(provider: CloudflareProvider):
+    respx.put("https://worker.test/sandboxes/sb-test").respond(400)
+    with pytest.raises(ProviderCommandError, match="rejected"):
         await provider.create_vm(
             name="sb-test", image="base", setup_script="true", instance_type="huge"
         )
-    assert not respx.calls
+    assert len(respx.calls) == 1
 
 
 @respx.mock