Fix for invalid characters in branch name (#1392)

The majority of the changes were merged in previous PRs?

extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts

@@ -1,11 +1,11 @@
 import { GitPullRequestMergeStrategy, VersionControlChangeType } from 'azure-devops-node-api/interfaces/GitInterfaces';
 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 { IFileChange } from '../azure-devops/interfaces/IFileChange';
 import { IPullRequestProperties } from '../azure-devops/interfaces/IPullRequest';
-import { IDependabotUpdate } from '../dependabot/interfaces/IDependabotConfig';
 import { ISharedVariables } from '../getSharedVariables';
+import { getBranchNameForUpdate } from './getBranchName';
 import { IDependabotUpdateOperation } from './interfaces/IDependabotUpdateOperation';
 import { IDependabotUpdateOutputProcessor } from './interfaces/IDependabotUpdateOutputProcessor';
 
@@ -96,10 +96,18 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess
         }
 
         // Create a new pull request
+        const changedFiles = getPullRequestChangedFilesForOutputData(data);
         const dependencies = getPullRequestDependenciesPropertyValueForOutputData(data);
         const targetBranch =
           update.config['target-branch'] || (await this.prAuthorClient.getDefaultBranch(project, repository));
-        const sourceBranch = getSourceBranchNameForUpdate(update.config, targetBranch, dependencies);
+        const sourceBranch = getBranchNameForUpdate(
+          update.config['package-ecosystem'],
+          targetBranch,
+          update.config.directory || update.config.directories?.find((dir) => changedFiles[0]?.path?.startsWith(dir)),
+          dependencies['dependency-group-name'],
+          dependencies['dependencies'] || dependencies,
+          update.config['pull-request-branch-name']?.separator,
+        );
         const newPullRequestId = await this.prAuthorClient.createPullRequest({
           project: project,
           repository: repository,
@@ -140,7 +148,7 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess
           reviewers: update.config.reviewers,
           labels: update.config.labels?.map((label) => label?.trim()) || [],
           workItems: update.config.milestone ? [update.config.milestone] : [],
-          changes: getPullRequestChangedFilesForOutputData(data),
+          changes: changedFiles,
           properties: buildPullRequestProperties(update.job['package-manager'], dependencies),
         });
 
@@ -323,30 +331,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 packageEcosystem = update['package-ecosystem'];
-  const targetBranchName = targetBranch?.replace(/^\/+|\/+$/g, ''); // strip leading/trailing slashes
-  if (dependencies['dependency-group-name']) {
-    // Group dependency update
-    // e.g. dependabot/nuget/main/microsoft-3b49c54d9e
-    const dependencyGroupName = dependencies['dependency-group-name'];
-    const dependencyHash = crypto
-      .createHash('md5')
-      .update(dependencies['dependencies'].map((d) => `${d['dependency-name']}-${d['dependency-version']}`).join(','))
-      .digest('hex')
-      .substring(0, 10);
-    return `${prefix}${separator}${packageEcosystem}${separator}${targetBranchName}${separator}${dependencyGroupName}-${dependencyHash}`;
-  } else {
-    // Single dependency update
-    // e.g. dependabot/nuget/main/Microsoft.Extensions.Logging-1.0.0
-    const leadDependency = dependencies.length === 1 ? dependencies[0] : null;
-    return `${prefix}${separator}${packageEcosystem}${separator}${targetBranchName}${separator}${leadDependency['dependency-name']}-${leadDependency['dependency-version']}`;
-  }
-}
-
-function getPullRequestChangedFilesForOutputData(data: any): any {
+function getPullRequestChangedFilesForOutputData(data: any): IFileChange[] {
   return data['updated-dependency-files']
     .filter((file) => file['type'] === 'file')
     .map((file) => {

extension/tasks/dependabotV2/utils/dependabot-cli/getBranchName.ts

@@ -0,0 +1,63 @@
+import * as crypto from 'crypto';
+
+export function getBranchNameForUpdate(
+  packageEcosystem: string,
+  targetBranchName: string,
+  directory: string,
+  dependencyGroupName: string,
+  dependencies: any,
+  separator?: string,
+): string {
+  // Based on dependabot-core implementation:
+  // https://github.com/dependabot/dependabot-core/blob/main/common/lib/dependabot/pull_request_creator/branch_namer/solo_strategy.rb
+  // https://github.com/dependabot/dependabot-core/blob/main/common/lib/dependabot/pull_request_creator/branch_namer/dependency_group_strategy.rb
+  let branchName: string;
+  const branchNameMightBeTooLong = dependencyGroupName || dependencies.length > 1;
+  if (branchNameMightBeTooLong) {
+    // Group/multi dependency update
+    // e.g. dependabot/nuget/main/microsoft-3b49c54d9e
+    const dependencyDigest = crypto
+      .createHash('md5')
+      .update(dependencies.map((d) => `${d['dependency-name']}-${d['dependency-version']}`).join(','))
+      .digest('hex')
+      .substring(0, 10);
+    branchName = `${dependencyGroupName || 'multi'}-${dependencyDigest}`;
+  } else {
+    // Single dependency update
+    // e.g. dependabot/nuget/main/Microsoft.Extensions.Logging-1.0.0
+    const dependencyNames = dependencies
+      .map((d) => d['dependency-name'])
+      .join('-and-')
+      .replace(/[:\[\]]/g, '-') // Replace `:` and `[]` with `-`
+      .replace(/@/g, ''); // Remove `@`
+    const versionSuffix = dependencies[0]?.['removed'] ? 'removed' : dependencies[0]?.['dependency-version'];
+    branchName = `${dependencyNames}-${versionSuffix}`;
+  }
+
+  // TODO: Add config for the branch prefix? Task V1 supported this via DEPENDABOT_BRANCH_NAME_PREFIX
+  return sanitizeRef(['dependabot', packageEcosystem, targetBranchName, directory, branchName], separator || '/');
+}
+
+function sanitizeRef(refParts: string[], separator): string {
+  // Based on dependabot-core implementation:
+  // https://github.com/dependabot/dependabot-core/blob/fc31ae64f492dc977cfe6773ab13fb6373aabec4/common/lib/dependabot/pull_request_creator/branch_namer/base.rb#L99
+
+  // This isn't a complete implementation of git's ref validation, but it
+  // covers most cases that crop up. Its list of allowed characters is a
+  // bit stricter than git's, but that's for cosmetic reasons.
+  return (
+    refParts
+      // Join the parts with the separator, ignore empty parts
+      .filter((p) => p?.trim()?.length > 0)
+      .join(separator)
+      // Remove forbidden characters (those not already replaced elsewhere)
+      .replace(/[^A-Za-z0-9\/\-_.(){}]/g, '')
+      // Slashes can't be followed by periods
+      .replace(/\/\./g, '/dot-')
+      // Squeeze out consecutive periods and slashes
+      .replace(/\.+/g, '.')
+      .replace(/\/+/g, '/')
+      // Trailing periods are forbidden
+      .replace(/\.$/, '')
+  );
+}