diff --git a/pkg/config/cfgfile.go b/pkg/config/cfgfile.go index dfafc6bc..9a0bdbdf 100644 --- a/pkg/config/cfgfile.go +++ b/pkg/config/cfgfile.go @@ -44,19 +44,17 @@ func FixExtraConfigPath(configRoot string) string { } func FromFiles(pArgs *ProgArgs, configRoot, extraConfigPath string) error { - cfgRoot, err := validateConfigRootPath(configRoot) - if err != nil { - return err + if configRoot == "" { + return errors.New("configRoot is not allowed to be an empty string") } - extraCfgPath, err := validateConfigRootPath(extraConfigPath) - if err != nil { - return err + if extraConfigPath == "" { + return errors.New("extraConfigPath is not allowed to be an empty string") } - err = fromDaemonFiles(pArgs, cfgRoot) + err := fromDaemonFiles(pArgs, configRoot) if err != nil && !errors.Is(err, os.ErrNotExist) { return err } - return fromExtraFile(pArgs, extraCfgPath) + return fromExtraFile(pArgs, extraConfigPath) } func fromExtraFile(pArgs *ProgArgs, extraConfigPath string) error { @@ -108,11 +106,7 @@ func fromDaemonFiles(pArgs *ProgArgs, configPathRoot string) error { klog.Infof("configlet %q not regular file: ignored", configlet.Name()) continue } - configletPath, err := validateConfigPath(filepath.Join(configletDir, configlet.Name())) - if err != nil { - klog.Infof("could not load %q: %v", configlet.Name(), err) - continue - } + configletPath := filepath.Join(configletDir, configlet.Name()) if pArgs.Global.Debug { klog.Infof("loading configlet: %q", configletPath) } @@ -130,11 +124,11 @@ func fromDaemonFiles(pArgs *ProgArgs, configPathRoot string) error { } func loadConfiglet(confObj map[string]interface{}, configPath string) error { - obj := make(map[string]interface{}) - data, err := os.ReadFile(configPath) + data, err := ReadConfiglet(configPath) if err != nil { return err } + obj := make(map[string]interface{}) err = yaml.Unmarshal(data, &obj) if err != nil { return err @@ -191,7 +185,7 @@ type config struct { func readExtraConfig(configPath string) (config, error) { conf := config{} - data, err := os.ReadFile(configPath) + data, err := ReadConfiglet(configPath) if err != nil { // config is optional if errors.Is(err, os.ErrNotExist) { diff --git a/pkg/config/flags.go b/pkg/config/flags.go index 3c311e6f..8acdfed5 100644 --- a/pkg/config/flags.go +++ b/pkg/config/flags.go @@ -108,20 +108,12 @@ Special targets: } params := flags.Args() - extraConfigPath, err := validateConfigRootPath(configPath) - if err != nil { - return DefaultConfigRoot, LegacyExtraConfigPath, err - } - if len(params) > 1 { - return DefaultConfigRoot, extraConfigPath, fmt.Errorf("too many config roots given (%d), currently supported up to 1", len(params)) + return DefaultConfigRoot, configPath, fmt.Errorf("too many config roots given (%d), currently supported up to 1", len(params)) } if len(params) == 0 { - return DefaultConfigRoot, extraConfigPath, nil - } - configRoot, err := validateConfigRootPath(params[0]) - if err != nil { - return DefaultConfigRoot, LegacyExtraConfigPath, err + return DefaultConfigRoot, configPath, nil } + configRoot := params[0] return configRoot, FixExtraConfigPath(configRoot), nil } diff --git a/pkg/config/validation.go b/pkg/config/validation.go index 65340da6..9fc36986 100644 --- a/pkg/config/validation.go +++ b/pkg/config/validation.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "io/fs" + "os" "path/filepath" "strings" @@ -43,35 +44,30 @@ func Validate(pArgs *ProgArgs) error { return nil } -func validateConfigPath(configletPath string) (string, error) { - configRoot := filepath.Clean(configletPath) +func ReadConfiglet(configPath string) ([]byte, error) { + // to make gosec happy, the validation logic must be in the same function on which we call `os.ReadFile`. + // IOW, it seems the linter cannot track variable sanitization across functions. + configRoot := filepath.Clean(configPath) cfgRoot, err := filepath.EvalSymlinks(configRoot) if err != nil { if errors.Is(err, fs.ErrNotExist) { // reset to original value, it somehow passed the symlink check cfgRoot = configRoot } else { - return "", fmt.Errorf("failed to validate configRoot path: %w", err) + return nil, fmt.Errorf("failed to validate config path: %w", err) } } // else either success or checking a non-existing path. Which can be still OK. pattern, err := IsConfigRootAllowed(cfgRoot, UserRunDir, UserHomeDir) if err != nil { - return "", err + return nil, err } if pattern == "" { - return "", fmt.Errorf("failed to validate configRoot path %q: does not match any allowed pattern", cfgRoot) + return nil, fmt.Errorf("failed to validate configRoot path %q: does not match any allowed pattern", cfgRoot) } - return cfgRoot, nil -} - -func validateConfigRootPath(configRoot string) (string, error) { - if configRoot == "" { - return "", fmt.Errorf("configRoot is not allowed to be an empty string") - } - return validateConfigPath(configRoot) + return os.ReadFile(cfgRoot) } // IsConfigRootAllowed checks if an *already cleaned and canonicalized* path is among the allowed list.