From d51237eebda25a803f1e3b636b57c84fcdfab66b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 21 Aug 2026 20:44:45 +0000 Subject: [PATCH] OUT-4093: Guard localStorage migration when third-party storage is denied migrateAssignees() was the only assignee-cache path still reading localStorage without checking document.hasStorageAccess(). In the embedded Copilot iframe this throws SecurityError and surfaces in Sentry when third-party cookies/storage are blocked. Extract hasAssigneeStorageAccess() shared by get/set/migrate helpers, skip migration when access is denied, and wrap localStorage calls in try/catch. Also stop calling requestStorageAccess() on mount since it requires a user gesture and cannot succeed during AssigneeCacheGetter initialization. Co-authored-by: Neil Raina --- src/app/_cache/forageStorage.test.ts | 102 +++++++++++++++++++++++++++ src/app/_cache/forageStorage.ts | 51 ++++++++------ 2 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 src/app/_cache/forageStorage.test.ts diff --git a/src/app/_cache/forageStorage.test.ts b/src/app/_cache/forageStorage.test.ts new file mode 100644 index 000000000..97f38ffdc --- /dev/null +++ b/src/app/_cache/forageStorage.test.ts @@ -0,0 +1,102 @@ +import localforage from 'localforage' +import { getAssignees, migrateAssignees, setAssignees } from './forageStorage' + +jest.mock('localforage', () => ({ + __esModule: true, + default: { + config: jest.fn(), + getItem: jest.fn(), + setItem: jest.fn(), + }, +})) + +const mockedLocalforage = jest.mocked(localforage) + +describe('forageStorage', () => { + const originalWindow = global.window + const originalDocument = global.document + const originalLocalStorage = global.localStorage + + const defineBrowserGlobals = ({ + hasStorageAccess, + requestStorageAccess = jest.fn(), + }: { + hasStorageAccess: jest.Mock, []> + requestStorageAccess?: jest.Mock, []> + }) => { + Object.defineProperty(global, 'window', { + configurable: true, + value: {}, + }) + Object.defineProperty(global, 'document', { + configurable: true, + value: { + hasStorageAccess, + requestStorageAccess, + }, + }) + Object.defineProperty(global, 'localStorage', { + configurable: true, + value: { + getItem: jest.fn(), + removeItem: jest.fn(), + }, + }) + } + + afterEach(() => { + jest.clearAllMocks() + Object.defineProperty(global, 'window', { + configurable: true, + value: originalWindow, + }) + Object.defineProperty(global, 'document', { + configurable: true, + value: originalDocument, + }) + Object.defineProperty(global, 'localStorage', { + configurable: true, + value: originalLocalStorage, + }) + }) + + it('skips assignee reads when the browser denies storage access', async () => { + const requestStorageAccess = jest.fn, []>() + defineBrowserGlobals({ + hasStorageAccess: jest.fn().mockResolvedValue(false), + requestStorageAccess, + }) + + await expect(getAssignees('client.company')).resolves.toEqual([]) + + expect(mockedLocalforage.getItem).not.toHaveBeenCalled() + expect(requestStorageAccess).not.toHaveBeenCalled() + }) + + it('skips assignee writes when the browser denies storage access', async () => { + const requestStorageAccess = jest.fn, []>() + defineBrowserGlobals({ + hasStorageAccess: jest.fn().mockResolvedValue(false), + requestStorageAccess, + }) + + await setAssignees('client.company', [{ id: 'assignee-1' }]) + + expect(mockedLocalforage.setItem).not.toHaveBeenCalled() + expect(requestStorageAccess).not.toHaveBeenCalled() + }) + + it('does not touch localStorage migration when storage access is denied', async () => { + const requestStorageAccess = jest.fn, []>() + defineBrowserGlobals({ + hasStorageAccess: jest.fn().mockResolvedValue(false), + requestStorageAccess, + }) + + await migrateAssignees('client.company') + + expect(global.localStorage.getItem).not.toHaveBeenCalled() + expect(mockedLocalforage.setItem).not.toHaveBeenCalled() + expect(requestStorageAccess).not.toHaveBeenCalled() + }) +}) diff --git a/src/app/_cache/forageStorage.ts b/src/app/_cache/forageStorage.ts index e8baf6391..de7c35978 100644 --- a/src/app/_cache/forageStorage.ts +++ b/src/app/_cache/forageStorage.ts @@ -8,52 +8,63 @@ localforage.config({ storeName: 'assignees', }) +async function hasAssigneeStorageAccess() { + if (typeof window === 'undefined') return false + + if (typeof document.hasStorageAccess !== 'function') { + return true + } + + try { + const hasAccess = await document.hasStorageAccess() + if (!hasAccess) { + console.info('Browser has no storage access') + } + return hasAccess + } catch { + return false + } +} + export async function migrateAssignees(lookupKey: string) { + if (!(await hasAssigneeStorageAccess())) return + const lKey = `assignees.${lookupKey}` - const existing = localStorage.getItem(lKey) - if (existing) { - try { + try { + const existing = localStorage.getItem(lKey) + + if (existing) { const parsed = JSON.parse(existing) await localforage.setItem(lKey, parsed) localStorage.removeItem(lKey) - } catch (err) { - console.error('Migration failed', err) } + } catch (err) { + console.error('Migration failed', err) } -} //a utility function to migrate existing assignee data from localStorage to localForage +} export async function getAssignees(lookupKey: string): Promise { - if (typeof window === 'undefined') return [] + if (!(await hasAssigneeStorageAccess())) return [] try { - if (!(await document.hasStorageAccess())) { - console.info('Browswer has no storage access') - await document.requestStorageAccess() - } - return (await localforage.getItem(`assignees.${lookupKey}`)) ?? [] } catch (error: unknown) { console.error( - "Storage access not granted. Under Chrome's Settings > Privacy and Security, make sure 'Third-party cookies' is allowed.", + "Storage access not granted. Under Chrome's Settings > Privacy and Security, make sure 'Third-party cookies' are allowed.", ) return [] } } export async function setAssignees(lookupKey: string, value: any) { - if (typeof window === 'undefined') return + if (!(await hasAssigneeStorageAccess())) return try { - if (!(await document.hasStorageAccess())) { - console.info('Browswer has no storage access') - await document.requestStorageAccess() - } - return await localforage.setItem(`assignees.${lookupKey}`, value) } catch (error: unknown) { console.error( - "Storage access not granted. Under Chrome's Settings > Privacy and Security, make sure 'Third-party cookies' is allowed.", + "Storage access not granted. Under Chrome's Settings > Privacy and Security, make sure 'Third-party cookies' are allowed.", ) } }