Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
14 changes: 10 additions & 4 deletions docs/src/content/docs/reference/policy-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ Rules over the `dependencies:` and `mcp:` blocks declared in consumer `apm.yml`
| Field | Type | Default | Notes |
|----------------------|-----------------------|------------------|---------------------------------------------------------------------------------------------|
| `allow` | list of patterns or null | `null` | `null` = no opinion. `[]` = nothing allowed. `[...]` = only these. |
| `deny` | list of patterns | `[]` | Always wins over `allow`. |
| `require` | list of refs | `[]` | Packages every consumer manifest must include. |
| `deny` | list of patterns or null | `null` | `null` = no opinion (transparent during merge). `[]` = explicitly empty. Always wins over `allow`. |
| `require` | list of refs or null | `null` | `null` = no opinion (transparent during merge). `[]` = explicitly empty. Packages every consumer manifest must include. |
| `require_resolution` | enum | `project-wins` | `project-wins` / `policy-wins` / `block` -- how to resolve version conflicts on required packages. |
| `max_depth` | integer | `50` | Maximum transitive dependency depth. Must be `> 0`. |

Expand Down Expand Up @@ -135,7 +135,7 @@ The child can tighten the parent but never relax it:
| `fetch_failure` | Child overrides if set. |
| `cache.ttl` | `min(parent, child)`. |
| `*.allow` lists | Set intersection. `null` is transparent (no opinion). |
| `*.deny` / `require` lists | Union, deduplicated, parent order preserved. |
| `*.deny` / `require` lists | Union, deduplicated, parent order preserved. Omitting the field (or setting it to `null`) is transparent — the parent value passes through unchanged. `[]` is an explicit empty override. |
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
| `dependencies.max_depth` | `min(parent, child)`. |
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
| `dependencies.require_resolution` | Stricter wins (`block` > `policy-wins` > `project-wins`). |
| `mcp.self_defined` | Stricter wins (`deny` > `warn` > `allow`). |
Expand All @@ -157,7 +157,13 @@ For every `allow:` field, the three states are distinct:
| `[]` | "explicitly nothing" | Intersects to nothing downstream. |
| `[...]` | "only these patterns" | Intersected with child list. |

`deny` and `require` lists are always tuples (omitted = empty); they accumulate by union during merge.
`deny` and `require` lists support the same three-state semantics as `allow`:

| Value | Meaning | Inheritance behavior |
|----------|----------------------------|----------------------------------------------------------|
| omitted / `null` | "no opinion" | Transparent during merge — parent value passes through. |
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
| `[]` | "explicitly empty" | Overrides parent; no entries accumulate. |
| `[...]` | "these entries" | Unioned with parent list (parent order preserved). |

## Complete example

Expand Down
4 changes: 2 additions & 2 deletions packages/apm-guide/.apm/skills/apm-usage/governance.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ Child policies can only tighten parent policies, never relax them:
|-------|-----------|
| `enforcement` | Escalates: `off` < `warn` < `block` |
| Allow lists | Intersection (child narrows parent) |
| Deny lists | Union (child adds to parent) |
| `require` | Union (combines required packages) |
| Deny lists | Union (child adds to parent). Omitting or `null` = transparent; `[]` = explicit empty override. |
| `require` | Union (combines required packages). Omitting or `null` = transparent; `[]` = explicit empty override. |
| `max_depth` | `min(parent, child)` |
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
| `mcp.self_defined` | Escalates: `allow` < `warn` < `deny` |
| `source_attribution` | `parent OR child` (either enables) |
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": _opt_list(policy.dependencies.deny),
"require": _opt_list(policy.dependencies.require),
"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
12 changes: 9 additions & 3 deletions src/apm_cli/policy/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,17 @@ 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 or deps_data["deny"] is None)
else _parse_tuple(deps_data["deny"]),
require=None
if (_deps_absent or "require" not in deps_data or deps_data["require"] is None)
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
48 changes: 48 additions & 0 deletions tests/unit/policy/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,54 @@ def test_get_cache_dir(self):
expected = root.resolve() / "apm_modules" / ".policy-cache"
self.assertEqual(_get_cache_dir(root), expected)

def test_round_trip_preserves_none_deny_and_require(self):
"""Cache write→read must preserve deny=None/require=None (tri-state Fix 1).

A policy with no dependencies: block must survive a cache round-trip
as None, not collapse to () which would prevent parent inheritance.
"""
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
repo_ref = "contoso/.github"

# Policy with no dependencies: block → deny=None, require=None
policy, _ = load_policy("name: p\nversion: '1'\nenforcement: warn\n")
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
self.assertIsNone(policy.dependencies.deny)
self.assertIsNone(policy.dependencies.require)

_write_cache(repo_ref, policy, root)
result = _read_cache(repo_ref, root)

self.assertIsNotNone(result)
self.assertIsNone(
result.policy.dependencies.deny,
"deny must survive cache round-trip as None, not collapse to ()",
)
self.assertIsNone(
result.policy.dependencies.require,
"require must survive cache round-trip as None, not collapse to ()",
)

def test_round_trip_preserves_explicit_empty_deny(self):
"""Cache round-trip must preserve deny=() (explicit empty override)."""
with tempfile.TemporaryDirectory() as tmpdir:
root = Path(tmpdir)
repo_ref = "contoso/.github"

yaml_str = "name: p\nversion: '1'\nenforcement: warn\ndependencies:\n deny: []\n"
policy, _ = load_policy(yaml_str)
self.assertEqual(policy.dependencies.deny, ())

_write_cache(repo_ref, policy, root)
result = _read_cache(repo_ref, root)

self.assertIsNotNone(result)
self.assertEqual(
result.policy.dependencies.deny,
(),
"deny=[] must survive cache round-trip as () (explicit empty)",
)


class TestFetchGithubContents(unittest.TestCase):
"""Test _fetch_github_contents with mocked requests."""
Expand Down
Loading
Loading