fix: compare only <owner>/<repo> when checking for rename (#886)

* fix: ignore protocol mismatch when checking if repository is renamed * fix: compare repositoryUrl to git_url instead of clone_url * fix: include values of repositoryUrl and git_url in EMISMATCHGITHUBURL * chore: fix format * fix: fix git repo check and tests * test: fix mock git_urls * test: add EMISMATCHGITHUBURL tests * chore: format code * fix: mistake that would cause error if run in github action * test: remove comment about repeat calls * refactor: code cleanup * fix: check for rename even when authenticated as a GitHub App installation * test: test rename check for GitHub installation * fix: switch back to clone_url, test a lot of URL formats --------- Co-authored-by: Olabode Lawal-Shittabey <[email protected]>
semantic-release · Aug 14, 2024 · 24ea2ee · 24ea2ee
1 parent 4adf13d
commit 24ea2ee
Show file tree

Hide file tree

Showing 6 changed files with 502 additions and 87 deletions.
diff --git a/lib/definitions/errors.js b/lib/definitions/errors.js
@@ -139,6 +139,19 @@ By default the \`repositoryUrl\` option is retrieved from the \`repository\` pro
   };
 }
 
+export function EMISMATCHGITHUBURL({ repositoryUrl, clone_url }) {
+  return {
+    message: "The git repository URL mismatches the GitHub URL.",
+    details: `The **semantic-release** \`repositoryUrl\` option must have the same repository name and owner as the GitHub repo.
+
+Your configuration for the \`repositoryUrl\` option is \`${stringify(repositoryUrl)}\` and the \`clone_url\` of your GitHub repo is \`${stringify(clone_url)}\`.
+
+By default the \`repositoryUrl\` option is retrieved from the \`repository\` property of your \`package.json\` or the [git origin url](https://git-scm.com/book/en/v2/Git-Basics-Working-with-Remotes) of the repository cloned by your CI environment.
+
+Note: If you have recently changed your GitHub repository name or owner, update the value in **semantic-release** \`repositoryUrl\` option and the \`repository\` property of your \`package.json\` respectively to match the new GitHub URL.`,
+  };
+}
+
 export function EINVALIDPROXY({ proxy }) {
   return {
     message: "Invalid `proxy` option.",

diff --git a/lib/verify.js b/lib/verify.js
@@ -103,35 +103,34 @@ export default async function verify(pluginConfig, context, { Octokit }) {
         proxy,
       }),
     );
