From e4d886f50334b309621f18374f57aef30a4b38db Mon Sep 17 00:00:00 2001
From: Michal Dorner <dorner.michal@gmail.com>
Date: Sun, 13 Dec 2020 21:07:47 +0100
Subject: [PATCH] Provide `shell` and `escape` options when formatting matching
 files

---
 .../workflows/pull-request-verification.yml   |  9 +--
 README.md                                     | 14 ++--
 __tests__/shell-escape.test.ts                | 64 +++++++++++++++----
 action.yml                                    |  7 +-
 dist/index.js                                 | 40 +++++++++---
 src/main.ts                                   |  8 ++-
 src/shell-escape.ts                           | 29 ++++++++-
 7 files changed, 124 insertions(+), 47 deletions(-)

diff --git a/.github/workflows/pull-request-verification.yml b/.github/workflows/pull-request-verification.yml
index 66e001c..27627a9 100644
--- a/.github/workflows/pull-request-verification.yml
+++ b/.github/workflows/pull-request-verification.yml
@@ -119,15 +119,12 @@ jobs:
     - name: Print 'deleted_files'
       run: echo ${{steps.filter.outputs.deleted_files}}
     - name: filter-test
-      # only single quotes are supported in GH action literal
-      # single quote needs to be escaped with single quote
-      # '''add.txt''' resolves to string 'add.txt'
       if: |
         steps.filter.outputs.added != 'true'
         || steps.filter.outputs.deleted != 'true'
         || steps.filter.outputs.modified != 'true'
         || steps.filter.outputs.any != 'true'
-        || steps.filter.outputs.added_files != '''add.txt'''
-        || steps.filter.outputs.modified_files != '''LICENSE'''
-        || steps.filter.outputs.deleted_files != '''README.md'''
+        || steps.filter.outputs.added_files != 'add.txt'
+        || steps.filter.outputs.modified_files != 'LICENSE'
+        || steps.filter.outputs.deleted_files != 'README.md'
       run: exit 1
