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
57 changes: 55 additions & 2 deletions packages/cezar/src/server/checkout.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { execFileSync } from 'node:child_process';
import {
existsSync,
mkdirSync,
Expand All @@ -11,12 +12,20 @@ import {
import { mkdir, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { RunStore } from '../runs/store.ts';
import type { RunManager } from '../workflows/run.ts';
import { mergeWriteWorkspaceConfig } from '../workspace/config.ts';
import { clearProjectProbeCache, registerProject } from '../workspace/projects.ts';
import { checkoutRepo, cleanupCheckout, isValidCheckoutName, parseRepoRef, type CloneRunner } from './checkout.ts';
import {
checkoutRepo,
cleanupCheckout,
ghCloneArgs,
ghCloneRunner,
isValidCheckoutName,
parseRepoRef,
type CloneRunner,
} from './checkout.ts';
import { apiRequest } from './loopback-request.testkit.ts';
import {
WorkspaceEventBus,
Expand Down Expand Up @@ -54,6 +63,7 @@ describe('checkout — repo reference parsing', () => {
owner: 'open-mercato',
repo: 'cezar',
slug: 'open-mercato/cezar',
cloneUrl: 'https://github.com/open-mercato/cezar.git',
});
}
});
Expand Down Expand Up @@ -83,6 +93,49 @@ describe('checkout — repo reference parsing', () => {
expect(isValidCheckoutName(name), name).toBe(false);
}
});

});

describe('checkout — GitHub transport', () => {
it('forces the validated HTTPS URL so a global SSH preference cannot bypass the OAuth grant', () => {
const ref = parseRepoRef('git@github.com:open-mercato/cezar.git');
expect(ref).not.toBeNull();
expect(ghCloneArgs(ref!, '/checkouts/cezar')).toEqual([
'repo',
'clone',
'https://github.com/open-mercato/cezar.git',
'/checkouts/cezar',
'--',
'--progress',
]);
});
});

describe('checkout — persisted GitHub credentials', () => {
it('leaves HTTPS origin and a local helper usable from task worktrees', async () => {
const root = mkdtempSync(join(realpathSync(tmpdir()), 'cez-credentials-'));
const bin = join(root, 'bin');
const repo = join(root, 'repo');
mkdirSync(bin);
// Substitute only gh: the runner and post-clone git configuration are real.
writeFileSync(join(bin, 'gh'), '#!/bin/sh\ngit init -q "$4" && git -C "$4" remote add origin "$3"\n', { mode: 0o755 });
vi.stubEnv('PATH', `${bin}:${process.env.PATH}`);
vi.stubEnv('GIT_CONFIG_GLOBAL', '/dev/null');
vi.stubEnv('GIT_CONFIG_NOSYSTEM', '1');
try {
expect(await ghCloneRunner(parseRepoRef('owner/repo')!, repo, () => {}, undefined)).toEqual({ ok: true });
const git = (...args: string[]) => execFileSync('git', ['-C', repo, ...args], { encoding: 'utf8' });
expect(git('remote', 'get-url', 'origin').trim()).toBe('https://github.com/owner/repo.git');
expect(git('config', '--local', '--get-all', 'credential.https://github.com.helper')).toBe('\n!gh auth git-credential\n');
git('-c', 'user.name=Test', '-c', 'user.email=test@example.com', 'commit', '--allow-empty', '-qm', 'initial');
const worktree = join(root, 'task');
git('worktree', 'add', '-qb', 'task', worktree);
expect(execFileSync('git', ['-C', worktree, 'config', '--get-all', 'credential.https://github.com.helper'], { encoding: 'utf8' })).toBe('\n!gh auth git-credential\n');
} finally {
vi.unstubAllEnvs();
rmSync(root, { recursive: true, force: true });
}
});
});

