Merge pull request #58 from dorny/unquoted-shell-escape

Improved listing of matching files with `list-files: shell` and `list-files: escape` options

.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

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: ''
 

__tests__/shell-escape.test.ts

@@ -1,16 +1,57 @@
-import shellEscape from '../src/shell-escape'
+import {escape, shellEscape} from '../src/shell-escape'
 
-test('simple path escaped', () => {
-  expect(shellEscape('file')).toBe("'file'")
+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('path with space is wrapped with single quotes', () => {
-  expect(shellEscape('file with space')).toBe("'file with space'")
-})
+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('path with quote is divided into quoted segments and escaped quote', () => {
-  expect(shellEscape("file'with quote")).toBe("'file'\\''with quote'")
-})
-test('path with leading quote does not have double quotes at beginning', () => {
-  expect(shellEscape("'file-leading-quote")).toBe("\\''file-leading-quote'")
+  test('directory separator should be preserved and not escaped', () => {
+    expect(shellEscape('path/to/file.txt')).toBe('path/to/file.txt')
+  })
+
+  test('filename with spaces should be quoted', () => {
+    expect(shellEscape('file with space')).toBe("'file with space'")
+  })
+
+  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'")
+  })
 })

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:

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();
 
@@ -15222,14 +15221,34 @@ module.exports = require("fs");
 
 "use strict";
 
-// Credits to https://github.com/xxorax/node-shell-escape
 Object.defineProperty(exports, "__esModule", { value: true });
-function shellEscape(value) {
-    return `'${value.replace(/'/g, "'\\''")}'`
-        .replace(/^(?:'')+/g, '') // unduplicate single-quote at the beginning
-        .replace(/\\'''/g, "\\'"); // remove non-escaped single-quote if there are enclosed between 2 escaped
+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;
 
 
 /***/ }),

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()

src/shell-escape.ts

@@ -1,7 +1,28 @@
-// Credits to https://github.com/xxorax/node-shell-escape
-
-export default function shellEscape(value: string): string {
-  return `'${value.replace(/'/g, "'\\''")}'`
-    .replace(/^(?:'')+/g, '') // unduplicate single-quote at the beginning
-    .replace(/\\'''/g, "\\'") // remove non-escaped single-quote if there are enclosed between 2 escaped
+// 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}'`
 }