fix: harden agent report-access program (post-merge review of #258)

app/components/portal/AgentReportActions.test.tsx

@@ -31,17 +31,17 @@ function renderActions(
 }
 
 describe("AgentReportActions — registered agent (hasAccount: true)", () => {
-  it('renders the "Go to my workspace" CTA', () => {
+  it('renders the "Email me a sign-in link" CTA', () => {
     const { getByTestId } = renderActions(true);
-    expect(getByTestId("agent-report-workspace-cta").textContent).toBe("Go to my workspace");
+    expect(getByTestId("agent-report-workspace-cta").textContent).toBe("Email me a sign-in link");
   });
 
   it("submitting posts the agent-magic-login intent with tenant/inspectionId/token", async () => {
     let submitted: Record<string, FormDataEntryValue | null> | null = null;
     const { getByTestId } = renderActions(true, async ({ request }) => {
       const formData = await request.formData();
       submitted = Object.fromEntries(formData.entries());
-      return { ok: true, intent: "agent-magic-login", loginUrl: null };
+      return { ok: true, intent: "agent-magic-login", sent: true };
     });
 
     fireEvent.click(getByTestId("agent-report-workspace-cta"));
@@ -56,6 +56,19 @@ describe("AgentReportActions — registered agent (hasAccount: true)", () => {
     });
   });
 
+  it("shows a check-your-email confirmation on success (never navigates)", async () => {
+    const { getByTestId, findByTestId } = renderActions(true, async () => ({
+      ok: true,
+      intent: "agent-magic-login",
+      sent: true,
+    }));
+
+    fireEvent.click(getByTestId("agent-report-workspace-cta"));
+
+    const confirm = await findByTestId("agent-report-workspace-sent");
+    expect(confirm.textContent).toMatch(/check your email/i);
+  });
+
   it("shows an error message when the action reports failure", async () => {
     const { getByTestId, findByText } = renderActions(true, async () => ({
       ok: false,
@@ -64,7 +77,7 @@ describe("AgentReportActions — registered agent (hasAccount: true)", () => {
 
     fireEvent.click(getByTestId("agent-report-workspace-cta"));
 
-    await findByText(/couldn't sign you in/i);
+    await findByText(/couldn't send your sign-in link/i);
   });
 });
 

app/components/portal/AgentReportActions.tsx

@@ -7,9 +7,11 @@
  * never sees this component.
  *
  * hasAccount branches the CTA:
- *   - true  : "Go to my workspace" — posts the agent-magic-login intent and
- *     full-navigates to the single-use loginUrl on success (a 302 that mints
- *     a cookie, so it cannot be a client fetch + client-side redirect).
+ *   - true  : "Email me a sign-in link" — posts the agent-magic-login intent;
+ *     the server EMAILS a single-use sign-in link to the agent's account inbox
+ *     (never returns it), so on success we show a "check your email"
+ *     confirmation rather than navigating. Emailing (vs. returning the link)
+ *     closes the report-link → agent-session takeover vector (#258 review #5).
  *   - false : "Create your free agent account" — a plain link into
  *     /agent-signup with the recipient email + returnTo prefilled.
  *
@@ -29,7 +31,8 @@ import { m } from "~/paraglide/messages";
 interface AgentMagicLoginActionResult {
   ok: boolean;
   intent?: string;
-  loginUrl?: string | null;
+  /** True when the server accepted the request and (if an account exists) emailed the sign-in link. */
+  sent?: boolean;
 }
 
 export interface AgentReportActionsProps {
@@ -53,18 +56,19 @@ export function AgentReportActions({
 }: AgentReportActionsProps) {
   const fetcher = useFetcher<AgentMagicLoginActionResult>();
   const [error, setError] = useState(false);
+  const [sent, setSent] = useState(false);
   const lastHandled = useRef<AgentMagicLoginActionResult | undefined>(undefined);
 
   // Consume the action result exactly once per response (mirrors
   // WordExportButton's guard — fetcher.data keeps the same reference across
-  // renders, so without it this effect would re-fire and re-navigate).
+  // renders). The server emails the sign-in link and answers { sent: true }
+  // (anti-oracle — identical whether or not an account exists), so on success
+  // we show a "check your email" confirmation instead of navigating anywhere.
   useEffect(() => {
     if (fetcher.state !== "idle" || !fetcher.data || fetcher.data === lastHandled.current) return;
     lastHandled.current = fetcher.data;
-    if (fetcher.data.ok && fetcher.data.loginUrl) {
-      // Full navigation (not client-side) — the redeem endpoint is a
-      // cookie-setting 302, which a client-side router transition can't relay.
-      window.location.assign(fetcher.data.loginUrl);
+    if (fetcher.data.ok) {
+      setSent(true);
     } else {
       setError(true);
     }
@@ -94,6 +98,16 @@ export function AgentReportActions({
 
   const submitting = fetcher.state !== "idle";
 
+  if (sent) {
+    return (
+      <div className="max-w-4xl mx-auto px-4 sm:px-6 pb-8 print:hidden" data-testid="agent-report-actions">
+        <Banner tone="success">
+          <span data-testid="agent-report-workspace-sent">{m.agent_report_actions_workspace_sent()}</span>
+        </Banner>
+      </div>
+    );
+  }
+
   return (
     <div className="max-w-4xl mx-auto px-4 sm:px-6 pb-8 print:hidden" data-testid="agent-report-actions">
       <Banner

app/components/settings/AutomationEditorModal.tsx

@@ -137,6 +137,11 @@ export function AutomationEditorModal({
             <input name="delayMinutes" type="number" min={0} defaultValue={rule?.delayMinutes ?? 0}
               className="w-24 h-9 px-3 rounded-md border border-ih-border bg-ih-bg-input text-[13px]" title={m.settings_automations_delay_title()} />
           </div>
+          {recipientKind === "all" && (
+            <p className="text-[11px] text-ih-watch-fg" data-testid="automation-all-recipients-warning">
+              {m.settings_automations_all_recipients_warning()}
+            </p>
+          )}
           {noChannel && (
             <p className="text-[11px] text-ih-watch-fg">{m.settings_automations_pick_channel()}</p>
           )}

app/routes/public/portal-inspection.tsx

@@ -212,7 +212,7 @@ export async function loader({ params, request, context }: Route.LoaderArgs) {
 /* ------------------------------------------------------------------ */
 
 type AgentMagicLoginActionResult =
-  | { ok: true; intent: "agent-magic-login"; loginUrl: string | null }
+  | { ok: true; intent: "agent-magic-login"; sent: boolean }
   | { ok: false; intent: "agent-magic-login"; error?: string }
   | { ok: false; intent: string };
 
@@ -232,11 +232,11 @@ export async function action({ request, params, context }: Route.ActionArgs) {
       if (!res.ok) {
         return { ok: false, intent: "agent-magic-login" } satisfies AgentMagicLoginActionResult;
       }
-      const body = (await res.json()) as { data?: { loginUrl: string | null } };
+      const body = (await res.json()) as { data?: { sent?: boolean } };
       return {
         ok: true,
         intent: "agent-magic-login",
-        loginUrl: body.data?.loginUrl ?? null,
+        sent: body.data?.sent ?? true,
       } satisfies AgentMagicLoginActionResult;
     } catch {
       return { ok: false, intent: "agent-magic-login" } satisfies AgentMagicLoginActionResult;

messages/en/reports.json

@@ -202,10 +202,11 @@
   "report_view_cert": "Certified Inspection Report",
   "report_view_repair_list_link": "View Repair List",
   "report_view_build_repair": "Build repair request",
-  "agent_report_actions_workspace_hint": "You already have an agent account for this email — sign in to track referrals and manage your profile.",
-  "agent_report_actions_workspace_cta": "Go to my workspace",
-  "agent_report_actions_workspace_pending": "Signing in…",
-  "agent_report_actions_workspace_error": "We couldn't sign you in right now. Please try again in a moment.",
+  "agent_report_actions_workspace_hint": "You already have an agent account for this email — we'll email you a secure link to sign in to your workspace.",
+  "agent_report_actions_workspace_cta": "Email me a sign-in link",
+  "agent_report_actions_workspace_pending": "Sending…",
+  "agent_report_actions_workspace_sent": "Check your email — we've sent a single-use sign-in link to your inbox. It expires in 15 minutes.",
+  "agent_report_actions_workspace_error": "We couldn't send your sign-in link right now. Please try again in a moment.",
   "agent_report_actions_signup_hint": "Create a free agent account to track referrals across every inspection you send.",
   "agent_report_actions_signup_cta": "Create your free agent account",
   "report_view_print": "Print",

messages/en/settings-integrations.json

@@ -257,6 +257,7 @@
   "settings_automations_dothis_legend": "Do this",
   "settings_automations_delay_title": "Delay in minutes (for reminders: minutes BEFORE the inspection)",
   "settings_automations_pick_channel": "Pick at least one delivery channel.",
+  "settings_automations_all_recipients_warning": "\"Everyone on the inspection\" sends to every report-receiving person added to each inspection — including any listing (seller's) agent. Use a specific role if the buyer's report should not reach the seller's side.",
   "settings_automations_template_label": "Template",
   "settings_automations_select_template": "— Select a template —",
   "settings_automations_edit_new_template": "Edit / New template",

scripts/file-size-baseline.json

@@ -18,7 +18,7 @@
   "server/api/inspections/report-delivery.ts": 667,
   "app/hooks/usePhotoOps.ts": 661,
   "server/lib/messaging/providers/telnyx-compliance.ts": 657,
-  "server/services/inspection/inspection-analytics.service.ts": 651,
+  "server/services/inspection/inspection-analytics.service.ts": 656,
   "app/components/editor/ItemEditor.tsx": 633,
   "server/api/admin/admin-settings.ts": 633,
   "app/components/inspection-edit/CompliancePanel.tsx": 623,
@@ -28,10 +28,10 @@
   "server/services/concierge.service.ts": 575,
   "server/api/auth.ts": 564,
   "app/lib/collab/results-binding.ts": 560,
+  "server/api/portal.ts": 551,
   "server/api/calendar.ts": 547,
   "app/components/NewInspectionWizard.tsx": 545,
   "server/api/inspections/core.ts": 541,
-  "server/api/portal.ts": 539,
   "server/services/inspection/inspection-publish.service.ts": 536,
   "server/services/agent/referral.ts": 533,
   "server/api/inspections/publish.ts": 532,
@@ -59,8 +59,8 @@
   "app/components/media-studio/VideoCapture.tsx": 433,
   "server/api/inspections/results.ts": 430,
   "server/api/inspections/hierarchy.ts": 428,
-  "server/lib/middleware/di.ts": 425,
   "app/hooks/useStructureEdit.ts": 424,
+  "server/lib/middleware/di.ts": 421,
   "app/routes/templates.tsx": 411,
   "app/routes/calendar.tsx": 402,
   "server/api/secrets.ts": 401

scripts/tenant-scoping-baseline.json

@@ -34,9 +34,9 @@
   "server/services/agreement/signer-state.ts:53",
   "server/services/agreement/signer-state.ts:68",
   "server/services/ai.service.ts:91",
-  "server/services/auth.service.ts:133",
-  "server/services/auth.service.ts:189",
-  "server/services/auth.service.ts:218",
+  "server/services/auth.service.ts:149",
+  "server/services/auth.service.ts:205",
+  "server/services/auth.service.ts:234",
   "server/services/automation/core.ts:161",
   "server/services/automation/core.ts:189",
   "server/services/automation/delivery.ts:181",

server/api/agent/magic-login.ts

@@ -30,15 +30,15 @@ const requestRoute = createRoute(withMcpMetadata({
     method: 'post',
     path: '/magic-login/request',
     tags: ['agents', 'public'],
-    summary: 'Exchange an agent report token for a login code',
-    description: 'Public, unauthenticated endpoint for the agent unified link. Exchanges a live, agent-kind report-link token for a single-use magic-login code (900 second TTL) that mints an agent session cookie on redeem. Returns loginUrl: null with 200 (not an error) when the token is valid but no global agent account exists yet for the recipient email, so probing tokens cannot be used to enumerate accounts.',
+    summary: 'Email an agent a single-use sign-in link for a report token',
+    description: 'Public, unauthenticated endpoint for the agent unified link. For a live, agent-kind report-link token whose recipient email has a global agent account, EMAILS a single-use magic-login link (900 second TTL) to that agent\'s account inbox — the link is never returned to the caller, so a leaked/forwarded report link cannot be replayed into a full agent session. Always returns { sent: true } with 200 (identical response and timing whether or not an account exists) so probing tokens cannot enumerate accounts.',
     request: {
         body: { content: { 'application/json': { schema: MagicLoginRequestSchema } } },
     },
     responses: {
         200: {
             content: { 'application/json': { schema: MagicLoginRequestResponseSchema } },
-            description: 'Login code issued, or loginUrl: null when no agent account exists for the recipient',
+            description: 'Always { sent: true } — a sign-in link is emailed to the agent when an account exists; nothing is sent otherwise. The link is never returned here.',
         },
         401: { description: 'Invalid, revoked, expired, inspection-mismatched, or non-agent report token' },
     },
@@ -66,15 +66,43 @@ const redeemRoute = createRoute(withMcpMetadata({
 export const agentMagicLoginRequestRoutes = createApiRouter()
     .openapi(requestRoute, async (c) => {
         const body = c.req.valid('json');
-        const loginUrl = await requestMagicLogin({
+        const result = await requestMagicLogin({
             portalAccess: c.var.services.portalAccess,
             kv: c.env.TENANT_CACHE,
             db: c.env.DB,
             inspectionId: body.inspectionId,
             reportToken: body.token,
             coreBaseUrl: getBaseUrl(c),
         });
-        return c.json({ success: true as const, data: { loginUrl } }, 200);
+        // EMAIL the single-use sign-in link to the agent's own account inbox —
+        // never return it to the caller. The durable report link is a reusable
+        // bearer, so returning a full-session login URL to whoever presents it
+        // was an account-takeover vector (#258 review #5). Only the agent's inbox
+        // can now complete sign-in; report VIEWING via the token is unchanged.
+        if (result) {
+            // Defer the send to waitUntil — awaiting only on the account-exists
+            // path would make it measurably slower than the no-account path, a
+            // timing enumeration oracle. Mirrors server/api/agent/login.ts's
+            // loginLink and portal.ts's requestLink.
+            const sendPromise = (async () => {
+                try {
+                    await c.var.services.email.sendAgentLoginLink(result.email, result.loginUrl);
+                } catch (err) {
+                    logger.error('agent.magic_login.link_send_failed', {}, err instanceof Error ? err : undefined);
+                }
+            })();
+            let execCtx: Pick<ExecutionContext, 'waitUntil'> | undefined;
+            try {
+                execCtx = c.executionCtx;
+            } catch {
+                execCtx = undefined;
+            }
+            if (execCtx) execCtx.waitUntil(sendPromise);
+            else await sendPromise;
+        }
+        // Anti-oracle: identical { sent: true } response (and timing) whether or
+        // not an agent account exists for the report link's recipient.
+        return c.json({ success: true as const, data: { sent: true as const } }, 200);
     });
 
 /** GET /agent/magic-login — mounted at the TOP LEVEL (not under /api); see workers/app.ts. */

server/api/inspections/people.ts

@@ -147,7 +147,7 @@ const peopleRoutes = createApiRouter()
         const tenantId = getTenantId(c);
         const { id, personId } = c.req.valid('param');
         await assertInspectionOwned(getDrizzle(c), id, tenantId);
-        await c.var.services.people.removePerson(tenantId, personId);
+        await c.var.services.people.removePerson(tenantId, id, personId);
         return c.json({ success: true as const }, 200);
     });
 

server/api/portal.ts

@@ -390,16 +390,28 @@ const portalRoutes = portalRouter
             return c.json({ error: 'Invalid or expired link' }, 401);
         }
 
-        // SECURITY: a find-my-report magic link redeemed by an agent must mint
-        // an AGENT JWT (mirrors server/api/agent/login.ts's password mint
-        // EXACTLY — no tenantId) and NEVER the client __Host-portal_session
-        // cookie. Global agent accounts (findGlobalAgentByEmail — the single
-        // source of the "live global agent" predicate) are a SEPARATE identity
-        // plane from tenant-scoped client/co_client contacts; there is no
-        // token/grant object on this magic-link path (unlike exchangeRoute),
-        // only a verified email, so the global-agent-account lookup is the
-        // correct signal to branch on here.
-        const agent = await findGlobalAgentByEmail(c.env.DB, verified.email);
+        // Prefer a client session when this email holds a live CLIENT-KIND grant
+        // in this tenant, even if it ALSO has a global agent account — else a
+        // dual-identity recipient could never reach their client report (#258
+        // review #9). hasLiveClientGrant is kind='client' specific, NOT
+        // listRecipientInspections (capability-based → also matches agent grants
+        // via the Spec 3 self-retrieve flip). Agent-role report tokens still can't
+        // unlock the client hub — that guard lives in exchangeRoute (role-based).
+        const tenantId = resolveTenantId(c);
+        let hasClientAccess = false;
+        if (tenantId) {
+            try {
+                hasClientAccess = await c.var.services.portal.hasLiveClientGrant(tenantId, verified.email);
+            } catch (err) {
+                logger.error('[portal] redeem client-access lookup failed', {}, err instanceof Error ? err : undefined);
+            }
+        }
+
+        // SECURITY: an agent-only email (no client access) mints an AGENT JWT
+        // (mirrors server/api/agent/login.ts — no tenantId) and NEVER the client
+        // __Host-portal_session cookie. Global agents (findGlobalAgentByEmail) are
+        // a SEPARATE identity plane from tenant-scoped client/co_client contacts.
+        const agent = hasClientAccess ? null : await findGlobalAgentByEmail(c.env.DB, verified.email);
         if (agent) {
             const keyring = await c.var.keyringPromise!;
             const now = Math.floor(Date.now() / 1000);

server/lib/mcp/openapi-snapshot.json

@@ -15065,8 +15065,8 @@
         "$ref": "#/components/schemas/AgentMagicLoginRequest"
       }
     },
-    "summary": "Exchange an agent report token for a login code",
-    "description": "Public, unauthenticated endpoint for the agent unified link. Exchanges a live, agent-kind report-link token for a single-use magic-login code (900 second TTL) that mints an agent session cookie on redeem. Returns loginUrl: null with 200 (not an error) when the token is valid but no global agent account exists yet for the recipient email, so probing tokens cannot be used to enumerate accounts."
+    "summary": "Email an agent a single-use sign-in link for a report token",
+    "description": "Public, unauthenticated endpoint for the agent unified link. For a live, agent-kind report-link token whose recipient email has a global agent account, EMAILS a single-use magic-login link (900 second TTL) to that agent's account inbox — the link is never returned to the caller, so a leaked/forwarded report link cannot be replayed into a full agent session. Always returns { sent: true } with 200 (identical response and timing whether or not an account exists) so probing tokens cannot enumerate accounts."
   },
   {
     "operationId": "requestInvoicePayment",

server/lib/people/capabilities.ts

@@ -17,5 +17,10 @@ export function capabilitiesForKind(kind: RoleKind): RoleCapabilities {
         case 'client': return { receivesReport: true, selfRetrieveReport: true,  canSign: true,  canPay: true,  canHaveAccount: false };
         case 'agent':  return { receivesReport: true, selfRetrieveReport: true,  canSign: true,  canPay: true,  canHaveAccount: true  };
         case 'other':  return { receivesReport: true, selfRetrieveReport: false, canSign: false, canPay: false, canHaveAccount: false };
+        // Fail closed: callers cast DB `kind` strings through `as RoleKind`, so a
+        // corrupt/out-of-enum value would otherwise return undefined and throw on
+        // `[cap]` access. An unknown role gets NO capabilities (no report, no
+        // sign/pay/account), never an accidental grant.
+        default:       return { receivesReport: false, selfRetrieveReport: false, canSign: false, canPay: false, canHaveAccount: false };
     }
 }