Merge pull request #7830 from kennethzfeng/kfeng.is-execute

plugins/scaffolder-backend: Fix a bug where only file mode 775 is con…

.changeset/wet-pears-thank.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Fix a bug where only file mode 775 is considered an executable

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts

@@ -254,7 +254,7 @@ describe('createPublishGithubPullRequestAction', () => {
     });
   });
 
-  describe('with executable file', () => {
+  describe('with executable file mode 755', () => {
     let input: GithubPullRequestActionInput;
     let ctx: ActionContext<GithubPullRequestActionInput>;
 
@@ -268,9 +268,9 @@ describe('createPublishGithubPullRequestAction', () => {
 
       mockFs({
         [workspacePath]: {
-          'file.txt': mockFs.file({
-            content: 'Hello there!',
-            mode: 33277, // File mode: 100755
+          'hello.sh': mockFs.file({
+            content: 'echo Hello there!',
+            mode: 0o100755,
           }),
         },
       });
@@ -297,8 +297,76 @@ describe('createPublishGithubPullRequestAction', () => {
           {
             commit: 'Create my new app',
             files: {
-              'file.txt': {
-                content: Buffer.from('Hello there!').toString('base64'),
+              'hello.sh': {
+                content: Buffer.from('echo Hello there!').toString('base64'),
+                encoding: 'base64',
+                mode: '100755',
+              },
+            },
+          },
+        ],
+      });
+    });
+
+    it('creates outputs for the url', async () => {
+      await instance.handler(ctx);
+
+      expect(ctx.output).toHaveBeenCalledWith(
+        'remoteUrl',
+        'https://github.com/myorg/myrepo/pull/123',
+      );
+    });
+    afterEach(() => {
+      mockFs.restore();
+      jest.resetAllMocks();
+    });
+  });
+
+  describe('with executable file mode 775', () => {
+    let input: GithubPullRequestActionInput;
+    let ctx: ActionContext<GithubPullRequestActionInput>;
+
+    beforeEach(() => {
+      input = {
+        repoUrl: 'github.com?owner=myorg&repo=myrepo',
+        title: 'Create my new app',
+        branchName: 'new-app',
+        description: 'This PR is really good',
+      };
+
+      mockFs({
+        [workspacePath]: {
+          'hello.sh': mockFs.file({
+            content: 'echo Hello there!',
+            mode: 0o100775,
+          }),
+        },
+      });
+
+      ctx = {
+        createTemporaryDirectory: jest.fn(),
+        output: jest.fn(),
+        logger: getRootLogger(),
+        logStream: new Writable(),
+        input,
+        workspacePath,
+      };
+    });
+    it('creates a pull request', async () => {
+      await instance.handler(ctx);
+
+      expect(fakeClient.createPullRequest).toHaveBeenCalledWith({
+        owner: 'myorg',
+        repo: 'myrepo',
+        title: 'Create my new app',
+        head: 'new-app',
+        body: 'This PR is really good',
+        changes: [
+          {
+            commit: 'Create my new app',
+            files: {
+              'hello.sh': {
+                content: Buffer.from('echo Hello there!').toString('base64'),
                 encoding: 'base64',
                 mode: '100755',
               },

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts

@@ -16,7 +16,7 @@
 
 import fs from 'fs-extra';
 import path from 'path';
-import { parseRepoUrl } from './util';
+import { parseRepoUrl, isExecutable } from './util';
 
 import {
   GithubCredentialsProvider,
@@ -202,10 +202,11 @@ export const createPublishGithubPullRequestAction = ({
             .readFileSync(absPath)
             .toString('base64');
           const fileStat = fs.statSync(absPath);
-          const isExecutable = fileStat.mode === 33277; // aka. 100755
           // See the properties of tree items
           // in https://docs.github.com/en/rest/reference/git#trees
-          const githubTreeItemMode = isExecutable ? '100755' : '100644';
+          const githubTreeItemMode = isExecutable(fileStat.mode)
+            ? '100755'
+            : '100644';
           // Always use base64 encoding to avoid doubling a binary file in size
           // due to interpreting a binary file as utf-8 and sending github
           // the utf-8 encoded content.

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.test.ts

@@ -15,7 +15,7 @@
  */
 
 import path from 'path';
-import { getRepoSourceDirectory } from './util';
+import { getRepoSourceDirectory, isExecutable } from './util';
 
 describe('getRepoSourceDirectory', () => {
   it('should return workspace root if no sub folder is given', () => {
@@ -55,3 +55,33 @@ describe('getRepoSourceDirectory', () => {
     ).toEqual(path.join('/', 'var', 'workspace', 'absolute', 'secret'));
   });
 });
+
+describe('isExecutable', () => {
+  it('should return true for file mode 777', () => {
+    expect(isExecutable(0o100777)).toBe(true);
+  });
+  it('should return true for file mode 775', () => {
+    expect(isExecutable(0o100775)).toBe(true);
+  });
+  it('should return true for file mode 755', () => {
+    expect(isExecutable(0o100755)).toBe(true);
+  });
+  it('should return true for file mode 700', () => {
+    expect(isExecutable(0o100700)).toBe(true);
+  });
+  it('should return true for file mode 770', () => {
+    expect(isExecutable(0o100770)).toBe(true);
+  });
+  it('should return true for file mode 670', () => {
+    expect(isExecutable(0o100670)).toBe(true);
+  });
+  it('should return false for file mode 644', () => {
+    expect(isExecutable(0o100644)).toBe(false);
+  });
+  it('should return false for file mode 600', () => {
+    expect(isExecutable(0o100600)).toBe(false);
+  });
+  it('should return false for file mode 640', () => {
+    expect(isExecutable(0o100640)).toBe(false);
+  });
+});

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts

@@ -101,3 +101,8 @@ export const parseRepoUrl = (
 
   return { host, owner, repo, organization, workspace, project };
 };
+export const isExecutable = (fileMode: number) => {
+  const executeBitMask = 0o000111;
+  const res = fileMode & executeBitMask;
+  return res > 0;
+};