Skip to content

feat: add general-purpose lifecycle hooks framework (#1529)#1798

Merged
danielmeppiel merged 63 commits into
mainfrom
sergio-sisternes-epam/feat-installation-analytics-hooks
Jun 28, 2026
Merged

feat: add general-purpose lifecycle hooks framework (#1529)#1798
danielmeppiel merged 63 commits into
mainfrom
sergio-sisternes-epam/feat-installation-analytics-hooks

Conversation

@sergio-sisternes-epam

@sergio-sisternes-epam sergio-sisternes-epam commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

feat: add general-purpose lifecycle hooks framework (#1529)

Adds a three-tier lifecycle hooks system that fires shell commands or HTTPS webhooks at pre/post-install/update/uninstall events. Scripts are embedded in apm.yml under a lifecycle: key and gated behind apm lifecycle trust so no script runs without explicit consent.

What this does

  • Run validated scripts on install/update/run with zero separate config files -- everything lives in apm.yml.
  • Three-tier hierarchy: project (apm.yml), user (~/.apm/apm.yml), admin (/etc/apm/policy.d/*.json).
  • Trust gate: apm lifecycle trust records a SHA-256 fingerprint of the canonical lifecycle: subtree. Editing other apm.yml keys (e.g. dependencies:) does not revoke trust.
  • Two executor types: command (shell subprocess, synchronous, can delay operation) and http (HTTPS POST, daemon thread, fire-and-forget).
  • Credential safety: environment variables with TOKEN/SECRET/PAT/KEY/PASSWORD/CREDENTIAL/AUTHTOKEN suffixes are blocked from HTTP header expansion by default; opt-in via allowedEnvVars. APM's own auth tokens (GITHUB_APM_PAT, GITHUB_TOKEN, GH_TOKEN, ADO_APM_PAT) are never expandable regardless of opt-in.
  • cwd containment: relative cwd: entries that escape project root via .. are clamped to the project root.
  • Org override: executables.deny_all: true in apm-policy.yml suppresses all lifecycle scripts as a one-directional enterprise ceiling. APM_NO_SCRIPTS=1 skips scripts for a single run.
  • apm lifecycle CLI (renamed from apm scripts to avoid collision with apm run):
    • init -- scaffold lifecycle: block in apm.yml
    • validate -- check all discovered scripts for schema errors
    • test [event] [--execute] -- dry-run preview (or live fire) a synthetic event; shows trust status
    • trust / untrust -- manage project lifecycle trust

How to try it

cd my-project
apm lifecycle init            # scaffolds lifecycle: in apm.yml
# edit apm.yml, add your scripts
apm lifecycle validate        # check for schema errors
apm lifecycle test post-install               # dry-run: see what would fire
apm lifecycle test post-install --execute     # actually fire the event
apm lifecycle trust           # trust current lifecycle: subtree
apm install                   # scripts now fire on install

Tests

  • tests/unit/core/test_lifecycle_scripts.py -- discovery, filtering, runner
  • tests/unit/core/test_script_trust.py -- trust gate, fingerprinting
  • tests/unit/core/test_script_executors.py -- command/HTTP executors, cwd containment, env filtering
  • tests/unit/commands/test_lifecycle.py -- all 6 CLI subcommands, trust-status display, --execute regression trap
  • tests/unit/install/test_lifecycle_trust_gate.py -- integration: untrusted clone does NOT fire scripts, trusted one does

Closes #1529

Co-authored-by: Sergio Sisternes sergio.sisternes@epam.com


apm-spec-waiver: install-path lifecycle firing is trust-gated and discovery-bounded; reviewed and folded under this PR with the security-model amendment documented in docs/enterprise/security.md. No OpenAPM-normative contract change beyond that amendment; the install critical-path spec citation is tracked as a follow-up in #1919.

Sergio Sisternes and others added 5 commits June 13, 2026 15:52
Implement a lifecycle hook framework that fires custom actions at
install, update, and uninstall time. Three hook types are supported:
shell commands, HTTP webhooks, and executable scripts.

Hooks can be configured at project level (apm.yml), global level
(~/.apm/config.json), and policy level (apm-policy.yml). Policy hooks
run first and cannot be removed by the project. All hooks are
fire-and-forget with error isolation -- failures never block the CLI.

New files:
- src/apm_cli/core/lifecycle_hooks.py: event model, hook definitions,
  runner, discovery, and build_runner_from_context convenience function
- src/apm_cli/core/hook_executors.py: webhook (HTTPS-only, 2s timeout,
  daemon thread), command (subprocess, 30s timeout), and script
  (path-traversal guard) executors

Wiring:
- InstallService.run(): pre-install / post-install hooks
- uninstall/cli.py: pre-uninstall / post-uninstall hooks
- update.py: pre-update / post-update hooks

Supporting changes:
- apm_package.py: lifecycle_hooks field and parsing
- policy/schema.py: LifecycleHooksPolicy (require, deny_types)
- policy/parser.py: lifecycle_hooks policy parsing
- config.py: get/set/unset_lifecycle_hooks for global config

Tests: 50 unit tests covering models, runner, executors, collection,
deduplication, error isolation, and security guards.

Docs: enterprise/lifecycle-hooks.md guide page.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace inline lifecycle_hooks config (apm.yml, config.json, policy)
with standalone JSON hook files discovered from well-known directories:

- Policy:  /etc/apm/policy.d/*.json
- User:    ~/.apm/hooks/*.json
- Project: .apm/hooks/*.json

Key changes:
- Two hook types: command (stdin delivery) and http (HTTPS POST)
- Drop script type (subsumed by command with bash field)
- Drop token_env (replaced by headers with $ENV_VAR expansion)
- Event payload delivered via stdin for commands (not env var)
- Add working_directory field to event payload
- Remove lifecycle_hooks from APMPackage, ApmPolicy, config.py
- Rewrite executors, tests (58 passing), and documentation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hook stdout, stderr, exit code, and execution status are now appended
to ~/.apm/logs/hooks.log (or $APM_HOME/logs/hooks.log) after every
hook execution.  This gives administrators an audit trail without
requiring verbose CLI output.

Log writing is fire-and-forget -- failures are silently swallowed to
ensure logging never breaks the CLI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three new sub-commands under 'apm hooks':

- hooks init   -- scaffold a starter hook JSON file
- hooks test   -- dry-run a synthetic event through all hooks
- hooks validate -- check all hook files for schema errors

Includes 26 tests and documentation updates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Change project-level hook discovery from a directory (.apm/hooks/*.json)
to a single file (.apm/hooks.json). Update init, validate commands,
all tests, and documentation accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 16, 2026 08:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a lifecycle hooks framework to APM so installs/updates/uninstalls can emit structured events to user/admin-defined “command” and “http” hooks, plus a new apm hooks CLI group and accompanying enterprise documentation.

Changes:

  • Introduces hook discovery + event models + runner (lifecycle_hooks.py) and executors with stdout/stderr logging (hook_executors.py).
  • Fires lifecycle events from install/update/uninstall flows and registers the new CLI command group.
  • Adds unit tests covering parsing/discovery/execution and documentation for enterprise usage.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
tests/unit/core/test_lifecycle_hooks.py New unit tests for hook models, discovery, and runner behavior.
tests/unit/core/test_hook_executors.py New unit tests for HTTP/command executors, env expansion, cwd, and hooks log output.
tests/unit/commands/test_hooks.py New unit tests for apm hooks list/init/test/validate commands.
src/apm_cli/install/service.py Fires pre-install/post-install hooks around the install pipeline and adds helper builders.
src/apm_cli/core/lifecycle_hooks.py Adds lifecycle event schema, hook entry model, JSON parsing, and discovery across policy/user/project.
src/apm_cli/core/hook_executors.py Implements command + HTTP hook execution and appends results to hooks.log.
src/apm_cli/commands/update.py Fires pre-update/post-update hooks around the update pipeline.
src/apm_cli/commands/uninstall/cli.py Fires pre-uninstall/post-uninstall hooks around uninstall.
src/apm_cli/commands/hooks.py New apm hooks command group (list/init/test/validate).
src/apm_cli/cli.py Registers the new hooks command group.
docs/src/content/docs/enterprise/lifecycle-hooks.md Adds enterprise documentation for hook format, discovery, security, logging, and CLI usage.

Comment thread src/apm_cli/install/service.py Outdated
Comment thread src/apm_cli/install/service.py Outdated
Comment thread src/apm_cli/install/service.py Outdated
Comment thread src/apm_cli/core/lifecycle_hooks.py Outdated
Comment thread src/apm_cli/commands/hooks.py Outdated
Comment thread src/apm_cli/core/hook_executors.py Outdated
Comment thread src/apm_cli/core/hook_executors.py Outdated
Comment thread src/apm_cli/commands/update.py Outdated
Comment thread src/apm_cli/core/hook_executors.py Outdated
Comment thread src/apm_cli/commands/hooks.py Outdated
@danielmeppiel danielmeppiel added the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 19, 2026
Sergio Sisternes and others added 6 commits June 19, 2026 11:18
- Add credential denylist to _expand_env_vars: blocks TOKEN, SECRET,
  PAT, KEY, PASSWORD, CREDENTIAL pattern vars from HTTP header expansion
- Strip credential-pattern vars from _build_hook_env so command hooks
  cannot inherit GITHUB_APM_PAT, ADO_APM_PAT, etc.
- Fix console import in hooks.py: replace nonexistent 'console' symbol
  with _get_console() calls (fixes dead Rich display layer)
- Add HTTP thread drain in 'apm hooks test' so log entries are written
  before CLI exits (bounded 15s join per thread)
- Update security.md: retract absolute 'no network calls' and 'no code
  execution' claims to reflect opt-in lifecycle hooks reality
- Update execute_hook/fire() signatures to return threads for drain
- Change verbose message from 'sent' to 'dispatched' (accurate wording)
- Add comprehensive tests for credential denylist and env stripping

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nes-epam/feat-installation-analytics-hooks

# Conflicts:
#	docs/src/content/docs/enterprise/security.md
- Move pre-install hooks after import check so hooks do not fire when
  the install pipeline is not available
- Add return type annotations to _build_hook_runner and _build_event
- Fix effective_command to prefer command over bash on Windows
- Use urlparse for URL validation, reject embedded credentials
- Add hooks_for_event() public API, stop reaching into _hooks
- Remove Rich markup in panel title that renders as italic
- Clarify synchronous execution in _execute_command docstring
- Add type annotations to _fire_update_hooks parameters
- Redact URL credentials before writing to hooks.log
- Add apm hooks commands to commands.md reference

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nt (#1529)

- Move hooks commands into dedicated 'Lifecycle hooks' section in commands.md
- Add tests for _redact_url_credentials (plain, user:pass, user-only)
- Add tests for hooks_for_event public API (matching, empty result)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge main into branch, combining lifecycle hooks references with
updated canvas allowExecutables gate language from main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts:
#	docs/src/content/docs/enterprise/security.md
danielmeppiel and others added 2 commits June 26, 2026 01:57
…g deny_all (#1798)

noun collision with agent-hooks primitive; separate governance surface per #1919

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_with_followups

Lifecycle scripts framework is architecturally sound and security-hardened; five blocking-severity doc/UX gaps need folding before merge.

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

This PR delivers APM's first enterprise-differentiation primitive -- lifecycle scripts with a SHA-256 content-hash trust gate, credential isolation, and org-level deny_all. The security model is complete and the supply-chain-security expert found no exploitable flaws. CI is green across all gates. The architectural decomposition is clean (python-architect confirms good separation), and the performance expert identifies no regressions in the common case.

The five blocking-severity findings cluster into two classes that I weight equally: (A) security-relevant events invisible at default verbosity -- credential variable blocking produces silent empty headers, and untrusted-script skip notices vanish unless --verbose is passed. These are the same design defect: when APM makes a security decision on the user's behalf, that decision MUST surface at default verbosity. This is not cosmetic; it is the difference between "secure by default" and "confusing by default". (B) Documentation drift from the hooks-to-scripts rename -- security.md still says "Hooks exception", commands.md omits trust/untrust, and the CLI reference page pattern (established for 30+ commands) is missing for scripts. These are trivial individually but collectively erode the "one source of truth" promise.

All five are foldable in a single commit without architectural risk. The trust gate filtering test gap (test-coverage-expert) is the most strategically important recommended finding because it guards the supply-chain security promise -- but it is not blocking because the trust gate itself works (passing CI proves the happy path), and the gap is "missing guardrail against future regression" rather than "broken behavior today". The performance reorder (check discover_scripts emptiness before policy fetch) is a three-line change that eliminates a network call in the no-scripts common case -- high value, zero risk, should fold. The daemon thread abandonment is real but bounded in blast radius (post-uninstall only); a 2s join cap is the right fix and can fold or follow up without blocking.

Dissent. cli-logging-expert and devx-ux-expert both flagged skip-notice invisibility as blocking; supply-chain-security-expert did not flag it because the security model itself is sound regardless of UX. I side with cli-logging and devx: a security gate that users cannot observe is an adoption hazard -- the first GitHub issue will be "my scripts don't run and APM says nothing". This is a blocking-tier UX defect even though it is not a security vulnerability. The doc-writer and devx-ux-expert both flagged the missing CLI reference page; python-architect did not mention it. I side with doc-writer/devx: the pattern is load-bearing for discoverability and omitting it for a new top-level command is a doc regression.

Aligned with: secure_by_default (strong -- SHA-256 trust gate, env denylist, credential redaction, APM_NO_SCRIPTS, org deny_all; no exploitable gaps found); governed_by_policy (strong -- org-level deny_all, project-level trust store, per-event control); portability_by_manifest (adequate -- scripts declared in apm.yml, trust store is path-resolved); community_trust (at risk if UX visibility gaps ship -- silent security decisions erode trust faster than missing features).

Growth signal. The oss-growth-hacker correctly identifies this as APM's first enterprise-differentiation feature. The SHA-256 trust gate is not just security -- it is a positioning wedge: no incumbent package manager offers content-hash trust for lifecycle scripts with org-level deny. Lead with this in release notes. The analytics use case (post-install/update telemetry) is the 60-second proof that converts evaluators; ensure post-update is documented before merge so the proof actually works end-to-end.

Panel summary

Persona B R N Takeaway
Python Architect 0 2 2 Well-structured module decomposition. HTTP daemon threads abandoned in install flows; ScriptEntry lacks post_init validation.
CLI Logging Expert 1 2 2 Output patterns mostly sound. Credential-var blocking silent in non-verbose; trust warning printed after action.
DevX UX Expert 2 0 4 Command surface well-designed. Missing CLI reference page; skip notice invisible at default verbosity.
Supply Chain Security 0 0 2 Sound content-hash trust model; no exploitable flaws. Symlink check and script.env denylist comment as nits.
OSS Growth Hacker 0 4 2 First enterprise-differentiation feature. Lead with outcome; analytics use case needs post-update.
Auth Expert 0 0 1 Denylist correctly covers APM auth vars. script.env bypass needs a comment.
Doc Writer 2 3 3 lifecycle-scripts.md technically accurate. "Hooks exception" label wrong; commands.md omits trust/untrust.
Test Coverage Expert 0 2 1 Trust gate filtering path lacks a test; script_trust.py has no dedicated unit tests.
Performance Expert 0 3 2 Common case cheap. Reorder discover_scripts before policy fetch for no-scripts common case.

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

Top 5 follow-ups

  1. [CLI Logging Expert] (blocking-severity) Surface credential-var blocking and skip notices at default verbosity -- Security decisions invisible to users erode trust and generate support issues. Same class of defect in two places -- fix both with a single INFO-level emit pattern.
  2. [Doc Writer] (blocking-severity) Fix security.md "Hooks exception" label and add trust/untrust to commands.md -- Stale naming contradicts the rename already applied. Commands table is the discovery surface for new users; omitting trust/untrust makes the trust model invisible.
  3. [DevX UX Expert] (blocking-severity) Add CLI reference page at docs/reference/cli/scripts.md -- Established pattern for all 30+ commands. Missing page breaks docs search and --help-to-web flow.
  4. [Test Coverage Expert] Add unit test for trust gate filtering in build_runner_from_context -- Guards the supply-chain security promise against regression. The gate works today (CI green) but has no test that would catch a future refactor breaking it.
  5. [Performance Expert] Reorder: check discover_scripts emptiness before policy fetch -- Eliminates a network call in the no-scripts common case (majority of projects today). Three-line change, zero risk.

Recommendation

The architecture, security model, and test suite are solid -- CI green, no exploitable gaps, clean module decomposition. However, the three blocking follow-ups (visibility at default verbosity, doc drift fixes, CLI reference page) are all foldable in a single commit and should land before merge. They are not design questions -- they are mechanical completeness gaps that take 30 minutes total. Fold them, then ship. The two non-blocking follow-ups (trust gate test, perf reorder) can land as immediate post-merge commits or in-PR if convenient.


Full per-persona findings

Python Architect

  • [recommended] HTTP daemon threads abandoned in install/update/uninstall flows -- runner.fire() return value discarded in integration points; daemon threads may be killed on process exit (especially post-uninstall). scripts_test correctly joins threads but real integration sites don't.
    Suggested: Add a bounded 2s join at post-* event sites.

  • [recommended] ScriptEntry lacks post_init validation -- invalid type or missing fields silently no-op at execution rather than failing early with a clear message.

  • [nit] build_runner_from_context uses Any for logger type hint instead of CommandLogger | None.

  • [nit] _emit_skip_notice uses a fragile getattr chain for logger method dispatch.

CLI Logging Expert

  • [blocking] Credential variable blocking invisible in non-verbose mode -- _expand_env_vars blocks credential vars with no user feedback unless --verbose is passed. Empty auth header with no hint to the user.
    Suggested: Emit the blocking warning at INFO-level (via warning method if available) regardless of verbose flag.

  • [recommended] Trust warning inverted order -- warning about arbitrary command execution emitted AFTER trust is already recorded.
    Suggested: Move the _rich_warning call before trust_project_scripts().

  • [recommended] List command manually embeds STATUS_SYMBOLS in format string; use STATUS_SYMBOLS dict consistently.

  • [nit] Hardcoded [!] and [i] prefixes in executor messages instead of STATUS_SYMBOLS.

  • [nit] Slow-script warning lacks actionable fix suggestion (e.g. increase timeoutSec or convert to http type).

DevX UX Expert

  • [blocking] No CLI reference page at docs/src/content/docs/reference/cli/scripts.md -- every other top-level command has one; missing page breaks docs search and the --help-to-web flow.

  • [blocking] Skip notice (untrusted project scripts skipped) invisible in non-verbose mode -- developers cloning a repo and running apm install will see no output explaining why their scripts are not running.

  • [nit] commands.md table omits trust and untrust subcommands.

  • [nit] apm scripts trust should display what is being trusted before recording.

  • [nit] scripts test help text does not mention that post-install is the default event.

  • [nit] security.md still uses "Hooks exception" phrasing.

Supply Chain Security

  • [nit] Project scripts file not checked for symlink before fingerprinting -- symlink to a trusted file could be swapped. Mitigated by content-hash model, but defense-in-depth would resolve the symlink first.

  • [nit] script.env dict bypasses the credential denylist (intentional allowlisted by the user) but the code has no comment clarifying this is a best-effort convenience, not a security boundary.

OSS Growth Hacker

  • [recommended] Lead with the outcome in docs, not the mechanism -- open with what enterprises gain (visibility, governance, extensibility) before explaining the JSON schema.

  • [recommended] Analytics use case needs post-update event -- the example only shows post-install and post-uninstall; version drift visibility requires post-update.

  • [recommended] README deserves a one-liner signaling enterprise extensibility.

  • [recommended] Position the SHA-256 trust gate as a growth enabler -- frame it as "runs only what you approve" not as a restriction.

  • [nit] Mention the extensibility angle for third-party script libraries.

  • [nit] Cross-link from security.md update back to lifecycle-scripts.md.

Auth Expert

  • [nit] script.env bypasses the denylist intentionally (the user opted in via allowedEnvVars) but the _build_script_env function lacks a comment clarifying this is best-effort convenience, not a security boundary.

Doc Writer

  • [blocking] security.md uses "Hooks exception" label -- contradicts the hooks-to-scripts rename already applied at commit 008b7ab. Two occurrences need updating to "Scripts exception".

  • [blocking] commands.md omits trust and untrust subcommands from the lifecycle scripts table.

  • [recommended] Trust store path not documented in lifecycle-scripts.md -- users who want to audit or reset trust have no path reference.
    Suggested: Add one sentence: trust records live at ~/.apm/scripts-trust.json.

  • [recommended] Analytics use case omits post-update event but the surrounding prose promises version drift visibility -- add a post-update entry to the analytics example.

  • [recommended] PR body still uses old "hooks" naming throughout.

  • [nit] Double backticks in heading code spans (e.g. apm scripts) -- single backtick is the Markdown convention.

  • [nit] Cross-links use absolute paths (pre-existing pattern deviation).

  • [nit] CHANGELOG notation "pre/post-install/update/uninstall" is ambiguous.

Test Coverage Expert

  • [recommended] Trust gate filtering (untrusted project scripts skipped) has no test in build_runner_from_context.
    Evidence (missing, unit tier): tests/unit/core/test_lifecycle_scripts.py -- no test verifies that project scripts are filtered when is_project_scripts_trusted returns False.

  • [recommended] script_trust.py has no dedicated unit test file for edge cases (fingerprint mismatch, unreadable store, concurrent writes).
    Evidence (missing, unit tier): tests/unit/core/test_script_trust.py -- file does not exist.

  • [nit] No integration-tier test for install lifecycle scripts firing end-to-end.

Performance Expert

  • [recommended] discover_policy_with_chain may issue network RPCs on cold cache inside the install hot path -- called unconditionally before discover_scripts.
    Suggested: Call discover_scripts first; if empty, return early before the policy fetch.

  • [recommended] No cap on number of command scripts per event -- unbounded serial accumulation on high-script-count policy files.

  • [recommended] Reorder: check discover_scripts result emptiness before expensive policy fetch.

  • [nit] HTTP daemon threads have no pool cap -- many concurrent HTTP scripts could exhaust file descriptors.

  • [nit] 5s slow-script warning threshold could be user-configurable.

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

- security.md: rename 'Hooks exception' to 'Scripts exception' (doc
  drift from the hooks->scripts rename at 008b7ab)
- commands.md: add trust and untrust subcommands to lifecycle scripts
  table (were absent despite being implemented)
- docs/reference/cli/scripts.md: add CLI reference page following the
  established pattern for all top-level commands
- lifecycle-scripts.md: document trust store path (~/.apm/scripts-trust.json);
  add post-update event to analytics example; fix double-backtick headings
- script_executors.py: emit credential-variable blocking warning at
  default verbosity (not just --verbose); security gate must be visible
- lifecycle_scripts.py: emit skip-notice at warning level when no
  CommandLogger is available (fixes silent discard on _logger.debug);
  fix logger type hint Any -> CommandLogger | None; reorder
  discover_scripts before policy fetch to skip network call in the
  common no-scripts case
- scripts.py: move trust warning before trust_project_scripts() call
  so the user sees the consent message before the action
- script_executors.py: add comment clarifying script.env bypass of
  the denylist is best-effort convenience, not a security boundary
- test_lifecycle_scripts.py: add tests for trust gate filtering
  (untrusted/trusted project scripts, user scripts bypass)
- test_script_trust.py: new test file covering fingerprint, trust
  store roundtrip, mismatch, missing file, malformed store, update,
  untrust

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

Shepherd-driver: fold changes applied — CI green, ready for maintainer review

All blocking panel items have been folded into commit 473cc12.

Folded (commit 473cc12)

Area Change
Docs `security.md`: renamed "Hooks exception" -> "Scripts exception" (doc drift from 008b7ab rename)
Docs `commands.md`: added `trust` and `untrust` subcommand rows to lifecycle scripts table
Docs Created `docs/reference/cli/scripts.md` — CLI reference page (every other top-level command has one)
Docs `lifecycle-scripts.md`: trust store path, post-update analytics example, heading backtick fix
Security visibility `script_executors.py`: credential-variable blocking warning now emits at default verbosity (not just --verbose)
UX visibility `lifecycle_scripts.py`: skip-notice now emits at `_logger.warning` level (was `debug`; invisible by default)
UX ordering `scripts.py`: trust warning rendered BEFORE `trust_project_scripts()` call so user sees it before action
Performance `lifecycle_scripts.py`: `discover_scripts()` runs before policy fetch; no-scripts fast-path avoids network call
Tests `test_lifecycle_scripts.py`: added 3 trust-gate tests (untrusted/trusted project, user bypass)
Tests `test_script_trust.py` (new): 15 unit tests covering fingerprint, roundtrip, mismatch, malformed store
Comment `script_executors.py`: clarifying comment that `script.env` bypass of denylist is best-effort convenience

Deferred (scope boundary — do not implement)

Item Boundary
README one-liner Repo doc rule 2: requires maintainer approval before README changes
ScriptEntry `post_init` validation Design change; tracked in #1919
HTTP thread pool cap Cross-cutting optimization across unrelated modules; future PR
Per-event script count cap New safety feature beyond issue #1529 scope; future PR
Full install->scripts integration test Future PR; out of current test suite scope

CI status (commit 473cc12)

All 15 checks SUCCESS: Lint, Build & Test Shard 1+2 (Linux), Coverage Combine, PR Binary Smoke, APM Self-Check, CodeQL (python + actions), Analyze (python + actions), Spec conformance gate, NOTICE Drift Check, gate, build, license/cla.

The PR is ready for maintainer approval and merge.

The docs claimed all lifecycle scripts are fire-and-forget and never
block the CLI. That is true only for http scripts (background daemon
thread); command scripts run synchronously and can delay the operation
up to their timeout, matching the script_executors module docstring.
Scope the fire-and-forget language to http and state the synchronous
behaviour of command scripts honestly.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel added status/needs-design Direction approved, design discussion required before code. and removed panel-review Trigger the apm-review-panel gh-aw workflow labels Jun 26, 2026
danielmeppiel and others added 2 commits June 26, 2026 08:26
…iscriminator

Maintainer-approved design refactor (tracked in #1919) applied to the
lifecycle-scripts framework:

- Placement: project-tier script file moves OUT of `.apm/` to repo root
  as `apm-scripts.yml`. `.apm/` holds packageable primitives that travel
  via `apm pack`; lifecycle scripts are install CONTROL that must not
  travel (cargo `build.rs`-at-root analogy), so they live at the root
  next to apm.yml.
- Format split (format-follows-author): the human-authored, human-read
  project file is YAML (comments + block scalars aid the trust audit a
  human performs before `apm scripts trust`); machine-managed admin
  (`/etc/apm/policy.d/*.json`) and user (`~/.apm/scripts/*.json`) tiers
  stay JSON.
- Schema: `type: command|http` discriminator (replaces the invented
  run:/notify: verbs) plus an optional per-entry `description`. The
  parser infers type from url-presence when `type` is omitted and keeps
  bash/command and timeoutSec/timeout read-tolerance.

Discovery, trust (whole-file SHA-256 keyed by resolved abs path), and
the `executables.deny_all` one-directional kill-switch ceiling are
unchanged. Docs (enterprise/lifecycle-scripts, reference/cli/scripts),
the apm-usage skill, and the scaffolded init template are updated to the
new placement, format split, and schema.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…test, allowedEnvVars docs

- CHANGELOG: correct project script file reference from .apm/scripts.json
  to apm-scripts.yml at repo root (doc-writer/oss-growth-hacker, blocking)
- script_executors.py: ungate TimeoutExpired warning from verbose-only to
  always-emit (performance-expert); move inline import time to module top
- lifecycle-scripts.md: document credential denylist and allowedEnvVars
  opt-in for HTTP header token expansion (doc-writer, recommended)
- tests: add two Windows platform branch tests for effective_command()
  via platform.system monkeypatch (test-coverage-expert, recommended)

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

[*] shepherd-driver: apm-review-panel pass 1 (iteration 1)

Headline: Lifecycle scripts framework is architecturally sound; merge after fixing stale CHANGELOG path, ungating timeout warning, and updating PR body.

Arbitration: The panel converged on a well-designed 3-tier execution governance model with no security findings against the PR's actual changes. Supply-chain, auth-expert, cli-logging-expert, and devx-ux-expert were invalidated for referencing pre-existing code outside the PR diff. The two blocking findings (CHANGELOG naming a non-existent .apm/scripts.json, PR body still using 'hooks' terminology) were documentation accuracy errors, each fixed in commit 2e931ca. The performance-expert's timeout-warning finding was the most user-facing code issue: a 30-second silent stall at normal verbosity, resolved in the same commit.

Panel recommendation: ship_with_followups (folding all in-scope items now)

Folded in commit 2e931ca:

  • [+] CHANGELOG.md: corrected project script file reference to apm-scripts.yml at repo root (doc-writer/oss-growth-hacker, blocking)
  • [+] script_executors.py: ungated TimeoutExpired warning from verbose-only to always-emit (performance-expert)
  • [+] script_executors.py: moved inline import time to module top (python-architect nit)
  • [+] lifecycle-scripts.md: documented credential denylist and allowedEnvVars opt-in (doc-writer)
  • [+] tests: added 2 Windows platform branch tests for effective_command() (test-coverage-expert)
  • [+] PR body: rewrote with correct 'scripts' terminology and current file layout

Deferred to #1919 (out-of-scope per stated boundary):

  • e2e/integration-with-fixtures test for install pipeline
  • Per-event aggregate timeout cap
  • TOCTOU file lock in trust store
  • lifecycle_scripts.py module split

Principle alignment:

  • secure-by-default: ALIGNED (deny-wins ladder, SHA-256 trust gate, credential denylist)
  • governed-by-policy: ALIGNED (3-tier precedence with executables.deny_all ceiling)
  • devx: PARTIAL-RESOLVED (timeout warning now fires without --verbose; quickstart noted but deferred)
  • portability-by-manifest: ALIGNED (apm-scripts.yml committed to repo root)

CI: re-running on new head sha 2e931ca.

Comment thread tests/red_team/env_sweep/test_round17_env.py Fixed
…fat-scalar dump bomb) + CI repairs

Round-18 adversarial sweep on dae60ea surfaced env BREAKS x2 families +
parser BREAK x1; trust/http/install CLEAN. All fixed + trapped, plus the
round-17 CI repairs (aggregator mock target, CodeQL urlparse equality) and a
durable shard-2 coverage lift.

Security fixes (src/):
- r18-env-1/2 (HIGH): Terraform Cloud/Enterprise TF_TOKEN_<host> bearer is an
  INFIX _TOKEN, so the suffix-anchored denylist missed it -- it leaked cleartext
  to the 0600 scripts.log AND expanded into an outbound HTTP header with no
  warning. Added a START-anchored _CREDENTIAL_NAME_PREFIX (TF_TOKEN_) routed
  through _matches_credential: redacted in the log, stripped from the child env,
  refused for $VAR header expansion. Benign TF_VAR_/TF_CLI_/TF_LOG untouched.
- r18-env-3 (MED): Bundler per-source BUNDLE_<host>=user:password keys the
  secret by host in the NAME, so a name-strip is wrong (it would break the
  bundle install it authenticates). Added _redact_bundler_source_credentials, a
  STRUCTURAL log-only masker of the user:PASSWORD pair: the password half is
  redacted in scripts.log while the var still reaches the child env intact.
  Benign BUNDLE_PATH/BUNDLE_JOBS config and mirror-URL values are not damaged.
- r18-parser-1 (HIGH): the round-13 expansion-weight guard charged a FLAT
  weight of 1, bounding occurrence COUNT but not BYTE size. PyYAML's representer
  reports ignore_aliases()==True for str/int/float/bytes/bool, so on DUMP a
  shared scalar is re-emitted once PER alias occurrence. A ~50KB scalar aliased
  ~150x composes as ~150 nodes (passing the node-count budget) yet re-serializes
  to ~7.5MB -- and aliased tens of thousands of times to ~GBs, hanging/OOMing
  the emitter on the pre-trust apm install / apm uninstall apm.yml round-trip.
  Made the leaf weight BYTE-AWARE (_leaf_byte_cost charges max(1, len(value)))
  so the budget models the real dump-amplification cost and the bomb fails
  closed as a yaml.YAMLError at PARSE, before any dump sink runs. A single large
  scalar referenced a handful of times stays under budget and still resolves.

CI repairs folded in:
- tests/unit/deps/test_aggregator.py: mock target -> load_frontmatter (r16
  routed frontmatter through the bounded loader).
- tests/red_team/env_sweep/test_round16_env.py + test_round17_env.py: CodeQL
  py/incomplete-url-substring-sanitization fixed via urlparse hostname equality
  (tests.instructions.md no-substring rule).
- Shard-2 coverage lift (top-level tests so pytest-split places them in group 2):
  test_lifecycle_cli.py (NEW, lifecycle.py 0->63%), test_lifecycle_executor_paths
  (executor failure/SSRF/HTTP branches), test_yaml_io (bounded-merge happy path +
  atomic writer), test_redaction_helpers. Per-shard: 66.57% / 60.48%.

Regression traps:
- tests/red_team/env_sweep/test_round18_env.py (14): TF_TOKEN_ log-redaction +
  child-env strip + header-expansion block + benign TF_VAR survival; Bundler
  password log-mask + child-env survival + benign config/mirror not damaged.
  Secret values assembled at runtime (no contiguous literal -> push-protection).
- tests/red_team/parser_sweep/test_round18_fat_scalar_dump.py (6): fat-scalar
  fanout fails closed fast at load_yaml_str + path load_yaml; daemon-thread
  watchdog proves the dump sinks are never reached; controls (large-scalar-few-
  refs + benign manifest) still round-trip through dump_yaml/yaml_to_str.

Verify: red_team 838 passed; shard 1 66.57% / shard 2 60.48% (8966 / 8943
passed); lint mirror green (ruff, pylint R0801 10/10, auth-signals, file
lengths < 2450, ASCII, yaml/relative_to guards). NOT terminal (env+parser
broke). Round-19 confirming sweep next (NO cap).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread tests/unit/test_redaction_helpers.py Fixed
danielmeppiel and others added 15 commits June 28, 2026 11:42
… parse DoS (round 19)

Round-19 adversarial sweep (env/parser/trust/http/install) found two
genuine breaks; trust, http, and install were CLEAN.

r19-env-1 (HIGH): a credential base64/hex-encoded into a single env var
named <token>_BASE64 / _B64 (a ubiquitous CI convention -- GCP_SA_KEY_BASE64,
TLS_PRIVATE_KEY_BASE64, JWT_SECRET_B64) put the credential token INFIX so
the suffix-anchored denylist missed it: the secret leaked to scripts.log,
the command-script child env, and outbound HTTP-header $VAR expansion. Fix:
append a token-anchored encoding tail to _CREDENTIAL_DENYLIST and add a
KUBE_CONFIG-with-required-encoding recognizer to _CREDENTIAL_BLOB_SUFFIX
(KUBE_CONFIG_BASE64). Token-anchored keeps benign assets (IMAGE_BASE64,
LOGO_B64, COLOR_HEX) and the bare KUBECONFIG path var reaching the child env.

r19-parser-1 (HIGH): policy/project_config.py read the untrusted project
apm.yml via stock yaml.safe_load (no merge/alias budget) on the apm install
policy gate and apm audit paths -- a sub-2KB `<<: [*a,*a]` merge bomb hung
the parser (O(2^N), GIL-held) so the except never fired. Fix: route
_read_or_default and read_project_policy_hash_pin through the bounded
load_yaml_str; the bomb fails closed as yaml.YAMLError -> existing except.

Regression traps: tests/red_team/env_sweep/test_round19_env.py (34),
tests/red_team/parser_sweep/test_round19_project_policy_safeload_bypass.py (4).
Verify: red_team 876 passed; shards 66.57% / 60.47%; lint mirror all 7 green.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…g (CodeQL)

CodeQL py/incomplete-url-substring-sanitization flagged
test_url_credentials_stripped's `assert "example.com" in out`: a host
substring of a sanitized URL can sit at an arbitrary position. Per
tests.instructions.md, assert urlparse(out).hostname == "example.com".
No behavior change; closes the lone open alert on PR #1798.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…p proxy, parser sinks)

Round-20 adversarial sweep on head 6ca602d across 5 domains: env,
install, http, parser each surfaced one genuine break; trust CLEAN.
All fixed + trapped; folds into PR #1798.

- r20-env-1 (HIGH): keystore credential blobs (_PFX/_P12/_PKCS12/_JKS/
  KEYSTORE, optionally encoding-suffixed) matched neither the suffix
  denylist nor blob-names -> base64/hex keystore leaked cleartext to
  the 0600 scripts.log AND survived into the child env + $VAR header
  expansion. Added a token-anchored keystore arm to the credential
  matcher. Benign KEY_/STORE_ names unaffected.
- r20-install-1 (HIGH): per-stream subprocess output capture was
  unbounded -- a lifecycle script emitting GBs of stdout/stderr OOMs
  the parent before the trust/redaction layer runs. Replaced the
  communicate() capture with a bounded, threaded _capture_bounded
  (per-stream _MAX_CAPTURE_CHARS cap + stdin feed + drain + wait),
  plus a non-finite-timeout fast-reject guard so a NaN deadline cannot
  disable the watchdog.
- r20-http-1 (MED): an attacker-controlled HTTP(S)_PROXY env var could
  re-route a lifecycle http event through a proxy (SSRF/exfil), because
  requests honors trust_env by default. Set session.trust_env=False AND
  explicit proxies={http:None,https:None} on both the guarded session
  and the bare-requests fallback.
- r20-parser-1 (HIGH): two policy safe_load sinks (discovery garbage
  detector + parser parse_policy) bypassed the bounded loader -> a
  merge/alias bomb in a policy doc was a parse-time CPU/OOM DoS. Routed
  both through load_yaml_str (the round-12/13 bounded _BoundedSafeLoader).
- trust: CLEAN (12 consecutive).

Unit-contract repairs (the _capture_bounded refactor replaced the
proc.communicate seam two unit files mocked): converted the affected
tests in tests/unit/core/test_script_executors.py and
tests/unit/test_lifecycle_executor_paths.py to patch the new
_capture_bounded seam (faithful _execute_command orchestration boundary).

Regression traps: env_sweep/test_round20_env.py,
install_sweep/test_round20_output_capture_oom.py (+_workers/emit.py),
http_sweep/test_round20_proxy_and_resource.py,
parser_sweep/test_r20_policy_parse_bypass.py.

Verify: red_team 907 passed; CI shard1 66.57% (8966) / shard2 60.49%
(8943); lint mirror all 7 green.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…concile)

Round-21 adversarial sweep (5 domains, opus/high) off 6ca119b:
env BREAKS x2, parser BREAKS x1, http BREAKS x1; trust + install CLEAN.
All genuine breaks fixed + trapped; the corporate-egress proxy reconcile
from the round-20 over-tightening folded in alongside.

- r21-env-1 (HIGH): encoding-tail credential names ending BASE32/B32
  (TOTP/2FA seeds, base32-armored tokens) slipped the suffix denylist
  (round-19 covered BASE64/B64/HEX/PEM/DER/ASC only). Leaked cleartext
  to 0600 scripts.log AND survived into child env + $VAR header
  expansion. Fix: add BASE32|B32 to all three encoding-tail arms
  (_CREDENTIAL_DENYLIST + _CREDENTIAL_BLOB_SUFFIX keystore + KUBECONFIG).
- r21-env-2 (MED): the literal AUTHORIZATION env name (a raw bearer
  header value) was an infix-only token; the suffix-anchored denylist
  missed it. Added AUTHORIZATION to _CREDENTIAL_DENYLIST alternation
  (AUTH_URL / OAUTH_* stay benign -- no AUTHORIZATION boundary).
- r21-parser-1 (MED): two bundle JSON readers (local_bundle
  read_bundle_plugin_json, local_bundle_handler _parse_bundle_mcp_servers)
  caught only json.JSONDecodeError; an oversized-int literal raises a
  bare ValueError (int_max_str_digits) and a deeply-nested doc raises
  RecursionError, both escaping and crashing the install/uninstall read
  of an untrusted bundle's plugin.json / .mcp.json. Widened both to
  (OSError, ValueError, RecursionError) -> fail closed like siblings.
- r21-http-1 (MED): lifecycle http dispatch had NO total wall-clock
  deadline -- requests/urllib3 treat a scalar timeout as PER-RECV, so a
  slow-loris dribble (one byte under the per-recv interval) resets it
  forever and holds the dispatch thread far past its configured timeout
  (measured 8.1s under timeout=1.0); the sequential join in
  `apm lifecycle <event>` then compounded to ~480s. Fix: run the post on
  an inner daemon worker with worker.join(total_deadline), abandon on
  expiry (daemon never blocks process exit, logs a timeout), and pass
  timeout=(connect, total_deadline). _coerce_http_deadline clamps the
  attacker-controlled timeoutSec to _MAX_HTTP_TIMEOUT=30s. Residual
  (accepted MED): a CONTINUOUS dribble leaks the abandoned daemon
  thread/socket until the short-lived install process exits.
- proxy reconcile (round-20 corrective): restore corporate-proxy
  egress on the DIRECT path via _environ_proxies_for(url) while keeping
  the SSRF destination gate + direct-path pin intact, so a corporate
  HTTPS_PROXY tunnels but an attacker-set proxy cannot re-route a
  guarded dispatch.

Stale-test corrections (same precedent as the round-13 yaml-bomb):
the prior per-recv-scalar timeout contract is rewritten to the
(connect, read)-tuple + clamp contract in http/test_redirect_and_timeout.py
(test_unusual_timeout_is_clamped) and http_sweep/test_response_cap.py.

Regression traps (+ workers): env_sweep/test_round21_env.py,
parser_sweep/test_round21_bundle_json.py, http_sweep/test_round21_http.py,
install_sweep/test_round21_install.py (+_workers/rt21_*.py),
trust_sweep/test_round21_trust.py.

Verify: red_team 955 passed; CI shard1 66.53%/8966, shard2 60.50%/8943;
lint mirror all 7 green (ruff, pylint R0801 10/10, auth-signals,
len<2450, ASCII, yaml/relative_to guards).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fifth-domain adversarial sweep on head 27a415f surfaced 5 genuine
breaks across 4 domains; trust CLEAN. All fixed + trapped.

- r22-env-1 (HIGH) / r22-env-2 (MED): credential names whose tail is a
  serialization (_JSON/_YAML/_YML/_TOML) or crypto base-encoding
  (_BASE58/_BASE62/_ASCII85/_A85/_Z85/_URL[_]SAFE) slipped the
  encoding-tail denylist (round-19/21 covered base64/32/hex/PEM/DER/ASC
  only): a GCP service-account JSON, a Solana base58 key, a TOTP base32
  seed leaked cleartext to scripts.log AND survived into child env +
  $VAR header expansion. Widened the token-anchored encoding tail to all
  three families (ASC kept last so ASCII85 matches first); token-less
  CONFIG_JSON/PACKAGE_JSON/IMAGE_BASE64 stay benign.

- r22-install-1 (MED): the round-20 threaded _capture_bounded returned
  on the shell leader's exit while a backgrounded same-group grandchild
  still held the capture pipes -- wedging the drains for the full 5s
  join budget and leaking the grandchild + its fds + two drain daemons.
  Fix: after a clean exit, grace-join the drains (_CAPTURE_DRAIN_GRACE
  0.5s); only if a drain is STILL alive (a group member holds the pipes)
  reap the group to release them. A daemon that correctly redirects its
  stdio (nohup svc >log 2>&1 &) EOFs within the grace and survives --
  matching npm/yarn; only the pipe-wedging case is reaped.

- r22-http-1 (HIGH): per-event http dispatch spawned one daemon worker
  with no live cap, so a run of slow/abandoned endpoints could leak
  unbounded daemon threads + sockets. Gate each dispatch behind a
  module-level BoundedSemaphore(MAX_HTTP_DISPATCH_THREADS=32),
  non-blocking acquire (drop+log on exhaustion, no per-entry stall),
  released in the worker's finally + on start() failure (exactly-once).

- r22-parser-1 (MED): five JSON readers (plugin_manifest collect/synth,
  plugin_exporter hooks-from-apm/-root) caught only
  (json.JSONDecodeError, OSError); an oversized-int (int_max_str_digits
  -> bare ValueError) or deep-nested (RecursionError) plugin.json on an
  untrusted clone escaped + crashed apm pack --format plugin. Widened to
  (OSError, ValueError, RecursionError) -> fail closed.

Traps (+ install holdpipe worker): test_round22_{env,parser,http,
install,trust}.py. Stale-test corrections (round-13 precedent):
test_success_path_grandchild_holds_stdout_join_delay -> ...reaped...
(old ~10s join bound -> prompt reap + empty-group assertion);
test_round21_trust.py hardcoded /tmp/rt21-trust tmpdir -> default
tempdir (reaped worktree made it error on collection).

red_team 1034 passed; CI shard1 66.51% / shard2 60.51%; lint mirror all
7 green.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Round-23 5-domain adversarial sweep (env/parser/install/http/trust):
four genuine breaks fixed + trapped; trust CLEAN. NOT terminal.

- r23-env-1 (HIGH): session-token credential names BW_SESSION /
  OP_SESSION / FASTLANE_SESSION (Bitwarden/1Password/Fastlane unlock
  tokens) and the OP_SESSION_<vault> / TF_TOKEN_<host> infix families
  escaped the suffix-anchored denylist -> leaked cleartext to 0600
  scripts.log AND survived into the child env + $VAR header expansion.
  Add the exact session-blob names to _CREDENTIAL_BLOB_NAMES and a
  START-anchored _CREDENTIAL_NAME_PREFIX (TF_TOKEN_ | OP_SESSION_).

- r23-parser-1 (HIGH): marketplace/audit.py::fetch_plugin_apm_yml read
  a fetched plugin apm.yml via stock yaml.safe_load, bypassing the
  round-12 merge budget + round-13 alias-expansion guard -> a hostile
  marketplace plugin could wedge `apm marketplace audit` with a
  billion-laughs merge/alias bomb. Route through load_yaml_str.

- r23-install-1 (HIGH): a setsid-detached escapee grandchild outliving
  its process group made the bounded post-kill settle join wait the
  full budget (~10.5s install hang). Bound the settle join; the
  escapee SURVIVES by design (npm/yarn parity) -- only the parent's
  wait is bounded.

- r23-http-1 (MED): round-22 regression -- a CONTINUOUS slow-loris
  dribble pinned a BoundedSemaphore permit for the process lifetime
  (the abandoned daemon never released it). Record the dispatch socket
  and force-close it at the deadline so the wedged worker unblocks and
  releases the permit; gate the 1.0s abandon-grace join on whether a
  socket was actually recorded (no-socket bare-requests path abandons
  immediately, preserving round-22 latency).

Corporate-proxy egress on the direct path is preserved (regression-
trapped): the SSRF destination gate + guarded-path pin stay intact
while HTTPS_PROXY tunnelling continues to work.

Regression traps (+36): env_sweep/test_round23_session_cred_gap.py,
parser_sweep/test_round23_marketplace_audit_safe_load.py,
trust_sweep/test_round23_trust.py,
http_sweep/test_round23_semaphore_starvation.py (reclaim + proxy-egress
contract), install_sweep/test_round23_setsid_reap_escape.py
(+_workers/rt23_setsid_holdpipe.py).

The round-23 http fix re-routes the direct egress path through a
capturing session before the bare requests.post fallback; the pre-
existing http hermetic-seam tests (http_sweep/http/install conftests +
TestDispatchHttpRequest unit tests) are updated to neutralize the new
capturing-session layer alongside the guarded session, mirroring the
round-20 unit-contract-repair precedent. The round-14 drift legit-parse
control is updated to main's post-#1924 canonical-token-list contract
(security assertion unchanged).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…adata bomb, _feed false-reap

Adversarial round-24 sweep (env/parser/install/http/trust) against the
lifecycle-scripts framework. http + trust returned clean; four genuine
breaks fixed across env/parser/install, each with a regression trap.

env (script_executors.py _CREDENTIAL_DENYLIST):
- r24-env-1 (MED): MFA_PASSCODE/VPN_PASSCODE/*_PASSCODE are password-class
  secrets but dodged the denylist -- the (?:^|_)PASS arm cannot reach them
  (trailing CODE blocks the end anchor) and there was no PASSCODE token, so
  they leaked into the child env, expanded into outbound headers, and
  persisted cleartext in scripts.log. Add a dedicated PASSCODE token; benign
  *_CODE names (BARCODE/ZIPCODE/STATUS_CODE/...) never contain PASSCODE.
- r24-env-2 (LOW): bare JWT/ACCESS_JWT/REFRESH_JWT bearer names dodged the
  denylist (the eyJ... value matches no structural masker) and leaked to the
  log + child env. Add a trailing-anchored JWT token; leading-JWT config
  names (JWT_ALGORITHM/JWT_ISSUER) keep a non-token tail and stay benign.

parser (marketplace/builder.py metadata readers):
- r24-parser-1 (HIGH): _fetch_local_metadata / _fetch_remote_metadata used
  stock yaml.safe_load + str(version) -- an aliased version bomb in an
  untrusted package apm.yml dump-amplified a ~400-byte file to a 226MB
  string on `apm pack --check-clean`. Route both readers through the bounded
  load_yaml_str (the expansion-weight guard fails the bomb closed) and cap
  the remote resp.read() at the same byte ceiling as the local reader.

install (script_executors.py _capture_bounded):
- r24-install-1 (HIGH): the reap predicate counted the _feed stdin-writer
  thread, not just the stdout/stderr drains. On a many-package install the
  event JSON exceeds the OS pipe buffer, so _feed blocks writing to a legit
  detached daemon that redirected its stdio but never reads stdin; the
  still-alive _feed wrongly triggered killpg, killing the daemon the round-22
  contract protects. Key the reap decision off the drains (workers[1:]) only
  -- a blocked _feed means stdin is merely unconsumed, never a held pipe.

All four traps fail at the parent head and pass after the fix; the round-22
redirected-daemon-survives and round-23 escapee/socket contracts stay green.
Test fake _FakeHTTPResponse.read now honors the optional amt arg to match the
real HTTPResponse.read([amt]) contract the capped reader uses.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…JSON cap, log rotation lock

Round-25 adversarial sweep on eb48f93 surfaced 3 genuine MED breaks
(env/parser/install); trust+http CLEAN. All fixed + trapped.

- r25-env-1 (MED): bearer-token JWT VALUES (eyJ...eyJ...) echoed to
  scripts.log were not redacted under any env name -- the name denylist
  only catches credential VARIABLES, not a raw JWT literal appearing in
  stdout/stderr. Added _JWT_VALUE_PATTERN (double-eyJ structural anchor,
  zero false positives) + _redact_jwt_values wired into _redact_secrets.
- r25-parser-1 (MED): marketplace API JSON readers (_fetch_via_api,
  _do_fetch, _fetch_ado_rest) used buffered resp.json()/resp.text,
  bypassing the 10 MiB _MAX_MARKETPLACE_JSON_BYTES cap -- a multi-GB
  API body OOMs the client before any size check. Switched to
  stream=True + iter_content via _read_capped_json (try/finally close).
- r25-install-1 (MED): scripts.log rotation was an unlocked
  read-rename-recreate -- two concurrent install processes rotating at
  the threshold lost-update the log. Added fcntl.flock on a dedicated
  scripts.log.lock + double-checked re-stat under the lock.

Marketplace unit fakes corrected for the stream=True contract
(11 closures + 3 helpers gain **kwargs / iter_content / real headers
dict) -- the stale-test-correction precedent (rounds 13/14/21-24): the
fix changed the response-reading seam, so the control fakes are updated
to the new contract, never the security fix reverted.

red_team 1085 passed; CI shard1 66.46% / shard2 60.51%; lint 7/7 green.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-store lock O_NOFOLLOW degrade-graceful, non-blocking rotation lock, registry JSON fail-closed cap

Round-26 adversarial sweep (env/parser/install broke; trust+http clean):

- r26-env-1 (MED): provider-token VALUES (gh*/github_pat_/xox*/AKIA.../
  sk-/glpat-) echoed to scripts.log were not masked under any env name.
  Added _PROVIDER_TOKEN_PATTERN + _redact_provider_tokens into
  _redact_secrets (structural value masker, zero false positives).

- r26-parser-1 (MED): RegistryClient read bodies via buffered resp.json()/
  .content -> a multi-GB or deeply-nested registry JSON OOM'd/recursed the
  client before any cap. Added _read_capped_body/_decode_capped_json/
  _safe_problem_json (stream=True + iter_content under a byte cap;
  fail-closed RegistryError on ValueError/JSONDecodeError/RecursionError/
  UnicodeDecodeError). Routed _response_json + 3 error branches through it.

- r26-install-1 (MED): scripts.log rotation lock now LOCK_NB skip-on-
  contention (no rotation-time stall); trust-store lock os.open with
  O_NOFOLLOW+0o600 (no O_TRUNC) inside suppress(OSError) -> symlink-swap
  fails closed/degrades gracefully, dir mode 0o700.

Stale-test corrections (rounds 13/14/21-25 precedent): the parser fix moved
the registry response-read seam from buffered .json()/.content to streamed
iter_content under a cap. Repaired the control fakes (NOT the security fix):
tests/unit/registry/test_client.py _make_response and
tests/integration/test_deps_registry_coverage.py _streamed_resp now expose a
real headers dict + .url + iter_content + .content matching the streamed
contract. A MagicMock headers/empty iter_content silently lost the body.

Regression traps (+35): env_sweep/test_round26_provider_token_values.py,
parser_sweep/test_round26_registry_json_recursion.py,
install_sweep/test_round26_install_concurrency.py (+_workers/flock_holder.py),
http_sweep/test_round26_{proxy_and_bomb,realserver}.py,
trust_sweep/test_round26_trust.py.

Verify: red_team 1120 passed; deps-registry integration 113 passed;
CI shard1 66.45% (8980) / shard2 60.54% (8956); lint mirror all 7 green
(ruff check/format, pylint R0801 10/10, auth-signals, file-len<2100,
ASCII, yaml/relative_to guards).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rketplace JSON cap

Round-27 5-domain adversarial sweep (env/parser/trust/http/install) on
fb009d3: trust + http + install CLEAN; env x2 + parser x2 genuine breaks.
All fixed + trapped.

- r27-env-1 (env, MED): session/cookie credential names ending COOKIE
  (e.g. SESSION_COOKIE, _COOKIE blobs) escaped the suffix-anchored
  denylist -> leaked to 0600 scripts.log + survived child env + $VAR
  header expansion. Added (?:^|_)COOKIE arm to _CREDENTIAL_DENYLIST.
- r27-env-2 (env, MED): provider-token VALUE shapes (glpat-/xapp-/ya29./
  hf_/dop_v1_/tskey-) echoed to scripts.log were not masked under any env
  name. Extended _PROVIDER_TOKEN_PATTERN with 6 structural arms (zero
  false positives).
- r27-parser-1 (parser, HIGH): MCP SimpleRegistryClient (registry/client.py)
  read bodies via buffered .json()/.content -> a multi-GB or deeply-nested
  registry JSON OOM'd/recursed the client before any cap. Added
  _read_capped_body/_decode_registry_json (stream=True + iter_content under
  a byte cap, fail-closed); routed all 3 get sites + 2 cache-read guards.
- r27-parser-2 (parser, MED): marketplace _read_capped_json + _fetch_url_direct
  json.loads guards did not catch RecursionError (deeply-nested URL JSON ->
  bare RecursionError escaped). Added RecursionError to both except tuples.

Stale-test corrections (rounds 13/14/21-26 precedent): the r27-parser-1
stream switch changed the MCP registry response-read seam from buffered
.json()/.content to streamed stream=True + iter_content. Repaired the
control fakes (NOT the security fix): test_registry_client.py (_streamed_response
helper + ~9 mock conversions + stream=True asserts + _http_cache=None),
test_registry_client_http_cache.py (_mock_response +iter_content/close/url),
test_mcp_registry_module.py (R fake +headers/url/iter_content/close,
_http_cache=None). headers must be a REAL dict (Content-Length int parse);
iter_content must yield the body (a MagicMock yields empty -> json.loads("")).

Verify: red_team 1139 passed; CI shard1 66.46% (8980) / shard2 60.53%
(8956); lint mirror all 7 green (ruff check/format, pylint R0801 10/10,
auth-signals, file-len<2100 -- script_executors 2021, ASCII, yaml/relative_to
guards).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…king + Contents-API RecursionError fail-closed

Adversarial sweep round 28 (env/parser/trust/http/install, 5 parallel
domains). trust (20th consecutive clean), http (corporate-proxy egress
preserved + SSRF gate not proxy-bypassable), and install (>1MiB
capture-boundary deadlock disproven end-to-end) returned CLEAN. env and
parser each surfaced one genuine break; both folded here with red-before
/ green-after regression traps.

env (r28-env-1, HIGH): a freshly minted HashiCorp Vault token
(hvs./hvb./hvr. service/batch/recovery) printed to a lifecycle script's
stdout has NO backing env var, so the os.environ value-redactor never
sees it, and it carries no @-userinfo / password= / eyJ.eyJ structural
delimiter -- so every structural masker missed it and the cleartext
bearer persisted in the 0600 ~/.apm/logs/scripts.log. _PROVIDER_TOKEN_
PATTERN gains a hv[sbr]\.[A-Za-z0-9]{24,} arm (literal-dot + 24-char
base62 floor rejects benign hvac.py / hvs.short lookalikes). The same
value-shape gap also leaked npm automation (npm_<36>), PyPI (pypi-...)
and Anthropic (sk-ant-...) tokens -- arms added for each.

parser (r28-parser-1, MED): DownloadDelegate._extract_contents_api_
payload decodes an untrusted non-GitHub (Gitea/Gogs) Contents-API JSON
envelope on the apm install path, but its fail-closed except tuple
omitted RecursionError (a RuntimeError subclass). A malicious host
returning deeply-nested JSON crashed apm install with an uncaught
traceback instead of degrading to the raw-body fallback. RecursionError
added to the except tuple -- same fail-closed class already closed at
the r21/r22/r27 JSON sinks, now routed at this sink.

Gates: ruff check/format clean; file-length 2025/1329 < 2100; YAML-IO
and relpath guards clean (untouched); pylint R0801 10.00/10; auth-
signals clean. CI shard1 8980 passed 66.46%; shard2 8956 passed 60.53%.
Full env+parser red_team 615 passed; download_strategies+script_executors
unit 317 passed (no fakes broken -- both fixes are pure additions).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…log-path fail-closed + marketplace oversized-int fail-closed

Round-29 adversarial sweep (5 domains): env BROKE x2, parser BROKE,
install BROKE; trust + http CLEAN. Four genuine breaks fixed + trapped.

- r29-env-1 (HIGH): AWS access-key id (AKIA/ASIA + 16 base32, STS-minted,
  no env var backing) leaked cleartext to 0600 scripts.log. The provider-
  token VALUE masker had no AWS arm. Fix: add (?:AKIA|ASIA)[A-Z0-9]{16}
  to _PROVIDER_TOKEN_PATTERN.
- r29-env-2 (HIGH): SendGrid SG.<22>.<43> API key leaked to scripts.log.
  Fix: add SG\.[A-Za-z0-9_-]{22}\.[A-Za-z0-9_-]{43} (rigid lengths exclude
  dotted module paths -> zero false positives).
- r29-install-1 (HIGH): a FIFO planted at the scripts.log path wedged
  install forever -- _append_to_script_log used O_NOFOLLOW (blocks symlinks,
  NOT FIFOs) without O_NONBLOCK, so os.open blocked on a no-reader FIFO.
  Fix: add O_NONBLOCK (no-reader FIFO open raises ENXIO, caught by the
  existing except) + S_ISREG fail-closed fstat check after open (closes the
  reader-present window) + S_ISREG guard in _rotate_log_if_large.
- r29-parser-1 (HIGH): an oversized-int ValueError (CPython int_max_str_digits,
  >4300 digits) escaped marketplace _fetch_url_direct's json.loads guard --
  JSONDecodeError is a ValueError subclass but a bare ValueError from int
  conversion was not in the narrow except tuple -> uncaught CLI crash on a
  hostile URL JSON. Fix: add bare ValueError (+ RecursionError already present)
  to _fetch_url_direct and _read_capped_json except tuples (fail closed).

Regression traps (43 tests, 5 probes): env (AWS/SendGrid value masking +
6 benign AWS/SendGrid look-alike FP-guards), parser (oversized-int fail-closed
at validate/fetch), install (FIFO log-path bounded + later-scripts isolated),
trust (12 invariant probes, CLEAN), http (14 proxy-pin/bounds probes, CLEAN;
corporate HTTPS_PROXY egress re-confirmed functional + safe).

Red-before: 8 break tests failed on HEAD. Green-after: all 43 pass.
Verify: red_team env+parser+install 685 pass; unit script_executors+marketplace
+lifecycle 1500 pass; lint mirror all 7 green (ruff, pylint R0801 10/10,
auth-signals, file-len 2045/1256<2100, ASCII, yaml/relpath guards);
CI shard1 8980 pass 66.46%, shard2 8956 pass 60.53%.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…+ registry config bounded-loader + self-healing audit log

Round-30 adversarial sweep (env/parser/trust/http/install) found 4
genuine breaks; trust (22nd consecutive) and http (7th; corporate
HTTPS_PROXY egress re-verified functional+safe) remained clean.

- env (HIGH): Shopify Admin API tokens (shp{at,ca,pa,ss}_ + 32 hex)
  leaked cleartext to scripts.log; add a rigid zero-FP arm to
  _PROVIDER_TOKEN_PATTERN.
- env (MED): Stripe TEST-mode secret/restricted keys (sk_test_/rk_test_)
  leaked while only live keys were masked; broaden the existing arm to
  [sr]k_(?:live|test)_ to close the consistency gap.
- parser (HIGH): a poisoned workspace ~/.apm/apm.yml merge-key alias
  bomb drove PyYAML flatten_mapping to O(3^N) CPU and wedged nearly
  every default command, because registry config_loader read it with
  stock yaml.safe_load (not the bounded loader). Route
  _load_yaml_registries through utils.yaml_io.load_yaml so the bounded
  loader fails closed (yaml.YAMLError) in milliseconds.
- install (MED, audit-evasion): the round-29 FIFO log fix dropped a
  planted non-regular log node but left it in place, permanently
  blackholing the audit log for this AND all later installs. Make the
  log open self-healing: unlink + O_EXCL retry on both the ENXIO and
  non-S_ISREG branches; a same-instant re-plant drops only the racing
  write (bounded), never a persistent blackout. Daemon-survival surface
  untouched.

Regression traps: 9 red_team probes (+3 _workers helpers). Red-before
confirmed 12 break assertions fail on HEAD; green-after all pass.
719 red_team + 1528 unit pass; 7-gate lint clean (script_executors.py
2063 < 2100); CI shard1 8980/66.45%, shard2 8956/60.54%.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… x3)

Round-31 adversarial sweep (5 parallel domains) found 6 genuine breaks
across 3 domains; trust (23rd consecutive clean) and http (8th) CLEAN.
Corporate HTTPS_PROXY egress re-verified functional + safe.

env (script_executors.py):
- r31-env-1 (MED): descriptive-suffix names (AUTHORIZATION_HEADER,
  SECRET_VALUE, TOKEN_VALUE) defeated the end-anchored credential
  denylist; an opaque/Basic Authorization header value leaked. Added a
  (_HEADERS?|_VALUES?|_DATA) suffix group before the encoding-tail
  anchor + curated AUTH_HEADER/AUTH_HEADERS into the credential blob
  names (an Authorization header value IS a bearer secret).
- r31-env-2 (LOW): age/sops AGE-SECRET-KEY-1... master key VALUE was
  unmasked in scripts.log. Added a Bech32 fixed-width provider arm
  (zero false positives).

install (script_executors.py):
- r31-install-1 (MED): the round-30 self-heal S_ISREG gate ignored file
  MODE; a pre-planted 0666 regular scripts.log appended forever without
  tightening. Extended the gate to fstat + (st_mode & 0o077) on both the
  open and the O_EXCL retry so a world/group-accessible log is reaped.

parser (plugin_parser.py, config_loader.py):
- r31-parser-1 (HIGH): .mcp.json / .lsp.json readers caught only
  (JSONDecodeError, OSError); a deep-nest RecursionError or huge-int
  ValueError escaped and crashed apm install. Added a shared
  _bounded_read_json helper (5MiB size cap -> single ValueError funnel)
  routed through both readers + parse_plugin_manifest +
  normalize_plugin_directory + validate_plugin_package.
- r31-parser-3 (LOW): ~/.apm/config.json registry-merge load was
  unguarded; wrapped _load_config_json_registries fail-closed -> {}.

Cross-round reconcile: round-25's value-masker demo used AUTH_HEADER as
its "benign name" example; round-31 proved AUTH_HEADER is a credential
carrier, so the demo was repointed to genuinely-benign RESPONSE_BODY
(intent preserved: value-masking catches structural secrets regardless
of name).

Regression traps (+99 cases): env_sweep/test_round31_leak_channels.py,
install_sweep/test_round31_{logmode_widening,selfheal_toctou}.py,
parser_sweep/test_round31_{plugin_json_chaos,config_json_chaos}.py,
trust_sweep/test_round31_trust.py,
http_sweep/test_round31_transitional_and_egress.py (+_workers).
Red-before: 17 break assertions fail on HEAD; green-after all pass.

Verify: CI shard1 8980 pass 66.45%, shard2 8956 pass 60.53%; 7-gate
lint mirror green (ruff, pylint R0801 10/10, auth-signals, file-len
< 2100, ASCII, yaml/relpath guards).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread src/apm_cli/core/script_trust.py Fixed
danielmeppiel and others added 2 commits June 28, 2026 19:06
…dening

APM operates in a SHARED-RESPONSIBILITY model: it is not a third-party
secret scanner. The lifecycle-script redaction layer had grown a
shape-based scanner that detected/masked the script author's OWN
provider secrets (JWT/AWS/SendGrid/Vault/Slack/GitLab/... by VALUE
SHAPE) in script stdout/stderr. Detecting another platform's secrets
by shape is the script author's responsibility, not APM's. Per
maintainer direction, remove that scanner and keep only what APM owns.

Stripped (out of scope -- script author owns):
- 9 value/structural maskers + their regex constants:
  _redact_jwt_values, _redact_provider_tokens,
  _redact_embedded_url_credentials, _redact_bundler_source_credentials,
  _redact_connection_string_password, _redact_webhook_urls,
  _redact_sas_signatures, _redact_pem_private_keys, and the original
  shape-scanning _redact_url_credentials.
- _redact_secrets now returns after Part-1 (APM-managed-credential
  NAME-based value masking only).

Kept (APM owns):
- NAME-based handling: _matches_credential + the credential NAME
  denylist/blob/prefix sets; Part-1 value masking of APM-managed,
  denylisted-NAME env vars; _NEVER_EXPAND header guard.
- Log hygiene of APM-OWNED fields: minimal urllib-based
  _redact_url_credentials strips userinfo from the http EVENT URL
  before APM writes it to scripts.log; wired as the logged safe_url in
  _prepare_http (dispatch still uses the raw url).
- newline/header/truncate log helpers.
- Surviving in-scope robustness from the round-32 sweep: marketplace
  local/git JSON parser widened excepts (APM owns its own parsers) plus
  http resource-bound, install audit-log, trust, and parser regression
  traps.

Tests: deleted 9 pure scanner-value env_sweep files; pruned 16 scanner
functions from 6 mixed files (keeping each file's NAME-denylist /
Part-1 / newline siblings); retargeted tests/unit/test_redaction_helpers
to the name-handling + scope-boundary contract.

Net: script_executors.py 2086 -> 1844 lines. red_team 1261 passed;
both CI shards green (66.43% / 60.51%); 7-gate lint mirror clean.

Closes the uncapped hardening loop; lifecycle redaction is now scoped
to APM-managed credentials only.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ug logs

CodeQL py/clear-text-logging-sensitive-data (high) flagged the apm.yml
lifecycle fingerprint debug log: a YAML parse exception can echo file
content into the log. Log the exception class name instead of its value
in both script_trust fingerprint debug sinks. APM owns log hygiene of
its own parse errors; no behavior change.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread src/apm_cli/core/script_trust.py Fixed
danielmeppiel and others added 2 commits June 28, 2026 19:19
…odeQL)

CodeQL py/clear-text-logging-sensitive-data keys on the apm.yml `path`
interpolated into the fingerprint-failure debug log (the sibling sink at
line 198 logging only the exception type is not flagged). Make the debug
message fully static -- no runtime value reaches the log sink. Debug
context loss is negligible (single known project apm.yml).

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…into CI

The tests/red_team corpus had grown to 188 files / ~25k lines during the
adversarial hardening campaign, none of which CI collected (the shard run
only globs tests/unit + tests/test_console.py). A large body of tests that
never executes is a merge blocker, not coverage.

Split the corpus by quality:

- DELETE the *_sweep/ directories (~134 files / ~20.8k lines). These are
  per-round campaign dumps (test_round21..round32) with heavy cross-round
  overlap, an unregistered `e2e` marker, and 3 files asserting the
  now-stripped shape-based secret scanner. Their load-bearing vectors are
  already represented by the curated suite.

- KEEP the curated per-concern suite (49 test files / 279 tests, ~4.3k
  lines): trust TOCTOU / kill-switch / canonicalization / tier-confusion /
  store-integrity, SSRF destinations + IP-encodings + header-injection,
  parser yaml/json bombs + huge-manifest, credential-NAME denylist +
  never-expand + log-redaction, and install containment + event-ordering +
  orphan-process. One concern per file, named by behavior.

- WIRE the curated suite into the CI shard run (pytest ... tests/red_team).
  279 tests, ~6s, pytest-split shards them across both runners. Every
  red_team line now executes on every PR.

Net: tests/red_team goes 188 files/+25,148 -> 54 files/+4,350, and the
suite that survives is the one that actually runs.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

danielmeppiel commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

PR #1798 - Lifecycle Scripts Framework: Standalone Engineering Report

A cross-cutting review-panel report (python-architect, devx-ux-expert,
supply-chain-security-expert, cli-logging-expert), synthesized. Grounded
in the PR head at 16a0a3183. ASCII-only.


0. Executive Summary

PR #1798 (closes #1529) adds an event-triggered lifecycle scripts
framework
to APM: a project / user / admin can register command or
http actions that fire on package-manager events (post-install,
pre-uninstall, ...). It is the third leg of an intentionally separated
noun taxonomy:

Concept Manifest key Command Status
Agent hooks (harness wiring) .apm/hooks/ (primitive) pre-existing
On-demand tasks / inference scripts: apm run <name> reserved
Event-triggered, trust-gated lifecycle: apm lifecycle THIS PR

Grounded footprint (120 files, ~11,693 insertions):

Area Files Lines Notes
src/ feature+wiring 41 +4407 / -198 6 core modules + install wiring
tests/red_team/ 54 +4350 curated regression suite (279 tests); now runs in CI
tests/unit+integ 19 +2440 the CI-collected suite
docs/ 4 +482 enterprise + reference + skill

Verdict (consolidated): SHIP. The feature is architecturally clean,
the trust model is correct, and the security floor (trust gate, SSRF
guard, bounded YAML loader, subprocess containment) is load-bearing and
genuinely safe. The 32-round hardening campaign was net positive but
ran ~3x longer than it should have
before the maintainer's scope
correction removed an out-of-scope third-party-secret scanner. The
former review-experience debt (a 253-file diff with a 188-file
CI-unrun test corpus) has been paid down in this PR: the per-round
sweep dumps (~134 files / ~20.8k lines) were deleted and the curated
behavior-organized regression suite (49 test files / 279 tests) is now
wired into the CI shard run. No residual safety defect.


1. Architecture

1.1 Module map

graph TD
    subgraph CLI
        A["commands/lifecycle.py<br/>apm lifecycle list/init/validate/test/trust/untrust"]
    end
    subgraph "Discovery and Parse"
        B["core/lifecycle_scripts.py<br/>discover_scripts: project/user/admin tiers"]
        C["utils/yaml_io.py<br/>_BoundedSafeLoader: merge/alias/byte budgets"]
    end
    subgraph Trust
        D["core/script_trust.py<br/>content-hash fingerprint of lifecycle subtree"]
    end
    subgraph Execute
        E["core/script_executors.py<br/>command + http executors, redaction, containment"]
    end
    subgraph InstallWiring
        F["install/service.py<br/>build_runner_from_context, fires on event"]
    end

    A --> B
    B --> C
    B --> D
    F --> B
    F --> D
    D -->|trusted?| E
    B --> E
    E --> C
    E -->|audit| G[("~/.apm/logs/scripts.log<br/>0600, rotated, locked")]
    D --> H[("~/.apm/scripts-trust.json<br/>0600, flock, atomic")]
Loading

1.2 Execution flow: apm install on a freshly-cloned project

sequenceDiagram
    participant U as User
    participant I as install/service.py
    participant DISC as lifecycle_scripts.discover
    participant Y as yaml_io (bounded loader)
    participant T as script_trust
    participant X as script_executors
    participant L as scripts.log

    U->>I: apm install (untrusted clone)
    I->>DISC: discover_scripts(event=post-install)
    DISC->>Y: load_yaml(apm.yml)
    Y-->>DISC: lifecycle subtree, or YAMLError fails closed
    DISC->>T: is_fingerprint_trusted(parsed subtree)
    Note over T: SHA-256 of canonical lifecycle subtree<br/>vs scripts-trust.json
    alt not trusted (default for new clone)
        T-->>I: False
        I-->>U: Skipped N untrusted lifecycle scripts
    else trusted (user ran apm lifecycle trust earlier)
        T-->>X: fire
        X->>X: strip credential-NAME env vars, build child env
        X->>X: start_new_session, bounded capture, SSRF-guard http
        X->>L: append redacted audit entry
        X-->>U: result (slow-script warning over 5s)
    end
Loading

1.3 Design decisions worth noting (python-architect)

  • Embed, not a sidecar file. Lifecycle actions live under a
    top-level lifecycle: key in the project's existing apm.yml
    (user tier: ~/.apm/apm.yml; admin tier: /etc/apm/policy.d/*.json).
    This was a unanimous package-manager DevUX panel decision (cargo
    [package.metadata] / Maven phases precedent) and makes the trust
    hash-domain equal to the auto-exec domain by construction.
  • One bounded loader, mechanical rollout. A single
    _BoundedSafeLoader in utils/yaml_io.py replaced ~30 raw
    yaml.safe_load sites across the codebase via an import swap. This
    is the campaign's biggest architectural dividend (Section 3).
  • Honest assessment: the diff was originally hard to review (a
    253-file PR, red_team 77% of insertions). Trimmed in this PR to 120
    files by deleting the per-round sweep dumps; the feature itself is a
    clean 6-module slice. Recommendation for future work: clearer
    commit-level separation (feature vs hardening sweep vs test corpus).

2. Developer Experience (devx-ux-expert)

2.1 Command surface

apm lifecycle with subcommands:

Subcommand Behavior
list (bare default) show discovered lifecycle actions across tiers
init scaffold a lifecycle: block into apm.yml
validate structural + URL validation, no execution
test dry-run by default; --execute actually fires
trust / untrust grant/revoke content-hash trust for this project

Note: the on-demand verb is reserved as apm run <name>; lifecycle
management deliberately uses a different verb (apm lifecycle),
mirroring pnpm approve-builds / bun pm trust. There is no
apm run collision.

2.2 Trust ergonomics

  • Model = direnv / VS Code Workspace Trust: the consuming repo
    re-asserts intent per edit of the lifecycle: subtree.
  • Fail-closed and visible: an untrusted project prints
    [!] Skipped ... (never silent).
  • Escape hatches: APM_NO_SCRIPTS, apm lifecycle untrust, and the
    org-policy deny_all ceiling.

2.3 Honest UX verdict

The trust gate is unambiguously the right call -- it closes the
project-tier auto-exec gap npm never solved (pnpm/bun added it later),
and APM's content-hash granularity is finer than a per-package allow.
Friction items (all minor, deferred to #1919): first-run skip on
your own project, PR-driven re-trust churn, no interactive y/N prompt,
no apm install --trust one-shot, and the test verb is slightly
overloaded (dry-run vs --execute).


3. Security (supply-chain-security-expert)

3.1 Threat model (the risks this feature introduces)

  • RCE at install -- apm install on an untrusted clone could
    auto-run the project's own lifecycle scripts.
  • SSRF / exfiltration via http events to internal/metadata hosts,
    including DNS-rebinding.
  • Parser DoS -- sub-KB YAML merge/alias bombs, reachable
    pre-trust (the manifest is parsed before the gate fires).
  • Credential leakage of APM's own tokens via log or $VAR header
    expansion.
  • Proxy re-routing via attacker-injected HTTP(S)_PROXY.

3.2 Defense-in-depth (all grounded in code)

Layer Mechanism
Trust gate (script_trust.py) SHA-256 of canonical lifecycle: subtree, keyed by resolved abs path; fail-closed None on any error; pre-canonicalization structural guard (cycle detection + node 100k / depth 64 / byte 1MB budgets); 0600 + atomic write + flock; deny_all org ceiling; TOCTOU closure (fingerprint the exact parsed content executed)
SSRF guard (script_executors.py) blocks RFC1918 / loopback / link-local / RFC6598 CGNAT / IPv6 site-local / IPv4-mapped; metadata hostnames; integer+hex IP literal decoding; DNS-pinned connect (same resolution that passed the guard is the one connected); all-addresses-checked; corporate HTTPS_PROXY preserved while attacker proxy-env cannot redirect the pinned path
Bounded YAML loader (yaml_io.py) merge-entry budget (100k), flatten-depth (200), byte-aware alias-expansion-weight guard (5M) memoized pre-construction; ValueError/RecursionError normalized to yaml.YAMLError; rolled out to ~30 sites; dump_yaml serializes-then-opens (no zero-truncation)
Subprocess containment start_new_session + killpg whole-tree reap (works even on a zombie leader); bounded per-stream capture (1MB cap -> SIGKILL); NaN/inf timeout rejection; drain-grace that preserves a correctly-detached daemon (npm/yarn parity)
NAME-based credentials denylist regex (TOKEN/SECRET/PAT/KEY/PASSWORD/...); curated blob names; _NEVER_EXPAND set (GITHUB_APM_PAT, GITHUB_TOKEN, GH_TOKEN, ADO_APM_PAT) blocked from $VAR header expansion even if allow-listed; _build_script_env strips by default; value-redaction of APM-managed names in the log

3.3 The shared-responsibility boundary (the scope correction)

  • STRIPPED (round 32): a 9-function, ~240-line shape-based
    scanner that recognized third-party secrets by VALUE shape
    (JWT eyJ..., AWS AKIA..., SendGrid, Vault hvs., Slack, GitLab,
    Stripe, Shopify, PEM, connection-string passwords, SAS, webhooks).
  • Why the maintainer was right: APM owns its own credential
    surface by NAME (it knows GITHUB_APM_PAT is a secret). It has no
    contract with AWS/Stripe/Slack and recognizing their shapes is an
    unbounded arms race that belongs in a dedicated secret scanner
    (gitleaks/trufflehog) the script author opts into. npm/pip/cargo do
    not scan lifecycle output for third-party keys either.

4. Logging / Audit (cli-logging-expert)

  • ~/.apm/logs/scripts.log -- dir 0700, file 0600; 5 MiB rotation
    to .1; non-blocking flock with double-checked re-stat (no
    lost-update clobber).
  • Records per dispatch: UTC timestamp, event name, executor type,
    effective target, status, exit code, plus truncated+redacted
    stdout/stderr. This is the forensic tuple for "what did that
    package's post-install do?"
  • Log-injection hardening: full str.splitlines() boundary-set
    neutralization (CR/LF/VT/FF/FS/GS/RS/NEL/LS/PS) so a script cannot
    forge a column-0 status=ok record; = escaping in the target
    field so no forged key=value; per-field 4096-char truncation;
    self-healing open path against planted symlink / FIFO / directory /
    0666 nodes (unlink + O_EXCL recreate; at worst one racing write is
    dropped, never a permanent audit blackout).
  • Redaction = NAME-based only post-correction: mask the VALUES of
    denylisted-NAME env vars + strip userinfo from the APM-constructed
    http event URL. No phantom "we detect all secrets" promise.
  • CodeQL fix (py/clear-text-logging-sensitive-data at
    script_trust.py): the fingerprint-failure debug message was made
    fully static (no path/exception interpolation). Correct
    false-positive suppression, not a behavior change.
  • Residual UX gaps (deferred): no apm lifecycle log viewer; no
    install-time [i] N scripts ran summary; single-generation rotation;
    no --log-format=jsonl for SIEM.

5. Honest Assessment of the 32-Round Campaign

5.1 What was genuinely won (load-bearing)

  • Bounded YAML loader across ~30 sites. Before this PR, every
    yaml.safe_load of untrusted content (marketplace responses,
    registry configs, lockfiles, policy files, installed-package
    frontmatter) was vulnerable to a sub-KB merge/alias bomb in an
    uncatchable O(2^N) GIL-holding loop. This is a codebase-wide
    security improvement, not lifecycle-specific.
  • SSRF guard with DNS-pinned connect -- closes the rebinding TOCTOU
    most ad-hoc guards miss; required for the http event type to be
    safe at all.
  • Trust gate + structural fingerprint guard -- the critical defense
    that makes the whole feature opt-in for untrusted clones.
  • Process-group containment + bounded capture -- prevents the
    command executor from being a process-bomb / OOM vector.
  • NAME-based credential stripping + _NEVER_EXPAND -- protects the
    one credential surface APM actually owns.

5.2 What the campaign over-spent

  • ~25 of 32 rounds were spent growing/refining the shape-based
    scanner (the "env domain"). Each round added a provider shape, each
    shape "broke" the next round's adversarial test, and the whole
    scaffold was removed in one scope correction. That effort is
    discarded.
  • The tests/red_team/ corpus was 188 files / +25,148 lines and NOT
    run by CI
    -- a merge blocker. Resolved in this PR: the per-round
    sweep dumps (*_sweep/, ~134 files / ~20.8k lines of overlapping
    test_roundNN_* accretion, plus 3 files asserting the stripped
    scanner) were deleted, and the curated behavior-organized suite
    (49 test files / 279 tests, one concern per file: trust TOCTOU /
    kill-switch / canonicalization, SSRF IP-encodings, parser bombs,
    credential-NAME gaps, install containment) was wired into the CI
    shard run
    (pytest ... tests/red_team, ~6s). The corpus now reads
    54 files / +4,350 lines and every line of it executes on every PR.
  • script_executors.py (1844 lines) remains large; the
    credential-NAME regex is comprehensive to the point of fragility
    (rotation/encoding/descriptive-suffix combinators). A simpler
    denylist + exact-name set would be easier to audit.

5.3 Net verdict

Net positive. The campaign produced real, load-bearing security
infrastructure (trust gate, SSRF guard, bounded loader, containment)
and a codebase-wide loader hardening dividend. Its methodology was
sound -- each round surfaced genuine bypass vectors in the kept
layers. But its scope drifted into "build a secrets scanner" for
too long; a tighter initial threat-model would have reached the same
shipped state in roughly a third of the rounds. The maintainer's
correction was timely and restored the right boundary: APM owns its
own credential names; the script author owns their third-party
secrets.

The feature is safe to ship. The former review-experience cost (a
253-file diff carrying a 188-file CI-unrun test corpus) has been paid
down in this PR -- sweeps deleted, curated suite wired into CI -- so
the remaining diff is 120 files and every test runs. Not a merge
blocker.


6. CI / Merge State (head 95f8c11)

  • All required checks green (Lint, both Build & Test shards, Analyze x2,
    CodeQL, Spec-conformance gate, Coverage Combine, gate); 0 open
    CodeQL alerts
    . The curated tests/red_team suite (279 tests) is now
    part of the sharded shard run, so it is a required check at PR time.
  • mergeable: MERGEABLE, mergeStateStatus: BLOCKED -- the only
    remaining gate is the protected-branch human approval. This
    autopilot does NOT auto-merge.
  • Authorship preserved: every commit carries both
    Sergio Sisternes <sergio.sisternes@epam.com> and the Copilot
    co-author trailer.
  • Deferred design items tracked in PR #1798 lifecycle "hooks" collides with the agent-hooks primitive -- rename to "scripts" + integrate governance #1919 (notify/run executor split,
    module split, HTTP thread cap, interactive trust prompt, log viewer).
    The red_team CI-wiring item is now done in this PR.

…d reads

PR #1798 routed the marketplace and registry fetch paths through a
bounded streaming reader (stream=True + resp.iter_content) under a byte
ceiling. The pre-existing integration doubles predate that contract:
_fake_response (test_marketplace_ado_rest) was a MagicMock without
iter_content and its fake_get(...) signatures rejected the new stream
kwarg, and _FakeResponse (test_wave7_policy_registry_coverage) exposed
.content/.json() but not iter_content. Both files only run in the
merge_group (Integration Tests) job, so the break was invisible on PR
pushes and would have failed the merge queue.

Update the doubles to honor the streaming contract: yield the payload
as a single chunk from iter_content and accept **kwargs on the get
doubles. Test-only change; no production behavior is affected.

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel merged commit 59b0c96 into main Jun 28, 2026
23 checks passed
@danielmeppiel danielmeppiel deleted the sergio-sisternes-epam/feat-installation-analytics-hooks branch June 28, 2026 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/needs-design Direction approved, design discussion required before code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Installation analytics

4 participants