fix: allow special characters in branch name

api/controllers/site-branch-config.js

@@ -14,7 +14,7 @@ function generateS3Key(site, context, branch) {
     return null;
   }
 
-  return `preview/${site.owner}/${site.repository}/${branch}`;
+  return `preview/${site.owner}/${site.repository}/${encodeURIComponent(branch)}`;
 }
 
 function validate({ branch, config = {}, context } = {}) {

api/models/site-build-task.js

@@ -1,3 +1,4 @@
+const { isEmptyOrBranch } = require('../utils/validators');
 const associate = ({ BuildTask, BuildTaskType, Site, SiteBuildTask }) => {
   SiteBuildTask.belongsTo(BuildTaskType, {
     foreignKey: 'buildTaskTypeId',
@@ -28,6 +29,9 @@ module.exports = (sequelize, DataTypes) => {
     {
       branch: {
         type: DataTypes.STRING,
+        validate: {
+          isEmptyOrBranch,
+        },
         allowNull: true,
       },
       metadata: {

api/queue-jobs/index.js

@@ -199,7 +199,8 @@ class QueueJobs {
   async startSiteBuild(build, priority) {
     const { branch, id: buildId, Site } = build;
     const { owner, repository } = Site;
-    const jobName = `${owner}/${repository}: ${truncateString(branch)}`;
+    const encodedBranch = `${truncateString(encodeURIComponent(branch))}`;
+    const jobName = `${owner}/${repository}: ${encodedBranch}`;
 
     await this.siteBuildsQueue.waitUntilReady();
 

api/services/GithubBuildHelper.js

@@ -105,13 +105,11 @@ const reportBuildStatus = async (build) => {
       config.app.hostname,
       `/sites/${site.id}/builds/${build.id}/logs`,
     );
-    // eslint-disable-next-line max-len
     options.description =
       'The build is running. Click "Details" to see the Pages build status.';
   } else if (build.state === 'success') {
     options.state = 'success';
     options.target_url = build.url;
-    // eslint-disable-next-line max-len
     options.description =
       'The build is complete! Click "Details" to visit the Site Preview.';
   } else if (build.state === 'error') {
@@ -120,7 +118,6 @@ const reportBuildStatus = async (build) => {
       config.app.hostname,
       `/sites/${site.id}/builds/${build.id}/logs`,
     );
-    // eslint-disable-next-line max-len
     options.description =
       'The build has encountered an error. Click "Details" to see the Pages build logs.';
   }

api/services/S3PublishedFileLister.js

@@ -90,7 +90,7 @@ function listPagedPublishedFilesForBranch(site, branch, startAtKey) {
   } else if (site.demoBranch === branch) {
     filepath = `demo/${site.owner}/${site.repository}`;
   } else {
-    filepath = `preview/${site.owner}/${site.repository}/${branch}`;
+    filepath = `preview/${site.owner}/${site.repository}/${encodeURIComponent(branch)}`;
   }
 
   return apiClient

api/utils/build.js

@@ -1,7 +1,7 @@
 const config = require('../../config');
 
 function previewPath(build, site) {
-  return `/preview/${site.owner}/${site.repository}/${build.branch}`;
+  return `/preview/${site.owner}/${site.repository}/${encodeURIComponent(build.branch)}`;
 }
 
 function proxyUrl(path, site) {

api/utils/validators.js

@@ -1,8 +1,6 @@
 const yaml = require('js-yaml');
 const validator = require('validator');
 
-// eslint-disable-next-line sonarjs/slow-regex
-const branchRegex = /^[\w.]+(?:[/-]*[\w.])*$/;
 const githubUsernameRegex = /^[^-][a-zA-Z-]+$/;
 const shaRegex = /^[a-f0-9]{40}$/;
 const subdomainRegex = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?$/;
@@ -101,14 +99,32 @@ function isEmptyOrBranch(value) {
     throw new Error('Invalid branch name — branch names are limitted to 299 characters.');
   }
 
-  if (value && value.length && !branchRegex.test(value)) {
+  if (value && value.length && !isValidBranchName(value)) {
     throw new Error(
       // eslint-disable-next-line max-len
       'Invalid branch name — branches can only contain alphanumeric characters, underscores, and hyphens.',
     );
   }
 }
 
+const isValidBranchName = (branchName) => {
+  if (branchName === '@') return false;
+
+  // Check control characters
+  for (const char of branchName) {
+    const code = char.charCodeAt(0);
+    if ((code >= 0 && code <= 31) || code === 127) return false;
+  }
+
+  // Combined regex for all other forbidden patterns
+  if (/^\/|[ ~^:?*[\\]|\.\..|@\{|\/\/|\.$|\/$/.test(branchName)) {
+    return false;
+  }
+
+  // Check components
+  return !branchName.split('/').some((c) => c.startsWith('.') || c.endsWith('.lock'));
+};
+
 function isEmptyOrUrl(value) {
   const validUrlOptions = {
     require_protocol: true,
@@ -136,7 +152,6 @@ const isDelimitedFQDN = (str) => {
 };
 
 module.exports = {
-  branchRegex,
   CustomError,
   shaRegex,
   githubUsernameRegex,
@@ -149,4 +164,5 @@ module.exports = {
   ValidationError,
   isValidSubdomain,
   isDelimitedFQDN,
+  isValidBranchName,
 };

frontend/pages/sites/$siteId/published/$name/BranchFileRow.jsx

@@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
 export default function BranchFileRow({ file }) {
   let viewFileLink;
   const branch = file.publishedBranch.name;
+  const encBranch = encodeURIComponent(branch);
   switch (branch) {
     case file.publishedBranch.site.defaultBranch:
       viewFileLink = `${file.publishedBranch.site.viewLink}${file.name}`;
@@ -12,7 +13,7 @@ export default function BranchFileRow({ file }) {
       viewFileLink = `${file.publishedBranch.site.demoViewLink}${file.name}`;
       break;
     default:
-      viewFileLink = `${file.publishedBranch.site.previewLink}${branch}/${file.name}`;
+      viewFileLink = `${file.publishedBranch.site.previewLink}${encBranch}/${file.name}`;
   }
   return (
     <tr key={viewFileLink}>

frontend/pages/sites/$siteId/published/BranchRow.jsx

@@ -7,7 +7,7 @@ import BranchViewLink from './BranchViewLink';
 import { SITE } from '@propTypes';
 
 function BranchFilesLink({ branch }) {
-  const href = `/sites/${branch.site.id}/published/${branch.name}`;
+  const href = `/sites/${branch.site.id}/published/${encodeURIComponent(branch.name)}`;
   return <Link to={href}>View files</Link>;
 }
 

frontend/pages/sites/$siteId/published/BranchViewLink.jsx

@@ -13,7 +13,7 @@ const getViewLink = (branch, site) => {
   // Checks to see if there is a site branch config
   // If not it returns the preview url
   if (!branchConfig) {
-    return `${origin}/preview/${owner}/${repository}/${branch}`;
+    return `${origin}/preview/${owner}/${repository}/${encodeURIComponent(branch)}`;
   }
 
   const domain = domains.find((d) => d.siteBranchConfigId === branchConfig.id);

frontend/pages/sites/$siteId/published/BranchViewLink.test.jsx

@@ -94,13 +94,14 @@ describe('<BranchViewLink/>', () => {
   });
 
   it('renders a preview link to the other branches', () => {
-    const branchName = 'some-other-branch';
+    const branchName = 'some-other-branch-with-special_characters-###';
+    const branchEncoded = 'some-other-branch-with-special_characters-%23%23%23';
     const updatedProps = { ...props, branchName };
     render(<BranchViewLink {...updatedProps} />);
     const anchor = screen.getByRole('link');
     expect(anchor).toHaveAttribute(
       'href',
-      `${proxyOrigin}/preview/${testSite.owner}/${testSite.repository}/${branchName}`,
+      `${proxyOrigin}/preview/${testSite.owner}/${testSite.repository}/${branchEncoded}`,
     );
     expect(anchor).toHaveTextContent(VIEW_BUILD);
   });

frontend/util/federalistApi.js

@@ -104,7 +104,7 @@ export default {
   },
 
   fetchPublishedFiles(id, branch, startAtKey = null) {
-    let path = `site/${id}/published-branch/${branch}/published-file`;
+    let path = `site/${id}/published-branch/${encodeURIComponent(branch)}/published-file`;
     if (startAtKey) {
       path += `?startAtKey=${startAtKey}`;
     }

test/api/unit/utils/validators.test.js

@@ -80,4 +80,139 @@ describe('validators', () => {
       );
     });
   });
+
+  describe('.isValidBranchName', () => {
+    // https://git-scm.com/docs/git-check-ref-format
+    // Git imposes the following rules on how references are named:
+    //
+    // 1. They can include slash / for hierarchical (directory) grouping,
+    // but no slash-separated component can begin with a dot .
+    // or end with the sequence .lock.
+    //
+    // 2. They must contain at least one /. This enforces the presence of a category
+    // like heads/, tags/ etc. but the actual names are not restricted.
+    // If the --allow-onelevel option is used, this rule is waived.
+    //
+    // 3. They cannot have two consecutive dots .. anywhere.
+    //
+    // 4. They cannot have ASCII control characters
+    // (i.e. bytes whose values are lower than \040, or \177 DEL),
+    // space, tilde ~, caret ^, or colon : anywhere.
+    //
+    // 5. They cannot have question-mark ?, asterisk *, or open bracket [ anywhere.
+    // (--refspec-pattern option is an exception to this rule).
+    //
+    // 6. They cannot begin or end with a slash / or contain multiple consecutive slashes
+    // (--normalize option is an exception to this rule).
+    //
+    // 7. They cannot end with a dot ..
+    //
+    // 8. They cannot contain a sequence @{.
+    //
+    // 9. They cannot be the single character @.
+    //
+    // 10. They cannot contain a \.
+
+    const specialChars = [
+      // Control characters (0x00-0x1F) - ALL forbidden
+      { specialChar: '\x00', allowed: false }, // NUL
+      { specialChar: '\x01', allowed: false }, // SOH
+      { specialChar: '\x02', allowed: false }, // STX
+      { specialChar: '\x03', allowed: false }, // ETX
+      { specialChar: '\x04', allowed: false }, // EOT
+      { specialChar: '\x05', allowed: false }, // ENQ
+      { specialChar: '\x06', allowed: false }, // ACK
+      { specialChar: '\x07', allowed: false }, // BEL
+      { specialChar: '\x08', allowed: false }, // BS
+      { specialChar: '\x09', allowed: false }, // TAB
+      { specialChar: '\x0A', allowed: false }, // LF
+      { specialChar: '\x0B', allowed: false }, // VT
+      { specialChar: '\x0C', allowed: false }, // FF
+      { specialChar: '\x0D', allowed: false }, // CR
+      { specialChar: '\x0E', allowed: false }, // SO
+      { specialChar: '\x0F', allowed: false }, // SI
+      { specialChar: '\x10', allowed: false }, // DLE
+      { specialChar: '\x11', allowed: false }, // DC1
+      { specialChar: '\x12', allowed: false }, // DC2
+      { specialChar: '\x13', allowed: false }, // DC3
+      { specialChar: '\x14', allowed: false }, // DC4
+      { specialChar: '\x15', allowed: false }, // NAK
+      { specialChar: '\x16', allowed: false }, // SYN
+      { specialChar: '\x17', allowed: false }, // ETB
+      { specialChar: '\x18', allowed: false }, // CAN
+      { specialChar: '\x19', allowed: false }, // EM
+      { specialChar: '\x1A', allowed: false }, // SUB
+      { specialChar: '\x1B', allowed: false }, // ESC
+      { specialChar: '\x1C', allowed: false }, // FS
+      { specialChar: '\x1D', allowed: false }, // GS
+      { specialChar: '\x1E', allowed: false }, // RS
+      { specialChar: '\x1F', allowed: false }, // US
+
+      // Printable special characters
+      { specialChar: ' ', allowed: false }, // Space - forbidden
+      { specialChar: '!', allowed: true },
+      { specialChar: '"', allowed: true },
+      { specialChar: '#', allowed: true },
+      { specialChar: '$', allowed: true },
+      { specialChar: '%', allowed: true },
+      { specialChar: '&', allowed: true },
+      { specialChar: "'", allowed: true },
+      { specialChar: '(', allowed: true },
+      { specialChar: ')', allowed: true },
+      { specialChar: '*', allowed: false }, // Asterisk - forbidden
+      { specialChar: '+', allowed: true },
+      { specialChar: ',', allowed: true },
+      { specialChar: '-', allowed: true },
+      { specialChar: '.', allowed: true }, // Allowed but with restrictions
+      { specialChar: '/', allowed: true }, // Allowed but with restrictions
+      { specialChar: ':', allowed: false }, // Colon - forbidden
+      { specialChar: ';', allowed: true },
+      { specialChar: '<', allowed: true },
+      { specialChar: '=', allowed: true },
+      { specialChar: '>', allowed: true },
+      { specialChar: '?', allowed: false }, // Question mark - forbidden
+      { specialChar: '@', allowed: true }, // Allowed but '@{' sequence forbidden
+      { specialChar: '[', allowed: false }, // Open bracket - forbidden
+      { specialChar: '\\', allowed: false }, // Backslash - forbidden
+      { specialChar: ']', allowed: true },
+      { specialChar: '^', allowed: false }, // Caret - forbidden
+      { specialChar: '_', allowed: true },
+      { specialChar: '`', allowed: true },
+      { specialChar: '{', allowed: true }, // Allowed but '@{' sequence forbidden
+      { specialChar: '|', allowed: true },
+      { specialChar: '}', allowed: true },
+      { specialChar: '~', allowed: false }, // Tilde - forbidden
+
+      // DEL control character
+      { specialChar: '\x7F', allowed: false }, // DEL
+    ];
+
+    it("validates Git reference names according to Git's rules", () => {
+      expect(validators.isValidBranchName('feature/my-branch')).to.equal(true);
+      expect(validators.isValidBranchName('hotfix/bug-123')).to.equal(true);
+      expect(validators.isValidBranchName('/invalid')).to.equal(false);
+      expect(validators.isValidBranchName('bad..name')).to.equal(false);
+      expect(validators.isValidBranchName('branch with spaces')).to.equal(false);
+      expect(validators.isValidBranchName('main')).to.equal(true);
+      expect(validators.isValidBranchName('refs/tags/v1.0.0')).to.equal(true);
+      expect(validators.isValidBranchName('main')).to.equal(true);
+      expect(validators.isValidBranchName('.hidden')).to.equal(false);
+      expect(validators.isValidBranchName('branch.lock')).to.equal(false);
+      expect(validators.isValidBranchName('bad..name')).to.equal(false);
+      expect(validators.isValidBranchName('bad@{name}')).to.equal(false);
+      expect(validators.isValidBranchName('bad\\name')).to.equal(false);
+      expect(validators.isValidBranchName('/main')).to.equal(false);
+      expect(validators.isValidBranchName('main/')).to.equal(false);
+      expect(validators.isValidBranchName('ma//in')).to.equal(false);
+      expect(validators.isValidBranchName('main.')).to.equal(false);
+      expect(validators.isValidBranchName('@{')).to.equal(false);
+
+      specialChars.forEach((i) => {
+        expect(
+          validators.isValidBranchName(`branch${i.specialChar}branch`),
+          i.specialChar,
+        ).to.equal(i.allowed);
+      });
+    });
+  });
 });