src-advisory/SRCmail
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(mail-index): real end-to-end verification, and the three bugs it found

Browse Source 
Adds integration/tests/12-electron-mail-index.spec.ts (3 tests, all passing
against the real Stalwart fixture) and fixes what running it exposed. None of
these were visible from reading the code.

1. JMAP session fetch never followed a redirect. Stalwart 307-redirects
   /.well-known/jmap to /jmap/session, and fetchJmapSession used
   `redirect: 'manual'` and treated any non-2xx as failure - so every reindex
   died with "JMAP session fetch failed (307)". Now follows up to 3 hops and
   REFUSES to follow off-origin, because the user's credentials ride on every
   hop; a blind `redirect: 'follow'` would hand the Authorization header to
   whatever host a misconfigured session pointed at. Same bound and same
   reasoning as lib/auth/verify-jmap-auth.ts.

2. The fd-3 key channel could only be adopted once per process, but its state
   was module-scoped. Next re-evaluates route modules, so a second instance hit
   `Could not open fd 3: Error: open EEXIST` from libuv. State moved to a
   Symbol on globalThis - the one place in a Node process that survives module
   re-evaluation.

3. Next's output file tracing does NOT carry @signalapp/sqlcipher's prebuilds/
   into .next/standalone. It traced the package's JS and its node-gyp-build
   dependency, but node-gyp-build resolves the .node binary by scanning a
   directory at runtime, which no static tracer can follow - so `require()`
   would have failed in every packaged build. scripts/assemble-standalone.mjs
   now copies it, alongside the public/ and .next/static copies it already does
   for the same "standalone output omits things" reason. All six platform/arch
   prebuilds are copied, not just this host's, because electron-builder
   cross-builds the x64 and arm64 macOS targets from one runner.

The three tests, and why it takes three - two constraints made a single
configuration impossible, and both were measured rather than assumed:

  * The renderer cannot reach this fixture from a production build. Its CSP
    pins connect-src to `'self' https: wss:` and the fixture's Stalwart is
    plain HTTP. NODE_ENV=development at RUNTIME does not help: `next build`
    INLINES process.env.NODE_ENV into the compiled middleware, so proxy.ts's
    `isDev` is frozen at build time (observed: a standalone server started with
    NODE_ENV=development still served the production CSP).
  * The fd-3 channel cannot survive `next dev`, which forks its server with an
    IPC channel that claims fd 3 (EEXIST); fd 4 there is not a pipe either
    (ENOTTY).

  So: PIPELINE drives the real standalone server over HTTP from Node with a
  real fd-3 key channel (no browser, so no CSP) and asserts a real SMTP
  delivery is findable by a word from its BODY, with a real snippet and
  contextBlock, idempotent catch-up, working type filters, and - reading the
  raw bytes of the .db AND its -wal - that nothing is recoverable in cleartext.
  TRIGGER proves the event-driven wiring: a real delivery makes the renderer
  POST /api/offline/reindex off its live push. WIRING launches the real shell
  with no ELECTRON_LOAD_URL and asserts the routes are reachable (401, not 404
  or 503) with real safeStorage behind them.

Each test now gets its own --user-data-dir. That is load-bearing, not hygiene:
Electron reuses one profile across launches, and a leftover jmap_stalwart_ctx
cookie from an earlier run made the WIRING test's 401 assertion pass as a 200.

