From c15763d9f8a887920ce9c369c245e0bc629dad40 Mon Sep 17 00:00:00 2001 From: Maxim Shulga Date: Sun, 2 Aug 2026 16:47:48 -0700 Subject: [PATCH] Stop server-provided values from reaching a shell or escaping /dsc/seafile Commands were joined into a string and passed to "su -c", so a login shell parsed every interpolated value. A library name is chosen on the server, and "sync -d /dsc/seafile/" put it straight into that string: a library named 'Docs; touch /tmp/pwned; #' ran as an arbitrary command. Confirmed against the built image before and after this change. Every command is now an argument list executed by "runuser --", which execs directly with no shell. runuser -u sets the same HOME and a PATH that still finds the seaf-cli wrapper, so the daemon environment is unchanged. The password no longer needs its surrounding quotes, which also fixes passwords containing quotes, dollar signs, and backticks. Library names are untrusted, so dsc/paths.py reduces each one to a single path component and proves the result stays under the libraries directory with symlinks resolved. Empty, ".", "..", and names containing a separator are skipped with a warning; names that would collide are suffixed with their library ID rather than sharing a directory. Tests cover quotes, command substitutions, separators, newlines, absolute paths, parent traversal, Unicode, and colliding names. They run in a separate CI job so application failures stay distinguishable from packaging failures. --- .github/workflows/docker-publish.yml | 21 +++- IMPROVEMENTS.md | 33 ++++--- README.md | 8 ++ dsc/client.py | 49 +++++----- dsc/misc.py | 15 +++ dsc/paths.py | 101 +++++++++++++++++++ pytest.ini | 4 + requirements-dev.txt | 3 + start.py | 11 ++- tests/conftest.py | 21 ++++ tests/test_commands.py | 139 +++++++++++++++++++++++++++ tests/test_paths.py | 111 +++++++++++++++++++++ 12 files changed, 478 insertions(+), 38 deletions(-) create mode 100644 dsc/paths.py create mode 100644 pytest.ini create mode 100644 requirements-dev.txt create mode 100644 tests/conftest.py create mode 100644 tests/test_commands.py create mode 100644 tests/test_paths.py diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 559d12e..241e2af 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -16,6 +16,25 @@ env: IMAGE: snegov/seafile-client jobs: + # Application failures must be distinguishable from packaging failures, so + # the unit tests do not depend on the image build. + unit-test: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: pip + + - name: Install dependencies + run: pip install -r requirements-dev.txt + + - name: Run unit tests + run: pytest -q + test: runs-on: ubuntu-latest @@ -34,7 +53,7 @@ jobs: tags: ${{ env.IMAGE }}:ci push: - needs: test + needs: [unit-test, test] runs-on: ubuntu-latest if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/') diff --git a/IMPROVEMENTS.md b/IMPROVEMENTS.md index 9db2392..fe9ab92 100644 --- a/IMPROVEMENTS.md +++ b/IMPROVEMENTS.md @@ -37,15 +37,19 @@ Acceptance criteria: ### Remove shell and path injection risks -- [ ] Pass subprocess arguments as arrays instead of joining values into shell - command strings in `dsc/client.py` and `dsc/misc.py`. -- [ ] Replace `su -c` with a mechanism such as `runuser --` that does not need a - shell. -- [ ] Treat server-provided library names as untrusted input. -- [ ] Resolve every target directory and prove that it remains under - `/dsc/seafile` before creating or writing it. -- [ ] Define how duplicate, absolute, empty, and traversal-style library names - are handled. +- [x] Pass subprocess arguments as arrays instead of joining values into shell + command strings in `dsc/client.py` and `dsc/misc.py`. The password is no + longer wrapped in quotes to survive a shell, which also fixes passwords + containing quotes, `$`, and backticks. +- [x] Replace `su -c` with a mechanism such as `runuser --` that does not need a + shell. `runuser -u` sets the same `HOME` and a `PATH` that still finds the + `seaf-cli` wrapper, so the daemon environment is unchanged. +- [x] Treat server-provided library names as untrusted input. +- [x] Resolve every target directory and prove that it remains under + `/dsc/seafile` before creating or writing it. Symlinks are resolved first, + so an existing symlink in the libraries directory cannot redirect a write. +- [x] Define how duplicate, absolute, empty, and traversal-style library names + are handled. See `dsc/paths.py` and the "Some notes" section of the README. Acceptance criteria: @@ -88,6 +92,8 @@ Acceptance criteria: - [ ] Check every important subprocess return code. - [ ] Add timeouts to daemon startup, shutdown, and readiness polling. + Confirmed: `start_daemon` and `stop_daemon` poll `while True` with no bound, + so a daemon that never changes state hangs the container silently. - [ ] Handle `SIGTERM` and `SIGINT` and stop the daemon in `finally`. - [ ] Return a nonzero container exit code on configuration or daemon failure. - [ ] Produce actionable errors instead of waiting forever. @@ -101,13 +107,16 @@ Acceptance criteria: ### Add real tests - [ ] Add unit tests for configuration parsing, command construction, library - resolution, path validation, retries, and status parsing. + resolution, path validation, retries, and status parsing. Command + construction and path validation are covered in `tests/`; configuration + parsing, retries, and status parsing are not. - [ ] Add regression tests for externally reported password and deletion threshold issues. - [ ] Add a container smoke test for imports, installed CLI version, startup, and shutdown using controlled fakes. -- [ ] Keep image construction as a separate CI job so packaging failures are - distinguishable from application failures. +- [x] Keep image construction as a separate CI job so packaging failures are + distinguishable from application failures. The `unit-test` job does not + depend on the image build. Acceptance criteria: diff --git a/README.md b/README.md index d092210..d74553b 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,14 @@ volumes: Libraries will be synced in subdirectories of `/dsc/seafile` directory inside container. You can mount it to host directory to access files. +The subdirectory is named after the library, with whitespace replaced by + underscores. A library is skipped with a warning when its name cannot be a + directory of its own: an empty name, `.`, `..`, or a name containing `/`. Two + libraries whose names would produce the same directory both get their library + ID appended, so they never share one. Everything is created under the + libraries directory (`/dsc/seafile`, or the deprecated `/data` when that + exists); a library name can never place it elsewhere. + `hostname` parameter is optional, but it's recommended to set it to some unique value, it will be shown in Seafile webUI as client name (`terminal-dsc` in given example). diff --git a/dsc/client.py b/dsc/client.py index f91117b..0ef5a66 100644 --- a/dsc/client.py +++ b/dsc/client.py @@ -12,7 +12,7 @@ from urllib3.util.retry import Retry import seafile from dsc import const -from dsc.misc import create_dir, hide_password +from dsc.misc import create_dir, hide_password, user_cmd _lg = logging.getLogger(__name__) @@ -56,8 +56,8 @@ class SeafileClient: def __str__(self): return f"SeafileClient({self.user}@{self.url})" - def __gen_cmd(self, cmd: str) -> list: - return ["su", "-", const.DEFAULT_USERNAME, "-c", cmd] + def __gen_cmd(self, argv: list) -> list: + return user_cmd(argv) @property def token(self): @@ -87,8 +87,8 @@ class SeafileClient: @property def daemon_ready(self) -> bool: - cmd = "seaf-cli status" - _lg.info("Checking seafile daemon status: %s", cmd) + cmd = ["seaf-cli", "status"] + _lg.info("Checking seafile daemon status: %s", " ".join(cmd)) proc = subprocess.run( self.__gen_cmd(cmd), stdout=subprocess.DEVNULL, @@ -99,13 +99,13 @@ class SeafileClient: def init_config(self): if self.config_initialized: return - cmd = "seaf-cli init -d %s" % self.app_dir - _lg.info("Initializing seafile config: %s", cmd) + cmd = ["seaf-cli", "init", "-d", self.app_dir] + _lg.info("Initializing seafile config: %s", " ".join(cmd)) subprocess.run(self.__gen_cmd(cmd)) def start_daemon(self): - cmd = "seaf-cli start" - _lg.info("Starting seafile daemon: %s", cmd) + cmd = ["seaf-cli", "start"] + _lg.info("Starting seafile daemon: %s", " ".join(cmd)) subprocess.run(self.__gen_cmd(cmd)) _lg.info("Waiting for seafile daemon to start") @@ -117,8 +117,8 @@ class SeafileClient: _lg.info("Seafile daemon is ready") def stop_daemon(self): - cmd = "seaf-cli stop" - _lg.info("Stopping seafile daemon: %s", cmd) + cmd = ["seaf-cli", "stop"] + _lg.info("Stopping seafile daemon: %s", " ".join(cmd)) subprocess.run(self.__gen_cmd(cmd)) _lg.info("Waiting for seafile daemon to stop") @@ -135,9 +135,12 @@ class SeafileClient: return lib_id return None - def sync_lib(self, lib_id: str, parent_dir: str = const.DEFAULT_LIBS_DIR): - lib_name = self.remote_libraries[lib_id] - lib_dir = os.path.join(parent_dir, lib_name.replace(" ", "_")) + def sync_lib(self, lib_id: str, lib_dir: str): + """ + Sync a library into an already resolved directory. The caller decides + the directory, see dsc.paths.plan_lib_dirs: the name comes from the + server and must not be turned into a path here. + """ create_dir(lib_dir) cmd = [ "seaf-cli", @@ -146,13 +149,13 @@ class SeafileClient: "-s", self.url, "-d", lib_dir, "-u", self.user, - "-p", '"' + self.password + '"', + "-p", self.password, ] _lg.info( - "Syncing library %s: %s", lib_name, + "Syncing library %s into %s: %s", lib_id, lib_dir, " ".join(hide_password(cmd, self.password)), ) - subprocess.run(self.__gen_cmd(" ".join(cmd))) + subprocess.run(self.__gen_cmd(cmd)) def __print_tx_task(self, tx_task) -> str: """ Print transfer task status """ @@ -234,8 +237,8 @@ class SeafileClient: prev_status[library] = cur_status[library] def get_local_libraries(self) -> set: - cmd = "seaf-cli list" - _lg.info("Listing local libraries: %s", cmd) + cmd = ["seaf-cli", "list"] + _lg.info("Listing local libraries: %s", " ".join(cmd)) out = subprocess.check_output(self.__gen_cmd(cmd)) out = out.decode().splitlines()[1:] # first line is a header @@ -256,8 +259,8 @@ class SeafileClient: continue # check current value - cmd = f"seaf-cli config -k {key}" - _lg.info("Checking seafile client option: %s", cmd) + cmd = ["seaf-cli", "config", "-k", key] + _lg.info("Checking seafile client option: %s", " ".join(cmd)) proc = subprocess.run(self.__gen_cmd(cmd), stdout=subprocess.PIPE) # stdout looks like "option = value" cur_value = proc.stdout.decode().strip() @@ -269,8 +272,8 @@ class SeafileClient: continue # set new value - cmd = f"seaf-cli config -k {key} -v {value}" - _lg.info("Setting seafile client option: %s", cmd) + cmd = ["seaf-cli", "config", "-k", key, "-v", str(value)] + _lg.info("Setting seafile client option: %s", " ".join(cmd)) subprocess.run(self.__gen_cmd(cmd)) need_restart = True diff --git a/dsc/misc.py b/dsc/misc.py index 8d2193e..edb1e9a 100644 --- a/dsc/misc.py +++ b/dsc/misc.py @@ -40,6 +40,21 @@ def create_dir(dir_path: str): raise RuntimeError(f"Data dir {dir_path} is not a directory") +def user_cmd(argv: list) -> list: + """ + Wrap a command so that it runs as the seafile user. + + ``runuser --`` execs the command directly. ``su -c`` would hand it to a + login shell instead, which turns every server-provided value in the + command into shell code. Like ``su``, ``runuser`` sets HOME, USER and a + default PATH for the target user, so seaf-cli still finds its config and + its own wrapper. + """ + if not isinstance(argv, list): + raise TypeError(f"Command must be a list of arguments, got {type(argv).__name__}") + return ["runuser", "-u", DEFAULT_USERNAME, "--"] + argv + + def hide_password(cmd: list, password: str) -> list: cmd = cmd.copy() for i, arg in enumerate(cmd): diff --git a/dsc/paths.py b/dsc/paths.py new file mode 100644 index 0000000..1fb3bdf --- /dev/null +++ b/dsc/paths.py @@ -0,0 +1,101 @@ +"""Turn server-provided library names into safe local directories. + +Library names come from the Seafile server and are untrusted: a name is chosen +by whoever can create a library, and it must never decide where the container +writes. Every name is reduced to a single path component and every resulting +path is proven to stay under the libraries directory. +""" + +import logging +import os +from collections import Counter + +_lg = logging.getLogger(__name__) + +# Whitespace is folded into underscores so that library names survive as +# readable directory names, as they always have. +_WHITESPACE_REPLACEMENT = "_" + +# Names that are not usable as a directory of their own. +_RESERVED_NAMES = {"", ".", "..", "..."} + +# Length of the library id used to break ties between duplicate names. +_ID_SUFFIX_LEN = 8 + + +class UnsafeLibraryName(ValueError): + """A library name cannot be mapped to a directory under the parent.""" + + +def lib_dirname(lib_name: str) -> str: + """ + Return the directory name for a library, or raise for a name that cannot + be one. Shell metacharacters are ordinary filename characters here: nothing + reaches a shell any more, so they are kept rather than mangled. + """ + if not isinstance(lib_name, str): + raise UnsafeLibraryName(f"Library name is not a string: {lib_name!r}") + + name = "".join( + _WHITESPACE_REPLACEMENT if ch.isspace() else ch + for ch in lib_name.strip() + ) + + if "\0" in name: + raise UnsafeLibraryName("Library name contains a null byte") + if os.sep in name or (os.altsep and os.altsep in name): + raise UnsafeLibraryName(f"Library name contains a path separator: {lib_name!r}") + if name in _RESERVED_NAMES: + raise UnsafeLibraryName(f"Library name is not usable as a directory: {lib_name!r}") + + return name + + +def resolve_lib_dir(parent_dir: str, dir_name: str) -> str: + """ + Resolve ``dir_name`` inside ``parent_dir`` and prove that the result stays + there. Symlinks are resolved first, so an existing symlink inside the + parent cannot be used to write outside it. + """ + parent = os.path.realpath(parent_dir) + resolved = os.path.realpath(os.path.join(parent, dir_name)) + + if resolved != parent and not resolved.startswith(parent + os.sep): + raise UnsafeLibraryName( + f"Library directory {resolved} is outside {parent}" + ) + if resolved == parent: + raise UnsafeLibraryName(f"Library directory resolves to {parent} itself") + + return resolved + + +def plan_lib_dirs(libraries: dict, parent_dir: str) -> dict: + """ + Map library id to its directory for every library that can safely have one. + + Libraries whose names collide after whitespace folding would otherwise + share a directory and corrupt each other, so each of them is suffixed with + the start of its own id. Unsafe names are skipped with a warning rather + than aborting the sync of the remaining libraries. + """ + names = dict() + for lib_id, lib_name in libraries.items(): + try: + names[lib_id] = lib_dirname(lib_name) + except UnsafeLibraryName as err: + _lg.warning("Skipping library %s: %s", lib_id, err) + + counts = Counter(names.values()) + duplicates = {name for name, count in counts.items() if count > 1} + + plan = dict() + for lib_id, name in names.items(): + if name in duplicates: + name = f"{name}-{lib_id[:_ID_SUFFIX_LEN]}" + try: + plan[lib_id] = resolve_lib_dir(parent_dir, name) + except UnsafeLibraryName as err: + _lg.warning("Skipping library %s: %s", lib_id, err) + + return plan diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..37d8462 --- /dev/null +++ b/pytest.ini @@ -0,0 +1,4 @@ +[pytest] +testpaths = tests +# The application is imported from the repository root, not installed. +pythonpath = . diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..5074ab7 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,3 @@ +-r requirements.txt + +pytest==9.1.1 diff --git a/start.py b/start.py index 9406c93..a71312c 100755 --- a/start.py +++ b/start.py @@ -7,6 +7,7 @@ import sys from dsc import SeafileClient, const from dsc.misc import setup_uid, create_dir +from dsc.paths import plan_lib_dirs _lg = logging.getLogger('dsc') @@ -76,8 +77,14 @@ def main(): else: libs_dir = const.DEFAULT_LIBS_DIR - for lib_id in libs_to_sync: - client.sync_lib(lib_id, libs_dir) + # Library names come from the server, so the directory for each one is + # resolved and checked before anything is created or written. + lib_dirs = plan_lib_dirs( + {lib_id: client.remote_libraries[lib_id] for lib_id in libs_to_sync}, + libs_dir, + ) + for lib_id, lib_dir in lib_dirs.items(): + client.sync_lib(lib_id, lib_dir) client.watch_status() client.stop_daemon() diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..7bded19 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,21 @@ +"""Make ``dsc.client`` importable outside the container. + +``seafile`` and ``pysearpc`` ship with the Seafile CLI AppImage and only exist +inside the image. The application tests must not need a live server or that +AppImage, so a stub module stands in for the RPC client. +""" + +import sys +import types +from unittest import mock + + +def _install_seafile_stub(): + if "seafile" in sys.modules: + return + seafile = types.ModuleType("seafile") + seafile.RpcClient = mock.MagicMock(name="RpcClient") + sys.modules["seafile"] = seafile + + +_install_seafile_stub() diff --git a/tests/test_commands.py b/tests/test_commands.py new file mode 100644 index 0000000..341682f --- /dev/null +++ b/tests/test_commands.py @@ -0,0 +1,139 @@ +"""Every subprocess must be an argument list, never a shell string.""" + +import argparse +import subprocess + +import pytest + +from dsc import client as client_module +from dsc import const +from dsc.client import SeafileClient +from dsc.misc import hide_password, user_cmd + +# Values that would be interpreted by a shell if a command were ever joined +# into a string and handed to ``su -c``. +SHELL_METACHARACTERS = [ + 'pa"ss', + "pa'ss", + "pa$(id)ss", + "pa`id`ss", + "pass; rm -rf /", + "pass && curl evil.example", + "pass | tee /tmp/x", + "pass\nrm -rf /", + "pass with spaces", + "pass\\backslash", + "паssшörд", +] + + +@pytest.fixture +def client(): + return SeafileClient( + host="seafile.example", + user="user@example", + passwd="s3cret", + app_dir="/dsc", + ) + + +@pytest.fixture +def calls(monkeypatch): + """Capture every subprocess invocation instead of running it.""" + recorded = [] + + def fake_run(argv, **kwargs): + recorded.append((argv, kwargs)) + return subprocess.CompletedProcess(argv, 0, stdout=b"") + + monkeypatch.setattr(subprocess, "run", fake_run) + # Directory creation is covered by the path tests; keep the real + # filesystem out of command construction. + monkeypatch.setattr(client_module, "create_dir", lambda path: None) + return recorded + + +def test_user_cmd_uses_runuser_without_a_shell(): + argv = user_cmd(["seaf-cli", "status"]) + assert argv == ["runuser", "-u", const.DEFAULT_USERNAME, "--", "seaf-cli", "status"] + + +def test_user_cmd_never_invokes_a_shell(): + argv = user_cmd(["seaf-cli", "sync", "-d", "a; rm -rf /"]) + assert "-c" not in argv + assert not any(arg.endswith("sh") for arg in argv) + # The dangerous value stays a single, unsplit argument. + assert argv[-1] == "a; rm -rf /" + + +def test_user_cmd_rejects_a_string(): + with pytest.raises(TypeError): + user_cmd("seaf-cli status") + + +@pytest.mark.parametrize("password", SHELL_METACHARACTERS) +def test_sync_passes_password_as_one_unquoted_argument(client, calls, password): + client.password = password + client.sync_lib("lib-id", "/dsc/seafile/Docs") + + argv, _ = calls[-1] + assert argv[argv.index("-p") + 1] == password + + +@pytest.mark.parametrize("lib_dir", [ + "/dsc/seafile/a\"; touch /tmp/pwned; #", + "/dsc/seafile/$(id)", + "/dsc/seafile/`id`", + "/dsc/seafile/name with spaces", +]) +def test_sync_passes_library_dir_as_one_argument(client, calls, lib_dir): + client.sync_lib("lib-id", lib_dir) + + argv, _ = calls[-1] + assert argv[argv.index("-d") + 1] == lib_dir + + +def test_sync_command_is_a_list_of_strings(client, calls): + client.sync_lib("lib-id", "/dsc/seafile/Docs") + + argv, kwargs = calls[-1] + assert isinstance(argv, list) + assert all(isinstance(arg, str) for arg in argv) + assert kwargs.get("shell") is not True + + +@pytest.mark.parametrize("password", SHELL_METACHARACTERS) +def test_sync_never_logs_the_password(client, calls, caplog, password): + client.password = password + with caplog.at_level("INFO"): + client.sync_lib("lib-id", "/dsc/seafile/Docs") + + assert password not in caplog.text + + +def test_hide_password_masks_every_occurrence(): + masked = hide_password(["-p", "secret", "-u", "secret-user"], "secret") + assert masked == ["-p", "********", "-u", "********-user"] + + +def test_hide_password_does_not_mutate_the_original(): + argv = ["-p", "secret"] + hide_password(argv, "secret") + assert argv == ["-p", "secret"] + + +def test_configure_passes_option_values_as_arguments(client, calls, monkeypatch): + # Changing an option restarts the daemon, and that restart polls without a + # timeout. Keep this test on command construction only. + monkeypatch.setattr(client, "stop_daemon", lambda: None) + monkeypatch.setattr(client, "start_daemon", lambda: None) + + args = argparse.Namespace(upload_limit=100, unrelated="ignored") + client.configure(args, check_for_daemon=False) + + argvs = [argv for argv, _ in calls] + prefix = ["runuser", "-u", const.DEFAULT_USERNAME, "--", "seaf-cli", "config"] + assert prefix + ["-k", "upload_limit"] in argvs + assert prefix + ["-k", "upload_limit", "-v", "100"] in argvs + # An option that seaf-cli does not know about is never forwarded. + assert not any("ignored" in argv for argv in argvs) diff --git a/tests/test_paths.py b/tests/test_paths.py new file mode 100644 index 0000000..03aadcc --- /dev/null +++ b/tests/test_paths.py @@ -0,0 +1,111 @@ +import os + +import pytest + +from dsc.paths import UnsafeLibraryName, lib_dirname, plan_lib_dirs, resolve_lib_dir + +PARENT = "/dsc/seafile" + + +@pytest.mark.parametrize("name, expected", [ + ("Documents", "Documents"), + ("My Library", "My_Library"), + (" padded ", "padded"), + ("Ünïcödé ☂", "Ünïcödé_☂"), + # The characters below used to reach a shell. They are now ordinary + # filename characters and must survive untouched. + ('quote"name', 'quote"name'), + ("single'name", "single'name"), + ("semi;colon", "semi;colon"), + ("dollar$(id)", "dollar$(id)"), + ("back`id`tick", "back`id`tick"), + ("pipe|amp&name", "pipe|amp&name"), + ("new\nline", "new_line"), + ("tab\tname", "tab_name"), + ("dash-lead", "dash-lead"), + ("-leading-dash", "-leading-dash"), +]) +def test_lib_dirname_accepts(name, expected): + assert lib_dirname(name) == expected + + +@pytest.mark.parametrize("name", [ + "", + " ", + "\n", + ".", + "..", + "...", # not special to the kernel, but never a library name we want + "/", + "/etc", + "/etc/passwd", + "../escape", + "../../root", + "nested/name", + "trailing/", + "nul\0byte", +]) +def test_lib_dirname_rejects(name): + with pytest.raises(UnsafeLibraryName): + lib_dirname(name) + + +def test_resolve_lib_dir_stays_under_parent(): + assert resolve_lib_dir(PARENT, "Documents") == "/dsc/seafile/Documents" + + +def test_resolve_lib_dir_rejects_escape_through_symlink(tmp_path): + parent = tmp_path / "seafile" + parent.mkdir() + outside = tmp_path / "outside" + outside.mkdir() + (parent / "escape").symlink_to(outside) + + with pytest.raises(UnsafeLibraryName): + resolve_lib_dir(str(parent), "escape") + + +def test_resolve_lib_dir_allows_real_dir_under_parent(tmp_path): + parent = tmp_path / "seafile" + parent.mkdir() + (parent / "Documents").mkdir() + + resolved = resolve_lib_dir(str(parent), "Documents") + assert resolved == os.path.realpath(str(parent / "Documents")) + + +def test_plan_lib_dirs_maps_each_library_to_its_own_dir(): + plan = plan_lib_dirs({"id-a": "Docs", "id-b": "My Photos"}, PARENT) + assert plan == { + "id-a": "/dsc/seafile/Docs", + "id-b": "/dsc/seafile/My_Photos", + } + + +def test_plan_lib_dirs_disambiguates_duplicate_names(): + libs = {"aaaaaaaabbbb": "Docs", "ccccccccdddd": "Docs"} + plan = plan_lib_dirs(libs, PARENT) + + assert plan == { + "aaaaaaaabbbb": "/dsc/seafile/Docs-aaaaaaaa", + "ccccccccdddd": "/dsc/seafile/Docs-cccccccc", + } + + +def test_plan_lib_dirs_duplicate_suffix_is_order_independent(): + libs = {"aaaaaaaabbbb": "Docs", "ccccccccdddd": "Docs"} + reversed_libs = dict(reversed(list(libs.items()))) + assert plan_lib_dirs(libs, PARENT) == plan_lib_dirs(reversed_libs, PARENT) + + +def test_plan_lib_dirs_duplicates_collide_after_space_folding(): + """'A B' and 'A_B' both fold to 'A_B' and must not share a directory.""" + libs = {"aaaaaaaabbbb": "A B", "ccccccccdddd": "A_B"} + plan = plan_lib_dirs(libs, PARENT) + assert len(set(plan.values())) == 2 + + +def test_plan_lib_dirs_skips_unsafe_names_and_keeps_the_rest(): + libs = {"good": "Docs", "bad": "../escape"} + plan = plan_lib_dirs(libs, PARENT) + assert plan == {"good": "/dsc/seafile/Docs"}