Skip to content

Redact DB credentials from ConnectionFailure detail (password + url userinfo) #12

Description

@HumanBean17

Problem

When a database connection fails, PostgreSQLDriver.connect attaches the full connection config — including secrets — to the error's detail, which is surfaced to the user/logs via the output envelope.

agctl/clients/db_drivers/postgresql.py:68-71:

raise ConnectionFailure(
    message=str(exc),
    detail={"driver": "postgresql", "config": dict(config)},
) from exc

config contains:

  • password (discrete field) — always present when configured
  • url (connection URI) — may embed credentials as user:pass@host (e.g. postgresql://user:s3cr3t@host/db)

AgctlError.to_dict() (agctl/errors.py:20-21) puts detail straight into the output envelope's error field, so these secrets reach stdout/structured logs on every connection failure.

Impact

  • A wrong host/port or transient outage produces a ConnectionError whose JSON envelope includes the cleartext password — exactly the failure mode most likely to be pasted into a ticket, chat, or log aggregator.
  • This is pre-existing (the discrete password leaked before the url feature); the url field is at parity (not a new channel), but it does mean credentials can now appear twice (as password and embedded in url). Not a regression from feat(db): add optional url connection field #11, but feat(db): add optional url connection field #11 is a good moment to fix it.

Proposed fix

Redact secrets before they enter detail:

  • Mask password (e.g. "***" or omit).
  • Strip the userinfo segment from url when present (e.g. postgresql://user:pass@host:5432/dbpostgresql://***@host:5432/db). psycopg ships psycopg.conninfo helpers for parsing conninfo/URIs; alternatively a small regex on the ://user:pass@ segment.

Keep the non-secret fields (host, port, dbname, user, type, driver) — they're genuinely useful for diagnosing a connection failure. Only credentials need redaction.

Scope notes

  • This site (connect) is the primary one. Worth a quick sweep for other places that embed raw connection config into errors/details (e.g. execute / execute_write currently use message=str(exc) without the config dict, so they may already be safe — confirm during the fix).
  • Consider centralizing a redact_connection_config(config) helper so future drivers/fields stay safe by default.

Tests

  • Unit: a connect() that raises psycopg.Error produces a ConnectionFailure whose detail["config"] has no cleartext password and no user:pass@ in url, while non-secret fields survive.

Context

Surfaced during code review of #11 (DB url connection support). That PR was merged at parity with the existing password-in-detail behavior; redaction is tracked here rather than scope-creeping #11.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions