wip: normalize untrusted project catalog rows at load boundaries

This commit is contained in:
Jinjing
2026-08-27 09:00:29 -07:00
parent 07b7e9e68d
commit 30e92d8dbe
9 changed files with 383 additions and 24 deletions
@@ -5,6 +5,10 @@ import { dirname } from 'node:path'
import { getDefaultPersistedState, getDefaultWorkspaceSession } from '../../shared/constants'
import type { ExecutionHostId } from '../../shared/execution-host'
import { projectHostSetupProjectionFromRepos } from '../../shared/project-host-setup-projection'
import {
normalizeProjectHostSetupRows,
normalizeProjectRows
} from '../../shared/project-catalog-row-normalization'
import { carryProjectStateThroughIdentityChange } from '../../shared/project-identity-succession'
import type { PersistedState } from '../../shared/persisted-state-types'
import type { Project, ProjectHostSetup } from '../../shared/project-types'
@@ -39,8 +43,12 @@ export function readProfileState(profileId: string, userDataPath: string): Trans
...defaults,
...parsed,
repos: arrayOrEmpty<Repo>(parsed.repos),
projects: arrayOrEmpty<Project>(parsed.projects),
projectHostSetups: arrayOrEmpty<ProjectHostSetup>(parsed.projectHostSetups),
// Why normalize: another profile's file is untrusted JSON, and a null repoId/path here would be
// carried straight into the importing app's state. Cast — these arrays stay mutably owned.
projects: normalizeProjectRows(arrayOrEmpty<Project>(parsed.projects)) as Project[],
projectHostSetups: normalizeProjectHostSetupRows(
arrayOrEmpty<ProjectHostSetup>(parsed.projectHostSetups)
) as ProjectHostSetup[],
projectGroups: arrayOrEmpty(parsed.projectGroups),
folderWorkspaces: arrayOrEmpty(parsed.folderWorkspaces),
sparsePresetsByRepo: recordOrEmpty<SparsePreset[]>(parsed.sparsePresetsByRepo),
@@ -21,7 +21,8 @@ import { normalizeLoadedUiState } from './normalize-loaded-ui-state'
import {
normalizeLoadedAutomationRuns,
normalizeLoadedHostSessions,
normalizeLoadedLocalSession
normalizeLoadedLocalSession,
normalizeLoadedProjectCatalog
} from './normalize-loaded-state-collections'
import { normalizeRetiredNameRegistryMap } from './retired-name-registry-normalization'
@@ -33,6 +34,7 @@ export function normalizeLoadedProfileState(
): PersistedState {
const { defaults, migratedExternalVisibility, osc52ClipboardNoticePending } = terminal
const { normalizedOnboarding, normalizedProjectGroups, loadedCompactWorktreeCards } = profile
const projectCatalog = normalizeLoadedProjectCatalog(parsed, markNeedsSave)
return {
...defaults,
@@ -42,6 +44,9 @@ export function normalizeLoadedProfileState(
),
projectGroups: normalizedProjectGroups,
repos: migratedExternalVisibility.repos,
// Why: persisted catalog rows are untrusted JSON; consumers call string methods on fields the type says are strings.
projects: projectCatalog.projects,
projectHostSetups: projectCatalog.projectHostSetups,
folderWorkspaces: normalizeFolderWorkspaces(parsed.folderWorkspaces, normalizedProjectGroups),
folderWorkspaceDiffComments: normalizeFolderWorkspaceDiffComments(
parsed.folderWorkspaceDiffComments
@@ -1,4 +1,8 @@
import type { PersistedState } from '../../../shared/persisted-state-types'
import {
normalizeProjectHostSetupRows,
normalizeProjectRows
} from '../../../shared/project-catalog-row-normalization'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
import { parseWorkspaceSessionSalvaging } from '../../../shared/workspace-session-salvage'
import {
@@ -64,3 +68,26 @@ export function normalizeLoadedAutomationRuns(
}
return runs
}
/**
* Repairs project/setup rows whose stored field types do not match the declared ones — a null
* `repoId` or `path` written by an older build reaches every consumer that calls `.trim()` on it.
* Marking dirty is the migration: without a save the bad rows stay on disk and are repaired again
* every launch, and this host keeps publishing them to paired clients over the wire.
*/
export function normalizeLoadedProjectCatalog(
parsed: PersistedState,
markNeedsSave: () => void
): Pick<PersistedState, 'projects' | 'projectHostSetups'> {
const projects = normalizeProjectRows(parsed.projects ?? [])
const projectHostSetups = normalizeProjectHostSetupRows(parsed.projectHostSetups ?? [])
if (projects !== parsed.projects || projectHostSetups !== parsed.projectHostSetups) {
markNeedsSave()
}
// Cast: persisted catalog rows are owned mutably by the store; the normalizer is identity-preserving
// and hands back either the input array or a fresh one, never a frozen view.
return {
projects: projects as PersistedState['projects'],
projectHostSetups: projectHostSetups as PersistedState['projectHostSetups']
}
}
@@ -9,6 +9,11 @@ import {
projectHostSetupProjectionFromRepos,
type ProjectHostSetupProjection
} from '../../../../shared/project-host-setup-projection'
import {
normalizeProjectHostSetupRow,
normalizeProjectHostSetupRows,
normalizeProjectRows
} from '../../../../shared/project-catalog-row-normalization'
import {
PROJECT_HOST_SETUP_RUNTIME_CAPABILITY,
WORKSPACE_RUN_CONTEXT_RUNTIME_CAPABILITY
@@ -51,13 +56,16 @@ export function setupWithFetchedOwner(
setup: ProjectHostSetup,
target: RuntimeClientTarget
): ProjectHostSetup {
// Why here: every setup row crossing IPC or RPC into the renderer passes through this
// adoption step, so it is where the declared field types stop being aspirational.
const adopted = normalizeProjectHostSetupRow(setup)
const hostId = getRuntimeTargetHostId(target)
if (target.kind !== 'environment') {
return setup
return adopted
}
const executionHostId = setup.executionHostId ?? setup.hostId
const executionHostId = adopted.executionHostId ?? adopted.hostId
return {
...setup,
...adopted,
hostId,
executionHostId: executionHostId === LOCAL_EXECUTION_HOST_ID ? hostId : executionHostId,
runtimeOwnerEnvironmentId: target.environmentId,
@@ -66,6 +74,15 @@ export function setupWithFetchedOwner(
}
}
function normalizeProjectCatalogProjection(
projection: ProjectHostSetupProjection
): ProjectHostSetupProjection {
return {
projects: normalizeProjectRows(projection.projects),
setups: normalizeProjectHostSetupRows(projection.setups)
}
}
async function assertProjectHostSetupRuntimeCapability(target: RuntimeClientTarget): Promise<void> {
if (target.kind !== 'environment') {
return
@@ -95,10 +112,10 @@ export async function fetchProjectHostSetupCompatibility(
if (!projectsApi?.list || !projectsApi.listHostSetups) {
throw new Error('projects_api_unavailable')
}
return {
return normalizeProjectCatalogProjection({
projects: await projectsApi.list(),
setups: await projectsApi.listHostSetups()
}
})
}
await assertProjectHostSetupRuntimeCapability(target)
const [projectResponse, setupResponse] = await Promise.all([
@@ -110,7 +127,9 @@ export async function fetchProjectHostSetupCompatibility(
})
])
return {
projects: projectResponse.projects,
// Why projects too: the same wire response carries them, and a remote host on another
// Orca version can publish a row whose declared field types do not hold.
projects: normalizeProjectRows(projectResponse.projects),
setups: setupResponse.setups.map((setup) => setupWithFetchedOwner(setup, target))
}
} catch {
@@ -16,6 +16,7 @@ import { getRepoExecutionHostId, parseExecutionHostId } from '../../../../shared
import type { RepoSlice } from '../repos/repo-state'
import { ERROR_TOAST_DURATION } from '../repos/repo-state'
import { repoWithFetchedOwner } from '../repos/owner-routing'
import { normalizeProjectRow } from '../../../../shared/project-catalog-row-normalization'
import {
assertProjectHostSetupMutationRuntimeCapabilities,
getProjectSetupRuntimeTarget,
@@ -57,6 +58,7 @@ export function createProjectHostSetupActions(
const repo = repoWithFetchedOwner(result.repo, target)
const repoHostId = getRepoExecutionHostId(repo)
const setup = setupWithFetchedOwner(result.setup, target)
const project = normalizeProjectRow(result.project)
set((s) => {
const nextRepos = s.repos.some((entry) =>
repoMatchesHostIdentity(entry, repo.id, repoHostId)
@@ -65,9 +67,9 @@ export function createProjectHostSetupActions(
repoMatchesHostIdentity(entry, repo.id, repoHostId) ? repo : entry
)
: [...s.repos, repo]
const nextProjects = s.projects.some((entry) => entry.id === result.project.id)
? s.projects.map((entry) => (entry.id === result.project.id ? result.project : entry))
: [...s.projects, result.project]
const nextProjects = s.projects.some((entry) => entry.id === project.id)
? s.projects.map((entry) => (entry.id === project.id ? project : entry))
: [...s.projects, project]
const nextSetups = s.projectHostSetups.some((entry) => entry.id === setup.id)
? s.projectHostSetups.map((entry) => (entry.id === setup.id ? setup : entry))
: [...s.projectHostSetups, setup]
@@ -80,7 +82,7 @@ export function createProjectHostSetupActions(
toast.success(translate('auto.store.slices.repos.8bb3ad7935', 'Project added'), {
description: repo.displayName
})
return { ...result, repo, setup }
return { ...result, project, repo, setup }
} catch (err) {
console.error('Failed to set up project on host:', err)
const message = err instanceof Error ? err.message : String(err)
@@ -108,15 +110,16 @@ export function createProjectHostSetupActions(
)
).result
const setup = setupWithFetchedOwner(result.setup, target)
const project = normalizeProjectRow(result.project)
set((s) => ({
projects: s.projects.some((entry) => entry.id === result.project.id)
? s.projects.map((entry) => (entry.id === result.project.id ? result.project : entry))
: [...s.projects, result.project],
projects: s.projects.some((entry) => entry.id === project.id)
? s.projects.map((entry) => (entry.id === project.id ? project : entry))
: [...s.projects, project],
projectHostSetups: s.projectHostSetups.some((entry) => entry.id === setup.id)
? s.projectHostSetups.map((entry) => (entry.id === setup.id ? setup : entry))
: [...s.projectHostSetups, setup]
}))
return { project: result.project, setup }
return { project, setup }
} catch (err) {
console.error('Failed to create project host setup:', err)
const message = err instanceof Error ? err.message : String(err)
@@ -147,6 +150,7 @@ export function createProjectHostSetupActions(
)
).result
const setup = setupWithFetchedOwner(result.setup, target)
const project = normalizeProjectRow(result.project)
const repo = result.repo ? repoWithFetchedOwner(result.repo, target) : undefined
const repoHostId = repo ? getRepoExecutionHostId(repo) : null
set((s) => ({
@@ -157,14 +161,14 @@ export function createProjectHostSetupActions(
)
: [...s.repos, repo]
: s.repos,
projects: s.projects.some((entry) => entry.id === result.project.id)
? s.projects.map((entry) => (entry.id === result.project.id ? result.project : entry))
: [...s.projects, result.project],
projects: s.projects.some((entry) => entry.id === project.id)
? s.projects.map((entry) => (entry.id === project.id ? project : entry))
: [...s.projects, project],
projectHostSetups: s.projectHostSetups.some((entry) => entry.id === setup.id)
? s.projectHostSetups.map((entry) => (entry.id === setup.id ? setup : entry))
: [...s.projectHostSetups, setup]
}))
return { ...result, repo, setup }
return { ...result, project, repo, setup }
} catch (err) {
console.error('Failed to update project host setup:', err)
const message = err instanceof Error ? err.message : String(err)
@@ -6,6 +6,7 @@ import { notifyInstalledAgentSkillsChanged } from '@/hooks/installed-agent-skill
import type { RepoSlice } from '../repos/repo-state'
import { getProjectUpdateRuntimeTarget } from './project-host-routing'
import { mergeUpdatedProjectCompatibilityProject } from './project-compatibility-core'
import { normalizeProjectRow } from '../../../../shared/project-catalog-row-normalization'
export function createProjectUpdateActions(
set: Parameters<StateCreator<AppState>>[0],
@@ -29,11 +30,13 @@ export function createProjectUpdateActions(
if (!updatedProject) {
return false
}
// Why: the merge spreads updatedProject.sourceRepoIds, which throws if the host sent a non-array.
const normalizedProject = normalizeProjectRow(updatedProject)
const runtimePreferenceChanged = 'localWindowsRuntimePreference' in updates
set((state) => ({
projects: state.projects.map((project) =>
project.id === projectId
? mergeUpdatedProjectCompatibilityProject(project, updatedProject, updates)
? mergeUpdatedProjectCompatibilityProject(project, normalizedProject, updates)
: project
),
folderWorkspacePathStatuses: {}
@@ -0,0 +1,181 @@
import { describe, expect, it } from 'vitest'
import type { Project, ProjectHostSetup } from './project-types'
import {
normalizeProjectHostSetupRow,
normalizeProjectHostSetupRows,
normalizeProjectRow,
normalizeProjectRows
} from './project-catalog-row-normalization'
function makeSetup(overrides: Record<string, unknown> = {}): ProjectHostSetup {
return {
id: 'setup-1',
projectId: 'project-1',
hostId: 'local',
repoId: 'repo-1',
path: '/Users/alice/orca',
displayName: 'orca',
setupState: 'ready',
setupMethod: 'legacy-repo',
createdAt: 1,
updatedAt: 2,
...overrides
} as unknown as ProjectHostSetup
}
function makeProject(overrides: Record<string, unknown> = {}): Project {
return {
id: 'project-1',
displayName: 'Project',
badgeColor: '#737373',
sourceRepoIds: ['repo-1'],
createdAt: 1,
updatedAt: 2,
...overrides
} as unknown as Project
}
describe('normalizeProjectHostSetupRow', () => {
it('returns the input reference when every declared type already holds', () => {
const setup = makeSetup()
expect(normalizeProjectHostSetupRow(setup)).toBe(setup)
})
it.each([
['null', null],
['undefined', undefined],
['a number', 42],
['an object', {}]
])('coerces a %s repoId to an empty string', (_label, repoId) => {
expect(normalizeProjectHostSetupRow(makeSetup({ repoId })).repoId).toBe('')
})
// Why path: `project-grouping.ts` calls `.trim()` and `parseWslUncPath()` on it during
// sidebar render, so the same bad row takes down the main window, not one error boundary.
it.each([
['null', null],
['a number', 7]
])('coerces a %s path to an empty string', (_label, path) => {
expect(normalizeProjectHostSetupRow(makeSetup({ path })).path).toBe('')
})
it('coerces every other required string field', () => {
const normalized = normalizeProjectHostSetupRow(
makeSetup({ id: null, projectId: null, displayName: null })
)
expect(normalized.id).toBe('')
expect(normalized.projectId).toBe('')
expect(normalized.displayName).toBe('')
})
it('falls back to the local host id for a non-string hostId', () => {
expect(normalizeProjectHostSetupRow(makeSetup({ hostId: null })).hostId).toBe('local')
})
// Why 0: the catalog merge already reads 0 as "unknown", so it degrades instead of
// restamping a fake timestamp that would win min()/max() against a real one.
it.each([
['null', null],
['NaN', Number.NaN],
['a string', '5']
])('coerces a %s timestamp to 0', (_label, value) => {
const normalized = normalizeProjectHostSetupRow(
makeSetup({ createdAt: value, updatedAt: value })
)
expect(normalized.createdAt).toBe(0)
expect(normalized.updatedAt).toBe(0)
})
// Why: these are only ever compared, never string-method'd — inventing a fallback would
// silently change what a corrupt row claims about itself.
it('leaves the setupState and setupMethod unions untouched', () => {
const normalized = normalizeProjectHostSetupRow(
makeSetup({ repoId: null, setupState: 'bogus', setupMethod: null })
)
expect(normalized.setupState).toBe('bogus')
expect(normalized.setupMethod).toBeNull()
})
// Why: `mergeProjectCompatibilityProject` and the setup projection branch on `in` /
// `!== undefined`, so materialising an absent optional key changes merge behaviour.
it('never adds or removes an optional key', () => {
const withoutOptionals = normalizeProjectHostSetupRow(makeSetup({ repoId: null }))
expect('connectionId' in withoutOptionals).toBe(false)
expect('worktreeBasePath' in withoutOptionals).toBe(false)
const withOptionals = normalizeProjectHostSetupRow(
makeSetup({ repoId: null, connectionId: null, worktreeBasePath: '/base' })
)
expect('connectionId' in withOptionals).toBe(true)
expect(withOptionals.worktreeBasePath).toBe('/base')
})
})
describe('normalizeProjectRow', () => {
it('returns the input reference when every declared type already holds', () => {
const project = makeProject()
expect(normalizeProjectRow(project)).toBe(project)
})
// Why: the compatibility merge spreads `sourceRepoIds`, which throws on a non-array.
it.each([
['null', null, []],
['a string', 'repo-1', []],
['an array with holes', ['repo-1', null, 3], ['repo-1']]
])('repairs a %s sourceRepoIds', (_label, sourceRepoIds, expected) => {
expect(normalizeProjectRow(makeProject({ sourceRepoIds })).sourceRepoIds).toEqual(expected)
})
it('coerces the required string fields', () => {
const normalized = normalizeProjectRow(
makeProject({ id: null, displayName: null, badgeColor: 42 })
)
expect(normalized.id).toBe('')
expect(normalized.displayName).toBe('')
expect(normalized.badgeColor).toBe('')
})
// Why: `'localWindowsRuntimePreference' in overlay` decides whether a preference clear wins.
it('never materialises an absent localWindowsRuntimePreference', () => {
const normalized = normalizeProjectRow(makeProject({ id: null }))
expect('localWindowsRuntimePreference' in normalized).toBe(false)
})
})
describe('row array normalization', () => {
it('returns the input array when nothing needed repair', () => {
const setups = [makeSetup(), makeSetup({ id: 'setup-2' })]
expect(normalizeProjectHostSetupRows(setups)).toBe(setups)
const projects = [makeProject()]
expect(normalizeProjectRows(projects)).toBe(projects)
})
// Why: TerminalPane and TabBarQuickCommandsButton use these arrays as useMemo deps, so an
// untouched row must keep its object identity across normalization.
it('reuses untouched rows and only reallocates the repaired one', () => {
const setups = [
makeSetup(),
makeSetup({ id: 'setup-2', repoId: null }),
makeSetup({ id: 's3' })
]
const normalized = normalizeProjectHostSetupRows(setups)
expect(normalized).not.toBe(setups)
expect(normalized[0]).toBe(setups[0])
expect(normalized[1]).not.toBe(setups[1])
expect(normalized[2]).toBe(setups[2])
})
// Why: coercing a field must change one value, never which rows exist — setting a "changed"
// flag on the old selector normalizer unioned in repo-derived rows and invented phantom rows.
it('preserves row count and order', () => {
const setups = [makeSetup({ repoId: null }), makeSetup({ id: 'setup-2' })]
expect(normalizeProjectHostSetupRows(setups).map((setup) => setup.id)).toEqual([
'setup-1',
'setup-2'
])
})
it('treats a non-array as empty', () => {
expect(normalizeProjectHostSetupRows(null as unknown as ProjectHostSetup[])).toEqual([])
expect(normalizeProjectRows(null as unknown as Project[])).toEqual([])
})
})
@@ -0,0 +1,106 @@
import { LOCAL_EXECUTION_HOST_ID } from './execution-host'
import type { Project, ProjectHostSetup } from './project-types'
/**
* Repairs untrusted `Project` / `ProjectHostSetup` rows so their declared field types are true.
*
* These rows reach typed code from persisted JSON and from remote Orca hosts running a different
* version, where a field the type promises is a `string` can arrive `null`, missing, or another
* type entirely — while consumers call `.trim()` on it unconditionally (crash 3bcc5be3). Call
* these at every boundary such a row enters the app, so nothing downstream has to re-guard.
*
* Coercion only. A normalizer never drops a row, never adds or removes an optional key, and
* returns its input reference untouched when it already conforms — callers keep row and array
* identity, which selectors and `useMemo` deps depend on.
*
* The string-literal unions (`setupState`, `setupMethod`) are deliberately left alone: they are
* only ever compared, so an unknown value degrades instead of crashing, and inventing a fallback
* would silently change what a corrupt row claims about itself.
*/
function isString(value: unknown): boolean {
return typeof value === 'string'
}
// Why 0 and not now(): the catalog merge already reads 0 as "timestamp unknown", not as the epoch.
function isTimestamp(value: unknown): boolean {
return typeof value === 'number' && Number.isFinite(value)
}
export function normalizeProjectHostSetupRow(setup: ProjectHostSetup): ProjectHostSetup {
const needsRepair =
!isString(setup.id) ||
!isString(setup.projectId) ||
!isString(setup.hostId) ||
!isString(setup.repoId) ||
!isString(setup.path) ||
!isString(setup.displayName) ||
!isTimestamp(setup.createdAt) ||
!isTimestamp(setup.updatedAt)
if (!needsRepair) {
return setup
}
return {
...setup,
id: isString(setup.id) ? setup.id : '',
projectId: isString(setup.projectId) ? setup.projectId : '',
hostId: isString(setup.hostId) ? setup.hostId : LOCAL_EXECUTION_HOST_ID,
repoId: isString(setup.repoId) ? setup.repoId : '',
path: isString(setup.path) ? setup.path : '',
displayName: isString(setup.displayName) ? setup.displayName : '',
createdAt: isTimestamp(setup.createdAt) ? setup.createdAt : 0,
updatedAt: isTimestamp(setup.updatedAt) ? setup.updatedAt : 0
}
}
export function normalizeProjectRow(project: Project): Project {
const sourceRepoIdsConform =
Array.isArray(project.sourceRepoIds) && project.sourceRepoIds.every(isString)
const needsRepair =
!isString(project.id) ||
!isString(project.displayName) ||
!isString(project.badgeColor) ||
!sourceRepoIdsConform ||
!isTimestamp(project.createdAt) ||
!isTimestamp(project.updatedAt)
if (!needsRepair) {
return project
}
return {
...project,
id: isString(project.id) ? project.id : '',
displayName: isString(project.displayName) ? project.displayName : '',
badgeColor: isString(project.badgeColor) ? project.badgeColor : '',
sourceRepoIds: Array.isArray(project.sourceRepoIds)
? project.sourceRepoIds.filter((repoId): repoId is string => isString(repoId))
: [],
createdAt: isTimestamp(project.createdAt) ? project.createdAt : 0,
updatedAt: isTimestamp(project.updatedAt) ? project.updatedAt : 0
}
}
function normalizeRows<T>(rows: readonly T[], normalize: (row: T) => T): readonly T[] {
if (!Array.isArray(rows)) {
return []
}
let repaired: T[] | null = null
for (let index = 0; index < rows.length; index += 1) {
const row = rows[index]!
const normalized = normalize(row)
if (normalized !== row && !repaired) {
repaired = rows.slice(0, index)
}
repaired?.push(normalized)
}
return repaired ?? rows
}
export function normalizeProjectHostSetupRows(
setups: readonly ProjectHostSetup[]
): readonly ProjectHostSetup[] {
return normalizeRows(setups, normalizeProjectHostSetupRow)
}
export function normalizeProjectRows(projects: readonly Project[]): readonly Project[] {
return normalizeRows(projects, normalizeProjectRow)
}
+8 -2
View File
@@ -1,4 +1,8 @@
import { getRepoExecutionHostId } from './execution-host'
import {
normalizeProjectHostSetupRow,
normalizeProjectRow
} from './project-catalog-row-normalization'
import { normalizeGitHubRemoteHost } from './git-remote-host-alias'
import { githubRepoIdentityKey, isDefaultGitHubHost } from './github/repository-identity-key'
import type { Project, ProjectHostSetup, ProjectProviderIdentity } from './project-types'
@@ -312,7 +316,9 @@ export function projectHostSetupProjectionFromRepos(
const project = existing
? mergeProjectRepo(existing.project, repo)
: createProjectFromRepo(repo)
const setup = createSetupFromRepo(repo, projectId)
// Why normalize here: a repo row is untrusted persisted/wire data too, and these
// constructors copy repo.path / repo.id straight onto fields consumers call .trim() on.
const setup = normalizeProjectHostSetupRow(createSetupFromRepo(repo, projectId))
projectById.set(projectId, {
project
})
@@ -320,7 +326,7 @@ export function projectHostSetupProjectionFromRepos(
}
return {
projects: [...projectById.values()].map((entry) => entry.project),
projects: [...projectById.values()].map((entry) => normalizeProjectRow(entry.project)),
setups
}
}