Skip to content

fix(gateway): enforce owner-only tools in invoke API#55724

Closed
RichardCao wants to merge 1 commit intoopenclaw:mainfrom
RichardCao:fix/tools-invoke-owneronly
Closed

fix(gateway): enforce owner-only tools in invoke API#55724
RichardCao wants to merge 1 commit intoopenclaw:mainfrom
RichardCao:fix/tools-invoke-owneronly

Conversation

@RichardCao
Copy link
Copy Markdown
Contributor

Summary

  • apply ownerOnly filtering to the HTTP /tools/invoke surface after the normal tool-policy pipeline
  • treat authenticated HTTP callers as non-owner contexts so owner-only tools stay unavailable even if separately allowlisted
  • add gateway HTTP regression coverage for nodes and for gateway.tools.allow not bypassing owner-only restrictions

Testing

  • pnpm exec vitest run --config vitest.gateway.config.ts src/gateway/tools-invoke-http.test.ts

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Mar 27, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 27, 2026

Greptile Summary

This PR enforces ownerOnly tool filtering on the HTTP /tools/invoke gateway surface by calling applyOwnerOnlyToolPolicy(gatewayFiltered, false) after the existing deny-list pipeline. Previously, placing a tool in gateway.tools.allow could bypass the HTTP deny-list for owner-only tools (like gateway, cron, whatsapp_login, and nodes); with this change, HTTP callers are permanently treated as non-owners so owner-only tools are never reachable via HTTP regardless of allowlist config.

Changes:

  • tools-invoke-http.ts: One-line addition applying applyOwnerOnlyToolPolicy with senderIsOwner = false after the gateway deny filter, so the final tool lookup searches ownerAuthorizedTools instead of gatewayFiltered.
  • tools-invoke-http.test.ts: Adds a nodes (ownerOnly) mock tool, adds a test verifying it is denied (404) even when in agents.tools.allow, and rewrites the existing "allows gateway tool via HTTP when explicitly enabled" test to assert the opposite — that gateway.tools.allow no longer bypasses the owner-only restriction.

The logic is correct: applyOwnerOnlyToolPolicy already handles both the explicit ownerOnly: true flag and the name-based fallback list (OWNER_ONLY_TOOL_NAME_FALLBACKS: whatsapp_login, cron, gateway, nodes). Passing false as senderIsOwner filters all such tools out for HTTP callers. The existing test for sessions_spawn still passes because that tool is not in OWNER_ONLY_TOOL_NAME_FALLBACKS and can be unlocked via gateway.tools.allow as before.

Confidence Score: 5/5

Safe to merge — the change is a narrow security hardening with correct logic and matching regression tests.

The implementation is a single well-scoped line that applies an already-tested policy function. New tests cover both the ownerOnly flag path (nodes) and the name-based fallback path (gateway via OWNER_ONLY_TOOL_NAME_FALLBACKS). No existing tests break. The note about sessions_spawn still being allowlistable is by design (it is not in OWNER_ONLY_TOOL_NAME_FALLBACKS). No P0/P1 issues found.

No files require special attention.

Important Files Changed

Filename Overview
src/gateway/tools-invoke-http.ts Adds a single applyOwnerOnlyToolPolicy(gatewayFiltered, false) call after the gateway deny filter, correctly enforcing owner-only restrictions for all HTTP callers.
src/gateway/tools-invoke-http.test.ts Adds nodes (ownerOnly) mock tool, new test for ownerOnly denial, and rewrites the gateway.tools.allow bypass test to assert the corrected 404 behavior.

Reviews (1): Last reviewed commit: "fix(gateway): enforce owner-only tools i..." | Re-trigger Greptile

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 26, 2026

Closing this as duplicate or superseded after Codex automated review.

PR #55724 is superseded by the current Gateway HTTP trust-boundary contract. Current main and the latest release intentionally treat shared-secret /tools/invoke callers as trusted operator/owner contexts, while still applying owner-only filtering for non-owner identity-scoped callers and keeping dangerous tools on the default HTTP deny list unless explicitly re-enabled.

Best possible solution:

Close #55724 as superseded. Keep the shipped /tools/invoke contract: shared-secret auth is trusted operator/owner access, identity-bearing HTTP modes can narrow owner semantics with scopes, and high-risk tools remain protected by the default HTTP deny list plus audit warnings unless the operator explicitly re-enables them.

What I checked:

  • Current handler applies owner-only policy from resolved sender context: handleToolsInvokeHttpRequest resolves senderIsOwner, passes it into resolveGatewayScopedTools, then calls applyOwnerOnlyToolPolicy(tools, senderIsOwner). This intentionally does not force all HTTP callers to non-owner the way fix(gateway): enforce owner-only tools in invoke API #55724 proposed. (src/gateway/tools-invoke-http.ts:223, 06d409dc2738)
  • Shared-secret HTTP auth is explicitly owner on direct tool invoke: resolveOpenAiCompatibleHttpSenderIsOwner returns true for token/password shared-secret auth, with an inline comment saying direct /tools/invoke follows the documented trusted-operator contract. (src/gateway/http-utils.ts:255, 06d409dc2738)
  • Docs and SECURITY define the superseding contract: The Tools Invoke docs call the endpoint full operator access and say shared-secret auth treats direct tool invokes as owner-sender turns; SECURITY.md also says reports treating shared-secret /tools/invoke as a narrower per-request authorization surface are false positives. Public docs: docs/gateway/tools-invoke-http-api.md. (docs/gateway/tools-invoke-http-api.md:41, 06d409dc2738)
  • Regression coverage matches current behavior: Tests assert non-owner identity-scoped callers get 404 for owner-only tools, admin-scoped callers can invoke owner-only tools, and shared-secret bearer auth is owner on /tools/invoke. They also preserve the current gateway.tools.allow behavior for the gateway tool when admin-scoped. (src/gateway/tools-invoke-http.test.ts:819, 06d409dc2738)
  • Latest release already contains the superseding behavior: Release tag v2026.4.24 points at cbcfdf62c7297bda66009ea7476f053c3e9addab, and that tree contains the same /tools/invoke owner semantics and SECURITY wording. (SECURITY.md:63, cbcfdf62c729)

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against 06d409dc2738; fix evidence: release v2026.4.24, commit cbcfdf62c729.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant