diff --git a/src/server.tsx b/src/server.tsx index 6c652330..ac2ca1a1 100644 --- a/src/server.tsx +++ b/src/server.tsx @@ -4,9 +4,10 @@ import { serve } from 'bun'; import { honoServer, PROTECTED_API_PREFIXES, UNPROTECTED_API_PREFIXES } from './servers/hono'; import { assertCapabilityTotality } from './servers/capabilities/totality'; import { assertSecretsClosed } from './servers/os-user'; +import { resolveHomeDir } from './servers/user-home'; import { resolveAuthToken } from './servers/auth-token'; import { isWsProviderAllowed } from './servers/capabilities/authorize'; -import { isTokenBlacklisted } from 'officerdb'; +import { isTokenBlacklisted, getUserById } from 'officerdb'; import { terminalWebsocket } from './servers/api/terminal/websocket'; import { chatWebsocket } from './servers/api/chat/websocket'; import { taskRunnerWebsocket } from './servers/api/tasks/task-executor'; @@ -193,6 +194,41 @@ async function upgradeWs( const command = url.searchParams.get('command') ?? undefined; const cols = url.searchParams.get('cols') ? Number(url.searchParams.get('cols')) : undefined; const rows = url.searchParams.get('rows') ? Number(url.searchParams.get('rows')) : undefined; + + // ── Whose shell is this ── + // + // The terminal bridge forwards this query string to the pty sidecar untouched, and the sidecar starts a + // shell from what it finds there. So `osUser` and `home` are resolved HERE, from the authenticated + // account, and any values the browser sent are deleted first. Trusting the client for either would let a + // member ask for the owner's uid in a query parameter. + // + // Absent for the owner: no `osUser` means the sidecar runs the shell as itself, which is the behaviour + // this has always had. + url.searchParams.delete('osUser'); + url.searchParams.delete('home'); + + // The other half of the temporary chat gap — see api/chat/chat.ts for the whole reasoning. The capability + // is grantable so the route resolves, but a turn would spawn `claude` as the OWNER, so the transport is + // owner-only until agents run under `runAs`. Refusing the socket is what makes that true rather than + // documented. + if (provider === 'chat') { + const chatUser = await getUserById(user.id); + if (chatUser?.role !== 'Super Admin') return new Response('Forbidden', { status: 403 }); + } + + if (provider === 'terminal') { + const resolved = await resolveHomeDir(user.id); + if (!resolved.ok) return new Response('Forbidden', { status: 403 }); + if (!resolved.isOwner) { + const dbUser = await getUserById(user.id); + // A confined capability is only granted to an account with an OS user, so this should not happen — + // and if it ever does, refusing beats opening the owner's shell. + if (!dbUser?.osUser) return new Response('Forbidden', { status: 403 }); + url.searchParams.set('osUser', dbUser.osUser); + url.searchParams.set('home', resolved.home); + } + } + const ok = server.upgrade(req, { data: { userId: user.id, diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index efffece4..fd2b8bef 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -1,5 +1,7 @@ import type { Context } from 'hono'; import { createRouter } from '../../create-router'; +import { isSuperAdmin } from '@@/super-admin'; +import * as errors from '@@/custom-errors'; import * as sidecar from '@@/sidecar-registry'; import { getUserSettings } from 'officerdb'; import { @@ -30,6 +32,27 @@ import { registerAgentPanelRoutes } from './agent-panels-routes'; export const chatRouter = createRouter(); +// ── Chat is grantable, and its machinery is not ready for a member. This is that gap, held open on purpose ── +// +// The `chat` capability moved from `execution` to `confined` so the owner can grant it and the route resolves. +// The agent underneath has NOT moved: `claude-manager.ts` drives turns through the Agent SDK, which spawns +// `claude` itself with no way to hand it a uid, and every transcript path here resolves through the owner's +// home. So a member reaching this router would read the owner's session list and run an agent as the owner — +// which is the whole thing the confinement work exists to prevent. +// +// Refused wholesale rather than per-route, and reads rather than just writes: `listClaudePwds` returns the +// directory names of the owner's projects, which is not a member's business either. +// +// What lifts this is per-user agents: the turn becomes its own process under `runAs`, with the member's own +// HOME so `~/.claude` and their transcripts are theirs. docs/per-user-linux-accounts.md § stage 5. Delete this +// middleware then — it is the only thing standing between a granted member and the owner's agent. +chatRouter.use(async (ctx, next) => { + if (!(await isSuperAdmin(ctx.get('user')))) { + throw errors.FORBIDDEN('Chat is not available to members yet — the agent still runs as the server owner.'); + } + return next(); +}); + // The working directory a request operates on: an explicit ?cwd= (a chosen pwd), else the default // general_chat_sessions dir. Claude groups sessions by cwd, so this selects which project group we read. // (OpenCode sessions all live in the one fixed server and ignore cwd.) diff --git a/src/servers/capabilities/registry.test.ts b/src/servers/capabilities/registry.test.ts index 64f39e61..cb472a29 100644 --- a/src/servers/capabilities/registry.test.ts +++ b/src/servers/capabilities/registry.test.ts @@ -154,9 +154,10 @@ describe('self-service routes', () => { describe('kinds', () => { test('execution capabilities are never grantable', () => { const grantable = new Set(GRANTABLE_CAPABILITIES.map((c) => c.key)); - // `files` left this list on 2026-08-11 when it became `confined` — see the test below and - // docs/per-user-linux-accounts.md. Everything still here runs as the OWNER in the owner's home. - for (const key of ['terminal', 'chat', 'tasks', 'desktop', 'browser', 'items']) { + // `files` left this list on 2026-08-11, then `terminal` and `chat` the same day — see the tests below and + // docs/per-user-linux-accounts.md. Everything still here runs as the OWNER in the owner's home with no + // per-caller resolution at all. + for (const key of ['tasks', 'desktop', 'browser', 'items']) { expect(CAPABILITY_BY_KEY.get(key)?.kind).toBe('execution'); expect(grantable.has(key)).toBe(false); } @@ -166,7 +167,7 @@ describe('kinds', () => { // authorize.ts, which is where the rule can cover routes, sockets and the dock at once. test('confined capabilities are grantable', () => { const grantable = new Set(GRANTABLE_CAPABILITIES.map((c) => c.key)); - for (const key of ['files']) { + for (const key of ['files', 'terminal', 'chat']) { expect(CAPABILITY_BY_KEY.get(key)?.kind).toBe('confined'); expect(grantable.has(key)).toBe(true); } @@ -174,9 +175,14 @@ describe('kinds', () => { // The claim `confined` makes is that every path it reaches resolves its directory from the CALLER. That // cannot be asserted from the registry, so this pins the inverse: nothing becomes confined without a - // deliberate edit here, and the list is short enough to audit by eye. + // deliberate edit here, and the list stays short enough to audit by eye. + // + // `chat` is on it ahead of its implementation, deliberately and temporarily: the capability is grantable so + // the route resolves, while api/chat/chat.ts and the chat socket both refuse a non-owner because a turn + // would still spawn the agent as the owner. If you are here because this test failed, check that those two + // guards moved together with whatever you changed. test('confined is a short, deliberate list', () => { - expect(CAPABILITIES.filter((c) => c.kind === 'confined').map((c) => c.key)).toEqual(['files']); + expect(CAPABILITIES.filter((c) => c.kind === 'confined').map((c) => c.key)).toEqual(['terminal', 'chat', 'files']); }); test('admin capabilities are never grantable', () => { diff --git a/src/servers/capabilities/registry.ts b/src/servers/capabilities/registry.ts index 6f8db18b..4ab75659 100644 --- a/src/servers/capabilities/registry.ts +++ b/src/servers/capabilities/registry.ts @@ -255,20 +255,33 @@ export const CAPABILITIES: Capability[] = [ }, // ── execution: never grantable ────────────────────────────────────────────────────────────────── + // Confined since 2026-08-11. A member's shell is spawned by the pty sidecar through `sudo setpriv` as their + // own Linux account, in their own home, with the platform's environment cleared — so it is their shell, and + // the kernel decides what it can reach. The sidecar also records whose each session is, so `list` and `kill` + // scope to the caller instead of every shell on the box. + // + // What this is NOT is a jail. A member with a shell can `cd /` and read whatever the system leaves + // world-readable, like any account on any machine. It isolates members from each other and from the owner's + // files, which is the promise `confined` makes. { key: 'terminal', label: 'Terminal', - description: 'A real shell as the server owner', - kind: 'execution', + description: 'A shell on this machine, as your own user', + kind: 'confined', api: ['/terminal'], ws: ['terminal'], routes: ['/terminal'], }, + // Confined so the owner can grant it and the route resolves — but the agent underneath still runs as the + // OWNER, so `api/chat/chat.ts` refuses a non-owner outright and the chat socket is refused in server.tsx. + // A deliberate, temporary gap: the permission exists, the functionality follows when a turn can be spawned + // under `runAs` with the member's own HOME. Until then this grant buys a route and a refusal, and the + // comments at both guards say so. { key: 'chat', label: 'Chat', - description: 'The agent, running unsandboxed as the server owner', - kind: 'execution', + description: 'The agent', + kind: 'confined', api: ['/chat'], ws: ['chat'], routes: ['/chat'], diff --git a/src/servers/sidecar/pty/server.mjs b/src/servers/sidecar/pty/server.mjs index cf640952..2b039ba3 100644 --- a/src/servers/sidecar/pty/server.mjs +++ b/src/servers/sidecar/pty/server.mjs @@ -27,13 +27,19 @@ export function startServer() { const url = new URL(req.url ?? '/', 'http://127.0.0.1'); // Officer-owned routes, reached through the platform's authenticated proxy. + // + // `osUser` scopes both to one account's sessions. The platform sends it for a member and omits it for the + // owner; absent means unscoped. Until this existed these two listed and killed EVERY shell on the box for + // anyone who could reach them, which was safe only because the terminal was owner-only. + const scope = url.searchParams.has('osUser') ? (url.searchParams.get('osUser') || null) : undefined; + if (url.pathname === '/_officer/sessions' && req.method === 'GET') { - return json(res, 200, { sessions: store.list() }); + return json(res, 200, { sessions: store.list(scope) }); } const killMatch = url.pathname.match(/^\/_officer\/sessions\/([^/]+)$/); if (killMatch && req.method === 'DELETE') { - const killed = store.kill(decodeURIComponent(killMatch[1])); + const killed = store.kill(decodeURIComponent(killMatch[1]), scope); return json(res, killed ? 200 : 404, { ok: killed }); } @@ -55,6 +61,10 @@ export function startServer() { cwd: url.searchParams.get('cwd') ?? undefined, cols: Number(url.searchParams.get('cols')) || 0, rows: Number(url.searchParams.get('rows')) || 0, + // Injected by the platform's bridge from the authenticated account, after stripping whatever the + // browser sent. Absent for the owner, whose shell runs as this process. + osUser: url.searchParams.get('osUser') || undefined, + home: url.searchParams.get('home') || undefined, }); if (!session) { socket.close(1011, 'failed to start terminal'); diff --git a/src/servers/sidecar/pty/sessions.mjs b/src/servers/sidecar/pty/sessions.mjs index 4935175f..a34792e0 100644 --- a/src/servers/sidecar/pty/sessions.mjs +++ b/src/servers/sidecar/pty/sessions.mjs @@ -28,12 +28,21 @@ const SHELL = { command: process.env.SHELL ?? '/bin/zsh', args: ['-i'] }; /** @type {Map} */ const sessions = new Map(); -const resolveCwd = (cwd) => { - if (!cwd || cwd === '~') return HOME_DIR; - if (cwd.startsWith('~/')) return join(HOME_DIR, cwd.slice(2)); +/** + * Where a shell opens. `base` is the requesting account's home; without one this is the owner's HOME_DIR, as + * it always was. + * + * An absolute `cwd` is honoured as given, for the owner and for a member alike. That is not a hole and not an + * oversight: the member's shell runs as their uid, so the kernel decides what they can see once they get + * there. A path check here would be theatre — `cd /` is one keystroke away in any shell. + */ +const resolveCwd = (cwd, base) => { + const home = base || HOME_DIR; + if (!cwd || cwd === '~') return home; + if (cwd.startsWith('~/')) return join(home, cwd.slice(2)); if (cwd.startsWith('/')) return cwd; // Relative paths have no meaning here — this process's cwd is the repo, not the user's folder. - return HOME_DIR; + return home; }; const appendBuffer = (session, data) => { @@ -58,11 +67,59 @@ const broadcast = (session, msg) => { } }; -/** Attach a client to a session, spawning the shell if this is the first time we've seen the id. */ -export function attach(sessionId, client, { cwd, cols, rows } = {}) { +/** + * The argv that opens a shell as `osUser`, or as this process when there is none. + * + * Mirrors `runAs` in src/servers/os-user.ts, and cannot import it: this sidecar runs under node (node-pty + * binds a native addon against node's ABI) while that file is Bun/TypeScript. If you change one, change both + * — the flags are load-bearing and the reasoning for each is documented there. + * + * node-pty does support `uid`/`gid` options, unlike Bun.spawn. They are deliberately NOT used: they set the + * ids without applying the account's supplementary groups or resetting the environment, so the shell would + * keep the OWNER's groups and the platform's entire env — including everything Bun loaded from `.env`. + * + * `--reset-env` keeps TERM (per setpriv(1)) and sets HOME/SHELL/USER/LOGNAME/PATH from the target's passwd + * entry. COLORTERM does not survive it, so it is re-stated through `env` — it is how programs decide they may + * emit 24-bit colour. + */ +function shellArgv(osUser) { + if (!osUser) return { command: SHELL.command, args: SHELL.args }; + return { + command: 'sudo', + args: [ + '-n', + 'setpriv', + `--reuid=${osUser}`, + `--regid=${osUser}`, + '--init-groups', + '--reset-env', + '--', + 'env', + 'COLORTERM=truecolor', + SHELL.command, + ...SHELL.args, + ], + }; +} + +/** + * Attach a client to a session, spawning the shell if this is the first time we've seen the id. + * + * `osUser` and `home` come from the PLATFORM, which resolves them from the authenticated account — never from + * anything the browser sent. This socket binds loopback and only the platform's bridge reaches it, which is + * the same trust model as the `X-Officer-User` header on the HTTP sidecars. + */ +export function attach(sessionId, client, { cwd, cols, rows, osUser, home } = {}) { let session = sessions.get(sessionId); if (session) { + // A session belongs to whoever started it. Re-attaching is only allowed for the same account, or this is + // how one member resumes another's shell by guessing a session id — and the id is in a query string. + if ((session.osUser ?? null) !== (osUser ?? null)) { + client.send({ type: 'output', data: '\r\n[Terminal error] that session belongs to another account\r\n' }); + client.send({ type: 'exit', exitCode: 1 }); + return null; + } session.clients.add(client); // Scrollback goes out as `replay`, not as ordinary output: the client may already be showing some of // it, so it resets and rebuilds from this rather than appending a second copy. @@ -76,12 +133,16 @@ export function attach(sessionId, client, { cwd, cols, rows } = {}) { let term; try { - term = pty.spawn(SHELL.command, SHELL.args, { + const { command, args } = shellArgv(osUser); + term = pty.spawn(command, args, { name: 'xterm-256color', cols: spawnCols, rows: spawnRows, - cwd: resolveCwd(cwd), - // COLORTERM is how programs decide they may emit 24-bit colour — TERM only advertises 256. + // A member's shell starts in THEIR home, not the owner's. `resolveCwd` resolves `~` and relative paths + // against HOME_DIR, which is the owner's — so the base has to be passed in for anyone else. + cwd: resolveCwd(cwd, home), + // For a member this env reaches `sudo` and `setpriv`, and `--reset-env` clears it before the shell: + // see shellArgv. For the owner it is the shell's environment as before. env: { ...process.env, TERM: 'xterm-256color', COLORTERM: 'truecolor' }, }); } catch (err) { @@ -101,6 +162,9 @@ export function attach(sessionId, client, { cwd, cols, rows } = {}) { lastActivityAt: now, title: '', pid: term.pid, + /** Whose shell this is, or null for the owner. Read by `list` and `kill` so one account cannot see or + * end another's — the gap TODO.md called the pty sidecar's identity blindness. */ + osUser: osUser ?? null, clients: new Set([client]), }; sessions.set(sessionId, session); @@ -145,9 +209,18 @@ export function resize(sessionId, cols, rows) { } } -export function kill(sessionId) { +/** + * End a session. `osUser` scopes it: pass an account name and only that account's sessions can be killed; + * pass nothing and it is the owner, who may kill any. + * + * Undefined and null mean different things here, which is why the check is explicit. `undefined` is "the + * caller did not scope this" (the owner). `null` is "the caller is the owner's own shell", which is a real + * value a member must not match. + */ +export function kill(sessionId, osUser) { const session = sessions.get(sessionId); if (!session) return false; + if (osUser !== undefined && (session.osUser ?? null) !== osUser) return false; try { session.term.kill(); } catch { @@ -157,17 +230,21 @@ export function kill(sessionId) { return true; } -export function list() { - return [...sessions.entries()].map(([sessionId, s]) => ({ - sessionId, - cols: s.cols, - rows: s.rows, - createdAt: s.createdAt, - lastActivityAt: s.lastActivityAt, - title: s.title || undefined, - pid: s.pid, - clients: s.clients.size, - })); +/** Sessions, scoped the same way as `kill`: an account name sees only its own, nothing sees everything. */ +export function list(osUser) { + return [...sessions.entries()] + .filter(([, s]) => osUser === undefined || (s.osUser ?? null) === osUser) + .map(([sessionId, s]) => ({ + sessionId, + cols: s.cols, + rows: s.rows, + createdAt: s.createdAt, + lastActivityAt: s.lastActivityAt, + title: s.title || undefined, + pid: s.pid, + clients: s.clients.size, + osUser: s.osUser ?? undefined, + })); } export function killAll() {