fix(auth): harden restore verification transaction

This commit is contained in:
2026-08-18 00:58:12 +02:00
parent 0651f3316f
commit dee17893b4
10 changed files with 698 additions and 115 deletions
+28
View File
@@ -104,20 +104,43 @@ func restoreWithDependencies(ctx context.Context, installation config.Installati
return result, err
}
mutated := false
maintenanceAttempted := false
stopAttempted := false
defer func() {
if resultErr != nil && mutated {
if recoveryErr := deps.recover(context.Background(), installation, recovery, wasRunning); recoveryErr != nil {
resultErr = errors.Join(resultErr, fmt.Errorf("restore recovery checkpoint: %w", recoveryErr))
} else {
stopAttempted = false
}
}
if maintenanceAttempted {
cleanupContext := context.Background()
if wasRunning && stopAttempted {
if startErr := composeStartAndVerify(cleanupContext, installation, deps.runner); startErr != nil {
resultErr = errors.Join(resultErr, fmt.Errorf("restore maintenance cleanup restart: %w", startErr))
} else {
stopAttempted = false
}
}
if deactivateErr := maintenance(cleanupContext, installation, deps.runner, false); deactivateErr != nil {
resultErr = errors.Join(resultErr, fmt.Errorf("restore maintenance cleanup: %w", deactivateErr))
}
}
}()
if wasRunning {
// The activation command may take effect even when its response is lost. Track the attempt,
// not merely a successful return, so every subsequent path removes the admissions barrier.
maintenanceAttempted = true
if err := maintenance(ctx, installation, deps.runner, true); err != nil {
return result, err
}
if err := waitForNoActiveSessions(ctx, installation, deps.runner, request.Drain, deps.sleep); err != nil {
return result, err
}
// Compose may stop the core and then lose its response. Cleanup must therefore restart after
// any stop attempt, including a command that returns an error.
stopAttempted = true
if err := runCompose(ctx, installation, deps.runner, "stop"); err != nil {
return result, err
}
@@ -133,7 +156,12 @@ func restoreWithDependencies(ctx context.Context, installation config.Installati
if err := composeStartAndVerify(ctx, installation, deps.runner); err != nil {
return result, err
}
stopAttempted = false
result.Restarted = true
if err := maintenance(ctx, installation, deps.runner, false); err != nil {
return result, err
}
maintenanceAttempted = false
}
for _, name := range []string{"health", "doctor", "pi", "workspace"} {
check := deps.verify[name]
@@ -3,57 +3,45 @@
package backup
import (
"errors"
"os"
"path/filepath"
"github.com/aritmolab/thothii/tools/tht/internal/safeio"
"golang.org/x/sys/windows"
)
func replaceRestoreFile(target string, contents []byte, mode os.FileMode) error {
if safeio.ValidateCanonicalPath(target) != nil || mode&os.ModeType != 0 || mode.Perm() == 0 {
// replaceRestoreFile deliberately supports only owner-private Windows parents. The retained
// native directory handle pins every ancestor, rejects reparse components, creates an owner-only
// temporary file, and publishes it by an NT RootDirectory-relative atomic rename. Installations
// whose restore targets do not satisfy that custody contract fail closed instead of falling back
// to a path-based replacement.
func replaceRestoreFile(target string, contents []byte, mode os.FileMode) (resultErr error) {
if safeio.ValidateCanonicalPath(target) != nil || mode&os.ModeType != 0 || mode.Perm() == 0 || len(contents) == 0 {
return safeio.ErrUnsafeFile
}
parent := filepath.Dir(target)
resolved, err := filepath.EvalSymlinks(parent)
if err != nil || resolved != parent || !safeWindowsRestoreTarget(target) {
parent, found, err := safeio.OpenPrivateDirectory(filepath.Dir(target), false)
if err != nil || !found || parent == nil {
return safeio.ErrUnsafeFile
}
temporary, err := os.CreateTemp(parent, ".tht-restore-*.tmp")
defer func() {
if closeErr := parent.Close(); closeErr != nil {
resultErr = safeio.ErrUnsafeFile
}
}()
safeio.NotifyPrivateDirectoryTestHookForTest("after-restore-parent-open")
if parent.Validate() != nil {
return safeio.ErrUnsafeFile
}
created, err := parent.CreateRegular(filepath.Base(target), contents)
if err != nil {
return safeio.ErrUnsafeFile
}
temporaryPath := temporary.Name()
defer os.Remove(temporaryPath)
if err := temporary.Chmod(mode.Perm()); err == nil {
_, err = temporary.Write(contents)
if !created {
if err := parent.ReplaceRegular(filepath.Base(target), contents); err != nil {
return safeio.ErrUnsafeFile
}
}
if err == nil {
err = temporary.Sync()
}
closeErr := temporary.Close()
if err == nil {
err = closeErr
}
if err != nil || !safeWindowsRestoreTarget(target) {
return safeio.ErrUnsafeFile
}
from, fromErr := windows.UTF16PtrFromString(temporaryPath)
to, toErr := windows.UTF16PtrFromString(target)
if fromErr != nil || toErr != nil {
return safeio.ErrUnsafeFile
}
if err := windows.MoveFileEx(from, to, windows.MOVEFILE_REPLACE_EXISTING|windows.MOVEFILE_WRITE_THROUGH); err != nil {
if parent.Validate() != nil {
return safeio.ErrUnsafeFile
}
return nil
}
func safeWindowsRestoreTarget(target string) bool {
info, err := os.Lstat(target)
if errors.Is(err, os.ErrNotExist) {
return true
}
return err == nil && info.Mode().IsRegular() && info.Mode()&os.ModeSymlink == 0
}
@@ -0,0 +1,31 @@
package backup
import (
"os"
"path/filepath"
"runtime"
"strings"
"testing"
)
func TestWindowsRestoreReplacementUsesPinnedHandleRelativeSafeIO(t *testing.T) {
_, current, _, ok := runtime.Caller(0)
if !ok {
t.Fatal("cannot locate Windows restore implementation")
}
contents, err := os.ReadFile(filepath.Join(filepath.Dir(current), "restore_file_windows.go"))
if err != nil {
t.Fatal(err)
}
source := string(contents)
for _, forbidden := range []string{"EvalSymlinks", "MoveFileEx", "CreateTemp"} {
if strings.Contains(source, forbidden) {
t.Fatalf("Windows restore replacement retains forbidden path-based primitive %q", forbidden)
}
}
for _, required := range []string{"OpenPrivateDirectory", "CreateRegular", "ReplaceRegular", "after-restore-parent-open"} {
if !strings.Contains(source, required) {
t.Fatalf("Windows restore replacement omits pinned safe-I/O primitive %q", required)
}
}
}
@@ -0,0 +1,83 @@
//go:build windows
package backup
import (
"errors"
"os"
"path/filepath"
"testing"
"github.com/aritmolab/thothii/tools/tht/internal/safeio"
)
func TestReplaceRestoreFileWindowsCreatesAndReplacesOwnerOnlyRegular(t *testing.T) {
parent := filepath.Join(t.TempDir(), "private")
if err := os.Mkdir(parent, 0o700); err != nil {
t.Fatal(err)
}
if err := safeio.ProtectPrivateDirectory(parent); err != nil {
t.Fatal(err)
}
target := filepath.Join(parent, "auth.yaml")
if err := replaceRestoreFile(target, []byte("created"), 0o600); err != nil {
t.Fatal(err)
}
if err := safeio.ValidatePrivateRegular(target); err != nil {
t.Fatalf("created restore file is not owner-only: %v", err)
}
if err := replaceRestoreFile(target, []byte("replacement"), 0o600); err != nil {
t.Fatal(err)
}
if err := safeio.ValidatePrivateRegular(target); err != nil {
t.Fatalf("replacement restore file is not owner-only: %v", err)
}
contents, err := os.ReadFile(target)
if err != nil || string(contents) != "replacement" {
t.Fatalf("replacement contents = %q, error = %v", contents, err)
}
}
func TestReplaceRestoreFileWindowsPinsParentAndRejectsReparsePath(t *testing.T) {
root := t.TempDir()
parent := filepath.Join(root, "private")
if err := os.Mkdir(parent, 0o700); err != nil {
t.Fatal(err)
}
if err := safeio.ProtectPrivateDirectory(parent); err != nil {
t.Fatal(err)
}
target := filepath.Join(parent, "users.yaml")
if err := os.WriteFile(target, []byte("old"), 0o600); err != nil {
t.Fatal(err)
}
if err := safeio.ProtectPrivateRegular(target); err != nil {
t.Fatal(err)
}
renamed := parent + "-renamed"
var hookCalled bool
restoreHook := safeio.SetPrivateDirectoryTestHookForTest(func(stage string) {
if stage != "after-restore-parent-open" {
return
}
hookCalled = true
if err := os.Rename(parent, renamed); err == nil {
t.Fatal("parent rename succeeded while restore retained its directory handles")
}
})
defer restoreHook()
if err := replaceRestoreFile(target, []byte("new"), 0o600); err != nil {
t.Fatal(err)
}
if !hookCalled {
t.Fatal("parent-pinning race hook was not reached")
}
link := filepath.Join(root, "reparse-parent")
if err := os.Symlink(parent, link); err != nil {
t.Skipf("Windows host does not permit symlink creation: %v", err)
}
if err := replaceRestoreFile(filepath.Join(link, "users.yaml"), []byte("unsafe"), 0o600); !errors.Is(err, safeio.ErrUnsafeFile) {
t.Fatalf("reparse-parent restore error = %v, want ErrUnsafeFile", err)
}
}
+6 -3
View File
@@ -11,6 +11,7 @@ import (
"io"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
"time"
@@ -420,9 +421,10 @@ func verifyRestoreWorkspace(ctx context.Context, installation config.Installatio
return errors.New("restored workspace registry returned an invalid result")
}
var payload struct {
Ready bool `json:"ready"`
State string `json:"state"`
Workspaces int `json:"workspaces"`
Ready bool `json:"ready"`
State string `json:"state"`
Workspaces int `json:"workspaces"`
Fingerprint string `json:"fingerprint"`
}
decoder := json.NewDecoder(strings.NewReader(result.Stdout))
decoder.DisallowUnknownFields()
@@ -434,6 +436,7 @@ func verifyRestoreWorkspace(ctx context.Context, installation config.Installatio
return errors.New("restored workspace registry returned an invalid result")
}
if !payload.Ready || payload.Workspaces < 0 ||
!regexp.MustCompile(`^sha256:[0-9a-f]{64}$`).MatchString(payload.Fingerprint) ||
(payload.State != "active" && payload.State != "uninitialized") ||
(payload.State == "uninitialized" && payload.Workspaces != 0) {
return errors.New("restored workspace registry did not pass integrity validation")
+145 -9
View File
@@ -466,8 +466,109 @@ func TestRestoreRefusesActiveSessionsWithoutDrain(t *testing.T) {
if !errors.Is(err, ErrActiveSessions) {
t.Fatalf("restore error = %v, want active-session refusal", err)
}
if restoredFiles != 0 || runner.stopCount != 0 || runner.startCount != 0 || !runner.running {
t.Fatalf("active-session refusal mutated target: files=%d stops=%d starts=%d running=%t", restoredFiles, runner.stopCount, runner.startCount, runner.running)
if restoredFiles != 0 || runner.stopCount != 0 || runner.startCount != 0 || !runner.running || runner.maintenance {
t.Fatalf("active-session refusal changed lifecycle state: files=%d stops=%d starts=%d running=%t maintenance=%t", restoredFiles, runner.stopCount, runner.startCount, runner.running, runner.maintenance)
}
}
func TestRestoreCleansMaintenanceAfterActivationFailure(t *testing.T) {
installation := preflightTestInstallation(t)
backing := newBackupRunner(installation, true)
activationErr := errors.New("activation response lost")
runner := &restoreFailureRunner{
fakeBackupRunner: backing,
failContains: "operator-command.js maintenance-activate",
err: activationErr,
beforeFailure: func() { backing.maintenance = true },
}
deps := restoreTestDependencies(t, runner)
_, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps)
if !errors.Is(err, activationErr) {
t.Fatalf("restore error = %v, want activation failure", err)
}
if backing.maintenance || !backing.running || runner.matchCount != 1 {
t.Fatalf("activation cleanup state: maintenance=%t running=%t matches=%d", backing.maintenance, backing.running, runner.matchCount)
}
}
func TestRestoreRestartsAndCleansMaintenanceAfterStopFailure(t *testing.T) {
installation := preflightTestInstallation(t)
backing := newBackupRunner(installation, true)
stopErr := errors.New("stop response lost")
runner := &restoreFailureRunner{
fakeBackupRunner: backing,
failSuffix: " stop",
err: stopErr,
beforeFailure: func() {
backing.stopCount++
backing.running, backing.coreRunning = false, false
},
}
deps := restoreTestDependencies(t, runner)
_, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps)
if !errors.Is(err, stopErr) {
t.Fatalf("restore error = %v, want stop failure", err)
}
if backing.maintenance || !backing.running || backing.startCount != 1 {
t.Fatalf("stop cleanup state: maintenance=%t running=%t starts=%d", backing.maintenance, backing.running, backing.startCount)
}
}
func TestRestoreCleansMaintenanceAfterMutationAndRollbackFailures(t *testing.T) {
installation := preflightTestInstallation(t)
for _, test := range []struct {
name string
recoveryErr error
}{
{name: "mutation"},
{name: "rollback", recoveryErr: errors.New("rollback failed")},
} {
t.Run(test.name, func(t *testing.T) {
backing := newBackupRunner(installation, true)
deps := restoreTestDependencies(t, backing)
mutationErr := errors.New("mutation failed")
deps.restoreFile = func(context.Context, config.Installation, ArchiveEntryMetadata, io.Reader) error {
return mutationErr
}
deps.recover = func(context.Context, config.Installation, PreflightResult, bool) error {
if test.recoveryErr == nil {
backing.running, backing.coreRunning = true, true
}
return test.recoveryErr
}
_, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps)
if !errors.Is(err, mutationErr) || (test.recoveryErr != nil && !errors.Is(err, test.recoveryErr)) {
t.Fatalf("restore error = %v, want mutation and rollback failures", err)
}
if backing.maintenance || !backing.running {
t.Fatalf("failure cleanup state: maintenance=%t running=%t", backing.maintenance, backing.running)
}
})
}
}
func TestRestorePreservesDrainAndMaintenanceCleanupFailures(t *testing.T) {
installation := preflightTestInstallation(t)
backing := newBackupRunner(installation, true)
backing.sessionResponses = []string{`[{"status":"running","archived":false}]`}
cleanupErr := errors.New("maintenance cleanup failed")
runner := &restoreFailureRunner{
fakeBackupRunner: backing,
failContains: "operator-command.js maintenance-deactivate",
err: cleanupErr,
beforeFailure: func() { backing.maintenance = false },
}
deps := restoreTestDependencies(t, runner)
_, err := restoreWithDependencies(context.Background(), installation, RestoreRequest{Archive: restoreArchive(t), Confirm: true}, deps)
if !errors.Is(err, ErrActiveSessions) || !errors.Is(err, cleanupErr) {
t.Fatalf("restore error = %v, want drain and cleanup failures", err)
}
if backing.maintenance || !backing.running {
t.Fatalf("cleanup failure state: maintenance=%t running=%t", backing.maintenance, backing.running)
}
}
@@ -491,6 +592,37 @@ func (runner failStartRestoreRunner) Run(ctx context.Context, args []string, std
return runner.fakeBackupRunner.Run(ctx, args, stdin)
}
type restoreFailureRunner struct {
*fakeBackupRunner
failContains string
failSuffix string
err error
beforeFailure func()
matchCount int
}
func (runner *restoreFailureRunner) Run(ctx context.Context, args []string, stdin io.Reader) (compose.Result, error) {
command := strings.Join(args, " ")
matches := runner.failContains != "" && strings.Contains(command, runner.failContains)
matches = matches || runner.failSuffix != "" && strings.HasSuffix(command, runner.failSuffix)
if matches {
runner.matchCount++
if runner.beforeFailure != nil {
runner.beforeFailure()
}
return compose.Result{}, runner.err
}
return runner.fakeBackupRunner.Run(ctx, args, stdin)
}
func (runner *restoreFailureRunner) Stream(ctx context.Context, args []string, stdin io.Reader, stdout io.Writer) (compose.Result, error) {
return runner.fakeBackupRunner.Stream(ctx, args, stdin, stdout)
}
func (runner *restoreFailureRunner) SessionInventoryScope() string {
return runner.fakeBackupRunner.SessionInventoryScope()
}
type fakeRestoreLock struct {
release func()
}
@@ -542,14 +674,15 @@ func (*workspaceVerificationRunner) SessionInventoryScope() string { return "min
func TestVerifyRestoreWorkspaceUsesFixedNonNetworkOperatorPath(t *testing.T) {
installation := preflightTestInstallation(t)
fingerprint := "sha256:" + strings.Repeat("a", 64)
for _, test := range []struct {
name string
running bool
payload string
prefix string
}{
{name: "stopped uninitialized", payload: `{"ready":true,"state":"uninitialized","workspaces":0}`, prefix: "run --rm --no-deps --no-TTY core"},
{name: "running active", running: true, payload: `{"ready":true,"state":"active","workspaces":1}`, prefix: "exec -T core"},
{name: "stopped uninitialized", payload: fmt.Sprintf(`{"ready":true,"state":"uninitialized","workspaces":0,"fingerprint":%q}`, fingerprint), prefix: "run --rm --no-deps --no-TTY core"},
{name: "running active", running: true, payload: fmt.Sprintf(`{"ready":true,"state":"active","workspaces":1,"fingerprint":%q}`, fingerprint), prefix: "exec -T core"},
} {
t.Run(test.name, func(t *testing.T) {
runner := &workspaceVerificationRunner{
@@ -573,6 +706,7 @@ func TestVerifyRestoreWorkspaceUsesFixedNonNetworkOperatorPath(t *testing.T) {
func TestVerifyRestoreWorkspaceRejectsInvalidOperatorResults(t *testing.T) {
installation := preflightTestInstallation(t)
valid := `{"ready":true,"state":"active","workspaces":1,"fingerprint":"sha256:` + strings.Repeat("a", 64) + `"}`
for _, test := range []struct {
name string
result compose.Result
@@ -580,11 +714,13 @@ func TestVerifyRestoreWorkspaceRejectsInvalidOperatorResults(t *testing.T) {
}{
{name: "empty"},
{name: "malformed", result: compose.Result{Stdout: `{malformed`}},
{name: "trailing document", result: compose.Result{Stdout: `{"ready":true,"state":"active","workspaces":1}{}`}},
{name: "unknown field", result: compose.Result{Stdout: `{"ready":true,"state":"active","workspaces":1,"detail":"unsafe"}`}},
{name: "not ready", result: compose.Result{Stdout: `{"ready":false,"state":"uninitialized","workspaces":0}`}},
{name: "unknown state", result: compose.Result{Stdout: `{"ready":true,"state":"unknown","workspaces":0}`}},
{name: "inconsistent count", result: compose.Result{Stdout: `{"ready":true,"state":"uninitialized","workspaces":1}`}},
{name: "trailing document", result: compose.Result{Stdout: valid + `{}`}},
{name: "unknown field", result: compose.Result{Stdout: strings.TrimSuffix(valid, "}") + `,"detail":"unsafe"}`}},
{name: "not ready", result: compose.Result{Stdout: strings.Replace(valid, `"ready":true`, `"ready":false`, 1)}},
{name: "unknown state", result: compose.Result{Stdout: strings.Replace(valid, `"state":"active"`, `"state":"unknown"`, 1)}},
{name: "inconsistent count", result: compose.Result{Stdout: strings.Replace(valid, `"state":"active"`, `"state":"uninitialized"`, 1)}},
{name: "missing fingerprint", result: compose.Result{Stdout: `{"ready":true,"state":"active","workspaces":1}`}},
{name: "malformed fingerprint", result: compose.Result{Stdout: `{"ready":true,"state":"active","workspaces":1,"fingerprint":"sha256:not-a-digest"}`}},
{name: "nonzero", result: compose.Result{ExitCode: 2}, err: errors.New("exit status 2")},
} {
t.Run(test.name, func(t *testing.T) {