From ada3f9fc7f53a9e96d5df5dd9623b4f4418d6479 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 18 Aug 2026 14:05:52 +0200 Subject: [PATCH] fix(windows): protect lifecycle and backup state --- .../tht/internal/authstorage/storage_test.go | 3 ++ tools/tht/internal/backup/create.go | 6 +++ tools/tht/internal/backup/create_test.go | 29 ++++++++++--- tools/tht/internal/backup/preflight_test.go | 13 +++++- tools/tht/internal/backup/restore_test.go | 11 ++++- tools/tht/internal/lifecycle/lock.go | 41 +++++++++++++++---- tools/tht/internal/lifecycle/lock_test.go | 40 ++++++++++++++++-- 7 files changed, 122 insertions(+), 21 deletions(-) diff --git a/tools/tht/internal/authstorage/storage_test.go b/tools/tht/internal/authstorage/storage_test.go index 4017e20b..1beb453f 100644 --- a/tools/tht/internal/authstorage/storage_test.go +++ b/tools/tht/internal/authstorage/storage_test.go @@ -820,6 +820,9 @@ func privateTestRoot(t *testing.T) string { if err != nil { t.Fatal(err) } + if err := safeio.ProtectPrivateDirectory(root); err != nil { + t.Fatal(err) + } t.Cleanup(func() { _ = os.RemoveAll(root) }) return root } diff --git a/tools/tht/internal/backup/create.go b/tools/tht/internal/backup/create.go index 807cca1e..1f1cdd95 100644 --- a/tools/tht/internal/backup/create.go +++ b/tools/tht/internal/backup/create.go @@ -186,6 +186,12 @@ func createWithDependenciesTransaction(ctx context.Context, transaction *lifecyc if err != nil { return Result{}, err } + if request.IncludeSecrets { + if err := safeio.ProtectPrivateRegular(reservation.path); err != nil { + _ = reservation.RemoveIfOwned() + return Result{}, errors.New("protect secret-bearing backup output") + } + } published := false defer func() { if !published { diff --git a/tools/tht/internal/backup/create_test.go b/tools/tht/internal/backup/create_test.go index b2734709..991a7126 100644 --- a/tools/tht/internal/backup/create_test.go +++ b/tools/tht/internal/backup/create_test.go @@ -11,6 +11,7 @@ import ( "io" "os" "path/filepath" + "runtime" "sort" "strings" "testing" @@ -19,6 +20,7 @@ import ( "github.com/aritmolab/thothii/tools/tht/internal/compose" "github.com/aritmolab/thothii/tools/tht/internal/config" "github.com/aritmolab/thothii/tools/tht/internal/lifecycle" + "github.com/aritmolab/thothii/tools/tht/internal/safeio" ) var requiredTestVolumes = []string{"settings", "pi-state", "workspace-registry", "workspace-secrets", "sessions", "qdrant-data", "embedding-models"} @@ -193,7 +195,7 @@ func TestCreateReferencesAuthFilesByDefaultAndArchivesThemOnlyWithSecretCustody( t.Fatalf("default backup did not record only the auth.yaml configuration path: %#v", defaultArchive.manifest.Entries) } - secretOutput := filepath.Join(t.TempDir(), "with-auth-secrets.zip") + secretOutput := canonicalBackupOutput(t, "with-auth-secrets.zip") secretResult, err := createWithDependencies(context.Background(), fixture.installation, CreateRequest{Output: secretOutput, IncludeSecrets: true, Confirm: true}, testDependencies(t, newBackupRunner(fixture.installation, false))) if err != nil { t.Fatal(err) @@ -410,7 +412,7 @@ func TestCreateExcludesExternalSecretPayloadsByDefaultButRecordsDigests(t *testi func TestCreateRequiresConfirmationToIncludeSecretsAndUsesOwnerOnlyMode(t *testing.T) { fixture := newBackupFixture(t, "local") - output := filepath.Join(t.TempDir(), "with-secrets.zip") + output := canonicalBackupOutput(t, "with-secrets.zip") request := CreateRequest{Output: output, IncludeSecrets: true} if _, err := createWithDependencies(context.Background(), fixture.installation, request, testDependencies(t, newBackupRunner(fixture.installation, false))); !errors.Is(err, ErrConfirmationRequired) { t.Fatalf("Create() error = %v, want ErrConfirmationRequired", err) @@ -428,8 +430,13 @@ func TestCreateRequiresConfirmationToIncludeSecretsAndUsesOwnerOnlyMode(t *testi if err != nil { t.Fatal(err) } - if got := info.Mode().Perm(); got != 0o600 { - t.Fatalf("archive mode = %#o, want 0600", got) + if err := safeio.ValidatePrivateRegular(output); err != nil { + t.Fatalf("secret-bearing archive protection = %v", err) + } + if runtime.GOOS != "windows" { + if got := info.Mode().Perm(); got != 0o600 { + t.Fatalf("archive mode = %#o, want 0600", got) + } } archive := readFixtureArchive(t, output) if !archive.manifest.IncludesSecrets || !bytes.Contains(bytes.Join(mapValues(archive.files), nil), []byte(fixture.secretValue)) { @@ -550,7 +557,10 @@ func TestCreateTransactionCapabilityRefusesUnlockedOrForeignInstallation(t *test t.Fatalf("Docker runner was called without a transaction capability: %v", runner.calls) } - otherRoot := t.TempDir() + otherRoot, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } other := config.Installation{ProjectDirectory: otherRoot, Path: filepath.Join(otherRoot, "thothii-installation.yaml")} transaction, err := lifecycle.AcquireTransaction(other) if err != nil { @@ -798,6 +808,15 @@ func (fixture *backupFixture) writeEnvironment(t *testing.T) { } } +func canonicalBackupOutput(t *testing.T, name string) string { + t.Helper() + directory, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + return filepath.Join(directory, name) +} + type fakeBackupRunner struct { installation config.Installation running bool diff --git a/tools/tht/internal/backup/preflight_test.go b/tools/tht/internal/backup/preflight_test.go index e31b8993..6aff7f66 100644 --- a/tools/tht/internal/backup/preflight_test.go +++ b/tools/tht/internal/backup/preflight_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/aritmolab/thothii/tools/tht/internal/config" + "github.com/aritmolab/thothii/tools/tht/internal/safeio" ) func TestPreflightReturnsValidatedMetadataAndCallsAllTargetChecksWithoutExtracting(t *testing.T) { @@ -573,10 +574,20 @@ func preflightTestInstallation(t *testing.T) config.Installation { if err != nil { t.Fatal(err) } - return config.Installation{ + installation := config.Installation{ Path: filepath.Join(root, "deploy", "local-dev", "thothii-installation.yaml"), ProjectDirectory: root, } + controlParent := filepath.Dir(installation.ControlDirectory()) + for _, directory := range []string{controlParent, installation.ControlDirectory()} { + if err := os.MkdirAll(directory, 0o700); err != nil { + t.Fatal(err) + } + if err := safeio.ProtectPrivateDirectory(directory); err != nil { + t.Fatalf("protect preflight fixture directory %q: %v", directory, err) + } + } + return installation } func permissivePreflightDependencies() PreflightDependencies { diff --git a/tools/tht/internal/backup/restore_test.go b/tools/tht/internal/backup/restore_test.go index 87075a0e..285c70c8 100644 --- a/tools/tht/internal/backup/restore_test.go +++ b/tools/tht/internal/backup/restore_test.go @@ -16,17 +16,21 @@ import ( "github.com/aritmolab/thothii/tools/tht/internal/compose" "github.com/aritmolab/thothii/tools/tht/internal/config" "github.com/aritmolab/thothii/tools/tht/internal/lifecycle" + "github.com/aritmolab/thothii/tools/tht/internal/safeio" ) func TestRestorePublicPathUsesConcreteProductionPreflight(t *testing.T) { - root := t.TempDir() + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } installation := config.Installation{ Path: filepath.Join(root, "deploy", "local-dev", "thothii-installation.yaml"), ProjectDirectory: root, } missing := filepath.Join(root, "missing.zip") - _, err := Restore(context.Background(), installation, RestoreRequest{Archive: missing, Confirm: true}) + _, err = Restore(context.Background(), installation, RestoreRequest{Archive: missing, Confirm: true}) if err == nil || strings.Contains(err.Error(), "dependencies are unavailable") || !strings.Contains(err.Error(), "backup archive") { t.Fatalf("Restore() error = %v, want production archive preflight", err) @@ -924,6 +928,9 @@ func TestRecoveryCheckpointCleanupRemovesOnlyPrivateRegularFile(t *testing.T) { if err := os.WriteFile(checkpoint, []byte("secret checkpoint"), 0o600); err != nil { t.Fatal(err) } + if err := safeio.ProtectPrivateRegular(checkpoint); err != nil { + t.Fatal(err) + } if err := cleanupRecoveryCheckpoint(checkpoint); err != nil { t.Fatal(err) } diff --git a/tools/tht/internal/lifecycle/lock.go b/tools/tht/internal/lifecycle/lock.go index 7d6299bb..ed3e6c2e 100644 --- a/tools/tht/internal/lifecycle/lock.go +++ b/tools/tht/internal/lifecycle/lock.go @@ -13,6 +13,7 @@ import ( "time" "github.com/aritmolab/thothii/tools/tht/internal/config" + "github.com/aritmolab/thothii/tools/tht/internal/safeio" ) var ( @@ -49,16 +50,19 @@ type Transaction struct { // Acquire obtains the shared lock used by backup, restore, Pi lifecycle and product updates. func Acquire(installation config.Installation) (*Lock, error) { directory := installation.ControlDirectory() - if err := os.MkdirAll(directory, 0o700); err != nil { - return nil, fmt.Errorf("create lifecycle control directory: %w", err) + // The lifecycle directory is also the restore staging parent. Protect both the shared + // .tht directory and this installation's child before any lock or staging artifact is + // created; chmod alone does not install an owner-only DACL on Windows. + if err := ensurePrivateLifecycleDirectory(filepath.Dir(directory)); err != nil { + return nil, fmt.Errorf("protect lifecycle control parent: %w", err) + } + if err := ensurePrivateLifecycleDirectory(directory); err != nil { + return nil, fmt.Errorf("protect lifecycle control directory: %w", err) } info, err := os.Lstat(directory) if err != nil || !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { return nil, errors.New("lifecycle control directory is not a regular directory") } - if err := os.Chmod(directory, 0o700); err != nil { - return nil, fmt.Errorf("protect lifecycle control directory: %w", err) - } tokenBytes := make([]byte, 16) if _, err := rand.Read(tokenBytes); err != nil { @@ -66,11 +70,11 @@ func Acquire(installation config.Installation) (*Lock, error) { } token := hex.EncodeToString(tokenBytes) path := filepath.Join(directory, lockFileName) - file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) - if errors.Is(err, os.ErrExist) { - return nil, ErrLocked - } + file, err := safeio.CreateCanonicalNewPrivateFile(path) if err != nil { + if _, statErr := os.Lstat(path); statErr == nil { + return nil, ErrLocked + } return nil, fmt.Errorf("acquire lifecycle lock: %w", err) } value := owner{Token: token, PID: os.Getpid(), CreatedAt: time.Now().UTC()} @@ -86,6 +90,25 @@ func Acquire(installation config.Installation) (*Lock, error) { return &Lock{path: path, token: token}, nil } +// ensurePrivateLifecycleDirectory repairs an existing directory's protection or creates the +// final missing component with the platform's owner-only primitive. It deliberately does not +// use os.MkdirAll for the security-sensitive path: safeio validates every canonical ancestor. +func ensurePrivateLifecycleDirectory(path string) error { + info, err := os.Lstat(path) + if errors.Is(err, os.ErrNotExist) { + if err := safeio.EnsurePrivateDirectory(path); err != nil { + return err + } + } else if err != nil || !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + return safeio.ErrUnsafeFile + } else { + if err := safeio.ProtectPrivateDirectory(path); err != nil { + return err + } + } + return safeio.ValidatePrivateDirectory(path) +} + // AcquireTransaction obtains a lifecycle lock and returns the capability required by callers // that perform nested work inside the same non-reentrant transaction. func AcquireTransaction(installation config.Installation) (*Transaction, error) { diff --git a/tools/tht/internal/lifecycle/lock_test.go b/tools/tht/internal/lifecycle/lock_test.go index 2fb4babc..d8eaaa89 100644 --- a/tools/tht/internal/lifecycle/lock_test.go +++ b/tools/tht/internal/lifecycle/lock_test.go @@ -7,15 +7,37 @@ import ( "testing" "github.com/aritmolab/thothii/tools/tht/internal/config" + "github.com/aritmolab/thothii/tools/tht/internal/safeio" ) func TestLifecycleLockIsExclusivePerInstallationAndReusableAfterRelease(t *testing.T) { - installation := config.Installation{ProjectDirectory: t.TempDir(), Path: filepath.Join(t.TempDir(), "thothii-installation.yaml")} + root := lifecycleTestRoot(t) + installation := config.Installation{ProjectDirectory: root, Path: filepath.Join(root, "thothii-installation.yaml")} + if err := os.MkdirAll(installation.ControlDirectory(), 0o755); err != nil { + t.Fatal(err) + } first, err := Acquire(installation) if err != nil { t.Fatal(err) } t.Cleanup(func() { _ = first.Release() }) + for name, path := range map[string]string{ + "lifecycle parent": filepath.Dir(installation.ControlDirectory()), + "lifecycle directory": installation.ControlDirectory(), + "owner file": first.Path(), + } { + t.Run(name, func(t *testing.T) { + var err error + if name == "owner file" { + err = safeio.ValidatePrivateRegular(path) + } else { + err = safeio.ValidatePrivateDirectory(path) + } + if err != nil { + t.Fatalf("%s protection = %v", name, err) + } + }) + } if _, err := Acquire(installation); !errors.Is(err, ErrLocked) { t.Fatalf("second Acquire() error = %v, want ErrLocked", err) @@ -33,7 +55,8 @@ func TestLifecycleLockIsExclusivePerInstallationAndReusableAfterRelease(t *testi } func TestLifecycleLockReleaseDoesNotRemoveAnotherOwnersFile(t *testing.T) { - installation := config.Installation{ProjectDirectory: t.TempDir(), Path: filepath.Join(t.TempDir(), "thothii-installation.yaml")} + root := lifecycleTestRoot(t) + installation := config.Installation{ProjectDirectory: root, Path: filepath.Join(root, "thothii-installation.yaml")} lock, err := Acquire(installation) if err != nil { t.Fatal(err) @@ -50,7 +73,7 @@ func TestLifecycleLockReleaseDoesNotRemoveAnotherOwnersFile(t *testing.T) { } func TestTransactionCapabilityIsInstallationBoundAndExpiresOnRelease(t *testing.T) { - root := t.TempDir() + root := lifecycleTestRoot(t) installation := config.Installation{ProjectDirectory: root, Path: filepath.Join(root, "thothii-installation.yaml")} transaction, err := AcquireTransaction(installation) if err != nil { @@ -60,7 +83,7 @@ func TestTransactionCapabilityIsInstallationBoundAndExpiresOnRelease(t *testing. t.Fatalf("Verify() active capability error = %v", err) } - otherRoot := t.TempDir() + otherRoot := lifecycleTestRoot(t) other := config.Installation{ProjectDirectory: otherRoot, Path: filepath.Join(otherRoot, "thothii-installation.yaml")} if err := transaction.Verify(other); !errors.Is(err, ErrTransactionInstallation) { t.Fatalf("Verify() for another installation error = %v, want ErrTransactionInstallation", err) @@ -72,3 +95,12 @@ func TestTransactionCapabilityIsInstallationBoundAndExpiresOnRelease(t *testing. t.Fatalf("Verify() after Release() error = %v, want ErrTransactionInactive", err) } } + +func lifecycleTestRoot(t *testing.T) string { + t.Helper() + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + return root +}