fix: gate owner-only HTTP tools by pgondhi987 · Pull Request #90261 · openclaw/openclaw

pgondhi987 · 2026-06-04T08:53:54Z

Summary

What problem does this PR solve?

Restores sender-owner gating for owner-only core tools exposed through Gateway POST /tools/invoke and the shared tools.invoke path.
Keeps gateway.tools.allow as an owner/trusted-operator override for the default HTTP deny list, but prevents non-owner identity-bearing callers from receiving server-credential gateway, cron, or nodes wrappers.
Leaves shared-secret bearer and operator.admin behavior unchanged: trusted operator/admin callers can still use explicitly enabled owner-only tools.

Why does this matter now?

The tool invocation path already threads sender-owner identity; this makes that signal effective before tool execution and before spawned-session inherited tool policy.

What is the intended outcome?

A write-scoped non-owner HTTP/RPC caller sees owner-only core tools as unavailable even if the gateway tool allowlist includes them.
Admin/owner and shared-secret trusted-operator callers can still use explicitly enabled owner-only tools.

What is intentionally out of scope?

No config shape, migration, provider, plugin, channel, storage, or dependency changes.
No per-method scope propagation inside the gateway/cron/nodes wrappers; this PR gates the affected server-credential wrappers as owner-only at the shared resolver boundary.

What does success look like?

Non-owner HTTP callers get 404 not_found for owner-only core tools.
Non-admin RPC callers get not_found for the same owner-only core tools.
Owner/shared-secret/admin callers keep the existing explicitly enabled behavior.

What should reviewers focus on?

src/gateway/tool-resolution.ts applies the owner-only deny after normal policy allow resolution so gateway.tools.allow cannot reopen these wrappers to non-owner callers.
src/gateway/server-methods/tools-invoke.ts passes RPC admin scope into the shared invocation path so admin RPC clients remain owner-capable.
src/security/dangerous-tools.ts keeps the HTTP default deny and owner-only lists centralized.
src/gateway/tools-invoke-http.test.ts covers non-owner denial, admin/shared-secret owner behavior, and spawned-session deny inheritance.

AI-assisted: yes.

Linked context

Which issue does this close?

No public issue is linked.

Which issues, PRs, or discussions are related?

Maintainer-requested security hardening for the Gateway tools boundary.

Was this requested by a maintainer or owner?

Yes, requested through the maintainer tracking workflow.

Real behavior proof (required for external PRs)

Behavior addressed: Write-scoped non-owner callers can no longer expose owner-only core tools through gateway.tools.allow on Gateway tool invocation surfaces.
Real environment tested: Local source checkout running a real foreground openclaw gateway run on loopback with a temporary OPENCLAW_HOME and OPENCLAW_CONFIG_PATH; channels/providers skipped.
Exact steps or command run after this patch: Start gateway with OPENCLAW_SKIP_CHANNELS=1 OPENCLAW_SKIP_PROVIDERS=1 pnpm openclaw gateway run --allow-unconfigured --auth none --bind loopback --port 26779, using a temp config with gateway.auth.mode="none", gateway.tools.allow=["gateway","cron","nodes"], and main-agent tools.allow=["gateway","cron","nodes"]; then call POST /tools/invoke with curl using x-openclaw-scopes: operator.write for each owner-only tool and x-openclaw-scopes: operator.admin for the owner control.
Evidence after fix: Real gateway curl results were nonowner-gateway http=404 {"ok":false,"errorType":"not_found"}, nonowner-cron http=404 {"ok":false,"errorType":"not_found"}, nonowner-nodes http=404 {"ok":false,"errorType":"not_found"}, and admin-gateway http=200 {"ok":true,"resultKeys":["content","details"]}.
Observed result after fix: The real loopback gateway denied gateway, cron, and nodes to a write-scoped non-owner caller despite explicit allowlists, while an admin-scope caller still reached the explicitly allowed gateway tool.
What was not tested: External trusted-proxy deployment, browser/UI, channel, plugin, provider, migration, or storage paths.
Proof limitations or environment constraints: The real proof used local auth.mode="none" identity-bearing loopback headers rather than an external reverse proxy, matching the documented private-ingress identity-bearing mode.
Before evidence (optional but encouraged): The prior code removed the default HTTP deny when gateway.tools.allow included a tool and did not apply any later sender-owner deny for these core wrappers.

Tests and validation

Which commands did you run?

node scripts/run-vitest.mjs src/gateway/tools-invoke-http.test.ts src/gateway/tools-invoke-http.cron-regression.test.ts src/gateway/tool-resolution.test.ts
node scripts/run-oxlint-shards.mjs --threads=8
git diff --check
Real loopback gateway curl proof described above.

