feat(inbound): Gmail push webhook receiver (HT-39) - #39
Conversation
POST /api/v1/inbound/gmail — a pre-auth, OIDC-JWT-verified surface that verifies the push, resolves the mailbox, and enqueues a reconcile job onto the QueueProvider (no inline Gmail fetch; that's HT-41). push-auth.ts (adapters/gmail): Google OIDC JWT verification via jose (createRemoteJWKSet + jwtVerify) — iss/aud/email/email_verified/exp, fails closed, JWKS-cached, kept out of src/api per the provider-boundary rule. gmail-webhook.ts: uniform 403 for every failed check (no oracle; unconfigured == rejected), streaming body-size cap, JWT-before-body-read, active-mailbox resolution, enqueue with a best-effort dedupe key. mailboxes.ts: MailboxStore.getMailboxByAddress. Pre-auth branch wired into createInboxApi before Bearer auth (mirrors the tracking pixel) + matchGmailPushWebhook. Added jose (^6, MIT, zero transitive deps). Implements specs/mail/gmail-push.md §2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughChangesAdds a Gmail Pub/Sub push webhook endpoint with Google OIDC JWT verification, strict request and payload validation, active mailbox lookup, reconciliation job enqueueing, uniform rejection responses, and pre-auth API routing. Tests cover authentication, validation, routing, and error handling. Gmail push webhook
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PubSub
participant InboxApi
participant GmailPushVerifier
participant MailboxStore
participant QueueProvider
PubSub->>InboxApi: POST push envelope
InboxApi->>GmailPushVerifier: verify request JWT
GmailPushVerifier-->>InboxApi: verified result
InboxApi->>MailboxStore: getMailboxByAddress
MailboxStore-->>InboxApi: active mailbox
InboxApi->>QueueProvider: enqueue gmail-reconcile job
QueueProvider-->>InboxApi: enqueue result
InboxApi-->>PubSub: 200 OK
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ook-receiver # Conflicts: # src/store/index.ts # src/store/mailboxes.test.ts # src/store/mailboxes.ts
f1584f2 to
0a0be11
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/gmail-webhook.test.ts`:
- Around line 39-45: Update the fakeMailboxes test helper to implement the
required MailboxStore.markNeedsReconnect method alongside getMailboxByAddress,
preserving the existing mailbox lookup behavior. Also inspect the related
partial mock in the pre-auth API routing tests and add the same required method
wherever it is missing.
In `@src/api/index.test.ts`:
- Around line 1552-1560: Update fakeMailboxes in the test file to satisfy the
MailboxStore type by importing MailboxStore as a type and annotating the
helper’s returned object accordingly. Implement the required markNeedsReconnect
method alongside getMailboxByAddress, preserving the existing mailbox lookup
behavior and allowing all GmailPushDeps call sites to typecheck.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e5128f0c-8990-4942-81fc-4b901f8c31c7
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
package.jsonsrc/api/gmail-webhook.test.tssrc/api/gmail-webhook.tssrc/api/index.test.tssrc/api/index.tssrc/api/router.test.tssrc/api/router.tssrc/providers/adapters/gmail/index.tssrc/providers/adapters/gmail/push-auth.test.tssrc/providers/adapters/gmail/push-auth.tssrc/store/index.tssrc/store/mailboxes.test.tssrc/store/mailboxes.ts
| function fakeMailboxes( | ||
| record: { id: string; address: string; provider: string; status: 'active' } | null, | ||
| ) { | ||
| return { | ||
| async getMailboxByAddress(address: string) { | ||
| return record !== null && record.address === address ? record : null | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
fakeMailboxes doesn't satisfy MailboxStore — breaks typecheck at every call site.
GmailPushDeps.mailboxes requires a full MailboxStore (including markNeedsReconnect), but this helper only implements getMailboxByAddress. Static analysis confirms tsc failures at every usage: Line 1598, Line 1620, Line 1700, and Line 1720 (Property 'markNeedsReconnect' is missing ... required in type 'MailboxStore'). This would fail the typecheck CI check despite the PR's summary claiming a clean typecheck run.
🛠️ Proposed fix
/** A `MailboxStore` fake for wiring tests that never need real persistence — always resolves to `record` (or `null`). */
function fakeMailboxes(
record: { id: string; address: string; provider: string; status: 'active' } | null,
- ) {
+ ): MailboxStore {
return {
async getMailboxByAddress(address: string) {
return record !== null && record.address === address ? record : null
},
+ async markNeedsReconnect(_mailboxId: string) {
+ throw new Error('fakeMailboxes: markNeedsReconnect not implemented')
+ },
}
}Also add the type import so the annotation resolves:
import type { MailboxStore } from '../store/mailboxes.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function fakeMailboxes( | |
| record: { id: string; address: string; provider: string; status: 'active' } | null, | |
| ) { | |
| return { | |
| async getMailboxByAddress(address: string) { | |
| return record !== null && record.address === address ? record : null | |
| }, | |
| } | |
| } | |
| function fakeMailboxes( | |
| record: { id: string; address: string; provider: string; status: 'active' } | null, | |
| ): MailboxStore { | |
| return { | |
| async getMailboxByAddress(address: string) { | |
| return record !== null && record.address === address ? record : null | |
| }, | |
| async markNeedsReconnect(_mailboxId: string) { | |
| throw new Error('fakeMailboxes: markNeedsReconnect not implemented') | |
| }, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/index.test.ts` around lines 1552 - 1560, Update fakeMailboxes in the
test file to satisfy the MailboxStore type by importing MailboxStore as a type
and annotating the helper’s returned object accordingly. Implement the required
markNeedsReconnect method alongside getMailboxByAddress, preserving the existing
mailbox lookup behavior and allowing all GmailPushDeps call sites to typecheck.
Source: Linters/SAST tools
Implements HT-39 [G] — the Gmail push webhook receiver, under the HT-33 epic. This is the authenticated front door: Google Pub/Sub POSTs a push notification here, we prove it's really Google, resolve it to a connected mailbox, and hand a reconcile job to the queue for the history sync (HT-41) to drain.
What's here
src/providers/adapters/gmail/push-auth.ts—verifyGmailPushJwt: OIDC JWT verification viajose. Checksiss(accounts.google.com),aud(our push endpoint),email(the configured push service account),email_verified === true, andexp— fails closed on anything missing or wrong.createGooglePushKeySource(Google's JWKS, cached byjose) andcreateGmailPushSignatureVerifierwire it to theInboundEmailProvider.verifySignatureseam.src/api/gmail-webhook.ts—handleGmailPushWebhook: verify signature → parse the Pub/Sub envelope → resolveemailAddressto an active mailbox (MailboxStore.getMailboxByAddress) → enqueue aGmailReconcileJob({ mailboxId, historyId }) toGMAIL_RECONCILE_TOPICwith a${mailbox.id}:${historyId}dedupe key. AMAX_BODY_BYTES(64 KiB) streaming cap rejects oversized bodies before buffering.src/api/router.ts,index.ts) — mounts the webhook with the push config threaded throughdeps.gmailPush.Security posture
gmailPushRejected) for every auth-class outcome — bad/missing JWT, wrong issuer/audience/email,email_verified=false, unknown mailbox, non-activemailbox, and even "push not configured." Identical body every time, so a probing attacker gets no oracle distinguishing "wrong signature" from "unknown mailbox."status; the handler applies the "must beactive" policy — the storage-layer/policy split the module docs call out.Merge note
Branched before HT-37/38 landed; I merged
origin/mainin and combined the twoMailboxStorevariants into one interface carrying bothgetMailboxByAddress(this ticket) andmarkNeedsReconnect(HT-38) — singlecreateMailboxStore, both test suites preserved. Conflicts weresrc/store/{index,mailboxes,mailboxes.test}.tsonly.Verification
typecheck+biomeclean; full suite 535 tests pass locally (single worktree, no concurrent-run contention this time).process.envhere:GmailPushJwtConfig=endpointUrl(matched to the JWTaud) +serviceAccountEmail(matched toemail). Binding those to real env vars is the composition root's job (HT-43).Scope: the receiver + auth only — no
history.listfetch/ingest (HT-41), nowatch()lifecycle (HT-42). The enqueued reconcile job is the hand-off boundary.🤖 Generated with Claude Code
Summary by CodeRabbit
/api/v1/inbound/gmail.