From b35675ab87b406a93bde1197b93de9891cc26754 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 16 Jun 2026 06:36:25 +0200 Subject: [PATCH 1/2] fix(policy): carry all ApmPolicy fields through inheritance merge merge_policies omitted fetch_failure, registry_source, and bin_deploy when reconstructing ApmPolicy, so a policy that extends another silently reset those three fields to defaults. Add _merge_fetch_failure, _merge_registry_source, and _merge_bin_deploy with tighten-not-relax semantics and wire them into merge_policies. Add a structural coverage guard so any future ApmPolicy field forgotten in the merge fails a test. Part of #1774 (prerequisite for #1778) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + src/apm_cli/policy/inheritance.py | 40 +++++++ tests/unit/policy/test_inheritance.py | 144 ++++++++++++++++++++++++++ 3 files changed, 185 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a296305d9..c95493b19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Policy inheritance no longer drops `fetch_failure`, `registry_source`, and `bin_deploy` when a policy `extends` another; these fields now carry and tighten through the merge like sibling sections. (closes #1778) - `apm install` no longer silently ignores MCP servers declared in `devDependencies.mcp`; dev MCP configs and lockfile entries now stay in sync on fresh installs. (closes #1780) (#1787) - `apm compile` now honors `managed_section` mode on distributed root `AGENTS.md` and `--single-agents` writes, preserving hand-authored diff --git a/src/apm_cli/policy/inheritance.py b/src/apm_cli/policy/inheritance.py index 25f0493f0..939aec641 100644 --- a/src/apm_cli/policy/inheritance.py +++ b/src/apm_cli/policy/inheritance.py @@ -14,6 +14,7 @@ from .schema import ( ApmPolicy, AuditPolicy, + BinDeployPolicy, CompilationPolicy, CompilationStrategyPolicy, CompilationTargetPolicy, @@ -22,6 +23,7 @@ McpPolicy, McpTransportPolicy, PolicyCache, + RegistrySourcePolicy, ScannerGovernance, SecurityPolicy, UnmanagedFilesPolicy, @@ -31,6 +33,7 @@ # Escalation ladders -- index = severity, higher is stricter. _ENFORCEMENT_LEVELS = {"off": 0, "warn": 1, "block": 2} +_FETCH_FAILURE_LEVELS = {"warn": 0, "block": 1} _RESOLUTION_LEVELS = {"project-wins": 0, "policy-wins": 1, "block": 2} _SELF_DEFINED_LEVELS = {"allow": 0, "warn": 1, "deny": 2} _UNMANAGED_ACTION_LEVELS = {"ignore": 0, "warn": 1, "deny": 2} @@ -59,13 +62,16 @@ def merge_policies(parent: ApmPolicy, child: ApmPolicy) -> ApmPolicy: version=child.version or parent.version, extends=None, # resolved, no longer needed enforcement=_merge_enforcement(parent.enforcement, child.enforcement), + fetch_failure=_merge_fetch_failure(parent.fetch_failure, child.fetch_failure), cache=_merge_cache(parent.cache, child.cache), dependencies=_merge_dependencies(parent.dependencies, child.dependencies), mcp=_merge_mcp(parent.mcp, child.mcp), compilation=_merge_compilation(parent.compilation, child.compilation), manifest=_merge_manifest(parent.manifest, child.manifest), unmanaged_files=_merge_unmanaged_files(parent.unmanaged_files, child.unmanaged_files), + registry_source=_merge_registry_source(parent.registry_source, child.registry_source), security=_merge_security(parent.security, child.security), + bin_deploy=_merge_bin_deploy(parent.bin_deploy, child.bin_deploy), ) @@ -134,6 +140,40 @@ def _merge_enforcement(parent: str, child: str) -> str: return _escalate(_ENFORCEMENT_LEVELS, parent, child) +def _merge_fetch_failure(parent: str, child: str) -> str: + """Escalate fetch_failure on the warn < block ladder (tighten, never relax).""" + return _escalate(_FETCH_FAILURE_LEVELS, parent, child) + + +def _merge_registry_source( + parent: RegistrySourcePolicy, child: RegistrySourcePolicy +) -> RegistrySourcePolicy: + """Merge registry-source policy: required registries union, restrict-only. + + ``require`` is union-merged like other require lists -- a child can add + mandated registries but never drop a parent's. ``allow_non_registry`` is + AND-merged so once any ancestor blocks non-registry sources (``False``), + a child cannot relax it back to ``True``. + """ + return RegistrySourcePolicy( + require=_union(parent.require, child.require), + allow_non_registry=parent.allow_non_registry and child.allow_non_registry, + ) + + +def _merge_bin_deploy(parent: BinDeployPolicy, child: BinDeployPolicy) -> BinDeployPolicy: + """Merge bin/ deployment policy: restrict-only. + + ``deny_all`` OR-merges so any ancestor's kill-switch (``True``) sticks; + ``deny`` is union-merged so a child can add packages but never drop a + parent's denied entries. + """ + return BinDeployPolicy( + deny_all=parent.deny_all or child.deny_all, + deny=_union(parent.deny, child.deny), + ) + + def _merge_cache(parent: PolicyCache, child: PolicyCache) -> PolicyCache: return PolicyCache(ttl=min(parent.ttl, child.ttl)) diff --git a/tests/unit/policy/test_inheritance.py b/tests/unit/policy/test_inheritance.py index 6afa1e118..e5f54c2f8 100644 --- a/tests/unit/policy/test_inheritance.py +++ b/tests/unit/policy/test_inheritance.py @@ -2,7 +2,9 @@ from __future__ import annotations +import dataclasses import unittest +from typing import ClassVar from apm_cli.policy.inheritance import ( MAX_CHAIN_DEPTH, @@ -15,6 +17,7 @@ from apm_cli.policy.schema import ( ApmPolicy, AuditPolicy, + BinDeployPolicy, CompilationPolicy, CompilationStrategyPolicy, CompilationTargetPolicy, @@ -23,6 +26,7 @@ McpPolicy, McpTransportPolicy, PolicyCache, + RegistrySourcePolicy, SecurityPolicy, UnmanagedFilesPolicy, ) @@ -746,5 +750,145 @@ def test_external_union_merged(self): self.assertEqual(set(result.security.audit.external), {"skillspector", "sarif"}) +class TestFetchFailureEscalation(unittest.TestCase): + """fetch_failure can only escalate: warn < block (closes #829 inheritance).""" + + def _merge(self, parent_val: str, child_val: str) -> str: + return merge_policies( + ApmPolicy(fetch_failure=parent_val), + ApmPolicy(fetch_failure=child_val), + ).fetch_failure + + def test_parent_block_not_relaxed_by_silent_child(self): + # Child default is "warn"; it must not relax a parent "block". + self.assertEqual(self._merge("block", "warn"), "block") + + def test_child_can_tighten(self): + self.assertEqual(self._merge("warn", "block"), "block") + + def test_same_level(self): + self.assertEqual(self._merge("warn", "warn"), "warn") + + +class TestRegistrySourceMerge(unittest.TestCase): + """registry_source tightens: require unions, allow_non_registry restricts.""" + + def test_parent_require_preserved_when_child_silent(self): + parent = ApmPolicy(registry_source=RegistrySourcePolicy(require=("corp",))) + result = merge_policies(parent, ApmPolicy()) + self.assertEqual(result.registry_source.require, ("corp",)) + + def test_require_union_merged(self): + parent = ApmPolicy(registry_source=RegistrySourcePolicy(require=("corp",))) + child = ApmPolicy(registry_source=RegistrySourcePolicy(require=("mirror",))) + result = merge_policies(parent, child) + self.assertEqual(set(result.registry_source.require), {"corp", "mirror"}) + + def test_allow_non_registry_parent_false_preserved(self): + parent = ApmPolicy(registry_source=RegistrySourcePolicy(allow_non_registry=False)) + result = merge_policies(parent, ApmPolicy()) + self.assertFalse(result.registry_source.allow_non_registry) + + def test_allow_non_registry_child_can_tighten(self): + parent = ApmPolicy(registry_source=RegistrySourcePolicy(allow_non_registry=True)) + child = ApmPolicy(registry_source=RegistrySourcePolicy(allow_non_registry=False)) + result = merge_policies(parent, child) + self.assertFalse(result.registry_source.allow_non_registry) + + def test_allow_non_registry_child_cannot_relax(self): + parent = ApmPolicy(registry_source=RegistrySourcePolicy(allow_non_registry=False)) + child = ApmPolicy(registry_source=RegistrySourcePolicy(allow_non_registry=True)) + result = merge_policies(parent, child) + self.assertFalse(result.registry_source.allow_non_registry) + + +class TestBinDeployMerge(unittest.TestCase): + """bin_deploy tightens: deny_all sticks True, deny unions.""" + + def test_parent_deny_all_preserved_when_child_silent(self): + parent = ApmPolicy(bin_deploy=BinDeployPolicy(deny_all=True)) + result = merge_policies(parent, ApmPolicy()) + self.assertTrue(result.bin_deploy.deny_all) + + def test_deny_all_child_can_tighten(self): + parent = ApmPolicy(bin_deploy=BinDeployPolicy(deny_all=False)) + child = ApmPolicy(bin_deploy=BinDeployPolicy(deny_all=True)) + result = merge_policies(parent, child) + self.assertTrue(result.bin_deploy.deny_all) + + def test_deny_all_child_cannot_relax(self): + parent = ApmPolicy(bin_deploy=BinDeployPolicy(deny_all=True)) + child = ApmPolicy(bin_deploy=BinDeployPolicy(deny_all=False)) + result = merge_policies(parent, child) + self.assertTrue(result.bin_deploy.deny_all) + + def test_parent_deny_preserved_when_child_silent(self): + parent = ApmPolicy(bin_deploy=BinDeployPolicy(deny=("owner/repo",))) + result = merge_policies(parent, ApmPolicy()) + self.assertEqual(result.bin_deploy.deny, ("owner/repo",)) + + def test_deny_union_merged(self): + parent = ApmPolicy(bin_deploy=BinDeployPolicy(deny=("a/b",))) + child = ApmPolicy(bin_deploy=BinDeployPolicy(deny=("c/d",))) + result = merge_policies(parent, child) + self.assertEqual(set(result.bin_deploy.deny), {"a/b", "c/d"}) + + +class TestMergeFieldCoverageGuard(unittest.TestCase): + """Regression trap for the whole class of "forgotten field" bugs. + + A field added to ``ApmPolicy`` but not handled in ``merge_policies`` + silently reverts to its default when a policy ``extends`` another. + This guard builds a parent whose every mergeable field is non-default, + merges it with a silent (default) child, and asserts no field reverted + -- so a future forgotten field MUST fail here. + """ + + EXEMPT: ClassVar[set[str]] = {"name", "version", "extends"} + + @staticmethod + def _non_default_parent() -> ApmPolicy: + # Each sample value is chosen so that merging with a silent child + # yields a result distinct from that field's default, proving the + # parent value was carried through rather than dropped. + return ApmPolicy( + enforcement="block", + fetch_failure="block", + cache=PolicyCache(ttl=1800), + dependencies=DependencyPolicy(max_depth=10), + mcp=McpPolicy(self_defined="deny"), + compilation=CompilationPolicy(source_attribution=True), + manifest=ManifestPolicy(scripts="deny"), + unmanaged_files=UnmanagedFilesPolicy(action="deny"), + registry_source=RegistrySourcePolicy(require=("corp",)), + security=SecurityPolicy(audit=AuditPolicy(on_install="block")), + bin_deploy=BinDeployPolicy(deny_all=True), + ) + + def test_sample_covers_every_mergeable_field(self): + default = ApmPolicy() + sample = self._non_default_parent() + sampled = { + f.name + for f in dataclasses.fields(ApmPolicy) + if getattr(sample, f.name) != getattr(default, f.name) + } + all_fields = {f.name for f in dataclasses.fields(ApmPolicy)} + self.assertEqual(sampled, all_fields - self.EXEMPT) + + def test_no_field_dropped_on_merge(self): + parent = self._non_default_parent() + merged = merge_policies(parent, ApmPolicy()) + default = ApmPolicy() + for f in dataclasses.fields(ApmPolicy): + if f.name in self.EXEMPT: + continue + self.assertNotEqual( + getattr(merged, f.name), + getattr(default, f.name), + f"merge_policies dropped field {f.name!r}: it reverted to its default", + ) + + if __name__ == "__main__": unittest.main() From a08fac414b1631acd401e43e9ba902f28be39131 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 16 Jun 2026 06:36:58 +0200 Subject: [PATCH 2/2] docs(changelog): add PR number to policy inheritance fix entry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c95493b19..114ff5439 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Policy inheritance no longer drops `fetch_failure`, `registry_source`, and `bin_deploy` when a policy `extends` another; these fields now carry and tighten through the merge like sibling sections. (closes #1778) +- Policy inheritance no longer drops `fetch_failure`, `registry_source`, and `bin_deploy` when a policy `extends` another; these fields now carry and tighten through the merge like sibling sections. (closes #1778) (#1791) - `apm install` no longer silently ignores MCP servers declared in `devDependencies.mcp`; dev MCP configs and lockfile entries now stay in sync on fresh installs. (closes #1780) (#1787) - `apm compile` now honors `managed_section` mode on distributed root `AGENTS.md` and `--single-agents` writes, preserving hand-authored