fix: ensure filenames passed to editors are not considered as flags (#156)

fixes GHSA-r825-w8mm-5pxw

Author: sapphi-red

packages/launch-editor/get-args.js

@@ -3,6 +3,8 @@ const path = require('path')
 
 // normalize a file and optional line/column numbers into command line args for specific editors
 module.exports = function getArgumentsForFile(editor, fileName, lineNumber, columnNumber = 1) {
+  fileName = path.resolve(fileName)
+
   if (
     process.platform === 'linux' &&
     fileName.startsWith('/mnt/') &&
@@ -14,7 +16,7 @@ module.exports = function getArgumentsForFile(editor, fileName, lineNumber, colu
     // build of WSL, see: https://github.com/Microsoft/BashOnWindows/issues/423#issuecomment-221627364
     // When a Windows editor is specified, interop functionality can
     // handle the path translation, but only if a relative path is used.
-    fileName = path.relative('', fileName)
+    fileName = `./${path.relative('', fileName)}`
   }
 
   if (!lineNumber) {

packages/launch-editor/get-args.test.js

@@ -1,25 +1,49 @@
 const assert = require('node:assert/strict')
 const os = require('node:os')
+const path = require('node:path')
 const { describe, test } = require('node:test')
 
 const getArgumentsForFile = require('./get-args.js')
+const absoluteFileName = path.resolve('file')
 
 describe('getArgumentsForFile', () => {
-  test('uses the file name as the only argument when no position is given', () => {
-    assert.deepEqual(getArgumentsForFile('vim', 'file'), ['file'])
+  test('uses an absolute file name as the only argument when no position is given', () => {
+    assert.deepEqual(getArgumentsForFile('vim', absoluteFileName), [absoluteFileName])
   })
 
-  test('uses a relative path for files on a Windows drive in WSL', (t) => {
+  test('resolves a relative file name to an absolute path', () => {
+    assert.deepEqual(getArgumentsForFile('vim', 'file'), [absoluteFileName])
+  })
+
+  describe('WSL handling', () => {
     if (process.platform !== 'linux') {
-      t.skip('not running on Linux')
       return
     }
 
-    t.mock.method(process, 'cwd', () => '/home/user/project')
-    t.mock.method(os, 'release', () => '4.4.0-43-Microsoft')
+    test('uses a relative path for files on a Windows drive in WSL', (t) => {
+      t.mock.method(process, 'cwd', () => '/home/user/project')
+      t.mock.method(os, 'release', () => '4.4.0-43-Microsoft')
+
+      const fileName = '/mnt/c/project/file.js'
+      assert.deepEqual(getArgumentsForFile('vim', fileName), ['./../../../mnt/c/project/file.js'])
+    })
+
+    test('resolves a relative path before converting it for WSL', (t) => {
+      t.mock.method(process, 'cwd', () => '/mnt/c/project')
+      t.mock.method(os, 'release', () => '4.4.0-43-Microsoft')
+
+      assert.deepEqual(getArgumentsForFile('vim', 'file.js'), ['./file.js'])
+    })
 
-    const fileName = '/mnt/c/project/file.js'
-    assert.deepEqual(getArgumentsForFile('vim', fileName), ['../../../mnt/c/project/file.js'])
+    test('prefixes an option-like relative path on a Windows drive in WSL', (t) => {
+      t.mock.method(process, 'cwd', () => '/mnt/c/project')
+      t.mock.method(os, 'release', () => '4.4.0-43-Microsoft')
+
+      const fileName = '+call system("touch LAUNCH_EDITOR_MARKER")'
+      assert.deepEqual(getArgumentsForFile('vim', fileName), [
+        './+call system("touch LAUNCH_EDITOR_MARKER")',
+      ])
+    })
   })
 
   describe('editor-specific argument formats', () => {
@@ -35,38 +59,54 @@ describe('getArgumentsForFile', () => {
       'zed',
     ]) {
       test(`${editor} uses "file:line:column"`, () => {
-        assert.deepEqual(getArgumentsForFile(editor, 'file', 10, 5), ['file:10:5'])
+        assert.deepEqual(getArgumentsForFile(editor, absoluteFileName, 10, 5), [
+          `${absoluteFileName}:10:5`,
+        ])
       })
     }
 
     test('notepad++ uses -n/-c flags', () => {
-      assert.deepEqual(getArgumentsForFile('notepad++', 'file', 10, 5), ['-n10', '-c5', 'file'])
+      assert.deepEqual(getArgumentsForFile('notepad++', absoluteFileName, 10, 5), [
+        '-n10',
+        '-c5',
+        absoluteFileName,
+      ])
     })
 
     for (const editor of ['vim', 'mvim']) {
       test(`${editor} uses +call cursor()`, () => {
-        assert.deepEqual(getArgumentsForFile(editor, 'file', 10, 5), [
+        assert.deepEqual(getArgumentsForFile(editor, absoluteFileName, 10, 5), [
           '+call cursor(10, 5)',
-          'file',
+          absoluteFileName,
         ])
       })
     }
 
     for (const editor of ['joe', 'gvim']) {
       test(`${editor} uses +line`, () => {
-        assert.deepEqual(getArgumentsForFile(editor, 'file', 10, 5), ['+10', 'file'])
+        assert.deepEqual(getArgumentsForFile(editor, absoluteFileName, 10, 5), [
+          '+10',
+          absoluteFileName,
+        ])
       })
     }
 
     for (const editor of ['emacs', 'emacsclient']) {
       test(`${editor} uses +line:column`, () => {
-        assert.deepEqual(getArgumentsForFile(editor, 'file', 10, 5), ['+10:5', 'file'])
+        assert.deepEqual(getArgumentsForFile(editor, absoluteFileName, 10, 5), [
+          '+10:5',
+          absoluteFileName,
+        ])
       })
     }
 
     for (const editor of ['rmate', 'mate', 'mine']) {
       test(`${editor} uses --line`, () => {
-        assert.deepEqual(getArgumentsForFile(editor, 'file', 10, 5), ['--line', 10, 'file'])
+        assert.deepEqual(getArgumentsForFile(editor, absoluteFileName, 10, 5), [
+          '--line',
+          10,
+          absoluteFileName,
+        ])
       })
     }
 
@@ -83,56 +123,96 @@ describe('getArgumentsForFile', () => {
       'VSCodium',
     ]) {
       test(`${editor} uses -r -g "file:line:column"`, () => {
-        assert.deepEqual(getArgumentsForFile(editor, 'file', 10, 5), ['-r', '-g', 'file:10:5'])
+        assert.deepEqual(getArgumentsForFile(editor, absoluteFileName, 10, 5), [
+          '-r',
+          '-g',
+          `${absoluteFileName}:10:5`,
+        ])
       })
     }
 
     for (const editor of ['idea', 'idea64', 'webstorm', 'pycharm', 'clion', 'rider']) {
       test(`${editor} uses --line/--column`, () => {
-        assert.deepEqual(getArgumentsForFile(editor, 'file', 10, 5), [
+        assert.deepEqual(getArgumentsForFile(editor, absoluteFileName, 10, 5), [
           '--line',
           10,
           '--column',
           5,
-          'file',
+          absoluteFileName,
         ])
       })
     }
   })
 
   test('defaults columnNumber to 1 when omitted', () => {
-    assert.deepEqual(getArgumentsForFile('code', 'file', 10), ['-r', '-g', 'file:10:1'])
+    assert.deepEqual(getArgumentsForFile('code', absoluteFileName, 10), [
+      '-r',
+      '-g',
+      `${absoluteFileName}:10:1`,
+    ])
   })
 
   describe('editor resolution via path.basename and extension stripping', () => {
     test('resolves a full POSIX path', () => {
-      assert.deepEqual(getArgumentsForFile('/usr/local/bin/code', 'file', 10, 5), [
+      assert.deepEqual(getArgumentsForFile('/usr/local/bin/code', absoluteFileName, 10, 5), [
         '-r',
         '-g',
-        'file:10:5',
+        `${absoluteFileName}:10:5`,
       ])
     })
 
     if (process.platform === 'win32') {
       test('resolves a full Windows path with .exe', () => {
-        assert.deepEqual(getArgumentsForFile('C:\\path\\Code.exe', 'file', 10, 5), [
+        assert.deepEqual(getArgumentsForFile('C:\\path\\Code.exe', absoluteFileName, 10, 5), [
           '-r',
           '-g',
-          'file:10:5',
+          `${absoluteFileName}:10:5`,
         ])
       })
 
       test('resolves notepad++ from a full path with .exe', () => {
-        assert.deepEqual(getArgumentsForFile('C:\\tools\\notepad++.exe', 'file', 10, 5), [
+        assert.deepEqual(getArgumentsForFile('C:\\tools\\notepad++.exe', absoluteFileName, 10, 5), [
           '-n10',
           '-c5',
-          'file',
+          absoluteFileName,
         ])
       })
     }
 
     test('strips .cmd extension case-insensitively', () => {
-      assert.deepEqual(getArgumentsForFile('code.CMD', 'file', 10, 5), ['-r', '-g', 'file:10:5'])
+      assert.deepEqual(getArgumentsForFile('code.CMD', absoluteFileName, 10, 5), [
+        '-r',
+        '-g',
+        `${absoluteFileName}:10:5`,
+      ])
     })
   })
+
+  describe('file names that look like editor options', () => {
+    const cases = [
+      ['vim', '+call writefile(["marker"], "LAUNCH_EDITOR_MARKER")|qa!'],
+      ['mvim', '-ccall writefile(["marker"], "LAUNCH_EDITOR_MARKER")|qa!'],
+      ['gvim', '+call writefile(["marker"], "LAUNCH_EDITOR_MARKER")|qa!'],
+      ['emacs', '--eval=(write-region "marker" nil "LAUNCH_EDITOR_MARKER")'],
+      ['emacsclient', '-eatouch LAUNCH_EDITOR_MARKER'],
+      ['notepad++', '-pluginMessage=LAUNCH_EDITOR_MARKER'],
+    ]
+
+    for (const [editor, fileName] of cases) {
+      for (const lineNumber of [undefined, 10]) {
+        const position = lineNumber ? 'with a position' : 'without a position'
+
+        test(`${editor} protects an option-like file name ${position}`, () => {
+          const args = getArgumentsForFile(editor, fileName, lineNumber, 5)
+          const fileArgument = args.at(-1)
+
+          assert.equal(path.resolve(fileArgument), path.resolve(fileName))
+          assert.ok(
+            path.isAbsolute(fileArgument),
+            `unsafe editor arguments: ${JSON.stringify(args)}`,
+          )
+        })
+      }
+    }
+  })
 })