Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/apm_cli/install/resolution_staging.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
from apm_cli.utils.path_security import ensure_path_within, safe_rmtree
from apm_cli.utils.staging_guard import STAGING_DIR_NAME

_STAGING_NAME = re.compile(r"[0-9a-f]{32}")
# Matches both the current 12-hex-char staging root name and the 32-hex-char
# (full uuid4().hex) name used before the MAX_PATH fix, so upgrading APM does
# not strand orphaned staging roots created by an older version (issue #2896).
_STAGING_NAME = re.compile(r"[0-9a-f]{12}|[0-9a-f]{32}")


class ResolutionStagingSession:
Expand All @@ -25,7 +28,9 @@ def __init__(self, apm_modules_dir: Path) -> None:
"""Create an empty staging session rooted below ``apm_modules``."""
self._modules_dir = apm_modules_dir
self._modules_existed = apm_modules_dir.exists()
self._staging_root = apm_modules_dir / STAGING_DIR_NAME / uuid.uuid4().hex
# 12 hex chars (48 bits) is ample entropy for one install session and
# keeps staged paths well clear of Windows MAX_PATH (issue #2896).
self._staging_root = apm_modules_dir / STAGING_DIR_NAME / uuid.uuid4().hex[:12]
self._staging_lock_path = self._staging_root.with_suffix(".lock")
self._staging_lock: FileLock | None = None
self._backups: dict[Path, Path | None] = {}
Expand Down Expand Up @@ -210,7 +215,10 @@ def _isolated_staging_path(self, bucket: str, destination: Path) -> Path:
"""Return an opaque slot so nested destinations never overlap."""
modules = ensure_path_within(self._modules_dir, self._modules_dir)
relative = destination.relative_to(modules).as_posix().encode("utf-8")
slot = sha256(relative).hexdigest()
# 16 hex chars (64 bits) of the digest is enough to make collisions
# between the destinations staged in one session negligible, while
# keeping staged paths well clear of Windows MAX_PATH (issue #2896).
slot = sha256(relative).hexdigest()[:16]
return self._staging_root / bucket / slot

@staticmethod
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/install/test_install_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,35 @@ def test_success_commit_removes_abandoned_resolution_staging_only(tmp_path: Path
assert (unrelated / "marker").read_text(encoding="ascii") == "keep"


@pytest.mark.parametrize(
"orphan_name_length",
[32, 12],
ids=["legacy-32-char-uuid4-hex", "current-12-char-staging-root"],
)
def test_success_commit_removes_abandoned_staging_of_either_name_length(
tmp_path: Path,
orphan_name_length: int,
) -> None:
"""Orphan cleanup recognises both the pre- and post-#2896 staging-root names.

Shortening the staging root from a full ``uuid4().hex`` (32 hex chars)
to ``uuid4().hex[:12]`` means ``_STAGING_NAME`` must keep matching BOTH
lengths, or an APM upgrade would strand every orphaned staging root left
behind by an older, pre-fix version.
"""
transaction = _transaction(tmp_path)
staging_parent = transaction.apm_modules_dir / ".apm-resolution-staging"
abandoned = staging_parent / ("d" * orphan_name_length)
(abandoned / "package").mkdir(parents=True)
abandoned.with_suffix(".lock").write_text("", encoding="ascii")
(abandoned / "package" / "marker").write_text("stale", encoding="ascii")

transaction.commit(InstallResult())

assert not abandoned.exists()
assert not abandoned.with_suffix(".lock").exists()


def test_success_commit_preserves_lockless_legacy_staging(tmp_path: Path) -> None:
"""Lockless backups remain until the user confirms no legacy install is active."""
transaction = _transaction(tmp_path)
Expand Down
49 changes: 49 additions & 0 deletions tests/unit/install/test_resolution_staging_relocate.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Direct regression tests for resolution-staging path relocations."""

import re
from pathlib import Path

import pytest
Expand Down Expand Up @@ -196,3 +197,51 @@ def test_case_only_relocation_updates_spelling_and_rolls_back(tmp_path: Path) ->

staging.rollback()
assert [path.name for path in modules.iterdir()] == ["mixedorg"]


def test_prepare_replacement_slot_names_fit_windows_max_path(tmp_path: Path) -> None:
"""A realistic staged path must not overflow Windows MAX_PATH (issue #2896).

Before the fix, every staged path carried a 32-hex-char staging root
(``uuid4().hex``) plus a 64-hex-char per-destination slot
(``sha256(...).hexdigest()``) -- 96 hex characters of pure entropy on
top of the project root and a package's own nested directories. Added to
a realistic Windows project location that reliably pushed staged paths
past the 260-character MAX_PATH, raising
``[WinError 206] The filename or extension is too long.``. This guards
the fix: the staging root is now 12 hex chars and the per-destination
slot is 16 (28 total), freeing 68 characters on every staged path.
"""
modules = tmp_path / "apm_modules"
destination = (
modules
/ "acme-platform-org"
/ "enterprise-notification-templates-package"
/ "src"
/ "templates"
/ "email"
/ "transactional"
)
staging = ResolutionStagingSession(modules)

replacement = staging.prepare_replacement(destination)

staging_root_name = replacement.parents[1].name
slot_name = replacement.name
assert re.fullmatch(r"[0-9a-f]{12}", staging_root_name), staging_root_name
assert re.fullmatch(r"[0-9a-f]{16}", slot_name), slot_name

# Re-root the real, generated relative path (unmocked, straight out of
# prepare_replacement) under a realistic Windows project location -- a
# OneDrive-synced repo checkout, a common enterprise layout -- to check
# the MAX_PATH arithmetic the issue describes independent of this test
# run's own (highly variable) tmp_path length.
realistic_root = (
r"C:\Users\jennifer.smith\OneDrive - Contoso Corporation\Documents"
r"\GitHub\internal-tools-platform\services\billing-reconciliation-worker"
)
relative_len = len(str(replacement.relative_to(modules))) + len("apm_modules") + 1
old_scheme_relative_len = relative_len + (32 - 12) + (64 - 16)

assert len(realistic_root) + 1 + relative_len <= 260
assert len(realistic_root) + 1 + old_scheme_relative_len > 260