fix: recognize host-level UNC roots in path containment checks

.github/workflows/windows_smb_test.yml

@@ -0,0 +1,66 @@
+name: Windows SMB Path Test
+
+# Validates UNC path containment against a real SMB share, which lexical ntpath modeling
+# cannot do. Regression coverage for issue #1321.
+#
+# Not part of Code Quality: creating a share needs administrator rights, and the loopback
+# share is slower and more environment-dependent than a unit test.
+on:
+  workflow_dispatch:
+  workflow_call:
+    inputs:
+      tag:
+        description: Git ref (tag/branch/SHA) to test. Defaults to the triggering ref.
+        required: false
+        type: string
+        default: ''
+
+jobs:
+  test:
+    name: UNC Containment (real SMB)
+    runs-on: windows-latest
+    timeout-minutes: 15
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          ref: ${{ inputs.tag || github.ref }}
+
+      - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
+        with:
+          python-version: '3.12'
+
+      - name: Confirm SMB prerequisites
+        # Fail with a clear message here rather than having every test skip itself,
+        # which would look like a pass.
+        shell: pwsh
+        run: |
+          $admin = ([Security.Principal.WindowsPrincipal] `
+            [Security.Principal.WindowsIdentity]::GetCurrent()
+          ).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)
+          if (-not $admin) { throw "Administrator rights are required to create an SMB share." }
+          Get-Service LanmanServer, LanmanWorkstation | Format-Table -AutoSize
+          Start-Service LanmanServer
+          Start-Service LanmanWorkstation
+          # Developer Mode lets a non-elevated process create symlinks; the escape
+          # test needs one and skips itself otherwise.
+          $key = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\AppModelUnlock'
+          New-Item -Path $key -Force | Out-Null
+          Set-ItemProperty -Path $key -Name AllowDevelopmentWithoutDevLicense -Value 1 -Type DWord
+
+      - name: Install
+        run: |
+          python -m pip install --upgrade pip hatch
+
+      - name: Run the SMB path tests
+        shell: pwsh
+        run: |
+          hatch run pytest test/integ/windows_smb -v --no-cov -p no:randomly
+
+      - name: Report skips
+        # A skipped SMB test is indistinguishable from a passing one in the summary,
+        # so surface the count explicitly.
+        if: always()
+        shell: pwsh
+        run: |
+          hatch run pytest test/integ/windows_smb --no-cov -q -rs 2>&1 |
+            Select-String -Pattern 'SKIPPED|passed|failed'

pyproject.toml

@@ -123,9 +123,29 @@ line-length = 100
 # E402 (imports must be at the top of the file) is pinned explicitly here so it
 # can't silently regress if a future ruff drops it from the defaults; it is
 # build-failing via `hatch run lint`.
-extend-select = ["RUF022", "E402"]
+# TID251 bans path helpers that raise on Windows UNC paths; see banned-api below.
+extend-select = ["RUF022", "E402", "TID251"]
 ignore = ["E501"]
 
+[tool.ruff.lint.flake8-tidy-imports.banned-api]
+# commonpath raises ValueError on Windows UNC paths (issue #1321): callers that catch it
+# silently reject valid paths, callers that don't crash. ntpath/posixpath are banned too
+# because this codebase passes explicit path modules around.
+"os.path.commonpath".msg = "Use deadline.client._path_utils.is_path_contained for containment checks, or deadline.client._path_summary.common_ancestor for a displayed summary; commonpath raises ValueError on Windows UNC paths. See issue #1321."
+"ntpath.commonpath".msg = "Use deadline.client._path_utils.is_path_contained for containment checks, or deadline.client._path_summary.common_ancestor for a displayed summary; commonpath raises ValueError on Windows UNC paths. See issue #1321."
+"posixpath.commonpath".msg = "Use deadline.client._path_utils.is_path_contained for containment checks, or deadline.client._path_summary.common_ancestor for a displayed summary; commonpath raises ValueError on Windows UNC paths. See issue #1321."
+# commonprefix compares strings, not path components, so it reports '\\host\share2' as
+# sharing a prefix with '\\host\share'. It is never the right containment primitive.
+"os.path.commonprefix".msg = "Use deadline.client._path_utils.is_path_contained or deadline.client._path_summary.common_ancestor; commonprefix is a string-prefix match, not a path-component match."
+
+[tool.ruff.lint.per-file-ignores]
+# The sanctioned wrappers, and the one place allowed to reach for what they replace.
+"src/deadline/client/_path_utils.py" = ["TID251"]
+# These compare the wrappers against the stdlib behavior they replace.
+"test/unit/deadline_client/test_path_utils.py" = ["TID251"]
+"test/unit/deadline_client/test_path_summary.py" = ["TID251"]
+"test/unit/deadline_client/api/test_job_bundle_submission_asset_refs.py" = ["TID251"]
+
 [tool.ruff.lint.isort]
 known-first-party = ["deadline"]
 

src/deadline/client/_path_summary.py

@@ -0,0 +1,74 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+
+"""
+Summarizing a group of paths for display.
+
+Kept out of ``_path_utils`` because this is presentation rather than a trust decision, and
+it carries cases that only a displayed string cares about -- unresolved ``..`` runs and
+preserving the caller's spelling -- which a reader auditing containment should not have to
+read past.
+
+Like the containment helpers, this is purely lexical and never raises.
+"""
+
+from __future__ import annotations
+
+import os
+from typing import Any, Sequence
+
+from ._path_utils import _PARDIR, _UNC_ANCHOR, _split_anchored
+
+__all__ = [
+    "common_ancestor",
+]
+
+
+def _leading_pardir_count(parts: list[str]) -> int:
+    """Count the leading '..' run that ``normpath`` could not resolve.
+
+    Counted on parts rather than whole components because an anchor can precede the run
+    ('C:..\\x' is the parent of the working directory on drive C:).
+    """
+    count = 0
+    for part in parts:
+        if part != _PARDIR:
+            break
+        count += 1
+    return count
+
+
+def common_ancestor(paths: Sequence[Any], *, path_module: Any = os.path) -> str:
+    """Return the deepest directory containing every path in ``paths``.
+
+    This is ``os.path.commonpath`` without the exceptions: paths in unrelated spaces return
+    ``""`` rather than raising, and a UNC host is a valid answer for paths on different
+    shares of one server. The result keeps the first path's spelling and, like
+    ``commonpath``, is purely lexical.
+    """
+    if not paths:
+        return ""
+
+    split = [_split_anchored(p, path_module, normalize_case=True) for p in paths]
+    normalized = [([a] if a else []) + parts for a, parts in split]
+    anchor, spelled_parts = _split_anchored(paths[0], path_module, normalize_case=False)
+    spelled = ([anchor] if anchor else []) + spelled_parts
+
+    # '..' and '../..' are rooted at different unknown places, so runs of differing depth
+    # share nothing. Comparing them positionally would return the shallower path, which is
+    # not an ancestor of the deeper one -- os.path.commonpath has that bug.
+    if len({_leading_pardir_count(parts) for _, parts in split}) > 1:
+        return ""
+
+    shared = min(len(components) for components in normalized)
+    while shared > 0 and any(other[:shared] != normalized[0][:shared] for other in normalized):
+        shared -= 1
+    if shared == 0:
+        return ""
+    # Matching only the bare anchor means different servers, so no shared directory.
+    if shared == 1 and normalized[0][0] == _UNC_ANCHOR:
+        return ""
+
+    # The anchor carries its own separator, so it abuts the first part directly.
+    if anchor:
+        return anchor + path_module.sep.join(spelled[1:shared])
+    return path_module.sep.join(spelled[:shared])