Skip to content

fix(alias): reject path-traversing dependency aliases — fixes #2900 - #2901

Open
Daniel (Danvs60) wants to merge 15 commits into
microsoft:mainfrom
Danvs60:fix/alias-poisoning
Open

fix(alias): reject path-traversing dependency aliases — fixes #2900#2901
Daniel (Danvs60) wants to merge 15 commits into
microsoft:mainfrom
Danvs60:fix/alias-poisoning

Conversation

@Danvs60

@Danvs60 Daniel (Danvs60) commented Sep 8, 2026

Copy link
Copy Markdown

fix(alias): contain install destinations without restricting source paths

TL;DR

Reject aliases that select apm_modules itself or escape it, using the existing parsing and destination owners. Preserve safe dotted aliases, local declaring-source-relative ../ chains, and same-repository remote siblings. Extend Daniel (@Danvs60)'s repair with durable lock-alias replay and current-remote cache observations so update, uninstall and reinstall retain updated hook bytes.

Fixes #2900.

Important

Current head is 5a0d872b9dd63b78b08923eddfad0ed1f9b369b4, including current main at e38261c5db4d893d6ddebc3925742e4e3bd2ba74 and the original contributor history. Local qualification and all seven exact-head hosted workflows have passed. Human ratification and review requirements remain outstanding.

req-mf-025 and the optional lock alias field still require genuine Section 9.3 ratification. Automated advice, routine workflow authorization and passing checks are not specification or merge approval.

Problem (WHY)

  • Raw alias joins in download and integration bypassed canonical materialization checks. Bare . selected the modules root; .. selected its parent. Containment equality also allowed a root-pointing alias symlink.
  • Selecting the wrong tree can read the consumer manifest or hash the wrong directory. Package name and manifest version are inventory, not canonical dependency identity, deduplication or trust keys.
  • [!] Destination restrictions must not redefine sources: local ../ references retain their declaring package, and remote siblings retain authenticated repository coordinates. Reused cache bytes and authenticated replacements have distinct admission paths.
  • Earlier hosted failures exposed lost lock-alias placement and stale named refs after hook update/uninstall/reinstall. The repair records placement and successful current-remote observations without letting an older lock downgrade them.
  • Downstream spec review caught alias constraints changing published, byte-immutable schema resources at unchanged IDs. Original URLs now retain their exact bytes; independent 0.1.41 schema IDs carry the new constraints.

The regression process follows Agent Skills' "do the work, run a validator (a script, a reference checklist, or a self-check), fix any issues, and repeat until validation passes.". The scenarios below identify the concrete tests and evidence.

Historical cause, not a new restriction on sibling sources

The initial available commit c50f223a already accepted dot aliases and joined them directly. #364 hardened the canonical path but missed independent alias branches. #379 and #1904 later exposed wrong hash/inventory consequences; #1111 and #1732 sibling support did not introduce the bypass. Slash and percent aliases were already rejected; the new reserved-name rejection is exactly . and ... Separate work in #2820 and #2827 is not imported or claimed here.

Approach (WHAT)

  • Reuse parse_alias_override for object, registry and SSH/SCP ingress.
  • Let get_install_path / build_materialization_path own alias selection and strict, symlink-aware containment; both install phases delegate.
  • Preserve canonical alias spelling in the lock and discover flat/dotted installs without changing source identity.
  • Derive remote source coordinates independently of alias destinations, without consumer-filesystem fallback.
  • Apply existing read-only legacy-cache admission after the fetch/replacement decision and before reused-byte normalization.
  • Publish named-ref observations only after successful current-remote resolution and checkout; older lock replay cannot overwrite them.
  • Document non-destructive rename/reinstall recovery, not deletion of an unsafe path or automatic historical repair.

Implementation (HOW)

d1dd63c6 cleanly merged 58190d852 with current main; 5a0d872b9 corrected schema publication identity and extended the existing manifest contract owner to accept the new exact ID. Alias/cache runtime guards remain unchanged. Current main's no-target Agent Plugin outcome has an additional installed-CLI lifecycle.

Files Intent
models/dependency/{object_fields,reference,registry_entry}.py One alias parser; preserve SSH userinfo, safe characters and source coordinates.
models/manifest_contract.py, tests/integration/test_architecture_contract_guards.py Accept original and 0.1.41 manifest IDs through one owner; unknown identities still fail closed.
models/dependency/materialization.py, install/phases/{download,integrate}.py One strict destination owner and explicit phase guards; no raw alias joins.
deps/apm_resolver.py, install/legacy_plugin_compat.py, install/phases/resolve.py Preserve remote source anchoring; inject existing legacy admission at actual reuse, after replacement choice.
deps/lockfile.py, commands/deps/_utils.py Round-trip optional alias placement and scan contained flat/dotted installed roots.
cache/git_cache.py, deps/{tiered_ref_resolver,github_downloader}.py Atomic per-ref observations, current-remote authorization, corrupt-receipt miss and receipt byte accounting.
install/helpers/ref_reuse.py Preserve existing freshness-policy and semver eligibility ownership.
.apm/architecture/owners/contracts-tooling.json, scripts/architecture_linter/checks/{contracts_test_taxonomy,transport_network_and_runtime}.py Registered guards for canonical routing, source coordinates and cache ordering.
tests/unit/test_{alias_traversal,registry_entry_alias_traversal}.py, tests/red_team/install/test_alias_path_escape.py Independent ingress/phase regressions, real symlinks, safe names and simulated Windows controls.
tests/unit/cache/test_git_cache.py, tests/unit/deps/test_{github_downloader_phase3,lockfile_consumer_contract,lockfile_field_properties,tiered_ref_resolver}.py Receipt, authorization, lock placement and field-preservation tests.
tests/unit/test_{command_helpers,deps_utils,dev_dependencies}.py, tests/test_apm_resolver.py Alias-aware inventory, explicit absent-alias mocks and remote siblings.
tests/integration/test_{architecture_dependency_reference,architecture_owner_rule_mutations}.py Clean-owner assertions and executable mutation protection.
tests/integration/test_{required_lifecycle_state_machine,transitive_chain_e2e,hook_js_sidecar_lifecycle_contract}.py Real replay, inventory/hash correctness, local chains, legacy replacement, updated hooks and old-lock isolation.
docs/src/content/docs/{reference,consumer,troubleshooting}/, packages/apm-guide/.apm/skills/apm-usage/{dependencies,package-authoring}.md, CHANGELOG.md Exact constraints, placement, cache/prune behavior and credited non-destructive migration.
docs/src/content/docs/specs/openapm-v0.1.md, docs/public/specs/{schemas,manifests}/, tests/spec_conformance/test_{alias,manifest,lockfile,resolution}_reqs.py, CONFORMANCE.{json,md} Proposed req-mf-025, optional lock field and aligned conformance:123 requirements,118 MUST and5 SHOULD. Select distinct manifest-v0.1.41 / lockfile-v0.1.41 schemas, preserving published v0.1 bytes and IDs.

Source paths are relative to src/apm_cli/ unless another root is shown.

Diagrams

Legend: install consumers share one destination authority; remote source coordinates remain independent of alias layout.

flowchart LR
    subgraph Ingress["Alias ingress"]
        O["object_fields.parse_alias_override"]
        R["DependencyReference.parse"]
        G["registry_entry parser"]
        R --> O
        G --> O
    end
    subgraph Destination["Destination authority"]
        D["download.run"]
        I["integrate dependency dispatch"]
        P["DependencyReference.get_install_path"]
        M["build_materialization_path"]
        C["ensure_path_within and strict root exclusion"]
        D --> P
        I --> P
        P --> M --> C
        M --> O
    end
    subgraph Sources["Independent source coordinates"]
        S["_remote_source_paths_for_parent"]
        E["_expand_remote_parent_local_path"]
        S --> E
        E --> V["Same host, repo and ref; virtual sibling"]
    end
    C --> A["Package tree under safe alias"]
    A --> H["Cached inventory and content hash"]
    R --> S
    classDef changed stroke-dasharray: 5 5;
    class O,P,M,C,S changed;
Loading

The retained diagram was rendered successfully with mmdc for this write-up.

