Merge pull request 'Reject path-traversal archive entries and make the install swap cross-filesystem safe' (#2) from benvin/extract-and-swap-fixes into main

Reviewed-on: #2

.luacheckrc

@@ -6,9 +6,9 @@ max_line_length = 140
 globals = { 'vim' }
 
 files['tests/'] = {
-  read_globals = { 'describe', 'it', 'assert_eq', 'assert_true', 'assert_nil', 'assert_match', 'tmpdir' },
+  read_globals = { 'describe', 'it', 'assert_eq', 'assert_true', 'assert_nil', 'assert_match', 'tmpdir', 'otherfs_dir' },
 }
 
 files['tests/minimal.lua'] = {
-  globals = { 'describe', 'it', 'assert_eq', 'assert_true', 'assert_nil', 'assert_match', 'tmpdir' },
+  globals = { 'describe', 'it', 'assert_eq', 'assert_true', 'assert_nil', 'assert_match', 'tmpdir', 'otherfs_dir' },
 }

.woodpecker/lint.yaml

@@ -9,7 +9,7 @@ steps:
       - luacheck lua plugin tests
     backend_options:
       kubernetes:
-        serviceAccountName: default
+        serviceAccountName: vimpack-ci
         resources:
           requests:
             memory: 512Mi
@@ -25,7 +25,7 @@ steps:
       - stylua --check lua plugin tests
     backend_options:
       kubernetes:
-        serviceAccountName: default
+        serviceAccountName: vimpack-ci
         resources:
           requests:
             memory: 512Mi

.woodpecker/pre-commit.yaml

@@ -10,7 +10,7 @@ steps:
       - pre-commit run --all-files --show-diff-on-failure
     backend_options:
       kubernetes:
-        serviceAccountName: default
+        serviceAccountName: vimpack-ci
         resources:
           requests:
             memory: 512Mi

.woodpecker/test.yaml

@@ -5,11 +5,11 @@ steps:
   - name: test
     image: alpine:3.21
     commands:
-      - apk add --no-cache neovim tar curl unzip
+      - apk add --no-cache neovim tar curl unzip zip
       - nvim --headless --clean -l tests/minimal.lua
     backend_options:
       kubernetes:
-        serviceAccountName: default
+        serviceAccountName: vimpack-ci
         resources:
           requests:
             memory: 512Mi

README.md

@@ -136,10 +136,20 @@ the UI stays responsive.
 1. Resolve `(name, version)` to a URL and headers.
 2. `curl` the archive into `stdpath('cache')/vimpack/archives/`.
 3. Verify `sha256` when the spec carries one; a mismatch aborts with expected and got.
-4. Extract into `stdpath('cache')/vimpack/stage/`, hoisting a single wrapper directory.
-5. Rename into `stdpath('data')/site/pack/vimpack/{start,opt}/<name>`, parking the old
-   tree aside first and restoring it if the rename fails.
-6. Record name, version, sha256 and bucket in `stdpath('data')/vimpack/manifest.json`.
+4. List every archive member (`tar -tvf`, `unzip -Z -l`) and reject the archive outright
+   if any entry is absolute, escapes the destination via `..` or `..\`, is a device or
+   fifo, or is a link pointing outside the destination. A zip keeps its symlink targets
+   in the member data rather than the listing, so each one is read with `unzip -p`, which
+   writes to stdout and never to disk. Nothing is written until the whole listing is
+   clean; whatever lands is walked again afterwards as a backstop.
+5. Extract into `<pack root>/.vimpack-stage/`, hoisting a single wrapper directory.
+   Staging lives under the pack root so the swap below is a same-filesystem rename;
+   `stdpath('cache')` and `stdpath('data')` are frequently separate mounts.
+6. Rename into `stdpath('data')/site/pack/vimpack/{start,opt}/<name>`, parking the old
+   tree aside first and restoring it if the rename fails. Should the two ever land on
+   different filesystems anyway, the staged tree is copied beside the target and that
+   copy is renamed into place, so the target is never half-written.
+7. Record name, version, sha256 and bucket in `stdpath('data')/vimpack/manifest.json`.
 
 A failure at any step leaves the previous install untouched.
 
@@ -148,6 +158,10 @@ A failure at any step leaves the previous install untouched.
 Lazy loading. `opt = true` places a plugin under `opt/` for `:packadd`; there is no
 event, filetype or command trigger machinery.
 
+Concurrent Neovim instances. The manifest is rewritten wholesale at the end of a run, so
+two `:VimpackInstall` runs overlapping across processes are last-writer-wins and one set
+of manifest updates is lost; the installed trees themselves stay consistent.
+
 ## Tests
 
 ```sh

doc/vimpack.txt

@@ -147,14 +147,20 @@ A literal `token` key is rejected at |vimpack-setup| time.
 7. LAYOUT                                                     *vimpack-layout*
 
   stdpath('cache')/vimpack/archives/    downloaded archives
-  stdpath('cache')/vimpack/stage/       extraction staging area
+  stdpath('data')/site/pack/vimpack/.vimpack-stage/   extraction staging area
   stdpath('data')/vimpack/manifest.json installed name/version/sha256
   stdpath('data')/site/pack/vimpack/start/<name>
   stdpath('data')/site/pack/vimpack/opt/<name>
 
 An install extracts to the staging area, hoists a single wrapper directory if
 the archive has one, then renames into place after parking any previous tree
-aside. If the rename fails the previous tree is restored.
+aside. If the rename fails the previous tree is restored. Staging sits under
+the pack root so that rename stays on one filesystem.
+
+Every member is listed and validated before extraction: absolute paths, `..`
+and `..\` escapes, devices, fifos and links pointing out of the destination
+are all refused. Zip symlink targets are read with `unzip -p` (stdout only,
+nothing written) because the zip listing does not carry them.
 
 Lazy loading is out of scope: `opt = true` only chooses the opt/ bucket.
 

lua/vimpack/config.lua

@@ -63,7 +63,19 @@ function M.normalize(opts)
     end
     spec.opt = spec.opt == true
     spec.version = spec.version or 'url'
-    spec.dir_name = spec.dir_name or require('vimpack.util').slug(spec.name)
+    local slug = require('vimpack.util').slug
+    if spec.dir_name ~= nil then
+      if type(spec.dir_name) ~= 'string' or spec.dir_name == '' then
+        return nil, string.format('plugin %q: dir_name must be a non-empty string', spec.name)
+      end
+      -- A dir_name is joined onto the pack root, so it must be a single
+      -- component: anything slug() would rewrite, plus "." and "..", is out.
+      if spec.dir_name == '.' or spec.dir_name == '..' or spec.dir_name ~= slug(spec.dir_name) then
+        return nil,
+          string.format('plugin %q: dir_name %q must not contain path separators or ".."', spec.name, spec.dir_name)
+      end
+    end
+    spec.dir_name = spec.dir_name or slug(spec.name)
     plugins[#plugins + 1] = spec
   end
   cfg.plugins = plugins

lua/vimpack/fs.lua

@@ -64,6 +64,83 @@ function M.strip_top_level(dir)
   return dir
 end
 
+--- Recursively copy `src` to `dst`, fsyncing every file as it lands.
+function M.copy_tree(src, dst)
+  local stat, serr = uv.fs_lstat(src)
+  if not stat then
+    return false, string.format('cannot stat %s: %s', src, serr)
+  end
+
+  if stat.type == 'link' then
+    local link, lerr = uv.fs_readlink(src)
+    if not link then
+      return false, string.format('cannot read symlink %s: %s', src, lerr)
+    end
+    local ok, err = uv.fs_symlink(link, dst)
+    return ok and true or false, ok and nil or string.format('cannot recreate symlink %s: %s', dst, err)
+  end
+
+  if stat.type == 'directory' then
+    local ok, err = uv.fs_mkdir(dst, 493)
+    if not ok and not M.exists(dst) then
+      return false, string.format('cannot create %s: %s', dst, err)
+    end
+    local handle, herr = uv.fs_scandir(src)
+    if not handle then
+      return false, string.format('cannot read %s: %s', src, herr)
+    end
+    while true do
+      local name = uv.fs_scandir_next(handle)
+      if not name then
+        return true
+      end
+      local cok, cerr = M.copy_tree(vim.fs.joinpath(src, name), vim.fs.joinpath(dst, name))
+      if not cok then
+        return false, cerr
+      end
+    end
+  end
+
+  local ok, err = uv.fs_copyfile(src, dst)
+  if not ok then
+    return false, string.format('cannot copy %s: %s', src, err)
+  end
+  local fd = uv.fs_open(dst, 'r', 420)
+  if fd then
+    uv.fs_fsync(fd)
+    uv.fs_close(fd)
+  end
+  return true
+end
+
+--- Rename `src` onto `dst`. Staging normally shares a filesystem with the pack
+--- root so this is a plain atomic rename; when it does not, copy beside the
+--- target first and rename that, so `dst` is never a partially written tree.
+local function move_into_place(src, dst)
+  local ok, err, code = uv.fs_rename(src, dst)
+  if ok then
+    return true
+  end
+  if code ~= 'EXDEV' and not tostring(err):match('^EXDEV') then
+    return false, err
+  end
+
+  local landing = dst .. '.vimpack-new'
+  M.rm_rf(landing)
+  local copied, cerr = M.copy_tree(src, landing)
+  if not copied then
+    M.rm_rf(landing)
+    return false, cerr
+  end
+  local renamed, rerr = uv.fs_rename(landing, dst)
+  if not renamed then
+    M.rm_rf(landing)
+    return false, rerr
+  end
+  M.rm_rf(src)
+  return true
+end
+
 --- Replace `target` with `staged` without ever leaving a half-written plugin
 --- behind: the old tree is parked aside first and restored on failure.
 function M.swap(staged, target)
@@ -79,7 +156,7 @@ function M.swap(staged, target)
     end
   end
 
-  local ok, err = uv.fs_rename(staged, target)
+  local ok, err = move_into_place(staged, target)
   if not ok then
     if had_previous then
       uv.fs_rename(backup, target)
@@ -91,41 +168,275 @@ function M.swap(staged, target)
   return true
 end
 
---- Extract an archive into `dest` asynchronously. Calls cb(ok, err).
+--- Collapse `.` and `..` in an archive member path. Returns nil when the entry
+--- is absolute or climbs above the directory it is extracted into.
+function M.safe_relative(path)
+  if type(path) ~= 'string' or path == '' then
+    return nil
+  end
+  -- Archives written on Windows separate with `\`, so treat it as a separator
+  -- here too: otherwise `\abs.txt` and `..\..\x` would look like plain names.
+  path = path:gsub('\\', '/')
+  if path:sub(1, 1) == '/' or path:match('^%a:/') then
+    return nil
+  end
+  local parts = {}
+  for part in path:gmatch('[^/]+') do
+    if part == '..' then
+      if #parts == 0 then
+        return nil
+      end
+      parts[#parts] = nil
+    elseif part ~= '.' then
+      parts[#parts + 1] = part
+    end
+  end
+  return table.concat(parts, '/')
+end
+
+--- True when `target`, followed from the directory holding `entry`, lands
+--- outside the extraction root.
+local function link_escapes(entry, target)
+  if type(target) ~= 'string' or target == '' then
+    return true
+  end
+  target = target:gsub('\\', '/')
+  if target:sub(1, 1) == '/' then
+    return true
+  end
+  local dir = entry:gsub('\\', '/'):match('^(.*)/[^/]*$')
+  return M.safe_relative(dir and (dir .. '/' .. target) or target) == nil
+end
+
+local SPECIAL = { c = 'a character device', b = 'a block device', p = 'a fifo', s = 'a socket' }
+
+--- Parse `tar -tvf` output into { name, kind, link, hardlink } records.
+local function parse_tar_listing(out)
+  local entries = {}
+  for line in (out or ''):gmatch('[^\n]+') do
+    local mode, rest = line:match('^(%S+)%s+%S+%s+%S+%s+%S+%s+%S+%s+(.+)$')
+    if not mode then
+      return nil, string.format('cannot parse archive listing line %q', line)
+    end
+    local entry = { kind = mode:sub(1, 1), name = rest }
+    if entry.kind == 'l' then
+      local name, target = rest:match('^(.*) %-> (.*)$')
+      entry.name, entry.link = name or rest, target
+    elseif entry.kind == 'h' then
+      local name, target = rest:match('^(.*) link to (.*)$')
+      entry.name, entry.hardlink = name or rest, target
+    end
+    entries[#entries + 1] = entry
+  end
+  return entries
+end
+
+-- perms, version, os, size, text/binary, compressed size, method, date, time, name
+local ZIP_ENTRY = '^(.)%S*%s+%d+%.%d+%s+%S+%s+%d+%s+%S+%s+%d+%s+%S+%s+%S+%s+%S+%s+(.+)$'
+local ZIP_NOISE = { '^Archive:%s', '^Zip file size:%s', '^%d+ files?,', '^Empty zipfile%.?$' }
+
+--- Parse `unzip -Z -l` output into { name, kind } records. Symlink targets live
+--- in the member data, not the listing; `resolve_zip_links` fills them in.
+local function parse_zip_listing(out)
+  local entries = {}
+  for line in (out or ''):gmatch('[^\n]+') do
+    local kind, name = line:match(ZIP_ENTRY)
+    if kind then
+      entries[#entries + 1] = { kind = kind, name = name }
+    else
+      local noise = false
+      for _, pattern in ipairs(ZIP_NOISE) do
+        noise = noise or line:match(pattern) ~= nil
+      end
+      if not noise then
+        return nil, string.format('cannot parse archive listing line %q', line)
+      end
+    end
+  end
+  return entries
+end
+
+--- Info-ZIP treats `*`, `?`, `[` and `\` in a member name as pattern syntax, so
+--- escape them before handing a name back to unzip.
+local function zip_filespec(name)
+  return (name:gsub('[%*%?%[\\]', '\\%0'))
+end
+
+--- Reject anything that would write outside the extraction root. Returns an
+--- error message naming the offending entry, or nil when every entry is safe.
+function M.check_entries(entries, archive)
+  for _, entry in ipairs(entries) do
+    local why
+    if SPECIAL[entry.kind] then
+      why = 'is ' .. SPECIAL[entry.kind]
+    elseif not M.safe_relative(entry.name) then
+      why = 'is absolute or escapes the destination directory'
+    elseif entry.link and link_escapes(entry.name, entry.link) then
+      why = string.format('is a symlink to %q outside the destination directory', entry.link)
+    -- Belt and braces: GNU tar already strips leading `/` and `..` from the
+    -- hard link targets it reports, so this only bites other tar flavours.
+    elseif entry.hardlink and not M.safe_relative(entry.hardlink) then
+      why = string.format('is a hard link to %q outside the destination directory', entry.hardlink)
+    end
+    if why then
+      return string.format('refusing to extract %s: entry %q %s', archive, entry.name, why)
+    end
+  end
+end
+
+--- Backstop for formats whose listing does not describe links: walk what landed
+--- on disk and report the first symlink pointing out of `dest`.
+local function escaping_symlink(dest)
+  for name, kind in vim.fs.dir(dest, { depth = 64 }) do
+    if kind == 'link' then
+      local target = uv.fs_readlink(vim.fs.joinpath(dest, name))
+      if link_escapes(name, target) then
+        return name, target or '?'
+      end
+    end
+  end
+end
+
+local function spawn(cmd, cb)
+  local ok, err = pcall(vim.system, cmd, { text = true }, function(res)
+    vim.schedule(function()
+      cb(res)
+    end)
+  end)
+  if not ok then
+    cb(nil, string.format('cannot run %s: %s (is it installed?)', cmd[1], err))
+  end
+end
+
+--- Read the stored target of every zip symlink with `unzip -p`, which writes the
+--- member to stdout and never to disk. Calls cb(ok, err) once each is filled in.
+local function resolve_zip_links(archive, entries, cb)
+  local links = vim.tbl_filter(function(entry)
+    return entry.kind == 'l'
+  end, entries)
+
+  local index = 0
+  local function step()
+    index = index + 1
+    local entry = links[index]
+    if not entry then
+      cb(true)
+      return
+    end
+    spawn({ 'unzip', '-p', archive, zip_filespec(entry.name) }, function(res, serr)
+      if not res then
+        cb(false, serr)
+        return
+      end
+      if res.code ~= 0 then
+        cb(
+          false,
+          string.format(
+            'refusing to extract %s: cannot read the symlink target of entry %q (unzip exited %d)',
+            archive,
+            entry.name,
+            res.code
+          )
+        )
+        return
+      end
+      entry.link = res.stdout or ''
+      step()
+    end)
+  end
+  step()
+end
+
+--- Run the unpack command, tearing `dest` down again on failure or when the
+--- backstop walk still finds a symlink pointing out of it.
+local function unpack_into(archive, dest, unpack, cb)
+  spawn(unpack, function(xres, xerr)
+    if not xres then
+      M.rm_rf(dest)
+      cb(false, xerr)
+      return
+    end
+    if xres.code ~= 0 then
+      M.rm_rf(dest)
+      local detail = string.format('(%s exited %d): %s', unpack[1], xres.code, vim.trim(xres.stderr or ''))
+      cb(false, string.format('extraction failed for %s %s', archive, detail))
+      return
+    end
+    local name, target = escaping_symlink(dest)
+    if name then
+      M.rm_rf(dest)
+      cb(
+        false,
+        string.format(
+          'refusing to extract %s: entry %q is a symlink to %q outside the destination directory',
+          archive,
+          name,
+          target
+        )
+      )
+      return
+    end
+    cb(true)
+  end)
+end
+
+--- Extract an archive into `dest` asynchronously. Calls cb(ok, err). Every
+--- member is enumerated and validated first, so a hostile archive is rejected
+--- before anything is written.
 function M.extract(archive, dest, cb)
-  M.mkdir_p(dest)
   local kind = util.archive_kind(archive)
-  local cmd
+  local list, unpack
   if kind == 'tar' then
-    cmd = { 'tar', '-xf', archive, '-C', dest }
+    list = { 'tar', '-tvf', archive }
+    unpack = { 'tar', '-xf', archive, '-C', dest }
   elseif kind == 'zip' then
-    cmd = { 'unzip', '-q', '-o', archive, '-d', dest }
+    list = { 'unzip', '-Z', '-l', archive }
+    unpack = { 'unzip', '-q', '-o', archive, '-d', dest }
   else
     cb(false, string.format('unsupported archive type for %s (expected .tar.*, .tgz or .zip)', archive))
     return
   end
 
-  local ok, handle = pcall(vim.system, cmd, { text = true }, function(res)
-    vim.schedule(function()
-      if res.code ~= 0 then
-        cb(
-          false,
-          string.format(
-            'extraction failed for %s (%s exited %d): %s',
-            archive,
-            cmd[1],
-            res.code,
-            vim.trim(res.stderr or '')
-          )
-        )
-      else
-        cb(true)
+  spawn(list, function(res, serr)
+    if not res then
+      cb(false, serr)
+      return
+    end
+    if res.code ~= 0 then
+      local detail =
+        string.format('cannot list entries (%s exited %d): %s', list[1], res.code, vim.trim(res.stderr or ''))
+      cb(false, string.format('extraction failed for %s: %s', archive, detail))
+      return
+    end
+
+    local parse = kind == 'tar' and parse_tar_listing or parse_zip_listing
+    local entries, perr = parse(res.stdout)
+    if not entries then
+      cb(false, string.format('extraction failed for %s: %s', archive, perr))
+      return
+    end
+    local function unpack_validated()
+      local unsafe = M.check_entries(entries, archive)
+      if unsafe then
+        cb(false, unsafe)
+        return
       end
-    end)
+      M.mkdir_p(dest)
+      unpack_into(archive, dest, unpack, cb)
+    end
+
+    if kind == 'zip' then
+      resolve_zip_links(archive, entries, function(ok, lerr)
+        if ok then
+          unpack_validated()
+        else
+          cb(false, lerr)
+        end
+      end)
+    else
+      unpack_validated()
+    end
   end)
-  if not ok then
-    cb(false, string.format('cannot run %s: %s (is it installed?)', cmd[1], handle))
-  end
 end
 
 --- Download a URL to `out` asynchronously with curl. Calls cb(ok, err).

lua/vimpack/installer.lua

@@ -41,10 +41,12 @@ function M.run_tasks(items, concurrency, worker, done)
   end
 end
 
-local function stage_path(spec)
+--- Stage inside the pack root so the final swap is a same-filesystem rename;
+--- the cache lives under stdpath('cache'), which is often a different mount.
+local function stage_path(spec, root)
   return vim.fs.joinpath(
-    fsmod.cache_dir(),
-    'stage',
+    root or fsmod.pack_root(),
+    '.vimpack-stage',
     string.format('%s.%d', util.slug(spec.name), vim.uv.hrtime() % 1000000)
   )
 end
@@ -64,7 +66,7 @@ function M.install_one(spec, ctx, cb)
     util.slug(spec.name) .. '-' .. spec.version .. '-' .. resolved.filename
   )
   local target = fsmod.target_dir(spec, ctx.pack_root)
-  local staged = stage_path(spec)
+  local staged = stage_path(spec, ctx.pack_root)
 
   local function fail(message)
     fsmod.rm_rf(staged)

lua/vimpack/registry.lua

@@ -129,9 +129,17 @@ end
 --- A spec-level `url` short-circuits registry resolution entirely.
 function M.resolve(spec, registry)
   if spec.url then
+    local headers = {}
+    if registry then
+      local herr
+      headers, herr = M.build_headers(registry)
+      if not headers then
+        return nil, string.format('plugin %q: %s', spec.name, herr)
+      end
+    end
     return {
       url = spec.url,
-      headers = registry and select(1, M.build_headers(registry)) or {},
+      headers = headers,
       filename = spec.filename or util.url_basename(spec.url),
       sha256 = spec.sha256,
     }

tests/fs_spec.lua

@@ -63,6 +63,93 @@ describe('swap', function()
     assert_match('cannot move new install into place', err)
     assert_eq({ 'old' }, vim.fn.readfile(root .. '/target/marker'))
   end)
+
+  -- Forced EXDEV: deterministic everywhere, no second mount required.
+  local function with_exdev_on(path, fn)
+    local uv = vim.uv or vim.loop
+    local real = uv.fs_rename
+    uv.fs_rename = function(from, to)
+      if from == path then
+        return nil, 'EXDEV: cross-device link not permitted: ' .. from .. ' -> ' .. to, 'EXDEV'
+      end
+      return real(from, to)
+    end
+    local ok, err = pcall(fn)
+    uv.fs_rename = real
+    if not ok then
+      error(err, 0)
+    end
+  end
+
+  it('falls back to a copy when staged and target are on different filesystems', function()
+    local root = tmpdir()
+    vim.fn.mkdir(root .. '/target', 'p')
+    vim.fn.writefile({ 'old' }, root .. '/target/marker')
+    vim.fn.mkdir(root .. '/staged/lua', 'p')
+    vim.fn.writefile({ 'new' }, root .. '/staged/lua/plug.lua')
+
+    with_exdev_on(root .. '/staged', function()
+      assert_true(fsmod.swap(root .. '/staged', root .. '/target'))
+    end)
+    assert_eq({ 'new' }, vim.fn.readfile(root .. '/target/lua/plug.lua'))
+    assert_true(not fsmod.exists(root .. '/target/marker'))
+    assert_true(not fsmod.exists(root .. '/target.vimpack-old'))
+    assert_true(not fsmod.exists(root .. '/target.vimpack-new'))
+    assert_true(not fsmod.exists(root .. '/staged'))
+  end)
+
+  it('restores the previous install when the cross-filesystem copy fails', function()
+    local root = tmpdir()
+    vim.fn.mkdir(root .. '/target', 'p')
+    vim.fn.writefile({ 'old' }, root .. '/target/marker')
+
+    local ok, err
+    with_exdev_on(root .. '/absent', function()
+      ok, err = fsmod.swap(root .. '/absent', root .. '/target')
+    end)
+    assert_true(not ok)
+    assert_match('cannot move new install into place', err)
+    assert_eq({ 'old' }, vim.fn.readfile(root .. '/target/marker'))
+    assert_true(not fsmod.exists(root .. '/target.vimpack-new'))
+  end)
+
+  it('copies a tree across a real filesystem boundary', function()
+    local root = tmpdir()
+    local other = otherfs_dir(root)
+    if not other then
+      return
+    end
+    vim.fn.mkdir(root .. '/staged/lua', 'p')
+    vim.fn.writefile({ 'new' }, root .. '/staged/lua/plug.lua')
+    vim.fn.mkdir(root .. '/staged/empty', 'p')
+
+    assert_true(fsmod.copy_tree(root .. '/staged', other .. '/copy'))
+    assert_eq({ 'new' }, vim.fn.readfile(other .. '/copy/lua/plug.lua'))
+    assert_true(fsmod.exists(other .. '/copy/empty'))
+    fsmod.rm_rf(other)
+  end)
+end)
+
+describe('safe_relative', function()
+  it('accepts a plain relative path', function()
+    assert_eq('lua/plug.lua', fsmod.safe_relative('./lua/plug.lua'))
+    assert_eq('lua', fsmod.safe_relative('lua/sub/..'))
+  end)
+
+  it('rejects absolute paths and escapes', function()
+    assert_nil(fsmod.safe_relative('/etc/passwd'))
+    assert_nil(fsmod.safe_relative('../evil'))
+    assert_nil(fsmod.safe_relative('a/../../evil'))
+    assert_nil(fsmod.safe_relative(''))
+  end)
+
+  it('treats a backslash as a separator', function()
+    assert_nil(fsmod.safe_relative('\\absolute_backslash.txt'))
+    assert_nil(fsmod.safe_relative('..\\..\\..\\deep_escape.txt'))
+    assert_nil(fsmod.safe_relative('a\\..\\..\\evil'))
+    assert_nil(fsmod.safe_relative('C:\\windows\\evil'))
+    assert_eq('lua/plug.lua', fsmod.safe_relative('lua\\plug.lua'))
+  end)
 end)
 
 describe('extract', function()
@@ -102,4 +189,168 @@ describe('extract', function()
     assert_true(not result.ok)
     assert_match('extraction failed', result.err)
   end)
+
+  it('unpacks a zip', function()
+    local root = tmpdir()
+    vim.fn.mkdir(root .. '/src/plug/lua', 'p')
+    vim.fn.writefile({ 'return {}' }, root .. '/src/plug/lua/plug.lua')
+    vim.fn.system({ 'sh', '-c', 'cd ' .. vim.fn.shellescape(root .. '/src') .. ' && zip -qr ../plug.zip plug' })
+    if vim.v.shell_error ~= 0 then
+      return
+    end
+
+    local result = wait_extract(root .. '/plug.zip', root .. '/dest')
+    assert_true(result.ok, tostring(result.err))
+    assert_true(fsmod.exists(root .. '/dest/plug/lua/plug.lua'))
+  end)
+
+  it('refuses a tarball that climbs out of the destination', function()
+    local root = tmpdir()
+    vim.fn.mkdir(root .. '/src/plug', 'p')
+    vim.fn.writefile({ 'pwned' }, root .. '/src/plug/payload')
+    vim.fn.system({
+      'tar',
+      '-czf',
+      root .. '/evil.tar.gz',
+      '-C',
+      root .. '/src',
+      '--transform',
+      's|^plug|../evil|',
+      'plug/payload',
+    })
+
+    local result = wait_extract(root .. '/evil.tar.gz', root .. '/dest')
+    assert_true(not result.ok)
+    assert_match('refusing to extract', result.err)
+    assert_match('../evil/payload', result.err)
+    assert_true(not fsmod.exists(root .. '/evil'))
+    assert_true(not fsmod.exists(root .. '/dest/payload'))
+  end)
+
+  it('refuses a tarball holding an absolute path', function()
+    local root = tmpdir()
+    local victim = root .. '/abs-evil'
+    vim.fn.writefile({ 'pwned' }, victim)
+    vim.fn.system({ 'tar', '-cPzf', root .. '/abs.tar.gz', victim })
+
+    local result = wait_extract(root .. '/abs.tar.gz', root .. '/dest')
+    assert_true(not result.ok)
+    assert_match('refusing to extract', result.err)
+    assert_match('is absolute or escapes the destination directory', result.err)
+    assert_true(not fsmod.exists(root .. '/dest'))
+  end)
+
+  it('refuses a tarball whose symlink points outside the destination', function()
+    local root = tmpdir()
+    vim.fn.mkdir(root .. '/src/plug', 'p')
+    vim.fn.system({ 'ln', '-s', '/etc/passwd', root .. '/src/plug/escape' })
+    vim.fn.system({ 'tar', '-czf', root .. '/link.tar.gz', '-C', root .. '/src', 'plug' })
+
+    local result = wait_extract(root .. '/link.tar.gz', root .. '/dest')
+    assert_true(not result.ok)
+    assert_match('refusing to extract', result.err)
+    assert_match('plug/escape', result.err)
+    assert_true(not fsmod.exists(root .. '/dest/plug'))
+  end)
+
+  it('refuses a tarball whose symlink climbs out with ..', function()
+    local root = tmpdir()
+    vim.fn.mkdir(root .. '/src/plug/lua', 'p')
+    vim.fn.system({ 'ln', '-s', '../../../outside', root .. '/src/plug/lua/escape' })
+    vim.fn.system({ 'tar', '-czf', root .. '/rel.tar.gz', '-C', root .. '/src', 'plug' })
+
+    local result = wait_extract(root .. '/rel.tar.gz', root .. '/dest')
+    assert_true(not result.ok)
+    assert_match('refusing to extract', result.err)
+    assert_match('plug/lua/escape', result.err)
+  end)
+
+  local function sh(root, script)
+    vim.fn.system({ 'sh', '-c', 'cd ' .. vim.fn.shellescape(root) .. ' && ' .. script })
+    return vim.v.shell_error == 0
+  end
+
+  -- A symlink out of the destination followed by a member written *through* it.
+  -- unzip would create the link and only then notice, so the whole archive has
+  -- to be refused off the listing, before a single byte lands.
+  it('refuses a zip that writes through an escaping symlink', function()
+    local root = tmpdir()
+    local made = sh(
+      root,
+      'mkdir -p src && cd src && ln -s ../../../outside link && zip -qy ../evil.zip link'
+        .. ' && rm link && mkdir link && echo pwned > link/pwned.txt && zip -qgy ../evil.zip link/pwned.txt'
+    )
+    if not made then
+      return
+    end
+
+    local result = wait_extract(root .. '/evil.zip', root .. '/dest')
+    assert_true(not result.ok)
+    assert_match('refusing to extract', result.err)
+    assert_match('is a symlink to', result.err)
+    assert_match('../../../outside', result.err)
+    assert_true(not fsmod.exists(root .. '/dest'))
+    assert_true(not fsmod.exists(root .. '/outside'))
+    assert_true(not fsmod.exists(vim.fs.dirname(root) .. '/outside'))
+  end)
+
+  it('refuses a zip whose symlink points at an absolute path', function()
+    local root = tmpdir()
+    if not sh(root, 'mkdir -p src/plug && ln -s /etc/passwd src/plug/escape && cd src && zip -qry ../abs.zip plug') then
+      return
+    end
+
+    local result = wait_extract(root .. '/abs.zip', root .. '/dest')
+    assert_true(not result.ok)
+    assert_match('refusing to extract', result.err)
+    assert_match('plug/escape', result.err)
+    assert_true(not fsmod.exists(root .. '/dest'))
+  end)
+
+  it('allows a zip symlink that stays inside the destination', function()
+    local root = tmpdir()
+    if
+      not sh(
+        root,
+        'mkdir -p src/plug/lua && echo "return {}" > src/plug/lua/plug.lua'
+          .. ' && ln -s lua/plug.lua src/plug/alias && cd src && zip -qry ../ok.zip plug'
+      )
+    then
+      return
+    end
+
+    local result = wait_extract(root .. '/ok.zip', root .. '/dest')
+    assert_true(result.ok, tostring(result.err))
+    assert_true(fsmod.exists(root .. '/dest/plug/alias'))
+  end)
+
+  it('reads the symlink target of a member whose name looks like a glob', function()
+    local root = tmpdir()
+    if
+      not sh(
+        root,
+        'mkdir -p src/plug && ln -s ../../../outside "src/plug/od[d]*link" && cd src && zip -qry ../glob.zip plug'
+      )
+    then
+      return
+    end
+
+    local result = wait_extract(root .. '/glob.zip', root .. '/dest')
+    assert_true(not result.ok)
+    assert_match('refusing to extract', result.err)
+    assert_match('is a symlink to', result.err)
+    assert_true(not fsmod.exists(root .. '/dest'))
+  end)
+
+  it('allows a symlink that stays inside the destination', function()
+    local root = tmpdir()
+    vim.fn.mkdir(root .. '/src/plug/lua', 'p')
+    vim.fn.writefile({ 'return {}' }, root .. '/src/plug/lua/plug.lua')
+    vim.fn.system({ 'ln', '-s', 'lua/plug.lua', root .. '/src/plug/alias' })
+    vim.fn.system({ 'tar', '-czf', root .. '/ok.tar.gz', '-C', root .. '/src', 'plug' })
+
+    local result = wait_extract(root .. '/ok.tar.gz', root .. '/dest')
+    assert_true(result.ok, tostring(result.err))
+    assert_true(fsmod.exists(root .. '/dest/plug/alias'))
+  end)
 end)

tests/install_spec.lua

@@ -126,6 +126,33 @@ describe('install end to end', function()
     assert_match(missing, results[1].err)
   end)
 
+  it('installs with the cache on a different filesystem from the pack root', function()
+    local root = tmpdir()
+    local pack_root = vim.fs.joinpath(root, 'pack')
+    local other = otherfs_dir(root)
+    if not other then
+      return
+    end
+
+    local real_cache_dir = fsmod.cache_dir
+    fsmod.cache_dir = function()
+      return vim.fs.joinpath(other, 'vimpack')
+    end
+
+    local archive = make_tarball(root, '1.0.0', 'return 1')
+    local cfg = assert(config.normalize({
+      plugins = { { name = 'demo.nvim', version = '1.0.0', url = 'file://' .. archive } },
+    }))
+    local data = manifest.empty()
+    local ok, results = pcall(install, cfg, data, pack_root)
+    fsmod.cache_dir = real_cache_dir
+    fsmod.rm_rf(other)
+
+    assert_true(ok, tostring(results))
+    assert_true(results[1].ok, tostring(results[1].err))
+    assert_eq({ 'return 1' }, vim.fn.readfile(vim.fs.joinpath(pack_root, 'start', 'demo.nvim', 'lua', 'demo.lua')))
+  end)
+
   it('moves a plugin from start to opt', function()
     local root = tmpdir()
     local pack_root = vim.fs.joinpath(root, 'pack')

tests/manifest_spec.lua

@@ -140,4 +140,23 @@ describe('config', function()
     assert_nil(cfg)
     assert_match('no registry', err)
   end)
+
+  it('keeps a well-formed dir_name override', function()
+    local cfg = assert(config.normalize({
+      registry = reg,
+      plugins = { { name = 'oil.nvim', version = '2.11.0', dir_name = 'oil' } },
+    }))
+    assert_eq('oil', cfg.plugins[1].dir_name)
+  end)
+
+  it('rejects a dir_name that is not a single path component', function()
+    for _, bad in ipairs({ '../evil', '/abs', 'a/b', '..', '.', '' }) do
+      local cfg, err = config.normalize({
+        registry = reg,
+        plugins = { { name = 'a', version = '1', dir_name = bad } },
+      })
+      assert_nil(cfg, 'dir_name ' .. vim.inspect(bad) .. ' should be rejected')
+      assert_match('dir_name', err)
+    end
+  end)
 end)

tests/minimal.lua

@@ -65,6 +65,26 @@ _G.tmpdir = function()
   return dir
 end
 
+-- A writable directory on a filesystem other than `reference`, so the EXDEV
+-- paths can be exercised for real. nil when the box has no second mount.
+_G.otherfs_dir = function(reference)
+  local uv = vim.uv or vim.loop
+  local ref = uv.fs_stat(reference)
+  if not ref then
+    return nil
+  end
+  for _, base in ipairs({ '/dev/shm', '/run/user/' .. tostring(uv.getuid and uv.getuid() or 0), '/var/tmp', '/tmp' }) do
+    local stat = uv.fs_stat(base)
+    if stat and stat.dev ~= ref.dev then
+      local dir = vim.fs.joinpath(base, 'vimpack-xdev-' .. tostring(uv.hrtime()))
+      if vim.fn.mkdir(dir, 'p') == 1 then
+        return dir
+      end
+    end
+  end
+  return nil
+end
+
 local files = {}
 for i = 1, #_G.arg do
   files[#files + 1] = _G.arg[i]

tests/registry_spec.lua

@@ -74,6 +74,28 @@ describe('url escape hatch', function()
     assert_eq('https://h/x.zip', resolved.url)
     assert_eq('x.zip', resolved.filename)
   end)
+
+  it('still applies the registry auth headers', function()
+    vim.env.VIMPACK_TEST_TOKEN = 'sekret'
+    local resolved = assert(
+      registry.resolve(
+        { name = 'x', version = '1', url = 'https://h/x.zip' },
+        { auth = { env = 'VIMPACK_TEST_TOKEN' } }
+      )
+    )
+    assert_eq('Bearer sekret', resolved.headers.Authorization)
+    vim.env.VIMPACK_TEST_TOKEN = nil
+  end)
+
+  it('propagates an auth failure instead of downloading unauthenticated', function()
+    vim.env.VIMPACK_TEST_TOKEN = nil
+    local resolved, err = registry.resolve(
+      { name = 'x', version = '1', url = 'https://h/x.zip' },
+      { auth = { env = 'VIMPACK_TEST_TOKEN' } }
+    )
+    assert_nil(resolved)
+    assert_match('VIMPACK_TEST_TOKEN', err)
+  end)
 end)
 
 describe('auth headers', function()