From e335ab05a1016e0dc343e4b253934f1a33333f29 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 9 Dec 2025 23:35:29 +0100 Subject: [PATCH] fix(core/ink): prevent self-referential symlinks + add unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds a safety guard to create_ink() to prevent creation of self-referential symlinks when the resolved command already lives at the intended link target (e.g. ~/.local/bin/package-manager). Such a situation previously resulted in broken shells with the error: "zsh: too many levels of symbolic links" Key changes: - create_ink(): • Introduce early-abort guard when command == link_path • Improve function signature and formatting • Enhance alias creation messaging - Added comprehensive unit tests under: tests/unit/pkgmgr/core/command/test_ink.py Tests cover: • Self-referential command path → skip symlink creation • Standard symlink + alias creation behaviour This prevents pkgmgr from overwriting user-managed binaries inside ~/.local/bin and ensures predictable, safe behaviour across all installer layers. https://chatgpt.com/share/6938a43b-0eb8-800f-9545-6cb555ab406d --- pkgmgr/core/command/ink.py | 41 +++++++- tests/unit/pkgmgr/core/command/__init__.py | 0 tests/unit/pkgmgr/core/command/test_ink.py | 108 +++++++++++++++++++++ 3 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 tests/unit/pkgmgr/core/command/__init__.py create mode 100644 tests/unit/pkgmgr/core/command/test_ink.py diff --git a/pkgmgr/core/command/ink.py b/pkgmgr/core/command/ink.py index a72d5d7..91c8ddb 100644 --- a/pkgmgr/core/command/ink.py +++ b/pkgmgr/core/command/ink.py @@ -6,8 +6,14 @@ from pkgmgr.core.repository.identifier import get_repo_identifier from pkgmgr.core.repository.dir import get_repo_dir -def create_ink(repo, repositories_base_dir, bin_dir, all_repos, - quiet=False, preview=False): +def create_ink( + repo, + repositories_base_dir, + bin_dir, + all_repos, + quiet: bool = False, + preview: bool = False, +) -> None: """ Create a symlink for the repository's command. @@ -18,6 +24,11 @@ def create_ink(repo, repositories_base_dir, bin_dir, all_repos, Behavior: - If repo["command"] is defined → create a symlink to it. - If repo["command"] is missing or None → do NOT create a link. + + Safety: + - If the resolved command path is identical to the final link target, + we skip symlink creation to avoid self-referential symlinks that + would break shell resolution ("too many levels of symbolic links"). """ repo_identifier = get_repo_identifier(repo, all_repos) @@ -31,6 +42,27 @@ def create_ink(repo, repositories_base_dir, bin_dir, all_repos, link_path = os.path.join(bin_dir, repo_identifier) + # ------------------------------------------------------------------ + # Safety guard: avoid self-referential symlinks + # + # Example of a broken situation we must avoid: + # - command = ~/.local/bin/package-manager + # - link_path = ~/.local/bin/package-manager + # - create_ink() removes the real binary and creates a symlink + # pointing to itself → zsh: too many levels of symbolic links + # + # If the resolved command already lives exactly at the target path, + # we treat it as "already installed" and skip any modification. + # ------------------------------------------------------------------ + if os.path.abspath(command) == os.path.abspath(link_path): + if not quiet: + print( + f"[pkgmgr] Command for '{repo_identifier}' already lives at " + f"'{link_path}'. Skipping symlink creation to avoid a " + "self-referential link." + ) + return + if preview: print(f"[Preview] Would link {link_path} → {command}") return @@ -65,7 +97,10 @@ def create_ink(repo, repositories_base_dir, bin_dir, all_repos, if alias_name == repo_identifier: if not quiet: - print(f"Alias '{alias_name}' equals identifier. Skipping alias creation.") + print( + f"Alias '{alias_name}' equals identifier. " + "Skipping alias creation." + ) return try: diff --git a/tests/unit/pkgmgr/core/command/__init__.py b/tests/unit/pkgmgr/core/command/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit/pkgmgr/core/command/test_ink.py b/tests/unit/pkgmgr/core/command/test_ink.py new file mode 100644 index 0000000..13b7a71 --- /dev/null +++ b/tests/unit/pkgmgr/core/command/test_ink.py @@ -0,0 +1,108 @@ +import os +import tempfile +import unittest +from unittest.mock import patch + +from pkgmgr.core.command.ink import create_ink + + +class TestCreateInk(unittest.TestCase): + @patch("pkgmgr.core.command.ink.get_repo_dir") + @patch("pkgmgr.core.command.ink.get_repo_identifier") + def test_self_referential_command_skips_symlink( + self, + mock_get_repo_identifier, + mock_get_repo_dir, + ): + """ + If the resolved command path is identical to the final link target, + create_ink() must NOT replace it with a self-referential symlink. + + This simulates the situation where the command already lives at + ~/.local/bin/ and we would otherwise create a symlink + pointing to itself. + """ + mock_get_repo_identifier.return_value = "package-manager" + mock_get_repo_dir.return_value = "/fake/repo" + + with tempfile.TemporaryDirectory() as bin_dir: + # Simulate an existing real binary at the final link location. + command_path = os.path.join(bin_dir, "package-manager") + with open(command_path, "w", encoding="utf-8") as f: + f.write("#!/bin/sh\necho package-manager\n") + + # Sanity check: not a symlink yet. + self.assertTrue(os.path.exists(command_path)) + self.assertFalse(os.path.islink(command_path)) + + repo = {"command": command_path} + + # This must NOT turn the file into a self-referential symlink. + create_ink( + repo=repo, + repositories_base_dir="/fake/base", + bin_dir=bin_dir, + all_repos=[], + quiet=True, + preview=False, + ) + + # After create_ink(), the file must still exist and must not be a symlink. + self.assertTrue(os.path.exists(command_path)) + self.assertFalse( + os.path.islink(command_path), + "create_ink() must not create a self-referential symlink " + "when command == link_path", + ) + + @patch("pkgmgr.core.command.ink.get_repo_dir") + @patch("pkgmgr.core.command.ink.get_repo_identifier") + def test_create_symlink_for_normal_command( + self, + mock_get_repo_identifier, + mock_get_repo_dir, + ): + """ + In the normal case (command path != link target), create_ink() + must create a symlink in bin_dir pointing to the given command, + and optionally an alias symlink when repo['alias'] is set. + """ + mock_get_repo_identifier.return_value = "mytool" + + with tempfile.TemporaryDirectory() as repo_dir, tempfile.TemporaryDirectory() as bin_dir: + mock_get_repo_dir.return_value = repo_dir + + # Create a fake executable inside the repository. + command_path = os.path.join(repo_dir, "main.sh") + with open(command_path, "w", encoding="utf-8") as f: + f.write("#!/bin/sh\necho mytool\n") + os.chmod(command_path, 0o755) + + repo = { + "command": command_path, + "alias": "mt", + } + + create_ink( + repo=repo, + repositories_base_dir="/fake/base", + bin_dir=bin_dir, + all_repos=[], + quiet=True, + preview=False, + ) + + link_path = os.path.join(bin_dir, "mytool") + alias_path = os.path.join(bin_dir, "mt") + + # Main link must exist and point to the command. + self.assertTrue(os.path.islink(link_path)) + self.assertEqual(os.readlink(link_path), command_path) + + # Alias must exist and point to the main link. + self.assertTrue(os.path.islink(alias_path)) + self.assertEqual(os.readlink(alias_path), link_path) + + +if __name__ == "__main__": + unittest.main()