Skip to content

feat(user-data-export): require an emailed code to download an export - #5204

Merged
St0rmz1 merged 4 commits into
mainfrom
data-export-email-code
Aug 11, 2026
Merged

feat(user-data-export): require an emailed code to download an export#5204
St0rmz1 merged 4 commits into
mainfrom
data-export-email-code

Conversation

@RSO

@RSO RSO commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Why

CleanShot.2026-08-11.at.16.53.36.mp4

A data export is a single archive of everything stored with an account, which makes it a far more attractive target than clicking around the UI. Today a live web session is sufficient to download one, so anyone riding a stolen session (XSS, malware, an unlocked laptop) can walk off with the whole thing.

This adds an email step-up on the download. It's worth being precise about what that buys: it defends against session theft, not account compromise. The ceiling is "does the attacker have the inbox", and the inbox is already a full auth factor here via the magic-link provider. It does not substitute for MFA, which this stack doesn't have.

What changed

Downloading is now two steps:

  1. userExports.requestDownloadCode — verifies ownership/freshness, emails a 6-digit code to the account address, returns only a challengeId.
  2. userExports.createDownload — reserves the code, calls the export Worker, and consumes the code once a signed URL exists.

A data export is a single archive of everything stored with an account, so a
held web session should not be enough to exfiltrate it. Downloading now takes
two steps: request a code, which is emailed to the account address, then redeem
it for one signed URL. That means an attacker riding a stolen session also needs
the inbox.

The code is a 6-digit value stored only as an HMAC keyed by the server secret,
over the purpose, email, export id, and code. Binding the export id means a code
minted for one export cannot authorize another. Each code lives 10 minutes, has
its own 5-attempt budget keyed by challenge id, and authorizes exactly one URL
mint, because `createDownload` can otherwise be called repeatedly to re-sign.
A failure to sign releases the code instead of consuming it, so an outage does
not cost the user a code. Repeat sends are throttled to one per minute so a held
session cannot be used to mail-bomb the owner.

Codes use their own `purpose` rather than reusing `sign_in_code`, which matters
for two reasons: `createSignInCode` deletes unconsumed rows for an email and
would silently invalidate an in-flight mobile sign-in, and
/api/auth/native/token accepts any live `sign_in_code` for an email as a full
sign-in credential, so a shared purpose would make a download code redeemable
for a session.

`purpose` is a plain text column with no constraint, so the TypeScript union was
unenforced. Since the value now decides what a token may authorize, add a CHECK
constraint for the known set. Every existing row is `magic_link` or
`sign_in_code`, so it validates without a backfill.

Note that forced-SSO accounts receive the emailed code like everyone else, which
bypasses the mandatory-SSO path that `checkDomainSignInEligibility` otherwise
enforces on every sign-in surface. This is deliberate for now: running that
check would leave those accounts unable to download their own data at all.
Comment thread apps/web/src/routers/user-exports-router.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The follow-up commit correctly fixes the undelivered-code cleanup (a Mailgun throw now drops the code row instead of tripping the resend cooldown, with test coverage) and widens codes to 8 digits; the one remaining issue is the fail-open Vercel Firewall rate limit, which is not enforced until the data-export-download-code rule exists in the project's dashboard firewall config.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/auth/data-export-download-code-rate-limit.ts 24 The firewall rate limit is fail-open: an unknown rule id is reported as "not rate limited" (only a Sentry message), so the issuance cap is not enforced until the data-export-download-code rule is created in the Vercel dashboard — nothing in the repo provisions it

Fix these issues in Kilo Cloud

Files Reviewed (7 files)
  • apps/web/src/app/(app)/data-exports/DownloadCodeDialog.tsx
  • apps/web/src/app/(app)/data-exports/data-export-contract.ts
  • apps/web/src/lib/auth/data-export-download-code-rate-limit.ts - 1 issue
  • apps/web/src/lib/auth/data-export-download-codes.test.ts
  • apps/web/src/lib/auth/data-export-download-codes.ts
  • apps/web/src/routers/user-exports-router.test.ts
  • apps/web/src/routers/user-exports-router.ts
