[AnnatarHe]

Summary

Readers can now email the HTML Notebook export from the Kindle iPhone/iPad app (subject like iPhone Notebook export for 经略幽燕：宋辽战争军事灾难的战略分析 (曾瑞龙)) to ClippingKK.

1. Resend receives the mail and calls POST /api/v3/inbound/resend with an email.received event.
2. The route checks the Svix signature and queues an import-email BullMQ job.
3. The job requires dmarc=pass, then parses the attached HTML. If the sender is an address a reader has bound and verified, the highlights become that reader's clippings.
4. Any other sender is logged as an error and gets a reply in the same thread with the notebook attached as a JSON file. Nothing is imported.

What's in it

• Sender addresses (Settings → Email import, email_bindings)
  • At most 10 addresses per reader.
  • Adding an address mails it a 6-digit code (30-minute expiry, 5 attempts). Adding a pending address again re-sends the code. A code that couldn't be mailed is withdrawn.
  • Code sends are rate-limited per reader and per target address.
  • A verified address belongs to exactly one reader, enforced by a partial unique index. The conflict is only reported at verify time, so adding an address can't be used to probe which addresses are bound.
  • REST routes under /api/v3/email-bindings accept the cookie session only, like the MCP token routes.
• Sender authentication (authentication.ts)
  • DMARC is read from Authentication-Results by a strict RFC 8601 parser:
    • every header that differs only in letter case is read;
    • comments and quoted strings are ignored;
    • if several results are found, any one that isn't a pass wins;
    • EMAIL_IMPORT_AUTHSERV_ID (optional) ignores results stamped by any other server.
  • Only dmarc=pass is imported or answered. Mail with no result is dropped too, unless EMAIL_IMPORT_ALLOW_MISSING_DMARC=1; each email let through that way logs a WARN.
  • Results are counted in app.email_import.authentication.
• JSON reply to unverified senders (reply.ts)
  • The attachment is a flat clipping list: { clippings: [{ title, author, content, pageAt, createdAt }], notes: [{ title, author, content, pageAt }] }.
  • The body is a short English and Chinese note on how to bind the address.
  • Limits (Redis, per UTC day and month, keyed by a hash of the address):
    • 5 replies a day and 30 a month per sender address
    • 500 replies a day overall
    • 1,000 fetches a day of strangers' mail
  • Quota is charged only when a reply goes out, so forged or junk mail spends nobody's allowance.
  • A sender already at its limit is turned away before anything is fetched.
  • Strangers' mail is limited to one 5 MB attachment.
  • A retried job is counted once and reuses a Resend idempotency key.
• Notes (reader_notes)
  • Kindle notes are saved to a new table, linked to the highlight they follow.
  • They are listed on a plain Settings → Notes page for now, to be polished later.
• Parser (kindle-notebook.ts, using node-html-parser)
  • Highlights are recognised by their colour span, so exports in other languages work.
  • It tolerates the exports' mismatched closing tags.
  • pageAt uses the .txt importer's format: the page when there is one, else #location.
• Importer (import.ts)
  • Book match, in order:
    1. the reader's same-title clipping that already has a book;
    2. the first Wenqu hit whose title matches the notebook's;
    3. otherwise the clippings are left unmatched.
  • Highlights already imported from My Clippings.txt are skipped, deleted ones included.
• Refactor: createClippings' insert moves to src/server/clippings/create.ts with no change in behaviour, so the mutation and the email import share it.
• Account deletion now also deletes a reader's bindings and notes. main is merged in, alongside its mcp_tokens purge.
• Docs: docs/email-import.md covers setup, flow, replies and limits. CLAUDE.md gets a short section.

Deploy notes

• Run pnpm db:migrate before deploying. Migration 0006_email_import adds email_bindings and reader_notes, and the startup schema check exits while they are missing.
• Resend: set up receiving (a receiving domain or the *.resend.app address) and add a webhook for email.received pointing at https://<APP_ORIGIN>/api/v3/inbound/resend.
• Env:
  • RESEND_WEBHOOK_SECRET (whsec_…) and INBOUND_EMAIL_ADDRESS are required. Without either, the webhook answers 503.
  • RESEND_API_KEY fetches received mail and sends the JSON replies.
  • EMAIL_IMPORT_ALLOW_MISSING_DMARC defaults to 0.
