Fail fast on insecure configuration in production #14

Closed
opened 2026-07-28 04:52:53 +00:00 by claude-bot · 2 comments

Context

CIRCA_SECRET_KEY defaults to the literal string change-me-in-production, which is
documented in the README. That default signs the session cookies. If the app is ever
deployed without overriding it, every session cookie is forgeable by anyone who has
read the README — and nothing in the app would report a problem.

Scope

Fail-fast validation of security-critical configuration at startup, gated on
environment.

Implementation notes

  • Add an explicit environment setting (CIRCA_ENV = development | production).
  • In production, refuse to start when:
    • CIRCA_SECRET_KEY is unset, still the default, or below a minimum entropy/length
    • OAuth client id/secret are unset
    • CIRCA_OAUTH_REDIRECT_URI is not HTTPS
    • the dev-login bypass is enabled
  • Fail loudly at startup with a precise message naming the offending variable. Do not
    warn and continue — a warning in a log nobody reads is how this default reaches production.
  • In development, keep the current permissive behaviour so local setup stays easy,
    but log a single clear notice that insecure defaults are in use.
  • Review session cookie attributes while here: HttpOnly, Secure in production,
    SameSite, and a sensible expiry.
  • Update the README so the default is presented as development-only.

Done when

  • Production startup aborts on any insecure default with a message naming the variable
  • Development startup is unchanged apart from one notice
  • Session cookie flags are correct per environment
  • A test covers both the refusal and the permissive path

References

  • README.md env var table (CIRCA_SECRET_KEY default change-me-in-production)
  • backend/app/auth/session.py, backend config module
  • AGENTS.md: "Security is very important when building the project, and should be a priority"
## Context `CIRCA_SECRET_KEY` defaults to the literal string `change-me-in-production`, which is documented in the README. That default signs the session cookies. If the app is ever deployed without overriding it, every session cookie is forgeable by anyone who has read the README — and nothing in the app would report a problem. ## Scope Fail-fast validation of security-critical configuration at startup, gated on environment. ## Implementation notes - Add an explicit environment setting (`CIRCA_ENV` = `development` | `production`). - In production, refuse to start when: - `CIRCA_SECRET_KEY` is unset, still the default, or below a minimum entropy/length - OAuth client id/secret are unset - `CIRCA_OAUTH_REDIRECT_URI` is not HTTPS - the dev-login bypass is enabled - Fail loudly at startup with a precise message naming the offending variable. Do not warn and continue — a warning in a log nobody reads is how this default reaches production. - In development, keep the current permissive behaviour so local setup stays easy, but log a single clear notice that insecure defaults are in use. - Review session cookie attributes while here: `HttpOnly`, `Secure` in production, `SameSite`, and a sensible expiry. - Update the README so the default is presented as development-only. ## Done when - [ ] Production startup aborts on any insecure default with a message naming the variable - [ ] Development startup is unchanged apart from one notice - [ ] Session cookie flags are correct per environment - [ ] A test covers both the refusal and the permissive path ## References - `README.md` env var table (`CIRCA_SECRET_KEY` default `change-me-in-production`) - `backend/app/auth/session.py`, backend config module - `AGENTS.md`: "Security is very important when building the project, and should be a priority"
claude-bot added this to the v0.2.0 milestone 2026-07-28 04:52:53 +00:00
Author

Amended by the audit of 2026-07-28.

