diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a17e8601..8b3d9a286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) - CI self-check job now uses `setup-only: true` + `apm audit --ci --no-drift` so managed files are not overwritten by `apm install` before `content-integrity` runs; documented the audit-only CI pattern and the install-before-audit blind spot in the enterprise and CI/CD guides. (#1291) - 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) diff --git a/docs/src/content/docs/reference/policy-schema.md b/docs/src/content/docs/reference/policy-schema.md index 155a1a537..2d9c569a0 100644 --- a/docs/src/content/docs/reference/policy-schema.md +++ b/docs/src/content/docs/reference/policy-schema.md @@ -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`. | @@ -125,9 +125,11 @@ Files in primitive target directories that are not recorded in `apm.lock.yaml`. For supply-chain safety, `extends:` references are pinned to the **leaf policy's host** -- a policy fetched from `github.com` cannot extend one on `evil.example.com`. -### Merge rules (tighten-only) +### Merge rules -The child can tighten the parent but never relax it: +Most fields tighten as the policy chain descends. The exceptions are +`deny`/`require` lists, where a child may use `[]` to explicitly clear an +inherited list (see the tri-state table below). | Field family | Merge rule | |-----------------------------|----------------------------------------------------------------------------------| @@ -135,7 +137,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. | | `dependencies.max_depth` | `min(parent, child)`. | | `dependencies.require_resolution` | Stricter wins (`block` > `policy-wins` > `project-wins`). | | `mcp.self_defined` | Stricter wins (`deny` > `warn` > `allow`). | @@ -157,7 +159,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. | +| `[]` | "explicitly empty" | Overrides parent; no entries accumulate. | +| `[...]` | "these entries" | Unioned with parent list (parent order preserved). | ## Complete example diff --git a/packages/apm-guide/.apm/skills/apm-usage/governance.md b/packages/apm-guide/.apm/skills/apm-usage/governance.md index 04c5e14c4..9f231a312 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/governance.md +++ b/packages/apm-guide/.apm/skills/apm-usage/governance.md @@ -77,16 +77,18 @@ undeclared local content at install / audit time. | `warn` | Violations reported but do not fail | | `block` | Violations abort `apm install` (exit 1) AND fail `apm audit --ci` | -## Inheritance rules (tighten-only) +## Inheritance rules -Child policies can only tighten parent policies, never relax them: +Most fields tighten as the policy chain descends. The exception is `deny` and +`require` lists: a child policy may use `[]` to explicitly clear an inherited +list (removing entries the parent set). All other fields obey the rules below: | Field | Merge rule | |-------|-----------| | `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)` | | `mcp.self_defined` | Escalates: `allow` < `warn` < `deny` | | `source_attribution` | `parent OR child` (either enables) | @@ -138,7 +140,7 @@ apm audit --ci --policy https://... # remote policy URL **Note:** Install-time policy enforcement (issue #827) is in active development. The behaviour described below reflects the shipping design. -**Non-goal — structured output:** install-time enforcement does NOT emit JSON or SARIF. Output is human-readable terminal text only. For machine-readable policy reports use `apm audit --ci --format json` or `apm audit --ci --format sarif`. +**Non-goal -- structured output:** install-time enforcement does NOT emit JSON or SARIF. Output is human-readable terminal text only. For machine-readable policy reports use `apm audit --ci --format json` or `apm audit --ci --format sarif`. ### 1. What APM policy is @@ -149,18 +151,18 @@ may use. This section covers how that contract is enforced at `apm install` time ### 2. Discovery and applicability APM auto-discovers policy from `/.github/apm-policy.yml` for any GitHub -remote — both `github.com` and GitHub Enterprise (GHE). Non-GitHub remotes (ADO, +remote -- both `github.com` and GitHub Enterprise (GHE). Non-GitHub remotes (ADO, GitLab, plain git) currently fall through with no policy applied; tracked as a follow-up. Repositories with no detectable git remote (unpacked bundles, temp dirs) emit an explicit "could not determine org" line and skip discovery. -The `--policy ` flag is **audit-only today** — it works on +The `--policy ` flag is **audit-only today** -- it works on `apm audit --ci` but is not yet wired through `apm install`. ### 3. Inheritance and composition Policy resolves through the chain: enterprise hub -> org -> repo override. -The merge is **tighten-only** (see "Inheritance rules" above). +The merge follows "Inheritance rules" above (most fields tighten; deny/require lists support explicit `[]` override). **Multi-level extends:** install-time enforcement and `apm audit --ci` both resolve the full `extends:` chain up to `MAX_CHAIN_DEPTH = 5`. Cycles are @@ -179,12 +181,12 @@ merges what it resolved and emits a `Policy chain incomplete` warning. | Command | Behaviour | |---------|-----------| -| `apm install` | NEW — gate runs after resolve, before integration / target writes | -| `apm install ` | NEW — snapshot apm.yml, run gate, rollback on block | -| `apm install --mcp` | NEW — dedicated MCP preflight | -| `apm deps update` | NEW — runs the install pipeline, so the same gate applies | -| `apm install --dry-run` | NEW — read-only preflight; renders "would be blocked" | -| `apm audit --ci` | Existing — same checks against on-disk manifest + lockfile | +| `apm install` | NEW -- gate runs after resolve, before integration / target writes | +| `apm install ` | NEW -- snapshot apm.yml, run gate, rollback on block | +| `apm install --mcp` | NEW -- dedicated MCP preflight | +| `apm deps update` | NEW -- runs the install pipeline, so the same gate applies | +| `apm install --dry-run` | NEW -- read-only preflight; renders "would be blocked" | +| `apm audit --ci` | Existing -- same checks against on-disk manifest + lockfile | `pack` and `bundle` are out of scope (author-side, not dependency consumers). @@ -194,9 +196,9 @@ merges what it resolved and emits a `Policy chain incomplete` warning. `require_resolution: project-wins` has a narrow semantic: - Downgrades **version-pin mismatches** on required packages to warnings only. -- Does **NOT** downgrade missing required packages — those still block under +- Does **NOT** downgrade missing required packages -- those still block under `enforcement: block`. -- Does **NOT** override an inherited org `deny` — parent deny always wins. +- Does **NOT** override an inherited org `deny` -- parent deny always wins. ### 7. CLI examples @@ -385,7 +387,7 @@ Violation classes: | `transitive_mcp` | MCP server pulled in by a transitive dep, blocked by `mcp.deny` / `transport` / `self_defined` | Remove offending dep, request policy update, or set `mcp.trust_transitive: true` | Full message text per outcome and per class lives in -`docs/src/content/docs/enterprise/policy-reference.md` §10. Violation messages +`docs/src/content/docs/enterprise/policy-reference.md` section10. Violation messages flow through `InstallLogger.policy_violation`; under `block` they print inline as `[x]` errors and exit `1`. @@ -399,7 +401,7 @@ Checklist to publish a policy: 3. Set `enforcement: warn` first. Let CI surface diagnostics across consuming repos for one cycle without breaking installs. 4. When the warn-cycle is clean, switch to `enforcement: block`. Communicate - the change — `apm install` will start failing for non-compliant repos. + the change -- `apm install` will start failing for non-compliant repos. 5. Use `extends:` for team-specific overrides on top of the org baseline rather than forking the file. diff --git a/src/apm_cli/commands/policy.py b/src/apm_cli/commands/policy.py index add68cb9a..e3edc8bbb 100644 --- a/src/apm_cli/commands/policy.py +++ b/src/apm_cli/commands/policy.py @@ -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), "mcp_transports_allowed": _allow_count(policy.mcp.transport.allow), diff --git a/src/apm_cli/policy/discovery.py b/src/apm_cli/policy/discovery.py index e88f65c57..b2ccd6e10 100644 --- a/src/apm_cli/policy/discovery.py +++ b/src/apm_cli/policy/discovery.py @@ -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, }, @@ -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 diff --git a/src/apm_cli/policy/inheritance.py b/src/apm_cli/policy/inheritance.py index d5f3024cc..cb2b67b40 100644 --- a/src/apm_cli/policy/inheritance.py +++ b/src/apm_cli/policy/inheritance.py @@ -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 ), @@ -211,6 +211,33 @@ 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 override; clears parent entries, + returning ``()``. Child can use ``[]`` in YAML to clear an inherited + deny/require list. + * both truthy -- union; child entries are added to parent entries + (deduped, parent order preserved). + + 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() diff --git a/src/apm_cli/policy/matcher.py b/src/apm_cli/policy/matcher.py index cf40e491d..db0be46c0 100644 --- a/src/apm_cli/policy/matcher.py +++ b/src/apm_cli/policy/matcher.py @@ -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( diff --git a/src/apm_cli/policy/parser.py b/src/apm_cli/policy/parser.py index 8093716c9..ab7bc3169 100644 --- a/src/apm_cli/policy/parser.py +++ b/src/apm_cli/policy/parser.py @@ -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), ) diff --git a/src/apm_cli/policy/policy_checks.py b/src/apm_cli/policy/policy_checks.py index 6fcbb863e..6780ae3a6 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -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, @@ -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, @@ -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) @@ -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, @@ -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 @@ -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", diff --git a/src/apm_cli/policy/schema.py b/src/apm_cli/policy/schema.py index 4b17c9e28..07315efcb 100644 --- a/src/apm_cli/policy/schema.py +++ b/src/apm_cli/policy/schema.py @@ -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 @@ -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 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: diff --git a/tests/unit/policy/test_chain_discovery_shared.py b/tests/unit/policy/test_chain_discovery_shared.py index 24279e244..3293408c5 100644 --- a/tests/unit/policy/test_chain_discovery_shared.py +++ b/tests/unit/policy/test_chain_discovery_shared.py @@ -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, diff --git a/tests/unit/policy/test_discovery.py b/tests/unit/policy/test_discovery.py index acc87f465..bc9f3af03 100644 --- a/tests/unit/policy/test_discovery.py +++ b/tests/unit/policy/test_discovery.py @@ -1,4 +1,4 @@ -"""Tests for apm_cli.policy.discovery — policy auto-discovery engine.""" +"""Tests for apm_cli.policy.discovery -- policy auto-discovery engine.""" from __future__ import annotations @@ -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") + 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.""" diff --git a/tests/unit/policy/test_inheritance.py b/tests/unit/policy/test_inheritance.py index 6872d7fd4..4e3602cb3 100644 --- a/tests/unit/policy/test_inheritance.py +++ b/tests/unit/policy/test_inheritance.py @@ -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.""" @@ -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): diff --git a/tests/unit/policy/test_parser.py b/tests/unit/policy/test_parser.py index 8fe9e8399..3b4bbffb4 100644 --- a/tests/unit/policy/test_parser.py +++ b/tests/unit/policy/test_parser.py @@ -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") @@ -325,6 +326,28 @@ def test_omitted_unmanaged_files_yields_none_action(self): self.assertEqual(policy.unmanaged_files.effective_action, "ignore") self.assertEqual(policy.unmanaged_files.directories, ()) + def test_absent_dependencies_block_gives_none_deny_and_require(self): + """Entirely absent dependencies: block -> deny=None, require=None (Fix 2).""" + policy, _ = load_policy("name: p\nversion: '1'\nenforcement: warn\n") + self.assertIsNone(policy.dependencies.deny, "absent block must yield deny=None") + self.assertIsNone(policy.dependencies.require, "absent block must yield require=None") + + def test_yaml_null_deny_gives_none(self): + """YAML 'deny: null' (or bare 'deny:') must be treated as no opinion, not empty list (Fix 2).""" + yaml_str = "name: p\nversion: '1'\nenforcement: warn\ndependencies:\n deny:\n require:\n" + policy, _ = load_policy(yaml_str) + self.assertIsNone(policy.dependencies.deny, "deny: null must yield None, not ()") + self.assertIsNone(policy.dependencies.require, "require: null must yield None, not ()") + + def test_explicit_empty_deny_list_gives_empty_tuple(self): + """Explicit 'deny: []' must give () (explicit empty override), not None.""" + yaml_str = ( + "name: p\nversion: '1'\nenforcement: warn\ndependencies:\n deny: []\n require: []\n" + ) + policy, _ = load_policy(yaml_str) + self.assertEqual(policy.dependencies.deny, (), "deny: [] must yield ()") + self.assertEqual(policy.dependencies.require, (), "require: [] must yield ()") + class TestLoadPolicyFromFile(unittest.TestCase): """Test load_policy from a file path.""" diff --git a/tests/unit/policy/test_schema.py b/tests/unit/policy/test_schema.py index 6eea415aa..d26818185 100644 --- a/tests/unit/policy/test_schema.py +++ b/tests/unit/policy/test_schema.py @@ -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)