Previous Review Summaries (2 snapshots, latest commit c655eb4)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit c655eb4)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The emailed-code step-up is well designed (HMAC bound to purpose+email+export, single-use consume-after-sign, release-on-signing-failure, purpose isolation from sign-in codes), but two failure paths remain: a Mailgun exception in requestDownloadCode bypasses code cleanup so an immediate retry misleadingly reports "A code was just sent", and the newly added Vercel Firewall rate limit fails open — it is a silent no-op until the data-export-download-code rule exists in the project's dashboard firewall config.

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/routers/user-exports-router.ts 245 sendDataExportDownloadCodeEmail can throw (Mailgun client.messages.create is not try/caught), skipping deleteDataExportDownloadCode; the leftover live row triggers the 60s resend cooldown with a misleading message even though no email was sent
apps/web/src/lib/auth/data-export-download-code-rate-limit.ts 24 The new firewall rate limit is fail-open: an unknown rule id is reported as "not rate limited" (only a Sentry message), so the issuance cap is not enforced until the data-export-download-code rule is created in the Vercel dashboard — nothing in the repo provisions it

Notes on areas checked with no findings: the incremental commit's rate-limit wiring is otherwise sound — keyed by user id (the held session is the threat model), checked before code creation, fail-open only on missing rule with a Sentry signal, and covered by a new router test asserting TOO_MANY_REQUESTS and no email; the headers()-based checkRateLimit call matches the documented tRPC-context form and the module is mocked in tests to avoid the request-scope requirement. The 0212 CHECK constraint follows the repo's established plain ADD CONSTRAINT migration pattern and validates without a backfill; all MagicLinkToken zod-schema consumers are purpose-filtered so data_export_download rows never reach them; the dialog resets state on challenge change and introduces no subscriptions/timers (no memory-leak surface); the email template contains no links (test-asserted) and year is auto-injected by send(); the emails/AGENTS.md table row matches the template's variables. The forced-SSO trade-off was explicitly flagged by the author in the PR description and is a product decision, not a code defect.

Fix these issues in Kilo Cloud

Files Reviewed (16 files)
  • apps/web/src/app/(app)/data-exports/DataExportsClient.tsx
  • apps/web/src/app/(app)/data-exports/DownloadCodeDialog.tsx
  • apps/web/src/emails/AGENTS.md
  • apps/web/src/emails/dataExportDownloadCode.html
  • apps/web/src/lib/auth/data-export-download-code-rate-limit.ts - 1 issue
  • apps/web/src/lib/auth/data-export-download-codes.test.ts
  • apps/web/src/lib/auth/data-export-download-codes.ts
  • apps/web/src/lib/email.test.ts
  • apps/web/src/lib/email.ts
  • apps/web/src/routers/admin/email-testing-router.ts
  • apps/web/src/routers/user-exports-router.test.ts
  • apps/web/src/routers/user-exports-router.ts - 1 issue
  • packages/db/src/migrations/0212_data_export_download_code_purpose.sql
  • packages/db/src/migrations/meta/0212_snapshot.json (generated)
  • packages/db/src/migrations/meta/_journal.json (generated)
  • packages/db/src/schema.ts

Previous review (commit 8b60d95)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The emailed-code step-up is well designed (HMAC bound to purpose+email+export, single-use consume-after-sign, release-on-signing-failure, purpose isolation from sign-in codes verified both directions), but a Mailgun exception in requestDownloadCode bypasses the code cleanup and leaves a leftover row that makes the immediate retry misleadingly report "A code was just sent".

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/routers/user-exports-router.ts 235 sendDataExportDownloadCodeEmail can throw (Mailgun client.messages.create is not try/caught), skipping deleteDataExportDownloadCode; the leftover live row triggers the 60s resend cooldown with a misleading message even though no email was sent

Notes on areas checked with no findings: the 0212 CHECK constraint follows the repo's established plain ADD CONSTRAINT migration pattern (0206, 0211) and validates without a backfill; all MagicLinkToken zod-schema consumers are purpose-filtered so data_export_download rows never reach them; the dialog resets state on challenge change and introduces no subscriptions/timers (no memory-leak surface); the email template contains no links (test-asserted) and year is auto-injected by send(); the emails/AGENTS.md table row matches the template's variables. The forced-SSO trade-off was explicitly flagged by the author in the PR description and is a product decision, not a code defect.

