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

[Daniel (Danvs60)]

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

TL;DR

Reject aliases that select apm_modules itself or escape it, and route alias parsing and destination selection through their existing owners. Preserve .safe, safe., foo..bar, my-skill.v2, local transitive ../ chains, and same-repository remote siblings. Extend Daniel (@Danvs60)'s fix with real install/reinstall, inventory/hash, source-composition and legacy-cache regression coverage.

Fixes #2900.

Problem (WHY)

• Alias-specific joins in download and integration bypassed canonical materialization checks. Bare . selected the modules root; .. selected its parent. Generic containment alone permits equality, so a root-pointing alias symlink also needs explicit exclusion.
• Selecting the wrong tree can read the consumer's manifest or compute a hash for the wrong directory. name and manifest version are inventory fields, not canonical dependency identity, deduplication or trust keys.
• [!] Tightening destinations must not redefine sources. Local ../ references resolve from their declaring package; remote siblings retain their authenticated repository coordinates. Cache validation must likewise distinguish reused bytes from bytes a requested ref change will replace.

These are concrete production-path failures and compatibility constraints, not a ban on dotted names. 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.". Fragile ordering is deliberately explicit: "when operations are fragile, consistency matters, or a specific sequence must be followed".

Why this could appear safe before: historical trace

• The initial available commit, c50f223a (2025-09-18), already accepted dot aliases and joined aliases directly.
• fix: harden dependency path validation #364 (2026-03-18) hardened canonical get_install_path, but the independent alias branches bypassed it. This was an old bypass, not a new effect of sibling support.
• feat: Plugin coexistence — apm pack --format plugin, apm init --plugin, devDependencies #379 (2026-03-20) added cached hash inputs; feat(lockfile): record installed package name + version per entry (#1888) #1904 (2026-06-25) exposed incorrect name/version inventory when the wrong manifest was read.
• The declaring-source-relative local fix fix(install): anchor transitive local_path deps on declaring package (#857) #1111 (2026-05-02) and authenticated same-repository sibling expansion fix(deps): allow same-repo remote path deps #1732 (2026-06-11) did not introduce the alias bypass.
• Slash and percent aliases were already rejected by the existing character regex. This change newly rejects the exact reserved names . and .., not otherwise-safe dotted names.
• The separate open local user-scope fix fix(lifecycle): repair local sources and audit intent with corrective OpenAPM v0.2 #2820 is not imported or claimed here; the published-source-path fix fix(install): record published package paths, never resolution-staging paths #2827 is also separate.

Approach (WHAT)

• Reuse parse_alias_override for object, registry and SSH/SCP alias ingress.
• Make get_install_path / build_materialization_path own alias selection, lexical validation and strict resolved containment; both install consumers delegate.
• Preserve alias spelling and skip repository case migration for alias directories.
• Derive remote repository source coordinates independently of flat alias destinations, without allowing consumer-filesystem fallback.
• Reuse the existing read-only legacy-cache admission policy after the canonical fetch decision and before normalizing reused bytes.
• Document a safe rename/reinstall procedure, without suggesting deletion of an unsafe alias path or promising automatic historical repair.

Implementation (HOW)

Review basis: ae3099de2, retaining the contributor's original commits and follow-up.

Files	Intent
src/apm_cli/models/dependency/object_fields.py	Shared exact reserved-name check and actionable diagnostics; retain the existing character pattern.
src/apm_cli/models/dependency/reference.py	Validate the already-extracted SSH/SCP alias without rewriting userinfo or source coordinates.
src/apm_cli/models/dependency/registry_entry.py	Delegate alias validation instead of keeping a second validator.
src/apm_cli/models/dependency/materialization.py	Own alias destinations, symlink-aware containment and normalized root exclusion; preserve spelling.
src/apm_cli/install/phases/download.py, integrate.py	Remove raw alias joins; retain explicit modules-path checks.
src/apm_cli/deps/apm_resolver.py	Separate remote source coordinates from alias layout; invoke injected admission only on actual cache reuse.
src/apm_cli/install/legacy_plugin_compat.py, phases/resolve.py	Share existing read-only legacy checks with transactional upgrade and inject them at the resolver boundary.
.apm/architecture/owners/contracts-tooling.json, scripts/architecture_linter/checks/contracts_test_taxonomy.py	Extend the existing registered authority and guard ingress, destination routing, source anchoring and fetch/admission/normalization order.
tests/unit/test_alias_traversal.py, test_registry_entry_alias_traversal.py, tests/red_team/install/test_alias_path_escape.py	Parser and independent phase regressions, actual symlinks, safe names and simulated Windows-prefix controls.
tests/integration/test_architecture_dependency_reference.py	Clean ownership check and source-override bypass mutations.
tests/integration/test_required_lifecycle_state_machine.py	Real alias replay/rejection, exact inventory/hash targets, no-mutation invalid caches and changed-ref replacement.
tests/integration/test_transitive_chain_e2e.py, tests/test_apm_resolver.py	Three-level local sibling chains plus aliased remote siblings and outside/different-repository negative controls.
docs/src/content/docs/reference/manifest-schema.md, troubleshooting/migration.md, consumer/manage-dependencies.md	Exact constraints, one non-destructive migration procedure and its consumer link.
packages/apm-guide/.apm/skills/apm-usage/dependencies.md, CHANGELOG.md	Self-contained bundled guidance and a credited Security entry.

Diagrams

Legend: install consumers share one destination authority; remote source coordinates are 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

Legend: only reused cache bytes enter legacy admission; required replacements retain the existing fetch and activation path.

sequenceDiagram
    participant R as APMDependencyResolver
    participant D as Canonical ref drift
    participant F as Download callback
    participant L as legacy_plugin_compat
    participant V as Package validation
    participant A as Activation callback
    R->>D: _should_force_recheck(dep_ref)
    alt Required fetch
        R->>F: Fetch candidate using existing source authorization
        F-->>R: Candidate path
    else Reuse existing cache
        rect rgb(255, 247, 200)
            R->>L: cache_validation_callback
            L-->>R: Read-only admission or error
        end
    end
    R->>V: Validate before integration or activation
    Note over R,V: Legacy normalization follows cache admission
    alt Validated downloaded candidate
        R->>A: Publish candidate
        A-->>R: Live materialization path
    end

Loading

Both diagrams rendered successfully with mmdc 11.15.0.

Trade-offs

• Exact reserved names, not a dotted-name ban. Keep all existing safe characters and source-path semantics; reject only invalid destination choices.
• Strict alias roots, unchanged generic containment. Normalize both comparison operands, including Windows extended prefixes, without changing callers that legitimately allow containment equality.
• No historical cleanup automation. Rename the alias, preserve its source, run normal install and inspect artifacts. Do not remove or prune the path selected by a rejected alias.
• Preserve existing compatibility policy. Validate only reused legacy bytes before normalization; do not impose reuse eligibility on an authenticated replacement candidate.
• Bounded evidence. Remote sibling tests use real resolver/file fixtures with a controlled download callback; CLI lifecycles use local Git sources. Windows prefix coverage is simulated, not native Windows certification. An unrelated absolute-path assertion in an unchanged wider-suite test remains out of scope.

Benefits

1. Both install phases independently refuse invalid aliases and root/outside symlink destinations.
2. All four safe dotted aliases retain correct dependency inventory and exact package-tree hashes across real replay.
3. Local three-level ../ chains and aliased remote siblings retain their distinct source semantics.
4. Invalid reused legacy caches remain unchanged; requested ref changes can replace missing-metadata caches and restore real deployed skills.

Validation

At exact head ae3099de229141401b770af29c32dc0f63510f9b, the focused pytest invocation below completed:

588 passed, 9 subtests passed in 279.68s (0:04:39)

The complete local CI lint mirror passed: Ruff check and format across source/tests/architecture tooling, pylint R0801, auth boundaries, architecture boundaries, and the actual YAML I/O, portable-relative-path and 2100-line predicates. On macOS the grep predicates were evaluated with equivalent Python regexes because BSD grep has no -P. This is local validation, not a claim that GitHub CI is green; after push the remote rollup exposed only the passing CLA check.

Executed focused test command and red/mutation evidence

Run from the repository with a source-installed apm binary:

APM_BINARY_PATH="$PWD/.venv/bin/apm" APM_E2E_TESTS=1 \
TMPDIR="$PWD/.work-2901" UV_FROZEN=1 uv run --extra dev pytest \
  tests/unit/models tests/unit/test_alias_traversal.py \
  tests/unit/test_registry_entry_alias_traversal.py \
  tests/red_team/install/test_alias_path_escape.py tests/unit/test_local_deps.py \
  tests/unit/install/test_legacy_plugin_compat.py tests/unit/deps/test_apm_resolver*.py \
  tests/test_apm_resolver.py tests/integration/test_deps_resolver_resolution.py \
  tests/integration/test_architecture_dependency_reference.py \
  tests/integration/test_transitive_chain_e2e.py::test_asymmetric_layout_anchors_on_declaring_pkg \
  tests/integration/test_required_lifecycle_state_machine.py::test_required_reinstall_is_byte_idempotent_across_durable_state \
  tests/integration/test_required_lifecycle_state_machine.py::test_required_invalid_receiptless_legacy_cache_fails_with_recovery \
  tests/integration/test_required_lifecycle_state_machine.py::test_required_legacy_content_hash_upgrade_preserves_skills_and_converges \
  tests/integration/test_mixed_case_materialization_lifecycle.py \
  tests/integration/test_legacy_plugin_skill_declaration_lifecycle.py \
  -q --basetemp=.pytest-2901-terminal

• Initial ingress/phase TDD: 11 failed / 29 passed before the extended repair.
• Remote-alias/replacement TDD: 6 failed / 3 passed / 5 passing subtests before correction; repaired focused families: 36 passed / 9 passing subtests.
• Real production mutations were executed and restored: lexical guard removal (9 failures), SSH/registry delegation removal (8), strict-root removal (3), raw phase joins (11), alias-selection removal (3), wrong cached hash target (1), wrong cached metadata target (1), raw Windows-root comparison plus missing legacy admission (4).
• Additional current-boundary mutations: remove remote alias source coordinates and move admission before fetch (7 failures); remove actual reused-cache admission (3 failures). These include functional and clean-architecture failures, not merely a replay of validation logic.
• The final exact-head run above includes all restored guards and source-override architecture tests. A preceding static mutation used a renamed keyword that still matched its substring check; the guard was tightened and the complete focused command rerun successfully.
• A separate wider run recorded 2804 passed / 1 failed. The unchanged test tests/unit/install/test_install_copilot_user_instructions.py::TestCopilotUserInstructionsIntegration::test_project_scope_unaffected rejects .copilot anywhere in an absolute deployed path, including this assigned checkout's .copilot/session-state ancestor. The correct .github/instructions output existed. No unrelated test was edited.

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 install phase.	Secure by default	tests/red_team/install/test_alias_path_escape.py::TestDownloadRejectsEscapingAlias; TestIntegrateRejectsEscapingAlias (regression-traps for #2900)	unit
2	My safe dotted alias retains the correct package inventory, exact hash and deployed bytes across replay; rejected aliases leave existing state alone.	Secure by default; Governed by policy; DevX	tests/integration/test_required_lifecycle_state_machine.py::test_required_reinstall_is_byte_idempotent_across_durable_state (regression-trap for #2900)	e2e
3	My local specialized -> ../base -> ../leaf chain works with aliases and without them.	Portability by manifest; DevX	tests/integration/test_transitive_chain_e2e.py::test_asymmetric_layout_anchors_on_declaring_pkg	e2e
4	A safe remote alias preserves same-repository siblings, never an outside or different-repository source.	Portability by manifest; Secure by default	tests/test_apm_resolver.py::TestRemoteParentLocalPathFailClosed::test_remote_parent_same_repo_sibling_path_expands_to_remote_virtual_dep and its two remote-path rejection controls	integration
5	An invalid reused cache stays unchanged, but changing my requested ref can replace missing metadata and restore both targets' skills.	Secure by default; DevX; Multi-harness support	tests/integration/test_required_lifecycle_state_machine.py::test_required_invalid_receiptless_legacy_cache_fails_with_recovery (nine cases); test_required_legacy_content_hash_upgrade_preserves_skills_and_converges	e2e
6	Equivalent Windows-prefixed spellings still exclude the modules root and outside paths while allowing a safe child.	Secure by default; Portability by manifest	tests/unit/test_alias_traversal.py::TestInstallPhaseSymlinkEscape::test_extended_prefix_alias_destination	unit

How to test

1. Use a source-installed development environment, create the local scratch directory with mkdir -p .work-2901, then run the focused command above; expect the alias, source and legacy-cache scenarios to pass.
2. Set an existing dependency's alias to .safe, safe., foo..bar or my-skill.v2; install twice and verify inventory/hash refer to that installed package, not the consumer.
3. Set only the alias to . or ..; expect an actionable failure without changing existing artifacts. Restore a safe alias; do not delete a path selected by the invalid alias.
4. Run the local and remote sibling regression selectors; expect valid siblings to remain accepted and remote escapes to remain refused.
5. Run bash scripts/lint-architecture-boundaries.sh; expect the single-owner and cache-ordering guards to pass. Check the PR's actual required CI separately before merging.

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

[Daniel (Danvs60)]

@microsoft-github-policy-service agree

[Copilot]

🟡 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.

[Daniel Meppiel (danielmeppiel)]

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>

[Daniel Meppiel (danielmeppiel)]

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.