Files
orca/src/relay/git-exec-validator.test.ts
T
Neil 8ac1c6e2ac perf(git): bound ref and worktree scans (#17655)
* perf(git): bound ref and worktree scans

* fix(repo-search): clamp oversized ref limits

* fix(worktree): keep strict worktree listing unshared

The shared-scan re-export flipped every `listWorktreesStrict` caller from an
isolated subprocess to the coalesced scan. `git worktree prune` in the removal
recovery path does not bump the scan generation, so a post-prune verification
could join a pre-prune scan, see the stale row, and report a successful removal
as a stale registration. The same gap defeats the post-archive-hook rechecks
that exist to catch an external Git client locking the row.

Restore the unshared export and make coalescing opt-in via
`listWorktreesSharedStrict`, which existing callers already use deliberately.

* fix(git): separate a proven absent ref from a failed probe

`show-ref --verify --quiet` exits 1 for a missing ref, but so does `wsl.exe`
when its own launch fails, so reading any exit 1 as absence collapsed
`unverifiable` into `exited`. A genuine miss prints nothing while a wrapper
failure always explains itself, so require empty stderr alongside the exit
code; a runner that reports no stderr at all keeps its exit-code contract.

That same signal removes a spawn regression: `show-ref` is a direct-git read
under WSL, and the runner retried any numeric exit through the user's
interactive login shell. The replaced `for-each-ref` exited 0 on a miss, so
absence never retried; every absent probe now would. Treat a quiet exit 1 as
Git control flow and skip the fallback.

Also narrow the hosted-review suffix fallback: the replaced
`refs/remotes/*/<base>` could not cross a slash, but `show-ref -- <base>`
matches at any depth, so `origin/feature/main` answered a query for `main`
and submitted a review against a base the provider rejects.

Refresh the real-binary compatibility contract to the shipped excludes, and
assert exact probe concurrency rather than an upper bound so a regression to
serial probing fails.
2026-08-31 16:27:28 -07:00

269 lines
9.0 KiB
TypeScript

import { describe, expect, it } from 'vitest'
import { validateGitExecArgs } from './git-exec-validator'
function expectAllowed(args: string[]): void {
expect(() => validateGitExecArgs(args)).not.toThrow()
}
function expectBlocked(args: string[], message: string): void {
expect(() => validateGitExecArgs(args)).toThrow(message)
}
describe('validateGitExecArgs', () => {
describe('allowed read-only subcommands', () => {
it.each([
[['rev-parse', '--show-toplevel']],
[['branch', '--list']],
[['log', '--oneline', '-10']],
[['show-ref', '--heads']],
[['show-ref', '--verify', '--quiet', '--', 'refs/remotes/origin/main']],
[['ls-remote', 'origin']],
[['remote', '-v']],
[['remote', 'get-url', 'origin']],
[['remote', 'show', 'origin']],
[['symbolic-ref', 'HEAD']],
[['symbolic-ref', '--short', 'HEAD']],
[['merge-base', 'main', 'HEAD']],
[['diff', '--cached', '--name-status']],
[['diff', '--cached', '--patch', '--minimal', '--no-color', '--no-ext-diff']],
[['ls-files', '--error-unmatch', 'foo.txt']],
[['config', '--get', 'user.name']],
[['config', '--get-all', 'remote.origin.url']],
[['config', '--list']],
[['config', '-l']],
[['config', '--get-regexp', 'user']],
[['check-ref-format', '--branch', 'feature/ssh-pr-head']],
[['for-each-ref', '--format=%(refname)', 'refs/remotes']],
[
[
'for-each-ref',
'--format=%(refname)%00%(refname:short)',
'--sort=-committerdate',
'refs/heads/*foo*'
]
]
])('allows %j', (args) => {
expectAllowed(args)
})
})
describe('blocked subcommands', () => {
it('rejects empty args', () => {
expectBlocked([], 'git subcommand not allowed: (empty)')
})
it.each([
'push',
'pull',
'checkout',
'reset',
'rebase',
'merge',
'stash',
'clean',
'gc',
'reflog',
'tag',
'fetch',
'worktree'
])('rejects %s', (cmd) => {
expectBlocked([cmd], 'git subcommand not allowed')
})
})
describe('global flags before subcommand', () => {
it.each([
[['-c', 'core.sshCommand=evil', 'log']],
[['--no-pager', 'log']],
[['-C', '/tmp', 'status']]
])('rejects %j', (args) => {
expectBlocked(args, 'Global git flags before the subcommand are not allowed')
})
})
describe('global denied flags', () => {
it.each([
[['log', '--output', '/tmp/leak']],
[['log', '--output=/tmp/leak']],
[['log', '-o', '/tmp/leak']],
[['rev-parse', '--exec-path=/evil']],
[['log', '--work-tree=/other']],
[['log', '--git-dir=/other/.git']],
// Pin global-deny coverage on for-each-ref so a future allowlist
// refactor that bypassed GLOBAL_DENIED_FLAGS for this subcommand fails
// loudly. The first round of for-each-ref enablement omitted these.
[['for-each-ref', '--git-dir=/other/.git', '--format=%(refname)']],
[['for-each-ref', '--output=/tmp/leak', '--format=%(refname)']],
[['for-each-ref', '--work-tree=/other']]
])('rejects %j', (args) => {
expectBlocked(args, 'Dangerous git flags are not allowed')
})
it('does not false-positive on unrelated =value flags', () => {
expectAllowed(['log', '--format=%H'])
expectAllowed(['log', '--pretty=oneline'])
})
})
describe('git config', () => {
it('rejects config without read-only flag', () => {
expectBlocked(['config', 'user.name', 'Evil'], 'restricted to read-only operations')
})
it.each([
['--add'],
['--unset'],
['--unset-all'],
['--replace-all'],
['--rename-section'],
['--remove-section'],
['--edit'],
['-e'],
['--file=/etc/passwd'],
['-f'],
['--global'],
['--system']
])('rejects config with write flag %s', (flag) => {
expectBlocked(
['config', '--list', flag, 'val'],
'git config write operations are not allowed'
)
})
})
describe('git branch', () => {
it('allows safe branch flags', () => {
expectAllowed(['branch', '--list'])
expectAllowed(['branch', '-a'])
expectAllowed(['branch', '-r'])
})
it.each(['-d', '-D', '--delete', '-m', '-M', '--move', '-c', '-C', '--copy'])(
'rejects branch %s',
(flag) => {
expectBlocked(['branch', flag, 'name'], 'Destructive git branch flags')
}
)
it('catches --delete=value compound syntax', () => {
expectBlocked(['branch', '--delete=feature'], 'Destructive git branch flags')
})
})
describe('git remote', () => {
it.each(['rm', 'rename', 'set-url', 'set-head', 'set-branches', 'prune', 'update'])(
'rejects remote %s',
(subcmd) => {
expectBlocked(['remote', subcmd, 'arg'], 'Destructive git remote operations')
}
)
it('skips flags when finding remote subcommand', () => {
expectBlocked(['remote', '-v', 'add', 'evil', 'url'], 'Destructive git remote operations')
})
it('allows the fork-PR remote add and remove shapes', () => {
expectAllowed([
'remote',
'add',
'pr-contributor-orca',
'https://github.com/contributor/orca.git'
])
expectAllowed(['remote', 'add', 'pr-contributor-orca', 'git@github.com:contributor/orca.git'])
expectAllowed(['remote', 'remove', 'pr-contributor-orca'])
})
it.each([
// Extra or missing operands are not a shape main ever sends.
[['remote', 'add', 'fork']],
[['remote', 'add', 'fork', 'https://github.com/contributor/orca.git', '--tags']],
[['remote', 'remove']],
[['remote', 'remove', 'fork', 'extra']],
// Names and URLs must pass the same rules the pushTarget RPCs enforce.
[['remote', 'add', '--mirror=push', 'https://github.com/contributor/orca.git']],
[['remote', 'add', '../escape', 'https://github.com/contributor/orca.git']],
[['remote', 'remove', '-f']],
[['remote', 'add', 'fork', 'ext::sh -c payload']],
[['remote', 'add', 'fork', 'https://evil.test/contributor/orca.git']],
[['remote', 'add', 'fork', '/etc/passwd']]
])('rejects unsafe remote write args %j', (args) => {
expectBlocked(args, 'Destructive git remote operations')
})
})
describe('git symbolic-ref', () => {
it('allows read operations', () => {
expectAllowed(['symbolic-ref', 'HEAD'])
expectAllowed(['symbolic-ref', '--short', 'HEAD'])
expectAllowed(['symbolic-ref', '-q', 'HEAD'])
})
it.each(['-d', '--delete', '-m'])('rejects symbolic-ref %s', (flag) => {
expectBlocked(['symbolic-ref', flag, 'HEAD'], 'git symbolic-ref write operations')
})
it('rejects two positional args (write form)', () => {
expectBlocked(
['symbolic-ref', 'HEAD', 'refs/heads/main'],
'git symbolic-ref write operations'
)
})
it('catches --delete=value compound syntax', () => {
expectBlocked(['symbolic-ref', '--delete=HEAD'], 'git symbolic-ref write operations')
})
})
describe('git diff', () => {
it('rejects unstaged diff reads', () => {
expectBlocked(['diff', '--name-status'], 'restricted to staged changes')
})
it('rejects arbitrary refs and pathspecs', () => {
expectBlocked(['diff', '--cached', 'HEAD~1'], 'git diff flag not allowed')
expectBlocked(['diff', '--cached', '--', 'src/file.ts'], 'git diff flag not allowed')
})
it('rejects no-index reads', () => {
expectBlocked(['diff', '--cached', '--no-index', '/etc/passwd'], 'git diff flag not allowed')
})
})
describe('git clone', () => {
it('allows only the project setup clone shape', () => {
expectAllowed(['clone', '--', 'https://github.com/stablyai/orca.git', 'orca'])
expectAllowed(['clone', '--progress', '--', 'git@github.com:stablyai/orca.git', 'orca'])
})
it.each([
[['clone', 'https://github.com/stablyai/orca.git']],
[['clone', 'https://github.com/stablyai/orca.git', 'orca']],
[['clone', '--depth=1', '--', 'https://github.com/stablyai/orca.git', 'orca']],
[['clone', '--', 'https://github.com/stablyai/orca.git', '.']],
[['clone', '--', 'https://github.com/stablyai/orca.git', '..']],
[['clone', '--', 'https://github.com/stablyai/orca.git', 'nested/orca']],
[['clone', '--', 'https://github.com/stablyai/orca.git', 'nested\\orca']]
])('rejects unsafe clone args %j', (args) => {
expectBlocked(args, 'git clone')
})
})
describe('git init and empty commit', () => {
it('allows only the SSH create-project init and empty commit shapes', () => {
expectAllowed(['init'])
expectAllowed(['commit', '--allow-empty', '-m', 'Initial commit'])
})
it.each([
[['init', '--bare']],
[['init', '/tmp/other']],
[['commit']],
[['commit', '-am', 'message']],
[['commit', '--allow-empty']],
[['commit', '--allow-empty', '-m', '']]
])('rejects unsafe create-project write args %j', (args) => {
expectBlocked(args, 'via exec is restricted')
})
})
})