Skip to content

Commit ef0dbcf

Browse files
eleqtrizitdrobison00
eleqtrizit
and
drobison00
authored
Guard current browser tab exports (#75731)
* fix(browser): guard current tab exports * fix(browser): expand tab guard coverage * fix(browser): guard tab reads * fix(browser): guard screenshot route * changelog: PR #75731 --------- Co-authored-by: Devin Robison <[email protected]>
1 parent f2efe33 commit ef0dbcf

9 files changed

Lines changed: 230 additions & 10 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ Docs: https://docs.openclaw.ai
212212
- Security/Windows: pin Windows registry-probe `reg.exe` resolution to the canonical Windows install root in install-root probing, so `SystemRoot`/`WINDIR` env overrides cannot redirect registry queries during Windows host detection. (#74454) Thanks @mmaps.
213213
- QQBot: preserve the framework command authorization decision when converting framework command contexts into engine slash command contexts, so downstream slash handlers see `commandAuthorized` matching the channel's resolved `isAuthorizedSender` instead of a hardcoded `true`. (#77453) Thanks @drobison00.
214214
- Security/Windows: block `LOCALAPPDATA` from workspace `.env` and resolve Windows update-flow portable Git path prepends from the trusted process-local `LOCALAPPDATA` only, so workspace-supplied values cannot redirect `git` discovery during `openclaw update`. (#77470) Thanks @drobison00.
215+
- Browser/SSRF: enforce the existing current-tab URL navigation policy before tab-scoped debug, export, and read routes (console, page errors, network requests, trace start/stop, response body, screenshot, snapshot, storage, etc.) collect from an already-selected tab, so blocked tabs return a policy error instead of being read first and redacted only at response time. (#75731) Thanks @eleqtrizit.
215216

216217
## 2026.5.3-1
217218

‎extensions/browser/src/browser/routes/agent.act.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,7 @@ export function registerBrowserAgentActRoutes(
695695
res,
696696
ctx,
697697
targetId,
698+
enforceCurrentUrlAllowed: true,
698699
run: async ({ profileCtx, cdpUrl, tab, resolveTabUrl }) => {
699700
if (getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp) {
700701
return jsonError(res, 501, EXISTING_SESSION_LIMITS.responseBody);

‎extensions/browser/src/browser/routes/agent.debug.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export function registerBrowserAgentDebugRoutes(
2929
ctx,
3030
targetId,
3131
feature: "console messages",
32+
enforceCurrentUrlAllowed: true,
3233
run: async ({ cdpUrl, tab, pw, resolveTabUrl }) => {
3334
const messages = await pw.getConsoleMessagesViaPlaywright({
3435
cdpUrl,
@@ -54,6 +55,7 @@ export function registerBrowserAgentDebugRoutes(
5455
ctx,
5556
targetId,
5657
feature: "page errors",
58+
enforceCurrentUrlAllowed: true,
5759
run: async ({ cdpUrl, tab, pw, resolveTabUrl }) => {
5860
const result = await pw.getPageErrorsViaPlaywright({
5961
cdpUrl,
@@ -80,6 +82,7 @@ export function registerBrowserAgentDebugRoutes(
8082
ctx,
8183
targetId,
8284
feature: "network requests",
85+
enforceCurrentUrlAllowed: true,
8386
run: async ({ cdpUrl, tab, pw, resolveTabUrl }) => {
8487
const result = await pw.getNetworkRequestsViaPlaywright({
8588
cdpUrl,
@@ -109,6 +112,7 @@ export function registerBrowserAgentDebugRoutes(
109112
ctx,
110113
targetId,
111114
feature: "trace start",
115+
enforceCurrentUrlAllowed: true,
112116
run: async ({ cdpUrl, tab, pw, resolveTabUrl }) => {
113117
await pw.traceStartViaPlaywright({
114118
cdpUrl,
@@ -137,6 +141,7 @@ export function registerBrowserAgentDebugRoutes(
137141
ctx,
138142
targetId,
139143
feature: "trace stop",
144+
enforceCurrentUrlAllowed: true,
140145
run: async ({ cdpUrl, tab, pw, resolveTabUrl }) => {
141146
const id = crypto.randomUUID();
142147
const tracePath = await resolveWritableOutputPathOrRespond({

‎extensions/browser/src/browser/routes/agent.shared.test.ts‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1-
import { describe, expect, it } from "vitest";
1+
import { describe, expect, it, vi } from "vitest";
2+
import type { BrowserRouteContext, ProfileContext } from "../server-context.js";
23
import {
34
readBody,
45
resolveSafeRouteTabUrl,
56
resolveTargetIdFromBody,
67
resolveTargetIdFromQuery,
8+
withRouteTabContext,
79
} from "./agent.shared.js";
10+
import { createBrowserRouteResponse } from "./test-helpers.js";
811
import type { BrowserRequest } from "./types.js";
912

1013
function requestWithBody(body: unknown): BrowserRequest {
@@ -36,6 +39,31 @@ function profileContext(tabs: Array<{ targetId: string; url: string }>) {
3639
};
3740
}
3841

42+
function routeContextForTab(url: string): BrowserRouteContext {
43+
const profileCtx = {
44+
profile: {
45+
cdpUrl: "http://127.0.0.1:9222",
46+
name: "default",
47+
},
48+
ensureTabAvailable: vi.fn(async () => ({
49+
targetId: "tab-1",
50+
title: "Tab",
51+
url,
52+
type: "page",
53+
})),
54+
} as unknown as ProfileContext;
55+
56+
return {
57+
forProfile: () => profileCtx,
58+
state: () => ({
59+
resolved: {
60+
ssrfPolicy: {},
61+
},
62+
}),
63+
mapTabError: () => null,
64+
} as unknown as BrowserRouteContext;
65+
}
66+
3967
describe("browser route shared helpers", () => {
4068
describe("readBody", () => {
4169
it("returns object bodies", () => {
@@ -100,4 +128,44 @@ describe("browser route shared helpers", () => {
100128
).resolves.toBeUndefined();
101129
});
102130
});
131+
132+
describe("withRouteTabContext", () => {
133+
it("does not enforce current-tab URL policy unless requested", async () => {
134+
const response = createBrowserRouteResponse();
135+
const run = vi.fn(async () => {
136+
response.res.json({ ok: true });
137+
});
138+
139+
await withRouteTabContext({
140+
req: requestWithBody({}),
141+
res: response.res,
142+
ctx: routeContextForTab("http://127.0.0.1:8080/admin"),
143+
run,
144+
});
145+
146+
expect(run).toHaveBeenCalledOnce();
147+
expect(response.body).toEqual({ ok: true });
148+
});
149+
150+
it("blocks guarded routes before running on a disallowed current tab", async () => {
151+
const response = createBrowserRouteResponse();
152+
const run = vi.fn(async () => {
153+
response.res.json({ ok: true });
154+
});
155+
156+
await withRouteTabContext({
157+
req: requestWithBody({}),
158+
res: response.res,
159+
ctx: routeContextForTab("http://127.0.0.1:8080/admin"),
160+
enforceCurrentUrlAllowed: true,
161+
run,
162+
});
163+
164+
expect(run).not.toHaveBeenCalled();
165+
expect(response.statusCode).toBe(400);
166+
expect(response.body).toMatchObject({ error: expect.any(String) });
167+
const body = response.body as { error?: unknown };
168+
expect(body.error).not.toBe("");
169+
});
170+
});
103171
});

‎extensions/browser/src/browser/routes/agent.shared.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ type RouteWithTabParams<T> = {
107107
res: BrowserResponse;
108108
ctx: BrowserRouteContext;
109109
targetId?: string;
110+
/**
111+
* Set for routes that read from or return data scoped to the selected tab.
112+
* Leave false only for routes that navigate, activate, close, or otherwise manage the tab.
113+
*/
114+
enforceCurrentUrlAllowed?: boolean;
110115
run: (ctx: RouteTabContext) => Promise<T>;
111116
};
112117

@@ -119,6 +124,17 @@ export async function withRouteTabContext<T>(
119124
}
120125
try {
121126
const tab = await profileCtx.ensureTabAvailable(params.targetId);
127+
if (params.enforceCurrentUrlAllowed) {
128+
await assertBrowserNavigationResultAllowed({
129+
url: tab.url,
130+
...withBrowserNavigationPolicy(params.ctx.state().resolved.ssrfPolicy, {
131+
browserProxyMode: resolveBrowserNavigationProxyMode({
132+
resolved: params.ctx.state().resolved,
133+
profile: profileCtx.profile,
134+
}),
135+
}),
136+
});
137+
}
122138
return await params.run({
123139
profileCtx,
124140
tab,
@@ -137,6 +153,10 @@ export async function withRouteTabContext<T>(
137153
}
138154
}
139155

156+
/**
157+
* Response-only URL redaction. This swallows policy failures and must not be used as
158+
* an execution gate; use enforceCurrentUrlAllowed on the route helper instead.
159+
*/
140160
export async function resolveSafeRouteTabUrl(params: {
141161
ctx: BrowserRouteContext;
142162
profileCtx: ProfileContext;
@@ -171,6 +191,11 @@ type RouteWithPwParams<T> = {
171191
ctx: BrowserRouteContext;
172192
targetId?: string;
173193
feature: string;
194+
/**
195+
* Set for routes that read from or return data scoped to the selected tab.
196+
* Leave false only for routes that navigate, activate, close, or otherwise manage the tab.
197+
*/
198+
enforceCurrentUrlAllowed?: boolean;
174199
run: (ctx: RouteTabPwContext) => Promise<T>;
175200
};
176201

@@ -182,6 +207,7 @@ export async function withPlaywrightRouteContext<T>(
182207
res: params.res,
183208
ctx: params.ctx,
184209
targetId: params.targetId,
210+
enforceCurrentUrlAllowed: params.enforceCurrentUrlAllowed,
185211
run: async ({ profileCtx, tab, cdpUrl, resolveTabUrl }) => {
186212
const pw = await requirePwAi(params.res, params.feature);
187213
if (!pw) {

‎extensions/browser/src/browser/routes/agent.snapshot.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ export function registerBrowserAgentSnapshotRoutes(
318318
ctx,
319319
targetId,
320320
feature: "pdf",
321+
enforceCurrentUrlAllowed: true,
321322
run: async ({ cdpUrl, tab, pw }) => {
322323
const pdf = await pw.pdfViaPlaywright({
323324
cdpUrl,
@@ -361,18 +362,12 @@ export function registerBrowserAgentSnapshotRoutes(
361362
res,
362363
ctx,
363364
targetId,
365+
enforceCurrentUrlAllowed: true,
364366
run: async ({ profileCtx, tab, cdpUrl }) => {
365367
if (getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp) {
366-
const ssrfPolicyOpts = browserNavigationPolicyForProfile(ctx, profileCtx);
367368
if (element) {
368369
return jsonError(res, 400, EXISTING_SESSION_LIMITS.snapshot.screenshotElement);
369370
}
370-
if (ssrfPolicyOpts.ssrfPolicy) {
371-
await assertBrowserNavigationResultAllowed({
372-
url: tab.url,
373-
...ssrfPolicyOpts,
374-
});
375-
}
376371
if (labels) {
377372
const snapshot = await takeChromeMcpSnapshot({
378373
profileName: profileCtx.profile.name,

‎extensions/browser/src/browser/routes/agent.storage.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export function registerBrowserAgentStorageRoutes(
8585
ctx,
8686
targetId,
8787
feature: "cookies",
88+
enforceCurrentUrlAllowed: true,
8889
run: async ({ cdpUrl, tab, pw }) => {
8990
const result = await pw.cookiesGetViaPlaywright({
9091
cdpUrl,
@@ -109,6 +110,7 @@ export function registerBrowserAgentStorageRoutes(
109110
return jsonError(res, 400, "cookie is required");
110111
}
111112

113+
// Intentional: mutation routes are outside the tab-scoped read/export guard scope.
112114
await withPlaywrightRouteContext({
113115
req,
114116
res,
@@ -148,6 +150,7 @@ export function registerBrowserAgentStorageRoutes(
148150
const body = readBody(req);
149151
const targetId = resolveTargetIdFromBody(body);
150152

153+
// Intentional: mutation routes are outside the tab-scoped read/export guard scope.
151154
await withPlaywrightRouteContext({
152155
req,
153156
res,
@@ -181,6 +184,7 @@ export function registerBrowserAgentStorageRoutes(
181184
ctx,
182185
targetId,
183186
feature: "storage get",
187+
enforceCurrentUrlAllowed: true,
184188
run: async ({ cdpUrl, tab, pw }) => {
185189
const result = await pw.storageGetViaPlaywright({
186190
cdpUrl,
@@ -207,6 +211,7 @@ export function registerBrowserAgentStorageRoutes(
207211
}
208212
const value = typeof mutation.body.value === "string" ? mutation.body.value : "";
209213

214+
// Intentional: mutation routes are outside the tab-scoped read/export guard scope.
210215
await withPlaywrightRouteContext({
211216
req,
212217
res,
@@ -235,6 +240,7 @@ export function registerBrowserAgentStorageRoutes(
235240
return;
236241
}
237242

243+
// Intentional: mutation routes are outside the tab-scoped read/export guard scope.
238244
await withPlaywrightRouteContext({
239245
req,
240246
res,
@@ -263,6 +269,7 @@ export function registerBrowserAgentStorageRoutes(
263269
return jsonError(res, 400, "offline is required");
264270
}
265271

272+
// Intentional: mutation routes are outside the tab-scoped read/export guard scope.
266273
await withPlaywrightRouteContext({
267274
req,
268275
res,
@@ -301,6 +308,7 @@ export function registerBrowserAgentStorageRoutes(
301308
}
302309
}
303310

311+
// Intentional: mutation routes are outside the tab-scoped read/export guard scope.
304312
await withPlaywrightRouteContext({
305313
req,
306314
res,

0 commit comments

Comments
 (0)