fix: harden plugin security, validation, tests, and docs (follow-up to #83) by danielmeppiel · Pull Request #208 · microsoft/apm

danielmeppiel · 2026-03-09T14:11:50Z

Plugin Support Hardening

Follow-up to #83 (SebastienDegodez's plugin support). Adds security hardening, validation, tests, and docs for the plugin pipeline.

Security & Validation

Symlink security: Skip symlinks during artifact mapping via ignore=_ignore_symlinks callback (prevents content exfiltration)
Validation hardening: Require plugin evidence (plugin.json or component directories) — empty dirs no longer classify as plugins
Name validation: Warning when name field missing from plugin.json

Code Fixes

Fixed package type: Synthesized apm.yml uses hybrid as a safe default so the standard pipeline handles all components; actual install behavior is content-driven (presence of SKILL.md, component directories, etc.)
Custom component paths: Support plugin.json arrays and individual file paths for non-standard directory layouts (e.g. "agents": ["./agents/planner.md", "./agents/coder.md"])
Named subdirectory preservation: Array of subdirectory paths preserves dir names to avoid collisions (e.g. multiple SKILL.md files)
Clean .prompt.md rename: Simplified conditional for files already correctly named
Install counter: Cached/plugin packages now included in installed_count
Package type in lockfile: LockedDependency persists package_type for accurate metadata
MARKETPLACE_PLUGIN detection: Cached-path now detects plugins (not just APM/skill/hybrid)
Agent scatter: Picks up plain .md files in .apm/agents/ (excludes README, CHANGELOG, etc.)
deps info sub-path: Resolution for virtual packages
Lockfile merge: Preserves entries from sequential installs; guarded for --update
Hooks support: File path ("hooks": "hooks.json"), inline object, and directory forms

Documentation

README.md: Plugin as supported source, awesome-copilot example, Plugins docs link
CHANGELOG.md: Unreleased entry for full plugin support
docs/manifest-schema.md: type field docs reflect content-driven default
docs/dependencies.md: Marketplace Plugin row in dependency types table
docs/primitives.md: Plugins in primitives overview
docs/plugins.md: Fixed priority order, custom component paths section, corrected required fields

Tests

E2E tests (test_plugin_e2e.py): Hero scenario + lockfile preservation (473+ lines)
Unit tests (test_plugin_parser.py): Comprehensive coverage (37 tests) — custom paths, file paths, mixed paths, hooks variants
Validation test fixes: Updated assertions for new validation behavior

Not in this PR (waiting for #201)

MCP/LSP deployment to runtime (should-fix Refactor apm init to minimal-only mode (breaking change) #18)
Inline mcpServers/hooks definitions in plugin.json (should-fix feat: add support for Awesome Copilot Collections as APM packages #22)

These require the MCP installation pipeline from PR #201 before they can be implemented.

Test Results

1692 passed, 92 skipped

- plugin_parser: skip symlinks entirely instead of dereferencing them (symlinks=False still copies target content, enabling exfiltration) - apm_package: require plugin.json or component dirs for MARKETPLACE_PLUGIN (empty/README-only directories no longer auto-classify as plugins) - agent_integrator: pick up plain .md files in .apm/agents/ (plugins may not use .agent.md convention) - deps info: resolve virtual sub-path packages at any directory depth - cli: preserve existing lockfile entries on sequential apm install

…tests - E2E hero scenario tests covering install/compile/deps/info/uninstall - E2E lockfile preservation test for sequential installs - Comprehensive plugin_parser unit tests (manifest parsing, artifact mapping, symlink skip, apm.yml generation, normalization) - Fix test_apm_package_models to match new validation behavior - Extract test_plugin_integrator_deployment as proper test method

- Correct priority: root > .github/plugin/ > .claude-plugin/ - Remove Legacy APM Format section (plugins/ directory is not checked) - Fix required fields: only name is required per the Claude plugin spec - Fix troubleshooting section to match

Copilot

Pull request overview

This PR is a hardening follow-up to #83 (plugin management system). It fixes security, validation, and behavioral bugs introduced in #83, adds a comprehensive test suite (800+ lines), and corrects documentation.

Changes:

Security: Replaced symlinks=False with an _ignore_symlinks ignore callback in plugin_parser.py to skip symlinks entirely during copytree, preventing content exfiltration
Validation hardening: Empty/README-only directories no longer auto-classify as MARKETPLACE_PLUGIN; plugin.json or a standard component directory is now required
Bug fixes: Agent .md discovery, deep virtual sub-path resolution in deps info, and lockfile preservation across sequential installs

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`src/apm_cli/deps/plugin_parser.py`	Replaces `symlinks=False` with `_ignore_symlinks` callback to skip symlinks, adds `Symlinks are skipped` docstring note
`src/apm_cli/models/apm_package.py`	Requires `plugin.json` or component dirs to classify as `MARKETPLACE_PLUGIN`; empty dirs now return invalid
`src/apm_cli/integration/agent_integrator.py`	Picks up plain `.md` files in `.apm/agents/` for plugins that don't use `.agent.md` naming
`src/apm_cli/commands/deps.py`	Adds direct-path match for deep virtual sub-path packages before 2-level fallback scan
`src/apm_cli/cli.py`	Merges existing lockfile entries when only a subset of packages is installed
`tests/unit/test_plugin_parser.py`	New unit tests covering manifest parsing, artifact mapping, symlink skip, apm.yml generation, normalization, validation
`tests/test_apm_package_models.py`	Updates expected behavior to match new validation (empty dirs are invalid)
`tests/integration/test_plugin_e2e.py`	New E2E integration tests covering the full plugin lifecycle, lockfile preservation, symlink safety
`tests/integration/test_marketplace_plugin_integration.py`	Extracts previously orphaned test content into `test_plugin_integrator_deployment` method
`docs/plugins.md`	Corrects `plugin.json` priority order, removes non-existent legacy format, fixes required fields

tests/unit/test_plugin_parser.py

tests/integration/test_plugin_e2e.py

src/apm_cli/cli.py

- Dynamic package type in synthesized apm.yml based on actual components (#12) - Name validation warning in parse_plugin_manifest (#13) - Clean .prompt.md rename logic (#14) - Install counter includes cached packages (#15) - Package type persisted in lockfile (#17) - Custom component path arrays from plugin.json (#23) - README: plugin as supported source, awesome-copilot example, docs link - CHANGELOG: unreleased entry for plugin support - docs: MARKETPLACE_PLUGIN in manifest-schema, dependencies, primitives

- Remove dead type-detection logic in plugin_parser.py: the computed type field was never read by the install pipeline (get_effective_type routes on file presence, not the apm.yml type field) - Hardcode type: hybrid in generated apm.yml with explanatory comment - Remove unused pkg_type parameter from _generate_apm_yml() - Fix manifest-schema.md: remove invalid marketplace_plugin from type enum, correct default to hybrid, note field is reserved for future use New E2E tests (local, no network): - test_compile_discovers_plugin_primitives - test_lockfile_package_type_roundtrip - test_generated_apm_yml_type_is_hybrid New E2E tests (network, requires GITHUB_TOKEN): - test_compile_includes_plugin_primitives - test_prune_removes_orphaned_plugin - test_install_counter_includes_plugin - test_lockfile_records_package_type - test_idempotent_reinstall

- Fix symlink test: assert copied_linked does not exist (not a conditional check). Update comments to describe _ignore_symlinks callback behavior instead of the old symlinks=False approach. - Fix deps_info test: exercise the same direct_match lookup pattern that deps info() uses, not just filesystem + from_apm_yml(). - Fix lockfile merge bug: guard merge with `not update_refs` so stale entries are not preserved when user runs `apm install --update`.

The plugin spec allows hooks to be a directory path, a config file path, or an inline JSON object. Previously only directory paths were handled. - Add file-path handling: "hooks": "hooks.json" copies file to .apm/hooks/hooks.json - Add inline-object handling: "hooks": {...} serializes to .apm/hooks/hooks.json - Directory path still works as before (including custom paths) Tests: custom agents string path, skills array merge, commands path, hooks file path, hooks inline object, hooks directory path, nonexistent custom paths silently ignored.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/apm_cli/cli.py:1975

The cached package type detection at lines 1968-1975 sets MARKETPLACE_PLUGIN only when plugin_json_exists and not apm_yml_exists. However, after validate_apm_package has run and synthesized an apm.yml for the plugin, the condition not apm_yml_exists is False, so the type falls through to APM_PACKAGE (line 1974). This means that on a second install (cached), the lockfile package_type for plugin packages will be overwritten as apm_package instead of marketplace_plugin, defeating the purpose of tracking the package_type. The detection should also check for plugin.json when apm.yml exists (to handle normalized plugins), or rely on the existing lockfile's stored package_type as a fallback.

                            from apm_cli.utils.helpers import find_plugin_json
                            plugin_json_exists = find_plugin_json(install_path) is not None
                            if plugin_json_exists and not apm_yml_exists:
                                cached_package_info.package_type = PackageType.MARKETPLACE_PLUGIN
                            elif skill_md_exists and apm_yml_exists:
                                cached_package_info.package_type = PackageType.HYBRID
                            elif skill_md_exists:
                                cached_package_info.package_type = PackageType.CLAUDE_SKILL
                            elif apm_yml_exists:
                                cached_package_info.package_type = PackageType.APM_PACKAGE

src/apm_cli/deps/plugin_parser.py

src/apm_cli/integration/agent_integrator.py

docs/manifest-schema.md

src/apm_cli/deps/plugin_parser.py

…n.json - _resolve_sources() now accepts both files and directories (not just dirs) - Array of subdirectories preserves dir names to avoid SKILL.md overwrites - Commands mapping handles individual file paths correctly - Added 4 unit tests for file path, mixed path, and subdirectory scenarios - Updated docs/plugins.md with custom component paths documentation

- Move 'import logging' to module level (plugin_parser.py) - Exclude README.md/CHANGELOG.md/etc from .apm/agents/ plain .md scan - Restore content-driven default for 'type' field in manifest-schema.md - Update PR description to reflect intentional hybrid hardcode - Add test for non-agent .md exclusion in agent_integrator

danielmeppiel added 3 commits March 9, 2026 15:10

Copilot AI review requested due to automatic review settings March 9, 2026 14:11

Copilot started reviewing on behalf of danielmeppiel March 9, 2026 14:12 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

tests/unit/test_plugin_parser.py Outdated Show resolved Hide resolved

tests/integration/test_plugin_e2e.py Show resolved Hide resolved

src/apm_cli/cli.py Show resolved Hide resolved

danielmeppiel added 4 commits March 9, 2026 15:30

danielmeppiel requested a review from Copilot March 9, 2026 15:55

Copilot started reviewing on behalf of danielmeppiel March 9, 2026 15:56 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

danielmeppiel added 2 commits March 9, 2026 17:20

danielmeppiel requested a review from SebastienDegodez March 9, 2026 16:32

chore: update changelog to remove unreleased plugin features and fixes

b4b1b67

danielmeppiel requested a review from sergio-sisternes-epam March 9, 2026 16:47

Merge branch 'main' into fix/plugin-hardening

0d5f0fc

danielmeppiel merged commit b0aa71b into main Mar 9, 2026
7 checks passed

danielmeppiel deleted the fix/plugin-hardening branch March 9, 2026 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: harden plugin security, validation, tests, and docs (follow-up to #83)#208

fix: harden plugin security, validation, tests, and docs (follow-up to #83)#208
danielmeppiel merged 11 commits intomainfrom
fix/plugin-hardening

danielmeppiel commented Mar 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

danielmeppiel commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Plugin Support Hardening

Security & Validation

Code Fixes

Documentation

Tests

Not in this PR (waiting for #201)

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danielmeppiel commented Mar 9, 2026 •

edited

Loading