What regression coverage was added or updated?

Added coverage that non-owner HTTP callers cannot access cron, gateway, or nodes even when both agent policy and gateway.tools.allow include them.
Added coverage that shared-secret bearer auth still receives owner semantics for explicitly allowed owner-only tools.
Added coverage that non-admin RPC callers cannot access the same owner-only tools while operator.admin RPC callers remain owner-capable.
Added coverage that spawned sessions inherit the owner-only denylist from non-owner tool resolution.

What failed before this fix, if known?

The new non-owner owner-only-tool denial assertion would have failed because gateway.tools.allow removed the default deny and no owner-only deny remained.

If no test was added, why not?

Regression tests were added.

Risk checklist

Did user-visible behavior change? (Yes/No)

Yes. Non-owner identity-bearing callers now see owner-only core tools as unavailable on Gateway tool invocation surfaces.

Did config, environment, or migration behavior change? (Yes/No)

No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No)

Yes. This restores an auth/tool-exposure boundary for owner-only core wrappers on the Gateway tools surface.

What is the highest-risk area?

The main risk is over-restricting legitimate direct automation that relied on write-scoped identity-bearing callers using cron, gateway, or nodes.

How is that risk mitigated?

The restriction is scoped to non-owner callers only. Admin/owner scope and shared-secret trusted-operator callers keep the existing explicitly enabled behavior, and the regression tests plus real loopback proof cover both sides.

Current review state

What is the next action?

Review the PR and run CI.

What is still waiting on author, maintainer, CI, or external proof?

CI has two failures currently annotated outside this PR's changed files; local focused validation, local oxlint, review-pr, autoreview, and GHSA dry-run are clean at the current head.

Which bot or reviewer comments were addressed?

ClawSweeper requested real behavior proof; the PR body now includes a real loopback gateway curl proof.

clawsweeper · 2026-06-04T08:56:02Z

Codex review: found issues before merge. Reviewed June 4, 2026, 5:54 AM ET / 09:54 UTC.

Summary
The PR adds an owner-only deny layer for cron, gateway, and nodes on Gateway tool invocation, threads RPC admin scope into the shared invoke path, and adds HTTP/RPC regression coverage.

PR surface: Source +17, Tests +116. Total +133 across 4 files.

Reproducibility: yes. Source inspection shows current main removes these tools from the default deny when gateway.tools.allow includes them, and the PR body gives a concrete loopback Gateway curl reproduction showing non-owner 404 after the fix.

Review metrics: 1 noteworthy metric.

Owner-only allowlist exceptions: 3 core tools restricted. cron, gateway, and nodes become unavailable to non-owner callers even when gateway.tools.allow includes them, which changes an existing config contract.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

Update the public Tools Invoke/config docs to describe the owner-only exception for cron, gateway, and nodes.
[P1] Get explicit maintainer acceptance that non-admin identity-bearing callers now fail closed for these explicitly allowlisted wrappers.

Risk before merge

[P1] Existing trusted-proxy or private-ingress automations that intentionally used non-admin operator.write plus gateway.tools.allow for cron, gateway, or nodes will start receiving not_found; that fail-closed upgrade behavior needs maintainer acceptance and docs before merge.

Maintainer options:

Document and accept owner-only allowlist behavior (recommended)
Update the public allowlist docs/config wording for cron, gateway, and nodes, then have a maintainer explicitly accept the non-owner not_found upgrade behavior.
Accept the fail-closed upgrade risk
Maintainers may intentionally ship the tighter boundary knowing existing write-scoped automations using these wrappers will need admin/shared-secret access instead.

Next step before merge

[P2] A maintainer/security decision is needed for the compatibility-sensitive allowlist behavior, and the contributor should update docs before merge.

Security
Cleared: The diff tightens an owner-only Gateway tool boundary and adds no workflow, dependency, secret, or supply-chain changes; the remaining concern is compatibility/documentation.

Review findings

[P2] Document the owner-only allowlist exception — src/gateway/tool-resolution.ts:119-122

Review details

Best possible solution:

Keep the centralized resolver gate, update the public gateway.tools.allow contract for the owner-only exception, and land only after maintainers accept the compatibility tradeoff for non-owner identity-bearing callers.

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

Yes. Source inspection shows current main removes these tools from the default deny when gateway.tools.allow includes them, and the PR body gives a concrete loopback Gateway curl reproduction showing non-owner 404 after the fix.

Is this the best way to solve the issue?

