From 1ed710f3df9b7349f1c0defd2a401716697de58d Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Thu, 19 Sep 2024 14:18:48 +0000 Subject: [PATCH] fix(unix): umask behavior for file creation - remove bad calculation of "our own" umask - remove umask "union effect" - do not touch system umask unless `UMASK` is set - set system umask only if `UMASK` is set --- cmd/ddns-updater/main.go | 9 +++++++-- internal/config/paths.go | 16 ++++++++++++---- internal/config/settings_test.go | 2 +- internal/config/umask_unix.go | 15 --------------- internal/config/umask_windows.go | 9 --------- internal/params/json.go | 14 +++++++------- internal/persistence/json/database.go | 9 +++------ internal/system/umask_unix.go | 12 ++++++++++++ internal/system/umask_windows.go | 7 +++++++ 9 files changed, 49 insertions(+), 44 deletions(-) delete mode 100644 internal/config/umask_unix.go delete mode 100644 internal/config/umask_windows.go create mode 100644 internal/system/umask_unix.go create mode 100644 internal/system/umask_windows.go diff --git a/cmd/ddns-updater/main.go b/cmd/ddns-updater/main.go index 3b4ce161e..539e1e86c 100644 --- a/cmd/ddns-updater/main.go +++ b/cmd/ddns-updater/main.go @@ -26,6 +26,7 @@ import ( "github.com/qdm12/ddns-updater/internal/resolver" "github.com/qdm12/ddns-updater/internal/server" "github.com/qdm12/ddns-updater/internal/shoutrrr" + "github.com/qdm12/ddns-updater/internal/system" "github.com/qdm12/ddns-updater/internal/update" "github.com/qdm12/ddns-updater/pkg/publicip" "github.com/qdm12/goservices" @@ -129,6 +130,10 @@ func _main(ctx context.Context, reader *reader.Reader, args []string, logger log return err } + if *config.Paths.Umask > 0 { + system.SetUmask(*config.Paths.Umask) + } + shoutrrrSettings := shoutrrr.Settings{ Addresses: config.Shoutrrr.Addresses, DefaultTitle: config.Shoutrrr.DefaultTitle, @@ -139,14 +144,14 @@ func _main(ctx context.Context, reader *reader.Reader, args []string, logger log return fmt.Errorf("setting up Shoutrrr: %w", err) } - persistentDB, err := persistence.NewDatabase(*config.Paths.DataDir, config.Paths.Umask) + persistentDB, err := persistence.NewDatabase(*config.Paths.DataDir) if err != nil { shoutrrrClient.Notify(err.Error()) return err } jsonReader := jsonparams.NewReader(logger) - providers, warnings, err := jsonReader.JSONProviders(*config.Paths.Config, config.Paths.Umask) + providers, warnings, err := jsonReader.JSONProviders(*config.Paths.Config) for _, w := range warnings { logger.Warn(w) shoutrrrClient.Notify(w) diff --git a/internal/config/paths.go b/internal/config/paths.go index 0e9144d2b..35f74726e 100644 --- a/internal/config/paths.go +++ b/internal/config/paths.go @@ -14,14 +14,17 @@ import ( type Paths struct { DataDir *string Config *string - Umask fs.FileMode + // Umask is the custom umask to use for the system, if different than zero. + // If it is set to zero, the system umask is unchanged. + // It cannot be nil in the internal state. + Umask *fs.FileMode } func (p *Paths) setDefaults() { p.DataDir = gosettings.DefaultPointer(p.DataDir, "./data") defaultConfig := filepath.Join(*p.DataDir, "config.json") p.Config = gosettings.DefaultPointer(p.Config, defaultConfig) - p.Umask = gosettings.DefaultComparable(p.Umask, getCurrentUmask()) + p.Umask = gosettings.DefaultPointer(p.Umask, fs.FileMode(0)) } func (p Paths) Validate() (err error) { @@ -36,7 +39,11 @@ func (p Paths) toLinesNode() *gotree.Node { node := gotree.New("Paths") node.Appendf("Data directory: %s", *p.DataDir) node.Appendf("Config file: %s", *p.Config) - node.Appendf("Umask: %s", p.Umask.String()) + umaskString := "system default" + if *p.Umask != 0 { + umaskString = p.Umask.String() + } + node.Appendf("Umask: %s", umaskString) return node } @@ -46,10 +53,11 @@ func (p *Paths) read(reader *reader.Reader) (err error) { umaskString := reader.String("UMASK") if umaskString != "" { - p.Umask, err = parseUmask(umaskString) + umask, err := parseUmask(umaskString) if err != nil { return fmt.Errorf("parse umask: %w", err) } + p.Umask = &umask } return nil diff --git a/internal/config/settings_test.go b/internal/config/settings_test.go index 08605f503..26c4a3cd0 100644 --- a/internal/config/settings_test.go +++ b/internal/config/settings_test.go @@ -41,7 +41,7 @@ func Test_Settings_String(t *testing.T) { ├── Paths | ├── Data directory: ./data | ├── Config file: data/config.json -| └── Umask: -----w--w- +| └── Umask: system default ├── Backup: disabled └── Logger ├── Level: INFO diff --git a/internal/config/umask_unix.go b/internal/config/umask_unix.go deleted file mode 100644 index bf4229055..000000000 --- a/internal/config/umask_unix.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build !windows - -package config - -import ( - "io/fs" - "syscall" -) - -func getCurrentUmask() (mask fs.FileMode) { - const tempMask = 0o022 - oldMask := syscall.Umask(tempMask) - syscall.Umask(oldMask) - return fs.FileMode(oldMask) -} diff --git a/internal/config/umask_windows.go b/internal/config/umask_windows.go deleted file mode 100644 index e5a512d20..000000000 --- a/internal/config/umask_windows.go +++ /dev/null @@ -1,9 +0,0 @@ -package config - -import ( - "io/fs" -) - -func getCurrentUmask() (mask fs.FileMode) { - return 0 -} diff --git a/internal/params/json.go b/internal/params/json.go index 6c0e1946d..5f78a31fd 100644 --- a/internal/params/json.go +++ b/internal/params/json.go @@ -34,19 +34,19 @@ type commonSettings struct { // JSONProviders obtain the update settings from the JSON content, // first trying from the environment variable CONFIG and then from // the file config.json. -func (r *Reader) JSONProviders(filePath string, umask fs.FileMode) ( +func (r *Reader) JSONProviders(filePath string) ( providers []provider.Provider, warnings []string, err error) { - providers, warnings, err = r.getProvidersFromEnv(filePath, umask) + providers, warnings, err = r.getProvidersFromEnv(filePath) if providers != nil || warnings != nil || err != nil { return providers, warnings, err } - return r.getProvidersFromFile(filePath, umask) + return r.getProvidersFromFile(filePath) } var errWriteConfigToFile = errors.New("cannot write configuration to file") // getProvidersFromFile obtain the update settings from config.json. -func (r *Reader) getProvidersFromFile(filePath string, umask fs.FileMode) ( +func (r *Reader) getProvidersFromFile(filePath string) ( providers []provider.Provider, warnings []string, err error) { r.logger.Info("reading JSON config from file " + filePath) bytes, err := r.readFile(filePath) @@ -57,7 +57,7 @@ func (r *Reader) getProvidersFromFile(filePath string, umask fs.FileMode) ( r.logger.Info("file not found, creating an empty settings file") - filePerm := fs.FileMode(0o666) - umask //nolint:gomnd + const filePerm = fs.FileMode(0o666) //nolint:gomnd err = r.writeFile(filePath, []byte(`{}`), filePerm) if err != nil { err = fmt.Errorf("%w: %w", errWriteConfigToFile, err) @@ -71,7 +71,7 @@ func (r *Reader) getProvidersFromFile(filePath string, umask fs.FileMode) ( // getProvidersFromEnv obtain the update settings from the environment variable CONFIG. // If the settings are valid, they are written to the filePath. -func (r *Reader) getProvidersFromEnv(filePath string, umask fs.FileMode) ( +func (r *Reader) getProvidersFromEnv(filePath string) ( providers []provider.Provider, warnings []string, err error) { s := os.Getenv("CONFIG") if s == "" { @@ -92,7 +92,7 @@ func (r *Reader) getProvidersFromEnv(filePath string, umask fs.FileMode) ( if err != nil { return providers, warnings, fmt.Errorf("%w: %w", errWriteConfigToFile, err) } - filePerm := fs.FileMode(0o666) - umask //nolint:gomnd + const filePerm = fs.FileMode(0o666) err = r.writeFile(filePath, buffer.Bytes(), filePerm) if err != nil { return providers, warnings, fmt.Errorf("%w: %w", errWriteConfigToFile, err) diff --git a/internal/persistence/json/database.go b/internal/persistence/json/database.go index 2f7d3f18c..f479b1c70 100644 --- a/internal/persistence/json/database.go +++ b/internal/persistence/json/database.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "io/fs" "os" "path/filepath" "sync" @@ -17,7 +16,6 @@ type Database struct { data dataModel filepath string mutex sync.RWMutex - umask fs.FileMode } func (db *Database) Close() error { @@ -27,7 +25,7 @@ func (db *Database) Close() error { } // NewDatabase opens or creates the JSON file database. -func NewDatabase(dataDir string, umask fs.FileMode) (*Database, error) { +func NewDatabase(dataDir string) (*Database, error) { filePath := filepath.Join(dataDir, "updates.json") file, err := os.Open(filePath) @@ -35,7 +33,7 @@ func NewDatabase(dataDir string, umask fs.FileMode) (*Database, error) { if !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("reading file: %w", err) } - dirPerm := os.FileMode(0o777) - umask //nolint:gomnd + const dirPerm = os.FileMode(0o777) err = os.MkdirAll(filepath.Dir(filePath), dirPerm) if err != nil { return nil, fmt.Errorf("creating data directory: %w", err) @@ -81,7 +79,6 @@ func NewDatabase(dataDir string, umask fs.FileMode) (*Database, error) { return &Database{ data: data, filepath: filePath, - umask: umask, }, nil } @@ -136,7 +133,7 @@ func checkHistoryEvents(events []models.HistoryEvent) (err error) { } func (db *Database) write() error { - filePerm := os.FileMode(0o666) - db.umask //nolint:gomnd + const filePerm = os.FileMode(0o666) file, err := os.OpenFile(db.filepath, os.O_TRUNC|os.O_CREATE|os.O_WRONLY, filePerm) if err != nil { return fmt.Errorf("opening file: %w", err) diff --git a/internal/system/umask_unix.go b/internal/system/umask_unix.go new file mode 100644 index 000000000..316b19748 --- /dev/null +++ b/internal/system/umask_unix.go @@ -0,0 +1,12 @@ +//go:build !windows + +package system + +import ( + "io/fs" + "syscall" +) + +func SetUmask(umask fs.FileMode) { + _ = syscall.Umask(int(umask)) +} diff --git a/internal/system/umask_windows.go b/internal/system/umask_windows.go new file mode 100644 index 000000000..7d893c2e0 --- /dev/null +++ b/internal/system/umask_windows.go @@ -0,0 +1,7 @@ +package system + +import ( + "io/fs" +) + +func SetUmask(umask fs.FileMode) {}