fix(sidebar): close legacy folder owner gaps

This commit is contained in:
Brennan Benson
2026-08-10 21:17:56 -07:00
parent 2fcc829f25
commit aa5a1ceed3
12 changed files with 275 additions and 47 deletions
+26
View File
@@ -258,6 +258,32 @@ describe('filesystem-auth path containment', () => {
}
})
it('does not authorize a repo-less legacy SSH folder through its local-stamped group', async () => {
const tempRoot = await mkdtemp(join(tmpdir(), 'orca-auth-legacy-remote-folder-'))
try {
const folderPath = join(tempRoot, 'remote-platform')
await mkdir(folderPath, { recursive: true })
const projectGroup = makeProjectGroup({
parentPath: folderPath,
connectionId: undefined,
executionHostId: 'local'
})
const folderWorkspace = makeFolderWorkspace({
folderPath,
projectGroupId: projectGroup.id,
connectionId: 'ssh-1'
})
const store = makeStore([], {
projectGroups: [projectGroup],
folderWorkspaces: [folderWorkspace]
})
await expect(resolveAuthorizedPath(folderPath, store)).rejects.toThrow('Access denied')
} finally {
await rm(tempRoot, { recursive: true, force: true })
}
})
it('does not authorize SSH-only folder-backed project group roots as local paths', async () => {
const tempRoot = await mkdtemp(join(tmpdir(), 'orca-auth-remote-project-group-'))
try {
+56 -11
View File
@@ -11,9 +11,16 @@ import {
getProjectGroupOwnerHostId,
resolveFolderWorkspaceProjectGroup
} from '../../shared/project-groups'
import { getRepoExecutionHostId, LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host'
import type { ProjectGroup, Repo } from '../../shared/types'
import { resolveFolderWorkspaceCatalogOwnerHostId } from '../../shared/folder-workspaces'
import {
getRepoExecutionHostId,
LOCAL_EXECUTION_HOST_ID,
normalizeExecutionHostId
} from '../../shared/execution-host'
import type { FolderWorkspace, ProjectGroup, Repo } from '../../shared/types'
import {
resolveFolderWorkspaceCatalogOwnerHostIdFromIndex,
resolveFolderWorkspaceProjectGroupWithLegacySsh
} from '../../shared/folder-workspaces'
export const PATH_ACCESS_DENIED_MESSAGE =
'Access denied: path resolves outside allowed directories. If this blocks a legitimate workflow, please file a GitHub issue.'
@@ -75,7 +82,9 @@ type FolderAuthIndex = {
type GroupMembershipSummary = {
repoCount: number
folderWorkspaceCount: number
hasLocalOwner: boolean
hasLocalFolderWorkspace: boolean
hasUnconnectedRepo: boolean
unsafeCycle: boolean
}
@@ -86,7 +95,8 @@ function ownerGroupKey(ownerHostId: string, groupId: string): string {
function buildFolderAuthIndex(
projectGroups: readonly ProjectGroup[],
repos: readonly Repo[]
repos: readonly Repo[],
folderWorkspaces: readonly FolderWorkspace[]
): FolderAuthIndex {
const projectGroupIndex = buildProjectGroupOwnerIndex(projectGroups)
const parentByGroup = new Map<string, string>()
@@ -156,6 +166,32 @@ function buildFolderAuthIndex(
}
}
}
for (const workspace of folderWorkspaces) {
if (
workspace.connectionId === undefined &&
!normalizeExecutionHostId(workspace.executionHostId)
) {
continue
}
const group = resolveFolderWorkspaceProjectGroupWithLegacySsh(projectGroupIndex, workspace)
if (!group) {
continue
}
const ownerHostId = resolveFolderWorkspaceCatalogOwnerHostIdFromIndex(
workspace,
projectGroupIndex
)
const summary = groupMembership.get(ownerGroupKey(getProjectGroupOwnerHostId(group), group.id))
if (summary) {
summary.folderWorkspaceCount++
summary.hasLocalFolderWorkspace ||= ownerHostId === LOCAL_EXECUTION_HOST_ID
}
const legacySummary = legacyGroupMembership.get(group.id)
if (legacySummary) {
legacySummary.folderWorkspaceCount++
legacySummary.hasLocalFolderWorkspace ||= ownerHostId === LOCAL_EXECUTION_HOST_ID
}
}
const pending = [...remainingChildren]
.filter(([, childCount]) => childCount === 0)
@@ -235,7 +271,9 @@ function buildFolderAuthIndex(
function emptyGroupMembershipSummary(): GroupMembershipSummary {
return {
repoCount: 0,
folderWorkspaceCount: 0,
hasLocalOwner: false,
hasLocalFolderWorkspace: false,
hasUnconnectedRepo: false,
unsafeCycle: false
}
@@ -243,7 +281,9 @@ function emptyGroupMembershipSummary(): GroupMembershipSummary {
function mergeGroupMembership(target: GroupMembershipSummary, child: GroupMembershipSummary): void {
target.repoCount += child.repoCount
target.folderWorkspaceCount += child.folderWorkspaceCount
target.hasLocalOwner ||= child.hasLocalOwner
target.hasLocalFolderWorkspace ||= child.hasLocalFolderWorkspace
target.hasUnconnectedRepo ||= child.hasUnconnectedRepo
target.unsafeCycle ||= child.unsafeCycle
}
@@ -293,18 +333,22 @@ function isRemoteOnlyFolderScopeWithIndex(
return true
}
const hasAnyCandidate =
legacySummary.repoCount > 0 || hasIndexedPathInside(folderPath, index.allRepoPaths)
legacySummary.repoCount > 0 ||
legacySummary.folderWorkspaceCount > 0 ||
hasIndexedPathInside(folderPath, index.allRepoPaths)
const hasLocalCandidate =
legacySummary.hasLocalOwner || hasIndexedPathInside(folderPath, index.localOwnerRepoPaths)
legacySummary.hasLocalOwner ||
legacySummary.hasLocalFolderWorkspace ||
hasIndexedPathInside(folderPath, index.localOwnerRepoPaths)
if (hasAnyCandidate && !hasLocalCandidate) {
return true
}
}
if (summary.repoCount === 0) {
if (summary.repoCount === 0 && summary.folderWorkspaceCount === 0) {
// Why: empty membership means no remote-linked evidence; authorize local folder scopes without path scans.
return false
}
if (summary.hasUnconnectedRepo) {
if (summary.hasUnconnectedRepo || summary.hasLocalFolderWorkspace) {
return false
}
if (hasIndexedPathInside(folderPath, index.unconnectedRepoPathsByOwner.get(ownerHostId) ?? [])) {
@@ -318,8 +362,9 @@ function getLocalFolderScopeRoots(store: Store): string[] {
const repos = scopeStore.getRepos()
// Why: many filesystem tests use narrow Store doubles; folder scopes are additive.
const projectGroups = scopeStore.getProjectGroups?.() ?? []
const folderWorkspaces = scopeStore.getFolderWorkspaces?.() ?? []
// Why: one owner-indexed pass avoids O((groups+folders)*(groups+repos)) remote-only checks.
const index = buildFolderAuthIndex(projectGroups, repos)
const index = buildFolderAuthIndex(projectGroups, repos, folderWorkspaces)
const roots: string[] = []
for (const group of projectGroups) {
if (
@@ -337,7 +382,7 @@ function getLocalFolderScopeRoots(store: Store): string[] {
roots.push(resolve(group.parentPath))
}
}
for (const workspace of scopeStore.getFolderWorkspaces?.() ?? []) {
for (const workspace of folderWorkspaces) {
const group = resolveFolderWorkspaceProjectGroup(index.projectGroupIndex, workspace)
if (!group && index.projectGroupIndex.byId.has(workspace.projectGroupId)) {
continue
@@ -345,7 +390,7 @@ function getLocalFolderScopeRoots(store: Store): string[] {
// Why: narrow Store doubles may omit groups; folder roots stay additive from the workspace catalog.
const ownerHostId = group
? getProjectGroupOwnerHostId(group)
: resolveFolderWorkspaceCatalogOwnerHostId(workspace, projectGroups)
: resolveFolderWorkspaceCatalogOwnerHostIdFromIndex(workspace, index.projectGroupIndex)
if (!ownerHostId) {
continue
}
@@ -1102,7 +1102,8 @@ export function renderRowContainsWorktree(
return (
folderWorkspaceKey(row.folderWorkspace.id) === worktreeId &&
(!ownerHostId ||
getProjectGroupMutationSelector(row.projectGroup).ownerHostId === ownerHostId)
resolveFolderWorkspaceCatalogOwnerHostId(row.folderWorkspace, [row.projectGroup]) ===
ownerHostId)
)
}
if (row.type === 'lineage-group') {
@@ -1261,7 +1262,9 @@ function getRenderedSidebarSelectionEntries(
} else if (row.type === 'folder-workspace') {
worktree = {
...folderWorkspaceToWorktree(row.folderWorkspace),
hostId: getProjectGroupMutationSelector(row.projectGroup).ownerHostId
hostId:
resolveFolderWorkspaceCatalogOwnerHostId(row.folderWorkspace, [row.projectGroup]) ??
getProjectGroupMutationSelector(row.projectGroup).ownerHostId
}
} else {
continue
@@ -5230,9 +5233,11 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
if (row.type === 'folder-workspace') {
const folderWorkspaceRow = row as FolderWorkspaceItemRow
const folderOwnerHostId = getProjectGroupMutationSelector(
folderWorkspaceRow.projectGroup
).ownerHostId
const folderOwnerHostId =
resolveFolderWorkspaceCatalogOwnerHostId(
folderWorkspaceRow.folderWorkspace,
projectGroups
) ?? getProjectGroupMutationSelector(folderWorkspaceRow.projectGroup).ownerHostId
const folderWorktree = {
...folderWorkspaceToWorktree(folderWorkspaceRow.folderWorkspace),
hostId: folderOwnerHostId
@@ -5844,8 +5849,14 @@ const WorktreeList = React.memo(function WorktreeList({
})
}, [defaultHostId, repos, visibleHostIdSet])
const visibleProjectGroupsForRows = useMemo(
() => filterProjectGroupsForVisibleHosts(projectGroups, visibleHostIdSet, defaultHostId),
[defaultHostId, projectGroups, visibleHostIdSet]
() =>
filterProjectGroupsForVisibleHosts(
projectGroups,
visibleHostIdSet,
defaultHostId,
folderWorkspaces
),
[defaultHostId, folderWorkspaces, projectGroups, visibleHostIdSet]
)
const ownerQualifiedProjectGroupIds = useMemo(
() => buildProjectGroupSidebarIndex(projectGroups).ambiguousIds,
@@ -182,6 +182,23 @@ describe('worktree list folder reveal', () => {
).toEqual([getProjectGroupHeaderKey(runtimeGroup.id, 'runtime:env-1')])
})
it('reveals a legacy SSH folder through its unambiguous local-stamped group', () => {
const localGroup = makeProjectGroup({ id: 'legacy-group', executionHostId: 'local' })
const legacyFolder = makeFolderWorkspace({
projectGroupId: localGroup.id,
connectionId: 'builder'
})
expect(
getFolderWorkspaceRevealGroupKeys(
folderWorkspaceKey(legacyFolder.id),
[legacyFolder],
[localGroup],
'ssh:builder'
)
).toEqual([getProjectGroupHeaderKey(localGroup.id)])
})
it('uses qualified DOM row keys only for collision-qualified folder rows', () => {
expect(
getFolderWorkspaceSidebarRowKey('folder-workspace-1', 'folder-workspace:folder-workspace-1')
@@ -5,9 +5,11 @@ import { resolveFolderWorkspaceCatalogOwnerHostId } from '../../../../shared/fol
import { folderWorkspaceKey, parseWorkspaceKey } from '../../../../shared/workspace-scope'
import {
buildProjectGroupSidebarIndex,
findProjectGroupForSidebarOwner,
findProjectGroupForFolderWorkspace,
findProjectGroupParentForSidebar,
getProjectGroupHeaderKey
getProjectGroupHeaderKey,
getProjectGroupOwnerHostId,
getProjectGroupSidebarIdentity
} from './worktree-list-groups'
function findFolderWorkspaceByKey(
@@ -107,29 +109,21 @@ export function getFolderWorkspaceRevealGroupKeys(
return []
}
const resolvedOwnerHostId =
ownerHostId ?? resolveFolderWorkspaceCatalogOwnerHostId(folderWorkspace, projectGroups)
if (!resolvedOwnerHostId) {
return []
}
const projectGroupIndex = buildProjectGroupSidebarIndex(projectGroups)
let group = findProjectGroupForSidebarOwner(
projectGroupIndex,
folderWorkspace.projectGroupId,
resolvedOwnerHostId
)
let group = findProjectGroupForFolderWorkspace(projectGroupIndex, folderWorkspace)
const keys: string[] = []
const seen = new Set<string>()
while (group) {
const identity = JSON.stringify([resolvedOwnerHostId, group.id])
const identity = getProjectGroupSidebarIdentity(group)
if (seen.has(identity)) {
break
}
seen.add(identity)
const groupOwnerHostId = getProjectGroupOwnerHostId(group)
keys.unshift(
getProjectGroupHeaderKey(
group.id,
projectGroupIndex.ambiguousIds.has(group.id) ? resolvedOwnerHostId : undefined
projectGroupIndex.ambiguousIds.has(group.id) ? groupOwnerHostId : undefined
)
)
group = findProjectGroupParentForSidebar(projectGroupIndex, group)
@@ -31,7 +31,10 @@ import {
getProjectGroupOwnerIdentity,
UNGROUPED_PROJECT_GROUP_KEY
} from '../../../../shared/project-groups'
import { getFolderWorkspaceRowKey } from '../../../../shared/folder-workspaces'
import {
getFolderWorkspaceCatalogOwnerHostId,
getFolderWorkspaceRowKey
} from '../../../../shared/folder-workspaces'
import { cloneDefaultWorkspaceStatuses } from '../../../../shared/workspace-statuses'
import type { AppState } from '../../store/types'
import { getGitHubPRCacheKey, getLegacyGitHubPRCacheKey } from '../../store/slices/github-cache-key'
@@ -235,7 +238,7 @@ export function getAmbiguousFolderWorkspaceSidebarIds(
if (!group || ambiguousIds.has(workspace.id)) {
continue
}
const ownerHostId = getProjectGroupOwnerHostId(group)
const ownerHostId = getFolderWorkspaceCatalogOwnerHostId(workspace, [group])
const existingOwnerHostId = ownerById.get(workspace.id)
if (existingOwnerHostId && existingOwnerHostId !== ownerHostId) {
ownerById.delete(workspace.id)
@@ -284,6 +287,9 @@ export function getFolderWorkspaceExecutionHostIdForRows({
}
if (projectGroup) {
const explicitProjectGroupHostId = normalizeExecutionHostId(projectGroup.executionHostId)
if (explicitProjectGroupHostId === LOCAL_EXECUTION_HOST_ID && folderWorkspace.connectionId) {
return toSshExecutionHostId(folderWorkspace.connectionId)
}
if (explicitProjectGroupHostId) {
return explicitProjectGroupHostId
}
@@ -4,6 +4,7 @@ import { addHostSectionRows } from './host-section-rows'
import type { Row } from './worktree-list-groups'
import {
filterFolderWorkspacesForVisibleHosts,
filterProjectGroupsForVisibleHosts,
getFolderPathStatusRouteOptionsForRows,
getFolderWorkspaceExecutionHostIdForRows,
getProjectGroupExecutionHostIdForRows,
@@ -78,6 +79,16 @@ describe('WorktreeList host filtering ownership', () => {
).toBe('runtime:env-1')
})
it('uses legacy SSH folder ownership over a local-stamped group', () => {
expect(
getFolderWorkspaceExecutionHostIdForRows({
folderWorkspace: folderWorkspace({ connectionId: 'builder' }),
projectGroup: group({ connectionId: undefined, executionHostId: 'local' }),
defaultHostId: 'runtime:env-1'
})
).toBe('ssh:builder')
})
it('extracts runtime route ids for folder path status requests', () => {
expect(getRuntimeEnvironmentIdForFolderPathStatusHost('runtime:env-1')).toBe('env-1')
expect(getRuntimeEnvironmentIdForFolderPathStatusHost('ssh:ssh-builder')).toBeNull()
@@ -298,4 +309,30 @@ describe('runtime folder host section rows', () => {
['runtime:env-2', 1]
])
})
it('retains a local-stamped group needed by a visible legacy SSH folder', () => {
const localGroup = group({
id: 'legacy-group',
connectionId: undefined,
executionHostId: 'local'
})
const legacyFolder = folderWorkspace({
projectGroupId: localGroup.id,
connectionId: 'builder'
})
expect(
filterProjectGroupsForVisibleHosts([localGroup], new Set(['ssh:builder']), 'local', [
legacyFolder
])
).toEqual([localGroup])
})
it('keeps groups when the caller cannot provide folder membership', () => {
const localGroup = group({ executionHostId: 'local' })
expect(
filterProjectGroupsForVisibleHosts([localGroup], new Set(['ssh:builder']), 'local')
).toEqual([localGroup])
})
})
@@ -11,8 +11,10 @@ import type { FolderWorkspace, ProjectGroup } from '../../../../shared/types'
import {
buildProjectGroupSidebarIndex,
findProjectGroupForFolderWorkspace,
findProjectGroupParentForSidebar,
getFolderWorkspaceExecutionHostIdForRows,
getProjectGroupExecutionHostIdForRows
getProjectGroupExecutionHostIdForRows,
getProjectGroupSidebarIdentity
} from './worktree-list-groups'
export { getFolderWorkspaceExecutionHostIdForRows, getProjectGroupExecutionHostIdForRows }
@@ -33,13 +35,41 @@ export function getVisibleSidebarHostIdSet(
export function filterProjectGroupsForVisibleHosts(
projectGroups: readonly ProjectGroup[],
visibleHostIdSet: ReadonlySet<ExecutionHostId> | null,
defaultHostId: ExecutionHostId
defaultHostId: ExecutionHostId,
folderWorkspaces?: readonly FolderWorkspace[]
): readonly ProjectGroup[] {
if (!visibleHostIdSet) {
if (!visibleHostIdSet || !folderWorkspaces) {
return projectGroups
}
return projectGroups.filter((group) =>
visibleHostIdSet.has(getProjectGroupExecutionHostIdForRows(group, defaultHostId))
const retainedFolderGroupIdentities = new Set<string>()
const projectGroupIndex = buildProjectGroupSidebarIndex(projectGroups)
for (const folderWorkspace of folderWorkspaces) {
let group = findProjectGroupForFolderWorkspace(projectGroupIndex, folderWorkspace)
if (
!group ||
!visibleHostIdSet.has(
getFolderWorkspaceExecutionHostIdForRows({
folderWorkspace,
projectGroup: group,
defaultHostId
})
)
) {
continue
}
while (group) {
const identity = getProjectGroupSidebarIdentity(group)
if (retainedFolderGroupIdentities.has(identity)) {
break
}
retainedFolderGroupIdentities.add(identity)
group = findProjectGroupParentForSidebar(projectGroupIndex, group)
}
}
return projectGroups.filter(
(group) =>
visibleHostIdSet.has(getProjectGroupExecutionHostIdForRows(group, defaultHostId)) ||
retainedFolderGroupIdentities.has(getProjectGroupSidebarIdentity(group))
)
}
@@ -112,6 +112,41 @@ describe('project group deletion store routing', () => {
expect(setActiveWorktree).toHaveBeenCalledWith(null)
})
it('clears an active legacy SSH folder removed with its local-stamped group', async () => {
const localStampedGroup = { ...projectGroup, executionHostId: 'local' as const }
const legacyFolder: FolderWorkspace = {
id: 'legacy-folder',
projectGroupId: localStampedGroup.id,
connectionId: 'builder',
name: 'Legacy SSH folder',
folderPath: '/remote/folder',
linkedTask: null,
comment: '',
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 1,
lastActivityAt: 0,
createdAt: 1,
updatedAt: 1
}
projectGroupsDelete.mockResolvedValue(true)
const store = createTestStore()
const setActiveWorktree = vi.fn()
store.setState({
projectGroups: [localStampedGroup],
folderWorkspaces: [legacyFolder],
activeWorktreeId: 'folder:legacy-folder',
activeWorkspaceExecutionHostId: 'ssh:builder',
setActiveWorktree
})
await expect(store.getState().deleteProjectGroup(projectGroup.id)).resolves.toBe(true)
expect(store.getState().folderWorkspaces).toEqual([])
expect(setActiveWorktree).toHaveBeenCalledWith(null)
})
it('uses the remote delete response shape before mutating local state', async () => {
runtimeEnvironmentCall.mockResolvedValue({
id: 'rpc-delete-group',
+11 -7
View File
@@ -3176,14 +3176,18 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
}
const activeWorkspaceScope = parseWorkspaceKey(stateBeforeDelete.activeWorktreeId ?? '')
if (activeWorkspaceScope?.type === 'folder') {
const activeOwnerIdentity = stateBeforeDelete.activeWorkspaceExecutionHostId
? getFolderWorkspaceIdentity(
activeWorkspaceScope.folderWorkspaceId,
stateBeforeDelete.activeWorkspaceExecutionHostId
const activeFolderWorkspace = findFolderWorkspaceOwner(
stateBeforeDelete,
activeWorkspaceScope.folderWorkspaceId,
stateBeforeDelete.activeWorkspaceExecutionHostId ?? undefined
)
const removedActiveOwner = activeFolderWorkspace
? targets.folderWorkspaceIdentities.has(
getProjectGroupRemovalFolderWorkspaceIdentity(
removalGroupIndex,
activeFolderWorkspace
) ?? ''
)
: null
const removedActiveOwner = activeOwnerIdentity
? targets.folderWorkspaceIdentities.has(activeOwnerIdentity)
: removedFolderWorkspaceIds.has(activeWorkspaceScope.folderWorkspaceId) &&
!get().folderWorkspaces.some(
(workspace) => workspace.id === activeWorkspaceScope.folderWorkspaceId
+2 -1
View File
@@ -7,6 +7,7 @@ import {
import {
buildProjectGroupOwnerIndex,
getFolderWorkspaceProjectGroupOwnerHostId,
getProjectGroupOwnerHostId,
resolveFolderWorkspaceProjectGroup,
resolveProjectGroupOwner,
type ProjectGroupOwnerIndex
@@ -30,7 +31,7 @@ export function resolveFolderWorkspaceProjectGroupWithLegacySsh(
const group = resolveProjectGroupOwner(index, workspace.projectGroupId)
return group &&
group.connectionId === undefined &&
!normalizeExecutionHostId(group.executionHostId)
getProjectGroupOwnerHostId(group) === LOCAL_EXECUTION_HOST_ID
? group
: null
}
+22
View File
@@ -188,6 +188,28 @@ describe('project-groups', () => {
).toHaveLength(1)
})
it('retains a legacy SSH folder under one local-stamped group', () => {
const localGroup = projectGroup({
id: 'local-stamped',
parentPath: '/local-stamped',
executionHostId: 'local'
})
const legacyFolder = {
id: 'legacy-folder',
projectGroupId: localGroup.id,
connectionId: 'builder'
}
expect(normalizeFolderWorkspaces([legacyFolder], [localGroup])).toHaveLength(1)
expect(
normalizeFolderWorkspaces(
[legacyFolder],
[localGroup, projectGroup({ id: localGroup.id, executionHostId: 'runtime:env-1' })]
)
).toEqual([])
})
it('does not reinterpret explicit local provenance as a unique SSH owner', () => {
const ssh = projectGroup({ id: 'same-id', connectionId: 'builder' })
const index = buildProjectGroupOwnerIndex([ssh])