fix(datastore): escape DSN values so pgx parses them back unchanged

What does this MR do?

DSN.String builds the keyword/value connection string that sqlConnector.Open passes to pgx.ParseConfig (db.go#L216-217, db.go#L410). It escapes a space as \ and a single quote as \', and leaves backslashes as they are. pgx (v5.10.0 in go.mod) skips the character after a backslash while it looks for the end of a value, and then only decodes \\ and \' (parseKeywordValueSettings), so the backslash in front of a space stays in the parsed value. For the user, password, dbname, sslcert, sslkey and sslrootcert settings of the database section this means:

• a value with a space reaches pgx with an extra backslash (jw8s 0F4 is parsed as jw8s\ 0F4): a password with a space fails authentication, a database name with a space names a database that does not exist, and a sslrootcert path with a space cannot be read (unable to read CA file);
• a tab cuts the value at the tab, and what follows joins the name of the next setting, so that setting is lost too (with a tab in the password, dbname is lost);
• a value that ends with a backslash takes the next setting with it (password jw8s0F4\ is parsed as jw8s0F4\ dbname=registry, and dbname is lost), or makes pgx.ParseConfig fail with invalid backslash when it is the last one;
• \\ and \' inside a value lose their backslash.

The space escape came with a58f04aa (Add initial database connection logic, 2020-03-26), when the driver was lib/pq, which reads a backslash followed by any character as that character. Since 4788a2ee (Move from lib/pq to jackc/pgx for the PostgreSQL driver, 2021-01-25) the string goes to pgx, which keeps the backslash.

This MR escapes backslashes first and single quotes next, and wraps a value in single quotes when it contains whitespace, which is the keyword/value format of libpq that pgx follows. A value without whitespace or backslashes produces the same string as before, so the usual configurations see no change.

TestDSN_String/with_spaces asserted the old password=jw8s\ 0F4 without checking what pgx makes of it; it now expects password='jw8s 0F4', and two new cases cover a quote next to a space and backslashes. The new TestDSN_String_ParsedByPgx parses the string of 16 configurations with pgx.ParseConfig and compares the result with the configured values, and TestDSN_String_SSLRootCertPathWithSpace checks that a CA file in a directory with a space is loaded. Neither needs a database.

Behavior change: a value that contains \\ or \' used to reach pgx with one backslash less, and now reaches it as configured. If a configuration doubled a backslash to work around that, the extra one now reaches pgx and has to be removed. A single backslash followed by any other character (a\b) already reached pgx unchanged and still does. A value that starts or ends with whitespace other than a space (a password with a trailing tab or newline, for example) used to work by accident, because pgx cut that whitespace off, and is now quoted and reaches pgx as configured. If that whitespace is not part of the real value, it has to be removed from the configuration.

Closes #2989 (closed)

The tests, and how they fail on master

On master at e4e3ea44, Go 1.26.9, with this MR's tests and registry/datastore/db.go from master, trimmed to the assertion lines:

--- FAIL: TestDSN_String/with_spaces
        expected: "password='jw8s 0F4'"
        actual  : "password=jw8s\\ 0F4"
--- FAIL: TestDSN_String_ParsedByPgx/password_with_space
        expected: "jw8s 0F4"
        actual  : "jw8s\\ 0F4"
--- FAIL: TestDSN_String_ParsedByPgx/password_with_tab
        expected: "jw8s\t0F4"
        actual  : "jw8s"
--- FAIL: TestDSN_String_ParsedByPgx/password_with_trailing_backslash
        expected: "jw8s0F4\\"
        actual  : "jw8s0F4\\ dbname=registry"
--- FAIL: TestDSN_String_ParsedByPgx/last_value_with_trailing_backslash
        cannot parse `host=127.0.0.1 port=5432 user=registry password=xxxxx dbname=registry\`: failed to parse as keyword/value (invalid backslash)
--- FAIL: TestDSN_String_ParsedByPgx/password_with_two_backslashes
        expected: "jw8s\\\\0F4"
        actual  : "jw8s\\0F4"
--- FAIL: TestDSN_String_ParsedByPgx/password_with_backslash_before_quote
        expected: "jw8s\\'0F4"
        actual  : "jw8s'0F4"
--- FAIL: TestDSN_String_ParsedByPgx/user_with_space
        expected: "registry user"
        actual  : "registry\\ user"
--- FAIL: TestDSN_String_SSLRootCertPathWithSpace
        failed to configure TLS (unable to read CA file: open /tmp/.../with\ space/root.crt: no such file or directory)

The values are printed with Go's %q, so \\ is one backslash, and the last line is shortened to the TLS error, with the temporary directory cut. 12 of the 16 round-trip cases fail on master; the 4 that pass (plain, a single quote inside the value or at its start, a backslash before an ordinary character) already worked. With the change, every case passes.

A real connection, before and after

With the steps in the issue (PostgreSQL 17.11 checking passwords with SCRAM, a role whose password is jw8s 0F4), go run ./cmd/registry database migrate status /tmp/config.yml from master fails:

failed to construct database connection: verification failed: failed to connect to `user=registry database=registry`: 127.0.0.1:5432 (127.0.0.1): failed SASL auth: FATAL: password authentication failed for user "registry" (SQLSTATE 28P01)

From this branch it prints the migration status.

I also connected through datastore.NewConnector().Open to the postgresql-ci:17 image the database:* jobs use, with roles and databases for nine configurations: a plain one, passwords with a space, a tab, a trailing backslash, \' and \\, a user name with a space, a database name with a space, and a database name ending in a backslash as the last setting. From master only the plain one connects: the five passwords and the user fail with password authentication failed (SQLSTATE 28P01), the database name with a space gets database "registry\ db" does not exist, and the last one fails to parse with invalid backslash. From this branch all nine connect, as the configured user to the configured database.

Checks I ran locally

• go test ./registry/datastore/... on Go 1.25.12, 1.26.9 and 1.27.3 (the CI matrix): pass.
• go test -tags integration ./registry/datastore against postgresql-ci:17 with the variables of the database:datastore job, on Go 1.26.9: pass (519 tests).
• go test -race -count=3 -run TestDSN ./registry/datastore/: pass. The rest of the package also passes under -race, apart from TestDBLoadBalancer_ThrottledResolveReplicas and TestDBLoadBalancer_LivenessProbeReplica, which report data races in their test stubs on master too.
• golangci-lint run ./registry/datastore/... with v2.13.1 (the version CI pins) and the project's .golangci.yml, for amd64, 386, arm and arm64: 0 issues. go vet on the package and gofmt -l on both files: clean. go build ./...: pass.
• Mutation check: removing the backslash escaping, never quoting, quoting only on a plain space, and escaping quotes before backslashes each make the tests fail.
• commitlint with @commitlint/config-conventional in node:lts-alpine: 0 problems.

Author checklist

• CODEOWNERS Review: This MR requires approval from at least one CODEOWNER per category/file.
  • If codeowners are absent or the change is urgent, any registry maintainer can temporarily disable CODEOWNERS reviews in the project settings. When doing so:
    • Add a comment on the MR justifying the bypass
    • Mention/CC the designated codeowners in the MR for async review
    • Re-enable CODEOWNERS reviews in project settings once the MR is merged
  • If you lack permissions to disable CODEOWNERS reviews, reach out to the Engineering Manager (@jaime) or Senior Engineering Manager (@crystalpoole) for assistance.
• Assign one of conventional-commit prefixes to the MR.
  • fix: Indicates a bug fix, triggers a patch release.
  • feat: Signals the introduction of a new feature, triggers a minor release.
  • perf: Focuses on performance improvements that don't introduce new features or fix bugs, triggers a patch release.
  • docs: Updates or changes to documentation. Does not trigger a release.
  • style: Changes that do not affect the code's functionality. Does not trigger a release.
  • refactor: Modifications to the code that do not fix bugs or add features but improve code structure or readability. Does not trigger a release.
  • test: Changes related to adding or modifying tests. Does not trigger a release.
  • chore: Routine tasks that don't affect the application, such as updating build processes, package manager configs, etc. Does not trigger a release.
  • build: Changes that affect the build system or external dependencies. May trigger a release.
  • ci: Modifications to continuous integration configuration files and scripts. Does not trigger a release.
  • revert: Reverts a previous commit. It could result in a patch, minor, or major release.
• MR contains database changes including schema/background migrations: not applicable.
• Change contains a breaking change - apply the breaking change label: I do not think it is one, since only values that pgx was already reading differently from the configuration change; the one case that could need a configuration change is described under "Behavior change" above.
• Change is considered high risk - apply the label high-risk-change
• I created or linked to an existing issue for every added or updated TODO, BUG, FIXME or OPTIMIZE prefixed comment (there are none).
• Changes cannot be rolled back: not applicable, a revert restores the previous behavior.

Documentation/resources

Code review guidelines

Go Style guidelines

Feature flags

When documentation is required

Documentation workflow

Reviewer checklist

• Ensure the commit and MR tittle are still accurate.
• If the change contains a breaking change, verify the breaking change label.
• If the change is considered high risk, verify the label high-risk-change
• Identify if the change can be rolled back safely. (note: all other reasons for not being able to rollback will be sufficiently captured by major version changes).