Mostly yes. The shared resolver is the right boundary because both HTTP and RPC invoke paths call invokeGatewayTool, but the compatibility-sensitive owner-only exception needs matching docs and maintainer acceptance.

Full review comments:

[P2] Document the owner-only allowlist exception — src/gateway/tool-resolution.ts:119-122
This new deny layer means gateway.tools.allow no longer fully removes the default deny for cron, gateway, and nodes: non-owner identity-bearing HTTP/RPC callers still get not_found. The public Tools Invoke/config docs still say gateway.tools.allow removes tools from the default deny list, so operators upgrading trusted-proxy or private-ingress setups will get an inaccurate contract unless the docs/config wording moves with this behavior.
Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against f7ef52e66d97.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes exact loopback openclaw gateway run setup plus curl results showing non-owner 404/not_found for the three tools and admin success for the explicitly allowed control.
add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes exact loopback openclaw gateway run setup plus curl results showing non-owner 404/not_found for the three tools and admin success for the explicitly allowed control.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: ⏳ waiting on author.

Label justifications:

P1: This is a security boundary fix for Gateway tool execution with immediate compatibility impact on real operator workflows.
merge-risk: 🚨 compatibility: Merging changes the behavior of existing gateway.tools.allow configurations for non-admin identity-bearing callers.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes exact loopback openclaw gateway run setup plus curl results showing non-owner 404/not_found for the three tools and admin success for the explicitly allowed control.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes exact loopback openclaw gateway run setup plus curl results showing non-owner 404/not_found for the three tools and admin success for the explicitly allowed control.

Evidence reviewed

PR surface:

Source +17, Tests +116. Total +133 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	19	2	+17
Tests	1	116	0	+116
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	135	2	+133

What I checked:

Current main removes default HTTP denies when allowlisted: On current main, defaultGatewayDeny filters DEFAULT_GATEWAY_HTTP_TOOL_DENY against gateway.tools.allow, and there is no later owner-only deny for cron, gateway, or nodes. (src/gateway/tool-resolution.ts:112, f7ef52e66d97)
PR adds centralized owner-only deny after allow resolution: The PR adds ownerOnlyGatewayDeny for non-owner HTTP-surface resolution and merges it into both inherited denies and the final Gateway deny set. (src/gateway/tool-resolution.ts:119, c01b172e9eb8)
HTTP owner identity is already derived from authenticated request context: Shared-secret HTTP auth returns owner semantics while identity-bearing modes honor declared scopes, so the new resolver gate is using an existing boundary signal rather than trusting caller-supplied owner headers. (src/gateway/http-auth-utils.ts:246, f7ef52e66d97)
RPC tools.invoke remains an operator.write method: The current method descriptor still exposes tools.invoke as operator.write; this makes the PR's per-tool owner-only behavior a compatibility-sensitive runtime policy change rather than a method-scope rename. (src/gateway/methods/core-descriptors.ts:95, f7ef52e66d97)
Public docs still describe allowlist as removing the default deny: The Tools Invoke docs say gateway.tools.allow removes tools from the default deny list, but the PR makes that incomplete for non-owner callers of cron, gateway, and nodes. Public docs: docs/gateway/tools-invoke-http-api.md. (docs/gateway/tools-invoke-http-api.md:123, f7ef52e66d97)
PR tests cover non-owner denial and owner/shared-secret success: The PR adds tests for non-owner HTTP/RPC denial despite gateway.tools.allow, shared-secret owner behavior, admin RPC success, and spawned-session deny inheritance. (src/gateway/tools-invoke-http.test.ts:765, c01b172e9eb8)

Likely related people:

