Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions companion/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ upstream hardened its loopback gate.

| | |
|---|---|
| **Pairing** | A six-digit code shown on the computer, valid two minutes, five attempts. Redeeming it returns a device token stored only as a SHA-256 digest. |
| **Authorisation** | Every request needs that token. A rebinding page cannot obtain one. |
| **Pairing** | A high-entropy QR credential plus a six-digit manual fallback, valid two minutes and single-use. Redeeming either returns a device token stored only as a SHA-256 digest. |
| **Authorisation** | Every request needs that token. Full cloud-desktop access is a separate per-device capability, off by default. A rebinding page cannot obtain either. |
| **The allowlist** | Default deny, per method and path (`src/routes.ts`) — the list is every request the app makes, and nothing else. General bot/room PATCH routes stay closed; read state and approval grants use narrow verbs. A route that appears in the harness later is closed to devices until someone adds it here on purpose. |
| **Scrubbing** | `resumeCursors` — the harness's own provider session ids — never reach a device, whether or not the harness still sends them. |
| **Discovery** | Bonjour, so a phone finds the computer by name instead of by typed address. |
Expand Down
21 changes: 20 additions & 1 deletion companion/src/control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,17 @@ export function createControlServer(options: ControlOptions): Server {
options.devices.closePairing();
return json(res, 200, companionState(options));
}
const cloudDesktop = path.match(/^\/devices\/([\w-]+)\/cloud-desktop$/);
if (cloudDesktop && (method === "POST" || method === "DELETE")) {
try {
if (!options.devices.setCloudDesktopAccess(cloudDesktop[1], method === "POST")) {
return json(res, 404, { error: "no such device" });
}
} catch {
return json(res, 500, { error: "could not save cloud desktop access" });
}
return json(res, 200, companionState(options));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const revoke = path.match(/^\/devices\/([\w-]+)$/);
if (revoke && method === "DELETE") {
if (!options.devices.revoke(revoke[1])) return json(res, 404, { error: "no such device" });
Expand Down Expand Up @@ -279,7 +290,9 @@ function render(s) {
(s.devices.length
? "<ul>" + s.devices.map((d) =>
"<li><div class='grow'><div class=name>" + esc(d.name) + "</div>" +
"<div class=dim>Last seen " + ago(d.lastSeenAt) + "</div></div>" +
"<div class=dim>Last seen " + ago(d.lastSeenAt) + "</div>" +
"<button data-cloud='" + esc(d.id) + "' data-allowed='" + (d.cloudDesktopAccess ? "1" : "0") + "'>" +
(d.cloudDesktopAccess ? "Cloud desktop on" : "Allow cloud desktop") + "</button></div>" +
"<button data-revoke='" + esc(d.id) + "'>Remove</button></li>").join("") + "</ul>"
: "<p class=dim>No phones are paired yet.</p>");

Expand All @@ -288,6 +301,12 @@ function render(s) {
for (const b of document.querySelectorAll("[data-revoke]")) {
b.addEventListener("click", async () => render(await api("/devices/" + b.dataset.revoke, "DELETE")));
}
for (const b of document.querySelectorAll("[data-cloud]")) {
b.addEventListener("click", async () => render(await api(
"/devices/" + b.dataset.cloud + "/cloud-desktop",
b.dataset.allowed === "1" ? "DELETE" : "POST"
)));
}
if (s.pairing) {
const tick = () => {
const left = Math.max(0, Math.round((s.pairing.expiresAt - Date.now()) / 1000));
Expand Down
22 changes: 22 additions & 0 deletions companion/src/devices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ export interface DeviceRecord {
tokenHash: string;
createdAt: number;
lastSeenAt: number;
/** Full interactive access to a bot's cloud desktop. Deliberately off on
* every new and migrated device until the computer owner enables it. */
cloudDesktopAccess: boolean;
}

/** What the UI is allowed to see: a device without its secret. */
Expand Down Expand Up @@ -100,6 +103,7 @@ function normalizeDevice(record: Partial<DeviceRecord> & { id: string; tokenHash
name: cleanDeviceName(record.name),
createdAt,
lastSeenAt: timestamp(record.lastSeenAt, createdAt),
cloudDesktopAccess: record.cloudDesktopAccess === true,
};
}

Expand Down Expand Up @@ -209,6 +213,7 @@ export class DeviceRegistry {
tokenHash: sha256(token),
createdAt: Date.now(),
lastSeenAt: Date.now(),
cloudDesktopAccess: false,
};
this.devices.push(device);
// Unlike the lastSeenAt write below, this one must not be swallowed. A
Expand Down Expand Up @@ -260,6 +265,23 @@ export class DeviceRegistry {
this.persist();
return true;
}

/** Grant or remove the one capability that crosses from companion actions
* into full desktop control. This is per device so a watch-only phone does
* not inherit a different phone's permission. */
setCloudDesktopAccess(id: string, allowed: boolean): boolean {
const device = this.devices.find((candidate) => candidate.id === id);
if (!device) return false;
const previous = device.cloudDesktopAccess;
device.cloudDesktopAccess = allowed;
try {
this.persist();
} catch (error) {
device.cloudDesktopAccess = previous;
throw error;
}
return true;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion companion/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ const companion = createServer(
harnessPort: HARNESS_PORT,
// `authenticate` also stamps lastSeenAt, which is what makes the control
// page able to say when a phone was last heard from.
authenticate: (token) => Boolean(devices.authenticate(token)),
authenticate: (token) => devices.authenticate(token),
redeem: (code, deviceName) => devices.redeem(code, deviceName),
serverName: machineName,
}),
Expand Down
17 changes: 14 additions & 3 deletions companion/src/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
import { request as httpRequest, type IncomingMessage, type ServerResponse } from "node:http";

import { bearerToken } from "./devices.ts";
import { denyReason } from "./routes.ts";
import { denyReason, isCloudDesktopJoin } from "./routes.ts";
import { createSseScrubber, isJson, scrub } from "./wire.ts";

/** What the forwarding handler needs from the process around it. */
export interface ProxyOptions {
/** Where the harness is listening on loopback. */
harnessPort: number;
/** Does this bearer token belong to a paired device? */
authenticate: (token: string | undefined) => boolean;
authenticate: (token: string | undefined) => { cloudDesktopAccess: boolean } | null;
/** Redeem a pairing code. Handled here and never forwarded: the harness
* has no such route and no idea devices exist — pairing is the sidecar's
* own concern, and the one thing a device does before it has a token. */
Expand Down Expand Up @@ -131,17 +131,28 @@ export function createProxyHandler(options: ProxyOptions) {
return sendJson(res, 403, { error: "forbidden: cross-origin request" });
}

const token = bearerToken(req.headers.authorization);
const device = options.authenticate(token);
const denial = denyReason({
path,
method,
// `bearerToken` is the registry's own parser, imported rather than
// reimplemented: this file used to have a second one, and two parsers
// that disagree about what a credential looks like means the header a
// phone sends authenticates on one code path and not the other.
authenticated: options.authenticate(bearerToken(req.headers.authorization)),
authenticated: Boolean(device),
});
if (denial) return sendJson(res, denial.status, { error: denial.error });

// Pairing a phone grants the ordinary companion surface, not a browser
// session with every credential that may exist inside the cloud desktop.
// The computer owner enables this capability per device, off by default.
if (isCloudDesktopJoin(method, path) && !device?.cloudDesktopAccess) {
return sendJson(res, 403, {
error: "cloud desktop access is off for this phone — enable it in OpenMausBot → Settings → Companion",
});
}

// Pairing terminates here. Forwarding it would hand the harness a route
// it does not have, and the 404 would read to a phone as "wrong address".
if (method === "POST" && path === "/api/pair") {
Expand Down
15 changes: 15 additions & 0 deletions companion/src/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ export interface RouteRequest {
authenticated: boolean;
}

/** The one companion route that crosses into full interactive desktop
* control. Both the allowlist and capability gate consume this classifier so
* their security decisions cannot drift apart. */
export const CLOUD_DESKTOP_JOIN_ROUTE = {
method: "POST",
path: /^\/api\/bots\/[\w-]+\/computer\/join$/,
} as const;

export function isCloudDesktopJoin(method: string, path: string): boolean {
return method === CLOUD_DESKTOP_JOIN_ROUTE.method && CLOUD_DESKTOP_JOIN_ROUTE.path.test(path);
}

/** Every request the iOS app makes, and nothing else.
*
* Ids are `[\w-]+`, matching the harness's own route patterns. The paths
Expand Down Expand Up @@ -61,6 +73,9 @@ const ALLOWED: ReadonlyArray<{ method: string; path: RegExp }> = [
{ method: "POST", path: /^\/api\/bots\/[\w-]+\/tasks\/[\w-]+$/ },
{ method: "PATCH", path: /^\/api\/bots\/[\w-]+\/tasks\/[\w-]+$/ },
{ method: "DELETE", path: /^\/api\/bots\/[\w-]+\/tasks\/[\w-]+$/ },
// Full cloud desktop access. The route is narrow and the proxy applies a
// second, per-device capability check before it reaches the harness.
CLOUD_DESKTOP_JOIN_ROUTE,

// rooms
{ method: "POST", path: /^\/api\/groups\/[\w-]+\/messages$/ },
Expand Down
37 changes: 36 additions & 1 deletion companion/test/control.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { DeviceRegistry } from "../src/devices.ts";

let control: Server;
let port = 0;
let devices: DeviceRegistry;

const ask = async (
method: string,
Expand All @@ -28,8 +29,9 @@ const ask = async (
};

beforeAll(async () => {
devices = new DeviceRegistry();
control = createControlServer({
devices: new DeviceRegistry(),
devices,
companionPort: 8810,
discovery: () => ({ advertising: false, name: "Test computer" }),
});
Expand All @@ -43,6 +45,39 @@ afterAll(async () => {
});

describe("origins the control server will change state for", () => {
it("controls cloud desktop access per paired device", async () => {
const { code } = devices.openPairing();
const paired = devices.redeem(code, "iPhone");
if ("error" in paired) throw new Error(paired.error);

expect(paired.device.cloudDesktopAccess).toBe(false);
expect((await ask("POST", `/devices/${paired.device.id}/cloud-desktop`)).status).toBe(200);
expect(devices.authenticate(paired.token)?.cloudDesktopAccess).toBe(true);
expect((await ask("DELETE", `/devices/${paired.device.id}/cloud-desktop`)).status).toBe(200);
expect(devices.authenticate(paired.token)?.cloudDesktopAccess).toBe(false);
expect((await ask("POST", "/devices/missing/cloud-desktop")).status).toBe(404);
});

it("reports a permission write failure without dropping the control server", async () => {
const [device] = devices.list();
const writable = devices as unknown as { persist: () => void };
const persist = writable.persist;
writable.persist = () => {
throw new Error("ENOSPC: no space left on device");
};
try {
const failed = await ask("POST", `/devices/${device.id}/cloud-desktop`);
expect(failed).toEqual({
status: 500,
body: { error: "could not save cloud desktop access" },
});
expect(devices.list().find((candidate) => candidate.id === device.id)?.cloudDesktopAccess).toBe(false);
expect((await ask("GET", "/state")).status).toBe(200);
} finally {
writable.persist = persist;
}
});

it("refuses a state change from a foreign page", async () => {
// The attack this exists for: a form POST needs no preflight, and the
// Host header on it is the loopback one this server already approves.
Expand Down
27 changes: 27 additions & 0 deletions companion/test/devices.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ describe("DeviceRegistry", () => {
delete stored.devices[0].name;
delete stored.devices[0].lastSeenAt;
delete stored.devices[0].createdAt;
delete stored.devices[0].cloudDesktopAccess;
writeFileSync(file, JSON.stringify(stored));

const reloaded = new DeviceRegistry();
Expand All @@ -68,6 +69,7 @@ describe("DeviceRegistry", () => {
expect(listed.name).toBe("Companion");
expect(Number.isFinite(listed.lastSeenAt)).toBe(true);
expect(Number.isFinite(listed.createdAt)).toBe(true);
expect(listed.cloudDesktopAccess).toBe(false);
// and the token it was paired with still works
expect(reloaded.authenticate(token)?.id).toBe(device.id);
});
Expand Down Expand Up @@ -126,6 +128,31 @@ describe("DeviceRegistry", () => {
expect(registry.count()).toBe(1);
});

it("keeps cloud desktop access off until enabled for that device", () => {
const registry = new DeviceRegistry();
const { token, device } = pair(registry);

expect(device.cloudDesktopAccess).toBe(false);
expect(registry.authenticate(token)?.cloudDesktopAccess).toBe(false);
expect(registry.setCloudDesktopAccess(device.id, true)).toBe(true);
expect(registry.authenticate(token)?.cloudDesktopAccess).toBe(true);
expect(new DeviceRegistry().authenticate(token)?.cloudDesktopAccess).toBe(true);
expect(registry.setCloudDesktopAccess(device.id, false)).toBe(true);
expect(registry.authenticate(token)?.cloudDesktopAccess).toBe(false);
expect(registry.setCloudDesktopAccess("missing", true)).toBe(false);
});

it("rolls cloud desktop access back when it cannot be saved", () => {
const registry = new DeviceRegistry();
const { token, device } = pair(registry);
(registry as unknown as { persist: () => void }).persist = () => {
throw new Error("ENOSPC: no space left on device");
};

expect(() => registry.setCloudDesktopAccess(device.id, true)).toThrow("ENOSPC");
expect(registry.authenticate(token)?.cloudDesktopAccess).toBe(false);
});

it("uses a high-entropy QR credential and burns the manual fallback with it", () => {
const registry = new DeviceRegistry();
const { code, token } = registry.openPairing();
Expand Down
27 changes: 25 additions & 2 deletions companion/test/proxy-response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const deeplyNested = (() => {
let harness: Server;
let sidecar: Server;
let sidecarPort = 0;
let cloudDesktopAccess = true;
/** What the stub harness answers with next. Set per test. */
let respond: (res: ServerResponse) => void = (res) => res.end();

Expand All @@ -33,8 +34,9 @@ const close = (server: Server | undefined): Promise<void> =>
new Promise((resolve) => (server ? server.close(() => resolve()) : resolve()));

/** A request as a paired device makes it. */
const device = async (path = "/api/bots"): Promise<{ status: number; text: string }> => {
const device = async (path = "/api/bots", method = "GET"): Promise<{ status: number; text: string }> => {
const res = await fetch(`http://127.0.0.1:${sidecarPort}${path}`, {
method,
headers: { authorization: `Bearer ${TOKEN}` },
});
return { status: res.status, text: await res.text() };
Expand All @@ -47,7 +49,7 @@ beforeAll(async () => {
sidecar = createServer(
createProxyHandler({
harnessPort,
authenticate: (t) => t === TOKEN,
authenticate: (t) => (t === TOKEN ? { cloudDesktopAccess } : null),
redeem: () => ({ error: "not used here" }),
serverName: () => "Test computer",
}),
Expand All @@ -61,6 +63,27 @@ afterAll(async () => {
});

describe("preparing a harness response for a device", () => {
it("requires the Mac to enable cloud desktop for this phone", async () => {
cloudDesktopAccess = false;
try {
const { status, text } = await device("/api/bots/b1/computer/join", "POST");
expect(status).toBe(403);
expect(text).toContain("enable it in OpenMausBot");
} finally {
cloudDesktopAccess = true;
}
});

it("forwards only the enabled device's request for a fresh viewer", async () => {
respond = (res) => {
res.writeHead(200, { "content-type": "application/json" });
res.end(JSON.stringify({ joinUrl: "https://desktop.example/session/fresh", state: "ready" }));
};
const { status, text } = await device("/api/bots/b1/computer/join", "POST");
expect(status).toBe(200);
expect(JSON.parse(text).joinUrl).toBe("https://desktop.example/session/fresh");
});

it("never forwards a body it could not scrub", async () => {
// `scrub` recurses once per level, so a deeply nested body throws
// RangeError while JSON.parse handles it without complaint. That gap is
Expand Down
Loading
Loading