Skip to content

fix(gateway): honor scopes for WebChat session mutations#110931

Merged
steipete merged 2 commits into
mainfrom
codex/webchat-session-mutation-scopes
Jul 18, 2026
Merged

fix(gateway): honor scopes for WebChat session mutations#110931
steipete merged 2 commits into
mainfrom
codex/webchat-session-mutation-scopes

Conversation

@steipete

@steipete steipete commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Closes #110930

What Problem This Solves

Fixes an issue where custom WebChat clients with approved operator scopes could
not patch, delete, compact, branch, restore, rewind, fork, switch, dispatch, or
reclaim sessions. The blanket WebChat restriction made client identity act as a
second authorization layer and encouraged clients to impersonate Control UI.

Why This Change Was Made

Session mutations now use the existing centralized method-scope policy,
including parameter-aware patch and delete checks. Client ID and mode no longer
grant or remove mutation authority. Connection admission, device pairing, scope
approval, and gateway.controlUi.dangerouslyDisableDeviceAuth are unchanged.

User Impact

Approved third-party WebChat clients can manage sessions with their negotiated
operator scopes. Lower-scope clients receive the same structured missing-scope
errors as other Gateway clients.

Evidence

  • Blacksmith Testbox at 6e20935154a76cd43f433f42df1a7a19006125b6:
    server.sessions.permissions-hooks plus method-scopes, 132/132 tests passed.
  • Blacksmith Testbox pnpm check:changed: core and core-test typechecks, changed
    formatting, core lint, import-cycle checks, plugin boundaries, and security
    guards passed.
  • The Gateway integration test verifies read-scoped WebChat rejection across
    every formerly identity-blocked mutation, unchanged session state after
    rejection, and successful persisted branch/delete mutations for an
    admin-scoped WebChat client.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime size: M maintainer Maintainer-authored PR labels Jul 18, 2026
@steipete steipete added bug:behavior Incorrect behavior without a crash clawsweeper:needs-security-review ClawSweeper marked this issue as needing security-sensitive review. labels Jul 18, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 18, 2026
@steipete
steipete merged commit 1d492f5 into main Jul 18, 2026
137 of 143 checks passed
@steipete
steipete deleted the codex/webchat-session-mutation-scopes branch July 18, 2026 19:48
@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 3:45 PM ET / 19:45 UTC.

Summary
The PR removes WebChat-specific identity rejections from session-mutation handlers so negotiated operator scopes and existing parameter-aware scope checks authorize those RPCs.

PR surface: Source -75, Tests +42, Docs +5. Total -28 across 10 files.

Reproducibility: yes. from source, but not through an independently run live session: current main’s shared helper rejects qualifying WebChat clients before handlers execute, while the proposed test exercises the scope-policy outcomes.

Review metrics: 1 noteworthy metric.

  • Authorization flows: 10 session-mutation flows rerouted. A single deleted identity guard changes the authorization boundary across patch, delete, compaction, checkpoint, rewind, dispatch, and reclaim paths.

Stored data model
Persistent data-model change detected: serialized state: src/gateway/server-methods/sessions-shared.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted terminal or live output from a non-Control-UI WebChat client showing read/write/admin mutation outcomes.
  • Obtain explicit confirmation that scope-only authorization is the intended replacement for the removed identity ACL.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR reports focused Testbox integration tests and checks, but it does not include redacted after-fix evidence from a real authenticated non-Control-UI WebChat client; add terminal/live output for denied and permitted scope cases, with private details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging removes a defense-in-depth client-identity ACL, so correctness depends on every affected RPC remaining accurately classified by the central method-scope and parameter-aware authorization policy.
  • [P1] The PR supplies automated integration evidence but no redacted after-fix run from an authenticated non-Control-UI WebChat client at read, write, and admin scope levels.

Maintainer options:

  1. Require real scope-boundary proof (recommended)
    Before merge, provide a redacted authenticated non-Control-UI WebChat run showing read/write/admin outcomes and confirm the central method-scope registry still covers each affected mutation.
  2. Pause for authorization-policy review
    Pause the PR if maintainers want WebChat identity to remain an independent defense-in-depth authorization constraint.

Next step before merge

  • [P1] Protected-label security-boundary review and contributor-supplied real behavior proof are required before a merge decision; no narrow automated repair is indicated.

Maintainer decision needed

  • Question: Should session mutation authority for non-Control-UI WebChat clients be determined solely by negotiated operator scopes, with no additional client-ID or mode ACL?
  • Rationale: The patch intentionally removes an existing authorization layer rather than merely correcting handler wiring; repository policy treats security-boundary behavior as requiring explicit maintainer handling.
  • Likely owner: steipete — They authored both the documented authorization decision and the focused implementation, making them the closest available routing candidate for the intended boundary.
  • Options:
    • Adopt scope-only authorization (recommended): Confirm negotiated operator scopes as the sole session-mutation authorization contract, contingent on redacted real-client proof for denied and permitted scope levels.
    • Retain an identity-specific ACL: Keep the current WebChat client restriction and document the supported client identity policy instead of relying exclusively on scopes.

