From 13cd673d50d0d4d5fac6ee5e4267fad739f27ffb Mon Sep 17 00:00:00 2001 From: "shaggy (mosaic-dev box)" Date: Sun, 9 Aug 2026 18:03:04 -0500 Subject: [PATCH 1/2] fix(docker): gateway runner needs git + MOSAIC_ROOT workspace dir; EXPOSE actual port 14242 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WorkspaceService shells out to git at runtime and roots workspaces at $MOSAIC_ROOT/.workspaces — the runner image had no git binary and no workspace directory. EXPOSE said 4000 but main.ts defaults to 14242. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ESFAnh2t9HmLwng8oW95St --- docker/gateway.Dockerfile | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docker/gateway.Dockerfile b/docker/gateway.Dockerfile index dc678990..7b4cc7c8 100644 --- a/docker/gateway.Dockerfile +++ b/docker/gateway.Dockerfile @@ -10,6 +10,8 @@ COPY pnpm-workspace.yaml pnpm-lock.yaml package.json ./ COPY apps/gateway/package.json ./apps/gateway/ COPY packages/ ./packages/ COPY plugins/ ./plugins/ +# the root prepare script runs scripts/install-hooks.mjs on install +COPY scripts/ ./scripts/ RUN pnpm install --frozen-lockfile COPY . . # Build gateway and all of its workspace dependencies via turbo dependency graph @@ -21,11 +23,17 @@ RUN pnpm --filter @mosaicstack/gateway --prod deploy --legacy /deploy FROM base AS runner WORKDIR /app ENV NODE_ENV=production +# WorkspaceService shells out to git at runtime and roots workspaces at +# $MOSAIC_ROOT/.workspaces (apps/gateway/src/workspace/workspace.service.ts); +# mount a volume over /opt/mosaic to persist workspaces across container restarts +RUN apk add --no-cache git && mkdir -p /opt/mosaic/.workspaces +ENV MOSAIC_ROOT=/opt/mosaic # Use the pnpm deploy output — resolves all deps into a flat, self-contained node_modules COPY --from=builder /deploy/node_modules ./node_modules COPY --from=builder /deploy/package.json ./package.json # 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 /app/apps/gateway/dist ./dist -EXPOSE 4000 +# gateway defaults to port 14242 (apps/gateway/src/main.ts) +EXPOSE 14242 CMD ["node", "dist/main.js"] From a34e92cf39077be54a15e743dd16139aabbedca6 Mon Sep 17 00:00:00 2001 From: "shaggy (mosaic-dev box)" Date: Sun, 9 Aug 2026 20:12:39 -0500 Subject: [PATCH 2/2] fix(gateway): harden workspace repository cloning --- .../workspace/workspace.controller.spec.ts | 104 ++++++++++++++++++ .../src/workspace/workspace.controller.ts | 24 ++-- apps/gateway/src/workspace/workspace.dto.ts | 33 ++++++ .../src/workspace/workspace.service.spec.ts | 91 ++++++++++++++- .../src/workspace/workspace.service.ts | 46 +++++++- docker/gateway.Dockerfile | 15 ++- 6 files changed, 289 insertions(+), 24 deletions(-) create mode 100644 apps/gateway/src/workspace/workspace.controller.spec.ts create mode 100644 apps/gateway/src/workspace/workspace.dto.ts diff --git a/apps/gateway/src/workspace/workspace.controller.spec.ts b/apps/gateway/src/workspace/workspace.controller.spec.ts new file mode 100644 index 00000000..c1a49e86 --- /dev/null +++ b/apps/gateway/src/workspace/workspace.controller.spec.ts @@ -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(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', 'git@example.com: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, + }); + }); +}); diff --git a/apps/gateway/src/workspace/workspace.controller.ts b/apps/gateway/src/workspace/workspace.controller.ts index 02bccdd2..769b0053 100644 --- a/apps/gateway/src/workspace/workspace.controller.ts +++ b/apps/gateway/src/workspace/workspace.controller.ts @@ -1,7 +1,11 @@ import { Body, Controller, Post, UseGuards } from '@nestjs/common'; import { AuthGuard } from '../auth/auth.guard.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') @UseGuards(AuthGuard) @@ -11,20 +15,14 @@ export class WorkspaceController { @Post() async create( @CurrentUser() user: { id: string }, - @Body() - body: { - name: string; - description?: string; - teamId?: string; - repoUrl?: string; - }, - ) { + @Body() dto: CreateWorkspaceDto, + ): Promise { return this.bootstrap.bootstrap({ - name: body.name, - description: body.description, + name: dto.name, + description: dto.description, userId: user.id, - teamId: body.teamId, - repoUrl: body.repoUrl, + teamId: dto.teamId, + repoUrl: dto.repoUrl, }); } } diff --git a/apps/gateway/src/workspace/workspace.dto.ts b/apps/gateway/src/workspace/workspace.dto.ts new file mode 100644 index 00000000..4ccce45e --- /dev/null +++ b/apps/gateway/src/workspace/workspace.dto.ts @@ -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; +} diff --git a/apps/gateway/src/workspace/workspace.service.spec.ts b/apps/gateway/src/workspace/workspace.service.spec.ts index c7de3c1d..52575b20 100644 --- a/apps/gateway/src/workspace/workspace.service.spec.ts +++ b/apps/gateway/src/workspace/workspace.service.spec.ts @@ -1,11 +1,33 @@ -import { describe, it, expect, beforeEach } from 'vitest'; -import { WorkspaceService } from './workspace.service.js'; +import { BadRequestException } from '@nestjs/common'; +import fs from 'node:fs/promises'; +import os from 'node:os'; 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(), +})); + +vi.mock('node:child_process', () => ({ + execFile: execFileMock, +})); describe('WorkspaceService', () => { let service: WorkspaceService; beforeEach(() => { + execFileMock.mockReset(); + execFileMock.mockImplementation((_command, _args, _options, callback) => { + callback(null, '', ''); + }); 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', 'git@example.com: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), + ); + }); + }); }); diff --git a/apps/gateway/src/workspace/workspace.service.ts b/apps/gateway/src/workspace/workspace.service.ts index a253ca26..01ac795f 100644 --- a/apps/gateway/src/workspace/workspace.service.ts +++ b/apps/gateway/src/workspace/workspace.service.ts @@ -1,10 +1,30 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { BadRequestException, Injectable, Logger } from '@nestjs/common'; import fs from 'node:fs/promises'; import path from 'node:path'; import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; 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 { id: string; @@ -39,14 +59,32 @@ export class WorkspaceService { * If repoUrl is provided, clone instead of init. */ async create(project: WorkspaceProject, repoUrl?: string): Promise { + if (repoUrl !== undefined) { + assertAllowedRepositoryUrl(repoUrl); + } + const workspacePath = this.resolvePath(project); // Create directory await fs.mkdir(workspacePath, { recursive: true }); - if (repoUrl) { - // Clone existing repo - await execFileAsync('git', ['clone', repoUrl, '.'], { cwd: workspacePath }); + if (repoUrl !== undefined) { + // Clone existing repo. Defense in depth keeps dangerous local helpers + // 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}`); } else { // Init new git repo diff --git a/docker/gateway.Dockerfile b/docker/gateway.Dockerfile index 7b4cc7c8..6534f188 100644 --- a/docker/gateway.Dockerfile +++ b/docker/gateway.Dockerfile @@ -25,15 +25,20 @@ WORKDIR /app ENV NODE_ENV=production # WorkspaceService shells out to git at runtime and roots workspaces at # $MOSAIC_ROOT/.workspaces (apps/gateway/src/workspace/workspace.service.ts); -# mount a volume over /opt/mosaic to persist workspaces across container restarts -RUN apk add --no-cache git && mkdir -p /opt/mosaic/.workspaces +# mount a volume over /opt/mosaic to persist workspaces across container restarts. +# 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 # Use the pnpm deploy output — resolves all deps into a flat, self-contained node_modules -COPY --from=builder /deploy/node_modules ./node_modules -COPY --from=builder /deploy/package.json ./package.json +COPY --chown=node:node --from=builder /deploy/node_modules ./node_modules +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; # 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) EXPOSE 14242 +USER node CMD ["node", "dist/main.js"]