diff --git a/README.md b/README.md index e650da55..3f66ec27 100644 --- a/README.md +++ b/README.md @@ -121,6 +121,9 @@ gcb gh pr create --repo owner/name --head feat/x --title "A title" --body "A bod gcb gh run view 123456 --log --repo owner/name ``` +Inside a checkout, `--repo` can be omitted only when `origin` names a usable GitHub repository whose +host matches `--host`; otherwise pass `--repo`, or the matching `--host`. + The exact command, flags and required values come from [`packages/cli/src/actions.ts`](packages/cli/src/actions.ts); `gcb gh --help` renders that catalog. diff --git a/packages/client/src/commands/gh.ts b/packages/client/src/commands/gh.ts index f8c20563..d8ebe440 100644 --- a/packages/client/src/commands/gh.ts +++ b/packages/client/src/commands/gh.ts @@ -7,6 +7,7 @@ import { GH_ACTIONS, getGhAction, parseArgs, say, fail, type ActionValues } from 'gcb-cli'; import { requestOverSocket, resolveSocketPath } from '../helper.ts'; +import { remoteRepo } from './remotes.ts'; const ACTION_LINES = Object.values(GH_ACTIONS) .map((action) => { @@ -29,13 +30,27 @@ const ACTION_LINES = Object.values(GH_ACTIONS) }) .join('\n'); +/** + * Read owner/repo from the checkout's `origin` remote. + * + * The URL parsing lives in `remotes.ts`, beside `--head origin:branch`, so the two cannot disagree + * about what a remote points at. + * + * @param cwd - the checkout to ask. + * @returns the normalized repository, or undefined when no remote names one. + */ +function repoFromCheckout(cwd: string): { repo: string; host: string } | undefined { + const found = remoteRepo('origin', cwd); + return found === undefined ? undefined : { repo: `${found.owner}/${found.repo}`, host: found.host }; +} + /** Usage text for `gh`. */ export const GH_USAGE = `Usage: gcb gh [options] Run a catalogued GitHub action through the broker. Common - --repo Repository the action belongs to [required] + --repo Repository the action belongs to (default: origin remote) --host Host as configured, e.g. github.com (default: github.com) --socket Broker socket (default: GIT_BROKER_SOCKET) -h, --help Show this help @@ -68,8 +83,16 @@ export async function runGh(argv: readonly string[]): Promise { const booleans = action.flags.filter((flag) => flag.type === 'boolean').map((flag) => flag.name); const args = parseArgs(rest, { booleans }); - const repo = args.value('repo') ?? fail('--repo is required'); const host = args.value('host') ?? 'github.com'; + let repo = args.value('repo'); + if (repo === undefined) { + const found = repoFromCheckout(process.cwd()); + if (found === undefined) fail('--repo is required when the checkout has no usable origin remote'); + if (found.host !== host.toLowerCase()) { + fail(`the checkout's remote is on ${found.host}, but --host is ${host}; pass --repo or --host`); + } + repo = found.repo; + } const socketPath = resolveSocketPath(args.value('socket'), process.env); if (socketPath === undefined) fail('no broker socket is configured'); diff --git a/packages/client/src/commands/remotes.ts b/packages/client/src/commands/remotes.ts index b60c13d0..c2bf009a 100644 --- a/packages/client/src/commands/remotes.ts +++ b/packages/client/src/commands/remotes.ts @@ -12,10 +12,12 @@ */ import { execFileSync } from 'node:child_process'; -/** What a remote's URL says: the owner and the repository, for a GitHub-shaped URL. */ +/** What a remote's URL says: the owner, repository and host key, for a GitHub-shaped URL. */ export interface RemoteRepo { readonly owner: string; readonly repo: string; + /** The host key: `host:port` for HTTP(S), or the host alone for SSH and git URLs. */ + readonly host: string; } /** @@ -58,7 +60,7 @@ export function listRemotes(cwd: string): string[] { * * @param name - the remote name. * @param cwd - the directory to ask. - * @returns the owner and repository, or undefined when there is no such remote or the URL is not one. + * @returns the owner, repository and host, or undefined when there is no such remote or the URL is not one. */ export function remoteRepo(name: string, cwd: string, push = false): RemoteRepo | undefined { // `--push` for a head: git lets a remote fetch from one place and push to another, and the branch a @@ -70,22 +72,31 @@ export function remoteRepo(name: string, cwd: string, push = false): RemoteRepo // path, a `file:` URL, a host with no dot — is refused rather than read as an owner that happens to be a // port number or a directory name. let path: string | undefined; - if (/^[a-z+]+:\/\//i.test(url)) { + let host: string | undefined; + const scheme = /^([a-z+]+):\/\//i.exec(url)?.[1]?.toLowerCase(); + if (scheme !== undefined) { const rest = url.replace(/^[a-z+]+:\/\//i, '').replace(/^[^@/]*@/, ''); const slash = rest.indexOf('/'); const authority = slash < 0 ? '' : rest.slice(0, slash); - const host = authority.split(':')[0] ?? ''; - if (slash >= 0 && host.includes('.')) path = rest.slice(slash + 1); + const hostname = authority.split(':')[0] ?? ''; + if (slash >= 0 && hostname.includes('.')) { + // An HTTP(S) port is part of the host key the broker matches; an SSH or git port is not. + host = scheme === 'http' || scheme === 'https' ? authority : hostname; + path = rest.slice(slash + 1); + } } else { const scp = /^(?:[^@/]+@)?([^/:]+\.[^/:]+):(.+)$/.exec(url); - if (scp !== null && scp[2] !== undefined) path = scp[2]; + if (scp !== null && scp[1] !== undefined && scp[2] !== undefined) { + host = scp[1]; + path = scp[2]; + } } - if (path === undefined) return undefined; + if (path === undefined || host === undefined) return undefined; const cleaned = path.replace(/^\/+/, '').replace(/\/+$/, '').replace(/\.git$/i, ''); const [owner, repo, ...rest] = cleaned.split('/').filter((part) => part !== ''); if (owner === undefined || repo === undefined || rest.length > 0) return undefined; - return { owner, repo }; + return { owner, repo, host: host.toLowerCase() }; } /** diff --git a/test/unit/gh-command.test.ts b/test/unit/gh-command.test.ts index e9f020b9..a7edf1f4 100644 --- a/test/unit/gh-command.test.ts +++ b/test/unit/gh-command.test.ts @@ -1,4 +1,5 @@ import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -13,11 +14,11 @@ import type { AuditSink, BrokerConfig, Provider, StaticHostConfig } from '../../ const CLIENT_BIN = fileURLToPath(new URL('../../packages/client/bin/gcb.js', import.meta.url)); /** Run the installed client as a child, so its output never replaces the test reporter. */ -function runGhCli(args: readonly string[]): Promise { +function runGhCli(args: readonly string[], cwd?: string): Promise { return tryRunAsync( process.execPath, ['--conditions=development', CLIENT_BIN, 'gh', ...args], - { env: process.env, timeoutMs: 5000 }, + { cwd, env: process.env, timeoutMs: 5000 }, ); } @@ -133,6 +134,72 @@ describe('gcb gh', () => { } }); + it('resolves the repository from the checkout remote', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-remote-')); + const checkout = path.join(root, 'checkout'); + fs.mkdirSync(checkout); + execFileSync('git', ['init', '--quiet'], { cwd: checkout }); + execFileSync('git', ['remote', 'add', 'origin', 'git@github.com:acme/widget.git'], { cwd: checkout }); + const socketPath = path.join(root, 'broker.sock'); + const block: StaticHostConfig = { provider: 'static', allow: ['acme/widget'] }; + const config: BrokerConfig = { + socketPath, + socketMode: 0o600, + auditPath: null, + tokenCacheSkewSeconds: 300, + hosts: { 'github.com': block }, + }; + const audit: AuditSink = { path: null, record: () => {}, close: () => Promise.resolve() }; + const provider: Provider = { + name: 'static', + getCredential: async () => ({ username: 'u', password: 'p', expiresAt: Date.now() + 1000 }), + runAction: async (request) => { + assert.equal(request.full, 'acme/widget'); + return { stdout: 'ok\n', stderr: '', code: 0, signal: null, truncated: false, timedOut: false }; + }, + }; + const broker = await startBroker({ config, audit, providers: new Map([['github.com', provider]]) }); + try { + const result = await runGhCli(['pr', 'view', '7', '--socket', socketPath], checkout); + assert.equal(result.status, 0); + assert.equal(result.stdout, 'ok\n'); + } finally { + await broker.close(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('refuses a remote URL that names more than owner/repo', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-remote-path-')); + try { + const checkout = path.join(root, 'checkout'); + fs.mkdirSync(checkout); + execFileSync('git', ['init', '--quiet'], { cwd: checkout }); + execFileSync('git', ['remote', 'add', 'origin', 'https://github.com/acme/widget/issues'], { cwd: checkout }); + const result = await runGhCli(['pr', 'view', '7', '--socket', path.join(root, 'broker.sock')], checkout); + assert.equal(result.status, 1); + assert.match(result.stderr, /no usable origin remote/); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('refuses a checkout remote on another host', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-remote-host-')); + try { + const checkout = path.join(root, 'checkout'); + fs.mkdirSync(checkout); + execFileSync('git', ['init', '--quiet'], { cwd: checkout }); + execFileSync('git', ['remote', 'add', 'origin', 'git@gitlab.com:acme/widget.git'], { cwd: checkout }); + const result = await runGhCli(['pr', 'view', '7', '--socket', path.join(root, 'broker.sock')], checkout); + assert.equal(result.status, 1); + assert.match(result.stderr, /gitlab\.com/); + assert.match(result.stderr, /--host/); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + it('sends a typed action to the broker and prints its stdout', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-gh-command-')); const socketPath = path.join(root, 'broker.sock'); diff --git a/test/unit/remotes.test.ts b/test/unit/remotes.test.ts index c750f566..b03b62ae 100644 --- a/test/unit/remotes.test.ts +++ b/test/unit/remotes.test.ts @@ -30,8 +30,8 @@ describe('resolving a branch that names a remote', () => { }); it('reads an SSH url and an HTTPS one the same way', () => { - assert.deepEqual(remoteRepo('origin', dir), { owner: 'me', repo: 'git-credential-broker' }); - assert.deepEqual(remoteRepo('upstream', dir), { owner: 'code-vaults', repo: 'git-credential-broker' }); + assert.deepEqual(remoteRepo('origin', dir), { owner: 'me', repo: 'git-credential-broker', host: 'github.com' }); + assert.deepEqual(remoteRepo('upstream', dir), { owner: 'code-vaults', repo: 'git-credential-broker', host: 'github.com' }); }); it('leaves a plain branch alone, because it is a branch of the repository --repo names', () => { @@ -69,6 +69,7 @@ describe('what a remote URL is allowed to look like', () => { const git = (...args: string[]) => execFileSync('git', args, { cwd: dir, stdio: 'ignore' }); git('init', '-q'); git('remote', 'add', 'ported', 'https://git.example.com:8443/me/thing.git'); + git('remote', 'add', 'ssh-ported', 'ssh://git@git.example.com:2222/me/thing.git'); git('remote', 'add', 'prefixed', 'https://git.example.com/gitlab/group/thing.git'); git('remote', 'add', 'local', '/srv/git/thing.git'); git('remote', 'add', 'files', 'file:///srv/git/thing.git'); @@ -78,8 +79,12 @@ describe('what a remote URL is allowed to look like', () => { rmSync(dir, { recursive: true, force: true }); }); - it('does not read a port as an owner', () => { - assert.deepEqual(remoteRepo('ported', dir), { owner: 'me', repo: 'thing' }); + it('does not read a port as an owner, and keeps it in an HTTP(S) host', () => { + assert.deepEqual(remoteRepo('ported', dir), { owner: 'me', repo: 'thing', host: 'git.example.com:8443' }); + }); + + it('drops an SSH port, because the API host has none', () => { + assert.deepEqual(remoteRepo('ssh-ported', dir), { owner: 'me', repo: 'thing', host: 'git.example.com' }); }); it('does not read a directory as a host', () => {