From 8f70a296eefa8104559da31ac67211875eff7879 Mon Sep 17 00:00:00 2001 From: lkshrk Date: Sat, 12 Sep 2026 17:54:54 +0200 Subject: [PATCH 01/12] fix(audit): bound the deploy-tree hidden-Unicode walk to primitive directories install_governance() returns the whole root for dedicated targets, which is right for ownership but wrong for a tree walk: at user scope ~/.claude and ~/.codex are mostly session transcripts APM never deploys. Scanning them cost minutes of CPU per `apm audit --ci` and produced critical content-integrity findings the audit cannot remediate. Add a `bounded` keyword that partitions every root the way `.agents/` already is, and use it from _scan_deployed_trees. Ownership and prune callers are unchanged. Lockfile-claimed paths outside primitive directories stay covered by the lockfile-scoped scan. _scan_deployed_trees also walks the scope-resolved targets the caller supplies rather than re-resolving at project scope, so a global audit's user roots such as ~/.copilot are no longer skipped. Fixes #2961 Claude-Session: https://claude.ai/code/session_01Nf7EcPHsefXRkPgvv3dAW3 --- docs/src/content/docs/enterprise/security.md | 2 +- .../content/docs/reference/baseline-checks.md | 2 +- src/apm_cli/install/manifest_reconcile.py | 23 +++++++++---- src/apm_cli/security/file_scanner.py | 21 ++++++++---- .../install/phases/test_lockfile_union.py | 25 ++++++++++++++ tests/unit/test_security_file_scanner.py | 34 +++++++++++++++++++ 6 files changed, 92 insertions(+), 15 deletions(-) diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index 7018ae6406..c8e55a52fc 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -197,7 +197,7 @@ This prevents lockfile membership from shrinking silently. Shared merge-hook targets and sidecars remain exempt because APM merges into user-owned files rather than claiming them. -A whole-project scan checks **every regular file under the deploy trees your targets govern** for hidden Unicode, not only files recorded in `apm.lock.yaml`. Hash verification and positional `PACKAGE` scans remain lockfile-scoped because they need recorded ownership. Source content under `.apm/` is not added by the deploy-tree walk; install-time scanning owns that surface, while any `.apm/` path already recorded in the lockfile remains covered. +A whole-project scan checks **every regular file under the deploy directories your targets govern** (primitive directories such as `.claude/skills/` and generated files, not the whole target root) for hidden Unicode, unioned with every file recorded in `apm.lock.yaml`, so a recorded path outside those directories such as `.claude/settings.json` stays covered. Hash verification and positional `PACKAGE` scans remain lockfile-scoped because they need recorded ownership. Source content under `.apm/` is not added by the deploy-tree walk; install-time scanning owns that surface, while any `.apm/` path already recorded in the lockfile remains covered. CI and remediation are separate commands because `--ci` and `--strip` are mutually exclusive: diff --git a/docs/src/content/docs/reference/baseline-checks.md b/docs/src/content/docs/reference/baseline-checks.md index 8b650c7756..036704cf27 100644 --- a/docs/src/content/docs/reference/baseline-checks.md +++ b/docs/src/content/docs/reference/baseline-checks.md @@ -131,7 +131,7 @@ the [policy schema](../policy-schema/). ### `content-integrity` - **What it verifies.** Two signals across every deployed file (including local `.apm/` content via the synthesized self-entry), each with the scope its evidence allows: - 1. Critical hidden Unicode (tag characters, bidi overrides, variation selectors 17-256, and similar steganographic markers), over **every file under the deploy trees the project's targets govern**. This signal needs no recorded baseline, so it is deliberately not limited to `deployed_files` -- otherwise a deployed file the lockfile omits would be exempt from scanning for as long as it stayed unrecorded. + 1. Critical hidden Unicode (tag characters, bidi overrides, variation selectors 17-256, and similar steganographic markers), over **every file under the deploy directories the project's targets govern** (each target's primitive directories and generated files, such as `.claude/skills/` and `.claude/hooks/`, not the whole `.claude/` root). This signal needs no recorded baseline, so it is deliberately not limited to `deployed_files` -- otherwise a deployed file the lockfile omits would be exempt from scanning for as long as it stayed unrecorded. The rest of a target root (session transcripts, history, caches) is never deployed by APM and is not walked. This walk is unioned with the lockfile-scoped pass, so a recorded path outside those directories (for example a claimed `.claude/settings.json`) is still scanned. 2. SHA-256 drift between the on-disk content and the hash recorded in `deployed_file_hashes` at install time. Necessarily lockfile-scoped: an unrecorded file has no baseline to compare against. - **Fails when.** Any deployed file contains a critical Unicode finding or its hash no longer matches the lockfile entry. Missing files are intentionally not reported here -- `deployed-files-present` owns that signal. Symlinks and entries without a recorded hash are skipped. - **Remediation.** Run `apm audit --strip` to clean Unicode findings, and `apm install` to restore hash-drifted files. Both may be needed. diff --git a/src/apm_cli/install/manifest_reconcile.py b/src/apm_cli/install/manifest_reconcile.py index 4cefc0cbfe..b1eefcbd8b 100644 --- a/src/apm_cli/install/manifest_reconcile.py +++ b/src/apm_cli/install/manifest_reconcile.py @@ -128,7 +128,11 @@ def _surface_target_cleanup( logger.stale_cleanup(dep_key, len(cleanup.deleted)) -def install_governance(targets: list[TargetProfile]) -> tuple[set[str], set[str]]: +def install_governance( + targets: list[TargetProfile], + *, + bounded: bool = False, +) -> tuple[set[str], set[str]]: """Return ``(file_prefixes, uri_schemes)`` governed by *targets*. Dedicated target roots govern their full subtree. The shared ``.agents`` @@ -136,6 +140,10 @@ def install_governance(targets: list[TargetProfile]) -> tuple[set[str], set[str] claim a declared sibling's files (for example, Copilot's ``.agents/skills`` versus Antigravity's ``.agents/rules``). + ``bounded`` partitions dedicated roots the same way. Ownership tests need + the whole root, but a tree walk does not: a root such as ``.claude`` also + holds session transcripts and other data APM never deploys. + ``uri_schemes`` is the set of lockfile URI schemes used by dynamic / user-machine targets (``copilot-app`` -> ``copilot-app-db://``, ``copilot-cowork`` -> ``cowork://``). @@ -149,15 +157,16 @@ def install_governance(targets: list[TargetProfile]) -> tuple[set[str], set[str] if target_schemes: uri_schemes.update(target_schemes) continue - root = getattr(target, "root_dir", None) - if root and str(root).rstrip("/") != ".agents": - file_prefixes.add(str(root).rstrip("/") + "/") + root = str(getattr(target, "root_dir", None) or "").rstrip("/") + shared_root = root == ".agents" + if root and not shared_root and not bounded: + file_prefixes.add(root + "/") primitives = getattr(target, "primitives", None) if isinstance(primitives, dict): for mapping in primitives.values(): deploy_root = getattr(mapping, "deploy_root", None) base = str(deploy_root or root or "").rstrip("/") - if base != ".agents": + if not base or (base != ".agents" and not bounded): continue subdir = getattr(mapping, "subdir", None) if subdir: @@ -169,9 +178,9 @@ def install_governance(targets: list[TargetProfile]) -> tuple[set[str], set[str] else: # Compatibility for minimal TargetProfile stand-ins. file_prefixes.add(f"{base}/") - if str(root or "").rstrip("/") == ".agents": + if root and (shared_root or bounded): for generated in getattr(target, "generated_files", ()) or (): - file_prefixes.add(f".agents/{str(generated).lstrip('/')}") + file_prefixes.add(f"{root}/{str(generated).lstrip('/')}") return file_prefixes, uri_schemes diff --git a/src/apm_cli/security/file_scanner.py b/src/apm_cli/security/file_scanner.py index d97b897227..b3b3b17299 100644 --- a/src/apm_cli/security/file_scanner.py +++ b/src/apm_cli/security/file_scanner.py @@ -128,7 +128,7 @@ def _scan_deployed_trees( project_root: Path, targets: Sequence[TargetProfile] = (), ) -> _FileScanResult: - """Scan every file under the deploy trees this project's targets govern. + """Scan every file under the deploy directories this project's targets govern. Hash verification has to be manifest-driven: comparing a hash needs a recorded baseline to compare against. Hidden-Unicode detection does not -- @@ -138,15 +138,24 @@ def _scan_deployed_trees( scope narrowed (issue #2379). For this signal the deploy tree is the boundary, not manifest membership. - Targets are resolved from the project, which is sufficient: a deploy tree - absent from disk has nothing to scan, and one present on disk is exactly - what ``detect_by_dir`` resolution keys on. Sources under ``.apm/`` are not - in scope here -- the install-time pre-deployment gate owns those. + Callers holding scope-resolved *targets* pass them: a global audit's + user-scope roots (``~/.copilot``) are not what a fresh project-scope + resolution returns. Without them, targets are resolved from the project, + which is sufficient: a deploy tree absent from disk has nothing to scan, + and one present on disk is exactly what ``detect_by_dir`` resolution keys + on. Sources under ``.apm/`` are not in scope here -- the install-time + pre-deployment gate owns those. + + The walk is bounded to each target's primitive deploy directories and + generated files rather than its whole root: at user scope a root such as + ``~/.claude`` also holds session transcripts APM never deploys, and + walking those costs minutes for no signal the audit can remediate. """ from ..install.manifest_reconcile import install_governance from ..integration.targets import resolve_targets - file_prefixes, _uri_schemes = install_governance(resolve_targets(project_root)) + scoped = list(targets) if targets else resolve_targets(project_root) + file_prefixes, _uri_schemes = install_governance(scoped, bounded=True) result = _empty_scan() for rel_path in _minimal_governed_prefixes(file_prefixes): diff --git a/tests/unit/install/phases/test_lockfile_union.py b/tests/unit/install/phases/test_lockfile_union.py index 34dcc82e25..a3eab168aa 100644 --- a/tests/unit/install/phases/test_lockfile_union.py +++ b/tests/unit/install/phases/test_lockfile_union.py @@ -223,6 +223,31 @@ def test_shared_agents_root_is_partitioned_by_primitive_subdirectory(self): assert ".agents/skills/" in file_prefixes assert ".agents/" not in file_prefixes + def test_bounded_governance_partitions_dedicated_roots(self): + from apm_cli.install.manifest_reconcile import install_governance + + file_prefixes, _ = install_governance([_known("claude"), _known("codex")], bounded=True) + + assert ".claude/skills/" in file_prefixes + assert ".claude/hooks/" in file_prefixes + assert ".codex/hooks.json" in file_prefixes + assert ".agents/skills/" in file_prefixes + assert ".claude/" not in file_prefixes + assert ".codex/" not in file_prefixes + + def test_bounded_governance_keeps_generated_files_under_dedicated_roots(self): + from apm_cli.install.manifest_reconcile import install_governance + + target = SimpleNamespace( + name="stub", + root_dir=".stub", + primitives={"skills": SimpleNamespace(subdir="skills", extension="/SKILL.md")}, + generated_files=("index.json",), + ) + file_prefixes, _ = install_governance([target], bounded=True) + + assert file_prefixes == {".stub/skills/", ".stub/index.json"} + def test_shared_root_filename_governance_requires_exact_match(self): from apm_cli.install.manifest_reconcile import union_preserving diff --git a/tests/unit/test_security_file_scanner.py b/tests/unit/test_security_file_scanner.py index d912e24236..18abf90431 100644 --- a/tests/unit/test_security_file_scanner.py +++ b/tests/unit/test_security_file_scanner.py @@ -11,6 +11,7 @@ from apm_cli.security.file_scanner import ( _is_safe_lockfile_path, _minimal_governed_prefixes, + _scan_deployed_trees, _scan_files_in_dir, scan_deployed_trees, scan_lockfile_packages, @@ -300,6 +301,39 @@ def test_sources_under_apm_dir_are_out_of_scope(self, tmp_path: Path) -> None: def test_no_deploy_tree_scans_nothing(self, tmp_path: Path) -> None: assert scan_deployed_trees(tmp_path) == ({}, 0) + def test_supplied_scope_resolved_targets_drive_the_walk(self, tmp_path: Path) -> None: + # A global audit resolves user-scope roots; re-resolving here at + # project scope would walk `.github/` and miss `~/.copilot/`. + from dataclasses import replace + + from apm_cli.integration.targets import KNOWN_TARGETS + + user_scoped = replace(KNOWN_TARGETS["claude"], root_dir=".claude-user") + rel = ".claude-user/skills/beta/SKILL.md" + (tmp_path / rel).parent.mkdir(parents=True) + (tmp_path / rel).write_text(_BIDI_PAYLOAD, encoding="utf-8") + + result = _scan_deployed_trees(tmp_path, targets=[user_scoped]) + + assert set(result.findings_by_file) == {rel} + assert scan_deployed_trees(tmp_path) == ({}, 0) + + def test_walk_is_bounded_to_primitive_deploy_directories(self, tmp_path: Path) -> None: + # A target root also holds data APM never deploys (session + # transcripts, history, caches). Those are outside the audit's + # remit and, at user scope, gigabytes deep. + deployed = ".claude/skills/beta/SKILL.md" + (tmp_path / deployed).parent.mkdir(parents=True) + (tmp_path / deployed).write_text(_BIDI_PAYLOAD, encoding="utf-8") + for stray in (".claude/projects/session.jsonl", ".claude/history.jsonl"): + (tmp_path / stray).parent.mkdir(parents=True, exist_ok=True) + (tmp_path / stray).write_text(_BIDI_PAYLOAD, encoding="utf-8") + + findings, scanned = scan_deployed_trees(tmp_path) + + assert scanned == 1 + assert set(findings) == {deployed} + class TestProjectFileScan: """Contracts for combining recorded and governed scan scopes.""" From 42a66b517f0ff72062b81529f41792ecd0842294 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 10:04:39 +0200 Subject: [PATCH 02/12] fix(audit): separate canonical primitive discovery from prompt checks Apply the approved scope for #2961: inventory native hooks without executing commands, inspect documented prompt fields, fail incomplete coverage, and preserve configuration and ownership boundaries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../owners/hooks-integrations.json | 10 + docs/src/content/docs/enterprise/security.md | 19 +- .../content/docs/reference/baseline-checks.md | 8 +- docs/src/content/docs/reference/cli/audit.md | 57 ++- .../.apm/skills/apm-usage/commands.md | 12 + .../checks/audit_discovery.py | 81 ++++ .../groups/mutation_writes.py | 9 +- src/apm_cli/commands/audit.py | 130 +++-- .../compilation/root_context_protection.py | 6 + src/apm_cli/hook_contract.py | 44 +- src/apm_cli/integration/hook_integrator.py | 15 + .../integration/hook_native_formats.py | 144 ++++++ src/apm_cli/integration/targets.py | 11 +- src/apm_cli/policy/ci_checks.py | 39 +- src/apm_cli/security/audit_report.py | 100 +++- src/apm_cli/security/content_scanner.py | 1 + src/apm_cli/security/file_scanner.py | 429 ++++++++++++---- src/apm_cli/security/primitive_discovery.py | 169 +++++++ .../test_architecture_audit_discovery.py | 39 ++ .../test_audit_primitive_contract.py | 459 ++++++++++++++++++ ...test_audit_unrecorded_unicode_lifecycle.py | 96 ++++ .../commands/test_audit_error_handling.py | 19 +- tests/unit/commands/test_audit_phase3.py | 19 +- tests/unit/policy/test_ci_checks.py | 2 +- tests/unit/test_audit_fail_on_drift.py | 8 +- tests/unit/test_security_file_scanner.py | 19 +- 26 files changed, 1730 insertions(+), 215 deletions(-) create mode 100644 scripts/architecture_linter/checks/audit_discovery.py create mode 100644 src/apm_cli/security/primitive_discovery.py create mode 100644 tests/integration/test_architecture_audit_discovery.py create mode 100644 tests/integration/test_audit_primitive_contract.py diff --git a/.apm/architecture/owners/hooks-integrations.json b/.apm/architecture/owners/hooks-integrations.json index 9d7796e6e5..c0263f5c08 100644 --- a/.apm/architecture/owners/hooks-integrations.json +++ b/.apm/architecture/owners/hooks-integrations.json @@ -1,6 +1,16 @@ { "version": 1, "owners": [ + { + "id": "audit-primitive-discovery", + "decision": "Registry-derived deployed primitive discovery and applicable prompt coverage", + "owner": "security/primitive_discovery.py (primitive_surfaces), security/file_scanner.py (scan_project_result) through target and native hook contracts", + "selectors": [ + "src/apm_cli/security/primitive_discovery.py", + "src/apm_cli/security/file_scanner.py" + ], + "guards": ["audit-primitive-discovery"] + }, { "id": "mcp-target-selection-precedence", "decision": "MCP target-selection precedence", diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index c8e55a52fc..bc000cb167 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -197,7 +197,13 @@ This prevents lockfile membership from shrinking silently. Shared merge-hook targets and sidecars remain exempt because APM merges into user-owned files rather than claiming them. -A whole-project scan checks **every regular file under the deploy directories your targets govern** (primitive directories such as `.claude/skills/` and generated files, not the whole target root) for hidden Unicode, unioned with every file recorded in `apm.lock.yaml`, so a recorded path outside those directories such as `.claude/settings.json` stays covered. Hash verification and positional `PACKAGE` scans remain lockfile-scoped because they need recorded ownership. Source content under `.apm/` is not added by the deploy-tree walk; install-time scanning owns that surface, while any `.apm/` path already recorded in the lockfile remains covered. +A whole-project scan discovers **recognized primitive filenames and patterns** +from resolved target profiles, unioned with recorded paths in `apm.lock.yaml`. +Both paths apply the same prompt/non-prompt distinction, including shared native +settings. Hash verification and positional `PACKAGE` scans remain +lockfile-scoped. Source content under `.apm/` is not added by discovery; +install-time scanning owns that surface, while recorded prompt documents remain +covered. CI and remediation are separate commands because `--ci` and `--strip` are mutually exclusive: @@ -221,6 +227,17 @@ deployment owners; see [Baseline CI checks](../../reference/baseline-checks/#deployment-ledger-owners) for the boundary and remediation. +Automatic audit separates discovery from prompt checks. It inventories +recognized hook definitions, including untracked entries in shared native +settings, but scans only documented prompt fields and prompt documents. +Commands, executables and unrelated settings are not prompt content; hooks +are never executed. Transcripts, history and caches are not walked. +Unreadable or unsupported recognized content exits nonzero with incomplete +coverage, not an unsafe-content verdict. File tracking does not establish +entry ownership or a hash baseline. Shared/structured settings and external +roots are not automatically rewritten by `--strip`; review them manually. +See [Discovery and prompt coverage](../../reference/cli/audit/#discovery-and-prompt-coverage). + :::tip[External scanners (Experimental)] `apm audit` can also ingest findings from **third-party SARIF scanners** (Semgrep, CodeQL, NVIDIA SkillSpector, etc.) so a single audit run reports both APM's native findings and external tool results. See [External scanners](../../integrations/external-scanners/) for setup. ::: diff --git a/docs/src/content/docs/reference/baseline-checks.md b/docs/src/content/docs/reference/baseline-checks.md index 036704cf27..ab6898df14 100644 --- a/docs/src/content/docs/reference/baseline-checks.md +++ b/docs/src/content/docs/reference/baseline-checks.md @@ -130,11 +130,11 @@ the [policy schema](../policy-schema/). ### `content-integrity` -- **What it verifies.** Two signals across every deployed file (including local `.apm/` content via the synthesized self-entry), each with the scope its evidence allows: - 1. Critical hidden Unicode (tag characters, bidi overrides, variation selectors 17-256, and similar steganographic markers), over **every file under the deploy directories the project's targets govern** (each target's primitive directories and generated files, such as `.claude/skills/` and `.claude/hooks/`, not the whole `.claude/` root). This signal needs no recorded baseline, so it is deliberately not limited to `deployed_files` -- otherwise a deployed file the lockfile omits would be exempt from scanning for as long as it stayed unrecorded. The rest of a target root (session transcripts, history, caches) is never deployed by APM and is not walked. This walk is unioned with the lockfile-scoped pass, so a recorded path outside those directories (for example a claimed `.claude/settings.json`) is still scanned. +- **What it verifies.** Two independent signals, including local `.apm/` deployments recorded through the synthesized self-entry: + 1. Critical hidden Unicode in recognized prompt documents and decoded native prompt fields. Discovery uses each resolved target's filenames and primitive patterns, including untracked files and shared hook settings. Recorded paths use the same applicability rules: tracking a command or script does not turn it into prompt text. Hook definitions remain visible even when their command-only content is not applicable to Unicode prompt checks. Session transcripts, history, caches, command strings and referenced executables are not scanned. See [Discovery and prompt coverage](../cli/audit/#discovery-and-prompt-coverage) for the format boundaries. 2. SHA-256 drift between the on-disk content and the hash recorded in `deployed_file_hashes` at install time. Necessarily lockfile-scoped: an unrecorded file has no baseline to compare against. -- **Fails when.** Any deployed file contains a critical Unicode finding or its hash no longer matches the lockfile entry. Missing files are intentionally not reported here -- `deployed-files-present` owns that signal. Symlinks and entries without a recorded hash are skipped. -- **Remediation.** Run `apm audit --strip` to clean Unicode findings, and `apm install` to restore hash-drifted files. Both may be needed. +- **Fails when.** An applicable prompt contains critical Unicode, recognized content is unreadable or unsupported (incomplete coverage), a recorded hash differs, or a legacy hash lacks canonical ownership metadata. Missing files are reported by `deployed-files-present`; unrecorded files have no hash baseline. Existing path and symlink protections still apply. +- **Remediation.** Review the reported format or access error for incomplete coverage. Use `apm audit --strip` only for supported regular prompt documents; structured/shared settings and external-root findings require manual review. Run `apm install` to restore hash-drifted files. Discovery does not grant ownership or permission to rewrite user configuration. ### `includes-consent` diff --git a/docs/src/content/docs/reference/cli/audit.md b/docs/src/content/docs/reference/cli/audit.md index ca34ddcbfa..40ec1eb417 100644 --- a/docs/src/content/docs/reference/cli/audit.md +++ b/docs/src/content/docs/reference/cli/audit.md @@ -15,7 +15,7 @@ apm audit [PACKAGE] [OPTIONS] `apm audit` is the explicit security and integrity tool. It runs in two modes: -- **Content scan mode** (default). Scans deployed files across the project for hidden Unicode, including governed files absent from the lockfile and lockfile-recorded files outside the current target directories. It replays the install pipeline into a scratch tree to detect drift (hand-edits to deployed files, missing integrations, orphaned files vs the lockfile). Can also remediate findings with `--strip` or scan an arbitrary file with `--file`. +- **Content scan mode** (default). Discovers recognized deployed primitives and checks applicable prompt content for hidden Unicode, including untracked primitives and recorded files outside currently selected target directories. It replays the install pipeline into a scratch tree to detect drift (hand-edits to deployed files, missing integrations, orphaned files vs the lockfile). Can also remediate regular prompt documents with `--strip` or scan an arbitrary file with `--file`. - **CI gate mode** (`--ci`). Runs lockfile consistency checks plus drift in machine-readable form (text, JSON, or SARIF) suitable for branch-protection gates. When `apm_modules/` is absent but `apm.lock.yaml` is present, CI mode self-hydrates a lock-pinned scratch install for `config-consistency` and drift without mutating the checkout. Auto-discovers org policy from your project's git remote unless `--no-policy` is set. Global audit also checks resolved external deployment roots such as @@ -68,6 +68,13 @@ list. Deployment-owner integrity findings render in every format -- text, JSON, SARIF, and markdown -- as a `deployment-owner` category entry naming the locator, its invalid owner(s), and the remediation. +Non-CI JSON also includes `coverage.complete` and `coverage.primitives`. +Each entry identifies its file, target, primitive kind, structured pointer, +file-level `tracked` status and prompt-check status (`checked`, +`not-applicable`, or `incomplete`). Structured findings include their prompt +pointer; SARIF does not invent a source line for a decoded JSON/TOML string. +CI reports hook inventory and coverage diagnostics under `content-integrity`. + ### CI gate | Flag | Default | Description | @@ -169,6 +176,54 @@ For the full workflow, see [Enforce in CI](../../../enterprise/enforce-in-ci/). ## Behavior +### Discovery and prompt coverage + +Whole-project audit discovers recognized primitives even without a lockfile. +Discovery is not a safety verdict: command-only hooks are listed as +discovered/untracked when appropriate, without failing the prompt check. +`tracked` means a file claim exists, not that every entry in a shared file +is APM-owned or hash-verified. Hash and ownership checks remain independent. + +The target registry supplies the following project patterns. Patterns are +recursive inside their named directory; skill entry points are `SKILL.md`, +not every file in a skill bundle. + +| Target | Prompt documents/fields | Hook definitions | +|---|---|---| +| Copilot | `.github/instructions/*.instructions.md`, `prompts/*.prompt.md`, `agents/*.agent.md`, `.github/copilot-instructions.md`; shared skills | `.github/hooks/*.json`; commands are non-prompt | +| Claude | `.claude/rules/*.md`, `agents/*.md`, `commands/*.md`, `skills/**/SKILL.md` | `.claude/hooks/*.json`, `.claude/settings.json`; nested `prompt`/`agent` handlers' `prompt` field | +| Cursor | `.cursor/rules/*.mdc`, `agents/*.md`, `commands/*.md`; shared skills | `.cursor/hooks/*.json`, `.cursor/hooks.json`; flat command handlers | +| Kiro | `.kiro/steering/*.md`, `agents/*.md`, `skills/**/SKILL.md` | `.kiro/hooks/*.json`; v1 `hooks[].action.prompt` for agent actions | +| Gemini | `.gemini/commands/*.toml`: `prompt`; shared skills | `.gemini/hooks/*.json`, `.gemini/settings.json`; nested command handlers | +| Codex | `.codex/agents/*.toml`: `developer_instructions`; shared skills | `.codex/hooks.json`; nested command handlers | +| Antigravity | `.agents/rules/*.md`, skills | `.agents/hooks.json`; named containers, nested tool events and flat other events | +| Windsurf | `.windsurf/rules/*.md`, `workflows/*.md`; shared skills | `.windsurf/hooks.json`; flat command handlers | +| OpenCode | `.opencode/agents/*.md`, `commands/*.md`; shared skills | None | +| Grok Build / Cloud | `.grok/skills/**/SKILL.md`; Build also `rules/*.md`, `agents/*.md`, `commands/*.md` | None | +| Agent Skills, OpenClaw, Hermes | `.agents/skills/**/SKILL.md` | None | +| Copilot Cowork / App | Managed skills / `workflows/*.prompt.md` in their resolved native roots | None | + +Directory shorthand in a row stays under that row's target root. Shared skills +use `.agents/skills/**/SKILL.md`. A target's generated context files and +compile-family root file (`AGENTS.md`, `CLAUDE.md`, or `GEMINI.md`) are also +recognized. Global audit uses scope-resolved profiles instead: for example +Copilot's root becomes `~/.copilot` with the exact `copilot-instructions.md` +aggregate, and Claude/Hermes respect their configured external roots. +Primitives unsupported in user scope are not discovered there. + +Only documented prompt fields are decoded and checked in structured formats. +Command strings, referenced scripts, unrelated settings, ownership sidecars, +transcripts, history and caches are excluded from prompt scanning; hooks are +never executed. A recorded script remains non-prompt. Recognized unreadable, +malformed or unsupported content produces **incomplete coverage and exit 1**, +not a malicious-content finding or a clean result. This also applies with +`--no-drift` and `--no-policy`. + +`--strip` refuses automatic remediation when findings involve structured/shared +configuration or external-root content, or coverage is incomplete; it leaves +all files unchanged. Review those files manually. Explicit `--file` retains +its user-directed whole-file scan and remediation behavior. + ### Severity levels (content scan) | Severity | Examples | Effect | diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index af16b03a24..2699dbdc14 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -253,6 +253,18 @@ descendants, are skipped. ## Security and audit +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, +unrelated settings, transcripts or caches. Command-only hooks remain visible +and non-failing; hooks are never executed. Unreadable or unsupported recognized +content fails with incomplete coverage, including with `--no-drift`/`--no-policy`. +Non-CI JSON exposes `coverage.complete` and `coverage.primitives`; CI includes +inventory and diagnostics under `content-integrity`. Tracking is file-level, +not entry ownership or hash verification. Automatic `--strip` refuses +structured/shared or external-root findings and incomplete coverage without +rewriting files. Explicit `--file` remains user-directed. + | Command | Purpose | Key flags | |---------|---------|-----------| | `apm audit [PKG]` | Scan installed primitives for hidden Unicode, drift, and lockfile/policy violations | `--file PATH`, `--strip`, `--dry-run`, `-v`, `-f [text\|json\|sarif\|md]`, `-o PATH`, `--ci`, `--policy SOURCE`, `--no-cache`, `--no-fail-fast`, `--no-drift`, `--external NAME` (experimental; ingest a third-party SARIF scanner, e.g. `skillspector`), `--external-sarif PATH`, `--external-llm/--no-external-llm`, `--external-args TEXT` | diff --git a/scripts/architecture_linter/checks/audit_discovery.py b/scripts/architecture_linter/checks/audit_discovery.py new file mode 100644 index 0000000000..2c4a807fda --- /dev/null +++ b/scripts/architecture_linter/checks/audit_discovery.py @@ -0,0 +1,81 @@ +"""Keep deployed audit discovery and content applicability on canonical owners.""" + +from __future__ import annotations + +from collections.abc import Iterable + +from scripts.architecture_linter.facts import FactsProvider +from scripts.architecture_linter.groups.common import checked_facts, violation +from scripts.architecture_linter.models import Rule, Violation + +RULE_ID = "audit-primitive-discovery" +_SCANNER = "src/apm_cli/security/file_scanner.py" +_DISCOVERY = "src/apm_cli/security/primitive_discovery.py" +_REQUIRED = { + _SCANNER: { + "_scan_claimed_files": {"primitive_surfaces", "_scan_primitive"}, + "_scan_deployed_trees": {"primitive_surfaces", "iter_surface_files", "_scan_primitive"}, + "_content_entries": {"inspect_native_hooks"}, + "_scan_primitive": {"safe_surface_path"}, + "scan_project_result": {"_scan_claimed_files", "_scan_deployed_trees"}, + }, + _DISCOVERY: { + "primitive_surfaces": {"native_hook_config", "_mapping_surface", "root_context_filename"}, + "safe_surface_path": {"ensure_path_within", "has_symlink_component"}, + }, + "src/apm_cli/integration/hook_native_formats.py": { + "inspect_native_hooks": {"hook_handlers"}, + }, + "src/apm_cli/commands/audit.py": {"_audit_content_scan": {"scan_project_result"}}, + "src/apm_cli/policy/ci_checks.py": {"_check_content_integrity": {"scan_project_result"}}, +} + + +def check_audit_discovery(provider: FactsProvider) -> Iterable[Violation]: + """Require the shared discovery/applicability route in both audit consumers.""" + for path, functions in _REQUIRED.items(): + facts, failures = checked_facts(provider, path, RULE_ID, require_python=True) + yield from failures + for function, required in functions.items(): + calls = { + call.qualname.rsplit(".", 1)[-1] + for call in facts.calls + if call.scope.rsplit(".", 1)[-1] == function + } + for name in sorted(required - calls): + yield violation(RULE_ID, path, f"{function} must route through canonical {name}.") + if path == _SCANNER: + for call in facts.calls: + if call.scope.rsplit(".", 1)[-1] in { + "_scan_deployed_trees", + "_scan_claimed_files", + "scan_project_result", + } and call.qualname.rsplit(".", 1)[-1] in {"scan_files", "scan_file", "walk"}: + yield violation( + RULE_ID, + path, + "Automatic audit must not bypass primitive applicability with a raw file/tree scan.", + line=call.line, + ) + if path in {_SCANNER, _DISCOVERY}: + for literal in facts.literals: + if literal.value_repr.strip("'\"").startswith( + (".claude/", ".codex/", ".github/", ".cursor/") + ): + yield violation( + RULE_ID, + path, + "Audit target paths must derive from target/hook registries.", + line=literal.line, + ) + + +RULES = ( + Rule( + id=RULE_ID, + group="mutation_writes", + guard_ids=(RULE_ID,), + description="Deployed primitive discovery and applicable checks share registry-derived owners.", + check=check_audit_discovery, + ), +) diff --git a/scripts/architecture_linter/groups/mutation_writes.py b/scripts/architecture_linter/groups/mutation_writes.py index 6f851105fe..04c7de8490 100644 --- a/scripts/architecture_linter/groups/mutation_writes.py +++ b/scripts/architecture_linter/groups/mutation_writes.py @@ -5,6 +5,7 @@ helper module, so no single check module outgrows the module size budget. """ +from scripts.architecture_linter.checks.audit_discovery import RULES as _AUDIT_RULES from scripts.architecture_linter.checks.mutation_hook_contract import ( COLLECTORS as _HOOK_CONTRACT_COLLECTORS, ) @@ -23,7 +24,13 @@ ) from scripts.architecture_linter.checks.onboarding import RULES as _ONBOARDING_RULES -RULES = _HOOK_CONTRACT_RULES + _MCP_SCOPE_RULES + _HOOK_MEMBERSHIP_RULES + _ONBOARDING_RULES +RULES = ( + _HOOK_CONTRACT_RULES + + _MCP_SCOPE_RULES + + _HOOK_MEMBERSHIP_RULES + + _ONBOARDING_RULES + + _AUDIT_RULES +) COLLECTORS = _HOOK_CONTRACT_COLLECTORS + _MCP_SCOPE_COLLECTORS + _HOOK_MEMBERSHIP_COLLECTORS __all__ = ["COLLECTORS", "RULES"] diff --git a/src/apm_cli/commands/audit.py b/src/apm_cli/commands/audit.py index ab1030dfd3..88159d99ff 100644 --- a/src/apm_cli/commands/audit.py +++ b/src/apm_cli/commands/audit.py @@ -26,7 +26,7 @@ from ..install.locking import serialized_lifecycle_when from ..policy._help_text import POLICY_SOURCE_FORMS_HELP from ..security.content_scanner import ContentScanner, ScanFinding -from ..security.file_scanner import scan_project_files +from ..security.file_scanner import CoverageEntry, scan_project_result from ..utils.console import ( STATUS_SYMBOLS, _get_console, @@ -285,6 +285,8 @@ def _render_summary( findings_by_file: dict[str, list[ScanFinding]], files_scanned: int, logger, + *, + coverage: tuple[CoverageEntry, ...] = (), ) -> None: """Render a summary panel with counts.""" all_findings: list[ScanFinding] = [] @@ -303,7 +305,11 @@ def _render_summary( f"{critical} critical finding(s) in {affected} file(s) -- hidden characters detected" ) logger.progress(" These characters may embed invisible instructions") - logger.progress(" Review file contents, then run 'apm audit --strip' to remove") + logger.progress( + "Review structured prompt fields manually; --strip does not rewrite native settings." + if any(not entry.strippable and entry.status == "checked" for entry in coverage) + else " Review file contents, then run 'apm audit --strip' to remove" + ) elif warning > 0: logger.warning(f"{warning} warning(s) in {affected} file(s) -- hidden characters detected") logger.progress(" Run 'apm audit --strip' to remove hidden characters") @@ -312,13 +318,32 @@ def _render_summary( f"{info} info-level finding(s) in " f"{affected} file(s) -- unusual characters (use --verbose to see)" ) - else: + elif not any(entry.status == "incomplete" for entry in coverage): logger.success(f"{files_scanned} file(s) scanned -- no issues found") if info > 0 and (critical > 0 or warning > 0): logger.progress(f" Plus {info} info-level finding(s) (use --verbose to see)") +def _render_coverage(coverage: tuple[CoverageEntry, ...], logger: CommandLogger) -> None: + """Keep discovery visible without inventing ownership or unsafe-content findings.""" + from ..utils.diagnostics import printable_ascii_text + + for entry in coverage: + label = printable_ascii_text(entry.file + entry.pointer) + if entry.status == "incomplete": + logger.error( + f"Incomplete coverage: {label}: {entry.diagnostic}. " + "Review the format or file access and rerun audit." + ) + elif entry.kind == "hooks": + tracking = "recorded-file" if entry.tracked else "untracked" + logger.progress( + f"Discovered/{tracking}: {label} (prompt check: {entry.status}; " + "ownership and hash verification are separate)" + ) + + def _render_owner_violations( violations: tuple[DeploymentOwnerViolation, ...], logger: CommandLogger, @@ -892,6 +917,8 @@ def _audit_content_scan( """ logger = cfg.logger project_root = cfg.project_root + coverage: tuple[CoverageEntry, ...] = () + protected_files: frozenset[str] = frozenset() # Resolve effective format (auto-detect from extension when needed) effective_format = cfg.output_format @@ -915,51 +942,44 @@ def _audit_content_scan( scan_paths = [project_root] # -- Package mode: scan from lockfile -- lockfile_path = get_lockfile_path(project_root) - if not lockfile_path.exists(): - if not external: - logger.progress( - "No apm.lock.yaml found -- nothing to scan. Use --file to scan a specific file." - ) - sys.exit(0) - # External scanners are an independent source: proceed with an - # empty native result set so their findings still surface. - findings_by_file, files_scanned = {}, 0 - else: - if effective_format == "text": - if package: - logger.progress(f"Scanning package: {package}") - else: - logger.start("Scanning installed packages and deployed files...") + if effective_format == "text": + if package: + logger.progress(f"Scanning package: {package}") + else: + logger.start("Scanning installed packages and deployed files...") - from apm_cli.deps.lockfile import LockfileFormatError + from apm_cli.deps.lockfile import LockfileFormatError - try: - lockfile = LockFile.read(lockfile_path) - owner_violations = ( - DeploymentLedgerCodec.owner_reference_violations(lockfile) - if lockfile is not None - else () - ) - findings_by_file, files_scanned = scan_project_files( - project_root, - package_filter=package, - lockfile=lockfile, - include_deployed_trees=package is None, - ) - except LockfileFormatError as exc: - logger.error(f"Cannot audit invalid apm.lock.yaml: {exc}") - sys.exit(1) + try: + lockfile = LockFile.read(lockfile_path) + owner_violations = ( + DeploymentLedgerCodec.owner_reference_violations(lockfile) + if lockfile is not None + else () + ) + scan = scan_project_result( + project_root, + package_filter=package, + lockfile=lockfile, + include_deployed_trees=package is None, + ) + findings_by_file = scan.findings_by_file + files_scanned = len(scan.scanned_files) + coverage = scan.inventory + protected_files = scan.protected_files + except LockfileFormatError as exc: + logger.error(f"Cannot audit invalid apm.lock.yaml: {exc}") + sys.exit(1) - if files_scanned == 0 and not external and not owner_violations: - if package: - logger.warning( - f"Package '{package}' not found in apm.lock.yaml or has no deployed files" - ) - else: - logger.progress("No deployed files found") + if files_scanned == 0 and not coverage and not external and not owner_violations: + if package: + logger.warning( + f"Package '{package}' not found in apm.lock.yaml or has no deployed files" + ) + elif effective_format == "text": + logger.progress("No recognized deployed primitives found -- nothing to scan") + if effective_format == "text": sys.exit(0) - if not lockfile_path.exists(): - owner_violations = () # -- External scanners (opt-in, additive) ----------------------- if external: @@ -977,6 +997,14 @@ def _audit_content_scan( # -- Strip mode -- if strip: + blocked = protected_files.intersection(findings_by_file) + if blocked or any(entry.status == "incomplete" for entry in coverage): + _render_coverage(coverage, logger) + logger.error( + "Content was not modified: structured/shared or external prompt findings, or incomplete coverage, " + "require manual review. --strip does not rewrite native configuration." + ) + sys.exit(1) if owner_violations: _render_owner_violations(owner_violations, logger) logger.error_detail("Content was not modified while lockfile ownership is invalid.") @@ -987,7 +1015,9 @@ def _audit_content_scan( if dry_run: _preview_strip(findings_by_file, logger) sys.exit(0) - modified = _apply_strip(findings_by_file, project_root, logger) + from ..core.scope import get_workspace_deploy_root + + modified = _apply_strip(findings_by_file, get_workspace_deploy_root(project_root), logger) if modified > 0: logger.success(f"Cleaned {modified} file(s)") else: @@ -1057,7 +1087,7 @@ def _audit_content_scan( else: all_findings = [f for ff in findings_by_file.values() for f in ff] exit_code = 1 if ContentScanner.has_critical(all_findings) else 2 - if owner_violations: + if owner_violations or any(entry.status == "incomplete" for entry in coverage): exit_code = 1 # Bare `apm audit` is advisory for drift by default: drift findings are @@ -1081,9 +1111,10 @@ def _audit_content_scan( sys.exit(1) if findings_by_file: _render_findings_table(findings_by_file, verbose=cfg.verbose) - _render_summary(findings_by_file, files_scanned, logger) + _render_summary(findings_by_file, files_scanned, logger, coverage=coverage) elif not owner_violations: - _render_summary(findings_by_file, files_scanned, logger) + _render_summary(findings_by_file, files_scanned, logger, coverage=coverage) + _render_coverage(coverage, logger) _render_owner_violations(owner_violations, logger) if not file_path: _render_canvas_note(cfg.project_root, package, logger) @@ -1099,6 +1130,7 @@ def _audit_content_scan( findings_by_file, files_scanned=files_scanned, owner_violations=owner_violations, + coverage=coverage, ) if cfg.output_path: Path(cfg.output_path).parent.mkdir(parents=True, exist_ok=True) @@ -1119,6 +1151,7 @@ def _audit_content_scan( findings_by_file, files_scanned=files_scanned, owner_violations=owner_violations, + coverage=coverage, ) else: report = findings_to_json( @@ -1126,6 +1159,7 @@ def _audit_content_scan( files_scanned=files_scanned, exit_code=exit_code, owner_violations=owner_violations, + coverage=coverage, ) if cfg.output_path: diff --git a/src/apm_cli/compilation/root_context_protection.py b/src/apm_cli/compilation/root_context_protection.py index be2935d77d..b533f3afcb 100644 --- a/src/apm_cli/compilation/root_context_protection.py +++ b/src/apm_cli/compilation/root_context_protection.py @@ -31,6 +31,12 @@ } +def root_context_filename(compile_family: str | None) -> str | None: + """Return the known context filename for one catalog compile family.""" + contract = _ROOT_CONTEXT_BY_COMPILE_FAMILY.get(compile_family) + return contract[0] if contract is not None else None + + def catalog_root_context_markers() -> dict[str, tuple[str, ...]]: """Return generated root context filenames implied by the target catalog.""" markers_by_name: dict[str, tuple[str, ...]] = {} diff --git a/src/apm_cli/hook_contract.py b/src/apm_cli/hook_contract.py index 79d744cc19..284d46845e 100644 --- a/src/apm_cli/hook_contract.py +++ b/src/apm_cli/hook_contract.py @@ -84,6 +84,20 @@ class HookSourceDocument: commands: tuple[HookCommandDeclaration, ...] +@dataclass(frozen=True) +class HookHandlerDeclaration: + """One handler in the canonical flat or nested hook grammar.""" + + event: str + value: Mapping[str, Any] + json_pointer: str + + +def hook_handlers(document: object) -> tuple[HookHandlerDeclaration, ...]: + """Validate the source grammar and expose handlers without executing them.""" + return _walk_hook_handlers(document, strict=True) + + def parse_hook_source(document: object) -> HookSourceDocument: """Validate wrapped or naked hook event maps and locate command fields.""" return HookSourceDocument(commands=_walk_hook_commands(document, strict=True)) @@ -100,6 +114,21 @@ def _walk_hook_commands( strict: bool, ) -> tuple[HookCommandDeclaration, ...]: """Walk one hook source through the canonical strict or tolerant grammar.""" + return tuple( + command + for handler in _walk_hook_handlers(document, strict=strict) + for command in _command_declarations( + handler.event, dict(handler.value), handler.json_pointer, strict=strict + ) + ) + + +def _walk_hook_handlers( + document: object, + *, + strict: bool, +) -> tuple[HookHandlerDeclaration, ...]: + """Locate handlers once for executable intent and native content readers.""" if not isinstance(document, dict): if strict: raise HookContractError("document must be a JSON object") @@ -119,7 +148,7 @@ def _walk_hook_commands( raise HookContractError("hooks must be a JSON object") return () - commands: list[HookCommandDeclaration] = [] + handlers: list[HookHandlerDeclaration] = [] for event, entries in hooks.items(): if not isinstance(event, str) or not isinstance(entries, list): if strict: @@ -133,7 +162,7 @@ def _walk_hook_commands( raise HookContractError(f"hook event {event!r} entries must be objects") continue entry_pointer = f"{event_pointer}/{entry_index}" - commands.extend(_command_declarations(event, entry, entry_pointer, strict=strict)) + handlers.append(HookHandlerDeclaration(event, entry, entry_pointer)) nested = entry.get("hooks") if nested is None: continue @@ -150,15 +179,10 @@ def _walk_hook_commands( f"hook event {event!r} nested handlers must be objects" ) continue - commands.extend( - _command_declarations( - event, - handler, - f"{entry_pointer}/hooks/{handler_index}", - strict=strict, - ) + handlers.append( + HookHandlerDeclaration(event, handler, f"{entry_pointer}/hooks/{handler_index}") ) - return tuple(commands) + return tuple(handlers) def _command_declarations( diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index f1f435e35c..fbc3d0517a 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -167,6 +167,9 @@ class _MergeHookConfig: # overwritten -- the guard in _integrate_merged_hooks() preserves any # value the user has set manually. top_level_defaults: dict[str, Any] = field(default_factory=dict) + prompt_handler_types: tuple[str, ...] = () + named_containers: bool = False + nested_handlers: bool | None = None # Per-target hook event name mapping. Packages are authored with @@ -337,39 +340,51 @@ def _validate_copilot_payload(payload: dict) -> list[str]: target_key="claude", require_dir=False, schema_strict=True, + prompt_handler_types=("prompt", "agent"), + nested_handlers=True, ), "cursor": _MergeHookConfig( config_filename="hooks.json", target_key="cursor", require_dir=True, top_level_defaults={"version": 1}, + nested_handlers=False, ), "codex": _MergeHookConfig( config_filename="hooks.json", target_key="codex", require_dir=True, + nested_handlers=True, ), "gemini": _MergeHookConfig( config_filename="settings.json", target_key="gemini", require_dir=True, + nested_handlers=True, ), "antigravity": _MergeHookConfig( config_filename="hooks.json", target_key="antigravity", require_dir=True, event_container_key="apm", + named_containers=True, ), "windsurf": _MergeHookConfig( config_filename="hooks.json", target_key="windsurf", require_dir=True, + nested_handlers=False, ), } _APM_HOOKS_SIDECAR = "apm-hooks.json" +def native_hook_config(target_name: str) -> _MergeHookConfig | None: + """Return the canonical native container and content contract for a target.""" + return _MERGE_HOOK_TARGETS.get(target_name) + + class HookIntegrator(BaseIntegrator): """Handles integration of APM package hooks into target locations. diff --git a/src/apm_cli/integration/hook_native_formats.py b/src/apm_cli/integration/hook_native_formats.py index 185736e0d9..994c12fa7c 100644 --- a/src/apm_cli/integration/hook_native_formats.py +++ b/src/apm_cli/integration/hook_native_formats.py @@ -2,18 +2,162 @@ from __future__ import annotations +from dataclasses import dataclass from typing import Any from apm_cli.hook_contract import ( + HOOK_COMMAND_KEYS, + HookContractError, HookDocument, HookHandler, _entries_to_ir, _handler_to_ir, + hook_handlers, ) _ANTIGRAVITY_NESTED_EVENTS: frozenset[str] = frozenset({"PreToolUse", "PostToolUse"}) +@dataclass(frozen=True) +class HookContentEntry: + """Discovered native hook content, independent of ownership and safety.""" + + pointer: str + prompt: str | None = None + error: str | None = None + + +def _inspect_handler( + value: dict[str, Any], + pointer: str, + prompt_types: tuple[str, ...], + command_keys: tuple[str, ...] = ("command",), +) -> HookContentEntry: + """Classify one documented handler without inspecting command text.""" + kind = value.get("type", "command") + if kind in prompt_types: + prompt = value.get("prompt") + if isinstance(prompt, str): + return HookContentEntry(f"{pointer}/prompt", prompt=prompt) + return HookContentEntry(pointer, error="prompt handler requires a string prompt") + if kind == "command": + commands = [value[key] for key in command_keys if key in value] + if commands and all(isinstance(command, str) for command in commands): + return HookContentEntry(pointer) + return HookContentEntry(pointer, error="command handler requires string command fields") + return HookContentEntry(pointer, error="unsupported native hook handler type") + + +def inspect_native_hooks( + document: object, + format_id: str, + *, + container: str = "hooks", + prompt_types: tuple[str, ...] = (), + named_containers: bool = False, + shared: bool = False, + nested_handlers: bool | None = None, +) -> tuple[HookContentEntry, ...]: + """Read only documented native hook containers and prompt-bearing fields.""" + if not isinstance(document, dict): + return (HookContentEntry("", error="hook document must be an object"),) + if format_id == "kiro_hooks": + if document.get("version") != "v1" or not isinstance(document.get("hooks"), list): + return (HookContentEntry("", error="unsupported Kiro hook document"),) + result = [] + for index, hook in enumerate(document["hooks"]): + pointer = f"/hooks/{index}" + if ( + not isinstance(hook, dict) + or not isinstance(hook.get("trigger"), str) + or not isinstance(hook.get("action"), dict) + ): + result.append(HookContentEntry(pointer, error="invalid Kiro hook action")) + else: + result.append(_inspect_handler(hook["action"], f"{pointer}/action", ("agent",))) + return tuple(result) + if format_id not in { + "github_hooks", + "claude_hooks", + "cursor_hooks", + "codex_hooks", + "gemini_hooks", + "antigravity_hooks", + "windsurf_hooks", + }: + return (HookContentEntry("", error="unsupported native hook format"),) + if format_id in {"github_hooks", "cursor_hooks"}: + version = document.get("version", 1) + if type(version) is not int or version != 1: + return (HookContentEntry("", error="unsupported native hook version"),) + if named_containers: + containers = [ + (f"/{name.replace('~', '~0').replace('/', '~1')}", value) + for name, value in document.items() + if name != "version" + ] + elif container in document: + containers = [(f"/{container}", document[container])] + elif shared: + return () + else: + return (HookContentEntry("", error="missing native hook container"),) + result: list[HookContentEntry] = [] + for prefix, events in containers: + try: + handlers = hook_handlers({"hooks": events}) + except HookContractError as exc: + # Shape errors can include user-controlled event names. The diagnostic + # identifies the container instead of echoing its contents. + result.append( + HookContentEntry(prefix, error=f"invalid hook container ({type(exc).__name__})") + ) + continue + for handler in handlers: + nested = ( + handler.event in _ANTIGRAVITY_NESTED_EVENTS if named_containers else nested_handlers + ) + is_child = "/hooks/" in handler.json_pointer.removeprefix("/hooks/") + if not is_child and nested is True and not isinstance(handler.value.get("hooks"), list): + result.append( + HookContentEntry( + prefix + handler.json_pointer.removeprefix("/hooks"), + error="native hook event requires nested handlers", + ) + ) + continue + if not is_child and nested is False and "hooks" in handler.value: + result.append( + HookContentEntry( + prefix + handler.json_pointer.removeprefix("/hooks"), + error="native hook event requires flat handlers", + ) + ) + continue + if is_child and nested is False: + continue + if is_child and "hooks" in handler.value: + result.append( + HookContentEntry( + prefix + handler.json_pointer.removeprefix("/hooks"), + error="unsupported additional hook nesting", + ) + ) + continue + if isinstance(handler.value.get("hooks"), list): + continue + pointer = prefix + handler.json_pointer.removeprefix("/hooks") + result.append( + _inspect_handler( + dict(handler.value), + pointer, + prompt_types, + HOOK_COMMAND_KEYS if format_id == "github_hooks" else ("command",), + ) + ) + return tuple(result) + + def _handler_from_ir(handler: HookHandler, *, timeout_milliseconds: bool) -> dict[str, Any]: """Render a portable handler into one native command object.""" result = dict(handler.metadata) diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index a327df2164..bf5b2e976b 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -97,6 +97,9 @@ class PrimitiveMapping: ``format_id`` to perform the transform. """ + prompt_fields: tuple[str, ...] = () + """Documented prompt-bearing fields in structured native primitives.""" + def __post_init__(self) -> None: """Keep ``output_compare`` and :data:`RULE_FORMATS` in lockstep. @@ -686,7 +689,9 @@ def _encode_copilot_app_locator(path: Path) -> str: capability=TARGET_CAPABILITIES["gemini"], root_dir=".gemini", primitives={ - "commands": PrimitiveMapping("commands", ".toml", "gemini_command"), + "commands": PrimitiveMapping( + "commands", ".toml", "gemini_command", prompt_fields=("prompt",) + ), "skills": PrimitiveMapping( "skills", "/SKILL.md", @@ -785,7 +790,9 @@ def _encode_copilot_app_locator(path: Path) -> str: capability=TARGET_CAPABILITIES["codex"], root_dir=".codex", primitives={ - "agents": PrimitiveMapping("agents", ".toml", "codex_agent"), + "agents": PrimitiveMapping( + "agents", ".toml", "codex_agent", prompt_fields=("developer_instructions",) + ), "skills": PrimitiveMapping( "skills", "/SKILL.md", diff --git a/src/apm_cli/policy/ci_checks.py b/src/apm_cli/policy/ci_checks.py index 5fa57f89f1..e28849ea02 100644 --- a/src/apm_cli/policy/ci_checks.py +++ b/src/apm_cli/policy/ci_checks.py @@ -490,6 +490,8 @@ def _check_content_integrity( project_root: Path, lock: LockFile, targets: Sequence[TargetProfile] = (), + *, + user_scope: bool = False, ) -> CheckResult: """Check deployed files for critical hidden Unicode and hash drift. @@ -509,18 +511,26 @@ def _check_content_integrity( ownership is rooted in resolved, managed paths. Lockfile entries without a recorded hash (e.g. directories) are skipped silently. """ - from ..security.file_scanner import scan_project_files + from ..security.file_scanner import scan_project_result from ..utils.content_hash import compute_file_hash # Reuse the already-parsed lock and union its recorded paths with the # independently governed deploy-tree scope. The scanner owns exact path # accounting and preserves lockfile findings outside resolved targets. - findings_by_file, _files_scanned = scan_project_files( + scan = scan_project_result( project_root, lockfile=lock, include_deployed_trees=True, targets=targets, + user_scope=user_scope, ) + findings_by_file = scan.findings_by_file + inventory_details = [ + f"discovered/{'recorded-file' if entry.tracked else 'untracked'}: " + f"{entry.file}{entry.pointer} (prompt check: {entry.status})" + for entry in scan.inventory + if entry.kind == "hooks" + ] # Only critical findings fail this check critical_files: list[str] = [] @@ -606,14 +616,18 @@ def _has_symlink_component(path: Path) -> bool: and not hash_mismatches and not missing_ownership and not unresolved_hash_paths + and not scan.incomplete ): return CheckResult( name="content-integrity", passed=True, message="No critical hidden Unicode or hash drift detected", + details=inventory_details, ) - details: list[str] = [] + details: list[str] = list(inventory_details) + for entry in scan.incomplete: + details.append(f"incomplete-coverage: {entry.file}{entry.pointer}: {entry.diagnostic}") for rel_path in critical_files: details.append(f"unicode: {rel_path}") for rel_path in missing_ownership: @@ -633,9 +647,16 @@ def _has_symlink_component(path: Path) -> bool: parts: list[str] = [] remedies: list[str] = [] + if scan.incomplete: + parts.append(f"{len(scan.incomplete)} primitive(s) with incomplete prompt coverage") + remedies.append("review the reported format or file access and rerun audit") if critical_files: parts.append(f"{len(critical_files)} file(s) with critical hidden Unicode") - remedies.append("'apm audit --strip' to clean Unicode") + remedies.append( + "review structured prompt fields manually" + if scan.protected_files.intersection(critical_files) + else "'apm audit --strip' to clean Unicode" + ) if hash_mismatches: parts.append(f"{len(hash_mismatches)} file(s) with hash drift") remedies.append("'apm install' to restore drifted files") @@ -796,6 +817,7 @@ def _check_drift( project_root, user_scope=user_scope, explicit_target=_read_apm_yml_target(project_root), + create_config=False, ) tracked_files = None else: @@ -933,6 +955,11 @@ def run_baseline_checks( ), ) ) + result.checks.append( + _check_content_integrity( + deployment_root, LockFile(), resolved_targets, user_scope=user_scope + ) + ) return result lock = LockFile.read(lockfile_path) @@ -988,7 +1015,9 @@ def _run(check: CheckResult) -> bool: return result # Check 8: Content integrity - if _run(_check_content_integrity(deployment_root, lock, resolved_targets)): + if _run( + _check_content_integrity(deployment_root, lock, resolved_targets, user_scope=user_scope) + ): return result # Check 9: Includes consent (advisory; never hard-fails) diff --git a/src/apm_cli/security/audit_report.py b/src/apm_cli/security/audit_report.py index cf6ade9fd0..0c014fc49f 100644 --- a/src/apm_cli/security/audit_report.py +++ b/src/apm_cli/security/audit_report.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from dataclasses import asdict from pathlib import Path from typing import TYPE_CHECKING, Any @@ -11,6 +12,7 @@ if TYPE_CHECKING: from ..core.deployment_ledger import DeploymentOwnerViolation + from .file_scanner import CoverageEntry def relative_path_for_report(file_path: str) -> str: @@ -84,6 +86,7 @@ def findings_to_json( files_scanned: int, exit_code: int, owner_violations: tuple[DeploymentOwnerViolation, ...] = (), + coverage: tuple[CoverageEntry, ...] = (), ) -> dict: """Convert scan findings to APM's JSON report format.""" all_findings = [f for ff in findings_by_file.values() for f in ff] @@ -109,23 +112,31 @@ def findings_to_json( "codepoint": finding.codepoint, "category": finding.category, "description": finding.description, + **({"pointer": finding.pointer} if finding.pointer else {}), } ) items.extend(_owner_json(violation) for violation in owner_violations) - return { + report = { "version": "1", "passed": exit_code == 0, "exit_code": exit_code, "summary": summary, "findings": items, } + if coverage: + report["coverage"] = { + "complete": not any(entry.status == "incomplete" for entry in coverage), + "primitives": [asdict(entry) for entry in coverage], + } + return report def findings_to_sarif( findings_by_file: dict[str, list[ScanFinding]], files_scanned: int, owner_violations: tuple[DeploymentOwnerViolation, ...] = (), + coverage: tuple[CoverageEntry, ...] = (), ) -> dict: """Convert scan findings to SARIF 2.1.0 format. @@ -172,16 +183,23 @@ def findings_to_sarif( "artifactLocation": { "uri": relative_path_for_report(finding.file), }, - "region": { - "startLine": finding.line, - "startColumn": finding.column, - }, + **( + { + "region": { + "startLine": finding.line, + "startColumn": finding.column, + } + } + if not finding.pointer + else {} + ), } } ], "properties": { "codepoint": finding.codepoint, "category": finding.category, + **({"pointer": finding.pointer} if finding.pointer else {}), }, } results.append(result) @@ -219,6 +237,30 @@ def findings_to_sarif( } ) + coverage_errors = [entry for entry in coverage if entry.status == "incomplete"] + if coverage_errors: + seen_rules["apm/audit/incomplete-coverage"] = { + "id": "apm/audit/incomplete-coverage", + "shortDescription": {"text": "Incomplete primitive prompt coverage"}, + } + for entry in coverage_errors: + results.append( + { + "ruleId": "apm/audit/incomplete-coverage", + "level": "error", + "message": { + "text": f"{entry.diagnostic}; review format or file access and rerun audit." + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": {"uri": relative_path_for_report(entry.file)}, + } + } + ], + "properties": {"pointer": entry.pointer}, + } + ) return { "$schema": _SARIF_SCHEMA, "version": _SARIF_VERSION, @@ -234,9 +276,14 @@ def findings_to_sarif( "results": results, "invocations": [ { - "executionSuccessful": True, + "executionSuccessful": not coverage_errors, "properties": { "filesScanned": files_scanned, + **( + {"primitiveCoverage": [asdict(entry) for entry in coverage]} + if coverage + else {} + ), }, } ], @@ -263,6 +310,7 @@ def findings_to_markdown( findings_by_file: dict[str, list[ScanFinding]], files_scanned: int, owner_violations: tuple[DeploymentOwnerViolation, ...] = (), + coverage: tuple[CoverageEntry, ...] = (), ) -> str: """Convert scan findings to GitHub-Flavored Markdown. @@ -270,11 +318,35 @@ def findings_to_markdown( """ all_findings = [f for ff in findings_by_file.values() for f in ff] - if not all_findings and not owner_violations: + coverage_lines: list[str] = [] + if coverage: + coverage_lines = [ + "", + "### Primitive coverage", + "", + "Discovery does not establish ownership or verify a hash.", + "", + "| File / field | Tracking | Prompt check | Detail |", + "|--------------|----------|--------------|--------|", + ] + for entry in coverage: + values = ( + entry.file + entry.pointer, + "recorded-file" if entry.tracked else "untracked", + entry.status, + entry.diagnostic or "", + ) + coverage_lines.append( + "| " + + " | ".join(value.replace("|", "\\|").replace("\n", " ") for value in values) + + " |" + ) + incomplete = any(entry.status == "incomplete" for entry in coverage) + if not all_findings and not owner_violations and not incomplete: return ( f"## APM Audit Report\n\n" f"**Clean** - no security findings across {files_scanned} files.\n" - ) + ) + "\n".join(coverage_lines) critical = sum(1 for f in all_findings if f.severity == "critical") + len(owner_violations) warning = sum(1 for f in all_findings if f.severity == "warning") @@ -307,6 +379,13 @@ def findings_to_markdown( "", summary, ] + if incomplete: + lines.extend( + [ + "", + "**Incomplete coverage** - review the reported format or file access and rerun audit.", + ] + ) if owner_violations: lines.extend( [ @@ -345,9 +424,12 @@ def findings_to_markdown( lines.extend( [ "", - "Run `apm audit --strip` to remove flagged characters.", + "Review structured prompt fields manually; use `apm audit --strip` only for eligible regular documents." + if any(not entry.strippable and entry.status == "checked" for entry in coverage) + else "Run `apm audit --strip` to remove flagged characters.", ] ) + lines.extend(coverage_lines) lines.append("") return "\n".join(lines) diff --git a/src/apm_cli/security/content_scanner.py b/src/apm_cli/security/content_scanner.py index 6e55fc17b0..ed8f21bfb3 100644 --- a/src/apm_cli/security/content_scanner.py +++ b/src/apm_cli/security/content_scanner.py @@ -27,6 +27,7 @@ class ScanFinding: severity: str # "critical", "warning", "info" category: str # e.g. "tag-character", "bidi-override", "zero-width" description: str + pointer: str | None = None # Each entry: (range_start, range_end, severity, category, description) diff --git a/src/apm_cli/security/file_scanner.py b/src/apm_cli/security/file_scanner.py index b3b3b17299..b2cb3cc8c4 100644 --- a/src/apm_cli/security/file_scanner.py +++ b/src/apm_cli/security/file_scanner.py @@ -15,15 +15,40 @@ from __future__ import annotations -from collections.abc import Sequence -from dataclasses import dataclass +import json +from collections.abc import Iterable, Sequence +from dataclasses import dataclass, replace from pathlib import Path, PurePosixPath +from typing import Literal + +import tomllib from ..deps.lockfile import LockFile, get_lockfile_path from ..integration.base_integrator import BaseIntegrator +from ..integration.hook_integrator import native_hook_config +from ..integration.hook_native_formats import HookContentEntry, inspect_native_hooks from ..integration.targets import TargetProfile from ..security.content_scanner import ContentScanner, ScanFinding -from ..utils.path_security import PathTraversalError, ensure_path_within +from .primitive_discovery import ( + PrimitiveSurface, + iter_surface_files, + primitive_surfaces, + safe_surface_path, +) + + +@dataclass(frozen=True) +class CoverageEntry: + """Discovery and check evidence, never an ownership or hash assertion.""" + + file: str + target: str + kind: str + pointer: str + tracked: bool + status: Literal["checked", "not-applicable", "incomplete"] + diagnostic: str | None = None + strippable: bool = False @dataclass(frozen=True) @@ -32,16 +57,35 @@ class _FileScanResult: findings_by_file: dict[str, list[ScanFinding]] scanned_files: frozenset[str] + inventory: tuple[CoverageEntry, ...] = () + + @property + def incomplete(self) -> tuple[CoverageEntry, ...]: + """Return recognized content whose prompt coverage could not be established.""" + return tuple(entry for entry in self.inventory if entry.status == "incomplete") + + @property + def protected_files(self) -> frozenset[str]: + """Keep shared/structured content out of whole-file Unicode remediation.""" + return frozenset(entry.file for entry in self.inventory if not entry.strippable) def merged(self, other: _FileScanResult) -> _FileScanResult: """Union scopes, treating ``self`` as the authoritative first scope.""" - findings = dict(self.findings_by_file) - for rel_path, file_findings in other.findings_by_file.items(): - findings.setdefault(rel_path, file_findings) - return _FileScanResult( - findings_by_file=findings, - scanned_files=self.scanned_files | other.scanned_files, - ) + return _merge_results((self, other)) + + +def _merge_results(results: Iterable[_FileScanResult]) -> _FileScanResult: + """Union scan evidence in linear time, retaining the first scope's metadata.""" + findings: dict[str, list[ScanFinding]] = {} + scanned: set[str] = set() + inventory: dict[tuple[str, str], CoverageEntry] = {} + for result in results: + for path, items in result.findings_by_file.items(): + findings.setdefault(path, items) + scanned.update(result.scanned_files) + for entry in result.inventory: + inventory.setdefault((entry.file, entry.pointer), entry) + return _FileScanResult(findings, frozenset(scanned), tuple(inventory.values())) def _empty_scan() -> _FileScanResult: @@ -93,99 +137,157 @@ def _minimal_governed_prefixes(prefixes: set[str]) -> tuple[str, ...]: return tuple(path.as_posix() for path in accepted) -def _scan_external_deployed_trees(targets: Sequence[TargetProfile]) -> _FileScanResult: - """Scan bounded governed paths below resolved external target roots.""" - from ..install.audit_target_roots import external_target_relative_roots +def _label(path: Path, surface: PrimitiveSurface, project_root: Path) -> str: + """Label external paths without disclosing an absolute user configuration root.""" + if path.is_relative_to(project_root): + return path.relative_to(project_root).as_posix() + return f"{surface.target}:{path.relative_to(surface.root).as_posix()}" - result = _empty_scan() - for target in targets: - deploy_root = target.managed_deploy_root - if deploy_root is None: - continue - for relative_root in external_target_relative_roots(target): - candidate = deploy_root / relative_root - if candidate.is_symlink(): - continue - try: - deploy_path = ensure_path_within(candidate, deploy_root) - except PathTraversalError: - continue - label = f"{target.name}:{relative_root}" - if deploy_path.is_dir(): - result = result.merged(_scan_directory_result(deploy_path, label)) - elif deploy_path.is_file(): - findings = ContentScanner.scan_file(deploy_path) - result = result.merged( - _FileScanResult( - findings_by_file={label: findings} if findings else {}, - scanned_files=frozenset({label}), + +def _reject_duplicate_keys(pairs: list[tuple[str, object]]) -> dict[str, object]: + """Reject ambiguous native configuration instead of silently dropping fields.""" + result: dict[str, object] = {} + for key, value in pairs: + if key in result: + raise ValueError("duplicate native configuration key") + result[key] = value + return result + + +def _content_entries(path: Path, surface: PrimitiveSurface) -> tuple[HookContentEntry, ...]: + """Extract documented prompt-bearing content, not arbitrary string values.""" + if surface.kind == "executable": + return (HookContentEntry(""),) + text = path.read_text(encoding="utf-8") + if surface.kind == "hooks": + config = native_hook_config(surface.target) + return inspect_native_hooks( + json.loads(text, object_pairs_hook=_reject_duplicate_keys), + surface.format_id, + container=config.event_container_key if config else "hooks", + prompt_types=config.prompt_handler_types if config else (), + named_containers=config.named_containers if config else False, + shared=surface.shared, + nested_handlers=config.nested_handlers if config else None, + ) + if surface.prompt_fields: + document = tomllib.loads(text) + return tuple( + HookContentEntry(f"/{field}", prompt=document[field]) + if isinstance(document.get(field), str) + else HookContentEntry(f"/{field}", error="missing or non-string native prompt field") + for field in surface.prompt_fields + ) + if path.suffix in {".md", ".mdc"}: + return (HookContentEntry("", prompt=text),) + return (HookContentEntry("", error="unsupported recognized primitive format"),) + + +def _scan_primitive( + path: Path, surface: PrimitiveSurface, label: str, *, tracked: bool +) -> _FileScanResult: + """Check one recognized surface and retain non-applicability/coverage evidence.""" + if not safe_surface_path(surface, path): + return _empty_scan() + try: + entries = _content_entries(path, surface) + except (OSError, UnicodeError, ValueError) as exc: + entries = ( + HookContentEntry("", error=f"cannot interpret primitive ({type(exc).__name__})"), + ) + inventory: list[CoverageEntry] = [] + findings: list[ScanFinding] = [] + for entry in entries: + status = ( + "incomplete" + if entry.error + else "checked" + if entry.prompt is not None + else "not-applicable" + ) + inventory.append( + CoverageEntry( + label, + surface.target, + surface.kind, + entry.pointer, + tracked, + status, + entry.error, + strippable=( + not surface.shared + and not entry.pointer + and status == "checked" + and not label.startswith(f"{surface.target}:") + ), + ) + ) + if entry.prompt is not None: + for finding in ContentScanner.scan_text(entry.prompt, filename=label): + findings.append( + replace( + finding, + description=f"{finding.description} (prompt field {entry.pointer})", + pointer=entry.pointer, ) + if entry.pointer + else finding ) - return result + return _FileScanResult( + {label: findings} if findings else {}, + frozenset({label}) + if any(entry.status == "checked" for entry in inventory) + else frozenset(), + tuple(inventory), + ) def _scan_deployed_trees( project_root: Path, targets: Sequence[TargetProfile] = (), + *, + user_scope: bool = False, ) -> _FileScanResult: - """Scan every file under the deploy directories this project's targets govern. - - Hash verification has to be manifest-driven: comparing a hash needs a - recorded baseline to compare against. Hidden-Unicode detection does not -- - a bidi override is dangerous on sight, with no baseline required. Scoping - both signals to ``deployed_files`` therefore leaves any deployed file the - lockfile omits unscanned, permanently and with no indication that the - scope narrowed (issue #2379). For this signal the deploy tree is the - boundary, not manifest membership. - - Callers holding scope-resolved *targets* pass them: a global audit's - user-scope roots (``~/.copilot``) are not what a fresh project-scope - resolution returns. Without them, targets are resolved from the project, - which is sufficient: a deploy tree absent from disk has nothing to scan, - and one present on disk is exactly what ``detect_by_dir`` resolution keys - on. Sources under ``.apm/`` are not in scope here -- the install-time - pre-deployment gate owns those. - - The walk is bounded to each target's primitive deploy directories and - generated files rather than its whole root: at user scope a root such as - ``~/.claude`` also holds session transcripts APM never deploys, and - walking those costs minutes for no signal the audit can remediate. - """ - from ..install.manifest_reconcile import install_governance + """Discover recognized primitives without consulting a deployment baseline.""" from ..integration.targets import resolve_targets - scoped = list(targets) if targets else resolve_targets(project_root) - file_prefixes, _uri_schemes = install_governance(scoped, bounded=True) - result = _empty_scan() - - for rel_path in _minimal_governed_prefixes(file_prefixes): - # These prefixes come from the target registry, not from untrusted - # input, so the deploy-path allow-list would be circular here (the - # prefixes ARE that list). Containment is the property worth asserting, - # via the sanctioned guard. Symlinked roots resolve outward and are - # skipped; SecurityGate.scan_files likewise never follows links. - candidate = project_root / rel_path - if candidate.is_symlink(): - continue + scoped = ( + list(targets) + if targets + else resolve_targets(project_root, user_scope=user_scope, create_config=False) + ) + results: list[_FileScanResult] = [] + visited: set[Path] = set() + for surface in primitive_surfaces(project_root, scoped, user_scope=user_scope): try: - deploy_path = ensure_path_within(candidate, project_root) - except PathTraversalError: - continue - if deploy_path == project_root.resolve(): - continue - - if deploy_path.is_dir(): - result = result.merged(_scan_directory_result(deploy_path, rel_path)) - elif deploy_path.is_file(): - findings = ContentScanner.scan_file(deploy_path) - result = result.merged( + for path in iter_surface_files(surface): + if path not in visited: + visited.add(path) + results.append( + _scan_primitive( + path, surface, _label(path, surface, project_root), tracked=False + ) + ) + except OSError as exc: + label = _label(surface.path, surface, project_root) + results.append( _FileScanResult( - findings_by_file={rel_path: findings} if findings else {}, - scanned_files=frozenset({rel_path}), + {}, + frozenset(), + ( + CoverageEntry( + label, + surface.target, + surface.kind, + "", + False, + "incomplete", + f"cannot discover primitives ({type(exc).__name__})", + ), + ), ) ) - - return result.merged(_scan_external_deployed_trees(targets)) + return _merge_results(results) def scan_deployed_trees(project_root: Path) -> tuple[dict[str, list[ScanFinding]], int]: @@ -198,34 +300,114 @@ def _scan_claimed_files( project_root: Path, claims: dict[str, str], package_filter: str | None, + targets: Sequence[TargetProfile] = (), + *, + user_scope: bool = False, ) -> _FileScanResult: """Scan one canonical deployment-claim projection.""" - result = _empty_scan() + from ..integration.targets import KNOWN_TARGETS + + # Catalog profiles classify historical claims; only selected profiles are + # enumerated by automatic discovery. + profiles = list(targets) + [ + profile + for profile in KNOWN_TARGETS.values() + if profile.name not in {target.name for target in targets} + and not profile.user_root_resolver + ] + surfaces = primitive_surfaces(project_root, profiles, user_scope=user_scope) + results: list[_FileScanResult] = [] for rel_path, owner in claims.items(): if package_filter and owner != package_filter: continue safe_path = rel_path.rstrip("/") - if not _is_safe_lockfile_path(safe_path, project_root): - continue - abs_path = project_root / rel_path + if "://" in rel_path: + decoded = next( + ( + target.decode_external_locator(rel_path, target.managed_deploy_root) + for target in targets + if target.managed_deploy_root is not None + and any(rel_path.startswith(scheme) for scheme in target.lockfile_uri_schemes) + ), + None, + ) + if decoded is None: + continue + abs_path = decoded + surface = next( + ( + s + for s in surfaces + if s.contains(abs_path) + or (s.pattern is not None and abs_path.is_dir() and abs_path.is_relative_to(s.path)) + ), + None, + ) + external = surface is not None and surface.root != project_root + if not external and not _is_safe_lockfile_path(safe_path, project_root): + continue if not abs_path.exists(): continue + if surface is None: + surface = PrimitiveSurface( + project_root, + abs_path, + "*.md" if abs_path.is_dir() else None, + "recorded", + "document" + if abs_path.suffix in {".md", ".mdc"} or abs_path.is_dir() + else "executable", + "recorded", + ) if abs_path.is_dir(): - result = result.merged(_scan_directory_result(abs_path, safe_path)) + directory_surfaces = [ + replace(candidate, path=abs_path) + if candidate.pattern and abs_path.is_relative_to(candidate.path) + else candidate + for candidate in surfaces + if candidate.path.is_relative_to(abs_path) + or (candidate.pattern and abs_path.is_relative_to(candidate.path)) + ] or [surface] + for directory_surface in directory_surfaces: + try: + for path in iter_surface_files(directory_surface): + results.append( + _scan_primitive( + path, + directory_surface, + _label(path, directory_surface, project_root), + tracked=True, + ) + ) + except OSError as exc: + results.append( + _FileScanResult( + {}, + frozenset(), + ( + CoverageEntry( + rel_path, + directory_surface.target, + directory_surface.kind, + "", + True, + "incomplete", + f"cannot discover recorded primitives ({type(exc).__name__})", + ), + ), + ) + ) continue - - findings = ContentScanner.scan_file(abs_path) - result = result.merged( - _FileScanResult( - findings_by_file={rel_path: findings} if findings else {}, - scanned_files=frozenset({rel_path}), + results.append( + _scan_primitive( + abs_path, surface, _label(abs_path, surface, project_root), tracked=True ) ) - return result + return _merge_results(results) def _scan_lockfile_packages( @@ -278,7 +460,48 @@ def scan_project_files( targets: Sequence[TargetProfile] = (), ) -> tuple[dict[str, list[ScanFinding]], int]: """Union lockfile and deploy-tree scopes with exact unique accounting.""" - result = _scan_lockfile_packages(project_root, package_filter, lockfile) - if include_deployed_trees: - result = result.merged(_scan_deployed_trees(project_root, targets)) + result = scan_project_result( + project_root, + package_filter=package_filter, + lockfile=lockfile, + include_deployed_trees=include_deployed_trees, + targets=targets, + ) return result.findings_by_file, len(result.scanned_files) + + +def scan_project_result( + project_root: Path, + *, + package_filter: str | None = None, + lockfile: LockFile | None = None, + include_deployed_trees: bool = True, + targets: Sequence[TargetProfile] = (), + user_scope: bool = False, +) -> _FileScanResult: + """Return findings, discovery inventory and explicit incomplete coverage.""" + from ..core.deployment_ledger import DeploymentLedgerCodec + from ..core.scope import get_workspace_deploy_root + from ..install.drift import _read_apm_yml_target + from ..integration.targets import resolve_targets + + deploy_root = get_workspace_deploy_root(project_root) + user_scope = user_scope or deploy_root != project_root.resolve() + scoped = ( + tuple(targets) + if targets + else tuple( + resolve_targets( + deploy_root, + user_scope=user_scope, + explicit_target=_read_apm_yml_target(project_root), + create_config=False, + ) + ) + ) + lock = lockfile if lockfile is not None else LockFile.read(get_lockfile_path(project_root)) + claims = DeploymentLedgerCodec.legacy_deployed_file_claims(lock) if lock else {} + result = _scan_claimed_files(deploy_root, claims, package_filter, scoped, user_scope=user_scope) + if include_deployed_trees: + result = result.merged(_scan_deployed_trees(deploy_root, scoped, user_scope=user_scope)) + return result diff --git a/src/apm_cli/security/primitive_discovery.py b/src/apm_cli/security/primitive_discovery.py new file mode 100644 index 0000000000..2205c9ebb0 --- /dev/null +++ b/src/apm_cli/security/primitive_discovery.py @@ -0,0 +1,169 @@ +"""Registry-derived deployed primitive boundaries, independent of ownership.""" + +from __future__ import annotations + +import fnmatch +import os +from collections.abc import Iterator, Sequence +from dataclasses import dataclass +from pathlib import Path + +from apm_cli.compilation.root_context_protection import root_context_filename +from apm_cli.integration.hook_integrator import native_hook_config +from apm_cli.integration.targets import PrimitiveMapping, TargetProfile +from apm_cli.utils.path_security import ( + PathTraversalError, + ensure_path_within, + has_symlink_component, +) + + +@dataclass(frozen=True) +class PrimitiveSurface: + """One bounded native filename or pattern supplied by a target profile.""" + + root: Path + path: Path + pattern: str | None + target: str + kind: str + format_id: str + prompt_fields: tuple[str, ...] = () + shared: bool = False + + def contains(self, path: Path) -> bool: + """Match a path without walking or resolving user-controlled links.""" + if self.pattern is None: + return path == self.path + return path.is_relative_to(self.path) and fnmatch.fnmatchcase(path.name, self.pattern) + + +def _mapping_surface( + project_root: Path, target: TargetProfile, kind: str, mapping: PrimitiveMapping +) -> PrimitiveSurface | None: + """Translate the existing deployment mapping, not an audit-local path list.""" + if kind == "canvas": + return None + root = target.managed_deploy_root or project_root + base = ( + project_root / mapping.deploy_root + if mapping.deploy_root + else target.deploy_path(project_root) + ) + if mapping.deploy_root: + root = project_root + path = base / mapping.subdir + if not mapping.subdir: + if mapping.extension.startswith("."): + # Root-level aggregate mappings declare exact generated filenames. + return None + path /= mapping.extension.lstrip("/") + pattern = None + elif mapping.extension.startswith("/"): + pattern = mapping.extension.lstrip("/") + else: + pattern = f"*{mapping.extension}" + return PrimitiveSurface( + root, path, pattern, target.name, kind, mapping.format_id, mapping.prompt_fields + ) + + +def primitive_surfaces( + project_root: Path, + targets: Sequence[TargetProfile], + *, + user_scope: bool = False, +) -> tuple[PrimitiveSurface, ...]: + """Describe recognized native files using scope-resolved target/hook owners.""" + surfaces: list[PrimitiveSurface] = [] + for target in targets: + for kind, mapping in target.primitives.items(): + surface = _mapping_surface(project_root, target, kind, mapping) + if surface is not None: + surfaces.append(surface) + root = target.managed_deploy_root or project_root + for filename in target.generated_files: + surfaces.append( + PrimitiveSurface( + root, + target.deploy_path(project_root, filename), + None, + target.name, + "context", + "markdown", + ) + ) + context = root_context_filename(target.compile_family) + if context is not None: + context_path = ( + target.deploy_path(project_root, context) if user_scope else project_root / context + ) + surfaces.append( + PrimitiveSurface( + root if user_scope else project_root, + context_path, + None, + target.name, + "context", + "markdown", + ) + ) + config = native_hook_config(target.name) + mapping = target.primitives.get("hooks") + if config is not None and mapping is not None: + surfaces.append( + PrimitiveSurface( + root, + target.deploy_path(project_root, config.config_filename), + None, + target.name, + "hooks", + mapping.format_id, + shared=True, + ) + ) + # Exact shared configs override an identical primitive filename (hooks.json). + return tuple({(s.path, s.pattern): s for s in surfaces}.values()) + + +def safe_surface_path(surface: PrimitiveSurface, path: Path) -> bool: + """Apply containment and reject symlinks at every component before reading.""" + if surface.root.is_symlink() or has_symlink_component(surface.root, path): + return False + try: + ensure_path_within(path, surface.root) + except (PathTraversalError, OSError, RuntimeError): + return False + return True + + +def iter_surface_files(surface: PrimitiveSurface) -> Iterator[Path]: + """Enumerate only a registered primitive directory, never a tool root.""" + if not safe_surface_path(surface, surface.path): + return + if surface.pattern is None: + if surface.path.exists(): + yield surface.path + return + if not surface.path.exists(): + return + if not surface.path.is_dir(): + yield surface.path + return + + def fail(exc: OSError) -> None: + raise exc + + for directory, dirs, files in os.walk(surface.path, followlinks=False, onerror=fail): + current = Path(directory) + dirs[:] = [ + name + for name in dirs + if not name.startswith(".") + and name not in {"__pycache__", "node_modules"} + and not (current / name).is_symlink() + ] + for filename in sorted(files): + path = current / filename + if surface.contains(path) and safe_surface_path(surface, path): + yield path diff --git a/tests/integration/test_architecture_audit_discovery.py b/tests/integration/test_architecture_audit_discovery.py new file mode 100644 index 0000000000..dbd593fff0 --- /dev/null +++ b/tests/integration/test_architecture_audit_discovery.py @@ -0,0 +1,39 @@ +"""Audit consumers must not rederive primitive discovery or content applicability.""" + +from pathlib import Path + +import pytest + +from scripts.architecture_linter.runner import run_selected_rules + +pytestmark = pytest.mark.component + +_ROOT = Path(__file__).resolve().parents[2] +_RULE = "audit-primitive-discovery" +_SCANNER = "src/apm_cli/security/file_scanner.py" + + +def test_audit_discovery_uses_canonical_contract() -> None: + result = run_selected_rules(_ROOT, {_RULE}) + assert result.failures == () + assert result.violations == () + + +@pytest.mark.parametrize( + "old,new", + [ + ( + "for surface in primitive_surfaces(project_root, scoped, user_scope=user_scope):", + "for surface in ():", + ), + ("return inspect_native_hooks(", "return bypass_native_hooks("), + ("if not safe_surface_path(surface, path):", "if False:"), + ], +) +def test_guard_rejects_discovery_applicability_or_containment_bypass(old: str, new: str) -> None: + source = (_ROOT / _SCANNER).read_text(encoding="utf-8") + mutated = source.replace(old, new, 1) + assert mutated != source + result = run_selected_rules(_ROOT, {_RULE}, source_overrides={_SCANNER: mutated}) + assert result.failures == () + assert any(v.rule_id == _RULE and v.path == _SCANNER for v in result.violations) diff --git a/tests/integration/test_audit_primitive_contract.py b/tests/integration/test_audit_primitive_contract.py new file mode 100644 index 0000000000..31bdf275eb --- /dev/null +++ b/tests/integration/test_audit_primitive_contract.py @@ -0,0 +1,459 @@ +"""Real CLI and filesystem contracts for target-aware audit discovery.""" + +from __future__ import annotations + +import json +import os +from dataclasses import replace +from pathlib import Path, PurePosixPath + +import pytest +from click.testing import CliRunner + +from apm_cli.cli import cli +from apm_cli.deps.lockfile import LockFile +from apm_cli.integration.targets import KNOWN_TARGETS +from apm_cli.policy.ci_checks import _check_content_integrity +from apm_cli.security.file_scanner import scan_project_result +from apm_cli.security.primitive_discovery import primitive_surfaces +from apm_cli.utils.content_hash import compute_file_hash +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment +from tests.utils.lifecycle_state import LifecycleStateSnapshot + +pytestmark = [pytest.mark.integration, pytest.mark.component] + +_BIDI = "\u202e" +_SETTINGS = ".claude/settings.json" + + +@pytest.fixture +def project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Bind every CLI call to isolated configuration, home and cache roots.""" + isolated = IsolatedApmEnvironment.create(tmp_path / "audit", base_env=os.environ) + for key, value in isolated.subprocess_env().items(): + monkeypatch.setenv(key, value) + root = isolated.work_root + root.mkdir(parents=True, exist_ok=True) + (root / "apm.yml").write_text("name: audit-contract\nversion: 1.0.0\ntarget: claude\n") + LockFile().write(root / "apm.lock.yaml") + monkeypatch.chdir(root) + return root + + +def _write(root: Path, relative: str, text: str) -> Path: + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +def _settings(root: Path, *, prompt: bool = False) -> Path: + handlers = [{"type": "command", "command": f"touch NEVER_EXECUTE{_BIDI}"}] + if prompt: + handlers.append({"type": "prompt", "prompt": f"Check this {_BIDI}"}) + return _write( + root, + _SETTINGS, + json.dumps( + { + "unrelated": _BIDI, + "hooks": {"Stop": [{"matcher": _BIDI, "hooks": handlers}]}, + } + ), + ) + + +@pytest.mark.parametrize("ci_mode", [False, True]) +def test_shared_settings_command_inventory_is_visible_and_non_failing( + project: Path, ci_mode: bool +) -> None: + settings = _settings(project) + before = settings.read_bytes() + args = ["audit", "--no-drift", "--format", "json"] + if ci_mode: + args += ["--ci", "--no-policy", "--no-fail-fast"] + result = CliRunner().invoke(cli, args) + assert result.exit_code == 0, result.output + report = json.loads(result.output) + if ci_mode: + check = next(c for c in report["checks"] if c["name"] == "content-integrity") + assert any("discovered/untracked:" in detail for detail in check["details"]) + assert any("not-applicable" in detail for detail in check["details"]) + else: + assert report["summary"]["files_scanned"] == 0 + assert report["findings"] == [] + entry = report["coverage"]["primitives"][0] + assert entry["tracked"] is False + assert entry["status"] == "not-applicable" + assert entry["pointer"] == "/hooks/Stop/0/hooks/0" + assert settings.read_bytes() == before + assert not list(project.glob("NEVER_EXECUTE*")) + + +@pytest.mark.parametrize("output_format", ["text", "json", "sarif", "markdown"]) +def test_decoded_prompt_fields_fail_without_scanning_unrelated_settings( + project: Path, output_format: str +) -> None: + settings = _settings(project, prompt=True) + before = settings.read_bytes() + result = CliRunner().invoke(cli, ["audit", "--no-drift", "--format", output_format]) + assert result.exit_code == 1, result.output + assert "/hooks/Stop/0/hooks/1/prompt" in result.output + assert settings.read_bytes() == before + if output_format == "json": + report = json.loads(result.output) + assert report["summary"]["critical"] == 1 + assert report["summary"]["files_scanned"] == 1 + assert [entry["status"] for entry in report["coverage"]["primitives"]] == [ + "not-applicable", + "checked", + ] + stripped = CliRunner().invoke(cli, ["audit", "--strip"]) + assert stripped.exit_code == 1 + assert "does not rewrite native configuration" in " ".join(stripped.output.split()) + assert settings.read_bytes() == before + + +@pytest.mark.parametrize( + "payload", + [ + '{"hooks":', + '{"hooks":{"Stop":"invalid"}}', + '{"hooks":{"Stop":[{"type":"future","prompt":"ignored"}]}}', + '{"hooks":{"Stop":[{"type":"prompt","prompt":12}]}}', + '{"hooks":{},"hooks":{}}', + '{"hooks":{"Stop":[{"hooks":[{"hooks":[{"type":"prompt","prompt":"hidden"}]}]}]}}', + ], +) +@pytest.mark.parametrize("ci_mode", [False, True]) +def test_unknown_or_malformed_recognized_content_never_claims_success( + project: Path, payload: str, ci_mode: bool +) -> None: + settings = _write(project, _SETTINGS, payload) + args = ["audit", "--no-drift", "--format", "json"] + if ci_mode: + args += ["--ci", "--no-policy", "--no-fail-fast"] + result = CliRunner().invoke(cli, args) + assert result.exit_code == 1, result.output + report = json.loads(result.output) + if ci_mode: + check = next(c for c in report["checks"] if c["name"] == "content-integrity") + assert check["passed"] is False + assert any("incomplete-coverage:" in detail for detail in check["details"]) + else: + assert report["passed"] is False + assert report["coverage"]["complete"] is False + assert report["findings"] == [] + assert settings.read_text() == payload + + +def test_unreadable_recognized_file_is_reported( + project: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + settings = _settings(project) + read_text = Path.read_text + + def unreadable(path: Path, *args: object, **kwargs: object) -> str: + if path == settings: + raise PermissionError("fixture access denied") + return read_text(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", unreadable) + result = CliRunner().invoke(cli, ["audit", "--no-drift", "--format", "json"]) + assert result.exit_code == 1, result.output + report = json.loads(result.output) + assert report["coverage"]["primitives"][0]["status"] == "incomplete" + assert report["summary"]["files_scanned"] == 0 + assert report["findings"] == [] + + +def test_lockfile_membership_does_not_turn_a_script_into_prompt_text(project: Path) -> None: + script = ".claude/hooks/scripts/run.py" + _write(project, script, f"raise RuntimeError('NEVER_EXECUTE') # {_BIDI}") + prompt = ".claude/skills/manual/SKILL.md" + _write(project, prompt, f"Instruction {_BIDI}") + lock = LockFile(local_deployed_files=[script, prompt]) + result = scan_project_result(project, lockfile=lock, targets=(KNOWN_TARGETS["claude"],)) + assert set(result.findings_by_file) == {prompt} + inventory = {entry.file: entry for entry in result.inventory} + assert inventory[script].status == "not-applicable" + assert inventory[script].tracked is True + assert inventory[prompt].status == "checked" + assert inventory[prompt].tracked is True + assert result.scanned_files == frozenset({prompt}) + + +@pytest.mark.parametrize("target", sorted(KNOWN_TARGETS)) +def test_registered_prompt_patterns_are_discovered(project: Path, target: str) -> None: + profile = KNOWN_TARGETS[target] + paths = set() + surfaces = primitive_surfaces(project, (profile,)) + expected_kinds = set(profile.primitives) - {"hooks", "canvas"} + assert expected_kinds <= {surface.kind for surface in surfaces} + for surface in surfaces: + if surface.kind == "hooks": + continue + path = ( + surface.path / surface.pattern.replace("*", "manual") + if surface.pattern + else surface.path + ) + content = ( + "\n".join(f'{field} = "\\u202e"' for field in surface.prompt_fields) + if surface.prompt_fields + else _BIDI + ) + _write(project, path.relative_to(project).as_posix(), content) + paths.add(path.relative_to(project).as_posix()) + result = scan_project_result(project, targets=(profile,)) + assert paths + assert set(result.findings_by_file) == paths + assert result.incomplete == () + + +def test_non_prompt_scripts_keep_hash_and_ownership_checks(project: Path) -> None: + relative = ".claude/hooks/scripts/run.py" + script = _write(project, relative, f"raise RuntimeError('NEVER_EXECUTE') # {_BIDI}") + original_hash = compute_file_hash(script) + lock_path = project / "apm.lock.yaml" + LockFile( + local_deployed_files=[relative], + local_deployed_file_hashes={ + relative: original_hash, + }, + ).write(lock_path) + lock = LockFile.read(lock_path) + assert lock is not None + clean = _check_content_integrity(project, lock, (KNOWN_TARGETS["claude"],)) + assert clean.passed, clean.details + script.write_text(f"# changed {_BIDI}", encoding="utf-8") + tampered = _check_content_integrity(project, lock, (KNOWN_TARGETS["claude"],)) + assert not tampered.passed + assert any(detail.startswith(f"hash-drift: {relative}") for detail in tampered.details) + assert not any(detail.startswith("unicode:") for detail in tampered.details) + + +@pytest.mark.parametrize( + "target,path,payload", + [ + ( + "copilot", + ".github/hooks/manual.json", + '{"version":1,"hooks":{"sessionStart":[{"bash":"\\u202e"}]}}', + ), + ( + "claude", + _SETTINGS, + '{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"\\u202e"}]}]}}', + ), + ( + "gemini", + ".gemini/settings.json", + '{"hooks":{"AfterAgent":[{"hooks":[{"type":"command","command":"\\u202e"}]}]}}', + ), + ( + "codex", + ".codex/hooks.json", + '{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"\\u202e"}]}]}}', + ), + ("cursor", ".cursor/hooks.json", '{"version":1,"hooks":{"stop":[{"command":"\\u202e"}]}}'), + ( + "windsurf", + ".windsurf/hooks.json", + '{"hooks":{"post_cascade_response":[{"command":"\\u202e"}]}}', + ), + ( + "kiro", + ".kiro/hooks/manual.json", + '{"version":"v1","hooks":[{"trigger":"agentStop","action":{"type":"command","command":"\\u202e"}}]}', + ), + ( + "antigravity", + ".agents/hooks.json", + '{"manual":{"PreToolUse":[{"hooks":[{"command":"\\u202e"}]}]}}', + ), + ], +) +def test_native_command_formats_are_inventory_not_prompt( + project: Path, target: str, path: str, payload: str +) -> None: + _write(project, path, payload) + result = scan_project_result(project, targets=(KNOWN_TARGETS[target],)) + assert result.findings_by_file == {} + assert result.scanned_files == frozenset() + assert len(result.inventory) == 1 + assert result.inventory[0].status == "not-applicable" + assert result.inventory[0].file == path + + +def test_excluded_transcripts_are_not_walked_or_read( + project: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + skill = _write(project, ".claude/skills/manual/SKILL.md", _BIDI) + transcripts = _write(project, ".claude/projects/session/log.jsonl", _BIDI * 1000) + _write(project, ".claude/history.jsonl", _BIDI) + walk = os.walk + read_text = Path.read_text + walks: list[Path] = [] + reads: list[Path] = [] + + def bounded_walk(path: Path, *args: object, **kwargs: object) -> object: + walks.append(Path(path)) + assert Path(path) != project / ".claude" + assert not Path(path).is_relative_to(transcripts.parent.parent) + return walk(path, *args, **kwargs) + + def bounded_read(path: Path, *args: object, **kwargs: object) -> str: + reads.append(path) + assert path.suffix != ".jsonl" + return read_text(path, *args, **kwargs) + + monkeypatch.setattr(os, "walk", bounded_walk) + monkeypatch.setattr(Path, "read_text", bounded_read) + result = scan_project_result(project, targets=(KNOWN_TARGETS["claude"],)) + assert result.scanned_files == frozenset({".claude/skills/manual/SKILL.md"}) + assert skill in reads + assert walks == [project / ".claude/skills"] + + +def test_directory_claims_use_native_applicability_without_tool_root_walk( + project: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _settings(project, prompt=True) + _write( + project, + ".claude/hooks/manual.json", + '{"hooks":{"Stop":[{"hooks":[{"type":"prompt","prompt":"\\u202e"}]}]}}', + ) + _write(project, ".claude/history/session.md", _BIDI) + walk = os.walk + + def bounded_walk(path: Path, *args: object, **kwargs: object) -> object: + assert Path(path) != project / ".claude" + assert not Path(path).is_relative_to(project / ".claude/history") + return walk(path, *args, **kwargs) + + monkeypatch.setattr(os, "walk", bounded_walk) + result = scan_project_result( + project, + lockfile=LockFile(local_deployed_files=[".claude/hooks/", _SETTINGS]), + targets=(KNOWN_TARGETS["claude"],), + include_deployed_trees=False, + ) + assert set(result.findings_by_file) == {_SETTINGS, ".claude/hooks/manual.json"} + assert all(entry.tracked for entry in result.inventory) + + +@pytest.mark.parametrize( + "target,relative,text,expected", + [ + ( + "gemini", + ".gemini/commands/manual.toml", + 'prompt = "\\u202e"\ndescription = "\\u202e"', + True, + ), + ( + "codex", + ".codex/agents/manual.toml", + 'developer_instructions = "\\u202e"\nname = "\\u202e"', + True, + ), + ( + "kiro", + ".kiro/hooks/manual.json", + '{"version":"v1","hooks":[{"trigger":"agentStop","action":{"type":"agent","prompt":"\\u202e"}}]}', + True, + ), + ( + "cursor", + ".cursor/hooks.json", + '{"version":1,"hooks":{"stop":[{"command":"\\u202e"}]}}', + False, + ), + ("antigravity", ".agents/hooks.json", '{"manual":{"Stop":[{"command":"\\u202e"}]}}', False), + ], +) +def test_native_formats_apply_only_documented_prompt_fields( + project: Path, target: str, relative: str, text: str, expected: bool +) -> None: + _write(project, relative, text) + result = scan_project_result(project, targets=(KNOWN_TARGETS[target],)) + assert bool(result.findings_by_file) is expected + assert not result.incomplete + assert len(result.inventory) == 1 + assert result.inventory[0].tracked is False + + +def test_scope_resolved_external_root_and_symlink_boundaries(project: Path) -> None: + external = project.parent / "external-claude" + _write(external, "skills/manual/SKILL.md", _BIDI) + _write( + external, + "settings.json", + '{"hooks":{"Stop":[{"hooks":[{"type":"prompt","prompt":"\\u202e"}]}]}}', + ) + outside = _write(project.parent, "outside/SKILL.md", _BIDI) + (external / "skills" / "linked").symlink_to(outside.parent, target_is_directory=True) + profile = replace(KNOWN_TARGETS["claude"], resolved_deploy_root=external) + result = scan_project_result(project, targets=(profile,), user_scope=True) + assert result.scanned_files == frozenset( + { + "claude:skills/manual/SKILL.md", + "claude:settings.json", + } + ) + assert all("linked" not in entry.file for entry in result.inventory) + assert result.protected_files == result.scanned_files + + +def test_audit_and_failures_preserve_complete_workspace_snapshot(project: Path) -> None: + _settings(project, prompt=True) + _write(project, ".claude/history.jsonl", _BIDI) + configs = (PurePosixPath(_SETTINGS), PurePosixPath(".claude/history.jsonl")) + before = LifecycleStateSnapshot.capture(project, targets=("claude",), config_paths=configs) + for args in ( + ["audit", "--no-drift"], + ["audit", "--ci", "--no-policy", "--no-drift", "--no-fail-fast"], + ["audit", "--strip"], + ): + result = CliRunner().invoke(cli, args) + assert result.exit_code == 1, result.output + after = LifecycleStateSnapshot.capture(project, targets=("claude",), config_paths=configs) + assert after == before + + +def test_symlink_prompt_is_never_read(project: Path, monkeypatch: pytest.MonkeyPatch) -> None: + outside = _write(project.parent, "outside-prompt.md", _BIDI) + link = project / ".claude/rules/linked.md" + link.parent.mkdir(parents=True) + link.symlink_to(outside) + read_text = Path.read_text + + def guarded_read(path: Path, *args: object, **kwargs: object) -> str: + assert path not in {link, outside}, "audit followed a user symlink" + return read_text(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", guarded_read) + result = scan_project_result(project, targets=(KNOWN_TARGETS["claude"],)) + assert result.scanned_files == frozenset() + assert result.inventory == () + + +@pytest.mark.parametrize("ci_mode", [False, True]) +def test_lockless_cli_still_discovers_prompt_primitives(project: Path, ci_mode: bool) -> None: + (project / "apm.lock.yaml").unlink() + _write(project, ".claude/skills/manual/SKILL.md", _BIDI) + args = ["audit", "--no-drift", "--format", "json"] + if ci_mode: + args += ["--ci", "--no-policy", "--no-fail-fast"] + result = CliRunner().invoke(cli, args) + assert result.exit_code == 1, result.output + report = json.loads(result.output) + if ci_mode: + content = next(check for check in report["checks"] if check["name"] == "content-integrity") + assert content["passed"] is False + assert content["details"] == ["unicode: .claude/skills/manual/SKILL.md"] + else: + assert report["summary"]["critical"] == 1 + assert report["coverage"]["primitives"][0]["tracked"] is False diff --git a/tests/integration/test_audit_unrecorded_unicode_lifecycle.py b/tests/integration/test_audit_unrecorded_unicode_lifecycle.py index 9ed45c1246..2724595b6f 100644 --- a/tests/integration/test_audit_unrecorded_unicode_lifecycle.py +++ b/tests/integration/test_audit_unrecorded_unicode_lifecycle.py @@ -161,6 +161,102 @@ def _assert_same_state( assert actual.semantic_bytes == expected.semantic_bytes +def test_native_hook_discovery_across_reinstall_update_and_user_scope( + tmp_path: Path, apm_binary_path: Path +) -> None: + """The real CLI preserves manual native settings through lifecycle transitions.""" + lifecycle = _install_lifecycle(tmp_path / "native-hooks", apm_binary_path) + settings_path = lifecycle.project.root / ".claude" / "settings.json" + settings = { + "unrelated": "\u202e", + "hooks": { + "Stop": [ + { + "hooks": [ + {"type": "command", "command": "touch NEVER_EXECUTE"}, + ] + } + ] + }, + } + settings_bytes = json.dumps(settings).encode() + settings_path.write_bytes(settings_bytes) + transcript = lifecycle.project.root / ".claude" / "history.jsonl" + transcript.write_bytes(_BIDI_BYTES) + configs = ( + PurePosixPath(".claude/settings.json"), + PurePosixPath(".claude/history.jsonl"), + ) + for action in (("install",), ("install", "--update")): + before_audit = LifecycleStateSnapshot.capture( + lifecycle.project.root, + targets=("claude",), + config_paths=configs, + ) + audit = _run( + lifecycle, + ("audit", "--no-drift", "--format", "json"), + expected=0, + scenario_id=f"native-hooks-{action[-1]}-audit", + ) + report = json.loads(audit.stdout) + hooks = [entry for entry in report["coverage"]["primitives"] if entry["kind"] == "hooks"] + assert hooks and all(entry["status"] == "not-applicable" for entry in hooks) + assert all(entry["tracked"] is False for entry in hooks) + _assert_same_state( + before_audit, + LifecycleStateSnapshot.capture( + lifecycle.project.root, + targets=("claude",), + config_paths=configs, + ), + ) + _run( + lifecycle, + (*action, "--no-policy", "--parallel-downloads", "0"), + expected=0, + scenario_id=f"native-hooks-{action[-1]}", + ) + assert settings_path.read_bytes() == settings_bytes + assert transcript.read_bytes() == _BIDI_BYTES + + global_install = lifecycle.runner.run( + ( + "install", + "--global", + str(lifecycle.project.root), + "--target", + "copilot", + "--no-policy", + "--parallel-downloads", + "0", + ), + scenario_id="native-hooks-user-transition", + cwd=lifecycle.isolated.work_root, + env=lifecycle.environment, + ) + assert global_install.returncode == 0, _result_evidence(global_install) + user_instructions = lifecycle.isolated.home / ".copilot" / "copilot-instructions.md" + user_instructions.parent.mkdir(parents=True, exist_ok=True) + user_instructions.write_bytes(_BIDI_BYTES) + global_lock = lifecycle.isolated.config_root / "apm.lock.yaml" + before_lock = global_lock.read_bytes() + global_audit = lifecycle.runner.run( + ("audit", "--ci", "--no-policy", "--no-drift", "--no-fail-fast", "--format", "json"), + scenario_id="native-hooks-global-untracked-prompt", + cwd=lifecycle.isolated.config_root, + env=lifecycle.environment, + ) + assert global_audit.returncode == 1, _result_evidence(global_audit) + report = json.loads(global_audit.stdout) + check = next(check for check in report["checks"] if check["name"] == "content-integrity") + assert "unicode: .copilot/copilot-instructions.md" in check["details"] + assert global_lock.read_bytes() == before_lock + assert settings_path.read_bytes() == settings_bytes + assert user_instructions.read_bytes() == _BIDI_BYTES + assert not (lifecycle.project.root / "NEVER_EXECUTE").exists() + + def test_unrecorded_unicode_detect_strip_and_idempotent_audit( tmp_path: Path, apm_binary_path: Path, diff --git a/tests/unit/commands/test_audit_error_handling.py b/tests/unit/commands/test_audit_error_handling.py index b71e3a7b9f..8b70b36c91 100644 --- a/tests/unit/commands/test_audit_error_handling.py +++ b/tests/unit/commands/test_audit_error_handling.py @@ -31,6 +31,7 @@ _scan_single_file, ) from apm_cli.security.content_scanner import ScanFinding +from apm_cli.security.file_scanner import _FileScanResult # --------------------------------------------------------------------------- # Helpers @@ -447,8 +448,10 @@ def test_strip_with_no_findings_exits_zero(self, tmp_path: Path) -> None: with pytest.raises(SystemExit) as exc_info: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = tmp_path / "apm.lock.yaml" - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 3) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult( + {}, frozenset({"a.md", "b.md", "c.md"}) + ) _audit_content_scan(cfg, None, None, strip=True, dry_run=False) assert exc_info.value.code == 0 @@ -487,8 +490,8 @@ def test_text_format_with_output_path_exits(self, tmp_path: Path) -> None: with pytest.raises(SystemExit) as exc_info: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 1) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): @@ -512,8 +515,8 @@ def test_package_not_found_warning_and_exit(self, tmp_path: Path) -> None: with pytest.raises(SystemExit) as exc_info: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 0) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult({}, frozenset()) _audit_content_scan(cfg, "owner/repo", None, strip=False, dry_run=False) assert exc_info.value.code == 0 logger.warning.assert_called() @@ -534,8 +537,8 @@ def test_dry_run_without_strip_warns(self, tmp_path: Path) -> None: with pytest.raises(SystemExit): with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 1) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): diff --git a/tests/unit/commands/test_audit_phase3.py b/tests/unit/commands/test_audit_phase3.py index cb8c5280bb..f90d8d7c95 100644 --- a/tests/unit/commands/test_audit_phase3.py +++ b/tests/unit/commands/test_audit_phase3.py @@ -31,6 +31,7 @@ _scan_single_file, ) from apm_cli.security.content_scanner import ScanFinding +from apm_cli.security.file_scanner import _FileScanResult # --------------------------------------------------------------------------- # Helpers @@ -447,8 +448,10 @@ def test_strip_with_no_findings_exits_zero(self, tmp_path: Path) -> None: with pytest.raises(SystemExit) as exc_info: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = tmp_path / "apm.lock.yaml" - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 3) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult( + {}, frozenset({"a.md", "b.md", "c.md"}) + ) _audit_content_scan(cfg, None, None, strip=True, dry_run=False) assert exc_info.value.code == 0 @@ -487,8 +490,8 @@ def test_text_format_with_output_path_exits(self, tmp_path: Path) -> None: with pytest.raises(SystemExit) as exc_info: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 1) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): @@ -512,8 +515,8 @@ def test_package_not_found_warning_and_exit(self, tmp_path: Path) -> None: with pytest.raises(SystemExit) as exc_info: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 0) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult({}, frozenset()) _audit_content_scan(cfg, "owner/repo", None, strip=False, dry_run=False) assert exc_info.value.code == 0 logger.warning.assert_called() @@ -534,8 +537,8 @@ def test_dry_run_without_strip_warns(self, tmp_path: Path) -> None: with pytest.raises(SystemExit): with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file - with patch("apm_cli.commands.audit.scan_project_files") as mock_scan: - mock_scan.return_value = ({}, 1) + with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: + mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): diff --git a/tests/unit/policy/test_ci_checks.py b/tests/unit/policy/test_ci_checks.py index 9aa44df4c1..0d9f2d0efd 100644 --- a/tests/unit/policy/test_ci_checks.py +++ b/tests/unit/policy/test_ci_checks.py @@ -1065,7 +1065,7 @@ def test_mixed_pass_fail(self, tmp_path): def test_no_apm_yml(self, tmp_path): result = run_baseline_checks(tmp_path) assert result.passed - assert len(result.checks) == 1 # only lockfile-exists + assert [check.name for check in result.checks] == ["lockfile-exists", "content-integrity"] def test_stops_early_on_lockfile_missing(self, tmp_path): _write_apm_yml(tmp_path, deps=["owner/repo"]) diff --git a/tests/unit/test_audit_fail_on_drift.py b/tests/unit/test_audit_fail_on_drift.py index 62e6c4b916..772f5bab62 100644 --- a/tests/unit/test_audit_fail_on_drift.py +++ b/tests/unit/test_audit_fail_on_drift.py @@ -15,6 +15,8 @@ import pytest +from apm_cli.security.file_scanner import _FileScanResult + def _make_cfg(tmp_path): from apm_cli.commands.audit import _AuditConfig @@ -57,7 +59,11 @@ def _run(tmp_path, fail_on_drift, drift_ret=None): return_value=drift_ret if drift_ret is not None else _drift_return(), ), patch.object(audit_mod.LockFile, "read", return_value=MagicMock()), - patch.object(audit_mod, "scan_project_files", return_value=({}, 1)), + patch.object( + audit_mod, + "scan_project_result", + return_value=_FileScanResult({}, frozenset({"checked.md"})), + ), patch.object(audit_mod, "_resolve_fail_on_drift", return_value=fail_on_drift), patch("apm_cli.install.drift.render_drift_text", return_value=""), pytest.raises(SystemExit) as exc, diff --git a/tests/unit/test_security_file_scanner.py b/tests/unit/test_security_file_scanner.py index 18abf90431..9e02287db5 100644 --- a/tests/unit/test_security_file_scanner.py +++ b/tests/unit/test_security_file_scanner.py @@ -183,25 +183,18 @@ def test_scans_directory_entry(self, tmp_path: Path) -> None: dir_path = tmp_path / ".github" / "skills" / "pkg" dir_path.mkdir(parents=True) - mock_findings = {"inner.md": [MagicMock()]} - mock_verdict = MagicMock() - mock_verdict.findings_by_file = mock_findings - mock_verdict.files_scanned = 2 - mock_verdict.scanned_files = frozenset({"inner.md", "clean.md"}) + (dir_path / "inner.md").write_text("hidden \u202e", encoding="utf-8") + (dir_path / "clean.md").write_text("clean", encoding="utf-8") + (dir_path / "run.py").write_text("# hidden \u202e", encoding="utf-8") dep = _make_dep([".github/skills/pkg/"]) lock = _make_lockfile({"pkg": dep}) - with ( - patch("apm_cli.security.file_scanner.LockFile.read", return_value=lock), - patch( - "apm_cli.security.gate.SecurityGate.scan_files", - return_value=mock_verdict, - ), - ): - _findings, count = scan_lockfile_packages(tmp_path) + with patch("apm_cli.security.file_scanner.LockFile.read", return_value=lock): + findings, count = scan_lockfile_packages(tmp_path) assert count == 2 + assert set(findings) == {".github/skills/pkg/inner.md"} def test_package_filter_limits_scan(self, tmp_path: Path) -> None: file_a = tmp_path / ".github" / "prompts" / "a.md" From 555c4bc4fc14ada5dbab63059d7f85ddf769dbf9 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 10:12:33 +0200 Subject: [PATCH 03/12] fix(audit): preserve user-scope claim metadata Pass user scope to canonical claim path validation and update the legacy scanner test seam. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/security/file_scanner.py | 8 +++++--- .../test_audit_primitive_contract.py | 17 +++++++++++++++++ .../integration/test_coverage_gaps_phase4w2.py | 2 +- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/apm_cli/security/file_scanner.py b/src/apm_cli/security/file_scanner.py index b2cb3cc8c4..ad5f43df55 100644 --- a/src/apm_cli/security/file_scanner.py +++ b/src/apm_cli/security/file_scanner.py @@ -93,13 +93,13 @@ def _empty_scan() -> _FileScanResult: return _FileScanResult({}, frozenset()) -def _is_safe_lockfile_path(rel_path: str, project_root: Path) -> bool: +def _is_safe_lockfile_path(rel_path: str, project_root: Path, *, user_scope: bool = False) -> bool: """Return True if a relative path from the lockfile is safe to read. Reuses the same logic as ``BaseIntegrator.validate_deploy_path`` (no ``..``, allowed prefix, resolves within root). """ - return BaseIntegrator.validate_deploy_path(rel_path, project_root) + return BaseIntegrator.validate_deploy_path(rel_path, project_root, user_scope=user_scope) def _scan_directory_result(dir_path: Path, base_label: str) -> _FileScanResult: @@ -346,7 +346,9 @@ def _scan_claimed_files( None, ) external = surface is not None and surface.root != project_root - if not external and not _is_safe_lockfile_path(safe_path, project_root): + if not external and not _is_safe_lockfile_path( + safe_path, project_root, user_scope=user_scope + ): continue if not abs_path.exists(): continue diff --git a/tests/integration/test_audit_primitive_contract.py b/tests/integration/test_audit_primitive_contract.py index 31bdf275eb..ffc811eea8 100644 --- a/tests/integration/test_audit_primitive_contract.py +++ b/tests/integration/test_audit_primitive_contract.py @@ -440,6 +440,23 @@ def guarded_read(path: Path, *args: object, **kwargs: object) -> str: assert result.inventory == () +def test_user_scope_claims_keep_tracking_metadata(project: Path) -> None: + profile = KNOWN_TARGETS["copilot"].for_scope(user_scope=True) + assert profile is not None + path = ".copilot/copilot-instructions.md" + _write(project, path, _BIDI) + result = scan_project_result( + project, + lockfile=LockFile(local_deployed_files=[path]), + targets=(profile,), + user_scope=True, + include_deployed_trees=False, + ) + assert set(result.findings_by_file) == {path} + assert len(result.inventory) == 1 + assert result.inventory[0].tracked is True + + @pytest.mark.parametrize("ci_mode", [False, True]) def test_lockless_cli_still_discovers_prompt_primitives(project: Path, ci_mode: bool) -> None: (project / "apm.lock.yaml").unlink() diff --git a/tests/integration/test_coverage_gaps_phase4w2.py b/tests/integration/test_coverage_gaps_phase4w2.py index a54b7fb460..8a9e855ad7 100644 --- a/tests/integration/test_coverage_gaps_phase4w2.py +++ b/tests/integration/test_coverage_gaps_phase4w2.py @@ -1626,7 +1626,7 @@ def test_collects_findings_for_file_with_issues(self, tmp_path: Path) -> None: patch("apm_cli.security.file_scanner.ContentScanner") as mock_scanner, ): mock_lf.read.return_value = mock_lock - mock_scanner.scan_file.return_value = [finding] + mock_scanner.scan_text.return_value = [finding] findings, scanned = scan_lockfile_packages(tmp_path) assert scanned == 1 From 41f29e12a4bd9ecffa81e2c89a43c76276851c0a Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 10:15:02 +0200 Subject: [PATCH 04/12] docs(hooks): reference canonical native layouts in module overview Replace duplicated JSON examples with the native format authority and keep the integrator within the module-length guardrail. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/integration/hook_integrator.py | 35 ++-------------------- 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index fbc3d0517a..702b2e93ca 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -3,38 +3,9 @@ Supports VSCode Copilot (.github/hooks/), Claude Code (.claude/settings.json), and Cursor (.cursor/hooks.json) targets. -Hook JSON format (Claude Code -- nested matcher groups): - { - "hooks": { - "PreToolUse": [ - { - "hooks": [ - {"type": "command", "command": "./scripts/validate.sh", "timeout": 10} - ] - } - ] - } - } - -Hook JSON format (GitHub Copilot -- flat arrays with bash/powershell keys): - { - "version": 1, - "hooks": { - "preToolUse": [ - {"type": "command", "bash": "./scripts/validate.sh", "timeoutSec": 10} - ] - } - } - -Hook JSON format (Cursor -- flat arrays with command key): - { - "version": 1, - "hooks": { - "afterFileEdit": [ - {"command": "./hooks/format.sh"} - ] - } - } +Native handler layouts are declared in the merge-target registry below and +converted through ``hook_native_formats``: Claude uses nested matcher groups, +GitHub Copilot accepts bash/powershell commands, and Cursor uses flat commands. Script path handling: - Supported plugin-root aliases -> package-relative path rewritten for target From df7e236dde56c893d5e83d9390cfead0c6e4653a Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 10:56:04 +0200 Subject: [PATCH 05/12] fix(audit): fold coverage and remediation panel findings Separate CI inventory from violations, preserve denied-access coverage, bind external protection to surfaces, and share the deployed skills destination. Qualify decoded offsets, harden human rendering, deduplicate scans and extend regression traps. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../owners/hooks-integrations.json | 2 +- docs/src/content/docs/reference/cli/audit.md | 13 +- .../.apm/skills/apm-usage/commands.md | 6 +- .../checks/audit_discovery.py | 4 + src/apm_cli/commands/audit.py | 52 +++-- src/apm_cli/integration/skill_integrator.py | 6 +- src/apm_cli/integration/targets.py | 7 + src/apm_cli/policy/ci_checks.py | 11 +- src/apm_cli/policy/models.py | 28 ++- src/apm_cli/security/audit_report.py | 45 +++- src/apm_cli/security/file_scanner.py | 136 +++++++---- src/apm_cli/security/primitive_discovery.py | 24 +- src/apm_cli/utils/path_security.py | 17 +- .../test_architecture_audit_discovery.py | 16 ++ .../test_audit_primitive_contract.py | 219 +++++++++++++++++- ...test_audit_unrecorded_unicode_lifecycle.py | 20 ++ .../commands/test_audit_error_handling.py | 12 +- tests/unit/commands/test_audit_phase3.py | 12 +- .../test_skill_integrator_hermetic.py | 4 + .../test_skill_integrator_phase3w4.py | 4 + .../unit/scripts/test_architecture_runner.py | 2 +- tests/unit/test_audit_fail_on_drift.py | 4 +- 22 files changed, 520 insertions(+), 124 deletions(-) diff --git a/.apm/architecture/owners/hooks-integrations.json b/.apm/architecture/owners/hooks-integrations.json index c0263f5c08..516bf531aa 100644 --- a/.apm/architecture/owners/hooks-integrations.json +++ b/.apm/architecture/owners/hooks-integrations.json @@ -4,7 +4,7 @@ { "id": "audit-primitive-discovery", "decision": "Registry-derived deployed primitive discovery and applicable prompt coverage", - "owner": "security/primitive_discovery.py (primitive_surfaces), security/file_scanner.py (scan_project_result) through target and native hook contracts", + "owner": "security/primitive_discovery.py (primitive_surfaces), security/file_scanner.py (scan_project_result) through target skills_deploy_path and native hook contracts", "selectors": [ "src/apm_cli/security/primitive_discovery.py", "src/apm_cli/security/file_scanner.py" diff --git a/docs/src/content/docs/reference/cli/audit.md b/docs/src/content/docs/reference/cli/audit.md index 40ec1eb417..4d1d64a886 100644 --- a/docs/src/content/docs/reference/cli/audit.md +++ b/docs/src/content/docs/reference/cli/audit.md @@ -72,8 +72,12 @@ Non-CI JSON also includes `coverage.complete` and `coverage.primitives`. Each entry identifies its file, target, primitive kind, structured pointer, file-level `tracked` status and prompt-check status (`checked`, `not-applicable`, or `incomplete`). Structured findings include their prompt -pointer; SARIF does not invent a source line for a decoded JSON/TOML string. -CI reports hook inventory and coverage diagnostics under `content-integrity`. +pointer. JSON uses `coordinate_space: decoded-prompt`, `decoded_line` and +`decoded_column` for structured values instead of physical `line`/`column`; +text and Markdown label these offsets as decoded. SARIF omits a physical region. +CI JSON stores inventory in `content-integrity.primitive_coverage`, separate +from violation `details`. SARIF retains it in invocation `primitiveCoverage` +properties, and text shows discovered hooks even when all checks pass. ### CI gate @@ -201,7 +205,8 @@ not every file in a skill bundle. | OpenCode | `.opencode/agents/*.md`, `commands/*.md`; shared skills | None | | Grok Build / Cloud | `.grok/skills/**/SKILL.md`; Build also `rules/*.md`, `agents/*.md`, `commands/*.md` | None | | Agent Skills, OpenClaw, Hermes | `.agents/skills/**/SKILL.md` | None | -| Copilot Cowork / App | Managed skills / `workflows/*.prompt.md` in their resolved native roots | None | +| Copilot Cowork | `**/SKILL.md` directly under the resolved managed skills root | None | +| Copilot App | Deployed workflow prompts are SQLite rows, not filesystem primitives; this scan does not cover them | None | Directory shorthand in a row stays under that row's target root. Shared skills use `.agents/skills/**/SKILL.md`. A target's generated context files and @@ -287,7 +292,7 @@ as metadata repair; see [`apm prune`](../prune/#canonical-deployment-ownership). | Code | Meaning | |---|---| | `0` | Clean, info-only findings, drift-only (advisory) in bare audit, or successful `--strip`. | -| `1` | Critical findings detected, or an invalid canonical deployment-owner reference in `apm.lock.yaml` (always hard-fails, unlike ordinary drift). | +| `1` | Critical findings, incomplete primitive coverage (including unreadable parents), refused `--strip` remediation, or an invalid canonical deployment-owner reference in `apm.lock.yaml`. | | `2` | Warning-only findings, or usage error (mutually exclusive flags). | | `3` | Configuration or infrastructure error (feature not enabled, scanner not found, malformed SARIF). | diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 7e5b08b57e..e8ffd783a2 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -260,7 +260,11 @@ unrelated settings, transcripts or caches. Command-only hooks remain visible and non-failing; hooks are never executed. Unreadable or unsupported recognized content fails with incomplete coverage, including with `--no-drift`/`--no-policy`. Non-CI JSON exposes `coverage.complete` and `coverage.primitives`; CI includes -inventory and diagnostics under `content-integrity`. Tracking is file-level, +inventory in `content-integrity.primitive_coverage`, separate from violation +`details`; SARIF uses invocation `primitiveCoverage` properties, including on +passing checks. Structured findings identify pointers and decoded offsets, +not physical file line/column positions. Copilot App's SQLite workflow prompts +are not covered by filesystem discovery. Tracking is file-level, not entry ownership or hash verification. Automatic `--strip` refuses structured/shared or external-root findings and incomplete coverage without rewriting files. Explicit `--file` remains user-directed. diff --git a/scripts/architecture_linter/checks/audit_discovery.py b/scripts/architecture_linter/checks/audit_discovery.py index 2c4a807fda..40917f6893 100644 --- a/scripts/architecture_linter/checks/audit_discovery.py +++ b/scripts/architecture_linter/checks/audit_discovery.py @@ -20,9 +20,13 @@ "scan_project_result": {"_scan_claimed_files", "_scan_deployed_trees"}, }, _DISCOVERY: { + "_mapping_surface": {"skills_deploy_path"}, "primitive_surfaces": {"native_hook_config", "_mapping_surface", "root_context_filename"}, "safe_surface_path": {"ensure_path_within", "has_symlink_component"}, }, + "src/apm_cli/integration/skill_integrator.py": { + "_target_skills_root": {"skills_deploy_path"}, + }, "src/apm_cli/integration/hook_native_formats.py": { "inspect_native_hooks": {"hook_handlers"}, }, diff --git a/src/apm_cli/commands/audit.py b/src/apm_cli/commands/audit.py index 88159d99ff..b41ca06e19 100644 --- a/src/apm_cli/commands/audit.py +++ b/src/apm_cli/commands/audit.py @@ -34,6 +34,7 @@ _rich_error, _rich_success, ) +from ..utils.diagnostics import printable_ascii_text # -- Shared config -------------------------------------------------- @@ -145,6 +146,8 @@ def _render_findings_table( verbose: bool = False, ) -> None: """Render a Rich table of scan findings.""" + from ..security.audit_report import finding_location, relative_path_for_report + console = _get_console() # Flatten into rows, sorted by severity (critical first) @@ -167,8 +170,7 @@ def _render_findings_table( if console: try: from rich.table import Table - - from ..security.audit_report import relative_path_for_report + from rich.text import Text table = Table( title=title, @@ -179,7 +181,7 @@ def _render_findings_table( if has_external: table.add_column("Source", style="cyan", width=14) table.add_column("File", style="white") - table.add_column("Location", style="dim", width=10) + table.add_column("Location", style="dim") if has_external: table.add_column("Category", style="bold white") else: @@ -205,13 +207,13 @@ def _render_findings_table( row_cells.extend( [ relative_path_for_report(f.file), - f"{f.line}:{f.column}", + finding_location(f), category_or_codepoint, f.description, ] ) table.add_row( - *row_cells, + *(Text(printable_ascii_text(cell)) for cell in row_cells), style=sev_styles.get(f.severity, "white"), ) console.print() @@ -231,8 +233,10 @@ def _render_findings_table( source_part = f" [{_finding_source(f)}]" if has_external else "" detail = f.category if has_external else f.codepoint _rich_echo( - f" {sev_label:<10}{source_part} {f.file} {f.line}:{f.column} {detail} " - f"{f.description}", + printable_ascii_text( + f" {sev_label:<10}{source_part} {f.file} {finding_location(f)} {detail} " + f"{f.description}" + ), color=color, ) @@ -298,6 +302,11 @@ def _render_summary( warning = counts.get("warning", 0) info = counts.get("info", 0) affected = len(findings_by_file) + remedy = ( + "Review structured or external prompt fields manually; --strip does not rewrite native settings." + if any(not entry.strippable and entry.file in findings_by_file for entry in coverage) + else " Review file contents, then run 'apm audit --strip' to remove hidden characters" + ) _rich_echo("") if critical > 0: @@ -305,14 +314,10 @@ def _render_summary( f"{critical} critical finding(s) in {affected} file(s) -- hidden characters detected" ) logger.progress(" These characters may embed invisible instructions") - logger.progress( - "Review structured prompt fields manually; --strip does not rewrite native settings." - if any(not entry.strippable and entry.status == "checked" for entry in coverage) - else " Review file contents, then run 'apm audit --strip' to remove" - ) + logger.progress(remedy) elif warning > 0: logger.warning(f"{warning} warning(s) in {affected} file(s) -- hidden characters detected") - logger.progress(" Run 'apm audit --strip' to remove hidden characters") + logger.progress(remedy) elif info > 0: logger.progress( f"{info} info-level finding(s) in " @@ -327,13 +332,11 @@ def _render_summary( def _render_coverage(coverage: tuple[CoverageEntry, ...], logger: CommandLogger) -> None: """Keep discovery visible without inventing ownership or unsafe-content findings.""" - from ..utils.diagnostics import printable_ascii_text - for entry in coverage: label = printable_ascii_text(entry.file + entry.pointer) if entry.status == "incomplete": logger.error( - f"Incomplete coverage: {label}: {entry.diagnostic}. " + f"Incomplete coverage: {label}: {printable_ascii_text(entry.diagnostic or '')}. " "Review the format or file access and rerun audit." ) elif entry.kind == "hooks": @@ -480,10 +483,13 @@ def _render_ci_results(ci_result: "CIAuditResult") -> None: """Render CI check results as a Rich table (text format).""" console = _get_console() + for check in ci_result.checks: + _render_coverage(check.coverage, CommandLogger("audit")) if console: try: from rich.table import Table + from rich.text import Text table = Table( title=f"{STATUS_SYMBOLS['search']} APM Policy Compliance", @@ -500,7 +506,7 @@ def _render_ci_results(ci_result: "CIAuditResult") -> None: if check.passed else f"[red]{STATUS_SYMBOLS['cross']}[/red]" ) - table.add_row(status, check.name, check.message) + table.add_row(status, Text(check.name), Text(printable_ascii_text(check.message))) console.print() console.print(table) @@ -515,7 +521,7 @@ def _render_ci_results(ci_result: "CIAuditResult") -> None: bold=True, ) for detail in check.details: - _rich_echo(f" - {detail}", color="dim") + _rich_echo(f" - {printable_ascii_text(detail)}", color="dim") console.print() summary = ci_result.to_json()["summary"] @@ -540,10 +546,10 @@ def _render_ci_results(ci_result: "CIAuditResult") -> None: for check in ci_result.checks: symbol = STATUS_SYMBOLS["check"] if check.passed else STATUS_SYMBOLS["cross"] color = "green" if check.passed else "red" - _rich_echo(f" {symbol} {check.name}: {check.message}", color=color) + _rich_echo(printable_ascii_text(f" {symbol} {check.name}: {check.message}"), color=color) if not check.passed and check.details: for detail in check.details: - _rich_echo(f" - {detail}", color="dim") + _rich_echo(f" - {printable_ascii_text(detail)}", color="dim") _rich_echo("") summary = ci_result.to_json()["summary"] @@ -1000,6 +1006,12 @@ def _audit_content_scan( blocked = protected_files.intersection(findings_by_file) if blocked or any(entry.status == "incomplete" for entry in coverage): _render_coverage(coverage, logger) + for entry in coverage: + if entry.file in blocked and not entry.strippable: + logger.error_detail( + "Manual review required: " + + printable_ascii_text(entry.file + entry.pointer) + ) logger.error( "Content was not modified: structured/shared or external prompt findings, or incomplete coverage, " "require manual review. --strip does not rewrite native configuration." diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index cd95438936..ed3d7aabf6 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/src/apm_cli/integration/skill_integrator.py @@ -394,11 +394,7 @@ def __init__(self) -> None: @staticmethod def _target_skills_root(target: TargetProfile, project_root: Path) -> Path: """Return the target skills root for static and dynamic-root targets.""" - if target.resolved_deploy_root is not None: - return target.deploy_path(project_root) - skills_mapping = target.primitives["skills"] - effective_root = skills_mapping.deploy_root or target.root_dir - return project_root / effective_root / "skills" + return target.skills_deploy_path(project_root) @staticmethod def _target_skill_dir(target: TargetProfile, project_root: Path, skill_name: str) -> Path: diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index bf5b2e976b..f2297a58d8 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -338,6 +338,13 @@ def supports_at_user_scope(self, primitive: str) -> bool: return False return primitive in self.primitives + def skills_deploy_path(self, project_root: Path) -> Path: + """Return the actual skills root for static and resolved dynamic targets.""" + if self.resolved_deploy_root is not None: + return self.deploy_path(project_root) + mapping = self.primitives["skills"] + return project_root / (mapping.deploy_root or self.root_dir) / "skills" + def deploy_path(self, project_root: Path, *parts: str) -> Path: """Return the filesystem path for deployment. diff --git a/src/apm_cli/policy/ci_checks.py b/src/apm_cli/policy/ci_checks.py index e28849ea02..cdc653501d 100644 --- a/src/apm_cli/policy/ci_checks.py +++ b/src/apm_cli/policy/ci_checks.py @@ -525,12 +525,6 @@ def _check_content_integrity( user_scope=user_scope, ) findings_by_file = scan.findings_by_file - inventory_details = [ - f"discovered/{'recorded-file' if entry.tracked else 'untracked'}: " - f"{entry.file}{entry.pointer} (prompt check: {entry.status})" - for entry in scan.inventory - if entry.kind == "hooks" - ] # Only critical findings fail this check critical_files: list[str] = [] @@ -622,10 +616,10 @@ def _has_symlink_component(path: Path) -> bool: name="content-integrity", passed=True, message="No critical hidden Unicode or hash drift detected", - details=inventory_details, + coverage=scan.inventory, ) - details: list[str] = list(inventory_details) + details: list[str] = [] for entry in scan.incomplete: details.append(f"incomplete-coverage: {entry.file}{entry.pointer}: {entry.diagnostic}") for rel_path in critical_files: @@ -673,6 +667,7 @@ def _has_symlink_component(path: Path) -> bool: passed=False, message=f"{summary} -- run {remedy}", details=details, + coverage=scan.inventory, ) diff --git a/src/apm_cli/policy/models.py b/src/apm_cli/policy/models.py index df8c87b685..972ed7577b 100644 --- a/src/apm_cli/policy/models.py +++ b/src/apm_cli/policy/models.py @@ -6,7 +6,11 @@ from __future__ import annotations -from dataclasses import dataclass, field +from dataclasses import asdict, dataclass, field +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from apm_cli.security.file_scanner import CoverageEntry # Check name -> most relevant artifact for SARIF locations. _CHECK_ARTIFACT_MAP: dict[str, str] = { @@ -50,6 +54,7 @@ class CheckResult: passed: bool message: str # human-readable description details: list[str] = field(default_factory=list) # individual violations + coverage: tuple[CoverageEntry, ...] = () @dataclass @@ -76,6 +81,11 @@ def to_json(self) -> dict: "passed": c.passed, "message": c.message, "details": c.details, + **( + {"primitive_coverage": [asdict(entry) for entry in c.coverage]} + if c.coverage + else {} + ), } for c in self.checks ], @@ -140,6 +150,22 @@ def to_sarif(self) -> dict: }, }, "results": results, + "invocations": [ + { + "executionSuccessful": not any( + entry.status == "incomplete" + for check in self.checks + for entry in check.coverage + ), + "properties": { + "primitiveCoverage": [ + asdict(entry) + for check in self.checks + for entry in check.coverage + ], + }, + } + ], } ], } diff --git a/src/apm_cli/security/audit_report.py b/src/apm_cli/security/audit_report.py index 0c014fc49f..e03b9731b7 100644 --- a/src/apm_cli/security/audit_report.py +++ b/src/apm_cli/security/audit_report.py @@ -3,11 +3,13 @@ from __future__ import annotations import json +import re from dataclasses import asdict from pathlib import Path from typing import TYPE_CHECKING, Any from ..core.deployment_ledger import DEPLOYMENT_OWNER_REMEDIATION +from ..utils.diagnostics import printable_ascii_text from .content_scanner import ScanFinding if TYPE_CHECKING: @@ -26,6 +28,24 @@ def relative_path_for_report(file_path: str) -> str: return file_path.replace("\\", "/") +def finding_location(finding: ScanFinding) -> str: + """Distinguish physical source positions from decoded structured prompt offsets.""" + offset = f"{finding.line}:{finding.column}" + return f"{finding.pointer} (decoded {offset})" if finding.pointer else offset + + +def _markdown_cell(value: str) -> str: + """Render untrusted labels as printable, literal Markdown table content.""" + return re.sub(r"([\\`*_{}\[\]<>()|#!])", r"\\\1", printable_ascii_text(value)) + + +def _markdown_code(value: str) -> str: + """Keep filename code spans literal even when the filename contains backticks.""" + value = printable_ascii_text(value).replace("|", "\\|") + delimiter = "`" * (1 + max((len(run) for run in re.findall(r"`+", value)), default=0)) + return f"{delimiter} {value} {delimiter}" if "`" in value else f"{delimiter}{value}{delimiter}" + + # SARIF schema version _SARIF_VERSION = "2.1.0" _SARIF_SCHEMA = ( @@ -107,8 +127,15 @@ def findings_to_json( { "severity": finding.severity, "file": relative_path_for_report(finding.file), - "line": finding.line, - "column": finding.column, + **( + { + "coordinate_space": "decoded-prompt", + "decoded_line": finding.line, + "decoded_column": finding.column, + } + if finding.pointer + else {"line": finding.line, "column": finding.column} + ), "codepoint": finding.codepoint, "category": finding.category, "description": finding.description, @@ -296,14 +323,14 @@ def write_report(report: dict, output_path: Path) -> None: """Write a report dict (JSON or SARIF) to a file.""" output_path.parent.mkdir(parents=True, exist_ok=True) output_path.write_text( - json.dumps(report, indent=2, ensure_ascii=False) + "\n", + json.dumps(report, indent=2, ensure_ascii=True) + "\n", encoding="utf-8", ) def serialize_report(report: dict) -> str: """Serialize a report dict to a JSON string (for stdout).""" - return json.dumps(report, indent=2, ensure_ascii=False) + return json.dumps(report, indent=2, ensure_ascii=True) def findings_to_markdown( @@ -337,9 +364,7 @@ def findings_to_markdown( entry.diagnostic or "", ) coverage_lines.append( - "| " - + " | ".join(value.replace("|", "\\|").replace("\n", " ") for value in values) - + " |" + "| " + " | ".join(_markdown_cell(value) for value in values) + " |" ) incomplete = any(entry.status == "incomplete" for entry in coverage) if not all_findings and not owner_violations and not incomplete: @@ -415,10 +440,10 @@ def findings_to_markdown( ) for finding in sorted_findings: severity = finding.severity.upper() - escaped_desc = finding.description.replace("|", "\\|") + escaped_desc = _markdown_cell(finding.description) lines.append( - f"| {severity} | `{relative_path_for_report(finding.file)}` | " - f"{finding.line}:{finding.column} | `{finding.codepoint}` | " + f"| {severity} | {_markdown_code(relative_path_for_report(finding.file))} | " + f"{_markdown_cell(finding_location(finding))} | `{finding.codepoint}` | " f"{escaped_desc} |" ) lines.extend( diff --git a/src/apm_cli/security/file_scanner.py b/src/apm_cli/security/file_scanner.py index ad5f43df55..d551b1559b 100644 --- a/src/apm_cli/security/file_scanner.py +++ b/src/apm_cli/security/file_scanner.py @@ -52,7 +52,7 @@ class CoverageEntry: @dataclass(frozen=True) -class _FileScanResult: +class FileScanResult: """Findings plus the exact repository-relative files examined.""" findings_by_file: dict[str, list[ScanFinding]] @@ -69,12 +69,12 @@ def protected_files(self) -> frozenset[str]: """Keep shared/structured content out of whole-file Unicode remediation.""" return frozenset(entry.file for entry in self.inventory if not entry.strippable) - def merged(self, other: _FileScanResult) -> _FileScanResult: + def merged(self, other: FileScanResult) -> FileScanResult: """Union scopes, treating ``self`` as the authoritative first scope.""" return _merge_results((self, other)) -def _merge_results(results: Iterable[_FileScanResult]) -> _FileScanResult: +def _merge_results(results: Iterable[FileScanResult]) -> FileScanResult: """Union scan evidence in linear time, retaining the first scope's metadata.""" findings: dict[str, list[ScanFinding]] = {} scanned: set[str] = set() @@ -85,12 +85,12 @@ def _merge_results(results: Iterable[_FileScanResult]) -> _FileScanResult: scanned.update(result.scanned_files) for entry in result.inventory: inventory.setdefault((entry.file, entry.pointer), entry) - return _FileScanResult(findings, frozenset(scanned), tuple(inventory.values())) + return FileScanResult(findings, frozenset(scanned), tuple(inventory.values())) -def _empty_scan() -> _FileScanResult: +def _empty_scan() -> FileScanResult: """Return an empty independent scan result.""" - return _FileScanResult({}, frozenset()) + return FileScanResult({}, frozenset()) def _is_safe_lockfile_path(rel_path: str, project_root: Path, *, user_scope: bool = False) -> bool: @@ -102,12 +102,12 @@ def _is_safe_lockfile_path(rel_path: str, project_root: Path, *, user_scope: boo return BaseIntegrator.validate_deploy_path(rel_path, project_root, user_scope=user_scope) -def _scan_directory_result(dir_path: Path, base_label: str) -> _FileScanResult: +def _scan_directory_result(dir_path: Path, base_label: str) -> FileScanResult: """Recursively scan a directory and label exact project-relative paths.""" from ..security.gate import REPORT_POLICY, SecurityGate verdict = SecurityGate.scan_files(dir_path, policy=REPORT_POLICY) - return _FileScanResult( + return FileScanResult( findings_by_file={ f"{base_label}/{rel_path}": file_findings for rel_path, file_findings in verdict.findings_by_file.items() @@ -184,12 +184,20 @@ def _content_entries(path: Path, surface: PrimitiveSurface) -> tuple[HookContent def _scan_primitive( - path: Path, surface: PrimitiveSurface, label: str, *, tracked: bool -) -> _FileScanResult: + path: Path, + surface: PrimitiveSurface, + label: str, + *, + tracked: bool, + cache: dict[PrimitiveSurface, FileScanResult] | None = None, +) -> FileScanResult: """Check one recognized surface and retain non-applicability/coverage evidence.""" - if not safe_surface_path(surface, path): - return _empty_scan() + key = replace(surface, path=path, pattern=None) try: + if not safe_surface_path(surface, path): + return _empty_scan() + if cache is not None and key in cache: + return cache[key] entries = _content_entries(path, surface) except (OSError, UnicodeError, ValueError) as exc: entries = ( @@ -218,7 +226,7 @@ def _scan_primitive( not surface.shared and not entry.pointer and status == "checked" - and not label.startswith(f"{surface.target}:") + and not surface.external ), ) ) @@ -227,19 +235,21 @@ def _scan_primitive( findings.append( replace( finding, - description=f"{finding.description} (prompt field {entry.pointer})", pointer=entry.pointer, ) if entry.pointer else finding ) - return _FileScanResult( + result = FileScanResult( {label: findings} if findings else {}, frozenset({label}) if any(entry.status == "checked" for entry in inventory) else frozenset(), tuple(inventory), ) + if cache is not None: + cache[key] = result + return result def _scan_deployed_trees( @@ -247,7 +257,8 @@ def _scan_deployed_trees( targets: Sequence[TargetProfile] = (), *, user_scope: bool = False, -) -> _FileScanResult: + cache: dict[PrimitiveSurface, FileScanResult] | None = None, +) -> FileScanResult: """Discover recognized primitives without consulting a deployment baseline.""" from ..integration.targets import resolve_targets @@ -256,22 +267,24 @@ def _scan_deployed_trees( if targets else resolve_targets(project_root, user_scope=user_scope, create_config=False) ) - results: list[_FileScanResult] = [] - visited: set[Path] = set() + results: list[FileScanResult] = [] + cache = {} if cache is None else cache for surface in primitive_surfaces(project_root, scoped, user_scope=user_scope): try: for path in iter_surface_files(surface): - if path not in visited: - visited.add(path) - results.append( - _scan_primitive( - path, surface, _label(path, surface, project_root), tracked=False - ) + results.append( + _scan_primitive( + path, + surface, + _label(path, surface, project_root), + tracked=False, + cache=cache, ) + ) except OSError as exc: label = _label(surface.path, surface, project_root) results.append( - _FileScanResult( + FileScanResult( {}, frozenset(), ( @@ -303,7 +316,8 @@ def _scan_claimed_files( targets: Sequence[TargetProfile] = (), *, user_scope: bool = False, -) -> _FileScanResult: + cache: dict[PrimitiveSurface, FileScanResult] | None = None, +) -> FileScanResult: """Scan one canonical deployment-claim projection.""" from ..integration.targets import KNOWN_TARGETS @@ -316,7 +330,8 @@ def _scan_claimed_files( and not profile.user_root_resolver ] surfaces = primitive_surfaces(project_root, profiles, user_scope=user_scope) - results: list[_FileScanResult] = [] + results: list[FileScanResult] = [] + cache = {} if cache is None else cache for rel_path, owner in claims.items(): if package_filter and owner != package_filter: continue @@ -336,13 +351,38 @@ def _scan_claimed_files( if decoded is None: continue abs_path = decoded + candidates = [ + s + for s in surfaces + if s.contains(abs_path) or (s.pattern is not None and abs_path.is_relative_to(s.path)) + ] + try: + if candidates and not safe_surface_path(candidates[0], abs_path): + continue + is_directory = abs_path.is_dir() + except OSError as exc: + if candidates: + candidate = candidates[0] + results.append( + FileScanResult( + {}, + frozenset(), + ( + CoverageEntry( + rel_path, + candidate.target, + candidate.kind, + "", + True, + "incomplete", + f"cannot inspect recorded primitive ({type(exc).__name__})", + ), + ), + ) + ) + continue surface = next( - ( - s - for s in surfaces - if s.contains(abs_path) - or (s.pattern is not None and abs_path.is_dir() and abs_path.is_relative_to(s.path)) - ), + (s for s in candidates if s.contains(abs_path) or is_directory), None, ) external = surface is not None and surface.root != project_root @@ -357,14 +397,12 @@ def _scan_claimed_files( surface = PrimitiveSurface( project_root, abs_path, - "*.md" if abs_path.is_dir() else None, + "*.md" if is_directory else None, "recorded", - "document" - if abs_path.suffix in {".md", ".mdc"} or abs_path.is_dir() - else "executable", + "document" if abs_path.suffix in {".md", ".mdc"} or is_directory else "executable", "recorded", ) - if abs_path.is_dir(): + if is_directory: directory_surfaces = [ replace(candidate, path=abs_path) if candidate.pattern and abs_path.is_relative_to(candidate.path) @@ -382,11 +420,12 @@ def _scan_claimed_files( directory_surface, _label(path, directory_surface, project_root), tracked=True, + cache=cache, ) ) except OSError as exc: results.append( - _FileScanResult( + FileScanResult( {}, frozenset(), ( @@ -405,7 +444,11 @@ def _scan_claimed_files( continue results.append( _scan_primitive( - abs_path, surface, _label(abs_path, surface, project_root), tracked=True + abs_path, + surface, + _label(abs_path, surface, project_root), + tracked=True, + cache=cache, ) ) @@ -416,7 +459,7 @@ def _scan_lockfile_packages( project_root: Path, package_filter: str | None = None, lockfile: LockFile | None = None, -) -> _FileScanResult: +) -> FileScanResult: """Collect the exact lockfile-driven scan result.""" lock = lockfile if lockfile is not None else LockFile.read(get_lockfile_path(project_root)) if lock is None: @@ -480,7 +523,7 @@ def scan_project_result( include_deployed_trees: bool = True, targets: Sequence[TargetProfile] = (), user_scope: bool = False, -) -> _FileScanResult: +) -> FileScanResult: """Return findings, discovery inventory and explicit incomplete coverage.""" from ..core.deployment_ledger import DeploymentLedgerCodec from ..core.scope import get_workspace_deploy_root @@ -503,7 +546,12 @@ def scan_project_result( ) lock = lockfile if lockfile is not None else LockFile.read(get_lockfile_path(project_root)) claims = DeploymentLedgerCodec.legacy_deployed_file_claims(lock) if lock else {} - result = _scan_claimed_files(deploy_root, claims, package_filter, scoped, user_scope=user_scope) + cache: dict[PrimitiveSurface, FileScanResult] = {} + result = _scan_claimed_files( + deploy_root, claims, package_filter, scoped, user_scope=user_scope, cache=cache + ) if include_deployed_trees: - result = result.merged(_scan_deployed_trees(deploy_root, scoped, user_scope=user_scope)) + result = result.merged( + _scan_deployed_trees(deploy_root, scoped, user_scope=user_scope, cache=cache) + ) return result diff --git a/src/apm_cli/security/primitive_discovery.py b/src/apm_cli/security/primitive_discovery.py index 2205c9ebb0..24c3834c1c 100644 --- a/src/apm_cli/security/primitive_discovery.py +++ b/src/apm_cli/security/primitive_discovery.py @@ -7,6 +7,7 @@ from collections.abc import Iterator, Sequence from dataclasses import dataclass from pathlib import Path +from stat import S_ISLNK from apm_cli.compilation.root_context_protection import root_context_filename from apm_cli.integration.hook_integrator import native_hook_config @@ -30,6 +31,7 @@ class PrimitiveSurface: format_id: str prompt_fields: tuple[str, ...] = () shared: bool = False + external: bool = False def contains(self, path: Path) -> bool: """Match a path without walking or resolving user-controlled links.""" @@ -52,7 +54,7 @@ def _mapping_surface( ) if mapping.deploy_root: root = project_root - path = base / mapping.subdir + path = target.skills_deploy_path(project_root) if kind == "skills" else base / mapping.subdir if not mapping.subdir: if mapping.extension.startswith("."): # Root-level aggregate mappings declare exact generated filenames. @@ -64,7 +66,14 @@ def _mapping_surface( else: pattern = f"*{mapping.extension}" return PrimitiveSurface( - root, path, pattern, target.name, kind, mapping.format_id, mapping.prompt_fields + root, + path, + pattern, + target.name, + kind, + mapping.format_id, + mapping.prompt_fields, + external=root != project_root, ) @@ -91,6 +100,7 @@ def primitive_surfaces( target.name, "context", "markdown", + external=root != project_root, ) ) context = root_context_filename(target.compile_family) @@ -106,6 +116,7 @@ def primitive_surfaces( target.name, "context", "markdown", + external=user_scope and root != project_root, ) ) config = native_hook_config(target.name) @@ -120,6 +131,7 @@ def primitive_surfaces( "hooks", mapping.format_id, shared=True, + external=root != project_root, ) ) # Exact shared configs override an identical primitive filename (hooks.json). @@ -128,11 +140,15 @@ def primitive_surfaces( def safe_surface_path(surface: PrimitiveSurface, path: Path) -> bool: """Apply containment and reject symlinks at every component before reading.""" - if surface.root.is_symlink() or has_symlink_component(surface.root, path): + try: + root_linked = S_ISLNK(surface.root.lstat().st_mode) + except FileNotFoundError: + return False + if root_linked or has_symlink_component(surface.root, path, raise_on_error=True): return False try: ensure_path_within(path, surface.root) - except (PathTraversalError, OSError, RuntimeError): + except (PathTraversalError, RuntimeError): return False return True diff --git a/src/apm_cli/utils/path_security.py b/src/apm_cli/utils/path_security.py index 72ecb926b3..247f8dd532 100644 --- a/src/apm_cli/utils/path_security.py +++ b/src/apm_cli/utils/path_security.py @@ -21,6 +21,7 @@ import urllib.parse as _up from pathlib import Path +from stat import S_ISLNK from .file_ops import robust_rmtree @@ -218,17 +219,25 @@ def ensure_path_within_resolved(path: Path, resolved_base: Path) -> Path: return resolved -def has_symlink_component(base_dir: Path, path: Path) -> bool: - """Return whether any component of *path* below *base_dir* is a symlink.""" +def has_symlink_component(base_dir: Path, path: Path, *, raise_on_error: bool = False) -> bool: + """Reject linked components, optionally preserving access errors for diagnostics.""" try: relative = path.relative_to(base_dir) current = base_dir for part in relative.parts: current /= part - if current.is_symlink(): + try: + linked = S_ISLNK(current.lstat().st_mode) + except FileNotFoundError: + return False + if linked: return True return False - except (OSError, ValueError): + except OSError: + if raise_on_error: + raise + return True + except ValueError: return True diff --git a/tests/integration/test_architecture_audit_discovery.py b/tests/integration/test_architecture_audit_discovery.py index dbd593fff0..392d523ddf 100644 --- a/tests/integration/test_architecture_audit_discovery.py +++ b/tests/integration/test_architecture_audit_discovery.py @@ -37,3 +37,19 @@ def test_guard_rejects_discovery_applicability_or_containment_bypass(old: str, n result = run_selected_rules(_ROOT, {_RULE}, source_overrides={_SCANNER: mutated}) assert result.failures == () assert any(v.rule_id == _RULE and v.path == _SCANNER for v in result.violations) + + +@pytest.mark.parametrize( + "path", + [ + "src/apm_cli/security/primitive_discovery.py", + "src/apm_cli/integration/skill_integrator.py", + ], +) +def test_skills_destination_has_one_authority(path: str) -> None: + source = (_ROOT / path).read_text(encoding="utf-8") + mutated = source.replace("target.skills_deploy_path(", "target.bypass_skills_root(") + assert mutated != source + result = run_selected_rules(_ROOT, {_RULE}, source_overrides={path: mutated}) + assert result.failures == () + assert any(v.rule_id == _RULE and v.path == path for v in result.violations) diff --git a/tests/integration/test_audit_primitive_contract.py b/tests/integration/test_audit_primitive_contract.py index ffc811eea8..eeac9539a6 100644 --- a/tests/integration/test_audit_primitive_contract.py +++ b/tests/integration/test_audit_primitive_contract.py @@ -77,8 +77,9 @@ def test_shared_settings_command_inventory_is_visible_and_non_failing( report = json.loads(result.output) if ci_mode: check = next(c for c in report["checks"] if c["name"] == "content-integrity") - assert any("discovered/untracked:" in detail for detail in check["details"]) - assert any("not-applicable" in detail for detail in check["details"]) + assert check["details"] == [] + assert check["primitive_coverage"][0]["tracked"] is False + assert check["primitive_coverage"][0]["status"] == "not-applicable" else: assert report["summary"]["files_scanned"] == 0 assert report["findings"] == [] @@ -387,19 +388,19 @@ def test_native_formats_apply_only_documented_prompt_fields( def test_scope_resolved_external_root_and_symlink_boundaries(project: Path) -> None: external = project.parent / "external-claude" - _write(external, "skills/manual/SKILL.md", _BIDI) + _write(external, "manual/SKILL.md", _BIDI) _write( external, "settings.json", '{"hooks":{"Stop":[{"hooks":[{"type":"prompt","prompt":"\\u202e"}]}]}}', ) outside = _write(project.parent, "outside/SKILL.md", _BIDI) - (external / "skills" / "linked").symlink_to(outside.parent, target_is_directory=True) + (external / "linked").symlink_to(outside.parent, target_is_directory=True) profile = replace(KNOWN_TARGETS["claude"], resolved_deploy_root=external) result = scan_project_result(project, targets=(profile,), user_scope=True) assert result.scanned_files == frozenset( { - "claude:skills/manual/SKILL.md", + "claude:manual/SKILL.md", "claude:settings.json", } ) @@ -407,6 +408,214 @@ def test_scope_resolved_external_root_and_symlink_boundaries(project: Path) -> N assert result.protected_files == result.scanned_files +@pytest.mark.parametrize("dirty", [False, True]) +@pytest.mark.parametrize("output_format", ["text", "json", "sarif"]) +def test_ci_inventory_is_informational_on_pass_and_failure( + project: Path, dirty: bool, output_format: str +) -> None: + _settings(project) + if dirty: + _write(project, ".claude/rules/dirty.md", _BIDI) + result = CliRunner().invoke( + cli, + ["audit", "--ci", "--no-policy", "--no-drift", "--no-fail-fast", "--format", output_format], + ) + assert result.exit_code == int(dirty), result.output + if output_format == "text": + assert "Discovered/untracked:" in result.output + assert "not-applicable" in " ".join(result.output.split()) + elif output_format == "json": + check = next( + c for c in json.loads(result.output)["checks"] if c["name"] == "content-integrity" + ) + assert any(e["status"] == "not-applicable" for e in check["primitive_coverage"]) + assert check["details"] == (["unicode: .claude/rules/dirty.md"] if dirty else []) + else: + run = json.loads(result.output)["runs"][0] + entries = run["invocations"][0]["properties"]["primitiveCoverage"] + assert any(e["status"] == "not-applicable" for e in entries) + assert len(run["results"]) == int(dirty) + assert all("not-applicable" not in r["message"]["text"] for r in run["results"]) + + +@pytest.mark.parametrize("ci_mode", [False, True]) +def test_incomplete_coverage_sarif_never_claims_success(project: Path, ci_mode: bool) -> None: + _write(project, _SETTINGS, '{"hooks":') + args = ["audit", "--no-drift", "--format", "sarif"] + if ci_mode: + args += ["--ci", "--no-policy", "--no-fail-fast"] + result = CliRunner().invoke(cli, args) + assert result.exit_code == 1, result.output + run = json.loads(result.output)["runs"][0] + assert run["invocations"][0]["executionSuccessful"] is False + assert len(run["results"]) == 1 + finding = run["results"][0] + assert finding["level"] == "error" + if not ci_mode: + assert finding["ruleId"] == "apm/audit/incomplete-coverage" + assert finding["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] == _SETTINGS + + +@pytest.mark.parametrize("tracked", [False, True]) +def test_denied_parent_reports_incomplete_coverage( + project: Path, monkeypatch: pytest.MonkeyPatch, tracked: bool +) -> None: + settings = _settings(project, prompt=True) + if tracked: + LockFile(local_deployed_files=[_SETTINGS]).write(project / "apm.lock.yaml") + lstat = Path.lstat + + def denied(path: Path, *args: object, **kwargs: object) -> os.stat_result: + if path == settings: + raise PermissionError("fixture parent traversal denied") + return lstat(path, *args, **kwargs) + + monkeypatch.setattr(Path, "lstat", denied) + result = CliRunner().invoke(cli, ["audit", "--no-drift", "--format", "json"]) + assert result.exit_code == 1, result.output + report = json.loads(result.output) + assert report["passed"] is False + assert report["summary"]["files_scanned"] == 0 + assert report["findings"] == [] + assert report["coverage"]["complete"] is False + assert any( + e["file"] == _SETTINGS and e["tracked"] == tracked for e in report["coverage"]["primitives"] + ) + + +@pytest.mark.skipif( + os.name == "nt" or getattr(os, "geteuid", lambda: 0)() == 0, + reason="requires enforced POSIX directory permissions", +) +def test_real_denied_parent_is_not_clean(project: Path) -> None: + settings = _settings(project, prompt=True) + settings.parent.chmod(0) + try: + result = CliRunner().invoke(cli, ["audit", "--no-drift", "--format", "json"]) + assert result.exit_code == 1, result.output + report = json.loads(result.output) + assert report["coverage"]["complete"] is False + assert report["summary"]["files_scanned"] == 0 + finally: + settings.parent.chmod(0o700) + + +@pytest.mark.parametrize("inside", [False, True]) +def test_cowork_installer_root_is_discovered_but_never_stripped( + project: Path, monkeypatch: pytest.MonkeyPatch, inside: bool +) -> None: + from apm_cli.integration.skill_integrator import SkillIntegrator + + root = (project if inside else project.parent) / "Documents/Cowork/skills" + profile = replace(KNOWN_TARGETS["copilot-cowork"], resolved_deploy_root=root) + skill = SkillIntegrator._target_skill_dir(profile, project, "manual") / "SKILL.md" + assert skill == root / "manual/SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text(_BIDI, encoding="utf-8") + result = scan_project_result(project, targets=(profile,)) + assert len(result.scanned_files) == 1 + assert result.protected_files == result.scanned_files + monkeypatch.setattr("apm_cli.integration.targets.resolve_targets", lambda *a, **k: [profile]) + before = skill.read_bytes() + stripped = CliRunner().invoke(cli, ["audit", "--strip"]) + assert stripped.exit_code == 1, stripped.output + assert skill.read_bytes() == before + assert "manual/SKILL.md" in " ".join(stripped.output.split()) + + +@pytest.mark.parametrize("output_format", ["text", "json", "sarif", "markdown"]) +def test_structured_locations_are_safe_and_not_physical_offsets( + project: Path, output_format: str +) -> None: + event = f"Stop{_BIDI}[red]|`" + _write( + project, + _SETTINGS, + json.dumps( + {"hooks": {event: [{"hooks": [{"type": "prompt", "prompt": f"safe {_BIDI}"}]}]}} + ), + ) + result = CliRunner().invoke(cli, ["audit", "--no-drift", "--format", output_format]) + assert result.exit_code == 1, result.output + pointer = f"/hooks/{event}/0/hooks/0/prompt" + if output_format == "json": + finding = json.loads(result.output)["findings"][0] + assert finding["pointer"] == pointer + assert finding["coordinate_space"] == "decoded-prompt" + assert (finding["decoded_line"], finding["decoded_column"]) == (1, 6) + assert "line" not in finding and "column" not in finding + elif output_format == "sarif": + finding = json.loads(result.output)["runs"][0]["results"][0] + assert finding["properties"]["pointer"] == pointer + assert "region" not in finding["locations"][0]["physicalLocation"] + else: + assert _BIDI not in result.output + assert "decoded" in result.output + if output_format == "markdown": + assert r"\[red\]\|\`" in result.output + + +def test_warning_protected_prompt_names_manual_remediation(project: Path) -> None: + relative = ".gemini/commands/manual.toml" + _write(project, relative, 'prompt = "warn\\u200b"') + (project / "apm.yml").write_text("name: audit-contract\nversion: 1.0.0\ntarget: gemini\n") + result = CliRunner().invoke(cli, ["audit", "--no-drift"]) + assert result.exit_code == 2, result.output + assert "manually" in result.output + stripped = CliRunner().invoke(cli, ["audit", "--strip"]) + assert stripped.exit_code == 1, stripped.output + assert relative + "/prompt" in " ".join(stripped.output.split()) + + +@pytest.mark.parametrize("count", [10, 100]) +def test_overlapping_claims_read_each_interpretation_once( + project: Path, monkeypatch: pytest.MonkeyPatch, count: int +) -> None: + paths = [_write(project, f".claude/skills/item{i}/SKILL.md", _BIDI) for i in range(count)] + relative = [path.relative_to(project).as_posix() for path in paths] + reads: list[Path] = [] + read_text = Path.read_text + + def counted(path: Path, *args: object, **kwargs: object) -> str: + if path in paths: + reads.append(path) + return read_text(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", counted) + result = scan_project_result( + project, + targets=(KNOWN_TARGETS["claude"],), + lockfile=LockFile(local_deployed_files=[".claude/skills/", *relative]), + ) + assert len(reads) == count + assert len(result.scanned_files) == count + assert all(entry.tracked for entry in result.inventory) + + +@pytest.mark.parametrize("count", [10, 100]) +def test_claim_directory_stat_is_not_per_surface( + project: Path, monkeypatch: pytest.MonkeyPatch, count: int +) -> None: + from apm_cli.security.file_scanner import _scan_claimed_files + + paths = {_write(project, f".claude/hooks/scripts/run{i}.py", "pass") for i in range(count)} + is_dir = Path.is_dir + probes: list[Path] = [] + + def counted(path: Path) -> bool: + if path in paths: + probes.append(path) + return is_dir(path) + + monkeypatch.setattr(Path, "is_dir", counted) + result = _scan_claimed_files( + project, {p.relative_to(project).as_posix(): "." for p in paths}, None + ) + assert len(probes) == count + assert len(result.inventory) == count + assert all(e.status == "not-applicable" for e in result.inventory) + + def test_audit_and_failures_preserve_complete_workspace_snapshot(project: Path) -> None: _settings(project, prompt=True) _write(project, ".claude/history.jsonl", _BIDI) diff --git a/tests/integration/test_audit_unrecorded_unicode_lifecycle.py b/tests/integration/test_audit_unrecorded_unicode_lifecycle.py index 2724595b6f..581b0e925a 100644 --- a/tests/integration/test_audit_unrecorded_unicode_lifecycle.py +++ b/tests/integration/test_audit_unrecorded_unicode_lifecycle.py @@ -256,6 +256,26 @@ def test_native_hook_discovery_across_reinstall_update_and_user_scope( assert user_instructions.read_bytes() == _BIDI_BYTES assert not (lifecycle.project.root / "NEVER_EXECUTE").exists() + dirty_skill = lifecycle.project.root / ".claude/skills/manual/SKILL.md" + dirty_skill.parent.mkdir(parents=True, exist_ok=True) + dirty_skill.write_bytes(_BIDI_BYTES) + settings["hooks"]["Stop"][0]["hooks"].append({"type": "prompt", "prompt": "\u202e"}) + settings_path.write_text(json.dumps(settings), encoding="utf-8") + mixed_before = LifecycleStateSnapshot.capture( + lifecycle.project.root, targets=("claude",), config_paths=configs + ) + refused = _run( + lifecycle, ("audit", "--strip"), expected=1, scenario_id="native-hooks-mixed-strip-refusal" + ) + assert "does not rewrite native configuration" in " ".join(refused.stdout.split()) + assert dirty_skill.read_bytes() == _BIDI_BYTES + _assert_same_state( + mixed_before, + LifecycleStateSnapshot.capture( + lifecycle.project.root, targets=("claude",), config_paths=configs + ), + ) + def test_unrecorded_unicode_detect_strip_and_idempotent_audit( tmp_path: Path, diff --git a/tests/unit/commands/test_audit_error_handling.py b/tests/unit/commands/test_audit_error_handling.py index 8b70b36c91..807cc4fd9e 100644 --- a/tests/unit/commands/test_audit_error_handling.py +++ b/tests/unit/commands/test_audit_error_handling.py @@ -31,7 +31,7 @@ _scan_single_file, ) from apm_cli.security.content_scanner import ScanFinding -from apm_cli.security.file_scanner import _FileScanResult +from apm_cli.security.file_scanner import FileScanResult # --------------------------------------------------------------------------- # Helpers @@ -449,9 +449,7 @@ def test_strip_with_no_findings_exits_zero(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = tmp_path / "apm.lock.yaml" with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult( - {}, frozenset({"a.md", "b.md", "c.md"}) - ) + mock_scan.return_value = FileScanResult({}, frozenset({"a.md", "b.md", "c.md"})) _audit_content_scan(cfg, None, None, strip=True, dry_run=False) assert exc_info.value.code == 0 @@ -491,7 +489,7 @@ def test_text_format_with_output_path_exits(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) + mock_scan.return_value = FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): @@ -516,7 +514,7 @@ def test_package_not_found_warning_and_exit(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult({}, frozenset()) + mock_scan.return_value = FileScanResult({}, frozenset()) _audit_content_scan(cfg, "owner/repo", None, strip=False, dry_run=False) assert exc_info.value.code == 0 logger.warning.assert_called() @@ -538,7 +536,7 @@ def test_dry_run_without_strip_warns(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) + mock_scan.return_value = FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): diff --git a/tests/unit/commands/test_audit_phase3.py b/tests/unit/commands/test_audit_phase3.py index f90d8d7c95..0faa48a450 100644 --- a/tests/unit/commands/test_audit_phase3.py +++ b/tests/unit/commands/test_audit_phase3.py @@ -31,7 +31,7 @@ _scan_single_file, ) from apm_cli.security.content_scanner import ScanFinding -from apm_cli.security.file_scanner import _FileScanResult +from apm_cli.security.file_scanner import FileScanResult # --------------------------------------------------------------------------- # Helpers @@ -449,9 +449,7 @@ def test_strip_with_no_findings_exits_zero(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = tmp_path / "apm.lock.yaml" with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult( - {}, frozenset({"a.md", "b.md", "c.md"}) - ) + mock_scan.return_value = FileScanResult({}, frozenset({"a.md", "b.md", "c.md"})) _audit_content_scan(cfg, None, None, strip=True, dry_run=False) assert exc_info.value.code == 0 @@ -491,7 +489,7 @@ def test_text_format_with_output_path_exits(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) + mock_scan.return_value = FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): @@ -516,7 +514,7 @@ def test_package_not_found_warning_and_exit(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult({}, frozenset()) + mock_scan.return_value = FileScanResult({}, frozenset()) _audit_content_scan(cfg, "owner/repo", None, strip=False, dry_run=False) assert exc_info.value.code == 0 logger.warning.assert_called() @@ -538,7 +536,7 @@ def test_dry_run_without_strip_warns(self, tmp_path: Path) -> None: with patch("apm_cli.commands.audit.get_lockfile_path") as mock_lf: mock_lf.return_value = lock_file with patch("apm_cli.commands.audit.scan_project_result") as mock_scan: - mock_scan.return_value = _FileScanResult({}, frozenset({"checked.md"})) + mock_scan.return_value = FileScanResult({}, frozenset({"checked.md"})) with patch( "apm_cli.commands.audit._has_actionable_findings", return_value=False ): diff --git a/tests/unit/integration/test_skill_integrator_hermetic.py b/tests/unit/integration/test_skill_integrator_hermetic.py index 86629e35b7..0b9a89aca8 100644 --- a/tests/unit/integration/test_skill_integrator_hermetic.py +++ b/tests/unit/integration/test_skill_integrator_hermetic.py @@ -29,6 +29,7 @@ should_compile_instructions, validate_skill_name, ) +from apm_cli.integration.targets import TargetProfile # --------------------------------------------------------------------------- # Helpers @@ -73,6 +74,9 @@ def _make_target( mapping.deploy_root = deploy_root prim.__getitem__ = MagicMock(return_value=mapping) target.primitives = {"skills": mapping} + target.skills_deploy_path.side_effect = lambda root: TargetProfile.skills_deploy_path( + target, root + ) return target diff --git a/tests/unit/integration/test_skill_integrator_phase3w4.py b/tests/unit/integration/test_skill_integrator_phase3w4.py index 96754cdc77..cd03879de3 100644 --- a/tests/unit/integration/test_skill_integrator_phase3w4.py +++ b/tests/unit/integration/test_skill_integrator_phase3w4.py @@ -29,6 +29,7 @@ should_compile_instructions, validate_skill_name, ) +from apm_cli.integration.targets import TargetProfile # --------------------------------------------------------------------------- # Helpers @@ -73,6 +74,9 @@ def _make_target( mapping.deploy_root = deploy_root prim.__getitem__ = MagicMock(return_value=mapping) target.primitives = {"skills": mapping} + target.skills_deploy_path.side_effect = lambda root: TargetProfile.skills_deploy_path( + target, root + ) return target diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index 82b33ad3fc..bcde805b24 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -738,7 +738,7 @@ def exiting_import( transport-platform-url-path-security transport-platform-windows-stable-path """ -_EXPECTED_RULE_IDS = frozenset(_EXPECTED_RULE_ID_TEXT.split()) +_EXPECTED_RULE_IDS = frozenset(_EXPECTED_RULE_ID_TEXT.split()) | {"audit-primitive-discovery"} def test_registered_rule_inventory_matches_frozen_semantic_contract() -> None: diff --git a/tests/unit/test_audit_fail_on_drift.py b/tests/unit/test_audit_fail_on_drift.py index 772f5bab62..194b1e6963 100644 --- a/tests/unit/test_audit_fail_on_drift.py +++ b/tests/unit/test_audit_fail_on_drift.py @@ -15,7 +15,7 @@ import pytest -from apm_cli.security.file_scanner import _FileScanResult +from apm_cli.security.file_scanner import FileScanResult def _make_cfg(tmp_path): @@ -62,7 +62,7 @@ def _run(tmp_path, fail_on_drift, drift_ret=None): patch.object( audit_mod, "scan_project_result", - return_value=_FileScanResult({}, frozenset({"checked.md"})), + return_value=FileScanResult({}, frozenset({"checked.md"})), ), patch.object(audit_mod, "_resolve_fail_on_drift", return_value=fail_on_drift), patch("apm_cli.install.drift.render_drift_text", return_value=""), From 58dafc046ad40152defbc0c72fd5220629f956c2 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 11:35:12 +0200 Subject: [PATCH 06/12] fix(audit): close review and lifecycle gaps with bounded conformance contract Preserve canonical emitted command layouts, safe Markdown text, shared-skill attribution, and valid native ownership fixtures. Propose optional deployed-prompt audit requirements without claiming completed specification adoption. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + CONFORMANCE.json | 30 ++++- CONFORMANCE.md | 8 +- .../manifests/openapm-v0.1.requirements.yml | 8 ++ docs/src/content/docs/reference/cli/audit.md | 14 ++- docs/src/content/docs/specs/openapm-v0.1.md | 70 ++++++++++- src/apm_cli/integration/hook_integrator.py | 2 - src/apm_cli/security/audit_report.py | 2 +- src/apm_cli/security/file_scanner.py | 29 ++++- .../test_audit_primitive_contract.py | 117 ++++++++++++++++++ .../test_ownership_invariant_lifecycle.py | 23 +++- tests/spec_conformance/gen_statement.py | 14 +++ tests/spec_conformance/test_policy_reqs.py | 96 ++++++++++++++ tests/unit/core/test_target_catalog.py | 18 ++- 14 files changed, 409 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d747d1599a..ac7090858d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `apm audit` discovers tracked and untracked target-native prompts without treating executable commands as prompt text; unreadable or unsupported recognized formats now fail with incomplete coverage, and shared/user configuration cannot be auto-stripped. Review named incomplete or protected locations manually before re-auditing. Proposed `specs/openapm-v0.1.md` audit contract remains subject to specification adoption. -- by @lkshrk (#2962) - Codex MCP headers now use native runtime environment references instead of literal placeholders; unsupported references are skipped with a warning. Unchanged reinstalls preserve existing entries; switch the affected header between `${VAR}` and `${env:VAR}` and reinstall with the same scope and targets to refresh an older managed entry. -- by @edenfunf (#3042) - Claude MCP redeclarations drop stale transport fields and repair mixed entries when rewritten, while preserving partial updates and unmanaged configuration. -- by @edenfunf (#3041) - Cursor rules now use comma-joined `globs` and readable descriptions, while retaining safe escaping for control characters. (by @YGuyomar, #3011) diff --git a/CONFORMANCE.json b/CONFORMANCE.json index d0f4c7a7e8..50afb8cc68 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -5,6 +5,9 @@ "target_capability_declaration": "MCPClientAdapter.supports_user_scope (OpenAPM Target Registry v0.1 implementation profile)" }, "generator": "gen_statement.py v1", + "optional_features": { + "deployed-prompt-audit": "APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), subject to the pending Section 9.3 amendment process, not adopted-spec approval. Project and user scopes follow TargetProfile deployment roots and PrimitiveMapping formats, including shared native hook settings via native_hook_config. The target-by-target locations, supported formats, prompt applicability and exclusions are published in docs/src/content/docs/reference/cli/audit.md. ContentScanner checks hidden controls and prompt-injection markers; this is not a guarantee that prompt text is safe. App SQLite state and unrelated transcripts are excluded. Detection does not grant execution, ownership or remediation authority." + }, "requirements": [ { "conformance_class": "consumer", @@ -836,6 +839,29 @@ "tests/spec_conformance/test_policy_reqs.py::test_policy_identity_casing_matches_dependency_source_semantics" ] }, + { + "conformance_class": "governance", + "id": "req-pl-019", + "keyword": "MUST", + "section": "6.8.1", + "status": "active", + "test_count": 2, + "tests": [ + "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_applicability_and_incomplete_exit[False]", + "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_applicability_and_incomplete_exit[True]" + ] + }, + { + "conformance_class": "governance", + "id": "req-pl-020", + "keyword": "MUST", + "section": "6.8.1", + "status": "active", + "test_count": 1, + "tests": [ + "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_protected_remediation_is_atomic" + ] + }, { "conformance_class": "consumer", "id": "req-pr-001", @@ -1508,7 +1534,7 @@ "xfail": 0 }, "governance": { - "active": 18, + "active": 20, "skipped": 0, "unbound": 0, "xfail": 0 @@ -1526,5 +1552,5 @@ "xfail": 0 } }, - "total_requirements": 123 + "total_requirements": 125 } diff --git a/CONFORMANCE.md b/CONFORMANCE.md index a0780b2bd1..8512dc5059 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -18,6 +18,10 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac Repository-coordinate segments are case-insensitive for `github.com`, GitHub Enterprise Cloud hosts ending in `.ghe.com`, the literal GitHub Enterprise Server host selected by `GITHUB_HOST`, and registry-sourced dependencies (including registry prefixes). Local paths, marketplace identities, and every other host remain case-sensitive. Policy matching and repository identity use the same rule (req-rs-016 clause 3; req-pl-018). +## Optional deployed-prompt audit + +APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), subject to the pending Section 9.3 amendment process, not adopted-spec approval. Project and user scopes follow TargetProfile deployment roots and PrimitiveMapping formats, including shared native hook settings via native_hook_config. The target-by-target locations, supported formats, prompt applicability and exclusions are published in docs/src/content/docs/reference/cli/audit.md. ContentScanner checks hidden controls and prompt-injection markers; this is not a guarantee that prompt text is safe. App SQLite state and unrelated transcripts are excluded. Detection does not grant execution, ownership or remediation authority. + ## Consumer user-scope disclosure - Manifest: `~/.apm/apm.yml` @@ -31,7 +35,7 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | Producer | 12 | 0 | 0 | 0 | | Consumer | 91 | 1 | 0 | 0 | | Registry | 1 | 0 | 0 | 0 | -| Governance | 18 | 0 | 0 | 0 | +| Governance | 20 | 0 | 0 | 0 | ## Per-requirement coverage @@ -106,6 +110,8 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | [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 | - | | [req-pl-018](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-018) | MUST | 6.3.1 | governance | active | 1 | - | +| [req-pl-019](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-019) | MUST | 6.8.1 | governance | active | 2 | - | +| [req-pl-020](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-020) | MUST | 6.8.1 | governance | active | 1 | - | | [req-pr-001](docs/src/content/docs/specs/openapm-v0.1.md#req-pr-001) | MUST | 8.2 | consumer | active | 1 | - | | [req-pr-002](docs/src/content/docs/specs/openapm-v0.1.md#req-pr-002) | MUST | 8.3 | consumer | active | 2 | - | | [req-pr-003](docs/src/content/docs/specs/openapm-v0.1.md#req-pr-003) | MUST | 8.3 | consumer | active | 1 | - | diff --git a/docs/public/specs/manifests/openapm-v0.1.requirements.yml b/docs/public/specs/manifests/openapm-v0.1.requirements.yml index 24576982dc..fa1dc29b0e 100644 --- a/docs/public/specs/manifests/openapm-v0.1.requirements.yml +++ b/docs/public/specs/manifests/openapm-v0.1.requirements.yml @@ -300,6 +300,14 @@ requirements: keyword: MUST section: "6.3.1" conformance_class: governance + - id: req-pl-019 + keyword: MUST + section: "6.8.1" + conformance_class: governance + - id: req-pl-020 + keyword: MUST + section: "6.8.1" + conformance_class: governance notes: "allow, deny, and exact require package operands use the documented per-host repository case rule of req-rs-016 plus the registry-coordinate case rule; ASCII normalization does not cross virtual paths, refs, registry names, MCP names, unmanaged paths, or case-sensitive host/source identities; recursive-glob ambiguity remains case-sensitive and deny precedence remains unchanged" - id: req-rs-001 keyword: MUST diff --git a/docs/src/content/docs/reference/cli/audit.md b/docs/src/content/docs/reference/cli/audit.md index 4d1d64a886..7758e523e8 100644 --- a/docs/src/content/docs/reference/cli/audit.md +++ b/docs/src/content/docs/reference/cli/audit.md @@ -196,12 +196,12 @@ not every file in a skill bundle. |---|---|---| | Copilot | `.github/instructions/*.instructions.md`, `prompts/*.prompt.md`, `agents/*.agent.md`, `.github/copilot-instructions.md`; shared skills | `.github/hooks/*.json`; commands are non-prompt | | Claude | `.claude/rules/*.md`, `agents/*.md`, `commands/*.md`, `skills/**/SKILL.md` | `.claude/hooks/*.json`, `.claude/settings.json`; nested `prompt`/`agent` handlers' `prompt` field | -| Cursor | `.cursor/rules/*.mdc`, `agents/*.md`, `commands/*.md`; shared skills | `.cursor/hooks/*.json`, `.cursor/hooks.json`; flat command handlers | +| Cursor | `.cursor/rules/*.mdc`, `agents/*.md`, `commands/*.md`; shared skills | `.cursor/hooks/*.json`, `.cursor/hooks.json`; flat or APM-preserved nested command handlers | | Kiro | `.kiro/steering/*.md`, `agents/*.md`, `skills/**/SKILL.md` | `.kiro/hooks/*.json`; v1 `hooks[].action.prompt` for agent actions | | Gemini | `.gemini/commands/*.toml`: `prompt`; shared skills | `.gemini/hooks/*.json`, `.gemini/settings.json`; nested command handlers | | Codex | `.codex/agents/*.toml`: `developer_instructions`; shared skills | `.codex/hooks.json`; nested command handlers | | Antigravity | `.agents/rules/*.md`, skills | `.agents/hooks.json`; named containers, nested tool events and flat other events | -| Windsurf | `.windsurf/rules/*.md`, `workflows/*.md`; shared skills | `.windsurf/hooks.json`; flat command handlers | +| Windsurf | `.windsurf/rules/*.md`, `workflows/*.md`; shared skills | `.windsurf/hooks.json`; flat or APM-preserved nested command handlers | | OpenCode | `.opencode/agents/*.md`, `commands/*.md`; shared skills | None | | Grok Build / Cloud | `.grok/skills/**/SKILL.md`; Build also `rules/*.md`, `agents/*.md`, `commands/*.md` | None | | Agent Skills, OpenClaw, Hermes | `.agents/skills/**/SKILL.md` | None | @@ -251,8 +251,10 @@ or missing APM-owned hooks report `modified`. `unrecorded` findings fail Drift is whole-project only; `--file` and explicit `PACKAGE` runs skip it. Use `--no-drift` to opt out with reduced coverage. In bare `apm audit`, drift -findings are advisory and do not change the exit code (see -[Exit codes](#exit-codes)). +findings are advisory by default. With `security.audit.fail_on_drift: true`, +a failed drift check (including one that could not run) promotes an otherwise +clean audit to exit `1`. An advisory cache-miss skip remains non-failing +(see [Exit codes](#exit-codes)). Bare `apm audit` keeps replay cache-only, so a cache miss produces an informational skip. `apm audit --ci` instead self-hydrates one lock-pinned @@ -291,8 +293,8 @@ as metadata repair; see [`apm prune`](../prune/#canonical-deployment-ownership). | Code | Meaning | |---|---| -| `0` | Clean, info-only findings, drift-only (advisory) in bare audit, or successful `--strip`. | -| `1` | Critical findings, incomplete primitive coverage (including unreadable parents), refused `--strip` remediation, or an invalid canonical deployment-owner reference in `apm.lock.yaml`. | +| `0` | Clean, info-only findings, advisory drift without `security.audit.fail_on_drift`, an advisory cache-miss skip, or successful `--strip`. | +| `1` | Critical findings, incomplete primitive coverage (including unreadable parents), refused `--strip` remediation, invalid canonical deployment-owner references, or drift-check failures promoted by `security.audit.fail_on_drift`. | | `2` | Warning-only findings, or usage error (mutually exclusive flags). | | `3` | Configuration or infrastructure error (feature not enabled, scanner not found, malformed SARIF). | diff --git a/docs/src/content/docs/specs/openapm-v0.1.md b/docs/src/content/docs/specs/openapm-v0.1.md index 28ed2abd09..dfcd5251e9 100644 --- a/docs/src/content/docs/specs/openapm-v0.1.md +++ b/docs/src/content/docs/specs/openapm-v0.1.md @@ -136,7 +136,7 @@ between the companion corpus and the implementation. ### 1.3 Document conventions -- OpenAPM v0.1 carries **123 normative statements (118 MUST, 5 SHOULD)** indexed in +- OpenAPM v0.1 carries **125 normative statements (120 MUST, 5 SHOULD)** indexed in [Appendix C](#appendix-c-index-of-normative-statements). - All on-disk files defined by this specification are **YAML 1.2** parsed under the safe subset defined in @@ -1791,6 +1791,62 @@ malformed-response, or other non-404 failures. When the legacy coordinate supplies a policy, the implementation MUST emit one actionable migration warning naming `apm/apm-policy`. +#### 6.8.1 Optional deployed-prompt audit + +`deployed-prompt-audit` is an OPTIONAL governance capability, distinct +from policy unmanaged-file evaluation ([req-pl-015](#req-pl-015)), +source materialization scanning ([req-sc-015](#req-sc-015)), and +drift checking ([req-pl-014](#req-pl-014)). A claim of this capability +identifies the supported targets, project or user scopes, recognized +primitive locations and native formats, and prompt checks in its +conformance statement. Concrete paths, native keys, check algorithms +and output serialization are implementation-defined; the following +requirements govern the declared scope, not arbitrary filesystem content. + + +**[req-pl-019]** A conforming **governance** implementation claiming +`deployed-prompt-audit` MUST: + +(a) discover recognized deployed primitives independently of ownership +records, including untracked content and shared native configuration in +the declared scope, using the same target location and format definitions +as its deployment implementation; + +(b) distinguish discovery from prompt-check applicability: check +prompt-bearing text, including decoded native prompt fields, while +reporting recognized command-only hooks as not applicable to prompt +checks, without treating their commands or executable bodies as prompts; + +(c) report incomplete coverage, with an affected location and reason, +when a recognized surface cannot be read or interpreted, including an +unsupported native format or version; terminate the audit non-zero for +incomplete coverage in default and CI modes, even without policy; and + +(d) distinguish coverage from content findings in its reports, identify +native prompt findings by field location, distinguish decoded prompt +coordinates from physical file coordinates, and render untrusted +locations and diagnostics as literal text rather than active markup +or display-control characters in human-facing output. + +A valid non-prompt entry does not fail this capability by itself. +Unrelated files outside the declared primitive scope are not incomplete +coverage merely because they were not inspected. These rules do not +change hash verification, executable trust or drift policy. + + +**[req-pl-020]** A conforming **governance** implementation claiming +`deployed-prompt-audit` MUST treat discovery as read-only evidence, not +as ownership, execution approval or permission to remediate. It MUST +NOT execute hooks, follow symlink files or directories, or widen traversal +to unrelated transcripts or user configuration to perform that audit. +If it offers automatic prompt-content remediation, it MUST preserve +shared native configuration and content outside the project root rather +than rewriting or deleting them, report affected protected locations +for manual review, and fail a remediation request that would touch such +content before writing any selected content. Existing ownership and +integrity checks, including [req-pl-016](#req-pl-016), remain applicable; +this capability grants no additional mutation authority. + ### 6.9 Conformance requirements (governance) This section's normative statements are: @@ -1803,7 +1859,8 @@ This section's normative statements are: [req-pl-011](#req-pl-011), [req-pl-012](#req-pl-012), [req-pl-013](#req-pl-013), [req-pl-014](#req-pl-014), [req-pl-015](#req-pl-015), [req-pl-016](#req-pl-016), - [req-pl-017](#req-pl-017), [req-pl-018](#req-pl-018). + [req-pl-017](#req-pl-017), [req-pl-018](#req-pl-018), + [req-pl-019](#req-pl-019), [req-pl-020](#req-pl-020). --- @@ -3567,7 +3624,8 @@ v0.2 will formalise the surrounding HTTP wire envelope. [req-pl-011](#req-pl-011), [req-pl-012](#req-pl-012), [req-pl-013](#req-pl-013), [req-pl-014](#req-pl-014), [req-pl-015](#req-pl-015), [req-pl-016](#req-pl-016), -[req-pl-017](#req-pl-017), [req-pl-018](#req-pl-018). +[req-pl-017](#req-pl-017), [req-pl-018](#req-pl-018), +[req-pl-019](#req-pl-019), [req-pl-020](#req-pl-020). ### 11.4 Worked conformance examples (informative) @@ -3949,6 +4007,8 @@ renumbering of conformance classes. | [req-pl-016](#req-pl-016) | MUST | 6.8 | governance | | [req-pl-017](#req-pl-017) | MUST | 6.8 | governance | | [req-pl-018](#req-pl-018) | MUST | 6.3.1 | governance | +| [req-pl-019](#req-pl-019) | MUST | 6.8.1 | governance | +| [req-pl-020](#req-pl-020) | MUST | 6.8.1 | governance | | [req-rs-001](#req-rs-001) | MUST | 7.2 | consumer | | [req-rs-002](#req-rs-002) | MUST | 7.3 | consumer | | [req-rs-003](#req-rs-003) | MUST | 7.3 | consumer | @@ -4006,7 +4066,7 @@ renumbering of conformance classes. | [req-cf-001](#req-cf-001) | MUST | 12.5 | consumer | | [req-cf-002](#req-cf-002) | MUST | 12.3 | consumer | -**Total normative statements: 123** (118 MUST, 5 SHOULD). +**Total normative statements: 125** (120 MUST, 5 SHOULD). --- @@ -4058,6 +4118,8 @@ renumbering of conformance classes. | 0.1.40 | 2026-09-07 | Spec-citation fold for dependency-policy identity casing in PR #2706. Added [req-pl-018] (Section 6.3.1, governance MUST) and extended [req-rs-016] clause (3): dependency allow, deny, and exact require operands use the documented per-host repository case rule, while registry-sourced repository coordinates are case-insensitive regardless of host; case normalization is ASCII-only, is bounded identically on both operands, stops at recursive-glob ambiguity, and does not cross virtual-path, ref, registry-name, MCP-name, unmanaged-path, or case-sensitive host/source boundaries; deny precedence is unchanged. Defined the policy glob grammar, documented byte-exact Section 6.4 merge behavior, and added the threat mapping. Classified this as a non-breaking correction of previously unspecified evaluation behavior under Section 9.2: existing lowercase workarounds remain matching; on registry sources and hosts documented as case-insensitive, case-variant allow entries can newly match, deny entries can newly enforce, and exact require entries can newly be satisfied, so those policies should be re-audited. Sections 1.3, 6.3.1, 6.3.5, 6.4, 6.5, 6.9, 7.2, 9.2, 10.8, 10.11, 11.2, and 11.3.4, Appendix C, and conformance coverage updated. Statement count: 121 -> 122 (117 MUST, 5 SHOULD). | | 0.1.41 | 2026-09-09 | Alias containment and lock-replay contract for PR #2901. Added [req-mf-025] (Section 4.3.2, consumer MUST), the optional lock-entry `alias` field, and conformance coverage. Under Section 9.2 this is an additive optional field and a defensive definition of previously unspecified alias behavior, not behavior-neutral errata: unsafe or reserved aliases can newly fail; valid dotted aliases remain accepted; surrounding whitespace is canonicalized; recorded aliases determine replay placement; absent aliases retain the unaliased layout. Source identity and permitted local source paths are unchanged. Older readers preserving the unknown field do not thereby implement placement support. Selects distinct 0.1.41 schema publication identities without changing published v0.1 URLs or bytes; Section 9.3 remains pending (see Appendix A). Sections 1.3, 4.9, 5.2, 10.7, 10.11, 11.3.2, and Appendix C updated. Statement count: 122 -> 123 (118 MUST, 5 SHOULD). | +| 0.1.42 (proposed) | 2026-09-30 | Optional deployed-prompt audit capability for PR #2962. Added conditional governance [req-pl-019] and [req-pl-020] in Section 6.8.1, both enumerations, Appendix C, requirements manifest and behavioral conformance coverage. Under Section 9.2 this is a new opt-in conformance capability, not behavior-neutral errata or a reinterpretation of an existing obligation: implementations not claiming it acquire no new required feature. APM claims it; newly discovered prompt findings and incomplete native coverage can newly fail default and CI audits, command-only content remains non-failing, and protected remediation is refused before writes. No schema, lockfile version, existing mandatory feature, drift-policy or ownership rule changes. Section 9.3 reviewer approvals and public comment period remain pending; this proposal is not evidence of adoption. Statement count: 123 -> 125 (120 MUST, 5 SHOULD). | + Errata (none at publication). --- diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py index 702b2e93ca..c1606db175 100644 --- a/src/apm_cli/integration/hook_integrator.py +++ b/src/apm_cli/integration/hook_integrator.py @@ -319,7 +319,6 @@ def _validate_copilot_payload(payload: dict) -> list[str]: target_key="cursor", require_dir=True, top_level_defaults={"version": 1}, - nested_handlers=False, ), "codex": _MergeHookConfig( config_filename="hooks.json", @@ -344,7 +343,6 @@ def _validate_copilot_payload(payload: dict) -> list[str]: config_filename="hooks.json", target_key="windsurf", require_dir=True, - nested_handlers=False, ), } diff --git a/src/apm_cli/security/audit_report.py b/src/apm_cli/security/audit_report.py index e03b9731b7..4604f34c19 100644 --- a/src/apm_cli/security/audit_report.py +++ b/src/apm_cli/security/audit_report.py @@ -36,7 +36,7 @@ def finding_location(finding: ScanFinding) -> str: def _markdown_cell(value: str) -> str: """Render untrusted labels as printable, literal Markdown table content.""" - return re.sub(r"([\\`*_{}\[\]<>()|#!])", r"\\\1", printable_ascii_text(value)) + return re.sub(r"([\\`*_{}\[\]<>()|#!&])", r"\\\1", printable_ascii_text(value)) def _markdown_code(value: str) -> str: diff --git a/src/apm_cli/security/file_scanner.py b/src/apm_cli/security/file_scanner.py index d551b1559b..3ff01500f4 100644 --- a/src/apm_cli/security/file_scanner.py +++ b/src/apm_cli/security/file_scanner.py @@ -192,12 +192,33 @@ def _scan_primitive( cache: dict[PrimitiveSurface, FileScanResult] | None = None, ) -> FileScanResult: """Check one recognized surface and retain non-applicability/coverage evidence.""" - key = replace(surface, path=path, pattern=None) + key = replace( + surface, + path=path, + pattern=None, + target=surface.target if surface.kind == "hooks" else "", + ) try: if not safe_surface_path(surface, path): return _empty_scan() if cache is not None and key in cache: - return cache[key] + cached = cache[key] + return FileScanResult( + { + label: [ + replace(finding, file=label) + for findings in cached.findings_by_file.values() + for finding in findings + ] + } + if cached.findings_by_file + else {}, + frozenset({label}) if cached.scanned_files else frozenset(), + tuple( + replace(entry, file=label, target=surface.target, tracked=tracked) + for entry in cached.inventory + ), + ) entries = _content_entries(path, surface) except (OSError, UnicodeError, ValueError) as exc: entries = ( @@ -323,12 +344,12 @@ def _scan_claimed_files( # Catalog profiles classify historical claims; only selected profiles are # enumerated by automatic discovery. - profiles = list(targets) + [ + profiles = [ profile for profile in KNOWN_TARGETS.values() if profile.name not in {target.name for target in targets} and not profile.user_root_resolver - ] + ] + list(reversed(targets)) surfaces = primitive_surfaces(project_root, profiles, user_scope=user_scope) results: list[FileScanResult] = [] cache = {} if cache is None else cache diff --git a/tests/integration/test_audit_primitive_contract.py b/tests/integration/test_audit_primitive_contract.py index eeac9539a6..467f635813 100644 --- a/tests/integration/test_audit_primitive_contract.py +++ b/tests/integration/test_audit_primitive_contract.py @@ -616,6 +616,123 @@ def counted(path: Path) -> bool: assert all(e.status == "not-applicable" for e in result.inventory) +def test_markdown_entities_cannot_reintroduce_hidden_controls(project: Path) -> None: + from markdown_it import MarkdownIt + + event = "‏ ‮‮" + _write( + project, + _SETTINGS, + json.dumps({"hooks": {event: [{"hooks": [{"type": "prompt", "prompt": _BIDI}]}]}}), + ) + result = CliRunner().invoke(cli, ["audit", "--no-drift", "--format", "markdown"]) + assert result.exit_code == 1, result.output + parser = MarkdownIt().enable("table") + cells = [ + child.content + for token in parser.parse(result.output) + for child in token.children or [] + if child.type == "text" and "/hooks/" in child.content + ] + assert len(cells) == 2 + assert all(event in cell for cell in cells) + assert all(all(0x20 <= ord(char) <= 0x7E for char in cell) for cell in cells) + + +@pytest.mark.parametrize("targets", [("copilot",), ("copilot", "agent-skills")]) +def test_shared_skills_use_one_interpretation_and_selected_attribution( + project: Path, monkeypatch: pytest.MonkeyPatch, targets: tuple[str, ...] +) -> None: + skill = _write(project, ".agents/skills/manual/SKILL.md", _BIDI) + read_text = Path.read_text + reads: list[Path] = [] + + def counted(path: Path, *args: object, **kwargs: object) -> str: + if path == skill: + reads.append(path) + return read_text(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", counted) + relative = skill.relative_to(project).as_posix() + result = scan_project_result( + project, + lockfile=LockFile(local_deployed_files=[relative]), + targets=tuple(KNOWN_TARGETS[name] for name in targets), + ) + assert reads == [skill] + assert len(result.inventory) == 1 + assert result.inventory[0].target == "copilot" + assert result.inventory[0].tracked is True + + +def test_cached_content_rebinds_attribution_without_reparsing( + project: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from apm_cli.security.file_scanner import FileScanResult, _scan_primitive + from apm_cli.security.primitive_discovery import PrimitiveSurface + + skill = _write(project, ".agents/skills/manual/SKILL.md", _BIDI) + surfaces = [ + next(s for s in primitive_surfaces(project, (KNOWN_TARGETS[name],)) if s.kind == "skills") + for name in ("copilot", "agent-skills") + ] + cache: dict[PrimitiveSurface, FileScanResult] = {} + first = _scan_primitive(skill, surfaces[0], "first", tracked=False, cache=cache) + + def no_read(*args: object, **kwargs: object) -> str: + pytest.fail("equivalent cached content was read again") + + monkeypatch.setattr(Path, "read_text", no_read) + second = _scan_primitive(skill, surfaces[1], "second", tracked=True, cache=cache) + assert first.inventory[0].tracked is False + assert second.inventory[0].tracked is True + assert second.inventory[0].target == "agent-skills" + assert second.scanned_files == frozenset({"second"}) + assert second.findings_by_file["second"][0].file == "second" + + +@pytest.mark.parametrize("target", ["cursor", "windsurf"]) +@pytest.mark.parametrize("nested", [False, True]) +def test_preserved_nested_command_layout_is_visible_non_prompt( + project: Path, target: str, nested: bool +) -> None: + profile = KNOWN_TARGETS[target] + path = profile.deploy_path(project, "hooks.json") + path.parent.mkdir(parents=True, exist_ok=True) + marker = project / "HOOK_MUST_NOT_RUN" + command = {"type": "command", "command": f"touch {marker}{_BIDI}"} + entry = {"matcher": "*", "hooks": [command]} if nested else command + payload = json.dumps({"version": 1, "hooks": {"PreToolUse": [entry]}}) + path.write_text(payload, encoding="utf-8") + result = scan_project_result(project, targets=(profile,)) + assert result.incomplete == () + assert result.findings_by_file == {} + assert [entry.status for entry in result.inventory] == ["not-applicable"] + assert path.read_text() == payload + assert not list(project.glob("HOOK_MUST_NOT_RUN*")) + + +@pytest.mark.parametrize("target", ["cursor", "windsurf"]) +@pytest.mark.parametrize( + "payload", + [ + '{"hooks":{"Stop":[{"hooks":[{"hooks":[{"command":"ignored"}]}]}]}}', + '{"hooks":{"Stop":[{"hooks":[{"type":"unknown","command":"ignored"}]}]}}', + '{"hooks":{"Stop":[{"type":"unknown","command":"ignored"}]}}', + ], +) +def test_preserved_command_layout_still_rejects_extra_depth( + project: Path, target: str, payload: str +) -> None: + profile = KNOWN_TARGETS[target] + path = profile.deploy_path(project, "hooks.json") + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(payload) + result = scan_project_result(project, targets=(profile,)) + assert result.incomplete + assert result.findings_by_file == {} + + def test_audit_and_failures_preserve_complete_workspace_snapshot(project: Path) -> None: _settings(project, prompt=True) _write(project, ".claude/history.jsonl", _BIDI) diff --git a/tests/integration/test_ownership_invariant_lifecycle.py b/tests/integration/test_ownership_invariant_lifecycle.py index 47ecca3dfb..b467743fbc 100644 --- a/tests/integration/test_ownership_invariant_lifecycle.py +++ b/tests/integration/test_ownership_invariant_lifecycle.py @@ -250,7 +250,9 @@ def _all_project_sentinels(project_root: Path) -> tuple[_Sentinel, ...]: relative_paths.setdefault(relative, _sentinel_content("project-generated", relative)) for primitive, mapping in profile.primitives.items(): relative = _mapping_sentinel_path(profile, primitive, mapping) - relative_paths.setdefault(relative, _sentinel_content("project", relative)) + relative_paths.setdefault( + relative, _primitive_sentinel_content("project", relative, mapping) + ) return tuple( _Sentinel("workspace", project_root, relative, content) for relative, content in sorted(relative_paths.items(), key=lambda item: item[0].as_posix()) @@ -292,7 +294,7 @@ def _all_user_sentinels( _root_id(root_dir), root, relative, - _sentinel_content(f"user-{profile.name}", relative), + _primitive_sentinel_content(f"user-{profile.name}", relative, mapping), ), ) return tuple( @@ -344,6 +346,23 @@ def _sentinel_content(label: str, relative_path: PurePosixPath) -> bytes: return _SENTINEL_PREFIX + f"{label}:{relative_path.as_posix()}\n".encode("ascii") +def _primitive_sentinel_content( + label: str, relative: PurePosixPath, mapping: PrimitiveMapping +) -> bytes: + """Happy-path ownership sentinels must also be valid recognized native content.""" + marker = _sentinel_content(label, relative).decode("ascii") + if mapping.prompt_fields: + return "\n".join( + f"{field} = {json.dumps(marker)}" for field in mapping.prompt_fields + ).encode() + if mapping.extension == ".json": + document = {"user_sentinel": marker, "hooks": {}} + if mapping.format_id == "kiro_hooks": + document.update(version="v1", hooks=[]) + return json.dumps(document).encode() + return marker.encode() + + def _root_id(root_dir: str) -> str: return "user-" + root_dir.replace("/", "-").replace(".", "dot") diff --git a/tests/spec_conformance/gen_statement.py b/tests/spec_conformance/gen_statement.py index cdacb34b73..2bd616aa97 100644 --- a/tests/spec_conformance/gen_statement.py +++ b/tests/spec_conformance/gen_statement.py @@ -41,6 +41,17 @@ "MCPClientAdapter.supports_user_scope (OpenAPM Target Registry v0.1 implementation profile)" ), } +DEPLOYED_PROMPT_AUDIT = ( + "APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), " + "subject to the pending Section 9.3 amendment process, not adopted-spec approval. " + "Project and user scopes follow TargetProfile deployment roots and PrimitiveMapping " + "formats, including shared native hook settings via native_hook_config. " + "The target-by-target locations, supported formats, prompt applicability and exclusions " + "are published in docs/src/content/docs/reference/cli/audit.md. " + "ContentScanner checks hidden controls and prompt-injection markers; this is not a " + "guarantee that prompt text is safe. App SQLite state and unrelated transcripts are " + "excluded. Detection does not grant execution, ownership or remediation authority." +) def _ensure_coverage() -> dict[str, list[dict[str, str]]]: @@ -155,6 +166,7 @@ def build_json() -> dict: "total_requirements": len(entries), "summary_by_class": summary, "consumer_user_scope": USER_SCOPE_DISCLOSURE, + "optional_features": {"deployed-prompt-audit": DEPLOYED_PROMPT_AUDIT}, "requirements": entries, } @@ -203,6 +215,8 @@ def build_md(doc: dict) -> str: "Local paths, marketplace identities, and every other host remain " "case-sensitive. Policy matching and repository identity use the same " "rule (req-rs-016 clause 3; req-pl-018).\n\n" + "## Optional deployed-prompt audit\n\n" + f"{doc['optional_features']['deployed-prompt-audit']}\n\n" ) summary_section = ( "## Coverage summary\n\n" + _md_class_summary(doc["summary_by_class"]) + "\n\n" diff --git a/tests/spec_conformance/test_policy_reqs.py b/tests/spec_conformance/test_policy_reqs.py index edf6765214..1e8bfcc546 100644 --- a/tests/spec_conformance/test_policy_reqs.py +++ b/tests/spec_conformance/test_policy_reqs.py @@ -4,6 +4,7 @@ import pytest +from tests.integration import test_audit_primitive_contract as audit_contract from tests.spec_conformance._helpers import ( assert_spec_contains, fixture_path, @@ -13,6 +14,101 @@ waive, ) +audit_project = audit_contract.project + + +@pytest.mark.req("req-pl-019") +@pytest.mark.parametrize("ci_mode", [False, True]) +def test_deployed_prompt_applicability_and_incomplete_exit(audit_project, ci_mode): + import json + + from click.testing import CliRunner + + from apm_cli.cli import cli + from apm_cli.security.file_scanner import scan_project_result + + root = audit_project + native = root / ".claude/settings.json" + native.parent.mkdir() + marker = root / "HOOK_MUST_NOT_RUN" + command = f"touch {marker}" + native.write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + {"type": "command", "command": command + "\u202e"}, + {"type": "prompt", "prompt": "Check \u202e"}, + ] + } + ] + } + } + ) + ) + skill = root / ".claude/skills/manual/SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text("Review \u202e") + report = scan_project_result(root) + assert set(report.findings_by_file) == { + ".claude/settings.json", + ".claude/skills/manual/SKILL.md", + } + assert {entry.status for entry in report.inventory} == {"checked", "not-applicable"} + assert all(not entry.tracked for entry in report.inventory) + assert not marker.exists() + native.write_text('{"hooks":') + before = native.read_bytes() + args = ["audit", "--no-drift", "--format", "json"] + if ci_mode: + args += ["--ci", "--no-policy", "--no-fail-fast"] + result = CliRunner().invoke(cli, args) + assert result.exit_code == 1, result.output + assert "incomplete" in result.output + assert native.read_bytes() == before + assert not marker.exists() + + +@pytest.mark.req("req-pl-020") +def test_deployed_prompt_protected_remediation_is_atomic(audit_project): + import json + + from click.testing import CliRunner + + from apm_cli.cli import cli + from apm_cli.security.file_scanner import scan_project_result + + root = audit_project + settings = root / ".claude/settings.json" + settings.parent.mkdir() + settings.write_text( + json.dumps({"hooks": {"Stop": [{"hooks": [{"type": "prompt", "prompt": "\u202e"}]}]}}) + ) + skill = root / ".claude/skills/manual/SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text("\u202e") + outside = root.parent / "outside.md" + outside.write_text("\u202e") + symlink = root / ".claude/rules/linked.md" + symlink.parent.mkdir() + symlink.symlink_to(outside) + transcript = root / ".claude/transcripts/unrelated.md" + transcript.parent.mkdir() + transcript.write_text("\u202e") + scanned = scan_project_result(root) + assert scanned.protected_files == frozenset({".claude/settings.json"}) + assert ".claude/rules/linked.md" not in scanned.scanned_files + assert ".claude/transcripts/unrelated.md" not in scanned.scanned_files + paths = [settings, skill, outside, transcript, root / "apm.yml", root / "apm.lock.yaml"] + before = {p: p.read_bytes() for p in paths} + result = CliRunner().invoke(cli, ["audit", "--strip"]) + assert result.exit_code == 1, result.output + assert "does not rewrite native configuration" in " ".join(result.output.split()) + assert {p: p.read_bytes() for p in paths} == before + assert symlink.is_symlink() + @pytest.mark.req("req-pl-001") def test_policy_valid_extends_passes_schema(): diff --git a/tests/unit/core/test_target_catalog.py b/tests/unit/core/test_target_catalog.py index 5ed9846998..794fafc78f 100644 --- a/tests/unit/core/test_target_catalog.py +++ b/tests/unit/core/test_target_catalog.py @@ -117,7 +117,7 @@ def test_current_native_profiles_are_characterized() -> None: ) for name, profile in KNOWN_TARGETS.items() } - assert actual == { + expected = { "copilot": ( ".github", { @@ -273,6 +273,22 @@ def test_current_native_profiles_are_characterized() -> None: "copilot_app", ), } + prompt_fields = { + ("gemini", "commands"): ("prompt",), + ("codex", "agents"): ("developer_instructions",), + } + assert actual == { + name: ( + root, + { + primitive: (*mapping, prompt_fields.get((name, primitive), ())) + for primitive, mapping in mappings.items() + }, + family, + flag, + ) + for name, (root, mappings, family, flag) in expected.items() + } def test_catalog_is_immutable_and_projects_accepted_values() -> None: From 24603682ce388b8914eadc7d3e44af024bffe493 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 11:52:31 +0200 Subject: [PATCH 07/12] fix(audit): preserve user-scope protection and CI prompt provenance Fold actual guardian findings without weakening conditional requirements, reporting guarantees, or pending specification adoption. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CONFORMANCE.json | 24 +++- CONFORMANCE.md | 6 +- .../manifests/openapm-v0.1.requirements.yml | 2 +- docs/src/content/docs/reference/cli/audit.md | 5 + docs/src/content/docs/specs/openapm-v0.1.md | 28 +++-- .../.apm/skills/apm-usage/commands.md | 6 +- src/apm_cli/policy/ci_checks.py | 12 +- src/apm_cli/policy/models.py | 21 ++++ src/apm_cli/security/audit_report.py | 102 ++++++++--------- src/apm_cli/security/file_scanner.py | 2 + src/apm_cli/security/primitive_discovery.py | 8 +- tests/spec_conformance/gen_statement.py | 6 +- tests/spec_conformance/test_policy_reqs.py | 107 ++++++++++++++++++ 13 files changed, 250 insertions(+), 79 deletions(-) diff --git a/CONFORMANCE.json b/CONFORMANCE.json index 50afb8cc68..3f1d3726de 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -6,7 +6,7 @@ }, "generator": "gen_statement.py v1", "optional_features": { - "deployed-prompt-audit": "APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), subject to the pending Section 9.3 amendment process, not adopted-spec approval. Project and user scopes follow TargetProfile deployment roots and PrimitiveMapping formats, including shared native hook settings via native_hook_config. The target-by-target locations, supported formats, prompt applicability and exclusions are published in docs/src/content/docs/reference/cli/audit.md. ContentScanner checks hidden controls and prompt-injection markers; this is not a guarantee that prompt text is safe. App SQLite state and unrelated transcripts are excluded. Detection does not grant execution, ownership or remediation authority." + "deployed-prompt-audit": "APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), subject to the pending Section 9.3 amendment process, not adopted-spec approval. APM's own Consumer deployment implementation supplies the definitions: project and user scopes follow TargetProfile deployment roots and PrimitiveMapping formats, including shared native hook settings via native_hook_config. The target-by-target locations, supported formats, prompt applicability and exclusions are published in docs/src/content/docs/reference/cli/audit.md. ContentScanner checks hidden or suspicious Unicode code points that may conceal instructions, not semantic prompt-injection detection; a clean result is not a guarantee that prompt text is safe. App SQLite state and unrelated transcripts are excluded. Detection does not grant execution, ownership or remediation authority." }, "requirements": [ { @@ -845,10 +845,22 @@ "keyword": "MUST", "section": "6.8.1", "status": "active", - "test_count": 2, + "test_count": 14, "tests": [ "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_applicability_and_incomplete_exit[False]", - "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_applicability_and_incomplete_exit[True]" + "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_applicability_and_incomplete_exit[True]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[0-json-False]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[0-json-True]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[0-sarif-False]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[0-sarif-True]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[0-text-False]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[0-text-True]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[1-json-False]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[1-json-True]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[1-sarif-False]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[1-sarif-True]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[1-text-False]", + "tests/spec_conformance/test_policy_reqs.py::test_native_findings_identify_only_the_offending_field[1-text-True]" ] }, { @@ -857,9 +869,11 @@ "keyword": "MUST", "section": "6.8.1", "status": "active", - "test_count": 1, + "test_count": 3, "tests": [ - "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_protected_remediation_is_atomic" + "tests/spec_conformance/test_policy_reqs.py::test_deployed_prompt_protected_remediation_is_atomic", + "tests/spec_conformance/test_policy_reqs.py::test_static_user_scope_refuses_automatic_strip_in_real_cli[False]", + "tests/spec_conformance/test_policy_reqs.py::test_static_user_scope_refuses_automatic_strip_in_real_cli[True]" ] }, { diff --git a/CONFORMANCE.md b/CONFORMANCE.md index 8512dc5059..070c6c4ec8 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -20,7 +20,7 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent ## Optional deployed-prompt audit -APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), subject to the pending Section 9.3 amendment process, not adopted-spec approval. Project and user scopes follow TargetProfile deployment roots and PrimitiveMapping formats, including shared native hook settings via native_hook_config. The target-by-target locations, supported formats, prompt applicability and exclusions are published in docs/src/content/docs/reference/cli/audit.md. ContentScanner checks hidden controls and prompt-injection markers; this is not a guarantee that prompt text is safe. App SQLite state and unrelated transcripts are excluded. Detection does not grant execution, ownership or remediation authority. +APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), subject to the pending Section 9.3 amendment process, not adopted-spec approval. APM's own Consumer deployment implementation supplies the definitions: project and user scopes follow TargetProfile deployment roots and PrimitiveMapping formats, including shared native hook settings via native_hook_config. The target-by-target locations, supported formats, prompt applicability and exclusions are published in docs/src/content/docs/reference/cli/audit.md. ContentScanner checks hidden or suspicious Unicode code points that may conceal instructions, not semantic prompt-injection detection; a clean result is not a guarantee that prompt text is safe. App SQLite state and unrelated transcripts are excluded. Detection does not grant execution, ownership or remediation authority. ## Consumer user-scope disclosure @@ -110,8 +110,8 @@ APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020 | [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 | - | | [req-pl-018](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-018) | MUST | 6.3.1 | governance | active | 1 | - | -| [req-pl-019](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-019) | MUST | 6.8.1 | governance | active | 2 | - | -| [req-pl-020](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-020) | MUST | 6.8.1 | governance | active | 1 | - | +| [req-pl-019](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-019) | MUST | 6.8.1 | governance | active | 14 | - | +| [req-pl-020](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-020) | MUST | 6.8.1 | governance | active | 3 | - | | [req-pr-001](docs/src/content/docs/specs/openapm-v0.1.md#req-pr-001) | MUST | 8.2 | consumer | active | 1 | - | | [req-pr-002](docs/src/content/docs/specs/openapm-v0.1.md#req-pr-002) | MUST | 8.3 | consumer | active | 2 | - | | [req-pr-003](docs/src/content/docs/specs/openapm-v0.1.md#req-pr-003) | MUST | 8.3 | consumer | active | 1 | - | diff --git a/docs/public/specs/manifests/openapm-v0.1.requirements.yml b/docs/public/specs/manifests/openapm-v0.1.requirements.yml index fa1dc29b0e..e67159dcb0 100644 --- a/docs/public/specs/manifests/openapm-v0.1.requirements.yml +++ b/docs/public/specs/manifests/openapm-v0.1.requirements.yml @@ -300,6 +300,7 @@ requirements: keyword: MUST section: "6.3.1" conformance_class: governance + notes: "allow, deny, and exact require package operands use the documented per-host repository case rule of req-rs-016 plus the registry-coordinate case rule; ASCII normalization does not cross virtual paths, refs, registry names, MCP names, unmanaged paths, or case-sensitive host/source identities; recursive-glob ambiguity remains case-sensitive and deny precedence remains unchanged" - id: req-pl-019 keyword: MUST section: "6.8.1" @@ -308,7 +309,6 @@ requirements: keyword: MUST section: "6.8.1" conformance_class: governance - notes: "allow, deny, and exact require package operands use the documented per-host repository case rule of req-rs-016 plus the registry-coordinate case rule; ASCII normalization does not cross virtual paths, refs, registry names, MCP names, unmanaged paths, or case-sensitive host/source identities; recursive-glob ambiguity remains case-sensitive and deny precedence remains unchanged" - id: req-rs-001 keyword: MUST section: "7.2" diff --git a/docs/src/content/docs/reference/cli/audit.md b/docs/src/content/docs/reference/cli/audit.md index 7758e523e8..c8b185c1a1 100644 --- a/docs/src/content/docs/reference/cli/audit.md +++ b/docs/src/content/docs/reference/cli/audit.md @@ -78,6 +78,11 @@ text and Markdown label these offsets as decoded. SARIF omits a physical region. CI JSON stores inventory in `content-integrity.primitive_coverage`, separate from violation `details`. SARIF retains it in invocation `primitiveCoverage` properties, and text shows discovered hooks even when all checks pass. +CI `content_findings` identifies each critical finding's actual file and prompt +pointer separately from inventory; SARIF uses that file, not the lockfile. +Automatic `--strip` also refuses user-scoped prompt files when auditing from +`~/.apm`, even though the traversal root is the home directory. The explicit +single-file `--file` operation is unchanged. ### CI gate diff --git a/docs/src/content/docs/specs/openapm-v0.1.md b/docs/src/content/docs/specs/openapm-v0.1.md index dfcd5251e9..64a3f0321b 100644 --- a/docs/src/content/docs/specs/openapm-v0.1.md +++ b/docs/src/content/docs/specs/openapm-v0.1.md @@ -1796,21 +1796,27 @@ actionable migration warning naming `apm/apm-policy`. `deployed-prompt-audit` is an OPTIONAL governance capability, distinct from policy unmanaged-file evaluation ([req-pl-015](#req-pl-015)), source materialization scanning ([req-sc-015](#req-sc-015)), and -drift checking ([req-pl-014](#req-pl-014)). A claim of this capability -identifies the supported targets, project or user scopes, recognized -primitive locations and native formats, and prompt checks in its -conformance statement. Concrete paths, native keys, check algorithms +drift checking ([req-pl-014](#req-pl-014)). Concrete paths, native keys, check algorithms and output serialization are implementation-defined; the following requirements govern the declared scope, not arbitrary filesystem content. **[req-pl-019]** A conforming **governance** implementation claiming -`deployed-prompt-audit` MUST: +`deployed-prompt-audit` MUST identify its supported targets, project +or user scopes, recognized primitive locations and native formats, +prompt checks and intentional exclusions in its conformance statement. +The declared profile MUST include at least one prompt-bearing location +or field and one specified content check. A workspace with no matching +content may nevertheless have complete, empty coverage. +Within that declared profile, the implementation MUST: (a) discover recognized deployed primitives independently of ownership records, including untracked content and shared native configuration in the declared scope, using the same target location and format definitions -as its deployment implementation; +as the Consumer deployment implementation identified by its conformance +statement. A Governance implementation may consume those definitions +from another implementation; claiming this capability does not require +implementing deployment or claiming the Consumer class; (b) distinguish discovery from prompt-check applicability: check prompt-bearing text, including decoded native prompt fields, while @@ -1840,10 +1846,15 @@ as ownership, execution approval or permission to remediate. It MUST NOT execute hooks, follow symlink files or directories, or widen traversal to unrelated transcripts or user configuration to perform that audit. If it offers automatic prompt-content remediation, it MUST preserve -shared native configuration and content outside the project root rather +shared native configuration, user-scoped prompt content, and content +outside the project root rather than rewriting or deleting them, report affected protected locations for manual review, and fail a remediation request that would touch such -content before writing any selected content. Existing ownership and +content before writing any selected content. User-scoped content remains +protected even when the user's home directory is the deployment traversal +root. This automatic-discovery capability does not redefine a separately +documented operation on an explicitly selected individual file. +Existing ownership and integrity checks, including [req-pl-016](#req-pl-016), remain applicable; this capability grants no additional mutation authority. @@ -4117,7 +4128,6 @@ renumbering of conformance classes. | 0.1.39 | 2026-09-01 | Spec-citation fold for user-scoped direct MCP target selection (closes #2548 Mode-B silent-extension gate). Added [req-tg-014] (Section 8.5.8, consumer MUST): explicit selection, the user-scope manifest, configured user default, and user-scope runtime discovery form one precedence chain; project-only signals cannot constrain final discovery; and a selected set with no user-capable runtime fails before user manifest, lockfile, or target-config mutation. Section 8.7, Section 11.3.2, and Appendix C updated. Statement count: 120 -> 121 (116 MUST, 5 SHOULD). | | 0.1.40 | 2026-09-07 | Spec-citation fold for dependency-policy identity casing in PR #2706. Added [req-pl-018] (Section 6.3.1, governance MUST) and extended [req-rs-016] clause (3): dependency allow, deny, and exact require operands use the documented per-host repository case rule, while registry-sourced repository coordinates are case-insensitive regardless of host; case normalization is ASCII-only, is bounded identically on both operands, stops at recursive-glob ambiguity, and does not cross virtual-path, ref, registry-name, MCP-name, unmanaged-path, or case-sensitive host/source boundaries; deny precedence is unchanged. Defined the policy glob grammar, documented byte-exact Section 6.4 merge behavior, and added the threat mapping. Classified this as a non-breaking correction of previously unspecified evaluation behavior under Section 9.2: existing lowercase workarounds remain matching; on registry sources and hosts documented as case-insensitive, case-variant allow entries can newly match, deny entries can newly enforce, and exact require entries can newly be satisfied, so those policies should be re-audited. Sections 1.3, 6.3.1, 6.3.5, 6.4, 6.5, 6.9, 7.2, 9.2, 10.8, 10.11, 11.2, and 11.3.4, Appendix C, and conformance coverage updated. Statement count: 121 -> 122 (117 MUST, 5 SHOULD). | | 0.1.41 | 2026-09-09 | Alias containment and lock-replay contract for PR #2901. Added [req-mf-025] (Section 4.3.2, consumer MUST), the optional lock-entry `alias` field, and conformance coverage. Under Section 9.2 this is an additive optional field and a defensive definition of previously unspecified alias behavior, not behavior-neutral errata: unsafe or reserved aliases can newly fail; valid dotted aliases remain accepted; surrounding whitespace is canonicalized; recorded aliases determine replay placement; absent aliases retain the unaliased layout. Source identity and permitted local source paths are unchanged. Older readers preserving the unknown field do not thereby implement placement support. Selects distinct 0.1.41 schema publication identities without changing published v0.1 URLs or bytes; Section 9.3 remains pending (see Appendix A). Sections 1.3, 4.9, 5.2, 10.7, 10.11, 11.3.2, and Appendix C updated. Statement count: 122 -> 123 (118 MUST, 5 SHOULD). | - | 0.1.42 (proposed) | 2026-09-30 | Optional deployed-prompt audit capability for PR #2962. Added conditional governance [req-pl-019] and [req-pl-020] in Section 6.8.1, both enumerations, Appendix C, requirements manifest and behavioral conformance coverage. Under Section 9.2 this is a new opt-in conformance capability, not behavior-neutral errata or a reinterpretation of an existing obligation: implementations not claiming it acquire no new required feature. APM claims it; newly discovered prompt findings and incomplete native coverage can newly fail default and CI audits, command-only content remains non-failing, and protected remediation is refused before writes. No schema, lockfile version, existing mandatory feature, drift-policy or ownership rule changes. Section 9.3 reviewer approvals and public comment period remain pending; this proposal is not evidence of adoption. Statement count: 123 -> 125 (120 MUST, 5 SHOULD). | Errata (none at publication). diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index a3a2a754c4..ed2a286738 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -268,10 +268,12 @@ Non-CI JSON exposes `coverage.complete` and `coverage.primitives`; CI includes inventory in `content-integrity.primitive_coverage`, separate from violation `details`; SARIF uses invocation `primitiveCoverage` properties, including on passing checks. Structured findings identify pointers and decoded offsets, -not physical file line/column positions. Copilot App's SQLite workflow prompts +not physical file line/column positions. CI `content_findings` retains the exact +offending file and pointer; SARIF locates that artifact, not the lockfile. +Copilot App's SQLite workflow prompts are not covered by filesystem discovery. Tracking is file-level, not entry ownership or hash verification. Automatic `--strip` refuses -structured/shared or external-root findings and incomplete coverage without +structured/shared, user-scope or external-root findings and incomplete coverage without rewriting files. Explicit `--file` remains user-directed. | Command | Purpose | Key flags | diff --git a/src/apm_cli/policy/ci_checks.py b/src/apm_cli/policy/ci_checks.py index cdc653501d..43ee0fee40 100644 --- a/src/apm_cli/policy/ci_checks.py +++ b/src/apm_cli/policy/ci_checks.py @@ -531,6 +531,12 @@ def _check_content_integrity( for rel_path, findings in findings_by_file.items(): if any(f.severity == "critical" for f in findings): critical_files.append(rel_path) + critical_findings = tuple( + finding + for findings in findings_by_file.values() + for finding in findings + if finding.severity == "critical" + ) from ..core.deployment_ledger import DeploymentLedgerCodec from ..core.deployment_state import LocatorKind @@ -622,8 +628,9 @@ def _has_symlink_component(path: Path) -> bool: details: list[str] = [] for entry in scan.incomplete: details.append(f"incomplete-coverage: {entry.file}{entry.pointer}: {entry.diagnostic}") - for rel_path in critical_files: - details.append(f"unicode: {rel_path}") + details.extend( + dict.fromkeys(f"unicode: {finding.file}{finding.pointer}" for finding in critical_findings) + ) for rel_path in missing_ownership: details.append(f"missing-ownership: {rel_path}") for rel_path in unresolved_hash_paths: @@ -668,6 +675,7 @@ def _has_symlink_component(path: Path) -> bool: message=f"{summary} -- run {remedy}", details=details, coverage=scan.inventory, + content_findings=critical_findings, ) diff --git a/src/apm_cli/policy/models.py b/src/apm_cli/policy/models.py index 972ed7577b..8a761aab60 100644 --- a/src/apm_cli/policy/models.py +++ b/src/apm_cli/policy/models.py @@ -10,6 +10,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: + from apm_cli.security.content_scanner import ScanFinding from apm_cli.security.file_scanner import CoverageEntry # Check name -> most relevant artifact for SARIF locations. @@ -55,6 +56,7 @@ class CheckResult: message: str # human-readable description details: list[str] = field(default_factory=list) # individual violations coverage: tuple[CoverageEntry, ...] = () + content_findings: tuple[ScanFinding, ...] = () @dataclass @@ -73,6 +75,8 @@ def failed_checks(self) -> list[CheckResult]: def to_json(self) -> dict: """Serialize to JSON-compatible dict.""" + from apm_cli.security.audit_report import finding_to_json + return { "passed": self.passed, "checks": [ @@ -81,6 +85,11 @@ def to_json(self) -> dict: "passed": c.passed, "message": c.message, "details": c.details, + **( + {"content_findings": [finding_to_json(f) for f in c.content_findings]} + if c.content_findings + else {} + ), **( {"primitive_coverage": [asdict(entry) for entry in c.coverage]} if c.coverage @@ -98,6 +107,8 @@ def to_json(self) -> dict: def to_sarif(self) -> dict: """Serialize to SARIF v2.1.0 format for GitHub Code Scanning.""" + from apm_cli.security.audit_report import finding_to_sarif + try: from importlib.metadata import version as pkg_version @@ -109,7 +120,17 @@ def to_sarif(self) -> dict: for check in self.checks: if not check.passed: artifact = _CHECK_ARTIFACT_MAP.get(check.name, "apm.lock.yaml") + content_details = { + f"unicode: {finding.file}{finding.pointer}" + for finding in check.content_findings + } + results.extend( + {**finding_to_sarif(finding), "ruleId": check.name} + for finding in check.content_findings + ) for detail in check.details or [check.message]: + if detail in content_details: + continue results.append( { "ruleId": check.name, diff --git a/src/apm_cli/security/audit_report.py b/src/apm_cli/security/audit_report.py index 4604f34c19..88250ce838 100644 --- a/src/apm_cli/security/audit_report.py +++ b/src/apm_cli/security/audit_report.py @@ -121,27 +121,7 @@ def findings_to_json( "info": sum(1 for f in all_findings if f.severity == "info"), } - items = [] - for finding in all_findings: - items.append( - { - "severity": finding.severity, - "file": relative_path_for_report(finding.file), - **( - { - "coordinate_space": "decoded-prompt", - "decoded_line": finding.line, - "decoded_column": finding.column, - } - if finding.pointer - else {"line": finding.line, "column": finding.column} - ), - "codepoint": finding.codepoint, - "category": finding.category, - "description": finding.description, - **({"pointer": finding.pointer} if finding.pointer else {}), - } - ) + items = [finding_to_json(finding) for finding in all_findings] items.extend(_owner_json(violation) for violation in owner_violations) report = { @@ -159,6 +139,53 @@ def findings_to_json( return report +def finding_to_json(finding: ScanFinding) -> dict[str, Any]: + """Serialize one finding without conflating decoded and physical coordinates.""" + return { + "severity": finding.severity, + "file": relative_path_for_report(finding.file), + **( + { + "coordinate_space": "decoded-prompt", + "decoded_line": finding.line, + "decoded_column": finding.column, + } + if finding.pointer + else {"line": finding.line, "column": finding.column} + ), + "codepoint": finding.codepoint, + "category": finding.category, + "description": finding.description, + **({"pointer": finding.pointer} if finding.pointer else {}), + } + + +def finding_to_sarif(finding: ScanFinding) -> dict[str, Any]: + """Use the actual prompt artifact and never invent physical native offsets.""" + return { + "ruleId": _rule_id(finding.category), + "level": _SEVERITY_MAP.get(finding.severity, "note"), + "message": {"text": f"{finding.description} ({finding.codepoint})"}, + "locations": [ + { + "physicalLocation": { + "artifactLocation": {"uri": relative_path_for_report(finding.file)}, + **( + {"region": {"startLine": finding.line, "startColumn": finding.column}} + if not finding.pointer + else {} + ), + } + } + ], + "properties": { + "codepoint": finding.codepoint, + "category": finding.category, + **({"pointer": finding.pointer} if finding.pointer else {}), + }, + } + + def findings_to_sarif( findings_by_file: dict[str, list[ScanFinding]], files_scanned: int, @@ -198,38 +225,7 @@ def findings_to_sarif( } # Build results - results = [] - for finding in all_findings: - result: dict[str, Any] = { - "ruleId": _rule_id(finding.category), - "level": _SEVERITY_MAP.get(finding.severity, "note"), - "message": {"text": f"{finding.description} ({finding.codepoint})"}, - "locations": [ - { - "physicalLocation": { - "artifactLocation": { - "uri": relative_path_for_report(finding.file), - }, - **( - { - "region": { - "startLine": finding.line, - "startColumn": finding.column, - } - } - if not finding.pointer - else {} - ), - } - } - ], - "properties": { - "codepoint": finding.codepoint, - "category": finding.category, - **({"pointer": finding.pointer} if finding.pointer else {}), - }, - } - results.append(result) + results = [finding_to_sarif(finding) for finding in all_findings] for violation in owner_violations: locator = violation.locator results.append( diff --git a/src/apm_cli/security/file_scanner.py b/src/apm_cli/security/file_scanner.py index 3ff01500f4..ddc0a2c003 100644 --- a/src/apm_cli/security/file_scanner.py +++ b/src/apm_cli/security/file_scanner.py @@ -248,6 +248,7 @@ def _scan_primitive( and not entry.pointer and status == "checked" and not surface.external + and not surface.user_scope ), ) ) @@ -422,6 +423,7 @@ def _scan_claimed_files( "recorded", "document" if abs_path.suffix in {".md", ".mdc"} or is_directory else "executable", "recorded", + user_scope=user_scope, ) if is_directory: directory_surfaces = [ diff --git a/src/apm_cli/security/primitive_discovery.py b/src/apm_cli/security/primitive_discovery.py index 24c3834c1c..5f676d97a2 100644 --- a/src/apm_cli/security/primitive_discovery.py +++ b/src/apm_cli/security/primitive_discovery.py @@ -5,7 +5,7 @@ import fnmatch import os from collections.abc import Iterator, Sequence -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from stat import S_ISLNK @@ -32,6 +32,7 @@ class PrimitiveSurface: prompt_fields: tuple[str, ...] = () shared: bool = False external: bool = False + user_scope: bool = False def contains(self, path: Path) -> bool: """Match a path without walking or resolving user-controlled links.""" @@ -135,7 +136,10 @@ def primitive_surfaces( ) ) # Exact shared configs override an identical primitive filename (hooks.json). - return tuple({(s.path, s.pattern): s for s in surfaces}.values()) + return tuple( + replace(surface, user_scope=user_scope) + for surface in {(s.path, s.pattern): s for s in surfaces}.values() + ) def safe_surface_path(surface: PrimitiveSurface, path: Path) -> bool: diff --git a/tests/spec_conformance/gen_statement.py b/tests/spec_conformance/gen_statement.py index 2bd616aa97..f697a98df7 100644 --- a/tests/spec_conformance/gen_statement.py +++ b/tests/spec_conformance/gen_statement.py @@ -44,11 +44,13 @@ DEPLOYED_PROMPT_AUDIT = ( "APM claims the proposed deployed-prompt-audit capability (req-pl-019, req-pl-020), " "subject to the pending Section 9.3 amendment process, not adopted-spec approval. " - "Project and user scopes follow TargetProfile deployment roots and PrimitiveMapping " + "APM's own Consumer deployment implementation supplies the definitions: " + "project and user scopes follow TargetProfile deployment roots and PrimitiveMapping " "formats, including shared native hook settings via native_hook_config. " "The target-by-target locations, supported formats, prompt applicability and exclusions " "are published in docs/src/content/docs/reference/cli/audit.md. " - "ContentScanner checks hidden controls and prompt-injection markers; this is not a " + "ContentScanner checks hidden or suspicious Unicode code points that may conceal " + "instructions, not semantic prompt-injection detection; a clean result is not a " "guarantee that prompt text is safe. App SQLite state and unrelated transcripts are " "excluded. Detection does not grant execution, ownership or remediation authority." ) diff --git a/tests/spec_conformance/test_policy_reqs.py b/tests/spec_conformance/test_policy_reqs.py index 1e8bfcc546..8ce4402afe 100644 --- a/tests/spec_conformance/test_policy_reqs.py +++ b/tests/spec_conformance/test_policy_reqs.py @@ -17,6 +17,113 @@ audit_project = audit_contract.project +@pytest.mark.req("req-pl-020") +@pytest.mark.parametrize("tracked", [False, True]) +def test_static_user_scope_refuses_automatic_strip_in_real_cli(audit_project, tracked): + import subprocess + import sys + from pathlib import Path + + from apm_cli.deps.lockfile import LockFile + from apm_cli.security.file_scanner import scan_project_result + + home = Path.home() + workspace = home / ".apm" + workspace.mkdir(exist_ok=True) + (workspace / "apm.yml").write_text("name: user-audit\nversion: 1.0.0\ntarget: copilot\n") + relative = ".copilot/copilot-instructions.md" + prompt = home / relative + prompt.parent.mkdir(exist_ok=True) + prompt.write_text("user-owned \u202e instructions") + LockFile(local_deployed_files=[relative] if tracked else []).write(workspace / "apm.lock.yaml") + scan = scan_project_result(workspace) + assert scan.inventory and all(not entry.strippable for entry in scan.inventory) + assert scan.protected_files == frozenset({relative}) + assert scan.inventory[0].tracked is tracked + paths = [prompt, workspace / "apm.yml", workspace / "apm.lock.yaml"] + before = {path: path.read_bytes() for path in paths} + result = subprocess.run( + [sys.executable, "-m", "apm_cli.cli", "audit", "--strip"], + cwd=workspace, + capture_output=True, + text=True, + timeout=60, + ) + assert result.returncode == 1, result.stdout + result.stderr + assert relative in result.stdout + assert {path: path.read_bytes() for path in paths} == before + + +@pytest.mark.req("req-pl-019") +@pytest.mark.parametrize("ci_mode", [False, True]) +@pytest.mark.parametrize("output_format", ["json", "sarif", "text"]) +@pytest.mark.parametrize("dirty_index", [0, 1]) +def test_native_findings_identify_only_the_offending_field( + audit_project, ci_mode, output_format, dirty_index +): + import json + from urllib.parse import unquote, urlparse + + from click.testing import CliRunner + + from apm_cli.cli import cli + + native = audit_project / ".claude/settings.json" + native.parent.mkdir() + native.write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + { + "type": "prompt", + "prompt": "\u202e" if i == dirty_index else "Clean", + } + for i in range(2) + ] + } + ] + } + } + ) + ) + expected = f"/hooks/Stop/0/hooks/{dirty_index}/prompt" + args = ["audit", "--no-drift", "--format", output_format] + if ci_mode: + args += ["--ci", "--no-policy", "--no-fail-fast"] + result = CliRunner().invoke(cli, args) + assert result.exit_code == 1, result.output + if output_format == "text": + assert expected in result.output + if ci_mode: + assert f"unicode: .claude/settings.json{expected}" in " ".join(result.output.split()) + return + report = json.loads(result.output) + if output_format == "json": + if ci_mode: + check = next(c for c in report["checks"] if c["name"] == "content-integrity") + findings = check["content_findings"] + assert len(check["primitive_coverage"]) == 2 + else: + findings = report["findings"] + assert len(findings) == 1 + assert findings[0]["pointer"] == expected + assert findings[0]["file"] == ".claude/settings.json" + assert findings[0]["coordinate_space"] == "decoded-prompt" + assert "line" not in findings[0] and "column" not in findings[0] + else: + findings = [f for f in report["runs"][0]["results"] if "pointer" in f.get("properties", {})] + assert len(findings) == 1 + assert findings[0]["properties"]["pointer"] == expected + location = findings[0]["locations"][0]["physicalLocation"] + assert ( + unquote(urlparse(location["artifactLocation"]["uri"]).path) == ".claude/settings.json" + ) + assert "region" not in location + + @pytest.mark.req("req-pl-019") @pytest.mark.parametrize("ci_mode", [False, True]) def test_deployed_prompt_applicability_and_incomplete_exit(audit_project, ci_mode): From 66ba9ea6ec4bb992a545042cd917f4c29d3aee26 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 11:55:35 +0200 Subject: [PATCH 08/12] fix(audit): retain plain file labels for unstructured findings Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/policy/ci_checks.py | 4 +++- src/apm_cli/policy/models.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/apm_cli/policy/ci_checks.py b/src/apm_cli/policy/ci_checks.py index 43ee0fee40..26b62f3168 100644 --- a/src/apm_cli/policy/ci_checks.py +++ b/src/apm_cli/policy/ci_checks.py @@ -629,7 +629,9 @@ def _has_symlink_component(path: Path) -> bool: for entry in scan.incomplete: details.append(f"incomplete-coverage: {entry.file}{entry.pointer}: {entry.diagnostic}") details.extend( - dict.fromkeys(f"unicode: {finding.file}{finding.pointer}" for finding in critical_findings) + dict.fromkeys( + f"unicode: {finding.file}{finding.pointer or ''}" for finding in critical_findings + ) ) for rel_path in missing_ownership: details.append(f"missing-ownership: {rel_path}") diff --git a/src/apm_cli/policy/models.py b/src/apm_cli/policy/models.py index 8a761aab60..5915c9082a 100644 --- a/src/apm_cli/policy/models.py +++ b/src/apm_cli/policy/models.py @@ -121,7 +121,7 @@ def to_sarif(self) -> dict: if not check.passed: artifact = _CHECK_ARTIFACT_MAP.get(check.name, "apm.lock.yaml") content_details = { - f"unicode: {finding.file}{finding.pointer}" + f"unicode: {finding.file}{finding.pointer or ''}" for finding in check.content_findings } results.extend( From d317fd2ffdbb622135f1f4e000c477a39fbfc0e2 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 12:14:11 +0200 Subject: [PATCH 09/12] test(architecture): enforce shared audit finding serializers Register the authority and reject all four ordinary/CI JSON/SARIF consumer bypasses without changing runtime or specification behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../owners/hooks-integrations.json | 10 ++++++++++ .../checks/audit_discovery.py | 10 +++++++++- .../test_architecture_audit_discovery.py | 19 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/.apm/architecture/owners/hooks-integrations.json b/.apm/architecture/owners/hooks-integrations.json index 516bf531aa..f0c4f897de 100644 --- a/.apm/architecture/owners/hooks-integrations.json +++ b/.apm/architecture/owners/hooks-integrations.json @@ -1,6 +1,16 @@ { "version": 1, "owners": [ + { + "id": "audit-finding-serialization", + "decision": "Ordinary and CI finding artifact, pointer and coordinate serialization", + "owner": "security/audit_report.py (finding_to_json, finding_to_sarif)", + "selectors": [ + "src/apm_cli/security/audit_report.py", + "src/apm_cli/policy/models.py" + ], + "guards": ["audit-finding-serialization"] + }, { "id": "audit-primitive-discovery", "decision": "Registry-derived deployed primitive discovery and applicable prompt coverage", diff --git a/scripts/architecture_linter/checks/audit_discovery.py b/scripts/architecture_linter/checks/audit_discovery.py index 40917f6893..7899776bc5 100644 --- a/scripts/architecture_linter/checks/audit_discovery.py +++ b/scripts/architecture_linter/checks/audit_discovery.py @@ -32,6 +32,14 @@ }, "src/apm_cli/commands/audit.py": {"_audit_content_scan": {"scan_project_result"}}, "src/apm_cli/policy/ci_checks.py": {"_check_content_integrity": {"scan_project_result"}}, + "src/apm_cli/security/audit_report.py": { + "findings_to_json": {"finding_to_json"}, + "findings_to_sarif": {"finding_to_sarif"}, + }, + "src/apm_cli/policy/models.py": { + "to_json": {"finding_to_json"}, + "to_sarif": {"finding_to_sarif"}, + }, } @@ -78,7 +86,7 @@ def check_audit_discovery(provider: FactsProvider) -> Iterable[Violation]: Rule( id=RULE_ID, group="mutation_writes", - guard_ids=(RULE_ID,), + guard_ids=(RULE_ID, "audit-finding-serialization"), description="Deployed primitive discovery and applicable checks share registry-derived owners.", check=check_audit_discovery, ), diff --git a/tests/integration/test_architecture_audit_discovery.py b/tests/integration/test_architecture_audit_discovery.py index 392d523ddf..d7f802136a 100644 --- a/tests/integration/test_architecture_audit_discovery.py +++ b/tests/integration/test_architecture_audit_discovery.py @@ -53,3 +53,22 @@ def test_skills_destination_has_one_authority(path: str) -> None: result = run_selected_rules(_ROOT, {_RULE}, source_overrides={path: mutated}) assert result.failures == () assert any(v.rule_id == _RULE and v.path == path for v in result.violations) + + +@pytest.mark.parametrize( + "path", + ["src/apm_cli/security/audit_report.py", "src/apm_cli/policy/models.py"], +) +@pytest.mark.parametrize("serializer", ["finding_to_json", "finding_to_sarif"]) +def test_finding_serialization_cannot_bypass_shared_authority(path: str, serializer: str) -> None: + source = (_ROOT / path).read_text(encoding="utf-8") + # Preserve the owner's definition and imports while replacing consumer calls. + mutated = source.replace(f"{serializer}(finding)", "bypass_serializer(finding)").replace( + f"{serializer}(f)", "bypass_serializer(f)" + ) + assert mutated != source + result = run_selected_rules(_ROOT, {_RULE}, source_overrides={path: mutated}) + assert result.failures == () + assert any( + v.rule_id == _RULE and v.path == path and serializer in v.message for v in result.violations + ) From 04cea96ac41aad3515e1b459ef35a6e190694680 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 12:53:10 +0200 Subject: [PATCH 10/12] test: cover audit owners in exhaustive mutation matrix Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../test_architecture_owner_rule_mutations.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index cc38df9a8e..16b6b9c039 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -75,6 +75,22 @@ class MutationCase: MUTATIONS: tuple[MutationCase, ...] = ( + MutationCase( + guard_id="audit-finding-serialization", + rule_id="audit-primitive-discovery", + path="src/apm_cli/security/audit_report.py", + old="items = [finding_to_json(finding) for finding in all_findings]", + new="items = [bypass_serializer(finding) for finding in all_findings]", + intent="Ordinary JSON reporting bypasses the canonical single-finding serializer.", + ), + MutationCase( + guard_id="audit-primitive-discovery", + rule_id="audit-primitive-discovery", + path="src/apm_cli/security/file_scanner.py", + old="for surface in primitive_surfaces(project_root, scoped, user_scope=user_scope):", + new="for surface in ():", + intent="Automatic audit discovery stops enumerating canonical primitive surfaces.", + ), MutationCase( guard_id="contracts-tests-taxonomy-classification", rule_id="contracts-tests-taxonomy-classification", From 0aad2c16c07146a165451c868f699a0244d19d0b Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 15:21:01 +0200 Subject: [PATCH 11/12] test: cover marketplace remote owner in mutation matrix Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../integration/test_architecture_owner_rule_mutations.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index 40fec9b889..b9dfb81ee8 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -1037,6 +1037,14 @@ class MutationCase: new="def parse_host_qualified_reference_disabled(", intent="Host-qualified reference parsing loses its canonical coordinate owner.", ), + MutationCase( + guard_id="transport-platform-marketplace-package-remote", + rule_id="transport-platform-marketplace-package-remote", + path="src/apm_cli/marketplace/resolver.py", + old=" dep_ref = lookup", + new=" dep_ref = None", + intent="Marketplace version resolution drops the canonical package-remote handoff.", + ), MutationCase( guard_id="transport-platform-network-host-parsing", rule_id="transport-platform-network-host-parsing", From 0cac8097a9bf3e4a4a1cc8a01cc2f303787642ad Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Wed, 30 Sep 2026 20:34:21 +0200 Subject: [PATCH 12/12] test(lifecycle): reconcile native Claude compile expectations Preserve exact native revisions and no redundant Claude root, with complete snapshot assertions and isolated Hermes compilation writes after main composition. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../docs/contributing/integration-testing.md | 6 ++++ .../test_generated_lifecycle_state_machine.py | 10 +++---- .../test_ownership_invariant_lifecycle.py | 27 ++++++++++++++--- .../test_required_lifecycle_state_machine.py | 30 +++++++++++++++---- 4 files changed, 59 insertions(+), 14 deletions(-) diff --git a/docs/src/content/docs/contributing/integration-testing.md b/docs/src/content/docs/contributing/integration-testing.md index 4f125426a0..65869c659f 100644 --- a/docs/src/content/docs/contributing/integration-testing.md +++ b/docs/src/content/docs/contributing/integration-testing.md @@ -72,6 +72,12 @@ also enforce one lockfile read per used root and integration phase, zero unused reads, and at most 15x record visits when fixture size grows from N to 10N. These counts do not claim a wall-clock speedup. +For fully native Claude fixtures, global compile verifies the installed revision +in `rules/revision.md`, absence of redundant `CLAUDE.md`, and unchanged complete +snapshots. Separate lifecycle cases retain unmatched fallback, policy refusal, +and protected explicit-cleanup coverage. Mixed Claude/Hermes fixtures additionally +check the Hermes revision and permit only its `AGENTS.md` compilation write. + Use a clean committed checkout. Set `BASE_SHA` to the full 40-hex commit SHA of a locally available, distinct ancestor of the checked-out `HEAD`. Set `REPORT_PATH` and `COMPLETION_PATH` to distinct, new paths outside the checkout: diff --git a/tests/integration/test_generated_lifecycle_state_machine.py b/tests/integration/test_generated_lifecycle_state_machine.py index dd75da0b2f..183cb4405e 100644 --- a/tests/integration/test_generated_lifecycle_state_machine.py +++ b/tests/integration/test_generated_lifecycle_state_machine.py @@ -637,11 +637,11 @@ def update(self) -> None: def compile(self) -> None: before = self._capture() self._run(("compile", "--global"), "compile") - text = (self.fixture.external_roots[0].path / "CLAUDE.md").read_text(encoding="ascii") - assert f"# revision-{self.installed_revision}" in text - assert_snapshot_changes_within( - before, self._capture(), exact_paths={"target": {"CLAUDE.md"}}, tree_prefixes={} - ) + target_root = self.fixture.external_roots[0].path + native = target_root / "rules" / "revision.md" + assert native.read_text(encoding="ascii") == f"# revision-{self.installed_revision}\n" + assert not (target_root / "CLAUDE.md").exists() + assert_snapshot_set_unchanged(before, self._capture()) @rule() @precondition( diff --git a/tests/integration/test_ownership_invariant_lifecycle.py b/tests/integration/test_ownership_invariant_lifecycle.py index d3b15326ac..4d162cf334 100644 --- a/tests/integration/test_ownership_invariant_lifecycle.py +++ b/tests/integration/test_ownership_invariant_lifecycle.py @@ -28,7 +28,11 @@ _run_success, _skill, ) -from tests.utils.artifact_snapshot import ArtifactSnapshotSet, assert_snapshot_set_unchanged +from tests.utils.artifact_snapshot import ( + ArtifactSnapshotSet, + assert_snapshot_changes_within, + assert_snapshot_set_unchanged, +) from tests.utils.lifecycle_state import LifecycleStateRoot, LifecycleStateSnapshot from tests.utils.local_package import LocalPackage @@ -907,6 +911,12 @@ def test_global_update_preserves_owned_external_skill_targets( assert str(hermes_skill.parent) in deployed assert str(external_roots["claude"] / "rules" / "revision.md") in deployed + artifact_roots = { + "project": consumer.root, + "user": scenario.isolated.home, + **external_roots, + } + before_compile = ArtifactSnapshotSet.capture(artifact_roots) _run_success( scenario, consumer, @@ -914,7 +924,16 @@ def test_global_update_preserves_owned_external_skill_targets( environment=environment, scenario_id="global-update-external-compile", ) - compiled_claude = (external_roots["claude"] / "CLAUDE.md").read_text(encoding="utf-8") - assert "revision-b" in compiled_claude - assert "revision-a" not in compiled_claude + native_claude = external_roots["claude"] / "rules" / "revision.md" + assert native_claude.read_text(encoding="utf-8") == "# revision-b\n" + assert not (external_roots["claude"] / "CLAUDE.md").exists() + compiled_hermes = (external_roots["hermes"] / "AGENTS.md").read_text(encoding="utf-8") + assert "# revision-b" in compiled_hermes + assert "# revision-a" not in compiled_hermes + assert_snapshot_changes_within( + before_compile, + ArtifactSnapshotSet.capture(artifact_roots), + exact_paths={"hermes": {"AGENTS.md"}}, + tree_prefixes={}, + ) assert commit_a.sha != commit_b.sha diff --git a/tests/integration/test_required_lifecycle_state_machine.py b/tests/integration/test_required_lifecycle_state_machine.py index 223789b86a..bb18ff1db2 100644 --- a/tests/integration/test_required_lifecycle_state_machine.py +++ b/tests/integration/test_required_lifecycle_state_machine.py @@ -2632,9 +2632,19 @@ def _exercise_global_revision_commands( _assert_same_state(installed, capture()) assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) + native_path = compiled_path.parent / "rules" / "revision.md" + hermes_path = artifact_roots["hermes"] / "AGENTS.md" run(("compile", "--global"), "global-compile-a") - assert "# revision-a" in compiled_path.read_text(encoding="ascii") - compiled_a = compiled_path.read_bytes() + assert native_path.read_text(encoding="ascii") == "# revision-a\n" + assert not compiled_path.exists() + assert "# revision-a" in hermes_path.read_text(encoding="ascii") + assert_snapshot_changes_within( + before, + ArtifactSnapshotSet.capture(artifact_roots), + exact_paths={"hermes": {"AGENTS.md"}}, + tree_prefixes={}, + ) + native_a = native_path.read_bytes() before_lock = capture() run(("lock", "--global", "--no-policy", "--parallel-downloads", "0"), "global-lock-a") assert capture().deployment_records == before_lock.deployment_records @@ -2666,10 +2676,20 @@ def _exercise_global_revision_commands( run(("update", "--global", "--yes", "--parallel-downloads", "0"), "global-update-b") assert_revision(commit_b, "b") assert capture().deployment_records != installed_a.deployment_records + before_compile = ArtifactSnapshotSet.capture(artifact_roots) run(("compile", "--global"), "global-compile-b") - assert "# revision-b" in compiled_path.read_text(encoding="ascii") - assert "# revision-a" not in compiled_path.read_text(encoding="ascii") - assert compiled_path.read_bytes() != compiled_a + assert native_path.read_text(encoding="ascii") == "# revision-b\n" + assert native_path.read_bytes() != native_a + assert not compiled_path.exists() + hermes_b = hermes_path.read_text(encoding="ascii") + assert "# revision-b" in hermes_b + assert "# revision-a" not in hermes_b + assert_snapshot_changes_within( + before_compile, + ArtifactSnapshotSet.capture(artifact_roots), + exact_paths={"hermes": {"AGENTS.md"}}, + tree_prefixes={}, + ) installed_b = capture() export_b = run(("lock", "export", "--global", "--format", "spdx"), "global-lock-export-b") assert json.loads(export_b.stdout)["spdxVersion"].startswith("SPDX-")