mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-07 13:21:25 +00:00
fix(security): harden channel auth path checks and exec approval routing
This commit is contained in:
@@ -98,6 +98,10 @@ export const ExecApprovalRequestParamsSchema = Type.Object(
|
||||
agentId: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
resolvedPath: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
sessionKey: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
turnSourceChannel: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
turnSourceTo: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
turnSourceAccountId: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
turnSourceThreadId: Type.Optional(Type.Union([Type.String(), Type.Number(), Type.Null()])),
|
||||
timeoutMs: Type.Optional(Type.Integer({ minimum: 1 })),
|
||||
twoPhase: Type.Optional(Type.Boolean()),
|
||||
},
|
||||
|
||||
@@ -88,6 +88,19 @@ function isCanvasPath(pathname: string): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
function decodePathnameOnce(pathname: string): string {
|
||||
try {
|
||||
return decodeURIComponent(pathname);
|
||||
} catch {
|
||||
return pathname;
|
||||
}
|
||||
}
|
||||
|
||||
function isProtectedPluginChannelPath(pathname: string): boolean {
|
||||
const normalized = decodePathnameOnce(pathname).toLowerCase();
|
||||
return normalized === "/api/channels" || normalized.startsWith("/api/channels/");
|
||||
}
|
||||
|
||||
function isNodeWsClient(client: GatewayWsClient): boolean {
|
||||
if (client.connect.role === "node") {
|
||||
return true;
|
||||
@@ -493,7 +506,7 @@ export function createGatewayHttpServer(opts: {
|
||||
// Channel HTTP endpoints are gateway-auth protected by default.
|
||||
// Non-channel plugin routes remain plugin-owned and must enforce
|
||||
// their own auth when exposing sensitive functionality.
|
||||
if (requestPath === "/api/channels" || requestPath.startsWith("/api/channels/")) {
|
||||
if (isProtectedPluginChannelPath(requestPath)) {
|
||||
const token = getBearerToken(req);
|
||||
const authResult = await authorizeHttpGatewayConnect({
|
||||
auth: resolvedAuth,
|
||||
|
||||
@@ -52,6 +52,10 @@ export function createExecApprovalHandlers(
|
||||
agentId?: string;
|
||||
resolvedPath?: string;
|
||||
sessionKey?: string;
|
||||
turnSourceChannel?: string;
|
||||
turnSourceTo?: string;
|
||||
turnSourceAccountId?: string;
|
||||
turnSourceThreadId?: string | number;
|
||||
timeoutMs?: number;
|
||||
twoPhase?: boolean;
|
||||
};
|
||||
@@ -91,6 +95,12 @@ export function createExecApprovalHandlers(
|
||||
agentId: p.agentId ?? null,
|
||||
resolvedPath: p.resolvedPath ?? null,
|
||||
sessionKey: p.sessionKey ?? null,
|
||||
turnSourceChannel:
|
||||
typeof p.turnSourceChannel === "string" ? p.turnSourceChannel.trim() || null : null,
|
||||
turnSourceTo: typeof p.turnSourceTo === "string" ? p.turnSourceTo.trim() || null : null,
|
||||
turnSourceAccountId:
|
||||
typeof p.turnSourceAccountId === "string" ? p.turnSourceAccountId.trim() || null : null,
|
||||
turnSourceThreadId: p.turnSourceThreadId ?? null,
|
||||
};
|
||||
const record = manager.create(request, timeoutMs, explicitId);
|
||||
record.requestedByConnId = client?.connId ?? null;
|
||||
|
||||
@@ -493,6 +493,56 @@ describe("exec approval handlers", () => {
|
||||
expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined);
|
||||
});
|
||||
|
||||
it("forwards turn-source metadata to exec approval forwarding", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const manager = new ExecApprovalManager();
|
||||
const forwarder = {
|
||||
handleRequested: vi.fn(async () => false),
|
||||
handleResolved: vi.fn(async () => {}),
|
||||
stop: vi.fn(),
|
||||
};
|
||||
const handlers = createExecApprovalHandlers(manager, { forwarder });
|
||||
const respond = vi.fn();
|
||||
const context = {
|
||||
broadcast: (_event: string, _payload: unknown) => {},
|
||||
hasExecApprovalClients: () => false,
|
||||
};
|
||||
|
||||
const requestPromise = requestExecApproval({
|
||||
handlers,
|
||||
respond,
|
||||
context,
|
||||
params: {
|
||||
timeoutMs: 60_000,
|
||||
turnSourceChannel: "whatsapp",
|
||||
turnSourceTo: "+15555550123",
|
||||
turnSourceAccountId: "work",
|
||||
turnSourceThreadId: "1739201675.123",
|
||||
},
|
||||
});
|
||||
for (let idx = 0; idx < 20; idx += 1) {
|
||||
await Promise.resolve();
|
||||
}
|
||||
expect(forwarder.handleRequested).toHaveBeenCalledTimes(1);
|
||||
expect(forwarder.handleRequested).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
request: expect.objectContaining({
|
||||
turnSourceChannel: "whatsapp",
|
||||
turnSourceTo: "+15555550123",
|
||||
turnSourceAccountId: "work",
|
||||
turnSourceThreadId: "1739201675.123",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
await vi.runOnlyPendingTimersAsync();
|
||||
await requestPromise;
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("expires immediately when no approver clients and no forwarding targets", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
|
||||
@@ -242,6 +242,78 @@ describe("gateway plugin HTTP auth boundary", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("requires gateway auth for canonicalized /api/channels variants", async () => {
|
||||
const resolvedAuth: ResolvedGatewayAuth = {
|
||||
mode: "token",
|
||||
token: "test-token",
|
||||
password: undefined,
|
||||
allowTailscale: false,
|
||||
};
|
||||
|
||||
await withTempConfig({
|
||||
cfg: { gateway: { trustedProxies: [] } },
|
||||
prefix: "openclaw-plugin-http-auth-canonicalized-test-",
|
||||
run: async () => {
|
||||
const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => {
|
||||
const pathname = new URL(req.url ?? "/", "http://localhost").pathname;
|
||||
const canonicalPath = decodeURIComponent(pathname).toLowerCase();
|
||||
if (canonicalPath === "/api/channels/nostr/default/profile") {
|
||||
res.statusCode = 200;
|
||||
res.setHeader("Content-Type", "application/json; charset=utf-8");
|
||||
res.end(JSON.stringify({ ok: true, route: "channel-canonicalized" }));
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
});
|
||||
|
||||
const server = createGatewayHttpServer({
|
||||
canvasHost: null,
|
||||
clients: new Set(),
|
||||
controlUiEnabled: false,
|
||||
controlUiBasePath: "/__control__",
|
||||
openAiChatCompletionsEnabled: false,
|
||||
openResponsesEnabled: false,
|
||||
handleHooksRequest: async () => false,
|
||||
handlePluginRequest,
|
||||
resolvedAuth,
|
||||
});
|
||||
|
||||
const unauthenticatedCaseVariant = createResponse();
|
||||
await dispatchRequest(
|
||||
server,
|
||||
createRequest({ path: "/API/channels/nostr/default/profile" }),
|
||||
unauthenticatedCaseVariant.res,
|
||||
);
|
||||
expect(unauthenticatedCaseVariant.res.statusCode).toBe(401);
|
||||
expect(unauthenticatedCaseVariant.getBody()).toContain("Unauthorized");
|
||||
expect(handlePluginRequest).not.toHaveBeenCalled();
|
||||
|
||||
const unauthenticatedEncodedSlash = createResponse();
|
||||
await dispatchRequest(
|
||||
server,
|
||||
createRequest({ path: "/api/channels%2Fnostr%2Fdefault%2Fprofile" }),
|
||||
unauthenticatedEncodedSlash.res,
|
||||
);
|
||||
expect(unauthenticatedEncodedSlash.res.statusCode).toBe(401);
|
||||
expect(unauthenticatedEncodedSlash.getBody()).toContain("Unauthorized");
|
||||
expect(handlePluginRequest).not.toHaveBeenCalled();
|
||||
|
||||
const authenticatedCaseVariant = createResponse();
|
||||
await dispatchRequest(
|
||||
server,
|
||||
createRequest({
|
||||
path: "/API/channels/nostr/default/profile",
|
||||
authorization: "Bearer test-token",
|
||||
}),
|
||||
authenticatedCaseVariant.res,
|
||||
);
|
||||
expect(authenticatedCaseVariant.res.statusCode).toBe(200);
|
||||
expect(authenticatedCaseVariant.getBody()).toContain('"route":"channel-canonicalized"');
|
||||
expect(handlePluginRequest).toHaveBeenCalledTimes(1);
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
test.each(["0.0.0.0", "::"])(
|
||||
"returns 404 (not 500) for non-hook routes with hooks enabled and bindHost=%s",
|
||||
async (bindHost) => {
|
||||
|
||||
Reference in New Issue
Block a user