feat: do not alter file ownership (#141)

BREAKING CHANGE: this module no longer attempts to change file ownership automatically

lib/content/read.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const fs = require('@npmcli/fs')
+const fs = require('fs/promises')
 const fsm = require('fs-minipass')
 const ssri = require('ssri')
 const contentPath = require('./path')

lib/content/rm.js

@@ -1,18 +1,16 @@
 'use strict'
 
-const util = require('util')
-
+const fs = require('fs/promises')
 const contentPath = require('./path')
 const { hasContent } = require('./read')
-const rimraf = util.promisify(require('rimraf'))
 
 module.exports = rm
 
 async function rm (cache, integrity) {
   const content = await hasContent(cache, integrity)
   // ~pretty~ sure we can't end up with a content lacking sri, but be safe
   if (content && content.sri) {
-    await rimraf(contentPath(cache, content.sri))
+    await fs.rm(contentPath(cache, content.sri), { recursive: true, force: true })
     return true
   } else {
     return false

lib/content/write.js

@@ -1,17 +1,14 @@
 'use strict'
 
 const events = require('events')
-const util = require('util')
 
 const contentPath = require('./path')
-const fixOwner = require('../util/fix-owner')
-const fs = require('@npmcli/fs')
+const fs = require('fs/promises')
 const moveFile = require('../util/move-file')
 const Minipass = require('minipass')
 const Pipeline = require('minipass-pipeline')
 const Flush = require('minipass-flush')
 const path = require('path')
-const rimraf = util.promisify(require('rimraf'))
 const ssri = require('ssri')
 const uniqueFilename = require('unique-filename')
 const fsm = require('fs-minipass')
@@ -40,7 +37,7 @@ async function write (cache, data, opts = {}) {
     return { integrity: sri, size: data.length }
   } finally {
     if (!tmp.moved) {
-      await rimraf(tmp.target)
+      await fs.rm(tmp.target, { recursive: true, force: true })
     }
   }
 }
@@ -111,7 +108,7 @@ async function handleContent (inputStream, cache, opts) {
     return res
   } finally {
     if (!tmp.moved) {
-      await rimraf(tmp.target)
+      await fs.rm(tmp.target, { recursive: true, force: true })
     }
   }
 }
@@ -152,7 +149,7 @@ async function pipeToTmp (inputStream, cache, tmpTarget, opts) {
 
 async function makeTmp (cache, opts) {
   const tmpTarget = uniqueFilename(path.join(cache, 'tmp'), opts.tmpPrefix)
-  await fixOwner.mkdirfix(cache, path.dirname(tmpTarget))
+  await fs.mkdir(path.dirname(tmpTarget), { recursive: true })
   return {
     target: tmpTarget,
     moved: false,
@@ -163,10 +160,9 @@ async function moveToDestination (tmp, cache, sri, opts) {
   const destination = contentPath(cache, sri)
   const destDir = path.dirname(destination)
 
-  await fixOwner.mkdirfix(cache, destDir)
+  await fs.mkdir(destDir, { recursive: true })
   await moveFile(tmp.target, destination)
   tmp.moved = true
-  await fixOwner.chownr(cache, destination)
 }
 
 function sizeError (expected, found) {

lib/entry-index.js

@@ -1,20 +1,23 @@
 'use strict'
 
-const util = require('util')
 const crypto = require('crypto')
-const fs = require('@npmcli/fs')
+const {
+  appendFile,
+  mkdir,
+  readFile,
+  readdir,
+  rm,
+  writeFile,
+} = require('fs/promises')
 const Minipass = require('minipass')
 const path = require('path')
 const ssri = require('ssri')
 const uniqueFilename = require('unique-filename')
 
 const contentPath = require('./content/path')
-const fixOwner = require('./util/fix-owner')
 const hashToSegments = require('./util/hash-to-segments')
 const indexV = require('../package.json')['cache-version'].index
 const moveFile = require('@npmcli/move-file')
-const _rimraf = require('rimraf')
-const rimraf = util.promisify(_rimraf)
 
 module.exports.NotFoundError = class NotFoundError extends Error {
   constructor (cache, key) {
@@ -65,7 +68,7 @@ async function compact (cache, key, matchFn, opts = {}) {
 
   const setup = async () => {
     const target = uniqueFilename(path.join(cache, 'tmp'), opts.tmpPrefix)
-    await fixOwner.mkdirfix(cache, path.dirname(target))
+    await mkdir(path.dirname(target), { recursive: true })
     return {
       target,
       moved: false,
@@ -74,24 +77,17 @@ async function compact (cache, key, matchFn, opts = {}) {
 
   const teardown = async (tmp) => {
     if (!tmp.moved) {
-      return rimraf(tmp.target)
+      return rm(tmp.target, { recursive: true, force: true })
     }
   }
 
   const write = async (tmp) => {
-    await fs.writeFile(tmp.target, newIndex, { flag: 'wx' })
-    await fixOwner.mkdirfix(cache, path.dirname(bucket))
+    await writeFile(tmp.target, newIndex, { flag: 'wx' })
+    await mkdir(path.dirname(bucket), { recursive: true })
     // we use @npmcli/move-file directly here because we
     // want to overwrite the existing file
     await moveFile(tmp.target, bucket)
     tmp.moved = true
-    try {
-      await fixOwner.chownr(cache, bucket)
-    } catch (err) {
-      if (err.code !== 'ENOENT') {
-        throw err
-      }
-    }
   }
 
   // write the file atomically
@@ -123,7 +119,7 @@ async function insert (cache, key, integrity, opts = {}) {
     metadata,
   }
   try {
-    await fixOwner.mkdirfix(cache, path.dirname(bucket))
+    await mkdir(path.dirname(bucket), { recursive: true })
     const stringified = JSON.stringify(entry)
     // NOTE - Cleverness ahoy!
     //
@@ -133,19 +129,13 @@ async function insert (cache, key, integrity, opts = {}) {
     //
     // Thanks to @isaacs for the whiteboarding session that ended up with
     // this.
-    await fs.appendFile(bucket, `\n${hashEntry(stringified)}\t${stringified}`)
-    await fixOwner.chownr(cache, bucket)
+    await appendFile(bucket, `\n${hashEntry(stringified)}\t${stringified}`)
   } catch (err) {
     if (err.code === 'ENOENT') {
       return undefined
     }
 
     throw err
-    // There's a class of race conditions that happen when things get deleted
-    // during fixOwner, or between the two mkdirfix/chownr calls.
-    //
-    // It's perfectly fine to just not bother in those cases and lie
-    // that the index entry was written. Because it's a cache.
   }
   return formatEntry(cache, entry)
 }
@@ -180,7 +170,7 @@ function del (cache, key, opts = {}) {
   }
 
   const bucket = bucketPath(cache, key)
-  return rimraf(bucket)
+  return rm(bucket, { recursive: true, force: true })
 }
 
 module.exports.lsStream = lsStream
@@ -246,7 +236,7 @@ async function ls (cache) {
 module.exports.bucketEntries = bucketEntries
 
 async function bucketEntries (bucket, filter) {
-  const data = await fs.readFile(bucket, 'utf8')
+  const data = await readFile(bucket, 'utf8')
   return _bucketEntries(data, filter)
 }
 
@@ -266,9 +256,8 @@ function _bucketEntries (data, filter) {
     let obj
     try {
       obj = JSON.parse(pieces[1])
-    } catch (e) {
-      // Entry is corrupted!
-      return
+    } catch (_) {
+      // eslint-ignore-next-line no-empty-block
     }
     // coverage disabled here, no need to test with an entry that parses to something falsey
     // istanbul ignore else
@@ -331,7 +320,7 @@ function formatEntry (cache, entry, keepAll) {
 }
 
 function readdirOrEmpty (dir) {
-  return fs.readdir(dir).catch((err) => {
+  return readdir(dir).catch((err) => {
     if (err.code === 'ENOENT' || err.code === 'ENOTDIR') {
       return []
     }

lib/rm.js

@@ -1,11 +1,10 @@
 'use strict'
 
-const util = require('util')
-
+const { rm } = require('fs/promises')
+const glob = require('./util/glob.js')
 const index = require('./entry-index')
 const memo = require('./memoization')
 const path = require('path')
-const rimraf = util.promisify(require('rimraf'))
 const rmContent = require('./content/rm')
 
 module.exports = entry
@@ -25,7 +24,8 @@ function content (cache, integrity) {
 
 module.exports.all = all
 
-function all (cache) {
+async function all (cache) {
   memo.clearMemoized()
-  return rimraf(path.join(cache, '*(content-*|index-*)'))
+  const paths = await glob(path.join(cache, '*(content-*|index-*)'), { silent: true, nosort: true })
+  return Promise.all(paths.map((p) => rm(p, { recursive: true, force: true })))
 }

lib/util/glob.js

@@ -0,0 +1,7 @@
+'use strict'
+
+const { promisify } = require('util')
+const glob = promisify(require('glob'))
+
+const globify = (pattern) => pattern.split('//').join('/')
+module.exports = (path, options) => glob(globify(path), options)

lib/util/move-file.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const fs = require('@npmcli/fs')
+const fs = require('fs/promises')
 const move = require('@npmcli/move-file')
 const pinflight = require('promise-inflight')
 

lib/util/tmp.js

@@ -1,8 +1,7 @@
 'use strict'
 
-const fs = require('@npmcli/fs')
-
-const fixOwner = require('./fix-owner')
+const { withTempDir } = require('@npmcli/fs')
+const fs = require('fs/promises')
 const path = require('path')
 
 module.exports.mkdir = mktmpdir
@@ -23,11 +22,5 @@ function withTmp (cache, opts, cb) {
     cb = opts
     opts = {}
   }
-  return fs.withTempDir(path.join(cache, 'tmp'), cb, opts)
-}
-
-module.exports.fix = fixtmpdir
-
-function fixtmpdir (cache) {
-  return fixOwner(cache, path.join(cache, 'tmp'))
+  return withTempDir(path.join(cache, 'tmp'), cb, opts)
 }