Skip to content

Require admin scope for node device token management [AI]#81067

Merged
pgondhi987 merged 9 commits into
openclaw:mainfrom
pgondhi987:fix/fix-632
May 13, 2026
Merged

Require admin scope for node device token management [AI]#81067
pgondhi987 merged 9 commits into
openclaw:mainfrom
pgondhi987:fix/fix-632

Conversation

@pgondhi987

@pgondhi987 pgondhi987 commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: Pairing-scoped operator sessions could request node-role device token rotation through the device management RPC surface.
  • Why it matters: Node tokens should only return to active use through an admin-authorized path.
  • What changed: Added a server-side role gate for device.token.rotate and device.token.revoke so non-admin sessions can only manage operator-role device tokens.
  • What did NOT change (scope boundary): Operator-token self-management and admin-managed token operations remain allowed.
  • AI-assisted: Yes.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

N/A

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: Node-role device token rotation and revocation now require admin authority.
  • Real environment tested: Not run in this metadata drafting pass.
  • Exact steps or command run after this patch: Not run in this metadata drafting pass.
  • Evidence after change: Regression coverage was added in src/gateway/server.device-token-rotate-authz.test.ts and src/gateway/server-methods/devices.test.ts.
  • Observed result after change: Not run in this metadata drafting pass.
  • What was not tested: Local runtime validation and full changed checks.
  • Before evidence (optional but encouraged): N/A

Root Cause (if applicable)

  • Root cause: Device token RPCs checked same-device ownership and caller scopes, but did not require admin authority for node-role token operations.
  • Missing detection / guardrail: There was no regression covering a mixed-role paired device with a revoked node token and a surviving pairing-scoped operator session.
  • Contributing context (if known): Node-role token entries use empty scopes, so caller-scope checks alone were not enough to distinguish privileged node-token management.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/gateway/server-methods/devices.test.ts; src/gateway/server.device-token-rotate-authz.test.ts
  • Scenario the test should lock in: A non-admin pairing-scoped operator session cannot rotate or revoke node-role device tokens, including after a node token has been revoked.
  • Why this is the smallest reliable guardrail: It exercises the server-side RPC authorization boundary directly and covers the real Gateway RPC path for the mixed-role device case.
  • Existing test that already covers this (if any): Existing ownership and caller-scope tests covered cross-device and operator-scope escalation, but not node-role token management on the same device.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

Non-admin sessions can no longer rotate or revoke node-role device tokens. Admin-authorized management remains available.

Diagram (if applicable)

Before:
[pairing-scoped operator session] -> [node token rotate RPC] -> [node token active]

After:
[pairing-scoped operator session] -> [node token rotate RPC] -> [denied]
[admin session] -> [node token management RPC] -> [authorized path]

Security Impact (required)

  • New permissions/capabilities? (Yes/No): No
  • Secrets/tokens handling changed? (Yes/No): Yes
  • New/changed network calls? (Yes/No): No
  • Command/tool execution surface changed? (Yes/No): No
  • Data access scope changed? (Yes/No): Yes
  • If any Yes, explain risk + mitigation: Node-role token management is restricted to admin-authorized callers. This narrows token authority without introducing new token material or network behavior.

Repro + Verification

Environment

  • OS: Not run in this metadata drafting pass
  • Runtime/container: Not run in this metadata drafting pass
  • Model/provider: N/A
  • Integration/channel (if any): Gateway WebSocket RPC
  • Relevant config (redacted): N/A

Steps

  1. Pair a device with both operator and node roles.
  2. Revoke the node-role device token through an admin-authorized RPC call.
  3. Connect using the same device's pairing-scoped operator token.
  4. Attempt to rotate the node-role device token.

Expected

  • The non-admin pairing-scoped operator session receives device token rotation denied.
  • The revoked node token remains revoked and unchanged.

Actual

  • Not run in this metadata drafting pass.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: Static review of the role gate and the added regression scenarios.
  • Edge cases checked: Operator-role token management remains allowed for non-admin callers within existing scope checks; admin callers bypass the new node-role restriction.
  • What you did not verify: Local runtime tests, typecheck, and full changed checks.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes/No): Yes
  • Config/env changes? (Yes/No): No
  • Migration needed? (Yes/No): No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: Workflows that expected non-admin sessions to manage node-role device tokens will now be denied.
    • Mitigation: Admin-authorized token management remains available, and operator-role token management is unchanged.

