Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a65d7e2
feat(cli): add 'apm mcp install' alias for 'apm install --mcp' (#807)
danielmeppiel Apr 21, 2026
12f6bc7
feat(mcp): harden MCPDependency.validate() with security checks (#807)
danielmeppiel Apr 21, 2026
7904d2a
feat(install): add --mcp flag for declaratively adding MCP servers to…
danielmeppiel Apr 21, 2026
385795a
docs: add MCP Servers guide consolidating the apm install --mcp workf…
danielmeppiel Apr 21, 2026
834f0f4
docs: document MCP_REGISTRY_URL for custom MCP registries
danielmeppiel Apr 21, 2026
325ae60
Merge branch 'main' into feat/install-mcp-flag
danielmeppiel Apr 21, 2026
a4b3d21
fix(mcp): honour MCP_REGISTRY_URL in search/list/show; diagnose regis…
danielmeppiel Apr 21, 2026
257f4c8
docs(readme): showcase MCP as a first-class primitive
danielmeppiel Apr 21, 2026
8364cba
security(mcp): validate MCP_REGISTRY_URL and fail-closed on overrides…
danielmeppiel Apr 21, 2026
32d62b0
docs(mcp): clarify 'transport: http' is an MCP transport name, not a …
danielmeppiel Apr 21, 2026
872dad3
fix(mcp): address PR #810 panel review (UX, architecture, security)
danielmeppiel Apr 21, 2026
e4b5185
fix(tests): use urllib.parse for URL assertions to clear CodeQL alerts
danielmeppiel Apr 21, 2026
621804b
feat(install): add --registry URL flag for custom MCP registries (#810)
danielmeppiel Apr 21, 2026
2f04861
fix(mcp): apply panel-review blockers (B1-B7) on PR #810
danielmeppiel Apr 21, 2026
c78ea0b
fix(mcp): chaos-report findings + CodeQL on PR #810
danielmeppiel Apr 21, 2026
e23fb21
fix(tests): use urlparse hostname equality + add tests instructions
danielmeppiel Apr 21, 2026
a7c9206
docs(install): point LOC budget violators at python-architecture skill
danielmeppiel Apr 21, 2026
3f97f94
chore: untrack server.pid (added by mistake)
danielmeppiel Apr 21, 2026
a4a5e96
docs: fidelity audit follow-up on PR #810 (doc-writer)
danielmeppiel Apr 21, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 109 additions & 0 deletions .github/instructions/tests.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
---
applyTo: "tests/**"
description: "Test conventions: URL assertions must use urllib.parse, never substring."
---

# Test Conventions

## URL assertions: use `urllib.parse`, never substring

Any assertion that a URL appears in or matches some output **must** parse the
URL with `urllib.parse.urlparse` and compare on a parsed component
(`hostname`, `port`, `scheme`, `path`). Substring assertions like
`assert "host.example.com" in msg` or `assert "https://x" in url` are flagged
by CodeQL as `py/incomplete-url-substring-sanitization` (high severity, "the
string may be at an arbitrary position in the URL") and **will fail CI**.

This rule applies regardless of whether the value being asserted looks like a
"safe" hostname — CodeQL is a static check and cannot infer that `host` in
`assert host in msg` is bounded; the alert fires anyway.

### Wrong

```python
# Substring match -- CodeQL py/incomplete-url-substring-sanitization
assert "registry.example.com" in msg
assert "https://api.github.com/v0/servers" in url
assert "127.0.0.1" in warning_text

# Set membership of substring -- still flagged (CodeQL can't infer set type)
hosts = {urlparse(tok).hostname for tok in msg.split() if "://" in tok}
assert "poisoned.example.com" in hosts
```

### Right

```python
from urllib.parse import urlparse

# Direct hostname equality on a parsed URL token
urls = [tok for tok in msg.split() if "://" in tok]
assert len(urls) == 1
assert urlparse(urls[0]).hostname == "registry.example.com"

# Set equality (not membership) when multiple URLs are expected
hosts = {urlparse(tok.strip("()")).hostname for tok in msg.split() if "://" in tok}
assert hosts == {"a.example.com", "b.example.com"}

# Component-level checks for path / scheme / port
parsed = urlparse(url)
assert parsed.scheme == "https"
assert parsed.hostname == "api.github.com"
assert parsed.path == "/v0/servers"
```

### Helper pattern for multi-URL output

When asserting against logger / CLI output that may contain multiple URLs,
extract them with a small helper and assert on the parsed tuple:

```python
def _printed_urls(text: str) -> list[tuple[str, str, str]]:
"""Extract (scheme, hostname, path) tuples from any URLs in text."""
from urllib.parse import urlparse
out = []
for token in text.split():
cleaned = token.strip("(),.;'\"")
if "://" not in cleaned:
continue
p = urlparse(cleaned)
out.append((p.scheme, p.hostname or "", p.path))
return out

assert ("https", "registry.example.com", "/v0/servers") in _printed_urls(msg)
```

`tests/unit/test_mcp_command.py` already uses this pattern; reuse it (or
copy it) rather than inventing a new substring check.

## Why the rule applies even to "obviously safe" tests

The CodeQL rule is intentionally conservative: a substring assertion against a
URL string is the same code shape as a security-critical sanitizer check, and
the analyzer cannot tell them apart. Treating every URL assertion uniformly
through `urlparse` keeps CI green AND reinforces the security pattern that
production code must follow (see
`src/apm_cli/install/mcp_registry.py::_redact_url_credentials` and
`src/apm_cli/install/mcp_registry.py::_is_local_or_metadata_host`).

## Other rules

- **No live network calls.** Tests must never hit a real HTTP endpoint; use
`unittest.mock.patch('requests.Session.get')` or
`monkeypatch.setattr(client.session, "get", fake)`. Live-inference tests
are isolated to `ci-runtime.yml` and gated by `APM_RUN_INFERENCE_TESTS=1`.

- **Patch where the name is looked up.** When a function moved to
`apm_cli/install/phases/X.py` is still patched by tests at
`apm_cli.commands.install.X`, the patch silently no-ops. Either patch at
the new canonical path, or use module-attribute access in the call site
(`X_mod.function`) so canonical patches survive the move. See
`src/apm_cli/install/phases/integrate.py:888` for the pattern.

- **Reuse existing fixtures.** Common fixtures live in `tests/conftest.py`
and `tests/unit/install/conftest.py`. Don't re-implement temp-dir or
mock-logger fixtures inline.

- **Targeted runs during iteration.** Run the specific test file first
(`uv run pytest tests/unit/install/test_X.py -x`) before running the
full suite (`uv run pytest tests/unit tests/test_console.py`).
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,4 @@ build/tmp/
scout-pipeline-result.png
.copilot/
.playwright-mcp/
server.pid
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed (BREAKING)

- MCP entry validation hardened (security): names must match `^[a-zA-Z0-9@_][a-zA-Z0-9._@/:=-]{0,127}$`, URLs must use `http` or `https` schemes, headers reject CR/LF in keys and values, self-defined stdio commands rejected if they contain path-traversal sequences. Migration: most existing `apm.yml` files unaffected; if you hit `Invalid MCP name`, the error message now includes a valid positive example (e.g. `io.github.acme/cool-server` or `my-server`) to pattern against. (#807)
- Strict-by-default transport selection: explicit `ssh://`/`https://` URLs no longer silently fall back to the other protocol; shorthand consults `git config url.<base>.insteadOf` and otherwise defaults to HTTPS. Set `APM_ALLOW_PROTOCOL_FALLBACK=1` (or pass `--allow-protocol-fallback`) to restore the legacy permissive chain; cross-protocol retries then emit a `[!]` warning. Closes #328 (#778)

### Changed
Expand All @@ -18,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- `apm install --mcp NAME [--transport ...] [--url ...] [--env K=V] [--header K=V] [--mcp-version V] [-- COMMAND ARGS...]` flag for declaratively adding MCP servers to `apm.yml`. Mirrors `apm install` for packages: validates input through `MCPDependency`, writes to `dependencies.mcp` (or `devDependencies.mcp` with `--dev`), and integrates the new server into client adapters. Idempotency policy: in a TTY, prompts before replacing an existing entry; in CI, requires `--force`. Also accessible via `apm mcp install` alias for discoverability. Closes #807 (#810)
- `apm install --mcp NAME --registry URL` flag for resolving registry-form MCP servers against a custom (e.g. private/enterprise) MCP registry. Precedence chain: CLI flag > `MCP_REGISTRY_URL` env var > default (`https://api.mcp.github.com`). The URL is validated (`http`/`https` only; `ws://`, `file://`, `javascript:`, schemeless and >2048-char values rejected with usage errors), captured in `apm.yml` on the entry's `registry:` field for auditability, and applied to the install session via the registry resolver. Both `http://` and `https://` are accepted via the CLI flag (explicit per-invocation user intent); the env-var path keeps the stricter `https`-by-default policy with `MCP_REGISTRY_ALLOW_HTTP=1` opt-in. Forwards transparently through the `apm mcp install` alias. Conflicts with `--url` or a stdio command (self-defined entries do not consult a registry). Per-project default via `apm config set mcp-registry-url` is tracked as a follow-up (#818). (#810)
- New **MCP Servers** guide (`docs/guides/mcp-servers.md`) consolidating the `apm install --mcp` workflow: stdio / registry / remote shapes, full flag reference, validation rules, and the curated conflict matrix in one page (#808). Sidebar entry added under Guides. Documents the `--mcp` / `--transport` / `--url` / `--env` / `--header` / `--mcp-version` flag family and the `apm mcp install` alias in `reference/cli-commands.md`. Documents the `MCP_REGISTRY_URL` environment variable for pointing `apm install --mcp` at a custom MCP registry (enterprise). Drift fixes in the same PR: removes the stale "Phase 2 - Coming Soon" MCP section in `guides/prompts.md`, fixes a broken `apm mcp info` reference in `integrations/ide-tool-integration.md`, replaces an emoji compatibility table with ASCII, and aligns MCP registry-name examples on the canonical `io.github.github/...` form across `key-concepts.md` and `ide-tool-integration.md`.
- `apm install --ssh` / `--https` flags and `APM_GIT_PROTOCOL=ssh|https` env to pick the initial transport for shorthand dependencies (#778)
- `apm install --allow-protocol-fallback` flag and `APM_ALLOW_PROTOCOL_FALLBACK=1` env as the migration escape hatch for cross-protocol fallback (#778)
- Add APM Review Panel skill (`.github/skills/apm-review-panel/`) and four new specialist personas (`devx-ux-expert`, `supply-chain-security-expert`, `apm-ceo`, `oss-growth-hacker`) with auto-activating per-persona skills. Routes specialist findings through an APM CEO arbiter for strategic / breaking-change calls, with the OSS growth hacker side-channeling adoption insights via `WIP/growth-strategy.md`. Instrumentation per Handbook Ch. 9 (`The Instrumented Codebase`); PROSE-compliant (thin SKILL.md routers, persona detail lazy-loaded via markdown links, explicit boundaries per persona).
Expand All @@ -39,6 +43,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `--trust-transitive-mcp` no longer silently ignored when combined with `--global` (#638)
- Token resolution now discriminates by port, fixing credential collisions across multiple self-hosted Git instances on the same host. Thanks @edenfunf! (#785)
- Fix `apm init` showing overwrite confirmation prompt three times on Windows CP950 terminals (#602)
- `apm mcp search`, `apm mcp list`, and `apm mcp show` now honour the `MCP_REGISTRY_URL` environment variable (previously hardcoded to the public registry), bringing them in line with `apm install --mcp`. When the variable is set, the discovery commands print a one-line `Registry: <url>` diagnostic and surface the configured URL in network-error messages so misconfigured enterprise registries are obvious (#813)
- `apm install --mcp ... --dry-run` now validates the would-be entry through the same chokepoint the real install uses, so dry-run never previews "success" for an entry `apm install` would reject (empty / whitespace-only / over-128-char / embedded `..` names, invalid transport-conflict combinations) (#810)
- `SimpleRegistryClient` now applies a `(connect=10s, read=30s)` timeout on every registry HTTP call, removing the unbounded-hang failure mode when a registry is slow or unreachable. Operators can tune via `MCP_REGISTRY_CONNECT_TIMEOUT` / `MCP_REGISTRY_READ_TIMEOUT` env vars; invalid values silently fall back to defaults (#810)

### Security

- `MCP_REGISTRY_URL` is now validated at startup: schemeless values, empty strings, and unsupported schemes are rejected with actionable errors. Plaintext `http://` is rejected by default; opt in with `MCP_REGISTRY_ALLOW_HTTP=1` for development or air-gapped intranets. When a custom registry is set and unreachable during install pre-flight, APM now fails closed instead of silently assuming all MCP dependencies are valid -- this prevents a misconfigured or down enterprise registry from quietly approving every server. The default registry (`https://api.mcp.github.com`) keeps the existing assume-valid behaviour for transient errors so unrelated network blips do not block installs (#814)
- MCP dependency names reject embedded `..` path segments (e.g. `a/../../../evil`) at the `MCPDependency.validate()` chokepoint as defense-in-depth on top of the allowlist regex; the rejection error now includes a valid positive example (`io.github.acme/cool-server` or `my-server`) instead of a suggestion that often produced still-invalid names (#810)
- URLs in `apm install --mcp` diagnostic output route through a urlparse-based credential redactor, so `https://user:token@host/` renders as `https://host/` in log messages and error text; prevents token leakage to CI logs and terminal scrollback (#810)
- `--registry` / `MCP_REGISTRY_URL` values pointing at loopback, link-local, RFC1918, or cloud-metadata hosts (including decimal-encoded loopback like `http://2130706433/`) now emit an informational `[!]` warning. Defense-in-depth signal on top of the existing allowlist -- does not block the request (#810)

## [0.8.12] - 2026-04-19

Expand Down
14 changes: 13 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ dependencies:
- github/awesome-copilot/agents/api-architect.agent.md
# A full APM package with instructions, skills, prompts, hooks...
- microsoft/apm-sample-package#v1.0.0
mcp:
# MCP servers -- installed into every detected client
- name: io.github.github/github-mcp-server
transport: http # MCP transport name, not URL scheme -- connects over HTTPS
```

```bash
Expand All @@ -37,7 +41,7 @@ apm install # every agent is configured

## Highlights

- **[One manifest for everything](https://microsoft.github.io/apm/reference/primitive-types/)** — instructions, skills, prompts, agents, hooks, plugins, MCP servers
- **[One manifest for everything](https://microsoft.github.io/apm/reference/primitive-types/)** — instructions, skills, prompts, agents, hooks, plugins, and [MCP servers](https://microsoft.github.io/apm/guides/mcp-servers/) declared in `apm.yml` and deployed across every client on install
- **[Install from anywhere](https://microsoft.github.io/apm/guides/dependencies/)** — GitHub, GitLab, Bitbucket, Azure DevOps, GitHub Enterprise, any git host
- **[Transitive dependencies](https://microsoft.github.io/apm/guides/dependencies/)** — packages can depend on packages; APM resolves the full tree
- **[Content security](https://microsoft.github.io/apm/enterprise/security/)** — `apm audit` scans for hidden Unicode; `apm install` blocks compromised packages before agents read them
Expand Down Expand Up @@ -99,6 +103,14 @@ apm marketplace add github/awesome-copilot
apm install azure-cloud-development@awesome-copilot
```

Or add an MCP server (wired into Copilot, Claude, Cursor, Codex, and OpenCode):

```bash
apm install --mcp io.github.github/github-mcp-server --transport http # connects over HTTPS
```

> *Codex CLI currently does not support remote MCP servers; the install will skip Codex with a notice. Omit `--transport http` to use the local Docker variant on Codex (requires `GITHUB_PERSONAL_ACCESS_TOKEN`).*

See the **[Getting Started guide](https://microsoft.github.io/apm/getting-started/quick-start/)** for the full walkthrough.

## Works with agentrc
Expand Down
1 change: 1 addition & 0 deletions docs/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export default defineConfig({
{ label: 'Skills', slug: 'guides/skills' },
{ label: 'Prompts', slug: 'guides/prompts' },
{ label: 'Plugins', slug: 'guides/plugins' },
{ label: 'MCP Servers', slug: 'guides/mcp-servers' },
{ label: 'Dependencies & Lockfile', slug: 'guides/dependencies' },
{ label: 'Pack & Distribute', slug: 'guides/pack-distribute' },
{ label: 'Private Packages', slug: 'guides/private-packages' },
Expand Down
10 changes: 10 additions & 0 deletions docs/src/content/docs/getting-started/quick-start.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,16 @@ When you update `apm.yml`, re-run `apm install` and commit the changed `.github/
These tools use different configuration formats. Run `apm compile` after installing to generate their native files. See the [Compilation guide](../../guides/compilation/) for details.
:::

## Add MCP servers

APM also manages MCP servers -- the tools your AI agent calls at runtime.

```bash
apm install --mcp io.github.github/github-mcp-server
```

This wires the server into every detected client (Copilot, Claude, Cursor, Codex, OpenCode). See the [MCP Servers guide](../../guides/mcp-servers/) for stdio and remote shapes.

## Next steps

- [Your First Package](../first-package/) -- create and share your own APM package.
Expand Down
6 changes: 5 additions & 1 deletion docs/src/content/docs/guides/dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,10 @@ Local path dependencies (`./path`, `../path`, `/abs/path`) are rejected at user

## MCP Dependency Formats

:::tip[Quick start]
For the CLI-first walkthrough (`apm install --mcp ...`), see the [MCP Servers guide](../mcp-servers/). This section covers the `apm.yml` manifest format in depth.
:::

MCP dependencies support three forms: string references, overlay objects, and self-defined servers.

### String Reference (default)
Expand Down Expand Up @@ -372,7 +376,7 @@ mcp:
| Field | Type | Description |
|-------|------|-------------|
| `name` | string | Server reference (required) |
| `transport` | string | `stdio`, `sse`, `http`, or `streamable-http` |
| `transport` | string | `stdio`, `sse`, `http`, or `streamable-http` (MCP transport names, not URL schemes -- remote variants connect over HTTPS) |
| `env` | dict | Environment variable overrides |
| `args` | list or dict | Runtime argument overrides |
| `version` | string | Pin server version |
Expand Down
Loading
Loading