fix(config): converge env/DB precedence onto one rule with an explicit-vs-seeded marker (#794)

[ericfitz]

Fixes #794.

The problem

Precedence between the config layer (YAML + env) and the system_settings table was decided per key by whichever accessor a caller happened to use. Five keys followed four different rules, and the gap caused a user-visible login outage on api.tmi.dev on 2026-08-20.

The two rules were deliberate and opposite, and each was right about half the problem:

Accessor	Rule	From
GetString	an explicit config value wins; a bare struct default yields to the DB	#415
GetDatabaseString	the DB always wins; config is a first-run fallback only	#767

#767 chose database-only for a real reason — reading through GetString inverted it, so env silently shadowed a row an admin had just edited. But it paid with the outage: RDS carried a registry-seeded default of http://localhost:8080/oauth2/callback, that row outranked a correctly-set TMI_OAUTH_CALLBACK_URL, and every provider rejected the resulting redirect_uri. Nothing warned.

The fix

The missing concept was on the database side. #415 already distinguished an operator-supplied config value from a struct default (MigratableSetting.Explicit); rows had no equivalent, so a seeded row and an admin-set row were indistinguishable. This adds system_settings.origin and collapses both accessors into GetResolvedString:

config explicit?	db explicit?	winner	why
no	no	database	both are defaults; prefer the hot-reloadable one
no	yes	database	#415 preserved
yes	no	config	fixes the outage
yes	yes	database	#419/#767 runtime editability preserved

Bootstrap keys are exempt, keyed on the classification registry so the guard holds even with no config provider wired.

The polarity is the safety property

Only a literal "explicit" reads as explicit. NULL, "seeded", an unexpected value and an empty string all read as seeded, so every way an origin can be lost — Oracle binding '' as NULL, a writer that forgets the stamp, a stale pre-upgrade Redis entry with no origin key — degrades toward config, the layer an operator can actually see and control.

BackfillSystemSettingOrigin stamps existing rows explicit only where they show operator intent (modified_by set, or the value has moved off the registry default), so deliberate rows keep their authority. For at-rest-encrypted values the comparison is meaningless (ciphertext never equals a plaintext default) and modified_by alone decides. A CHECK constraint enforces the domain on both engines.

Also here

• warnIfConfigDatabaseDiverges logs at startup when an explicit config value and a DB row disagree, naming both and the winner. This would have caught both 2026-08-20 regressions in seconds. Reads every row in one List rather than one query per key — as serial round-trips that was seconds of added startup on a cold Oracle ADB.
• auth.everyone_is_a_reviewer gains a runtime reader at all. Its only consumer read the config struct directly, so the DB row was visible in the admin API, editable, and completely inert. Memoized for 60s because it sits on every authenticated request, not just /oauth2/*.
• config-reference.md documents per-key precedence, including that auth.oauth.providers.* merges per-ID rather than following the scalar rule.

TMI_OAUTH_CALLBACK_URL is deliberately kept — it remains the fallback for the row-deletion case, and the converged rule makes it non-shadowing.

Verification

• make lint 0 issues · make build-server clean · make test-unit 2678 passed / 0 failed
• make test-integration 85 passed / 0 failed — up from 83/2. TestIdentityLink and TestIdentityLink_SecondConfirmRejected were failing on main for exactly this bug: a seeded row beating the harness's explicit TMI_OAUTH_CALLBACK_URL, sending the callback to the wrong port. Confirmed against a clean main worktree, which fails both with authorization_url = http://localhost:8080.
• oracle-db-admin: APPROVED WITH NOTES across three review rounds. Blocking findings fixed: a plain Update silently rewrote modified_at on every backfilled row (now UpdateColumn); encryption-mismatched rows originally defaulted to explicit, which reproduced the outage class on encrypted installs.
• Security review: one High finding, fixed. Redaction in the new warning went through Class.Secret, which is false for the auth.oauth.providers. / auth.saml.providers. / content_oauth.providers. subtrees — a blanket true there would mis-mask non-secret sub-keys like .client_id, so those carry secrecy only on the per-setting Secret flag. Checking Class.Secret alone would have written real OAuth client secrets and SAML private keys in plaintext to logs/tmi.log on every boot during a rotation. Now goes through a new MigratableSetting.IsSecret() so the two flags cannot be consulted inconsistently again, with a regression test that loads a real YAML config and asserts neither secret reaches any log line.

Deployment notes

• Do not run tmi-dbtool --import-config before the new server binary has booted against a given database — the server's migration adds the origin column, and dbtool ahead of it would fail on the unknown column.
• --import-config marks imported rows explicit, so after a fix(dev): make dev-cluster configuration survive a reset — provider Secret + DB settings snapshot/restore #792 snapshot/restore the database outranks env for those keys. That is the intended reading of "an operator deliberately put this value here", and the new startup warning makes it visible rather than silent.

Follow-ups filed

• feat(api): expose system_settings.origin (seeded vs explicit) through the admin settings API #803 — expose origin through the admin settings API (left out to avoid an OpenAPI regen and the api/api.go rebase conflicts it causes)
• test(oracle): add Oracle-side integration coverage for system_settings.origin precedence (#794) #804 — Oracle-side coverage; every new test runs on SQLite, which cannot reproduce the ''-is-NULL mechanics the feature depends on
• fix(settings): ReEncryptAll stamps modified_by on every row, making it a poor proxy for operator intent (#794) #805 — ReEncryptAll stamps modified_by on every row, weakening it as an intent signal for the backfill on installs that have rotated their encryption key

Refs #419, #767, #415.

🤖 Generated with Claude Code

https://claude.ai/code/session_018YadGYzmxyMtJbxanvtzRJ