Skip to content

Commit

Permalink
fix(unix): umask behavior for file creation
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
qdm12 committed Sep 19, 2024
1 parent 0674864 commit 1ed710f
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 44 deletions.
9 changes: 7 additions & 2 deletions cmd/ddns-updater/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down
16 changes: 12 additions & 4 deletions internal/config/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
}

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/config/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 0 additions & 15 deletions internal/config/umask_unix.go

This file was deleted.

9 changes: 0 additions & 9 deletions internal/config/umask_windows.go

This file was deleted.

14 changes: 7 additions & 7 deletions internal/params/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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 == "" {
Expand All @@ -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)
Expand Down
9 changes: 3 additions & 6 deletions internal/persistence/json/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/json"
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"sync"
Expand All @@ -17,7 +16,6 @@ type Database struct {
data dataModel
filepath string
mutex sync.RWMutex
umask fs.FileMode
}

func (db *Database) Close() error {
Expand All @@ -27,15 +25,15 @@ 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)
if err != nil {
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)
Expand Down Expand Up @@ -81,7 +79,6 @@ func NewDatabase(dataDir string, umask fs.FileMode) (*Database, error) {
return &Database{
data: data,
filepath: filePath,
umask: umask,
}, nil
}

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions internal/system/umask_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//go:build !windows

package system

import (
"io/fs"
"syscall"
)

func SetUmask(umask fs.FileMode) {
_ = syscall.Umask(int(umask))
}
7 changes: 7 additions & 0 deletions internal/system/umask_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package system

import (
"io/fs"
)

func SetUmask(umask fs.FileMode) {}

0 comments on commit 1ed710f

Please sign in to comment.