From 25f4f73f6dc9744757787c82351120cd1baee5f8 Mon Sep 17 00:00:00 2001
From: isaacs <i@izs.me>
Date: Fri, 12 Jul 2019 10:58:28 -0700
Subject: [PATCH] add a util for writing arbitrary files to cache

This prevents metrics timing and debug logs from becoming root-owned.
---
 lib/utils/cache-file.js    | 69 ++++++++++++++++++++++++++++++++++++++
 lib/utils/error-handler.js | 10 +++---
 lib/utils/metrics.js       |  7 ++--
 3 files changed, 77 insertions(+), 9 deletions(-)
 create mode 100644 lib/utils/cache-file.js

diff --git a/lib/utils/cache-file.js b/lib/utils/cache-file.js
new file mode 100644
index 0000000000000..77df7d4e09361
--- /dev/null
+++ b/lib/utils/cache-file.js
@@ -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)
diff --git a/lib/utils/error-handler.js b/lib/utils/error-handler.js
index ba9d9f8e252e5..7cb43be2900ec 100644
--- a/lib/utils/error-handler.js
+++ b/lib/utils/error-handler.js
@@ -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 () {
@@ -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
     }
@@ -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]
@@ -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
diff --git a/lib/utils/metrics.js b/lib/utils/metrics.js
index 0f99c841dbe26..1a4bb79a6e85d 100644
--- a/lib/utils/metrics.js
+++ b/lib/utils/metrics.js
@@ -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
 
@@ -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.
   }
 }
 
@@ -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)
   })
 }