fix(cron): do not bypass ownership check when callerSessionKey is present
When a gateway caller supplies a callerSessionKey it is explicitly requesting session-scoped access (multi-agent / multi-user deployments). Previously, resolveCronCallerOptions unconditionally set ownerOverride to true whenever the client held ADMIN_SCOPE, which meant the service-layer ownership check was a no-op for every mutation (cron.update, cron.remove, cron.run) since those methods all require ADMIN_SCOPE. Now ownerOverride is only true when the client is an admin that did NOT supply a session key — the typical local-CLI / control-UI case. When a session key is present the ownership check fires as intended. Also exports resolveCronCallerOptions and adds direct unit tests covering admin + sessionKey, admin without sessionKey, non-admin, and null client scenarios. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
555013a875
commit
62daaba64a
61
src/gateway/server-methods/cron.caller-options.test.ts
Normal file
61
src/gateway/server-methods/cron.caller-options.test.ts
Normal file
@ -0,0 +1,61 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { ADMIN_SCOPE, READ_SCOPE, WRITE_SCOPE } from "../method-scopes.js";
|
||||
import { resolveCronCallerOptions } from "./cron.js";
|
||||
import type { GatewayClient } from "./types.js";
|
||||
|
||||
function makeClient(scopes: string[]): GatewayClient {
|
||||
return {
|
||||
connect: {
|
||||
minProtocol: 1,
|
||||
maxProtocol: 1,
|
||||
client: {
|
||||
id: "control-ui",
|
||||
version: "1.0.0",
|
||||
platform: "test",
|
||||
mode: "operator",
|
||||
},
|
||||
scopes,
|
||||
},
|
||||
} as GatewayClient;
|
||||
}
|
||||
|
||||
describe("resolveCronCallerOptions", () => {
|
||||
it("sets ownerOverride=true for admin without sessionKey", () => {
|
||||
const opts = resolveCronCallerOptions(makeClient([ADMIN_SCOPE]));
|
||||
expect(opts.ownerOverride).toBe(true);
|
||||
expect(opts.callerSessionKey).toBeUndefined();
|
||||
});
|
||||
|
||||
it("sets ownerOverride=false for admin WITH sessionKey", () => {
|
||||
const opts = resolveCronCallerOptions(makeClient([ADMIN_SCOPE]), "telegram:direct:111");
|
||||
expect(opts.ownerOverride).toBe(false);
|
||||
expect(opts.callerSessionKey).toBe("telegram:direct:111");
|
||||
});
|
||||
|
||||
it("sets ownerOverride=false for non-admin without sessionKey", () => {
|
||||
const opts = resolveCronCallerOptions(makeClient([READ_SCOPE]));
|
||||
expect(opts.ownerOverride).toBe(false);
|
||||
expect(opts.callerSessionKey).toBeUndefined();
|
||||
});
|
||||
|
||||
it("sets ownerOverride=false for non-admin with sessionKey", () => {
|
||||
const opts = resolveCronCallerOptions(
|
||||
makeClient([READ_SCOPE, WRITE_SCOPE]),
|
||||
"discord:channel:ops",
|
||||
);
|
||||
expect(opts.ownerOverride).toBe(false);
|
||||
expect(opts.callerSessionKey).toBe("discord:channel:ops");
|
||||
});
|
||||
|
||||
it("handles null client gracefully", () => {
|
||||
const opts = resolveCronCallerOptions(null, "telegram:direct:111");
|
||||
expect(opts.ownerOverride).toBe(false);
|
||||
expect(opts.callerSessionKey).toBe("telegram:direct:111");
|
||||
});
|
||||
|
||||
it("handles null client without sessionKey", () => {
|
||||
const opts = resolveCronCallerOptions(null);
|
||||
expect(opts.ownerOverride).toBe(false);
|
||||
expect(opts.callerSessionKey).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@ -26,17 +26,27 @@ import type { GatewayClient, GatewayRequestHandlers } from "./types.js";
|
||||
/**
|
||||
* Resolves the caller identity and admin-bypass flag from the connected client.
|
||||
*
|
||||
* ownerOverride is true when the client holds the operator.admin scope, meaning
|
||||
* it can read and mutate any cron job regardless of ownership metadata.
|
||||
* When the caller supplies a `callerSessionKey` it is explicitly requesting
|
||||
* session-scoped access (multi-agent / multi-user deployments). In that case
|
||||
* the ownership check in the service layer must fire even if the client holds
|
||||
* `ADMIN_SCOPE`, so `ownerOverride` stays false.
|
||||
*
|
||||
* `ownerOverride` is only true when the client is an admin that did **not**
|
||||
* supply a session key — the typical local-CLI / control-UI case where a
|
||||
* single operator manages all jobs.
|
||||
*/
|
||||
function resolveCronCallerOptions(
|
||||
export function resolveCronCallerOptions(
|
||||
client: GatewayClient | null,
|
||||
callerSessionKey?: string,
|
||||
): CronMutationCallerOptions {
|
||||
const scopes: readonly string[] = Array.isArray(client?.connect?.scopes)
|
||||
? (client.connect.scopes as string[])
|
||||
: [];
|
||||
const ownerOverride = scopes.includes(ADMIN_SCOPE);
|
||||
const isAdmin = scopes.includes(ADMIN_SCOPE);
|
||||
// Only bypass ownership when the caller is an admin that did NOT supply a
|
||||
// session key. A present session key signals session-scoped intent, so the
|
||||
// service-layer ownership check must still run.
|
||||
const ownerOverride = isAdmin && !callerSessionKey;
|
||||
return {
|
||||
callerSessionKey: callerSessionKey ?? undefined,
|
||||
ownerOverride,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user