Skip to content

sec(ci): other raw database-password interpolations and two DSN encoders after #555 #557

Closed

Description

@cristim

Found in the #555 review (merged). #555 percent-encodes the DB password for the pgx5 migration URL, but other builders still interpolate it raw or use a different encoder. Related: #301 (closed, same defect class in two builders; the remaining sites below were not covered).

Sites on origin/main (e372d3f):

  • .github/workflows/ci.yml:401: pgx5://${DB_USER}:${DB_PASSWORD}@... (the password is a throwaway test_password, so it works today but is a copy-paste hazard).
  • docs/DEPLOYMENT.md:675 and :689: raw ${DB_PASSWORD} in the documented migrate commands. An operator with @, :, /, % or # in the password hits the original bug.
  • internal/database/config.go:144: keyword DSN password=%s is unquoted; a space or single quote in the password breaks it (or injects keywords) if this path is used.
  • internal/testutil/postgres.go:79: postgresql://%s:%s@... built by Sprintf without escaping (test password only).
  • scripts/entrypoint.sh:66: encodes with awk, while fix(ci/db): percent-encode the DB password in the pgx5 migration URL #555 encodes with jq, so two encoders exist.

Failure scenario: password p@ss:w/rd in the deployment docs' migrate command produces a malformed URL and the migration fails to connect; a password with a space makes config.go's keyword DSN fail to parse.

Acceptance: one documented encoding approach is used by every builder above, DEPLOYMENT.md shows it, and the keyword DSN quotes and escapes the password, with a test using a password containing a space, quote, @, : and /.

Activity

  1. cristim commented on Oct 6, 2026

    @cristim
    MemberAuthor

    PR #588 merged (Refs, not Closes). Fixed: keyword DSN quoting (internal/database/config.go), testutil.ConnectionString, the ci.yml migrate step and both DEPLOYMENT.md migrate commands, verified against real Postgres for 15 hostile passwords. Remaining decision: scripts/entrypoint.sh:66 keeps its own awk percent-encoder. The reviewer ran it in alpine:3.24.1 against 12 hostile passwords and it matched jq -sRr @uri byte for byte; only a password containing a newline differs (awk drops it), which random_password does not emit. Choose: unify on one encoder (the container may lack jq) or document the two as equivalent and close this issue.

  2. IAmJSD commented on Oct 16, 2026

    @IAmJSD
    Collaborator

    Claiming: working on this

  3. self-assigned this
    on Oct 16, 2026
  4. added a commit that references this issue on Oct 17, 2026
    f288b31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions