fix(realtime): re-check workspace role on mutating socket events by icecrasher321 · Pull Request #5080 · simstudioai/sim

icecrasher321 · 2026-06-16T00:49:37Z

Summary

Realtime server authorized a user once at join-workflow and cached the
workspace role in room presence, then trusted that cached role for every
subsequent workflow-operation, subblock-update, and variable-update with no
DB re-check and no revocation path. A collaborator removed from the workspace
or downgraded to read kept live write access on an open connection until they
disconnected.

Re-validate the role against the permissions table on each mutating event,
cached per pod for 30s to keep the hot path cheap. Revoked users are denied
(fail-closed); a transient DB error falls back to the last-known role so a
blip doesn't block legitimate editors.

Type of Change

Other: Security Improvement

Testing

Tested manually

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

vercel · 2026-06-16T00:49:42Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jun 16, 2026 12:59am

cursor · 2026-06-16T00:49:45Z

PR Summary

High Risk
This changes authorization on every persisted realtime mutation; incorrect cache or fallback logic could wrongly allow writes or block editors, with up to ~30s stale access across pods by design.

Overview
Fixes a gap where mutating realtime events trusted the workspace role cached at join-workflow, so removed or downgraded collaborators could keep writing until disconnect.

workflow-operation, subblock-update, and variable-update now go through checkWorkflowOperationPermission, which re-reads workspace permissions via authorizeWorkflowByWorkspacePermission (30s per-pod cache on userId:workflowId). Revoked users are denied; downgrades still allow read-only ops like position updates. On DB errors, the gate prefers the last cached decision (including revocations) over the stale join-time role.

Tests cover caching, TTL refresh, revocation/downgrade behavior, and DB-failure fallbacks; integration mocks were updated for the async permission check.

^{Reviewed by Cursor Bugbot for commit 21c5e8d. Configure here.}

greptile-apps · 2026-06-16T00:53:59Z

Greptile Summary

This PR addresses a real-time authorization gap where a collaborator's workspace role was checked only at socket join and then trusted indefinitely from cached presence state. The fix re-validates the role on every mutating operation via a per-pod in-memory cache with a 30-second TTL, backed by comprehensive unit tests that cover caching, TTL expiry, downgrade, revocation, and the DB-error fallback path.

permissions.ts introduces resolveCurrentWorkflowRole (module-level Map, 30s TTL, soft 5000-entry cap with opportunistic purge) and checkWorkflowOperationPermission, which gates all three mutating handler paths; the DB-error fallback correctly reads the last recorded (possibly expired) cache entry — including a null for a previously revoked user — so a transient DB blip neither blocks legitimate editors nor resurrects revoked access.
operations.ts, subblocks.ts, variables.ts are updated to await the new async permission check in place of the old synchronous checkRolePermission call against the stale presence role.
Tests use unique workflowId values per test case to prevent cross-test cache pollution and cover the key edge cases (revocation survives TTL + DB error, join-time fallback only fires when no prior decision exists).

Confidence Score: 5/5

Safe to merge — the core revocation logic is fail-closed, TTL behavior is well-tested, and the DB-error fallback correctly preserves a recorded null rather than reverting to join-time role

The security-relevant code path is straightforward and covered by tests that simulate cache misses, TTL expiry, revocation, and DB errors in sequence. The two observations are minor: an unnecessary function export and a low-probability DB-call stampede on TTL expiry that does not affect correctness.

permissions.ts warrants a second look at the concurrent-expiry path and the exported resolveCurrentWorkflowRole

Important Files Changed

