diff --git a/CHANGELOG.md b/CHANGELOG.md index cb7d88963..f9fea1855 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 ### Added +- `apm audit` now surfaces unmanaged files in governance directories as a single enriched report: each finding states a factual reason (`not tracked in apm.lock.yaml`), a lazy primitive-type tag (`[type: skill|agent|instruction|mcp]`), and a deny-conflict note (`matches deny rule ()`) when the path matches the policy's own `dependencies.deny` / `mcp.deny`. A new `unmanaged_files.exclude` policy key suppresses known harness-managed paths, and a symlink guard prevents following links out of the workspace. This is drift / divergence visibility, not supply-chain-attack prevention. (closes #1775) (#1793) - Azure DevOps is now documented as a first-class marketplace authoring host: a `marketplace.sourceBase` of `https://dev.azure.com/{org}/{project}/_git` composes relative package sources and preserves the `dev.azure.com` host through to the consumer (authenticated with `ADO_APM_PAT`). The end-to-end authoring -> consume path is pinned by a hermetic test. (closes #1010) (#1810) - `apm install --target antigravity` and `apm compile -t antigravity` add Google Antigravity CLI (`agy`, successor to Gemini CLI) as a new target. diff --git a/CONFORMANCE.json b/CONFORMANCE.json index 4bcf8d7af..531ea00a1 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -642,6 +642,17 @@ "tests/spec_conformance/test_policy_reqs.py::test_policy_fail_on_drift_parses_and_is_specified" ] }, + { + "conformance_class": "governance", + "id": "req-pl-015", + "keyword": "MUST", + "section": "6.3.5", + "status": "active", + "test_count": 1, + "tests": [ + "tests/spec_conformance/test_policy_reqs.py::test_unmanaged_files_surfacing_completeness" + ] + }, { "conformance_class": "consumer", "id": "req-pr-001", @@ -1019,7 +1030,7 @@ "xfail": 0 }, "governance": { - "active": 14, + "active": 15, "skipped": 0, "unbound": 0, "xfail": 0 @@ -1037,5 +1048,5 @@ "xfail": 0 } }, - "total_requirements": 89 + "total_requirements": 90 } diff --git a/CONFORMANCE.md b/CONFORMANCE.md index df4c7ba46..d8d900a9e 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -21,7 +21,7 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac | Producer | 12 | 0 | 0 | 0 | | Consumer | 61 | 1 | 0 | 0 | | Registry | 1 | 0 | 0 | 0 | -| Governance | 14 | 0 | 0 | 0 | +| Governance | 15 | 0 | 0 | 0 | ## Per-requirement coverage @@ -84,6 +84,7 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac | [req-pl-012](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-012) | MUST | 6.1.1 | governance | active | 1 | | [req-pl-013](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-013) | MUST | 6.8 | governance | active | 1 | | [req-pl-014](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-014) | MUST | 6.8 | governance | active | 1 | +| [req-pl-015](docs/src/content/docs/specs/openapm-v0.1.md#req-pl-015) | MUST | 6.3.5 | 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 | 1 | | [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 349171038..87d4c9685 100644 --- a/docs/public/specs/manifests/openapm-v0.1.requirements.yml +++ b/docs/public/specs/manifests/openapm-v0.1.requirements.yml @@ -243,6 +243,10 @@ requirements: keyword: MUST section: "6.8" conformance_class: governance + - id: req-pl-015 + keyword: MUST + section: "6.3.5" + conformance_class: governance - id: req-rs-001 keyword: MUST section: "7.2" diff --git a/docs/src/content/docs/enterprise/policy-reference.md b/docs/src/content/docs/enterprise/policy-reference.md index 9105a5cd1..57ddc6952 100644 --- a/docs/src/content/docs/enterprise/policy-reference.md +++ b/docs/src/content/docs/enterprise/policy-reference.md @@ -330,6 +330,40 @@ unmanaged_files: - .cursor/rules - .claude - .opencode + - .kiro +``` + +### `exclude` + +Glob allow-list of workspace paths to suppress from the report. Use it to +silence known harness-managed artifacts that legitimately live in a governance +directory but are not APM-tracked. Excluded paths are never reported. Merges as +a union down an `extends:` chain. + +```yaml +unmanaged_files: + action: warn + exclude: + - .github/copilot-instructions.md + - .claude/settings.local.json +``` + +### Enriched findings + +Each reported file is annotated in place within the single unmanaged-files +report. This is drift / divergence visibility -- `apm.lock.yaml` is +hand-editable YAML -- not supply-chain-attack prevention: + +- a factual reason: `not tracked in apm.lock.yaml`; +- a lazy primitive-type tag (`[type: skill|agent|instruction|mcp]`), computed + only for already-flagged files (a directory merely named `mcp` is not treated + as MCP config -- only a `.mcp/` root or an `mcp.json` file is); +- a deny-conflict note `matches deny rule ()` when the path matches + this policy's own `dependencies.deny` or `mcp.deny`, surfaced for a human to + resolve. APM reports only -- it never removes or blocks the file. + +```text +[!] .claude/skills/rogue/SKILL.md [type: skill] -- not tracked in apm.lock.yaml; matches deny rule (**/rogue/**) ``` --- @@ -445,6 +479,7 @@ A child policy can only tighten constraints — never relax them: | `mcp.self_defined` | Escalates: `allow` < `warn` < `deny` | | `manifest.scripts` | Escalates: `allow` < `deny` | | `unmanaged_files.action` | Escalates: `ignore` < `warn` < `deny` | +| `unmanaged_files.exclude` | Union, additive-only -- child adds suppression globs to the parent set; `null` and `[]` both preserve the parent list (a child cannot clear it) | | `source_attribution` | `parent OR child` — either enables it | | `trust_transitive` | `parent AND child` — both must allow it | diff --git a/docs/src/content/docs/reference/policy-schema.md b/docs/src/content/docs/reference/policy-schema.md index 4f24a514c..03d88fd98 100644 --- a/docs/src/content/docs/reference/policy-schema.md +++ b/docs/src/content/docs/reference/policy-schema.md @@ -175,6 +175,22 @@ Files in primitive target directories that are not recorded in `apm.lock.yaml`. |---------------|----------------|----------|----------------------------------------------------------------------------------| | `action` | enum | `ignore` | `ignore` / `warn` / `deny`. `deny` blocks installs that would leave drift. | | `directories` | list of paths | `[]` | Subset of target directories to check. Empty = all known target directories. | +| `exclude` | list of globs | `null` | Workspace path globs to suppress from the report. Use to silence known harness-managed artifacts. Excluded paths are never reported. `null` = no opinion (transparent in the `extends:` merge); merges as a union down the chain. | + +Each reported file is a divergence-visibility finding, not a security verdict +-- `apm.lock.yaml` is hand-editable YAML, so this surfaces drift rather than +proving a supply-chain attack. Every finding is enriched in place: + +- a factual reason -- `not tracked in apm.lock.yaml`; +- a lazy primitive-type tag (`[type: skill|agent|instruction|mcp]`) classified + only for the already-flagged file, never the whole tree; +- a deny-conflict note -- `matches deny rule ()` -- when the path + matches this policy's own `dependencies.deny` or `mcp.deny`. This is surfaced + for a human to resolve; APM never removes or blocks the file on this basis. + +```text +[!] .github/agents/rogue.agent.md [type: agent] -- not tracked in apm.lock.yaml; matches deny rule (**/rogue*) +``` ## security @@ -269,6 +285,7 @@ inherited list (see the tri-state table below). | `mcp.trust_transitive` | Logical AND (`true` only if both sides true). | | `manifest.scripts` | Stricter wins (`deny` > `allow`). | | `unmanaged_files.action` | Stricter wins (`deny` > `warn` > `ignore`). | +| `unmanaged_files.exclude` | Union, deduplicated; additive-only. `null` and `[]` both preserve the parent list -- unlike `deny`/`require`, a child cannot clear an inherited `exclude`. | | `security.audit.on_install` | Stricter wins (`block` > `warn` > `off`). `null` is transparent. | | `security.audit.external` | Union, deduplicated. `null` is transparent. | | `security.audit.scanners` | Union of scanner names; per scanner `allow_args` is AND-merged (any ancestor `false` wins -- tightening). `null` is transparent. | @@ -359,6 +376,8 @@ unmanaged_files: directories: - .github/instructions - .github/prompts + exclude: + - .github/copilot-instructions.md ``` ## registry_source diff --git a/docs/src/content/docs/specs/openapm-v0.1.md b/docs/src/content/docs/specs/openapm-v0.1.md index 5278a4362..b465efece 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 **89 normative statements** indexed in +- OpenAPM v0.1 carries **90 normative statements** 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 @@ -1218,7 +1218,42 @@ requirement). #### 6.3.5 `unmanaged_files` The `unmanaged_files` block governs files in primitive target -directories that are not recorded in `apm.lock.yaml`. +directories that are not recorded in `apm.lock.yaml`. `directories` +names the managed primitive target trees to scan, `action` selects +the response (`ignore` | `warn` | `deny`), and `exclude` is a glob +allow-list of workspace paths to suppress from the report. Its glob +patterns are matched with the same pattern semantics as the policy +allow-list and deny-list fields (see +[Section 6.5](#65-allow-list--deny-list-tri-state-semantics)). + + +**[req-pl-015]** A conforming **governance** implementation MUST, +when it evaluates policy over a populated primitive target tree (for +example during an audit), report unmanaged artifacts with the +following completeness guarantees: + +(a) It MUST surface every file under a managed primitive target +directory that is neither recorded in `apm.lock.yaml` nor matched by +a configured `unmanaged_files.exclude` glob. + +(b) Each surfaced path MUST be reported with the reason it is +unmanaged (that it is not tracked in `apm.lock.yaml`). Where the path +also matches a configured dependency or MCP deny pattern, the report +MUST additionally carry a supplemental conflict note naming that +pattern; this note is enrichment only and never itself causes a +tracked path to be surfaced. Where the primitive type is +determinable, the surfaced entry MUST carry its +inferred primitive type; where it is not determinable, the type +annotation MUST be omitted. + +(c) A path matched by a configured `unmanaged_files.exclude` glob +MUST NOT be surfaced, even when it also matches a deny pattern. + +This requirement governs the **completeness** of unmanaged-artifact +reporting only: whether a surfaced artifact yields a non-passing +audit result remains governed by `unmanaged_files.action` per the +merge table in [Section 6.4](#64-inheritance-and-merge-rules), so +req-pl-015 is not an enforcement claim. #### 6.3.6 `security` @@ -1277,6 +1312,7 @@ merge a policy chain per the following table: | `mcp.trust_transitive` | Logical AND. | | `manifest.scripts` | Stricter wins (`deny` > `allow`). | | `unmanaged_files.action` | Stricter wins (`deny` > `warn` > `ignore`). | +| `unmanaged_files.exclude` | Union, deduplicated (byte-exact on each pattern's UTF-8 string), parent order preserved (additive: a child adds exclusions and cannot clear a parent's; `null` and `[]` both preserve the parent list). | | `security.integrity.require_hashes` | Logical OR (once `true`, stays `true`). | | `security.audit.fail_on_drift` | Logical OR (once `true`, stays `true`). | @@ -1370,7 +1406,8 @@ This section's normative statements are: [req-pl-007](#req-pl-007), [req-pl-008](#req-pl-008), [req-pl-009](#req-pl-009), [req-pl-010](#req-pl-010), [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-013](#req-pl-013), [req-pl-014](#req-pl-014), + [req-pl-015](#req-pl-015). --- @@ -2712,6 +2749,7 @@ renumbering of conformance classes. | [req-pl-012](#req-pl-012) | MUST | 6.1.1 | governance | | [req-pl-013](#req-pl-013) | MUST | 6.8 | governance | | [req-pl-014](#req-pl-014) | MUST | 6.8 | governance | +| [req-pl-015](#req-pl-015) | MUST | 6.3.5 | 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 | @@ -2747,7 +2785,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: 89** (84 MUST, 5 SHOULD). +**Total normative statements: 90** (85 MUST, 5 SHOULD). --- @@ -2760,6 +2798,7 @@ renumbering of conformance classes. | 0.1.1 | 2026-05-24 | v1.1 editorial+defensive fold. Closed convergent round-2 followups: Section 12.3 CI-binding MUST-for-claim (req-cf-002); req-mf-019 class reclassification (producer -> consumer); three stale heading labels in req-cf-001 and Appendix E.4; depEntry oneOf source-key requirement plus new fixture `manifest/invalid-no-source-key.yml`; normative-count reconciliation across Section 1.3, Appendix C trailer, and this row; bare-hex pattern anchored to exactly 64 hex characters; req-sc-007 redaction scope extended to packed bundles, lockfiles, and audit records, plus producer secret-pattern refuse-to-pack rule; workspaces MUST-NOT-use in v0.1 (req-mf-021); nest-mode reject-in-v0.1 (req-rs-013); tag-name regex tightened to the semver.org 2.0.0 reference grammar; build-metadata tie-break rule (req-rs-014); mirror-tolerance editorial note (replicate-verbatim); req-rg-001 cross-references added in Section 7.5.1 and Section 10.5; bare-hex reader-tolerance deprecation horizon; interoperability informative note Section 6.1.2; conformance-summary precedence rule in Section 11.1; wildcard typo `x.y.x` -> `x.y.z`; resolved_by worked-example fragment in Section 7.4. Statement count: 83 -> 87. Drift-detection scaffolding lands in-spec and in-tree (informative machine-readable manifest at `docs/public/specs/manifests/openapm-v0.1.requirements.yml`; 4-way orphan_check + spec-conformance pytest suite + generated `CONFORMANCE.{md,json}` at repo root); Section 12.3 language updated to identify HTML anchors as the canonical source. No normative count change. | | 0.1.2 | 2026-05-28 | Round-3 spec-guardian editorial fold (no new normative statements; statement count remains 87). Section 11.3.2 Consumer enumeration appended `[req-rs-014]` and `[req-cf-002]` (closing drift vs Appendix C). req-lk-005 extended: writers MUST canonicalise the `dependencies` list in ascending lexicographic order of (`repo_url`, `virtual_path`) so frozen-install diffs are stable across implementations. req-sc-003 extended: consumers MUST drop the originating Authorization header before issuing a cross-host-class redirect (closes the mirror-redirect token-leak surface in Section 10.3). req-rg-001 extended with publish-side idempotency clause: a Registry MUST either reject a republish or accept ONLY if bytes are byte-identical to the previously-served bytes. Section 6.2 + Section 6.3.1 defaults pinned: `fetch_failure` defaults to `warn` and `dependencies.require_resolution` defaults to `project-wins` (mirrored as advisory `"default"` annotations in `policy-v0.1.schema.json`). Manifest schema `conflict_resolution` enum aligned to prose: renamed `intersect` -> `intersection-pick`, dropped `nest` from the v0.1 enum (`nest` remains reserved-for-v0.2 per req-rs-013, now noted via schema `$comment`). Mode B silent-extension detector landed in `.github/workflows/spec-conformance.yml` and `tests/spec_conformance/mode_b_detector.sh`; closes the named sole-implementer rot risk by gating PRs that add substantive code under critical paths (`primitives/`, `deps/`, `policy/`, `registry/`, `runtime/`, `install/`, `integration/`) without a spec citation, with auditable `apm-spec-waiver:` opt-out. | | 0.1.3 | 2026-06-16 | Spec-citation fold for the declarable integrity policy keys. Added two governance MUSTs under a new Section 6.8 "Integrity controls": [req-pl-013] (`security.integrity.require_hashes` -- fail-closed install when a resolved non-local dependency lacks a recorded hash in `apm.lock.yaml`, or the lockfile is absent/unreadable) and [req-pl-014] (`security.audit.fail_on_drift` -- audit exits non-zero on detected or indeterminate drift). Both keys are default-off and merge by logical OR (tighten-not-relax). Added the non-normative Section 6.3.6 `security` field reference and two merge-table rows; renumbered the governance conformance trailer 6.8 -> 6.9. Statement count: 87 -> 89 (84 MUST, 5 SHOULD). NOTE: a sibling spec-citation amendment also edits the shared count sites (Section 1.3, Appendix C trailer, this revision history); whichever lands second reconciles the cumulative total and takes the union of the added Appendix C rows. | +| 0.1.4 | 2026-06-16 | Normative addition (semver-zero `0.x` minor): added `[req-pl-015]` (Section 6.3.5, governance MUST) codifying unmanaged-artifact surfacing completeness -- a governance implementation evaluating policy over a populated primitive target tree MUST surface every file under a managed primitive target directory that is neither recorded in `apm.lock.yaml` nor matched by a configured `unmanaged_files.exclude` glob, each with its unmanaged reason and a supplemental dependency/MCP deny-conflict note where applicable; the inferred primitive type is carried where determinable and omitted otherwise; an excluded path MUST NOT be surfaced even when it also matches a deny pattern. The requirement body is structured as sub-clauses (a)/(b)/(c) so each obligation is individually citable. Added the `unmanaged_files.exclude` row to the Section 6.4 merge table (additive union, deduplicated, parent order preserved). The requirement governs reporting COMPLETENESS only; enforcement stays governed by `unmanaged_files.action`. Reconciled with the sibling 0.1.3 amendment (req-pl-013/req-pl-014): cumulative statement count 89 -> 90 (85 MUST, 5 SHOULD); Appendix C carries the union of all three new governance rows. | Errata (none at publication). diff --git a/packages/apm-guide/.apm/skills/apm-usage/governance.md b/packages/apm-guide/.apm/skills/apm-usage/governance.md index b436ce8e7..d148c97f8 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/governance.md +++ b/packages/apm-guide/.apm/skills/apm-usage/governance.md @@ -56,6 +56,7 @@ manifest: unmanaged_files: action: ignore # ignore | warn | deny directories: [] # directories to scan + exclude: [] # path globs to suppress (known harness-managed files) registry_source: # experimental: requires `apm experimental enable registries` require: [] # registry names that MUST be reachable in the merged registry map diff --git a/src/apm_cli/policy/discovery.py b/src/apm_cli/policy/discovery.py index 97e52f29c..b76d68262 100644 --- a/src/apm_cli/policy/discovery.py +++ b/src/apm_cli/policy/discovery.py @@ -1080,6 +1080,7 @@ def _opt_list(val: tuple[str, ...] | None) -> list | None: "unmanaged_files": { "action": policy.unmanaged_files.action, "directories": list(policy.unmanaged_files.directories or ()), + "exclude": list(policy.unmanaged_files.exclude or ()), }, } diff --git a/src/apm_cli/policy/inheritance.py b/src/apm_cli/policy/inheritance.py index 2489b984f..afc4df4a7 100644 --- a/src/apm_cli/policy/inheritance.py +++ b/src/apm_cli/policy/inheritance.py @@ -254,7 +254,7 @@ def _merge_unmanaged_files( parent: UnmanagedFilesPolicy, child: UnmanagedFilesPolicy ) -> UnmanagedFilesPolicy: """Merge unmanaged-files policy; omitted child block is transparent (#1198).""" - if child.action is None and child.directories is None: + if child.action is None and child.directories is None and child.exclude is None: return parent if child.action is None: @@ -274,10 +274,15 @@ def _merge_unmanaged_files( child.directories, ) + if child.exclude is None: + eff_exclude = parent.exclude + else: + eff_exclude = _union(parent.exclude or (), child.exclude) + eff_action = eff_action_raw if eff_action_raw is not None else "ignore" eff_dirs_out: tuple[str, ...] = () if eff_dirs is None else eff_dirs - return UnmanagedFilesPolicy(action=eff_action, directories=eff_dirs_out) + return UnmanagedFilesPolicy(action=eff_action, directories=eff_dirs_out, exclude=eff_exclude) def _merge_security(parent: SecurityPolicy, child: SecurityPolicy) -> SecurityPolicy: diff --git a/src/apm_cli/policy/matcher.py b/src/apm_cli/policy/matcher.py index 237897b9d..a28b0f33d 100644 --- a/src/apm_cli/policy/matcher.py +++ b/src/apm_cli/policy/matcher.py @@ -40,6 +40,20 @@ def matches_pattern(canonical_ref: str, pattern: str) -> bool: return bool(_compile_pattern(pattern).match(canonical_ref)) +def first_matching_pattern(name: str, patterns: tuple[str, ...] | None) -> str | None: + """Return the first glob pattern in *patterns* that matches *name*. + + Single source of truth for glob membership. Reused by the dependency / + MCP deny-list checks (via :func:`_check_allow_deny`) and by the + unmanaged-files check for both deny-conflict surfacing and ``exclude`` + suppression -- so there is exactly one matcher, never a parallel impl. + """ + for pattern in patterns or (): + if matches_pattern(name, pattern): + return pattern + return None + + def _check_allow_deny( ref: str, allow: tuple[str, ...] | None, @@ -53,16 +67,15 @@ def _check_allow_deny( 4. If ref matches any allow pattern -> allowed. 5. Otherwise -> not in allowed sources. """ - for pattern in deny: - if matches_pattern(ref, pattern): - return False, f"denied by pattern: {pattern}" + denied = first_matching_pattern(ref, deny) + if denied is not None: + return False, f"denied by pattern: {denied}" if allow is None: return True, "" - for pattern in allow: - if matches_pattern(ref, pattern): - return True, "" + if first_matching_pattern(ref, allow) is not None: + return True, "" return False, "not in allowed sources" diff --git a/src/apm_cli/policy/parser.py b/src/apm_cli/policy/parser.py index 74183ef60..538408483 100644 --- a/src/apm_cli/policy/parser.py +++ b/src/apm_cli/policy/parser.py @@ -170,6 +170,12 @@ def validate_policy(data: dict) -> tuple[list[str], list[str]]: f"unmanaged_files.action must be one of " f"{sorted(_VALID_UNMANAGED_ACTION)}, got '{action}'" ) + exclude = uf.get("exclude") + if exclude is not None and not isinstance(exclude, list): + errors.append( + "unmanaged_files.exclude must be a YAML list of path globs " + f"(got {type(exclude).__name__})" + ) # security.audit (install-time audit + external scanners) security = data.get("security") @@ -278,7 +284,14 @@ def _build_policy(data: dict) -> ApmPolicy: uf_data = raw_uf action = uf_data.get("action") directories = _parse_tuple(uf_data.get("directories")) if "directories" in uf_data else None - unmanaged_files = UnmanagedFilesPolicy(action=action, directories=directories) + exclude = ( + None + if ("exclude" not in uf_data or uf_data["exclude"] is None) + else _parse_tuple(uf_data["exclude"]) + ) + unmanaged_files = UnmanagedFilesPolicy( + action=action, directories=directories, exclude=exclude + ) reg_data = data.get("registry_source") or {} registry_source = RegistrySourcePolicy( diff --git a/src/apm_cli/policy/policy_checks.py b/src/apm_cli/policy/policy_checks.py index dc566cebe..a757414ac 100644 --- a/src/apm_cli/policy/policy_checks.py +++ b/src/apm_cli/policy/policy_checks.py @@ -8,6 +8,7 @@ from __future__ import annotations import logging +import os from pathlib import Path from .models import CheckResult, CIAuditResult @@ -685,13 +686,121 @@ def _check_scripts_policy( _MAX_UNMANAGED_SCAN_FILES = 10_000 +# Appended once to a non-empty unmanaged-files report so a flagged file is +# self-resolving: the reader learns how to track it or how to suppress it. +_UNMANAGED_NEXT_ACTION = ( + "Next: run 'apm install ' to track a flagged file, " + "or add a glob to unmanaged_files.exclude to suppress it." +) + + +def _classify_primitive_type(rel_path: str) -> str | None: + """Lazily classify an already-flagged unmanaged file by APM convention. + + Called ONLY on files already flagged as unmanaged -- never on the whole + tree -- so a user can triage skill / agent / instruction / mcp artifacts. + Returns ``None`` when the path matches no known primitive convention. + """ + posix = rel_path.replace("\\", "/").lower() + name = posix.rsplit("/", 1)[-1] + segments = posix.split("/") + # Explicit filename conventions win first (most specific signal). + if name.endswith(".instructions.md"): + return "instruction" + if name.endswith(".agent.md"): + return "agent" + if name == "mcp.json" or name.endswith(".mcp.json"): + return "mcp" + if name == "skill.md": + return "skill" + # Directory-segment hints next (less specific). MCP is narrowed to a + # dedicated ``.mcp/`` root -- a directory merely named ``mcp`` is not an + # MCP config and must not mislabel files under it. + if "instructions" in segments: + return "instruction" + if "agents" in segments: + return "agent" + if "skills" in segments: + return "skill" + if ".mcp" in segments: + return "mcp" + return None + + +def _unmanaged_deny_conflict( + rel_path: str, + dependency_deny: tuple[str, ...] | None, + mcp_deny: tuple[str, ...] | None, +) -> str | None: + """Return the deny pattern an unmanaged file conflicts with, or ``None``. + + Surfaces APM's OWN deny policy as a human-resolve conflict: the dependency + side is defaults-inclusive (``dependencies.effective_deny``) and the MCP + side is the raw ``mcp.deny`` -- mirroring the deny-list checks exactly. + Routes through the same ``first_matching_pattern`` matcher the deny-list + checks use -- never a second matcher. + """ + from .matcher import first_matching_pattern + + name = rel_path.rsplit("/", 1)[-1] + for patterns in (dependency_deny, mcp_deny): + hit = first_matching_pattern(rel_path, patterns) + if hit is None: + # Fall back to the basename so a deny glob written against a bare + # filename (e.g. 'mcp.json') still surfaces the conflict. + hit = first_matching_pattern(name, patterns) + if hit is not None: + return hit + return None + + +def _format_unmanaged_detail( + rel_path: str, + primitive_type: str | None, + deny_hit: str | None, +) -> str: + """Render one enriched, ASCII-only finding line for an unmanaged file.""" + label = f"{rel_path} [type: {primitive_type}]" if primitive_type else rel_path + reasons = ["not tracked in apm.lock.yaml"] + if deny_hit: + reasons.append(f"matches deny rule ({deny_hit})") + return f"{label} -- {'; '.join(reasons)}" + + +def _symlink_escapes_workspace(path: Path, project_root: Path) -> bool: + """Return True if *path* is a symlink resolving outside *project_root*. + + Guards the traversal so a symlink pointing out of the workspace is never + followed (no traversal bomb); broken or looping links also count as + escaping and are skipped. + """ + try: + resolved = path.resolve() + resolved.relative_to(project_root.resolve()) + return False + except (OSError, RuntimeError, ValueError): + return True + def _check_unmanaged_files( project_root: Path, lock: LockFile | None, policy: UnmanagedFilesPolicy, + *, + dependency_deny: tuple[str, ...] | None = None, + mcp_deny: tuple[str, ...] | None = None, ) -> CheckResult: - """Check 16: no untracked files in governance directories.""" + """Check 16: surface files in governance dirs not tracked in apm.lock.yaml. + + This is the ONE unified unmanaged-files report. Each flagged file is + enriched in-place (within this single scan) with a factual reason, a lazy + primitive-type classification, and -- where it matches APM's own + ``dependencies.deny`` / ``mcp.deny`` -- a deny-conflict note for a human to + resolve. Paths matching ``policy.exclude`` are suppressed. This is drift / + divergence visibility, not supply-chain-attack prevention. + """ + from .matcher import first_matching_pattern + if policy.effective_action == "ignore": return CheckResult( name="unmanaged-files", @@ -700,10 +809,11 @@ def _check_unmanaged_files( ) dirs = policy.directories if policy.directories else _DEFAULT_GOVERNANCE_DIRS + exclude = policy.exclude or () # Build set of deployed files AND directory prefixes from lockfile - deployed: set = set() - deployed_dir_prefixes: list = [] + deployed: set[str] = set() + deployed_dir_prefixes: list[str] = [] if lock: for _key, dep in lock.dependencies.items(): for f in dep.deployed_files: @@ -714,24 +824,41 @@ def _check_unmanaged_files( dir_prefix_tuple = tuple(deployed_dir_prefixes) - unmanaged: list[str] = [] + details: list[str] = [] + unmanaged_count = 0 files_scanned = 0 cap_hit = False for gov_dir in dirs: dir_path = project_root / gov_dir if not dir_path.exists() or not dir_path.is_dir(): continue - for file_path in dir_path.rglob("*"): - if file_path.is_file(): + # os.walk(followlinks=False) never recurses INTO a directory symlink, so + # a symlinked dir resolving outside the workspace is never traversed + # (the house pattern from security/gate.py). File symlinks still appear + # in the listing and are guarded individually below. + for dirpath, _subdirs, filenames in os.walk(dir_path, followlinks=False): + for fname in filenames: + file_path = Path(dirpath) / fname + # File-symlink guard: never follow a link out of the workspace. + if file_path.is_symlink() and _symlink_escapes_workspace(file_path, project_root): + continue + if not file_path.is_file(): + continue files_scanned += 1 if files_scanned > _MAX_UNMANAGED_SCAN_FILES: cap_hit = True break rel = file_path.relative_to(project_root).as_posix() - if rel not in deployed and not ( - dir_prefix_tuple and rel.startswith(dir_prefix_tuple) - ): - unmanaged.append(rel) + if rel in deployed or (dir_prefix_tuple and rel.startswith(dir_prefix_tuple)): + continue + if first_matching_pattern(rel, exclude) is not None: + continue + primitive_type = _classify_primitive_type(rel) + deny_hit = _unmanaged_deny_conflict(rel, dependency_deny, mcp_deny) + details.append(_format_unmanaged_detail(rel, primitive_type, deny_hit)) + unmanaged_count += 1 + if cap_hit: + break if cap_hit: break @@ -745,31 +872,34 @@ def _check_unmanaged_files( ), details=[ f"Governance directories contain > {_MAX_UNMANAGED_SCAN_FILES:,} files; " - "consider adding exclude patterns in a future policy version" + "consider adding exclude patterns in the unmanaged_files policy" ], ) - if not unmanaged: + if not details: return CheckResult( name="unmanaged-files", passed=True, message="No unmanaged files in governance directories", ) + # One report carries a single next-action hint after the per-file lines. + details.append(_UNMANAGED_NEXT_ACTION) + if policy.effective_action == "warn": return CheckResult( name="unmanaged-files", passed=True, - message=f"{len(unmanaged)} unmanaged file(s) found (warn)", - details=unmanaged, + message=f"{unmanaged_count} unmanaged file(s) found (warn)", + details=details, ) # action == "deny" return CheckResult( name="unmanaged-files", passed=False, - message=f"{len(unmanaged)} unmanaged file(s) in governance directories", - details=unmanaged, + message=f"{unmanaged_count} unmanaged file(s) in governance directories", + details=details, ) @@ -1130,6 +1260,14 @@ def _run(check: CheckResult) -> bool: return result # Unmanaged files check (16) - _run(_check_unmanaged_files(project_root, lock, policy.unmanaged_files)) + _run( + _check_unmanaged_files( + project_root, + lock, + policy.unmanaged_files, + dependency_deny=policy.dependencies.effective_deny, + mcp_deny=policy.mcp.deny, + ) + ) return result diff --git a/src/apm_cli/policy/schema.py b/src/apm_cli/policy/schema.py index f5dc40203..8246a3e9d 100644 --- a/src/apm_cli/policy/schema.py +++ b/src/apm_cli/policy/schema.py @@ -117,10 +117,15 @@ class UnmanagedFilesPolicy: When either field is set (including ``directories=()`` with a declared ``directories`` key), the merge applies escalation / union rules. ``action`` is then one of ``ignore`` | ``warn`` | ``deny``. + + ``exclude`` is a glob allow-list of workspace paths to suppress from the + report -- used to silence known harness-managed artifacts. ``None`` means + "no opinion" (transparent during merge); a set list is union-merged. """ action: str | None = None # None | ignore | warn | deny directories: tuple[str, ...] | None = None # None -> no opinion; () explicit + exclude: tuple[str, ...] | None = None # None -> no opinion; globs to suppress @property def effective_action(self) -> str: diff --git a/tests/fixtures/policy/golden/parsed-policies.json b/tests/fixtures/policy/golden/parsed-policies.json index af8da56d0..251482c70 100644 --- a/tests/fixtures/policy/golden/parsed-policies.json +++ b/tests/fixtures/policy/golden/parsed-policies.json @@ -64,7 +64,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -130,7 +131,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -198,7 +200,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -264,7 +267,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -330,7 +334,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -398,7 +403,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -466,7 +472,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -532,7 +539,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -598,7 +606,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -664,7 +673,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -738,7 +748,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -804,7 +815,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -870,7 +882,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -936,7 +949,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -1004,7 +1018,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -1072,7 +1087,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -1140,7 +1156,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -1206,7 +1223,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -1281,7 +1299,8 @@ }, "unmanaged_files": { "action": "deny", - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -1347,7 +1366,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" }, @@ -1441,7 +1461,8 @@ }, "unmanaged_files": { "action": "warn", - "directories": [] + "directories": [], + "exclude": null }, "version": "1.0.0" }, @@ -1509,7 +1530,8 @@ }, "unmanaged_files": { "action": null, - "directories": null + "directories": null, + "exclude": null }, "version": "1.0.0" } diff --git a/tests/spec_conformance/test_policy_reqs.py b/tests/spec_conformance/test_policy_reqs.py index ddc127039..450deae92 100644 --- a/tests/spec_conformance/test_policy_reqs.py +++ b/tests/spec_conformance/test_policy_reqs.py @@ -164,4 +164,50 @@ def test_policy_fail_on_drift_parses_and_is_specified(): ) +@pytest.mark.req("req-pl-015") +def test_unmanaged_files_surfacing_completeness(tmp_path): + """req-pl-015: surface every untracked file under a managed primitive + target tree with its reason, deny-conflict note, and inferred type, + while an ``unmanaged_files.exclude`` match is never surfaced. + + Binds the spec MUST to the real ``_check_unmanaged_files`` behavior so a + regression in reason strings, type tagging, deny-conflict, or exclude + suppression breaks at PR time. Also asserts the normative spec phrases. + """ + from apm_cli.policy.policy_checks import _check_unmanaged_files + from apm_cli.policy.schema import UnmanagedFilesPolicy + + inst_dir = tmp_path / ".github" / "instructions" + agents_dir = tmp_path / ".github" / "agents" + inst_dir.mkdir(parents=True) + agents_dir.mkdir(parents=True) + (inst_dir / "foo.instructions.md").write_text("x", encoding="utf-8") + (inst_dir / "ignored.md").write_text("x", encoding="utf-8") + (agents_dir / "evil.agent.md").write_text("x", encoding="utf-8") + + policy = UnmanagedFilesPolicy( + action="warn", + directories=(".github/instructions", ".github/agents"), + exclude=("**/ignored.md",), + ) + result = _check_unmanaged_files(tmp_path, None, policy, dependency_deny=("**/evil*",)) + body = "\n".join(result.details) + + # Untracked primitive surfaced with reason + inferred type. + assert ".github/instructions/foo.instructions.md [type: instruction]" in body + assert "not tracked in apm.lock.yaml" in body + # Deny-conflict note carried where the path also matches a deny pattern. + assert "matches deny rule (**/evil*)" in body + assert ".github/agents/evil.agent.md [type: agent]" in body + # An excluded path MUST NOT be surfaced. + assert "ignored.md" not in body + + # Spec-text drift guard: the normative phrasing MUST stay in the body. + assert_spec_contains( + "not tracked in `apm.lock.yaml`", + "inferred primitive type", + "MUST NOT be surfaced", + ) + + _ = waive # keep import for any future structural waiver diff --git a/tests/unit/policy/test_inheritance.py b/tests/unit/policy/test_inheritance.py index e5f54c2f8..bcf08842f 100644 --- a/tests/unit/policy/test_inheritance.py +++ b/tests/unit/policy/test_inheritance.py @@ -834,6 +834,39 @@ def test_deny_union_merged(self): self.assertEqual(set(result.bin_deploy.deny), {"a/b", "c/d"}) +class TestMergeUnmanagedExclude(unittest.TestCase): + """The unmanaged-files ``exclude`` list is union-merged across a chain.""" + + def test_child_exclude_unions_with_parent(self): + parent = ApmPolicy( + unmanaged_files=UnmanagedFilesPolicy(action="deny", exclude=(".github/copilot",)) + ) + child = ApmPolicy( + unmanaged_files=UnmanagedFilesPolicy(action=None, exclude=(".vscode/mcp.json",)) + ) + merged = merge_policies(parent, child) + self.assertEqual( + set(merged.unmanaged_files.exclude), + {".github/copilot", ".vscode/mcp.json"}, + ) + + def test_parent_exclude_inherited_when_child_silent(self): + parent = ApmPolicy( + unmanaged_files=UnmanagedFilesPolicy(action="deny", exclude=(".github/copilot",)) + ) + child = ApmPolicy() # no unmanaged_files block at all + merged = merge_policies(parent, child) + self.assertEqual(merged.unmanaged_files.exclude, (".github/copilot",)) + + def test_child_only_exclude_is_not_transparent(self): + # A child that sets only exclude must still carry it through, even + # though action and directories are None. + parent = ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(action="deny")) + child = ApmPolicy(unmanaged_files=UnmanagedFilesPolicy(exclude=(".cursor/rules/local.md",))) + merged = merge_policies(parent, child) + self.assertIn(".cursor/rules/local.md", merged.unmanaged_files.exclude) + + class TestMergeFieldCoverageGuard(unittest.TestCase): """Regression trap for the whole class of "forgotten field" bugs. diff --git a/tests/unit/policy/test_matcher.py b/tests/unit/policy/test_matcher.py index 1ae6b24cf..099f96a54 100644 --- a/tests/unit/policy/test_matcher.py +++ b/tests/unit/policy/test_matcher.py @@ -5,11 +5,43 @@ from apm_cli.policy.matcher import ( check_dependency_allowed, check_mcp_allowed, + first_matching_pattern, matches_pattern, ) from apm_cli.policy.schema import DependencyPolicy, McpPolicy +class TestFirstMatchingPattern(unittest.TestCase): + """The single glob matcher reused by deny-list and unmanaged-files checks.""" + + def test_returns_first_matching_pattern(self): + self.assertEqual( + first_matching_pattern("contoso/evil", ("contoso/good", "contoso/*")), + "contoso/*", + ) + + def test_returns_none_when_no_match(self): + self.assertIsNone(first_matching_pattern("contoso/repo", ("other/*",))) + + def test_none_patterns_is_safe(self): + self.assertIsNone(first_matching_pattern("contoso/repo", None)) + + def test_empty_patterns_is_safe(self): + self.assertIsNone(first_matching_pattern("contoso/repo", ())) + + def test_deny_check_uses_same_matcher(self): + # Deny enforcement and unmanaged-files deny-conflict must agree: + # both route through first_matching_pattern. + policy = DependencyPolicy(deny=("contoso/*",)) + allowed, reason = check_dependency_allowed("contoso/evil", policy) + self.assertFalse(allowed) + self.assertIn("contoso/*", reason) + self.assertEqual( + first_matching_pattern("contoso/evil", policy.effective_deny), + "contoso/*", + ) + + class TestMatchesPattern(unittest.TestCase): """Test glob-style pattern matching.""" diff --git a/tests/unit/policy/test_parser.py b/tests/unit/policy/test_parser.py index b44e09a28..e2bcdc066 100644 --- a/tests/unit/policy/test_parser.py +++ b/tests/unit/policy/test_parser.py @@ -339,6 +339,50 @@ def test_omitted_unmanaged_files_yields_none_action(self): self.assertEqual(policy.unmanaged_files.effective_action, "ignore") self.assertIsNone(policy.unmanaged_files.directories) + def test_unmanaged_exclude_null_is_transparent(self): + """`exclude: null` -> None (no opinion / transparent during merge).""" + yaml_str = textwrap.dedent(""" + unmanaged_files: + action: warn + exclude: null + """) + policy, _ = load_policy(yaml_str) + self.assertIsNone(policy.unmanaged_files.exclude) + + def test_unmanaged_exclude_empty_list_is_explicit(self): + """`exclude: []` -> empty tuple (explicit override, not transparent).""" + yaml_str = textwrap.dedent(""" + unmanaged_files: + action: warn + exclude: [] + """) + policy, _ = load_policy(yaml_str) + self.assertEqual(policy.unmanaged_files.exclude, ()) + + def test_unmanaged_exclude_list_parses_to_tuple(self): + """`exclude: [..]` -> tuple of globs.""" + yaml_str = textwrap.dedent(""" + unmanaged_files: + action: warn + exclude: + - .github/copilot-instructions.md + - .claude/settings.local.json + """) + policy, _ = load_policy(yaml_str) + self.assertEqual( + policy.unmanaged_files.exclude, + (".github/copilot-instructions.md", ".claude/settings.local.json"), + ) + + def test_unmanaged_exclude_absent_is_none(self): + """Key absent -> None (no opinion / transparent during merge).""" + yaml_str = textwrap.dedent(""" + unmanaged_files: + action: warn + """) + policy, _ = load_policy(yaml_str) + self.assertIsNone(policy.unmanaged_files.exclude) + def test_absent_dependencies_block_gives_none_deny_and_require(self): """Entirely absent dependencies: block -> deny=None, require=None (Fix 2).""" policy, _ = load_policy("name: p\nversion: '1'\nenforcement: warn\n") diff --git a/tests/unit/policy/test_policy_checks.py b/tests/unit/policy/test_policy_checks.py index d5fe5332e..b9a39a443 100644 --- a/tests/unit/policy/test_policy_checks.py +++ b/tests/unit/policy/test_policy_checks.py @@ -616,7 +616,7 @@ def test_fail_deny_unmanaged(self, tmp_path): policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) result = _check_unmanaged_files(tmp_path, lock, policy) assert not result.passed - assert ".github/agents/rogue.md" in result.details + assert any(".github/agents/rogue.md" in d for d in result.details) def test_warn_unmanaged(self, tmp_path): (tmp_path / ".cursor" / "rules").mkdir(parents=True) @@ -692,7 +692,7 @@ def test_handrolled_instruction_flagged_as_unmanaged(self, tmp_path: Path) -> No policy = UnmanagedFilesPolicy(action="deny", directories=[".github/instructions"]) result = _check_unmanaged_files(tmp_path, lock, policy) assert not result.passed - assert ".github/instructions/handrolled.instructions.md" in result.details + assert any(".github/instructions/handrolled.instructions.md" in d for d in result.details) def test_handrolled_file_not_masked_by_deployed_deps(self, tmp_path: Path) -> None: """A hand-rolled file is flagged even when sibling files are managed. @@ -722,7 +722,7 @@ def test_handrolled_file_not_masked_by_deployed_deps(self, tmp_path: Path) -> No policy = UnmanagedFilesPolicy(action="deny", directories=[".github/instructions"]) result = _check_unmanaged_files(tmp_path, lock, policy) assert not result.passed - assert ".github/instructions/rogue.instructions.md" in result.details + assert any(".github/instructions/rogue.instructions.md" in d for d in result.details) # Managed files must NOT appear in the unmanaged details list. assert ".github/instructions/a.instructions.md" not in result.details assert ".github/instructions/b.instructions.md" not in result.details @@ -749,9 +749,9 @@ def test_handrolled_file_across_governance_dirs(self, tmp_path: Path) -> None: ) result = _check_unmanaged_files(tmp_path, lock, policy) assert not result.passed - assert ".github/instructions/handrolled.instructions.md" in result.details - assert ".github/hooks/handrolled.hooks.md" in result.details - assert ".github/agents/handrolled.agents.md" in result.details + assert any(".github/instructions/handrolled.instructions.md" in d for d in result.details) + assert any(".github/hooks/handrolled.hooks.md" in d for d in result.details) + assert any(".github/agents/handrolled.agents.md" in d for d in result.details) def test_local_deployed_files_not_flagged(self, tmp_path: Path) -> None: """Files in local_deployed_files are treated as managed and not flagged. @@ -813,7 +813,7 @@ def test_dir_prefix_does_not_mask_instructions(self, tmp_path: Path) -> None: result = _check_unmanaged_files(tmp_path, lock, policy) assert not result.passed # The hand-rolled instruction file must be flagged... - assert ".github/instructions/handrolled.instructions.md" in result.details + assert any(".github/instructions/handrolled.instructions.md" in d for d in result.details) # ...while files under the managed skill directory must NOT be flagged. assert ".github/skills/my-skill/SKILL.md" not in result.details @@ -848,7 +848,7 @@ def test_rogue_file_caught_parent_deny_child_omits_block(self, tmp_path): lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) result = _check_unmanaged_files(tmp_path, lock, merged.unmanaged_files) assert not result.passed - assert ".github/agents/rogue.md" in result.details + assert any(".github/agents/rogue.md" in d for d in result.details) def test_integration_chain_deny_propagates(self, tmp_path): """Integration chain: parent deny + child omits block -> merge -> check catches rogue file.""" @@ -862,12 +862,238 @@ def test_integration_chain_deny_propagates(self, tmp_path): lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) result = _check_unmanaged_files(tmp_path, lock, merged.unmanaged_files) assert not result.passed - assert ".github/agents/rogue.md" in result.details + assert any(".github/agents/rogue.md" in d for d in result.details) -# -- Integration: run_policy_checks --------------------------------- +# -- Check 16b: enriched unmanaged-files (#1775) -------------------- + + +def _detail_for(result, path: str) -> str | None: + """Return the single enriched detail line that mentions *path*.""" + matches = [d for d in result.details if path in d] + assert len(matches) == 1, f"{path} reported {len(matches)} time(s): {result.details}" + return matches[0] + + +class TestUnmanagedClassification: + """Lazy primitive-type classification of already-flagged files.""" + + def test_classify_skill_dir(self): + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".claude/skills/my-skill/helper.md") == "skill" + + def test_classify_instruction_file(self): + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".github/instructions/x.instructions.md") == "instruction" + + def test_classify_agent_dir(self): + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".github/agents/reviewer.agent.md") == "agent" + + def test_classify_mcp_config(self): + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".vscode/mcp.json") == "mcp" + + def test_classify_agent_in_mcp_named_dir(self): + # A path segment literally named "mcp" must NOT override an explicit + # .agent.md filename convention. + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".github/agents/mcp/rogue.agent.md") == "agent" + + def test_classify_dot_mcp_root_config(self): + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".mcp/servers.json") == "mcp" + + def test_classify_plain_mcp_segment_not_mcp(self): + # A non-config file under a dir merely named "mcp" is not an MCP config. + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".github/mcp/notes.txt") is None + + def test_classify_unknown_returns_none(self): + from apm_cli.policy.policy_checks import _classify_primitive_type + + assert _classify_primitive_type(".github/random/notes.txt") is None + + +class TestUnmanagedEnrichment: + """Each flagged file carries reason + type, surfaced in details.""" + + def test_reason_not_tracked_present(self, tmp_path): + (tmp_path / ".github" / "agents").mkdir(parents=True) + (tmp_path / ".github" / "agents" / "rogue.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files(tmp_path, lock, policy) + detail = _detail_for(result, ".github/agents/rogue.agent.md") + assert "not tracked in apm.lock.yaml" in detail + + def test_type_present_in_detail(self, tmp_path): + (tmp_path / ".github" / "agents").mkdir(parents=True) + (tmp_path / ".github" / "agents" / "rogue.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files(tmp_path, lock, policy) + detail = _detail_for(result, ".github/agents/rogue.agent.md") + assert "[type: agent]" in detail + + def test_next_action_hint_present(self, tmp_path): + # A non-empty report must carry exactly one actionable next-step hint + # (track vs suppress) so a flagged file is self-resolving. + (tmp_path / ".github" / "agents").mkdir(parents=True) + (tmp_path / ".github" / "agents" / "rogue.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files(tmp_path, lock, policy) + hints = [d for d in result.details if "unmanaged_files.exclude" in d and "apm install" in d] + assert len(hints) == 1, f"expected one next-action hint, got: {result.details}" + + +class TestUnmanagedDenyConflict: + """Unmanaged files matching apm-policy deny rules are surfaced as conflicts.""" + + def test_dependency_deny_conflict(self, tmp_path): + (tmp_path / ".github" / "agents").mkdir(parents=True) + (tmp_path / ".github" / "agents" / "rogue.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files( + tmp_path, + lock, + policy, + dependency_deny=("**/rogue*",), + ) + detail = _detail_for(result, ".github/agents/rogue.agent.md") + assert "matches deny rule" in detail + + def test_mcp_deny_conflict(self, tmp_path): + (tmp_path / ".vscode").mkdir(parents=True) + (tmp_path / ".vscode" / "mcp.json").write_text("{}", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".vscode"]) + result = _check_unmanaged_files( + tmp_path, + lock, + policy, + mcp_deny=("*mcp.json",), + ) + detail = _detail_for(result, ".vscode/mcp.json") + assert "matches deny rule" in detail + + def test_no_deny_conflict_when_no_match(self, tmp_path): + (tmp_path / ".github" / "agents").mkdir(parents=True) + (tmp_path / ".github" / "agents" / "clean.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files( + tmp_path, + lock, + policy, + dependency_deny=("**/evil*",), + ) + detail = _detail_for(result, ".github/agents/clean.agent.md") + assert "matches deny rule" not in detail + +class TestUnmanagedExcludeSuppression: + """Excluded paths are not reported.""" + def test_excluded_path_not_reported(self, tmp_path): + agents = tmp_path / ".github" / "agents" + agents.mkdir(parents=True) + (agents / "harness.agent.md").write_text("x", encoding="utf-8") + (agents / "rogue.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy( + action="deny", + directories=[".github/agents"], + exclude=[".github/agents/harness.agent.md"], + ) + result = _check_unmanaged_files(tmp_path, lock, policy) + assert not any(".github/agents/harness.agent.md" in d for d in result.details) + assert any(".github/agents/rogue.agent.md" in d for d in result.details) + + def test_exclude_glob_suppresses_subtree(self, tmp_path): + target = tmp_path / ".github" / "agents" / "vendor" + target.mkdir(parents=True) + (target / "a.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy( + action="deny", + directories=[".github/agents"], + exclude=[".github/agents/vendor/**"], + ) + result = _check_unmanaged_files(tmp_path, lock, policy) + assert result.passed + assert not result.details + + +class TestUnmanagedSymlinkGuard: + """Symlinks pointing outside the workspace are not followed.""" + + def test_symlink_outside_project_not_followed(self, tmp_path): + outside = tmp_path.parent / "outside_target.md" + outside.write_text("secret", encoding="utf-8") + agents = tmp_path / ".github" / "agents" + agents.mkdir(parents=True) + link = agents / "escape.agent.md" + try: + link.symlink_to(outside) + except (OSError, NotImplementedError): + pytest.skip("symlinks not supported on this platform") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files(tmp_path, lock, policy) + # The traversal must not follow the symlink out of the workspace. + assert not any("escape.agent.md" in d for d in result.details) + + def test_symlinked_directory_outside_not_recursed(self, tmp_path): + # A symlinked *directory* resolving outside the workspace must not be + # recursed into -- its contents must never surface. rglob() follows + # directory symlinks, so the scan must use a non-following walk. + outside = tmp_path.parent / "outside_tree" + outside.mkdir() + (outside / "secret.agent.md").write_text("secret", encoding="utf-8") + agents = tmp_path / ".github" / "agents" + agents.mkdir(parents=True) + link = agents / "linked" + try: + link.symlink_to(outside, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("symlinks not supported on this platform") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files(tmp_path, lock, policy) + assert not any("secret.agent.md" in d for d in result.details) + assert not any("linked" in d for d in result.details) + + +class TestUnmanagedNonDuplication: + """A file is reported exactly once -- one unified unmanaged report.""" + + def test_file_reported_once_even_with_deny_conflict(self, tmp_path): + (tmp_path / ".github" / "agents").mkdir(parents=True) + (tmp_path / ".github" / "agents" / "rogue.agent.md").write_text("x", encoding="utf-8") + lock = _make_lockfile([{"repo_url": "org/pkg", "deployed_files": []}]) + policy = UnmanagedFilesPolicy(action="deny", directories=[".github/agents"]) + result = _check_unmanaged_files( + tmp_path, + lock, + policy, + dependency_deny=("**/rogue*",), + mcp_deny=("**/rogue*",), + ) + occurrences = [d for d in result.details if ".github/agents/rogue.agent.md" in d] + assert len(occurrences) == 1 + + +# -- Integration: run_policy_checks --------------------------------- class TestRunPolicyChecks: def test_returns_all_18_checks(self, tmp_path): """Full run should produce exactly 18 checks."""