fix: honor uniqueId-scoped support exceptions on server-signed path - #3404
Conversation
The exception scope check introduced in #3323 requires exceptions.uniqueId to equal server.uniqueID, but the server-signed validation path returns before getUniqueId() ever runs and /api/info does not include a uniqueId field, so the local uniqueID is missing on that path and uniqueId-scoped exceptions were always disqualified. Resolve the workspace uniqueID from the server before validating when the payload carries a uniqueId-scoped exceptions block, persist it via WEBVIEW_SERVER_UNIQUE_ID_UPDATED so subsequent runs (including offline cache validation) keep working, and keep rejecting when the fetched value does not match. Also compare exceptions.domain case-insensitively (DNS names are case-insensitive per RFC 4343).
WalkthroughThis PR fixes exception-scope validation for server supported-versions checks. It makes ChangesException scope validation fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Main as updateSupportedVersionsData
participant Helper as withExceptionScopeUniqueId
participant Server as Server API
participant Store as Dispatch/Store
participant Validator as isServerVersionSupported
Main->>Helper: serverView, exceptions.uniqueId
alt uniqueId mismatch
Helper->>Server: fetch fresh uniqueId
Server-->>Helper: uniqueId
Helper->>Store: dispatch WEBVIEW_SERVER_UNIQUE_ID_UPDATED
Helper-->>Main: serverView with fresh uniqueID
else uniqueId matches
Helper-->>Main: original serverView
end
Main->>Validator: validate serverForValidation
Validator-->>Main: supported verdict
Main->>Store: dispatch WEBVIEW_SERVER_IS_SUPPORTED_VERSION
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/servers/supportedVersions/main.ts (1)
594-604: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor: possible duplicate
getUniqueIdfetch on the server-signed decode-error fallback.When
withExceptionScopeUniqueIdtriggers a fetch (uniqueId-scoped exception with a stale/missing local id) but the surroundingtrylater throws (e.g. a subsequent decode/validation error clearsserverEncoded), control falls through to the general path at Line 631, which callsgetUniqueIdagain for the same server. Not incorrect, just a redundant network round-trip on that edge path. Consider caching the resolved id to reuse downstream.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/servers/supportedVersions/main.ts` around lines 594 - 604, Avoid the redundant second unique-id lookup on the server-signed decode-error fallback path. In the flow around withExceptionScopeUniqueId, serverForValidation, and the later general retry/validation handling, cache the resolved uniqueId from the first fetch and reuse it if the same server reaches the fallback path again. Update the relevant server version validation logic so getUniqueId is only called once per server in this edge case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/servers/supportedVersions/main.ts`:
- Around line 594-604: Avoid the redundant second unique-id lookup on the
server-signed decode-error fallback path. In the flow around
withExceptionScopeUniqueId, serverForValidation, and the later general
retry/validation handling, cache the resolved uniqueId from the first fetch and
reuse it if the same server reaches the fallback path again. Update the relevant
server version validation logic so getUniqueId is only called once per server in
this edge case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6f50e92-93bd-48cb-bb3c-05d073033165
📒 Files selected for processing (2)
src/servers/supportedVersions/main.main.spec.tssrc/servers/supportedVersions/main.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; only create custom components when Fuselage does not provide what is needed.
Import UI components from@rocket.chat/fuselage.
CheckTheme.d.tsfor valid color tokens before using Fuselage theme colors.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs likeprocess.getuid(),process.getgid(),process.geteuid(), andprocess.getegid().
Only mock platform-specific APIs when defensive coding is not possible.
Use TypeScript strict mode.
Use React functional components with hooks.
Redux actions must follow the Flux Standard Action (FSA) pattern.
Use camelCase for file names and PascalCase for components.
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from@rocket.chat/fuselagefor UI work unless the design requires something Fuselage does not provide.
CheckTheme.d.tsfor valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.
Files:
src/servers/supportedVersions/main.tssrc/servers/supportedVersions/main.main.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
*.spec.tsfor renderer process tests.
Files:
src/servers/supportedVersions/main.main.spec.ts
**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
*.main.spec.tsfor main process tests.Main-process specs use
*.main.spec.ts.
Files:
src/servers/supportedVersions/main.main.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Renderer specs use
*.spec.ts/*.spec.tsx.
Files:
src/servers/supportedVersions/main.main.spec.ts
src/*/*/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Renderer specs must live in a Jest-matched nested path, such as
src/<module>/<subdir>/*.spec.ts(x); flatsrc/<module>/*.spec.tsfiles are not discovered by the currenttestMatch.
Files:
src/servers/supportedVersions/main.main.spec.ts
🔇 Additional comments (4)
src/servers/supportedVersions/main.ts (2)
356-362: LGTM!
506-528: LGTM!src/servers/supportedVersions/main.main.spec.ts (2)
374-539: LGTM!
1007-1041: LGTM!
Linux installer download |
macOS installer download |
1 participant
What
Workspaces with a uniqueId-scoped support exception (issued via Rocket.Chat Cloud) were shown the "Unsupported" screen on desktop 4.15.0–4.15.2 even though the exception was valid and present in the server's signed
supportedVersionspayload. Reported through a customer support escalation (workspace running 7.13.9 with a valid exception; mobile unaffected).Why
The exception scope check added in #3323 requires
exceptions.uniqueId === server.uniqueID. That check did not cover the common deployment path:server.uniqueIDis only written bygetUniqueId()insideupdateSupportedVersionsData, but the server-signed validation path returns before that call — so the local uniqueID is never populated for servers whose/api/infosucceeds.serverInfoResult.uniqueId, never applies:/api/infodoes not include auniqueIdfield (verified against servers running 7.13 and 8.7). The existing test fixtures included it, which is why the suite stayed green.With the local uniqueID undefined, every uniqueId-scoped exception was disqualified and the version fell through to the supported-versions list, which no longer contains the excepted version.
How
withExceptionScopeUniqueIdhelper on the server-signed path: when the payload carries a uniqueId-scoped exceptions block and the local uniqueID is missing or different, resolve it from the server (settings.public?_id=uniqueID), dispatchWEBVIEW_SERVER_UNIQUE_ID_UPDATEDso it persists for subsequent runs (including offline cache validation), and validate against the resolved identity.exceptions.uniqueId, the exception remains rejected. The cross-tenant scope check semantics from Fix "Unsupported Version" Block on Valid Servers After App Update #3323 are unchanged.exceptions.domainis now compared case-insensitively (DNS names are case-insensitive per RFC 4343);URL.hostnameis already lowercased.Tests
main.main.spec.ts: exception honored when/api/infoomitsuniqueId; stale persisted uniqueID re-resolved; mismatched uniqueID still rejected; no extra fetch when persisted value matches; domain case-insensitive matching.yarn jest src/servers/supportedVersions/main.main.spec.ts: 82/82 pass.tsc --noEmitandyarn lintclean.Summary by CodeRabbit