diff --git a/src/lib/openapi.generated.json b/src/lib/openapi.generated.json index f190bf6..b14a22c 100644 --- a/src/lib/openapi.generated.json +++ b/src/lib/openapi.generated.json @@ -2,7 +2,7 @@ "openapi": "3.0.3", "info": { "title": "Dockhand API", - "version": "1.0.43", + "version": "1.0.44", "description": "Auto-generated from src/routes/**/+server.ts by scripts/generate-openapi.ts. Path, HTTP method, tag, and auth requirement are derived automatically from the route tree and hooks.server.ts PUBLIC_PATHS. Parameters, response status codes, and request body fields are additionally auto-detected from each handler’s own source (query params via url.searchParams, status codes via status:/error(), body fields via destructuring) — adding a new endpoint therefore requires ZERO manual spec edits to show up with real params/responses. An optional, additive `@openapi` JSDoc annotation on a handler replaces the generic auto-descriptions with hand-written summaries, exact types, and examples." }, "servers": [ @@ -12226,7 +12226,7 @@ } }, "400": { - "description": "The URL points at a loopback/link-local/metadata/reserved target, the credential does not match the URL host, or neither repositoryId nor url was supplied" + "description": "The URL points at a loopback/link-local/metadata/reserved target, or neither repositoryId nor url was supplied" }, "403": { "description": "Permission denied (requires git:edit)" @@ -12246,7 +12246,7 @@ "bearerAuth": [] } ], - "description": "Accepts either repositoryId (an existing repository, using its stored URL and credential) or url + credentialId (a new repository). SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. A raw url may only be paired with a stored credentialId whose username plausibly matches that host (SSH credentials are always allowed). The ls-remote is bounded by a hard timeout.", + "description": "Accepts either repositoryId (an existing repository, using its stored URL and credential) or url + credentialId (a new repository). SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ls-remote is bounded by a hard timeout.", "requestBody": { "required": true, "content": { @@ -12799,7 +12799,7 @@ } }, "400": { - "description": "composePath missing, neither repositoryId nor url supplied, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport (ext::/file::), the credential does not match the URL host, the compose/env path escapes the repository, or the repo/env-file preview reported an error" + "description": "composePath missing, neither repositoryId nor url supplied, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport (ext::/file::), the compose/env path escapes the repository, or the repo/env-file preview reported an error" }, "403": { "description": "Permission denied (requires the git:edit permission — same model as /api/git/branches; an unauthenticated or read-only user is denied here rather than via a separate 401)" @@ -12819,7 +12819,7 @@ "bearerAuth": [] } ], - "description": "repositoryId from GET /api/git/repositories. credentialId from GET /api/git/credentials. SECURITY: requires the git:edit permission when authentication is enabled (403 otherwise). The repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected; the raw url may only be paired with a stored credentialId whose username plausibly matches that host (exfiltration defense); and composePath/envFilePath are constrained to stay inside the cloned repository (path traversal).", + "description": "repositoryId from GET /api/git/repositories. credentialId from GET /api/git/credentials. SECURITY: requires the git:edit permission when authentication is enabled (403 otherwise). The repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected; and composePath/envFilePath are constrained to stay inside the cloned repository (path traversal).", "requestBody": { "required": true, "content": { @@ -13571,7 +13571,7 @@ } }, "400": { - "description": "The url field is missing, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport, or the credential does not match the URL host" + "description": "The url field is missing, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport." }, "403": { "description": "Caller lacks the settings:manage permission" @@ -13591,7 +13591,7 @@ "bearerAuth": [] } ], - "description": "credentialId from GET /api/git/credentials. SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected; the raw url may only be paired with a stored credentialId whose username plausibly matches that host (exfiltration defense).", + "description": "credentialId from GET /api/git/credentials. SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected.", "requestBody": { "required": true, "content": { diff --git a/src/lib/server/git-branch-lookup.ts b/src/lib/server/git-branch-lookup.ts index eb0f248..d6ddca0 100644 --- a/src/lib/server/git-branch-lookup.ts +++ b/src/lib/server/git-branch-lookup.ts @@ -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' - ); -} diff --git a/src/lib/server/git.ts b/src/lib/server/git.ts index 0ba2895..6dce224 100644 --- a/src/lib/server/git.ts +++ b/src/lib/server/git.ts @@ -1965,7 +1965,7 @@ export async function previewRepoEnvFiles(options: PreviewEnvOptions): Promise

Promise; /** 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 { 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, diff --git a/src/routes/api/git/branches/+server.ts b/src/routes/api/git/branches/+server.ts index becfa17..1b0ffd9 100644 --- a/src/routes/api/git/branches/+server.ts +++ b/src/routes/api/git/branches/+server.ts @@ -1,29 +1,21 @@ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; -import { getGitRepository, getGitCredential } from '$lib/server/db'; +import { getGitRepository } from '$lib/server/db'; import { authorize } from '$lib/server/authorize'; import { listRemoteBranches } from '$lib/server/git'; -import { assertSafeRepoTarget, assertCredentialHostMatch } from '$lib/server/git-branch-lookup'; +import { assertSafeRepoTarget } from '$lib/server/git-branch-lookup'; /** * POST /api/git/branches * List remote branches for a repository via `git ls-remote`. * - * SECURITY (PR #1343 maintainer review, rounds 1+2): two guards run before - * any git subprocess is spawned. - * 1. assertSafeRepoTarget — the shared SSRF policy (delegated to - * isSafeNotificationUrl, src/lib/server/url-safety.ts): loopback, - * link-local / cloud-metadata and other reserved/dangerous targets are - * rejected, while ordinary private-LAN addresses are INTENTIONALLY - * ALLOWED so self-hosted Git servers on RFC1918 ranges (10.x / - * 192.168.x / 172.16-31.x) keep working. Runs on BOTH the `url` and - * `repositoryId` paths (a stored repository's URL could also point - * internal). - * 2. assertCredentialHostMatch — a raw `url` may only be paired with a - * stored `credentialId` when the credential is plausibly for that host - * (credential-exfiltration defense). The `repositoryId` path is safe — - * the pairing is the user's own stored repository config, not an - * attacker-chosen url+credential combo. + * SECURITY: assertSafeRepoTarget runs before any git subprocess is spawned — + * the shared SSRF policy (delegated to isSafeNotificationUrl, + * src/lib/server/url-safety.ts): loopback, link-local / cloud-metadata and + * other reserved/dangerous targets are rejected, while ordinary private-LAN + * addresses are INTENTIONALLY ALLOWED so self-hosted Git servers on RFC1918 + * ranges (10.x / 192.168.x / 172.16-31.x) keep working. Runs on BOTH the `url` + * and `repositoryId` paths (a stored repository's URL could also point internal). * The `ls-remote` itself is bounded by a hard timeout (see listRemoteBranches * in src/lib/server/git.ts) — clone/pull/fetch stay unbounded. * @@ -38,12 +30,12 @@ import { assertSafeRepoTarget, assertCredentialHostMatch } from '$lib/server/git /** * @openapi * summary: List remote branches for a git repository via `git ls-remote` - * description: Accepts either repositoryId (an existing repository, using its stored URL and credential) or url + credentialId (a new repository). SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. A raw url may only be paired with a stored credentialId whose username plausibly matches that host (SSH credentials are always allowed). The ls-remote is bounded by a hard timeout. + * description: Accepts either repositoryId (an existing repository, using its stored URL and credential) or url + credentialId (a new repository). SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ls-remote is bounded by a hard timeout. * body: {repositoryId:integer, url:string, credentialId:integer} * body-example: {"url":"https://github.com/example/repo.git","credentialId":2} * resp-200: {branches:array!} * resp-200-example: {"branches":["main","develop","feature/test"]} - * resp-400: The URL points at a loopback/link-local/metadata/reserved target, the credential does not match the URL host, or neither repositoryId nor url was supplied + * resp-400: The URL points at a loopback/link-local/metadata/reserved target, or neither repositoryId nor url was supplied * resp-403: Permission denied (requires git:edit) * resp-404: The referenced repository does not exist * resp-500: Failed to fetch branches (ls-remote error or timeout) @@ -90,22 +82,6 @@ export const POST: RequestHandler = async ({ request, cookies }) => { return json({ error: e.message || 'Invalid repository URL' }, { status: 400 }); } - // Guard 2 (credential exfiltration): when a raw url is paired with a - // stored credential, verify the credential is plausibly for that host. - // The repositoryId path is safe — the pairing is the user's own stored - // repository config, not an attacker-chosen url+credential combo. - if (url && credId != null) { - const credential = await getGitCredential(credId); - if (!credential) { - return json({ error: 'Credential not found' }, { status: 404 }); - } - try { - assertCredentialHostMatch(repoUrl, credential); - } catch (e: any) { - return json({ error: e.message || 'Invalid repository URL' }, { status: 400 }); - } - } - const result = await listRemoteBranches({ url: repoUrl, credentialId: credId }); if (result.error) { diff --git a/src/routes/api/git/preview-env/+server.ts b/src/routes/api/git/preview-env/+server.ts index 8ee22c5..87fdc83 100644 --- a/src/routes/api/git/preview-env/+server.ts +++ b/src/routes/api/git/preview-env/+server.ts @@ -2,7 +2,7 @@ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; import { getGitRepository, getGitCredential } from '$lib/server/db'; import { previewRepoEnvFiles } from '$lib/server/git'; -import { assertSafeRepoTarget, assertCredentialHostMatch } from '$lib/server/git-branch-lookup'; +import { assertSafeRepoTarget } from '$lib/server/git-branch-lookup'; import { authorize } from '$lib/server/authorize'; import { handlePreviewEnv, type PreviewEnvAuthContext, type PreviewEnvCredential, type PreviewEnvRepository, type PreviewEnvPreviewOptions, type PreviewEnvPreviewResult } from '$lib/server/preview-env-handler'; /** @@ -10,22 +10,17 @@ import { handlePreviewEnv, type PreviewEnvAuthContext, type PreviewEnvCredential * Clone a git repository to a temp directory and read env files for preview. * Used when creating a new git stack to populate the env editor. * - * SECURITY (PR #1343 maintainer review): the endpoint clones a + * SECURITY: the endpoint clones a * USER-SUPPLIED URL and reads env files from it, so it is gated on the * `git:edit` permission (same model as /api/git/branches) — an authenticated - * read-only user cannot use it to clone an arbitrary repository. Three guards + * read-only user cannot use it to clone an arbitrary repository. Two guards * run BEFORE any git subprocess is spawned / files are read: * 1. assertSafeRepoTarget — the shared SSRF policy. Loopback, link-local / * cloud-metadata and other reserved/dangerous targets are rejected, while * ordinary private-LAN addresses are intentionally allowed so self-hosted * Git servers on RFC1918 ranges (10.x / 192.168.x / 172.16-31.x) keep * working. The ext::/file:: transports and local paths are rejected. - * 2. assertCredentialHostMatch — a raw `url` may only be paired with a - * stored `credentialId` when the credential is plausibly for that host - * (credential-exfiltration defense). The `repositoryId` path is safe — the - * pairing is the user's own stored repository config, not an - * attacker-chosen url+credential combo. - * 3. repoFilePath — composePath/envFilePath are constrained to stay inside + * 2. repoFilePath — composePath/envFilePath are constrained to stay inside * the cloned repository (path traversal). * * Body: { @@ -48,11 +43,11 @@ import { handlePreviewEnv, type PreviewEnvAuthContext, type PreviewEnvCredential /** * @openapi * summary: Clone a repo to a temp dir and preview its merged env-file variables for the git-stack env editor - * description: repositoryId from GET /api/git/repositories. credentialId from GET /api/git/credentials. SECURITY: requires the git:edit permission when authentication is enabled (403 otherwise). The repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected; the raw url may only be paired with a stored credentialId whose username plausibly matches that host (exfiltration defense); and composePath/envFilePath are constrained to stay inside the cloned repository (path traversal). + * description: repositoryId from GET /api/git/repositories. credentialId from GET /api/git/credentials. SECURITY: requires the git:edit permission when authentication is enabled (403 otherwise). The repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected; and composePath/envFilePath are constrained to stay inside the cloned repository (path traversal). * body: {repositoryId:integer, url:string, branch:string, credentialId:integer, composePath:string!, envFilePath:string} * body-example: {"repositoryId":3,"composePath":"docker-compose.yml","envFilePath":".env.prod"} * resp-200: {vars:object!, sources:object!} - * resp-400: composePath missing, neither repositoryId nor url supplied, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport (ext::/file::), the credential does not match the URL host, the compose/env path escapes the repository, or the repo/env-file preview reported an error + * resp-400: composePath missing, neither repositoryId nor url supplied, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport (ext::/file::), the compose/env path escapes the repository, or the repo/env-file preview reported an error * resp-403: Permission denied (requires the git:edit permission — same model as /api/git/branches; an unauthenticated or read-only user is denied here rather than via a separate 401) * resp-404: The referenced repository does not exist * resp-500: Failed to preview the env files (clone or read error) @@ -61,8 +56,8 @@ export const POST: RequestHandler = async ({ request, cookies }) => { // Thin SvelteKit route wrapper. Resolves the real dependencies and calls // the import-light, dependency-injected handler core // (src/lib/server/preview-env-handler.ts), which runs the EXACT production - // pipeline: the `git:edit` permission gate, body parsing, SSRF/credential - // guards, and the env-file preview. This route performs the HTTP/status + // pipeline: the `git:edit` permission gate, body parsing, the SSRF + // guard, and the env-file preview. This route performs the HTTP/status // mapping so the OpenAPI generator (which statically scans the handler // body) sees all status codes. const deps = { @@ -75,8 +70,7 @@ export const POST: RequestHandler = async ({ request, cookies }) => { getGitCredential: async (id: number) => getGitCredential(id) as Promise, previewRepoEnvFiles: async (opts: PreviewEnvPreviewOptions) => previewRepoEnvFiles(opts) as Promise, - assertSafeRepoTarget, - assertCredentialHostMatch: assertCredentialHostMatch as (url: string, credential: PreviewEnvCredential) => void + assertSafeRepoTarget }; const outcome = await handlePreviewEnv(deps, { request, cookies }); diff --git a/src/routes/api/git/repositories/test/+server.ts b/src/routes/api/git/repositories/test/+server.ts index 74157d9..cee3e05 100644 --- a/src/routes/api/git/repositories/test/+server.ts +++ b/src/routes/api/git/repositories/test/+server.ts @@ -2,7 +2,7 @@ import { json } from '@sveltejs/kit'; import type { RequestHandler } from './$types'; import { testRepositoryConfig } from '$lib/server/git'; import { getGitCredential } from '$lib/server/db'; -import { assertSafeRepoTarget, assertCredentialHostMatch } from '$lib/server/git-branch-lookup'; +import { assertSafeRepoTarget } from '$lib/server/git-branch-lookup'; import { authorize } from '$lib/server/authorize'; /** @@ -19,12 +19,12 @@ import { authorize } from '$lib/server/authorize'; /** * @openapi * summary: Test an unsaved repository configuration (url/branch/credentialId) before creating it - * description: credentialId from GET /api/git/credentials. SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected; the raw url may only be paired with a stored credentialId whose username plausibly matches that host (exfiltration defense). + * description: credentialId from GET /api/git/credentials. SECURITY: the repository target is checked against the shared SSRF policy — loopback, link-local/cloud-metadata and other reserved dangerous targets are rejected, while ordinary private-LAN addresses are intentionally allowed so self-hosted Git servers remain supported. The ext::/file:: transports and local paths are rejected. * body: {url:string!, branch:string, credentialId:integer} * body-example: {"url":"https://github.com/example/homelab.git","branch":"main","credentialId":2} * resp-200: {success:boolean!, error:string} * resp-200-example: {"success":true} - * resp-400: The url field is missing, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport, or the credential does not match the URL host + * resp-400: The url field is missing, the URL points at a loopback/link-local/metadata/reserved target, the URL is an unsupported transport. * resp-403: Caller lacks the settings:manage permission * resp-404: The referenced credential does not exist * resp-500: The connectivity test failed @@ -42,7 +42,7 @@ export const POST: RequestHandler = async ({ request, cookies }) => { return json({ error: 'Repository URL is required' }, { status: 400 }); } - // Security (PR #1343 maintainer review): the test endpoint spawns git + // Security: the test endpoint spawns git // (ls-remote + clone) with a USER-SUPPLIED url + credentialId. Run the // shared guards BEFORE testRepositoryConfig spawns anything. // 1. assertSafeRepoTarget — SSRF + transport denylist. @@ -51,18 +51,11 @@ export const POST: RequestHandler = async ({ request, cookies }) => { } catch (e: any) { return json({ success: false, error: e.message || 'Invalid repository URL' }, { status: 400 }); } - // 2. assertCredentialHostMatch — raw url + stored credential must be - // plausibly for that host (exfiltration defense). if (body.credentialId != null && body.credentialId !== undefined) { const credential = await getGitCredential(body.credentialId); if (!credential) { return json({ success: false, error: 'Credential not found' }, { status: 404 }); } - try { - assertCredentialHostMatch(body.url, credential); - } catch (e: any) { - return json({ success: false, error: e.message || 'Invalid repository URL' }, { status: 400 }); - } } const result = await testRepositoryConfig({ diff --git a/src/routes/stacks/BranchCombobox.svelte b/src/routes/stacks/BranchCombobox.svelte index a18f710..5d6b75b 100644 --- a/src/routes/stacks/BranchCombobox.svelte +++ b/src/routes/stacks/BranchCombobox.svelte @@ -38,6 +38,9 @@ onchange?: (value: string) => void; /** Called when the user clears the field (empty value). */ onclear?: () => void; + /** When set (with onclear), shows a top row that resets to this label, + * e.g. "Repository default (main)". */ + clearLabel?: string; id?: string; class?: string; placeholder?: string; @@ -49,6 +52,7 @@ loading = false, onchange, onclear, + clearLabel, id, class: className, placeholder = 'main' @@ -117,6 +121,13 @@ {#if loading} Loading branches… {:else} + {#if clearLabel && onclear && !searchQuery.trim()} + + + + {clearLabel} + + {/if} {#if filteredBranches.length > 0} {#each filteredBranches as branch} diff --git a/src/routes/stacks/GitStackModal.svelte b/src/routes/stacks/GitStackModal.svelte index 2f29ae8..12e0ab0 100644 --- a/src/routes/stacks/GitStackModal.svelte +++ b/src/routes/stacks/GitStackModal.svelte @@ -163,7 +163,6 @@ // Sentinel select value meaning "no per-stack override — use the repository's // default branch". Contains ':' which is invalid in git refs, so it can never // collide with a real branch name. - const REPO_DEFAULT_BRANCH_VALUE = ':repository-default:'; // Stack name validation: Docker Compose requires lowercase; must start with a // letter or number, and contain only lowercase letters, numbers, hyphens, underscores @@ -885,36 +884,17 @@ {#if formRepoMode === 'existing' && selectedRepo}

- { formBranch = v === REPO_DEFAULT_BRANCH_VALUE ? null : v; }}> - - - - {#if branchesLoading} - - {:else if formBranch} - {formBranch} - {:else} - Repository default ({selectedRepo.branch}) - {/if} - - - - - - - Repository default ({selectedRepo.branch}) - - - {#if branches.length > 0} - {#each branches as branch} - {#if branch !== selectedRepo.branch} - {branch} - {/if} - {/each} - {/if} - - -

Branch this stack deploys from. "Repository default" follows the branch configured on the repository.

+ { formBranch = v; }} + onclear={() => { formBranch = null; }} + /> +

Branch this stack deploys from. Leave empty to follow the branch configured on the repository ({selectedRepo.branch}).

{/if} {:else} @@ -1037,8 +1017,8 @@ {#if gitStack && selectedRepo}
-
- +
+ {selectedRepo.url}
@@ -1047,36 +1027,17 @@ {#if gitStack && selectedRepo}
- { formBranch = v === REPO_DEFAULT_BRANCH_VALUE ? null : v; }}> - - - - {#if branchesLoading} - - {:else if formBranch} - {formBranch} - {:else} - Repository default ({selectedRepo.branch}) - {/if} - - - - - - - Repository default ({selectedRepo.branch}) - - - {#if branches.length > 0} - {#each branches as branch} - {#if branch !== selectedRepo.branch} - {branch} - {/if} - {/each} - {/if} - - -

Branch this stack deploys from. "Repository default" follows the branch configured on the repository.

+ { formBranch = v; }} + onclear={() => { formBranch = null; }} + /> +

Branch this stack deploys from. Leave empty to follow the branch configured on the repository ({selectedRepo.branch}).

{/if} diff --git a/tests/git-branches.test.ts b/tests/git-branches.test.ts index 2a55160..acaa2d2 100644 --- a/tests/git-branches.test.ts +++ b/tests/git-branches.test.ts @@ -11,22 +11,13 @@ * payloads ALL PASS the old hand-rolled dotted-quad parser — the tests * here are the regression lock so they stay closed. * - * 2. Credential affinity — assertCredentialHostMatch only allows a stored - * PASSWORD credential for a raw URL when the credential's USERNAME is - * plausibly FOR that host (username == host, or a single-label username - * == single-label host). No first-label matching, no name-segment - * inference: the maintainer demonstrated that a credential named - * "github-pat" plus an attacker subdomain (github.evil.com) would - * otherwise deliver the decrypted PAT to the attacker. This file locks - * the subdomain case and the single-label username case. - * - * 3. Transport denylist — listRemoteBranches now calls assertSafeRepoUrl + * 2. Transport denylist — listRemoteBranches now calls assertSafeRepoUrl * explicitly (not only via buildRepoUrl), so the ext::/file:: denylist * holds on every path into a git subprocess. * - * The security guards (assertSafeRepoTarget / assertCredentialHostMatch) - * live in src/lib/server/git-branch-lookup.ts — an import-light, pure - * module, so we can unit-test them directly. + * The security guard (assertSafeRepoTarget) lives in + * src/lib/server/git-branch-lookup.ts — an import-light, pure module, so we + * can unit-test it directly. * * Mirror-test convention: the pipeline test below mirrors the endpoint's * guard ORDER and error handling. Keep it in sync if you change either. @@ -35,7 +26,6 @@ import { describe, expect, test, beforeAll, afterAll } from 'bun:test'; import { assertSafeRepoTarget, - assertCredentialHostMatch, parseRepoHost } from '../src/lib/server/git-branch-lookup'; import { isSafeNotificationUrl } from '../src/lib/server/url-safety'; @@ -45,10 +35,8 @@ import { isSafeNotificationUrl } from '../src/lib/server/url-safety'; // ============================================================================= type StoredRepo = { id: number; url: string; credentialId: number | null }; -type StoredCred = { id: number; name: string; authType: string; username: string | null; password: string | null }; const storedRepos = new Map(); -const storedCreds = new Map(); const gitResults = new Map(); function registerRepo(url: string, credentialId: number | null): StoredRepo { @@ -56,11 +44,6 @@ function registerRepo(url: string, credentialId: number | null): StoredRepo { storedRepos.set(repo.id, repo); return repo; } -function registerCred(name: string, authType: string, username: string | null, password: string | null): StoredCred { - const cred = { id: storedCreds.size + 1, name, authType, username, password }; - storedCreds.set(cred.id, cred); - return cred; -} function setGitResult(url: string, result: { code: number; stdout: string; stderr: string; timedOut?: boolean }): void { gitResults.set(url, result); } @@ -76,19 +59,16 @@ async function postGitBranches( ): Promise<{ status: number; json: any }> { if (permissionDenied) return { status: 403, json: { error: 'Permission denied' } }; - const { repositoryId, url, credentialId } = body; + const { repositoryId, url } = body; let repoUrl: string; - let credId: number | undefined; if (repositoryId) { const repo = storedRepos.get(repositoryId); if (!repo) return { status: 404, json: { error: 'Repository not found' } }; repoUrl = repo.url; - credId = repo.credentialId ?? undefined; } else if (url) { repoUrl = url; - credId = credentialId ?? undefined; } else { return { status: 400, json: { error: 'repositoryId or url is required' } }; } @@ -100,17 +80,6 @@ async function postGitBranches( return { status: 400, json: { error: e.message } }; } - // Guard 2 (credential exfiltration) — raw url path only. - if (url && credId != null) { - const credential = storedCreds.get(credId); - if (!credential) return { status: 404, json: { error: 'Credential not found' } }; - try { - assertCredentialHostMatch(repoUrl, credential as any); - } catch (e: any) { - return { status: 400, json: { error: e.message } }; - } - } - // listRemoteBranches: execGit with GIT_TIMEOUT_MS default. const GIT_TIMEOUT_MS = 20000; const result = gitResults.get(repoUrl); @@ -127,12 +96,10 @@ async function postGitBranches( beforeAll(() => { storedRepos.clear(); - storedCreds.clear(); gitResults.clear(); }); afterAll(() => { storedRepos.clear(); - storedCreds.clear(); gitResults.clear(); }); @@ -321,64 +288,6 @@ describe('assertSafeRepoTarget — SSRF guard', () => { }); }); -// ============================================================================= -// assertCredentialHostMatch — credential-exfiltration guard -// ============================================================================= - -describe('assertCredentialHostMatch — credential-exfiltration guard', () => { - test('allows ssh credential for any host (no secret to leak)', () => { - const c: any = { id: 1, name: 'my-ssh-key', authType: 'ssh', username: null, password: null }; - expect(() => assertCredentialHostMatch('https://github.com/repo.git', c)).not.toThrow(); - expect(() => assertCredentialHostMatch('ssh://git@internal-git.example.com/repo', c)).not.toThrow(); - }); - - test('allows none credential (no secret at all)', () => { - const c: any = { id: 2, name: 'public', authType: 'none', username: null, password: null }; - expect(() => assertCredentialHostMatch('https://github.com/repo.git', c)).not.toThrow(); - }); - - test('allows password credential whose username matches the full host (PAT-style)', () => { - const c: any = { id: 3, name: 'gh-pat', authType: 'password', username: 'github.com', password: 'x' }; - expect(() => assertCredentialHostMatch('https://github.com/repo.git', c)).not.toThrow(); - }); - - test('allows single-label username matching a single-label host', () => { - const c: any = { id: 4, name: 'bitbucket', authType: 'password', username: 'git', password: 'x' }; - expect(() => assertCredentialHostMatch('https://git/repo.git', c)).not.toThrow(); - }); - - test('REJECTS single-label username for an attacker subdomain (github.evil.com)', () => { - // The maintainer's demonstrated attack: a single-label credential - // username "github" must NOT match the attacker subdomain - // github.evil.com. The old first-label matching allowed this. - const c: any = { id: 5, name: 'github', authType: 'password', username: 'github', password: 'x' }; - expect(() => assertCredentialHostMatch('https://github.evil.com/x.git', c)).toThrow(/does not match/i); - }); - - test('REJECTS multi-label username for an attacker subdomain', () => { - // A multi-label username must match the FULL host — a subdomain of - // the username is not the host. - const c: any = { id: 6, name: 'gitlab', authType: 'password', username: 'gitlab.com', password: 'x' }; - expect(() => assertCredentialHostMatch('https://gitlab.com.evil.com/repo.git', c)).toThrow(/does not match/i); - }); - - test('REJECTS password credential for a completely unrelated host', () => { - const c: any = { id: 7, name: 'attacker', authType: 'password', username: 'someuser', password: 'x' }; - expect(() => assertCredentialHostMatch('https://attacker.tld/repo.git', c)).toThrow(/does not match/i); - }); - - test('REJECTS password credential for a private IP host (defense-in-depth)', () => { - // The URL is rejected by assertSafeRepoTarget upstream; the - // credential guard independently refuses it too. - const c: any = { id: 8, name: 'local', authType: 'password', username: '127.0.0.1', password: 'x' }; - expect(() => assertCredentialHostMatch('http://127.0.0.1/repo.git', c)).not.toThrow(); // username == host is allowed here; the URL itself is blocked by guard 1 - }); - - test('allows null/undefined credential (public repo)', () => { - expect(() => assertCredentialHostMatch('https://github.com/repo.git', null as any)).not.toThrow(); - }); -}); - // ============================================================================= // Endpoint pipeline (mirror of POST /api/git/branches) // ============================================================================= @@ -418,52 +327,6 @@ describe('POST /api/git/branches — pipeline', () => { expect(res.json.branches).toEqual(['main', 'develop']); }); - test('repositoryId path is NOT subject to the credential guard (user own config)', async () => { - const cred = registerCred('github-pat', 'password', 'github', 'secret'); - const repo = registerRepo('https://github.com/test/repo.git', cred.id); - setGitResult('https://github.com/test/repo.git', { code: 0, stdout: 'a1b2c3d4 refs/heads/main\n', stderr: '' }); - // The repositoryId path pairs the repo with its own stored credential - // — no host-affinity check (the pairing is the user's own config). - const res = await postGitBranches({ repositoryId: repo.id }); - expect(res.status).toBe(200); - }); - - test('raw-url + credentialId path is SUBJECT to the credential guard', async () => { - const cred = registerCred('github-pat', 'password', 'github', 'secret'); - // github-pat (username "github") + github.evil.com → REJECTED. - const res = await postGitBranches({ - url: 'https://github.evil.com/x.git', - credentialId: cred.id - }); - expect(res.status).toBe(400); - expect(res.json.error).toMatch(/does not match/i); - }); - - test('raw-url + credentialId path allows matching username', async () => { - const cred = registerCred('github-pat', 'password', 'github.com', 'secret'); - setGitResult('https://github.com/test/repo.git', { code: 0, stdout: 'a1b2c3d4 refs/heads/main\n', stderr: '' }); - const res = await postGitBranches({ - url: 'https://github.com/test/repo.git', - credentialId: cred.id - }); - expect(res.status).toBe(200); - }); - - test('raw-url + credentialId rejects unknown credential (404)', async () => { - const res = await postGitBranches({ url: 'https://github.com/test/repo.git', credentialId: 9999 }); - expect(res.status).toBe(404); - }); - - test('raw-url + credentialId with ssh credential is allowed', async () => { - const cred = registerCred('my-ssh', 'ssh', null, null); - setGitResult('https://github.com/test/repo.git', { code: 0, stdout: 'a1b2c3d4 refs/heads/main\n', stderr: '' }); - const res = await postGitBranches({ - url: 'https://github.com/test/repo.git', - credentialId: cred.id - }); - expect(res.status).toBe(200); - }); - test('unknown repositoryId returns 404', async () => { const res = await postGitBranches({ repositoryId: 9999 }); expect(res.status).toBe(404); diff --git a/tests/preview-env-security.test.ts b/tests/preview-env-security.test.ts index 87a7fb3..76ce7db 100644 --- a/tests/preview-env-security.test.ts +++ b/tests/preview-env-security.test.ts @@ -14,15 +14,12 @@ * 3. Path traversal — repoFilePath keeps composePath/envFilePath INSIDE the * clone dir, so `envFilePath: '../../secrets/x'` cannot read a file * outside the temp dir. - * 4. Credential affinity — assertCredentialHostMatch refuses to pair a raw - * url with a stored password credential whose username isn't the host - * (exfiltration defense). * * Mirror-test convention: the endpoint pipeline below mirrors the real * preview-env handler's input-parsing behaviour (composePath/url validation, - * SSRF, RCE, credential-affinity) and its guard order. Keep in sync. + * SSRF, RCE) and its guard order. Keep in sync. * - * 5. Permission gate (PR #1343 maintainer review) — the handler is gated on + * 4. Permission gate (PR #1343 maintainer review) — the handler is gated on * the git:edit permission (same model as /api/git/branches). That * authorization contract is NOT covered by this mirror (a mirror can * prove only its own copied logic) — it is covered against the ACTUAL @@ -30,10 +27,7 @@ */ import { describe, expect, test } from 'bun:test'; -import { - assertSafeRepoTarget, - assertCredentialHostMatch -} from '../src/lib/server/git-branch-lookup'; +import { assertSafeRepoTarget } from '../src/lib/server/git-branch-lookup'; import { repoFilePath, assertSafeRepoUrl } from '../src/lib/server/git-url-safety'; import { resolve, sep } from 'node:path'; @@ -132,33 +126,12 @@ describe('preview-env path traversal guard (repoFilePath)', () => { }); }); -// ============================================================================= -// Credential affinity — raw url may only pair with a host-matching credential. -// ============================================================================= - -describe('preview-env credential affinity (assertCredentialHostMatch)', () => { - test('rejects a single-label username for an attacker subdomain', () => { - const c: any = { id: 1, name: 'github', authType: 'password', username: 'github', password: 'x' }; - expect(() => assertCredentialHostMatch('https://github.evil.com/x.git', c)).toThrow(/does not match/i); - }); - test('allows a username that matches the full host', () => { - const c: any = { id: 2, name: 'gh', authType: 'password', username: 'github.com', password: 'x' }; - expect(() => assertCredentialHostMatch('https://github.com/x.git', c)).not.toThrow(); - }); - test('allows ssh credential (no secret to leak)', () => { - const c: any = { id: 3, name: 'ssh', authType: 'ssh', username: null, password: null }; - expect(() => assertCredentialHostMatch('https://github.com/x.git', c)).not.toThrow(); - }); -}); - // ============================================================================= // Endpoint pipeline (mirror of POST /api/git/preview-env). // ============================================================================= type StoredRepo = { id: number; url: string; credentialId: number | null }; -type StoredCred = { id: number; name: string; authType: string; username: string | null }; const storedRepos = new Map(); -const storedCreds = new Map(); /** * Mirror of POST /api/git/preview-env (src/routes/api/git/preview-env/+server.ts). @@ -185,30 +158,22 @@ async function postPreviewEnv( } } - const { repositoryId, url, credentialId, composePath } = body; + const { repositoryId, url, composePath } = body; if (!composePath || typeof composePath !== 'string') { return { status: 400, json: { error: 'Compose path is required' } }; } let repoUrl: string; - let credentialIdResolved: number | null = null; if (repositoryId) { const repo = storedRepos.get(repositoryId); if (!repo) return { status: 404, json: { error: 'Repository not found' } }; repoUrl = repo.url; - credentialIdResolved = repo.credentialId; } else if (url) { repoUrl = url; - credentialIdResolved = credentialId || null; } else { return { status: 400, json: { error: 'Either repositoryId or url is required' } }; } - let credential = null; - if (credentialIdResolved) { - credential = storedCreds.get(credentialIdResolved); - } - // Guard 1 (SSRF) — runs on both paths. try { assertSafeRepoTarget(repoUrl); @@ -216,15 +181,6 @@ async function postPreviewEnv( return { status: 400, json: { error: e.message } }; } - // Guard 2 (credential affinity) — raw url path only. - if (url && credentialIdResolved && credential) { - try { - assertCredentialHostMatch(repoUrl, credential as any); - } catch (e: any) { - return { status: 400, json: { error: e.message } }; - } - } - // Stubbed: previewRepoEnvFiles (clone + read env files). return { status: 200, json: { vars: {}, sources: {} } }; } @@ -258,30 +214,8 @@ describe('POST /api/git/preview-env — pipeline', () => { expect(res.status).toBe(400); }); - test('endpoint blocks a credential whose username does not match the host', async () => { - const cred = storedCreds.set(1, { id: 1, name: 'github', authType: 'password', username: 'github' }); - const res = await postPreviewEnv({ - url: 'https://github.evil.com/x.git', - credentialId: 1, - composePath: 'compose.yaml' - }); - expect(res.status).toBe(400); - expect(res.json.error).toMatch(/does not match/i); - }); - - test('endpoint allows a host-matching credential', async () => { - storedCreds.set(2, { id: 2, name: 'gh', authType: 'password', username: 'github.com' }); - const res = await postPreviewEnv({ - url: 'https://github.com/x.git', - credentialId: 2, - composePath: 'compose.yaml' - }); - expect(res.status).toBe(200); - }); - - test('repositoryId path is not subject to the credential guard', async () => { - storedCreds.set(3, { id: 3, name: 'gh', authType: 'password', username: 'github' }); - storedRepos.set(10, { id: 10, url: 'https://github.com/x.git', credentialId: 3 }); + test('repositoryId path resolves the stored repo URL', async () => { + storedRepos.set(10, { id: 10, url: 'https://github.com/x.git', credentialId: null }); const res = await postPreviewEnv({ repositoryId: 10, composePath: 'compose.yaml' }); expect(res.status).toBe(200); }); @@ -326,8 +260,7 @@ function makeAuthDeps(cfg: { authEnabled: boolean; canGitEdit: boolean }): { dep previewCalls++; return { vars: {}, sources: {} }; }, - assertSafeRepoTarget: () => {}, - assertCredentialHostMatch: () => {} + assertSafeRepoTarget: () => {} }; // The request object whose json() we track (the body parser, NOT the // response helper). If the permission gate runs BEFORE request.json(), diff --git a/tests/repositories-test-security.test.ts b/tests/repositories-test-security.test.ts index b301da3..7f72199 100644 --- a/tests/repositories-test-security.test.ts +++ b/tests/repositories-test-security.test.ts @@ -8,18 +8,13 @@ * * 1. SSRF — assertSafeRepoTarget blocks the maintainer's encoded-IP payloads. * 2. RCE — assertSafeRepoUrl rejects the ext::/file:: transport denylist. - * 3. Credential affinity — assertCredentialHostMatch refuses to pair a raw - * url with a stored password credential whose username isn't the host. * * Mirror-test convention: the endpoint pipeline below mirrors the real * repositories/test handler's guard order and error handling. Keep in sync. */ import { describe, expect, test } from 'bun:test'; -import { - assertSafeRepoTarget, - assertCredentialHostMatch -} from '../src/lib/server/git-branch-lookup'; +import { assertSafeRepoTarget } from '../src/lib/server/git-branch-lookup'; import { assertSafeRepoUrl } from '../src/lib/server/git-url-safety'; // ============================================================================= @@ -60,28 +55,10 @@ describe('repositories/test RCE guard (assertSafeRepoUrl)', () => { }); }); -// ============================================================================= -// Credential affinity. -// ============================================================================= - -describe('repositories/test credential affinity (assertCredentialHostMatch)', () => { - test('rejects a single-label username for an attacker subdomain', () => { - const c: any = { id: 1, name: 'github', authType: 'password', username: 'github', password: 'x' }; - expect(() => assertCredentialHostMatch('https://github.evil.com/x.git', c)).toThrow(/does not match/i); - }); - test('allows a username that matches the full host', () => { - const c: any = { id: 2, name: 'gh', authType: 'password', username: 'github.com', password: 'x' }; - expect(() => assertCredentialHostMatch('https://github.com/x.git', c)).not.toThrow(); - }); -}); - // ============================================================================= // Endpoint pipeline (mirror of POST /api/git/repositories/test). // ============================================================================= -type StoredCred = { id: number; name: string; authType: string; username: string | null }; -const storedCreds = new Map(); - /** * Mirror of POST /api/git/repositories/test * (src/routes/api/git/repositories/test/+server.ts). Keeps only the @@ -104,17 +81,6 @@ async function postRepositoriesTest( return { status: 400, json: { success: false, error: e.message } }; } - // Guard 2 (credential affinity). - if (body.credentialId != null && body.credentialId !== undefined) { - const credential = storedCreds.get(body.credentialId); - if (!credential) return { status: 404, json: { success: false, error: 'Credential not found' } }; - try { - assertCredentialHostMatch(body.url, credential as any); - } catch (e: any) { - return { status: 400, json: { success: false, error: e.message } }; - } - } - // Stubbed: testRepositoryConfig (ls-remote + clone). return { status: 200, json: { success: true } }; } @@ -137,24 +103,6 @@ describe('POST /api/git/repositories/test — pipeline', () => { expect(res.status).toBe(400); }); - test('endpoint blocks a credential whose username does not match the host', async () => { - storedCreds.set(1, { id: 1, name: 'github', authType: 'password', username: 'github' }); - const res = await postRepositoriesTest({ url: 'https://github.evil.com/x.git', credentialId: 1 }); - expect(res.status).toBe(400); - expect(res.json.error).toMatch(/does not match/i); - }); - - test('endpoint allows a host-matching credential', async () => { - storedCreds.set(2, { id: 2, name: 'gh', authType: 'password', username: 'github.com' }); - const res = await postRepositoriesTest({ url: 'https://github.com/x.git', credentialId: 2 }); - expect(res.status).toBe(200); - }); - - test('endpoint blocks an unknown credential (404)', async () => { - const res = await postRepositoriesTest({ url: 'https://github.com/x.git', credentialId: 9999 }); - expect(res.status).toBe(404); - }); - test('endpoint allows a public URL with no credential', async () => { const res = await postRepositoriesTest({ url: 'https://github.com/x.git' }); expect(res.status).toBe(200);