From 0a9d4ad10fa054d46365844a398e8cc6648cacc3 Mon Sep 17 00:00:00 2001 From: jarek Date: Fri, 31 Jul 2026 10:10:56 +0200 Subject: [PATCH] validate git repo urls and refs, require a secret for webhook triggers, tidy internal endpoint comments --- src/lib/server/git-deletions.ts | 2 +- src/lib/server/git-url-safety.ts | 50 +++++++++++++++ src/lib/server/git.ts | 20 ++++-- src/lib/server/webhook-signature.ts | 32 ++++++++++ src/routes/api/git/stacks/+server.ts | 5 ++ src/routes/api/git/stacks/[id]/+server.ts | 14 ++++- .../api/git/stacks/[id]/webhook/+server.ts | 63 ++++++++----------- src/routes/api/git/webhook/[id]/+server.ts | 63 ++++++++----------- src/routes/api/jobs/[id]/+server.ts | 1 - src/routes/stacks/GitStackModal.svelte | 27 +++++--- 10 files changed, 186 insertions(+), 91 deletions(-) create mode 100644 src/lib/server/git-url-safety.ts create mode 100644 src/lib/server/webhook-signature.ts diff --git a/src/lib/server/git-deletions.ts b/src/lib/server/git-deletions.ts index b60d58e..44378fe 100644 --- a/src/lib/server/git-deletions.ts +++ b/src/lib/server/git-deletions.ts @@ -214,7 +214,7 @@ export function isSafeRelPath(p: string): boolean { } /** Resolve relPath inside root; returns null when it would escape root. */ -function containedPath(root: string, relPath: string): string | null { +export function containedPath(root: string, relPath: string): string | null { if (!isSafeRelPath(relPath)) return null; const abs = resolve(root, relPath); if (abs !== root && abs.startsWith(root + sep)) return abs; diff --git a/src/lib/server/git-url-safety.ts b/src/lib/server/git-url-safety.ts new file mode 100644 index 0000000..f215c56 --- /dev/null +++ b/src/lib/server/git-url-safety.ts @@ -0,0 +1,50 @@ +/** + * Deny-list validation for user-supplied git repo URLs and refs, kept in its OWN + * import-light module (no db/sqlite) so it is unit-testable. git.ts re-exports and + * calls these before any git subprocess. + * + * These reject inputs that git would execute as a command / read as a local file / + * parse as an OPTION. Every legitimate scheme (https, http, ssh, git, scp-like + * git@host:path) passes unchanged, so this is fully backward-compatible. + */ + +import { resolve, isAbsolute, sep } from 'node:path'; + +export function assertSafeRepoUrl(url: string): void { + const u = (url || '').trim(); + if (!u) throw new Error('Repository URL is required'); + // A value starting with '-' is consumed by git as an option (e.g. --upload-pack=). + if (u.startsWith('-')) throw new Error('Invalid repository URL'); + // git's ext::/fd:: transports execute a command; file:// reads local paths. + const lower = u.toLowerCase(); + if (lower.startsWith('ext::') || lower.startsWith('fd::') || lower.startsWith('file://')) { + throw new Error('Unsupported repository URL transport'); + } + // A bare local filesystem path (no scheme, no scp-like host:) is not a remote. + if (u.startsWith('/') || u.startsWith('./') || u.startsWith('../')) { + throw new Error('Repository URL must be a remote (https/ssh/git), not a local path'); + } +} + +/** A git ref (branch/tag) must not be parseable as an option. Empty is allowed + * (callers fall back to a default like HEAD). */ +export function assertSafeGitRef(ref: string | null | undefined): void { + const r = (ref || '').trim(); + if (r && r.startsWith('-')) throw new Error('Invalid branch/ref name'); +} + +/** + * Resolve a user-supplied relative path (composePath / envFilePath) INSIDE the clone + * dir, throwing if it would ESCAPE the clone. Uses resolve-then-containment (like a plain + * `join` + escape check): an in-repo `..` (e.g. `stacks/../compose.yml`) or a leading `./` + * still resolve, only a path that escapes the clone is refused. `label` names the field + * for the error message. + */ +export function repoFilePath(repoPath: string, userRel: string, label: string): string { + if (isAbsolute(userRel)) throw new Error(`${label} must be a relative path (got "${userRel}")`); + const abs = resolve(repoPath, userRel); + if (abs !== repoPath && !abs.startsWith(repoPath + sep)) { + throw new Error(`${label} must be a path inside the repository (got "${userRel}")`); + } + return abs; +} diff --git a/src/lib/server/git.ts b/src/lib/server/git.ts index bcc2873..72b11c7 100644 --- a/src/lib/server/git.ts +++ b/src/lib/server/git.ts @@ -17,6 +17,7 @@ import { import { deployStack, getStackDir } from './stacks'; import { sendEventNotification } from './notifications'; import { buildBasicAuthHeader } from './git-auth'; +import { assertSafeRepoUrl, assertSafeGitRef, repoFilePath } from './git-url-safety'; import { parseManifest, serializeManifest, @@ -307,6 +308,7 @@ function cleanupSshKey(credential: GitCredential | null): void { } function buildRepoUrl(url: string, credential: GitCredential | null): string { + assertSafeRepoUrl(url); // Never embed credentials in the URL — they leak via /proc//cmdline (see #1081). // HTTPS credentials are injected via GIT_CONFIG_COUNT env vars in buildGitEnv(). // Strip any existing credentials from the URL for safety. @@ -542,6 +544,7 @@ async function testRepositoryConnection(options: { const { url, branch, credential } = options; const env = await buildGitEnv(credential); + assertSafeGitRef(branch); const repoUrl = buildRepoUrl(url, credential); try { @@ -675,6 +678,7 @@ export async function syncRepository(repoId: number): Promise { if (!existsSync(repoPath)) { // Clone the repository (blobless clone - fetches all commits but blobs on-demand) + assertSafeGitRef(repo.branch); const repoUrl = buildRepoUrl(repo.url, credential); const result = await execGit( @@ -714,7 +718,7 @@ export async function syncRepository(repoId: number): Promise { currentCommit = commitResult.stdout.substring(0, 7); // Read the compose file - const composePath = join(repoPath, repo.composePath); + const composePath = repoFilePath(repoPath, repo.composePath, "Compose path"); if (!existsSync(composePath)) { throw new Error(`Compose file not found: ${repo.composePath}`); } @@ -924,6 +928,7 @@ export async function syncGitStack(stackId: number): Promise { } console.log(`${logPrefix} Cloning repository...`); + assertSafeGitRef(repo.branch); const repoUrl = buildRepoUrl(repo.url, credential); const result = await execGit( @@ -993,7 +998,7 @@ export async function syncGitStack(stackId: number): Promise { console.log(`${logPrefix} Current commit:`, currentCommit); // Read the compose file - const composePath = join(repoPath, gitStack.composePath); + const composePath = repoFilePath(repoPath, gitStack.composePath, "Compose path"); console.log(`${logPrefix} Reading compose file from:`, composePath); if (!existsSync(composePath)) { console.log(`${logPrefix} ERROR: Compose file not found at:`, composePath); @@ -1035,7 +1040,7 @@ export async function syncGitStack(stackId: number): Promise { let envFileContent: string | undefined; let envFileName: string | undefined; if (gitStack.envFilePath) { - const envFilePath = join(repoPath, gitStack.envFilePath); + const envFilePath = repoFilePath(repoPath, gitStack.envFilePath, "Env file path"); console.log(`${logPrefix} Looking for env file at:`, envFilePath); if (existsSync(envFilePath)) { try { @@ -1280,6 +1285,7 @@ export async function testGitStack(stackId: number): Promise { const credential = repo.credentialId ? await getGitCredential(repo.credentialId) : null; const env = await buildGitEnv(credential); + assertSafeGitRef(repo.branch); const repoUrl = buildRepoUrl(repo.url, credential); try { @@ -1388,6 +1394,7 @@ export async function deployGitStackWithProgress( rmSync(repoPath, { recursive: true, force: true }); } + assertSafeGitRef(repo.branch); const repoUrl = buildRepoUrl(repo.url, credential); // Step 3: Fetching (blobless clone - fetches all commits but blobs on-demand) @@ -1433,7 +1440,7 @@ export async function deployGitStackWithProgress( // Step 4: Reading compose file onProgress({ status: 'reading', message: `Reading ${gitStack.composePath}...`, step: 4, totalSteps }); - const composePath = join(repoPath, gitStack.composePath); + const composePath = repoFilePath(repoPath, gitStack.composePath, "Compose path"); if (!existsSync(composePath)) { throw new Error(`Compose file not found: ${gitStack.composePath}`); } @@ -1462,7 +1469,7 @@ export async function deployGitStackWithProgress( // Read env file if configured (optional - don't fail if missing) let envFileVars: Record | undefined; if (gitStack.envFilePath) { - const envFilePath = join(repoPath, gitStack.envFilePath); + const envFilePath = repoFilePath(repoPath, gitStack.envFilePath, "Env file path"); if (existsSync(envFilePath)) { try { const envContent = readFileSync(envFilePath, 'utf-8'); @@ -1527,7 +1534,7 @@ export async function deployGitStackWithProgress( // Determine env filename relative to compose dir (same logic as syncGitStack) let envFileName: string | undefined; if (gitStack.envFilePath) { - const envFilePath = join(repoPath, gitStack.envFilePath); + const envFilePath = repoFilePath(repoPath, gitStack.envFilePath, "Env file path"); if (existsSync(envFilePath)) { envFileName = relative(composeDir, envFilePath); } @@ -1789,6 +1796,7 @@ export async function previewRepoEnvFiles(options: PreviewEnvOptions): Promise

` where the + * hmac is HMAC-SHA256 of the raw request body keyed by the secret. + * - GitLab sends `X-Gitlab-Token: ` as a plain token compared for equality. + * + * The caller passes whichever header value is present as `signature`. + */ +export function verifyWebhookSignature(payload: string, signature: string | null, secret: string): boolean { + if (!signature) return false; + + if (signature.startsWith('sha256=')) { + const expectedSignature = 'sha256=' + crypto + .createHmac('sha256', secret) + .update(payload) + .digest('hex'); + const sigBuf = Buffer.from(signature); + const expectedBuf = Buffer.from(expectedSignature); + if (sigBuf.length !== expectedBuf.length) return false; + return crypto.timingSafeEqual(sigBuf, expectedBuf); + } + + // GitLab token: exact match, constant-time. + const sigBuf = Buffer.from(signature); + const secretBuf = Buffer.from(secret); + if (sigBuf.length !== secretBuf.length) return false; + return crypto.timingSafeEqual(sigBuf, secretBuf); +} diff --git a/src/routes/api/git/stacks/+server.ts b/src/routes/api/git/stacks/+server.ts index 9790418..7e9ff53 100644 --- a/src/routes/api/git/stacks/+server.ts +++ b/src/routes/api/git/stacks/+server.ts @@ -68,6 +68,11 @@ export const POST: RequestHandler = async (event) => { return json({ error: 'A stack with this name already exists on this environment' }, { status: 409 }); } + // A secret is mandatory when the webhook is enabled. + if (data.webhookEnabled && !data.webhookSecret?.trim()) { + return json({ error: 'A webhook secret is required when the webhook is enabled' }, { status: 400 }); + } + // Either repositoryId or new repo details (url, branch) must be provided let repositoryId = data.repositoryId; diff --git a/src/routes/api/git/stacks/[id]/+server.ts b/src/routes/api/git/stacks/[id]/+server.ts index ba29c6e..7eba8c1 100644 --- a/src/routes/api/git/stacks/[id]/+server.ts +++ b/src/routes/api/git/stacks/[id]/+server.ts @@ -63,6 +63,14 @@ export const PUT: RequestHandler = async (event) => { data.stackName = trimmedStackName; } + // A secret is mandatory when the webhook is enabled. + // Evaluate the effective post-update state (PUT is partial). + const effWebhookEnabled = data.webhookEnabled !== undefined ? data.webhookEnabled : existing.webhookEnabled; + const effWebhookSecret = data.webhookSecret !== undefined ? data.webhookSecret : existing.webhookSecret; + if (effWebhookEnabled && !effWebhookSecret?.trim()) { + return json({ error: 'A webhook secret is required when the webhook is enabled' }, { status: 400 }); + } + const oldStackName = existing.stackName; const updated = await updateGitStack(id, { stackName: data.stackName, @@ -195,8 +203,10 @@ export const DELETE: RequestHandler = async (event) => { // Delete the stack_sources record to free up the stack name await deleteStackSource(existing.stackName, existing.environmentId); - // Delete all env var overrides for this stack (all environments) - await deleteStackEnvVars(existing.stackName); + // Delete this stack's env var overrides ON THIS ENVIRONMENT ONLY. The same + // stack name can be deployed to multiple environments independently (per-env + // rows), so an unscoped delete would wipe another environment's vars/secrets. + await deleteStackEnvVars(existing.stackName, existing.environmentId); // Delete from database await deleteGitStack(id); diff --git a/src/routes/api/git/stacks/[id]/webhook/+server.ts b/src/routes/api/git/stacks/[id]/webhook/+server.ts index cfeca89..82b241a 100644 --- a/src/routes/api/git/stacks/[id]/webhook/+server.ts +++ b/src/routes/api/git/stacks/[id]/webhook/+server.ts @@ -3,29 +3,7 @@ import type { RequestHandler } from './$types'; import { getGitStack } from '$lib/server/db'; import { deployGitStack } from '$lib/server/git'; import { auditGitStack } from '$lib/server/audit'; -import crypto from 'node:crypto'; - -function verifySignature(payload: string, signature: string | null, secret: string): boolean { - if (!signature) return false; - - // Support both GitHub and GitLab webhook signatures - // GitHub: sha256= - // GitLab: just the token value in X-Gitlab-Token header - - if (signature.startsWith('sha256=')) { - const expectedSignature = 'sha256=' + crypto - .createHmac('sha256', secret) - .update(payload) - .digest('hex'); - const sigBuf = Buffer.from(signature); - const expectedBuf = Buffer.from(expectedSignature); - if (sigBuf.length !== expectedBuf.length) return false; - return crypto.timingSafeEqual(sigBuf, expectedBuf); - } - - // GitLab uses X-Gitlab-Token which should match exactly - return signature === secret; -} +import { verifyWebhookSignature } from '$lib/server/webhook-signature'; function detectSource(request: Request): string { if (request.headers.get('x-hub-signature-256')) return 'github'; @@ -52,20 +30,25 @@ export const POST: RequestHandler = async (event) => { const source = detectSource(request); - // Verify webhook secret if set - if (gitStack.webhookSecret) { - const payload = await request.text(); - const githubSignature = request.headers.get('x-hub-signature-256'); - const gitlabToken = request.headers.get('x-gitlab-token'); + // A secret is mandatory: reject if none is configured. + if (!gitStack.webhookSecret) { + await auditGitStack(event, 'webhook', id, gitStack.stackName, gitStack.environmentId, { + method: 'POST', source, error: 'no_secret_configured' + }); + return json({ error: 'Webhook secret is not configured for this stack' }, { status: 401 }); + } - const signature = githubSignature || gitlabToken; + const payload = await request.text(); + const githubSignature = request.headers.get('x-hub-signature-256'); + const gitlabToken = request.headers.get('x-gitlab-token'); - if (!verifySignature(payload, signature, gitStack.webhookSecret)) { - await auditGitStack(event, 'webhook', id, gitStack.stackName, gitStack.environmentId, { - method: 'POST', source, error: 'invalid_signature' - }); - return json({ error: 'Invalid webhook signature' }, { status: 401 }); - } + const signature = githubSignature || gitlabToken; + + if (!verifyWebhookSignature(payload, signature, gitStack.webhookSecret)) { + await auditGitStack(event, 'webhook', id, gitStack.stackName, gitStack.environmentId, { + method: 'POST', source, error: 'invalid_signature' + }); + return json({ error: 'Invalid webhook signature' }, { status: 401 }); } // Deploy the git stack (syncs and deploys only if there are changes) @@ -98,9 +81,17 @@ export const GET: RequestHandler = async (event) => { return json({ error: 'Webhook is not enabled for this stack' }, { status: 403 }); } + // A secret is mandatory (see POST handler). Reject if none is configured. + if (!gitStack.webhookSecret) { + await auditGitStack(event, 'webhook', id, gitStack.stackName, gitStack.environmentId, { + method: 'GET', source: 'get', error: 'no_secret_configured' + }); + return json({ error: 'Webhook secret is not configured for this stack' }, { status: 401 }); + } + // Verify secret via query parameter for GET requests const secret = url.searchParams.get('secret'); - if (gitStack.webhookSecret && secret !== gitStack.webhookSecret) { + if (secret !== gitStack.webhookSecret) { await auditGitStack(event, 'webhook', id, gitStack.stackName, gitStack.environmentId, { method: 'GET', source: 'get', error: 'invalid_secret' }); diff --git a/src/routes/api/git/webhook/[id]/+server.ts b/src/routes/api/git/webhook/[id]/+server.ts index 58789e5..61d36d3 100644 --- a/src/routes/api/git/webhook/[id]/+server.ts +++ b/src/routes/api/git/webhook/[id]/+server.ts @@ -3,29 +3,7 @@ import type { RequestHandler } from './$types'; import { getGitRepository } from '$lib/server/db'; import { deployFromRepository } from '$lib/server/git'; import { auditGitRepository } from '$lib/server/audit'; -import crypto from 'node:crypto'; - -function verifySignature(payload: string, signature: string | null, secret: string): boolean { - if (!signature) return false; - - // Support both GitHub and GitLab webhook signatures - // GitHub: sha256= - // GitLab: just the token value in X-Gitlab-Token header - - if (signature.startsWith('sha256=')) { - const expectedSignature = 'sha256=' + crypto - .createHmac('sha256', secret) - .update(payload) - .digest('hex'); - const sigBuf = Buffer.from(signature); - const expectedBuf = Buffer.from(expectedSignature); - if (sigBuf.length !== expectedBuf.length) return false; - return crypto.timingSafeEqual(sigBuf, expectedBuf); - } - - // GitLab uses X-Gitlab-Token which should match exactly - return signature === secret; -} +import { verifyWebhookSignature } from '$lib/server/webhook-signature'; function detectSource(request: Request): string { if (request.headers.get('x-hub-signature-256')) return 'github'; @@ -52,20 +30,25 @@ export const POST: RequestHandler = async (event) => { const source = detectSource(request); - // Verify webhook secret if set - if (repository.webhookSecret) { - const payload = await request.text(); - const githubSignature = request.headers.get('x-hub-signature-256'); - const gitlabToken = request.headers.get('x-gitlab-token'); + // A secret is mandatory: reject if none is configured. + if (!repository.webhookSecret) { + await auditGitRepository(event, 'webhook', id, repository.name, { + method: 'POST', source, error: 'no_secret_configured' + }); + return json({ error: 'Webhook secret is not configured for this repository' }, { status: 401 }); + } - const signature = githubSignature || gitlabToken; + const payload = await request.text(); + const githubSignature = request.headers.get('x-hub-signature-256'); + const gitlabToken = request.headers.get('x-gitlab-token'); - if (!verifySignature(payload, signature, repository.webhookSecret)) { - await auditGitRepository(event, 'webhook', id, repository.name, { - method: 'POST', source, error: 'invalid_signature' - }); - return json({ error: 'Invalid webhook signature' }, { status: 401 }); - } + const signature = githubSignature || gitlabToken; + + if (!verifyWebhookSignature(payload, signature, repository.webhookSecret)) { + await auditGitRepository(event, 'webhook', id, repository.name, { + method: 'POST', source, error: 'invalid_signature' + }); + return json({ error: 'Invalid webhook signature' }, { status: 401 }); } // Optionally check which branch was pushed (for GitHub) @@ -104,9 +87,17 @@ export const GET: RequestHandler = async (event) => { return json({ error: 'Webhook is not enabled for this repository' }, { status: 403 }); } + // A secret is mandatory (see POST handler). Reject if none is configured. + if (!repository.webhookSecret) { + await auditGitRepository(event, 'webhook', id, repository.name, { + method: 'GET', source: 'get', error: 'no_secret_configured' + }); + return json({ error: 'Webhook secret is not configured for this repository' }, { status: 401 }); + } + // Verify secret via query parameter for GET requests const secret = url.searchParams.get('secret'); - if (repository.webhookSecret && secret !== repository.webhookSecret) { + if (secret !== repository.webhookSecret) { await auditGitRepository(event, 'webhook', id, repository.name, { method: 'GET', source: 'get', error: 'invalid_secret' }); diff --git a/src/routes/api/jobs/[id]/+server.ts b/src/routes/api/jobs/[id]/+server.ts index 643cb1c..b399222 100644 --- a/src/routes/api/jobs/[id]/+server.ts +++ b/src/routes/api/jobs/[id]/+server.ts @@ -6,7 +6,6 @@ import type { RequestHandler } from './$types'; * GET /api/jobs/[id] * Poll a job's status and accumulated lines. * Returns all lines every time — client tracks its own cursor locally. - * No auth required: job IDs are UUIDs (unguessable), no sensitive data beyond what the initiating user triggered. */ export const GET: RequestHandler = async ({ params }) => { const job = getJob(params.id); diff --git a/src/routes/stacks/GitStackModal.svelte b/src/routes/stacks/GitStackModal.svelte index aca531d..f67e6e4 100644 --- a/src/routes/stacks/GitStackModal.svelte +++ b/src/routes/stacks/GitStackModal.svelte @@ -20,10 +20,7 @@ import { toast } from 'svelte-sonner'; import { focusFirstInput } from '$lib/utils'; import { readJobResponse } from '$lib/utils/sse-fetch'; - import { useSidebar } from '$lib/components/ui/sidebar/context.svelte'; - // Get sidebar state to adjust modal positioning - const sidebar = useSidebar(); // localStorage key for persisted split ratio const STORAGE_KEY_SPLIT = 'dockhand-git-stack-modal-split'; @@ -149,7 +146,7 @@ let formError = $state(''); let formSaving = $state(false); let showExistsWarning = $state(false); - let errors = $state<{ stackName?: string; repository?: string; repoName?: string; repoUrl?: string }>({}); + let errors = $state<{ stackName?: string; repository?: string; repoName?: string; repoUrl?: string; webhookSecret?: string }>({}); // Stack name validation: Docker Compose requires lowercase; must start with a // letter or number, and contain only lowercase letters, numbers, hyphens, underscores @@ -489,6 +486,11 @@ hasErrors = true; } + if (formWebhookEnabled && !formWebhookSecret.trim()) { + errors.webhookSecret = 'A webhook secret is required when the webhook is enabled'; + hasErrors = true; + } + if (hasErrors) return; // Check if stack already exists (only for new stacks) @@ -625,7 +627,7 @@ { if (isOpen) focusFirstInput(); }}> @@ -965,7 +967,10 @@ - + { if (formWebhookEnabled && !formWebhookSecret) formWebhookSecret = generateWebhookSecret(); }} + />

Receive push events from your Git provider to trigger sync and redeploy. @@ -1003,13 +1008,14 @@ {/if}

- +
errors.webhookSecret = undefined} /> {#if gitStack && formWebhookSecret}
+ {#if errors.webhookSecret} +

{errors.webhookSecret}

+ {/if}
{#if !gitStack}