From f5416fea9b65f36f43e2f86c57508d50dc99bb9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Fri, 2 Oct 2026 13:47:45 +0200 Subject: [PATCH 1/3] cascache.py: Ensure that `checkout()` doesn't follow directory symlinks `bst artifact checkout` verifies that the destination is empty. However, `bst artifact checkout --force` allows checkout to a non-empty directory. Let's not blindly follow pre-existing directory symlinks even in that case. --- src/buildstream/_cas/cascache.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/buildstream/_cas/cascache.py b/src/buildstream/_cas/cascache.py index 0db18dd4a..5fecff654 100644 --- a/src/buildstream/_cas/cascache.py +++ b/src/buildstream/_cas/cascache.py @@ -215,6 +215,11 @@ def checkout(self, dest, tree, *, can_link=False, _fetch=True): for dirnode in directory.directories: fullpath = os.path.join(dest, dirnode.name) + # If the destination is a regular file, `os.makedirs()` will catch it. + # However, it will not raise an error if it's a valid directory symlink, + # so check here that we're not unexpectedly following a symlink. + if os.path.islink(fullpath): + raise CASCacheError(f"Destination already exists and is a symlink, not a directory: {fullpath}") self.checkout(fullpath, dirnode.digest, can_link=can_link, _fetch=False) for symlinknode in directory.symlinks: From e169c1974113a21fb3dc5772dd41c16c52fe3a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Fri, 2 Oct 2026 14:25:16 +0200 Subject: [PATCH 2/3] cascache.py: Replace existing files in `checkout` for symlinks `bst artifact checkout --force` intentionally replaces existing files. The same should apply to symlinks. Without `--force`, it should not be possible to hit that code path. --- src/buildstream/_cas/cascache.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/buildstream/_cas/cascache.py b/src/buildstream/_cas/cascache.py index 5fecff654..b0187f296 100644 --- a/src/buildstream/_cas/cascache.py +++ b/src/buildstream/_cas/cascache.py @@ -225,7 +225,14 @@ def checkout(self, dest, tree, *, can_link=False, _fetch=True): for symlinknode in directory.symlinks: # symlink fullpath = os.path.join(dest, symlinknode.name) - os.symlink(symlinknode.target, fullpath) + try: + os.symlink(symlinknode.target, fullpath) + except FileExistsError: + try: + os.unlink(fullpath) + except OSError as e: + raise CASCacheError(f"Failed to remove destination file '{fullpath}': {e}") from e + os.symlink(symlinknode.target, fullpath) node_properties = directory.node_properties if node_properties.HasField("mtime"): From 93714a0d4e904fe3d1f2c06edfbe33192e4362ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Fri, 16 Oct 2026 15:40:38 +0200 Subject: [PATCH 3/3] tests/internals/cascache.py: Add `checkout()` tests --- tests/internals/cascache.py | 88 +++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/internals/cascache.py b/tests/internals/cascache.py index c455809f1..f65097f5d 100644 --- a/tests/internals/cascache.py +++ b/tests/internals/cascache.py @@ -15,8 +15,12 @@ import time from unittest.mock import MagicMock +import pytest + from buildstream._cas import casdprocessmanager +from buildstream._exceptions import CASCacheError from buildstream._messenger import Messenger +from buildstream._protos.build.bazel.remote.execution.v2 import remote_execution_pb2 from tests.testutils import casd_cache # @@ -128,3 +132,87 @@ def test_casd_redirects_stderr_to_file_and_rotate(tmp_path, monkeypatch): assert len(existing_log_files) == n_max_log_files assert evicted_file not in existing_log_files assert existing_log_files[-1].read_text() == "hello\n" + + +def add_directory(cas, *, files=None, directories=None, symlinks=None): + directory = remote_execution_pb2.Directory() + for name, digest in (files or {}).items(): + filenode = directory.files.add() + filenode.name = name + filenode.digest.CopyFrom(digest) + for name, digest in (directories or {}).items(): + dirnode = directory.directories.add() + dirnode.name = name + dirnode.digest.CopyFrom(digest) + for name, target in (symlinks or {}).items(): + symlinknode = directory.symlinks.add() + symlinknode.name = name + symlinknode.target = target + return cas.add_object(buffer=directory.SerializeToString()) + + +# Test that `checkout()` doesn't follow a symlink in the destination +# directory when checking out a subdirectory. +# +@pytest.mark.parametrize("can_link", [False, True], ids=["copy", "link"]) +def test_checkout_existing_directory_symlink(tmp_path, can_link): + dest = tmp_path.joinpath("dest") + outside = tmp_path.joinpath("outside") + dest.mkdir() + outside.mkdir() + dest.joinpath("subdir").symlink_to(outside) + + with casd_cache(tmp_path.joinpath("cas")) as cas: + subdir = add_directory(cas, files={"file": cas.add_object(buffer=b"content")}, symlinks={"link": "target"}) + tree = add_directory(cas, directories={"subdir": subdir}) + + with pytest.raises(CASCacheError, match="symlink"): + cas.checkout(str(dest), tree, can_link=can_link) + + assert os.listdir(outside) == [] + assert dest.joinpath("subdir").is_symlink() + + +# Test that `checkout()` replaces an existing symlink in the destination +# directory with a regular file instead of writing to the symlink target. +# +@pytest.mark.parametrize("can_link", [False, True], ids=["copy", "link"]) +def test_checkout_replace_symlink_with_file(tmp_path, can_link): + dest = tmp_path.joinpath("dest") + outside = tmp_path.joinpath("outside") + dest.mkdir() + outside.write_bytes(b"original") + dest.joinpath("file").symlink_to(outside) + + with casd_cache(tmp_path.joinpath("cas")) as cas: + tree = add_directory(cas, files={"file": cas.add_object(buffer=b"content")}) + cas.checkout(str(dest), tree, can_link=can_link) + + assert outside.read_bytes() == b"original" + assert not dest.joinpath("file").is_symlink() + assert dest.joinpath("file").read_bytes() == b"content" + + +# Test that `checkout()` replaces existing files in the destination +# directory with symlinks. +# +@pytest.mark.parametrize("existing", ["file", "symlink", "dangling-symlink"]) +def test_checkout_replace_with_symlink(tmp_path, existing): + dest = tmp_path.joinpath("dest") + outside = tmp_path.joinpath("outside") + dest.mkdir() + outside.write_bytes(b"original") + + if existing == "file": + dest.joinpath("link").write_bytes(b"existing") + elif existing == "symlink": + dest.joinpath("link").symlink_to(outside) + else: + dest.joinpath("link").symlink_to(tmp_path.joinpath("does-not-exist")) + + with casd_cache(tmp_path.joinpath("cas")) as cas: + tree = add_directory(cas, symlinks={"link": "new-target"}) + cas.checkout(str(dest), tree) + + assert os.readlink(dest.joinpath("link")) == "new-target" + assert outside.read_bytes() == b"original"