From f72c67864e8f8c92ec9d4509916c0a2a377b5911 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Thu, 27 Aug 2026 21:48:23 -0700 Subject: [PATCH] Let the container run with nothing writable The server writes to one path and no other: SESSION_FILE, from sessions.ts. Everything else it touches on disk it only reads. So a container with a read-only root filesystem already works -- except that `VOLUME ["/data"]` quietly undid it. Docker acts on that directive: a container started without `-v` gets an anonymous volume mounted there anyway, writable even under `--read-only`. It persisted nothing across a redeploy, since each new container got a fresh empty volume, and it left an orphan behind every time one was replaced. Deployments that want the sessions to survive already say so themselves -- docker-compose.yml and deploy.example.sh both mount a named volume -- so removing the line changes nothing for them. IMMUTABLE=1 asserts that this is how the instance is running. It is checked rather than believed: the server refuses to start if SESSION_FILE is still set, or if the filesystem it is installed on turns out to be writable. Left unchecked the misconfiguration is silent, because persisting sessions is best-effort -- a read-only /data costs one warning at the first sign-in and nothing more until the instance is replaced and everyone is signed out. SessionBackend names what the rest of the server asks of a session store, and `sessions` in app.ts is typed as it. Nothing changes today; SessionStore is still the only implementation. It is there so the OAuth work is written against the interface rather than the class, and so the interface can record which of its methods a stateless backend could satisfy alone: create, resolve, reseal and destroy each touch one session, while listForUser and destroyAllForUser have to reach sessions other than the caller's. The second of those carries the guarantee that changing a password invalidates the sessions still holding the old one, which is why it needs a registry -- Stalwart's token registry, once sign-in goes through OAuth. --- .env.example | 12 +++++++ Dockerfile | 10 +++++- README.md | 23 ++++++++++++++ server/src/app.ts | 4 +-- server/src/config.test.ts | 40 +++++++++++++++++++++++ server/src/config.ts | 58 +++++++++++++++++++++++++++++++-- server/src/sessions.ts | 67 +++++++++++++++++++++++++++++++++------ 7 files changed, 200 insertions(+), 14 deletions(-) create mode 100644 server/src/config.test.ts diff --git a/.env.example b/.env.example index 29f3bae..2758fa5 100644 --- a/.env.example +++ b/.env.example @@ -28,8 +28,20 @@ SESSION_TTL=43200 SESSION_REMEMBER_TTL=2592000 # Where to persist sessions so restarts don't log everyone out (optional). +# Leave it empty to hold sessions in memory only, which is what an immutable +# instance does -- see IMMUTABLE below. SESSION_FILE=./data/sessions.json +# Assert that this instance is running as an immutable container: read-only +# root filesystem, no durable state of its own. It is checked rather than +# taken on trust -- the server refuses to start if SESSION_FILE is set, or if +# the filesystem it is installed on turns out to be writable. Off by default. +# Running one looks like: +# docker run --read-only --tmpfs /tmp -e IMMUTABLE=1 -e SESSION_FILE= ... +# The cost today is that a restart signs everyone out, since there is nowhere +# left to keep the sessions. Removing that cost is what the OAuth work is for. +# IMMUTABLE=1 + # Upstream timeouts / limits UPSTREAM_TIMEOUT=30000 MAX_UPLOAD_BYTES=52428800 diff --git a/Dockerfile b/Dockerfile index b9cc1a6..952700c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -37,7 +37,15 @@ COPY --from=build /app/server/dist ./server/dist COPY --from=build /app/web/dist ./web/dist RUN mkdir -p /data && chown -R node:node /data /app USER node -VOLUME ["/data"] +# No `VOLUME ["/data"]`. It reads like documentation for where the session file +# goes, but Docker acts on it: a container started without `-v` gets an +# anonymous volume mounted there anyway, and that mount stays writable even +# under `--read-only`. So the directive quietly put a writable hole in a +# container meant to be immutable, and left an orphaned volume behind every +# time one was replaced -- while never persisting anything across a redeploy, +# since each new container got a fresh empty volume of its own. Deployments +# that want the sessions to survive say so themselves: docker-compose.yml and +# deploy.example.sh both mount a *named* volume at /data, which is unaffected. EXPOSE 8080 HEALTHCHECK --interval=30s --timeout=5s CMD wget -qO- http://127.0.0.1:8080/api/health || exit 1 CMD ["node", "server/dist/index.js"] diff --git a/README.md b/README.md index 57cbc03..463af00 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,29 @@ Full instructions, TLS, and every environment variable: [Installing](https://docs.ihasmail.org/install/) · [Configuring](https://docs.ihasmail.org/configure/). +### Running immutably + +The server writes to exactly one path, the optional `SESSION_FILE`. Clear it +and there is nothing left to write, so the container can run with no writable +filesystem at all: + +```bash +docker run --read-only --tmpfs /tmp -e IMMUTABLE=1 -e SESSION_FILE= ... +``` + +`IMMUTABLE=1` is an assertion the server checks at startup rather than a switch +that changes what it does: it refuses to start if `SESSION_FILE` is still set, +or if the filesystem it is installed on turns out to be writable after all. +Without it the same misconfiguration is silent — sessions are held in memory +and persisting them is best-effort, so a read-only `/data` costs one warning at +the first sign-in and nothing else until the instance is replaced and everyone +is signed out. + +That sign-out is the standing cost of this mode today, since sessions have +nowhere to live across a restart. Removing it means moving the session upstream +into a token Stalwart itself issues and can revoke, which is what the OAuth work +in [ROADMAP.md](ROADMAP.md) is for. + ## Architecture ``` diff --git a/server/src/app.ts b/server/src/app.ts index f0dab0f..6797394 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -3,7 +3,7 @@ import type { Context, MiddlewareHandler } from "hono"; import { getCookie, setCookie, deleteCookie } from "hono/cookie"; import { getConnInfo } from "@hono/node-server/conninfo"; import { config } from "./config.js"; -import { SessionStore, type LiveSession } from "./sessions.js"; +import { SessionStore, type SessionBackend, type LiveSession } from "./sessions.js"; import { RateLimiter } from "./ratelimit.js"; import { resolveClientIp } from "./clientip.js"; import { @@ -34,7 +34,7 @@ import { staticHandler } from "./static.js"; type Env = { Variables: { session: LiveSession } }; -export const sessions = new SessionStore(config.sessionFile); +export const sessions: SessionBackend = new SessionStore(config.sessionFile); const loginLimiter = new RateLimiter(config.loginRateLimit, 15 * 60_000); /** * Credential changes verify the current password upstream, and Stalwart's diff --git a/server/src/config.test.ts b/server/src/config.test.ts new file mode 100644 index 0000000..5dae676 --- /dev/null +++ b/server/src/config.test.ts @@ -0,0 +1,40 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { chmodSync, existsSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { assertImmutable } from "./config.js"; + +function tempRoot(): string { + return mkdtempSync(join(tmpdir(), "ihasmail-immutable-")); +} + +test("IMMUTABLE refuses a configured SESSION_FILE", () => { + const root = tempRoot(); + try { + assert.throws(() => assertImmutable("/data/sessions.json", root), /SESSION_FILE is \/data\/sessions\.json/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("IMMUTABLE refuses a writable root, and leaves no probe behind", () => { + const root = tempRoot(); + try { + assert.throws(() => assertImmutable("", root), /is writable/); + assert.equal(existsSync(join(root, ".immutable-probe")), false); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("IMMUTABLE accepts a root it cannot write to", () => { + const root = tempRoot(); + try { + chmodSync(root, 0o555); + assert.doesNotThrow(() => assertImmutable("", root)); + } finally { + chmodSync(root, 0o755); + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/server/src/config.ts b/server/src/config.ts index ff5a14a..49658a0 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -1,7 +1,7 @@ import { resolveVersion } from "../../scripts/version.mjs"; import { randomBytes } from "node:crypto"; import { fileURLToPath } from "node:url"; -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; import { resolve } from "node:path"; /** Minimal .env loader (no dependency): first match wins, never overrides real env. */ @@ -58,6 +58,58 @@ if (!appSecret || appSecret === "change-me") { const stalwartUrl = env("STALWART_URL", "https://mail.example.com").replace(/\/+$/, ""); +/** + * Declares that this instance is running as an immutable container: read-only + * root filesystem, nothing durable of its own, replaceable by its image. + * + * It is a claim the process checks rather than one it takes on trust, because + * the failure it guards against is silent. Left to itself the server survives + * a read-only filesystem perfectly well -- sessions are held in memory and the + * write is best-effort, so the only sign that `SESSION_FILE` is going nowhere + * is one warning at the first login, long after anyone was watching. The + * instance looks healthy right up until it is replaced and everyone is signed + * out. Setting IMMUTABLE turns both halves of that into a refusal to start. + */ +const immutable = bool("IMMUTABLE", false); +const sessionFile = process.env.SESSION_FILE ?? ""; + +/** + * Refuse to run when the promise IMMUTABLE makes is not one this instance can + * keep. Exported so it can be tested without a read-only filesystem to hand. + */ +export function assertImmutable(sessionFile: string, root: string): void { + // The image sets SESSION_FILE=/data/sessions.json, so this is a deliberate + // refusal rather than a formality: running immutably means clearing it. It + // is not quietly ignored, because a configured path that silently persists + // nothing is exactly the failure this flag exists to surface. + if (sessionFile) { + throw new Error( + `IMMUTABLE is set, but SESSION_FILE is ${sessionFile}. An immutable instance keeps no durable state of its own: ` + + "pass SESSION_FILE= (empty) to hold sessions in memory, or unset IMMUTABLE.", + ); + } + // And check the property itself, not just the intention to have it. Setting + // the variable while forgetting `--read-only` is the easy mistake, and it + // leaves an instance claiming a guarantee it does not have. + const probe = resolve(root, ".immutable-probe"); + let writable = false; + try { + writeFileSync(probe, ""); + writable = true; + unlinkSync(probe); + } catch { + /* EROFS, or EACCES on a root we do not own: either way, not writable by us */ + } + if (writable) { + throw new Error( + `IMMUTABLE is set, but ${root} is writable. Run the container with --read-only (and --tmpfs /tmp), or unset IMMUTABLE.`, + ); + } +} + +if (immutable) assertImmutable(sessionFile, fileURLToPath(new URL("../..", import.meta.url))); + + export const config = { isProd, appName: env("APP_NAME", "ihasmail"), @@ -92,7 +144,9 @@ export const config = { secureCookies: (process.env.SECURE_COOKIES ?? "auto").toLowerCase(), sessionTtl: int("SESSION_TTL", 12 * 60 * 60), sessionRememberTtl: int("SESSION_REMEMBER_TTL", 30 * 24 * 60 * 60), - sessionFile: process.env.SESSION_FILE ?? "", + sessionFile, + /** True when this instance has asserted, and verified, that it is immutable. */ + immutable, upstreamTimeout: int("UPSTREAM_TIMEOUT", 30_000), maxUploadBytes: int("MAX_UPLOAD_BYTES", 50 * 1024 * 1024), imageProxy: bool("IMAGE_PROXY", true), diff --git a/server/src/sessions.ts b/server/src/sessions.ts index dbeb7f1..e8d9953 100644 --- a/server/src/sessions.ts +++ b/server/src/sessions.ts @@ -34,9 +34,64 @@ export interface LiveSession { ip: string; } +/** What `/api/auth/sessions` reports about a session, with nothing secret in it. */ +export interface SessionSummary { + id: string; + username: string; + createdAt: number; + lastSeenAt: number; + expiresAt: number; + remember: boolean; + userAgent: string; + ip: string; +} + +export interface CreateSessionParams { + username: string; + password: string; + remember: boolean; + userAgent: string; + ip: string; +} + +/** + * Everything the rest of the server asks of a session store. + * + * There is one implementation today -- `SessionStore` below, which keeps the + * records in memory and optionally mirrors them to `SESSION_FILE`. The reason + * it is named as an interface anyway is that a second one is planned: a + * stateless backend that carries the whole record in the cookie, so that a + * replica can serve a session it never issued and `/data` can go away. Callers + * written against the concrete class would all have to be revisited then. + * + * Five of these are already stateless in shape -- `create`, `resolve`, + * `reseal` and `destroy` each touch exactly one session, and the sealing key is + * derived from the cookie secret (see `crypto.ts`), so the record can move into + * the cookie without the server keeping a map. + * + * The other two cannot be. `listForUser` and `destroyAllForUser` have to reach + * sessions other than the one presenting itself, which means something has to + * be enumerable somewhere. `destroyAllForUser` is not only the "sign out my + * other sessions" button: `app.ts` also calls it when the password or the app + * password changes, so it carries the guarantee that changing a credential + * invalidates the sessions still holding the old one. A stateless backend + * cannot honour that alone; the plan is for OAuth to hand the job to + * Stalwart's own token registry, which can already answer both questions. + */ +export interface SessionBackend { + init(): Promise; + close(): Promise; + create(params: CreateSessionParams): { cookie: string; session: LiveSession }; + resolve(cookie: string | undefined): LiveSession | null; + reseal(cookie: string | undefined, password: string): boolean; + destroy(id: string): void; + destroyAllForUser(username: string, exceptId?: string): number; + listForUser(username: string): SessionSummary[]; +} + const COOKIE_SEP = "."; -export class SessionStore { +export class SessionStore implements SessionBackend { private sessions = new Map(); private dirty = false; private saveTimer: NodeJS.Timeout | null = null; @@ -104,13 +159,7 @@ export class SessionStore { } /** Create a session; returns the cookie value to hand to the client. */ - create(params: { - username: string; - password: string; - remember: boolean; - userAgent: string; - ip: string; - }): { cookie: string; session: LiveSession } { + create(params: CreateSessionParams): { cookie: string; session: LiveSession } { const id = randomToken(18); const secret = randomToken(32); const salt = randomBytes(16); @@ -211,7 +260,7 @@ export class SessionStore { return n; } - listForUser(username: string): Array> { + listForUser(username: string): SessionSummary[] { const out = []; for (const s of this.sessions.values()) { if (s.username !== username) continue;