fix: audit contained local plugin sources (supersedes #2444, closes #2428)

[Daniel Meppiel (danielmeppiel)]

fix(marketplace): audit contained local plugin sources

TL;DR

This supersedes #2444 and closes #2428. It audits local marketplace plugin manifests only after symlink-aware containment, and strict audit now fails when no plugin could be verified. The branch also makes the failure actionable, documents the local workflow, and proves the metadata.pluginRoot path with the installed CLI.

Important

A local source that resolves outside its registered marketplace is reported as skipped and is never read.

Problem (WHY)

• Local marketplace string sources were skipped instead of being audited, leaving a coverage gap for a common offline CI shape.
• Strict audit could succeed with only skipped plugins, so CI had no verified manifest while returning success.
• A lexical path check alone does not stop a symlink from exposing an external apm.yml.
• [!] The strict all-skipped failure hid per-plugin reasons unless the user had already enabled verbose output.

The validation approach follows "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action.". The implementation stays surgical because "Favor small, chainable primitives over monolithic frameworks.".

Approach (WHAT)

#	Fix
1	Resolve every local audit manifest through the canonical symlink-aware containment helper.
2	Report missing or unsafe manifests as skipped, and make strict fail when no plugin was audited.
3	Preserve a clear distinction between skipped sources and verification errors in output and docs.
4	Guard containment with unit, boundary, mutation, and real-binary lifecycle coverage.

Implementation (HOW)

• src/apm_cli/marketplace/resolver.py -- exposes resolve_local_plugin_path() as the containment authority for a source, optional plugin root, and required apm.yml target.
• src/apm_cli/marketplace/audit.py -- reads local manifests through that resolver and threads MarketplaceManifest.plugin_root into each local audit.
• src/apm_cli/commands/marketplace/audit.py -- exits strict mode when nothing was audited and directs users to verbose skipped-source reasons.
• scripts/lint-architecture-boundaries.sh and tests/integration/test_architecture_authorities.py -- prevent audit code from bypassing the local resolver or weakening containment.
• tests/unit/marketplace/test_marketplace_audit.py -- covers clean local reads, missing manifests, traversal, plugin and manifest symlink escapes, and strict status.
• tests/integration/test_marketplace_local_audit_lifecycle.py -- exercises clean, skipped, traversal, escaped symlink, and metadata.pluginRoot cases through the installed binary.
• docs/src/content/docs/reference/cli/marketplace.md and packages/apm-guide/.apm/skills/apm-usage/commands.md -- describe local-source scope, skip versus verification-error behavior, strict semantics, and runnable local commands.

Diagrams

Legend: the new contained-resolution stage is the only path to a local apm.yml; strict mode treats a report set with no audited plugin as a failure.

flowchart LR
    subgraph Resolve[Resolve]
        S[local plugin source] --> R[resolve_local_plugin_path]
        R --> C{contained after resolve}
    end
    subgraph Audit[Audit]
        C -->|yes| M[read apm.yml]
        C -->|no| K[skipped report]
        M --> A[audit report]
        K --> A
    end
    subgraph Strict[Strict]
        A --> T{none audited}
        T -->|yes| E[exit 1]
        T -->|no| X[complete]
    end
    classDef new stroke-dasharray: 5 5;
    class R,C,K,T,E new;

Loading

Trade-offs

• Resolve before reading. Chose resolved-path containment over lexical validation because symlink targets must remain inside the marketplace root.
• Skip unsafe local sources. Chose an isolated skipped report over aborting the whole audit so one malformed plugin does not hide other results; strict mode still prevents false CI success when nothing is verifiable.
• Keep default output concise. Chose a recovery hint over always printing every skipped detail; --verbose remains the detailed diagnostics surface.
• Keep the existing fetch seam. Remote and test fetcher behavior remains unchanged while only production local reads receive plugin_root.

Benefits

1. A contained local source with apm.yml passes apm marketplace audit NAME --strict with exit 0.
2. Missing manifests, parent traversal, and escaping symlinks are not read; an all-skipped strict audit exits 1.
3. A bare source rooted by metadata.pluginRoot is exercised by an installed-binary lifecycle test.
4. The boundary lint rejects future local audit reads that bypass the canonical resolver.

Validation

APM_E2E_TESTS=1 uv run --extra dev pytest tests/integration/test_marketplace_local_audit_lifecycle.py::test_local_marketplace_audit_strict_lifecycle -q:

1 passed in 7.10s

uv run --extra dev pytest tests/unit/marketplace/test_marketplace_audit.py tests/integration/test_architecture_authorities.py::test_local_marketplace_audit_paths_have_single_owner tests/integration/test_architecture_authorities.py::test_local_marketplace_audit_path_owner_guard_rejects_bypass -q:

62 passed in 61.51s

Lint and mutation evidence

ruff check: All checks passed!
ruff format: 1596 files already formatted
pylint R0801: 10.00/10
auth-signal lint: clean
architecture boundary lint: clean

Removing plugin_root=manifest.plugin_root made test_local_marketplace_audit_strict_with_plugin_root fail with the strict audit exit 1; the call was restored before validation.

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	A contained local plugin manifest completes strict marketplace audit.	Governed by policy, DevX	tests/integration/test_marketplace_local_audit_lifecycle.py::test_local_marketplace_audit_strict_lifecycle (regression-trap for #2428)	e2e
2	A missing manifest, traversal, or escaping symlink is not read and cannot produce a strict success.	Secure by default, Governed by policy	tests/integration/test_marketplace_local_audit_lifecycle.py::test_local_marketplace_audit_strict_lifecycle	e2e
3	A bare local source under metadata.pluginRoot completes strict audit.	Governed by policy, DevX	tests/integration/test_marketplace_local_audit_lifecycle.py::test_local_marketplace_audit_strict_with_plugin_root	e2e
4	Strict all-skipped audit explains how to see the skipped reasons.	DevX	tests/unit/marketplace/test_marketplace_audit.py::TestAuditCLI::test_strict_exits_nonzero_when_every_plugin_is_skipped	unit

How to test

• Create a local marketplace with plugins/clean/apm.yml, add it, and run strict audit; expect exit 0.
• Use a marketplace whose only plugin lacks apm.yml; run strict audit and expect exit 1 with the verbose recovery command.
• Point a source at ../outside and a symlink outside the root; run strict verbose audit and expect skipped reasons with exit 1.
• Set metadata.pluginRoot to plugins with a bare plugin source; strict audit should report one clean plugin.
• Run bash scripts/lint-architecture-boundaries.sh; expect the local audit resolution authority check to pass.

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

[Copilot]

Pull request overview

This PR closes the local-marketplace audit coverage gap by auditing local string-source plugins only after symlink-aware containment, and by making --strict fail when nothing could be verified (preventing false CI “success” with all-skipped plugins).

Changes:

• Add resolve_local_plugin_path() as the canonical containment/target-resolution helper and route local apm.yml reads through it.
• Extend marketplace audit to read local plugin manifests safely (skip traversal/symlink escapes; treat malformed/unreadable as verification errors).
• Add unit + real-binary lifecycle coverage plus an architecture-boundary lint/test guard to prevent future bypasses.

Show a summary per file

File	Description
src/apm_cli/marketplace/resolver.py	Introduces resolve_local_plugin_path() as the symlink-aware containment authority (with relative_target).
src/apm_cli/marketplace/audit.py	Reads local string-source apm.yml via the resolver and threads plugin_root through audit calls.
src/apm_cli/commands/marketplace/audit.py	Updates --strict semantics/output and adds failure when no plugins were verified.
scripts/lint-architecture-boundaries.sh	Adds AC10b guard to enforce the single containment authority for local audit paths.
tests/unit/marketplace/test_marketplace_audit.py	Adds coverage for clean local reads + traversal/symlink escape rejection + strict all-skipped exit behavior.
tests/integration/test_marketplace_local_audit_lifecycle.py	Adds installed-binary lifecycle tests for clean/skipped/traversal/symlink/pluginRoot scenarios.
tests/integration/test_architecture_authorities.py	Adds mutation-style tests proving the architecture boundary guard rejects bypasses.
docs/src/content/docs/reference/cli/marketplace.md	Documents local-source audit behavior and updated strict semantics.
packages/apm-guide/.apm/skills/apm-usage/commands.md	Updates maintained command reference to reflect new audit semantics for local sources.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 2
• Review effort level: Lite

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: ship_now

PR #2460 closes the local marketplace audit lifecycle with validated strict-mode behavior and no remaining panel follow-ups.

cc Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

Panel signals converge: containment, security, performance, architecture, growth, and regression coverage are clean at head a7dd62511d0d14a9b5515126ffd40a552feeee86. The earlier strict skipped-source and output-UX findings were folded into a3a7a32fd and a7dd62511. Focused, integration, lint, duplication, auth, and boundary checks substantiate the intended lifecycle.

Aligned with: Secure by default: strict local audit behavior is covered by the required end-to-end lifecycle test. Pragmatic as npm: focused local-install and audit validation avoids user-facing friction.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	0	One resolver authority remains for local containment.
CLI Logging Expert	0	0	0	Strict skipped-source failures now explain recovery.
DevX UX Expert	0	0	0	Verbose guidance only appears when skipped reasons exist.
Supply Chain Security Expert	0	0	0	Traversal and escaping symlinks fail closed.
OSS Growth Hacker	0	0	0	Changelog and reference explain the CI contract.
Doc Writer	0	0	0	Prior validated review found documentation accurate.
Test Coverage Expert	0	0	0	Audit and install escape paths have regression coverage.
Performance Expert	0	0	0	No marketplace audit performance regression found.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Architecture

classDiagram
    direction LR
    class MarketplaceAuditCommand {
      <<IOBoundary>>
      +audit(name, strict, verbose)
    }
    class MarketplaceAuditModule {
      <<Adapter>>
      +fetch_plugin_apm_yml(plugin, source, plugin_root)
      +run_audit(manifest, source)
    }
    class MarketplaceResolverModule {
      <<CanonicalAuthority>>
      +resolve_local_plugin_path(source, marketplace, plugin_root, relative_target) Path
    }
    class PathSecurity {
      <<Guard>>
      +ensure_path_within(path, base_dir) Path
    }
    MarketplaceAuditCommand ..> MarketplaceAuditModule : invokes
    MarketplaceAuditModule ..> MarketplaceResolverModule : resolves local manifest
    MarketplaceResolverModule ..> PathSecurity : delegates containment

Loading

flowchart TD
    A["marketplace audit strict"] --> B["run_audit"]
    B --> C["resolve_local_plugin_path target apm.yml"]
    C --> D["ensure_path_within marketplace root"]
    D -->|contained| E["read and audit manifest"]
    D -->|escape or traversal| F["skipped source"]
    E --> G{"strict?"}
    F --> G
    G -->|all verified| H["exit 0"]
    G -->|skip error or bypass| I["exit 1"]

Loading

Folded in this run

• (panel) Strict mode now fails and explains a partial skipped source -- resolved in a3a7a32fd and a7dd62511.
• (panel) Verbose recovery guidance no longer repeats when skipped details are already visible -- resolved in a3a7a32fd.
• (panel) Local install resolution rejects escaping symlink targets -- resolved in a3a7a32fd.
• (panel) Changelog and strict-mode documentation now describe skipped-source behavior -- resolved in a3a7a32fd.

Copilot signals reviewed

• No inline Copilot findings were present in this run.

Regression-trap evidence (mutation-break gate)

• test_strict_exits_nonzero_when_a_clean_plugin_is_skipped -- deleted the strict skipped-source guard; test failed as expected; guard restored.
• test_install_rejects_local_marketplace_symlink_escape -- removed containment enforcement; test failed as expected; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ were silent. Pylint R0801, auth-signal lint, and architecture-boundary lint also passed.

CI

All PR checks passed on a7dd62511d0d14a9b5515126ffd40a552feeee86 after 0 CI fix iterations.

Mergeability status

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#2460	a7dd625	ship_now	2	4	0	1	green	MERGEABLE	BLOCKED	awaiting maintainer review

Recommendation

Ship now. The exact head passed the required validation, and the panel has no remaining follow-ups.

---

Full per-persona findings

Python Architect

No remaining findings after convergence.

CLI Logging Expert

No remaining findings after convergence.

DevX UX Expert

No remaining findings after convergence.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

No auth-relevant surface touched.

Doc Writer

Prior validated documentation review found no findings; final retry schema output was malformed twice.

Test Coverage Expert

No findings.

Performance Expert

No findings.

_{This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.}