From e0546066679abaa38bcfb48d6051149723c85c77 Mon Sep 17 00:00:00 2001 From: Prem Kumar Duraisamy Date: Tue, 24 Mar 2026 12:41:36 +0530 Subject: [PATCH 1/2] fix: allow spaces in ADO repo names when parsing URLs Azure DevOps repository names can contain spaces (URL-encoded as %20). The Phase 3 validation regex in DependencyReference.parse() was incorrectly using [a-zA-Z0-9._-]+ for the repo segment, which disallows spaces. Fixed the regex to match the already-correct intermediate validation pattern [a-zA-Z0-9._\\- ]+ that permits spaces in ADO names. Also fixed to_github_url() to percent-encode the repo name when it contains spaces, consistent with how the project name was already encoded. Fixes: apm install dev.azure.com/org/Project%20Name/_git/Repo%20Name failing with 'Invalid Azure DevOps repository format' error. --- src/apm_cli/models/dependency/reference.py | 484 +++++++------ tests/test_apm_package_models.py | 799 ++++++++++++--------- 2 files changed, 738 insertions(+), 545 deletions(-) diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index 55207a9e..a83572d9 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -20,99 +20,118 @@ from .types import VirtualPackageType -@dataclass +@dataclass class DependencyReference: """Represents a reference to an APM dependency.""" + repo_url: str # e.g., "user/repo" for GitHub or "org/project/repo" for Azure DevOps - host: Optional[str] = None # Optional host (github.com, dev.azure.com, or enterprise host) + host: Optional[str] = ( + None # Optional host (github.com, dev.azure.com, or enterprise host) + ) reference: Optional[str] = None # e.g., "main", "v1.0.0", "abc123" alias: Optional[str] = None # Optional alias for the dependency - virtual_path: Optional[str] = None # Path for virtual packages (e.g., "prompts/file.prompt.md") - is_virtual: bool = False # True if this is a virtual package (individual file or collection) - + virtual_path: Optional[str] = ( + None # Path for virtual packages (e.g., "prompts/file.prompt.md") + ) + is_virtual: bool = ( + False # True if this is a virtual package (individual file or collection) + ) + # Azure DevOps specific fields (ADO uses org/project/repo structure) ado_organization: Optional[str] = None # e.g., "dmeppiel-org" - ado_project: Optional[str] = None # e.g., "market-js-app" - ado_repo: Optional[str] = None # e.g., "compliance-rules" + ado_project: Optional[str] = None # e.g., "market-js-app" + ado_repo: Optional[str] = None # e.g., "compliance-rules" # Local path dependency fields is_local: bool = False # True if this is a local filesystem dependency - local_path: Optional[str] = None # Original local path string (e.g., "./packages/my-pkg") + local_path: Optional[str] = ( + None # Original local path string (e.g., "./packages/my-pkg") + ) + + artifactory_prefix: Optional[str] = ( + None # e.g., "artifactory/github" (repo key path) + ) - artifactory_prefix: Optional[str] = None # e.g., "artifactory/github" (repo key path) - # Supported file extensions for virtual packages - VIRTUAL_FILE_EXTENSIONS = ('.prompt.md', '.instructions.md', '.chatmode.md', '.agent.md') + VIRTUAL_FILE_EXTENSIONS = ( + ".prompt.md", + ".instructions.md", + ".chatmode.md", + ".agent.md", + ) def is_artifactory(self) -> bool: """Check if this reference points to a JFrog Artifactory VCS repository.""" return self.artifactory_prefix is not None - + def is_azure_devops(self) -> bool: """Check if this reference points to Azure DevOps.""" from ...utils.github_host import is_azure_devops_hostname + return self.host is not None and is_azure_devops_hostname(self.host) - + @property - def virtual_type(self) -> 'Optional[VirtualPackageType]': + def virtual_type(self) -> "Optional[VirtualPackageType]": """Return the type of virtual package, or None if not virtual.""" if not self.is_virtual or not self.virtual_path: return None if any(self.virtual_path.endswith(ext) for ext in self.VIRTUAL_FILE_EXTENSIONS): return VirtualPackageType.FILE - if '/collections/' in self.virtual_path or self.virtual_path.startswith('collections/'): + if "/collections/" in self.virtual_path or self.virtual_path.startswith( + "collections/" + ): return VirtualPackageType.COLLECTION return VirtualPackageType.SUBDIRECTORY def is_virtual_file(self) -> bool: """Check if this is a virtual file package (individual file).""" return self.virtual_type == VirtualPackageType.FILE - + def is_virtual_collection(self) -> bool: """Check if this is a virtual collection package.""" return self.virtual_type == VirtualPackageType.COLLECTION - + def is_virtual_subdirectory(self) -> bool: """Check if this is a virtual subdirectory package (e.g., Claude Skill). - + A subdirectory package is a virtual package that: - Has a virtual_path that is NOT a file extension we recognize - Is NOT a collection (doesn't have /collections/ in path) - Is a directory path (likely containing SKILL.md or apm.yml) - + Examples: - ComposioHQ/awesome-claude-skills/brand-guidelines → True - owner/repo/prompts/file.prompt.md → False (is_virtual_file) - owner/repo/collections/name → False (is_virtual_collection) """ return self.virtual_type == VirtualPackageType.SUBDIRECTORY - + def get_virtual_package_name(self) -> str: """Generate a package name for this virtual package. - + For virtual packages, we create a sanitized name from the path: - owner/repo/prompts/code-review.prompt.md → repo-code-review - owner/repo/collections/project-planning → repo-project-planning - owner/repo/collections/project-planning.collection.yml → repo-project-planning """ if not self.is_virtual or not self.virtual_path: - return self.repo_url.split('/')[-1] # Return repo name as fallback - + return self.repo_url.split("/")[-1] # Return repo name as fallback + # Extract repo name and file/collection name - repo_parts = self.repo_url.split('/') + repo_parts = self.repo_url.split("/") repo_name = repo_parts[-1] if repo_parts else "package" - + # Get the basename without extension - path_parts = self.virtual_path.split('/') + path_parts = self.virtual_path.split("/") if self.is_virtual_collection(): # For collections: use the collection name without extension # collections/project-planning → project-planning # collections/project-planning.collection.yml → project-planning collection_name = path_parts[-1] # Strip .collection.yml/.collection.yaml extension if present - for ext in ('.collection.yml', '.collection.yaml'): + for ext in (".collection.yml", ".collection.yaml"): if collection_name.endswith(ext): - collection_name = collection_name[:-len(ext)] + collection_name = collection_name[: -len(ext)] break return f"{repo_name}-{collection_name}" else: @@ -121,30 +140,30 @@ def get_virtual_package_name(self) -> str: filename = path_parts[-1] for ext in self.VIRTUAL_FILE_EXTENSIONS: if filename.endswith(ext): - filename = filename[:-len(ext)] + filename = filename[: -len(ext)] break return f"{repo_name}-{filename}" @staticmethod def is_local_path(dep_str: str) -> bool: """Check if a dependency string looks like a local filesystem path. - + Local paths start with './', '../', '/', or '~'. Protocol-relative URLs ('//...') are explicitly excluded. """ s = dep_str.strip() # Reject protocol-relative URLs ('//...') - if s.startswith('//'): + if s.startswith("//"): return False - return s.startswith(('./','../', '/', '~/', '~\\', '.\\', '..\\')) + return s.startswith(("./", "../", "/", "~/", "~\\", ".\\", "..\\")) def get_unique_key(self) -> str: """Get a unique key for this dependency for deduplication. - + For regular packages: repo_url For virtual packages: repo_url + virtual_path to ensure uniqueness For local packages: the local_path - + Returns: str: Unique key for this dependency """ @@ -153,19 +172,19 @@ def get_unique_key(self) -> str: if self.is_virtual and self.virtual_path: return f"{self.repo_url}/{self.virtual_path}" return self.repo_url - + def to_canonical(self) -> str: """Return the canonical form of this dependency for storage in apm.yml. - + Follows the Docker-style default-registry convention: - Default host (github.com) is stripped -> owner/repo - Non-default hosts are preserved -> gitlab.com/owner/repo - Virtual paths are appended -> owner/repo/path/to/thing - Refs are appended with # -> owner/repo#v1.0 - Local paths are returned as-is -> ./packages/my-pkg - + No .git suffix, no https://, no git@ -- just the canonical identifier. - + Returns: str: Canonical dependency string """ @@ -174,7 +193,7 @@ def to_canonical(self) -> str: host = self.host or default_host() is_default = host.lower() == default_host().lower() - + # Start with optional host prefix if is_default and not self.artifactory_prefix: result = self.repo_url @@ -182,23 +201,23 @@ def to_canonical(self) -> str: result = f"{host}/{self.artifactory_prefix}/{self.repo_url}" else: result = f"{host}/{self.repo_url}" - + # Append virtual path for virtual packages if self.is_virtual and self.virtual_path: result = f"{result}/{self.virtual_path}" - + # Append reference (branch, tag, commit) if self.reference: result = f"{result}#{self.reference}" - + return result - + def get_identity(self) -> str: """Return the identity of this dependency (canonical form without ref/alias). - + Two deps with the same identity are the same package, regardless of which ref or alias they specify. Used for duplicate detection and uninstall matching. - + Returns: str: Identity string (e.g., "owner/repo" or "gitlab.com/owner/repo/path") """ @@ -207,70 +226,70 @@ def get_identity(self) -> str: host = self.host or default_host() is_default = host.lower() == default_host().lower() - + if is_default and not self.artifactory_prefix: result = self.repo_url elif self.artifactory_prefix: result = f"{host}/{self.artifactory_prefix}/{self.repo_url}" else: result = f"{host}/{self.repo_url}" - + if self.is_virtual and self.virtual_path: result = f"{result}/{self.virtual_path}" - + return result - + @staticmethod def canonicalize(raw: str) -> str: """Parse any raw input form and return its canonical storage form. - + Convenience method that combines parse() + to_canonical(). - + Args: raw: Any supported input form (shorthand, FQDN, HTTPS, SSH, etc.) - + Returns: str: Canonical form for apm.yml storage """ return DependencyReference.parse(raw).to_canonical() - + def get_canonical_dependency_string(self) -> str: """Get the host-blind canonical string for filesystem and orphan-detection matching. - + This returns repo_url (+ virtual_path) without host prefix — it matches the filesystem layout in apm_modules/ which is also host-blind. - + For identity-based matching that includes non-default hosts, use get_identity(). For the full canonical form suitable for apm.yml storage, use to_canonical(). - + Returns: str: Host-blind canonical string (e.g., "owner/repo") """ return self.get_unique_key() - + def get_install_path(self, apm_modules_dir: Path) -> Path: """Get the canonical filesystem path where this package should be installed. - + This is the single source of truth for where a package lives in apm_modules/. - + For regular packages: - GitHub: apm_modules/owner/repo/ - ADO: apm_modules/org/project/repo/ - + For virtual file/collection packages: - GitHub: apm_modules/owner// - ADO: apm_modules/org/project// - + For subdirectory packages (Claude Skills, nested APM packages): - GitHub: apm_modules/owner/repo/subdir/path/ - ADO: apm_modules/org/project/repo/subdir/path/ For local packages: - apm_modules/_local// - + Args: apm_modules_dir: Path to the apm_modules directory - + Raises: PathTraversalError: If the computed path escapes apm_modules_dir Returns: @@ -278,7 +297,7 @@ def get_install_path(self, apm_modules_dir: Path) -> Path: """ if self.is_local and self.local_path: pkg_dir_name = Path(self.local_path).name - if pkg_dir_name in ('', '.', '..'): + if pkg_dir_name in ("", ".", ".."): raise PathTraversalError( f"Invalid local package path '{self.local_path}': " f"basename must not be empty, '.', or '..'" @@ -290,7 +309,7 @@ def get_install_path(self, apm_modules_dir: Path) -> Path: repo_parts = self.repo_url.split("/") # Security: reject traversal in repo_url segments (catches lockfile injection) - if any(seg in ('.', '..') for seg in repo_parts): + if any(seg in (".", "..") for seg in repo_parts): raise PathTraversalError( f"Invalid repo_url '{self.repo_url}': " f"path segments must not be '.' or '..'" @@ -298,21 +317,28 @@ def get_install_path(self, apm_modules_dir: Path) -> Path: # Security: reject traversal in virtual_path (catches lockfile injection) if self.virtual_path and any( - seg in ('.', '..') for seg in self.virtual_path.replace('\\', '/').split('/') + seg in (".", "..") + for seg in self.virtual_path.replace("\\", "/").split("/") ): raise PathTraversalError( f"Invalid virtual_path '{self.virtual_path}': " f"path segments must not be '.' or '..'" ) result: Path | None = None - + if self.is_virtual: # Subdirectory packages (like Claude Skills) should use natural path structure if self.is_virtual_subdirectory(): # Use repo path + subdirectory path if self.is_azure_devops() and len(repo_parts) >= 3: # ADO: org/project/repo/subdir - result = apm_modules_dir / repo_parts[0] / repo_parts[1] / repo_parts[2] / self.virtual_path + result = ( + apm_modules_dir + / repo_parts[0] + / repo_parts[1] + / repo_parts[2] + / self.virtual_path + ) elif len(repo_parts) >= 2: # owner/repo/subdir or group/subgroup/repo/subdir result = apm_modules_dir.joinpath(*repo_parts, self.virtual_path) @@ -321,7 +347,9 @@ def get_install_path(self, apm_modules_dir: Path) -> Path: package_name = self.get_virtual_package_name() if self.is_azure_devops() and len(repo_parts) >= 3: # ADO: org/project/virtual-pkg-name - result = apm_modules_dir / repo_parts[0] / repo_parts[1] / package_name + result = ( + apm_modules_dir / repo_parts[0] / repo_parts[1] / package_name + ) elif len(repo_parts) >= 2: # owner/virtual-pkg-name (use first segment as namespace) result = apm_modules_dir / repo_parts[0] / package_name @@ -341,43 +369,43 @@ def get_install_path(self, apm_modules_dir: Path) -> Path: # Security: ensure the computed path stays within apm_modules/ ensure_path_within(result, apm_modules_dir) return result - + @staticmethod def _normalize_ssh_protocol_url(url: str) -> str: """Normalize ssh:// protocol URLs to git@ format for consistent parsing. - + Converts: - ssh://git@gitlab.com/owner/repo.git → git@gitlab.com:owner/repo.git - ssh://git@host:port/owner/repo.git → git@host:owner/repo.git - + Non-SSH URLs are returned unchanged. """ - if not url.startswith('ssh://'): + if not url.startswith("ssh://"): return url - + # Parse the ssh:// URL # Format: ssh://[user@]host[:port]/path remainder = url[6:] # Remove 'ssh://' - + # Extract user if present (typically 'git@') user_prefix = "" - if '@' in remainder.split('/')[0]: - user_at_idx = remainder.index('@') - user_prefix = remainder[:user_at_idx + 1] # e.g., "git@" - remainder = remainder[user_at_idx + 1:] - + if "@" in remainder.split("/")[0]: + user_at_idx = remainder.index("@") + user_prefix = remainder[: user_at_idx + 1] # e.g., "git@" + remainder = remainder[user_at_idx + 1 :] + # Extract host (and optional port) - slash_idx = remainder.find('/') + slash_idx = remainder.find("/") if slash_idx == -1: return url # Invalid format, return as-is - + host_part = remainder[:slash_idx] - path_part = remainder[slash_idx + 1:] - + path_part = remainder[slash_idx + 1 :] + # Strip port if present (e.g., host:22) - if ':' in host_part: - host_part = host_part.split(':')[0] - + if ":" in host_part: + host_part = host_part.split(":")[0] + # Convert to git@ format: git@host:path if user_prefix: return f"{user_prefix}{host_part}:{path_part}" @@ -387,32 +415,32 @@ def _normalize_ssh_protocol_url(url: str) -> str: @classmethod def parse_from_dict(cls, entry: dict) -> "DependencyReference": """Parse an object-style dependency entry from apm.yml. - + Supports the Cargo-inspired object format: - + - git: https://gitlab.com/acme/coding-standards.git path: instructions/security ref: v2.0 - + - git: git@bitbucket.org:team/rules.git path: prompts/review.prompt.md - + Also supports local path entries: - + - path: ./packages/my-shared-skills - + Args: entry: Dictionary with 'git' or 'path' (required), plus optional fields - + Returns: DependencyReference: Parsed dependency reference - + Raises: ValueError: If the entry is missing required fields or has invalid format """ # Support dict-form local path: { path: ./local/dir } - if 'path' in entry and 'git' not in entry: - local = entry['path'] + if "path" in entry and "git" not in entry: + local = entry["path"] if not isinstance(local, str) or not local.strip(): raise ValueError("'path' field must be a non-empty string") local = local.strip() @@ -424,52 +452,56 @@ def parse_from_dict(cls, entry: dict) -> "DependencyReference": ) return cls.parse(local) - if 'git' not in entry: - raise ValueError("Object-style dependency must have a 'git' or 'path' field") - - git_url = entry['git'] + if "git" not in entry: + raise ValueError( + "Object-style dependency must have a 'git' or 'path' field" + ) + + git_url = entry["git"] if not isinstance(git_url, str) or not git_url.strip(): raise ValueError("'git' field must be a non-empty string") - - sub_path = entry.get('path') - ref_override = entry.get('ref') - alias_override = entry.get('alias') - + + sub_path = entry.get("path") + ref_override = entry.get("ref") + alias_override = entry.get("alias") + # Validate sub_path if provided if sub_path is not None: if not isinstance(sub_path, str) or not sub_path.strip(): raise ValueError("'path' field must be a non-empty string") - sub_path = sub_path.strip().strip('/') + sub_path = sub_path.strip().strip("/") # Normalize backslashes to forward slashes for cross-platform safety - sub_path = sub_path.replace('\\', '/').strip().strip('/') + sub_path = sub_path.replace("\\", "/").strip().strip("/") # Security: reject path traversal - if any(seg in ('.', '..') for seg in sub_path.split('/')): + if any(seg in (".", "..") for seg in sub_path.split("/")): raise PathTraversalError( f"Invalid path '{sub_path}': path segments must not be '.' or '..'" ) - + # Parse the git URL using the standard parser dep = cls.parse(git_url) - + # Apply overrides from the object fields if ref_override is not None: if not isinstance(ref_override, str) or not ref_override.strip(): raise ValueError("'ref' field must be a non-empty string") dep.reference = ref_override.strip() - + if alias_override is not None: if not isinstance(alias_override, str) or not alias_override.strip(): raise ValueError("'alias' field must be a non-empty string") alias_override = alias_override.strip() - if not re.match(r'^[a-zA-Z0-9._-]+$', alias_override): - raise ValueError(f"Invalid alias: {alias_override}. Aliases can only contain letters, numbers, dots, underscores, and hyphens") + if not re.match(r"^[a-zA-Z0-9._-]+$", alias_override): + raise ValueError( + f"Invalid alias: {alias_override}. Aliases can only contain letters, numbers, dots, underscores, and hyphens" + ) dep.alias = alias_override - + # Apply sub-path as virtual package if sub_path: dep.virtual_path = sub_path dep.is_virtual = True - + return dep @classmethod @@ -481,22 +513,22 @@ def _detect_virtual_package(cls, dependency_str: str): """ # Temporarily remove reference for path segment counting temp_str = dependency_str - if '#' in temp_str: - temp_str = temp_str.rsplit('#', 1)[0] + if "#" in temp_str: + temp_str = temp_str.rsplit("#", 1)[0] is_virtual_package = False virtual_path = None validated_host = None - if temp_str.startswith(('git@', 'https://', 'http://')): + if temp_str.startswith(("git@", "https://", "http://")): return is_virtual_package, virtual_path, validated_host check_str = temp_str - if '/' in check_str: - first_segment = check_str.split('/')[0] + if "/" in check_str: + first_segment = check_str.split("/")[0] - if '.' in first_segment: + if "." in first_segment: test_url = f"https://{check_str}" try: parsed = urllib.parse.urlparse(test_url) @@ -504,9 +536,9 @@ def _detect_virtual_package(cls, dependency_str: str): if hostname and is_supported_git_host(hostname): validated_host = hostname - path_parts = parsed.path.lstrip('/').split('/') + path_parts = parsed.path.lstrip("/").split("/") if len(path_parts) >= 2: - check_str = '/'.join(check_str.split('/')[1:]) + check_str = "/".join(check_str.split("/")[1:]) else: raise ValueError( unsupported_host_error(hostname or first_segment) @@ -514,22 +546,22 @@ def _detect_virtual_package(cls, dependency_str: str): except (ValueError, AttributeError) as e: if isinstance(e, ValueError) and "Invalid Git host" in str(e): raise - raise ValueError( - unsupported_host_error(first_segment) - ) - elif check_str.startswith('gh/'): - check_str = '/'.join(check_str.split('/')[1:]) + raise ValueError(unsupported_host_error(first_segment)) + elif check_str.startswith("gh/"): + check_str = "/".join(check_str.split("/")[1:]) - path_segments = [seg for seg in check_str.split('/') if seg] + path_segments = [seg for seg in check_str.split("/") if seg] is_ado = validated_host is not None and is_azure_devops_hostname(validated_host) - is_generic_host = (validated_host is not None - and not is_github_hostname(validated_host) - and not is_azure_devops_hostname(validated_host)) + is_generic_host = ( + validated_host is not None + and not is_github_hostname(validated_host) + and not is_azure_devops_hostname(validated_host) + ) - if is_ado and '_git' in path_segments: - git_idx = path_segments.index('_git') - path_segments = path_segments[:git_idx] + path_segments[git_idx+1:] + if is_ado and "_git" in path_segments: + git_idx = path_segments.index("_git") + path_segments = path_segments[:git_idx] + path_segments[git_idx + 1 :] # Detect Artifactory VCS paths (artifactory/{repo-key}/{owner}/{repo}) is_artifactory = is_generic_host and is_artifactory_path(path_segments) @@ -544,7 +576,7 @@ def _detect_virtual_package(cls, dependency_str: str): any(seg.endswith(ext) for ext in cls.VIRTUAL_FILE_EXTENSIONS) for seg in path_segments ) - has_collection = 'collections' in path_segments + has_collection = "collections" in path_segments if has_virtual_ext or has_collection: min_base_segments = 2 else: @@ -556,22 +588,22 @@ def _detect_virtual_package(cls, dependency_str: str): if len(path_segments) >= min_virtual_segments: is_virtual_package = True - virtual_path = '/'.join(path_segments[min_base_segments:]) + virtual_path = "/".join(path_segments[min_base_segments:]) # Security: reject path traversal in virtual path - vp_check = virtual_path.replace('\\', '/') - if any(seg in ('.', '..') for seg in vp_check.split('/')): + vp_check = virtual_path.replace("\\", "/") + if any(seg in (".", "..") for seg in vp_check.split("/")): raise PathTraversalError( f"Invalid virtual path '{virtual_path}': path segments must not be '.' or '..'" ) - if '/collections/' in check_str or virtual_path.startswith('collections/'): + if "/collections/" in check_str or virtual_path.startswith("collections/"): pass elif any(virtual_path.endswith(ext) for ext in cls.VIRTUAL_FILE_EXTENSIONS): pass else: - last_segment = virtual_path.split('/')[-1] - if '.' in last_segment: + last_segment = virtual_path.split("/")[-1] + if "." in last_segment: raise InvalidVirtualPackageExtensionError( f"Invalid virtual package path '{virtual_path}'. " f"Individual files must end with one of: {', '.join(cls.VIRTUAL_FILE_EXTENSIONS)}. " @@ -587,7 +619,7 @@ def _parse_ssh_url(dependency_str: str): Returns: ``(host, repo_url, reference, alias)`` or *None* if not an SSH URL. """ - ssh_match = re.match(r'^git@([^:]+):(.+)$', dependency_str) + ssh_match = re.match(r"^git@([^:]+):(.+)$", dependency_str) if not ssh_match: return None @@ -607,15 +639,16 @@ def _parse_ssh_url(dependency_str: str): else: repo_part = ssh_repo_part - if repo_part.endswith('.git'): + if repo_part.endswith(".git"): repo_part = repo_part[:-4] repo_url = repo_part.strip() return host, repo_url, reference, alias @classmethod - def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, - virtual_path, validated_host): + def _parse_standard_url( + cls, dependency_str: str, is_virtual_package: bool, virtual_path, validated_host + ): """Parse a non-SSH dependency string (HTTPS, FQDN, or shorthand). Returns: @@ -638,15 +671,17 @@ def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, if is_virtual_package and not repo_url.startswith(("https://", "http://")): parts = repo_url.split("/") - if '_git' in parts: - git_idx = parts.index('_git') - parts = parts[:git_idx] + parts[git_idx+1:] + if "_git" in parts: + git_idx = parts.index("_git") + parts = parts[:git_idx] + parts[git_idx + 1 :] if len(parts) >= 3 and is_supported_git_host(parts[0]): host = parts[0] if is_azure_devops_hostname(parts[0]): if len(parts) < 5: - raise ValueError("Invalid Azure DevOps virtual package format: must be dev.azure.com/org/project/repo/path") + raise ValueError( + "Invalid Azure DevOps virtual package format: must be dev.azure.com/org/project/repo/path" + ) repo_url = "/".join(parts[1:4]) elif is_artifactory_path(parts[1:]): art_result = parse_artifactory_path(parts[1:]) @@ -659,7 +694,9 @@ def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, host = default_host() if validated_host and is_azure_devops_hostname(validated_host): if len(parts) < 4: - raise ValueError("Invalid Azure DevOps virtual package format: expected at least org/project/repo/path") + raise ValueError( + "Invalid Azure DevOps virtual package format: expected at least org/project/repo/path" + ) repo_url = "/".join(parts[:3]) else: repo_url = "/".join(parts[:2]) @@ -671,15 +708,17 @@ def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, else: parts = repo_url.split("/") - if '_git' in parts: - git_idx = parts.index('_git') - parts = parts[:git_idx] + parts[git_idx+1:] + if "_git" in parts: + git_idx = parts.index("_git") + parts = parts[:git_idx] + parts[git_idx + 1 :] if len(parts) >= 3 and is_supported_git_host(parts[0]): host = parts[0] if is_azure_devops_hostname(host) and len(parts) >= 4: user_repo = "/".join(parts[1:4]) - elif not is_github_hostname(host) and not is_azure_devops_hostname(host): + elif not is_github_hostname(host) and not is_azure_devops_hostname( + host + ): if is_artifactory_path(parts[1:]): art_result = parse_artifactory_path(parts[1:]) if art_result: @@ -695,36 +734,50 @@ def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, host = default_host() if is_azure_devops_hostname(host) and len(parts) >= 3: user_repo = "/".join(parts[:3]) - elif host and not is_github_hostname(host) and not is_azure_devops_hostname(host): + elif ( + host + and not is_github_hostname(host) + and not is_azure_devops_hostname(host) + ): user_repo = "/".join(parts) else: user_repo = "/".join(parts[:2]) else: - raise ValueError(f"Use 'user/repo' or 'github.com/user/repo' or 'dev.azure.com/org/project/repo' format") + raise ValueError( + f"Use 'user/repo' or 'github.com/user/repo' or 'dev.azure.com/org/project/repo' format" + ) if not user_repo or "/" not in user_repo: - raise ValueError(f"Invalid repository format: {repo_url}. Expected 'user/repo' or 'org/project/repo'") + raise ValueError( + f"Invalid repository format: {repo_url}. Expected 'user/repo' or 'org/project/repo'" + ) uparts = user_repo.split("/") is_ado_host = host and is_azure_devops_hostname(host) if is_ado_host: if len(uparts) < 3: - raise ValueError(f"Invalid Azure DevOps repository format: {repo_url}. Expected 'org/project/repo'") + raise ValueError( + f"Invalid Azure DevOps repository format: {repo_url}. Expected 'org/project/repo'" + ) else: if len(uparts) < 2: - raise ValueError(f"Invalid repository format: {repo_url}. Expected 'user/repo'") + raise ValueError( + f"Invalid repository format: {repo_url}. Expected 'user/repo'" + ) - allowed_pattern = r'^[a-zA-Z0-9._\- ]+$' if is_ado_host else r'^[a-zA-Z0-9._-]+$' + allowed_pattern = ( + r"^[a-zA-Z0-9._\- ]+$" if is_ado_host else r"^[a-zA-Z0-9._-]+$" + ) for part in uparts: - if part in ('.', '..'): + if part in (".", ".."): raise PathTraversalError( f"Invalid repository path component: '{part}' is a traversal sequence" ) - if not re.match(allowed_pattern, part.rstrip('.git')): + if not re.match(allowed_pattern, part.rstrip(".git")): raise ValueError(f"Invalid repository path component: {part}") - quoted_repo = '/'.join(urllib.parse.quote(p, safe='') for p in uparts) + quoted_repo = "/".join(urllib.parse.quote(p, safe="") for p in uparts) github_url = urllib.parse.urljoin(f"https://{host}/", quoted_repo) parsed_url = urllib.parse.urlparse(github_url) @@ -740,18 +793,22 @@ def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, path = path[:-4] path_parts = [urllib.parse.unquote(p) for p in path.split("/")] - if '_git' in path_parts: - git_idx = path_parts.index('_git') - path_parts = path_parts[:git_idx] + path_parts[git_idx+1:] + if "_git" in path_parts: + git_idx = path_parts.index("_git") + path_parts = path_parts[:git_idx] + path_parts[git_idx + 1 :] is_ado_host = is_azure_devops_hostname(hostname) if is_ado_host: if len(path_parts) != 3: - raise ValueError(f"Invalid Azure DevOps repository path: expected 'org/project/repo', got '{path}'") + raise ValueError( + f"Invalid Azure DevOps repository path: expected 'org/project/repo', got '{path}'" + ) else: if len(path_parts) < 2: - raise ValueError(f"Invalid repository path: expected at least 'user/repo', got '{path}'") + raise ValueError( + f"Invalid repository path: expected at least 'user/repo', got '{path}'" + ) for pp in path_parts: if any(pp.endswith(ext) for ext in cls.VIRTUAL_FILE_EXTENSIONS): raise ValueError( @@ -759,10 +816,14 @@ def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, f"Use the dict format with 'path:' for virtual packages in HTTPS URLs" ) - allowed_pattern = r'^[a-zA-Z0-9._\- ]+$' if is_ado_host else r'^[a-zA-Z0-9._-]+$' + allowed_pattern = ( + r"^[a-zA-Z0-9._\- ]+$" if is_ado_host else r"^[a-zA-Z0-9._-]+$" + ) for i, part in enumerate(path_parts): if not part: - raise ValueError(f"Invalid repository format: path component {i+1} cannot be empty") + raise ValueError( + f"Invalid repository format: path component {i+1} cannot be empty" + ) if not re.match(allowed_pattern, part): raise ValueError(f"Invalid repository path component: {part}") @@ -776,7 +837,7 @@ def _parse_standard_url(cls, dependency_str: str, is_virtual_package: bool, @classmethod def parse(cls, dependency_str: str) -> "DependencyReference": """Parse a dependency string into a DependencyReference. - + Supports formats: - user/repo - user/repo#branch @@ -790,20 +851,20 @@ def parse(cls, dependency_str: str) -> "DependencyReference": - https://gitlab.com/owner/repo.git (generic HTTPS git URL) - git@gitlab.com:owner/repo.git (SSH git URL) - ssh://git@gitlab.com/owner/repo.git (SSH protocol URL) - + - ./local/path (local filesystem path) - /absolute/path (local filesystem path) - ../relative/path (local filesystem path) - + Any valid FQDN is accepted as a git host (GitHub, GitLab, Bitbucket, self-hosted instances, etc.). - + Args: dependency_str: The dependency string to parse - + Returns: DependencyReference: Parsed dependency reference - + Raises: ValueError: If the dependency string format is invalid """ @@ -819,7 +880,7 @@ def parse(cls, dependency_str: str) -> "DependencyReference": if cls.is_local_path(dependency_str): local = dependency_str.strip() pkg_name = Path(local).name - if not pkg_name or pkg_name in ('.', '..'): + if not pkg_name or pkg_name in (".", ".."): raise ValueError( f"Local path '{local}' does not resolve to a named directory. " f"Use a path that ends with a directory name " @@ -830,14 +891,20 @@ def parse(cls, dependency_str: str) -> "DependencyReference": is_local=True, local_path=local, ) - - if dependency_str.startswith('//'): - raise ValueError(unsupported_host_error("//...", context="Protocol-relative URLs are not supported")) - + + if dependency_str.startswith("//"): + raise ValueError( + unsupported_host_error( + "//...", context="Protocol-relative URLs are not supported" + ) + ) + dependency_str = cls._normalize_ssh_protocol_url(dependency_str) # Phase 1: detect virtual packages - is_virtual_package, virtual_path, validated_host = cls._detect_virtual_package(dependency_str) + is_virtual_package, virtual_path, validated_host = cls._detect_virtual_package( + dependency_str + ) # Phase 2: parse SSH or standard URL ssh_result = cls._parse_ssh_url(dependency_str) @@ -851,18 +918,26 @@ def parse(cls, dependency_str: str) -> "DependencyReference": # Phase 3: final validation and ADO field extraction is_ado_final = host and is_azure_devops_hostname(host) if is_ado_final: - if not re.match(r'^[a-zA-Z0-9._-]+/[a-zA-Z0-9._\- ]+/[a-zA-Z0-9._-]+$', repo_url): - raise ValueError(f"Invalid Azure DevOps repository format: {repo_url}. Expected 'org/project/repo'") - ado_parts = repo_url.split('/') + if not re.match( + r"^[a-zA-Z0-9._-]+/[a-zA-Z0-9._\- ]+/[a-zA-Z0-9._\- ]+$", repo_url + ): + raise ValueError( + f"Invalid Azure DevOps repository format: {repo_url}. Expected 'org/project/repo'" + ) + ado_parts = repo_url.split("/") ado_organization = ado_parts[0] ado_project = ado_parts[1] ado_repo = ado_parts[2] else: - segments = repo_url.split('/') + segments = repo_url.split("/") if len(segments) < 2: - raise ValueError(f"Invalid repository format: {repo_url}. Expected 'user/repo'") - if not all(re.match(r'^[a-zA-Z0-9._-]+$', s) for s in segments): - raise ValueError(f"Invalid repository format: {repo_url}. Contains invalid characters") + raise ValueError( + f"Invalid repository format: {repo_url}. Expected 'user/repo'" + ) + if not all(re.match(r"^[a-zA-Z0-9._-]+$", s) for s in segments): + raise ValueError( + f"Invalid repository format: {repo_url}. Contains invalid characters" + ) for seg in segments: if any(seg.endswith(ext) for ext in cls.VIRTUAL_FILE_EXTENSIONS): raise ValueError( @@ -872,17 +947,19 @@ def parse(cls, dependency_str: str) -> "DependencyReference": ado_organization = None ado_project = None ado_repo = None - - if alias and not re.match(r'^[a-zA-Z0-9._-]+$', alias): - raise ValueError(f"Invalid alias: {alias}. Aliases can only contain letters, numbers, dots, underscores, and hyphens") + + if alias and not re.match(r"^[a-zA-Z0-9._-]+$", alias): + raise ValueError( + f"Invalid alias: {alias}. Aliases can only contain letters, numbers, dots, underscores, and hyphens" + ) # Extract Artifactory prefix from the original path if applicable artifactory_prefix = None if host and not is_ado_final: - _art_str = dependency_str.split('#')[0].split('@')[0] + _art_str = dependency_str.split("#")[0].split("@")[0] # Strip scheme if present (e.g., https://host/artifactory/...) - if '://' in _art_str: - _art_str = _art_str.split('://', 1)[1] + if "://" in _art_str: + _art_str = _art_str.split("://", 1)[1] _art_segs = _art_str.replace(f"{host}/", "", 1).split("/") if is_artifactory_path(_art_segs): art_result = parse_artifactory_path(_art_segs) @@ -899,12 +976,12 @@ def parse(cls, dependency_str: str) -> "DependencyReference": ado_organization=ado_organization, ado_project=ado_project, ado_repo=ado_repo, - artifactory_prefix=artifactory_prefix + artifactory_prefix=artifactory_prefix, ) def to_github_url(self) -> str: """Convert to full repository URL. - + For Azure DevOps, generates: https://dev.azure.com/org/project/_git/repo For GitHub, generates: https://github.com/owner/repo For local packages, returns the local path. @@ -913,17 +990,18 @@ def to_github_url(self) -> str: return self.local_path host = self.host or default_host() - + if self.is_azure_devops(): # ADO format: https://dev.azure.com/org/project/_git/repo - project = urllib.parse.quote(self.ado_project, safe='') - return f"https://{host}/{self.ado_organization}/{project}/_git/{self.ado_repo}" + project = urllib.parse.quote(self.ado_project, safe="") + repo = urllib.parse.quote(self.ado_repo, safe="") + return f"https://{host}/{self.ado_organization}/{project}/_git/{repo}" elif self.artifactory_prefix: return f"https://{host}/{self.artifactory_prefix}/{self.repo_url}" else: # GitHub format: https://github.com/owner/repo return f"https://{host}/{self.repo_url}" - + def to_clone_url(self) -> str: """Convert to a clone-friendly URL (same as to_github_url for most purposes).""" return self.to_github_url() diff --git a/tests/test_apm_package_models.py b/tests/test_apm_package_models.py index a5f13aea..493dd3b2 100644 --- a/tests/test_apm_package_models.py +++ b/tests/test_apm_package_models.py @@ -1,71 +1,72 @@ """Unit tests for APM package data models and validation.""" import json -import pytest import tempfile -import yaml from pathlib import Path -from unittest.mock import patch, mock_open +from unittest.mock import mock_open, patch + +import pytest +import yaml +from apm_cli.utils import github_host from src.apm_cli.models.apm_package import ( APMPackage, - DependencyReference, - ValidationResult, - ValidationError, - ResolvedReference, - PackageInfo, + DependencyReference, GitReferenceType, PackageContentType, + PackageInfo, PackageType, - validate_apm_package, + ResolvedReference, + ValidationError, + ValidationResult, parse_git_reference, + validate_apm_package, ) -from apm_cli.utils import github_host class TestDependencyReference: """Test DependencyReference parsing and functionality.""" - + def test_parse_simple_repo(self): """Test parsing simple user/repo format.""" dep = DependencyReference.parse("user/repo") assert dep.repo_url == "user/repo" assert dep.reference is None assert dep.alias is None - + def test_parse_with_branch(self): """Test parsing with branch reference.""" dep = DependencyReference.parse("user/repo#main") assert dep.repo_url == "user/repo" assert dep.reference == "main" assert dep.alias is None - + def test_parse_with_tag(self): """Test parsing with tag reference.""" dep = DependencyReference.parse("user/repo#v1.0.0") assert dep.repo_url == "user/repo" assert dep.reference == "v1.0.0" assert dep.alias is None - + def test_parse_with_commit(self): """Test parsing with commit SHA.""" dep = DependencyReference.parse("user/repo#abc123def") assert dep.repo_url == "user/repo" assert dep.reference == "abc123def" assert dep.alias is None - + def test_parse_with_alias_shorthand_removed(self): """Shorthand @alias syntax is no longer supported — @ in shorthand is rejected.""" with pytest.raises(ValueError): DependencyReference.parse("user/repo@myalias") - + def test_parse_with_reference_and_alias_shorthand_not_parsed(self): """Shorthand #ref@alias — @ is no longer parsed as alias separator.""" dep = DependencyReference.parse("user/repo#main@myalias") assert dep.repo_url == "user/repo" assert dep.reference == "main@myalias" # @ treated as part of ref assert dep.alias is None - + def test_parse_github_urls(self): """Test parsing various GitHub URL formats.""" host = github_host.default_host() @@ -76,7 +77,7 @@ def test_parse_github_urls(self): f"git@{host}:user/repo", f"git@{host}:user/repo.git", ] - + for url_format in formats: dep = DependencyReference.parse(url_format) assert dep.repo_url == "user/repo" @@ -92,7 +93,7 @@ def test_parse_ghe_urls(self): for url_format in formats: dep = DependencyReference.parse(url_format) assert dep.repo_url == "user/repo" - + def test_parse_invalid_formats(self): """Test parsing invalid dependency formats.""" invalid_formats = [ @@ -101,11 +102,11 @@ def test_parse_invalid_formats(self): "just-repo-name", "user/", ] - + for invalid_format in invalid_formats: with pytest.raises(ValueError): DependencyReference.parse(invalid_format) - + def test_parse_malicious_url_bypass_attempts(self): """Test that URL parsing prevents injection attacks. @@ -121,25 +122,32 @@ def test_parse_malicious_url_bypass_attempts(self): # Attack vectors that should still be REJECTED rejected_formats = [ # Protocol-relative URL attacks - ("//evil.com/github.com/user/repo", "Protocol-relative URLs are not supported"), + ( + "//evil.com/github.com/user/repo", + "Protocol-relative URLs are not supported", + ), ] - + for malicious_url, expected_match in rejected_formats: with pytest.raises(ValueError, match=expected_match): DependencyReference.parse(malicious_url) - + # With generic git host support + nested groups, these are valid # (host is correctly identified, remaining segments are repo path) nested_group_on_generic_host = [ ("evil.com/github.com/user/repo", "evil.com", "github.com/user/repo"), - ("attacker.net/github.com/malicious/repo", "attacker.net", "github.com/malicious/repo"), + ( + "attacker.net/github.com/malicious/repo", + "attacker.net", + "github.com/malicious/repo", + ), ] for url, expected_host, expected_repo in nested_group_on_generic_host: dep = DependencyReference.parse(url) assert dep.host == expected_host assert dep.repo_url == expected_repo assert dep.is_virtual is False - + # With generic git host support, valid FQDNs are accepted as hosts. # These are not injection attacks — they are legitimate host references. accepted_as_generic_hosts = [ @@ -151,15 +159,15 @@ def test_parse_malicious_url_bypass_attempts(self): "GitHub.COM.evil.com/user/repo", "GITHUB.com.attacker.net/user/repo", ] - + for url in accepted_as_generic_hosts: dep = DependencyReference.parse(url) assert dep.repo_url == "user/repo" assert dep.host is not None - + def test_parse_legitimate_github_enterprise_formats(self): """Test that legitimate GitHub Enterprise hostnames are accepted. - + Ensures the security fix doesn't break valid GHE instances. According to is_github_hostname(), only github.com and *.ghe.com are valid. """ @@ -169,26 +177,28 @@ def test_parse_legitimate_github_enterprise_formats(self): "myorg.ghe.com/user/repo", "github.com/user/repo", # Standard GitHub ] - + for valid_url in valid_ghe_formats: dep = DependencyReference.parse(valid_url) assert dep.repo_url == "user/repo" assert dep.host is not None - + def test_parse_azure_devops_formats(self): """Test that Azure DevOps hostnames are accepted with org/project/repo format. - + Azure DevOps uses 3 segments (org/project/repo) instead of GitHub's 2 segments (owner/repo). """ # Full ADO URL with _git segment - dep = DependencyReference.parse("dev.azure.com/dmeppiel-org/market-js-app/_git/compliance-rules") + dep = DependencyReference.parse( + "dev.azure.com/dmeppiel-org/market-js-app/_git/compliance-rules" + ) assert dep.host == "dev.azure.com" assert dep.ado_organization == "dmeppiel-org" assert dep.ado_project == "market-js-app" assert dep.ado_repo == "compliance-rules" assert dep.is_azure_devops() == True assert dep.repo_url == "dmeppiel-org/market-js-app/compliance-rules" - + # Simplified ADO format (without _git) dep = DependencyReference.parse("dev.azure.com/myorg/myproject/myrepo") assert dep.host == "dev.azure.com" @@ -196,19 +206,23 @@ def test_parse_azure_devops_formats(self): assert dep.ado_project == "myproject" assert dep.ado_repo == "myrepo" assert dep.is_azure_devops() == True - + # Legacy visualstudio.com format - dep = DependencyReference.parse("mycompany.visualstudio.com/myorg/myproject/myrepo") + dep = DependencyReference.parse( + "mycompany.visualstudio.com/myorg/myproject/myrepo" + ) assert dep.host == "mycompany.visualstudio.com" assert dep.is_azure_devops() == True assert dep.ado_organization == "myorg" assert dep.ado_project == "myproject" assert dep.ado_repo == "myrepo" - + def test_parse_azure_devops_virtual_package(self): """Test ADO virtual package parsing with 4-segment format (org/project/repo/path).""" # ADO virtual package with host prefix - dep = DependencyReference.parse("dev.azure.com/myorg/myproject/myrepo/prompts/code-review.prompt.md") + dep = DependencyReference.parse( + "dev.azure.com/myorg/myproject/myrepo/prompts/code-review.prompt.md" + ) assert dep.is_azure_devops() == True assert dep.is_virtual == True assert dep.repo_url == "myorg/myproject/myrepo" @@ -216,9 +230,11 @@ def test_parse_azure_devops_virtual_package(self): assert dep.ado_organization == "myorg" assert dep.ado_project == "myproject" assert dep.ado_repo == "myrepo" - + # ADO virtual package with _git segment - dep = DependencyReference.parse("dev.azure.com/myorg/myproject/_git/myrepo/prompts/test.prompt.md") + dep = DependencyReference.parse( + "dev.azure.com/myorg/myproject/_git/myrepo/prompts/test.prompt.md" + ) assert dep.is_azure_devops() == True assert dep.is_virtual == True assert dep.virtual_path == "prompts/test.prompt.md" @@ -232,20 +248,20 @@ def test_parse_azure_devops_invalid_virtual_package(self): # The bounds check kicks in when we have a recognized virtual package format # but not enough segments. This test verifies ADO virtual package paths require # the full org/project/repo/path structure. - + # Valid 4-segment ADO virtual package should work dep = DependencyReference.parse("dev.azure.com/org/proj/repo/file.prompt.md") assert dep.is_virtual == True assert dep.repo_url == "org/proj/repo" - + # 3 segments after host (org/proj/repo) without a path - this is a regular package, not virtual dep = DependencyReference.parse("dev.azure.com/myorg/myproject/myrepo") assert dep.is_virtual == False assert dep.repo_url == "myorg/myproject/myrepo" - + def test_parse_azure_devops_project_with_spaces(self): """Test that ADO project names with spaces are correctly parsed. - + Azure DevOps project names can contain spaces (e.g., 'My Project'). Users may specify them with %20 encoding or literal spaces (shell-quoted). """ @@ -276,31 +292,69 @@ def test_parse_azure_devops_project_with_spaces(self): url = dep.to_github_url() assert url == "https://dev.azure.com/myorg/My%20Project/_git/myrepo" + # Spaces in repo name (percent-encoded) with _git segment + dep = DependencyReference.parse( + "dev.azure.com/Zifo/AIdeate%20and%20AIterate/_git/AiDeate%20SDLC%20Guidelines" + ) + assert dep.host == "dev.azure.com" + assert dep.ado_organization == "Zifo" + assert dep.ado_project == "AIdeate and AIterate" + assert dep.ado_repo == "AiDeate SDLC Guidelines" + assert dep.is_azure_devops() == True + assert dep.repo_url == "Zifo/AIdeate and AIterate/AiDeate SDLC Guidelines" + + # Spaces in both project and repo names (literal) + dep = DependencyReference.parse("dev.azure.com/myorg/My Project/My Repo Name") + assert dep.ado_organization == "myorg" + assert dep.ado_project == "My Project" + assert dep.ado_repo == "My Repo Name" + + # Spaces in repo name only (percent-encoded, no _git) + dep = DependencyReference.parse( + "dev.azure.com/myorg/myproject/My%20Repo%20Name" + ) + assert dep.ado_organization == "myorg" + assert dep.ado_project == "myproject" + assert dep.ado_repo == "My Repo Name" + + # to_github_url() with spaces in repo name encodes correctly + dep = DependencyReference.parse( + "dev.azure.com/Zifo/AIdeate%20and%20AIterate/_git/AiDeate%20SDLC%20Guidelines" + ) + url = dep.to_github_url() + assert "AiDeate%20SDLC%20Guidelines" in url + # Spaces should NOT be allowed in GitHub owner/repo names with pytest.raises(ValueError): DependencyReference.parse("github.com/my%20owner/repo") def test_parse_virtual_package_with_malicious_host(self): """Test virtual packages with various host types. - + With generic git host support, valid FQDNs are accepted as hosts. Path injection (embedding a host in a sub-path) is still rejected. """ # Path injection: still rejected (creates invalid repo format) with pytest.raises(ValueError): - DependencyReference.parse("evil.com/github.com/user/repo/prompts/file.prompt.md") - + DependencyReference.parse( + "evil.com/github.com/user/repo/prompts/file.prompt.md" + ) + # Valid generic hosts: now accepted with generic git URL support - dep1 = DependencyReference.parse("github.com.evil.com/user/repo/prompts/file.prompt.md") + dep1 = DependencyReference.parse( + "github.com.evil.com/user/repo/prompts/file.prompt.md" + ) assert dep1.host == "github.com.evil.com" assert dep1.repo_url == "user/repo" assert dep1.is_virtual is True - - dep2 = DependencyReference.parse("attacker.net/user/repo/prompts/file.prompt.md") + + dep2 = DependencyReference.parse( + "attacker.net/user/repo/prompts/file.prompt.md" + ) assert dep2.host == "attacker.net" assert dep2.repo_url == "user/repo" assert dep2.is_virtual is True - + def test_parse_virtual_file_package(self): """Test parsing virtual file package (individual file).""" dep = DependencyReference.parse("owner/test-repo/prompts/code-review.prompt.md") @@ -310,26 +364,28 @@ def test_parse_virtual_file_package(self): assert dep.is_virtual_file() is True assert dep.is_virtual_collection() is False assert dep.get_virtual_package_name() == "test-repo-code-review" - + def test_parse_virtual_file_with_reference(self): """Test parsing virtual file package with git reference.""" - dep = DependencyReference.parse("owner/test-repo/prompts/code-review.prompt.md#v1.0.0") + dep = DependencyReference.parse( + "owner/test-repo/prompts/code-review.prompt.md#v1.0.0" + ) assert dep.repo_url == "owner/test-repo" assert dep.is_virtual is True assert dep.virtual_path == "prompts/code-review.prompt.md" assert dep.reference == "v1.0.0" assert dep.is_virtual_file() is True - + def test_parse_virtual_file_all_extensions(self): """Test parsing virtual files with all supported extensions.""" - extensions = ['.prompt.md', '.instructions.md', '.chatmode.md', '.agent.md'] - + extensions = [".prompt.md", ".instructions.md", ".chatmode.md", ".agent.md"] + for ext in extensions: dep = DependencyReference.parse(f"user/repo/path/to/file{ext}") assert dep.is_virtual is True assert dep.is_virtual_file() is True assert dep.virtual_path == f"path/to/file{ext}" - + def test_parse_virtual_collection(self): """Test parsing virtual collection package.""" dep = DependencyReference.parse("owner/test-repo/collections/project-planning") @@ -339,7 +395,7 @@ def test_parse_virtual_collection(self): assert dep.is_virtual_file() is False assert dep.is_virtual_collection() is True assert dep.get_virtual_package_name() == "test-repo-project-planning" - + def test_parse_virtual_collection_with_reference(self): """Test parsing virtual collection with git reference.""" dep = DependencyReference.parse("owner/test-repo/collections/testing#main") @@ -348,7 +404,7 @@ def test_parse_virtual_collection_with_reference(self): assert dep.virtual_path == "collections/testing" assert dep.reference == "main" assert dep.is_virtual_collection() is True - + def test_parse_invalid_virtual_file_extension(self): """Test that invalid file extensions are rejected for virtual files.""" invalid_paths = [ @@ -357,27 +413,33 @@ def test_parse_invalid_virtual_file_extension(self): "user/repo/path/to/README.md", "user/repo/path/to/script.py", ] - + for path in invalid_paths: - with pytest.raises(ValueError, match="Individual files must end with one of"): + with pytest.raises( + ValueError, match="Individual files must end with one of" + ): DependencyReference.parse(path) - + def test_virtual_package_str_representation(self): """Test string representation of virtual packages. - + Note: After PR #33, host is explicit in string representation. """ - dep = DependencyReference.parse("owner/test-repo/prompts/code-review.prompt.md#v1.0.0") + dep = DependencyReference.parse( + "owner/test-repo/prompts/code-review.prompt.md#v1.0.0" + ) # Check that key components are present (host may be explicit now) assert "owner/test-repo" in str(dep) assert "prompts/code-review.prompt.md" in str(dep) assert "#v1.0.0" in str(dep) - - dep_with_ref = DependencyReference.parse("owner/test-repo/prompts/test.prompt.md#v2.0") + + dep_with_ref = DependencyReference.parse( + "owner/test-repo/prompts/test.prompt.md#v2.0" + ) assert "owner/test-repo" in str(dep_with_ref) assert "prompts/test.prompt.md" in str(dep_with_ref) assert "#v2.0" in str(dep_with_ref) - + def test_regular_package_not_virtual(self): """Test that regular packages (2 segments) are not marked as virtual.""" dep = DependencyReference.parse("user/repo") @@ -385,16 +447,19 @@ def test_regular_package_not_virtual(self): assert dep.virtual_path is None assert dep.is_virtual_file() is False assert dep.is_virtual_collection() is False - + def test_parse_control_characters_rejected(self): """Test that control characters are rejected.""" invalid_formats = [ "user//repo", "user repo", ] - + for invalid_format in invalid_formats: - with pytest.raises(ValueError, match="Invalid Git host|Empty dependency string|Invalid repository|Use 'user/repo'|path component"): + with pytest.raises( + ValueError, + match="Invalid Git host|Empty dependency string|Invalid repository|Use 'user/repo'|path component", + ): DependencyReference.parse(invalid_format) def test_parse_absolute_path_as_local(self): @@ -402,25 +467,27 @@ def test_parse_absolute_path_as_local(self): dep = DependencyReference.parse("/repo") assert dep.is_local is True assert dep.local_path == "/repo" - + def test_to_github_url(self): """Test converting to GitHub URL.""" dep = DependencyReference.parse("user/repo") expected = f"https://{github_host.default_host()}/user/repo" assert dep.to_github_url() == expected - + def test_get_display_name(self): """Test getting display name.""" dep1 = DependencyReference.parse("user/repo") assert dep1.get_display_name() == "user/repo" - + # Dict format alias still works for display name - dep2 = DependencyReference.parse_from_dict({"git": "https://github.com/user/repo.git", "alias": "myalias"}) + dep2 = DependencyReference.parse_from_dict( + {"git": "https://github.com/user/repo.git", "alias": "myalias"} + ) assert dep2.get_display_name() == "myalias" - + def test_string_representation(self): """Test string representation. - + Note: After PR #33, bare "user/repo" references will have host defaulted to github.com, so string representation includes it explicitly. """ @@ -428,26 +495,26 @@ def test_string_representation(self): # After PR #33 changes, host is explicit in string representation assert dep1.repo_url == "user/repo" assert "user/repo" in str(dep1) - + dep2 = DependencyReference.parse("user/repo#main") assert dep2.repo_url == "user/repo" assert dep2.reference == "main" assert "user/repo" in str(dep2) and "#main" in str(dep2) - + def test_string_representation_with_enterprise_host(self): """Test that string representation includes host for enterprise dependencies. - + This tests the fix from PR #33 where __str__ now includes the host prefix for dependencies from non-default GitHub hosts. """ # Enterprise host with just repo dep1 = DependencyReference.parse("company.ghe.com/user/repo") assert str(dep1) == "company.ghe.com/user/repo" - + # Enterprise host with reference dep2 = DependencyReference.parse("company.ghe.com/user/repo#v1.0.0") assert str(dep2) == "company.ghe.com/user/repo#v1.0.0" - + # Explicit github.com should also include host dep5 = DependencyReference.parse("github.com/user/repo") assert str(dep5) == "github.com/user/repo" @@ -455,136 +522,131 @@ def test_string_representation_with_enterprise_host(self): class TestAPMPackage: """Test APMPackage functionality.""" - + def test_from_apm_yml_minimal(self): """Test loading minimal valid apm.yml.""" - apm_content = { - 'name': 'test-package', - 'version': '1.0.0' - } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + apm_content = {"name": "test-package", "version": "1.0.0"} + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) - assert package.name == 'test-package' - assert package.version == '1.0.0' + assert package.name == "test-package" + assert package.version == "1.0.0" assert package.description is None assert package.author is None assert package.dependencies is None - + Path(f.name).unlink() # Clean up - + def test_from_apm_yml_complete(self): """Test loading complete apm.yml.""" apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'description': 'A test package', - 'author': 'Test Author', - 'license': 'MIT', - 'dependencies': { - 'apm': ['user/repo#main', 'another/repo#v2.0'], - 'mcp': ['some-mcp-server'] + "name": "test-package", + "version": "1.0.0", + "description": "A test package", + "author": "Test Author", + "license": "MIT", + "dependencies": { + "apm": ["user/repo#main", "another/repo#v2.0"], + "mcp": ["some-mcp-server"], }, - 'scripts': { - 'start': 'echo hello' - } + "scripts": {"start": "echo hello"}, } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) - assert package.name == 'test-package' - assert package.version == '1.0.0' - assert package.description == 'A test package' - assert package.author == 'Test Author' - assert package.license == 'MIT' + assert package.name == "test-package" + assert package.version == "1.0.0" + assert package.description == "A test package" + assert package.author == "Test Author" + assert package.license == "MIT" assert len(package.get_apm_dependencies()) == 2 assert len(package.get_mcp_dependencies()) == 1 - assert package.scripts['start'] == 'echo hello' - + assert package.scripts["start"] == "echo hello" + Path(f.name).unlink() # Clean up - + def test_from_apm_yml_missing_file(self): """Test loading from non-existent file.""" with pytest.raises(FileNotFoundError): APMPackage.from_apm_yml(Path("/non/existent/file.yml")) - + def test_from_apm_yml_missing_required_fields(self): """Test loading apm.yml with missing required fields.""" # Missing name - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: - yaml.dump({'version': '1.0.0'}, f) + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: + yaml.dump({"version": "1.0.0"}, f) f.flush() - + with pytest.raises(ValueError, match="Missing required field 'name'"): APMPackage.from_apm_yml(Path(f.name)) - + Path(f.name).unlink() - + # Missing version - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: - yaml.dump({'name': 'test'}, f) + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: + yaml.dump({"name": "test"}, f) f.flush() - + with pytest.raises(ValueError, match="Missing required field 'version'"): APMPackage.from_apm_yml(Path(f.name)) - + Path(f.name).unlink() - + def test_from_apm_yml_invalid_yaml(self): """Test loading invalid YAML.""" - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: f.write("name: test\nversion: 1.0.0\ninvalid: [unclosed") f.flush() - + with pytest.raises(ValueError, match="Invalid YAML format"): APMPackage.from_apm_yml(Path(f.name)) - + Path(f.name).unlink() - + def test_from_apm_yml_invalid_dependencies(self): """Test loading apm.yml with invalid dependency format.""" apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'dependencies': { - 'apm': ['invalid-repo-format'] - } + "name": "test-package", + "version": "1.0.0", + "dependencies": {"apm": ["invalid-repo-format"]}, } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + with pytest.raises(ValueError, match="Invalid APM dependency"): APMPackage.from_apm_yml(Path(f.name)) - + Path(f.name).unlink() - + def test_has_apm_dependencies(self): """Test checking for APM dependencies.""" # Package without dependencies pkg1 = APMPackage(name="test", version="1.0.0") assert not pkg1.has_apm_dependencies() - + # Package with MCP dependencies only - pkg2 = APMPackage(name="test", version="1.0.0", dependencies={'mcp': ['server']}) + pkg2 = APMPackage( + name="test", version="1.0.0", dependencies={"mcp": ["server"]} + ) assert not pkg2.has_apm_dependencies() - + # Package with APM dependencies apm_deps = [DependencyReference.parse("user/repo")] - pkg3 = APMPackage(name="test", version="1.0.0", dependencies={'apm': apm_deps}) + pkg3 = APMPackage(name="test", version="1.0.0", dependencies={"apm": apm_deps}) assert pkg3.has_apm_dependencies() class TestValidationResult: """Test ValidationResult functionality.""" - + def test_initial_state(self): """Test initial validation result state.""" result = ValidationResult() @@ -593,36 +655,36 @@ def test_initial_state(self): assert result.warnings == [] assert result.package is None assert not result.has_issues() - + def test_add_error(self): """Test adding validation errors.""" result = ValidationResult() result.add_error("Test error") - + assert result.is_valid is False assert "Test error" in result.errors assert result.has_issues() - + def test_add_warning(self): """Test adding validation warnings.""" result = ValidationResult() result.add_warning("Test warning") - + assert result.is_valid is True # Warnings don't make package invalid assert "Test warning" in result.warnings assert result.has_issues() - + def test_summary(self): """Test validation summary messages.""" # Valid with no issues result1 = ValidationResult() assert "[+] Package is valid" in result1.summary() - + # Valid with warnings result2 = ValidationResult() result2.add_warning("Test warning") assert "[!] Package is valid with 1 warning(s)" in result2.summary() - + # Invalid with errors result3 = ValidationResult() result3.add_error("Test error") @@ -631,20 +693,20 @@ def test_summary(self): class TestPackageValidation: """Test APM package validation functionality.""" - + def test_validate_non_existent_directory(self): """Test validating non-existent directory.""" result = validate_apm_package(Path("/non/existent/dir")) assert not result.is_valid assert any("does not exist" in error for error in result.errors) - + def test_validate_file_instead_of_directory(self): """Test validating a file instead of directory.""" with tempfile.NamedTemporaryFile() as f: result = validate_apm_package(Path(f.name)) assert not result.is_valid assert any("not a directory" in error for error in result.errors) - + def test_validate_missing_apm_yml(self): """Test that a directory without apm.yml/SKILL.md/plugin evidence is invalid.""" with tempfile.TemporaryDirectory() as tmpdir: @@ -652,97 +714,104 @@ def test_validate_missing_apm_yml(self): # Empty directories without plugin.json or component dirs are not valid assert not result.is_valid assert result.package_type == PackageType.INVALID - + def test_validate_invalid_apm_yml(self): """Test validating directory with invalid apm.yml.""" with tempfile.TemporaryDirectory() as tmpdir: apm_yml = Path(tmpdir) / "apm.yml" apm_yml.write_text("invalid: [yaml") - + result = validate_apm_package(Path(tmpdir)) assert not result.is_valid assert any("Invalid apm.yml" in error for error in result.errors) - + def test_validate_missing_apm_directory(self): """Test validating package without .apm directory.""" with tempfile.TemporaryDirectory() as tmpdir: apm_yml = Path(tmpdir) / "apm.yml" apm_yml.write_text("name: test\nversion: 1.0.0") - + result = validate_apm_package(Path(tmpdir)) assert not result.is_valid - assert any("Missing required directory: .apm/" in error for error in result.errors) - + assert any( + "Missing required directory: .apm/" in error for error in result.errors + ) + def test_validate_apm_file_instead_of_directory(self): """Test validating package with .apm as file instead of directory.""" with tempfile.TemporaryDirectory() as tmpdir: apm_yml = Path(tmpdir) / "apm.yml" apm_yml.write_text("name: test\nversion: 1.0.0") - + apm_file = Path(tmpdir) / ".apm" apm_file.write_text("this should be a directory") - + result = validate_apm_package(Path(tmpdir)) assert not result.is_valid assert any(".apm must be a directory" in error for error in result.errors) - + def test_validate_empty_apm_directory(self): """Test validating package with empty .apm directory.""" with tempfile.TemporaryDirectory() as tmpdir: apm_yml = Path(tmpdir) / "apm.yml" apm_yml.write_text("name: test\nversion: 1.0.0") - + apm_dir = Path(tmpdir) / ".apm" apm_dir.mkdir() - + result = validate_apm_package(Path(tmpdir)) assert result.is_valid # Should be valid but with warning - assert any("No primitive files found" in warning for warning in result.warnings) - + assert any( + "No primitive files found" in warning for warning in result.warnings + ) + def test_validate_valid_package(self): """Test validating completely valid package.""" with tempfile.TemporaryDirectory() as tmpdir: # Create apm.yml apm_yml = Path(tmpdir) / "apm.yml" apm_yml.write_text("name: test\nversion: 1.0.0\ndescription: Test package") - + # Create .apm directory with primitives apm_dir = Path(tmpdir) / ".apm" apm_dir.mkdir() - + instructions_dir = apm_dir / "instructions" instructions_dir.mkdir() (instructions_dir / "test.instructions.md").write_text("# Test instruction") - + chatmodes_dir = apm_dir / "chatmodes" chatmodes_dir.mkdir() (chatmodes_dir / "test.chatmode.md").write_text("# Test chatmode") - + result = validate_apm_package(Path(tmpdir)) assert result.is_valid assert result.package is not None assert result.package.name == "test" assert result.package.version == "1.0.0" - + def test_validate_version_format_warning(self): """Test validation warning for non-semver version.""" with tempfile.TemporaryDirectory() as tmpdir: apm_yml = Path(tmpdir) / "apm.yml" apm_yml.write_text("name: test\nversion: v1.0") # Not proper semver - + apm_dir = Path(tmpdir) / ".apm" apm_dir.mkdir() instructions_dir = apm_dir / "instructions" instructions_dir.mkdir() (instructions_dir / "test.instructions.md").write_text("# Test") - + result = validate_apm_package(Path(tmpdir)) assert result.is_valid - assert any("doesn't follow semantic versioning" in warning for warning in result.warnings) - + assert any( + "doesn't follow semantic versioning" in warning + for warning in result.warnings + ) + def test_validate_numeric_version_types(self): """Test that version validation handles YAML numeric types. - + This tests the fix from PR #33 for non-string version values. YAML may parse unquoted version numbers as numeric types (int/float). """ @@ -750,13 +819,13 @@ def test_validate_numeric_version_types(self): apm_yml = Path(tmpdir) / "apm.yml" # Write YAML with numeric version (no quotes) apm_yml.write_text("name: test\nversion: 1.0\ndescription: Test") - + apm_dir = Path(tmpdir) / ".apm" apm_dir.mkdir() instructions_dir = apm_dir / "instructions" instructions_dir.mkdir() (instructions_dir / "test.instructions.md").write_text("# Test") - + # Should not crash when validating result = validate_apm_package(Path(tmpdir)) assert result is not None @@ -770,7 +839,7 @@ def test_validate_numeric_version_types(self): class TestClaudeSkillValidation: """Test Claude Skill (SKILL.md-only) validation and APMPackage creation from SKILL metadata without generating an apm.yml.""" - + def test_validate_skill_with_simple_description(self): """Test validating a Claude Skill with simple description.""" with tempfile.TemporaryDirectory() as tmpdir: @@ -784,18 +853,18 @@ def test_validate_skill_with_simple_description(self): This is a test skill content. """) - + result = validate_apm_package(Path(tmpdir)) assert result.is_valid, f"Errors: {result.errors}" assert result.package is not None assert result.package.name == "test-skill" - + def test_validate_skill_with_colons_in_description(self): """Test validating a Claude Skill with colons in description (GitHub issue #66).""" with tempfile.TemporaryDirectory() as tmpdir: skill_md = Path(tmpdir) / "SKILL.md" # This is the actual pptx skill description that was causing issues - skill_md.write_text('''--- + skill_md.write_text("""--- name: pptx description: "Presentation creation, editing, and analysis. When Claude needs to work with presentations (.pptx files) for: (1) Creating new presentations, (2) Modifying or editing content, (3) Working with layouts, (4) Adding comments or speaker notes, or any other presentation tasks" --- @@ -803,59 +872,59 @@ def test_validate_skill_with_colons_in_description(self): # PPTX Skill Content here. -''') - +""") + result = validate_apm_package(Path(tmpdir)) assert result.is_valid, f"Errors: {result.errors}" assert result.package is not None assert result.package.name == "pptx" # Verify the description was preserved (colons and all) assert "for:" in result.package.description - + def test_validate_skill_with_quotes_in_description(self): """Test validating a Claude Skill with quotes in description.""" with tempfile.TemporaryDirectory() as tmpdir: skill_md = Path(tmpdir) / "SKILL.md" - skill_md.write_text('''--- + skill_md.write_text("""--- name: test-skill description: 'A skill that handles "quoted" strings' --- # Test Skill -''') - +""") + result = validate_apm_package(Path(tmpdir)) assert result.is_valid, f"Errors: {result.errors}" assert result.package is not None assert '"quoted"' in result.package.description - + def test_validate_skill_with_special_yaml_characters(self): """Test validating a Claude Skill with various YAML special characters.""" with tempfile.TemporaryDirectory() as tmpdir: skill_md = Path(tmpdir) / "SKILL.md" - skill_md.write_text('''--- + skill_md.write_text("""--- name: special-skill description: "Handles: colons, #hashtags, [brackets], {braces}, and 'quotes'" --- # Special Skill -''') - +""") + result = validate_apm_package(Path(tmpdir)) assert result.is_valid, f"Errors: {result.errors}" assert result.package is not None - + def test_validate_skill_without_description(self): """Test validating a Claude Skill without description field.""" with tempfile.TemporaryDirectory() as tmpdir: skill_md = Path(tmpdir) / "SKILL.md" - skill_md.write_text('''--- + skill_md.write_text("""--- name: minimal-skill --- # Minimal Skill -''') - +""") + result = validate_apm_package(Path(tmpdir)) assert result.is_valid, f"Errors: {result.errors}" assert result.package is not None @@ -872,16 +941,21 @@ def test_validate_hook_package_with_hooks_dir(self): hooks_dir = Path(tmpdir) / "hooks" hooks_dir.mkdir() hooks_json = hooks_dir / "hooks.json" - hooks_json.write_text(json.dumps({ - "hooks": { - "PreToolUse": [{ - "hooks": [{ - "type": "command", - "command": "echo hello" - }] - }] - } - })) + hooks_json.write_text( + json.dumps( + { + "hooks": { + "PreToolUse": [ + { + "hooks": [ + {"type": "command", "command": "echo hello"} + ] + } + ] + } + } + ) + ) result = validate_apm_package(Path(tmpdir)) assert result.is_valid, f"Errors: {result.errors}" @@ -895,9 +969,17 @@ def test_validate_hook_package_with_apm_hooks_dir(self): hooks_dir = Path(tmpdir) / ".apm" / "hooks" hooks_dir.mkdir(parents=True) hooks_json = hooks_dir / "my-hooks.json" - hooks_json.write_text(json.dumps({ - "hooks": {"Stop": [{"hooks": [{"type": "command", "command": "echo bye"}]}]} - })) + hooks_json.write_text( + json.dumps( + { + "hooks": { + "Stop": [ + {"hooks": [{"type": "command", "command": "echo bye"}]} + ] + } + } + ) + ) result = validate_apm_package(Path(tmpdir)) assert result.is_valid, f"Errors: {result.errors}" @@ -1010,45 +1092,45 @@ def test_hook_package_apm_yml_precedence(self, tmp_path): class TestGitReferenceUtils: """Test Git reference parsing utilities.""" - + def test_parse_git_reference_branch(self): """Test parsing branch references.""" ref_type, ref = parse_git_reference("main") assert ref_type == GitReferenceType.BRANCH assert ref == "main" - + ref_type, ref = parse_git_reference("feature/new-stuff") assert ref_type == GitReferenceType.BRANCH assert ref == "feature/new-stuff" - + def test_parse_git_reference_tag(self): """Test parsing tag references.""" ref_type, ref = parse_git_reference("v1.0.0") assert ref_type == GitReferenceType.TAG assert ref == "v1.0.0" - + ref_type, ref = parse_git_reference("1.2.3") assert ref_type == GitReferenceType.TAG assert ref == "1.2.3" - + def test_parse_git_reference_commit(self): """Test parsing commit SHA references.""" # Full SHA ref_type, ref = parse_git_reference("abcdef1234567890abcdef1234567890abcdef12") assert ref_type == GitReferenceType.COMMIT assert ref == "abcdef1234567890abcdef1234567890abcdef12" - + # Short SHA ref_type, ref = parse_git_reference("abcdef1") assert ref_type == GitReferenceType.COMMIT assert ref == "abcdef1" - + def test_parse_git_reference_empty(self): """Test parsing empty reference defaults to main branch.""" ref_type, ref = parse_git_reference("") assert ref_type == GitReferenceType.BRANCH assert ref == "main" - + ref_type, ref = parse_git_reference(None) assert ref_type == GitReferenceType.BRANCH assert ref == "main" @@ -1056,7 +1138,7 @@ def test_parse_git_reference_empty(self): class TestResolvedReference: """Test ResolvedReference functionality.""" - + def test_string_representation(self): """Test string representation of resolved references.""" # Commit reference @@ -1064,60 +1146,60 @@ def test_string_representation(self): original_ref="abc123", ref_type=GitReferenceType.COMMIT, resolved_commit="abc123def456", - ref_name="abc123" + ref_name="abc123", ) assert str(commit_ref) == "abc123de" # First 8 chars - + # Branch reference branch_ref = ResolvedReference( original_ref="main", ref_type=GitReferenceType.BRANCH, resolved_commit="abc123def456", - ref_name="main" + ref_name="main", ) assert str(branch_ref) == "main (abc123de)" - + # Tag reference tag_ref = ResolvedReference( original_ref="v1.0.0", ref_type=GitReferenceType.TAG, resolved_commit="abc123def456", - ref_name="v1.0.0" + ref_name="v1.0.0", ) assert str(tag_ref) == "v1.0.0 (abc123de)" class TestPackageInfo: """Test PackageInfo functionality.""" - + def test_get_primitives_path(self): """Test getting primitives path.""" package = APMPackage(name="test", version="1.0.0") install_path = Path("/tmp/package") - + info = PackageInfo(package=package, install_path=install_path) assert info.get_primitives_path() == install_path / ".apm" - + def test_has_primitives(self): """Test checking if package has primitives.""" with tempfile.TemporaryDirectory() as tmpdir: package = APMPackage(name="test", version="1.0.0") install_path = Path(tmpdir) - + info = PackageInfo(package=package, install_path=install_path) - + # No .apm directory assert not info.has_primitives() - + # Empty .apm directory apm_dir = install_path / ".apm" apm_dir.mkdir() assert not info.has_primitives() - + # .apm with empty subdirectories (apm_dir / "instructions").mkdir() assert not info.has_primitives() - + # .apm with primitive files (apm_dir / "instructions" / "test.md").write_text("# Test") assert info.has_primitives() @@ -1125,206 +1207,200 @@ def test_has_primitives(self): class TestPackageContentType: """Test PackageContentType enum and parsing.""" - + def test_enum_values(self): """Test that all expected enum values exist.""" assert PackageContentType.INSTRUCTIONS.value == "instructions" assert PackageContentType.SKILL.value == "skill" assert PackageContentType.HYBRID.value == "hybrid" assert PackageContentType.PROMPTS.value == "prompts" - + def test_from_string_valid_values(self): """Test parsing all valid type values.""" - assert PackageContentType.from_string("instructions") == PackageContentType.INSTRUCTIONS + assert ( + PackageContentType.from_string("instructions") + == PackageContentType.INSTRUCTIONS + ) assert PackageContentType.from_string("skill") == PackageContentType.SKILL assert PackageContentType.from_string("hybrid") == PackageContentType.HYBRID assert PackageContentType.from_string("prompts") == PackageContentType.PROMPTS - + def test_from_string_case_insensitive(self): """Test that parsing is case-insensitive.""" - assert PackageContentType.from_string("INSTRUCTIONS") == PackageContentType.INSTRUCTIONS + assert ( + PackageContentType.from_string("INSTRUCTIONS") + == PackageContentType.INSTRUCTIONS + ) assert PackageContentType.from_string("Skill") == PackageContentType.SKILL assert PackageContentType.from_string("HYBRID") == PackageContentType.HYBRID assert PackageContentType.from_string("Prompts") == PackageContentType.PROMPTS - + def test_from_string_with_whitespace(self): """Test that parsing handles leading/trailing whitespace.""" - assert PackageContentType.from_string(" instructions ") == PackageContentType.INSTRUCTIONS + assert ( + PackageContentType.from_string(" instructions ") + == PackageContentType.INSTRUCTIONS + ) assert PackageContentType.from_string("\tskill\n") == PackageContentType.SKILL - + def test_from_string_invalid_value(self): """Test that invalid values raise ValueError with helpful message.""" with pytest.raises(ValueError) as exc_info: PackageContentType.from_string("invalid") - + error_msg = str(exc_info.value) assert "Invalid package type 'invalid'" in error_msg assert "'instructions'" in error_msg assert "'skill'" in error_msg assert "'hybrid'" in error_msg assert "'prompts'" in error_msg - + def test_from_string_empty_value(self): """Test that empty string raises ValueError.""" with pytest.raises(ValueError, match="Package type cannot be empty"): PackageContentType.from_string("") - + def test_from_string_typo_suggestions(self): """Test helpful error message for common typos.""" # Test that error message lists all valid types with pytest.raises(ValueError) as exc_info: PackageContentType.from_string("instruction") # Missing 's' - + error_msg = str(exc_info.value) assert "'instructions'" in error_msg # Shows correct spelling class TestAPMPackageTypeField: """Test APMPackage type field parsing from apm.yml.""" - + def test_type_field_instructions(self): """Test parsing type: instructions from apm.yml.""" apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'type': 'instructions' + "name": "test-package", + "version": "1.0.0", + "type": "instructions", } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) assert package.type == PackageContentType.INSTRUCTIONS - + Path(f.name).unlink() - + def test_type_field_skill(self): """Test parsing type: skill from apm.yml.""" - apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'type': 'skill' - } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + apm_content = {"name": "test-package", "version": "1.0.0", "type": "skill"} + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) assert package.type == PackageContentType.SKILL - + Path(f.name).unlink() - + def test_type_field_hybrid(self): """Test parsing type: hybrid from apm.yml.""" - apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'type': 'hybrid' - } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + apm_content = {"name": "test-package", "version": "1.0.0", "type": "hybrid"} + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) assert package.type == PackageContentType.HYBRID - + Path(f.name).unlink() - + def test_type_field_prompts(self): """Test parsing type: prompts from apm.yml.""" - apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'type': 'prompts' - } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + apm_content = {"name": "test-package", "version": "1.0.0", "type": "prompts"} + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) assert package.type == PackageContentType.PROMPTS - + Path(f.name).unlink() - + def test_type_field_missing_defaults_to_none(self): """Test that missing type field defaults to None (hybrid behavior).""" - apm_content = { - 'name': 'test-package', - 'version': '1.0.0' - } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + apm_content = {"name": "test-package", "version": "1.0.0"} + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) assert package.type is None # Default to None for backward compatibility - + Path(f.name).unlink() - + def test_type_field_invalid_raises_error(self): """Test that invalid type value raises ValueError.""" apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'type': 'invalid-type' + "name": "test-package", + "version": "1.0.0", + "type": "invalid-type", } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + with pytest.raises(ValueError) as exc_info: APMPackage.from_apm_yml(Path(f.name)) - + error_msg = str(exc_info.value) assert "Invalid 'type' field" in error_msg assert "invalid-type" in error_msg - + Path(f.name).unlink() - + def test_type_field_non_string_raises_error(self): """Test that non-string type value raises ValueError.""" apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'type': 123 # Numeric type + "name": "test-package", + "version": "1.0.0", + "type": 123, # Numeric type } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + with pytest.raises(ValueError) as exc_info: APMPackage.from_apm_yml(Path(f.name)) - + error_msg = str(exc_info.value) assert "expected string" in error_msg assert "int" in error_msg - + Path(f.name).unlink() - + def test_type_field_case_insensitive_in_yaml(self): """Test that type field parsing is case-insensitive in YAML.""" apm_content = { - 'name': 'test-package', - 'version': '1.0.0', - 'type': 'SKILL' # Uppercase + "name": "test-package", + "version": "1.0.0", + "type": "SKILL", # Uppercase } - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: yaml.dump(apm_content, f) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) assert package.type == PackageContentType.SKILL - + Path(f.name).unlink() - + def test_type_field_null_treated_as_missing(self): """Test that explicit null type field is treated as missing.""" # Write YAML directly to handle null explicitly @@ -1332,25 +1408,23 @@ def test_type_field_null_treated_as_missing(self): version: "1.0.0" type: null """ - - with tempfile.NamedTemporaryFile(mode='w', suffix='.yml', delete=False) as f: + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", delete=False) as f: f.write(yaml_content) f.flush() - + package = APMPackage.from_apm_yml(Path(f.name)) assert package.type is None - + Path(f.name).unlink() - + def test_package_dataclass_with_type(self): """Test that APMPackage dataclass accepts type parameter.""" package = APMPackage( - name="test", - version="1.0.0", - type=PackageContentType.SKILL + name="test", version="1.0.0", type=PackageContentType.SKILL ) assert package.type == PackageContentType.SKILL - + def test_package_dataclass_type_defaults_to_none(self): """Test that APMPackage type defaults to None when not provided.""" package = APMPackage(name="test", version="1.0.0") @@ -1364,12 +1438,35 @@ class TestGenericHostSubdirectoryRoundTrip: not just GitHub and ADO. """ - @pytest.mark.parametrize("git_url,path,ref,desc", [ - ("https://git.example.com/ai/grandpa-s-skills", "dist/brain-council", "master", "reporter case"), - ("https://gitlab.com/my-org/my-group/my-skills", "dist/skill-a", "main", "GitLab nested groups"), - ("https://gitea.example.com/org/repo", "prompts/helper", "v1.0", "Gitea simple"), - ("https://bitbucket.example.com/team/prompts", "agents/summarizer", "develop", "Bitbucket self-hosted"), - ]) + @pytest.mark.parametrize( + "git_url,path,ref,desc", + [ + ( + "https://git.example.com/ai/grandpa-s-skills", + "dist/brain-council", + "master", + "reporter case", + ), + ( + "https://gitlab.com/my-org/my-group/my-skills", + "dist/skill-a", + "main", + "GitLab nested groups", + ), + ( + "https://gitea.example.com/org/repo", + "prompts/helper", + "v1.0", + "Gitea simple", + ), + ( + "https://bitbucket.example.com/team/prompts", + "agents/summarizer", + "develop", + "Bitbucket self-hosted", + ), + ], + ) def test_parse_from_dict_preserves_virtual_path(self, git_url, path, ref, desc): """parse_from_dict correctly separates repo URL from subdirectory path.""" entry = {"git": git_url, "path": path, "ref": ref} @@ -1377,11 +1474,22 @@ def test_parse_from_dict_preserves_virtual_path(self, git_url, path, ref, desc): assert dep.virtual_path == path, f"Failed for {desc}" assert dep.is_virtual is True, f"Failed for {desc}" - @pytest.mark.parametrize("git_url,path,ref", [ - ("https://git.example.com/ai/grandpa-s-skills", "dist/brain-council", "master"), - ("https://gitlab.com/org/repo", "prompts/helper", "v1.0"), - ("https://bitbucket.example.com/team/prompts", "agents/summarizer", "develop"), - ]) + @pytest.mark.parametrize( + "git_url,path,ref", + [ + ( + "https://git.example.com/ai/grandpa-s-skills", + "dist/brain-council", + "master", + ), + ("https://gitlab.com/org/repo", "prompts/helper", "v1.0"), + ( + "https://bitbucket.example.com/team/prompts", + "agents/summarizer", + "develop", + ), + ], + ) def test_download_package_skips_parse_with_structured_dep(self, git_url, path, ref): """download_package must skip DependencyReference.parse() when given a structured object, avoiding the lossy round-trip.""" @@ -1389,11 +1497,13 @@ def test_download_package_skips_parse_with_structured_dep(self, git_url, path, r dep = DependencyReference.parse_from_dict(entry) from apm_cli.deps.github_downloader import GitHubPackageDownloader + downloader = GitHubPackageDownloader() # Monkey-patch DependencyReference.parse to detect if it's called original_parse = DependencyReference.parse parse_called = False + @classmethod def tracking_parse(cls, s): nonlocal parse_called @@ -1417,7 +1527,11 @@ def tracking_parse(cls, s): def test_github_round_trip_works(self): """GitHub round-trip works because min_base_segments=2 is hardcoded.""" - entry = {"git": "https://github.com/anthropics/skills", "path": "skills/skill-creator", "ref": "main"} + entry = { + "git": "https://github.com/anthropics/skills", + "path": "skills/skill-creator", + "ref": "main", + } dep = DependencyReference.parse_from_dict(entry) dep2 = DependencyReference.parse(str(dep)) assert dep2.virtual_path == dep.virtual_path @@ -1426,9 +1540,10 @@ def test_github_round_trip_works(self): def test_build_download_ref_preserves_virtual_path(self): """build_download_ref returns a DependencyReference that preserves virtual_path for generic hosts (not a lossy flat string).""" - from apm_cli.drift import build_download_ref from unittest.mock import Mock + from apm_cli.drift import build_download_ref + dep = DependencyReference( repo_url="org/my-skills", host="git.example.com", @@ -1446,4 +1561,4 @@ def test_build_download_ref_preserves_virtual_path(self): assert result.repo_url == "org/my-skills" assert result.host == "git.example.com" assert result.reference == "abc123" - assert result.is_virtual is True \ No newline at end of file + assert result.is_virtual is True From 0e353ed84948b21489b6827997b164d7a9d59e15 Mon Sep 17 00:00:00 2001 From: Prem Kumar Duraisamy Date: Tue, 24 Mar 2026 14:36:30 +0530 Subject: [PATCH 2/2] fix: address PR review comments - Replace non-ASCII arrows and em-dashes with ASCII equivalents in reference.py and test_apm_package_models.py (encoding rule compliance) - Add path traversal guards for '.' and '..' segments in ADO and non-ADO validation blocks in DependencyReference.parse() - Update authentication.md and dependencies.md docs with note on using percent-encoded spaces in ADO project/repo names --- .../docs/getting-started/authentication.md | 8 ++++- docs/src/content/docs/guides/dependencies.md | 2 +- src/apm_cli/models/dependency/reference.py | 35 ++++++++++++------- tests/test_apm_package_models.py | 12 +++---- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/docs/src/content/docs/getting-started/authentication.md b/docs/src/content/docs/getting-started/authentication.md index ec84f511..ade1bb61 100644 --- a/docs/src/content/docs/getting-started/authentication.md +++ b/docs/src/content/docs/getting-started/authentication.md @@ -131,13 +131,19 @@ export ADO_APM_PAT=your_ado_pat apm install dev.azure.com/myorg/myproject/myrepo ``` -ADO is always auth-required. Uses 3-segment paths (`org/project/repo`). No `ADO_HOST` equivalent — always use FQDN syntax: +ADO is always auth-required. Uses 3-segment paths (`org/project/repo`). No `ADO_HOST` equivalent - always use FQDN syntax: ```bash apm install dev.azure.com/myorg/myproject/myrepo#main apm install mycompany.visualstudio.com/org/project/repo # legacy URL ``` +If your ADO project or repository name contains spaces, URL-encode them as `%20`: + +```bash +apm install dev.azure.com/myorg/My%20Project/_git/My%20Repo%20Name +``` + Create the PAT at `https://dev.azure.com/{org}/_usersSettings/tokens` with **Code (Read)** permission. ## Package source behavior diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md index d9777762..7134dc01 100644 --- a/docs/src/content/docs/guides/dependencies.md +++ b/docs/src/content/docs/guides/dependencies.md @@ -30,7 +30,7 @@ APM supports multiple dependency types: | **Virtual Subdirectory Package** | Folder path in monorepo | `ComposioHQ/awesome-claude-skills/mcp-builder` | | **Virtual Subdirectory Package** | Folder path in repo | `github/awesome-copilot/skills/review-and-refactor` | | **Local Path Package** | Path starts with `./`, `../`, or `/` | `./packages/my-shared-skills` | -| **ADO Package** | Azure DevOps repo | `dev.azure.com/org/project/_git/repo` | +| **ADO Package** | Azure DevOps repo | `dev.azure.com/org/project/_git/repo` or `dev.azure.com/org/My%20Project/_git/My%20Repo` | **Virtual Subdirectory Packages** are skill folders from monorepos - they download an entire folder and may contain a SKILL.md plus resources. diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index a83572d9..bbcd6d38 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -100,9 +100,9 @@ def is_virtual_subdirectory(self) -> bool: - Is a directory path (likely containing SKILL.md or apm.yml) Examples: - - ComposioHQ/awesome-claude-skills/brand-guidelines → True - - owner/repo/prompts/file.prompt.md → False (is_virtual_file) - - owner/repo/collections/name → False (is_virtual_collection) + - ComposioHQ/awesome-claude-skills/brand-guidelines -> True + - owner/repo/prompts/file.prompt.md -> False (is_virtual_file) + - owner/repo/collections/name -> False (is_virtual_collection) """ return self.virtual_type == VirtualPackageType.SUBDIRECTORY @@ -110,9 +110,9 @@ def get_virtual_package_name(self) -> str: """Generate a package name for this virtual package. For virtual packages, we create a sanitized name from the path: - - owner/repo/prompts/code-review.prompt.md → repo-code-review - - owner/repo/collections/project-planning → repo-project-planning - - owner/repo/collections/project-planning.collection.yml → repo-project-planning + - owner/repo/prompts/code-review.prompt.md -> repo-code-review + - owner/repo/collections/project-planning -> repo-project-planning + - owner/repo/collections/project-planning.collection.yml -> repo-project-planning """ if not self.is_virtual or not self.virtual_path: return self.repo_url.split("/")[-1] # Return repo name as fallback @@ -125,8 +125,8 @@ def get_virtual_package_name(self) -> str: path_parts = self.virtual_path.split("/") if self.is_virtual_collection(): # For collections: use the collection name without extension - # collections/project-planning → project-planning - # collections/project-planning.collection.yml → project-planning + # collections/project-planning -> project-planning + # collections/project-planning.collection.yml -> project-planning collection_name = path_parts[-1] # Strip .collection.yml/.collection.yaml extension if present for ext in (".collection.yml", ".collection.yaml"): @@ -136,7 +136,7 @@ def get_virtual_package_name(self) -> str: return f"{repo_name}-{collection_name}" else: # For individual files: use the filename without extension - # prompts/code-review.prompt.md → code-review + # prompts/code-review.prompt.md -> code-review filename = path_parts[-1] for ext in self.VIRTUAL_FILE_EXTENSIONS: if filename.endswith(ext): @@ -256,7 +256,7 @@ def canonicalize(raw: str) -> str: def get_canonical_dependency_string(self) -> str: """Get the host-blind canonical string for filesystem and orphan-detection matching. - This returns repo_url (+ virtual_path) without host prefix — it matches + This returns repo_url (+ virtual_path) without host prefix -- it matches the filesystem layout in apm_modules/ which is also host-blind. For identity-based matching that includes non-default hosts, use get_identity(). @@ -375,8 +375,8 @@ def _normalize_ssh_protocol_url(url: str) -> str: """Normalize ssh:// protocol URLs to git@ format for consistent parsing. Converts: - - ssh://git@gitlab.com/owner/repo.git → git@gitlab.com:owner/repo.git - - ssh://git@host:port/owner/repo.git → git@host:owner/repo.git + - ssh://git@gitlab.com/owner/repo.git -> git@gitlab.com:owner/repo.git + - ssh://git@host:port/owner/repo.git -> git@host:owner/repo.git Non-SSH URLs are returned unchanged. """ @@ -925,6 +925,12 @@ def parse(cls, dependency_str: str) -> "DependencyReference": f"Invalid Azure DevOps repository format: {repo_url}. Expected 'org/project/repo'" ) ado_parts = repo_url.split("/") + for part in ado_parts: + if part in (".", ".."): + raise PathTraversalError( + f"Path traversal segment '{part}' is not allowed in " + f"Azure DevOps repository path: {repo_url}" + ) ado_organization = ado_parts[0] ado_project = ado_parts[1] ado_repo = ado_parts[2] @@ -939,6 +945,11 @@ def parse(cls, dependency_str: str) -> "DependencyReference": f"Invalid repository format: {repo_url}. Contains invalid characters" ) for seg in segments: + if seg in (".", ".."): + raise ValueError( + f"Invalid repository format: {repo_url}. " + f"Contains '.' or '..' path segments" + ) if any(seg.endswith(ext) for ext in cls.VIRTUAL_FILE_EXTENSIONS): raise ValueError( f"Invalid repository format: '{repo_url}' contains a virtual file extension. " diff --git a/tests/test_apm_package_models.py b/tests/test_apm_package_models.py index 493dd3b2..7813ead5 100644 --- a/tests/test_apm_package_models.py +++ b/tests/test_apm_package_models.py @@ -56,12 +56,12 @@ def test_parse_with_commit(self): assert dep.alias is None def test_parse_with_alias_shorthand_removed(self): - """Shorthand @alias syntax is no longer supported — @ in shorthand is rejected.""" + """Shorthand @alias syntax is no longer supported -- @ in shorthand is rejected.""" with pytest.raises(ValueError): DependencyReference.parse("user/repo@myalias") def test_parse_with_reference_and_alias_shorthand_not_parsed(self): - """Shorthand #ref@alias — @ is no longer parsed as alias separator.""" + """Shorthand #ref@alias -- @ is no longer parsed as alias separator.""" dep = DependencyReference.parse("user/repo#main@myalias") assert dep.repo_url == "user/repo" assert dep.reference == "main@myalias" # @ treated as part of ref @@ -117,7 +117,7 @@ def test_parse_malicious_url_bypass_attempts(self): With nested group support on generic hosts, path segments that happen to look like hostnames (e.g., 'github.com/user/repo') are treated as - repo path segments — not injection. The host is correctly identified. + repo path segments -- not injection. The host is correctly identified. """ # Attack vectors that should still be REJECTED rejected_formats = [ @@ -149,7 +149,7 @@ def test_parse_malicious_url_bypass_attempts(self): assert dep.is_virtual is False # With generic git host support, valid FQDNs are accepted as hosts. - # These are not injection attacks — they are legitimate host references. + # These are not injection attacks -- they are legitimate host references. accepted_as_generic_hosts = [ "evil-github.com/user/repo", "malicious-github.com/user/repo", @@ -1434,7 +1434,7 @@ def test_package_dataclass_type_defaults_to_none(self): class TestGenericHostSubdirectoryRoundTrip: """Regression tests for issue #382: subdirectory packages on generic git hosts. - The str() → parse() round-trip must preserve virtual_path for all hosts, + The str() -> parse() round-trip must preserve virtual_path for all hosts, not just GitHub and ADO. """ @@ -1522,7 +1522,7 @@ def tracking_parse(cls, s): assert not parse_called, ( "DependencyReference.parse() was called when passing a structured " - "DependencyReference — the lossy round-trip was NOT avoided" + "DependencyReference -- the lossy round-trip was NOT avoided" ) def test_github_round_trip_works(self):