From b9625801b670ae47f3cf5546ed0ab0b63b573d06 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 27 Jul 2026 14:17:42 -0500 Subject: [PATCH] feat(environments): require one source control connection per environment Environments already had to keep all repositories on a single GitHub App installation, but nothing enforced the analogous invariant for the other providers: task launch resolves one provider token and one base URL per run, so an environment mixing providers or self-managed instances could never authenticate against all of its repositories. Replace getEnvironmentRepositoryInstallationError with getEnvironmentRepositoryConnectionError, which checks in order that all environment repositories share the same source control provider, the same GitHub App installation, and the same instance host. Provider and host comparisons ignore null/undefined values so historical rows that predate host backfill keep validating. All existing enforcement sites (web environment commands, MCP create/update environment handlers, environment-definition task launch, declarative environments) now query sourceControlProvider and host and run the broader check. Co-Authored-By: Claude Fable 5 --- .../environments/createEnvironment.ts | 14 +- .../environments/updateEnvironment.ts | 8 +- apps/api/src/handlers/tasks/launchTask.ts | 23 +-- .../trpc/commands/environments/index.test.ts | 89 ++++++++++- .../src/trpc/commands/environments/index.ts | 15 +- .../db/src/lib/declarative-environments.ts | 12 +- .../environment-repository-connection.test.ts | 147 ++++++++++++++++++ packages/types/src/environment-config.ts | 63 ++++++-- 8 files changed, 339 insertions(+), 32 deletions(-) create mode 100644 packages/types/src/__tests__/environment-repository-connection.test.ts diff --git a/apps/api/src/handlers/environments/createEnvironment.ts b/apps/api/src/handlers/environments/createEnvironment.ts index 4cb06fa45..303fbfb12 100644 --- a/apps/api/src/handlers/environments/createEnvironment.ts +++ b/apps/api/src/handlers/environments/createEnvironment.ts @@ -15,7 +15,7 @@ import { type TaskPayload, environmentConfigSchema, getMissingEnvironmentRepositoryError, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, } from '@roomote/types'; import { captureActivationEnvironmentSaved } from '@roomote/telemetry/server'; @@ -66,10 +66,12 @@ export function isEnvironmentNameUniqueViolation(error: unknown): boolean { export function getEnvironmentRepositoryConfigError( repositoryRows: Array<{ fullName: string; + sourceControlProvider: string | null | undefined; + host: string | null | undefined; installationId: string | number | null | undefined; }>, ): string | null { - return getEnvironmentRepositoryInstallationError(repositoryRows); + return getEnvironmentRepositoryConnectionError(repositoryRows); } function extractRunId(auth: McpAuth): number | null { @@ -273,7 +275,13 @@ export async function createEnvironment( eq(repositories.isActive, true), inArray(repositories.fullName, repositoryNames), ), - columns: { id: true, fullName: true, installationId: true }, + columns: { + id: true, + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }) : []; diff --git a/apps/api/src/handlers/environments/updateEnvironment.ts b/apps/api/src/handlers/environments/updateEnvironment.ts index 1b6e5a4e2..2c36ada60 100644 --- a/apps/api/src/handlers/environments/updateEnvironment.ts +++ b/apps/api/src/handlers/environments/updateEnvironment.ts @@ -132,7 +132,13 @@ export async function updateEnvironment( eq(repositories.isActive, true), inArray(repositories.fullName, repositoryNames), ), - columns: { id: true, fullName: true, installationId: true }, + columns: { + id: true, + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }) : []; diff --git a/apps/api/src/handlers/tasks/launchTask.ts b/apps/api/src/handlers/tasks/launchTask.ts index 3080e2613..17c003a60 100644 --- a/apps/api/src/handlers/tasks/launchTask.ts +++ b/apps/api/src/handlers/tasks/launchTask.ts @@ -18,7 +18,7 @@ import { ADMIN_REQUIRED_LAUNCH_TYPES, ALL_REPOSITORIES, buildTaskTypePromptAndWorkspacePayload, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, type StandardTask, type SuggestedTasksTask, TaskPayloadKind, @@ -48,7 +48,7 @@ function normalizeRepositoryFullNames(body: TaskLaunchRequest): string[] { async function validateSelectedRepositories( repositoryFullNames: string[], - options: { requireSingleInstallation?: boolean } = {}, + options: { requireSingleConnection?: boolean } = {}, ): Promise<{ error: string; status: 400 | 404 } | null> { if (repositoryFullNames.length === 0) { return null; @@ -59,7 +59,12 @@ async function validateSelectedRepositories( eq(repositories.isActive, true), inArray(repositories.fullName, repositoryFullNames), ), - columns: { fullName: true, installationId: true }, + columns: { + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }); const foundRepositories = new Set( @@ -76,12 +81,12 @@ async function validateSelectedRepositories( }; } - if (options.requireSingleInstallation) { - const installationError = - getEnvironmentRepositoryInstallationError(orgRepositories); + if (options.requireSingleConnection) { + const connectionError = + getEnvironmentRepositoryConnectionError(orgRepositories); - if (installationError) { - return { error: installationError, status: 400 }; + if (connectionError) { + return { error: connectionError, status: 400 }; } } @@ -223,7 +228,7 @@ export async function launchTask( const repositoryValidationError = shouldValidateRepositorySelection ? await validateSelectedRepositories(repositoryFullNames, { - requireSingleInstallation: requestedType === 'environment-definition', + requireSingleConnection: requestedType === 'environment-definition', }) : null; diff --git a/apps/web/src/trpc/commands/environments/index.test.ts b/apps/web/src/trpc/commands/environments/index.test.ts index b65bedf87..069b19ade 100644 --- a/apps/web/src/trpc/commands/environments/index.test.ts +++ b/apps/web/src/trpc/commands/environments/index.test.ts @@ -18,11 +18,15 @@ const { { id: 'repo-1', fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', installationId: 'installation-1', }, { id: 'repo-2', fullName: 'acme/web', + sourceControlProvider: 'github', + host: 'github.com', installationId: 'installation-1', }, ]), @@ -93,7 +97,11 @@ vi.mock('@/lib/server', () => ({ getRepositories: mockGetRepositories, })); -import { TaskPayloadKind } from '@roomote/types'; +import { + MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR, + MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR, + TaskPayloadKind, +} from '@roomote/types'; import type { UserAuthSuccess } from '@/types'; import { createEnvironmentCommand, @@ -392,6 +400,85 @@ describe('environment repository validation', () => { }); }); + it('rejects create when repositories span multiple instances of a provider', async () => { + mockDbSelect + .mockReturnValueOnce({ + from: () => ({ where: () => ({ limit: async () => [] }) }), + }) + .mockReturnValueOnce({ + from: () => ({ + where: async () => [ + { + id: 'repo-1', + fullName: 'acme/api', + sourceControlProvider: 'gitea', + host: 'gitea-one.example.com', + installationId: null, + }, + { + id: 'repo-2', + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea-two.example.com', + installationId: null, + }, + ], + }), + }); + + const result = await createEnvironmentCommand(buildMockAuth(), { + name: 'Gitea Test', + config: { + name: 'Gitea Test', + repositories: [{ repository: 'acme/api' }, { repository: 'acme/web' }], + }, + }); + + expect(result).toEqual({ + success: false, + error: MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR, + }); + }); + + it('reports mixed providers from validateConfigCommand', async () => { + mockDbSelect.mockReturnValueOnce({ + from: () => ({ + where: async () => [ + { + id: 'repo-1', + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + id: 'repo-2', + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: null, + }, + ], + }), + }); + mockCheckRepoAccess.mockResolvedValue(true); + mockGetBranches.mockResolvedValue(['main']); + + const result = await validateConfigCommand(buildMockAuth(), { + config: { + name: 'Mixed Test', + repositories: [ + { repository: 'acme/api', branch: 'main' }, + { repository: 'acme/web', branch: 'main' }, + ], + }, + }); + + expect(result.errors).toContain( + MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR, + ); + }); + it('does not validate Azure DevOps branches through GitHub', async () => { mockDbSelect.mockReturnValueOnce({ from: () => ({ diff --git a/apps/web/src/trpc/commands/environments/index.ts b/apps/web/src/trpc/commands/environments/index.ts index b781300de..30971de6d 100644 --- a/apps/web/src/trpc/commands/environments/index.ts +++ b/apps/web/src/trpc/commands/environments/index.ts @@ -37,7 +37,7 @@ import { type ComputeProvider, type EnvironmentConfig, environmentConfigSchema, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, getMissingEnvironmentRepositoryError, isExitedRunStatus, normalizeRepositorySelection, @@ -131,21 +131,27 @@ function assertAdmin(auth: UserAuthSuccess) { type SelectedRepositorySummary = { id: string; fullName: string; + sourceControlProvider: string; + host: string | null; installationId: string | null; }; type EnvironmentRepositoryRow = { id: string; fullName: string; + sourceControlProvider: string; + host: string | null; installationId: string | null; }; function getEnvironmentRepositoryConfigError( repositoriesToValidate: EnvironmentRepositoryRow[], ): string | null { - return getEnvironmentRepositoryInstallationError( + return getEnvironmentRepositoryConnectionError( repositoriesToValidate.map((repository) => ({ fullName: repository.fullName, + sourceControlProvider: repository.sourceControlProvider, + host: repository.host, installationId: repository.installationId, })), ); @@ -168,6 +174,8 @@ async function getEnvironmentRepositoryRows( .select({ id: repositories.id, fullName: repositories.fullName, + sourceControlProvider: repositories.sourceControlProvider, + host: repositories.host, installationId: repositories.installationId, }) .from(repositories) @@ -203,6 +211,8 @@ async function resolveSelectedRepositories( return { id: repository.id, fullName: repository.fullName, + sourceControlProvider: repository.sourceControlProvider, + host: repository.host, installationId: repository.installationId, }; }); @@ -1203,6 +1213,7 @@ export async function validateConfigCommand( fullName: repositories.fullName, installationId: repositories.installationId, sourceControlProvider: repositories.sourceControlProvider, + host: repositories.host, }) .from(repositories) .where( diff --git a/packages/db/src/lib/declarative-environments.ts b/packages/db/src/lib/declarative-environments.ts index 24a0e70c9..075a00bca 100644 --- a/packages/db/src/lib/declarative-environments.ts +++ b/packages/db/src/lib/declarative-environments.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import type { EnvironmentConfig } from '@roomote/types'; import { environmentConfigSchema, - getEnvironmentRepositoryInstallationError, + getEnvironmentRepositoryConnectionError, getMissingEnvironmentRepositoryError, } from '@roomote/types'; import { and, eq, inArray, isNotNull, notInArray } from 'drizzle-orm'; @@ -364,7 +364,13 @@ async function resolveConfiguredRepositories(config: EnvironmentConfig) { eq(repositories.isActive, true), inArray(repositories.fullName, repositoryNames), ), - columns: { id: true, fullName: true, installationId: true }, + columns: { + id: true, + fullName: true, + sourceControlProvider: true, + host: true, + installationId: true, + }, }) : []; @@ -473,7 +479,7 @@ async function applyDefinition( await resolveConfiguredRepositories(config); const repositoryConfigError = - getEnvironmentRepositoryInstallationError(repositoryRows); + getEnvironmentRepositoryConnectionError(repositoryRows); if (repositoryConfigError) { return { skip: { source, reason: repositoryConfigError } }; diff --git a/packages/types/src/__tests__/environment-repository-connection.test.ts b/packages/types/src/__tests__/environment-repository-connection.test.ts new file mode 100644 index 000000000..30aad4f46 --- /dev/null +++ b/packages/types/src/__tests__/environment-repository-connection.test.ts @@ -0,0 +1,147 @@ +// pnpm --filter @roomote/types test src/__tests__/environment-repository-connection.test.ts + +import { + MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR, + MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR, + MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR, + getEnvironmentRepositoryConnectionError, +} from '../environment-config'; + +describe('getEnvironmentRepositoryConnectionError', () => { + it('accepts an empty repository list', () => { + expect(getEnvironmentRepositoryConnectionError([])).toBeNull(); + }); + + it('accepts repositories from a single connection', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + ]), + ).toBeNull(); + }); + + it('rejects repositories that span multiple providers', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: null, + }, + ]), + ).toBe(MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR); + }); + + it('rejects repositories that span multiple GitHub App installations', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'other/web', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-2', + }, + ]), + ).toBe(MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR); + }); + + it('rejects repositories that span multiple instances of the same provider', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'gitea', + host: 'gitea-one.example.com', + installationId: null, + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea-two.example.com', + installationId: null, + }, + ]), + ).toBe(MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR); + }); + + it('ignores null hosts on rows that predate host backfill', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'gitea', + host: null, + installationId: null, + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: null, + }, + ]), + ).toBeNull(); + }); + + it('ignores missing provider metadata', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: undefined, + host: undefined, + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + ]), + ).toBeNull(); + }); + + it('reports the provider mismatch before installation or host mismatches', () => { + expect( + getEnvironmentRepositoryConnectionError([ + { + fullName: 'acme/api', + sourceControlProvider: 'github', + host: 'github.com', + installationId: 'installation-1', + }, + { + fullName: 'acme/web', + sourceControlProvider: 'gitea', + host: 'gitea.example.com', + installationId: 'installation-2', + }, + ]), + ).toBe(MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR); + }); +}); diff --git a/packages/types/src/environment-config.ts b/packages/types/src/environment-config.ts index 4f6f32993..eb7cec4aa 100644 --- a/packages/types/src/environment-config.ts +++ b/packages/types/src/environment-config.ts @@ -1025,31 +1025,68 @@ export function hasEnvironmentOidcTargets( return getEnvironmentOidcTargets(config).length > 0; } +export const MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR = + 'Environment repositories must all belong to the same source control provider.'; + export const MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR = 'Environment repositories must all belong to the same GitHub App installation.'; -type EnvironmentRepositoryInstallationReference = { +export const MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR = + 'Environment repositories must all belong to the same source control instance.'; + +type EnvironmentRepositoryConnectionReference = { fullName: string; + sourceControlProvider: string | null | undefined; + /** + * Source-control host (e.g. a self-managed Gitea/GitLab instance). Nullable + * for historical rows that predate host backfill, so only non-null values + * participate in the same-instance comparison. + */ + host: string | null | undefined; installationId: string | number | null | undefined; }; -export function getEnvironmentRepositoryInstallationError( - repositories: EnvironmentRepositoryInstallationReference[], +function distinctDefined(values: Array): Set { + return new Set( + values.filter((value): value is T => value !== null && value !== undefined), + ); +} + +/** + * Environments must not mix repositories from different source control + * connections: one provider, one GitHub App installation, and one + * provider instance (host) per environment. Task launch resolves a single + * provider token and base URL per run, so a mixed environment could never + * authenticate against all of its repositories. + */ +export function getEnvironmentRepositoryConnectionError( + repositories: EnvironmentRepositoryConnectionReference[], ): string | null { - const installationIds = new Set( - repositories - .map((repository) => repository.installationId) - .filter( - (installationId): installationId is string | number => - installationId !== null && installationId !== undefined, - ), + const providers = distinctDefined( + repositories.map((repository) => repository.sourceControlProvider), ); - if (installationIds.size <= 1) { - return null; + if (providers.size > 1) { + return MULTI_PROVIDER_ENVIRONMENT_REPOSITORIES_ERROR; + } + + const installationIds = distinctDefined( + repositories.map((repository) => repository.installationId), + ); + + if (installationIds.size > 1) { + return MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR; + } + + const hosts = distinctDefined( + repositories.map((repository) => repository.host), + ); + + if (hosts.size > 1) { + return MULTI_HOST_ENVIRONMENT_REPOSITORIES_ERROR; } - return MULTI_INSTALLATION_ENVIRONMENT_REPOSITORIES_ERROR; + return null; } export function getMissingEnvironmentRepositoryError(