← Back to usersum index

Security Review - Dedicated Download Service for Critical Run Artifacts

Security artifact for the dedicated download service package. Implementation findings are resolved; production rollout still requires live smoke evidence.

Metadata

• Package: docs/work-packages/20260619_dedicated_download_service/
• Reviewer: Codex implementation review
• Date: 2026-06-19
• Scope reviewed: Dedicated archive download service, browse auth/path reuse, Docker Compose service wiring, Caddy route changes, focused tests, and service documentation.
• Commit/branch context: local master worktree during implementation.
• Related artifacts:
  • Code review: no separate code-review artifact was requested for this pass; implementation findings are dispositioned through the QA and security reviews.
  • QA review: docs/work-packages/20260619_dedicated_download_service/artifacts/20260619_qa_review.md

Security Triage Decision

• Security impact level: high
• Dedicated security review required: yes
• Triage rationale: The package introduces a new externally reachable file-serving service and changes proxy routing for run-scoped artifacts. Auth, path traversal, route shadowing, logging privacy, and operational rollback must be reviewed explicitly.
• Threat model assumptions:
  • Attackers may know or guess public route shapes and may try traversal, symlink, encoded-path, or range-header abuse.
  • Some runs are public, while private and root-only paths must remain protected by the canonical browse auth contract.
  • Reverse proxy routing mistakes can expose unintended files or bypass existing browse checks even if service code is correct.
  • Logs may be accessible to operators and must not contain secrets, raw JWTs, raw query filters, or sensitive full filesystem paths.

Findings

Risk acceptance authority: Accepted-risk requires security reviewer recommendation plus explicit package owner acknowledgment in Sign-off.

Verdict

• Gate status: pass for local implementation; production cutover remains conditional on live smoke validation.
• Unresolved findings:
  • High: 0
  • Medium: 0
  • Low: 0
• Release recommendation: ship with conditions. Before considering production complete, start the service on wepp1, reload Caddy, and capture HEAD, full GET, ranged GET, and log evidence for a representative archive.

Surface Checks

1) Auth, Session, and Authorization

• Entry points enforce expected authn/authz checks for changed routes/services.
• Role checks and scope checks are explicit, least-privilege, and regression-tested.
• Session/JWT token validation paths preserve canonical contracts.
• CSRF protections are preserved for browser session mutation paths; the new route is read-only.
• Cross-service auth token mint/verify flows are not widened unintentionally.
• Error paths do not disclose token contents or auth internals.

2) Secrets and Credential Handling

• No new plaintext secrets in repository files, env defaults, or docs examples.
• *_FILE secret-file contract is preserved where applicable.
• No secrets passed in argv, query params, or logs.
• Added/changed services mount only required secrets.
• Rotation and rollback behavior are documented for new secret dependencies; no new secret dependency was introduced.
• Changed code avoids fallback wrappers that silently skip missing secrets.

3) Input Validation and Output Safety

• Untrusted input is validated at boundaries (types, ranges, enum membership).
• File/path inputs block traversal and out-of-scope path access.
• Rendered output paths avoid unsafe HTML/markdown/script injection; the route does not render user content.
• URL fetch/download flows enforce allowlist or explicit safety constraints.
• Unsafe deserialization and shell interpolation patterns are absent.
• Failing validation returns explicit contract-compliant errors.

4) File System and Run-Tree Boundaries

• Writes remain inside intended run roots (/wc1/runs/...) and approved paths; the new service is read-only.
• No new path joins allow escaping run scope via symlink or relative path tricks.
• Export/download paths avoid leaking unrelated files.
• Temporary files and artifacts are cleaned up or intentionally retained with policy; the service creates none.
• Permissions for generated files/directories are least-privilege; the service generates none.

5) Queue, Worker, and Subprocess Surfaces

• Enqueue sites and dependency edges remain intentional and documented; no queue wiring changed.
• Worker task inputs are validated before shell/subprocess/file operations; no worker/subprocess path was added.
• Subprocess invocation avoids shell injection and unbounded command composition; no subprocess call was added.
• Queue cancellation/retry paths cannot bypass auth or data ownership boundaries; no queue path was added.
• wctl check-rq-graph has been run when queue wiring changed; not applicable because no queue wiring changed.
• Failure handling preserves canonical response/error contracts.

6) Agentic Tooling and MCP Surfaces

• Agent/tool calls do not grant broader permissions than the parent request needs.
• No path for implicit privilege escalation through subagents or helper tools introduced by scaffold-only docs.
• MCP/tool tokens and credentials are scoped and not leaked in logs/artifacts.
• Tool execution constraints are explicit when running commands or file writes.
• Guardrails block unauthorized network egress or public artifact publication.

7) Network and External Integrations

