Compare commits

..
2 Commits
Author SHA1 Message Date
sneak bd0b186cf2 config: a metrics username without a password stops startup (closes #268)
check / check (push) Canceled after 0s
DNSWATCHER_METRICS_USERNAME turns on /metrics, and its Basic Auth used
DNSWATCHER_METRICS_PASSWORD as it was, empty included, so the username
with an empty password got in. config.New now refuses a username
without a password with ErrNoMetricsPassword, whose message names both
variables. The README's variable table says the password is required
with the username.

Judgement call: the check is in New, after buildConfig, since it
compares two settings buildConfig has already read; buildConfig is
also at the linter's length limit.

Model: opus-5-5
2026-10-13 17:00:35 +00:00
clawbot 5617c92fb3 state: Save syncs the state file before renaming it into place (closes #264)
check / check (push) Canceled after 0s
Save wrote state.json.tmp with os.WriteFile and renamed it over
state.json without syncing. After a power loss the rename could reach
the disk before the data, leaving an empty state.json that Load cannot
parse, so every start failed.

Save now writes the temp file through its own handle, syncs and closes
it, renames it, then syncs the data directory so the rename is on disk
too. The new test starts from a leftover temp file longer than the
state; a test cannot observe the syncs, which are confirmed by reading.

Rule suppressed: gosec's file-path warning on the temp file's open, as
Load already does.

Model: opus-5-5
2026-10-13 18:27:31 +02:00
3 changed files with 71 additions and 2 deletions
+2
View File
@@ -21,6 +21,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
- 2026-10-13: `DNSWATCHER_METRICS_USERNAME` set without - 2026-10-13: `DNSWATCHER_METRICS_USERNAME` set without
`DNSWATCHER_METRICS_PASSWORD` stops startup (closes #268). `DNSWATCHER_METRICS_PASSWORD` stops startup (closes #268).
- 2026-10-13: a save syncs the state file to disk before renaming it into place,
so a crash cannot leave it empty and stop the next start (closes #264).
- 2026-10-13: a `PORT` that cannot be bound stops startup with an error naming - 2026-10-13: a `PORT` that cannot be bound stops startup with an error naming
the address, instead of running without the HTTP server (closes #266). the address, instead of running without the HTTP server (closes #266).
- 2026-10-13: a bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false - 2026-10-13: a bad `PORT`, `DNSWATCHER_TLS_EXPIRY_WARNING` or true/false
+22 -2
View File
@@ -4,6 +4,7 @@ package state
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
"maps" "maps"
@@ -253,10 +254,20 @@ func (s *State) Save() error {
return fmt.Errorf("creating data directory: %w", err) return fmt.Errorf("creating data directory: %w", err)
} }
// Atomic write: write to temp file, then rename // Atomic write: write to temp file, then rename. The temp file is
// synced before the rename and the directory after it, or a crash
// can leave the renamed state file empty.
tmpPath := path + ".tmp" tmpPath := path + ".tmp"
err = os.WriteFile(tmpPath, data, filePermissions) //nolint:gosec // path is from trusted config
tmpFile, err := os.OpenFile(
tmpPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, filePermissions,
)
if err == nil {
_, err = tmpFile.Write(data)
err = errors.Join(err, tmpFile.Sync(), tmpFile.Close())
}
if err != nil { if err != nil {
return fmt.Errorf("writing temp state file: %w", err) return fmt.Errorf("writing temp state file: %w", err)
} }
@@ -266,6 +277,15 @@ func (s *State) Save() error {
return fmt.Errorf("renaming state file: %w", err) return fmt.Errorf("renaming state file: %w", err)
} }
dir, err := os.Open(filepath.Dir(path))
if err == nil {
err = errors.Join(dir.Sync(), dir.Close())
}
if err != nil {
return fmt.Errorf("syncing data directory: %w", err)
}
s.log.Debug("state saved to disk", "path", path) s.log.Debug("state saved to disk", "path", path)
return nil return nil
+47
View File
@@ -618,6 +618,53 @@ func TestSaveAtomicWrite(t *testing.T) {
} }
} }
// TestSaveOverLeftoverTempFile verifies that a temp file left by a
// crash, longer than the state Save writes, leaves nothing of itself in
// the state file, and that no temp file remains after Save.
func TestSaveOverLeftoverTempFile(t *testing.T) {
t.Parallel()
dir := t.TempDir()
statePath := filepath.Join(dir, "state.json")
tmpPath := statePath + ".tmp"
err := os.WriteFile(tmpPath, []byte(strings.Repeat("x", 64*1024)), 0o600)
if err != nil {
t.Fatalf("writing leftover temp file: %v", err)
}
s := state.NewForTestWithDataDir(dir)
populateState(t, s)
err = s.Save()
if err != nil {
t.Fatalf("Save() error: %v", err)
}
//nolint:gosec // test file with known path
data, err := os.ReadFile(statePath)
if err != nil {
t.Fatalf("reading state file: %v", err)
}
var snap state.Snapshot
err = json.Unmarshal(data, &snap)
if err != nil {
t.Fatalf("state file is not complete JSON: %v", err)
}
if len(snap.Domains) != 2 {
t.Errorf("state file has %d domains, want 2", len(snap.Domains))
}
_, err = os.Stat(tmpPath)
if !os.IsNotExist(err) {
t.Error("temp file should not exist after save")
}
}
// TestLoadMissingFile verifies that loading a nonexistent file is not an error. // TestLoadMissingFile verifies that loading a nonexistent file is not an error.
func TestLoadMissingFile(t *testing.T) { func TestLoadMissingFile(t *testing.T) {
t.Parallel() t.Parallel()