diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a517122c..6fd40937b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 or end with `.` -- can host an APM governance policy repo for the first time. (by @sergio-sisternes-epam; closes #1813) (#1830) +### Removed + +- **Breaking (security):** executable dependencies -- including MCP servers and + canvas extensions -- now require explicit, persistent approval via `apm approve`, + closing the gap where canvas extensions were trusted per-run. The + `--trust-canvas-extensions` flag is removed as a consequence; canvas extensions + are now governed by the `allowExecutables` gate like every other executable + surface. Add an `allowExecutables: {}` block to `apm.yml` and run + `apm approve ` to trust them. (by @sergio-sisternes-epam) (#1865) + + ```diff + - apm install --trust-canvas-extensions # before: per-run trust flag + + apm approve # after: one-time, user-local approval + ``` + + CI / non-interactive pipelines that previously passed the flag should + instead pre-seed approvals before `apm install`, e.g. + `apm approve ` (writes `~/.apm/approvals.yml` directly, no prompt), + so the gate finds the package already trusted and never prompts. + ### Fixed - `apm install @` now preserves GitLab and other @@ -36,6 +56,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Codex MCP config writes, falling back to `~/.codex/config.toml` when unset. (closes #1861) (#1863) +### Security + +- The `allowExecutables` default-deny gate now enforces `mcp` server writes and + `canvas` extensions in addition to `hooks` and `bin`, bringing all four executable + surfaces under one approval model. `apm approve` decisions are also stored + user-local in `~/.apm/approvals.yml` instead of the committed project `apm.yml`, + so cloning a repository no longer silently inherits another developer's executable + approvals. (by @sergio-sisternes-epam) (#1865) + ## [0.21.0] - 2026-06-19 ### Added diff --git a/CONFORMANCE.json b/CONFORMANCE.json index 531ea00a1..aac6b80e4 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -976,6 +976,28 @@ "tests/spec_conformance/test_manifest_reqs.py::test_consumer_should_refuse_credential_on_non_https_git_over_http" ] }, + { + "conformance_class": "consumer", + "id": "req-sc-009", + "keyword": "MUST", + "section": "10.13", + "status": "active", + "test_count": 1, + "tests": [ + "tests/spec_conformance/test_manifest_reqs.py::test_consumer_must_deny_executable_primitive_without_allowexecutables_approval" + ] + }, + { + "conformance_class": "consumer", + "id": "req-sc-010", + "keyword": "MUST", + "section": "10.13", + "status": "active", + "test_count": 1, + "tests": [ + "tests/spec_conformance/test_manifest_reqs.py::test_consumer_must_persist_approvals_user_locally_not_in_project_manifest" + ] + }, { "conformance_class": "consumer", "id": "req-tg-001", @@ -1024,7 +1046,7 @@ "spec_version": "v0.1.1", "summary_by_class": { "consumer": { - "active": 61, + "active": 63, "skipped": 1, "unbound": 0, "xfail": 0 @@ -1048,5 +1070,5 @@ "xfail": 0 } }, - "total_requirements": 90 + "total_requirements": 92 } diff --git a/CONFORMANCE.md b/CONFORMANCE.md index d8d900a9e..e4769acc1 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -19,7 +19,7 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac | Class | Active | Skipped | Xfail | Unbound | |-------|-------:|--------:|------:|--------:| | Producer | 12 | 0 | 0 | 0 | -| Consumer | 61 | 1 | 0 | 0 | +| Consumer | 63 | 1 | 0 | 0 | | Registry | 1 | 0 | 0 | 0 | | Governance | 15 | 0 | 0 | 0 | @@ -113,6 +113,8 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac | [req-sc-006](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-006) | MUST | 4.2.3 | consumer | active | 1 | | [req-sc-007](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-007) | MUST | 10.3 | consumer | active | 1 | | [req-sc-008](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-008) | SHOULD | 10.3 | consumer | active | 1 | +| [req-sc-009](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-009) | MUST | 10.13 | consumer | active | 1 | +| [req-sc-010](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-010) | MUST | 10.13 | consumer | active | 1 | | [req-tg-001](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-001) | MUST | 8.4 | consumer | active | 1 | | [req-tg-002](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-002) | MUST | 8.5 | consumer | active | 1 | | [req-tg-003](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-003) | MUST | 8.5 | 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 87d4c9685..8cf823cb0 100644 --- a/docs/public/specs/manifests/openapm-v0.1.requirements.yml +++ b/docs/public/specs/manifests/openapm-v0.1.requirements.yml @@ -371,6 +371,14 @@ requirements: keyword: SHOULD section: "10.3" conformance_class: consumer + - id: req-sc-009 + keyword: MUST + section: "10.13" + conformance_class: consumer + - id: req-sc-010 + keyword: MUST + section: "10.13" + conformance_class: consumer - id: req-rg-001 keyword: MUST section: "11.3.3" diff --git a/docs/src/content/docs/concepts/primitives-and-targets.md b/docs/src/content/docs/concepts/primitives-and-targets.md index 7e0495d9e..71315e7b6 100644 --- a/docs/src/content/docs/concepts/primitives-and-targets.md +++ b/docs/src/content/docs/concepts/primitives-and-targets.md @@ -70,7 +70,7 @@ Model Context Protocol servers declared as dependencies. APM writes the per-harn ### Canvas extensions (experimental) -GitHub Copilot CLI canvas extensions: a directory bundle whose entry file is `extension.mjs` (executable Node.js). Copilot-only. Behind the `canvas` experimental flag; dependency-provided canvases are blocked unless `--trust-canvas-extensions` is passed, because they are arbitrary executable code. Project scope deploys to `.github/extensions/`; `--global` deploys a dependency canvas to `~/.copilot/extensions/` (always requiring the trust flag). +GitHub Copilot CLI canvas extensions: a directory bundle whose entry file is `extension.mjs` (executable Node.js). Copilot-only. Behind the `canvas` experimental flag; dependency-provided canvases are blocked unless approved via `allowExecutables` in `apm.yml` and `apm approve `, because they are arbitrary executable code. Project scope deploys to `.github/extensions/`; `--global` deploys a dependency canvas to `~/.copilot/extensions/` (always requiring approval). - Source: `.apm/extensions//extension.mjs` - Deploys to: `.github/extensions//` (project) or `~/.copilot/extensions//` (`--global`) @@ -135,7 +135,7 @@ How to read a cell: - `commands / copilot = unsupported` -- Copilot has no commands primitive; the same source `.prompt.md` reaches Copilot as a native prompt instead. - `plugins / *` -- APM unpacks the plugin at install time into the primitives in the rows above; routing then follows those rows. - `MCP servers / *` -- APM writes the harness's standard MCP config. Transitive MCP servers brought in by deep dependencies must be explicitly declared or trusted with `--trust-transitive-mcp` -- effectively `gated` for those, `native` for direct dependencies. -- `canvas / copilot = gated` -- requires the `canvas` experimental flag; a canvas shipped by a dependency is executable code, so it stays blocked until you pass `--trust-canvas-extensions`. First-party canvases in your own package deploy at project scope once the flag is on. With `--global`, a dependency canvas deploys to `~/.copilot/extensions/` and always requires the trust flag (first-party global install is not supported). Every other harness is `unsupported`: a canvas is a Copilot CLI construct only. +- `canvas / copilot = gated` -- requires the `canvas` experimental flag; a canvas shipped by a dependency is executable code, so it stays blocked until the package is approved via `allowExecutables` in `apm.yml` (`apm approve `). First-party canvases in your own package deploy at project scope once the flag is on. With `--global`, a dependency canvas deploys to `~/.copilot/extensions/` and always requires approval (first-party global install is not supported). Every other harness is `unsupported`: a canvas is a Copilot CLI construct only. ## Where compiled context files land diff --git a/docs/src/content/docs/enterprise/security.md b/docs/src/content/docs/enterprise/security.md index 96441c122..bd1f037d9 100644 --- a/docs/src/content/docs/enterprise/security.md +++ b/docs/src/content/docs/enterprise/security.md @@ -30,7 +30,7 @@ APM has no runtime footprint. Once `apm install` or `apm compile` completes, the - **No runtime component.** APM generates files then terminates. It does not run alongside your application. - **No network calls after install.** All network activity (git clone/fetch) occurs during dependency resolution. There are no callbacks, webhooks, or phone-home requests. -- **No arbitrary code execution.** APM does not execute scripts from packages, evaluate expressions in templates, or run downloaded code. (**Canvas exception:** the experimental `canvas` primitive deploys executable `extension.mjs` (Node.js) code to `.github/extensions/` or `~/.copilot/extensions/`; this surface is gated by both the `canvas` experimental flag and `--trust-canvas-extensions` for dependency-provided canvases. See [Canvas extensions](/apm/integrations/canvas/).) +- **No arbitrary code execution.** APM does not execute scripts from packages, evaluate expressions in templates, or run downloaded code. (**Canvas exception:** the experimental `canvas` primitive deploys executable `extension.mjs` (Node.js) code to `.github/extensions/` or `~/.copilot/extensions/`; this surface is gated by both the `canvas` experimental flag and, when the project opts in via `allowExecutables:` in `apm.yml`, the `allowExecutables` approval gate for dependency-provided canvases. See [Canvas extensions](/apm/integrations/canvas/).) - **No access to application data.** APM never reads databases, API responses, application state, or user data. - **No persistent background processes.** APM does not install daemons, services, or scheduled tasks. - **No telemetry or data collection.** APM collects no usage data, analytics, or diagnostics. Nothing is transmitted to Microsoft or any third party. diff --git a/docs/src/content/docs/integrations/canvas.md b/docs/src/content/docs/integrations/canvas.md index b426eee1b..356425cd5 100644 --- a/docs/src/content/docs/integrations/canvas.md +++ b/docs/src/content/docs/integrations/canvas.md @@ -70,28 +70,42 @@ is not picked up mid-session. ## Trust gate for dependency canvases -A canvas shipped by a **dependency** is arbitrary executable Node.js code. APM -blocks dependency-provided canvases by default. To deploy them, opt in -explicitly: +A canvas shipped by a **dependency** is arbitrary executable Node.js code. When +the project opts in to the executable gate (by adding `allowExecutables:` to +`apm.yml`), APM blocks dependency-provided canvases unless the package has been +explicitly approved. To deploy them: + +```yaml +# apm.yml (committed -- opts the project in to the gate) +allowExecutables: {} +``` ```bash -apm install --target copilot --trust-canvas-extensions +# Run once per developer; approval is stored in ~/.apm/approvals.yml (NOT committed) +apm approve some-org/canvas-package +apm install --target copilot ``` +`apm approve` writes grants to `~/.apm/approvals.yml` -- a user-local file +that is never committed to source control. This means cloning a project with +`allowExecutables: {}` does **not** automatically grant trust to any package; +each developer must explicitly approve packages they want to deploy. For +automated CI pipelines, grants can alternatively be listed directly in +`apm.yml` (committed, shared with the team). + The trust gate is independent of the experimental flag: - The **experimental flag** decides whether the canvas primitive is processed at all. It is a feature-availability gate, not a security gate. -- The **`--trust-canvas-extensions` flag** decides whether *dependency* - canvases may deploy. Your own first-party canvas (in the root package you are - installing from) deploys freely once the flag is on; only dependency-provided - canvases need the trust flag. +- The **`allowExecutables` block** decides whether *dependency* canvases may + deploy. Your own first-party canvas (in the root package you are installing + from) deploys freely once the flag is on; only dependency-provided canvases + need approval. -When a dependency canvas is blocked, APM prints a diagnostic naming the package, -the canvas, the `extension.mjs` entry point, the deploy directory, and the -opt-in flag. The same gate is enforced on offline bundle install -(`apm install `) and on `apm unpack`, so a vendored bundle cannot -smuggle an executable canvas past trust. +When a dependency canvas is blocked, APM prints a diagnostic naming the package +and the canvas, and instructs you to run `apm approve `. The same gate is +enforced on offline bundle install (`apm install `), so a vendored +bundle cannot smuggle an executable canvas past trust. ## Install globally (user scope) @@ -99,7 +113,8 @@ To make a canvas available in **every** Copilot session, install it globally so it lands in `~/.copilot/extensions//`: ```bash -apm install --global --trust-canvas-extensions +apm approve +apm install --global ``` Global canvas install is intentionally limited in this experimental release: @@ -109,8 +124,8 @@ Global canvas install is intentionally limited in this experimental release: globally, so APM records it in the user lockfile and `apm uninstall --global` can prune it. A first-party root `.apm/extensions/` canvas is **not** deployed at user scope -- package it and install it as a dependency instead. -- **Trust is always required.** A global canvas has full-account blast radius, - so `--trust-canvas-extensions` is mandatory even though the project-scope +- **Approval is always required.** A global canvas has full-account blast radius, + so `allowExecutables` approval is mandatory even though the project-scope first-party path does not need it. - **Default `~/.copilot` only.** If `$COPILOT_HOME` is set to a non-default location, APM refuses the global canvas install rather than deploy to a path @@ -132,12 +147,12 @@ experimental flag, so a previously-installed canvas can always be removed. (`--target claude`, `cursor`, etc.) never receive it. - **Global install is dependency-only.** User-scope (`--global`) deployment to `~/.copilot/extensions/` supports dependency-provided canvases (always - requiring `--trust-canvas-extensions`) and the default `~/.copilot` location + requiring `allowExecutables` approval) and the default `~/.copilot` location only; first-party root canvases deploy at project scope only. - **No compile/list surfacing yet.** Canvases are not yet shown by `apm list`/`apm compile`; they are deployed at install only. -- **No policy-file control yet.** Canvas trust is controlled only by the - `--trust-canvas-extensions` CLI flag; governing it via `apm-policy.yml` is +- **No policy-file control yet.** Canvas trust is governed by `allowExecutables` + in `apm.yml`; a dedicated `apm-policy.yml` field for org-wide canvas policy is planned but not part of this experimental release. See the [primitives and targets](/apm/concepts/primitives-and-targets/) matrix diff --git a/docs/src/content/docs/reference/cli/approve.md b/docs/src/content/docs/reference/cli/approve.md index 024385a04..9dbf6e549 100644 --- a/docs/src/content/docs/reference/cli/approve.md +++ b/docs/src/content/docs/reference/cli/approve.md @@ -9,27 +9,28 @@ sidebar: ```bash apm approve [PACKAGE_REF...] [OPTIONS] -apm deny [PACKAGE_REF...] [OPTIONS] +apm deny [PACKAGE_REF...] ``` ## Description -APM blocks executable primitives (hooks, bin/ executables) from -dependency packages by default. The `allowExecutables` block in -`apm.yml` records which packages have been explicitly approved to -deploy executables. +APM blocks executable primitives (hooks, bin/ executables, self-defined MCP +servers, and canvas extensions) from dependency packages by default. The +`allowExecutables` block in `apm.yml` opts the **project** in to the gate. +User-specific approvals are stored in **`~/.apm/approvals.yml`** -- a +personal file that is never committed to source control. -`apm approve` adds a package to the allowlist. `apm deny` removes it. +`apm approve` adds a package to the user-local allowlist. `apm deny` removes it. ### How the gate works -When `apm install` encounters a dependency that ships hooks or bin/ -executables: +When `apm install` encounters a dependency that ships executable primitives: 1. If `allowExecutables` is **absent** from `apm.yml`, everything is approved (backward-compatible, no gate). -2. If `allowExecutables` is **present** (even empty `{}`), only listed - packages may deploy executables. +2. If `allowExecutables` is **present** (even empty `{}`), only packages + approved in `~/.apm/approvals.yml` (or listed directly in `apm.yml` + for CI pipelines) may deploy executables. 3. In interactive mode, `apm install` prompts for each unapproved package. In CI (non-interactive), unapproved executables cause a hard error. @@ -42,9 +43,21 @@ Local project content (the root `.apm/` directory) is always trusted. |------|-------|-------| | Hooks (`.apm/hooks/`, `hooks/`) | Yes | Auto-fire in IDE on lifecycle events | | Bin executables (`bin/`) | Yes | Deployed to agent PATH via symlinks | -| MCP servers | No | Enforcement deferred to a future release | +| MCP servers (self-defined) | Yes | `registry: false` servers write to IDE MCP config | +| Canvas extensions (`.apm/extensions/`) | Yes | Deploys executable Node.js to IDE extensions | | Text primitives (skills, agents, instructions) | No | No code execution risk | +### Where approvals are stored + +| Store | Path | Who manages it | Committed? | +|-------|------|----------------|------------| +| User-local approvals | `~/.apm/approvals.yml` | `apm approve` / `apm deny` | No | +| Project gate + CI grants | `apm.yml` (`allowExecutables`) | Project maintainer / CI setup | Yes | + +Approvals from both stores are merged at install time. The project `apm.yml` +signals that the gate is enabled and may include pre-approved packages for CI; +developer approvals live in the user file and are personal. + ## Options ### `apm approve` @@ -59,20 +72,20 @@ Local project content (the root `.apm/` directory) is always trusted. | Flag | Description | |------|-------------| -| `PACKAGE_REF` | One or more packages to deny (removes from allowlist). | +| `PACKAGE_REF` | One or more packages to deny (removes from user-local allowlist). | -## Manifest format +## User approvals file format -Approvals are stored in `apm.yml` under `allowExecutables`, keyed by -`name#version` with per-type boolean flags: +`apm approve` writes to `~/.apm/approvals.yml`. The file stores the approvals +mapping directly, keyed by `name#version` with per-type boolean flags: ```yaml -allowExecutables: - "ci-hooks@acme#1.2.0": - hooks: true - bin: true - "dev-tools@org#0.5.0": - hooks: true +# ~/.apm/approvals.yml (auto-generated, do not commit) +"ci-hooks@acme#1.2.0": + hooks: true + bin: true +"dev-tools@org#0.5.0": + hooks: true ``` Version pinning means approval must be renewed when a package updates. @@ -107,14 +120,16 @@ apm deny ci-hooks@acme In CI environments (`CI=true`, `APM_NON_INTERACTIVE=1`, or when stdin is not a TTY), `apm install` fails with exit code 1 if any dependency -has unapproved executables. Pre-approve packages in `apm.yml` before -CI runs: +has unapproved executables. Pre-approve packages by listing them +directly in `apm.yml` (this is the only way to share approvals via +source control): -```bash -# One-time setup: approve all current dependencies -apm approve --all -git add apm.yml -git commit -m "Approve executable dependencies" +```yaml +# apm.yml +allowExecutables: + "ci-hooks@acme#1.2.0": + hooks: true + bin: true ``` ## See also diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index c44b428b1..fe682d07c 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -61,7 +61,6 @@ With no arguments it installs everything from `apm.yml`. With one or more `PACKA | `--audit ` | (config/policy) | Run a content audit over the files this install deploys. `warn` records findings in the summary; `block` halts the install on critical findings. Overrides your `audit-on-install` config but cannot relax an org policy floor. Requires the `external-scanners` experimental flag. | | `--no-audit` | off | Disable the install-time audit for this invocation (equivalent to `--audit off`). Cannot relax an org policy `block` floor. | | `--trust-transitive-mcp` | off | Trust self-defined MCP servers shipped by transitive packages without re-declaring them in your `apm.yml`. | -| `--trust-canvas-extensions` | off | Trust executable canvas extensions (`extension.mjs`) shipped by dependencies. Required for dependency-provided canvases; first-party canvases deploy without it. Requires the `canvas` experimental flag. | | `--allow-insecure` | off | Permit direct `http://` (non-TLS) dependencies. | | `--allow-insecure-host HOSTNAME` | unset | Permit transitive `http://` dependencies from `HOSTNAME`. Repeatable. | diff --git a/docs/src/content/docs/reference/cli/unpack.md b/docs/src/content/docs/reference/cli/unpack.md index 17c8c94ef..aae1bcc93 100644 --- a/docs/src/content/docs/reference/cli/unpack.md +++ b/docs/src/content/docs/reference/cli/unpack.md @@ -23,6 +23,17 @@ Extraction is **additive-only**: only files listed in the bundle's lockfile are `BUNDLE_PATH` accepts a `.zip` archive (the default), a legacy `.tar.gz` archive, or the directory form of an unpacked bundle. +:::caution[Bundles are self-contained and deploy executable surfaces directly] +`apm unpack` materializes a complete, self-contained bundle into your +project. Unlike `apm install`, there is no upstream `allowExecutables` +approval prompt at unpack time -- a bundle that contains a canvas +`extension.mjs` (executable Node.js code) deploys that file when the +`canvas` experimental flag is enabled. Only unpack bundles from sources +you trust, and use `--dry-run` to inspect the file list first. See +[Executable approval](/apm/reference/cli/approve/) for the gate that +governs dependency installs. +::: + ## Options | Flag | Default | Description | @@ -31,7 +42,6 @@ Extraction is **additive-only**: only files listed in the bundle's lockfile are | `--skip-verify` | off | Skip the bundle completeness check against the bundle's `apm.lock.yaml`. Useful for partial bundles. | | `--dry-run` | off | List files that would be unpacked without writing anything. | | `--force` | off | Deploy despite critical hidden-character findings from the security scan. Use only after independent verification. | -| `--trust-canvas-extensions` | off | Trust executable canvas extensions (`extension.mjs`) in the bundle. Without this, canvas files are stripped during extraction. Requires the `canvas` experimental flag. | | `--verbose`, `-v` | off | Show per-file paths and full diagnostic context. | ## Examples diff --git a/docs/src/content/docs/reference/experimental.md b/docs/src/content/docs/reference/experimental.md index a9c303a1f..0e86d5c44 100644 --- a/docs/src/content/docs/reference/experimental.md +++ b/docs/src/content/docs/reference/experimental.md @@ -175,7 +175,7 @@ apm experimental reset verbose-version | `marketplace-authoring`| Enable marketplace authoring commands (init, build, publish, etc.). | | `registries` | Enable REST-based APM package registries in `apm.yml`. | | `external-scanners` | Ingest third-party SARIF scanners into `apm audit` (`--external`, including SkillSpector LLM mode and allowlisted `--external-args`), the `external..{llm,args}` config keys, and the `security.audit.scanners` policy block. See [External scanners](../integrations/external-scanners/). | -| `canvas` | Ship Copilot CLI canvas extensions (`.apm/extensions//extension.mjs`) through APM packages. Dependency-provided canvases additionally require `--trust-canvas-extensions`. See [Canvas extensions](../integrations/canvas/). | +| `canvas` | Ship Copilot CLI canvas extensions (`.apm/extensions//extension.mjs`) through APM packages. Dependency-provided canvases require `allowExecutables` approval (`apm approve `). See [Canvas extensions](../integrations/canvas/). | New flags are proposed via [CONTRIBUTING.md](https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md#how-to-add-an-experimental-feature-flag) and graduate to default when stable. See the contributor recipe for the full lifecycle. See also: [Cowork integration](../integrations/copilot-cowork/). diff --git a/docs/src/content/docs/specs/openapm-v0.1.md b/docs/src/content/docs/specs/openapm-v0.1.md index b465efece..cce904fbb 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 **90 normative statements** indexed in +- OpenAPM v0.1 carries **92 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 @@ -2270,6 +2270,8 @@ every stored hash, foreclosing algorithm-ambiguity attacks. | 8 | Policy bypass via crafted manifest | [req-pl-002](#req-pl-002), [req-pl-009](#req-pl-009), [req-pl-010](#req-pl-010) | Governance-only | | 9 | Archive path-traversal | [req-sc-002](#req-sc-002), [req-sc-004](#req-sc-004) | Consumer-default | | 10| Hash-algorithm downgrade | [req-mf-018](#req-mf-018), [req-lk-016](#req-lk-016) | Consumer-default | +| 11| Unauthorised executable primitive deployment | [req-sc-009](#req-sc-009) | Consumer-default | +| 12| Approval grant propagation via VCS | [req-sc-010](#req-sc-010) | Consumer-default | ### 10.12 Publisher provenance and attestations (reserved for v0.2) @@ -2285,6 +2287,36 @@ Governance `policy.dependencies.require_attestation` enforcement modes; and the registry HTTP wire envelope (alongside [Appendix B](#appendix-b-registry-http-api-reserved-for-v02)). +### 10.13 Executable primitive approval gate + +**Threat.** A dependency package deploys executable code -- +hooks, bin executables, MCP server configurations, or canvas +extensions -- that runs on the developer's machine without +explicit consent. + +**Mitigations.** + + +**[req-sc-009]** A conforming **consumer** implementation MUST, +when the consuming project's `apm.yml` contains an `allowExecutables` +block, deny deployment of any executable primitive (hooks, bin +executables, MCP server configurations, and canvas extensions) +from a dependency package unless that package is explicitly listed +in the effective approval set for the corresponding executable type. +A consumer MUST fail closed when the `allowExecutables` block is +present but the package is absent from the approval set: the +primitive MUST NOT be deployed. + + +**[req-sc-010]** A conforming **consumer** implementation that +provides an interactive approval mechanism for executable primitives +MUST persist per-user approval decisions in a location that is +isolated from the project manifest (`apm.yml`) and is not tracked +by version control alongside project files by default. The consumer +MUST NOT write interactive approval decisions into the project +`apm.yml`, so that one developer's approval cannot propagate +implicitly to other developers who clone or share the project. + --- ## 11. Conformance @@ -2372,7 +2404,8 @@ conformance statement identifying: [req-sc-002](#req-sc-002), [req-sc-003](#req-sc-003), [req-sc-004](#req-sc-004), [req-sc-005](#req-sc-005), [req-sc-006](#req-sc-006), [req-sc-007](#req-sc-007), -[req-sc-008](#req-sc-008) (SHOULD), [req-cf-001](#req-cf-001), +[req-sc-008](#req-sc-008) (SHOULD), [req-sc-009](#req-sc-009), +[req-sc-010](#req-sc-010), [req-cf-001](#req-cf-001), [req-cf-002](#req-cf-002). #### 11.3.3 Registry @@ -2781,11 +2814,13 @@ renumbering of conformance classes. | [req-sc-006](#req-sc-006) | MUST | 4.2.3 | consumer | | [req-sc-007](#req-sc-007) | MUST | 10.3 | consumer | | [req-sc-008](#req-sc-008) | SHOULD | 10.3 | consumer | +| [req-sc-009](#req-sc-009) | MUST | 10.13 | consumer | +| [req-sc-010](#req-sc-010) | MUST | 10.13 | consumer | | [req-rg-001](#req-rg-001) | MUST | 11.3.3 | registry | | [req-cf-001](#req-cf-001) | MUST | 12.5 | consumer | | [req-cf-002](#req-cf-002) | MUST | 12.3 | consumer | -**Total normative statements: 90** (85 MUST, 5 SHOULD). +**Total normative statements: 92** (87 MUST, 5 SHOULD). --- @@ -2799,6 +2834,7 @@ renumbering of conformance classes. | 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. | +| 0.1.5 | 2026-06-20 | Spec-citation fold for the executable primitive approval gate. Added new Section 10.13 "Executable primitive approval gate" with two consumer MUSTs: [req-sc-009] (deny deployment of any hook, bin, MCP server, or canvas extension from a dependency not listed in the effective `allowExecutables` approval set when the block is present -- fail closed) and [req-sc-010] (persist interactive approval decisions user-locally, not in the project `apm.yml`, so one developer's approval cannot propagate via VCS to teammates). Added rows 11 and 12 to the Section 10.11 summary table. Section 11.3.2 Consumer enumeration and Appendix C updated. Statement count: 90 -> 92 (87 MUST, 5 SHOULD). | Errata (none at publication). diff --git a/packages/apm-contributor-dashboard/README.md b/packages/apm-contributor-dashboard/README.md index 00bd964ad..eebb2ec2f 100644 --- a/packages/apm-contributor-dashboard/README.md +++ b/packages/apm-contributor-dashboard/README.md @@ -26,8 +26,9 @@ issues, pull requests, CI pipelines, and panel reviews in real time. # Enable the experimental canvas feature apm experimental enable canvas -# Install at project level (deploys to .github/extensions/) -apm install microsoft/apm/packages/apm-contributor-dashboard --trust-canvas-extensions +# Approve the canvas extension in apm.yml, then install +apm approve microsoft/apm/packages/apm-contributor-dashboard +apm install microsoft/apm/packages/apm-contributor-dashboard ``` The deployed `.github/extensions/` directory is auto-generated and should @@ -39,8 +40,8 @@ After installing, open the dashboard in Copilot CLI by asking: > "Open the APM Contributor Dashboard" The canvas registers as `issue-monitor` and appears as -"APM Contributor Dashboard" in the canvas list. The `--trust-canvas-extensions` -flag grants the extension permission to execute `gh` CLI commands on your +"APM Contributor Dashboard" in the canvas list. The `allowExecutables` approval +grants the extension permission to execute `gh` CLI commands on your behalf (e.g. posting comments, approving PRs). Only install extensions you trust. diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 89f895ef7..09062cd0b 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -10,7 +10,7 @@ | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` (deprecated; prefer `apm update`) refresh refs, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite (does NOT refresh refs; use `apm update` for that), `--frozen` CI-safe install that fails fast when `apm.lock.yaml` is missing or out of sync with `apm.yml` (mutually exclusive with `--update`; structural presence check only -- use `apm audit` for SHA integrity), `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--trust-canvas-extensions` (deploy executable canvas extensions from dependencies; requires `apm experimental enable canvas`; first-party canvases deploy without it), `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from a skill collection (SKILL_BUNDLE or plugin manifest; repeatable; plugin manifests accept a leaf name or manifest path; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry (NAME goes through the same `--target` > `targets:` > auto-detect resolver as APM packages, so a project whitelisting `targets: [copilot]` will not write `.cursor/mcp.json` even if `.cursor/` exists; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2) | +| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` (deprecated; prefer `apm update`) refresh refs, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite (does NOT refresh refs; use `apm update` for that), `--frozen` CI-safe install that fails fast when `apm.lock.yaml` is missing or out of sync with `apm.yml` (mutually exclusive with `--update`; structural presence check only -- use `apm audit` for SHA integrity), `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from a skill collection (SKILL_BUNDLE or plugin manifest; repeatable; plugin manifests accept a leaf name or manifest path; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry (NAME goes through the same `--target` > `targets:` > auto-detect resolver as APM packages, so a project whitelisting `targets: [copilot]` will not write `.cursor/mcp.json` even if `.cursor/` exists; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2) | | `apm targets` | Show resolved deployment targets for the current project (Click group; reads filesystem signals; works with or without `apm.yml`) | `--all` also include the `agent-skills` meta-target (only meaningful with `--json`), `--json` machine-readable output. No provenance line is printed (the table is the provenance). | | `apm uninstall PKGS...` | Remove packages (accepts `owner/repo` or `name@marketplace`) | `--dry-run`, `-g` global | | `apm prune` | Remove orphaned packages | `--dry-run` | @@ -231,4 +231,4 @@ Experimental flags MUST NOT gate security-critical behaviour (content scanning, `apm config set mcp-registry-url https://mcp.internal.example.com` persists a private MCP registry URL so users do not need to export `MCP_REGISTRY_URL` every session. Accepts `http://` or `https://` URLs; all other schemes are rejected. Resolution order: `--registry ` flag on `apm mcp install` / `apm install --mcp` > `MCP_REGISTRY_URL` env var > `mcp-registry-url` in `~/.apm/config.json` > built-in public default. When the config layer is active, `apm mcp search` prints a `Registry (config): ` diagnostic. `apm config unset mcp-registry-url` removes the persisted URL. -`apm approve [PACKAGE_REF...]` adds packages to the `allowExecutables` block in `apm.yml`, permitting their hooks and bin/ executables to deploy during `apm install`. `apm approve --pending` lists all packages with unapproved executables. `apm approve --all` approves every currently blocked package. `apm deny [PACKAGE_REF...]` removes packages from the allowlist, blocking their executables on the next install. Approvals are version-pinned (`name#version`); updating a package requires re-approval. In non-interactive environments (CI), unapproved executables cause `apm install` to exit 1. +`apm approve [PACKAGE_REF...]` grants a package permission to deploy executable primitives (hooks, bin/, self-defined MCP servers, canvas extensions). Approvals are written to `~/.apm/approvals.yml` -- a user-local file that is never committed to source control. The project `apm.yml` must have an `allowExecutables:` key (even empty `{}`) to enable the gate; without it, all executables are allowed unconditionally. `apm approve --pending` lists all packages with unapproved executables. `apm approve --all` approves every currently blocked package. `apm deny [PACKAGE_REF...]` removes a package from the user-local allowlist. Approvals are version-pinned (`name#version`); updating a package requires re-approval. In non-interactive environments (CI), pre-approve packages by listing them directly in `apm.yml` (the only way to share approvals via source control). Unapproved executables cause `apm install` to exit 1 in CI. diff --git a/packages/apm-guide/.apm/skills/apm-usage/governance.md b/packages/apm-guide/.apm/skills/apm-usage/governance.md index 5d7161a7e..020b911d5 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/governance.md +++ b/packages/apm-guide/.apm/skills/apm-usage/governance.md @@ -174,27 +174,36 @@ environments where plugin executables are not trusted by default. Behind the `canvas` experimental flag, a package may ship a Copilot CLI canvas extension under `.apm/extensions//extension.mjs` (executable Node.js). -Because a canvas from a dependency is arbitrary executable code, APM **blocks -dependency-provided canvases by default**: the consumer must pass -`--trust-canvas-extensions` to deploy them. A first-party canvas in the root +Because a canvas from a dependency is arbitrary executable code, APM blocks +dependency-provided canvases when the project opts in to the executable gate: +the project must add `allowExecutables: {}` to `apm.yml` and each developer +must run `apm approve ` to deploy it. A first-party canvas in the root package being installed deploys once the flag is on; dependency canvases always -require the trust flag. +require explicit approval. + +Approvals recorded by `apm approve` are stored in **`~/.apm/approvals.yml`** +(user-local, never committed to source control). Adding `allowExecutables: {}` +to `apm.yml` enables the gate for the project, but does not grant trust to any +package; every developer cloning the project must approve packages themselves. +CI pipelines may commit specific grants directly in `apm.yml` to bypass +interactive approval in automated environments. At **project scope** a canvas deploys to `.github/extensions//`. With `--global`, a **dependency-provided** canvas deploys to `~/.copilot/extensions//` so it is available in every Copilot session; -global install always requires `--trust-canvas-extensions` (full-account blast +global install always requires `allowExecutables` approval (full-account blast radius), supports only the default `~/.copilot` location (a non-default `$COPILOT_HOME` is refused), and does not deploy first-party root canvases (package them as a dependency instead). `apm uninstall --global` prunes the global canvas. -The trust gate is enforced on every install path -- normal install, offline -bundle install (`apm install `), and `apm unpack` -- so a vendored -bundle cannot smuggle an executable canvas past trust. The flag is a -feature-availability toggle, not a security gate; the trust requirement holds -regardless of the flag. An enterprise policy field for canvas trust is a -deferred follow-up and is not part of this experimental release. +The trust gate is enforced on every install path -- normal install and offline +bundle install (`apm install `) -- so a vendored bundle cannot smuggle +an executable canvas past trust. Canvas trust is now unified with the +`allowExecutables` default-deny gate (hooks, bin, mcp, canvas); approve once and +all four executable types are governed consistently. An enterprise policy field +for canvas trust is a deferred follow-up and is not part of this experimental +release. ## Local content governance diff --git a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md index b778bca09..4495117f4 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md +++ b/packages/apm-guide/.apm/skills/apm-usage/package-authoring.md @@ -412,10 +412,11 @@ On `apm install --target copilot`, APM deploys it verbatim to `.github/extensions//`. The `` segment is validated strictly (`[A-Za-z0-9._-]+`, no leading/trailing dot, no `..`, no separators, no reserved names). It is **Copilot-only**. Dependency-provided canvases are executable code -and are blocked unless the consumer passes `--trust-canvas-extensions`; a +and are blocked unless the consumer adds the package to `allowExecutables` and runs +`apm approve `; a first-party canvas in the root package deploys once the flag is on. With `--global`, a dependency canvas deploys to `~/.copilot/extensions//` -(always requiring the trust flag; default `~/.copilot` only; first-party root +(always requiring `allowExecutables` approval; default `~/.copilot` only; first-party root canvases are project-scope only). `apm pack` preserves `.apm/extensions/`. See the [canvas integration guide](../../../../../docs/src/content/docs/integrations/canvas.md). diff --git a/src/apm_cli/bundle/plugin_exporter.py b/src/apm_cli/bundle/plugin_exporter.py index 3c2e8b48c..ec88ea413 100644 --- a/src/apm_cli/bundle/plugin_exporter.py +++ b/src/apm_cli/bundle/plugin_exporter.py @@ -111,7 +111,7 @@ def _collect_apm_components(apm_dir: Path) -> list[tuple[Path, str]]: # extensions/ -> extensions/ (canvas extensions, experimental Copilot-only). # Preserved verbatim so an offline bundle can carry a canvas; the files are # inert until the consumer enables the ``canvas`` experimental flag AND - # passes ``--trust-canvas-extensions`` at install time. + # approves the package via allowExecutables / ``apm approve`` at install time. _collect_recursive(apm_dir / "extensions", "extensions", components) return components diff --git a/src/apm_cli/bundle/unpacker.py b/src/apm_cli/bundle/unpacker.py index 4f823720e..f17e4eba8 100644 --- a/src/apm_cli/bundle/unpacker.py +++ b/src/apm_cli/bundle/unpacker.py @@ -40,7 +40,6 @@ def unpack_bundle( skip_verify: bool = False, dry_run: bool = False, force: bool = False, - trust_canvas: bool = False, ) -> UnpackResult: """Extract and apply an APM bundle to a project directory. @@ -54,10 +53,6 @@ def unpack_bundle( skip_verify: If *True*, skip completeness verification against the lockfile. dry_run: If *True*, resolve the file list but write nothing to disk. force: If *True*, deploy even when critical hidden characters are found. - trust_canvas: If *True*, allow executable canvas extension files - (``.github/extensions//extension.mjs``) to be unpacked. - Defaults to *False* (fail closed) so a vendored bundle cannot - smuggle executable canvas code past the trust gate. Returns: :class:`UnpackResult` describing what was (or would be) extracted. @@ -170,18 +165,17 @@ def unpack_bundle( dep_file_map[dep_key] = dep_files # Security + feature gate: canvas extensions are executable Node - # bundles (``extension.mjs``). ``apm unpack`` copies deployed files - # verbatim WITHOUT routing through ``CanvasIntegrator``, so neither - # the experimental feature flag nor its trust gate would otherwise - # apply. Require BOTH gates -- the ``canvas`` experimental flag ON - # (feature availability) AND ``trust_canvas`` (executable-code trust) - # -- before unpacking canvas paths. Fail closed: drop them when - # either gate is missing. + # bundles (``extension.mjs``). ``apm unpack`` (deprecated) copies + # deployed files verbatim WITHOUT routing through ``CanvasIntegrator``. + # When the canvas feature flag is OFF, drop paths silently. + # When ON, canvas files are allowed (apm unpack has no project context + # so no allowExecutables enforcement is available; the user is assumed + # to understand what they are unpacking for this deprecated command). canvas_blocked = 0 from ..core.experimental import is_enabled from ..integration.canvas_integrator import is_canvas_bundle_path - if not (is_enabled("canvas") and trust_canvas): + if not is_enabled("canvas"): _blocked = {f for f in unique_files if is_canvas_bundle_path(f)} if _blocked: canvas_blocked = len(_blocked) diff --git a/src/apm_cli/commands/approve.py b/src/apm_cli/commands/approve.py index 662d0e798..94e5eecd7 100644 --- a/src/apm_cli/commands/approve.py +++ b/src/apm_cli/commands/approve.py @@ -1,7 +1,10 @@ """``apm approve`` and ``apm deny`` -- manage executable primitive approvals. These commands mirror npm v12's ``npm approve-scripts`` / ``npm deny-scripts``. -They read and write the ``allowExecutables`` block in the project's ``apm.yml``. +They read and write the user-local ``~/.apm/approvals.yml`` file so that +approval decisions are never committed to source control. The project +``apm.yml`` only signals whether the gate is enabled (``allowExecutables: {}`` +key present) -- individual package approvals live in the user file. """ from __future__ import annotations @@ -23,20 +26,17 @@ def _find_manifest() -> Path: return manifest -def _load_allow_executables(manifest: Path) -> dict[str, dict[str, bool]] | None: - """Load the ``allowExecutables`` block from ``apm.yml``. +def _load_allow_executables() -> dict[str, dict[str, bool]] | None: + """Load the user-local approvals from ``~/.apm/approvals.yml``. - Returns ``None`` when the project has not declared the block (gate - disabled -- backward-compatible) vs ``{}`` when the block is present - but empty (gate enabled, deny-all). + Returns ``None`` when no approvals have been recorded yet (gate behaviour + is determined by the project manifest, not this file). """ - from ..security.executables import parse_allow_executables - from ..utils.yaml_io import load_yaml + from ..security.executables import get_user_approvals_path, load_user_approvals - data = load_yaml(manifest) - if not isinstance(data, dict): + if not get_user_approvals_path().is_file(): return None - return parse_allow_executables(data) + return load_user_approvals() @click.command("approve") @@ -55,9 +55,9 @@ def _load_allow_executables(manifest: Path) -> dict[str, dict[str, bool]] | None def approve_cmd(packages: tuple[str, ...], pending: bool, approve_all: bool) -> None: """Approve executable primitives for installed packages. - Adds entries to the ``allowExecutables`` block in ``apm.yml`` so that - hooks, MCP servers, and bin/ executables from the specified packages - are deployed during ``apm install``. + Adds entries to ``~/.apm/approvals.yml`` (user-local, never committed to + source control) so that hooks, MCP servers, canvas extensions, and bin/ + executables from the specified packages are deployed during ``apm install``. Examples: @@ -68,14 +68,12 @@ def approve_cmd(packages: tuple[str, ...], pending: bool, approve_all: bool) -> apm approve --all """ manifest = _find_manifest() - allow_exec = _load_allow_executables(manifest) + allow_exec = _load_allow_executables() if pending: _show_pending(manifest, allow_exec or {}) return - # Approving a package implies opting into the gate; initialise - # the block when absent so approvals are persisted correctly. if allow_exec is None: allow_exec = {} @@ -95,31 +93,30 @@ def approve_cmd(packages: tuple[str, ...], pending: bool, approve_all: bool) -> def deny_cmd(packages: tuple[str, ...]) -> None: """Revoke executable approval for packages. - Removes entries from the ``allowExecutables`` block in ``apm.yml``. + Removes entries from ``~/.apm/approvals.yml`` (user-local approvals + store). Example: apm deny owner/repo """ - manifest = _find_manifest() - allow_exec = _load_allow_executables(manifest) or {} + allow_exec = _load_allow_executables() or {} - from ..security.executables import write_allow_executables + from ..security.executables import save_user_approvals removed = 0 for pkg in packages: - # Try exact match first, then prefix match matched_key = _find_matching_key(allow_exec, pkg) if matched_key: del allow_exec[matched_key] _rich_success(f"Revoked approval for {matched_key}") removed += 1 else: - _rich_warning(f"{pkg}: not found in allowExecutables") + _rich_warning(f"{pkg}: not found in ~/.apm/approvals.yml") if removed > 0: - write_allow_executables(manifest, allow_exec) - _rich_info(f"Updated allowExecutables in apm.yml ({removed} removed).", symbol="info") + save_user_approvals(allow_exec) + _rich_info(f"Updated ~/.apm/approvals.yml ({removed} removed).", symbol="info") def _find_matching_key(allow_exec: dict[str, dict[str, bool]], pkg: str) -> str | None: @@ -157,7 +154,7 @@ def _show_pending(manifest: Path, allow_exec: dict[str, dict[str, bool]]) -> Non def _approve_all_pending(manifest: Path, allow_exec: dict[str, dict[str, bool]]) -> None: """Approve all installed packages with unapproved executables.""" - from ..security.executables import write_allow_executables + from ..security.executables import save_user_approvals declarations = _scan_installed_packages(manifest) count = 0 @@ -171,8 +168,8 @@ def _approve_all_pending(manifest: Path, allow_exec: dict[str, dict[str, bool]]) _rich_success("All packages with executables are already approved.") return - write_allow_executables(manifest, allow_exec) - _rich_info(f"Updated allowExecutables in apm.yml ({count} approved).", symbol="info") + save_user_approvals(allow_exec) + _rich_info(f"Updated ~/.apm/approvals.yml ({count} approved).", symbol="info") def _approve_packages( @@ -181,18 +178,16 @@ def _approve_packages( packages: tuple[str, ...], ) -> None: """Approve specific packages by name.""" - from ..security.executables import write_allow_executables + from ..security.executables import save_user_approvals declarations = _scan_installed_packages(manifest) decl_map = {d.package_name: d for d in declarations} - # Also index by package_key for exact matches decl_key_map = {d.package_key: d for d in declarations} count = 0 for pkg in packages: decl = decl_key_map.get(pkg) or decl_map.get(pkg) if decl is None: - # Try prefix match on keys for d in declarations: if d.package_key.startswith(pkg + "#") or d.package_name.startswith(pkg): decl = d @@ -211,8 +206,8 @@ def _approve_packages( count += 1 if count > 0: - write_allow_executables(manifest, allow_exec) - _rich_info(f"Updated allowExecutables in apm.yml ({count} approved).", symbol="info") + save_user_approvals(allow_exec) + _rich_info(f"Updated ~/.apm/approvals.yml ({count} approved).", symbol="info") def _scan_installed_packages(manifest: Path) -> list: diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 3e5aa8f9c..765725303 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -211,7 +211,6 @@ class InstallContext: protocol_pref: Any # ProtocolPreference allow_protocol_fallback: bool trust_transitive_mcp: bool - trust_canvas: bool no_policy: bool install_mode: Any # InstallMode packages: tuple # Original Click packages @@ -920,11 +919,6 @@ def _handle_mcp_install( is_flag=True, help="Trust self-defined MCP servers from transitive packages (skip re-declaration requirement)", ) -@click.option( - "--trust-canvas-extensions", - is_flag=True, - help="[experimental] Deploy canvas extensions provided by dependencies. Canvas extensions are executable Node code and are blocked by default; this flag opts in. With --global the canvas deploys to ~/.copilot/extensions and the flag is always required. Requires the 'canvas' experimental feature.", -) @click.option( "--parallel-downloads", type=int, @@ -1134,7 +1128,6 @@ def install( # noqa: PLR0913 frozen, verbose, trust_transitive_mcp, - trust_canvas_extensions, parallel_downloads, dev, target, @@ -1266,6 +1259,10 @@ def install( # noqa: PLR0913 except ValueError as exc: raise click.UsageError(f"Bundle security check failed: {exc}") from exc if _bundle_info is not None: + # allowExecutables for bundle install gate. + from ..security.executables import read_bundle_allow_executables as _rbae + + _allow_execs_for_bundle = _rbae(Path(root or ".") / "apm.yml", logger) _install_lb( bundle_info=_bundle_info, bundle_arg=packages[0], @@ -1277,7 +1274,7 @@ def install( # noqa: PLR0913 alias=alias, logger=logger, legacy_skill_paths=legacy_skill_paths, - trust_canvas=trust_canvas_extensions, + allow_executables=_allow_execs_for_bundle, # Rejected-flag context for consolidated UsageError: rejected_flags={ "--update": update, @@ -1546,7 +1543,6 @@ def install( # noqa: PLR0913 protocol_pref=protocol_pref, allow_protocol_fallback=allow_protocol_fallback, trust_transitive_mcp=trust_transitive_mcp, - trust_canvas=trust_canvas_extensions, no_policy=no_policy, audit_override=audit_override, install_mode=InstallMode(only) if only else InstallMode.ALL, @@ -1798,7 +1794,6 @@ def _install_apm_packages(ctx, outcome): skill_subset=ctx.skill_subset, skill_subset_from_cli=ctx.skill_subset_from_cli, refresh=ctx.refresh, - trust_canvas=ctx.trust_canvas, ) apm_count = install_result.installed_count apm_diagnostics = install_result.diagnostics @@ -1862,7 +1857,11 @@ def _install_apm_packages(ctx, outcome): logger.verbose_detail(f"Collected {len(transitive_mcp)} transitive MCP dependency(ies)") mcp_deps = MCPIntegrator.deduplicate(mcp_deps + transitive_mcp) - # -- S1/S2 fix (#827-C2/C3): enforce policy on ALL MCP deps ---- + # allowExecutables MCP gate. + from ..security.executables import filter_mcp_by_allow_executables as _fmcp + + mcp_deps = _fmcp(mcp_deps, getattr(apm_package, "allow_executables", None), logger) + # The pipeline gate phase (policy_gate.py) checks direct APM deps # and direct MCP deps from apm.yml. However, transitive MCP # servers (discovered via collect_transitive above) are only known @@ -2052,7 +2051,6 @@ def _install_apm_dependencies( # noqa: PLR0913 plan_callback=None, refresh: bool = False, lockfile_only: bool = False, - trust_canvas: bool = False, ): """Thin wrapper -- builds an :class:`InstallRequest` and delegates to :class:`apm_cli.install.service.InstallService`. @@ -2093,6 +2091,5 @@ def _install_apm_dependencies( # noqa: PLR0913 plan_callback=plan_callback, refresh=refresh, lockfile_only=lockfile_only, - trust_canvas=trust_canvas, ) return InstallService().run(request) diff --git a/src/apm_cli/commands/pack.py b/src/apm_cli/commands/pack.py index 41bbd4956..51df3ef41 100644 --- a/src/apm_cli/commands/pack.py +++ b/src/apm_cli/commands/pack.py @@ -747,17 +747,9 @@ def _render_marketplace_catalog(logger, written: list[tuple[str | None, Path]]) default=False, help="Deploy despite critical hidden-character findings.", ) -@click.option( - "--trust-canvas-extensions", - is_flag=True, - default=False, - help="Deploy executable canvas extensions (.github/extensions/) from the bundle.", -) @click.option("--verbose", "-v", is_flag=True, help="Show detailed unpacking information") @click.pass_context -def unpack_cmd( - ctx, bundle_path, output, skip_verify, dry_run, force, trust_canvas_extensions, verbose -): +def unpack_cmd(ctx, bundle_path, output, skip_verify, dry_run, force, verbose): """Extract an APM bundle into the project.""" logger = CommandLogger("unpack", verbose=verbose, dry_run=dry_run) logger.warning( @@ -773,7 +765,6 @@ def unpack_cmd( skip_verify=skip_verify, dry_run=dry_run, force=force, - trust_canvas=trust_canvas_extensions, ) # Surface bundle metadata and warn on target mismatch @@ -782,19 +773,11 @@ def unpack_cmd( if result.canvas_blocked > 0: from apm_cli.core.experimental import is_enabled - if is_enabled("canvas"): - logger.warning( - f"Blocked {result.canvas_blocked} canvas extension file(s): canvas " - "extensions are executable code and are not unpacked by default. " - "Re-run with '--trust-canvas-extensions' to deploy them to " - ".github/extensions/." - ) - else: + if not is_enabled("canvas"): logger.warning( f"Blocked {result.canvas_blocked} canvas extension file(s): canvas " "extensions are an experimental feature and are disabled. Enable " - "them with 'apm experimental enable canvas' (then re-run with " - "'--trust-canvas-extensions' to deploy executable canvas code)." + "them with 'apm experimental enable canvas'." ) if dry_run: diff --git a/src/apm_cli/core/experimental.py b/src/apm_cli/core/experimental.py index d1463c3cc..feb694ebc 100644 --- a/src/apm_cli/core/experimental.py +++ b/src/apm_cli/core/experimental.py @@ -101,8 +101,8 @@ class ExperimentalFlag: hint=( "Author a canvas under .apm/extensions//extension.mjs, then " "'apm install' deploys it to .github/extensions/. Dependency-provided " - "canvases are executable and blocked unless you pass " - "'--trust-canvas-extensions'. See " + "canvases are executable and blocked unless approved via allowExecutables " + "in apm.yml and 'apm approve '. See " "https://microsoft.github.io/apm/integrations/canvas/" ), ), diff --git a/src/apm_cli/install/context.py b/src/apm_cli/install/context.py index d1a910607..ad7e8ae57 100644 --- a/src/apm_cli/install/context.py +++ b/src/apm_cli/install/context.py @@ -63,10 +63,6 @@ class InstallContext: verbose: bool = False refresh: bool = False dev: bool = False - # --trust-canvas-extensions: opt in to deploying dependency-provided - # canvas extensions (executable Node code). First-party (root project .apm/) - # canvases deploy without this; only dependency canvases are gated. - trust_canvas: bool = False only_packages: list[str] | None = None protocol_pref: Any = None # ProtocolPreference (NONE/SSH/HTTPS) for shorthand transport allow_protocol_fallback: bool | None = None # None => read APM_ALLOW_PROTOCOL_FALLBACK env diff --git a/src/apm_cli/install/exec_gate.py b/src/apm_cli/install/exec_gate.py index 28772f24c..b1e41a101 100644 --- a/src/apm_cli/install/exec_gate.py +++ b/src/apm_cli/install/exec_gate.py @@ -21,8 +21,8 @@ def check_executable_approval( allow_executables: builtins.dict[str, builtins.dict[str, bool]] | None, *, ctx: InstallContext | None = None, -) -> tuple[bool, bool]: - """Return ``(hooks_approved, bin_approved)`` for a package. +) -> tuple[bool, bool, bool, bool]: + """Return ``(hooks_approved, bin_approved, mcp_approved, canvas_approved)`` for a package. Local project content (``_local``) is always trusted. Dependency packages are checked against the ``allowExecutables`` block. When @@ -34,11 +34,13 @@ def check_executable_approval( """ is_local = package_name == "_local" if is_local or allow_executables is None: - return True, True + return True, True, True, True from apm_cli.security.executables import ( EXEC_TYPE_BIN, + EXEC_TYPE_CANVAS, EXEC_TYPE_HOOKS, + EXEC_TYPE_MCP, build_approval_key, is_package_approved, ) @@ -61,9 +63,13 @@ def check_executable_approval( is_package_approved(allow_executables, k, EXEC_TYPE_HOOKS) for k in candidate_keys ) bin_ok = any(is_package_approved(allow_executables, k, EXEC_TYPE_BIN) for k in candidate_keys) + mcp_ok = any(is_package_approved(allow_executables, k, EXEC_TYPE_MCP) for k in candidate_keys) + canvas_ok = any( + is_package_approved(allow_executables, k, EXEC_TYPE_CANVAS) for k in candidate_keys + ) # Track blocked packages for the post-loop approval prompt. - if ctx is not None and (not hooks_ok or not bin_ok): + if ctx is not None and (not hooks_ok or not bin_ok or not mcp_ok or not canvas_ok): from apm_cli.security.executables import scan_package_executables _install = Path(package_info.install_path) @@ -75,7 +81,7 @@ def check_executable_approval( if _decl.has_executables: ctx.blocked_executables.append(_decl) - return hooks_ok, bin_ok + return hooks_ok, bin_ok, mcp_ok, canvas_ok def resolve_package_key(package_info: Any, package_name: str) -> str: diff --git a/src/apm_cli/install/local_bundle_handler.py b/src/apm_cli/install/local_bundle_handler.py index 627068329..bb9202bbb 100644 --- a/src/apm_cli/install/local_bundle_handler.py +++ b/src/apm_cli/install/local_bundle_handler.py @@ -44,7 +44,7 @@ def install_local_bundle( logger, legacy_skill_paths: bool = False, rejected_flags: dict[str, object], - trust_canvas: bool = False, + allow_executables: dict | None = None, ) -> None: """Deploy a local bundle into project / user scope. @@ -138,7 +138,7 @@ def install_local_bundle( logger=logger, scope=scope, alias=alias, - trust_canvas=trust_canvas, + allow_executables=allow_executables, ) deployed = result.get("deployed_files", []) diff --git a/src/apm_cli/install/phases/integrate.py b/src/apm_cli/install/phases/integrate.py index 8d7a62652..b8dc87592 100644 --- a/src/apm_cli/install/phases/integrate.py +++ b/src/apm_cli/install/phases/integrate.py @@ -468,41 +468,37 @@ def _run_executable_approval_prompt(ctx: InstallContext) -> None: After the integration loop, any package that had hooks or bin/ blocked is collected in ``ctx.blocked_executables``. This function runs the interactive approval flow (or hard-errors in CI) and - persists approved entries to ``apm.yml`` so the next install - deploys them. + persists approved entries to ``~/.apm/approvals.yml`` (user-local, + never committed to source control) so the next install deploys them. """ if not ctx.blocked_executables: return from apm_cli.security.executables import ( + load_user_approvals, prompt_executable_approval, - write_allow_executables, + save_user_approvals, ) - allow_exec = None - if ctx.apm_package is not None: - allow_exec = getattr(ctx.apm_package, "allow_executables", None) + # Seed the prompt with existing user-local approvals (not project entries, + # which are read-only from the install pipeline's perspective). + allow_exec = load_user_approvals() or {} updated = prompt_executable_approval( ctx.blocked_executables, allow_executables=allow_exec, ) - # Persist approvals to apm.yml if user approved anything new. - if updated and updated != (allow_exec or {}): - manifest_path = ctx.source_root or ctx.project_root - apm_yml = manifest_path / "apm.yml" - if apm_yml.is_file(): - write_allow_executables(apm_yml, updated) - # Update the in-memory model so subsequent code sees the change. - if ctx.apm_package is not None: - ctx.apm_package.allow_executables = updated - if ctx.logger: - ctx.logger.info( - "Updated allowExecutables in apm.yml. " - "Run 'apm install' again to deploy approved executables.", - symbol="info", - ) + # Persist new approvals to user-local file if user approved anything new. + if updated and updated != allow_exec: + save_user_approvals(updated) + if ctx.logger: + _new_count = sum(1 for _k in updated if _k not in allow_exec) + ctx.logger.info( + f"Updated ~/.apm/approvals.yml ({_new_count} newly approved). " + "Run 'apm install' again to deploy approved executables.", + symbol="info", + ) # ====================================================================== diff --git a/src/apm_cli/install/pipeline.py b/src/apm_cli/install/pipeline.py index 7bb9c6b47..dd392b064 100644 --- a/src/apm_cli/install/pipeline.py +++ b/src/apm_cli/install/pipeline.py @@ -418,7 +418,6 @@ def run_install_pipeline( # noqa: PLR0913, RUF100 plan_callback=None, refresh: bool = False, lockfile_only: bool = False, - trust_canvas: bool = False, ): """Install APM package dependencies. @@ -548,7 +547,6 @@ def run_install_pipeline( # noqa: PLR0913, RUF100 legacy_skill_paths=legacy_skill_paths, refresh=refresh, lockfile_only=lockfile_only, - trust_canvas=trust_canvas, ) # ------------------------------------------------------------------ diff --git a/src/apm_cli/install/request.py b/src/apm_cli/install/request.py index f1a0eb989..31270dbea 100644 --- a/src/apm_cli/install/request.py +++ b/src/apm_cli/install/request.py @@ -63,12 +63,6 @@ class InstallRequest: # --refresh only forces re-resolution without discarding orphans. refresh: bool = False - # --trust-canvas-extensions: opt in to deploying canvas extensions - # shipped by dependencies. Canvas extensions are executable Node code, - # so dependency-provided ones are blocked by default; first-party - # (root project .apm/) canvases always deploy once the experimental flag is on. - trust_canvas: bool = False - # Plan-gate hook: if set, run_install_pipeline invokes this callable # AFTER resolve completes and BEFORE downloads begin, passing the # computed UpdatePlan. The callable returns True to proceed or diff --git a/src/apm_cli/install/service.py b/src/apm_cli/install/service.py index 6b8155499..f01b75247 100644 --- a/src/apm_cli/install/service.py +++ b/src/apm_cli/install/service.py @@ -93,7 +93,6 @@ def run(self, request: InstallRequest) -> InstallResult: plan_callback=request.plan_callback, refresh=request.refresh, lockfile_only=request.lockfile_only, - trust_canvas=request.trust_canvas, ) @staticmethod diff --git a/src/apm_cli/install/services.py b/src/apm_cli/install/services.py index 45255c786..d428ec471 100644 --- a/src/apm_cli/install/services.py +++ b/src/apm_cli/install/services.py @@ -188,7 +188,7 @@ def _check_executable_approval( allow_executables: builtins.dict[str, builtins.dict[str, bool]] | None, *, ctx: InstallContext | None = None, -) -> tuple[bool, bool]: +) -> tuple[bool, bool, bool, bool]: """Delegate to ``exec_gate.check_executable_approval``.""" from apm_cli.install.exec_gate import check_executable_approval @@ -227,6 +227,27 @@ def _log_hooks_skip( ) +def _log_canvas_skip(package_name: str, package_info: Any, logger: InstallLogger | None) -> None: + """Warn about skipped canvas extensions when the package ships them.""" + _install = Path(package_info.install_path) + extensions_root = _install / ".apm" / "extensions" + try: + has_canvas = extensions_root.is_dir() and any( + (d / "extension.mjs").is_file() for d in extensions_root.iterdir() if d.is_dir() + ) + except OSError: + has_canvas = False + if not has_canvas: + return + _pkg_label = package_name or getattr(package_info, "name", "unknown") + if logger: + logger.warning( + f"{_pkg_label}: canvas extension(s) skipped (not approved in allowExecutables). " + f"Run 'apm approve {_pkg_label}' to approve.", + symbol="warning", + ) + + def integrate_package_primitives( # noqa: PLR0913 package_info: Any, project_root: Path, @@ -261,9 +282,9 @@ def integrate_package_primitives( # noqa: PLR0913 contain non-skill primitives when the cowork target is active. When *allow_executables* is provided, executable primitives (hooks, - bin/) are only deployed for packages whose key appears in the dict - with the matching type set to ``True``. Local project content - (``package_name == "_local"``) is always trusted. + bin/, MCP servers, canvas extensions) are only deployed for packages + whose key appears in the dict with the matching type set to ``True``. + Local project content (``package_name == "_local"``) is always trusted. Returns a dict with integration counters and the list of deployed file paths. """ @@ -307,8 +328,9 @@ def integrate_package_primitives( # noqa: PLR0913 # OR sit inside it. ensure_path_within(child, parent) raises if not. ensure_path_within(Path(project_root).resolve(), scratch_root) - # Executable approval gate (npm v12-style default-deny). - _hooks_approved, _bin_approved = _check_executable_approval( + # Executable approval gate (npm v12-style default-deny). hooks/bin gate + # below (~424, ~585); mcp/canvas unused (mcp filtered upstream, canvas re-derived ~433). + _hooks_approved, _bin_approved, _mcp_approved, _canvas_approved = _check_executable_approval( package_name, package_info, allow_executables, ctx=ctx ) @@ -403,6 +425,25 @@ def _format_target_collapse(paths: list[str], verbose: bool) -> tuple[str, list[ if _prim_name == "hooks" and not _hooks_approved: _log_hooks_skip(package_name, package_info, targets, logger) continue + # Executable approval gate: skip canvas if not approved. + # First-party (is_first_party=True) always deploys. + # Dependency canvas requires allowExecutables approval; the _local + # name shortcut in check_executable_approval does NOT bypass this + # gate when is_first_party=False (defence-in-depth: a malicious dep + # named _local must not bypass canvas trust via the name alone). + if _prim_name == "canvas" and not is_first_party: + if allow_executables is None: + _dep_canvas_ok = True + else: + from apm_cli.install.exec_gate import resolve_package_key as _rpk + from apm_cli.security.executables import EXEC_TYPE_CANVAS, is_package_approved + + _dep_canvas_ok = is_package_approved( + allow_executables, _rpk(package_info, package_name), EXEC_TYPE_CANVAS + ) or is_package_approved(allow_executables, package_name, EXEC_TYPE_CANVAS) + if not _dep_canvas_ok: + _log_canvas_skip(package_name, package_info, logger) + continue _integrator = _INTEGRATOR_KWARGS[_prim_name] # A primitive can be statically present on a target (e.g. the # copilot canvas mapping) while a given IntegratorBundle omits its @@ -431,15 +472,12 @@ def _format_target_collapse(paths: list[str], verbose: bool) -> tuple[str, list[ # don't accept this kwarg, so include it only for hooks. if _prim_name == "hooks": _call_kwargs["user_scope"] = scope is InstallScope.USER - # Canvas alone needs the trust signal: dependency-provided - # canvases are executable code blocked unless the operator - # passed --trust-canvas-extensions. First-party (root/local) - # status is decided by the CALL PATH, not by a package-name - # string a dependency could spoof: only integrate_local_content - # passes is_first_party=True. Every dependency call defaults to - # False, so a dependency canvas always requires the trust flag. + # Canvas integration: always pass is_first_party. Approval + # is enforced by the gate above (canvas already skipped if + # not approved and not is_first_party), so here we always + # pass trust_canvas=True to let the integrator proceed. if _prim_name == "canvas": - _call_kwargs["trust_canvas"] = bool(getattr(ctx, "trust_canvas", False)) + _call_kwargs["trust_canvas"] = True _call_kwargs["is_first_party"] = is_first_party _call_kwargs["package_name"] = package_name _int_result = getattr(_integrator, _entry.integrate_method)( @@ -710,7 +748,7 @@ def integrate_local_bundle( logger: InstallLogger | None = None, scope: InstallScope | None = None, alias: str | None = None, - trust_canvas: bool = False, + allow_executables: builtins.dict[str, builtins.dict[str, bool]] | None = None, ) -> dict: """Integrate a detected local bundle into project / user scope. @@ -740,11 +778,11 @@ def integrate_local_bundle( logger: Install-flow logger. scope: ``InstallScope`` (project vs user) for downstream consumers. alias: Slug override from ``--as``. - trust_canvas: When ``True``, allow executable canvas extension - bundles (``extensions//extension.mjs``) to deploy from the - offline bundle. Defaults to ``False`` (fail closed) so a - vendored bundle cannot smuggle executable canvas code past the - dependency trust gate. + allow_executables: The ``allowExecutables`` block from the consuming + project's ``apm.yml``. When ``None`` (no enforcement), all + executable primitives including canvas are allowed. When + provided, canvas extensions from the bundle are only deployed if + the bundle slug is approved for the ``canvas`` exec type. Returns: Dict with keys ``deployed_files`` (list[str]), @@ -809,33 +847,39 @@ def integrate_local_bundle( # Security + feature gate: canvas extensions are executable Node bundles # (``extension.mjs``). A local / offline bundle copies its files - # verbatim WITHOUT routing through ``CanvasIntegrator``, so neither the - # experimental feature flag nor its trust gate would otherwise apply - # here. Require BOTH gates: the ``canvas`` experimental flag must be ON - # (feature availability) AND ``--trust-canvas-extensions`` must be set - # (executable-code trust). Fail closed -- drop canvas paths when either - # gate is missing. + # verbatim WITHOUT routing through ``CanvasIntegrator``, so the + # experimental feature flag and the allowExecutables gate must be + # checked explicitly here. When the canvas feature flag is OFF, drop + # paths silently (no-op; canvas type does not exist yet). When ON, + # check allowExecutables: if no enforcement block is present (None) + # canvas deploys freely; otherwise the bundle slug must be approved for + # the ``canvas`` exec type. from ..core.experimental import is_enabled from ..integration.canvas_integrator import is_canvas_bundle_path _canvas_enabled = is_enabled("canvas") - if not (_canvas_enabled and trust_canvas): + if _canvas_enabled: + from ..security.executables import EXEC_TYPE_CANVAS, is_package_approved + + _canvas_approved_bundle = allow_executables is None or is_package_approved( + allow_executables, slug, EXEC_TYPE_CANVAS + ) + else: + _canvas_approved_bundle = False + + if not (_canvas_enabled and _canvas_approved_bundle): _blocked = sorted(r for r in pack_files if is_canvas_bundle_path(r)) if _blocked: for _r in _blocked: pack_files.pop(_r, None) - # Flag ON but untrusted: this is a genuine blocked deploy, so - # count it as skipped and surface the trust opt-in. Flag OFF: - # the canvas type does not exist yet, so drop the paths silently - # WITHOUT inflating the skip count -- mirroring the silent no-op - # of the normal CanvasIntegrator path when the flag is off. if _canvas_enabled: + # Canvas feature on but not approved: block and surface message. skipped += len(_blocked) _msg = ( f"Blocked {len(_blocked)} canvas extension file(s) from bundle " f"'{slug}': canvas extensions are executable extension.mjs code " - "and are not deployed from bundles by default. Re-run with " - "'--trust-canvas-extensions' to deploy them to .github/extensions/." + f"and are not approved in allowExecutables. " + f"Run 'apm approve {slug}' to approve them." ) if diagnostics is not None: diagnostics.warn(message=_msg, package=str(slug)) diff --git a/src/apm_cli/install/template.py b/src/apm_cli/install/template.py index d8b6579f3..2a95510c0 100644 --- a/src/apm_cli/install/template.py +++ b/src/apm_cli/install/template.py @@ -13,10 +13,40 @@ from __future__ import annotations +import contextlib + from apm_cli.install.helpers.security_scan import _pre_deploy_security_scan from apm_cli.install.services import IntegratorBundle, integrate_package_primitives from apm_cli.install.sources import DependencySource, Materialization +_EFFECTIVE_ALLOW_UNSET = object() + + +def _effective_allow(ctx) -> dict | None: + """Return the effective allowExecutables map for the install context. + + Reads the gate opt-in signal from the project ``apm.yml`` via + ``ctx.apm_package.allow_executables`` and merges it with the + user-local approvals from ``~/.apm/approvals.yml``. Returns + ``None`` when the gate is disabled (backward-compatible behaviour). + + The merged map is memoized on ``ctx`` for the duration of the install + run: it is read once per dependency during materialization and the + approvals file does not change mid-run, so re-reading it per dep is + pure overhead. + """ + cached = getattr(ctx, "_effective_allow_cache", _EFFECTIVE_ALLOW_UNSET) + if cached is not _EFFECTIVE_ALLOW_UNSET: + return cached + + from apm_cli.security.executables import effective_allow_executables + + project_val = getattr(getattr(ctx, "apm_package", None), "allow_executables", None) + result = effective_allow_executables(project_val) + with contextlib.suppress(Exception): + ctx._effective_allow_cache = result + return result + def run_integration_template( source: DependencySource, @@ -101,7 +131,7 @@ def _integrate_materialization( else (tuple(dep_ref.skill_subset) if dep_ref.skill_subset else None) ), ctx=ctx, - allow_executables=getattr(getattr(ctx, "apm_package", None), "allow_executables", None), + allow_executables=_effective_allow(ctx), ) mutation_keys = ( "prompts", diff --git a/src/apm_cli/integration/canvas_integrator.py b/src/apm_cli/integration/canvas_integrator.py index dce0f5901..44d7bfe38 100644 --- a/src/apm_cli/integration/canvas_integrator.py +++ b/src/apm_cli/integration/canvas_integrator.py @@ -10,11 +10,11 @@ * The ``canvas`` experimental feature flag turns the primitive ON at all (feature availability -- NOT a security gate). -* A trust gate protects against arbitrary executable code: a canvas +* A trust gate enforced via ``allowExecutables`` in ``apm.yml``: a canvas shipped by a *dependency* is blocked by default and requires the - operator to pass ``--trust-canvas-extensions``. The author's own - first-party (root/local) ``.apm/extensions/`` deploys freely once the - experimental flag is on. + operator to declare it in the ``allowExecutables`` block (using + ``apm approve ``). The author's own first-party (root/local) + ``.apm/extensions/`` deploys freely once the experimental flag is on. The integrator is Copilot-only. At **project scope** a canvas deploys to ``.github/extensions//``. At **user scope** (``apm install --global``) @@ -23,9 +23,9 @@ solely on the ``copilot`` target. Global canvas install is intentionally limited for the MVP: only dependency-provided canvases are supported (so the dependency lockfile tracks them and uninstall can prune them), the operator -must always pass ``--trust-canvas-extensions`` (the blast radius is the whole -account), and only the default ``~/.copilot`` location is honored (a custom -``$COPILOT_HOME`` is refused). +must approve the package via ``allowExecutables`` (the blast radius is the +whole account), and only the default ``~/.copilot`` location is honored (a +custom ``$COPILOT_HOME`` is refused). """ from __future__ import annotations @@ -171,14 +171,15 @@ def integrate_canvases_for_target( Trust model: * **Project scope** -- a dependency-provided canvas requires - *trust_canvas*; a first-party (root/local) canvas deploys freely - once the flag is on. + *trust_canvas* (resolved via ``allowExecutables`` in the calling + service); a first-party (root/local) canvas deploys freely once the + flag is on. * **User scope** (``--global``) -- only *dependency-provided* canvases - deploy, and they ALWAYS require *trust_canvas* (full-account blast - radius). First-party user-scope canvases are refused because the - user-scope lockfile pipeline does not track them, so uninstall could - not prune the executable bundle. A non-default ``$COPILOT_HOME`` is - also refused (APM deploys global canvases to ``~/.copilot`` only). + deploy, and they ALWAYS require *trust_canvas*. First-party + user-scope canvases are refused because the user-scope lockfile + pipeline does not track them, so uninstall could not prune the + executable bundle. A non-default ``$COPILOT_HOME`` is also refused + (APM deploys global canvases to ``~/.copilot`` only). """ empty = IntegrationResult(0, 0, 0, []) @@ -470,9 +471,9 @@ def _emit_trust_block( diagnostics.warn( message=( f"Blocked {len(bundles)} canvas extension(s) ({names}) from '{pkg}': " - f"canvas extensions are executable {CANVAS_MARKER} code and are not " - f"deployed from dependencies by default. Re-run with " - f"'--trust-canvas-extensions' to deploy them to {deploy_dir}." + f"canvas extensions run {CANVAS_MARKER} code and require approval " + f"before deployment. Run 'apm approve {pkg}' to record approval in " + f"~/.apm/approvals.yml and deploy them to {deploy_dir}." ), package=package_name or "", ) diff --git a/src/apm_cli/models/validation.py b/src/apm_cli/models/validation.py index 148940bc5..a7b3e128c 100644 --- a/src/apm_cli/models/validation.py +++ b/src/apm_cli/models/validation.py @@ -793,7 +793,7 @@ def _validate_apm_package_with_yml( "Canvas extension(s) found (experimental, Copilot-only): " f"{', '.join(canvas_names)}. These ship executable extension.mjs " "code; consumers must enable the 'canvas' experimental flag, and " - "dependents must pass --trust-canvas-extensions to install them." + "dependents must approve the package via 'apm approve ' to install them." ) if not has_primitives: diff --git a/src/apm_cli/security/executables.py b/src/apm_cli/security/executables.py index b39b367f2..9cc754589 100644 --- a/src/apm_cli/security/executables.py +++ b/src/apm_cli/security/executables.py @@ -1,8 +1,8 @@ """Executable primitive approval gate (npm v12-inspired opt-in model). -APM packages can declare three kinds of executable primitives -- hooks, -MCP servers, and bin/ executables -- that run arbitrary code on the -developer's machine. When the consuming project declares an +APM packages can declare four kinds of executable primitives -- hooks, +MCP servers, bin/ executables, and canvas extensions -- that run arbitrary +code on the developer's machine. When the consuming project declares an ``allowExecutables`` block in its ``apm.yml``, this module enforces a deny-by-default policy: none of these primitives are deployed unless explicitly approved. Projects that omit the block entirely get @@ -25,16 +25,15 @@ # Executable type constants used as keys in the allowExecutables block. EXEC_TYPE_HOOKS = "hooks" -EXEC_TYPE_MCP = "mcp" # Reserved for future enforcement. +EXEC_TYPE_MCP = "mcp" EXEC_TYPE_BIN = "bin" +EXEC_TYPE_CANVAS = "canvas" -# Types with active enforcement in the install gate. MCP is excluded -# because MCPIntegrator does not yet honour the approval state -- -# surfacing it in the UI would create a false-assurance control. -ENFORCED_EXEC_TYPES = (EXEC_TYPE_HOOKS, EXEC_TYPE_BIN) +# Types with active enforcement in the install gate. +ENFORCED_EXEC_TYPES = (EXEC_TYPE_HOOKS, EXEC_TYPE_BIN, EXEC_TYPE_MCP, EXEC_TYPE_CANVAS) # All recognised exec-type keys (for manifest validation). -ALL_EXEC_TYPES = (EXEC_TYPE_HOOKS, EXEC_TYPE_MCP, EXEC_TYPE_BIN) +ALL_EXEC_TYPES = (EXEC_TYPE_HOOKS, EXEC_TYPE_MCP, EXEC_TYPE_BIN, EXEC_TYPE_CANVAS) @dataclass(frozen=True) @@ -51,9 +50,11 @@ class ExecutableDeclaration: hook_count: Number of hook files discovered. mcp_count: Number of MCP server entries discovered. bin_count: Number of bin/ executables discovered. + canvas_count: Number of canvas extensions discovered. hook_details: Per-hook summaries for ``inspect`` display. mcp_details: Per-MCP-server summaries. bin_details: Per-binary summaries. + canvas_details: Per-canvas summaries. """ package_key: str @@ -63,14 +64,18 @@ class ExecutableDeclaration: hook_count: int = 0 mcp_count: int = 0 bin_count: int = 0 + canvas_count: int = 0 hook_details: list[str] = field(default_factory=list) mcp_details: list[str] = field(default_factory=list) bin_details: list[str] = field(default_factory=list) + canvas_details: list[str] = field(default_factory=list) @property def has_executables(self) -> bool: """Return True if this package declares enforced executable primitives.""" - return self.hook_count > 0 or self.bin_count > 0 + return ( + self.hook_count > 0 or self.bin_count > 0 or self.mcp_count > 0 or self.canvas_count > 0 + ) @property def exec_types(self) -> list[str]: @@ -78,8 +83,12 @@ def exec_types(self) -> list[str]: types: list[str] = [] if self.hook_count > 0: types.append(EXEC_TYPE_HOOKS) + if self.mcp_count > 0: + types.append(EXEC_TYPE_MCP) if self.bin_count > 0: types.append(EXEC_TYPE_BIN) + if self.canvas_count > 0: + types.append(EXEC_TYPE_CANVAS) return types def summary_line(self) -> str: @@ -87,8 +96,12 @@ def summary_line(self) -> str: parts: list[str] = [] if self.hook_count: parts.append(f"{self.hook_count} hook(s)") + if self.mcp_count: + parts.append(f"{self.mcp_count} MCP server(s)") if self.bin_count: parts.append(f"{self.bin_count} bin executable(s)") + if self.canvas_count: + parts.append(f"{self.canvas_count} canvas extension(s)") return ", ".join(parts) @@ -109,7 +122,7 @@ def is_package_approved( consuming project's ``apm.yml``. ``None`` means no block exists (nothing approved). package_key: The approval key (e.g. ``owner/repo#v1.0``). - exec_type: One of ``hooks``, ``mcp``, ``bin``. + exec_type: One of ``hooks``, ``mcp``, ``bin``, ``canvas``. Returns: ``True`` only when the block contains a matching entry with @@ -175,6 +188,8 @@ def scan_package_executables( - ``bin/`` directory -- bin executables - MCP is declared in the package's ``apm.yml`` under ``dependencies.mcp``, not as files -- so we parse that instead. + - ``.apm/extensions//extension.mjs`` -- canvas extension bundles + (mirrors :meth:`CanvasIntegrator.find_canvas_bundles`) Returns an :class:`ExecutableDeclaration` (may have zero counts if the package declares no executables). @@ -233,6 +248,18 @@ def scan_package_executables( except Exception: pass # Non-fatal: if we cannot parse, treat as zero MCP + # 4. Canvas extensions: .apm/extensions//extension.mjs + # Mirrors CanvasIntegrator.find_canvas_bundles marker detection. + canvas_marker = "extension.mjs" + canvas_dirs: list[Path] = [] + extensions_root = install_path / ".apm" / "extensions" + if extensions_root.is_dir(): + for ext_dir in extensions_root.iterdir(): + if ext_dir.is_dir() and (ext_dir / canvas_marker).is_file(): + canvas_dirs.append(ext_dir) + canvas_dirs = sorted(canvas_dirs) + canvas_details = [d.name for d in canvas_dirs] + return ExecutableDeclaration( package_key=key, package_name=package_name, @@ -241,9 +268,11 @@ def scan_package_executables( hook_count=len(hook_files), mcp_count=mcp_count, bin_count=len(bin_files), + canvas_count=len(canvas_dirs), hook_details=hook_details, mcp_details=mcp_details, bin_details=bin_details, + canvas_details=canvas_details, ) @@ -271,14 +300,16 @@ def prompt_executable_approval( Args: declarations: Executable declarations for packages that need approval (already filtered to only those with executables). - allow_executables: Existing ``allowExecutables`` block from - ``apm.yml`` (merged into result for packages already approved). + allow_executables: Existing approvals map (project ``apm.yml`` + ``allowExecutables`` overlaid with user-local + ``~/.apm/approvals.yml``); merged into result for packages + already approved. trust_all: When True, auto-approve everything without prompting. no_executables: When True, deny everything without prompting. Returns: - Updated ``allowExecutables`` dict ready to write back to - ``apm.yml``. + Updated approvals dict ready to persist to the user-local + ``~/.apm/approvals.yml``. Raises: SystemExit: In non-interactive mode when unapproved executables @@ -381,7 +412,7 @@ def parse_allow_executables(data: dict[str, Any]) -> dict[str, dict[str, bool]] if not isinstance(raw, dict): raise ValueError( "allowExecutables must be a mapping of " - "package keys to {hooks: bool, mcp: bool, bin: bool}" + "package keys to {hooks: bool, mcp: bool, bin: bool, canvas: bool}" ) result: dict[str, dict[str, bool]] = {} @@ -420,6 +451,12 @@ def write_allow_executables( Reads the existing YAML, updates the ``allowExecutables`` key, and writes it back using the standard ``dump_yaml`` helper. + + Note: individual package approvals should live in the user-local + ``~/.apm/approvals.yml`` (see :func:`save_user_approvals`), not in the + project manifest which is committed to source control. This function is + retained for writing the gate opt-in signal (``allowExecutables: {}``) and + for CI/automated contexts that intentionally commit approvals. """ from ..utils.yaml_io import dump_yaml, load_yaml @@ -433,3 +470,150 @@ def write_allow_executables( del data["allowExecutables"] dump_yaml(data, manifest_path) + + +# ------------------------------------------------------------------- +# User-local approvals store (~/.apm/approvals.yml) +# ------------------------------------------------------------------- + + +def get_user_approvals_path() -> Path: + """Return the path to the user-local executable approvals file. + + The file lives at ``~/.apm/approvals.yml`` and is never committed to + source control. It stores the same ``allowExecutables`` mapping as the + project ``apm.yml`` but is scoped to the current user's machine, so + cloning a project does not implicitly grant trust to its dependencies. + """ + return Path.home() / ".apm" / "approvals.yml" + + +def load_user_approvals() -> dict[str, dict[str, bool]]: + """Load the user-local approvals from ``~/.apm/approvals.yml``. + + Returns an empty dict when the file does not exist. The file stores the + approvals mapping directly (package key -> exec-type booleans), without + the ``allowExecutables:`` YAML wrapper used in project ``apm.yml``. + """ + path = get_user_approvals_path() + if not path.is_file(): + return {} + from ..utils.yaml_io import load_yaml + + data = load_yaml(path) + if not isinstance(data, dict): + return {} + # Defensive: a corrupt or hand-tampered approvals file must never weaken + # or crash the gate. Keep only well-formed entries (str key -> dict of + # bool); silently drop anything malformed so the caller sees a clean map. + cleaned: dict[str, dict[str, bool]] = {} + for key, entry in data.items(): + if ( + isinstance(key, str) + and isinstance(entry, dict) + and all(isinstance(k, str) for k in entry) + and all(isinstance(v, bool) for v in entry.values()) + ): + cleaned[key] = entry + return cleaned + + +def save_user_approvals(approvals: dict[str, dict[str, bool]]) -> None: + """Persist *approvals* to ``~/.apm/approvals.yml``. + + Creates ``~/.apm/`` if it does not exist. Both the directory (mode + ``0o700``) and the file (mode ``0o600``) are owner-only to prevent + other users on a shared system from reading the approval list. + """ + import contextlib + import os + + from ..utils.yaml_io import dump_yaml + + path = get_user_approvals_path() + path.parent.mkdir(parents=True, exist_ok=True) + with contextlib.suppress(NotImplementedError, OSError): + os.chmod(path.parent, 0o700) + dump_yaml(approvals, path) + with contextlib.suppress(NotImplementedError, OSError): + os.chmod(path, 0o600) + + +def effective_allow_executables( + project_allow_executables: dict[str, dict[str, bool]] | None, +) -> dict[str, dict[str, bool]] | None: + """Return the effective allowExecutables map for an install run. + + Merges the project-level gate signal with the user-local approvals: + + - Returns ``None`` when the project has no ``allowExecutables`` block + (gate disabled -- backward-compatible behaviour, all executables + deployed). + - Returns a merged dict when the gate is enabled: project-level entries + (retained for CI / automated pipelines) are overlaid with user-local + approvals from ``~/.apm/approvals.yml``. User approvals take + precedence so an ``apm approve`` decision is always honoured even if the + project entry is absent or stale. + """ + if project_allow_executables is None: + return None + user = load_user_approvals() + return {**project_allow_executables, **user} + + +def filter_mcp_by_allow_executables( + mcp_deps: list, + project_allow_execs: dict | None, + logger: Any, +) -> list: + """Filter MCP deps not approved in allowExecutables. Returns filtered list.""" + if project_allow_execs is None or not mcp_deps: + return mcp_deps + _allow_execs = effective_allow_executables(project_allow_execs) + if _allow_execs is None: + return mcp_deps + _filtered = [] + for _dep in mcp_deps: + _slug = _dep.name + # Fail-closed: keep a server only when it carries a name AND that name + # is approved. A falsy/missing name is treated as NOT approved so an + # unnamed dep can never slip past the gate. + if _slug and is_package_approved(_allow_execs, _slug, EXEC_TYPE_MCP): + _filtered.append(_dep) + elif _slug: + logger.verbose_detail( + f"Skipping MCP server from '{_slug}': not approved in allowExecutables. " + f"Run 'apm approve {_slug}' to approve." + ) + else: + logger.verbose_detail( + "Skipping an unnamed MCP server: not approved in allowExecutables. " + "Identify it in apm.yml and run 'apm approve ' to approve." + ) + if len(_filtered) < len(mcp_deps): + logger.warning( + f"Filtered {len(mcp_deps) - len(_filtered)} MCP server(s) not approved " + "in allowExecutables. Run 'apm approve ' to approve.", + symbol="warning", + ) + return _filtered + + +def read_bundle_allow_executables(apm_yml_path: Path, logger: Any) -> dict | None: + """Read allowExecutables from apm.yml for bundle install. Fail-closed on error.""" + try: + from ..utils.yaml_io import load_yaml # local import avoids circular at module init + + if not apm_yml_path.is_file(): + return None + data = load_yaml(apm_yml_path) + if isinstance(data, dict): + return parse_allow_executables(data) + return None + except Exception as exc: + logger.warning( + f"Could not read allowExecutables from apm.yml: {exc}. " + "Treating as fully enforced with no approvals.", + symbol="warning", + ) + return {} diff --git a/tests/integration/test_executables_gate_integration.py b/tests/integration/test_executables_gate_integration.py index e8bc2d21e..272245b38 100644 --- a/tests/integration/test_executables_gate_integration.py +++ b/tests/integration/test_executables_gate_integration.py @@ -23,6 +23,7 @@ ALL_EXEC_TYPES, ENFORCED_EXEC_TYPES, EXEC_TYPE_BIN, + EXEC_TYPE_CANVAS, EXEC_TYPE_HOOKS, EXEC_TYPE_MCP, ExecutableDeclaration, @@ -131,9 +132,9 @@ def test_scan_mcp_from_apm_yml(self, tmp_path: Path) -> None: decl = scan_package_executables(tmp_path, "mcp-pkg", "1.0") assert decl.mcp_count == 2 - # MCP is not in enforced types - assert decl.has_executables is False - assert EXEC_TYPE_MCP not in decl.exec_types + # MCP is now an enforced executable type + assert decl.has_executables is True + assert EXEC_TYPE_MCP in decl.exec_types def test_scan_package_key_format(self, tmp_path: Path) -> None: decl = scan_package_executables(tmp_path, "owner/repo", "v2.1.0") @@ -307,33 +308,41 @@ def _make_pkg_info(self, tmp_path: Path, name: str, version: str) -> MagicMock: def test_local_always_approved(self, tmp_path: Path) -> None: pkg_info = self._make_pkg_info(tmp_path, "_local", "") - hooks_ok, bin_ok = check_executable_approval("_local", pkg_info, {"deny-all": {}}) + hooks_ok, bin_ok, mcp_ok, canvas_ok = check_executable_approval( + "_local", pkg_info, {"deny-all": {}} + ) assert hooks_ok is True assert bin_ok is True + assert mcp_ok is True + assert canvas_ok is True def test_none_allow_executables_means_all_approved(self, tmp_path: Path) -> None: pkg_info = self._make_pkg_info(tmp_path, "any-pkg", "1.0") - hooks_ok, bin_ok = check_executable_approval("any-pkg", pkg_info, None) + hooks_ok, bin_ok, mcp_ok, canvas_ok = check_executable_approval("any-pkg", pkg_info, None) assert hooks_ok is True assert bin_ok is True + assert mcp_ok is True + assert canvas_ok is True def test_empty_allow_executables_blocks_all(self, tmp_path: Path) -> None: pkg_info = self._make_pkg_info(tmp_path, "pkg", "1.0") - hooks_ok, bin_ok = check_executable_approval("pkg", pkg_info, {}) + hooks_ok, bin_ok, mcp_ok, canvas_ok = check_executable_approval("pkg", pkg_info, {}) assert hooks_ok is False assert bin_ok is False + assert mcp_ok is False + assert canvas_ok is False def test_approved_package_passes(self, tmp_path: Path) -> None: pkg_info = self._make_pkg_info(tmp_path, "pkg", "1.0") allow = {"pkg#1.0": {"hooks": True, "bin": True}} - hooks_ok, bin_ok = check_executable_approval("pkg", pkg_info, allow) + hooks_ok, bin_ok, _mcp_ok, _canvas_ok = check_executable_approval("pkg", pkg_info, allow) assert hooks_ok is True assert bin_ok is True def test_partial_approval(self, tmp_path: Path) -> None: pkg_info = self._make_pkg_info(tmp_path, "pkg", "1.0") allow = {"pkg#1.0": {"hooks": True, "bin": False}} - hooks_ok, bin_ok = check_executable_approval("pkg", pkg_info, allow) + hooks_ok, bin_ok, _mcp_ok, _canvas_ok = check_executable_approval("pkg", pkg_info, allow) assert hooks_ok is True assert bin_ok is False @@ -344,7 +353,7 @@ def test_dep_ref_key_takes_priority(self, tmp_path: Path) -> None: pkg_info.dependency_ref = dep_ref allow = {"github:owner/repo#v1.0": {"hooks": True, "bin": True}} - hooks_ok, bin_ok = check_executable_approval("pkg", pkg_info, allow) + hooks_ok, bin_ok, _mcp_ok, _canvas_ok = check_executable_approval("pkg", pkg_info, allow) assert hooks_ok is True assert bin_ok is True @@ -356,7 +365,7 @@ def test_fallback_to_name_version_key(self, tmp_path: Path) -> None: # Approved under name#version, not dep-ref allow = {"pkg#1.0": {"hooks": True, "bin": True}} - hooks_ok, bin_ok = check_executable_approval("pkg", pkg_info, allow) + hooks_ok, bin_ok, _mcp_ok, _canvas_ok = check_executable_approval("pkg", pkg_info, allow) assert hooks_ok is True assert bin_ok is True @@ -507,8 +516,9 @@ def test_enforced_types_subset_of_all(self) -> None: for t in ENFORCED_EXEC_TYPES: assert t in ALL_EXEC_TYPES - def test_mcp_not_in_enforced(self) -> None: - assert EXEC_TYPE_MCP not in ENFORCED_EXEC_TYPES + def test_mcp_and_canvas_in_enforced(self) -> None: + assert EXEC_TYPE_MCP in ENFORCED_EXEC_TYPES + assert EXEC_TYPE_CANVAS in ENFORCED_EXEC_TYPES def test_hooks_and_bin_in_enforced(self) -> None: assert EXEC_TYPE_HOOKS in ENFORCED_EXEC_TYPES @@ -656,11 +666,12 @@ def test_approve_pending_all_approved(self, tmp_path: Path, monkeypatch) -> None from apm_cli.commands.approve import approve_cmd project = self._setup_project(tmp_path) - manifest = project / "apm.yml" - manifest.write_text( - "name: test\nallowExecutables:\n" - " risky-pkg#2.0.0:\n hooks: true\n" - " tool-pkg#1.0.0:\n bin: true\n" + approvals_file = tmp_path / ".apm" / "approvals.yml" + approvals_file.parent.mkdir(parents=True, exist_ok=True) + approvals_file.write_text("risky-pkg#2.0.0:\n hooks: true\ntool-pkg#1.0.0:\n bin: true\n") + monkeypatch.setattr( + "apm_cli.security.executables.get_user_approvals_path", + lambda: approvals_file, ) monkeypatch.chdir(project) @@ -676,6 +687,11 @@ def test_approve_specific_package(self, tmp_path: Path, monkeypatch) -> None: from apm_cli.commands.approve import approve_cmd project = self._setup_project(tmp_path) + approvals_file = tmp_path / ".apm" / "approvals.yml" + monkeypatch.setattr( + "apm_cli.security.executables.get_user_approvals_path", + lambda: approvals_file, + ) monkeypatch.chdir(project) runner = CliRunner() @@ -685,12 +701,14 @@ def test_approve_specific_package(self, tmp_path: Path, monkeypatch) -> None: assert "Approved" in result.output assert "risky-pkg" in result.output - # Verify manifest was updated + # Verify the user-local approvals file was updated, not project apm.yml from apm_cli.utils.yaml_io import load_yaml - data = load_yaml(project / "apm.yml") - assert "allowExecutables" in data - assert "risky-pkg#2.0.0" in data["allowExecutables"] + assert approvals_file.is_file() + data = load_yaml(approvals_file) + assert "risky-pkg#2.0.0" in data + project_data = load_yaml(project / "apm.yml") + assert "allowExecutables" not in project_data def test_approve_all(self, tmp_path: Path, monkeypatch) -> None: from click.testing import CliRunner @@ -698,6 +716,11 @@ def test_approve_all(self, tmp_path: Path, monkeypatch) -> None: from apm_cli.commands.approve import approve_cmd project = self._setup_project(tmp_path) + approvals_file = tmp_path / ".apm" / "approvals.yml" + monkeypatch.setattr( + "apm_cli.security.executables.get_user_approvals_path", + lambda: approvals_file, + ) monkeypatch.chdir(project) runner = CliRunner() @@ -708,8 +731,8 @@ def test_approve_all(self, tmp_path: Path, monkeypatch) -> None: from apm_cli.utils.yaml_io import load_yaml - data = load_yaml(project / "apm.yml") - allow = data["allowExecutables"] + assert approvals_file.is_file() + allow = load_yaml(approvals_file) assert "risky-pkg#2.0.0" in allow assert "tool-pkg#1.0.0" in allow @@ -761,11 +784,12 @@ def test_deny_package(self, tmp_path: Path, monkeypatch) -> None: from apm_cli.commands.approve import deny_cmd project = self._setup_project(tmp_path) - manifest = project / "apm.yml" - manifest.write_text( - "name: test\nallowExecutables:\n" - " risky-pkg#2.0.0:\n hooks: true\n" - " tool-pkg#1.0.0:\n bin: true\n" + approvals_file = tmp_path / ".apm" / "approvals.yml" + approvals_file.parent.mkdir(parents=True, exist_ok=True) + approvals_file.write_text("risky-pkg#2.0.0:\n hooks: true\ntool-pkg#1.0.0:\n bin: true\n") + monkeypatch.setattr( + "apm_cli.security.executables.get_user_approvals_path", + lambda: approvals_file, ) monkeypatch.chdir(project) @@ -777,10 +801,10 @@ def test_deny_package(self, tmp_path: Path, monkeypatch) -> None: from apm_cli.utils.yaml_io import load_yaml - data = load_yaml(manifest) - assert "risky-pkg#2.0.0" not in data.get("allowExecutables", {}) + data = load_yaml(approvals_file) + assert "risky-pkg#2.0.0" not in data # tool-pkg should still be there - assert "tool-pkg#1.0.0" in data["allowExecutables"] + assert "tool-pkg#1.0.0" in data def test_deny_nonexistent_package(self, tmp_path: Path, monkeypatch) -> None: from click.testing import CliRunner @@ -811,18 +835,26 @@ def test_approve_no_manifest_errors(self, tmp_path: Path, monkeypatch) -> None: assert result.exit_code == 1 assert "No apm.yml" in result.output - def test_deny_no_manifest_errors(self, tmp_path: Path, monkeypatch) -> None: + def test_deny_no_manifest_succeeds(self, tmp_path: Path, monkeypatch) -> None: from click.testing import CliRunner from apm_cli.commands.approve import deny_cmd + # deny no longer requires a project manifest: it operates purely on the + # user-local approvals store. With no approvals recorded, the package is + # simply reported as not found and the command exits cleanly. + approvals_file = tmp_path / ".apm" / "approvals.yml" + monkeypatch.setattr( + "apm_cli.security.executables.get_user_approvals_path", + lambda: approvals_file, + ) monkeypatch.chdir(tmp_path) runner = CliRunner() result = runner.invoke(deny_cmd, ["pkg"]) - assert result.exit_code == 1 - assert "No apm.yml" in result.output + assert result.exit_code == 0 + assert "not found" in result.output def test_approve_all_already_approved(self, tmp_path: Path, monkeypatch) -> None: from click.testing import CliRunner @@ -830,11 +862,12 @@ def test_approve_all_already_approved(self, tmp_path: Path, monkeypatch) -> None from apm_cli.commands.approve import approve_cmd project = self._setup_project(tmp_path) - manifest = project / "apm.yml" - manifest.write_text( - "name: test\nallowExecutables:\n" - " risky-pkg#2.0.0:\n hooks: true\n" - " tool-pkg#1.0.0:\n bin: true\n" + approvals_file = tmp_path / ".apm" / "approvals.yml" + approvals_file.parent.mkdir(parents=True, exist_ok=True) + approvals_file.write_text("risky-pkg#2.0.0:\n hooks: true\ntool-pkg#1.0.0:\n bin: true\n") + monkeypatch.setattr( + "apm_cli.security.executables.get_user_approvals_path", + lambda: approvals_file, ) monkeypatch.chdir(project) diff --git a/tests/spec_conformance/test_manifest_reqs.py b/tests/spec_conformance/test_manifest_reqs.py index d5497e44b..6bfcbeedb 100644 --- a/tests/spec_conformance/test_manifest_reqs.py +++ b/tests/spec_conformance/test_manifest_reqs.py @@ -1,6 +1,6 @@ """Manifest (apm.yml) + scheme + tag + conformance-class tests. -Covers req-mf-001..021, req-ext-001..002, req-sc-001..008, +Covers req-mf-001..021, req-ext-001..002, req-sc-001..010, req-tg-001..004, req-cf-001..002. Every requirement is exercised either by (a) schema validation @@ -307,7 +307,22 @@ def test_consumer_should_refuse_credential_on_non_https_git_over_http(): ) -# --- req-tg-001..004: targets ----------------------------------------- +@pytest.mark.req("req-sc-009") +def test_consumer_must_deny_executable_primitive_without_allowexecutables_approval(): + assert_spec_contains( + "deny deployment of any executable primitive", + "allowExecutables", + "fail closed", + ) + + +@pytest.mark.req("req-sc-010") +def test_consumer_must_persist_approvals_user_locally_not_in_project_manifest(): + assert_spec_contains( + "persist per-user approval decisions", + "isolated from the project manifest", + "MUST NOT write interactive approval decisions into the project", + ) @pytest.mark.req("req-tg-001") diff --git a/tests/unit/commands/test_approve_deny.py b/tests/unit/commands/test_approve_deny.py index bf271ee0f..b54508d54 100644 --- a/tests/unit/commands/test_approve_deny.py +++ b/tests/unit/commands/test_approve_deny.py @@ -9,6 +9,7 @@ from __future__ import annotations from pathlib import Path +from unittest.mock import patch import yaml from click.testing import CliRunner @@ -34,6 +35,12 @@ def _write_manifest(tmpdir: str, extra: dict | None = None) -> Path: return manifest +def _write_user_approvals(approvals_path: Path, data: dict) -> None: + """Write approval data to the given (mocked) user approvals file.""" + approvals_path.parent.mkdir(parents=True, exist_ok=True) + approvals_path.write_text(yaml.dump(data)) + + def _create_pkg_with_hooks(apm_modules: Path, name: str) -> None: """Create a package directory with a hook file.""" pkg_dir = apm_modules / name @@ -117,7 +124,9 @@ def test_pending_with_unapproved_packages(self) -> None: assert result.exit_code == 0 assert "hook-pkg" in result.output - def test_approve_all(self) -> None: + def test_approve_all_writes_user_file(self, tmp_path: Path) -> None: + """apm approve --all must write to ~/.apm/approvals.yml, not project apm.yml.""" + approvals_file = tmp_path / ".apm" / "approvals.yml" runner = CliRunner() with runner.isolated_filesystem(): _write_manifest(".") @@ -125,26 +134,43 @@ def test_approve_all(self) -> None: _create_pkg_with_hooks(apm_modules, "hook-pkg") _create_pkg_with_bin(apm_modules, "bin-pkg") - result = runner.invoke(approve_cmd, ["--all"]) + with patch( + "apm_cli.security.executables.get_user_approvals_path", + return_value=approvals_file, + ): + result = runner.invoke(approve_cmd, ["--all"]) + assert result.exit_code == 0 assert "Approved" in result.output - # Verify it wrote to apm.yml + # Verify written to user-local file, NOT project apm.yml from apm_cli.utils.yaml_io import load_yaml - data = load_yaml(Path("apm.yml")) - assert "allowExecutables" in data + project_data = load_yaml(Path("apm.yml")) + assert "allowExecutables" not in project_data - def test_approve_specific_package(self) -> None: + assert approvals_file.is_file() + user_data = load_yaml(approvals_file) + assert isinstance(user_data, dict) + assert len(user_data) > 0 + + def test_approve_specific_package_writes_user_file(self, tmp_path: Path) -> None: + approvals_file = tmp_path / ".apm" / "approvals.yml" runner = CliRunner() with runner.isolated_filesystem(): _write_manifest(".") apm_modules = Path("apm_modules") _create_pkg_with_hooks(apm_modules, "hook-pkg") - result = runner.invoke(approve_cmd, ["hook-pkg"]) + with patch( + "apm_cli.security.executables.get_user_approvals_path", + return_value=approvals_file, + ): + result = runner.invoke(approve_cmd, ["hook-pkg"]) + assert result.exit_code == 0 assert "Approved" in result.output + assert approvals_file.is_file() def test_approve_unknown_package(self) -> None: runner = CliRunner() @@ -165,42 +191,59 @@ def test_approve_unknown_package(self) -> None: class TestDenyCmd: """Tests for the apm deny CLI command.""" - def test_deny_existing_entry(self) -> None: + def test_deny_existing_entry(self, tmp_path: Path) -> None: + """apm deny must remove from user file, not project apm.yml.""" + approvals_file = tmp_path / ".apm" / "approvals.yml" + _write_user_approvals(approvals_file, {"pkg#1.0": {"hooks": True}}) + runner = CliRunner() with runner.isolated_filesystem(): - _write_manifest( - ".", - extra={ - "allowExecutables": {"pkg#1.0": {"hooks": True}}, - }, - ) - result = runner.invoke(deny_cmd, ["pkg#1.0"]) + _write_manifest(".") + + with patch( + "apm_cli.security.executables.get_user_approvals_path", + return_value=approvals_file, + ): + result = runner.invoke(deny_cmd, ["pkg#1.0"]) + assert result.exit_code == 0 assert "Revoked" in result.output from apm_cli.utils.yaml_io import load_yaml - data = load_yaml(Path("apm.yml")) - ae = data.get("allowExecutables", {}) - assert "pkg#1.0" not in ae + user_data = load_yaml(approvals_file) or {} + assert "pkg#1.0" not in user_data + + def test_deny_prefix_match(self, tmp_path: Path) -> None: + approvals_file = tmp_path / ".apm" / "approvals.yml" + _write_user_approvals(approvals_file, {"owner/repo#v1.0": {"hooks": True}}) - def test_deny_prefix_match(self) -> None: runner = CliRunner() with runner.isolated_filesystem(): - _write_manifest( - ".", - extra={ - "allowExecutables": {"owner/repo#v1.0": {"hooks": True}}, - }, - ) - result = runner.invoke(deny_cmd, ["owner/repo"]) + _write_manifest(".") + + with patch( + "apm_cli.security.executables.get_user_approvals_path", + return_value=approvals_file, + ): + result = runner.invoke(deny_cmd, ["owner/repo"]) + assert result.exit_code == 0 assert "Revoked" in result.output - def test_deny_not_found(self) -> None: + def test_deny_not_found(self, tmp_path: Path) -> None: + approvals_file = tmp_path / ".apm" / "approvals.yml" + _write_user_approvals(approvals_file, {}) + runner = CliRunner() with runner.isolated_filesystem(): - _write_manifest(".", extra={"allowExecutables": {}}) - result = runner.invoke(deny_cmd, ["nonexistent"]) + _write_manifest(".") + + with patch( + "apm_cli.security.executables.get_user_approvals_path", + return_value=approvals_file, + ): + result = runner.invoke(deny_cmd, ["nonexistent"]) + assert result.exit_code == 0 assert "not found" in result.output diff --git a/tests/unit/commands/test_install_context.py b/tests/unit/commands/test_install_context.py index 57e2efe48..fc2cac878 100644 --- a/tests/unit/commands/test_install_context.py +++ b/tests/unit/commands/test_install_context.py @@ -58,7 +58,6 @@ class TestInstallContextFields: "protocol_pref", "allow_protocol_fallback", "trust_transitive_mcp", - "trust_canvas", "no_policy", "install_mode", "packages", @@ -122,15 +121,12 @@ def _build_minimal(self, **overrides): protocol_pref=sentinel.PROTO, allow_protocol_fallback=False, trust_transitive_mcp=False, - trust_canvas=False, no_policy=False, install_mode=sentinel.MODE, packages=(), ) defaults.update(overrides) return InstallContext(**defaults) - - def test_only_packages_defaults_to_none(self): ctx = self._build_minimal() assert ctx.only_packages is None @@ -169,7 +165,6 @@ def test_round_trip_required_fields(self): protocol_pref=sentinel.PROTO, allow_protocol_fallback=True, trust_transitive_mcp=True, - trust_canvas=True, no_policy=True, install_mode=sentinel.MODE, packages=("owner/repo",), @@ -223,7 +218,6 @@ def test_round_trip_optional_fields(self): protocol_pref=sentinel.PROTO, allow_protocol_fallback=False, trust_transitive_mcp=False, - trust_canvas=False, no_policy=False, install_mode=sentinel.MODE, packages=(), diff --git a/tests/unit/commands/test_install_context_and_resolution.py b/tests/unit/commands/test_install_context_and_resolution.py index 2512a287e..87a8bfc4f 100644 --- a/tests/unit/commands/test_install_context_and_resolution.py +++ b/tests/unit/commands/test_install_context_and_resolution.py @@ -563,7 +563,6 @@ def test_refresh_default_is_false(self) -> None: protocol_pref=None, allow_protocol_fallback=False, trust_transitive_mcp=False, - trust_canvas=False, no_policy=False, install_mode=None, packages=(), @@ -601,7 +600,6 @@ def test_context_is_mutable(self) -> None: protocol_pref=None, allow_protocol_fallback=False, trust_transitive_mcp=False, - trust_canvas=False, no_policy=False, install_mode=None, packages=(), diff --git a/tests/unit/commands/test_install_phase3.py b/tests/unit/commands/test_install_phase3.py index b49cf4131..a14aa35be 100644 --- a/tests/unit/commands/test_install_phase3.py +++ b/tests/unit/commands/test_install_phase3.py @@ -564,7 +564,6 @@ def test_refresh_default_is_false(self) -> None: protocol_pref=None, allow_protocol_fallback=False, trust_transitive_mcp=False, - trust_canvas=False, no_policy=False, install_mode=None, packages=(), @@ -602,7 +601,6 @@ def test_context_is_mutable(self) -> None: protocol_pref=None, allow_protocol_fallback=False, trust_transitive_mcp=False, - trust_canvas=False, no_policy=False, install_mode=None, packages=(), diff --git a/tests/unit/install/test_architecture_invariants.py b/tests/unit/install/test_architecture_invariants.py index d966ee12d..a50ebeac7 100644 --- a/tests/unit/install/test_architecture_invariants.py +++ b/tests/unit/install/test_architecture_invariants.py @@ -210,13 +210,14 @@ def test_install_py_under_legacy_budget(): boundary; the chdir + source-root-override mechanism lives in ``apm_cli/install/root_redirect.py`` and ``apm_cli/core/scope.py``. - Experimental canvas support raised 2085 -> 2110 to add the - ``--trust-canvas-extensions`` Click option plus its signature param, - the ``trust_canvas`` ``InstallContext`` field, and the trust-signal - wiring through ``_install_apm_dependencies`` / ``InstallRequest`` and - the local-bundle handler. All glue at the handler boundary; the - integrator and its trust gate live in - ``apm_cli/integration/canvas_integrator.py``. + ``allowExecutables`` MCP gate raised 2100 -> 2130 to add the + MCP filtering loop after ``collect_transitive`` that filters out MCP + servers from packages not approved in ``allowExecutables``. The gate + also removed ``--trust-canvas-extensions`` Click option and the + ``trust_canvas`` ``InstallContext`` field; canvas trust is now unified + under ``allowExecutables``. All logic at the handler boundary; the + actual approval check delegates to ``is_package_approved`` in + ``apm_cli/security/executables.py``. Default-registry CLI routing raised 2110 -> 2128 to wire ``_default_registry_for_cli`` into ``_validate_and_add_packages_to_apm_yml`` @@ -246,8 +247,8 @@ def test_install_py_under_legacy_budget(): install_py = Path(__file__).resolve().parents[3] / "src" / "apm_cli" / "commands" / "install.py" assert install_py.is_file() n = _line_count(install_py) - assert n <= 2100, ( - f"commands/install.py grew to {n} LOC (budget 2100). " + assert n <= 2150, ( + f"commands/install.py grew to {n} LOC (budget 2150). " "Do NOT trim cosmetically -- engage the python-architecture skill " "(.apm/skills/python-architecture/SKILL.md) and propose an " "extraction into apm_cli/install/." diff --git a/tests/unit/install/test_install_local_bundle.py b/tests/unit/install/test_install_local_bundle.py index 5f64639f6..3367a2f7c 100644 --- a/tests/unit/install/test_install_local_bundle.py +++ b/tests/unit/install/test_install_local_bundle.py @@ -518,13 +518,15 @@ def test_as_rejected_on_registry_install( class TestLocalBundleCanvasTrust: - """A vendored bundle must not smuggle an executable canvas past the gates. + """A vendored bundle must not smuggle an executable canvas past the feature gate. - Both gates must hold: the ``canvas`` experimental flag (feature - availability) and ``--trust-canvas-extensions`` (executable-code trust). + The ``canvas`` experimental flag controls feature availability. + When ON, canvas in bundles deploys freely (no allowExecutables enforcement + for the deprecated ``apm unpack`` path and the local bundle path when + the project has no allowExecutables block). """ - def test_canvas_blocked_without_trust(self, tmp_path, monkeypatch): + def test_canvas_blocked_when_feature_flag_off(self, tmp_path, monkeypatch): bundle = _make_bundle( tmp_path, files={ @@ -535,11 +537,12 @@ def test_canvas_blocked_without_trust(self, tmp_path, monkeypatch): project = _make_project(tmp_path) result = _invoke(project, monkeypatch, str(bundle), "--target", "copilot") assert result.exit_code == 0, result.output - # Agent deploys; canvas is blocked. + # Agent deploys; canvas is blocked (feature flag off). assert (project / ".github" / "agents" / "a.md").exists() assert not (project / ".github" / "extensions" / "widget").exists() - def test_canvas_deployed_with_trust_and_flag(self, tmp_path, monkeypatch): + def test_canvas_deployed_when_flag_on_no_enforcement(self, tmp_path, monkeypatch): + """Canvas deploys when the experimental flag is on and no allowExecutables enforcement.""" import apm_cli.config as _conf monkeypatch.setattr(_conf, "_config_cache", {"experimental": {"canvas": True}}) @@ -557,13 +560,12 @@ def test_canvas_deployed_with_trust_and_flag(self, tmp_path, monkeypatch): str(bundle), "--target", "copilot", - "--trust-canvas-extensions", ) assert result.exit_code == 0, result.output assert (project / ".github" / "extensions" / "widget" / "extension.mjs").exists() - def test_canvas_blocked_when_trusted_but_flag_off(self, tmp_path, monkeypatch): - """Trust alone is not enough: the experimental flag must also be on.""" + def test_canvas_blocked_when_feature_off_regardless(self, tmp_path, monkeypatch): + """When the experimental flag is off, canvas is always silently dropped.""" import apm_cli.config as _conf monkeypatch.setattr(_conf, "_config_cache", {"experimental": {}}) @@ -581,13 +583,9 @@ def test_canvas_blocked_when_trusted_but_flag_off(self, tmp_path, monkeypatch): str(bundle), "--target", "copilot", - "--trust-canvas-extensions", ) assert result.exit_code == 0, result.output assert (project / ".github" / "agents" / "a.md").exists() assert not (project / ".github" / "extensions" / "widget").exists() - # Flag OFF mirrors the silent CanvasIntegrator no-op: the canvas type - # does not exist yet, so the bundle path must not surface a - # trust-specific warning telling the operator to pass a flag they - # already passed. + # Flag OFF: canvas dropped silently -- no trust-specific warning in output. assert "trust-canvas-extensions" not in result.output diff --git a/tests/unit/integration/test_canvas_integrator.py b/tests/unit/integration/test_canvas_integrator.py index 4b87a9dbb..8f42d35bb 100644 --- a/tests/unit/integration/test_canvas_integrator.py +++ b/tests/unit/integration/test_canvas_integrator.py @@ -213,7 +213,7 @@ def test_dependency_blocked_without_trust(tmp_path: Path, enable_canvas): assert not (project / ".github" / "extensions").exists() messages = " ".join(d.message for d in diags._diagnostics) assert "widget" in messages - assert "--trust-canvas-extensions" in messages + assert "apm approve" in messages def test_dependency_deploys_with_trust(tmp_path: Path, enable_canvas): @@ -327,7 +327,7 @@ def test_user_scope_dependency_requires_trust(tmp_path: Path, enable_canvas, mon assert result.files_integrated == 0 assert not (home / ".copilot" / "extensions").exists() messages = " ".join(d.message for d in diags._diagnostics) - assert "--trust-canvas-extensions" in messages + assert "apm approve" in messages def test_user_scope_nondefault_copilot_home_blocked(tmp_path: Path, enable_canvas, monkeypatch): @@ -567,6 +567,7 @@ def test_dispatch_dependency_named_local_is_not_first_party(tmp_path: Path, enab First-party status is decided by the call path (the ``is_first_party`` kwarg), never inferred from the package name, so an attacker cannot bypass the trust gate by naming their package ``_local``. + Canvas blocking now requires allowExecutables enforcement (non-None dict). """ from apm_cli.install.services import integrate_package_primitives @@ -584,8 +585,10 @@ def test_dispatch_dependency_named_local_is_not_first_party(tmp_path: Path, enab managed_files=set(), diagnostics=diags, package_name="_local", - ctx=SimpleNamespace(trust_canvas=False, verbose=False), + ctx=SimpleNamespace(verbose=False), # is_first_party defaults to False (dependency call path) + # allow_executables enforcement active, canvas NOT approved for _local + allow_executables={"other/pkg": {"canvas": True}}, ) assert result["canvases"] == 0 @@ -610,7 +613,7 @@ def test_dispatch_first_party_flag_deploys(tmp_path: Path, enable_canvas): managed_files=set(), diagnostics=diags, package_name="owner/dep", - ctx=SimpleNamespace(trust_canvas=False, verbose=False), + ctx=SimpleNamespace(verbose=False), is_first_party=True, ) @@ -619,7 +622,7 @@ def test_dispatch_first_party_flag_deploys(tmp_path: Path, enable_canvas): def test_dispatch_dependency_deploys_with_trust(tmp_path: Path, enable_canvas): - """A dependency canvas deploys when the operator trusts canvas extensions.""" + """A dependency canvas deploys when allowExecutables is None (no enforcement).""" from apm_cli.install.services import integrate_package_primitives _make_canvas(tmp_path, "widget") @@ -636,7 +639,8 @@ def test_dispatch_dependency_deploys_with_trust(tmp_path: Path, enable_canvas): managed_files=set(), diagnostics=diags, package_name="owner/dep", - ctx=SimpleNamespace(trust_canvas=True, verbose=False), + ctx=SimpleNamespace(verbose=False), + # allow_executables=None means no enforcement; canvas deploys freely ) assert result["canvases"] == 1 diff --git a/tests/unit/security/test_executables.py b/tests/unit/security/test_executables.py index c3056fdc3..300535ac9 100644 --- a/tests/unit/security/test_executables.py +++ b/tests/unit/security/test_executables.py @@ -21,15 +21,20 @@ from apm_cli.security.executables import ( EXEC_TYPE_BIN, + EXEC_TYPE_CANVAS, EXEC_TYPE_HOOKS, EXEC_TYPE_MCP, ExecutableDeclaration, _is_fully_approved, build_approval_key, + effective_allow_executables, + filter_mcp_by_allow_executables, is_any_type_approved, is_package_approved, + load_user_approvals, parse_allow_executables, prompt_executable_approval, + save_user_approvals, scan_package_executables, write_allow_executables, ) @@ -51,9 +56,9 @@ def test_has_executables_true_with_hooks(self) -> None: assert decl.has_executables def test_has_executables_true_with_mcp_only(self) -> None: - """MCP-only packages are not flagged (MCP enforcement deferred).""" + """MCP-only packages are now flagged (MCP enforcement is active).""" decl = ExecutableDeclaration(package_key="a#1.0", package_name="a", mcp_count=1) - assert not decl.has_executables + assert decl.has_executables def test_has_executables_true_with_bin(self) -> None: decl = ExecutableDeclaration(package_key="a#1.0", package_name="a", bin_count=3) @@ -64,7 +69,7 @@ def test_exec_types_empty(self) -> None: assert decl.exec_types == [] def test_exec_types_all(self) -> None: - """exec_types only includes enforced types (hooks, bin); MCP is excluded.""" + """exec_types includes all enforced types (hooks, mcp, bin, canvas).""" decl = ExecutableDeclaration( package_key="a#1.0", package_name="a", @@ -72,7 +77,9 @@ def test_exec_types_all(self) -> None: mcp_count=1, bin_count=1, ) - assert decl.exec_types == [EXEC_TYPE_HOOKS, EXEC_TYPE_BIN] + assert EXEC_TYPE_HOOKS in decl.exec_types + assert EXEC_TYPE_BIN in decl.exec_types + assert EXEC_TYPE_MCP in decl.exec_types def test_exec_types_partial(self) -> None: decl = ExecutableDeclaration( @@ -81,7 +88,7 @@ def test_exec_types_partial(self) -> None: assert decl.exec_types == [EXEC_TYPE_HOOKS, EXEC_TYPE_BIN] def test_summary_line(self) -> None: - """summary_line only shows enforced types (hooks, bin).""" + """summary_line shows all enforced types (hooks, mcp, bin, canvas).""" decl = ExecutableDeclaration( package_key="a#1.0", package_name="a", @@ -91,7 +98,7 @@ def test_summary_line(self) -> None: ) summary = decl.summary_line() assert "2 hook(s)" in summary - assert "MCP" not in summary + assert "1 MCP server(s)" in summary assert "3 bin executable(s)" in summary def test_summary_line_hooks_only(self) -> None: @@ -249,8 +256,8 @@ def test_detects_mcp_from_apm_yml(self) -> None: ) decl = scan_package_executables(Path(tmpdir), "mcp-pkg", "1.0") assert decl.mcp_count == 2 - # MCP is scanned but not included in enforced exec_types. - assert EXEC_TYPE_MCP not in decl.exec_types + # MCP is now enforced so it appears in exec_types. + assert EXEC_TYPE_MCP in decl.exec_types assert "server-a" in decl.mcp_details def test_transitive_flag(self) -> None: @@ -280,6 +287,19 @@ def test_combined_hooks_and_bin(self) -> None: assert decl.bin_count == 1 assert decl.exec_types == [EXEC_TYPE_HOOKS, EXEC_TYPE_BIN] + def test_detects_canvas_extensions(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + ext_dir = Path(tmpdir) / ".apm" / "extensions" / "widget" + ext_dir.mkdir(parents=True) + (ext_dir / "extension.mjs").write_text("export default {};") + # A sibling directory without the marker is ignored. + (Path(tmpdir) / ".apm" / "extensions" / "nomarker").mkdir() + + decl = scan_package_executables(Path(tmpdir), "canvas-pkg", "1.0") + assert decl.canvas_count == 1 + assert "widget" in decl.canvas_details + assert EXEC_TYPE_CANVAS in decl.exec_types + # --------------------------------------------------------------------------- # parse_allow_executables @@ -516,3 +536,155 @@ def test_interactive_deny(self, mock_confirm, mock_interactive) -> None: decl = self._make_decl() result = prompt_executable_approval([decl]) assert "pkg#1.0" not in result + + +# --------------------------------------------------------------------------- +# effective_allow_executables (project + user-local merge) +# --------------------------------------------------------------------------- + + +class TestEffectiveAllowExecutables: + """Tests for effective_allow_executables merge precedence.""" + + def test_none_project_returns_none(self) -> None: + # Gate disabled -> None regardless of user approvals. + with patch( + "apm_cli.security.executables.load_user_approvals", + return_value={"x#1.0": {"mcp": True}}, + ): + assert effective_allow_executables(None) is None + + def test_user_approvals_overlay_project(self) -> None: + project = {"a#1.0": {"mcp": True}} + with patch( + "apm_cli.security.executables.load_user_approvals", + return_value={"b#1.0": {"canvas": True}}, + ): + merged = effective_allow_executables(project) + assert merged == {"a#1.0": {"mcp": True}, "b#1.0": {"canvas": True}} + + def test_user_approval_wins_on_overlap(self) -> None: + project = {"a#1.0": {"mcp": False}} + with patch( + "apm_cli.security.executables.load_user_approvals", + return_value={"a#1.0": {"mcp": True}}, + ): + merged = effective_allow_executables(project) + # User-local approval takes precedence over a stale project entry. + assert merged == {"a#1.0": {"mcp": True}} + + def test_empty_project_plus_user(self) -> None: + with patch( + "apm_cli.security.executables.load_user_approvals", + return_value={"a#1.0": {"bin": True}}, + ): + merged = effective_allow_executables({}) + assert merged == {"a#1.0": {"bin": True}} + + +# --------------------------------------------------------------------------- +# filter_mcp_by_allow_executables +# --------------------------------------------------------------------------- + + +class _FakeMcpDep: + """Minimal stand-in for an MCP dependency exposing ``.name``.""" + + def __init__(self, name: str | None) -> None: + self.name = name + + +class _RecordingLogger: + """Captures verbose_detail / warning calls for assertions.""" + + def __init__(self) -> None: + self.verbose: list[str] = [] + self.warnings: list[str] = [] + + def verbose_detail(self, message: str, *args, **kwargs) -> None: + self.verbose.append(message) + + def warning(self, message: str, *args, **kwargs) -> None: + self.warnings.append(message) + + +class TestFilterMcpByAllowExecutables: + """Tests for filter_mcp_by_allow_executables (fail-closed gate).""" + + def test_none_gate_passes_all(self) -> None: + deps = [_FakeMcpDep("a"), _FakeMcpDep("b")] + logger = _RecordingLogger() + assert filter_mcp_by_allow_executables(deps, None, logger) == deps + assert logger.warnings == [] + + def test_unapproved_filtered_out(self) -> None: + deps = [_FakeMcpDep("a")] + logger = _RecordingLogger() + with patch( + "apm_cli.security.executables.load_user_approvals", + return_value={}, + ): + result = filter_mcp_by_allow_executables(deps, {}, logger) + assert result == [] + assert logger.warnings # surfaced a remediation warning + + def test_approved_slug_passes(self) -> None: + deps = [_FakeMcpDep("a")] + logger = _RecordingLogger() + with patch( + "apm_cli.security.executables.load_user_approvals", + return_value={"a": {"mcp": True}}, + ): + result = filter_mcp_by_allow_executables(deps, {}, logger) + assert result == deps + assert logger.warnings == [] + + def test_unnamed_dep_is_fail_closed(self) -> None: + # A falsy/missing name must never bypass the gate. + deps = [_FakeMcpDep(None), _FakeMcpDep("")] + logger = _RecordingLogger() + with patch( + "apm_cli.security.executables.load_user_approvals", + return_value={"a": {"mcp": True}}, + ): + result = filter_mcp_by_allow_executables(deps, {}, logger) + assert result == [] + assert logger.warnings + + +class TestLoadUserApprovalsShapeValidation: + """``load_user_approvals`` must drop malformed entries (fail-closed).""" + + def test_drops_malformed_entries(self) -> None: + raw = { + "good": {"mcp": True, "canvas": False}, + "bad_value": {"mcp": "yes"}, + "bad_inner_key": {5: True}, + "not_a_dict": ["mcp"], + 7: {"mcp": True}, + } + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "approvals.yml" + with open(path, "w", encoding="utf-8") as handle: # yaml-io-exempt + yaml.safe_dump(raw, handle) + with patch( + "apm_cli.security.executables.get_user_approvals_path", + return_value=path, + ): + result = load_user_approvals() + assert result == {"good": {"mcp": True, "canvas": False}} + + +class TestSaveUserApprovalsDirMode: + """``save_user_approvals`` must create ``~/.apm`` as user-private (0o700).""" + + def test_creates_dir_mode_0o700(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "nested" / "approvals.yml" + with patch( + "apm_cli.security.executables.get_user_approvals_path", + return_value=path, + ): + save_user_approvals({"a": {"mcp": True}}) + assert path.exists() + assert (path.parent.stat().st_mode & 0o777) == 0o700 diff --git a/tests/unit/test_models_validation_rules.py b/tests/unit/test_models_validation_rules.py index b891daaa6..1604a4682 100644 --- a/tests/unit/test_models_validation_rules.py +++ b/tests/unit/test_models_validation_rules.py @@ -816,7 +816,7 @@ def test_canvas_emits_gated_executable_warning(self, tmp_path: Path) -> None: canvas_warns = [w for w in result.warnings if "Canvas extension" in w] assert len(canvas_warns) == 1 assert "widget" in canvas_warns[0] - assert "--trust-canvas-extensions" in canvas_warns[0] + assert "apm approve" in canvas_warns[0] def test_directory_without_marker_is_not_canvas(self, tmp_path: Path) -> None: """An extensions/ subdir lacking extension.mjs is not a canvas bundle.""" diff --git a/tests/unit/test_unpacker.py b/tests/unit/test_unpacker.py index 6063ba730..77366be1b 100644 --- a/tests/unit/test_unpacker.py +++ b/tests/unit/test_unpacker.py @@ -536,11 +536,12 @@ def test_unpack_cmd_multi_dep_logging(self, tmp_path): class TestUnpackCanvasTrust: - """Canvas extensions are executable code: unpack must enforce both gates. + """Canvas extensions are executable code: unpack must enforce the feature gate. - Two independent gates must BOTH hold before a canvas unpacks: the - ``canvas`` experimental flag (feature availability) and ``trust_canvas`` - (executable-code trust). Missing either one blocks the canvas. + The ``canvas`` experimental flag controls feature availability. + When OFF, canvas paths are silently dropped. When ON, canvas files + are allowed (``apm unpack`` is deprecated and has no project context + for allowExecutables enforcement). """ @pytest.fixture(autouse=True) @@ -557,7 +558,8 @@ def enable_canvas(self, monkeypatch): monkeypatch.setattr(_conf, "_config_cache", {"experimental": {"canvas": True}}) - def test_canvas_blocked_by_default(self, tmp_path): + def test_canvas_blocked_when_flag_off(self, tmp_path): + """Canvas is silently dropped when the experimental flag is off.""" deployed = [".github/agents/a.md", ".github/extensions/widget/extension.mjs"] bundle = _build_bundle_dir(tmp_path, deployed) output = tmp_path / "target" @@ -571,26 +573,27 @@ def test_canvas_blocked_by_default(self, tmp_path): assert not (output / ".github" / "extensions" / "widget").exists() assert (output / ".github" / "agents" / "a.md").exists() - def test_canvas_deployed_with_trust_and_flag(self, tmp_path, enable_canvas): + def test_canvas_deployed_when_flag_on(self, tmp_path, enable_canvas): + """Canvas deploys when the experimental flag is on (apm unpack has no allowExecutables).""" deployed = [".github/agents/a.md", ".github/extensions/widget/extension.mjs"] bundle = _build_bundle_dir(tmp_path, deployed) output = tmp_path / "target" output.mkdir() - result = unpack_bundle(bundle, output, trust_canvas=True) + result = unpack_bundle(bundle, output) assert result.canvas_blocked == 0 assert ".github/extensions/widget/extension.mjs" in result.files assert (output / ".github" / "extensions" / "widget" / "extension.mjs").exists() - def test_canvas_blocked_when_trusted_but_flag_off(self, tmp_path): - """Trust alone is not enough: the experimental flag must also be on.""" + def test_canvas_blocked_when_flag_off_no_trust(self, tmp_path): + """When the experimental flag is off, canvas is always blocked regardless.""" deployed = [".github/extensions/widget/extension.mjs"] bundle = _build_bundle_dir(tmp_path, deployed) output = tmp_path / "target" output.mkdir() - result = unpack_bundle(bundle, output, trust_canvas=True) + result = unpack_bundle(bundle, output) assert result.canvas_blocked == 1 assert ".github/extensions/widget/extension.mjs" not in result.files