Multi-host dependency support: lockfile identity, token resolution consistency, error clarity

[danielmeppiel]

Summary

PR #587 (generic git host / Gitea support) surfaces several pre-existing architectural and DX gaps in the dependency identity, token resolution, and error-reporting paths. None of these block #587 directly, but they should land before multi-host support is GA so we don't ship known sharp edges to users adopting Gitea / Bitbucket / self-hosted GitLab.

Items

1. Lockfile identity collision: get_unique_key() omits host

DependencyReference.get_unique_key() and LockedDependency.get_unique_key() both return the bare repo_url (e.g. team/skills) without the host. Two dependencies with the same owner/repo on different hosts collide silently in apm.lock:

- name: skills
  repo_url: gitea.myorg.com/team/skills
- name: skills
  repo_url: github.com/team/skills

→ second entry overwrites the first; install order becomes load-bearing.

Severity: correctness / data-integrity. Becomes much more likely once #587 lands.

Files: src/apm_cli/models/dependency/reference.py (lines ~176-190), src/apm_cli/deps/lockfile.py (lines ~43-49, ~196).

Suggested fix: include host in get_unique_key() for non-default hosts (preserve bare owner/repo for github.com to keep existing lockfiles stable).

2. Asymmetric token resolution between download and clone paths

_download_github_file calls auth_resolver.resolve() directly (line ~1207 of github_downloader.py), while the clone path correctly routes through _resolve_dep_token() which returns None for generic hosts. The asymmetry is not a security issue (HTTPS is the transport boundary per core/auth.py:377-379), but it produces misleading 401 errors when a GitHub PAT is sent to a Gitea / GitLab server that rejects it — instead of falling through to git-credential-helper as the clone path does.

Severity: DX / error-message clarity.

Suggested fix: route _download_github_file token resolution through _resolve_dep_token() for consistency.

3. is_gitlab_hostname() is convention-based and fragile

startswith("gitlab.") misses org-managed GitLab instances on bespoke hostnames (code.acme.com, git.team.org, etc.). The whole GitLab-vs-other-generic-host bifurcation in #587 falls back to a heuristic that doesn't hold.

Severity: correctness on non-conformant hostnames.

Suggested fix: consider explicit user signaling (type: gitlab in apm.yml) or capability-detection via API probe + cache, rather than relying on hostname conventions.

4. Silent error swallowing in download fallback chain

_download_github_file runs up to 7 attempts (raw + API × ref × API version), and currently swallows non-404 errors at each step — auth failures, 5xx, network errors all surface to the user as "file not found." There's also no verbose_callback breadcrumb per attempt, so --verbose doesn't help users understand which endpoint was tried.

Severity: DX / debuggability.

Suggested fix: classify exceptions, only swallow 404, surface 401/403/5xx with the host + endpoint context. Emit one verbose_callback line per attempt.

5. Pre-existing GitHub-specific error strings reachable for generic hosts

Strings like "GitHub API rate limit exceeded" are now reachable from Gitea / GitLab error paths via the shared download code. Should be parameterized on host.

Severity: polish.

Non-goals

Not changing the deliberate project stance that "HTTPS is the transport security boundary" (auth.py:377-379) — global PATs may be sent to user-configured hosts. This issue is about correctness, identity, and error-clarity, not transport gating.

Why now

PR #587 quadruples the surface area of the download fallback chain and introduces multi-host scenarios the lockfile identity model wasn't designed for. Tracking these explicitly so they don't get forgotten when #587 lands.

[tanbro]

Reproducible scenario: private GitLab project depending on a GitHub package

I have a project hosted on a private GitLab instance, and apm.yml declares a dependency on a GitHub repository (owner/repo shorthand).

