fix(gateway): harden workspace repository cloning
ci/woodpecker/pr/ci Pipeline was successful

This commit is contained in:
shaggy (mosaic-dev box)
2026-08-09 20:12:39 -05:00
parent 13cd673d50
commit a34e92cf39
6 changed files with 289 additions and 24 deletions
@@ -0,0 +1,104 @@
import 'reflect-metadata';
import {
type CanActivate,
type ExecutionContext,
type INestApplication,
ValidationPipe,
} from '@nestjs/common';
import { FastifyAdapter, type NestFastifyApplication } from '@nestjs/platform-fastify';
import { Test } from '@nestjs/testing';
import request from 'supertest';
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
import { AuthGuard } from '../auth/auth.guard.js';
import { ProjectBootstrapService } from './project-bootstrap.service.js';
import { WorkspaceController } from './workspace.controller.js';
const bootstrapMock = vi.fn(() =>
Promise.resolve({
projectId: 'project-1',
workspacePath: '/opt/mosaic/.workspaces/users/user-1/project-1',
}),
);
const authGuard: CanActivate = {
canActivate(context: ExecutionContext): boolean {
const requestContext = context.switchToHttp().getRequest<{ user?: { id: string } }>();
requestContext.user = { id: 'user-1' };
return true;
},
};
describe('POST /api/workspaces repoUrl validation', () => {
let app: INestApplication;
beforeAll(async () => {
const moduleRef = await Test.createTestingModule({
controllers: [WorkspaceController],
providers: [
{
provide: ProjectBootstrapService,
useValue: { bootstrap: bootstrapMock },
},
],
})
.overrideGuard(AuthGuard)
.useValue(authGuard)
.compile();
app = moduleRef.createNestApplication<NestFastifyApplication>(new FastifyAdapter());
app.useGlobalPipes(
new ValidationPipe({
whitelist: true,
forbidNonWhitelisted: true,
transform: true,
}),
);
await app.init();
await app.getHttpAdapter().getInstance().ready();
});
beforeEach(() => {
bootstrapMock.mockClear();
});
afterAll(async () => {
await app.close();
});
it.each([
['a leading-dash value', '--upload-pack=sh -c id'],
['an ext remote helper', 'ext::sh -c id'],
['a file URL', 'file:///tmp/repository'],
['an unparseable value', 'not a url'],
['an SSH shorthand', '[email protected]:acme/repository.git'],
['a scheme without //', 'https:example.com/acme/repository.git'],
['a hostless git URL', 'git:///tmp/repository'],
])('returns 400 for %s', async (_description, repoUrl) => {
const response = await request(app.getHttpServer())
.post('/api/workspaces')
.send({ name: 'Example', repoUrl })
.set('Content-Type', 'application/json');
expect(response.status).toBe(400);
expect(bootstrapMock).not.toHaveBeenCalled();
});
it.each([
['a plain HTTPS repository URL', 'https://example.com/acme/repository.git'],
['a git protocol repository URL', 'git://example.com/acme/repository.git'],
])('accepts %s', async (_description, repoUrl) => {
const response = await request(app.getHttpServer())
.post('/api/workspaces')
.send({ name: 'Example', repoUrl })
.set('Content-Type', 'application/json');
expect(response.status).toBe(201);
expect(bootstrapMock).toHaveBeenCalledWith({
name: 'Example',
description: undefined,
userId: 'user-1',
teamId: undefined,
repoUrl,
});
});
});
@@ -1,7 +1,11 @@
import { Body, Controller, Post, UseGuards } from '@nestjs/common'; import { Body, Controller, Post, UseGuards } from '@nestjs/common';
import { AuthGuard } from '../auth/auth.guard.js'; import { AuthGuard } from '../auth/auth.guard.js';
import { CurrentUser } from '../auth/current-user.decorator.js'; import { CurrentUser } from '../auth/current-user.decorator.js';
import { ProjectBootstrapService } from './project-bootstrap.service.js'; import {
ProjectBootstrapService,
type BootstrapProjectResult,
} from './project-bootstrap.service.js';
import { CreateWorkspaceDto } from './workspace.dto.js';
@Controller('api/workspaces') @Controller('api/workspaces')
@UseGuards(AuthGuard) @UseGuards(AuthGuard)
@@ -11,20 +15,14 @@ export class WorkspaceController {
@Post() @Post()
async create( async create(
@CurrentUser() user: { id: string }, @CurrentUser() user: { id: string },
@Body() @Body() dto: CreateWorkspaceDto,
body: { ): Promise<BootstrapProjectResult> {
name: string;
description?: string;
teamId?: string;
repoUrl?: string;
},
) {
return this.bootstrap.bootstrap({ return this.bootstrap.bootstrap({
name: body.name, name: dto.name,
description: body.description, description: dto.description,
userId: user.id, userId: user.id,
teamId: body.teamId, teamId: dto.teamId,
repoUrl: body.repoUrl, repoUrl: dto.repoUrl,
}); });
} }
} }
@@ -0,0 +1,33 @@
import { IsOptional, IsString, IsUrl, Matches, MaxLength } from 'class-validator';
export class CreateWorkspaceDto {
@IsString()
@MaxLength(255)
name!: string;
@IsOptional()
@IsString()
@MaxLength(10_000)
description?: string;
@IsOptional()
@IsString()
teamId?: string;
@IsOptional()
@IsString()
@Matches(/^(?:https|git):\/\//i, {
message: 'repoUrl must be a valid https:// or git:// URL',
})
@IsUrl(
{
protocols: ['https', 'git'],
require_host: true,
require_protocol: true,
require_tld: false,
require_valid_protocol: true,
},
{ message: 'repoUrl must be a valid https:// or git:// URL' },
)
repoUrl?: string;
}
@@ -1,11 +1,33 @@
import { describe, it, expect, beforeEach } from 'vitest'; import { BadRequestException } from '@nestjs/common';
import { WorkspaceService } from './workspace.service.js'; import fs from 'node:fs/promises';
import os from 'node:os';
import path from 'node:path'; import path from 'node:path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { WorkspaceService } from './workspace.service.js';
type ExecFileMock = (
command: string,
args: readonly string[],
options: { cwd: string },
callback: (error: Error | null, stdout: string, stderr: string) => void,
) => void;
const { execFileMock } = vi.hoisted(() => ({
execFileMock: vi.fn<ExecFileMock>(),
}));
vi.mock('node:child_process', () => ({
execFile: execFileMock,
}));
describe('WorkspaceService', () => { describe('WorkspaceService', () => {
let service: WorkspaceService; let service: WorkspaceService;
beforeEach(() => { beforeEach(() => {
execFileMock.mockReset();
execFileMock.mockImplementation((_command, _args, _options, callback) => {
callback(null, '', '');
});
service = new WorkspaceService(); service = new WorkspaceService();
}); });
@@ -76,4 +98,69 @@ describe('WorkspaceService', () => {
} }
}); });
}); });
describe('create', () => {
const project = {
id: 'project-1',
ownerType: 'user',
userId: 'user-1',
teamId: null,
} as const;
let originalRoot: string | undefined;
let temporaryRoot: string;
beforeEach(async () => {
originalRoot = process.env['MOSAIC_ROOT'];
temporaryRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'mosaic-workspace-'));
process.env['MOSAIC_ROOT'] = temporaryRoot;
service = new WorkspaceService();
});
afterEach(async () => {
if (originalRoot === undefined) {
delete process.env['MOSAIC_ROOT'];
} else {
process.env['MOSAIC_ROOT'] = originalRoot;
}
await fs.rm(temporaryRoot, { recursive: true, force: true });
});
it.each([
['a leading-dash URL', '--upload-pack=sh -c id'],
['an ext remote helper', 'ext::sh -c id'],
['a file URL', 'file:///tmp/repository'],
['an unparseable value', 'not a url'],
['an SSH shorthand', '[email protected]:acme/repository.git'],
['a scheme without //', 'https:example.com/acme/repository.git'],
['a hostless git URL', 'git:///tmp/repository'],
])('rejects %s before invoking git', async (_description, repoUrl) => {
await expect(service.create(project, repoUrl)).rejects.toBeInstanceOf(BadRequestException);
expect(execFileMock).not.toHaveBeenCalled();
});
it.each([
['an HTTPS URL', 'https://example.com/acme/repository.git'],
['a git protocol URL', 'git://example.com/acme/repository.git'],
])('accepts %s and invokes hardened git clone arguments', async (_description, repoUrl) => {
const workspacePath = await service.create(project, repoUrl);
expect(execFileMock).toHaveBeenCalledOnce();
expect(execFileMock).toHaveBeenCalledWith(
'git',
[
'-c',
'protocol.ext.allow=never',
'-c',
'protocol.file.allow=never',
'clone',
'--',
repoUrl,
'.',
],
{ cwd: workspacePath },
expect.any(Function),
);
});
});
}); });
@@ -1,10 +1,30 @@
import { Injectable, Logger } from '@nestjs/common'; import { BadRequestException, Injectable, Logger } from '@nestjs/common';
import fs from 'node:fs/promises'; import fs from 'node:fs/promises';
import path from 'node:path'; import path from 'node:path';
import { execFile } from 'node:child_process'; import { execFile } from 'node:child_process';
import { promisify } from 'node:util'; import { promisify } from 'node:util';
const execFileAsync = promisify(execFile); const execFileAsync = promisify(execFile);
const allowedRepositoryProtocols = new Set(['https:', 'git:']);
const repositoryUrlPrefixPattern = /^(?:https|git):\/\//i;
const repositoryUrlError = 'repoUrl must be a valid https:// or git:// URL';
function assertAllowedRepositoryUrl(repoUrl: string): void {
if (repoUrl.startsWith('-') || !repositoryUrlPrefixPattern.test(repoUrl)) {
throw new BadRequestException(repositoryUrlError);
}
let parsedUrl: URL;
try {
parsedUrl = new URL(repoUrl);
} catch {
throw new BadRequestException(repositoryUrlError);
}
if (!allowedRepositoryProtocols.has(parsedUrl.protocol) || parsedUrl.hostname.length === 0) {
throw new BadRequestException(repositoryUrlError);
}
}
export interface WorkspaceProject { export interface WorkspaceProject {
id: string; id: string;
@@ -39,14 +59,32 @@ export class WorkspaceService {
* If repoUrl is provided, clone instead of init. * If repoUrl is provided, clone instead of init.
*/ */
async create(project: WorkspaceProject, repoUrl?: string): Promise<string> { async create(project: WorkspaceProject, repoUrl?: string): Promise<string> {
if (repoUrl !== undefined) {
assertAllowedRepositoryUrl(repoUrl);
}
const workspacePath = this.resolvePath(project); const workspacePath = this.resolvePath(project);
// Create directory // Create directory
await fs.mkdir(workspacePath, { recursive: true }); await fs.mkdir(workspacePath, { recursive: true });
if (repoUrl) { if (repoUrl !== undefined) {
// Clone existing repo // Clone existing repo. Defense in depth keeps dangerous local helpers
await execFileAsync('git', ['clone', repoUrl, '.'], { cwd: workspacePath }); // disabled and terminates option parsing before positional arguments.
await execFileAsync(
'git',
[
'-c',
'protocol.ext.allow=never',
'-c',
'protocol.file.allow=never',
'clone',
'--',
repoUrl,
'.',
],
{ cwd: workspacePath },
);
this.logger.log(`Cloned ${repoUrl} into workspace ${workspacePath}`); this.logger.log(`Cloned ${repoUrl} into workspace ${workspacePath}`);
} else { } else {
// Init new git repo // Init new git repo
+10 -5
View File
@@ -25,15 +25,20 @@ WORKDIR /app
ENV NODE_ENV=production ENV NODE_ENV=production
# WorkspaceService shells out to git at runtime and roots workspaces at # WorkspaceService shells out to git at runtime and roots workspaces at
# $MOSAIC_ROOT/.workspaces (apps/gateway/src/workspace/workspace.service.ts); # $MOSAIC_ROOT/.workspaces (apps/gateway/src/workspace/workspace.service.ts);
# mount a volume over /opt/mosaic to persist workspaces across container restarts # mount a volume over /opt/mosaic to persist workspaces across container restarts.
RUN apk add --no-cache git && mkdir -p /opt/mosaic/.workspaces # Intentionally unpinned: Alpine's signed repository is the trust anchor; pinning
# git was declined so routine base-image security updates remain maintainable.
RUN apk add --no-cache git \
&& mkdir -p /opt/mosaic/.workspaces \
&& chown -R node:node /opt/mosaic /app
ENV MOSAIC_ROOT=/opt/mosaic ENV MOSAIC_ROOT=/opt/mosaic
# Use the pnpm deploy output — resolves all deps into a flat, self-contained node_modules # Use the pnpm deploy output — resolves all deps into a flat, self-contained node_modules
COPY --from=builder /deploy/node_modules ./node_modules COPY --chown=node:node --from=builder /deploy/node_modules ./node_modules
COPY --from=builder /deploy/package.json ./package.json COPY --chown=node:node --from=builder /deploy/package.json ./package.json
# dist is declared in package.json "files" so pnpm deploy copies it into /deploy; # dist is declared in package.json "files" so pnpm deploy copies it into /deploy;
# copy from builder explicitly as belt-and-suspenders # copy from builder explicitly as belt-and-suspenders
COPY --from=builder /app/apps/gateway/dist ./dist COPY --chown=node:node --from=builder /app/apps/gateway/dist ./dist
# gateway defaults to port 14242 (apps/gateway/src/main.ts) # gateway defaults to port 14242 (apps/gateway/src/main.ts)
EXPOSE 14242 EXPOSE 14242
USER node
CMD ["node", "dist/main.js"] CMD ["node", "dist/main.js"]