[Hardening] Reject placeholder and weak secrets at startup

claude-bot commented

2026-07-14 19:49:51 +00:00

Collaborator

Context

The backend accepts whatever secrets it is given — there is no strength or placeholder check anywhere:

webapp/backend/app/config.py:28 — secret_key: str (required but unvalidated). This value both signs sessions and derives the AES-GCM key that encrypts all stored secrets (webapp/backend/app/crypto.py:35-46, _derive_key), so a guessable SECRET_KEY exposes every credential in the settings table.
webapp/backend/app/config.py:53 — bot_api_key: str = "" (defaults to empty; an empty shared key weakens backend↔bot auth).
Database credentials arrive embedded in database_url / database_migrate_url (config.py:31-33); there is no standalone password field to validate.

.env.example ships guessable placeholders that a self-hoster can miss:

:24 SECRET_KEY=replace-with-openssl-rand-hex-32
:35 POSTGRES_PASSWORD=changeme
:38 POSTGRES_MIGRATE_PASSWORD=changeme_migrate
:41 / :45 — the same passwords appear inside DATABASE_URL / DATABASE_MIGRATE_URL

Motivation

A deployment that copies .env.example and misses one value boots successfully with guessable credentials. Failing fast at startup with an actionable message is the cheapest possible protection for self-hosters.

Fix / Spec

Add pydantic field_validators in webapp/backend/app/config.py:

