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..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 @@ -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,20 @@ 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. 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 { + log.Warnf(ctx, "Failed to preserve mode %o of %s: %v", info.Mode().Perm(), db.Path, err) + } + } else if !errors.Is(err, fs.ErrNotExist) { + log.Warnf(ctx, "Failed to read mode of %s: %v", 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()) +}