Skip to content

fix(config): ~25% of TMI_* environment variables are absent from the generated config reference #810

Description

@ericfitz

Summary

config-reference.md doubles as this project's TMI_* environment-variable
allowlist, but 35 of the 142 declared env vars (~25%) never appear in it.
A variable missing from that file is effectively undocumented, and anything
that treats the file as an allowlist will not know it exists.

Found while adding a completeness guardrail during the config registry work.
The guardrail asserts that every emitted setting declaring an EnvVar has
that variable named in the generated reference — but it structurally cannot see
these 35, because they are never emitted in the first place.

Root cause

The generators consume Config.GetMigratableSettings(), which omits a setting
entirely when its value is empty. That omission is deliberate and load-bearing
for the database path: DefaultOperationalSettings() derives from the same
function and feeds both SeedDefaults and #794's origin backfill, so emitting
empty-valued settings would seed spurious rows and change seeded-vs-explicit
classification on existing databases.

The problem is that the same filter also drives documentation, where the
opposite is wanted: a setting with an empty default is exactly the one an
operator needs to be told about.

The 35

31 — omitted because their default is empty, including:

TMI_SERVER_BASE_URL              TMI_REDIS_URL
TMI_CORS_ALLOWED_ORIGINS         TMI_OAUTH_CLIENT_CALLBACK_ALLOWLIST
TMI_CONTENT_OAUTH_CALLBACK_URL   TMI_ALERTING_WEBHOOK_URL
TMI_ALERTING_WEBHOOK_SECRET      TMI_OPERATOR_NAME / _CONTACT / _JURISDICTION

plus all ~11 secrets-provider variables (AWS/GCP/Azure/OCI/Vault) and ~10
content-source picker and credential variables.

2 — omitted by a conditional on a different field: TMI_SERVER_TLS_CERT_FILE
and TMI_SERVER_TLS_KEY_FILE, skipped because the default config has TLS off.

2 — deliberately never migratable: TMI_CONTENT_TOKEN_ENCRYPTION_KEY and
TMI_ORACLE_WALLET_LOCATION. Their exclusion from the settings path is
correct and documented in ExpectedMigratableKeysSkipped(), but the env vars
themselves are real and operators still need them documented.

Impact

Documentation only — no runtime behavior is affected. But it undercuts the file's
role as an allowlist, and the affected set is not a random tail: it includes the
public base URL, the Redis URL, the OAuth callback allowlist, and every
secrets-provider variable. Those are among the most likely things an operator
needs to look up.

Suggested fix

Either:

  1. A registry-level completeness test over AllSettingDefs() (every def with
    EnvVar != ""), with an explicit, justified exception list — the same shape
    as the existing ExpectedMigratableKeysSkipped(). This catches the whole
    surface rather than the emitted subset.
  2. Point the reference generator at the registry directly rather than at the
    omitting projection, so documentation stops inheriting a filter that exists
    for database-seeding reasons. The generated example file can keep using the
    projection, since omitting empty values from a sample is reasonable.

Option 2 is the cleaner end state and is already earmarked in a doc comment, but
it changes generated output, so it wants its own review.

Notes

Pre-existing, not a regression — the hand-written builders this replaced had the
same conditional emission. It only became visible once a completeness gate
existed to measure it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdocumentationImprovements or additions to documentation

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions