fix(gateway): require admin for device role approvals#87146
Conversation
|
Codex review: found issues before merge. Reviewed May 27, 2026, 10:57 AM ET / 14:57 UTC. Summary PR surface: Source +3, Tests +203, Docs +20. Total +226 across 7 files. Reproducibility: yes. for the original bug shape from source and PR proof, but not as a current-main failure because current Review metrics: 2 noteworthy metrics.
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: Keep the current-main hardening and have the assigned maintainer either close this superseded dirty PR or refresh it to remove release-owned changelog churn and carry only any missing useful delta. Do we have a high-confidence way to reproduce the issue? Yes for the original bug shape from source and PR proof, but not as a current-main failure because current Is this the best way to solve the issue? Mostly yes for the hardening itself: the handler-level pending-request check is narrow and preserves operator-role approvals. The remaining solution question is maintainer ownership of the compatibility break and whether this dirty PR should be closed as superseded by current main. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 0d565833e15e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +3, Tests +203, Docs +20. Total +226 across 7 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
|
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Neon Patch Peep Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
Before this PR, some existing setups could approve a device role like node using only pairing/shared-auth/trusted-proxy trust. After this PR, those approvals require operator.admin. So an existing installation that upgrades may start rejecting approvals that used to work. That is the “compatibility break.” The maintainer decision is:
|
* fix(gateway): require admin for device role approvals * fix(gateway): add trusted-proxy approval proof
…026.5.27) (#698) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/openclaw/openclaw](https://openclaw.ai) ([source](https://github.com/openclaw/openclaw)) | patch | `2026.5.26` → `2026.5.27` | --- ### Release Notes <details> <summary>openclaw/openclaw (ghcr.io/openclaw/openclaw)</summary> ### [`v2026.5.27`](https://github.com/openclaw/openclaw/blob/HEAD/CHANGELOG.md#2026527) [Compare Source](openclaw/openclaw@v2026.5.26...v2026.5.27) ##### Highlights - Safer local/runtime boundaries: OpenClaw now rejects unsafe command wrappers, malformed CLI numeric options, unsafe Node runtime env overrides, no-auth Tailscale exposure, and non-admin device-role pairing approvals before they can affect live runs. ([#​87308](openclaw/openclaw#87308), [#​87305](openclaw/openclaw#87305), [#​87292](openclaw/openclaw#87292), [#​87146](openclaw/openclaw#87146)) - Matrix and auto-reply delivery are steadier: mention previews stay inert, final mention replies deliver normally, shared-DM notices are awaited, MXID parsing ignores filenames, and reasoning-prefixed `NO_REPLY` responses stay suppressed. - Provider and agent reliability improved across OpenAI-compatible embeddings, cached token usage, Anthropic/Codex/Claude runtime state, unsupported tool-schema quarantine, heartbeat templates, and session fallback errors. ([#​85269](openclaw/openclaw#85269), [#​82062](openclaw/openclaw#82062), [#​85416](openclaw/openclaw#85416), [#​86855](openclaw/openclaw#86855)) - Plugin and package release paths got tighter: Pixverse ships as an external video plugin with region selection, package exclusions and shrinkwrap inventory match the published npm shape, and release/package smoke commands fail bounded instead of hanging. - Gateway hot paths do less rediscovery by reusing current plugin metadata fingerprints, stable plugin index fingerprints, read-only session metadata, active working stores, status fast paths, and auth/env snapshots. ([#​86439](openclaw/openclaw#86439)) ##### Changes - Memory: add a core OpenAI-compatible embedding provider for local and hosted OpenAI-style endpoints, with config, doctor, and docs support. ([#​85269](openclaw/openclaw#85269)) Thanks [@​dutifulbob](https://github.com/dutifulbob). - Plugin SDK: mark memory-specific embedding provider registration as deprecated compatibility and surface non-bundled usage in plugin compatibility diagnostics. ([#​85072](openclaw/openclaw#85072)) Thanks [@​mbelinky](https://github.com/mbelinky). - Pixverse: add video generation provider support, API region selection, and external plugin publishing. - Plugins: expose approval action metadata for plugin-driven approval surfaces. ##### Fixes - Security/CLI/runtime: harden hostname normalization for repeated trailing dots, block side-effecting command wrappers, reject unsafe Node runtime env overrides, reject loose numeric CLI and gateway options, require admin approval for node device-role pairing, and reject no-auth Tailscale exposure. ([#​87305](openclaw/openclaw#87305), [#​87292](openclaw/openclaw#87292), [#​87308](openclaw/openclaw#87308), [#​87146](openclaw/openclaw#87146)) Thanks [@​pgondhi987](https://github.com/pgondhi987). - Doctor: validate runtime tool schemas for every configured embedded agent while skipping ACP-only profiles, so bad non-default plugin or MCP tools are reported before assistant turns. - Telegram: route `sendMessage` action replies through durable outbound delivery so completed agent responses remain retryable when the gateway send path times out. ([#​87261](openclaw/openclaw#87261)) Thanks [@​mbelinky](https://github.com/mbelinky). - Matrix/auto-reply: keep draft previews mention-inert, preserve final mention delivery, send mention finals normally, await shared DM notices, ignore filename-embedded MXIDs, and suppress reasoning-prefixed `NO_REPLY` responses. - Agents/providers: add OpenAI-compatible cache retention, forward cached token usage in chat completions, preserve runtime context before active user turns, strip stale Anthropic thinking, load Claude CLI OAuth for Pi auth profiles, avoid false Codex runtime live switches, and quarantine unsupported tool schemas. ([#​82062](openclaw/openclaw#82062), [#​87167](openclaw/openclaw#87167), [#​86855](openclaw/openclaw#86855)) - Gateway/performance: cache plugin metadata fingerprints and stable plugin index fingerprints, borrow read-only session metadata safely, keep the active session working store hot, keep status on a bounded fast path, and preserve model auth profile suffixes. ([#​86439](openclaw/openclaw#86439)) - Package/install/release: align npm package exclusions and inventory, omit unpacked test helpers, skip Homebrew until macOS packages need it, cap tsdown heap in containers, bound install/release smoke waits, and harden post-publish verification. - Codex/Auth: bound ChatGPT OAuth token exchange and refresh requests, and honor cancellation across Codex and Anthropic OAuth login flows. - QA/E2E/CI: bound Telegram, kitchen-sink, Open WebUI, ClawHub, MCP, Discord, realtime, labeler, and GitHub API waits; fail empty explicit test, live-media, gateway CPU, startup benchmark, plugin gauntlet, and beta-smoke runs instead of false-greening. - Agents/Codex: keep spawned agent bootstrap files rooted in the agent workspace while running task commands, transcripts, and compaction from the requested cwd. ([#​87218](openclaw/openclaw#87218)) Thanks [@​mbelinky](https://github.com/mbelinky). </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL3BhdGNoIl19--> Reviewed-on: https://git.erwanleboucher.dev/eleboucher/homelab/pulls/698
* fix(gateway): require admin for device role approvals * fix(gateway): add trusted-proxy approval proof
* fix(gateway): require admin for device role approvals * fix(gateway): add trusted-proxy approval proof
Summary
Require non-admin
device.pair.approvecallers to pass the non-operator device-role gate regardless of whether the session was authenticated with a device token or shared auth.Changes
approveDevicePairingruns.operator.admin.Validation
Real behavior proof
Behavior addressed: Non-admin pairing-only trusted-proxy/shared-auth callers can no longer approve non-operator device roles through
device.pair.approve; trusted-proxy callers withoperator.adminstill can.Real environment tested: Local source checkout on branch
688at8b131d444975fd2a01e40894aee49e7ce33df680, Nodev22.22.2, pnpm11.2.2. The proof started a local Gateway configured withgateway.auth.mode: "trusted-proxy",gateway.trustedProxies: ["127.0.0.1"],allowLoopback: true, and WebSocket headersx-forwarded-userplusx-forwarded-proto.Exact steps or command run after this patch:
node scripts/run-vitest.mjs run src/gateway/trusted-proxy-proof.tmp.test.ts --reporter=verbose. The temporary proof test opened real Gateway WebSocket sessions with trusted-proxy headers, signed paired device identities, omittedauth.deviceToken, submitteddevice.pair.approve, printed the RPC responses below, and was removed after the run so it is not part of the PR diff.Evidence after fix:
Observed result after fix: The pairing-only trusted-proxy session reached the real WS
device.pair.approvepath and was denied withdevice pairing approval denied; the pending node device stayed unpaired. The admin trusted-proxy session reached the same path and approved the node device withrole=node.What was not tested: I did not run a packaged install, cross-OS lane, or external production reverse proxy. The proof uses the repository Gateway test harness with a local loopback trusted-proxy configuration and real WebSocket/RPC traffic.
Proof limitations or environment constraints: The proof output is copied terminal output, not a screenshot or video. It redacts private paths and uses synthetic device identities, local ports, and test headers.
Focused regression proof
node scripts/run-vitest.mjs run src/gateway/server.device-pair-approve-authz.test.ts --reporter=verbose -t "trusted-proxy"passed and showed both committed trusted-proxy WS regressions:Additional validation already run for this branch:
node scripts/run-vitest.mjs src/gateway/server-methods/devices.test.ts src/gateway/server.device-pair-approve-authz.test.tscorepack pnpm exec oxfmt --check --threads=1 docs/cli/devices.md docs/gateway/operator-scopes.md src/gateway/device-authz.test-helpers.ts src/gateway/server-methods/devices.ts src/gateway/server-methods/devices.test.ts src/gateway/server.device-pair-approve-authz.test.tsnode scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json src/gateway/device-authz.test-helpers.ts src/gateway/server-methods/devices.ts src/gateway/server-methods/devices.test.ts src/gateway/server.device-pair-approve-authz.test.tscorepack pnpm docs:listgit diff --checkNotes
CHANGELOG.mdwas not updated.USER.mdis a local worklog only and is intentionally not part of this PR.