diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fcf6a60f..c78c4e335 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed (BREAKING) + +- Strict-by-default transport selection: explicit `ssh://`/`https://` URLs no longer silently fall back to the other protocol; shorthand consults `git config url..insteadOf` and otherwise defaults to HTTPS. Set `APM_ALLOW_PROTOCOL_FALLBACK=1` (or pass `--allow-protocol-fallback`) to restore the legacy permissive chain; cross-protocol retries then emit a `[!]` warning. Closes #328 (#778) + ### Added +- `apm install --ssh` / `--https` flags and `APM_GIT_PROTOCOL=ssh|https` env to pick the initial transport for shorthand dependencies (#778) +- `apm install --allow-protocol-fallback` flag and `APM_ALLOW_PROTOCOL_FALLBACK=1` env as the migration escape hatch for cross-protocol fallback (#778) - Add APM Review Panel skill (`.github/skills/apm-review-panel/`) and four new specialist personas (`devx-ux-expert`, `supply-chain-security-expert`, `apm-ceo`, `oss-growth-hacker`) with auto-activating per-persona skills. Routes specialist findings through an APM CEO arbiter for strategic / breaking-change calls, with the OSS growth hacker side-channeling adoption insights via `WIP/growth-strategy.md`. Instrumentation per Handbook Ch. 9 (`The Instrumented Codebase`); PROSE-compliant (thin SKILL.md routers, persona detail lazy-loaded via markdown links, explicit boundaries per persona). ### Fixed diff --git a/docs/src/content/docs/getting-started/authentication.md b/docs/src/content/docs/getting-started/authentication.md index 8768531e1..c852a2d32 100644 --- a/docs/src/content/docs/getting-started/authentication.md +++ b/docs/src/content/docs/getting-started/authentication.md @@ -284,6 +284,37 @@ gh auth login # GitHub CLI ### SSH connection hangs on corporate/VPN networks -When no token is available, APM tries SSH before falling back to plain HTTPS. Firewalls that silently drop SSH packets (port 22) can make `apm install` appear to hang. APM sets `GIT_SSH_COMMAND="ssh -o ConnectTimeout=30"` so SSH attempts fail within 30 seconds and the fallback proceeds to HTTPS with git credential helpers. +When APM clones over SSH (because the dependency is an SSH URL, the user +passed `--ssh`, `git config url..insteadOf` rewrites to SSH, or +`--allow-protocol-fallback` is in effect), firewalls that silently drop SSH +packets (port 22) can make `apm install` appear to hang. APM sets +`GIT_SSH_COMMAND="ssh -o ConnectTimeout=30"` so SSH attempts fail within 30 +seconds. -If you already set `GIT_SSH_COMMAND` (e.g., for a custom key), APM appends `-o ConnectTimeout=30` unless `ConnectTimeout` is already present in your value. +If you already set `GIT_SSH_COMMAND` (e.g., for a custom key), APM appends +`-o ConnectTimeout=30` unless `ConnectTimeout` is already present in your +value. + +If SSH is unreachable from your network, force HTTPS: + +```bash +apm install --https +export APM_GIT_PROTOCOL=https +``` + +## Choosing transport (SSH vs HTTPS) + +Authentication and transport are independent decisions: + +- **HTTPS** uses the token resolution chain documented above. APM resolves a + token per `(host, org)` and embeds it in the clone URL. +- **SSH** uses your existing ssh-agent and `~/.ssh/config`. APM does not + select keys or override agent behavior -- whatever `git clone` would do + on the same machine, APM does. + +APM picks the transport per dependency using a strict contract (explicit +URL scheme honored exactly; shorthand uses HTTPS unless +`git config url..insteadOf` rewrites it to SSH). For the full +selection matrix, the `--ssh` / `--https` flags, the `APM_GIT_PROTOCOL` +env var, and the `--allow-protocol-fallback` escape hatch, see +[Dependencies: Transport selection](../../guides/dependencies/#transport-selection-ssh-vs-https). diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md index 5306ef5d0..1a5532d6b 100644 --- a/docs/src/content/docs/guides/dependencies.md +++ b/docs/src/content/docs/guides/dependencies.md @@ -145,7 +145,7 @@ dependencies: ref: v1.0 ``` -Fields: `git` (required), `path`, `ref`, `alias` (all optional). The `git` value is any HTTPS or SSH clone URL. Custom ports are preserved across protocols — if the SSH clone fails and APM falls back to HTTPS, the same port number is reused (so `ssh://host:7999/...` falls back to `https://host:7999/...`). +Fields: `git` (required), `path`, `ref`, `alias` (all optional). The `git` value is any HTTPS or SSH clone URL. Explicit URL schemes are honored exactly -- see [Transport selection](#transport-selection-ssh-vs-https) for the full contract. Custom ports are preserved across every attempt (including any cross-protocol fallback enabled with `--allow-protocol-fallback`), so `ssh://host:7999/...` retried over HTTPS becomes `https://host:7999/...`. > **Nested groups (GitLab, Gitea, etc.):** APM treats all path segments after the host as the repo path, so `gitlab.com/group/subgroup/repo` resolves to a repo at `group/subgroup/repo`. Virtual paths on simple 2-segment repos work with shorthand (`gitlab.com/owner/repo/file.prompt.md`). But for **nested-group repos + virtual paths**, use the object format — the shorthand is ambiguous: > @@ -426,6 +426,70 @@ apm install --trust-transitive-mcp Run `apm install --dry-run` to preview MCP dependency configuration without writing any files. Self-defined deps are validated for required fields and transport values; overlay deps are loaded as-is and unknown fields are ignored. +## Transport selection (SSH vs HTTPS) + +APM picks SSH or HTTPS per dependency using a strict, predictable contract. + +:::caution[Breaking change in APM 0.8.13] +APM versions before 0.8.13 silently retried failed clones across protocols. +Starting in 0.8.13 the behavior is **strict by default**: explicit URL schemes are honored exactly, +and shorthand uses HTTPS unless `git config url..insteadOf` rewrites it +to SSH. To restore the legacy permissive chain temporarily (e.g. while +migrating CI), set `APM_ALLOW_PROTOCOL_FALLBACK=1` or pass +`--allow-protocol-fallback`. +::: + +| Dependency form | What APM tries | +|-----------------|----------------| +| `ssh://...` or `git@host:...` | SSH only | +| `https://...` or `http://...` | HTTPS only | +| Shorthand (`owner/repo`, `host/owner/repo`) with `git config url..insteadOf` rewriting to SSH | SSH only | +| Shorthand without a matching `insteadOf` rewrite | HTTPS only | + +A failed clone fails loudly, naming the URL and the protocol attempted. APM +no longer downgrades `ssh://` to HTTPS or vice-versa. + +### Honoring `git config insteadOf` + +If your machine rewrites HTTPS to SSH for a host, APM matches `git clone`'s +behavior on that machine. Example: + +```bash +git config --global url."git@github.com:".insteadOf "https://github.com/" +apm install owner/repo # APM clones over SSH +``` + +No CLI flag is needed. `insteadOf` is consulted only for shorthand +dependencies; explicit URLs in `apm.yml` are not rewritten. + +### Forcing the initial protocol for shorthand + +```bash +apm install owner/repo --ssh # force SSH for shorthand +apm install owner/repo --https # force HTTPS for shorthand +export APM_GIT_PROTOCOL=ssh # session default +``` + +`--ssh` and `--https` are mutually exclusive and apply only to shorthand +dependencies. URLs with an explicit scheme ignore them. + +### Restoring the legacy permissive chain + +```bash +apm install --allow-protocol-fallback +export APM_ALLOW_PROTOCOL_FALLBACK=1 # CI / migration window +``` + +When fallback runs, each cross-protocol retry emits a `[!]` warning naming +both protocols. Use this to unblock a pipeline while you fix the root +cause -- not as a long-term setting. + +For SSH key selection (ssh-agent, `~/.ssh/config`) and HTTPS token +resolution, see +[Authentication](../../getting-started/authentication/#choosing-transport-ssh-vs-https). +For the CLI flag and env var reference, see +[`apm install`](../../reference/cli-commands/#apm-install---install-dependencies-and-deploy-local-content). + ## GitHub Authentication Setup For GitHub and GitHub Enterprise repositories, set up a personal access token: @@ -465,7 +529,7 @@ If authentication fails, you'll see an error with guidance on token setup. For non-GitHub repositories, APM delegates authentication to git — it never sends GitHub tokens to non-GitHub hosts: - **Public repos**: Work without authentication via HTTPS -- **Private repos via SSH**: Configure SSH keys for your host — APM falls back to SSH automatically +- **Private repos via SSH**: Configure SSH keys for your host. Use an `ssh://` or `git@host:` URL, or set up `git config url..insteadOf` to rewrite shorthand to SSH (see [Transport selection](#transport-selection-ssh-vs-https)) - **Private repos via HTTPS**: Configure a [git credential helper](https://git-scm.com/docs/gitcredentials) — APM allows credential helpers for non-GitHub hosts ```bash diff --git a/docs/src/content/docs/reference/cli-commands.md b/docs/src/content/docs/reference/cli-commands.md index 1f93856ce..9190dc4f7 100644 --- a/docs/src/content/docs/reference/cli-commands.md +++ b/docs/src/content/docs/reference/cli-commands.md @@ -96,6 +96,18 @@ apm install [PACKAGES...] [OPTIONS] - `--trust-transitive-mcp` - Trust self-defined MCP servers from transitive packages (skip re-declaration requirement) - `--dev` - Add packages to [`devDependencies`](../manifest-schema/#5-devdependencies) instead of `dependencies`. Dev deps are installed locally but excluded from `apm pack --format plugin` bundles - `-g, --global` - Install to user scope (`~/.apm/`) instead of the current project. Primitives deploy to `~/.copilot/`, `~/.claude/`, etc. +- `--ssh` - Force SSH for shorthand (`owner/repo`) dependencies. Mutually exclusive with `--https`. Ignored for URLs with an explicit scheme. +- `--https` - Force HTTPS for shorthand dependencies. Mutually exclusive with `--ssh`. Default unless `git config url..insteadOf` rewrites the candidate to SSH. +- `--allow-protocol-fallback` - Restore the legacy permissive cross-protocol fallback chain (HTTPS-then-SSH or vice-versa). Strict-by-default otherwise. Each retry emits a `[!]` warning naming both protocols. + +**Transport env vars:** + +| Variable | Purpose | +|----------|---------| +| `APM_GIT_PROTOCOL` | `ssh` or `https`. Default initial transport for shorthand dependencies (overridden by `--ssh` / `--https`). | +| `APM_ALLOW_PROTOCOL_FALLBACK` | Set to `1` to enable the legacy permissive chain without passing `--allow-protocol-fallback`. | + +See [Dependencies: Transport selection](../../guides/dependencies/#transport-selection-ssh-vs-https) for the full selection matrix. **Behavior:** - `apm install` (no args): Installs **all** packages from `apm.yml` and deploys the project's own `.apm/` content diff --git a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md index e417a9ad7..00aa82c58 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md +++ b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md @@ -38,8 +38,8 @@ dependencies: ### Custom git ports Non-default git ports are preserved on `https://`, `http://`, and `ssh://` URLs -and threaded through all clone attempts. When the SSH clone fails, the HTTPS -fallback reuses the same port instead of silently dropping it. +and threaded through every clone attempt (including any cross-protocol +fallback enabled with `--allow-protocol-fallback`). - Use the `ssh://` form to specify an SSH port (e.g. `ssh://git@host:7999/owner/repo.git`). The SCP shorthand @@ -48,6 +48,50 @@ fallback reuses the same port instead of silently dropping it. is set. Port is a transport detail, not part of the package identity -- the same repo reachable on different ports dedupes to one entry. +## Transport selection (SSH vs HTTPS) + +Strict by default. Pick the transport up front; APM never silently retries +across protocols. + +| Dependency form | What APM tries | +|-----------------|----------------| +| `ssh://...` or `git@host:...` | SSH only | +| `https://...` or `http://...` | HTTPS only | +| Shorthand with `git config url..insteadOf` rewriting to SSH | SSH only | +| Shorthand otherwise | HTTPS only | + +A failed clone fails loudly, naming the URL and the protocol attempted. +Explicit URL schemes are honored exactly. + +Force the initial protocol for shorthand: + +```bash +apm install owner/repo --ssh # SSH for shorthand +apm install owner/repo --https # HTTPS for shorthand +export APM_GIT_PROTOCOL=ssh # session default +``` + +`--ssh` and `--https` are mutually exclusive and apply only to shorthand. +URLs with an explicit scheme ignore them. + +Match local `git clone` behavior by configuring `insteadOf` once: + +```bash +git config --global url."git@github.com:".insteadOf "https://github.com/" +apm install owner/repo # APM clones over SSH +``` + +Restore the legacy permissive chain (escape hatch -- not a long-term +setting): + +```bash +apm install --allow-protocol-fallback +export APM_ALLOW_PROTOCOL_FALLBACK=1 # CI / migration window +``` + +When fallback runs, each cross-protocol retry emits a `[!]` warning naming +both protocols. + ## Object form (complex cases) ```yaml diff --git a/scripts/test-integration.sh b/scripts/test-integration.sh index 1ca3a19d8..148f8bd29 100755 --- a/scripts/test-integration.sh +++ b/scripts/test-integration.sh @@ -378,6 +378,19 @@ run_e2e_tests() { exit 1 fi + # Run Transport Selection integration tests (issue #778) + # Always-on cases use HTTPS against a public repo. SSH cases auto-skip + # when no usable SSH key is available for git@github.com. + log_info "Running Transport Selection integration tests..." + echo "Command: APM_RUN_INTEGRATION_TESTS=1 pytest tests/integration/test_transport_selection_integration.py -v -s --tb=short" + + if APM_RUN_INTEGRATION_TESTS=1 pytest tests/integration/test_transport_selection_integration.py -v -s --tb=short; then + log_success "Transport Selection integration tests passed!" + else + log_error "Transport Selection integration tests failed!" + exit 1 + fi + # Run Azure DevOps E2E tests (requires ADO_APM_PAT) if [[ -n "${ADO_APM_PAT:-}" ]]; then log_info "Running Azure DevOps E2E tests..." diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 6d138c82e..66439c51c 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -3,7 +3,7 @@ import builtins import sys from pathlib import Path -from typing import List +from typing import List, Optional import click @@ -392,8 +392,29 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo default=False, help="Install to user scope (~/.apm/) instead of the current project", ) +@click.option( + "--ssh", + "use_ssh", + is_flag=True, + default=False, + help="Prefer SSH transport for shorthand (owner/repo) dependencies. Mutually exclusive with --https.", +) +@click.option( + "--https", + "use_https", + is_flag=True, + default=False, + help="Prefer HTTPS transport for shorthand (owner/repo) dependencies. Mutually exclusive with --ssh.", +) +@click.option( + "--allow-protocol-fallback", + "allow_protocol_fallback", + is_flag=True, + default=False, + help="Restore the legacy permissive cross-protocol fallback chain (escape hatch for migrating users; also: APM_ALLOW_PROTOCOL_FALLBACK=1).", +) @click.pass_context -def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbose, trust_transitive_mcp, parallel_downloads, dev, target, global_): +def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbose, trust_transitive_mcp, parallel_downloads, dev, target, global_, use_ssh, use_https, allow_protocol_fallback): """Install APM and MCP dependencies from apm.yml (like npm install). This command automatically detects AI runtimes from your apm.yml scripts and installs @@ -421,6 +442,24 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo is_partial = bool(packages) logger = InstallLogger(verbose=verbose, dry_run=dry_run, partial=is_partial) + # Resolve transport selection inputs. + from ..deps.transport_selection import ( + ProtocolPreference, + is_fallback_allowed, + protocol_pref_from_env, + ) + if use_ssh and use_https: + _rich_error("Options --ssh and --https are mutually exclusive.", symbol="error") + sys.exit(2) + if use_ssh: + protocol_pref = ProtocolPreference.SSH + elif use_https: + protocol_pref = ProtocolPreference.HTTPS + else: + protocol_pref = protocol_pref_from_env() + # CLI flag OR env var enables fallback. + allow_protocol_fallback = allow_protocol_fallback or is_fallback_allowed() + # Resolve scope from ..core.scope import InstallScope, get_apm_dir, get_manifest_path, get_modules_dir, ensure_user_dirs, warn_unsupported_user_scope scope = InstallScope.USER if global_ else InstallScope.PROJECT @@ -593,6 +632,8 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo marketplace_provenance=( outcome.marketplace_provenance if packages and outcome else None ), + protocol_pref=protocol_pref, + allow_protocol_fallback=allow_protocol_fallback, ) apm_count = install_result.installed_count prompt_count = install_result.prompts_integrated @@ -732,6 +773,8 @@ def _install_apm_dependencies( auth_resolver: "AuthResolver" = None, target: str = None, marketplace_provenance: dict = None, + protocol_pref=None, + allow_protocol_fallback: "Optional[bool]" = None, ): """Thin wrapper -- builds an :class:`InstallRequest` and delegates to :class:`apm_cli.install.service.InstallService`. @@ -759,6 +802,8 @@ def _install_apm_dependencies( auth_resolver=auth_resolver, target=target, marketplace_provenance=marketplace_provenance, + protocol_pref=protocol_pref, + allow_protocol_fallback=allow_protocol_fallback, ) return InstallService().run(request) diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py index 2a137739d..3c4e6d762 100644 --- a/src/apm_cli/deps/github_downloader.py +++ b/src/apm_cli/deps/github_downloader.py @@ -29,6 +29,7 @@ validate_apm_package, APMPackage ) +from ..utils.console import _rich_warning from ..utils.github_host import ( build_https_clone_url, build_ssh_url, @@ -43,6 +44,15 @@ is_github_hostname ) from ..utils.yaml_io import yaml_to_str +from .transport_selection import ( + GitConfigInsteadOfResolver, + InsteadOfResolver, + ProtocolPreference, + TransportPlan, + TransportSelector, + is_fallback_allowed, + protocol_pref_from_env, +) def normalize_collection_path(virtual_path: str) -> str: @@ -159,12 +169,36 @@ def _get_op_name(self, op_code): class GitHubPackageDownloader: """Downloads and validates APM packages from GitHub repositories.""" - def __init__(self, auth_resolver=None): - """Initialize the GitHub package downloader.""" + def __init__( + self, + auth_resolver=None, + transport_selector: Optional[TransportSelector] = None, + protocol_pref: Optional[ProtocolPreference] = None, + allow_fallback: Optional[bool] = None, + ): + """Initialize the GitHub package downloader. + + Args: + auth_resolver: Auth resolver instance. Defaults to a new AuthResolver. + transport_selector: TransportSelector for protocol decisions. + Defaults to a new selector with GitConfigInsteadOfResolver. + protocol_pref: User-stated transport preference for shorthand + deps. When None, reads APM_GIT_PROTOCOL env. + allow_fallback: When True, permits cross-protocol fallback + (legacy behavior). When None, reads + APM_ALLOW_PROTOCOL_FALLBACK env. + """ from apm_cli.core.auth import AuthResolver self.auth_resolver = auth_resolver or AuthResolver() self.token_manager = self.auth_resolver._token_manager # Backward compat self.git_env = self._setup_git_environment() + self._transport_selector = transport_selector or TransportSelector() + self._protocol_pref = ( + protocol_pref if protocol_pref is not None else protocol_pref_from_env() + ) + self._allow_fallback = ( + allow_fallback if allow_fallback is not None else is_fallback_allowed() + ) def _setup_git_environment(self) -> Dict[str, Any]: """Set up Git environment with authentication using centralized token manager. @@ -589,9 +623,16 @@ def _build_repo_url(self, repo_ref: str, use_ssh: bool = False, dep_ref: Depende # Check if this is Azure DevOps (either via dep_ref or host detection) is_ado = (dep_ref and dep_ref.is_azure_devops()) or is_azure_devops_hostname(host) - # Use provided token or fall back to instance default - github_token = token if token is not None else self.github_token - ado_token = token if (token is not None and is_ado) else self.ado_token + # Use provided token or fall back to instance default. Pass an empty + # string ("") explicitly to suppress the per-instance token (used by + # the TransportSelector for "plain HTTPS" / "SSH" attempts that must + # NOT embed credentials in the URL). + if token == "": + github_token = "" + ado_token = "" + else: + github_token = token if token is not None else self.github_token + ado_token = token if (token is not None and is_ado) else self.ado_token _debug(f"_build_repo_url: host={host}, is_ado={is_ado}, dep_ref={'present' if dep_ref else 'None'}, " f"ado_org={dep_ref.ado_organization if dep_ref else None}") @@ -631,17 +672,20 @@ def _build_repo_url(self, repo_ref: str, use_ssh: bool = False, dep_ref: Depende return build_https_clone_url(host, repo_ref, token=None, port=port) def _clone_with_fallback(self, repo_url_base: str, target_path: Path, progress_reporter=None, dep_ref: DependencyReference = None, verbose_callback=None, **clone_kwargs) -> Repo: - """Attempt to clone a repository with fallback authentication methods. + """Clone a repository following the TransportSelector plan. - Uses authentication patterns appropriate for the platform: - - GitHub: x-access-token format for private repos, SSH, or HTTPS - - Azure DevOps: PAT-based authentication + The transport selector decides protocol order and strictness based on + the user's URL form, CLI/env preferences, and git ``insteadOf`` config. + Strict-by-default: explicit ``ssh://`` and ``https://`` URLs no longer + silently fall back to a different protocol. To restore the legacy + permissive chain, set ``--allow-protocol-fallback`` or + ``APM_ALLOW_PROTOCOL_FALLBACK=1``. Args: repo_url_base: Base repository reference (owner/repo) target_path: Target path for cloning progress_reporter: GitProgressReporter instance for progress updates - dep_ref: Optional DependencyReference for platform-specific URL building + dep_ref: DependencyReference for platform/protocol decisions verbose_callback: Optional callable for verbose logging (receives str messages) **clone_kwargs: Additional arguments for Repo.clone_from @@ -649,75 +693,112 @@ def _clone_with_fallback(self, repo_url_base: str, target_path: Path, progress_r Repo: Successfully cloned repository Raises: - RuntimeError: If all authentication methods fail + RuntimeError: If the planned attempt(s) all fail. """ last_error = None is_ado = dep_ref and dep_ref.is_azure_devops() - # Determine host type for auth decisions dep_host = dep_ref.host if dep_ref else None if dep_host: is_github = is_github_hostname(dep_host) else: - # When no host is specified, default to GitHub behavior is_github = True is_generic = not is_ado and not is_github - # Resolve per-dependency token via AuthResolver. dep_token = self._resolve_dep_token(dep_ref) - has_token = dep_token + has_token = dep_token is not None - _debug(f"_clone_with_fallback: repo={repo_url_base}, is_ado={is_ado}, is_generic={is_generic}, has_token={has_token is not None}") + _debug( + f"_clone_with_fallback: repo={repo_url_base}, is_ado={is_ado}, " + f"is_generic={is_generic}, has_token={has_token}, " + f"protocol_pref={self._protocol_pref.value}, allow_fallback={self._allow_fallback}" + ) - # When APM has a token for this host, use the locked-down env (APM manages auth). - # When no token is available, relax the env so git credential helpers (gh auth, - # macOS Keychain, etc.) can provide credentials -- regardless of host. - if has_token: - clone_env = self.git_env - else: - clone_env = {k: v for k, v in self.git_env.items() - if k not in ('GIT_ASKPASS', 'GIT_CONFIG_GLOBAL', 'GIT_CONFIG_NOSYSTEM')} - clone_env['GIT_TERMINAL_PROMPT'] = '0' # Still prevent interactive prompts + # Choose the clone env PER ATTEMPT (not per dependency): only the + # token-bearing attempt should run with the locked-down env that + # silences credential helpers. SSH and plain-HTTPS attempts in a + # mixed allow_fallback plan need the relaxed env so user-configured + # credential helpers (gh auth, Keychain, ssh-agent passphrase + # prompts) keep working. + def _env_for(use_token_attempt: bool) -> Dict[str, str]: + if use_token_attempt: + return self.git_env + relaxed = {k: v for k, v in self.git_env.items() + if k not in ('GIT_ASKPASS', 'GIT_CONFIG_GLOBAL', 'GIT_CONFIG_NOSYSTEM')} + relaxed['GIT_TERMINAL_PROMPT'] = '0' + return relaxed + + plan: TransportPlan = self._transport_selector.select( + dep_ref=dep_ref, + cli_pref=self._protocol_pref, + allow_fallback=self._allow_fallback, + has_token=has_token, + ) + _debug( + "transport plan: " + f"strict={plan.strict}, attempts={[(a.scheme, a.use_token, a.label) for a in plan.attempts]}" + ) + + prev_label: Optional[str] = None + prev_scheme: Optional[str] = None + for attempt in plan.attempts: + # Defensive: skip token-bearing attempts when no token available. + if attempt.use_token and not has_token: + continue + + use_ssh = attempt.scheme == "ssh" + try: + url = self._build_repo_url( + repo_url_base, + use_ssh=use_ssh, + dep_ref=dep_ref, + token=dep_token if attempt.use_token else "", + ) + except Exception as e: + last_error = e + continue + + # Surface a [!] warning when the plan permits fallback and we + # are actually switching git protocols (ssh <-> https) mid-clone + # rather than just retrying with different auth on the same protocol. + if ( + not plan.strict + and prev_label + and prev_scheme + and prev_scheme != attempt.scheme + ): + _rich_warning( + f"Protocol fallback: {prev_label} clone of {repo_url_base} failed; retrying with {attempt.label}.", + symbol="warning", + ) - # Method 1: Try authenticated HTTPS if token is available (GitHub/ADO only) - if has_token: try: - auth_url = self._build_repo_url(repo_url_base, use_ssh=False, dep_ref=dep_ref, token=dep_token) - _debug(f"Attempting clone with authenticated HTTPS (URL sanitized)") - repo = Repo.clone_from(auth_url, target_path, env=clone_env, progress=progress_reporter, **clone_kwargs) + _debug(f"Attempting clone with {attempt.label} (URL sanitized)") + repo = Repo.clone_from( + url, target_path, env=_env_for(attempt.use_token), + progress=progress_reporter, **clone_kwargs, + ) if verbose_callback: - masked = self._sanitize_git_error(auth_url) - verbose_callback(f"Cloned from: {masked}") + display = self._sanitize_git_error(url) if attempt.use_token else url + verbose_callback(f"Cloned from: {display}") return repo except GitCommandError as e: last_error = e - # Continue to next method - - # Method 2: Try SSH (works with SSH keys for any host). - # dep_ref.port is threaded through build_ssh_url so non-default SSH ports - # (e.g. Bitbucket Datacenter on 7999) are preserved in the emitted URL. - try: - ssh_url = self._build_repo_url(repo_url_base, use_ssh=True, dep_ref=dep_ref) - repo = Repo.clone_from(ssh_url, target_path, env=clone_env, progress=progress_reporter, **clone_kwargs) - if verbose_callback: - verbose_callback(f"Cloned from: {ssh_url}") - return repo - except GitCommandError as e: - last_error = e - # Continue to next method - - # Method 3: Try standard HTTPS (public repos, or git credential helper for generic hosts) - try: - https_url = self._build_repo_url(repo_url_base, use_ssh=False, dep_ref=dep_ref) - repo = Repo.clone_from(https_url, target_path, env=clone_env, progress=progress_reporter, **clone_kwargs) - if verbose_callback: - verbose_callback(f"Cloned from: {https_url}") - return repo - except GitCommandError as e: - last_error = e - - # All methods failed - error_msg = f"Failed to clone repository {repo_url_base} using all available methods. " + prev_label = attempt.label + prev_scheme = attempt.scheme + if plan.strict: + break + + # All planned attempts failed (or strict-mode single failure) + if plan.strict and len(plan.attempts) >= 1: + tried = plan.attempts[0].label + error_msg = ( + f"Failed to clone repository {repo_url_base} via {tried}. " + ) + if plan.fallback_hint: + error_msg += plan.fallback_hint + " " + else: + error_msg = f"Failed to clone repository {repo_url_base} using all available methods. " configured_host = os.environ.get("GITHUB_HOST", "") if is_ado and not self.has_ado_token: host = dep_host or "dev.azure.com" diff --git a/src/apm_cli/deps/transport_selection.py b/src/apm_cli/deps/transport_selection.py new file mode 100644 index 000000000..a22f7a44a --- /dev/null +++ b/src/apm_cli/deps/transport_selection.py @@ -0,0 +1,314 @@ +"""Transport (protocol) selection for dependency clones. + +Issue microsoft/apm#778. Pure decision engine: given a dependency reference, +the user's CLI/env preferences, and whether an auth token is available, +produce an ordered :class:`TransportPlan` of attempts plus a strictness flag. + +The selector contains no I/O. Discovery of git ``insteadOf`` rewrites is +delegated to an injected :class:`InsteadOfResolver` so unit tests can +substitute fakes and the orchestrator can re-use a single resolver instance +across many dependency clones in one ``apm install`` run. + +Strict-by-default: explicit ``ssh://`` or ``https://`` URLs are honored +exactly. Cross-protocol fallback is only attempted when the user opts in +via ``--allow-protocol-fallback`` or ``APM_ALLOW_PROTOCOL_FALLBACK=1``. +""" + +from __future__ import annotations + +import os +import subprocess +import threading +from dataclasses import dataclass, field +from enum import Enum +from typing import List, Optional, Protocol, runtime_checkable + +# Public env vars (also recognized by CLI flag plumbing). +ENV_PROTOCOL = "APM_GIT_PROTOCOL" +ENV_ALLOW_FALLBACK = "APM_ALLOW_PROTOCOL_FALLBACK" + +# Documented escape-hatch hint surfaced on strict-mode failures. +FALLBACK_HINT = ( + "To allow cross-protocol fallback (not recommended), pass " + "--allow-protocol-fallback or set APM_ALLOW_PROTOCOL_FALLBACK=1." +) + + +class ProtocolPreference(Enum): + """User-stated default transport for shorthand dependencies. + + ``NONE`` means the user did not state a preference; the selector then + consults git ``insteadOf`` config to decide between SSH and HTTPS. + """ + + NONE = "none" + SSH = "ssh" + HTTPS = "https" + + @classmethod + def from_str(cls, value: Optional[str]) -> "ProtocolPreference": + if not value: + return cls.NONE + v = value.strip().lower() + if v in ("ssh",): + return cls.SSH + if v in ("https", "http"): + return cls.HTTPS + return cls.NONE + + +@dataclass(frozen=True) +class TransportAttempt: + """A single clone attempt in the transport plan. + + Attributes: + scheme: ``"ssh"`` or ``"https"``. Drives the URL builder. + use_token: When ``True`` the orchestrator embeds the resolved auth + token in the HTTPS URL (auth-HTTPS). Only meaningful for + ``scheme == "https"``. + label: Human-readable description for log/error output. + """ + + scheme: str + use_token: bool + label: str + + +@dataclass(frozen=True) +class TransportPlan: + """Ordered list of attempts plus strictness policy. + + Attributes: + attempts: Ordered list. The orchestrator iterates in order. + strict: When ``True`` the orchestrator must stop after the first + failed attempt and surface a clear error. When ``False`` the + orchestrator may try the next attempt (legacy permissive path). + fallback_hint: Optional message to include in the error when a + strict-mode attempt fails. Surfaces the escape-hatch flag. + """ + + attempts: List[TransportAttempt] + strict: bool + fallback_hint: Optional[str] = None + + +@runtime_checkable +class InsteadOfResolver(Protocol): + """Discovers ``git config url..insteadOf`` rewrites. + + Implementations return the rewritten URL when a rule matches the + candidate, otherwise ``None``. Implementations are expected to cache + results internally so the selector can be invoked many times per + install without re-shelling to git. + """ + + def resolve(self, candidate_url: str) -> Optional[str]: # pragma: no cover - Protocol + ... + + +class NoOpInsteadOfResolver: + """Test/fallback resolver that always returns ``None``. + + Used in unit tests that don't care about ``insteadOf`` and as a graceful + degradation when ``git`` is missing. + """ + + def resolve(self, candidate_url: str) -> Optional[str]: + return None + + +class GitConfigInsteadOfResolver: + """Reads all ``url.*.insteadOf`` rewrites from git config (lazy + cached). + + Implementation note: this resolver MUST run ``git config`` with the + process's normal environment, NOT with the downloader's locked-down + git env (which sets ``GIT_CONFIG_GLOBAL=/dev/null`` and would suppress + the user's ``insteadOf`` rules entirely, defeating the purpose). + """ + + def __init__(self) -> None: + self._rewrites: Optional[List[tuple]] = None # list of (insteadof_value, target_base) + self._lock = threading.Lock() + + def resolve(self, candidate_url: str) -> Optional[str]: + if self._rewrites is None: + with self._lock: + if self._rewrites is None: + self._rewrites = self._load_rewrites() + best_prefix = "" + best_base = "" + for insteadof_value, target_base in self._rewrites: + if ( + candidate_url.startswith(insteadof_value) + and len(insteadof_value) > len(best_prefix) + ): + best_prefix = insteadof_value + best_base = target_base + if best_prefix: + return best_base + candidate_url[len(best_prefix):] + return None + + @staticmethod + def _load_rewrites() -> List[tuple]: + """Load all ``url.*.insteadof`` entries from the user's git config. + + Returns an empty list if git is missing, exits non-zero, or no + rewrites are configured. + """ + try: + result = subprocess.run( + ["git", "config", "--get-regexp", r"^url\..*\.insteadof$"], + capture_output=True, + text=True, + timeout=5, + ) + except (FileNotFoundError, OSError, subprocess.TimeoutExpired): + return [] + if result.returncode != 0 or not result.stdout.strip(): + return [] + rewrites: List[tuple] = [] + suffix = ".insteadof" + for line in result.stdout.splitlines(): + parts = line.split(None, 1) + if len(parts) != 2: + continue + key, insteadof_value = parts + key_lower = key.lower() + if not (key_lower.startswith("url.") and key_lower.endswith(suffix)): + continue + base = key[4:-len(suffix)] + if base: + rewrites.append((insteadof_value, base)) + return rewrites + + +def is_fallback_allowed(cli_flag: bool = False, env: Optional[dict] = None) -> bool: + """Return ``True`` when the user opted into cross-protocol fallback. + + Truthy via either the CLI flag or ``APM_ALLOW_PROTOCOL_FALLBACK=1``. + """ + if cli_flag: + return True + env_map = env if env is not None else os.environ + raw = env_map.get(ENV_ALLOW_FALLBACK, "") + return raw.strip().lower() in ("1", "true", "yes", "on") + + +def protocol_pref_from_env(env: Optional[dict] = None) -> ProtocolPreference: + """Read :class:`ProtocolPreference` from ``APM_GIT_PROTOCOL`` env.""" + env_map = env if env is not None else os.environ + return ProtocolPreference.from_str(env_map.get(ENV_PROTOCOL)) + + +# Internal attempt builders kept here so the selection matrix is one file. + +_AUTH_HTTPS = TransportAttempt(scheme="https", use_token=True, label="authenticated HTTPS") +_PLAIN_HTTPS = TransportAttempt(scheme="https", use_token=False, label="plain HTTPS") +_SSH = TransportAttempt(scheme="ssh", use_token=False, label="SSH") + + +class TransportSelector: + """Pure decision engine. Maps inputs to a :class:`TransportPlan`. + + The selector itself performs no network or git calls. It delegates + ``insteadOf`` discovery to an injected :class:`InsteadOfResolver`. + + Args: + insteadof_resolver: Resolver instance. Defaults to + :class:`GitConfigInsteadOfResolver` (production behavior). + Inject :class:`NoOpInsteadOfResolver` (or a fake) in tests. + """ + + def __init__(self, insteadof_resolver: Optional[InsteadOfResolver] = None) -> None: + self._resolver: InsteadOfResolver = insteadof_resolver or GitConfigInsteadOfResolver() + + def select( + self, + dep_ref, + cli_pref: ProtocolPreference = ProtocolPreference.NONE, + allow_fallback: bool = False, + has_token: bool = False, + ) -> TransportPlan: + """Compute the transport plan for ``dep_ref``. + + Args: + dep_ref: A :class:`~apm_cli.models.dependency.reference.DependencyReference`. + cli_pref: Default protocol preference for shorthand deps. + Ignored when ``dep_ref.explicit_scheme`` is set. + allow_fallback: When ``True`` cross-protocol fallback is + permitted (legacy behavior). When ``False`` (default, + strict) the plan contains exactly one attempt for explicit + URLs / pinned shorthand. + has_token: Whether an auth token is available for this dep. + Drives whether the auth-HTTPS attempt is included. + + Returns: + :class:`TransportPlan`. + """ + explicit = (getattr(dep_ref, "explicit_scheme", None) or "").lower() or None + + # 1. Explicit scheme on the URL wins for the *initial* attempt. + # In strict mode (default) the plan contains exactly that one attempt. + # With allow_fallback (escape hatch for migration), we fall through to + # the shorthand chain logic below so the legacy permissive behavior is + # preserved exactly (auth-HTTPS-if-token, SSH, plain-HTTPS). + if explicit in ("ssh", "https", "http") and not allow_fallback: + if explicit == "ssh": + initial = [_SSH] + else: + # https or http: build an HTTPS attempt either way. Plain `http://` + # URLs are normalized to HTTPS here (and never carry a token, so a + # cleartext credential leak is impossible) until #700 introduces a + # first-class HTTP transport with explicit `--allow-insecure` gating. + if explicit == "https": + initial = [_AUTH_HTTPS] if has_token else [_PLAIN_HTTPS] + else: + initial = [_PLAIN_HTTPS] + return TransportPlan( + attempts=initial, + strict=True, + fallback_hint=FALLBACK_HINT, + ) + + # 2. Shorthand (no explicit scheme), OR explicit scheme with + # allow_fallback=True (legacy behavior). Consult the CLI preference + # and git insteadOf rewrites to pick the initial protocol. + prefer_ssh = cli_pref == ProtocolPreference.SSH + prefer_https = cli_pref == ProtocolPreference.HTTPS + if cli_pref == ProtocolPreference.NONE: + # Build the candidate HTTPS URL from the dep and ask the resolver. + host = getattr(dep_ref, "host", None) or "github.com" + candidate = f"https://{host}/{getattr(dep_ref, 'repo_url', '')}" + rewrite = self._resolver.resolve(candidate) + if rewrite and not rewrite.lower().startswith(("https://", "http://")): + # Resolver mapped HTTPS -> non-HTTPS form (typically git@host:..). Prefer SSH. + prefer_ssh = True + + if prefer_ssh: + initial = [_SSH] + chained = [_AUTH_HTTPS, _PLAIN_HTTPS] if has_token else [_PLAIN_HTTPS] + elif prefer_https: + initial = [_AUTH_HTTPS] if has_token else [_PLAIN_HTTPS] + chained = [_SSH, _PLAIN_HTTPS] if has_token else [_SSH] + else: + # Strict-by-default for shorthand-no-rewrite-no-pref: HTTPS only. + initial = [_AUTH_HTTPS] if has_token else [_PLAIN_HTTPS] + chained = [_SSH, _PLAIN_HTTPS] if has_token else [_SSH] + + if not allow_fallback: + return TransportPlan( + attempts=initial, + strict=True, + fallback_hint=FALLBACK_HINT, + ) + + # Permissive: append the chain, dedup while preserving order. + seen = set() + attempts: List[TransportAttempt] = [] + for a in initial + chained: + key = (a.scheme, a.use_token) + if key in seen: + continue + seen.add(key) + attempts.append(a) + return TransportPlan(attempts=attempts, strict=False, fallback_hint=None) diff --git a/src/apm_cli/install/context.py b/src/apm_cli/install/context.py index c78cdc056..155c20f62 100644 --- a/src/apm_cli/install/context.py +++ b/src/apm_cli/install/context.py @@ -52,6 +52,8 @@ class InstallContext: verbose: bool = False dev: bool = False only_packages: Optional[List[str]] = None + protocol_pref: Any = None # ProtocolPreference (NONE/SSH/HTTPS) for shorthand transport + allow_protocol_fallback: Optional[bool] = None # None => read APM_ALLOW_PROTOCOL_FALLBACK env # ------------------------------------------------------------------ # Resolve phase outputs diff --git a/src/apm_cli/install/phases/resolve.py b/src/apm_cli/install/phases/resolve.py index 524422cd1..adce119d2 100644 --- a/src/apm_cli/install/phases/resolve.py +++ b/src/apm_cli/install/phases/resolve.py @@ -89,7 +89,11 @@ def run(ctx: "InstallContext") -> None: if ctx.auth_resolver is None: ctx.auth_resolver = AuthResolver() - downloader = _ghd_mod.GitHubPackageDownloader(auth_resolver=ctx.auth_resolver) + downloader = _ghd_mod.GitHubPackageDownloader( + auth_resolver=ctx.auth_resolver, + protocol_pref=ctx.protocol_pref, + allow_fallback=ctx.allow_protocol_fallback, + ) ctx.downloader = downloader # ------------------------------------------------------------------ diff --git a/src/apm_cli/install/pipeline.py b/src/apm_cli/install/pipeline.py index f361ae45c..7a8ba79b1 100644 --- a/src/apm_cli/install/pipeline.py +++ b/src/apm_cli/install/pipeline.py @@ -23,7 +23,7 @@ import builtins import sys -from typing import TYPE_CHECKING, List +from typing import TYPE_CHECKING, List, Optional from ..models.results import InstallResult from ..utils.console import _rich_error @@ -54,6 +54,8 @@ def run_install_pipeline( auth_resolver: "AuthResolver" = None, target: str = None, marketplace_provenance: dict = None, + protocol_pref=None, + allow_protocol_fallback: "Optional[bool]" = None, ): """Install APM package dependencies. @@ -136,6 +138,8 @@ def run_install_pipeline( auth_resolver=auth_resolver, target_override=target, marketplace_provenance=marketplace_provenance, + protocol_pref=protocol_pref, + allow_protocol_fallback=allow_protocol_fallback, all_apm_deps=all_apm_deps, root_has_local_primitives=_root_has_local_primitives, old_local_deployed=_old_local_deployed, diff --git a/src/apm_cli/install/request.py b/src/apm_cli/install/request.py index 190e72249..0d77e9b20 100644 --- a/src/apm_cli/install/request.py +++ b/src/apm_cli/install/request.py @@ -37,3 +37,5 @@ class InstallRequest: auth_resolver: Optional["AuthResolver"] = None target: Optional[str] = None marketplace_provenance: Optional[Dict[str, Any]] = None + protocol_pref: Any = None # ProtocolPreference (NONE/SSH/HTTPS) for shorthand transport + allow_protocol_fallback: Optional[bool] = None # None => read APM_ALLOW_PROTOCOL_FALLBACK env diff --git a/src/apm_cli/install/service.py b/src/apm_cli/install/service.py index 0cb1a9def..e96aeb4c0 100644 --- a/src/apm_cli/install/service.py +++ b/src/apm_cli/install/service.py @@ -72,4 +72,6 @@ def run(self, request: InstallRequest) -> "InstallResult": auth_resolver=request.auth_resolver, target=request.target, marketplace_provenance=request.marketplace_provenance, + protocol_pref=request.protocol_pref, + allow_protocol_fallback=request.allow_protocol_fallback, ) diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index fd0547942..bcdb1ffdb 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -33,6 +33,9 @@ class DependencyReference: None # Optional host (github.com, dev.azure.com, or enterprise host) ) port: Optional[int] = None # Non-standard SSH/HTTPS port (e.g. 7999 for Bitbucket DC) + explicit_scheme: Optional[str] = ( + None # User-stated transport: "ssh", "https", "http", or None for shorthand + ) reference: Optional[str] = None # e.g., "main", "v1.0.0", "abc123" alias: Optional[str] = None # Optional alias for the dependency virtual_path: Optional[str] = ( @@ -937,17 +940,25 @@ def parse(cls, dependency_str: str) -> "DependencyReference": # Phase 2: parse SSH (ssh:// URL first — it preserves port; then SCP shorthand), # otherwise fall back to HTTPS/shorthand parsing. + explicit_scheme: Optional[str] = None ssh_proto_result = cls._parse_ssh_protocol_url(dependency_str) if ssh_proto_result: host, port, repo_url, reference, alias = ssh_proto_result + explicit_scheme = "ssh" else: scp_result = cls._parse_ssh_url(dependency_str) if scp_result: host, port, repo_url, reference, alias = scp_result + explicit_scheme = "ssh" else: host, port, repo_url, reference, alias = cls._parse_standard_url( dependency_str, is_virtual_package, virtual_path, validated_host ) + _stripped = dependency_str.strip().lower() + if _stripped.startswith("https://"): + explicit_scheme = "https" + elif _stripped.startswith("http://"): + explicit_scheme = "http" # Phase 3: final validation and ADO field extraction is_ado_final = host and is_azure_devops_hostname(host) @@ -1008,6 +1019,7 @@ def parse(cls, dependency_str: str) -> "DependencyReference": repo_url=repo_url, host=host, port=port, + explicit_scheme=explicit_scheme, reference=reference, alias=alias, virtual_path=virtual_path, diff --git a/tests/fixtures/gitconfig_insteadof_to_ssh b/tests/fixtures/gitconfig_insteadof_to_ssh new file mode 100644 index 000000000..2af6ad688 --- /dev/null +++ b/tests/fixtures/gitconfig_insteadof_to_ssh @@ -0,0 +1,2 @@ +[url "git@github.com:"] + insteadOf = https://github.com/ diff --git a/tests/integration/test_transport_selection_integration.py b/tests/integration/test_transport_selection_integration.py new file mode 100644 index 000000000..febce9517 --- /dev/null +++ b/tests/integration/test_transport_selection_integration.py @@ -0,0 +1,224 @@ +"""End-to-end integration tests for Transport Selection v1 (issue #778). + +Validates the strict-by-default transport selection contract against a real +public GitHub repository (`github/awesome-copilot`). Wraps gitpython's +``Repo.clone_from`` so we can record which URLs were actually attempted by the +production downloader code path. + +Cases: + 1. Public shorthand path -- runs unconditionally with network. + 2. Explicit ``https://`` strict-by-default -- runs unconditionally with network. + 3. Explicit ``ssh://`` strict-by-default -- requires SSH key, gated. + 4. ``insteadOf`` rewrite honored for shorthand -- requires SSH key, gated. + 5. ``APM_GIT_PROTOCOL=ssh`` env -- requires SSH key, gated. + 6. ``APM_ALLOW_PROTOCOL_FALLBACK=1`` escape hatch -- requires SSH key, gated. + +Network-dependent. Skipped unless ``APM_RUN_INTEGRATION_TESTS=1``. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import tempfile +from pathlib import Path +from typing import List, Tuple +from unittest.mock import patch + +import pytest + +from apm_cli.deps.github_downloader import GitHubPackageDownloader +from apm_cli.deps.transport_selection import ( + ENV_ALLOW_FALLBACK, + ENV_PROTOCOL, + ProtocolPreference, +) +from apm_cli.models.apm_package import DependencyReference + + +pytestmark = pytest.mark.skipif( + os.environ.get("APM_RUN_INTEGRATION_TESTS") != "1", + reason="Set APM_RUN_INTEGRATION_TESTS=1 to run network-dependent tests", +) + + +_OWNER = "github" +_REPO = "awesome-copilot" + + +def _ssh_available() -> bool: + try: + result = subprocess.run( + ["ssh", "-o", "BatchMode=yes", "-o", "StrictHostKeyChecking=no", + "-T", "git@github.com"], + capture_output=True, + timeout=10, + ) + return b"successfully authenticated" in (result.stderr or b"") + except (FileNotFoundError, subprocess.TimeoutExpired): + return False + + +_REQUIRES_SSH = pytest.mark.skipif( + not _ssh_available(), + reason="No usable SSH key for git@github.com", +) + + +@pytest.fixture +def tmp_clone_dir(): + d = Path(tempfile.mkdtemp(prefix="apm-transport-it-")) + try: + yield d + finally: + shutil.rmtree(d, ignore_errors=True) + + +@pytest.fixture +def isolated_env(monkeypatch): + for var in (ENV_PROTOCOL, ENV_ALLOW_FALLBACK): + monkeypatch.delenv(var, raising=False) + yield monkeypatch + + +def _attempt_clone( + spec: str, + *, + target_dir: Path, + protocol_pref: ProtocolPreference = ProtocolPreference.NONE, + allow_fallback: bool = False, +) -> Tuple[bool, List[str]]: + """Drive the production clone path and capture the URLs attempted.""" + dep = DependencyReference.parse(spec) + captured: List[str] = [] + + from git import Repo as _Repo + + real_clone_from = _Repo.clone_from + + def _record(url, to_path, *args, **kwargs): + captured.append(url) + return real_clone_from(url, to_path, *args, **kwargs) + + dl = GitHubPackageDownloader( + protocol_pref=protocol_pref, + allow_fallback=allow_fallback, + ) + + with patch("apm_cli.deps.github_downloader.Repo.clone_from", side_effect=_record): + try: + dl._clone_with_fallback( + repo_url_base=dep.repo_url, + target_path=target_dir, + dep_ref=dep, + ) + return True, captured + except Exception: + return False, captured + + +# --------------------------------------------------------------------------- +# Always-on cases (no SSH key required) +# --------------------------------------------------------------------------- + + +class TestPublicShorthandPath: + def test_https_clone_succeeds_for_shorthand(self, tmp_clone_dir, isolated_env): + ok, urls = _attempt_clone(f"{_OWNER}/{_REPO}", target_dir=tmp_clone_dir / "c") + assert ok, f"clone failed; URLs tried: {urls}" + assert any(u.startswith("https://") for u in urls) + assert not any( + u.startswith("git@") or u.startswith("ssh://") for u in urls + ), "shorthand-default must not silently try SSH; URLs tried: %s" % urls + + +class TestExplicitHttpsStrict: + def test_explicit_https_clones_no_ssh_attempt(self, tmp_clone_dir, isolated_env): + ok, urls = _attempt_clone( + f"https://github.com/{_OWNER}/{_REPO}.git", + target_dir=tmp_clone_dir / "c", + ) + assert ok + assert all(u.startswith("https://") for u in urls), urls + + +# --------------------------------------------------------------------------- +# SSH-required cases +# --------------------------------------------------------------------------- + + +@_REQUIRES_SSH +class TestExplicitSshStrict: + def test_explicit_ssh_clones_no_https_fallback(self, tmp_clone_dir, isolated_env): + ok, urls = _attempt_clone( + f"ssh://git@github.com/{_OWNER}/{_REPO}.git", + target_dir=tmp_clone_dir / "c", + ) + assert ok + assert all( + (u.startswith("git@") or u.startswith("ssh://")) for u in urls + ), urls + + def test_explicit_ssh_bad_host_does_not_fall_back(self, tmp_clone_dir, isolated_env): + ok, urls = _attempt_clone( + "ssh://git@nonexistent-host-apm-test.invalid/foo/bar.git", + target_dir=tmp_clone_dir / "c", + ) + assert not ok + assert all(not u.startswith("https://") for u in urls), urls + + +@_REQUIRES_SSH +class TestInsteadOfHonored: + def test_gitconfig_insteadof_makes_shorthand_use_ssh( + self, tmp_clone_dir, isolated_env, monkeypatch + ): + fixture_home = Path(tempfile.mkdtemp(prefix="apm-it-home-")) + try: + shutil.copy( + Path(__file__).parent.parent / "fixtures" / "gitconfig_insteadof_to_ssh", + fixture_home / ".gitconfig", + ) + monkeypatch.setenv("HOME", str(fixture_home)) + ok, urls = _attempt_clone( + f"{_OWNER}/{_REPO}", target_dir=tmp_clone_dir / "c" + ) + assert ok + assert urls and ( + urls[0].startswith("git@") or urls[0].startswith("ssh://") + ), urls + finally: + shutil.rmtree(fixture_home, ignore_errors=True) + + +@_REQUIRES_SSH +class TestEnvProtocolOverride: + def test_env_apm_git_protocol_ssh_picks_ssh_for_shorthand( + self, tmp_clone_dir, isolated_env + ): + isolated_env.setenv(ENV_PROTOCOL, "ssh") + ok, urls = _attempt_clone( + f"{_OWNER}/{_REPO}", + target_dir=tmp_clone_dir / "c", + protocol_pref=ProtocolPreference.SSH, + ) + assert ok + assert urls and ( + urls[0].startswith("git@") or urls[0].startswith("ssh://") + ), urls + + +@_REQUIRES_SSH +class TestAllowFallbackEscapeHatch: + def test_explicit_ssh_with_allow_fallback_can_reach_https( + self, tmp_clone_dir, isolated_env + ): + ok, urls = _attempt_clone( + "ssh://git@nonexistent-host-apm-test.invalid/foo/bar.git", + target_dir=tmp_clone_dir / "c", + allow_fallback=True, + ) + assert any(u.startswith("https://") for u in urls), ( + "allow_fallback must permit cross-protocol retry; URLs tried: %s" % urls + ) diff --git a/tests/unit/test_auth_scoping.py b/tests/unit/test_auth_scoping.py index c231291de..304001962 100644 --- a/tests/unit/test_auth_scoping.py +++ b/tests/unit/test_auth_scoping.py @@ -165,15 +165,15 @@ def _run_clone(self, dl, dep, succeed_on=1): return MockRepo.clone_from.call_args_list def test_generic_host_env_allows_credential_helpers(self): - """For GitLab/Bitbucket, GIT_ASKPASS / GIT_CONFIG_GLOBAL are NOT set.""" + """For GitLab/Bitbucket without token, GIT_ASKPASS / GIT_CONFIG_GLOBAL are NOT set.""" dl = _make_downloader(github_token="ghp_TESTTOKEN") dep = _dep("https://gitlab.com/acme/rules.git") calls = self._run_clone(dl, dep, succeed_on=1) assert len(calls) >= 1 - # First call should be SSH (no token for generic), check its env - # Actually for generic: no token → skip method 1, go to method 2 (SSH) + # Under strict default, explicit https:// -> single HTTPS attempt; env is relaxed + # because no token was available for the generic host. env_used = calls[0][1].get("env", calls[0].kwargs.get("env")) assert "GIT_ASKPASS" not in env_used assert "GIT_CONFIG_GLOBAL" not in env_used @@ -213,15 +213,34 @@ def test_github_host_no_token_allows_credential_helpers(self): assert "GIT_CONFIG_NOSYSTEM" not in env_used assert env_used.get("GIT_TERMINAL_PROMPT") == "0" - def test_generic_host_no_token_skips_method1(self): - """Generic hosts have no token → Method 1 (auth HTTPS) is skipped.""" + def test_generic_host_explicit_https_strict_no_ssh_fallback(self): + """Explicit https:// URL no longer silently falls back to SSH (issue #661).""" dl = _make_downloader(github_token="ghp_TESTTOKEN") dep = _dep("https://gitlab.com/acme/rules.git") - calls = self._run_clone(dl, dep, succeed_on=1) - # Should only attempt SSH (method 2) first, since no token for generic - first_url = calls[0][0][0] - assert "git@" in first_url or "ssh://" in first_url + # All clone attempts fail; verify only HTTPS is attempted (no SSH). + calls = self._run_clone(dl, dep, succeed_on=99) + for call in calls: + url = call[0][0] + assert "git@" not in url, f"explicit https:// must not fall back to SSH, got {url}" + assert not url.startswith("ssh://"), f"explicit https:// must not fall back to ssh://, got {url}" + + def test_generic_host_legacy_chain_with_allow_fallback(self): + """APM_ALLOW_PROTOCOL_FALLBACK=1 restores cross-protocol fallback so + clones that succeeded under the legacy chain still succeed.""" + dl = _make_downloader(github_token="ghp_TESTTOKEN") + # Force the downloader to permit fallback regardless of env state. + dl._allow_fallback = True + dep = _dep("https://gitlab.com/acme/rules.git") + + # All clone attempts fail; verify both HTTPS and SSH were attempted. + calls = self._run_clone(dl, dep, succeed_on=99) + urls = [c[0][0] for c in calls] + has_https = any(u.startswith("https://") for u in urls) + has_ssh = any("git@" in u or u.startswith("ssh://") for u in urls) + assert has_https and has_ssh, ( + f"allow_fallback should attempt both HTTPS and SSH, got urls={urls}" + ) def test_github_host_with_token_tries_method1_first(self): """GitHub with a token → Method 1 (auth HTTPS) is tried first.""" @@ -268,6 +287,74 @@ def test_clone_env_includes_ssh_connect_timeout(self): assert "GIT_SSH_COMMAND" in relaxed assert "ConnectTimeout" in relaxed["GIT_SSH_COMMAND"] + def test_allow_fallback_env_is_per_attempt_not_per_dep(self): + """In a mixed allow_fallback plan, only token-bearing attempts get the + locked-down env; SSH and plain-HTTPS attempts get the relaxed env so + user credential helpers (gh auth, Keychain, ssh-agent) keep working. + + Regression for the Wave 2 panel finding: previously the env was decided + once per dependency from `has_token`, so SSH attempts in a token-having + dep ran with `GIT_ASKPASS=echo` and `GIT_CONFIG_GLOBAL=/dev/null`, + breaking encrypted-key prompts and credential helpers. + """ + dl = _make_downloader(github_token="ghp_TESTTOKEN") + dl._allow_fallback = True + dep = _dep("owner/repo") + + mock_repo = Mock() + mock_repo.head.commit.hexsha = "abc123" + # Fail every attempt so we capture envs from all of them. + effects = [GitCommandError("clone", "failed")] * 5 + + with patch.dict(os.environ, {"GITHUB_APM_PAT": "ghp_TESTTOKEN"}, clear=True), \ + patch("apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git", + return_value=None), \ + patch('apm_cli.deps.github_downloader.Repo') as MockRepo: + MockRepo.clone_from.side_effect = effects + dl.auth_resolver._cache.clear() + target = Path(tempfile.mkdtemp()) + try: + with pytest.raises(RuntimeError): + dl._clone_with_fallback(dep.repo_url, target, dep_ref=dep) + finally: + import shutil + shutil.rmtree(target, ignore_errors=True) + calls = MockRepo.clone_from.call_args_list + + # Map URL -> env used. Token-bearing attempts must get locked-down env; + # SSH attempts and plain-HTTPS must get the relaxed env. Plain-HTTPS + # must NOT embed the token in the URL. + assert len(calls) >= 2, f"expected mixed chain, got {len(calls)} attempts" + seen_auth_https = False + seen_plain_https = False + seen_ssh = False + for c in calls: + url = c[0][0] + env_used = c[1].get("env", {}) + if url.startswith("git@") or url.startswith("ssh://"): + seen_ssh = True + assert "GIT_ASKPASS" not in env_used, ( + f"SSH URL must run with relaxed env; got env={env_used}" + ) + assert "GIT_CONFIG_GLOBAL" not in env_used + elif "ghp_TESTTOKEN" in url: + seen_auth_https = True + assert env_used.get("GIT_ASKPASS") == "echo", ( + f"token URL must run with locked-down env; got env={env_used}" + ) + else: + # Plain HTTPS attempt: must NOT embed the token, and must run + # with relaxed env so credential helpers (gh auth, Keychain) + # can supply credentials. + seen_plain_https = True + assert url.startswith("https://"), url + assert "GIT_ASKPASS" not in env_used, ( + f"plain HTTPS must run with relaxed env; got env={env_used}" + ) + assert seen_auth_https and seen_ssh and seen_plain_https, ( + "expected at least one of each attempt kind in the mixed chain" + ) + # =========================================================================== # Regression: ssh:// URLs with custom ports (issues #661, #731) @@ -282,15 +369,17 @@ class TestCloneWithFallbackPortPreservation: SSH and HTTPS URL builders so the port is never silently dropped. """ - def _run_clone_capture_urls(self, dep, clone_fails=True): + def _run_clone_capture_urls(self, dep, clone_fails=True, allow_fallback=False): """Run _clone_with_fallback and return every URL passed to clone_from. When clone_fails is True, all clone attempts raise GitCommandError, - letting the fallback chain exercise both SSH and HTTPS attempts so - we can capture every emitted URL. + letting the fallback chain (when permitted) exercise both SSH and + HTTPS attempts so we can capture every emitted URL. """ dl = _make_downloader() dl.auth_resolver._cache.clear() + if allow_fallback: + dl._allow_fallback = True called_urls = [] @@ -332,10 +421,12 @@ def test_ssh_attempt_uses_port_when_dep_ref_has_port(self): ) def test_https_attempt_preserves_same_port_across_protocols(self): - """Method 3 (HTTPS) must emit https://host:7999/... — same port as SSH.""" + """Under allow_fallback (legacy chain), HTTPS attempt must emit https://host:7999/... — + same port as SSH. Port preservation across protocols must hold whenever the + chain runs, even though strict default never reaches the HTTPS attempt.""" dep = _dep("ssh://git@bitbucket.example.com:7999/project/repo.git") - urls = self._run_clone_capture_urls(dep) + urls = self._run_clone_capture_urls(dep, allow_fallback=True) https_urls = [u for u in urls if u.startswith("https://")] assert https_urls, f"no https:// fallback attempted, got: {urls!r}" parsed = urlparse(https_urls[0]) diff --git a/tests/unit/test_transport_selection.py b/tests/unit/test_transport_selection.py new file mode 100644 index 000000000..05a08c6dd --- /dev/null +++ b/tests/unit/test_transport_selection.py @@ -0,0 +1,335 @@ +"""Unit tests for src/apm_cli/deps/transport_selection.py. + +Covers the selection matrix from issue microsoft/apm#778: + +* explicit ssh:// URLs are strict (no HTTPS fallback) -- closes #661 +* explicit https:// URLs are strict (no SSH fallback) +* shorthand consults git insteadOf rewrites and prefers SSH on rewrite hits + -- closes #328 +* shorthand defaults to HTTPS-only when no rewrite, no CLI pref, no env +* CLI / env overrides for shorthand: --ssh / --https, APM_GIT_PROTOCOL +* APM_ALLOW_PROTOCOL_FALLBACK / --allow-protocol-fallback restores the + legacy permissive cross-protocol chain so today's clones still succeed +* token presence drives whether auth-HTTPS is in the plan +* per-instance cache: insteadOf rewrites are loaded once +""" + +from __future__ import annotations + +from typing import Dict, List, Optional +from unittest.mock import patch + +import pytest + +from apm_cli.deps.transport_selection import ( + ENV_ALLOW_FALLBACK, + ENV_PROTOCOL, + FALLBACK_HINT, + GitConfigInsteadOfResolver, + InsteadOfResolver, + NoOpInsteadOfResolver, + ProtocolPreference, + TransportAttempt, + TransportPlan, + TransportSelector, + is_fallback_allowed, + protocol_pref_from_env, +) +from apm_cli.models.dependency.reference import DependencyReference + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +class FakeInsteadOfResolver: + """Test double for ``InsteadOfResolver``. + + Constructed with a dict ``{candidate_prefix: replacement_prefix}``; the + ``resolve`` call returns the rewritten URL when a prefix matches, mirroring + git's ``url..insteadof`` semantics. + """ + + def __init__(self, rewrites: Optional[Dict[str, str]] = None): + self._rewrites = rewrites or {} + self.calls: List[str] = [] + + def resolve(self, candidate_url: str) -> Optional[str]: + self.calls.append(candidate_url) + for prefix, replacement in self._rewrites.items(): + if candidate_url.startswith(prefix): + return replacement + candidate_url[len(prefix):] + return None + + +def _dep(spec: str) -> DependencyReference: + return DependencyReference.parse(spec) + + +def _scheme_labels(plan: TransportPlan) -> List[str]: + return [a.scheme for a in plan.attempts] + + +# --------------------------------------------------------------------------- +# Selection matrix +# --------------------------------------------------------------------------- + + +class TestExplicitSchemeStrict: + """Explicit ssh:// / https:// URLs must not silently change protocol.""" + + def test_explicit_ssh_strict_single_attempt(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("ssh://git@github.com/owner/repo.git"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=False, + has_token=True, + ) + assert plan.strict is True + assert _scheme_labels(plan) == ["ssh"] + assert plan.fallback_hint == FALLBACK_HINT + + def test_explicit_https_with_token_uses_auth_https(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("https://github.com/owner/repo.git"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=False, + has_token=True, + ) + assert plan.strict is True + assert _scheme_labels(plan) == ["https"] + assert plan.attempts[0].use_token is True + + def test_explicit_https_without_token_uses_plain_https(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("https://gitlab.com/acme/lib.git"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=False, + has_token=False, + ) + assert plan.strict is True + assert _scheme_labels(plan) == ["https"] + assert plan.attempts[0].use_token is False + + def test_explicit_ssh_ignores_cli_pref_https(self): + """User-stated scheme on the dep wins over CLI default for shorthand.""" + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("ssh://git@github.com/owner/repo.git"), + cli_pref=ProtocolPreference.HTTPS, + allow_fallback=False, + has_token=True, + ) + assert _scheme_labels(plan) == ["ssh"] + assert plan.strict is True + + +class TestShorthandWithInsteadOf: + """`git config url..insteadof` must be honored for shorthand deps.""" + + def test_insteadof_https_to_ssh_prefers_ssh(self): + rewrites = {"https://github.com/": "git@github.com:"} + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver(rewrites)) + plan = sel.select( + dep_ref=_dep("owner/repo"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=False, + has_token=True, + ) + assert _scheme_labels(plan) == ["ssh"] + assert plan.strict is True + + def test_no_insteadof_defaults_to_https_strict(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("owner/repo"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=False, + has_token=True, + ) + assert _scheme_labels(plan) == ["https"] + assert plan.attempts[0].use_token is True + assert plan.strict is True + + def test_no_insteadof_no_token_defaults_to_plain_https(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("owner/repo"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=False, + has_token=False, + ) + assert _scheme_labels(plan) == ["https"] + assert plan.attempts[0].use_token is False + assert plan.strict is True + + +class TestCliPreferences: + """--ssh / --https flags steer shorthand transport selection.""" + + def test_cli_pref_ssh_for_shorthand(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("owner/repo"), + cli_pref=ProtocolPreference.SSH, + allow_fallback=False, + has_token=True, + ) + assert plan.attempts[0].scheme == "ssh" + + def test_cli_pref_https_for_shorthand(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("owner/repo"), + cli_pref=ProtocolPreference.HTTPS, + allow_fallback=False, + has_token=True, + ) + assert plan.attempts[0].scheme == "https" + assert plan.attempts[0].use_token is True + + def test_cli_pref_does_not_override_explicit_scheme(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("https://github.com/owner/repo.git"), + cli_pref=ProtocolPreference.SSH, + allow_fallback=False, + has_token=True, + ) + assert _scheme_labels(plan) == ["https"] + + +class TestAllowFallback: + """allow_fallback=True restores the legacy permissive cross-protocol chain.""" + + def test_shorthand_with_token_legacy_chain(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("owner/repo"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=True, + has_token=True, + ) + # legacy default for shorthand: auth-HTTPS, then SSH, then plain-HTTPS + schemes = _scheme_labels(plan) + assert "https" in schemes and "ssh" in schemes + assert plan.strict is False + + def test_shorthand_without_token_legacy_chain(self): + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("owner/repo"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=True, + has_token=False, + ) + schemes = _scheme_labels(plan) + assert "ssh" in schemes and "https" in schemes + assert plan.strict is False + + def test_explicit_https_with_allow_fallback_runs_chain(self): + """Migration safety: clones that succeeded under the legacy chain still succeed.""" + sel = TransportSelector(insteadof_resolver=FakeInsteadOfResolver()) + plan = sel.select( + dep_ref=_dep("https://gitlab.com/acme/lib.git"), + cli_pref=ProtocolPreference.NONE, + allow_fallback=True, + has_token=False, + ) + schemes = _scheme_labels(plan) + assert "ssh" in schemes and "https" in schemes + assert plan.strict is False + + +# --------------------------------------------------------------------------- +# Helper functions +# --------------------------------------------------------------------------- + + +class TestProtocolPrefFromEnv: + @pytest.mark.parametrize( + "value,expected", + [ + ("ssh", ProtocolPreference.SSH), + ("SSH", ProtocolPreference.SSH), + ("https", ProtocolPreference.HTTPS), + ("HTTPS", ProtocolPreference.HTTPS), + ("", ProtocolPreference.NONE), + ("auto", ProtocolPreference.NONE), + ("garbage", ProtocolPreference.NONE), + ], + ) + def test_env_value(self, value, expected, monkeypatch): + if value: + monkeypatch.setenv(ENV_PROTOCOL, value) + else: + monkeypatch.delenv(ENV_PROTOCOL, raising=False) + assert protocol_pref_from_env() is expected + + +class TestIsFallbackAllowed: + @pytest.mark.parametrize( + "value,expected", + [ + ("1", True), + ("true", True), + ("yes", True), + ("on", True), + ("0", False), + ("false", False), + ("", False), + ], + ) + def test_env_value(self, value, expected, monkeypatch): + if value: + monkeypatch.setenv(ENV_ALLOW_FALLBACK, value) + else: + monkeypatch.delenv(ENV_ALLOW_FALLBACK, raising=False) + assert is_fallback_allowed() is expected + + +# --------------------------------------------------------------------------- +# Resolver caching +# --------------------------------------------------------------------------- + + +class TestGitConfigInsteadOfResolver: + def test_lookup_cached_per_instance(self): + """`git config --get-regexp` is shelled out at most once per instance.""" + resolver = GitConfigInsteadOfResolver() + with patch("apm_cli.deps.transport_selection.subprocess.run") as run: + # Simulate one rewrite rule: https://github.com/ -> git@github.com: + run.return_value.returncode = 0 + run.return_value.stdout = "url.git@github.com:.insteadof https://github.com/\n" + resolver.resolve("https://github.com/owner/repo") + resolver.resolve("https://github.com/other/proj") + resolver.resolve("https://gitlab.com/acme/lib") + assert run.call_count == 1 + + def test_uses_normal_env_not_locked_down(self): + """The resolver MUST use the process env so user .gitconfig is visible. + + The downloader's locked-down git_env sets GIT_CONFIG_GLOBAL=/dev/null, + which would suppress user insteadOf rewrites. Issue #328 stays broken + unless the resolver runs with the normal env (no env= override). + """ + resolver = GitConfigInsteadOfResolver() + with patch("apm_cli.deps.transport_selection.subprocess.run") as run: + run.return_value.returncode = 0 + run.return_value.stdout = "" + resolver.resolve("https://github.com/owner/repo") + _args, kwargs = run.call_args + # subprocess.run must be called WITHOUT env override so the + # user's normal git config is visible. + assert "env" not in kwargs or kwargs["env"] is None + + def test_resolve_returns_none_when_no_rewrites(self): + resolver = GitConfigInsteadOfResolver() + with patch("apm_cli.deps.transport_selection.subprocess.run") as run: + run.return_value.returncode = 0 + run.return_value.stdout = "" + assert resolver.resolve("https://github.com/owner/repo") is None