Prevent unscoped GitHub task queries for SSH repos (#2506)
* fix: prevent unscoped ssh github task queries Co-authored-by: Orca <help@stably.ai> * test: update worktree card context menu mock Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
9adf9058e9
commit
fbf53aa4cc
|
|
@ -36,7 +36,8 @@ vi.mock('./gh-utils', () => ({
|
|||
repoPath,
|
||||
connectionId: connectionId ?? null
|
||||
}),
|
||||
ghRepoExecOptions: (context: { repoPath: string }) => ({ cwd: context.repoPath }),
|
||||
ghRepoExecOptions: (context: { repoPath: string; connectionId?: string | null }) =>
|
||||
context.connectionId ? {} : { cwd: context.repoPath },
|
||||
getOwnerRepo: getOwnerRepoMock,
|
||||
getIssueOwnerRepo: getIssueOwnerRepoMock,
|
||||
getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
|
||||
|
|
@ -548,4 +549,27 @@ describe('listWorkItems', () => {
|
|||
}
|
||||
])
|
||||
})
|
||||
|
||||
it('rejects unresolved SSH repositories without running unscoped GitHub work-item queries', async () => {
|
||||
getIssueOwnerRepoMock.mockResolvedValue(null)
|
||||
getOwnerRepoMock.mockResolvedValue(null)
|
||||
getOwnerRepoForRemoteMock.mockResolvedValue(null)
|
||||
|
||||
await expect(
|
||||
listWorkItems('/remote/repo', 10, undefined, undefined, undefined, 'ssh-1')
|
||||
).rejects.toThrow('GitHub work items require a GitHub remote for SSH repositories')
|
||||
|
||||
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
|
||||
ghExecFileAsyncMock.mockClear()
|
||||
getIssueOwnerRepoMock.mockResolvedValue(null)
|
||||
getOwnerRepoMock.mockResolvedValue(null)
|
||||
getOwnerRepoForRemoteMock.mockResolvedValue(null)
|
||||
|
||||
await expect(
|
||||
listWorkItems('/remote/repo', 10, 'is:open', undefined, undefined, 'ssh-1')
|
||||
).rejects.toThrow('GitHub work items require a GitHub remote for SSH repositories')
|
||||
|
||||
expect(ghExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -779,6 +779,19 @@ type PartialWorkItemsResult = {
|
|||
issuesError?: ClassifiedError
|
||||
}
|
||||
|
||||
function assertSshRepoHasResolvedGitHubSource(args: {
|
||||
connectionId?: string | null
|
||||
issueOwnerRepo: OwnerRepo | null
|
||||
prOwnerRepo: OwnerRepo | null
|
||||
}): void {
|
||||
if (!args.connectionId || args.issueOwnerRepo || args.prOwnerRepo) {
|
||||
return
|
||||
}
|
||||
// Why: SSH repo paths are remote-only, so gh cannot use cwd to infer repo
|
||||
// context. Without a resolved owner/repo, running gh would query local state.
|
||||
throw new Error('GitHub work items require a GitHub remote for SSH repositories')
|
||||
}
|
||||
|
||||
async function listRecentWorkItems(
|
||||
repoPath: string,
|
||||
issueOwnerRepo: OwnerRepo | null,
|
||||
|
|
@ -787,7 +800,9 @@ async function listRecentWorkItems(
|
|||
connectionId?: string | null
|
||||
): Promise<PartialWorkItemsResult> {
|
||||
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId))
|
||||
if (issueOwnerRepo || prOwnerRepo) {
|
||||
const requiresExplicitRepo = Boolean(connectionId)
|
||||
assertSshRepoHasResolvedGitHubSource({ connectionId, issueOwnerRepo, prOwnerRepo })
|
||||
if (issueOwnerRepo || prOwnerRepo || requiresExplicitRepo) {
|
||||
// Why: allSettled so a 403 on upstream issues doesn't zero out the origin
|
||||
// PR half — the UI renders partial results plus a banner for the failing
|
||||
// side, matching the parent design doc's partial-failure rule (§2).
|
||||
|
|
@ -802,19 +817,21 @@ async function listRecentWorkItems(
|
|||
],
|
||||
ghOptions
|
||||
)
|
||||
: ghExecFileAsync(
|
||||
[
|
||||
'issue',
|
||||
'list',
|
||||
'--limit',
|
||||
String(limit),
|
||||
'--state',
|
||||
'open',
|
||||
'--json',
|
||||
'number,title,state,url,labels,updatedAt,author'
|
||||
],
|
||||
ghOptions
|
||||
),
|
||||
: requiresExplicitRepo
|
||||
? Promise.resolve({ stdout: '[]' })
|
||||
: ghExecFileAsync(
|
||||
[
|
||||
'issue',
|
||||
'list',
|
||||
'--limit',
|
||||
String(limit),
|
||||
'--state',
|
||||
'open',
|
||||
'--json',
|
||||
'number,title,state,url,labels,updatedAt,author'
|
||||
],
|
||||
ghOptions
|
||||
),
|
||||
prOwnerRepo
|
||||
? ghExecFileAsync(
|
||||
[
|
||||
|
|
@ -825,19 +842,21 @@ async function listRecentWorkItems(
|
|||
],
|
||||
ghOptions
|
||||
)
|
||||
: ghExecFileAsync(
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--limit',
|
||||
String(limit),
|
||||
'--state',
|
||||
'open',
|
||||
'--json',
|
||||
WORK_ITEM_PR_LIST_JSON_FIELDS
|
||||
],
|
||||
ghOptions
|
||||
)
|
||||
: requiresExplicitRepo
|
||||
? Promise.resolve({ stdout: '[]' })
|
||||
: ghExecFileAsync(
|
||||
[
|
||||
'pr',
|
||||
'list',
|
||||
'--limit',
|
||||
String(limit),
|
||||
'--state',
|
||||
'open',
|
||||
'--json',
|
||||
WORK_ITEM_PR_LIST_JSON_FIELDS
|
||||
],
|
||||
ghOptions
|
||||
)
|
||||
])
|
||||
|
||||
let issues: MainWorkItem[] = []
|
||||
|
|
@ -946,6 +965,8 @@ async function listQueriedWorkItems(
|
|||
connectionId?: string | null
|
||||
): Promise<PartialWorkItemsResult> {
|
||||
const ghOptions = ghRepoExecOptions(githubRepoContext(repoPath, connectionId))
|
||||
const requiresExplicitRepo = Boolean(connectionId)
|
||||
assertSshRepoHasResolvedGitHubSource({ connectionId, issueOwnerRepo, prOwnerRepo })
|
||||
const hasPrOnlyFilter =
|
||||
query.state === 'merged' ||
|
||||
query.draft ||
|
||||
|
|
@ -961,6 +982,9 @@ async function listQueriedWorkItems(
|
|||
if (!issueScope) {
|
||||
return { items: [] }
|
||||
}
|
||||
if (requiresExplicitRepo && !issueOwnerRepo) {
|
||||
return { items: [] }
|
||||
}
|
||||
const args = buildWorkItemListArgs({
|
||||
kind: 'issue',
|
||||
ownerRepo: issueOwnerRepo,
|
||||
|
|
@ -983,6 +1007,9 @@ async function listQueriedWorkItems(
|
|||
if (!prScope) {
|
||||
return []
|
||||
}
|
||||
if (requiresExplicitRepo && !prOwnerRepo) {
|
||||
return []
|
||||
}
|
||||
const args = buildWorkItemListArgs({
|
||||
kind: 'pr',
|
||||
ownerRepo: prOwnerRepo,
|
||||
|
|
|
|||
|
|
@ -57,6 +57,7 @@ vi.mock('./SshDisconnectedDialog', () => ({
|
|||
vi.mock('./WorktreeContextMenu', () => ({
|
||||
default: ({ children }: { children: ReactNode }) => <>{children}</>,
|
||||
CLOSE_ALL_CONTEXT_MENUS_EVENT: 'orca:test-close-context-menus',
|
||||
WORKTREE_NATIVE_CONTEXT_MENU_ATTR: 'data-worktree-native-context-menu',
|
||||
WORKTREE_CONTEXT_MENU_SCOPE_ATTR: 'data-orca-context-menu-scope'
|
||||
}))
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue