feat(#320): convert upload_artifact to true streaming multipart parsing with mid-stream 413 abort

[Copilot]

The old upload_artifact handler used FastAPI's File(...)/Form(...) DI, which causes python-multipart to fully buffer the multipart body before the handler runs. The "size check" was reading from an already-spooled temp file — an attacker could exhaust disk/spool space before the 413 was ever returned.

Core change — streaming handler

Replaced File(...)/Form(...) with request: Request + streaming-form-data. A custom _SizeLimitedFileTarget raises a _SizeLimitExceeded sentinel inside on_data_received the moment cumulative bytes exceed BGSTM_ARTIFACT_MAX_BYTES. The stream loop exits immediately — no further reads from the connection.

class _SizeLimitedFileTarget(FileTarget):
    def on_data_received(self, chunk: bytes) -> None:
        self.size_bytes += len(chunk)
        if self.size_bytes > self._max_bytes:
            if self._fd:
                self._fd.close()
                self._fd = None
            raise _SizeLimitExceeded()
        super().on_data_received(chunk)

# in handler:
try:
    async for chunk in request.stream():
        parser.data_received(chunk)
except _SizeLimitExceeded:
    _safe_unlink(tmp_path)
    raise HTTPException(413, ...)

All downstream validation (kind enum, filename regex, UUID, FK lookup, content-type allowlist) runs post-parse, unchanged.

New tests

• test_oversized_upload_aborts_stream_without_reading_full_body — the load-bearing test. Uses httpx.AsyncClient(ASGITransport) with a counting async generator as the request body. Sends a 100 KiB file against a 1 KiB limit and asserts bytes_yielded < body_size // 2. This test fails against the old buffer-then-reject implementation.
• test_malformed_multipart_returns_422 — malformed body returns 422 code=validation_error, not 500.
• Updated docstring on existing test_oversized_file_returns_413_and_cleans_up to reflect that streaming abort is now the actual behavior.

Deps & docs

• backend/requirements.txt: added streaming-form-data>=1.16,<2.0 (pinned minor, Dependabot will bump).
• docs/specs/external_results_v1.md: § Size enforcement rewritten; artifact.too_large registry entry updated; new "Recommended deploy hardening" sub-section added to § g (nginx client_max_body_size as belt-and-suspenders).

Original prompt

Goal

Convert the /external-results/artifact upload handler from FastAPI's dependency-injected File(...)/Form(...) (which buffers the entire multipart body before the handler runs) to true streaming multipart parsing that aborts with 413 as soon as the in-flight byte total exceeds BGSTM_ARTIFACT_MAX_BYTES. Bytes past the limit must NOT be read from the request stream.

Closes: #320

Why this is needed (reread before starting)

The current handler in backend/app/api/external_results.py (function upload_artifact) uses UploadFile = File(...) and Form(...) parameters. By the time the handler body executes, Starlette has already consumed the full multipart body via python-multipart and spooled it to a SpooledTemporaryFile. The chunk-loop "size check" is reading from that already-fully-buffered file, not from the request stream. An attacker can tie up disk/spool space with arbitrarily large uploads even though the handler "rejects" them.

The existing test backend/tests/test_external_results_artifact.py::TestSizeEnforcement::test_oversized_file_returns_413_and_cleans_up documents this limitation in its own docstring. That test must be kept (passes either way) and a new test must be added that fails the buffer-then-reject implementation but passes the streaming implementation.

Decisions already made — do not re-open

• Use streaming-form-data (https://pypi.org/project/streaming-form-data/) as the parser. It's the only well-maintained Python library with a stable API for this exact use case and is already pulled in by similar FastAPI projects. Add it to backend/requirements.txt (and backend/pyproject.toml if used).
• Do not switch to a different HTTP framework or a custom multipart parser. No NIH; streaming-form-data is small, audited, and battle-tested.
• Field-name contract is locked at reporter SHA ab5d7c1 — case_result_id, kind, filename, file. Don't rename or add fields. The reporter side is frozen.
• Preserve all existing validation logic — kind enum coercion, filename safety regex (_SAFE_FILENAME_RE), content-type allowlist (_ALLOWED_CONTENT_TYPES) with kind=other bypass, case_result_id UUID parsing, FK-existence check, audit-log write, storage-backend round-trip. The only change is HOW the body is parsed; not WHAT is validated.
• Audit details schema is locked. All five fields (case_result_id, kind, size_bytes, filename, content_type) are required by the smoke job's assert.py. Do not change them.
• Don't touch any other endpoint. Only upload_artifact changes. Session / case-result endpoints stay on FastAPI's dependency-injected JSON body — they don't have this problem.
• Don't change the response shape, status codes, or error codes. 201 on success, 413 with code=artifact.too_large, 415 with code=artifact.unsupported_type, 422 with code=validation_error, 404 with code=case_result.not_found.

Scope

1. Convert upload_artifact to streaming multipart parsing

Rewrite the handler signature to take a raw request: Request instead of UploadFile/Form params:

from fastapi import Request

@router.post(
    "/external-results/artifact",
    response_model=ArtifactResponse,
    status_code=status.HTTP_201_CREATED,
)
async def upload_artifact(
    request: Request,
    db: AsyncSession = Depends(get_db),
    token: RunnerToken = Depends(require_runner_scope(_WRITE_SCOPE)),
) -> ArtifactResponse:
    ...

Use streaming_form_data.StreamingFormDataParser with ValueTarget (for the three text fields) and a custom file-target class. The custom file-target class:

• Spools to a tempfile.NamedTemporaryFile (prefix bgstm_artifact_).
• Tracks running byte total in self.size_bytes.
• Tracks the part's Content-Type header (passed to its __init__).
• Raises a sentinel exception (e.g. class _SizeLimitExceeded(Exception): pass) inside on_data_received as soon as size_bytes > max_bytes.

Drive the parser by reading from request.stream() chunk-by-chunk. As soon as _SizeLimitExceeded is raised:

1. Stop reading the stream. Do NOT continue to drain. Bytes past the limit must not be consumed.
2. Clean up the temp file via _safe_unlink.
3. Raise HTTPException(413, ...) with code=artifact.too_large and the existing message.

Pseudocode skeleton:

class _SizeLimitExceeded(Exception):
    pass


class _SizeLimitedFileTarget(FileTarget):
    def __init__(self, *args, max_bytes: int, **kwargs):
        super().__init__(*args, **kwargs)
        self._max_bytes = max_bytes
        self.size_bytes = 0
        self.content_type: str | None = None  # populated from part headers

    def on_data_received(self, chunk: bytes) -> None:
        self.size_bytes += len(chunk)
        if self.size_bytes > self._max_bytes:
            raise _SizeLimitExceeded()
        super().on_data_received(chunk)


# in the handler:
fd, tmp_path = tempfile.mkstemp(prefix="bgs...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

[bg-playground]

LGTM ✅ — all 11 checks green, including External Results contract smoke (proof the reporter's contract is unchanged).

Acceptance check:

Criterion	Status
request: Request handler, no File(...)/Form(...)	✅
streaming-form-data pinned >=1.16,<2.0	✅
_SizeLimitExceeded raised inside on_data_received	✅
Stream loop exits immediately on sentinel	✅
Load-bearing test asserts bytes_yielded < body_size // 2	✅
Malformed multipart → 422, not 500	✅
Spec doc § f and § g updated	✅
_safe_unlink on every error path (no temp file leaks)	✅
All pre-existing artifact tests still pass	✅

The load-bearing test is engineered correctly: a nonlocal bytes_yielded counter inside an async generator passed as httpx.AsyncClient content, with ASGITransport so each yield maps to a server receive() call. Sends 100 KiB against a 1 KiB limit, asserts the server consumed less than half the body. This test would fail against the old buffer-then-reject implementation — exactly what #320 needed to prove the fix.

Two small notes (not blocking, fix-on-touch later if you like):

1. Spec doc § f Size enforcement section has a bit of redundant phrasing — the rewritten paragraph already says "bytes past the limit are not read from the connection," then a verbatim sentence repeats it. Trimmable in any future spec polish.
2. getattr(file_target, "multipart_content_type", None) is defensive against a streaming-form-data API change. If the library ever drops that attribute, content-type silently falls through to application/octet-stream — but the existing 415 allowlist test would catch the regression loudly.

Closes #320. Merging.