Verified: typecheck clean; unit suite 2379 tests with the SAME 3 pre-existing
failures as the base commit b15098a6 (2 builtin-themes, 1 jmap-client-
resilience) and 48 net new passing; both `docker build`s succeed; the
hosted-deployment gate returns 404 with an empty body and materialises no file
in the production image; e2e/electron-smoke 4/4; 11-electron-notification
still passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Bernd Rodler
2026-08-04 23:43:52 +02:00
co-authored by Claude Sonnet 5
parent 7e9aefcfa1
commit 0271df4338
6 changed files with 635 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
integration/tests/12-electron-mail-index.spec.ts
+508
View File
@@ -0,0 +1,508 @@
import { test, expect, _electron as electron } from '@playwright/test';
import type { ElectronApplication, Page } from '@playwright/test';
import { spawn, type ChildProcess } from 'node:child_process';
import { createServer } from 'node:net';
import { get as httpGet } from 'node:http';
import { createHash, randomBytes } from 'node:crypto';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { ACCOUNTS, JMAP_URL } from './helpers/config';
import { sendMail } from './helpers/smtp';
import { JmapClient } from './helpers/jmap';
import { expectFolderUnread } from './helpers/app';
/**
* The encrypted local search index (lib/mail-index/**) against the real
* Stalwart fixture. THREE tests, because no single configuration can cover the
* whole feature - the reasons are specific and worth reading before changing
* any of them.
*
* Constraint 1 - the renderer cannot reach this fixture from a production
* build. The renderer talks JMAP DIRECTLY to Stalwart, and this fixture's
* Stalwart is deliberately plain HTTP (integration/webmail.Dockerfile explains
* why). The production CSP pins `connect-src` to `'self' https: wss:`. Setting
* NODE_ENV=development at RUNTIME does not help: `next build` INLINES
* process.env.NODE_ENV into the compiled middleware, so proxy.ts's `isDev` is
* frozen at build time. Verified by watching a standalone server started with
* NODE_ENV=development still serve the production CSP, and the login fail with
* "Refused to connect ... violates connect-src 'self' https: wss:".
*
* Constraint 2 - the fd-3 key channel cannot survive `next dev`. `next dev`
* forks its server process with an IPC channel that claims fd 3, so adopting it
* fails with EEXIST; fd 4 in that process is not a pipe either (ENOTTY). Both
* were observed, not assumed. Extra file descriptors simply are not plumbed
* through `npx -> next dev -> forked server`. The real standalone server is a
* single process and has no such problem (test 3 proves it).
*
* So each test takes the configuration that lets it prove its own half:
*
* 1. PIPELINE - drives the REAL standalone server over HTTP from Node, with a
* real fd-3 key channel. CSP is irrelevant here because there is no
* browser: a Node client with a real session cookie exercises the real
* routes. This is the test that proves a real delivery becomes searchable
* by a word from its BODY, and that the file on disk is really encrypted.
*
* 2. TRIGGER - proves the EVENT-DRIVEN wiring: a real SMTP delivery makes the
* renderer POST /api/offline/reindex off the back of its live JMAP push.
* Runs against `next dev` (constraint 1), and asserts the request is made -
* the indexing itself is test 1's job.
*
* 3. WIRING - launches the REAL shell with no ELECTRON_LOAD_URL, so
* electron/main.ts boots the real standalone artifact and stands up the real
* fd-3 key service on real safeStorage. Asserts the index routes are
* REACHABLE in a real build (401 "sign in", not 404 "feature absent", not
* 503 "no native binding / no key channel").
*
* Nothing is mocked anywhere: real SMTP, real Stalwart, real Electron, real
* SQLCipher, real safeStorage.
*/
const alice = ACCOUNTS.alice;
const projectRoot = path.resolve(__dirname, '../..');
/** Mirrors lib/mail-index/paths.ts's accountFileToken(). */
function accountFileToken(accountId: string): string {
return createHash('sha256').update(accountId, 'utf8').digest('hex').slice(0, 32);
}
function getFreePort(): Promise<number> {
return new Promise((resolve, reject) => {
const server = createServer();
server.unref();
server.on('error', reject);
server.listen(0, '127.0.0.1', () => {
const address = server.address();
if (address && typeof address === 'object') {
const { port } = address;
server.close(() => resolve(port));
} else {
server.close(() => reject(new Error('Could not allocate a free localhost port')));
}
});
});
}
function waitForServerReady(url: string, timeoutMs: number): Promise<void> {
const deadline = Date.now() + timeoutMs;
return new Promise((resolve, reject) => {
const attempt = () => {
const req = httpGet(url, (res) => {
res.resume();
resolve();
});
req.on('error', () => {
if (Date.now() > deadline) {
reject(new Error(`Server never became reachable at ${url}`));
return;
}
setTimeout(attempt, 300);
});
};
attempt();
});
}
/**
* Serves the key protocol of electron/key-service.ts over the child's inherited
* fd. The key and the encryption are real; only safeStorage's wrapping of it is
* out of the picture here, which is what test 3 covers.
*/
function serveKeyChannel(child: ChildProcess, fdIndex: number, key: Buffer): void {
const channel = child.stdio[fdIndex] as NodeJS.ReadWriteStream | null;
if (!channel) throw new Error(`child has no fd ${fdIndex} - key channel missing`);
let buffer = '';
channel.on('data', (chunk: Buffer) => {
buffer += chunk.toString('utf8');
let newline: number;
while ((newline = buffer.indexOf('\n')) >= 0) {
const line = buffer.slice(0, newline);
buffer = buffer.slice(newline + 1);
if (!line.trim()) continue;
const req = JSON.parse(line) as { id?: number; op?: string };
const reply =
req.op === 'getIndexKey'
? { id: req.id, ok: true, key: key.toString('hex') }
: req.op === 'deleteIndexKey'
? { id: req.id, ok: true }
: { id: req.id, ok: false, code: 'bad-request', error: 'unknown op' };
channel.write(`${JSON.stringify(reply)}\n`);
}
});
}
/** Minimal cookie jar - the index routes are cookie-authenticated. */
class Jar {
private cookies = new Map<string, string>();
absorb(response: Response): void {
for (const raw of response.headers.getSetCookie()) {
const [pair] = raw.split(';');
const eq = pair.indexOf('=');
if (eq <= 0) continue;
this.cookies.set(pair.slice(0, eq).trim(), pair.slice(eq + 1).trim());
}
}
header(): string {
return [...this.cookies].map(([k, v]) => `${k}=${v}`).join('; ');
}
}
interface SearchHit {
contentType: string;
id: string;
title: string;
snippet: string;
}
interface SearchResponse {
ok?: boolean;
count?: number;
hits?: SearchHit[];
contextBlock?: string;
stats?: Array<{ contentType: string; count: number }>;
error?: string;
}
test.describe('Electron desktop shell - encrypted local search index', () => {
test('pipeline: a real delivery becomes searchable by a body word, and the file is encrypted', async () => {
const jmap = await JmapClient.connect(alice.email, alice.password);
await jmap.reset();
const stamp = Date.now();
const subject = `IT index subject ${stamp}`;
// Appears ONLY in the body, so a hit proves the body was actually fetched
// and indexed - not merely the subject, which any list view already holds.
const bodyPhrase = `zurichlease${stamp}`;
// Deliver BEFORE indexing, so the catch-up path has something real to find.
await sendMail({
from: alice.email,
authPass: alice.password,
to: alice.email,
subject,
body: `Please review the ${bodyPhrase} renewal before September.`,
});
const storeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vncmail-index-it-'));
const key = randomBytes(32);
const port = await getFreePort();
const baseUrl = `http://127.0.0.1:${port}`;
const serverEntry = path.join(projectRoot, '.next', 'standalone', 'server.js');
expect(
fs.existsSync(serverEntry),
`missing ${serverEntry} - run "npm run build:standalone" first`,
).toBe(true);
// The REAL standalone artifact, spawned exactly as electron/main.ts spawns
// it (including the fd-3 key channel), just with plain node rather than
// ELECTRON_RUN_AS_NODE - the server code is identical either way.
const server = spawn(process.execPath, [serverEntry], {
cwd: path.dirname(serverEntry),
env: {
...process.env,
PORT: String(port),
HOSTNAME: '127.0.0.1',
NODE_ENV: 'production',
JMAP_SERVER_URL: JMAP_URL,
SESSION_SECRET: 'integration-not-a-real-secret-32-chars!!',
VNCMAIL_DESKTOP_STORE_DIR: storeDir,
VNCMAIL_DESKTOP_KEY_FD: '3',
},
stdio: ['pipe', 'pipe', 'pipe', 'pipe'],
});
server.stderr?.on('data', (chunk) => process.stderr.write(`[standalone] ${chunk}`));
serveKeyChannel(server, 3, key);
const jar = new Jar();
const call = async (url: string, init?: RequestInit): Promise<Response> => {
const response = await fetch(`${baseUrl}${url}`, {
...init,
headers: { ...(init?.headers ?? {}), cookie: jar.header() },
});
jar.absorb(response);
return response;
};
const search = async (query: string, types?: string): Promise<SearchResponse> => {
const params = new URLSearchParams({ q: query, stats: 'true' });
if (types) params.set('types', types);
const response = await call(`/api/offline/search?${params.toString()}`);
if (!response.ok) return { error: `HTTP ${response.status}: ${await response.text()}` };
return (await response.json()) as SearchResponse;
};
try {
await waitForServerReady(baseUrl, 60000);
// Server-side login. This route verifies the credentials against Stalwart
// from Node and writes BOTH the session cookie and the jmap_stalwart_ctx
// auth context the index routes read (app/api/auth/session/route.ts:94).
const login = await call('/api/auth/session?slot=0', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
serverUrl: JMAP_URL,
username: alice.email,
password: alice.password,
slot: 0,
}),
});
expect(login.status, `login failed: ${await login.text()}`).toBe(200);
// The gate must be open and the native binding loaded, or every assertion
// below would fail for an unrelated reason.
const reachable = await call('/api/offline/search?stats=true&q=');
expect(
reachable.status,
`index routes unreachable: ${(await reachable.text()).slice(0, 300)}`,
).toBe(200);
// Index it. This is the catch-up shape (no ids), which is what the app
// runs at launch.
const reindex = await call('/api/offline/reindex', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ catchUp: true }),
});
const reindexBody = await reindex.json();
expect(reindex.status, JSON.stringify(reindexBody)).toBe(200);
expect(
reindexBody.written?.mail,
`no mail indexed: ${JSON.stringify(reindexBody)}`,
).toBeGreaterThan(0);
// THE assertion: found by a word that exists only in the message body.
const hit = await search(bodyPhrase);
expect(hit.error).toBeUndefined();
expect(hit.count, `search for a body word found nothing: ${JSON.stringify(hit)}`)
.toBeGreaterThan(0);
expect(hit.hits?.[0].contentType).toBe('mail');
expect(hit.hits?.[0].title).toBe(subject);
expect(hit.hits?.[0].snippet).toContain(bodyPhrase);
// The prompt-ready retrieval surface an AI feature would consume.
expect(hit.contextBlock).toContain('[EMAIL]');
expect(hit.contextBlock).toContain(subject);
// Also findable by sender address, which lives in the `people` column.
expect((await search(alice.email)).count).toBeGreaterThan(0);
// Type filtering must filter, and a word in no message must not match -
// otherwise the hit above proves nothing about relevance.
expect((await search(bodyPhrase, 'calendar')).count).toBe(0);
expect((await search(bodyPhrase, 'mail')).count).toBeGreaterThan(0);
expect((await search(`absent${stamp}`)).count).toBe(0);
// Catch-up must be idempotent: a second pass must not duplicate rows.
const before = ((await search(bodyPhrase)).stats ?? [])
.find((s) => s.contentType === 'mail')?.count ?? 0;
const second = await call('/api/offline/reindex', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ catchUp: true }),
});
expect(second.status).toBe(200);
const after = ((await search(bodyPhrase)).stats ?? [])
.find((s) => s.contentType === 'mail')?.count ?? 0;
expect(after).toBe(before);
expect((await search(bodyPhrase)).count).toBe(1);
// Calendar/contacts/files: assert they were ATTEMPTED and did not error,
// rather than asserting counts - this fixture provisions mailboxes only,
// so an empty calendar is the correct result and a count assertion would
// be testing the fixture rather than the code.
const errors = (reindexBody.errors ?? []) as Array<{ contentType: string; message: string }>;
expect(errors, `per-type failures during reindex: ${JSON.stringify(errors)}`).toEqual([]);
const attempted = Object.keys(reindexBody.written ?? {});
const skipped = (reindexBody.skipped ?? []) as string[];
expect(
[...attempted, ...skipped].sort(),
'every content type must be either attempted or explicitly skipped',
).toEqual(['calendar', 'contact', 'file', 'mail']);
} finally {
server.kill();
// Let the process release its WAL files before reading them.
await new Promise((r) => setTimeout(r, 500));
}
// ── the file on disk is genuinely encrypted ──────────────────────────────
const accountId = `${alice.email}@${new URL(JMAP_URL).hostname}`;
const dbPath = path.join(storeDir, 'index', `${accountFileToken(accountId)}.db`);
expect(fs.existsSync(dbPath), `no index database at ${dbPath}`).toBe(true);
// Read every file the store wrote, WAL included: the newest rows can still
// be sitting in the -wal, so checking only the main database could miss
// plaintext that is genuinely on disk.
const onDisk = Buffer.concat(
['', '-wal', '-shm']
.map((suffix) => `${dbPath}${suffix}`)
.filter((f) => fs.existsSync(f))
.map((f) => fs.readFileSync(f)),
);
expect(onDisk.length).toBeGreaterThan(0);
// The assertions that catch a silently-UNENCRYPTED store. `PRAGMA key` is a
// no-op on a non-SQLCipher binding - no error, working database, mailbox in
// cleartext - so every functional assertion above would pass either way.
expect(
fs.readFileSync(dbPath).subarray(0, 15).toString('latin1'),
'the index file has a plain SQLite header - it is NOT encrypted',
).not.toBe('SQLite format 3');
expect(
onDisk.includes(bodyPhrase),
'the message body is recoverable from the raw database bytes - not encrypted',
).toBe(false);
expect(
onDisk.includes(subject),
'the subject is recoverable from the raw database bytes - not encrypted',
).toBe(false);
fs.rmSync(storeDir, { recursive: true, force: true });
});
test('trigger: a real delivery makes the renderer ask the index to update', async () => {
const jmap = await JmapClient.connect(alice.email, alice.password);
await jmap.reset();
const devPort = await getFreePort();
const devUrl = `http://127.0.0.1:${devPort}`;
const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vncmail-trigger-profile-'));
// `next dev` for the CSP reason in the header comment. No key channel here:
// this test asserts the REQUEST is made, which is the wiring it owns; the
// indexing itself is test 1's job. (Extra fds don't survive next dev
// anyway - constraint 2 above.)
const devServer = spawn('npx', ['next', 'dev', '--turbopack', '-p', String(devPort)], {
cwd: projectRoot,
env: {
...process.env,
JMAP_SERVER_URL: JMAP_URL,
SESSION_SECRET: 'integration-not-a-real-secret-32-chars!!',
NODE_ENV: 'development',
// Enough for the route to exist and pass its gate; it fails later on the
// absent key channel, which this test deliberately does not assert on.
VNCMAIL_DESKTOP_STORE_DIR: path.join(userDataDir, 'offline'),
VNCMAIL_DESKTOP_KEY_FD: '3',
},
stdio: 'pipe',
});
devServer.stderr?.on('data', (chunk) => process.stderr.write(`[next dev] ${chunk}`));
let electronApp: ElectronApplication | undefined;
try {
await waitForServerReady(devUrl, 90000);
electronApp = await electron.launch({
args: [projectRoot, `--user-data-dir=${userDataDir}`],
env: { ...process.env, ELECTRON_LOAD_URL: devUrl },
});
const appWindow: Page = await electronApp.firstWindow();
await appWindow.waitForLoadState('domcontentloaded');
await appWindow.locator('#username').waitFor({ state: 'visible', timeout: 60000 });
await appWindow.fill('#username', alice.email);
await appWindow.fill('#password', alice.password);
await appWindow.click('button[type="submit"]');
await appWindow
.locator('[data-testid="account-switcher"]')
.first()
.waitFor({ state: 'visible', timeout: 60000 });
// An actively-selected inbox is a precondition for the push handler's
// refresh, which is what schedules the index update - the same reason
// 11-electron-notification.spec.ts waits here.
await expectFolderUnread(appWindow, { role: 'inbox' }, 0);
const reindexCalls: string[] = [];
appWindow.on('request', (request) => {
if (request.method() === 'POST' && request.url().includes('/api/offline/reindex')) {
reindexCalls.push(request.postData() ?? '');
}
});
// Let the launch-time catch-up land first so it is not mistaken for the
// delivery-driven call below.
await appWindow.waitForTimeout(8000);
const baseline = reindexCalls.length;
await sendMail({
from: alice.email,
authPass: alice.password,
to: alice.email,
subject: `IT index trigger ${Date.now()}`,
body: 'a delivery should make the renderer ask the index to update',
});
await expect
.poll(() => reindexCalls.length, {
timeout: 60000,
message:
'a real delivery did not make the renderer POST /api/offline/reindex - ' +
'the push -> handleStateChange -> indexOnStateChange wiring is broken',
})
.toBeGreaterThan(baseline);
// The delivery-driven call must name the mail type, rather than being an
// unconditional full catch-up.
const triggered = reindexCalls.slice(baseline);
expect(
triggered.some((body) => body.includes('"mail"')),
`no reindex call mentioned the mail type: ${JSON.stringify(triggered)}`,
).toBe(true);
} finally {
await electronApp?.close();
devServer.kill();
fs.rmSync(userDataDir, { recursive: true, force: true });
}
});
test('wiring: the real standalone boot reaches the index with a real safeStorage key', async () => {
// A FRESH profile is load-bearing, not hygiene: the 401 this test asserts is
// "nobody is signed in", and a leftover jmap_stalwart_ctx cookie from any
// previous run turns it into a 200. That actually happened while writing this.
const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vncmail-wiring-profile-'));
const electronApp = await electron.launch({
args: [projectRoot, `--user-data-dir=${userDataDir}`],
env: {
...process.env,
JMAP_SERVER_URL: JMAP_URL,
SESSION_SECRET: 'integration-not-a-real-secret-32-chars!!',
},
});
try {
const appWindow: Page = await electronApp.firstWindow();
await appWindow.waitForLoadState('domcontentloaded');
await appWindow.locator('#username').waitFor({ state: 'visible', timeout: 60000 });
// safeStorage must be usable, or main.ts deliberately refuses to enable
// the feature at all (electron/key-service.ts's checkEncryptionAvailable).
const encryptionAvailable = await electronApp.evaluate(({ safeStorage }) =>
safeStorage.isEncryptionAvailable(),
);
expect(
encryptionAvailable,
'safeStorage reports no encryption available on this host, so main.ts ' +
'correctly disabled the index - this assertion cannot pass here',
).toBe(true);
const probe = await appWindow.evaluate(async () => {
const response = await fetch('/api/offline/search?q=anything');
return { status: response.status, body: (await response.text()).slice(0, 300) };
});
// 401 = the gate opened, the native binding loaded and the fd-3 key
// channel is present; it refuses only because nobody is signed in (this
// build cannot log in against a plain-HTTP Stalwart - constraint 1).
// 404 => VNCMAIL_DESKTOP_STORE_DIR was never set (gate closed, or
// main.ts refused because no OS keyring is available)
// 503 => the native binding or the key channel is missing from the real
// artifact - the class of failure only a real build reveals
expect(
probe.status,
`expected 401 (reachable, unauthenticated) but got ${probe.status}: ${probe.body}`,
).toBe(401);
} finally {
await electronApp.close();
fs.rmSync(userDataDir, { recursive: true, force: true });
}
});
});