Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **Policy inheritance:** a child policy that uses `extends:` but omits `unmanaged_files:` no longer silently downgrades the parent org `action: deny`; the child block is treated as fully transparent (same semantics as allow-list `None` in merge). (#1198)
Comment thread
abhinavgautam01 marked this conversation as resolved.
Outdated
- `apm marketplace add` accepts GitLab-class hosts (`gitlab.com` and self-managed instances configured via `GITLAB_HOST` / `APM_GITLAB_HOSTS`); unsupported generic hosts now show separate recovery hints for GHES (`GITHUB_HOST`) and self-managed GitLab instead of only `GITHUB_HOST`. (#1149)
- **GitLab monorepo marketplaces:** `apm install plugin@marketplace` now resolves plugins whose sources live in a subdirectory of the marketplace repository on GitLab-class hosts (`gitlab.com` and self-managed GitLab when classified as GitLab), matching explicit `git:` + `path:` semantics without requiring that hand-written object form. (#1149)
- `apm install` now rejects unsupported flat-format `dependencies` (e.g. `dependencies: [owner/repo]`) with a clear error and structured-format hint instead of silently ignoring them; the resolver also surfaces `ValueError` from malformed transitive manifests as warnings instead of swallowing them. (#1189)
Expand Down
2 changes: 1 addition & 1 deletion src/apm_cli/commands/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def _allow_count(value: tuple | None) -> int:
"mcp_transports_allowed": _allow_count(policy.mcp.transport.allow),
"compilation_targets_allowed": _allow_count(policy.compilation.target.allow),
"manifest_required_fields": len(policy.manifest.required_fields),
"unmanaged_files_directories": len(policy.unmanaged_files.directories),
"unmanaged_files_directories": len(policy.unmanaged_files.directories or ()),
}


Expand Down
4 changes: 2 additions & 2 deletions src/apm_cli/policy/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,7 @@ def _opt_list(val: tuple[str, ...] | None) -> list | None:
},
"unmanaged_files": {
"action": policy.unmanaged_files.action,
"directories": list(policy.unmanaged_files.directories),
"directories": list(policy.unmanaged_files.directories or ()),
},
}

Expand Down 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.action in (None, "ignore")
)


Expand Down
38 changes: 34 additions & 4 deletions src/apm_cli/policy/inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,43 @@ def _merge_manifest(parent: ManifestPolicy, child: ManifestPolicy) -> ManifestPo
)


def _coerce_unmanaged_action_for_escalate(value: str | None) -> str:
"""Treat ``None`` as the weakest rung when comparing two concrete opinions."""
return "ignore" if value is None else value


def _coerce_unmanaged_directories_for_union(value: tuple[str, ...] | None) -> tuple[str, ...]:
return () if value is None else value


def _merge_unmanaged_files(
parent: UnmanagedFilesPolicy, child: UnmanagedFilesPolicy
) -> UnmanagedFilesPolicy:
return UnmanagedFilesPolicy(
action=_escalate(_UNMANAGED_ACTION_LEVELS, parent.action, child.action),
directories=_union(parent.directories, child.directories),
)
"""Merge unmanaged-files policy; omitted child block is transparent (#1198)."""
if child.action is None and child.directories is None:
return parent

if child.action is None:
eff_action_raw = parent.action
else:
eff_action_raw = _escalate(
_UNMANAGED_ACTION_LEVELS,
_coerce_unmanaged_action_for_escalate(parent.action),
child.action,
)

if child.directories is None:
eff_dirs = parent.directories
else:
eff_dirs = _union(
_coerce_unmanaged_directories_for_union(parent.directories),
child.directories,
)

eff_action = eff_action_raw if eff_action_raw is not None else "ignore"
eff_dirs_out: tuple[str, ...] = () if eff_dirs is None else eff_dirs

return UnmanagedFilesPolicy(action=eff_action, directories=eff_dirs_out)


# ---------------------------------------------------------------------------
Expand Down
17 changes: 12 additions & 5 deletions src/apm_cli/policy/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,18 @@ def _build_policy(data: dict) -> ApmPolicy:
require_explicit_includes=bool(manifest_data.get("require_explicit_includes", False)),
)

uf_data = data.get("unmanaged_files") or {}
unmanaged_files = UnmanagedFilesPolicy(
action=uf_data.get("action", UnmanagedFilesPolicy.action),
directories=_parse_tuple(uf_data.get("directories")),
)
raw_uf = data.get("unmanaged_files")
if raw_uf is None:
unmanaged_files = UnmanagedFilesPolicy(action=None, directories=None)
else:
uf_data = {} if not isinstance(raw_uf, dict) else raw_uf
action = uf_data.get("action")
directories = (
_parse_tuple(uf_data.get("directories"))
if "directories" in uf_data
Comment thread
abhinavgautam01 marked this conversation as resolved.
Outdated
else None
)
unmanaged_files = UnmanagedFilesPolicy(action=action, directories=directories)

return ApmPolicy(
name=data.get("name", "") or "",
Expand Down
2 changes: 1 addition & 1 deletion 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.action in (None, "ignore"):
return CheckResult(
name="unmanaged-files",
passed=True,
Expand Down
16 changes: 13 additions & 3 deletions src/apm_cli/policy/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,20 @@ class ManifestPolicy:

@dataclass(frozen=True)
class UnmanagedFilesPolicy:
"""Rules for files not tracked in apm.lock."""
"""Rules for files not tracked in apm.lock.

action: str = "ignore" # ignore | warn | deny
directories: tuple[str, ...] = ()
``action=None`` and ``directories=None`` together mean the policy file
expressed no ``unmanaged_files:`` section (or an empty mapping) — during
Comment thread
abhinavgautam01 marked this conversation as resolved.
Outdated
:func:`~apm_cli.policy.inheritance.merge_policies` the child is transparent
and the parent block is inherited unchanged.

When either field is set (including ``directories=()`` with a declared
``directories`` key), the merge applies escalation / union rules.
``action`` is then one of ``ignore`` | ``warn`` | ``deny``.
"""

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


@dataclass(frozen=True)
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.assertIsNone(policy.unmanaged_files.action)
self.assertIsNone(policy.unmanaged_files.directories)

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

def test_child_omitting_unmanaged_files_inherits_parent_issue_1198(self):
"""extends: child without unmanaged_files must not downgrade org deny."""
parent = ApmPolicy(
unmanaged_files=UnmanagedFilesPolicy(
action="deny",
directories=(
".github/instructions",
".github/agents",
".github/hooks",
),
),
)
child = ApmPolicy(
dependencies=DependencyPolicy(deny=("**/some-pattern",)),
unmanaged_files=UnmanagedFilesPolicy(action=None, directories=None),
)
result = merge_policies(parent, child)
self.assertEqual(result.unmanaged_files.action, "deny")
self.assertEqual(
result.unmanaged_files.directories,
(
".github/instructions",
".github/agents",
".github/hooks",
),
)


class TestResolvePolicyChain(unittest.TestCase):
"""Full chain resolution with three levels."""
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/policy/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ def test_minimal_policy(self):
self.assertIsNone(policy.dependencies.allow)
self.assertEqual(policy.dependencies.max_depth, 50)
self.assertFalse(policy.manifest.require_explicit_includes)
self.assertIsNone(policy.unmanaged_files.action)
self.assertIsNone(policy.unmanaged_files.directories)

def test_require_explicit_includes_true(self):
yaml_str = textwrap.dedent("""
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/policy/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ class TestUnmanagedFilesPolicyDefaults(unittest.TestCase):

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


class TestApmPolicyDefaults(unittest.TestCase):
Expand Down
Loading