• After deploying, send yourself a notebook export and open its headers in the Resend dashboard:
  • If it has an Authentication-Results header: set EMAIL_IMPORT_AUTHSERV_ID to the first word of its value.
  • If it has none: nothing will import until EMAIL_IMPORT_ALLOW_MISSING_DMARC=1. The docs explain the risk.

Test plan

• pnpm test: 86 files, 657 tests. New tests cover:
  • the parser: sample export, notes, bookmarks, localized headings, malformed HTML
  • Authentication-Results: comments, quoted strings, duplicate and case-variant headers, joined values, the authserv-id pin
  • bindings (PGlite): cap, re-send, a withdrawn unsent code, verify-time 409 in both orders, expiry, attempt cap, display-name addresses
  • the importer (PGlite): idempotent re-import, .txt dedup and the repeated-highlight case, book reuse and title matching, notes, the body fallback
  • DMARC rules: pass, none, temperror, absent, the env switch and its warning
  • the JSON reply: shape, threading, idempotency, daily limit, retries counted once, no quota spent on forged or junk mail, nothing fetched over the limit, a single attachment for strangers
  • reply quotas: monthly cap, a fresh day, the 500 and 1,000 caps, the read-only check, retries not spending a second fetch
  • the webhook: signature, replay, ignored events, a 503 without the secret or the inbound address
  • the dialog component
  • the deletion purge
• pnpm lint, pnpm format:check, pnpm typecheck, pnpm typecheck:server
• Manual run of the first commit on Postgres 16 and Redis with pnpm dev:
  • The migration applies.
  • Settings → Email import and Settings → Notes render in light and dark, on desktop and mobile.
  • The binding routes return the expected codes.
  • A signed webhook is queued once per email id, and a bad signature gets 401.
  • Fetching mail and sending through Resend could not be exercised: the sandbox blocks resend.com.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CCcqmwTy6GqiC7gN4HCjMa

---

Generated by Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Review: email import of Kindle notebook exports

I read the webhook, bindings, importer and reply code. I didn't run the tests or the app. The design is careful. Svix verification, a bounded body, a deterministic job id, a counted-before-compare code check, the partial unique index on verified addresses, a Resend idempotency key on replies, and docs that state the forgery limits openly all hold up. Tests cover the parser, importer, bindings, reply and webhook.

Worth addressing

1. Unauthenticated senders cost a lot before any quota applies (import.ts, run()).
   • For every email addressed to the import address, the worker does the following before claimReplyQuota runs:
     • fetches the email from Resend;
     • downloads up to 5 attachments of 10 MB each;
     • parses them with node-html-parser.
   • Only an unbound sender's reply is capped. The work is not.
   • The queue is shared with exports and runs at concurrency 1, so a stream of junk mail with large HTML attachments can starve legitimate imports and exports.
   • Suggested fixes:
     • For unbound senders, check the sender's own quota (read-only) before downloading anything.
     • Or drop mail that arrives with no HTML attachment, using the attachment metadata Resend already gives you, before the heavy path.
     • Consider a separate queue, or a lower attachment cap, for unbound senders.
2. A missing Authentication-Results header is treated as pass (import.ts, the dmarc=fail check).
   • Only an explicit dmarc=fail is rejected. Mail with no header, or with dmarc=none, goes through.
   • The docs already say a forger who knows a reader's bound address can add highlights. Consider requiring dmarc=pass (or spf/dkim pass) for the bound-sender import path. The JSON-reply path can stay lenient.
   • If Resend doesn't always set the header, at least log how often it's absent.
3. log('ERROR', 'Email import sender is not bound to any reader') fires for every stranger or spam message.
   • It is intended, per the PR description, but it will make an error-level alert noisy and hide real failures.
   • Consider WARN, or ERROR only once the sender has passed the notebook-present check.

Smaller points

• claimReplyQuota increments three counters one at a time. If the global limit rejects, the sender's day and month counters have already been consumed, so a global-limit episode burns senders' allowances. The ordering is documented, but the reverse effect isn't.
• claimReplyQuota writes the claimed marker before the send. If the job dies after the quota claim and the Resend idempotency key has expired (24 h), a retry can't resend. That seems acceptable, given the 30 s exponential backoff stays well inside 24 h.
• requestEmailBinding upserts the code before sendEmailBindingCode. If the send fails, the row has a live code the reader never received, and the rate-limit slot is already spent. Either roll back or note it as accepted.
• saveNotebook dedupes on sameText across everything the reader has under the same title. Two genuine highlights with identical text on different pages collapse into one. This is probably the right trade-off against the .txt importer's duplicates, and a one-line comment saying so would help.
• readerNotes.dataId comes from clippingDataId(userId, content, pageAt), so two identical notes on one page collapse. This is the same trade-off.
• The code hash is sha256(email\ncode). As the comment says, the 5-attempt cap and the expiry carry the security. An HMAC with a server secret would cost little and remove the offline brute force if the table ever leaks.

Nits

• replyHtml interpolates origin directly. It comes from env, so it's safe, but escaping makes that obvious.
• Migration 0006 has no foreign keys, matching the other tables. If that is a deliberate convention, ignore this point.

Overall this is solid, and I'd merge after addressing (1), and (2) if the header is dependable.

[AnnatarHe]

Thanks for the review. These points are addressed in e65d030:

1. Work done for unauthenticated senders

• An email from an unverified address is now counted against the reply limits before anything is fetched, whether or not it holds a notebook.
• Once a sender is over a limit (5 a day or 30 a month per address, 500 a day overall), the job ends without calling Resend or downloading anything. That bounds the heavy work for strangers to at most 500 emails a day.
• Unverified senders are also limited to one HTML attachment of up to 5 MB. Verified readers keep 5 attachments of 10 MB.
• I kept the shared queue rather than adding a second one: with the work bounded, a separate queue wasn't worth the extra plumbing.

2. Missing Authentication-Results

• The DMARC result (pass, fail, none, temperror, permerror, other, or absent) is now counted in app.email_import.authentication, split by verified and unverified sender.
• I haven't made dmarc=pass a requirement yet. I can't confirm that Resend always sets this header, and if it doesn't, requiring a pass would block every import. The metric will show whether it's safe; the docs say so.

Smaller points

• claimReplyQuota now reads all three counters before bumping any. A full global allowance no longer spends senders' own, and a sender at its limit doesn't spend the global one. There is a test for each direction.
• A binding code that couldn't be mailed is withdrawn, so the row no longer holds a live code nobody received. The rate-limit slot stays spent on purpose.
• I added comments on the two text-only dedup trade-offs: identical highlights on different pages, and identical notes at one location.
• The reply's origin link is now escaped.

Not changed

• 3. ERROR for every unbound sender: the requirement was an error log for any sender that doesn't match a binding, so it stays at ERROR. Over-limit strangers are now cut off before any fetch, so the log line is all that's left of them.
• Writing the quota marker before the send: agreed it's acceptable. Retries back off from 30 s, well inside Resend's 24 h idempotency window.
• HMAC on the code: the expiry and the 5-attempt cap carry the security. An HMAC would need a new dedicated server secret just for this, so I'm leaving it.
• Foreign keys: no other table in the schema declares one, so 0006 follows that.

---

Generated by Claude Code

[claude]

Review: Kindle notebook import by email

I read the webhook, bindings, reply and importer code. I did not run the tests or the app, and I skipped the UI, the locales and the migration. The structure is clean and the layers are well separated. The webhook verifies the Svix signature, bounds the body size and answers quickly. Binding codes are compared in constant time and the attempt is counted before the compare. The reply path is rate-limited with a retry-safe idempotency key. The notes are explicit about what the code does and doesn't protect.

Security

1. Imports into a reader's account only block on dmarc=fail (src/server/email-import/import.ts:329-345)

• From is the only credential for a verified binding. Today absent, none, temperror, permerror and other all proceed to saveNotebook.
• Anyone who knows a reader's bound address can forge From from a sender with no DMARC record, or one at p=none. They can then write arbitrary clippings and notes into that reader's library.
• The JSON-reply path, which sends nothing into an account, can reasonably stay lenient. The import path should require dmarc === 'pass', or at least SPF/DKIM alignment.
• If strictness would break legitimate iCloud/Gmail senders, make it a deliberate decision and document it in docs/email-import.md.

