Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- `extends: org` now correctly layers `dependencies.require` and `dependencies.deny` from the parent policy when the child omits the `dependencies:` block entirely; `None` signals "no opinion" (transparent) while `[]` signals explicit override. (#1290)
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
- Pin `Path.home()` under unit tests via a session-scoped autouse conftest fixture, fixing 56 Windows runner failures on the new `windows-2025-vs2026` GitHub-hosted image where `USERPROFILE`/`HOMEDRIVE`+`HOMEPATH` are not seeded for pytest workers; also patch the `_check_and_notify_updates` import binding in the disabled-self-update test so it no longer races on the version-check cache. (#1270)
- `apm install` now works on macOS git 2.53.0 (Homebrew): bare-cache commands switch to `--git-dir` to satisfy the `safe.bareRepository=explicit` default; fetched SHAs are pinned as synthetic refs so `git clone --local --shared` no longer silently omits them. (#1268)
- Set the unit-test hermetic HOME at conftest import time so a single xdist worker on the `windows-2025-vs2026` runner can no longer race fixture setup and re-trigger the 53 `Path.home()` failures the session-scoped autouse fixture was supposed to prevent. (#1271)
Expand Down
4 changes: 2 additions & 2 deletions src/apm_cli/commands/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ def _allow_count(value: tuple | None) -> int:
return -1 if value is None else len(value)

return {
"dependencies_deny": len(policy.dependencies.deny),
"dependencies_deny": len(policy.dependencies.effective_deny),
"dependencies_allow": _allow_count(policy.dependencies.allow),
"dependencies_require": len(policy.dependencies.require),
"dependencies_require": len(policy.dependencies.effective_require),
"mcp_deny": len(policy.mcp.deny),
"mcp_allow": _allow_count(policy.mcp.allow),
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
"mcp_transports_allowed": _allow_count(policy.mcp.transport.allow),
Expand Down
8 changes: 4 additions & 4 deletions src/apm_cli/policy/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -1049,8 +1049,8 @@ def _opt_list(val: tuple[str, ...] | None) -> list | None:
"cache": {"ttl": policy.cache.ttl},
"dependencies": {
"allow": _opt_list(policy.dependencies.allow),
"deny": list(policy.dependencies.deny),
"require": list(policy.dependencies.require),
"deny": list(policy.dependencies.effective_deny),
"require": list(policy.dependencies.effective_require),
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
"require_resolution": policy.dependencies.require_resolution,
"max_depth": policy.dependencies.max_depth,
},
Expand Down Expand Up @@ -1104,9 +1104,9 @@ def _is_policy_empty(policy: ApmPolicy) -> bool:
beyond the permissive defaults.
"""
return (
not policy.dependencies.deny
not policy.dependencies.effective_deny
and policy.dependencies.allow is None
and not policy.dependencies.require
and not policy.dependencies.effective_require
and not policy.mcp.deny
and policy.mcp.allow is None
and policy.mcp.transport.allow is None
Expand Down
28 changes: 26 additions & 2 deletions src/apm_cli/policy/inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ def _merge_cache(parent: PolicyCache, child: PolicyCache) -> PolicyCache:

def _merge_dependencies(parent: DependencyPolicy, child: DependencyPolicy) -> DependencyPolicy:
return DependencyPolicy(
deny=_union(parent.deny, child.deny),
deny=_merge_list_field(parent.deny, child.deny),
allow=_intersect_allow(parent.allow, child.allow),
require=_union(parent.require, child.require),
require=_merge_list_field(parent.require, child.require),
require_resolution=_escalate(
_RESOLUTION_LEVELS, parent.require_resolution, child.require_resolution
),
Expand Down Expand Up @@ -211,6 +211,30 @@ def _merge_unmanaged_files(
# ---------------------------------------------------------------------------


def _merge_list_field(
parent: tuple[str, ...] | None,
child: tuple[str, ...] | None,
) -> tuple[str, ...] | None:
"""Merge a deny/require list field with None-transparency and union.

* ``child is None`` -- no opinion; parent flows through (transparent).
* ``child`` is empty -- explicit empty; overrides parent with ``()``.
* both truthy -- union (child adds, never removes from parent).

Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Always returns a ``tuple`` or ``None``; never a bare list.
"""
if child is None:
# Transparent: parent flows through. Normalise to tuple if non-None
# so callers always receive a uniform type regardless of how parent
# was constructed in tests (list vs tuple).
return _union((), parent) if parent is not None else None
if not child:
return () # explicit empty: override parent
if parent is None or not parent:
return _union((), child) # parent has nothing; child wins
return _union(parent, child) # both have values: union


def _union(a: tuple[str, ...], b: tuple[str, ...]) -> tuple[str, ...]:
"""Deduplicated union preserving first-seen order."""
seen: set[str] = set()
Expand Down
2 changes: 1 addition & 1 deletion src/apm_cli/policy/matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def check_dependency_allowed(
policy: DependencyPolicy,
) -> tuple[bool, str]:
"""Check if a dependency is allowed by policy."""
return _check_allow_deny(canonical_ref, policy.allow, policy.deny)
return _check_allow_deny(canonical_ref, policy.allow, policy.effective_deny)


def check_mcp_allowed(
Expand Down
10 changes: 7 additions & 3 deletions src/apm_cli/policy/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,15 @@ def _build_policy(data: dict) -> ApmPolicy:
ttl=cache_data.get("ttl", PolicyCache.ttl),
)

deps_data = data.get("dependencies") or {}
_raw_deps = data.get("dependencies")
deps_data = _raw_deps if isinstance(_raw_deps, dict) else {}
_deps_absent = _raw_deps is None
dependencies = DependencyPolicy(
allow=_parse_allow(deps_data.get("allow")),
deny=_parse_tuple(deps_data.get("deny")),
require=_parse_tuple(deps_data.get("require")),
deny=None if (_deps_absent or "deny" not in deps_data) else _parse_tuple(deps_data["deny"]),
require=None
if (_deps_absent or "require" not in deps_data)
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
else _parse_tuple(deps_data["require"]),
require_resolution=deps_data.get("require_resolution", DependencyPolicy.require_resolution),
max_depth=deps_data.get("max_depth", DependencyPolicy.max_depth),
)
Expand Down
12 changes: 6 additions & 6 deletions src/apm_cli/policy/policy_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def _check_dependency_denylist(
"""Check 2: no dependency matches policy deny list."""
from .matcher import check_dependency_allowed

if not policy.deny:
if not policy.effective_deny:
return CheckResult(
name="dependency-denylist",
passed=True,
Expand Down Expand Up @@ -144,7 +144,7 @@ def _check_required_packages(
policy: DependencyPolicy,
) -> CheckResult:
"""Check 3: every required package is in manifest deps."""
if not policy.require:
if not policy.effective_require:
return CheckResult(
name="required-packages",
passed=True,
Expand All @@ -153,7 +153,7 @@ def _check_required_packages(

dep_names = {dep.get_canonical_dependency_string().split("#")[0] for dep in deps}
missing: list[str] = []
for req in policy.require:
for req in policy.effective_require:
pkg_name = req.split("#")[0]
if pkg_name not in dep_names:
missing.append(pkg_name)
Expand All @@ -178,7 +178,7 @@ def _check_required_packages_deployed(
policy: DependencyPolicy,
) -> CheckResult:
"""Check 4: required packages appear in lockfile with deployed files."""
if not policy.require or lock is None:
if not policy.effective_require or lock is None:
return CheckResult(
name="required-packages-deployed",
passed=True,
Expand All @@ -188,7 +188,7 @@ def _check_required_packages_deployed(
dep_names = {dep.get_canonical_dependency_string().split("#")[0] for dep in deps}
lock_by_name = {locked.get_unique_key(): locked for _key, locked in lock.dependencies.items()}
not_deployed: list[str] = []
for req in policy.require:
for req in policy.effective_require:
pkg_name = req.split("#")[0]
if pkg_name not in dep_names:
continue # not in manifest -- check 3 handles this
Expand Down Expand Up @@ -218,7 +218,7 @@ def _check_required_package_version(
policy: DependencyPolicy,
) -> CheckResult:
"""Check 5: required packages with version pins match per resolution strategy."""
pinned = [(r, r.split("#", 1)) for r in policy.require if "#" in r]
pinned = [(r, r.split("#", 1)) for r in policy.effective_require if "#" in r]
if not pinned or lock is None:
return CheckResult(
name="required-package-version",
Expand Down
20 changes: 16 additions & 4 deletions src/apm_cli/policy/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
* ``()`` -- "explicitly empty" (after merge: nothing is allowed).
* ``(...)`` -- "allow only matching patterns".

Deny/require lists use ``Tuple[str, ...]`` (never ``None``) because
they accumulate via union -- an absent key simply means "nothing to add".
Deny/require list semantics:
* ``None`` -- "no opinion" (transparent during inheritance merge).
* ``()`` -- "explicitly empty" (overrides parent in merge).
* ``(...)`` -- union-merged with parent during inheritance.
"""

from __future__ import annotations
Expand All @@ -29,11 +31,21 @@ class DependencyPolicy:
"""Rules governing which APM dependencies are permitted."""

allow: tuple[str, ...] | None = None
deny: tuple[str, ...] = ()
require: tuple[str, ...] = ()
deny: tuple[str, ...] | None = None # None = no opinion; () = explicit empty
require: tuple[str, ...] | None = None # None = no opinion; () = explicit empty
require_resolution: str = "project-wins" # project-wins | policy-wins | block
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
max_depth: int = 50

@property
def effective_deny(self) -> tuple[str, ...]:
"""Resolved deny list for runtime checks (None -> ())."""
return self.deny if self.deny is not None else ()

@property
def effective_require(self) -> tuple[str, ...]:
"""Resolved require list for runtime checks (None -> ())."""
return self.require if self.require is not None else ()


@dataclass(frozen=True)
class McpTransportPolicy:
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/policy/test_chain_discovery_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
# -- Helpers ---------------------------------------------------------------


def _make_policy(*, enforcement="warn", extends=None, deny=()):
def _make_policy(*, enforcement="warn", extends=None, deny=None):
"""Build a minimal ApmPolicy for testing."""
return ApmPolicy(
enforcement=enforcement,
Expand Down
69 changes: 68 additions & 1 deletion tests/unit/policy/test_inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,72 @@ def test_deduplication(self):
self.assertEqual(result.dependencies.require, ("contoso/hooks",))


class TestDependencyTransparency(unittest.TestCase):
"""Child omitting dependencies block is transparent for deny/require (fixes #1201)."""

def test_parent_require_child_omits_deps_block(self):
"""Parent require + child omits dependencies entirely -> require flows through."""
result = merge_policies(
ApmPolicy(dependencies=DependencyPolicy(require=("contoso/hooks",))),
ApmPolicy(), # child omits dependencies -> require=None
)
self.assertEqual(result.dependencies.require, ("contoso/hooks",))

def test_parent_deny_child_omits_deps_block(self):
"""Parent deny + child omits dependencies entirely -> deny flows through."""
result = merge_policies(
ApmPolicy(dependencies=DependencyPolicy(deny=("evil/*",))),
ApmPolicy(), # child omits dependencies -> deny=None
)
self.assertEqual(result.dependencies.deny, ("evil/*",))

def test_parent_require_child_explicit_empty_require(self):
"""Child explicit empty require=() overrides parent (AC#2)."""
result = merge_policies(
ApmPolicy(dependencies=DependencyPolicy(require=("contoso/hooks",))),
ApmPolicy(dependencies=DependencyPolicy(require=())),
)
self.assertEqual(result.dependencies.require, ())

def test_parent_deny_child_explicit_empty_deny(self):
"""Child explicit empty deny=() overrides parent."""
result = merge_policies(
ApmPolicy(dependencies=DependencyPolicy(deny=("evil/*",))),
ApmPolicy(dependencies=DependencyPolicy(deny=())),
)
self.assertEqual(result.dependencies.deny, ())

def test_three_level_chain_require_transparency(self):
"""Enterprise require -> org omits -> repo omits -> require preserved."""
result = resolve_policy_chain(
[
ApmPolicy(dependencies=DependencyPolicy(require=("contoso/core",))),
ApmPolicy(), # org omits
ApmPolicy(), # repo omits
]
)
self.assertEqual(result.dependencies.require, ("contoso/core",))

def test_three_level_chain_deny_transparency(self):
"""Enterprise deny -> org omits -> repo omits -> deny preserved."""
result = resolve_policy_chain(
[
ApmPolicy(dependencies=DependencyPolicy(deny=("banned/*",))),
ApmPolicy(), # org omits
ApmPolicy(), # repo omits
]
)
self.assertEqual(result.dependencies.deny, ("banned/*",))

def test_both_none_merged_none(self):
"""Both parent and child omit dependencies -> None (no opinion)."""
result = merge_policies(ApmPolicy(), ApmPolicy())
self.assertIsNone(result.dependencies.deny)
self.assertIsNone(result.dependencies.require)
self.assertEqual(result.dependencies.effective_deny, ())
self.assertEqual(result.dependencies.effective_require, ())


class TestRequireResolutionEscalation(unittest.TestCase):
"""require_resolution: project-wins < policy-wins < block."""

Expand Down Expand Up @@ -601,7 +667,8 @@ def test_both_defaults(self):
result = merge_policies(ApmPolicy(), ApmPolicy())
self.assertEqual(result.enforcement, "warn")
self.assertEqual(result.cache.ttl, 3600)
self.assertEqual(result.dependencies.deny, ())
self.assertIsNone(result.dependencies.deny) # None = no opinion from either side
self.assertEqual(result.dependencies.effective_deny, ())
self.assertIsNone(result.dependencies.allow)

def test_extends_cleared_after_merge(self):
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/policy/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ def test_nested_missing_sections_use_defaults(self):
""")
policy, warnings = load_policy(yaml_str) # noqa: RUF059
self.assertEqual(policy.dependencies.allow, ("org/*",))
self.assertEqual(policy.dependencies.deny, ())
self.assertIsNone(policy.dependencies.deny)
self.assertEqual(policy.dependencies.effective_deny, ())
self.assertEqual(policy.dependencies.max_depth, 50)
self.assertEqual(policy.mcp.self_defined, "warn")

Expand Down
6 changes: 4 additions & 2 deletions tests/unit/policy/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ class TestDependencyPolicyDefaults(unittest.TestCase):
def test_defaults(self):
dep = DependencyPolicy()
self.assertIsNone(dep.allow)
self.assertEqual(dep.deny, ())
self.assertEqual(dep.require, ())
self.assertIsNone(dep.deny)
self.assertIsNone(dep.require)
self.assertEqual(dep.effective_deny, ())
self.assertEqual(dep.effective_require, ())
self.assertEqual(dep.require_resolution, "project-wins")
self.assertEqual(dep.max_depth, 50)

Expand Down
Loading