populate environment variables again finds the repo env file

This commit is contained in:
Jarek Krochmalski committed 2026-09-02 15:52:44 +02:00
1 parent 34ceb74dc1
commit d03554c69d
3 files changed
+74 -13

No files matched your search

+12 -1
View File
@@ -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');
}
+15 -12
View File
@@ -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: "<sha>\trefs/heads/<name>". Keep the SHA (short) so the
@@ -1983,7 +1985,9 @@ export async function previewRepoEnvFiles(options: PreviewEnvOptions): Promise<P
// (path traversal). Without this, `envFilePath: '../../secrets/x'`
// reads a file outside the clone.
assertSafeRepoTarget(repoUrl);
const safeComposePath = repoFilePath(tempDir, composePath, 'Compose path');
// Validate containment now (throws on traversal) before any read; the base .env
// path itself is derived via repoBaseEnvPath below.
repoFilePath(tempDir, composePath, 'Compose path');
const safeEnvFilePath = envFilePath ? repoFilePath(tempDir, envFilePath, 'Env file path') : null;
assertSafeGitRef(branch);
const authenticatedUrl = buildRepoUrl(repoUrl, credential as GitCredential | null);
@@ -2009,10 +2013,9 @@ export async function previewRepoEnvFiles(options: PreviewEnvOptions): Promise<P
console.log(`${logPrefix} Clone successful`);
// Determine the compose directory (where .env file should be) — uses the
// VALIDATED path (already checked to be inside the temp dir).
const composeDir = dirname(safeComposePath);
const baseEnvPath = join(tempDir, composeDir, '.env');
// The base .env sits beside the compose file (repoBaseEnvPath keeps it inside the
// temp dir without doubling the prefix, #1495).
const baseEnvPath = repoBaseEnvPath(tempDir, composePath);
const vars: Record<string, string> = {};
const sources: Record<string, '.env' | 'envFile'> = {};
+47
View File
@@ -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();
});
});