Trade-offs

  • Reserved names, not a dotted-name ban: retain .safe, safe., foo..bar and my-skill.v2; reject invalid destinations.
  • Placement, not source authority: validation does not tighten local source paths or expand authenticated remote permissions. Generic containment callers that permit equality remain unchanged.
  • No historical cleanup automation: rename, install and inspect artifacts; never delete a rejected alias destination. Frozen alias drift, automatic relocation and graph-wide alias-conflict policy remain deferred.
  • Existing cache policy, explicit ordering: only reused bytes need legacy admission; authenticated replacement remains possible.
  • Stable schema pins and bounded proof: add independent resources rather than overwrite cached validators. CLI evidence uses hermetic Git sources; Windows-prefix fixtures are simulated. Hosted checks and human ratification are separate obligations.

Benefits

  1. Both install phases reject root/outside alias destinations.
  2. All four safe dotted aliases retain inventory, hashes and deployed bytes across replay.
  3. Three-level ../ chains and authenticated remote siblings retain source semantics.
  4. Updated hook bytes survive older-lock replay, uninstall and reinstall without downgrading shared observations.
  5. Old schema IDs retain their bytes; new IDs enforce alias constraints without silently reinterpreting unknown contracts.

Validation

Exact head: 5a0d872b9dd63b78b08923eddfad0ed1f9b369b4; integrated main: e38261c5db4d893d6ddebc3925742e4e3bd2ba74.

The focused, installed-CLI, touched-owner mutation and conformance pytest invocations reported, respectively:

605 passed, 9 subtests passed in 150.03s (0:02:30)
27 passed in 352.14s (0:05:52)
30 passed, 459 deselected in 60.47s (0:01:00)
251 passed, 2 skipped in 54.92s

All seven canonical lint checks plus architecture passed, including the2100-line predicates. The exact-revision owner-v2 verifier covers all four detected owners; explicit old/new manifest-ID cases also run without inventing a fifth detector result. All27 native cases ran, including the previous26 and main's no-target Agent Plugin case. The two conformance skips are existing publisher-timestamp and absolute-source-schema waivers, not alias exemptions. All123 requirement bindings align; CONFORMANCE regeneration is unchanged.

The schema correction first produced five genuine failing regressions, then passed them after the fix. Scoped TAG review closed the publication-identity finding; the coverage specialist found no in-scope gaps. npm --prefix docs run build and python scripts/check_schema_ids.py passed:1,039 relative links resolve and all six schema IDs map to byte-identical build assets. A local build is not live publication.

The isolated CPython3.12 environment came from a frozen requirements export installed with --require-hashes through the already configured trusted enterprise feed. All95 runtime/dev versions match the lock; the editable points only at this checkout and uv pip check passes. UV_NO_SYNC=1 avoided redundant access to the unavailable official CDN; no check, hash, dependency version or source boundary was waived.

All seven exact-head hosted workflows succeeded: CI, Spec, CRLF, Docs, CodeQL, NOTICE, Merge Gate. Routine workflow authorization followed fresh head/workflow/event/repository checks under standing maintainer authority. This does not approve the PR or specification.

Historical evidence retained with its original SHA

At 58190d852,169 focused cases,26 native lifecycles and seven hosted workflows passed against base f8df1b751, before main advanced. At c5b4a860, separate scopes had465 passes plus9 subtests,24 owner mutations and247 conformance passes; CI then exposed four mocks missing alias=None, fixed in581 without relaxing production checks. At d1dd63c6, local590+9 focused,27 native and247 conformance cases passed, but hosted workflows never ran. None is relabelled as5a0d evidence.

Earlier removed guards produced16 lock-projection failures,14 alias/root failures,2 stale-ref failures including a real hook-byte mismatch, and2 unauthorized-observation failures. Every guard was restored. The corrupt-receipt mutation initially survived because its bare-cache directory was absent; the corrected fixture reaches stale fallback and fails without the guard.

