fix(core/ink): prevent self-referential symlinks + add unit tests
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
This commit is contained in:
@@ -6,8 +6,14 @@ from pkgmgr.core.repository.identifier import get_repo_identifier
|
|||||||
from pkgmgr.core.repository.dir import get_repo_dir
|
from pkgmgr.core.repository.dir import get_repo_dir
|
||||||
|
|
||||||
|
|
||||||
def create_ink(repo, repositories_base_dir, bin_dir, all_repos,
|
def create_ink(
|
||||||
quiet=False, preview=False):
|
repo,
|
||||||
|
repositories_base_dir,
|
||||||
|
bin_dir,
|
||||||
|
all_repos,
|
||||||
|
quiet: bool = False,
|
||||||
|
preview: bool = False,
|
||||||
|
) -> None:
|
||||||
"""
|
"""
|
||||||
Create a symlink for the repository's command.
|
Create a symlink for the repository's command.
|
||||||
|
|
||||||
@@ -18,6 +24,11 @@ def create_ink(repo, repositories_base_dir, bin_dir, all_repos,
|
|||||||
Behavior:
|
Behavior:
|
||||||
- If repo["command"] is defined → create a symlink to it.
|
- If repo["command"] is defined → create a symlink to it.
|
||||||
- If repo["command"] is missing or None → do NOT create a link.
|
- 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)
|
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)
|
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:
|
if preview:
|
||||||
print(f"[Preview] Would link {link_path} → {command}")
|
print(f"[Preview] Would link {link_path} → {command}")
|
||||||
return
|
return
|
||||||
@@ -65,7 +97,10 @@ def create_ink(repo, repositories_base_dir, bin_dir, all_repos,
|
|||||||
|
|
||||||
if alias_name == repo_identifier:
|
if alias_name == repo_identifier:
|
||||||
if not quiet:
|
if not quiet:
|
||||||
print(f"Alias '{alias_name}' equals identifier. Skipping alias creation.")
|
print(
|
||||||
|
f"Alias '{alias_name}' equals identifier. "
|
||||||
|
"Skipping alias creation."
|
||||||
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
|||||||
0
tests/unit/pkgmgr/core/command/__init__.py
Normal file
0
tests/unit/pkgmgr/core/command/__init__.py
Normal file
108
tests/unit/pkgmgr/core/command/test_ink.py
Normal file
108
tests/unit/pkgmgr/core/command/test_ink.py
Normal file
@@ -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/<identifier> 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()
|
||||||
Reference in New Issue
Block a user