From 868a24fc5f5f9475ae97a53eb7b8985c7eb34ea4 Mon Sep 17 00:00:00 2001 From: ankurmans <12939357+ankurmans@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:40:59 +0100 Subject: [PATCH 1/4] Rate limits: key on cf-connecting-ip before the client's x-forwarded-for --- lib/rate-limit.js | 3 +++ tests/rate-limit.test.mjs | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/rate-limit.js b/lib/rate-limit.js index 5b5e7e90..838a0ca9 100644 --- a/lib/rate-limit.js +++ b/lib/rate-limit.js @@ -72,7 +72,10 @@ async function reportUnavailable(name, error, store) { } } +// Cloudflare sets cf-connecting-ip itself; the first x-forwarded-for entry is whatever the client sent. export function getClientIp(headers) { + const connecting = headers.get("cf-connecting-ip")?.trim(); + if (connecting) return connecting; const forwardedFor = headers.get("x-forwarded-for")?.split(",")[0].trim(); return forwardedFor || headers.get("x-real-ip") || "unknown"; } diff --git a/tests/rate-limit.test.mjs b/tests/rate-limit.test.mjs index 64522f5a..206d9678 100644 --- a/tests/rate-limit.test.mjs +++ b/tests/rate-limit.test.mjs @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { after, before, test } from "node:test"; import { setDriver, sqliteDriver } from "../lib/docdb/driver.js"; -import { isLimitedInDatabase, isLimitedInMemory, isRateLimited } from "../lib/rate-limit.js"; +import { getClientIp, isLimitedInDatabase, isLimitedInMemory, isRateLimited } from "../lib/rate-limit.js"; delete process.env.UPSTASH_REDIS_REST_URL; delete process.env.UPSTASH_REDIS_REST_TOKEN; @@ -85,3 +85,11 @@ test("when the database errors, limits fall back to memory and say so once", asy setDriver(database); } }); + +test("the client address is the one Cloudflare set, whatever x-forwarded-for starts with", () => { + const spoofed = new Headers({ "x-forwarded-for": "10.9.1.1, 198.51.100.200", "cf-connecting-ip": "203.0.113.9" }); + assert.equal(getClientIp(spoofed), "203.0.113.9"); + assert.equal(getClientIp(new Headers({ "x-forwarded-for": "203.0.113.5, 10.0.0.1" })), "203.0.113.5", "without Cloudflare, the first forwarded address"); + assert.equal(getClientIp(new Headers({ "x-real-ip": "203.0.113.6" })), "203.0.113.6"); + assert.equal(getClientIp(new Headers()), "unknown"); +}); From 2d83abd0e9a26014a72aa5bdf94b47beb06f9943 Mon Sep 17 00:00:00 2001 From: ankurmans <12939357+ankurmans@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:42:13 +0100 Subject: [PATCH 2/4] Signup: on a newsletter that confirms, every address gets the same answer --- lib/signup.js | 6 ++++- tests/signup-uniform.test.mjs | 50 +++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 tests/signup-uniform.test.mjs diff --git a/lib/signup.js b/lib/signup.js index 0346b5b1..31f5ce3e 100644 --- a/lib/signup.js +++ b/lib/signup.js @@ -122,6 +122,10 @@ export async function signUp({ formData, ip, headers = null, source = "website", } const isPending = contact.status === "pending"; const isHeld = contact.status === "held"; + // Where an address has to confirm, every address gets the answer a new one gets (an active, unsubscribed or suppressed one + // too), so the form can't be used to ask who is on the list. Nothing is sent to those: a confirmation would only tell + // their owner about someone else's try. + const answersAsPending = isPending || (!isHeld && (forceConfirmation || newsletter.requireConfirmation)); if (isHeld && created) after(() => notifyHeld({ newsletter }).catch((error) => console.error(`[signup] Held signup alert for ${newsletter.slug} failed:`, error.message))); if (isSendingEnabled() && (isPending || (created && contact.status === "active"))) { const queue = isPending ? queueConfirmationEmail : queueWelcomeEmail; @@ -142,7 +146,7 @@ export async function signUp({ formData, ip, headers = null, source = "website", } const survey = flowUrl ? null : await getSignupSurvey({ newsletter, contact, created }); if (isHeld) return { ok: true, held: true, ...(flowUrl && { flowUrl }), ...(survey && { survey }) }; - if (!isPending) { + if (!answersAsPending) { const step = flowUrl ? null : await getStep({ newsletter, contact }); return { ok: true, ...(flowUrl && { flowUrl }), ...(step && { step }), ...(survey && { survey }) }; } diff --git a/tests/signup-uniform.test.mjs b/tests/signup-uniform.test.mjs new file mode 100644 index 00000000..64dc736a --- /dev/null +++ b/tests/signup-uniform.test.mjs @@ -0,0 +1,50 @@ +import assert from "node:assert/strict"; +import { after, before, test } from "node:test"; +import { form, startTestApp } from "./helpers/app.mjs"; +import { creator, nextIp } from "./helpers/growth.mjs"; +import { runWithRequest } from "./stubs/next-headers.mjs"; +import { flushAfter } from "./stubs/next-server.mjs"; + +let app; +let m; +before(async () => { + app = await startTestApp(); + m = { + archive: await import("../app/archive/actions.js"), + contacts: await import("../lib/contacts.js"), + Contact: (await import("../models/Contact.js")).default, + Message: (await import("../models/Message.js")).default, + }; +}); +after(() => app?.stop()); + +async function subscribe(user, email) { + const ip = nextIp(); + const result = await runWithRequest({ headers: { "x-forwarded-for": ip } }, () => m.archive.subscribe(null, form({ newsletter: user.slug, email }))); + await flushAfter(); + return result; +} + +test("a newsletter that confirms answers the same for a new address and one already subscribed", async () => { + const user = await creator("uniform1"); + const fresh = await subscribe(user, "stranger@example.net"); + assert.equal(fresh.pending, true); + await subscribe(user, "member@example.net"); + const member = await m.Contact.findOne({ accountId: user.accountId, newsletterId: user.newsletterId, email: "member@example.net" }).lean(); + await m.contacts.confirmContact({ accountId: user.accountId, contactId: member._id, newsletterId: user.newsletterId }); + assert.equal((await m.Contact.findOne({ accountId: user.accountId, _id: member._id }).lean()).status, "active"); + const emailsBefore = await m.Message.countDocuments({ accountId: user.accountId, email: "member@example.net" }); + + const again = await subscribe(user, "member@example.net"); + assert.deepEqual({ ...again, email: undefined }, { ...fresh, email: undefined }); + assert.equal(again.email, "member@example.net"); + assert.equal(await m.Message.countDocuments({ accountId: user.accountId, email: "member@example.net" }), emailsBefore, "and nothing is sent to the subscriber"); + assert.equal((await m.Contact.findOne({ accountId: user.accountId, _id: member._id }).lean()).status, "active", "who stays subscribed"); +}); + +test("a newsletter without confirmation still answers a returning subscriber as before", async () => { + const user = await creator("uniform2", { confirm: false }); + const first = await subscribe(user, "member@example.net"); + assert.equal(first.pending, undefined); + assert.equal((await subscribe(user, "member@example.net")).pending, undefined); +}); From a7c063f5f9d089b67d31aa0b0900cedc70845aa4 Mon Sep 17 00:00:00 2001 From: ankurmans <12939357+ankurmans@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:42:56 +0100 Subject: [PATCH 3/4] Dashboard: API keys, approvals and agent activity pages are the owner's, not a team seat's --- app/(app)/agent-activity/page.js | 5 ++- app/(app)/api-keys/page.js | 5 ++- app/(app)/approvals/page.js | 5 ++- components/NoticeOwnerOnly.jsx | 11 +++++++ tests/team-owner-pages.test.mjs | 53 ++++++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 components/NoticeOwnerOnly.jsx create mode 100644 tests/team-owner-pages.test.mjs diff --git a/app/(app)/agent-activity/page.js b/app/(app)/agent-activity/page.js index 95ee281e..1e7c8ecd 100644 --- a/app/(app)/agent-activity/page.js +++ b/app/(app)/agent-activity/page.js @@ -2,6 +2,7 @@ import Link from "next/link"; import { Activity, ArrowUpRight } from "lucide-react"; import BadgeStatus from "@/components/BadgeStatus"; import HeaderPage from "@/components/HeaderPage"; +import NoticeOwnerOnly from "@/components/NoticeOwnerOnly"; import StateEmpty from "@/components/StateEmpty"; import { Button } from "@/components/ui/button"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table"; @@ -12,6 +13,7 @@ import { ACTIVITY_PAGE_SIZE, getDayCounts, listActivity, listDayCounts } from "@ import { ACTIVITY_RETENTION_DAYS } from "@/lib/agent-activity-rules"; import { getKeyLevel, listApiKeys } from "@/lib/api-keys"; import { requireAccount } from "@/lib/auth"; +import { isAccountOwner } from "@/lib/team"; import { connectDb } from "@/lib/db"; import { formatDate, formatRelative } from "@/lib/format"; import { listConnectedApps } from "@/lib/oauth-apps"; @@ -31,7 +33,8 @@ function pick(value) { // ?actor= is the per-key view. export default async function AgentActivityPage({ searchParams }) { const params = await searchParams; - const { account } = await requireAccount(); + const { account, user } = await requireAccount(); + if (!isAccountOwner(account, user)) return Agent activity is available to the account owner.; await connectDb(); const actor = pick(params.actor); const action = pick(params.action)?.slice(0, 100); diff --git a/app/(app)/api-keys/page.js b/app/(app)/api-keys/page.js index 823fbc4a..a705a99d 100644 --- a/app/(app)/api-keys/page.js +++ b/app/(app)/api-keys/page.js @@ -6,6 +6,7 @@ import ButtonRevoke from "@/components/ButtonRevoke"; import DialogCreateApiKey from "@/components/DialogCreateApiKey"; import GatePlan from "@/components/GatePlan"; import HeaderPage from "@/components/HeaderPage"; +import NoticeOwnerOnly from "@/components/NoticeOwnerOnly"; import IconBrand, { AGENTS } from "@/components/IconBrand"; import StateEmpty from "@/components/StateEmpty"; import TableConnectedApps from "@/components/TableConnectedApps"; @@ -24,6 +25,7 @@ import { getDayCounts } from "@/lib/agent-activity"; import { getKeyLevel, listApiKeys } from "@/lib/api-keys"; import { isKeyExpired } from "@/lib/api-key-tokens"; import { requireAccount } from "@/lib/auth"; +import { isAccountOwner } from "@/lib/team"; import { getAppUrl } from "@/lib/config"; import { connectDb } from "@/lib/db"; import { getMcpClients, getMcpConnectors, getMcpUrl } from "@/lib/docs/mcp-clients"; @@ -35,7 +37,8 @@ import Newsletter from "@/models/Newsletter"; export const metadata = { title: "API keys" }; export default async function ApiKeysPage() { - const { account } = await requireAccount(); + const { account, user } = await requireAccount(); + if (!isAccountOwner(account, user)) return API keys and connected apps are available to the account owner.; await connectDb(); const [apiKeys, connectedApps, newsletters] = await Promise.all([ listApiKeys(account), diff --git a/app/(app)/approvals/page.js b/app/(app)/approvals/page.js index 9f1dcb25..30f7658d 100644 --- a/app/(app)/approvals/page.js +++ b/app/(app)/approvals/page.js @@ -4,12 +4,14 @@ import BadgeStatus from "@/components/BadgeStatus"; import CardAgentApproval from "@/components/CardAgentApproval"; import CardApproval from "@/components/CardApproval"; import HeaderPage from "@/components/HeaderPage"; +import NoticeOwnerOnly from "@/components/NoticeOwnerOnly"; import StateEmpty from "@/components/StateEmpty"; import { Button } from "@/components/ui/button"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table"; import { scoped } from "@/lib/account-scope"; import { getStaleness, listApprovals, toApprovalView } from "@/lib/approvals"; import { requireAccount } from "@/lib/auth"; +import { isAccountOwner } from "@/lib/team"; import { connectDb } from "@/lib/db"; import { formatRelative } from "@/lib/format"; import Newsletter from "@/models/Newsletter"; @@ -19,7 +21,8 @@ export const metadata = { title: "Approvals" }; const HISTORY_LIMIT = 30; export default async function ApprovalsPage() { - const { account } = await requireAccount(); + const { account, user } = await requireAccount(); + if (!isAccountOwner(account, user)) return Approvals are available to the account owner.; await connectDb(); const [pendingRows, decidedRows, newsletters] = await Promise.all([ listApprovals(account, { status: "pending", limit: 50 }), diff --git a/components/NoticeOwnerOnly.jsx b/components/NoticeOwnerOnly.jsx new file mode 100644 index 00000000..e89f191d --- /dev/null +++ b/components/NoticeOwnerOnly.jsx @@ -0,0 +1,11 @@ +import HeaderPage from "@/components/HeaderPage"; + +// A page of the account owner's that a team seat can open: the title and a sentence, nothing from the account. +export default function NoticeOwnerOnly({ title, children }) { + return ( + <> + +

