diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index a0872223aa..8b40e71348 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -157,6 +157,13 @@ "owner": "models/dependency/selection.py (via DependencyReference)", "selectors": ["src/apm_cli/models/dependency/selection.py"], "guards": ["install-deployment-uninstall-selection"] + }, + { + "id": "policy-remote-origin-reader", + "decision": "Reading and parsing the project git remote for org-policy discovery", + "owner": "policy/_remote.py (_git_remote_origin_url, _remote_url_parts, _parse_remote_url, _extract_org_host_port_from_git_remote)", + "selectors": ["src/apm_cli/policy/_remote.py"], + "guards": ["install-deployment-policy-remote-origin-owner"] } ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ef4721095..bcc1b5b933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- GitLab org-policy discovery now walks the subgroup tree from the project's own group up to the top-level group and applies the closest `apm-policy` project, so a team can scope its own governance under a subgroup instead of only the top-level group; a subgroup policy can `extends:` an ancestor group's policy (including nested-namespace references). Flat `/` remotes are unchanged. (closes #2753) (#2933) - gh-aw's shared APM import now supports `token-source: github-token`; after consumers re-vendor the workflow, its read-only current-repository identity can fetch same-repository private packages, while `cascade` remains the default and cross-repository packages still require a dedicated token or GitHub App. (#2706) - OpenAPM v0.1 adds `req-pl-018` for dependency-policy identity casing and amends `req-rs-016` clause (3), the Section 6.4 merge rules, and the Section 6.5 pattern grammar so repository identity and policy matching cannot diverge; Section 11.2 item 6 now requires the per-host case rule in `CONFORMANCE.md`. (#2706) diff --git a/docs/src/content/docs/enterprise/apm-policy.md b/docs/src/content/docs/enterprise/apm-policy.md index 6fd3b4e025..fc1a3d1327 100644 --- a/docs/src/content/docs/enterprise/apm-policy.md +++ b/docs/src/content/docs/enterprise/apm-policy.md @@ -74,11 +74,30 @@ On GitHub and GitHub API-compatible hosts, the `.github-private` repo is preferr ``` / - apm-policy/ - apm-policy.yml # auto-discovered by projects whose remote starts with / + [/] # optional -- apm-policy may sit at any level + apm-policy/ + apm-policy.yml # discovered by projects under this group or any descendant ``` -GitLab discovery uses only the top-level group: APM takes the first path segment from the project remote and looks for `/apm-policy`. It does not search nested subgroup scopes. Set `APM_GITLAB_POLICY_REPO` to use a different project name if your org already publishes policy under another name. A project without `apm-policy` (or the configured override) is treated as a clean "no policy" outcome, matching the fallthrough behaviour on GitHub and ADO -- it does not print a warning. +GitLab discovery walks the subgroup tree from the project's own group up to the top-level group, and the **closest** `apm-policy` wins. For a project remote `gitlab.com/acme/dept-a/team-x/my-project`, APM probes in order: + +1. `acme/dept-a/team-x/apm-policy` +2. `acme/dept-a/apm-policy` +3. `acme/apm-policy` + +This lets an individual team publish its own `apm-policy` under its subgroup without affecting the rest of the organization, while a top-level `acme/apm-policy` still applies to every project that has no closer policy. A flat `/` remote (including a personal namespace like `gitlab.com//`) probes only `/apm-policy`, identical to the previous behaviour. + +Composing policies across levels is opt-in via `extends:`. A team policy can extend an ancestor group's policy to inherit and tighten it, using either an explicit path (`extends: "acme/dept-a/apm-policy"` for the immediate parent group, or a deeper ancestor) or the `extends: "org"` shorthand, which on GitLab always resolves to the **top-level** group's policy (`/apm-policy`) -- not the nearest ancestor. Children can only tighten, never relax. + +:::note[Host-qualified `extends:` on GitLab] +A bare nested `extends:` ref whose first segment contains a dot (e.g. a group named `acme.tools`) is read as a host, so `extends: "acme.tools/team/apm-policy"` is rejected as cross-host. Spell it host-qualified instead -- `extends: "gitlab.com/acme.tools/team/apm-policy"` (or `/...` on self-managed) -- so the leaf host is matched and stripped. When the project's remote uses an explicit port, the host-qualified ref must carry the exact `host:port` authority (e.g. `gitlab.example.com:8443/acme/team/apm-policy`); `GITLAB_HOST` is only the hostname, so a ref without the matching port is rejected. +::: + +Set `APM_GITLAB_POLICY_REPO` to use a different project name if your org already publishes policy under another name. A project with no `apm-policy` at any level (or the configured override) is treated as a clean "no policy" outcome, matching the fallthrough behaviour on GitHub and ADO -- it does not print a warning. + +:::caution[GitLab conceals private projects with 404] +GitLab returns HTTP 404 both for a missing `apm-policy` project and for a private one the token cannot read. A closer policy the CI token is denied is therefore indistinguishable from "no policy at this level," so the walk skips it and applies the next ancestor -- the same `404 == no policy` behaviour GitHub and ADO discovery already have. Grant the CI token read access to every `apm-policy` project it should honour, so a closer policy is never silently skipped. +::: :::caution[Self-managed GitLab requires GITLAB_HOST or APM_GITLAB_HOSTS] An arbitrary FQDN is never auto-classified as GitLab -- the same domain shape could be Bitbucket, Gitea, or a plain git server. `gitlab.com` is recognised automatically, but a self-managed instance (e.g. `gitlab.example.com`) is only recognised once you set `GITLAB_HOST=gitlab.example.com` (or `APM_GITLAB_HOSTS` for more than one instance). Without it, APM falls through to the GitHub-style cascade above, which is invalid on GitLab and behaves exactly like the unfixed discovery this section describes. This mirrors `GITHUB_HOST` for GitHub Enterprise Server and `ADO_HOST` for on-prem Azure DevOps Server -- see [Environment Variables](../../reference/environment-variables/). diff --git a/docs/src/content/docs/enterprise/governance-guide.md b/docs/src/content/docs/enterprise/governance-guide.md index 9b77060b59..239e8f7716 100644 --- a/docs/src/content/docs/enterprise/governance-guide.md +++ b/docs/src/content/docs/enterprise/governance-guide.md @@ -504,7 +504,8 @@ These are the sharp edges. Plan around them; do not assume they are solved. - **`apm compile` and `apm run` do not re-check policy.** They trust install. Operational mitigation: ensure that no compile or run step in CI is reachable without a preceding `apm install` that ran the gate. - **`apm audit --ci` in `warn` mode rewrites violations to `passed=True`.** Warn mode never fails CI exit. The visibility is in the SARIF output, not the exit code. Operational mitigation: monitor Code Scanning alerts during the warn-mode rollout phase; do not assume CI green means "no policy violations" while in warn mode. - **`apm install` has no `--policy ` flag.** Only `apm audit` does. This is the air-gapped install gap. Operational mitigation: use `extends:` from a reachable mirror, or run audit (which does support `--policy `) as the gating check and skip install-time enforcement in air-gapped CI. -- **Plain git remotes are not auto-discovered.** GitHub and GitHub Enterprise use the `.github` cascade, Azure DevOps uses the org `apm` project and `apm-policy` repository (with `_apm/_apm` as a legacy fallback), and GitLab uses `/apm-policy/apm-policy.yml`, where the top-level group is the first path segment of the remote. Nested subgroup scopes are not searched. Configure self-managed GitLab with `GITLAB_HOST` or `APM_GITLAB_HOSTS`. For unsupported remotes, pass `apm audit --ci --policy ` explicitly. +- **Plain git remotes are not auto-discovered.** GitHub and GitHub Enterprise use the `.github` cascade, Azure DevOps uses the org `apm` project and `apm-policy` repository (with `_apm/_apm` as a legacy fallback), and GitLab walks the subgroup tree from the project's own group up to the top-level group and applies the closest `apm-policy` (see [Policy Files](./apm-policy/#where-it-lives)). Configure self-managed GitLab with `GITLAB_HOST` or `APM_GITLAB_HOSTS`. For unsupported remotes, pass `apm audit --ci --policy ` explicitly. +- **GitLab closest-wins is not tighten-only, and a closer policy the token cannot read is skipped.** Two edges of subgroup-scoped discovery: (1) anyone who can create an `apm-policy` project in an intermediate subgroup can publish a *weaker* policy that overrides a stricter ancestor for every project under that subgroup (closest wins; discovery does not enforce tighten-only -- only `extends:` chains do). Operational mitigation: restrict who can create `apm-policy` projects at the GitLab RBAC layer. (2) GitLab 404s a private project the token cannot read, so a closer `apm-policy` the CI token is denied is indistinguishable from "no policy here" and is skipped in favour of the next ancestor (the same `404 == no policy` behaviour as GitHub/ADO). Operational mitigation: grant the CI token read access to every `apm-policy` project it should honour. See [Policy Files](./apm-policy/#where-it-lives). - **Trust anchor is `git remote get-url origin`.** A developer who pushes the project to a personal org will have policy discovery resolve `/.github/apm-policy.yml` -- which they control. Operational mitigation: branch protection on the canonical repo is the trust boundary; nothing about a personal fork can bypass what your CI requires before merge. - **`apm install --dry-run` silently downgrades hash-mismatch.** In dry-run, `raise_blocking_errors=False` (outcome_routing.py:104-119) causes the mismatch to surface as `discovery_miss` with no "Would be blocked" line and exit 0. Operational mitigation: rely on `apm audit --ci` in CI for hash-pin verification, not on `apm install --dry-run`. - **`apm audit --ci --no-policy` and `APM_POLICY_DISABLE=1` skip policy checks.** The 21 policy checks are bypassed in audit, but the 8 baseline lockfile checks still run. Operational mitigation: keep bypass flags out of required CI workflows; the bypass contract in section 7 is authoritative. diff --git a/docs/src/content/docs/enterprise/policy-reference.md b/docs/src/content/docs/enterprise/policy-reference.md index 827ca3601b..26e10ab22b 100644 --- a/docs/src/content/docs/enterprise/policy-reference.md +++ b/docs/src/content/docs/enterprise/policy-reference.md @@ -100,8 +100,9 @@ Inherit from a parent policy. See [Inheritance](#inheritance). | Value | Source | |-------|--------| -| `org` | Parent org's `.github-private/apm-policy.yml` (falls back to `.github`) | +| `org` | Parent org's `.github-private/apm-policy.yml` (falls back to `.github`); on GitLab, the top-level group's `apm-policy` | | `owner/repo` | Cross-org policy from a specific repository | +| `namespace/.../repo` | On GitLab, an ancestor subgroup's policy, e.g. `acme/dept-a/apm-policy` (host-qualify a namespace whose first segment contains a dot) | | `https://...` | Direct URL to a policy file | ### `fetch_failure` @@ -475,7 +476,7 @@ There are 21 policy checks. ## Inheritance :::note[Discovery vs. `extends:` -- two different concepts] -APM auto-discovers exactly **one** policy file: `/.github/apm-policy.yml`, derived from the project's git remote. There is no automatic per-repo or per-enterprise discovery. `extends:` is what composes policies **inside** that one discovered file -- it lets the discovered policy pull in a parent (and that parent's parent, up to `MAX_CHAIN_DEPTH=5`) so you can model an enterprise -> org -> team chain through composition. Most teams who say "3 levels (repo, org, enterprise)" actually want `extends:`, not more discovery sites. +APM auto-discovers exactly **one** policy file per project, derived from the git remote: on GitHub `/.github/apm-policy.yml`, and on GitLab the **closest** `apm-policy` walking up the subgroup tree to the top-level group (see [Policy Files](./apm-policy/#where-it-lives)). There is no automatic per-repo or per-enterprise discovery. `extends:` is what composes policies **inside** that one discovered file -- it lets the discovered policy pull in a parent (and that parent's parent, up to `MAX_CHAIN_DEPTH=5`) so you can model an enterprise -> org -> team chain through composition. Most teams who say "3 levels (repo, org, enterprise)" actually want `extends:`, not more discovery sites. ::: Policies can inherit from a parent using `extends`. This enables a three-level chain: @@ -619,8 +620,9 @@ found in the organization cascade: `.github-private`, `.github`, `.apm`, then `_apm`. Azure DevOps remotes use the org `apm` project and `apm-policy` repository. Legacy `_apm/_apm` is a temporary fallback after a 404 from the primary coordinate. -GitLab remotes use `/apm-policy/apm-policy.yml`, using the -first path segment of the remote; nested subgroup scopes are not searched. Set +GitLab walks the subgroup tree from the project's own group up to the top-level +group and applies the closest `apm-policy` project; a flat `/` +remote probes only `/apm-policy`. Set `GITLAB_HOST` or `APM_GITLAB_HOSTS` to recognize a self-managed host, and use `APM_GITLAB_POLICY_REPO` to select another project name. Plain git remotes fall through with no policy applied. Repositories with no detectable git remote diff --git a/packages/apm-guide/.apm/skills/apm-usage/governance.md b/packages/apm-guide/.apm/skills/apm-usage/governance.md index b2abf2cdfc..b3f09f7230 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/governance.md +++ b/packages/apm-guide/.apm/skills/apm-usage/governance.md @@ -423,9 +423,17 @@ may use. This section covers how that contract is enforced at `apm install` time APM auto-discovers org policy from the project's git remote by checking `.github-private`, `.github`, `.apm`, and `_apm` policy repos in order on GitHub API-compatible hosts. Azure DevOps hosts use repository `apm-policy` in project -`apm`, with a legacy `_apm/_apm` fallback after a 404. GitLab uses -`/apm-policy/apm-policy.yml`, derived from the first remote -path segment; nested subgroup scopes are not searched. Configure a self-managed host with +`apm`, with a legacy `_apm/_apm` fallback after a 404. GitLab walks the subgroup +tree from the project's own group up to the top-level group and applies the +closest `apm-policy` (e.g. `acme/dept-a/team-x/apm-policy` before +`acme/dept-a/apm-policy` before `acme/apm-policy`), so a team can scope its own +policy under a subgroup; a team policy can `extends:` an ancestor group's policy +to inherit it. A flat `/` remote probes only `/apm-policy`. +GitLab 404s a private project the token cannot read, so a closer `apm-policy` the +token is denied is indistinguishable from "no policy here" and is skipped in +favour of the next ancestor (same `404 == no policy` behaviour as GitHub/ADO) -- +grant the token read access to every `apm-policy` it should honour. +Configure a self-managed host with `GITLAB_HOST` or `APM_GITLAB_HOSTS`, and use `APM_GITLAB_POLICY_REPO` to select another project name. Repositories with no detectable git remote (unpacked bundles, temp dirs) emit an explicit "could not determine org" line and skip @@ -675,7 +683,8 @@ as `[x]` errors and exit `1`. Checklist to publish a policy: 1. Create `apm-policy.yml` in the org policy repo (`.github-private` or `.github` on GitHub, `apm` - project and `apm-policy` repository on Azure DevOps, or `apm-policy` under the top-level GitLab group). + project and `apm-policy` repository on Azure DevOps, or an `apm-policy` project under the GitLab + group you want it to govern -- any ancestor namespace of the project, closest wins). 2. Start from the recommended starter below and trim to the minimum reflecting your governance posture. 3. Set `enforcement: warn` first. Let CI surface diagnostics across consuming diff --git a/scripts/architecture_linter/checks/install_policy_gitlab_and_bundle.py b/scripts/architecture_linter/checks/install_policy_gitlab_and_bundle.py index da1b284f15..de5df2b935 100644 --- a/scripts/architecture_linter/checks/install_policy_gitlab_and_bundle.py +++ b/scripts/architecture_linter/checks/install_policy_gitlab_and_bundle.py @@ -13,7 +13,6 @@ from scripts.architecture_linter.checks.install_policy_shared import ( _APM_RESOLVER, - _ELSE_TERMINATOR, _POLICY_DISCOVERY, _banned, _configured, @@ -43,6 +42,9 @@ RULE_WINNER_SELECTION = "install-deployment-dependency-winner-selection" +RULE_REMOTE_ORIGIN_OWNER = "install-deployment-policy-remote-origin-owner" + + def _count_text(lines: Sequence[tuple[int, str]], needle: str) -> int: """Return how many lines contain `needle` (``grep -Fc``).""" return sum(1 for _, text in lines if needle in text) @@ -113,7 +115,94 @@ def check_gitlab_policy_adapter(provider: FactsProvider) -> tuple[Violation, ... return tuple(findings) -_GITLAB_BRANCH_START = re.compile(r"^[ \t]*elif is_gitlab_hostname\(host\):") +_REMOTE_ORIGIN_ARGV = re.compile(r'"remote",\s*"get-url",\s*"origin"') + + +_REMOTE_PARSER_DEFS = re.compile( + r"^def (_remote_url_parts|_parse_remote_url|_git_remote_origin_url" + r"|_extract_org_host_port_from_git_remote)\(" +) + + +_REMOTE_PARSER_DEF_COUNT = 4 + + +_REMOTE_ORIGIN_READ_COUNT = 1 + + +_REMOTE_MODULE = "src/apm_cli/policy/_remote.py" + + +def check_policy_remote_origin_owner(provider: FactsProvider) -> tuple[Violation, ...]: + """Reading and parsing the project git remote for policy discovery has one owner. + + ``policy/_remote.py`` is the sole reader of ``git remote get-url origin`` and + the sole home of the remote-URL splitter/parsers/identity extractor + (``_remote_url_parts``, ``_parse_remote_url``, ``_git_remote_origin_url``, + ``_extract_org_host_port_from_git_remote``). The owner MUST define all four, + and no other module in the policy tree may re-read or re-parse the remote -- + either would reintroduce the double-read / divergent-parse the single-owner + refactor removed (#2753). + """ + rule_id = RULE_REMOTE_ORIGIN_OWNER + owner, owner_fail = _configured(provider, _REMOTE_MODULE, rule_id) + if owner_fail: + return tuple(owner_fail) + findings: list[Violation] = [] + definitions = _count_re(owner, _REMOTE_PARSER_DEFS) + if definitions != _REMOTE_PARSER_DEF_COUNT: + findings.append( + _report( + rule_id, + _REMOTE_MODULE, + "policy/_remote.py must define exactly " + f"{_REMOTE_PARSER_DEF_COUNT} canonical git-remote read/parse helpers " + f"(found {definitions})", + ) + ) + origin_reads = _count_re(owner, _REMOTE_ORIGIN_ARGV) + if origin_reads != _REMOTE_ORIGIN_READ_COUNT: + findings.append( + _report( + rule_id, + _REMOTE_MODULE, + "policy/_remote.py must read the git remote origin exactly " + f"{_REMOTE_ORIGIN_READ_COUNT} time via _git_remote_origin_url " + f"(found {origin_reads} origin-read argv occurrences)", + ) + ) + findings.extend( + _banned( + provider, + rule_id=rule_id, + paths=_tree_python_paths(provider, _POLICY_TREE, excluded=(_REMOTE_MODULE,)), + pattern=_REMOTE_ORIGIN_ARGV, + message="Read the git remote origin only via _remote.py::_git_remote_origin_url", + configured=False, + respect_exempt=True, + ) + ) + findings.extend( + _banned( + provider, + rule_id=rule_id, + paths=_tree_python_paths(provider, _POLICY_TREE, excluded=(_REMOTE_MODULE,)), + pattern=_REMOTE_PARSER_DEFS, + message="Remote-URL split/parse owner is _remote.py; do not redefine these helpers", + configured=False, + respect_exempt=True, + ) + ) + return tuple(findings) + + +# The facade delegation now lives in ``_gitlab_walk_candidate`` (the GitLab +# branch of ``_auto_discover`` calls it; see #2753). Scope the orchestration +# scan to that helper's body -- from its ``def`` to the next top-level ``def``. +_GITLAB_WALK_START = re.compile(r"^def _gitlab_walk_candidate\(") + + +_TOP_LEVEL_DEF = re.compile(r"^def ") _NON_WHITESPACE = re.compile(r"[^\s]") @@ -134,8 +223,8 @@ def check_gitlab_facade_orchestration(provider: FactsProvider) -> tuple[Violatio branch = _indent_scoped_branch( lines, - start=_GITLAB_BRANCH_START, - terminator=_ELSE_TERMINATOR, + start=_GITLAB_WALK_START, + terminator=_TOP_LEVEL_DEF, probe=_NON_WHITESPACE, include_start=False, restart_skips=True, diff --git a/scripts/architecture_linter/checks/install_policy_intent.py b/scripts/architecture_linter/checks/install_policy_intent.py index 173d9f2024..c9371fc3f2 100644 --- a/scripts/architecture_linter/checks/install_policy_intent.py +++ b/scripts/architecture_linter/checks/install_policy_intent.py @@ -28,12 +28,14 @@ RULE_GITLAB_ADAPTER, RULE_GITLAB_FACADE, RULE_LOCAL_BUNDLE_PREFLIGHT, + RULE_REMOTE_ORIGIN_OWNER, RULE_REQUIRE_HASHES, RULE_WINNER_SELECTION, check_dependency_winner_selection, check_gitlab_facade_orchestration, check_gitlab_policy_adapter, check_local_bundle_preflight, + check_policy_remote_origin_owner, check_require_hashes_enforcement, ) from scripts.architecture_linter.checks.install_policy_plugin_and_approval import ( @@ -101,6 +103,15 @@ "GitLab policy cache and transport remain in policy/_gitlab.py.", check_gitlab_facade_orchestration, ), + Rule( + id=RULE_REMOTE_ORIGIN_OWNER, + group=GROUP, + guard_ids=(RULE_REMOTE_ORIGIN_OWNER,), + description=( + "Policy discovery reads and parses the git remote through one owner (policy/_remote.py)." + ), + check=check_policy_remote_origin_owner, + ), _semantic_rule( RULE_LOCAL_BUNDLE_PREFLIGHT, "Local bundle installs route policy through install_preflight.py.", diff --git a/src/apm_cli/core/command_logger.py b/src/apm_cli/core/command_logger.py index 7b4a32cafa..4cff63c90c 100644 --- a/src/apm_cli/core/command_logger.py +++ b/src/apm_cli/core/command_logger.py @@ -601,11 +601,15 @@ def policy_resolved( cached: bool, enforcement: str, age_seconds: int | None = None, + subgroup_scoped: bool = False, ): """Log policy discovery outcome. Verbose by default; always shown when ``enforcement == "block"`` - (users must know blocking is active). + (users must know blocking is active) or when ``subgroup_scoped`` + is set -- a GitLab subgroup policy that closest-wins resolved + BELOW the top-level group is surprising enough that the resolved + level must name itself even at default verbosity (#2753). Format: ``[i] Policy: (cached, fetched 5m ago) -- enforcement=block`` """ @@ -629,9 +633,11 @@ def policy_resolved( if enforcement == "block": # Always visible — blocking installs is a big deal _rich_warning(message, symbol="warning") - elif self.verbose: + elif self.verbose or subgroup_scoped: + # Subgroup-scoped GitLab policy: show at info even in non-verbose + # warn mode so the dev sees which subgroup level applied (#2753). _rich_info(message, symbol="info") - # Non-verbose + non-block: silent (no noise for warn/off) + # Non-verbose + non-block + top-level: silent (no noise for warn/off) def policy_discovery_miss( self, diff --git a/src/apm_cli/policy/_gitlab.py b/src/apm_cli/policy/_gitlab.py index 1175d6cac7..088860a68c 100644 --- a/src/apm_cli/policy/_gitlab.py +++ b/src/apm_cli/policy/_gitlab.py @@ -136,26 +136,39 @@ def _fetch_gitlab_chain_parent( ) org = current_org else: - parts = parent_ref.strip("/").split("/") - if len(parts) == 3: + parts = [p for p in parent_ref.strip("/").split("/") if p] + invalid = PolicyFetchResult( + source=f"org:{parent_ref}", + error=f"Invalid GitLab policy reference: {parent_ref}", + outcome="cache_miss_fetch_fail", + ) + # A host-like first segment (FQDN or ``host:port``) MUST match the leaf + # host and port exactly, then it is stripped so ``host/namespace/.../repo`` + # and ``namespace/.../repo`` are treated the same. A host-like segment + # that does not match the leaf -- or carries a malformed port -- is a + # cross-host/invalid ref and is rejected, never silently folded into the + # namespace. A bare single-label first segment is a namespace segment + # (GitLab nested subgroups, see #2753) and is left in place. + if len(parts) >= 3 and ("." in parts[0] or ":" in parts[0]): try: explicit = urlsplit(f"//{parts[0]}") + same_leaf = ( + explicit.hostname is not None + and explicit.hostname.lower() == leaf_host.lower() + and explicit.port == port + ) except ValueError: - explicit = None - if ( - explicit is not None - and explicit.hostname is not None - and explicit.hostname.lower() == leaf_host.lower() - and explicit.port == port - ): - parts = parts[1:] - if len(parts) != 2: - return PolicyFetchResult( - source=f"org:{parent_ref}", - error=f"Invalid GitLab policy reference: {parent_ref}", - outcome="cache_miss_fetch_fail", - ) - org, repo = parts + same_leaf = False + if not same_leaf: + return invalid + parts = parts[1:] + # A GitLab namespace may be nested (subgroups): everything before the + # final segment is the namespace, the final segment is the policy repo. + # Requires at least ``namespace/repo``. + if len(parts) < 2: + return invalid + org = "/".join(parts[:-1]) + repo = parts[-1] return _fetch_from_gitlab_repo( org=org, repo=repo, diff --git a/src/apm_cli/policy/_remote.py b/src/apm_cli/policy/_remote.py new file mode 100644 index 0000000000..24d9118fb5 --- /dev/null +++ b/src/apm_cli/policy/_remote.py @@ -0,0 +1,191 @@ +"""Git remote-URL identity parsing for org-policy discovery. + +Split out of ``discovery.py`` (which grew past the file-length ratchet): the +canonical splitter :func:`_remote_url_parts` and the host-specific +:func:`_parse_remote_url` live here. ``discovery.py`` re-exports both and reads +the ``origin`` remote via its own ``_git_remote_origin_url``; the architecture +guard ``install-deployment-policy-remote-origin-owner`` keeps these the single +owners of git-remote reading and URL parsing. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path +from urllib.parse import urlparse + +from ..cache.url_normalize import SCP_LIKE_RE +from ..utils.git_env import get_git_executable +from ..utils.github_host import ( + is_azure_devops_hostname, + is_visualstudio_legacy_hostname, + parse_ado_repo_url, +) + + +class _Unset: + """Sentinel type: an argument was not provided (distinct from ``None``).""" + + +_UNSET = _Unset() + + +def _git_remote_origin_url(project_root: Path) -> str | None: + """Return the ``origin`` remote URL, or ``None`` when unavailable. + + Canonical reader of the project's ``origin`` remote, shared by + :func:`_extract_org_host_port_from_git_remote` and GitLab subgroup + discovery so the ``git remote get-url`` subprocess lives in one place. + """ + try: + result = subprocess.run( + [get_git_executable(), "remote", "get-url", "origin"], + capture_output=True, + text=True, + encoding="utf-8", + cwd=project_root, + timeout=5, + ) + except (subprocess.TimeoutExpired, FileNotFoundError): + return None + if result.returncode != 0: + return None + return result.stdout.strip() + + +def _extract_org_host_port_from_git_remote( + project_root: Path, + *, + remote_url: str | None | _Unset = _UNSET, +) -> tuple[str, str, int | None] | None: + """Extract ``(org, host, port)`` from git remote origin. + + ``remote_url`` lets a caller that already read ``origin`` (e.g. + ``discovery._auto_discover``) pass it in so the ``git remote get-url`` + subprocess runs exactly once per discovery. When left unset the origin is + read here; an explicit ``None`` means "no remote" and is not re-read. + """ + if isinstance(remote_url, _Unset): + remote_url = _git_remote_origin_url(project_root) + if not remote_url: + return None + try: + parsed_identity = _parse_remote_url(remote_url) + except ValueError: + return None + if parsed_identity is None: + return None + port = None + if "://" in remote_url: + try: + port = urlparse(remote_url).port + except ValueError: + return None + return parsed_identity[0], parsed_identity[1], port + + +def _extract_org_from_git_remote( + project_root: Path, +) -> tuple[str, str] | None: + """Extract (org, host) from git remote origin URL. + + Handles: + - https://github.com/contoso/my-project.git -> ("contoso", "github.com") + - git@github.com:contoso/my-project.git -> ("contoso", "github.com") + - https://github.example.com/contoso/my-project.git -> ("contoso", "github.example.com") + """ + identity = _extract_org_host_port_from_git_remote(project_root) + return (identity[0], identity[1]) if identity is not None else None + + +def _remote_url_parts(url: str) -> tuple[str, list[str]] | None: + """Split a git remote URL into ``(host, path_segments)``. + + Canonical splitter shared by :func:`_parse_remote_url` (which applies + host-specific org interpretation on top) and GitLab subgroup discovery + (:func:`discovery._gitlab_namespace_descending`). Handles SCP-like SSH URLs + with any username (not just ``git@``) and ``scheme://`` URLs. Path segments + are cleaned of empty parts and the trailing ``.git`` suffix; no host-specific + interpretation (ADO ``v3/`` prefix, visualstudio subdomain, ...) is applied + here -- that stays the caller's responsibility. + + Returns ``None`` when the URL cannot be split. + """ + if not url: + return None + + # SCP-like SSH: @: -- any user, not just `git`. + # Closes #1159 for non-`git` SSH users (EMU, custom GHE accounts). + scp_match = SCP_LIKE_RE.match(url) + if scp_match: + host = scp_match.group("host") + path_part = scp_match.group("path") + segments = [p for p in path_part.rstrip("/").removesuffix(".git").split("/") if p] + if not host or not segments: + return None + return (host, segments) + + # HTTPS: https://github.com/owner/repo.git + if "://" in url: + try: + parsed = urlparse(url) + except Exception: + # Deliberately broad: the legacy parser swallowed ANY exception + # from ``urlparse`` here (cross-version defensiveness), and + # ``test_https_url_parse_exception_returns_none`` pins that + # contract. The ``/tfs/`` ValueError comes from + # ``parse_ado_repo_url`` (in ``_parse_remote_url``), never from + # ``urlparse`` here, so there is nothing to re-raise. + return None + host = parsed.hostname or "" + segments = [ + p for p in parsed.path.strip("/").removesuffix(".git").rstrip("/").split("/") if p + ] + if not host or not segments: + return None + return (host, segments) + + return None + + +def _parse_remote_url(url: str) -> tuple[str, str] | None: + """Parse a git remote URL into (org, host). + + Accepts SCP-style SSH URLs with any username (not just ``git@``), so + EMU/GHE deployments that use a non-``git`` SSH user + (e.g. ``enterprise-user@ghe.corp.com:org/repo.git``) parse correctly. + Also handles Azure DevOps SSH URLs which carry an extra ``v3/`` + path prefix (``git@ssh.dev.azure.com:v3///``). + + Returns None if URL can't be parsed. + """ + parts = _remote_url_parts(url) + if parts is None: + return None + host, segments = parts + + # SCP-like SSH: @: -- any user, not just `git`. + if SCP_LIKE_RE.match(url): + # Azure DevOps SSH carries a leading 'v3/' segment that is + # NOT the org. The org is the second segment. + if host == "ssh.dev.azure.com" and segments[0] == "v3" and len(segments) >= 2: + return (segments[1], host) + return (segments[0], host) + + # HTTPS: https://github.com/owner/repo.git + # ADO: https://dev.azure.com/org/project/_git/repo + try: + if is_azure_devops_hostname(host): + ado_coordinates = parse_ado_repo_url(url) + if ado_coordinates is None: + return None + return ado_coordinates[0], host + if is_visualstudio_legacy_hostname(host): + return (host[: -len(".visualstudio.com")], host) + return (segments[0], host) + except ValueError as exc: + if "mounted below '/tfs/'" in str(exc): + raise + return None + except Exception: + return None diff --git a/src/apm_cli/policy/discovery.py b/src/apm_cli/policy/discovery.py index 670457d1bd..aa81b99e63 100644 --- a/src/apm_cli/policy/discovery.py +++ b/src/apm_cli/policy/discovery.py @@ -7,7 +7,6 @@ import json import logging import os -import subprocess import threading import time from dataclasses import dataclass, field @@ -18,18 +17,22 @@ import yaml from ..cache.paths import get_cache_root -from ..cache.url_normalize import SCP_LIKE_RE -from ..utils.git_env import get_git_executable from ..utils.github_host import ( build_ado_api_url, is_azure_devops_hostname, is_gitlab_hostname, is_visualstudio_legacy_hostname, - parse_ado_repo_url, ) from ..utils.path_security import ensure_path_within from ..utils.yaml_io import load_yaml_str from . import _gitlab +from ._remote import ( # noqa: F401 -- re-exported for callers/tests + _extract_org_from_git_remote, + _extract_org_host_port_from_git_remote, + _git_remote_origin_url, + _parse_remote_url, + _remote_url_parts, +) from .parser import PolicyValidationError, load_policy from .project_config import ( _DEFAULT_HASH_ALGORITHM, @@ -225,6 +228,13 @@ class PolicyFetchResult: raw_bytes_hash: str | None = None expected_hash: str | None = None # The pin that was checked, if any + # -- Discovery-provenance fields (#2753) -- + # True when GitLab subgroup discovery resolved a policy BELOW the + # top-level group (closest-wins diverged from the legacy top-level + # probe). Surfaced at info even in non-verbose warn mode so a dev can + # see which subgroup's policy applied without needing --verbose. + subgroup_scoped: bool = False + @property def found(self) -> bool: return self.policy is not None @@ -389,13 +399,17 @@ def _derive_leaf_host(source: str, project_root: Path) -> str | None: return _derive_leaf_identity(source, project_root)[0] -def _extract_extends_host(ref: str) -> str | None: +def _extract_extends_host(ref: str, leaf_host: str | None = None) -> str | None: """Return the host an ``extends:`` ref resolves against, if explicit. * Full URL -> URL host (lowercase) * ``//`` (3+ slash-segments) -> ```` (lowercase) * ``/`` shorthand -> None (intrinsically same-host) * ```` shorthand (no slash) -> None (intrinsically same-host) + + ``leaf_host`` gates the GitLab-only relaxation for nested-subgroup refs + (see the 3-segment branch below); it defaults to ``None`` so non-GitLab + callers keep the strict host-pin behaviour. """ if not ref: return None @@ -411,8 +425,19 @@ def _extract_extends_host(ref: str) -> str | None: return None parts = ref.split("/") if len(parts) >= 3: + first = parts[0] + host_like = "." in first or ":" in first + # A bare single-label first segment is a namespace segment ONLY on + # GitLab, where nested subgroups make ``acme/dept-a/apm-policy`` a valid + # same-host ``extends:`` ref (#2753). On every other provider a + # 3-segment ref is ``host/owner/repo``; treating a single label as a + # namespace there would let ``extends: "evil/org/repo"`` bypass the + # host-pin and route a credential to ``evil`` (Security Finding F1), so + # the first segment is parsed as a (cross-)host and rejected. + if is_gitlab_hostname(leaf_host) and not host_like: + return None try: - return (urlsplit(f"//{parts[0]}").hostname or "").lower() or None + return (urlsplit(f"//{first}").hostname or "").lower() or None except ValueError: return None return None @@ -432,7 +457,7 @@ def _validate_extends_host(leaf_host: str | None, extends_ref: str) -> None: """ from . import inheritance as _inheritance_mod - extends_host = _extract_extends_host(extends_ref) + extends_host = _extract_extends_host(extends_ref, leaf_host) if extends_host is None: return # shorthand: intrinsically same-host, allowed. @@ -845,7 +870,10 @@ def _auto_discover( - Found -> return (first match wins) 5. All candidates exhausted -> outcome="absent" """ - identity = _extract_org_host_port_from_git_remote(project_root) + # Read origin once and reuse it for both identity and the GitLab namespace + # walk so discovery never invokes ``git remote get-url`` more than once. + remote_url = _git_remote_origin_url(project_root) + identity = _extract_org_host_port_from_git_remote(project_root, remote_url=remote_url) if identity is None: return PolicyFetchResult( error="Could not determine org from git remote", @@ -858,6 +886,11 @@ def _auto_discover( except ValueError as exc: return PolicyFetchResult(error=str(exc), outcome="cache_miss_fetch_fail") is_ado = is_azure_devops_hostname(host) + is_gitlab = is_gitlab_hostname(host) + # GitLab discovery walks the subgroup tree from the deepest subgroup up to + # the top-level group (closest policy wins, see #2753). Fall back to the + # single top-level org when the namespace cannot be derived. + gitlab_namespaces = (_gitlab_namespace_descending(remote_url) or [org]) if is_gitlab else [] for candidate_repo in candidates: logger.debug("Trying org policy repo candidate %s on host %s", candidate_repo, host) @@ -871,10 +904,10 @@ def _auto_discover( expected_hash=expected_hash, cache_only=cache_only, ) - elif is_gitlab_hostname(host): - result = _gitlab._fetch_from_gitlab_repo( - org=org, - repo=candidate_repo, + elif is_gitlab: + result = _gitlab_walk_candidate( + candidate_repo=candidate_repo, + namespaces=gitlab_namespaces, host=host, port=port, project_root=project_root, @@ -923,107 +956,81 @@ def _auto_discover( return PolicyFetchResult(error=None, outcome="absent") -def _extract_org_from_git_remote( - project_root: Path, -) -> tuple[str, str] | None: - """Extract (org, host) from git remote origin URL. +def _gitlab_namespace_descending(remote_url: str | None) -> list[str] | None: + """Return GitLab policy namespaces from the deepest subgroup to the top. - Handles: - - https://github.com/contoso/my-project.git -> ("contoso", "github.com") - - git@github.com:contoso/my-project.git -> ("contoso", "github.com") - - https://github.example.com/contoso/my-project.git -> ("contoso", "github.example.com") + Probe order is closest-first. For a project remote + ``gitlab.com/acme/dept-a/team-x/my-project`` this returns + ``["acme/dept-a/team-x", "acme/dept-a", "acme"]``; a flat + ``acme/my-project`` returns ``["acme"]`` (identical to top-level-only + discovery). Returns ``None`` when the origin remote is missing or has no + namespace segment (a degenerate remote with no owner), so callers can fall + back to their existing single-org behaviour. Takes the already-read + ``origin`` URL so discovery reads the remote exactly once. """ - identity = _extract_org_host_port_from_git_remote(project_root) - return (identity[0], identity[1]) if identity is not None else None - - -def _extract_org_host_port_from_git_remote( - project_root: Path, -) -> tuple[str, str, int | None] | None: - """Extract ``(org, host, port)`` from git remote origin.""" - try: - result = subprocess.run( - [get_git_executable(), "remote", "get-url", "origin"], - capture_output=True, - text=True, - encoding="utf-8", - cwd=project_root, - timeout=5, - ) - if result.returncode != 0: - return None - remote_url = result.stdout.strip() - parsed_identity = _parse_remote_url(remote_url) - if parsed_identity is None: - return None - port = None - if "://" in remote_url: - try: - port = urlparse(remote_url).port - except ValueError: - return None - return parsed_identity[0], parsed_identity[1], port - except (ValueError, subprocess.TimeoutExpired, FileNotFoundError): + if not remote_url: return None - - -def _parse_remote_url(url: str) -> tuple[str, str] | None: - """Parse a git remote URL into (org, host). - - Accepts SCP-style SSH URLs with any username (not just ``git@``), so - EMU/GHE deployments that use a non-``git`` SSH user - (e.g. ``enterprise-user@ghe.corp.com:org/repo.git``) parse correctly. - Also handles Azure DevOps SSH URLs which carry an extra ``v3/`` - path prefix (``git@ssh.dev.azure.com:v3///``). - - Returns None if URL can't be parsed. - """ - if not url: + parts = _remote_url_parts(remote_url) + if parts is None: return None + _host, segments = parts + # The final segment is the project; everything before it is the namespace. + namespace_segments = segments[:-1] + if not namespace_segments: + return None + return ["/".join(namespace_segments[:n]) for n in range(len(namespace_segments), 0, -1)] - # SCP-like SSH: @: -- any user, not just `git`. - # Closes #1159 for non-`git` SSH users (EMU, custom GHE accounts). - scp_match = SCP_LIKE_RE.match(url) - if scp_match: - host = scp_match.group("host") - path_part = scp_match.group("path") - try: - parts = path_part.rstrip("/").removesuffix(".git").split("/") - parts = [p for p in parts if p] - if not parts: - return None - # Azure DevOps SSH carries a leading 'v3/' segment that is - # NOT the org. The org is the second segment. - if host == "ssh.dev.azure.com" and parts[0] == "v3" and len(parts) >= 2: - return (parts[1], host) - return (parts[0], host) - except (ValueError, IndexError): - return None - - # HTTPS: https://github.com/owner/repo.git - # ADO: https://dev.azure.com/org/project/_git/repo - if "://" in url: - try: - parsed = urlparse(url) - host = parsed.hostname or "" - if is_azure_devops_hostname(host): - ado_coordinates = parse_ado_repo_url(url) - if ado_coordinates is None: - return None - return ado_coordinates[0], host - path_parts = parsed.path.strip("/").removesuffix(".git").rstrip("/").split("/") - if is_visualstudio_legacy_hostname(host): - return (host[: -len(".visualstudio.com")], host) - if host and path_parts and path_parts[0]: - return (path_parts[0], host) - except ValueError as exc: - if "mounted below '/tfs/'" in str(exc): - raise - return None - except Exception: - return None - return None +def _gitlab_walk_candidate( + *, + candidate_repo: str, + namespaces: list[str], + host: str, + port: int | None, + project_root: Path, + no_cache: bool, + expected_hash: str | None, + cache_only: bool, +) -> PolicyFetchResult: + """Probe one GitLab policy repo up the subgroup tree (closest wins, #2753). + + Fetches ``//apm-policy.yml`` for each namespace + in *namespaces* (already ordered deepest-first). The first non-``absent`` + outcome wins; an ``absent`` level continues to the next-shallower group. + When every level is absent the last ``absent`` result is returned so the + caller's candidate cascade can proceed unchanged. + + A GitLab ``absent`` is a 404, which GitLab returns both for a missing + project and for a private one the token cannot read -- the same + "404 == no policy at this level" ambiguity GitHub/ADO discovery already + accept. A closer policy the token cannot read is therefore skipped in + favour of the next ancestor (documented as a GitLab-platform limitation). + """ + result = PolicyFetchResult(error=None, outcome="absent") + for namespace in namespaces: + result = _gitlab._fetch_from_gitlab_repo( + org=namespace, + repo=candidate_repo, + host=host, + port=port, + project_root=project_root, + no_cache=no_cache, + expected_hash=expected_hash, + cache_only=cache_only, + ) + if result.outcome != "absent": + # A namespace with more than one segment is a subgroup below the + # top-level group, so closest-wins diverged from the legacy + # top-level-only probe; flag it so the resolved level is surfaced + # to the user even at default verbosity (#2753). + result.subgroup_scoped = "/" in namespace + return result + logger.debug( + "GitLab policy absent at %s/%s; trying parent group", + namespace, + candidate_repo, + ) + return result def _fetch_from_url( diff --git a/src/apm_cli/policy/outcome_routing.py b/src/apm_cli/policy/outcome_routing.py index a562907725..f2a4c79547 100644 --- a/src/apm_cli/policy/outcome_routing.py +++ b/src/apm_cli/policy/outcome_routing.py @@ -194,6 +194,7 @@ def route_discovery_outcome( cached=True, enforcement=policy.enforcement, age_seconds=fetch_result.cache_age_seconds, + subgroup_scoped=fetch_result.subgroup_scoped, ) logger.policy_discovery_miss( outcome="cached_stale", @@ -218,6 +219,7 @@ def route_discovery_outcome( cached=fetch_result.cached, enforcement=policy.enforcement, age_seconds=fetch_result.cache_age_seconds, + subgroup_scoped=fetch_result.subgroup_scoped, ) return policy diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index 036f70b41d..cd1f810321 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -397,6 +397,17 @@ class MutationCase: new="def resolve_effective_package_targets_disabled(", intent="Effective package-target authorization loses its single resolver.", ), + MutationCase( + guard_id="install-deployment-policy-remote-origin-owner", + rule_id="install-deployment-policy-remote-origin-owner", + path="src/apm_cli/policy/_remote.py", + old='[get_git_executable(), "remote", "get-url", "origin"],', + new=( + '[get_git_executable(), "remote", "get-url", "origin"],\n' + ' [get_git_executable(), "remote", "get-url", "origin"],' + ), + intent="A second git-remote origin read appears outside the single canonical reader.", + ), MutationCase( guard_id="install-deployment-primitive-classification", rule_id="install-deployment-primitive-classification", diff --git a/tests/unit/install/test_install_logger_policy.py b/tests/unit/install/test_install_logger_policy.py index 4e35eab4e2..b5be5c96af 100644 --- a/tests/unit/install/test_install_logger_policy.py +++ b/tests/unit/install/test_install_logger_policy.py @@ -347,6 +347,37 @@ def test_warn_non_verbose_silent(self, mock_info): ) mock_info.assert_not_called() + @patch("apm_cli.core.command_logger._rich_info") + def test_subgroup_scoped_visible_non_verbose_warn(self, mock_info): + """#2753: a GitLab subgroup policy (closest-wins below the top-level + group) names the resolved level at info even in non-verbose warn mode, + so the dev sees which subgroup applied without --verbose.""" + logger = InstallLogger(verbose=False) + logger.policy_resolved( + source="org:gitlab.com/acme/dept-a/apm-policy", + cached=False, + enforcement="warn", + subgroup_scoped=True, + ) + mock_info.assert_called_once() + msg = mock_info.call_args[0][0] + assert "org:gitlab.com/acme/dept-a/apm-policy" in msg + assert "enforcement=warn" in msg + assert mock_info.call_args[1].get("symbol") == "info" + + @patch("apm_cli.core.command_logger._rich_info") + def test_top_level_scope_stays_silent_non_verbose_warn(self, mock_info): + """A top-level (non-subgroup) policy keeps the pre-#2753 silence: + non-verbose + warn emits nothing when subgroup_scoped is False.""" + logger = InstallLogger(verbose=False) + logger.policy_resolved( + source="org:gitlab.com/acme/apm-policy", + cached=False, + enforcement="warn", + subgroup_scoped=False, + ) + mock_info.assert_not_called() + @patch("apm_cli.core.command_logger._rich_info") def test_off_verbose_shows_info(self, mock_info): logger = InstallLogger(verbose=True) diff --git a/tests/unit/policy/test_discovery.py b/tests/unit/policy/test_discovery.py index 95546ce2e2..5667e49a6a 100644 --- a/tests/unit/policy/test_discovery.py +++ b/tests/unit/policy/test_discovery.py @@ -29,6 +29,7 @@ PolicyFetchResult, _auto_discover, _cache_key, + _extract_extends_host, _extract_org_from_git_remote, _extract_org_host_port_from_git_remote, _fetch_ado_contents, @@ -39,11 +40,14 @@ _fetch_from_url, _fetch_github_contents, _get_cache_dir, + _gitlab_namespace_descending, _load_from_file, _parse_remote_url, _policy_repo_candidates, _read_cache, _read_cache_entry, + _remote_url_parts, + _validate_extends_host, _write_cache, discover_policy, ) @@ -157,11 +161,78 @@ def test_ado_ssh_v3_prefix_with_git_suffix(self): result = _parse_remote_url("git@ssh.dev.azure.com:v3/myorg/myproject/myrepo.git") self.assertEqual(result, ("myorg", "ssh.dev.azure.com")) + # --- Nested GitLab subgroups: org stays the top-level group (#2753) --- + + def test_https_gitlab_nested_subgroups_org_is_top_level(self): + result = _parse_remote_url("https://gitlab.com/acme/dept-a/team-x/my-project.git") + self.assertEqual(result, ("acme", "gitlab.com")) + + def test_ssh_gitlab_nested_subgroups_org_is_top_level(self): + result = _parse_remote_url("git@gitlab.com:acme/dept-a/team-x/my-project.git") + self.assertEqual(result, ("acme", "gitlab.com")) + + +class TestRemoteUrlParts(unittest.TestCase): + """Test _remote_url_parts -- the shared host + path-segments splitter.""" + + def test_https_nested_segments(self): + result = _remote_url_parts("https://gitlab.com/acme/dept-a/team-x/my-project.git") + self.assertEqual(result, ("gitlab.com", ["acme", "dept-a", "team-x", "my-project"])) + + def test_ssh_nested_segments(self): + result = _remote_url_parts("git@gitlab.com:acme/dept-a/team-x/my-project.git") + self.assertEqual(result, ("gitlab.com", ["acme", "dept-a", "team-x", "my-project"])) + + def test_https_flat_segments(self): + result = _remote_url_parts("https://gitlab.com/acme/my-project") + self.assertEqual(result, ("gitlab.com", ["acme", "my-project"])) + + def test_https_trailing_slash_is_cleaned(self): + result = _remote_url_parts("https://gitlab.com/acme/my-project/") + self.assertEqual(result, ("gitlab.com", ["acme", "my-project"])) + + def test_empty_string_returns_none(self): + self.assertIsNone(_remote_url_parts("")) + + def test_non_url_returns_none(self): + self.assertIsNone(_remote_url_parts("not-a-url")) + + def test_ssh_empty_path_returns_none(self): + self.assertIsNone(_remote_url_parts("git@gitlab.com:")) + + def test_https_no_path_returns_none(self): + self.assertIsNone(_remote_url_parts("https://gitlab.com/")) + + +class TestGitlabNamespaceDescending(unittest.TestCase): + """Test _gitlab_namespace_descending -- subgroup probe order (#2753). + + The helper is pure: it takes the already-read ``origin`` URL so discovery + reads the remote exactly once. + """ + + def test_nested_subgroups_deepest_first(self): + result = _gitlab_namespace_descending( + "https://gitlab.com/acme/dept-a/team-x/my-project.git" + ) + self.assertEqual(result, ["acme/dept-a/team-x", "acme/dept-a", "acme"]) + + def test_flat_project_yields_single_top_level(self): + result = _gitlab_namespace_descending("git@gitlab.com:acme/my-project.git") + self.assertEqual(result, ["acme"]) + + def test_no_remote_returns_none(self): + self.assertIsNone(_gitlab_namespace_descending(None)) + + def test_no_namespace_segment_returns_none(self): + # A remote with only one path segment has no owning namespace. + self.assertIsNone(_gitlab_namespace_descending("https://gitlab.com/solo")) + class TestExtractOrgFromGitRemote(unittest.TestCase): """Test _extract_org_from_git_remote with mocked subprocess.""" - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_successful_remote(self, mock_run): mock_run.return_value = MagicMock( returncode=0, @@ -178,26 +249,26 @@ def test_successful_remote(self, mock_run): timeout=5, ) - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_git_command_fails(self, mock_run): mock_run.return_value = MagicMock(returncode=1, stdout="") result = _extract_org_from_git_remote(Path("/fake")) self.assertIsNone(result) - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_git_not_found(self, mock_run): mock_run.side_effect = FileNotFoundError("git not found") result = _extract_org_from_git_remote(Path("/fake")) self.assertIsNone(result) - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_timeout(self, mock_run): mock_run.side_effect = subprocess.TimeoutExpired(cmd="git", timeout=5) result = _extract_org_from_git_remote(Path("/fake")) self.assertIsNone(result) - @patch("apm_cli.policy.discovery._parse_remote_url") - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote._parse_remote_url") + @patch("apm_cli.policy._remote.subprocess.run") def test_remote_parser_value_error_returns_none(self, mock_run, mock_parse): mock_run.return_value = MagicMock( returncode=0, @@ -209,9 +280,9 @@ def test_remote_parser_value_error_returns_none(self, mock_run, mock_parse): self.assertIsNone(result) - @patch("apm_cli.policy.discovery.urlparse") - @patch("apm_cli.policy.discovery._parse_remote_url") - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.urlparse") + @patch("apm_cli.policy._remote._parse_remote_url") + @patch("apm_cli.policy._remote.subprocess.run") def test_remote_port_value_error_returns_none(self, mock_run, mock_parse, mock_urlparse): mock_run.return_value = MagicMock( returncode=0, @@ -730,7 +801,7 @@ def test_override_owner_repo(self, mock_fetch): self.assertIn("org:", result.source) @patch("apm_cli.policy.discovery._fetch_github_contents") - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_override_org_auto_discovers(self, mock_run, mock_fetch): mock_run.return_value = MagicMock( returncode=0, @@ -744,7 +815,7 @@ def test_override_org_auto_discovers(self, mock_run, mock_fetch): mock_fetch.assert_called_once() @patch("apm_cli.policy.discovery._fetch_github_contents") - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_none_auto_discovers(self, mock_run, mock_fetch): mock_run.return_value = MagicMock( returncode=0, @@ -757,7 +828,7 @@ def test_none_auto_discovers(self, mock_run, mock_fetch): self.assertTrue(result.found) self.assertEqual(result.source, "org:contoso/.github-private") - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_no_git_remote(self, mock_run): mock_run.return_value = MagicMock(returncode=1, stdout="") @@ -767,7 +838,7 @@ def test_no_git_remote(self, mock_run): self.assertIn("Could not determine org", result.error) @patch("apm_cli.policy.discovery._fetch_github_contents") - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_cache_hit_returns_cached(self, mock_run, mock_fetch): mock_run.return_value = MagicMock( returncode=0, @@ -788,7 +859,7 @@ def test_cache_hit_returns_cached(self, mock_run, mock_fetch): self.assertEqual(mock_fetch.call_count, 1) @patch("apm_cli.policy.discovery._fetch_github_contents") - @patch("apm_cli.policy.discovery.subprocess.run") + @patch("apm_cli.policy._remote.subprocess.run") def test_ghe_repo_ref_includes_host(self, mock_run, mock_fetch): mock_run.return_value = MagicMock( returncode=0, @@ -1047,7 +1118,7 @@ def fake_run(cmd, **kwargs): mock_candidates.return_value = ("_apm",) with ( - patch("apm_cli.policy.discovery.subprocess.run", side_effect=fake_run) as mock_run, + patch("apm_cli.policy._remote.subprocess.run", side_effect=fake_run) as mock_run, patch.dict(os.environ, {"ADO_HOST": "ado.example.test"}, clear=False), tempfile.TemporaryDirectory() as tmpdir, ): @@ -1112,6 +1183,193 @@ def test_gitlab_absent_is_clean_no_op(self, mock_extract, mock_gitlab_fetch): self.assertEqual(result.outcome, "absent") self.assertIsNone(result.error) + # --- GitLab subgroup walk: closest policy wins (#2753) --- + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy.discovery._gitlab_namespace_descending") + @patch("apm_cli.policy.discovery._extract_org_host_port_from_git_remote") + def test_gitlab_subgroup_closest_wins(self, mock_extract, mock_ns, mock_gitlab_fetch): + """The deepest subgroup with a policy wins; parents are not probed.""" + mock_extract.return_value = ("acme", "gitlab.com", None) + mock_ns.return_value = ["acme/dept-a/team-x", "acme/dept-a", "acme"] + mock_gitlab_fetch.return_value = PolicyFetchResult( + policy=ApmPolicy(), + source="org:gitlab.com/acme/dept-a/team-x/apm-policy", + outcome="found", + ) + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertTrue(result.found) + mock_gitlab_fetch.assert_called_once() + self.assertEqual(mock_gitlab_fetch.call_args.kwargs["org"], "acme/dept-a/team-x") + # Won at a subgroup below the top-level group -> flagged so the + # resolved level is surfaced even at default verbosity (#2753). + self.assertTrue(result.subgroup_scoped) + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy.discovery._gitlab_namespace_descending") + @patch("apm_cli.policy.discovery._extract_org_host_port_from_git_remote") + def test_gitlab_subgroup_absent_ascends_to_parent( + self, mock_extract, mock_ns, mock_gitlab_fetch + ): + """Absent at the deepest levels -> ascend until a policy is found.""" + mock_extract.return_value = ("acme", "gitlab.com", None) + mock_ns.return_value = ["acme/dept-a/team-x", "acme/dept-a", "acme"] + mock_gitlab_fetch.side_effect = [ + PolicyFetchResult(outcome="absent"), # team-x: no policy + PolicyFetchResult(outcome="absent"), # dept-a: no policy + PolicyFetchResult( + policy=ApmPolicy(), + source="org:gitlab.com/acme/apm-policy", + outcome="found", + ), # acme: top-level policy + ] + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertTrue(result.found) + self.assertEqual(mock_gitlab_fetch.call_count, 3) + probed = [c.kwargs["org"] for c in mock_gitlab_fetch.call_args_list] + self.assertEqual(probed, ["acme/dept-a/team-x", "acme/dept-a", "acme"]) + # Resolved at the top-level group -> NOT subgroup-scoped, so the + # pre-#2753 default-verbosity silence is preserved for this case. + self.assertFalse(result.subgroup_scoped) + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy.discovery._gitlab_namespace_descending") + @patch("apm_cli.policy.discovery._extract_org_host_port_from_git_remote") + def test_gitlab_subgroup_all_absent_is_clean_absent( + self, mock_extract, mock_ns, mock_gitlab_fetch + ): + """No policy at any level -> clean absent, every level probed once.""" + mock_extract.return_value = ("acme", "gitlab.com", None) + mock_ns.return_value = ["acme/dept-a/team-x", "acme/dept-a", "acme"] + mock_gitlab_fetch.return_value = PolicyFetchResult(outcome="absent") + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertEqual(result.outcome, "absent") + self.assertIsNone(result.error) + self.assertEqual(mock_gitlab_fetch.call_count, 3) + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy.discovery._gitlab_namespace_descending") + @patch("apm_cli.policy.discovery._extract_org_host_port_from_git_remote") + def test_gitlab_subgroup_error_fails_closed(self, mock_extract, mock_ns, mock_gitlab_fetch): + """A fetch error at the deepest level fails closed -- parents not probed.""" + mock_extract.return_value = ("acme", "gitlab.com", None) + mock_ns.return_value = ["acme/dept-a/team-x", "acme/dept-a", "acme"] + mock_gitlab_fetch.side_effect = [ + PolicyFetchResult(error="403: Access denied", outcome="cache_miss_fetch_fail"), + PolicyFetchResult(policy=ApmPolicy(), outcome="found"), + ] + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertFalse(result.found) + self.assertEqual(result.outcome, "cache_miss_fetch_fail") + mock_gitlab_fetch.assert_called_once() + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy.discovery._gitlab_namespace_descending") + @patch("apm_cli.policy.discovery._extract_org_host_port_from_git_remote") + def test_gitlab_subgroup_error_at_inner_level_fails_closed( + self, mock_extract, mock_ns, mock_gitlab_fetch + ): + """Fail-closed holds when the error appears after ascending (not first).""" + mock_extract.return_value = ("acme", "gitlab.com", None) + mock_ns.return_value = ["acme/dept-a/team-x", "acme/dept-a", "acme"] + mock_gitlab_fetch.side_effect = [ + PolicyFetchResult(outcome="absent"), # team-x: no policy -> ascend + PolicyFetchResult(error="403: Access denied", outcome="cache_miss_fetch_fail"), + PolicyFetchResult(policy=ApmPolicy(), outcome="found"), # must NOT be reached + ] + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertFalse(result.found) + self.assertEqual(result.outcome, "cache_miss_fetch_fail") + self.assertEqual(mock_gitlab_fetch.call_count, 2) + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy.discovery._gitlab_namespace_descending") + @patch("apm_cli.policy.discovery._extract_org_host_port_from_git_remote") + def test_gitlab_flat_project_falls_back_to_top_level_org( + self, mock_extract, mock_ns, mock_gitlab_fetch + ): + """No derivable namespace -> single top-level org probe (back-compat).""" + mock_extract.return_value = ("contoso", "gitlab.com", None) + mock_ns.return_value = None # namespace not derivable + mock_gitlab_fetch.return_value = PolicyFetchResult( + policy=ApmPolicy(), source="org:gitlab.com/contoso/apm-policy", outcome="found" + ) + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertTrue(result.found) + mock_gitlab_fetch.assert_called_once() + self.assertEqual(mock_gitlab_fetch.call_args.kwargs["org"], "contoso") + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy.discovery._gitlab_namespace_descending") + @patch("apm_cli.policy.discovery._extract_org_host_port_from_git_remote") + def test_gitlab_subgroup_empty_at_deep_level_shadows_ancestor( + self, mock_extract, mock_ns, mock_gitlab_fetch + ): + """An ``empty`` (present-but-no-rules) policy at a deep subgroup wins + and stops the ascent -- a README-only apm-policy at team-x is closest + and must NOT fall through to a real ancestor policy (#2753).""" + mock_extract.return_value = ("acme", "gitlab.com", None) + mock_ns.return_value = ["acme/dept-a/team-x", "acme/dept-a", "acme"] + mock_gitlab_fetch.side_effect = [ + PolicyFetchResult( + policy=ApmPolicy(), + source="org:gitlab.com/acme/dept-a/team-x/apm-policy", + outcome="empty", + ), # team-x: present but empty -> closest wins, stop + PolicyFetchResult(policy=ApmPolicy(), outcome="found"), # must NOT be reached + ] + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertEqual(result.outcome, "empty") + mock_gitlab_fetch.assert_called_once() + self.assertEqual(mock_gitlab_fetch.call_args.kwargs["org"], "acme/dept-a/team-x") + self.assertTrue(result.subgroup_scoped) + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + @patch("apm_cli.policy._remote.subprocess.run") + def test_gitlab_subgroup_walk_wiring_from_real_remote(self, mock_run, mock_gitlab_fetch): + """End-to-end seam: a real nested origin URL flows through the actual + namespace-descending helper into the walk (no _gitlab_namespace_descending + mock), so the ``_auto_discover`` wiring -- read origin once, derive the + descending namespaces from that same URL, probe deepest-first -- is + exercised, not just each half in isolation (#2753).""" + mock_run.return_value = MagicMock( + returncode=0, + stdout="https://gitlab.com/acme/dept-a/team-x/my-project.git\n", + ) + mock_gitlab_fetch.return_value = PolicyFetchResult(outcome="absent") + + with tempfile.TemporaryDirectory() as tmpdir: + result = _auto_discover(Path(tmpdir), no_cache=True) + + self.assertEqual(result.outcome, "absent") + probed = [c.kwargs["org"] for c in mock_gitlab_fetch.call_args_list] + self.assertEqual(probed, ["acme/dept-a/team-x", "acme/dept-a", "acme"]) + # Every level targeted the project's own remote host, never a foreign one. + self.assertTrue( + all(c.kwargs["host"] == "gitlab.com" for c in mock_gitlab_fetch.call_args_list) + ) + class TestPolicyRepoCandidates(unittest.TestCase): """Test _policy_repo_candidates host profile selection.""" @@ -1232,6 +1490,173 @@ def test_gitlab_parent_accepts_same_host_qualified_reference(self, mock_fetch): self.assertEqual(mock_fetch.call_args.kwargs["org"], "platform") self.assertEqual(mock_fetch.call_args.kwargs["repo"], "baseline") + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + def test_gitlab_parent_accepts_nested_namespace_reference(self, mock_fetch): + """An extends: ref into an intermediate subgroup resolves (#2753).""" + mock_fetch.return_value = PolicyFetchResult(outcome="absent") + + with tempfile.TemporaryDirectory() as tmpdir: + _fetch_chain_parent( + "acme/dept-a/apm-policy", + current_source="org:gitlab.com/acme/dept-a/team-x/apm-policy", + leaf_host="gitlab.com", + leaf_port=None, + project_root=Path(tmpdir), + no_cache=True, + ) + + # Namespace is everything before the final segment; repo is the last. + self.assertEqual(mock_fetch.call_args.kwargs["org"], "acme/dept-a") + self.assertEqual(mock_fetch.call_args.kwargs["repo"], "apm-policy") + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + def test_gitlab_parent_accepts_host_qualified_nested_namespace(self, mock_fetch): + """A host-qualified extends: ref into a nested subgroup strips the host.""" + mock_fetch.return_value = PolicyFetchResult(outcome="absent") + + with tempfile.TemporaryDirectory() as tmpdir: + _fetch_chain_parent( + "gitlab.com/acme/dept-a/apm-policy", + current_source="org:gitlab.com/acme/dept-a/team-x/apm-policy", + leaf_host="gitlab.com", + leaf_port=None, + project_root=Path(tmpdir), + no_cache=True, + ) + + self.assertEqual(mock_fetch.call_args.kwargs["org"], "acme/dept-a") + self.assertEqual(mock_fetch.call_args.kwargs["repo"], "apm-policy") + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + def test_gitlab_parent_accepts_host_qualified_ref_with_matching_port(self, mock_fetch): + """A host-qualified extends: ref whose ``host:port`` matches the leaf's + non-null port is stripped and fetched -- the positive side of the + ``explicit.port == port`` gate (mismatch/malformed are covered + separately). Pins that a matching self-managed port proceeds.""" + mock_fetch.return_value = PolicyFetchResult(outcome="absent") + + with ( + patch.dict(os.environ, {"GITLAB_HOST": "gitlab.example.test"}, clear=False), + tempfile.TemporaryDirectory() as tmpdir, + ): + _fetch_chain_parent( + "gitlab.example.test:8443/acme/dept-a/apm-policy", + current_source="org:gitlab.example.test:8443/acme/dept-a/team-x/apm-policy", + leaf_host="gitlab.example.test", + leaf_port=8443, + project_root=Path(tmpdir), + no_cache=True, + ) + + self.assertEqual(mock_fetch.call_args.kwargs["org"], "acme/dept-a") + self.assertEqual(mock_fetch.call_args.kwargs["repo"], "apm-policy") + self.assertEqual(mock_fetch.call_args.kwargs["port"], 8443) + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + def test_gitlab_parent_rejects_single_segment_reference(self, mock_fetch): + """A bare single-segment extends: ref fails closed, never probes.""" + mock_fetch.return_value = PolicyFetchResult(outcome="found", policy=ApmPolicy()) + + with tempfile.TemporaryDirectory() as tmpdir: + result = _fetch_chain_parent( + "apm-policy", + current_source="org:gitlab.com/acme/apm-policy", + leaf_host="gitlab.com", + leaf_port=None, + project_root=Path(tmpdir), + no_cache=True, + ) + + self.assertEqual(result.outcome, "cache_miss_fetch_fail") + self.assertIn("Invalid GitLab policy reference", result.error) + mock_fetch.assert_not_called() + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + def test_gitlab_parent_rejects_host_prefix_with_mismatched_port(self, mock_fetch): + """A host-like prefix that does not match the leaf host+port is rejected, + never silently folded into the namespace.""" + mock_fetch.return_value = PolicyFetchResult(outcome="found", policy=ApmPolicy()) + + with ( + patch.dict(os.environ, {"GITLAB_HOST": "gitlab.example.test"}, clear=False), + tempfile.TemporaryDirectory() as tmpdir, + ): + result = _fetch_chain_parent( + "gitlab.example.test/acme/apm-policy", # host prefix, but leaf has :8443 + current_source="org:gitlab.example.test:8443/acme/team/apm-policy", + leaf_host="gitlab.example.test", + leaf_port=8443, + project_root=Path(tmpdir), + no_cache=True, + ) + + self.assertEqual(result.outcome, "cache_miss_fetch_fail") + self.assertIn("Invalid GitLab policy reference", result.error) + mock_fetch.assert_not_called() + + @patch("apm_cli.policy._gitlab._fetch_from_gitlab_repo") + def test_gitlab_parent_rejects_malformed_port_without_crashing(self, mock_fetch): + """A host-like prefix with a malformed port fails closed, not raises.""" + mock_fetch.return_value = PolicyFetchResult(outcome="found", policy=ApmPolicy()) + + with ( + patch.dict(os.environ, {"GITLAB_HOST": "gitlab.example.test"}, clear=False), + tempfile.TemporaryDirectory() as tmpdir, + ): + result = _fetch_chain_parent( + "gitlab.example.test:not-a-port/acme/apm-policy", + current_source="org:gitlab.example.test/acme/team/apm-policy", + leaf_host="gitlab.example.test", + leaf_port=None, + project_root=Path(tmpdir), + no_cache=True, + ) + + self.assertEqual(result.outcome, "cache_miss_fetch_fail") + self.assertIn("Invalid GitLab policy reference", result.error) + mock_fetch.assert_not_called() + + +class TestValidateExtendsHostNestedNamespace(unittest.TestCase): + """The pre-fetch host-pin guard must allow nested GitLab namespaces (#2753). + + ``_validate_extends_host`` runs BEFORE the GitLab adapter fetch, so a bare + nested-namespace ``extends:`` ref (single-label first segment) must not be + misread as a cross-host reference -- while a real attacker FQDN still is. + """ + + def test_nested_namespace_ref_is_not_a_host_on_gitlab_leaf(self): + # First segment is a top-level group, not a host -> shorthand (GitLab). + self.assertIsNone(_extract_extends_host("acme/dept-a/apm-policy", "gitlab.com")) + + def test_single_label_first_segment_stays_a_host_on_github_leaf(self): + # F1: on a non-GitLab leaf a single-label first segment is a host, so a + # cross-host ``extends: "evil/org/repo"`` must NOT be treated as + # same-host (which would route a credential to ``evil``). + self.assertEqual(_extract_extends_host("evil/org/.github", "github.com"), "evil") + + def test_host_qualified_nested_ref_extracts_host(self): + self.assertEqual( + _extract_extends_host("gitlab.com/acme/dept-a/apm-policy", "gitlab.com"), + "gitlab.com", + ) + + def test_validate_allows_nested_same_host_reference(self): + # Must NOT raise: this is the exact ref the subgroup feature enables. + _validate_extends_host("gitlab.com", "acme/dept-a/apm-policy") + + def test_validate_rejects_single_label_cross_host_on_github_leaf(self): + import apm_cli.policy.inheritance as _inh + + with self.assertRaisesRegex(_inh.PolicyInheritanceError, "cross-host"): + _validate_extends_host("github.com", "evil/org/.github") + + def test_validate_still_rejects_cross_host_fqdn(self): + import apm_cli.policy.inheritance as _inh + + with self.assertRaisesRegex(_inh.PolicyInheritanceError, "cross-host"): + _validate_extends_host("gitlab.com", "evil.example.com/org/apm-policy") + class TestFetchAdoContents(unittest.TestCase): """Test _fetch_ado_contents for Azure DevOps Items API.""" diff --git a/tests/unit/policy/test_discovery_phase3w4.py b/tests/unit/policy/test_discovery_phase3w4.py index 8da423e05e..dfb370b032 100644 --- a/tests/unit/policy/test_discovery_phase3w4.py +++ b/tests/unit/policy/test_discovery_phase3w4.py @@ -358,7 +358,7 @@ def test_ssh_empty_path_returns_none(self) -> None: def test_https_url_parse_exception_returns_none(self) -> None: """HTTPS URL where urlparse raises -> returns None.""" - with patch("apm_cli.policy.discovery.urlparse", side_effect=Exception("boom")): + with patch("apm_cli.policy._remote.urlparse", side_effect=Exception("boom")): result = _parse_remote_url("https://github.com/owner/repo") assert result is None diff --git a/tests/unit/policy/test_discovery_policy_resolution.py b/tests/unit/policy/test_discovery_policy_resolution.py index ff3a96f9af..29cdb10444 100644 --- a/tests/unit/policy/test_discovery_policy_resolution.py +++ b/tests/unit/policy/test_discovery_policy_resolution.py @@ -435,7 +435,7 @@ def test_ssh_empty_path_returns_none(self) -> None: def test_https_url_parse_exception_returns_none(self) -> None: """HTTPS URL where urlparse raises -> returns None.""" - with patch("apm_cli.policy.discovery.urlparse", side_effect=Exception("boom")): + with patch("apm_cli.policy._remote.urlparse", side_effect=Exception("boom")): result = _parse_remote_url("https://github.com/owner/repo") assert result is None diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index 59f7b4ed61..9ac28393cb 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -643,6 +643,7 @@ def exiting_import( install-deployment-outcome install-deployment-package-target-authorization install-deployment-plugin-bin-eligibility +install-deployment-policy-remote-origin-owner install-deployment-primitive-classification install-deployment-prospective-dry-run-plan install-deployment-provenance-state