Skip to content

Commit

Permalink
chore: Do not send user secret in the referer header
Browse files Browse the repository at this point in the history
PR-URL: #1663
Credit: @assapir
Close: #1663
Reviewed-by: @ruyadorno
  • Loading branch information
Assaf Sapir authored and ruyadorno committed Aug 17, 2020
1 parent 765cfe0 commit 4e28de7
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 0 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ npm-debug.log
/node_modules/.cache
.DS_Store
**/.DS_Store
.vscode/
13 changes: 13 additions & 0 deletions lib/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>]',
Expand Down Expand Up @@ -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]) {
Expand Down Expand Up @@ -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(' ')
}
60 changes: 60 additions & 0 deletions test/tap/referer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
})

0 comments on commit 4e28de7

Please sign in to comment.