secret_key: reject the exact .env.example placeholder (replace-with-openssl-rand-hex-32) and enforce len >= 32.
database_url / database_migrate_url: reject when the parsed password component is changeme or changeme_migrate (parse with urllib.parse/SQLAlchemy URL, don't substring-match the whole URL).
bot_api_key: reject the empty string is not desired if the bot is optional — instead reject only known placeholder values (e.g. changeme) and values shorter than 16 chars when set. (If the codebase treats empty as "bot not configured", keep that semantic — verify how require_bot_auth handles an empty key and close any bypass there.)
Each rejection must raise with a message naming the exact env var and the fix, e.g.:
SECRET_KEY is still the .env.example placeholder. Generate one with: openssl rand -hex 32
Update backend test fixtures/CI env to use compliant values so the suite stays green.

Acceptance criteria

Startup with any .env.example placeholder secret exits immediately with a message naming the env var and the openssl rand -hex 32 generation command.
SECRET_KEY shorter than 32 chars is rejected.
A DB URL whose password is changeme/changeme_migrate is rejected.
Tests cover each validator (accept + reject cases).
Full backend test suite green.

References

webapp/backend/app/config.py:28 (secret_key), :31-33 (DB URLs), :53 (bot_api_key)
webapp/backend/app/crypto.py:35-46 (_derive_key from SECRET_KEY)
.env.example:24, :35, :38, :41, :45
webapp/backend/app/auth/dependencies.py (require_bot_auth — empty-key behavior)

Filed from the July 2026 full-project review.

## Context The backend accepts whatever secrets it is given — there is no strength or placeholder check anywhere: - `webapp/backend/app/config.py:28` — `secret_key: str` (required but unvalidated). This value both signs sessions **and derives the AES-GCM key that encrypts all stored secrets** (`webapp/backend/app/crypto.py:35-46`, `_derive_key`), so a guessable `SECRET_KEY` exposes every credential in the settings table. - `webapp/backend/app/config.py:53` — `bot_api_key: str = ""` (defaults to empty; an empty shared key weakens backend↔bot auth). - Database credentials arrive embedded in `database_url` / `database_migrate_url` (`config.py:31-33`); there is no standalone password field to validate. `.env.example` ships guessable placeholders that a self-hoster can miss: - `:24` `SECRET_KEY=replace-with-openssl-rand-hex-32` - `:35` `POSTGRES_PASSWORD=changeme` - `:38` `POSTGRES_MIGRATE_PASSWORD=changeme_migrate` - `:41` / `:45` — the same passwords appear inside `DATABASE_URL` / `DATABASE_MIGRATE_URL` ## Motivation A deployment that copies `.env.example` and misses one value boots **successfully** with guessable credentials. Failing fast at startup with an actionable message is the cheapest possible protection for self-hosters. ## Fix / Spec Add pydantic `field_validator`s in `webapp/backend/app/config.py`: 1. `secret_key`: reject the exact `.env.example` placeholder (`replace-with-openssl-rand-hex-32`) and enforce `len >= 32`. 2. `database_url` / `database_migrate_url`: reject when the parsed password component is `changeme` or `changeme_migrate` (parse with `urllib.parse`/SQLAlchemy URL, don't substring-match the whole URL). 3. `bot_api_key`: reject the empty string is **not** desired if the bot is optional — instead reject only known placeholder values (e.g. `changeme`) and values shorter than 16 chars *when set*. (If the codebase treats empty as "bot not configured", keep that semantic — verify how `require_bot_auth` handles an empty key and close any bypass there.) 4. Each rejection must raise with a message naming the exact env var and the fix, e.g.: `SECRET_KEY is still the .env.example placeholder. Generate one with: openssl rand -hex 32` 5. Update backend test fixtures/CI env to use compliant values so the suite stays green. ## Acceptance criteria - Startup with any `.env.example` placeholder secret exits immediately with a message naming the env var and the `openssl rand -hex 32` generation command. - `SECRET_KEY` shorter than 32 chars is rejected. - A DB URL whose password is `changeme`/`changeme_migrate` is rejected. - Tests cover each validator (accept + reject cases). - Full backend test suite green. ## References - `webapp/backend/app/config.py:28` (`secret_key`), `:31-33` (DB URLs), `:53` (`bot_api_key`) - `webapp/backend/app/crypto.py:35-46` (`_derive_key` from SECRET_KEY) - `.env.example:24`, `:35`, `:38`, `:41`, `:45` - `webapp/backend/app/auth/dependencies.py` (`require_bot_auth` — empty-key behavior) _Filed from the July 2026 full-project review._

claude-bot added the

backend

enhancement

labels

2026-07-14 19:49:58 +00:00

claude-bot added this to the v3.3.0 - Hardening: Bugs & Security milestone

2026-07-14 19:49:59 +00:00

claude-bot referenced this issue

2026-07-14 20:26:35 +00:00

[Hardening] Replace builtin hash() with a deterministic digest in lore match chunk indexing #88

claude-bot referenced this issue

2026-07-14 20:26:41 +00:00

[Hardening] Remove duplicate reminder delivery on session reschedule #90

claude-bot referenced this issue

2026-07-14 20:26:42 +00:00

[Hardening] Validate campaign discord_webhook_url against a Discord host allowlist (SSRF) #97

claude-bot commented

2026-07-14 20:26:45 +00:00

Author

Collaborator

Picking this up as part of a v3.3.0 push. Landing on branch hardening/backend together with #88, #90, #97, and #109 (grouped by component to keep the diffs reviewable).

Note for implementation: CI (.forgejo/workflows/ci.yml) currently sets SECRET_KEY: ci-secret-not-used-in-tests — 27 chars, which the new len >= 32 rule rejects. CI env has to move to a compliant dummy in the same commit or the build breaks.

Picking this up as part of a v3.3.0 push. Landing on branch `hardening/backend` together with #88, #90, #97, and #109 (grouped by component to keep the diffs reviewable). Note for implementation: CI (`.forgejo/workflows/ci.yml`) currently sets `SECRET_KEY: ci-secret-not-used-in-tests` — 27 chars, which the new `len >= 32` rule rejects. CI env has to move to a compliant dummy in the same commit or the build breaks.

claude-bot referenced this issue

2026-07-14 20:26:47 +00:00

[Hardening] Escape HTML in recap emails; stop echoing raw LLM errors from /ask #109

claude-bot commented

2026-07-14 21:33:53 +00:00

Author

Collaborator

Fixed on main (commit f25ee12, merged via 1c9c19f). Pydantic field_validators in config.py: secret_key rejects the exact .env.example placeholder and enforces len >= 32; database_url/database_migrate_url reject changeme/changeme_migrate matched against the parsed password (via SQLAlchemy make_url, not a substring — a DB whose name is changeme is accepted); bot_api_key allows empty (see below) but rejects known placeholders and <16 chars when set. Each message names the env var and gives openssl rand -hex 32. CI's SECRET_KEY (was 27 chars) updated to a compliant dummy in the same commit.

require_bot_auth empty-key finding — not a bypass, it is fail-closed. get_bot_api_key returns None for an empty key, and the dependency does if not stored or not compare_digest(...) → 401, so every bot request is rejected when no key is configured. Empty genuinely means "bot not configured", so it's left permitted; added a regression test asserting the 401. Tests (12) cover accept + reject per validator. Backend suite green (358 passed), ruff clean.

Fixed on `main` (commit `f25ee12`, merged via `1c9c19f`). Pydantic `field_validator`s in `config.py`: `secret_key` rejects the exact `.env.example` placeholder and enforces `len >= 32`; `database_url`/`database_migrate_url` reject `changeme`/`changeme_migrate` matched against the **parsed** password (via SQLAlchemy `make_url`, not a substring — a DB whose *name* is `changeme` is accepted); `bot_api_key` allows empty (see below) but rejects known placeholders and `<16` chars when set. Each message names the env var and gives `openssl rand -hex 32`. CI's `SECRET_KEY` (was 27 chars) updated to a compliant dummy in the same commit. **`require_bot_auth` empty-key finding — not a bypass, it is fail-closed.** `get_bot_api_key` returns `None` for an empty key, and the dependency does `if not stored or not compare_digest(...) → 401`, so every bot request is rejected when no key is configured. Empty genuinely means "bot not configured", so it's left permitted; added a regression test asserting the 401. Tests (12) cover accept + reject per validator. Backend suite green (358 passed), ruff clean.

claude-bot closed this issue

2026-07-14 21:34:08 +00:00