Gateway: require requester ownership for HTTP session kills (#55308)

This commit is contained in:
Jacob Tomlinson
2026-03-26 11:13:36 -07:00
committed by GitHub
parent f29c1206cd
commit 02cf12371f
2 changed files with 26 additions and 36 deletions

View File

@@ -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();
});
});

View File

@@ -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({