fix(git-sources): harden validation, RBAC, concurrency, and deploy recovery (#603)

* fix(git-sources): harden validation, RBAC, concurrency, and deploy recovery

Tightens the surface area around the Git source feature:

- Enforce HTTPS-only repo URLs server-side (regex was permissive).
- Add stack:read permission check on git-source reads and filter the
  list endpoint by callable permission.
- Validate stack names before permission checks on mutation routes so
  scoped lookups never see unvalidated input.
- Cap repo_url / branch / compose_path / env_path / token lengths and
  require the stack directory to exist before upsert.
- Wrap pull() in the per-stack mutex to eliminate the pull/delete race
  that could orphan pending data.
- Block .git/ path components in compose_path / env_path so a
  misconfigured clone cannot leak repo metadata.
- Return {applied, deployed, deployError?} on deploy failure instead of
  throwing, and surface deployError as a warning toast so the user can
  retry deploy without re-pulling.
- Always clean the stack_git_sources row on stack delete even when the
  file deletion step fails.
- Add shadow-card-bevel to the pending alert and metadata card per the
  design system.
- Handle the new 403 response on the panel fetch gracefully.
- Add diagnostic logging gated on developer_mode (isDebugEnabled) across
  fetch / pull / apply / webhook paths with credential scrubbing.

* test(git-sources): expand coverage for hardening and route validation

- New route-level suite covers HTTPS enforcement, required fields,
  max-length caps on repo_url / branch / compose_path / env_path /
  token, the stack-existence 404 guard, and GET authz.
- Service tests cover the .git metadata guard on compose and env
  paths (including nested and substring-containing "git"), pull and
  apply rejections when no source is configured or pending is
  cleared, the sha-mismatch branch, and the deploy-failure return
  shape that now carries deployError.
- E2E adds three server-side contract assertions: PUT against a
  missing stack returns 404, http:// is rejected with 400, and
  .git/config is rejected as compose_path.

* docs(git-sources): document deploy-failure recovery path

Adds a Troubleshooting entry explaining that when apply succeeds but
the subsequent deploy fails, the compose content is already on disk
and the user can retry deploy from the stack editor without
re-pulling.

* docs(git-sources): add configuration, diff, pending, and webhook screenshots
This commit is contained in:
Anso
2026-04-14 22:32:42 -04:00
committed by GitHub
parent 789437cc46
commit 00901cf5bf
11 changed files with 575 additions and 59 deletions
@@ -0,0 +1,181 @@
/**
* Route-layer tests for the git-source API.
*
* Covers input-validation and guard behavior that lives in the Express
* handlers (not in GitSourceService), specifically:
* - HTTPS-only repo URL enforcement
* - Max-length caps on repo_url / branch / compose_path / env_path / token
* - Stack-existence 404 guard on PUT
* - 400 on invalid stack names
*
* Service-layer logic (encryption, error mapping, mutex, pending lifecycle)
* is covered in git-source-service.test.ts.
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import request from 'supertest';
import jwt from 'jsonwebtoken';
import fs from 'fs';
import path from 'path';
import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './helpers/setupTestDb';
let tmpDir: string;
let app: import('express').Express;
function adminToken(): string {
return jwt.sign({ username: TEST_USERNAME, role: 'admin' }, TEST_JWT_SECRET, { expiresIn: '1m' });
}
beforeAll(async () => {
tmpDir = await setupTestDb();
({ app } = await import('../index'));
// Seed a real stack directory so the PUT handler's existence guard is satisfied
// for tests that need to exercise validation past that point.
const composeDir = process.env.COMPOSE_DIR!;
fs.mkdirSync(path.join(composeDir, 'existing-stack'), { recursive: true });
fs.writeFileSync(path.join(composeDir, 'existing-stack', 'compose.yaml'), 'services:\n x:\n image: nginx\n');
});
afterAll(() => {
cleanupTestDb(tmpDir);
});
describe('PUT /api/stacks/:stackName/git-source — URL validation', () => {
it('rejects http:// URLs with 400', async () => {
const res = await request(app)
.put('/api/stacks/existing-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
repo_url: 'http://github.com/example/repo.git',
branch: 'main',
compose_path: 'compose.yaml',
auth_type: 'none',
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/HTTPS/i);
});
it('rejects missing repo_url with 400', async () => {
const res = await request(app)
.put('/api/stacks/existing-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
branch: 'main',
compose_path: 'compose.yaml',
auth_type: 'none',
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/repo_url/i);
});
});
describe('PUT /api/stacks/:stackName/git-source — max-length caps', () => {
const baseBody = {
branch: 'main',
compose_path: 'compose.yaml',
auth_type: 'none' as const,
};
it('rejects oversized repo_url', async () => {
const res = await request(app)
.put('/api/stacks/existing-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({ ...baseBody, repo_url: 'https://example.com/' + 'a'.repeat(2048) });
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/repo_url/i);
});
it('rejects oversized branch', async () => {
const res = await request(app)
.put('/api/stacks/existing-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
...baseBody,
repo_url: 'https://github.com/example/repo.git',
branch: 'b'.repeat(300),
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/branch/i);
});
it('rejects oversized compose_path', async () => {
const res = await request(app)
.put('/api/stacks/existing-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
...baseBody,
repo_url: 'https://github.com/example/repo.git',
compose_path: 'c'.repeat(1100),
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/compose_path/i);
});
it('rejects oversized env_path', async () => {
const res = await request(app)
.put('/api/stacks/existing-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
...baseBody,
repo_url: 'https://github.com/example/repo.git',
env_path: 'e'.repeat(1100),
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/env_path/i);
});
it('rejects oversized token', async () => {
const res = await request(app)
.put('/api/stacks/existing-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
...baseBody,
repo_url: 'https://github.com/example/repo.git',
auth_type: 'token',
token: 't'.repeat(9000),
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/token/i);
});
});
describe('PUT /api/stacks/:stackName/git-source — stack existence guard', () => {
it('returns 404 when the stack does not exist on the active node', async () => {
const res = await request(app)
.put('/api/stacks/ghost-stack/git-source')
.set('Authorization', `Bearer ${adminToken()}`)
.send({
repo_url: 'https://github.com/example/repo.git',
branch: 'main',
compose_path: 'compose.yaml',
auth_type: 'none',
});
expect(res.status).toBe(404);
expect(res.body.error).toMatch(/stack not found/i);
});
});
describe('git-source routes — invalid stack names', () => {
it('returns 400 for traversal attempts on GET per-stack', async () => {
const res = await request(app)
.get('/api/stacks/..%2fescape/git-source')
.set('Authorization', `Bearer ${adminToken()}`);
// URL-decoded name `../escape` fails isValidStackName.
expect([400, 404]).toContain(res.status);
});
});
describe('GET /api/git-sources', () => {
it('returns 200 and a JSON array for an authenticated admin', async () => {
const res = await request(app)
.get('/api/git-sources')
.set('Authorization', `Bearer ${adminToken()}`);
expect(res.status).toBe(200);
expect(Array.isArray(res.body)).toBe(true);
});
it('returns 401 without a valid token', async () => {
const res = await request(app).get('/api/git-sources');
expect(res.status).toBe(401);
});
});
@@ -446,3 +446,110 @@ describe('GitSourceService per-stack mutex', () => {
expect(order.indexOf('beta:end')).toBeLessThan(order.indexOf('alpha:end'));
});
});
describe('GitSourceService.fetchFromGit (.git metadata guard)', () => {
const svc = () => GitSourceService.getInstance();
it('rejects compose paths that target the .git directory', async () => {
await expect(svc().fetchFromGit({
repoUrl: 'https://github.com/example/repo.git',
branch: 'main',
composePath: '.git/config',
})).rejects.toMatchObject({ code: 'FILE_NOT_FOUND' });
expect(mockGitClone).not.toHaveBeenCalled();
});
it('rejects nested .git paths', async () => {
await expect(svc().fetchFromGit({
repoUrl: 'https://github.com/example/repo.git',
branch: 'main',
composePath: 'subdir/.git/HEAD',
})).rejects.toMatchObject({ code: 'FILE_NOT_FOUND' });
});
it('rejects env paths that target the .git directory', async () => {
await expect(svc().fetchFromGit({
repoUrl: 'https://github.com/example/repo.git',
branch: 'main',
composePath: 'compose.yaml',
envPath: '.git/config',
})).rejects.toMatchObject({ code: 'FILE_NOT_FOUND' });
});
it('allows paths that merely contain the substring "git"', async () => {
mockSuccessfulClone({ composePath: 'gitops.yaml' });
await expect(svc().fetchFromGit({
repoUrl: 'https://github.com/example/repo.git',
branch: 'main',
composePath: 'gitops.yaml',
})).resolves.toBeDefined();
});
});
describe('GitSourceService.pull', () => {
it('rejects when no Git source is configured for the stack', async () => {
const svc = GitSourceService.getInstance();
await expect(svc.pull('does-not-exist')).rejects.toMatchObject({ code: 'GIT_ERROR' });
});
});
describe('GitSourceService.apply', () => {
async function seedPending(stackName: string, composeContent: string, commitSha: string) {
mockSuccessfulClone({ compose: composeContent, sha: commitSha });
const svc = GitSourceService.getInstance();
await svc.upsert({
stackName,
repoUrl: 'https://github.com/example/repo.git',
branch: 'main',
composePath: 'compose.yaml',
syncEnv: false,
envPath: null,
authType: 'none',
autoApplyOnWebhook: false,
autoDeployOnApply: false,
});
await svc.pull(stackName);
return svc;
}
it('throws when pending has been cleared between pull and apply', async () => {
const svc = await seedPending('apply-cleared', 'services:\n x:\n image: alpine\n', 'aaaa111aaaa111aaaa111aaaa111aaaa111aaaa1');
DatabaseService.getInstance().clearGitSourcePending('apply-cleared');
await expect(svc.apply('apply-cleared', 'aaaa111aaaa111aaaa111aaaa111aaaa111aaaa1'))
.rejects.toMatchObject({ code: 'GIT_ERROR', message: expect.stringMatching(/no pending pull/i) });
});
it('throws when the commit sha does not match the pending sha', async () => {
const svc = await seedPending('apply-mismatch', 'services:\n x:\n image: alpine\n', 'bbbb222bbbb222bbbb222bbbb222bbbb222bbbb2');
await expect(svc.apply('apply-mismatch', 'deadbeef1234567890deadbeef1234567890dead'))
.rejects.toMatchObject({ code: 'GIT_ERROR', message: expect.stringMatching(/pending commit has changed/i) });
});
it('returns deployError when the deploy step fails after writing to disk', async () => {
const sha = 'cccc333cccc333cccc333cccc333cccc333cccc3';
const svc = await seedPending('apply-deploy-fail', 'services:\n x:\n image: alpine\n', sha);
// Stub validation (docker compose config is expensive and not needed here)
const validateSpy = vi.spyOn(svc, 'validateCompose').mockResolvedValue({ ok: true });
// Stub file write (FileSystemService expects a real stack dir)
const { FileSystemService } = await import('../services/FileSystemService');
const saveSpy = vi.spyOn(FileSystemService.prototype, 'saveStackContent').mockResolvedValue();
// Deploy will fail organically (docker CLI unavailable in the test env).
// We only assert the return SHAPE: apply must not throw, deployError must
// carry the failure detail so the UI can surface "applied but not deployed".
const result = await svc.apply('apply-deploy-fail', sha, { deploy: true });
expect(result.applied).toBe(true);
expect(result.deployed).toBe(false);
expect(result.deployError).toBeTruthy();
// Disk write happened; DB was marked applied even though deploy failed.
expect(saveSpy).toHaveBeenCalled();
const row = DatabaseService.getInstance().getGitSource('apply-deploy-fail');
expect(row?.last_applied_commit_sha).toBe(sha);
expect(row?.pending_commit_sha).toBeNull();
validateSpy.mockRestore();
saveSpy.mockRestore();
});
});