Harden rate-limit keys, signup answers, owner pages and briefing prices

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 <NoticeOwnerOnly title="Agent activity">Agent activity is available to the account owner.</NoticeOwnerOnly>;
 	await connectDb();
 	const actor = pick(params.actor);
 	const action = pick(params.action)?.slice(0, 100);

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 <NoticeOwnerOnly title="API keys">API keys and connected apps are available to the account owner.</NoticeOwnerOnly>;
 	await connectDb();
 	const [apiKeys, connectedApps, newsletters] = await Promise.all([
 		listApiKeys(account),

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 <NoticeOwnerOnly title="Approvals">Approvals are available to the account owner.</NoticeOwnerOnly>;
 	await connectDb();
 	const [pendingRows, decidedRows, newsletters] = await Promise.all([
 		listApprovals(account, { status: "pending", limit: 50 }),

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 (
+		<>
+			<HeaderPage title={title} />
+			<p className="max-w-xl text-sm text-pretty text-muted-foreground">{children}</p>
+		</>
+	);
+}

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 = {}) {

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)

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";
 }

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 }) };
 	}

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);

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");
+});

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);
+});

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/);
+});