Security
Needs attention: The patch deliberately changes an authorization boundary; no separate exploit is shown, but merge should wait for explicit scope-policy confirmation and real-client evidence.

Review details

Best possible solution:

Adopt the centralized negotiated-scope contract only after a security owner confirms it is the intended WebChat boundary and a redacted live WebSocket/WebChat proof demonstrates both denied low-scope and permitted approved-scope mutations.

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

Yes from source, but not through an independently run live session: current main’s shared helper rejects qualifying WebChat clients before handlers execute, while the proposed test exercises the scope-policy outcomes.

Is this the best way to solve the issue?

Yes, conditionally: using one centralized scope policy avoids a competing client-reported identity ACL, provided the security owner confirms that no affected method lacks the required scope or dynamic check.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against bf2da1ce1af7.

Label changes

Label changes:

  • add P1: This fixes a current Gateway authorization regression that prevents approved third-party WebChat clients from performing permitted session-management work.
  • add merge-risk: 🚨 security-boundary: The diff removes an existing client-identity authorization gate and relies on the centralized scope policy for all affected mutations.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports focused Testbox integration tests and checks, but it does not include redacted after-fix evidence from a real authenticated non-Control-UI WebChat client; add terminal/live output for denied and permitted scope cases, with private details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: This fixes a current Gateway authorization regression that prevents approved third-party WebChat clients from performing permitted session-management work.
  • merge-risk: 🚨 security-boundary: The diff removes an existing client-identity authorization gate and relies on the centralized scope policy for all affected mutations.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports focused Testbox integration tests and checks, but it does not include redacted after-fix evidence from a real authenticated non-Control-UI WebChat client; add terminal/live output for denied and permitted scope cases, with private details redacted. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source -75, Tests +42, Docs +5. Total -28 across 10 files.

View PR surface stats
Area Files Added Removed Net
Source 8 9 84 -75
Tests 1 78 36 +42
Docs 1 5 0 +5
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 10 92 120 -28

Security concerns:

  • [medium] Confirm all mutation scopes before removing the identity ACL — src/gateway/server-methods/sessions-shared.ts:279
    Deleting the shared WebChat rejection makes method classification and dynamic scope checks the only authorization boundary for every affected session mutation. Verify the real negotiated-scope behavior before landing.
    Confidence: 0.9

What I checked:

  • Current authorization behavior: Current main retains a WebChat identity guard that rejects non-Control-UI WebChat clients before the individual session handlers can rely on their normal scope policy. (src/gateway/server-methods/sessions-shared.ts:279, bf2da1ce1af7)
  • Scope contract: Gateway documentation defines method scopes as the first authorization gate and states that handlers may apply stricter checks for the concrete mutation; the PR’s stated approach retains those centralized and parameter-aware checks. Public docs: docs/gateway/operator-scopes.md. (docs/gateway/operator-scopes.md:5, bf2da1ce1af7)
  • Focused branch validation: The PR adds a Gateway session-permissions integration test for read-scoped rejection, unchanged state after rejection, and successful admin-scoped branch/delete mutations; its body reports 132 focused tests and changed-surface checks passing. (src/gateway/server.sessions.permissions-hooks.test.ts:99, 6e20935154a7)
  • Related implementation candidate: The linked issue explicitly selects negotiated scopes rather than client identity as the session-mutation authorization contract, and this PR is its open implementation candidate. (fdbcf426d48a)

Likely related people:

  • steipete: Authored the linked scope-policy proposal and the focused Gateway authorization implementation under review; protected-label routing still requires broader maintainer confirmation. (role: recent gateway authorization contributor; confidence: medium; commits: 6e20935154a7, fdbcf426d48a; files: src/gateway/server-methods/sessions-shared.ts, src/gateway/server.sessions.permissions-hooks.test.ts, docs/gateway/operator-scopes.md)
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.

@steipete

steipete commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 18, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 19, 2026
@steipete
fix(gateway): honor scopes for WebChat session mutations (openclaw#11…
44ca376 
…0931)

* fix(gateway): authorize webchat session mutations by scope

* chore: keep gateway release note in PR body
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

bug:behavior Incorrect behavior without a crash clawsweeper:needs-security-review ClawSweeper marked this issue as needing security-sensitive review. docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Gateway: authorize session mutations by operator scope, not client identity

1 participant

@steipete