diff --git a/README.md b/README.md
index 0bb10be..145ecc0 100644
--- a/README.md
+++ b/README.md
@@ -66,18 +66,12 @@ For more scenarios see [examples](#examples) section.
 
 
 # What's New
+- Improved listing of matching files with `list-files: shell` and `list-files: escape` options
 - Support local changes
 - Fixed retrieval of all changes via Github API when there are 100+ changes
 - Paths expressions are now evaluated using [picomatch](https://github.com/micromatch/picomatch) library
 - Support workflows triggered by any event
 - Fixed compatibility with older (<2.23) versions of git
-- Support for tag pushes and tags as a base reference
-- Fixes for various edge cases when event payload is incomplete
-  - Supports local execution with [act](https://github.com/nektos/act)
-- Fixed behavior of feature branch workflow:
-  - Detects only changes introduced by feature branch. Later modifications on base branch are ignored
-- Filter by type of file change:
-  - Optionally consider if file was added, modified or deleted
 
 For more information see [CHANGELOG](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
 
@@ -128,8 +122,10 @@ For more information see [CHANGELOG](https://github.com/actions/checkout/blob/ma
     # Enables listing of files matching the filter:
     #   'none'  - Disables listing of matching files (default).
     #   'json'  - Matching files paths are formatted as JSON array.
-    #   'shell' - Matching files paths are escaped and space-delimited.
-    #             Output is usable as command line argument list in linux shell.
+    #   'shell' - Space delimited list usable as command line argument list in linux shell.
+    #             If needed it uses single or double quotes to wrap filename with unsafe characters.
+    #   'escape'- Space delimited list usable as command line argument list in linux shell.
+    #             Backslash escapes every potentially unsafe character.
     # Default: none
     list-files: ''
 
diff --git a/__tests__/shell-escape.test.ts b/__tests__/shell-escape.test.ts
index d03669a..e706dfb 100644
--- a/__tests__/shell-escape.test.ts
+++ b/__tests__/shell-escape.test.ts
@@ -1,21 +1,57 @@
-import shellEscape from '../src/shell-escape'
+import {escape, shellEscape} from '../src/shell-escape'
 
-test('simple filename should not be modified', () => {
-  expect(shellEscape('file.txt')).toBe('file.txt')
+describe('escape() backslash escapes every character except subset of definitely safe characters', () => {
+  test('simple filename should not be modified', () => {
+    expect(escape('file.txt')).toBe('file.txt')
+  })
+
+  test('directory separator should be preserved and not escaped', () => {
+    expect(escape('path/to/file.txt')).toBe('path/to/file.txt')
+  })
+
+  test('spaces should be escaped with backslash', () => {
+    expect(escape('file with space')).toBe('file\\ with\\ space')
+  })
+
+  test('quotes should be escaped with backslash', () => {
+    expect(escape('file\'with quote"')).toBe('file\\\'with\\ quote\\"')
+  })
+
+  test('$variables should be escaped', () => {
+    expect(escape('$var')).toBe('\\$var')
+  })
 })
 
-test('directory separator should be preserved and not escaped', () => {
-  expect(shellEscape('path/to/file.txt')).toBe('path/to/file.txt')
-})
+describe('shellEscape() returns human readable filenames with as few escaping applied as possible', () => {
+  test('simple filename should not be modified', () => {
+    expect(shellEscape('file.txt')).toBe('file.txt')
+  })
 
-test('spaces should be escaped with backslash', () => {
-  expect(shellEscape('file with space')).toBe('file\\ with\\ space')
-})
+  test('directory separator should be preserved and not escaped', () => {
+    expect(shellEscape('path/to/file.txt')).toBe('path/to/file.txt')
+  })
 
-test('quotes should be escaped  with backslash', () => {
-  expect(shellEscape('file\'with quote"')).toBe('file\\\'with\\ quote\\"')
-})
+  test('filename with spaces should be quoted', () => {
+    expect(shellEscape('file with space')).toBe("'file with space'")
+  })
 
-test('$variables sould be escaped', () => {
-  expect(shellEscape('$var')).toBe('\\$var')
+  test('filename with spaces should be quoted', () => {
+    expect(shellEscape('file with space')).toBe("'file with space'")
+  })
+
+  test('filename with $ should be quoted', () => {
+    expect(shellEscape('$var')).toBe("'$var'")
+  })
+
+  test('filename with " should be quoted', () => {
+    expect(shellEscape('file"name')).toBe("'file\"name'")
+  })
+
+  test('filename with single quote should be wrapped in double quotes', () => {
+    expect(shellEscape("file'with quote")).toBe('"file\'with quote"')
+  })
+
+  test('filename with single quote and special characters is split and quoted/escaped as needed', () => {
+    expect(shellEscape("file'with $quote")).toBe("file\\''with $quote'")
+  })
 })
diff --git a/action.yml b/action.yml
index 76938e8..d1321d6 100644
--- a/action.yml
+++ b/action.yml
@@ -22,8 +22,11 @@ inputs:
     description: |
       Enables listing of files matching the filter:
         'none'  - Disables listing of matching files (default).
-        'json'  - Matching files paths are serialized as JSON array.
-        'shell' - Matching files paths are escaped and space-delimited. Output is usable as command line argument list in linux shell.
+        'json'  - Serialized as JSON array.
+        'shell' - Space delimited list usable as command line argument list in linux shell.
+                  If needed it uses single or double quotes to wrap filename with unsafe characters.
+        'escape'- Space delimited list usable as command line argument list in linux shell.
+                  Backslash escapes every potentially unsafe character.
     required: true
     default: none
   initial-fetch-depth:
diff --git a/dist/index.js b/dist/index.js
index c0b7926..953026b 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -4641,9 +4641,6 @@ var __importStar = (this && this.__importStar) || function (mod) {
     __setModuleDefault(result, mod);
     return result;
 };
-var __importDefault = (this && this.__importDefault) || function (mod) {
-    return (mod && mod.__esModule) ? mod : { "default": mod };
-};
 Object.defineProperty(exports, "__esModule", { value: true });
 const fs = __importStar(__webpack_require__(747));
 const core = __importStar(__webpack_require__(470));
@@ -4651,7 +4648,7 @@ const github = __importStar(__webpack_require__(469));
 const filter_1 = __webpack_require__(235);
 const file_1 = __webpack_require__(258);
 const git = __importStar(__webpack_require__(136));
-const shell_escape_1 = __importDefault(__webpack_require__(751));
+const shell_escape_1 = __webpack_require__(751);
 async function run() {
     try {
         const workingDirectory = core.getInput('working-directory', { required: false });
@@ -4819,14 +4816,16 @@ function serializeExport(files, format) {
     switch (format) {
         case 'json':
             return JSON.stringify(fileNames);
+        case 'escape':
+            return fileNames.map(shell_escape_1.escape).join(' ');
         case 'shell':
-            return fileNames.map(shell_escape_1.default).join(' ');
+            return fileNames.map(shell_escape_1.shellEscape).join(' ');
         default:
             return '';
     }
 }
 function isExportFormat(value) {
-    return value === 'none' || value === 'shell' || value === 'json';
+    return value === 'none' || value === 'shell' || value === 'json' || value === 'escape';
 }
 run();
 
@@ -15223,12 +15222,33 @@ module.exports = require("fs");
 "use strict";
 
 Object.defineProperty(exports, "__esModule", { value: true });
-// Uses easy safe set of characters which can be left unescaped to keep it readable.
-// Every other character will be backslash-escaped
-function shellEscape(value) {
+exports.shellEscape = exports.escape = void 0;
+// Backslash escape every character except small subset of definitely safe characters
+function escape(value) {
     return value.replace(/([^a-zA-Z0-9,._+:@%/-])/gm, '\\$1');
 }
-exports.default = shellEscape;
+exports.escape = escape;
+// Returns filename escaped for usage as shell argument.
+// Applies "human readable" approach with as few escaping applied as possible
+function shellEscape(value) {
+    if (value === '')
+        return value;
+    // Only safe characters
+    if (/^[a-zA-Z0-9,._+:@%/-]+$/m.test(value)) {
+        return value;
+    }
+    if (value.includes("'")) {
+        // Only safe characters, single quotes and white-spaces
+        if (/^[a-zA-Z0-9,._+:@%/'\s-]+$/m.test(value)) {
+            return `"${value}"`;
+        }
+        // Split by single quote and apply escaping recursively
+        return value.split("'").map(shellEscape).join("\\'");
+    }
+    // Contains some unsafe characters but no single quote
+    return `'${value}'`;
+}
+exports.shellEscape = shellEscape;
 
 
 /***/ }),
diff --git a/src/main.ts b/src/main.ts
index 6464367..176dafc 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -6,9 +6,9 @@ import {Webhooks} from '@octokit/webhooks'
 import {Filter, FilterResults} from './filter'
 import {File, ChangeStatus} from './file'
 import * as git from './git'
-import shellEscape from './shell-escape'
+import {escape, shellEscape} from './shell-escape'
 
-type ExportFormat = 'none' | 'json' | 'shell'
+type ExportFormat = 'none' | 'json' | 'shell' | 'escape'
 
 async function run(): Promise<void> {
   try {
@@ -201,6 +201,8 @@ function serializeExport(files: File[], format: ExportFormat): string {
   switch (format) {
     case 'json':
       return JSON.stringify(fileNames)
+    case 'escape':
+      return fileNames.map(escape).join(' ')
     case 'shell':
       return fileNames.map(shellEscape).join(' ')
     default:
@@ -209,7 +211,7 @@ function serializeExport(files: File[], format: ExportFormat): string {
 }
 
 function isExportFormat(value: string): value is ExportFormat {
-  return value === 'none' || value === 'shell' || value === 'json'
+  return value === 'none' || value === 'shell' || value === 'json' || value === 'escape'
 }
 
 run()
diff --git a/src/shell-escape.ts b/src/shell-escape.ts
index 643b7cc..25bb940 100644
--- a/src/shell-escape.ts
+++ b/src/shell-escape.ts
@@ -1,5 +1,28 @@
-// Uses easy safe set of characters which can be left unescaped to keep it readable.
-// Every other character will be backslash-escaped
-export default function shellEscape(value: string): string {
+// Backslash escape every character except small subset of definitely safe characters
+export function escape(value: string): string {
   return value.replace(/([^a-zA-Z0-9,._+:@%/-])/gm, '\\$1')
 }
+
+// Returns filename escaped for usage as shell argument.
+// Applies "human readable" approach with as few escaping applied as possible
+export function shellEscape(value: string): string {
+  if (value === '') return value
+
+  // Only safe characters
+  if (/^[a-zA-Z0-9,._+:@%/-]+$/m.test(value)) {
+    return value
+  }
+
+  if (value.includes("'")) {
+    // Only safe characters, single quotes and white-spaces
+    if (/^[a-zA-Z0-9,._+:@%/'\s-]+$/m.test(value)) {
+      return `"${value}"`
+    }
+
+    // Split by single quote and apply escaping recursively
+    return value.split("'").map(shellEscape).join("\\'")
+  }
+
+  // Contains some unsafe characters but no single quote
+  return `'${value}'`
+}