feat(git-sources): harden create-from-git with LFS + submodule warnings (#609)

* feat(git-sources): surface LFS and submodule warnings on create Creating a stack from a Git repo now detects two common anomalies and tells the user about them rather than silently producing broken stacks. - LFS-pointer compose/env files fail early with a clear error instead of writing a 130-byte pointer stub to disk as real content. - Repositories containing .gitmodules produce a non-fatal warning so the user knows build contexts or volumes inside submodules will be empty at deploy time. Also refines the create dialog: sr-only DialogDescription for a11y, short commit SHA suffix on the success toast, env-path hint under the "Sync .env" checkbox showing which path will be read, and a route-level diagnostic log line gated on developer mode for support debugging. * test(git-sources): cover LFS, submodule, and nested env_path paths Adds unit coverage for the new LFS-pointer rejection and submodule warning plumbing, plus a nested compose_path case that exercises the default env_path resolution ("apps/web/compose.yaml" with sync_env on and env_path unset writes "apps/web/.env" both to disk and to the DB). Extends the E2E suite with a happy-path assertion that the full-length commit SHA is returned in the create response, and a UI flow that verifies the short-SHA suffix appears in the success toast. * docs(git-sources): add troubleshooting for LFS, submodules, HTTPS-only Adds troubleshooting entries for the newly surfaced LFS and submodule anomalies, expands the clone-timeout entry with the bounded-fetch explanation, and adds a dedicated HTTPS-only entry. Also consolidates the known limitations into a single list covering LFS, submodules, branch-tracking, and HTTPS-only. * fix(settings): use Route icon for notification routing The routing section in Settings previously used GitBranch, which now clashes with the Git Source feature's icon across the editor. Switch to Route (a branching-flow glyph) so routing rules have a distinct visual identity and aren't visually conflated with Git-backed stacks. * fix(git-sources): return 400 for upstream auth failures and disambiguate 404s Upstream git-host auth failures were mapping to HTTP 401, which the frontend apiFetch treats as a Sencho session expiry and fires the global logout event. They now return 400 with code=AUTH_FAILED in the body so the UI can branch on the discriminator without logging the user out. The status mapping moved into utils/gitSourceHttp so it can be unit-tested without booting the app. mapGitError also relied on the HttpError class alone, so any non-2xx response (including 404) was classified as auth failure. It now inspects the numeric status on err.data and considers whether a token was supplied, producing more actionable messages for missing repos, private repos, and wrong-scope tokens.
2026-08-27 18:57:09 +00:00 · 2026-04-15 11:31:29 -04:00
parent d9f50b3229
commit 6529a24530
11 changed files with 536 additions and 64 deletions
@@ -78,9 +78,13 @@ function mockSuccessfulClone(options: {
    mockGitClone.mockImplementation(async (args: { dir: string }) => {
        const { promises: fsp } = await import('fs');
        const path = await import('path');
-        await fsp.writeFile(path.join(args.dir, composePath), compose, 'utf-8');
+        const composeAbs = path.join(args.dir, composePath);
+        await fsp.mkdir(path.dirname(composeAbs), { recursive: true });
+        await fsp.writeFile(composeAbs, compose, 'utf-8');
        if (env !== null && envPath) {
-            await fsp.writeFile(path.join(args.dir, envPath), env, 'utf-8');
+            const envAbs = path.join(args.dir, envPath);
+            await fsp.mkdir(path.dirname(envAbs), { recursive: true });
+            await fsp.writeFile(envAbs, env, 'utf-8');
        }
    });
    mockGitLog.mockResolvedValue([{ oid: sha }]);
@@ -294,9 +298,50 @@ describe('GitSourceService error mapping', () => {
        composePath: 'compose.yaml',
    };

-    it('maps 401/auth errors to AUTH_FAILED', async () => {
-        mockGitClone.mockRejectedValueOnce(Object.assign(new Error('HTTP 401 Unauthorized'), { code: 'HttpError' }));
-        await expect(svc().fetchFromGit(fetchParams)).rejects.toMatchObject({ code: 'AUTH_FAILED' });
+    it('maps 401 with supplied token to AUTH_FAILED', async () => {
+        // A 401 only means "your token is wrong" when the caller actually sent one.
+        mockGitClone.mockRejectedValueOnce(Object.assign(new Error('HTTP Error: 401 Unauthorized'), {
+            code: 'HttpError',
+            data: { statusCode: 401 },
+        }));
+        await expect(svc().fetchFromGit({ ...fetchParams, token: 'ghp_some_token_value' }))
+            .rejects.toMatchObject({ code: 'AUTH_FAILED' });
+    });
+
+    it('maps 401 without a token to REPO_NOT_FOUND with a private-repo hint', async () => {
+        // GitHub returns 404 for genuinely missing public repos but 401/403 can
+        // also reach us for private repos that the caller did not authenticate
+        // to. Without a supplied token, "check your token" is misleading, so we
+        // surface it as "not found or private" and suggest adding a PAT.
+        mockGitClone.mockRejectedValueOnce(Object.assign(new Error('HTTP Error: 401 Unauthorized'), {
+            code: 'HttpError',
+            data: { statusCode: 401 },
+        }));
+        await expect(svc().fetchFromGit(fetchParams))
+            .rejects.toMatchObject({ code: 'REPO_NOT_FOUND', message: expect.stringMatching(/private/i) });
+    });
+
+    it('maps 404 HttpError to REPO_NOT_FOUND (not AUTH_FAILED)', async () => {
+        // Regression: isomorphic-git throws HttpError for every non-2xx, so a
+        // 404 on info/refs was previously misclassified as auth failure.
+        mockGitClone.mockRejectedValueOnce(Object.assign(new Error('HTTP Error: 404 Not Found'), {
+            code: 'HttpError',
+            data: { statusCode: 404 },
+        }));
+        await expect(svc().fetchFromGit(fetchParams))
+            .rejects.toMatchObject({ code: 'REPO_NOT_FOUND', message: expect.stringMatching(/private/i) });
+    });
+
+    it('maps 404 with a supplied token to REPO_NOT_FOUND with a token-scope hint', async () => {
+        // GitHub returns 404 for both "missing repo" and "token lacks access",
+        // so when the caller did supply a token we point them at URL + scopes
+        // instead of "add a PAT" (which they already did).
+        mockGitClone.mockRejectedValueOnce(Object.assign(new Error('HTTP Error: 404 Not Found'), {
+            code: 'HttpError',
+            data: { statusCode: 404 },
+        }));
+        await expect(svc().fetchFromGit({ ...fetchParams, token: 'ghp_some_token_value' }))
+            .rejects.toMatchObject({ code: 'REPO_NOT_FOUND', message: expect.stringMatching(/token has read access/i) });
    });

    it('maps 404/not-found errors to REPO_NOT_FOUND', async () => {
@@ -486,6 +531,74 @@ describe('GitSourceService.fetchFromGit (.git metadata guard)', () => {
    });
 });

+describe('GitSourceService.fetchFromGit (LFS + submodule detection)', () => {
+    const svc = () => GitSourceService.getInstance();
+    // Real pointer files start with this exact header (git-lfs spec v1).
+    const LFS_POINTER = 'version https://git-lfs.github.com/spec/v1\noid sha256:abc123\nsize 1024\n';
+
+    it('rejects an LFS-pointer compose file with a GIT_ERROR mentioning LFS', async () => {
+        mockSuccessfulClone({ compose: LFS_POINTER });
+        await expect(svc().fetchFromGit({
+            repoUrl: 'https://github.com/example/repo.git',
+            branch: 'main',
+            composePath: 'compose.yaml',
+        })).rejects.toMatchObject({
+            code: 'GIT_ERROR',
+            message: expect.stringMatching(/LFS/i),
+        });
+    });
+
+    it('rejects an LFS-pointer env file with a GIT_ERROR mentioning LFS', async () => {
+        mockSuccessfulClone({
+            compose: 'services:\n  web:\n    image: nginx\n',
+            env: LFS_POINTER,
+            envPath: '.env',
+        });
+        await expect(svc().fetchFromGit({
+            repoUrl: 'https://github.com/example/repo.git',
+            branch: 'main',
+            composePath: 'compose.yaml',
+            envPath: '.env',
+        })).rejects.toMatchObject({
+            code: 'GIT_ERROR',
+            message: expect.stringMatching(/LFS/i),
+        });
+    });
+
+    it('returns a submodule warning when .gitmodules is present', async () => {
+        mockGitClone.mockImplementation(async (args: { dir: string }) => {
+            const { promises: fsp } = await import('fs');
+            const p = await import('path');
+            await fsp.writeFile(p.join(args.dir, 'compose.yaml'), 'services:\n  web:\n    image: nginx\n', 'utf-8');
+            await fsp.writeFile(
+                p.join(args.dir, '.gitmodules'),
+                '[submodule "vendor"]\n\tpath = vendor\n\turl = https://github.com/example/vendor.git\n',
+                'utf-8',
+            );
+        });
+        mockGitLog.mockResolvedValue([{ oid: 'abc1234567890abc1234567890abc1234567890a' }]);
+
+        const result = await svc().fetchFromGit({
+            repoUrl: 'https://github.com/example/repo.git',
+            branch: 'main',
+            composePath: 'compose.yaml',
+        });
+        expect(result.warnings).toEqual(
+            expect.arrayContaining([expect.stringMatching(/submodules/i)]),
+        );
+    });
+
+    it('returns no warnings when .gitmodules is absent', async () => {
+        mockSuccessfulClone();
+        const result = await svc().fetchFromGit({
+            repoUrl: 'https://github.com/example/repo.git',
+            branch: 'main',
+            composePath: 'compose.yaml',
+        });
+        expect(result.warnings).toEqual([]);
+    });
+});
+
 describe('GitSourceService.pull', () => {
    it('rejects when no Git source is configured for the stack', async () => {
        const svc = GitSourceService.getInstance();
@@ -536,6 +649,44 @@ describe('GitSourceService.createStackFromGit', () => {
        await cleanupStackDir('create-happy');
    });

+    it('resolves a nested compose_path and nested env_path into the stack dir', async () => {
+        const sha = 'deadbeef1234567890deadbeef1234567890abcd';
+        mockSuccessfulClone({
+            compose: 'services:\n  web:\n    image: nginx\n',
+            env: 'FOO=nested\n',
+            composePath: 'apps/web/compose.yaml',
+            envPath: 'apps/web/.env',
+            sha,
+        });
+        const svc = GitSourceService.getInstance();
+
+        const result = await svc.createStackFromGit({
+            stackName: 'create-nested',
+            repoUrl: 'https://github.com/example/repo.git',
+            branch: 'main',
+            composePath: 'apps/web/compose.yaml',
+            syncEnv: true,
+            envPath: 'apps/web/.env',
+            authType: 'none',
+            token: null,
+            autoApplyOnWebhook: false,
+            autoDeployOnApply: false,
+        });
+
+        expect(result.envWritten).toBe(true);
+        expect(result.source.compose_path).toBe('apps/web/compose.yaml');
+        expect(result.source.env_path).toBe('apps/web/.env');
+
+        const { FileSystemService } = await import('../services/FileSystemService');
+        const env = await FileSystemService.getInstance().getEnvContent('create-nested');
+        expect(env).toBe('FOO=nested\n');
+
+        const row = DatabaseService.getInstance().getGitSource('create-nested');
+        expect(row?.env_path).toBe('apps/web/.env');
+
+        await cleanupStackDir('create-nested');
+    });
+
    it('writes the env file when sync_env is enabled', async () => {
        const sha = '0101010101010101010101010101010101010101';
        mockSuccessfulClone({