Skip to content

Commit

Permalink
add a util for writing arbitrary files to cache
Browse files Browse the repository at this point in the history
This prevents metrics timing and debug logs from becoming root-owned.
  • Loading branch information
isaacs committed Jul 16, 2019
1 parent 554b641 commit 25f4f73
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 9 deletions.
69 changes: 69 additions & 0 deletions lib/utils/cache-file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
const npm = require('../npm.js')
const path = require('path')
const chownr = require('chownr')
const writeFileAtomic = require('write-file-atomic')
const mkdirp = require('mkdirp')
const fs = require('graceful-fs')

let cache = null
let cacheUid = null
let cacheGid = null
let needChown = typeof process.getuid === 'function'

const getCacheOwner = () => {
let st
try {
st = fs.lstatSync(cache)
} catch (er) {
if (er.code !== 'ENOENT') {
throw er
}
st = fs.lstatSync(path.dirname(cache))
}

cacheUid = st.uid
cacheGid = st.gid

needChown = st.uid !== process.getuid() ||
st.gid !== process.getgid()
}

const writeOrAppend = (method, file, data) => {
if (!cache) {
cache = npm.config.get('cache')
}

// redundant if already absolute, but prevents non-absolute files
// from being written as if they're part of the cache.
file = path.resolve(cache, file)

if (cacheUid === null && needChown) {
getCacheOwner()
}

const dir = path.dirname(file)
const firstMade = mkdirp.sync(dir)

if (!needChown) {
return method(file, data)
}

let methodThrew = true
try {
method(file, data)
methodThrew = false
} finally {
// always try to leave it in the right ownership state, even on failure
// let the method error fail it instead of the chownr error, though
if (!methodThrew) {
chownr.sync(firstMade || file, cacheUid, cacheGid)
} else {
try {
chownr.sync(firstMade || file, cacheUid, cacheGid)
} catch (_) {}
}
}
}

exports.append = (file, data) => writeOrAppend(fs.appendFileSync, file, data)
exports.write = (file, data) => writeOrAppend(writeFileAtomic.sync, file, data)
10 changes: 4 additions & 6 deletions lib/utils/error-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ var wroteLogFile = false
var exitCode = 0
var rollbacks = npm.rollbacks
var chain = require('slide').chain
var writeFileAtomic = require('write-file-atomic')
var errorMessage = require('./error-message.js')
var stopMetrics = require('./metrics.js').stop
var mkdirp = require('mkdirp')
var fs = require('graceful-fs')

const cacheFile = require('./cache-file.js')

var logFileName
function getLogFile () {
Expand All @@ -40,7 +39,7 @@ process.on('exit', function (code) {
if (npm.config.loaded && npm.config.get('timing')) {
try {
timings.logfile = getLogFile()
fs.appendFileSync(path.join(npm.config.get('cache'), '_timing.json'), JSON.stringify(timings) + '\n')
cacheFile.append('_timing.json', JSON.stringify(timings) + '\n')
} catch (_) {
// ignore
}
Expand Down Expand Up @@ -228,7 +227,6 @@ function writeLogFile () {
var os = require('os')

try {
mkdirp.sync(path.resolve(npm.config.get('cache'), '_logs'))
var logOutput = ''
log.record.forEach(function (m) {
var pref = [m.id, m.level]
Expand All @@ -241,7 +239,7 @@ function writeLogFile () {
logOutput += line + os.EOL
})
})
writeFileAtomic.sync(getLogFile(), logOutput)
cacheFile.write(getLogFile(), logOutput)

// truncate once it's been written.
log.record.length = 0
Expand Down
7 changes: 4 additions & 3 deletions lib/utils/metrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const path = require('path')
const npm = require('../npm.js')
const regFetch = require('libnpm/fetch')
const uuid = require('uuid')
const cacheFile = require('./cache-file.js')

let inMetrics = false

Expand Down Expand Up @@ -51,9 +52,9 @@ function saveMetrics (itWorked) {
}
}
try {
fs.writeFileSync(metricsFile, JSON.stringify(metrics))
cacheFile.write(metricsFile, JSON.stringify(metrics))
} catch (ex) {
// we couldn't write the error metrics file, um, well, oh well.
// we couldn't write and/or chown the error metrics file, oh well.
}
}

Expand All @@ -72,6 +73,6 @@ function sendMetrics (metricsFile, metricsRegistry) {
).then(() => {
fs.unlinkSync(metricsFile)
}, err => {
fs.writeFileSync(path.join(path.dirname(metricsFile), 'last-send-metrics-error.txt'), err.stack)
cacheFile.write(path.join(path.dirname(metricsFile), 'last-send-metrics-error.txt'), err.stack)
})
}

0 comments on commit 25f4f73

Please sign in to comment.