the notify sidecar stops trusting whoever reaches it
to send one message to four accounts an hour ago i found the sidecar's ephemeral port and POSTed to it directly, once per user, with no authentication of any kind. that worked, which was the problem: the same door was open to anything else on the machine, including a commander with a shell. two changes. POST /api/notify/broadcast, owner only via `ownerOnly` on the permission — the supported way to say one thing to everyone. teams will narrow it later; today the choice is everyone or one person. and the sidecar now requires a secret on everything under /_officer/. it is generated per boot, held in memory, announced to the platform over the registration socket beside the port, and never written to disk. X-Officer-User could not carry this on its own, and my first attempt got that wrong: it says WHO is asking, and a caller on loopback can write it itself. only the secret says the PLATFORM is asking. so the check covers the whole /_officer/ surface rather than just the send route — /_officer/ devices takes the same header to decide whose phones are listed and deleted. what it bounds, honestly: a MEMBER's commander runs as its own Linux user and cannot read this process, so for every account but the owner's this is kernel-enforced. the owner's own commander runs as the owner, same as the platform, and is not bounded by this and cannot be — the same reason `execution` permissions are never grantable. the owner's decision, and this is a real improvement rather than a boundary. `bun run notify:test` posts to that port and will now be refused. it is a developer CLI on a box where the platform is running; left to fail loudly rather than given a way in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -76,6 +76,14 @@ export type SidecarProxy = {
|
||||
router: Hono<{ Variables: HonoVariables }>;
|
||||
/** `http://127.0.0.1:<port>`, or null before the sidecar has reported in. */
|
||||
getHttpUrl: () => string | null;
|
||||
/**
|
||||
* The secret this sidecar announced at registration, for a producer INSIDE the platform calling it
|
||||
* directly rather than through this proxy. Null until it registers.
|
||||
*
|
||||
* In memory on both sides and never written down — see `sidecar/notify/index.ts` for why a file would
|
||||
* not have bounded the thing this exists to bound.
|
||||
*/
|
||||
getProducerSecret: () => string | null;
|
||||
/** The same server as a `ws://` base, for sidecars whose sockets the platform relays. */
|
||||
getWsUrl: () => string | null;
|
||||
};
|
||||
@@ -89,15 +97,19 @@ export function createSidecarProxy({
|
||||
permissionKey,
|
||||
}: SidecarProxyParams): SidecarProxy {
|
||||
let serverPort: number | null = null;
|
||||
let producerSecret: string | null = null;
|
||||
|
||||
// Tells bodyParser to keep its hands off this prefix's request bodies. Load-bearing for uploads, not an
|
||||
// optimisation: see proxied-prefixes.ts.
|
||||
registerProxiedPrefix(prefix);
|
||||
|
||||
sidecar.on(`${name}:server`, (msg) => {
|
||||
const port = (msg as { port?: number }).port;
|
||||
const { port, secret } = msg as { port?: number; secret?: string };
|
||||
if (typeof port !== 'number') return;
|
||||
serverPort = port;
|
||||
// Optional: only a sidecar that guards a producer path announces one, and one that stops announcing
|
||||
// (an older build after a rollback) must not leave a stale secret behind that no longer opens it.
|
||||
producerSecret = typeof secret === 'string' && secret ? secret : null;
|
||||
console.log(`[${name}] sidecar registered on port ${port}`);
|
||||
onRegister?.(port);
|
||||
});
|
||||
@@ -150,6 +162,10 @@ export function createSidecarProxy({
|
||||
// permissions page instead of compiled into an `if`.
|
||||
const userId = ctx.get('user').id;
|
||||
headers['X-Officer-User'] = String(userId);
|
||||
// Proof that this request came THROUGH the proxy, for a sidecar that guards a path with it. Without
|
||||
// it, `X-Officer-User` says who is asking but nothing says the platform is the one asking — and on a
|
||||
// machine where an agent has a shell, anything on loopback can write that header itself.
|
||||
if (producerSecret) headers['X-Officer-Producer'] = producerSecret;
|
||||
|
||||
if (permissionKey) {
|
||||
// Cached by role in `authorize.ts`, and the gate has already computed this for the same request, so
|
||||
@@ -224,5 +240,5 @@ export function createSidecarProxy({
|
||||
return new Response(upstream.body, { status: upstream.status, headers: new Headers(upstream.headers) });
|
||||
});
|
||||
|
||||
return { router, getHttpUrl, getWsUrl };
|
||||
return { router, getHttpUrl, getWsUrl, getProducerSecret: () => producerSecret };
|
||||
}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { randomBytes } from 'node:crypto';
|
||||
import type { SidecarCommand, SidecarEvent } from '../protocol';
|
||||
import { createSidecarConnector } from '../connect';
|
||||
import { dispatch, configuredChannels } from './dispatch';
|
||||
@@ -62,12 +63,49 @@ const port = getFreePort();
|
||||
// thing that works until it doesn't. One file, rewritten on every boot, deleted on clean shutdown.
|
||||
const ADDRESS_FILE = join(DATA_PATH, 'notify-server.url');
|
||||
|
||||
/*
|
||||
What an in-process producer must present, when it is not speaking for a signed-in user.
|
||||
|
||||
Two callers reach POST /_officer/notify: a browser through the platform's proxy, which arrives with
|
||||
X-Officer-User injected and can only ever notify itself; and a producer inside the platform — sendoff,
|
||||
chat — which is notifying somebody else and therefore names them in the body. The second had no
|
||||
authentication at all, on the reasoning that binding loopback was enough.
|
||||
|
||||
It stopped being enough when commanders arrived. A commander has a shell on this machine, so "anything
|
||||
on loopback is the platform" became "anything on loopback, including an agent, can notify any user on
|
||||
this server as if the platform had asked".
|
||||
|
||||
Generated per boot and held in MEMORY, announced to the platform over the registration socket alongside
|
||||
the port. Deliberately not a file: a member's commander runs as its own Linux user and cannot read this
|
||||
process, but any file both processes can read is a file that account could read too if it were ever
|
||||
granted a shell in the wrong place. The owner's own commander is not bounded by this and cannot be —
|
||||
it runs as the owner, same as the platform — which is the same reason `execution` permissions are
|
||||
never grantable.
|
||||
*/
|
||||
const PRODUCER_SECRET = randomBytes(32).toString('base64url');
|
||||
|
||||
const server = Bun.serve({
|
||||
port,
|
||||
hostname: '127.0.0.1',
|
||||
async fetch(req) {
|
||||
const url = new URL(req.url);
|
||||
|
||||
/*
|
||||
Everything under `/_officer/` acts on a named user, so all of it needs the secret — not just the
|
||||
send route.
|
||||
|
||||
`X-Officer-User` cannot carry this on its own. It says WHO, and a caller on loopback can write it
|
||||
itself; only the secret says the PLATFORM is the one asking. Guarding just the send route would have
|
||||
left `/_officer/devices`, where the same header decides whose phones are listed, registered and
|
||||
deleted.
|
||||
|
||||
`/_health` and `/mapping` stay open: one is a liveness probe and the other is the code-to-text
|
||||
catalogue, which is shipped to every device and is not a secret.
|
||||
*/
|
||||
if (url.pathname.startsWith('/_officer/') && req.headers.get('X-Officer-Producer') !== PRODUCER_SECRET) {
|
||||
return Response.json({ error: 'not the platform' }, { status: 403 });
|
||||
}
|
||||
|
||||
if (url.pathname === '/_health') {
|
||||
return Response.json({ ok: true, channels: configuredChannels() });
|
||||
}
|
||||
@@ -147,7 +185,7 @@ const connection = createSidecarConnector({
|
||||
// Bun types `Server.port` as optional because a unix-socket server has none; this one is TCP, so it
|
||||
// is a number. Asserted narrowly here — it used to be `as SidecarEvent` on the whole object, which
|
||||
// suppressed this and would have suppressed a genuinely wrong event shape too.
|
||||
connection.send({ type: 'notify:server', port: server.port! });
|
||||
connection.send({ type: 'notify:server', port: server.port!, secret: PRODUCER_SECRET });
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -128,7 +128,19 @@ export type SidecarCommand =
|
||||
* `<name>:server`, where the name is the sidecar's own — `music:server`, `pty:server`, and whatever a
|
||||
* plugin nobody has written yet decides to call itself. Deliberately open: see the note in SidecarEvent.
|
||||
*/
|
||||
export type SidecarPortAnnouncement = { type: `${string}:server`; port: number };
|
||||
export type SidecarPortAnnouncement = {
|
||||
type: `${string}:server`;
|
||||
port: number;
|
||||
/**
|
||||
* A per-boot secret for callers INSIDE the platform that reach this sidecar directly rather than
|
||||
* through its proxy — announced here, in memory on both sides, never written to disk.
|
||||
*
|
||||
* Optional because only a sidecar with a producer path to guard has one. `notify` is the first: its
|
||||
* send route accepts a target user in the body, which loopback alone stopped being enough to protect
|
||||
* once commanders could run a shell on the same machine.
|
||||
*/
|
||||
secret?: string;
|
||||
};
|
||||
|
||||
export type SidecarEvent =
|
||||
| { type: 'pong'; id: string }
|
||||
|
||||
Reference in New Issue
Block a user