fix(node-pairing): require operator.admin to approve browser.proxy nodes by yetval · Pull Request #104491 · openclaw/openclaw

yetval · 2026-07-11T13:30:57Z

What Problem This Solves

Approving a node whose pairing surface advertises the browser.proxy capability only requires
operator.write, even though directly invoking browser.proxy requires operator.admin. A
lower-privileged operator.write operator can therefore approve (trust) a browser.proxy-capable
node. Because the bundled browser tool auto-routes its traffic to a trusted paired node, that
write-scoped approval is the one-time trust decision that lets a node become the target for
admin-level browser traffic (page content, cookies, credentials).

Root cause: resolveNodePairApprovalScopes bumps the required approval scope to operator.admin
only when a declared command is in NODE_SYSTEM_RUN_COMMANDS. browser.proxy
(NODE_BROWSER_PROXY_COMMAND) is not in that set, so a browser.proxy surface falls into the
generic branch and returns [operator.pairing, operator.write].

src/infra/node-pairing-authz.ts (before):

import { NODE_SYSTEM_RUN_COMMANDS } from "./node-commands.js";
...
if (
  normalized.some((command) => NODE_SYSTEM_RUN_COMMANDS.some((allowed) => allowed === command))
) {
  return [OPERATOR_PAIRING_SCOPE, OPERATOR_ADMIN_SCOPE];
}

The invoke path already gates browser.proxy at operator.admin
(src/gateway/server-methods/nodes.ts):

