feat(gateway): thread authenticated user identity into presence#111179
Conversation
openclaw-barnacle
Bot
added
app: web-ui
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: feat(gateway): thread authenticated user identity into presence This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 289095234a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ...(authenticatedUserId | ||
| ? { user: { id: authenticatedUserId, email: authenticatedUserId } } | ||
| : {}), |
There was a problem hiding this comment.
When the same presenceKey reconnects through a userless path after identity-bearing auth—for example, a paired Control UI device switches from Tailscale Serve to an explicit token while allowTailscale is enabled—this branch omits user. upsertPresence merges the update over the existing entry (src/infra/system-presence.ts:269-272), so the prior user's identity survives and the new session is falsely attributed to that user in presence snapshots. Explicitly clear user when authenticatedUserId is absent.
Useful? React with 👍 / 👎.
| scopes, | ||
| instanceId: device?.id ?? instanceId, | ||
| ...(authenticatedUserId | ||
| ? { user: { id: authenticatedUserId, email: authenticatedUserId } } |
There was a problem hiding this comment.
For trusted-proxy configurations whose identity header contains a username or subject ID (both are allowed because authorizeTrustedProxy only requires a nonblank string), this publishes that value as user.email. The Tailscale auth contract likewise permits identities such as peter. Downstream clients therefore receive a knowingly invalid email field; keep the opaque value in id and populate email only from a contract that actually guarantees an email address.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 11:52 PM ET / July 19, 2026, 03:52 UTC. Summary PR surface: Source +34, Tests +185, Other +4. Total +223 across 9 files. Reproducibility: yes. from source: connect once with an authenticated identity, then reconnect the same presence key through a userless auth path; the merge-based presence update can retain the old Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Make Do we have a high-confidence way to reproduce the issue? Yes, from source: connect once with an authenticated identity, then reconnect the same presence key through a userless auth path; the merge-based presence update can retain the old Is this the best way to solve the issue? No. The intended threading is appropriate, but the patch must clear stale identity on userless reconnects and must not claim opaque auth subjects are email addresses. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 63f8b8454b75. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +34, Tests +185, Other +4. Total +223 across 9 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
|
|
Merged via squash.
|
…claw#111179) * feat(gateway): thread authenticated user identity into presence * feat(gateway): regenerate Swift protocol bindings for presence user
1 participant
What Problem This Solves
Trusted-proxy gateway auth verifies a user's email and then drops it:
GatewayAuthResult.useris used for the allow/deny decision and never attached to the connection. On shared team gateways this makes people invisible — presence is device-flavored only, and there is no gateway-side audit line tying a connection to the authenticated identity.Part of the multiplayer-identity program: #111133 (PR 1 of the train).
Why This Change Was Made
This is the keystone the rest of the identity program (viewer avatars, prompt attribution, profiles) builds on: carry the already-verified identity into WS connection state and expose it on the connection's presence entry, strictly optionally.
PresenceEntrygains an optional closeduser: { id, email?, name?, avatarUrl? }object.idis opaque: the authenticated email today, a durable profile id once the profiles PR lands. Clients group presence entries byuser.id(the gateway keeps reporting one truthful entry per connection).authenticatedUserIdwhen auth carried a user (trusted-proxy and Tailscale modes).Identity is purely optional: token/password/no-auth connections carry no identity and behave byte-identically to before.
Compatibility: additive optional field on a closed schema, matching established convention (e.g.
appliedConfigHashin #107577). Audit of consumers: Control UI casts presence entries without schema validation; macOS/iOS SwiftCodableignores unknown keys;packages/gateway-clientframe guards are explicitly additive-tolerant; SDK exposes hello asunknown; no in-tree consumer validates inbound snapshots against the closed schema, and@openclaw/gateway-protocolis not npm-published.User Impact
No visible change yet on its own. Shared-gateway operators get audit log lines for identified connects/disconnects; presence snapshots start carrying identity for UIs to render (follow-up PRs).
Evidence
node scripts/run-vitest.mjs packages/gateway-protocol/src/schema/snapshot.test.ts— schema round-trip with and withoutuser(2 tests).node scripts/run-vitest.mjs src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts— trusted-proxy connection projectsuser.id; token connection has nouser(36 tests).node scripts/run-vitest.mjs src/gateway/server/ws-connection.test.ts— disconnect audit log (17 tests).