From 02cf12371f9353a16455da01cc02e6c4ecfc4152 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Thu, 26 Mar 2026 11:13:36 -0700 Subject: [PATCH] Gateway: require requester ownership for HTTP session kills (#55308) --- src/gateway/session-kill-http.test.ts | 35 ++++++++++++++++++--------- src/gateway/session-kill-http.ts | 27 +++------------------ 2 files changed, 26 insertions(+), 36 deletions(-) diff --git a/src/gateway/session-kill-http.test.ts b/src/gateway/session-kill-http.test.ts index a049b4c428e..31578538161 100644 --- a/src/gateway/session-kill-http.test.ts +++ b/src/gateway/session-kill-http.test.ts @@ -151,21 +151,20 @@ describe("POST /sessions/:sessionKey/kill", () => { await expect(response.json()).resolves.toEqual({ ok: true, killed: false }); }); - it("allows remote admin kills with an authorized bearer token", async () => { + it("rejects remote bearer-auth kills without requester ownership", async () => { isLocalDirectRequestMock.mockReturnValue(false); loadSessionEntryMock.mockReturnValue({ entry: { sessionId: "sess-worker", updatedAt: Date.now() }, canonicalKey: "agent:main:subagent:worker", }); - killSubagentRunAdminMock.mockResolvedValue({ found: true, killed: true }); const response = await post("/sessions/agent%3Amain%3Asubagent%3Aworker/kill"); - expect(response.status).toBe(200); - await expect(response.json()).resolves.toEqual({ ok: true, killed: true }); - expect(killSubagentRunAdminMock).toHaveBeenCalledWith({ - cfg, - sessionKey: "agent:main:subagent:worker", + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ + ok: false, + error: { type: "forbidden" }, }); + expect(killSubagentRunAdminMock).not.toHaveBeenCalled(); }); it("rejects remote kills without requester ownership or an authorized token", async () => { @@ -240,13 +239,17 @@ describe("POST /sessions/:sessionKey/kill", () => { }); }); - it("prefers admin kill when a valid bearer token is present alongside requester headers", async () => { + it("keeps bearer-auth requester kills on the requester-owned path", async () => { isLocalDirectRequestMock.mockReturnValue(false); loadSessionEntryMock.mockReturnValue({ entry: { sessionId: "sess-worker", updatedAt: Date.now() }, canonicalKey: "agent:main:subagent:worker", }); - killSubagentRunAdminMock.mockResolvedValue({ found: true, killed: true }); + getLatestSubagentRunByChildSessionKeyMock.mockReturnValue({ + runId: "run-1", + childSessionKey: "agent:main:subagent:worker", + }); + killControlledSubagentRunMock.mockResolvedValue({ status: "ok" }); const response = await post( "/sessions/agent%3Amain%3Asubagent%3Aworker/kill", @@ -255,10 +258,18 @@ describe("POST /sessions/:sessionKey/kill", () => { ); expect(response.status).toBe(200); await expect(response.json()).resolves.toEqual({ ok: true, killed: true }); - expect(killSubagentRunAdminMock).toHaveBeenCalledWith({ + expect(resolveSubagentControllerMock).toHaveBeenCalledWith({ cfg, - sessionKey: "agent:main:subagent:worker", + agentSessionKey: "agent:other:main", }); - expect(killControlledSubagentRunMock).not.toHaveBeenCalled(); + expect(killControlledSubagentRunMock).toHaveBeenCalledWith({ + cfg, + controller: { controllerSessionKey: "agent:main:main" }, + entry: expect.objectContaining({ + runId: "run-1", + childSessionKey: "agent:main:subagent:worker", + }), + }); + expect(killSubagentRunAdminMock).not.toHaveBeenCalled(); }); }); diff --git a/src/gateway/session-kill-http.ts b/src/gateway/session-kill-http.ts index 0d0b6e6ca70..5d272085d85 100644 --- a/src/gateway/session-kill-http.ts +++ b/src/gateway/session-kill-http.ts @@ -10,26 +10,10 @@ import type { AuthRateLimiter } from "./auth-rate-limit.js"; import { isLocalDirectRequest, type ResolvedGatewayAuth } from "./auth.js"; import { authorizeGatewayBearerRequestOrReply } from "./http-auth-helpers.js"; import { sendJson, sendMethodNotAllowed } from "./http-common.js"; -import { getBearerToken } from "./http-utils.js"; -import { ADMIN_SCOPE, WRITE_SCOPE, authorizeOperatorScopesForMethod } from "./method-scopes.js"; import { loadSessionEntry } from "./session-utils.js"; const REQUESTER_SESSION_KEY_HEADER = "x-openclaw-requester-session-key"; -function canBearerTokenKillSessions(token: string | undefined, authOk: boolean): boolean { - if (!token || !authOk) { - return false; - } - - // Authenticated HTTP bearer requests are operator-authenticated control-plane - // calls, so treat them as carrying the standard write/admin operator scopes. - const bearerScopes = [ADMIN_SCOPE, WRITE_SCOPE]; - return ( - authorizeOperatorScopesForMethod("sessions.delete", bearerScopes).allowed || - authorizeOperatorScopesForMethod("sessions.abort", bearerScopes).allowed - ); -} - function resolveSessionKeyFromPath(pathname: string): string | null { const match = pathname.match(/^\/sessions\/([^/]+)\/kill$/); if (!match) { @@ -65,7 +49,6 @@ export async function handleSessionKillHttpRequest( return true; } - const token = getBearerToken(req); const ok = await authorizeGatewayBearerRequestOrReply({ req, res, @@ -94,24 +77,20 @@ export async function handleSessionKillHttpRequest( const allowRealIpFallback = opts.allowRealIpFallback ?? cfg.gateway?.allowRealIpFallback; const requesterSessionKey = req.headers[REQUESTER_SESSION_KEY_HEADER]?.toString().trim(); const allowLocalAdminKill = isLocalDirectRequest(req, trustedProxies, allowRealIpFallback); - const allowBearerOperatorKill = canBearerTokenKillSessions(token, true); - if (!requesterSessionKey && !allowLocalAdminKill && !allowBearerOperatorKill) { + if (!requesterSessionKey && !allowLocalAdminKill) { sendJson(res, 403, { ok: false, error: { type: "forbidden", - message: - "Session kills require a local admin request, requester session ownership, or an authorized operator token.", + message: "Session kills require a local admin request or requester session ownership.", }, }); return true; } - const allowAdminKill = allowLocalAdminKill || allowBearerOperatorKill; - let killed = false; - if (!allowAdminKill && requesterSessionKey) { + if (!allowLocalAdminKill && requesterSessionKey) { const runEntry = getLatestSubagentRunByChildSessionKey(canonicalKey); if (runEntry) { const result = await killControlledSubagentRun({