Skip to content

/admin/config leaks credential-bearing URLs (inline proxy creds, Slack webhook URL) despite 'never their value' promise #273

Description

@richardmhope

Summary

The /admin/config effective-config page renders resolved configuration to admins under an explicit promise (admin_config.html:11): "Secrets are shown only as a set / not-set status — never their value." Two credential-bearing URL classes escape this today because SECRET_FIELDS omits them and every DB-row field is hard-coded secret=False (src/iceberg/services/effective_config.py:35-52, 179-190):

  1. Inline proxy credentials. http://user:pass@proxy:3128 is the standard proxy-URL form and it works — services/proxy.py:79 only injects env credentials when proxy_username is set, otherwise the URL (userinfo included) passes through. The password renders verbatim in both the ICEBERG_PROXY_URL and Proxy.proxy_url rows, plus a second copy in the data-search attribute (admin_config.html:68).
  2. Slack webhook URLs. The webhook subsystem supports format: "slack" (services/webhook_settings.py:17); a Slack incoming-webhook URL (hooks.slack.com/services/T…/B…/secret) is bearer-equivalent by design. Webhook.url and ICEBERG_WEBHOOK_URL render in full.

Harm is amplified by an admin screen-sharing this page having been told it's safe.

Root cause / fragility

Redaction is allow-by-default: a new field renders in full unless someone remembers to add it to the frozenset. The guard test test_secret_looking_settings_are_all_classified_as_secret is good but (a) its name heuristic (_key/_secret/_password/_token) misses exactly the class that already required a manual add — credential-bearing URLs/DSNs (database_url, rate_limit_redis_url are listed only because someone remembered); a future sentry_dsn sails through; and (b) it runs over Settings.model_fields only — the six DB settings models get unconditional secret=False with no equivalent guard.

Suggested fix

Scrub userinfo from any URL-shaped value in _display; treat the webhook URL as a secret (or show host-only). Extend the heuristic guard over the DB models and add a _url/_dsn audit (even an "acknowledged-plaintext" allowlist forcing a conscious decision).

Found in Fable review of PR #232 → HEAD.

Metadata

Metadata

Assignees

No one assigned

    Labels

    fable-reviewCorrectness findings from Claude Fable code reviewpriority: mediumImportant, not blockingsecuritySecurity hardening / vulnerability

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions