fix(policy): fail closed on malformed manifest YAML

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

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` |
 

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 (<error>) -- 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 <source>) -- 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 <source> 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 <local-path>`. |
+| `manifest-parse` | `1` (always) | error | `Cannot parse apm.yml: <error>` | 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 <url>`. |
 | `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. |

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.

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 |

src/apm_cli/policy/ci_checks.py

@@ -12,45 +12,43 @@
 
 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
         (``local_deployed_files``) for this project.
     """
     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,28 +429,40 @@ 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
     # (no deps needed).  Skip remaining 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

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",
 }
 
 

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)