- Narrow except Exception to except ImportError for lazy marketplace import (comment #1)
- Fix provenance key mismatch: use dep identity instead of canonical for lockfile lookup (comment #2)
- Include subdir in git-subdir source resolution with path traversal validation (comment #3)
- Include relative path in relative source resolution with traversal validation (comment #4)
- Sanitize marketplace name in cache file paths to prevent path traversal (comment #5)
- Fix docs: stale-if-error, not stale-while-revalidate (comment #6)
- Consolidate CHANGELOG entries into single line with (#503) (comment #7)
- Remove unused _SUPPORTED_SOURCE_TYPES set (comment #8)
- Let auth errors propagate in _auto_detect_path instead of swallowing (comment #9)
- Validate marketplace --name against [a-zA-Z0-9._-]+ charset (comment #10)
- Fix doc examples to use identifier-compatible names (comments #11, #12)
- Update tests to match corrected resolver behavior, add traversal tests

Co-authored-by: Copilot <[email protected]>

…covery + governance (#503)

* Initial plan

* Initial plan for marketplace integration

Agent-Logs-Url: https://github.com/microsoft/apm/sessions/12a9b016-7930-41b8-a340-c64f11486b71

Co-authored-by: danielmeppiel <[email protected]>

* feat: marketplace integration core implementation

- Add marketplace/ package: models, errors, registry, client, resolver
- Add marketplace CLI commands: add, list, browse, update, remove, search
- Add lockfile provenance fields: discovered_via, marketplace_plugin_name
- Add install hook for NAME@MARKETPLACE syntax pre-parse intercept
- Wire marketplace commands in cli.py

Agent-Logs-Url: https://github.com/microsoft/apm/sessions/12a9b016-7930-41b8-a340-c64f11486b71

Co-authored-by: danielmeppiel <[email protected]>

* docs: add marketplace integration guide and CLI reference

- Create guides/marketplaces.md covering marketplace concepts,
  registration, browsing, search, install syntax, provenance tracking,
  and cache behavior
- Add apm marketplace and apm search command sections to cli-commands.md
- Update apm install arguments to include NAME@MARKETPLACE syntax
- Update plugins.md Finding Plugins section with marketplace cross-refs

Co-authored-by: danielmeppiel <[email protected]>

* docs: fix marketplace.json format and lockfile field names to match implementation

- Use array-based plugins format matching models.py parser expectations
- Use discovered_via and marketplace_plugin_name matching lockfile.py fields
- Document both Copilot CLI (repository/ref) and Claude Code (source) formats

Co-authored-by: danielmeppiel <[email protected]>

* docs: fix git-subdir and relative source descriptions to match resolver

- git-subdir uses separate repo and subdir fields
- Relative string sources resolve to marketplace repo subdirectory

Co-authored-by: danielmeppiel <[email protected]>

* feat: add marketplace unit tests and docs

- 114 unit tests across 8 test files covering all marketplace modules
- New marketplace guide at docs/src/content/docs/guides/marketplaces.md
- Updated CLI reference with marketplace and search commands
- Updated plugins guide with marketplace integration section
- CHANGELOG entry for marketplace feature

Agent-Logs-Url: https://github.com/microsoft/apm/sessions/12a9b016-7930-41b8-a340-c64f11486b71

Co-authored-by: danielmeppiel <[email protected]>

* refactor: address code review feedback

- Use List[MarketplacePlugin] from typing instead of lowercase generic
- Eliminate duplicated condition in install.py marketplace intercept
- Restructure control flow for clarity

Agent-Logs-Url: https://github.com/microsoft/apm/sessions/12a9b016-7930-41b8-a340-c64f11486b71

Co-authored-by: danielmeppiel <[email protected]>

* fix: address all 12 PR review comments on marketplace integration

- Narrow except Exception to except ImportError for lazy marketplace import (comment #1)
- Fix provenance key mismatch: use dep identity instead of canonical for lockfile lookup (comment #2)
- Include subdir in git-subdir source resolution with path traversal validation (comment #3)
- Include relative path in relative source resolution with traversal validation (comment #4)
- Sanitize marketplace name in cache file paths to prevent path traversal (comment #5)
- Fix docs: stale-if-error, not stale-while-revalidate (comment #6)
- Consolidate CHANGELOG entries into single line with (#503) (comment #7)
- Remove unused _SUPPORTED_SOURCE_TYPES set (comment #8)
- Let auth errors propagate in _auto_detect_path instead of swallowing (comment #9)
- Validate marketplace --name against [a-zA-Z0-9._-]+ charset (comment #10)
- Fix doc examples to use identifier-compatible names (comments #11, #12)
- Update tests to match corrected resolver behavior, add traversal tests

Co-authored-by: Copilot <[email protected]>

* fix: Copilot CLI format compatibility and marketplace provenance bugs

Bug #1 - Format incompatibility with awesome-copilot marketplace:
  - Parser now accepts 'source' key (Copilot CLI) as type discriminator
    fallback when 'type' key is absent, normalizing to 'type' for resolvers
  - GitHub source resolver now accepts 'path' field (Copilot CLI) as
    virtual subdirectory, same as 'subdir' in git-subdir sources
  - Path traversal validation applied to 'path' field
  - Fixes: 8 of 62 plugins in awesome-copilot that use github source
    objects with 'source'+'path' keys instead of 'type'+'subdir'

Bug #2 - Lockfile provenance never written:
  - Root cause: install passed raw marketplace refs (NAME@MARKETPLACE)
    as only_packages, but DependencyReference.parse() can't parse those,
    so identity filtering removed all deps -> 'already installed'
  - Fix: use validated_packages (canonical owner/repo strings) instead
    of raw click argument for only_pkgs

Both bugs verified fixed via E2E tests against real marketplaces:
  - github/awesome-copilot (62 plugins)
  - anthropics/skills (3 plugins)
  - microsoft/azure-skills (1 plugin)

Co-authored-by: Copilot <[email protected]>

* feat: scope marketplace search to QUERY@MARKETPLACE format

Search now requires QUERY@MARKETPLACE (e.g. apm search security@skills)
to eliminate name collisions across marketplaces. Added search_marketplace()
client function for single-marketplace search.

- Rejects bare queries without @ — clear error with usage example
- Validates marketplace exists before searching
- Updated docs/guides/marketplaces.md with new syntax
- 7 test cases: format validation, unknown marketplace, results, no results

Co-authored-by: Copilot <[email protected]>

* docs: update CLI reference and plugins guide for scoped search syntax

Align all documentation with QUERY@MARKETPLACE search format.

Co-authored-by: Copilot <[email protected]>

* refactor: use centralized path_security for marketplace traversal checks

Replace 3 ad-hoc '..' in x.split('/') checks in marketplace/resolver.py
with validate_path_segments() from utils/path_security.py. Add
defense-in-depth validate_path_segments() call to _sanitize_cache_name()
in client.py.

This ensures marketplace code uses the same cross-platform path safety
utilities (backslash normalization, single-dot rejection) as the rest
of APM.

Co-authored-by: Copilot <[email protected]>

* docs: add path safety rule to copilot-instructions.md

Directs contributors to use validate_path_segments() and
ensure_path_within() from utils/path_security.py instead of
ad-hoc traversal checks.

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: danielmeppiel <[email protected]>
Co-authored-by: danielmeppiel <[email protected]>
Co-authored-by: Copilot <[email protected]>

Round-2 apm-review-panel findings on PR microsoft#700. Five specialists converged
on APPROVE with minor nits; this commit applies the actionable ones.

N1 (cli-logging): fix double-URL rendering in the validation-time path.
`_format_insecure_dependency_requirements` already embeds the full URL in
the reason string, but `logger.validation_fail(package, reason)` prepends
"{package} -- " and produced output like
"[x] http://host/repo -- http://host/repo -- HTTP dependency...". Switch
to `logger.error(reason)` on the insecure-reject branch to match the
other two failure paths. `invalid_outcomes` still tracks the package so
the later validation summary sees it.

N2 (cli-logging): unify jargon register. Warning copy already says
"(unencrypted)" post-review microsoft#11; align the error recipe from
"(no transport encryption)" to "(unencrypted)" so both surfaces use the
same wording.

N3 (devx): context-aware remediation steps. `_check_insecure_dependencies`
previously emitted the full two-step recipe in both failure branches,
even when the user had already performed step 1 (manifest edit). The
factory now takes `missing_dep_allow` and `missing_cli_flag` keyword
args and only renders the missing step(s). The add-time validation
path keeps the default of both-steps since the dep is not yet in
apm.yml. Tests updated to match the new per-branch assertions.

N4 (security): document the intentional two-stage credential-helper
policy in `_build_noninteractive_git_env`. The pop-then-conditionally-
restore of GIT_ASKPASS is correct but fragile; an explicit docstring
prevents a future refactor from inverting the logic and leaking
credentials over plaintext HTTP or blocking system keychains on
HTTPS/SSH fallback.

N5 (doc-writer): add `apm deps list --insecure` sample output to
cli-commands.md showing the bold-red `Origin` column with `direct`
and `via <parent>` example rows.

F2 (doc-writer): document `is_insecure` and `allow_insecure` fields in
lockfile-spec.md section 4.2, including replay semantics and legacy
fail-closed behaviour.

F3 (doc-writer): add "HTTP dependencies (opt-in)" section to the
apm-guide dependencies.md skill resource so LLM agents consuming the
skill have the dual-opt-in model, example manifest, example CLI
invocation, and cross-references to commands.md and the enterprise
security guide.

Full unit suite: 4767/4767 passing (one deselect unrelated).
`install.py` at 1497 LOC (under the 1525-LOC invariant).

Panel verdict: microsoft#700 (comment 4290979672)

Co-authored-by: arika0093 <[email protected]>
Co-authored-by: Copilot <[email protected]>

* feat: support allow-insecure HTTP dependencies (review item 14 stack split)

Co-authored-by: Copilot <[email protected]>

* fix: address Sergio review on insecure HTTP dependencies (review item 14 stack split)

Co-authored-by: Copilot <[email protected]>

* feat: tighten insecure HTTP dependency safeguards (review item 14 stack split)

Co-authored-by: Copilot <[email protected]>

* feat: gate transitive insecure dependencies by host (review item 14 stack split)

Co-authored-by: Copilot <[email protected]>

* feat: refine HTTP dependency transport and safeguards (review item 14 stack split)

Co-authored-by: Copilot <[email protected]>

* refactor(config): share config command boolean helpers (review item 14 split)

Co-authored-by: Copilot <[email protected]>

* feat: refine HTTP clone and validation transport behavior (review item 14 stack split)

Co-authored-by: Copilot <[email protected]>

* fix(security): suppress credential helpers on HTTP git attempts (review item 1)

Co-authored-by: Copilot <[email protected]>

* fix(drift): treat HTTP transport flips as dependency drift (review item 2)

Co-authored-by: Copilot <[email protected]>

* refactor(install): extract insecure dependency policy module (review item 3)

Co-authored-by: Copilot <[email protected]>

* fix(lockfile): replay allow_insecure for HTTP dependencies (review item 4)

Co-authored-by: Copilot <[email protected]>

* fix(reference): keep canonical dependency strings scheme-free (review item 5)

Co-authored-by: Copilot <[email protected]>

* refactor(lockfile): centralize locked dependency reference mapping (review item 6)

Co-authored-by: Copilot <[email protected]>

* fix(install): unify HTTP dependency remediation errors (review item 7)

Co-authored-by: Copilot <[email protected]>

* docs(security): document HTTP dependency threat model (review item 8)

Co-authored-by: Copilot <[email protected]>

* refactor(install): require logger for insecure policy diagnostics (review item 9)

Co-authored-by: Copilot <[email protected]>

* refactor(install): raise policy errors instead of exiting (review item 10)

Co-authored-by: Copilot <[email protected]>

* fix(install): tighten insecure HTTP warning copy (review item 11)

Co-authored-by: Copilot <[email protected]>

* fix(install): shorten transitive HTTP remediation error (review item 12)

Co-authored-by: Copilot <[email protected]>

* fix(deps): rename insecure list origin column (review item 13)

Co-authored-by: Copilot <[email protected]>

* review: address panel round-2 nits (N1-N5, F2, F3)

Round-2 apm-review-panel findings on PR #700. Five specialists converged
on APPROVE with minor nits; this commit applies the actionable ones.

N1 (cli-logging): fix double-URL rendering in the validation-time path.
`_format_insecure_dependency_requirements` already embeds the full URL in
the reason string, but `logger.validation_fail(package, reason)` prepends
"{package} -- " and produced output like
"[x] http://host/repo -- http://host/repo -- HTTP dependency...". Switch
to `logger.error(reason)` on the insecure-reject branch to match the
other two failure paths. `invalid_outcomes` still tracks the package so
the later validation summary sees it.

N2 (cli-logging): unify jargon register. Warning copy already says
"(unencrypted)" post-review #11; align the error recipe from
"(no transport encryption)" to "(unencrypted)" so both surfaces use the
same wording.

N3 (devx): context-aware remediation steps. `_check_insecure_dependencies`
previously emitted the full two-step recipe in both failure branches,
even when the user had already performed step 1 (manifest edit). The
factory now takes `missing_dep_allow` and `missing_cli_flag` keyword
args and only renders the missing step(s). The add-time validation
path keeps the default of both-steps since the dep is not yet in
apm.yml. Tests updated to match the new per-branch assertions.

N4 (security): document the intentional two-stage credential-helper
policy in `_build_noninteractive_git_env`. The pop-then-conditionally-
restore of GIT_ASKPASS is correct but fragile; an explicit docstring
prevents a future refactor from inverting the logic and leaking
credentials over plaintext HTTP or blocking system keychains on
HTTPS/SSH fallback.

N5 (doc-writer): add `apm deps list --insecure` sample output to
cli-commands.md showing the bold-red `Origin` column with `direct`
and `via <parent>` example rows.

F2 (doc-writer): document `is_insecure` and `allow_insecure` fields in
lockfile-spec.md section 4.2, including replay semantics and legacy
fail-closed behaviour.

F3 (doc-writer): add "HTTP dependencies (opt-in)" section to the
apm-guide dependencies.md skill resource so LLM agents consuming the
skill have the dual-opt-in model, example manifest, example CLI
invocation, and cross-references to commands.md and the enterprise
security guide.

Full unit suite: 4767/4767 passing (one deselect unrelated).
`install.py` at 1497 LOC (under the 1525-LOC invariant).

Panel verdict: #700 (comment 4290979672)

Co-authored-by: arika0093 <[email protected]>
Co-authored-by: Copilot <[email protected]>

* review: fix CodeQL "Incomplete URL substring sanitization" finding

CodeQL flagged `"http://gitlab.company.internal" in arg` at
test_install_command.py:963 as a potentially unsafe URL substring check:
the hostname could appear at an arbitrary position in the checked URL
(e.g. inside a path segment, query value, or userinfo) rather than as
the actual host, producing a false-positive match.

Replace substring checks with `urllib.parse.urlsplit` and compare
`scheme` and `netloc` explicitly:

- test_explicit_http_generic_host_tries_http_first (the flagged case):
  parse each subprocess-arg URL and require scheme == "http" AND
  netloc == "gitlab.company.internal".
- test_generic_host_falls_back_to_https_when_ssh_fails (sibling with
  the same pattern at line 929): same treatment for the HTTPS arg;
  keep the SSH SCP-form check as `"[email protected]:" in arg` since
  SCP-style URLs are not parseable by urlsplit.

Both tests still assert the same contract: explicit `http://` does not
fall back to SSH, and SSH failure falls back to HTTPS on the right host.

Co-authored-by: arika0093 <[email protected]>
Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>
Co-authored-by: danielmeppiel <[email protected]>
Co-authored-by: arika0093 <[email protected]>
Co-authored-by: Daniel Meppiel <[email protected]>