2. A forged sender can burn another address's reply quota (import.ts:307 before :341)

• claimReplyQuota runs before the DMARC check, so spoofed mail from victim@x.com uses up that address's 5/day and 30/month allowance.
• The real owner then gets reply_limited. This is low impact.
• Moving the authentication check ahead of the quota claim fixes it, but that needs receiving.get first. That is the cost the current ordering avoids, so it's a trade-off.
• At least document it.

3. Attacker-controlled content is mailed to a third party

• The attachment is the sender's own HTML converted to JSON. Combined with a forged From it is a small spam or backscatter channel, capped at 5 per day per address and 500 per day overall.
• DMARC fail is already dropped, so this only applies to domains without DMARC.
• Requiring pass before replying would close it as well.

4. Account enumeration (bindings.ts:92-103)

• The EMAIL_ALREADY_BOUND 409 tells any signed-in user whether an arbitrary address is registered with ClippingKK.
• That may be acceptable, but it is worth a conscious call.
• Checking it after the code is sent, or returning the same response either way, would avoid the leak.

Correctness and robustness

• requestEmailBinding check-then-insert is racy (bindings.ts:104-142). Parallel requests can exceed MAX_EMAIL_BINDINGS. Enforcing the cap in a transaction, or with a trigger or lock, would make it hold.
• claimReplyQuota isn't atomic (reply.ts:102-107).
  • The three rateLimit increments run in sequence. If the second or third is denied or throws, the earlier counters stay incremented. A retry then won't have the claimed key and will count again.
  • A small Lua script, or a single MULTI, would make it all-or-nothing.
  • The read-then-bump window also lets a burst slightly exceed the limits.
• Duplicate-content matching (import.ts:113-117, 161). sameText strips all whitespace and punctuation at the edges, so identical highlights on different pages merge. The comment acknowledges this. A test pinning the behaviour would help.
• Wenqu lookup (import.ts:138). books[0] is accepted for any title, with no similarity check. A wrong match silently attaches the highlights to the wrong book. Matching author or title would be safer. The book-page rematch exists, but the wrong match isn't visible.
• Reply on retry (reply.ts). Resend idempotency keys expire (about 24h). A job retried after that could send a second reply, though the claimed key blocks it for 7 days anyway.

Performance

• saveNotebook loads every existing clipping (content) for the title (import.ts:155-160), and again for notes. That is fine for typical books but unbounded for heavy readers. Selecting once and reusing the rows would halve it.
• The attachments loop downloads sequentially. That's fine at ≤5 files.

Tests

I saw coverage for bindings, import, parser, reply and webhook. I'd add:

