From bad7711930c2b5ebf614f91fffc1bce3453aeaab Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 12 Jun 2026 20:16:21 -0700 Subject: [PATCH] Guard GitHub metadata selectors Co-authored-by: Orca --- .../host-context-implementation-checklist.md | 1 + src/main/ipc/github.test.ts | 42 +++++++++++++++++++ src/main/ipc/github.ts | 4 +- src/preload/api-types.ts | 7 +++- src/preload/index.ts | 8 +++- 5 files changed, 57 insertions(+), 5 deletions(-) diff --git a/notes/host-context-implementation-checklist.md b/notes/host-context-implementation-checklist.md index 58478898375..bfc920ea9fd 100644 --- a/notes/host-context-implementation-checklist.md +++ b/notes/host-context-implementation-checklist.md @@ -231,3 +231,4 @@ Done means: reviewers and future agents can understand the vision, implementatio - [x] 2026-06-12: Scoped GitHub Tasks partial-failure retry ownership to the selected source cache scope instead of `repoPath`, so same-project/same-path multi-host failures cannot make the wrong source banner look in-flight; verified with `task-page-cache-selectors.test.ts`, targeted oxlint, and full `pnpm run typecheck`. - [x] 2026-06-12: Routed GitHub issue reads, PR check reads/details, PR comments, review replies, and review-thread resolution through explicit `TaskSourceContext` in the store/preload/main IPC boundary, and source-scoped their comment/check caches; verified with GitHub slice regressions (106 tests), focused oxlint, `git diff --check`, and full `pnpm run typecheck`. - [x] 2026-06-12: Guarded remaining GitLab path-only fallbacks by making `workItemByPath` use the same repoId/source-context selector as the rest of GitLab IPC, passing repoId from URL picker and right-sidebar GitLab review actions, and verifying source-host routing with GitLab IPC regressions plus focused oxlint and full `pnpm run typecheck`. +- [x] 2026-06-12: Guarded GitHub label and assignable-user metadata lookups with the same repo/source-context selector as task mutations, so drawer edit metadata cannot bypass source-host validation; verified with GitHub IPC regressions (17 tests), focused oxlint, and full `pnpm run typecheck`. diff --git a/src/main/ipc/github.test.ts b/src/main/ipc/github.test.ts index d64ba8fca4a..9846bd5f3cc 100644 --- a/src/main/ipc/github.test.ts +++ b/src/main/ipc/github.test.ts @@ -8,6 +8,8 @@ const { getIssueMock, listIssuesMock, listWorkItemsMock, + listLabelsMock, + listAssignableUsersMock, getAuthenticatedViewerMock, mergePRMock, setPRAutoMergeMock, @@ -22,6 +24,8 @@ const { getIssueMock: vi.fn(), listIssuesMock: vi.fn(), listWorkItemsMock: vi.fn(), + listLabelsMock: vi.fn(), + listAssignableUsersMock: vi.fn(), getAuthenticatedViewerMock: vi.fn(), mergePRMock: vi.fn(), setPRAutoMergeMock: vi.fn(), @@ -46,6 +50,8 @@ vi.mock('../github/client', () => ({ getIssue: getIssueMock, listIssues: listIssuesMock, listWorkItems: listWorkItemsMock, + listLabels: listLabelsMock, + listAssignableUsers: listAssignableUsersMock, getAuthenticatedViewer: getAuthenticatedViewerMock, mergePR: mergePRMock, setPRAutoMerge: setPRAutoMergeMock, @@ -74,6 +80,7 @@ describe('registerGitHubHandlers', () => { badgeColor: string addedAt: number connectionId?: string | null + executionHostId?: string | null issueSourcePreference?: 'origin' | 'upstream' } let repos: FixtureRepo[] = [] @@ -91,6 +98,8 @@ describe('registerGitHubHandlers', () => { getIssueMock.mockReset() listIssuesMock.mockReset() listWorkItemsMock.mockReset() + listLabelsMock.mockReset() + listAssignableUsersMock.mockReset() getAuthenticatedViewerMock.mockReset() mergePRMock.mockReset() setPRAutoMergeMock.mockReset() @@ -175,6 +184,39 @@ describe('registerGitHubHandlers', () => { expect(listWorkItemsMock).not.toHaveBeenCalled() }) + it('guards label metadata lookups with source host context', async () => { + listLabelsMock.mockResolvedValue(['bug']) + repos = [ + ...repos, + { + id: 'repo-ssh', + path: '/workspace/remote-repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'openclaw-2', + executionHostId: 'ssh:openclaw-2' + } + ] + registerGitHubHandlers(store as never, stats as never) + + await expect( + handlers['gh:listLabels'](null, { + repoPath: '/workspace/remote-repo', + repoId: 'repo-ssh', + sourceContext: { + kind: 'task-source', + provider: 'github', + projectId: 'project-1', + hostId: 'ssh:openclaw-2', + repoId: 'repo-ssh' + } + }) + ).resolves.toEqual(['bug']) + + expect(listLabelsMock).toHaveBeenCalledWith('/workspace/remote-repo', undefined, 'openclaw-2') + }) + it('forwards listIssues for registered repositories and unwraps items', async () => { listIssuesMock.mockResolvedValue({ items: [] }) diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index a75ab49cdac..2551585bd0c 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -939,12 +939,12 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi } ) - ipcMain.handle('gh:listLabels', (_event, args: { repoPath: string }) => { + ipcMain.handle('gh:listLabels', (_event, args: RepoScopedArgs) => { const repo = assertRegisteredRepo(args, store) return listLabels(repo.path, repo.issueSourcePreference, repoConnectionId(repo)) }) - ipcMain.handle('gh:listAssignableUsers', (_event, args: { repoPath: string }) => { + ipcMain.handle('gh:listAssignableUsers', (_event, args: RepoScopedArgs) => { const repo = assertRegisteredRepo(args, store) return listAssignableUsers(repo.path, repo.issueSourcePreference, repoConnectionId(repo)) }) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 9c4e5a1c31b..faee76224d4 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1314,10 +1314,15 @@ export type PreloadApi = { sourceContext?: TaskSourceContext | null } ) => Promise - listLabels: (args: { repoPath: string; repoId?: string }) => Promise + listLabels: (args: { + repoPath: string + repoId?: string + sourceContext?: TaskSourceContext | null + }) => Promise listAssignableUsers: (args: { repoPath: string repoId?: string + sourceContext?: TaskSourceContext | null }) => Promise /** * Subscribe to local-mutation broadcasts. Used by the work-item-drawer diff --git a/src/preload/index.ts b/src/preload/index.ts index d7e87f4f6a6..9d9eba83bc7 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1169,12 +1169,16 @@ const api = { body: string }): Promise => ipcRenderer.invoke('gh:addPRReviewComment', args), - listLabels: (args: { repoPath: string; repoId?: string }): Promise => - ipcRenderer.invoke('gh:listLabels', args), + listLabels: (args: { + repoPath: string + repoId?: string + sourceContext?: TaskSourceContext | null + }): Promise => ipcRenderer.invoke('gh:listLabels', args), listAssignableUsers: (args: { repoPath: string repoId?: string + sourceContext?: TaskSourceContext | null }): Promise => ipcRenderer.invoke('gh:listAssignableUsers', args), // Why: every renderer subscribes to local mutation broadcasts so each