From 01de449a99bf68c3f2870940e662ec8366228277 Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Sun, 9 Aug 2026 23:13:45 -0700 Subject: [PATCH] fix: harden malformed renderer state normalization --- .../profile-project-transfer.test.ts | 37 ++++++++++++++ src/main/persistence.test.ts | 37 ++++++++++++++ .../src/store/plugin-language-packs.test.ts | 50 +++++++++++++++++++ .../plugin-language-pack-artifact.test.ts | 21 ++++++++ .../plugins/plugin-language-pack-artifact.ts | 24 ++++++--- ...oject-source-repo-id-normalization.test.ts | 47 +++++++++++++++++ .../project-source-repo-id-normalization.ts | 36 +++++++++---- 7 files changed, 236 insertions(+), 16 deletions(-) create mode 100644 src/shared/project-source-repo-id-normalization.test.ts diff --git a/src/main/orca-profiles/profile-project-transfer.test.ts b/src/main/orca-profiles/profile-project-transfer.test.ts index b73833e17cd..864bdda1b81 100644 --- a/src/main/orca-profiles/profile-project-transfer.test.ts +++ b/src/main/orca-profiles/profile-project-transfer.test.ts @@ -144,6 +144,43 @@ describe('profile project transfer', () => { ) }) + it('loads null and malformed project containers without inventing project rows', async () => { + const validProject = { + id: 'folder-project', + displayName: 'Folder Project', + badgeColor: '#737373', + sourceRepoIds: [], + createdAt: 1, + updatedAt: 1 + } + const validSetup = { + id: 'folder-project::local', + projectId: 'folder-project', + hostId: 'local' as const, + repoId: '', + path: 'C:\\workspace\\folder-project', + displayName: 'Folder Project', + setupState: 'ready' as const, + setupMethod: 'imported-existing-folder' as const, + createdAt: 1, + updatedAt: 1 + } + writeProfileState('personal', makeState({ projects: null as never })) + writeProfileState( + 'work', + makeState({ + projects: [null, 42, [], validProject, 'project'] as never, + projectHostSetups: [validSetup] + }) + ) + + const { readProfileState: readPersistedProfileState } = + await import('./profile-project-state-file') + + expect(readPersistedProfileState('personal', testState.dir).projects).toEqual([]) + expect(readPersistedProfileState('work', testState.dir).projects).toEqual([validProject]) + }) + it('copies a project into another profile with a new repo id and re-keyed metadata', async () => { const sourceWorktreeId = 'repo-1::/workspace/orca-feature' writeProfileState( diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 0bc2084b931..bf5ea38d79b 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -590,6 +590,43 @@ describe('Store', () => { }) }) + it('loads a null persisted project container as empty', async () => { + writeDataFile({ + ...getDefaultPersistedState(testState.dir), + projects: null + }) + + const store = await createStore() + + expect(store.getProjects()).toEqual([]) + store.flush() + expect((readDataFile() as PersistedState).projects).toEqual([]) + }) + + it('drops malformed persisted project members before compatibility projection', async () => { + const first = makeProject({ id: 'folder-first', displayName: 'First' }) + const second = makeProject({ id: 'folder-second', displayName: 'Second' }) + const firstSetup = makeProjectHostSetup({ + id: 'folder-first::local', + projectId: first.id, + path: 'C:\\workspace\\first' + }) + const secondSetup = makeProjectHostSetup({ + id: 'folder-second::local', + projectId: second.id, + path: 'C:\\workspace\\second' + }) + writeDataFile({ + ...getDefaultPersistedState(testState.dir), + projects: [null, first, 42, [], second, 'project'], + projectHostSetups: [firstSetup, secondSetup] + }) + + const store = await createStore() + + expect(store.getProjects()).toEqual([first, second]) + }) + it('updates and persists a project Windows runtime preference', async () => { const project = makeProject({ id: 'project-1', diff --git a/src/renderer/src/store/plugin-language-packs.test.ts b/src/renderer/src/store/plugin-language-packs.test.ts index 664405a26e0..812d5f60153 100644 --- a/src/renderer/src/store/plugin-language-packs.test.ts +++ b/src/renderer/src/store/plugin-language-packs.test.ts @@ -60,6 +60,56 @@ describe('plugin language pack loading', () => { expect(usePluginLanguagePackStore.getState().packs).toEqual([first, second]) }) + it.each([ + ['Date', new Date(0)], + ['Map', new Map([['greeting', 'Hello']])] + ])('rejects a %s language-pack catalog at renderer ingress', async (_name, catalog) => { + const id = 'plugin:malformed' as const + vi.stubGlobal('window', { + api: { + plugins: { + listLanguagePacks: vi.fn().mockResolvedValue([ + { + id, + resourceLanguage: pluginLanguageResourceId(id), + pluginKey: 'malformed', + locale: 'en', + catalog + } + ]) + } + } + }) + + await usePluginLanguagePackStore.getState().fetchPacks() + + expect(usePluginLanguagePackStore.getState().packs).toEqual([]) + }) + + it('rejects a structured-cloned cyclic catalog before i18next resource installation', async () => { + const id = 'plugin:cyclic' as const + const catalog: Record = { greeting: 'Hello' } + catalog.self = catalog + const response = structuredClone([ + { + id, + resourceLanguage: pluginLanguageResourceId(id), + pluginKey: 'cyclic', + locale: 'en', + catalog + } + ]) + vi.stubGlobal('window', { + api: { plugins: { listLanguagePacks: vi.fn().mockResolvedValue(response) } } + }) + + await usePluginLanguagePackStore.getState().fetchPacks() + const packs = usePluginLanguagePackStore.getState().packs + + expect(() => setRendererPluginLanguagePacks(packs)).not.toThrow() + expect(packs).toEqual([]) + }) + it('rejects an inconsistent resource language before i18next owns its cleanup', async () => { vi.stubGlobal('window', { api: { diff --git a/src/shared/plugins/plugin-language-pack-artifact.test.ts b/src/shared/plugins/plugin-language-pack-artifact.test.ts index e684bb50095..85c7841e599 100644 --- a/src/shared/plugins/plugin-language-pack-artifact.test.ts +++ b/src/shared/plugins/plugin-language-pack-artifact.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it } from 'vitest' import { parsePluginLanguagePackArtifact, PLUGIN_LANGUAGE_CATALOG_MAX_DEPTH, + PLUGIN_LANGUAGE_CATALOG_MAX_ENTRIES, + validatePluginLanguagePackCatalog, pluginLanguageResourceId } from './plugin-language-pack-artifact' @@ -163,4 +165,23 @@ describe('plugin language-pack artifacts', () => { error: expect.stringContaining('depth') }) }) + + it('accepts the entry limit and rejects one additional entry', () => { + const catalog = Object.fromEntries( + Array.from({ length: PLUGIN_LANGUAGE_CATALOG_MAX_ENTRIES }, (_, index) => [ + `key${index}`, + 'value' + ]) + ) + + expect(validatePluginLanguagePackCatalog(catalog)).toMatchObject({ + ok: true, + entries: PLUGIN_LANGUAGE_CATALOG_MAX_ENTRIES + }) + catalog.overflow = 'value' + expect(validatePluginLanguagePackCatalog(catalog)).toMatchObject({ + ok: false, + error: expect.stringContaining(`${PLUGIN_LANGUAGE_CATALOG_MAX_ENTRIES} entries`) + }) + }) }) diff --git a/src/shared/plugins/plugin-language-pack-artifact.ts b/src/shared/plugins/plugin-language-pack-artifact.ts index c344184479c..68f790f57cc 100644 --- a/src/shared/plugins/plugin-language-pack-artifact.ts +++ b/src/shared/plugins/plugin-language-pack-artifact.ts @@ -37,9 +37,7 @@ export function isPluginLanguagePackRegistration( pack.resourceLanguage === pluginLanguageResourceId(pack.id as `plugin:${string}`) && typeof pack.pluginKey === 'string' && typeof pack.locale === 'string' && - typeof pack.catalog === 'object' && - pack.catalog !== null && - !Array.isArray(pack.catalog) + validatePluginLanguagePackCatalog(pack.catalog).ok ) } @@ -65,7 +63,12 @@ type CatalogFrame = { } function isCatalogObject(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value) + return ( + typeof value === 'object' && + value !== null && + !Array.isArray(value) && + Object.getPrototypeOf(value) === Object.prototype + ) } function protectedTranslation(path: string): boolean { @@ -89,12 +92,17 @@ export function parsePluginLanguagePackArtifact(raw: string): PluginLanguagePack } catch { return { ok: false, error: 'language pack must contain one JSON object' } } - if (!isCatalogObject(json)) { + return validatePluginLanguagePackCatalog(json) +} + +export function validatePluginLanguagePackCatalog(source: unknown): PluginLanguagePackParseResult { + if (!isCatalogObject(source)) { return { ok: false, error: 'language pack root must be an object' } } const catalog: Record = {} - const stack: CatalogFrame[] = [{ source: json, target: catalog, path: '', depth: 0 }] + const stack: CatalogFrame[] = [{ source, target: catalog, path: '', depth: 0 }] + const seen = new WeakSet([source]) let entries = 0 while (stack.length > 0) { const frame = stack.pop()! @@ -136,6 +144,10 @@ export function parsePluginLanguagePackArtifact(raw: string): PluginLanguagePack if (!isCatalogObject(value)) { return { ok: false, error: `translation at ${path} must be a string or object` } } + if (seen.has(value)) { + return { ok: false, error: `catalog contains a repeated or cyclic object at ${path}` } + } + seen.add(value) const child: Record = {} frame.target[key] = child stack.push({ source: value, target: child, path, depth: frame.depth + 1 }) diff --git a/src/shared/project-source-repo-id-normalization.test.ts b/src/shared/project-source-repo-id-normalization.test.ts new file mode 100644 index 00000000000..ef252690442 --- /dev/null +++ b/src/shared/project-source-repo-id-normalization.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import type { Project } from './types' +import { normalizePersistedProjectSourceRepoIds } from './project-source-repo-id-normalization' + +function makeProject(id: string, sourceRepoIds: unknown = []): Project { + return { + id, + displayName: id, + badgeColor: '#737373', + sourceRepoIds, + createdAt: 1, + updatedAt: 1 + } as Project +} + +describe('normalizePersistedProjectSourceRepoIds', () => { + it.each([null, undefined, {}, 'projects'])('treats a non-array container as empty', (input) => { + expect(normalizePersistedProjectSourceRepoIds(input)).toEqual({ projects: [], changed: true }) + }) + + it('drops malformed members while preserving valid row identity and order', () => { + const first = Object.freeze(makeProject('first')) + const second = Object.freeze(makeProject('second', ['repo-2'])) + const input = Object.freeze([null, first, 42, [], second, 'project']) + + const result = normalizePersistedProjectSourceRepoIds(input) + + expect(result).toEqual({ projects: [first, second], changed: true }) + expect(result.projects[0]).toBe(first) + expect(result.projects[1]).toBe(second) + }) + + it('copies frozen rows only when source repo ids need repair', () => { + const missing = Object.freeze(makeProject('missing', null)) + const mixedIds = Object.freeze(makeProject('mixed', Object.freeze(['repo-1', null, 7]))) + const input = Object.freeze([missing, mixedIds]) + + const result = normalizePersistedProjectSourceRepoIds(input) + + expect(result.projects).toEqual([ + { ...missing, sourceRepoIds: [] }, + { ...mixedIds, sourceRepoIds: ['repo-1'] } + ]) + expect(result.projects[0]).not.toBe(missing) + expect(result.projects[1]).not.toBe(mixedIds) + }) +}) diff --git a/src/shared/project-source-repo-id-normalization.ts b/src/shared/project-source-repo-id-normalization.ts index 11fdeb7e5ec..4ce400be82a 100644 --- a/src/shared/project-source-repo-id-normalization.ts +++ b/src/shared/project-source-repo-id-normalization.ts @@ -1,24 +1,40 @@ import type { Project } from './types' -export function normalizePersistedProjectSourceRepoIds(projects: readonly Project[]): { +function isProjectRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +export function normalizePersistedProjectSourceRepoIds(projects: unknown): { projects: Project[] changed: boolean } { + if (!Array.isArray(projects)) { + return { projects: [], changed: true } + } + let changed = false - const normalizedProjects = projects.map((project) => { + const normalizedProjects: Project[] = [] + for (const candidate of projects) { + if (!isProjectRecord(candidate)) { + changed = true + continue + } + const project = candidate as Project + const sourceRepoIds = candidate.sourceRepoIds if ( - Array.isArray(project.sourceRepoIds) && - project.sourceRepoIds.every((repoId) => typeof repoId === 'string') + Array.isArray(sourceRepoIds) && + sourceRepoIds.every((repoId) => typeof repoId === 'string') ) { - return project + normalizedProjects.push(project) + continue } changed = true - return { + normalizedProjects.push({ ...project, - sourceRepoIds: Array.isArray(project.sourceRepoIds) - ? project.sourceRepoIds.filter((repoId): repoId is string => typeof repoId === 'string') + sourceRepoIds: Array.isArray(sourceRepoIds) + ? sourceRepoIds.filter((repoId): repoId is string => typeof repoId === 'string') : [] - } - }) + }) + } return { projects: normalizedProjects, changed } }