• a case showing dmarc=none or an absent header does not import (after fixing fix(parser): fix clippings parser and add testcase #1)
• the quota-claim failure and retry paths
• a concurrent binding-cap test

Nits

• isUniqueViolation could live in a shared DB helper if another place needs it.
• bindings.ts:163 declares two variables with one const and a comma, which is unlike the rest of the code.

Overall this is solid, thoughtful work. Item 1 is the one I'd want addressed before merge. The rest can be follow-ups.

[AnnatarHe]

Second review addressed in 8443c0d:

Security

1. Imports and replies now need dmarc=pass whenever Resend reports a result. none, temperror, permerror and other are dropped, as fail already was: nothing is imported and nothing is answered. This also covers item 3 (spam sent through the JSON reply).
   • Mail with no reported result still goes through. If Resend turns out not to set Authentication-Results, imports keep working instead of all failing, and app.email_import.authentication{dmarc="absent"} shows how often that happens.
   • Trade-off, documented: a reader whose domain publishes no DMARC record can't import.
2. A forged sender spending another address's reply allowance: left as is and now documented. Counting before the fetch is what keeps mail from over-limit senders cheap, and the cost here is low (5 a day).
3. Account enumeration: adding an address that another reader holds no longer returns EMAIL_ALREADY_BOUND straight away. The code is mailed as usual, and the unique index refuses the address at verify time, to someone who has proved they own it. Tests cover both orders.

Performance and tests

• saveNotebook now reads the same-title clippings once. It links notes from those rows plus the newly inserted ones, instead of querying again.
• New tests:
  • dmarc=none and dmarc=temperror block both importing and replying, and pass imports.
  • A highlight repeated on another page is kept once.
• The const x = …, y = … nit is fixed.

Not changed

• Binding cap race: only the reader's own parallel requests can push past 10, and they're already held to 5 code sends an hour, so the most it can overshoot is a few addresses on their own account. Not worth a lock. mcp_tokens uses the same check-then-insert.
• claimReplyQuota atomicity: a job runs once per email id at worker concurrency 1. The read-then-bump window can let a burst go a few over a spam limit, or count once more if Redis fails mid-claim. A Lua script isn't worth that.
• Wenqu books[0]: this is the same first-hit rule the browser .txt import uses (findDoubanId). Changing how books are matched belongs in its own change that covers both paths.
• Resend idempotency key expiring after 24 h: retries back off from 30 s and stop after 5 attempts, well inside that window.
• isUniqueViolation as a shared helper: it has one caller, so it stays local until a second one appears.

---

Generated by Claude Code

[claude]

Review: Kindle notebook email import

I read the server code (bindings.ts, import.ts, the routes) and skimmed webhook.ts, reply.ts and the UI. I didn't run the tests. The structure is solid:

• Code verification counts the attempt before comparing, so parallel guesses can't share one.
• The comparison is constant-time.
• Rate limits cover both the reader and the target address.
• Attachment sizes are bounded.
• The unique index decides who owns a verified address.
• requestEmailBinding doesn't reveal which addresses other readers have bound.
• The retry and idempotency story is well thought out.

Security / correctness (worth addressing)

1. Mail with no DMARC result is imported (import.ts, the dmarc !== 'pass' && dmarc !== 'absent' check).

   • The sender's From address is the only credential for importing into a reader's account. If Resend reports no Authentication-Results, anyone who knows or guesses a bound address can forge it and write clippings into that account.
   • The comment says this is a deliberate trade-off. I'd still make "absent" fail closed by default, or put it behind an env flag. The metric will tell you how often it happens before you decide.
   • The unverified-sender path has the same weakness. A spoofed From makes us send a reply to a victim's address, which is backscatter, though the quotas cap it.

2. The INBOUND_EMAIL_ADDRESS recipient check is skipped when the variable is unset (import.ts). That is fail-open. I'd require the variable in production, or at least log a warning when it's missing.

3. The reply quota is claimed before the checks that make a reply worthwhile (import.ts).

   • claimReplyQuota runs before the DMARC check and before notebook detection.
   • Failed-DMARC and notebook-less mail from strangers therefore uses up the per-sender allowance (5 a day) and the global one (500 a day).
   • A spoofed sender can use up a victim's daily reply quota. A spam wave from many addresses can use up the global quota, so real unbound readers stop getting replies.
   • It's a trade-off against download cost. Consider a separate cheap pre-check, and claim the real quota only after the DMARC and notebook checks pass. The PR description says mail without a notebook "is never answered", which holds, but it still burns quota.

4. Brute-force window on the 6-digit code (bindings.ts).

   • Re-requesting resets codeAttempts to 0.
   • A reader can request 5 codes an hour, and each code allows 5 guesses. That is about 25 guesses an hour per reader against 10⁶ values.
   • It's only exploitable by someone who can receive mail at the target address, since the code is per-address. Because the same address is also what you'd claim for it, that's probably acceptable.
   • A cumulative attempt cap per binding, not per code, would remove the question.

Smaller points

• The routes parse the id from request.url with .split('/').at(-1) and .at(-2). That breaks on a trailing slash or a path change. Use the route params instead.
• saveNotebook isn't transactional. Retries are safe because of text-based dedupe, but a partial failure after the clippings insert will import with no notes the first time. The retry catches up, so this is only worth a comment.
• Deduping by sameText across the whole title silently merges two identical highlights on different pages. The code comment already says so. It's fine, but it's worth a line in the docs.
• console.warn and console.error run alongside OTel logs, so each event is logged twice when a collector is configured.
• maskEmail keeps the full domain plus the first letter. That's fine for tracing. The email.sender_domain attribute also carries the domain, so maskEmail doesn't add much privacy.

Tests

Coverage looks good: bindings, import, reply, webhook, parser and dialog. I'd add:

• A test for the "DMARC absent" and "INBOUND_EMAIL_ADDRESS unset" behaviour, so the fail-open choices are explicit.
• A test that a spoofed or failed-auth mail from a stranger does or doesn't consume reply quota, whichever you choose.
• A concurrent verify test for the unique-violation path (EMAIL_ALREADY_BOUND).

Style

It follows CLAUDE.md: the settings UI uses lake-ui, namespaces and locales are in place, and the schema comes with its drizzle migration. I'd confirm that en, zhCN and ko have matching keys, since the repo has a test for that. This PR adds no ja settings file, so check whether ja falls back correctly.

[AnnatarHe]

Third review addressed in 5caf273:

Security / correctness

1. Missing DMARC result now fails closed. Imports and replies need dmarc=pass.
   • Mail Resend reports no result for is refused, unless EMAIL_IMPORT_ALLOW_MISSING_DMARC=1 is set (default 0).
   • The docs tell whoever deploys this to check app.email_import.authentication{dmarc="absent"} before turning the switch on.
2. INBOUND_EMAIL_ADDRESS is now required. While it's unset, the webhook answers 503 and the worker drops mail, instead of importing anything sent to the receiving domain.
3. Reply quota is charged only when a reply goes out, after the DMARC and notebook checks. Forged or notebook-less mail spends neither the sender's allowance nor the global one.
   • Before fetching there's only a read-only check, which turns away a sender already at its limit.
   • A separate cap of 1,000 fetches a day bounds the work done for strangers' mail.
   • A spam wave can use up that fetch cap. Then strangers get no reply that day, but verified readers are unaffected.
4. Code brute force: not changed.
   • Each address gets at most 3 code sends an hour, and each code allows 5 tries, so about 15 guesses an hour against 10⁶ values. That's roughly 7 years for an even chance.
   • Every code send also lands in the real owner's inbox.
   • A per-binding cumulative cap is bypassed by removing and re-adding the address, so it would need a separate Redis counter. Not worth it at those odds.

Smaller points

• The binding routes now read id from the route params, as rss/user/[uid] does.
• I added a comment that saveNotebook isn't one transaction and why a retry recovers.
• The docs now say a highlight whose exact text recurs on another page is kept once.
• Logging to both OTel and the console: left as is on purpose. It's the repo's pattern (see api/auth/callback), so container logs stay useful without a collector.
• maskEmail plus email.sender_domain: left as is. The domain is the part that's useful for tracing; the mask is there to keep the mailbox name out of the logs.

Tests

• Added: the missing-DMARC switch, the unset inbound address (worker and webhook), forged and junk mail not spending the quota, the fetch cap, and the read-only check before a fetch.
• The EMAIL_ALREADY_BOUND unique-violation path is already covered by "keeps the address with whoever verifies it first".

Locales: the repo has no ja locale directory (src/locales/ holds only en, zhCN and ko), and the namespace test confirms those three have matching keys.

---

Generated by Claude Code

[claude]

Review: email import of Kindle notebook exports

Well-structured PR. I read webhook.ts, bindings.ts, import.ts, reply.ts and the bindings route. I did not run the tests or read the parser, UI, migration or locale files. The Svix signature check, body-size bound, deterministic job id, DMARC gating, hashed rate-limit keys, idempotent reply and bounded downloads are all good.

Security

1. DMARC is the only credential, and the Authentication-Results parsing is loose (import.ts, header() and the \bdmarc=(\w+) match).
   • header() takes the first matching key of a Record<string,string>. If the sender's own message carries an Authentication-Results: dmarc=pass header, I can't tell from the code which value ends up in email.headers.
   • The regex isn't anchored to Resend's authserv-id, so dmarc=pass could match inside a comment or another method's reason text, e.g. spf=fail (reason: dmarc=pass).
   • A forged pass would allow import into a verified reader's account and replies to arbitrary addresses.
   • Please confirm how Resend exposes duplicate headers, then restrict the match to Resend's authserv-id or the dmarc= method token at a method boundary. A test with a sender-supplied header would help.
2. EMAIL_IMPORT_ALLOW_MISSING_DMARC=1 disables the only sender check. The docs flag it; I'd also log a startup warning when it is on.
3. Code brute force (bindings.ts): re-adding a pending address resets codeAttempts. With 5 codes/hour per reader and 3/hour per address, that is about 25 guesses an hour per target against 10^6 codes. Acceptable.
4. Quota counters (reply.ts):
   • claimReplyQuota bumps three counters in sequence, so a later failure leaves earlier ones spent. Minor.
   • admitUnverifiedEmail spends a daily fetch on every attempt, retries included, because claimedKey is only set later. Spoofed mail can exhaust the 1000/day cap and stop unverified senders getting replies for the day. Verified senders are unaffected. Bounded, but worth a docs line.

Correctness

• saveNotebook dedupes by whitespace-stripped content within a title, so same-title books merge and identical highlights on different pages fold together. The code comment already accepts the second case.
• resolveBookId takes the first Wenqu hit blindly, so common titles can match the wrong book. Consider requiring a close title match.
• Retry after a crash between clippings and notes works, since idByText is rebuilt from existing.

Performance

saveNotebook loads all of the user's clippings with that title, which is fine at expected sizes. INSERT_CHUNK = 200 is well within bind limits.

Tests and style

Coverage across the webhook, bindings, import, reply and parser looks solid. I'd add: the forged Authentication-Results case from item 1, a retry of an already-claimed email not re-spending the quota, and parseEmailAddress on "a@victim.com" <evil@x.com>. Conventions from CLAUDE.md are followed.

Overall a solid approach. I'd resolve item 1 before merging; the rest are suggestions.

[AnnatarHe]

Fourth review addressed in 38d8c7b:

1. Authentication-Results parsing. A small RFC 8601 parser now replaces the regex (src/server/email-import/authentication.ts):

• it reads every header that differs only in letter case, so a second lowercase header can't shadow the receiver's;
• it drops comments (nested ones included) and quoted strings, then counts only a dmarc= that starts a result. spf=fail (reason: dmarc=pass) and reason="…; dmarc=pass" therefore count for nothing;
• if several results are found, whether repeated headers or values joined by newline or comma, any one that isn't a pass wins;
• the new EMAIL_IMPORT_AUTHSERV_ID setting ignores results stamped by any other server. The docs say how to read that value off a received email in the Resend dashboard.

Not fixable from here: if Resend's headers record kept only a sender-written header and dropped its own, no parsing could tell the two apart. Pinning the authserv-id covers that case as long as Resend strips sender headers that claim its own name, which RFC 8601 requires of receiving servers. The docs now spell this out.

Unit tests cover each forgery above. Import tests cover the pinned id rejecting a pass from another server.

2. EMAIL_IMPORT_ALLOW_MISSING_DMARC=1: every email let through only because of it now logs a WARN. I chose a per-email warning over a startup warning because it points at the emails actually affected.

3. Code brute force: agreed it's acceptable, unchanged.

4. Quota counters:

• A retried job no longer spends a second fetch from the daily cap; each admitted email gets a marker.
• That a spam wave can exhaust the 1,000-a-day fetch cap is already in the docs' limits list.
• The three counters still update one after another, as you note. It's minor, so unchanged.

Correctness

• Wenqu match: the importer now takes the first search hit whose title contains the notebook's title, or the other way round, ignoring case, spacing and punctuation. It needs at least two characters to match, so a one-character title can't match everything. If nothing matches, the clippings stay unmatched, which is fixable from the book page, instead of being filed under whatever ranked first.
• Same-title dedup: a test now pins the one-copy behaviour, and the docs mention it.

Tests added:

• the forged-header cases;
• the authserv-id pin;
• the missing-DMARC warning;
• a retry not spending the fetch cap again;
• parseEmailAddress('"reader@victim.example" <evil@example.com>') → evil@example.com.

A retry of an already-claimed reply not re-spending quota was already covered.

---

Generated by Claude Code

[claude]

Review: email import of Kindle notebook exports

I read the webhook, authentication.ts, bindings.ts and import.ts. I did not run the code, and I did not read the UI components, reply.ts, kindle-notebook.ts or the tests in detail. The design is careful. The flow is signature, then queue, then DMARC, then binding, and quota is charged only on an actual reply. Idempotent job ids, bounded bodies and attachments, and the verify-time EMAIL_ALREADY_BOUND (so adding can't be used to probe bindings) are all good choices.

Security

1. Fail-open default for sender authentication (most important).

   • The From address is the only credential. Whether it is trusted rests on Authentication-Results headers that arrive through Resend's email.headers.
   • With EMAIL_IMPORT_AUTHSERV_ID unset (the default in .env.example), dmarcResult (authentication.ts:47-60) accepts a dmarc=pass from any Authentication-Results header.
   • If Resend passes through a sender-written header, an attacker who knows a reader's bound address can send mail with their own Authentication-Results: x; dmarc=pass. That would inject highlights into the reader's library.
   • results.find(r => r !== 'pass') only helps when Resend's own header is also present and failing.
   • The docs say to set the authserv-id after deploying, but nothing enforces it. Suggest failing closed: when EMAIL_IMPORT_AUTHSERV_ID is empty, treat every result as absent, or refuse to import and log loudly. An explicit opt-out env could cover the "I accept the risk" case.

2. Per-address rate limit can be used to block someone's verification.

   • In requestEmailBinding, rateLimit(...send:a:<sha256(email)>, 3, HOUR) is charged before anything else and is global per target address.
   • A signed-in user can repeatedly "add" a victim's address, or another reader's address, and use up its 3 per hour. The real owner then gets TOO_MANY_CODES. Impact is limited to a temporary nuisance, but it is cheap to do.
   • Consider charging the per-address limit only after the pending-row insert succeeds. Also consider keying it by (user, address) in addition to the global cap.

3. Attempt counter resets on re-send.

   • onConflictDoUpdate sets codeAttempts: 0, so a reader gets 5 guesses per code and up to 5 codes per hour. That's about 25 guesses per hour against a 10^6 space within the 30-minute expiry. It's acceptable, but worth noting in the docs next to the "expiry and attempt cap protect it" statement.

Correctness and robustness

• saveNotebook dedupes by sameText only. A recurring identical highlight on a different page is dropped; the PR documents this trade-off. The same idByText map links notes, so a note after a repeated highlight can attach to the wrong clipping.
• markEmailImported runs after the saves. A retry after a crash there is safe (it is idempotent), but lastImportedAt can stay stale.
• resolveBookId runs once per notebook, and a Wenqu failure silently becomes '0'. That is fine, but a debug log for the timeout case would help when matches look wrong.
• notebookSources fetches attachment.download_url without checking its host. It comes from the Resend API, so the risk is low. A pinned https/host check would be cheap defence in depth.
• The queue is shared with exports at concurrency 1 (acknowledged in the docs). Please keep an eye on delays for import jobs.

Style and conventions

• Matches CLAUDE.md for the server layout, ApiError and the shared insertClippings. CLAUDE.md is updated.
• src/server/email-import/import.ts is about 445 lines mixing logging helpers, matching and persistence. Consider splitting resolveBookId and saveNotebook into their own module.
• The new settings.json keys are in en, ko and zhCN. Please confirm ja is covered, since CLAUDE.md lists it as supported. The i18n key-parity test should catch this.

Tests

Coverage is strong: bindings (PGlite), DMARC variants, the webhook signature and replay, the importer, quotas and the dialog. Gaps:

• No test that a sender-supplied Authentication-Results header is ignored when only an unpinned authserv-id is configured (the scenario in point 1).
• No test for the per-address rate-limit abuse in point 2.
• The Resend fetch and send paths were not exercised end to end (the sandbox blocks resend.com), so please do a manual run on staging before enabling.

Overall this is a solid feature. I'd treat point 1 as blocking, or at least as a decision to record explicitly, and the rest as follow-ups.