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/.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/SECURITY.md b/SECURITY.md index ea87c47f..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. @@ -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/Chart.yaml b/charts/libredb-studio/Chart.yaml index 487f5d00..92061161 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 @@ -28,10 +28,10 @@ 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.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 266c707a..8ed17996 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 ``` @@ -245,6 +245,27 @@ 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, 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 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 +``` + +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/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/docs/SECURITY.md b/docs/SECURITY.md new file mode 100644 index 00000000..3115e645 --- /dev/null +++ b/docs/SECURITY.md @@ -0,0 +1,112 @@ +# 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) | [`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) | +| 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), [`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) | + +## 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. 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 +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 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 + 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 +``` diff --git a/docs/STORAGE.md b/docs/STORAGE.md index edea6551..591dc838 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,99 @@ 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 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. + +--- + ## Environment Variables Reference | Variable | Required | Default | Description | @@ -411,6 +505,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 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/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..92061161 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 @@ -28,10 +28,10 @@ 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.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..8ed17996 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,27 @@ 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, 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 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 +``` + +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 12da036f..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" @@ -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..1d70888b --- /dev/null +++ b/scripts/security-check.mjs @@ -0,0 +1,279 @@ +#!/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. + * + * 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. 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. + * + * 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. `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, requireTest = false }) { + if (!target.startsWith("tests/") && !target.startsWith("e2e/")) { + 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); + 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)) { + 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) { + // 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 + // 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`); + + // 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; +} + +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/src/lib/storage/connection-secrets.ts b/src/lib/storage/connection-secrets.ts new file mode 100644 index 00000000..fea7ee23 --- /dev/null +++ b/src/lib/storage/connection-secrets.ts @@ -0,0 +1,184 @@ +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 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 === "decrypted" ? value : encryptSecret(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: 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); +} + +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/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/encryption.ts b/src/lib/storage/encryption.ts new file mode 100644 index 00000000..2c4dfaf3 --- /dev/null +++ b/src/lib/storage/encryption.ts @@ -0,0 +1,167 @@ +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 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 + * + * 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/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 `. + * + * 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/isolated/factory-singleton.test.ts b/tests/isolated/factory-singleton.test.ts index 4235e8b2..0744d8ef 100644 --- a/tests/isolated/factory-singleton.test.ts +++ b/tests/isolated/factory-singleton.test.ts @@ -98,6 +98,22 @@ describe("factory: getStorageProvider", () => { 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..14ada8a0 --- /dev/null +++ b/tests/security/credential-at-rest.test.ts @@ -0,0 +1,226 @@ +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"; +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:"); + }); +}); + +/** + * 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; + } + }); +}); 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/lib/storage/connection-secrets.test.ts b/tests/unit/lib/storage/connection-secrets.test.ts new file mode 100644 index 00000000..fa2a6337 --- /dev/null +++ b/tests/unit/lib/storage/connection-secrets.test.ts @@ -0,0 +1,279 @@ +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, readSecret, 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(""); + }); + + 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", () => { + 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 }); + }); +}); 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..fa2972a3 --- /dev/null +++ b/tests/unit/lib/storage/encrypting-provider.test.ts @@ -0,0 +1,169 @@ +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; + 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(); +}); + +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/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" }); + }); +}); 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(() => []), diff --git a/tests/unit/security-check.test.ts b/tests/unit/security-check.test.ts new file mode 100644 index 00000000..9ab7dbb7 --- /dev/null +++ b/tests/unit/security-check.test.ts @@ -0,0 +1,314 @@ +import { describe, expect, test } from "bun:test"; +import path from "node:path"; +import { + checkPosture, + findControlTable, + isExecuted, + linkTargets, + parseTables, + 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. + */ + +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); + }); + + 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", () => { + 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("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", + componentsRunner: COMPONENTS_RUNNER, + playwrightConfig: PLAYWRIGHT_CONFIG, + exists: () => true, + securityTestFiles: CLEAN_SECURITY_TESTS, + }); + + expect(violations).toHaveLength(1); + 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"); + }); +});