Skip to content

Commit

Permalink
Buildcache/ensure symlinks proper prefix (#43851)
Browse files Browse the repository at this point in the history
* archive: relative links only

Ensure all links written into tarfiles generated from Spack prefixes do not contain symlinks pointing outside the prefix

* binary_distribution: limit extraction to prefix

Ensure files extracted from spackballs are not links pointing outside of the prefix

* Ensure rpaths are properly set on Windows

* hard error on extraction of absolute links

* refactor for non link-modifying approach

* Restore tarball extraction to original impl

* use custom readlink

* cleanup symlink module

* make lstrip
  • Loading branch information
johnwparent committed May 10, 2024
1 parent 4270136 commit 095aba0
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 13 deletions.
7 changes: 7 additions & 0 deletions lib/spack/llnl/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,10 @@ def path_filter_caller(*args, **kwargs):
if _func:
return holder_func(_func)
return holder_func


def sanitize_win_longpath(path: str) -> str:
"""Strip Windows extended path prefix from strings
Returns sanitized string.
no-op if extended path prefix is not present"""
return path.lstrip("\\\\?\\")
5 changes: 3 additions & 2 deletions lib/spack/llnl/util/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -2450,9 +2450,10 @@ def add_library_dependent(self, *dest):
"""
for pth in dest:
if os.path.isfile(pth):
self._additional_library_dependents.add(pathlib.Path(pth).parent)
new_pth = pathlib.Path(pth).parent
else:
self._additional_library_dependents.add(pathlib.Path(pth))
new_pth = pathlib.Path(pth)
self._additional_library_dependents.add(new_pth)

@property
def rpaths(self):
Expand Down
12 changes: 6 additions & 6 deletions lib/spack/llnl/util/symlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from llnl.util import lang, tty

from ..path import system_path_filter
from ..path import sanitize_win_longpath, system_path_filter

if sys.platform == "win32":
from win32file import CreateHardLink
Expand Down Expand Up @@ -247,9 +247,9 @@ def _windows_create_junction(source: str, link: str):
out, err = proc.communicate()
tty.debug(out.decode())
if proc.returncode != 0:
err = err.decode()
tty.error(err)
raise SymlinkError("Make junction command returned a non-zero return code.", err)
err_str = err.decode()
tty.error(err_str)
raise SymlinkError("Make junction command returned a non-zero return code.", err_str)


def _windows_create_hard_link(path: str, link: str):
Expand All @@ -269,14 +269,14 @@ def _windows_create_hard_link(path: str, link: str):
CreateHardLink(link, path)


def readlink(path: str):
def readlink(path: str, *, dir_fd=None):
"""Spack utility to override of os.readlink method to work cross platform"""
if _windows_is_hardlink(path):
return _windows_read_hard_link(path)
elif _windows_is_junction(path):
return _windows_read_junction(path)
else:
return os.readlink(path)
return sanitize_win_longpath(os.readlink(path, dir_fd=dir_fd))


def _windows_read_hard_link(link: str) -> str:
Expand Down
4 changes: 3 additions & 1 deletion lib/spack/spack/binary_distribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import llnl.util.lang
import llnl.util.tty as tty
from llnl.util.filesystem import BaseDirectoryVisitor, mkdirp, visit_directory_tree
from llnl.util.symlink import readlink

import spack.caches
import spack.cmd
Expand Down Expand Up @@ -658,7 +659,7 @@ def get_buildfile_manifest(spec):
# 2. paths are used as strings.
for rel_path in visitor.symlinks:
abs_path = os.path.join(root, rel_path)
link = os.readlink(abs_path)
link = readlink(abs_path)
if os.path.isabs(link) and link.startswith(spack.store.STORE.layout.root):
data["link_to_relocate"].append(rel_path)

Expand Down Expand Up @@ -2001,6 +2002,7 @@ def install_root_node(spec, unsigned=False, force=False, sha256=None):
with spack.util.path.filter_padding():
tty.msg('Installing "{0}" from a buildcache'.format(spec.format()))
extract_tarball(spec, download_result, force)
spec.package.windows_establish_runtime_linkage()
spack.hooks.post_install(spec, False)
spack.store.STORE.db.add(spec, spack.store.STORE.layout)

Expand Down
1 change: 1 addition & 0 deletions lib/spack/spack/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ def _process_binary_cache_tarball(

with timer.measure("install"), spack.util.path.filter_padding():
binary_distribution.extract_tarball(pkg.spec, download_result, force=False, timer=timer)
pkg.windows_establish_runtime_linkage()

if hasattr(pkg, "_post_buildcache_install_hook"):
pkg._post_buildcache_install_hook()
Expand Down
5 changes: 3 additions & 2 deletions lib/spack/spack/relocate.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import llnl.util.lang
import llnl.util.tty as tty
from llnl.util.lang import memoized
from llnl.util.symlink import symlink
from llnl.util.symlink import readlink, symlink

import spack.paths
import spack.platforms
Expand All @@ -25,6 +25,7 @@
import spack.store
import spack.util.elf as elf
import spack.util.executable as executable
import spack.util.path

from .relocate_text import BinaryFilePrefixReplacer, TextFilePrefixReplacer

Expand Down Expand Up @@ -613,7 +614,7 @@ def relocate_links(links, prefix_to_prefix):
"""Relocate links to a new install prefix."""
regex = re.compile("|".join(re.escape(p) for p in prefix_to_prefix.keys()))
for link in links:
old_target = os.readlink(link)
old_target = readlink(link)
match = regex.match(old_target)

# No match.
Expand Down
8 changes: 6 additions & 2 deletions lib/spack/spack/util/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
from gzip import GzipFile
from typing import Callable, Dict, Tuple

from llnl.util.symlink import readlink


class ChecksumWriter(io.BufferedIOBase):
"""Checksum writer computes a checksum while writing to a file."""
Expand Down Expand Up @@ -193,12 +195,14 @@ def reproducible_tarfile_from_prefix(
file_info = tarfile.TarInfo(path_to_name(entry.path))

if entry.is_symlink():
file_info.type = tarfile.SYMTYPE
file_info.linkname = os.readlink(entry.path)
# strip off long path reg prefix on Windows
link_dest = readlink(entry.path)
file_info.linkname = link_dest
# According to POSIX: "the value of the file mode bits returned in the
# st_mode field of the stat structure is unspecified." So we set it to
# something sensible without lstat'ing the link.
file_info.mode = 0o755
file_info.type = tarfile.SYMTYPE
tar.addfile(file_info)

elif entry.is_file(follow_symlinks=False):
Expand Down

0 comments on commit 095aba0

Please sign in to comment.