feat: decouple read FeedId from inbound MailboxId
Separate the two feed identities so the public read URL never reveals the inbound address and vice-versa: - FeedId becomes an opaque high-entropy token (read id + KV key); MailboxId (noun.noun.NN) owns the inbound address and the untrusted-input boundary via MailboxId.parse. They map only through the inbound:<mailbox> secondary index, resolved solely at reception. - inbound index lifecycle is owned by FeedRepository: written by save/saveConfig, dropped by removeFromList(Bulk) — symmetric, never mirrored by hand (removes the manual delete in feed-service + the cron loop, and a silent empty-catch). - Feed.mailboxId exposes a MailboxId VO (symmetry with Feed.id); the mailbox@domain shape lives on MailboxId.emailAddress(domain). - Distinguish mailbox_unknown (no feed claims the address) from feed_not_found (dangling index) for observability; both forwardable, both 404. - Drop the redundant EmailParser.extractMailbox pass-through so MailboxId.parse is the single parse boundary. Docs (README/INSTALL/CLAUDE.md/landing) and tests updated; 439 tests green, tsc clean, build dry-run OK. Co-Authored-By: Claude Opus 4.7 <[email protected]>
This commit is contained in:
1 parent
f7f10779bc
commit
1a4a479190
43 files changed
+649
-149
No files matched your search
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { http, HttpResponse } from "msw";
|
||||
import { createMockEnv, MockR2, server } from "../test/setup";
|
||||
import { createMockEnv, MockR2, seedInboundIndex, server } from "../test/setup";
|
||||
import {
|
||||
processEmail,
|
||||
ProcessEmailInput,
|
||||
@@ -30,8 +30,10 @@ function makeInput(
|
||||
describe("processEmail", () => {
|
||||
let env: ReturnType<typeof createMockEnv>;
|
||||
|
||||
beforeEach(() => {
|
||||
beforeEach(async () => {
|
||||
env = createMockEnv();
|
||||
// The inbound address resolves to a feed of the same id in these unit tests.
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
});
|
||||
|
||||
it("returns 400 when toAddress has no valid feedId", async () => {
|
||||
@@ -42,7 +44,18 @@ describe("processEmail", () => {
|
||||
expect(res).toMatchObject({ ok: false, reason: "invalid_address" });
|
||||
});
|
||||
|
||||
it("returns 404 when feed does not exist", async () => {
|
||||
it("returns mailbox_unknown when no feed claims the inbound address", async () => {
|
||||
// A well-formed mailbox (noun.noun.NN) that was never registered in the
|
||||
// inbound index — distinct from a dangling index pointing at a missing feed.
|
||||
const res = await processEmail(
|
||||
makeInput({ toAddress: "[email protected]" }),
|
||||
env as any,
|
||||
);
|
||||
expect(res).toMatchObject({ ok: false, reason: "mailbox_unknown" });
|
||||
});
|
||||
|
||||
it("returns feed_not_found when the index resolves but the feed is gone", async () => {
|
||||
// The inbound index is seeded (beforeEach) but no config exists for it.
|
||||
const res = await processEmail(makeInput(), env as any);
|
||||
expect(res).toMatchObject({ ok: false, reason: "feed_not_found" });
|
||||
});
|
||||
@@ -319,14 +332,14 @@ describe("processEmail", () => {
|
||||
passThroughOnException: () => {},
|
||||
} as unknown as ExecutionContext;
|
||||
|
||||
// Feed ID is valid format but config doesn't exist → 404
|
||||
// Well-formed mailbox but not registered → mailbox_unknown (an error path).
|
||||
const res = await processEmail(
|
||||
makeInput({ toAddress: `[email protected]` }),
|
||||
env as any,
|
||||
ctx,
|
||||
);
|
||||
|
||||
expect(res).toMatchObject({ ok: false, reason: "feed_not_found" });
|
||||
expect(res).toMatchObject({ ok: false, reason: "mailbox_unknown" });
|
||||
expect(waitUntilCalled).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -343,6 +356,7 @@ describe("processEmail — attachments", () => {
|
||||
|
||||
it("skips R2 upload when ATTACHMENT_BUCKET is not configured", async () => {
|
||||
const env = createMockEnv();
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
await env.EMAIL_STORAGE.put(
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
JSON.stringify({}),
|
||||
@@ -366,6 +380,7 @@ describe("processEmail — attachments", () => {
|
||||
|
||||
it("skips R2 upload when ATTACHMENTS_ENABLED is 'false' even with R2 bound", async () => {
|
||||
const env = createMockEnv({ withR2: true });
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
(env as any).ATTACHMENTS_ENABLED = "false";
|
||||
const mockR2 = (env as any).ATTACHMENT_BUCKET as unknown as MockR2;
|
||||
await env.EMAIL_STORAGE.put(
|
||||
@@ -392,6 +407,7 @@ describe("processEmail — attachments", () => {
|
||||
|
||||
it("uploads attachments to R2 and stores AttachmentData in emailData", async () => {
|
||||
const env = createMockEnv({ withR2: true });
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
const mockR2 = (env as any).ATTACHMENT_BUCKET as unknown as MockR2;
|
||||
await env.EMAIL_STORAGE.put(
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
@@ -423,6 +439,7 @@ describe("processEmail — attachments", () => {
|
||||
|
||||
it("stores attachmentIds in EmailMetadata for trim-time cleanup", async () => {
|
||||
const env = createMockEnv({ withR2: true });
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
await env.EMAIL_STORAGE.put(
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
JSON.stringify({}),
|
||||
@@ -439,6 +456,7 @@ describe("processEmail — attachments", () => {
|
||||
|
||||
it("classifies a cid-referenced image as inline, not a downloadable attachment", async () => {
|
||||
const env = createMockEnv({ withR2: true });
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
await env.EMAIL_STORAGE.put(
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
JSON.stringify({}),
|
||||
@@ -484,6 +502,7 @@ describe("processEmail — attachments", () => {
|
||||
|
||||
it("deletes inline image R2 objects when a trimmed email had them", async () => {
|
||||
const env = createMockEnv({ withR2: true });
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
const mockR2 = (env as any).ATTACHMENT_BUCKET as unknown as MockR2;
|
||||
await env.EMAIL_STORAGE.put(
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
@@ -537,6 +556,7 @@ describe("processEmail — attachments", () => {
|
||||
|
||||
it("deletes R2 objects when a trimmed email had attachments", async () => {
|
||||
const env = createMockEnv({ withR2: true });
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
const mockR2 = (env as any).ATTACHMENT_BUCKET as unknown as MockR2;
|
||||
await env.EMAIL_STORAGE.put(
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
@@ -604,6 +624,7 @@ describe("processEmail — deduplication", () => {
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
JSON.stringify({}),
|
||||
);
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
});
|
||||
|
||||
it("stores only one email when the same Message-ID is delivered twice", async () => {
|
||||
@@ -726,6 +747,7 @@ describe("processEmail — deduplication", () => {
|
||||
describe("processEmail — monitoring counters", () => {
|
||||
it("increments emails_received and sets last_email_at on success", async () => {
|
||||
const env = createMockEnv();
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
await env.EMAIL_STORAGE.put(
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
JSON.stringify({}),
|
||||
@@ -760,6 +782,7 @@ describe("processEmail — feed icon", () => {
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
JSON.stringify({}),
|
||||
);
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
});
|
||||
|
||||
it("persists the latest sender domain on the feed metadata", async () => {
|
||||
@@ -811,6 +834,7 @@ describe("processEmail — unsubscribe capture", () => {
|
||||
`feed:${VALID_FEED_ID}:config`,
|
||||
JSON.stringify({}),
|
||||
);
|
||||
await seedInboundIndex(env, VALID_FEED_ID);
|
||||
});
|
||||
|
||||
it("stores the one-click unsubscribe URL on the feed metadata, keyed by sender", async () => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { EmailParser } from "../domain/email-parser";
|
||||
import { MailboxId } from "../domain/value-objects/mailbox-id";
|
||||
import { AttachmentData, EmailMetadata, Env } from "../types";
|
||||
import { bumpCounters } from "../application/stats";
|
||||
import { dispatchFeedEvents } from "../application/feed-events";
|
||||
@@ -33,6 +33,7 @@ export interface ProcessEmailInput {
|
||||
|
||||
export type IngestRejectionReason =
|
||||
| "invalid_address"
|
||||
| "mailbox_unknown"
|
||||
| "feed_not_found"
|
||||
| "feed_expired"
|
||||
| "sender_blocked";
|
||||
@@ -79,17 +80,33 @@ async function loadAcceptingFeed(
|
||||
): Promise<
|
||||
{ ok: true; feed: Feed } | { ok: false; reason: IngestRejectionReason }
|
||||
> {
|
||||
const feedId = EmailParser.extractFeedId(input.toAddress);
|
||||
if (!feedId) {
|
||||
// MailboxId.parse is the single boundary where an untrusted inbound address
|
||||
// (the most untrusted input in the system) becomes a validated mailbox.
|
||||
const mailbox = MailboxId.parse(input.toAddress);
|
||||
if (!mailbox) {
|
||||
logger.error("Invalid email address format", {
|
||||
toAddress: input.toAddress,
|
||||
});
|
||||
return { ok: false, reason: "invalid_address" };
|
||||
}
|
||||
|
||||
const feed = await FeedRepository.from(env).load(feedId);
|
||||
// Resolve the inbound mailbox to the feed's opaque id (decoupled identities).
|
||||
const repo = FeedRepository.from(env);
|
||||
const feedId = await repo.resolveInbound(mailbox);
|
||||
if (!feedId) {
|
||||
// No feed claims this address — the common "wrong/unknown alias" case.
|
||||
logger.error("Unknown inbound mailbox", { mailbox: mailbox.value });
|
||||
return { ok: false, reason: "mailbox_unknown" };
|
||||
}
|
||||
|
||||
const feed = await repo.load(feedId);
|
||||
if (!feed) {
|
||||
logger.error("Feed not found", { feedId: feedId.value });
|
||||
// The index resolved but the feed is gone — a dangling index (should be
|
||||
// near-impossible now the index is dropped on feed deletion).
|
||||
logger.error("Feed not found", {
|
||||
mailbox: mailbox.value,
|
||||
feedId: feedId.value,
|
||||
});
|
||||
return { ok: false, reason: "feed_not_found" };
|
||||
}
|
||||
if (feed.isExpired()) {
|
||||
|
||||
@@ -21,6 +21,9 @@ export async function fetchFeedData(
|
||||
title: `Newsletter Feed ${feedId.value}`,
|
||||
description: "Converted email newsletter",
|
||||
language: "en",
|
||||
// Read-model fallback only: the RSS/Atom/JSON path never builds the inbound
|
||||
// address, so an empty mailbox is inert here (the real one lives on config).
|
||||
mailbox_id: "",
|
||||
created_at: Date.now(),
|
||||
};
|
||||
|
||||
|
||||
@@ -1,8 +1,15 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { createMockEnv } from "../test/setup";
|
||||
import { createFeedRecord, editFeed } from "./feed-service";
|
||||
import {
|
||||
createFeedRecord,
|
||||
editFeed,
|
||||
deleteFeedRecord,
|
||||
deleteFeedFastDetailed,
|
||||
} from "./feed-service";
|
||||
import { getCounters } from "./stats";
|
||||
import { FeedRepository } from "../infrastructure/feed-repository";
|
||||
import { FeedId } from "../domain/value-objects/feed-id";
|
||||
import { MailboxId } from "../domain/value-objects/mailbox-id";
|
||||
import type { Env } from "../types";
|
||||
|
||||
const mkEnv = (overrides: Partial<Env> = {}) =>
|
||||
@@ -54,6 +61,40 @@ describe("createFeedRecord — TTL policy", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("deleting a feed drops its inbound mailbox index", () => {
|
||||
it("deleteFeedRecord removes the inbound index so the address stops resolving", async () => {
|
||||
const env = mkEnv();
|
||||
const { feedId, mailboxId } = await createFeedRecord(env, { ...baseInput });
|
||||
const repo = FeedRepository.from(env);
|
||||
|
||||
// Sanity: the address resolves to the feed before deletion.
|
||||
expect(
|
||||
(await repo.resolveInbound(MailboxId.unchecked(mailboxId)))?.value,
|
||||
).toBe(feedId);
|
||||
|
||||
await deleteFeedRecord(env, FeedId.unchecked(feedId), () => {});
|
||||
|
||||
expect(
|
||||
await repo.resolveInbound(MailboxId.unchecked(mailboxId)),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("the bulk path (deleteFeedFastDetailed + removeFromListBulk) clears the inbound index", async () => {
|
||||
const env = mkEnv();
|
||||
const { feedId, mailboxId } = await createFeedRecord(env, { ...baseInput });
|
||||
const repo = FeedRepository.from(env);
|
||||
|
||||
// The bulk admin path drops config/metadata, then removes from the list —
|
||||
// the latter is what clears the inbound index (symmetric with save()).
|
||||
await deleteFeedFastDetailed(env.EMAIL_STORAGE, FeedId.unchecked(feedId));
|
||||
await repo.removeFromListBulk([feedId]);
|
||||
|
||||
expect(
|
||||
await repo.resolveInbound(MailboxId.unchecked(mailboxId)),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("editFeed — TTL policy", () => {
|
||||
it("recomputes expiry from the server override on edit", async () => {
|
||||
const env = mkEnv({ FEED_TTL_HOURS: "1" });
|
||||
|
||||
@@ -7,6 +7,7 @@ import { FeedRepository } from "../infrastructure/feed-repository";
|
||||
import { toConfigDTO } from "../infrastructure/feed-mapper";
|
||||
import { BackgroundScheduler } from "../infrastructure/worker";
|
||||
import { FeedId } from "../domain/value-objects/feed-id";
|
||||
import { MailboxId } from "../domain/value-objects/mailbox-id";
|
||||
import { Lifetime } from "../domain/value-objects/lifetime";
|
||||
import {
|
||||
Feed,
|
||||
@@ -33,24 +34,32 @@ function resolveLifetime(env: Env, requestedHours?: number): Lifetime {
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a feed: write its config + empty metadata, register it in the global
|
||||
* list, and bump the `feeds_created` counter. Returns the new feed id + config.
|
||||
* Create a feed: mint an opaque `FeedId` (the read id) and a friendly `MailboxId`
|
||||
* (the inbound address), write its config + empty metadata, register it in the
|
||||
* global list + inbound index, and bump the `feeds_created` counter. Returns the
|
||||
* new feed id, its mailbox, and config.
|
||||
*/
|
||||
export async function createFeedRecord(
|
||||
env: Env,
|
||||
input: CreateFeedInput,
|
||||
): Promise<{ feedId: string; config: FeedConfig }> {
|
||||
): Promise<{ feedId: string; mailboxId: string; config: FeedConfig }> {
|
||||
const repo = FeedRepository.from(env);
|
||||
const feed = Feed.create(FeedId.generate(), input, {
|
||||
mailboxId: MailboxId.generate(),
|
||||
lifetime: resolveLifetime(env, input.lifetimeHours),
|
||||
});
|
||||
|
||||
// save() also writes the inbound:<mailbox> → feedId index.
|
||||
await repo.save(feed);
|
||||
|
||||
// FeedCreated → bumps the feeds_created counter (no background work to schedule).
|
||||
await dispatchFeedEvents(feed, env, () => {});
|
||||
|
||||
return { feedId: feed.id.value, config: toConfigDTO(feed.state()) };
|
||||
return {
|
||||
feedId: feed.id.value,
|
||||
mailboxId: feed.mailboxId.value,
|
||||
config: toConfigDTO(feed.state()),
|
||||
};
|
||||
}
|
||||
|
||||
export type UpdateFeedResult =
|
||||
@@ -118,8 +127,10 @@ type DeleteFeedFastResult = {
|
||||
};
|
||||
|
||||
/**
|
||||
* Delete a feed's config + metadata keys, reporting per-key outcomes. The
|
||||
* larger email/attachment cleanup is handled separately via purgeFeedKeysStep.
|
||||
* Delete a feed's config + metadata keys, reporting per-key outcomes. The larger
|
||||
* email/attachment cleanup is handled separately via purgeFeedKeysStep, and the
|
||||
* inbound `inbound:<mailbox>` index is dropped by `removeFromList(Bulk)` (which
|
||||
* every caller invokes next) — symmetric with `save()` writing it.
|
||||
*/
|
||||
export async function deleteFeedFastDetailed(
|
||||
emailStorage: KVNamespace,
|
||||
@@ -166,6 +177,7 @@ export async function deleteFeedRecord(
|
||||
const unsubscribeUrls = await collectUnsubscribeUrls(emailStorage, feedId);
|
||||
|
||||
await deleteFeedFastDetailed(emailStorage, feedId);
|
||||
// removeFromList also drops the feed's inbound mailbox index.
|
||||
const removed = await repo.removeFromList(feedId);
|
||||
if (removed) {
|
||||
await bumpCounters(emailStorage, { feeds_deleted: 1 });
|
||||
|
||||
Reference in new issue
Block a user