fix(ui): scope last workspace cookie by user#2793
Open
daryllimyt wants to merge 1 commit into
Open
Conversation
|
✅ No security or compliance issues detected. Reviewed everything up to dd8687a. Security Overview
Detected Code Changes
|
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 3/5
- There is a concrete regression risk in
frontend/src/lib/last-workspace.ts: the!userIdcheck can treat authenticated-but-still-loading users like anonymous users and read/write the legacy shared cookie. - Because this is severity 6/10 with fairly strong confidence (7/10) and can affect workspace selection behavior for signed-in sessions, the change carries some user-facing risk until this condition is tightened.
- Pay close attention to
frontend/src/lib/last-workspace.ts- auth-loading vs anonymous state handling may route users to the wrong cookie path.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/lib/last-workspace.ts">
<violation number="1" location="frontend/src/lib/last-workspace.ts:18">
P2: Using `!userId` to select the legacy cookie conflates anonymous and auth-loading states, so authenticated sessions can still hit the shared cookie before user data resolves.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * previously selected workspace across users. | ||
| */ | ||
| export function getLastWorkspaceIdForUser(userId?: string): string | undefined { | ||
| if (!userId) { |
Contributor
There was a problem hiding this comment.
P2: Using !userId to select the legacy cookie conflates anonymous and auth-loading states, so authenticated sessions can still hit the shared cookie before user data resolves.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/lib/last-workspace.ts, line 18:
<comment>Using `!userId` to select the legacy cookie conflates anonymous and auth-loading states, so authenticated sessions can still hit the shared cookie before user data resolves.</comment>
<file context>
@@ -0,0 +1,47 @@
+ * previously selected workspace across users.
+ */
+export function getLastWorkspaceIdForUser(userId?: string): string | undefined {
+ if (!userId) {
+ return Cookies.get(LEGACY_LAST_WORKSPACE_COOKIE)
+ }
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
__tracecat:workspaces:last-viewed), which could leak a previously selected workspace across different authenticated users in the same browser.Description
frontend/src/lib/last-workspace.tswhich exposesgetLastWorkspaceIdForUser,setLastWorkspaceIdForUser, andclearLastWorkspaceIdForUser, and uses per-user cookie names__tracecat:workspaces:last-viewed:<userId>for authenticated users.useWorkspaceManagerinfrontend/src/lib/hooks.tsxto read the current user viauseAuth()and route cookie get/set/clear through the new helpers instead of the shared cookie.Cookiesusage for the shared key inuseWorkspaceManagerso workspace persistence is user-scoped.frontend/tests/last-workspace.test.tsto verify per-user storage, legacy anonymous behavior, and that clearing one user's value does not clear another's.Testing
pnpm -C frontend exec biome check --write src/lib/hooks.tsx src/lib/last-workspace.ts tests/last-workspace.test.tswhich completed and autofixed one file. (success)pnpm -C frontend test -- last-workspace.test.tsand all tests passed (3 passed). (success)pnpm -C frontend run typecheckwhich completed without type errors. (success)Codex Task
Summary by cubic
Scopes the “last viewed workspace” cookie by user to prevent cross-account leakage in the same browser. Keeps the legacy shared cookie for anonymous sessions.
__tracecat:workspaces:last-viewed:<userId>.getLastWorkspaceIdForUser,setLastWorkspaceIdForUser,clearLastWorkspaceIdForUserinfrontend/src/lib/last-workspace.ts.useWorkspaceManagerto useuseAuth()and the new helpers; remove directCookiesusage.__tracecat:workspaces:last-viewedfor anonymous users.frontend/tests/last-workspace.test.tsfor per-user storage, legacy behavior, and scoped clearing.Written for commit dd8687a. Summary will update on new commits.