@pgondhi987 pgondhi987 requested a review from a team as a code owner May 12, 2026 15:19
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: S maintainer Maintainer-authored PR labels May 12, 2026
@clawsweeper

clawsweeper Bot commented May 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
The branch adds Gateway authorization gates and regression tests requiring admin authority for non-operator device token rotation/revocation, same-device non-operator pairing approval/removal, and local reconnect reactivation paths.

Reproducibility: yes. source-reproducible: current main routes same-device non-admin node rotate/revoke requests into token mutation, and the scope check accepts empty node scope sets. I did not run a live Gateway repro in this read-only review.

Real behavior proof
Needs real behavior proof before merge: Missing: the PR body says no real environment or runtime validation was run; add redacted terminal output, logs, screenshots/recordings, or linked artifacts, then update the PR body or ask for @clawsweeper re-review.

Next step before merge
Protected maintainer/security authorization work plus missing contributor real behavior proof require maintainer review before merge; automation should not repair or merge this branch.

Security
Cleared: Cleared: the diff narrows Gateway token authority and adds no dependencies, secret exposure, network calls, or code execution paths.

Review findings

  • [P3] Update the protocol docs for the admin gate — src/gateway/server-methods/devices.ts:427
Review details

Best possible solution:

Land a maintainer-approved security fix that gates node/non-operator token reactivation behind operator.admin, preserves operator-token self-management, updates protocol docs, and includes redacted runtime proof.

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

Yes, source-reproducible: current main routes same-device non-admin node rotate/revoke requests into token mutation, and the scope check accepts empty node scope sets. I did not run a live Gateway repro in this read-only review.

Is this the best way to solve the issue?

Likely yes: the PR gates non-operator role management before token mutation and blocks the local reconnect reactivation path. The merge-ready version should also update protocol docs and include real after-fix proof.

Full review comments:

  • [P3] Update the protocol docs for the admin gate — src/gateway/server-methods/devices.ts:427
    This PR denies non-admin management of non-operator device roles, but docs/gateway/protocol.md still says device.token.rotate/device.token.revoke require operator.pairing plus same-device self-scope. Update the public contract so clients know node-role token management requires operator.admin.
    Confidence: 0.89

Overall correctness: patch is correct
Overall confidence: 0.82

Acceptance criteria:

  • pnpm test src/gateway/server-methods/devices.test.ts src/gateway/server.device-token-rotate-authz.test.ts
  • pnpm check:changed

What I checked:

  • Current main rotate/revoke path: Checked-out main routes device.token.rotate and device.token.revoke through cross-device ownership checks and then calls token mutation without a non-operator role admin gate. (src/gateway/server-methods/devices.ts:329, ae773272ac8e)
  • Current main caller-scope behavior: The token layer checks requested or target scopes against caller scopes; empty node-token scopes do not produce a missing-scope denial, so role alone is not distinguished by this check. (src/infra/device-pairing.ts:1049, ae773272ac8e)
  • PR authorization change: The PR adds deniesDeviceTokenRoleManagement and applies it before rotate/revoke mutation so non-admin callers cannot manage non-operator token roles. (src/gateway/server-methods/devices.ts:427, c292c4ea447a)
  • PR reconnect change: The PR disables silent local pairing for existing paired devices requesting a non-operator role, matching the added revoked-node-token reconnect coverage. (src/gateway/server/ws-connection/message-handler.ts:1015, c292c4ea447a)
  • Docs contract gap: Current protocol docs still describe device token rotate/revoke as operator.pairing plus self-scoped management and do not mention the new non-operator role admin requirement. Public docs: docs/gateway/protocol.md. (docs/gateway/protocol.md:703, ae773272ac8e)
  • Proof and label state: Live PR API/body data shows the PR is open with the protected maintainer label, and the body says no real environment or runtime validation was run after the patch. (c292c4ea447a)

Likely related people:

  • @steipete: Recent GitHub path history shows token echo/scope-containment and device-pairing fixes in the central files, and the checked-out grafted blame points the current handlers/docs to Peter Steinberger's provenance commit. (role: recent area contributor; confidence: high; commits: 016a0b4de945, 8bbb143ab87e, c9053ff20853; files: src/gateway/server-methods/devices.ts, src/infra/device-pairing.ts, src/gateway/server/ws-connection/message-handler.ts)
  • @eleqtrizit: Path history includes paired-device management authorization and unapproved token-role rejection changes adjacent to this handler policy. (role: adjacent authz contributor; confidence: medium; commits: 5a12f30441d5, 7cda9df4cb27; files: src/gateway/server-methods/devices.ts)
  • @pgondhi987: Beyond this PR, GitHub path history shows recent merged work on device-pairing caller-scope validation, gateway auth scope clamping, and node-pairing pending request behavior. (role: recent adjacent contributor; confidence: medium; commits: 189c91eae6e1, 0e702f106313, 3aeb55b5e7b1; files: src/infra/device-pairing.ts, src/gateway/server/ws-connection/message-handler.ts)

Remaining risk / open question:

  • External real behavior proof is missing; the PR body explicitly says no real environment or runtime validation was run.
  • The PR changes a security-sensitive authorization boundary and carries the protected maintainer label, so it needs explicit maintainer/security review.
  • The public Gateway protocol docs still need to describe the new node-role admin requirement before this user-visible contract change lands.

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

@pgondhi987

pgondhi987 commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Not applicable to this automation stage; changelog/release-note and external real behavior proof requirements are handled outside auto-pr stages.

Quoted comment from @clawsweeper:

Codex review: needs real behavior proof before merge.

Summary
The branch adds Gateway authorization gates and regression tests requiring admin authority for non-operator device token rotation/revocation, same-device non-operator pairing approval/removal, and local reconnect reactivation paths.

Reproducibility: yes. source-reproducible: current main routes same-device non-admin node rotate/revoke requests into token mutation, and the scope check accepts empty node scope sets. I did not run a live Gateway repro in this read-only review.

Real behavior proof
Needs real behavior proof before merge: Missing: the PR body says no real environment or runtime validation was run; add redacted terminal output, logs, screenshots/recordings, or linked artifacts, then update the PR body or ask for @clawsweeper re-review.

Next step before merge
Protected maintainer/security authorization work plus missing contributor real behavior proof require maintainer review before merge; automation should not repair or merge this branch.

Security
Cleared: Cleared: the diff narrows Gateway token authority and adds no dependencies, secret exposure, network calls, or code execution paths.

Review findings

  • [P3] Update the protocol docs for the admin gate — src/gateway/server-methods/devices.ts:427
Review details

Best possible solution:

Land a maintainer-approved security fix that gates node/non-operator token reactivation behind operator.admin, preserves operator-token self-management, updates protocol docs, and includes redacted runtime proof.

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

Yes, source-reproducible: current main routes same-device non-admin node rotate/revoke requests into token mutation, and the scope check accepts empty node scope sets. I did not run a live Gateway repro in this read-only review.

Is this the best way to solve the issue?

Likely yes: the PR gates non-operator role management before token mutation and blocks the local reconnect reactivation path. The merge-ready version should also update protocol docs and include real after-fix proof.

Full review comments:

  • [P3] Update the protocol docs for the admin gate — src/gateway/server-methods/devices.ts:427
    This PR denies non-admin management of non-operator device roles, but docs/gateway/protocol.md still says device.token.rotate/device.token.revoke require operator.pairing plus same-device self-scope. Update the public contract so clients know node-role token management requires operator.admin.
    Confidence: 0.89

Overall correctness: patch is correct
Overall confidence: 0.82

Acceptance criteria:

  • pnpm test src/gateway/server-methods/devices.test.ts src/gateway/server.device-token-rotate-authz.test.ts
  • pnpm check:changed

