Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
27 changes: 25 additions & 2 deletions packages/client/src/commands/gh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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 <domain> <command> [options]

Run a catalogued GitHub action through the broker.

Common
--repo <owner/name> Repository the action belongs to [required]
--repo <owner/name> Repository the action belongs to (default: origin remote)
--host <host> Host as configured, e.g. github.com (default: github.com)
--socket <path> Broker socket (default: GIT_BROKER_SOCKET)
-h, --help Show this help
Expand Down Expand Up @@ -68,8 +83,16 @@ export async function runGh(argv: readonly string[]): Promise<number> {

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');

Expand Down
27 changes: 19 additions & 8 deletions packages/client/src/commands/remotes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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() };
}

/**
Expand Down
71 changes: 69 additions & 2 deletions test/unit/gh-command.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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<RunResult> {
function runGhCli(args: readonly string[], cwd?: string): Promise<RunResult> {
return tryRunAsync(
process.execPath,
['--conditions=development', CLIENT_BIN, 'gh', ...args],
{ env: process.env, timeoutMs: 5000 },
{ cwd, env: process.env, timeoutMs: 5000 },
);
}

Expand Down Expand Up @@ -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');
Expand Down
13 changes: 9 additions & 4 deletions test/unit/remotes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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');
Expand All @@ -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', () => {
Expand Down
Loading