Skip to content

fix(files): count the document body against the export limit - #6006

Merged
waleedlatif1 merged 4 commits into
stagingfrom
fix/export-count-markdown-body
Jul 28, 2026
Merged

fix(files): count the document body against the export limit#6006
waleedlatif1 merged 4 commits into
stagingfrom
fix/export-count-markdown-body

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Follow-up to #5995, from Bugbot's finding on the v0.7.47 release PR (#6005).

Summary

  • The 250 MB markdown-export cap measured only the embedded assets' declared sizes. The document body was downloaded with no limit and never counted, so a large document with modest attachments cleared the check and still produced a zip well over the stated limit, materialized whole in memory
  • The body is the largest single entry in most bundles, so excluding it left the limit unenforced against the item most able to exceed it
  • It is now capped on read and counted alongside its assets, and the message names both

Provenance

#5995 added the asset caps but did not extend them to the body. The unbounded body read predates that PR; what #5995 introduced was an export limit that silently excluded the largest item — so this closes a gap it created rather than an unrelated defect.

Type of Change

  • Bug fix

Testing

Two tests added to the suite #5995 introduced: the body counting toward the limit when assets alone sit under it, and the body read carrying the cap. Seven tests on this route pass; lint, check:utils, typecheck and check:api-validation clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

The 250 MB export cap measured only the embedded assets' declared sizes. The
markdown body was downloaded with no limit and never counted, so a large
document with modest attachments cleared the check and still produced a zip well
over the stated limit, materialized whole in memory.

The body is the largest single entry in most bundles, so excluding it left the
limit unenforced against the item most able to exceed it. It is now capped on
read and counted alongside its assets, and the message names both.

Follow-up to #5995, which introduced the asset caps without extending them to
the body.
@vercel

vercel Bot commented Jul 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 28, 2026 5:55pm

Request Review

@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes bound server memory and rejection behavior on a user-facing export path; scope is limited to size accounting and error mapping, not auth or storage semantics.

Overview
Fixes a gap where the markdown export route only enforced size on embedded assets: the document body was downloaded without a byte cap and excluded from the pre-zip total, so large bodies could pass checks and blow past the stated limit in memory.

The export handler now downloads the markdown with maxBytes set to the same 250 MB total ceiling, treats PayloadSizeLimitError as a 400 with an export-limit message, and sums body + declared asset sizes before fetching attachments (updated error text mentions “document and its embedded files”). The separate 500-embed rejection is removed in favor of the existing upstream MAX_EMBEDDED_IMAGES bound.

Export route tests are extended for body counting, capped reads, and size-limit errors; a new v1 file GET test file documents rendered download behavior (content type, disposition, Content-Length, 409 while compiling, 404).

Reviewed by Cursor Bugbot for commit 6950e22. Configure here.

Comment thread apps/sim/app/api/files/export/[id]/route.ts Outdated
Comment thread apps/sim/app/api/files/export/[id]/route.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Caps markdown export body size and counts it toward the 250 MB bundle limit.

  • Document body download uses maxBytes: MAX_EXPORT_TOTAL_BYTES and maps PayloadSizeLimitError to 400 with an export-limit message.
  • Bundle size check is mdBuffer.length plus declared asset sizes, with updated error copy naming document and embeds.
  • Removes the local 500-asset count cap (relies on upstream embed bound); adds export tests for body counting, body cap, and body-oversize 400; adds unrelated v1 file download tests.

Confidence Score: 5/5

Safe to merge; the prior body-oversize 500 path is fixed and no blocking failures remain.

Oversized body downloads are caught as PayloadSizeLimitError and returned as 400 with an export-limit message, covered by tests; body bytes are included in the bundle budget with matching coverage.

Important Files Changed

Filename Overview
apps/sim/app/api/files/export/[id]/route.ts Body download is capped and size-limited errors return 400; total budget includes body plus assets; asset count cap removed in favor of upstream embed bound.
apps/sim/app/api/files/export/[id]/route.test.ts Tests body contribution to the limit, maxBytes on the body download, and 400 (not 500) on PayloadSizeLimitError for an oversized body.
apps/sim/app/api/v1/files/[fileId]/route.test.ts New tests for v1 file download content type, disposition, Content-Length, compile 409, and missing-file 404.

Reviews (3): Last reviewed commit: "chore(files): drop the unreachable expor..." | Re-trigger Greptile

The route had no tests, and #5995 changed what it serves: rendered bytes, the
resolved content type rather than the record's source MIME, Content-Length from
the rendered length, and a retryable 409 while an artifact compiles.

One test pins the filename/content-type relationship. A review flagged the
download as naming a rendered file with a source extension, but the renderer
picks its output format from the file name — getE2BDocFormat and
COMPILABLE_FORMATS both key on it — so a .docx renders to a docx and the two
cannot disagree. The test makes that argument executable rather than a comment.
Capping the body read meant an oversized document threw PayloadSizeLimitError,
which nothing caught, so the caller got a generic 500 — hiding the very limit
message the cap was added to produce. It now returns the same 400 as the bundle
check, naming the export limit.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b7f6acd. Configure here.

extractEmbeddedFileRefs stops collecting at MAX_EMBEDDED_IMAGES, so the list this
route receives is already bounded before it arrives and the count check could
never fire. Its test only passed because it mocked the extractor, so it asserted
a branch production cannot reach.

The byte ceilings are the real bound and stay. A comment records where the count
is actually enforced, so the next reader does not add a second one.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Dropped the export's asset-count cap — it was unreachable.

extractEmbeddedFileRefs stops collecting at MAX_EMBEDDED_IMAGES (50), so imageIds is bounded before this route sees it and a check at 500 could never fire. Its test passed only because it mocked the extractor, so it asserted a branch production cannot reach — worse than no test, since it read as coverage.

The byte ceilings are the real bound and stay. A comment now records where the count is actually enforced so the next reader does not add a second one.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6950e22. Configure here.

@waleedlatif1
waleedlatif1 merged commit ec3156f into staging Jul 28, 2026
15 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/export-count-markdown-body branch July 28, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant