diff --git a/frontend/src/lib/userDraftLegacyMigration.test.ts b/frontend/src/lib/userDraftLegacyMigration.test.ts index 8d76ab2b55..45b03e26cf 100644 --- a/frontend/src/lib/userDraftLegacyMigration.test.ts +++ b/frontend/src/lib/userDraftLegacyMigration.test.ts @@ -124,6 +124,40 @@ describe('migrateLegacyUserDrafts', () => { expect(localStorage.getItem('app-u/me/garbled')).toBe('not-base64!!!') }) + it('leaves keys whose path does not match the legacy `u|f/owner/name` shape alone', () => { + // A future feature or neighbouring code might pick a key like + // `app-recent` for its own purposes. The path doesn't look like a + // Windmill item path, so the migration must skip it. + localStorage.setItem('app-recent', 'whatever') + localStorage.setItem('app-some_other_app', 'whatever') + // `flow-u/me/foo` matches the shape and would be migrated, but the + // payload also needs to look like a Windmill draft (asserted below). + localStorage.setItem('flow-u/me/foo', encodeLegacy({ flow: { value: { modules: [] } } })) + + migrateLegacyUserDrafts('main') + + expect(localStorage.getItem('app-recent')).toBe('whatever') + expect(localStorage.getItem('app-some_other_app')).toBe('whatever') + expect(localStorage.getItem('userdraft/w/main/flow/u/me/foo')).not.toBeNull() + }) + + it('skips legacy-shaped keys whose payload does not look like a Windmill draft', () => { + // `app-u/me/dash` matches LEGACY_PATH_SHAPE and decodes to valid JSON, + // but none of the App-shape fields (summary/value/policy/path) are + // present. Treat it as unrelated and leave it untouched. + const unrelated = encodeLegacy({ random: 'data', count: 7 }) + localStorage.setItem('app-u/me/dash', unrelated) + const unrelatedFlow = encodeLegacy({ stepsState: {} }) + localStorage.setItem('flow-u/me/bar', unrelatedFlow) + + migrateLegacyUserDrafts('main') + + expect(localStorage.getItem('app-u/me/dash')).toBe(unrelated) + expect(localStorage.getItem('userdraft/w/main/app/u/me/dash')).toBeNull() + expect(localStorage.getItem('flow-u/me/bar')).toBe(unrelatedFlow) + expect(localStorage.getItem('userdraft/w/main/flow/u/me/bar')).toBeNull() + }) + it('migrates multiple legacy entries in a single invocation', () => { localStorage.setItem( 'app-u/me/a', diff --git a/frontend/src/lib/userDraftLegacyMigration.ts b/frontend/src/lib/userDraftLegacyMigration.ts index 1c77f9d883..1efb5b432b 100644 --- a/frontend/src/lib/userDraftLegacyMigration.ts +++ b/frontend/src/lib/userDraftLegacyMigration.ts @@ -36,13 +36,24 @@ const LEGACY_PREFIXES: ReadonlyArray<{ prefix: string; newKind: LegacyKind }> = { prefix: 'app', newKind: 'app' } ] +/** + * A Windmill item path: `u//` or `f//`. The + * `` segment may itself contain slashes, so we don't constrain it + * past requiring at least one character. Used to reject incidentally-named + * localStorage keys (e.g. `app-recent` from a future feature, or a + * neighbouring app's data) before treating them as Windmill drafts. + */ +const LEGACY_PATH_SHAPE = /^[uf]\/[^/]+\/.+$/ + function matchLegacyKey( key: string ): { prefix: string; newKind: LegacyKind; path: string } | undefined { for (const { prefix, newKind } of LEGACY_PREFIXES) { if (key === prefix) return { prefix, newKind, path: '' } if (key.startsWith(prefix + '-')) { - return { prefix, newKind, path: key.slice(prefix.length + 1) } + const path = key.slice(prefix.length + 1) + if (!LEGACY_PATH_SHAPE.test(path)) return undefined + return { prefix, newKind, path } } } return undefined @@ -56,6 +67,33 @@ function decodeLegacyState(raw: string): unknown { } } +/** + * Per-kind shape gate. The legacy keys (`app-foo`, `flow-foo`, ...) are + * unusual enough that nothing else in the codebase has used them, but + * matching `LEGACY_PATH_SHAPE` doesn't prove the payload is actually a + * Windmill draft (any base64-of-JSON could pass). Promoting a stray payload + * would silently surface as a phantom "Restored from local storage" toast + * on the next edit, so we reject anything that doesn't carry the fields the + * legacy writers actually produced. + */ +function isPlausibleLegacyValue(kind: LegacyKind, decoded: unknown): boolean { + if (decoded == null || typeof decoded !== 'object') return false + const obj = decoded as Record + switch (kind) { + case 'flow': + // Legacy FlowBuilder wrote { flow, path, selectedId, draft_triggers, ... }. + return obj.flow != null && typeof obj.flow === 'object' + case 'app': + // Legacy AppEditor wrote the App object directly. It carries summary, + // value, policy, and path among other fields — any one of those is a + // strong signal it's actually a Windmill app payload. + return 'summary' in obj || 'value' in obj || 'policy' in obj || 'path' in obj + case 'raw_app': + // Legacy RawAppEditor wrote { files, runnables, data }. + return 'files' in obj || 'runnables' in obj || 'data' in obj + } +} + function transformLegacyValue(kind: LegacyKind, decoded: unknown): unknown { const obj = decoded as Record switch (kind) { @@ -113,7 +151,7 @@ export function migrateLegacyUserDrafts(workspace: string): void { try { const decoded = decodeLegacyState(raw) - if (decoded == null || typeof decoded !== 'object') continue + if (!isPlausibleLegacyValue(match.newKind, decoded)) continue const value = transformLegacyValue(match.newKind, decoded) const target = newKey(workspace, match.newKind, match.path) if (value !== undefined && localStorage.getItem(target) == null) {