fix: resolve tilda paths and validate datadir config (#4485)

Running the command `bacalhau --repo=~/some/path <command>` results in a
config with a `DataDir` value of `~/some/path` rather than an expanded
path, e.g.` /home/$USER/some/path`. We must handle tilda paths
gracefully by expanding them to an absolute path before accepting them
as configuration.

Further, we must either dis-allow relative paths, or expand them to
absolute paths, before accepting them as configuration. In this PR we
have chosen the latter. This is because the `DataDir` field is used as
the root path for our [hard-coded config
paths](https://github.com/bacalhau-project/bacalhau/blob/main/pkg/config/types/paths.go#L1),
and said path must be absolute when provided to docker as a volume mount
point, else we get errors like:
#4506

This PR adds path expansion when `DataDir` is a tilda path, and
validation to enforces `DataDir` be an absolute path before accepting it
as configuration.

Below are some examples of valid, and invalid paths - enforced via unit
tests:
### Valid
```
DataDir:  "/absolute/path",
DataDir:  "~/absolute/path",
DataDir:  "~",
DataDir:  "~/",
DataDir:  "~/.",
DataDir:  "/path/with/special/char$",
DataDir:  "/path/with space",
DataDir:  "/absolute/path/",
DataDir:  "~/~/path",
DataDir:  "~/path/with space",
DataDir:  "/very/long/path/" + strings.Repeat("a", 255),
DataDir:  "/path/~to/file",
DataDir:  "~/path/~to/file",
DataDir:  "/path/with?invalid",
```
### Invalid
```
DataDir: "",
DataDir: "not/absolute/path",
DataDir: "./not/absolute/path",
DataDir: "~not/absolute/path",
```

- closes #4484

---------

Co-authored-by: frrist <[email protected]>
Co-authored-by: Walid Baruni <[email protected]>

cmd/cli/config/set.go

@@ -72,7 +72,7 @@ func newSetCmd() *cobra.Command {
 				value = args[1:]
 			}
 
-			return setConfig(cmd, configPath, key, value...)
+			return setConfig(cmd, rawConfig, configPath, key, value...)
 		},
 		// Provide auto completion for arguments to the `set` command
 		ValidArgsFunction: setAutoComplete,
@@ -82,7 +82,7 @@ func newSetCmd() *cobra.Command {
 	return setCmd
 }
 
-func setConfig(cmd *cobra.Command, cfgFilePath, key string, value ...string) error {
+func setConfig(cmd *cobra.Command, cfg *config.Config, cfgFilePath, key string, value ...string) error {
 	cmd.Printf("Writing config to %s", cfgFilePath)
 	v := viper.New()
 	v.SetConfigFile(cfgFilePath)
@@ -95,7 +95,15 @@ func setConfig(cmd *cobra.Command, cfgFilePath, key string, value ...string) err
 	if err != nil {
 		return err
 	}
-	v.Set(key, parsed)
+	if key == types.DataDirKey {
+		dataDirPath := config.AbsPathSilent(parsed.(string))
+		if err = config.ValidatePath(dataDirPath); err != nil {
+			return err
+		}
+		v.Set(key, dataDirPath)
+	} else {
+		v.Set(key, parsed)
+	}
 	if err := v.WriteConfigAs(cfgFilePath); err != nil {
 		return err
 	}

pkg/config/config.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/mitchellh/go-homedir"
 	"github.com/mitchellh/mapstructure"
 	"github.com/rs/zerolog/log"
 	"github.com/samber/lo"
@@ -15,13 +16,16 @@ import (
 
 	"github.com/bacalhau-project/bacalhau/pkg/bacerrors"
 	"github.com/bacalhau-project/bacalhau/pkg/config/types"
+	"github.com/bacalhau-project/bacalhau/pkg/models"
 	"github.com/bacalhau-project/bacalhau/pkg/util/idgen"
 )
 
 const (
 	environmentVariablePrefix = "BACALHAU"
 	inferConfigTypes          = true
 	DefaultFileName           = "config.yaml"
+
+	errComponent = "config"
 )
 
 var (
@@ -116,23 +120,13 @@ func New(opts ...Option) (*Config, error) {
 
 	// To absolute paths for better logging. This is best effort and will not return an error if it fails.
 	for i, path := range c.paths {
-		if !filepath.IsAbs(path) {
-			absPath, err := filepath.Abs(path)
-			if err != nil {
-				log.Debug().Msgf("failed to resolve absolute path for %s: %v", path, err)
-			} else {
-				c.paths[i] = absPath
-			}
-		}
+		c.paths[i] = AbsPathSilent(path)
 	}
 
 	// merge the config files in the order they were passed.
 	for _, path := range c.paths {
-		if err := c.Merge(path); err != nil {
-			if os.IsNotExist(err) {
-				return nil, fmt.Errorf("the specified configuration file %q doesn't exist", path)
-			}
-			return nil, fmt.Errorf("opening config file %q: %w", path, err)
+		if err := c.merge(path); err != nil {
+			return nil, err
 		}
 	}
 
@@ -181,6 +175,10 @@ func New(opts ...Option) (*Config, error) {
 							c.values[types.ComputeEnabledKey] = true
 						}
 					}
+				case types.DataDirKey:
+					// Handle relative paths for data-dir flag
+					path := AbsPathSilent(flag.Value.String())
+					c.base.Set(types.DataDirKey, path)
 				case "default.publisher.deprecated":
 					// allow the deprecated --default-publisher flag to bind to related fields in the config.
 					for _, key := range []string{
@@ -202,10 +200,26 @@ func New(opts ...Option) (*Config, error) {
 	}
 
 	// merge the passed values last as they take highest precedence
+	// allow the user to set datadir as relative paths and resolve them to absolute paths
 	for name, value := range c.values {
+		if name == types.DataDirKey {
+			if val, ok := value.(string); ok {
+				value = AbsPathSilent(val)
+			}
+		}
 		c.base.Set(name, value)
 	}
 
+	// allow the users to set datadir to a path like ~/.bacalhau or ~/something/idk/whatever
+	// and expand the path for them
+	if expandedPath, err := homedir.Expand(c.base.GetString(types.DataDirKey)); err == nil {
+		c.base.Set(types.DataDirKey, expandedPath)
+	}
+
+	if err := ValidatePath(c.base.GetString(types.DataDirKey)); err != nil {
+		return nil, err
+	}
+
 	// if no config file was provided, we look for a config.yaml under the resolved data directory,
 	// and if it exists, we create and return a new config with the resolved path.
 	// we attempt this last to ensure the data-dir is resolved correctly from all config sources.
@@ -255,12 +269,15 @@ func (c *Config) Load(path string) error {
 	return nil
 }
 
-// Merge merges a new configuration file specified by `path` with the existing config.
-// Merge returns an error if the file cannot be read
-func (c *Config) Merge(path string) error {
+// merge merges a new configuration file specified by `path` with the existing config.
+// merge returns an error if the file cannot be read
+func (c *Config) merge(path string) error {
 	c.base.SetConfigFile(path)
 	if err := c.base.MergeInConfig(); err != nil {
-		return err
+		if os.IsNotExist(err) {
+			return fmt.Errorf("the specified configuration file %q doesn't exist", path)
+		}
+		return fmt.Errorf("opening config file %q: %w", path, err)
 	}
 	return nil
 }
@@ -285,6 +302,9 @@ func (c *Config) Unmarshal(out interface{}) error {
 	if err := c.base.Unmarshal(&out, DecoderHook); err != nil {
 		return err
 	}
+	if v, ok := out.(models.Validatable); ok {
+		return v.Validate()
+	}
 	return nil
 }
 
@@ -317,6 +337,44 @@ func GenerateNodeID(ctx context.Context, nodeNameProviderType string) (string, e
 	return nodeName, nil
 }
 
+func AbsPathSilent(path string) string {
+	expandedPath, err := homedir.Expand(path)
+	if err != nil {
+		log.Debug().Msgf("failed to expand path %s: %v", path, err)
+		return path
+	}
+	absPath, err := filepath.Abs(expandedPath)
+	if err != nil {
+		log.Debug().Msgf("failed to resolve absolute path for %s: %v", path, err)
+		return path
+	}
+	return absPath
+}
+
+func ValidatePath(path string) error {
+	if path == "" {
+		return bacerrors.New("data dir path is empty").
+			WithHint("Provide a valid path for the data directory").
+			WithComponent(errComponent).
+			WithCode(bacerrors.ValidationError)
+	}
+	if strings.Contains(path, "$") {
+		return bacerrors.New("data dir path %q contains a '$' character", path).
+			WithHint("Note that environment variables are not expanded will be used as-is").
+			WithComponent(errComponent).
+			WithCode(bacerrors.ValidationError)
+	}
+
+	if !filepath.IsAbs(path) {
+		return bacerrors.New("data dir path %q is not an absolute path", path).
+			WithHint("Use an absolute path for the data directory").
+			WithComponent(errComponent).
+			WithCode(bacerrors.ValidationError)
+	}
+
+	return nil
+}
+
 // checkFlagConfigConflicts checks for conflicts between cli flags and config values.
 // e.g. bacalhau serve --config=api.host=0.0.0.0 --api-host=0.0.0.0 should be rejected.
 func checkFlagConfigConflicts(flags map[string][]*pflag.Flag, cfgValues map[string]any) error {