Recipient flow backend + merge main into ember/recipient-flow-v1

[xBalbinus]

Summary

Resolves the merge conflict between ember/recipient-flow-v1 and main, then implements the full backend the parent (recipient) UI calls for in BACKEND_HANDOFF_V2.md §5.5.

Merge resolution

• 16 add/add conflicts on web/app/r/[token]/** — resolved by taking main's EmberChrome-polished pages. Those pages already imported from _lib/{state,entries,sharing,prompts}, but _lib/ only existed on ember/recipient-flow-v1 — so the merge fixes a broken-on-main state by combining the polished pages with the missing libs.
• Cleaned up two pre-existing unresolved git stash markers on main in web/app/onboarding/questions/write/page.tsx and web/app/onboarding/questions/review/page.tsx so type-check passes.

Backend implementation

• Migration 004 (server/src/db/migrations/004_recipient_flow.sql) adds journal_entries, sharing, recipients.password_hash/account_created_at, gifts.personal_message.
• Routes (server/src/routes/recipient.ts, server/src/routes/auth.ts):
  • GET /r/:token returns the full envelope (giver, recipient, prompts, sharing, archived, personalMessage).
  • GET /r/:token/prompts derives parent prompts from giver questions.
  • Journal entries CRUD: GET/POST/PATCH/DELETE /r/:token/entries[/:eid] with snapshot-on-write of promptText.
  • Multipart POST /r/:token/entries/:eid/photo and .../audio (audio includes Whisper transcription that gracefully falls back when Whisper fails).
  • Sharing: GET/PATCH /r/:token/sharing + idempotent POST /r/:token/share.
  • Recipient account: POST /r/:token/account sets bcrypt-hashed credentials; POST /auth/recipient/login verifies + returns the access token.
  • Write enforcement: every entries / sharing-config write returns 409 once the journal is shared.
• Storage layer (server/src/lib/gift-store.ts) gains createEntry/listEntries/getEntry/patchEntry/deleteEntry, getSharing/patchSharing/performShare/isJournalArchived, setRecipientCredentials/recipientLogin/recipientHasAccount.

The legacy per-question responses endpoints (/questions/:qid/{text,voice,photo}) remain in place for the older flow — the new parent UI uses /entries instead.

Tests

• 20 new INTEGRATION-gated tests in server/src/routes/recipient-flow.integration.test.ts cover envelope, prompts, all four CRUD verbs, photo + audio upload (happy + Whisper-failure), sharing patch + share + idempotency + lock enforcement, account creation + login + wrong-password + unknown-email rejection.
• End-to-end smoke verified by curl-ing a fresh bun run'd server through the full flow: create gift → patch + 3 questions → send → recipient envelope → entries (free-write + prompt + photo + voice) → sharing config → account → login → share → 409 on writes after share.

Test plan

• DATABASE_URL=postgresql://localhost:5432/ember_test bun run --filter server db:migrate applies all 4 migrations cleanly
• INTEGRATION=1 DATABASE_URL=... bun --cwd server test src/routes/recipient-flow.integration.test.ts passes (20/20)
• INTEGRATION=1 DATABASE_URL=... bun --cwd server test src/routes/recipient.integration.test.ts still passes (legacy 9/9)
• bun run --filter web type-check passes
• bun run --filter server type-check passes

Notes

• One pre-existing failing test (auth-login.integration.test.ts) is unrelated — it asserts usr_ ID prefix that the upstream xors API stopped using. Same failure happens on main.
• Two pre-existing biome lint warnings (messages.test.ts, auth-login.test.ts — noAssignInExpressions) are also pre-existing on main.

[slopless-scanner]

⚠️ Slopless Review

Confidence: 🟡 4/5 · Verdict: REQUEST_CHANGES · Risk: HIGH · Findings: 14

Reviewed the PR diff without prior scan context (no architecture artifacts found). Produced 14 finding(s) across 4 vulnerability class(es) (architecture, best_practice, code_quality (+1 more)) in 46.1s.

PR review complete: 14 findings, verdict: request_changes

Scope & coverage

• Lines: 2967
• Checks performed: architecture · best_practice · code_quality · security

Findings

🔴 HIGH — Hardcoded OpenAI API Key in Test File

server/src/routes/recipient-flow.integration.test.ts:110 · confidence: high · CWE-798

The test file contains a hardcoded stub API key sk-test-stub for OpenAI. While this is a test stub, the pattern is dangerous because: (1) it could be accidentally committed to production, (2) it normalizes hardcoding secrets in code, (3) the actual OPENAI_API_KEY environment variable is used in production routes without validation that it's not a test stub.

Suggested fix: Use a proper test harness that stubs the fetch call (as done in the same file with stubWhisper). Never set real environment variables to stub values. Instead, mock the Anthropic client or fetch layer before importing routes.

Code context

process.env.OPENAI_API_KEY = "sk-test-stub";

🛑 CRITICAL — Missing Authorization Check on Journal Entry Endpoints

server/src/routes/recipient.ts:200-350 · confidence: high · CWE-639

The journal entry CRUD endpoints (POST /r/:token/entries, PATCH /r/:token/entries/:id, DELETE /r/:token/entries/:id) accept a token parameter but don't verify that the authenticated user (recipient) owns the journal being modified. An attacker with one recipient's token could potentially modify another recipient's entries if they can guess or enumerate token values. The token is used to load the recipient, but there's no check that the current request's auth context matches the token's recipient.

Suggested fix: Add explicit ownership verification: before creating/updating/deleting an entry, verify that currentRecipient.id === foundRecipient.id where currentRecipient comes from the auth context and foundRecipient comes from the token lookup. Consider using a middleware to enforce this on all /r/:token/* routes.

Code context

// Example from the truncated diff - the actual implementation needs review
// POST /r/:token/entries should verify: current_recipient.id === token_recipient.id

🔴 HIGH — Insufficient Validation on Audio/Photo File Uploads

server/src/routes/recipient.ts:1-50 · confidence: medium · CWE-434

The recipient flow allows audio and photo uploads via /r/:token/entries with audio_url and photo_url fields. The code doesn't show explicit validation of: (1) file size limits, (2) MIME type validation, (3) virus/malware scanning, (4) path traversal prevention in storage keys. The keyFor() function is used but its implementation isn't shown - if it doesn't properly sanitize the filename, it could allow directory traversal attacks.

Suggested fix: Implement strict file upload validation: (1) enforce maximum file sizes (e.g., 50MB for audio, 10MB for photos), (2) validate MIME types against a whitelist, (3) use a UUID-based filename instead of user-provided names, (4) scan uploads with a malware detection service, (5) verify keyFor() uses path.join() or similar to prevent traversal, (6) store files outside the web root.

Code context

// Storage operations visible in the diff but implementation details missing
// Need to verify keyFor() and bucketFor() prevent path traversal

🔴 HIGH — Password Hash Storage Without Verification of Algorithm

server/src/routes/auth.ts:135-170 · confidence: high · CWE-327

The migration adds a password_hash column to recipients, and the code calls setRecipientCredentials() which presumably hashes passwords. However, the implementation of password hashing is not visible in the diff. If bcrypt or Argon2 isn't used with proper cost factors, passwords could be vulnerable to brute force attacks. Additionally, there's no evidence of password complexity requirements or rate limiting on the /auth/recipient/login endpoint.

Suggested fix: Verify that recipientLogin() and setRecipientCredentials() use bcrypt with cost factor ≥12 or Argon2id. Implement rate limiting on /auth/recipient/login (e.g., 5 attempts per 15 minutes per IP). Add password complexity requirements (minimum 8 chars, mixed case, numbers). Consider adding account lockout after N failed attempts.

Code context

const recipient = await recipientLogin(body.email, body.password);
if (!recipient) {
  set.status = 401;
  return { error: "Wrong email or password." };
}

🔴 HIGH — Recipient Login Returns Access Token Without Secure Cookie

server/src/routes/auth.ts:145-155 · confidence: high · CWE-614

The /auth/recipient/login endpoint returns the access token in the JSON response body (token: recipient.accessToken). This token is then used in the URL path (/r/:token). Tokens in URLs are logged in server logs, browser history, and referrer headers, making them vulnerable to exposure. The giver flow uses secure HTTP-only cookies, but the recipient flow uses URL-based tokens.

Suggested fix: Set the access token as an HTTP-only, Secure, SameSite=Strict cookie instead of returning it in the response body. Update the frontend to use the cookie automatically. If URL-based tokens are required for email links, use short-lived tokens (5-15 minutes) that exchange for a session cookie, or implement a separate 'magic link' flow with one-time use tokens.

Code context

return {
  ok: true as const,
  token: recipient.accessToken,
  redirectTo: `/r/${recipient.accessToken}/journal`,
};

🟠 MEDIUM — Missing CSRF Protection on State-Changing Endpoints

server/src/routes/recipient.ts:200-400 · confidence: medium · CWE-352

The recipient flow endpoints that modify state (POST/PATCH/DELETE on entries, PATCH on sharing) don't show explicit CSRF token validation. If the frontend makes these requests via fetch without CSRF tokens, a malicious site could trick a logged-in recipient into modifying their journal.

Suggested fix: Implement CSRF protection: (1) use SameSite=Strict cookies (already done for auth), (2) require CSRF tokens in request headers for state-changing operations, (3) validate origin/referer headers, (4) consider using the 'double-submit cookie' pattern if tokens aren't feasible.

Code context

// POST /r/:token/entries, PATCH /r/:token/entries/:id, DELETE /r/:token/entries/:id
// No visible CSRF token validation

🟠 MEDIUM — Insufficient Input Validation on Text Fields

server/src/lib/route-helpers.ts:28-60 · confidence: medium · CWE-79

The journal entry creation accepts text, promptText, preface, and milestoneText fields without visible length limits or sanitization. While the schema shows some max lengths (e.g., personalMessage: 8000), there's no evidence of XSS prevention (HTML escaping) or injection attack prevention when these fields are returned in API responses or rendered in the frontend.

Suggested fix: Add maxLength constraints to all text fields in the schema (e.g., text: 50000, preface: 1000). Ensure the frontend escapes all user-provided text when rendering. Use a library like DOMPurify if rendering rich text. Consider storing text as plain text only, not HTML.

Code context

export const journalEntrySchema = t.Object({
  text: t.Union([t.String(), t.Null()]),
  promptText: t.Union([t.String(), t.Null()]),
  preface: t.Union([t.String(), t.Null()]),
  // No maxLength constraints visible
});

🟠 MEDIUM — Sharing State Modification Without Proper Validation

server/src/routes/recipient.ts:400-450 · confidence: medium · CWE-20

The PATCH /r/:token/sharing endpoint allows recipients to change their sharing mode and dates. The code calls patchSharing() but doesn't show validation that: (1) the date is in the future, (2) the milestone text is reasonable, (3) the sharing mode transition is valid (e.g., can't unshare after sharing). An attacker could set a past date to trigger immediate sharing, or set invalid milestone values.

Suggested fix: Add validation in patchSharing(): (1) if mode is 'date', verify the date is >= today, (2) if mode is 'milestone', validate milestoneText is non-empty and reasonable length, (3) prevent downgrading from 'shared' state, (4) log all sharing state changes for audit purposes.

Code context

// PATCH /r/:token/sharing endpoint - validation logic not visible in diff

🟠 MEDIUM — Token-Based Authentication Lacks Expiration

server/src/db/schema.ts:175-195 · confidence: high · CWE-613

The recipient access tokens (used in /r/:token URLs) don't show an expiration mechanism. If a token is leaked, it could be used indefinitely. The schema shows accessToken but no accessTokenExpiresAt field. This is especially concerning given that tokens are passed in URLs.

Suggested fix: Add accessTokenExpiresAt timestamp field to recipients table. Set tokens to expire after 30-90 days. Implement token refresh mechanism or require re-authentication for sensitive operations. Consider using JWT tokens with embedded expiration instead of opaque tokens.

Code context

export const recipients = pgTable(
  "recipients",
  {
    accessToken: text("access_token").notNull(),
    // No expiration field visible
  },
);

🟠 MEDIUM — Incomplete Error Handling in AI Routes

server/src/routes/ai.ts:1-50 · confidence: medium · CWE-755

The AI conversation endpoints (/ai/converse, /ai/summarize) call external APIs (Anthropic Claude) but the error handling isn't visible in the diff. If the API fails, returns invalid JSON, or times out, the error response might leak sensitive information or crash the server.

Suggested fix: Wrap all external API calls in try-catch blocks. Return generic error messages to clients (e.g., 'Service temporarily unavailable'). Log detailed errors server-side for debugging. Implement timeouts (e.g., 30 seconds) for API calls. Add circuit breaker pattern if API is frequently failing.

Code context

// AI route implementation not fully visible in diff

🟡 LOW — Test Environment Variable Pollution

server/src/routes/recipient.integration.test.ts:88-95 · confidence: low

The test files set process.env.TEST_USER_EMAIL and process.env.OPENAI_API_KEY globally, which can leak across test files in the same process. While auth-login.integration.test.ts attempts to clean this up with beforeAll/afterAll, the pattern is fragile and could cause test flakiness.

Suggested fix: Use a test setup file that isolates environment variables per test file. Consider using a library like dotenv-safe or env-var that validates and scopes environment variables. Alternatively, use dependency injection to pass test doubles instead of relying on environment variables.

Code context

beforeAll(() => {
  process.env.TEST_USER_EMAIL = TEST_EMAIL;
  process.env.OPENAI_API_KEY = "sk-test-stub";
});

🟡 LOW — Inconsistent Error Response Format

server/src/routes/auth.ts:145-170 · confidence: low

Some endpoints return { error: string } while others return { ok: boolean, ... }. This inconsistency makes client-side error handling harder and could lead to bugs where the client doesn't properly detect errors.

Suggested fix: Standardize error responses across all endpoints. Use a consistent format like { ok: false, error: { code: string, message: string } } for errors and { ok: true, data: T } for success. Document this in the API spec.

Code context

if (!recipient) {
  set.status = 401;
  return { error: "Wrong email or password." };
}
return {
  ok: true as const,
  token: recipient.accessToken,
  redirectTo: `/r/${recipient.accessToken}/journal`,
};

🟠 MEDIUM — Missing Rate Limiting on Sensitive Endpoints

server/src/routes/auth.ts:135-170 · confidence: high · CWE-770

The /auth/recipient/login endpoint has no visible rate limiting. An attacker could brute-force recipient passwords by trying many email/password combinations. Similarly, the /ai/converse endpoint could be abused to generate unlimited API calls, incurring costs.

Suggested fix: Implement rate limiting using a library like @elysiajs/rate-limit or similar. For login endpoints: limit to 5 attempts per 15 minutes per IP. For AI endpoints: limit to 10 requests per minute per user. Use Redis or in-memory store to track attempt counts.

Code context

.post(
  "/recipient/login",
  async ({ body, set }) => {
    const recipient = await recipientLogin(body.email, body.password);
    // No rate limiting visible
  },

🟡 LOW — Potential Information Disclosure in Error Messages

server/src/routes/auth.ts:145-150 · confidence: low · CWE-209

The error message 'Wrong email or password' is good (doesn't reveal which field is wrong), but other endpoints might leak information. For example, if an endpoint returns different errors for 'user not found' vs 'invalid token', an attacker could enumerate valid tokens.

Suggested fix: Audit all error messages to ensure they don't leak information about valid users, tokens, or system state. Use generic messages like 'Invalid credentials' or 'Not found' for authentication/authorization errors.

Code context

return { error: "Wrong email or password." };

---

Reviewed by Slopless · Install on your repo · comment @slopless to re-run