Skip to content

Commit 7c62db4

Browse files
Sync dev to main (#90)
* deps(deps): bump github.com/gdamore/tcell/v2 from 2.13.1 to 2.13.2 in the security-patches group (#88) deps(deps): bump github.com/gdamore/tcell/v2 Bumps the security-patches group with 1 update: [github.com/gdamore/tcell/v2](https://github.com/gdamore/tcell). Updates `github.com/gdamore/tcell/v2` from 2.13.1 to 2.13.2 - [Release notes](https://github.com/gdamore/tcell/releases) - [Changelog](https://github.com/gdamore/tcell/blob/main/CHANGESv3.md) - [Commits](gdamore/tcell@v2.13.1...v2.13.2) --- updated-dependencies: - dependency-name: github.com/gdamore/tcell/v2 dependency-version: 2.13.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: security-patches ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Improve permission checks and test coverage Refactors permission checking logic to better handle transient I/O errors, adds error codes to CheckResult, and improves logging of failed checks. Extends test coverage for permission checks, backup safety, system compatibility, decryption workflow, and TUI wizards. Adds indirection for file creation to facilitate testing, and introduces new test helpers and runners for TUI flows. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
1 parent cc47832 commit 7c62db4

12 files changed

Lines changed: 622 additions & 14 deletions

File tree

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ toolchain go1.25.5
66

77
require (
88
filippo.io/age v1.2.1
9-
github.com/gdamore/tcell/v2 v2.13.1
9+
github.com/gdamore/tcell/v2 v2.13.2
1010
github.com/rivo/tview v0.42.0
1111
golang.org/x/crypto v0.45.0
1212
golang.org/x/term v0.37.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
66
filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
77
github.com/gdamore/encoding v1.0.1 h1:YzKZckdBL6jVt2Gc+5p82qhrGiqMdG/eNs6Wy0u3Uhw=
88
github.com/gdamore/encoding v1.0.1/go.mod h1:0Z0cMFinngz9kS1QfMjCP8TY7em3bZYeeklsSDPivEo=
9-
github.com/gdamore/tcell/v2 v2.13.1 h1:Ca2N6mHxhXuElCgn+nfKuZjS7gwNiIRKHFiljrZQ26A=
10-
github.com/gdamore/tcell/v2 v2.13.1/go.mod h1:+Wfe208WDdB7INEtCsNrAN6O2m+wsTPk1RAovjaILlo=
9+
github.com/gdamore/tcell/v2 v2.13.2 h1:5j4srfF8ow3HICOv/61/sOhQtA25qxEB2XR3Q/Bhx2g=
10+
github.com/gdamore/tcell/v2 v2.13.2/go.mod h1:+Wfe208WDdB7INEtCsNrAN6O2m+wsTPk1RAovjaILlo=
1111
github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag=
1212
github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
1313
github.com/rivo/tview v0.42.0 h1:b/ftp+RxtDsHSaynXTbJb+/n/BxDEi+W3UfF5jILK6c=

internal/checks/checks.go

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package checks
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"math"
78
"os"
@@ -12,6 +13,11 @@ import (
1213
"github.com/tis24dev/proxsave/internal/logging"
1314
)
1415

16+
// createTestFile is a small indirection over os.Create used by permission
17+
// checks to allow tests to inject controlled failures (e.g., EIO) without
18+
// depending on specific filesystem behavior.
19+
var createTestFile = os.Create
20+
1521
// Checker performs pre-backup validation checks
1622
type Checker struct {
1723
logger *logging.Logger
@@ -83,6 +89,7 @@ type CheckResult struct {
8389
Passed bool
8490
Message string
8591
Error error
92+
Code string
8693
}
8794

8895
// NewChecker creates a new pre-backup checker
@@ -260,13 +267,17 @@ func (c *Checker) CheckLockFile() CheckResult {
260267
// CheckPermissions verifies write permissions on required directories
261268
func (c *Checker) CheckPermissions() CheckResult {
262269
result := CheckResult{
263-
Name: "Permissions",
264-
Passed: false,
270+
Name: "Permissions",
271+
Passed: false,
272+
Code: "PERMISSION_CHECK",
265273
}
266274

267275
dirs := []string{c.config.BackupPath, c.config.LogPath}
268276

269-
for _, dir := range dirs {
277+
const maxAttempts = 3
278+
const retryDelay = 100 * time.Millisecond
279+
280+
for _, dir := range dirs {
270281
// Check if directory is writable
271282
testFile := filepath.Join(dir, fmt.Sprintf(".permission_test_%d", os.Getpid()))
272283

@@ -275,14 +286,55 @@ func (c *Checker) CheckPermissions() CheckResult {
275286
continue
276287
}
277288

278-
f, err := os.Create(testFile)
279-
if err != nil {
280-
result.Error = fmt.Errorf("no write permission in %s: %w", dir, err)
289+
var lastErr error
290+
291+
for attempt := 1; attempt <= maxAttempts; attempt++ {
292+
f, err := createTestFile(testFile)
293+
if err == nil {
294+
f.Close()
295+
lastErr = nil
296+
break
297+
}
298+
299+
lastErr = err
300+
301+
// Treat filesystem I/O errors as potentially transient and retry
302+
if errors.Is(err, syscall.EIO) && attempt < maxAttempts {
303+
c.logger.Warning("I/O error while testing write in %s (attempt %d/%d), will retry: %v",
304+
dir, attempt, maxAttempts, err)
305+
time.Sleep(retryDelay)
306+
continue
307+
}
308+
309+
// For non-transient errors or after exhausting retries, stop retrying
310+
break
311+
}
312+
313+
if lastErr != nil {
314+
err := lastErr
315+
var reason string
316+
code := "PERMISSION_CHECK_FAILED"
317+
318+
switch {
319+
case errors.Is(err, os.ErrPermission) || errors.Is(err, syscall.EACCES) || errors.Is(err, syscall.EPERM):
320+
reason = "no write permission"
321+
code = "PERMISSION_DENIED"
322+
case errors.Is(err, syscall.EROFS):
323+
reason = "filesystem is read-only"
324+
code = "FS_READONLY"
325+
case errors.Is(err, syscall.EIO):
326+
reason = "filesystem I/O error while testing write"
327+
code = "FS_IO_ERROR"
328+
default:
329+
reason = "failed to test write permission"
330+
}
331+
332+
result.Code = code
333+
result.Error = fmt.Errorf("%s in %s: %w", reason, dir, err)
281334
result.Message = result.Error.Error()
282335
c.logger.Error("%s", result.Message)
283336
return result
284337
}
285-
f.Close()
286338

287339
// Clean up test file
288340
if err := os.Remove(testFile); err != nil {

internal/checks/checks_test.go

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"os"
66
"path/filepath"
7+
"syscall"
78
"testing"
89
"time"
910

@@ -147,6 +148,109 @@ func TestCheckPermissions(t *testing.T) {
147148
if !result.Passed {
148149
t.Errorf("CheckPermissions failed: %s", result.Message)
149150
}
151+
if result.Code != "PERMISSION_CHECK" {
152+
t.Errorf("expected Code PERMISSION_CHECK on success, got %q", result.Code)
153+
}
154+
}
155+
156+
func TestCheckPermissionsPermissionDenied(t *testing.T) {
157+
logger := logging.New(types.LogLevelInfo, false)
158+
tmpDir := t.TempDir()
159+
160+
// When running tests as root, permission checks based on chmod may not
161+
// behave as expected because root can bypass filesystem permissions.
162+
// In that case, skip this test.
163+
if os.Geteuid() == 0 {
164+
t.Skip("skipping permission-denied check when running as root")
165+
}
166+
167+
// Create a directory that is not writable
168+
protectedDir := filepath.Join(tmpDir, "protected")
169+
if err := os.Mkdir(protectedDir, 0755); err != nil {
170+
t.Fatalf("failed to create protected dir: %v", err)
171+
}
172+
if err := os.Chmod(protectedDir, 0555); err != nil {
173+
t.Fatalf("failed to chmod protected dir: %v", err)
174+
}
175+
176+
config := &CheckerConfig{
177+
BackupPath: protectedDir,
178+
LogPath: tmpDir,
179+
LockDirPath: tmpDir,
180+
SkipPermissionCheck: false,
181+
DryRun: false,
182+
}
183+
184+
checker := NewChecker(logger, config)
185+
result := checker.CheckPermissions()
186+
187+
if result.Passed {
188+
t.Fatalf("CheckPermissions should fail for non-writable directory")
189+
}
190+
if result.Code != "PERMISSION_DENIED" {
191+
t.Errorf("expected Code PERMISSION_DENIED, got %q", result.Code)
192+
}
193+
}
194+
195+
func TestCheckPermissionsDryRun(t *testing.T) {
196+
logger := logging.New(types.LogLevelInfo, false)
197+
tmpDir := t.TempDir()
198+
199+
config := &CheckerConfig{
200+
BackupPath: tmpDir,
201+
LogPath: tmpDir,
202+
LockDirPath: tmpDir,
203+
SkipPermissionCheck: false,
204+
DryRun: true,
205+
}
206+
207+
checker := NewChecker(logger, config)
208+
result := checker.CheckPermissions()
209+
210+
if !result.Passed {
211+
t.Fatalf("CheckPermissions should pass in dry-run mode, got: %s", result.Message)
212+
}
213+
if result.Code != "PERMISSION_CHECK" {
214+
t.Errorf("expected Code PERMISSION_CHECK in dry-run mode, got %q", result.Code)
215+
}
216+
}
217+
218+
func TestCheckPermissionsEIORetryAndFailure(t *testing.T) {
219+
logger := logging.New(types.LogLevelInfo, false)
220+
tmpDir := t.TempDir()
221+
222+
// Save and restore original createTestFile to avoid side effects
223+
origCreate := createTestFile
224+
defer func() {
225+
createTestFile = origCreate
226+
}()
227+
228+
attempts := 0
229+
createTestFile = func(name string) (*os.File, error) {
230+
attempts++
231+
return nil, syscall.EIO
232+
}
233+
234+
config := &CheckerConfig{
235+
BackupPath: tmpDir,
236+
LogPath: tmpDir,
237+
LockDirPath: tmpDir,
238+
SkipPermissionCheck: false,
239+
DryRun: false,
240+
}
241+
242+
checker := NewChecker(logger, config)
243+
result := checker.CheckPermissions()
244+
245+
if result.Passed {
246+
t.Fatalf("CheckPermissions should fail when all attempts return EIO")
247+
}
248+
if result.Code != "FS_IO_ERROR" {
249+
t.Errorf("expected Code FS_IO_ERROR, got %q", result.Code)
250+
}
251+
if attempts != 3 {
252+
t.Errorf("expected 3 attempts on EIO, got %d", attempts)
253+
}
150254
}
151255

152256
func TestCheckDirectories(t *testing.T) {
@@ -257,6 +361,39 @@ func TestRunAllChecks(t *testing.T) {
257361
checker.ReleaseLock()
258362
}
259363

364+
func TestRunAllChecksSkipPermissionCheck(t *testing.T) {
365+
logger := logging.New(types.LogLevelInfo, false)
366+
tmpDir := t.TempDir()
367+
368+
config := &CheckerConfig{
369+
BackupPath: tmpDir,
370+
LogPath: tmpDir,
371+
LockDirPath: tmpDir,
372+
MinDiskPrimaryGB: 0.001,
373+
MinDiskSecondaryGB: 0,
374+
MinDiskCloudGB: 0,
375+
LockFilePath: filepath.Join(tmpDir, ".backup.lock"),
376+
MaxLockAge: 1 * time.Hour,
377+
SkipPermissionCheck: true,
378+
DryRun: false,
379+
}
380+
381+
checker := NewChecker(logger, config)
382+
ctx := context.Background()
383+
384+
results, err := checker.RunAllChecks(ctx)
385+
if err != nil {
386+
t.Fatalf("RunAllChecks (SkipPermissionCheck=true) failed unexpectedly: %v", err)
387+
}
388+
389+
// Permissions check should be skipped: assert that no result is named "Permissions".
390+
for _, r := range results {
391+
if r.Name == "Permissions" {
392+
t.Fatalf("expected Permissions check to be skipped, but it was executed")
393+
}
394+
}
395+
}
396+
260397
func TestDryRunMode(t *testing.T) {
261398
logger := logging.New(types.LogLevelInfo, false)
262399
tmpDir := t.TempDir()

internal/orchestrator/.backup.lock

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
pid=3086032
1+
pid=37703
22
host=pve1
3-
time=2025-12-07T11:05:54+01:00
3+
time=2025-12-08T16:51:00+01:00

internal/orchestrator/backup_safety_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,98 @@ func TestCleanupOldSafetyBackups(t *testing.T) {
175175
_ = os.Remove(newBackup)
176176
}
177177

178+
func TestCreateSafetyBackupArchivesSelectedPaths(t *testing.T) {
179+
fake := NewFakeFS()
180+
origFS := safetyFS
181+
safetyFS = fake
182+
t.Cleanup(func() { safetyFS = origFS })
183+
184+
fixed := time.Date(2024, time.March, 1, 15, 4, 5, 0, time.UTC)
185+
origNow := safetyNow
186+
safetyNow = func() time.Time { return fixed }
187+
t.Cleanup(func() { safetyNow = origNow })
188+
189+
destRoot := "/restore-target"
190+
if err := fake.AddFile(filepath.Join(destRoot, "etc/config.txt"), []byte("config-data")); err != nil {
191+
t.Fatalf("add config file: %v", err)
192+
}
193+
if err := fake.AddDir(filepath.Join(destRoot, "var/lib/app")); err != nil {
194+
t.Fatalf("add directory: %v", err)
195+
}
196+
if err := fake.WriteFile(filepath.Join(destRoot, "var/lib/app/state.txt"), []byte("state"), 0o640); err != nil {
197+
t.Fatalf("add state file: %v", err)
198+
}
199+
200+
categories := []Category{
201+
{ID: "etc", Paths: []string{"./etc/config.txt"}},
202+
{ID: "var", Paths: []string{"./var/lib/app"}},
203+
}
204+
205+
logger := logging.New(types.LogLevelInfo, false)
206+
207+
result, err := CreateSafetyBackup(logger, categories, destRoot)
208+
if err != nil {
209+
t.Fatalf("CreateSafetyBackup error: %v", err)
210+
}
211+
212+
expectedName := "restore_backup_" + fixed.Format("20060102_150405") + ".tar.gz"
213+
expectedPath := filepath.Join("/tmp", "proxsave", expectedName)
214+
if result.BackupPath != expectedPath {
215+
t.Fatalf("unexpected backup path: got %s want %s", result.BackupPath, expectedPath)
216+
}
217+
if !result.Timestamp.Equal(fixed) {
218+
t.Fatalf("timestamp mismatch: got %v want %v", result.Timestamp, fixed)
219+
}
220+
if result.FilesBackedUp != 2 {
221+
t.Fatalf("expected 2 files backed up, got %d", result.FilesBackedUp)
222+
}
223+
if result.TotalSize == 0 {
224+
t.Fatalf("expected non-zero total size")
225+
}
226+
227+
archiveFile, err := os.Open(fake.onDisk(result.BackupPath))
228+
if err != nil {
229+
t.Fatalf("open archive: %v", err)
230+
}
231+
defer archiveFile.Close()
232+
233+
gzr, err := gzip.NewReader(archiveFile)
234+
if err != nil {
235+
t.Fatalf("gzip reader: %v", err)
236+
}
237+
defer gzr.Close()
238+
tr := tar.NewReader(gzr)
239+
240+
var entries []string
241+
for {
242+
hdr, err := tr.Next()
243+
if err != nil {
244+
break
245+
}
246+
entries = append(entries, filepath.ToSlash(hdr.Name))
247+
}
248+
249+
assertContains := func(name string) {
250+
for _, entry := range entries {
251+
if entry == name {
252+
return
253+
}
254+
}
255+
t.Fatalf("archive missing %s; entries=%v", name, entries)
256+
}
257+
258+
assertContains("etc/config.txt")
259+
assertContains("var/lib/app/state.txt")
260+
261+
locationData, err := os.ReadFile(fake.onDisk(filepath.Join("/tmp", "proxsave", "restore_backup_location.txt")))
262+
if err != nil {
263+
t.Fatalf("location file: %v", err)
264+
}
265+
if strings.TrimSpace(string(locationData)) != result.BackupPath {
266+
t.Fatalf("location file contents mismatch: %q", string(locationData))
267+
}
268+
}
269+
178270
func TestRestoreSafetyBackup_MaliciousSymlinks(t *testing.T) {
179271
logger := logging.New(types.LogLevelInfo, false)
180272

0 commit comments

Comments
 (0)