direct: keep the state file's mode when saving

.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.

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)
 	}

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())
+}