The earlier ae3099de had588 local passes plus9 subtests but later failed Linux shard1, lifecycle and ModeB checks. Historical TDD and real lexical, parser-delegation, root, raw-join, metadata/hash, Windows-prefix, remote-anchor and cache-order mutations remain recorded in the recovery advisory. A separate wider run's2804 passes/1 failure concerned an unchanged absolute-path assertion matching the checkout's .copilot ancestor; no unrelated test was modified. Historical results are not current-head certification.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 Invalid aliases cannot select my modules root or another directory through either phase. Secure by default tests/red_team/install/test_alias_path_escape.py::TestDownloadRejectsEscapingAlias; TestIntegrateRejectsEscapingAlias (regression-traps for #2900) unit
2 Safe dotted aliases retain inventory, hashes and deployed bytes across replay; rejection preserves state. Governed by policy; DevX tests/integration/test_required_lifecycle_state_machine.py::test_required_reinstall_is_byte_idempotent_across_durable_state e2e
3 My specialized -> ../base -> ../leaf chain works with and without aliases. Portability by manifest tests/integration/test_transitive_chain_e2e.py::test_asymmetric_layout_anchors_on_declaring_pkg e2e
4 A remote alias preserves same-repository siblings, never outside or different-repository sources. Secure by default; Portability by manifest tests/test_apm_resolver.py::TestRemoteParentLocalPathFailClosed integration
5 Invalid reused caches stay unchanged; a changed ref can replace missing metadata. Secure by default; DevX tests/integration/test_required_lifecycle_state_machine.py::test_required_invalid_receiptless_legacy_cache_fails_with_recovery; test_required_legacy_content_hash_upgrade_preserves_skills_and_converges e2e
6 Windows-prefixed spellings still reject root/outside destinations. Portability by manifest tests/unit/test_alias_traversal.py::TestInstallPhaseSymlinkEscape::test_extended_prefix_alias_destination unit
7 Lock replay preserves placement; inventory names do not invent aliases. Governed by policy tests/spec_conformance/test_alias_reqs.py::test_alias_survives_lock_replay_without_changing_source; test_absent_alias_is_not_inferred_from_inventory_name integration
8 Updated hooks survive older-lock replay, uninstall and reinstall. Multi-harness support; Governed by policy tests/integration/test_hook_js_sidecar_lifecycle_contract.py::test_required_copilot_and_vscode_js_hook_lifecycle e2e
9 A corrupt receipt cannot select stale refs, and old locks cannot publish fresh observations. Secure by default tests/unit/cache/test_git_cache.py::test_corrupt_remote_ref_receipt_does_not_revive_stale_bare_ref; tests/unit/deps/test_github_downloader_phase3.py::test_persistent_cache_only_records_authoritative_named_ref unit
10 Pruning removes the actual alias directory and owned outputs. DevX; Governed by policy tests/integration/test_required_lifecycle_state_machine.py::test_required_dependency_prune_then_uninstall_cascades_owned_state e2e
11 Main's target-excluded Agent Plugin is not a successful no-op. DevX tests/integration/test_required_lifecycle_state_machine.py::test_required_agent_plugin_target_exclusion_noop_fails_without_mutating_state e2e
12 Old schema pins retain their bytes; either supported manifest ID selects the same loader contract. Portability by manifest; Governed by policy tests/spec_conformance/test_alias_reqs.py::test_published_schema_content_remains_immutable; test_alias_schema_revision_has_distinct_published_identity; tests/integration/test_architecture_contract_guards.py::test_manifest_schema_negotiates_normative_v01_registry_shape integration

Human actions before landing

Note

Maintainer decision, relayed by the parent shepherd and received here at 2026-09-09T21:30:01.242+02:00: "Authorize the one-off #2901 comment-period exception; keep all other gates." Only this alias amendment's 14-day public comment period is omitted. This receipt timestamp is not an approval date. Two distinct qualified non-author spec approvals, normal GitHub approvals and all technical gates remain mandatory; this exception is not ratification or any other waiver.

  1. Apply or explicitly authorize Section9.3's versioned spec/openapm-v0.1 issue label; [BUG] Aliases containing ".." escape apm_modules, poisoning the lockfile identity #2900 currently has only type/bug.
  2. Obtain two distinct qualified non-author amendment approvals: one implementation reviewer and one consumer/integrator reviewer, recording roles and reviewed SHA. Daniel authored the amendment commit, so his requested review is not automatically independent; Sergio plus another qualified reviewer may be needed.
  3. Obtain required codeowner and latest-push approvals for the final head. Existing COMMENTED reviews and automated panels are not approvals; no merge or queue action is performed by this driver.

How to test

  • Run uv run --frozen --extra dev pytest -q tests/spec_conformance/test_alias_reqs.py tests/unit/cache/test_git_cache.py; expect placement and receipt cases to pass.
  • Install twice using .safe, safe., foo..bar or my-skill.v2; inspect lock alias, inventory/hash and deployed bytes.
  • Change only the alias to . or ..; expect actionable failure without artifact changes. Restore a safe alias; never delete the rejected destination.
  • Run APM_E2E_TESTS=1 APM_RUN_INTEGRATION_TESTS=1 APM_BINARY_PATH="$PWD/.venv/bin/apm" uv run --frozen --extra dev pytest -q tests/integration/test_hook_js_sidecar_lifecycle_contract.py tests/integration/test_transitive_chain_e2e.py; expect updated hooks and declaring-source anchors to survive replay.
  • Run bash scripts/lint-architecture-boundaries.sh, inspect all current hosted workflows, and complete independent Section9.3/codeowner/latest-push requirements.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

Aliases were only matched against a lax character regex, letting values
like "..", "./x", or "foo/../bar" escape apm_modules at download and
integrate time. Now validating every parsed alias through
path_security's
segment checks and guarding the resolved install path inside the
download/integrate phases. Adds unit coverage for traversal and safe
aliases.
Wrap validate_path_segments in parse_alias_override so '.'/'..' aliases
translate PathTraversalError into the existing allowed-character message
instead of leaking a low-level technical error. Strengthen traversal
tests
to assert the friendly message.
A valid alias like 'safe-name' passes parse-time validation, but
apm_modules_dir/safe-name can itself be a symlink pointing outside
apm_modules_dir. ensure_path_within is the only guard that resolves
symlinks before containment (download.py:65, integrate.py:622); this
test proves the escape raises PathTraversalError and never writes
outside the managed tree.
The parser rejects traversal aliases (parse_alias_override), and the
ensure_path_within containment guards exist at download.py:65 and
integrate.py:622 as the defense-in-depth last line. But no test drove
those
guards through the real phase entry points -- the PR "Scenario 4" claim
(install path can never escape apm_modules even if the parser is
bypassed)
was proven only at unit tier.

Add install-tier regression traps that route malicious aliases through
the
actual phase run() functions:

- tests/red_team/install/test_alias_path_escape.py
  * download.run() rejects a '..' traversal alias and a symlink that
    resolves outside apm_modules_dir via PathTraversalError, asserting
    no
    download bytes land.
  * integrate.run() rejects the same two vectors before materialization.
  * Safe-alias controls confirm no false positive.

- tests/unit/test_registry_entry_alias_traversal.py
  * Covers the secondary parse_registry_object_entry alias validation
    (registry_entry.py:86): regex layer (%2e%2e) and
    validate_path_segments
    layer ('..', 'pkg/..'), plus a safe-alias affirmative control.
@Danvs60

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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.

🟡 Changes recommended

One new registry-alias test does not actually exercise the intended validation layer, and a couple of new containment-guard call sites rely on type: ignore where a small explicit non-None guard would make the contract and types correct.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR closes a path traversal / alias poisoning hole by rejecting traversal-like dependency aliases at parse time and adding defense-in-depth containment checks when constructing alias-derived install paths during apm install.

Changes:

  • Add validate_path_segments(..., context="dependency alias") to alias parsing for object-form dependencies (including registry object entries).
  • Add ensure_path_within() containment checks at the two alias join sites in the download and integrate phases.
  • Add unit and red-team regression tests covering traversal aliases and symlink-based escapes.
File summaries
File Description
tests/unit/test_registry_entry_alias_traversal.py New unit tests targeting registry object-entry alias validation.
tests/unit/test_alias_traversal.py New unit tests for alias override parsing and ensure_path_within symlink containment.
tests/red_team/install/test_alias_path_escape.py New red-team tests driving install phases directly to ensure containment guards trip before writes.
src/apm_cli/models/dependency/registry_entry.py Add validate_path_segments check for registry entry aliases.
src/apm_cli/models/dependency/object_fields.py Add validate_path_segments check for alias overrides and map traversal rejection into ValueError.
src/apm_cli/install/phases/integrate.py Add ensure_path_within guard when alias is used to build install_path.
src/apm_cli/install/phases/download.py Add ensure_path_within guard when alias is used to build pre-download path.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/unit/test_registry_entry_alias_traversal.py Outdated
Comment thread src/apm_cli/install/phases/download.py Outdated
Comment thread src/apm_cli/install/phases/integrate.py Outdated
Comment thread src/apm_cli/models/dependency/object_fields.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@danielmeppiel

Daniel Meppiel (danielmeppiel) commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

APM Review Panel: needs_discussion

Published head: 58190d852693b04821d6a9d133d869ea3432a82f. Outer 4/4, Copilot 2/2 drained, CI recovery 2/3. GitHub approval-generated run_attempt=2 is not another recovery iteration. No counter reset or new outer panel occurred.

Validation base: f8df1b751efc30b32dc01b125616b51f777b4c81. Independently observed current main: d26ccdaf2fbcb29775abc94b7413790b3e32b600. Main advanced after qualification through #2806 and #2520. The current MERGEABLE result is not combined-base runtime proof; the seven successful runs and local evidence apply to published581 against the recorded validation base. Integration and requalification against the then-current main remain necessary before landing.

Driver disposition: implementation follow-ups and exact-head CI are complete on the recorded validation base; current-main integration and the resumed CEO's human specification/review reservation remain. Automated advisory agreement is not ratification or permission to merge.

Faithful9167 restores durable canonical lock-alias projection, flat/dotted alias scanning, original declaring-source anchors, cache admission after replacement choice but before reused-byte normalization, and current-remote observations after successful checkout. Source, host, repo and ref are independent of placement. c5 changes only a two-line corrupt-receipt fixture; final581 changes only one Mock alias default. Neither weakens production validation.

Reservations carried unchanged

  • "Scope validation and containment to alias destinations; preserve local transitive ../ sibling source paths and existing remote-source boundaries. Do not validate local_path as an alias."
  • "Preserve safe aliases such as .safe, safe., foo..bar and my-skill.v2; do not adopt the issue's blanket ban on dotted names."
  • "Require automated coverage for both destination joins, '.'/'..' rejection, correct metadata/hash targets, safe aliases and transitive ../ siblings. Strategic alignment is not test or code approval."
  • "Add a CHANGELOG entry and migration guidance to replace rejected aliases with safe names; explain the historical bypass without blaming sibling support or calling inventory fields canonical identity."

All four remain the review boundary: safe destinations without source restrictions; all four dotted spellings; automated behavioral/static/mutation evidence; credited, non-destructive migration guidance. No README or unrelated feature change.

Actual recovered panel

Persona Blocking Recommended Nit
python-architect 0 0 1
cli-logging-expert 0 1 0
devx-ux-expert 0 0 0
supply-chain-security-expert 0 0 0
oss-growth-hacker 0 0 0
auth-expert 0 0 0
doc-writer 0 1 0
test-coverage-expert 0 2 0
performance-expert 0 1 0

These are the nine genuine recovered returns, not a fresh fan-out. Scoped lock-error, prune-doc, receipt-size, old-lock negative-twin and L2-documentation folds are present; PR Scenario Evidence is updated. The architect's nit requests no code change. Historical reviewer effort was not recorded; all requested models were Astra. Only the interrupted CEO was resumed, explicitly Astra/high.

The four genuine specification reviewers scored 8/10; their authoritative round 1 synthesis is fold_and_ship. It does not satisfy Section 9.3. Frozen alias drift and automatic nonfrozen relocation remain deferred; graph-wide alias-conflict policy remains outside scope.

Exact evidence and failures retained

At the final head, 169 local fixture/owner regressions, all 26 affected installed-CLI lifecycles, and the complete current seven-part lint plus architecture pass. The c5 predecessor additionally recorded 465 passes plus 9 subtests, 24 touched-owner mutation cases and 247 conformance passes with two pre-existing documented waivers. These scopes are separate, not additive or relabelled as final-head runs. Current hosted CI is green: Deploy Docs: https://github.com/microsoft/apm/actions/runs/34347943524 (success); NOTICE Drift Check: https://github.com/microsoft/apm/actions/runs/34347943579 (success); Spec conformance: https://github.com/microsoft/apm/actions/runs/34347943825 (success); CodeQL: https://github.com/microsoft/apm/actions/runs/34347943857 (success); CRLF Invariance (apm#1952): https://github.com/microsoft/apm/actions/runs/34347943594 (success); CI: https://github.com/microsoft/apm/actions/runs/34347943485 (success); Merge Gate: https://github.com/microsoft/apm/actions/runs/34347943763 (success).

Actual removed guards produced 16 lock-projection failures, 14 alias/root failures, 2 stale-ref failures including actual hook-v2/v1 byte mismatch, and 2 unauthorized observation failures. All guards were restored. A corrupt-receipt test initially survived because its fixture lacked a bare-cache directory; the corrected fixture reaches the stale fallback and fails without the guard. CI1's four dev-dependency Mock failures are preserved, not hidden; CI2 corrects the fixture's absent alias to None.

Docs-impact classification is in_place_resolved. The old custom classifier and one replacement failed without filesystem tools; the owner then executed the actual classifier directly. Those failed specialist calls are not represented as successful reviews.

Outstanding landing requirements

Recovery candidate qualification and exact-head hosted CI are complete on the recorded validation base. Main subsequently advanced through #2806 and #2520; integration/requalification on then-current main is still required before landing. Human OpenAPM Section 9.3 amendment disposition remains outstanding: appropriately labelled issue, qualified nonauthor implementation and consumer/integrator approvals, and subsequent 14-day public period or an explicitly applicable human waiver. Last-push/code-owner review and merge-queue policy also remain human-owned. No AI ratification, borrowed #2818 waiver, merge, or queue admission is claimed.

Exact resumed CEO assessment, made before final-candidate validation

The historical remote failures in spec-conformance, Linux shard 1 and lifecycle checks remain the highest-signal readiness evidence; their causes are repaired in the restored candidate, not demonstrated green at its exact HEAD. The nine genuine Astra reviewer returns converge on preserving existing owners and repairing existing promises; their historical reasoning effort was not recorded. Local commit 9167a99 preserves the recovered bytes and modes over parent ae3099d, with main f8df1b7 ancestral. The scoped folds are present: lockfile-specific alias recovery guidance, flat-alias prune documentation, 40-byte per-ref receipt accounting in the existing scan with N/10N no-extra-scan coverage, the older-lock A consumer replay retaining shared receipt B before uninstall/reinstall B, and corrected L2 documentation. Receipt publication remains authorized by an exact current-remote observation and successful checkout through existing auth policy. This is restored implementation, not new acceptance evidence.

P4, P5 and P6 require destination safety without source restrictions, predictable placement and proof at the actual user boundary. The prior governed-by-policy lifecycle gap therefore outweighs opinion-only polish: its negative twin is now implemented, but final-candidate execution remains pending. Historical positive evidence in tests/integration/test_hook_js_sidecar_lifecycle_contract.py::test_required_copilot_and_vscode_js_hook_lifecycle includes the verbatim assertion assert _script(expected_marker).encode() == snapshot.file(_SCRIPT.as_posix()).content and the reviewer's 5-pass run. The independent 397 passes including 10 lifecycles, earlier broad 729 passes, and 72 post-fold passes are preserved historical diagnostics with differing scopes, not additive totals or validation of 9167a99. Final owner, mutation, native, full lint and hosted CI evidence remain pending; the remote is still ae3099d, with no new CI run. The PR body's 588-test result and Scenario Evidence remain tied to that older candidate.

The authoritative guardian round-1 synthesis records four genuine reviews at 8/10, no blocking findings, and fold_and_ship at 8/10; its short folds are present. Candidate req-mf-025 defines canonical trimming, strict contained placement and optional lock-alias projection separately from source identity, without expanding source permissions. It is not behavior-neutral errata or a ratified amendment. Section 9.3 still needs human handling: issue #2900 has only type/bug, the three human reviews at ae3099d are COMMENTED rather than APPROVED, and there is no evidence of two qualified non-author approvals, the subsequent 14-day public comment period, or a specific waiver. The #2818 waiver does not apply. P6 and P7 favor a credited, candid repair over declaring readiness from automated advisory agreement.

The restored code supports the bounded repair narrative, and the scoped reviewer recommendations are folded; this is not a request for another panel or redesign. Readiness remains unresolved because exact-head validation and the human spec-amendment process are incomplete. Continue only the authorized outer4/4, Copilot2/2-drained, CI recovery1 path after VP releases the host slot, and obtain the human amendment disposition. This advisory does not authorize merge, ratify the spec, or claim current CI is green.

Preserved previous ae3099d panel and evidence (historical, not current readiness)

APM Review Panel: ship_now

PR #2901 closes the scoped alias and cache-composition faults with passing exact-head local evidence; GitHub Actions still requires maintainer action before a human ship decision.

cc Daniel (@Danvs60) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

At reviewed and pushed head ae3099d, the eight active reviewers converge on no remaining substantive production finding; auth is explicitly inactive after its scope check. Both round3 counterexamples are now repaired, not waived. Remote sibling expansion obtains alias-independent source coordinates through the existing resolver/materialization authorities, retains actual-source and repository containment, and does not introduce a consumer-filesystem fallback. The independently executed tests/test_apm_resolver.py::TestRemoteParentLocalPathFailClosed::test_remote_parent_same_repo_sibling_path_expands_to_remote_virtual_dep verifies self.assertEqual(shared.dependency_ref.virtual_path, "packages/shared") and self.assertFalse(shared.dependency_ref.is_local) for the unaliased parent and all four safe aliases; outside-repository and different-clone controls remain. Legacy admission stays in its existing read-only owner and is injected after the canonical replacement decision, before normalization of reused bytes, excluding downloaded keys. In tests/integration/test_required_lifecycle_state_machine.py::test_required_invalid_receiptless_legacy_cache_fails_with_recovery, both replacement-missing-apm-yml and replacement-missing-apm-dir pass assert replaced.resolved_commit == source.commit.sha, assert replaced.content_hash == compute_package_hash(cached_package) and assert deployed.read_text(encoding="utf-8") == _skill("legacy-skill"). The seven same-ref rejection cases also pass their applicable preservation assertions, including assert_unchanged(before_cache, ArtifactSnapshot.capture(cached_package)) and _assert_same_state(before_state, after_state). These are load-bearing regression results under P4/P6, not architectural opinion. The architect's sole nit explicitly requests no change; another abstraction would not improve this bounded repair.

All four prior reservations are satisfied for the scoped repair. (1) "Scope validation and containment to alias destinations; preserve local transitive ../ sibling source paths and existing remote-source boundaries. Do not validate local_path as an alias." Closed: local original-source anchoring is unchanged, aliased/unaliased three-level local chains pass, and the repaired remote composition preserves repository coordinates and rejection boundaries. (2) "Preserve safe aliases such as .safe, safe., foo..bar and my-skill.v2; do not adopt the issue's blanket ban on dotted names." Closed: all four spellings retain lexical admission, materialization spelling, real CLI replay coverage and remote-sibling coverage; no blanket ban was substituted. (3) "Require automated coverage for both destination joins, '.'/'..' rejection, correct metadata/hash targets, safe aliases and transitive ../ siblings. Strategic alignment is not test or code approval." Closed by executed scenario evidence: independent phase guards, reserved-name ingress, actual alias-tree inventory/hash checks, preserved-state lifecycles, local chains and both newly added composition regressions are covered. I read the completed test-2901-terminal-head.log: 588 passed, 9 subtests passed in 279.68s; the complete exact-head local lint mirror also finished clean. The test reviewer independently executed 13 focused cases plus 9 subtests, all passing. Recorded pre-repair failures and restored production mutations discriminate the repaired source-coordinate route, replacement ordering and retained reuse admission; the new combined mutation produced 7 failures and removing reuse admission produced 3. These intentional failures are not current-head failures. (4) "Add a CHANGELOG entry and migration guidance to replace rejected aliases with safe names; explain the historical bypass without blaming sibling support or calling inventory fields canonical identity." Closed: the credited Security entry and non-destructive rename/unchanged-source/install/artifact-review procedure are present. The final PR body now includes Scenario Evidence, exact tests and mutations, two mmdc-validated diagrams, the pre-sibling historical bypass, the distinction between newly rejected bare dots and already-invalid slash/percent forms, and inventory-not-identity wording. I verified that the live PR body matches pr-2901-body.md. The test reviewer's sole recommended writer action is completed, not deferred; exact-head docs classification is in_place_resolved and the earlier real CDO agrees.

No further in-scope code folds or outstanding panel follow-ups remain on this evidence. This conclusion follows the repaired regressions, not the fourth-round cap, and is separate from mergeability or check policy. Evidence remains bounded: remote tests use real resolver/filesystem fixtures with a controlled download callback, not live authenticated transport; the supply-chain nondefault-host/port probe is supplementary manual evidence, not an automated regression guarantee. CLI replacement uses fixture Git sources and changes the requested ref while retaining the same resolved commit, proving replacement admission and restored artifacts rather than changed upstream content. Windows-prefix coverage is simulated, not native Windows certification. Preserve the earlier wider-run result of 2804 passed and one unrelated failure in tests/unit/install/test_install_copilot_user_instructions.py::TestCopilotUserInstructionsIntegration::test_project_scope_unaffected, whose absolute-path assertion rejects the assigned checkout's .copilot ancestor despite correct .github/instructions deployment; no unrelated fix or all-suite success is claimed. The live PR head matches Danvs60/apm:fix/alias-poisoning and the contributor commits remain retained. However, ci-2901-runs.json records six exact-head GitHub Actions runs with conclusion action_required, and ci-2901-suites.json records zero check runs for those suites; no workflow jobs executed and only CLA succeeded. The driver must therefore pause for maintainer CI action, not on unimplemented code. Local success is not GitHub CI success, and this advisory grants no approval or permission to merge.

Dissent. There is no remaining substantive specialist disagreement: the architect's nit is an explicit no-change assessment, and the test reviewer's sole recommendation has been completed and verified against the live PR body. Earlier panelist statements that exact-head verification or body writing was pending are superseded by the completed logs and posted body, not by lowering the evidence standard.

Aligned with: Portable by manifest, Secure by default, Governed by policy, Multi-harness / multi-host, OSS community-driven, Pragmatic as npm

Growth signal. Amplify the credited Security entry and single non-destructive migration procedure as a repair to APM's existing package-manager promises, not a new-feature launch or universal safety claim. The completed historical explanation protects community trust: sibling support did not introduce the bypass, only bare dot names are newly rejected, and correct inventory/hash targets do not redefine identity. No README, onboarding or broader launch work is needed.

Panel summary

Persona B R N Takeaway
CLI Logging Expert 0 0 0 Safe alias diagnostics remain actionable; cache admission now distinguishes reuse from replacement without adding output noise. No CLI-logging findings.
DevX UX Expert 0 0 0 Both composition repairs address the reproduced DevX faults; safe aliases, unchanged local sources and non-destructive errors remain covered. Exact-head verification is still pending.
Doc Writer 0 0 0 Unchanged docs match ae3099d: the bounded repairs preserve existing semantics without a new interface or inventory-as-identity claim. Final PR-body evidence remains driver-owned.
OSS Growth Hacker 0 0 0 No new growth findings. Credited release guidance and non-destructive migration remain appropriate; the bounded composition repairs need no additional onboarding or launch surface.
performance-expert 0 0 0 No actionable performance regression: reuse-only validation preserves fetch decisions; remote alias anchoring adds bounded path checks, not downloads or tree copies.
Python Architect 0 0 1 Both round3 composition faults now route through existing owners. The final focused gate passed; no remaining correctness or compounding architecture fault was found in scope.
Supply Chain Security 0 0 0 Both round3 faults are repaired at the intended boundaries; retained containment, provenance and cache-admission controls show no new scoped security concern.
Test Coverage 0 1 0 Both composition gaps have regression traps: 13 independent probes pass; exact-head gate records 588 passes plus 9 subtests. Only the owned Scenario Evidence update remains.

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

Architecture

### 1. OO / module boundaries - Before round4
```mermaid
classDiagram
    direction LR
    class DependencyReference {
      <<Dataclass>>
      +get_install_path(modules) Path
    }
    class APMPackage {
      <<Dataclass>>
      +source_path Path
      +package_path Path
    }
    class APMDependencyResolver {
      +_try_load_dependency_package()
      +_remote_repo_root_for_parent() Path
      +_expand_remote_parent_local_path()
    }
    class DownloadCallback {
      <<Protocol>>
      +__call__()
    }
    class ActivationCallback {
      <<Protocol>>
      +__call__()
    }
    class LegacyPluginCompat {
      <<IOBoundary>>
      +validate_cached_legacy_plugin()
      +upgrade_cached_legacy_plugin()
    }
    ProviderCoordinateMixin <|-- DependencyReference
    DependencyReference ..> ObjectFields : object and SSH aliases
    RegistryEntry ..> ObjectFields : alias parser
    DependencyReference ..> MaterializationPaths : destination owner
    MaterializationPaths ..> ObjectFields : defensive validation
    MaterializationPaths ..> PathSecurity : strict resolved descent
    ResolvePhase ..> MaterializationPaths : prepare destinations
    ResolvePhase ..> LegacyPluginCompat : preflight BEFORE resolver
    ResolvePhase ..> APMDependencyResolver : constructs
    APMDependencyResolver o-- DownloadCallback : injected fetch
    APMDependencyResolver o-- ActivationCallback : injected publish
    APMDependencyResolver ..> APMPackage : physical source_path
    APMDependencyResolver ..> DependencyReference : identity and destination
    APMDependencyResolver ..> PathSecurity : remote containment
    DownloadPhase ..> DependencyReference : get_install_path
    IntegratePhase ..> DependencyReference : get_install_path
    DependencySource <|-- CachedDependencySource
    DependencySource <|-- FreshDependencySource
    DependencySource <|-- LocalDependencySource
    IntegratePhase ..> DependencySource : make_dependency_source
    CachedDependencySource ..> LegacyPluginCompat : transactional upgrade
    LegacyPluginCompat ..> ResolutionStagingSession : normalize safely
    note for APMDependencyResolver "Remote repo root subtracts virtual depth from physical alias destination"
    note for ResolvePhase "Legacy preflight runs before knowing whether cached bytes will be discarded"
Loading

1. OO / module boundaries - After round4

classDiagram
    direction LR
    class DependencyReference {
      <<Dataclass>>
      +get_install_path(modules) Path
    }
    class APMPackage {
      <<Dataclass>>
      +source_path Path
      +package_path Path
    }
    class APMDependencyResolver {
      <<CallbackInjection>>
      +_try_load_dependency_package()
      +_remote_source_paths_for_parent() tuple
      +_expand_remote_parent_local_path()
    }
    class DownloadCallback {
      <<Protocol>>
      +__call__()
    }
    class ActivationCallback {
      <<Protocol>>
      +__call__()
    }
    class LegacyPluginCompat {
      <<IOBoundary>>
      +validate_cached_legacy_plugin()
      +upgrade_cached_legacy_plugin()
    }
    ProviderCoordinateMixin <|-- DependencyReference
    DependencyReference ..> ObjectFields : object and SSH aliases
    RegistryEntry ..> ObjectFields : alias parser
    DependencyReference ..> MaterializationPaths : destination owner
    MaterializationPaths ..> ObjectFields : defensive validation
    MaterializationPaths ..> PathSecurity : normalize destination AND root
    ResolvePhase ..> MaterializationPaths : prepare destinations only
    ResolvePhase ..> APMDependencyResolver : constructs and injects partial
    ResolvePhase ..> LegacyPluginCompat : binds read-only callback
    APMDependencyResolver ..> LegacyPluginCompat : invokes callback, no import
    APMDependencyResolver o-- DownloadCallback : injected fetch
    APMDependencyResolver o-- ActivationCallback : injected publish
    APMDependencyResolver ..> APMPackage : validate actual source_path
    APMDependencyResolver ..> DependencyReference : replace alias=None for coordinates
    APMDependencyResolver ..> PathSecurity : remote containment
    DownloadPhase ..> DependencyReference : get_install_path
    IntegratePhase ..> DependencyReference : get_install_path
    DependencySource <|-- CachedDependencySource
    DependencySource <|-- FreshDependencySource
    DependencySource <|-- LocalDependencySource
    IntegratePhase ..> DependencySource : make_dependency_source
    CachedDependencySource ..> LegacyPluginCompat : transactional upgrade
    LegacyPluginCompat ..> ResolutionStagingSession : normalize safely
    note for ObjectFields "Guard Clauses: one alias vocabulary; reject exactly bare dots"
    note for MaterializationPaths "Guard Clauses and Single-owner delegation: strict destination; explicit alias skips case migration"
    note for APMDependencyResolver "Dependency Injection: read-only cache callback AFTER fetch decision, BEFORE normalization; remote coordinates reuse unaliased destination owner"
    note for LegacyPluginCompat "Extract Function: one admission policy for reused resolver bytes and cached upgrade"
    class DependencyReference:::touched
    class ObjectFields:::touched
    class RegistryEntry:::touched
    class MaterializationPaths:::touched
    class ResolvePhase:::touched
    class APMDependencyResolver:::touched
    class DownloadPhase:::touched
    class IntegratePhase:::touched
    class LegacyPluginCompat:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading

```mermaid
### 2. Execution flow - Before round4
```mermaid
flowchart TD
    A["[LOCK] commands/install.py::install\nserialized_lifecycle_unless"] --> R["[I/O] [FS] install/phases/resolve.py::run\n_load_lockfile; _ensure_modules_dir\n_prepare_existing_materialization_paths"]
    R --> O["[I/O] models/dependency/materialization.py::prepare_materialization_path\nget_install_path -> build_materialization_path\nstrict alias destination; skip alias case migration"]
    O -->|invalid destination| E["[FS] commands/install.py: transaction.fail\napply_install_command_outcome; ctx.exit: 1"]
    O --> P{"[I/O] resolve.py::_prepare_existing_materialization_paths\ndestination.exists()?"}
    P -->|yes| V["[I/O] install/legacy_plugin_compat.py::validate_cached_legacy_plugin\npreflight before fetch decision"]
    V -->|invalid old metadata, even if replacement needed| E
    P -->|no| L
    V -->|valid or ineligible| L["[I/O] deps/apm_resolver.py::_try_load_dependency_package\nget_install_path"]
    L --> F{"[I/O] local OR missing path OR\n_should_force_recheck(dep_ref)?"}
    F -->|yes| D["[LOCK] [NET] [FS] _try_load_dependency_package reserves key\nresolve.py::download_callback\nprepare_replacement; download_package"]
    D -->|usable candidate| M
    D -->|failed required dependency| E
    F -->|no| M["[I/O] [FS] deps/apm_resolver.py\nmaterialize_marketplace_manifest\nvalidate_apm_package / APMPackage.from_apm_yml"]
    M -->|invalid| E
    M -->|valid| Q["[FS] apm_resolver.py::_activate_validated_package\nresolve.py::_activate_validated_candidate for staged bytes"]
    Q --> C{"apm_resolver.py BFS\nremote parent declares relative path?"}
    C -->|yes| S["[I/O] _remote_repo_root_for_parent\nphysical parent.source_path minus virtual depth"]
    S -->|flat alias makes root escape modules| E
    S -->|contained| H["[I/O] _expand_remote_parent_local_path\nensure_path_within(child, repo_root)\n_inherit_remote_parent_fields"]
    H -->|valid remote child| L
    H -->|outside repo| E
    C -->|local parent| LC["[I/O] _compute_dep_source_path\noriginal local source remains anchor"]
    LC --> L
    C -->|graph complete| I["[I/O] [NET] [FS] download.py::run; integrate.py::run\neach uses get_install_path\nCachedDependencySource.acquire -> upgrade_cached_legacy_plugin"]
    I --> K["[FS] install/template.py::run_integration_template\n[LOCK] phases/lockfile.py::LockfileBuilder.build_and_save"]
    K --> Z["commands/install.py\napply_install_command_outcome; ctx.exit: 0 on success"]

2. Execution flow - After round4

flowchart TD
    A["[LOCK] commands/install.py::install\nserialized_lifecycle_unless"] --> R["[I/O] [FS] install/phases/resolve.py::run\n_load_lockfile; _ensure_modules_dir\n_prepare_existing_materialization_paths"]
    R --> O["[I/O] models/dependency/materialization.py::prepare_materialization_path\nget_install_path -> build_materialization_path\nparse_alias_override; ensure_path_within both operands\nstrict alias destination; skip alias case migration"]
    O -->|invalid destination| E["[FS] commands/install.py: transaction.fail\napply_install_command_outcome; ctx.exit: 1"]
    O --> W["resolve.py::_resolve_dependencies\ninject partial(validate_cached_legacy_plugin,\nlockfile=existing_lockfile, fetched_this_run=False)"]
    W --> L["[I/O] deps/apm_resolver.py::_try_load_dependency_package\nget_install_path"]
    L --> F{"[I/O] local OR missing path OR\n_should_force_recheck(dep_ref)?"}
    F -->|yes| D["[LOCK] [NET] [FS] _try_load_dependency_package reserves key\nresolve.py::download_callback\nprepare_replacement; download_package"]
    D -->|usable path| U
    D -->|failed required dependency| E
    F -->|no| U{"apm_resolver.py:1204\ncallback present AND dedup key NOT in\n_downloaded_packages?"}
    U -->|yes: reused key| V["[I/O] install/legacy_plugin_compat.py::validate_cached_legacy_plugin\nreceiptless locked 0.28 marketplace eligibility\nmetadata, symlink and hash checks"]
    U -->|no: downloaded key or no callback| M
    V -->|invalid reused metadata| E
    V -->|valid or ineligible| M["[I/O] [FS] deps/apm_resolver.py\nmaterialize_marketplace_manifest\nvalidate_apm_package / APMPackage.from_apm_yml"]
    M -->|invalid| E
    M -->|valid| Q["[FS] apm_resolver.py::_activate_validated_package\nresolve.py::_activate_validated_candidate for staged bytes"]
    Q --> C{"apm_resolver.py BFS\nremote parent declares relative path?"}
    C -->|yes| S["[I/O] apm_resolver.py::_remote_source_paths_for_parent\ncontain actual parent.source_path within modules"]
    S --> PA{"parent_dep.alias?"}
    PA -->|yes| SA["[I/O] replace(parent_dep, alias=None).get_install_path\ncontain source-coordinate anchor within modules\nNO source copy or local-read fallback"]
    PA -->|no| SN["Use contained actual parent source anchor"]
    SA --> SR
    SN --> SR["[I/O] validate_path_segments(virtual_path)\nderive repository root by virtual depth\nensure_path_within(repo_root, modules)"]
    SR --> H["[I/O] _expand_remote_parent_local_path\nensure_path_within(parent_source, repo_root)\nensure_path_within(child, repo_root)"]
    H -->|outside repo| E
    H -->|contained sibling or repo root| IN["_inherit_remote_parent_fields\nretain host, port, repo, ref\nis_local=False; local_path=None"]
    IN --> L
    C -->|local parent| LC["[I/O] _compute_dep_source_path\noriginal local source remains anchor"]
    LC --> L
    C -->|graph complete| I["[I/O] [NET] [FS] download.py::run; integrate.py::run\neach uses get_install_path\nCachedDependencySource.acquire -> upgrade_cached_legacy_plugin\nshared validator; transactional normalization when eligible"]
    I --> K["[FS] install/template.py::run_integration_template\n[LOCK] phases/lockfile.py::LockfileBuilder.build_and_save"]
    K --> Z["commands/install.py\napply_install_command_outcome; ctx.exit: 0 on success"]
Loading

### Recommendation

Code-readiness advisory only for ae3099de229141401b770af29c32dc0f63510f9b: no further in-scope code folds remain. This stance is not approval, merge authorization or a claim of green GitHub CI. The driver terminal awaits maintainer CI action because six exact-head Actions runs require maintainer action and have no jobs; only CLA has succeeded. The maintainer must address that external CI state, starting with https://github.com/microsoft/apm/actions/runs/34231025610, then evaluate the actual required checks and make the human ship decision. Do not bypass check policy or use the fourth-round cap as a reason to ship.

---

<details>
<summary>Full per-persona findings</summary>

#### Python Architect

- **[nit]** Retain the existing owners and callback seam; no further abstraction is needed at `src/apm_cli/deps/apm_resolver.py:1204`
  Architecture assessment only, not a requested code change. The source-coordinate repair extends the resolver's existing remote-parent authority and obtains the unaliased package coordinate through DependencyReference.get_install_path. It does not invent another repository-layout algorithm. The compatibility check remains in legacy_plugin_compat.py and is injected into the resolver, so deps does not import install or duplicate legacy eligibility. The resolver retains the existing ref-drift and download decision before the new reuse check. This corrects my round3 assessment: unchanged individual owners were not sufficient to establish that alias destinations composed correctly with remote source anchoring or replacement fetches.

**Design patterns**
- Used in this PR: Guard Clauses -- ObjectFields and MaterializationPaths reject reserved aliases and non-descendant destinations before returning an install path.
- Used in this PR: Single-owner delegation -- alias ingress, install phases and remote coordinate recovery route through their existing lexical, destination and containment owners.
- Used in this PR: Dependency Injection -- ResolvePhase supplies the read-only compatibility callback to APMDependencyResolver using the established callback seam.
- Used in this PR: Extract Function -- LegacyPluginCompat shares one read-only admission function between resolver cache reuse and transactional cached upgrade.
- Pragmatic suggestion: none -- the current shape is the simplest correct design at this scope.

#### CLI Logging Expert

No findings.

#### DevX UX Expert

No findings.

#### Supply Chain Security

No findings.

#### OSS Growth Hacker

No findings.

#### Auth Expert -- inactive

Changes in src/apm_cli/deps/apm_resolver.py, src/apm_cli/models/dependency/{reference,object_fields,registry_entry,materialization}.py, src/apm_cli/install/phases/{resolve,download,integrate}.py and src/apm_cli/install/legacy_plugin_compat.py change alias destinations, source-coordinate anchoring and cache admission without changing authentication inputs, host classification, credential resolution, authorization or remote-host fallback semantics.

#### Doc Writer

No findings.

#### Test Coverage

- **[recommended]** Complete the already-owned Scenario Evidence update with final-head results.
  The supplied historical body in pr-2901-context.json has testing checkboxes but no Scenario Evidence table. This carries forward the acknowledged driver-owned writing action, not a new production defect. Read the scenario-evidence rubric and audited the full current diff. The new aliased remote-parent and invalid-cache/ref-change scenarios now have real regression traps; narrower round3 passing tests did not cover those combinations. The completed terminal-head summary is 588 passed, 9 subtests passed in 279.68s. The body must distinguish this focused result from the prior static-mutation failure, the unrelated broader-run path-sensitive failure, and unverified GitHub CI.
  *Suggested:* Map the two new composition regressions and retained alias, source-boundary, inventory/hash and no-mutation promises to user-worded scenarios, principles and real test identifiers. Mark the #2900 regression traps. Attribute red/green and mutation evidence separately from final-head execution. Preserve the owned historical corrections: only bare dots are newly rejected, slash/percent inputs were already invalid, sibling support did not introduce the bypass, and inventory fields are not canonical identity.
  *Proof (test passed):* `tests/integration/test_required_lifecycle_state_machine.py::test_required_invalid_receiptless_legacy_cache_fails_with_recovery[replacement-missing-apm-yml]` -- proves: Changing my requested ref can replace an invalid old cache and restore the installed package and deployed skills. [portability-by-manifest,secure-by-default,devx]
  `assert receipt.is_file() assert replaced.content_hash == compute_package_hash(cached_package) assert deployed.read_text(encoding="utf-8") == _skill("legacy-skill")`

</details>

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

</details>

Address the PR microsoft#2901 panel and Copilot follow-ups: route all alias ingress and materialization through existing owners, reject root-equal destinations, preserve local sibling sources, and defend real reinstall metadata and hashes with regression and architecture tests. Include actionable diagnostics and migration guidance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve e845ebf and its explicit non-None contract through the phase-level RuntimeError guard. The shared materialization owner replaces the parallel alias join.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Address round-two architecture/security and lifecycle findings. Compare both roots through path_security, give safe alias recovery guidance, and preserve existing legacy-plugin validation before alias-aware resolution can normalize cached files. Main passes the legacy missing-metadata cases; added preflight keeps that behavior. Windows prefix and legacy-preflight mutation controls fail with guards removed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep authenticated remote coordinates separate from flat aliases. Inject the existing read-only legacy cache admission at actual reuse after canonical fetch decisions, not preparation; retain same-ref failure and transactional replacement. Extend real resolver and CLI lifecycle contracts with aliased remote siblings and invalid-cache ref changes. Mutation controls detect both source-anchor regression and misplaced or missing admission.

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

Daniel Meppiel (danielmeppiel) commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

PR #2901: engineering complete; human ratification and review remain

Published 5a0d872b9dd63b78b08923eddfad0ed1f9b369b4 on the original Danvs60/apm:fix/alias-poisoning branch. It includes authoritative main at e38261c5db4d893d6ddebc3925742e4e3bd2ba74, not the previously cached PR base. Contributor history, safe dotted aliases, declaring-package-relative ../ sources, lock placement and cache recovery are preserved.

The downstream TAG finding about changed immutable schema URLs is closed: old manifest-v0.1.schema.json and lockfile-v0.1.schema.json retain their exact public bytes; independent 0.1.41 schemas carry the alias constraints. Both known manifest IDs route through the existing contract owner; unknown IDs still fail closed. Scoped TAG and test-coverage follow-ups found no remaining in-scope issue. This is not spec ratification or live schema publication.

All seven exact-head hosted workflows succeeded: CI, Spec conformance, CRLF, Docs, CodeQL, NOTICE, Merge Gate. These are real workflow results, not a CLA-only rollup. Routine workflow authorization used standing maintainer authority after fresh exact-run trust inspection; no human PR/spec approval was supplied.

Exact-head local evidence: 605 focused passes + 9 subtests, 27 installed-CLI lifecycles, 30 owner/manifest mutation cases, and 251 conformance passes with 2 existing documented skips. All seven canonical lint gates plus architecture passed. Strict owner-v2 evidence covers all four detected owners; both known manifest IDs have additional real-loader proof. The docs build resolves 1,039 relative links and all six schema IDs to byte-identical assets. The schema correction failed five genuine pre-fix regressions before passing with the fix.

The earlier frozen-dependency block is resolved through the existing trusted enterprise feed using the frozen export and required hashes: all 95 versions match the lock and the editable package belongs only to this checkout. No dependency substitution, source borrowing, cache surgery or proxy workaround occurred.

Human actions before landing

Note

Maintainer decision, relayed by the parent shepherd and received here at 2026-09-09T21:30:01.242+02:00: "Authorize the one-off #2901 comment-period exception; keep all other gates." Only this alias amendment's 14-day public comment period is omitted. This receipt timestamp is not an approval date. Two distinct qualified non-author spec approvals, normal GitHub approvals and all technical gates remain mandatory; this exception is not ratification or any other waiver.

  1. Apply or explicitly authorize the versioned spec/openapm-v0.1 issue label required by Section 9.3. Issue [BUG] Aliases containing ".." escape apm_modules, poisoning the lockfile identity #2900 currently has only type/bug; the generic spec label is not silently treated as equivalent.
  2. Obtain two distinct qualified non-author amendment approvals, one from an implementation reviewer and one from a consumer/integrator reviewer. Record roles, final reviewed SHA and approval timestamps. Daniel authored the amendment commit; requested-reviewer status does not make him independent. Sergio plus another qualified non-author reviewer may be needed.
  3. Obtain the required codeowner and latest-push approvals for this final head. These are separate from spec ratification. The current human review snapshot has three old COMMENTED reviews and zero APPROVED reviews.

Terminal state: MERGEABLE / BLOCKED, CI green, human-gated. No engineering repair remains in this scoped handoff. Frozen alias drift, automatic alias relocation and graph-wide alias-conflict policy retain their documented scope deferrals. No merge, queue entry, release, review dismissal or approval impersonation occurred.

Historical outer4 / Copilot2 / CI-recovery2 are retained; later maintainer instructions lifted administrative holds. This continuation did not restart the completed general-panel roster or Copilot fetch. The schema correction was a scoped actual-review fold, not another CI-repair iteration. Downstream #2820 can use this exact prospective source pin, but must qualify its own head and cannot claim human-landed ancestry.

Preserved previous advisory: historical 581 and ae3099 states, before the one-off period exception; not current qualification or gates ## PR #2901 recovery advisory: human approval required

Published head: 58190d852693b04821d6a9d133d869ea3432a82f. Outer 4/4, Copilot 2/2 drained, CI recovery 2/3. GitHub approval-generated run_attempt=2 is not another recovery iteration. No counter reset or new outer panel occurred.

Validation base: f8df1b751efc30b32dc01b125616b51f777b4c81. Independently observed current main: d26ccdaf2fbcb29775abc94b7413790b3e32b600. Main advanced after qualification through #2806 and #2520. The current MERGEABLE result is not combined-base runtime proof; the seven successful runs and local evidence apply to published581 against the recorded validation base. Integration and requalification against the then-current main remain necessary before landing.

Faithful9167 restores durable canonical lock-alias projection, flat/dotted alias scanning, original declaring-source anchors, cache admission after replacement choice but before reused-byte normalization, and current-remote observations after successful checkout. Source, host, repo and ref are independent of placement. c5 changes only a two-line corrupt-receipt fixture; final581 changes only one Mock alias default. Neither weakens production validation.

At the final head, 169 local fixture/owner regressions, all 26 affected installed-CLI lifecycles, and the complete current seven-part lint plus architecture pass. The c5 predecessor additionally recorded 465 passes plus 9 subtests, 24 touched-owner mutation cases and 247 conformance passes with two pre-existing documented waivers. These scopes are separate, not additive or relabelled as final-head runs. Current hosted CI is green: Deploy Docs: https://github.com/microsoft/apm/actions/runs/34347943524 (success); NOTICE Drift Check: https://github.com/microsoft/apm/actions/runs/34347943579 (success); Spec conformance: https://github.com/microsoft/apm/actions/runs/34347943825 (success); CodeQL: https://github.com/microsoft/apm/actions/runs/34347943857 (success); CRLF Invariance (apm#1952): https://github.com/microsoft/apm/actions/runs/34347943594 (success); CI: https://github.com/microsoft/apm/actions/runs/34347943485 (success); Merge Gate: https://github.com/microsoft/apm/actions/runs/34347943763 (success).

Actual removed guards produced 16 lock-projection failures, 14 alias/root failures, 2 stale-ref failures including actual hook-v2/v1 byte mismatch, and 2 unauthorized observation failures. All guards were restored. A corrupt-receipt test initially survived because its fixture lacked a bare-cache directory; the corrected fixture reaches the stale fallback and fails without the guard. CI1's four dev-dependency Mock failures are preserved, not hidden; CI2 corrects the fixture's absent alias to None.

Final technical state: MERGEABLE / BLOCKED; all seven exact-head workflows SUCCESS, not CLA-only or an incomplete rollup. Original contributor commits and all unpublished recovery bytes/history are retained. No merge, queue entry, tag, release or branch deletion was performed.

Outstanding landing requirements: Recovery candidate qualification and exact-head hosted CI are complete on the recorded validation base. Main subsequently advanced through #2806 and #2520; integration/requalification on then-current main is still required before landing. Human OpenAPM Section 9.3 amendment disposition remains outstanding: appropriately labelled issue, qualified nonauthor implementation and consumer/integrator approvals, and subsequent 14-day public period or an explicitly applicable human waiver. Last-push/code-owner review and merge-queue policy also remain human-owned. No AI ratification, borrowed #2818 waiver, merge, or queue admission is claimed.

The original four strategic reservations remain unchanged in the existing panel comment. The actual guardian/CEO assessments and all historical failures are preserved. No additional Copilot fetch or outer iteration was started. Exact interface, requirement-ID and schema-ID handoffs were supplied to dependent owners; their own integration proof is not inherited.

Preserved previous terminal advisory (historical ae3099d state)

Shepherd driver: code complete; CI requires maintainer action

Pushed ae3099de229141401b770af29c32dc0f63510f9b to Danvs60/apm:fix/alias-poisoning, preserving the contributor's commits, including the concurrent follow-up. No merge was performed.

External stop: all six exact-head Actions runs report action_required with no workflow jobs. Only CLA has succeeded; GitHub reports MERGEABLE / BLOCKED. Start with the CI run. A maintainer must address the workflow approval/action requirement, then evaluate the actual required checks. I did not approve workflows or bypass policy.

The fourth real expert panel and CEO recommend ship_now for code readiness only, with no remaining in-scope follow-ups. The persistent panel recommendation was updated, not duplicated. All four Copilot findings are resolved: three received in-thread replies; the contributor had already resolved the fourth.

What was folded

  • Shared object/registry/SSH alias validation and one strict, symlink-aware destination authority used by both install phases.
  • Normalized Windows root comparison, safe dotted names, actionable non-destructive errors and correct alias-tree inventory/hash targets.
  • Independent remote source-coordinate anchoring, retaining same repository/ref boundaries despite flat aliases.
  • Existing legacy admission injected at actual cache reuse, after the fetch decision and before normalization; ref-change replacement remains available.
  • Credited CHANGELOG, migration, manifest/consumer and bundled guide updates; accurate historical PR body, Scenario Evidence and two validated diagrams. Synchronized docs classification: in_place_resolved.

Binding reservations and disposition

  1. "Scope validation and containment to alias destinations; preserve local transitive ../ sibling source paths and existing remote-source boundaries. Do not validate local_path as an alias." Satisfied: real three-level local chains pass aliased and unaliased; remote alias sibling tests retain origin/ref and outside/different-repository rejection.
  2. "Preserve safe aliases such as .safe, safe., foo..bar and my-skill.v2; do not adopt the issue's blanket ban on dotted names." Satisfied: all four retain spelling and pass parser, real replay and remote sibling scenarios.
  3. "Require automated coverage for both destination joins, '.'/'..' rejection, correct metadata/hash targets, safe aliases and transitive ../ siblings. Strategic alignment is not test or code approval." Satisfied by executed functional tests and real mutations, including the two composition cases discovered during review.
  4. "Add a CHANGELOG entry and migration guidance to replace rejected aliases with safe names; explain the historical bypass without blaming sibling support or calling inventory fields canonical identity." Satisfied. The bypass existed in the initial available 2025-09-18 commit; fix: harden dependency path validation #364 hardened the canonical path but missed separate aliases. feat: Plugin coexistence — apm pack --format plugin, apm init --plugin, devDependencies #379 and feat(lockfile): record installed package name + version per entry (#1888) #1904 later exposed hash/inventory consequences. fix(install): anchor transitive local_path deps on declaring package (#857) #1111/fix(deps): allow same-repo remote path deps #1732 sibling support did not introduce it. Only bare dots are newly rejected; slash/percent aliases were already invalid.

Verification and limits

  • Exact pushed head: 588 tests passed, plus 9 subtests. The full canonical local lint mirror passed, including duplication, architecture/auth boundaries and actual YAML/relative-path/2100-line predicates.
  • Real mutations failed as expected and were restored: lexical (9 failures), parser delegation (8), strict root (3), raw phase joins (11), alias selection (3), wrong hash (1), wrong metadata (1), Windows normalization plus missing admission (4), remote anchoring plus misplaced admission (7), missing actual reuse admission (3). Functional and clean-architecture assertions were both exercised.
  • The exact-head detector identifies the existing dependency identity/materialization/source-coordinate authority; functional, registered static and architecture mutation evidence is recorded.
  • A wider prior run had 2804 passes and one unchanged test's absolute-path assertion tripping over this checkout's .copilot ancestor. No unrelated test was changed. Remote resolver fixtures do not certify live transport; Windows-prefix tests are simulated. No GitHub CI-green claim.

Terminal classification: blocked solely on maintainer CI action, not on unimplemented code. Four outer iterations, two Copilot fetch/classification rounds, zero CI repair commits. No fifth review loop, auto-approval or auto-merge.

Continue the existing PR microsoft#2901 CI recovery without resetting its run. Preserve the recovered implementation and review folds: durable alias projection, contained alias scanning, specification conformance, and current-remote ref observations after successful checkout. Original contributor and follow-up commits remain in the lineage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The mutation gate showed the previous fixture passed even when receipt handling was removed because no bare-cache directory existed. Materialize the canonical shard directory so the stale-ref fallback is reachable and the regression fails without the receipt guard. Production code is unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Continue CI recovery2 after Linux shard2 exposed four generic Mock dependency references whose undeclared alias attribute became another Mock. Model the real unaliased DependencyReference default explicitly instead of weakening production alias validation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve Danvs60's alias traversal repair and the recovered alias/cache contracts while integrating upstream main.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep existing public v0.1 schemas byte-identical and select independent v0.1.41 schemas for alias validation. Accept the new exact manifest ID through the existing contract owner while retaining old-ID compatibility and fail-closed unknowns.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Sep 9, 2026
Preserve the original #2820 history while moving executable ownership to the published #2919 and #2923 successors. Restore current main runtime and the active v0.1 assessment verbatim; retain the exact corrective draft overlay and its release-note obligation. Final normative repin still follows qualified-human #2901 landing.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Sep 9, 2026
Integrates exact #2901 d1dd63c as a prospective prerequisite only. Human amendment review and landing remain outstanding. Original local/audit and aggregate implementation work stays in #2919 and #2923.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Sep 9, 2026
Integrate exact prospective #2901 5a0d872. Human amendment approval and landing remain outstanding.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Aliases containing ".." escape apm_modules, poisoning the lockfile identity

3 participants