feat(api): harden the bulk upload CSV contract (#2362)#2366
Open
jh-RLI wants to merge 2 commits into
Open
Conversation
Adds POST /api/v0/tables/<table>/bulk-upload - the tracer bullet of the bulk upload path (slice 2 of #2362): - The request body IS the CSV (text/csv); the server streams it into PostgreSQL COPY FROM STDIN without buffering the file in memory. - Append-only, one transaction per request: a malformed row anywhere rolls back the entire upload. - The delimiter is a required, whitelisted parameter (comma, semicolon, tab) - never inferred from metadata or content. - The CSV header (required) maps columns by name; header names are whitelisted against the table's actual columns and quoted, so no unvalidated identifier ever reaches the SQL. - Same authorization chain as the row API: token auth, write permission, embargo check, table-registry resolution (internal tables are unreachable by construction). - Deliberately bypasses the edit-journal meta tables: bulk-loaded rows have no per-row change history. This trades the (currently unread) per-row provenance for order-of-magnitude ingestion speed; an audit event record follows in a later slice. - COPY is FROM STDIN only; no code path for COPY FROM file/PROGRAM. New HTTP-seam test module api/tests/test_bulk_upload.py (14 tests) covers the happy path per delimiter, auth/permission/embargo denials, all-or-nothing rollback, journal bypass, and target-table containment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Slice 3 of #2362, on top of the bulk upload tracer bullet: - Header preflight before the body streams: reject duplicate column names, names not in the table, and missing required columns (NOT NULL without default), each with a 400 naming the offenders. - Strip a UTF-8 BOM from the header (Excel exports). - FORCE_NULL on all uploaded columns: an empty field is NULL whether quoted or not - a deliberate deviation from COPY's native CSV rule, because many writers quote every field and would silently store empty strings instead of NULLs. - Sanitized failure responses: the database's data-level message plus the CSV line number and column (header-adjusted), never raw SQL, server context dumps, or internal paths - including the no-diagnostics fallback (lost connection), which stays generic. Test module grows to 20 HTTP-seam tests covering each contract rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the discussion
This PR stacks ontop of #2364 wait until its merged and rebase this PR.
Part of #2362 (Slice 3 — CSV contract hardening). Implements the explicit
CSV contract on top of the bulk upload tracer bullet:
names not in the table, and missing required columns (NOT NULL without a
default) are each rejected with a 400 naming the offending columns —
a wrong column mapping costs milliseconds, not gigabytes.
FORCE_NULLon all uploaded columns),whether the field is quoted or not. Deliberate deviation from COPY's
native CSV rule (quoted
""= empty string): many common writers quoteevery field and would otherwise have their NULLs silently stored as empty
strings. Do not "fix" this back to Postgres defaults — it would silently
corrupt NULL handling for clients.
message plus the CSV line number (header-adjusted) and column — e.g.
invalid input syntax for type bigint: "boom" (CSV line 3, column id)—and never raw SQL, server context dumps, or internal paths. When the
server provides no diagnostics (connection lost mid-COPY), the message
stays generic instead of echoing socket paths.
Known minor limitation (accepted): Postgres reports COPY records; a quoted
field containing embedded newlines earlier in the file shifts the reported
line relative to physical file lines.
Tests: module grows to 20 HTTP-seam tests — one per contract rule
(duplicates, missing-required incl. serial-default interplay, BOM, column
order freedom, quoted/unquoted NULL, line+column error shape with
no-internals assertions). Full suite green. Changelog entry included.
Workflow checklist
Automation
Closes #
Part of #2362
PR-Assignee
CONTRIBUTING.md
CHANGELOG.md
mkdocs
Reviewer
Reviewer Guidelines