From a9e0ea4be912801a65216d59ba361ac7c566fa23 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:07:00 +0100 Subject: [PATCH 01/12] fix(callback): validate callback requests --- web/src/store/callbackActions.ts | 75 +++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index a0f5bcd19a..a1cc448288 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -25,16 +25,80 @@ import { useServerStore } from '~/store/server'; import { useUpdateOsActionsStore } from '~/store/updateOsActions'; const callbackEncryptionKey = import.meta.env.VITE_CALLBACK_KEY ?? ''; +const callbackNonceParameter = 'callback_nonce'; +const callbackNonceStoragePrefix = 'unraid-callback-nonce:'; +const callbackNonceTtlMs = 10 * 60 * 1000; export const useCallbackActionsStore = defineStore('callbackActions', () => { const { - send, + send: sendCallback, watcher: providedWatcher, - generateUrl, + generateUrl: generateCallbackUrl, } = useCallback({ encryptionKey: callbackEncryptionKey, }); + const createCallbackNonce = (): string => { + const bytes = new Uint8Array(16); + window.crypto.getRandomValues(bytes); + return Array.from(bytes, (byte) => byte.toString(16).padStart(2, '0')).join(''); + }; + + const rememberCallbackNonce = (nonce: string) => { + sessionStorage.setItem( + `${callbackNonceStoragePrefix}${nonce}`, + String(Date.now() + callbackNonceTtlMs) + ); + }; + + const senderWithCallbackNonce = (sender?: string): string | undefined => { + if (typeof window === 'undefined') { + return sender; + } + + const nonce = createCallbackNonce(); + const senderUrl = new URL(sender ?? window.location.href); + senderUrl.hash = ''; + senderUrl.searchParams.delete('data'); + senderUrl.searchParams.set(callbackNonceParameter, nonce); + rememberCallbackNonce(nonce); + return senderUrl.toString(); + }; + + const send = (...args: Parameters) => { + const [url, payload, redirectType, sendType, sender] = args; + return sendCallback(url, payload, redirectType, sendType, senderWithCallbackNonce(sender)); + }; + + const generateUrl = (...args: Parameters) => { + const [url, payload, sendType, sender] = args; + return generateCallbackUrl(url, payload, sendType, senderWithCallbackNonce(sender)); + }; + + const validateAndConsumeCallbackNonce = (payload: QueryPayloads): boolean => { + if (!isExternalCallbackPayload(payload)) { + return true; + } + + let senderUrl: URL; + try { + senderUrl = new URL(payload.sender); + } catch { + return false; + } + + const nonce = senderUrl.searchParams.get(callbackNonceParameter); + if (!nonce) { + // Callbacks from older clients do not carry a nonce. + return true; + } + + const storageKey = `${callbackNonceStoragePrefix}${nonce}`; + const expiresAt = Number(sessionStorage.getItem(storageKey)); + sessionStorage.removeItem(storageKey); + return Number.isSafeInteger(expiresAt) && expiresAt >= Date.now(); + }; + // Lazy store initialization - call stores inside functions to avoid circular dependencies const getAccountStore = () => useAccountStore(); const getInstallKeyStore = () => useInstallKeyStore(); @@ -66,6 +130,13 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { }; const saveCallbackData = async (decryptedData?: QueryPayloads) => { + if (decryptedData && !validateAndConsumeCallbackNonce(decryptedData)) { + callbackData.value = undefined; + callbackError.value = 'Callback request is not active or has expired'; + callbackStatus.value = 'error'; + return; + } + if (decryptedData) { callbackData.value = decryptedData; } From 0470674020d62d75cfe24b9b55ccaaf676a103e1 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:45:18 +0100 Subject: [PATCH 02/12] fix(callback): preserve legacy return flows --- web/src/store/callbackActions.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index a1cc448288..dba123fb2a 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -57,7 +57,7 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { } const nonce = createCallbackNonce(); - const senderUrl = new URL(sender ?? window.location.href); + const senderUrl = new URL(sender ?? window.location.href, window.location.origin); senderUrl.hash = ''; senderUrl.searchParams.delete('data'); senderUrl.searchParams.set(callbackNonceParameter, nonce); @@ -67,11 +67,17 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const send = (...args: Parameters) => { const [url, payload, redirectType, sendType, sender] = args; + if (sendCallback.length < 5) { + return sendCallback(url, payload, redirectType, sendType); + } return sendCallback(url, payload, redirectType, sendType, senderWithCallbackNonce(sender)); }; const generateUrl = (...args: Parameters) => { const [url, payload, sendType, sender] = args; + if (generateCallbackUrl.length < 4) { + return generateCallbackUrl(url, payload, sendType); + } return generateCallbackUrl(url, payload, sendType, senderWithCallbackNonce(sender)); }; @@ -82,7 +88,10 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { let senderUrl: URL; try { - senderUrl = new URL(payload.sender); + if (typeof payload.sender !== 'string') { + return false; + } + senderUrl = new URL(payload.sender, window.location.origin); } catch { return false; } From ac1ebe270501bcfb124e8d7fe477fad1f2bd4ba0 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:34:22 +0100 Subject: [PATCH 03/12] fix(callback): refresh request state at use OS-983 --- web/src/components/Onboarding/OnboardingModal.vue | 4 ++-- .../Onboarding/steps/OnboardingLicenseStep.vue | 10 +++++++--- web/src/store/callbackActions.ts | 15 ++++----------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/web/src/components/Onboarding/OnboardingModal.vue b/web/src/components/Onboarding/OnboardingModal.vue index 729ea6bd6e..df9c7dfe30 100644 --- a/web/src/components/Onboarding/OnboardingModal.vue +++ b/web/src/components/Onboarding/OnboardingModal.vue @@ -69,7 +69,7 @@ const allowActivationSkip = computed( () => hasKeyfile.value || activationRequired.value || showActivationStep.value ); const showKeyfileHint = computed(() => activationRequired.value && hasKeyfile.value); -const activateHref = computed(() => purchaseStore.generateUrl('activate')); +const getActivateHref = () => purchaseStore.generateUrl('activate'); const activateExternal = computed(() => purchaseStore.openInNewTab); // Hardcoded step definitions - order matters for UI flow @@ -591,7 +591,7 @@ const currentStepProps = computed>(() => { modalDescription: modalDescription.value, docsButtons: docsButtons.value, canGoBack: canGoBack.value, - activateHref: activateHref.value, + activateHref: getActivateHref, activateExternal: activateExternal.value, allowSkip: allowActivationSkip.value, showKeyfileHint: showKeyfileHint.value, diff --git a/web/src/components/Onboarding/steps/OnboardingLicenseStep.vue b/web/src/components/Onboarding/steps/OnboardingLicenseStep.vue index e85e70a8ab..38e8ff826f 100644 --- a/web/src/components/Onboarding/steps/OnboardingLicenseStep.vue +++ b/web/src/components/Onboarding/steps/OnboardingLicenseStep.vue @@ -21,7 +21,7 @@ interface Props { onComplete?: () => void; onBack?: () => void; showBack?: boolean; - activateHref: string; + activateHref: string | (() => string); activateExternal?: boolean; allowSkip?: boolean; } @@ -88,14 +88,18 @@ const isSkipDialogOpen = ref(false); const isRefreshing = ref(false); // Methods +const resolveActivateHref = () => + typeof props.activateHref === 'function' ? props.activateHref() : props.activateHref; + const openActivate = () => { + const activateHref = resolveActivateHref(); if (props.activateExternal) { - const opened = window.open(props.activateHref, '_blank', 'noopener,noreferrer'); + const opened = window.open(activateHref, '_blank', 'noopener,noreferrer'); if (opened) { opened.opener = null; } } else { - window.location.href = props.activateHref; + window.location.href = activateHref; } }; diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index dba123fb2a..9f8942b88b 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -45,7 +45,7 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { }; const rememberCallbackNonce = (nonce: string) => { - sessionStorage.setItem( + localStorage.setItem( `${callbackNonceStoragePrefix}${nonce}`, String(Date.now() + callbackNonceTtlMs) ); @@ -67,17 +67,11 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const send = (...args: Parameters) => { const [url, payload, redirectType, sendType, sender] = args; - if (sendCallback.length < 5) { - return sendCallback(url, payload, redirectType, sendType); - } return sendCallback(url, payload, redirectType, sendType, senderWithCallbackNonce(sender)); }; const generateUrl = (...args: Parameters) => { const [url, payload, sendType, sender] = args; - if (generateCallbackUrl.length < 4) { - return generateCallbackUrl(url, payload, sendType); - } return generateCallbackUrl(url, payload, sendType, senderWithCallbackNonce(sender)); }; @@ -98,13 +92,12 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const nonce = senderUrl.searchParams.get(callbackNonceParameter); if (!nonce) { - // Callbacks from older clients do not carry a nonce. - return true; + return false; } const storageKey = `${callbackNonceStoragePrefix}${nonce}`; - const expiresAt = Number(sessionStorage.getItem(storageKey)); - sessionStorage.removeItem(storageKey); + const expiresAt = Number(localStorage.getItem(storageKey)); + localStorage.removeItem(storageKey); return Number.isSafeInteger(expiresAt) && expiresAt >= Date.now(); }; From 53360f9acb077124e6512a529910b78d08ca83da Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:45:05 +0100 Subject: [PATCH 04/12] fix(callback): preserve existing sender compatibility OS-983 --- web/src/store/callbackActions.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index 9f8942b88b..58405a8439 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -67,11 +67,17 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const send = (...args: Parameters) => { const [url, payload, redirectType, sendType, sender] = args; + if (sendCallback.length < 5) { + return sendCallback(url, payload, redirectType, sendType); + } return sendCallback(url, payload, redirectType, sendType, senderWithCallbackNonce(sender)); }; const generateUrl = (...args: Parameters) => { const [url, payload, sendType, sender] = args; + if (generateCallbackUrl.length < 4) { + return generateCallbackUrl(url, payload, sendType); + } return generateCallbackUrl(url, payload, sendType, senderWithCallbackNonce(sender)); }; @@ -92,7 +98,9 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const nonce = senderUrl.searchParams.get(callbackNonceParameter); if (!nonce) { - return false; + // Browser callbacks produced by this store always use an absolute sender URL. + // Keep relative payloads compatible with direct in-process callers. + return !/^[a-z][a-z\d+.-]*:\/\//i.test(payload.sender); } const storageKey = `${callbackNonceStoragePrefix}${nonce}`; From a99347b06ee36447777d5441847f6eebfc473112 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:05:30 +0100 Subject: [PATCH 05/12] fix(callback): preserve update return path --- web/src/store/callbackActions.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index 58405a8439..277ee1033a 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -58,6 +58,9 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const nonce = createCallbackNonce(); const senderUrl = new URL(sender ?? window.location.href, window.location.origin); + if (sender === undefined && senderUrl.pathname === '/Tools/Update') { + senderUrl.pathname = '/Tools'; + } senderUrl.hash = ''; senderUrl.searchParams.delete('data'); senderUrl.searchParams.set(callbackNonceParameter, nonce); From 0179915cf349508287ec125995ae5951c58011f9 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:11:25 +0100 Subject: [PATCH 06/12] fix: validate callback return state --- web/src/store/callbackActions.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index 277ee1033a..2b53d1707f 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -27,7 +27,7 @@ import { useUpdateOsActionsStore } from '~/store/updateOsActions'; const callbackEncryptionKey = import.meta.env.VITE_CALLBACK_KEY ?? ''; const callbackNonceParameter = 'callback_nonce'; const callbackNonceStoragePrefix = 'unraid-callback-nonce:'; -const callbackNonceTtlMs = 10 * 60 * 1000; +const callbackNonceTtlMs = 30 * 60 * 1000; export const useCallbackActionsStore = defineStore('callbackActions', () => { const { @@ -103,7 +103,9 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { if (!nonce) { // Browser callbacks produced by this store always use an absolute sender URL. // Keep relative payloads compatible with direct in-process callers. - return !/^[a-z][a-z\d+.-]*:\/\//i.test(payload.sender); + const isAbsoluteSender = + payload.sender.startsWith('//') || /^[a-z][a-z\d+.-]*:/i.test(payload.sender); + return !isAbsoluteSender && senderUrl.origin === window.location.origin; } const storageKey = `${callbackNonceStoragePrefix}${nonce}`; From c731e2e4b62d30bc10efb7db178607de8d1d6c52 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Sat, 17 Oct 2026 10:31:03 +0100 Subject: [PATCH 07/12] fix(callback): require active return state --- web/src/store/callbackActions.ts | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index 2b53d1707f..de48e0181a 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -68,6 +68,19 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { return senderUrl.toString(); }; + const hasCallbackDataInCurrentUrl = (): boolean => { + if (typeof window === 'undefined') { + return false; + } + + try { + const currentUrl = new URL(window.location.href); + return currentUrl.searchParams.has('data') || currentUrl.hash.startsWith('#data='); + } catch { + return false; + } + }; + const send = (...args: Parameters) => { const [url, payload, redirectType, sendType, sender] = args; if (sendCallback.length < 5) { @@ -84,7 +97,7 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { return generateCallbackUrl(url, payload, sendType, senderWithCallbackNonce(sender)); }; - const validateAndConsumeCallbackNonce = (payload: QueryPayloads): boolean => { + const validateAndConsumeCallbackNonce = (payload: QueryPayloads, requireNonce = false): boolean => { if (!isExternalCallbackPayload(payload)) { return true; } @@ -101,7 +114,10 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const nonce = senderUrl.searchParams.get(callbackNonceParameter); if (!nonce) { - // Browser callbacks produced by this store always use an absolute sender URL. + if (requireNonce) { + return false; + } + // Keep relative payloads compatible with direct in-process callers. const isAbsoluteSender = payload.sender.startsWith('//') || /^[a-z][a-z\d+.-]*:/i.test(payload.sender); @@ -140,12 +156,15 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const watcher = () => { const result = providedWatcher(); if (result) { - void saveCallbackData(result); + void saveCallbackData(result, { requireNonce: hasCallbackDataInCurrentUrl() }); } }; - const saveCallbackData = async (decryptedData?: QueryPayloads) => { - if (decryptedData && !validateAndConsumeCallbackNonce(decryptedData)) { + const saveCallbackData = async ( + decryptedData?: QueryPayloads, + options: { requireNonce?: boolean } = {} + ) => { + if (decryptedData && !validateAndConsumeCallbackNonce(decryptedData, options.requireNonce)) { callbackData.value = undefined; callbackError.value = 'Callback request is not active or has expired'; callbackStatus.value = 'error'; From 9a2b9542bd4e23dd67395933c82ca74b50ed87fe Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Mon, 19 Oct 2026 08:15:54 +0100 Subject: [PATCH 08/12] fix(callback): ignore duplicate callback processing --- web/src/store/callbackActions.ts | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index de48e0181a..78b8af2b1f 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -68,16 +68,20 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { return senderUrl.toString(); }; - const hasCallbackDataInCurrentUrl = (): boolean => { + const getCallbackDataInCurrentUrl = (): string | undefined => { if (typeof window === 'undefined') { - return false; + return undefined; } try { const currentUrl = new URL(window.location.href); - return currentUrl.searchParams.has('data') || currentUrl.hash.startsWith('#data='); + const queryData = currentUrl.searchParams.get('data'); + if (queryData !== null) { + return queryData; + } + return currentUrl.hash.startsWith('#data=') ? currentUrl.hash.slice('#data='.length) : undefined; } catch { - return false; + return undefined; } }; @@ -142,6 +146,7 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const callbackActionsExecuting = ref(false); const callbackReconciliationPending = ref(false); const sendType = 'fromUpc'; + let lastWatchedCallbackData: string | undefined; const getCallbackActionErrorMessage = (error: unknown): string => { if (error instanceof Error) { @@ -154,9 +159,17 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { }; const watcher = () => { + const callbackDataInCurrentUrl = getCallbackDataInCurrentUrl(); + if (callbackDataInCurrentUrl && callbackDataInCurrentUrl === lastWatchedCallbackData) { + return; + } + const result = providedWatcher(); if (result) { - void saveCallbackData(result, { requireNonce: hasCallbackDataInCurrentUrl() }); + if (callbackDataInCurrentUrl) { + lastWatchedCallbackData = callbackDataInCurrentUrl; + } + void saveCallbackData(result, { requireNonce: Boolean(callbackDataInCurrentUrl) }); } }; From ca5a1c3d82ccd7cbc7e0b35a9a28afffb1ca92b9 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:39:38 +0100 Subject: [PATCH 09/12] fix(callback): prefer non-empty query payload Allow hash payloads to be read when the query data parameter is blank. --- web/src/store/callbackActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index 78b8af2b1f..f3750b8043 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -76,7 +76,7 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { try { const currentUrl = new URL(window.location.href); const queryData = currentUrl.searchParams.get('data'); - if (queryData !== null) { + if (queryData) { return queryData; } return currentUrl.hash.startsWith('#data=') ? currentUrl.hash.slice('#data='.length) : undefined; From 15c243adb587302de3cf532243c95646ad685863 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:52:06 +0100 Subject: [PATCH 10/12] test(callback): cover return flows --- web/__test__/store/callbackActions.test.ts | 150 ++++++++++++++++++++- 1 file changed, 148 insertions(+), 2 deletions(-) diff --git a/web/__test__/store/callbackActions.test.ts b/web/__test__/store/callbackActions.test.ts index d0e3ec958c..c61eb37115 100644 --- a/web/__test__/store/callbackActions.test.ts +++ b/web/__test__/store/callbackActions.test.ts @@ -38,11 +38,28 @@ const mockRefreshServerState = vi.fn(); vi.mock('@unraid/shared-callbacks', () => { const mockWatcher = vi.fn(); - const mockSend = vi.fn(); + const mockSend = vi.fn( + ( + url: string, + payload: unknown, + redirectType: string | null | undefined, + sendType: string | undefined, + sender?: string + ) => ({ url, payload, redirectType, sendType, sender }) + ); + const mockGenerateUrl = vi.fn( + (url: string, payload: unknown, sendType: string | undefined, sender?: string) => ({ + url, + payload, + sendType, + sender, + }) + ); return { useCallback: vi.fn(({ encryptionKey: _encryptionKey }) => ({ send: mockSend, + generateUrl: mockGenerateUrl, watcher: mockWatcher, parse: vi.fn(), })), @@ -146,9 +163,13 @@ describe('Callback Actions Store', () => { let store: ReturnType; let preventClose: { addPreventClose: Mock; removePreventClose: Mock }; let mockWatcher: Mock; + let mockSend: Mock; + let mockGenerateUrl: Mock; beforeEach(async () => { setActivePinia(createPinia()); + localStorage.clear(); + window.history.replaceState(null, '', '/'); mockAccountActionStatus.value = 'success'; mockKeyInstallStatus.value = 'success'; mockRefreshServerStateStatus.value = 'done'; @@ -163,7 +184,10 @@ describe('Callback Actions Store', () => { const { useCallback } = await import('@unraid/shared-callbacks'); - mockWatcher = vi.mocked(useCallback).mock.results[0].value.watcher; + const callbackMock = vi.mocked(useCallback).mock.results[0].value; + mockWatcher = callbackMock.watcher; + mockSend = callbackMock.send; + mockGenerateUrl = callbackMock.generateUrl; vi.clearAllMocks(); }); @@ -213,6 +237,128 @@ describe('Callback Actions Store', () => { }); }); + describe('Callback Return Paths', () => { + it('adds a marker to the default return path', () => { + window.history.replaceState(null, '', '/Tools/Update'); + + store.send( + 'https://example.com/callback', + [{ type: 'updateOs', sha256: 'test-sha256' } as ExternalUpdateOsAction], + undefined, + 'forUpc' + ); + + const sender = mockSend.mock.calls[0][4] as string; + const senderUrl = new URL(sender); + const nonce = senderUrl.searchParams.get('callback_nonce'); + + expect(senderUrl.pathname).toBe('/Tools'); + expect(senderUrl.searchParams.get('data')).toBeNull(); + expect(nonce).toMatch(/^[0-9a-f]{32}$/); + expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeTruthy(); + }); + + it('preserves explicit return details when adding a marker', () => { + const sender = 'https://example.com/return?state=ready#section'; + + store.send( + 'https://example.com/callback', + [{ type: 'updateOs', sha256: 'test-sha256' } as ExternalUpdateOsAction], + undefined, + 'forUpc', + sender + ); + + const returnedUrl = new URL(mockSend.mock.calls[0][4] as string); + + expect(returnedUrl.origin).toBe('https://example.com'); + expect(returnedUrl.pathname).toBe('/return'); + expect(returnedUrl.searchParams.get('state')).toBe('ready'); + expect(returnedUrl.hash).toBe(''); + expect(returnedUrl.searchParams.get('callback_nonce')).toMatch(/^[0-9a-f]{32}$/); + }); + + it('adds a marker when creating a return link', () => { + const payload = [{ type: 'updateOs', sha256: 'test-sha256' } as ExternalUpdateOsAction]; + + store.generateUrl('https://example.com/callback', payload, 'forUpc'); + + const sender = mockGenerateUrl.mock.calls[0][3] as string; + const senderUrl = new URL(sender); + + expect(senderUrl.searchParams.get('callback_nonce')).toMatch(/^[0-9a-f]{32}$/); + expect( + localStorage.getItem(`unraid-callback-nonce:${senderUrl.searchParams.get('callback_nonce')}`) + ).toBeTruthy(); + }); + }); + + describe('Callback Return Validation', () => { + const updatePayload = (sender: string): QueryPayloads => ({ + type: 'forUpc', + actions: [ + { + type: 'updateOs', + sha256: 'test-sha256', + } as ExternalUpdateOsAction, + ], + sender, + }); + + it('uses the fragment when the query value is blank', async () => { + window.history.replaceState(null, '', '/Tools?data=#data=fragment'); + mockWatcher.mockReturnValue(updatePayload('/Tools')); + + store.watcher(); + await nextTick(); + store.watcher(); + + expect(mockWatcher).toHaveBeenCalledTimes(1); + expect(store.callbackStatus).toBe('error'); + expect(store.callbackError).toBe('Callback request is not active or has expired'); + }); + + it('consumes a return marker once', async () => { + store.send( + 'https://example.com/callback', + [{ type: 'updateOs', sha256: 'test-sha256' } as ExternalUpdateOsAction], + undefined, + 'forUpc' + ); + const sender = mockSend.mock.calls[0][4] as string; + const nonce = new URL(sender).searchParams.get('callback_nonce'); + + await store.saveCallbackData(updatePayload(sender), { requireNonce: true }); + await store.saveCallbackData(updatePayload(sender), { requireNonce: true }); + + expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeNull(); + expect(store.callbackStatus).toBe('error'); + expect(store.callbackError).toBe('Callback request is not active or has expired'); + }); + + it('rejects an expired return marker', async () => { + const nonce = 'expired-marker'; + localStorage.setItem(`unraid-callback-nonce:${nonce}`, String(Date.now() - 1)); + + await store.saveCallbackData(updatePayload(`/Tools?callback_nonce=${nonce}`), { + requireNonce: true, + }); + + expect(store.callbackStatus).toBe('error'); + expect(store.callbackError).toBe('Callback request is not active or has expired'); + expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeNull(); + }); + + it('rejects an external return path without a marker', async () => { + await store.saveCallbackData(updatePayload('https://example.com/return'), { + requireNonce: true, + }); + + expect(store.callbackStatus).toBe('error'); + expect(store.callbackError).toBe('Callback request is not active or has expired'); + }); + }); + describe('Save Callback Data', () => { it('should save data and trigger redirect when valid data provided', () => { const mockData: QueryPayloads = { From 9d3f754729f3bb3241ab17c608924837e9e87a98 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Mon, 19 Oct 2026 12:32:14 -0400 Subject: [PATCH 11/12] fix(callback): use dedicated shared callback nonce --- pnpm-lock.yaml | 10 +- web/__test__/store/callbackActions.test.ts | 118 ++++++++++++++++----- web/package.json | 2 +- web/src/store/callbackActions.ts | 49 +++++---- 4 files changed, 126 insertions(+), 53 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8a158aa8b9..b87da0075a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1154,8 +1154,8 @@ importers: specifier: 3.0.7 version: 3.0.7(prosemirror-model@1.25.11)(prosemirror-state@1.4.4)(prosemirror-view@1.42.3)(y-protocols@1.0.7(yjs@13.6.31))(yjs@13.6.31) '@unraid/shared-callbacks': - specifier: 3.1.0 - version: 3.1.0 + specifier: 3.4.0 + version: 3.4.0 '@unraid/ui': specifier: link:../unraid-ui version: link:../unraid-ui @@ -4973,8 +4973,8 @@ packages: cpu: [x64, arm64] os: [linux, darwin] - '@unraid/shared-callbacks@3.1.0': - resolution: {integrity: sha512-Zvz9nlvLSjbTstplvarbH2rwZrsl4ns7djpDmqPi4xH35PvYeK9ut1A/Hkd2Rbl/+nvAMF77d5ZzZkSTb4IoSg==} + '@unraid/shared-callbacks@3.4.0': + resolution: {integrity: sha512-9tCVlt+nFK9W/ZdwL+nBS8Mzc5RhoVeMJ+6y6okATB/rLJZMlgGxiU2K5sCo88Vn98FRa9TNHII/oMxp/23lGw==} '@unraid/tailwind-rem-to-rem@2.0.0': resolution: {integrity: sha512-zccpQx5fvEBkAB0JkRwwtyRrT9l26LsjkozLy44LGv0NdZGaxgscniIqJRM+OQj5pSpsWDzExebAtUKdE98Flg==} @@ -16269,7 +16269,7 @@ snapshots: - encoding - supports-color - '@unraid/shared-callbacks@3.1.0': + '@unraid/shared-callbacks@3.4.0': dependencies: crypto-js: 4.2.0 diff --git a/web/__test__/store/callbackActions.test.ts b/web/__test__/store/callbackActions.test.ts index c61eb37115..533651b667 100644 --- a/web/__test__/store/callbackActions.test.ts +++ b/web/__test__/store/callbackActions.test.ts @@ -36,7 +36,7 @@ const mockRefreshServerStateStatus = ref<'done' | 'ready' | 'refreshing' | 'time const mockInstall = vi.fn(); const mockRefreshServerState = vi.fn(); -vi.mock('@unraid/shared-callbacks', () => { +vi.mock('@unraid/shared-callbacks/client', () => { const mockWatcher = vi.fn(); const mockSend = vi.fn( ( @@ -44,20 +44,22 @@ vi.mock('@unraid/shared-callbacks', () => { payload: unknown, redirectType: string | null | undefined, sendType: string | undefined, - sender?: string - ) => ({ url, payload, redirectType, sendType, sender }) + sender?: string, + nonce?: string + ) => ({ url, payload, redirectType, sendType, sender, nonce }) ); const mockGenerateUrl = vi.fn( - (url: string, payload: unknown, sendType: string | undefined, sender?: string) => ({ + (url: string, payload: unknown, sendType: string | undefined, sender?: string, nonce?: string) => ({ url, payload, sendType, sender, + nonce, }) ); return { - useCallback: vi.fn(({ encryptionKey: _encryptionKey }) => ({ + createCallback: vi.fn(({ encryptionKey: _encryptionKey }) => ({ send: mockSend, generateUrl: mockGenerateUrl, watcher: mockWatcher, @@ -182,9 +184,9 @@ describe('Callback Actions Store', () => { removePreventClose: vi.mocked(preventCloseModule.removePreventClose), }; - const { useCallback } = await import('@unraid/shared-callbacks'); + const { createCallback } = await import('@unraid/shared-callbacks/client'); - const callbackMock = vi.mocked(useCallback).mock.results[0].value; + const callbackMock = vi.mocked(createCallback).mock.results[0].value; mockWatcher = callbackMock.watcher; mockSend = callbackMock.send; mockGenerateUrl = callbackMock.generateUrl; @@ -238,7 +240,7 @@ describe('Callback Actions Store', () => { }); describe('Callback Return Paths', () => { - it('adds a marker to the default return path', () => { + it('passes a dedicated nonce and cleans the default return path', () => { window.history.replaceState(null, '', '/Tools/Update'); store.send( @@ -250,15 +252,16 @@ describe('Callback Actions Store', () => { const sender = mockSend.mock.calls[0][4] as string; const senderUrl = new URL(sender); - const nonce = senderUrl.searchParams.get('callback_nonce'); + const nonce = mockSend.mock.calls[0][5] as string; expect(senderUrl.pathname).toBe('/Tools'); expect(senderUrl.searchParams.get('data')).toBeNull(); + expect(senderUrl.searchParams.has('callback_nonce')).toBe(false); expect(nonce).toMatch(/^[0-9a-f]{32}$/); expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeTruthy(); }); - it('preserves explicit return details when adding a marker', () => { + it('preserves explicit return details without adding a nonce parameter', () => { const sender = 'https://example.com/return?state=ready#section'; store.send( @@ -275,10 +278,11 @@ describe('Callback Actions Store', () => { expect(returnedUrl.pathname).toBe('/return'); expect(returnedUrl.searchParams.get('state')).toBe('ready'); expect(returnedUrl.hash).toBe(''); - expect(returnedUrl.searchParams.get('callback_nonce')).toMatch(/^[0-9a-f]{32}$/); + expect(returnedUrl.searchParams.has('callback_nonce')).toBe(false); + expect(mockSend.mock.calls[0][5]).toMatch(/^[0-9a-f]{32}$/); }); - it('adds a marker when creating a return link', () => { + it('passes a dedicated nonce when creating a return link', () => { const payload = [{ type: 'updateOs', sha256: 'test-sha256' } as ExternalUpdateOsAction]; store.generateUrl('https://example.com/callback', payload, 'forUpc'); @@ -286,15 +290,15 @@ describe('Callback Actions Store', () => { const sender = mockGenerateUrl.mock.calls[0][3] as string; const senderUrl = new URL(sender); - expect(senderUrl.searchParams.get('callback_nonce')).toMatch(/^[0-9a-f]{32}$/); - expect( - localStorage.getItem(`unraid-callback-nonce:${senderUrl.searchParams.get('callback_nonce')}`) - ).toBeTruthy(); + const nonce = mockGenerateUrl.mock.calls[0][4] as string; + expect(senderUrl.searchParams.has('callback_nonce')).toBe(false); + expect(nonce).toMatch(/^[0-9a-f]{32}$/); + expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeTruthy(); }); }); describe('Callback Return Validation', () => { - const updatePayload = (sender: string): QueryPayloads => ({ + const updatePayload = (sender: string, nonce?: string): QueryPayloads => ({ type: 'forUpc', actions: [ { @@ -303,6 +307,7 @@ describe('Callback Actions Store', () => { } as ExternalUpdateOsAction, ], sender, + nonce, }); it('uses the fragment when the query value is blank', async () => { @@ -315,10 +320,10 @@ describe('Callback Actions Store', () => { expect(mockWatcher).toHaveBeenCalledTimes(1); expect(store.callbackStatus).toBe('error'); - expect(store.callbackError).toBe('Callback request is not active or has expired'); + expect(store.callbackError).toBeDefined(); }); - it('consumes a return marker once', async () => { + it('consumes a dedicated return nonce once', async () => { store.send( 'https://example.com/callback', [{ type: 'updateOs', sha256: 'test-sha256' } as ExternalUpdateOsAction], @@ -326,36 +331,91 @@ describe('Callback Actions Store', () => { 'forUpc' ); const sender = mockSend.mock.calls[0][4] as string; - const nonce = new URL(sender).searchParams.get('callback_nonce'); + const nonce = mockSend.mock.calls[0][5] as string; - await store.saveCallbackData(updatePayload(sender), { requireNonce: true }); - await store.saveCallbackData(updatePayload(sender), { requireNonce: true }); + await store.saveCallbackData(updatePayload(sender, nonce), { requireNonce: true }); + await store.saveCallbackData(updatePayload(sender, nonce), { requireNonce: true }); expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeNull(); expect(store.callbackStatus).toBe('error'); - expect(store.callbackError).toBe('Callback request is not active or has expired'); + expect(store.callbackError).toBeDefined(); }); - it('rejects an expired return marker', async () => { - const nonce = 'expired-marker'; - localStorage.setItem(`unraid-callback-nonce:${nonce}`, String(Date.now() - 1)); + it('round-trips the dedicated nonce through encrypted callback URLs', async () => { + const { createCallback } = await vi.importActual( + '@unraid/shared-callbacks/client' + ); + const callback = createCallback({ encryptionKey: 'test-callback-key' }); + mockGenerateUrl.mockImplementationOnce(callback.generateUrl); + const requestUrl = store.generateUrl( + 'https://example.com/callback', + [{ type: 'updateOs', sha256: 'test-sha256' }], + 'forUpc' + ); + const request = callback.watcher({ baseUrl: requestUrl }); + if (!request) { + throw new Error('Callback request was not parsed'); + } + expect(request.nonce).toMatch(/^[0-9a-f]{32}$/); + expect(new URL(request.sender).searchParams.has('callback_nonce')).toBe(false); + + const responseUrl = callback.generateUrl( + request.sender, + [{ type: 'updateOs', sha256: 'test-sha256' }], + 'forUpc', + 'https://example.com/callback', + request.nonce + ); + mockWatcher.mockReturnValue(callback.watcher({ baseUrl: responseUrl })); + window.history.replaceState(null, '', responseUrl); + store.watcher(); + await nextTick(); + + expect(store.callbackData?.nonce).toBe(request.nonce); + expect(store.callbackStatus).not.toBe('error'); + expect(localStorage.getItem(`unraid-callback-nonce:${request.nonce}`)).toBeNull(); + }); + + it('rejects a nonce carried only in the sender URL', async () => { + const nonce = 'b'.repeat(32); + localStorage.setItem(`unraid-callback-nonce:${nonce}`, String(Date.now() + 60_000)); await store.saveCallbackData(updatePayload(`/Tools?callback_nonce=${nonce}`), { requireNonce: true, }); expect(store.callbackStatus).toBe('error'); - expect(store.callbackError).toBe('Callback request is not active or has expired'); + expect(store.callbackData).toBeUndefined(); + expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeTruthy(); + }); + + it.each(['', 'unknown', 'c'.repeat(32)])('rejects an inactive nonce %j', async (nonce) => { + await store.saveCallbackData(updatePayload('/Tools', nonce), { requireNonce: true }); + + expect(store.callbackStatus).toBe('error'); + expect(store.callbackData).toBeUndefined(); + }); + + it('rejects an expired return nonce', async () => { + const nonce = 'a'.repeat(32); + localStorage.setItem(`unraid-callback-nonce:${nonce}`, String(Date.now() - 1)); + + await store.saveCallbackData(updatePayload('/Tools', nonce), { + requireNonce: true, + }); + + expect(store.callbackStatus).toBe('error'); + expect(store.callbackError).toBeDefined(); expect(localStorage.getItem(`unraid-callback-nonce:${nonce}`)).toBeNull(); }); - it('rejects an external return path without a marker', async () => { + it('rejects an external return path without a nonce', async () => { await store.saveCallbackData(updatePayload('https://example.com/return'), { requireNonce: true, }); expect(store.callbackStatus).toBe('error'); - expect(store.callbackError).toBe('Callback request is not active or has expired'); + expect(store.callbackError).toBeDefined(); }); }); diff --git a/web/package.json b/web/package.json index 38706ad567..2f052eab07 100644 --- a/web/package.json +++ b/web/package.json @@ -117,7 +117,7 @@ "@nuxt/ui": "4.8.2", "@tanstack/vue-table": "8.21.3", "@tiptap/y-tiptap": "3.0.7", - "@unraid/shared-callbacks": "3.1.0", + "@unraid/shared-callbacks": "3.4.0", "@unraid/ui": "link:../unraid-ui", "@vue/apollo-composable": "4.2.2", "@vueuse/components": "13.8.0", diff --git a/web/src/store/callbackActions.ts b/web/src/store/callbackActions.ts index f3750b8043..b2914ba009 100644 --- a/web/src/store/callbackActions.ts +++ b/web/src/store/callbackActions.ts @@ -1,7 +1,7 @@ import { computed, ref, watch } from 'vue'; import { defineStore } from 'pinia'; -import { useCallback } from '@unraid/shared-callbacks'; +import { createCallback } from '@unraid/shared-callbacks/client'; import type { ExternalActions, QueryPayloads } from '@unraid/shared-callbacks'; import type { CallbackStatus } from '~/store/callbackActions.helpers'; @@ -25,7 +25,6 @@ import { useServerStore } from '~/store/server'; import { useUpdateOsActionsStore } from '~/store/updateOsActions'; const callbackEncryptionKey = import.meta.env.VITE_CALLBACK_KEY ?? ''; -const callbackNonceParameter = 'callback_nonce'; const callbackNonceStoragePrefix = 'unraid-callback-nonce:'; const callbackNonceTtlMs = 30 * 60 * 1000; @@ -34,7 +33,7 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { send: sendCallback, watcher: providedWatcher, generateUrl: generateCallbackUrl, - } = useCallback({ + } = createCallback({ encryptionKey: callbackEncryptionKey, }); @@ -44,27 +43,30 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { return Array.from(bytes, (byte) => byte.toString(16).padStart(2, '0')).join(''); }; - const rememberCallbackNonce = (nonce: string) => { + const createAndRememberCallbackNonce = (): string | undefined => { + if (typeof window === 'undefined') { + return undefined; + } + + const nonce = createCallbackNonce(); localStorage.setItem( `${callbackNonceStoragePrefix}${nonce}`, String(Date.now() + callbackNonceTtlMs) ); + return nonce; }; - const senderWithCallbackNonce = (sender?: string): string | undefined => { + const getCallbackSender = (sender?: string): string | undefined => { if (typeof window === 'undefined') { return sender; } - const nonce = createCallbackNonce(); const senderUrl = new URL(sender ?? window.location.href, window.location.origin); if (sender === undefined && senderUrl.pathname === '/Tools/Update') { senderUrl.pathname = '/Tools'; } senderUrl.hash = ''; senderUrl.searchParams.delete('data'); - senderUrl.searchParams.set(callbackNonceParameter, nonce); - rememberCallbackNonce(nonce); return senderUrl.toString(); }; @@ -87,18 +89,25 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { const send = (...args: Parameters) => { const [url, payload, redirectType, sendType, sender] = args; - if (sendCallback.length < 5) { - return sendCallback(url, payload, redirectType, sendType); - } - return sendCallback(url, payload, redirectType, sendType, senderWithCallbackNonce(sender)); + return sendCallback( + url, + payload, + redirectType, + sendType, + getCallbackSender(sender), + createAndRememberCallbackNonce() + ); }; const generateUrl = (...args: Parameters) => { const [url, payload, sendType, sender] = args; - if (generateCallbackUrl.length < 4) { - return generateCallbackUrl(url, payload, sendType); - } - return generateCallbackUrl(url, payload, sendType, senderWithCallbackNonce(sender)); + return generateCallbackUrl( + url, + payload, + sendType, + getCallbackSender(sender), + createAndRememberCallbackNonce() + ); }; const validateAndConsumeCallbackNonce = (payload: QueryPayloads, requireNonce = false): boolean => { @@ -116,8 +125,8 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { return false; } - const nonce = senderUrl.searchParams.get(callbackNonceParameter); - if (!nonce) { + const nonce = payload.nonce; + if (nonce === undefined) { if (requireNonce) { return false; } @@ -128,6 +137,10 @@ export const useCallbackActionsStore = defineStore('callbackActions', () => { return !isAbsoluteSender && senderUrl.origin === window.location.origin; } + if (typeof nonce !== 'string' || !/^[0-9a-f]{32}$/.test(nonce)) { + return false; + } + const storageKey = `${callbackNonceStoragePrefix}${nonce}`; const expiresAt = Number(localStorage.getItem(storageKey)); localStorage.removeItem(storageKey); From 1a55dd84f95801721bd9a83bfb61b19e20bba013 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:09:50 +0100 Subject: [PATCH 12/12] test(callback): update return flow fixture --- web/__test__/store/updateOs.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/web/__test__/store/updateOs.test.ts b/web/__test__/store/updateOs.test.ts index 940787e265..80134313c4 100644 --- a/web/__test__/store/updateOs.test.ts +++ b/web/__test__/store/updateOs.test.ts @@ -11,10 +11,11 @@ import { useUpdateOsStore } from '~/store/updateOs'; const mockSend = vi.fn(); -vi.mock('@unraid/shared-callbacks', () => ({ - useCallback: vi.fn(() => ({ +vi.mock('@unraid/shared-callbacks/client', () => ({ + createCallback: vi.fn(() => ({ send: mockSend, watcher: vi.fn(), + generateUrl: vi.fn(), })), })); @@ -153,7 +154,9 @@ describe('UpdateOs Store', () => { }, ], undefined, - 'forUpc' + 'forUpc', + 'https://littlebox.tail45affd.ts.net/Plugins', + expect.stringMatching(/^[0-9a-f]{32}$/) ); Object.defineProperty(window, 'location', {