diff --git a/services/recordly-share/worker/README.md b/services/recordly-share/worker/README.md index 637c4d71..108a2b28 100644 --- a/services/recordly-share/worker/README.md +++ b/services/recordly-share/worker/README.md @@ -104,3 +104,41 @@ npm test The web build resolves shared desktop authentication components against its own React, HeroUI and icon dependencies; a root Electron dependency installation is not required. + +## Recovering a recording moderation lock + +A stopped Worker invocation or a failed lock cleanup can leave a row in +`recording_locks`. The service deliberately keeps that recording busy. The row's +`created_at` does not prove that its action has stopped, so it must not be expired +or removed automatically based on age. + +Recovery requires an operator with direct access to the deployment's D1 database: + +1. Stop new invocations that can mutate that database, including public requests, + service bindings and scheduled work. Confirm that previous invocations and + their database operations have ended. If quiescence cannot be established, + keep the lock and investigate with the hosting provider. +2. While the service remains quiescent, inspect the exact recording's lock: + + ```sql + SELECT share_code, token, created_at FROM recording_locks + WHERE share_code = ''; + ``` + +3. Remove only the observed lock, matching both its recording code and token: + + ```sql + DELETE FROM recording_locks + WHERE share_code = '' AND token = '' + RETURNING share_code, token, created_at; + ``` + + Preserve the returned row in the maintenance record. Zero returned rows means + that this lock was already removed or changed; inspect again rather than + deleting a different token or clearing the table. +4. Resume service after maintenance is complete and retry the owner's operation. + +The SQL can be run with `wrangler d1 execute` using the explicit deployment +configuration, database and `--local` or `--remote` selection. This is a manual +maintenance procedure; changing a token or checking a confirmation box does not +establish that an earlier Worker action has ended. diff --git a/services/recordly-share/worker/src/feedback.js b/services/recordly-share/worker/src/feedback.js index b07b9809..1db8135a 100644 --- a/services/recordly-share/worker/src/feedback.js +++ b/services/recordly-share/worker/src/feedback.js @@ -94,7 +94,7 @@ export async function handleComment(request, env, shareCode) { // Password check if (video.password_hash) { const authed = await verifyPasswordAuth(request, env, shareCode, video); - if (!authed) return errorResponse("Unauthorized", 401); + if (!authed) return errorResponse("Password required", 401); } const controls = await env.DB.prepare("SELECT feedback_disabled FROM recording_controls WHERE share_code = ?") diff --git a/services/recordly-share/worker/src/recordingLock.js b/services/recordly-share/worker/src/recordingLock.js index c17853c0..4a8b09f9 100644 --- a/services/recordly-share/worker/src/recordingLock.js +++ b/services/recordly-share/worker/src/recordingLock.js @@ -1,7 +1,8 @@ import { errorResponse } from "./http.js"; -// Serialize upload/delete/finalize for a recording across Worker isolates. A crash -// leaves a lock for maintenance instead of allowing a competing writer through. +// Serialize owner moderation for a recording across Worker isolates. A crash +// leaves a lock rather than admitting a second writer. See the README maintenance +// procedure: lock age alone is never proof that the previous action has ended. export async function withRecordingLock(env, code, action) { const token = crypto.randomUUID(); const result = await env.DB.prepare( diff --git a/services/recordly-share/worker/test/api.test.js b/services/recordly-share/worker/test/api.test.js index 1f048f71..4fb7d2f8 100644 --- a/services/recordly-share/worker/test/api.test.js +++ b/services/recordly-share/worker/test/api.test.js @@ -1,6 +1,7 @@ import { describe, it, expect, beforeAll, vi } from 'vitest'; import { env, SELF } from 'cloudflare:test'; import worker, { sha256Hex } from '../src/index.js'; +import { withRecordingLock } from '../src/recordingLock.js'; import { localCommentHeaders } from './commentFixtures.js'; let commentAuth; @@ -279,6 +280,16 @@ describe('password protection', () => { expect((await res.json()).password_protected).toBe(true); }); + it('distinguishes an expired recording unlock from an account sign-in failure when posting', async () => { + const shareCode = await protectedShare(); + const response = await SELF.fetch(`${BASE}/s/${shareCode}/comment`, { + method: 'POST', headers: { ...commentAuth, 'Content-Type': 'application/json' }, + body: JSON.stringify({ timestamp: 1, text: 'Keep this draft' }), + }); + expect(response.status).toBe(401); + expect(await response.json()).toEqual({ error: 'Password required' }); + }); + it('gates reactions, comments, and the thumbnail behind the password', async () => { const shareCode = await protectedShare(); @@ -676,3 +687,40 @@ it('upgrades salted legacy recording passwords only after correct verification', expect(row.password_hash).toMatch(/^pbkdf2-sha256:210000:[0-9a-f]{64}$/); expect((await verify(password)).status).toBe(200); }); + + +it('keeps an old live lock exclusive and recovers an orphan only by its exact observed token', async () => { + const code = crypto.randomUUID(); + let entered; + let release; + const started = new Promise(resolve => { entered = resolve; }); + const finish = new Promise(resolve => { release = resolve; }); + const running = withRecordingLock(env, code, async () => { + entered(); + await finish; + return new Response('first action completed'); + }); + await started; + try { + await env.DB.prepare("UPDATE recording_locks SET created_at = '2000-01-01' WHERE share_code = ?").bind(code).run(); + const contender = vi.fn(async () => new Response('must not run')); + expect((await withRecordingLock(env, code, contender)).status).toBe(409); + expect(contender).not.toHaveBeenCalled(); + expect((await env.DB.prepare('DELETE FROM recording_locks WHERE share_code = ? AND token = ? RETURNING token').bind(code, 'different-token').all()).results).toEqual([]); + expect((await withRecordingLock(env, code, contender)).status).toBe(409); + } finally { + release(); + await running; + } + // No invocation owns this simulated orphan: maintenance is quiescent by construction. + const orphanToken = crypto.randomUUID(); + await env.DB.prepare("INSERT INTO recording_locks (share_code, token, created_at) VALUES (?, ?, '2000-01-01')").bind(code, orphanToken).run(); + const contender = vi.fn(async () => new Response('recovered action')); + expect((await withRecordingLock(env, code, contender)).status).toBe(409); + expect(contender).not.toHaveBeenCalled(); + const removed = await env.DB.prepare('DELETE FROM recording_locks WHERE share_code = ? AND token = ? RETURNING share_code, token, created_at').bind(code, orphanToken).all(); + expect(removed.results).toEqual([{ share_code: code, token: orphanToken, created_at: '2000-01-01' }]); + expect(await (await withRecordingLock(env, code, contender)).text()).toBe('recovered action'); + expect(contender).toHaveBeenCalledOnce(); + expect(await env.DB.prepare('SELECT token FROM recording_locks WHERE share_code = ?').bind(code).first()).toBeNull(); +}); diff --git a/services/recordly-share/worker/web/src/pages/privacy.astro b/services/recordly-share/worker/web/src/pages/privacy.astro index b3ee6272..0b92f173 100644 --- a/services/recordly-share/worker/web/src/pages/privacy.astro +++ b/services/recordly-share/worker/web/src/pages/privacy.astro @@ -13,7 +13,7 @@ import { legalDetails } from '../legal/config';
  • Account information: your account ID, email address, display name, avatar and information returned by your chosen sign-in provider. Authentication tokens allow the app to maintain your session.
  • Published recordings: video and audio you upload, thumbnails, titles, duration, size, captions or other recording metadata you provide, sharing settings, ownership and retention status.
    • -
  • Viewer interactions: view counts, reactions, comment text, the name a commenter supplies and timestamps. Comments and names are visible to people who can access that recording. IP addresses and request metadata may be used or stored for delivery, abuse prevention and rate limiting.
    • +
  • Viewer interactions: view counts, reactions, comment text, the signed-in commenter’s account display name and account identifier, and timestamps. Comment text and display names are visible to people who can access that recording. We link comments to account identifiers to verify who can edit or delete them. IP addresses and request metadata may be used or stored for delivery, abuse prevention and rate limiting.
  • Billing information: Stripe customer and subscription identifiers, plan, payment status, billing period and payment-event records. Stripe collects payment details on its own checkout and portal. Recordly’s billing database does not store your full card number or security code.
  • Support information: messages, account identifiers, attachments and diagnostic information sent with a report. The in-app feedback form automatically attaches relevant diagnostic logs, which can include application, device and export information. Review any files you attach and avoid including passwords, payment details or confidential recordings in a report.
  • Technical information: request times, IP addresses, browser or device information available with requests, errors and security logs needed to operate and protect the service.
    • @@ -36,7 +36,7 @@ import { legalDetails } from '../legal/config';

    These providers may process information outside your country. {legalDetails.locations || 'The configured processing countries and hosting locations are awaiting confirmation before launch.'} Providers also apply their own privacy policies when they act independently, such as when Stripe processes payment information.

    We may disclose information when reasonably necessary to comply with law, respond to a valid legal request or protect people and the service. If the service changes operator, any transfer of personal information must remain subject to applicable law and notice requirements.

    Storage, cookies and security

    -

    The desktop app stores settings and project information locally. Sign-in sessions use the operating system’s encrypted credential storage when it is available, with a browser-storage fallback in supported preview or development environments. Hosted pages use local storage for preferences such as theme and a remembered commenter name. Password-protected recording access uses an unlock cookie.

    +

    The desktop app stores settings and project information locally. Sign-in sessions use the operating system’s encrypted credential storage when it is available, with a browser-storage fallback in supported preview or development environments. Hosted pages use local storage for theme preferences, library folders and the cloud sign-in session, including the temporary verifier used to complete sign-in. They use session storage for comment and edit drafts, a sign-in return path, the legal notice acknowledgement associated with sign-in and callback completion state used to resume sign-in. Password-protected recording access uses an unlock cookie. Self-hosted comment accounts use a separate sign-in cookie to identify the authenticated author.

    Service providers use security and operational technologies needed to deliver their services. Blocking necessary storage may prevent sign-in or protected playback. We do not add advertising trackers to Recordly-hosted recording pages. A separately hosted marketing site or external page may have its own provider technologies.

    We use access controls and authenticated server checks to protect accounts and uploads. No storage or transmission system is completely secure, and a sharing link or password can be redistributed by its recipient.

    How long information remains

    diff --git a/src/components/auth/OnboardingController.tsx b/src/components/auth/OnboardingController.tsx index ca207933..8fa99d2f 100644 --- a/src/components/auth/OnboardingController.tsx +++ b/src/components/auth/OnboardingController.tsx @@ -31,6 +31,7 @@ export function OnboardingController({ const [returningSignIn, setReturningSignIn] = useState(false); const [startWithPermissions, setStartWithPermissions] = useState(false); const checked = useRef(false); + const finishingPermissions = useRef(false); const openWindow = useCallback(() => { setReturningSignIn(false); if (standalone) setOpen(true); @@ -102,13 +103,39 @@ export function OnboardingController({ ); const finishPermissions = () => { - if (!preview && !user) return; - close(); + if ((!preview && !user) || finishingPermissions.current) return; + finishingPermissions.current = true; + let restorePermissionsRequest = false; + let previousPreviewDismissal: boolean | null = null; + let markedPreviewDismissed = false; // Recreate the HUD before closing the last window, including on Windows/Linux. void window.electronAPI .showRecordingHud() - .then(() => window.electronAPI.closeOnboardingWindow()) - .catch(() => console.error("[onboarding] Could not return to the recording HUD.")); + .then(() => { + // Native close may destroy this renderer before its next continuation. + if (preview) { + previousPreviewDismissal = loadAppSetting(DEV_DISMISSED_KEY); + saveAppSetting(DEV_DISMISSED_KEY, true); + markedPreviewDismissed = true; + } else { + restorePermissionsRequest = + loadAppSetting("recordly.onboarding.permissionsRequested") === + true; + saveAppSetting("recordly.onboarding.permissionsRequested", false); + } + return window.electronAPI.closeOnboardingWindow(); + }) + .then(() => close()) + .catch(() => { + if (restorePermissionsRequest) + saveAppSetting("recordly.onboarding.permissionsRequested", true); + if (markedPreviewDismissed) + saveAppSetting(DEV_DISMISSED_KEY, previousPreviewDismissal); + console.error("[onboarding] Could not return to the recording HUD."); + }) + .finally(() => { + finishingPermissions.current = false; + }); }; const finish = () => { if (!preview && !user) return; diff --git a/src/components/video-editor/cloud/projectShareLinks.test.ts b/src/components/video-editor/cloud/projectShareLinks.test.ts index a1d4d94e..7922c5bc 100644 --- a/src/components/video-editor/cloud/projectShareLinks.test.ts +++ b/src/components/video-editor/cloud/projectShareLinks.test.ts @@ -309,3 +309,71 @@ it("never bases native writes on an invalid existing map", () => { expect(write).not.toHaveBeenCalled(); expect(browserRead).not.toHaveBeenCalled(); }); + +it.each([ + [false, "{broken"], + [false, "[]"], + [false, "null"], + [false, "3"], + [true, "{broken"], + [true, "[]"], + [true, "null"], + [true, "3"], +] as const)("replaces corrupt browser share links after a successful share, native settings=%s, cache=%s", (native, corrupt) => { + const key = "recordly.project-share-links.v1"; + const settings = native ? installNativeSettings() : null; + const values = new Map([[key, corrupt]]); + vi.stubGlobal("localStorage", { + getItem: (name: string) => values.get(name) ?? null, + setItem: (name: string, value: string) => values.set(name, value), + }); + expect(() => + saveProjectShareLink("current", "https://example.test/current", { accountId: "owner" }), + ).not.toThrow(); + expect(getProjectShareLinkStrict("missing")).toBeUndefined(); + expect(getProjectShare("current", "owner")).toMatchObject({ + url: "https://example.test/current", + accountId: "owner", + }); + expect(JSON.parse(values.get(key)!)).toEqual({ + current: { + url: "https://example.test/current", + accountId: "owner", + sharedAt: expect.any(Number), + }, + }); + if (settings) expect(settings.get(key)).toEqual(JSON.parse(values.get(key)!)); +}); + +it("does not treat a failed native read as permission to replace corrupt browser history", () => { + const nativeWrite = vi.fn(() => true); + const browserRead = vi.fn(() => "{broken"); + const browserWrite = vi.fn(); + vi.stubGlobal("electronAPI", { + getAppSetting: () => null, + getAppSettingWithStatus: () => ({ success: false }), + setAppSetting: nativeWrite, + }); + vi.stubGlobal("localStorage", { getItem: browserRead, setItem: browserWrite }); + expect(() => saveProjectShareLink("current", "https://example.test/current")).toThrow( + "desktop settings", + ); + expect(nativeWrite).not.toHaveBeenCalled(); + expect(browserRead).not.toHaveBeenCalled(); + expect(browserWrite).not.toHaveBeenCalled(); +}); + +it("keeps browser storage access failures fatal for strict eligibility and mutations", () => { + const browserWrite = vi.fn(); + vi.stubGlobal("localStorage", { + getItem: () => { + throw new Error("Browser storage unavailable"); + }, + setItem: browserWrite, + }); + expect(() => getProjectShareLinkStrict("current")).toThrow("Browser storage unavailable"); + expect(() => saveProjectShareLink("current", "https://example.test/current")).toThrow( + "Browser storage unavailable", + ); + expect(browserWrite).not.toHaveBeenCalled(); +}); diff --git a/src/components/video-editor/cloud/projectShareLinks.ts b/src/components/video-editor/cloud/projectShareLinks.ts index 6f783c4a..f8e0efc6 100644 --- a/src/components/video-editor/cloud/projectShareLinks.ts +++ b/src/components/video-editor/cloud/projectShareLinks.ts @@ -22,6 +22,20 @@ export type ProjectShareLink = { revision?: string; accountId?: string; }; +function readBrowserLinks(): Record { + // A corrupt browser cache must not prevent recording a new successful share. + // Keep storage access errors distinct from malformed serialized cache data. + const raw = localStorage.getItem(KEY); + let value: unknown; + try { + value = JSON.parse(raw || "{}"); + } catch { + return {}; + } + return value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : {}; +} function readLinks(): Record { const api = getNativeSettingsApi(); let missingNativeKey = false; @@ -43,9 +57,9 @@ function readLinks(): Record { // to resurrect an older browser-origin snapshot. stored = result.found ? (result.value ?? {}) - : JSON.parse(localStorage.getItem(KEY) || "{}"); + : readBrowserLinks(); } else { - stored = JSON.parse(localStorage.getItem(KEY) || "{}"); + stored = readBrowserLinks(); } if (!stored || typeof stored !== "object" || Array.isArray(stored)) throw new Error("Saved share links could not be read"); diff --git a/src/lib/exporter/nativeStaticLayoutBackground.test.ts b/src/lib/exporter/nativeStaticLayoutBackground.test.ts index 0fd4be65..1182d075 100644 --- a/src/lib/exporter/nativeStaticLayoutBackground.test.ts +++ b/src/lib/exporter/nativeStaticLayoutBackground.test.ts @@ -124,3 +124,75 @@ describe("native static-layout background asset lifecycle", () => { ).resolves.toBeUndefined(); }); }); + +describe("native bundled background containment", () => { + function resolveBundled(assetPath: string) { + return ( + createAssets("#ffffff") as unknown as { + resolveBundledAssetPath: (path: string) => Promise; + } + ).resolveBundledAssetPath(assetPath); + } + + it.each([ + true, + false, + ])("rejects nested, encoded and backslash path escapes with directory probe=%s", async (probe) => { + vi.stubGlobal("window", { + electronAPI: { + getAssetBasePath: vi.fn(async () => "file:///Recordly/resources/"), + ...(probe + ? { + listAssetDirectory: vi.fn(async () => ({ + success: true, + files: ["known.jpg"], + })), + } + : {}), + }, + }); + for (const asset of [ + "/wallpapers/known.jpg/../../../../private.png", + "/wallpapers/../private.png", + "/wallpapers/%2e%2e/private.png", + "/wallpapers/known.jpg\\..\\..\\private.png", + "/wallpapers/%2e%2e%2fprivate.png", + "/wallpapers/%2e%2e%5cprivate.png", + "/wallpapers/%2fprivate.png", + "/wallpapers/%", + "/wallpapers//known.jpg", + "/wallpapers/.", + "/app-icons/..", + "/wallpapers/..%20", + "/wallpapers/...", + "/wallpapers/known.jpg.", + "/wallpapers/known.jpg%20", + "/wallpapers/C%3aprivate.png", + "/wallpapers/known.jpg%3astream", + "/wallpapers/%00hidden.png", + "/wallpapers/tab%09.jpg", + "/wallpapers/control%7f.png", + ]) { + await expect(resolveBundled(asset), asset).resolves.toBeNull(); + } + }); + + it.each([ + "file:///Recordly/resources/", + "file:///C:/Recordly/resources/", + ])("preserves valid bundled filenames under %s", async (base) => { + vi.stubGlobal("window", { + electronAPI: { + getAssetBasePath: vi.fn(async () => base), + listAssetDirectory: vi.fn(async () => ({ + success: true, + files: ["known file.jpg"], + })), + }, + }); + const result = await resolveBundled("/wallpapers/known%20file.jpg"); + expect(result?.replace(/\\/g, "/")).toMatch( + /(?:C:)?\/Recordly\/resources\/wallpapers\/known file\.jpg$/, + ); + }); +}); diff --git a/src/lib/exporter/nativeStaticLayoutBackground.ts b/src/lib/exporter/nativeStaticLayoutBackground.ts index 196a895f..36d20fdb 100644 --- a/src/lib/exporter/nativeStaticLayoutBackground.ts +++ b/src/lib/exporter/nativeStaticLayoutBackground.ts @@ -256,34 +256,55 @@ export class NativeStaticLayoutBackgroundAssets { } private async resolveBundledAssetPath(assetPath: string): Promise { - const normalizedAssetPath = assetPath.replace(/^\/+/, ""); - const [assetDirectory, fileName] = normalizedAssetPath.split("/"); - if (!assetDirectory || !fileName) { + const segments = assetPath.replace(/^\/+/, "").split("/"); + if (segments.length !== 2) return null; + const [assetDirectory, encodedFileName] = segments; + if (assetDirectory !== "wallpapers" && assetDirectory !== "app-icons") return null; + + let fileName: string; + try { + fileName = decodeURIComponent(encodedFileName); + } catch { + return null; + } + if ( + !fileName || + fileName === "." || + fileName === ".." || + fileName !== fileName.trimEnd() || + fileName.endsWith(".") || + /[/:\\\p{Cc}]/u.test(fileName) + ) { return null; } try { const result = await window.electronAPI?.listAssetDirectory?.(assetDirectory); - if ( - result?.success && - result.files && - !result.files.includes(decodeURIComponent(fileName)) - ) { + if (result?.success && result.files && !result.files.includes(fileName)) { console.warn("[VideoExporter] Native static layout wallpaper asset is missing", { assetPath, }); return null; } } catch { - // Keep native export opportunistic when directory probing is unavailable. + // Directory probing is optional; path containment is always enforced below. } const assetBasePath = await window.electronAPI?.getAssetBasePath?.(); - if (!assetBasePath) { + if (!assetBasePath) return null; + try { + const base = new URL(assetBasePath.endsWith("/") ? assetBasePath : `${assetBasePath}/`); + const assetUrl = new URL(`${assetDirectory}/${encodeURIComponent(fileName)}`, base); + if ( + assetUrl.protocol !== base.protocol || + assetUrl.host !== base.host || + !assetUrl.pathname.startsWith(base.pathname) + ) { + return null; + } + return getLocalFilePath(assetUrl.href); + } catch { return null; } - - const assetUrl = new URL(normalizedAssetPath, assetBasePath).toString(); - return getLocalFilePath(assetUrl); } }