mirror of
https://github.com/Termix-SSH/Termix.git
synced 2026-10-28 14:15:24 +00:00
fix: harden plugin auth and add per-request identity for ctx calls
Require auth on /plugin-api, gate plugin enable/grant/revoke routes behind admin.plugins.manage, and thread the calling user through worker ctx.hosts/ctx.ssh calls instead of always using the install owner. Also adds the plugin permissions grant/revoke UI.
This commit is contained in:
1 parent
19b70430a1
commit
c58ccbfc3f
16 files changed
+1317
-78
No files matched your search
@@ -216,7 +216,7 @@ export class PluginBroker {
|
||||
runtime: PluginRuntime,
|
||||
request: PluginRequest,
|
||||
): Promise<void> {
|
||||
const { method, args } = request;
|
||||
const { method, args, callerUserId } = request;
|
||||
let failure: Error | null = null;
|
||||
|
||||
try {
|
||||
@@ -229,7 +229,7 @@ export class PluginBroker {
|
||||
);
|
||||
}
|
||||
|
||||
const value = await this.invoke(runtime, method, args);
|
||||
const value = await this.invoke(runtime, method, args, callerUserId);
|
||||
this.reply(runtime, { id: request.id, ok: true, value });
|
||||
} catch (error) {
|
||||
failure = error instanceof Error ? error : new Error(String(error));
|
||||
@@ -246,7 +246,7 @@ export class PluginBroker {
|
||||
}
|
||||
|
||||
if (AUDITED.has(method)) {
|
||||
void this.audit(runtime, method, args, failure);
|
||||
void this.audit(runtime, method, args, failure, callerUserId);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -263,6 +263,7 @@ export class PluginBroker {
|
||||
runtime: PluginRuntime,
|
||||
method: string,
|
||||
args: unknown[],
|
||||
callerUserId?: string,
|
||||
): Promise<unknown> {
|
||||
switch (method) {
|
||||
case "log.debug":
|
||||
@@ -277,10 +278,10 @@ export class PluginBroker {
|
||||
return null;
|
||||
|
||||
case "hosts.list":
|
||||
return this.handleHostsList(runtime);
|
||||
return this.handleHostsList(runtime, callerUserId);
|
||||
|
||||
case "hosts.get":
|
||||
return this.handleHostsGet(runtime, Number(args[0]));
|
||||
return this.handleHostsGet(runtime, Number(args[0]), callerUserId);
|
||||
|
||||
case "storage.get":
|
||||
return this.handleStorageGet(runtime, storageKey(args[0]));
|
||||
@@ -304,7 +305,7 @@ export class PluginBroker {
|
||||
return this.handleHttpRoute(runtime, String(args[0]), String(args[1]));
|
||||
|
||||
case "ssh.connect":
|
||||
return this.handleSshConnect(runtime, Number(args[0]));
|
||||
return this.handleSshConnect(runtime, Number(args[0]), callerUserId);
|
||||
|
||||
case "ssh.exec":
|
||||
return this.handleSshExec(
|
||||
@@ -349,8 +350,9 @@ export class PluginBroker {
|
||||
|
||||
private async handleHostsList(
|
||||
runtime: PluginRuntime,
|
||||
callerUserId?: string,
|
||||
): Promise<PluginHostView[]> {
|
||||
const userId = this.requireOwner(runtime);
|
||||
const userId = this.requireActor(runtime, callerUserId);
|
||||
const hosts = await this.deps.listHosts(userId);
|
||||
return hosts.map(toPluginHostView);
|
||||
}
|
||||
@@ -358,8 +360,9 @@ export class PluginBroker {
|
||||
private async handleHostsGet(
|
||||
runtime: PluginRuntime,
|
||||
hostId: number,
|
||||
callerUserId?: string,
|
||||
): Promise<PluginHostView | null> {
|
||||
const userId = this.requireOwner(runtime);
|
||||
const userId = this.requireActor(runtime, callerUserId);
|
||||
if (!Number.isFinite(hostId)) {
|
||||
throw new PluginFacingError("hosts.get requires a numeric host id");
|
||||
}
|
||||
@@ -453,8 +456,9 @@ export class PluginBroker {
|
||||
private async handleSshConnect(
|
||||
runtime: PluginRuntime,
|
||||
hostId: number,
|
||||
callerUserId?: string,
|
||||
): Promise<string> {
|
||||
const userId = this.requireOwner(runtime);
|
||||
const userId = this.requireActor(runtime, callerUserId);
|
||||
if (!Number.isFinite(hostId)) {
|
||||
throw new PluginFacingError("ssh.connect requires a numeric host id");
|
||||
}
|
||||
@@ -651,8 +655,30 @@ export class PluginBroker {
|
||||
}
|
||||
}
|
||||
|
||||
private requireOwner(runtime: PluginRuntime): string {
|
||||
const userId = runtime.plugin.ownerUserId;
|
||||
/**
|
||||
* Resolves the user a privileged ctx call acts as. This is where the two
|
||||
* identity models that coexist in one plugin meet:
|
||||
*
|
||||
* - Inside an HTTP handler, `callerUserId` is set -- the worker attached
|
||||
* it from the AsyncLocalStorage context it entered around that specific
|
||||
* ctx.http.route invocation (see worker-bootstrap.ts). The call acts as
|
||||
* whichever user's request is currently being served, so
|
||||
* ctx.hosts.list() returns THEIR hosts, not the plugin installer's.
|
||||
* Grants stay install-wide (an admin unlocks the capability for the
|
||||
* plugin as a whole); this is only about whose DATA the call touches.
|
||||
*
|
||||
* - From a schedule.every timer or an events.on listener, there is no
|
||||
* inbound request and so no AsyncLocalStorage context to read from --
|
||||
* `callerUserId` is undefined. These fall back to ownerUserId, the user
|
||||
* who last enabled the plugin, which is what background work has
|
||||
* always run as.
|
||||
*
|
||||
* A plugin cannot influence which branch it gets: no ctx method takes a
|
||||
* userId argument, so `callerUserId` only ever reflects a real inbound
|
||||
* request the server's own auth middleware verified.
|
||||
*/
|
||||
private requireActor(runtime: PluginRuntime, callerUserId?: string): string {
|
||||
const userId = callerUserId ?? runtime.plugin.ownerUserId;
|
||||
if (!userId) {
|
||||
throw new PluginFacingError(
|
||||
`Plugin ${runtime.plugin.id} has no owning user, so it cannot act on user data`,
|
||||
@@ -666,13 +692,17 @@ export class PluginBroker {
|
||||
method: string,
|
||||
args: unknown[],
|
||||
failure: Error | null,
|
||||
callerUserId?: string,
|
||||
): Promise<void> {
|
||||
const userId = runtime.plugin.ownerUserId;
|
||||
const userId = callerUserId ?? runtime.plugin.ownerUserId;
|
||||
if (!userId) return;
|
||||
|
||||
// Attribution comes from the broker, never from the plugin: username is
|
||||
// the plugin id, not the user's, so a plugin action is never mistaken for
|
||||
// something the person did themselves.
|
||||
// something the person did themselves. The userId this is logged against
|
||||
// is whichever identity requireActor would have used for the same call
|
||||
// (see its comment), so a request-triggered action is attributed to the
|
||||
// requester, not always to whoever installed the plugin.
|
||||
await logAudit({
|
||||
userId,
|
||||
username: `plugin:${runtime.plugin.id}`,
|
||||
|
||||
@@ -39,8 +39,8 @@ export function buildPluginRouter(
|
||||
query: req.query,
|
||||
body: req.body ?? null,
|
||||
headers: pickHeaders(req),
|
||||
// The acting user, resolved by the server's own auth middleware.
|
||||
// A plugin cannot set or spoof this.
|
||||
// The acting user, verified by authenticateJWT ahead of the
|
||||
// /plugin-api mount in database.ts. A plugin cannot set or spoof this.
|
||||
userId: (req as Request & { userId?: string }).userId ?? null,
|
||||
});
|
||||
|
||||
|
||||
@@ -254,7 +254,9 @@ export class PluginLoader {
|
||||
/**
|
||||
* `ownerUserId` is optional only for in-process first-party plugins, which
|
||||
* do not use the gated ctx and so never act as a user. A worker plugin
|
||||
* without one cannot reach any user data (see broker.requireOwner).
|
||||
* without one cannot reach any user data outside of a per-request caller
|
||||
* identity (see broker.requireActor), which only exists inside an HTTP
|
||||
* handler invocation and is never a substitute for having an owner at all.
|
||||
*/
|
||||
async activate(pluginId: string, ownerUserId?: string): Promise<void> {
|
||||
const plugin = this.requirePlugin(pluginId);
|
||||
|
||||
@@ -13,6 +13,15 @@ export interface PluginRequest {
|
||||
/** Dotted ctx path, e.g. "hosts.get" or "storage.set". */
|
||||
method: string;
|
||||
args: unknown[];
|
||||
/**
|
||||
* The user whose HTTP request is currently executing inside the worker, if
|
||||
* any. Set by the worker's own call() from its AsyncLocalStorage context,
|
||||
* never by plugin code -- no ctx method accepts a userId argument, so a
|
||||
* plugin has no way to set or forge this itself. Absent for a ctx call made
|
||||
* from a timer or event listener, which has no inbound request to derive an
|
||||
* actor from; the broker falls back to the plugin's ownerUserId then.
|
||||
*/
|
||||
callerUserId?: string;
|
||||
}
|
||||
|
||||
export interface PluginResponseOk {
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
* account of what that does and does not guarantee.
|
||||
*/
|
||||
|
||||
import { AsyncLocalStorage } from "node:async_hooks";
|
||||
import { parentPort, workerData } from "node:worker_threads";
|
||||
import type {
|
||||
PluginBootstrapData,
|
||||
@@ -33,11 +34,30 @@ const httpRoutes = new Map<string, (req: unknown) => unknown>();
|
||||
const eventListeners = new Map<string, Set<(payload: unknown) => void>>();
|
||||
const timers = new Map<string, () => void>();
|
||||
|
||||
/**
|
||||
* Tracks which user's HTTP request is currently executing, so a ctx call made
|
||||
* anywhere in that request's call stack -- including deep inside plugin code
|
||||
* that has no idea this exists -- can be attributed to them without adding a
|
||||
* userId parameter to every ctx method.
|
||||
*
|
||||
* Only entered around an ctx.http.route handler invocation (see handlePush
|
||||
* below). A ctx call made from a schedule.every timer or an events.on
|
||||
* listener runs outside any run() call, so getStore() correctly returns
|
||||
* undefined for those and the broker falls back to the plugin's owner.
|
||||
*/
|
||||
const requestActor = new AsyncLocalStorage<{ userId: string }>();
|
||||
|
||||
function call(method: string, ...args: unknown[]): Promise<unknown> {
|
||||
const id = nextRequestId++;
|
||||
const callerUserId = requestActor.getStore()?.userId;
|
||||
return new Promise((resolve, reject) => {
|
||||
pending.set(id, { resolve, reject });
|
||||
port!.postMessage({ id, method, args });
|
||||
port!.postMessage({
|
||||
id,
|
||||
method,
|
||||
args,
|
||||
...(callerUserId && { callerUserId }),
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -113,8 +133,16 @@ async function handlePush(push: PluginPush): Promise<void> {
|
||||
return;
|
||||
}
|
||||
|
||||
const requestUserId =
|
||||
(push.payload as { userId?: string | null } | undefined)?.userId ??
|
||||
undefined;
|
||||
|
||||
try {
|
||||
const value = await handler(push.payload);
|
||||
const value = await (requestUserId
|
||||
? requestActor.run({ userId: requestUserId }, () =>
|
||||
handler(push.payload),
|
||||
)
|
||||
: handler(push.payload));
|
||||
port!.postMessage({ id: push.replyTo, ok: true, value });
|
||||
} catch (error) {
|
||||
port!.postMessage({
|
||||
|
||||
Reference in new issue
Block a user