describe('checkout — the cleanup guard', () => {
Expand Down
54 changes: 47 additions & 7 deletions packages/cezar/src/server/checkout.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { spawn } from 'node:child_process';
import { execFile, spawn } from 'node:child_process';
import { lstat, mkdir, realpath, rm, writeFile } from 'node:fs/promises';
import { dirname, join, resolve } from 'node:path';

Expand Down Expand Up @@ -63,9 +63,16 @@ export type CheckoutResult = { ok: true; target: string; name: string } | Checko
export interface RepoRef {
owner: string;
repo: string;
/** What `gh repo clone` is handed — always the normalized `owner/repo`, so a
* URL spelling can never smuggle flags or a different host past `gh`. */
/** Normalized identity used in messages and dry-run output. */
slug: string;
/** What `gh repo clone` is handed. Always reconstructed from validated
* segments rather than preserving user input. Forcing HTTPS is load-bearing:
* a machine configured with `gh config set git_protocol ssh` may have an
* OAuth token authorized for an organization's SAML policy while its SSH key
* is not. Passing only `owner/repo` silently selects that rejected key.
* The resulting HTTPS `origin` needs a credential path of its own after the
* clone — see `persistGhCredentialHelper`. */
cloneUrl: string;
}

/** `owner` and `repo` as GitHub itself allows them: alphanumerics, `-`, `_`,
Expand Down Expand Up @@ -103,7 +110,12 @@ export function parseRepoRef(input: string): RepoRef | null {
if (parts.length !== 2) return null;
const [owner, repo] = parts;
if (!owner || !repo || !NAME_SEGMENT.test(owner) || !NAME_SEGMENT.test(repo)) return null;
return { owner, repo, slug: `${owner}/${repo}` };
return {
owner,
repo,
slug: `${owner}/${repo}`,
cloneUrl: `https://github.com/${owner}/${repo}.git`,
};
}

/**
Expand Down Expand Up @@ -170,8 +182,31 @@ export type CloneRunner = (
signal: AbortSignal | undefined,
) => Promise<{ ok: true } | { ok: false; error: string; notFound?: boolean }>;

/** Kept pure so the SAML-safe transport choice is pinned without spawning a
* real GitHub process in the unit suite. */
export function ghCloneArgs(ref: RepoRef, dir: string): string[] {
return ['repo', 'clone', ref.cloneUrl, dir, '--', '--progress'];
}

/** PR #968: gh injects credentials only for the clone command. Persist the
* helper locally so subsequent raw git pushes (including task worktrees) use
* the same OAuth grant. Reset inherited helpers first, as gh setup-git does. */
async function persistGhCredentialHelper(dir: string): Promise<boolean> {
for (const args of [
['--replace-all', 'credential.https://github.com.helper', ''],
['--add', 'credential.https://github.com.helper', '!gh auth git-credential'],
]) {
const ok = await new Promise<boolean>((resolvePromise) => {
execFile('git', ['-C', dir, 'config', '--local', ...args],
{ timeout: 10_000 }, (err) => resolvePromise(!err));
});
if (!ok) return false;
}
return true;
}

/**
* `gh repo clone <owner/repo> <dir> -- --progress`.
* `gh repo clone <validated HTTPS URL> <dir> -- --progress`.
*
* `spawn`, not `execFile`, because the whole point of this route is that the
* dialog sees progress while it happens: `git clone --progress` writes its
Expand All @@ -181,7 +216,7 @@ export type CloneRunner = (
*/
export const ghCloneRunner: CloneRunner = (ref, dir, onLine, signal) =>
new Promise((resolvePromise) => {
const child = spawn('gh', ['repo', 'clone', ref.slug, dir, '--', '--progress'], {
const child = spawn('gh', ghCloneArgs(ref, dir), {
stdio: ['ignore', 'pipe', 'pipe'],
timeout: CLONE_TIMEOUT_MS,
// No inherited stdin and `GH_PROMPT_DISABLED`: an unauthenticated `gh`
Expand Down Expand Up @@ -237,7 +272,12 @@ export const ghCloneRunner: CloneRunner = (ref, dir, onLine, signal) =>
});
child.on('close', (code) => {
signal?.removeEventListener('abort', onAbort);
if (code === 0) return finish({ ok: true });
if (code === 0) {
void persistGhCredentialHelper(dir).then((ok) => finish(ok
? { ok: true }
: { ok: false, error: 'Could not configure GitHub credentials for the checkout. Check directory permissions and retry.' }));
return;
}
// The tail of gh/git's own output IS the error message — `gh` writes
// "could not find repository", "authentication required" and the network
// errors itself, and paraphrasing them would only lose detail.
Expand Down
18 changes: 18 additions & 0 deletions packages/cezar/src/server/forge/draft-pr-autosave.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,24 @@ describe('createDraftPr pre-PR autosave (#471 follow-up)', () => {
rmSync(repo, { recursive: true, force: true });
});

it('disables terminal credential prompts for raw git pushes', async () => {
vi.stubEnv('CEZ_DRY_RUN', '0');
vi.stubEnv('GIT_TERMINAL_PROMPT', '1');
const remote = mkdtempSync(join(tmpdir(), 'cez-push-remote-'));
try {
await run('git', ['init', '--bare', '-q', remote]);
await git(['remote', 'add', 'origin', remote]);
await git(['branch', 'cez/abc123']);
writeFileSync(join(repo, '.git', 'hooks', 'pre-push'),
'#!/bin/sh\necho "prompt-setting=$GIT_TERMINAL_PROMPT" >&2\nexit 1\n', { mode: 0o755 });
const outcome = await createDraftPr(input());
expect(outcome.ok).toBe(false);
expect(outcome.ok === false && outcome.error).toContain('prompt-setting=0');
} finally {
rmSync(remote, { recursive: true, force: true });
}
});

it('refuses to publish a worktree holding conflict markers', async () => {
writeFileSync(
join(repo, 'a.txt'),
Expand Down
12 changes: 11 additions & 1 deletion packages/cezar/src/server/forge/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2547,7 +2547,17 @@ function execTool(args: string[], cwd: string, bin: string, timeoutMs = 30_000):
execFile(
bin,
args,
{ cwd, timeout: timeoutMs, maxBuffer: 4 * 1024 * 1024, encoding: 'utf8' },
{
cwd,
timeout: timeoutMs,
maxBuffer: 4 * 1024 * 1024,
encoding: 'utf8',
// Nobody can answer a prompt here: git opens /dev/tty directly, so
// piped stdio is not enough to stop it. Without this a `git push` that
// cannot authenticate hangs for the whole timeout and surfaces as a
// blank one-minute stall instead of git's own "could not read Username".
env: { ...process.env, GIT_TERMINAL_PROMPT: '0' },
},
(err, stdout, stderr) =>
resolve({
ok: !err,
Expand Down
82 changes: 81 additions & 1 deletion packages/web/src/components/clone-project-dialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

import { createQueryClient } from '@/api/query-client'
import type { ProjectListEntry } from '@open-mercato/cezar-api-client'
import { CloneProjectDialog } from '@/components/clone-project-dialog'
import { CloneProjectDialog, githubSsoUrl } from '@/components/clone-project-dialog'

/**
* The clone-from-GitHub dialog (multi-project spec, "Add project" option B / step 4.3).
Expand Down Expand Up @@ -35,6 +35,7 @@ beforeEach(() => {
})

afterEach(() => {
vi.restoreAllMocks()
cleanup()
fetchMock.mockReset()
emitWorkspaceEvent = null
Expand Down Expand Up @@ -190,6 +191,71 @@ describe('CloneProjectDialog', () => {
expect(cloneButton().disabled).toBe(false)
})

it.each(['focus', 'visibility', 'manual'])('keeps SAML details and retries once via %s', async (mode) => {
const ssoUrl =
'https://github.com/orgs/Bug-Bounty-Switzerland/sso?authorization_request=AHBV4IUOPSOOJDNNTYUU4ALKUCFKNA5P'
let attempts = 0
serve(async () => {
attempts += 1
return attempts === 1
? json(
{
error:
`GraphQL: Resource protected by organization SAML enforcement. ` +
`You must grant your OAuth token access to this organization. (repository) ` +
`Authorize in your web browser: ${ssoUrl}`,
},
500,
)
: json({ project: PROJECT })
})
const { onOpenChange } = renderDialog()
fireEvent.change(urlInput(), { target: { value: 'Bug-Bounty-Switzerland/private-repo' } })
fireEvent.click(cloneButton())

await waitFor(() => expect(slot('clone-sso-link')).toBeTruthy())
const link = slot('clone-sso-link') as HTMLAnchorElement
expect(link.textContent).toBe('Authorize this GitHub organization')
expect(link.href).toBe(ssoUrl)
expect(link.target).toBe('_blank')
expect(link.rel).toBe('noreferrer')
expect(slot('clone-error')?.textContent).toContain('return to this tab to retry')
expect(slot('clone-error')?.querySelector('details')?.textContent).toContain(ssoUrl)
expect(slot('clone-error')?.querySelector('details')?.open).toBe(false)
expect(cloneButton().textContent).toBe('Retry clone')

fireEvent.click(link, { ctrlKey: mode === 'manual' })
expect(slot('clone-error')?.textContent).toContain('or choose Retry clone')
if (mode === 'visibility') {
const visibility = vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('hidden')
fireEvent(document, new Event('visibilitychange'))
expect(posted).toHaveLength(1)
visibility.mockReturnValue('visible')
fireEvent(document, new Event('visibilitychange'))
fireEvent.focus(window)
} else if (mode === 'manual') {
expect(posted).toHaveLength(1)
fireEvent.click(cloneButton())
} else {
fireEvent.focus(window)
}

await waitFor(() => expect(posted).toHaveLength(2))
await waitFor(() => expect(onOpenChange).toHaveBeenCalledWith(false))
})

it('does not turn an arbitrary URL in clone output into a link', async () => {
serve(async () => json({ error: 'remote: visit https://evil.example/authorize' }, 500))
renderDialog()
fireEvent.change(urlInput(), { target: { value: 'open-mercato/cezar' } })
fireEvent.click(cloneButton())

await waitFor(() => expect(slot('clone-error')?.textContent).toContain('evil.example'))
expect(slot('clone-sso-link')).toBeNull()
expect(slot('clone-error')?.querySelector('p')?.className).toContain('break-all')
expect(cloneButton().textContent).toBe('Retry clone')
})

it('surfaces the existing-folder 409 as an error rather than navigating anywhere', async () => {
serve(async () => json({ error: 'folder already exists: /home/me/cezar/projects/cezar' }, 409))
renderDialog()
Expand All @@ -209,3 +275,17 @@ describe('CloneProjectDialog', () => {
await waitFor(() => expect(cloneButton().disabled).toBe(false))
})
})


describe('githubSsoUrl', () => {
const url = 'https://github.com/orgs/Acme/sso?authorization_request=ABC'
it.each(['==', '%2FDEF', '&extra=1', ')'])('rejects a partial token before %s', (suffix) => {
expect(githubSsoUrl(new Error(url + suffix))).toBeNull()
})
it.each(['', '\nnext line', '"', "'"])('accepts a complete URL before %s', (suffix) => {
expect(githubSsoUrl(new Error(url + suffix))).toBe(url)
})
it.each(['https://github.com.evil.example', 'http://github.com', 'https://evil.example@github.com'])('rejects %s', (host) => {
expect(githubSsoUrl(new Error(url.replace('https://github.com', host)))).toBeNull()
})
})
Loading