diff --git a/docs/internal/command-restore.md b/docs/internal/command-restore.md index 331e495..a73dd79 100644 --- a/docs/internal/command-restore.md +++ b/docs/internal/command-restore.md @@ -62,6 +62,9 @@ Execution order and safety: - manifest replacement is validated before rename; - each committed object is verified against its declared checksum, size, and generation before installation; +- installation reopens the selected session root, creates only validated + missing directory components with workspace modes, and rejects symlinked or + non-regular ancestors and leaves before replacement; - a committed manifest already verified during discovery is retained for the matching restore action and revalidated before installation, avoiding a second body transfer; diff --git a/docs/internal/workspace.md b/docs/internal/workspace.md index 5872f69..8989cdf 100644 --- a/docs/internal/workspace.md +++ b/docs/internal/workspace.md @@ -74,6 +74,8 @@ syntax and exact deletion scope belong in [CLI](../cli.md#clean) and - campaign-aware session root is mandatory. - manifest-driven stage state is durable across runs. - cleanup guardrails prevent destructive root/out-of-scope deletion. +- restore installation is confined to validated relative paths beneath the + selected session root. - ordinary workspace paths retain group-writable directory and file modes across nested creation, replacement, and Notarius promotion. diff --git a/docs/operations.md b/docs/operations.md index b85a265..cffb218 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -451,6 +451,10 @@ Restore treats a local leaf symlink or any symlinked ancestor beneath the selected session root as a conflict, even when the linked content matches the selected remote object. `--force` does not override that conflict. +Before each install, restore validates the destination again beneath the +selected session root. A target changed into a symlink, directory, or other +non-regular entry is not overwritten. + Restore writes an execution report at `reports/restore-latest.json`. If restore fails after beginning installation, it leaves a durable diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index cdbf3cd..260d719 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -470,7 +470,7 @@ symlink and classify its target as an already matching regular file. ## Stage 6 — Install Restore Results Through The Session Root -**Status: Pending** +**Status: Completed** ### Goal diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index e2af00f..5dd90b1 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -564,6 +564,9 @@ Safe fix: - rerun with `--force` only when remote state should overwrite an eligible regular local file; repair a symlink or other non-regular conflict first. +If the filesystem changed after the plan was printed, rerun restore after +repairing the destination; installation performs the same safety check again. + Relevant reference: [Operations: Restore Workflow](./operations.md#restore-workflow). ## Restore current-state discovery failure diff --git a/internal/app/restore_execute.go b/internal/app/restore_execute.go index 39cd839..2073995 100644 --- a/internal/app/restore_execute.go +++ b/internal/app/restore_execute.go @@ -5,6 +5,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "errors" "fmt" "io" "os" @@ -46,6 +47,14 @@ func executeRestorePlan( } sessionRoot := artifacts.SessionWorkDirForCampaign(cfg.Pipeline.Workspace.Root, cfg.Session.Campaign, cfg.Session.SessionID) + if err := fileops.EnsureWorkspaceDirectory(sessionRoot); err != nil { + return nil, fmt.Errorf("create session root for restore: %w", err) + } + root, err := os.OpenRoot(sessionRoot) + if err != nil { + return nil, fmt.Errorf("open session root for restore: %w", err) + } + defer func() { _ = root.Close() }() manifestActions := make([]RestoreAction, 0, 1) actions := make([]RestoreAction, 0, len(plan.Actions)) for _, action := range plan.Actions { @@ -67,7 +76,7 @@ func executeRestorePlan( result := &RestoreExecutionResult{} for _, action := range actions { - if err := executeRestoreDownloadAction(ctx, cfg, sessionRoot, current, action, store); err != nil { + if err := executeRestoreDownloadAction(ctx, cfg, sessionRoot, root, current, action, store); err != nil { if report != nil { report.markFailed(action, err) } @@ -86,6 +95,7 @@ func executeRestoreDownloadAction( ctx context.Context, cfg *config.Config, sessionRoot string, + root *os.Root, current *RemoteCurrentState, action RestoreAction, store storage.ObjectStore, @@ -97,6 +107,14 @@ func executeRestoreDownloadAction( if strings.TrimSpace(action.LocalPath) != "" && filepath.Clean(action.LocalPath) != safeLocalPath { return fmt.Errorf("restore plan local path mismatch for %q", action.LocalRelativePath) } + parent, leaf, err := openRestoreDestinationParent(root, action.LocalRelativePath) + if err != nil { + return err + } + defer func() { _ = parent.Close() }() + if err := validateRestoreDestinationLeaf(parent, leaf); err != nil { + return err + } if restoreActionIsAudio(action) { return executeRestoreAudioAction(ctx, cfg, safeLocalPath, action, store) @@ -135,12 +153,18 @@ func executeRestoreDownloadAction( if closeErr != nil { return fmt.Errorf("close restored manifest: %w", closeErr) } + if err := validateRestoreDestinationLeaf(parent, leaf); err != nil { + return err + } if err := (&manifest.LocalStore{}).Save(ctx, safeLocalPath, restored); err != nil { return fmt.Errorf("install rebased manifest atomically: %w", err) } return nil } + if err := validateRestoreDestinationLeaf(parent, leaf); err != nil { + return err + } if err := fileops.InstallSiblingTempFile(temporaryPath, safeLocalPath, fileops.WorkspaceFileMode); err != nil { return fmt.Errorf("install file atomically: %w", err) } @@ -148,6 +172,71 @@ func executeRestoreDownloadAction( return nil } +func openRestoreDestinationParent(root *os.Root, relative string) (*os.Root, string, error) { + if root == nil { + return nil, "", fmt.Errorf("restore session root is required") + } + name := filepath.Clean(filepath.FromSlash(strings.TrimSpace(relative))) + if name == "." || name == ".." || filepath.IsAbs(name) || strings.HasPrefix(name, ".."+string(filepath.Separator)) { + return nil, "", fmt.Errorf("restore path %q is not a normalized relative path", relative) + } + parts := strings.Split(name, string(filepath.Separator)) + current := root + for _, part := range parts[:len(parts)-1] { + info, err := current.Lstat(part) + if errors.Is(err, os.ErrNotExist) { + if err := current.Mkdir(part, fileops.WorkspaceDirectoryMode.Perm()); err != nil && !errors.Is(err, os.ErrExist) { + if current != root { + _ = current.Close() + } + return nil, "", fmt.Errorf("create restore destination directory %q: %w", part, err) + } + info, err = current.Lstat(part) + } + if err != nil { + if current != root { + _ = current.Close() + } + return nil, "", fmt.Errorf("inspect restore destination ancestor %q: %w", part, err) + } + if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + if current != root { + _ = current.Close() + } + return nil, "", fmt.Errorf("restore destination ancestor %q is not a directory", part) + } + if err := current.Chmod(part, fileops.WorkspaceDirectoryMode); err != nil { + if current != root { + _ = current.Close() + } + return nil, "", fmt.Errorf("set restore destination directory permissions %q: %w", part, err) + } + next, err := current.OpenRoot(part) + if current != root { + _ = current.Close() + } + if err != nil { + return nil, "", fmt.Errorf("open restore destination ancestor %q: %w", part, err) + } + current = next + } + return current, parts[len(parts)-1], nil +} + +func validateRestoreDestinationLeaf(parent *os.Root, leaf string) error { + info, err := parent.Lstat(leaf) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("inspect restore destination %q: %w", leaf, err) + } + if !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("restore destination %q is not an eligible regular file", leaf) + } + return nil +} + func isRestoreManifest(relativePath string) bool { clean := filepath.ToSlash(filepath.Clean(strings.TrimSpace(relativePath))) return clean == config.PathManifestFile || clean == config.PathPreviousDirSegment+"/"+config.PathManifestFile diff --git a/internal/app/restore_execution_test.go b/internal/app/restore_execution_test.go index 6e9f86c..a5866d2 100644 --- a/internal/app/restore_execution_test.go +++ b/internal/app/restore_execution_test.go @@ -524,6 +524,36 @@ func TestExecuteRestorePlanPathMismatchFails(t *testing.T) { } } +func TestExecuteRestorePlanRejectsAncestorSymlinkCreatedAfterPlanning(t *testing.T) { + cfg := restorePlanConfig(t) + current := restorePlanCurrentState(t, cfg) + store := &storage.FakeBackend{} + seedRestoreObject(store, current.CurrentManifestKey, []byte(`{"session_id":"2026-05-03"}`)) + seedRestoreObject(store, current.SessionPrefix+"artifacts/session_recap.md", []byte("remote-content\n")) + plan, err := buildRestorePlan(context.Background(), cfg, current, store, RestorePlanOptions{Force: true}) + if err != nil { + t.Fatalf("buildRestorePlan() error = %v", err) + } + + sessionRoot := artifacts.SessionWorkDirForCampaign(cfg.Pipeline.Workspace.Root, cfg.Session.Campaign, cfg.Session.SessionID) + outside := t.TempDir() + if err := os.Symlink(outside, filepath.Join(sessionRoot, "artifacts")); err != nil { + t.Skipf("Symlink() unavailable: %v", err) + } + + report, err := newRestoreReport(current, plan, RestorePlanOptions{Force: true}) + if err != nil { + t.Fatalf("newRestoreReport() error = %v", err) + } + _, err = executeRestorePlan(context.Background(), cfg, current, plan, report, store) + if err == nil || !strings.Contains(err.Error(), "not a directory") { + t.Fatalf("executeRestorePlan() error = %v, want ancestor conflict", err) + } + if _, err := os.Stat(filepath.Join(outside, "session_recap.md")); !os.IsNotExist(err) { + t.Fatalf("outside target was changed: %v", err) + } +} + func mustReadRestoreReport(t *testing.T, path string) *RestoreReport { t.Helper() data, err := os.ReadFile(path)