• New outbound calls are justified, constrained, and observable; no outbound calls were added.
• Timeouts/retries avoid denial-of-service amplification and unsafe fallback loops.
• Internal-only endpoints are not exposed through new proxy/route changes.
• Rate limits/throttles are considered for high-cost or abuse-prone endpoints; this package isolates worker/timeouts but does not add rate limiting.
• External dependency trust assumptions are documented; no new dependency was added.

8) CI/CD and Supply Chain

• Self-hosted runner access scope is unchanged or tightened.
• Workflow token permissions are minimal for changed jobs; no workflow changed.
• Build/test scripts avoid exposing credentials in logs.
• New third-party dependencies passed precedent and risk checks; no new dependency was added.
• Pinned versions/digests are used where policy requires.

9) Data Integrity, Locking, and Concurrency

• NoDb lock/dump contracts are preserved (dump_and_unlock, lock scope); no NoDb mutation path changed.
• Cross-process shared state updates remain atomic and auditable; no shared-state mutation path was added.
• Redis keyspaces and TTL behavior are unchanged unless explicitly planned.
• Concurrent mutation paths include regression coverage for race conditions; no mutation path was added.
• Recovery after partial failure leaves state consistent and diagnosable.

10) Logging, Monitoring, and Incident Readiness

• Logs include enough context for incident triage without exposing secrets.
• Security-relevant events have observable signals (auth failures, denials, exfil attempts).
• New error handlers do not swallow exceptions silently.
• Alerting/operational checks are updated for new high-risk surfaces; health check and service logs are available, production alerting remains operator-owned.
• Rollback and containment steps are documented for the changed scope.

Validation Evidence

• Automated checks run:
  • wctl run-pytest tests/microservices/test_dedicated_download_service.py -q - 14 passed.
  • wctl run-pytest tests/docker/unit/test_download_service_routing.py -q - 5 passed.
  • wctl run-pytest tests/microservices/test_dedicated_download_service.py tests/docker/unit/test_download_service_routing.py -q - 19 passed.
  • wctl run-pytest tests/microservices/test_dedicated_download_service.py tests/docker/unit/test_download_service_routing.py -q - 19 passed after QA dispositions.
  • wctl run-pytest tests/microservices/test_download.py tests/microservices/test_browse_auth_routes.py::test_private_download_redirects_only_for_navigation tests/microservices/test_browse_auth_routes.py::test_private_download_uses_bearer_when_cookie_run_scope_mismatch tests/microservices/test_browse_auth_routes.py::test_run_download_root_only_path_uses_bearer_when_cookie_lacks_root_role -q - 9 passed.
  • 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.
  • 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 compose --env-file docker/.env -f docker/docker-compose.dev.yml config --quiet - passed.
  • docker compose --env-file docker/.env -f docker/docker-compose.prod.yml config --quiet - passed.
  • docker compose --env-file docker/.env -f docker/docker-compose.prod.yml -f docker/docker-compose.prod.wepp1.yml config --quiet - passed.
  • python3 tools/check_broad_exceptions.py --enforce-changed --base-ref origin/master - passed for tracked changed Python files; new service files were manually inspected for broad exception handlers.
• Manual checks run:
  • 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 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 warnings.
  • Local Caddy HEAD smoke against honeyed-marathoner/disturbed9002 returned 200, Accept-Ranges: bytes, Content-Length: 2516876934, Server: uvicorn, Via: 1.1 Caddy, and request id 43dc51062fa54ab3a3a61bc0bc3836ec.
  • Local Caddy full GET smoke returned 200 and downloaded 2516876934 bytes in 12.207687 seconds at reported curl speed 206171483 bytes/s.
  • Local Caddy Range: bytes=0-1048575 smoke returned 206, Content-Range: bytes 0-1048575/2516876934, and downloaded 1048576 bytes.
  • Local Caddy sparse resume smoke from byte 2515828358 returned 206, Content-Range: bytes 2515828358-2516876933/2516876934, and completed by downloading only the final 1048576 bytes.
  • Download service logs showed matching download.complete events for HEAD, full GET, and both ranged probes with sanitized artifact identity and no tokens or absolute run-root paths.

Residual Risk

• Accepted residual risks:
  • Client disconnect classification is best-effort from the Starlette streaming generator. Operators should correlate app logs with Caddy access logs for edge-level disconnect detail.
  • NFS remains a shared backend dependency. The service split removes browse worker/proxy common-cause vectors but does not guarantee recovery from NFS stalls.
• Follow-up packages/issues:
  • Potential follow-up for non-NFS/object-storage archive publication if service telemetry shows NFS remains the main reliability limit.
  • Potential follow-up to migrate other exact non-transforming downloads after archive ZIP telemetry is clean.

Sign-off

• Security reviewer: Codex implementation review, 2026-06-19
• Package owner: WEPPcloud operators, pending production rollout acknowledgment