From 32ac2cfa693c8ee1f130bcac49aef66982087959 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 02:58:24 +0300 Subject: [PATCH 01/12] feat(storage): add the versioned credential envelope and its derived key Per D4: once a stored value's first colon-separated segment matches a version tag (^v\d+$), it is classified as an envelope claim. Every way that claim can be malformed - wrong segment count, unrecognised version, bad IV, failed authentication - resolves to undecryptable, never to plaintext. Only a value whose first segment is not version-tag-shaped is plaintext. Per D3, the accepted cost is that an existing plaintext password shaped exactly ^v\d+: becomes unreadable after this ships. --- src/lib/storage/encryption.ts | 156 +++++++++++++++ tests/unit/lib/storage/encryption.test.ts | 221 ++++++++++++++++++++++ 2 files changed, 377 insertions(+) create mode 100644 src/lib/storage/encryption.ts create mode 100644 tests/unit/lib/storage/encryption.test.ts diff --git a/src/lib/storage/encryption.ts b/src/lib/storage/encryption.ts new file mode 100644 index 00000000..db11dfa9 --- /dev/null +++ b/src/lib/storage/encryption.ts @@ -0,0 +1,156 @@ +import { createCipheriv, createDecipheriv, hkdfSync, randomBytes } from "node:crypto"; +import { getJwtSecret, JWT_SECRET_MIN_LENGTH } from "@/lib/config/auth-env"; + +/** + * Credential encryption at rest for the SERVER-SIDE store (STORAGE_PROVIDER=sqlite|postgres). + * + * What this buys and what it does not: a leaked database file or dump is useless on its own, + * because the key lives in the environment and never in the store. It is NOT a vault - anyone who + * can read the process environment can read the credentials, and the browser's localStorage copy + * stays plaintext by deliberate product decision (that is what lets Studio work without a master + * password, and it is why the XSS controls carry the weight they do). + * + * Key derivation: + * STORAGE_ENCRYPTION_KEY, when set -> HKDF-SHA256 -> 32 bytes + * otherwise -> JWT_SECRET -> HKDF-SHA256 -> 32 bytes + * + * Deriving from JWT_SECRET is what keeps the zero-config promise every distribution channel + * depends on: no new mandatory variable, and the auth bootstrap (src/lib/auth-bootstrap.ts:199) + * already generates and persists a JWT_SECRET on first run, so even an operator who configured + * nothing has a stable key across restarts. The cost, documented in docs/STORAGE.md, is that + * rotating JWT_SECRET invalidates every stored credential. + * + * The salt is empty and the domain separation is carried entirely by `info`, which is the + * canonical single-purpose HKDF shape; RFC 5869 section 3.1 explicitly permits an absent salt. + */ + +const ALGORITHM = "aes-256-gcm"; +const KEY_BYTES = 32; +/** 96 bits: the IV length GCM is specified for, and the only one that needs no rehashing. */ +const IV_BYTES = 12; +const TAG_BYTES = 16; + +export const ENVELOPE_VERSION = "v1"; +/** Any `vN` tag, so a value written by a future version is recognised as an envelope, not as text. */ +const VERSION_TAG = /^v\d+$/; +const HKDF_INFO = "libredb-studio/storage-encryption/v1"; +const HKDF_SALT = new Uint8Array(0); + +// Single-line messages, hoisted to module scope: bun's line coverage under-counts the continuation +// lines of a wrapped string literal, which then reads as uncovered code. Same reason as +// src/lib/config/auth-env.ts:20. +export const STORAGE_ENCRYPTION_KEY_TOO_SHORT_MESSAGE = + "STORAGE_ENCRYPTION_KEY is too short; it must be at least 32 characters. Update it and restart the server."; +export const STORAGE_ENCRYPTION_KEY_MISSING_MESSAGE = + "Server storage cannot encrypt credentials: neither STORAGE_ENCRYPTION_KEY nor JWT_SECRET is configured. Set one (at least 32 characters) and restart the server."; + +/** + * Derived once per process. A key is on the write path of every storage push, so re-deriving per + * call would put an HKDF on a hot path for no benefit: neither source variable can change without + * a restart in any deployment this product ships to. + */ +let cachedKey: Buffer | null = null; + +/** Test seam: clears the derived key so each case observes a fresh process. */ +export function resetStorageEncryptionKey(): void { + cachedKey = null; +} + +function inputKeyMaterial(): Uint8Array { + const explicit = process.env.STORAGE_ENCRYPTION_KEY; + if (explicit) { + // The same floor as JWT_SECRET, from the same constant: an operator who sets a dedicated key + // must not end up with weaker material than the fallback they were trying to improve on. + if (explicit.length < JWT_SECRET_MIN_LENGTH) { + throw new Error(STORAGE_ENCRYPTION_KEY_TOO_SHORT_MESSAGE); + } + return new TextEncoder().encode(explicit); + } + // allowDevFallback stays at its default (true): forcing it off would break + // STORAGE_PROVIDER=sqlite under `bun dev` with no JWT_SECRET, which is exactly the zero-config + // path this control must not break. In production the fallback does not apply and this throws. + return getJwtSecret({ missingMessage: STORAGE_ENCRYPTION_KEY_MISSING_MESSAGE }); +} + +function encryptionKey(): Buffer { + if (!cachedKey) { + cachedKey = Buffer.from(hkdfSync("sha256", inputKeyMaterial(), HKDF_SALT, HKDF_INFO, KEY_BYTES)); + } + return cachedKey; +} + +/** + * Seals a credential into `v1::`. + * + * The GCM authentication tag is appended to the ciphertext segment rather than given a fourth + * segment: the stored shape is a fixed contract and has three parts. Dropping the tag instead + * would turn this into unauthenticated encryption, where a flipped byte yields a different, + * silently wrong password rather than a detected failure. + * + * Throws when no usable key material exists. That is deliberate and fails CLOSED: a write that + * could not encrypt must not fall back to writing the credential in the clear. The caller is + * PUT /api/storage/connections, whose 500 the sync hook reports as syncError without blocking the + * UI (docs/STORAGE.md, "Graceful Degradation"). + */ +export function encryptSecret(plaintext: string): string { + const iv = randomBytes(IV_BYTES); + const cipher = createCipheriv(ALGORITHM, encryptionKey(), iv); + const body = Buffer.concat([cipher.update(plaintext, "utf8"), cipher.final()]); + const sealed = Buffer.concat([body, cipher.getAuthTag()]); + return `${ENVELOPE_VERSION}:${iv.toString("base64url")}:${sealed.toString("base64url")}`; +} + +export type SecretReadResult = + | { kind: "plaintext"; value: string } + | { kind: "decrypted"; value: string } + | { kind: "undecryptable" }; + +/** + * Classifies a stored value. Three outcomes, not two, and the third is the important one: + * + * - `plaintext` the value's first segment does not look like a version tag at all - this + * predates the feature (lazy migration; the next write envelopes it) + * - `decrypted` a v1 envelope this key opens + * - `undecryptable` anything whose first segment DOES look like a version tag but is not a valid, + * openable v1 envelope: the wrong number of segments, an unrecognised version, a + * malformed IV or body, or a body that fails authentication. The value is NEVER + * returned in this case. Handing back the raw envelope would put "v1:abc:def" + * into a driver's password field, producing a connection failure with no + * diagnosable cause. + * + * D4 governs the boundary between the first and third outcomes: once `parts[0]` matches `^v\d+$`, + * the value is treated as an envelope CLAIM, and every way that claim can be malformed resolves to + * `undecryptable` - never to plaintext, even for a two-segment value. The plan already accepts this + * exposure for three-segment values (a legitimate password shaped "v1:a:b" is rejected today + * because it decodes to garbage and fails authentication); treating the two-segment case as + * plaintext instead would protect nothing while opening a hole that corruption - a truncated or + * partially written envelope - passes straight through as if it were a real password. The accepted + * cost, per D3: an existing plaintext password shaped exactly `^v\d+:` (for example "v1:x") becomes + * permanently unreadable once this ships, surfacing as an empty field the user retypes rather than + * as a deleted record. + */ +export function readSecret(stored: string): SecretReadResult { + const parts = stored.split(":"); + if (!VERSION_TAG.test(parts[0])) { + return { kind: "plaintext", value: stored }; + } + if (parts.length !== 3 || parts[0] !== ENVELOPE_VERSION) { + return { kind: "undecryptable" }; + } + const iv = Buffer.from(parts[1], "base64url"); + const sealed = Buffer.from(parts[2], "base64url"); + if (iv.length !== IV_BYTES || sealed.length < TAG_BYTES) { + return { kind: "undecryptable" }; + } + try { + const decipher = createDecipheriv(ALGORITHM, encryptionKey(), iv); + decipher.setAuthTag(sealed.subarray(sealed.length - TAG_BYTES)); + const opened = Buffer.concat([decipher.update(sealed.subarray(0, sealed.length - TAG_BYTES)), decipher.final()]); + return { kind: "decrypted", value: opened.toString("utf8") }; + } catch { + // A wrong key, a rotated secret, a truncated or tampered value, or missing key material all + // land here. Reads never throw: see docs/STORAGE.md - an unreadable password must not take the + // user's query history, saved queries and charts down with it. + return { kind: "undecryptable" }; + } +} diff --git a/tests/unit/lib/storage/encryption.test.ts b/tests/unit/lib/storage/encryption.test.ts new file mode 100644 index 00000000..dc6f74f3 --- /dev/null +++ b/tests/unit/lib/storage/encryption.test.ts @@ -0,0 +1,221 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + ENVELOPE_VERSION, + encryptSecret, + readSecret, + resetStorageEncryptionKey, + STORAGE_ENCRYPTION_KEY_MISSING_MESSAGE, + STORAGE_ENCRYPTION_KEY_TOO_SHORT_MESSAGE, +} from "@/lib/storage/encryption"; + +/** + * The envelope's job is narrow: make a stolen database file useless on its own, and never hand a + * caller something that is not the credential. WHICH fields go through it is Task 3's question. + */ + +const MUTATED = ["STORAGE_ENCRYPTION_KEY", "JWT_SECRET", "NODE_ENV"] as const; +const snapshot: Record = {}; + +beforeEach(() => { + for (const key of MUTATED) snapshot[key] = process.env[key]; + process.env.JWT_SECRET = "jwt-secret-used-only-by-this-test-file"; + delete process.env.STORAGE_ENCRYPTION_KEY; + resetStorageEncryptionKey(); +}); + +afterEach(() => { + for (const key of MUTATED) { + const value = snapshot[key]; + if (value === undefined) delete process.env[key]; + else (process.env as Record)[key] = value; + } + resetStorageEncryptionKey(); +}); + +describe("encryptSecret", () => { + test("a dump of the ciphertext contains no fragment of the credential", () => { + const sealed = encryptSecret("correct horse battery staple"); + + expect(sealed).not.toContain("correct"); + expect(sealed).not.toContain("staple"); + expect(sealed).not.toContain("horse battery"); + }); + + test("produces the three-part versioned envelope the storage contract promises", () => { + const parts = encryptSecret("hunter2").split(":"); + + expect(parts).toHaveLength(3); + expect(parts[0]).toBe(ENVELOPE_VERSION); + // base64url only, so the ':' separator can never be ambiguous and the value stays JSON-safe. + expect(parts[1]).toMatch(/^[A-Za-z0-9_-]+$/); + expect(parts[2]).toMatch(/^[A-Za-z0-9_-]+$/); + }); + + test("never repeats an IV, so two equal passwords do not look equal in the store", () => { + const first = encryptSecret("same-password"); + const second = encryptSecret("same-password"); + + expect(first).not.toBe(second); + expect(first.split(":")[1]).not.toBe(second.split(":")[1]); + }); + + test("round-trips a value carrying every character a password is allowed to contain", () => { + const nasty = 'p@ss:w/o?rd#=+ "quoted" newline-and-tab: ' + String.fromCharCode(10, 9) + " accented e"; + + expect(readSecret(encryptSecret(nasty))).toEqual({ kind: "decrypted", value: nasty }); + }); + + test("round-trips an empty string rather than treating it as absent", () => { + expect(readSecret(encryptSecret(""))).toEqual({ kind: "decrypted", value: "" }); + }); +}); + +describe("readSecret: what a stored value is", () => { + test("a value written before this feature existed is plaintext, not corruption", () => { + expect(readSecret("legacy-plaintext-password")).toEqual({ + kind: "plaintext", + value: "legacy-plaintext-password", + }); + }); + + test("a colon-bearing plaintext that is not a version tag stays plaintext", () => { + expect(readSecret("host:5432:db")).toEqual({ kind: "plaintext", value: "host:5432:db" }); + }); + + test("a connection-string plaintext with many colons and no version-tag prefix stays plaintext", () => { + const connectionString = "postgresql://u:p@h:5432/db"; + + expect(readSecret(connectionString)).toEqual({ kind: "plaintext", value: connectionString }); + }); + + // D4: once the first segment matches a version tag (^v\d+$), the value is an envelope CLAIM, and + // every way that claim can be malformed - including the wrong segment count - is undecryptable, + // never plaintext. The plan already accepts this exposure for three-segment values (a legitimate + // password shaped "v1:a:b" is rejected today); a laxer rule for the two-segment case would protect + // nothing while letting corruption pass straight through as if it were a real password. + test("a version-tag-shaped value with too few segments is corruption, not plaintext", () => { + expect(readSecret("v1:only-two-parts")).toEqual({ kind: "undecryptable" }); + }); + + test("a version-tag-shaped value with an empty second segment is corruption, not plaintext", () => { + expect(readSecret("v1:")).toEqual({ kind: "undecryptable" }); + }); + + test("a version-tag-shaped value with too many segments is corruption, not plaintext", () => { + expect(readSecret("v1:a:b:c")).toEqual({ kind: "undecryptable" }); + }); + + test("an unrecognised version with the wrong segment count is corruption too, not plaintext", () => { + expect(readSecret("v2:abc")).toEqual({ kind: "undecryptable" }); + }); + + test("an envelope written by a NEWER version is never handed back as if it were the password", () => { + // The failure this pins: returning the raw string would put "v2:aaaa:bbbb" into a driver's + // password field on a downgrade, which fails in a way nobody can diagnose. + expect(readSecret("v2:aaaa:bbbb")).toEqual({ kind: "undecryptable" }); + }); + + test("a v1 envelope with a wrong-length IV is corruption, not plaintext", () => { + expect(readSecret("v1:AAAA:BBBBBBBBBBBBBBBBBBBBBBBB")).toEqual({ kind: "undecryptable" }); + }); + + test("a v1 envelope too short to hold an authentication tag is corruption", () => { + const iv = encryptSecret("x").split(":")[1]; + + expect(readSecret(`v1:${iv}:AAAA`)).toEqual({ kind: "undecryptable" }); + }); + + test("a truncated ciphertext fails authentication instead of returning a partial credential", () => { + const sealed = encryptSecret("a-long-enough-password-to-truncate"); + const [version, iv, body] = sealed.split(":"); + + expect(readSecret(`${version}:${iv}:${body.slice(0, body.length - 4)}`)).toEqual({ + kind: "undecryptable", + }); + }); + + test("a flipped ciphertext byte is rejected by the tag, not silently returned", () => { + const sealed = encryptSecret("tamper-me"); + const [version, iv, body] = sealed.split(":"); + const bytes = Buffer.from(body, "base64url"); + bytes[0] ^= 0xff; // a CIPHERTEXT byte: the 16-byte tag occupies the trailing indices + const flipped = bytes.toString("base64url"); + + expect(readSecret(`${version}:${iv}:${flipped}`)).toEqual({ kind: "undecryptable" }); + }); + + test("a flipped tag byte is rejected too, not just a flipped ciphertext byte", () => { + const sealed = encryptSecret("tamper-me"); + const [version, iv, body] = sealed.split(":"); + const bytes = Buffer.from(body, "base64url"); + bytes[bytes.length - 1] ^= 0xff; // a TAG byte: the trailing 16 bytes of the sealed body + const flipped = bytes.toString("base64url"); + + expect(readSecret(`${version}:${iv}:${flipped}`)).toEqual({ kind: "undecryptable" }); + }); + + test("a rotated JWT_SECRET makes the value undecryptable, never wrongly decrypted", () => { + const sealed = encryptSecret("password-under-the-old-secret"); + + process.env.JWT_SECRET = "a-completely-different-secret-value-32"; + resetStorageEncryptionKey(); + + expect(readSecret(sealed)).toEqual({ kind: "undecryptable" }); + }); +}); + +describe("the key", () => { + test("an explicit STORAGE_ENCRYPTION_KEY takes over from JWT_SECRET", () => { + process.env.STORAGE_ENCRYPTION_KEY = "a-dedicated-storage-key-of-enough-length"; + resetStorageEncryptionKey(); + const sealed = encryptSecret("separated"); + + delete process.env.STORAGE_ENCRYPTION_KEY; + resetStorageEncryptionKey(); + + // Key separation is the whole point of the variable: falling back to JWT_SECRET here would + // mean the dedicated key never actually separated anything. + expect(readSecret(sealed)).toEqual({ kind: "undecryptable" }); + }); + + test("the same explicit key reads back what it wrote", () => { + process.env.STORAGE_ENCRYPTION_KEY = "a-dedicated-storage-key-of-enough-length"; + resetStorageEncryptionKey(); + const sealed = encryptSecret("separated"); + resetStorageEncryptionKey(); + + expect(readSecret(sealed)).toEqual({ kind: "decrypted", value: "separated" }); + }); + + test("a short STORAGE_ENCRYPTION_KEY is refused rather than quietly stretched", () => { + process.env.STORAGE_ENCRYPTION_KEY = "too-short"; + resetStorageEncryptionKey(); + + expect(() => encryptSecret("x")).toThrow(STORAGE_ENCRYPTION_KEY_TOO_SHORT_MESSAGE); + }); + + test("a missing JWT_SECRET in production refuses to write rather than writing plaintext", () => { + delete process.env.JWT_SECRET; + (process.env as Record).NODE_ENV = "production"; + resetStorageEncryptionKey(); + + expect(() => encryptSecret("x")).toThrow(STORAGE_ENCRYPTION_KEY_MISSING_MESSAGE); + }); + + test("a plaintext read needs no key at all, so a broken key never hides existing data", () => { + delete process.env.JWT_SECRET; + (process.env as Record).NODE_ENV = "production"; + resetStorageEncryptionKey(); + + expect(readSecret("legacy-plaintext")).toEqual({ kind: "plaintext", value: "legacy-plaintext" }); + }); + + test("a broken key turns an enveloped read into undecryptable, not into a thrown request", () => { + const sealed = encryptSecret("x"); + delete process.env.JWT_SECRET; + (process.env as Record).NODE_ENV = "production"; + resetStorageEncryptionKey(); + + expect(readSecret(sealed)).toEqual({ kind: "undecryptable" }); + }); +}); From 0ed0518671833adb6f2b3d39d63fdb5891102028 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 03:34:25 +0300 Subject: [PATCH 02/12] feat(storage): classify every connection field and seal the six credential-bearing ones --- src/lib/storage/connection-secrets.ts | 167 ++++++++++++ .../lib/storage/connection-secrets.test.ts | 257 ++++++++++++++++++ 2 files changed, 424 insertions(+) create mode 100644 src/lib/storage/connection-secrets.ts create mode 100644 tests/unit/lib/storage/connection-secrets.test.ts diff --git a/src/lib/storage/connection-secrets.ts b/src/lib/storage/connection-secrets.ts new file mode 100644 index 00000000..ae068e42 --- /dev/null +++ b/src/lib/storage/connection-secrets.ts @@ -0,0 +1,167 @@ +import { encryptSecret, readSecret } from "./encryption"; +import type { DatabaseConnection, SSHTunnelConfig, SSLConfig } from "@/lib/types"; + +/** + * The single answer to "which stored fields are credentials". + * + * Why these are `Record` maps and not a list of secret field names: lazy + * migration only re-writes what it reads, so a credential-bearing field nobody remembered to add + * to a list stays in the clear forever in every existing deployment. A list fails silently. These + * maps fail at `bun run typecheck` - add a field to DatabaseConnection, SSLConfig or + * SSHTunnelConfig and the object literal below stops satisfying its type until the new field is + * classified. A field has to opt OUT of scrutiny, deliberately; there is no opt-in step to forget. + * + * `nested` marks a container whose own map carries the classification, so "I did not think about + * it" and "it is a container" are different answers rather than the same silence. + */ +export type FieldClass = "secret" | "public" | "nested"; + +export const CONNECTION_FIELDS: Record = { + id: "public", + name: "public", + type: "public", + host: "public", + port: "public", + user: "public", + password: "secret", + database: "public", + // Carries scheme://user:pass@host. The same shape src/lib/audit.ts redacts out of log lines. + connectionString: "secret", + createdAt: "public", + color: "public", + environment: "public", + group: "public", + ssl: "nested", + sshTunnel: "nested", + serviceName: "public", + instanceName: "public", + managed: "public", + seedId: "public", +}; + +export const SSL_FIELDS: Record = { + mode: "public", + // Certificates are public by construction; encrypting them costs diagnosability and buys nothing. + caCert: "public", + clientCert: "public", + clientKey: "secret", + rejectUnauthorized: "public", +}; + +export const SSH_TUNNEL_FIELDS: Record = { + enabled: "public", + host: "public", + port: "public", + username: "public", + authMethod: "public", + password: "secret", + privateKey: "secret", + // Encrypting the private key and leaving the passphrase that unlocks it readable protects nothing. + passphrase: "secret", +}; + +/** Derived, never written out twice: a second hand-maintained list is a second thing that drifts. */ +function secretsOf(map: Record): string[] { + return Object.keys(map).filter((key) => map[key] === "secret"); +} + +const CONNECTION_SECRET_KEYS = secretsOf(CONNECTION_FIELDS); +const SSL_SECRET_KEYS = secretsOf(SSL_FIELDS); +const SSH_TUNNEL_SECRET_KEYS = secretsOf(SSH_TUNNEL_FIELDS); + +/** + * Applies `transform` to each named field of `target` in place. A transform returning `undefined` + * DELETES the field rather than storing undefined, and is counted. Returns the number deleted. + * + * An empty string is skipped: there is nothing to protect, and enveloping it would turn "no + * password set" into a value the UI renders as a filled-in field. + */ +function mapSecretFields( + target: Record, + keys: string[], + transform: (value: string) => string | undefined, +): number { + let dropped = 0; + for (const key of keys) { + const value = target[key]; + if (typeof value !== "string" || value.length === 0) continue; + const next = transform(value); + if (next === undefined) { + delete target[key]; + dropped += 1; + } else { + target[key] = next; + } + } + return dropped; +} + +/** + * Walks one connection's three field groups. Both directions share this so the encrypt and decrypt + * paths can never disagree about WHICH fields they cover - the classic way a round trip loses a + * field is two walkers with two opinions. + */ +function walkConnection( + connection: DatabaseConnection, + transform: (value: string) => string | undefined, +): { connection: DatabaseConnection; dropped: number } { + const copy: Record = { ...connection }; + let dropped = mapSecretFields(copy, CONNECTION_SECRET_KEYS, transform); + + if (copy.ssl) { + const ssl = { ...(copy.ssl as Record) }; + dropped += mapSecretFields(ssl, SSL_SECRET_KEYS, transform); + copy.ssl = ssl; + } + if (copy.sshTunnel) { + const tunnel = { ...(copy.sshTunnel as Record) }; + dropped += mapSecretFields(tunnel, SSH_TUNNEL_SECRET_KEYS, transform); + copy.sshTunnel = tunnel; + } + + return { connection: copy as unknown as DatabaseConnection, dropped }; +} + +/** Seals a plaintext value; leaves an already-sealed one alone so a re-save is not a re-encryption. */ +function sealIfPlaintext(value: string): string { + return readSecret(value).kind === "plaintext" ? encryptSecret(value) : value; +} + +/** Opens a sealed value, passes a legacy plaintext through, and returns undefined for a dead one. */ +function openOrDrop(value: string): string | undefined { + const result = readSecret(value); + return result.kind === "undecryptable" ? undefined : result.value; +} + +/** Every write goes through this. Never returns a connection with a plaintext credential in it. */ +export function encryptConnections(connections: DatabaseConnection[]): DatabaseConnection[] { + return connections.map((connection) => walkConnection(connection, sealIfPlaintext).connection); +} + +export interface ConnectionReadResult { + connections: DatabaseConnection[]; + /** How many secret fields could not be opened. The caller reports it once, not per field. */ + undecryptable: number; +} + +/** + * Every read goes through this. An unreadable field is OMITTED and the record kept: + * + * - Throwing would empty all ten collections for a rotated key, taking the user's query history, + * saved queries, charts and snapshots down with the passwords. + * - Dropping the record would be worse. useStorageSync is a write-through cache, so a connection + * missing from a read is persisted as a deletion on the next push - destroying ciphertext that a + * restored key could still have opened. + * + * Omission leaves an empty password box the user can retype, and the next sync re-seals it under + * the current key. + */ +export function decryptConnections(connections: DatabaseConnection[]): ConnectionReadResult { + let undecryptable = 0; + const opened = connections.map((connection) => { + const result = walkConnection(connection, openOrDrop); + undecryptable += result.dropped; + return result.connection; + }); + return { connections: opened, undecryptable }; +} diff --git a/tests/unit/lib/storage/connection-secrets.test.ts b/tests/unit/lib/storage/connection-secrets.test.ts new file mode 100644 index 00000000..66e10a53 --- /dev/null +++ b/tests/unit/lib/storage/connection-secrets.test.ts @@ -0,0 +1,257 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + CONNECTION_FIELDS, + decryptConnections, + encryptConnections, + SSH_TUNNEL_FIELDS, + SSL_FIELDS, +} from "@/lib/storage/connection-secrets"; +import { ENVELOPE_VERSION, resetStorageEncryptionKey } from "@/lib/storage/encryption"; +import type { DatabaseConnection } from "@/lib/types"; + +const snapshot: Record = {}; + +beforeEach(() => { + snapshot.JWT_SECRET = process.env.JWT_SECRET; + snapshot.STORAGE_ENCRYPTION_KEY = process.env.STORAGE_ENCRYPTION_KEY; + process.env.JWT_SECRET = "connection-secrets-test-jwt-secret-32"; + delete process.env.STORAGE_ENCRYPTION_KEY; + resetStorageEncryptionKey(); +}); + +afterEach(() => { + if (snapshot.JWT_SECRET === undefined) delete process.env.JWT_SECRET; + else process.env.JWT_SECRET = snapshot.JWT_SECRET; + if (snapshot.STORAGE_ENCRYPTION_KEY === undefined) delete process.env.STORAGE_ENCRYPTION_KEY; + else process.env.STORAGE_ENCRYPTION_KEY = snapshot.STORAGE_ENCRYPTION_KEY; + resetStorageEncryptionKey(); +}); + +/** A connection carrying every secret-bearing field at once, each with a unique canary value. */ +function fullConnection(): DatabaseConnection { + return { + id: "c1", + name: "Prod", + type: "postgres", + host: "db.internal", + port: 5432, + user: "app", + password: "CANARY-DB-PASSWORD", + database: "prod", + connectionString: "postgres://app:CANARY-IN-URL@db.internal:5432/prod", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + ssl: { + mode: "verify-full", + caCert: "-----BEGIN CERTIFICATE-----CA-----END CERTIFICATE-----", + clientCert: "-----BEGIN CERTIFICATE-----CLIENT-----END CERTIFICATE-----", + clientKey: "CANARY-TLS-CLIENT-KEY", + rejectUnauthorized: true, + }, + sshTunnel: { + enabled: true, + host: "bastion.internal", + port: 22, + username: "tunnel", + authMethod: "privateKey", + password: "CANARY-SSH-PASSWORD", + privateKey: "CANARY-SSH-PRIVATE-KEY", + passphrase: "CANARY-SSH-PASSPHRASE", + }, + }; +} + +const CANARIES = [ + "CANARY-DB-PASSWORD", + "CANARY-IN-URL", + "CANARY-TLS-CLIENT-KEY", + "CANARY-SSH-PASSWORD", + "CANARY-SSH-PRIVATE-KEY", + "CANARY-SSH-PASSPHRASE", +]; + +describe("the classification is exhaustive by construction", () => { + // These three assertions are a tripwire, not the mechanism. The MECHANISM is that each map is + // typed Record, so a new interface field breaks `bun run typecheck` before + // any test runs. What these pin is the opposite direction: a field DELETED from the map (or a + // map quietly widened to Record) still gets caught here. + test("every DatabaseConnection field is classified", () => { + const classified = Object.keys(CONNECTION_FIELDS).sort(); + + expect(classified).toEqual( + [ + "color", + "connectionString", + "createdAt", + "database", + "environment", + "group", + "host", + "id", + "instanceName", + "managed", + "name", + "password", + "port", + "seedId", + "serviceName", + "ssl", + "sshTunnel", + "type", + "user", + ].sort(), + ); + }); + + test("exactly the six credential-bearing fields are classified secret", () => { + const secrets = [ + ...Object.keys(CONNECTION_FIELDS).filter((k) => CONNECTION_FIELDS[k as never] === "secret"), + ...Object.keys(SSL_FIELDS) + .filter((k) => SSL_FIELDS[k as never] === "secret") + .map((k) => `ssl.${k}`), + ...Object.keys(SSH_TUNNEL_FIELDS) + .filter((k) => SSH_TUNNEL_FIELDS[k as never] === "secret") + .map((k) => `sshTunnel.${k}`), + ].sort(); + + expect(secrets).toEqual( + [ + "connectionString", + "password", + "ssl.clientKey", + "sshTunnel.passphrase", + "sshTunnel.password", + "sshTunnel.privateKey", + ].sort(), + ); + }); + + test("a certificate is not a secret and stays readable for diagnosis", () => { + expect(SSL_FIELDS.caCert).toBe("public"); + expect(SSL_FIELDS.clientCert).toBe("public"); + }); +}); + +describe("encryptConnections", () => { + test("no canary survives anywhere in the serialized result", () => { + const serialized = JSON.stringify(encryptConnections([fullConnection()])); + + for (const canary of CANARIES) { + expect({ canary, present: serialized.includes(canary) }).toEqual({ canary, present: false }); + } + }); + + test("every secret field becomes a versioned envelope", () => { + const [encrypted] = encryptConnections([fullConnection()]); + const prefix = `${ENVELOPE_VERSION}:`; + + expect(encrypted.password?.startsWith(prefix)).toBe(true); + expect(encrypted.connectionString?.startsWith(prefix)).toBe(true); + expect(encrypted.ssl?.clientKey?.startsWith(prefix)).toBe(true); + expect(encrypted.sshTunnel?.password?.startsWith(prefix)).toBe(true); + expect(encrypted.sshTunnel?.privateKey?.startsWith(prefix)).toBe(true); + expect(encrypted.sshTunnel?.passphrase?.startsWith(prefix)).toBe(true); + }); + + test("leaves the fields an operator needs to identify the deployment readable", () => { + const [encrypted] = encryptConnections([fullConnection()]); + + expect(encrypted.host).toBe("db.internal"); + expect(encrypted.user).toBe("app"); + expect(encrypted.database).toBe("prod"); + expect(encrypted.ssl?.caCert).toContain("BEGIN CERTIFICATE"); + }); + + test("does not mutate the caller's object", () => { + const original = fullConnection(); + encryptConnections([original]); + + expect(original.password).toBe("CANARY-DB-PASSWORD"); + expect(original.sshTunnel?.privateKey).toBe("CANARY-SSH-PRIVATE-KEY"); + }); + + test("does not double-envelope a value that is already sealed", () => { + const once = encryptConnections([fullConnection()]); + const twice = encryptConnections(once); + + expect(twice[0].password).toBe(once[0].password); + }); + + test("handles a connection with no ssl, no tunnel and no password", () => { + const minimal: DatabaseConnection = { + id: "c2", + name: "Local SQLite", + type: "sqlite", + database: "/tmp/app.db", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + }; + + expect(encryptConnections([minimal])).toEqual([minimal]); + }); + + test("leaves an empty-string secret alone rather than enveloping nothing", () => { + const blank = { ...fullConnection(), password: "" }; + + expect(encryptConnections([blank])[0].password).toBe(""); + }); +}); + +describe("decryptConnections", () => { + test("round-trips every secret field", () => { + const original = fullConnection(); + const result = decryptConnections(encryptConnections([original])); + + expect(result.undecryptable).toBe(0); + expect(result.connections[0]).toEqual(original); + }); + + test("passes a pre-encryption store through untouched, which is the whole migration", () => { + const legacy = fullConnection(); + const result = decryptConnections([legacy]); + + expect(result.undecryptable).toBe(0); + expect(result.connections[0].password).toBe("CANARY-DB-PASSWORD"); + }); + + test("reads a half-migrated record where only some fields were re-written", () => { + const encrypted = encryptConnections([fullConnection()])[0]; + const halfMigrated = { ...encrypted, password: "STILL-PLAINTEXT" } as DatabaseConnection; + const result = decryptConnections([halfMigrated]); + + expect(result.undecryptable).toBe(0); + expect(result.connections[0].password).toBe("STILL-PLAINTEXT"); + expect(result.connections[0].sshTunnel?.privateKey).toBe("CANARY-SSH-PRIVATE-KEY"); + }); + + test("omits an unreadable secret, keeps the record, and counts the loss", () => { + const encrypted = encryptConnections([fullConnection()]); + + process.env.JWT_SECRET = "a-different-secret-that-cannot-open-it"; + resetStorageEncryptionKey(); + const result = decryptConnections(encrypted); + + // Six unreadable fields on one record. + expect(result.undecryptable).toBe(6); + // The record SURVIVES. Dropping it would be persisted as a deletion by the write-through + // cache on the next sync, destroying ciphertext a restored key could still have opened. + expect(result.connections).toHaveLength(1); + expect(result.connections[0].name).toBe("Prod"); + expect(result.connections[0].host).toBe("db.internal"); + // And the field is ABSENT, never the raw envelope: "v1:..." must never reach a driver. + expect("password" in result.connections[0]).toBe(false); + expect(result.connections[0].sshTunnel?.privateKey).toBeUndefined(); + expect(JSON.stringify(result.connections)).not.toContain(ENVELOPE_VERSION + ":"); + }); + + test("counts across records rather than reporting only the first", () => { + const encrypted = encryptConnections([fullConnection(), { ...fullConnection(), id: "c2" }]); + + process.env.JWT_SECRET = "a-different-secret-that-cannot-open-it"; + resetStorageEncryptionKey(); + + expect(decryptConnections(encrypted).undecryptable).toBe(12); + }); + + test("an empty list is not an error", () => { + expect(decryptConnections([])).toEqual({ connections: [], undecryptable: 0 }); + }); +}); From 05e15152df89e1112b2729a9c9a57b78bfb286cb Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 04:05:39 +0300 Subject: [PATCH 03/12] feat(storage): encrypt credentials at rest above the provider boundary --- src/lib/storage/encrypting-provider.ts | 88 ++++++++ src/lib/storage/factory.ts | 9 +- tests/isolated/factory-singleton.test.ts | 16 ++ tests/security/credential-at-rest.test.ts | 191 ++++++++++++++++++ .../lib/storage/encrypting-provider.test.ts | 160 +++++++++++++++ .../lib/storage/providers/postgres.test.ts | 13 ++ .../unit/lib/storage/providers/sqlite.test.ts | 15 ++ 7 files changed, 490 insertions(+), 2 deletions(-) create mode 100644 src/lib/storage/encrypting-provider.ts create mode 100644 tests/security/credential-at-rest.test.ts create mode 100644 tests/unit/lib/storage/encrypting-provider.test.ts diff --git a/src/lib/storage/encrypting-provider.ts b/src/lib/storage/encrypting-provider.ts new file mode 100644 index 00000000..d7e68a34 --- /dev/null +++ b/src/lib/storage/encrypting-provider.ts @@ -0,0 +1,88 @@ +import { logger } from "@/lib/logger"; +import { decryptConnections, encryptConnections } from "./connection-secrets"; +import type { ServerStorageProvider, StorageCollection, StorageData } from "./types"; +import type { DatabaseConnection } from "@/lib/types"; + +/** + * Credential encryption, applied ABOVE the ServerStorageProvider boundary. + * + * Why here and not inside each provider: one implementation means sqlite and postgres cannot + * drift, a third provider inherits the control instead of having to remember it, and the + * ciphertext stays portable so copying rows from a SQLite store into PostgreSQL still opens. + * Neither shipped provider knows this exists; both simply receive a connection list whose secret + * fields are already sealed and JSON.stringify it into their `data` column. + * + * Only `connections` is touched. No other collection carries a credential field: history and + * saved_queries hold SQL text (the product's data, not its secrets), audit_log is already + * sanitized by src/lib/audit.ts, and the remaining six hold metadata. + */ + +const CONNECTIONS: StorageCollection = "connections"; + +/** + * Quoted verbatim in docs/STORAGE.md's troubleshooting section, and exported so the doc and the + * code cannot drift into describing different messages. + */ +export const UNDECRYPTABLE_WARNING_PREFIX = "Stored connection secrets could not be decrypted"; + +/** + * One line per read, carrying a count - not one line per field. A read happens on every page load + * through the sync hook, and a per-field line would turn a single misconfiguration into a log + * flood that buries the one thing the operator needs to see. + */ +function reportUndecryptable(count: number): void { + if (count === 0) return; + logger.warn( + `${UNDECRYPTABLE_WARNING_PREFIX}: ${count} field(s) were omitted. Restore the previous JWT_SECRET (or STORAGE_ENCRYPTION_KEY) BEFORE the app writes again, or re-enter the affected credentials.`, + { provider: "storage-encryption" }, + ); +} + +class CredentialEncryptingProvider implements ServerStorageProvider { + constructor(private readonly inner: ServerStorageProvider) {} + + initialize(): Promise { + return this.inner.initialize(); + } + + isHealthy(): Promise { + return this.inner.isHealthy(); + } + + close(): Promise { + return this.inner.close(); + } + + async getAllData(userId: string): Promise> { + const data = await this.inner.getAllData(userId); + if (!data.connections) return data; + const { connections, undecryptable } = decryptConnections(data.connections); + reportUndecryptable(undecryptable); + return { ...data, connections }; + } + + async getCollection(userId: string, collection: K): Promise { + const value = await this.inner.getCollection(userId, collection); + if (collection !== CONNECTIONS || value === null) return value; + // TypeScript cannot narrow StorageData[K] from a runtime comparison on K, so the two casts are + // unavoidable; the runtime guard above is what makes them sound. + const { connections, undecryptable } = decryptConnections(value as DatabaseConnection[]); + reportUndecryptable(undecryptable); + return connections as StorageData[K]; + } + + setCollection(userId: string, collection: K, data: StorageData[K]): Promise { + if (collection !== CONNECTIONS) return this.inner.setCollection(userId, collection, data); + const sealed = encryptConnections(data as DatabaseConnection[]) as StorageData[K]; + return this.inner.setCollection(userId, collection, sealed); + } + + mergeData(userId: string, data: Partial): Promise { + if (!data.connections) return this.inner.mergeData(userId, data); + return this.inner.mergeData(userId, { ...data, connections: encryptConnections(data.connections) }); + } +} + +export function withCredentialEncryption(provider: ServerStorageProvider): ServerStorageProvider { + return new CredentialEncryptingProvider(provider); +} diff --git a/src/lib/storage/factory.ts b/src/lib/storage/factory.ts index b86056db..2c57319e 100644 --- a/src/lib/storage/factory.ts +++ b/src/lib/storage/factory.ts @@ -5,6 +5,7 @@ */ import type { ServerStorageProvider, StorageConfigResponse } from "./types"; +import { withCredentialEncryption } from "./encrypting-provider"; let _provider: ServerStorageProvider | null = null; let _initialized = false; @@ -51,15 +52,19 @@ export async function getStorageProvider(): Promise { await expect(getStorageProvider()).rejects.toThrow("DB init failed"); }); + + test("the provider it hands out encrypts credentials before the backend ever sees them", async () => { + // Wiring, not crypto: if the factory ever returns the bare provider, every other test in the + // suite still passes and every credential silently goes to disk in the clear. + process.env.STORAGE_PROVIDER = "sqlite"; + process.env.JWT_SECRET = "factory-singleton-test-jwt-secret-32ch"; + const provider = await getStorageProvider(); + + await provider?.setCollection("u@example.org", "connections", [ + { id: "c1", name: "Prod", type: "postgres", password: "FACTORY-CANARY", createdAt: new Date(0) }, + ] as never); + + const written = JSON.stringify(mockSQLiteInstance.setCollection.mock.calls); + expect(written).not.toContain("FACTORY-CANARY"); + expect(written).toContain("v1:"); + }); }); describe("factory: closeStorageProvider", () => { diff --git a/tests/security/credential-at-rest.test.ts b/tests/security/credential-at-rest.test.ts new file mode 100644 index 00000000..bdea0566 --- /dev/null +++ b/tests/security/credential-at-rest.test.ts @@ -0,0 +1,191 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { withCredentialEncryption } from "@/lib/storage/encrypting-provider"; +import { resetStorageEncryptionKey } from "@/lib/storage/encryption"; +import type { ServerStorageProvider, StorageCollection, StorageData } from "@/lib/storage/types"; +import type { DatabaseConnection } from "@/lib/types"; + +/** + * Threat: someone who obtains the server store - a stolen SQLite file, a PostgreSQL dump, a leaked + * backup, a misconfigured volume snapshot - can read every database credential the user configured. + * + * The assertion is deliberately about the BYTES that reach the persistence layer, not about which + * function was called: an implementation that encrypts and then also writes a plaintext copy under + * a different key would pass a call-shape test and fail this one. + * + * No mock.module here on purpose. `bun run test` runs tests/security as ONE process and + * route-auth.test.ts imports every API route (hence every driver), so mocking pg or better-sqlite3 + * in this file would poison it. Provider faithfulness is pinned instead in + * tests/unit/lib/storage/providers/*.test.ts. + */ + +const CANARIES = [ + "CANARY-DB-PASSWORD", + "CANARY-IN-URL", + "CANARY-TLS-CLIENT-KEY", + "CANARY-SSH-PASSWORD", + "CANARY-SSH-PRIVATE-KEY", + "CANARY-SSH-PASSPHRASE", +]; + +function connectionWithEverySecret(): DatabaseConnection { + return { + id: "c1", + name: "Prod", + type: "postgres", + host: "db.internal", + port: 5432, + user: "app", + password: "CANARY-DB-PASSWORD", + database: "prod", + connectionString: "postgres://app:CANARY-IN-URL@db.internal:5432/prod", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + ssl: { mode: "verify-full", clientKey: "CANARY-TLS-CLIENT-KEY", rejectUnauthorized: true }, + sshTunnel: { + enabled: true, + host: "bastion.internal", + port: 22, + username: "tunnel", + authMethod: "privateKey", + password: "CANARY-SSH-PASSWORD", + privateKey: "CANARY-SSH-PRIVATE-KEY", + passphrase: "CANARY-SSH-PASSPHRASE", + }, + }; +} + +/** + * Stands in for a real provider at exactly the point one receives its argument. Both shipped + * providers do `JSON.stringify(data)` into a bound parameter and nothing else + * (src/lib/storage/providers/sqlite.ts:118, postgres.ts:107), which is what `persisted()` models. + */ +class CaptureProvider implements ServerStorageProvider { + readonly rows = new Map(); + + async initialize(): Promise {} + async isHealthy(): Promise { + return true; + } + async close(): Promise {} + + async getAllData(): Promise> { + const data: Record = {}; + for (const [key, value] of this.rows) data[key] = value; + return data as Partial; + } + async getCollection(_userId: string, collection: K): Promise { + return (this.rows.get(collection) as StorageData[K]) ?? null; + } + async setCollection( + _userId: string, + collection: K, + data: StorageData[K], + ): Promise { + this.rows.set(collection, data); + } + async mergeData(_userId: string, data: Partial): Promise { + for (const [key, value] of Object.entries(data)) this.rows.set(key, value); + } + + /** Everything this store holds, as the bytes a dump would contain. */ + persisted(): string { + return JSON.stringify([...this.rows.entries()]); + } +} + +const snapshot: Record = {}; + +beforeEach(() => { + snapshot.JWT_SECRET = process.env.JWT_SECRET; + snapshot.STORAGE_ENCRYPTION_KEY = process.env.STORAGE_ENCRYPTION_KEY; + process.env.JWT_SECRET = "credential-at-rest-test-jwt-secret-32"; + delete process.env.STORAGE_ENCRYPTION_KEY; + resetStorageEncryptionKey(); +}); + +afterEach(() => { + if (snapshot.JWT_SECRET === undefined) delete process.env.JWT_SECRET; + else process.env.JWT_SECRET = snapshot.JWT_SECRET; + if (snapshot.STORAGE_ENCRYPTION_KEY === undefined) delete process.env.STORAGE_ENCRYPTION_KEY; + else process.env.STORAGE_ENCRYPTION_KEY = snapshot.STORAGE_ENCRYPTION_KEY; + resetStorageEncryptionKey(); +}); + +describe("no plaintext credential reaches the server store", () => { + test("through a single-collection write, the path the sync hook uses on every change", async () => { + const inner = new CaptureProvider(); + await withCredentialEncryption(inner).setCollection("u@example.org", "connections", [connectionWithEverySecret()]); + + for (const canary of CANARIES) { + expect({ canary, inTheStore: inner.persisted().includes(canary) }).toEqual({ canary, inTheStore: false }); + } + }); + + test("through the migration write, the path that carries a whole localStorage dump at once", async () => { + const inner = new CaptureProvider(); + await withCredentialEncryption(inner).mergeData("u@example.org", { + connections: [connectionWithEverySecret()], + history: [], + }); + + for (const canary of CANARIES) { + expect({ canary, inTheStore: inner.persisted().includes(canary) }).toEqual({ canary, inTheStore: false }); + } + }); + + test("the record is still there, still identifiable, and still usable after a round trip", async () => { + const inner = new CaptureProvider(); + const provider = withCredentialEncryption(inner); + const original = connectionWithEverySecret(); + await provider.setCollection("u@example.org", "connections", [original]); + + // The store names the host in the clear on purpose: an operator holding a dump must be able to + // answer "which of my databases is in here". That is incident response, not leakage. + expect(inner.persisted()).toContain("db.internal"); + expect(await provider.getCollection("u@example.org", "connections")).toEqual([original]); + }); + + test("a store written before this feature existed still opens, with no migration step", async () => { + const inner = new CaptureProvider(); + // Seed the INNER provider directly: this is a row that predates the decorator. + await inner.setCollection("u@example.org", "connections", [connectionWithEverySecret()]); + + const data = await withCredentialEncryption(inner).getAllData("u@example.org"); + + expect(data.connections?.[0].password).toBe("CANARY-DB-PASSWORD"); + }); + + test("a credential is never readable through a collection that does not hold one", async () => { + const inner = new CaptureProvider(); + await withCredentialEncryption(inner).setCollection("u@example.org", "saved_queries", [ + { + id: "q1", + name: "canary", + query: "SELECT 'CANARY-DB-PASSWORD'", + connectionType: "postgres", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + updatedAt: new Date("2026-01-01T00:00:00.000Z"), + }, + ]); + + // SQL text is the product's data, not its secrets, and is stored as written. This pins the + // scope of the control so nobody later reads its absence here as a gap. + expect(inner.persisted()).toContain("CANARY-DB-PASSWORD"); + }); +}); + +describe("a key the store cannot be opened with", () => { + test("returns the connection without its secrets instead of failing the whole read", async () => { + const inner = new CaptureProvider(); + await withCredentialEncryption(inner).setCollection("u@example.org", "connections", [connectionWithEverySecret()]); + + process.env.JWT_SECRET = "an-entirely-different-secret-value-32c"; + resetStorageEncryptionKey(); + const data = await withCredentialEncryption(inner).getAllData("u@example.org"); + + expect(data.connections).toHaveLength(1); + expect(data.connections?.[0].name).toBe("Prod"); + expect(data.connections?.[0].password).toBeUndefined(); + // Never the envelope itself: "v1:..." reaching a driver as a password is undiagnosable. + expect(JSON.stringify(data.connections)).not.toContain("v1:"); + }); +}); diff --git a/tests/unit/lib/storage/encrypting-provider.test.ts b/tests/unit/lib/storage/encrypting-provider.test.ts new file mode 100644 index 00000000..c141be33 --- /dev/null +++ b/tests/unit/lib/storage/encrypting-provider.test.ts @@ -0,0 +1,160 @@ +import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test"; +import { logger } from "@/lib/logger"; +import { UNDECRYPTABLE_WARNING_PREFIX, withCredentialEncryption } from "@/lib/storage/encrypting-provider"; +import { encryptSecret, resetStorageEncryptionKey } from "@/lib/storage/encryption"; +import type { ServerStorageProvider } from "@/lib/storage/types"; +import type { DatabaseConnection } from "@/lib/types"; + +/** Mechanics: delegation, collection narrowing, and the single warning line. */ + +function stubProvider(overrides: Partial = {}) { + return { + initialize: mock(async () => {}), + isHealthy: mock(async () => true), + close: mock(async () => {}), + getAllData: mock(async () => ({})), + getCollection: mock(async () => null), + setCollection: mock(async () => {}), + mergeData: mock(async () => {}), + ...overrides, + } as unknown as ServerStorageProvider & Record>; +} + +const connection: DatabaseConnection = { + id: "c1", + name: "Prod", + type: "postgres", + password: "s3cret", + createdAt: new Date("2026-01-01T00:00:00.000Z"), +}; + +/** + * `bun run test` runs this file in the same process as every route test under tests/api (see + * package.json's "test" script), several of which fire an audit/log call that outlives the + * request and only actually reaches `logger.warn` on a later microtask tick - one that can land + * inside this file's `spyOn` window regardless of which test happens to be running at the time. + * Filtering by the module's own message keeps these assertions about OUR warning, not about + * however much unrelated logging the rest of the suite happens to have in flight. + */ +function ownWarnings(warn: ReturnType>): string[] { + return warn.mock.calls.map((call) => call[0]).filter((message) => message.startsWith(UNDECRYPTABLE_WARNING_PREFIX)); +} + +const snapshot: Record = {}; + +beforeEach(() => { + snapshot.JWT_SECRET = process.env.JWT_SECRET; + process.env.JWT_SECRET = "encrypting-provider-test-jwt-secret-32"; + resetStorageEncryptionKey(); +}); + +afterEach(() => { + if (snapshot.JWT_SECRET === undefined) delete process.env.JWT_SECRET; + else process.env.JWT_SECRET = snapshot.JWT_SECRET; + resetStorageEncryptionKey(); +}); + +describe("delegation", () => { + test("initialize, isHealthy and close reach the inner provider unchanged", async () => { + const inner = stubProvider(); + const provider = withCredentialEncryption(inner); + + await provider.initialize(); + await provider.close(); + + expect(await provider.isHealthy()).toBe(true); + expect(inner.initialize).toHaveBeenCalledTimes(1); + expect(inner.close).toHaveBeenCalledTimes(1); + }); + + test("a collection that holds no credential is written through byte for byte", async () => { + const inner = stubProvider(); + await withCredentialEncryption(inner).setCollection("u", "dismissed_seeds", ["seed-1"]); + + expect(inner.setCollection).toHaveBeenCalledWith("u", "dismissed_seeds", ["seed-1"]); + }); + + test("a non-connections read is returned untouched", async () => { + const inner = stubProvider({ getCollection: mock(async () => ["seed-1"]) as never }); + + expect(await withCredentialEncryption(inner).getCollection("u", "dismissed_seeds")).toEqual(["seed-1"]); + }); + + test("a null connections read stays null rather than becoming an empty list", async () => { + const inner = stubProvider(); + + expect(await withCredentialEncryption(inner).getCollection("u", "connections")).toBeNull(); + }); + + test("getAllData without a connections key is passed straight back", async () => { + const inner = stubProvider({ getAllData: mock(async () => ({ history: [] })) as never }); + + expect(await withCredentialEncryption(inner).getAllData("u")).toEqual({ history: [] }); + }); + + test("mergeData without a connections key is passed straight back", async () => { + const inner = stubProvider(); + await withCredentialEncryption(inner).mergeData("u", { history: [] }); + + expect(inner.mergeData).toHaveBeenCalledWith("u", { history: [] }); + }); +}); + +describe("the warning", () => { + test("names the count and the recovery action exactly once per read", async () => { + const sealed = encryptSecret("s3cret"); + const inner = stubProvider({ + getAllData: mock(async () => ({ + connections: [ + { ...connection, password: sealed }, + { ...connection, id: "c2", password: sealed }, + ], + })) as never, + }); + + process.env.JWT_SECRET = "a-different-secret-that-cannot-open-it"; + resetStorageEncryptionKey(); + const warn = spyOn(logger, "warn").mockImplementation(() => {}); + try { + await withCredentialEncryption(inner).getAllData("u"); + + const calls = ownWarnings(warn); + expect(calls).toHaveLength(1); + expect(calls[0]).toContain(UNDECRYPTABLE_WARNING_PREFIX); + expect(calls[0]).toContain("2 field(s)"); + expect(calls[0]).toContain("BEFORE the app writes again"); + } finally { + warn.mockRestore(); + } + }); + + test("stays silent when everything opened, so the line means something when it appears", async () => { + const inner = stubProvider({ + getAllData: mock(async () => ({ connections: [{ ...connection, password: encryptSecret("s3cret") }] })) as never, + }); + const warn = spyOn(logger, "warn").mockImplementation(() => {}); + try { + await withCredentialEncryption(inner).getAllData("u"); + + expect(ownWarnings(warn)).toHaveLength(0); + } finally { + warn.mockRestore(); + } + }); + + test("also fires on the single-collection read path, not only on getAllData", async () => { + const sealed = encryptSecret("s3cret"); + const inner = stubProvider({ getCollection: mock(async () => [{ ...connection, password: sealed }]) as never }); + + process.env.JWT_SECRET = "a-different-secret-that-cannot-open-it"; + resetStorageEncryptionKey(); + const warn = spyOn(logger, "warn").mockImplementation(() => {}); + try { + await withCredentialEncryption(inner).getCollection("u", "connections"); + + expect(ownWarnings(warn)).toHaveLength(1); + } finally { + warn.mockRestore(); + } + }); +}); diff --git a/tests/unit/lib/storage/providers/postgres.test.ts b/tests/unit/lib/storage/providers/postgres.test.ts index d01ea206..b74d0670 100644 --- a/tests/unit/lib/storage/providers/postgres.test.ts +++ b/tests/unit/lib/storage/providers/postgres.test.ts @@ -177,6 +177,19 @@ describe("PostgresStorageProvider", () => { expect(sql).toContain("ON CONFLICT"); }); + test("persists exactly JSON.stringify of what it was given, adding and hiding nothing", async () => { + // Same reasoning as the SQLite twin: the threat test's claim about the store depends on the + // provider being a faithful serializer. + await provider.initialize(); + mockQuery.mockClear(); + const data = [{ id: "c1", name: "Prod", type: "postgres", password: "v1:aaa:bbb" }]; + + await provider.setCollection("u@example.org", "connections", data as never); + + const [, params] = mockQuery.mock.calls[mockQuery.mock.calls.length - 1]; + expect(params).toEqual(["u@example.org", "connections", JSON.stringify(data)]); + }); + test("isHealthy returns true on success", async () => { await provider.initialize(); mockQuery.mockResolvedValueOnce({ rows: [{ ok: 1 }] }); diff --git a/tests/unit/lib/storage/providers/sqlite.test.ts b/tests/unit/lib/storage/providers/sqlite.test.ts index 2ff07bcc..945afc33 100644 --- a/tests/unit/lib/storage/providers/sqlite.test.ts +++ b/tests/unit/lib/storage/providers/sqlite.test.ts @@ -128,6 +128,21 @@ describe("SQLiteStorageProvider", () => { expect(args[1]).toBe("connections"); }); + test("persists exactly JSON.stringify of what it was given, adding and hiding nothing", async () => { + // The credential-at-rest threat test asserts on what the DECORATOR hands a provider. That is + // only a statement about the store if the provider is a faithful serializer, which is what + // this pins: a provider that re-shaped, re-encoded or supplemented the value would break the + // chain without any security test noticing. + const run = mock((..._args: unknown[]) => {}); + mockPrepare.mockImplementation(() => ({ all: mock(() => []), get: mock(() => undefined), run })); + await provider.initialize(); + const data = [{ id: "c1", name: "Prod", type: "postgres", password: "v1:aaa:bbb" }]; + + await provider.setCollection("u@example.org", "connections", data as never); + + expect(run).toHaveBeenCalledWith("u@example.org", "connections", JSON.stringify(data)); + }); + test("isHealthy returns true when db works", async () => { mockPrepare.mockReturnValue({ all: mock(() => []), From 29d34c9537213c354ba4ddbe0a8a0d32979c9a7c Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 04:15:30 +0300 Subject: [PATCH 04/12] docs(storage): state what credential encryption at rest does, and what rotating the key costs --- .env.example | 15 +++++ SECURITY.md | 18 ++++-- charts/libredb-studio/README.md | 18 ++++++ docs/STORAGE.md | 100 ++++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index 6de61281..be46c93b 100644 --- a/.env.example +++ b/.env.example @@ -117,6 +117,21 @@ STORAGE_PROVIDER=local # Cloud PostgreSQL with SSL: # STORAGE_POSTGRES_URL=postgresql://user:pass@host:5432/libredb?sslmode=require +# Credential encryption key for the SERVER-SIDE store (sqlite/postgres only) — OPTIONAL. +# When STORAGE_PROVIDER is sqlite or postgres, database passwords, connection strings, TLS +# client keys and SSH keys/passphrases are encrypted before they are written, so a stolen +# database file or dump is useless on its own. +# Leave this unset and the key is derived from JWT_SECRET, so there is nothing to configure. +# Set it (at least 32 characters, generate with: openssl rand -base64 32) when you want the +# storage key separated from the session-signing key — for example so JWT_SECRET can be +# rotated without invalidating every saved connection password. +# IMPORTANT: rotating whichever key is in use makes existing stored credentials unreadable. +# They are omitted from the connection, not deleted; the rest of the connection survives and +# you re-enter the password once. Restore the previous key BEFORE the app writes again if you +# want the old values back. +# Browser localStorage is NOT encrypted; this variable does not change that. +# STORAGE_ENCRYPTION_KEY=your_32_character_random_string_here + # =========================================== # SQLite DB Provider Driver (advanced) # =========================================== diff --git a/SECURITY.md b/SECURITY.md index ea87c47f..c45237e9 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -82,10 +82,20 @@ When using LibreDB Studio, please follow these security best practices: today #### Database Connections -- Connection details, including database passwords and SSH private keys, are stored unencrypted - in browser `localStorage`, and in the server-side store when `STORAGE_PROVIDER` is set to - `sqlite` or `postgres`. Treat both as secret material: anyone who can read that browser - profile or that database can read every configured credential. +- Connection details, including database passwords and SSH private keys, are stored **unencrypted + in browser `localStorage`**. Treat that browser profile as secret material: anyone who can read + it can read every configured credential. This is deliberate — it is what allows Studio to work + without a master password — and it is why cross-site scripting is treated as a top-severity + issue in this project. +- In the **server-side store** (`STORAGE_PROVIDER=sqlite` or `postgres`), those same fields are + encrypted at rest with AES-256-GCM before they are written: the database password, the + connection string, the TLS client key, and the SSH password, private key and passphrase. The key + is `STORAGE_ENCRYPTION_KEY` when set, and is otherwise derived from `JWT_SECRET`, so there is no + new required configuration. Host, port, user and database name stay readable so an operator can + still identify what a dump contains. A leaked database file or backup is therefore not by itself + enough to read the credentials — but anyone who can read the server's environment still can. + Rotating the key makes stored credentials unreadable; see + [docs/STORAGE.md](docs/STORAGE.md#credential-encryption-at-rest). - Database credentials are not written to application logs, but they are returned in plaintext to the authenticated owner through storage API responses (for example `GET /api/storage`), because the app must be able to redisplay a saved connection's password for editing and reuse diff --git a/charts/libredb-studio/README.md b/charts/libredb-studio/README.md index 266c707a..fd7dd8d3 100644 --- a/charts/libredb-studio/README.md +++ b/charts/libredb-studio/README.md @@ -245,6 +245,24 @@ helm install libredb libredb/libredb-studio \ --set extraEnv[1].value="true" ``` +### Separating the storage encryption key + +With `STORAGE_PROVIDER` set to `sqlite` or `postgres`, connection credentials are encrypted at rest +using a key derived from `JWT_SECRET`. Nothing needs configuring for that to work. Set +`STORAGE_ENCRYPTION_KEY` when you want the two separated — most usefully so rotating the +session-signing secret does not invalidate every saved connection password: + +```bash +helm install libredb-studio libredb-studio/libredb-studio \ + --set extraEnv[0].name=STORAGE_ENCRYPTION_KEY \ + --set extraEnv[0].valueFrom.secretKeyRef.name=libredb-studio-storage \ + --set extraEnv[0].valueFrom.secretKeyRef.key=encryption-key +``` + +Rotating the key makes existing stored credentials unreadable — the connections survive and their +passwords are omitted. See +[docs/STORAGE.md](https://github.com/libredb/libredb-studio/blob/main/docs/STORAGE.md#credential-encryption-at-rest). + ## External Secrets Use `secrets.existingSecret` to reference a secret managed by External Secrets Operator, Sealed Secrets, or Vault: diff --git a/docs/STORAGE.md b/docs/STORAGE.md index edea6551..b5f5f56f 100644 --- a/docs/STORAGE.md +++ b/docs/STORAGE.md @@ -19,6 +19,7 @@ This document is split into two parts. Most readers want **[Part 1 — Setup & C - [2. SQLite Mode](#2-sqlite-mode) - [3. PostgreSQL Mode](#3-postgresql-mode) - [Migration: Local to Server](#migration-local-to-server) +- [Credential Encryption at Rest](#credential-encryption-at-rest) - [Environment Variables Reference](#environment-variables-reference) - [Health Check](#health-check) - [Troubleshooting](#troubleshooting) @@ -340,6 +341,89 @@ For the full migration lifecycle and the underlying merge semantics, see [Migrat --- +## Credential Encryption at Rest + +In `sqlite` and `postgres` modes, credentials are encrypted before they are written. There is +nothing to switch on and nothing to configure. + +### What is encrypted + +Six fields, all on a saved connection: + +| Field | What it is | +|-------|------------| +| `password` | The database password | +| `connectionString` | A URL that can embed `user:password@` | +| `ssl.clientKey` | The TLS client private key | +| `sshTunnel.password` | The SSH password | +| `sshTunnel.privateKey` | The SSH private key | +| `sshTunnel.passphrase` | The passphrase that unlocks the key above | + +Everything else stays readable, deliberately: `host`, `port`, `user`, `database`, `name` and the +TLS certificates (`ssl.caCert`, `ssl.clientCert` — certificates are public by construction). An +operator holding a dump has to be able to answer "which of my databases is in here"; that is +incident response, not a leak. No other collection is touched — `history` and `saved_queries` hold +SQL text, which is the product's data rather than its secrets. + +Each value is stored as `v1::` using AES-256-GCM with a fresh random IV, so two +identical passwords do not look identical in the store, and a tampered value is detected rather +than silently decrypting to something else. + +### The key + +| `STORAGE_ENCRYPTION_KEY` | Key used | +|--------------------------|----------| +| unset (default) | Derived from `JWT_SECRET` via HKDF-SHA256 | +| set (min 32 characters) | Derived from that value via HKDF-SHA256 | + +Deriving from `JWT_SECRET` is what keeps the zero-config promise: no new required variable, and if +you never set `JWT_SECRET` either, the first-run bootstrap generates and persists one +(`/auth-bootstrap.json`), so the key is stable across restarts. + +Set a dedicated `STORAGE_ENCRYPTION_KEY` where key separation matters — most usefully so that +rotating your session-signing secret does not also invalidate every saved connection password. + +### Rotating a key invalidates stored credentials + +**This is the trade-off, stated plainly: rotating whichever key is in use makes every stored +credential unreadable.** That applies to rotating `JWT_SECRET` when `STORAGE_ENCRYPTION_KEY` is +unset, and to rotating `STORAGE_ENCRYPTION_KEY` when it is set. It also applies if you lose the +bootstrap file — a container without a persistent volume for its data directory regenerates +`JWT_SECRET` on every start. + +What happens is bounded and recoverable: + +- The **connection survives**. Its name, host, port, user and database are still there. +- The unreadable field is **omitted**, not replaced with garbage. The password box is empty. +- A warning is written to the server log once per read: `Stored connection secrets could not be + decrypted: N field(s) were omitted.` +- Nothing is deleted from the database by the read itself. + +To recover, restore the previous key and restart, **before** using the app. Reads happen on page +load; the first time the app writes that collection back, the omitted values are gone for good, +because the browser copy is the source the server is updated from. If you cannot restore the key, +re-enter the affected passwords once and they are re-encrypted under the current key. + +### Existing deployments migrate themselves + +Reads accept both plaintext and encrypted values, and writes always produce encrypted ones. An +existing store therefore migrates as it is used, with no migration command, no downtime and no +version column. A row that is never written again stays plaintext — which is why the +`STORAGE_ENCRYPTION_KEY` upgrade is safe to roll back. + +### What this does not protect + +- **Browser `localStorage` is not encrypted.** It is the rendering source and it holds the same + credentials in the clear. That is a deliberate product decision — it is what lets Studio work + without a master password — and it is why cross-site scripting is treated as a top-severity + issue in this project rather than a session-theft issue. +- **Anyone who can read the server's environment can read the credentials.** The key lives there. + This protects a stolen database file, a dump, a backup or a volume snapshot; it is not a vault. +- **`GET /api/storage` returns credentials in plaintext to their authenticated owner.** It has to: + the app must be able to redisplay a saved password for editing. + +--- + ## Environment Variables Reference | Variable | Required | Default | Description | @@ -411,6 +495,22 @@ curl -b cookies.txt http://localhost:3000/api/storage - Local mode only reads from localStorage - To recover: switch back to server mode, the data is still in the database +### "My connection passwords are blank after a restart" + +Look for this line in the server log: + +``` +Stored connection secrets could not be decrypted: 3 field(s) were omitted. +``` + +The encryption key changed. Either `JWT_SECRET` was rotated (and `STORAGE_ENCRYPTION_KEY` is not +set), or `STORAGE_ENCRYPTION_KEY` itself changed, or the data directory holding +`auth-bootstrap.json` was not persisted so a fresh `JWT_SECRET` was generated on start. + +Restore the previous key and restart **before** using the app — see +[Rotating a key invalidates stored credentials](#rotating-a-key-invalidates-stored-credentials). +If the key is gone, re-enter the affected passwords; everything else about the connection is intact. + ### "Duplicate data after migration" - Migration runs once per browser (guarded by the `libredb_server_migrated` flag) and replaces each collection wholesale, so duplicates shouldn't occur From 3d0c5b5df806c180e0a63f802052e0fced38ab90 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 04:31:11 +0300 Subject: [PATCH 05/12] docs(security): publish the control inventory as a posture page --- docs/SECURITY.md | 105 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 docs/SECURITY.md diff --git a/docs/SECURITY.md b/docs/SECURITY.md new file mode 100644 index 00000000..86c13797 --- /dev/null +++ b/docs/SECURITY.md @@ -0,0 +1,105 @@ +# Security Posture + +What LibreDB Studio actually implements, and what it does not. Every row that claims a control +names the file that enforces it and the test that fails when it breaks; `bun run security:check` +runs in CI and fails the build when a row points at something that does not exist, or does not run, +or when a security test exists that no row accounts for. + +For **reporting a vulnerability**, the disclosure timeline, supply-chain details and the SBOM, see +[SECURITY.md](../SECURITY.md) in the repository root. This page is the inventory; that one is the +policy. + +## Scope + +The deployment in scope is **self-hosted, standalone Studio**. There is one trust boundary and the +operator is the owner. The adversary this list is built against is an unauthenticated attacker on +the internet, because most of the distribution channels put the app on a public address. + +Two consequences worth stating before the table: + +- **Running arbitrary SQL is the product's purpose**, not a vulnerability. What is in scope is SQL + the application composes itself — schema browsing, identifiers, pagination, filters. +- **The browser copy of your credentials is not encrypted.** See "Known limits" below. It is the + reason the cross-site scripting controls are the highest-leverage entries in the table. + +## Controls + +| ID | Control | Status | Enforced in | Verified by | +|---|---|---|---|---| +| 0.1 | LLM output never becomes an HTML string; the renderer builds React elements | Implemented | [`src/components/DatabaseDocs.tsx`](../src/components/DatabaseDocs.tsx), [`src/components/AIAutopilotPanel.tsx`](../src/components/AIAutopilotPanel.tsx) | [`tests/security/xss-sinks.test.tsx`](../tests/security/xss-sinks.test.tsx) | +| 0.2 | No remote origin can be fetched through the image optimizer | Implemented | [`next.config.ts`](../next.config.ts) | [`tests/security/image-proxy.test.ts`](../tests/security/image-proxy.test.ts) | +| 0.3 | Every route that reaches a database or an LLM verifies the session in its own handler | Implemented | [`src/lib/api/require-session.ts`](../src/lib/api/require-session.ts) | [`tests/security/route-auth.test.ts`](../tests/security/route-auth.test.ts) | +| 0.4 | The security policy states only what the code does | Implemented | [`SECURITY.md`](../SECURITY.md) | [`scripts/security-check.mjs`](../scripts/security-check.mjs) | +| 0.5 | A published reporting channel with a stated response time | Implemented | [`SECURITY.md`](../SECURITY.md) | [`SECURITY.md`](../SECURITY.md) | +| 1.1 | Every response carries CSP, HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy and Permissions-Policy | Implemented | [`src/lib/security/headers.ts`](../src/lib/security/headers.ts), [`src/lib/security/config.ts`](../src/lib/security/config.ts), [`src/proxy.ts`](../src/proxy.ts) | [`tests/security/headers.test.ts`](../tests/security/headers.test.ts), [`tests/security/header-delivery.test.ts`](../tests/security/header-delivery.test.ts), [`e2e/security-headers.spec.ts`](../e2e/security-headers.spec.ts) | +| 1.2 | Login, AI and database-reaching routes are rate limited | Implemented | [`src/lib/api/rate-limit.ts`](../src/lib/api/rate-limit.ts) | [`tests/security/rate-limit-keying.test.ts`](../tests/security/rate-limit-keying.test.ts), [`tests/security/rate-limit-routes.test.ts`](../tests/security/rate-limit-routes.test.ts) | +| 1.3 | State-changing requests are checked against the deployment's own origin | Implemented | [`src/lib/api/origin-check.ts`](../src/lib/api/origin-check.ts), [`src/proxy.ts`](../src/proxy.ts) | [`tests/security/csrf-origin.test.ts`](../tests/security/csrf-origin.test.ts) | +| 1.4 | Authentication transitions and denials are audited | Partial | [`src/lib/audit.ts`](../src/lib/audit.ts), [`src/lib/api/require-session.ts`](../src/lib/api/require-session.ts) | [`tests/security/auth-audit.test.ts`](../tests/security/auth-audit.test.ts) | +| 1.5 | The login comparison is constant time and its failure response is uniform | Implemented | [`src/lib/auth-compare.ts`](../src/lib/auth-compare.ts), [`src/app/api/auth/login/route.ts`](../src/app/api/auth/login/route.ts) | [`tests/security/login-enumeration.test.ts`](../tests/security/login-enumeration.test.ts) | +| 2.1 | Secrets, dependencies and the container image are scanned in CI | Implemented | [`.github/workflows/security-scan.yml`](../.github/workflows/security-scan.yml), [`.gitleaks.toml`](../.gitleaks.toml), [`.trivyignore.yaml`](../.trivyignore.yaml) | [`tests/unit/security-scan-workflow.test.ts`](../tests/unit/security-scan-workflow.test.ts), [`tests/unit/gitleaks-config.test.ts`](../tests/unit/gitleaks-config.test.ts), [`tests/unit/trivyignore-policy.test.ts`](../tests/unit/trivyignore-policy.test.ts) | +| 2.2 | An SBOM is published with every release | Implemented | [`.github/workflows/release-artifacts.yml`](../.github/workflows/release-artifacts.yml) | [`tests/unit/release-sbom.test.ts`](../tests/unit/release-sbom.test.ts) | +| 2.3 | No TypeScript error is suppressed at build time | Implemented | [`next.config.ts`](../next.config.ts) | [`tests/unit/next-config-typecheck.test.ts`](../tests/unit/next-config-typecheck.test.ts) | +| 3.1 | Credentials are encrypted at rest in the server-side store | Implemented | [`src/lib/storage/encryption.ts`](../src/lib/storage/encryption.ts), [`src/lib/storage/connection-secrets.ts`](../src/lib/storage/connection-secrets.ts), [`src/lib/storage/encrypting-provider.ts`](../src/lib/storage/encrypting-provider.ts) | [`tests/security/credential-at-rest.test.ts`](../tests/security/credential-at-rest.test.ts) | +| 3.2 | Every audit event is emitted as one structured JSON line on stdout | Implemented | [`src/lib/audit.ts`](../src/lib/audit.ts) | [`tests/security/audit-redaction.test.ts`](../tests/security/audit-redaction.test.ts), [`tests/security/audit-type-safety.test.ts`](../tests/security/audit-type-safety.test.ts) | +| 3.3 | This page is checked against the repository on every build | Implemented | [`scripts/security-check.mjs`](../scripts/security-check.mjs) | [`tests/unit/security-check.test.ts`](../tests/unit/security-check.test.ts) | + +## Notes on individual rows + +**0.1.** The fix removed the HTML-string path rather than escaping its input, so a future edit to the +markdown rules cannot reintroduce the sink. Fixed in 0.10.0; earlier releases are affected. + +**1.1.** The Content-Security-Policy permits inline scripts, because the application is statically +prerendered and its hydration scripts are inline and nonce-less. What the policy contains is +**where an injected script could send data** — not whether one can run. Set `CSP_REPORT_ONLY=true` +(a runtime variable, no rebuild) if an upgrade blocks a resource you need while you identify the +directive. + +**1.2.** The counters live in the application process. With more than one replica the budgets apply +per replica; multi-replica deployments should enforce the same budgets at the ingress. See +[`charts/libredb-studio/README.md`](../charts/libredb-studio/README.md). + +**1.4.** Marked Partial: sessions and origin failures are audited, role failures are not. Four +in-handler admin checks and the middleware's `/admin` redirect return their denial with no audit +line. Tracked in [`docs/BACKLOG.md`](./BACKLOG.md), entry H12. + +**3.1.** Applies to `STORAGE_PROVIDER=sqlite` and `postgres` only. Six fields are encrypted; +`host`, `port`, `user`, `database` and the TLS certificates stay readable so a dump can still be +identified. Rotating the key makes stored credentials unreadable — the connection survives, the +field is omitted. Full detail in [`docs/STORAGE.md`](./STORAGE.md#credential-encryption-at-rest). + +**3.2.** `POST /api/admin/audit` is the one writer that reaches the in-app buffer without reaching +stdout, and that is deliberate: its body is client-supplied, so giving it the authoritative channel +would let an admin session forge an indistinguishable log line. + +## Known limits + +These are real, current, and not oversights. Each is a decision with a reason. + +- **Browser `localStorage` holds your credentials in plaintext.** It is the rendering source, and + encrypting it would require a master password and a recovery flow, changing what the product is. + This is why 0.1 and 1.1 matter as much as they do. +- **Anyone who can read the server's environment can read the stored credentials.** 3.1 protects a + stolen database file, dump, backup or volume snapshot. It is not a vault. +- **A `user` can connect to any host and port and run any statement.** The product ships two roles, + and the boundary between them is not a policy engine. Target allowlists, per-provider command + capabilities and a locked-down deployment profile are a coherent direction and are not + implemented. +- **Local login credentials are not hashed.** They arrive as `ADMIN_PASSWORD` and `USER_PASSWORD` + environment variables, so the environment already holds the secret. Rate limiting (1.2) and the + constant-time comparison (1.5) address the reachable part of the risk. +- **Rate limiting is per process and every bucket is keyed on something the caller supplies.** See + [`docs/BACKLOG.md`](./BACKLOG.md), entries H11 and H13. +- **A test linked from this table is checked to exist and to run — not to be true.** Nothing + verifies that a linked test actually exercises the control it is linked from. That is the + residual this page carries knowingly; the same limitation is recorded for the route-guard + allowlist in [`docs/BACKLOG.md`](./BACKLOG.md), entry H10. +- **No dynamic application security testing, no penetration test, no OpenSSF Scorecard badge yet.** + Each was deferred deliberately rather than skipped. + +## Verifying this page yourself + +```bash +bun run security:check # the drift guard CI runs +bun run test # includes tests/security/ +bun run test:e2e # includes the CSP verification against a real browser +``` From 6564e71d940741a24b3c5012779b78bb05f6cb61 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 04:57:18 +0300 Subject: [PATCH 06/12] feat(security): fail the build when the posture page drifts from the repository --- .github/workflows/ci.yml | 8 + package.json | 3 +- scripts/security-check.mjs | 248 ++++++++++++++++++++++++++++++ tests/unit/security-check.test.ts | 240 +++++++++++++++++++++++++++++ 4 files changed, 498 insertions(+), 1 deletion(-) create mode 100644 scripts/security-check.mjs create mode 100644 tests/unit/security-check.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5280c8b..f1dd0854 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,14 @@ jobs: # engine table and install commands, and nothing else notices drift. run: bun run readme:check + - name: Security posture drift guard (control 3.3) + # Same reasoning as the two guards above: inside the required check so it actually gates. + # docs/SECURITY.md claims which controls exist and names what verifies each one; this fails + # the build when a row points at a file that is missing or never runs, when a security test + # exists that no row accounts for, or when a control claims to be implemented with nothing + # verifying it. + run: bun run security:check + - name: Check formatting (Biome) run: bun run format diff --git a/package.json b/package.json index 12da036f..8a408a92 100644 --- a/package.json +++ b/package.json @@ -119,7 +119,8 @@ "chart:bump": "node scripts/sync-chart-version.mjs --write", "distribution:check": "node scripts/distribution-check.mjs", "distribution:matrix": "node scripts/distribution-check.mjs --matrix", - "readme:check": "node scripts/readme-check.mjs" + "readme:check": "node scripts/readme-check.mjs", + "security:check": "node scripts/security-check.mjs" }, "dependencies": { "@google/generative-ai": "^0.24.1", diff --git a/scripts/security-check.mjs b/scripts/security-check.mjs new file mode 100644 index 00000000..6a122d99 --- /dev/null +++ b/scripts/security-check.mjs @@ -0,0 +1,248 @@ +#!/usr/bin/env node +/** + * Posture-page drift guard for docs/SECURITY.md. + * + * The programme design specified "assert that each linked file exists". That is not enough, and + * Phase 2 is the evidence: its digest-pinning guard - protecting the one scanner allowed to block a + * merge - inspected a single physical line and stayed green against the exact downgrade it existed + * to catch. docs/BACKLOG.md H10 records the same shape on the route-guard allowlist. A check that + * cannot fail is worse than no check, because it is believed. + * + * Five checks, each of which a deliberate sabotage turns red: + * + * 1. RESOLVE every markdown link in "Enforced in" and "Verified by" points at a real file + * 2. RUN every linked path under tests/ or e2e/ is actually executed by a runner + * 3. ACCOUNT every tests/security/*.test.ts(x) file is named by at least one row + * 4. CLAIM Status is from a closed set, and a claim of Implemented/Partial links a verifier + * 5. COVER the row IDs are exactly the programme's controls (a zero-row parse fails here) + * + * NOT checked, deliberately and stated on the page itself: whether a linked test actually + * exercises the control it is linked from. No script can decide that honestly. + * + * Pure functions below are unit tested in tests/unit/security-check.test.ts. + */ + +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const POSTURE = "docs/SECURITY.md"; +const COMPONENTS_RUNNER = "tests/run-components.sh"; +const PLAYWRIGHT_CONFIG = "playwright.config.ts"; +const SECURITY_TEST_DIR = "tests/security"; + +/** The header that identifies the control table, matched structurally rather than by heading text. */ +const CONTROL_HEADER = ["ID", "Control", "Status", "Enforced in", "Verified by"]; + +/** + * The programme's control set, written here as a SECOND witness. The page is one statement of what + * exists; this list is another, and the check is that they agree. Adding a control means editing + * both - deliberate friction, which is what an inventory is for. + */ +export const PROGRAMME_CONTROL_IDS = [ + "0.1", + "0.2", + "0.3", + "0.4", + "0.5", + "1.1", + "1.2", + "1.3", + "1.4", + "1.5", + "2.1", + "2.2", + "2.3", + "3.1", + "3.2", + "3.3", +]; + +export const STATUSES = new Set(["Implemented", "Partial", "Not implemented"]); + +/** Directories tests/run-core.sh enumerates with `find ... -name '*.test.ts' -o -name '*.test.tsx'`. */ +const CORE_TEST_DIRS = ["tests/unit/", "tests/api/", "tests/integration/", "tests/hooks/", "tests/security/"]; + +/** Splits a markdown row into trimmed cells, dropping the leading and trailing empties. */ +function cells(line) { + return line + .trim() + .replace(/^\|/, "") + .replace(/\|$/, "") + .split("|") + .map((c) => c.trim()); +} + +/** + * Returns every markdown table as { header, rows }. A table is a run of consecutive pipe lines + * whose second line is the delimiter row; anything else starting with a pipe is skipped. Same + * shape as scripts/readme-check.mjs, deliberately - one markdown parser idiom in this repository. + */ +export function parseTables(markdown) { + const tables = []; + const lines = markdown.split("\n"); + let block = []; + const flush = () => { + if (block.length >= 3 && /^[\s|:-]+$/.test(block[1]) && block[1].includes("-")) { + tables.push({ header: cells(block[0]), rows: block.slice(2).map(cells) }); + } + block = []; + }; + for (const line of lines) { + if (line.trim().startsWith("|")) block.push(line); + else flush(); + } + flush(); + return tables; +} + +/** The control table is the one whose header is exactly the five expected columns. */ +export function findControlTable(tables) { + return ( + tables.find( + (t) => t.header.length === CONTROL_HEADER.length && t.header.every((c, i) => c === CONTROL_HEADER[i]), + ) ?? null + ); +} + +/** + * Repository-relative targets of every markdown link in a cell. Page links are written relative to + * docs/, so a leading "../" is stripped; an absolute URL names nothing in this repository and is + * skipped rather than reported as a missing file. + */ +export function linkTargets(cell) { + const targets = []; + for (const match of cell.matchAll(/\]\(([^)]+)\)/g)) { + const target = match[1]; + if (/^[a-z][a-z0-9+.-]*:/i.test(target)) continue; + targets.push(target.replace(/^\.\.\//, "")); + } + return targets; +} + +/** + * Whether a linked path is actually executed by one of this repository's three runners. + * + * This is the check existence cannot make. A file renamed to *.disabled exists. A test moved from + * tests/security/ to tests/components/ exists. Neither runs, and a posture page that links one is + * claiming a verification nobody performs. + * + * A path that is not a test at all (a source file in the "Enforced in" column) is not subject to + * this and reports executed: true - existence is the only claim being made about it. + */ +export function isExecuted(target, { componentsRunner, playwrightConfig }) { + if (!target.startsWith("tests/") && !target.startsWith("e2e/")) { + return { executed: true, reason: "not a test path" }; + } + if (target.startsWith("e2e/")) { + const inTestDir = /testDir:\s*["']\.\/e2e["']/.test(playwrightConfig); + if (inTestDir && target.endsWith(".spec.ts")) return { executed: true, reason: "playwright testDir" }; + return { executed: false, reason: "not collected by playwright.config.ts" }; + } + const isCoreName = target.endsWith(".test.ts") || target.endsWith(".test.tsx"); + if (isCoreName && CORE_TEST_DIRS.some((dir) => target.startsWith(dir))) { + return { executed: true, reason: "tests/run-core.sh" }; + } + if (componentsRunner.includes(target)) return { executed: true, reason: COMPONENTS_RUNNER }; + return { executed: false, reason: `named by neither tests/run-core.sh nor ${COMPONENTS_RUNNER}` }; +} + +/** + * Returns violation messages (empty = in sync). + * + * `exists` is injected rather than read here so the whole rule set is testable without a + * filesystem, following checkReadmes in scripts/readme-check.mjs. + */ +export function checkPosture({ posture, componentsRunner, playwrightConfig, exists, securityTestFiles }) { + const table = findControlTable(parseTables(posture)); + if (!table) { + return [`${POSTURE}: no control table found (expected a header of exactly: ${CONTROL_HEADER.join(" | ")})`]; + } + + const violations = []; + const linkedTests = new Set(); + const seenIds = []; + + for (const row of table.rows) { + const [id, , status, enforced = "", verified = ""] = row; + seenIds.push(id); + + if (!STATUSES.has(status)) { + violations.push( + `${POSTURE}: control ${id} has status '${status}', which is not one of ${[...STATUSES].join(", ")}`, + ); + } + + const verifiers = linkTargets(verified); + if (verifiers.length === 0 && (status === "Implemented" || status === "Partial")) { + violations.push(`${POSTURE}: control ${id} claims '${status}' but links nothing that verifies it`); + } + + for (const target of [...linkTargets(enforced), ...verifiers]) { + if (!exists(target)) { + violations.push(`${POSTURE}: control ${id} links ${target}, which does not exist`); + continue; + } + const { executed, reason } = isExecuted(target, { componentsRunner, playwrightConfig }); + if (!executed) { + violations.push(`${POSTURE}: control ${id} links ${target}, which is never executed (${reason})`); + } + } + for (const target of verifiers) linkedTests.add(target); + } + + // The other direction. A control can ship with a test and never reach the page; nothing above + // would notice, because every row it checked was fine. + for (const file of securityTestFiles) { + if (!linkedTests.has(file)) { + violations.push(`${POSTURE}: ${file} is verified by no control row - add the row or delete the test`); + } + } + + const missing = PROGRAMME_CONTROL_IDS.filter((id) => !seenIds.includes(id)); + const extra = seenIds.filter((id) => !PROGRAMME_CONTROL_IDS.includes(id)); + if (missing.length > 0) violations.push(`${POSTURE}: missing ${missing.join(", ")} from the control table`); + if (extra.length > 0) violations.push(`${POSTURE}: rows ${extra.join(", ")} are not in the programme control set`); + + return violations; +} + +function main(argv) { + const rootFlag = argv.indexOf("--root"); + const root = rootFlag === -1 ? path.resolve(import.meta.dirname, "..") : argv[rootFlag + 1]; + const posturePath = path.join(root, POSTURE); + if (!fs.existsSync(posturePath)) { + console.error(`ERROR: ${POSTURE} not found in ${root}`); + process.exit(1); + } + const securityDir = path.join(root, SECURITY_TEST_DIR); + const securityTestFiles = fs.existsSync(securityDir) + ? fs + .readdirSync(securityDir) + .filter((name) => name.endsWith(".test.ts") || name.endsWith(".test.tsx")) + .map((name) => `${SECURITY_TEST_DIR}/${name}`) + .sort() + : []; + + const violations = checkPosture({ + posture: fs.readFileSync(posturePath, "utf8"), + componentsRunner: fs.readFileSync(path.join(root, COMPONENTS_RUNNER), "utf8"), + playwrightConfig: fs.readFileSync(path.join(root, PLAYWRIGHT_CONFIG), "utf8"), + exists: (target) => fs.existsSync(path.join(root, target)), + securityTestFiles, + }); + + if (violations.length > 0) { + for (const violation of violations) console.error(`ERROR: ${violation}`); + console.error(`\nFix: bring ${POSTURE} back in line with the repository in this PR.`); + process.exit(1); + } + console.log( + `OK: ${PROGRAMME_CONTROL_IDS.length} controls documented, ${securityTestFiles.length} security tests accounted for`, + ); +} + +// CLI entry only when executed directly (the unit test imports this module). +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + main(process.argv.slice(2)); +} diff --git a/tests/unit/security-check.test.ts b/tests/unit/security-check.test.ts new file mode 100644 index 00000000..4180312b --- /dev/null +++ b/tests/unit/security-check.test.ts @@ -0,0 +1,240 @@ +import { describe, expect, test } from "bun:test"; +import { + checkPosture, + findControlTable, + isExecuted, + linkTargets, + parseTables, + PROGRAMME_CONTROL_IDS, +} from "../../scripts/security-check.mjs"; + +/** + * The guard's own guard. Phase 2's digest-pinning check was structurally unable to fail; these + * cases exist so this one is not. Each violation family gets a case that PRODUCES it. + */ + +const COMPONENTS_RUNNER = ` +run_group "Group 0b: Factory singleton" \\ + tests/isolated/factory-singleton.test.ts +`; +const PLAYWRIGHT_CONFIG = `export default defineConfig({ testDir: "./e2e", projects: [] });`; + +const HEADER = "| ID | Control | Status | Enforced in | Verified by |"; +const DIVIDER = "|---|---|---|---|---|"; + +function page(rows: string[]): string { + return ["# Security Posture", "", HEADER, DIVIDER, ...rows, ""].join("\n"); +} + +function row(id: string, status: string, verified: string, enforced = "[`src/proxy.ts`](../src/proxy.ts)"): string { + return `| ${id} | Something | ${status} | ${enforced} | ${verified} |`; +} + +/** A complete, clean page: one row per programme control, all linking a file that exists and runs. */ +function cleanRows(): string[] { + return PROGRAMME_CONTROL_IDS.map((id) => + row(id, "Implemented", `[\`tests/security/c-${id}.test.ts\`](../tests/security/c-${id}.test.ts)`), + ); +} + +const CLEAN_SECURITY_TESTS = PROGRAMME_CONTROL_IDS.map((id) => `tests/security/c-${id}.test.ts`); + +function run(overrides: Record = {}) { + return checkPosture({ + posture: page(cleanRows()), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS, + ...overrides, + }); +} + +describe("parseTables and findControlTable", () => { + test("finds the control table by its header, not by a heading a translator could change", () => { + const table = findControlTable(parseTables(page(cleanRows()))); + + expect(table?.rows).toHaveLength(PROGRAMME_CONTROL_IDS.length); + }); + + test("ignores a table that is not the control table", () => { + const other = ["| A | B |", "|---|---|", "| 1 | 2 |", ""].join("\n"); + + expect(findControlTable(parseTables(other))).toBeNull(); + }); + + test("a line starting with a pipe that is not a table is not mistaken for one", () => { + expect(parseTables("| not a table\nnor this\n")).toEqual([]); + }); +}); + +describe("linkTargets", () => { + test("extracts every markdown link target in a cell", () => { + expect(linkTargets("[`a`](../a.ts), [`b`](../b.ts)")).toEqual(["a.ts", "b.ts"]); + }); + + test("returns nothing for a cell with no link", () => { + expect(linkTargets("Implemented")).toEqual([]); + }); + + test("ignores an absolute URL, which names nothing in this repository", () => { + expect(linkTargets("[docs](https://example.com/x)")).toEqual([]); + }); +}); + +describe("isExecuted", () => { + const context = { componentsRunner: COMPONENTS_RUNNER, playwrightConfig: PLAYWRIGHT_CONFIG }; + + test("a tests/security file is run by tests/run-core.sh", () => { + expect(isExecuted("tests/security/headers.test.ts", context).executed).toBe(true); + }); + + test("a tests/unit .test.tsx file is run", () => { + expect(isExecuted("tests/unit/a.test.tsx", context).executed).toBe(true); + }); + + test("a disabled file is NOT run even though it exists", () => { + expect(isExecuted("tests/security/headers.test.ts.disabled", context).executed).toBe(false); + }); + + test("a tests/isolated file is run only because run-components.sh names it", () => { + expect(isExecuted("tests/isolated/factory-singleton.test.ts", context).executed).toBe(true); + expect(isExecuted("tests/isolated/never-listed.test.ts", context).executed).toBe(false); + }); + + test("an e2e spec is run when playwright's testDir is the e2e directory", () => { + expect(isExecuted("e2e/security-headers.spec.ts", context).executed).toBe(true); + }); + + test("an e2e spec is NOT counted when playwright points somewhere else", () => { + const moved = { ...context, playwrightConfig: 'export default defineConfig({ testDir: "./other" });' }; + + expect(isExecuted("e2e/security-headers.spec.ts", moved).executed).toBe(false); + }); + + test("a source file is not a test and is not subject to this check", () => { + expect(isExecuted("src/proxy.ts", context).executed).toBe(true); + }); +}); + +describe("checkPosture", () => { + test("a page in sync produces no violations", () => { + expect(run()).toEqual([]); + }); + + test("names a linked file that does not exist", () => { + const violations = run({ exists: (p: string) => p !== "tests/security/c-1.3.test.ts" }); + + expect(violations).toHaveLength(1); + expect(violations[0]).toContain("tests/security/c-1.3.test.ts"); + expect(violations[0]).toContain("does not exist"); + }); + + test("names a linked test that exists but never runs", () => { + const rows = cleanRows(); + rows[0] = row("0.1", "Implemented", "[`t`](../tests/security/c-0.1.test.ts.disabled)"); + + const violations = checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS.filter((f) => f !== "tests/security/c-0.1.test.ts"), + }); + + expect(violations.some((v) => v.includes("is never executed"))).toBe(true); + }); + + test("names a security test that no row accounts for", () => { + const violations = run({ + securityTestFiles: [...CLEAN_SECURITY_TESTS, "tests/security/orphan.test.ts"], + }); + + expect(violations).toHaveLength(1); + expect(violations[0]).toContain("tests/security/orphan.test.ts"); + expect(violations[0]).toContain("no control row"); + }); + + test("rejects a status outside the closed vocabulary", () => { + const rows = cleanRows(); + rows[0] = row("0.1", "Shipped", "[`t`](../tests/security/c-0.1.test.ts)"); + + const violations = checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS, + }); + + expect(violations.some((v) => v.includes("Shipped"))).toBe(true); + }); + + test("rejects a claim with nothing verifying it", () => { + const rows = cleanRows(); + rows[0] = row("0.1", "Implemented", "none yet"); + + const violations = checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS.filter((f) => f !== "tests/security/c-0.1.test.ts"), + }); + + expect(violations.some((v) => v.includes("claims 'Implemented' but links nothing"))).toBe(true); + }); + + test("allows 'Not implemented' to link nothing, because there is nothing to verify", () => { + const rows = cleanRows(); + rows[0] = row("0.1", "Not implemented", "-"); + + const violations = checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS.filter((f) => f !== "tests/security/c-0.1.test.ts"), + }); + + expect(violations).toEqual([]); + }); + + test("names a programme control the page forgot", () => { + const violations = checkPosture({ + posture: page(cleanRows().slice(1)), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS.filter((f) => f !== "tests/security/c-0.1.test.ts"), + }); + + expect(violations.some((v) => v.includes("missing 0.1"))).toBe(true); + }); + + test("names a row the programme does not have", () => { + const rows = [...cleanRows(), row("9.9", "Implemented", "[`t`](../tests/security/c-9.9.test.ts)")]; + const violations = checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: [...CLEAN_SECURITY_TESTS, "tests/security/c-9.9.test.ts"], + }); + + expect(violations.some((v) => v.includes("not in the programme"))).toBe(true); + }); + + test("a page whose control table cannot be found fails loudly instead of passing vacuously", () => { + const violations = checkPosture({ + posture: "# Security Posture\n\nno table here\n", + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS, + }); + + expect(violations).toHaveLength(1); + expect(violations[0]).toContain("no control table"); + }); +}); From 387dd57b77fdca4c38b740a93151f0022aed1f26 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 05:47:07 +0300 Subject: [PATCH 07/12] chore(release): 0.10.0 - credential encryption at rest and the security posture page --- SECURITY.md | 8 +-- charts/libredb-studio/Chart.yaml | 11 ++-- charts/libredb-studio/README.md | 2 +- docs/BACKLOG.md | 55 +++++++++++++++++++ ...studio-operator.clusterserviceversion.yaml | 10 ++-- operator/config/manager/kustomization.yaml | 2 +- .../helm-charts/libredb-studio/Chart.yaml | 11 ++-- operator/helm-charts/libredb-studio/README.md | 20 ++++++- package.json | 2 +- 9 files changed, 94 insertions(+), 27 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index c45237e9..63292ee8 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -4,10 +4,10 @@ We actively support and provide security updates for the following versions of LibreDB Studio: -| Version | Supported | -| ------- | ------------------ | -| 0.9.x | :white_check_mark: | -| < 0.9.0 | :x: | +| Version | Supported | +| -------- | ------------------ | +| 0.10.x | :white_check_mark: | +| < 0.10.0 | :x: | > **Note**: We recommend always using the latest version to ensure you have the most recent security patches. diff --git a/charts/libredb-studio/Chart.yaml b/charts/libredb-studio/Chart.yaml index 487f5d00..ed74d59d 100644 --- a/charts/libredb-studio/Chart.yaml +++ b/charts/libredb-studio/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: libredb-studio description: Web-based SQL IDE for cloud-native teams supporting PostgreSQL, MySQL, SQLite, Oracle, SQL Server, MongoDB, and Redis type: application -version: 0.1.30 -appVersion: "0.9.67" +version: 0.1.31 +appVersion: "0.10.0" kubeVersion: ">=1.26.0-0" home: https://github.com/libredb/libredb-studio icon: https://raw.githubusercontent.com/libredb/libredb-studio/main/public/logo.svg @@ -31,7 +31,7 @@ annotations: artifacthub.io/containsSecurityUpdates: "false" artifacthub.io/images: | - name: libredb-studio - image: ghcr.io/libredb/libredb-studio:0.9.67 + image: ghcr.io/libredb/libredb-studio:0.10.0 platforms: - linux/amd64 - linux/arm64 @@ -43,10 +43,7 @@ annotations: - name: Source url: https://github.com/libredb/libredb-studio artifacthub.io/changes: | - - "Documented how to enforce rate limits at the ingress when running more than one replica: Studio's built-in limiter keeps its counters per process, so N replicas allow N times the configured budget" - - "Documented ALLOWED_ORIGINS, which a deployment needs when the ingress rewrites the Host header without setting x-forwarded-host - without it every state-changing request, including login, is refused with a 403" - - "Documented CSP_REPORT_ONLY, the runtime escape hatch for the enforced Content-Security-Policy, and how to set both variables through extraEnv" - - "No chart template changes - the rendered manifests are identical to 0.1.29" + - "Track app release 0.10.0 (appVersion bump; default image tag follows)" dependencies: - name: postgresql version: "16.x.x" diff --git a/charts/libredb-studio/README.md b/charts/libredb-studio/README.md index fd7dd8d3..8c460913 100644 --- a/charts/libredb-studio/README.md +++ b/charts/libredb-studio/README.md @@ -40,7 +40,7 @@ helm install libredb libredb/libredb-studio \ ```bash helm install libredb oci://ghcr.io/libredb/charts/libredb-studio \ - --version 0.1.30 \ + --version 0.1.31 \ --set secrets.jwtSecret=$(openssl rand -base64 32) \ --set secrets.adminPassword=MyAdmin123 ``` diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index c2d6d455..7714de1d 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -620,3 +620,58 @@ behind it. Done when the bundled runtime's version and provenance appear in the SBOM or a sibling document - a second Trivy pass over the `fetch-node.sh` scripts' pinned version, or a hand-maintained component entry, whichever ships without adding a new failure mode to the release chain. + +--- + +## Security Phase 3 deferrals + +Each of these was decided during Phase 3, not overlooked. Delete an entry when the work lands. + +### K1. Nothing stops a new route from bypassing the authoritative audit channel + +`src/lib/audit.ts` exports both `emitAuditEvent` (ring buffer **and** the `libredb.audit.v1` stdout +line) and `getServerAuditBuffer`, and `POST /api/admin/audit` legitimately uses the second on its +own — its body is client-supplied and must never gain authority over the authoritative channel. But +nothing prevents a future route from doing the same by accident: an event pushed straight to the +buffer is visible in the admin UI, invisible to every log pipeline, and no test notices. The +existing tests all pin the CONTENT of the stdout line, not the set of call sites permitted to skip +it. Done when a check enumerates `getServerAuditBuffer(...).push(` call sites across `src/` and +fails on any that is not on a short, commented allowlist - the same inversion +`tests/security/route-auth.test.ts` applied to route discovery, where a hand-curated list had +already lost eleven routes. + +### K2. A legacy plaintext password shaped exactly like an envelope is treated as corruption + +`src/lib/storage/encryption.ts`'s `readSecret` treats a three-segment value whose first segment +matches `/^v\d+$/` as an envelope. A password stored before this feature existed that happens to be +literally `v1::`, with a 12-byte first segment and a second of at least 16 +bytes, is therefore classified `undecryptable` and omitted rather than returned. The compounded +probability is negligible, the failure is recoverable (the connection survives and the user retypes +the password once), and the alternative - passing an unrecognised value through - would hand +`v1:abc:def` to a driver as a password. Accepted rather than designed away, because the fix would +be a longer, non-colliding prefix, and the stored envelope shape is a fixed contract. Done when the +envelope format is versioned forward for an unrelated reason, at which point a longer prefix costs +nothing. + +### K3. `STORAGE_ENCRYPTION_KEY` is validated at first write, not at boot + +`src/lib/config/auth-preflight.ts` validates `JWT_SECRET` at startup, so a short one stops the +server rather than producing a green health check and a 503 on every login. +`STORAGE_ENCRYPTION_KEY` has no equivalent: a value shorter than 32 characters throws only when the +first storage write happens, which is after login, after the migration attempt, and only in server +storage modes. The failure surfaces as a `syncError` in the UI rather than as a boot failure. Done +when the preflight also reads `STORAGE_ENCRYPTION_KEY` - noting that it must stay silent when +`STORAGE_PROVIDER` is `local`, where the variable is inert and an error would be wrong. + +### K4. Rotating the key back does not recover credentials once the app has written + +`src/lib/storage/connection-secrets.ts`'s `decryptConnections` omits an unreadable secret and keeps +the record, which is correct - dropping the record would be persisted as a deletion. But the +omission is only recoverable until the next write: `useStorageSync` is a write-through cache, so the +first push of the `connections` collection after a failed read overwrites the ciphertext with a +record that has no password field at all. The warning fires on READ, which is before any write, so +an operator who reads their logs promptly has a window. Making the window unnecessary would mean +reading the stored row before every write and preserving an existing envelope when the incoming +value is absent - which would also silently resurrect a password the user deliberately cleared, a +worse bug than the one it fixes. Done when a design is found that distinguishes "the client never +had this value" from "the client cleared this value" without adding a field to the stored shape. diff --git a/operator/bundle/manifests/libredb-studio-operator.clusterserviceversion.yaml b/operator/bundle/manifests/libredb-studio-operator.clusterserviceversion.yaml index 290d2170..ddf74ab3 100644 --- a/operator/bundle/manifests/libredb-studio-operator.clusterserviceversion.yaml +++ b/operator/bundle/manifests/libredb-studio-operator.clusterserviceversion.yaml @@ -21,8 +21,8 @@ metadata: ] capabilities: Basic Install categories: Database, Developer Tools - containerImage: ghcr.io/libredb/libredb-studio-operator:0.9.67 - createdAt: "2026-08-07T17:38:03Z" + containerImage: ghcr.io/libredb/libredb-studio-operator:0.10.0 + createdAt: "2026-08-10T02:42:57Z" description: Open-source web-based SQL IDE for PostgreSQL, MySQL, Oracle, SQL Server, SQLite, MongoDB and Redis, with AI-powered query assistance. operators.operatorframework.io/builder: operator-sdk-v1.42.3 @@ -33,7 +33,7 @@ metadata: operatorframework.io/arch.amd64: supported operatorframework.io/arch.arm64: supported operatorframework.io/os.linux: supported - name: libredb-studio-operator.v0.9.67 + name: libredb-studio-operator.v0.10.0 namespace: placeholder spec: apiservicedefinitions: {} @@ -255,7 +255,7 @@ spec: - --leader-elect - --leader-election-id=libredb-studio-operator - --health-probe-bind-address=:8081 - image: ghcr.io/libredb/libredb-studio-operator:0.9.67 + image: ghcr.io/libredb/libredb-studio-operator:0.10.0 livenessProbe: httpGet: path: /healthz @@ -360,4 +360,4 @@ spec: provider: name: LibreDB url: https://libredb.org - version: 0.9.67 + version: 0.10.0 diff --git a/operator/config/manager/kustomization.yaml b/operator/config/manager/kustomization.yaml index ed937836..e91d64fb 100644 --- a/operator/config/manager/kustomization.yaml +++ b/operator/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: ghcr.io/libredb/libredb-studio-operator - newTag: 0.9.67 + newTag: 0.10.0 diff --git a/operator/helm-charts/libredb-studio/Chart.yaml b/operator/helm-charts/libredb-studio/Chart.yaml index 487f5d00..ed74d59d 100644 --- a/operator/helm-charts/libredb-studio/Chart.yaml +++ b/operator/helm-charts/libredb-studio/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: libredb-studio description: Web-based SQL IDE for cloud-native teams supporting PostgreSQL, MySQL, SQLite, Oracle, SQL Server, MongoDB, and Redis type: application -version: 0.1.30 -appVersion: "0.9.67" +version: 0.1.31 +appVersion: "0.10.0" kubeVersion: ">=1.26.0-0" home: https://github.com/libredb/libredb-studio icon: https://raw.githubusercontent.com/libredb/libredb-studio/main/public/logo.svg @@ -31,7 +31,7 @@ annotations: artifacthub.io/containsSecurityUpdates: "false" artifacthub.io/images: | - name: libredb-studio - image: ghcr.io/libredb/libredb-studio:0.9.67 + image: ghcr.io/libredb/libredb-studio:0.10.0 platforms: - linux/amd64 - linux/arm64 @@ -43,10 +43,7 @@ annotations: - name: Source url: https://github.com/libredb/libredb-studio artifacthub.io/changes: | - - "Documented how to enforce rate limits at the ingress when running more than one replica: Studio's built-in limiter keeps its counters per process, so N replicas allow N times the configured budget" - - "Documented ALLOWED_ORIGINS, which a deployment needs when the ingress rewrites the Host header without setting x-forwarded-host - without it every state-changing request, including login, is refused with a 403" - - "Documented CSP_REPORT_ONLY, the runtime escape hatch for the enforced Content-Security-Policy, and how to set both variables through extraEnv" - - "No chart template changes - the rendered manifests are identical to 0.1.29" + - "Track app release 0.10.0 (appVersion bump; default image tag follows)" dependencies: - name: postgresql version: "16.x.x" diff --git a/operator/helm-charts/libredb-studio/README.md b/operator/helm-charts/libredb-studio/README.md index 266c707a..8c460913 100644 --- a/operator/helm-charts/libredb-studio/README.md +++ b/operator/helm-charts/libredb-studio/README.md @@ -40,7 +40,7 @@ helm install libredb libredb/libredb-studio \ ```bash helm install libredb oci://ghcr.io/libredb/charts/libredb-studio \ - --version 0.1.30 \ + --version 0.1.31 \ --set secrets.jwtSecret=$(openssl rand -base64 32) \ --set secrets.adminPassword=MyAdmin123 ``` @@ -245,6 +245,24 @@ helm install libredb libredb/libredb-studio \ --set extraEnv[1].value="true" ``` +### Separating the storage encryption key + +With `STORAGE_PROVIDER` set to `sqlite` or `postgres`, connection credentials are encrypted at rest +using a key derived from `JWT_SECRET`. Nothing needs configuring for that to work. Set +`STORAGE_ENCRYPTION_KEY` when you want the two separated — most usefully so rotating the +session-signing secret does not invalidate every saved connection password: + +```bash +helm install libredb-studio libredb-studio/libredb-studio \ + --set extraEnv[0].name=STORAGE_ENCRYPTION_KEY \ + --set extraEnv[0].valueFrom.secretKeyRef.name=libredb-studio-storage \ + --set extraEnv[0].valueFrom.secretKeyRef.key=encryption-key +``` + +Rotating the key makes existing stored credentials unreadable — the connections survive and their +passwords are omitted. See +[docs/STORAGE.md](https://github.com/libredb/libredb-studio/blob/main/docs/STORAGE.md#credential-encryption-at-rest). + ## External Secrets Use `secrets.existingSecret` to reference a secret managed by External Secrets Operator, Sealed Secrets, or Vault: diff --git a/package.json b/package.json index 8a408a92..9fd296bf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@libredb/studio", - "version": "0.9.67", + "version": "0.10.0", "private": false, "publishConfig": { "access": "public" From 287d48dc5482677b1015b07f07e8bff47689e0df Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 06:43:43 +0300 Subject: [PATCH 08/12] fix(storage): seal a credential shaped like an envelope on write, not pass it through sealIfPlaintext only encrypted the 'plaintext' outcome of readSecret and returned the other two outcomes verbatim. That is correct for 'decrypted' (already-sealed, skip re-encryption) but wrong for 'undecryptable': a real password shaped like an envelope claim, e.g. literally v2:hunter2, classifies as undecryptable and was being written to SQLite/PostgreSQL in the clear - exactly what this phase promises cannot happen. Fix: only skip encryption for a value PROVEN to open under the current key (kind === decrypted). Plaintext and anything merely envelope-shaped, including a corrupted three-segment claim, are both sealed. The write path is fed from localStorage through useStorageSync, which holds plaintext by design, so a value reaching here is overwhelmingly a real credential rather than corruption in transit. Adds regression tests for both shapes (two-segment and three-segment lookalikes) and corrects the encryptConnections and sealIfPlaintext doc comments to state what is now actually true. --- src/lib/storage/connection-secrets.ts | 23 +++++++++++++++--- .../lib/storage/connection-secrets.test.ts | 24 ++++++++++++++++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/lib/storage/connection-secrets.ts b/src/lib/storage/connection-secrets.ts index ae068e42..fea7ee23 100644 --- a/src/lib/storage/connection-secrets.ts +++ b/src/lib/storage/connection-secrets.ts @@ -122,9 +122,21 @@ function walkConnection( return { connection: copy as unknown as DatabaseConnection, dropped }; } -/** Seals a plaintext value; leaves an already-sealed one alone so a re-save is not a re-encryption. */ +/** + * Seals everything except a value already PROVEN to open under the current key; leaves that one + * alone so a re-save is not a re-encryption. + * + * Deliberately not `kind === "plaintext" ? encryptSecret(value) : value`: that would also pass an + * `undecryptable` value through unchanged, and `undecryptable` is not "already sealed" - it is + * "unopenable", which a real password shaped like an envelope claim (a user's actual password is + * `v2:hunter2`, or a corrupted three-segment value) satisfies just as well as genuine ciphertext + * does. This path is fed from `localStorage` through `useStorageSync`, which holds plaintext by + * design, so a value reaching here is overwhelmingly a real credential, not corruption in transit; + * encrypting it is the safe default. `decrypted` is the only outcome that proves the value already + * opens under this key, so it is the only one left untouched. + */ function sealIfPlaintext(value: string): string { - return readSecret(value).kind === "plaintext" ? encryptSecret(value) : value; + return readSecret(value).kind === "decrypted" ? value : encryptSecret(value); } /** Opens a sealed value, passes a legacy plaintext through, and returns undefined for a dead one. */ @@ -133,7 +145,12 @@ function openOrDrop(value: string): string | undefined { return result.kind === "undecryptable" ? undefined : result.value; } -/** Every write goes through this. Never returns a connection with a plaintext credential in it. */ +/** + * Every write goes through this. Never returns a connection with a plaintext credential in it: a + * secret field is left alone only when it is already a v1 envelope that opens under the current + * key (`readSecret` returns `decrypted`) - plaintext and anything merely shaped like an envelope + * are both sealed. + */ export function encryptConnections(connections: DatabaseConnection[]): DatabaseConnection[] { return connections.map((connection) => walkConnection(connection, sealIfPlaintext).connection); } diff --git a/tests/unit/lib/storage/connection-secrets.test.ts b/tests/unit/lib/storage/connection-secrets.test.ts index 66e10a53..fa2a6337 100644 --- a/tests/unit/lib/storage/connection-secrets.test.ts +++ b/tests/unit/lib/storage/connection-secrets.test.ts @@ -6,7 +6,7 @@ import { SSH_TUNNEL_FIELDS, SSL_FIELDS, } from "@/lib/storage/connection-secrets"; -import { ENVELOPE_VERSION, resetStorageEncryptionKey } from "@/lib/storage/encryption"; +import { ENVELOPE_VERSION, readSecret, resetStorageEncryptionKey } from "@/lib/storage/encryption"; import type { DatabaseConnection } from "@/lib/types"; const snapshot: Record = {}; @@ -193,6 +193,28 @@ describe("encryptConnections", () => { expect(encryptConnections([blank])[0].password).toBe(""); }); + + test("encrypts a real password shaped like an envelope, rather than writing it verbatim", () => { + // A user whose actual password is "v2:hunter2" produces a two-segment string matching the + // envelope version tag. readSecret classifies it "undecryptable" - the write path must not + // treat that the same as "already sealed" and store it in the clear. + const lookalike = { ...fullConnection(), password: "v2:hunter2" }; + const [sealed] = encryptConnections([lookalike]); + + expect(sealed.password).not.toBe("v2:hunter2"); + expect(readSecret(sealed.password as string).kind).toBe("decrypted"); + expect(decryptConnections([sealed]).connections[0].password).toBe("v2:hunter2"); + }); + + test("encrypts a corrupted three-segment envelope claim rather than writing it verbatim", () => { + // A three-segment value whose IV/body cannot be decoded or authenticated is also + // "undecryptable", not "already sealed" - the write path must seal it, not pass it through. + const corrupted = { ...fullConnection(), password: "v1:not-base64url-iv:not-base64url-body" }; + const [sealed] = encryptConnections([corrupted]); + + expect(sealed.password).not.toBe(corrupted.password); + expect(readSecret(sealed.password as string).kind).toBe("decrypted"); + }); }); describe("decryptConnections", () => { From abdafbda1b6faf2048471e92e3266c4741e1e742 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 06:46:02 +0300 Subject: [PATCH 09/12] fix(security): require a Verified by link to be a test, and catch duplicate control ids isExecuted treated every non-test path as executed, so a control could name the checker script or the policy document itself as its own Verified by link and the drift guard would accept it - 0.4 and 0.5 both did exactly this. Added requireTest to isExecuted and apply it only to the Verified by column (the Enforced in column legitimately links source files); a non-test target there now fails as 'not a test'. Added real verifier tests for both claims: tests/unit/security-check.test.ts gained a CLI-level case that runs the checker against the actual repository (the real proof behind 0.4, 'the security policy states only what the code does'), and tests/security/vulnerability-disclosure.test.ts reads the root SECURITY.md to verify 0.5's reporting-channel and response-time claim. Both control rows now link these instead of a non-test path. Also detects a duplicate control id: the exact-control-set check only compared presence, so a page carrying all 16 ids plus a second 0.1 produced neither a missing nor an extra id and passed. Added an explicit duplicate check plus a sabotage test. Finally, scoped control 3.2's claim to authoritative (server-generated) audit events: the row claimed every audit event reaches stdout, contradicting the note directly below it that documents POST /api/admin/audit deliberately writing only to the ring buffer, because its body is fully client-supplied. --- docs/SECURITY.md | 6 +- scripts/security-check.mjs | 49 +++++++++--- .../security/vulnerability-disclosure.test.ts | 31 ++++++++ tests/unit/security-check.test.ts | 74 +++++++++++++++++++ 4 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 tests/security/vulnerability-disclosure.test.ts diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 86c13797..65eac179 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -29,8 +29,8 @@ Two consequences worth stating before the table: | 0.1 | LLM output never becomes an HTML string; the renderer builds React elements | Implemented | [`src/components/DatabaseDocs.tsx`](../src/components/DatabaseDocs.tsx), [`src/components/AIAutopilotPanel.tsx`](../src/components/AIAutopilotPanel.tsx) | [`tests/security/xss-sinks.test.tsx`](../tests/security/xss-sinks.test.tsx) | | 0.2 | No remote origin can be fetched through the image optimizer | Implemented | [`next.config.ts`](../next.config.ts) | [`tests/security/image-proxy.test.ts`](../tests/security/image-proxy.test.ts) | | 0.3 | Every route that reaches a database or an LLM verifies the session in its own handler | Implemented | [`src/lib/api/require-session.ts`](../src/lib/api/require-session.ts) | [`tests/security/route-auth.test.ts`](../tests/security/route-auth.test.ts) | -| 0.4 | The security policy states only what the code does | Implemented | [`SECURITY.md`](../SECURITY.md) | [`scripts/security-check.mjs`](../scripts/security-check.mjs) | -| 0.5 | A published reporting channel with a stated response time | Implemented | [`SECURITY.md`](../SECURITY.md) | [`SECURITY.md`](../SECURITY.md) | +| 0.4 | The security policy states only what the code does | Implemented | [`SECURITY.md`](../SECURITY.md) | [`tests/unit/security-check.test.ts`](../tests/unit/security-check.test.ts) | +| 0.5 | A published reporting channel with a stated response time | Implemented | [`SECURITY.md`](../SECURITY.md) | [`tests/security/vulnerability-disclosure.test.ts`](../tests/security/vulnerability-disclosure.test.ts) | | 1.1 | Every response carries CSP, HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy and Permissions-Policy | Implemented | [`src/lib/security/headers.ts`](../src/lib/security/headers.ts), [`src/lib/security/config.ts`](../src/lib/security/config.ts), [`src/proxy.ts`](../src/proxy.ts) | [`tests/security/headers.test.ts`](../tests/security/headers.test.ts), [`tests/security/header-delivery.test.ts`](../tests/security/header-delivery.test.ts), [`e2e/security-headers.spec.ts`](../e2e/security-headers.spec.ts) | | 1.2 | Login, AI and database-reaching routes are rate limited | Implemented | [`src/lib/api/rate-limit.ts`](../src/lib/api/rate-limit.ts) | [`tests/security/rate-limit-keying.test.ts`](../tests/security/rate-limit-keying.test.ts), [`tests/security/rate-limit-routes.test.ts`](../tests/security/rate-limit-routes.test.ts) | | 1.3 | State-changing requests are checked against the deployment's own origin | Implemented | [`src/lib/api/origin-check.ts`](../src/lib/api/origin-check.ts), [`src/proxy.ts`](../src/proxy.ts) | [`tests/security/csrf-origin.test.ts`](../tests/security/csrf-origin.test.ts) | @@ -40,7 +40,7 @@ Two consequences worth stating before the table: | 2.2 | An SBOM is published with every release | Implemented | [`.github/workflows/release-artifacts.yml`](../.github/workflows/release-artifacts.yml) | [`tests/unit/release-sbom.test.ts`](../tests/unit/release-sbom.test.ts) | | 2.3 | No TypeScript error is suppressed at build time | Implemented | [`next.config.ts`](../next.config.ts) | [`tests/unit/next-config-typecheck.test.ts`](../tests/unit/next-config-typecheck.test.ts) | | 3.1 | Credentials are encrypted at rest in the server-side store | Implemented | [`src/lib/storage/encryption.ts`](../src/lib/storage/encryption.ts), [`src/lib/storage/connection-secrets.ts`](../src/lib/storage/connection-secrets.ts), [`src/lib/storage/encrypting-provider.ts`](../src/lib/storage/encrypting-provider.ts) | [`tests/security/credential-at-rest.test.ts`](../tests/security/credential-at-rest.test.ts) | -| 3.2 | Every audit event is emitted as one structured JSON line on stdout | Implemented | [`src/lib/audit.ts`](../src/lib/audit.ts) | [`tests/security/audit-redaction.test.ts`](../tests/security/audit-redaction.test.ts), [`tests/security/audit-type-safety.test.ts`](../tests/security/audit-type-safety.test.ts) | +| 3.2 | Every authoritative (server-generated) audit event is emitted as one structured JSON line on stdout | Implemented | [`src/lib/audit.ts`](../src/lib/audit.ts) | [`tests/security/audit-redaction.test.ts`](../tests/security/audit-redaction.test.ts), [`tests/security/audit-type-safety.test.ts`](../tests/security/audit-type-safety.test.ts) | | 3.3 | This page is checked against the repository on every build | Implemented | [`scripts/security-check.mjs`](../scripts/security-check.mjs) | [`tests/unit/security-check.test.ts`](../tests/unit/security-check.test.ts) | ## Notes on individual rows diff --git a/scripts/security-check.mjs b/scripts/security-check.mjs index 6a122d99..1d70888b 100644 --- a/scripts/security-check.mjs +++ b/scripts/security-check.mjs @@ -8,13 +8,16 @@ * to catch. docs/BACKLOG.md H10 records the same shape on the route-guard allowlist. A check that * cannot fail is worse than no check, because it is believed. * - * Five checks, each of which a deliberate sabotage turns red: + * Seven checks, each of which a deliberate sabotage turns red: * * 1. RESOLVE every markdown link in "Enforced in" and "Verified by" points at a real file * 2. RUN every linked path under tests/ or e2e/ is actually executed by a runner - * 3. ACCOUNT every tests/security/*.test.ts(x) file is named by at least one row - * 4. CLAIM Status is from a closed set, and a claim of Implemented/Partial links a verifier - * 5. COVER the row IDs are exactly the programme's controls (a zero-row parse fails here) + * 3. PROVE every "Verified by" link is itself a test - a source file or a policy document + * does not count, no matter how real it is (0.4 and 0.5 shipped this exact gap) + * 4. ACCOUNT every tests/security/*.test.ts(x) file is named by at least one row + * 5. CLAIM Status is from a closed set, and a claim of Implemented/Partial links a verifier + * 6. COVER the row IDs are exactly the programme's controls (a zero-row parse fails here) + * 7. UNIQUE no row ID repeats (all-16-plus-a-second-0.1 is neither missing nor extra) * * NOT checked, deliberately and stated on the page itself: whether a linked test actually * exercises the control it is linked from. No script can decide that honestly. @@ -128,11 +131,14 @@ export function linkTargets(cell) { * claiming a verification nobody performs. * * A path that is not a test at all (a source file in the "Enforced in" column) is not subject to - * this and reports executed: true - existence is the only claim being made about it. + * this and reports executed: true - existence is the only claim being made about it. `requireTest` + * flips that for the "Verified by" column: a control naming the checker script or the policy + * document itself as its own verifier (0.4, 0.5) is not linking a test, and existence is not the + * claim a "Verified by" cell makes. */ -export function isExecuted(target, { componentsRunner, playwrightConfig }) { +export function isExecuted(target, { componentsRunner, playwrightConfig, requireTest = false }) { if (!target.startsWith("tests/") && !target.startsWith("e2e/")) { - return { executed: true, reason: "not a test path" }; + return requireTest ? { executed: false, reason: "not a test" } : { executed: true, reason: "not a test path" }; } if (target.startsWith("e2e/")) { const inTestDir = /testDir:\s*["']\.\/e2e["']/.test(playwrightConfig); @@ -178,7 +184,7 @@ export function checkPosture({ posture, componentsRunner, playwrightConfig, exis violations.push(`${POSTURE}: control ${id} claims '${status}' but links nothing that verifies it`); } - for (const target of [...linkTargets(enforced), ...verifiers]) { + for (const target of linkTargets(enforced)) { if (!exists(target)) { violations.push(`${POSTURE}: control ${id} links ${target}, which does not exist`); continue; @@ -188,7 +194,24 @@ export function checkPosture({ posture, componentsRunner, playwrightConfig, exis violations.push(`${POSTURE}: control ${id} links ${target}, which is never executed (${reason})`); } } - for (const target of verifiers) linkedTests.add(target); + for (const target of verifiers) { + // Recorded regardless of what follows: the reciprocal check below asks "does some row CLAIM + // to be verified by this file", not "does that claim also check out" - those are separate + // violations, and folding them together would hide the missing-file or not-a-test violation + // behind a second, misleading "no control row claims this test" one. + linkedTests.add(target); + if (!exists(target)) { + violations.push(`${POSTURE}: control ${id} links ${target}, which does not exist`); + continue; + } + // requireTest: true - a "Verified by" cell is a claim that a test verifies the control, and + // isExecuted's normal existence-is-enough-for-a-source-file allowance would otherwise let a + // checker script or a policy document stand in for a test that does not exist (0.4, 0.5). + const { executed, reason } = isExecuted(target, { componentsRunner, playwrightConfig, requireTest: true }); + if (!executed) { + violations.push(`${POSTURE}: control ${id} links ${target}, which is never executed (${reason})`); + } + } } // The other direction. A control can ship with a test and never reach the page; nothing above @@ -204,6 +227,14 @@ export function checkPosture({ posture, componentsRunner, playwrightConfig, exis if (missing.length > 0) violations.push(`${POSTURE}: missing ${missing.join(", ")} from the control table`); if (extra.length > 0) violations.push(`${POSTURE}: rows ${extra.join(", ")} are not in the programme control set`); + // Presence alone cannot see this: a page carrying all 16 IDs plus a second "0.1" produces + // neither a missing nor an extra ID, because both filters only ask "is this ID somewhere in the + // other list" - never "how many times does it appear here". + const duplicates = [...new Set(seenIds.filter((id, index) => seenIds.indexOf(id) !== index))]; + if (duplicates.length > 0) { + violations.push(`${POSTURE}: duplicate control id(s) ${duplicates.join(", ")} in the control table`); + } + return violations; } diff --git a/tests/security/vulnerability-disclosure.test.ts b/tests/security/vulnerability-disclosure.test.ts new file mode 100644 index 00000000..312cdfda --- /dev/null +++ b/tests/security/vulnerability-disclosure.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import path from "node:path"; + +/** + * The real verifier for control 0.5 in docs/SECURITY.md: "A published reporting channel with a + * stated response time." Before this test existed, 0.5's "Verified by" column linked SECURITY.md + * to itself - a claim is not evidence for itself, and security-check.mjs's drift guard could not + * tell the difference because it never required a "Verified by" link to be a test. + * + * This reads the actual root SECURITY.md (the disclosure policy; docs/SECURITY.md is the + * inventory that links here) and fails the moment the reporting channel or its response-time + * commitment is removed or reworded past recognition. + */ +const POLICY = readFileSync(path.resolve(import.meta.dir, "../../SECURITY.md"), "utf8"); + +describe("the published vulnerability disclosure channel (docs/SECURITY.md control 0.5)", () => { + test("names an email address to report a vulnerability to", () => { + expect(POLICY).toMatch(/\*\*Email:\*\*\s*[^\s@]+@[^\s@]+\.[^\s@]+/); + }); + + test("states a response time for the initial acknowledgment", () => { + expect(POLICY).toMatch(/acknowledge receipt of your vulnerability report within \*\*\d+\s+hours\*\*/i); + }); + + test("tells a reporter where NOT to go, ahead of where to go", () => { + // A public issue tracker is the wrong channel for a vulnerability; the policy has to say so + // before it names the right one, or a reporter finds the wrong door first. + expect(POLICY).toMatch(/do not report security vulnerabilities through public github issues/i); + }); +}); diff --git a/tests/unit/security-check.test.ts b/tests/unit/security-check.test.ts index 4180312b..9ab7dbb7 100644 --- a/tests/unit/security-check.test.ts +++ b/tests/unit/security-check.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import path from "node:path"; import { checkPosture, findControlTable, @@ -8,6 +9,8 @@ import { PROGRAMME_CONTROL_IDS, } from "../../scripts/security-check.mjs"; +const SCRIPT = path.resolve(import.meta.dir, "../../scripts/security-check.mjs"); + /** * The guard's own guard. Phase 2's digest-pinning check was structurally unable to fail; these * cases exist so this one is not. Each violation family gets a case that PRODUCES it. @@ -115,6 +118,17 @@ describe("isExecuted", () => { test("a source file is not a test and is not subject to this check", () => { expect(isExecuted("src/proxy.ts", context).executed).toBe(true); }); + + test("a non-test file is rejected when the caller requires a test (the Verified by column)", () => { + expect(isExecuted("SECURITY.md", { ...context, requireTest: true })).toEqual({ + executed: false, + reason: "not a test", + }); + }); + + test("a real test still passes when the caller requires a test", () => { + expect(isExecuted("tests/security/headers.test.ts", { ...context, requireTest: true }).executed).toBe(true); + }); }); describe("checkPosture", () => { @@ -225,6 +239,53 @@ describe("checkPosture", () => { expect(violations.some((v) => v.includes("not in the programme"))).toBe(true); }); + test("rejects a 'Verified by' link that is not a test at all", () => { + const rows = cleanRows(); + rows[0] = row("0.1", "Implemented", "[`policy`](../SECURITY.md)"); + const violations = checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS.filter((f) => f !== "tests/security/c-0.1.test.ts"), + }); + + expect(violations.some((v) => v.includes("SECURITY.md") && v.includes("not a test"))).toBe(true); + }); + + test("still allows a non-test source file in the 'Enforced in' column", () => { + const rows = cleanRows(); + rows[0] = row( + "0.1", + "Implemented", + "[`t`](../tests/security/c-0.1.test.ts)", + "[`docs/SECURITY.md`](../docs/SECURITY.md)", + ); + + expect( + checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS, + }), + ).toEqual([]); + }); + + test("sabotage: a duplicate control id is caught even though every id individually belongs to the programme", () => { + const rows = [...cleanRows(), row("0.1", "Implemented", "[`t`](../tests/security/c-0.1-again.test.ts)")]; + const violations = checkPosture({ + posture: page(rows), + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: [...CLEAN_SECURITY_TESTS, "tests/security/c-0.1-again.test.ts"], + }); + + expect(violations.some((v) => v.includes("duplicate") && v.includes("0.1"))).toBe(true); + }); + test("a page whose control table cannot be found fails loudly instead of passing vacuously", () => { const violations = checkPosture({ posture: "# Security Posture\n\nno table here\n", @@ -238,3 +299,16 @@ describe("checkPosture", () => { expect(violations[0]).toContain("no control table"); }); }); + +describe("security-check CLI", () => { + // This is the real verifier for control 0.4 ("The security policy states only what the code + // does"): every other test above proves the RULES are right against synthetic fixtures, but 0.4 + // is a claim about docs/SECURITY.md itself, and only running the checker against the actual + // repository proves that claim. Docs/SECURITY.md's own 0.4 row links this test. + test("passes against the real docs/SECURITY.md and the real repository", () => { + const result = Bun.spawnSync(["node", SCRIPT]); + + expect(result.exitCode).toBe(0); + expect(result.stdout.toString()).toContain("OK"); + }); +}); From 3a0bdcff8b0f94ff037385bb573ab5daa41013b2 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 06:48:18 +0300 Subject: [PATCH 10/12] chore(chart): mark the 0.10.0 release as containing security updates Both chart copies carried artifacthub.io/containsSecurityUpdates: "false" on a release that is four phases of security work. Set to "true" in the source chart and re-ran chart:bump to refresh the operator's vendored copy. --- charts/libredb-studio/Chart.yaml | 2 +- operator/helm-charts/libredb-studio/Chart.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/libredb-studio/Chart.yaml b/charts/libredb-studio/Chart.yaml index ed74d59d..92061161 100644 --- a/charts/libredb-studio/Chart.yaml +++ b/charts/libredb-studio/Chart.yaml @@ -28,7 +28,7 @@ annotations: artifacthub.io/category: database artifacthub.io/license: MIT artifacthub.io/prerelease: "false" - artifacthub.io/containsSecurityUpdates: "false" + artifacthub.io/containsSecurityUpdates: "true" artifacthub.io/images: | - name: libredb-studio image: ghcr.io/libredb/libredb-studio:0.10.0 diff --git a/operator/helm-charts/libredb-studio/Chart.yaml b/operator/helm-charts/libredb-studio/Chart.yaml index ed74d59d..92061161 100644 --- a/operator/helm-charts/libredb-studio/Chart.yaml +++ b/operator/helm-charts/libredb-studio/Chart.yaml @@ -28,7 +28,7 @@ annotations: artifacthub.io/category: database artifacthub.io/license: MIT artifacthub.io/prerelease: "false" - artifacthub.io/containsSecurityUpdates: "false" + artifacthub.io/containsSecurityUpdates: "true" artifacthub.io/images: | - name: libredb-studio image: ghcr.io/libredb/libredb-studio:0.10.0 From ad06913e432ff23eb6303a2e2f5f74c1ab45045b Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 10:37:35 +0300 Subject: [PATCH 11/12] docs(security): scope the backup/volume-snapshot claim to when it actually holds docs/SECURITY.md and docs/STORAGE.md both claimed credential encryption at rest protects a stolen backup or volume snapshot. For STORAGE_PROVIDER=sqlite with no STORAGE_ENCRYPTION_KEY set, that is false: the fallback key derives from JWT_SECRET, persisted in auth-bootstrap.json beside the SQLite file (both resolve through the same getDataDir(), and the Helm chart mounts that one directory as a single /app/data volume), so a snapshot of it carries the ciphertext and the key that opens it side by side. Narrowed both claims to a stolen database file or dump on its own, and stated plainly what closes the gap: set STORAGE_ENCRYPTION_KEY from outside the mounted volume (a Kubernetes Secret, an environment variable). postgres deployments do not share this exposure by default, since the key material lives in the app's own filesystem, a volume separate from the database being backed up. STORAGE_ENCRYPTION_KEY stays optional; the zero-config default is unchanged. Also corrected the same claim in the encryption.ts module docstring, and added the same actionable note to the chart README's key-separation section (ran chart:bump afterward to refresh the operator's vendored copy). Pinned the underlying fact rather than just the prose: two new tests in tests/security/credential-at-rest.test.ts assert that the auth-bootstrap file and the SQLite store resolve to the same directory, so a future change that separates them fails the test - the signal that the docs can be widened back. --- charts/libredb-studio/README.md | 5 ++- docs/SECURITY.md | 11 ++++-- docs/STORAGE.md | 12 ++++++- operator/helm-charts/libredb-studio/README.md | 5 ++- src/lib/storage/encryption.ts | 19 +++++++--- tests/security/credential-at-rest.test.ts | 35 +++++++++++++++++++ 6 files changed, 78 insertions(+), 9 deletions(-) diff --git a/charts/libredb-studio/README.md b/charts/libredb-studio/README.md index 8c460913..16ada93c 100644 --- a/charts/libredb-studio/README.md +++ b/charts/libredb-studio/README.md @@ -250,7 +250,10 @@ helm install libredb libredb/libredb-studio \ With `STORAGE_PROVIDER` set to `sqlite` or `postgres`, connection credentials are encrypted at rest using a key derived from `JWT_SECRET`. Nothing needs configuring for that to work. Set `STORAGE_ENCRYPTION_KEY` when you want the two separated — most usefully so rotating the -session-signing secret does not invalidate every saved connection password: +session-signing secret does not invalidate every saved connection password, and, for +`STORAGE_PROVIDER=sqlite`, so a backup or volume snapshot of `/app/data` does not also carry the +key that opens the ciphertext it contains (with nothing set, the fallback key is persisted in that +same directory, alongside the database file): ```bash helm install libredb-studio libredb-studio/libredb-studio \ diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 65eac179..7d641de1 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -65,7 +65,12 @@ line. Tracked in [`docs/BACKLOG.md`](./BACKLOG.md), entry H12. **3.1.** Applies to `STORAGE_PROVIDER=sqlite` and `postgres` only. Six fields are encrypted; `host`, `port`, `user`, `database` and the TLS certificates stay readable so a dump can still be identified. Rotating the key makes stored credentials unreadable — the connection survives, the -field is omitted. Full detail in [`docs/STORAGE.md`](./STORAGE.md#credential-encryption-at-rest). +field is omitted. For `STORAGE_PROVIDER=sqlite` with no `STORAGE_ENCRYPTION_KEY` set, the fallback +key is persisted beside the SQLite file, in the same directory the Helm chart mounts as one volume +— a backup or snapshot of it carries the key alongside the ciphertext it opens. Set +`STORAGE_ENCRYPTION_KEY` from outside that volume (a Kubernetes Secret, an environment variable) to +close that gap; `postgres` deployments do not share this exposure by default. Full detail in +[`docs/STORAGE.md`](./STORAGE.md#credential-encryption-at-rest). **3.2.** `POST /api/admin/audit` is the one writer that reaches the in-app buffer without reaching stdout, and that is deliberate: its body is client-supplied, so giving it the authoritative channel @@ -79,7 +84,9 @@ These are real, current, and not oversights. Each is a decision with a reason. encrypting it would require a master password and a recovery flow, changing what the product is. This is why 0.1 and 1.1 matter as much as they do. - **Anyone who can read the server's environment can read the stored credentials.** 3.1 protects a - stolen database file, dump, backup or volume snapshot. It is not a vault. + stolen database file or dump on its own; it is not a vault. For `STORAGE_PROVIDER=sqlite` with no + `STORAGE_ENCRYPTION_KEY` configured, that protection does not extend to a backup or volume + snapshot of the data directory — see the note on 3.1 below. - **A `user` can connect to any host and port and run any statement.** The product ships two roles, and the boundary between them is not a policy engine. Target allowlists, per-provider command capabilities and a locked-down deployment profile are a coherent direction and are not diff --git a/docs/STORAGE.md b/docs/STORAGE.md index b5f5f56f..591dc838 100644 --- a/docs/STORAGE.md +++ b/docs/STORAGE.md @@ -418,7 +418,17 @@ version column. A row that is never written again stays plaintext — which is w without a master password — and it is why cross-site scripting is treated as a top-severity issue in this project rather than a session-theft issue. - **Anyone who can read the server's environment can read the credentials.** The key lives there. - This protects a stolen database file, a dump, a backup or a volume snapshot; it is not a vault. + This protects a stolen database file or dump on its own; it is not a vault. +- **For `STORAGE_PROVIDER=sqlite` with no `STORAGE_ENCRYPTION_KEY` set, a backup or volume snapshot + is NOT protected.** The fallback key derives from `JWT_SECRET`, persisted in + `/auth-bootstrap.json` — the same directory `STORAGE_SQLITE_PATH` puts the database + file in, and the one directory the Helm chart mounts as a single `/app/data` volume. A snapshot + of that volume carries the ciphertext and the key that opens it side by side. Set + `STORAGE_ENCRYPTION_KEY` from outside that volume — a Kubernetes Secret, an environment variable + your orchestrator supplies — to close this: once the key is not itself part of the backup, a + snapshot is genuinely useless without it. `STORAGE_PROVIDER=postgres` does not share this + exposure by default, because the key material lives in the app's own filesystem, a volume + separate from the database that gets backed up. - **`GET /api/storage` returns credentials in plaintext to their authenticated owner.** It has to: the app must be able to redisplay a saved password for editing. diff --git a/operator/helm-charts/libredb-studio/README.md b/operator/helm-charts/libredb-studio/README.md index 8c460913..16ada93c 100644 --- a/operator/helm-charts/libredb-studio/README.md +++ b/operator/helm-charts/libredb-studio/README.md @@ -250,7 +250,10 @@ helm install libredb libredb/libredb-studio \ With `STORAGE_PROVIDER` set to `sqlite` or `postgres`, connection credentials are encrypted at rest using a key derived from `JWT_SECRET`. Nothing needs configuring for that to work. Set `STORAGE_ENCRYPTION_KEY` when you want the two separated — most usefully so rotating the -session-signing secret does not invalidate every saved connection password: +session-signing secret does not invalidate every saved connection password, and, for +`STORAGE_PROVIDER=sqlite`, so a backup or volume snapshot of `/app/data` does not also carry the +key that opens the ciphertext it contains (with nothing set, the fallback key is persisted in that +same directory, alongside the database file): ```bash helm install libredb-studio libredb-studio/libredb-studio \ diff --git a/src/lib/storage/encryption.ts b/src/lib/storage/encryption.ts index db11dfa9..2c4dfaf3 100644 --- a/src/lib/storage/encryption.ts +++ b/src/lib/storage/encryption.ts @@ -4,12 +4,23 @@ import { getJwtSecret, JWT_SECRET_MIN_LENGTH } from "@/lib/config/auth-env"; /** * Credential encryption at rest for the SERVER-SIDE store (STORAGE_PROVIDER=sqlite|postgres). * - * What this buys and what it does not: a leaked database file or dump is useless on its own, - * because the key lives in the environment and never in the store. It is NOT a vault - anyone who - * can read the process environment can read the credentials, and the browser's localStorage copy - * stays plaintext by deliberate product decision (that is what lets Studio work without a master + * What this buys and what it does not: a leaked database file or dump is useless ON ITS OWN, + * because the key is never written into the store itself. It is NOT a vault - anyone who can read + * the process environment can read the credentials, and the browser's localStorage copy stays + * plaintext by deliberate product decision (that is what lets Studio work without a master * password, and it is why the XSS controls carry the weight they do). * + * A stolen BACKUP OR VOLUME SNAPSHOT is a narrower claim than a stolen database file, and for + * STORAGE_PROVIDER=sqlite with no STORAGE_ENCRYPTION_KEY set it does not hold: the fallback key is + * derived from JWT_SECRET, which the first-run bootstrap persists in auth-bootstrap.json + * (src/lib/auth-bootstrap.ts) beside the SQLite file - both resolve through the same + * src/lib/data-dir.ts:getDataDir(), and the Helm chart mounts that one directory as a single + * /app/data volume. A snapshot of it carries the ciphertext and the key that opens it side by + * side. Set STORAGE_ENCRYPTION_KEY from outside that volume (a Kubernetes Secret, an environment + * variable supplied by the orchestrator) to close this gap; see docs/STORAGE.md. postgres + * deployments do not share this exposure by default, because the key material lives in the app's + * own filesystem, a volume separate from the database being backed up. + * * Key derivation: * STORAGE_ENCRYPTION_KEY, when set -> HKDF-SHA256 -> 32 bytes * otherwise -> JWT_SECRET -> HKDF-SHA256 -> 32 bytes diff --git a/tests/security/credential-at-rest.test.ts b/tests/security/credential-at-rest.test.ts index bdea0566..14ada8a0 100644 --- a/tests/security/credential-at-rest.test.ts +++ b/tests/security/credential-at-rest.test.ts @@ -1,4 +1,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import path from "node:path"; +import { resolveBootstrapPath } from "@/lib/auth-bootstrap"; +import { getDataDir } from "@/lib/data-dir"; import { withCredentialEncryption } from "@/lib/storage/encrypting-provider"; import { resetStorageEncryptionKey } from "@/lib/storage/encryption"; import type { ServerStorageProvider, StorageCollection, StorageData } from "@/lib/storage/types"; @@ -189,3 +192,35 @@ describe("a key the store cannot be opened with", () => { expect(JSON.stringify(data.connections)).not.toContain("v1:"); }); }); + +/** + * A stolen database file "on its own" and a stolen backup or volume snapshot are different + * threats, and the difference matters only for STORAGE_PROVIDER=sqlite with no + * STORAGE_ENCRYPTION_KEY set: the fallback key derives from JWT_SECRET, which the first-run + * bootstrap persists in auth-bootstrap.json (src/lib/auth-bootstrap.ts), and both that file and the + * SQLite store resolve their directory through the SAME function - getDataDir() - reading the SAME + * STORAGE_SQLITE_PATH. The Helm chart then mounts that one directory as a single /app/data volume + * (charts/libredb-studio/templates/deployment.yaml). A snapshot of it therefore contains the + * ciphertext and the key that opens it side by side; docs/SECURITY.md and docs/STORAGE.md scope + * their "protects a backup or volume snapshot" claim to exclude this case for exactly that reason. + * + * This test pins the fact the caveat depends on, not the prose. If a future change gives the + * bootstrap file its own directory (or its own env var), this test fails - which is the signal that + * the docs can be widened back, not a sign this test is stale. + */ +describe("the SQLite default-key backup boundary the docs carve out", () => { + test("the auth-bootstrap file (carrying the fallback key) resolves to the same directory as the SQLite store", () => { + expect(path.dirname(resolveBootstrapPath())).toBe(path.resolve(getDataDir())); + }); + + test("both resolve through the same STORAGE_SQLITE_PATH, not independent configuration", () => { + const original = process.env.STORAGE_SQLITE_PATH; + process.env.STORAGE_SQLITE_PATH = "/custom/deployment/path/store.db"; + try { + expect(path.dirname(resolveBootstrapPath())).toBe(path.resolve(getDataDir())); + } finally { + if (original === undefined) delete process.env.STORAGE_SQLITE_PATH; + else process.env.STORAGE_SQLITE_PATH = original; + } + }); +}); From dea4f0b20f54a25b5f8e965f083766b3cfbb13c4 Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 10 Aug 2026 10:55:30 +0300 Subject: [PATCH 12/12] fix(security): a real verifier for the choke point, a fixed Helm alias, and test isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-reviewer batch on #324, all four verified independently: 1. Control 3.1's Verified by named only tests/security/credential-at-rest.test.ts, which constructs withCredentialEncryption(inner) directly and cannot fail if the factory ever stopped wrapping the provider it hands out - the phase's central choke point. Linked the existing tests/isolated/factory-singleton.test.ts, which does guard the factory, and src/lib/storage/factory.ts in Enforced in. Chose linking the existing test over writing a new one: it already exercises exactly this claim and a second copy would just be a second thing to drift. 2. charts/libredb-studio/README.md told the reader to 'helm install libredb-studio libredb-studio/libredb-studio', but the repo alias documented two lines above (and used by every other example in the file) is 'libredb', not 'libredb-studio' - the command as written does not resolve. Fixed to match every other example in the file. Verified with git blame: introduced on this branch, absent from origin/main. Edited the source chart, then ran chart:bump to refresh the operator's vendored copy. 3. tests/unit/lib/storage/encrypting-provider.test.ts snapshotted and restored only JWT_SECRET; an ambient STORAGE_ENCRYPTION_KEY (a developer's .env.local) takes precedence in inputKeyMaterial(), so rotating JWT_SECRET in that case would not change the derived key and the undecryptable-warning tests would stop testing what they claim to. Matched the sibling file's isolation (tests/security/credential-at-rest.test.ts already snapshots both). No new test added: the key-precedence behavior itself is already covered correctly in tests/unit/lib/storage/encryption.test.ts - this was purely a test-isolation gap, not a missing behavioral test. 4. eslint.config.mjs's type-aware layer (no-floating-promises, no-misused-promises, await-thenable) was scoped to src/app/api/** and src/lib/db/**, never reaching src/lib/storage/**, where this phase's async-heavy credential-encryption decorator lives. Added src/lib/storage/**/*.ts to the scope. Verified both directions: a floating promise injected into encrypting-provider.ts is now caught (confirmed, then reverted), and src/lib/db/providers/document/couchbase/introspect.ts:253:23: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/monitoring/tabs/PerformanceTab.tsx:21:59: warning react(no-object-type-as-default-prop): Do not use an array literal as default prop value. Use a stable reference instead. help: Default values are re-created on every render and break referential equality, causing unnecessary re-renders. Move the value out of the component or memoize it. src/components/monitoring/tabs/PerformanceTab.tsx:265:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/monitoring/tabs/OverviewTab.tsx:21:56: warning react(no-object-type-as-default-prop): Do not use an array literal as default prop value. Use a stable reference instead. help: Default values are re-created on every render and break referential equality, causing unnecessary re-renders. Move the value out of the component or memoize it. src/components/monitoring/tabs/OverviewTab.tsx:247:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders tests/helpers/mock-fetch.ts:33:15: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/monitoring/tabs/SessionsTab.tsx:267:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/monitoring/tabs/SessionsTab.tsx:284:25: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/lib/storage/providers/postgres.ts:119:11: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/utils/pool-manager.ts:168:14: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/utils/pool-manager.ts:180:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/monitoring/tabs/TablesTab.tsx:246:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/monitoring/tabs/TablesTab.tsx:263:25: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/monitoring/tabs/StorageTab.tsx:279:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/monitoring/tabs/StorageTab.tsx:296:20: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/monitoring/tabs/QueriesTab.tsx:229:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/monitoring/tabs/QueriesTab.tsx:246:25: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/hooks/use-ai-chat.ts:143:49: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/QuerySafetyDialog.tsx:183:35: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/QuerySafetyDialog.tsx:270:23: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/ResultsGrid.tsx:467:21: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DataImportModal.tsx:437:27: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DataImportModal.tsx:440:29: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/QueryEditor.tsx:112:16: warning react(no-object-type-as-default-prop): Do not use an array literal as default prop value. Use a stable reference instead. help: Default values are re-created on every render and break referential equality, causing unnecessary re-renders. Move the value out of the component or memoize it. tests/components/QuerySafetyDialog.test.tsx:275:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/admin/tabs/OperationsTab.tsx:352:29: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/admin/tabs/OperationsTab.tsx:456:29: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/AIAutopilotPanel.tsx:98:33: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/CreateTableModal.tsx:161:19: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/NL2SQLPanel.tsx:135:35: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/NL2SQLPanel.tsx:208:16: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/admin/tabs/OverviewTab.tsx:658:18: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DatabaseDocs.tsx:81:33: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/DatabaseDocs.tsx:131:15: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DatabaseDocs.tsx:137:15: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DatabaseDocs.tsx:143:15: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DatabaseDocs.tsx:150:15: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DatabaseDocs.tsx:157:14: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DatabaseDocs.tsx:162:19: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/lib/db/factory.ts:169:9: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/factory.ts:176:9: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/PivotTable.tsx:259:21: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/SchemaDiff.tsx:415:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/SchemaDiff.tsx:427:29: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/SchemaDiff.tsx:449:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/SchemaDiff.tsx:459:25: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/SchemaDiff.tsx:477:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/SchemaDiff.tsx:486:25: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:331:23: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:426:27: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:510:33: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/VisualExplain.tsx:540:18: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:580:15: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:586:15: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:592:16: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:600:16: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:606:28: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:609:14: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:625:19: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:632:17: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:864:21: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/VisualExplain.tsx:928:22: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DataCharts.tsx:228:12: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/DataCharts.tsx:961:27: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders tests/security/rate-limit-routes.test.ts:62:17: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/rate-limit-routes.test.ts:78:9: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/rate-limit-routes.test.ts:115:9: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/rate-limit-routes.test.ts:130:9: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/rate-limit-routes.test.ts:153:17: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/rate-limit-routes.test.ts:197:9: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/rate-limit-routes.test.ts:201:26: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/rate-limit-routes.test.ts:214:21: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:124:15: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:136:15: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:144:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:150:15: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:157:19: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:197:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:213:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/login-enumeration.test.ts:248:36: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/api/storage/storage-routes.test.ts:307:19: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/api/storage/storage-routes.test.ts:322:19: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/unit/llm/ollama-provider.test.ts:45:29: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/unit/llm/custom-provider.test.ts:52:29: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/unit/llm/gemini-provider.test.ts:55:29: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/helpers/mock-next.ts:74:29: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/hooks/use-inline-editing.ts:152:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/keyvalue/redis.ts:330:36: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/keyvalue/redis.ts:344:28: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/unit/llm/openai-provider.test.ts:52:29: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/hooks/use-inline-editing.test.ts:602:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/admin/tabs/AuditTab.tsx:163:25: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/admin/tabs/AuditTab.tsx:308:27: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/lib/db/providers/document/mongodb.ts:466:24: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:471:23: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:478:26: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:482:25: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:663:17: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:685:19: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:748:27: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:895:27: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:928:32: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/document/mongodb.ts:931:25: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/DataProfiler.tsx:151:35: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/DataProfiler.tsx:310:35: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders tests/api/auth/oidc-login.test.ts:108:9: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/app/api/db/multi-query/route.ts:128:23: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/sql/mysql.ts:563:31: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/sql/mysql.ts:568:26: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/sql/mysql.ts:573:29: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/llm/utils/retry.ts:62:14: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/llm/utils/retry.ts:82:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/ui/toggle-group.tsx:44:36: warning react(jsx-no-constructed-context-values): The Context `value` prop should not be constructed. help: Wrap the `value` prop in useMemo() or useCallback(), or use a constant value to prevent unnecessary re-renders. Alternatively, move the value outside the render function if it doesn't depend on props or state. tests/security/csrf-origin.test.ts:175:19: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/security/route-auth.test.ts:143:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/unit/instrumentation.test.ts:27:5: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/db/providers/sql/oracle.ts:737:19: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/unit/llm/streaming.test.ts:12:29: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/lib/llm/utils/streaming.ts:168:37: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/app/api/db/profile/route.ts:116:26: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/ui/carousel.tsx:107:7: warning react(jsx-no-constructed-context-values): The Context `value` prop should not be constructed. help: Wrap the `value` prop in useMemo() or useCallback(), or use a constant value to prevent unnecessary re-renders. Alternatively, move the value outside the render function if it doesn't depend on props or state. e2e/functional-smoke.spec.ts:69:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/api/auth/login.test.ts:158:19: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/api/auth/login.test.ts:316:7: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. src/components/ui/field.tsx:194:67: warning react(no-array-index-key): Usage of Array index in keys is not allowed help: Use a unique data-dependent key to avoid unnecessary rerenders src/components/ui/form.tsx:37:32: warning react(jsx-no-constructed-context-values): The Context `value` prop should not be constructed. help: Wrap the `value` prop in useMemo() or useCallback(), or use a constant value to prevent unnecessary re-renders. Alternatively, move the value outside the render function if it doesn't depend on props or state. src/components/ui/form.tsx:76:31: warning react(jsx-no-constructed-context-values): The Context `value` prop should not be constructed. help: Wrap the `value` prop in useMemo() or useCallback(), or use a constant value to prevent unnecessary re-renders. Alternatively, move the value outside the render function if it doesn't depend on props or state. src/components/ui/chart.tsx:48:28: warning react(jsx-no-constructed-context-values): The Context `value` prop should not be constructed. help: Wrap the `value` prop in useMemo() or useCallback(), or use a constant value to prevent unnecessary re-renders. Alternatively, move the value outside the render function if it doesn't depend on props or state. tests/unit/lib/auth.test.ts:268:20: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. tests/unit/lib/auth.test.ts:273:20: warning eslint(no-await-in-loop): Unexpected `await` inside a loop. help: Collect all promises into an array and use `Promise.all()` to run them in parallel, rather than awaiting each one sequentially inside the loop. /home/cevheri/projects/libredb/libredb-studio/.remember/tmp/last-ndc.ts 1:1 warning Expected an assignment or function call and instead saw an expression @typescript-eslint/no-unused-expressions /home/cevheri/projects/libredb/libredb-studio/bin/studio.js 159:7 warning Unused eslint-disable directive (no problems were reported from 'no-await-in-loop') 167:7 warning Unused eslint-disable directive (no problems were reported from 'no-await-in-loop') /home/cevheri/projects/libredb/libredb-studio/src/app/admin/error.tsx 27:15 warning Do not use `window.location.href` to navigate to internal Next.js pages. Use `redirect()` in the render phase, or `useRouter().push()` in Client Components' event handlers instead. See: https://nextjs.org/docs/messages/no-location-assign-relative-destination @next/next/no-location-assign-relative-destination /home/cevheri/projects/libredb/libredb-studio/src/app/api/connections/managed/route.ts 19:17 warning 'password' is assigned a value but never used @typescript-eslint/no-unused-vars 19:27 warning 'connectionString' is assigned a value but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/src/app/login/login-form.tsx 226:23 warning Do not use `window.location.href` to navigate to internal Next.js pages. Use `redirect()` in the render phase, or `useRouter().push()` in Client Components' event handlers instead. See: https://nextjs.org/docs/messages/no-location-assign-relative-destination @next/next/no-location-assign-relative-destination /home/cevheri/projects/libredb/libredb-studio/src/components/QueryEditor.tsx 146:9 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/QueryEditor.tsx:146:9 144 | const saved = localStorage.getItem("editor-line-numbers"); 145 | if (saved !== null) { > 146 | setShowLineNumbers(saved === "true"); | ^^^^^^^^^^^^^^^^^^ Avoid calling setState() directly within an effect 147 | } 148 | setLineNumbersPreferenceReady(true); 149 | }, []); react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/QueryHistory.tsx 49:5 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/QueryHistory.tsx:49:5 47 | // Refresh history when refreshTrigger changes (replaces key-based re-mount) 48 | useEffect(() => { > 49 | setHistory(storage.getHistory()); | ^^^^^^^^^^ Avoid calling setState() directly within an effect 50 | }, [refreshTrigger]); 51 | 52 | const filteredHistory = useMemo(() => { react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/ResultsGrid.tsx 412:17 warning Compilation Skipped: Use of incompatible library This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized. /home/cevheri/projects/libredb/libredb-studio/src/components/ResultsGrid.tsx:412:17 410 | ]); 411 | > 412 | const table = useReactTable({ | ^^^^^^^^^^^^^ TanStack Table's `useReactTable()` API returns functions that cannot be memoized safely 413 | data: filteredRows, 414 | columns, 415 | state: { react-hooks/incompatible-library /home/cevheri/projects/libredb/libredb-studio/src/components/SavedQueries.tsx 23:5 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/SavedQueries.tsx:23:5 21 | // Refresh queries when refreshTrigger changes (replaces key-based re-mount) 22 | useEffect(() => { > 23 | setQueries(storage.getSavedQueries()); | ^^^^^^^^^^ Avoid calling setState() directly within an effect 24 | }, [refreshTrigger]); 25 | 26 | const filteredQueries = queries.filter((q) => { react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/VisualExplain.tsx 757:5 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/VisualExplain.tsx:757:5 755 | useEffect(() => { 756 | if (kind === null) return; > 757 | setActiveTab((current) => { | ^^^^^^^^^^^^ Avoid calling setState() directly within an effect 758 | const available: readonly ExplainTab[] = kind === "tree" ? TREE_TABS : POSTGRES_TABS; 759 | return available.includes(current) ? current : available[0]; 760 | }); react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/admin/tabs/AuditTab.tsx 236:5 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/admin/tabs/AuditTab.tsx:236:5 234 | 235 | useEffect(() => { > 236 | setHistory(storage.getHistory()); | ^^^^^^^^^^ Avoid calling setState() directly within an effect 237 | }, []); 238 | 239 | const filteredHistory = useMemo(() => { react-hooks/set-state-in-effect 352:5 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/admin/tabs/AuditTab.tsx:352:5 350 | 351 | useEffect(() => { > 352 | setHistory(storage.getHistory()); | ^^^^^^^^^^ Avoid calling setState() directly within an effect 353 | }, []); 354 | 355 | const stats = useMemo(() => { react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/admin/tabs/OverviewTab.tsx 155:7 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/admin/tabs/OverviewTab.tsx:155:7 153 | prevTarget.current = target; 154 | if (target === 0) { > 155 | setValue(0); | ^^^^^^^^ Avoid calling setState() directly within an effect 156 | return; 157 | } 158 | react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/admin/tabs/SecurityTab.tsx 138:5 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/admin/tabs/SecurityTab.tsx:138:5 136 | 137 | useEffect(() => { > 138 | setThresholds(storage.getThresholdConfig()); | ^^^^^^^^^^^^^ Avoid calling setState() directly within an effect 139 | }, []); 140 | 141 | const updateThreshold = (index: number, field: "warning" | "critical", value: number) => { react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/monitoring/MonitoringDashboard.tsx 78:5 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/components/monitoring/MonitoringDashboard.tsx:78:5 76 | useEffect(() => { 77 | if (allConns.length === 0) return; > 78 | setConnections(allConns); | ^^^^^^^^^^^^^^ Avoid calling setState() directly within an effect 79 | 80 | setSelectedConnection((prev) => { 81 | if (prev) return prev; react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/components/schema-explorer/SchemaExplorer.tsx 7:10 warning 'Button' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/src/components/schema-explorer/TableItem.tsx 17:10 warning 'Button' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/src/components/sidebar/ConnectionItem.tsx 5:10 warning 'Button' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/src/components/sidebar/Sidebar.tsx 7:10 warning 'Button' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/src/components/ui/sidebar.tsx 570:26 warning Error: Cannot call impure function during render `Math.random` is an impure function. Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). /home/cevheri/projects/libredb/libredb-studio/src/components/ui/sidebar.tsx:570:26 568 | // Random width between 50 to 90%. 569 | const width = React.useMemo(() => { > 570 | return `${Math.floor(Math.random() * 40) + 50}%`; | ^^^^^^^^^^^^^ Cannot call impure function 571 | }, []); 572 | 573 | return ( react-hooks/purity /home/cevheri/projects/libredb/libredb-studio/src/hooks/use-connection-form.ts 293:6 warning React Hook useCallback has an unnecessary dependency: 'type'. Either exclude it or remove the dependency array react-hooks/exhaustive-deps /home/cevheri/projects/libredb/libredb-studio/src/hooks/use-provider-metadata.ts 22:7 warning Error: Calling setState synchronously within an effect can trigger cascading renders Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following: * Update external systems with the latest state from React. * Subscribe for updates from some external system, calling setState in a callback function when external state changes. Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect). /home/cevheri/projects/libredb/libredb-studio/src/hooks/use-provider-metadata.ts:22:7 20 | useEffect(() => { 21 | if (!connection) { > 22 | setMetadata(null); | ^^^^^^^^^^^ Avoid calling setState() directly within an effect 23 | lastConnectionId.current = null; 24 | return; 25 | } react-hooks/set-state-in-effect /home/cevheri/projects/libredb/libredb-studio/src/hooks/use-tab-manager.ts 160:6 warning React Hook useCallback has an unnecessary dependency: 'activeConnection'. Either exclude it or remove the dependency array react-hooks/exhaustive-deps /home/cevheri/projects/libredb/libredb-studio/src/lib/db/providers/sql/druid/http-transport.ts 264:7 warning 'UNDESCRIBED' is assigned a value but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/src/lib/db/providers/sql/sqlite-driver.ts 99:33 warning '_options' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/src/workspace/StudioWorkspace.tsx 278:5 warning React Hook useCallback has a missing dependency: 'conn.activeConnection?.type'. Either include it or remove the dependency array react-hooks/exhaustive-deps /home/cevheri/projects/libredb/libredb-studio/tests/api/db/maintenance.test.ts 32:29 warning '_event' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/components/DataCharts.test.tsx 59:33 warning '_el' is defined but never used @typescript-eslint/no-unused-vars 59:48 warning '_options' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/components/QueryEditor.test.tsx 19:37 warning '_a' is defined but never used @typescript-eslint/no-unused-vars 20:34 warning '_a' is defined but never used @typescript-eslint/no-unused-vars 311:37 warning '_a' is defined but never used @typescript-eslint/no-unused-vars 312:34 warning '_a' is defined but never used @typescript-eslint/no-unused-vars 1820:26 warning '_a' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/components/QuerySafetyDialog.test.tsx 399:41 warning '_params' is defined but never used @typescript-eslint/no-unused-vars 428:41 warning '_params' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/components/SchemaDiagram.test.tsx 136:40 warning '_options' is defined but never used @typescript-eslint/no-unused-vars 339:50 warning '_options' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/components/StudioWorkspace.test.tsx 46:25 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 48:35 warning '_blob' is defined but never used @typescript-eslint/no-unused-vars 303:37 warning '_query' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/components/sidebar/ConnectionItem.test.tsx 13:7 warning Unused eslint-disable directive (no problems were reported from '@typescript-eslint/no-unused-vars') 18:13 warning 'initial' is defined but never used @typescript-eslint/no-unused-vars 19:13 warning 'animate' is defined but never used @typescript-eslint/no-unused-vars 20:13 warning 'exit' is defined but never used @typescript-eslint/no-unused-vars 21:13 warning 'variants' is defined but never used @typescript-eslint/no-unused-vars 22:13 warning 'whileHover' is defined but never used @typescript-eslint/no-unused-vars 23:13 warning 'whileTap' is defined but never used @typescript-eslint/no-unused-vars 24:13 warning 'layoutId' is defined but never used @typescript-eslint/no-unused-vars 25:13 warning 'transition' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/components/sidebar/ConnectionsList.test.tsx 13:7 warning Unused eslint-disable directive (no problems were reported from '@typescript-eslint/no-unused-vars') 18:13 warning 'initial' is defined but never used @typescript-eslint/no-unused-vars 19:13 warning 'animate' is defined but never used @typescript-eslint/no-unused-vars 20:13 warning 'exit' is defined but never used @typescript-eslint/no-unused-vars 21:13 warning 'variants' is defined but never used @typescript-eslint/no-unused-vars 22:13 warning 'whileHover' is defined but never used @typescript-eslint/no-unused-vars 23:13 warning 'whileTap' is defined but never used @typescript-eslint/no-unused-vars 24:13 warning 'layoutId' is defined but never used @typescript-eslint/no-unused-vars 25:13 warning 'transition' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/hooks/use-ai-chat.test.ts 628:14 warning '_params' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/integration/db/oracle-provider.test.ts 14:38 warning '_opts' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/isolated/factory-singleton.test.ts 38:52 warning 'getStorageProviderType' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/isolated/use-storage-sync.test.ts 444:13 warning 'fetchMock' is assigned a value but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/unit/lib/connection-string-parser.test.ts 654:5 warning Unused eslint-disable directive (no problems were reported from 'no-extend-native') 662:7 warning Unused eslint-disable directive (no problems were reported from 'no-extend-native') /home/cevheri/projects/libredb/libredb-studio/tests/unit/lib/storage/providers/postgres.test.ts 9:34 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 238:41 warning 'sql' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/unit/lib/storage/providers/sqlite.test.ts 10:17 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 12:27 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 13:29 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 115:30 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 136:26 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 164:30 warning '_args' is defined but never used @typescript-eslint/no-unused-vars 198:30 warning '_args' is defined but never used @typescript-eslint/no-unused-vars /home/cevheri/projects/libredb/libredb-studio/tests/unit/schema-diagram/layout-engine.test.ts 29:15 warning '_url' is defined but never used @typescript-eslint/no-unused-vars 126:19 warning '_url' is defined but never used @typescript-eslint/no-unused-vars ✖ 78 problems (0 errors, 78 warnings) 0 errors and 6 warnings potentially fixable with the `--fix` option. surfaces zero new errors against the existing code in that directory, so nothing pre-existing needed silencing. Also attempted, as requested: a real end-to-end round trip against a genuine STORAGE_PROVIDER=sqlite file. tests/integration/storage/ now has a Node-run harness (bundled with bun build v1.3.14 (0d9b296a) error: Missing entrypoints. What would you like to bundle? Usage: $ bun build [...] [...flags] To see full documentation: $ bun build --help and executed under real , since Bun cannot load better-sqlite3) that writes a canary password through the same withCredentialEncryption() decorator the factory uses, checkpoints the WAL, and reads the RAW BYTES ON DISK plus the read-back after a simulated key rotation. Confirmed it catches a real regression: reverting the connection-secrets.ts fix from the previous commit turns this test red (verified locally, then re-fixed). The one obstacle noted in advance - better-sqlite3 needing real node_modules resolution when the bundle lives outside the project tree - is worked around with a symlink into the temp build directory rather than moving output into the repo. docs/SECURITY.md's control 3.1 row now lists three verifiers instead of one. --- charts/libredb-studio/README.md | 2 +- docs/SECURITY.md | 2 +- eslint.config.mjs | 15 +-- operator/helm-charts/libredb-studio/README.md | 2 +- ...lite-credential-encryption-node-harness.ts | 91 +++++++++++++++++++ .../sqlite-credential-encryption.test.ts | 86 ++++++++++++++++++ .../lib/storage/encrypting-provider.test.ts | 9 ++ 7 files changed, 198 insertions(+), 9 deletions(-) create mode 100644 tests/integration/storage/sqlite-credential-encryption-node-harness.ts create mode 100644 tests/integration/storage/sqlite-credential-encryption.test.ts diff --git a/charts/libredb-studio/README.md b/charts/libredb-studio/README.md index 16ada93c..8ed17996 100644 --- a/charts/libredb-studio/README.md +++ b/charts/libredb-studio/README.md @@ -256,7 +256,7 @@ key that opens the ciphertext it contains (with nothing set, the fallback key is same directory, alongside the database file): ```bash -helm install libredb-studio libredb-studio/libredb-studio \ +helm install libredb libredb/libredb-studio \ --set extraEnv[0].name=STORAGE_ENCRYPTION_KEY \ --set extraEnv[0].valueFrom.secretKeyRef.name=libredb-studio-storage \ --set extraEnv[0].valueFrom.secretKeyRef.key=encryption-key diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 7d641de1..3115e645 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -39,7 +39,7 @@ Two consequences worth stating before the table: | 2.1 | Secrets, dependencies and the container image are scanned in CI | Implemented | [`.github/workflows/security-scan.yml`](../.github/workflows/security-scan.yml), [`.gitleaks.toml`](../.gitleaks.toml), [`.trivyignore.yaml`](../.trivyignore.yaml) | [`tests/unit/security-scan-workflow.test.ts`](../tests/unit/security-scan-workflow.test.ts), [`tests/unit/gitleaks-config.test.ts`](../tests/unit/gitleaks-config.test.ts), [`tests/unit/trivyignore-policy.test.ts`](../tests/unit/trivyignore-policy.test.ts) | | 2.2 | An SBOM is published with every release | Implemented | [`.github/workflows/release-artifacts.yml`](../.github/workflows/release-artifacts.yml) | [`tests/unit/release-sbom.test.ts`](../tests/unit/release-sbom.test.ts) | | 2.3 | No TypeScript error is suppressed at build time | Implemented | [`next.config.ts`](../next.config.ts) | [`tests/unit/next-config-typecheck.test.ts`](../tests/unit/next-config-typecheck.test.ts) | -| 3.1 | Credentials are encrypted at rest in the server-side store | Implemented | [`src/lib/storage/encryption.ts`](../src/lib/storage/encryption.ts), [`src/lib/storage/connection-secrets.ts`](../src/lib/storage/connection-secrets.ts), [`src/lib/storage/encrypting-provider.ts`](../src/lib/storage/encrypting-provider.ts) | [`tests/security/credential-at-rest.test.ts`](../tests/security/credential-at-rest.test.ts) | +| 3.1 | Credentials are encrypted at rest in the server-side store | Implemented | [`src/lib/storage/encryption.ts`](../src/lib/storage/encryption.ts), [`src/lib/storage/connection-secrets.ts`](../src/lib/storage/connection-secrets.ts), [`src/lib/storage/encrypting-provider.ts`](../src/lib/storage/encrypting-provider.ts), [`src/lib/storage/factory.ts`](../src/lib/storage/factory.ts) | [`tests/security/credential-at-rest.test.ts`](../tests/security/credential-at-rest.test.ts), [`tests/isolated/factory-singleton.test.ts`](../tests/isolated/factory-singleton.test.ts), [`tests/integration/storage/sqlite-credential-encryption.test.ts`](../tests/integration/storage/sqlite-credential-encryption.test.ts) | | 3.2 | Every authoritative (server-generated) audit event is emitted as one structured JSON line on stdout | Implemented | [`src/lib/audit.ts`](../src/lib/audit.ts) | [`tests/security/audit-redaction.test.ts`](../tests/security/audit-redaction.test.ts), [`tests/security/audit-type-safety.test.ts`](../tests/security/audit-type-safety.test.ts) | | 3.3 | This page is checked against the repository on every build | Implemented | [`scripts/security-check.mjs`](../scripts/security-check.mjs) | [`tests/unit/security-check.test.ts`](../tests/unit/security-check.test.ts) | diff --git a/eslint.config.mjs b/eslint.config.mjs index c4b8997e..eef1b858 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -64,13 +64,16 @@ const eslintConfig = defineConfig([ "react/no-danger": "off", }, }, - // Narrow type-aware safety net for the async-heavy code paths (API routes - // and DB providers). These rules need the real TypeScript type checker - // (projectService), so they are scoped to keep lint fast and to catch - // unhandled-promise bugs where they matter most. Strategy A: eslint-config-next - // still owns all React/Next/hooks linting above; this only adds promise safety. + // Narrow type-aware safety net for the async-heavy code paths (API routes, + // DB providers, and the storage layer). These rules need the real TypeScript + // type checker (projectService), so they are scoped to keep lint fast and to + // catch unhandled-promise bugs where they matter most. Strategy A: + // eslint-config-next still owns all React/Next/hooks linting above; this + // only adds promise safety. src/lib/storage/** joined this list in Phase 3 + // (0.10.0): the credential-encryption decorator wraps async provider + // methods, which is exactly the shape this layer exists to catch. ...tseslint.config({ - files: ["src/app/api/**/*.ts", "src/lib/db/**/*.ts"], + files: ["src/app/api/**/*.ts", "src/lib/db/**/*.ts", "src/lib/storage/**/*.ts"], languageOptions: { parserOptions: { projectService: true, diff --git a/operator/helm-charts/libredb-studio/README.md b/operator/helm-charts/libredb-studio/README.md index 16ada93c..8ed17996 100644 --- a/operator/helm-charts/libredb-studio/README.md +++ b/operator/helm-charts/libredb-studio/README.md @@ -256,7 +256,7 @@ key that opens the ciphertext it contains (with nothing set, the fallback key is same directory, alongside the database file): ```bash -helm install libredb-studio libredb-studio/libredb-studio \ +helm install libredb libredb/libredb-studio \ --set extraEnv[0].name=STORAGE_ENCRYPTION_KEY \ --set extraEnv[0].valueFrom.secretKeyRef.name=libredb-studio-storage \ --set extraEnv[0].valueFrom.secretKeyRef.key=encryption-key diff --git a/tests/integration/storage/sqlite-credential-encryption-node-harness.ts b/tests/integration/storage/sqlite-credential-encryption-node-harness.ts new file mode 100644 index 00000000..1cd75224 --- /dev/null +++ b/tests/integration/storage/sqlite-credential-encryption-node-harness.ts @@ -0,0 +1,91 @@ +/** + * Node-runtime harness for a real STORAGE_PROVIDER=sqlite credential-encryption round trip. + * + * Bun cannot load better-sqlite3 in-process ("'better-sqlite3' is not yet supported in Bun"), so + * the storage layer's SQLite provider - decorated with the same withCredentialEncryption() the + * factory applies in production - is exercised in a real `node` subprocess: the test bundles this + * file with `bun build --target=node --external better-sqlite3` and runs the bundle with + * `node `. + * + * What this proves that tests/security/credential-at-rest.test.ts cannot: that file uses an + * in-memory CaptureProvider standing in for a real store. This harness writes through a REAL + * better-sqlite3 file, checkpoints its WAL, and inspects the actual bytes on disk - the threat + * model the posture page names (a stolen file, dump, backup or snapshot) - rather than an + * in-process JavaScript object. + */ + +import fs from "node:fs"; +import { withCredentialEncryption } from "../../../src/lib/storage/encrypting-provider"; +import { resetStorageEncryptionKey } from "../../../src/lib/storage/encryption"; +import { SQLiteStorageProvider } from "../../../src/lib/storage/providers/sqlite"; +import type { DatabaseConnection } from "../../../src/lib/types"; + +const USER_ID = "u@example.org"; +const CANARY = "HARNESS-CANARY-PASSWORD"; + +function connection(): DatabaseConnection { + return { + id: "c1", + name: "Prod", + type: "postgres", + host: "db.internal", + password: CANARY, + createdAt: new Date(0), + }; +} + +async function main(): Promise { + const dbPath = process.argv[2]; + if (!dbPath) { + throw new Error("usage: node sqlite-credential-encryption-node-harness.mjs "); + } + + const report: Record = { runtime: typeof Bun === "undefined" ? "node" : "bun" }; + + // Phase 1: write a connection carrying the canary password, under the FIRST key. + process.env.JWT_SECRET = "harness-first-jwt-secret-at-least-32-chars"; + delete process.env.STORAGE_ENCRYPTION_KEY; + resetStorageEncryptionKey(); + + const writer = withCredentialEncryption(new SQLiteStorageProvider(dbPath)); + await writer.initialize(); + await writer.setCollection(USER_ID, "connections", [connection()]); + await writer.close(); + + // Checkpoint the WAL into the main file and inspect the ACTUAL PERSISTED ROW plus the RAW BYTES + // ON DISK - not the API surface, the file a backup or volume snapshot would actually contain. + const { default: Database } = await import("better-sqlite3"); + const raw = new Database(dbPath); + raw.pragma("wal_checkpoint(TRUNCATE)"); + const row = raw + .prepare("SELECT data FROM user_storage WHERE user_id = ? AND collection = ?") + .get(USER_ID, "connections") as { data: string }; + raw.close(); + + const fileBytes = fs.readFileSync(dbPath).toString("latin1"); + report.canaryInFile = fileBytes.includes(CANARY); + report.rowContainsCanary = row.data.includes(CANARY); + report.rowLooksSealed = /"password":"v1:/.test(row.data); + + // Phase 2: rotate the key (a fresh JWT_SECRET, exactly what an operator does) and read back + // through a NEW provider instance against the SAME on-disk file. + process.env.JWT_SECRET = "harness-second-jwt-secret-totally-different-value"; + resetStorageEncryptionKey(); + + const reader = withCredentialEncryption(new SQLiteStorageProvider(dbPath)); + await reader.initialize(); + const afterRotation = await reader.getCollection(USER_ID, "connections"); + await reader.close(); + + report.survivesRotation = Array.isArray(afterRotation) && afterRotation.length === 1; + const first = afterRotation?.[0] as (DatabaseConnection & Record) | undefined; + report.passwordOmittedAfterRotation = first ? !("password" in first) : null; + report.hostStillReadableAfterRotation = first?.host ?? null; + + console.log(JSON.stringify(report)); +} + +main().catch((error: unknown) => { + console.error(error); + process.exit(1); +}); diff --git a/tests/integration/storage/sqlite-credential-encryption.test.ts b/tests/integration/storage/sqlite-credential-encryption.test.ts new file mode 100644 index 00000000..05c5a0e9 --- /dev/null +++ b/tests/integration/storage/sqlite-credential-encryption.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, test, beforeAll, afterAll } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { existsSync, mkdtempSync, rmSync, symlinkSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +/** + * Closes the gap named in the pull request: the threat suite + * (tests/security/credential-at-rest.test.ts) proves the DECORATOR never hands plaintext to a + * provider, using an in-memory CaptureProvider standing in for one. Nothing in `bun run test` or + * `bun run test:e2e` (which runs with STORAGE_PROVIDER unset) exercises the decorator against a + * REAL STORAGE_PROVIDER=sqlite file. This does: it bundles the harness for Node (Bun cannot load + * better-sqlite3) and checks the actual bytes on disk after a write, and the actual read-back + * after a key rotation. + */ + +const nodeSqliteProbe = spawnSync("node", ["-e", "require('better-sqlite3'); process.exit(0)"], { timeout: 30_000 }); +const nodeBetterSqliteTestable = nodeSqliteProbe.status === 0; +if (!nodeBetterSqliteTestable) { + console.warn("Skipping the real-file credential-encryption test: `node` cannot load better-sqlite3 here"); +} + +describe.skipIf(!nodeBetterSqliteTestable)( + "credential encryption at rest against a real STORAGE_PROVIDER=sqlite file (posture control 3.1)", + () => { + let tmpDir: string; + + beforeAll(() => { + tmpDir = mkdtempSync(join(tmpdir(), "libredb-storage-sqlite-enc-")); + // The bundle is `--external better-sqlite3` (a native addon; a bundler cannot inline it), + // so plain Node module resolution needs a node_modules it can find by walking up from the + // bundle's own directory. tmpDir sits outside the project tree, so nothing is found without + // this: a symlink is cheaper and more honest than moving the bundle output into the repo. + symlinkSync(resolve(import.meta.dir, "../../../node_modules"), join(tmpDir, "node_modules"), "dir"); + }); + + afterAll(() => { + rmSync(tmpDir, { recursive: true, force: true }); + }); + + test("a canary password never reaches the file on disk, and a rotated key omits it on read instead of exposing it or crashing", () => { + const harnessEntry = join(import.meta.dir, "sqlite-credential-encryption-node-harness.ts"); + const bundlePath = join(tmpDir, "sqlite-credential-encryption-node-harness.mjs"); + const dbPath = join(tmpDir, "storage.db"); + + const build = spawnSync( + process.execPath, + [ + "build", + harnessEntry, + "--target=node", + "--format=esm", + "--external", + "better-sqlite3", + "--outfile", + bundlePath, + ], + { timeout: 60_000 }, + ); + if (build.status !== 0) { + throw new Error(`bun build failed: ${build.stderr?.toString()}`); + } + + const run = spawnSync("node", [bundlePath, dbPath], { timeout: 60_000 }); + if (run.status !== 0) { + throw new Error(`node harness failed: ${run.stderr?.toString() || run.stdout?.toString()}`); + } + + const report = JSON.parse(run.stdout.toString()) as Record; + + expect(report.runtime).toBe("node"); + expect(existsSync(dbPath)).toBe(true); // a real file-backed database, not an in-memory stand-in + + // The canary is never in the bytes a backup or volume snapshot would actually contain. + expect(report.canaryInFile).toBe(false); + expect(report.rowContainsCanary).toBe(false); + expect(report.rowLooksSealed).toBe(true); + + // A rotated key omits the field on the SAME on-disk file - it does not expose the old + // password, and it does not crash or drop the record. + expect(report.survivesRotation).toBe(true); + expect(report.passwordOmittedAfterRotation).toBe(true); + expect(report.hostStillReadableAfterRotation).toBe("db.internal"); + }); + }, +); diff --git a/tests/unit/lib/storage/encrypting-provider.test.ts b/tests/unit/lib/storage/encrypting-provider.test.ts index c141be33..fa2972a3 100644 --- a/tests/unit/lib/storage/encrypting-provider.test.ts +++ b/tests/unit/lib/storage/encrypting-provider.test.ts @@ -44,13 +44,22 @@ const snapshot: Record = {}; beforeEach(() => { snapshot.JWT_SECRET = process.env.JWT_SECRET; + snapshot.STORAGE_ENCRYPTION_KEY = process.env.STORAGE_ENCRYPTION_KEY; process.env.JWT_SECRET = "encrypting-provider-test-jwt-secret-32"; + // An ambient STORAGE_ENCRYPTION_KEY (a developer's local .env.local) takes precedence over + // JWT_SECRET in inputKeyMaterial(), so the "rotate JWT_SECRET" tests below would silently stop + // rotating anything - the derived key would never change, and the undecryptable-warning case + // they exist to exercise would never fire. Match the sibling test's isolation + // (tests/security/credential-at-rest.test.ts), which snapshots both variables. + delete process.env.STORAGE_ENCRYPTION_KEY; resetStorageEncryptionKey(); }); afterEach(() => { if (snapshot.JWT_SECRET === undefined) delete process.env.JWT_SECRET; else process.env.JWT_SECRET = snapshot.JWT_SECRET; + if (snapshot.STORAGE_ENCRYPTION_KEY === undefined) delete process.env.STORAGE_ENCRYPTION_KEY; + else process.env.STORAGE_ENCRYPTION_KEY = snapshot.STORAGE_ENCRYPTION_KEY; resetStorageEncryptionKey(); });