fix: preserve credential helpers for generic marketplace git

[Aryan Singh K. (aryansk)]

Summary

Preserve the resolver's normal Git environment for generic marketplace hosts so configured credential helpers such as macOS Keychain or libsecret remain available. Known platform paths such as Azure DevOps keep using the hardened Git environment.

Testing

• Updated generic-host coverage to assert the hardened helper-stripping path is not used
• Kept Azure DevOps coverage on the hardened path

Fixes #2526

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: ship_with_followups

Generic marketplace HTTPS now delegates credentials to native Git helpers without exposing APM platform tokens.

cc Aryan Singh K. (@aryansk) -- a fresh advisory pass is ready for your review.

Proceed with constraints. The policy is centralized in AuthResolver and host providers: generic HTTPS defers native-helper invocation to Git, generic HTTP suppresses credentials, generic SSH remains token-free and noninteractive, and recognized providers retain hardened credentials. The marketplace error is now static, with a unit assertion that helper output is absent from the surfaced exception.

Documentation and changelog framing match the implemented transport policy. The installed-candidate lifecycle test is present but local execution is unavailable because the binary is stale and uv cannot fetch build dependencies after socket error 57; CI must execute that test before merge.

Dissent. The add-specific retry hint remains intentionally deferred: threading operation context through all fetchers and errors exceeds this auth-policy scope and predates this change.

Aligned with: secure_by_default: Platform tokens and inherited authorization-header channels are removed before generic Git transport. multi_harness_multi_host: Provider and transport policy preserve hardened known-host behavior while supporting native generic HTTPS credentials. pragmatic_as_npm: Generic HTTPS follows the user's configured native Git credential helper without interactive prompts.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	1	Remote-aware policy remains centralized.
CLI Logging Expert	0	0	0	Credential-bearing exception text was removed.
DevX UX Expert	0	1	0	Add-specific retry guidance is a separate concern.
Supply Chain Security Expert	0	0	0	Generic Git diagnostics no longer render helper output.
OSS Growth Hacker	0	0	0	Native helper support is clear and accurately documented.
Auth Expert	0	0	0	Provider and transport policy meet the auth boundary.
Doc Writer	0	0	0	Transport docs and reference wording match implementation.
Test Coverage Expert	0	1	0	CI must execute the installed-candidate lifecycle.
Performance Expert	0	0	0	GitCache and sparse checkout behavior remain unchanged.

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

Top 2 follow-ups

1. [Test Coverage Expert] Require CI to run the installed-candidate generic HTTPS credential-helper lifecycle test. -- Local validation could not run it because the available binary is stale and uv failed to download build dependencies with socket error 57.
2. [DevX UX Expert] Track an add-specific retry hint as a separate follow-up. -- Adding operation context through all fetchers and error paths is broader than this auth-policy change and predates it.

Architecture

classDiagram
    direction LR
    class MarketplaceClientModule {
      +_fetch_git(source, file_path, host_info, auth_resolver)
    }
    class AuthResolver {
      +resolve_for_remote(host, remote_url, org) AuthContext
      +git_env_for_remote(ctx, remote_url) dict
    }
    class GitTransportPolicy {
      +use_resolved_credentials bool
      +allow_native_credential_lookup bool
    }
    MarketplaceClientModule ..> AuthResolver : delegates
    AuthResolver ..> GitTransportPolicy : applies

Loading

flowchart TD
    A[marketplace add/list] --> B[_fetch_git]
    B --> C[AuthResolver]
    C --> D[git_transport_policy]
    D --> E{remote and host kind}
    E -->|generic HTTPS| F[native Git helper]
    E -->|generic HTTP| G[isolated configuration]
    E -->|generic SSH| H[token-free SSH]
    E -->|recognized host| I[hardened environment]
    F --> J[GitCache]
    G --> J
    H --> J
    I --> J

Loading

Recommendation

Proceed with constraints. Merge after CI executes the installed-candidate lifecycle test; retain the add-specific retry hint as a scoped follow-up rather than expanding this auth-policy PR.

---

Full per-persona findings

Python Architect

• [nit] Keep future marketplace Git transports routed through the provider registry policy at src/apm_cli/core/host_providers.py:171.

CLI Logging Expert

No findings after the credential-safe error fix.

DevX UX Expert

• [recommended] Track an add-specific retry hint separately. This predates the auth-policy change and requires operation context through all fetchers and errors.

Supply Chain Security Expert

No findings after the credential-safe error fix.

OSS Growth Hacker

No findings after native helper lookup was delegated to Git and docs were corrected.

Auth Expert

No findings.

Doc Writer

No findings after documentation corrections and the reference update.

Test Coverage Expert

• [recommended] Run test_generic_https_marketplace_add_uses_native_credential_helper against the installed candidate in CI. Local package build dependencies cannot be downloaded.

Performance Expert

No findings.

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

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: ship_with_followups

Marketplace Git transport now isolates HTTP credential lookup and limits provider credentials to HTTPS, closing the reported leak and fail-open paths.

cc Aryan Singh K. (@aryansk) -- a fresh advisory pass is ready for your review.

The final patch folds the security findings without changing the supported marketplace workflow: HTTP uses an isolated Git configuration and probes the exact final environment, while provider-resolved credentials are confined to HTTPS. Validated evidence supports shipping: 19 transport-policy tests passed, the generic HTTPS lifecycle passed, and the architecture mutation guard passed. Full uv lint remains unavailable because dependency retrieval requires network access, not because of a reported code failure.

Dissent. No active disagreement remains on a required change. DevX, docs, and test-presentation suggestions are recommended follow-ups rather than reasons to delay the security correction.

Aligned with: secure by default, multi-harness multi-host, pragmatic as npm.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	1	Marketplace delegates remote policy to AuthResolver and host providers.
CLI Logging Expert	0	0	0	Fetch failures retain an actionable retry path without exposing details.
DevX UX Expert	0	2	0	Transport-specific recovery guidance would improve failures.
Supply Chain Security Expert	0	0	0	Initial findings were folded before this final recommendation.
OSS Growth Hacker	0	0	0	Documentation communicates the security improvement clearly.
Auth Expert	0	0	0	Provider credentials are now confined to HTTPS.
Doc Writer	0	1	1	A short cross-reference would improve discoverability.
Test Coverage Expert	0	1	0	Lifecycle and policy coverage passed; add reviewer-facing scenario mapping.
Performance Expert	0	0	0	One local config probe is negligible beside Git network work.

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

Top 3 follow-ups

1. [DevX UX Expert] Make marketplace transport failures identify the attempted transport rather than using generic credentials-or-SSH-key wording -- transport-specific guidance shortens recovery time.
2. [Doc Writer] Add the authentication cross-reference when its documentation patch can be cleanly applied -- it improves discoverability.
3. [Test Coverage Expert] Add a PR Scenario Evidence table -- the exercised HTTPS and SSH policy matrix becomes easier to audit.

Recommendation

Ship the security-hardened transport policy. Track the three recommended clarity and reviewability improvements separately; none outweigh the passed policy and lifecycle evidence for the corrected behavior.

---

Full per-persona findings

The panel's initial blocking auth and supply-chain findings were folded: known-provider SSH is token-free, HTTP replaces caller global config with the isolated config, and HTTPS rewrite checking examines the final Git environment and fails closed. Remaining findings are the three recommended follow-ups above; CLI logging, growth, Python architecture, and performance reported no required change.

_{This panel is advisory. Re-apply the panel-review label after addressing feedback to re-run.}