fix(marketplace): preserve ADO URL and bearer auth#2121
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes Azure DevOps (ADO) marketplace resolution by (1) generating ADO git URLs without an invalid .git suffix and (2) preserving the full auth context so ADO Azure CLI credentials reach git ls-remote as a bearer http.extraheader rather than being treated like a basic/PAT token.
Changes:
- Preserve
AuthContext.auth_schemethrough marketplace check/pack flows and plumb it intoRefResolver. - Generate suffix-free ADO marketplace URLs and inject bearer auth via git environment (
http.extraheader). - Add/adjust regression coverage (new hermetic integration test + updated unit/integration fixtures) and update docs + changelog.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/commands/test_marketplace_check.py | Updates unit tests to patch/expect resolve_auth_for_host and auth_scheme propagation into RefResolver. |
| tests/integration/test_ado_marketplace_e2e.py | Extends the integration auth stub to include auth_scheme so existing E2E coverage remains accurate. |
| tests/integration/test_ado_marketplace_check_auth.py | Adds a hermetic regression test asserting suffix-free ADO URL and bearer header injection into git ls-remote. |
| src/apm_cli/marketplace/ref_resolver.py | Adds auth_scheme support and ADO-specific URL/env building for git ls-remote. |
| src/apm_cli/marketplace/builder.py | Preserves full auth context when creating per-host RefResolver instances during pack/build resolution. |
| src/apm_cli/marketplace/auth_helpers.py | Introduces resolve_auth_for_host() (context-preserving) while retaining token-only helper for callers that need it. |
| src/apm_cli/commands/marketplace/check.py | Uses resolve_auth_for_host, passes auth_scheme into RefResolver, and prints suffix-free ADO endpoint in verbose output. |
| packages/apm-guide/.apm/skills/apm-usage/dependencies.md | Documents ADO sourceBase format and suffix-free repo appending behavior. |
| packages/apm-guide/.apm/skills/apm-usage/authentication.md | Documents bearer-header behavior for ADO marketplace check (Azure CLI fallback). |
| docs/src/content/docs/reference/cli/marketplace.md | Updates CLI reference docs for ADO sourceBase behavior and auth chain. |
| docs/src/content/docs/getting-started/authentication.md | Notes that marketplace check follows the same ADO auth chain and uses bearer headers (not URL embedding). |
| CHANGELOG.md | Adds an Unreleased fix entry for the ADO marketplace URL/auth behavior. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Low
| requested_bearer = self._auth_scheme == "bearer" | ||
| bearer = requested_bearer and is_azure_devops_hostname(self._host) | ||
| token = None if requested_bearer else self._token | ||
| if is_azure_devops_hostname(self._host): | ||
| credentials = f"x-access-token:{token}@" if token else "" | ||
| url = f"https://{credentials}{self._host}/{owner_repo}" | ||
| else: | ||
| url = build_https_clone_url(self._host, owner_repo, token=token) |
| - Azure DevOps marketplace checks now preserve suffix-free `/_git/<repo>` URLs | ||
| and pass Azure CLI bearer authentication through to `git ls-remote`. (closes #2119) |
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 2 | Clean AuthContext lift; harden unsupported bearer hosts. |
| CLI Logging Expert | 0 | 0 | 0 | CLI output follows existing conventions. |
| DevX UX Expert | 0 | 0 | 2 | ADO behavior is clearer; minor diagnostics polish remains. |
| Supply Chain Security Expert | 0 | 0 | 1 | Bearer transport is safe; fail closed on unsupported hosts. |
| Auth Expert | 0 | 0 | 1 | ADO auth plumbing is sound after the guard. |
| Doc Writer | 0 | 2 | 0 | Correct fallback wording and consolidate sourceBase guidance. |
| Test Coverage Expert | 0 | 1 | 0 | Add builder bearer-scheme propagation coverage. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 3 follow-ups
- [Test Coverage Expert] Verify the builder bearer auth-scheme propagation test in the full suite.
- [Doc Writer] Verify consolidated marketplace/auth guidance has no stale claims.
- [DevX UX Expert] Keep unsupported bearer-host diagnostics actionable.
Recommendation
Run the full integration suite and canonical lint against the folded changes, then observe CI green. The strongest test and security findings are addressed in-tree.
Full per-persona findings
- Security/auth: unsupported bearer authentication must fail closed rather than silently become unauthenticated.
- Documentation: distinguish PAT selection from Azure CLI selection; do not imply a rejected-PAT retry in this command path.
- Coverage: prove MarketplaceBuilder preserves
auth_scheme="bearer", in addition to the CLI-level URL/header flow.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
Fail closed when bearer auth is paired with an unsupported host, prove builder scheme propagation, repair the stale auth fixture, and consolidate accurate ADO sourceBase guidance. Addresses panel auth, security, coverage, and docs follow-ups. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ace-auth-url Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep ADO PATs out of process-visible URLs, surface unsupported bearer schemes through the marketplace error type, and remove duplicate builder auth resolution. Addresses the final auth, supply-chain, DevX, and architecture panel findings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Prove the context-preserving helper directly, clarify the URL-only token variable, and replace duplicated marketplace auth prose with the canonical sourceBase cross-reference. Addresses the terminal panel findings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | Architecture is clean and the auth helper lifecycle is unified. |
| CLI Logging Expert | 0 | 0 | 0 | User-visible URL output follows existing conventions. |
| DevX UX Expert | 0 | 0 | 0 | Domain errors are actionable and the happy path remains unchanged. |
| Supply Chain Security Expert | 0 | 0 | 0 | ADO credentials stay out of process-visible URLs. |
| OSS Growth Hacker | 0 | 0 | 0 | The ADO enterprise compatibility story is clear. |
| Auth Expert | 0 | 0 | 0 | Auth scheme propagation and host boundaries are correct. |
| Doc Writer | 0 | 0 | 0 | Auth precedence and sourceBase guidance are accurate and consolidated. |
| Test Coverage Expert | 0 | 0 | 0 | E2E and unit regression traps cover every changed auth branch. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Folded in this run
- (panel) Fail closed for bearer authentication on non-ADO hosts -- resolved in
fcddea690. - (panel) Preserve bearer scheme through MarketplaceBuilder -- resolved in
fcddea690. - (panel) Consolidate accurate ADO auth and sourceBase guidance -- resolved in
fcddea690andb6c45eec6. - (panel) Keep ADO PATs out of process-visible URLs -- resolved in
bcfc7f135. - (panel) Raise marketplace-domain auth errors and unify builder auth resolution -- resolved in
bcfc7f135. - (panel) Directly prove AuthContext scheme preservation -- resolved in
b6c45eec6.
Regression-trap evidence (mutation-break gate)
test_marketplace_check_uses_ado_url_and_bearer_auth-- adding.gitto the ADO URL failed; guard restored.test_marketplace_check_uses_ado_url_and_bearer_auth-- removing bearer header injection failed; guard restored.test_ado_bearer_auth_scheme_reaches_resolver-- forcing basic auth failed; guard restored.test_bearer_auth_rejects_non_ado_host-- removing fail-closed guard failed; guard restored.test_ado_pat_uses_basic_header_without_url_credentials-- removing Basic header injection failed; guard restored.test_resolve_auth_for_host_preserves_bearer_scheme-- discarding AuthContext failed; guard restored.
Validation
bash scripts/test-integration.sh: 10206 passed, 172 skipped, 16 deselected, 2 xfailed.- Marketplace suites: 1445 passed.
- Canonical ruff, format, pylint R0801, YAML, file-length, relative-path, and auth-signal checks are clean.
- GitHub CI run
29112660522: all required checks passed onb6c45eec6.
Mergeability status
| PR | head SHA | CEO stance | iters | folds | defers | Copilot rounds | CI | mergeable | merge state | notes |
|---|---|---|---|---|---|---|---|---|---|---|
| #2121 | b6c45ee |
ship_now | 3 | 6 | 0 | 2 | green | MERGEABLE | required review pending | awaiting maintainer review |
Recommendation
All in-scope findings are folded and every automated gate is green. Ready for maintainer review.
This panel is advisory. It does not gate merge.
fix(marketplace): preserve Azure DevOps URL and bearer auth
TL;DR
Azure DevOps marketplace checks now use the suffix-free
/_git/<repo>URL that ADO accepts. Marketplace resolution also preservesAuthContext.auth_scheme, so Azure CLI credentials reachgit ls-remoteas a bearer header instead of being treated like a PAT.Note
Fixes #2119 with a hermetic CLI-level regression test covering both failures in one flow.
Problem (WHY)
apm marketplace checkappended.gitto ADOsourceBaserepositories, making ADO resolve a different repository name (reported failure).AuthContextto a token string, discarding the bearer scheme beforegit ls-remote(reported failure)..gitendpoint (reported output).Approach (WHAT)
.git; retain suffix behavior for other hosts.http.extraheaderenvironment configuration.Implementation (HOW)
marketplace/auth_helpers.pymarketplace/ref_resolver.pycommands/marketplace/check.pyauth_schemeintoRefResolverand prints the suffix-free ADO endpoint.marketplace/builder.pyDiagrams
Legend: dashed stages are corrected by this PR; the bearer never enters the URL.
flowchart LR M["apm.yml sourceBase"] --> C["marketplace check"] C --> A["AuthResolver context"] A --> R["RefResolver"] R --> U["ADO URL without .git"] R --> H["Bearer http.extraheader"] U --> G["git ls-remote"] H --> G classDef new stroke-dasharray: 5 5; class A,U,H new;Trade-offs
auth_scheme; it does not guess from token contents..gitcompatibility.sourceBasecreates wrong URL for packages #2119.Benefits
sourceBaseresolves exactly to/org/project/_git/repo, checked withurllib.parse.Validation
Regression test before and after the fix:
Mutation-break evidence after independently changing the production URL and auth guards, then restoring them:
Relevant ADO integration and marketplace unit suites:
Canonical lint gate:
Complete integration-suite evidence
bash scripts/test-integration.shcollected the entire integration directory:The ADO fixture failure was corrected and passes in the targeted ADO integration run. The remaining failure reproduces independently in unchanged code:
tests/integration/test_intra_package_cleanup.py::TestCrossPackageSharedFileCleanup::test_shared_file_survives_when_other_package_still_deploys_it.Scenario Evidence
apm marketplace checksucceeds for an ADOsourceBasewhile signed in through Azure CLItests/integration/test_ado_marketplace_check_auth.py::test_marketplace_check_uses_ado_url_and_bearer_auth(regression-trap for #2119)tests/integration/test_ado_marketplace_e2e.pyHow to test
uv run --extra dev pytest tests/integration/test_ado_marketplace_check_auth.py -q; expect one pass without network.az login, runapm marketplace check --verbose; expect a suffix-free URL and successful ref check.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com