diff --git a/.golangci.yml b/.golangci.yml index e79d810..0e163da 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -17,11 +17,13 @@ linters: - path: cmd/.*/main\.go$ linters: [gosec] text: "G706" - # G304 (file inclusion): the config loader reads the operator-supplied - # --config path by design. + # G304 (file inclusion) / G703 (path traversal via taint): the config + # loader opens and stats the operator-supplied config path by design — + # the --config flag, $COLDFRONT_CONFIG, or the packaged default. An + # operator who can set those already controls the process. - path: internal/config/config\.go$ linters: [gosec] - text: "G304" + text: "G304|G703" # G115 (integer overflow): intentional bit-packing truncation in the # snowflake / UUIDv7 id-to-partition-bound encoding. - path: internal/partition/idmap\.go$ diff --git a/cmd/archiver/main.go b/cmd/archiver/main.go index 3f4be45..2e438fa 100644 --- a/cmd/archiver/main.go +++ b/cmd/archiver/main.go @@ -48,7 +48,7 @@ func main() { return } - configPath := flag.String("config", "config.yaml", "path to config file") + configPath := flag.String("config", "", "path to config file (default: $COLDFRONT_CONFIG, ./config.yaml, then /etc/pgedge/coldfront/config.yaml)") debugExportDelay := flag.Duration("debug-export-delay", 0, "sleep this long after Phase 2 (capture+bulk-export) and before Phase 3 "+ "(replay+cutover). Test-only knob to widen the window so concurrent "+ @@ -61,7 +61,7 @@ func main() { return } - cfg, err := config.Load(*configPath) + cfg, err := config.LoadDefault(*configPath) if err != nil { log.Fatalf("load config: %v", err) } diff --git a/cmd/partitioner/main.go b/cmd/partitioner/main.go index 721382c..584a244 100644 --- a/cmd/partitioner/main.go +++ b/cmd/partitioner/main.go @@ -49,18 +49,14 @@ func main() { return } - cfgPath := flag.String("config", "", "path to the YAML config file") + cfgPath := flag.String("config", "", "path to the YAML config file (default: $COLDFRONT_CONFIG, ./config.yaml, then /etc/pgedge/coldfront/config.yaml)") showVersion := flag.Bool("version", false, "print the version and exit") flag.Parse() if *showVersion { fmt.Printf("%s %s (built %s)\n", filepath.Base(os.Args[0]), version.Version, version.BuildTime) return } - if *cfgPath == "" { - log.Fatal("--config is required") - } - - cfg, err := config.Load(*cfgPath) + cfg, err := config.LoadDefault(*cfgPath) if err != nil { log.Fatalf("load config: %v", err) } diff --git a/internal/config/config.go b/internal/config/config.go index 5d951a9..162acc9 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,6 +1,7 @@ package config import ( + "errors" "fmt" "os" "strings" @@ -111,6 +112,84 @@ type SubPartitionConfig struct { ValuesSource string `yaml:"values_source"` } +// ErrNoConfig reports that no config file was found. A caller that can proceed +// without one -- the CLI's --dsn path -- tests for it with errors.Is; any other +// error means a candidate existed but could not be used, which must not be +// mistaken for an absent config. +var ErrNoConfig = errors.New("no config file found") + +// defaultPackagedConfigPath is where pgedge-coldfront installs its config, and +// the last place Resolve looks -- so an RPM install needs no -config flag. +const defaultPackagedConfigPath = "/etc/pgedge/coldfront/config.yaml" + +// packagedConfigPath is a variable only so tests can redirect it; the real path +// is root-owned. +var packagedConfigPath = defaultPackagedConfigPath + +// Resolve reports which config file to read, given the -config flag's value +// ("" when the flag was not passed). +// +// A file the operator named -- via -config or COLDFRONT_CONFIG -- must exist. +// Falling through to a different file in that case would silently run against +// the wrong database or object store, so it is an error. Only the implicit +// chain falls through: +// +// -config → $COLDFRONT_CONFIG → ./config.yaml → /etc/pgedge/coldfront/config.yaml +func Resolve(flagPath string) (string, error) { + if flagPath != "" { + if err := readableFile(flagPath); err != nil { + return "", fmt.Errorf("config %q: %w", flagPath, err) + } + return flagPath, nil + } + + if env := os.Getenv("COLDFRONT_CONFIG"); env != "" { + if err := readableFile(env); err != nil { + return "", fmt.Errorf("config %q from COLDFRONT_CONFIG: %w", env, err) + } + return env, nil + } + + implicit := []string{"config.yaml", packagedConfigPath} + for _, candidate := range implicit { + if readableFile(candidate) == nil { + return candidate, nil + } + } + + return "", fmt.Errorf("%w: pass -config, set COLDFRONT_CONFIG, or create one of %s", + ErrNoConfig, strings.Join(implicit, " or ")) +} + +// readableFile reports whether path is a regular file that can be opened. +// Anything else is not a config: a directory cannot be parsed, a FIFO would +// block Load forever, and a device file would read without end. Opening it +// here also lets an unreadable candidate fall through to the next one rather +// than shadowing it. +func readableFile(path string) error { + info, err := os.Stat(path) + if err != nil { + return err + } + if !info.Mode().IsRegular() { + return fmt.Errorf("not a regular file") + } + f, err := os.Open(path) + if err != nil { + return err + } + return f.Close() +} + +// LoadDefault resolves the config file (see Resolve) and loads it. +func LoadDefault(flagPath string) (*Config, error) { + path, err := Resolve(flagPath) + if err != nil { + return nil, err + } + return Load(path) +} + // Load reads a YAML config file from path, applies defaults, and validates // the result. Returns the parsed Config or an error describing the first // problem encountered (read, parse, or validation). diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f556943..809ad0c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "syscall" "testing" "github.com/stretchr/testify/assert" @@ -565,3 +566,161 @@ archiver: require.Error(t, err) assert.Contains(t, err.Error(), "lakekeeper_endpoint") } + +// --- Resolve ----------------------------------------------------------------- + +func TestResolveExplicitPathWins(t *testing.T) { + dir := t.TempDir() + named := filepath.Join(dir, "named.yaml") + require.NoError(t, os.WriteFile(named, []byte(validConfig), 0o600)) + t.Setenv("COLDFRONT_CONFIG", filepath.Join(dir, "from-env.yaml")) + + got, err := Resolve(named) + require.NoError(t, err) + assert.Equal(t, named, got) +} + +func TestResolveExplicitPathMissingIsAnError(t *testing.T) { + // Falling through to another file when the operator named one would + // silently use different credentials. + _, err := Resolve(filepath.Join(t.TempDir(), "absent.yaml")) + require.Error(t, err) + assert.Contains(t, err.Error(), "absent.yaml") +} + +func TestResolveUsesEnvWhenNoFlag(t *testing.T) { + dir := t.TempDir() + env := filepath.Join(dir, "from-env.yaml") + require.NoError(t, os.WriteFile(env, []byte(validConfig), 0o600)) + t.Setenv("COLDFRONT_CONFIG", env) + + got, err := Resolve("") + require.NoError(t, err) + assert.Equal(t, env, got) +} + +func TestResolveEnvMissingIsAnError(t *testing.T) { + t.Setenv("COLDFRONT_CONFIG", filepath.Join(t.TempDir(), "absent.yaml")) + + _, err := Resolve("") + require.Error(t, err) + assert.Contains(t, err.Error(), "COLDFRONT_CONFIG") +} + +func TestResolveFallsBackToWorkingDirectory(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(validConfig), 0o600)) + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(dir) + + got, err := Resolve("") + require.NoError(t, err) + assert.Equal(t, "config.yaml", got) +} + +func TestResolveFallsBackToPackagedPath(t *testing.T) { + // The packaged location is the last resort, so an RPM install needs no + // -config flag. Redirected here because the real path is root-owned. + dir := t.TempDir() + packaged := filepath.Join(dir, "etc", "pgedge", "coldfront", "config.yaml") + require.NoError(t, os.MkdirAll(filepath.Dir(packaged), 0o755)) + require.NoError(t, os.WriteFile(packaged, []byte(validConfig), 0o600)) + + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(t.TempDir()) // no ./config.yaml here + packagedConfigPath = packaged + t.Cleanup(func() { packagedConfigPath = defaultPackagedConfigPath }) + + got, err := Resolve("") + require.NoError(t, err) + assert.Equal(t, packaged, got) +} + +func TestResolveNothingFoundListsWhatWasTried(t *testing.T) { + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(t.TempDir()) + packagedConfigPath = filepath.Join(t.TempDir(), "absent.yaml") + t.Cleanup(func() { packagedConfigPath = defaultPackagedConfigPath }) + + _, err := Resolve("") + require.Error(t, err) + assert.Contains(t, err.Error(), "config.yaml") + assert.Contains(t, err.Error(), "COLDFRONT_CONFIG") +} + +func TestResolveIgnoresADirectory(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "config.yaml"), 0o755)) + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(dir) + packagedConfigPath = filepath.Join(t.TempDir(), "absent.yaml") + t.Cleanup(func() { packagedConfigPath = defaultPackagedConfigPath }) + + _, err := Resolve("") + require.Error(t, err) +} + +func TestLoadDefaultResolvesThenLoads(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(validConfig), 0o600)) + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(dir) + + cfg, err := LoadDefault("") + require.NoError(t, err) + assert.Equal(t, "wh", cfg.Iceberg.Warehouse) +} + +func TestResolveRejectsANonRegularFile(t *testing.T) { + // A FIFO would make Load block forever; a device file would read without + // end. Only a regular file is a config. + dir := t.TempDir() + fifo := filepath.Join(dir, "config.yaml") + require.NoError(t, syscall.Mkfifo(fifo, 0o600)) + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(dir) + packagedConfigPath = filepath.Join(t.TempDir(), "absent.yaml") + t.Cleanup(func() { packagedConfigPath = defaultPackagedConfigPath }) + + _, err := Resolve("") + require.Error(t, err) + assert.ErrorIs(t, err, ErrNoConfig) +} + +func TestResolveFallsThroughAnUnreadableCandidate(t *testing.T) { + // An unreadable ./config.yaml must not shadow the packaged path. + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(validConfig), 0o000)) + packaged := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(packaged, []byte(validConfig), 0o600)) + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(dir) + packagedConfigPath = packaged + t.Cleanup(func() { packagedConfigPath = defaultPackagedConfigPath }) + + got, err := Resolve("") + require.NoError(t, err) + assert.Equal(t, packaged, got) +} + +func TestResolveNotFoundIsErrNoConfig(t *testing.T) { + // Callers distinguish "nothing configured" from "configured but broken". + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(t.TempDir()) + packagedConfigPath = filepath.Join(t.TempDir(), "absent.yaml") + t.Cleanup(func() { packagedConfigPath = defaultPackagedConfigPath }) + + _, err := Resolve("") + assert.ErrorIs(t, err, ErrNoConfig) +} + +func TestLoadDefaultParseErrorIsNotErrNoConfig(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("postgres: [oops"), 0o600)) + t.Setenv("COLDFRONT_CONFIG", "") + t.Chdir(dir) + + _, err := LoadDefault("") + require.Error(t, err) + assert.NotErrorIs(t, err, ErrNoConfig) +} diff --git a/internal/partcfg/commands.go b/internal/partcfg/commands.go index 2b210c9..0ebe7be 100644 --- a/internal/partcfg/commands.go +++ b/internal/partcfg/commands.go @@ -102,12 +102,20 @@ func addConn(fs *flag.FlagSet) func(context.Context) (*pgx.Conn, error) { cfgPath := fs.String("config", "", "path to the deployment YAML; its postgres.dsn is used if --dsn is unset") return func(ctx context.Context) (*pgx.Conn, error) { d := *dsn - if d == "" && *cfgPath != "" { - cfg, err := config.Load(*cfgPath) - if err != nil { - return nil, fmt.Errorf("read --config: %w", err) + if d == "" { + // With no --dsn, take the DSN from a config file. An explicitly + // named one must load; otherwise discovery is best-effort so the + // "pass --dsn or --config" message below still describes the + // problem when there is simply no configuration anywhere. + cfg, err := config.LoadDefault(*cfgPath) + switch { + case err == nil: + d = cfg.Postgres.DSN + case !errors.Is(err, config.ErrNoConfig): + // A config was found but is unusable. Reporting "pass --dsn or + // --config" here would hide a parse or validation failure. + return nil, fmt.Errorf("read config: %w", err) } - d = cfg.Postgres.DSN } if d == "" { return nil, fmt.Errorf("a connection is required: pass --dsn or --config") @@ -850,7 +858,7 @@ EXAMPLES: // runImport seeds partition_config from a deployment YAML's archiver.tables list. func runImport(ctx context.Context, args []string) error { fs := flag.NewFlagSet("import", flag.ContinueOnError) - cfgPath := fs.String("config", "", "deployment YAML to import: its archiver.tables become partition_config rows (required)") + cfgPath := fs.String("config", "", "deployment YAML to import: its archiver.tables become partition_config rows (default: $COLDFRONT_CONFIG, ./config.yaml, then /etc/pgedge/coldfront/config.yaml)") dsn := fs.String("dsn", "", "connection DSN (default: postgres.dsn from --config)") printSQL := fs.Bool("print-sql", false, "print the INSERTs instead of running them") dryRun := fs.Bool("dry-run", false, "validate/parse but make no changes") @@ -870,16 +878,13 @@ EXAMPLES: if err := fs.Parse(args); err != nil { return err } - if *cfgPath == "" { - fs.Usage() - return fmt.Errorf("--config is required (the YAML to import)") - } - cfg, err := config.Load(*cfgPath) + cfg, err := config.LoadDefault(*cfgPath) if err != nil { + fs.Usage() return fmt.Errorf("read --config: %w", err) } if len(cfg.Archiver.Tables) == 0 { - return fmt.Errorf("no archiver.tables in %s", *cfgPath) + return fmt.Errorf("no archiver.tables in the config") } if *printSQL { for _, t := range cfg.Archiver.Tables {