Fix these issues in Kilo Cloud

Files Reviewed (15 files)
  • apps/web/src/app/(app)/data-exports/DataExportsClient.tsx
  • apps/web/src/app/(app)/data-exports/DownloadCodeDialog.tsx
  • apps/web/src/emails/AGENTS.md
  • apps/web/src/emails/dataExportDownloadCode.html
  • apps/web/src/lib/auth/data-export-download-codes.test.ts
  • apps/web/src/lib/auth/data-export-download-codes.ts
  • apps/web/src/lib/email.test.ts
  • apps/web/src/lib/email.ts
  • apps/web/src/routers/admin/email-testing-router.ts
  • apps/web/src/routers/user-exports-router.test.ts
  • apps/web/src/routers/user-exports-router.ts - 1 issue
  • packages/db/src/migrations/0212_data_export_download_code_purpose.sql
  • packages/db/src/migrations/meta/0212_snapshot.json (generated)
  • packages/db/src/migrations/meta/_journal.json (generated)
  • packages/db/src/schema.ts

Reviewed by kimi-k3 · Input: 59.9K · Output: 9.3K · Cached: 461.8K

Review guidance: REVIEW.md from base branch main

The 60-second resend cooldown only spaces consecutive codes, and each new
code starts a fresh five-attempt budget, so a held web session could keep
guessing the six-digit code and keep mailing the account owner. Adds a
Vercel Firewall rate limit keyed by user id, mirroring `magic-link-email`
on the sign-in path.

The `data-export-download-code` rule lives in the firewall configuration of
both web projects. `checkRateLimit` reports an unknown id as "not limited",
so a missing rule is reported to Sentry.
* is an opaque internal identifier, not PII like the email keying
* `magic-link-email`.
*/
export async function isDataExportDownloadCodeRateLimited(kiloUserId: string): Promise<boolean> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WARNING: This new firewall rate limit is fail-open and is not enforced until a matching rule is created in the Vercel dashboard

The file's own comment says an unknown rule id is reported as "not rate limited", and checkRateLimit never throws on a missing rule — it only surfaces error === 'not-found', which is logged to Sentry but does not block the request. Nothing in this repo creates the data-export-download-code rule (the Vercel Firewall UI/API owns it; existing magic-link-email and device-auth-deny rules live there too), so as soon as this merges the check is a no-op and the commit's stated goal — bounding a held session's total guesses and mail volume beyond the 60-second cooldown — is not actually in effect, with only a Sentry message as the signal. Please create/verify the rule in the Vercel firewall config before merging, or gate deployment on it; otherwise a rule that is absent or later removed fails silently open.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The data-export-download-code rule exists on kilocode-app: fixed window, 5 requests / 3600 seconds, API-controlled keys (so it buckets on the rateLimitKey the code passes, data-export-download-code:), action 429.

"Nothing in this repo provisions it" is accurate but is the intended state.

Separately, codes are now 8 digits rather than 6.

…code space

  Two fixes to the emailed download code.

  sendDataExportDownloadCodeEmail reports a refused address as { sent: false }
  but throws on an API or network failure, and only the first shape was handled.
  A throw escaped before deleteDataExportDownloadCode ran, so the code row
  survived and its created_at tripped the 60-second resend cooldown -- the
  immediate retry told the user a code had just been sent when none had. Both
  shapes now drop the code and return the intended error, with the cleanup
  delete caught separately so it cannot mask that message.

  The per-challenge attempt budget resets whenever a new code is issued, so it
  caps the guess rate rather than the total; the search space is what bounds a
  held session's odds over time. Widen the code from 6 to 8 digits and hoist the
  length into a single shared constant, replacing six hardcoded occurrences.
@St0rmz1
St0rmz1 merged commit 63e2677 into main Aug 11, 2026
48 checks passed
@St0rmz1
St0rmz1 deleted the data-export-email-code branch August 11, 2026 22:38
St0rmz1 added a commit that referenced this pull request Aug 12, 2026
  Resolves against the two-step emailed-code download from #5204: the
  organization branch moves into requireDownloadableExport, the shared gate
  for both download steps, so neither can be reached on stale authority.

  Migration renumbered to 0213 behind main's 0212.
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.

3 participants