fix: add security checks and tests for code review
Security: - Add authorization checks to list_packages, update_package, delete_package endpoints - Add MAX_TOTAL_ARTIFACTS limit (1000) to prevent memory exhaustion during dependency resolution - Add TooManyArtifactsError exception for proper error handling UI: - Display reverse dependency errors in PackagePage - Add warning display for failed dependency fetches in DependencyGraph Tests: - Add unit tests for metadata extraction (deb, wheel, tarball, jar) - Add unit tests for rate limit configuration - Add unit tests for PyPI registry client
This commit is contained in:
@@ -109,9 +109,17 @@ class DependencyDepthExceededError(DependencyError):
|
||||
super().__init__(f"Dependency resolution exceeded maximum depth of {max_depth}")
|
||||
|
||||
|
||||
class TooManyArtifactsError(DependencyError):
|
||||
"""Raised when dependency resolution resolves too many artifacts."""
|
||||
def __init__(self, max_artifacts: int):
|
||||
self.max_artifacts = max_artifacts
|
||||
super().__init__(f"Dependency resolution exceeded maximum of {max_artifacts} artifacts")
|
||||
|
||||
|
||||
# Safety limits to prevent DoS attacks
|
||||
MAX_DEPENDENCY_DEPTH = 100 # Maximum levels of nested dependencies
|
||||
MAX_DEPENDENCIES_PER_ARTIFACT = 200 # Maximum direct dependencies per artifact
|
||||
MAX_TOTAL_ARTIFACTS = 1000 # Maximum total artifacts in resolution to prevent memory issues
|
||||
|
||||
|
||||
def parse_ensure_file(content: bytes) -> EnsureFileContent:
|
||||
@@ -849,6 +857,10 @@ def resolve_dependencies(
|
||||
visited.add(artifact_id)
|
||||
resolution_path_sync.pop()
|
||||
|
||||
# Check total artifacts limit
|
||||
if len(resolution_order) >= MAX_TOTAL_ARTIFACTS:
|
||||
raise TooManyArtifactsError(MAX_TOTAL_ARTIFACTS)
|
||||
|
||||
# Add to resolution order (dependencies before dependents)
|
||||
resolution_order.append(artifact_id)
|
||||
|
||||
@@ -1257,6 +1269,10 @@ async def resolve_dependencies_with_fetch(
|
||||
visited.add(artifact_id)
|
||||
resolution_path.pop()
|
||||
|
||||
# Check total artifacts limit
|
||||
if len(resolution_order) >= MAX_TOTAL_ARTIFACTS:
|
||||
raise TooManyArtifactsError(MAX_TOTAL_ARTIFACTS)
|
||||
|
||||
resolution_order.append(artifact_id)
|
||||
|
||||
resolved_artifacts[artifact_id] = ResolvedArtifact(
|
||||
|
||||
@@ -147,6 +147,7 @@ from .dependencies import (
|
||||
DependencyConflictError,
|
||||
DependencyNotFoundError,
|
||||
DependencyDepthExceededError,
|
||||
TooManyArtifactsError,
|
||||
)
|
||||
from .config import get_settings, get_env_upstream_sources
|
||||
from .checksum import (
|
||||
@@ -2666,10 +2667,10 @@ def list_packages(
|
||||
format: Optional[str] = Query(default=None, description="Filter by package format"),
|
||||
platform: Optional[str] = Query(default=None, description="Filter by platform"),
|
||||
db: Session = Depends(get_db),
|
||||
current_user: Optional[User] = Depends(get_current_user_optional),
|
||||
):
|
||||
project = db.query(Project).filter(Project.name == project_name).first()
|
||||
if not project:
|
||||
raise HTTPException(status_code=404, detail="Project not found")
|
||||
# Check read access (handles private project visibility)
|
||||
project = check_project_access(db, project_name, current_user, "read")
|
||||
|
||||
# Validate sort field
|
||||
valid_sort_fields = {
|
||||
@@ -2950,13 +2951,13 @@ def update_package(
|
||||
package_update: PackageUpdate,
|
||||
request: Request,
|
||||
db: Session = Depends(get_db),
|
||||
current_user: Optional[User] = Depends(get_current_user_optional),
|
||||
):
|
||||
"""Update a package's metadata."""
|
||||
user_id = get_user_id(request)
|
||||
|
||||
project = db.query(Project).filter(Project.name == project_name).first()
|
||||
if not project:
|
||||
raise HTTPException(status_code=404, detail="Project not found")
|
||||
# Check write access to project
|
||||
project = check_project_access(db, project_name, current_user, "write")
|
||||
|
||||
package = (
|
||||
db.query(Package)
|
||||
@@ -3033,6 +3034,7 @@ def delete_package(
|
||||
package_name: str,
|
||||
request: Request,
|
||||
db: Session = Depends(get_db),
|
||||
current_user: Optional[User] = Depends(get_current_user_optional),
|
||||
):
|
||||
"""
|
||||
Delete a package and all its versions.
|
||||
@@ -3043,9 +3045,8 @@ def delete_package(
|
||||
"""
|
||||
user_id = get_user_id(request)
|
||||
|
||||
project = db.query(Project).filter(Project.name == project_name).first()
|
||||
if not project:
|
||||
raise HTTPException(status_code=404, detail="Project not found")
|
||||
# Check write access to project (deletion requires write permission)
|
||||
project = check_project_access(db, project_name, current_user, "write")
|
||||
|
||||
package = (
|
||||
db.query(Package)
|
||||
@@ -7137,6 +7138,15 @@ async def resolve_artifact_dependencies(
|
||||
"max_depth": e.max_depth,
|
||||
}
|
||||
)
|
||||
except TooManyArtifactsError as e:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"error": "too_many_artifacts",
|
||||
"message": str(e),
|
||||
"max_artifacts": e.max_artifacts,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
# --- Upstream Caching Routes ---
|
||||
|
||||
Reference in New Issue
Block a user