WEPPcloud

← Back to usersum index

QA Review - Dedicated Download Service for Critical Run Artifacts

Metadata

  • Package: docs/work-packages/20260619_dedicated_download_service/
  • Reviewer: Codex QA review
  • Date: 2026-06-19
  • Scope reviewed:
    • wepppy/microservices/download/app.py
    • wepppy/microservices/download/README.md
    • tests/microservices/test_dedicated_download_service.py
    • tests/docker/unit/test_download_service_routing.py
    • docker/caddy/Caddyfile
    • docker/caddy/Caddyfile.wepp1
    • docker/docker-compose.dev.yml
    • docker/docker-compose.prod.yml
    • docker/docker-compose.prod.wepp1.yml
  • Related artifacts:
    • Security review: docs/work-packages/20260619_dedicated_download_service/artifacts/20260619_security_review.md
    • ExecPlan: docs/work-packages/20260619_dedicated_download_service/prompts/active/dedicated_download_service_execplan.md

Verdict

  • Gate status: pass for local implementation
  • Release recommendation: ship with conditions. Production completion still requires wepp1 service/Caddy rollout and live production archive smoke/log evidence.
  • Unresolved findings:
    • High: 0
    • Medium: 0
    • Low: 0

Findings and Disposition

ID Severity Area Finding Evidence Disposition Status
QA-01 Medium Streaming implementation Archive response streaming used synchronous file reads inside the ASGI response loop, which could block other requests in the same worker during slow NFS reads. Initial wepppy/microservices/download/app.py iterated a synchronous file generator from an async response body. Resolved by moving file open, seek, read, and close operations through asyncio.to_thread(...) and changing _stream_file_range to an async iterator. Resolved
QA-02 Low Caddy route parity Caddy route matching only captured lowercase .zip, while the service accepted uppercase ZIP extensions. Uppercase archives would fall through to the legacy browse download path and lose dedicated-service observability. Initial archive_download_proxy regex ended in \.zip$; service used archive_subpath.casefold().endswith(".zip"). Resolved by changing both Caddyfiles to [Zz][Ii][Pp] and adding routing coverage for /archives/run.ZIP. Resolved

Coverage Review

  • Full archive GET returns expected bytes and attachment headers.
  • HEAD returns archive metadata without body.
  • Single closed, open-ended, and suffix byte ranges return 206 with Content-Range.
  • Unsatisfiable range returns 416 with Content-Range: bytes */<size>.
  • Public-run anonymous archive access remains allowed.
  • Private-run navigation requests redirect for cookie minting while API-style requests return 401.
  • Bearer token access remains accepted for private archive access.
  • Traversal, hidden, repeated-separator, and non-ZIP paths are rejected or not found.
  • Structured logs include route family, request id, run id, config, path category, basename, size, method, status, range start/end, bytes, duration, outcome, client IP, and user agent while excluding Authorization and absolute run-root paths.
  • Caddy route tests prove exact archive ZIP traffic matches before the broader browse matcher and non-archive downloads remain on browse.
  • Compose config validation proves dev, prod, and prod+wepp1 service definitions parse.

Validation Evidence

  • wctl run-pytest tests/microservices/test_dedicated_download_service.py tests/docker/unit/test_download_service_routing.py -q - 19 passed after QA fixes.
  • PYTHONPATH=/home/workdir/wepppy .venv/bin/pytest tests/microservices/test_dedicated_download_service.py tests/microservices/test_download.py tests/microservices/test_browse_auth_routes.py tests/microservices/test_browse_security.py tests/microservices/test_browse_routes.py tests/docker/unit/test_download_service_routing.py -q - 140 passed after QA/security dispositions.
  • wctl run-pytest tests/microservices/test_dedicated_download_service.py tests/microservices/test_download.py tests/microservices/test_browse_auth_routes.py tests/microservices/test_browse_security.py tests/microservices/test_browse_routes.py tests/docker/unit/test_download_service_routing.py -q - 140 passed after the local stack was restarted.
  • docker run --rm -v /workdir/wepppy/docker/caddy/Caddyfile:/etc/caddy/Caddyfile:ro caddy:2-alpine caddy validate --config /etc/caddy/Caddyfile - valid configuration with preexisting formatting/header warnings.
  • docker run --rm -v /workdir/wepppy/docker/caddy/Caddyfile.wepp1:/etc/caddy/Caddyfile:ro caddy:2-alpine caddy validate --config /etc/caddy/Caddyfile - valid configuration with preexisting formatting/header warnings.
  • Local Caddy smoke against https://wc.bearhive.duckdns.org/weppcloud/runs/honeyed-marathoner/disturbed9002/download/archives/mdobre-conceivable-hickory.20260518T012715Z.zip:
    • HEAD returned 200, Accept-Ranges: bytes, Content-Length: 2516876934, Server: uvicorn, Via: 1.1 Caddy, and request id 43dc51062fa54ab3a3a61bc0bc3836ec.
    • Full GET returned 200 and downloaded 2516876934 bytes in 12.207687 seconds at reported curl speed 206171483 bytes/s.
    • Range: bytes=0-1048575 returned 206, Content-Range: bytes 0-1048575/2516876934, and downloaded 1048576 bytes.
    • Sparse curl resume from byte 2515828358 returned 206, Content-Range: bytes 2515828358-2516876933/2516876934, and completed by downloading only the final 1048576 bytes.
    • docker compose --env-file docker/.env -f docker/docker-compose.dev.yml logs --tail=80 download showed matching download.complete events for the HEAD, full GET, and both 206 probes with bytes, duration, request id, route family, range start/end, and sanitized basename.

Residual QA Risk

  • Live browser resume behavior and production throughput cannot be closed from local tests alone. This remains a production rollout validation item, not a local implementation blocker.
  • NFS remains a shared storage dependency. The dedicated service improves isolation and observability but does not remove NFS as a possible bottleneck.

Sign-off

  • QA reviewer: Codex QA review, 2026-06-19