Add allowExecutables approval gate for dependency executables

[sergio-sisternes-epam]

TL;DR

Implements npm v12-inspired default-deny model for executable primitives (hooks, bin/) from dependency packages. Packages declaring hooks or bin/ executables are blocked at install time unless explicitly approved in the allowExecutables block of apm.yml.

Problem (WHY)

npm v12 is shipping default-deny for lifecycle scripts (July 2026). APM currently deploys hooks and bin/ executables from dependency packages without any consent gate -- a malicious or compromised package can silently register hooks that fire on every tool invocation or deploy executables to the user's PATH.

Approach (WHAT)

Mirror npm v12's allowScripts pattern adapted for APM's executable surface:

npm v12	APM
allowScripts in package.json	allowExecutables in apm.yml
Version-pinned: "esbuild@0.21.5": true	Version-pinned: "ci-hooks@acme#1.2.0": {hooks: true}
npm approve-scripts	apm approve
npm deny-scripts	apm deny

What's gated:

• Hooks (.apm/hooks/*.json) -- highest risk, auto-fire in IDE
• bin/ executables -- deployed to Claude Code's PATH

What's NOT gated (and why):

• Root project content (_local) -- user-authored, always trusted
• Text-only primitives (agents, skills, instructions) -- no code execution
• MCP servers from root apm.yml -- user-authored; MCP from deps deferred (requires provenance tracking)
• Scripts via apm run -- explicit user invocation, not supply chain risk

Implementation (HOW)

New files

• src/apm_cli/security/executables.py -- Core approval model: ExecutableDeclaration dataclass, approval checking, package scanning, interactive prompt, manifest read/write
• src/apm_cli/commands/approve.py -- apm approve / apm deny CLI commands

Modified files

• src/apm_cli/models/apm_package.py -- allow_executables field on APMPackage
• src/apm_cli/install/services.py -- Hook skip gate in integrate_package_primitives, bin approval check
• src/apm_cli/integration/skill_integrator.py -- skip_bin parameter gates _deploy_plugin_bin
• src/apm_cli/install/template.py -- Passes allow_executables from context
• src/apm_cli/cli.py -- Registers approve/deny commands

Behaviour matrix

Scenario	Before	After
apm install pkg-with-hooks	Hooks silently deployed	Hooks blocked; user prompted
apm install pkg-with-bin	bin/ silently deployed	bin/ blocked until approved
apm install --trust-all	N/A	All primitives approved (CI escape hatch)
Non-interactive (CI)	Silent	Hard error if unapproved executables
No allowExecutables block	N/A	All approved (backward compatible)

Manifest example

allowExecutables:
  "ci-hooks@acme#1.2.0":
    hooks: true
  "mcp-tools@corp#2.1.0":
    bin: true

Validation

• 66 unit tests covering approval model, scanning, CLI commands, interactive/CI paths
• Full CI-mirror lint chain passes (ruff check, ruff format, pylint R0801, auth-signals)
• Backward compatible: no allowExecutables block = everything approved (opt-in enforcement)

Trade-offs

• Deferred: MCP provenance tracking through dependency collection (requires deeper refactor)
• Deferred: Content hashing for re-approval on executable content change
• Deferred: apm audit executables visibility command
• Deferred: --trust-all / --no-executables CLI flags on apm install

[Copilot]

Pull request overview

This PR introduces an allowExecutables approval model for dependency-provided executable surfaces (hooks and bin/) and wires it into the install/integration pipeline, plus adds apm approve / apm deny commands to manage approvals.

Changes:

• Adds core executable scanning + approval parsing/prompting in apm_cli.security.executables.
• Enforces hook/bin deployment gating during install (including skipping skill/plugin bin/ deployment when unapproved).
• Adds apm approve / apm deny commands and accompanying unit tests.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
tests/unit/security/test_executables.py	New unit coverage for executable declaration, scanning, parsing, write-back, and prompting paths.
tests/unit/security/init.py	New package marker for unit security tests.
tests/unit/commands/test_approve_deny.py	New unit coverage for approve/deny CLI behavior and key matching.
src/apm_cli/security/executables.py	New executable declaration model, filesystem scanning, allowExecutables parsing/writing, and interactive approval flow.
src/apm_cli/security/init.py	Re-exports executable gate utilities.
src/apm_cli/models/apm_package.py	Adds allow_executables to the manifest model and parses allowExecutables.
src/apm_cli/integration/skill_integrator.py	Adds skip_bin plumbing + copy-ignore logic to avoid deploying unapproved bin/.
src/apm_cli/install/template.py	Passes allow_executables from install context into integration.
src/apm_cli/install/services.py	Adds hook/bin gating checks and centralized bin-status logging.
src/apm_cli/install/phases/integrate.py	Adds a post-loop approval prompt that can persist approvals back to apm.yml.
src/apm_cli/install/exec_gate.py	New helpers for approval checking and bin skip/deploy logging.
src/apm_cli/install/context.py	Tracks blocked executable declarations for the post-loop prompt.
src/apm_cli/commands/approve.py	New CLI commands to manage allowExecutables in apm.yml.
src/apm_cli/cli.py	Registers approve and deny commands in the main CLI.

[github-actions]

APM Review Panel: needs_rework

MCP approval UI has no enforcement (false-assurance security control), CI escape hatch cites a non-existent flag, denial-path test gap leaves the core security promise unverified -- five blockers require rework before ship.

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

Five blocking findings share a common thread: the security contract shown to users diverges from the one implemented in code. The highest-signal defect -- flagged as blocking independently by both python-architect and supply-chain-security, and directly implicated by the test-coverage-expert's evidence-backed finding -- is that MCP executable approval is scanned, interactively prompted, and durably written to apm.yml, yet MCPIntegrator deploys MCP servers unconditionally regardless of approval state. Users make a deliberate security choice that has zero runtime effect; this is not an incomplete feature but a false-assurance control, which is categorically worse than no control. The fix is binary and must precede ship: either plumb mcp_ok through to MCPIntegrator.integrate(), or excise EXEC_TYPE_MCP and all MCP copy from the approval UI entirely. The second blocking defect is equally unambiguous: the non-interactive hard-error message instructs CI operators to run apm install --trust-all, a flag that does not exist, permanently deadlocking CI pipelines with a broken remediation path -- one-line fix, no excuse to ship it broken. The test-coverage-expert finding carries the highest evidence weighting available under the panel's rules: outcome: missing on surfaces tagged both secure-by-default and governed-by-policy. The integration denial path -- the core install promise of this PR -- has zero test coverage at any tier; a one-line deletion of the continue at services.py:396 would silently re-enable unconditional hook deployment with no automated guard, and no test would surface the regression. That gap ranks above every opinion-only blocking finding, and the missing approve/deny reference docs together with the double [i] log glyph (hardcoded '[i] ' prefix doubled by CommandLogger's prepended symbol) complete the five blockers, both tractable in a single revision pass.

Three panelists at escalating severities flag the gap between 'default-deny' branding and actual enforcement scope: when allowExecutables is absent from apm.yml, check_executable_approval returns (True, True), making the gate strictly opt-in for every project upgrading from an older APM version. The backward-compatibility rationale is sound -- a silent breaking change on upgrade would be worse -- and the recommended severity classification is correct. The behavior is defensible if and only if the CHANGELOG entry and install.md update are explicit that 'default-deny' applies to new projects that have never held an allowExecutables block; existing projects inherit the legacy pass-through until they explicitly opt in. This is a messaging obligation that must ship in the same PR. The devx-ux and oss-growth panelists both surface the two-step install dance (approve mid-install, then re-run apm install to activate executables) as real architectural friction that breaks the single-command mental model. The friction is real, but it is a design trade-off -- not a security fault -- and it belongs in a tracked follow-up issue rather than on this PR's blocking list.

The oss-growth-hacker's timing analysis is strategically correct and material to this arbitration: PR #1723 is the technical prerequisite for APM's 'Secure by default' positioning, and npm v12's comparable feature targeting July 2026 creates a genuine window -- roughly six months -- to own the 'safe AI agent package manager' narrative before npm catches up. All three preconditions for safe launch amplification map directly to items already on the blocking list: remove the --trust-all dead reference, ship the executable safety doc page, and verify the quickstart sample package has no executables. Resolve the five blockers with the scope-boundary clarification in messaging, and this PR becomes APM's strongest positioning statement to date.

Dissent. On opt-in vs default-deny severity: python-architect calls it recommended; auth-expert calls it a nit -- the recommended classification wins because the gap between 'default-deny' in the PR title and (True, True) in the code requires an explicit CHANGELOG boundary statement, even though the behavior itself is a correct upgrade-safety decision. On two-step install: devx-ux and oss-growth both surface it; neither treats it as blocking, and neither should -- it is architectural debt for a follow-up issue, not a correctness fault.

Aligned with: Secure by default (partially satisfied -- new projects get default-deny, upgrading projects get opt-in; MCP false-assurance gate must close before the claim is technically defensible), Governed by policy (allowExecutables block is the right artifact; MCP enforcement absent and denial-path test gap leave the policy gate unverified by any automated guard), Pragmatic as npm (v12 alignment is sound and the timing window is real; dead --trust-all reference and two-step install create friction npm avoids -- both addressable without rearchitecting), OSS community driven (two new top-level commands ship with zero reference docs and no CHANGELOG entry, directly contradicting the community-trust principle).

Growth signal. Once the five blocking defects are resolved and the CHANGELOG entry frames the opt-in boundary honestly, this PR gives APM a defensible 'Secure by default since [ship date]' claim approximately six months ahead of npm v12's promised July 2026 delivery. The pitch is concrete and differentiating: APM shipped executable approval gates when npm was still in RFC. Amplify only after the CI escape hatch is corrected and the doc page ships -- a broken remediation path or a false-assurance MCP gate disclosed publicly in the launch window would be a credibility sink that no growth narrative survives.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	1	3	2	Clean frozen-dataclass value object and sensible module split; blocked by MCP approval that is tracked, prompted, and stored but never enforced at the install gate, misleading users about actual protection scope.
CLI Logging Expert	1	3	2	Double [i] glyph in integrate.py post-approval message; 'not_approved' tree path emits green (success color) for a security-gate skip; CI fatal exit leads with yellow warning.
DevX UX Expert	2	4	2	Two blocking issues (phantom --trust-all flag in CI error output; no docs for approve/deny), plus four recommended ergonomics gaps in prompt wording, MCP help-text accuracy, two-step install dance, and epilog discoverability.
Supply Chain Security Expert	2	3	2	MCP gate is UI-only (no enforcement), --trust-all CI escape hatch is a dead reference, and approval key asymmetry can make gate permanently ineffective for git packages.
OSS Growth Hacker	0	4	1	Strong 'Secure by default' substantiation ahead of npm v12 -- ideal timing. Three growth risks: CI error references a deferred flag, two-run install breaks the quickstart narrative, and zero docs ship with the feature.
Auth Expert	0	1	1	No credential leakage via dep_ref serialization; canonical_string branch is dead code creating a latent future-leakage footgun worth closing now.
Doc Writer	0	6	0	5 substantive doc gaps: no CHANGELOG entry, no CLI ref pages for approve/deny, install.md silent on approval gate, manifest-schema.md missing allowExecutables, commands.md stale.
Test Coverage Expert	1	3	0	1 blocking gap: integrate_package_primitives hooks/bin-blocked path has no test at any tier; 3 recommended gaps: exec_gate candidate_keys fallback, _run_executable_approval_prompt orchestration, skill_integrator skip_bin exclusion.
Performance Expert	0	2	3	Gate overhead is zero on warm/approved paths; scan_package_executables is uncached and _log_hooks_skip redundantly re-probes the hooks dir per blocked package -- both recommended fixes, neither blocking.

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

Top 5 follow-ups

1. [Test Coverage Expert] (blocking-severity) Add integration-with-fixtures test for integrate_package_primitives when allow_executables denies a dependency's hooks and bin/ -- outcome: missing on secure-by-default + governed-by-policy surface; a one-line deletion at services.py:396 would silently re-enable hook deployment from unapproved packages with no test to catch it.
2. [Supply Chain Security Expert] (blocking-severity) Excise EXEC_TYPE_MCP from all user-facing approval surfaces, or plumb mcp_ok into MCPIntegrator.integrate() -- MCP approval is interactively prompted and written to apm.yml but never enforced at deploy time, a false-assurance control architecturally worse than no control.
3. [DevX UX Expert] (blocking-severity) Remove 'apm install --trust-all' from the CI hard-error message; replace with 'apm approve (package) or apm approve --all' -- the flag does not exist; CI operators following the error message hit 'unrecognized arguments' with no working escape hatch, permanently blocking pipelines.
4. [DevX UX Expert] (blocking-severity) Add CLI reference docs for apm approve and apm deny under docs/src/content/docs/reference/cli/ -- two new top-level commands that change the install contract ship with zero user-facing documentation; per DevX contract, a CLI change not reflected in docs in the same PR is incomplete.
5. [CLI Logging Expert] (blocking-severity) Remove hardcoded '[i] ' prefix from the integrate.py post-approval message body -- CommandLogger.info() prepends STATUS_SYMBOLS['info']; the literal string doubles it to '[i] [i]' in every interactive approval flow.

Architecture

classDiagram
    direction LR

    class ExecutableDeclaration {
        <<ValueObject>>
        +package_key str
        +package_name str
        +hook_count int
        +mcp_count int
        +bin_count int
        +has_executables bool
        +exec_types list~str~
        +summary_line() str
    }
    note for ExecutableDeclaration "frozen=True: immutable value object\nexec_types yields hooks / mcp / bin\nbut install gate checks hooks + bin only"

    class APMPackage {
        +allow_executables dict~str,dict~ | None
        +from_apm_yml(path) APMPackage
    }

    class InstallContext {
        +apm_package APMPackage | None
        +blocked_executables list~ExecutableDeclaration~
        +project_root Path
        +source_root Path | None
    }

    class security_executables {
        <<Module>>
        +is_package_approved(allow,key,type) bool
        +is_any_type_approved(allow,key) bool
        +_is_fully_approved(allow,decl) bool
        +build_approval_key(name,ver) str
        +scan_package_executables(path,name,ver) ExecutableDeclaration
        +prompt_executable_approval(decls) dict
        +parse_allow_executables(data) dict | None
        +write_allow_executables(path,data) None
    }
    note for security_executables "_is_fully_approved checks hooks+mcp+bin\nbut exec_gate returns (hooks,bin) only\n-- MCP approval has no gate counterpart"

    class install_exec_gate {
        <<Module>>
        +check_executable_approval(name,info,allow,ctx) tuple~bool,bool~
        +resolve_package_key(info,name) str
        +log_bin_status(result,suffix,...) None
    }
    note for install_exec_gate "Chain-of-Responsibility key resolution:\ndep_ref.canonical_string -> str(dep_ref) -> name#version\nReturns (hooks_ok, bin_ok) -- no mcp_ok slot"

    class SkillIntegrator {
        +integrate_package_skill(skip_bin) SkillIntegrationResult
        -_integrate_native_skill(skip_bin)
        -_integrate_skill_bundle(skip_bin)
        -_promote_sub_skills(skip_bin)
    }

    class approve_cmd {
        <<ClickCommand>>
        +invoke(packages,pending,approve_all)
        -_scan_installed_packages(manifest) list
        -_approve_packages(manifest,allow,pkgs)
        -_approve_all_pending(manifest,allow)
        -_show_pending(manifest,allow)
    }

    class deny_cmd {
        <<ClickCommand>>
        +invoke(packages)
        -_find_matching_key(allow,pkg) str | None
    }

    InstallContext o-- APMPackage : apm_package
    InstallContext *-- ExecutableDeclaration : blocked_executables
    install_exec_gate ..> security_executables : delegates approval checks
    install_exec_gate ..> InstallContext : appends blocked decls
    security_executables ..> ExecutableDeclaration : creates
    approve_cmd ..> security_executables : parse / scan / write
    approve_cmd ..> ExecutableDeclaration : displays
    deny_cmd ..> security_executables : write_allow_executables
    SkillIntegrator ..> install_exec_gate : skip_bin param

    class ExecutableDeclaration:::touched
    class APMPackage:::touched
    class InstallContext:::touched
    class security_executables:::touched
    class install_exec_gate:::touched
    class SkillIntegrator:::touched
    class approve_cmd:::touched
    class deny_cmd:::touched

    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A([apm install]) --> B["integrate.py: run ctx"]
    B --> C["template.py: _integrate_materialization\nallow_executables = ctx.apm_package.allow_executables"]
    C --> D{allow_executables\nis None?}
    D -->|"Yes -- existing projects\nno allowExecutables block"| E["True, True returned\nblocked_executables stays empty\nprompt NEVER fires -- opt-in only"]
    D -->|"No -- block present"| F["exec_gate: check_executable_approval\nresolve_package_key: dep_ref.canonical -> name#version"]

    F --> MCP_GAP
    subgraph MCP_GAP ["[!] MCP: tracked but NEVER gated"]
        M["exec_gate returns bool,bool -- no mcp slot\nMCP servers deployed unconditionally\neven when mcp_count>0 and mcp not approved\n_is_fully_approved checks mcp -- gate does not"]
    end

    F --> G{hooks approved?}
    G -->|No| H["skip hooks in dispatch loop\nFS: scan_package_executables reads .apm/hooks/\nctx.blocked_executables.append decl"]
    G -->|Yes| I["FS: deploy hook files normally"]
    F --> J{bin approved?}
    J -->|No| K["skill_integrator: integrate_package_skill skip_bin=True\nFS: _build_copy_ignore excludes bin/ from copytree"]
    J -->|Yes| L["skill_integrator: integrate_package_skill skip_bin=False\nFS: deploy bin/ executables"]

    H --> N
    I --> N
    K --> N
    L --> N
    N["integrate.py: _run_executable_approval_prompt ctx\nonly reached when blocked_executables non-empty"] --> O{_is_interactive?\nCI or APM_NON_INTERACTIVE}
    O -->|CI| P["sys.exit 1\nlist blocked packages"]
    O -->|TTY| Q["executables.py: prompt_executable_approval\nclick.confirm per decl in blocked_executables"]
    Q --> R{user approves?}
    R -->|No| S["leave blocked -- no write"]
    R -->|Yes| T["FS: write_allow_executables apm.yml\nload_yaml -> mutate dict -> dump_yaml\nsilent drop if YAML root not dict"]
    T --> U["APMPackage.allow_executables = updated\nin-memory mutation\nlog: run apm install again"]

    V([apm approve pkg]) --> W["approve.py: _scan_installed_packages\nFS: walk apm_modules/ tree recursively"]
    W --> X["scan_package_executables per pkg\nFS: reads .apm/hooks/ + bin/ + apm.yml deps.mcp"]
    X --> Y["allow_exec updated in memory"]
    Y --> Z["FS: write_allow_executables apm.yml\nread-modify-write, no file lock"]

Loading

Recommendation

The architecture is correct, the module structure is clean, the frozen-dataclass value object is a sound design, and the strategic timing against npm v12 is right -- but five blocking defects make this PR unshippable as the foundation of APM's 'Secure by default' claim. Three of the five are one-line or one-file fixes: remove the --trust-all dead reference, correct the double [i] glyph, and add the approve/deny reference docs. The MCP false-assurance gate requires a binary architectural decision -- enforce or excise -- and the test-coverage gap on the core denial path requires at minimum one integration-with-fixtures test before the 'Secure by default' promise is verifiable by CI. Resolve these five items, ship the CHANGELOG entry with explicit opt-in framing for upgrading projects, and this PR is the right feature at the right time.

---

Full per-persona findings

Python Architect

• [blocking] MCP approval is counted, prompted, and stored but the install gate never enforces it at src/apm_cli/install/exec_gate.py:17
  check_executable_approval returns (hooks_approved, bin_approved) with no mcp slot. The install pipeline only consumes those two booleans; MCP servers deploy unconditionally regardless of approval state. Meanwhile, ExecutableDeclaration.exec_types includes EXEC_TYPE_MCP, the interactive prompt lists MCP entries, _is_fully_approved requires mcp: true, and write_allow_executables persists the mcp decision to apm.yml. Users make a deliberate security choice that has zero runtime effect -- an architectural correctness fault: the security model presented to users diverges from the one implemented.
  Suggested: Extend check_executable_approval to return (hooks_approved, mcp_approved, bin_approved) and thread mcp_approved through to wherever dependencies.mcp entries are activated. Alternatively, if MCP enforcement is intentionally deferred, remove mcp from exec_types, _is_fully_approved, and the interactive approval prompt entirely.

• [recommended] allow_executables=None returns (True, True) making the gate opt-in, inverting the stated default-deny promise at src/apm_cli/install/exec_gate.py
  check_executable_approval returns (True, True) when allow_executables is None. Any project that has never written an allowExecutables block gets full executable deployment with no prompt, no warning, and no gate. The module docstring names this 'opt-in enforcement' but every user-facing string calls it 'default-deny'.

• [recommended] Three thin pass-through wrappers in services.py add zero behavior and pollute the call stack at src/apm_cli/install/services.py:181
  _check_executable_approval, _resolve_package_key, and _log_bin_status are single-line delegations to exec_gate functions. Each adds a call-stack frame, a lazy import, and a private-looking symbol. The module docstring admits it was 'extracted from services.py to stay within the LOC budget' -- LOC budget is not a valid architectural reason for a module boundary.
  Suggested: Remove the three wrapper functions. Import exec_gate.check_executable_approval and exec_gate.log_bin_status directly in integrate_package_primitives. The _resolve_package_key wrapper is unused outside services.py and can be deleted entirely.

• [recommended] write_allow_executables silently drops the write when the YAML root is not a dict at src/apm_cli/security/executables.py
  The function returns silently when apm.yml is empty, a bare scalar, or corrupted. The user receives a success log line but the file is unchanged, approvals are lost, and the next install blocks the same packages again.
  Suggested: Raise a descriptive RuntimeError when the YAML root is not a dict, rather than silently returning.

• [nit] _find_matching_key returns only the first prefix match; multi-version packages silently retain stale approvals in deny_cmd at src/apm_cli/commands/approve.py
  Suggested: Collect all prefix-matching keys into a list and remove all of them, logging each removal.

• [nit] builtins.dict annotation in exec_gate.py is a shadowing artifact cargo-culted from the extraction at src/apm_cli/install/exec_gate.py:10
  Suggested: Remove import builtins. Use plain dict in all type annotations in exec_gate.py.

CLI Logging Expert

• [blocking] Double [i] symbol rendered in post-approval message (integrate.py) at src/apm_cli/install/phases/integrate.py
  Line hardcodes the literal string '[i] ' inside the message body while also passing symbol='info' to ctx.logger.info(). CommandLogger.info() prepends STATUS_SYMBOLS['info'] = '[i]' a second time. Rendered output: '[i] [i] Updated allowExecutables in apm.yml. Run apm install again...'
  Suggested: Remove the hardcoded '[i] ' prefix from the string literal. Keep symbol='info'.

• [recommended] 'not_approved' branch in log_bin_status renders in green via tree_item -- inverts the traffic-light contract at src/apm_cli/install/exec_gate.py
  log_bin_status calls log_fn() for the 'not_approved' branch which routes to logger.tree_item() unconditionally calling _rich_echo(message, color='green'). Green = success. A security-gate skip surfaced in green directly contradicts both the visual contract and the companion _log_hooks_skip warning (yellow [!]).
  Suggested: Accept a separate warn_fn parameter alongside log_fn, or emit via logger.warning() for the 'not_approved' branch directly.

• [recommended] CI non-interactive exit path opens with _rich_warning (yellow) before sys.exit(1) at src/apm_cli/security/executables.py
  The non-interactive branch is a hard install failure with sys.exit(1) and no recovery path. Leading with _rich_warning produces yellow output in CI logs that operators may read as advisory-only. The header must be _rich_error.
  Suggested: Replace _rich_warning with _rich_error(symbol='error') for the CI block header.

• [recommended] approve.py routes all output via rich* directly, bypassing CommandLogger entirely at src/apm_cli/commands/approve.py
  The architecture states CommandLogger is the base for ALL commands. Bypassing it means approve/deny have no dry-run awareness, no verbose-mode gating, and no DiagnosticCollector integration.
  Suggested: Instantiate a CommandLogger at the top of approve_cmd and deny_cmd and route all output through logger.success/warning/info/error.

• [nit] Interactive 'execute code on your machine' risk declaration rendered as plain white _rich_echo at src/apm_cli/security/executables.py
  Suggested: Merge into a single _rich_warning call to match the visual weight of the companion header.

• [nit] 'invoked without confirmation' appended to the bin-deployed success line is alarming post-approval at src/apm_cli/install/exec_gate.py
  Suggested: Drop the parenthetical; the user already approved.

DevX UX Expert

• [blocking] apm install --trust-all cited in CI hard-error recovery message but the flag is explicitly deferred and does not exist at src/apm_cli/security/executables.py
  In prompt_executable_approval (non-interactive path), the error instructs 'Run apm approve (package) or apm install --trust-all to approve'. A CI operator following that message will run apm install --trust-all, receive 'unrecognized arguments', and have no working escape hatch. This is a correctness regression in the failure-mode experience.
  Suggested: Remove 'apm install --trust-all' from the error string. Interim message: 'Run apm approve (package) to approve individual packages, apm approve --all to approve everything, or add entries to allowExecutables in apm.yml.'

• [blocking] No reference documentation added for apm approve or apm deny -- two new top-level commands with zero doc coverage at docs/src/content/docs/reference/cli/
  Every other command has a reference page there. This PR adds two first-class commands that change the install contract for any project using packages with executables. A user who hits the interactive prompt or CI hard-error and types apm approve --help gets only the inline docstring with no web-searchable reference.
  Suggested: Add approve.md and deny.md following the format of existing command pages (synopsis, flags table, manifest example, common workflows, backward-compat note).

• [recommended] approve_cmd help text says it approves 'MCP servers' but MCP gating is deferred and never enforced at src/apm_cli/commands/approve.py
  check_executable_approval only returns (hooks_ok, bin_ok). Users who approve a package to 'run MCP servers' receive a false guarantee.
  Suggested: Remove 'MCP servers' from the approve_cmd docstring/help text until MCP gating is enforced.

• [recommended] Two-step install dance -- user approves mid-install then must re-run apm install -- breaks the 'one command succeeds' mental model at src/apm_cli/install/phases/integrate.py
  No major package manager requires two installs. A developer who approves and closes the terminal has not activated the hooks/bin. The gap between 'I approved' and 'it works' is invisible.
  Suggested: Emit a prominent [!] banner rather than an info line; consider a distinct exit code (2) callers can detect. Document the two-step contract in approve.md.

• [recommended] Interactive prompt Trust {decl.package_name}? conflates security identity with execution permission at src/apm_cli/security/executables.py
  'Trust' is underspecified in a security context; 'allow executables' is concrete and actionable.
  Suggested: Change to Allow {decl.package_name} to deploy executables ({decl.summary_line()})?

• [recommended] apm --help epilog has no mention of apm approve -- new users have no discovery path for the mandatory new install step at src/apm_cli/cli.py
  Suggested: Add to _CLI_EPILOG: ' apm approve --pending Review and approve packages with executables'

• [nit] apm approve --pending dual-modes a write command as a read/list command -- surprises users who expect flags to modulate the write, not replace it.

• [nit] apm deny as a bare top-level verb is context-free -- consider apm revoke to match security/auth ergonomics conventions.

Supply Chain Security Expert

• [blocking] MCP executable type is scanned, displayed, and stored in allowExecutables but never enforced at install time -- the gate is illusory for MCP at src/apm_cli/install/exec_gate.py
  check_executable_approval() returns only (hooks_ok, bin_ok). There is no mcp_ok. MCPIntegrator has zero reference to allow_executables, exec_gate, or any approval check. MCP servers declared in a package's apm.yml deploy unconditionally regardless of the allowExecutables block. Shipping a UI that asks users to approve MCP but then ignores those approvals at deploy time is a false-assurance security control -- the worst outcome.
  Suggested: Either (a) add mcp_ok to check_executable_approval and plumb it into MCPIntegrator.integrate() so that packages with unapproved MCP entries are not wired into .mcp.json / ~/.claude.json, or (b) explicitly remove EXEC_TYPE_MCP from ALL_EXEC_TYPES and all user-facing messages in this PR.

• [blocking] apm install --trust-all is mentioned in CI hard-error escape-hatch message but the flag does not exist, permanently blocking CI with a broken remediation path at src/apm_cli/security/executables.py
  In CI where a new dependency ships hooks or bin/, sys.exit(1) hard-blocks the pipeline and the operator is directed to a command that does not exist. There is no other documented escape for CI without hand-editing apm.yml or running apm approve interactively first.
  Suggested: Either register --trust-all on apm install and pass it through to prompt_executable_approval(), or change the error message to reference only 'apm approve --all'.

• [recommended] Approval keys written by apm approve --pending may not match the keys checked at install time, silently making approvals ineffective for git-source packages at src/apm_cli/commands/approve.py
  _scan_dir() reads name/version from the package's own apm.yml and calls build_approval_key(name, version). At install time, resolve_package_key() prefers dep_ref.canonical_string() or str(dep_ref) which for git packages yields 'owner/repo' or 'owner/repo#v1.0'. These may not match what _scan_dir reads.
  Suggested: In _scan_dir(), derive the package key using the same resolve_package_key() logic as the install pipeline, or cross-reference against the lockfile to find the canonical dep_ref key.

• [recommended] scan_package_executables traverses .apm/skills/*/bin/ without calling ensure_path_within, allowing symlinks to exfiltrate filesystem paths into ExecutableDeclaration details at src/apm_cli/security/executables.py
  A compromised package can create .apm/skills/evil-skill symlinks pointing outside install_path; the scanner iterates that directory and populates bin_details with filenames from outside the install boundary.
  Suggested: After computing each bin_dir path, call ensure_path_within(bin_dir.resolve(), install_path.resolve()) before iterating.

• [recommended] write_allow_executables uses a direct open('w') write with no atomic replace; SIGINT or concurrent installs can truncate or lose-race apm.yml at src/apm_cli/security/executables.py
  Suggested: Write to manifest_path.with_suffix('.tmp'), then os.replace(str(tmp), str(manifest_path)) for atomic swap.

• [nit] MCP parse errors in scan_package_executables are silently swallowed as mcp_count=0, allowing a package to hide MCP declarations by crafting an unparseable apm.yml at src/apm_cli/security/executables.py
  Suggested: Log a debug/warning via diagnostics when the except branch fires.

• [nit] Version-pinless approval keys (when version='') silently cover all future versions of a package with no user warning at src/apm_cli/security/executables.py
  Suggested: When build_approval_key returns just package_name (no version suffix), emit a warning: 'Approving (name) without a version pin -- all future versions will be approved.'

OSS Growth Hacker

• [recommended] CI hard-error message cites apm install --trust-all but the flag is not wired to apm install (deferred per PR body) at src/apm_cli/security/executables.py
  A CI user hitting the approval gate hard-error will follow the recovery instruction, run apm install --trust-all, receive 'unrecognized option', and have no clear path forward. This is the exact moment CI adoption is won or lost.
  Suggested: Remove --trust-all from the non-interactive error message until the flag is wired.

• [recommended] MCP server counts appear in the approval prompt and summary lines but are not gated -- creates false security assurance at src/apm_cli/security/executables.py
  scan_package_executables() tallies MCP servers and summary_line() surfaces them in the interactive prompt but MCP from deps is not deployed either way. A user who approves based on seeing 'MCP server(s)' may believe they have consented to (or blocked) MCP deployment when nothing changed.
  Suggested: Filter EXEC_TYPE_MCP out of exec_types, summary_line(), and the approval map until MCP gating ships.

• [recommended] No documentation ships with this feature -- users hitting the approval prompt or CI hard-error have no reference page at docs/src/content/docs/
  All 14 changed files are src/ and tests/. The quickstart, security docs, and CLI reference make no mention of the approval gate. First impressions of a security gate that hard-errors CI with no doc link generate bug reports, not adoption.

• [recommended] Two-run install model ('Run apm install again') breaks the single-command quickstart story for opt-in users at src/apm_cli/install/phases/integrate.py
  The quickstart sells git clone && apm install as the magic one-liner. Adding any new package with executables now requires: install -> prompt -> approve -> 'run install again' -> install. The second run is architecturally necessary but the current message reads as an error, not as a normal confirmation step.
  Suggested: Reword to: '[+] Approvals saved to apm.yml. Run apm install once more to deploy your newly approved primitives.'

• [nit] Hooks-blocked log line only suggests apm approve <pkg> -- misses apm approve --pending for multi-package dependency trees at src/apm_cli/install/services.py

Auth Expert

• [recommended] canonical_string dispatch branch in resolve_package_key is dead code with a latent credential-leakage footgun at src/apm_cli/install/exec_gate.py
  DependencyReference has no canonical_string method -- the getattr dispatch always returns None and the branch never executes. The danger is forward-looking: any future contributor who adds canonical_string() returning a full HTTPS clone URL including an auth token would silently cause that credential-bearing string to become the allowExecutables approval key and get durably persisted to apm.yml.
  Suggested: Remove the dead canonical_string branch entirely and add a comment: '# str(dep_ref) serializes host/repo_url only -- no credentials (see DependencyReference.str)'

• [nit] Opt-in enforcement contradicts the 'default-deny' framing in the PR title at src/apm_cli/install/exec_gate.py
  When allow_executables is None, check_executable_approval returns (True, True). The docstring correctly says 'opt-in enforcement' but the PR title and body say 'default-deny model'. This gap could mislead a security auditor.
  Suggested: Add a single-sentence note clarifying the opt-in scope for upgrading projects.

Doc Writer

• [recommended] No CHANGELOG [Unreleased] entry for allowExecutables / apm approve / apm deny at CHANGELOG.md
  apm install now silently skips hooks and bin/ executables for unapproved packages and hard-errors in CI. This is a behavioral change users will hit on upgrade with no release-note pointer.
  Suggested: Add a '### Added' entry under [Unreleased] covering the executable approval gate, apm approve/deny commands, CI hard-error behavior, and the opt-in framing for existing projects.

• [recommended] No CLI reference pages for apm approve or apm deny in docs/src/content/docs/reference/cli/ at docs/src/content/docs/reference/cli/
  Every other command has a reference page. apm approve and apm deny are new user-facing commands with flags and specific behavior. The error message from apm install already tells users to run apm approve, but there is no page to land on.
  Suggested: Create approve.md and deny.md following the pattern of audit.md: Synopsis, Description, Options table, Behavior bullets, Examples, Exit codes.

• [recommended] install.md Behavior section does not mention the executable approval gate at docs/src/content/docs/reference/cli/install.md
  The Behavior section says nothing about the new default-deny model for hooks and bin/ executables. A user whose install silently skips hooks will not understand why; a CI user who hits a hard error gets no doc pointer.
  Suggested: Add a bullet: 'Executable approval gate. Packages declaring hooks or bin/ executables are skipped unless listed in allowExecutables (see Manifest schema). In TTY sessions APM prompts; in CI the install exits 1.'

• [recommended] manifest-schema.md missing allowExecutables top-level key at docs/src/content/docs/reference/manifest-schema.md
  allowExecutables is a new top-level key that users must write (or have written by apm approve) to deploy executables from dependencies. It is completely absent from the schema reference.
  Suggested: Add allowExecutables to the Section 2 YAML shape block and a new subsection documenting key format, value shape, semantics, and an annotated example.

• [recommended] packages/apm-guide/.apm/skills/apm-usage/commands.md not updated for approve/deny at packages/apm-guide/.apm/skills/apm-usage/commands.md
  The agent skill guide's Security and audit section lists apm audit but has no row for apm approve or apm deny. The APM assistant agent uses this file to answer questions about available commands.
  Suggested: Add rows for apm approve [PKGS...] [--pending] [--all] and apm deny PKGS... to the Security and audit section.

• [recommended] security-and-supply-chain.md 'Policy gates that block install' section missing allowExecutables gate at docs/src/content/docs/enterprise/security-and-supply-chain.md
  The page is the canonical reference for security reviewers and lists what gates block install, but not the new allowExecutables executable gate.
  Suggested: Add a bullet or subsection describing the default-deny model for hooks and bin/ in dependency packages, CI hard-error vs interactive prompt behavior.

Test Coverage Expert

• [blocking] integrate_package_primitives with allow_executables denying a dep has no test at any tier -- hooks-blocked path is the core install promise of this PR at tests/unit/install/test_services_hook_scope.py
  grep -r 'allow_executables' tests/ excluding test_executables.py and test_approve_deny.py returns zero hits. Every existing test for integrate_package_primitives passes allow_executables=None (the legacy trust-all path). The denial branch at services.py:396 and skip_bin=True path at services.py:544 are never exercised. A one-line deletion of the continue at line 396 would silently deploy hooks from every unapproved package.
  Proof (missing at integration-with-fixtures): tests/unit/install/test_services_hook_scope.py::test_integrate_package_primitives_skips_hooks_when_not_approved -- proves: packages whose hooks key is absent or False in allowExecutables are not integrated at install time [secure-by-default, governed-by-policy]

• [recommended] exec_gate.check_executable_approval has no unit test -- dual candidate_keys fallback and ctx.blocked_executables recording are silent-drift risks at tests/unit/install/test_exec_gate.py
  grep -r 'check_executable_approval|exec_gate' tests/ returns NO MATCHES. If the candidate_keys fallback broke, packages approved under the name#version format would be permanently blocked despite a valid apm.yml entry.
  Proof (missing at unit): tests/unit/install/test_exec_gate.py::test_check_executable_approval_name_version_fallback_when_dep_ref_key_differs -- proves: a package approved under its name#version key is granted approval even when the dep_ref canonical string differs [secure-by-default]

• [recommended] _run_executable_approval_prompt orchestration (blocked_executables -> prompt -> apm.yml write-back) is untested at tests/unit/install/phases/test_integrate_phase.py
  grep -r '_run_executable_approval_prompt|blocked_executables' tests/ returns NO MATCHES. The glue layer that guards on blocked_executables being non-empty, calls prompt_executable_approval, and writes the result back to apm.yml is never exercised.
  Proof (missing at unit): tests/unit/install/phases/test_integrate_phase.py::test_run_executable_approval_prompt_persists_approval_to_apm_yml -- proves: approvals entered interactively during apm install are persisted to apm.yml so the next install does not re-prompt [devx, governed-by-policy]

• [recommended] skill_integrator skip_bin parameter has no test asserting bin/ directory exclusion from copytree at tests/unit/integration/test_skill_integrator.py
  grep -r 'skip_bin' tests/ returns NO MATCHES. If _build_copy_ignore's conditional were accidentally dropped, bin/ executables would deploy silently alongside skill content.
  Proof (missing at unit): tests/unit/integration/test_skill_integrator.py::test_integrate_package_skill_excludes_bin_when_skip_bin_true -- proves: bin/ executables from a dependency package are not deployed when the package is not approved in allowExecutables [secure-by-default]

Performance Expert

• [recommended] scan_package_executables has no result cache; future parallel-install or apm-audit paths will re-scan the same materialised package repeatedly at src/apm_cli/security/executables.py
  Each call issues at minimum 4x stat() + 1x readdir() + 1x YAML parse (~0.5ms NVMe warm). In the current sequential install loop there is no repeated work today, but the function is stateless with no memoisation.
  Suggested: Add @functools.lru_cache(maxsize=256) to scan_package_executables, converting install_path: Path to str for hashability.

• [recommended] _log_hooks_skip (services.py) re-probes the hooks directory even though check_executable_approval already ran scan_package_executables and stored hook_count in ctx.blocked_executables at src/apm_cli/install/services.py
  1 extra stat() + 1 extra readdir() per blocked-hooks package, duplicating work already done. The declaration is already in ctx.
  Suggested: When ctx is not None, look up the blocked declaration instead of re-probing the FS.

• [nit] allow_executables=None fast path is correct and costs exactly 2 comparisons -- zero FS, zero network, zero import overhead for all existing users at src/apm_cli/install/exec_gate.py

• [nit] _build_copy_ignore(skip_bin=True) adds one regex match + one set union per copytree directory traversal -- nanosecond range, not measurable at src/apm_cli/integration/skill_integrator.py

• [nit] _run_executable_approval_prompt post-loop gate costs O(1) on warm/zero-blocked installs at src/apm_cli/install/phases/integrate.py

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

Generated by PR Review Panel for issue #1723 · sonnet46 25.6M · ◷