fix(web): clear share and quota stores on logout#265
Merged
Conversation
Share store's clearShares() was defined but never called during logout, causing stale share data to persist across sessions. Quota store had no reset method at all. Both are now cleared in the useAuth logout sequence. Closes M2 tech debt items from milestone audit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: b5b5f70621d5
Quick task completed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 89f45663f542
WalkthroughAdds a centralized Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthHook as useAuth.ts
participant APIClient as api/client.ts
participant Cleaner as clear-user-stores.ts
participant Stores as "Folders/Vault/Sync/Device/Bin/Shares/Quota/MFA"
User->>AuthHook: click Logout
AuthHook->>APIClient: call backend logout
APIClient-->>AuthHook: logout response (success/failure)
AuthHook->>Cleaner: clearAllUserStores()
Cleaner->>Stores: clear sensitive stores (folders, vault keys)
Cleaner->>Stores: reset/clear remaining stores (sync, device registry, bin, shares, quota, MFA)
Cleaner->>AuthHook: clear auth state (zero keys, isAuthenticated=false)
AuthHook->>User: navigate to login
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #265 +/- ##
==========================================
+ Coverage 46.31% 47.92% +1.61%
==========================================
Files 106 109 +3
Lines 8266 8321 +55
Branches 591 639 +48
==========================================
+ Hits 3828 3988 +160
+ Misses 4271 4163 -108
- Partials 167 170 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses user-scoped state leakage in the web app by ensuring share and quota Zustand stores are cleared during logout, preventing stale data from persisting across sessions for different users.
Changes:
- Added a
reset()action to the quota store to restore initial quota state on logout. - Wired share/quota store cleanup into the
useAuthlogout flow (both success and error paths). - Documented the quick task plan/summary and updated
.planning/STATE.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/stores/quota.store.ts | Adds reset() to clear quota state back to defaults. |
| apps/web/src/hooks/useAuth.ts | Calls clearShares() and quota.reset() during logout (try + catch). |
| .planning/quick/023-m2-tech-debt-store-logout-cleanup/023-SUMMARY.md | Records what changed and how it was verified. |
| .planning/quick/023-m2-tech-debt-store-logout-cleanup/023-PLAN.md | Documents the intended work items and verification steps. |
| .planning/STATE.md | Adds Quick Task 023 entry. |
You can also share your feedback on Copilot code review. Take the survey.
- Extract clearAllUserStores() helper used by both useAuth logout and apiClient 401 interceptor, ensuring all stores are cleared on every logout path (addresses Copilot comments #1 and #2) - Extract initialState constant in quota store so reset() and init values cannot drift (addresses Copilot comment #4) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: ef84f2610701
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/stores/quota.store.ts (1)
24-37:⚠️ Potential issue | 🟠 MajorPrevent stale
fetchQuota()responses from repopulating state after reset.
reset()on Line [50] clears state, but an earlierfetchQuota()(Lines [24]-[37]) can still resolve later and overwrite quota with pre-logout data.💡 Proposed fix (session version guard)
const initialState = { usedBytes: 0, limitBytes: DEFAULT_LIMIT_BYTES, remainingBytes: DEFAULT_LIMIT_BYTES, loading: false, error: null as string | null, }; +let quotaSessionVersion = 0; + type QuotaState = typeof initialState & { fetchQuota: () => Promise<void>; removeUsage: (bytes: number) => void; canUpload: (bytes: number) => boolean; reset: () => void; }; export const useQuotaStore = create<QuotaState>((set, get) => ({ ...initialState, fetchQuota: async () => { + const requestVersion = quotaSessionVersion; set({ loading: true, error: null }); try { const quota: QuotaResponse = await vaultApi.getQuota(); + if (requestVersion !== quotaSessionVersion) return; set({ usedBytes: quota.usedBytes, limitBytes: quota.limitBytes, remainingBytes: quota.remainingBytes, loading: false, }); } catch { + if (requestVersion !== quotaSessionVersion) return; set({ error: 'Failed to fetch quota', loading: false }); } }, @@ - reset: () => set(initialState), + reset: () => { + quotaSessionVersion += 1; + set({ ...initialState }); + }, }));Based on learnings: "Store React session-scoped state (ECDSA private key) in memory via context/Zustand/Redux and clear it completely on logout - do not persist to disk or localStorage."
Also applies to: 50-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/stores/quota.store.ts` around lines 24 - 37, fetchQuota can resolve after reset() and repopulate cleared quota state; fix by adding a session/version guard: add a numeric sessionVersion in the store that's incremented in reset(), capture the current sessionVersion at start of fetchQuota (before awaiting vaultApi.getQuota()), and only call set({ usedBytes, limitBytes, remainingBytes, loading: false }) if the captured sessionVersion still equals the store's current sessionVersion; reference functions/identifiers fetchQuota, reset, vaultApi.getQuota and the state keys usedBytes/limitBytes/remainingBytes so the stale response is ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/src/stores/quota.store.ts`:
- Around line 24-37: fetchQuota can resolve after reset() and repopulate cleared
quota state; fix by adding a session/version guard: add a numeric sessionVersion
in the store that's incremented in reset(), capture the current sessionVersion
at start of fetchQuota (before awaiting vaultApi.getQuota()), and only call
set({ usedBytes, limitBytes, remainingBytes, loading: false }) if the captured
sessionVersion still equals the store's current sessionVersion; reference
functions/identifiers fetchQuota, reset, vaultApi.getQuota and the state keys
usedBytes/limitBytes/remainingBytes so the stale response is ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31f565b6-4208-4b35-b0d1-5df857441693
📒 Files selected for processing (4)
apps/web/src/hooks/useAuth.tsapps/web/src/lib/api/client.tsapps/web/src/lib/clear-user-stores.tsapps/web/src/stores/quota.store.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/hooks/useAuth.ts
Prevents in-flight fetchQuota() responses from repopulating cleared state after reset/logout. Flagged by both Copilot and CodeRabbit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 4d1d4cd4dad6
Owner
Author
|
Re: CodeRabbit review — the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/stores/quota.store.ts (1)
29-33: Consider guarding against out-of-order responses within the same session.
quotaSessionVersionprotects reset/logout flows, but two concurrentfetchQuota()calls in one session can still race, allowing an older response to overwrite a newer one. A per-request monotonic id check would close this reliability gap.Possible refinement
let quotaSessionVersion = 0; +let latestQuotaRequestId = 0; fetchQuota: async () => { const requestVersion = quotaSessionVersion; + const requestId = ++latestQuotaRequestId; set({ loading: true, error: null }); try { const quota: QuotaResponse = await vaultApi.getQuota(); - if (requestVersion !== quotaSessionVersion) return; + if (requestVersion !== quotaSessionVersion || requestId !== latestQuotaRequestId) return; set({ usedBytes: quota.usedBytes, limitBytes: quota.limitBytes, remainingBytes: quota.remainingBytes, loading: false, }); } catch { - if (requestVersion !== quotaSessionVersion) return; + if (requestVersion !== quotaSessionVersion || requestId !== latestQuotaRequestId) return; set({ error: 'Failed to fetch quota', loading: false }); } },Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/stores/quota.store.ts` around lines 29 - 33, The current logic uses quotaSessionVersion to guard across sessions but allows two concurrent fetchQuota() calls in the same session to race; add a per-request monotonic id: introduce a store-scoped counter (e.g., quotaRequestId) that you increment and capture as localRequestId at the start of fetchQuota(), store the latest seen id (e.g., latestQuotaRequestId) in state, and before applying any async response compare localRequestId === latestQuotaRequestId; if not equal, discard the response. Apply the same request-id check to the other quota-fetching call mentioned (the one around the code at the other site) so older responses in the same session cannot overwrite newer ones.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/stores/quota.store.ts`:
- Around line 29-33: The current logic uses quotaSessionVersion to guard across
sessions but allows two concurrent fetchQuota() calls in the same session to
race; add a per-request monotonic id: introduce a store-scoped counter (e.g.,
quotaRequestId) that you increment and capture as localRequestId at the start of
fetchQuota(), store the latest seen id (e.g., latestQuotaRequestId) in state,
and before applying any async response compare localRequestId ===
latestQuotaRequestId; if not equal, discard the response. Apply the same
request-id check to the other quota-fetching call mentioned (the one around the
code at the other site) so older responses in the same session cannot overwrite
newer ones.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83c85ab4-797f-4cde-8501-fcafd4dd5d5a
📒 Files selected for processing (1)
apps/web/src/stores/quota.store.ts
6 tasks
Closed
This was referenced Mar 26, 2026
Merged
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.
Summary
clearShares()) was defined but never called on logout — stale share data persisted across sessionsreset()method — previous session's quota data leaked into new sessionsuseAuthlogout sequence (try + catch paths)Closes M2 tech debt items from milestone audit.
Test plan
pnpm --filter web buildpasses🤖 Generated with Claude Code
Summary by CodeRabbit