a commander's bound is a rule, not a list
the allow list shipped an hour ago was a snapshot. plugin permissions
register at mount, so a plugin installed after a commander was created was
missing from its list and failed as a bare 403 with nothing saying why —
and every future thing a commander is meant to do is exactly that shape:
notify a device, send mail, spawn a sub-agent, hand a transcription to an
idle one, reach a plugin that does not exist yet.
so a credential now narrows two ways, and both only subtract:
permissions an ALLOW list. exact, a snapshot. right for a credential
with one job.
excludedKinds kinds it may never reach, resolved against the LIVE
registry on every request. right for a credential whose
job is open-ended.
a commander gets the second: every kind except admin. bounded away from
installing plugins, managing users and roles and changing server settings,
forever — and open to everything the platform learns to do, with nothing
to re-issue. still intersected with the account, so for a member it
narrows itself against their role.
verified against the running server with a commander-shaped key: tasks,
notify, file-browser and the transcribe PLUGIN all reach their routes;
/api/users, /api/plugins and /api/roles are 403. the plugin case is the
one an allow list could not have passed — it was mounted long after any
key was minted.
the commander's CLAUDE.md says what it cannot do in the rule's own terms,
and that a 403 is an answer rather than a problem to route around.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043
This commit is contained in:
@@ -2,7 +2,6 @@ import { mkdirSync, writeFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { createApiKey, revokeApiKeysByName } from 'officerdb';
|
||||
import { mintApiKey } from '@@/auth-token';
|
||||
import { PERMISSION_BY_KEY } from '@@/permissions/registry';
|
||||
import { PORT, OFFICER_API_URL } from '../../officer-url.mjs';
|
||||
import { getCommanderDir } from '../../data-path';
|
||||
|
||||
@@ -24,21 +23,28 @@ import { getCommanderDir } from '../../data-path';
|
||||
for months unattended. `api_keys.permissions` is the only thing that can bound a Super Admin — see
|
||||
`permissions/authorize.ts` → `decideApiRequest`, which applies it BEFORE the owner short-circuit.
|
||||
|
||||
── Why the default is "everything except admin" ──
|
||||
── Why it excludes a KIND rather than listing what it allows ──
|
||||
|
||||
A commander that can reach nothing is useless on the day it is created, and a default nobody can work
|
||||
with gets widened in a hurry rather than thought about. So the default removes the class that is never
|
||||
part of a watcher's job — installing plugins, managing users and roles, changing server settings — and
|
||||
leaves the rest.
|
||||
A commander's job is open-ended and grows. It should send a notification, send mail, spawn a sub-agent,
|
||||
hand a transcription to an idle one — and reach a plugin installed six months from now without being
|
||||
re-issued. What it should never do is administer the server it runs on.
|
||||
|
||||
It is a DEFAULT, not a policy: the list is stored on the key row and can be narrowed per commander.
|
||||
And for a member it narrows itself, because the check is an intersection with what their role holds.
|
||||
An allow list cannot express that. A list is a snapshot of what existed on the day it was written, so a
|
||||
plugin added afterwards is simply missing from it, and the failure arrives as a bare 403 with nothing
|
||||
saying why. Every future capability would land on that.
|
||||
|
||||
So the default is a rule: every kind except `admin`. It is resolved against the live registry on each
|
||||
request, which means it keeps meaning the same thing as the platform grows — bounded away from
|
||||
installing plugins, managing users and roles and changing server settings, forever, and open to
|
||||
everything else the platform learns to do.
|
||||
|
||||
A DEFAULT, not a policy: it is stored on the key row and can be tightened per commander with an explicit
|
||||
allow list alongside it. And for a member it narrows itself, because the check is an intersection with
|
||||
what their role holds.
|
||||
*/
|
||||
|
||||
/** Permission keys a commander gets unless told otherwise: everything the platform does that is not administration. */
|
||||
export function defaultCommanderPermissions(): string[] {
|
||||
return [...PERMISSION_BY_KEY.values()].filter((p) => p.kind !== 'admin').map((p) => p.key);
|
||||
}
|
||||
/** What a commander gets unless told otherwise: everything the platform can do, except administering it. */
|
||||
export const COMMANDER_EXCLUDED_KINDS = ['admin'];
|
||||
|
||||
/** The env file's path, and the name the key is stored under — both derived, so revocation cannot miss one. */
|
||||
export const commanderKeyName = (dirName: string) => `commander:${dirName}`;
|
||||
@@ -60,9 +66,9 @@ export async function writeCommanderCredential(params: {
|
||||
userId: number;
|
||||
email: string;
|
||||
dirName: string;
|
||||
permissions?: string[];
|
||||
}): Promise<{ path: string; permissions: string[] }> {
|
||||
const permissions = params.permissions ?? defaultCommanderPermissions();
|
||||
/** An explicit allow list, to tighten one commander further. Absent leaves it at the default rule. */
|
||||
permissions?: string[] | null;
|
||||
}): Promise<{ path: string; excludedKinds: string[] }> {
|
||||
const { secret, keyHash, prefix } = mintApiKey();
|
||||
|
||||
// Revoke first. A commander recreated under a directory name that has been used before would otherwise
|
||||
@@ -73,7 +79,8 @@ export async function writeCommanderCredential(params: {
|
||||
name: commanderKeyName(params.dirName),
|
||||
keyHash,
|
||||
prefix,
|
||||
permissions,
|
||||
permissions: params.permissions ?? null,
|
||||
excludedKinds: COMMANDER_EXCLUDED_KINDS,
|
||||
});
|
||||
|
||||
const path = commanderEnvPath(params.email, params.dirName);
|
||||
@@ -94,5 +101,5 @@ export async function writeCommanderCredential(params: {
|
||||
{ mode: 0o600 },
|
||||
);
|
||||
|
||||
return { path, permissions };
|
||||
return { path, excludedKinds: COMMANDER_EXCLUDED_KINDS };
|
||||
}
|
||||
|
||||
@@ -88,9 +88,13 @@ export function commanderClaudeMd(params: CommanderClaudeMdParams): string {
|
||||
'`POST /api/tasks/<dirName>/run` runs one and returns what it did — one reviewed definition on',
|
||||
'disk instead of a re-implementation living in a loop nobody reads.',
|
||||
'',
|
||||
'The credential is narrowed and revoked when this commander is deleted. A 403 is an answer, not a',
|
||||
'problem to route around: it means this commander was not given that, and the owner is the one who',
|
||||
'decides otherwise.',
|
||||
'It reaches everything the platform does EXCEPT administering it — installing plugins, managing',
|
||||
'users or roles, changing server settings. That holds for features added later too: the rule is',
|
||||
'resolved per request rather than frozen into a list, so a plugin installed next year is already',
|
||||
'covered without this commander being re-issued.',
|
||||
'',
|
||||
'A 403 is an answer, not a problem to route around. It means this commander was not given that,',
|
||||
'and the owner is the one who decides otherwise. Say so rather than finding another way in.',
|
||||
'',
|
||||
]
|
||||
: [
|
||||
|
||||
@@ -4,6 +4,7 @@ import { tmpdir } from 'node:os';
|
||||
import { getTaskByDirName, type TaskRecord } from './task-files';
|
||||
import { getOwnerHomeDir } from '../../data-path';
|
||||
import { buildTaskApiEnv } from './task-api-env';
|
||||
import type { CredentialScope } from '../../auth-token';
|
||||
|
||||
/**
|
||||
* Everything a script-mode task run needs before something spawns it.
|
||||
@@ -58,7 +59,7 @@ export type ScriptRunUser = {
|
||||
email: string;
|
||||
username?: string | null;
|
||||
/** The caller's credential narrowing, propagated to the script's own token. See `task-api-env.ts`. */
|
||||
permissions?: string[] | null;
|
||||
scope?: CredentialScope | null;
|
||||
};
|
||||
|
||||
export type PreparedScriptRun = {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { sign } from '@@/jwt';
|
||||
import type { CredentialScope } from '@@/auth-token';
|
||||
import { PORT, OFFICER_API_URL } from '../../officer-url.mjs';
|
||||
|
||||
const { PUBLIC_URL } = process.env;
|
||||
@@ -24,7 +25,7 @@ type TaskApiUser = {
|
||||
* back through the API with the owner's full authority and the limit would have bought nothing. The
|
||||
* script gets exactly what its caller had — never more.
|
||||
*/
|
||||
permissions?: string[] | null;
|
||||
scope?: CredentialScope | null;
|
||||
};
|
||||
|
||||
// Environment that lets a script task call Officer's own API instead of reimplementing server-side
|
||||
@@ -45,7 +46,7 @@ export async function buildTaskApiEnv(user: TaskApiUser): Promise<Record<string,
|
||||
username: user.username ?? '',
|
||||
// Only when narrowed. An absent claim is what every ordinary run already signs, and `resolveAuthToken`
|
||||
// spreads the payload onto the identity, so this reaches both doors with no further wiring.
|
||||
...(user.permissions ? { permissions: user.permissions } : {}),
|
||||
...(user.scope ? { scope: user.scope } : {}),
|
||||
},
|
||||
'12h',
|
||||
);
|
||||
|
||||
@@ -94,7 +94,7 @@ tasksRouter.post('/:name/run', async (ctx) => {
|
||||
id: user.id,
|
||||
email: user.email,
|
||||
username: user.username ?? '',
|
||||
permissions: ctx.get('credentialPermissions'),
|
||||
scope: ctx.get('credentialScope'),
|
||||
},
|
||||
});
|
||||
// A non-zero exit is reported, not thrown: the script ran, and its output is the useful part of the
|
||||
|
||||
Reference in New Issue
Block a user