fix(audit): fail closed when an explicit --policy cannot be loaded

[Daniel Meppiel (danielmeppiel)]

Description

TL;DR

apm audit --ci --policy <source> exited 0 when the named policy could not be loaded: an unreachable URL, a missing file, or a malformed or garbage policy. It only printed enforcement skipped. An explicit path, https:// URL or owner/repo now fails the gate with exit 1 and an [x] message that names the source and the cause. Auto-discovery and --policy org keep the #829 warn default. CI JSON and SARIF now also report whether a policy was actually enforced.

Important

This changes an observable exit code on a governance gate. It is a bug fix: the old behaviour turned a typo, a missing file or a network failure into a green "compliant" result.

Problem (WHY)

• apm audit --ci --policy https://nonexistent.invalid/policy.yml → [!] Policy fetch failed … enforcement skipped, exit 0.
• apm audit --ci --policy ./missing-policy.yml → exit 0 with no warning at all. The path was not a file, so discovery treated it as an owner/repo ref. It made a GitHub lookup for org:./missing-policy.yml, which came back absent, and explicit absent fell through silently.
• A malformed local policy (YAML error, a list, or enforcement: bogus) → malformed → warn, exit 0.
• [!] Wrappers that check only the exit code cannot tell "passed with policy" from "passed without policy". -f json gave no signal.

Why this matters: #829 made fetch failures fail open so that auto-discovered org policy does not block developers on a network blip. An explicit --policy is the caller saying "enforce exactly this". The project's policy.fetch_failure_default is not a usable lever there, because the audited project may not belong to the caller (see #3195).

Approach (WHAT)