steipete: Recent GitHub history shows repeated Gateway tool-resolution/docs work, and local history shows the prior owner-gating removal commit in this path. (role: recent area contributor; confidence: high; commits: 02182d5a3031, 9ead0ae9219e, 3ad7049cba9d; files: src/gateway/tool-resolution.ts, src/gateway/tools-invoke-http.ts, docs/gateway/tools-invoke-http-api.md)
ai-hpc: Introduced the SDK-facing tools.invoke RPC that this PR now changes for owner-only core wrappers. (role: feature introducer; confidence: high; commits: ef0eb126159f; files: src/gateway/server-methods/tools-invoke.ts, packages/gateway-protocol/src/schema/agents-models-skills.ts, docs/gateway/protocol.md)
jacobtomlinson: Authored the earlier Gateway HTTP tool invoke authorization hardening that established the owner-only HTTP direction for this surface. (role: historical hardening contributor; confidence: medium; commits: 29cb1e3c7edd; files: docs/gateway/tools-invoke-http-api.md, src/gateway/tools-invoke-http.test.ts)
Brian Mendonca: Added the default cron deny for /tools/invoke, which is one of the owner-only core tools affected by this PR. (role: adjacent security contributor; confidence: medium; commits: d51a4695f0ce; files: src/security/dangerous-tools.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.

pgondhi987 · 2026-06-04T09:45:55Z

@clawsweeper re-review

I updated the PR body with real loopback Gateway proof. Summary: a real openclaw gateway run with auth.mode="none", gateway.tools.allow=["gateway","cron","nodes"], and matching main-agent allowlist returned 404/not_found for gateway, cron, and nodes as x-openclaw-scopes: operator.write, while x-openclaw-scopes: operator.admin still reached the explicitly allowed gateway tool with HTTP 200.

clawsweeper · 2026-06-04T09:45:58Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26944141229
Updated: 2026-06-04T09:54:45.443Z

pgondhi987 · 2026-06-04T09:52:47Z

Verification update for current head c01b172e9eb83c8f489a20afee42add51edddcf6:

Focused local validation passed: git diff --check; node scripts/run-vitest.mjs src/gateway/tools-invoke-http.test.ts src/gateway/tools-invoke-http.cron-regression.test.ts src/gateway/tool-resolution.test.ts (12 files, 168 tests); node scripts/run-oxlint-shards.mjs --threads=8.
Real behavior proof passed on a real loopback gateway: openclaw gateway run --allow-unconfigured --auth none --bind loopback --port 26779 with temp auth.mode="none", gateway.tools.allow=["gateway","cron","nodes"], and matching main-agent allowlist. x-openclaw-scopes: operator.write returned 404/not_found for gateway, cron, and nodes; x-openclaw-scopes: operator.admin reached the explicitly allowed gateway tool with HTTP 200.
review-pr passed: READY FOR /prepare-pr, findings 0.
autoreview passed: clean, no accepted/actionable findings.
GHSA gate passed: scope ACCEPT, fix SOLVES, backward compatibility PASS; tracking comment posted by the gate.
Mergeability: GitHub reports MERGEABLE; current UNSTABLE state is from CI, not conflicts.

Known external gaps:

CI currently has two failures outside this PR's changed files: check-lint annotations in src/plugins/host-hook-runtime.ts, and checks-fast-contracts-plugins-a drift for packages/plugin-sdk/src/testing.ts in src/plugins/contracts/plugin-sdk-package-contract-guardrails.test.ts. The PR diff only touches src/gateway/server-methods/tools-invoke.ts, src/gateway/tool-resolution.ts, src/gateway/tools-invoke-http.test.ts, and src/security/dangerous-tools.ts; local oxlint passed on the branch.
ClawSweeper re-review was requested after adding real behavior proof and is still running: https://github.com/openclaw/clawsweeper/actions/runs/26944141229

* fix: gate owner-only HTTP tools * fix: inherit HTTP owner tool denies * fix: use mutable HTTP owner deny policy * fix: preserve RPC owner tool access * docs: clarify owner-only gateway tool allowlist --------- Co-authored-by: joshavant <830519+joshavant@users.noreply.github.com>

fix: gate owner-only HTTP tools

b62ecae

pgondhi987 requested a review from a team as a code owner June 4, 2026 08:53

openclaw-barnacle Bot added gateway Gateway runtime size: S maintainer Maintainer-authored PR labels Jun 4, 2026

pgondhi987 added 3 commits June 4, 2026 09:08

fix: inherit HTTP owner tool denies

7fe5e35

fix: use mutable HTTP owner deny policy

de55537

fix: preserve RPC owner tool access

c01b172

joshavant assigned steipete Jun 7, 2026

docs: clarify owner-only gateway tool allowlist

2c80366

openclaw-barnacle Bot added the docs Improvements or additions to documentation label Jun 7, 2026

joshavant merged commit 2a21de6 into openclaw:main Jun 7, 2026
165 of 167 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: gate owner-only HTTP tools#90261

fix: gate owner-only HTTP tools#90261
joshavant merged 5 commits into
openclaw:mainfrom
pgondhi987:fix/fix-720

pgondhi987 commented Jun 4, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

pgondhi987 commented Jun 4, 2026

Uh oh!

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

pgondhi987 commented Jun 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

pgondhi987 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linked context

Real behavior proof (required for external PRs)

Tests and validation

Risk checklist

Current review state

Uh oh!

clawsweeper Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pgondhi987 commented Jun 4, 2026

Uh oh!

clawsweeper Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pgondhi987 commented Jun 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pgondhi987 commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading

clawsweeper Bot commented Jun 4, 2026 •

edited

Loading