Skip to content

[v0.3] Artifact upload: enforce size limit during multipart parse (streaming, DoS-safe) #320

Description

@bg-playground

Goal

Artifact upload size enforcement should abort the streaming multipart parse as soon as a request exceeds the configured BGSTM_ARTIFACT_MAX_BYTES. Oversized uploads should be cut off as early as possible — not only after the full body has been buffered to disk by Starlette / python-multipart.

Background

  • As of PR feat(#298): artifact upload endpoint + pluggable storage backend #319, the /external-results/artifact endpoint applies the size limit after FastAPI/Starlette has parsed the full multipart body, so oversized uploads are only rejected after fully receiving and spooling to a server-side temp file. This leaves a DoS vector open (attackers can tie up disk/spool space even if the handler's in-code check is correct).
  • The current "partial-write before 413" test does not distinguish buffer-then-reject from true streaming abort — it passes against either implementation.
  • Production deployments should still use a reverse proxy or WAF as first-line defense (client_max_body_size in nginx, LimitRequestBody in Apache, etc.) — but DoS-class uploads should not reach handler code if avoidable.

Scope

  • Switch the artifact upload handler from FastAPI's File(...) / Form(...) dependency-injected params to direct request.stream() body reading using a streaming multipart parser (e.g. streaming-form-data or python-multipart's low-level MultipartParser).
  • Parse incoming multipart bytes incrementally, honoring the locked field-name contract (case_result_id, kind, filename, file).
  • Abort upload with 413 as soon as the in-flight running byte total crosses BGSTM_ARTIFACT_MAX_BYTES — before consuming the rest of the body.
  • Ensure partial temp files (and parser state) are cleaned up robustly on abort or error.
  • Tests:
    • True streaming abort before all data is received (use a monkeypatchable stream/fd to assert nothing else is read after the limit is crossed).
    • Handler still enforces content-type allowlist and kind=other bypass as before.
    • Malformed multipart returns 4xx, not 500.
    • Path-traversal sanitization (from feat(#298): artifact upload endpoint + pluggable storage backend #319 round 1) still in force.
  • Spec doc §f and §g updates: clarify server aborts in-stream; does not buffer full body first.
  • Recommended client_max_body_size (or equivalent) value documented in deploy docs as belt-and-suspenders defense.

Out of scope

Acceptance criteria

  • Oversized artifact uploads are aborted during body parse — server does not buffer the full body before returning 413.
  • Test asserts that bytes past the limit are not read from the request stream.
  • All existing artifact tests still pass: happy path, oversized, content-type, validation errors, FK violation, S3 stub, filename sanitization.
  • Spec doc §f / §g updated to accurately describe in-stream enforcement.
  • Deploy docs include recommended reverse-proxy body-size limit.
  • Handler remains robust against malformed, slow, or partial uploads (no 500s on bad input).

References

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions