A campaign's export and import are handled on disk, within bounds, and the largest import is a setting (#991, #1054) #1072

Merged

claude-bot merged 7 commits from feat/991-archive-on-disk into main 2026-10-04 06:56:42 +00:00

Conversation 2 Commits 7 Files changed 31 +3105 -114

claude-bot commented 2026-10-04 05:57:58 +00:00

Collaborator

Copy link

Part of #991 (part 2, PR A). Addresses #1054 but does not close it: see "What stays open".

Summary

• Import on disk. The upload is spooled to a temp file (app/services/archive_zip.py), not held in memory. Every read from the archive is bounded.
• Export on disk. The zip is built into a temp file on a thread and streamed. The export button is now a link the browser downloads.
• The largest import is an Admin setting. 1 GB by default, 100 MB to 4 GB. GET/PUT /api/admin/settings/import-limit, audited.
• nginx. location = /api/campaigns/import has no body limit and no request buffering; the backend enforces the setting. /api/ keeps 110m.
• The import says why it refused. The form now shows the server's sentence for a 413 or 422.
• A data.json whose root is not an object is a 422. It was a 500.

The limits

Limit	Value	Answer
Upload size	the Admin setting	413
Total decompressed	same as the upload limit	422
data.json size	8 MB	422
data.json values	400,000 (commas, colons and opening brackets outside strings)	422
A page picture	media_max_upload_bytes (8 MB)	422
Entries	20,000	422
Encrypted, duplicate names, not a zip, CRC or size mismatch	n/a	422
Disk full while spooling	n/a	507

Why data.json is capped at 8 MB

It is the one part parsed in memory, in a 512 MB backend that runs at 340 to 425 MB warm. Python holds a string with one emoji at four bytes a character, twice. At the first cap tried (32 MB), a 32 KB upload got a worker OOM-killed on the real stack.

Measured in a 512 MB container at 8 MB / 400,000:

Shape	Extra peak
Empty lists	refused unparsed, +9 MB
One wide string	+65 MB
Nested dicts, a key each, plus a wide string (dearest found)	+117 MB
Genuine-looking session text with emoji (7.8 MB)	+59 MB

"Dearest found" is the worst shape tried, not a proof.

Production's 21 sessions hold 858 KB of transcript and 1,618 segments, so a real file is several times under both caps today.

