fix(audit): ignore hook sidecar object-key order

docs/src/content/docs/enterprise/drift-detection.md

@@ -48,9 +48,10 @@ 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, and
+duplicate keys 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

docs/src/content/docs/reference/baseline-checks.md

@@ -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, and duplicate keys 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.

docs/src/content/docs/reference/cli/audit.md

@@ -252,8 +252,10 @@ 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 and duplicate keys. `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.

src/apm_cli/install/drift.py

@@ -901,13 +901,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
@@ -976,6 +976,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(

src/apm_cli/install/manifest_reconcile.py

@@ -208,8 +208,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

src/apm_cli/integration/hook_ownership.py

@@ -62,6 +62,29 @@ def extract_apm_source_sidecar(hooks: dict) -> dict[str, list[dict[str, Any]]]:
     return sidecar
 
 
+def _reject_duplicate_keys(pairs: list[tuple[str, Any]]) -> dict[str, Any]:
+    result: dict[str, Any] = {}
+    for key, value in pairs:
+        if key in result:
+            raise ValueError(f"duplicate JSON key: {key}")
+        result[key] = value
+    return result
+
+
+def canonicalize_hook_sidecar(sidecar_bytes: bytes) -> bytes:
+    """Compare all ownership content without depending on JSON object-key order.
+
+    Keep list order and every field, including ownership markers. Reject
+    duplicate keys rather than silently discarding part of a modified sidecar.
+    """
+    # The integrator reads sidecars as UTF-8; json.loads(bytes) would also
+    # accept UTF-16/32 files that the next install cannot read.
+    sidecar = json.loads(sidecar_bytes.decode("utf-8"), object_pairs_hook=_reject_duplicate_keys)
+    if not isinstance(sidecar, dict):
+        raise ValueError("ownership sidecar must be a JSON object")
+    return json.dumps(sidecar, sort_keys=True, separators=(",", ":")).encode("utf-8")
+
+
 def project_apm_owned_hook_entries(
     config_bytes: bytes,
     sidecar_bytes: bytes,
@@ -75,14 +98,6 @@ def project_apm_owned_hook_entries(
     ``_apm_source`` markers; retain only that marked slice for comparison.
     """
 
-    def _reject_duplicate_keys(pairs: list[tuple[str, Any]]) -> dict[str, Any]:
-        result: dict[str, Any] = {}
-        for key, value in pairs:
-            if key in result:
-                raise ValueError(f"duplicate JSON key: {key}")
-            result[key] = value
-        return result
-
     config = json.loads(config_bytes, object_pairs_hook=_reject_duplicate_keys)
     sidecar = json.loads(sidecar_bytes, object_pairs_hook=_reject_duplicate_keys)
     if not isinstance(config, dict) or not isinstance(sidecar, dict):

tests/integration/test_hook_sidecar_drift_e2e.py

@@ -0,0 +1,66 @@
+"""Hermetic install/audit regression for ownership sidecar object-key order."""
+
+from __future__ import annotations
+
+import json
+import os
+from pathlib import Path
+
+import pytest
+
+from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner
+from tests.utils.isolated_apm_environment import IsolatedApmEnvironment
+from tests.utils.lifecycle_state import LifecycleStateSnapshot
+from tests.utils.local_package import LocalPackageFactory
+
+pytestmark = pytest.mark.integration
+
+
+def test_preexisting_hook_event_order_does_not_cause_sidecar_drift(
+    tmp_path: Path, apm_engine_command: tuple[str, ...]
+) -> None:
+    isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=dict(os.environ))
+    environment = isolated.subprocess_env()
+    factory = LocalPackageFactory(isolated.work_root)
+    project = factory.create("hook-order", targets=("claude",))
+    factory.add_instruction(project, "rules", '---\napplyTo: "**"\n---\nBe brief.\n')
+    user_hook = {"hooks": [{"type": "command", "command": "echo user"}]}
+    settings_path = project.root / ".claude" / "settings.json"
+    settings_path.parent.mkdir()
+    settings_path.write_text(json.dumps({"hooks": {"SessionStart": [user_hook]}}), encoding="utf-8")
+    factory.add_hook(
+        project,
+        "events",
+        {
+            "hooks": {
+                "PreToolUse": [
+                    {"matcher": "Bash", "hooks": [{"type": "command", "command": "echo tool"}]}
+                ],
+                "SessionStart": [{"hooks": [{"type": "command", "command": "echo start"}]}],
+            }
+        },
+    )
+    runner = ApmLifecycleRunner(apm_engine_command)
+    install_args = ("install", "--no-policy", "--parallel-downloads", "0")
+    first = runner.run(install_args, cwd=project.root, env=environment)
+    assert first.returncode == 0, first.stdout + first.stderr
+    sidecar_path = project.root / ".claude" / "apm-hooks.json"
+    installed_bytes = sidecar_path.read_bytes()
+    assert list(json.loads(installed_bytes)) == ["SessionStart", "PreToolUse"]
+
+    second = runner.run(install_args, cwd=project.root, env=environment)
+    assert second.returncode == 0, second.stdout + second.stderr
+    assert sidecar_path.read_bytes() == installed_bytes
+    before = LifecycleStateSnapshot.capture(project.root, targets=("claude",))
+
+    result = runner.run(
+        ("audit", "--ci", "--no-policy", "--format", "json"),
+        cwd=project.root,
+        env=environment,
+    )
+
+    assert result.returncode == 0, result.stdout + result.stderr
+    assert json.loads(result.stdout)["drift"]["drift"] == []
+    assert LifecycleStateSnapshot.capture(project.root, targets=("claude",)) == before
+    settings = json.loads(settings_path.read_text(encoding="utf-8"))
+    assert user_hook in settings["hooks"]["SessionStart"]

tests/unit/install/test_drift.py

@@ -387,6 +387,62 @@ def test_diff_engine_hook_merge_target_tampering_is_modified(tmp_path):
     assert [(finding.kind, finding.path) for finding in findings] == [("modified", rel)]
 
 
+@pytest.mark.parametrize(
+    ("project_bytes", "modified"),
+    [
+        (b'{"B":[], "A":[{"command":"one","timeout":1},{"command":"two"}]}', False),
+        (b'{"A":[{"timeout":1,"command":"one"},{"command":"two"}],"B":[]}', False),
+        (b'{"A":[{"command":"changed","timeout":1},{"command":"two"}],"B":[]}', True),
+        (b'{"A":[{"command":"one","timeout":true},{"command":"two"}],"B":[]}', True),
+        (b'{"A":[{"command":"two"},{"command":"one","timeout":1}],"B":[]}', True),
+        (b'{"A":[{"command":"one","timeout":1}],"B":[]}', True),
+        (b'{"A":[{"command":"one","timeout":1},{"command":"two"}],"B":[],"C":[]}', True),
+        (b'{"A":[],"A":[{"command":"one","timeout":1},{"command":"two"}],"B":[]}', True),
+        (
+            b'{"A":[{"command":"discarded","command":"one","timeout":1},{"command":"two"}],"B":[]}',
+            True,
+        ),
+        (b"{invalid json", True),
+        (b"[]", True),
+        ('{"A":[{"command":"one","timeout":1},{"command":"two"}],"B":[]}'.encode("utf-16"), True),
+        ('{"A":[{"command":"one","timeout":1},{"command":"two"}],"B":[]}'.encode("utf-32"), True),
+    ],
+    ids=[
+        "event-key-order",
+        "entry-key-order",
+        "command",
+        "value-type",
+        "execution-order",
+        "removed-entry",
+        "added-event",
+        "duplicate-event",
+        "duplicate-field",
+        "invalid-json",
+        "non-object",
+        "utf-16",
+        "utf-32",
+    ],
+)
+def test_diff_engine_hook_sidecar_compares_json_content(tmp_path, project_bytes, modified):
+    """Only object ordering/formatting is irrelevant; all content is owned."""
+    from apm_cli.integration.targets import KNOWN_TARGETS
+
+    scratch = tmp_path / "scratch"
+    project = tmp_path / "project"
+    rel = ".claude/apm-hooks.json"
+    _write(scratch / rel, b'{"A":[{"command":"one","timeout":1},{"command":"two"}],"B":[]}')
+    _write(project / rel, project_bytes)
+
+    findings = diff_scratch_against_project(
+        scratch, project, _empty_lockfile(), targets=[KNOWN_TARGETS["claude"]]
+    )
+
+    assert [(finding.kind, finding.path) for finding in findings] == (
+        [("modified", rel)] if modified else []
+    )
+    assert (project / rel).read_bytes() == project_bytes
+
+
 def test_diff_engine_reports_malformed_shared_hook_event_as_modified(tmp_path):
     """A malformed native event remains drift rather than aborting audit."""
     from apm_cli.integration.targets import KNOWN_TARGETS