aboutsummaryrefslogtreecommitdiff
path: root/internal/admin
diff options
context:
space:
mode:
Diffstat (limited to 'internal/admin')
-rw-r--r--internal/admin/correct_audit_fixes_test.go33
-rw-r--r--internal/admin/undo.go99
2 files changed, 97 insertions, 35 deletions
diff --git a/internal/admin/correct_audit_fixes_test.go b/internal/admin/correct_audit_fixes_test.go
index d7b713e..282e943 100644
--- a/internal/admin/correct_audit_fixes_test.go
+++ b/internal/admin/correct_audit_fixes_test.go
@@ -89,3 +89,36 @@ func TestResolveDataPathRejectsSymlinkEscape(t *testing.T) {
t.Fatalf("normal path rejected: %v", err)
}
}
+
+// Regression: an intentionally empty before-image must be restorable after a
+// restart. The existence flag distinguishes it from a missing (failed-read)
+// image, which both persist as "".
+func TestUndoRestoresIntentionallyEmptyFile(t *testing.T) {
+ dir := t.TempDir()
+ path := filepath.Join(dir, "rooms", "1.yaml")
+ if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(path, []byte("id: 1\n"), 0644); err != nil {
+ t.Fatal(err)
+ }
+ us := NewUndoStack(dir)
+ us.Push(ChangeDesc{
+ Description: "write to an originally empty file",
+ FilePath: path,
+ OldContent: []byte{}, // exists, but empty
+ NewContent: []byte("id: 1\n"),
+ })
+ // Reload from disk to exercise the persistence round trip.
+ us2 := NewUndoStack(dir)
+ if ch := us2.Undo(); ch == nil {
+ t.Fatalf("undo of an empty-but-existing before-image was refused: %v", us2.LastError())
+ }
+ data, err := os.ReadFile(path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(data) != 0 {
+ t.Fatalf("file not restored to empty: %q", data)
+ }
+}
diff --git a/internal/admin/undo.go b/internal/admin/undo.go
index c3231e3..d9ef263 100644
--- a/internal/admin/undo.go
+++ b/internal/admin/undo.go
@@ -21,15 +21,23 @@ type ExtraFile struct {
IsDelete bool `json:"is_delete,omitempty"`
IsCreate bool `json:"is_create,omitempty"`
IsDirDelete bool `json:"is_dir_delete,omitempty"`
+ OldExists bool `json:"-"`
+ NewExists bool `json:"-"`
}
type ChangeDesc struct {
- Time string `json:"time"`
- Description string `json:"description"`
- FilePath string `json:"file_path"`
- NewFilePath string `json:"new_file_path,omitempty"`
- OldContent []byte `json:"old_content"`
- NewContent []byte `json:"new_content"`
+ Time string `json:"time"`
+ Description string `json:"description"`
+ FilePath string `json:"file_path"`
+ NewFilePath string `json:"new_file_path,omitempty"`
+ OldContent []byte `json:"old_content"`
+ NewContent []byte `json:"new_content"`
+ // OldExists/NewExists record whether the before/after image was actually
+ // captured. A failed read leaves nil content, but nil and an empty file
+ // both persist as ""; the flag distinguishes them so undo can restore an
+ // intentionally empty file yet still refuse a missing snapshot.
+ OldExists bool `json:"old_exists,omitempty"`
+ NewExists bool `json:"new_exists,omitempty"`
IsDelete bool `json:"is_delete"`
IsCreate bool `json:"is_create"`
IsDirDelete bool `json:"is_dir_delete,omitempty"`
@@ -164,6 +172,23 @@ func (us *UndoStack) Push(desc ChangeDesc) {
us.mu.Lock()
defer us.mu.Unlock()
desc.Time = time.Now().Format(time.RFC3339)
+ // Infer the existence flags from the captured content: a non-nil image
+ // (including an empty file's []byte{}) means it exists; nil means the read
+ // failed or the file was absent. Callers may set the flags explicitly.
+ if desc.OldContent != nil {
+ desc.OldExists = true
+ }
+ if desc.NewContent != nil {
+ desc.NewExists = true
+ }
+ for i := range desc.ExtraFiles {
+ if desc.ExtraFiles[i].OldContent != nil {
+ desc.ExtraFiles[i].OldExists = true
+ }
+ if desc.ExtraFiles[i].NewContent != nil {
+ desc.ExtraFiles[i].NewExists = true
+ }
+ }
us.history = append(us.history, desc)
us.redo = nil
if len(us.history) > us.maxSize {
@@ -235,12 +260,13 @@ func (us *UndoStack) Redo() *ChangeDesc {
return &last
}
-// requireSnapshot rejects a nil before/after image on a change that must
-// restore real content. A failed before-image read leaves nil, and writing
-// it would truncate the file to zero bytes (which parses as a zero-value
-// def), so a missing image must fail the undo/redo instead.
-func requireSnapshot(content []byte, path, action string) error {
- if content == nil {
+// requireSnapshot rejects a change whose before/after image was never
+// recorded (a failed read). Writing a missing image would truncate the file to
+// zero bytes, which parses as a zero-value def. The existence flag (not the
+// content length) is the source of truth, since nil and an empty file both
+// persist as "".
+func requireSnapshot(exists bool, path, action string) error {
+ if !exists {
return fmt.Errorf("%s %s: no snapshot recorded", action, path)
}
return nil
@@ -277,7 +303,7 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error {
return fmt.Errorf("recreate dir %s: %w", c.FilePath, err)
}
} else {
- if c.OldContent == nil {
+ if !c.OldExists {
return fmt.Errorf("restore deleted %s: no snapshot recorded", c.FilePath)
}
if err := us.writeSnapshot(c.FilePath, restored); err != nil {
@@ -289,7 +315,7 @@ func (us *UndoStack) applyUndo(c ChangeDesc) error {
return fmt.Errorf("remove created %s: %w", c.FilePath, err)
}
} else {
- if err := requireSnapshot(c.OldContent, c.FilePath, "revert"); err != nil {
+ if err := requireSnapshot(c.OldExists, c.FilePath, "revert"); err != nil {
return err
}
if err := us.writeSnapshot(c.FilePath, restored); err != nil {
@@ -354,7 +380,7 @@ func (us *UndoStack) applyRedo(c ChangeDesc) error {
if c.NewFilePath != "" {
target = c.NewFilePath
}
- if err := requireSnapshot(c.NewContent, target, "reapply"); err != nil {
+ if err := requireSnapshot(c.NewExists, target, "reapply"); err != nil {
return err
}
if err := us.writeSnapshot(target, restored); err != nil {
@@ -380,14 +406,14 @@ func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error {
return fmt.Errorf("undo mkdir: %w", err)
}
case ef.IsDelete:
- if ef.OldContent == nil {
+ if !ef.OldExists {
return fmt.Errorf("undo restore %s: no snapshot recorded", ef.FilePath)
}
if err := us.writeRestored(ef.FilePath, ef.OldContent, ef.NewFilePath); err != nil {
return fmt.Errorf("undo restore %s: %w", ef.FilePath, err)
}
case ef.NewFilePath != "":
- if err := requireSnapshot(ef.OldContent, ef.FilePath, "undo restore"); err != nil {
+ if err := requireSnapshot(ef.OldExists, ef.FilePath, "undo restore"); err != nil {
return err
}
if err := os.Remove(ef.NewFilePath); err != nil && !os.IsNotExist(err) {
@@ -397,7 +423,7 @@ func (us *UndoStack) applyUndoExtraFile(ef ExtraFile) error {
return fmt.Errorf("undo restore %s: %w", ef.FilePath, err)
}
default:
- if err := requireSnapshot(ef.OldContent, ef.FilePath, "undo revert"); err != nil {
+ if err := requireSnapshot(ef.OldExists, ef.FilePath, "undo revert"); err != nil {
return err
}
if err := us.writeRestored(ef.FilePath, ef.OldContent); err != nil {
@@ -418,7 +444,7 @@ func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error {
return fmt.Errorf("redo create %s: %w", ef.FilePath, err)
}
case ef.NewFilePath != "":
- if err := requireSnapshot(ef.NewContent, ef.NewFilePath, "redo create"); err != nil {
+ if err := requireSnapshot(ef.NewExists, ef.NewFilePath, "redo create"); err != nil {
return err
}
if err := os.Remove(ef.FilePath); err != nil && !os.IsNotExist(err) {
@@ -428,7 +454,7 @@ func (us *UndoStack) applyRedoExtraFile(ef ExtraFile) error {
return fmt.Errorf("redo create %s: %w", ef.NewFilePath, err)
}
default:
- if err := requireSnapshot(ef.NewContent, ef.FilePath, "redo reapply"); err != nil {
+ if err := requireSnapshot(ef.NewExists, ef.FilePath, "redo reapply"); err != nil {
return err
}
if err := us.writeRestored(ef.FilePath, ef.NewContent); err != nil {
@@ -561,6 +587,8 @@ type undoExtraEntry struct {
NewFilePath string `json:"new_file_path,omitempty"`
OldContent string `json:"old_content"`
NewContent string `json:"new_content"`
+ OldExists bool `json:"old_exists,omitempty"`
+ NewExists bool `json:"new_exists,omitempty"`
IsDelete bool `json:"is_delete,omitempty"`
IsCreate bool `json:"is_create,omitempty"`
IsDirDelete bool `json:"is_dir_delete,omitempty"`
@@ -573,6 +601,8 @@ type undoEntry struct {
NewFilePath string `json:"new_file_path,omitempty"`
OldContent string `json:"old_content"`
NewContent string `json:"new_content"`
+ OldExists bool `json:"old_exists,omitempty"`
+ NewExists bool `json:"new_exists,omitempty"`
IsDelete bool `json:"is_delete"`
IsCreate bool `json:"is_create"`
IsDirDelete bool `json:"is_dir_delete,omitempty"`
@@ -596,6 +626,8 @@ func (us *UndoStack) save() {
NewFilePath: ef.NewFilePath,
OldContent: string(us.redactContent(ef.OldContent, ef.FilePath, ef.NewFilePath)),
NewContent: string(us.redactContent(ef.NewContent, ef.FilePath, ef.NewFilePath)),
+ OldExists: ef.OldExists,
+ NewExists: ef.NewExists,
IsDelete: ef.IsDelete,
IsCreate: ef.IsCreate,
IsDirDelete: ef.IsDirDelete,
@@ -608,6 +640,8 @@ func (us *UndoStack) save() {
NewFilePath: c.NewFilePath,
OldContent: string(us.redactContent(c.OldContent, c.FilePath, c.NewFilePath)),
NewContent: string(us.redactContent(c.NewContent, c.FilePath, c.NewFilePath)),
+ OldExists: c.OldExists,
+ NewExists: c.NewExists,
IsDelete: c.IsDelete,
IsCreate: c.IsCreate,
IsDirDelete: c.IsDirDelete,
@@ -762,37 +796,32 @@ func (us *UndoStack) load() {
toChanges := func(entries []undoEntry) []ChangeDesc {
out := make([]ChangeDesc, 0, len(entries))
for _, e := range entries {
- // A before-image recorded as nil is persisted as "" and would
- // reload as a non-nil empty slice, defeating requireSnapshot and
- // letting undo truncate the file to zero bytes. Map it back to nil
- // so the guard fires. An intentionally empty before-image is also
- // refused, which is safer than zeroing a live file.
- oldContent := []byte(e.OldContent)
- if e.OldContent == "" {
- oldContent = nil
- }
+ // The existence flag distinguishes a missing before-image (a failed
+ // read, persisted as "") from an intentionally empty file. Older
+ // history files predate the flag, so infer it from non-empty content;
+ // an old empty image stays missing (the safe behaviour).
c := ChangeDesc{
Time: e.Time,
Description: e.Description,
FilePath: e.FilePath,
NewFilePath: e.NewFilePath,
- OldContent: oldContent,
+ OldContent: []byte(e.OldContent),
NewContent: []byte(e.NewContent),
+ OldExists: e.OldExists || e.OldContent != "",
+ NewExists: e.NewExists || e.NewContent != "",
IsDelete: e.IsDelete,
IsCreate: e.IsCreate,
IsDirDelete: e.IsDirDelete,
IDMap: e.IDMap,
}
for _, ef := range e.ExtraFiles {
- oldExtra := []byte(ef.OldContent)
- if ef.OldContent == "" {
- oldExtra = nil
- }
c.ExtraFiles = append(c.ExtraFiles, ExtraFile{
FilePath: ef.FilePath,
NewFilePath: ef.NewFilePath,
- OldContent: oldExtra,
+ OldContent: []byte(ef.OldContent),
NewContent: []byte(ef.NewContent),
+ OldExists: ef.OldExists || ef.OldContent != "",
+ NewExists: ef.NewExists || ef.NewContent != "",
IsDelete: ef.IsDelete,
IsCreate: ef.IsCreate,
IsDirDelete: ef.IsDirDelete,
generated by cgit v1.2.3 (git 2.25.1) at 2026-10-24 10:37:29 +0000