Skip to content

fix(policy): close audit --ci silent governance bypass + SCP/ADO URL parsing (#1159)#1164

Merged
danielmeppiel merged 8 commits into
mainfrom
fix/1159-audit-silent-skip
May 6, 2026
Merged

fix(policy): close audit --ci silent governance bypass + SCP/ADO URL parsing (#1159)#1164
danielmeppiel merged 8 commits into
mainfrom
fix/1159-audit-silent-skip

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

TL;DR

Closes #1159. Fixes a silent governance bypass in apm audit --ci plus a sibling SCP/ADO SSH URL parsing bug that produced the same class of failure on the install path. Adds install-path parity so policy.fetch_failure_default: block actually fails closed for the three no-policy outcomes (no_git_remote, absent, empty) on BOTH install and audit. Documents the new contract on five doc surfaces.

Problem (WHY)

#1159 reports two interlocking bugs:

  1. apm audit --ci silently passed (exit 0, no log line) when auto-discovery resolved no enforceable policy -- specifically the no_git_remote (no origin), absent (no apm-policy.yml at the org), and empty (file present but empty) outcomes. CI stayed green even when no enforcement ran. policy.fetch_failure_default: block did NOT block these on the install path either -- only malformed / cache_miss_fetch_fail / garbage_response honoured the knob.
  2. SCP-shorthand SSH URLs from non-git users failed to parse, dropping <user>@github.com:owner/repo (EMU) and <user>@ssh.dev.azure.com:v3/<org>/<project>/<repo> (Azure DevOps) on the floor. Auto-discovery then fell through to no_git_remote -- which (per Why do we need a GitHub token? #1) was silent. So an enterprise / ADO user got NO policy enforcement and NO warning.

The two together meant: an EMU/ADO project with policy.fetch_failure_default: block set, expecting fail-closed CI gating, would ship green with zero enforcement. That is the silent-bypass class the issue calls out.

Approach (WHAT)

Five phases, each its own commit:

  1. Fix SCP / ADO URL parsing at the source. Share one _SCP_LIKE_RE between models/dependency/reference.py and policy/discovery.py so EMU and ADO patterns parse on both code paths. Unwrap ADO v3/<org> to the real org.
  2. Stop silent-skip in apm audit --ci. For the three no-policy outcomes (no_git_remote, absent, empty) emit a [!] warning to stderr by default; honour policy.fetch_failure_default: block to fail closed ([x] + exit 1). Emit a forensic [i] line for disabled (auto-discovery turned off) so it's visible in CI logs. All diagnostics on stderr -- JSON / SARIF on stdout stays clean.
  3. Install-path parity. Extend _FETCH_FAILURE_OUTCOMES in policy/outcome_routing.py from 3 to 6 outcomes. policy.fetch_failure_default: block now means what it says on BOTH apm install and apm audit --ci. Reword PolicyViolationError to "no enforceable org policy was resolved" (honest about scope -- covers both fetch failure and absent/empty).
  4. e2e integration tests with real git init. No mocks on the discovery layer -- spawns a temp dir, runs git init, asserts on real CLI behavior.
  5. Documentation. Five surfaces: manifest schema, policy reference (new section 9.5.1), governance guide (failure-semantics table + offline matrix), apm-guide skill, CHANGELOG.

Implementation (HOW)

Layer Change
src/apm_cli/policy/discovery.py _parse_remote_url uses shared _SCP_LIKE_RE; ADO v3/<org> unwrapped
src/apm_cli/models/dependency/reference.py _parse_ssh_url uses shared _SCP_LIKE_RE; error messages cite captured user
src/apm_cli/commands/audit.py _audit_outcome_cause() helper; auto_discovered flag; per-outcome click.echo(..., err=True) (NOT logger -- logger routes to stdout via Rich Console and corrupts JSON)
src/apm_cli/policy/outcome_routing.py _FETCH_FAILURE_OUTCOMES extended (malformed, cache_miss_fetch_fail, garbage_response, no_git_remote, absent, empty); consumed by both install/phases/policy_gate.py and policy/install_preflight.py via single dispatch

Explicit --policy <file> opt-out: the new no-policy warnings only fire when auto_discovered=True. An explicit pointer at a minimal/empty baseline file does not regress.

ASCII-only: [!] warn, [x] error, [i] info -- per the repo's encoding contract.

Click 8.2.1 stderr/stdout split: result.stdout / result.stderr are now distinct (mix_stderr removed). Tests parsing JSON updated from result.output to result.stdout to avoid stderr contamination.

Trade-offs

  • install-parity is in this PR, not a follow-up. Per CEO arbitration on the apm-review-panel pass: the audit-only fix would have left the install half of the "block means block" promise broken, which was the original silent bypass. Bundling them is the smallest correct change.
  • Legacy fetch-failure paths in audit still use logger (which routes to stdout). Latent JSON-corruption bug for malformed/cache_miss_fetch_fail/garbage_response. Out of scope here -- confined fix to the new code paths to limit blast radius. Tracked as a follow-up.
  • No --require-policy flag yet (noted in the issue). Deferred to a follow-up so this PR stays focused on the bypass.

Validation evidence

  • Lint (CI mirror): uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/ -- silent.
  • Targeted: 61 tests across test_audit_silent_skip_e2e.py (3 new e2e), test_audit_ci_auto_discovery.py (13: 6 pre-existing + 7 new), test_audit_ci_command.py, test_audit_policy_command.py, test_fetch_failure_knob.py (4 -- 2 flipped from regression-trap, 2 new). All pass.
  • Full unit sweep (7699 tests) ran clean during phase 3.

How to test manually

# Bug repro: no remote, fetch_failure_default=block, expect FAIL CLOSED
mkdir /tmp/apm-1159 && cd /tmp/apm-1159
git init
cat > apm.yml <<YAML
name: test
version: '1.0'
policy:
  fetch_failure_default: block
YAML
apm audit --ci
# Pre-fix: exit 0, no output  -- silent bypass
# Post-fix: [x] on stderr, exit 1

Refs #1159

Daniel Meppiel and others added 5 commits May 6, 2026 15:46
_parse_remote_url() in policy/discovery.py and _parse_ssh_url() in
models/dependency/reference.py both hardcoded 'git@' as the SCP user
prefix, silently dropping valid EMU/GHE SSH URLs that use a non-'git'
account (e.g. enterprise-user@ghe.corp.com:org/repo.git).

For #1159 audit auto-discovery, a dropped URL meant the org could not
be extracted, which then triggered the silent fail-open (separate fix).

Also fixes Azure DevOps SSH URLs of the form
git@ssh.dev.azure.com:v3/<org>/<project>/<repo>: the leading 'v3/'
segment is part of the protocol, not the org. Previously the org was
parsed as 'v3'.

Both functions now use an SCP-like regex that accepts any RFC-3986
username, and the ADO v3 prefix is detected and stripped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pre-#1159, ``apm audit --ci`` silently swallowed the auto-discovery
outcomes ``no_git_remote``, ``absent`` and ``empty`` (set
``fetch_result = None`` with no log line), leaving CI green even when
no org policy was enforced. This is the audit-side fail-open
governance bypass.

Now those outcomes are explicit:

* warn (default): ``[!]`` line on stderr explaining the cause and
  pointing at ``policy.fetch_failure_default=block`` for fail-closed
  parity with the install path.
* block (when project apm.yml sets ``policy.fetch_failure_default:
  block``): ``[x]`` line on stderr + exit 1.
* ``disabled`` outcome (auto-discovery turned off in apm.yml) emits
  a forensic ``[i]`` breadcrumb on stderr; exit 0.

All new diagnostics route to STDERR via ``click.echo(..., err=True)``
so JSON / SARIF on stdout stays parseable for CI consumers. Existing
fetch-failure outcomes (``malformed`` / ``cache_miss_fetch_fail`` /
``garbage_response``) keep their existing wording; the same outcomes
on auto-discovery now reach the same code path.

Explicit ``--policy <file>`` keeps the legacy fall-through for the
no-policy outcomes -- an opt-in pointer at a baseline file should not
regress.

13 unit tests in tests/unit/test_audit_ci_auto_discovery.py cover
warn, block, disabled, and the explicit-policy negative case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…es (#1159)

Pre-#1159, the install pipeline silently swallowed the auto-discovery
outcomes no_git_remote / absent / empty even when the project's apm.yml
declared policy.fetch_failure_default: block. That was an install-side
fail-open governance bypass symmetrical to the audit bug fixed in the
previous commit.

Now those outcomes are added to _FETCH_FAILURE_OUTCOMES in
policy/outcome_routing.py, so the same block-on-no-policy semantics
apply to BOTH install paths (install/phases/policy_gate.py and
policy/install_preflight.py) without per-call duplication. The
PolicyViolationError message is updated to be honest about scope.

Default warn still keeps fail-open (no behavior change for projects
that have not opted in).

Two regression-trap tests in tests/unit/policy/test_fetch_failure_knob.py
that asserted the buggy fail-open behavior are flipped to assert the
opposite (DOES raise), and a new test_empty_block_raises plus a
test_absent_warn_does_not_raise round out the matrix.

Also: lint format + drop unused PolicyFetchResult.found kwarg in tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Real `git init` fixtures (no mocks on discovery layer) cover the three
no-policy outcomes that pre-#1159 silently passed:

- test_default_warn_emits_stderr_and_exits_zero (no remote, default warn)
- test_block_fails_closed_with_exit_one (no remote + fetch_failure_default=block)
- test_warn_when_not_a_git_repo (no .git at all)

Asserts: stderr contains the new [!]/[x] diagnostics, stdout JSON stays
clean, exit codes match contract.

Refs #1159

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ault=block)

Updates 5 surfaces to reflect new fail-closed parity for the
no_git_remote / absent / empty outcomes:

- CHANGELOG.md [Unreleased]: ### Fixed (audit + SCP), ### Changed (install parity)
- docs/.../reference/manifest-schema.md: fetch_failure_default scope expanded
- docs/.../enterprise/policy-reference.md: new section 9.5.1 No-policy outcomes
- docs/.../enterprise/governance-guide.md: failure-semantics table + offline matrix
- packages/apm-guide/.apm/skills/apm-usage/governance.md: section 9.5 + 9.8

Refs #1159

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 6, 2026 14:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR closes #1159 by ensuring org-policy auto-discovery cannot silently bypass governance in apm audit --ci, and by fixing SCP-like SSH remote parsing for non-git usernames (including Azure DevOps v3/<org> SSH forms). It also aligns install-path behavior so policy.fetch_failure_default: block consistently fails closed when no enforceable org policy is resolved.

Changes:

  • Extend SCP-like SSH parsing to accept arbitrary usernames and correctly interpret Azure DevOps SSH v3/<org>/... paths.
  • Make apm audit --ci surface previously-silent no-policy outcomes (no_git_remote, absent, empty, plus a breadcrumb for disabled) via stderr, honoring policy.fetch_failure_default: block.
  • Update unit/integration tests and documentation (manifest schema, policy reference, governance guide, apm-guide skill, changelog) to match the new fail-open/fail-closed semantics and stderr/stdout split.
Show a summary per file
File Description
src/apm_cli/commands/audit.py Adds outcome-to-message helper and new stderr diagnostics for --ci auto-discovery no-policy outcomes, honoring fetch_failure_default.
src/apm_cli/policy/discovery.py Uses an SCP-like regex to parse non-git SSH remotes and unwraps ADO v3/<org> org extraction.
src/apm_cli/policy/outcome_routing.py Expands the set of outcomes gated by policy.fetch_failure_default to include no-policy outcomes for install-path parity.
src/apm_cli/models/dependency/reference.py Broadens SCP-like dependency SSH parsing to accept arbitrary usernames and improves actionable error messaging.
tests/unit/test_generic_git_urls.py Adds regression tests for SCP-like SSH dependencies with non-git usernames.
tests/unit/policy/test_discovery.py Adds regression tests for policy remote parsing (non-git SCP and ADO SSH v3/).
tests/unit/test_audit_ci_command.py Switches JSON/SARIF parsing to result.stdout to avoid stderr diagnostics corrupting JSON parsing.
tests/unit/test_audit_policy_command.py Reads JSON from result.stdout for baseline-only --ci to avoid stderr warnings corrupting parsing.
tests/unit/test_audit_ci_auto_discovery.py Adds coverage for warn/block behavior and stderr messaging for no-policy outcomes and disabled.
tests/unit/policy/test_fetch_failure_knob.py Updates expectations so fetch_failure_default=block fails closed for absent/no-remote/empty outcomes (install parity).
tests/integration/test_audit_silent_skip_e2e.py New e2e coverage using real git init verifying stderr diagnostics and exit codes for --ci with/without fail-closed.
docs/src/content/docs/reference/manifest-schema.md Updates fetch_failure_default semantics to include no-policy outcomes and audit/install parity.
docs/src/content/docs/enterprise/policy-reference.md Documents no-policy outcomes and their warn/block behavior, plus audit stderr/stdout contract.
docs/src/content/docs/enterprise/governance-guide.md Updates offline/failure matrix to reflect no-policy outcomes and parity semantics.
packages/apm-guide/.apm/skills/apm-usage/governance.md Updates governance skill docs to reflect new no-policy outcome behavior and stderr-only diagnostics.
CHANGELOG.md Adds Unreleased entries for the audit silent-skip fix, SSH parsing fix, and install parity change.

Copilot's findings

  • Files reviewed: 16/16 changed files
  • Comments generated: 5

Comment thread src/apm_cli/commands/audit.py Outdated
Comment on lines +59 to +67
Used by both the ``warn`` (`[!]`) and ``block`` (`[x]`) branches so the
wording is identical; only the prefix and suffix change. Closes #1159
by replacing the prior silent-skip with explicit, actionable causes
for ``no_git_remote`` / ``absent`` / ``empty`` outcomes (and matching
the existing wording for fetch failures).
"""
src = source or "unknown"
if outcome == "no_git_remote":
return "Could not determine org from git remote; org-policy enforcement skipped"
Comment on lines +36 to 51
# Outcomes that honour the project-side ``policy.fetch_failure_default``
# knob. Pre-#1159, ``absent`` / ``no_git_remote`` / ``empty`` were
# excluded here -- they meant "no org policy" and were always fail-open
# even when the project explicitly opted in to ``block``. That was an
# install-path silent-skip (governance bypass) symmetrical to the audit
# bug fixed in the same PR. They now route through the ``block``
# branch so a project that asserts "no policy = no install" gets that
# guarantee on BOTH install and audit paths.
_FETCH_FAILURE_OUTCOMES = (
"malformed",
"cache_miss_fetch_fail",
"garbage_response",
"no_git_remote",
"absent",
"empty",
)
Comment on lines 33 to 37
import requests
import yaml

from ..cache.url_normalize import _SCP_LIKE_RE
from ..utils.path_security import PathTraversalError, ensure_path_within
Comment on lines +678 to +681
ssh_match = re.match(
r"^(?P<user>[a-zA-Z0-9_][a-zA-Z0-9_.+-]*)@(?P<host>[^:/]+):(?P<path>.+)$",
dependency_str,
)
Comment on lines +724 to +731
- `empty` -- the file exists but parses to an empty policy (no rules).

Pre-#1159, these outcomes were silently fail-open on BOTH `apm install` and `apm audit --ci` even when the project pinned `policy.fetch_failure_default: block`. As of v0.12.3 they honour the same knob as fetch failures:

- **`warn` (default):** `[!]` warning on stderr explaining the cause; install / audit proceeds.
- **`block`:** `[x]` error on stderr; install raises `PolicyViolationError`, `apm audit --ci` exits 1.

Explicit `--policy <file>` keeps the legacy fall-through for these three outcomes -- an opt-in pointer at a baseline file should not regress.
Five inline review findings, all valid:

1. audit.py: drop the duplicated "; enforcement skipped" wording (cause
   string is now neutral so warn-suffix and block-suffix don't compound)
2. outcome_routing.py: rename _FETCH_FAILURE_OUTCOMES ->
   _OUTCOMES_HONORING_FETCH_FAILURE_DEFAULT (the constant now includes
   no-policy outcomes, not just fetch failures)
3. cache/url_normalize.py: rename _SCP_LIKE_RE -> SCP_LIKE_RE (public)
   so the cross-layer import from policy/discovery.py is intentional
   instead of reaching into cache internals
4. models/dependency/reference.py: replace the inline SCP regex with
   the shared SCP_LIKE_RE so dependency parsing and policy discovery
   cannot drift on what counts as an SCP-shorthand SSH URL (this was
   the consolidation the PR body claimed but didn't deliver)
5. docs/.../policy-reference.md: drop hardcoded "v0.12.3" in favor of
   linking #1159 -- next-release version pins go stale fast

Verification: lint silent, 7701 unit tests pass.

Refs #1159

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

Addressed all 5 inline comments from @copilot-pull-request-reviewer in 3d5b8ab:

# File Disposition
1 audit.py -- duplicate "skipped; enforcement skipped" wording Fixed. _audit_outcome_cause() now returns a neutral cause for no_git_remote; warn-suffix and block-suffix no longer compound.
2 outcome_routing.py -- _FETCH_FAILURE_OUTCOMES name misleading Fixed. Renamed to _OUTCOMES_HONORING_FETCH_FAILURE_DEFAULT with a docstring stating the membership rule.
3 discovery.py -- importing private _SCP_LIKE_RE from cache layer Fixed. Promoted to public SCP_LIKE_RE in cache/url_normalize.py; the cross-layer dependency is now intentional.
4 reference.py -- duplicated SCP regex despite PR claim of sharing Fixed. Now uses the shared SCP_LIKE_RE -- this was the consolidation the PR body claimed but didn't deliver. Removes the drift risk between dependency parsing and policy discovery.
5 policy-reference.md -- hardcoded "v0.12.3" Fixed. Replaced with a link to #1159.

Verification: lint silent (uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/), 7701 unit tests pass.

Closes the test-coverage gaps flagged by the apm-review-panel on
PR #1164:

  TC-1: install-side parity for the no_git_remote + project
        policy.fetch_failure_default=block contract (real `git
        init`, real CliRunner, mocks only the downloader and
        update check)
  TC-2: SARIF stdout cleanliness on `apm audit --ci -f sarif`
        for both warn (parseable v2.1.0 JSON) and block (empty
        stdout, exit non-zero) paths
  TC-3: SCP/EMU + ADO v3 SSH URL parsing through three real
        consumers of the shared SCP_LIKE_RE:
          1. _extract_org_from_git_remote (real git remote)
          2. discover_policy_with_chain end-to-end (org routing)
          3. APMPackage.from_apm_yml (dep parse codepath)

All three test files are wired into scripts/test-integration.sh
so the gating CI script now covers the full #1159 surface.

TC-5 (legacy fetch-failure stderr cleanliness) was verified in
src/apm_cli/commands/audit.py:495-516 and is already correct: the
malformed / cache_miss_fetch_fail / garbage_response paths route
to click.echo(..., err=True). No new test added.

TC-4 / TC-6 are accepted as nits; not addressed in this commit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

Test-coverage panel finding disposition

Pushed c8955f89 addressing the test-coverage-expert findings from the panel review.

Closed in this commit

ID Gap New file Test count
TC-1 Install-side parity for no_git_remote + policy.fetch_failure_default=block; existing test_policy_install_e2e.py::TestI17NoGitRemote mocks discovery so the real git remote introspection wiring was untested. tests/integration/test_install_silent_skip_e2e.py 2
TC-2 SARIF stdout cleanliness on apm audit --ci -f sarif -- existing audit-silent-skip e2e only covered JSON. Block path additionally asserts empty stdout. tests/integration/test_audit_silent_skip_e2e.py (new TestAuditCiNoGitRemoteSarifE2E class) 2
TC-3 SCP/EMU + ADO v3 SSH URL parsing through the three real consumers of the shared SCP_LIKE_RE: _extract_org_from_git_remote (real git remote), discover_policy_with_chain (org routing end-to-end), and APMPackage.from_apm_yml (dep parse codepath). tests/integration/test_dep_url_parsing_e2e.py 9

All three files are wired into scripts/test-integration.sh so the gating CI script now covers the full #1159 surface.

Verified-already-correct (no test needed)

TC-5 (legacy fetch-failure stderr cleanliness): inspected src/apm_cli/commands/audit.py:495-516. The malformed / cache_miss_fetch_fail / garbage_response paths already route to click.echo(..., err=True). The persona's "latent risk" reading was a misread of the audit codepath; nothing to fix.

Pipeline-ordering note (relevant to TC-1)

In apm install the resolve phase (which constructs the downloader and downloads packages) runs before policy_gate. The block contract on the install path is therefore "non-zero exit with the verbatim policy.fetch_failure_default=block message," not "downloader was never called." The audit-side test asserts the stronger pre-fetch contract because audit's order is reversed (discovery -> gate -> render). Documented in the new file's module docstring.

Local verification

  • uv run pytest tests/integration/test_audit_silent_skip_e2e.py tests/integration/test_install_silent_skip_e2e.py tests/integration/test_dep_url_parsing_e2e.py -> 16 passed
  • Broader regression sweep across tests/unit/policy, tests/unit/cache, test_audit_ci_*, test_policy_install_e2e.py + new files -> 667 passed
  • Lint CI-mirror pair clean: uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/

Not addressed

  • TC-4 / TC-6: accepted as nits per the panel calibration; not required for this PR.

Per repo convention, docs reflect current behaviour only -- they do
not reference past behaviour, fix history, PR numbers, or issue
numbers. Strip three such references introduced by this branch and
one pre-existing #829 callout in the same paragraph being edited:

  - governance-guide.md: drop "since #1159" qualifier from the
    decision-matrix row and the failure-semantics row
  - policy-reference.md: rewrite the no-policy-outcomes paragraph
    to describe current behaviour without the "Pre-#1159 ..." /
    "Since #1159 ..." framing and without the github.com issue link
  - policy-reference.md: strip the "(closes #829)" parenthetical
    in the cache-miss bullet

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel merged commit ceb14bb into main May 6, 2026
11 checks passed
@danielmeppiel danielmeppiel deleted the fix/1159-audit-silent-skip branch May 6, 2026 15:13
sergio-sisternes-epam pushed a commit that referenced this pull request May 19, 2026
…parsing (#1159) (#1164)

* fix(policy): SCP shorthand and ADO v3 SSH org extraction (#1159)

_parse_remote_url() in policy/discovery.py and _parse_ssh_url() in
models/dependency/reference.py both hardcoded 'git@' as the SCP user
prefix, silently dropping valid EMU/GHE SSH URLs that use a non-'git'
account (e.g. enterprise-user@ghe.corp.com:org/repo.git).

For #1159 audit auto-discovery, a dropped URL meant the org could not
be extracted, which then triggered the silent fail-open (separate fix).

Also fixes Azure DevOps SSH URLs of the form
git@ssh.dev.azure.com:v3/<org>/<project>/<repo>: the leading 'v3/'
segment is part of the protocol, not the org. Previously the org was
parsed as 'v3'.

Both functions now use an SCP-like regex that accepts any RFC-3986
username, and the ADO v3 prefix is detected and stripped.

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

* fix(audit): surface no-policy auto-discovery outcomes in --ci (#1159)

Pre-#1159, ``apm audit --ci`` silently swallowed the auto-discovery
outcomes ``no_git_remote``, ``absent`` and ``empty`` (set
``fetch_result = None`` with no log line), leaving CI green even when
no org policy was enforced. This is the audit-side fail-open
governance bypass.

Now those outcomes are explicit:

* warn (default): ``[!]`` line on stderr explaining the cause and
  pointing at ``policy.fetch_failure_default=block`` for fail-closed
  parity with the install path.
* block (when project apm.yml sets ``policy.fetch_failure_default:
  block``): ``[x]`` line on stderr + exit 1.
* ``disabled`` outcome (auto-discovery turned off in apm.yml) emits
  a forensic ``[i]`` breadcrumb on stderr; exit 0.

All new diagnostics route to STDERR via ``click.echo(..., err=True)``
so JSON / SARIF on stdout stays parseable for CI consumers. Existing
fetch-failure outcomes (``malformed`` / ``cache_miss_fetch_fail`` /
``garbage_response``) keep their existing wording; the same outcomes
on auto-discovery now reach the same code path.

Explicit ``--policy <file>`` keeps the legacy fall-through for the
no-policy outcomes -- an opt-in pointer at a baseline file should not
regress.

13 unit tests in tests/unit/test_audit_ci_auto_discovery.py cover
warn, block, disabled, and the explicit-policy negative case.

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

* fix(install): honour fetch_failure_default=block for no-policy outcomes (#1159)

Pre-#1159, the install pipeline silently swallowed the auto-discovery
outcomes no_git_remote / absent / empty even when the project's apm.yml
declared policy.fetch_failure_default: block. That was an install-side
fail-open governance bypass symmetrical to the audit bug fixed in the
previous commit.

Now those outcomes are added to _FETCH_FAILURE_OUTCOMES in
policy/outcome_routing.py, so the same block-on-no-policy semantics
apply to BOTH install paths (install/phases/policy_gate.py and
policy/install_preflight.py) without per-call duplication. The
PolicyViolationError message is updated to be honest about scope.

Default warn still keeps fail-open (no behavior change for projects
that have not opted in).

Two regression-trap tests in tests/unit/policy/test_fetch_failure_knob.py
that asserted the buggy fail-open behavior are flipped to assert the
opposite (DOES raise), and a new test_empty_block_raises plus a
test_absent_warn_does_not_raise round out the matrix.

Also: lint format + drop unused PolicyFetchResult.found kwarg in tests.

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

* test(audit): add e2e integration tests for --ci silent-skip fix

Real `git init` fixtures (no mocks on discovery layer) cover the three
no-policy outcomes that pre-#1159 silently passed:

- test_default_warn_emits_stderr_and_exits_zero (no remote, default warn)
- test_block_fails_closed_with_exit_one (no remote + fetch_failure_default=block)
- test_warn_when_not_a_git_repo (no .git at all)

Asserts: stderr contains the new [!]/[x] diagnostics, stdout JSON stays
clean, exit codes match contract.

Refs #1159

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

* docs: document #1159 fix (no-policy outcomes honour fetch_failure_default=block)

Updates 5 surfaces to reflect new fail-closed parity for the
no_git_remote / absent / empty outcomes:

- CHANGELOG.md [Unreleased]: ### Fixed (audit + SCP), ### Changed (install parity)
- docs/.../reference/manifest-schema.md: fetch_failure_default scope expanded
- docs/.../enterprise/policy-reference.md: new section 9.5.1 No-policy outcomes
- docs/.../enterprise/governance-guide.md: failure-semantics table + offline matrix
- packages/apm-guide/.apm/skills/apm-usage/governance.md: section 9.5 + 9.8

Refs #1159

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

* review: address Copilot reviewer comments on #1164

Five inline review findings, all valid:

1. audit.py: drop the duplicated "; enforcement skipped" wording (cause
   string is now neutral so warn-suffix and block-suffix don't compound)
2. outcome_routing.py: rename _FETCH_FAILURE_OUTCOMES ->
   _OUTCOMES_HONORING_FETCH_FAILURE_DEFAULT (the constant now includes
   no-policy outcomes, not just fetch failures)
3. cache/url_normalize.py: rename _SCP_LIKE_RE -> SCP_LIKE_RE (public)
   so the cross-layer import from policy/discovery.py is intentional
   instead of reaching into cache internals
4. models/dependency/reference.py: replace the inline SCP regex with
   the shared SCP_LIKE_RE so dependency parsing and policy discovery
   cannot drift on what counts as an SCP-shorthand SSH URL (this was
   the consolidation the PR body claimed but didn't deliver)
5. docs/.../policy-reference.md: drop hardcoded "v0.12.3" in favor of
   linking #1159 -- next-release version pins go stale fast

Verification: lint silent, 7701 unit tests pass.

Refs #1159

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

* test(#1159): add E2E coverage for TC-1, TC-2, TC-3 from panel review

Closes the test-coverage gaps flagged by the apm-review-panel on
PR #1164:

  TC-1: install-side parity for the no_git_remote + project
        policy.fetch_failure_default=block contract (real `git
        init`, real CliRunner, mocks only the downloader and
        update check)
  TC-2: SARIF stdout cleanliness on `apm audit --ci -f sarif`
        for both warn (parseable v2.1.0 JSON) and block (empty
        stdout, exit non-zero) paths
  TC-3: SCP/EMU + ADO v3 SSH URL parsing through three real
        consumers of the shared SCP_LIKE_RE:
          1. _extract_org_from_git_remote (real git remote)
          2. discover_policy_with_chain end-to-end (org routing)
          3. APMPackage.from_apm_yml (dep parse codepath)

All three test files are wired into scripts/test-integration.sh
so the gating CI script now covers the full #1159 surface.

TC-5 (legacy fetch-failure stderr cleanliness) was verified in
src/apm_cli/commands/audit.py:495-516 and is already correct: the
malformed / cache_miss_fetch_fail / garbage_response paths route
to click.echo(..., err=True). No new test added.

TC-4 / TC-6 are accepted as nits; not addressed in this commit.

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

* docs: drop PR/issue references and historical phrasing from policy docs

Per repo convention, docs reflect current behaviour only -- they do
not reference past behaviour, fix history, PR numbers, or issue
numbers. Strip three such references introduced by this branch and
one pre-existing #829 callout in the same paragraph being edited:

  - governance-guide.md: drop "since #1159" qualifier from the
    decision-matrix row and the failure-semantics row
  - policy-reference.md: rewrite the no-policy-outcomes paragraph
    to describe current behaviour without the "Pre-#1159 ..." /
    "Since #1159 ..." framing and without the github.com issue link
  - policy-reference.md: strip the "(closes #829)" parenthetical
    in the cache-miss bullet

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

---------

Co-authored-by: Daniel Meppiel <copilot-rework@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] _parse_remote_url fails for SSH remotes with non-git usernames

2 participants