Merge pull request #98 from marp-team/warn-local-file-access

Output warning if detected blocking local resources while rendering by Chrome

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Added
+
+- Output warning if detected blocking local resources while rendering by Chrome ([#84](https://github.com/marp-team/marp-cli/issues/84), [#98](https://github.com/marp-team/marp-cli/pull/98))
+
 ## v0.9.2 - 2019-05-10
 
 ### Added

jest.setup.js

@@ -1,3 +1,5 @@
 const { enterTestMode } = require('carlo')
 
 enterTestMode()
+
+jest.mock('wrap-ansi')

package.json

@@ -101,7 +101,6 @@
     "rollup-plugin-url": "^2.2.1",
     "sass": "^1.20.1",
     "screenfull": "^4.2.0",
-    "strip-ansi": "^5.2.0",
     "stylelint": "^10.0.1",
     "stylelint-config-prettier": "^5.1.0",
     "stylelint-config-standard": "^18.3.0",
@@ -137,8 +136,10 @@
     "puppeteer-core": "^1.15.0",
     "rimraf": "^2.6.3",
     "serve-index": "^1.9.1",
+    "strip-ansi": "^5.2.0",
     "terminate": "^2.1.2",
     "tmp": "^0.1.0",
+    "wrap-ansi": "^5.1.0",
     "ws": "^7.0.0",
     "yargs": "^13.2.2"
   },

src/cli.ts

@@ -1,20 +1,42 @@
 import chalk from 'chalk'
+import stripAnsi from 'strip-ansi'
+import wrapAnsi from 'wrap-ansi'
+import { terminalWidth } from 'yargs'
+
+interface CLIOption {
+  singleLine?: boolean
+}
+
+const width: number = terminalWidth() || 80
+
+const messageBlock = (
+  kind: string,
+  message: string,
+  opts: CLIOption
+): string => {
+  const indent = stripAnsi(kind).length + 1
+  const target = opts.singleLine ? message : wrapAnsi(message, width - indent)
+
+  return `${kind} ${target.split('\n').join(`\n${' '.repeat(indent)}`)}`
+}
 
 let silent = false
 
 export function silence(value: boolean) {
   silent = value
 }
 
-export function info(message: string): void {
+export function info(message: string, opts: CLIOption = {}): void {
   // Use console.warn to output into stderr
-  if (!silent) console.warn(`${chalk.bgCyan.black('[  INFO ]')} ${message}`)
+  if (!silent)
+    console.warn(messageBlock(chalk.bgCyan.black('[  INFO ]'), message, opts))
 }
 
-export function warn(message: string): void {
-  if (!silent) console.warn(`${chalk.bgYellow.black('[  WARN ]')} ${message}`)
+export function warn(message: string, opts: CLIOption = {}): void {
+  if (!silent)
+    console.warn(messageBlock(chalk.bgYellow.black('[  WARN ]'), message, opts))
 }
 
-export function error(message: string): void {
-  console.error(`${chalk.bgRed.white('[ ERROR ]')} ${message}`)
+export function error(message: string, opts: CLIOption = {}): void {
+  console.error(messageBlock(chalk.bgRed.white('[ ERROR ]'), message, opts))
 }

src/converter.ts

@@ -1,7 +1,9 @@
-import { Marp, MarpOptions } from '@marp-team/marp-core'
+import { MarpOptions } from '@marp-team/marp-core'
 import { Marpit, MarpitOptions } from '@marp-team/marpit'
+import chalk from 'chalk'
 import * as chromeFinder from 'chrome-launcher/dist/chrome-finder'
 import puppeteer from 'puppeteer-core'
+import { URL } from 'url'
 import { silence, warn } from './cli'
 import { Engine } from './engine'
 import metaPlugin from './engine/meta-plugin'
@@ -89,7 +91,9 @@ export class Converter {
       lang,
       readyScript,
       base:
-        isFile && type !== ConvertType.html ? file!.absolutePath : undefined,
+        isFile && type !== ConvertType.html
+          ? file!.absoluteFileScheme
+          : undefined,
       notifyWS:
         isFile && this.options.watch && type === ConvertType.html
           ? await notifier.register(file!.absolutePath)
@@ -225,9 +229,9 @@ export class Converter {
     const tmpFile: File.TmpFileInterface | undefined = await (() => {
       if (!this.options.allowLocalFiles) return undefined
 
-      const warning = `Insecure local file accessing is enabled for conversion of ${file.relativePath()}.`
-      warn(warning)
-
+      warn(
+        `Insecure local file accessing is enabled for conversion of ${file.relativePath()}.`
+      )
       return file.saveTmpFile('.html')
     })()
 
@@ -238,17 +242,42 @@ export class Converter {
     try {
       const browser = await Converter.runBrowser()
       const page = await browser.newPage()
+      const tracker = this.trackFailedLocalFileAccess(page)
 
       try {
         return await process(page, uri)
       } finally {
+        if (tracker.size > 0) {
+          warn(
+            `Marp CLI has detected accessing to local file${
+              tracker.size > 1 ? 's' : ''
+            }. ${
+              tracker.size > 1 ? 'They are' : 'That is'
+            } blocked by security reason. Instead we recommend using assets uploaded to online. (Or you can use ${chalk.yellow(
+              '--allow-local-files'
+            )} option if you are understood of security risk)`
+          )
+        }
         await page.close()
       }
     } finally {
       if (tmpFile) await tmpFile.cleanup()
     }
   }
 
+  private trackFailedLocalFileAccess(page: puppeteer.Page): Set<string> {
+    const failedFileSet = new Set<string>()
+
+    page.on('requestfailed', (req: puppeteer.Request) => {
+      try {
+        const url = new URL(req.url())
+        if (url.protocol === 'file:') failedFileSet.add(url.href)
+      } catch (e) {}
+    })
+
+    return failedFileSet
+  }
+
   static async closeBrowser() {
     if (Converter.browser) await Converter.browser.close()
   }

src/file.ts

@@ -3,6 +3,7 @@ import getStdin from 'get-stdin'
 import globby from 'globby'
 import mkdirp from 'mkdirp'
 import path from 'path'
+import * as url from 'url'
 import { tmpName } from 'tmp'
 import { promisify } from 'util'
 
@@ -30,10 +31,18 @@ export class File {
     this.path = filepath
   }
 
-  get absolutePath() {
+  get absolutePath(): string {
     return path.resolve(this.path)
   }
 
+  get absoluteFileScheme(): string {
+    if (url.pathToFileURL)
+      return url.pathToFileURL(this.absolutePath).toString()
+
+    // Fallback for Node < v10.12.0
+    return `file://${path.posix.resolve(this.path)}`
+  }
+
   convert(output: string | false | undefined, extension: string): File {
     switch (output) {
       case undefined:

src/marp-cli.ts

@@ -237,7 +237,8 @@ export default async function(argv: string[] = []): Promise<number> {
       cli.info(
         `${fn(i, '<stdin>')} ${
           o.type === FileType.Null ? 'processed.' : `=> ${fn(o, '<stdout>')}`
-        }`
+        }`,
+        { singleLine: true }
       )
     }
 

test/__mocks__/wrap-ansi.ts

@@ -0,0 +1 @@
+module.exports = jest.fn().mockImplementation(m => m) // No ops

test/converter.ts

@@ -3,6 +3,7 @@ import { MarpitOptions } from '@marp-team/marpit'
 import fs from 'fs'
 import os from 'os'
 import path from 'path'
+import { URL } from 'url'
 import { Converter, ConvertType, ConverterOption } from '../src/converter'
 import { CLIError } from '../src/error'
 import { File, FileType } from '../src/file'
@@ -182,9 +183,14 @@ describe('Converter', () => {
       context(`with ${type} convert type`, () => {
         it('adds <base> element with specified base path from passed file', async () => {
           const converter = instance({ type })
-
           const { result } = await converter.convert(md, dummyFile)
-          expect(result).toContain(`<base href="${process.cwd()}">`)
+
+          const matched = result.match(/<base href="(.+?)">/)
+          expect(matched).toBeTruthy()
+
+          const url = new URL((matched && matched[1]) as string)
+          expect(url.protocol).toBe('file:')
+          expect(path.resolve(url.pathname)).toBe(path.resolve(process.cwd()))
         })
       })
     }

test/marp-cli.ts

@@ -241,7 +241,7 @@ describe('Marp CLI', () => {
         serverStart.mockResolvedValue(0)
 
         await marpCli(['--input-dir', files, '--server'])
-        expect(info).toBeCalledWith(
+        expect(info.mock.calls.map(([m]) => m)).toContainEqual(
           expect.stringContaining('http://localhost:8080/')
         )
         expect(serverStart).toBeCalledTimes(1)
@@ -276,7 +276,7 @@ describe('Marp CLI', () => {
 
             await run()
             expect(Preview.prototype.open).not.toBeCalled()
-            expect(warn).toBeCalledWith(
+            expect(warn.mock.calls.map(([m]) => m)).toContainEqual(
               expect.stringContaining('Preview option was ignored')
             )
           })
@@ -356,7 +356,7 @@ describe('Marp CLI', () => {
         const args = [assetFn('_files/1.md'), '--theme', themesPath]
 
         expect(await marpCli(args)).toBe(1)
-        expect(cliError).toHaveBeenCalledWith(
+        expect(cliError.mock.calls.map(([m]) => m)).toContainEqual(
           expect.stringContaining('Directory cannot pass to theme option')
         )
         expect(info).toHaveBeenCalledTimes(1)
@@ -470,12 +470,10 @@ describe('Marp CLI', () => {
       ;(<any>fs).__mockWriteFile()
 
       expect(await marpCli([onePath])).toBe(0)
-      expect(cliInfo).toHaveBeenCalledWith(
-        expect.stringContaining('1 markdown')
-      )
-      expect(cliInfo).toHaveBeenCalledWith(
-        expect.stringMatching(/1\.md => .+1\.html/)
-      )
+
+      const logs = cliInfo.mock.calls.map(([m]) => m)
+      expect(logs).toContainEqual(expect.stringContaining('1 markdown'))
+      expect(logs).toContainEqual(expect.stringMatching(/1\.md => .+1\.html/))
     })
 
     it('prints error and return error code when CLIError is raised', async () => {
@@ -487,7 +485,9 @@ describe('Marp CLI', () => {
         .mockImplementation(() => Promise.reject(new CLIError('FAIL', 123)))
 
       expect(await marpCli([onePath])).toBe(123)
-      expect(cliError).toHaveBeenCalledWith(expect.stringContaining('FAIL'))
+      expect(cliError.mock.calls.map(([m]) => m)).toContainEqual(
+        expect.stringContaining('FAIL')
+      )
     })
 
     context('with --pdf option', () => {
@@ -698,7 +698,7 @@ describe('Marp CLI', () => {
         .mockImplementation(() => [])
 
       expect(await marpCli([assetFn('_files')])).toBe(0)
-      expect(cliInfo).toHaveBeenCalledWith(
+      expect(cliInfo.mock.calls.map(([m]) => m)).toContainEqual(
         expect.stringContaining('4 markdowns')
       )
     })
@@ -775,7 +775,7 @@ describe('Marp CLI', () => {
 
     it('converts markdown came from stdin and outputs to stdout', async () => {
       expect(await marpCli()).toBe(0)
-      expect(cliInfo).toHaveBeenCalledWith(
+      expect(cliInfo.mock.calls.map(([m]) => m)).toContainEqual(
         expect.stringContaining('<stdin> => <stdout>')
       )
       expect(stdout).toHaveBeenCalledWith(expect.any(Buffer))
@@ -786,7 +786,7 @@ describe('Marp CLI', () => {
         jest.spyOn(console, 'error').mockImplementation()
 
         expect(await marpCli(['--stdin=false'])).toBe(0)
-        expect(cliInfo).not.toHaveBeenCalledWith(
+        expect(cliInfo.mock.calls.map(([m]) => m)).not.toContainEqual(
           expect.stringContaining('<stdin> => <stdout>')
         )
         expect(stdout).not.toHaveBeenCalledWith(expect.any(Buffer))

yarn.lock

@@ -7923,6 +7923,15 @@ wrap-ansi@^2.0.0:
     string-width "^1.0.1"
     strip-ansi "^3.0.1"
 
+wrap-ansi@^5.1.0:
+  version "5.1.0"
+  resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-5.1.0.tgz#1fd1f67235d5b6d0fee781056001bfb694c03b09"
+  integrity sha512-QC1/iN/2/RPVJ5jYK8BGttj5z83LmSKmvbvrXPNCLZSEb32KKVDJDl/MOt2N01qU2H/FkzEa9PKto1BqDjtd7Q==
+  dependencies:
+    ansi-styles "^3.2.0"
+    string-width "^3.0.0"
+    strip-ansi "^5.0.0"
+
 wrappy@1:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f"