Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .apm/architecture/owners/hooks-integrations.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
},
{
"id": "neutral-hook-contract",
"decision": "Neutral hook source grammar, per-target native shape, and shared-config APM-owned drift projection",
"owner": "hook_contract.py (HOOK_COMMAND_KEYS, parse_hook_source, _entries_to_ir), per-target renderers, hook_ownership.py (project_apm_owned_hook_entries)",
"decision": "Neutral hook source grammar, per-target native shape, shared-config APM-owned drift projection, and ownership-sidecar comparison",
"owner": "hook_contract.py (HOOK_COMMAND_KEYS, parse_hook_source, _entries_to_ir), per-target renderers, hook_ownership.py (project_apm_owned_hook_entries, canonicalize_hook_sidecar)",
"selectors": [
"src/apm_cli/hook_contract.py",
"src/apm_cli/integration/hook_ir.py",
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Hook ownership sidecars ignore JSON formatting during drift checks while preserving meaningful and invalid-content findings. -- by @Pybsama (#3155)
- `apm install` now reconciles renamed or deleted project-local skills and removed targets using recorded concrete ownership, preserving ambiguous legacy output, shared claims, and user edits -- by @tillig (#3190)
- `apm install plugin@mkt` for an in-repository plugin of a `github.com` marketplace registered over SSH pre-flights the repository over SSH instead of HTTPS-only probes, so a private marketplace installs with an SSH key and no GitHub token. (#3164)
- `apm pack --check-clean` now certifies GitHub-hosted manifestless skills when `SKILL.md` has valid frontmatter at the resolved repository, ref, and subdirectory, while invalid skill metadata and missing repositories, refs, and package paths remain fail-closed. (#3064)
Expand Down
4 changes: 3 additions & 1 deletion CONFORMANCE.json
Original file line number Diff line number Diff line change
Expand Up @@ -823,8 +823,10 @@
"keyword": "MUST",
"section": "6.8",
"status": "active",
"test_count": 3,
"test_count": 5,
"tests": [
"tests/spec_conformance/test_policy_reqs.py::test_hook_sidecar_comparison_preserves_governed_drift_exit[False]",
"tests/spec_conformance/test_policy_reqs.py::test_hook_sidecar_comparison_preserves_governed_drift_exit[True]",
"tests/spec_conformance/test_policy_reqs.py::test_policy_fail_on_drift_parses_and_is_specified",
"tests/spec_conformance/test_policy_reqs.py::test_project_self_replay_preserves_governed_drift[False]",
"tests/spec_conformance/test_policy_reqs.py::test_project_self_replay_preserves_governed_drift[True]"
Expand Down
2 changes: 1 addition & 1 deletion CONFORMANCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020
| [req-pl-011](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-011) | MUST | 6.1.1 | governance | active | 2 | - |
| [req-pl-012](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-012) | MUST | 6.1.1 | governance | active | 1 | - |
| [req-pl-013](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-013) | MUST | 6.8 | governance | active | 1 | - |
| [req-pl-014](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-014) | MUST | 6.8 | governance | active | 3 | - |
| [req-pl-014](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-014) | MUST | 6.8 | governance | active | 5 | - |
| [req-pl-015](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-015) | MUST | 6.3.5 | governance | active | 1 | - |
| [req-pl-016](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-016) | MUST | 6.8 | governance | active | 1 | - |
| [req-pl-017](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-017) | MUST | 6.8 | governance | active | 1 | - |
Expand Down
8 changes: 5 additions & 3 deletions docs/src/content/docs/enterprise/drift-detection.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ only the deployment destination. `unrecorded` means replay produced the
same normalized bytes as the project but no exact or directory
`deployed_files` claim covers the path; shared merge-hook targets are exempt
from `unrecorded`. For their drift comparison, only APM-owned hook entries are
considered; user-owned hooks do not create drift. The APM-owned sidecar remains
byte-for-byte checked, so tampered or missing APM-owned hooks report
`modified`. Pass `--no-drift` to skip the replay.
considered; user-owned hooks do not create drift. The APM-owned sidecar is
compared as JSON: object-key order and formatting are ignored, but all fields
and list order remain significant. Changed content, malformed JSON, duplicate
keys, and non-JSON constants (`NaN`, `Infinity`, `-Infinity`) report `modified`.
Pass `--no-drift` to skip the replay.
In bare `apm audit`, the replay remains cache-only and a fresh checkout
without a warm cache yields an informational skip. In `apm audit --ci`, a
cold cache instead triggers the lock-pinned scratch self-hydration owned by
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/baseline-checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ the [policy schema](../policy-schema/).
### `drift`

- **What it verifies.** That the working tree matches what an install from the current lockfile would produce. The check replays the install pipeline into a scratch tree and diffs the result against the project.
- **Fails when.** Any deployed file differs from the replay output (hand-edits, missing integrations, orphaned files). For shared hook configuration files, the comparison covers only APM-owned hook entries; user-owned hooks are preserved and do not create drift. The APM-owned sidecar remains byte-for-byte checked, so tampered or missing APM-owned hooks still fail. In `apm audit --ci`, a cold cache no longer yields a green skip: if `apm_modules/` is absent, APM self-hydrates a lock-pinned scratch install first and then diffs against the checkout. If that scratch replay cannot be materialized, the drift check fails closed with the replay error.
- **Fails when.** Any deployed file differs from the replay output (hand-edits, missing integrations, orphaned files). For shared hook configuration files, the comparison covers only APM-owned hook entries; user-owned hooks are preserved and do not create drift. The complete APM-owned sidecar is compared as JSON, ignoring object-key order and formatting but preserving all fields and list order; changed content, malformed JSON, duplicate keys, and non-JSON constants (`NaN`, `Infinity`, `-Infinity`) still fail. In `apm audit --ci`, a cold cache no longer yields a green skip: if `apm_modules/` is absent, APM self-hydrates a lock-pinned scratch install first and then diffs against the checkout. If that scratch replay cannot be materialized, the drift check fails closed with the replay error.
- **Skips when.** Only in bare `apm audit`, where the replay remains cache-only and a missing cache yields an informational skip advising the user to run `apm install` first.
- **Skip with.** `apm audit --ci --no-drift` (reduces coverage; reserve for performance-constrained CI loops).
- **Remediation.** Run `apm install` to restore the deployed state, or revert the hand-edit. For a cache-miss skip, the same `apm install` warms the cache and enables the check on the next run.
Expand Down
7 changes: 5 additions & 2 deletions docs/src/content/docs/reference/cli/audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,11 @@ integrations, orphaned files, and `unrecorded` files. `unrecorded` applies when
replay produced the same normalized bytes as the project but no exact or
directory `deployed_files` claim covers the path. For shared merge-hook
targets, audit compares only APM-owned hook entries; user-owned hooks do not
create drift. The APM-owned sidecar remains byte-for-byte checked, so tampered
or missing APM-owned hooks report `modified`. `unrecorded` findings fail
create drift. The APM-owned sidecar is compared as JSON, ignoring object-key
order and formatting while retaining every field and list order. Changed
commands, ownership markers, or execution order still report `modified`, as
do malformed JSON, duplicate keys, and non-JSON constants (`NaN`, `Infinity`,
`-Infinity`). `unrecorded` findings fail
`--ci`; run `apm install`, then commit the regenerated `apm.lock.yaml`.

Drift is whole-project only; `--file` and explicit `PACKAGE` runs skip it.
Expand Down
6 changes: 6 additions & 0 deletions packages/apm-guide/.apm/skills/apm-usage/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,12 @@ descendants, are skipped.

## Security and audit

For shared hook targets, drift compares only APM-owned hook entries. Ownership
sidecars are compared as JSON: object-key order and formatting do not cause
drift, but changed fields, ownership markers, or list order do. Malformed JSON,
duplicate keys, and non-JSON constants (`NaN`, `Infinity`, `-Infinity`) report
`modified`. User-owned hooks do not cause drift.

Automatic `apm audit` discovers registry-recognized primitive files, including
untracked native hook definitions in shared settings. It checks prompt
documents and decoded native prompt fields, not command strings, executables,
Expand Down
52 changes: 25 additions & 27 deletions scripts/architecture_linter/checks/mutation_hook_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -812,40 +812,38 @@ def _nhc_ownership_markers(provider: FactsProvider, rule_id: str) -> tuple[Viola


def _nhc_drift_projection(provider: FactsProvider, rule_id: str) -> tuple[Violation, ...]:
"""Shared hook drift projection must route through ``hook_ownership.py``."""
"""Shared hook drift comparisons must route through ``hook_ownership.py``."""
facts_by_path, failures = _read_required(provider, rule_id, (_HOOK_OWNERSHIP, _INSTALL_DRIFT))
findings: list[Violation] = list(failures)
if not failures:
findings.extend(
_require(
_count_regex_lines(
facts_by_path[_HOOK_OWNERSHIP], r"^def project_apm_owned_hook_entries\("
scan_paths = _python_paths(provider, under=_SRC, exclude=(_HOOK_OWNERSHIP,))
for helper in ("project_apm_owned_hook_entries", "canonicalize_hook_sidecar"):
if not failures:
findings.extend(
_require(
_count_regex_lines(facts_by_path[_HOOK_OWNERSHIP], rf"^def {helper}\(") == 1,
rule_id,
_HOOK_OWNERSHIP,
f"{helper} must be defined exactly once",
)
)
findings.extend(
_require(
_count_fixed_lines(facts_by_path[_INSTALL_DRIFT], f"{helper}(") == 2,
rule_id,
_INSTALL_DRIFT,
f"install drift must call {helper} exactly twice",
)
== 1,
rule_id,
_HOOK_OWNERSHIP,
"project_apm_owned_hook_entries must be defined exactly once",
)
)
findings.extend(
_require(
_count_fixed_lines(facts_by_path[_INSTALL_DRIFT], "project_apm_owned_hook_entries(")
== 2,
rule_id,
_INSTALL_DRIFT,
"install drift must call project_apm_owned_hook_entries exactly twice",
_duplicate_scan(
provider,
rule_id=rule_id,
paths=scan_paths,
pattern=rf"^def {helper}\(",
message="shared hook drift comparisons must route through hook_ownership.py",
exempt=False,
)
)
findings.extend(
_duplicate_scan(
provider,
rule_id=rule_id,
paths=_python_paths(provider, under=_SRC, exclude=(_HOOK_OWNERSHIP,)),
pattern=r"^def project_apm_owned_hook_entries\(",
message="shared hook drift projection must route through hook_ownership.py",
exempt=False,
)
)
return tuple(findings)


Expand Down
66 changes: 12 additions & 54 deletions src/apm_cli/install/drift.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@

import click

from apm_cli.core.command_logger import CommandLogger
from apm_cli.deps.path_anchoring import build_local_parent_index, resolve_local_dep_dir
from apm_cli.install.drift_render import (
_INLINE_DIFF_BYTE_CAP as _INLINE_DIFF_BYTE_CAP,
)
from apm_cli.install.drift_render import (
CheckLogger as CheckLogger,
)
from apm_cli.install.drift_render import (
_inline_diff_for,
)
Expand Down Expand Up @@ -182,55 +184,6 @@ def _copy_install_tree(source: Path, target: Path) -> None:
shutil.copytree(source, target, symlinks=True)


# ---------------------------------------------------------------------------
# Stderr-only logger for audit phases (CommandLogger writes to stdout)
# ---------------------------------------------------------------------------


class CheckLogger(CommandLogger):
"""CheckLogger emits drift phase markers to stderr.

``CommandLogger._rich_*`` writes to stdout (intended for human
install output). Audit/drift output must stay on stderr so that
machine-parseable JSON/SARIF on stdout is never polluted.
"""

def __init__(self, verbose: bool = False) -> None:
super().__init__("audit-drift", verbose=verbose)

def _emit(self, symbol_key: str, msg: str) -> None:
click.echo(f"{STATUS_SYMBOLS[symbol_key]} {msg}", err=True)

def replay_start(self) -> None:
self._emit("running", "Replaying install (cache-only)...")

def scratch_root(self, path: Path) -> None:
"""Verbose-only: announce the scratch tmpdir to stderr.

Stays on stderr so JSON/SARIF stdout payloads remain
machine-parseable. Self-gates on ``self.verbose`` so the
normal-mode user never sees it.
"""
if not self.verbose:
return
click.echo(
f"{STATUS_SYMBOLS['info']} drift scratch root: {path}",
err=True,
)

def diff_start(self) -> None:
self._emit("running", "Diffing scratch vs working tree...")

def replay_complete(self, n: int) -> None:
self._emit("check", f"Replayed {n} package(s)")

def clean(self) -> None:
self._emit("check", "No drift detected")

def findings(self, n: int) -> None:
self._emit("warning", f"Drift detected: {n} file(s)")


# ---------------------------------------------------------------------------
# Package materialization
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -907,13 +860,13 @@ def diff_scratch_against_project(
)
# Hook merge targets are shared with the user and never claimed in
# deployed_files, so they can never be "unrecorded". Their APM-owned slice
# is compared through hook_ownership; sidecars remain byte-for-byte owned.
# is compared through hook_ownership; sidecars compare all owned content
# while ignoring JSON object-key ordering from the live merge.
from apm_cli.install.manifest_reconcile import merge_hook_config_projection_specs

merge_config_specs = merge_hook_config_projection_specs(targets)
merge_config_paths = set(merge_config_specs) | {
sidecar_path for sidecar_path, _ in merge_config_specs.values()
}
merge_sidecar_paths = {sidecar_path for sidecar_path, _ in merge_config_specs.values()}
merge_config_paths = set(merge_config_specs) | merge_sidecar_paths

# Imperative local bundles have no authored source tree for replay. Their
# deployed bytes are already bound by local_deployed_file_hashes and the
Expand Down Expand Up @@ -982,6 +935,11 @@ def _is_canvas(rel: str) -> bool:
_normalize((project_root / sidecar_rel).read_bytes()),
event_container_key,
)
elif rel in merge_sidecar_paths:
from apm_cli.integration.hook_ownership import canonicalize_hook_sidecar

s_bytes = canonicalize_hook_sidecar(s_bytes)
p_bytes = canonicalize_hook_sidecar(p_bytes)
except (OSError, ValueError) as exc:
findings.append(
DriftFinding(
Expand Down
47 changes: 47 additions & 0 deletions src/apm_cli/install/drift_render.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
from pathlib import Path
from typing import TYPE_CHECKING

import click

from apm_cli.core.command_logger import CommandLogger
from apm_cli.utils.console import STATUS_SYMBOLS

if TYPE_CHECKING:
Expand All @@ -14,6 +17,50 @@
_INLINE_DIFF_BYTE_CAP = 100 * 1024 # 100 KB


class CheckLogger(CommandLogger):
"""CheckLogger emits drift phase markers to stderr.

``CommandLogger._rich_*`` writes to stdout (intended for human
install output). Audit/drift output must stay on stderr so that
machine-parseable JSON/SARIF on stdout is never polluted.
"""

def __init__(self, verbose: bool = False) -> None:
super().__init__("audit-drift", verbose=verbose)

def _emit(self, symbol_key: str, msg: str) -> None:
click.echo(f"{STATUS_SYMBOLS[symbol_key]} {msg}", err=True)

def replay_start(self) -> None:
self._emit("running", "Replaying install (cache-only)...")

def scratch_root(self, path: Path) -> None:
"""Verbose-only: announce the scratch tmpdir to stderr.

Stays on stderr so JSON/SARIF stdout payloads remain
machine-parseable. Self-gates on ``self.verbose`` so the
normal-mode user never sees it.
"""
if not self.verbose:
return
click.echo(
f"{STATUS_SYMBOLS['info']} drift scratch root: {path}",
err=True,
)

def diff_start(self) -> None:
self._emit("running", "Diffing scratch vs working tree...")

def replay_complete(self, n: int) -> None:
self._emit("check", f"Replayed {n} package(s)")

def clean(self) -> None:
self._emit("check", "No drift detected")

def findings(self, n: int) -> None:
self._emit("warning", f"Drift detected: {n} file(s)")


def _inline_diff_for(scratch_path: Path, project_path: Path) -> str:
"""Build an inline diff hint, capped to keep findings compact."""
try:
Expand Down
4 changes: 2 additions & 2 deletions src/apm_cli/install/manifest_reconcile.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ def merge_hook_config_paths(targets: list[TargetProfile]) -> set[str]:
tracking -- the same state ``reconcile_dropped_merge_hook_targets`` below
exists to reconcile. A membership-driven check must therefore exempt them,
or it reports every hooks-using project as under-recording. Content
coverage is unaffected: the drift replay reproduces these files and
compares them byte-for-byte.
coverage is unaffected: the drift replay compares APM-owned native hook
entries and the complete JSON content of their ownership sidecars.

Lives here rather than beside ``_MERGE_HOOK_TARGETS`` because
``hook_integrator.py`` is at its CI line-count budget, the same reason
Expand Down
Loading
Loading