Filename	Overview
apps/realtime/src/middleware/permissions.ts	Adds resolveCurrentWorkflowRole (per-pod 30s TTL cache) and checkWorkflowOperationPermission; DB-error fallback correctly reads the last known (possibly expired) cache entry; minor stampede risk on concurrent cache expiry
apps/realtime/src/middleware/permissions.test.ts	Comprehensive new tests for checkWorkflowOperationPermission cover caching, TTL expiry, revocation, fallback, and the DB-error-after-revocation scenario with unique workflowIds per test to prevent cache leakage
apps/realtime/src/handlers/operations.ts	Correctly swaps synchronous checkRolePermission for async checkWorkflowOperationPermission; warning log updated to use DB-validated role from permissionCheck.role
apps/realtime/src/handlers/subblocks.ts	Correctly awaits checkWorkflowOperationPermission for subblock-update events; no other changes needed
apps/realtime/src/handlers/variables.ts	Correctly awaits checkWorkflowOperationPermission for variable-update events; no other changes needed
apps/realtime/src/index.test.ts	Adds checkWorkflowOperationPermission mock returning a resolved promise; correctly mirrors the real async signature

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as Client Socket
    participant H as Handler (ops/subblocks/vars)
    participant P as checkWorkflowOperationPermission
    participant Cache as roleCache (in-memory, 30s TTL)
    participant DB as Database

    C->>H: mutating socket event
    H->>P: checkWorkflowOperationPermission(userId, workflowId, op, fallbackRole)
    P->>Cache: get(userId:workflowId)

    alt Cache HIT (within TTL)
        Cache-->>P: "{role, expiresAt}"
        P-->>H: "{allowed, role}"
    else Cache MISS or EXPIRED
        P->>DB: authorizeWorkflowByWorkspacePermission()
        alt DB succeeds — user still has access
            DB-->>P: "{allowed: true, workspacePermission: role}"
            P->>Cache: "set entry with expiresAt = now + 30s"
            P-->>H: checkRolePermission(role, op)
        else DB succeeds — user revoked
            DB-->>P: "{allowed: false, workspacePermission: null}"
            P->>Cache: set entry with role: null
            P-->>H: "{allowed: false, reason: revoked, role: null}"
        else DB error — prior recorded decision exists
            P->>Cache: get last known entry (possibly expired)
            Cache-->>P: prior role (null if previously revoked)
            P-->>H: "{allowed based on last known role}"
        else DB error — no prior record
            P-->>H: "{allowed based on fallbackRole from join time}"
        end
    end

    H-->>C: emit result or operation-forbidden

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as Client Socket
    participant H as Handler (ops/subblocks/vars)
    participant P as checkWorkflowOperationPermission
    participant Cache as roleCache (in-memory, 30s TTL)
    participant DB as Database

    C->>H: mutating socket event
    H->>P: checkWorkflowOperationPermission(userId, workflowId, op, fallbackRole)
    P->>Cache: get(userId:workflowId)

    alt Cache HIT (within TTL)
        Cache-->>P: "{role, expiresAt}"
        P-->>H: "{allowed, role}"
    else Cache MISS or EXPIRED
        P->>DB: authorizeWorkflowByWorkspacePermission()
        alt DB succeeds — user still has access
            DB-->>P: "{allowed: true, workspacePermission: role}"
            P->>Cache: "set entry with expiresAt = now + 30s"
            P-->>H: checkRolePermission(role, op)
        else DB succeeds — user revoked
            DB-->>P: "{allowed: false, workspacePermission: null}"
            P->>Cache: set entry with role: null
            P-->>H: "{allowed: false, reason: revoked, role: null}"
        else DB error — prior recorded decision exists
            P->>Cache: get last known entry (possibly expired)
            Cache-->>P: prior role (null if previously revoked)
            P-->>H: "{allowed based on last known role}"
        else DB error — no prior record
            P-->>H: "{allowed based on fallbackRole from join time}"
        end
    end

    H-->>C: emit result or operation-forbidden

_{Reviews (2): Last reviewed commit: "address comments" | Re-trigger Greptile}

icecrasher321 · 2026-06-16T00:59:42Z

@greptile

icecrasher321 · 2026-06-16T00:59:45Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 21c5e8d. Configure here.}

fix(realtime): re-check workspace role on mutating socket events

5996aa2

cursor Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread apps/realtime/src/middleware/permissions.ts Outdated

greptile-apps Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread apps/realtime/src/middleware/permissions.ts Outdated

Comment thread apps/realtime/src/middleware/permissions.test.ts

address comments

21c5e8d

vercel Bot temporarily deployed to Preview June 16, 2026 00:59 Inactive

cursor Bot reviewed Jun 16, 2026

View reviewed changes

icecrasher321 merged commit d14bc78 into staging Jun 16, 2026
16 checks passed

waleedlatif1 deleted the improvements/sockets-sec branch June 16, 2026 18:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(realtime): re-check workspace role on mutating socket events#5080

fix(realtime): re-check workspace role on mutating socket events#5080
icecrasher321 merged 2 commits into
stagingfrom
improvements/sockets-sec

icecrasher321 commented Jun 16, 2026

Uh oh!

vercel Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

cursor Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

icecrasher321 commented Jun 16, 2026

Uh oh!

icecrasher321 commented Jun 16, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

icecrasher321 commented Jun 16, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

Uh oh!

greptile-apps Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

icecrasher321 commented Jun 16, 2026

Uh oh!

icecrasher321 commented Jun 16, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Jun 16, 2026 •

edited

Loading

cursor Bot commented Jun 16, 2026 •

edited

Loading

greptile-apps Bot commented Jun 16, 2026 •

edited

Loading