What stays open (#1054)

• Parse the import in the worker. That is what would let the caps rise. A long-running campaign will reach 8 MB eventually; its export still works, and its import would be refused with a sentence.
• The margin is thin. On the real stack, 18 in-bounds hostile uploads (up to four at once) all answered and oom_kill stayed 0, but the sampled peak reached 505 MB of 512 MB.

Changelog

• changelog.d/991-archive-on-disk.changed.md

Test plan

• Full backend suite (before the cap change and the merge of main): 7167 passed, 14 skipped, 107 deselected.
• Full frontend suite (same tree, run alone): 3806 passed | 1 skipped, eslint 0 errors.
• At the head commit: tests/export, the nginx limit test, the compose media test and both wiki reader audits: 332 passed. Lint clean. Version and changelog checks OK. All 16 coverage scripts report unclassified: none.
• Frontend at the merged tree: the seven touched test files, 183 passed.
• End to end through nginx, own compose project, no host ports:
  • export 200, import of that export 201, re-export has the same counts;
  • 101 MB with a limit of 100 MB: 413 with the worded detail, in 0.6 s;
  • 300 MB chunked with no Content-Length: 413;
  • a 1,000 MB archive with a limit of 1,024 MB: 201 in 23 s, backend peak 349 MB, oom_kill 0, nothing left in /tmp.
• Not run: a real browser against an early 413 (the proof client was httpx); journeys, bot and synthetic suites (CI).

For review

• The wiki .body and .title read-audit lists gained an entry, because the export's file-writing moved into a nested function. Same reads, same GM-only route. Those lists are a privacy control, so it is worth a glance.
• Archive reads run on one dedicated thread. asyncio.to_thread left about 17 MB in each pool thread's arena.
• Pictures are stored, not deflated, in the export. No schema change.

🤖 Generated with Claude Code

Part of #991 (part 2, PR A). Addresses #1054 but does **not** close it: see "What stays open". ## Summary - **Import on disk.** The upload is spooled to a temp file (`app/services/archive_zip.py`), not held in memory. Every read from the archive is bounded. - **Export on disk.** The zip is built into a temp file on a thread and streamed. The export button is now a link the browser downloads. - **The largest import is an Admin setting.** 1 GB by default, 100 MB to 4 GB. `GET`/`PUT /api/admin/settings/import-limit`, audited. - **nginx.** `location = /api/campaigns/import` has no body limit and no request buffering; the backend enforces the setting. `/api/` keeps `110m`. - **The import says why it refused.** The form now shows the server's sentence for a 413 or 422. - **A `data.json` whose root is not an object** is a 422. It was a 500. ## The limits | Limit | Value | Answer | |---|---|---| | Upload size | the Admin setting | 413 | | Total decompressed | same as the upload limit | 422 | | `data.json` size | 8 MB | 422 | | `data.json` values | 400,000 (commas, colons and opening brackets outside strings) | 422 | | A page picture | `media_max_upload_bytes` (8 MB) | 422 | | Entries | 20,000 | 422 | | Encrypted, duplicate names, not a zip, CRC or size mismatch | n/a | 422 | | Disk full while spooling | n/a | 507 | ## Why `data.json` is capped at 8 MB It is the one part parsed in memory, in a 512 MB backend that runs at 340 to 425 MB warm. Python holds a string with one emoji at four bytes a character, twice. At the first cap tried (32 MB), a 32 KB upload got a worker OOM-killed on the real stack. Measured in a 512 MB container at 8 MB / 400,000: | Shape | Extra peak | |---|---| | Empty lists | refused unparsed, +9 MB | | One wide string | +65 MB | | Nested dicts, a key each, plus a wide string (dearest found) | +117 MB | | Genuine-looking session text with emoji (7.8 MB) | +59 MB | "Dearest found" is the worst shape tried, not a proof. Production's 21 sessions hold 858 KB of transcript and 1,618 segments, so a real file is several times under both caps today. ## What stays open (#1054) - **Parse the import in the worker.** That is what would let the caps rise. A long-running campaign will reach 8 MB eventually; its export still works, and its import would be refused with a sentence. - **The margin is thin.** On the real stack, 18 in-bounds hostile uploads (up to four at once) all answered and `oom_kill` stayed 0, but the sampled peak reached 505 MB of 512 MB. ## Changelog - [x] `changelog.d/991-archive-on-disk.changed.md` ## Test plan - [x] **Full backend suite** (before the cap change and the merge of main): `7167 passed, 14 skipped, 107 deselected`. - [x] **Full frontend suite** (same tree, run alone): `3806 passed | 1 skipped`, eslint 0 errors. - [x] **At the head commit:** `tests/export`, the nginx limit test, the compose media test and both wiki reader audits: `332 passed`. Lint clean. Version and changelog checks OK. All 16 coverage scripts report `unclassified: none`. - [x] **Frontend at the merged tree:** the seven touched test files, `183 passed`. - [x] **End to end through nginx**, own compose project, no host ports: - export 200, import of that export 201, re-export has the same counts; - 101 MB with a limit of 100 MB: 413 with the worded detail, in 0.6 s; - 300 MB chunked with no `Content-Length`: 413; - a 1,000 MB archive with a limit of 1,024 MB: 201 in 23 s, backend peak 349 MB, `oom_kill 0`, nothing left in `/tmp`. - [ ] **Not run:** a real browser against an early 413 (the proof client was httpx); journeys, bot and synthetic suites (CI). ## For review - The wiki `.body` and `.title` read-audit lists gained an entry, because the export's file-writing moved into a nested function. Same reads, same GM-only route. Those lists are a privacy control, so it is worth a glance. - Archive reads run on one dedicated thread. `asyncio.to_thread` left about 17 MB in each pool thread's arena. - Pictures are stored, not deflated, in the export. No schema change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 6 commits 2026-10-04 05:57:59 +00:00

A campaign's export and import are handled on disk, within bounds, and the largest import is a setting (#991 part 2, #1054) ... 7f89f54107

An import held the whole upload in memory (up to 100 MB) and read data.json
out of it with no cap on what it unpacked to, so a few kilobytes of zip could
exhaust the backend. An export built the zip, the text of data.json and every
picture in memory, on the event loop.

- services/archive_zip.py (new): spool_upload receives the multipart file into
  an anonymous temp file as it arrives and refuses one over the limit with a
  413, on Content-Length and then on the running count. Archive reads entries
  through bounded readers: the declared size is checked first, the real bytes
  are counted 64 KB at a time against the entry's cap and the archive's total.
  An encrypted entry, two entries of one name, more than 20,000 entries (read
  from the end record before zipfile lists them), a file that is not a zip and
  a CRC failure are refusals, 422 in words. data.json: 32 MB, and 2,000,000
  values counted before it is parsed, since size alone does not bound what
  parsing costs. write_zip and SpoolResponse build an export in a temp file
  and send it with its length, closing it when the response ends or fails.
- import_service.import_campaign takes the Archive. data.json and pictures go
  through it, on one dedicated thread (the default pool left an arena's worth
  of freed pictures in every thread). A data.json that is not an object is
  refused rather than a 500.
- The import route no longer uses UploadFile, ends the transaction it opened
  to read the limit before the upload starts, and closes the file in a
  finally. The export route builds on a thread and stores pictures as they
  are.
- The limit: settings_service.get_campaign_import_limit_bytes (1 GB, held to
  100 MB-4 GB), GET and PUT /api/admin/settings/import-limit, audited.
- config: transfer_spool_dir.
- nginx: the import has a location of its own with no body limit, no request
  buffering and 30 minute timeouts; /api/ keeps the limit #1057 gave it.

No change to what an export holds (schema 17); every older file imports as it
did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The export is a link the browser downloads, the import says why it was refused, and Admin sets the largest import (#991 part 2) ... 95a4ae5b0f

- api/campaigns.js: exportCampaign follows a link to the export where it used
  to fetch the whole zip as a blob. It asks for the campaign first, so a
  session that has ended or a server that is not answering still rejects and
  the buttons keep their error state.
- ImportFailedBanner: a 413 and a 422 show the server's own sentence, on
  Home's import and Admin's. A 413 with no words is a proxy's.
- Admin → Import a campaign: "Largest import", in megabytes, with the
  range, what it is now, Save, and what was stored (IS-486).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Docs, changelog and catalogue for exports and imports on disk (#991 part 2) ... 405fe91483

- docs/API.md: the import's limit, its 413, 507 and every 422 with its
  wording; the import-limit setting; the export's Content-Length.
- docs/OPERATIONS.md: an outer reverse proxy's upload limit for the import
  route, and the disk the spool needs; what data.json costs in memory.
- Ledger rows IS-486 (the Admin field) and IS-487 (the refusal's words),
  classified in lanes 7 and 1b; the catalogue rebuilt with build.ps1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The wiki's read lists name the export's file-writing part (#991 part 2) ... a2984708fe

The .body and .title audits key each read by its enclosing function. The
export's reads that write the zip's files moved into write_entries, which
runs on a thread: the same reads on the same GM-only route, listed under
the new name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge origin/main into feat/991-archive-on-disk 20cfead940

data.json is read within 8 MB and 400,000 values, counted outside strings (#991 part 2, #1054) ... 84d1c76b1b

At 32 MB and 2,000,000 values a 32 KB zip still got a backend worker
OOM-killed: Python holds a string with one emoji in it at four bytes a
character, twice (the decoded text, then the parsed value), and every
object is a few hundred bytes. The backend has 512 MB and runs at 340 to
380 MB once warm.

- DATA_JSON_MAX_BYTES 8 MB, DATA_JSON_MAX_VALUES 400,000. In a 512 MB
  container the dearest file found inside both (dicts nested in dicts, each
  under a key of its own, then one long wide string) cost 117 MB while it
  was parsed; at 12 MB it cost 149 MB, and at 500,000 values 127 MB.
- A value is now a comma, a colon or an opening bracket outside a string:
  every value and every key. Counting keys is what bounds nested dicts,
  which cost one counted value each before and were the dearest shape.
  Leaving strings out means a transcript full of commas is as many values
  as it has fields. Counted with finditer and an early stop: a substitution
  built a fragment per string and cost more than the parse it prevents.
  The pattern always matches from an opening quote, so the scan is one pass
  on any input.
- Docs and the changelog say the read is bounded and that parsing in the
  worker, which would let the bounds rise, remains: #1054 is not closed by
  this.
- The catalogue rebuilt from the merged sources.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot commented 2026-10-04 06:05:32 +00:00

Author

Collaborator

Copy link

Independent review of 84d1c76b: not to be merged yet. Two confirmed defects let any signed-in user push the backend past its memory with a small upload. Fixes are in progress on this branch.

Findings

1. High: a bzip2 or LZMA entry decompresses without bound. zipfile decompresses each compressed block whole before truncating to the declared size. A 741-byte zip took memory from 22 MB to 1,626 MB before the refusal. main has the same hole. Fix: refuse any entry that is not stored or deflated.
2. High: the entry-count pre-check reads a different end record than zipfile. A classic end record that says "1 entry", with a zip64 locator before it, passes the pre-check; zipfile then builds every entry. 400,000 empty entries cost 230 MB. Fix: read the same record zipfile reads, and pin the two together with a test.
3. Medium: nothing caps imports in flight. A user can hold many 1 to 4 GB spools open by trickling bytes. Fix: two at once per process, one per user, and a 120 s stall bound.
4. Low: an integer of more than 4,300 digits is a 500, not a 422.
5. Note: the read-audit entry is keyed on a generic function name. It will be renamed to a specific one.

Found sound

• Nothing is spooled before the user is authenticated.
• Import, export and the admin setting keep the right roles.
• Deflate and stored entries are bounded.
• No archive name reaches the filesystem.
• The value counter matched a reference tokenizer on 200,000 inputs and is linear.
• Temp files are closed on every path.

Not checked by the review: behaviour in a running stack (it read the code and ran the module locally), and the frontend.

**Independent review of `84d1c76b`: not to be merged yet.** Two confirmed defects let any signed-in user push the backend past its memory with a small upload. Fixes are in progress on this branch. **Findings** 1. **High: a bzip2 or LZMA entry decompresses without bound.** `zipfile` decompresses each compressed block whole before truncating to the declared size. A 741-byte zip took memory from 22 MB to 1,626 MB before the refusal. `main` has the same hole. Fix: refuse any entry that is not stored or deflated. 2. **High: the entry-count pre-check reads a different end record than `zipfile`.** A classic end record that says "1 entry", with a zip64 locator before it, passes the pre-check; `zipfile` then builds every entry. 400,000 empty entries cost 230 MB. Fix: read the same record `zipfile` reads, and pin the two together with a test. 3. **Medium: nothing caps imports in flight.** A user can hold many 1 to 4 GB spools open by trickling bytes. Fix: two at once per process, one per user, and a 120 s stall bound. 4. **Low: an integer of more than 4,300 digits is a 500**, not a 422. 5. **Note: the read-audit entry is keyed on a generic function name.** It will be renamed to a specific one. **Found sound** - Nothing is spooled before the user is authenticated. - Import, export and the admin setting keep the right roles. - Deflate and stored entries are bounded. - No archive name reaches the filesystem. - The value counter matched a reference tokenizer on 200,000 inputs and is linear. - Temp files are closed on every path. **Not checked by the review:** behaviour in a running stack (it read the code and ran the module locally), and the frontend.

rbrooks added 1 commit 2026-10-04 06:32:21 +00:00

Harden the import reader: five review fixes (#991 part 2, PR 1072) ... d7e755ac37

An independent review of the archive reader found two HIGH defects and three
smaller ones. Each is fixed with a failing-first test.

1. HIGH — a bzip2 or LZMA entry decompressed a whole block before the size was
   checked, so a 741-byte zip whose data.json declared two bytes and held
   800 MB of zeros took 1.6 GB. Archive.__init__ now refuses any entry whose
   compress_type is not store or deflate (the only kinds an export writes),
   before any read. Refused at 22 MB / unchanged RSS.
2. HIGH — the directory pre-check read the zip64 record only when the classic
   end record held the 0xFFFF sentinels, but zipfile uses the zip64 record
   whenever a locator precedes the end record. A classic record lying '1 entry'
   over a zip64 record of 400,000 passed the pre-check and zipfile then built
   400,000 ZipInfos (230 MB, seconds, on the one archive thread). The pre-check
   now mirrors zipfile (always look for the locator at eocd-20, read the record
   at locator-56) and cross-checks against zipfile._EndRecData, refusing on any
   disagreement. A test pins the mirror to the library on crafted files. Peak
   while opening the bomb: 0 MB.
3. MEDIUM — nothing bounded imports in flight. Now: two at a time per worker
   process (503 + Retry-After, before any body is read), one per user (429), and
   an upload that sends nothing for 120 s is dropped (408). The slot is released
   on every path. The backend runs two workers, so the server-wide bound is
   workers x 2 x the import limit of spool disk; documented in OPERATIONS.
4. LOW — an integer of more than 4,300 digits made json.loads raise a bare
   ValueError, a 500. read_json now catches ValueError and refuses in words.
5. NOTE — the wiki read-audit lists keyed the export's writer on the generic
   name 'write_entries'; renamed to '_write_export_entries' so a future function
   of the old name is not silently allowed.

Docs (API.md 503/429/408 and the bzip2 refusal; OPERATIONS the real disk bound),
the import form's refusal banner (shows the server's 503/429/408 sentence), and
the changelog fragment updated. No source under redesign-surfaces changed, so the
catalogue is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude-bot commented 2026-10-04 06:34:15 +00:00

Author

Collaborator

Copy link

All five review findings fixed in d7e755ac. Each has a test that fails on the earlier commit.

Finding	Fix	Answer
1. bzip2 / LZMA entries unbounded	An entry that is not stored or deflated is refused when the archive is opened, before any read	422
2. Entry-count pre-check bypassed	The pre-check reads the record zipfile reads, compares its answer with zipfile._EndRecData, and refuses the file if they disagree	422
3. No cap on imports in flight	Two at once per worker process; one per user; no body bytes for 120 s ends the upload	503 with Retry-After; 429; 408
4. A 5,000-digit integer was a 500	read_json catches ValueError	422
5. Audit entry keyed on a generic name	The nested function is _write_export_entries	n/a

The reviewer's two proof files, re-run by me against d7e755ac

• bzip2 (741 bytes, 800 MB of zeros): refused at open with the compression sentence. Before: memory went from 22 MB to 1,626 MB.
• zip64 (400,000 entries, both variants): the pre-check sees 400,000 and refuses; 0 MB traced, 0.0 s. Before: 230 MB over 34 s.

Ran at this commit

• tests/export, the nginx limit test and both wiki reader audits: 344 passed.
• The compose media test: 2 passed.
• Lint clean; version and changelog checks OK.
• The touched frontend tests: 77 passed.

Not run at this commit: the full suites (CI), and the end-to-end run through nginx. The new 503, 429 and 408 answers are covered by route tests only.

The in-flight cap is per worker process, so the disk bound is workers × 2 × the import limit. docs/OPERATIONS.md says so.

**All five review findings fixed in `d7e755ac`.** Each has a test that fails on the earlier commit. | Finding | Fix | Answer | |---|---|---| | 1. bzip2 / LZMA entries unbounded | An entry that is not stored or deflated is refused when the archive is opened, before any read | 422 | | 2. Entry-count pre-check bypassed | The pre-check reads the record `zipfile` reads, compares its answer with `zipfile._EndRecData`, and refuses the file if they disagree | 422 | | 3. No cap on imports in flight | Two at once per worker process; one per user; no body bytes for 120 s ends the upload | 503 with `Retry-After`; 429; 408 | | 4. A 5,000-digit integer was a 500 | `read_json` catches `ValueError` | 422 | | 5. Audit entry keyed on a generic name | The nested function is `_write_export_entries` | n/a | **The reviewer's two proof files, re-run by me against `d7e755ac`** - **bzip2 (741 bytes, 800 MB of zeros):** refused at open with the compression sentence. Before: memory went from 22 MB to 1,626 MB. - **zip64 (400,000 entries, both variants):** the pre-check sees 400,000 and refuses; 0 MB traced, 0.0 s. Before: 230 MB over 34 s. **Ran at this commit** - `tests/export`, the nginx limit test and both wiki reader audits: 344 passed. - The compose media test: 2 passed. - Lint clean; version and changelog checks OK. - The touched frontend tests: 77 passed. **Not run at this commit:** the full suites (CI), and the end-to-end run through nginx. The new 503, 429 and 408 answers are covered by route tests only. The in-flight cap is per worker process, so the disk bound is workers × 2 × the import limit. `docs/OPERATIONS.md` says so.

claude-bot merged commit cbfae94ee3 into main 2026-10-04 06:56:42 +00:00

claude-bot referenced this pull request from a commit 2026-10-04 06:56:43 +00:00

Merge PR #1072: a campaign's export and import are handled on disk, within bounds, and the largest import is a setting (#991, #1054)

claude-bot deleted branch feat/991-archive-on-disk 2026-10-04 06:56:43 +00:00

claude-bot referenced this pull request 2026-10-04 06:56:51 +00:00

[Security] Campaign import reads data.json with no cap on its decompressed size #1054

claude-bot referenced this pull request 2026-10-04 06:56:54 +00:00

Campaign export leaves out quest details, character sheets, maps and handout files #991

rbrooks referenced this pull request from a commit 2026-10-04 06:57:14 +00:00

chore(release): v4.5.0-rc3

claude-bot referenced this pull request 2026-10-04 06:57:21 +00:00

chore(release): v4.5.0-rc3 #1073

claude-bot referenced this pull request 2026-10-04 10:33:01 +00:00

Campaign export leaves out quest details, character sheets, maps and handout files #991

claude-bot referenced this pull request 2026-10-05 07:44:10 +00:00

[Security] Campaign import reads data.json with no cap on its decompressed size #1054

claude-bot referenced this pull request 2026-10-05 10:14:39 +00:00

An import can build far more than it parses: bound it by row counts #1105

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Quest-Board!1072

No description provided.