chore: Do not send user secret in the referer header

PR-URL: #1663
Credit: @assapir
Close: #1663
Reviewed-by: @ruyadorno

Author: Assaf Sapir

.gitignore

@@ -26,3 +26,4 @@ npm-debug.log
 /node_modules/.cache
 .DS_Store
 **/.DS_Store
+.vscode/

lib/hook.js

@@ -10,6 +10,7 @@ const pudding = require('figgy-pudding')
 const relativeDate = require('tiny-relative-date')
 const Table = require('cli-table3')
 const validate = require('aproba')
+const npm = require('./npm')
 
 hook.usage = [
   'npm hook add <pkg> <url> <secret> [--type=<type>]',
@@ -40,6 +41,10 @@ module.exports = (args, cb) => BB.try(() => hook(args)).then(
   err => err.code === 'EUSAGE' ? cb(err.message) : cb(err)
 )
 function hook (args) {
+  if (args.length === 4) { // secret is passed in the args
+    // we have the user secret in the CLI args, we need to redact it from the referer.
+    redactUserSecret()
+  }
   return otplease(npmConfig(), opts => {
     opts = HookConfig(opts)
     switch (args[0]) {
@@ -150,3 +155,11 @@ function hookName (hook) {
   if (hook.type === 'owner') { target = '~' + target }
   return target
 }
+
+function redactUserSecret () {
+  const referer = npm.referer
+  if (!referer) return
+  const splittedReferer = referer.split(' ')
+  splittedReferer[4] = '[REDACTED]'
+  npm.referer = splittedReferer.join(' ')
+}

test/tap/referer.js

@@ -21,3 +21,63 @@ test('should send referer http header', function (t) {
     })
   })
 })
+
+test('should redact user secret from hook add command', function (t) {
+  http.createServer(function (q, s) {
+    t.equal(q.headers.referer, 'hook add ~zkat [REDACTED] [REDACTED]')
+    s.statusCode = 204
+    s.end()
+    this.close()
+  }).listen(common.port, function () {
+    var reg = `http://localhost:${common.port}`
+    var args = [ 'hook', 'add', '~zkat', 'https://example.com', 'sekrit', '--registry', reg ]
+    common.npm(args, {}, function (er, code) {
+      if (er) {
+        throw er
+      }
+      // should not have ended nicely, since we returned an error
+      t.ok(code)
+      t.end()
+    })
+  })
+})
+
+test('should redact user secret from hook up command', function (t) {
+  http.createServer(function (q, s) {
+    t.equal(q.headers.referer, 'hook up ~zkat [REDACTED] [REDACTED]')
+    s.statusCode = 204
+    s.end()
+    this.close()
+  }).listen(common.port, function () {
+    var reg = `http://localhost:${common.port}`
+    var args = [ 'hook', 'up', '~zkat', 'https://example.com', 'sekrit', '--registry', reg ]
+    common.npm(args, {}, function (er, code) {
+      if (er) {
+        throw er
+      }
+      // should not have ended nicely, since we returned an error
+      t.ok(code)
+      t.end()
+    })
+  })
+})
+
+test('should redact user secret from hook update command', function (t) {
+  http.createServer(function (q, s) {
+    t.equal(q.headers.referer, 'hook update ~zkat [REDACTED] [REDACTED]')
+    s.statusCode = 204
+    s.end()
+    this.close()
+  }).listen(common.port, function () {
+    var reg = `http://localhost:${common.port}`
+    var args = [ 'hook', 'update', '~zkat', 'https://example.com', 'sekrit', '--registry', reg ]
+    common.npm(args, {}, function (er, code) {
+      if (er) {
+        throw er
+      }
+      // should not have ended nicely, since we returned an error
+      t.ok(code)
+      t.end()
+    })
+  })
+})