{children}

+ + ); +} diff --git a/tests/team-owner-pages.test.mjs b/tests/team-owner-pages.test.mjs new file mode 100644 index 00000000..29199a29 --- /dev/null +++ b/tests/team-owner-pages.test.mjs @@ -0,0 +1,53 @@ +import assert from "node:assert/strict"; +import { after, before, test } from "node:test"; +import { createElement } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { as, form, signIn, startTestApp } from "./helpers/app.mjs"; +import { addDraft, agentOwner, api, loadAgentModules } from "./helpers/agents.mjs"; +import { TooltipProvider } from "../components/ui/tooltip.jsx"; +import { TEAM_COOKIE, acceptTeamInvitation } from "../lib/team.js"; + +const PAGES = ["app/(app)/api-keys/page.js", "app/(app)/approvals/page.js", "app/(app)/agent-activity/page.js"]; +const text = (markup) => markup.replaceAll(/<[^>]+>/g, " ").replaceAll(/\s+/g, " "); +const toUser = (who) => ({ id: String(who.userId), email: who.email, emailVerified: true }); + +let app, m, owner, account; +const seats = {}; + +async function render(who, path) { + const { default: Page } = await import(`../${path}`); + return text(renderToStaticMarkup(createElement(TooltipProvider, null, await as(who, () => Page({ searchParams: Promise.resolve({}) }))))); +} + +before(async () => { + app = await startTestApp(); + m = await loadAgentModules(); + owner = await agentOwner(m, "pages-owner@example.com"); + account = await m.Account.findById(owner.accountId).lean(); + for (const role of ["viewer", "editor", "admin"]) { + const seat = await signIn(`pages-${role}@example.com`); + const invite = await as(owner, () => m.runAction("invite_team_member", { account, user: toUser(owner), input: { email: seat.email, role }, via: "dashboard" })); + await acceptTeamInvitation(toUser(seat), invite.id); + seat.jar.set(TEAM_COOKIE, String(account._id)); + seats[role] = seat; + } + const id = await addDraft(m, owner.keys.send_with_approval, { subject: "Secret pending issue" }); + await api(owner.keys.send_with_approval, "POST", `/campaigns/${id}/send`, {}); + await as(owner, () => m.apiKeys.createApiKey(null, form({ name: "Owner private key", level: "full" }))); +}); +after(() => app?.stop()); + +test("a team seat opening the owner's key, approval and activity pages sees none of their data", async () => { + for (const [role, seat] of Object.entries(seats)) { + for (const path of PAGES) { + const page = await render(seat, path); + assert.match(page, /available to the account owner/, `${role} ${path}`); + assert.doesNotMatch(page, /Owner private key|Secret pending issue|The exact input that will run|send_with_approval/, `${role} ${path}`); + } + } +}); + +test("the owner still sees them", async () => { + assert.match(await render(owner, PAGES[0]), /Owner private key/); + assert.match(await render(owner, PAGES[1]), /Secret pending issue/); +}); From 57a7fe76a842147dcd2990a6e9bd9b54eb1015e5 Mon Sep 17 00:00:00 2001 From: ankurmans <12939357+ankurmans@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:43:23 +0100 Subject: [PATCH 4/4] Briefings: setting prices waits for approval like raising the budget --- lib/action-scopes.js | 4 +++- lib/actions/briefings.js | 2 +- tests/agent-scopes.test.mjs | 6 ++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/action-scopes.js b/lib/action-scopes.js index 9c8fc3c7..d5a28cc0 100644 --- a/lib/action-scopes.js +++ b/lib/action-scopes.js @@ -221,7 +221,9 @@ const ELEVATIONS = { // DEFAULT_BUDGET_USD and the default discoveryBudgetUsd (lib/briefing-rules.js), repeated here: this file imports nothing. const BRIEFING_DEFAULT_BUDGET_USD = 0.25; function isBriefingSend(input) { - return Number(input.budgetUsd ?? 0) > BRIEFING_DEFAULT_BUDGET_USD || Number(input.discovery?.discoveryBudgetUsd ?? 0) > 0; + // The briefing's own prices replace the built-in table (lib/briefings/prices.js), so a price of zero lifts the cap as surely as a budget does. + const setsPrices = Array.isArray(input.prices) && input.prices.length > 0; + return setsPrices || Number(input.budgetUsd ?? 0) > BRIEFING_DEFAULT_BUDGET_USD || Number(input.discovery?.discoveryBudgetUsd ?? 0) > 0; } export function getActionClass(name, input = {}) { diff --git a/lib/actions/briefings.js b/lib/actions/briefings.js index 15a97c1d..6a46016b 100644 --- a/lib/actions/briefings.js +++ b/lib/actions/briefings.js @@ -662,7 +662,7 @@ const fields = { }) ) .max(20) - .describe("Prices for models the built-in price table doesn't have, or to override it. A model with no known price can't run under the budget."), + .describe("Prices for models the built-in price table doesn't have, or to override it. A model with no known price can't run under the budget. A key that needs approval for sends needs it to set prices."), budgetUsd: z.number().positive().max(MAX_BUDGET_USD).describe(`The most a run may spend on model calls, in USD: ${DEFAULT_BUDGET_USD} by default, ${MAX_BUDGET_USD} at most. A call that would pass it isn't made.`), reviewMode: z .enum(BRIEFING_REVIEW_MODES) diff --git a/tests/agent-scopes.test.mjs b/tests/agent-scopes.test.mjs index 53b89adb..78dc88b4 100644 --- a/tests/agent-scopes.test.mjs +++ b/tests/agent-scopes.test.mjs @@ -77,6 +77,12 @@ describe("scope classes", () => { assert.equal(scopes.getActionClass("create_series", { mode: "send" }), "send"); assert.equal(scopes.getActionClass("send_test_email", {}), "write-draft", "a test to the owner is drafting"); assert.equal(scopes.getActionClass("send_test_email", { to: ["someone@example.com"] }), "send", "a test to addresses the agent names is an email to strangers"); + for (const name of ["create_briefing", "update_briefing"]) { + assert.equal(scopes.getActionClass(name, { name: "Morning" }), "write"); + assert.equal(scopes.getActionClass(name, { budgetUsd: 1 }), "send"); + assert.equal(scopes.getActionClass(name, { prices: [] }), "write", "clearing the briefing's own prices puts the table back"); + assert.equal(scopes.getActionClass(name, { prices: [{ model: "gpt-5", inputPerMillion: 0, outputPerMillion: 0 }] }), "send", "a price of its own can lift the run's cap, so it waits like a bigger budget"); + } assert.equal(scopes.getActionClass("nonexistent_action"), null); for (const name of ["send_test_email", "create_series", "send_campaign", "delete_campaign", "import_contacts", "add_contact"]) assert.equal(scopes.isGateable(name), true, name); for (const name of ["create_campaign", "update_campaign", "list_campaigns", "update_newsletter", "pause_campaign"]) assert.equal(scopes.isGateable(name), false, name);