Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **`git: parent` monorepo transitive dependency inheritance:** packages in a git monorepo can reference sibling paths via `{ git: parent, path: ... }` without repeating the full `git:` URL; the lockfile stores expanded host, repository, subdirectory path, and resolved ref/commit like other virtual git dependencies (no `parent` sentinel as durable identity). (#1149)
- If you use the `gh` CLI, APM is now zero-config for private GitHub packages on github.com, `*.ghe.com`, and GHES: APM uses your active `gh auth login` token (`gh auth token --hostname <host>`) before falling back to `git credential fill`. Silently skipped when `gh` is not installed or not logged in for the host. (#630)

### Fixed

- ADO full HTTPS URLs with sub-path virtual packages (e.g. `https://dev.azure.com/org/proj/_git/repo/sub/path`) are now parsed correctly instead of being rejected. (#1128)
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated

Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
### Changed

- `apm install --force` help text now states explicitly that the flag does NOT refresh refs; users who want newer commits should run `apm update`. (#1244)
Expand Down
84 changes: 72 additions & 12 deletions src/apm_cli/models/dependency/reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -1078,16 +1078,24 @@ def _resolve_shorthand_to_parsed_url(cls, repo_url, host):
return parsed_url, host

@classmethod
def _validate_url_repo_path(cls, parsed_url):
def _validate_url_repo_path(cls, parsed_url) -> tuple[str, str | None]:
"""Validate and normalise the repository path from a parsed URL.

Checks host support, strips ``.git`` suffixes, removes ``_git``
segments, and validates each path component against the allowed
character set for the detected host type.

For Azure DevOps URLs with extra path segments beyond
``org/project/repo`` (e.g.
``https://dev.azure.com/org/proj/_git/repo/sub/path``), the extra
segments are extracted as a virtual package path and validated with
the same rules as the shorthand virtual-path detector.

Returns:
repo_url (str): Normalised repository path
(e.g. ``owner/repo`` or ``org/project/repo``).
``(repo_url, virtual_path)`` where *repo_url* is the normalised
base repository path (e.g. ``owner/repo`` or
``org/project/repo``) and *virtual_path* is ``None`` unless
extra ADO sub-path segments were detected.
"""
hostname = parsed_url.hostname or ""
if not is_supported_git_host(hostname):
Expand All @@ -1107,11 +1115,46 @@ def _validate_url_repo_path(cls, parsed_url):

is_ado_host = is_azure_devops_hostname(hostname)

url_virtual_path: str | None = None

if is_ado_host:
if len(path_parts) != 3:
if len(path_parts) < 3:
raise ValueError(
f"Invalid Azure DevOps repository path: expected 'org/project/repo', got '{path}'"
)
if len(path_parts) > 3:
# Extra segments are a virtual sub-path (e.g. sub/path in
# https://dev.azure.com/org/proj/_git/repo/sub/path).
ado_virtual = "/".join(path_parts[3:])

# Security: reject path traversal in virtual path.
validate_path_segments(ado_virtual, context="virtual path")

# Reject removed .collection.yml extensions.
if any(ado_virtual.endswith(ext) for ext in cls.REMOVED_COLLECTION_EXTENSIONS):
raise ValueError(
f".collection.yml is no longer supported. "
f"Convert '{ado_virtual}' to an apm.yml with a "
f"'dependencies' section. "
f"See: https://microsoft.github.io/apm/guides/dependencies/"
)

# Accept any recognised virtual file extension; reject other
# dotted final segments (mirrors shorthand virtual detection).
if any(ado_virtual.endswith(ext) for ext in cls.VIRTUAL_FILE_EXTENSIONS):
pass
else:
last_segment = ado_virtual.split("/")[-1]
if "." in last_segment:
raise InvalidVirtualPackageExtensionError(
f"Invalid virtual package path '{ado_virtual}'. "
f"Individual files must end with one of: "
f"{', '.join(cls.VIRTUAL_FILE_EXTENSIONS)}. "
f"For subdirectory packages, the path should not have a file extension."
)

url_virtual_path = ado_virtual
path_parts = path_parts[:3]
else:
if len(path_parts) < 2:
raise ValueError(
Expand All @@ -1134,19 +1177,25 @@ def _validate_url_repo_path(cls, parsed_url):
if not re.match(allowed_pattern, part):
raise ValueError(f"Invalid repository path component: {part}")

return "/".join(path_parts)
return "/".join(path_parts), url_virtual_path

@classmethod
def _parse_standard_url(
cls, dependency_str: str, is_virtual_package: bool, virtual_path, validated_host
):
cls,
dependency_str: str,
is_virtual_package: bool,
virtual_path: str | None,
validated_host: str | None,
) -> tuple[str, int | None, str, str | None, str | None, bool, str | None]:
"""Parse a non-SSH dependency string (HTTPS, FQDN, or shorthand).

Detects scheme vs shorthand, delegates host-specific resolution to
helpers, then validates the resulting URL path.

Returns:
``(host, port, repo_url, reference, alias)``
``(host, port, repo_url, reference, alias, effective_is_virtual,
effective_virtual_path)`` -- the last two reflect any ADO sub-path
segments embedded in the URL itself (issue #1128).
"""
host = None
port = None
Expand Down Expand Up @@ -1185,12 +1234,21 @@ def _parse_standard_url(
else:
parsed_url, host = cls._resolve_shorthand_to_parsed_url(repo_url, host)

repo_url = cls._validate_url_repo_path(parsed_url)
repo_url, url_virtual_path = cls._validate_url_repo_path(parsed_url)

# If URL contained extra ADO sub-path segments, they become the virtual
# path (overriding the _detect_virtual_package result which returns
# early for https:// URLs).
effective_is_virtual = is_virtual_package
effective_virtual_path = virtual_path
if url_virtual_path is not None:
effective_is_virtual = True
effective_virtual_path = url_virtual_path

if not host:
host = default_host()

return host, port, repo_url, reference, alias
return host, port, repo_url, reference, alias, effective_is_virtual, effective_virtual_path

@classmethod
def _validate_final_repo_fields(cls, host, repo_url):
Expand Down Expand Up @@ -1333,8 +1391,10 @@ def parse(cls, dependency_str: str) -> "DependencyReference":
host, port, repo_url, reference, alias = scp_result
explicit_scheme = "ssh"
else:
host, port, repo_url, reference, alias = cls._parse_standard_url(
dependency_str, is_virtual_package, virtual_path, validated_host
host, port, repo_url, reference, alias, is_virtual_package, virtual_path = (
cls._parse_standard_url(
dependency_str, is_virtual_package, virtual_path, validated_host
)
)
_stripped = dependency_str.strip().lower()
if _stripped.startswith("https://"):
Expand Down
44 changes: 44 additions & 0 deletions tests/unit/test_ado_path_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,3 +615,47 @@ def test_prune_joinpath_works_for_variable_depth(self):
ado_parts = ["org", "project", "repo"]
ado_path = base.joinpath(*ado_parts)
assert ado_path.as_posix().endswith("/tmp/apm_modules/org/project/repo")


class TestADOFullURLSubPath:
"""Test 8-shape matrix for ADO URL parsing (issue #1128)."""

@pytest.mark.parametrize(
"input_str,expected_virtual,expected_virtual_path,expected_ref",
[
# Case 1: shorthand base
("dev.azure.com/org/proj/_git/repo", False, None, None),
# Case 2: shorthand + virtual
("dev.azure.com/org/proj/_git/repo/sub/path", True, "sub/path", None),
# Case 3: shorthand + ref
("dev.azure.com/org/proj/_git/repo#main", False, None, "main"),
# Case 4: shorthand + virtual + ref
("dev.azure.com/org/proj/_git/repo/sub/path#main", True, "sub/path", "main"),
# Case 5: full URL base
("https://dev.azure.com/org/proj/_git/repo", False, None, None),
# Case 6: full URL + ref
("https://dev.azure.com/org/proj/_git/repo#main", False, None, "main"),
# Case 7: full URL + virtual (THE BUG)
("https://dev.azure.com/org/proj/_git/repo/sub/path", True, "sub/path", None),
# Case 8: full URL + virtual + ref (THE BUG)
("https://dev.azure.com/org/proj/_git/repo/sub/path#main", True, "sub/path", "main"),
],
)
def test_ado_url_matrix(
self,
input_str: str,
expected_virtual: bool,
expected_virtual_path: str | None,
expected_ref: str | None,
) -> None:
"""All 8 ADO URL forms parse correctly (issue #1128)."""
dep = DependencyReference.parse(input_str)

assert dep.host == "dev.azure.com"
Comment thread
sergio-sisternes-epam marked this conversation as resolved.
Outdated
assert dep.repo_url == "org/proj/repo"
assert dep.ado_organization == "org"
assert dep.ado_project == "proj"
assert dep.ado_repo == "repo"
assert dep.is_virtual == expected_virtual
assert dep.virtual_path == expected_virtual_path
assert dep.reference == expected_ref
Loading