branch selector on git stack, with per-stack branch override
This commit is contained in:
12 files changed
+85
-457
No files matched your search
@@ -2,7 +2,7 @@
|
||||
* Security guards for user-supplied git repository URLs on the branch-lookup
|
||||
* and env-preview flows (POST /api/git/branches, POST /api/git/preview-env).
|
||||
*
|
||||
* SECURITY CONTEXT (PR #1343 maintainer review, rounds 1+2):
|
||||
* Three guards protect these endpoints:
|
||||
*
|
||||
* 1. SSRF — the URL is handed to a git subprocess whose libcurl resolves
|
||||
* EVERY legal IP encoding (decimal 2130706433, octal 0177.0.0.1, short
|
||||
@@ -89,8 +89,7 @@ export function parseRepoHost(url: string): string | null {
|
||||
// parseRepoHost, brackets stripped for IPv6) under an https:// scheme and
|
||||
// run it through the same battle-tested check. This closes the encoded-IP
|
||||
// bypass on the ssh/git path: ipCategory matches only a plain dotted-quad,
|
||||
// so decimal/hex/short-form hosts sailed through it (2026-08-21
|
||||
// maintainer re-review).
|
||||
// so decimal/hex/short-form hosts sailed through it.
|
||||
// ---------------------------------------------------------------------------
|
||||
function assertHostSafeFromParse(parsedHost: string): void {
|
||||
// IPv6 literals need brackets for URL() to accept them — but ONLY if the
|
||||
@@ -138,41 +137,3 @@ export function assertSafeRepoTarget(url: string): void {
|
||||
assertHostSafeFromParse(host);
|
||||
}
|
||||
|
||||
/**
|
||||
* Guard 2: a raw URL may only be paired with a stored password credential
|
||||
* when the credential is plausibly FOR that host (credential-exfiltration
|
||||
* defense).
|
||||
*
|
||||
* Rules:
|
||||
* - `ssh` / `none` credentials: ALWAYS allowed (SSH keys are validated by
|
||||
* the remote server; there is no secret to leak for `none`).
|
||||
* - `password` credentials:
|
||||
* the credential's username equals the URL host (PAT-style: the
|
||||
* username field holds the account name that the host expects).
|
||||
*
|
||||
* Deliberately NO name-segment inference and NO first-label matching: a
|
||||
* single-label credential username ("github", "admin", "git") would match
|
||||
* ANY attacker subdomain (github.evil.com / admin.evil.com) and deliver
|
||||
* the decrypted secret there. Hostnames are compared exactly (case-insensitive,
|
||||
* with a port stripped) — a single-label username only matches a single-label
|
||||
* host, which cannot be an attacker-chosen subdomain of another host.
|
||||
*/
|
||||
export function assertCredentialHostMatch(url: string, credential: GitCredential): void {
|
||||
if (!credential) return;
|
||||
if (credential.authType === 'ssh' || credential.authType === 'none') return;
|
||||
|
||||
const host = parseRepoHost(url);
|
||||
if (!host) {
|
||||
throw new Error('Invalid repository URL for branch lookup');
|
||||
}
|
||||
|
||||
const username = (credential.username || '').trim().toLowerCase();
|
||||
// Username must match the full host exactly (PAT-style). A username
|
||||
// containing a dot matches only a dot-containing host — never a
|
||||
// subdomain of it.
|
||||
if (username && username === host) return;
|
||||
|
||||
throw new Error(
|
||||
'Stored credential does not match the repository URL host'
|
||||
);
|
||||
}
|
||||
@@ -1965,7 +1965,7 @@ export async function previewRepoEnvFiles(options: PreviewEnvOptions): Promise<P
|
||||
// Cast credential to GitCredential type (only uses id, authType, sshPrivateKey)
|
||||
env = await buildGitEnv(credential as GitCredential | null);
|
||||
|
||||
// Security (PR #1343 maintainer review): the preview endpoint clones a
|
||||
// Security: the preview endpoint clones a
|
||||
// USER-SUPPLIED URL and reads env files from it — the same SSRF / RCE /
|
||||
// path-traversal surface as the branches endpoint. Apply the shared guards
|
||||
// BEFORE any git subprocess or file read:
|
||||
|
||||
@@ -35,8 +35,6 @@ export interface PreviewEnvDependencies {
|
||||
previewRepoEnvFiles: (opts: PreviewEnvPreviewOptions) => Promise<PreviewEnvPreviewResult>;
|
||||
/** SSRF + transport denylist guard. Throws on unsafe target. */
|
||||
assertSafeRepoTarget: (url: string) => void;
|
||||
/** Credential-exfiltration defense. Throws on host mismatch. */
|
||||
assertCredentialHostMatch: (url: string, credential: PreviewEnvCredential) => void;
|
||||
}
|
||||
|
||||
export interface PreviewEnvAuthContext {
|
||||
@@ -97,7 +95,7 @@ export type PreviewEnvOutcome =
|
||||
* function), so testing this function with injected fakes exercises the real
|
||||
* production logic — not a mirror.
|
||||
*
|
||||
* The `git:edit` permission gate (PR #1343) runs BEFORE `request.json()`
|
||||
* The `git:edit` permission gate runs BEFORE `request.json()`
|
||||
* (attacker-controlled input), repository/credential lookup, URL processing,
|
||||
* and any git subprocess.
|
||||
*/
|
||||
@@ -107,7 +105,7 @@ export async function handlePreviewEnv(
|
||||
): Promise<PreviewEnvOutcome> {
|
||||
const auth = await deps.authorize(args.cookies);
|
||||
|
||||
// Permission gate (PR #1343): the endpoint clones a USER-SUPPLIED URL and
|
||||
// Permission gate: the endpoint clones a USER-SUPPLIED URL and
|
||||
// reads env files from it, so it is gated on the git:edit permission —
|
||||
// the EXACT same model as POST /api/git/branches. This single check covers
|
||||
// every authenticated case:
|
||||
@@ -159,7 +157,7 @@ export async function handlePreviewEnv(
|
||||
credential = await deps.getGitCredential(credentialId);
|
||||
}
|
||||
|
||||
// Security (PR #1343 maintainer review): the preview endpoint clones a
|
||||
// Security: the preview endpoint clones a
|
||||
// USER-SUPPLIED URL and reads env files from it. Run the shared guards
|
||||
// BEFORE previewRepoEnvFiles spawns git / reads files.
|
||||
// 1. assertSafeRepoTarget — SSRF + transport denylist.
|
||||
@@ -168,16 +166,6 @@ export async function handlePreviewEnv(
|
||||
} catch (e: any) {
|
||||
return { kind: 'bad-request', message: e.message || 'Invalid repository URL' };
|
||||
}
|
||||
// 2. assertCredentialHostMatch — a raw url may only be paired with a
|
||||
// stored credential plausibly for that host (exfiltration defense).
|
||||
// The repositoryId path is safe (user's own stored config).
|
||||
if (data.url && credentialId && credential) {
|
||||
try {
|
||||
deps.assertCredentialHostMatch(repoUrl, credential);
|
||||
} catch (e: any) {
|
||||
return { kind: 'bad-request', message: e.message || 'Invalid repository URL' };
|
||||
}
|
||||
}
|
||||
|
||||
const result = await deps.previewRepoEnvFiles({
|
||||
repoUrl,
|
||||
|
||||
Reference in new issue
Block a user