Allow a component-registered operation to be granted in a role's operations allowlist

integrationTests/components/fixtures/registered-operation/resources.js

@@ -37,3 +37,16 @@ server.registerOperation({
 		throw error;
 	},
 });
+
+server.registerOperation({
+	name: 'component_registered_grantable',
+	requiresSuperUser: true,
+	execute: async function componentRegisteredGrantable(op) {
+		return {
+			granted: true,
+			executedOnMainThread: isMainThread,
+			executedOnThreadId: threadId,
+			username: op.hdb_user?.username ?? null,
+		};
+	},
+});

integrationTests/components/registered-operation.test.ts

@@ -6,6 +6,11 @@
  * OPERATION_FUNCTION_MAP instances; the ops-API dispatcher runs on the main thread with its
  * own instance. The cross-thread bridge (server/serverHelpers/registeredOperations.ts)
  * forwards an unrecognized operation to one registering worker and relays the result.
+ *
+ * The same split governs the role `operations` allowlist: registerOperationPermission marks a
+ * declared op grantable on the worker, but validateOperations is consulted on the main thread.
+ * Only an integration test crosses that boundary — unit tests register and validate on one
+ * thread, so the topology, and therefore the gap, is invisible to them.
  */
 import { suite, test, before, after } from 'node:test';
 import { strictEqual, ok } from 'node:assert';
@@ -15,6 +20,24 @@ import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '
 
 const FIXTURE_PATH = resolve(import.meta.dirname, 'fixtures/registered-operation');
 
