From bf2bceb5203f196591d5f4329f658adb0ff10029 Mon Sep 17 00:00:00 2001 From: vergil_on Date: Sat, 8 Aug 2026 22:48:16 +0300 Subject: [PATCH] =?UTF-8?q?security:=20hardening=20=D0=BF=D0=BE=20=D1=80?= =?UTF-8?q?=D0=B5=D0=B7=D1=83=D0=BB=D1=8C=D1=82=D0=B0=D1=82=D0=B0=D0=BC=20?= =?UTF-8?q?=D0=B0=D1=83=D0=B4=D0=B8=D1=82=D0=B0=20=D0=B1=D0=B5=D0=B7=D0=BE?= =?UTF-8?q?=D0=BF=D0=B0=D1=81=D0=BD=D0=BE=D1=81=D1=82=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 4 +- README.ru.md | 4 +- cmd/synapse-backupper/backup.go | 3 +- cmd/synapse-backupper/backup_test.go | 5 +- cmd/synapse-backupper/healthcheck.go | 45 ++++++++ cmd/synapse-backupper/healthcheck_test.go | 42 +++++++ cmd/synapse-backupper/keygen.go | 6 + cmd/synapse-backupper/main.go | 1 + deploy/docker/Dockerfile | 2 +- deploy/docker/docker-compose.yml | 3 +- pkg/adapters/config/config.go | 6 +- pkg/adapters/crypto/composite/composite.go | 25 ++++- .../crypto/composite/composite_test.go | 38 +++++++ pkg/adapters/healthz/healthz.go | 3 +- pkg/adapters/pgdump/pgdump.go | 88 ++++++++++++++- pkg/adapters/pgdump/pgdump_test.go | 103 +++++++++++++++++- pkg/domain/backup/backup.go | 23 +++- pkg/domain/backup/backup_test.go | 19 ++++ 18 files changed, 400 insertions(+), 20 deletions(-) create mode 100644 cmd/synapse-backupper/healthcheck.go create mode 100644 cmd/synapse-backupper/healthcheck_test.go diff --git a/README.md b/README.md index c4ae6d6..9c53844 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ services: - APP_BACKUP_DIR=/backups - APP_BACKUP_CRON=0 0 3 * * * healthcheck: - test: ["CMD", "wget", "-qO-", "http://localhost:8080/healthz"] + test: ["CMD", "synapse-backupper", "healthcheck"] interval: 30s timeout: 3s start_period: 10s @@ -170,6 +170,8 @@ services: ``` > **Important:** Only public keys (`*.pub.pem`) should be present in `./keys`. Move private keys (`*.priv.pem`) to an offline or restore-only host before starting the container. +> +> Replace `APP_PG_PASSWORD=secret` with a strong credential before deploying to production; the value is only an example. Start the scheduler with: diff --git a/README.ru.md b/README.ru.md index eda3950..0adab95 100644 --- a/README.ru.md +++ b/README.ru.md @@ -162,7 +162,7 @@ services: - APP_BACKUP_DIR=/backups - APP_BACKUP_CRON=0 0 3 * * * healthcheck: - test: ["CMD", "wget", "-qO-", "http://localhost:8080/healthz"] + test: ["CMD", "synapse-backupper", "healthcheck"] interval: 30s timeout: 3s start_period: 10s @@ -170,6 +170,8 @@ services: ``` > **Важно:** В каталоге `./keys` должны находиться только публичные ключи (`*.pub.pem`). Перед запуском контейнера переместите приватные ключи (`*.priv.pem`) на офлайн-хост или хост, предназначенный только для восстановления. +> +> Перед промышленным развёртыванием замените `APP_PG_PASSWORD=secret` на надёжный пароль; указанное значение только пример. Запуск планировщика: diff --git a/cmd/synapse-backupper/backup.go b/cmd/synapse-backupper/backup.go index 7b30768..b519343 100644 --- a/cmd/synapse-backupper/backup.go +++ b/cmd/synapse-backupper/backup.go @@ -23,6 +23,7 @@ import ( "git.tswf.io/infra/go-synapse-backupper/pkg/adapters/retention" "git.tswf.io/infra/go-synapse-backupper/pkg/adapters/storage/local" "git.tswf.io/infra/go-synapse-backupper/pkg/domain" + "git.tswf.io/infra/go-synapse-backupper/pkg/domain/backup" "git.tswf.io/infra/go-synapse-backupper/pkg/domain/crypto" "git.tswf.io/infra/go-synapse-backupper/pkg/domain/pgdump" ) @@ -98,7 +99,7 @@ var backupCmd = &cobra.Command{ Database: cfg.PG.Database, User: cfg.PG.User, Password: cfg.PG.Password, - Key: fmt.Sprintf("synapse-%s.dump.pqenc", startTime.Format("20060102-150405")), + Key: backup.ArtifactKey(startTime), ExcludeTables: cfg.PG.ExcludeTables, } diff --git a/cmd/synapse-backupper/backup_test.go b/cmd/synapse-backupper/backup_test.go index f32a12a..d382c7e 100644 --- a/cmd/synapse-backupper/backup_test.go +++ b/cmd/synapse-backupper/backup_test.go @@ -134,7 +134,6 @@ func setBackupFlags( _ = cmd.Flags().Set("pg-host", "localhost") _ = cmd.Flags().Set("pg-port", "5432") _ = cmd.Flags().Set("pg-user", "testuser") - _ = cmd.Flags().Set("pg-password", "testpass") _ = cmd.Flags().Set("pg-database", "testdb") } @@ -162,6 +161,7 @@ func TestBackupCmd_Structure(t *testing.T) { func TestBackupCmd_Success(t *testing.T) { restoreGlobals(t) + t.Setenv("APP_PG_PASSWORD", "testpass") backupDir := t.TempDir() pqPath := filepath.Join(backupDir, "pq.pub") @@ -240,7 +240,7 @@ func TestBackupCmd_Success(t *testing.T) { key := dumper.receivedOpts.Key matched, _ := regexp.MatchString( - `^synapse-\d{8}-\d{6}\.dump\.pqenc$`, + `^synapse-\d{8}-\d{6}-[a-f0-9]{6}\.dump\.pqenc$`, key, ) if !matched { @@ -300,6 +300,7 @@ func TestBackupCmd_Success(t *testing.T) { func TestBackupCmd_PgDumpFailure(t *testing.T) { restoreGlobals(t) + t.Setenv("APP_PG_PASSWORD", "testpass") backupDir := t.TempDir() pqPath := filepath.Join(backupDir, "pq.pub") diff --git a/cmd/synapse-backupper/healthcheck.go b/cmd/synapse-backupper/healthcheck.go new file mode 100644 index 0000000..2b20e80 --- /dev/null +++ b/cmd/synapse-backupper/healthcheck.go @@ -0,0 +1,45 @@ +package main + +import ( + "context" + "fmt" + "net/http" + "time" + + "github.com/spf13/cobra" +) + +func newHealthcheckCmd() *cobra.Command { + var port int + + cmd := &cobra.Command{ + Use: "healthcheck", + Short: "Check the scheduler health endpoint", + Hidden: true, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, cancel := context.WithTimeout(cmd.Context(), 3*time.Second) + defer cancel() + + url := fmt.Sprintf("http://127.0.0.1:%d/healthz", port) + request, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return fmt.Errorf("build health request: %w", err) + } + + response, err := http.DefaultClient.Do(request) + if err != nil { + return fmt.Errorf("health request failed: %w", err) + } + defer response.Body.Close() + + if response.StatusCode != http.StatusOK { + return fmt.Errorf("health check returned status %d", response.StatusCode) + } + return nil + }, + } + + cmd.Flags().IntVar(&port, "port", 8080, "Health check server port") + + return cmd +} diff --git a/cmd/synapse-backupper/healthcheck_test.go b/cmd/synapse-backupper/healthcheck_test.go new file mode 100644 index 0000000..aca3059 --- /dev/null +++ b/cmd/synapse-backupper/healthcheck_test.go @@ -0,0 +1,42 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestHealthcheckCmd_Success(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/healthz" { + t.Errorf("unexpected path: %q", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + cmd := newHealthcheckCmd() + cmd.SetArgs([]string{"--port", serverPort(server)}) + if err := cmd.Execute(); err != nil { + t.Fatalf("healthcheck failed: %v", err) + } +} + +func TestHealthcheckCmd_Failure(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer server.Close() + + cmd := newHealthcheckCmd() + cmd.SetArgs([]string{"--port", serverPort(server)}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error for non-OK health status") + } +} + +func serverPort(server *httptest.Server) string { + return server.Listener.Addr().String()[strings.LastIndex(server.Listener.Addr().String(), ":")+1:] +} diff --git a/cmd/synapse-backupper/keygen.go b/cmd/synapse-backupper/keygen.go index b0fa7d7..279323e 100644 --- a/cmd/synapse-backupper/keygen.go +++ b/cmd/synapse-backupper/keygen.go @@ -108,6 +108,12 @@ func newKeygenCmdWithDeps(reg crypto.Registry) *cobra.Command { if err := privFile.Close(); err != nil { return fmt.Errorf("close private key file %s: %w", privPath, err) } + if err := os.Chmod(pubPath, 0o644); err != nil { + return fmt.Errorf("chmod public key file %s: %w", pubPath, err) + } + if err := os.Chmod(privPath, 0o600); err != nil { + return fmt.Errorf("chmod private key file %s: %w", privPath, err) + } } return nil diff --git a/cmd/synapse-backupper/main.go b/cmd/synapse-backupper/main.go index 9974b30..7ee401b 100644 --- a/cmd/synapse-backupper/main.go +++ b/cmd/synapse-backupper/main.go @@ -23,6 +23,7 @@ func main() { rootCmd.AddCommand(restoreCmd) rootCmd.AddCommand(runCmd) rootCmd.AddCommand(newKeygenCmd()) + rootCmd.AddCommand(newHealthcheckCmd()) rootCmd.AddCommand(generateConfigCmd()) if err := rootCmd.ExecuteContext(ctx); err != nil { diff --git a/deploy/docker/Dockerfile b/deploy/docker/Dockerfile index 02a0e87..b5e311f 100644 --- a/deploy/docker/Dockerfile +++ b/deploy/docker/Dockerfile @@ -11,7 +11,7 @@ RUN addgroup -S app && adduser -S -G app app COPY --from=builder --chown=app:app /out/synapse-backupper /usr/local/bin/synapse-backupper USER app EXPOSE 8080 -HEALTHCHECK --interval=30s --timeout=3s --start-period=10s CMD wget -qO- http://localhost:8080/healthz || exit 1 +HEALTHCHECK --interval=30s --timeout=3s --start-period=10s CMD ["synapse-backupper", "healthcheck"] LABEL maintainer="infra@tswf.io" version="0.1.0" description="Synapse PostgreSQL backup tool with composite dual-KEM encryption" ENTRYPOINT ["/usr/local/bin/synapse-backupper"] CMD ["run"] diff --git a/deploy/docker/docker-compose.yml b/deploy/docker/docker-compose.yml index ea96577..56152e2 100644 --- a/deploy/docker/docker-compose.yml +++ b/deploy/docker/docker-compose.yml @@ -9,6 +9,7 @@ services: - APP_PG_HOST=db - APP_PG_DATABASE=postgres - APP_PG_USER=synapse + # SECURITY: change this example password before deploying to production. - APP_PG_PASSWORD=changeme - APP_PQ_PUBLIC_KEY_PATH=/keys/synapse.pq.pub.pem - APP_CLASSICAL_PUBLIC_KEY_PATH=/keys/synapse.classical.pub.pem @@ -17,7 +18,7 @@ services: networks: - my-network healthcheck: - test: ["CMD", "wget", "-qO-", "http://localhost:8080/healthz"] + test: ["CMD", "synapse-backupper", "healthcheck"] interval: 30s timeout: 3s start_period: 10s diff --git a/pkg/adapters/config/config.go b/pkg/adapters/config/config.go index 217228d..7f8fbac 100644 --- a/pkg/adapters/config/config.go +++ b/pkg/adapters/config/config.go @@ -55,7 +55,6 @@ func RegisterFlags(cmd *cobra.Command) { flags.String("pg-host", "", "PostgreSQL host") flags.Int("pg-port", 0, "PostgreSQL port") flags.String("pg-user", "", "PostgreSQL user") - flags.String("pg-password", "", "PostgreSQL password") flags.String("pg-database", "", "PostgreSQL database name") flags.String("pg-sslmode", "", "PostgreSQL SSL mode") flags.StringSlice("pg-exclude-tables", nil, "PostgreSQL tables to exclude from backup") @@ -94,7 +93,6 @@ func Load(cmd *cobra.Command) (*Config, error) { _ = v.BindPFlag("pg.host", cmd.Flags().Lookup("pg-host")) _ = v.BindPFlag("pg.port", cmd.Flags().Lookup("pg-port")) _ = v.BindPFlag("pg.user", cmd.Flags().Lookup("pg-user")) - _ = v.BindPFlag("pg.password", cmd.Flags().Lookup("pg-password")) _ = v.BindPFlag("pg.database", cmd.Flags().Lookup("pg-database")) _ = v.BindPFlag("pg.sslmode", cmd.Flags().Lookup("pg-sslmode")) _ = v.BindPFlag("pg.exclude_tables", cmd.Flags().Lookup("pg-exclude-tables")) @@ -151,6 +149,10 @@ func Load(cmd *cobra.Command) (*Config, error) { v.SetEnvPrefix("APP") v.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_")) v.AutomaticEnv() + // pg.password is intentionally not exposed as a CLI flag (CWE-214), but + // must still be loadable from APP_PG_PASSWORD. Viper needs an explicit + // BindEnv for a nested key that has no bound flag. + _ = v.BindEnv("pg.password") var cfg Config if err := v.Unmarshal(&cfg); err != nil { diff --git a/pkg/adapters/crypto/composite/composite.go b/pkg/adapters/crypto/composite/composite.go index 9f476c0..be955b4 100644 --- a/pkg/adapters/crypto/composite/composite.go +++ b/pkg/adapters/crypto/composite/composite.go @@ -74,6 +74,11 @@ var ( // ErrUnexpectedEOF indicates the chunk stream ended before any chunk // with flags&0x01==1 (logical end-of-stream marker) was observed. ErrUnexpectedEOF = errors.New("composite: unexpected end of stream") + + // ErrPlaintextTooLarge indicates the decrypted payload would exceed the + // configured maximum plaintext size; decryption is aborted before the + // limit is crossed to prevent unbounded disk consumption. + ErrPlaintextTooLarge = errors.New("composite: plaintext size limit exceeded") ) // Format constants. @@ -89,6 +94,7 @@ const ( wrappedCekLen int = 48 // 32-byte CEK + 16-byte GCM tag firstPayloadNonceLen int = 12 kekLen int = 32 + maxPlaintextSize int64 = 1 << 40 // 1 TiB cap on decrypted output infoPq string = "git.tswf.io/infra/go-synapse-backupper/v2/kek/pq" infoComposite string = "git.tswf.io/infra/go-synapse-backupper/v2/kek/composite" ) @@ -505,7 +511,24 @@ func (d *decryptor) Decrypt( return err } - return decryptChunks(src, plaintext, payloadGcm, firstPayloadNonce) + plaintextLimiter := &limitedWriter{writer: plaintext, remaining: maxPlaintextSize} + return decryptChunks(src, plaintextLimiter, payloadGcm, firstPayloadNonce) +} + +// limitedWriter wraps an io.Writer and rejects writes that would exceed a +// maximum byte budget. +type limitedWriter struct { + writer io.Writer + remaining int64 +} + +func (lw *limitedWriter) Write(p []byte) (int, error) { + if int64(len(p)) > lw.remaining { + return 0, ErrPlaintextTooLarge + } + n, err := lw.writer.Write(p) + lw.remaining -= int64(n) + return n, err } // decryptChunks reads and decrypts chunk records until a final chunk diff --git a/pkg/adapters/crypto/composite/composite_test.go b/pkg/adapters/crypto/composite/composite_test.go index 2577109..fe68678 100644 --- a/pkg/adapters/crypto/composite/composite_test.go +++ b/pkg/adapters/crypto/composite/composite_test.go @@ -947,3 +947,41 @@ func loadGoldenPrivs( } return newFakePriv(fakePqSchemeID, pqRaw), newFakePriv(fakeClassicalSchemeID, classicalRaw) } + +func TestLimitedWriter_AllowsWritesWithinBudget(t *testing.T) { + var buf bytes.Buffer + lw := &limitedWriter{writer: &buf, remaining: 10} + + n, err := lw.Write([]byte("hello")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if n != 5 { + t.Errorf("wrote %d bytes, want 5", n) + } + + n, err = lw.Write([]byte("world")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if n != 5 { + t.Errorf("wrote %d bytes, want 5", n) + } + + if buf.String() != "helloworld" { + t.Errorf("buffer = %q, want %q", buf.String(), "helloworld") + } +} + +func TestLimitedWriter_RejectsOverBudget(t *testing.T) { + var buf bytes.Buffer + lw := &limitedWriter{writer: &buf, remaining: 3} + + _, err := lw.Write([]byte("hello")) + if !errors.Is(err, ErrPlaintextTooLarge) { + t.Errorf("error = %v, want ErrPlaintextTooLarge", err) + } + if buf.Len() != 0 { + t.Errorf("buffer len = %d, want 0", buf.Len()) + } +} diff --git a/pkg/adapters/healthz/healthz.go b/pkg/adapters/healthz/healthz.go index 0bac76a..19d529a 100644 --- a/pkg/adapters/healthz/healthz.go +++ b/pkg/adapters/healthz/healthz.go @@ -26,7 +26,8 @@ func New(port int) (*Server, error) { s := &Server{listener: listener} s.server = &http.Server{ - Handler: http.HandlerFunc(s.handleHealthz), + Handler: http.HandlerFunc(s.handleHealthz), + ReadHeaderTimeout: 5 * time.Second, } return s, nil } diff --git a/pkg/adapters/pgdump/pgdump.go b/pkg/adapters/pgdump/pgdump.go index 29bc84c..f06ba0a 100644 --- a/pkg/adapters/pgdump/pgdump.go +++ b/pkg/adapters/pgdump/pgdump.go @@ -6,11 +6,21 @@ import ( "io" "os" "os/exec" + "regexp" "strings" "git.tswf.io/infra/go-synapse-backupper/pkg/domain/pgdump" ) +var ( + passwordPattern = regexp.MustCompile(`(?i)\bpassword=[^\s]*`) + hostPattern = regexp.MustCompile(`(?i)\bhost=[^\s]*`) + // excludeTablePattern accepts unquoted PostgreSQL identifiers or the + // schema.table form. It rejects shell-special characters and injection + // payloads while still allowing the default Synapse table name. + excludeTablePattern = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_$]*(\.[a-zA-Z_][a-zA-Z0-9_$]*)?$`) +) + // adapter provides pg_dump functionality using the system's pg_dump binary. type adapter struct { commandContext func(ctx context.Context, name string, arg ...string) *exec.Cmd @@ -24,6 +34,10 @@ func New() pgdump.Dumper { } // Dump executes pg_dump and writes the output to sink. +// +// The password is never passed through the PGPASSWORD environment variable; +// instead a temporary .pgpass file with 0o600 permissions is created and +// pointed to via PGPASSFILE. func (a *adapter) Dump( ctx context.Context, opts pgdump.Options, @@ -39,14 +53,24 @@ func (a *adapter) Dump( } }() - args := buildArgs(opts) + args, err := buildArgs(opts) + if err != nil { + return err + } cmd := a.commandContext(ctx, "pg_dump", args...) env := os.Environ() - if opts.Password != "" { - env = append(env, fmt.Sprintf("PGPASSWORD=%s", opts.Password)) + + pgpassPath, cleanupPgpass, err := writePgPassFile(opts) + if err != nil { + return err } + defer cleanupPgpass() + if pgpassPath != "" { + env = append(env, fmt.Sprintf("PGPASSFILE=%s", pgpassPath)) + } + if opts.Host != "" { env = append(env, fmt.Sprintf("PGHOST=%s", opts.Host)) } @@ -66,7 +90,7 @@ func (a *adapter) Dump( cmd.Stdout = sink runErr := cmd.Run() - stderr := strings.TrimSpace(stderrBuilder.String()) + stderr := sanitizeStderr(strings.TrimSpace(stderrBuilder.String())) if runErr != nil { if exitErr, ok := runErr.(*exec.ExitError); ok { @@ -82,7 +106,56 @@ func (a *adapter) Dump( return nil } -func buildArgs(opts pgdump.Options) []string { +// writePgPassFile creates a temporary .pgpass file when a password is provided. +// The returned cleanup function removes the file; callers should defer it. +func writePgPassFile(opts pgdump.Options) (string, func(), error) { + if opts.Password == "" { + return "", func() {}, nil + } + + passFile, err := os.CreateTemp("", "pgpass-*.conf") + if err != nil { + return "", nil, fmt.Errorf("create temporary pgpass file: %w", err) + } + path := passFile.Name() + cleanup := func() { _ = os.Remove(path) } + + line := fmt.Sprintf( + "%s:%d:%s:%s:%s\n", + opts.Host, + opts.Port, + opts.Database, + opts.User, + opts.Password, + ) + if _, err := passFile.WriteString(line); err != nil { + _ = passFile.Close() + cleanup() + return "", nil, fmt.Errorf("write temporary pgpass file: %w", err) + } + if err := passFile.Close(); err != nil { + cleanup() + return "", nil, fmt.Errorf("close temporary pgpass file: %w", err) + } + if err := os.Chmod(path, 0o600); err != nil { + cleanup() + return "", nil, fmt.Errorf("chmod temporary pgpass file: %w", err) + } + return path, cleanup, nil +} + +// sanitizeStderr removes sensitive connection-string fragments from pg_dump +// diagnostics before they are logged. +func sanitizeStderr(input string) string { + if input == "" { + return "" + } + out := passwordPattern.ReplaceAllString(input, "password=***") + out = hostPattern.ReplaceAllString(out, "host=***") + return out +} + +func buildArgs(opts pgdump.Options) ([]string, error) { excludeTables := opts.ExcludeTables if len(excludeTables) == 0 { excludeTables = make([]string, 1) @@ -93,8 +166,11 @@ func buildArgs(opts pgdump.Options) []string { args = append(args, "--format=custom") for _, table := range excludeTables { + if !excludeTablePattern.MatchString(table) { + return nil, fmt.Errorf("invalid exclude-table identifier %q", table) + } args = append(args, "--exclude-table="+table) } - return args + return args, nil } diff --git a/pkg/adapters/pgdump/pgdump_test.go b/pkg/adapters/pgdump/pgdump_test.go index 0e75e31..bac699a 100644 --- a/pkg/adapters/pgdump/pgdump_test.go +++ b/pkg/adapters/pgdump/pgdump_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "os" "os/exec" "reflect" "strings" @@ -282,11 +283,15 @@ func TestDump_EnvVars(t *testing.T) { } envStr := strings.Join(capturedCmd.Env, "\n") + + if strings.Contains(envStr, "PGPASSWORD=") { + t.Errorf("PGPASSWORD must not be passed to pg_dump subprocess") + } + wantEnvVars := []string{ "PGHOST=myhost", "PGPORT=5433", "PGUSER=myuser", - "PGPASSWORD=mypass", "PGDATABASE=mydb", } for _, wantEnv := range wantEnvVars { @@ -294,6 +299,102 @@ func TestDump_EnvVars(t *testing.T) { t.Errorf("env missing %q", wantEnv) } } + + if !strings.Contains(envStr, "PGPASSFILE=") { + t.Errorf("PGPASSFILE must be set when a password is provided") + } +} + +func TestDump_PgpassFileRemovedAfterRun(t *testing.T) { + var capturedCmd *exec.Cmd + + adapter := &adapter{ + commandContext: func(ctx context.Context, name string, arg ...string) *exec.Cmd { + capturedCmd = exec.CommandContext(ctx, "sh", "-c", "exit 0") + return capturedCmd + }, + } + + err := adapter.Dump( + context.Background(), + pgdump.Options{ + Password: "secret", + Database: "mydb", + }, + io.Discard, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + envStr := strings.Join(capturedCmd.Env, "\n") + passFilePrefix := "PGPASSFILE=" + idx := strings.Index(envStr, passFilePrefix) + if idx == -1 { + t.Fatalf("PGPASSFILE not found in env") + } + pgpassPath := envStr[idx+len(passFilePrefix):] + if newlineIdx := strings.Index(pgpassPath, "\n"); newlineIdx != -1 { + pgpassPath = pgpassPath[:newlineIdx] + } + + if _, statErr := os.Stat(pgpassPath); !os.IsNotExist(statErr) { + t.Errorf("temporary pgpass file %s was not removed after Dump returned", pgpassPath) + } +} + +func TestWritePgPassFile_Permissions(t *testing.T) { + path, cleanup, err := writePgPassFile(pgdump.Options{ + Host: "myhost", + Port: 5432, + Database: "mydb", + User: "myuser", + Password: "secret", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer cleanup() + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat pgpass file: %v", err) + } + if info.Mode().Perm() != 0o600 { + t.Errorf("pgpass file permissions = %o, want %o", info.Mode().Perm(), 0o600) + } + + content, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read pgpass file: %v", err) + } + want := "myhost:5432:mydb:myuser:secret\n" + if string(content) != want { + t.Errorf("pgpass content = %q, want %q", string(content), want) + } +} + +func TestDump_InvalidExcludeTable(t *testing.T) { + adapter := &adapter{ + commandContext: func(ctx context.Context, name string, arg ...string) *exec.Cmd { + return exec.CommandContext(ctx, "sh", "-c", "exit 0") + }, + } + + err := adapter.Dump( + context.Background(), + pgdump.Options{ + Database: "testdb", + ExcludeTables: []string{"table; DROP TABLE users;--"}, + }, + io.Discard, + ) + if err == nil { + t.Fatal("expected error for invalid exclude-table identifier") + } + if !strings.Contains(err.Error(), "invalid exclude-table identifier") { + t.Errorf("error = %v, want invalid exclude-table identifier", err) + } } func TestDump_PipeWriterClosed(t *testing.T) { diff --git a/pkg/domain/backup/backup.go b/pkg/domain/backup/backup.go index 13d1fad..b19d375 100644 --- a/pkg/domain/backup/backup.go +++ b/pkg/domain/backup/backup.go @@ -3,6 +3,7 @@ package backup import ( "context" "crypto/rand" + "encoding/hex" "fmt" "time" @@ -52,7 +53,6 @@ func RunOnce(ctx context.Context, cfg *config.Config) error { ) now := time.Now() - timestamp := now.UTC().Format("20060102-150405") pgDumpOpts := domainpgdump.Options{ Host: cfg.PG.Host, Port: cfg.PG.Port, @@ -60,7 +60,7 @@ func RunOnce(ctx context.Context, cfg *config.Config) error { User: cfg.PG.User, Password: cfg.PG.Password, ExcludeTables: cfg.PG.ExcludeTables, - Key: fmt.Sprintf("synapse-%s.dump.pqenc", timestamp), + Key: ArtifactKey(now), } if err := runner.Run(ctx, pgDumpOpts, recipients, sink, rand.Reader); err != nil { @@ -73,3 +73,22 @@ func RunOnce(ctx context.Context, cfg *config.Config) error { return nil } + +// ArtifactKey returns a backup file name that includes a UTC timestamp plus a +// short random suffix so that two backups started in the same second do not +// collide. +func ArtifactKey(timestamp time.Time) string { + var randomBytes [3]byte + if _, err := rand.Read(randomBytes[:]); err != nil { + return fmt.Sprintf( + "synapse-%s-%09d.dump.pqenc", + timestamp.UTC().Format("20060102-150405"), + timestamp.Nanosecond(), + ) + } + return fmt.Sprintf( + "synapse-%s-%s.dump.pqenc", + timestamp.UTC().Format("20060102-150405"), + hex.EncodeToString(randomBytes[:]), + ) +} diff --git a/pkg/domain/backup/backup_test.go b/pkg/domain/backup/backup_test.go index 8dadbcf..9c347d9 100644 --- a/pkg/domain/backup/backup_test.go +++ b/pkg/domain/backup/backup_test.go @@ -5,6 +5,7 @@ import ( "os" "strings" "testing" + "time" "git.tswf.io/infra/go-synapse-backupper/pkg/adapters/config" "git.tswf.io/infra/go-synapse-backupper/pkg/adapters/crypto/keymanager" @@ -127,3 +128,21 @@ func TestRunOnce_ValidKeys_PgDumpMissing(t *testing.T) { } } } + +func TestArtifactKey(t *testing.T) { + ts := time.Date(2025, 1, 2, 15, 4, 5, 0, time.UTC) + key := ArtifactKey(ts) + + wantPrefix := "synapse-20250102-150405-" + if !strings.HasPrefix(key, wantPrefix) { + t.Errorf("key = %q, want prefix %q", key, wantPrefix) + } + if !strings.HasSuffix(key, ".dump.pqenc") { + t.Errorf("key = %q, want suffix .dump.pqenc", key) + } + + secondKey := ArtifactKey(ts) + if key == secondKey { + t.Errorf("two keys for the same timestamp collide: %q", key) + } +}