-
Notifications
You must be signed in to change notification settings - Fork 0
feat(dev): make the dev harness able to actually deliver a webhook #183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,9 +40,12 @@ | |
| import type { AuthProvider } from '../src/auth/provider.js' | ||
| import { createPgliteDb } from '../src/db/client.js' | ||
| import { migrate } from '../src/db/migrate.js' | ||
| import { createDevBlobStore } from '../src/dev/dev-blob-store.js' | ||
| import { createDevEmailSender } from '../src/dev/dev-sender.js' | ||
| import { createHttpBridge } from '../src/dev/http-adapter.js' | ||
| import { injectInboundMessage } from '../src/dev/inject-inbound.js' | ||
| import { seedDevData } from '../src/dev/seed.js' | ||
| import { startWebhookWorker } from '../src/dev/webhook-worker.js' | ||
| import { createImapConnectService } from '../src/mail/imap-connect.js' | ||
| import type { Keyring } from '../src/mail/reply-token.js' | ||
| import type { SenderResolver } from '../src/mail/sender-resolver.js' | ||
|
|
@@ -52,9 +55,11 @@ | |
| import { createAgentStore } from '../src/store/agents.js' | ||
| import { createAssistantStore } from '../src/store/assistants.js' | ||
| import { createConversationStore } from '../src/store/conversations.js' | ||
| import { createEventOutboxStore } from '../src/store/event-outbox.js' | ||
| import { createImapConfigStore } from '../src/store/imap-config.js' | ||
| import { createImapCredentialStore } from '../src/store/imap-credentials.js' | ||
| import { createImapWatchStateStore } from '../src/store/imap-watch-state.js' | ||
| import { createInboundDeliveryStore } from '../src/store/inbound-deliveries.js' | ||
| import { createMailboxStore } from '../src/store/mailboxes.js' | ||
| import { createSavedReplyStore } from '../src/store/saved-replies.js' | ||
| import { createWebhookEndpointStore } from '../src/store/webhook-endpoints.js' | ||
|
|
@@ -136,6 +141,18 @@ | |
| }, | ||
| } | ||
|
|
||
| // --- webhook delivery (HT-69) ------------------------------------------- | ||
| // Production runs two passes on a schedule to move a webhook out of the | ||
| // engine: the outbox drain fans each committed event to its endpoints, | ||
| // and the queue drain signs and POSTs them. Without both, the engine | ||
| // writes to `event_outbox` and stops there β registering a webhook | ||
| // appears to work and nothing is ever delivered. See | ||
| // `src/dev/webhook-worker.ts`. Built here, above `createInboxApi`, so the | ||
| // API and the worker share one store and one queue: a webhook registered | ||
| // through the API is one the worker can see. | ||
| const webhookEndpointStore = createWebhookEndpointStore(db, DEV_TOKEN_ENC_KEY) | ||
| const queue = createPostgresQueue(db) | ||
|
|
||
| // `assistants`, `webhooks`, and `savedReplies` are REQUIRED on | ||
| // `InboxApiDeps`, and this file sits outside `tsconfig.json`'s `include` | ||
| // (only `scripts/migrate.ts` is listed), so `npm run typecheck` never | ||
|
|
@@ -154,9 +171,11 @@ | |
| assistants: { store: createAssistantStore(db) }, | ||
| webhooks: { | ||
| // Same throwaway dev key as the IMAP credential store above β webhook | ||
| // secrets are encrypted at rest by the same AES-256-GCM path. | ||
| store: createWebhookEndpointStore(db, DEV_TOKEN_ENC_KEY), | ||
| queue: createPostgresQueue(db), | ||
| // secrets are encrypted at rest by the same AES-256-GCM path. The | ||
| // SAME store and queue instances the delivery worker below drains, | ||
| // so a webhook registered through the API is one the worker sees. | ||
| store: webhookEndpointStore, | ||
| queue, | ||
| }, | ||
| savedReplies: { store: createSavedReplyStore(db), mailboxStore }, | ||
| imapConnect: { | ||
|
|
@@ -166,8 +185,68 @@ | |
| }, | ||
| }) | ||
|
|
||
| const webhookWorker = startWebhookWorker( | ||
| { | ||
| eventOutbox: createEventOutboxStore(db), | ||
| webhookEndpoints: webhookEndpointStore, | ||
| queue, | ||
| }, | ||
| { | ||
| onActivity: ({ dispatched, delivered, failed }) => { | ||
| console.log(`[webhooks] dispatched ${dispatched}, delivered ${delivered}, failed ${failed}`) | ||
| }, | ||
| }, | ||
| ) | ||
|
|
||
| // --- dev-only inbound injection ----------------------------------------- | ||
| // `POST /__dev/inbound` with { mailboxId, from, to, subject, text } drives | ||
| // the REAL ingest pipeline, so a local run can produce a conversation and | ||
| // the `conversation.message_received` event that follows it. Deliberately | ||
| // namespaced under `/__dev/` and handled before the bridge, so it is | ||
| // visibly not part of the engine's API surface. | ||
| const ingestDeps = { | ||
| db, | ||
| inboundDeliveryStore: createInboundDeliveryStore(db), | ||
| blobStore: createDevBlobStore(), | ||
| keyring: KEYRING, | ||
| } | ||
|
|
||
| const baseUrl = `http://127.0.0.1:${PORT}` | ||
| const server = createServer(createHttpBridge(api, baseUrl)) | ||
| const apiBridge = createHttpBridge(api, baseUrl) | ||
| const server = createServer((req, res) => { | ||
| if (req.url === '/__dev/inbound' && req.method === 'POST') { | ||
| // Same Bearer gate every other route in this harness enforces. It is | ||
| // a loopback-only server, so this is not a production exposure β but | ||
| // "every request carries the token" should not have an exception | ||
| // carved into it by the one route that fabricates customer mail. | ||
| if (req.headers.authorization !== `Bearer ${API_TOKEN}`) { | ||
| res.statusCode = 401 | ||
| res.setHeader('Content-Type', 'application/json') | ||
| res.end(JSON.stringify({ error: 'missing or invalid bearer token' })) | ||
| return | ||
| } | ||
| void (async () => { | ||
| try { | ||
| const chunks: Buffer[] = [] | ||
| for await (const chunk of req) chunks.push(Buffer.from(chunk)) | ||
| const body = JSON.parse(Buffer.concat(chunks).toString('utf8')) as Parameters< | ||
| typeof injectInboundMessage | ||
| >[0] | ||
| const outcome = await injectInboundMessage(body, ingestDeps) | ||
| res.statusCode = 200 | ||
| res.setHeader('Content-Type', 'application/json') | ||
| res.end(JSON.stringify(outcome)) | ||
| } catch (err) { | ||
| console.error('[dev-api] inbound injection failed', err) | ||
| res.statusCode = 500 | ||
| res.setHeader('Content-Type', 'application/json') | ||
| res.end(JSON.stringify({ error: err instanceof Error ? err.message : String(err) })) | ||
|
|
||
| } | ||
| })() | ||
| return | ||
| } | ||
| apiBridge(req, res) | ||
| }) | ||
| // Bind explicitly to loopback β this dev harness must never listen on the | ||
| // LAN (the default token is public knowledge, right there in this file). | ||
| await new Promise<void>((resolve) => { | ||
|
|
@@ -201,6 +280,9 @@ | |
| await new Promise<void>((resolve, reject) => { | ||
| server.close((err) => (err ? reject(err) : resolve())) | ||
| }) | ||
| // Stop the delivery loop and let any pass in flight finish, so nothing | ||
| // is mid-query against a database that is about to close. | ||
| await webhookWorker.stop() | ||
|
Comment on lines
+283
to
+285
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π©Ί Stability & Availability | π‘ Minor | β‘ Quick win Guard
π‘οΈ Proposed guard+ let shuttingDown = false
const shutdown = async (): Promise<void> => {
+ if (shuttingDown) return
+ shuttingDown = true
console.log('\n[dev-api] shutting down...')π€ Prompt for AI Agents |
||
| await db.close() | ||
| process.exit(0) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /** | ||
| * A dev-only, in-memory `BlobStore` (`src/providers/blob.ts`) β attachment | ||
| * bytes live in a `Map` for the life of the process and are gone on | ||
| * restart. Mirrors `dev-sender.ts`'s role for `EmailSender` and | ||
| * `dev-inbound-email.ts`'s for `InboundEmailProvider`: a real | ||
| * implementation of the interface with no external dependency, so the | ||
| * local harness can run the ingest pipeline without Supabase Storage | ||
| * credentials. | ||
| * | ||
| * `getSignedUrl` returns a `dev-blob:` URL that nothing can actually | ||
| * fetch. Nothing in the harness serves attachments, and a URL that | ||
| * obviously is not a URL beats one that looks real and 404s. | ||
| */ | ||
|
|
||
| import type { BlobStore } from '../providers/blob.js' | ||
|
|
||
| export function createDevBlobStore(): BlobStore { | ||
| const objects = new Map<string, Uint8Array>() | ||
|
|
||
| return { | ||
| async put(key, data) { | ||
| objects.set(key, data) | ||
| }, | ||
| async get(key) { | ||
| const found = objects.get(key) | ||
| if (found === undefined) { | ||
| throw new Error(`dev blob store: no object at ${key}`) | ||
| } | ||
| return found | ||
| }, | ||
| async getSignedUrl(key, expiresInSeconds) { | ||
| return `dev-blob:${encodeURIComponent(key)}?expires_in=${expiresInSeconds}` | ||
| }, | ||
| async delete(key) { | ||
| objects.delete(key) | ||
| }, | ||
| async exists(key) { | ||
| return objects.has(key) | ||
| }, | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /** | ||
| * `buildRawMessage` / `injectInboundMessage` (src/dev/inject-inbound.ts). | ||
| * | ||
| * Two properties are worth holding even in dev tooling: a header value | ||
| * cannot rewrite the message around it, and the transport message id is | ||
| * the caller's to control when they want to replay a delivery. | ||
| */ | ||
|
|
||
| import { describe, expect, it } from 'vitest' | ||
| import { buildRawMessage } from './inject-inbound.js' | ||
|
|
||
| const BASE = { | ||
| mailboxId: 'mailbox-1', | ||
| from: 'customer@example.test', | ||
| to: 'support@example.test', | ||
| subject: 'Hello', | ||
| text: 'Body text.', | ||
| } | ||
|
|
||
| describe('buildRawMessage', () => { | ||
| it('separates headers from the body with exactly one blank line', () => { | ||
| const raw = buildRawMessage(BASE, 'id-1@example.test') | ||
| const [headers, ...rest] = raw.split('\r\n\r\n') | ||
|
|
||
| expect(headers).toContain('From: customer@example.test') | ||
| expect(headers).toContain('Subject: Hello') | ||
| expect(rest.join('\r\n\r\n')).toBe('Body text.\r\n') | ||
| }) | ||
|
|
||
| it('threads a reply with In-Reply-To and References', () => { | ||
| const raw = buildRawMessage({ ...BASE, inReplyTo: 'parent@example.test' }, 'id-2@example.test') | ||
|
|
||
| expect(raw).toContain('In-Reply-To: <parent@example.test>') | ||
| expect(raw).toContain('References: <parent@example.test>') | ||
| }) | ||
|
|
||
| // A bare CR or LF ends a header; two end the header block. Interpolating | ||
| // one unescaped lets a "subject" append its own headers, or terminate the | ||
| // block early and turn the real headers into body text. | ||
| it.each([ | ||
| ['subject', { subject: 'Hi\r\nBcc: attacker@example.test' }], | ||
| ['from', { from: 'a@example.test\r\nX-Injected: yes' }], | ||
| ['to', { to: 'b@example.test\nX-Injected: yes' }], | ||
| ['inReplyTo', { inReplyTo: 'p@example.test>\r\nX-Injected: yes' }], | ||
| ])('refuses a line break in %s', (_name, override) => { | ||
| expect(() => buildRawMessage({ ...BASE, ...override }, 'id@example.test')).toThrow( | ||
| /must not contain a line break/, | ||
| ) | ||
| }) | ||
|
|
||
| it('refuses a line break in the generated message id', () => { | ||
| expect(() => buildRawMessage(BASE, 'id\r\nX-Injected: yes')).toThrow( | ||
| /must not contain a line break/, | ||
| ) | ||
| }) | ||
|
|
||
| it('leaves line breaks in the body alone β they are not header injection', () => { | ||
| const raw = buildRawMessage({ ...BASE, text: 'line one\r\nline two' }, 'id@example.test') | ||
|
|
||
| expect(raw.endsWith('line one\r\nline two\r\n')).toBe(true) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| /** | ||
| * Push a synthetic inbound email through the REAL ingest pipeline | ||
| * (`src/mail/ingest.ts`) from the local harness β the missing half of a | ||
| * local end-to-end run. | ||
| * | ||
| * `dev-inbound-email.ts` fakes the provider *interface*, which suits tests | ||
| * that drive `InboundEmailProvider` consumers. It does not help someone who | ||
| * wants a conversation to appear and a `conversation.message_received` | ||
| * event to fire, because nothing in the harness was pumping that provider. | ||
| * This goes the other way: build RFC822 bytes, hand them to | ||
| * `ingestInboundMessage`, and let every downstream consequence β threading, | ||
| * dedup, the outbox write β happen exactly as it does in production. | ||
| * | ||
| * Dev-only by construction: it fabricates a `providerMessageId` and treats | ||
| * the caller's word as the transport's, both of which a real provider | ||
| * would supply and neither of which anything should trust outside a | ||
| * local harness. | ||
| */ | ||
|
|
||
| import { randomUUID } from 'node:crypto' | ||
| import { type IngestDeps, type IngestOutcome, ingestInboundMessage } from '../mail/ingest.js' | ||
|
|
||
| export interface InjectInboundOptions { | ||
| /** Which connected mailbox the message arrives at. */ | ||
| mailboxId: string | ||
| /** Envelope sender β the "customer" writing in. */ | ||
| from: string | ||
| /** Envelope recipient; normally the mailbox's own support address. */ | ||
| to: string | ||
| subject: string | ||
| /** Plain-text body. */ | ||
| text: string | ||
| /** | ||
| * The transport's own message id β ingest's idempotency authority | ||
| * (inbound-ingestion.md Β§4). Omitted, a fresh one is generated, so each | ||
| * call is a distinct delivery. Supply the SAME value twice to replay one | ||
| * delivery and exercise dedup, which is otherwise unreachable from this | ||
| * harness. | ||
| */ | ||
| providerMessageId?: string | ||
| /** | ||
| * `In-Reply-To` for a reply into an existing thread. Omitted for a fresh | ||
| * message, which is what makes the difference between | ||
| * `conversation.created` + `message_received` and a bare | ||
| * `message_received`. | ||
| */ | ||
| inReplyTo?: string | ||
| } | ||
|
|
||
| /** | ||
| * Reject a header value carrying a line break. A bare CR or LF ends the | ||
| * header β and two end the header block β so an unescaped newline in a | ||
| * subject or address silently rewrites the rest of the message, including | ||
| * the body boundary. Dev-only input is still input. | ||
| */ | ||
| function headerValue(name: string, value: string): string { | ||
| if (/[\r\n]/.test(value)) { | ||
| throw new Error(`inject-inbound: ${name} must not contain a line break`) | ||
| } | ||
| return value | ||
| } | ||
|
|
||
| /** Build the RFC822 bytes for {@link injectInboundMessage}. Exported for tests that want to assert on the wire format rather than the outcome. */ | ||
| export function buildRawMessage(options: InjectInboundOptions, messageId: string): string { | ||
| const headers = [ | ||
| `From: ${headerValue('from', options.from)}`, | ||
| `To: ${headerValue('to', options.to)}`, | ||
| `Subject: ${headerValue('subject', options.subject)}`, | ||
| `Message-ID: <${headerValue('messageId', messageId)}>`, | ||
| `Date: ${new Date().toUTCString()}`, | ||
| 'MIME-Version: 1.0', | ||
| 'Content-Type: text/plain; charset=utf-8', | ||
| ] | ||
| if (options.inReplyTo !== undefined) { | ||
| const inReplyTo = headerValue('inReplyTo', options.inReplyTo) | ||
| headers.push(`In-Reply-To: <${inReplyTo}>`) | ||
| headers.push(`References: <${inReplyTo}>`) | ||
| } | ||
| return `${headers.join('\r\n')}\r\n\r\n${options.text}\r\n` | ||
|
Comment on lines
+64
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π― Functional Correctness | π‘ Minor | β‘ Quick win Reject CR/LF in header values.
π‘οΈ Proposed guard+function headerValue(name: string, value: string): string {
+ if (/[\r\n]/.test(value)) {
+ throw new Error(`inject-inbound: ${name} must not contain CR or LF`)
+ }
+ return value
+}
+
export function buildRawMessage(options: InjectInboundOptions, messageId: string): string {
const headers = [
- `From: ${options.from}`,
- `To: ${options.to}`,
- `Subject: ${options.subject}`,
+ `From: ${headerValue('from', options.from)}`,
+ `To: ${headerValue('to', options.to)}`,
+ `Subject: ${headerValue('subject', options.subject)}`,
`Message-ID: <${messageId}>`,π€ Prompt for AI Agents |
||
| } | ||
|
|
||
| /** | ||
| * Ingest one synthetic message. Returns the pipeline's own outcome, so a | ||
| * caller can see which conversation it landed on and whether it threaded | ||
| * or created. | ||
| */ | ||
| export async function injectInboundMessage( | ||
| options: InjectInboundOptions, | ||
| deps: IngestDeps, | ||
| ): Promise<IngestOutcome> { | ||
| const messageId = `dev-${randomUUID()}@dev.localhost` | ||
| const raw = buildRawMessage(options, messageId) | ||
|
|
||
| return ingestInboundMessage( | ||
| { | ||
| content: { kind: 'inline', bytes: new TextEncoder().encode(raw) }, | ||
| mailboxId: options.mailboxId, | ||
| // A real provider's own id. Generated per call by default, so two | ||
| // injections of identical text are two deliveries β but caller- | ||
| // supplied when replaying, because regenerating it unconditionally | ||
| // made a retried POST silently create a second conversation and a | ||
| // second webhook, and made the harness structurally unable to | ||
| // exercise dedup at all. | ||
| providerMessageId: options.providerMessageId ?? `dev-${randomUUID()}`, | ||
| receivedAt: new Date(), | ||
| // Nothing classified this message; 'unknown' is the honest answer and | ||
| // keeps a synthetic message out of the spam status. | ||
| providerSpamVerdict: 'unknown', | ||
| }, | ||
| deps, | ||
| ) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π Security & Privacy | π‘ Minor | β‘ Quick win
/__dev/inboundaccepts a request with no caller check and no body check. The route is handled beforeapiBridge, so it inherits none of the API's input handling. Both findings share that root cause: the handler trusts the caller and trusts the parsed JSON.scripts/dev-api.ts#L217-L218: require theAPI_TOKENbearer header before reading the body, and return HTTP 401 when it is absent or wrong. Loopback binding does not stop a cross-origintext/plainform POST from a page in the operator's browser.scripts/dev-api.ts#L220-L225: replace theas Parameters<typeof injectInboundMessage>[0]cast with a runtime shape check onmailboxId,from,to,subject, andtext, and return HTTP 400 for a bad body.π Affects 1 file
scripts/dev-api.ts#L217-L218(this comment)scripts/dev-api.ts#L220-L225π€ Prompt for AI Agents