#	Fix
1	In _audit_ci_gate, when --policy is an explicit source (anything except org) and discovery returns no policy object, print [x] Policy '<source>' could not be loaded: <cause> to stderr and exit 1, before the fetch-failure-default branch.
2	Key the decision on fetch_result.found, not on outcome literals. Every non-loading outcome (cache_miss_fetch_fail, malformed, garbage_response, absent, hash_mismatch, incomplete_chain, refused http://) fails closed. cached_stale and empty still carry a valid policy, so they still load.
3	discover_policy (canonical owner of the --policy forms) returns absent + Policy file not found: <path> for an unambiguous local path that does not exist. It no longer makes a GitHub lookup for it. owner/name.yml stays ambiguous and is still tried as a repo.
4	CI JSON adds a top-level policy: {enforced, source, enforcement}. SARIF adds the same object under runs[0].invocations[0].properties.policy.
5	APM_POLICY_DISABLE=1 with an explicit --policy stays an operator override. It now prints a [!] line instead of skipping silently.

Implementation (HOW)

• src/apm_cli/commands/audit.py: new helpers _is_explicit_policy_source, _explicit_policy_load_reason and _policy_enforcement_report. One new fail-closed block before the existing [BUG] _parse_remote_url fails for SSH remotes with non-git usernames #1159 routing; that routing is untouched for auto-discovery and --policy org. Output follows the gate's existing convention (click.echo(err=True) with STATUS_SYMBOLS) so JSON and SARIF on stdout stay parseable. Sources are passed through _redact_policy_ref, so URL credentials and query strings never reach the message.
• src/apm_cli/policy/discovery.py: _is_unambiguous_local_policy_path, plus an early absent return in discover_policy. Install never passes policy_override, so install-time behaviour is unchanged. apm policy status --policy-source ./missing.yml now shows a clear "Policy file not found" discovery error instead of a bogus repo lookup.
• OpenAPM spec (req-pl-021, proposed 0.1.46): the Mode-B gate flagged policy/discovery.py as a critical path, and this is new observable governance behaviour on the existing req-pl-001 explicit-ref path, so it gets a spec citation rather than a waiver. The PR adds a §6.1 governance MUST, both enumerations, the Appendix C row, the manifest entry, two @pytest.mark.req("req-pl-021") behavioural tests in tests/spec_conformance/test_policy_reqs.py, and regenerated CONFORMANCE.{md,json}. Statement count goes from 128 to 129.
• Docs / skill: policy-reference.md (§9.5, §9.5.1, §9.8), governance-guide.md (§9 and §10 failure table), enforce-in-ci.md, reference/cli/audit.md (flag row, exit codes, JSON/SARIF field), manifest-schema.md (fetch_failure_default scope), and apm-usage/governance.md + commands.md.

Diagram

Legend: how apm audit --ci now routes a policy fetch result. The boxed node is new; everything else is the existing #829/#1159 path.

flowchart TD
    A["apm audit --ci"] --> B{"--policy given?"}
    B -->|"no"| C["discover_policy_with_chain (auto)"]
    B -->|"org"| C2["discover_policy_with_chain (org)"]
    B -->|"path / URL / owner-repo"| D["discover_policy_with_chain (explicit)"]
    D --> E{"policy object resolved?"}
    E -->|"yes (found, empty, cached_stale)"| G["run policy checks"]
    E -->|"no"| X["NEW: [x] could not be loaded, exit 1"]
    C --> F{"fetch failed or no policy?"}
    C2 --> F
    F -->|"yes"| H{"fetch_failure_default"}
    H -->|"warn"| W["[!] enforcement skipped, baseline only"]
    H -->|"block"| Y["[x] exit 1"]
    F -->|"no"| G
    style X fill:#fdd,stroke:#c00

Loading

Trade-offs

• Fail closed unconditionally for explicit sources; no new flag. We rejected the alternative --policy-fetch-failure block switch from the issue. An explicit source that cannot load is never a valid pass, and a flag would keep the unsafe default.
• --policy org keeps discovery semantics. It is explicitly out of scope ([BUG] apm audit --ci --policy <source> exits 0 when that explicitly requested policy cannot be loaded #3195). It is still governed by policy.fetch_failure_default.
• Local-path classification lives in discover_policy, not in audit. discover_policy already owns the --policy form contract (_help_text.POLICY_SOURCE_FORMS_HELP). Classifying paths in audit would create a second authority.
• Explicit absent now fails. The old docs said explicit --policy "falls through" absent. A 404 URL, a missing file or a repo without apm-policy.yml all mean "the requested policy does not exist", so they fail. A present-but-empty policy still loads and passes.
• Pre-existing gap left in place: auto-discovered hash_mismatch in apm audit --ci still follows the warn/block knob. The docs say it is always fail-closed. That is a separate discovery-path fix (out of scope here).

Benefits

1. A missing, unreachable or malformed explicit policy can no longer produce a green CI gate (exit 1 instead of 0).
2. ./missing-policy.yml no longer makes a pointless GitHub API call, and the error says "Policy file not found" instead of nothing.
3. Wrappers can read policy.enforced from JSON or SARIF instead of inferring it from check counts (the workaround from [BUG] apm audit --ci --policy <source> exits 0 when that explicitly requested policy cannot be loaded #3195).
4. Discovery and install behaviour, and the fetch_failure_default semantics, are unchanged.

Validation

Manual repro on the fixed build (scratch project with apm.yml + apm.lock.yaml)

== https://nonexistent.invalid/policy.yml   exit=1
[x] Policy 'https://nonexistent.invalid/policy.yml' could not be loaded: Connection error fetching https://nonexistent.invalid/policy.yml
== ./missing-policy.yml                      exit=1
[x] Policy './missing-policy.yml' could not be loaded: Policy file not found: ./missing-policy.yml
== contoso/nope                              exit=1
[x] Policy 'contoso/nope' could not be loaded: no apm-policy.yml found in repository 'contoso/nope'
== garbage.yml / list.yml / bad.yml          exit=1 (YAML parse error / must be a YAML mapping / enforcement must be one of ...)
== good.yml                                  exit=0   policy={'enforced': True, 'source': 'file:good.yml', 'enforcement': 'block'}
== (no --policy, no git remote)              exit=0   [!] Could not determine org from git remote; enforcement skipped (...)
== APM_POLICY_DISABLE=1 --policy good.yml    exit=0   [!] --policy good.yml ignored: APM_POLICY_DISABLE=1 disables policy enforcement; no policy checks ran

Each failing case also prints: --policy was passed explicitly, so the audit fails closed instead of skipping enforcement. Fix the policy source above and re-run.

Tests and CI-mirror lint

$ uv run --extra dev pytest -q tests/unit/test_audit_policy_command.py tests/unit/policy \
    tests/unit/test_audit_ci_auto_discovery.py tests/integration/test_audit_silent_skip_e2e.py \
    tests/integration/test_architecture_outcome_guards.py -n auto
1288 passed, 1 xfailed, 63 subtests passed

$ uv run --extra dev pytest -q -n auto tests/unit ...   # full unit suite
24356 passed; 6 failed -> 2 were mocked-SARIF tests (fixed), 4 were xdist-flaky
  (install lock concurrency, public-release metadata, targets provenance);
  re-run in isolation: 271 passed

ruff check / ruff format --check                     -> clean
YAML I/O guard, 2100-line guard, relative_to guard   -> clean
pylint R0801 (src/apm_cli + architecture linter)     -> 10.00/10
scripts/lint-auth-signals.sh                         -> clean
scripts/lint-architecture-boundaries.sh              -> exit 0

$ uv run --extra dev python -m tests.spec_conformance.orphan_check
[+] orphan_check OK: 129 requirements aligned across anchors / manifest / Appendix C / pytest markers
$ bash tests/spec_conformance/mode_b_detector.sh
[+] mode_b: spec-concurrent edit detected; orphan_check owns this PR
$ uv run --extra dev pytest tests/spec_conformance -p no:randomly -q
302 passed, 2 skipped

Scenario evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	--policy https://… that is unreachable, 404s, or returns HTML fails the gate	Governed by policy	tests/unit/test_audit_policy_command.py::TestExplicitPolicyLoadFailureFailsClosed::test_unreachable_url_exits_1 …::test_url_404_exits_1 …::test_garbage_url_response_exits_1	unit (CLI via CliRunner, requests.get patched)
2	--policy ./missing.yml (relative, absolute, any suffix case) or a directory fails without a remote lookup	Governed by policy, DevX	…::test_missing_local_file_exits_1_without_remote_lookup[True/False] tests/unit/policy/test_discovery.py::TestDiscoverPolicy::test_override_missing_local_path_skips_repo_lookup (incl. missing.YML) …::test_override_existing_directory_reports_not_a_file	unit
3	A malformed local policy (garbage YAML, list, schema-invalid) fails the gate	Governed by policy	…::test_malformed_local_policy_exits_1[garbage-yaml/not-a-mapping/schema-invalid]	unit
4	--policy owner/repo with no apm-policy.yml fails; owner/name.yml is still tried as a repo	Governed by policy	…::test_repo_without_policy_exits_1 tests/unit/policy/test_discovery.py::TestDiscoverPolicy::test_override_owner_repo_with_yaml_suffix_still_tries_repo	unit
5	fetch_failure_default: warn does not reopen an explicit failure	Governed by policy	…::test_project_warn_default_does_not_reopen_explicit_failure	unit
6	A stale-but-valid cached policy is still enforced	Governed by policy	…::test_stale_but_valid_cached_policy_is_still_enforced	unit
7	Auto-discovery and --policy org fetch failures still warn and exit 0	Governed by policy, DevX	tests/unit/test_audit_policy_command.py::TestDiscoveryFetchFailureUnchanged::test_discovery_fetch_failure_still_warns_and_exits_0[auto/org] existing tests/unit/test_audit_ci_auto_discovery.py, tests/integration/test_audit_silent_skip_e2e.py	unit / integration
8	JSON and SARIF say whether a policy was enforced	Governed by policy, DevX	tests/unit/test_audit_policy_command.py::TestPolicyEnforcementReport::* (4 tests)	unit
9	URL credentials never appear in the failure message	Secure by default	…::test_failure_message_redacts_url_credentials	unit
10	APM_POLICY_DISABLE=1 with explicit --policy warns instead of skipping silently	Governed by policy	…::test_policy_disable_env_warns_and_skips	unit

How to test

• In any project with apm.yml + apm.lock.yaml: apm audit --ci --policy ./missing-policy.yml; echo $? → [x] … Policy file not found, 1.
• apm audit --ci --policy https://nonexistent.invalid/policy.yml; echo $? → [x] … Connection error …, 1.
• printf 'enforcement: bogus\n' > bad.yml && apm audit --ci --policy bad.yml; echo $? → 1.
• apm audit --ci --policy <valid-file> -f json | jq .policy → {"enforced": true, …}.
• apm audit --ci with no --policy and no git remote → [!] … enforcement skipped, 0 (unchanged).

Issue and approved scope

Issue: #3195

Human scope-approval comment: maintainer Daniel Meppiel (@danielmeppiel) (also the issue author) requested this fix directly and set its scope in the delivery session. The scope matches the triage advisory on #3195. A formal apm-scope:v1 record on the issue is still pending from the maintainer.

Does this PR complete the issue, or what remains? Completes it, including the optional JSON/SARIF policy.enforced field. Follow-up candidate (not in #3195's scope): auto-discovered hash_mismatch in apm audit --ci should be unconditionally fail-closed, as the docs state.

Fixes #3195

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass
• Added tests for new functionality (if applicable)

Spec conformance (OpenAPM v0.1)

• Spec edit: docs/src/content/docs/specs/openapm-v0.1.md adds req-pl-021 (§6.1), the §6.9 and §11.3.2 enumerations, the Appendix C row, and the 0.1.46 (proposed) history row.
• Manifest edit: docs/public/specs/manifests/openapm-v0.1.requirements.yml.
• Test edit: tests/spec_conformance/test_policy_reqs.py (req-pl-021, 4 cases).
• CONFORMANCE.{md,json} regenerated via uv run --extra dev python -m tests.spec_conformance.gen_statement and committed.
• N/A -- this PR does not change OpenAPM-observable behaviour.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

🟡 Changes recommended

Path handling and documentation contain unresolved edge cases, one test violates the URL assertion rule, and human scope approval is still pending.

4 open findings

• Report directory paths distinctly from missing policy files · New
• Handle case-insensitive YAML suffixes in policy path detection · New
• Document audit exceptions for absent or empty policies · New
• Compare parsed URL components instead of URL substrings · New

What changed in this PR

Makes explicit audit policies fail closed when unavailable while preserving discovery defaults.

Changes:

• Fails explicit policy load errors with exit 1.
• Adds policy status to JSON/SARIF.
• Updates tests, documentation, and changelog.

File	Description
src/​apm_cli/​commands/​audit.py	Adds fail-closed routing and reports.
src/​apm_cli/​policy/​discovery.py	Detects missing local policies.
tests/​unit/​test_audit_policy_command.py	Covers audit behavior and output.
tests/​unit/​policy/​test_discovery.py	Covers path classification.
packages/​apm-guide/​.apm/​skills/​apm-usage/​governance.md	Documents governance behavior.
packages/​apm-guide/​.apm/​skills/​apm-usage/​commands.md	Updates command guidance.
docs/​src/​content/​docs/​reference/​manifest-schema.md	Clarifies configuration scope.
docs/​src/​content/​docs/​reference/​cli/​audit.md	Documents output and exit codes.
docs/​src/​content/​docs/​enterprise/​policy-reference.md	Details policy failure semantics.
docs/​src/​content/​docs/​enterprise/​governance-guide.md	Updates operational guidance.
docs/​src/​content/​docs/​enterprise/​enforce-in-ci.md	Documents CI enforcement.
CHANGELOG.md	Records the fix.

🧠 Review effort: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.