From 777f711ef367327bfefbd1f0786602f8d4eba95e Mon Sep 17 00:00:00 2001 From: Cailyn Sinclair Date: Sun, 12 Jul 2026 20:43:15 -0700 Subject: [PATCH 1/2] Remove WCA email from user data Request only public WCA identity, allowlist login fields, and stop MongoDB and PostgreSQL writes. Add a value-free two-phase purge with an enforced privacy rollback floor so the compatibility column can remain safely empty until a later drop. --- .gitignore | 1 + README.md | 20 +++++-- README_DEPLOYMENT.md | 47 +++++++++++++++++ client/src/components/Header.jsx | 16 +++--- client/src/lib/wcaAuth.js | 12 +++++ client/src/lib/wcaAuth.test.js | 15 ++++++ scripts/deploy.sh | 13 +++++ scripts/test-deploy.sh | 40 ++++++++++++++ server/auth/index.js | 11 +--- server/auth/wcaProfile.js | 11 ++++ server/auth/wcaProfile.test.js | 28 ++++++++++ server/models/user.js | 3 -- server/models/user.test.js | 25 +++++++++ server/package.json | 1 + server/postgres/dualWrite.js | 9 ++-- server/postgres/dualWrite.test.js | 7 +-- server/prisma/schema.prisma | 1 + server/privacy/purgeUserEmails.js | 42 +++++++++++++++ server/privacy/userEmailPurge.js | 33 ++++++++++++ server/privacy/userEmailPurge.test.js | 76 +++++++++++++++++++++++++++ 20 files changed, 379 insertions(+), 32 deletions(-) create mode 100644 client/src/lib/wcaAuth.js create mode 100644 client/src/lib/wcaAuth.test.js create mode 100644 server/auth/wcaProfile.js create mode 100644 server/auth/wcaProfile.test.js create mode 100644 server/models/user.test.js create mode 100644 server/privacy/purgeUserEmails.js create mode 100644 server/privacy/userEmailPurge.js create mode 100644 server/privacy/userEmailPurge.test.js diff --git a/.gitignore b/.gitignore index c4b0b9f..0b56507 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,4 @@ client/config/.env.development # Local build/worktree scratch .turbo .worktrees +.privacy-email-cutover diff --git a/README.md b/README.md index 363d1e4..557bfba 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,19 @@ yarn start:client For more on the internals and contributing, check out the [wiki](https://github.com/coder13/LetsCube/wiki) +## Identity privacy + +Let's Cube requests only the WCA OAuth `public` scope. WCA profile data is +allowlisted at login, and the application does not request, retain, mirror, +return, log, analyze, or search WCA email addresses or dates of birth. Product +identity uses the WCA numeric user ID internally and exposes a WCA ID only when +the user has explicitly enabled that existing profile preference. + +Any user-discovery feature must accept only its documented username and visible +WCA ID formats. An email-like input must not be treated as an identifier or +produce an existence signal. This is a required invariant for the Friend System, +not a future discovery mode. + ## Metrics The server stores pseudonymous room and authentication events in both the @@ -71,9 +84,10 @@ access codes, OAuth credentials, chat content, scramble text, or solve times. ## PostgreSQL dual writes New MongoDB writes are mirrored into PostgreSQL without changing application -reads. PostgreSQL receives users and preferences, rooms and participant state, -attempts, durable solve results, and sanitized analytics events. OAuth access -tokens are deliberately not copied. Writes use deterministic UUIDs and upserts, +reads. PostgreSQL receives public identity and preferences, rooms and +participant state, attempts, durable solve results, and sanitized analytics +events. OAuth access tokens are deliberately not copied. Writes use +deterministic UUIDs and upserts, so retries and future backfills are idempotent. Live room saves mirror only the attempts and results changed by that save; complete room snapshots are reserved for explicit backfills. Changing a room event explicitly replaces that room's diff --git a/README_DEPLOYMENT.md b/README_DEPLOYMENT.md index ce85f15..9274f4e 100644 --- a/README_DEPLOYMENT.md +++ b/README_DEPLOYMENT.md @@ -187,6 +187,53 @@ Keep this setting disabled until the legacy scheduler is redesigned. Setting it to `true` explicitly restores the old mode for development or controlled testing. +## WCA Email Privacy Cutover + +The privacy cutover is deliberately separate from ordinary database migrations. +PostgreSQL migrations run before a new application image is healthy, so using a +migration to purge data could let the automatic rollback restore an image that +writes the data again. + +Perform the cutover in this order: + +1. Deploy this release to both application services with `DEPLOY_TARGET=all`. +2. Confirm the API and socket health checks pass, the browser's WCA authorization + request has exactly `scope=public`, and a new login succeeds. +3. From `/opt/letscube`, record the running safe commit as the rollback floor: + + ```bash + git rev-parse HEAD | tee .privacy-email-cutover + ``` + + Future `scripts/deploy.sh` runs refuse any commit that does not descend from + this floor. Do not use direct Compose commands or a saved image tag to restore + an older release after creating the marker. +4. Run the purge from the newly deployed image: + + ```bash + docker compose -f compose.yml -f compose.prod.yml --env-file .env.prod run --rm --no-deps api node server/privacy/purgeUserEmails.js + ``` + + The command reports record counts only. It never prints field values and + exits unsuccessfully if either database still has a value. +5. Run the same command a second time. Every matched, modified, cleared, and + remaining count should be zero; this verifies that the operation is + idempotent. +6. Create and verify a fresh post-cutover backup. Remove every pre-cutover local + and remote backup according to the storage provider's secure deletion + procedure; normal retention is not sufficient for historical private data. + +If the initial deployment rolls back before step 3, do not create the marker or +run the purge. Correct the release and redeploy it first. After step 3, the +privacy-safe commit is the minimum supported rollback image. + +The PostgreSQL `app.users.email` column remains temporarily as an always-empty +compatibility column. Current dual writes never send it a value and clear a +legacy value whenever they update an existing row. Removing the column is phase two: +first deploy an application release that no longer references the compatibility +column, advance every supported rollback image to that release, and only then +apply a migration that drops it. Do not combine that drop with this cutover. + ### Health Checks The API and socket processes expose dependency-aware health endpoints. diff --git a/client/src/components/Header.jsx b/client/src/components/Header.jsx index a30e337..113ee0c 100644 --- a/client/src/components/Header.jsx +++ b/client/src/components/Header.jsx @@ -1,5 +1,4 @@ import React from 'react'; -import qs from 'qs'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import { Link } from 'react-router-dom'; @@ -15,6 +14,7 @@ import Button from '@material-ui/core/Button'; import MenuItem from '@material-ui/core/MenuItem'; import { apiOrigin } from '../lib/fetch'; import { getNameFromId } from '../lib/events'; +import { getWcaAuthorizationUrl } from '../lib/wcaAuth'; const useStyles = makeStyles(() => ({ root: { @@ -57,13 +57,13 @@ function Header({ user, room }) { }; const login = () => { - localStorage.setItem('letscube.redirect_uri', `${document.location.origin}/wca-redirect`); - const url = `${process.env.REACT_APP_WCA_ORIGIN}/oauth/authorize?${qs.stringify({ - response_type: 'code', - scope: 'public dob email', - redirect_uri: `${document.location.origin}/wca-redirect`, - client_id: process.env.REACT_APP_WCA_CLIENT_ID, - })}`; + const redirectUri = `${document.location.origin}/wca-redirect`; + localStorage.setItem('letscube.redirect_uri', redirectUri); + const url = getWcaAuthorizationUrl({ + origin: process.env.REACT_APP_WCA_ORIGIN, + clientId: process.env.REACT_APP_WCA_CLIENT_ID, + redirectUri, + }); window.location = url; }; diff --git a/client/src/lib/wcaAuth.js b/client/src/lib/wcaAuth.js new file mode 100644 index 0000000..53fb131 --- /dev/null +++ b/client/src/lib/wcaAuth.js @@ -0,0 +1,12 @@ +import qs from 'qs'; + +export const WCA_OAUTH_SCOPE = 'public'; + +export const getWcaAuthorizationUrl = ({ origin, clientId, redirectUri }) => ( + `${origin}/oauth/authorize?${qs.stringify({ + response_type: 'code', + scope: WCA_OAUTH_SCOPE, + redirect_uri: redirectUri, + client_id: clientId, + })}` +); diff --git a/client/src/lib/wcaAuth.test.js b/client/src/lib/wcaAuth.test.js new file mode 100644 index 0000000..5bd42b2 --- /dev/null +++ b/client/src/lib/wcaAuth.test.js @@ -0,0 +1,15 @@ +import { getWcaAuthorizationUrl, WCA_OAUTH_SCOPE } from './wcaAuth'; + +describe('WCA authorization request', () => { + it('requests only the public identity scope', () => { + const url = new URL(getWcaAuthorizationUrl({ + origin: 'https://www.worldcubeassociation.org', + clientId: 'client-id', + redirectUri: 'https://letscube.net/wca-redirect', + })); + + expect(WCA_OAUTH_SCOPE).toBe('public'); + expect(url.searchParams.get('scope')).toBe('public'); + expect(url.searchParams.get('scope')).not.toMatch(/dob|email/); + }); +}); diff --git a/scripts/deploy.sh b/scripts/deploy.sh index b177e38..62605b5 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -58,6 +58,19 @@ esac current_commit="$(git rev-parse HEAD)" +privacy_cutover_file="${PRIVACY_EMAIL_CUTOVER_FILE:-$APP_DIR/.privacy-email-cutover}" +if [ -f "$privacy_cutover_file" ]; then + privacy_cutover_commit="$(tr -d '[:space:]' < "$privacy_cutover_file")" + if ! [[ "$privacy_cutover_commit" =~ ^[0-9a-f]{40}$ ]]; then + echo "Invalid email privacy cutover commit in $privacy_cutover_file" >&2 + exit 2 + fi + if ! git merge-base --is-ancestor "$privacy_cutover_commit" "$current_commit"; then + echo "Refusing to deploy $current_commit: it predates the email privacy cutover." >&2 + exit 1 + fi +fi + if [ "$DEPLOY_TARGET" = "auto" ]; then missing_services=() for service in api socket nginx; do diff --git a/scripts/test-deploy.sh b/scripts/test-deploy.sh index ef82299..c707956 100755 --- a/scripts/test-deploy.sh +++ b/scripts/test-deploy.sh @@ -26,6 +26,9 @@ fake_git() { ;; fetch|merge) ;; + merge-base) + [ "${FAKE_CUTOVER_ANCESTOR:-true}" = "true" ] + ;; diff) if [ -n "${FAKE_CHANGED_PATH:-}" ]; then printf '%s\0' "$FAKE_CHANGED_PATH" @@ -269,7 +272,44 @@ run_rollback_test() { assert_contains "$(<"$log")" "git show $previous_commit:compose.prod.yml" } +run_privacy_floor_test() { + local scenario="$TEST_ROOT/privacy-floor" + local app_dir="$scenario/app" + local log="$scenario/commands.log" + local output="$scenario/output.log" + local git_state="$scenario/git-state" + local rollback_marker="$scenario/rollback" + local previous_commit="2222222222222222222222222222222222222222" + local current_commit="1111111111111111111111111111111111111111" + + mkdir -p "$scenario/tmp" + prepare_app "$app_dir" + printf '%s\n' "$previous_commit" > "$app_dir/.privacy-email-cutover" + + if PATH="$FAKE_BIN:$PATH" \ + APP_DIR="$app_dir" \ + DEPLOY_TEST_LOG="$log" \ + ENV_FILE=.env.prod \ + FAKE_CURRENT_COMMIT="$current_commit" \ + FAKE_CUTOVER_ANCESTOR=false \ + FAKE_GIT_STATE="$git_state" \ + FAKE_PREVIOUS_COMMIT="$previous_commit" \ + FAKE_ROLLBACK_MARKER="$rollback_marker" \ + TMPDIR="$scenario/tmp" \ + bash "$app_dir/scripts/deploy.sh" > "$output" 2>&1; then + echo 'Expected a pre-cutover deployment to be rejected.' >&2 + exit 1 + fi + + assert_contains "$(<"$output")" 'predates the email privacy cutover' + if grep -q '^docker ' "$log"; then + echo 'Privacy floor rejection ran Docker commands.' >&2 + exit 1 + fi +} + run_bootstrap_test run_rollback_test +run_privacy_floor_test echo 'Deploy script checks passed.' diff --git a/server/auth/index.js b/server/auth/index.js index 91219c9..525ce30 100644 --- a/server/auth/index.js +++ b/server/auth/index.js @@ -3,6 +3,7 @@ const CustomStrategy = require('passport-custom').Strategy; const { URLSearchParams } = require('url'); const { User } = require('../models'); const metrics = require('../metrics'); +const { buildWcaUserUpdate } = require('./wcaProfile'); const checkStatus = async (res) => { if (res.ok) { // res.status >= 200 && res.status < 300 @@ -37,7 +38,6 @@ module.exports = (app, passport) => { id: +(process.env.LETSCUBE_TEST_USER_ID || 990001), name: 'Cypress Test User', username: 'cypress', - email: 'cypress@example.com', wcaId: '2026TEST01', accessToken: `test-token-${code}`, avatar: {}, @@ -89,14 +89,7 @@ module.exports = (app, passport) => { User.findOneAndUpdate({ id: +profile.id, - }, { - id: +profile.id, - name: profile.name, - email: profile.email, - wcaId: profile.wca_id, - accessToken: tokenRes.access_token, - avatar: profile.avatar, - }, { + }, buildWcaUserUpdate(profile, tokenRes.access_token), { upsert: true, useFindAndModify: false, new: true, diff --git a/server/auth/wcaProfile.js b/server/auth/wcaProfile.js new file mode 100644 index 0000000..d41a023 --- /dev/null +++ b/server/auth/wcaProfile.js @@ -0,0 +1,11 @@ +const buildWcaUserUpdate = (profile, accessToken) => ({ + id: Number(profile.id), + name: profile.name, + wcaId: profile.wca_id, + accessToken, + avatar: profile.avatar, +}); + +module.exports = { + buildWcaUserUpdate, +}; diff --git a/server/auth/wcaProfile.test.js b/server/auth/wcaProfile.test.js new file mode 100644 index 0000000..0769ee3 --- /dev/null +++ b/server/auth/wcaProfile.test.js @@ -0,0 +1,28 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { buildWcaUserUpdate } = require('./wcaProfile'); + +describe('WCA profile ingestion', () => { + it('allowlists the identity fields used by LetsCube', () => { + const update = buildWcaUserUpdate({ + id: '1234', + name: 'Test Solver', + wca_id: '2026TEST01', + avatar: { thumb_url: 'avatar.png' }, + email: 'private@example.com', + dob: '2000-01-01', + unrecognized_field: 'do not retain', + }, 'oauth-token'); + + expect(update).toEqual({ + id: 1234, + name: 'Test Solver', + wcaId: '2026TEST01', + accessToken: 'oauth-token', + avatar: { thumb_url: 'avatar.png' }, + }); + expect(update).not.toHaveProperty('email'); + expect(update).not.toHaveProperty('dob'); + }); +}); diff --git a/server/models/user.js b/server/models/user.js index eb01155..053e993 100644 --- a/server/models/user.js +++ b/server/models/user.js @@ -20,9 +20,6 @@ const schema = new mongoose.Schema({ type: Number, required: true, }, - email: { - type: String, - }, name: { type: String, required: true, diff --git a/server/models/user.test.js b/server/models/user.test.js new file mode 100644 index 0000000..874ac54 --- /dev/null +++ b/server/models/user.test.js @@ -0,0 +1,25 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const mongoose = require('mongoose'); +const UserSchema = require('./user'); + +const User = mongoose.models.UserPrivacyTest + || mongoose.model('UserPrivacyTest', UserSchema); + +describe('user privacy', () => { + it('does not define or serialize an email field', () => { + const user = new User({ + id: 1234, + name: 'Test Solver', + accessToken: 'oauth-token', + showWCAID: true, + }); + user.set('email', 'historical@example.com', { strict: false }); + + expect(UserSchema.path('email')).toBeUndefined(); + expect(user.toObject()).not.toHaveProperty('email'); + expect(user.toJSON()).not.toHaveProperty('email'); + expect(user.toObject()).not.toHaveProperty('accessToken'); + }); +}); diff --git a/server/package.json b/server/package.json index fed3219..99e154c 100644 --- a/server/package.json +++ b/server/package.json @@ -38,6 +38,7 @@ "postgres:migrate:status": "prisma migrate status --config prisma.config.mjs", "postgres:schema:check": "prisma migrate diff --exit-code --from-config-datasource --to-schema prisma/schema.prisma --config prisma.config.mjs", "postgres:schema:validate": "prisma validate --config prisma.config.mjs", + "privacy:purge-user-emails": "node privacy/purgeUserEmails.js", "test": "jest --passWithNoTests", "test:ci": "yarn test" }, diff --git a/server/postgres/dualWrite.js b/server/postgres/dualWrite.js index 7fea99b..ec04f6f 100644 --- a/server/postgres/dualWrite.js +++ b/server/postgres/dualWrite.js @@ -60,11 +60,11 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { const updatedAt = sourceDate(user.updatedAt, fallbackUpdatedAt); await client.query(` INSERT INTO app.users ( - id, wca_user_id, email, name, username, wca_id, preferences, avatar, + id, wca_user_id, name, username, wca_id, preferences, avatar, source_created_at, source_updated_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) ON CONFLICT (wca_user_id) DO UPDATE SET - email = EXCLUDED.email, + email = NULL, name = EXCLUDED.name, username = EXCLUDED.username, wca_id = EXCLUDED.wca_id, @@ -76,14 +76,12 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { OR ( app.users.source_updated_at = EXCLUDED.source_updated_at AND ROW( - app.users.email, app.users.name, app.users.username, app.users.wca_id, app.users.preferences, app.users.avatar ) IS DISTINCT FROM ROW( - EXCLUDED.email, EXCLUDED.name, EXCLUDED.username, EXCLUDED.wca_id, @@ -94,7 +92,6 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { `, [ id, wcaUserId, - user.email || null, user.name, user.username || null, user.wcaId || null, diff --git a/server/postgres/dualWrite.test.js b/server/postgres/dualWrite.test.js index 39503fe..9bfeb96 100644 --- a/server/postgres/dualWrite.test.js +++ b/server/postgres/dualWrite.test.js @@ -18,7 +18,7 @@ const { const user = { id: 1234, - email: 'solver@example.com', + email: 'private@example.com', name: 'Test Solver', username: 'solver', wcaId: '2026TEST01', @@ -46,13 +46,14 @@ describe('PostgreSQL dual writer', () => { expect(stableId('user', 1234)).not.toBe(stableId('room', 1234)); }); - it('mirrors users without copying OAuth access tokens', async () => { + it('mirrors users without copying private profile or OAuth fields', async () => { await mirrorUser(user); expect(client.query).toHaveBeenCalledTimes(1); const values = client.query.mock.calls[0][1]; expect(values).toContain(1234); - expect(values).toContain('solver@example.com'); + expect(client.query.mock.calls[0][0]).toContain('email = NULL'); + expect(values).not.toContain('private@example.com'); expect(values).not.toContain('must-not-be-mirrored'); }); diff --git a/server/prisma/schema.prisma b/server/prisma/schema.prisma index 25c9a74..fa4c43c 100644 --- a/server/prisma/schema.prisma +++ b/server/prisma/schema.prisma @@ -6,6 +6,7 @@ datasource db { model User { id String @id @db.Uuid wcaUserId BigInt @unique(map: "users_wca_user_id_key") @map("wca_user_id") + /// Compatibility-only during the privacy cutover. This column must remain null. email String? name String username String? diff --git a/server/privacy/purgeUserEmails.js b/server/privacy/purgeUserEmails.js new file mode 100644 index 0000000..4de0bf7 --- /dev/null +++ b/server/privacy/purgeUserEmails.js @@ -0,0 +1,42 @@ +const mongoose = require('mongoose'); +const database = require('../database'); +const logger = require('../logger'); +const { pool } = require('../postgres'); +const { purgeUserEmails } = require('./userEmailPurge'); + +const report = (summary) => { + logger.info( + '[PRIVACY] User email purge result: ' + + `MongoDB matched=${summary.mongoMatched} modified=${summary.mongoModified} ` + + `remaining=${summary.mongoRemaining}; PostgreSQL cleared=${summary.postgresCleared} ` + + `remaining=${summary.postgresRemaining}`, + ); +}; + +const run = async () => { + const connection = await database.connect(); + + try { + return await purgeUserEmails({ + mongoUsers: connection.connection.collection('users'), + postgresClient: pool, + report, + }); + } finally { + await Promise.allSettled([ + mongoose.disconnect(), + pool.end(), + ]); + } +}; + +if (require.main === module) { + run().catch((err) => { + logger.error(err); + process.exitCode = 1; + }); +} + +module.exports = { + run, +}; diff --git a/server/privacy/userEmailPurge.js b/server/privacy/userEmailPurge.js new file mode 100644 index 0000000..9a79996 --- /dev/null +++ b/server/privacy/userEmailPurge.js @@ -0,0 +1,33 @@ +const purgeUserEmails = async ({ mongoUsers, postgresClient, report = () => {} }) => { + const mongoResult = await mongoUsers.updateMany( + { email: { $exists: true } }, + { $unset: { email: '' } }, + ); + const mongoRemaining = await mongoUsers.countDocuments({ email: { $exists: true } }); + + const postgresResult = await postgresClient.query( + 'UPDATE app.users SET email = NULL WHERE email IS NOT NULL', + ); + const postgresVerification = await postgresClient.query( + 'SELECT count(*)::int AS remaining FROM app.users WHERE email IS NOT NULL', + ); + + const summary = { + mongoMatched: mongoResult.matchedCount, + mongoModified: mongoResult.modifiedCount, + mongoRemaining, + postgresCleared: postgresResult.rowCount, + postgresRemaining: postgresVerification.rows[0].remaining, + }; + report(summary); + + if (mongoRemaining !== 0 || summary.postgresRemaining !== 0) { + throw new Error('User email purge verification failed'); + } + + return summary; +}; + +module.exports = { + purgeUserEmails, +}; diff --git a/server/privacy/userEmailPurge.test.js b/server/privacy/userEmailPurge.test.js new file mode 100644 index 0000000..947cb05 --- /dev/null +++ b/server/privacy/userEmailPurge.test.js @@ -0,0 +1,76 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { purgeUserEmails } = require('./userEmailPurge'); + +const runPurge = async ({ matchedCount, modifiedCount, postgresCleared }) => { + const mongoUsers = { + updateMany: jest.fn().mockResolvedValue({ matchedCount, modifiedCount }), + countDocuments: jest.fn().mockResolvedValue(0), + }; + const postgresClient = { + query: jest.fn() + .mockResolvedValueOnce({ rowCount: postgresCleared }) + .mockResolvedValueOnce({ rows: [{ remaining: 0 }] }), + }; + const report = jest.fn(); + const summary = await purgeUserEmails({ mongoUsers, postgresClient, report }); + + return { + mongoUsers, postgresClient, report, summary, + }; +}; + +describe('user email purge', () => { + it('removes every MongoDB field and clears every PostgreSQL copy', async () => { + const { + mongoUsers, postgresClient, report, summary, + } = await runPurge({ matchedCount: 4, modifiedCount: 4, postgresCleared: 4 }); + + expect(mongoUsers.updateMany).toHaveBeenCalledWith( + { email: { $exists: true } }, + { $unset: { email: '' } }, + ); + expect(postgresClient.query).toHaveBeenNthCalledWith( + 1, + 'UPDATE app.users SET email = NULL WHERE email IS NOT NULL', + ); + expect(summary).toEqual({ + mongoMatched: 4, + mongoModified: 4, + mongoRemaining: 0, + postgresCleared: 4, + postgresRemaining: 0, + }); + expect(report).toHaveBeenCalledWith(summary); + }); + + it('is idempotent and reports zero changes on a rerun', async () => { + const { summary } = await runPurge({ + matchedCount: 0, + modifiedCount: 0, + postgresCleared: 0, + }); + + expect(summary).toEqual(expect.objectContaining({ + mongoMatched: 0, + mongoModified: 0, + postgresCleared: 0, + })); + }); + + it('fails when either data store still contains a value', async () => { + const mongoUsers = { + updateMany: jest.fn().mockResolvedValue({ matchedCount: 1, modifiedCount: 0 }), + countDocuments: jest.fn().mockResolvedValue(1), + }; + const postgresClient = { + query: jest.fn() + .mockResolvedValueOnce({ rowCount: 0 }) + .mockResolvedValueOnce({ rows: [{ remaining: 0 }] }), + }; + + await expect(purgeUserEmails({ mongoUsers, postgresClient })) + .rejects.toThrow('User email purge verification failed'); + }); +}); From 72e14945d5446db7fb88bc44a9fb533ba3e7e023 Mon Sep 17 00:00:00 2001 From: Cailyn Sinclair Date: Sun, 12 Jul 2026 20:48:10 -0700 Subject: [PATCH 2/2] Make email purge fail closed Exit unsuccessfully when MongoDB cannot be reached and clear the PostgreSQL compatibility column independently of guarded user upserts. This prevents connection failures or equal timestamps from leaving legacy email values behind. --- server/database.js | 2 +- server/database.test.js | 32 +++++++++++++++++++++++++++++++ server/postgres/dualWrite.js | 6 ++++++ server/postgres/dualWrite.test.js | 11 ++++++++--- 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 server/database.test.js diff --git a/server/database.js b/server/database.js index 90533f5..44543be 100644 --- a/server/database.js +++ b/server/database.js @@ -12,7 +12,7 @@ module.exports.connect = async () => { logger.debug('[MONGODB] Connected to database.', { url: config.mongodb }); }).catch((err) => { logger.error('[MONGODB] Error when connecting to database', err); - process.exit(); + process.exit(1); }); return mongoose; diff --git a/server/database.test.js b/server/database.test.js new file mode 100644 index 0000000..dec2416 --- /dev/null +++ b/server/database.test.js @@ -0,0 +1,32 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +jest.mock('mongoose', () => ({ + connect: jest.fn(), + set: jest.fn(), +})); +jest.mock('./logger', () => ({ + debug: jest.fn(), + error: jest.fn(), +})); + +const mongoose = require('mongoose'); +const database = require('./database'); + +describe('MongoDB connection failure', () => { + it('exits unsuccessfully so one-off privacy commands fail closed', async () => { + const connectionError = new Error('MongoDB unavailable'); + const exitError = new Error('process exit'); + mongoose.connect.mockRejectedValue(connectionError); + const exit = jest.spyOn(process, 'exit').mockImplementation((code) => { + exitError.code = code; + throw exitError; + }); + + await expect(database.connect()).rejects.toBe(exitError); + expect(exit).toHaveBeenCalledWith(1); + expect(exitError.code).toBe(1); + + exit.mockRestore(); + }); +}); diff --git a/server/postgres/dualWrite.js b/server/postgres/dualWrite.js index ec04f6f..6a5c869 100644 --- a/server/postgres/dualWrite.js +++ b/server/postgres/dualWrite.js @@ -58,6 +58,12 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { const id = stableId('user', wcaUserId); const updatedAt = sourceDate(user.updatedAt, fallbackUpdatedAt); + // Clear the compatibility column independently so a stale or identical + // source timestamp cannot make the guarded upsert preserve a legacy value. + await client.query( + 'UPDATE app.users SET email = NULL WHERE wca_user_id = $1 AND email IS NOT NULL', + [wcaUserId], + ); await client.query(` INSERT INTO app.users ( id, wca_user_id, name, username, wca_id, preferences, avatar, diff --git a/server/postgres/dualWrite.test.js b/server/postgres/dualWrite.test.js index 9bfeb96..fb26cdb 100644 --- a/server/postgres/dualWrite.test.js +++ b/server/postgres/dualWrite.test.js @@ -49,10 +49,15 @@ describe('PostgreSQL dual writer', () => { it('mirrors users without copying private profile or OAuth fields', async () => { await mirrorUser(user); - expect(client.query).toHaveBeenCalledTimes(1); - const values = client.query.mock.calls[0][1]; + expect(client.query).toHaveBeenCalledTimes(2); + expect(client.query).toHaveBeenNthCalledWith( + 1, + 'UPDATE app.users SET email = NULL WHERE wca_user_id = $1 AND email IS NOT NULL', + [1234], + ); + const values = client.query.mock.calls[1][1]; expect(values).toContain(1234); - expect(client.query.mock.calls[0][0]).toContain('email = NULL'); + expect(client.query.mock.calls[1][0]).toContain('email = NULL'); expect(values).not.toContain('private@example.com'); expect(values).not.toContain('must-not-be-mirrored'); });