From db7f753509ec54487f07f1f3e32334e50b3a9189 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 24 Aug 2026 14:33:27 +0200 Subject: [PATCH 1/2] direct: keep the state file's mode when saving Saving writes a temp file and renames it into place, and a temp file always starts at 0o600, so an existing state file that was deliberately readable to a group or to CI narrowed on every save. Writing in place, before v1.13.0, left the mode alone. Carry the replaced file's mode over to the temp file before the rename. Co-authored-by: Isaac --- .../bundles/state-save-preserve-mode.md | 1 + bundle/direct/dstate/state.go | 15 +++++++++- bundle/direct/dstate/state_test.go | 29 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 .nextchanges/bundles/state-save-preserve-mode.md diff --git a/.nextchanges/bundles/state-save-preserve-mode.md b/.nextchanges/bundles/state-save-preserve-mode.md new file mode 100644 index 0000000000..cf58649219 --- /dev/null +++ b/.nextchanges/bundles/state-save-preserve-mode.md @@ -0,0 +1 @@ +Keep the permissions of the deployment state file when saving it. Since v1.13.0 an existing state file was narrowed to `0600` on every save. diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go index af2e2638eb..79c240ee9a 100644 --- a/bundle/direct/dstate/state.go +++ b/bundle/direct/dstate/state.go @@ -614,7 +614,8 @@ func (db *DeploymentState) unlockedSave() error { return fmt.Errorf("failed to create directory %#v: %w", dir, err) } - // CreateTemp creates the file with mode 0o600, matching the state file. + // CreateTemp creates the file with mode 0o600, which is the mode a new state + // file gets; an existing one keeps its own mode, see below. tmp, err := os.CreateTemp(dir, "."+filepath.Base(db.Path)+".tmp-*") if err != nil { return fmt.Errorf("failed to create temp file for %#v: %w", db.Path, err) @@ -633,6 +634,18 @@ func (db *DeploymentState) unlockedSave() error { return fmt.Errorf("failed to close %#v: %w", tmpPath, err) } + // Carry over the mode of the state file being replaced. Writing in place used + // to leave it alone, whereas the temp file always starts at 0o600, so without + // this a state file deliberately made readable to a group or to CI silently + // narrows on the next deploy. + if info, err := os.Stat(db.Path); err == nil { + if err := os.Chmod(tmpPath, info.Mode().Perm()); err != nil { + return fmt.Errorf("failed to set mode on %#v: %w", tmpPath, err) + } + } else if !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("failed to stat %#v: %w", db.Path, err) + } + if err := os.Rename(tmpPath, db.Path); err != nil { return fmt.Errorf("failed to save resources state to %#v: %w", db.Path, err) } diff --git a/bundle/direct/dstate/state_test.go b/bundle/direct/dstate/state_test.go index 34ed9ff1c5..eb3b8c5f9c 100644 --- a/bundle/direct/dstate/state_test.go +++ b/bundle/direct/dstate/state_test.go @@ -281,3 +281,32 @@ func TestOpenFailureLeavesStateClosed(t *testing.T) { assert.Equal(t, "test-lineage", db.Data.Lineage) mustFinalize(t, &db) } + +// TestSaveKeepsStateFileMode pins that saving keeps the mode of the state file it +// replaces. The file is written as a temp file and renamed into place, and a temp +// file always starts at 0o600, so a state file deliberately made readable to a +// group or to CI would otherwise narrow on every deploy. +// +// On Windows only the read-only bit is modelled, so this asserts the mode is +// carried over rather than asserting a specific value. +func TestSaveKeepsStateFileMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + + var db DeploymentState + require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true))) + require.NoError(t, db.SaveState("jobs.my_job", "123", map[string]string{"key": "val"}, nil)) + mustFinalize(t, &db) + + require.NoError(t, os.Chmod(path, 0o640)) + before, err := os.Stat(path) + require.NoError(t, err) + + var db2 DeploymentState + require.NoError(t, db2.Open(t.Context(), path, WithRecovery(true), WithWrite(true))) + require.NoError(t, db2.SaveState("jobs.my_job", "456", map[string]string{"key": "val2"}, nil)) + mustFinalize(t, &db2) + + after, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, before.Mode().Perm(), after.Mode().Perm()) +} From a99ea1298d46b0b587bed36d954a5fa66184bbf0 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 24 Aug 2026 15:16:13 +0200 Subject: [PATCH 2/2] direct: warn instead of failing when the state file's mode cannot be kept Persisting the state matters more than its mode, so neither the stat nor the chmod should abort the save. Co-authored-by: Isaac --- bundle/direct/dstate/state.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go index 79c240ee9a..1493f9ab8c 100644 --- a/bundle/direct/dstate/state.go +++ b/bundle/direct/dstate/state.go @@ -352,7 +352,7 @@ func (db *DeploymentState) replayWAL(ctx context.Context) error { return fmt.Errorf("WAL recovery failed: %w", err) } if hasEntries { - if err := db.unlockedSave(); err != nil { + if err := db.unlockedSave(ctx); err != nil { return err } } @@ -603,7 +603,7 @@ func (db *DeploymentState) ExportState(ctx context.Context) resourcestate.Export // only then removes the WAL, and Open parses the state file before it looks at // the WAL. A torn write would therefore leave a state file that Open rejects // next to an intact WAL it never reads. -func (db *DeploymentState) unlockedSave() error { +func (db *DeploymentState) unlockedSave(ctx context.Context) error { data, err := json.MarshalIndent(db.Data, "", " ") if err != nil { return err @@ -637,13 +637,15 @@ func (db *DeploymentState) unlockedSave() error { // Carry over the mode of the state file being replaced. Writing in place used // to leave it alone, whereas the temp file always starts at 0o600, so without // this a state file deliberately made readable to a group or to CI silently - // narrows on the next deploy. + // narrows on the next deploy. Best-effort: persisting the state matters more + // than its mode, so a failure here warns and the save goes ahead. A state file + // that does not exist yet keeps the temp file's 0o600. if info, err := os.Stat(db.Path); err == nil { if err := os.Chmod(tmpPath, info.Mode().Perm()); err != nil { - return fmt.Errorf("failed to set mode on %#v: %w", tmpPath, err) + log.Warnf(ctx, "Failed to preserve mode %o of %s: %v", info.Mode().Perm(), db.Path, err) } } else if !errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("failed to stat %#v: %w", db.Path, err) + log.Warnf(ctx, "Failed to read mode of %s: %v", db.Path, err) } if err := os.Rename(tmpPath, db.Path); err != nil {