fix: support symlinks when commit signing (#3359 )

fix: support submodules when commit signing (#3354 )
* fix: support submodules when commit signing * create correct tree object for submodule * update log messages
2024-09-18 10:40:10 -07:00 · 2024-09-18 09:46:39 -07:00 · 2024-09-17 12:34:38 +00:00 · 2024-09-17 12:34:19 +00:00 · 2024-09-16 09:49:30 -07:00 · 2024-09-16 02:17:19 -07:00
9 changed files with 152 additions and 97 deletions
--- a/test/create-or-update-branch.int.test.ts
+++ b/test/create-or-update-branch.int.test.ts
@ -250,11 +250,15 @@ describe('create-or-update-branch tests', () => {
    expect(branchCommits.length).toEqual(1)
    expect(branchCommits[0].subject).toEqual('Test changes')
    expect(branchCommits[0].changes.length).toEqual(3)
-    expect(branchCommits[0].changes).toEqual([
-      {mode: '100755', path: UNTRACKED_EXE_FILE, status: 'A'},
-      {mode: '100644', path: TRACKED_FILE, status: 'M'},
-      {mode: '100644', path: UNTRACKED_FILE, status: 'A'}
-    ])
+    expect(branchCommits[0].changes[0].mode).toEqual('100755')
+    expect(branchCommits[0].changes[0].path).toEqual(UNTRACKED_EXE_FILE)
+    expect(branchCommits[0].changes[0].status).toEqual('A')
+    expect(branchCommits[0].changes[1].mode).toEqual('100644')
+    expect(branchCommits[0].changes[1].path).toEqual(TRACKED_FILE)
+    expect(branchCommits[0].changes[1].status).toEqual('M')
+    expect(branchCommits[0].changes[2].mode).toEqual('100644')
+    expect(branchCommits[0].changes[2].path).toEqual(UNTRACKED_FILE)
+    expect(branchCommits[0].changes[2].status).toEqual('A')
  })

  it('tests buildBranchCommits with addition and deletion', async () => {
@ -272,11 +276,15 @@ describe('create-or-update-branch tests', () => {
    expect(branchCommits.length).toEqual(1)
    expect(branchCommits[0].subject).toEqual('Test changes')
    expect(branchCommits[0].changes.length).toEqual(3)
-    expect(branchCommits[0].changes).toEqual([
-      {mode: '100644', path: TRACKED_FILE, status: 'D'},
-      {mode: '100644', path: UNTRACKED_FILE, status: 'A'},
-      {mode: '100644', path: TRACKED_FILE_NEW_PATH, status: 'A'}
-    ])
+    expect(branchCommits[0].changes[0].mode).toEqual('100644')
+    expect(branchCommits[0].changes[0].path).toEqual(TRACKED_FILE)
+    expect(branchCommits[0].changes[0].status).toEqual('D')
+    expect(branchCommits[0].changes[1].mode).toEqual('100644')
+    expect(branchCommits[0].changes[1].path).toEqual(UNTRACKED_FILE)
+    expect(branchCommits[0].changes[1].status).toEqual('A')
+    expect(branchCommits[0].changes[2].mode).toEqual('100644')
+    expect(branchCommits[0].changes[2].path).toEqual(TRACKED_FILE_NEW_PATH)
+    expect(branchCommits[0].changes[2].status).toEqual('A')
  })

  it('tests buildBranchCommits with multiple commits', async () => {
@ -294,10 +302,13 @@ describe('create-or-update-branch tests', () => {
      expect(branchCommits[i].subject).toEqual(`Test changes ${i}`)
      expect(branchCommits[i].changes.length).toEqual(2)
      const untrackedFileStatus = i == 0 ? 'A' : 'M'
-      expect(branchCommits[i].changes).toEqual([
-        {mode: '100644', path: TRACKED_FILE, status: 'M'},
-        {mode: '100644', path: UNTRACKED_FILE, status: untrackedFileStatus}
-      ])
+
+      expect(branchCommits[i].changes[0].mode).toEqual('100644')
+      expect(branchCommits[i].changes[0].path).toEqual(TRACKED_FILE)
+      expect(branchCommits[i].changes[0].status).toEqual('M')
+      expect(branchCommits[i].changes[1].mode).toEqual('100644')
+      expect(branchCommits[i].changes[1].path).toEqual(UNTRACKED_FILE)
+      expect(branchCommits[i].changes[1].status).toEqual(untrackedFileStatus)
    }
  })

--- a/test/git-command-manager.int.test.ts
+++ b/test/git-command-manager.int.test.ts
@ -18,9 +18,9 @@ describe('git-command-manager integration tests', () => {

    expect(initialCommit.subject).toEqual('initial commit')
    expect(initialCommit.signed).toBeFalsy()
-    expect(initialCommit.changes).toEqual([
-      {mode: '100644', status: 'A', path: 'README_TEMP.md'}
-    ])
+    expect(initialCommit.changes[0].mode).toEqual('100644')
+    expect(initialCommit.changes[0].status).toEqual('A')
+    expect(initialCommit.changes[0].path).toEqual('README_TEMP.md')

    expect(emptyCommit.subject).toEqual('empty commit for tests')
    expect(emptyCommit.tree).toEqual(initialCommit.tree) // empty commits have no tree and reference the parent's
@ -31,16 +31,18 @@ describe('git-command-manager integration tests', () => {
    expect(modifiedCommit.subject).toEqual('add sparkles')
    expect(modifiedCommit.parents[0]).toEqual(emptyCommit.sha)
    expect(modifiedCommit.signed).toBeFalsy()
-    expect(modifiedCommit.changes).toEqual([
-      {mode: '100644', status: 'M', path: 'README_TEMP.md'}
-    ])
+    expect(modifiedCommit.changes[0].mode).toEqual('100644')
+    expect(modifiedCommit.changes[0].status).toEqual('M')
+    expect(modifiedCommit.changes[0].path).toEqual('README_TEMP.md')

    expect(headCommit.subject).toEqual('rename readme')
    expect(headCommit.parents[0]).toEqual(modifiedCommit.sha)
    expect(headCommit.signed).toBeFalsy()
-    expect(headCommit.changes).toEqual([
-      {mode: '100644', status: 'A', path: 'README.md'},
-      {mode: '100644', status: 'D', path: 'README_TEMP.md'}
-    ])
+    expect(headCommit.changes[0].mode).toEqual('100644')
+    expect(headCommit.changes[0].status).toEqual('A')
+    expect(headCommit.changes[0].path).toEqual('README.md')
+    expect(headCommit.changes[1].mode).toEqual('100644')
+    expect(headCommit.changes[1].status).toEqual('D')
+    expect(headCommit.changes[1].path).toEqual('README_TEMP.md')
  })
 })
--- a/dist/index.js
+++ b/dist/index.js
@ -746,6 +746,7 @@ class GitCommandManager {
                '--raw',
                '--cc',
                '--no-renames',
+                '--no-abbrev',
                `--format=%H%n%T%n%P%n%G?%n%s%n%b%n${endOfBody}`,
                ref
            ]);
@ -761,12 +762,13 @@ class GitCommandManager {
                subject: detailLines[4],
                body: detailLines.slice(5, endOfBodyIndex).join('\n'),
                changes: lines.slice(endOfBodyIndex + 2, -1).map(line => {
-                    const change = line.match(/^:(\d{6}) (\d{6}) \w{7} \w{7} ([AMD])\s+(.*)$/);
+                    const change = line.match(/^:(\d{6}) (\d{6}) \w{40} (\w{40}) ([AMD])\s+(.*)$/);
                    if (change) {
                        return {
-                            mode: change[3] === 'D' ? change[1] : change[2],
-                            status: change[3],
-                            path: change[4]
+                            mode: change[4] === 'D' ? change[1] : change[2],
+                            dstSha: change[3],
+                            status: change[4],
+                            path: change[5]
                        };
                    }
                    else {
@ -1367,25 +1369,37 @@ class GitHubHelper {
            let treeSha = parentCommit.tree;
            if (commit.changes.length > 0) {
                core.info(`Creating tree objects for local commit ${commit.sha}`);
-                const treeObjects = yield Promise.all(commit.changes.map((_a) => __awaiter(this, [_a], void 0, function* ({ path, mode, status }) {
-                    let sha = null;
-                    if (status === 'A' || status === 'M') {
-                        try {
-                            const { data: blob } = yield blobCreationLimit(() => this.octokit.rest.git.createBlob(Object.assign(Object.assign({}, repository), { content: utils.readFileBase64([repoPath, path]), encoding: 'base64' })));
-                            sha = blob.sha;
-                        }
-                        catch (error) {
-                            core.error(`Error creating blob for file '${path}': ${utils.getErrorMessage(error)}`);
-                            throw error;
-                        }
+                const treeObjects = yield Promise.all(commit.changes.map((_a) => __awaiter(this, [_a], void 0, function* ({ path, mode, status, dstSha }) {
+                    if (mode === '160000') {
+                        // submodule
+                        core.info(`Creating tree object for submodule commit at '${path}'`);
+                        return {
+                            path,
+                            mode,
+                            sha: dstSha,
+                            type: 'commit'
+                        };
+                    }
+                    else {
+                        let sha = null;
+                        if (status === 'A' || status === 'M') {
+                            try {
+                                const { data: blob } = yield blobCreationLimit(() => this.octokit.rest.git.createBlob(Object.assign(Object.assign({}, repository), { content: utils.readFileBase64([repoPath, path]), encoding: 'base64' })));
+                                sha = blob.sha;
+                            }
+                            catch (error) {
+                                core.error(`Error creating blob for file '${path}': ${utils.getErrorMessage(error)}`);
+                                throw error;
+                            }
+                        }
+                        core.info(`Creating tree object for blob at '${path}' with status '${status}'`);
+                        return {
+                            path,
+                            mode,
+                            sha,
+                            type: 'blob'
+                        };
                    }
-                    core.info(`Created blob for file '${path}'`);
-                    return {
-                        path,
-                        mode,
-                        sha,
-                        type: 'blob'
-                    };
                })));
                const chunkSize = 100;
                const chunkedTreeObjects = Array.from({ length: Math.ceil(treeObjects.length / chunkSize) }, (_, i) => treeObjects.slice(i * chunkSize, i * chunkSize + chunkSize));
@ -1767,7 +1781,13 @@ function readFile(path) {
    return fs.readFileSync(path, 'utf-8');
 }
 function readFileBase64(pathParts) {
-    return fs.readFileSync(path.resolve(...pathParts)).toString('base64');
+    const resolvedPath = path.resolve(...pathParts);
+    if (fs.lstatSync(resolvedPath).isSymbolicLink()) {
+        return fs
+            .readlinkSync(resolvedPath, { encoding: 'buffer' })
+            .toString('base64');
+    }
+    return fs.readFileSync(resolvedPath).toString('base64');
 }
 /* eslint-disable  @typescript-eslint/no-explicit-any */
 function hasErrorCode(error) {
--- a/docs/concepts-guidelines.md
+++ b/docs/concepts-guidelines.md
@ -303,7 +303,7 @@ GitHub App generated tokens can be configured with fine-grained permissions and
    - Uncheck `Active` under `Webhook`. You do not need to enter a `Webhook URL`.
    - Under `Repository permissions: Contents` select `Access: Read & write`.
    - Under `Repository permissions: Pull requests` select `Access: Read & write`.
-    - Under `Repository permissions: Workflows` select `Access: Read-only`.
+    - Under `Repository permissions: Workflows` select `Access: Read & write`.
      - **NOTE**: Only needed if pull requests could contain changes to Actions workflows.
    - Under `Organization permissions: Members` select `Access: Read-only`.
      - **NOTE**: Only needed if you would like add teams as reviewers to PRs.
--- a/package-lock.json
+++ b/package-lock.json
@ -21,12 +21,12 @@
        "uuid": "^9.0.1"
      },
      "devDependencies": {
-        "@types/jest": "^29.5.12",
+        "@types/jest": "^29.5.13",
        "@types/node": "^18.19.50",
        "@typescript-eslint/eslint-plugin": "^7.18.0",
        "@typescript-eslint/parser": "^7.18.0",
        "@vercel/ncc": "^0.38.1",
-        "eslint": "^8.57.0",
+        "eslint": "^8.57.1",
        "eslint-import-resolver-typescript": "^3.6.3",
        "eslint-plugin-github": "^4.10.2",
        "eslint-plugin-import": "^2.30.0",
@ -735,9 +735,9 @@
      }
    },
    "node_modules/@eslint/js": {
-      "version": "8.57.0",
-      "resolved": "https://registry.npmjs.org/@eslint/js/-/js-8.57.0.tgz",
-      "integrity": "sha512-Ys+3g2TaW7gADOJzPt83SJtCDhMjndcDMFVQ/Tj9iA1BfJzFKD9mAUXT3OenpuPHbI6P/myECxRJrofUsDx/5g==",
+      "version": "8.57.1",
+      "resolved": "https://registry.npmjs.org/@eslint/js/-/js-8.57.1.tgz",
+      "integrity": "sha512-d9zaMRSTIKDLhctzH12MtXvJKSSUhaHcjV+2Z+GK+EEY7XKpP5yR4x+N3TAcHTcu963nIr+TMcCb4DBCYX1z6Q==",
      "dev": true,
      "engines": {
        "node": "^12.22.0 || ^14.17.0 || >=16.0.0"
@ -758,13 +758,13 @@
      "dev": true
    },
    "node_modules/@humanwhocodes/config-array": {
-      "version": "0.11.14",
-      "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.14.tgz",
-      "integrity": "sha512-3T8LkOmg45BV5FICb15QQMsyUSWrQ8AygVfC7ZG32zOalnqrilm018ZVCw0eapXux8FtA33q8PSRSstjee3jSg==",
+      "version": "0.13.0",
+      "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.13.0.tgz",
+      "integrity": "sha512-DZLEEqFWQFiyK6h5YIeynKx7JlvCYWL0cImfSRXZ9l4Sg2efkFGTuFf6vzXjK1cq6IYkU+Eg/JizXw+TD2vRNw==",
      "deprecated": "Use @eslint/config-array instead",
      "dev": true,
      "dependencies": {
-        "@humanwhocodes/object-schema": "^2.0.2",
+        "@humanwhocodes/object-schema": "^2.0.3",
        "debug": "^4.3.1",
        "minimatch": "^3.0.5"
      },
@ -1527,9 +1527,9 @@
      }
    },
    "node_modules/@types/jest": {
-      "version": "29.5.12",
-      "resolved": "https://registry.npmjs.org/@types/jest/-/jest-29.5.12.tgz",
-      "integrity": "sha512-eDC8bTvT/QhYdxJAulQikueigY5AsdBRH2yDKW3yveW7svY3+DzN84/2NUgkw10RTiJbWqZrTtoGVdYlvFJdLw==",
+      "version": "29.5.13",
+      "resolved": "https://registry.npmjs.org/@types/jest/-/jest-29.5.13.tgz",
+      "integrity": "sha512-wd+MVEZCHt23V0/L642O5APvspWply/rGY5BcW4SUETo2UzPU3Z26qr8jC2qxpimI2jjx9h7+2cj2FwIr01bXg==",
      "dev": true,
      "dependencies": {
        "expect": "^29.0.0",
@ -3501,16 +3501,16 @@
      }
    },
    "node_modules/eslint": {
-      "version": "8.57.0",
-      "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.57.0.tgz",
-      "integrity": "sha512-dZ6+mexnaTIbSBZWgou51U6OmzIhYM2VcNdtiTtI7qPNZm35Akpr0f6vtw3w1Kmn5PYo+tZVfh13WrhpS6oLqQ==",
+      "version": "8.57.1",
+      "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.57.1.tgz",
+      "integrity": "sha512-ypowyDxpVSYpkXr9WPv2PAZCtNip1Mv5KTW0SCurXv/9iOpcrH9PaqUElksqEB6pChqHGDRCFTyrZlGhnLNGiA==",
      "dev": true,
      "dependencies": {
        "@eslint-community/eslint-utils": "^4.2.0",
        "@eslint-community/regexpp": "^4.6.1",
        "@eslint/eslintrc": "^2.1.4",
-        "@eslint/js": "8.57.0",
-        "@humanwhocodes/config-array": "^0.11.14",
+        "@eslint/js": "8.57.1",
+        "@humanwhocodes/config-array": "^0.13.0",
        "@humanwhocodes/module-importer": "^1.0.1",
        "@nodelib/fs.walk": "^1.2.8",
        "@ungap/structured-clone": "^1.2.0",
--- a/package.json
+++ b/package.json
@ -41,12 +41,12 @@
    "uuid": "^9.0.1"
  },
  "devDependencies": {
-    "@types/jest": "^29.5.12",
+    "@types/jest": "^29.5.13",
    "@types/node": "^18.19.50",
    "@typescript-eslint/eslint-plugin": "^7.18.0",
    "@typescript-eslint/parser": "^7.18.0",
    "@vercel/ncc": "^0.38.1",
-    "eslint": "^8.57.0",
+    "eslint": "^8.57.1",
    "eslint-import-resolver-typescript": "^3.6.3",
    "eslint-plugin-github": "^4.10.2",
    "eslint-plugin-import": "^2.30.0",
--- a/src/git-command-manager.ts
+++ b/src/git-command-manager.ts
@ -14,6 +14,7 @@ export type Commit = {
  body: string
  changes: {
    mode: string
+    dstSha: string
    status: 'A' | 'M' | 'D'
    path: string
  }[]
@ -160,6 +161,7 @@ export class GitCommandManager {
      '--raw',
      '--cc',
      '--no-renames',
+      '--no-abbrev',
      `--format=%H%n%T%n%P%n%G?%n%s%n%b%n${endOfBody}`,
      ref
    ])
@ -177,13 +179,14 @@ export class GitCommandManager {
      body: detailLines.slice(5, endOfBodyIndex).join('\n'),
      changes: lines.slice(endOfBodyIndex + 2, -1).map(line => {
        const change = line.match(
-          /^:(\d{6}) (\d{6}) \w{7} \w{7} ([AMD])\s+(.*)$/
+          /^:(\d{6}) (\d{6}) \w{40} (\w{40}) ([AMD])\s+(.*)$/
        )
        if (change) {
          return {
-            mode: change[3] === 'D' ? change[1] : change[2],
-            status: change[3],
-            path: change[4]
+            mode: change[4] === 'D' ? change[1] : change[2],
+            dstSha: change[3],
+            status: change[4],
+            path: change[5]
          }
        } else {
          unparsedChanges.push(line)
--- a/src/github-helper.ts
+++ b/src/github-helper.ts
@ -35,7 +35,7 @@ type TreeObject = {
  path: string
  mode: '100644' | '100755' | '040000' | '160000' | '120000'
  sha: string | null
-  type: 'blob'
+  type: 'blob' | 'commit'
 }

 export class GitHubHelper {
@ -255,31 +255,44 @@ export class GitHubHelper {
    if (commit.changes.length > 0) {
      core.info(`Creating tree objects for local commit ${commit.sha}`)
      const treeObjects = await Promise.all(
-        commit.changes.map(async ({path, mode, status}) => {
-          let sha: string | null = null
-          if (status === 'A' || status === 'M') {
-            try {
-              const {data: blob} = await blobCreationLimit(() =>
-                this.octokit.rest.git.createBlob({
-                  ...repository,
-                  content: utils.readFileBase64([repoPath, path]),
-                  encoding: 'base64'
-                })
-              )
-              sha = blob.sha
-            } catch (error) {
-              core.error(
-                `Error creating blob for file '${path}': ${utils.getErrorMessage(error)}`
-              )
-              throw error
+        commit.changes.map(async ({path, mode, status, dstSha}) => {
+          if (mode === '160000') {
+            // submodule
+            core.info(`Creating tree object for submodule commit at '${path}'`)
+            return <TreeObject>{
+              path,
+              mode,
+              sha: dstSha,
+              type: 'commit'
+            }
+          } else {
+            let sha: string | null = null
+            if (status === 'A' || status === 'M') {
+              try {
+                const {data: blob} = await blobCreationLimit(() =>
+                  this.octokit.rest.git.createBlob({
+                    ...repository,
+                    content: utils.readFileBase64([repoPath, path]),
+                    encoding: 'base64'
+                  })
+                )
+                sha = blob.sha
+              } catch (error) {
+                core.error(
+                  `Error creating blob for file '${path}': ${utils.getErrorMessage(error)}`
+                )
+                throw error
+              }
+            }
+            core.info(
+              `Creating tree object for blob at '${path}' with status '${status}'`
+            )
+            return <TreeObject>{
+              path,
+              mode,
+              sha,
+              type: 'blob'
            }
-          }
-          core.info(`Created blob for file '${path}'`)
-          return <TreeObject>{
-            path,
-            mode,
-            sha,
-            type: 'blob'
          }
        })
      )
--- a/src/utils.ts
+++ b/src/utils.ts
@ -127,7 +127,13 @@ export function readFile(path: string): string {
 }

 export function readFileBase64(pathParts: string[]): string {
-  return fs.readFileSync(path.resolve(...pathParts)).toString('base64')
+  const resolvedPath = path.resolve(...pathParts)
+  if (fs.lstatSync(resolvedPath).isSymbolicLink()) {
+    return fs
+      .readlinkSync(resolvedPath, {encoding: 'buffer'})
+      .toString('base64')
+  }
+  return fs.readFileSync(resolvedPath).toString('base64')
 }

 /* eslint-disable  @typescript-eslint/no-explicit-any */
Author	SHA1	Message	Date
Peter Evans	5e914681df	fix: support symlinks when commit signing (#3359 )	2024-09-18 10:40:10 -07:00
Peter Evans	2f38cd26bf	fix: support submodules when commit signing (#3354 ) * fix: support submodules when commit signing * create correct tree object for submodule * update log messages	2024-09-18 09:46:39 -07:00
dependabot[bot]	7a8aeac749	build(deps-dev): bump eslint from 8.57.0 to 8.57.1 (#3344 ) Bumps [eslint](https://github.com/eslint/eslint) from 8.57.0 to 8.57.1. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md) - [Commits](https://github.com/eslint/eslint/compare/v8.57.0...v8.57.1) --- updated-dependencies: - dependency-name: eslint dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2024-09-17 12:34:38 +00:00
dependabot[bot]	d39d596a77	build(deps-dev): bump @types/jest from 29.5.12 to 29.5.13 (#3343 ) Bumps [@types/jest](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/jest) from 29.5.12 to 29.5.13. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/jest) --- updated-dependencies: - dependency-name: "@types/jest" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2024-09-17 12:34:19 +00:00
Henry Schreiner	f6f978fd3d	docs: correct suggestion for bot setup (#3342 ) The option suggested here (Read only) is not an option, and wouldn't mean/do anything - you can read these files if you have access to the repo files. The description says this is needed if the PR could change the workflow files, so you need "Read and Write". Pretty sure this is just a typo, copied from the line below instead of the line above.	2024-09-16 09:49:30 -07:00
Peter Evans	6cd32fd936	fix: disable abbreviated commit shas in diff (#3337 )	2024-09-16 02:17:19 -07:00