When running apm install, APM incorrectly triggers a credential dialog for my private GitLab instance instead of using the GitHub credentials already stored in Windows Credential Manager (git:https://github.com). The GitLab PAT is then sent to GitHub, which of course fails.

• --ssh flag does not help — still triggers the GitLab auth dialog.
• GIT_ASKPASS env var is ignored by APM's own auth flow.
• Credential Manager has separate entries for both hosts (git:https://github.com and git:https://gitlab.example.com), correctly isolated.
• git clone works fine for both hosts independently.

This aligns with item #2 in this issue (asymmetric token resolution). The AuthResolver appears to pick up the wrong host's credentials when resolving for a GitHub package in a GitLab-originated project.

[github-actions]

Triage decision

accept

Proposed labels

theme/security
area/lockfile
area/cli
type/architecture
status/accepted
priority/high

Milestone

0.9.4

Suggested next action

Address the lockfile identity collision (item 1) first -- it has a migration implication -- then resolve token resolution and error clarity items in the same PR or follow-up, ideally before multi-host support is marked GA.

Suggested issue comment

Thanks for the thorough write-up. These four items are well-scoped and the priority ordering is right: the lockfile identity collision (item 1) is the most critical because it is a silent correctness failure -- two different hosts with the same owner/repo path could silently resolve to the same lockfile key.

A few thoughts before implementation:

- **Item 1 (lockfile key):** Adding the host to `get_unique_key()` is straightforward, but it is a lockfile schema change. Any repo with an existing `apm.lock.yaml` that includes non-GitHub hosts will see key drift after the upgrade. A one-time migration step (or at minimum a clear upgrade note in the CHANGELOG) should accompany the PR.
- **Items 3 and 4 (error clarity + validation):** These are self-contained and can land independently of item 1 if that helps isolate the migration risk.
- **Item 2 (token resolution):** The ADO/Entra ID auth chain already has precedence logic in `token_manager.py`; make sure the non-GitHub host chain is explicit about fallback order.

If you want to split this into two PRs (item 1 with migration + items 2-4), that is fine -- just cross-reference them here.

Per-lens notes (collapsed)

DevX UX Expert -- User-Need Reviewer

Four concrete pre-GA quality gaps in the multi-host dependency surface (Gitea, Bitbucket, self-hosted GitLab). Each item maps to a user-visible failure: silent lockfile key collision, confusing auth behavior on non-GitHub hosts, unclear error messages, and loose URL validation. All four are aligned with APM's dependency management mission and the cargo/npm mental model that dependency identity must be deterministic regardless of host.

Supply Chain Security Expert -- Risk-Surface Reviewer

Item 1 is supply-chain-adjacent: get_unique_key() omitting the host means two repos on different hosts with identical owner/repo paths map to the same lockfile entry. An attacker who controls a self-hosted GitLab instance could potentially shadow a legitimate GitHub dependency if the key collision is exploitable. This warrants theme/security and area/lockfile. Items 2-4 are correctness and DX issues without direct supply-chain risk.

OSS Growth Hacker -- Contributor-Tone Reviewer

Not activated -- danielmeppiel is a core maintainer with extensive project ownership; warmth tuning for a new contributor is not needed.

Python Architect -- Architecture Reviewer

The compound issue is well-understood. Item 1 (lockfile key schema change) is the highest-risk: get_unique_key() is called across the codebase wherever dependencies are resolved, and changing the key format is a migration boundary. Items 2-4 are localized. status/accepted is appropriate -- the design is implicit in the issue text and the maintainer is the author. Recommend a clear migration strategy for existing lockfiles in the PR description.

Doc Writer -- Documentation Reviewer

Not activated -- the implementing PR will touch auth and error messages but no new user-facing guide pages are required; inline code comments and CHANGELOG entries are sufficient.

APM CEO -- Triage Arbiter

Pre-GA quality work from the core maintainer with a real supply-chain dimension (lockfile identity collision). Accept at priority/high for 0.9.4: the lockfile key collision must be resolved before multi-host support is recommended to users. The migration concern is real but manageable; the PR description must document it explicitly.

{
  "decision": "accept",
  "decision_detail": "",
  "theme": "theme/security",
  "areas": ["area/lockfile", "area/cli"],
  "type": "type/architecture",
  "status": "status/accepted",
  "priority": "priority/high",
  "preserved_labels": [],
  "milestone": "0.9.4",
  "next_action": "Address the lockfile identity collision (item 1) with a migration strategy first, then items 2-4 in the same or a follow-up PR before multi-host GA.",
  "comment_markdown": "Thanks for the thorough write-up. These four items are well-scoped and the priority ordering is right: the lockfile identity collision (item 1) is the most critical because it is a silent correctness failure -- two different hosts with the same owner/repo path could silently resolve to the same lockfile key.\n\nA few thoughts before implementation:\n\n- **Item 1 (lockfile key):** Adding the host to `get_unique_key()` is straightforward, but it is a lockfile schema change. Any repo with an existing `apm.lock.yaml` that includes non-GitHub hosts will see key drift after the upgrade. A one-time migration step (or at minimum a clear upgrade note in the CHANGELOG) should accompany the PR.\n- **Items 3 and 4 (error clarity + validation):** These are self-contained and can land independently of item 1 if that helps isolate the migration risk.\n- **Item 2 (token resolution):** The ADO/Entra ID auth chain already has precedence logic in `token_manager.py`; make sure the non-GitHub host chain is explicit about fallback order.\n\nIf you want to split this into two PRs (item 1 with migration + items 2-4), that is fine -- just cross-reference them here."
}

---

Triage status: agentic proposal pending human ratification.
Silence is approval. Maintainers can:

• Override any label or milestone above by editing it directly --
  human edits are authoritative and will not be reverted on
  subsequent runs.
• Re-trigger triage by applying the status/needs-triage label, or
  by removing status/triaged to enroll the issue in the next
  daily sweep.

Posted by the Triage Panel workflow. See .apm/skills/apm-triage-panel for the panel skill.

Generated by Triage Panel · ● 2.3M · ◷