Optionally obfuscate file paths

[sjberry]

@TomFrost Love Bristol, been using it for a long time.

I recently came across a need to obfuscate my file paths. I found that this was relevant for live demos both for the sake of security and clarity and that it would be helpful in certain production deployments to reduce the logging payload when I already know the executable path.

I'd like to see an option on the Bristol constructor to support relative pathing from process.cwd(). Using this setting the logged stack trace path would be limited to the folders in a given project directory assuming that it was the current working directory when the process was started.

let logger = new Bristol({
    relativeStackTracePaths: true // default: false (current behavior)
});

Would end up using:

path.relative(process.cwd(), filePath);

I wouldn't worry about handling the cases of starting a node process from a different directory.

I've modified a version of Bristol and can PR in the fix if you think it would be appropriate/useful. With respect to performance, this would add a single if statement in every log event unless you wanted to maintain two path retrieval functions and dynamically set them on the Bristol instance.

I have not tested how this affects globally linked modules, but based on my experience with the path module, worst case scenario you'd end up with a bunch of ../../.. relative path directives.

[TomFrost]

I really love this idea. I don't think an extra if is the end of the world, but since Bristol already has a mechanism for modifying output, I wonder if we couldn't plug into that instead. I'm open to any PRs -- I'll play with this as well!

[sjberry]

I'm assuming that the "mechanism for modifying output" that you're referencing is the support for formatters? I'm not so intricately familiar with the code base, so I don't want to get the wrong idea.

I think it would be certainly possible to modify the output when it's actually formatted. What I'd personally shy away from is having a formatter like human-relativePathed, since, in my opinion, relative pathing isn't something that's necessarily coupled to the idea of a "format." However, I can see the argument for both sides.

Putting that discussion aside temporarily, let's assume we'd be modifying the log origin globally in the code block:

https://github.com/TomFrost/Bristol/blob/8f08f08ff2826fe2519f96b33f49e23cd1155aac/src/Bristol.js#L237-L253

Modifying that section to look like:

_getOrigin() {
  Error.prepareStackTrace = arrayPrepareStackTrace
  const stack = (new Error()).stack
  let origin = null
  let cwd = this.relativeStackTracePaths ? process.cwd() : null;
  for (let i = 1; i < stack.length; i++) {
    const file = stack[i].getFileName()
    if (file !== __filename) {
      origin = {
        file: this.relativeStackTracePaths ? ('.' + path.sep + path.relative(cwd, file)) : file,
        line: stack[i].getLineNumber().toString()
      }
      break
    }
  }
  Error.prepareStackTrace = originalPrepareStackTrace
  return origin
}

I think would work. But we might be more inclined to apply a pathTransform function to bring the solution more in line with the existing .addTransform() interface. Something like:

log.addPathTransform(function(location) {
    return '.' + path.sep + path.relative(process.cwd(), location);
});

This discussion also begs the question of whether or not this whole idea is a hack fix to circumvent the apparent lack of a similar V8 option. We're also left with the problem of how (and indeed if) we should handle the full error stack trace.

For instance:

Error: oops
    at Object.<anonymous> (/path/to/file.js:1:69)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Module.require (module.js:367:17)
    at require (internal/module.js:16:19)
    at repl:1:7
    at REPLServer.defaultEval (repl.js:252:27)
    at bound (domain.js:287:14)

Is the result of just logging the stack value on the error. Do we need to account for relative paths in this error as well? Would a transform be the best scope for such a solution, perhaps one that would automatically be injected by virtue of the hypothetical global relative path option? For example:

class Bristol {
  constructor(options) {
    ...

    this.relativeStackTracePaths = options.relativeStackTracePaths === true

    if (this.relativeStackTracePaths) {
      this.addTransform(function(elem) {
        if (elem instanceof Error) {
          let cwd = process.cwd()
          let re = new RegExp('(' + cwd + '.+)(?=:[0-9]+:[0-9]+)', 'g')

          return elem.stack.replace(re, function(match) {
            return '.' + path.sep + path.relative(cwd, match)
          })
        }

        return null
      })
    }

    ...
  }
}

(Which works in my admittedly simple manual tests for the sake of discussion.)

[dandv]

I'd also like the ability to use relative paths, or omit paths altogether from the output. For example, it's not that relevant where a startup message like log.info("We're up and running") is in the code.

It's easy enough to add a transform to remove the path:

logger.addTransform(e => {
  if (e.file && e.line)
    e.file = e.file.match(/([^/\\]*?$)/)[1];
  return e;
});

logger.info('foo');  // [2018-07-12 16:00:31] INFO: up and running (test.mjs:28)

I can't figure out how to omit paths completely though (#53). I've tried passing {line: /./} to .excluding, but the file:line still went through.

[MarkHerhold]

I'm sure you're interested in this change being made to the default Bristol formatter, but you could also roll your own.

For instance, my formatter allows trimming paths. You could write your own to remove paths completely.

Just food for thought. Good luck.