Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `Unknown target` error suggestions no longer advertise the `agent-skills` meta-target, which `apm targets` intentionally omits from its table. The canonical set still accepts `agent-skills` via `--target` and `apm.yml`, but the recovery path printed on errors now matches what the discovery command actually lists. (#1215)
- `apm pack` no longer hardcodes `pack.target` into bundles; bundles are target-agnostic and `apm install <bundle>` resolves the consumer target from project context and wires bundle `.mcp.json` servers per target via `MCPIntegrator`. (#1217)
- Multi-account Git Credential Manager users: APM now selects the right GitHub account automatically per repository (no account-picker prompt) when `credential.useHttpPath = true` is set. Existing single-account setups are unaffected. (#1226)
- Policy inheritance now fails closed: child policies that omit `unmanaged_files` inherit the parent's action instead of silently defaulting to `ignore`. (#1253)
- Protocol-fallback port warnings are now deduplicated across parallel download workers via a threading lock, so each (host, repo, port) triple warns exactly once. (#1238)
- `apm install --global <abs-local-path>` no longer rejects absolute local paths at user scope (regression introduced by #1149); restores the post-#937 contract that only relative local paths are ambiguous at user scope. (#1247)
- Realigned the failing integration suite with current product contracts: copilot-target detection requires `.github/copilot-instructions.md` (post-#1154), `apm marketplace build` is removed in favor of `apm pack`, ADO virtual collections use the SUBDIRECTORY layout (post-#1094), and the `repo:` apm.yml key is replaced by `git:`. (#1247)
Expand Down
2 changes: 1 addition & 1 deletion src/apm_cli/policy/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,7 @@ def _is_policy_empty(policy: ApmPolicy) -> bool:
and not policy.manifest.required_fields
and policy.manifest.scripts == "allow"
and policy.manifest.content_types is None
and policy.unmanaged_files.action == "ignore"
and policy.unmanaged_files.effective_action == "ignore"
)


Expand Down
8 changes: 7 additions & 1 deletion src/apm_cli/policy/inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,14 @@ def _merge_manifest(parent: ManifestPolicy, child: ManifestPolicy) -> ManifestPo
def _merge_unmanaged_files(
parent: UnmanagedFilesPolicy, child: UnmanagedFilesPolicy
) -> UnmanagedFilesPolicy:
if child.action is None:
merged_action = parent.action
elif parent.action is None:
merged_action = child.action
else:
merged_action = _escalate(_UNMANAGED_ACTION_LEVELS, parent.action, child.action)
return UnmanagedFilesPolicy(
action=_escalate(_UNMANAGED_ACTION_LEVELS, parent.action, child.action),
action=merged_action,
directories=_union(parent.directories, child.directories),
)

Expand Down
2 changes: 1 addition & 1 deletion src/apm_cli/policy/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def _build_policy(data: dict) -> ApmPolicy:

uf_data = data.get("unmanaged_files") or {}
unmanaged_files = UnmanagedFilesPolicy(
action=uf_data.get("action", UnmanagedFilesPolicy.action),
action=uf_data.get("action"), # None when absent -> "no opinion"
directories=_parse_tuple(uf_data.get("directories")),
)

Expand Down
4 changes: 2 additions & 2 deletions src/apm_cli/policy/policy_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ def _check_unmanaged_files(
policy: UnmanagedFilesPolicy,
) -> CheckResult:
"""Check 16: no untracked files in governance directories."""
if policy.action == "ignore":
if policy.effective_action == "ignore":
return CheckResult(
name="unmanaged-files",
passed=True,
Expand Down Expand Up @@ -752,7 +752,7 @@ def _check_unmanaged_files(
message="No unmanaged files in governance directories",
)

if policy.action == "warn":
if policy.effective_action == "warn":
return CheckResult(
name="unmanaged-files",
passed=True,
Expand Down
7 changes: 6 additions & 1 deletion src/apm_cli/policy/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,14 @@ class ManifestPolicy:
class UnmanagedFilesPolicy:
"""Rules for files not tracked in apm.lock."""

action: str = "ignore" # ignore | warn | deny
action: str | None = None # None = no opinion; "ignore" | "warn" | "deny"
directories: tuple[str, ...] = ()

@property
def effective_action(self) -> str:
"""Resolved action for runtime checks (None -> 'ignore')."""
return self.action if self.action is not None else "ignore"


@dataclass(frozen=True)
class ApmPolicy:
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/policy/test_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ def test_minimal_policy_defaults(self):
self.assertEqual(policy.name, "minimal")
self.assertEqual(policy.enforcement, "warn")
self.assertEqual(policy.dependencies.require_resolution, "project-wins")
self.assertEqual(policy.unmanaged_files.action, "ignore")
self.assertEqual(policy.unmanaged_files.action, None)
self.assertEqual(policy.unmanaged_files.effective_action, "ignore")

def test_repo_override_has_extends(self):
"""Repo override fixture declares extends=org."""
Expand Down
67 changes: 67 additions & 0 deletions tests/unit/policy/test_inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,73 @@ def test_directories_dedup(self):
self.assertEqual(result.unmanaged_files.directories, (".prompts",))


class TestUnmanagedFilesTransparency(unittest.TestCase):
"""Child omitting unmanaged_files is transparent (fixes #1198)."""

def test_parent_deny_child_omits_block(self):
"""Parent deny + child omits -> merged deny."""
result = merge_policies(
ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="deny")),
ApmPolicy(), # child omits unmanaged_files entirely -> action=None
)
self.assertEqual(result.unmanaged_files.action, "deny")

def test_parent_deny_child_explicit_ignore(self):
"""Parent deny + child explicitly sets ignore -> merged deny (escalation)."""
result = merge_policies(
ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="deny")),
ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="ignore")),
)
self.assertEqual(result.unmanaged_files.action, "deny")

def test_parent_warn_child_explicit_deny(self):
"""Parent warn + child explicitly sets deny -> merged deny (child tightens)."""
result = merge_policies(
ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="warn")),
ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="deny")),
)
self.assertEqual(result.unmanaged_files.action, "deny")

