From 4a2f3697608c88d2dd2e916d2b61fcbc967e9651 Mon Sep 17 00:00:00 2001 From: Bircck <55695195+Bircck@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:32:45 +0200 Subject: [PATCH 1/2] fix(website): keep Azure DevOps request URLs inside the configured repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL alert #3 (js/request-forgery, critical): request data reached the URL of an Azure DevOps call that carries the managed-identity token. /api/diagram/version read `repositoryName` from the query string, and AzureDevOpsService interpolated it raw into the path, so a value like `../../../OtherProject/_apis/...` moved the request to a different repository. `filePath` and `commitId` went unencoded into the query string, where an `&` could add parameters the caller never wrote. Three changes, all behaviour-preserving for valid input: - `/api/diagram/version` now reads ADO_REPOSITORY_NAME from the environment, like every sibling route already did. Nothing calls this endpoint with the parameter — no frontend caller exists at all. - `buildGitApiUrl` builds every Git REST URL, encoding the repository path segment and building the query with URLSearchParams. `getRepositoryInfo` already encoded its segment; this applies the same treatment everywhere. - `makeAuthenticatedRequest` refuses to attach credentials to a URL outside the configured organization and project. `new URL()` resolves `..` before the check, so traversal is caught after normalization. Each of the eight URLs decodes to the same origin, path and query as before; `npm run lint` and `npm run build` pass. --- .../azuredevops/ManagedIdentityAuthService.ts | 21 +++++ Website/app/api/diagram/version/route.ts | 5 +- .../app/api/services/AzureDevOpsService.ts | 79 +++++++++++++------ 3 files changed, 81 insertions(+), 24 deletions(-) diff --git a/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts b/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts index daaddb44..7ecb69fe 100644 --- a/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts +++ b/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts @@ -46,7 +46,28 @@ class ManagedIdentityAuth { } } + /** + * Rejects any URL that does not sit inside the configured organization and + * project. Callers assemble URLs from request data, so this is the last + * point at which a value that escaped its path segment can be caught - + * before the managed-identity token is attached and sent. `new URL()` + * resolves `..` first, so traversal is compared after normalization. + */ + private assertConfiguredTarget(url: string): void { + const target = new URL(url); + const allowed = new URL(`${this.config.organizationUrl}${this.config.projectName}/`); + + if (target.origin !== allowed.origin || !target.pathname.startsWith(allowed.pathname)) { + throw new Error( + `Refusing to send Azure DevOps credentials to ${target.origin}${target.pathname} - ` + + `only ${allowed.origin}${allowed.pathname} is configured.` + ); + } + } + async makeAuthenticatedRequest(url: string, options: RequestInit = {}): Promise { + this.assertConfiguredTarget(url); + // Use PAT for local development, Managed Identity for production const pat = process.env.ADO_PAT; const isLocal = process.env.NODE_ENV === 'development' || pat; diff --git a/Website/app/api/diagram/version/route.ts b/Website/app/api/diagram/version/route.ts index 01ea6b78..028457ed 100644 --- a/Website/app/api/diagram/version/route.ts +++ b/Website/app/api/diagram/version/route.ts @@ -6,7 +6,10 @@ export async function GET(request: NextRequest) { const { searchParams } = new URL(request.url); const filePath = searchParams.get('filePath'); const commitId = searchParams.get('commitId'); - const repositoryName = searchParams.get('repositoryName') || undefined; + // Sourced from configuration, never from the request: it lands in the + // path of the Azure DevOps URL, where a caller-supplied value could + // point the authenticated request at another repository. + const repositoryName = process.env.ADO_REPOSITORY_NAME || ''; if (!filePath) { return NextResponse.json( diff --git a/Website/app/api/services/AzureDevOpsService.ts b/Website/app/api/services/AzureDevOpsService.ts index 94dc7420..03d6f92f 100644 --- a/Website/app/api/services/AzureDevOpsService.ts +++ b/Website/app/api/services/AzureDevOpsService.ts @@ -84,6 +84,37 @@ class AzureDevOpsError extends Error { } } +/** + * Builds a URL for the Azure DevOps Git REST API. + * + * The organization and project come from configuration, but the repository + * name and the query values are routinely taken from a request. Interpolating + * those straight into the URL lets a `../` walk out of the repositories path + * and an `&` append parameters the caller never wrote - on a request that + * carries the managed-identity token. Encoding each part keeps every value + * inside the slot it was meant for. + */ +function buildGitApiUrl( + repositoryName: string | undefined, + resourcePath = '', + query: Record = {} +): string { + const config = managedAuth.getConfig(); + + const params = new URLSearchParams(); + for (const [key, value] of Object.entries(query)) { + if (value !== undefined) { + params.set(key, String(value)); + } + } + params.set('api-version', '7.0'); + + const repository = encodeURIComponent(repositoryName ?? ''); + const resource = resourcePath ? `/${resourcePath}` : ''; + + return `${config.organizationUrl}${config.projectName}/_apis/git/repositories/${repository}${resource}?${params}`; +} + /** * Lists files in the Azure DevOps Git repository * @param options Configuration for file retrieval @@ -97,12 +128,13 @@ export async function listFilesFromRepo(options: LoadFileOptions): Promise(options: LoadFileOptions): Promise } = options; try { - // Get ADO configuration - const config = managedAuth.getConfig(); - // Validate inputs if (!filePath) { throw new AzureDevOpsError('File path is required'); @@ -271,7 +297,12 @@ export async function pullFileFromRepo(options: LoadFileOptions): Promise // Construct the API URL for getting file content const normalizedPath = filePath.startsWith('/') ? filePath.substring(1) : filePath; - const fileUrl = `${config.organizationUrl}${config.projectName}/_apis/git/repositories/${repositoryName}/items?path=/${normalizedPath}&versionDescriptor.version=${branch}&versionDescriptor.versionType=branch&includeContent=true&api-version=7.0`; + const fileUrl = buildGitApiUrl(repositoryName, 'items', { + path: `/${normalizedPath}`, + 'versionDescriptor.version': branch, + 'versionDescriptor.versionType': 'branch', + includeContent: 'true' + }); const response = await managedAuth.makeAuthenticatedRequest(fileUrl); @@ -316,9 +347,6 @@ export async function listFileVersions(options: FileVersionOptions): Promise(options: LoadFileVersionOptions): Promi } = options; try { - // Get ADO configuration - const config = managedAuth.getConfig(); - // Validate inputs if (!filePath || !commitId) { throw new AzureDevOpsError('File path and commit ID are required'); @@ -419,7 +447,12 @@ export async function pullFileVersion(options: LoadFileVersionOptions): Promi // Construct the API URL for getting file content at specific commit const normalizedPath = filePath.startsWith('/') ? filePath.substring(1) : filePath; - const fileUrl = `${config.organizationUrl}${config.projectName}/_apis/git/repositories/${repositoryName}/items?path=/${normalizedPath}&versionDescriptor.version=${commitId}&versionDescriptor.versionType=commit&includeContent=true&api-version=7.0`; + const fileUrl = buildGitApiUrl(repositoryName, 'items', { + path: `/${normalizedPath}`, + 'versionDescriptor.version': commitId, + 'versionDescriptor.versionType': 'commit', + includeContent: 'true' + }); const response = await managedAuth.makeAuthenticatedRequest(fileUrl); @@ -461,7 +494,7 @@ export async function getRepositoryInfo(repositoryName?: string): Promise<{ id: throw new AzureDevOpsError('Repository name not found. Set AdoRepositoryName environment variable or pass repositoryName parameter.'); } - const repoUrl = `${config.organizationUrl}${config.projectName}/_apis/git/repositories/${encodeURIComponent(repoName)}?api-version=7.0`; + const repoUrl = buildGitApiUrl(repoName); const response = await managedAuth.makeAuthenticatedRequest(repoUrl); if (!response.ok) { From 78939a0842661eb33862c41f325a392961642bfd Mon Sep 17 00:00:00 2001 From: Bircck <55695195+Bircck@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:37:12 +0200 Subject: [PATCH 2/2] fix(website): fetch the URL the guard checked, not the raw string assertConfiguredTarget parsed the URL to validate it but the raw string was still handed to fetch, so what went out was never quite what was approved. Returning the parsed URL and fetching that closes the gap and sends the normalized form. --- .../api/auth/azuredevops/ManagedIdentityAuthService.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts b/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts index 7ecb69fe..c1939cc5 100644 --- a/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts +++ b/Website/app/api/auth/azuredevops/ManagedIdentityAuthService.ts @@ -53,7 +53,7 @@ class ManagedIdentityAuth { * before the managed-identity token is attached and sent. `new URL()` * resolves `..` first, so traversal is compared after normalization. */ - private assertConfiguredTarget(url: string): void { + private assertConfiguredTarget(url: string): URL { const target = new URL(url); const allowed = new URL(`${this.config.organizationUrl}${this.config.projectName}/`); @@ -63,10 +63,14 @@ class ManagedIdentityAuth { `only ${allowed.origin}${allowed.pathname} is configured.` ); } + + return target; } async makeAuthenticatedRequest(url: string, options: RequestInit = {}): Promise { - this.assertConfiguredTarget(url); + // Send the URL that was checked, not the string it was parsed from, so + // the request cannot differ from what the guard approved. + const target = this.assertConfiguredTarget(url); // Use PAT for local development, Managed Identity for production const pat = process.env.ADO_PAT; @@ -92,7 +96,7 @@ class ManagedIdentityAuth { }; } - return fetch(url, { + return fetch(target, { ...options, headers: { ...options.headers,