feat(gateway): restrict session tools by authenticated hosting node#93411
feat(gateway): restrict session tools by authenticated hosting node#93411anagnorisis2peripeteia wants to merge 4 commits into
Conversation
Add gateway.tools.byNode: a per-node tool restriction keyed off the AUTHENTICATED node hosting a turn. When a turn is dispatched from a node-originated agent.request, the gateway records the originating nodeId (from the node's cryptographic device pairing — a client cannot forge it) against the session key, and tool resolution narrows the toolset to that node's policy. The restriction is enforced in BOTH tool-resolution paths via a shared resolveNodeScopedToolPolicy helper: the gateway scoped resolver (resolveGatewayScopedTools, for MCP/HTTP callers) and the in-process agent tool builder (createOpenClawCodingTools), which a node-originated turn actually runs through. Restriction-only: it can narrow the toolset, never escalate; an explicitly-present (even empty) allow is fail-closed; spawned subagents inherit the narrowed surface. Tests: gateway resolver byNode (allow intersect, no-escalate, fail-closed empty allow, deny, no-op cases), embedded-builder byNode (same matrix), and config round-trip (schema accept/reject).
|
Codex review: found issues before merge. Reviewed June 30, 2026, 11:44 PM ET / 03:44 UTC. Summary PR surface: Source +136, Tests +150, Docs +19. Total +305 across 15 files. Reproducibility: yes. for the review finding: PR head adds two Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Add the missing config labels, rebase or resolve the conflicts, and then have maintainers explicitly accept the fail-closed authenticated-node tool boundary before landing this primitive ahead of the stacked browser-routing work. Do we have a high-confidence way to reproduce the issue? Yes for the review finding: PR head adds two Is this the best way to solve the issue? No, not merge-ready as-is. The run-scoped authenticated-node design is the right direction compared with the closed client-id approach, but the metadata repair, branch refresh, and explicit owner acceptance are still needed. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 21d1e1f0fc9d. Label changesLabel justifications:
Evidence reviewedPR surface: Source +136, Tests +150, Docs +19. Total +305 across 15 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
The merge-base extraction pulled the browser extension-driver schema reintroduction into this PR; it belongs to the (separate) extension work, not the byNode primitive. Revert that hunk so the PR is purely the gateway.tools.byNode addition. (ClawSweeper openclaw#93411 P2.)
Address ClawSweeper openclaw#93411 P1: the session-global sessionKey->nodeId map could apply a node's policy to a later/concurrent turn for the same session. Replace it with a run-scoped hosting node id threaded through the run: server-node-events passes hostingNodeId on the node-originated agent.request dispatch -> ingress opts -> runEmbeddedAgent -> attempt params -> createOpenClawCodingTools, which resolves gateway.tools.byNode from that explicit id. resolveNodeScopedToolPolicy now takes the node id directly (no session lookup); the MCP/HTTP scoped resolver passes none (not node-originated). No cross-turn bleed.
Replace the session-global sessionKey->nodeId map with a run-scoped hosting node id threaded through the run, so a node's policy can never bleed onto a later or concurrent turn for the same session. - node-tool-policy.ts (renamed from session-node-id-registry.ts): resolveNode ScopedToolPolicy now takes the authenticated node id directly; the map + set/get are gone. - server-node-events passes hostingNodeId on the node-originated agent.request dispatch -> AgentCommandOpts -> runEmbeddedAgent (RunEmbeddedAgentParams) -> EmbeddedRunAttemptParams -> createOpenClawCodingTools, which resolves byNode from that explicit id. - The MCP/HTTP scoped resolver (resolveGatewayScopedTools) passes no hosting node (not node-originated) -> byNode no-op there. byNode is enforced in the embedded agent tool builder, the path a node-originated turn actually runs through. - Tests: drop the resolver byNode test; add a run-scoped node-tool-policy unit test; the embedded-builder byNode tests now pass hostingNodeId.
|
Updated for ClawSweeper's review:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review (the prior re-review run failed waiting for Codex capacity, not on the diff — retrying) |
|
@clawsweeper re-review |
|
@clawsweeper re-review |
Add config-help entries (schema.help.ts) for gateway.tools.byNode and its per-node allow/deny, and a Gateway tool policy section in the configuration reference: restriction-only, authenticated-node keyed (unforgeable), fail-closed empty allow, nodeId discovered via /whoami or nodes list.
|
Addressed the re-review:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
…t hook (private seam) The interim F3 fix (b3e840c) made node-gateway-events-internal a real exported plugin-SDK entrypoint to clear the runtime ERR_PACKAGE_PATH_NOT_EXPORTED, but that re-grew the public SDK surface (the plugin-sdk surface/api budgets correctly flagged it) -- the exact thing F3 was about. Replace it with a true private seam: the node-host injects its emitNodeGatewayEvent into the bridge in-process via the onNodeHostStart hook context (new extensions/browser/.../node-gateway-emitter.ts registry). server-lifecycle emits through the registry; the node-host passes the real emitter when it runs the startup hook. emitNodeGatewayEvent is never on the public plugin-SDK surface -- a third-party plugin cannot import it or forge node-attributed events -- and it still throws without a registered node (gateway-only fallback). Removed the node-gateway-events-internal subpath + entrypoint; re-synced package exports. Verified live on a clean install: relay binds on the node-host eager-start with NO kick, the bundled extension reconnects + attaches, and a side-panel turn routes through the node (gateway log node=ffc1b7d3...). Build clean. (api/surface baselines now reflect only the intended removal of emitNodeGatewayEvent from the public gateway-runtime barrel plus pre-existing openclaw#93411 stacked-base deprecated-export drift, which the rebase drops.)
|
Closing after #100619 merged. Its remote browser-node support (the gateway proxies browser actions to node-hosted Chrome over the existing authenticated node link) removes this primitive's consuming feature: the #93680 side-panel copilot no longer needs node-anchored turn routing, so the byNode confinement boundary would ship with no consumer. Rather than ask maintainers to accept a new public fail-closed security surface nothing uses, I'm closing it. The branch and review history stay available if node-scoped tool confinement finds a real consumer later. #93680 is being rebased onto the merged relay per the coordination note there. |
Summary
gateway.tools.byNode— a per-node tool restriction keyed off the authenticated node hosting a turn, enforced run-scoped.agent.request, the gateway threads that node's id (from its cryptographic device pairing — unforgeable) through the run ashostingNodeId; the in-process agent tool builder resolves the node'sallow/denyfrom it and narrows that turn's toolset.allow, spawned subagents inherit the narrowed surface. The id is carried with the run (not a session-global map), so a node's policy cannot bleed onto a later/concurrent turn for the same session.node-tool-policy.ts, thehostingNodeIdthreading (server-node-events → ingress opts → runEmbeddedAgent → attempt → createOpenClawCodingTools), and the fail-closed/no-escalate semantics.Linked context
Related #93347 (the earlier
byClientIdapproach, closed —client.idis self-declared/forgeable; this replaces it with the authenticated node identity).Requested via ClawSweeper's review of #93347 (gate the toolset off the authenticated node, not a self-declared name).
Real behavior proof (required for external PRs)
gateway.tools.byNodeconfining a node-originated agent turn's tools to the authenticated hosting node's policy, resolved run-scoped.openclaw node runas two processes, loopback device pairing auto-approved viagateway.nodes.pairing.autoApproveCidrs.openclaw gateway run --dev --auth none --port 18789withgateway.nodes.pairing.autoApproveCidrs: ["127.0.0.1/32"].gateway.tools.byNode["<node-id>"] = { allow: ["web_search", "image"] }.openclaw node run --host 127.0.0.1 --port 18789→ the node device-pairs and is auto-approved (gateway logsdevice pairing auto-approved device=06f4ec…e71a7c role=node).agent.requestover its authenticated connection (the same emitter path the browser bridge uses in production).[BYNODE_DEBUG] session=agent:dev:box-rs allow=[web_search,image] before=5{tts,web_search,web_fetch,image,canvas} after=2{web_search,image}byNodepolicy allows, resolved run-scoped from the authenticatedhostingNodeIdthreaded through the run. Changing the allow list changes the surviving set; an emptyallowyields zero tools (fail-closed); a non-node turn keeps all tools.[BYNODE_DEBUG]line and the one-shot nodeagent.requestare env-gated box-only instrumentation (not part of this PR) used to trigger and observe the otherwise-internal node-originated path. A combined GUI recording was blocked by available disk (no room for a VM image).gateway.tools.byNodeentry for the node, the same node-originated turn keeps all 5 tools (no restriction); withallow: ["browser","memory_search","memory_get"](none present in that dev agent's set) the turn resolves to 0 tools — confirming fail-closed.Tests and validation
pnpm build: clean.node-tool-policyunit tests — allow/deny resolution, fail-closed empty allow, deny-only, no-op when no hosting node, no-op when node unconfigured.byNodetests (in thecreateOpenClawCodingToolssuite) — assert the resolved toolset narrows via the run-scopedhostingNodeId.byNode, rejects an unknown inner key.Risk checklist
gateway.tools.byNodeis set and a turn is node-originated).gateway.tools.byNodeconfig surface (TS type, Zod schema, config help, and gateway docs added).Current review state
main(a rebase/CI-fixture refresh may be wanted at merge time — the flagged CI failure is in unrelated tooling, not the touched files).hostingNodeId; ClawSweeper confirmed the session-global bug is gone, patch quality 🦐); P2byNodedocumentation (config help + gateway docs added); the straydriver:"extension"schema change was reverted. This update adds the required real-behavior-proof fields above.