diff --git a/src/lib/server/git-url-safety.ts b/src/lib/server/git-url-safety.ts index 2cc83b0..3abf0b7 100644 --- a/src/lib/server/git-url-safety.ts +++ b/src/lib/server/git-url-safety.ts @@ -8,7 +8,7 @@ * git@host:path) passes unchanged, so this is fully backward-compatible. */ -import { resolve, sep } from 'node:path'; +import { resolve, sep, join, dirname } from 'node:path'; export function assertSafeRepoUrl(url: string): void { const u = (url || '').trim(); @@ -51,3 +51,14 @@ export function repoFilePath(repoPath: string, userRel: string, label: string): } return abs; } + +/** + * Absolute path of the base `.env` that sits beside a compose file inside a clone. The + * compose path is resolved through repoFilePath (containment-checked, ABSOLUTE), so the + * `.env` is just its sibling - NOT re-joined onto repoPath, which would double the prefix + * (#1495). + */ +export function repoBaseEnvPath(repoPath: string, composeUserPath: string): string { + const safeComposePath = repoFilePath(repoPath, composeUserPath, 'Compose path'); + return join(dirname(safeComposePath), '.env'); +} diff --git a/src/lib/server/git.ts b/src/lib/server/git.ts index 8d1dc0c..bff2dac 100644 --- a/src/lib/server/git.ts +++ b/src/lib/server/git.ts @@ -3,6 +3,7 @@ import { join, resolve, dirname, basename, relative } from 'node:path'; import { spawn as nodeSpawn, spawnSync } from 'node:child_process'; import type { ChildProcess } from 'node:child_process'; import { GIT_SSH_KEY_PATH_ENV, makeSshKeyPath, removeSshKey } from './git-ssh-key'; +import { permissionDeniedMessage } from './git-error'; import { getGitRepository, getGitCredential, @@ -18,7 +19,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 { assertSafeRepoUrl, assertSafeGitRef, repoFilePath, repoBaseEnvPath } from './git-url-safety'; import { assertSafeRepoTarget } from './git-branch-lookup'; import { resolveStackBranch } from '../git-stack-branch'; import { @@ -564,9 +565,10 @@ export interface TestResult { } /** - * Clean up git/SSH error messages for user display + * Clean up git/SSH error messages for user display. `ctx` tailors the permission-denied + * message to the credential's auth type and URL (#1509); omit it for a generic message. */ -function cleanGitError(stderr: string): string { +function cleanGitError(stderr: string, ctx?: { authType?: string | null; url?: string | null }): string { // Remove SSH warnings and noise const lines = stderr.split('\n').filter(line => { const l = line.trim().toLowerCase(); @@ -585,7 +587,7 @@ function cleanGitError(stderr: string): string { // Return cleaner message if (permissionLine) { - return 'Permission denied. Check your SSH credentials.'; + return permissionDeniedMessage(ctx?.authType, ctx?.url); } if (fatalLine) { // Clean up common fatal messages @@ -628,7 +630,7 @@ async function testRepositoryConnection(options: { if (result.code !== 0) { console.error('[Git] Connection test failed:', result.stderr); - return { success: false, error: cleanGitError(result.stderr) }; + return { success: false, error: cleanGitError(result.stderr, { authType: credential?.authType, url }) }; } // Parse the output to get commit hash @@ -642,7 +644,7 @@ async function testRepositoryConnection(options: { ); if (allBranchesResult.code !== 0) { - return { success: false, error: cleanGitError(allBranchesResult.stderr) }; + return { success: false, error: cleanGitError(allBranchesResult.stderr, { authType: credential?.authType, url }) }; } const allBranches = allBranchesResult.stdout.split('\n') @@ -785,7 +787,7 @@ export async function listRemoteBranches(options: { } if (result.code !== 0) { - return { branches: [], error: cleanGitError(result.stderr) }; + return { branches: [], error: cleanGitError(result.stderr, { authType: credential?.authType, url }) }; } // Each line: "\trefs/heads/". Keep the SHA (short) so the @@ -1983,7 +1985,9 @@ export async function previewRepoEnvFiles(options: PreviewEnvOptions): Promise

= {}; const sources: Record = {}; diff --git a/tests/preview-env-envpath.test.ts b/tests/preview-env-envpath.test.ts new file mode 100644 index 0000000..a121ecb --- /dev/null +++ b/tests/preview-env-envpath.test.ts @@ -0,0 +1,47 @@ +/** + * #1495 - "Populate environment variables" found no .env because the preview clone + * computed the base .env path by re-joining the temp dir onto an already-absolute + * compose path, doubling the prefix: + * join(tempDir, dirname(safeComposePath), '.env') + * -> /tmp/preview-x/tmp/preview-x/stacks/repro/.env (cannot exist -> 0 vars) + * + * This calls the REAL production helper repoBaseEnvPath (the exact code previewRepoEnvFiles + * uses), so a revert to the buggy join fails these tests - unlike a re-implemented formula, + * which would stay green. + */ +import { describe, expect, test } from 'bun:test'; +import { sep } from 'node:path'; +import { repoBaseEnvPath } from '../src/lib/server/git-url-safety'; + +describe('#1495 preview base .env path is not doubled', () => { + const tempDir = '/app/data/git-repos/preview-1788100341098-d7vddc'; + + test('relative compose path -> .env beside it, single temp prefix', () => { + const p = repoBaseEnvPath(tempDir, 'stacks/repro/docker-compose.yml'); + expect(p).toBe(`${tempDir}/stacks/repro/.env`); + // the temp dir must appear exactly once (the regression put it in twice) + expect(p.split(tempDir).length - 1).toBe(1); + expect(p.startsWith(tempDir + sep)).toBe(true); + }); + + test('leading-slash compose path is treated repo-root-relative, still single prefix', () => { + const p = repoBaseEnvPath(tempDir, '/stacks/repro/docker-compose.yml'); + expect(p).toBe(`${tempDir}/stacks/repro/.env`); + expect(p.split(tempDir).length - 1).toBe(1); + }); + + test('./ compose path resolves the same', () => { + const p = repoBaseEnvPath(tempDir, './stacks/repro/compose.yaml'); + expect(p).toBe(`${tempDir}/stacks/repro/.env`); + expect(p.split(tempDir).length - 1).toBe(1); + }); + + test('compose at repo root -> .env at repo root', () => { + const p = repoBaseEnvPath(tempDir, 'docker-compose.yml'); + expect(p).toBe(`${tempDir}/.env`); + }); + + test('a traversing compose path is still rejected (containment guard preserved)', () => { + expect(() => repoBaseEnvPath(tempDir, '../../etc/docker-compose.yml')).toThrow(); + }); +});