What I checked:

  • Current main rotate/revoke path: Checked-out main routes device.token.rotate and device.token.revoke through cross-device ownership checks and then calls token mutation without a non-operator role admin gate. (src/gateway/server-methods/devices.ts:329, ae773272ac8e)
  • Current main caller-scope behavior: The token layer checks requested or target scopes against caller scopes; empty node-token scopes do not produce a missing-scope denial, so role alone is not distinguished by this check. (src/infra/device-pairing.ts:1049, ae773272ac8e)
  • PR authorization change: The PR adds deniesDeviceTokenRoleManagement and applies it before rotate/revoke mutation so non-admin callers cannot manage non-operator token roles. (src/gateway/server-methods/devices.ts:427, c292c4ea447a)
  • PR reconnect change: The PR disables silent local pairing for existing paired devices requesting a non-operator role, matching the added revoked-node-token reconnect coverage. (src/gateway/server/ws-connection/message-handler.ts:1015, c292c4ea447a)
  • Docs contract gap: Current protocol docs still describe device token rotate/revoke as operator.pairing plus self-scoped management and do not mention the new non-operator role admin requirement. Public docs: docs/gateway/protocol.md. (docs/gateway/protocol.md:703, ae773272ac8e)
  • Proof and label state: Live PR API/body data shows the PR is open with the protected maintainer label, and the body says no real environment or runtime validation was run after the patch. (c292c4ea447a)

Likely related people:

  • @steipete: Recent GitHub path history shows token echo/scope-containment and device-pairing fixes in the central files, and the checked-out grafted blame points the current handlers/docs to Peter Steinberger's provenance commit. (role: recent area contributor; confidence: high; commits: 016a0b4de945, 8bbb143ab87e, c9053ff20853; files: src/gateway/server-methods/devices.ts, src/infra/device-pairing.ts, src/gateway/server/ws-connection/message-handler.ts)
  • @eleqtrizit: Path history includes paired-device management authorization and unapproved token-role rejection changes adjacent to this handler policy. (role: adjacent authz contributor; confidence: medium; commits: 5a12f30441d5, 7cda9df4cb27; files: src/gateway/server-methods/devices.ts)
  • @pgondhi987: Beyond this PR, GitHub path history shows recent merged work on device-pairing caller-scope validation, gateway auth scope clamping, and node-pairing pending request behavior. (role: recent adjacent contributor; confidence: medium; commits: 189c91eae6e1, 0e702f106313, 3aeb55b5e7b1; files: src/infra/device-pairing.ts, src/gateway/server/ws-connection/message-handler.ts)

Remaining risk / open question:

  • External real behavior proof is missing; the PR body explicitly says no real environment or runtime validation was run.
  • The PR changes a security-sensitive authorization boundary and carries the protected maintainer label, so it needs explicit maintainer/security review.
  • The public Gateway protocol docs still need to describe the new node-role admin requirement before this user-visible contract change lands.

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

@pgondhi987 pgondhi987 merged commit 1c85eff into openclaw:main May 13, 2026
21 of 22 checks passed
@Haderach-Ram Haderach-Ram mentioned this pull request May 13, 2026
Open
@github-actions github-actions Bot mentioned this pull request May 13, 2026
Open
l3ocifer pushed a commit to l3ocifer/frack-openclaw that referenced this pull request May 13, 2026
@pgondhi987 @l3ocifer
Require admin scope for node device token management [AI] (openclaw#8…
b90e368 
…1067)

* fix: require admin for node device token management

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing claude review

* addressing ci

* docs: add changelog entry for PR merge
@clawsweeper clawsweeper Bot mentioned this pull request May 21, 2026
Closed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
@pgondhi987
Require admin scope for node device token management [AI] (openclaw#8…
363275e 
…1067)

* fix: require admin for node device token management

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing claude review

* addressing ci

* docs: add changelog entry for PR merge
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
@pgondhi987
Require admin scope for node device token management [AI] (openclaw#8…
e3ee57c 
…1067)

* fix: require admin for node device token management

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing claude review

* addressing ci

* docs: add changelog entry for PR merge
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
@pgondhi987
Require admin scope for node device token management [AI] (openclaw#8…
f39f8a5 
…1067)

* fix: require admin for node device token management

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing review-skill

* addressing claude review

* addressing ci

* docs: add changelog entry for PR merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime maintainer Maintainer-authored PR size: L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pgondhi987