diff --git a/CHANGELOG.md b/CHANGELOG.md index fd77deecb..1695510ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **ADO Entra ID auth path no longer silently fails.** Bearer tokens from `az account get-access-token` are now correctly plumbed through validation (auth scheme, git env). Auth failures raise a typed `AuthenticationError` with an actionable 4-case diagnostic instead of the ambiguous "not accessible or doesn't exist" message. `apm install --update` runs a pre-flight auth check before modifying any files -- on failure it aborts with "No files were modified". (#1015) - Correct targeting of compiled artifacts so GEMINI.md is only created if requested (#1019) +### Security + +- `apm audit --ci` and `apm install` now fail-closed when `apm.yml` is malformed YAML or not a mapping -- previously, policy and baseline checks were silently skipped (severity: medium -- policy bypass). **Migration:** repos with latently malformed `apm.yml` will go from CI-pass to CI-fail on upgrade. Validate before upgrading with `python -c "import yaml; yaml.safe_load(open('apm.yml'))"` or run `apm audit --ci` locally. Fix any YAML syntax errors in `apm.yml` (stray tabs, unquoted colons, non-mapping root). (#936) + ## [0.10.0] - 2026-04-27 ### Added diff --git a/docs/src/content/docs/enterprise/governance-guide.md b/docs/src/content/docs/enterprise/governance-guide.md index 9002353d6..2f8bf8450 100644 --- a/docs/src/content/docs/enterprise/governance-guide.md +++ b/docs/src/content/docs/enterprise/governance-guide.md @@ -351,9 +351,10 @@ Workarounds when the network is unreliable: |---|---|---|---| | Network failure (`cache_miss_fetch_fail`) | Fail-OPEN, log warning, install proceeds with no policy | `policy.fetch_failure_default: block` in `apm.yml` | [policy-reference#95-network-failure-semantics](../policy-reference/#95-network-failure-semantics) | | Cached stale (1h - 7d, refresh failed) | Warn and proceed with cached policy | `policy.fetch_failure: block` set in the cached policy itself | [policy-reference#95-network-failure-semantics](../policy-reference/#95-network-failure-semantics) | -| Malformed YAML (`malformed`) | Fail-OPEN by default | `policy.fetch_failure_default: block` | `policy/parser.py` | +| Malformed YAML (`malformed`) (org policy file) | Fail-OPEN by default | `policy.fetch_failure_default: block` | `policy/parser.py` | | Hash-mismatch (project pin vs fetched) | **Always fail-CLOSED** | n/a (cannot be relaxed) | [policy-reference#95-network-failure-semantics](../policy-reference/#95-network-failure-semantics) | | Garbage response | Fail-OPEN by default | `policy.fetch_failure_default: block` | [policy-reference#95-network-failure-semantics](../policy-reference/#95-network-failure-semantics) | +| Malformed project manifest (`manifest_parse`) | **Always fail-CLOSED** | n/a (cannot be relaxed) | `policy/policy_checks.py`, `policy/ci_checks.py` | | `extends:` cycle detected | Fail-CLOSED, raises `PolicyInheritanceError` | n/a | `policy/inheritance.py` | | Cross-host `extends:` rejected | Fail-CLOSED, raises before any fetch | n/a (security mitigation, cannot be relaxed) | `policy/discovery.py` | diff --git a/docs/src/content/docs/enterprise/policy-reference.md b/docs/src/content/docs/enterprise/policy-reference.md index 4ff52107b..ed1660df4 100644 --- a/docs/src/content/docs/enterprise/policy-reference.md +++ b/docs/src/content/docs/enterprise/policy-reference.md @@ -818,6 +818,7 @@ Each row maps a `PolicyFetchResult.outcome` to its exit impact, severity, the me | `cache_miss_fetch_fail` | `0` | warn | `Could not fetch org policy () -- policy enforcement skipped for this invocation` | Retry, check VPN/firewall/`gh auth status`/`GITHUB_APM_PAT`. Fail-open by design (CEO-ratified); CI will still fail for the same violation. | | `garbage_response` | `0` | warn | `Could not fetch org policy (invalid YAML body from ) -- policy enforcement skipped for this invocation` | Likely a captive portal or auth wall returning HTML. Restore direct connectivity, then re-run. | | `malformed` | `0` (no enforcement) | warn | `Policy at is malformed -- contact your org admin to fix the policy file` | Contact org admin to fix the YAML. Validate locally with `apm audit --ci --policy `. | +| `manifest-parse` | `1` (always) | error | `Cannot parse apm.yml: ` | Fix the YAML syntax error in `apm.yml`. This is a local audit check (not a fetch outcome) -- malformed manifests always fail the audit unconditionally. | | `disabled` | `0` | warn | `Policy enforcement disabled by --no-policy for this invocation. This does NOT bypass apm audit --ci. CI will still fail the PR for the same policy violation.` | Single-invocation only. Drop the flag / env var to re-enable. | | `no_git_remote` | `0` | warn | `Could not determine org from git remote; policy auto-discovery skipped` | Run inside a checkout with a GitHub remote, or set the remote with `git remote add origin `. | | `empty` | `0` | warn | `Org policy is present but empty; no enforcement applied` | Org admin should populate the policy file (see section 11) or remove it. | diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index a0eace24f..fffa3c2b4 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -205,6 +205,7 @@ Symlinks are never followed during file discovery or artifact operations: - **Tree copy operations** skip symlinks entirely -- they are excluded from the copy via an ignore filter. - **MCP configuration files** that are symlinks are rejected with a warning and not parsed. - **Manifest parsing** requires files to pass both `.is_file()` and `not .is_symlink()` checks. +- **Manifest integrity** -- a malformed `apm.yml` (invalid YAML or non-mapping content) triggers a failing `manifest-parse` audit check. Policy and baseline CI checks never silently pass when the manifest cannot be parsed. If this check fires, fix the YAML syntax error in your `apm.yml` and re-run the audit. - **Archive creation** -- `apm pack` excludes symlinks from bundled archives. Packaged artifacts contain no symbolic links, preventing symlink-based escape attacks in distributed bundles. This prevents symlink-based attacks that could escape allowed directories or cause APM to read or write outside the project root. diff --git a/packages/apm-guide/.apm/skills/apm-usage/governance.md b/packages/apm-guide/.apm/skills/apm-usage/governance.md index 961f47c8b..a4068f535 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/governance.md +++ b/packages/apm-guide/.apm/skills/apm-usage/governance.md @@ -354,6 +354,12 @@ Discovery outcomes APM can emit (see `PolicyFetchResult.outcome`): are fail-open by default and become fail-closed when the project opts in via `policy.fetch_failure_default: block`. +A malformed project manifest (`apm.yml`) is a separate concern from a +malformed policy file. When `apm.yml` cannot be parsed (invalid YAML or +non-mapping content), both `run_policy_checks()` and +`run_baseline_checks()` produce a failing `manifest-parse` check. This +is unconditionally fail-closed and cannot be relaxed. + Violation classes: | Class | Triggers | Remediation | diff --git a/src/apm_cli/policy/ci_checks.py b/src/apm_cli/policy/ci_checks.py index a7e0345ef..6e1f4fe8a 100644 --- a/src/apm_cli/policy/ci_checks.py +++ b/src/apm_cli/policy/ci_checks.py @@ -12,19 +12,29 @@ from __future__ import annotations +import logging from pathlib import Path -from typing import List +from typing import List, Optional from .models import CIAuditResult, CheckResult from ..deps.lockfile import _SELF_KEY +_logger = logging.getLogger(__name__) + # -- Individual checks --------------------------------------------- -def _check_lockfile_exists(project_root: Path) -> CheckResult: +def _check_lockfile_exists( + project_root: Path, + manifest: Optional["APMPackage"], +) -> CheckResult: """Check that ``apm.lock.yaml`` is present when relevant. + Receives the already-parsed manifest from :func:`run_baseline_checks` + (``None`` when no ``apm.yml`` exists on disk). This function never + parses ``apm.yml`` itself and always returns ``name="lockfile-exists"``. + Relevance is determined by either: * the manifest declaring APM/MCP dependencies, or * a lockfile already on disk recording local-only content @@ -32,25 +42,13 @@ def _check_lockfile_exists(project_root: Path) -> CheckResult: """ from ..deps.lockfile import LockFile, get_lockfile_path - apm_yml_path = project_root / "apm.yml" - if not apm_yml_path.exists(): + if manifest is None: return CheckResult( name="lockfile-exists", passed=True, message="No apm.yml found -- nothing to check", ) - from ..models.apm_package import APMPackage - - try: - manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, FileNotFoundError): - return CheckResult( - name="lockfile-exists", - passed=True, - message="Could not parse apm.yml -- skipping lockfile check", - ) - has_deps = manifest.has_apm_dependencies() or bool(manifest.get_mcp_dependencies()) lockfile_path = get_lockfile_path(project_root) @@ -63,8 +61,8 @@ def _check_lockfile_exists(project_root: Path) -> CheckResult: lock_for_gating = LockFile.read(lockfile_path) if lock_for_gating is not None and lock_for_gating.local_deployed_files: has_deps = True - except Exception: - pass # fall through; if unreadable the missing-lockfile branch warns + except Exception as exc: + _logger.debug("Could not read lockfile for gating: %s", exc) if not has_deps: return CheckResult( @@ -431,15 +429,33 @@ def run_baseline_checks( from ..models.apm_package import APMPackage, clear_apm_yml_cache result = CIAuditResult() + apm_yml_path = project_root / "apm.yml" + + # Parse manifest ONCE -- this function owns parse-error handling. + manifest = None + if apm_yml_path.exists(): + import yaml - # Check 1: Lockfile exists - result.checks.append(_check_lockfile_exists(project_root)) + try: + clear_apm_yml_cache() + manifest = APMPackage.from_apm_yml(apm_yml_path) + except (ValueError, yaml.YAMLError, OSError) as exc: + result.checks.append( + CheckResult( + name="manifest-parse", + passed=False, + message="Cannot parse apm.yml: %s -- fix the YAML syntax error in apm.yml and re-run." % exc, + ) + ) + return result + + # Check 1: Lockfile exists (manifest already parsed, pass it in) + result.checks.append(_check_lockfile_exists(project_root, manifest)) # If lockfile doesn't exist or isn't needed, remaining checks can't run if not result.checks[0].passed: return result - apm_yml_path = project_root / "apm.yml" lockfile_path = get_lockfile_path(project_root) # If there's no apm.yml or no lockfile, the first check already passed @@ -447,12 +463,6 @@ def run_baseline_checks( if not apm_yml_path.exists() or not lockfile_path.exists(): return result - try: - clear_apm_yml_cache() - manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, FileNotFoundError): - return result - lock = LockFile.read(lockfile_path) if lock is None: return result diff --git a/src/apm_cli/policy/models.py b/src/apm_cli/policy/models.py index b9fbf914e..3bcd5227d 100644 --- a/src/apm_cli/policy/models.py +++ b/src/apm_cli/policy/models.py @@ -34,6 +34,7 @@ "required-manifest-fields": "apm.yml", "scripts-policy": "apm.yml", "unmanaged-files": "apm.yml", + "manifest-parse": "apm.yml", } diff --git a/src/apm_cli/policy/policy_checks.py b/src/apm_cli/policy/policy_checks.py index 39dbee595..2e3847824 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -10,14 +10,34 @@ from pathlib import Path from typing import List, Optional +import logging + from .models import CIAuditResult, CheckResult +_logger = logging.getLogger(__name__) + # -- Helpers ------------------------------------------------------- def _load_raw_apm_yml(project_root: Path) -> Optional[dict]: - """Load raw apm.yml as a dict for policy checks that inspect raw fields.""" + """Load raw apm.yml as a dict for policy checks that inspect raw fields. + + This helper is called **after** :pymethod:`APMPackage.from_apm_yml` has + already succeeded in :func:`run_policy_checks`. The primary security + gate is ``from_apm_yml()`` -- if it fails, the audit aborts with a + ``manifest-parse`` check result and this function is never reached. + + Returning ``None`` here is therefore **defence-in-depth**: it covers + edge cases (TOCTOU race, transient I/O error) where the file becomes + unreadable between the two calls. Callers that receive ``None`` + gracefully skip supplementary raw-field checks (e.g. + ``compilation-target``, ``extensions-present``) rather than hard-failing. + + Returns ``None`` when the file is absent, unreadable, malformed YAML, + or not a mapping -- but logs a warning so the failure is visible + rather than silently swallowed. + """ import yaml apm_yml_path = project_root / "apm.yml" @@ -26,9 +46,25 @@ def _load_raw_apm_yml(project_root: Path) -> Optional[dict]: try: with open(apm_yml_path, "r", encoding="utf-8") as f: data = yaml.safe_load(f) - return data if isinstance(data, dict) else None - except Exception: + except FileNotFoundError: + # TOCTOU: file disappeared between exists() check and open(); normal condition. + return None + except yaml.YAMLError as exc: + _logger.warning("Malformed YAML in %s: %s", apm_yml_path, exc) return None + except OSError as exc: + _logger.warning("Cannot read %s: %s", apm_yml_path, exc) + return None + except UnicodeDecodeError as exc: + _logger.warning("Cannot decode %s as UTF-8: %s", apm_yml_path, exc) + return None + if not isinstance(data, dict): + _logger.warning( + "apm.yml is not a YAML mapping (got %s) -- skipping raw-field checks", + type(data).__name__, + ) + return None + return data # -- Individual policy checks -------------------------------------- @@ -930,10 +966,19 @@ def run_policy_checks( if not apm_yml_path.exists(): return result + import yaml + try: clear_apm_yml_cache() manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, FileNotFoundError): + except (ValueError, yaml.YAMLError, OSError) as exc: + result.checks.append( + CheckResult( + name="manifest-parse", + passed=False, + message="Cannot parse apm.yml: %s -- fix the YAML syntax error in apm.yml and re-run." % exc, + ) + ) return result # Load lockfile (optional -- some checks work without it) diff --git a/tests/unit/policy/test_ci_checks.py b/tests/unit/policy/test_ci_checks.py index 21513b629..34e6fe9ce 100644 --- a/tests/unit/policy/test_ci_checks.py +++ b/tests/unit/policy/test_ci_checks.py @@ -17,12 +17,21 @@ run_baseline_checks, ) from apm_cli.policy.models import CIAuditResult, CheckResult -from apm_cli.models.apm_package import clear_apm_yml_cache +from apm_cli.models.apm_package import APMPackage, clear_apm_yml_cache # -- Helpers -------------------------------------------------------- +def _parse_manifest(project: Path): + """Parse apm.yml and return the manifest, or ``None`` if absent.""" + apm_yml = project / "apm.yml" + if not apm_yml.exists(): + return None + clear_apm_yml_cache() + return APMPackage.from_apm_yml(apm_yml) + + def _write_apm_yml(project: Path, *, deps: list[str] | None = None, mcp: list | None = None) -> None: """Write a minimal apm.yml with optional dependencies.""" lines = ["name: test-project", "version: '1.0.0'"] @@ -86,25 +95,28 @@ def test_pass_lockfile_present(self, tmp_path): resolved_ref: main """), ) - result = _check_lockfile_exists(tmp_path) + manifest = _parse_manifest(tmp_path) + result = _check_lockfile_exists(tmp_path, manifest) assert result.passed assert result.name == "lockfile-exists" def test_fail_lockfile_missing(self, tmp_path): _write_apm_yml(tmp_path, deps=["owner/repo"]) - result = _check_lockfile_exists(tmp_path) + manifest = _parse_manifest(tmp_path) + result = _check_lockfile_exists(tmp_path, manifest) assert not result.passed assert "missing" in result.message.lower() assert len(result.details) > 0 def test_pass_no_deps_no_lockfile(self, tmp_path): _write_apm_yml(tmp_path) # no deps - result = _check_lockfile_exists(tmp_path) + manifest = _parse_manifest(tmp_path) + result = _check_lockfile_exists(tmp_path, manifest) assert result.passed assert "not required" in result.message.lower() def test_pass_no_apm_yml(self, tmp_path): - result = _check_lockfile_exists(tmp_path) + result = _check_lockfile_exists(tmp_path, None) assert result.passed @@ -754,7 +766,7 @@ def test_lockfile_exists_passes_when_local_content_recorded(self, tmp_path): - .github/prompts/local.prompt.md """), ) - result = _check_lockfile_exists(tmp_path) + result = _check_lockfile_exists(tmp_path, _parse_manifest(tmp_path)) assert result.passed assert "lockfile present" in result.message.lower() # Must NOT have been short-circuited as "no dependencies declared" @@ -765,11 +777,9 @@ def test_lockfile_exists_still_passes_when_no_deps_and_no_local(self, tmp_path): returns the 'no dependencies declared' fast-path (no false fail).""" _write_apm_yml(tmp_path) # no deps # No lockfile on disk at all. - result = _check_lockfile_exists(tmp_path) + result = _check_lockfile_exists(tmp_path, _parse_manifest(tmp_path)) assert result.passed assert "not required" in result.message.lower() - - def test_aggregate_runs_deployed_files_check_for_local_only_repo(self, tmp_path): """(c) Aggregate must NOT short-circuit before deployed-files-present runs against the synthesized self-entry.""" # File declared in lockfile but missing on disk -> deployed check fails. @@ -1021,3 +1031,97 @@ def test_aggregate_runner_includes_consent_check_last(self, tmp_path): consent = next(c for c in result.checks if c.name == "includes-consent") assert consent.passed assert "consider adding 'includes: auto'" in consent.message + + +# -- Group 3: _check_lockfile_exists contract tests ---------------- + + +class TestCheckLockfileExistsContract: + """_check_lockfile_exists must ALWAYS return name='lockfile-exists'. + + After the refactor (fix #936), manifest parsing is hoisted into + run_baseline_checks. _check_lockfile_exists receives the already-parsed + manifest and never emits 'manifest-parse'. + """ + + def test_none_manifest_returns_lockfile_exists(self, tmp_path: Path) -> None: + """When manifest is None (no apm.yml), returns lockfile-exists pass.""" + check = _check_lockfile_exists(tmp_path, None) + assert check.name == "lockfile-exists" + assert check.passed + assert "No apm.yml" in check.message + + def test_valid_manifest_no_lockfile_returns_lockfile_exists(self, tmp_path: Path) -> None: + """When manifest has deps but no lockfile, returns lockfile-exists fail.""" + _write_apm_yml(tmp_path, deps=["owner/repo"]) + manifest = _parse_manifest(tmp_path) + check = _check_lockfile_exists(tmp_path, manifest) + assert check.name == "lockfile-exists" + assert not check.passed + + def test_valid_manifest_with_lockfile_returns_lockfile_exists(self, tmp_path: Path) -> None: + """When manifest has deps and lockfile present, returns lockfile-exists pass.""" + _write_apm_yml(tmp_path, deps=["owner/repo"]) + _write_lockfile( + tmp_path, + textwrap.dedent("""\ + lockfile_version: '1' + generated_at: '2025-01-01T00:00:00Z' + dependencies: + - repo_url: owner/repo + resolved_ref: main + """), + ) + manifest = _parse_manifest(tmp_path) + check = _check_lockfile_exists(tmp_path, manifest) + assert check.name == "lockfile-exists" + assert check.passed + + def test_no_deps_manifest_returns_lockfile_exists(self, tmp_path: Path) -> None: + """When manifest has no deps, returns lockfile-exists pass.""" + _write_apm_yml(tmp_path) # no deps + manifest = _parse_manifest(tmp_path) + check = _check_lockfile_exists(tmp_path, manifest) + assert check.name == "lockfile-exists" + assert check.passed + + +# -- Group 4: run_baseline_checks malformed-manifest tests --------- + + +class TestRunBaselineChecksMalformedManifest: + """run_baseline_checks must fail-closed on malformed apm.yml (fix #936).""" + + def test_malformed_yaml_produces_failing_check(self, tmp_path: Path) -> None: + """Malformed YAML is caught by the single parse block in + run_baseline_checks and returned as manifest-parse failure.""" + (tmp_path / "apm.yml").write_text(": :\n bad: [yaml\n", encoding="utf-8") + clear_apm_yml_cache() + result = run_baseline_checks(tmp_path) + assert not result.passed + parse_checks = [c for c in result.checks if c.name == "manifest-parse"] + assert len(parse_checks) == 1 + assert not parse_checks[0].passed + assert "fix the YAML syntax error" in parse_checks[0].message + + def test_non_dict_yaml_produces_failing_check(self, tmp_path: Path) -> None: + """Non-dict YAML (bare list) propagates as manifest-parse failure.""" + (tmp_path / "apm.yml").write_text("- item1\n- item2\n", encoding="utf-8") + clear_apm_yml_cache() + result = run_baseline_checks(tmp_path) + assert not result.passed + parse_checks = [c for c in result.checks if c.name == "manifest-parse"] + assert len(parse_checks) == 1 + assert not parse_checks[0].passed + assert "fix the YAML syntax error" in parse_checks[0].message + + def test_remediation_hint_present_in_error_message(self, tmp_path: Path) -> None: + """The manifest-parse error message includes a remediation hint + guiding users to fix the YAML and re-run.""" + (tmp_path / "apm.yml").write_text(": :\n bad: [yaml\n", encoding="utf-8") + clear_apm_yml_cache() + result = run_baseline_checks(tmp_path) + parse_check = result.checks[0] + assert parse_check.name == "manifest-parse" + assert "Cannot parse apm.yml" in parse_check.message + assert "fix the YAML syntax error in apm.yml and re-run" in parse_check.message diff --git a/tests/unit/policy/test_policy_checks.py b/tests/unit/policy/test_policy_checks.py index 57dc81515..d6bdf75c6 100644 --- a/tests/unit/policy/test_policy_checks.py +++ b/tests/unit/policy/test_policy_checks.py @@ -28,6 +28,7 @@ _check_source_attribution, _check_transitive_depth, _check_unmanaged_files, + _load_raw_apm_yml, run_policy_checks, ) from apm_cli.policy.schema import ( @@ -851,3 +852,138 @@ def test_multiple_failures(self, tmp_path): assert "dependency-denylist" in failed_names assert "scripts-policy" in failed_names assert "required-manifest-fields" in failed_names + + +# -- Group 1: _load_raw_apm_yml malformed-manifest tests ----------- + + +class TestLoadRawApmYml: + """Tests for _load_raw_apm_yml with malformed content (fix #936).""" + + def test_malformed_yaml_returns_none_and_logs( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """Malformed YAML returns None and logs a WARNING.""" + (tmp_path / "apm.yml").write_text(": :\n bad: [yaml\n", encoding="utf-8") + import logging + + with caplog.at_level(logging.WARNING, logger="apm_cli.policy.policy_checks"): + result = _load_raw_apm_yml(tmp_path) + assert result is None + assert "Malformed YAML" in caplog.text + + def test_non_dict_yaml_returns_none_and_logs( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """Non-mapping YAML (bare list) returns None and logs a WARNING.""" + (tmp_path / "apm.yml").write_text("- item1\n- item2\n", encoding="utf-8") + import logging + + with caplog.at_level(logging.WARNING, logger="apm_cli.policy.policy_checks"): + result = _load_raw_apm_yml(tmp_path) + assert result is None + assert "not a YAML mapping" in caplog.text + + def test_scalar_yaml_returns_none_and_logs( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """Plain scalar YAML returns None and logs a WARNING.""" + (tmp_path / "apm.yml").write_text("just a string\n", encoding="utf-8") + import logging + + with caplog.at_level(logging.WARNING, logger="apm_cli.policy.policy_checks"): + result = _load_raw_apm_yml(tmp_path) + assert result is None + assert "not a YAML mapping" in caplog.text + + def test_empty_file_returns_none_and_logs( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + """Empty apm.yml (yaml.safe_load returns None) is not a dict.""" + (tmp_path / "apm.yml").write_text("", encoding="utf-8") + import logging + + with caplog.at_level(logging.WARNING, logger="apm_cli.policy.policy_checks"): + result = _load_raw_apm_yml(tmp_path) + assert result is None + assert "not a YAML mapping" in caplog.text + + def test_missing_file_returns_none_silently(self, tmp_path: Path) -> None: + """Missing apm.yml returns None without raising or logging.""" + result = _load_raw_apm_yml(tmp_path) + assert result is None + + def test_valid_yaml_returns_dict(self, tmp_path: Path) -> None: + """Valid YAML mapping is returned as a dict unchanged.""" + (tmp_path / "apm.yml").write_text( + "name: test\nversion: '1.0'\n", encoding="utf-8" + ) + result = _load_raw_apm_yml(tmp_path) + assert result == {"name": "test", "version": "1.0"} + + +# -- Group 2: run_policy_checks malformed-manifest tests ----------- + + +class TestRunPolicyChecksMalformedManifest: + """run_policy_checks must fail-closed on malformed apm.yml (fix #936).""" + + def test_malformed_yaml_produces_failing_check(self, tmp_path: Path) -> None: + """Malformed YAML appends manifest-parse check with passed=False.""" + (tmp_path / "apm.yml").write_text(": :\n bad: [yaml\n", encoding="utf-8") + clear_apm_yml_cache() + policy = ApmPolicy() + result = run_policy_checks(tmp_path, policy) + assert not result.passed + parse_checks = [c for c in result.checks if c.name == "manifest-parse"] + assert len(parse_checks) == 1 + assert not parse_checks[0].passed + assert "Cannot parse apm.yml" in parse_checks[0].message + + def test_non_dict_yaml_produces_failing_check(self, tmp_path: Path) -> None: + """Non-dict YAML (bare list) triggers a manifest-parse failing check.""" + (tmp_path / "apm.yml").write_text("- item1\n- item2\n", encoding="utf-8") + clear_apm_yml_cache() + policy = ApmPolicy() + result = run_policy_checks(tmp_path, policy) + assert not result.passed + parse_checks = [c for c in result.checks if c.name == "manifest-parse"] + assert len(parse_checks) == 1 + assert not parse_checks[0].passed + + def test_empty_file_produces_failing_check(self, tmp_path: Path) -> None: + """Empty apm.yml triggers ValueError from from_apm_yml.""" + (tmp_path / "apm.yml").write_text("", encoding="utf-8") + clear_apm_yml_cache() + policy = ApmPolicy() + result = run_policy_checks(tmp_path, policy) + assert not result.passed + parse_checks = [c for c in result.checks if c.name == "manifest-parse"] + assert len(parse_checks) == 1 + assert not parse_checks[0].passed + + def test_missing_apm_yml_returns_empty_passing_result( + self, tmp_path: Path + ) -> None: + """No apm.yml means nothing to check -- result is empty and passes.""" + clear_apm_yml_cache() + policy = ApmPolicy() + result = run_policy_checks(tmp_path, policy) + assert result.passed + assert len(result.checks) == 0 + + # -- Group 5 regression guard -- + + def test_malformed_yaml_does_not_silently_pass(self, tmp_path: Path) -> None: + """Regression: malformed YAML must NOT produce an empty passing result. + + Before fix #936, malformed apm.yml returned CIAuditResult() with + no checks, which trivially passed (all([]) is True). + """ + (tmp_path / "apm.yml").write_text("{{invalid yaml}}\n", encoding="utf-8") + clear_apm_yml_cache() + policy = ApmPolicy() + result = run_policy_checks(tmp_path, policy) + assert not result.passed, ( + "Malformed apm.yml must not silently pass policy checks" + )