From 704e3d7acabd39ed1a94df2fa7c0bdc167a6ea45 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Wed, 29 Apr 2026 19:34:45 +0100 Subject: [PATCH 1/3] fix(policy): fail closed on malformed manifest YAML (#936) Replace silent bypass with failing manifest-parse CheckResult when apm.yml cannot be parsed. Fixes four code paths: - run_policy_checks(): catch (ValueError, yaml.YAMLError) instead of silently returning empty CIAuditResult - run_baseline_checks(): same pattern in ci_checks.py - _check_lockfile_exists(): return passed=False instead of passed=True - _load_raw_apm_yml(): replace bare except Exception with yaml.YAMLError and OSError, log WARNING on parse failure Add manifest-parse to _CHECK_ARTIFACT_MAP for SARIF output. 14 new tests cover malformed YAML, non-dict YAML, scalar YAML, and regression guards. 522 policy tests pass, 6744 total unit tests pass. Update security.md, policy-reference.md, governance-guide.md, governance.md skill resource, and CHANGELOG. Closes #936 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 + .../docs/enterprise/governance-guide.md | 1 + .../docs/enterprise/policy-reference.md | 1 + docs/src/content/docs/enterprise/security.md | 1 + .../.apm/skills/apm-usage/governance.md | 6 + src/apm_cli/policy/ci_checks.py | 21 ++- src/apm_cli/policy/models.py | 1 + src/apm_cli/policy/policy_checks.py | 36 ++++- tests/unit/policy/test_ci_checks.py | 99 +++++++++++++ tests/unit/policy/test_policy_checks.py | 136 ++++++++++++++++++ 10 files changed, 297 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd77deecb..17b77d3e9 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. (#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..e113afa1d 100644 --- a/docs/src/content/docs/enterprise/governance-guide.md +++ b/docs/src/content/docs/enterprise/governance-guide.md @@ -354,6 +354,7 @@ Workarounds when the network is unreliable: | Malformed YAML (`malformed`) | 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..b4c7cefef 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 check is unconditional -- malformed manifests always fail the audit. | | `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..0306e546b 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. - **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..288e0e9b1 100644 --- a/src/apm_cli/policy/ci_checks.py +++ b/src/apm_cli/policy/ci_checks.py @@ -40,15 +40,17 @@ def _check_lockfile_exists(project_root: Path) -> CheckResult: message="No apm.yml found -- nothing to check", ) + import yaml + from ..models.apm_package import APMPackage try: manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, FileNotFoundError): + except (ValueError, yaml.YAMLError) as exc: return CheckResult( - name="lockfile-exists", - passed=True, - message="Could not parse apm.yml -- skipping lockfile check", + name="manifest-parse", + passed=False, + message="Cannot parse apm.yml: %s" % exc, ) has_deps = manifest.has_apm_dependencies() or bool(manifest.get_mcp_dependencies()) @@ -447,10 +449,19 @@ def run_baseline_checks( if not apm_yml_path.exists() or not lockfile_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) as exc: + result.checks.append( + CheckResult( + name="manifest-parse", + passed=False, + message="Cannot parse apm.yml: %s" % exc, + ) + ) return result lock = LockFile.read(lockfile_path) 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..96d0a7f03 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -10,14 +10,23 @@ 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. + + Returns ``None`` when the file is absent, unreadable, malformed YAML, + or not a mapping -- but now logs a warning so the failure is visible + rather than silently swallowed. + """ import yaml apm_yml_path = project_root / "apm.yml" @@ -26,9 +35,19 @@ 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 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 + 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 +949,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) as exc: + result.checks.append( + CheckResult( + name="manifest-parse", + passed=False, + message="Cannot parse apm.yml: %s" % 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..16df6cf09 100644 --- a/tests/unit/policy/test_ci_checks.py +++ b/tests/unit/policy/test_ci_checks.py @@ -1021,3 +1021,102 @@ 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 malformed-manifest tests ------ + + +class TestCheckLockfileExistsMalformedManifest: + """_check_lockfile_exists must fail-closed on malformed apm.yml (fix #936).""" + + def test_malformed_yaml_returns_failing_check(self, tmp_path: Path) -> None: + """Malformed YAML produces manifest-parse check with passed=False.""" + (tmp_path / "apm.yml").write_text(": :\n bad: [yaml\n", encoding="utf-8") + check = _check_lockfile_exists(tmp_path) + assert check.name == "manifest-parse" + assert not check.passed + assert "Cannot parse apm.yml" in check.message + + def test_non_dict_yaml_returns_failing_check(self, tmp_path: Path) -> None: + """Non-dict YAML (bare list) produces manifest-parse failure.""" + (tmp_path / "apm.yml").write_text("- item1\n- item2\n", encoding="utf-8") + check = _check_lockfile_exists(tmp_path) + assert check.name == "manifest-parse" + assert not check.passed + assert "Cannot parse apm.yml" in check.message + + def test_empty_file_returns_failing_check(self, tmp_path: Path) -> None: + """Empty apm.yml produces manifest-parse failure.""" + (tmp_path / "apm.yml").write_text("", encoding="utf-8") + check = _check_lockfile_exists(tmp_path) + assert check.name == "manifest-parse" + assert not check.passed + + def test_binary_content_returns_failing_check(self, tmp_path: Path) -> None: + """Binary content in apm.yml raises ReaderError (subclass of YAMLError).""" + (tmp_path / "apm.yml").write_bytes(b"\x89PNG\r\n\x1a\n") + check = _check_lockfile_exists(tmp_path) + assert check.name == "manifest-parse" + assert not 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 causes check 1 (_check_lockfile_exists) to return + a manifest-parse failure, which propagates through run_baseline_checks.""" + (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 + + 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 + + def test_second_parse_catch_with_mocked_check1( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Test the run_baseline_checks manifest-parse catch after check 1 passes. + + _check_lockfile_exists now fails on malformed YAML, so we monkeypatch it + to pass in order to exercise the second try/except block in + run_baseline_checks (line ~454) independently. + """ + from apm_cli.policy.models import CheckResult as _CheckResult + + # Write malformed apm.yml and a dummy lockfile so the path is reached. + (tmp_path / "apm.yml").write_text(": :\n bad: [yaml\n", encoding="utf-8") + (tmp_path / "apm.lock.yaml").write_text( + "lockfile_version: '1'\ngenerated_at: '2025-01-01T00:00:00Z'\ndependencies: []\n", + encoding="utf-8", + ) + + # Mock _check_lockfile_exists so check 1 passes and execution continues. + monkeypatch.setattr( + "apm_cli.policy.ci_checks._check_lockfile_exists", + lambda project_root: _CheckResult( + name="lockfile-exists", passed=True, message="mocked pass" + ), + ) + + 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 "Cannot parse apm.yml" in parse_checks[0].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" + ) From a40e9117096d9edeca8d8483b5739cdf90bba426 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Wed, 29 Apr 2026 21:45:22 +0100 Subject: [PATCH 2/3] fix(policy): address Copilot review feedback - Split FileNotFoundError from OSError in _load_raw_apm_yml() to avoid noisy warnings for normal "missing file" conditions (TOCTOU) - Add UnicodeDecodeError handling for non-UTF8/binary files - Add OSError to TOCTOU catch in run_policy_checks() and _check_lockfile_exists() so file-disappear races map to manifest-parse failure instead of unhandled exceptions - Fix docs: manifest_parse -> manifest-parse (hyphen) and clarify it is a local audit check, not a fetch outcome Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/enterprise/policy-reference.md | 2 +- src/apm_cli/policy/ci_checks.py | 2 +- src/apm_cli/policy/policy_checks.py | 8 +++++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/src/content/docs/enterprise/policy-reference.md b/docs/src/content/docs/enterprise/policy-reference.md index b4c7cefef..ed1660df4 100644 --- a/docs/src/content/docs/enterprise/policy-reference.md +++ b/docs/src/content/docs/enterprise/policy-reference.md @@ -818,7 +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 check is unconditional -- malformed manifests always fail the audit. | +| `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/src/apm_cli/policy/ci_checks.py b/src/apm_cli/policy/ci_checks.py index 288e0e9b1..e0e9fc051 100644 --- a/src/apm_cli/policy/ci_checks.py +++ b/src/apm_cli/policy/ci_checks.py @@ -46,7 +46,7 @@ def _check_lockfile_exists(project_root: Path) -> CheckResult: try: manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, yaml.YAMLError) as exc: + except (ValueError, yaml.YAMLError, OSError) as exc: return CheckResult( name="manifest-parse", passed=False, diff --git a/src/apm_cli/policy/policy_checks.py b/src/apm_cli/policy/policy_checks.py index 96d0a7f03..318c18d79 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -35,12 +35,18 @@ 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) + 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", @@ -954,7 +960,7 @@ def run_policy_checks( try: clear_apm_yml_cache() manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, yaml.YAMLError) as exc: + except (ValueError, yaml.YAMLError, OSError) as exc: result.checks.append( CheckResult( name="manifest-parse", From bc72ff5dc4d0b1c689ac4e1a49a070ebe8cebf5b Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Wed, 29 Apr 2026 23:09:55 +0100 Subject: [PATCH 3/3] fix(policy): address review panel findings Architecture: - Hoist manifest parsing out of _check_lockfile_exists into run_baseline_checks -- single parse eliminates TOCTOU window - _check_lockfile_exists now accepts Optional[APMPackage] and always returns name="lockfile-exists" (contract violation fixed) - Remove redundant clear_apm_yml_cache() + second from_apm_yml() call - Add OSError to catch tuple in run_baseline_checks - Add _logger + debug logging for lockfile read exceptions UX: - All manifest-parse error messages include remediation hint - CHANGELOG: add Migration block with detection command - governance-guide.md: disambiguate org policy vs project manifest rows - security.md: add remediation callout Docstrings: - _load_raw_apm_yml: document ordering, defence-in-depth contract Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- .../docs/enterprise/governance-guide.md | 2 +- docs/src/content/docs/enterprise/security.md | 2 +- src/apm_cli/policy/ci_checks.py | 73 +++++---- src/apm_cli/policy/policy_checks.py | 15 +- tests/unit/policy/test_ci_checks.py | 147 +++++++++--------- 6 files changed, 128 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17b77d3e9..1695510ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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. (#936) +- `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 diff --git a/docs/src/content/docs/enterprise/governance-guide.md b/docs/src/content/docs/enterprise/governance-guide.md index e113afa1d..2f8bf8450 100644 --- a/docs/src/content/docs/enterprise/governance-guide.md +++ b/docs/src/content/docs/enterprise/governance-guide.md @@ -351,7 +351,7 @@ 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` | diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index 0306e546b..fffa3c2b4 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -205,7 +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. +- **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/src/apm_cli/policy/ci_checks.py b/src/apm_cli/policy/ci_checks.py index e0e9fc051..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,27 +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", ) - import yaml - - from ..models.apm_package import APMPackage - - try: - manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, yaml.YAMLError, OSError) as exc: - return CheckResult( - name="manifest-parse", - passed=False, - message="Cannot parse apm.yml: %s" % exc, - ) - has_deps = manifest.has_apm_dependencies() or bool(manifest.get_mcp_dependencies()) lockfile_path = get_lockfile_path(project_root) @@ -65,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( @@ -433,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 + + 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 - result.checks.append(_check_lockfile_exists(project_root)) + # 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 @@ -449,21 +463,6 @@ def run_baseline_checks( if not apm_yml_path.exists() or not lockfile_path.exists(): return result - import yaml - - try: - clear_apm_yml_cache() - manifest = APMPackage.from_apm_yml(apm_yml_path) - except (ValueError, yaml.YAMLError) as exc: - result.checks.append( - CheckResult( - name="manifest-parse", - passed=False, - message="Cannot parse apm.yml: %s" % exc, - ) - ) - return result - lock = LockFile.read(lockfile_path) if lock is None: return result diff --git a/src/apm_cli/policy/policy_checks.py b/src/apm_cli/policy/policy_checks.py index 318c18d79..2e3847824 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -23,8 +23,19 @@ def _load_raw_apm_yml(project_root: Path) -> Optional[dict]: """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 now logs a warning so the failure is visible + or not a mapping -- but logs a warning so the failure is visible rather than silently swallowed. """ import yaml @@ -965,7 +976,7 @@ def run_policy_checks( CheckResult( name="manifest-parse", passed=False, - message="Cannot parse apm.yml: %s" % exc, + message="Cannot parse apm.yml: %s -- fix the YAML syntax error in apm.yml and re-run." % exc, ) ) return result diff --git a/tests/unit/policy/test_ci_checks.py b/tests/unit/policy/test_ci_checks.py index 16df6cf09..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. @@ -1023,41 +1033,57 @@ def test_aggregate_runner_includes_consent_check_last(self, tmp_path): assert "consider adding 'includes: auto'" in consent.message -# -- Group 3: _check_lockfile_exists malformed-manifest tests ------ +# -- Group 3: _check_lockfile_exists contract tests ---------------- -class TestCheckLockfileExistsMalformedManifest: - """_check_lockfile_exists must fail-closed on malformed apm.yml (fix #936).""" +class TestCheckLockfileExistsContract: + """_check_lockfile_exists must ALWAYS return name='lockfile-exists'. - def test_malformed_yaml_returns_failing_check(self, tmp_path: Path) -> None: - """Malformed YAML produces manifest-parse check with passed=False.""" - (tmp_path / "apm.yml").write_text(": :\n bad: [yaml\n", encoding="utf-8") - check = _check_lockfile_exists(tmp_path) - assert check.name == "manifest-parse" - assert not check.passed - assert "Cannot parse apm.yml" in check.message + 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_non_dict_yaml_returns_failing_check(self, tmp_path: Path) -> None: - """Non-dict YAML (bare list) produces manifest-parse failure.""" - (tmp_path / "apm.yml").write_text("- item1\n- item2\n", encoding="utf-8") - check = _check_lockfile_exists(tmp_path) - assert check.name == "manifest-parse" - assert not check.passed - assert "Cannot parse apm.yml" in check.message + 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_empty_file_returns_failing_check(self, tmp_path: Path) -> None: - """Empty apm.yml produces manifest-parse failure.""" - (tmp_path / "apm.yml").write_text("", encoding="utf-8") - check = _check_lockfile_exists(tmp_path) - assert check.name == "manifest-parse" + 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_binary_content_returns_failing_check(self, tmp_path: Path) -> None: - """Binary content in apm.yml raises ReaderError (subclass of YAMLError).""" - (tmp_path / "apm.yml").write_bytes(b"\x89PNG\r\n\x1a\n") - check = _check_lockfile_exists(tmp_path) - assert check.name == "manifest-parse" - 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 --------- @@ -1067,15 +1093,16 @@ 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 causes check 1 (_check_lockfile_exists) to return - a manifest-parse failure, which propagates through run_baseline_checks.""" + """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 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.""" @@ -1084,39 +1111,17 @@ def test_non_dict_yaml_produces_failing_check(self, tmp_path: Path) -> None: 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 len(parse_checks) == 1 assert not parse_checks[0].passed + assert "fix the YAML syntax error" in parse_checks[0].message - def test_second_parse_catch_with_mocked_check1( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch - ) -> None: - """Test the run_baseline_checks manifest-parse catch after check 1 passes. - - _check_lockfile_exists now fails on malformed YAML, so we monkeypatch it - to pass in order to exercise the second try/except block in - run_baseline_checks (line ~454) independently. - """ - from apm_cli.policy.models import CheckResult as _CheckResult - - # Write malformed apm.yml and a dummy lockfile so the path is reached. + 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") - (tmp_path / "apm.lock.yaml").write_text( - "lockfile_version: '1'\ngenerated_at: '2025-01-01T00:00:00Z'\ndependencies: []\n", - encoding="utf-8", - ) - - # Mock _check_lockfile_exists so check 1 passes and execution continues. - monkeypatch.setattr( - "apm_cli.policy.ci_checks._check_lockfile_exists", - lambda project_root: _CheckResult( - name="lockfile-exists", passed=True, message="mocked pass" - ), - ) - 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 "Cannot parse apm.yml" in parse_checks[0].message + 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