+const GRANTABLE_OP = 'component_registered_grantable';
+const GRANTED_ROLE = 'component_op_granted_role';
+const GRANTED_USER = 'component_op_granted_user';
+const UNGRANTED_ROLE = 'component_op_ungranted_role';
+const UNGRANTED_USER = 'component_op_ungranted_user';
+const USER_PASS = 'Abc1234!';
+// A trust policy the OIDC operation will accept on every axis except the one under test, so a
+// rejection can only be about the operation name. Shapes taken from the GitHub Actions profile's
+// own requirements: a canonical audience (explicit port, trailing slash) and specific claims.
+const TRUST_POLICY = {
+	issuer: 'https://token.actions.githubusercontent.com',
+	audience: 'https://my-instance.harperdb.io:9925/',
+	claims: {
+		repository_id: '67890',
+		workflow_ref: 'HarperFast/my-app/.github/workflows/deploy.yml@refs/heads/main',
+	},
+};
+
 suite('Component: registered-operation (#1736)', (ctx: ContextWithHarper) => {
 	async function op(body: any): Promise<{ status: number; body: any }> {
 		const { username, password } = ctx.harper.admin;
@@ -29,6 +52,18 @@ suite('Component: registered-operation (#1736)', (ctx: ContextWithHarper) => {
 		return { status: response.status, body: await response.json() };
 	}
 
+	async function asUser(username: string, body: any): Promise<{ status: number; body: any }> {
+		const response = await fetch(ctx.harper.operationsAPIURL, {
+			method: 'POST',
+			headers: {
+				'Content-Type': 'application/json',
+				'Authorization': `Basic ${Buffer.from(`${username}:${USER_PASS}`).toString('base64')}`,
+			},
+			body: JSON.stringify(body),
+		});
+		return { status: response.status, body: await response.json() };
+	}
+
 	before(async () => {
 		// Multiple HTTP workers so the forward actually has a choice of registering threads.
 		await setupHarperWithFixture(ctx, FIXTURE_PATH, {
@@ -88,4 +123,122 @@ suite('Component: registered-operation (#1736)', (ctx: ContextWithHarper) => {
 		strictEqual(status, 400, JSON.stringify(body));
 		ok(JSON.stringify(body).includes('not found'), `expected operation-not-found error, got: ${JSON.stringify(body)}`);
 	});
+
+	suite('grantable in a role `operations` allowlist across the worker/main boundary', () => {
+		before(async () => {
+			// The announcement is fire-and-forget ITC. A successful forward proves the handler ran, and
+			// it carries the grantable flag in the same message — so this gates on the exact state
+			// these tests depend on, rather than on elapsed time.
+			const deadline = Date.now() + 15_000;
+			for (;;) {
+				const { status } = await op({ operation: GRANTABLE_OP });
+				if (status === 200) break;
+				if (Date.now() > deadline) throw new Error(`main thread never registered '${GRANTABLE_OP}'`);
+				await new Promise((resolve) => setTimeout(resolve, 100));
+			}
+		});
+
+		test('add_role accepts the worker-registered op name in `operations`', async () => {
+			const { status, body } = await op({
+				operation: 'add_role',
+				role: GRANTED_ROLE,
+				permission: { operations: [GRANTABLE_OP] },
+			});
+			strictEqual(status, 200, JSON.stringify(body));
+		});
+
+		test('add_role still rejects an op name no component registered', async () => {
+			const { status, body } = await op({
+				operation: 'add_role',
+				role: 'component_op_bogus_role',
+				permission: { operations: ['component_registered_never_declared'] },
+			});
+			strictEqual(status, 400, JSON.stringify(body));
+			ok(
+				JSON.stringify(body).includes('component_registered_never_declared'),
+				`expected the offending op name in the error, got: ${JSON.stringify(body)}`
+			);
+		});
+
+		test('alter_role accepts it too', async () => {
+			const { status, body } = await op({
+				operation: 'alter_role',
+				id: GRANTED_ROLE,
+				permission: { operations: [GRANTABLE_OP, 'user_info'] },
+			});
+			strictEqual(status, 200, JSON.stringify(body));
+		});
+
+		test('a non-super_user granted the op can actually call it', async () => {
+			const added = await op({
+				operation: 'add_user',
+				role: GRANTED_ROLE,
+				username: GRANTED_USER,
+				password: USER_PASS,
+				active: true,
+			});
+			strictEqual(added.status, 200, JSON.stringify(added.body));
+
+			const { status, body } = await asUser(GRANTED_USER, { operation: GRANTABLE_OP });
+			strictEqual(status, 200, JSON.stringify(body));
+			strictEqual(body.granted, true);
+			strictEqual(body.username, GRANTED_USER);
+			strictEqual(body.executedOnMainThread, false);
+		});
+
+		test('a non-super_user without the grant is still denied (enforcement unchanged)', async () => {
+			const role = await op({
+				operation: 'add_role',
+				role: UNGRANTED_ROLE,
+				permission: { operations: ['user_info'] },
+			});
+			strictEqual(role.status, 200, JSON.stringify(role.body));
+			const added = await op({
+				operation: 'add_user',
+				role: UNGRANTED_ROLE,
+				username: UNGRANTED_USER,
+				password: USER_PASS,
+				active: true,
+			});
+			strictEqual(added.status, 200, JSON.stringify(added.body));
+
+			const { status, body } = await asUser(UNGRANTED_USER, { operation: GRANTABLE_OP });
+			strictEqual(status, 403, JSON.stringify(body));
+		});
+
+		test('add_oidc_trust accepts the op in a trust policy scope', async () => {
+			const { status, body } = await op({
+				operation: 'add_oidc_trust',
+				id: 'component-op-policy',
+				...TRUST_POLICY,
+				user: GRANTED_USER,
+				operations: [GRANTABLE_OP],
+			});
+			strictEqual(status, 200, JSON.stringify(body));
+		});
+
+		test('add_oidc_trust still rejects an op name no component registered', async () => {
+			const { status, body } = await op({
+				operation: 'add_oidc_trust',
+				id: 'component-op-bogus-policy',
+				...TRUST_POLICY,
+				user: GRANTED_USER,
+				operations: ['component_registered_never_declared'],
+			});
+			strictEqual(status, 400, JSON.stringify(body));
+			ok(
+				JSON.stringify(body).includes('not a Harper operation'),
+				`expected the trust-policy rejection, got: ${JSON.stringify(body)}`
+			);
+		});
+
+		test('impersonation accepts an inline role naming the op', async () => {
+			const { status, body } = await op({
+				operation: GRANTABLE_OP,
+				impersonate: { role: { permission: { operations: [GRANTABLE_OP] } } },
+			});
+			strictEqual(status, 200, JSON.stringify(body));
+			strictEqual(body.granted, true);
+		});
+	});
 });

security/authn/oidc/trustPolicyOperations.ts

@@ -53,15 +53,6 @@ function validate(validation: any): void {
  * caught here, where the reader is the administrator who wrote it. Delegates to the same helper
  * add_role/alter_role use, so group names resolve identically rather than through a second
  * definition that could drift.
- *
- * Known limitation, inherited rather than introduced: that helper's registry of runtime-registered
- * operations is process-local, and the OPERATION_REGISTERED bridge propagates only name→thread
- * routing, never grantability (server/serverHelpers/registeredOperations.ts). A component's
- * `server.registerOperation` runs in a worker while this operation runs on the main thread, so an
- * operation registered that way is NOT recognized here and a policy naming one is rejected. It
- * fails closed — a rejected policy, never a widened one — and `add_role`, `alter_role`, and
- * impersonation validation all share the gap, which is why the fix belongs to that bridge rather
- * than to a local workaround here.
  */
 function assertOperationsAreKnown(operations: string[]): void {
 	const invalidOperation = validateOperations(operations);

server/DESIGN.md

@@ -148,6 +148,16 @@ must run on a worker, `registeredOperations.ts` carries that state in the same-p
 separately from the structured-cloned body. Never attach trusted dispatch state to an operation
 payload.
 
+`server.registerOperation()` runs per-worker, so anything the **main** thread must later know about a
+registered op has to ride the OPERATION_REGISTERED announcement — a module-local registry populated
+during registration exists only in the worker that registered. The bridge carries two such facts
+today: name→thread routing (for execution forwarding) and `grantable` (so `validateOperations` on
+main will accept the name in a role's `operations` allowlist, for add_role/alter_role, impersonation,
+and OIDC trust policies). Adding a third main-thread consumer of a worker-registered fact means
+extending that message, not reading a registry that main never populated. Grantability is safe to
+mirror because it only widens what an allowlist may _name_; enforcement stays on the worker's
+`chooseOperation`.
+
 ## Resource ↔ HTTP boundary
 
 `REST.ts → http(request, nextHandler)` is the chief integration point: it takes a `Request`, asks the `Resources` registry for a match, builds a `RequestTarget`, and dispatches into the Resource class's static method. Cache headers are translated to `request.expiresAt` / `onlyIfCached` / `noCache` flags within the same function.

server/serverHelpers/registeredOperations.ts

@@ -11,7 +11,7 @@
  * so a request is sent to exactly ONE registering worker (never broadcast-first-wins).
  *
  *  - Worker: `registerOperation()` announces the name (OPERATION_REGISTERED) to all threads;
- *    only the main thread records it, as name -> Set<threadId>.
+ *    only the main thread records it, as name -> Set<threadId>, plus `grantable`.
  *  - Main: on an OPERATION_FUNCTION_MAP miss, `getRemoteOperationFunction()` supplies a forwarding
  *    function that sends the request body (OPERATION_EXECUTE_REQUEST) to one live registering
  *    worker and awaits the correlated OPERATION_EXECUTE_RESPONSE.
@@ -26,7 +26,11 @@ import * as env from '../../utility/environment/environmentManager.ts';
 import harperLogger from '../../utility/logging/harper_logger.ts';
 import { ServerError } from '../../utility/errors/hdbError.ts';
 import { sendItcEvent } from '../threads/itc.js';
-import { onMessageByType, onThreadExit } from '../threads/manageThreads.js';
+import { hasThreadExited, onMessageByType, onThreadExit } from '../threads/manageThreads.js';
+import {
+	registerWorkerGrantableOperation,
+	unregisterWorkerGrantableOperation,
+} from '../../utility/operationPermissions.ts';
 import { runWithOperationAuthorizationBypass } from './operationAuthorizationState.ts';
 
 const operationLog = harperLogger.loggerWithTag('operation');
@@ -59,6 +63,9 @@ export function setLocalOperationDispatch(dispatch: typeof localDispatch) {
 
 /** name -> threadIds of workers that registered it (main thread only) */
 const registeredByWorker = new Map<string, Set<number>>();
+// Per originator, not per name: a rolling deploy whose new generation drops `requiresSuperUser`
+// must keep routing the name while retracting grantability, which a name-level flag cannot express.
+const grantableByWorker = new Map<string, Set<number>>();
 const pendingExecutions = new Map<
 	number,
 	{ targetThreadId: number; resolve: (result: any) => void; reject: (error: Error) => void }
@@ -71,27 +78,82 @@ let mainListenersAttached = false;
  * a lost announcement just means the op stays unreachable (the pre-#1736 status quo), and the
  * broadcast has its own ack timeout.
  */
-export function announceRegisteredOperation(name: string) {
+export function announceRegisteredOperation(name: string, grantable = false) {
 	if (isMainThread) return;
 	sendItcEvent({
 		type: terms.ITC_EVENT_TYPES.OPERATION_REGISTERED,
-		message: { name },
+		message: { name, grantable },
 	}).catch((error) => operationLog.error(`Failed to announce registered operation '${name}'`, error));
 }
 
 /**
  * ITC handler (all threads receive the broadcast; only main records it).
  */
-export function operationRegisteredHandler(event: { message: { name: string; originator: number } }) {
+export function operationRegisteredHandler(event: {
+	message?: { name?: string; grantable?: boolean; originator?: number };
+}) {
 	if (!isMainThread) return;
-	const { name, originator } = event.message;
+	const { name, grantable, originator } = event?.message ?? {};
 	if (typeof name !== 'string' || typeof originator !== 'number') return;
+	// An announcement can lose the race with its own thread's exit, and exit notification fires once
+	// per thread, so without this the entry would never be cleaned up. Reads manageThreads' tombstone
+	// rather than keeping a second one: job threads are one-shot workers, so a duplicate here would
+	// grow per completed job, not per worker restart.
+	if (hasThreadExited(originator)) {
+		operationLog.debug(`Ignoring operation '${name}' announced by exited worker thread ${originator}`);
+		return;
+	}
 	let workerIds = registeredByWorker.get(name);
 	if (!workerIds) registeredByWorker.set(name, (workerIds = new Set()));
 	workerIds.add(originator);
+	// Mirroring only widens what an allowlist may name; enforcement stays on the worker's
+	// chooseOperation. A re-announcement that drops the permission retracts this thread's claim.
+	setWorkerGrantable(name, originator, grantable === true);
 	operationLog.debug(`Registered operation '${name}' announced by worker thread ${originator}`);
 }
 
+/**
+ * A worker that dies mid-execution can never respond, so fail its in-flight forwards rather than
+ * waiting out the timeout, and forget its registrations (a replacement re-registers on load).
+ */
+function handleThreadExit(deadThreadId: number) {
+	for (const [name, workerIds] of registeredByWorker) {
+		if (!workerIds.delete(deadThreadId)) continue;
+		// A surviving worker that never declared a permission must not keep the name admissible.
+		if (workerIds.size === 0) dropRegistration(name);
+		else setWorkerGrantable(name, deadThreadId, false);
+	}
+	for (const [requestId, pending] of pendingExecutions) {
+		if (pending.targetThreadId === deadThreadId) {
+			pendingExecutions.delete(requestId);
+			pending.reject(new ServerError('The worker thread executing this operation exited', 503));
+		}
+	}
+}
+
+function setWorkerGrantable(name: string, threadId: number, grantable: boolean) {
+	let grantableIds = grantableByWorker.get(name);
+	if (grantable) {
+		if (!grantableIds) grantableByWorker.set(name, (grantableIds = new Set()));
+		grantableIds.add(threadId);
+		registerWorkerGrantableOperation(name);
+	} else if (grantableIds?.delete(threadId) && grantableIds.size === 0) {
+		grantableByWorker.delete(name);
+		unregisterWorkerGrantableOperation(name);
+	}
+}
+
+/**
+ * Forget an operation no live worker offers any more. Both prune paths — thread exit, and a failed
+ * send discovering a dead port — route through here so a name can never keep a route without an
+ * owner. Grantability is dropped per owner instead, in `setWorkerGrantable`.
+ */
+function dropRegistration(name: string) {
+	registeredByWorker.delete(name);
+	grantableByWorker.delete(name);
+	unregisterWorkerGrantableOperation(name);
+}
+
 let rotation = 0;
 /**
  * Main-thread dispatch fallback: if a worker registered `name`, return a forwarding operation
@@ -122,23 +184,15 @@ function attachMainListeners() {
 		if (error) pending.reject(new ServerError(error.message, error.statusCode || 500));
 		else pending.resolve(result);
 	});
-	// A worker that dies mid-execution can never respond; fail its in-flight forwards rather
-	// than waiting out the timeout, and forget its registrations (a replacement worker
-	// re-registers on component load).
-	onThreadExit((deadThreadId: number) => {
-		for (const [name, workerIds] of registeredByWorker) {
-			workerIds.delete(deadThreadId);
-			if (workerIds.size === 0) registeredByWorker.delete(name);
-		}
-		for (const [requestId, pending] of pendingExecutions) {
-			if (pending.targetThreadId === deadThreadId) {
-				pendingExecutions.delete(requestId);
-				pending.reject(new ServerError('The worker thread executing this operation exited', 503));
-			}
-		}
-	});
+	onThreadExit(handleThreadExit);
 }
 
+// Armed at load rather than on first use: serverUtilities imports this module during its own load,
+// before any worker exists. Thread-exit notification fires once per thread and is dropped outright
+// if no listener is attached yet, so a worker dying before its first announcement is processed
+// would otherwise leave a registration nothing could ever clean up.
+if (isMainThread) attachMainListeners();
+
 async function executeRemoteOperation(name: string, body: any, bypassAuth: boolean): Promise<any> {
 	attachMainListeners();
 	const workerIds = registeredByWorker.get(name);
@@ -168,7 +222,10 @@ async function executeRemoteOperation(name: string, body: any, bypassAuth: boole
 			);
 		}
 		if (!sent) {
+			// The port is gone, so this thread's claims go with it — grantability included, which
+			// handleThreadExit would otherwise not retract while other workers still route the name.
 			workerIds.delete(targetThreadId);
+			setWorkerGrantable(name, targetThreadId, false);
 			continue;
 		}
 		return new Promise((promiseResolve, promiseReject) => {
@@ -190,7 +247,7 @@ async function executeRemoteOperation(name: string, body: any, bypassAuth: boole
 			});
 		});
 	}
-	if (registeredByWorker.get(name)?.size === 0) registeredByWorker.delete(name);
+	if (registeredByWorker.get(name)?.size === 0) dropRegistration(name);
 	throw new ServerError(
 		`Operation '${name}' is registered by a component but no worker thread is available to run it`,
 		503