From 1773d5a862709c078a6b647ae646cb9facb9fb39 Mon Sep 17 00:00:00 2001 From: Diego Date: Mon, 31 Aug 2026 10:30:54 -0300 Subject: [PATCH 1/3] fix(gather): forward --timeout to collect.Run so the flag is honored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gather() dropped f.timeout, so collect.Run fell back to its own 20s+interval budget and --timeout was silently ignored on every command routed through gather (vacuum, tables, indexes, queries, ask) — the exact flag whose help text says to raise it for slow or remote databases. --- cmd/pgbot/gather.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/pgbot/gather.go b/cmd/pgbot/gather.go index 8026a15..16e8cb9 100644 --- a/cmd/pgbot/gather.go +++ b/cmd/pgbot/gather.go @@ -30,7 +30,11 @@ func gather(ctx context.Context, connString string, f inspectFlags) (*model.Cont fmt.Fprintln(os.Stderr, target.Pooler.Note()) } - c, err := collect.Run(ctx, target, collect.Options{Interval: f.interval, ASHHz: f.ashHz, ASHWindow: f.window}) + // Deadline must be forwarded: without it collect.Run falls back to its own + // 20s+interval budget and every --timeout on the commands routed through + // gather (vacuum, tables, indexes, queries, ask) is silently ignored — the + // exact flag whose help text says to raise it for slow or remote databases. + c, err := collect.Run(ctx, target, collect.Options{Interval: f.interval, ASHHz: f.ashHz, ASHWindow: f.window, Deadline: f.timeout}) if err != nil { return nil, "", fmt.Errorf("collect: %s", conn.RedactConnString(err.Error())) } From 377567465e46a6de74f8cab35e911b5a50e64bb0 Mon Sep 17 00:00:00 2001 From: Diego Date: Mon, 31 Aug 2026 10:30:54 -0300 Subject: [PATCH 2/3] =?UTF-8?q?feat(conn):=20--ssh-tunnel=20=E2=80=94=20re?= =?UTF-8?q?ach=20a=20database=20through=20an=20SSH=20jump=20host?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A managed database on a private network (RDS/Aurora inside a VPC, a Postgres behind a bastion) is unreachable from a laptop without a jump host. --ssh-tunnel, or $PGBOT_SSH_TUNNEL, routes the TCP leg through one. The tunnel is installed as pgx's DialFunc rather than as a local port forward. pgconn documents DialFunc as running before TLS is established, so the DSN keeps naming the real host all the way through: sslmode= verify-full still validates against that hostname and .pgpass still matches on it. An `ssh -L` forward would force the DSN to say 127.0.0.1, silently breaking both, besides leaving a port open to every local user. Host identity is not pgbot's policy to invent. StrictHostKeyChecking, UserKnownHostsFile, IdentityFile, IdentitiesOnly, IdentityAgent, User and Port are all read from the user's ssh_config, so pgbot behaves the way their own ssh already does for that host; the agent is offered before any key read off disk. One SSH connection is shared per process and re-dials once if the transport dies under a long-lived pool (`mcp`, --all-databases). New dependencies: github.com/kevinburke/ssh_config, golang.org/x/crypto. --- cmd/pgbot/main.go | 18 +- go.mod | 4 +- go.sum | 8 +- internal/conn/connect.go | 9 + internal/conn/sshtunnel.go | 478 ++++++++++++++++++++++++++++++++ internal/conn/sshtunnel_test.go | 142 ++++++++++ 6 files changed, 655 insertions(+), 4 deletions(-) create mode 100644 internal/conn/sshtunnel.go create mode 100644 internal/conn/sshtunnel_test.go diff --git a/cmd/pgbot/main.go b/cmd/pgbot/main.go index b9eadd8..4f04fd0 100644 --- a/cmd/pgbot/main.go +++ b/cmd/pgbot/main.go @@ -12,6 +12,7 @@ import ( "os/signal" "syscall" + "github.com/pgrundev/pgbot/internal/conn" "github.com/spf13/cobra" ) @@ -24,6 +25,9 @@ func main() { // and the store finishes its write. cmd.Context() in every handler is this ctx. ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() + // One SSH connection serves every Target this process opens; drop it on the + // way out rather than per-Target (--all-databases and `mcp` open many). + defer conn.CloseSSHTunnel() root := &cobra.Command{ Use: "pgbot", @@ -51,12 +55,24 @@ func main() { root.AddCommand(newInitCmd()) root.AddCommand(newWhyCmd()) + // --ssh-tunnel is global: every command that takes a connection can need it, + // and it changes only HOW the DSN is reached, never what is inspected. + var sshTunnel string + root.PersistentFlags().StringVar(&sshTunnel, "ssh-tunnel", "", + "reach the database through this SSH jump host ([user@]host[:port]; a bare alias is resolved via ~/.ssh/config)") + // enteredRun distinguishes a malformed invocation (bad flags/args/unknown // command — cobra fails before PersistentPreRun) from an execution failure // (a handler ran and returned an error). B5's --fail-on makes exit codes a // public interface, so the two must not share code 3. enteredRun := false - root.PersistentPreRun = func(*cobra.Command, []string) { enteredRun = true } + root.PersistentPreRun = func(*cobra.Command, []string) { + enteredRun = true + if sshTunnel == "" { + sshTunnel = os.Getenv(conn.SSHTunnelEnv) + } + conn.SetSSHTunnel(sshTunnel) + } if err := root.ExecuteContext(ctx); err != nil { fmt.Fprintln(os.Stderr, "pgbot: "+err.Error()) diff --git a/go.mod b/go.mod index 8f05762..8b6ec23 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,10 @@ require ( github.com/charmbracelet/lipgloss v1.1.0 github.com/invopop/jsonschema v0.14.0 github.com/jackc/pgx/v5 v5.10.0 + github.com/kevinburke/ssh_config v1.4.0 github.com/owenrumney/go-sarif/v2 v2.3.3 github.com/spf13/cobra v1.10.2 + golang.org/x/crypto v0.54.0 golang.org/x/sync v0.22.0 golang.org/x/term v0.45.0 modernc.org/sqlite v1.56.0 @@ -40,7 +42,7 @@ require ( github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect go.yaml.in/yaml/v4 v4.0.0-rc.2 // indirect golang.org/x/sys v0.47.0 // indirect - golang.org/x/text v0.39.0 // indirect + golang.org/x/text v0.40.0 // indirect modernc.org/libc v1.74.4 // indirect modernc.org/mathutil v1.7.1 // indirect modernc.org/memory v1.11.0 // indirect diff --git a/go.sum b/go.sum index cc9190a..ec1b3d4 100644 --- a/go.sum +++ b/go.sum @@ -44,6 +44,8 @@ github.com/jackc/pgx/v5 v5.10.0 h1:VhSvgU2jSli8o3AqIEOTJr7rZwAEUVo4E4XhR94Zfr0= github.com/jackc/pgx/v5 v5.10.0/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= +github.com/kevinburke/ssh_config v1.4.0 h1:6xxtP5bZ2E4NF5tuQulISpTO2z8XbtH8cg1PWkxoFkQ= +github.com/kevinburke/ssh_config v1.4.0/go.mod h1:q2RIzfka+BXARoNexmF9gkxEX7DmvbW9P4hIVx2Kg4M= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= @@ -88,6 +90,8 @@ go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= go.yaml.in/yaml/v4 v4.0.0-rc.2 h1:/FrI8D64VSr4HtGIlUtlFMGsm7H7pWTbj6vOLVZcA6s= go.yaml.in/yaml/v4 v4.0.0-rc.2/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= +golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 h1:MDc5xs78ZrZr3HMQugiXOAkSZtfTpbJLDr/lwfgO53E= golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= @@ -104,8 +108,8 @@ golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= -golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= +golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= +golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= diff --git a/internal/conn/connect.go b/internal/conn/connect.go index 4c8dd26..c79b89e 100644 --- a/internal/conn/connect.go +++ b/internal/conn/connect.go @@ -56,6 +56,15 @@ func ConnectDB(ctx context.Context, connString, database string) (*Target, error cfg.MaxConnLifetime = 5 * time.Minute cfg.ConnConfig.RuntimeParams["application_name"] = "pgbot" + // Route the TCP leg through the SSH jump host when one is configured. This has + // to happen before probe(): the probe connection dials too, and it must take + // the same path as the pool. Installing it here rather than rewriting the DSN + // to a local forward is what keeps sslmode= and .pgpass matching on the real + // hostname — see sshtunnel.go. + if dial := sshDialFunc(); dial != nil { + cfg.ConnConfig.DialFunc = dial + } + // Drop client-only params pgx forwarded into RuntimeParams (it would send them // as server GUCs, which the server rejects). See clientOnlyParams. for _, p := range clientOnlyParams { diff --git a/internal/conn/sshtunnel.go b/internal/conn/sshtunnel.go new file mode 100644 index 0000000..6a94dc4 --- /dev/null +++ b/internal/conn/sshtunnel.go @@ -0,0 +1,478 @@ +package conn + +// SSH tunnelling. pgbot's connection path is libpq-only: it reaches whatever the +// DSN's host resolves to, which leaves out every database that only answers from +// inside a bastion, a VPN-routed jump host, or a private VPC subnet. +// +// The tunnel is installed as pgx's DialFunc rather than as a local port forward. +// That distinction matters: pgconn documents DialFunc as running BEFORE TLS is +// established, so the DSN keeps naming the REAL host all the way through. +// sslmode=verify-full still validates against that hostname, and .pgpass still +// matches on it. A `ssh -L` forward would force the DSN to say 127.0.0.1 and +// silently break both, besides leaving a port open to every local user. +// +// Host identity is NOT pgbot's policy to invent — it reads StrictHostKeyChecking +// and UserKnownHostsFile out of ssh_config and behaves the way the user's own ssh +// already does for that host. + +import ( + "context" + "errors" + "fmt" + "net" + "os" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/kevinburke/ssh_config" + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/agent" + "golang.org/x/crypto/ssh/knownhosts" + "golang.org/x/term" +) + +// SSHTunnelEnv is the environment variable that configures the jump host when +// --ssh-tunnel isn't passed. +const SSHTunnelEnv = "PGBOT_SSH_TUNNEL" + +// tunnel state. The client is a process-wide singleton: --all-databases opens one +// Target per database and `pgbot mcp` opens one per request, and every one of them +// should ride the same SSH connection rather than re-authenticating. +var ( + tunnelMu sync.Mutex + tunnelSpec string + tunnelConn *ssh.Client +) + +// SetSSHTunnel configures the jump host every subsequent Connect dials through. +// Spec is `[user@]host[:port]`, where host is looked up in ssh_config exactly as +// the ssh client would resolve it — so a bare alias picks up its HostName, User, +// Port and IdentityFile. An explicit user or port in the spec wins over the file. +// Empty spec (the default) means connect directly, and costs nothing. +func SetSSHTunnel(spec string) { + tunnelMu.Lock() + defer tunnelMu.Unlock() + tunnelSpec = strings.TrimSpace(spec) +} + +// SSHTunnelActive reports whether a jump host is configured, for callers that +// want to say so in their header line. +func SSHTunnelActive() bool { + tunnelMu.Lock() + defer tunnelMu.Unlock() + return tunnelSpec != "" +} + +// CloseSSHTunnel tears down the shared SSH connection. Safe to call when none was +// ever opened. +func CloseSSHTunnel() { + tunnelMu.Lock() + defer tunnelMu.Unlock() + if tunnelConn != nil { + _ = tunnelConn.Close() + tunnelConn = nil + } +} + +// sshDialFunc returns a dialer that opens the database connection as a channel on +// the SSH connection, or nil when no tunnel is configured (pgx then keeps its own +// default dialer, timeouts included). +func sshDialFunc() func(context.Context, string, string) (net.Conn, error) { + if !SSHTunnelActive() { + return nil + } + return func(ctx context.Context, network, addr string) (net.Conn, error) { + c, err := tunnelClient(ctx) + if err != nil { + return nil, err + } + nc, err := c.DialContext(ctx, network, addr) + if err == nil { + return nc, nil + } + // A pooled connection can outlive the SSH transport (an idle timeout on the + // jump host, a laptop that slept, a VPN that flapped). Drop the dead client + // and re-dial once before surfacing the failure — the pool would otherwise + // stay broken for the rest of a long-lived `pgbot mcp` process. + CloseSSHTunnel() + c, rerr := tunnelClient(ctx) + if rerr != nil { + return nil, fmt.Errorf("%w (reconnect failed: %v)", err, rerr) + } + return c.DialContext(ctx, network, addr) + } +} + +// tunnelClient returns the shared SSH connection, dialing it on first use. +func tunnelClient(ctx context.Context) (*ssh.Client, error) { + tunnelMu.Lock() + defer tunnelMu.Unlock() + if tunnelConn != nil { + return tunnelConn, nil + } + if tunnelSpec == "" { + return nil, errors.New("no ssh tunnel configured") + } + c, err := dialSSH(ctx, tunnelSpec) + if err != nil { + return nil, fmt.Errorf("ssh tunnel %q: %w", tunnelSpec, err) + } + tunnelConn = c + return c, nil +} + +// sshHost is a jump host resolved from the spec plus ssh_config. +type sshHost struct { + alias string // what the user typed — the ssh_config lookup key + addr string // host:port actually dialed + user string // login user + keys []string // IdentityFile paths, in config order + idsOnly bool // IdentitiesOnly=yes — offer only the IdentityFile identities +} + +// dialSSH resolves the spec against ssh_config and opens the SSH connection. +func dialSSH(ctx context.Context, spec string) (*ssh.Client, error) { + h, err := resolveSSHHost(spec) + if err != nil { + return nil, err + } + auths, err := sshAuthMethods(h) + if err != nil { + return nil, err + } + if len(auths) == 0 { + return nil, fmt.Errorf("no usable credentials: no key in the agent and no readable IdentityFile for %q", h.alias) + } + hkcb, err := hostKeyCallback(h.alias) + if err != nil { + return nil, err + } + + // Dial the TCP leg through a context-aware dialer so a hung jump host respects + // the run's deadline instead of blocking until the TCP stack gives up. + var d net.Dialer + rawConn, err := d.DialContext(ctx, "tcp", h.addr) + if err != nil { + return nil, fmt.Errorf("dial %s: %w", h.addr, err) + } + cfg := &ssh.ClientConfig{ + User: h.user, + Auth: auths, + HostKeyCallback: hkcb, + } + if dl, ok := ctx.Deadline(); ok { + _ = rawConn.SetDeadline(dl) + } + sc, chans, reqs, err := ssh.NewClientConn(rawConn, h.addr, cfg) + if err != nil { + _ = rawConn.Close() + return nil, fmt.Errorf("handshake with %s: %w", h.addr, err) + } + // Clear the handshake deadline: it was for the handshake, and leaving it set + // would expire every database query that rides this connection later. + _ = rawConn.SetDeadline(time.Time{}) + return ssh.NewClient(sc, chans, reqs), nil +} + +// splitTunnelSpec breaks `[user@]host[:port]` apart. Bracketed IPv6 literals keep +// their colons; a bare IPv6 address has no unambiguous port syntax, so its colons +// are left alone too and the port comes from ssh_config. +func splitTunnelSpec(spec string) (user, host, port string, err error) { + rest := strings.TrimSpace(spec) + if i := strings.LastIndex(rest, "@"); i >= 0 { + user, rest = rest[:i], rest[i+1:] + } + switch { + case strings.HasPrefix(rest, "["): + // [::1]:2222 or [::1] + if end := strings.LastIndex(rest, "]"); end > 0 { + if tail := rest[end+1:]; strings.HasPrefix(tail, ":") { + port = tail[1:] + } + rest = rest[1:end] + } + case strings.Count(rest, ":") == 1: + i := strings.LastIndex(rest, ":") + rest, port = rest[:i], rest[i+1:] + } + if rest == "" { + return "", "", "", fmt.Errorf("malformed tunnel spec %q — want [user@]host[:port]", spec) + } + return user, rest, port, nil +} + +// resolveSSHHost merges the spec with ssh_config. The spec's user/port win, +// because an explicit flag should not be silently overridden by a config file. +func resolveSSHHost(spec string) (sshHost, error) { + user, alias, port, err := splitTunnelSpec(spec) + if err != nil { + return sshHost{}, err + } + h := sshHost{alias: alias, user: user} + + host := ssh_config.Get(h.alias, "HostName") + if host == "" { + host = h.alias + } + if port == "" { + if port = ssh_config.Get(h.alias, "Port"); port == "" { + port = "22" + } + } + h.addr = net.JoinHostPort(host, port) + + if h.user == "" { + if h.user = ssh_config.Get(h.alias, "User"); h.user == "" { + // ssh falls back to the local login name. + if u := os.Getenv("USER"); u != "" { + h.user = u + } else { + h.user = os.Getenv("LOGNAME") + } + } + } + for _, k := range ssh_config.GetAll(h.alias, "IdentityFile") { + if p := expandTilde(k); p != "" { + h.keys = append(h.keys, p) + } + } + // IdentitiesOnly=yes does NOT disable the agent — OpenSSH still uses agent-held + // keys, it just restricts the offer to the identities named here. Treating it + // as "no agent" breaks the common setup of an encrypted key that lives only in + // the agent. + h.idsOnly = isYes(ssh_config.Get(h.alias, "IdentitiesOnly")) + return h, nil +} + +// sshAuthMethods builds the auth chain: the agent first (it holds keys pgbot +// cannot read off disk, and never exposes the private material), then each +// readable IdentityFile. +func sshAuthMethods(h sshHost) ([]ssh.AuthMethod, error) { + var out []ssh.AuthMethod + if sock := agentSocket(h.alias); sock != "" { + if ac, err := net.Dial("unix", sock); err == nil { + client := agent.NewClient(ac) + signers := client.Signers + if h.idsOnly { + signers = onlyIdentities(client.Signers, h.keys) + } + out = append(out, ssh.PublicKeysCallback(signers)) + } + } + for _, path := range h.keys { + raw, err := os.ReadFile(path) + if err != nil { + continue // a listed-but-absent IdentityFile is normal; ssh skips it too + } + signer, err := ssh.ParsePrivateKey(raw) + if err != nil { + var pm *ssh.PassphraseMissingError + if !errors.As(err, &pm) { + warnOnce(path, fmt.Sprintf("pgbot: ignoring unusable key %s: %v", path, err)) + continue + } + signer, err = promptForKey(path, raw) + if err != nil { + warnOnce(path, fmt.Sprintf("pgbot: skipping %s: %v", path, err)) + continue + } + } + out = append(out, ssh.PublicKeys(signer)) + } + return out, nil +} + +// onlyIdentities implements IdentitiesOnly against the agent: keep just the +// agent-held keys whose public half matches one of the configured IdentityFiles. +// When no .pub is readable there is nothing to match on, so the full set is +// offered rather than authenticating with nothing. +func onlyIdentities(next func() ([]ssh.Signer, error), keys []string) func() ([]ssh.Signer, error) { + return func() ([]ssh.Signer, error) { + all, err := next() + if err != nil { + return nil, err + } + want := map[string]bool{} + for _, k := range keys { + pub, err := os.ReadFile(k + ".pub") + if err != nil { + continue + } + if pk, _, _, _, err := ssh.ParseAuthorizedKey(pub); err == nil { + want[string(pk.Marshal())] = true + } + } + if len(want) == 0 { + return all, nil + } + var keep []ssh.Signer + for _, s := range all { + if want[string(s.PublicKey().Marshal())] { + keep = append(keep, s) + } + } + if len(keep) == 0 { + return all, nil + } + return keep, nil + } +} + +// promptForKey unlocks a passphrase-protected key. Only on a TTY: in CI the right +// answer is to load the key into an agent, not to hang waiting on stdin. +func promptForKey(path string, raw []byte) (ssh.Signer, error) { + fd := int(os.Stdin.Fd()) + if !term.IsTerminal(fd) { + return nil, errors.New("key is passphrase-protected and there is no terminal to ask on — add it to your ssh-agent") + } + fmt.Fprintf(os.Stderr, "Enter passphrase for %s: ", path) + pw, err := term.ReadPassword(fd) + fmt.Fprintln(os.Stderr) + if err != nil { + return nil, err + } + return ssh.ParsePrivateKeyWithPassphrase(raw, pw) +} + +// agentSocket honours IdentityAgent before falling back to SSH_AUTH_SOCK. +// OpenSSH expands environment references in this value — `IdentityAgent +// $SSH_AUTH_SOCK` is the idiomatic way to spell "whatever agent this shell has" +// — and accepts the bare name SSH_AUTH_SOCK for the same thing. Taking the value +// literally yields a path that cannot be dialed, and the agent silently drops out +// of the auth chain. +func agentSocket(alias string) string { + return expandAgentSpec(ssh_config.Get(alias, "IdentityAgent")) +} + +// expandAgentSpec turns an IdentityAgent value into a socket path, falling back +// to SSH_AUTH_SOCK for the empty, "none", and unresolvable cases. +func expandAgentSpec(ia string) string { + ia = strings.Trim(strings.TrimSpace(ia), `"`) + switch { + case ia == "", strings.EqualFold(ia, "none"), ia == "SSH_AUTH_SOCK": + return os.Getenv("SSH_AUTH_SOCK") + } + if p := expandTilde(os.ExpandEnv(ia)); p != "" { + return p + } + return os.Getenv("SSH_AUTH_SOCK") +} + +// hostKeyCallback reproduces the user's own ssh policy for this host rather than +// imposing one. StrictHostKeyChecking=no/off accepts anything; accept-new (and +// ask, which pgbot cannot honour non-interactively) accepts a host it has never +// seen but still refuses one whose key CHANGED — the case that actually signals +// interception. A changed key is refused under every mode except an explicit no. +func hostKeyCallback(alias string) (ssh.HostKeyCallback, error) { + strict := strings.ToLower(ssh_config.Get(alias, "StrictHostKeyChecking")) + files := knownHostsFiles(alias) + + if len(files) == 0 { + if strict == "yes" { + return nil, errors.New("StrictHostKeyChecking=yes but no readable UserKnownHostsFile — cannot verify the jump host") + } + fmt.Fprintf(os.Stderr, "pgbot: ssh host key for %q is not being verified (no known_hosts in effect)\n", alias) + return ssh.InsecureIgnoreHostKey(), nil + } + + base, err := knownhosts.New(files...) + if err != nil { + return nil, fmt.Errorf("read known_hosts: %w", err) + } + return func(hostname string, remote net.Addr, key ssh.PublicKey) error { + err := base(hostname, remote, key) + if err == nil { + return nil + } + var ke *knownhosts.KeyError + if !errors.As(err, &ke) { + return err + } + if len(ke.Want) > 0 { + // The host is known and presented a DIFFERENT key. Never auto-accept. + if strict == "no" || strict == "off" { + fmt.Fprintf(os.Stderr, "pgbot: WARNING — host key for %q CHANGED; continuing because StrictHostKeyChecking=no\n", alias) + return nil + } + return fmt.Errorf("host key for %q does not match known_hosts — refusing to connect", alias) + } + // Unknown host. + switch strict { + case "yes": + return fmt.Errorf("host %q is not in known_hosts and StrictHostKeyChecking=yes", alias) + default: + fmt.Fprintf(os.Stderr, "pgbot: accepting unknown ssh host key for %q (%s)\n", alias, ssh.FingerprintSHA256(key)) + return nil + } + }, nil +} + +// knownHostsFiles resolves UserKnownHostsFile to the files that actually exist. +// /dev/null is a deliberate "don't verify" and is dropped along with the rest. +func knownHostsFiles(alias string) []string { + var specified []string + for _, v := range ssh_config.GetAll(alias, "UserKnownHostsFile") { + specified = append(specified, strings.Fields(v)...) + } + if len(specified) == 0 { + specified = []string{"~/.ssh/known_hosts", "~/.ssh/known_hosts2"} + } + return filterUsableKnownHosts(specified) +} + +// filterUsableKnownHosts keeps only paths that can actually verify a host key. +// /dev/null is the idiomatic "don't verify" spelling, and an absent or empty file +// verifies nothing — knownhosts.New errors on those, and keeping them would turn +// "unverified" into "every host rejected". +func filterUsableKnownHosts(paths []string) []string { + var out []string + for _, f := range paths { + p := expandTilde(strings.Trim(f, `"`)) + if p == "" || p == os.DevNull { + continue + } + if st, err := os.Stat(p); err != nil || st.IsDir() || st.Size() == 0 { + continue + } + out = append(out, p) + } + return out +} + +// expandTilde resolves a leading ~ or ~/ against the home directory. +func expandTilde(p string) string { + p = strings.TrimSpace(p) + if p == "" { + return "" + } + if p == "~" || strings.HasPrefix(p, "~/") { + home, err := os.UserHomeDir() + if err != nil { + return p + } + return filepath.Join(home, strings.TrimPrefix(strings.TrimPrefix(p, "~"), "/")) + } + return p +} + +// warnOnce prints a per-key diagnostic a single time. pgx dials more than once +// (probe, then each pool connection, then any fallback host), and repeating the +// same "skipping key" line four times reads like four different problems. +var warned sync.Map + +func warnOnce(key, msg string) { + if _, dup := warned.LoadOrStore(key, true); !dup { + fmt.Fprintln(os.Stderr, msg) + } +} + +// isYes reports whether an ssh_config boolean is on. +func isYes(v string) bool { + switch strings.ToLower(strings.TrimSpace(v)) { + case "yes", "true", "on": + return true + } + return false +} diff --git a/internal/conn/sshtunnel_test.go b/internal/conn/sshtunnel_test.go new file mode 100644 index 0000000..b47321a --- /dev/null +++ b/internal/conn/sshtunnel_test.go @@ -0,0 +1,142 @@ +package conn + +import ( + "os" + "path/filepath" + "testing" +) + +func TestSplitTunnelSpec(t *testing.T) { + cases := []struct { + in string + user, host, port string + wantErr bool + }{ + {in: "lm1", host: "lm1"}, + {in: "daf@lm1", user: "daf", host: "lm1"}, + {in: "lm1:2222", host: "lm1", port: "2222"}, + {in: "daf@lm1:2222", user: "daf", host: "lm1", port: "2222"}, + {in: "bastion.example.com", host: "bastion.example.com"}, + {in: " lm1 ", host: "lm1"}, + // An IPv6 literal must keep its colons; only the bracketed form can carry + // a port, because a bare one is ambiguous. + {in: "[::1]:2222", host: "::1", port: "2222"}, + {in: "[fe80::1]", host: "fe80::1"}, + {in: "fe80::1", host: "fe80::1"}, + {in: "daf@[::1]:22", user: "daf", host: "::1", port: "22"}, + {in: "", wantErr: true}, + {in: "daf@", wantErr: true}, + } + for _, c := range cases { + user, host, port, err := splitTunnelSpec(c.in) + if c.wantErr { + if err == nil { + t.Errorf("splitTunnelSpec(%q): want error, got %q/%q/%q", c.in, user, host, port) + } + continue + } + if err != nil { + t.Errorf("splitTunnelSpec(%q): %v", c.in, err) + continue + } + if user != c.user || host != c.host || port != c.port { + t.Errorf("splitTunnelSpec(%q) = %q/%q/%q, want %q/%q/%q", + c.in, user, host, port, c.user, c.host, c.port) + } + } +} + +// A tunnel that was never configured must leave pgx's own dialer in place — +// otherwise every direct connection would start paying for this feature. +func TestSSHDialFunc_nilWhenUnconfigured(t *testing.T) { + SetSSHTunnel("") + defer SetSSHTunnel("") + if SSHTunnelActive() { + t.Fatal("SSHTunnelActive() true with an empty spec") + } + if sshDialFunc() != nil { + t.Fatal("sshDialFunc() returned a dialer with no tunnel configured") + } + SetSSHTunnel(" lm1 ") + if !SSHTunnelActive() { + t.Fatal("SSHTunnelActive() false after SetSSHTunnel") + } + if sshDialFunc() == nil { + t.Fatal("sshDialFunc() returned nil with a tunnel configured") + } +} + +func TestExpandTilde(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Skip("no home directory") + } + cases := map[string]string{ + "~/.ssh/id_ed25519": filepath.Join(home, ".ssh/id_ed25519"), + "~": home, + "/etc/ssh/key": "/etc/ssh/key", + " ~/x ": filepath.Join(home, "x"), + "": "", + } + for in, want := range cases { + if got := expandTilde(in); got != want { + t.Errorf("expandTilde(%q) = %q, want %q", in, got, want) + } + } +} + +func TestIsYes(t *testing.T) { + for _, v := range []string{"yes", "YES", "Yes", "true", "on", " yes "} { + if !isYes(v) { + t.Errorf("isYes(%q) = false", v) + } + } + for _, v := range []string{"no", "off", "", "ask", "accept-new"} { + if isYes(v) { + t.Errorf("isYes(%q) = true", v) + } + } +} + +// knownHostsFiles must drop anything that verifies nothing — /dev/null (the +// idiomatic "don't check" spelling), missing files, and empty ones. Passing an +// empty file to knownhosts.New is an error, and passing /dev/null would make the +// callback reject every host instead of falling through to the configured policy. +func TestKnownHostsFiles_dropsUnusable(t *testing.T) { + dir := t.TempDir() + empty := filepath.Join(dir, "empty") + if err := os.WriteFile(empty, nil, 0o600); err != nil { + t.Fatal(err) + } + real := filepath.Join(dir, "known_hosts") + if err := os.WriteFile(real, []byte("example.com ssh-ed25519 AAAA\n"), 0o600); err != nil { + t.Fatal(err) + } + missing := filepath.Join(dir, "nope") + + got := filterUsableKnownHosts([]string{os.DevNull, empty, missing, real}) + if len(got) != 1 || got[0] != real { + t.Errorf("filterUsableKnownHosts = %v, want [%s]", got, real) + } +} + +func TestAgentSocket_expandsEnvReference(t *testing.T) { + // OpenSSH expands environment references in IdentityAgent; `$SSH_AUTH_SOCK` + // is the common spelling and must not be taken as a literal path. + t.Setenv("SSH_AUTH_SOCK", "/tmp/agent.test") + if got := expandAgentSpec("$SSH_AUTH_SOCK"); got != "/tmp/agent.test" { + t.Errorf("expandAgentSpec($SSH_AUTH_SOCK) = %q", got) + } + if got := expandAgentSpec("SSH_AUTH_SOCK"); got != "/tmp/agent.test" { + t.Errorf("expandAgentSpec(SSH_AUTH_SOCK) = %q", got) + } + if got := expandAgentSpec(""); got != "/tmp/agent.test" { + t.Errorf("expandAgentSpec(empty) = %q", got) + } + if got := expandAgentSpec("none"); got != "/tmp/agent.test" { + t.Errorf("expandAgentSpec(none) = %q", got) + } + if got := expandAgentSpec(`"/run/user/1000/keyring/ssh"`); got != "/run/user/1000/keyring/ssh" { + t.Errorf("expandAgentSpec(quoted path) = %q", got) + } +} From d5818631563043838c6edbe4702793e5ffd75002 Mon Sep 17 00:00:00 2001 From: Diego Date: Mon, 31 Aug 2026 11:20:20 -0300 Subject: [PATCH 3/3] =?UTF-8?q?docs:=20--ssh-tunnel=20=E2=80=94=20reaching?= =?UTF-8?q?=20a=20private=20database=20through=20a=20jump=20host?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flag had no prose: the README's environment reference didn't list $PGBOT_SSH_TUNNEL, and the RDS/Aurora page still offered an EC2 in the VPC as the only way into a private instance, with "no SSH tunnel" as one of its selling points. Document the dialer-not-a-forward property where a reader looks for it — it's the reason sslmode=verify-full and .pgpass keep working against the real hostname — and say that the jump host's own ssh_config is what governs the connection. --- README.md | 26 ++++++++++++++++++++++++++ docs/providers.md | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1284423..fb86475 100644 --- a/README.md +++ b/README.md @@ -388,6 +388,29 @@ pgbot resolves the connection in this order: the argument first, then `$DATABASE_URL`, then `$PGBOT_DATABASE_URL`. Add `?sslmode=require` (or stricter) for any database reached over a network. +### Reaching a private database + +A database on a private network — RDS/Aurora inside a VPC, or a Postgres behind a +bastion — is reached through an SSH jump host: + +```sh +pgbot inspect "postgres://pgbot_ro@db.internal:5432/appdb?sslmode=verify-full" \ + --ssh-tunnel bastion.example.com # or user@host:port, or a ~/.ssh/config alias +``` + +`--ssh-tunnel` is global — every command that opens a connection takes it — and +`$PGBOT_SSH_TUNNEL` sets it for a whole session. + +The tunnel is a dialer, not an `ssh -L` forward, so **the DSN keeps naming the real +host**: `sslmode=verify-full` still validates against that hostname, `.pgpass` still +matches on it, and no local port is left open to everyone else on your machine. + +How the jump host is reached comes from your own `ssh_config` — `HostName`, `Port`, +`User`, `IdentityFile`, `IdentitiesOnly`, `IdentityAgent`, `StrictHostKeyChecking`, +`UserKnownHostsFile` — so a bare alias works and the host key is verified exactly +the way your `ssh` verifies it. Your agent is offered before any key on disk, and +one SSH connection serves the whole run. Raise `--timeout` if the link is slow. + ### Environment reference | Variable | Purpose | @@ -395,6 +418,7 @@ for any database reached over a network. | `DATABASE_URL` / `PGBOT_DATABASE_URL` | Connection used when no connection string is passed (checked in that order, after the argument). | | `NO_COLOR` | Disables ANSI output (as does a non-TTY, or `--no-color`). | | `XDG_STATE_HOME` | Where the baseline store lives; defaults to `~/.local/state`. | +| `PGBOT_SSH_TUNNEL` | SSH jump host used when `--ssh-tunnel` isn't passed (`[user@]host[:port]`, or a `~/.ssh/config` alias). | | `PGBOT_CONFIG` | Path to `.pgbot.toml` (otherwise discovered from cwd upward, then `$XDG_CONFIG_HOME`). | | `OPENAI_API_KEY` | Enables `ask` / `explain` via OpenAI. Keys are never accepted as flags. | | `GEMINI_API_KEY` / `GOOGLE_API_KEY` | Enables `ask` / `explain` via Google Gemini. | @@ -513,6 +537,8 @@ pgbot inspect # URL or libpq DSN, or set $DATABASE_URL --interval 1s gap between the two counter samples (min 500ms) --no-store don't read or write the local baseline --no-color disable ANSI (also honors NO_COLOR and non-TTY) + --ssh-tunnel reach the database through an SSH jump host — global, so + every command that connects takes it (also $PGBOT_SSH_TUNNEL) pgbot baselines list # what's stored locally, per database pgbot baselines prune # delete a database's snapshots diff --git a/docs/providers.md b/docs/providers.md index 9025b61..c375098 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -33,7 +33,7 @@ detection works even when the host is a bare IP or sits behind a proxy. ## Amazon RDS / Aurora -- **Connecting:** you can't install on the RDS/Aurora instance (managed, no OS access) — run pgbot from a client that can reach it. For a **private** instance (typical prod), run pgbot from a small **EC2 in the same VPC**: it reaches the private endpoint over AWS's internal network, so the DB never needs public access, no SSH tunnel, no IP allow-listing — the only rule is the RDS security group allowing `5432` from the EC2's security group. For a **publicly accessible** instance, allow your IP in the security group and connect from your laptop. +- **Connecting:** you can't install on the RDS/Aurora instance (managed, no OS access) — run pgbot from a client that can reach it. For a **private** instance (typical prod) there are two ways in: run pgbot from a small **EC2 in the same VPC** — it reaches the private endpoint over AWS's internal network, so the DB never needs public access, no SSH tunnel, no IP allow-listing, and the only rule is the RDS security group allowing `5432` from the EC2's security group — or keep pgbot on your laptop and reach the endpoint through a bastion with `--ssh-tunnel` (see [Reaching a private database](../README.md#reaching-a-private-database)), which still validates `sslmode=verify-full` against the real endpoint name. For a **publicly accessible** instance, allow your IP in the security group and connect from your laptop. ```bash pgbot inspect "postgres://pgbot_ro@mydb.abc123.us-east-1.rds.amazonaws.com:5432/appdb?sslmode=require" ```