Conversation
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes CodeQL alert #3 —
js/request-forgery, critical, open onmainsince 2026-08-20.Independent of #104 and of #103; touches no workflow and no shared component. Safe to merge last.
The problem
GET /api/diagram/versionreadrepositoryNamestraight off the query string:and
AzureDevOpsServiceinterpolated it raw into the path:`${config.organizationUrl}${config.projectName}/_apis/git/repositories/${repositoryName}/items?path=/${normalizedPath}&...`The host always comes from
ADO_ORGANIZATION_URL, so nobody can point this at an arbitrary server. But?repositoryName=../../../OtherProject/_apis/git/repositories/Secrettraverses to a different endpoint inside the organization — and that request goes out with the managed-identity token attached.filePathandcommitIdwere likewise dropped unencoded into the query string, where an&could append parameters the caller never wrote.The fix
Three changes. None alters behaviour for valid input.
1.
version/route.tstakes the repository from configuration. Every sibling route (list,load,save,versions,export-png,repository-info) already readsprocess.env.ADO_REPOSITORY_NAME; this one route was the outlier. Nothing calls it with the parameter — in fact nothing calls this endpoint at all, only the plural/api/diagram/versions.2.
buildGitApiUrlbuilds all eight Git REST URLs. The repository path segment goes throughencodeURIComponent, the query throughURLSearchParams.getRepositoryInfoalready encoded its segment; this applies the same treatment everywhere and removes five now-unusedmanagedAuth.getConfig()calls.3.
makeAuthenticatedRequestrefuses to leave the configured org/project. The single choke point through which every call passes, so the token cannot be attached to a URL outside{organizationUrl}{projectName}/.new URL()resolves..before the comparison, so traversal is caught after normalization rather than by string matching. The guard returns the parsed URL andfetchis given that, so what goes out is exactly what was approved.Verification
Each of the eight constructed URLs was checked to decode to the same origin, path and query as the string it replaced:
Percent-encoding in a query value is decoded server-side, so the request Azure DevOps receives is byte-for-byte the same as today. With hostile input the traversal now stays in one segment (
..%2F..%2F..%2FOther%2F…) and the injected&becomes%26.npm run lintandnpm run buildpass locally.