diff --git a/.apm/architecture/owners/transport-auth-platform.json b/.apm/architecture/owners/transport-auth-platform.json index 5cfce7a032..82e77772ca 100644 --- a/.apm/architecture/owners/transport-auth-platform.json +++ b/.apm/architecture/owners/transport-auth-platform.json @@ -1,6 +1,16 @@ { "version": 1, "owners": [ + { + "id": "git-transport-selection", + "decision": "Initial Git scheme, ordered protocol attempts, and GitLab sparse-fetch plan consumption", + "owner": "deps/transport_selection.py (initial_transport_scheme, TransportSelector); deps/download_strategies.py (DownloadDelegate.download_gitlab_file)", + "selectors": [ + "src/apm_cli/deps/transport_selection.py", + "src/apm_cli/deps/download_strategies.py" + ], + "guards": ["transport-platform-gitlab-sparse-plan"] + }, { "id": "unix-install-ownership", "decision": "Unix installer destinations, prior-install ownership, native shell setup receipt and unprivileged replacement", diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ef4721095..475b5a0480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The shared gh-aw APM pack job now declares `contents: read` (previously `permissions: {}`), the minimum the explicit built-in-token path needs. No write scope is added, and the token is not forwarded to restore or agent jobs. (#2706) - Dependency policy `allow`, `deny`, and exact `require` matching now follows canonical owner/repository casing, fixing mixed-case blocks and deny fail-open behavior while retaining lazy shared required-package lookup. APM 0.30.0 and earlier match patterns byte-exactly against the lowercased identity; lowercase patterns keep matching in every release, so drop workaround duplicates only after every runner uses a release carrying this fix. (#2706) +### Fixed + +- GitLab `path:` dependencies now preserve the selected SSH transport, username, and port instead of silently using HTTPS; REST fallback requires an executed same-origin HTTPS attempt admitted by the transport policy. (#2938) + ## [0.30.0] - 2026-09-07 ### Security diff --git a/CONFORMANCE.json b/CONFORMANCE.json index b2fbc74e4d..5c15c0cc99 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -1056,8 +1056,11 @@ "keyword": "MUST", "section": "7.2", "status": "active", - "test_count": 7, + "test_count": 10, "tests": [ + "tests/spec_conformance/test_gitlab_sparse_transport_reqs.py::test_gitlab_sparse_fetch_preserves_port_identity_and_ref[main]", + "tests/spec_conformance/test_gitlab_sparse_transport_reqs.py::test_gitlab_sparse_fetch_preserves_port_identity_and_ref[sha]", + "tests/spec_conformance/test_gitlab_sparse_transport_reqs.py::test_gitlab_sparse_fetch_preserves_port_identity_and_ref[v1]", "tests/spec_conformance/test_resolution_reqs.py::test_repository_identity_isolates_l0_cache_across_hosts", "tests/spec_conformance/test_resolution_reqs.py::test_repository_identity_isolates_same_host_nested_repositories_through_resolver", "tests/spec_conformance/test_resolution_reqs.py::test_repository_identity_normalizes_safe_syntax_dimensions", @@ -1231,8 +1234,11 @@ "keyword": "MUST", "section": "10.3", "status": "active", - "test_count": 1, + "test_count": 4, "tests": [ + "tests/spec_conformance/test_gitlab_sparse_transport_reqs.py::test_gitlab_sparse_fetch_preserves_port_identity_and_ref[main]", + "tests/spec_conformance/test_gitlab_sparse_transport_reqs.py::test_gitlab_sparse_fetch_preserves_port_identity_and_ref[sha]", + "tests/spec_conformance/test_gitlab_sparse_transport_reqs.py::test_gitlab_sparse_fetch_preserves_port_identity_and_ref[v1]", "tests/spec_conformance/test_manifest_reqs.py::test_configured_host_class_precedence_is_credential_isolated" ] }, diff --git a/CONFORMANCE.md b/CONFORMANCE.md index a6c0ce79fd..8359816d0b 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -128,7 +128,7 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | [req-rs-013](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-013) | MUST | 7.2 | consumer | active | 1 | - | | [req-rs-014](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-014) | MUST | 7.3.1 | consumer | active | 1 | - | | [req-rs-015](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-015) | MUST | 7.5 | consumer | active | 1 | - | -| [req-rs-016](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-016) | MUST | 7.2 | consumer | active | 7 | - | +| [req-rs-016](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-016) | MUST | 7.2 | consumer | active | 10 | - | | [req-rs-017](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-017) | MUST | 7.7 | consumer | active | 15 | - | | [req-sc-001](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-001) | MUST | 10.4 | consumer | active | 2 | - | | [req-sc-002](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-002) | MUST | 10.9 | consumer | active | 1 | - | @@ -142,7 +142,7 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | [req-sc-010](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-010) | MUST | 10.13 | consumer | active | 1 | - | | [req-sc-011](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-011) | MUST | 10.14 | consumer | active | 1 | - | | [req-sc-012](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-012) | MUST | 10.14 | consumer | active | 1 | - | -| [req-sc-013](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-013) | MUST | 10.3 | consumer | active | 1 | - | +| [req-sc-013](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-013) | MUST | 10.3 | consumer | active | 4 | - | | [req-sc-014](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-014) | MUST | 10.15 | consumer | active | 1 | - | | [req-sc-015](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-015) | MUST | 10.16 | consumer | active | 3 | tests/fixtures/spec-conformance/source-plan/req-sc-015.json | | [req-tg-001](docs/src/content/docs/specs/openapm-v0.1.md#req-tg-001) | MUST | 8.4 | consumer | active | 1 | - | diff --git a/docs/src/content/docs/consumer/authentication.md b/docs/src/content/docs/consumer/authentication.md index dcfdb87535..04debec55f 100644 --- a/docs/src/content/docs/consumer/authentication.md +++ b/docs/src/content/docs/consumer/authentication.md @@ -58,29 +58,35 @@ key non-interactively or use token-backed HTTPS. ## GitLab (SaaS or self-managed) -**If `git clone` works, `apm install` works** -- no token is needed for GitLab `path:` files. - -APM fetches `path:`-specified files from GitLab dependencies via git -sparse/partial checkout (the same transport used for the clone), so your -existing SSH keys and git credential helpers work without any extra token. -This is the default for all GitLab sources, including self-hosted instances -where the REST API is restricted or returns 410 -- if `git clone` works, so -does `apm install`. For self-hosted hosts, explicit `git:` / SSH URLs carry -the host in the dependency. Set `GITLAB_HOST` (or `APM_GITLAB_HOSTS`) only -when you want bare-host or shorthand forms to classify as GitLab. -If you need to fall back to the GitLab REST API (for environments where git -transport is not available), set `GITLAB_APM_PAT`: +GitLab `path:` single-file fetches use sparse/partial Git checkout and the +same [transport policy](../manage-dependencies/#transport-selection) as clones. +SSH keys and Git credential helpers work without an extra token, even when +the REST API is disabled. + +In strict mode, APM passes the selected SSH/SCP URL to Git, preserving its +user, host, port, and requested ref without choosing another protocol. +Safe Git `insteadOf` rewrites still apply. If the effective transport remains +SSH, failure never triggers REST, even with a PAT available. Fix SSH access +or explicitly declare the HTTPS web endpoint; APM does not map SSH aliases +to web hostnames. + +REST fallback runs only after the selected Git plan is exhausted and an +executed attempt used effective HTTPS with the same normalized scheme, +host, and port as the API endpoint. Default HTTPS fallback remains supported. +HTTP is not automatically upgraded to HTTPS; an HTTPS URL rewritten by Git +to SSH or a local mirror does not authorize REST. + +For token-backed HTTPS: ```bash export GITLAB_APM_PAT=glpat_your_token apm install ``` -Use a project- or group-scoped token with **read_repository** scope. Self-managed GitLab works with the same env var; APM resolves the host from the dependency URL. - -If you have configured a git credential helper for GitLab (e.g. `git credential-manager` on Windows / macOS), APM falls back to it after the env-var lookup -- you do not need `GITLAB_APM_PAT` if `git clone https://gitlab.com//` already prompts you once and caches. - -`GITLAB_TOKEN` is also accepted as a lower-precedence fallback for compatibility with CI environments that already set it. +Use **read_repository** scope. APM checks `GITLAB_APM_PAT`, then +`GITLAB_TOKEN`, then the Git credential helper for trusted GitLab hosts. +For self-managed host registration and token trust, see +[GitLab authentication](../../getting-started/authentication/#gitlab-saas-and-self-managed). ## Azure DevOps diff --git a/docs/src/content/docs/consumer/manage-dependencies.md b/docs/src/content/docs/consumer/manage-dependencies.md index fa06637c1c..1938e87cf8 100644 --- a/docs/src/content/docs/consumer/manage-dependencies.md +++ b/docs/src/content/docs/consumer/manage-dependencies.md @@ -152,9 +152,8 @@ the HTTP file-read path. If a private host fails with 401/403, use a whole-repo git dependency for full clone auth support, or choose the supported HTTP backend signal (`type: gitlab` for GitLab-compatible hosts, `GITHUB_HOST` for GHES). -**GitLab `path:` fetch transport:** GitLab `path:` files are fetched over -git transport (not the REST API), so self-hosted instances with the API disabled -still install. See [Authentication](../authentication/#gitlab-saas-or-self-managed). +**GitLab `path:` files** use Git first, with +[restricted REST fallback](../authentication/#gitlab-saas-or-self-managed). For private repos and non-GitHub hosts, see [Private and org packages](../private-and-org-packages/). @@ -209,12 +208,13 @@ Git-hook isolation guarantee. ## Transport selection -APM selects one initial transport per dependency. Git then applies any matching -safe `url..insteadOf` rule to that selected URL. +APM selects one initial transport per dependency, including GitLab `path:` +single-file sparse fetches. Git then applies matching safe +`url..insteadOf` rules. | Dependency form | Initial transport | |---|---| -| `ssh://...` or `git@host:...` | SSH | +| `ssh://...` or `user@host:...` (SCP-style) | SSH | | `https://...` or `http://...` | The explicit HTTP(S) scheme | | Shorthand with `--ssh`, `APM_GIT_PROTOCOL=ssh`, or saved `prefer-ssh` | SSH | | Other shorthand | HTTPS | @@ -232,6 +232,13 @@ preference with `apm config set prefer-ssh true`, or save the retry escape hatch with `apm config set allow-protocol-fallback true`. See the [`apm config` reference](../../reference/cli/config/). +Opt-in SSH/HTTPS fallback warns when a failed attempt switches protocol. +It reuses the declared custom port and warns about that port once; it does +not map an SSH alias to a web hostname. If protocols use different endpoints, +declare the intended URL instead. GitLab REST additionally requires an +executed same-origin HTTPS attempt; see +[GitLab authentication](../authentication/#gitlab-saas-or-self-managed). + If Git reports an HTTPS `Failed to connect...` / `Couldn't connect to server` error for the requested remote, APM retries that Git action once after 1 second with the same URL, credentials, and transport. A persistent connection failure diff --git a/docs/src/content/docs/getting-started/authentication.md b/docs/src/content/docs/getting-started/authentication.md index 96c9da447a..5d89c3ff31 100644 --- a/docs/src/content/docs/getting-started/authentication.md +++ b/docs/src/content/docs/getting-started/authentication.md @@ -387,6 +387,12 @@ hostname, use object form instead of a hostname convention: For `gitlab.com` and hosts explicitly trusted through `GITLAB_HOST` or `APM_GITLAB_HOSTS`, credentials follow **`GITLAB_APM_PAT` → `GITLAB_TOKEN`** and then **`git credential fill`** (see [GitLab-class hosts](#gitlab-class-hosts-gitlabcom-gitlab_host-apm_gitlab_hosts) under [Token lookup](#token-lookup)). `type: gitlab` selects backend/API routing only; other hinted hosts use host-scoped `git credential fill` or anonymous access and do not receive global GitLab tokens. GitHub PAT env vars are not used on GitLab. Use a GitLab personal or project access token with API read access where your policy requires it. +For GitLab `path:` sparse fetches, APM consults this credential chain only +when the effective Git remote uses HTTPS. SSH, HTTP, and local-mirror +attempts use their native transport policy without PATs or HTTPS credential +helper lookup. Safe Git `insteadOf` rewrites still apply; see the +[GitLab sparse-fetch policy](../../consumer/authentication/#gitlab-saas-or-self-managed). + ### REST headers (GitLab vs GitHub) For GitHub and GHES, APM sends repository API requests with `Authorization: token ` (or equivalent). For **GitLab REST v4**, PATs are sent with the **`PRIVATE-TOKEN`** header (GitLab’s convention). OAuth-style access tokens can use `Authorization: Bearer` when applicable. APM does not log token values. @@ -399,7 +405,7 @@ For GitHub and GHES, APM sends repository API requests with `Authorization: toke | `github.com/org/repo` | github.com | Global env vars -> `gh auth token` -> credential fill | Unauth for public repos | | `contoso.ghe.com/org/repo` | *.ghe.com | Global env vars -> `gh auth token` -> credential fill | Auth-only (no public repos) | | GHES via `GITHUB_HOST` | ghes.company.com | Global env vars -> `gh auth token` -> credential fill | Unauth for public repos | -| GitLab (`gitlab.com` or host listed in `GITLAB_HOST` / `APM_GITLAB_HOSTS`) | gitlab.com or self-managed | `GITLAB_APM_PAT` -> `GITLAB_TOKEN` -> credential helper; REST uses `PRIVATE-TOKEN`; GitHub env vars excluded | Unauth where the instance allows it | +| GitLab (`gitlab.com` or host listed in `GITLAB_HOST` / `APM_GITLAB_HOSTS`) | gitlab.com or self-managed | HTTPS/API: `GITLAB_APM_PAT` -> `GITLAB_TOKEN` -> credential helper; REST uses `PRIVATE-TOKEN`; SSH uses native SSH auth; GitHub env vars excluded | Sparse-fetch REST requires exhausted same-origin effective HTTPS; otherwise native transport access | | `dev.azure.com/org/proj/repo` | ADO (cloud) | `ADO_APM_PAT` -> AAD bearer via `az` | Auth-only | | ADO Server via `ADO_HOST` / `APM_ADO_HOSTS` | on-prem ADO | `ADO_APM_PAT` only | Auth-only | | Artifactory registry proxy | custom FQDN | `PROXY_REGISTRY_TOKEN` | Error if `PROXY_REGISTRY_ONLY=1` | diff --git a/packages/apm-guide/.apm/skills/apm-usage/authentication.md b/packages/apm-guide/.apm/skills/apm-usage/authentication.md index e9792fc74c..052682704b 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/authentication.md +++ b/packages/apm-guide/.apm/skills/apm-usage/authentication.md @@ -214,23 +214,21 @@ apm pack # marketplace.json also resolves agains ## GitLab (SaaS or self-managed) -APM fetches `path:`-specified files from GitLab dependencies via git sparse/partial -checkout (the same transport as the clone). Git transport is tried first, so SSH -keys and git credential helpers work without any extra token, and self-hosted -GitLab instances where the API returns 410 (disabled) no longer fail. Explicit -`git:` / SSH URLs carry the host in the dependency; set `GITLAB_HOST` (or -`APM_GITLAB_HOSTS`) only when bare-host or shorthand forms should classify as -GitLab. - -If git transport is unavailable, `GITLAB_APM_PAT` is the fallback: - -```bash -export GITLAB_APM_PAT=glpat_your_token -apm install -``` - -`GITLAB_TOKEN` is accepted as a lower-precedence fallback. `git credential fill` is -also tried (same as for GitHub) so credential-manager users need no env var at all. +GitLab `path:` single-file sparse fetches follow the clone transport policy: +`--ssh`, `APM_GIT_PROTOCOL`, saved `prefer-ssh`, and opt-in +`--allow-protocol-fallback` / `APM_ALLOW_PROTOCOL_FALLBACK`. In strict mode, +APM passes the selected SSH/SCP URL to Git with its user, host, port, and +requested ref intact; safe Git `insteadOf` rewrites still apply. If the +effective transport remains SSH, failure never unlocks REST, even with a +PAT available. Fix SSH or declare the HTTPS web endpoint. + +Default HTTPS compatibility remains. REST requires an exhausted Git plan +and an executed effective HTTPS attempt matching the API's normalized +scheme/host/port. HTTP is not upgraded; HTTPS rewritten to SSH/local does +not qualify. Opt-in alternate protocol reuses the declared custom port and +warns, without mapping SSH aliases to web hostnames. See the +[GitLab fetch policy](https://microsoft.github.io/apm/consumer/authentication/#gitlab-saas-or-self-managed) +and [GitLab hosts](#gitlab-hosts) for token trust. ## GHE Cloud data residency (*.ghe.com) @@ -406,7 +404,7 @@ credential under a fully qualified `https://:/` URL. ### SSH connection hangs on corporate/VPN networks -APM tries SSH as a fallback when HTTPS auth is not available. It forces +APM tries SSH when selected or cross-protocol fallback is enabled. It forces `BatchMode=yes`, disables askpass and HTTP credential channels, and uses a 30-second connection timeout so SSH attempts fail without prompting. diff --git a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md index 7841cbf178..f21672b42d 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md +++ b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md @@ -65,10 +65,11 @@ parent's remote host/repo/ref and fetches the sibling from the same origin. Absolute paths, paths that escape the repo root, and cross-repo local paths are rejected. -**GitLab `path:` fetch transport:** GitLab `path:` files are fetched over git -transport, not the REST API, so self-hosted instances with the API disabled -still install. Path containment is enforced on the materialized file to reject -symlink or traversal escapes. For fallback token setup, see `authentication.md`. +**GitLab `path:` fetch transport:** GitLab `path:` files use Git first, so +self-hosted instances with the API disabled still install. Restricted REST +fallback requires an exhausted plan with an executed same-origin effective +HTTPS attempt. Path containment rejects symlink or traversal escapes. +See [GitLab authentication and fetch policy](authentication.md#gitlab-saas-or-self-managed). ### Custom git ports diff --git a/scripts/architecture_linter/checks/transport_auth_platform.py b/scripts/architecture_linter/checks/transport_auth_platform.py index d9f8cb702e..8b976ebc5b 100644 --- a/scripts/architecture_linter/checks/transport_auth_platform.py +++ b/scripts/architecture_linter/checks/transport_auth_platform.py @@ -32,6 +32,9 @@ direct_definitions, effective_definition, ) +from scripts.architecture_linter.checks.transport_gitlab_sparse import ( + check_gitlab_prepared_remote, +) from scripts.architecture_linter.checks.transport_platform_shared import ( _SRC_PREFIX, GROUP, @@ -1031,20 +1034,7 @@ def _check_git_child_environment(provider: FactsProvider) -> tuple[Violation, .. exempt=False, ) ) - findings.extend( - _require_subs( - provider, - inv, - _RID_GIT_CHILD_ENV, - "src/apm_cli/deps/download_strategies.py", - ( - "tokenless_url_builder = partial(", - 'token="",', - "build_repo_url_fn=tokenless_url_builder", - ), - "Git file transport must keep managed credentials out of remote URLs", - ) - ) + findings.extend(check_gitlab_prepared_remote(provider, _RID_GIT_CHILD_ENV)) findings.extend( _require_subs( provider, diff --git a/scripts/architecture_linter/checks/transport_gitlab_sparse.py b/scripts/architecture_linter/checks/transport_gitlab_sparse.py new file mode 100644 index 0000000000..db9a30190b --- /dev/null +++ b/scripts/architecture_linter/checks/transport_gitlab_sparse.py @@ -0,0 +1,232 @@ +"""Executable ownership edges for the bounded GitLab sparse-fetch consumer.""" + +from __future__ import annotations + +import ast + +from scripts.architecture_linter.checks.python_semantics import assignments_to +from scripts.architecture_linter.checks.transport_platform_shared import GROUP +from scripts.architecture_linter.checks.tree_index import TreeIndex +from scripts.architecture_linter.facts import FactsProvider +from scripts.architecture_linter.models import Rule, Violation + +RULE_ID = "transport-platform-gitlab-sparse-plan" +_CONSUMER = "src/apm_cli/deps/download_strategies.py" +_OWNER = "src/apm_cli/deps/transport_selection.py" + + +def _expr(node: ast.AST | None) -> str: + """Compare executable expressions, never comments or source substrings.""" + return "" if node is None else ast.unparse(node) + + +def _calls(index: TreeIndex, scope: ast.AST, target: str) -> tuple[ast.Call, ...]: + return tuple( + node + for node in index.own_scope(scope) + if isinstance(node, ast.Call) and _expr(node.func) == target + ) + + +def _binding(index: TreeIndex, scope: ast.AST, name: str) -> ast.AST | None: + bindings = assignments_to(index, scope, name) + return bindings[0].value if len(bindings) == 1 else None + + +def _keywords(call: ast.Call) -> dict[str, str]: + return {item.arg: _expr(item.value) for item in call.keywords if item.arg is not None} + + +def check_gitlab_prepared_remote(provider: FactsProvider, rule_id: str) -> tuple[Violation, ...]: + """Share tokenless construction and prepared-callback evidence with auth guards.""" + index = provider.tree_index(_CONSUMER) + method = None if index is None else index.function("DownloadDelegate.download_gitlab_file") + executor = ( + None if index is None else index.function("DownloadDelegate._download_gitlab_file_via_git") + ) + valid = False + if ( + index is not None + and isinstance(method, ast.FunctionDef) + and isinstance(executor, ast.FunctionDef) + ): + builders = _calls(index, method, "self.build_repo_url") + constructors = [ + node + for node in index.own_scope(executor) + if isinstance(node, ast.Call) and "build_repo_url_fn" in _keywords(node) + ] + if len(constructors) == 1: + callback = index.function( + "DownloadDelegate._download_gitlab_file_via_git." + + _keywords(constructors[0])["build_repo_url_fn"] + ) + valid = ( + bool(builders) + and all(_keywords(call).get("token") == "''" for call in builders) + and isinstance(callback, ast.FunctionDef) + and len(callback.body) == 1 + and isinstance(callback.body[0], ast.Return) + and _expr(callback.body[0].value) == "requested_url" + and _keywords(constructors[0]).get("git_env") == "git_env" + ) + if valid: + return () + return ( + Violation( + rule_id, + _CONSUMER, + getattr(executor, "lineno", 1), + 1, + "GitLab sparse transport must preserve the prepared URL callback ownership " + "edge and keep managed credentials out of every constructed remote URL", + ), + ) + + +def _rest_gate(index: TreeIndex, method: ast.FunctionDef, loop: ast.For) -> bool: + """Require authorization accumulated from executed attempts before REST.""" + bindings = assignments_to(index, method, "rest_eligible") + if len(bindings) != 2 or _expr(bindings[0].value) != "False": + return False + update = bindings[1] + value = update.value + if not ( + isinstance(value, ast.BoolOp) + and isinstance(value.op, ast.Or) + and len(value.values) == 2 + and _expr(value.values[0]) == "rest_eligible" + and isinstance(value.values[1], ast.Call) + and _expr(value.values[1].func) == "self._gitlab_rest_eligible" + and tuple(map(_expr, value.values[1].args)) == ("effective_url", "host_info.api_base") + ): + return False + handler = index.parent(update.node) + if not ( + isinstance(handler, ast.ExceptHandler) + and _expr(handler.type) == "GitFileTransportError" + and handler in index.walk(loop) + ): + return False + rest_calls = _calls(index, method, "self._download_gitlab_file_via_rest") + gates = [ + statement + for statement in method.body + if isinstance(statement, ast.If) and _expr(statement.test) == "rest_eligible" + ] + return ( + len(rest_calls) == 1 + and len(gates) == 1 + and method.body.index(gates[0]) > method.body.index(loop) + and any(rest_calls[0] in index.walk(statement) for statement in gates[0].body) + and not gates[0].orelse + ) + + +def _check_gitlab_sparse_plan(provider: FactsProvider) -> tuple[Violation, ...]: + """Defend selector, prepared remote, auth owner, and REST authorization edges.""" + index = provider.tree_index(_CONSUMER) + owner = provider.tree_index(_OWNER) + findings: list[Violation] = [] + + def require(condition: bool, edge: str, node: ast.AST | None = None) -> None: + if not condition: + findings.append( + Violation( + RULE_ID, + _CONSUMER, + getattr(node, "lineno", 1), + 1, + f"GitLab sparse transport must preserve the {edge} ownership edge", + ) + ) + + if index is None or owner is None: + require(False, "parseable transport policy") + return tuple(findings) + method = index.function("DownloadDelegate.download_gitlab_file") + executor = index.function("DownloadDelegate._download_gitlab_file_via_git") + require( + owner.function("initial_transport_scheme") is not None + and owner.function("TransportSelector.select") is not None, + "transport-selection authority", + ) + if not isinstance(method, ast.FunctionDef) or not isinstance(executor, ast.FunctionDef): + require(False, "orchestrator and prepared-attempt executor") + return tuple(findings) + + initial_calls = _calls(index, method, "initial_transport_scheme") + candidate = _binding(index, method, "candidate_url") + require( + len(initial_calls) == 1 + and isinstance(candidate, ast.Call) + and initial_calls[0] in index.walk(candidate) + and _keywords(candidate).get("token") == "''", + "initial-scheme helper", + method, + ) + plan = _binding(index, method, "plan") + anonymous_plan = _binding(index, method, "anonymous_plan") + loops = [ + node + for node in method.body + if isinstance(node, ast.For) + and _expr(node.target) == "attempt" + and _expr(node.iter) == "plan.attempts" + ] + require( + isinstance(plan, ast.Call) + and _expr(plan.func) == "self._host._transport_selector.select" + and isinstance(anonymous_plan, ast.Call) + and _expr(anonymous_plan.func) == "self._host._transport_selector.select" + and _keywords(plan).get("candidate_url") == "candidate_url" + and _keywords(plan).get("allow_fallback") == "self._host._allow_fallback" + and len(loops) == 1, + "selector plan execution", + method, + ) + attempt_ctx = _binding(index, method, "attempt_ctx") + git_env = _binding(index, method, "git_env") + require( + _expr(_binding(index, method, "resolver")) == "self._host.auth_resolver" + and isinstance(attempt_ctx, ast.Call) + and _expr(attempt_ctx.func) == "resolver.resolve_for_remote" + and tuple(map(_expr, attempt_ctx.args[:2])) == ("host", "effective_url") + and isinstance(git_env, ast.IfExp) + and _expr(git_env.test) == "attempt.use_token" + and _expr(git_env.body) == "resolver.git_env_for_remote(attempt_ctx, effective_url)" + and _expr(git_env.orelse) + == "resolver.build_native_git_credential_env(host_info, effective_url)", + "AuthResolver routing", + method, + ) + prepared = _calls(index, method, "self._download_gitlab_file_via_git") + require( + len(prepared) == 1 + and len(loops) == 1 + and prepared[0] in index.walk(loops[0]) + and all( + _keywords(prepared[0]).get(name) == name + for name in ("requested_url", "effective_url", "git_env") + ), + "prepared attempt arguments", + method, + ) + findings.extend(check_gitlab_prepared_remote(provider, RULE_ID)) + require( + len(loops) == 1 and _rest_gate(index, method, loops[0]), + "executed HTTPS REST gate", + method, + ) + return tuple(findings) + + +RULES = ( + Rule( + id=RULE_ID, + group=GROUP, + guard_ids=(RULE_ID,), + description="GitLab sparse fetch consumes transport selection and prepared auth before gated REST.", + check=_check_gitlab_sparse_plan, + ), +) diff --git a/scripts/architecture_linter/groups/transport_platform.py b/scripts/architecture_linter/groups/transport_platform.py index 777fdf5aa1..5dadc1dc68 100644 --- a/scripts/architecture_linter/groups/transport_platform.py +++ b/scripts/architecture_linter/groups/transport_platform.py @@ -9,6 +9,9 @@ COLLECTORS as _CACHE_COLLECTORS, ) from scripts.architecture_linter.checks.transport_cache_identity import RULES as _CACHE_RULES +from scripts.architecture_linter.checks.transport_gitlab_sparse import ( + RULES as _GITLAB_SPARSE_RULES, +) from scripts.architecture_linter.checks.transport_network_and_runtime import ( COLLECTORS as _NETWORK_COLLECTORS, ) @@ -33,6 +36,7 @@ + _CACHE_RULES + _CLEANUP_RULES + _SPARSE_RULES + + _GITLAB_SPARSE_RULES + _REVISION_PIN_RULES + _NETWORK_RULES ) diff --git a/src/apm_cli/core/auth.py b/src/apm_cli/core/auth.py index 03fc873e81..ad66efa0dc 100644 --- a/src/apm_cli/core/auth.py +++ b/src/apm_cli/core/auth.py @@ -1123,7 +1123,8 @@ def _resolve_token( 4. Host-specific git credential helper Resolution order (``gitlab``): ``GITLAB_APM_PAT`` -> ``GITLAB_TOKEN`` -> - credential helper. GitHub env vars are not consulted. + credential helper when the remote transport permits lookup. + GitHub env vars are not consulted. Resolution order (``generic``): credential helper only (no GitHub or GitLab platform env vars). @@ -1187,7 +1188,7 @@ def _resolve_token( # 4. Git credential helper (not for ADO) if host_info.kind not in ("ado",) and ( - host_info.kind != "generic" or allow_generic_credential_lookup + host_info.kind not in ("generic", "gitlab") or allow_generic_credential_lookup ): # Most primary resolution calls remain host-scoped. The public # github.com anonymous-first fallback supplies path= after a diff --git a/src/apm_cli/deps/clone_engine.py b/src/apm_cli/deps/clone_engine.py index 19b659203e..620726d38b 100644 --- a/src/apm_cli/deps/clone_engine.py +++ b/src/apm_cli/deps/clone_engine.py @@ -39,15 +39,16 @@ is_github_hostname, ) from .bare_cache import build_clone_failure_message -from .transport_selection import ProtocolPreference, TransportAttempt, TransportPlan +from .transport_selection import ( + TransportAttempt, + TransportPlan, + fallback_port_warning, + initial_transport_scheme, +) if TYPE_CHECKING: from ..core.auth import AuthResolver -_PROTOCOL_FALLBACK_DOCS_URL = ( - "https://microsoft.github.io/apm/guides/dependencies/#restoring-the-legacy-permissive-chain" -) - def _is_connect_failure(error: GitCommandError | subprocess.CalledProcessError, url: str) -> bool: """Recognize only Git's pre-connection HTTPS failure for this exact remote.""" @@ -197,12 +198,7 @@ def _clone(url: str, env: dict[str, str], target: Path) -> None: dep_host = dep_ref.host if dep_ref else None is_github = is_github_hostname(dep_host) if dep_host else True is_generic = not is_ado and not is_github - explicit_scheme = ( - (getattr(dep_ref, "explicit_scheme", None) or "").lower() if dep_ref else "" - ) - candidate_uses_ssh = explicit_scheme == "ssh" or ( - not explicit_scheme and self._protocol_pref == ProtocolPreference.SSH - ) + candidate_uses_ssh = initial_transport_scheme(dep_ref, self._protocol_pref) == "ssh" rewrite_candidate = ( host._build_repo_url( repo_url_base, @@ -321,17 +317,12 @@ def _without_platform_credentials(env: dict[str, str]) -> dict[str, str]: ) # Cross-protocol fallback custom-port warning (#786). - dep_port = getattr(dep_ref, "port", None) if dep_ref else None - if ( - not plan.strict - and dep_port is not None - and any(a.scheme == "ssh" for a in plan.attempts) - and any(a.scheme == "https" for a in plan.attempts) - ): + port_warning = fallback_port_warning(dep_ref, plan) + if port_warning is not None: warn_key = ( dep_host.lower() if dep_host else dep_host, repo_url_base, - dep_port, + dep_ref.port, ) # Guard the check-then-add under the lock so two threads # racing on the same warn_key cannot both pass the @@ -342,20 +333,7 @@ def _without_platform_credentials(env: dict[str, str]) -> dict[str, str]: self._fallback_port_warned.add(warn_key) _should_warn = True if _should_warn: - initial_scheme = plan.attempts[0].scheme.upper() - fallback_scheme = next( - a.scheme.upper() for a in plan.attempts if a.scheme != plan.attempts[0].scheme - ) - host_display = dep_host or "host" - _rich_warning( - f"Custom port {dep_port} on {host_display}/{repo_url_base}: " - f"if {initial_scheme} fails, APM will retry over " - f"{fallback_scheme} on the same port.\n" - f" Pin the URL scheme, or drop " - f"--allow-protocol-fallback to fail fast.\n" - f" See: {_PROTOCOL_FALLBACK_DOCS_URL}", - symbol="warning", - ) + _rich_warning(port_warning, symbol="warning") def _run_public_github_attempt() -> tuple[str, bool]: if dep_ref is None: diff --git a/src/apm_cli/deps/download_strategies.py b/src/apm_cli/deps/download_strategies.py index 0c28d98761..d0424ce533 100644 --- a/src/apm_cli/deps/download_strategies.py +++ b/src/apm_cli/deps/download_strategies.py @@ -19,15 +19,17 @@ import zipfile from functools import partial from pathlib import Path -from urllib.parse import quote +from urllib.parse import quote, urlsplit import requests +from ..cache.url_normalize import normalize_repo_url from ..core.auth import AuthResolver, HostInfo from ..models.apm_package import DependencyReference from ..models.dependency.host_virtual import dependency_repository_owner, repository_owner_and_repo from ..utils.archive import ArchiveError, safe_extract_zip -from ..utils.git_env import redact_git_diagnostic +from ..utils.console import _rich_warning +from ..utils.git_env import redact_git_diagnostic, validate_git_url_rewrite_safety from ..utils.github_host import ( build_ado_api_url, build_artifactory_archive_url, @@ -37,16 +39,15 @@ default_host, is_github_hostname, ) -from ..utils.path_security import PathTraversalError from .artifactory_entry import _NoNetrcSession from .git_file_transport import ( GitFileFetchResult, GitFileTransportError, - GitFileTransportSecurityError, GitSparseFileTransport, ) from .github_rate_limit import GitHubThrottleError, github_throttle_error from .host_backends import backend_for +from .transport_selection import fallback_port_warning, initial_transport_scheme # --------------------------------------------------------------------------- # Module-level debug helper (mirrors the one in github_downloader so that @@ -110,9 +111,7 @@ def __init__(self, host, git_file_transport_factory=None): this delegate. """ self._host = host - self._git_file_transports: dict[ - tuple[str, str, str, int | None], GitSparseFileTransport - ] = {} + self._git_file_transports: dict[tuple[str, str, str, str, str], GitSparseFileTransport] = {} self._git_file_transports_lock = threading.Lock() self._git_file_transport_factory = git_file_transport_factory self._git_file_transport_finalizer = weakref.finalize( @@ -770,63 +769,66 @@ def _bearer_op(bearer: str): raise RuntimeError(f"Network error downloading {file_path}: {e}") from e def _git_file_transport_key( - self, dep_ref: DependencyReference, ref: str - ) -> tuple[str, str, str, int | None]: + self, + dep_ref: DependencyReference, + ref: str, + requested_url: str, + effective_url: str, + auth_mode: str, + ) -> tuple[str, str, str, str, str]: """Return the cache key for one path-scoped Git file checkout.""" - return (dep_ref.host or default_host(), dep_ref.repo_url, ref, dep_ref.port) + provider = self._host.auth_resolver.classify_host( + dep_ref.host or default_host(), port=dep_ref.port, host_type=dep_ref.host_type + ) + return ( + provider.kind, + ref, + normalize_repo_url(requested_url), + normalize_repo_url(effective_url), + auth_mode, + ) - def _discard_git_file_transport(self, key: tuple[str, str, str, int | None]) -> None: + def _discard_git_file_transport( + self, key: tuple[str, str, str, str, str], failed_transport: GitSparseFileTransport + ) -> None: """Close and remove a failed cached git-file checkout.""" with self._git_file_transports_lock: - transport = self._git_file_transports.pop(key, None) - if transport is not None: - transport.close() + if self._git_file_transports.get(key) is failed_transport: + del self._git_file_transports[key] + failed_transport.close() def _download_gitlab_file_via_git( self, dep_ref: DependencyReference, file_path: str, ref: str, + *, + requested_url: str, + effective_url: str, + git_env: dict[str, str], + auth_mode: str, ) -> bytes: - """Fetch a GitLab path: file via a reusable sparse checkout.""" - key = self._git_file_transport_key(dep_ref, ref) + """Execute one prepared GitLab attempt through a reusable sparse checkout.""" + key = self._git_file_transport_key(dep_ref, ref, requested_url, effective_url, auth_mode) + + def _prepared_repo_url(repo_ref: str, *, dep_ref: DependencyReference) -> str: + return requested_url + with self._git_file_transports_lock: transport = self._git_file_transports.get(key) if transport is None: - auth_ctx = self._host.auth_resolver.resolve_for_dep(dep_ref) - remote_url = self.build_repo_url( - dep_ref.repo_url, - dep_ref=dep_ref, - token="", - auth_scheme=auth_ctx.auth_scheme if auth_ctx is not None else "basic", - ) - git_env = ( - self._host.auth_resolver.git_env_for_remote( - auth_ctx, - remote_url, - ) - if auth_ctx is not None - else dict(self._host.git_env or {}) - ) - from functools import partial - - tokenless_url_builder = partial( - self.build_repo_url, - token="", - auth_scheme=auth_ctx.auth_scheme if auth_ctx is not None else "basic", - ) transport_factory = self._git_file_transport_factory or GitSparseFileTransport transport = transport_factory( dep_ref, ref, - build_repo_url_fn=tokenless_url_builder, + build_repo_url_fn=_prepared_repo_url, git_env=git_env, ) self._git_file_transports[key] = transport try: return transport.fetch_file(file_path) except GitFileTransportError: - self._discard_git_file_transport(key) + self._discard_git_file_transport(key, transport) raise def _download_github_file_via_git( @@ -836,7 +838,6 @@ def _download_github_file_via_git( ref: str, ) -> GitFileFetchResult: """Fetch a throttled GitHub virtual file through one sparse Git transport.""" - key = self._git_file_transport_key(dep_ref, ref) host = dep_ref.host or default_host() if ( self._host.auth_resolver.uses_public_github_anonymous_first( @@ -902,6 +903,11 @@ def _tokenless_repo_url( # inherit an unrelated downloader token into this fallback. git_env = self._host._build_noninteractive_git_env() + effective_url = validate_git_url_rewrite_safety(remote_url, git_env) or remote_url + key = self._git_file_transport_key( + dep_ref, ref, remote_url, effective_url, "managed" if auth_ctx.token else "native" + ) + def _tokenless_repo_url(repo_ref: str, *, dep_ref: DependencyReference) -> str: return remote_url @@ -919,7 +925,7 @@ def _tokenless_repo_url(repo_ref: str, *, dep_ref: DependencyReference) -> str: try: return transport.fetch_file_with_commit(file_path) except GitFileTransportError: - self._discard_git_file_transport(key) + self._discard_git_file_transport(key, transport) raise def download_github_file_via_throttle_fallback( @@ -947,6 +953,17 @@ def download_github_file_via_throttle_fallback( # GitLab file download # ------------------------------------------------------------------ + @staticmethod + def _gitlab_rest_eligible(effective_url: str, api_base: str) -> bool: + """Authorize REST only for an executed same-origin HTTPS Git attempt.""" + remote = urlsplit(effective_url) + api = urlsplit(api_base) + return ( + remote.scheme.lower() == api.scheme.lower() == "https" + and remote.hostname == api.hostname + and (remote.port or 443) == (api.port or 443) + ) + def download_gitlab_file( self, dep_ref: DependencyReference, @@ -954,17 +971,7 @@ def download_gitlab_file( ref: str = "main", verbose_callback=None, ) -> bytes: - """Download a GitLab file: git-transport-first, REST API as fallback. - - Primary path (the 410-killer): extracts the file via git sparse/ - partial checkout (blob:none + file-level sparse paths) so SSH keys and - system git credentials are sufficient -- no REST API token needed. - - Fallback (thin GITLAB_PAT path): if the git transport fails (e.g. - SSH not available, network restriction), the existing GitLab REST v4 - ``repository/files/.../raw`` endpoint is tried with the GITLAB_APM_PAT - / GITLAB_TOKEN credential, mirroring the ADO_APM_PAT pattern. - """ + """Execute the selected Git plan before an authorized HTTPS REST fallback.""" host = dep_ref.host or default_host() host_info = self._host.auth_resolver.classify_host( host, @@ -975,27 +982,132 @@ def download_gitlab_file( if not project_path: raise RuntimeError("Missing repository path for GitLab file download") - # -- Primary: git sparse/partial checkout (works even when API is 410) -- - try: - content = self._download_gitlab_file_via_git(dep_ref, file_path, ref) - if verbose_callback: - verbose_callback( - f"Fetched file via git transport: {host}/{dep_ref.repo_url}/{file_path}" + resolver = self._host.auth_resolver + candidate_url = self.build_repo_url( + project_path, + dep_ref=dep_ref, + use_ssh=initial_transport_scheme(dep_ref, self._host._protocol_pref) == "ssh", + token="", + ) + anonymous_plan = self._host._transport_selector.select( + dep_ref=dep_ref, + cli_pref=self._host._protocol_pref, + allow_fallback=self._host._allow_fallback, + has_token=False, + candidate_url=candidate_url, + ) + initial = anonymous_plan.attempts[0] + initial_ctx = resolver.resolve_for_remote( + host, + initial.effective_url or initial.requested_url or candidate_url, + dependency_repository_owner(dep_ref), + port=dep_ref.port, + host_type=dep_ref.host_type, + ) + plan = self._host._transport_selector.select( + dep_ref=dep_ref, + cli_pref=self._host._protocol_pref, + allow_fallback=self._host._allow_fallback, + has_token=bool(initial_ctx.token), + candidate_url=candidate_url, + ) + notice = fallback_port_warning(dep_ref, plan) + if notice: + warn_key = (host.lower(), project_path, dep_ref.port) + with self._host._fallback_port_warned_lock: + should_warn = warn_key not in self._host._fallback_port_warned + self._host._fallback_port_warned.add(warn_key) + if should_warn: + _rich_warning(notice, symbol="warning") + + rest_eligible = False + failures: list[str] = [] + last_error: GitFileTransportError | None = None + previous_attempt = None + for attempt in plan.attempts: + requested_url = attempt.requested_url or self.build_repo_url( + project_path, + dep_ref=dep_ref, + use_ssh=attempt.scheme == "ssh", + token="", + ) + effective_url = attempt.effective_url or requested_url + attempt_ctx = resolver.resolve_for_remote( + host, + effective_url, + dependency_repository_owner(dep_ref), + port=dep_ref.port, + host_type=dep_ref.host_type, + ) + git_env = ( + resolver.git_env_for_remote(attempt_ctx, effective_url) + if attempt.use_token + else resolver.build_native_git_credential_env(host_info, effective_url) + ) + # Revalidate before reuse: a cached checkout must not bypass changed policy. + effective_url = validate_git_url_rewrite_safety(requested_url, git_env) or effective_url + if ( + not plan.strict + and previous_attempt is not None + and previous_attempt.scheme != attempt.scheme + ): + _rich_warning( + redact_git_diagnostic( + f"Protocol fallback: {previous_attempt.label} GitLab sparse fetch of " + f"{project_path} failed; retrying with {attempt.label}." + ), + symbol="warning", ) - return content - except (PathTraversalError, GitFileTransportSecurityError): - # A traversal / symlink-escape attempt must hard-fail. It must - # NOT be silently retried over the REST transport -- letting a - # rejected path fall through would hand an attacker a second - # transport to probe. Propagate the security failure unchanged. - raise - except (GitFileTransportError, RuntimeError, OSError) as exc: - fallback_target = f"{host}/{dep_ref.repo_url}" - _debug( - f"git transport unavailable for {fallback_target}; " - f"falling back to GitLab REST API ({type(exc).__name__})" + try: + content = self._download_gitlab_file_via_git( + dep_ref, + file_path, + ref, + requested_url=requested_url, + effective_url=effective_url, + git_env=git_env, + auth_mode="managed" if attempt.use_token else "native", + ) + except GitFileTransportError as exc: + last_error = exc + previous_attempt = attempt + failures.append(redact_git_diagnostic(f"{attempt.label}: {exc}")) + rest_eligible = rest_eligible or self._gitlab_rest_eligible( + effective_url, host_info.api_base + ) + else: + if verbose_callback: + verbose_callback( + f"Fetched file via git transport: {host}/{project_path}/{file_path}" + ) + return content + + if rest_eligible: + return self._download_gitlab_file_via_rest(dep_ref, file_path, ref, verbose_callback) + failure_detail = "; ".join(failures) + raise RuntimeError( + redact_git_diagnostic( + f"Failed to download {file_path} at ref '{ref}' from {host}/{project_path}. " + f"Git transport failed ({failure_detail}); GitLab REST is not authorized " + "by the selected transport plan. Verify access using the selected Git " + "transport (including SSH keys for SSH), or explicitly configure an HTTPS " + "dependency using the web endpoint." ) - # -- Fallback: GitLab REST v4 API (requires GITLAB_APM_PAT / GITLAB_TOKEN) -- + ) from last_error + + def _download_gitlab_file_via_rest( + self, + dep_ref: DependencyReference, + file_path: str, + ref: str, + verbose_callback=None, + ) -> bytes: + """Read the existing GitLab REST endpoint after transport authorization.""" + host = dep_ref.host or default_host() + host_info = self._host.auth_resolver.classify_host( + host, port=dep_ref.port, host_type=dep_ref.host_type + ) + project_path = dep_ref.repo_url org = project_path.split("/")[0] file_ctx = self._host.auth_resolver.resolve( host, diff --git a/src/apm_cli/deps/git_reference_resolver.py b/src/apm_cli/deps/git_reference_resolver.py index ae97c3ddc3..5ae70febaa 100644 --- a/src/apm_cli/deps/git_reference_resolver.py +++ b/src/apm_cli/deps/git_reference_resolver.py @@ -51,7 +51,7 @@ ) from .git_remote_ops import validate_ls_remote_tag_output from .github_rate_limit import raise_for_github_throttle -from .transport_selection import ProtocolPreference +from .transport_selection import ProtocolPreference, initial_transport_scheme if TYPE_CHECKING: import requests @@ -150,10 +150,7 @@ def _list_remote_refs( is_ado = dep_ref.is_azure_devops() repo_url_base = dep_ref.repo_url - explicit_scheme = (getattr(dep_ref, "explicit_scheme", None) or "").lower() - candidate_uses_ssh = explicit_scheme == "ssh" or ( - not explicit_scheme and host._protocol_pref == ProtocolPreference.SSH - ) + candidate_uses_ssh = initial_transport_scheme(dep_ref, host._protocol_pref) == "ssh" rewrite_candidate = host._build_repo_url( repo_url_base, use_ssh=candidate_uses_ssh, diff --git a/src/apm_cli/deps/transport_selection.py b/src/apm_cli/deps/transport_selection.py index 67ce401904..43a7572cc5 100644 --- a/src/apm_cli/deps/transport_selection.py +++ b/src/apm_cli/deps/transport_selection.py @@ -20,9 +20,12 @@ import threading from dataclasses import dataclass from enum import Enum -from typing import Protocol, runtime_checkable +from typing import TYPE_CHECKING, Protocol, runtime_checkable from urllib.parse import urlsplit +if TYPE_CHECKING: + from ..models.dependency.reference import DependencyReference + # Public env vars (also recognized by CLI flag plumbing). ENV_PROTOCOL = "APM_GIT_PROTOCOL" ENV_ALLOW_FALLBACK = "APM_ALLOW_PROTOCOL_FALLBACK" @@ -38,6 +41,9 @@ "Inspect matching rules with " "'git config --show-origin --get-regexp ^url\\..*\\.insteadOf$'." ) +_PROTOCOL_FALLBACK_DOCS_URL = ( + "https://microsoft.github.io/apm/consumer/manage-dependencies/#transport-selection" +) class ProtocolPreference(Enum): @@ -104,6 +110,53 @@ class TransportPlan: fallback_hint: str | None = None +def initial_transport_scheme( + dep_ref: DependencyReference | None, + cli_pref: ProtocolPreference = ProtocolPreference.NONE, +) -> str: + """Return the explicit scheme, shorthand SSH preference, or HTTPS default. + + This is the candidate scheme before Git URL rewrites, not the effective + transport selected by :class:`TransportSelector`. + """ + explicit = (getattr(dep_ref, "explicit_scheme", None) or "").lower() + if explicit: + return explicit + return "ssh" if cli_pref == ProtocolPreference.SSH else "https" + + +def fallback_port_warning( + dep_ref: DependencyReference | None, + plan: TransportPlan, +) -> str | None: + """Describe an admitted SSH/HTTPS fallback sharing a declared custom port. + + Callers own emission and deduplication using the downloader's shared state. + """ + dep_port = getattr(dep_ref, "port", None) + if ( + dep_ref is None + or plan.strict + or dep_port is None + or not any(a.scheme == "ssh" for a in plan.attempts) + or not any(a.scheme == "https" for a in plan.attempts) + ): + return None + initial_scheme = plan.attempts[0].scheme.upper() + fallback_scheme = next( + a.scheme.upper() for a in plan.attempts if a.scheme != plan.attempts[0].scheme + ) + host_display = dep_ref.host or "host" + return ( + f"Custom port {dep_port} on {host_display}/{dep_ref.repo_url}: " + f"if {initial_scheme} fails, APM will retry over " + f"{fallback_scheme} on the same port.\n" + f" Disable protocol fallback in CLI flags, environment, " + f"and saved config to fail fast.\n" + f" See: {_PROTOCOL_FALLBACK_DOCS_URL}" + ) + + @runtime_checkable class InsteadOfResolver(Protocol): """Discovers ``git config url..insteadOf`` rewrites. @@ -356,9 +409,10 @@ def select( Returns: :class:`TransportPlan`. """ - explicit = (getattr(dep_ref, "explicit_scheme", None) or "").lower() or None + initial_scheme = initial_transport_scheme(dep_ref, cli_pref) + explicit = bool(getattr(dep_ref, "explicit_scheme", None)) candidate = candidate_url - if candidate is None and explicit is None: + if candidate is None and not explicit: builder = getattr(dep_ref, "to_github_url", None) candidate = ( builder() @@ -387,11 +441,11 @@ def select( # In strict mode (default) the plan contains exactly that one attempt. # With allow_fallback (escape hatch for migration), we keep the user's # explicit starting protocol and then append the opposite protocol. - if explicit in ("ssh", "https", "http"): - if explicit == "ssh": + if explicit and initial_scheme in ("ssh", "https", "http"): + if initial_scheme == "ssh": initial = [_SSH] chained = [_AUTH_HTTPS, _PLAIN_HTTPS] if has_token else [_PLAIN_HTTPS] - elif explicit == "https": + elif initial_scheme == "https": initial = [_AUTH_HTTPS] if has_token else [_PLAIN_HTTPS] chained = [_SSH, _PLAIN_HTTPS] if has_token else [_SSH] else: @@ -422,12 +476,9 @@ def select( # 2. Shorthand (no explicit scheme). Consult the CLI preference and git # insteadOf rewrites to pick the initial protocol. - if cli_pref == ProtocolPreference.SSH: + if initial_scheme == "ssh": initial = [_SSH] chained = [_AUTH_HTTPS, _PLAIN_HTTPS] if has_token else [_PLAIN_HTTPS] - elif cli_pref == ProtocolPreference.HTTPS: - initial = [_AUTH_HTTPS] if has_token else [_PLAIN_HTTPS] - chained = [_SSH, _PLAIN_HTTPS] if has_token else [_SSH] else: # Default shorthand initial attempt: HTTPS. If allow_fallback is on, # append SSH (and plain HTTPS after auth) below. diff --git a/src/apm_cli/install/validation.py b/src/apm_cli/install/validation.py index 500b2bdbe5..63ec9de1bf 100644 --- a/src/apm_cli/install/validation.py +++ b/src/apm_cli/install/validation.py @@ -399,6 +399,7 @@ def _validate_ado_git_package( from apm_cli.deps.github_downloader import GitHubPackageDownloader from apm_cli.deps.transport_selection import ( ProtocolPreference, + initial_transport_scheme, is_fallback_allowed, protocol_pref_from_env, ) @@ -443,10 +444,7 @@ def _validate_ado_git_package( resolved_fallback = ( is_fallback_allowed() if allow_protocol_fallback is None else allow_protocol_fallback ) - explicit_scheme = (getattr(dep_ref, "explicit_scheme", None) or "").lower() or None - candidate_uses_ssh = explicit_scheme == "ssh" or ( - explicit_scheme is None and resolved_pref == ProtocolPreference.SSH - ) + candidate_uses_ssh = initial_transport_scheme(dep_ref, resolved_pref) == "ssh" candidate_url = ado_downloader._build_repo_url( dep_ref.repo_url, use_ssh=candidate_uses_ssh, diff --git a/tests/integration/test_architecture_gitlab_sparse_transport.py b/tests/integration/test_architecture_gitlab_sparse_transport.py new file mode 100644 index 0000000000..8f76ff9973 --- /dev/null +++ b/tests/integration/test_architecture_gitlab_sparse_transport.py @@ -0,0 +1,174 @@ +"""In-memory source mutations prove the GitLab sparse ownership boundaries.""" + +from __future__ import annotations + +import ast +import json +from pathlib import Path + +import pytest + +from scripts.architecture_linter.runner import registered_rules, run_selected_rules + +pytestmark = pytest.mark.component + +ROOT = Path(__file__).resolve().parents[2] +RULE_ID = "transport-platform-gitlab-sparse-plan" +AUTH_RULE_ID = "transport-platform-git-child-environment" +CONSUMER = "src/apm_cli/deps/download_strategies.py" + + +def test_gitlab_sparse_plan_is_registered_and_compliant() -> None: + """The owner registry and executable rule agree on the integrated consumer.""" + registry = json.loads( + (ROOT / ".apm/architecture/owners/transport-auth-platform.json").read_text(encoding="utf-8") + ) + owner = next(item for item in registry["owners"] if item["id"] == "git-transport-selection") + assert owner["guards"] == [RULE_ID] + assert set(owner["selectors"]) == { + CONSUMER, + "src/apm_cli/deps/transport_selection.py", + } + assert [rule.id for rule in registered_rules() if rule.id == RULE_ID] == [RULE_ID] + report = run_selected_rules(ROOT, (RULE_ID, AUTH_RULE_ID)) + assert not report.failures + assert not report.violations + + +@pytest.mark.parametrize( + ("old", "new", "edge"), + [ + ( + "initial_transport_scheme(dep_ref, self._host._protocol_pref)", + "'https'", + "initial-scheme helper", + ), + ( + " plan = self._host._transport_selector.select(", + " plan = bypass_selector(", + "selector plan execution", + ), + ( + "anonymous_plan = self._host._transport_selector.select(", + "anonymous_plan = bypass_selector(", + "selector plan execution", + ), + ( + "for attempt in plan.attempts:", + "for attempt in bypass_attempts:", + "selector plan execution", + ), + ( + "resolver = self._host.auth_resolver", + "resolver = bypass_auth", + "AuthResolver routing", + ), + ( + "attempt_ctx = resolver.resolve_for_remote(", + "attempt_ctx = bypass_auth(", + "AuthResolver routing", + ), + ( + "resolver.git_env_for_remote(attempt_ctx, effective_url)", + "bypass_env(attempt_ctx, effective_url)", + "AuthResolver routing", + ), + ( + "resolver.build_native_git_credential_env(host_info, effective_url)", + "bypass_native_env(host_info, effective_url)", + "AuthResolver routing", + ), + ( + "requested_url=requested_url,", + "requested_url=candidate_url,", + "prepared attempt arguments", + ), + ( + "return requested_url", + "return self.build_repo_url(repo_ref, dep_ref=dep_ref)", + "prepared URL callback", + ), + ( + "build_repo_url_fn=_prepared_repo_url,", + "build_repo_url_fn=self.build_repo_url,", + "prepared URL callback", + ), + ( + "if rest_eligible:", + "if True:", + "executed HTTPS REST gate", + ), + ( + "rest_eligible = False", + "rest_eligible = True", + "executed HTTPS REST gate", + ), + ( + "rest_eligible = rest_eligible or self._gitlab_rest_eligible(", + "rest_eligible = rest_eligible or bypass_gate(", + "executed HTTPS REST gate", + ), + ( + "effective_url, host_info.api_base", + "candidate_url, host_info.api_base", + "executed HTTPS REST gate", + ), + ], + ids=[ + "initial-scheme", + "selector", + "initial-selector", + "executed-plan", + "auth-owner", + "auth-resolution", + "managed-env", + "native-env", + "prepared-arguments", + "rebuilt-url", + "callback", + "unconditional-rest", + "preauthorized-rest", + "rest-predicate", + "requested-not-effective-origin", + ], +) +def test_gitlab_sparse_guard_rejects_executable_edge_bypass(old: str, new: str, edge: str) -> None: + """Each syntactically valid bypass fails even with owner names in comments.""" + source = (ROOT / CONSUMER).read_text(encoding="utf-8") + start = source.index(" def _download_gitlab_file_via_git(") + end = source.index(" def _download_gitlab_file_via_rest(") + bounded = source[start:end] + assert old in bounded, f"Mutation seam changed: {old}" + mutated = ( + source[:start] + + bounded.replace(old, new, 1) + + source[end:] + + f"\n# Retained ownership evidence: {old}\n" + ) + ast.parse(mutated) + rule_ids = (RULE_ID, AUTH_RULE_ID) if edge == "prepared URL callback" else (RULE_ID,) + report = run_selected_rules(ROOT, rule_ids, source_overrides={CONSUMER: mutated}) + assert not report.failures + assert any( + violation.rule_id == RULE_ID and edge in violation.message + for violation in report.violations + ), report.violations + if AUTH_RULE_ID in rule_ids: + assert any(violation.rule_id == AUTH_RULE_ID for violation in report.violations) + + +@pytest.mark.parametrize("builder", ["candidate", "attempt"]) +def test_both_guards_reject_credentials_in_prepared_url(builder: str) -> None: + """Both guard families retain executable proof that every builder is tokenless.""" + source = (ROOT / CONSUMER).read_text(encoding="utf-8") + start = source.index(" def download_gitlab_file(") + end = source.index(" def _download_gitlab_file_via_rest(") + bounded = source[start:end] + old = 'token="",' + assert bounded.count(old) == 2 + before, after = bounded.split(old, 1) if builder == "candidate" else bounded.rsplit(old, 1) + mutated = source[:start] + before + "token=managed_token," + after + source[end:] + ast.parse(mutated) + report = run_selected_rules(ROOT, (RULE_ID, AUTH_RULE_ID), source_overrides={CONSUMER: mutated}) + assert not report.failures + assert {violation.rule_id for violation in report.violations} == {RULE_ID, AUTH_RULE_ID} diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index 036f70b41d..9b46e44603 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -953,6 +953,14 @@ class MutationCase: ), intent="A parallel throttle classifier appears outside deps/github_rate_limit.py.", ), + MutationCase( + guard_id="transport-platform-gitlab-sparse-plan", + rule_id="transport-platform-gitlab-sparse-plan", + path="src/apm_cli/deps/download_strategies.py", + old=" if rest_eligible:", + new=" if True: # bypass rest_eligible", + intent="GitLab file downloads bypass the executed HTTPS plan gate before REST.", + ), MutationCase( guard_id="transport-platform-host-credential-resolution", rule_id="transport-platform-host-credential-resolution", diff --git a/tests/integration/test_download_strategies_phase3w5.py b/tests/integration/test_download_strategies_phase3w5.py index dcea34fad5..c50b8deffa 100644 --- a/tests/integration/test_download_strategies_phase3w5.py +++ b/tests/integration/test_download_strategies_phase3w5.py @@ -11,7 +11,13 @@ import requests from apm_cli.deps.download_strategies import DownloadDelegate +from apm_cli.deps.git_file_transport import GitFileTransportError from apm_cli.deps.github_rate_limit import GitHubThrottleError +from apm_cli.deps.transport_selection import ( + NoOpInsteadOfResolver, + ProtocolPreference, + TransportSelector, +) from apm_cli.models.apm_package import DependencyReference @@ -39,12 +45,18 @@ def make_host( ctx = SimpleNamespace(token=effective_token, source=source, auth_scheme="basic") auth_resolver.resolve.return_value = ctx auth_resolver.resolve_for_dep.return_value = ctx + auth_resolver.resolve_for_remote.return_value = ctx + auth_resolver.git_env_for_remote.return_value = {} + auth_resolver.build_native_git_credential_env.return_value = {} auth_resolver.classify_host.return_value = SimpleNamespace(kind="generic", api_base=api_base) auth_resolver.build_error_context.return_value = ( "Check PAT permissions." if effective_token else "Set a token." ) host.auth_resolver = auth_resolver host._resolve_dep_auth_ctx = MagicMock(return_value=ctx) + host._protocol_pref = ProtocolPreference.NONE + host._allow_fallback = False + host._transport_selector = TransportSelector(NoOpInsteadOfResolver()) return host @@ -495,7 +507,7 @@ def test_success_calls_verbose_callback(self) -> None: with patch( "apm_cli.deps.download_strategies.GitSparseFileTransport", return_value=MagicMock( - fetch_file=MagicMock(side_effect=RuntimeError("git transport unavailable")) + fetch_file=MagicMock(side_effect=GitFileTransportError("git transport unavailable")) ), ): result = delegate.download_gitlab_file(dep, "README.md", verbose_callback=callback) diff --git a/tests/integration/test_download_strategies_selection.py b/tests/integration/test_download_strategies_selection.py index f2dd517270..93631e8502 100644 --- a/tests/integration/test_download_strategies_selection.py +++ b/tests/integration/test_download_strategies_selection.py @@ -13,7 +13,13 @@ import requests from apm_cli.deps.download_strategies import DownloadDelegate +from apm_cli.deps.git_file_transport import GitFileTransportError from apm_cli.deps.github_rate_limit import GitHubThrottleError +from apm_cli.deps.transport_selection import ( + NoOpInsteadOfResolver, + ProtocolPreference, + TransportSelector, +) from apm_cli.models.apm_package import DependencyReference @@ -41,12 +47,18 @@ def make_host( ctx = SimpleNamespace(token=effective_token, source=source, auth_scheme="basic") auth_resolver.resolve.return_value = ctx auth_resolver.resolve_for_dep.return_value = ctx + auth_resolver.resolve_for_remote.return_value = ctx + auth_resolver.git_env_for_remote.return_value = {} + auth_resolver.build_native_git_credential_env.return_value = {} auth_resolver.classify_host.return_value = SimpleNamespace(kind="generic", api_base=api_base) auth_resolver.build_error_context.return_value = ( "Check PAT permissions." if effective_token else "Set a token." ) host.auth_resolver = auth_resolver host._resolve_dep_auth_ctx = MagicMock(return_value=ctx) + host._protocol_pref = ProtocolPreference.NONE + host._allow_fallback = False + host._transport_selector = TransportSelector(NoOpInsteadOfResolver()) return host @@ -544,7 +556,7 @@ def test_success_calls_verbose_callback(self) -> None: with patch( "apm_cli.deps.download_strategies.GitSparseFileTransport", return_value=MagicMock( - fetch_file=MagicMock(side_effect=RuntimeError("git transport unavailable")) + fetch_file=MagicMock(side_effect=GitFileTransportError("git transport unavailable")) ), ): result = delegate.download_gitlab_file(dep, "README.md", verbose_callback=callback) diff --git a/tests/integration/test_gitlab_sparse_transport_contract.py b/tests/integration/test_gitlab_sparse_transport_contract.py new file mode 100644 index 0000000000..8620b83d7d --- /dev/null +++ b/tests/integration/test_gitlab_sparse_transport_contract.py @@ -0,0 +1,267 @@ +"""Real Git materialization with only the external fetch boundary redirected.""" + +from __future__ import annotations + +import os +import socket +import subprocess +import tempfile +import threading +from collections.abc import Iterator +from pathlib import Path +from unittest.mock import Mock, patch +from urllib.parse import urlparse + +import pytest + +from apm_cli.core.auth import AuthResolver +from apm_cli.deps.github_downloader import GitHubPackageDownloader +from apm_cli.models.apm_package import DependencyReference +from apm_cli.utils.path_security import PathTraversalError +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment +from tests.utils.local_git_repository import LocalGitRepositoryFactory + +pytestmark = pytest.mark.component + +_REMOTE = "ssh://git@gitlab-ssh.example.com:2222/owner/repo.git" +_PATHS = ("agents/first.agent.md", "instructions/second.instructions.md") +_OLD = (b"# old agent\r\n\x00\n", b"old instruction\n") +_NEW = (b"# new agent\n\xff\n", b"new instruction\r\n") + + +@pytest.fixture +def materialization(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Iterator[dict]: + """Reuse isolated config/local repositories, never faking checkout or bytes.""" + isolated = IsolatedApmEnvironment.create(tmp_path / "isolated", base_env=os.environ) + env = isolated.subprocess_env() + factory = LocalGitRepositoryFactory(isolated.repository_root, env=env) + repo = factory.create("gitlab") + for path, content in zip(_PATHS, _OLD, strict=True): + target = repo.worktree / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(content) + first = factory.commit(repo, message="first") + factory.tag(repo, "v1", first) + for path, content in zip(_PATHS, _NEW, strict=True): + (repo.worktree / path).write_bytes(content) + second = factory.commit(repo, message="second") + real_run = subprocess.run + observations: list[tuple[str, list[str], dict[str, str]]] = [] + expected = {"remote": _REMOTE} + gate: dict[str, threading.Event] = {} + + def guarded_run(command: list[str], *args: object, **kwargs: object) -> object: + argv = list(command) + assert Path(argv[0]).stem == "git", f"Unexpected subprocess: {argv}" + if "fetch" in argv: + remote = real_run( + [argv[0], "config", "--get", "remote.origin.url"], + cwd=kwargs["cwd"], + env=kwargs["env"], + capture_output=True, + text=True, + check=True, + timeout=10, + ).stdout.strip() + parsed = urlparse(remote) + wanted = urlparse(expected["remote"]) + assert (parsed.scheme, parsed.username, parsed.hostname, parsed.port, parsed.path) == ( + wanted.scheme, + wanted.username, + wanted.hostname, + wanted.port, + wanted.path, + ), "P1/P2 manifest transport must reach the actual Git remote" + assert argv[1:5] == ["fetch", "--filter=blob:none", "--depth=1", "origin"] + assert len(argv) == 6 + assert "glpat-2938-dummy-secret" not in repr((argv, remote, kwargs["env"])), ( + "P7 credentials must not enter Git subprocess" + ) + observations.append((remote, argv.copy(), dict(kwargs["env"]))) + if gate: + gate["entered"].set() + assert gate["release"].wait(10), "fetch release timed out" + argv[4] = repo.file_url + else: + assert any( + operation in argv + for operation in ( + "init", + "config", + "remote", + "sparse-checkout", + "checkout", + "rev-parse", + ) + ) or ("ls-remote" in argv and "--get-url" in argv), f"Network command: {argv}" + return real_run(argv, *args, **kwargs) + + with patch.dict(os.environ, env, clear=True): + monkeypatch.setattr(tempfile, "tempdir", str(isolated.temp_root)) + monkeypatch.setenv("APM_GITLAB_HOSTS", "gitlab-ssh.example.com") + monkeypatch.setenv("GITLAB_APM_PAT", "glpat-2938-dummy-secret") + monkeypatch.setattr(subprocess, "run", guarded_run) + monkeypatch.setattr( + socket, "create_connection", Mock(side_effect=AssertionError("network forbidden")) + ) + monkeypatch.setattr( + socket, "getaddrinfo", Mock(side_effect=AssertionError("DNS forbidden")) + ) + monkeypatch.setattr(socket, "socket", Mock(side_effect=AssertionError("socket forbidden"))) + monkeypatch.setattr( + "requests.sessions.Session.request", + Mock(side_effect=AssertionError("HTTP forbidden")), + ) + owner = GitHubPackageDownloader( + auth_resolver=AuthResolver(allow_external_fallback=False), allow_fallback=False + ) + api = Mock(side_effect=AssertionError("P3/P8 REST must not follow SSH/local Git")) + monkeypatch.setattr(owner, "_resilient_get", api) + yield { + "owner": owner, + "first": first.sha, + "second": second.sha, + "observations": observations, + "api": api, + "gate": gate, + "factory": factory, + "repo": repo, + "expected": expected, + } + owner._strategies._git_file_transport_finalizer() + + +def _fetch(state: dict, path: str, ref: str, url: str = _REMOTE) -> bytes: + """Exercise public downloader routing with the real manifest object.""" + dep = DependencyReference.parse_from_dict({"git": url, "path": path, "type": "gitlab"}) + return state["owner"]._download_github_file(dep, path, ref) + + +@pytest.mark.parametrize("ref_name", ["main", "v1", "sha"]) +def test_real_git_manifest_remote_bytes_and_single_fetch( + materialization: dict, ref_name: str +) -> None: + """P1/P7/P9: actual remote fidelity plus exact branch/tag/SHA materialization.""" + state = materialization + ref = state["first"] if ref_name == "sha" else ref_name + expected = _NEW if ref_name == "main" else _OLD + assert tuple(_fetch(state, path, ref) for path in _PATHS) == expected, ( + "P9 requested ref must materialize exact local Git bytes" + ) + assert len(state["observations"]) == 1 + assert state["observations"][0][1][-1] == ref + transports = list(state["owner"]._strategies._git_file_transports.values()) + assert len(transports) == 1 + checkout = transports[0]._work_dir + assert checkout.is_dir() + state["api"].assert_not_called() + state["owner"]._strategies._git_file_transport_finalizer() + assert not checkout.exists() + + +def test_missing_custom_ref_is_not_replaced(materialization: dict) -> None: + """P9: failure never silently materializes main or master instead.""" + with pytest.raises(RuntimeError, match="missing-release"): + _fetch(materialization, _PATHS[0], "missing-release") + assert [entry[1][-1] for entry in materialization["observations"]] == ["missing-release"] + assert materialization["owner"]._strategies._git_file_transports == {} + materialization["api"].assert_not_called() + + +def test_concurrent_paths_share_one_initial_checkout(materialization: dict) -> None: + """P10: bounded barriers overlap callers without timing-sensitive sleeps.""" + state = materialization + state["gate"].update(entered=threading.Event(), release=threading.Event()) + start = threading.Barrier(3, timeout=10) + results: dict[str, bytes] = {} + failures: list[Exception] = [] + + def worker(path: str) -> None: + try: + start.wait() + results[path] = _fetch(state, path, "main") + except Exception as exc: + failures.append(exc) + + workers = [threading.Thread(target=worker, args=(path,), daemon=True) for path in _PATHS] + for worker_thread in workers: + worker_thread.start() + try: + start.wait() + assert state["gate"]["entered"].wait(10), "no caller reached fetch" + finally: + state["gate"]["release"].set() + for worker_thread in workers: + worker_thread.join(timeout=15) + assert all(not worker_thread.is_alive() for worker_thread in workers) + assert failures == [] + assert results == dict(zip(_PATHS, _NEW, strict=True)) + assert len(state["observations"]) == 1 + assert _fetch(state, _PATHS[0], "main") == _NEW[0] + assert len(state["observations"]) == 1 + state["api"].assert_not_called() + + +def test_traversal_is_terminal_before_fetch(materialization: dict) -> None: + """P6: path validation remains real and cannot advance an opted-in plan.""" + materialization["owner"]._allow_fallback = True + dep = DependencyReference.parse_from_dict({"git": _REMOTE, "type": "gitlab"}) + with pytest.raises(PathTraversalError): + materialization["owner"]._download_github_file(dep, "../outside.md", "main") + assert materialization["observations"] == [] + materialization["api"].assert_not_called() + + +def test_local_read_failure_is_terminal( + materialization: dict, monkeypatch: pytest.MonkeyPatch +) -> None: + """P6: a real materialized file's I/O failure cannot become a Git retry.""" + state = materialization + state["owner"]._allow_fallback = True + read_bytes = Path.read_bytes + + def failed_read(path: Path) -> bytes: + if path.name == "first.agent.md": + raise OSError("simulated unreadable materialized file") + return read_bytes(path) + + monkeypatch.setattr(Path, "read_bytes", failed_read) + with pytest.raises(OSError, match="unreadable materialized"): + _fetch(state, _PATHS[0], "main") + assert len(state["observations"]) == 1 + state["api"].assert_not_called() + + +def test_cached_symlink_escape_is_terminal(materialization: dict) -> None: + """P6: real containment is rechecked before reading even an existing checkout.""" + state = materialization + assert _fetch(state, _PATHS[0], "main") == _NEW[0] + transport = next(iter(state["owner"]._strategies._git_file_transports.values())) + target = transport._work_dir / _PATHS[0] + target.unlink() + try: + target.symlink_to(transport._work_dir.parent / "outside.md") + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlink creation unavailable: {exc}") + state["owner"]._allow_fallback = True + with pytest.raises(PathTraversalError): + _fetch(state, _PATHS[0], "main") + assert len(state["observations"]) == 1 + state["api"].assert_not_called() + + +def test_effective_local_rewrite_does_not_authorize_rest(materialization: dict) -> None: + """P8: real Git rewrite policy sees a local mirror, not nominal HTTPS.""" + state = materialization + url = "https://gitlab.com/owner/repo.git" + state["factory"].install_url_rewrite(state["repo"], url) + state["expected"]["remote"] = url + failure = None + try: + _fetch(state, _PATHS[0], "missing-release", url=url) + except Exception as exc: + failure = exc + assert state["api"].call_count == 0, "P8 effective rewrite must not authorize REST" + assert isinstance(failure, RuntimeError), "P8 effective rewrite must preserve Git failure" + assert "missing-release" in str(failure) + assert len(state["observations"]) == 1 diff --git a/tests/integration/test_gitlab_sparse_transport_mutations.py b/tests/integration/test_gitlab_sparse_transport_mutations.py new file mode 100644 index 0000000000..7e3387a05e --- /dev/null +++ b/tests/integration/test_gitlab_sparse_transport_mutations.py @@ -0,0 +1,318 @@ +"""Bounded behavioral mutations run only against disposable candidate copies. + +The static M9 ownership mutation lives in test_architecture_gitlab_sparse_transport. +These component tests reuse the contract assertions rather than replacing Git +materialization with mutation-specific fake behavior. +""" + +from __future__ import annotations + +import ast +import os +import shutil +import subprocess +import sys +import xml.etree.ElementTree as ET +from collections.abc import Iterator +from dataclasses import dataclass +from pathlib import Path + +import pytest + +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment + +pytestmark = pytest.mark.component + +ROOT = Path(__file__).resolve().parents[2] +SOURCE = Path("src/apm_cli/deps/download_strategies.py") +UNIT_CONTRACT = "tests/unit/deps/test_gitlab_sparse_transport_contract.py" +GIT_CONTRACT = "tests/integration/test_gitlab_sparse_transport_contract.py" + + +@dataclass(frozen=True) +class Mutation: + """One source mutation and the exact behavioral assertion that must kill it.""" + + name: str + method: str + old: str + new: str + node: str + assertion: str + source: Path = SOURCE + + +MUTATIONS: tuple[Mutation, ...] = ( + Mutation( + "M1-force-https", + "_prepared_repo_url", + "return requested_url", + 'return self.build_repo_url(repo_ref, dep_ref=dep_ref, use_ssh=False, token="")', + f"{GIT_CONTRACT}::test_real_git_manifest_remote_bytes_and_single_fetch[main]", + "manifest transport must reach the actual Git remote", + ), + Mutation( + "M2a-drop-port", + "_prepared_repo_url", + "return requested_url", + 'return requested_url.replace(f":{dep_ref.port}", "")', + f"{GIT_CONTRACT}::test_real_git_manifest_remote_bytes_and_single_fetch[main]", + "manifest transport must reach the actual Git remote", + ), + Mutation( + "M2b-drop-ssh-user", + "_prepared_repo_url", + "return requested_url", + 'return requested_url.replace(f"{dep_ref.ssh_user}@", "")', + f"{GIT_CONTRACT}::test_real_git_manifest_remote_bytes_and_single_fetch[main]", + "manifest transport must reach the actual Git remote", + ), + Mutation( + "M3-unconditional-rest", + "download_gitlab_file", + "if rest_eligible:", + "if True:", + f"{UNIT_CONTRACT}::test_strict_ssh_failure_never_rest[with-pat]", + "P3 strict SSH", + ), + Mutation( + "M4-rewritten-https-authorizes-rest", + "download_gitlab_file", + "effective_url, host_info.api_base\n", + "requested_url, host_info.api_base\n", + f"{GIT_CONTRACT}::test_effective_local_rewrite_does_not_authorize_rest", + "P8 effective rewrite must not authorize REST", + ), + Mutation( + "M5-managed-token-in-ssh-child", + "_run", + "env=git_subprocess_env(self._git_env),", + "env={**git_subprocess_env(self._git_env), " + '"GITLAB_APM_PAT": os.environ.get("GITLAB_APM_PAT", "")},', + f"{GIT_CONTRACT}::test_real_git_manifest_remote_bytes_and_single_fetch[main]", + "P7 credentials", + source=Path("src/apm_cli/deps/git_file_transport.py"), + ), + Mutation( + "M6a-old-reuse-key", + "_git_file_transport_key", + """return ( + provider.kind, + ref, + normalize_repo_url(requested_url), + normalize_repo_url(effective_url), + auth_mode, + )""", + "return (dep_ref.host or default_host(), dep_ref.repo_url, ref, dep_ref.port)", + f"{UNIT_CONTRACT}::test_prepared_identity_separates_checkouts[user]", + "P10 distinct prepared identities must not reuse a checkout", + ), + Mutation( + "M6b-omit-effective-url", + "_git_file_transport_key", + " normalize_repo_url(effective_url),\n", + "", + f"{UNIT_CONTRACT}::test_prepared_identity_separates_checkouts[effective-mirror]", + "P10 distinct prepared identities must not reuse a checkout", + ), + Mutation( + "M6c-omit-auth-mode", + "_git_file_transport_key", + " auth_mode,\n", + "", + f"{UNIT_CONTRACT}::test_prepared_identity_separates_checkouts[auth-mode]", + "P10 distinct prepared identities must not reuse a checkout", + ), + Mutation( + "M7a-security-fallback", + "download_gitlab_file", + "except GitFileTransportError as exc:", + "except (GitFileTransportError, ValueError) as exc:", + f"{UNIT_CONTRACT}::test_non_transport_failures_are_terminal[failure3]", + "P6 non-transport failures must remain terminal", + ), + Mutation( + "M7b-local-io-fallback", + "download_gitlab_file", + "except GitFileTransportError as exc:", + "except (GitFileTransportError, OSError) as exc:", + f"{UNIT_CONTRACT}::test_non_transport_failures_are_terminal[failure1]", + "P6 non-transport failures must remain terminal", + ), + Mutation( + "M8a-bypass-materialization", + "_download_gitlab_file_via_git", + "return transport.fetch_file(file_path)", + 'return b""', + f"{GIT_CONTRACT}::test_real_git_manifest_remote_bytes_and_single_fetch[v1]", + "P9 requested ref must materialize exact local Git bytes", + ), + Mutation( + "M8b-ignore-requested-ref", + "_download_gitlab_file_via_git", + " ref,\n", + ' "main",\n', + f"{GIT_CONTRACT}::test_real_git_manifest_remote_bytes_and_single_fetch[v1]", + "P9 requested ref must materialize exact local Git bytes", + ), + Mutation( + "M10-silent-protocol-switch", + "download_gitlab_file", + " and previous_attempt is not None\n", + " and False\n", + f"{UNIT_CONTRACT}::test_protocol_switch_warning_matches_executed_attempts[True-True-ssh]", + "P11 warn exactly when the executed protocol changes", + ), +) + + +def _mutate(candidate: Path, mutation: Mutation) -> None: + """Require a unique executable seam and modify only the candidate copy.""" + path = candidate / mutation.source + assert not path.samefile(ROOT / mutation.source), "Never mutate the active source checkout" + source = path.read_text(encoding="utf-8") + methods = [ + node + for node in ast.walk(ast.parse(source)) + if isinstance(node, ast.FunctionDef) and node.name == mutation.method + ] + assert len(methods) == 1, f"{mutation.name}: missing/ambiguous mutation method" + method = methods[0] + lines = source.splitlines(keepends=True) + body = "".join(lines[method.lineno - 1 : method.end_lineno]) + assert body.count(mutation.old) == 1, f"{mutation.name}: source seam drifted" + mutated = ( + "".join(lines[: method.lineno - 1]) + + body.replace(mutation.old, mutation.new, 1) + + "".join(lines[method.end_lineno :]) + ) + assert mutated != source, f"{mutation.name}: mutation made no change" + compile(mutated, str(path), "exec") + path.write_text(mutated, encoding="utf-8") + + +def _copy_candidate(destination: Path) -> None: + """Copy bounded production/test inputs without caches or editable imports.""" + destination.mkdir() + ignore = shutil.ignore_patterns("__pycache__", "*.pyc", ".pytest_cache") + for directory in ("src", "tests/utils"): + shutil.copytree(ROOT / directory, destination / directory, ignore=ignore) + for relative in ( + "pyproject.toml", + "tests/__init__.py", + "tests/unit/__init__.py", + "tests/unit/deps/__init__.py", + "tests/integration/__init__.py", + "tests/conftest.py", + "tests/integration/conftest.py", + UNIT_CONTRACT, + GIT_CONTRACT, + ): + target = destination / relative + target.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(ROOT / relative, target) + + +def _run_contracts( + candidate: Path, run_root: Path, nodes: tuple[str, ...] +) -> tuple[subprocess.CompletedProcess[str], list[ET.Element]]: + """Run real pytest under isolated Git/config/network policy and read JUnit.""" + isolated = IsolatedApmEnvironment.create(run_root, base_env=os.environ) + environment = isolated.subprocess_env( + overrides={ + "PYTEST_DISABLE_PLUGIN_AUTOLOAD": "1", + "PYTEST_ADDOPTS": "", + "PYTHONDONTWRITEBYTECODE": "1", + } + ) + environment["PYTHONPATH"] = os.pathsep.join( + (environment["PYTHONPATH"], str(candidate / "src"), str(candidate)) + ) + report = run_root / "results.xml" + bootstrap = ( + "from pathlib import Path; import apm_cli; " + "assert Path(apm_cli.__file__).resolve().is_relative_to(Path.cwd() / 'src'), " + "'mutation imported source outside candidate'; " + "import pytest, sys; raise SystemExit(pytest.main(sys.argv[1:]))" + ) + result = subprocess.run( + [ + sys.executable, + "-B", + "-c", + bootstrap, + "-p", + "no:cacheprovider", + "-o", + "addopts=", + "-q", + "--tb=short", + f"--junitxml={report}", + f"--basetemp={run_root / 'pytest-work'}", + *nodes, + ], + cwd=candidate, + env=environment, + capture_output=True, + text=True, + timeout=120, + check=False, + ) + assert report.is_file(), f"No JUnit evidence:\n{result.stdout}\n{result.stderr}" + # The report is generated by the bounded local pytest child, not external XML. + cases = list(ET.parse(report).iter("testcase")) # noqa: S314 + assert cases, f"No collected behavioral assertions:\n{result.stdout}\n{result.stderr}" + assert not any(case.find("error") is not None for case in cases), ( + f"Setup/import/collection errors do not kill mutants:\n{result.stdout}\n{result.stderr}" + ) + assert not any(case.find("skipped") is not None for case in cases), ( + f"Skipped assertions do not prove mutations:\n{result.stdout}" + ) + return result, cases + + +@pytest.fixture(scope="module") +def passing_candidate(tmp_path_factory: pytest.TempPathFactory) -> Iterator[Path]: + """Freeze and prove the integrated candidate before applying any mutation.""" + root = tmp_path_factory.mktemp("gitlab-mutation-baseline") + candidate = root / "candidate" + _copy_candidate(candidate) + nodes = tuple(dict.fromkeys(mutation.node for mutation in MUTATIONS)) + assert len(MUTATIONS) == 14, "Expected 14 behavioral mutants plus the separate static M9" + result, cases = _run_contracts(candidate, root / "baseline-run", nodes) + assert result.returncode == 0, f"Unmutated candidate failed:\n{result.stdout}\n{result.stderr}" + assert len(cases) == len(nodes), "Baseline did not execute every named assertion" + assert all(case.find("failure") is None for case in cases), "Baseline assertions failed" + yield candidate + shutil.rmtree(root) + + +@pytest.mark.parametrize("mutation", MUTATIONS, ids=lambda mutation: mutation.name) +def test_gitlab_sparse_behavioral_mutation( + passing_candidate: Path, tmp_path: Path, mutation: Mutation +) -> None: + """Count a kill only when the named behavioral assertion fails in JUnit.""" + candidate = tmp_path / "candidate" + shutil.copytree(passing_candidate, candidate) + original = (passing_candidate / mutation.source).read_bytes() + _mutate(candidate, mutation) + result, cases = _run_contracts(candidate, tmp_path / "mutant-run", (mutation.node,)) + assert (passing_candidate / mutation.source).read_bytes() == original, ( + "Baseline source was modified" + ) + assert result.returncode == 1, ( + f"{mutation.name}: survived or infrastructure failed ({result.returncode}):\n" + f"{result.stdout}\n{result.stderr}" + ) + assert len(cases) == 1, f"{mutation.name}: expected exactly one named behavioral assertion" + case = cases[0] + module, name = mutation.node.split("::", 1) + assert case.attrib["classname"] == module.removesuffix(".py").replace("/", ".") + assert case.attrib["name"] == name + failure = case.find("failure") + assert failure is not None, f"{mutation.name}: named assertion did not fail" + evidence = failure.text or "" + message = failure.attrib.get("message", "") + assert message.startswith("AssertionError:") and mutation.assertion in message, ( + f"{mutation.name}: incidental failure is not a behavioral kill:\n{evidence}" + ) diff --git a/tests/spec_conformance/test_gitlab_sparse_transport_reqs.py b/tests/spec_conformance/test_gitlab_sparse_transport_reqs.py new file mode 100644 index 0000000000..3820927acb --- /dev/null +++ b/tests/spec_conformance/test_gitlab_sparse_transport_reqs.py @@ -0,0 +1,22 @@ +"""Bind GitLab sparse materialization to existing port and cache requirements.""" + +import pytest + +from tests.integration.test_gitlab_sparse_transport_contract import ( + materialization as materialization, +) +from tests.integration.test_gitlab_sparse_transport_contract import ( + test_real_git_manifest_remote_bytes_and_single_fetch as _run_sparse_transport_contract, +) + +pytestmark = pytest.mark.component + + +@pytest.mark.req("req-sc-013") +@pytest.mark.req("req-rs-016") +@pytest.mark.parametrize("ref_name", ["main", "v1", "sha"]) +def test_gitlab_sparse_fetch_preserves_port_identity_and_ref( + materialization: dict, ref_name: str +) -> None: + """Exercise non-default port fidelity and same-identity/ref checkout reuse.""" + _run_sparse_transport_contract(materialization, ref_name) diff --git a/tests/test_github_downloader.py b/tests/test_github_downloader.py index b88f7dd2b3..6d37b28801 100644 --- a/tests/test_github_downloader.py +++ b/tests/test_github_downloader.py @@ -13,6 +13,7 @@ import requests as requests_lib from apm_cli.core.auth import AuthResolver +from apm_cli.deps.git_file_transport import GitFileTransportError from apm_cli.deps.github_downloader import GitHubPackageDownloader from apm_cli.deps.github_rate_limit import GitHubThrottleError from apm_cli.models.apm_package import ( @@ -2617,7 +2618,7 @@ def test_object_form_type_gitlab_routes_untrusted_host_without_global_pat(self): patch.object( downloader._strategies, "_download_gitlab_file_via_git", - side_effect=RuntimeError("force REST fallback"), + side_effect=GitFileTransportError("force REST fallback"), ), patch.object(downloader, "_resilient_get", return_value=response) as mock_get, ): @@ -2651,7 +2652,7 @@ def test_object_form_type_gitlab_routes_trusted_host_with_token(self): patch.object( downloader._strategies, "_download_gitlab_file_via_git", - side_effect=RuntimeError("force REST fallback"), + side_effect=GitFileTransportError("force REST fallback"), ), patch.object(downloader, "_resilient_get", return_value=response) as mock_get, ): diff --git a/tests/test_gitlab_git_transport.py b/tests/test_gitlab_git_transport.py index dc3ca33832..a868e2bba0 100644 --- a/tests/test_gitlab_git_transport.py +++ b/tests/test_gitlab_git_transport.py @@ -21,7 +21,9 @@ import pytest +from apm_cli.deps.git_file_transport import GitFileTransportError from apm_cli.models.apm_package import DependencyReference +from apm_cli.utils.git_env import get_git_executable from apm_cli.utils.path_security import PathTraversalError # Patch cred helper so tests never call real git for token resolution. @@ -213,7 +215,7 @@ def fake_run(cmd: list[str], **_kwargs) -> Mock: assert result.content == b"# Agent" assert result.resolved_commit == expected_sha assert any( - call.args[0] == ["git", "rev-parse", "--verify", "FETCH_HEAD^{commit}"] + call.args[0] == [get_git_executable(), "rev-parse", "--verify", "FETCH_HEAD^{commit}"] for call in mock_run.call_args_list ) @@ -769,7 +771,7 @@ def test_gitlab_pat_fallback_when_git_fails(self) -> None: patch( "apm_cli.deps.download_strategies.GitSparseFileTransport", return_value=_mock_git_transport( - side_effect=RuntimeError("git transport failed") + side_effect=GitFileTransportError("git transport failed") ), ), patch.object(downloader, "_resilient_get", return_value=mock_response) as mock_api, @@ -801,7 +803,9 @@ def test_gitlab_pat_absent_and_git_fails_raises(self) -> None: with ( patch( "apm_cli.deps.download_strategies.GitSparseFileTransport", - return_value=_mock_git_transport(side_effect=RuntimeError("SSH auth failed")), + return_value=_mock_git_transport( + side_effect=GitFileTransportError("SSH auth failed") + ), ), patch.object(downloader, "_resilient_get") as mock_api, ): diff --git a/tests/unit/core/test_git_transport_policy.py b/tests/unit/core/test_git_transport_policy.py index 494a43e1ae..4bd247f16e 100644 --- a/tests/unit/core/test_git_transport_policy.py +++ b/tests/unit/core/test_git_transport_policy.py @@ -6,6 +6,7 @@ import os import subprocess from pathlib import Path +from unittest.mock import Mock from urllib.parse import urlsplit import pytest @@ -88,6 +89,43 @@ def _context(kind: str) -> AuthContext: ) +@pytest.mark.parametrize( + "remote_url", + ( + "ssh://git@gitlab.com/group/repo.git", + "git@gitlab.com:group/repo.git", + "file:///local/mirror.git", + "http://gitlab.com/group/repo.git", + ), +) +def test_gitlab_non_https_resolution_never_probes_https_credentials(remote_url: str) -> None: + """GitLab honors the auth owner's no-native-lookup transport policy.""" + manager = _RecordingTokenManager() + resolver = AuthResolver(token_manager=manager) + + context = resolver.resolve_for_remote("gitlab.com", remote_url, "group") + + assert context.token is None + assert manager.credential_envs == [] + + +def test_gitlab_https_resolution_retains_native_credential_lookup( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An admitted HTTPS attempt still resolves credentials for its declared port.""" + manager = _RecordingTokenManager() + lookup = Mock(return_value="helper-token") + monkeypatch.setattr(manager, "resolve_credential_from_git", lookup) + resolver = AuthResolver(token_manager=manager) + + context = resolver.resolve_for_remote( + "gitlab.com", "https://gitlab.com:8443/group/repo.git", "group", port=8443 + ) + + assert context.token == "helper-token" + lookup.assert_called_once_with("gitlab.com", port=8443) + + @pytest.mark.parametrize( "effective_url", ( diff --git a/tests/unit/deps/test_download_strategies_phase3.py b/tests/unit/deps/test_download_strategies_phase3.py index 291ca03a24..b955e83390 100644 --- a/tests/unit/deps/test_download_strategies_phase3.py +++ b/tests/unit/deps/test_download_strategies_phase3.py @@ -34,6 +34,12 @@ import requests from apm_cli.deps.download_strategies import DownloadDelegate, _debug +from apm_cli.deps.git_file_transport import GitFileTransportError +from apm_cli.deps.transport_selection import ( + NoOpInsteadOfResolver, + ProtocolPreference, + TransportSelector, +) from apm_cli.models.apm_package import DependencyReference from apm_cli.utils.archive import safe_extract_zip @@ -1250,6 +1256,19 @@ def test_basic_context_uses_ado_pat_header(self) -> None: class TestDownloadGitlabFile: + @pytest.fixture(autouse=True) + def _git_unavailable(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Exercise REST recovery after a selected HTTPS Git attempt fails.""" + monkeypatch.setattr( + DownloadDelegate, + "_download_gitlab_file_via_git", + MagicMock(side_effect=GitFileTransportError("git transport unavailable")), + ) + monkeypatch.setattr( + "apm_cli.deps.download_strategies.validate_git_url_rewrite_safety", + lambda requested_url, _env: requested_url, + ) + def _dep(self) -> DependencyReference: return DependencyReference( repo_url="mygroup/myproject", @@ -1258,11 +1277,18 @@ def _dep(self) -> DependencyReference: def _setup_host_info(self, host_mock: MagicMock) -> None: info = MagicMock() + info.kind = "gitlab" info.api_base = "https://gitlab.example.com/api/v4" host_mock.auth_resolver.classify_host.return_value = info ctx = MagicMock() ctx.token = "gl-tok" host_mock.auth_resolver.resolve.return_value = ctx + host_mock.auth_resolver.resolve_for_remote.return_value = ctx + host_mock.auth_resolver.git_env_for_remote.return_value = {} + host_mock.auth_resolver.build_native_git_credential_env.return_value = {} + host_mock._protocol_pref = ProtocolPreference.NONE + host_mock._allow_fallback = False + host_mock._transport_selector = TransportSelector(NoOpInsteadOfResolver()) def test_success_returns_content(self) -> None: host = _make_host() @@ -1336,12 +1362,8 @@ def test_404_fallback_also_fails_raises(self) -> None: def test_401_without_token_includes_context(self) -> None: host = _make_host() - info = MagicMock() - info.api_base = "https://gitlab.example.com/api/v4" - host.auth_resolver.classify_host.return_value = info - ctx = MagicMock() - ctx.token = None # no token - host.auth_resolver.resolve.return_value = ctx + self._setup_host_info(host) + host.auth_resolver.resolve.return_value.token = None host.auth_resolver.build_error_context.return_value = "Set GITLAB_APM_PAT." d = DownloadDelegate(host) @@ -1405,17 +1427,9 @@ def test_verbose_callback_called_on_success(self) -> None: host._resilient_get.return_value = resp callback = MagicMock() - with ( - patch( - "apm_cli.deps.download_strategies.AuthResolver.gitlab_rest_headers", - return_value={}, - ), - patch( - "apm_cli.deps.download_strategies.GitSparseFileTransport", - return_value=MagicMock( - fetch_file=MagicMock(side_effect=RuntimeError("git transport unavailable")) - ), - ), + with patch( + "apm_cli.deps.download_strategies.AuthResolver.gitlab_rest_headers", + return_value={}, ): d.download_gitlab_file(self._dep(), "apm.yml", verbose_callback=callback) # The mocked git failure drives the REST fallback path without spawning diff --git a/tests/unit/deps/test_download_strategies_selection.py b/tests/unit/deps/test_download_strategies_selection.py index 8c42dec5a5..56e6c7cdaa 100644 --- a/tests/unit/deps/test_download_strategies_selection.py +++ b/tests/unit/deps/test_download_strategies_selection.py @@ -26,6 +26,7 @@ import json import threading import zipfile +from collections.abc import Iterator from concurrent.futures import ThreadPoolExecutor from pathlib import Path from types import SimpleNamespace @@ -36,6 +37,12 @@ import requests from apm_cli.deps.download_strategies import DownloadDelegate, _debug +from apm_cli.deps.git_file_transport import GitFileTransportError +from apm_cli.deps.transport_selection import ( + NoOpInsteadOfResolver, + ProtocolPreference, + TransportSelector, +) from apm_cli.models.apm_package import DependencyReference from apm_cli.utils.archive import safe_extract_zip from apm_cli.utils.path_security import PathTraversalError @@ -1013,6 +1020,21 @@ def test_no_ado_token_no_auth_header(self) -> None: class TestDownloadGitlabFile: + @pytest.fixture(autouse=True) + def _isolate_sparse_git(self) -> Iterator[None]: + """Drive REST-only cases with a typed Git failure, never external Git.""" + with ( + patch( + "apm_cli.deps.git_file_transport.GitSparseFileTransport._run", + side_effect=GitFileTransportError("Git fetch failed"), + ), + patch( + "apm_cli.deps.download_strategies.validate_git_url_rewrite_safety", + return_value=None, + ), + ): + yield + def _dep(self) -> DependencyReference: return DependencyReference( repo_url="mygroup/myproject", @@ -1021,11 +1043,19 @@ def _dep(self) -> DependencyReference: def _setup_host_info(self, host_mock: MagicMock) -> None: info = MagicMock() + info.kind = "gitlab" info.api_base = "https://gitlab.example.com/api/v4" host_mock.auth_resolver.classify_host.return_value = info ctx = MagicMock() ctx.token = "gl-tok" + ctx.auth_scheme = "basic" host_mock.auth_resolver.resolve.return_value = ctx + host_mock.auth_resolver.resolve_for_remote.return_value = ctx + host_mock.auth_resolver.git_env_for_remote.return_value = {} + host_mock.auth_resolver.build_native_git_credential_env.return_value = {} + host_mock._protocol_pref = ProtocolPreference.NONE + host_mock._allow_fallback = False + host_mock._transport_selector = TransportSelector(NoOpInsteadOfResolver()) def test_success_returns_content(self) -> None: host = _make_host() @@ -1099,7 +1129,9 @@ def test_404_fallback_also_fails_raises(self) -> None: def test_401_without_token_includes_context(self) -> None: host = _make_host() + self._setup_host_info(host) info = MagicMock() + info.kind = "gitlab" info.api_base = "https://gitlab.example.com/api/v4" host.auth_resolver.classify_host.return_value = info ctx = MagicMock() @@ -1167,7 +1199,7 @@ def test_verbose_callback_called_on_success(self, monkeypatch: pytest.MonkeyPatc resp = _fake_response(200, b"data") host._resilient_get.return_value = resp callback = MagicMock() - git_error = RuntimeError( + git_error = GitFileTransportError( "fatal: could not read from https://oauth2:secret@gitlab.example.com/group/repo.git" ) monkeypatch.setenv("APM_DEBUG", "1") diff --git a/tests/unit/deps/test_github_downloader_gitlab_routing.py b/tests/unit/deps/test_github_downloader_gitlab_routing.py index 2817ac2f68..6399d3c824 100644 --- a/tests/unit/deps/test_github_downloader_gitlab_routing.py +++ b/tests/unit/deps/test_github_downloader_gitlab_routing.py @@ -5,6 +5,7 @@ from urllib.parse import urlparse from apm_cli.core.auth import AuthResolver +from apm_cli.deps.git_file_transport import GitFileTransportError from apm_cli.deps.github_downloader import GitHubPackageDownloader from apm_cli.models.apm_package import DependencyReference @@ -24,7 +25,7 @@ def _download_from_bespoke_gitlab_host(env: dict[str, str]) -> tuple[str, dict[s patch.object( downloader._strategies, "_download_gitlab_file_via_git", - side_effect=RuntimeError("force REST fallback"), + side_effect=GitFileTransportError("force REST fallback"), ), patch.object(downloader, "_resilient_get", return_value=response) as mock_get, ): diff --git a/tests/unit/deps/test_gitlab_sparse_transport_contract.py b/tests/unit/deps/test_gitlab_sparse_transport_contract.py new file mode 100644 index 0000000000..567e20c05d --- /dev/null +++ b/tests/unit/deps/test_gitlab_sparse_transport_contract.py @@ -0,0 +1,458 @@ +"""Hermetic protocol, authentication, and reuse contracts for issue #2938.""" + +from __future__ import annotations + +import os +import socket +import subprocess +import tempfile +from collections.abc import Iterator +from pathlib import Path +from unittest.mock import Mock, patch +from urllib.parse import parse_qs, urlparse + +import pytest + +from apm_cli.core.auth import AuthResolver +from apm_cli.deps.git_file_transport import ( + GitFileTransportError, + GitFileTransportSecurityError, +) +from apm_cli.deps.github_downloader import GitHubPackageDownloader +from apm_cli.deps.transport_selection import ProtocolPreference +from apm_cli.models.apm_package import DependencyReference +from apm_cli.utils.path_security import PathTraversalError +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment + +pytestmark = pytest.mark.component + +_TOKEN = "glpat-2938-dummy-secret" +_REPORTED_URL = "ssh://git@gitlab-ssh.example.com:2222/owner/repo.git" + + +def _dep(url: str = _REPORTED_URL) -> DependencyReference: + """Parse the reported object form without replacing parser or URL builders.""" + return DependencyReference.parse_from_dict( + {"git": url, "path": "agents/spec.agent.md", "type": "gitlab"} + ) + + +def _components(url: str) -> tuple[str, str | None, str | None, int | None, str]: + """Compare SCP and URL forms using parsed components.""" + if "://" not in url: + authority, path = url.split(":", 1) + url = f"ssh://{authority}/{path}" + parsed = urlparse(url) + return parsed.scheme, parsed.username, parsed.hostname, parsed.port, parsed.path + + +@pytest.fixture +def isolated(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Iterator[None]: + """Isolate credentials/config and reject network-capable subprocesses.""" + env = IsolatedApmEnvironment.create(tmp_path / "isolated", base_env=os.environ) + real_run = subprocess.run + + def local_only(command: list[str], *args: object, **kwargs: object) -> object: + if Path(command[0]).stem != "git" or not ( + "config" in command or ("ls-remote" in command and "--get-url" in command) + ): + raise AssertionError(f"Unexpected subprocess: {command}") + return real_run(command, *args, **kwargs) + + with patch.dict(os.environ, env.subprocess_env(), clear=True): + monkeypatch.setattr(tempfile, "tempdir", str(env.temp_root)) + monkeypatch.setenv("APM_GITLAB_HOSTS", "gitlab-ssh.example.com") + monkeypatch.setattr(subprocess, "run", local_only) + monkeypatch.setattr( + socket, "create_connection", Mock(side_effect=AssertionError("network forbidden")) + ) + monkeypatch.setattr( + socket, "getaddrinfo", Mock(side_effect=AssertionError("DNS forbidden")) + ) + monkeypatch.setattr(socket, "socket", Mock(side_effect=AssertionError("socket forbidden"))) + monkeypatch.setattr( + "requests.sessions.Session.request", + Mock(side_effect=AssertionError("HTTP forbidden")), + ) + yield + + +@pytest.fixture +def downloader(isolated: None) -> Iterator[GitHubPackageDownloader]: + """Use real policy/auth owners, but replace sparse materialization at its seam.""" + owner = GitHubPackageDownloader( + auth_resolver=AuthResolver(allow_external_fallback=False), + allow_fallback=False, + ) + yield owner + owner._strategies._git_file_transport_finalizer() + + +def _capture(owner: GitHubPackageDownloader, outcomes: list[object]) -> tuple[list, Mock]: + """Capture each prepared remote/environment while supplying bounded outcomes.""" + attempts: list = [] + + def factory(dep: DependencyReference, ref: str, **kwargs: object) -> Mock: + remote = kwargs["build_repo_url_fn"](dep.repo_url, dep_ref=dep) + attempts.append((remote, kwargs["git_env"], ref)) + outcome = outcomes[len(attempts) - 1] + transport = Mock() + transport.fetch_file.side_effect = outcome if isinstance(outcome, Exception) else None + transport.fetch_file.return_value = outcome + return transport + + owner._strategies._git_file_transport_factory = factory + response = Mock(content=b"REST", status_code=200) + api = Mock(return_value=response) + owner._resilient_get = api + return attempts, api + + +@pytest.mark.parametrize( + ("url", "preference", "expected"), + [ + (_REPORTED_URL, "https", ("ssh", "git", "gitlab-ssh.example.com", 2222, "/owner/repo.git")), + ( + "git@gitlab.com:group/repo.git", + "https", + ("ssh", "git", "gitlab.com", None, "/group/repo.git"), + ), + ( + "ssh://git@gitlab.com/group/repo.git", + "https", + ("ssh", "git", "gitlab.com", None, "/group/repo.git"), + ), + ( + "ssh://alice@gitlab.com:2200/group/sub/repo.git", + "https", + ("ssh", "alice", "gitlab.com", 2200, "/group/sub/repo.git"), + ), + ( + "https://gitlab.com/group/repo.git", + "ssh", + ("https", None, "gitlab.com", None, "/group/repo.git"), + ), + ( + "http://gitlab.com/group/repo.git", + "ssh", + ("http", None, "gitlab.com", None, "/group/repo.git"), + ), + ("gitlab.com/group/repo", "ssh", ("ssh", "git", "gitlab.com", None, "/group/repo.git")), + ("gitlab.com/group/repo", "https", ("https", None, "gitlab.com", None, "/group/repo.git")), + ], + ids=[ + "reported", + "scp", + "ssh-default", + "custom-user", + "https-explicit", + "http-explicit", + "ssh-preference", + "https-preference", + ], +) +def test_manifest_transport_components( + downloader: GitHubPackageDownloader, url: str, preference: str, expected: tuple +) -> None: + """P1/P2: manifest intent wins over the shorthand preference.""" + downloader._protocol_pref = ProtocolPreference.from_str(preference) + downloader.auth_resolver = AuthResolver(allow_external_fallback=True) + attempts, api = _capture(downloader, [b"Git"]) + with patch( + "apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git", + return_value=None, + ) as credential_fill: + assert downloader._download_github_file(_dep(url), "agents/spec.agent.md", "v1") == b"Git" + assert _components(attempts[0][0]) == expected, "P1/P2 manifest remote fidelity" + assert attempts[0][2] == "v1" + if expected[0] in ("ssh", "http"): + credential_fill.assert_not_called() + api.assert_not_called() + + +@pytest.mark.parametrize("token", ["", _TOKEN], ids=["without-pat", "with-pat"]) +def test_strict_ssh_failure_never_rest( + downloader: GitHubPackageDownloader, monkeypatch: pytest.MonkeyPatch, token: str +) -> None: + """P3/P7: PAT presence cannot authorize an SSH-to-REST downgrade.""" + monkeypatch.setenv("GITLAB_APM_PAT", token) + failure = GitFileTransportError( + f"SSH authentication rejected https://oauth2:{_TOKEN}@gitlab.com/owner/repo.git" + ) + attempts, api = _capture(downloader, [failure]) + raised = None + try: + downloader._download_github_file(_dep(), "agents/spec.agent.md", "release/one") + except RuntimeError as exc: + raised = exc + assert raised is not None, "P3 strict SSH must fail without REST" + assert raised.__cause__ is failure + assert "agents/spec.agent.md" in str(raised) + assert "release/one" in str(raised) + assert "SSH" in str(raised) + assert _TOKEN not in str(raised), "P7 failure diagnostics must redact credentials" + assert len(attempts) == 1 + assert _components(attempts[0][0])[0] == "ssh" + api.assert_not_called() + + +@pytest.mark.parametrize("scheme", ["ssh", "http"]) +def test_unmanaged_transports_strip_pat( + downloader: GitHubPackageDownloader, monkeypatch: pytest.MonkeyPatch, scheme: str +) -> None: + """P7: plaintext/SSH children inherit neither PATs nor managed auth headers.""" + monkeypatch.setenv("GITLAB_APM_PAT", _TOKEN) + attempts, api = _capture(downloader, [b"Git"]) + dep = _dep(f"{scheme}://gitlab.com/group/repo.git") + assert downloader._download_github_file(dep, "agents/spec.agent.md", "main") == b"Git" + remote, env, _ = attempts[0] + assert urlparse(remote).password is None + assert _TOKEN not in repr((remote, env)), "P7 credentials must not enter SSH/HTTP attempts" + assert not {"GIT_TOKEN", "GITLAB_APM_PAT", "GITLAB_TOKEN"} & env.keys(), ( + "P7 child token variables must be removed" + ) + assert all( + env.get(f"GIT_CONFIG_VALUE_{index}", "") == "" + for index in range(int(env.get("GIT_CONFIG_COUNT", "0"))) + if "extraheader" in env.get(f"GIT_CONFIG_KEY_{index}", "").lower() + ) + api.assert_not_called() + + +@pytest.mark.parametrize("successful_attempt", [0, 1, None]) +def test_opt_in_fallback_order_port_and_warning( + downloader: GitHubPackageDownloader, successful_attempt: int | None +) -> None: + """P4: opt-in preserves port, short-circuits success, and warns once.""" + downloader._allow_fallback = True + outcomes = [GitFileTransportError("unavailable"), GitFileTransportError("unavailable")] + if successful_attempt is not None: + outcomes[successful_attempt] = b"Git" + attempts, api = _capture(downloader, outcomes) + with patch("apm_cli.deps.download_strategies._rich_warning") as warning: + result = downloader._download_github_file(_dep(), "agents/spec.agent.md", "main") + assert result == (b"REST" if successful_attempt is None else b"Git") + if successful_attempt == 0: + assert downloader._download_github_file(_dep(), "other.md", "main") == b"Git" + assert [_components(remote)[0] for remote, _, _ in attempts] == ( + ["ssh"] if successful_attempt == 0 else ["ssh", "https"] + ) + assert {_components(remote)[2:4] for remote, _, _ in attempts} == { + ("gitlab-ssh.example.com", 2222) + } + notices = [call.args[0] for call in warning.call_args_list] + assert len([notice for notice in notices if notice.startswith("Custom port ")]) == 1 + assert len(notices) == (1 if successful_attempt == 0 else 2) + assert api.call_count == (1 if successful_attempt is None else 0) + + +@pytest.mark.parametrize("scheme", ["ssh", "https"]) +@pytest.mark.parametrize("allow_fallback", [False, True]) +@pytest.mark.parametrize("first_fails", [False, True]) +def test_protocol_switch_warning_matches_executed_attempts( + downloader: GitHubPackageDownloader, scheme: str, allow_fallback: bool, first_fails: bool +) -> None: + """Warn only after failure actually advances an opt-in cross-protocol attempt.""" + downloader._allow_fallback = allow_fallback + first = GitFileTransportError("unavailable") if first_fails else b"Git" + attempts, api = _capture(downloader, [first, b"Git"]) + dep = _dep(f"{scheme}://gitlab.com/group/repo.git") + with patch("apm_cli.deps.download_strategies._rich_warning") as warning: + if first_fails and not allow_fallback: + if scheme == "ssh": + with pytest.raises(RuntimeError, match="REST is not authorized"): + downloader._download_github_file(dep, "agents/spec.agent.md", "main") + else: + assert downloader._download_github_file(dep, "agents/spec.agent.md") == b"REST" + else: + assert downloader._download_github_file(dep, "agents/spec.agent.md") == b"Git" + switched = first_fails and allow_fallback + assert len(attempts) == (2 if switched else 1) + messages = [call.args[0] for call in warning.call_args_list] + labels = ("SSH", "plain HTTPS") if scheme == "ssh" else ("plain HTTPS", "SSH") + expected = ( + [ + f"Protocol fallback: {labels[0]} GitLab sparse fetch of group/repo " + f"failed; retrying with {labels[1]}." + ] + if switched + else [] + ) + assert messages == expected, "P11 warn exactly when the executed protocol changes" + assert api.call_count == int(first_fails and not allow_fallback and scheme == "https") + + +def test_https_rest_preserves_headers_endpoint_and_ref( + downloader: GitHubPackageDownloader, monkeypatch: pytest.MonkeyPatch +) -> None: + """P5/P7: an admitted HTTPS failure retains scoped REST compatibility.""" + monkeypatch.setenv("GITLAB_APM_PAT", _TOKEN) + attempts, api = _capture(downloader, [GitFileTransportError("unavailable")] * 2) + dep = _dep("https://gitlab.com/group/sub/repo.git") + assert downloader._download_github_file(dep, "agents/a b.md", "release/one") == b"REST" + parsed = urlparse(api.call_args.args[0]) + assert (parsed.scheme, parsed.hostname, parsed.port) == ("https", "gitlab.com", None) + assert ( + parsed.path == "/api/v4/projects/group%2Fsub%2Frepo/repository/files/agents%2Fa%20b.md/raw" + ) + assert parse_qs(parsed.query) == {"ref": ["release/one"]} + assert api.call_args.kwargs["headers"]["PRIVATE-TOKEN"] == _TOKEN + assert urlparse(attempts[0][0]).username is None + assert any( + value.startswith("Authorization: Basic ") + for key, value in attempts[0][1].items() + if key.startswith("GIT_CONFIG_VALUE_") + ) + + +def test_http_failure_has_no_rest(downloader: GitHubPackageDownloader) -> None: + """P5: explicit HTTP never silently upgrades to HTTPS REST.""" + attempts, api = _capture(downloader, [GitFileTransportError("unavailable")]) + with pytest.raises(RuntimeError): + downloader._download_github_file(_dep("http://gitlab.com/g/r.git"), "x.md", "main") + assert [_components(remote)[0] for remote, _, _ in attempts] == ["http"] + api.assert_not_called() + + +@pytest.mark.parametrize( + "failure", + [ + RuntimeError("programming defect"), + OSError("local disk failure"), + PathTraversalError("escape"), + GitFileTransportSecurityError("unsafe ref"), + ], +) +def test_non_transport_failures_are_terminal( + downloader: GitHubPackageDownloader, failure: Exception +) -> None: + """P6: only a typed Git failure may advance even an opted-in plan.""" + downloader._allow_fallback = True + attempts, api = _capture(downloader, [failure, b"unauthorized retry"]) + raised = None + try: + downloader._download_github_file(_dep(), "x.md", "main") + except Exception as exc: + raised = exc + assert raised is failure, "P6 non-transport failures must remain terminal" + assert len(attempts) == 1, "P6 non-transport failures must not advance the Git plan" + api.assert_not_called() + + +def test_cache_reuse_revalidates_policy(downloader: GitHubPackageDownloader) -> None: + """P8: a previously successful checkout cannot bypass rewrite validation.""" + attempts, api = _capture(downloader, [b"Git"]) + assert downloader._download_github_file(_dep(), "first.md", "main") == b"Git" + with patch( + "apm_cli.deps.download_strategies.validate_git_url_rewrite_safety", + side_effect=GitFileTransportSecurityError("unsafe rewrite"), + ): + with pytest.raises(GitFileTransportSecurityError, match="unsafe rewrite"): + downloader._download_github_file(_dep(), "second.md", "main") + assert len(attempts) == 1 + api.assert_not_called() + + +@pytest.mark.parametrize( + "rewrite", + ["ssh://mirror@gitlab.com:2222/owner/repo.git", "file:///local/repo.git"], + ids=["ssh-mirror", "local-mirror"], +) +@pytest.mark.parametrize("token", ["", _TOKEN], ids=["without-pat", "with-pat"]) +def test_effective_rewrite_never_authorizes_rest( + downloader: GitHubPackageDownloader, + monkeypatch: pytest.MonkeyPatch, + rewrite: str, + token: str, +) -> None: + """P8: real insteadOf probing cannot turn nominal HTTPS into REST permission.""" + url = "https://gitlab.com/owner/repo.git" + monkeypatch.setenv("GITLAB_APM_PAT", token) + downloader.auth_resolver = AuthResolver(allow_external_fallback=True) + subprocess.run( + ["git", "config", "--global", f"url.{rewrite}.insteadOf", url], + check=True, + capture_output=True, + timeout=10, + ) + attempts, api = _capture(downloader, [GitFileTransportError("mirror unavailable")]) + with patch( + "apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git", + return_value=None, + ) as credential_fill: + with pytest.raises(RuntimeError, match="mirror unavailable"): + downloader._download_github_file(_dep(url), "x.md", "main") + credential_fill.assert_not_called() + assert len(attempts) == 1 + assert _components(attempts[0][0]) == _components(url) + assert _TOKEN not in repr(attempts[0][1]) + api.assert_not_called() + + +def test_rewrite_probe_error_is_terminal(downloader: GitHubPackageDownloader) -> None: + """P6: a failed local rewrite probe must not authorize any transport.""" + downloader._allow_fallback = True + attempts, api = _capture(downloader, []) + with patch.object( + downloader._transport_selector._resolver, + "resolve", + side_effect=RuntimeError("rewrite probe failed"), + ): + with pytest.raises(RuntimeError, match="rewrite probe failed"): + downloader._download_github_file(_dep(), "x.md", "main") + assert attempts == [] + api.assert_not_called() + + +@pytest.mark.parametrize( + ("other_url", "other_ref", "other_effective", "other_auth"), + [ + ("ssh://alice@gitlab.com/g/r.git", "main", None, "native"), + ("https://gitlab.com/g/r.git", "main", None, "native"), + ("ssh://git@gitlab.com:2222/g/r.git", "main", None, "native"), + ("ssh://git@gitlab.com/g/r.git", "release", None, "native"), + ("ssh://git@gitlab.com/g/r.git", "main", "file:///mirror/repo.git", "native"), + ("ssh://git@gitlab.com/g/r.git", "main", None, "managed"), + ], + ids=["user", "protocol", "port", "ref", "effective-mirror", "auth-mode"], +) +def test_prepared_identity_separates_checkouts( + downloader: GitHubPackageDownloader, + other_url: str, + other_ref: str, + other_effective: str | None, + other_auth: str, +) -> None: + """P10: only identical prepared attempts share a live sparse transport.""" + attempts, _ = _capture(downloader, [b"one", b"two"]) + delegate = downloader._strategies + url = "ssh://git@gitlab.com/g/r.git" + common = dict(requested_url=url, effective_url=url, git_env={}, auth_mode="native") + assert delegate._download_gitlab_file_via_git(_dep(url), "a", "main", **common) == b"one" + assert delegate._download_gitlab_file_via_git(_dep(url), "b", "main", **common) == b"one" + assert ( + delegate._download_gitlab_file_via_git( + _dep(other_url), + "c", + other_ref, + requested_url=other_url, + effective_url=other_effective or other_url, + git_env={}, + auth_mode=other_auth, + ) + == b"two" + ), "P10 distinct prepared identities must not reuse a checkout" + assert len(attempts) == 2, "P10 distinct prepared identities need separate transports" + assert len(delegate._git_file_transports) == 2 + + +def test_failed_eviction_preserves_replacement(downloader: GitHubPackageDownloader) -> None: + """P10: a delayed failed caller cannot evict a newer transport at its key.""" + delegate = downloader._strategies + key = delegate._git_file_transport_key(_dep(), "main", _REPORTED_URL, _REPORTED_URL, "native") + failed, replacement = Mock(), Mock() + delegate._git_file_transports[key] = replacement + delegate._discard_git_file_transport(key, failed) + assert delegate._git_file_transports[key] is replacement + failed.close.assert_called_once() + replacement.close.assert_not_called() diff --git a/tests/unit/deps/test_initial_transport_scheme.py b/tests/unit/deps/test_initial_transport_scheme.py new file mode 100644 index 0000000000..4fac73a045 --- /dev/null +++ b/tests/unit/deps/test_initial_transport_scheme.py @@ -0,0 +1,125 @@ +"""Pure initial-scheme and shared custom-port warning contracts.""" + +from types import SimpleNamespace +from urllib.parse import urlsplit + +import pytest + +from apm_cli.deps.transport_selection import ( + NoOpInsteadOfResolver, + ProtocolPreference, + TransportAttempt, + TransportPlan, + TransportSelector, + fallback_port_warning, + initial_transport_scheme, +) +from apm_cli.models.dependency.reference import DependencyReference + +pytestmark = pytest.mark.unit + + +@pytest.mark.parametrize( + ("spec", "explicit_scheme"), + [ + ("git@gitlab.com:group/subgroup/repo.git", "ssh"), + ("ssh://git@gitlab.com/group/repo.git", "ssh"), + ("ssh://deploy@gitlab.example:2222/group/subgroup/repo.git", "ssh"), + ("https://gitlab.example:8443/group/repo.git", "https"), + ("http://gitlab.example:8080/group/repo.git", "http"), + ("gitlab.com/group/repo", None), + ("owner/repo", None), + ("https://github.com/owner/repo", "https"), + ("https://dev.azure.com/org/project/_git/repo", "https"), + ], +) +@pytest.mark.parametrize("preference", list(ProtocolPreference)) +@pytest.mark.parametrize("has_token", [False, True]) +def test_initial_scheme_matches_selector( + spec: str, + explicit_scheme: str | None, + preference: ProtocolPreference, + has_token: bool, +) -> None: + dep_ref = DependencyReference.parse(spec) + expected = explicit_scheme or ("ssh" if preference is ProtocolPreference.SSH else "https") + + assert initial_transport_scheme(dep_ref, preference) == expected + plan = TransportSelector(NoOpInsteadOfResolver()).select( + dep_ref, + cli_pref=preference, + has_token=has_token, + ) + assert plan.strict is True + assert len(plan.attempts) == 1 + assert plan.attempts[0].scheme == expected + assert plan.attempts[0].use_token is (has_token and expected == "https") + + +@pytest.mark.parametrize( + ("preference", "expected"), + [ + (ProtocolPreference.NONE, "https"), + (ProtocolPreference.HTTPS, "https"), + (ProtocolPreference.SSH, "ssh"), + ], +) +def test_missing_reference_uses_preference(preference: ProtocolPreference, expected: str) -> None: + assert initial_transport_scheme(None, preference) == expected + + +def test_explicit_scheme_is_case_normalized() -> None: + dep_ref = SimpleNamespace(explicit_scheme="HTTPS") + assert initial_transport_scheme(dep_ref, ProtocolPreference.SSH) == "https" + + +@pytest.mark.parametrize("schemes", [("ssh", "https"), ("https", "ssh")]) +def test_custom_port_warning_preserves_text_and_docs_link(schemes: tuple[str, str]) -> None: + dep_ref = DependencyReference.parse("ssh://deploy@gitlab.example:2222/group/repo.git") + plan = TransportPlan( + attempts=[TransportAttempt(scheme, False, scheme) for scheme in schemes], + strict=False, + ) + + warning = fallback_port_warning(dep_ref, plan) + + assert warning is not None + body, docs_url = warning.rsplit("See: ", 1) + assert body == ( + f"Custom port 2222 on {dep_ref.host}/{dep_ref.repo_url}: " + f"if {schemes[0].upper()} fails, APM will retry over " + f"{schemes[1].upper()} on the same port.\n" + " Disable protocol fallback in CLI flags, environment, " + "and saved config to fail fast.\n" + " " + ) + parsed = urlsplit(docs_url) + assert (parsed.scheme, parsed.hostname, parsed.path, parsed.fragment) == ( + "https", + "microsoft.github.io", + "/apm/consumer/manage-dependencies/", + "transport-selection", + ) + + +@pytest.mark.parametrize( + ("spec", "strict", "schemes"), + [ + (None, False, ("ssh", "https")), + ("ssh://git@gitlab.example/group/repo", False, ("ssh", "https")), + ("ssh://git@gitlab.example:2222/group/repo", True, ("ssh", "https")), + ("ssh://git@gitlab.example:2222/group/repo", False, ("ssh",)), + ("ssh://git@gitlab.example:2222/group/repo", False, ("https", "https")), + ("ssh://git@gitlab.example:2222/group/repo", False, ("http", "ssh")), + ("ssh://git@gitlab.example:2222/group/repo", False, ()), + ], +) +def test_no_warning_without_custom_port_cross_protocol_plan( + spec: str | None, strict: bool, schemes: tuple[str, ...] +) -> None: + dep_ref = DependencyReference.parse(spec) if spec else None + plan = TransportPlan( + attempts=[TransportAttempt(scheme, False, scheme) for scheme in schemes], + strict=strict, + ) + assert fallback_port_warning(dep_ref, plan) is None diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index 59f7b4ed61..9c560ff266 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -719,6 +719,7 @@ def exiting_import( transport-platform-git-child-environment transport-platform-git-semver-preflight transport-platform-github-throttle +transport-platform-gitlab-sparse-plan transport-platform-host-credential-resolution transport-platform-host-reference-coordinates transport-platform-network-host-parsing diff --git a/tests/unit/test_protocol_fallback_warning.py b/tests/unit/test_protocol_fallback_warning.py index 5b7816186c..7cbe6d4d22 100644 --- a/tests/unit/test_protocol_fallback_warning.py +++ b/tests/unit/test_protocol_fallback_warning.py @@ -8,14 +8,15 @@ serve SSH and HTTPS on different ports (e.g. Bitbucket Datacenter: SSH 7999, HTTPS 7990). The warning names the offending dependency (``{host}/{repo}``), names the planned initial + fallback schemes, -lists two remediations (pin the URL scheme, or drop -``--allow-protocol-fallback`` to fail fast), and links to the docs. +explains how to disable fallback across CLI flags, environment, and saved +config to fail fast, and links to the docs. """ import os import tempfile from pathlib import Path from unittest.mock import Mock, patch # noqa: F401 +from urllib.parse import urlsplit import pytest # noqa: F401 from git.exc import GitCommandError @@ -84,8 +85,8 @@ def _port_warnings(self, calls): def test_warning_fires_on_ssh_url_with_port_when_fallback_allowed(self): """ssh:// URL with port + allow_fallback => plan has SSH and HTTPS => - exactly one warning naming the offender, both schemes, both - remediations, and the docs URL.""" + exactly one warning naming the offender, both schemes, the + fallback-disable remediation, and the docs URL.""" dep = DependencyReference.parse("ssh://git@bitbucket.example.com:7999/project/repo.git") assert dep.port == 7999 @@ -104,15 +105,18 @@ def test_warning_fires_on_ssh_url_with_port_when_fallback_allowed(self): f"{{host}}/{{repo}}:' form: {msg!r}" ) assert "SSH" in msg and "HTTPS" in msg, f"warning must name both planned schemes: {msg!r}" - assert "Pin the URL scheme" in msg, ( - f"warning must offer the 'pin the URL scheme' remediation: {msg!r}" + assert "Disable protocol fallback" in msg, ( + f"warning must offer the fallback-disable remediation: {msg!r}" ) - assert "--allow-protocol-fallback" in msg, ( - f"warning must offer the 'drop --allow-protocol-fallback' escape " - f"hatch as an alternative: {msg!r}" + assert "CLI flags, environment, and saved config" in msg, ( + f"warning must cover every fallback configuration source: {msg!r}" ) - assert "See: https://microsoft.github.io/apm/" in msg, ( - f"warning must link to the public docs via the 'See: ' prefix: {msg!r}" + docs = urlsplit(msg.rsplit("See: ", 1)[1]) + assert (docs.scheme, docs.hostname, docs.path, docs.fragment) == ( + "https", + "microsoft.github.io", + "/apm/consumer/manage-dependencies/", + "transport-selection", ) def test_warning_fires_on_https_url_with_port_when_fallback_allowed(self):