Two things this issue does not currently capture:

  1. The amplifier that makes the default key directly exploitable (#57). The session cookie
    contains only a user UUID, and the API hands out other users' UUIDs via created_by on
    evidence, decisions, comments, and jobs. So forging an admin session is: authenticate → read
    decisions → collect the admin UUID → sign a cookie with the README-published default key. No
    cryptanalysis. Without that disclosure the attacker would need to guess a UUIDv4.

  2. The environment default points the wrong way. CIRCA_ENVIRONMENT defaults to
    development, and no .env exists in the repo — so an operator who sets only the OAuth vars
    gets Secure=False cookies, credentialed CORS for localhost:5173, and echo=True, which
    logs every SQL statement including comment bodies and notes. Recommend defaulting to
    production so the failure mode is safe, and gating SQL echo on its own setting rather than on
    environment.

**Amended by the audit of 2026-07-28.** Two things this issue does not currently capture: 1. **The amplifier that makes the default key directly exploitable (#57).** The session cookie contains only a user UUID, and the API hands out *other users'* UUIDs via `created_by` on evidence, decisions, comments, and jobs. So forging an admin session is: authenticate → read decisions → collect the admin UUID → sign a cookie with the README-published default key. No cryptanalysis. Without that disclosure the attacker would need to guess a UUIDv4. 2. **The environment default points the wrong way.** `CIRCA_ENVIRONMENT` defaults to `development`, and no `.env` exists in the repo — so an operator who sets only the OAuth vars gets `Secure=False` cookies, credentialed CORS for `localhost:5173`, and `echo=True`, which logs every SQL statement **including comment bodies and notes**. Recommend defaulting to `production` so the failure mode is safe, and gating SQL echo on its own setting rather than on environment.
Author

Done in f61226a. backend/tests/test_startup_config.py, 27 tests.

The decision worth flagging

The issue body says "keep the current permissive behaviour so local setup stays easy"; the audit comment says the environment default points the wrong way. CIRCA_ENVIRONMENT now defaults to production — I asked, and that was the call.

The reasoning is the audit's: there is no .env in the repo, so an operator who set only the OAuth variables previously got Secure=False cookies, credentialed CORS for localhost:5173, SQL echo printing note and comment bodies to the log, interactive docs published to anyone who could reach the host, and session cookies signed with the key printed in the README. Nothing anywhere reported a problem. Failing closed costs one line in a local .env, and the refusal message names it.

It is also a Literal now. As a free-form str, CIRCA_ENVIRONMENT=prod would have compared unequal to "development", satisfied every is_development check, and then sailed past a production guard keyed on the exact word — permissive and unvalidated at once. It is now a startup error.

What production refuses to start on

  • CIRCA_SECRET_KEY unset, still the documented default, or under 32 characters
  • CIRCA_OAUTH_CLIENT_ID / CIRCA_OAUTH_CLIENT_SECRET unset
  • an unresolvable or plaintext OIDC discovery URL — reached through the same resolved_server_metadata_url property the flow uses, so the check cannot pass while the flow fails (previously a raw ValueError from register_provider half way through building the app)
  • CIRCA_OAUTH_REDIRECT_URI not HTTPS — the authorization code is delivered there, and a code is a session
  • CIRCA_DEV_LOGIN_ENABLED on (#15's guard, in place before #15's route)

Two properties of the refusal are load-bearing and tested as such:

It raises, it does not warn. A warning is precisely what the default key would have produced: a line in a log nobody reads, on a deployment that keeps serving. There is no state of this application worth having where it runs with a signing key published in its own README.

It reports every problem at once. An operator who fixes one variable, restarts, and meets the next refusal three times is an operator who gives up half-configured — which is the state this issue exists to prevent. The message names each variable, includes the secrets.token_urlsafe command, and names CIRCA_ENVIRONMENT=development as the way to run locally, because a refusal that does not say that teaches people to disable the check instead of configuring the deployment.

The audit's second point, taken

SQL echo moved to its own CIRCA_SQLITE_ECHO, off by default, rather than being implied by environment == "development". The echoed statements include note and comment bodies — what a relative said about a photograph — and "am I in development" is not the same question as "may this go in a log file."

Development

Unchanged and permissive: every value that fails in production passes there. It now logs one line naming the concessions in play (default key, no Secure, credentialed CORS, the bypass, SQL echo), because silently permissive is how the default reached a deployment in the first place.

Reviewed as asked and now pinned per environment: HttpOnly and SameSite=strict always, Secure outside development, Max-Age present — and the deletion mirrors the attributes, because an asymmetry there invites someone to add a path to one side only and quietly break logout.

Done when

  • Production startup aborts on any insecure default with a message naming the variable
  • Development startup is unchanged apart from one notice
  • Session cookie flags are correct per environment
  • A test covers both the refusal and the permissive path

README env table and .env.example updated; the default is now presented as development-only. 992 passed, 8 skipped; ruff clean.

One knock-on: CI's backend job now sets CIRCA_ENVIRONMENT=development, because importing app.main to export a schema or run migrations is not a deployment.

Done in f61226a. `backend/tests/test_startup_config.py`, 27 tests. ## The decision worth flagging The issue body says "keep the current permissive behaviour so local setup stays easy"; the audit comment says the environment default points the wrong way. **`CIRCA_ENVIRONMENT` now defaults to `production`** — I asked, and that was the call. The reasoning is the audit's: there is no `.env` in the repo, so an operator who set only the OAuth variables previously got `Secure=False` cookies, credentialed CORS for `localhost:5173`, SQL echo printing note and comment bodies to the log, interactive docs published to anyone who could reach the host, and session cookies signed with the key printed in the README. Nothing anywhere reported a problem. Failing closed costs one line in a local `.env`, and the refusal message names it. It is also a `Literal` now. As a free-form `str`, `CIRCA_ENVIRONMENT=prod` would have compared unequal to `"development"`, satisfied every `is_development` check, and then sailed past a production guard keyed on the exact word — permissive **and** unvalidated at once. It is now a startup error. ## What production refuses to start on - `CIRCA_SECRET_KEY` unset, still the documented default, or under 32 characters - `CIRCA_OAUTH_CLIENT_ID` / `CIRCA_OAUTH_CLIENT_SECRET` unset - an unresolvable or plaintext OIDC discovery URL — reached through the same `resolved_server_metadata_url` property the flow uses, so the check cannot pass while the flow fails (previously a raw `ValueError` from `register_provider` half way through building the app) - `CIRCA_OAUTH_REDIRECT_URI` not HTTPS — the authorization code is delivered there, and a code is a session - `CIRCA_DEV_LOGIN_ENABLED` on (#15's guard, in place before #15's route) Two properties of the refusal are load-bearing and tested as such: **It raises, it does not warn.** A warning is precisely what the default key would have produced: a line in a log nobody reads, on a deployment that keeps serving. There is no state of this application worth having where it runs with a signing key published in its own README. **It reports every problem at once.** An operator who fixes one variable, restarts, and meets the next refusal three times is an operator who gives up half-configured — which is the state this issue exists to prevent. The message names each variable, includes the `secrets.token_urlsafe` command, and names `CIRCA_ENVIRONMENT=development` as the way to run locally, because a refusal that does not say that teaches people to disable the check instead of configuring the deployment. ## The audit's second point, taken **SQL echo moved to its own `CIRCA_SQLITE_ECHO`**, off by default, rather than being implied by `environment == "development"`. The echoed statements include note and comment bodies — what a relative said about a photograph — and "am I in development" is not the same question as "may this go in a log file." ## Development Unchanged and permissive: every value that fails in production passes there. It now logs one line naming the concessions in play (default key, no `Secure`, credentialed CORS, the bypass, SQL echo), because *silently* permissive is how the default reached a deployment in the first place. ## Cookie flags Reviewed as asked and now pinned per environment: `HttpOnly` and `SameSite=strict` always, `Secure` outside development, `Max-Age` present — and the deletion mirrors the attributes, because an asymmetry there invites someone to add a `path` to one side only and quietly break logout. ## Done when - [x] Production startup aborts on any insecure default with a message naming the variable - [x] Development startup is unchanged apart from one notice - [x] Session cookie flags are correct per environment - [x] A test covers both the refusal and the permissive path README env table and `.env.example` updated; the default is now presented as development-only. **992 passed, 8 skipped**; ruff clean. One knock-on: CI's backend job now sets `CIRCA_ENVIRONMENT=development`, because importing `app.main` to export a schema or run migrations is not a deployment.
Sign in to join this conversation.
No description provided.