-
-    // https://github.com/semantic-release/github/issues/182
-    // Do not check for permissions in GitHub actions, as the provided token is an installation access token.
-    // octokit.request("GET /repos/{owner}/{repo}", {repo, owner}) does not return the "permissions" key in that case.
-    // But GitHub Actions have all permissions required for @semantic-release/github to work
-    if (env.GITHUB_ACTION) {
-      return;
-    }
-
     try {
       const {
-        data: {
-          permissions: { push },
-        },
+        data: { permissions, clone_url },
       } = await octokit.request("GET /repos/{owner}/{repo}", { repo, owner });
-      if (!push) {
+      // Verify if Repository Name wasn't changed
+      const parsedCloneUrl = parseGithubUrl(clone_url);
+      if (owner !== parsedCloneUrl.owner || repo !== parsedCloneUrl.repo) {
+        errors.push(
+          getError("EMISMATCHGITHUBURL", { repositoryUrl, clone_url }),
+        );
+      }
+
+      // https://github.com/semantic-release/github/issues/182
+      // Do not check for permissions in GitHub actions, as the provided token is an installation access token.
+      // octokit.request("GET /repos/{owner}/{repo}", {repo, owner}) does not return the "permissions" key in that case.
+      // But GitHub Actions have all permissions required for @semantic-release/github to work
+      if (!env.GITHUB_ACTION && !permissions?.push) {
         // If authenticated as GitHub App installation, `push` will always be false.
         // We send another request to check if current authentication is an installation.
         // Note: we cannot check if the installation has all required permissions, it's
         // up to the user to make sure it has
         if (
-          await octokit
+          !(await octokit
             .request("HEAD /installation/repositories", { per_page: 1 })
-            .catch(() => false)
+            .catch(() => false))
         ) {
-          return;
+          errors.push(getError("EGHNOPERMISSION", { owner, repo }));
         }
-
-        errors.push(getError("EGHNOPERMISSION", { owner, repo }));
       }
     } catch (error) {
       if (error.status === 401) {

diff --git a/test/fail.test.js b/test/fail.test.js
@@ -115,6 +115,7 @@ test("Open a new issue with the list of errors and custom title and comment", as
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .getOnce(
       `https://api.github.local/search/issues?q=${encodeURIComponent(
@@ -182,6 +183,7 @@ test("Open a new issue with assignees and the list of errors", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .getOnce(
       `https://api.github.local/search/issues?q=${encodeURIComponent(
@@ -254,6 +256,7 @@ test("Open a new issue without labels and the list of errors", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .getOnce(
       `https://api.github.local/search/issues?q=${encodeURIComponent(
@@ -330,6 +333,7 @@ test("Update the first existing issue with the list of errors", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .getOnce(
       `https://api.github.local/search/issues?q=${encodeURIComponent(

diff --git a/test/integration.test.js b/test/integration.test.js
@@ -27,7 +27,10 @@ test("Verify GitHub auth", async (t) => {
   const fetch = fetchMock
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
-      permissions: { push: true },
+      permissions: {
+        push: true,
+      },
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     });
 
   await t.notThrowsAsync(
@@ -56,8 +59,11 @@ test("Verify GitHub auth with publish options", async (t) => {
   };
   const fetch = fetchMock
     .sandbox()
-    .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
-      permissions: { push: true },
+    .get(`https://api.github.local/repos/${owner}/${repo}`, {
+      permissions: {
+        push: true,
+      },
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     });
 
   await t.notThrowsAsync(
@@ -94,7 +100,10 @@ test("Verify GitHub auth and assets config", async (t) => {
   const fetch = fetchMock
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
-      permissions: { push: true },
+      permissions: {
+        push: true,
+      },
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     });
 
   await t.notThrowsAsync(
@@ -197,7 +206,10 @@ test("Publish a release with an array of assets", async (t) => {
   const fetch = fetchMock
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
-      permissions: { push: true },
+      permissions: {
+        push: true,
+      },
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce(
       `https://api.github.local/repos/${owner}/${repo}/releases`,
@@ -289,7 +301,10 @@ test("Publish a release with release information in assets", async (t) => {
   const fetch = fetchMock
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
-      permissions: { push: true },
+      permissions: {
+        push: true,
+      },
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce(
       `https://api.github.local/repos/${owner}/${repo}/releases`,
@@ -359,7 +374,10 @@ test("Update a release", async (t) => {
   const fetch = fetchMock
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
-      permissions: { push: true },
+      permissions: {
+        push: true,
+      },
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .getOnce(
       `https://api.github.local/repos/${owner}/${repo}/releases/tags/${nextRelease.gitTag}`,
@@ -426,9 +444,9 @@ test("Comment and add labels on PR included in the releases", async (t) => {
       {
         permissions: { push: true },
         full_name: `${owner}/${repo}`,
+        clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
       },
       {
-        // TODO: why do we call the same endpoint twice?
         repeat: 2,
       },
     )
@@ -529,10 +547,9 @@ test("Open a new issue with the list of errors", async (t) => {
       {
         permissions: { push: true },
         full_name: `${owner}/${repo}`,
+        clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
       },
-      {
-        repeat: 2,
-      },
+      { repeat: 2 },
     )
     .getOnce(
       `https://api.github.local/search/issues?q=${encodeURIComponent(
@@ -625,6 +642,7 @@ test("Verify, release and notify success", async (t) => {
       {
         permissions: { push: true },
         full_name: `${owner}/${repo}`,
+        clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
       },
       {
         repeat: 2,
@@ -785,6 +803,7 @@ test("Verify, update release and notify success", async (t) => {
       {
         permissions: { push: true },
         full_name: `${owner}/${repo}`,
+        clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
       },
       {
         repeat: 2,
@@ -917,6 +936,7 @@ test("Verify and notify failure", async (t) => {
       {
         permissions: { push: true },
         full_name: `${owner}/${repo}`,
+        clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
       },
       {
         repeat: 2,

diff --git a/test/success.test.js b/test/success.test.js
@@ -57,6 +57,7 @@ test("Add comment and labels to PRs associated with release commits and issues s
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${redirectedOwner}/${redirectedRepo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -418,6 +419,7 @@ test("Make multiple search queries if necessary", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .post("https://api.github.local/graphql", {
       data: {
@@ -662,6 +664,7 @@ test("Do not add comment and labels for unrelated PR returned by search (compare
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -764,6 +767,7 @@ test("Do not add comment and labels if no PR is associated with release commits"
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -826,6 +830,7 @@ test("Do not add comment and labels if no commits is found for release", async (
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .getOnce(
       `https://api.github.local/search/issues?q=${encodeURIComponent(
@@ -885,6 +890,7 @@ test("Do not add comment and labels to PR/issues from other repo", async (t) =>
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -985,6 +991,7 @@ test("Ignore missing and forbidden issues/PRs", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1154,6 +1161,7 @@ test("Add custom comment and labels", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1249,6 +1257,7 @@ test("Add custom label", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1339,6 +1348,7 @@ test("Comment on issue/PR without ading a label", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1432,6 +1442,7 @@ test("Editing the release to include all release links at the bottom", async (t)
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1536,6 +1547,7 @@ test("Editing the release to include all release links at the top", async (t) =>
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1637,6 +1649,7 @@ test("Editing the release to include all release links with no additional releas
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1727,6 +1740,7 @@ test("Editing the release to include all release links with no additional releas
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1810,6 +1824,7 @@ test("Editing the release to include all release links with no releases", async
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1895,6 +1910,7 @@ test("Editing the release with no ID in the release", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -1985,6 +2001,7 @@ test("Ignore errors when adding comments and closing issues", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -2118,6 +2135,7 @@ test("Close open issues when a release is successful", async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -2218,6 +2236,7 @@ test('Skip commention on issues/PR if "successComment" is "false"', async (t) =>
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .getOnce(
       `https://api.github.local/search/issues?q=${encodeURIComponent(
@@ -2269,6 +2288,7 @@ test('Skip closing issues if "failComment" is "false"', async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {
@@ -2320,6 +2340,7 @@ test('Skip closing issues if "failTitle" is "false"', async (t) => {
     .sandbox()
     .getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
       full_name: `${owner}/${repo}`,
+      clone_url: `https://api.github.local/${owner}/${repo}.git`,
     })
     .postOnce("https://api.github.local/graphql", {
       data: {