From 62daaba64aa82b90dfaf2a3e45406ed80ebb231b Mon Sep 17 00:00:00 2001
From: Antonio <ajgcvm@gmail.com>
Date: Tue, 17 Mar 2026 20:09:59 -0300
Subject: [PATCH] fix(cron): do not bypass ownership check when
 callerSessionKey is present
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 .../cron.caller-options.test.ts               | 61 +++++++++++++++++++
 src/gateway/server-methods/cron.ts            | 18 ++++--
 2 files changed, 75 insertions(+), 4 deletions(-)
 create mode 100644 src/gateway/server-methods/cron.caller-options.test.ts

diff --git a/src/gateway/server-methods/cron.caller-options.test.ts b/src/gateway/server-methods/cron.caller-options.test.ts
new file mode 100644
index 00000000000..eb9de34f927
--- /dev/null
+++ b/src/gateway/server-methods/cron.caller-options.test.ts
@@ -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();
+  });
+});
diff --git a/src/gateway/server-methods/cron.ts b/src/gateway/server-methods/cron.ts
index 46a0566c080..4dbf8cb3917 100644
--- a/src/gateway/server-methods/cron.ts
+++ b/src/gateway/server-methods/cron.ts
@@ -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,