def test_parent_ignore_child_omits(self):
"""Parent ignore + child omits -> merged ignore."""
result = merge_policies(
ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="ignore")),
ApmPolicy(),
)
self.assertEqual(result.unmanaged_files.action, "ignore")

def test_parent_none_child_omits(self):
"""Both omit -> merged None (no opinion)."""
result = merge_policies(
ApmPolicy(),
ApmPolicy(),
)
self.assertIsNone(result.unmanaged_files.action)
self.assertEqual(result.unmanaged_files.effective_action, "ignore")

def test_directories_inherited_when_child_omits(self):
"""Parent directories preserved when child omits the block."""
result = merge_policies(
ApmPolicy(
unmanaged_files=UnmanagedFilesPolicy(action="deny", directories=(".github", "docs"))
),
ApmPolicy(), # child omits
)
self.assertEqual(result.unmanaged_files.action, "deny")
self.assertEqual(sorted(result.unmanaged_files.directories), [".github", "docs"])

def test_three_level_chain_transparency(self):
"""Enterprise deny -> org omits -> repo omits -> deny preserved."""
result = resolve_policy_chain(
[
ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="deny")),
ApmPolicy(), # org omits
ApmPolicy(), # repo omits
]
)
self.assertEqual(result.unmanaged_files.action, "deny")


class TestResolvePolicyChain(unittest.TestCase):
"""Full chain resolution with three levels."""

Expand Down
7 changes: 7 additions & 0 deletions tests/unit/policy/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,13 @@ def test_long_yaml_string_does_not_crash(self):
self.assertEqual(policy.version, "1.0")
self.assertEqual(policy.enforcement, "off")