if (command === "browser.proxy" && !clientHasOperatorAdminScope(client)) {
  respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${BROWSER_PROXY_REQUIRED_SCOPE}`));
  return;
}

PR #85916 added that invoke-time admin gate but never updated the sibling approval-scope
resolver, leaving the two gates inconsistent.

Why This Change Was Made

Add browser.proxy to the command set that triggers the operator.admin approval bump, so
approval is at least as strict as invocation.

src/infra/node-pairing-authz.ts (after):

import { NODE_BROWSER_PROXY_COMMAND, NODE_SYSTEM_RUN_COMMANDS } from "./node-commands.js";
...
const ADMIN_APPROVAL_COMMANDS = [...NODE_SYSTEM_RUN_COMMANDS, NODE_BROWSER_PROXY_COMMAND];
...
if (
  normalized.some((command) => ADMIN_APPROVAL_COMMANDS.some((allowed) => allowed === command))
) {
  return [OPERATOR_PAIRING_SCOPE, OPERATOR_ADMIN_SCOPE];
}

Both constants already live in src/infra/node-commands.ts and are reused, not re-declared.

resolveNodePairApprovalScopes is the single owner of the approval-scope mapping; both callers
(approveNodePairing in src/infra/node-pairing.ts and node.pair.approve in
src/gateway/server-methods/nodes.ts) route their scope check through it, so one change covers
every approval path. The only command gated at operator.admin on the invoke side is
browser.proxy; system.run.* was already covered, so after this change the approval-scope
admin set is at least as strict as the invoke-time admin gate for every command.

Note on the internal browser dispatch: the gateway browser.request method
(extensions/browser/src/gateway/browser-request.ts) is registered with scope: operator.admin
(extensions/browser/src/browser-gateway-contract.ts) and that scope is enforced at
authorizeGatewayMethod before the handler runs, so its direct nodeRegistry.invoke call is
not an authorization bypass and is intentionally out of scope here. This change only corrects the
approval-scope asymmetry.

User Impact

A non-admin operator.write operator can no longer approve a node advertising browser.proxy;
that approval now requires operator.admin, matching the scope already required to invoke
browser.proxy. Existing admin approvals and all non-browser, non-exec surface approvals are
unaffected.

Evidence

node scripts/run-vitest.mjs src/infra/node-pairing-authz.test.ts - 5 passed. The new
assertion fails on pristine main (returns [operator.pairing, operator.write]) and passes
with the patch.
node scripts/run-oxlint.mjs and oxfmt --check on both changed files - clean.
run-tsgo.mjs -p tsconfig.core.json and -p test/tsconfig/tsconfig.core.test.json - clean.

Real behavior proof

Behavior addressed: an operator.write (non-admin) operator can approve a node advertising the browser.proxy capability, whereas invoking browser.proxy requires operator.admin; the fix makes approval require operator.admin too.
Real environment tested: drove the real approveNodePairing reducer through its on-disk paired-device store in a fresh temp base directory on pristine main e681646 and on the patched tree; the real requestDevicePairing, approveDevicePairing, requestNodePairing, approveNodePairing, and resolveMissingRequestedScope all ran against the real store, with nothing stubbed (the store was seeded through the real pairing request/approve functions).
Exact steps or command run after this patch: seed a paired node device, request a node surface declaring commands ["browser.proxy"], then call approveNodePairing(requestId, { callerScopes: ["operator.pairing", "operator.write"] }, baseDir) and record the returned outcome.
Evidence after fix:

# BEFORE (pristine main e681646834)
caller scopes: [operator.pairing, operator.write]
surface commands: [browser.proxy]
approveNodePairing -> APPROVED nodeId=node-1 commands=["browser.proxy"]
# AFTER (this patch, identical inputs)
caller scopes: [operator.pairing, operator.write]
surface commands: [browser.proxy]
approveNodePairing -> FORBIDDEN missingScope=operator.admin

Observed result after fix: the same write-scoped approval that previously trusted the browser.proxy node is now refused with missingScope operator.admin, matching the invoke-time gate.
What was not tested: no live gateway RPC round trip or real remote node; the full build was not run.

Approving a node whose pairing surface advertises browser.proxy only required operator.write, while invoking browser.proxy already requires operator.admin (server-methods/nodes.ts). resolveNodePairApprovalScopes bumped the approval scope to operator.admin only for NODE_SYSTEM_RUN_COMMANDS, so a write-scoped operator could trust a browser.proxy-capable node that later routes bundled browser-tool traffic. Add NODE_BROWSER_PROXY_COMMAND to the admin-approval set so approval scope matches the invoke-time gate.

clawsweeper · 2026-07-11T13:40:49Z

Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 9:39 AM ET / 13:39 UTC.

Summary
Require operator.admin when approving node surfaces that declare browser.proxy, with focused resolver regression coverage.

PR surface: Source +2, Tests +7. Total +9 across 2 files.

Reproducibility: yes. Current main maps browser.proxy approval to operator.write while the invocation path requires operator.admin, and the PR supplies direct before/after output through the real pairing reducers and store.

Review metrics: 1 noteworthy metric.

Approval policy promotion: 1 command moved from write-scoped to admin-scoped approval. The change intentionally prevents an existing operator role from completing a trust decision for a sensitive browser surface.

Root-cause cluster
Relationship: canonical
Canonical: #104491
Summary: This PR is the canonical remaining fix for the approval-side half of the browser-proxy authorization mismatch.

Members:

same_root_cause: fix(gateway): require admin scope for browser proxy invoke #85916 - The merged PR made direct browser.proxy invocation admin-only but did not align node-surface approval authorization.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Risk before merge

[P1] After merge, write-only operators can no longer approve nodes advertising browser.proxy; they must use an admin-scoped operator. This is intentional security hardening but changes an operator workflow that the latest release currently permits.

Maintainer options:

Land the stricter trust boundary (recommended)
Accept that write-only operators can no longer approve browser-proxy nodes because approval should match the admin-level data access that trusted nodes can receive.
Pause for an upgrade transition
Hold the PR only if maintainers require a documented transition path for existing write-scoped approval workflows before enforcing admin approval.

Next step before merge

No automated repair is needed; the patch is correct and should proceed through ordinary exact-head review and merge handling with the operator compatibility impact visible.

Security
Cleared: The focused diff tightens an existing authorization boundary and introduces no dependency, secret, workflow, downloaded-artifact, permission-expansion, or supply-chain changes.

Review details

Best possible solution:

Land the focused resolver change while explicitly accepting the write-to-admin approval upgrade impact, keeping the shared browser command constant as the command identity and the existing resolver as the single approval-policy owner.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main maps browser.proxy approval to operator.write while the invocation path requires operator.admin, and the PR supplies direct before/after output through the real pairing reducers and store.

Is this the best way to solve the issue?

Yes. Updating the existing approval-scope resolver with the shared browser command constant is the narrowest maintainable fix and covers both approval entry points without adding another authorization path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against fa77fe10d526.

Label changes

Label changes:

add P0: The PR closes a privilege-boundary gap that lets a lower-scoped operator influence which node is trusted to receive admin-level browser content, cookies, and credentials.
add merge-risk: 🚨 compatibility: Upgrading with this hardening means existing write-only operators can no longer approve nodes advertising browser.proxy and must use an admin-scoped operator.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body shows identical inputs against the real pairing reducers and on-disk store before and after the patch, changing approval from success to missingScope=operator.admin.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body shows identical inputs against the real pairing reducers and on-disk store before and after the patch, changing approval from success to missingScope=operator.admin.

Label justifications:

P0: The PR closes a privilege-boundary gap that lets a lower-scoped operator influence which node is trusted to receive admin-level browser content, cookies, and credentials.
merge-risk: 🚨 compatibility: Upgrading with this hardening means existing write-only operators can no longer approve nodes advertising browser.proxy and must use an admin-scoped operator.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body shows identical inputs against the real pairing reducers and on-disk store before and after the patch, changing approval from success to missingScope=operator.admin.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body shows identical inputs against the real pairing reducers and on-disk store before and after the patch, changing approval from success to missingScope=operator.admin.

Evidence reviewed

PR surface:

Source +2, Tests +7. Total +9 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	4	2	+2
Tests	1	7	0	+7
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	11	2	+9

What I checked:

Current-main mismatch: The approval resolver only promotes NODE_SYSTEM_RUN_COMMANDS to operator.admin; a declared browser.proxy command therefore falls through to operator.write. (src/infra/node-pairing-authz.ts:17, fa77fe10d526)
Sibling invocation boundary: The current node invocation handler separately rejects browser.proxy unless the caller has operator.admin, confirming the inconsistent authorization levels. (src/gateway/server-methods/nodes.ts:1266, fa77fe10d526)
Complete approval coverage: Both the public pending-request scope display and the actual approval decision call resolveNodePairApprovalScopes, so changing this resolver covers the shared approval boundary. (src/infra/node-pairing.ts:511, fa77fe10d526)
Focused patch: The PR adds the shared NODE_BROWSER_PROXY_COMMAND constant to the admin-approval command set and adds a direct regression assertion for the resulting scopes. (src/infra/node-pairing-authz.ts:11, c3fd3f500e8f)
Real behavior proof: The PR body reports identical real pairing-reducer and on-disk-store inputs: pristine main approves the write-scoped request, while the patched tree returns missingScope=operator.admin. (c3fd3f500e8f)
Root-cause provenance: Merged PR fix(gateway): require admin scope for browser proxy invoke #85916 introduced the operator.admin invocation restriction but did not modify the approval resolver, leaving this approval-side remainder. (src/gateway/server-methods/nodes.ts:1266, 456c48f368cb)

Likely related people:

steipete: Authored the end-to-end pairing-scope enforcement and later extracted the shared approval resolver now changed by this PR. (role: recent approval-path contributor; confidence: high; commits: ca2fdcc45f36, 01a24c20bfb7; files: src/infra/node-pairing.ts, src/infra/node-pairing-authz.ts)
LaPhilosophie: Authored the merged change that established the operator.admin requirement for direct browser.proxy invocation. (role: introduced sibling invoke restriction; confidence: high; commits: 456c48f368cb; files: src/gateway/server-methods/nodes.ts)
Jacob Tomlinson: Authored earlier gateway work that restricted node pairing approvals and is relevant to the history of this authorization boundary. (role: introduced pairing approval restrictions; confidence: medium; commits: 4d7cc6bb4fac; files: src/infra/node-pairing.ts, src/gateway/server-methods/nodes.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-11T16:58:05Z

Merged via squash.

Prepared head SHA: c3fd3f500e8fa6e72021eabed1ad7d4d83a89ee2
Landed commit: 68c18caca06cebdc8eee55b1a73b03e996bc4764

…des (#104491) Approving a node whose pairing surface advertises browser.proxy only required operator.write, while invoking browser.proxy already requires operator.admin (server-methods/nodes.ts). resolveNodePairApprovalScopes bumped the approval scope to operator.admin only for NODE_SYSTEM_RUN_COMMANDS, so a write-scoped operator could trust a browser.proxy-capable node that later routes bundled browser-tool traffic. Add NODE_BROWSER_PROXY_COMMAND to the admin-approval set so approval scope matches the invoke-time gate. (cherry picked from commit 68c18ca) (cherry picked from commit f902110)

…des (openclaw#104491) Approving a node whose pairing surface advertises browser.proxy only required operator.write, while invoking browser.proxy already requires operator.admin (server-methods/nodes.ts). resolveNodePairApprovalScopes bumped the approval scope to operator.admin only for NODE_SYSTEM_RUN_COMMANDS, so a write-scoped operator could trust a browser.proxy-capable node that later routes bundled browser-tool traffic. Add NODE_BROWSER_PROXY_COMMAND to the admin-approval set so approval scope matches the invoke-time gate.

openclaw-barnacle Bot added the size: XS label Jul 11, 2026

yetval marked this pull request as ready for review July 11, 2026 13:31

steipete self-assigned this Jul 11, 2026

steipete merged commit 68c18ca into openclaw:main Jul 11, 2026
196 of 203 checks passed

github-actions Bot mentioned this pull request Jul 11, 2026

📡 Upstream Digest — 2026-07-11 18:48 UTC curtismercier/openclaw-mods#1264

Open

steipete mentioned this pull request Jul 11, 2026

fix(release/2026.7.1): low-risk stable sweep — pairing authz, UTF-16 chunk safety, channel timeouts #104684

Merged

yetval mentioned this pull request Jul 13, 2026

gateway: fs.listDir node pairing can be approved without operator.admin #105936

Closed

yangxiansheng mentioned this pull request Jul 13, 2026

fix(node-pairing): require operator.admin to approve fs.listDir nodes (#105936) #106004

Merged

nikodim-ai mentioned this pull request Jul 16, 2026

[Bug]: nodes invoke browser.proxy requests operator.write but gateway requires operator.admin #108951

Open

clawsweeper Bot mentioned this pull request Jul 16, 2026

fix(gateway): request admin scope for admin-only node.invoke commands #109198

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(node-pairing): require operator.admin to approve browser.proxy nodes#104491

fix(node-pairing): require operator.admin to approve browser.proxy nodes#104491
steipete merged 1 commit into
openclaw:mainfrom
yetval:fix/node-pairing-browser-proxy-admin-scope

yetval commented Jul 11, 2026

Uh oh!

clawsweeper Bot commented Jul 11, 2026

Uh oh!

Uh oh!

steipete commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

yetval commented Jul 11, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Real behavior proof

Uh oh!

clawsweeper Bot commented Jul 11, 2026

Uh oh!

Uh oh!

steipete commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants