From af35bd94441b16a8fbfe47061059b45f956ca8d0 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 30 Sep 2024 23:51:50 +1300 Subject: [PATCH 01/36] Add more logging --- .../azure-devops/AzureDevOpsWebApiClient.ts | 115 +++++++++++------- 1 file changed, 71 insertions(+), 44 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 8a4de107..9105db20 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -133,7 +133,38 @@ export class AzureDevOpsWebApiClient { const userId = await this.getUserId(); const git = await this.connection.getGitApi(); - // Create the source branch and commit the file changes + // Map the list of the pull request reviewer ids + // NOTE: Azure DevOps does not have a concept of assignees, only reviewers. + // We treat assignees as required reviewers and all other reviewers as optional. + const allReviewers: IdentityRefWithVote[] = []; + if (pr.assignees?.length > 0) { + for (const assignee of pr.assignees) { + const identityId = isGuid(assignee) ? assignee : await this.getUserId(assignee); + if (identityId) { + allReviewers.push({ + id: identityId, + isRequired: true, + isFlagged: true, + }); + } else { + warning(` ! Unable to resolve assignee identity '${assignee}'`); + } + } + } + if (pr.reviewers?.length > 0) { + for (const reviewer of pr.reviewers) { + const identityId = isGuid(reviewer) ? reviewer : await this.getUserId(reviewer); + if (identityId) { + allReviewers.push({ + id: identityId, + }); + } else { + warning(` ! Unable to resolve reviewer identity '${reviewer}'`); + } + } + } + + // Create the source branch and push a commit with the dependency file changes console.info(` - Pushing ${pr.changes.length} change(s) to branch '${pr.source.branch}'...`); const push = await git.createPush( { @@ -165,37 +196,10 @@ export class AzureDevOpsWebApiClient { pr.repository, pr.project, ); - - // Build the list of the pull request reviewers - // NOTE: Azure DevOps does not have a concept of assignees, only reviewers. - // We treat assignees as required reviewers and all other reviewers as optional. - const allReviewers: IdentityRefWithVote[] = []; - if (pr.assignees?.length > 0) { - for (const assignee of pr.assignees) { - const identityId = isGuid(assignee) ? assignee : await this.getUserId(assignee); - if (identityId) { - allReviewers.push({ - id: identityId, - isRequired: true, - isFlagged: true, - }); - } else { - warning(` - Unable to resolve assignee identity '${assignee}'`); - } - } - } - if (pr.reviewers?.length > 0) { - for (const reviewer of pr.reviewers) { - const identityId = isGuid(reviewer) ? reviewer : await this.getUserId(reviewer); - if (identityId) { - allReviewers.push({ - id: identityId, - }); - } else { - warning(` - Unable to resolve reviewer identity '${reviewer}'`); - } - } + if (!push?.commits?.length) { + throw new Error('Failed to push changes to source branch, no commits were created'); } + console.log(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); @@ -218,11 +222,15 @@ export class AzureDevOpsWebApiClient { pr.project, true, ); + if (!pullRequest?.pullRequestId) { + throw new Error('Failed to create pull request, no pull request id was returned'); + } + console.log(` - Created pull request: #${pullRequest.pullRequestId}.`); // Add the pull request properties if (pr.properties?.length > 0) { console.info(` - Adding dependency metadata to pull request properties...`); - await git.updatePullRequestProperties( + const newProperties = await git.updatePullRequestProperties( null, pr.properties.map((property) => { return { @@ -235,6 +243,9 @@ export class AzureDevOpsWebApiClient { pullRequest.pullRequestId, pr.project, ); + if (!newProperties?.count) { + throw new Error('Failed to add dependency metadata properties to pull request'); + } } // TODO: Upload the pull request description as a 'changes.md' file attachment? @@ -243,8 +254,8 @@ export class AzureDevOpsWebApiClient { // Set the pull request auto-complete status if (pr.autoComplete) { - console.info(` - Setting auto-complete...`); - await git.updatePullRequest( + console.info(` - Updating auto-complete options...`); + const updatedPullRequest = await git.updatePullRequest( { autoCompleteSetBy: { id: userId, @@ -261,9 +272,12 @@ export class AzureDevOpsWebApiClient { pullRequest.pullRequestId, pr.project, ); + if (!updatedPullRequest || updatedPullRequest.autoCompleteSetBy?.id !== userId) { + throw new Error('Failed to set auto-complete on pull request'); + } } - console.info(` - Pull request #${pullRequest.pullRequestId} was created successfully.`); + console.log(` - Pull request was created successfully.`); return pullRequest.pullRequestId; } catch (e) { error(`Failed to create pull request: ${e}`); @@ -346,8 +360,12 @@ export class AzureDevOpsWebApiClient { options.repository, options.project, ); + if (!push?.commits?.length) { + throw new Error('Failed to push changes to source branch, no commits were created'); + } + console.info(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); - console.info(` - Pull request #${options.pullRequestId} was updated successfully.`); + console.info(` - Pull request was updated successfully.`); return true; } catch (e) { error(`Failed to update pull request: ${e}`); @@ -373,7 +391,7 @@ export class AzureDevOpsWebApiClient { // Approve the pull request console.info(` - Creating reviewer vote on pull request...`); - await git.createPullRequestReviewer( + const userVote = await git.createPullRequestReviewer( { vote: 10, // 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected isReapprove: true, @@ -383,8 +401,11 @@ export class AzureDevOpsWebApiClient { userId, options.project, ); + if (userVote?.vote != 10) { + throw new Error('Failed to approve pull request, vote was not recorded'); + } - console.info(` - Pull request #${options.pullRequestId} was approved.`); + console.info(` - Pull request was approved successfully.`); } catch (e) { error(`Failed to approve pull request: ${e}`); console.debug(e); // Dump the error stack trace to help with debugging @@ -411,8 +432,8 @@ export class AzureDevOpsWebApiClient { // Add a comment to the pull request, if supplied if (options.comment) { - console.info(` - Adding comment to pull request...`); - await git.createThread( + console.info(` - Adding abandonment reason comment to pull request...`); + const thread = await git.createThread( { status: CommentThreadStatus.Closed, comments: [ @@ -429,11 +450,14 @@ export class AzureDevOpsWebApiClient { options.pullRequestId, options.project, ); + if (!thread?.id) { + throw new Error('Failed to add comment to pull request, thread was not created'); + } } // Close the pull request console.info(` - Abandoning pull request...`); - const pullRequest = await git.updatePullRequest( + const abandonedPullRequest = await git.updatePullRequest( { status: PullRequestStatus.Abandoned, closedBy: { @@ -444,14 +468,17 @@ export class AzureDevOpsWebApiClient { options.pullRequestId, options.project, ); + if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { + throw new Error('Failed to close pull request, status was not updated'); + } // Delete the source branch if required if (options.deleteSourceBranch) { console.info(` - Deleting source branch...`); await git.updateRef( { - name: `refs/heads/${pullRequest.sourceRefName}`, - oldObjectId: pullRequest.lastMergeSourceCommit.commitId, + name: `refs/heads/${abandonedPullRequest.sourceRefName}`, + oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, newObjectId: '0000000000000000000000000000000000000000', isLocked: false, }, @@ -461,7 +488,7 @@ export class AzureDevOpsWebApiClient { ); } - console.info(` - Pull request #${options.pullRequestId} was closed successfully.`); + console.info(` - Pull request was closed successfully.`); return true; } catch (e) { error(`Failed to close pull request: ${e}`); From 868b20ed77b421212097adcf6de606f535cf7dc3 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 1 Oct 2024 18:15:56 +1300 Subject: [PATCH 02/36] Clean up logging --- .../utils/azure-devops/AzureDevOpsWebApiClient.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 9105db20..aef49a2c 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -165,7 +165,7 @@ export class AzureDevOpsWebApiClient { } // Create the source branch and push a commit with the dependency file changes - console.info(` - Pushing ${pr.changes.length} change(s) to branch '${pr.source.branch}'...`); + console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pr.source.branch}'...`); const push = await git.createPush( { refUpdates: [ @@ -199,7 +199,7 @@ export class AzureDevOpsWebApiClient { if (!push?.commits?.length) { throw new Error('Failed to push changes to source branch, no commits were created'); } - console.log(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); + console.info(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); @@ -225,7 +225,7 @@ export class AzureDevOpsWebApiClient { if (!pullRequest?.pullRequestId) { throw new Error('Failed to create pull request, no pull request id was returned'); } - console.log(` - Created pull request: #${pullRequest.pullRequestId}.`); + console.info(` - Created pull request: #${pullRequest.pullRequestId}.`); // Add the pull request properties if (pr.properties?.length > 0) { @@ -277,7 +277,7 @@ export class AzureDevOpsWebApiClient { } } - console.log(` - Pull request was created successfully.`); + console.info(` - Pull request was created successfully.`); return pullRequest.pullRequestId; } catch (e) { error(`Failed to create pull request: ${e}`); @@ -327,7 +327,7 @@ export class AzureDevOpsWebApiClient { } // Push changes to the source branch - console.info(` - Pushing ${options.changes.length} change(s) branch '${pullRequest.sourceRefName}'...`); + console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); const push = await git.createPush( { refUpdates: [ From 840d5cd1c4a6660c60a7dd501b5243956a451d13 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Wed, 2 Oct 2024 12:02:00 +1300 Subject: [PATCH 03/36] Use Rest API instead of TypedClient, fixes undefined responses in some environments --- .../azure-devops/AzureDevOpsWebApiClient.ts | 118 ++++++++++++------ 1 file changed, 80 insertions(+), 38 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index aef49a2c..bd567262 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -8,6 +8,7 @@ import { PullRequestStatus, } from 'azure-devops-node-api/interfaces/GitInterfaces'; import { error, warning } from 'azure-pipelines-task-lib/task'; +import { IHttpClientResponse } from 'typed-rest-client/Interfaces'; import { IFileChange } from './interfaces/IFileChange'; import { IPullRequest } from './interfaces/IPullRequest'; import { IPullRequestProperties } from './interfaces/IPullRequestProperties'; @@ -88,21 +89,21 @@ export class AzureDevOpsWebApiClient { ): Promise { try { const git = await this.connection.getGitApi(); - const pullRequests = await git.getPullRequests( - repository, - { - creatorId: isGuid(creator) ? creator : await this.getUserId(creator), - status: PullRequestStatus.Active, - }, - project, - ); + const pullRequests = (await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests?searchCriteria.creatorId=${isGuid(creator) ? creator : await this.getUserId(creator)}&searchCriteria.status${PullRequestStatus.Active}&api-version=7.1`, + ))?.value || []; if (!pullRequests || pullRequests.length === 0) { return []; } return await Promise.all( pullRequests.map(async (pr) => { - const properties = (await git.getPullRequestProperties(repository, pr.pullRequestId, project))?.value || {}; + const properties = + ( + await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties?api-version=7.1`, + ) + )?.value || {}; return { id: pr.pullRequestId, properties: @@ -166,7 +167,8 @@ export class AzureDevOpsWebApiClient { // Create the source branch and push a commit with the dependency file changes console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pr.source.branch}'...`); - const push = await git.createPush( + const push = await this.restApiPost( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes?api-version=7.1`, { refUpdates: [ { @@ -193,8 +195,6 @@ export class AzureDevOpsWebApiClient { }, ], }, - pr.repository, - pr.project, ); if (!push?.commits?.length) { throw new Error('Failed to push changes to source branch, no commits were created'); @@ -203,7 +203,8 @@ export class AzureDevOpsWebApiClient { // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); - const pullRequest = await git.createPullRequest( + const pullRequest = await this.restApiPost( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests?api-version=7.1`, { sourceRefName: `refs/heads/${pr.source.branch}`, targetRefName: `refs/heads/${pr.target.branch}`, @@ -218,9 +219,6 @@ export class AzureDevOpsWebApiClient { }), isDraft: false, // TODO: Add config for this? }, - pr.repository, - pr.project, - true, ); if (!pullRequest?.pullRequestId) { throw new Error('Failed to create pull request, no pull request id was returned'); @@ -230,8 +228,8 @@ export class AzureDevOpsWebApiClient { // Add the pull request properties if (pr.properties?.length > 0) { console.info(` - Adding dependency metadata to pull request properties...`); - const newProperties = await git.updatePullRequestProperties( - null, + const newProperties = await this.restApiPatch( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}/properties?api-version=7.1`, pr.properties.map((property) => { return { op: 'add', @@ -239,9 +237,7 @@ export class AzureDevOpsWebApiClient { value: property.value, }; }), - pr.repository, - pullRequest.pullRequestId, - pr.project, + 'application/json-patch+json', ); if (!newProperties?.count) { throw new Error('Failed to add dependency metadata properties to pull request'); @@ -255,7 +251,8 @@ export class AzureDevOpsWebApiClient { // Set the pull request auto-complete status if (pr.autoComplete) { console.info(` - Updating auto-complete options...`); - const updatedPullRequest = await git.updatePullRequest( + const updatedPullRequest = await this.restApiPatch( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}?api-version=7.1`, { autoCompleteSetBy: { id: userId, @@ -268,9 +265,6 @@ export class AzureDevOpsWebApiClient { transitionWorkItems: false, }, }, - pr.repository, - pullRequest.pullRequestId, - pr.project, ); if (!updatedPullRequest || updatedPullRequest.autoCompleteSetBy?.id !== userId) { throw new Error('Failed to set auto-complete on pull request'); @@ -297,6 +291,7 @@ export class AzureDevOpsWebApiClient { pullRequestId: number; changes: IFileChange[]; skipIfCommitsFromUsersOtherThan?: string; + skipIfNotBehindTargetBranch?: boolean; skipIfNoConflicts?: boolean; }): Promise { console.info(`Updating pull request #${options.pullRequestId}...`); @@ -310,25 +305,35 @@ export class AzureDevOpsWebApiClient { throw new Error(`Pull request #${options.pullRequestId} not found`); } + // Skip if the pull request has been modified by another user + if (options.skipIfCommitsFromUsersOtherThan) { + const commits = await git.getPullRequestCommits(options.repository, options.pullRequestId, options.project); + if (commits.some((c) => c.author?.email !== options.skipIfCommitsFromUsersOtherThan)) { + console.info(` - Skipping update as pull request has been modified by another user.`); + return true; + } + } + + // Skip if the source branch is not behind the target branch + if (options.skipIfNotBehindTargetBranch) { + // TODO: https://learn.microsoft.com/en-us/rest/api/azure/devops/git/stats/get?view=azure-devops-rest-7.1&tabs=HTTP#examples + //const stats = await this.restApiGet(...) + //if (stats.behindCount > 0) { + // console.info(` - Skipping update as source branch is not behind target branch.`); + // return true; + //} + } + // Skip if no merge conflicts if (options.skipIfNoConflicts && pullRequest.mergeStatus !== PullRequestAsyncStatus.Conflicts) { console.info(` - Skipping update as pull request has no merge conflicts.`); return true; } - // Skip if the pull request has been modified by another user - const commits = await git.getPullRequestCommits(options.repository, options.pullRequestId, options.project); - if ( - options.skipIfCommitsFromUsersOtherThan && - commits.some((c) => c.author?.email !== options.skipIfCommitsFromUsersOtherThan) - ) { - console.info(` - Skipping update as pull request has been modified by another user.`); - return true; - } - // Push changes to the source branch console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); - const push = await git.createPush( + const push = await this.restApiPost( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes?api-version=7.1`, { refUpdates: [ { @@ -357,8 +362,6 @@ export class AzureDevOpsWebApiClient { }, ], }, - options.repository, - options.project, ); if (!push?.commits?.length) { throw new Error('Failed to push changes to source branch, no commits were created'); @@ -546,6 +549,45 @@ export class AzureDevOpsWebApiClient { console.debug(e); // Dump the error stack trace to help with debugging } } + + private async restApiGet(url: string): Promise { + return await this.restApiRequest(await this.connection.rest.client.get(url, { Accept: 'application/json' })); + } + + private async restApiPost(url: string, data?: any): Promise { + return await this.restApiRequest( + await this.connection.rest.client.post(url, JSON.stringify(data), { 'Content-Type': 'application/json' }), + ); + } + + private async restApiPatch(url: string, data?: any, contentType?: string): Promise { + return await this.restApiRequest( + await this.connection.rest.client.patch(url, JSON.stringify(data), { + 'Content-Type': contentType || 'application/json', + }), + ); + } + + private async restApiRequest(response: IHttpClientResponse): Promise { + try { + const body = JSON.parse(await response.readBody()); + if (body?.errorCode !== undefined && body?.message) { + throw new Error(body.message); + } + return body; + } catch (error) { + var responseStatusCode = error?.response?.statusCode; + if (responseStatusCode === 404) { + return undefined; + } else if (responseStatusCode === 401) { + throw new Error(`No access token has been provided to access '${response.message.url}'`); + } else if (responseStatusCode === 403) { + throw new Error(`The access token provided does not have permissions to access '${response.message.url}'`); + } else { + throw error; + } + } + } } function normalizeDevOpsPath(path: string): string { From 587aea9c3dba7e8a07258f695e8f882b2c2087cd Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 01:44:13 +1300 Subject: [PATCH 04/36] Refactor DevOps client to use rest client instead of typed client --- .../azure-devops/AzureDevOpsWebApiClient.ts | 358 ++++++++++-------- .../resolveAzureDevOpsIdentities.ts | 161 -------- .../DependabotOutputProcessor.ts | 6 +- 3 files changed, 202 insertions(+), 323 deletions(-) delete mode 100644 extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 12f6df75..d9143134 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -12,62 +12,81 @@ import { IHttpClientResponse } from 'typed-rest-client/Interfaces'; import { IFileChange } from './interfaces/IFileChange'; import { IPullRequest } from './interfaces/IPullRequest'; import { IPullRequestProperties } from './interfaces/IPullRequestProperties'; -import { resolveAzureDevOpsIdentities } from './resolveAzureDevOpsIdentities'; /** * Wrapper for DevOps WebApi client with helper methods for easier management of dependabot pull requests */ export class AzureDevOpsWebApiClient { private readonly organisationApiUrl: string; + private readonly identityApiUrl: string; private readonly accessToken: string; private readonly connection: WebApi; - private cachedUserIds: Record; + private authenticatedUserId: string; + private resolvedUserIds: Record; + + public static API_VERSION = '7.1'; constructor(organisationApiUrl: string, accessToken: string) { this.organisationApiUrl = organisationApiUrl; + this.identityApiUrl = getIdentityApiUrl(organisationApiUrl); this.accessToken = accessToken; this.connection = new WebApi(organisationApiUrl, getPersonalAccessTokenHandler(accessToken)); - this.cachedUserIds = {}; + this.resolvedUserIds = {}; } /** - * Get the identity of a user by email address. If no email is provided, the identity of the authenticated user is returned. - * @param email + * Get the identity of the authenticated user. * @returns */ - public async getUserId(email?: string): Promise { - // If no email is provided, resolve to the authenticated user - if (!email) { - this.cachedUserIds[this.accessToken] ||= (await this.connection.connect())?.authenticatedUser?.id || ''; - return this.cachedUserIds[this.accessToken]; - } + public async getUserId(): Promise { + this.authenticatedUserId ||= (await this.connection.connect()).authenticatedUser.id; + return this.authenticatedUserId; + } - // Otherwise, do a cached identity lookup of the supplied email address - // TODO: When azure-devops-node-api supports Graph API, use that instead of the REST API - else if (!this.cachedUserIds[email]) { - const identities = await resolveAzureDevOpsIdentities(new URL(this.organisationApiUrl), [email]); - identities.forEach((i) => (this.cachedUserIds[i.input] ||= i.id)); + /** + * Get the identity id from a user name, email, or group name. + * Requires scope "Identity (Read)" (vso.identity). + * @param nameEmailOrGroup + * @returns + */ + public async resolveIdentityId(nameEmailOrGroup?: string): Promise { + if (this.resolvedUserIds[nameEmailOrGroup]) { + return this.resolvedUserIds[nameEmailOrGroup]; + } + try { + const identities = await this.restApiGet(`${this.identityApiUrl}/_apis/identities`, { + searchFilter: 'General', + filterValue: nameEmailOrGroup, + queryMembership: 'None', + }); + if (!identities?.value || identities.value.length === 0) { + return undefined; + } + this.resolvedUserIds[nameEmailOrGroup] = identities.value.map((i) => i.id) || []; + return this.resolvedUserIds[nameEmailOrGroup]; + } catch (e) { + error(`Failed to resolve user id: ${e}`); + console.debug(e); // Dump the error stack trace to help with debugging + return undefined; } - - return this.cachedUserIds[email]; } /** - * Get the default branch for a repository + * Get the default branch for a repository. + * Requires scope "Code (Read)" (vso.code). * @param project * @param repository * @returns */ public async getDefaultBranch(project: string, repository: string): Promise { try { - const git = await this.connection.getGitApi(); - const repo = await git.getRepository(repository, project); + const repo = await this.restApiGet(`${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}`); + console.log(repo); if (!repo) { throw new Error(`Repository '${project}/${repository}' not found`); } - // Strip reference prefix from the branch name, the caller doesn't need to know this - return repo.defaultBranch?.replace(/^refs\/heads\//i, ''); + return normalizeBranchName(repo.defaultBranch); } catch (e) { error(`Failed to get default branch for '${project}/${repository}': ${e}`); console.debug(e); // Dump the error stack trace to help with debugging @@ -76,7 +95,8 @@ export class AzureDevOpsWebApiClient { } /** - * Get the properties for all active pull request created by the supplied user + * Get the properties for all active pull request created by the supplied user. + * Requires scope "Code (Read)" (vso.code). * @param project * @param repository * @param creator @@ -88,10 +108,16 @@ export class AzureDevOpsWebApiClient { creator: string, ): Promise { try { - const git = await this.connection.getGitApi(); - const pullRequests = (await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests?searchCriteria.creatorId=${isGuid(creator) ? creator : await this.getUserId(creator)}&searchCriteria.status${PullRequestStatus.Active}&api-version=7.1`, - ))?.value || []; + const pullRequests = + ( + await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests`, + { + 'searchCriteria.creatorId': isGuid(creator) ? creator : await this.getUserId(), + 'searchCriteria.status': 'Active', + }, + ) + )?.value || []; if (!pullRequests || pullRequests.length === 0) { return []; } @@ -101,7 +127,7 @@ export class AzureDevOpsWebApiClient { const properties = ( await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties?api-version=7.1`, + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties`, ) )?.value || {}; return { @@ -124,7 +150,9 @@ export class AzureDevOpsWebApiClient { } /** - * Create a new pull request + * Create a new pull request. + * Requires scope "Code (Write)" (vso.code_write). + * Requires scope "Identity (Read)" (vso.identity), if assignees or reviewers are specified. * @param pr * @returns */ @@ -132,7 +160,6 @@ export class AzureDevOpsWebApiClient { console.info(`Creating pull request '${pr.title}'...`); try { const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); // Map the list of the pull request reviewer ids // NOTE: Azure DevOps does not have a concept of assignees, only reviewers. @@ -140,7 +167,7 @@ export class AzureDevOpsWebApiClient { const allReviewers: IdentityRefWithVote[] = []; if (pr.assignees?.length > 0) { for (const assignee of pr.assignees) { - const identityId = isGuid(assignee) ? assignee : await this.getUserId(assignee); + const identityId = isGuid(assignee) ? assignee : await this.resolveIdentityId(assignee); if (identityId) { allReviewers.push({ id: identityId, @@ -154,7 +181,7 @@ export class AzureDevOpsWebApiClient { } if (pr.reviewers?.length > 0) { for (const reviewer of pr.reviewers) { - const identityId = isGuid(reviewer) ? reviewer : await this.getUserId(reviewer); + const identityId = isGuid(reviewer) ? reviewer : await this.resolveIdentityId(reviewer); if (identityId) { allReviewers.push({ id: identityId, @@ -168,7 +195,7 @@ export class AzureDevOpsWebApiClient { // Create the source branch and push a commit with the dependency file changes console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pr.source.branch}'...`); const push = await this.restApiPost( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes`, { refUpdates: [ { @@ -184,7 +211,7 @@ export class AzureDevOpsWebApiClient { return { changeType: change.changeType, item: { - path: normalizeDevOpsPath(change.path), + path: normalizeFilePath(change.path), }, newContent: { content: Buffer.from(change.content, change.encoding).toString('base64'), @@ -204,7 +231,7 @@ export class AzureDevOpsWebApiClient { // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); const pullRequest = await this.restApiPost( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests`, { sourceRefName: `refs/heads/${pr.source.branch}`, targetRefName: `refs/heads/${pr.target.branch}`, @@ -229,7 +256,7 @@ export class AzureDevOpsWebApiClient { if (pr.properties?.length > 0) { console.info(` - Adding dependency metadata to pull request properties...`); const newProperties = await this.restApiPatch( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}/properties?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}/properties`, pr.properties.map((property) => { return { op: 'add', @@ -252,7 +279,7 @@ export class AzureDevOpsWebApiClient { if (pr.autoComplete) { console.info(` - Updating auto-complete options...`); const updatedPullRequest = await this.restApiPatch( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}`, { autoCompleteSetBy: { id: userId, @@ -281,7 +308,8 @@ export class AzureDevOpsWebApiClient { } /** - * Update a pull request + * Update a pull request. + * Requires scope "Code (Read & Write)" (vso.code, vso.code_write). * @param options * @returns */ @@ -290,25 +318,32 @@ export class AzureDevOpsWebApiClient { repository: string; pullRequestId: number; changes: IFileChange[]; - skipIfCommitsFromUsersOtherThan?: string; + skipIfDraft?: boolean; + skipIfCommitsFromAuthorsOtherThan?: string; skipIfNotBehindTargetBranch?: boolean; - skipIfNoConflicts?: boolean; }): Promise { console.info(`Updating pull request #${options.pullRequestId}...`); try { - const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); - // Get the pull request details - const pullRequest = await git.getPullRequest(options.repository, options.pullRequestId, options.project); + const pullRequest = await this.restApiGet( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, + ); if (!pullRequest) { throw new Error(`Pull request #${options.pullRequestId} not found`); } - // Skip if the pull request has been modified by another user - if (options.skipIfCommitsFromUsersOtherThan) { - const commits = await git.getPullRequestCommits(options.repository, options.pullRequestId, options.project); - if (commits.some((c) => c.author?.email !== options.skipIfCommitsFromUsersOtherThan)) { + // Skip if the pull request is a draft + if (options.skipIfDraft && pullRequest.isDraft) { + console.info(` - Skipping update as pull request is currently marked as a draft.`); + return true; + } + + // Skip if the pull request has been modified by another author + if (options.skipIfCommitsFromAuthorsOtherThan) { + const commits = await this.restApiGet( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/commits`, + ); + if (commits?.value?.some((c) => c.author?.email !== options.skipIfCommitsFromAuthorsOtherThan)) { console.info(` - Skipping update as pull request has been modified by another user.`); return true; } @@ -316,24 +351,22 @@ export class AzureDevOpsWebApiClient { // Skip if the source branch is not behind the target branch if (options.skipIfNotBehindTargetBranch) { - // TODO: https://learn.microsoft.com/en-us/rest/api/azure/devops/git/stats/get?view=azure-devops-rest-7.1&tabs=HTTP#examples - //const stats = await this.restApiGet(...) - //if (stats.behindCount > 0) { - // console.info(` - Skipping update as source branch is not behind target branch.`); - // return true; - //} - } - - // Skip if no merge conflicts - if (options.skipIfNoConflicts && pullRequest.mergeStatus !== PullRequestAsyncStatus.Conflicts) { - console.info(` - Skipping update as pull request has no merge conflicts.`); - return true; + const stats = await this.restApiGet( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/stats/branches`, + { + name: normalizeBranchName(pullRequest.sourceRefName), + }, + ); + if (stats?.behindCount === 0) { + console.info(` - Skipping update as source branch is not behind target branch.`); + return true; + } } // Push changes to the source branch console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); const push = await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes?api-version=7.1`, + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes`, { refUpdates: [ { @@ -351,7 +384,7 @@ export class AzureDevOpsWebApiClient { return { changeType: change.changeType, item: { - path: normalizeDevOpsPath(change.path), + path: normalizeFilePath(change.path), }, newContent: { content: Buffer.from(change.content, change.encoding).toString('base64'), @@ -378,7 +411,8 @@ export class AzureDevOpsWebApiClient { } /** - * Approve a pull request + * Approve a pull request. + * Requires scope "Code (Write)" (vso.code_write). * @param options * @returns */ @@ -389,20 +423,15 @@ export class AzureDevOpsWebApiClient { }): Promise { console.info(`Approving pull request #${options.pullRequestId}...`); try { - const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); - // Approve the pull request console.info(` - Creating reviewer vote on pull request...`); - const userVote = await git.createPullRequestReviewer( + const userId = await this.getUserId(); + const userVote = await this.restApiPut( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/reviewers/${userId}`, { vote: 10, // 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected isReapprove: true, }, - options.repository, - options.pullRequestId, - userId, - options.project, ); if (userVote?.vote != 10) { throw new Error('Failed to approve pull request, vote was not recorded'); @@ -417,7 +446,8 @@ export class AzureDevOpsWebApiClient { } /** - * Close a pull request + * Close a pull request. + * Requires scope "Code (Write)" (vso.code_write). * @param options * @returns */ @@ -431,12 +461,12 @@ export class AzureDevOpsWebApiClient { console.info(`Closing pull request #${options.pullRequestId}...`); try { const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); // Add a comment to the pull request, if supplied if (options.comment) { console.info(` - Adding abandonment reason comment to pull request...`); - const thread = await git.createThread( + const thread = await this.restApiPost( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/threads`, { status: CommentThreadStatus.Closed, comments: [ @@ -449,9 +479,6 @@ export class AzureDevOpsWebApiClient { }, ], }, - options.repository, - options.pullRequestId, - options.project, ); if (!thread?.id) { throw new Error('Failed to add comment to pull request, thread was not created'); @@ -460,16 +487,14 @@ export class AzureDevOpsWebApiClient { // Close the pull request console.info(` - Abandoning pull request...`); - const abandonedPullRequest = await git.updatePullRequest( + const abandonedPullRequest = await this.restApiPatch( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, { status: PullRequestStatus.Abandoned, closedBy: { id: userId, }, }, - options.repository, - options.pullRequestId, - options.project, ); if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { throw new Error('Failed to close pull request, status was not updated'); @@ -478,16 +503,14 @@ export class AzureDevOpsWebApiClient { // Delete the source branch if required if (options.deleteSourceBranch) { console.info(` - Deleting source branch...`); - await git.updateRef( + await this.restApiPost( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/refs`, { - name: `refs/heads/${abandonedPullRequest.sourceRefName}`, + name: abandonedPullRequest.sourceRefName, oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, newObjectId: '0000000000000000000000000000000000000000', isLocked: false, }, - options.repository, - '', - options.project, ); } @@ -500,97 +523,96 @@ export class AzureDevOpsWebApiClient { } } - /** - * Get project properties - * @param projectId - * @param valueBuilder - * @returns - */ - public async getProjectProperties(projectId: string): Promise | undefined> { - try { - const core = await this.connection.getCoreApi(); - const properties = await core.getProjectProperties(projectId); - return properties?.map((p) => ({ [p.name]: p.value }))?.reduce((a, b) => ({ ...a, ...b }), {}); - } catch (e) { - error(`Failed to get project properties: ${e}`); - console.debug(e); // Dump the error stack trace to help with debugging - return undefined; - } - } - - /** - * Update a project property - * @param project - * @param name - * @param valueBuilder - * @returns - */ - public async updateProjectProperty( - projectId: string, - name: string, - valueBuilder: (existingValue: string) => string, - ): Promise { - try { - // Get the existing project property value - const core = await this.connection.getCoreApi(); - const properties = await core.getProjectProperties(projectId); - const propertyValue = properties?.find((p) => p.name === name)?.value; - - // Update the project property - await core.setProjectProperties(undefined, projectId, [ - { - op: 'add', - path: '/' + name, - value: valueBuilder(propertyValue || ''), - }, - ]); - } catch (e) { - error(`Failed to update project property '${name}': ${e}`); - console.debug(e); // Dump the error stack trace to help with debugging - } + private async restApiGet( + url: string, + params?: Record, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const queryString = Object.keys(params || {}) + .map((key) => `${key}=${params[key]}`) + .join('&'); + const fullUrl = `${url}?api-version=${apiVersion}${queryString ? `&${queryString}` : ''}`; + return await this.restApiRequest('GET', url, () => + this.connection.rest.client.get(fullUrl, { + Accept: 'application/json', + }), + ); } - private async restApiGet(url: string): Promise { - return await this.restApiRequest(await this.connection.rest.client.get(url, { Accept: 'application/json' })); + private async restApiPost( + url: string, + data?: any, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const fullUrl = `${url}?api-version=${apiVersion}`; + return await this.restApiRequest('POST', url, () => + this.connection.rest.client.post(fullUrl, JSON.stringify(data), { + 'Content-Type': 'application/json', + }), + ); } - private async restApiPost(url: string, data?: any): Promise { - return await this.restApiRequest( - await this.connection.rest.client.post(url, JSON.stringify(data), { 'Content-Type': 'application/json' }), + private async restApiPut( + url: string, + data?: any, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const fullUrl = `${url}?api-version=${apiVersion}`; + return await this.restApiRequest('PUT', url, () => + this.connection.rest.client.put(fullUrl, JSON.stringify(data), { + 'Content-Type': 'application/json', + }), ); } - private async restApiPatch(url: string, data?: any, contentType?: string): Promise { - return await this.restApiRequest( - await this.connection.rest.client.patch(url, JSON.stringify(data), { + private async restApiPatch( + url: string, + data?: any, + contentType?: string, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const fullUrl = `${url}?api-version=${apiVersion}`; + return await this.restApiRequest('PATCH', url, () => + this.connection.rest.client.patch(fullUrl, JSON.stringify(data), { 'Content-Type': contentType || 'application/json', }), ); } - private async restApiRequest(response: IHttpClientResponse): Promise { + private async restApiRequest( + method: string, + url: string, + request: () => Promise, + ): Promise { + console.debug(`🌎 🠊 [${method}] ${url}`); + const response = await request(); + console.debug(`🌎 🠈 [${response.message.statusCode}] ${response.message.statusMessage}`); + if (response.message.statusCode === 401) { + throw new Error(`No access token has been provided to access '${url}'`); + } + if (response.message.statusCode === 403) { + throw new Error(`The access token provided does not have permissions to access '${url}'`); + } + if (response.message.statusCode < 200 || response.message.statusCode > 299) { + throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); + } try { - const body = JSON.parse(await response.readBody()); - if (body?.errorCode !== undefined && body?.message) { - throw new Error(body.message); - } - return body; - } catch (error) { - var responseStatusCode = error?.response?.statusCode; - if (responseStatusCode === 404) { - return undefined; - } else if (responseStatusCode === 401) { - throw new Error(`No access token has been provided to access '${response.message.url}'`); - } else if (responseStatusCode === 403) { - throw new Error(`The access token provided does not have permissions to access '${response.message.url}'`); - } else { - throw error; + const responseBodyJson = JSON.parse(await response.readBody()); + if (responseBodyJson?.errorCode !== undefined && responseBodyJson?.message) { + // .NET API error response + throw new Error(responseBodyJson.message); } + + return responseBodyJson; + } catch (e) { + // JSON parsing failed, log the error and return undefined + console.debug(response.message); + throw new Error(`Failed to parse response body as JSON: ${e}`); } } } -function normalizeDevOpsPath(path: string): string { +function normalizeFilePath(path: string): string { // Convert backslashes to forward slashes, convert './' => '/' and ensure the path starts with a forward slash if it doesn't already, this is how DevOps paths are formatted return path .replace(/\\/g, '/') @@ -598,6 +620,11 @@ function normalizeDevOpsPath(path: string): string { .replace(/^([^/])/, '/$1'); } +function normalizeBranchName(branch: string): string { + // Strip the 'refs/heads/' prefix from the branch name, if present + return branch.replace(/^refs\/heads\//i, ''); +} + function mergeCommitMessage(id: number, title: string, description: string): string { // // The merge commit message should contain the PR number and title for tracking. @@ -625,3 +652,14 @@ function isGuid(guid: string): boolean { const regex = /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/; return regex.test(guid); } + +function getIdentityApiUrl(organisationApiUrl: string): string { + const uri = new URL(organisationApiUrl); + const hostname = uri.hostname.toLowerCase(); + + // If the organisation is hosted on Azure DevOps, use the 'vssps.dev.azure.com' domain + if (hostname === 'dev.azure.com' || hostname.endsWith('.visualstudio.com')) { + uri.host = 'vssps.dev.azure.com'; + } + return uri.toString(); +} diff --git a/extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts b/extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts deleted file mode 100644 index 77e0a617..00000000 --- a/extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts +++ /dev/null @@ -1,161 +0,0 @@ -import axios from 'axios'; -import * as tl from 'azure-pipelines-task-lib/task'; -import extractOrganization from '../extractOrganization'; - -export interface IIdentity { - /** - * The identity id to use for PR reviewer or assignee Id. - */ - id: string; - /** - * Human readable Username. - */ - displayName?: string; - /** - * The provided input to use for searching an identity. - */ - input: string; -} - -/** - * Resolves the given input email addresses to an array of IIdentity information. - * It also handles non email input, which is assumed to be already an identity id - * to pass as reviewer id to an PR. - * - * @param organizationUrl - * @param inputs - * @returns - */ -export async function resolveAzureDevOpsIdentities(organizationUrl: URL, inputs: string[]): Promise { - const result: IIdentity[] = []; - - tl.debug(`Attempting to fetch configuration file via REST API ...`); - for (const input of inputs) { - if (input.indexOf('@') > 0) { - // input is email to look-up - const identityInfo = await querySubject(organizationUrl, input); - if (identityInfo) { - result.push(identityInfo); - } - } else { - // input is already identity id - result.push({ id: input, input: input }); - } - } - return result; -} - -/** - * Returns whether the extension is run in a hosted environment (as opposed to an on-premise environment). - * In Azure DevOps terms, hosted environment is also known as "Azure DevOps Services" and on-premise environment is known as - * "Team Foundation Server" or "Azure DevOps Server". - */ -export function isHostedAzureDevOps(uri: URL): boolean { - const hostname = uri.hostname.toLowerCase(); - return hostname === 'dev.azure.com' || hostname.endsWith('.visualstudio.com'); -} - -function decodeBase64(input: string): string { - return Buffer.from(input, 'base64').toString('utf8'); -} - -function encodeBase64(input: string): string { - return Buffer.from(input, 'utf8').toString('base64'); -} - -function isSuccessStatusCode(statusCode?: number): boolean { - return statusCode >= 200 && statusCode <= 299; -} - -async function querySubject(organizationUrl: URL, email: string): Promise { - if (isHostedAzureDevOps(organizationUrl)) { - const organization: string = extractOrganization(organizationUrl.toString()); - return await querySubjectHosted(organization, email); - } else { - return await querySubjectOnPrem(organizationUrl, email); - } -} - -/** - * Make the HTTP Request for an OnPrem Azure DevOps Server to resolve an email to an IIdentity - * @param organizationUrl - * @param email - * @returns - */ -async function querySubjectOnPrem(organizationUrl: URL, email: string): Promise { - const url = `${organizationUrl}_apis/identities?searchFilter=MailAddress&queryMembership=None&filterValue=${email}`; - tl.debug(`GET ${url}`); - try { - const response = await axios.get(url, { - headers: { - Authorization: `Basic ${encodeBase64('PAT:' + tl.getVariable('System.AccessToken'))}`, - Accept: 'application/json;api-version=5.0', - }, - }); - - if (isSuccessStatusCode(response.status)) { - return { - id: response.data.value[0]?.id, - displayName: response.data.value[0]?.providerDisplayName, - input: email, - }; - } - } catch (error) { - const responseStatusCode = error?.response?.status; - tl.debug(`HTTP Response Status: ${responseStatusCode}`); - if (responseStatusCode > 400 && responseStatusCode < 500) { - tl.debug(`Access token is ${tl.getVariable('System.AccessToken')?.length > 0 ? 'not' : ''} null or empty.`); - throw new Error(`The access token provided is empty or does not have permissions to access '${url}'`); - } else { - throw error; - } - } -} - -/** - * * Make the HTTP Request for a hosted Azure DevOps Service, to resolve an email to an IIdentity - * @param organization - * @param email - * @returns - */ -async function querySubjectHosted(organization: string, email: string): Promise { - // make HTTP request - const url = `https://vssps.dev.azure.com/${organization}/_apis/graph/subjectquery`; - tl.debug(`GET ${url}`); - try { - const response = await axios.post(url, { - headers: { - 'Authorization': `Basic ${encodeBase64('PAT:' + tl.getVariable('System.AccessToken'))}`, - 'Accept': 'application/json;api-version=6.0-preview.1', - 'Content-Type': 'application/json', - }, - data: { - query: email, - subjectKind: ['User'], - }, - }); - - tl.debug(`Got Http Response: ${response.status}`); - - if (!isSuccessStatusCode(response.status) || response.data.value.length === 0) { - throw new Error('Failed to resolve given email in organization'); - } - - const descriptor: string = response.data.value[0]?.descriptor || ''; - const id = decodeBase64(descriptor.substring(descriptor.indexOf('.') + 1)); - return { - id: id, - displayName: response.data.value[0]?.displayName, - input: email, - }; - } catch (error) { - const responseStatusCode = error?.response?.status; - tl.debug(`HTTP Response Status: ${responseStatusCode}`); - if (responseStatusCode > 400 && responseStatusCode < 500) { - tl.debug(`Access token is ${tl.getVariable('System.AccessToken')?.length > 0 ? 'not' : ''} null or empty.`); - throw new Error(`The access token provided is empty or does not have permissions to access '${url}'`); - } else { - throw error; - } - } -} diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index 97ab9d00..dc8428b5 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -180,9 +180,11 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfCommitsFromUsersOtherThan: + skipIfDraft: true, // TODO: Add config for this? + // TODO: Add config for this? + skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNoConflicts: true, + skipIfNotBehindTargetBranch: true, // TODO: Add config for this? }); // Re-approve the pull request, if required From 9561273b7fe1cd155398a0a41e65331512e3909e Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 02:01:57 +1300 Subject: [PATCH 05/36] Clean up --- extension/tasks/dependabotV2/index.ts | 12 +----- .../DependabotOutputProcessor.ts | 39 ++----------------- 2 files changed, 5 insertions(+), 46 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 94c4c87f..c52ad2d5 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -4,7 +4,6 @@ import { DependabotCli } from './utils/dependabot-cli/DependabotCli'; import { DependabotJobBuilder } from './utils/dependabot-cli/DependabotJobBuilder'; import { DependabotOutputProcessor, - parseProjectDependencyListProperty, parsePullRequestProperties, } from './utils/dependabot-cli/DependabotOutputProcessor'; import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; @@ -84,15 +83,6 @@ async function run() { for (const update of updates) { const updateId = updates.indexOf(update).toString(); - // Parse the last dependency list snapshot (if any) from the project properties. - // This is required when doing a security-only update as dependabot requires the list of vulnerable dependencies to be updated. - // Automatic discovery of vulnerable dependencies during a security-only update is not currently supported by dependabot-updater. - const dependencyList = parseProjectDependencyListProperty( - await prAuthorClient.getProjectProperties(taskInputs.projectId), - taskInputs.repository, - update['package-ecosystem'], - ); - // Parse the Dependabot metadata for the existing pull requests that are related to this update // Dependabot will use this to determine if we need to create new pull requests or update/close existing ones const existingPullRequests = parsePullRequestProperties(prAuthorActivePullRequests, update['package-ecosystem']); @@ -104,7 +94,7 @@ async function run() { updateId, update, dependabotConfig.registries, - dependencyList?.['dependencies'], + undefined, // TODO: Implement this, required for security-only updates existingPullRequestDependencies, ); const allDependenciesUpdateOutputs = await dependabot.update(allDependenciesJob, dependabotUpdaterOptions); diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index dc8428b5..8fdd2fc8 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -18,10 +18,6 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess private readonly existingPullRequests: IPullRequestProperties[]; private readonly taskInputs: ISharedVariables; - // Custom properties used to store dependabot metadata in projects. - // https://learn.microsoft.com/en-us/rest/api/azure/devops/core/projects/set-project-properties - public static PROJECT_PROPERTY_NAME_DEPENDENCY_LIST = 'Dependabot.DependencyList'; - // Custom properties used to store dependabot metadata in pull requests. // https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-properties public static PR_PROPERTY_NAME_PACKAGE_MANAGER = 'Dependabot.PackageManager'; @@ -58,25 +54,9 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess // See: https://github.com/dependabot/cli/blob/main/internal/model/update.go case 'update_dependency_list': - // Store the dependency list snapshot in project properties, if configured + // Store the dependency list snapshot, if configured if (this.taskInputs.storeDependencyList) { - console.info(`Storing the dependency list snapshot for project '${project}'...`); - await this.prAuthorClient.updateProjectProperty( - this.taskInputs.projectId, - DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST, - function (existingValue: string) { - const repoDependencyLists = JSON.parse(existingValue || '{}'); - repoDependencyLists[repository] = repoDependencyLists[repository] || {}; - repoDependencyLists[repository][update.job['package-manager']] = { - 'dependencies': data['dependencies'], - 'dependency-files': data['dependency_files'], - 'last-updated': new Date().toISOString(), - }; - - return JSON.stringify(repoDependencyLists); - }, - ); - console.info(`Dependency list snapshot was updated for project '${project}'`); + // TODO: Store the dependency list snapshot } return true; @@ -180,11 +160,10 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfDraft: true, // TODO: Add config for this? - // TODO: Add config for this? + skipIfDraft: true, skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNotBehindTargetBranch: true, // TODO: Add config for this? + skipIfNotBehindTargetBranch: true, }); // Re-approve the pull request, if required @@ -286,16 +265,6 @@ export function buildPullRequestProperties(packageManager: string, dependencies: ]; } -export function parseProjectDependencyListProperty( - properties: Record, - repository: string, - packageManager: string, -): any { - const dependencyList = properties?.[DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST] || '{}'; - const repoDependencyLists = JSON.parse(dependencyList); - return repoDependencyLists[repository]?.[packageManager]; -} - export function parsePullRequestProperties( pullRequests: IPullRequestProperties[], packageManager: string | null, From 8012d22ccdc41f3ae81461e57f515b5b81d87a44 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 02:21:50 +1300 Subject: [PATCH 06/36] Revert "Clean up" This reverts commit 9561273b7fe1cd155398a0a41e65331512e3909e. --- extension/tasks/dependabotV2/index.ts | 12 +++++- .../DependabotOutputProcessor.ts | 39 +++++++++++++++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index c52ad2d5..94c4c87f 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -4,6 +4,7 @@ import { DependabotCli } from './utils/dependabot-cli/DependabotCli'; import { DependabotJobBuilder } from './utils/dependabot-cli/DependabotJobBuilder'; import { DependabotOutputProcessor, + parseProjectDependencyListProperty, parsePullRequestProperties, } from './utils/dependabot-cli/DependabotOutputProcessor'; import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; @@ -83,6 +84,15 @@ async function run() { for (const update of updates) { const updateId = updates.indexOf(update).toString(); + // Parse the last dependency list snapshot (if any) from the project properties. + // This is required when doing a security-only update as dependabot requires the list of vulnerable dependencies to be updated. + // Automatic discovery of vulnerable dependencies during a security-only update is not currently supported by dependabot-updater. + const dependencyList = parseProjectDependencyListProperty( + await prAuthorClient.getProjectProperties(taskInputs.projectId), + taskInputs.repository, + update['package-ecosystem'], + ); + // Parse the Dependabot metadata for the existing pull requests that are related to this update // Dependabot will use this to determine if we need to create new pull requests or update/close existing ones const existingPullRequests = parsePullRequestProperties(prAuthorActivePullRequests, update['package-ecosystem']); @@ -94,7 +104,7 @@ async function run() { updateId, update, dependabotConfig.registries, - undefined, // TODO: Implement this, required for security-only updates + dependencyList?.['dependencies'], existingPullRequestDependencies, ); const allDependenciesUpdateOutputs = await dependabot.update(allDependenciesJob, dependabotUpdaterOptions); diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index 8fdd2fc8..dc8428b5 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -18,6 +18,10 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess private readonly existingPullRequests: IPullRequestProperties[]; private readonly taskInputs: ISharedVariables; + // Custom properties used to store dependabot metadata in projects. + // https://learn.microsoft.com/en-us/rest/api/azure/devops/core/projects/set-project-properties + public static PROJECT_PROPERTY_NAME_DEPENDENCY_LIST = 'Dependabot.DependencyList'; + // Custom properties used to store dependabot metadata in pull requests. // https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-properties public static PR_PROPERTY_NAME_PACKAGE_MANAGER = 'Dependabot.PackageManager'; @@ -54,9 +58,25 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess // See: https://github.com/dependabot/cli/blob/main/internal/model/update.go case 'update_dependency_list': - // Store the dependency list snapshot, if configured + // Store the dependency list snapshot in project properties, if configured if (this.taskInputs.storeDependencyList) { - // TODO: Store the dependency list snapshot + console.info(`Storing the dependency list snapshot for project '${project}'...`); + await this.prAuthorClient.updateProjectProperty( + this.taskInputs.projectId, + DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST, + function (existingValue: string) { + const repoDependencyLists = JSON.parse(existingValue || '{}'); + repoDependencyLists[repository] = repoDependencyLists[repository] || {}; + repoDependencyLists[repository][update.job['package-manager']] = { + 'dependencies': data['dependencies'], + 'dependency-files': data['dependency_files'], + 'last-updated': new Date().toISOString(), + }; + + return JSON.stringify(repoDependencyLists); + }, + ); + console.info(`Dependency list snapshot was updated for project '${project}'`); } return true; @@ -160,10 +180,11 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfDraft: true, + skipIfDraft: true, // TODO: Add config for this? + // TODO: Add config for this? skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNotBehindTargetBranch: true, + skipIfNotBehindTargetBranch: true, // TODO: Add config for this? }); // Re-approve the pull request, if required @@ -265,6 +286,16 @@ export function buildPullRequestProperties(packageManager: string, dependencies: ]; } +export function parseProjectDependencyListProperty( + properties: Record, + repository: string, + packageManager: string, +): any { + const dependencyList = properties?.[DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST] || '{}'; + const repoDependencyLists = JSON.parse(dependencyList); + return repoDependencyLists[repository]?.[packageManager]; +} + export function parsePullRequestProperties( pullRequests: IPullRequestProperties[], packageManager: string | null, From 37917b5931f222bdce0727d5382913e99f603aac Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 02:24:10 +1300 Subject: [PATCH 07/36] Revert --- .../azure-devops/AzureDevOpsWebApiClient.ts | 50 +++++++++++++++++++ .../DependabotOutputProcessor.ts | 5 +- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index d9143134..744e9bfd 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -523,6 +523,56 @@ export class AzureDevOpsWebApiClient { } } + /** + * Get project properties + * @param projectId + * @param valueBuilder + * @returns + */ + public async getProjectProperties(projectId: string): Promise | undefined> { + try { + const core = await this.connection.getCoreApi(); + const properties = await core.getProjectProperties(projectId); + return properties?.map((p) => ({ [p.name]: p.value }))?.reduce((a, b) => ({ ...a, ...b }), {}); + } catch (e) { + error(`Failed to get project properties: ${e}`); + console.debug(e); // Dump the error stack trace to help with debugging + return undefined; + } + } + + /** + * Update a project property + * @param project + * @param name + * @param valueBuilder + * @returns + */ + public async updateProjectProperty( + projectId: string, + name: string, + valueBuilder: (existingValue: string) => string, + ): Promise { + try { + // Get the existing project property value + const core = await this.connection.getCoreApi(); + const properties = await core.getProjectProperties(projectId); + const propertyValue = properties?.find((p) => p.name === name)?.value; + + // Update the project property + await core.setProjectProperties(undefined, projectId, [ + { + op: 'add', + path: '/' + name, + value: valueBuilder(propertyValue || ''), + }, + ]); + } catch (e) { + error(`Failed to update project property '${name}': ${e}`); + console.debug(e); // Dump the error stack trace to help with debugging + } + } + private async restApiGet( url: string, params?: Record, diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index dc8428b5..c1e81aff 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -180,11 +180,10 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfDraft: true, // TODO: Add config for this? - // TODO: Add config for this? + skipIfDraft: true, skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNotBehindTargetBranch: true, // TODO: Add config for this? + skipIfNotBehindTargetBranch: true, }); // Re-approve the pull request, if required From e9c950bd62f2e84a7a5d57d9ef2cdb65b514e703 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 10:33:55 +1300 Subject: [PATCH 08/36] Remove double forward slash in DevOps API urls --- .../utils/azure-devops/AzureDevOpsWebApiClient.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 744e9bfd..0cf13d6f 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -27,8 +27,8 @@ export class AzureDevOpsWebApiClient { public static API_VERSION = '7.1'; constructor(organisationApiUrl: string, accessToken: string) { - this.organisationApiUrl = organisationApiUrl; - this.identityApiUrl = getIdentityApiUrl(organisationApiUrl); + this.organisationApiUrl = organisationApiUrl.replace(/\/$/, ''); // trim trailing slash + this.identityApiUrl = getIdentityApiUrl(organisationApiUrl).replace(/\/$/, '');; // trim trailing slash this.accessToken = accessToken; this.connection = new WebApi(organisationApiUrl, getPersonalAccessTokenHandler(accessToken)); this.resolvedUserIds = {}; From d037409b54d5e3a8ef5f2242c6acf21bff0d01bb Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 10:35:36 +1300 Subject: [PATCH 09/36] Add more detailed error info when REST APIs fail --- .../azure-devops/AzureDevOpsWebApiClient.ts | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 0cf13d6f..bf94d6f5 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -28,7 +28,7 @@ export class AzureDevOpsWebApiClient { constructor(organisationApiUrl: string, accessToken: string) { this.organisationApiUrl = organisationApiUrl.replace(/\/$/, ''); // trim trailing slash - this.identityApiUrl = getIdentityApiUrl(organisationApiUrl).replace(/\/$/, '');; // trim trailing slash + this.identityApiUrl = getIdentityApiUrl(organisationApiUrl).replace(/\/$/, ''); // trim trailing slash this.accessToken = accessToken; this.connection = new WebApi(organisationApiUrl, getPersonalAccessTokenHandler(accessToken)); this.resolvedUserIds = {}; @@ -636,28 +636,24 @@ export class AzureDevOpsWebApiClient { ): Promise { console.debug(`🌎 🠊 [${method}] ${url}`); const response = await request(); + const body = await response.readBody(); console.debug(`🌎 🠈 [${response.message.statusCode}] ${response.message.statusMessage}`); - if (response.message.statusCode === 401) { - throw new Error(`No access token has been provided to access '${url}'`); - } - if (response.message.statusCode === 403) { - throw new Error(`The access token provided does not have permissions to access '${url}'`); - } - if (response.message.statusCode < 200 || response.message.statusCode > 299) { - throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); - } try { - const responseBodyJson = JSON.parse(await response.readBody()); - if (responseBodyJson?.errorCode !== undefined && responseBodyJson?.message) { - // .NET API error response - throw new Error(responseBodyJson.message); + if (response.message.statusCode === 401) { + throw new Error(`No access token has been provided to access '${url}'`); } - - return responseBodyJson; + if (response.message.statusCode === 403) { + throw new Error(`The access token provided does not have permissions to access '${url}'`); + } + if (response.message.statusCode < 200 || response.message.statusCode > 299) { + throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); + } + return JSON.parse(body); } catch (e) { - // JSON parsing failed, log the error and return undefined - console.debug(response.message); - throw new Error(`Failed to parse response body as JSON: ${e}`); + if (body) { + console.debug(body); + } + throw e; } } } From 302e36a437726509fe0ea2ad834e005dfb8f30b3 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 11:29:45 +1300 Subject: [PATCH 10/36] Fix for identity id being mapped as an array instead of a string --- .../dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index bf94d6f5..07573639 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -62,7 +62,7 @@ export class AzureDevOpsWebApiClient { if (!identities?.value || identities.value.length === 0) { return undefined; } - this.resolvedUserIds[nameEmailOrGroup] = identities.value.map((i) => i.id) || []; + this.resolvedUserIds[nameEmailOrGroup] = identities.value[0]?.id; return this.resolvedUserIds[nameEmailOrGroup]; } catch (e) { error(`Failed to resolve user id: ${e}`); From 27586186b849694c75ffb5feb2fe9036c1328585 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:13:15 +1300 Subject: [PATCH 11/36] Add documentation for configuring assigness and reviewers --- README.md | 20 ++++++++++++++++++-- extension/README.md | 1 + 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index fc6962b8..b7ed31e3 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ In this repository you'll find: - [Configuring private feeds and registries](#configuring-private-feeds-and-registries) - [Configuring security advisories and known vulnerabilities](#configuring-security-advisories-and-known-vulnerabilities) - [Configuring experiments](#configuring-experiments) +- [Configuring assignees and reviewers](#configuring-assignees-and-reviewers) - [Unsupported features and configurations](#unsupported-features-and-configurations) * [Extension Task](#extension-task) + [dependabot@V2](#dependabotv2) @@ -174,11 +175,23 @@ Experiments vary depending on the package ecyosystem used; They can be enabled u | NuGet | nuget_native_updater | true/false | https://github.com/dependabot/dependabot-core/pull/10521 | | NuGet | nuget_dependency_solver | true/false | https://github.com/dependabot/dependabot-core/pull/10343 | - - > [!TIP] > To find the latest list of Dependabot experiments, search the `dependabot-core` GitHub repository using queries like ["enabled?(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Fenabled%5CW%5C%28.*%5C%29%2F&type=code) and ["options.fetch(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Foptions%5C.fetch%5C%28.*%2C%2F&type=code). + + +## Configuring assignees and reviewers +Dependabot allows for the configuration of both [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers). However, Azure DevOps does not have the concept of pull request assignees. Because of this, `assignees` will be treated as **required** reviewers and `reviewers` will be treated as **optional** reviewers. + +Reviewers can be any of the following values: + +- User GUID +- User username +- User email address +- User full [display] name +- Group name +- Team name + ## Unsupported features and configurations We aim to support all [official configuration options](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file), but there are some limitations for: @@ -193,15 +206,18 @@ We aim to support all [official configuration options](https://docs.github.com/e - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are only supported if task input `useUpdateScriptVNext: true` is set. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are only supported if task input `useUpdateScriptVNext: true` is set. - [`ignore`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#ignore) may not behave to official specifications unless task input `useUpdateScriptVNext: true` is set. If you are having issues, search for related issues such as before creating a new issue. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids or email addressess; group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. Support is _slightly_ improved when task input `useUpdateScriptVNext: true` is set, but not still not fully supported. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Updater Docker image +- `DEPENDABOT_ASSIGNEES` and `DEPENDABOT_REVIEWERS` must be a list of user guids; email addressess and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Server - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are not supported. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are not supported. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids; email addressess and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ## Migration Guide diff --git a/extension/README.md b/extension/README.md index abf31027..06493650 100644 --- a/extension/README.md +++ b/extension/README.md @@ -105,5 +105,6 @@ Dependabot uses Docker containers, which may take time to install if not already - [Configuring private feeds and registries](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-private-feeds-and-registries) - [Configuring security advisories and known vulnerabilities](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-security-advisories-and-known-vulnerabilities) - [Configuring experiments](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-experiments) +- [Configuring assignees and reviewers](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-assignees-and-reviewers) - [Unsupported features and configurations](https://github.com/tinglesoftware/dependabot-azure-devops/#unsupported-features-and-configurations) - [Task migration guide for V1 → V2](https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md) From a3e87ddae77b7c52dc93285e69393ea3f108f546 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:15:56 +1300 Subject: [PATCH 12/36] Fix typos --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b7ed31e3..5bc37eac 100644 --- a/README.md +++ b/README.md @@ -206,18 +206,18 @@ We aim to support all [official configuration options](https://docs.github.com/e - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are only supported if task input `useUpdateScriptVNext: true` is set. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are only supported if task input `useUpdateScriptVNext: true` is set. - [`ignore`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#ignore) may not behave to official specifications unless task input `useUpdateScriptVNext: true` is set. If you are having issues, search for related issues such as before creating a new issue. -- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids or email addressess; group/team names are not supported. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids or email addresses; group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. Support is _slightly_ improved when task input `useUpdateScriptVNext: true` is set, but not still not fully supported. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Updater Docker image -- `DEPENDABOT_ASSIGNEES` and `DEPENDABOT_REVIEWERS` must be a list of user guids; email addressess and group/team names are not supported. +- `DEPENDABOT_ASSIGNEES` and `DEPENDABOT_REVIEWERS` must be a list of user guids; email addresses and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Server - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are not supported. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are not supported. -- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids; email addressess and group/team names are not supported. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids; email addresses and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ## Migration Guide From 00248ce59482091a45810b33cb460f3029f0a2e7 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:23:09 +1300 Subject: [PATCH 13/36] Alerts cannot be embeeded inside
--- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 5bc37eac..cf2c746e 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,6 @@ Experiments vary depending on the package ecyosystem used; They can be enabled u | NuGet | nuget_native_updater | true/false | https://github.com/dependabot/dependabot-core/pull/10521 | | NuGet | nuget_dependency_solver | true/false | https://github.com/dependabot/dependabot-core/pull/10343 | -> [!TIP] > To find the latest list of Dependabot experiments, search the `dependabot-core` GitHub repository using queries like ["enabled?(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Fenabled%5CW%5C%28.*%5C%29%2F&type=code) and ["options.fetch(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Foptions%5C.fetch%5C%28.*%2C%2F&type=code).
From d86e16b9e1c3ffab5a1486a91a20743172f80109 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:32:15 +1300 Subject: [PATCH 14/36] Clean up --- .../azure-devops/AzureDevOpsWebApiClient.ts | 47 ++++++++----------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 07573639..f7229f62 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -46,24 +46,24 @@ export class AzureDevOpsWebApiClient { /** * Get the identity id from a user name, email, or group name. * Requires scope "Identity (Read)" (vso.identity). - * @param nameEmailOrGroup + * @param userNameEmailOrGroupName * @returns */ - public async resolveIdentityId(nameEmailOrGroup?: string): Promise { - if (this.resolvedUserIds[nameEmailOrGroup]) { - return this.resolvedUserIds[nameEmailOrGroup]; + public async resolveIdentityId(userNameEmailOrGroupName?: string): Promise { + if (this.resolvedUserIds[userNameEmailOrGroupName]) { + return this.resolvedUserIds[userNameEmailOrGroupName]; } try { const identities = await this.restApiGet(`${this.identityApiUrl}/_apis/identities`, { searchFilter: 'General', - filterValue: nameEmailOrGroup, + filterValue: userNameEmailOrGroupName, queryMembership: 'None', }); if (!identities?.value || identities.value.length === 0) { return undefined; } - this.resolvedUserIds[nameEmailOrGroup] = identities.value[0]?.id; - return this.resolvedUserIds[nameEmailOrGroup]; + this.resolvedUserIds[userNameEmailOrGroupName] = identities.value[0]?.id; + return this.resolvedUserIds[userNameEmailOrGroupName]; } catch (e) { error(`Failed to resolve user id: ${e}`); console.debug(e); // Dump the error stack trace to help with debugging @@ -81,7 +81,6 @@ export class AzureDevOpsWebApiClient { public async getDefaultBranch(project: string, repository: string): Promise { try { const repo = await this.restApiGet(`${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}`); - console.log(repo); if (!repo) { throw new Error(`Repository '${project}/${repository}' not found`); } @@ -108,32 +107,26 @@ export class AzureDevOpsWebApiClient { creator: string, ): Promise { try { - const pullRequests = - ( - await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests`, - { - 'searchCriteria.creatorId': isGuid(creator) ? creator : await this.getUserId(), - 'searchCriteria.status': 'Active', - }, - ) - )?.value || []; - if (!pullRequests || pullRequests.length === 0) { + const pullRequests = await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests`, + { + 'searchCriteria.creatorId': isGuid(creator) ? creator : await this.getUserId(), + 'searchCriteria.status': 'Active', + }, + ); + if (!pullRequests?.value || pullRequests.value.length === 0) { return []; } return await Promise.all( - pullRequests.map(async (pr) => { - const properties = - ( - await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties`, - ) - )?.value || {}; + pullRequests.value.map(async (pr) => { + const properties = await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties`, + ); return { id: pr.pullRequestId, properties: - Object.keys(properties).map((key) => { + Object.keys(properties?.value || {}).map((key) => { return { name: key, value: properties[key]?.$value, From 0d51fb7cec8ccec4dd43c6c46ab8c90e27bcef4f Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 14:41:34 +1300 Subject: [PATCH 15/36] Tidy up --- .../azure-devops/AzureDevOpsWebApiClient.ts | 152 +++++++++--------- .../azure-devops/interfaces/IPullRequest.ts | 53 +++++- .../interfaces/IPullRequestProperties.ts | 10 -- .../DependabotOutputProcessor.ts | 9 +- 4 files changed, 137 insertions(+), 87 deletions(-) delete mode 100644 extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index f7229f62..6dd47487 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -9,9 +9,13 @@ import { } from 'azure-devops-node-api/interfaces/GitInterfaces'; import { error, warning } from 'azure-pipelines-task-lib/task'; import { IHttpClientResponse } from 'typed-rest-client/Interfaces'; -import { IFileChange } from './interfaces/IFileChange'; -import { IPullRequest } from './interfaces/IPullRequest'; -import { IPullRequestProperties } from './interfaces/IPullRequestProperties'; +import { + IAbandonPullRequest, + IApprovePullRequest, + ICreatePullRequest, + IPullRequestProperties, + IUpdatePullRequest, +} from './interfaces/IPullRequest'; /** * Wrapper for DevOps WebApi client with helper methods for easier management of dependabot pull requests @@ -129,7 +133,7 @@ export class AzureDevOpsWebApiClient { Object.keys(properties?.value || {}).map((key) => { return { name: key, - value: properties[key]?.$value, + value: properties.value[key]?.$value, }; }) || [], }; @@ -149,7 +153,7 @@ export class AzureDevOpsWebApiClient { * @param pr * @returns */ - public async createPullRequest(pr: IPullRequest): Promise { + public async createPullRequest(pr: ICreatePullRequest): Promise { console.info(`Creating pull request '${pr.title}'...`); try { const userId = await this.getUserId(); @@ -303,68 +307,85 @@ export class AzureDevOpsWebApiClient { /** * Update a pull request. * Requires scope "Code (Read & Write)" (vso.code, vso.code_write). - * @param options + * @param pr * @returns */ - public async updatePullRequest(options: { - project: string; - repository: string; - pullRequestId: number; - changes: IFileChange[]; - skipIfDraft?: boolean; - skipIfCommitsFromAuthorsOtherThan?: string; - skipIfNotBehindTargetBranch?: boolean; - }): Promise { - console.info(`Updating pull request #${options.pullRequestId}...`); + public async updatePullRequest(pr: IUpdatePullRequest): Promise { + console.info(`Updating pull request #${pr.pullRequestId}...`); try { // Get the pull request details const pullRequest = await this.restApiGet( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}`, ); if (!pullRequest) { - throw new Error(`Pull request #${options.pullRequestId} not found`); + throw new Error(`Pull request #${pr.pullRequestId} not found`); } // Skip if the pull request is a draft - if (options.skipIfDraft && pullRequest.isDraft) { + if (pr.skipIfDraft && pullRequest.isDraft) { console.info(` - Skipping update as pull request is currently marked as a draft.`); return true; } // Skip if the pull request has been modified by another author - if (options.skipIfCommitsFromAuthorsOtherThan) { + if (pr.skipIfCommitsFromAuthorsOtherThan) { const commits = await this.restApiGet( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/commits`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}/commits`, ); - if (commits?.value?.some((c) => c.author?.email !== options.skipIfCommitsFromAuthorsOtherThan)) { + if (commits?.value?.some((c) => c.author?.email !== pr.skipIfCommitsFromAuthorsOtherThan)) { console.info(` - Skipping update as pull request has been modified by another user.`); return true; } } + // Get the branch stats to check if the source branch is behind the target branch + const stats = await this.restApiGet( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/stats/branches`, + { + name: normalizeBranchName(pullRequest.sourceRefName), + }, + ); + if (stats?.behindCount === undefined) { + throw new Error(`Failed to get branch stats for '${pullRequest.sourceRefName}'`); + } + // Skip if the source branch is not behind the target branch - if (options.skipIfNotBehindTargetBranch) { - const stats = await this.restApiGet( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/stats/branches`, - { - name: normalizeBranchName(pullRequest.sourceRefName), - }, + if (pr.skipIfNotBehindTargetBranch && stats.behindCount === 0) { + console.info(` - Skipping update as source branch is not behind target branch.`); + return true; + } + + // Rebase the target branch into the source branch to reset the "behind" count + const sourceBranchName = normalizeBranchName(pullRequest.sourceRefName); + const targetBranchName = normalizeBranchName(pullRequest.targetRefName); + if (stats.behindCount > 0) { + console.info( + ` - Rebasing '${targetBranchName}' into '${sourceBranchName}' (${stats.behindCount} commit(s) behind)...`, ); - if (stats?.behindCount === 0) { - console.info(` - Skipping update as source branch is not behind target branch.`); - return true; + const rebase = await this.restApiPost( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/refs`, + [ + { + name: pullRequest.sourceRefName, + oldObjectId: pullRequest.lastMergeSourceCommit.commitId, + newObjectId: pr.commit, + }, + ], + ); + if (rebase?.value?.[0]?.success !== true) { + throw new Error('Failed to rebase the target branch into the source branch'); } } - // Push changes to the source branch - console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); + // Push all file changes to the source branch + console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pullRequest.sourceRefName}'...`); const push = await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes`, { refUpdates: [ { name: pullRequest.sourceRefName, - oldObjectId: pullRequest.lastMergeSourceCommit.commitId, + oldObjectId: pr.commit, }, ], commits: [ @@ -372,8 +393,9 @@ export class AzureDevOpsWebApiClient { comment: pullRequest.mergeStatus === PullRequestAsyncStatus.Conflicts ? 'Resolve merge conflicts' - : 'Update dependency files', - changes: options.changes.map((change) => { + : `Rebase with '${targetBranchName}'`, + author: pr.author, + changes: pr.changes.map((change) => { return { changeType: change.changeType, item: { @@ -406,24 +428,20 @@ export class AzureDevOpsWebApiClient { /** * Approve a pull request. * Requires scope "Code (Write)" (vso.code_write). - * @param options + * @param pr * @returns */ - public async approvePullRequest(options: { - project: string; - repository: string; - pullRequestId: number; - }): Promise { - console.info(`Approving pull request #${options.pullRequestId}...`); + public async approvePullRequest(pr: IApprovePullRequest): Promise { + console.info(`Approving pull request #${pr.pullRequestId}...`); try { // Approve the pull request - console.info(` - Creating reviewer vote on pull request...`); + console.info(` - Updating reviewer vote on pull request...`); const userId = await this.getUserId(); const userVote = await this.restApiPut( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/reviewers/${userId}`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}/reviewers/${userId}`, { vote: 10, // 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected - isReapprove: true, + isReapprove: false, // don't re-approve if already approved }, ); if (userVote?.vote != 10) { @@ -439,27 +457,21 @@ export class AzureDevOpsWebApiClient { } /** - * Close a pull request. + * Abandon a pull request. * Requires scope "Code (Write)" (vso.code_write). - * @param options + * @param pr * @returns */ - public async closePullRequest(options: { - project: string; - repository: string; - pullRequestId: number; - comment: string; - deleteSourceBranch: boolean; - }): Promise { - console.info(`Closing pull request #${options.pullRequestId}...`); + public async abandonPullRequest(pr: IAbandonPullRequest): Promise { + console.info(`Abandoning pull request #${pr.pullRequestId}...`); try { const userId = await this.getUserId(); // Add a comment to the pull request, if supplied - if (options.comment) { + if (pr.comment) { console.info(` - Adding abandonment reason comment to pull request...`); const thread = await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/threads`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}/threads`, { status: CommentThreadStatus.Closed, comments: [ @@ -467,7 +479,7 @@ export class AzureDevOpsWebApiClient { author: { id: userId, }, - content: options.comment, + content: pr.comment, commentType: CommentType.System, }, ], @@ -478,10 +490,10 @@ export class AzureDevOpsWebApiClient { } } - // Close the pull request + // Abandon the pull request console.info(` - Abandoning pull request...`); const abandonedPullRequest = await this.restApiPatch( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}`, { status: PullRequestStatus.Abandoned, closedBy: { @@ -490,14 +502,14 @@ export class AzureDevOpsWebApiClient { }, ); if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { - throw new Error('Failed to close pull request, status was not updated'); + throw new Error('Failed to abandon pull request, status was not updated'); } // Delete the source branch if required - if (options.deleteSourceBranch) { + if (pr.deleteSourceBranch) { console.info(` - Deleting source branch...`); await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/refs`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/refs`, { name: abandonedPullRequest.sourceRefName, oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, @@ -507,10 +519,10 @@ export class AzureDevOpsWebApiClient { ); } - console.info(` - Pull request was closed successfully.`); + console.info(` - Pull request was abandoned successfully.`); return true; } catch (e) { - error(`Failed to close pull request: ${e}`); + error(`Failed to abandon pull request: ${e}`); console.debug(e); // Dump the error stack trace to help with debugging return false; } @@ -632,12 +644,6 @@ export class AzureDevOpsWebApiClient { const body = await response.readBody(); console.debug(`🌎 🠈 [${response.message.statusCode}] ${response.message.statusMessage}`); try { - if (response.message.statusCode === 401) { - throw new Error(`No access token has been provided to access '${url}'`); - } - if (response.message.statusCode === 403) { - throw new Error(`The access token provided does not have permissions to access '${url}'`); - } if (response.message.statusCode < 200 || response.message.statusCode > 299) { throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); } diff --git a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts b/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts index aaa21a02..f27464bf 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts @@ -2,9 +2,20 @@ import { GitPullRequestMergeStrategy } from 'azure-devops-node-api/interfaces/Gi import { IFileChange } from './IFileChange'; /** - * Pull request creation + * Pull request properties */ -export interface IPullRequest { +export interface IPullRequestProperties { + id: number; + properties?: { + name: string; + value: string; + }[]; +} + +/** + * Pull request creation request + */ +export interface ICreatePullRequest { project: string; repository: string; source: { @@ -35,3 +46,41 @@ export interface IPullRequest { value: string; }[]; } + +/** + * Pull request update request + */ +export interface IUpdatePullRequest { + project: string; + repository: string; + pullRequestId: number; + commit?: string; + author?: { + email: string; + name: string; + }; + changes: IFileChange[]; + skipIfDraft?: boolean; + skipIfCommitsFromAuthorsOtherThan?: string; + skipIfNotBehindTargetBranch?: boolean; +} + +/** + * Pull request approval request + */ +export interface IApprovePullRequest { + project: string; + repository: string; + pullRequestId: number; +} + +/** + * Pull request abandon request + */ +export interface IAbandonPullRequest { + project: string; + repository: string; + pullRequestId: number; + comment: string; + deleteSourceBranch: boolean; +} diff --git a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts b/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts deleted file mode 100644 index 2fd2e6fb..00000000 --- a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts +++ /dev/null @@ -1,10 +0,0 @@ -/** - * Pull request properties - */ -export interface IPullRequestProperties { - id: number; - properties?: { - name: string; - value: string; - }[]; -} diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index c1e81aff..7607f7d3 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -3,7 +3,7 @@ import { error, warning } from 'azure-pipelines-task-lib/task'; import * as crypto from 'crypto'; import * as path from 'path'; import { AzureDevOpsWebApiClient } from '../azure-devops/AzureDevOpsWebApiClient'; -import { IPullRequestProperties } from '../azure-devops/interfaces/IPullRequestProperties'; +import { IPullRequestProperties } from '../azure-devops/interfaces/IPullRequest'; import { IDependabotUpdate } from '../dependabot/interfaces/IDependabotConfig'; import { ISharedVariables } from '../getSharedVariables'; import { IDependabotUpdateOperation } from './interfaces/IDependabotUpdateOperation'; @@ -179,6 +179,11 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess project: project, repository: repository, pullRequestId: pullRequestToUpdate.id, + commit: data['base-commit-sha'] || update.job.source.commit, + author: { + email: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, + name: this.taskInputs.authorName || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_NAME, + }, changes: getPullRequestChangedFilesForOutputData(data), skipIfDraft: true, skipIfCommitsFromAuthorsOtherThan: @@ -219,7 +224,7 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess // How do we detect this? Do we need to? // Close the pull request - return await this.prAuthorClient.closePullRequest({ + return await this.prAuthorClient.abandonPullRequest({ project: project, repository: repository, pullRequestId: pullRequestToClose.id, From 29dab82b106e30ab91f8be26771ff611077448d1 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 14:52:43 +1300 Subject: [PATCH 16/36] Tidy up --- .../azure-devops/AzureDevOpsWebApiClient.ts | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 6dd47487..fe00ad27 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -501,22 +501,27 @@ export class AzureDevOpsWebApiClient { }, }, ); - if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { + if (abandonedPullRequest?.status?.toLowerCase() !== 'abandoned') { throw new Error('Failed to abandon pull request, status was not updated'); } // Delete the source branch if required if (pr.deleteSourceBranch) { console.info(` - Deleting source branch...`); - await this.restApiPost( + const deletedBranch = await this.restApiPost( `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/refs`, - { - name: abandonedPullRequest.sourceRefName, - oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, - newObjectId: '0000000000000000000000000000000000000000', - isLocked: false, - }, + [ + { + name: abandonedPullRequest.sourceRefName, + oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, + newObjectId: '0000000000000000000000000000000000000000', + isLocked: false, + }, + ], ); + if (deletedBranch?.value?.[0]?.success !== true) { + throw new Error('Failed to delete the source branch'); + } } console.info(` - Pull request was abandoned successfully.`); From 0ea23dbd9bef779c178f103753d14a63e0aef1ae Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 15:51:47 +1300 Subject: [PATCH 17/36] Fix undefined reference when reading branch name seprarator --- .../utils/dependabot-cli/DependabotOutputProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index 7607f7d3..1b08e0da 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -326,7 +326,7 @@ export function parsePullRequestProperties( function getSourceBranchNameForUpdate(update: IDependabotUpdate, targetBranch: string, dependencies: any): string { const prefix = 'dependabot'; // TODO: Add config for this? Task V1 supported this via DEPENDABOT_BRANCH_NAME_PREFIX - const separator = update['pull-request-branch-name'].separator || '/'; + const separator = update['pull-request-branch-name']?.separator || '/'; const packageEcosystem = update['package-ecosystem']; const targetBranchName = targetBranch?.replace(/^\/+|\/+$/g, ''); // strip leading/trailing slashes if (dependencies['dependency-group-name']) { From fc8fb8893edff6bf4265f20e5346cd27dd6cc98b Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 23:37:50 +1300 Subject: [PATCH 18/36] Add support for security-only updates --- extension/tasks/dependabotV2/index.ts | 54 +++++--- .../dependabot-cli/DependabotJobBuilder.ts | 125 +++++++++++------- .../interfaces/IDependabotUpdateJobConfig.ts | 2 +- .../utils/github/getSecurityAdvisories.ts | 122 +++++++++++++++++ 4 files changed, 240 insertions(+), 63 deletions(-) create mode 100644 extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 94c4c87f..bd07ec59 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -4,12 +4,12 @@ import { DependabotCli } from './utils/dependabot-cli/DependabotCli'; import { DependabotJobBuilder } from './utils/dependabot-cli/DependabotJobBuilder'; import { DependabotOutputProcessor, - parseProjectDependencyListProperty, parsePullRequestProperties, } from './utils/dependabot-cli/DependabotOutputProcessor'; import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; import parseDependabotConfigFile from './utils/dependabot/parseConfigFile'; import parseTaskInputConfiguration from './utils/getSharedVariables'; +import { getSecurityAdvisories, ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; async function run() { let dependabot: DependabotCli = undefined; @@ -83,33 +83,55 @@ async function run() { // Loop through the [targeted] update blocks in dependabot.yaml and perform updates for (const update of updates) { const updateId = updates.indexOf(update).toString(); - - // Parse the last dependency list snapshot (if any) from the project properties. - // This is required when doing a security-only update as dependabot requires the list of vulnerable dependencies to be updated. - // Automatic discovery of vulnerable dependencies during a security-only update is not currently supported by dependabot-updater. - const dependencyList = parseProjectDependencyListProperty( - await prAuthorClient.getProjectProperties(taskInputs.projectId), - taskInputs.repository, - update['package-ecosystem'], - ); + const packageEcosystem = update['package-ecosystem']; // Parse the Dependabot metadata for the existing pull requests that are related to this update // Dependabot will use this to determine if we need to create new pull requests or update/close existing ones - const existingPullRequests = parsePullRequestProperties(prAuthorActivePullRequests, update['package-ecosystem']); + const existingPullRequests = parsePullRequestProperties(prAuthorActivePullRequests, packageEcosystem); const existingPullRequestDependencies = Object.entries(existingPullRequests).map(([id, deps]) => deps); + // If this is a security-only update (i.e. 'open-pull-requests-limit: 0'), then we first need to discover the dependencies + // that need updating and check each one for security advisories. This is because Dependabot requires the list of vulnerable dependencies + // to be supplied in the job definition of security-only update job, it will not automatically discover them like a versioned update does. + // https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates#overriding-the-default-behavior-with-a-configuration-file + // TODO: If/when Dependabot supports a better way to do security-only updates, we should remove this code block. + let securityAdvisories: ISecurityAdvisory[] = undefined; + let dependencyNamesToUpdate: string[] = undefined; + const securityUpdatesOnly = update['open-pull-requests-limit'] === 0; + if (securityUpdatesOnly) { + warning( + 'Security-only updates are not yet fully supported by Dependabot CLI. ' + + 'The task will now attempt to discover the dependencies that need updating using an "ignore everything" update job, ' + + 'then check the discovered dependencies for security advisories before finally performing the requested security-only update. ' + + 'Because of this, the task may take longer to complete than usual.', + ); + const discoveredDependencyListOutputs = await dependabot.update( + DependabotJobBuilder.newDiscoverDependencyListJob(taskInputs, updateId, update, dependabotConfig.registries), + dependabotUpdaterOptions, + ); + dependencyNamesToUpdate = discoveredDependencyListOutputs + ?.find((x) => x.output.type == 'update_dependency_list') + ?.output?.data?.dependencies?.map((d) => d.name); + securityAdvisories = await getSecurityAdvisories( + taskInputs.githubAccessToken, + packageEcosystem, + dependencyNamesToUpdate || [], + ); + } + // Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating - const allDependenciesJob = DependabotJobBuilder.newUpdateAllJob( + const updateAllDependenciesJob = DependabotJobBuilder.newUpdateAllJob( taskInputs, updateId, update, dependabotConfig.registries, - dependencyList?.['dependencies'], + dependencyNamesToUpdate, existingPullRequestDependencies, + securityAdvisories, ); - const allDependenciesUpdateOutputs = await dependabot.update(allDependenciesJob, dependabotUpdaterOptions); - if (!allDependenciesUpdateOutputs || allDependenciesUpdateOutputs.filter((u) => !u.success).length > 0) { - allDependenciesUpdateOutputs?.filter((u) => !u.success)?.forEach((u) => exception(u.error)); + const updateAllDependenciesOutputs = await dependabot.update(updateAllDependenciesJob, dependabotUpdaterOptions); + if (!updateAllDependenciesOutputs || updateAllDependenciesOutputs.filter((u) => !u.success).length > 0) { + updateAllDependenciesOutputs?.filter((u) => !u.success)?.forEach((u) => exception(u.error)); failedJobs++; } diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts index 005a4c18..01f2476f 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts @@ -1,4 +1,3 @@ -import { warning } from 'azure-pipelines-task-lib'; import { IDependabotAllowCondition, IDependabotGroup, @@ -6,19 +5,48 @@ import { IDependabotUpdate, } from '../dependabot/interfaces/IDependabotConfig'; import { ISharedVariables } from '../getSharedVariables'; +import { ISecurityAdvisory } from '../github/getSecurityAdvisories'; import { IDependabotUpdateOperation } from './interfaces/IDependabotUpdateOperation'; /** * Wrapper class for building dependabot update job objects */ export class DependabotJobBuilder { + /** + * Create a dependabot update job that updates nothing, but will discover the dependency list for a package ecyosystem + * @param taskInputs + * @param update + * @param registries + * @returns + */ + public static newDiscoverDependencyListJob( + taskInputs: ISharedVariables, + id: string, + update: IDependabotUpdate, + registries: Record, + ): IDependabotUpdateOperation { + return { + config: update, + job: { + 'id': `discover-${id}-${update['package-ecosystem']}-dependency-list`, + 'package-manager': update['package-ecosystem'], + 'ignore-conditions': [{ 'dependency-name': '*' }], + 'source': mapSourceFromDependabotConfigToJobConfig(taskInputs, update), + 'experiments': taskInputs.experiments, + 'debug': taskInputs.debug, + }, + credentials: mapRegistryCredentialsFromDependabotConfigToJobConfig(taskInputs, registries), + }; + } + /** * Create a dependabot update job that updates all dependencies for a package ecyosystem * @param taskInputs * @param update * @param registries - * @param dependencyList + * @param dependencyNamesToUpdate * @param existingPullRequests + * @param securityAdvisories * @returns */ public static newUpdateAllJob( @@ -26,12 +54,12 @@ export class DependabotJobBuilder { id: string, update: IDependabotUpdate, registries: Record, - dependencyList: any[], - existingPullRequests: any[], + dependencyNamesToUpdate?: string[], + existingPullRequests?: any[], + securityAdvisories?: ISecurityAdvisory[], ): IDependabotUpdateOperation { const packageEcosystem = update['package-ecosystem']; const securityUpdatesOnly = update['open-pull-requests-limit'] == 0; - const updateDependencyNames = securityUpdatesOnly ? mapDependenciesForSecurityUpdate(dependencyList) : undefined; return buildUpdateJobConfig( `update-${id}-${packageEcosystem}-${securityUpdatesOnly ? 'security-only' : 'all'}`, taskInputs, @@ -39,8 +67,11 @@ export class DependabotJobBuilder { registries, false, undefined, - updateDependencyNames, + securityUpdatesOnly + ? dependencyNamesToUpdate?.filter((d) => securityAdvisories?.find((a) => a['dependency-name'] == d)) + : dependencyNamesToUpdate, existingPullRequests, + securityAdvisories, ); } @@ -62,7 +93,7 @@ export class DependabotJobBuilder { pullRequestToUpdate: any, ): IDependabotUpdateOperation { const dependencyGroupName = pullRequestToUpdate['dependency-group-name']; - const dependencies = (dependencyGroupName ? pullRequestToUpdate['dependencies'] : pullRequestToUpdate)?.map( + const dependencyNames = (dependencyGroupName ? pullRequestToUpdate['dependencies'] : pullRequestToUpdate)?.map( (d) => d['dependency-name'], ); return buildUpdateJobConfig( @@ -72,7 +103,7 @@ export class DependabotJobBuilder { registries, true, dependencyGroupName, - dependencies, + dependencyNames, existingPullRequests, ); } @@ -83,38 +114,29 @@ function buildUpdateJobConfig( taskInputs: ISharedVariables, update: IDependabotUpdate, registries: Record, - updatingPullRequest: boolean, - updateDependencyGroupName: string | undefined, - updateDependencyNames: string[] | undefined, - existingPullRequests: any[], + updatingPullRequest?: boolean | undefined, + updateDependencyGroupName?: string | undefined, + updateDependencyNames?: string[] | undefined, + existingPullRequests?: any[], + securityAdvisories?: ISecurityAdvisory[], ) { - const hasMultipleDirectories = update.directories?.length > 1; return { config: update, job: { 'id': id, 'package-manager': update['package-ecosystem'], 'update-subdependencies': true, // TODO: add config for this? - 'updating-a-pull-request': updatingPullRequest, + 'updating-a-pull-request': updatingPullRequest || false, 'dependency-group-to-refresh': updateDependencyGroupName, 'dependency-groups': mapGroupsFromDependabotConfigToJobConfig(update.groups), 'dependencies': updateDependencyNames, 'allowed-updates': mapAllowedUpdatesFromDependabotConfigToJobConfig(update.allow), 'ignore-conditions': mapIgnoreConditionsFromDependabotConfigToJobConfig(update.ignore), 'security-updates-only': update['open-pull-requests-limit'] == 0, - 'security-advisories': [], // TODO: add config for this! - 'source': { - 'provider': 'azure', - 'api-endpoint': taskInputs.apiEndpointUrl, - 'hostname': taskInputs.hostname, - 'repo': `${taskInputs.organization}/${taskInputs.project}/_git/${taskInputs.repository}`, - 'branch': update['target-branch'], - 'commit': undefined, // use latest commit of target branch - 'directory': hasMultipleDirectories ? undefined : update.directory || '/', - 'directories': hasMultipleDirectories ? update.directories : undefined, - }, - 'existing-pull-requests': existingPullRequests.filter((pr) => !pr['dependency-group-name']), - 'existing-group-pull-requests': existingPullRequests.filter((pr) => pr['dependency-group-name']), + 'security-advisories': mapSecurityAdvisories(securityAdvisories), + 'source': mapSourceFromDependabotConfigToJobConfig(taskInputs, update), + 'existing-pull-requests': existingPullRequests?.filter((pr) => !pr['dependency-group-name']), + 'existing-group-pull-requests': existingPullRequests?.filter((pr) => pr['dependency-group-name']), 'commit-message-options': update['commit-message'] === undefined ? undefined @@ -137,25 +159,18 @@ function buildUpdateJobConfig( }; } -function mapDependenciesForSecurityUpdate(dependencyList: any[]): string[] { - if (!dependencyList || dependencyList.length == 0) { - // This happens when no previous dependency list snapshot exists yet; - // TODO: Find a way to discover dependencies for a first-time security-only update (no existing dependency list snapshot). - // It would be nice if we could use dependabot-cli for this (e.g. `dependabot --discover-only`), but this is not supported currently. - // TODO: Open a issue in dependabot-cli project, ask how we should handle this scenario. - warning( - 'Security updates can only be performed if there is a previous dependency list snapshot available, but there is none as you have not completed a successful update job yet. ' + - 'Dependabot does not currently support discovering vulnerable dependencies during security-only updates and it is likely that this update operation will fail.', - ); - - // Attempt to do a security update for "all dependencies"; it will probably fail this is not supported in dependabot-updater yet, but it is best we can do... - return []; - } - - // Return only dependencies that are vulnerable, ignore the rest - const dependencyNames = dependencyList.map((dependency) => dependency['name']); - const dependencyVulnerabilities = {}; // TODO: getGitHubSecurityAdvisoriesForDependencies(dependencyNames); - return dependencyNames.filter((dependency) => dependencyVulnerabilities[dependency]?.length > 0); +function mapSourceFromDependabotConfigToJobConfig(taskInputs: ISharedVariables, update: IDependabotUpdate): any { + const hasMultipleDirectories = update.directories?.length > 1; + return { + 'provider': 'azure', + 'api-endpoint': taskInputs.apiEndpointUrl, + 'hostname': taskInputs.hostname, + 'repo': `${taskInputs.organization}/${taskInputs.project}/_git/${taskInputs.repository}`, + 'branch': update['target-branch'], + 'commit': undefined, // use latest commit of target branch + 'directory': hasMultipleDirectories ? undefined : update.directory || '/', + 'directories': hasMultipleDirectories ? update.directories : undefined, + }; } function mapGroupsFromDependabotConfigToJobConfig(dependencyGroups: Record): any[] { @@ -206,6 +221,24 @@ function mapIgnoreConditionsFromDependabotConfigToJobConfig(ignoreConditions: ID }); } +function mapSecurityAdvisories(securityAdvisories: ISecurityAdvisory[]): any[] { + if (!securityAdvisories) { + return undefined; + } + return securityAdvisories.map((advisory) => { + return { + 'dependency-name': advisory['dependency-name'], + 'affected-versions': advisory['affected-versions'], + 'patched-versions': advisory['patched-versions'], + 'unaffected-versions': advisory['unaffected-versions'], + 'title': advisory['title'], + 'description': advisory['description'], + 'source-name': advisory['source-name'], + 'source-url': advisory['source-url'], + }; + }); +} + function mapVersionStrategyToRequirementsUpdateStrategy(versioningStrategy: string): string | undefined { if (!versioningStrategy) { return undefined; diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/interfaces/IDependabotUpdateJobConfig.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/interfaces/IDependabotUpdateJobConfig.ts index fa8b18e8..81bdbb5e 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/interfaces/IDependabotUpdateJobConfig.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/interfaces/IDependabotUpdateJobConfig.ts @@ -34,7 +34,7 @@ export interface IDependabotUpdateJobConfig { 'updated-at'?: string; 'version-requirement'?: string; }[]; - 'security-updates-only': boolean; + 'security-updates-only'?: boolean; 'security-advisories'?: { 'dependency-name': string; 'affected-versions': string[]; diff --git a/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts b/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts new file mode 100644 index 00000000..dc516b98 --- /dev/null +++ b/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts @@ -0,0 +1,122 @@ +import axios from 'axios'; + +const GITHUB_ADVISORY_GRAPHQL_API = 'https://api.github.com/graphql'; +const GITHUB_ADVISORY_SOURCE_NAME = 'GitHub Advisory Database'; +const GITHUB_ECOSYSTEM: { [key: string]: string } = { + github_actions: 'ACTIONS', + composer: 'COMPOSER', + elm: 'ERLANG', + go_modules: 'GO', + maven: 'MAVEN', + npm_and_yarn: 'NPM', + nuget: 'NUGET', + pip: 'PIP', + pub: 'PUB', + bundler: 'RUBYGEMS', + cargo: 'RUST', + swift: 'SWIFT', +}; + +/** + * Security advisory object + */ +export interface ISecurityAdvisory { + 'dependency-name': string; + 'affected-versions': string[]; + 'patched-versions': string[]; + 'unaffected-versions': string[]; + 'title': string; + 'description': string; + 'source-name': string; + 'source-url': string; +} + +/** + * Get the list of security advisories from the GitHub Security Advisory API using GraphQL + * @param accessToken + * @param packageEcosystem + * @param dependencyNames + */ +export async function getSecurityAdvisories( + accessToken: string, + packageEcosystem: string, + dependencyNames: string[], +): Promise { + const ecosystem = GITHUB_ECOSYSTEM[packageEcosystem]; + const query = ` + query($ecosystem: SecurityAdvisoryEcosystem, $package: String) { + securityVulnerabilities(first: 100, ecosystem: $ecosystem, package: $package) { + nodes { + advisory { + summary, + description, + permalink + } + firstPatchedVersion { + identifier + } + vulnerableVersionRange + } + } + } + `; + + // GitHub API doesn't support querying multiple dependencies at once, so we need to make a request for each dependency individually. + // To speed up the process, we can make the requests in parallel. We batch the requests to avoid hitting the rate limit too quickly. + // https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api + console.log(`Fetching security advisories for ${dependencyNames.length} dependencies...`); + const securityAdvisories = await batch(100, dependencyNames, async (dependencyName) => { + const variables = { + ecosystem: ecosystem, + package: dependencyName, + }; + const response = await axios.post( + GITHUB_ADVISORY_GRAPHQL_API, + JSON.stringify({ + query, + variables, + }), + { + headers: { + 'Authorization': `Bearer ${accessToken}`, + 'Content-Type': 'application/json', + }, + }, + ); + if (response.status < 200 || response.status > 299) { + throw new Error( + `Failed to fetch security advisories for '${dependencyName}': ${response.status} ${response.statusText}`, + ); + } + const vulnerabilities = response.data?.data?.securityVulnerabilities?.nodes; + return vulnerabilities.map((vulnerabilitity: any) => { + return { + 'dependency-name': dependencyName, + 'affected-versions': vulnerabilitity.vulnerableVersionRange + ? [vulnerabilitity.vulnerableVersionRange?.trim()] + : [], + 'patched-versions': vulnerabilitity.firstPatchedVersion + ? [vulnerabilitity.firstPatchedVersion?.identifier] + : [], + 'unaffected-versions': [], + 'title': vulnerabilitity.advisory?.summary, + 'description': vulnerabilitity.advisory?.description, + 'source-name': GITHUB_ADVISORY_SOURCE_NAME, + 'source-url': vulnerabilitity.advisory?.permalink, + }; + }); + }); + + console.log(`Found ${securityAdvisories.length} security advisories`); + return securityAdvisories; +} + +async function batch(batchSize: number, items: string[], callback: (item: string) => Promise) { + const results: ISecurityAdvisory[] = []; + for (let i = 0; i < items.length; i += batchSize) { + const batch = items.slice(i, i + batchSize); + const batchResults = await Promise.all(batch.map(callback)); + results.push(...batchResults.flat()); + } + return results; +} From 229c3f1003438cafd06a0f2e29db83bd5391aa4c Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 8 Oct 2024 14:48:42 +1300 Subject: [PATCH 19/36] Update warning messages --- extension/tasks/dependabotV2/index.ts | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index bd07ec59..1ac9e204 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -94,29 +94,23 @@ async function run() { // that need updating and check each one for security advisories. This is because Dependabot requires the list of vulnerable dependencies // to be supplied in the job definition of security-only update job, it will not automatically discover them like a versioned update does. // https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates#overriding-the-default-behavior-with-a-configuration-file - // TODO: If/when Dependabot supports a better way to do security-only updates, we should remove this code block. let securityAdvisories: ISecurityAdvisory[] = undefined; let dependencyNamesToUpdate: string[] = undefined; const securityUpdatesOnly = update['open-pull-requests-limit'] === 0; if (securityUpdatesOnly) { + // TODO: If and when Dependabot supports a better way to do security-only updates, we should remove this code block. warning( - 'Security-only updates are not yet fully supported by Dependabot CLI. ' + - 'The task will now attempt to discover the dependencies that need updating using an "ignore everything" update job, ' + - 'then check the discovered dependencies for security advisories before finally performing the requested security-only update. ' + - 'Because of this, the task may take longer to complete than usual.', + 'Security-only updates are only partially supported by Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates' + ); + warning( + 'To work around the limitations of Dependabot CLI, vulnerable dependencies will be discovered using an "ignore everything" regular update job. ' + + 'After discovery has completed, security advisories for your dependencies will be checked before finally performing your requested security-only update job. ' + + 'Because of these required extra steps, the task may take longer to complete than usual.', ); const discoveredDependencyListOutputs = await dependabot.update( DependabotJobBuilder.newDiscoverDependencyListJob(taskInputs, updateId, update, dependabotConfig.registries), dependabotUpdaterOptions, ); - dependencyNamesToUpdate = discoveredDependencyListOutputs - ?.find((x) => x.output.type == 'update_dependency_list') - ?.output?.data?.dependencies?.map((d) => d.name); - securityAdvisories = await getSecurityAdvisories( - taskInputs.githubAccessToken, - packageEcosystem, - dependencyNamesToUpdate || [], - ); } // Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating From fe1ac87845141c7fecae8e0dcebb6f0b710d90ab Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Sun, 13 Oct 2024 12:56:20 +1300 Subject: [PATCH 20/36] Fix formatting --- extension/tasks/dependabotV2/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 1ac9e204..e4bd066a 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -9,7 +9,7 @@ import { import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; import parseDependabotConfigFile from './utils/dependabot/parseConfigFile'; import parseTaskInputConfiguration from './utils/getSharedVariables'; -import { getSecurityAdvisories, ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; +import { ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; async function run() { let dependabot: DependabotCli = undefined; @@ -100,7 +100,7 @@ async function run() { if (securityUpdatesOnly) { // TODO: If and when Dependabot supports a better way to do security-only updates, we should remove this code block. warning( - 'Security-only updates are only partially supported by Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates' + 'Security-only updates are only partially supported by Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', ); warning( 'To work around the limitations of Dependabot CLI, vulnerable dependencies will be discovered using an "ignore everything" regular update job. ' + From 951c5381aefe797ac47164fb263da23dbe077845 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Sun, 13 Oct 2024 12:59:38 +1300 Subject: [PATCH 21/36] Update documentation --- README.md | 1 - docs/migrations/v1-to-v2.md | 4 ---- extension/README.md | 2 +- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/README.md b/README.md index cf2c746e..87e23a14 100644 --- a/README.md +++ b/README.md @@ -198,7 +198,6 @@ We aim to support all [official configuration options](https://docs.github.com/e #### `dependabot@V2` - [`schedule`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#scheduleinterval) is ignored, use [pipeline scheduled triggers](https://learn.microsoft.com/en-us/azure/devops/pipelines/process/scheduled-triggers?view=azure-devops&tabs=yaml#scheduled-triggers) instead. -- [Security-only updates](https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates#overriding-the-default-behavior-with-a-configuration-file) (`open-pull-requests-limit: 0`) are not supported. _(coming soon)_ #### `dependabot@V1` - [`schedule`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#scheduleinterval) is ignored, use [pipeline scheduled triggers](https://learn.microsoft.com/en-us/azure/devops/pipelines/process/scheduled-triggers?view=azure-devops&tabs=yaml#scheduled-triggers) instead. diff --git a/docs/migrations/v1-to-v2.md b/docs/migrations/v1-to-v2.md index e28a8e67..8c7a7e91 100644 --- a/docs/migrations/v1-to-v2.md +++ b/docs/migrations/v1-to-v2.md @@ -25,10 +25,6 @@ Dependabot CLI requires [Go](https://go.dev/doc/install) (1.22+) and [Docker](ht If you use [Microsoft-hosted agents](https://learn.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml#software), we recommend using the [ubuntu-latest](https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md) image, which meets all task requirements. For self-hosted agents, you will need to install Go 1.22+. -### Security-only updates and "fixed vulnerabilities" are not implemented (yet) -Using configuration `open-pull-requests-limit: 0` will cause a "not implemented" error. This is [current limitation of V2](../../README.md#unsupported-features-and-configurations). A solution is still under development and is expected to be resolved before general availability. -See: https://github.com/dependabot/cli/issues/360 for more technical details. - ### Task Input `updaterOptions` has been renamed to `experiments` Renamed to match Dependabot Core/CLI terminology. The input value remains unchanged. See [configuring experiments](../../README.md#configuring-experiments) for more details. diff --git a/extension/README.md b/extension/README.md index 06493650..34facf79 100644 --- a/extension/README.md +++ b/extension/README.md @@ -64,7 +64,7 @@ Dependabot uses Docker containers, which may take time to install if not already |azureDevOpsAccessToken|**_Optional_**. The Personal Access Token for accessing Azure DevOps. Supply a value here to avoid using permissions for the Build Service either because you cannot change its permissions or because you prefer that the Pull Requests be done by a different user. When not provided, the current authentication scope is used. In either case, be use the following permissions are granted:
- Code (Full)
- Pull Requests Threads (Read & Write).
See the [documentation](https://docs.microsoft.com/en-us/azure/devops/organizations/accounts/use-personal-access-tokens-to-authenticate?view=azure-devops&tabs=preview-page#create-a-pat) to know more about creating a Personal Access Token.
Use this in place of `azureDevOpsServiceConnection` such as when it is not possible to create a service connection.| |gitHubConnection|**_Optional_**. The GitHub service connection for authenticating requests against GitHub repositories. This is useful to avoid rate limiting errors. The token must include permissions to read public repositories. See the [GitHub docs](https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/creating-a-personal-access-token) for more on Personal Access Tokens and [Azure DevOps docs](https://docs.microsoft.com/en-us/azure/devops/pipelines/library/service-endpoints?view=azure-devops&tabs=yaml#sep-github) for the GitHub service connection.| |gitHubAccessToken|**_Optional_**. The raw GitHub PAT for authenticating requests against GitHub repositories. Use this in place of `gitHubConnection` such as when it is not possible to create a service connection.| -|storeDependencyList|**_Optional_**. Determines if the last know dependency list information should be stored in the parent DevOps project properties. If enabled, the authenticated user must have the "Project & Team (Write)" permission for the project. Enabling this option improves performance when doing security-only updates. Defaults to `false`.| +|storeDependencyList|**_Optional_**. Determines if the last know dependency list information should be stored in the parent DevOps project properties. If enabled, the authenticated user must have the "Project & Team (Write)" permission for the project. Defaults to `false`.| |targetRepositoryName|**_Optional_**. The name of the repository to target for processing. If this value is not supplied then the Build Repository Name is used. Supplying this value allows creation of a single pipeline that runs Dependabot against multiple repositories by running a `dependabot` task for each repository to update.| |targetUpdateIds|**_Optional_**. A semicolon (`;`) delimited list of update identifiers run. Index are zero-based and in the order written in the configuration file. When not present, all the updates are run. This is meant to be used in scenarios where you want to run updates a different times from the same configuration file given you cannot schedule them independently in the pipeline.| |experiments|**_Optional_**. Comma separated list of Dependabot experiments; available options depend on the ecosystem. Example: `tidy=true,vendor=true,goprivate=*`. See: [Configuring experiments](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-experiments)| From daa1ce5bd1b7b0f8289d0956d343ecf6306fe79f Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 15 Oct 2024 10:43:39 +1300 Subject: [PATCH 22/36] Fix merge issues --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index ad15d0c3..3138832a 100644 --- a/README.md +++ b/README.md @@ -179,8 +179,6 @@ Experiments vary depending on the package ecyosystem used; They can be enabled u > To find the latest list of Dependabot experiments, search the `dependabot-core` GitHub repository using queries like ["enabled?(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Fenabled%5CW%5C%28.*%5C%29%2F&type=code) and ["options.fetch(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Foptions%5C.fetch%5C%28.*%2C%2F&type=code). -> To find the latest list of Dependabot experiments, search the `dependabot-core` GitHub repository using queries like ["enabled?(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Fenabled%5CW%5C%28.*%5C%29%2F&type=code) and ["options.fetch(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Foptions%5C.fetch%5C%28.*%2C%2F&type=code). - ## Configuring assignees and reviewers From 3186ba27eee36e15c85b6b62504fb3bd7e23d8b9 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 15 Oct 2024 10:58:33 +1300 Subject: [PATCH 23/36] Fix merge issues --- extension/tasks/dependabotV2/index.ts | 32 ++++++++++++++++++--------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 6e0de360..dbe16cab 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -10,7 +10,7 @@ import { IDependabotUpdateOperationResult } from './utils/dependabot-cli/interfa import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; import parseDependabotConfigFile from './utils/dependabot/parseConfigFile'; import parseTaskInputConfiguration from './utils/getSharedVariables'; -import { ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; +import { getSecurityAdvisories, ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; async function run() { let dependabot: DependabotCli = undefined; @@ -34,6 +34,19 @@ async function run() { throw new Error('Failed to parse dependabot.yaml configuration file from the target repository'); } + // Print a warning about the required workarounds for security-only updates, if any update is configured as such + // TODO: If and when Dependabot supports a better way to do security-only updates, remove this. + if (dependabotConfig.updates?.some((u) => u['open-pull-requests-limit'] === 0)) { + warning( + 'Security-only updates have a performance overhead due to limitations in Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', + ); + warning( + 'To work around the Dependabot CLI limitations, vulnerable dependencies will first be discovered using an "ignore everything" update job. ' + + 'After discovery has completed, security advisories for the dependencies will be checked before finally performing the requested security-only update job. ' + + 'Because of these required extra steps, the task will take longer to complete than usual due to the need to discover dependencies multiple times.', + ); + } + // Initialise the DevOps API clients // There are two clients; one for authoring pull requests and one for auto-approving pull requests (if configured) const prAuthorClient = new AzureDevOpsWebApiClient( @@ -103,19 +116,18 @@ async function run() { let dependencyNamesToUpdate: string[] = undefined; const securityUpdatesOnly = update['open-pull-requests-limit'] === 0; if (securityUpdatesOnly) { - // TODO: If and when Dependabot supports a better way to do security-only updates, we should remove this code block. - warning( - 'Security-only updates are only partially supported by Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', - ); - warning( - 'To work around the limitations of Dependabot CLI, vulnerable dependencies will be discovered using an "ignore everything" regular update job. ' + - 'After discovery has completed, security advisories for your dependencies will be checked before finally performing your requested security-only update job. ' + - 'Because of these required extra steps, the task may take longer to complete than usual.', - ); const discoveredDependencyListOutputs = await dependabot.update( DependabotJobBuilder.newDiscoverDependencyListJob(taskInputs, updateId, update, dependabotConfig.registries), dependabotUpdaterOptions, ); + dependencyNamesToUpdate = discoveredDependencyListOutputs + ?.find((x) => x.output.type == 'update_dependency_list') + ?.output?.data?.dependencies?.map((d) => d.name); + securityAdvisories = await getSecurityAdvisories( + taskInputs.githubAccessToken, + packageEcosystem, + dependencyNamesToUpdate || [], + ); } // Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating From 31861775702581e7818b6c327eb6637c23494adb Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 15 Oct 2024 11:00:38 +1300 Subject: [PATCH 24/36] Fix merge issues --- .../azure-devops/AzureDevOpsWebApiClient.ts | 78 ------------------- 1 file changed, 78 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 73601b9e..77313c9c 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -663,84 +663,6 @@ export class AzureDevOpsWebApiClient { throw e; } } - - private async restApiGet( - url: string, - params?: Record, - apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, - ): Promise { - const queryString = Object.keys(params || {}) - .map((key) => `${key}=${params[key]}`) - .join('&'); - const fullUrl = `${url}?api-version=${apiVersion}${queryString ? `&${queryString}` : ''}`; - return await this.restApiRequest('GET', url, () => - this.connection.rest.client.get(fullUrl, { - Accept: 'application/json', - }), - ); - } - - private async restApiPost( - url: string, - data?: any, - apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, - ): Promise { - const fullUrl = `${url}?api-version=${apiVersion}`; - return await this.restApiRequest('POST', url, () => - this.connection.rest.client.post(fullUrl, JSON.stringify(data), { - 'Content-Type': 'application/json', - }), - ); - } - - private async restApiPut( - url: string, - data?: any, - apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, - ): Promise { - const fullUrl = `${url}?api-version=${apiVersion}`; - return await this.restApiRequest('PUT', url, () => - this.connection.rest.client.put(fullUrl, JSON.stringify(data), { - 'Content-Type': 'application/json', - }), - ); - } - - private async restApiPatch( - url: string, - data?: any, - contentType?: string, - apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, - ): Promise { - const fullUrl = `${url}?api-version=${apiVersion}`; - return await this.restApiRequest('PATCH', url, () => - this.connection.rest.client.patch(fullUrl, JSON.stringify(data), { - 'Content-Type': contentType || 'application/json', - }), - ); - } - - private async restApiRequest( - method: string, - url: string, - request: () => Promise, - ): Promise { - console.debug(`🌎 🠊 [${method}] ${url}`); - const response = await request(); - const body = await response.readBody(); - console.debug(`🌎 🠈 [${response.message.statusCode}] ${response.message.statusMessage}`); - try { - if (response.message.statusCode < 200 || response.message.statusCode > 299) { - throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); - } - return JSON.parse(body); - } catch (e) { - if (body) { - console.debug(body); - } - throw e; - } - } } function normalizeFilePath(path: string): string { From 08f1c3871584012273886d0c98298760b4a5d525 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 15 Oct 2024 11:07:55 +1300 Subject: [PATCH 25/36] Fix merge issues --- .../utils/dependabot-cli/DependabotJobBuilder.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts index 01f2476f..2ffcda9f 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts @@ -67,6 +67,7 @@ export class DependabotJobBuilder { registries, false, undefined, + // TODO: To make this more accurate, we should be evaluating the dependency name AND version against the security advisories, but that requires a lot of complex dependabot-core logic to do correctly securityUpdatesOnly ? dependencyNamesToUpdate?.filter((d) => securityAdvisories?.find((a) => a['dependency-name'] == d)) : dependencyNamesToUpdate, @@ -160,7 +161,6 @@ function buildUpdateJobConfig( } function mapSourceFromDependabotConfigToJobConfig(taskInputs: ISharedVariables, update: IDependabotUpdate): any { - const hasMultipleDirectories = update.directories?.length > 1; return { 'provider': 'azure', 'api-endpoint': taskInputs.apiEndpointUrl, @@ -168,8 +168,8 @@ function mapSourceFromDependabotConfigToJobConfig(taskInputs: ISharedVariables, 'repo': `${taskInputs.organization}/${taskInputs.project}/_git/${taskInputs.repository}`, 'branch': update['target-branch'], 'commit': undefined, // use latest commit of target branch - 'directory': hasMultipleDirectories ? undefined : update.directory || '/', - 'directories': hasMultipleDirectories ? update.directories : undefined, + 'directory': update.directory, + 'directories': update.directories, }; } @@ -231,6 +231,7 @@ function mapSecurityAdvisories(securityAdvisories: ISecurityAdvisory[]): any[] { 'affected-versions': advisory['affected-versions'], 'patched-versions': advisory['patched-versions'], 'unaffected-versions': advisory['unaffected-versions'], + // TODO: These are not currently used by dependabot-core, but should be required so that dependabot can process "fixed vulnerabilities" correctly 'title': advisory['title'], 'description': advisory['description'], 'source-name': advisory['source-name'], From e3987e91879622528cccb10380ecbb5645d612c5 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 22 Oct 2024 13:56:12 +1300 Subject: [PATCH 26/36] Make the security-only update warning more concise --- extension/tasks/dependabotV2/index.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index dbe16cab..9596db73 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -38,12 +38,7 @@ async function run() { // TODO: If and when Dependabot supports a better way to do security-only updates, remove this. if (dependabotConfig.updates?.some((u) => u['open-pull-requests-limit'] === 0)) { warning( - 'Security-only updates have a performance overhead due to limitations in Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', - ); - warning( - 'To work around the Dependabot CLI limitations, vulnerable dependencies will first be discovered using an "ignore everything" update job. ' + - 'After discovery has completed, security advisories for the dependencies will be checked before finally performing the requested security-only update job. ' + - 'Because of these required extra steps, the task will take longer to complete than usual due to the need to discover dependencies multiple times.', + 'Security-only updates have a performance overhead due to the limitations of Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', ); } From 5108b0dd6be94760118f185a9aa13bc081c1edd6 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 22 Oct 2024 14:50:31 +1300 Subject: [PATCH 27/36] Filter out security advisories which do not affected the discovered dependencies during a security-only update --- extension/tasks/dependabotV2/index.ts | 20 +++++--- .../dependabot-cli/DependabotJobBuilder.ts | 7 ++- .../utils/github/getSecurityAdvisories.ts | 47 +++++++++++++++---- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 9596db73..9aa978e2 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -111,24 +111,32 @@ async function run() { let dependencyNamesToUpdate: string[] = undefined; const securityUpdatesOnly = update['open-pull-requests-limit'] === 0; if (securityUpdatesOnly) { + // Run an update job to discover all dependencies const discoveredDependencyListOutputs = await dependabot.update( - DependabotJobBuilder.newDiscoverDependencyListJob(taskInputs, updateId, update, dependabotConfig.registries), + DependabotJobBuilder.listAllDependenciesJob(taskInputs, updateId, update, dependabotConfig.registries), dependabotUpdaterOptions, ); - dependencyNamesToUpdate = discoveredDependencyListOutputs + + // Get the list of security advisories that apply to the discovered dependencies + const dependenciesToCheckForSecurityAdvisories = discoveredDependencyListOutputs ?.find((x) => x.output.type == 'update_dependency_list') - ?.output?.data?.dependencies?.map((d) => d.name); + ?.output?.data?.dependencies?.map((d) => { + d.name, d.version; + }); securityAdvisories = await getSecurityAdvisories( taskInputs.githubAccessToken, packageEcosystem, - dependencyNamesToUpdate || [], + dependenciesToCheckForSecurityAdvisories || [], ); + + // Only update dependencies that have security advisories + dependencyNamesToUpdate = Array.from(new Set(securityAdvisories.map((a) => a['dependency-name']))); } // Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating failedTasks += handleUpdateOperationResults( await dependabot.update( - DependabotJobBuilder.newUpdateAllJob( + DependabotJobBuilder.updateAllDependenciesJob( taskInputs, updateId, update, @@ -148,7 +156,7 @@ async function run() { for (const pullRequestId in existingPullRequests) { failedTasks += handleUpdateOperationResults( await dependabot.update( - DependabotJobBuilder.newUpdatePullRequestJob( + DependabotJobBuilder.updatePullRequestJob( taskInputs, pullRequestId, update, diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts index 2ffcda9f..8fbe996d 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts @@ -19,7 +19,7 @@ export class DependabotJobBuilder { * @param registries * @returns */ - public static newDiscoverDependencyListJob( + public static listAllDependenciesJob( taskInputs: ISharedVariables, id: string, update: IDependabotUpdate, @@ -49,7 +49,7 @@ export class DependabotJobBuilder { * @param securityAdvisories * @returns */ - public static newUpdateAllJob( + public static updateAllDependenciesJob( taskInputs: ISharedVariables, id: string, update: IDependabotUpdate, @@ -67,7 +67,6 @@ export class DependabotJobBuilder { registries, false, undefined, - // TODO: To make this more accurate, we should be evaluating the dependency name AND version against the security advisories, but that requires a lot of complex dependabot-core logic to do correctly securityUpdatesOnly ? dependencyNamesToUpdate?.filter((d) => securityAdvisories?.find((a) => a['dependency-name'] == d)) : dependencyNamesToUpdate, @@ -85,7 +84,7 @@ export class DependabotJobBuilder { * @param pullRequestToUpdate * @returns */ - public static newUpdatePullRequestJob( + public static updatePullRequestJob( taskInputs: ISharedVariables, id: string, update: IDependabotUpdate, diff --git a/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts b/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts index dc516b98..6564704a 100644 --- a/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts +++ b/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts @@ -40,7 +40,7 @@ export interface ISecurityAdvisory { export async function getSecurityAdvisories( accessToken: string, packageEcosystem: string, - dependencyNames: string[], + dependencies: { name: string; version?: string }[], ): Promise { const ecosystem = GITHUB_ECOSYSTEM[packageEcosystem]; const query = ` @@ -62,10 +62,11 @@ export async function getSecurityAdvisories( `; // GitHub API doesn't support querying multiple dependencies at once, so we need to make a request for each dependency individually. - // To speed up the process, we can make the requests in parallel. We batch the requests to avoid hitting the rate limit too quickly. + // To speed up the process, we can make the requests in parallel, 100 at a time. We batch the requests to avoid hitting the rate limit too quickly. // https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api - console.log(`Fetching security advisories for ${dependencyNames.length} dependencies...`); - const securityAdvisories = await batch(100, dependencyNames, async (dependencyName) => { + console.log(`Checking security advisories for ${dependencies.length} dependencies...`); + const dependencyNames = dependencies.map((dependency) => dependency.name); + const securityAdvisories = await batchSecurityAdvisoryQuery(100, dependencyNames, async (dependencyName) => { const variables = { ecosystem: ecosystem, package: dependencyName, @@ -107,16 +108,46 @@ export async function getSecurityAdvisories( }); }); - console.log(`Found ${securityAdvisories.length} security advisories`); - return securityAdvisories; + // Filter out advisories that are not relevant the version of the dependency we are using + const affectedAdvisories = securityAdvisories.filter((advisory) => { + const dependency = dependencies.find((d) => d.name === advisory['dependency-name']); + if (!dependency) { + return false; + } + const isAffected = + advisory['affected-versions'].length > 0 && + advisory['affected-versions'].find((range) => versionIsInRange(dependency.version, range)); + const isPatched = + advisory['patched-versions'].length > 0 && + advisory['patched-versions'].find((range) => versionIsInRange(dependency.version, range)); + const isUnaffected = + advisory['unaffected-versions'].length > 0 && + advisory['unaffected-versions'].find((range) => versionIsInRange(dependency.version, range)); + return (isAffected && !isPatched) || isUnaffected; + }); + + const vulnerableDependencyCount = new Set(affectedAdvisories.map((advisory) => advisory['dependency-name'])).size; + console.log( + `Found ${vulnerableDependencyCount} vulnerable dependencies affected by ${affectedAdvisories.length} security advisories`, + ); + return affectedAdvisories; } -async function batch(batchSize: number, items: string[], callback: (item: string) => Promise) { +async function batchSecurityAdvisoryQuery( + batchSize: number, + items: string[], + action: (item: string) => Promise, +) { const results: ISecurityAdvisory[] = []; for (let i = 0; i < items.length; i += batchSize) { const batch = items.slice(i, i + batchSize); - const batchResults = await Promise.all(batch.map(callback)); + const batchResults = await Promise.all(batch.map(action)); results.push(...batchResults.flat()); } return results; } + +function versionIsInRange(version: string, range: string): boolean { + // TODO: Parse the major/minor/patch version and do a proper comparison, taking in to considerations version ranges (e.g. ">=1.0.0") + return true; +} From 6609b2b8582a775b328f3784307a0860244920f0 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 22 Oct 2024 15:18:47 +1300 Subject: [PATCH 28/36] Update documentation --- README.md | 18 ++++++++++-------- docs/migrations/v1-to-v2.md | 20 +++++++++++--------- extension/tasks/dependabotV2/index.ts | 2 +- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 3138832a..dcf971b9 100644 --- a/README.md +++ b/README.md @@ -23,11 +23,11 @@ In this repository you'll find: - [Configuring experiments](#configuring-experiments) - [Configuring assignees and reviewers](#configuring-assignees-and-reviewers) - [Unsupported features and configurations](#unsupported-features-and-configurations) - * [Extension Task](#extension-task) + * [Dependabot Task](#dependabot-task) + [dependabot@V2](#dependabotv2) + [dependabot@V1](#dependabotv1) - * [Updater Docker image](#updater-docker-image) - * [Server](#server) + * [Dependabot Updater Docker image](#dependabot-updater-docker-image) + * [Dependabot Server](#dependabot-server) - [Migration Guide](#migration-guide) - [Contributing](#contributing) * [Reporting issues and feature requests](#reporting-issues-and-feature-requests) @@ -154,7 +154,7 @@ Experiments vary depending on the package ecyosystem used; They can be enabled u > Dependabot experinment names are not [publicly] documented. For convenience, some known experiments are listed below; However, **be aware that this may be out-of-date at the time of reading.**
-List of known experiments from dependabot-core@0.280.0 +List of known experiments from dependabot-core@0.281.0 |Package Ecosystem|Experiment Name|Value Type|More Information| |--|--|--|--| @@ -166,6 +166,7 @@ Experiments vary depending on the package ecyosystem used; They can be enabled u | All | dependency_change_validation | true/false | https://github.com/dependabot/dependabot-core/pull/9888 | | All | add_deprecation_warn_to_pr_message | true/false | https://github.com/dependabot/dependabot-core/pull/10421 | | All | threaded_metadata | true/false | https://github.com/dependabot/dependabot-core/pull/9485 | +| All | lead_security_dependency | true/false | https://github.com/dependabot/dependabot-core/pull/10727 | | Bundler | bundler_v1_unsupported_error | true/false | https://github.com/dependabot/dependabot-core/pull/10601 | | Composer | composer_v1_deprecation_warning | true/false | https://github.com/dependabot/dependabot-core/pull/10716 | | Composer | composer_v1_unsupported_error | true/false | https://github.com/dependabot/dependabot-core/pull/10716 | @@ -194,12 +195,13 @@ Reviewers can be any of the following values: - Team name ## Unsupported features and configurations -We aim to support all [official configuration options](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file), but there are some limitations for: +We aim to support all [official configuration options](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file), but there are some limitations: -### Extension Task +### Dependabot Task #### `dependabot@V2` - [`schedule`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#scheduleinterval) is ignored, use [pipeline scheduled triggers](https://learn.microsoft.com/en-us/azure/devops/pipelines/process/scheduled-triggers?view=azure-devops&tabs=yaml#scheduled-triggers) instead. +- [`securityAdvisoriesFile`](#configuring-security-advisories-and-known-vulnerabilities) task input is not yet supported. #### `dependabot@V1` - [`schedule`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#scheduleinterval) is ignored, use [pipeline scheduled triggers](https://learn.microsoft.com/en-us/azure/devops/pipelines/process/scheduled-triggers?view=azure-devops&tabs=yaml#scheduled-triggers) instead. @@ -209,11 +211,11 @@ We aim to support all [official configuration options](https://docs.github.com/e - [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids or email addresses; group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. Support is _slightly_ improved when task input `useUpdateScriptVNext: true` is set, but not still not fully supported. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. -### Updater Docker image +### Dependabot Updater Docker Image - `DEPENDABOT_ASSIGNEES` and `DEPENDABOT_REVIEWERS` must be a list of user guids; email addresses and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. -### Server +### Dependabot Server - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are not supported. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are not supported. diff --git a/docs/migrations/v1-to-v2.md b/docs/migrations/v1-to-v2.md index 8c7a7e91..caf24491 100644 --- a/docs/migrations/v1-to-v2.md +++ b/docs/migrations/v1-to-v2.md @@ -20,6 +20,11 @@ The task now uses [Dependabot CLI](https://github.com/dependabot/cli) to perform > [!WARNING] > **It is strongly recommended that you complete (or abandon) all active Depedabot pull requests created in V1 before migrating to V2.** Due to changes in Dependabot dependency metadata, V2 pull requests are not compatible with V1 (and vice versa). Migrating to V2 before completing existing pull requests will lead to duplication of pull requests. +### Security-only updates +Security-only updates (i.e. `open-pull-requests-limit: 0`) incur a performance overhead due to [limitations in Dependabot CLI (#360)](https://github.com/dependabot/cli/issues/360). To work around this, vulnerable dependencies will first be discovered using an "ignore everything" update job; After which, security advisories for the discovered dependencies will be checked against the [GitHub Advisory Database](https://github.com/advisories) before finally performing the requested security-only update job. Because of these required extra steps, the task will take longer to complete than usual. + +Currently the [`securityAdvisoriesFile`](../../README.md#configuring-security-advisories-and-known-vulnerabilities) task input is not supported, but is expected to be supported before GA. + ### New pipeline agent requirements; "Go" must be installed Dependabot CLI requires [Go](https://go.dev/doc/install) (1.22+) and [Docker](https://docs.docker.com/engine/install/) (with Linux containers). If you use [Microsoft-hosted agents](https://learn.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml#software), we recommend using the [ubuntu-latest](https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md) image, which meets all task requirements. @@ -66,13 +71,10 @@ The following environment variables have been removed entirely; the feature is n ## Todo before general availability Before removing the preview flag from V2 `task.json`, we need to: - - [x] Open an issue in Dependabot-CLI, enquire how security-advisories are expected to be provided **before** knowing the list of dependencies. (https://github.com/dependabot/cli/issues/360) - - [ ] Convert GitHub security advisory client in `vulnerabilities.rb` to TypeScript code - - [ ] Implement `security-advisories` config once the answer the above is known - - [x] Review `task.json`, add documentation for new V2 inputs - - [x] Update `\docs\extension.md` with V2 docs - - [x] Update `\extension\README.MD` with V2 docs - - [x] Update `\README.MD` with V2 docs - - [ ] Do a general code tidy-up pass (check all "TODO" comments) + - [ ] Fix PR description text "@���" encoding issues + - [ ] Add "superseded by X" close reason when PR is closed during a PR update + - [ ] Add documentation for required permissions and PAT scopes + - [ ] Add support for 'securityAdvisoriesFile' task input - [ ] Add unit tests for V2 utils scripts - - [ ] Investigate https://zod.dev/ \ No newline at end of file + - [ ] General code tidy-up (check all "TODO" comments) + - [ ] Investigate https://zod.dev/ diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 9aa978e2..2ddea286 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -38,7 +38,7 @@ async function run() { // TODO: If and when Dependabot supports a better way to do security-only updates, remove this. if (dependabotConfig.updates?.some((u) => u['open-pull-requests-limit'] === 0)) { warning( - 'Security-only updates have a performance overhead due to the limitations of Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', + 'Security-only updates incur a performance overhead due to the limitations of Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', ); } From 0934cfe1f27b0260dc4915e80b3b13f78c368563 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 22 Oct 2024 16:38:31 +1300 Subject: [PATCH 29/36] Do not run security-only update when there are no vulnerable dependencies --- extension/tasks/dependabotV2/index.ts | 42 +++++++++++-------- .../utils/github/getSecurityAdvisories.ts | 16 +++++-- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 2ddea286..6641d051 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -10,7 +10,7 @@ import { IDependabotUpdateOperationResult } from './utils/dependabot-cli/interfa import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; import parseDependabotConfigFile from './utils/dependabot/parseConfigFile'; import parseTaskInputConfiguration from './utils/getSharedVariables'; -import { getSecurityAdvisories, ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; +import { getSecurityAdvisories, IDependency, ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; async function run() { let dependabot: DependabotCli = undefined; @@ -118,11 +118,9 @@ async function run() { ); // Get the list of security advisories that apply to the discovered dependencies - const dependenciesToCheckForSecurityAdvisories = discoveredDependencyListOutputs + const dependenciesToCheckForSecurityAdvisories: IDependency[] = discoveredDependencyListOutputs ?.find((x) => x.output.type == 'update_dependency_list') - ?.output?.data?.dependencies?.map((d) => { - d.name, d.version; - }); + ?.output?.data?.dependencies?.map((d) => ({ name: d.name, version: d.version })); securityAdvisories = await getSecurityAdvisories( taskInputs.githubAccessToken, packageEcosystem, @@ -134,20 +132,28 @@ async function run() { } // Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating - failedTasks += handleUpdateOperationResults( - await dependabot.update( - DependabotJobBuilder.updateAllDependenciesJob( - taskInputs, - updateId, - update, - dependabotConfig.registries, - dependencyNamesToUpdate, - existingPullRequestDependencies, - securityAdvisories, + const dependenciesHaveVulnerabilities = (dependencyNamesToUpdate.length && securityAdvisories.length); + if (!securityUpdatesOnly || dependenciesHaveVulnerabilities) + { + failedTasks += handleUpdateOperationResults( + await dependabot.update( + DependabotJobBuilder.updateAllDependenciesJob( + taskInputs, + updateId, + update, + dependabotConfig.registries, + dependencyNamesToUpdate, + existingPullRequestDependencies, + securityAdvisories, + ), + dependabotUpdaterOptions, ), - dependabotUpdaterOptions, - ), - ); + ); + } + else + { + console.info('Nothing to update; dependencies are not affected by any known vulnerability') + } // If there are existing pull requests, run an update job for each one; this will resolve merge conflicts and close pull requests that are no longer needed const numberOfPullRequestsToUpdate = Object.keys(existingPullRequests).length; diff --git a/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts b/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts index 6564704a..bf706a7b 100644 --- a/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts +++ b/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts @@ -17,6 +17,14 @@ const GITHUB_ECOSYSTEM: { [key: string]: string } = { swift: 'SWIFT', }; +/** + * Dependency object + */ +export interface IDependency { + name: string; + version?: string; +} + /** * Security advisory object */ @@ -40,7 +48,7 @@ export interface ISecurityAdvisory { export async function getSecurityAdvisories( accessToken: string, packageEcosystem: string, - dependencies: { name: string; version?: string }[], + dependencies: IDependency[], ): Promise { const ecosystem = GITHUB_ECOSYSTEM[packageEcosystem]; const query = ` @@ -64,7 +72,7 @@ export async function getSecurityAdvisories( // GitHub API doesn't support querying multiple dependencies at once, so we need to make a request for each dependency individually. // To speed up the process, we can make the requests in parallel, 100 at a time. We batch the requests to avoid hitting the rate limit too quickly. // https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api - console.log(`Checking security advisories for ${dependencies.length} dependencies...`); + console.info(`Checking security advisories for ${dependencies.length} dependencies...`); const dependencyNames = dependencies.map((dependency) => dependency.name); const securityAdvisories = await batchSecurityAdvisoryQuery(100, dependencyNames, async (dependencyName) => { const variables = { @@ -127,8 +135,8 @@ export async function getSecurityAdvisories( }); const vulnerableDependencyCount = new Set(affectedAdvisories.map((advisory) => advisory['dependency-name'])).size; - console.log( - `Found ${vulnerableDependencyCount} vulnerable dependencies affected by ${affectedAdvisories.length} security advisories`, + console.info( + `Found ${affectedAdvisories.length} vulnerabilities; affecting ${vulnerableDependencyCount} dependencies`, ); return affectedAdvisories; } From 1a037904bf6f6aca7fc93a00479013fa3dbf4eaf Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 22 Oct 2024 23:48:24 +1300 Subject: [PATCH 30/36] Fix merge issues --- extension/tasks/dependabotV2/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 4da65e70..06a36f25 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -136,8 +136,8 @@ async function run() { // that need updating and check each one for security advisories. This is because Dependabot requires the list of vulnerable dependencies // to be supplied in the job definition of security-only update job, it will not automatically discover them like a versioned update does. // https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates#overriding-the-default-behavior-with-a-configuration-file - let securityAdvisories: ISecurityAdvisory[] = undefined; - let dependencyNamesToUpdate: string[] = undefined; + let securityAdvisories: ISecurityAdvisory[] = []; + let dependencyNamesToUpdate: string[] = []; const securityUpdatesOnly = update['open-pull-requests-limit'] === 0; if (securityUpdatesOnly) { // Run an update job to discover all dependencies From 6befe6b1e0e28347e5ed819ffb021ad3e741691e Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 2 Dec 2024 23:42:00 +1300 Subject: [PATCH 31/36] Clean up --- docs/migrations/v1-to-v2.md | 5 ++--- extension/tasks/dependabotV2/index.ts | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/migrations/v1-to-v2.md b/docs/migrations/v1-to-v2.md index caf24491..620e7242 100644 --- a/docs/migrations/v1-to-v2.md +++ b/docs/migrations/v1-to-v2.md @@ -21,9 +21,9 @@ The task now uses [Dependabot CLI](https://github.com/dependabot/cli) to perform > **It is strongly recommended that you complete (or abandon) all active Depedabot pull requests created in V1 before migrating to V2.** Due to changes in Dependabot dependency metadata, V2 pull requests are not compatible with V1 (and vice versa). Migrating to V2 before completing existing pull requests will lead to duplication of pull requests. ### Security-only updates -Security-only updates (i.e. `open-pull-requests-limit: 0`) incur a performance overhead due to [limitations in Dependabot CLI (#360)](https://github.com/dependabot/cli/issues/360). To work around this, vulnerable dependencies will first be discovered using an "ignore everything" update job; After which, security advisories for the discovered dependencies will be checked against the [GitHub Advisory Database](https://github.com/advisories) before finally performing the requested security-only update job. Because of these required extra steps, the task will take longer to complete than usual. +Security-only updates (i.e. `open-pull-requests-limit: 0`) incur a slight performance overhead due to limitations in Dependabot CLI, detailed in [dependabot/cli#360](https://github.com/dependabot/cli/issues/360). To work around this, vulnerable dependencies will first be discovered using an "ignore everything" update job; After which, security advisories for the discovered dependencies will be checked against the [GitHub Advisory Database](https://github.com/advisories) before finally performing the requested security-only update job. -Currently the [`securityAdvisoriesFile`](../../README.md#configuring-security-advisories-and-known-vulnerabilities) task input is not supported, but is expected to be supported before GA. +Currently the [`securityAdvisoriesFile`](../../README.md#configuring-security-advisories-and-known-vulnerabilities) task input is not supported, but is expected to be supported in the near future. ### New pipeline agent requirements; "Go" must be installed Dependabot CLI requires [Go](https://go.dev/doc/install) (1.22+) and [Docker](https://docs.docker.com/engine/install/) (with Linux containers). @@ -71,7 +71,6 @@ The following environment variables have been removed entirely; the feature is n ## Todo before general availability Before removing the preview flag from V2 `task.json`, we need to: - - [ ] Fix PR description text "@���" encoding issues - [ ] Add "superseded by X" close reason when PR is closed during a PR update - [ ] Add documentation for required permissions and PAT scopes - [ ] Add support for 'securityAdvisoriesFile' task input diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 9e2ef80a..2aa55dc5 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -55,7 +55,7 @@ async function run() { // TODO: If and when Dependabot supports a better way to do security-only updates, remove this. if (dependabotConfig.updates?.some((u) => u['open-pull-requests-limit'] === 0)) { warning( - 'Security-only updates incur a performance overhead due to the limitations of Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', + 'Security-only updates incur a slight performance overhead due to limitations in Dependabot CLI. For more info, see: https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md#security-only-updates', ); } From e170acaa6b3c7c2d8d6b6516a0ba5e0e245d5b86 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 3 Dec 2024 00:42:22 +1300 Subject: [PATCH 32/36] Improved GHSA security advisory parsing --- extension/package-lock.json | 78 +++---- extension/package.json | 4 +- extension/tasks/dependabotV2/index.ts | 33 +-- .../dependabot-cli/DependabotJobBuilder.ts | 44 ++-- .../utils/github/GitHubGraphClient.ts | 202 ++++++++++++++++++ .../dependabotV2/utils/github/IPackage.ts | 4 + .../utils/github/ISecurityAdvisory.ts | 39 ++++ .../utils/github/ISecurityVulnerability.ts | 11 + .../utils/github/PackageEcosystem.ts | 47 ++++ .../utils/github/getSecurityAdvisories.ts | 161 -------------- 10 files changed, 374 insertions(+), 249 deletions(-) create mode 100644 extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts create mode 100644 extension/tasks/dependabotV2/utils/github/IPackage.ts create mode 100644 extension/tasks/dependabotV2/utils/github/ISecurityAdvisory.ts create mode 100644 extension/tasks/dependabotV2/utils/github/ISecurityVulnerability.ts create mode 100644 extension/tasks/dependabotV2/utils/github/PackageEcosystem.ts delete mode 100644 extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts diff --git a/extension/package-lock.json b/extension/package-lock.json index e284c4d2..0c661a7a 100644 --- a/extension/package-lock.json +++ b/extension/package-lock.json @@ -12,13 +12,15 @@ "axios": "1.7.8", "azure-devops-node-api": "14.1.0", "azure-pipelines-task-lib": "4.17.3", - "js-yaml": "4.1.0" + "js-yaml": "4.1.0", + "semver": "7.6.3" }, "devDependencies": { "@types/jest": "29.5.14", "@types/js-yaml": "4.0.9", "@types/node": "22.10.0", "@types/q": "1.5.8", + "@types/semver": "7.5.8", "jest": "29.7.0", "ts-jest": "29.2.5", "ts-node": "10.9.2", @@ -1163,6 +1165,12 @@ "integrity": "sha512-hroOstUScF6zhIi+5+x0dzqrHA1EJi+Irri6b1fxolMTqqHIV/Cg77EtnQcZqZCu8hR3mX2BzIxN4/GzI68Kfw==", "dev": true }, + "node_modules/@types/semver": { + "version": "7.5.8", + "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.5.8.tgz", + "integrity": "sha512-I8EUhyrgfLrcTkzV3TSsGyl1tSuPrEDzr0yd5m90UgNxQkyDXULk3b6MlQqTCpZpNtWe1K0hzclnZkTcLBe2UQ==", + "dev": true + }, "node_modules/@types/stack-utils": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz", @@ -1341,6 +1349,14 @@ "uuid": "^3.0.1" } }, + "node_modules/azure-pipelines-task-lib/node_modules/semver": { + "version": "5.7.2", + "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.2.tgz", + "integrity": "sha512-cBznnQ9KjJqU67B52RMC65CMarK2600WFnbkcaiwWq3xy/5haFJlshgnpjovMVJ+Hff49d8GEn0b87C5pDQ10g==", + "bin": { + "semver": "bin/semver" + } + }, "node_modules/babel-jest": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/babel-jest/-/babel-jest-29.7.0.tgz", @@ -2475,18 +2491,6 @@ "node": ">=10" } }, - "node_modules/istanbul-lib-instrument/node_modules/semver": { - "version": "7.6.2", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.6.2.tgz", - "integrity": "sha512-FNAIBWCx9qcRhoHcgcJ0gvU7SN1lYU2ZXuSfl04bSC5OpvDHFyJCjdNHomPXxjQlCBU67YW64PzY7/VIEH7F2w==", - "dev": true, - "bin": { - "semver": "bin/semver.js" - }, - "engines": { - "node": ">=10" - } - }, "node_modules/istanbul-lib-report": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/istanbul-lib-report/-/istanbul-lib-report-3.0.1.tgz", @@ -3020,18 +3024,6 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, - "node_modules/jest-snapshot/node_modules/semver": { - "version": "7.6.2", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.6.2.tgz", - "integrity": "sha512-FNAIBWCx9qcRhoHcgcJ0gvU7SN1lYU2ZXuSfl04bSC5OpvDHFyJCjdNHomPXxjQlCBU67YW64PzY7/VIEH7F2w==", - "dev": true, - "bin": { - "semver": "bin/semver.js" - }, - "engines": { - "node": ">=10" - } - }, "node_modules/jest-util": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-util/-/jest-util-29.7.0.tgz", @@ -3246,18 +3238,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/make-dir/node_modules/semver": { - "version": "7.6.2", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.6.2.tgz", - "integrity": "sha512-FNAIBWCx9qcRhoHcgcJ0gvU7SN1lYU2ZXuSfl04bSC5OpvDHFyJCjdNHomPXxjQlCBU67YW64PzY7/VIEH7F2w==", - "dev": true, - "bin": { - "semver": "bin/semver.js" - }, - "engines": { - "node": ">=10" - } - }, "node_modules/make-error": { "version": "1.3.6", "resolved": "https://registry.npmjs.org/make-error/-/make-error-1.3.6.tgz", @@ -3732,11 +3712,14 @@ } }, "node_modules/semver": { - "version": "5.7.2", - "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.2.tgz", - "integrity": "sha512-cBznnQ9KjJqU67B52RMC65CMarK2600WFnbkcaiwWq3xy/5haFJlshgnpjovMVJ+Hff49d8GEn0b87C5pDQ10g==", + "version": "7.6.3", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.6.3.tgz", + "integrity": "sha512-oVekP1cKtI+CTDvHWYFUcMtsK/00wmAEfyqKfNdARm8u1wNVhSgaX7A8d4UuIlUI5e84iEwOhs7ZPYRmzU9U6A==", "bin": { - "semver": "bin/semver" + "semver": "bin/semver.js" + }, + "engines": { + "node": ">=10" } }, "node_modules/set-function-length": { @@ -4059,19 +4042,6 @@ } } }, - "node_modules/ts-jest/node_modules/semver": { - "version": "7.6.3", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.6.3.tgz", - "integrity": "sha512-oVekP1cKtI+CTDvHWYFUcMtsK/00wmAEfyqKfNdARm8u1wNVhSgaX7A8d4UuIlUI5e84iEwOhs7ZPYRmzU9U6A==", - "dev": true, - "license": "ISC", - "bin": { - "semver": "bin/semver.js" - }, - "engines": { - "node": ">=10" - } - }, "node_modules/ts-node": { "version": "10.9.2", "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-10.9.2.tgz", diff --git a/extension/package.json b/extension/package.json index f3ad99d8..17213e26 100644 --- a/extension/package.json +++ b/extension/package.json @@ -31,13 +31,15 @@ "axios": "1.7.8", "azure-devops-node-api": "14.1.0", "azure-pipelines-task-lib": "4.17.3", - "js-yaml": "4.1.0" + "js-yaml": "4.1.0", + "semver": "7.6.3" }, "devDependencies": { "@types/jest": "29.5.14", "@types/js-yaml": "4.0.9", "@types/node": "22.10.0", "@types/q": "1.5.8", + "@types/semver": "7.5.8", "jest": "29.7.0", "ts-jest": "29.2.5", "ts-node": "10.9.2", diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 2aa55dc5..5fe153ff 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -11,7 +11,10 @@ import { IDependabotUpdateOperationResult } from './utils/dependabot-cli/interfa import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; import parseDependabotConfigFile from './utils/dependabot/parseConfigFile'; import parseTaskInputConfiguration from './utils/getSharedVariables'; -import { getSecurityAdvisories, IDependency, ISecurityAdvisory } from './utils/github/getSecurityAdvisories'; +import { GitHubGraphClient } from './utils/github/GitHubGraphClient'; +import { IPackage } from './utils/github/IPackage'; +import { ISecurityVulnerability } from './utils/github/ISecurityVulnerability'; +import { getGhsaPackageEcosystemFromDependabotPackageEcosystem } from './utils/github/PackageEcosystem'; async function run() { let dependabot: DependabotCli = undefined; @@ -135,10 +138,10 @@ async function run() { ).map(([id, deps]) => deps); // If this is a security-only update (i.e. 'open-pull-requests-limit: 0'), then we first need to discover the dependencies - // that need updating and check each one for security advisories. This is because Dependabot requires the list of vulnerable dependencies + // that need updating and check each one for vulnerabilities. This is because Dependabot requires the list of vulnerable dependencies // to be supplied in the job definition of security-only update job, it will not automatically discover them like a versioned update does. // https://docs.github.com/en/code-security/dependabot/dependabot-security-updates/configuring-dependabot-security-updates#overriding-the-default-behavior-with-a-configuration-file - let securityAdvisories: ISecurityAdvisory[] = []; + let securityVulnerabilities: ISecurityVulnerability[] = []; let dependencyNamesToUpdate: string[] = []; const securityUpdatesOnly = update['open-pull-requests-limit'] === 0; if (securityUpdatesOnly) { @@ -148,18 +151,20 @@ async function run() { dependabotUpdaterOptions, ); - // Get the list of security advisories that apply to the discovered dependencies - const dependenciesToCheckForSecurityAdvisories: IDependency[] = discoveredDependencyListOutputs + // Get the list of vulnerabilities that apply to the discovered dependencies + const ghsaClient = new GitHubGraphClient(taskInputs.githubAccessToken); + const packagesToCheckForVulnerabilities: IPackage[] = discoveredDependencyListOutputs ?.find((x) => x.output.type == 'update_dependency_list') ?.output?.data?.dependencies?.map((d) => ({ name: d.name, version: d.version })); - securityAdvisories = await getSecurityAdvisories( - taskInputs.githubAccessToken, - packageEcosystem, - dependenciesToCheckForSecurityAdvisories || [], - ); + if (packagesToCheckForVulnerabilities?.length) { + securityVulnerabilities = await ghsaClient.getSecurityVulnerabilitiesAsync( + getGhsaPackageEcosystemFromDependabotPackageEcosystem(packageEcosystem), + packagesToCheckForVulnerabilities || [], + ); - // Only update dependencies that have security advisories - dependencyNamesToUpdate = Array.from(new Set(securityAdvisories.map((a) => a['dependency-name']))); + // Only update dependencies that have vulnerabilities + dependencyNamesToUpdate = Array.from(new Set(securityVulnerabilities.map((v) => v.package.name))); + } } // Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating @@ -168,7 +173,7 @@ async function run() { const hasReachedOpenPullRequestLimit = openPullRequestsLimit > 0 && openPullRequestsCount >= openPullRequestsLimit; if (!hasReachedOpenPullRequestLimit) { - const dependenciesHaveVulnerabilities = dependencyNamesToUpdate.length && securityAdvisories.length; + const dependenciesHaveVulnerabilities = dependencyNamesToUpdate.length && securityVulnerabilities.length; if (!securityUpdatesOnly || dependenciesHaveVulnerabilities) { failedTasks += handleUpdateOperationResults( await dependabot.update( @@ -179,7 +184,7 @@ async function run() { dependabotConfig.registries, dependencyNamesToUpdate, existingPullRequestDependenciesForPackageEcosystem, - securityAdvisories, + securityVulnerabilities, ), dependabotUpdaterOptions, ), diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts index 8fbe996d..6b5dfc7b 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts @@ -5,7 +5,7 @@ import { IDependabotUpdate, } from '../dependabot/interfaces/IDependabotConfig'; import { ISharedVariables } from '../getSharedVariables'; -import { ISecurityAdvisory } from '../github/getSecurityAdvisories'; +import { ISecurityVulnerability } from '../github/ISecurityVulnerability'; import { IDependabotUpdateOperation } from './interfaces/IDependabotUpdateOperation'; /** @@ -46,7 +46,7 @@ export class DependabotJobBuilder { * @param registries * @param dependencyNamesToUpdate * @param existingPullRequests - * @param securityAdvisories + * @param securityVulnerabilities * @returns */ public static updateAllDependenciesJob( @@ -56,7 +56,7 @@ export class DependabotJobBuilder { registries: Record, dependencyNamesToUpdate?: string[], existingPullRequests?: any[], - securityAdvisories?: ISecurityAdvisory[], + securityVulnerabilities?: ISecurityVulnerability[], ): IDependabotUpdateOperation { const packageEcosystem = update['package-ecosystem']; const securityUpdatesOnly = update['open-pull-requests-limit'] == 0; @@ -68,10 +68,10 @@ export class DependabotJobBuilder { false, undefined, securityUpdatesOnly - ? dependencyNamesToUpdate?.filter((d) => securityAdvisories?.find((a) => a['dependency-name'] == d)) + ? dependencyNamesToUpdate?.filter((d) => securityVulnerabilities?.find((v) => v.package.name == d)) : dependencyNamesToUpdate, existingPullRequests, - securityAdvisories, + securityVulnerabilities, ); } @@ -118,7 +118,7 @@ function buildUpdateJobConfig( updateDependencyGroupName?: string | undefined, updateDependencyNames?: string[] | undefined, existingPullRequests?: any[], - securityAdvisories?: ISecurityAdvisory[], + securityVulnerabilities?: ISecurityVulnerability[], ) { return { config: update, @@ -133,7 +133,7 @@ function buildUpdateJobConfig( 'allowed-updates': mapAllowedUpdatesFromDependabotConfigToJobConfig(update.allow), 'ignore-conditions': mapIgnoreConditionsFromDependabotConfigToJobConfig(update.ignore), 'security-updates-only': update['open-pull-requests-limit'] == 0, - 'security-advisories': mapSecurityAdvisories(securityAdvisories), + 'security-advisories': mapSecurityAdvisories(securityVulnerabilities), 'source': mapSourceFromDependabotConfigToJobConfig(taskInputs, update), 'existing-pull-requests': existingPullRequests?.filter((pr) => !pr['dependency-group-name']), 'existing-group-pull-requests': existingPullRequests?.filter((pr) => pr['dependency-group-name']), @@ -220,21 +220,27 @@ function mapIgnoreConditionsFromDependabotConfigToJobConfig(ignoreConditions: ID }); } -function mapSecurityAdvisories(securityAdvisories: ISecurityAdvisory[]): any[] { - if (!securityAdvisories) { +function mapSecurityAdvisories(securityVulnerabilities: ISecurityVulnerability[]): any[] { + if (!securityVulnerabilities) { return undefined; } - return securityAdvisories.map((advisory) => { + + // A single security advisory can cause a vulnerability in multiple versions of a package. + // We need to map each unique security advisory to a list of affected-versions and patched-versions. + const vulnerabilitiesGroupedByPackageNameAndAdvisoryId = new Map(); + for (const vuln of securityVulnerabilities) { + const key = `${vuln.package.name}/${vuln.advisory.identifiers.map((i) => `${i.type}:${i.value}`).join('/')}`; + if (!vulnerabilitiesGroupedByPackageNameAndAdvisoryId.has(key)) { + vulnerabilitiesGroupedByPackageNameAndAdvisoryId.set(key, []); + } + vulnerabilitiesGroupedByPackageNameAndAdvisoryId.get(key).push(vuln); + } + return Array.from(vulnerabilitiesGroupedByPackageNameAndAdvisoryId.values()).map((vulns) => { return { - 'dependency-name': advisory['dependency-name'], - 'affected-versions': advisory['affected-versions'], - 'patched-versions': advisory['patched-versions'], - 'unaffected-versions': advisory['unaffected-versions'], - // TODO: These are not currently used by dependabot-core, but should be required so that dependabot can process "fixed vulnerabilities" correctly - 'title': advisory['title'], - 'description': advisory['description'], - 'source-name': advisory['source-name'], - 'source-url': advisory['source-url'], + 'dependency-name': vulns[0].package.name, + 'affected-versions': vulns.map((v) => v.vulnerableVersionRange).filter((v) => v && v.length > 0), + 'patched-versions': vulns.map((v) => v.firstPatchedVersion).filter((v) => v && v.length > 0), + 'unaffected-versions': [], }; }); } diff --git a/extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts b/extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts new file mode 100644 index 00000000..9df10047 --- /dev/null +++ b/extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts @@ -0,0 +1,202 @@ +import axios from 'axios'; +import * as semver from 'semver'; + +import { IPackage } from './IPackage'; +import { ISecurityVulnerability } from './ISecurityVulnerability'; +import { PackageEcosystem } from './PackageEcosystem'; + +const GHSA_GRAPHQL_API = 'https://api.github.com/graphql'; + +const GHSA_SECURITY_VULNERABILITIES_QUERY = ` + query($ecosystem: SecurityAdvisoryEcosystem, $package: String) { + securityVulnerabilities(first: 100, ecosystem: $ecosystem, package: $package) { + nodes { + advisory { + identifiers { + type, + value + }, + severity, + summary, + description, + references { + url + } + cvss { + score + vectorString + } + epss { + percentage + percentile + } + cwes (first: 100) { + nodes { + cweId + name + description + } + } + publishedAt + updatedAt + withdrawnAt + permalink + } + vulnerableVersionRange + firstPatchedVersion { + identifier + } + } + } + } +`; + +/** + * GitHub GraphQL client + */ +export class GitHubGraphClient { + private readonly accessToken: string; + + constructor(accessToken: string) { + this.accessToken = accessToken; + } + + /** + * Get the list of security vulnerabilities for a given package ecosystem and list of packages + * @param packageEcosystem + * @param packages + */ + public async getSecurityVulnerabilitiesAsync( + packageEcosystem: PackageEcosystem, + packages: IPackage[], + ): Promise { + // GitHub API doesn't support querying multiple package at once, so we need to make a request for each package individually. + // To speed up the process, we can make the requests in parallel, 100 at a time. We batch the requests to avoid hitting the rate limit too quickly. + // https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api + const securityVulnerabilities = await this.batchGraphQueryAsync( + 100, + packages, + async (pkg) => { + const variables = { + ecosystem: packageEcosystem, + package: pkg.name, + }; + const response = await axios.post( + GHSA_GRAPHQL_API, + JSON.stringify({ + query: GHSA_SECURITY_VULNERABILITIES_QUERY, + variables: variables, + }), + { + headers: { + 'Authorization': `Bearer ${this.accessToken}`, + 'Content-Type': 'application/json', + }, + }, + ); + if (response.status < 200 || response.status > 299) { + throw new Error(`GHSA GraphQL request failed with response: ${response.status} ${response.statusText}`); + } + const errors = response.data?.errors; + if (errors) { + throw new Error(`GHSA GraphQL request failed with errors: ${JSON.stringify(errors)}`); + } + + const vulnerabilities = response.data?.data?.securityVulnerabilities?.nodes; + return vulnerabilities + ?.filter((v: any) => v?.advisory) + ?.map((v: any) => { + return { + ecosystem: packageEcosystem, + package: pkg, + advisory: { + identifiers: v.advisory.identifiers?.map((i: any) => { + return { + type: i.type, + value: i.value, + }; + }), + severity: v.advisory.severity, + summary: v.advisory.summary, + description: v.advisory.description, + references: v.advisory.references?.map((r: any) => r.url), + cvss: !v.advisory.cvss + ? undefined + : { + score: v.advisory.cvss.score, + vectorString: v.advisory.cvss.vectorString, + }, + epss: !v.advisory.epss + ? undefined + : { + percentage: v.advisory.epss.percentage, + percentile: v.advisory.epss.percentile, + }, + cwes: v.advisory.cwes?.nodes?.map((c: any) => { + return { + id: c.cweId, + name: c.name, + description: c.description, + }; + }), + publishedAt: v.advisory.publishedAt, + updatedAt: v.advisory.updatedAt, + withdrawnAt: v.advisory.withdrawnAt, + permalink: v.advisory.permalink, + }, + vulnerableVersionRange: v.vulnerableVersionRange, + firstPatchedVersion: v.firstPatchedVersion?.identifier, + }; + }); + }, + ); + + // Filter out vulnerabilities that have been withdrawn or that are not relevant the current version of the package + const affectedVulnerabilities = securityVulnerabilities + .filter((v) => !v.advisory.withdrawnAt) + .filter((v) => { + const pkg = v.package; + if (!pkg || !pkg.version || !v.vulnerableVersionRange) { + return false; + } + + /** + * The vulnerable version range follows a basic syntax with a few forms: + * `= 0.2.0` denotes a single vulnerable version + * `<= 1.0.8` denotes a version range up to and including the specified version + * `< 0.1.11` denotes a version range up to, but excluding, the specified version + * `>= 4.3.0, < 4.3.5` denotes a version range with a known minimum and maximum version + * `>= 0.0.1` denotes a version range with a known minimum, but no known maximum + */ + const versionRangeRequirements = v.vulnerableVersionRange.split(',').map((v) => v.trim()); + return versionRangeRequirements.every((r) => pkg.version && semver.satisfies(pkg.version, r)); + }); + + return affectedVulnerabilities; + } + + /** + * Batch requests in parallel to speed up the process when we are forced to do a N+1 query + * @param batchSize + * @param items + * @param action + * @returns + */ + private async batchGraphQueryAsync(batchSize: number, items: T1[], action: (item: T1) => Promise) { + const results: T2[] = []; + for (let i = 0; i < items.length; i += batchSize) { + const batch = items.slice(i, i + batchSize); + if (batch?.length) { + try { + const batchResults = await Promise.all(batch.map(action)); + if (batchResults?.length) { + results.push(...batchResults.flat()); + } + } catch (error) { + console.warn(`Request batch [${i}-${i + batchSize}] failed; The data may be incomplete. ${error}`); + } + } + } + return results; + } +} diff --git a/extension/tasks/dependabotV2/utils/github/IPackage.ts b/extension/tasks/dependabotV2/utils/github/IPackage.ts new file mode 100644 index 00000000..af519517 --- /dev/null +++ b/extension/tasks/dependabotV2/utils/github/IPackage.ts @@ -0,0 +1,4 @@ +export interface IPackage { + name: string; + version?: string; +} diff --git a/extension/tasks/dependabotV2/utils/github/ISecurityAdvisory.ts b/extension/tasks/dependabotV2/utils/github/ISecurityAdvisory.ts new file mode 100644 index 00000000..96e0c43a --- /dev/null +++ b/extension/tasks/dependabotV2/utils/github/ISecurityAdvisory.ts @@ -0,0 +1,39 @@ +export interface ISecurityAdvisory { + identifiers: { + type: SecurityAdvisoryIdentifierType | string; + value: string; + }[]; + severity: SecurityAdvisorySeverity; + summary: string; + description: string; + references: string[]; + cvss: { + score: number; + vectorString: string; + }; + epss: { + percentage: number; + percentile: number; + }; + cwes: { + id: string; + name: string; + description: string; + }[]; + publishedAt: string; + updatedAt: string; + withdrawnAt: string; + permalink: string; +} + +export enum SecurityAdvisoryIdentifierType { + Cve = 'CVE', + Ghsa = 'GHSA', +} + +export enum SecurityAdvisorySeverity { + Low = 'LOW', + Moderate = 'MODERATE', + High = 'HIGH', + Critical = 'CRITICAL', +} diff --git a/extension/tasks/dependabotV2/utils/github/ISecurityVulnerability.ts b/extension/tasks/dependabotV2/utils/github/ISecurityVulnerability.ts new file mode 100644 index 00000000..e6640425 --- /dev/null +++ b/extension/tasks/dependabotV2/utils/github/ISecurityVulnerability.ts @@ -0,0 +1,11 @@ +import { IPackage } from './IPackage'; +import { ISecurityAdvisory } from './ISecurityAdvisory'; +import { PackageEcosystem } from './PackageEcosystem'; + +export interface ISecurityVulnerability { + ecosystem: PackageEcosystem; + package: IPackage; + advisory: ISecurityAdvisory; + vulnerableVersionRange: string; + firstPatchedVersion: string; +} diff --git a/extension/tasks/dependabotV2/utils/github/PackageEcosystem.ts b/extension/tasks/dependabotV2/utils/github/PackageEcosystem.ts new file mode 100644 index 00000000..8b129543 --- /dev/null +++ b/extension/tasks/dependabotV2/utils/github/PackageEcosystem.ts @@ -0,0 +1,47 @@ +export enum PackageEcosystem { + Composer = 'COMPOSER', + Erlang = 'ERLANG', + Actions = 'ACTIONS', + Go = 'GO', + Maven = 'MAVEN', + Npm = 'NPM', + Nuget = 'NUGET', + Pip = 'PIP', + Pub = 'PUB', + Rubygems = 'RUBYGEMS', + Rust = 'RUST', + Swift = 'SWIFT', +} + +export function getGhsaPackageEcosystemFromDependabotPackageEcosystem( + dependabotPackageEcosystem: string, +): PackageEcosystem { + switch (dependabotPackageEcosystem) { + case 'composer': + return PackageEcosystem.Composer; + case 'elm': + return PackageEcosystem.Erlang; + case 'github_actions': + return PackageEcosystem.Actions; + case 'go_modules': + return PackageEcosystem.Go; + case 'maven': + return PackageEcosystem.Maven; + case 'npm_and_yarn': + return PackageEcosystem.Npm; + case 'nuget': + return PackageEcosystem.Nuget; + case 'pip': + return PackageEcosystem.Pip; + case 'pub': + return PackageEcosystem.Pub; + case 'bundler': + return PackageEcosystem.Rubygems; + case 'cargo': + return PackageEcosystem.Rust; + case 'swift': + return PackageEcosystem.Swift; + default: + throw new Error(`Unknown dependabot package ecosystem: ${dependabotPackageEcosystem}`); + } +} diff --git a/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts b/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts deleted file mode 100644 index bf706a7b..00000000 --- a/extension/tasks/dependabotV2/utils/github/getSecurityAdvisories.ts +++ /dev/null @@ -1,161 +0,0 @@ -import axios from 'axios'; - -const GITHUB_ADVISORY_GRAPHQL_API = 'https://api.github.com/graphql'; -const GITHUB_ADVISORY_SOURCE_NAME = 'GitHub Advisory Database'; -const GITHUB_ECOSYSTEM: { [key: string]: string } = { - github_actions: 'ACTIONS', - composer: 'COMPOSER', - elm: 'ERLANG', - go_modules: 'GO', - maven: 'MAVEN', - npm_and_yarn: 'NPM', - nuget: 'NUGET', - pip: 'PIP', - pub: 'PUB', - bundler: 'RUBYGEMS', - cargo: 'RUST', - swift: 'SWIFT', -}; - -/** - * Dependency object - */ -export interface IDependency { - name: string; - version?: string; -} - -/** - * Security advisory object - */ -export interface ISecurityAdvisory { - 'dependency-name': string; - 'affected-versions': string[]; - 'patched-versions': string[]; - 'unaffected-versions': string[]; - 'title': string; - 'description': string; - 'source-name': string; - 'source-url': string; -} - -/** - * Get the list of security advisories from the GitHub Security Advisory API using GraphQL - * @param accessToken - * @param packageEcosystem - * @param dependencyNames - */ -export async function getSecurityAdvisories( - accessToken: string, - packageEcosystem: string, - dependencies: IDependency[], -): Promise { - const ecosystem = GITHUB_ECOSYSTEM[packageEcosystem]; - const query = ` - query($ecosystem: SecurityAdvisoryEcosystem, $package: String) { - securityVulnerabilities(first: 100, ecosystem: $ecosystem, package: $package) { - nodes { - advisory { - summary, - description, - permalink - } - firstPatchedVersion { - identifier - } - vulnerableVersionRange - } - } - } - `; - - // GitHub API doesn't support querying multiple dependencies at once, so we need to make a request for each dependency individually. - // To speed up the process, we can make the requests in parallel, 100 at a time. We batch the requests to avoid hitting the rate limit too quickly. - // https://docs.github.com/en/graphql/overview/rate-limits-and-node-limits-for-the-graphql-api - console.info(`Checking security advisories for ${dependencies.length} dependencies...`); - const dependencyNames = dependencies.map((dependency) => dependency.name); - const securityAdvisories = await batchSecurityAdvisoryQuery(100, dependencyNames, async (dependencyName) => { - const variables = { - ecosystem: ecosystem, - package: dependencyName, - }; - const response = await axios.post( - GITHUB_ADVISORY_GRAPHQL_API, - JSON.stringify({ - query, - variables, - }), - { - headers: { - 'Authorization': `Bearer ${accessToken}`, - 'Content-Type': 'application/json', - }, - }, - ); - if (response.status < 200 || response.status > 299) { - throw new Error( - `Failed to fetch security advisories for '${dependencyName}': ${response.status} ${response.statusText}`, - ); - } - const vulnerabilities = response.data?.data?.securityVulnerabilities?.nodes; - return vulnerabilities.map((vulnerabilitity: any) => { - return { - 'dependency-name': dependencyName, - 'affected-versions': vulnerabilitity.vulnerableVersionRange - ? [vulnerabilitity.vulnerableVersionRange?.trim()] - : [], - 'patched-versions': vulnerabilitity.firstPatchedVersion - ? [vulnerabilitity.firstPatchedVersion?.identifier] - : [], - 'unaffected-versions': [], - 'title': vulnerabilitity.advisory?.summary, - 'description': vulnerabilitity.advisory?.description, - 'source-name': GITHUB_ADVISORY_SOURCE_NAME, - 'source-url': vulnerabilitity.advisory?.permalink, - }; - }); - }); - - // Filter out advisories that are not relevant the version of the dependency we are using - const affectedAdvisories = securityAdvisories.filter((advisory) => { - const dependency = dependencies.find((d) => d.name === advisory['dependency-name']); - if (!dependency) { - return false; - } - const isAffected = - advisory['affected-versions'].length > 0 && - advisory['affected-versions'].find((range) => versionIsInRange(dependency.version, range)); - const isPatched = - advisory['patched-versions'].length > 0 && - advisory['patched-versions'].find((range) => versionIsInRange(dependency.version, range)); - const isUnaffected = - advisory['unaffected-versions'].length > 0 && - advisory['unaffected-versions'].find((range) => versionIsInRange(dependency.version, range)); - return (isAffected && !isPatched) || isUnaffected; - }); - - const vulnerableDependencyCount = new Set(affectedAdvisories.map((advisory) => advisory['dependency-name'])).size; - console.info( - `Found ${affectedAdvisories.length} vulnerabilities; affecting ${vulnerableDependencyCount} dependencies`, - ); - return affectedAdvisories; -} - -async function batchSecurityAdvisoryQuery( - batchSize: number, - items: string[], - action: (item: string) => Promise, -) { - const results: ISecurityAdvisory[] = []; - for (let i = 0; i < items.length; i += batchSize) { - const batch = items.slice(i, i + batchSize); - const batchResults = await Promise.all(batch.map(action)); - results.push(...batchResults.flat()); - } - return results; -} - -function versionIsInRange(version: string, range: string): boolean { - // TODO: Parse the major/minor/patch version and do a proper comparison, taking in to considerations version ranges (e.g. ">=1.0.0") - return true; -} From 293684c993cdc7896a1ee773bb5de5c973b21061 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 3 Dec 2024 01:16:27 +1300 Subject: [PATCH 33/36] Clean up logging --- extension/tasks/dependabotV2/index.ts | 12 +++++++++++- .../utils/azure-devops/AzureDevOpsWebApiClient.ts | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 5fe153ff..7efb0aa0 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -1,6 +1,6 @@ import { debug, error, setResult, TaskResult, warning, which } from 'azure-pipelines-task-lib/task'; import { AzureDevOpsWebApiClient } from './utils/azure-devops/AzureDevOpsWebApiClient'; -import { setSecrets } from './utils/azure-devops/formattingCommands'; +import { section, setSecrets } from './utils/azure-devops/formattingCommands'; import { DependabotCli } from './utils/dependabot-cli/DependabotCli'; import { DependabotJobBuilder } from './utils/dependabot-cli/DependabotJobBuilder'; import { @@ -152,11 +152,15 @@ async function run() { ); // Get the list of vulnerabilities that apply to the discovered dependencies + section(`GHSA dependency vulnerability check`); const ghsaClient = new GitHubGraphClient(taskInputs.githubAccessToken); const packagesToCheckForVulnerabilities: IPackage[] = discoveredDependencyListOutputs ?.find((x) => x.output.type == 'update_dependency_list') ?.output?.data?.dependencies?.map((d) => ({ name: d.name, version: d.version })); if (packagesToCheckForVulnerabilities?.length) { + console.info( + `Detected ${packagesToCheckForVulnerabilities.length} dependencies; Checking for vulnerabilities...`, + ); securityVulnerabilities = await ghsaClient.getSecurityVulnerabilitiesAsync( getGhsaPackageEcosystemFromDependabotPackageEcosystem(packageEcosystem), packagesToCheckForVulnerabilities || [], @@ -164,6 +168,12 @@ async function run() { // Only update dependencies that have vulnerabilities dependencyNamesToUpdate = Array.from(new Set(securityVulnerabilities.map((v) => v.package.name))); + console.info( + `Detected ${securityVulnerabilities.length} vulnerabilities affecting ${dependencyNamesToUpdate.length} dependencies`, + ); + console.log(dependencyNamesToUpdate); + } else { + console.info('No vulnerabilities detected in any dependencies'); } } diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index e310986a..e9d250bc 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -418,7 +418,7 @@ export class AzureDevOpsWebApiClient { comment: pullRequest.mergeStatus === PullRequestAsyncStatus.Conflicts ? 'Resolve merge conflicts' - : `Rebase with '${targetBranchName}'`, + : `Rebase '${sourceBranchName}' onto '${targetBranchName}'`, author: pr.author, changes: pr.changes.map((change) => { return { From 9fd04ab5fde4a30c25dd5ca6c5e03e05ee2258c3 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 3 Dec 2024 01:33:48 +1300 Subject: [PATCH 34/36] Fix for security-only PRs being closed on next update --- extension/tasks/dependabotV2/index.ts | 1 + .../dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts | 3 +++ 2 files changed, 4 insertions(+) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 7efb0aa0..ddcae107 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -222,6 +222,7 @@ async function run() { dependabotConfig.registries, existingPullRequestDependenciesForPackageEcosystem, existingPullRequestsForPackageEcosystem[pullRequestId], + securityVulnerabilities, ), dependabotUpdaterOptions, ), diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts index 6b5dfc7b..b6ff1dbc 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotJobBuilder.ts @@ -82,6 +82,7 @@ export class DependabotJobBuilder { * @param registries * @param existingPullRequests * @param pullRequestToUpdate + * @param securityVulnerabilities * @returns */ public static updatePullRequestJob( @@ -91,6 +92,7 @@ export class DependabotJobBuilder { registries: Record, existingPullRequests: any[], pullRequestToUpdate: any, + securityVulnerabilities?: ISecurityVulnerability[], ): IDependabotUpdateOperation { const dependencyGroupName = pullRequestToUpdate['dependency-group-name']; const dependencyNames = (dependencyGroupName ? pullRequestToUpdate['dependencies'] : pullRequestToUpdate)?.map( @@ -105,6 +107,7 @@ export class DependabotJobBuilder { dependencyGroupName, dependencyNames, existingPullRequests, + securityVulnerabilities?.filter((v) => dependencyNames.includes(v.package.name)), ); } } From b2e105a9bcc97bb32e9c5947a6704feb02a3d8ad Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 3 Dec 2024 17:44:22 +1300 Subject: [PATCH 35/36] Tidy up logging commands --- .../tasks/dependabotV2/utils/github/GitHubGraphClient.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts b/extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts index 9df10047..d669bd54 100644 --- a/extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts +++ b/extension/tasks/dependabotV2/utils/github/GitHubGraphClient.ts @@ -1,6 +1,8 @@ import axios from 'axios'; import * as semver from 'semver'; +import { warning } from 'azure-pipelines-task-lib/task'; + import { IPackage } from './IPackage'; import { ISecurityVulnerability } from './ISecurityVulnerability'; import { PackageEcosystem } from './PackageEcosystem'; @@ -193,7 +195,7 @@ export class GitHubGraphClient { results.push(...batchResults.flat()); } } catch (error) { - console.warn(`Request batch [${i}-${i + batchSize}] failed; The data may be incomplete. ${error}`); + warning(`Request batch [${i}-${i + batchSize}] failed; The data may be incomplete. ${error}`); } } } From c516daa8b5100b28911b6fe62f1c2509f4d3e144 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 3 Dec 2024 17:55:37 +1300 Subject: [PATCH 36/36] Tidy up logging commands --- extension/tasks/dependabotV2/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index ddcae107..868f5a1a 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -171,7 +171,9 @@ async function run() { console.info( `Detected ${securityVulnerabilities.length} vulnerabilities affecting ${dependencyNamesToUpdate.length} dependencies`, ); - console.log(dependencyNamesToUpdate); + if (dependencyNamesToUpdate.length) { + console.log(dependencyNamesToUpdate); + } } else { console.info('No vulnerabilities detected in any dependencies'); }