def test_omitted_unmanaged_files_yields_none_action(self):
"""Absent unmanaged_files block -> action is None (no opinion)."""
policy, _ = load_policy("name: test\nenforcement: warn\n")
self.assertIsNone(policy.unmanaged_files.action)
self.assertEqual(policy.unmanaged_files.effective_action, "ignore")
self.assertEqual(policy.unmanaged_files.directories, ())


class TestLoadPolicyFromFile(unittest.TestCase):
"""Test load_policy from a file path."""
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/policy/test_policy_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import yaml

from apm_cli.models.apm_package import clear_apm_yml_cache
from apm_cli.policy.inheritance import merge_policies
from apm_cli.policy.models import CheckResult, CIAuditResult # noqa: F401
from apm_cli.policy.policy_checks import (
_check_compilation_strategy,
Expand Down Expand Up @@ -653,6 +654,53 @@ def test_rglob_cap_skips_check(self, tmp_path, monkeypatch):
assert result.passed
assert "capped" in result.message.lower()

def test_action_none_resolves_to_ignore(self, tmp_path):
"""action=None standalone: effective_action resolves to 'ignore', check passes."""
policy = UnmanagedFilesPolicy(action=None)
result = _check_unmanaged_files(tmp_path, None, policy)
assert result.passed

def test_action_none_inherits_parent_deny(self, tmp_path):
"""action=None + parent deny: merge propagates deny to child."""
(tmp_path / ".github" / "agents").mkdir(parents=True)
(tmp_path / ".github" / "agents" / "rogue.md").write_text("rogue", encoding="utf-8")
parent = ApmPolicy(
unmanaged_files=UnmanagedFilesPolicy(action="deny", directories=[".github/agents"])
)
child = ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action=None))
merged = merge_policies(parent, child)
lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}])
result = _check_unmanaged_files(tmp_path, lock, merged.unmanaged_files)
assert not result.passed

def test_rogue_file_caught_parent_deny_child_omits_block(self, tmp_path):
"""Rogue file is caught when parent says deny but child omits unmanaged_files block."""
(tmp_path / ".github" / "agents").mkdir(parents=True)
(tmp_path / ".github" / "agents" / "rogue.md").write_text("rogue", encoding="utf-8")
parent = ApmPolicy(
unmanaged_files=UnmanagedFilesPolicy(action="deny", directories=[".github/agents"])
)
child = ApmPolicy() # child omits unmanaged_files entirely (defaults to action=None)
merged = merge_policies(parent, child)
lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}])
result = _check_unmanaged_files(tmp_path, lock, merged.unmanaged_files)
assert not result.passed
assert ".github/agents/rogue.md" in result.details

def test_integration_chain_deny_propagates(self, tmp_path):
"""Integration chain: parent deny + child omits block -> merge -> check catches rogue file."""
(tmp_path / ".github" / "agents").mkdir(parents=True)
(tmp_path / ".github" / "agents" / "rogue.md").write_text("rogue", encoding="utf-8")
parent_policy = ApmPolicy(
unmanaged_files=UnmanagedFilesPolicy(action="deny", directories=[".github/agents"])
)
child_policy = ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action=None))
merged = merge_policies(parent_policy, child_policy)
lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}])
result = _check_unmanaged_files(tmp_path, lock, merged.unmanaged_files)
assert not result.passed
assert ".github/agents/rogue.md" in result.details


# -- Integration: run_policy_checks ---------------------------------

Expand Down
3 changes: 2 additions & 1 deletion tests/unit/policy/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ class TestUnmanagedFilesPolicyDefaults(unittest.TestCase):

def test_defaults(self):
uf = UnmanagedFilesPolicy()
self.assertEqual(uf.action, "ignore")
self.assertIsNone(uf.action)
self.assertEqual(uf.effective_action, "ignore")
self.assertEqual(uf.directories, ())


Expand Down
Loading