Repository navigation
fix(import/d1): keep the destination password off psql's argv - #1486
Open
e-akashsaini wants to merge 1 commit into
Open
e-akashsaini wants to merge 1 commit into
e-akashsaini wants to merge 1 commit into
Conversation
runPsqlFile passed the full connection string, password included, as psql's first argument, exposing the 2-hour import role credential to any local user via ps. Strip the password from the keyword/value or URI connection string and pass it to psql via PGPASSWORD instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why?
runPsqlFilepasses the full destination connection string, password included, aspsql's first argument:cli/internal/import/d1/import.go
Line 678 in 956946a
Process arguments are world-readable (
ps,/proc/<pid>/cmdlineon Linux,pson macOS), so any other user on the machine can read the password while eachpsqlruns.pscale import d1runs it up to three times per import (tables, indexes, foreign keys). By default the credential is the roleResolveDestURIcreates: it inheritspostgresand is valid for 2 hours, so seeing it once is enough to use it long after the import finishes. Shared hosts (CI runners, bastions, shared dev boxes) are where this matters.The rest of the importer already keeps the password out of sight: the pgloader control file is written 0600, and
pscale shellpasses the Postgres password throughPGPASSWORD. This is the one call that puts it on the command line.How?
splitConnPasswordremoves the password from the connection string and returns it separately. It handles both forms libpq accepts:BuildConnectionStringproduces, with libpq quoting and escaping rulespostgres:///postgresql://URIs, either in the userinfo part or as a?password=parameterrunPsqlFilethen passes the stripped string topsqland setsPGPASSWORDin the child's environment. The environment can only be read by the same user or root.Checked with a fake
psqlonPATH. Before:password=...was in the arguments. After:The tests parse the original and stripped strings with
pgconn.ParseConfig(withPGPASSWORDset) and check that both resolve to the same host, port, user, database and password, so the stripped string connects the same way.Decisions
PGPASSWORDover a temporary.pgpassfile. A pgpass file would avoid the environment too, but it adds a temp file to create, set to 0600 and clean up on every exit path, and that file would sit on disk for the whole run.pscale shellalready usesPGPASSWORD, so this matches existing practice.runPsqlFilerather than changingResolveDestURI. The same string goes to pgx (in-process, so it's fine) and to pgloader (written to a 0600 control file). Only thepsqlchild needed changing, so the diff stays at that boundary.Follow-up
postgres.quoteValue(internal/postgres/postgres.go:184) escapes'but not\. libpq treats backslash as an escape inside quoted values, so a password containing\gets silently altered (a\secretbecomesasecret). That bug is older than this change and is better as its own PR. Generated role passwords probably never contain a backslash, butquoteValueis used for every option value.🤖 Generated with Claude Code