diff --git a/docs/internal/command-restore.md b/docs/internal/command-restore.md index f22923a..331e495 100644 --- a/docs/internal/command-restore.md +++ b/docs/internal/command-restore.md @@ -37,8 +37,12 @@ Planner behavior: generation. Coherent legacy state remains on the isolated compatibility path; - remote-to-local mapping is traversal-safe; - actions are sorted by local relative path and then remote key; +- planning opens the selected session root and inspects every target by its + normalized session-relative path. Missing ancestors produce a missing target; + an ancestor or leaf symlink is a non-regular conflict and is never opened for + hashing; - force converts differing eligible regular files from conflicts to downloads; - directories and other non-regular targets remain conflicts. + directories, symlinks, and other non-regular targets remain conflicts. For a non-dry-run restore, planning/classification happens only after acquiring the session lock. Runner manifest/reuse checks acquire that same lock first. diff --git a/docs/operations.md b/docs/operations.md index e5a90e6..b85a265 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -445,7 +445,11 @@ Optional: - `--include-audio` to include `audio/**` - `--force` to overwrite eligible conflicting regular files; it never replaces - directories or other non-regular local targets + directories, symlinks, or other non-regular local targets + +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. Restore writes an execution report at `reports/restore-latest.json`. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 13346a6..cdbf3cd 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -416,7 +416,7 @@ recursive traversal, no-follow handle chains, and sorting machinery. ## Stage 5 — Make Restore Planning Symlink-Safe And Root-Relative -**Status: Pending** +**Status: Completed** ### Goal diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 43d3ac5..e2af00f 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -549,6 +549,8 @@ Symptom: Likely cause: - local durable files differ from remote restore sources. +- a restore target or one of its ancestors is a symlink or another non-regular + filesystem entry. Diagnostics: @@ -559,7 +561,8 @@ narratio session restore 2026-04-04 --dry-run Safe fix: - review conflicts; -- rerun with `--force` only when remote state should overwrite local. +- rerun with `--force` only when remote state should overwrite an eligible + regular local file; repair a symlink or other non-regular conflict first. Relevant reference: [Operations: Restore Workflow](./operations.md#restore-workflow). diff --git a/internal/app/restore_plan.go b/internal/app/restore_plan.go index 83a81c1..f4631fb 100644 --- a/internal/app/restore_plan.go +++ b/internal/app/restore_plan.go @@ -2,6 +2,8 @@ package app import ( "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "io" @@ -14,6 +16,7 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" + "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/pathsafe" "gitea.maximumdirect.net/eric/narratio/internal/previouscache" ) @@ -80,12 +83,21 @@ func buildRestorePlan(ctx context.Context, cfg *config.Config, current *RemoteCu return nil, fmt.Errorf("remote object store is required") } sessionPaths := artifacts.NewLocalStore(cfg.Pipeline.Workspace.Root).SessionPathsFor(cfg.Session.Campaign, cfg.Session.SessionID) - actions, err := buildCurrentRestoreActions(ctx, current, store, sessionPaths, opts) + if err := fileops.EnsureWorkspaceDirectory(sessionPaths.Root); err != nil { + return nil, fmt.Errorf("create session root for restore planning: %w", err) + } + sessionRoot, err := os.OpenRoot(sessionPaths.Root) + if err != nil { + return nil, fmt.Errorf("open session root for restore planning: %w", err) + } + defer func() { _ = sessionRoot.Close() }() + + actions, err := buildCurrentRestoreActions(ctx, current, store, sessionPaths, sessionRoot, opts) if err != nil { return nil, err } - previousActions, err := buildPreviousCacheRestoreActions(ctx, cfg, sessionPaths, store, opts.Force) + previousActions, err := buildPreviousCacheRestoreActions(ctx, cfg, sessionPaths, sessionRoot, store, opts.Force) if err != nil { return nil, err } @@ -119,12 +131,13 @@ func buildCurrentRestoreActions( current *RemoteCurrentState, store storage.ObjectStore, sessionPaths artifacts.SessionPaths, + sessionRoot *os.Root, opts RestorePlanOptions, ) ([]RestoreAction, error) { if current != nil && current.Commit != nil { - return buildCommittedRestoreActions(ctx, current, store, sessionPaths, opts) + return buildCommittedRestoreActions(ctx, current, store, sessionPaths, sessionRoot, opts) } - return buildLegacyRestoreActions(ctx, current, store, sessionPaths, opts) + return buildLegacyRestoreActions(ctx, current, store, sessionPaths, sessionRoot, opts) } func buildCommittedRestoreActions( @@ -132,6 +145,7 @@ func buildCommittedRestoreActions( current *RemoteCurrentState, store storage.ObjectStore, sessionPaths artifacts.SessionPaths, + sessionRoot *os.Root, opts RestorePlanOptions, ) ([]RestoreAction, error) { if current == nil || current.Commit == nil { @@ -156,7 +170,7 @@ func buildCommittedRestoreActions( } action, err := classifyRestoreAction(ctx, store, storage.ObjectInfo{ Key: artifact.DestinationKey, Size: artifact.Size, ETag: artifact.Generation, - }, artifact.SHA256, rel, localPath, opts.Force) + }, artifact.SHA256, rel, localPath, sessionRoot, opts.Force) if err != nil { return nil, fmt.Errorf("classify committed object %q: %w", artifact.DestinationKey, err) } @@ -173,6 +187,7 @@ func buildLegacyRestoreActions( current *RemoteCurrentState, store storage.ObjectStore, sessionPaths artifacts.SessionPaths, + sessionRoot *os.Root, opts RestorePlanOptions, ) ([]RestoreAction, error) { prefix := normalizeRemoteKey(current.SessionPrefix) @@ -212,7 +227,7 @@ func buildLegacyRestoreActions( if err != nil { return nil, fmt.Errorf("map remote key %q: %w", key, err) } - action, err := classifyRestoreAction(ctx, store, object, "", rel, localPath, opts.Force) + action, err := classifyRestoreAction(ctx, store, object, "", rel, localPath, sessionRoot, opts.Force) if err != nil { return nil, fmt.Errorf("classify remote key %q: %w", key, err) } @@ -337,6 +352,7 @@ func buildPreviousCacheRestoreActions( ctx context.Context, cfg *config.Config, sessionPaths artifacts.SessionPaths, + sessionRoot *os.Root, store storage.ObjectStore, force bool, ) ([]RestoreAction, error) { @@ -359,7 +375,7 @@ func buildPreviousCacheRestoreActions( for _, record := range plan.Records { action, err := classifyRestoreAction(ctx, store, storage.ObjectInfo{ Key: record.RemoteKey, Size: record.Size, ETag: record.Generation, - }, record.SHA256, record.LocalRelativePath, record.LocalPath, force) + }, record.SHA256, record.LocalRelativePath, record.LocalPath, sessionRoot, force) if err != nil { return nil, fmt.Errorf("classify previous-session cache object %q: %w", record.RemoteKey, err) } @@ -376,6 +392,7 @@ func classifyRestoreAction( expectedSHA256 string, localRelPath string, localPath string, + sessionRoot *os.Root, force bool, ) (RestoreAction, error) { action := RestoreAction{ @@ -390,33 +407,37 @@ func classifyRestoreAction( action.Generation = strings.TrimSpace(object.ETag) } - info, err := os.Stat(localPath) + target, err := inspectRestoreTarget(sessionRoot, localRelPath) if err != nil { - if os.IsNotExist(err) { - action.Kind = RestoreActionDownload - action.Reason = "local file missing" - return action, nil - } - return RestoreAction{}, fmt.Errorf("stat local file: %w", err) + return RestoreAction{}, err + } + if target.kind == restoreTargetMissing { + action.Kind = RestoreActionDownload + action.Reason = "local file missing" + return action, nil } - action.ExistsLocal = true - if info.IsDir() { + if target.kind == restoreTargetDirectory { action.Kind = RestoreActionConflict action.Conflict = true action.ConflictKind = RestoreConflictDirectory action.Reason = "local path is a directory" return action, nil } - if !info.Mode().IsRegular() { + if target.kind == restoreTargetNonRegular { action.Kind = RestoreActionConflict action.Conflict = true action.ConflictKind = RestoreConflictNonRegular action.Reason = "local path is not a regular file" return action, nil } + defer func() { _ = target.file.Close() }() + info, err := target.file.Stat() + if err != nil { + return RestoreAction{}, fmt.Errorf("stat opened local file: %w", err) + } if action.SHA256 != "" { - localDigest, err := artifacts.SHA256File(localPath) + localDigest, err := sha256OpenedFile(target.file) if err != nil { return RestoreAction{}, fmt.Errorf("checksum local file: %w", err) } @@ -464,7 +485,7 @@ func classifyRestoreAction( return action, nil } - localDigest, err := artifacts.SHA256File(localPath) + localDigest, err := sha256OpenedFile(target.file) if err != nil { return RestoreAction{}, fmt.Errorf("checksum local file: %w", err) } @@ -499,6 +520,86 @@ func classifyRestoreAction( return action, nil } +type restoreTargetKind uint8 + +const ( + restoreTargetMissing restoreTargetKind = iota + restoreTargetRegular + restoreTargetDirectory + restoreTargetNonRegular +) + +type restoreTarget struct { + kind restoreTargetKind + file *os.File +} + +func inspectRestoreTarget(root *os.Root, relative string) (restoreTarget, error) { + if root == nil { + return restoreTarget{}, 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 restoreTarget{}, fmt.Errorf("restore path %q is not a normalized relative path", relative) + } + parts := strings.Split(name, string(filepath.Separator)) + current := root + var opened []*os.Root + defer func() { + for _, directory := range opened { + _ = directory.Close() + } + }() + for _, part := range parts[:len(parts)-1] { + info, err := current.Lstat(part) + if errors.Is(err, os.ErrNotExist) { + return restoreTarget{kind: restoreTargetMissing}, nil + } + if err != nil { + return restoreTarget{}, fmt.Errorf("inspect restore ancestor %q: %w", part, err) + } + if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 { + return restoreTarget{kind: restoreTargetNonRegular}, nil + } + next, err := current.OpenRoot(part) + if err != nil { + return restoreTarget{}, fmt.Errorf("open restore ancestor %q: %w", part, err) + } + opened = append(opened, next) + current = next + } + leaf := parts[len(parts)-1] + info, err := current.Lstat(leaf) + if errors.Is(err, os.ErrNotExist) { + return restoreTarget{kind: restoreTargetMissing}, nil + } + if err != nil { + return restoreTarget{}, fmt.Errorf("inspect restore target %q: %w", relative, err) + } + if info.IsDir() { + return restoreTarget{kind: restoreTargetDirectory}, nil + } + if !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 { + return restoreTarget{kind: restoreTargetNonRegular}, nil + } + file, err := current.Open(leaf) + if err != nil { + return restoreTarget{}, fmt.Errorf("open restore target %q: %w", relative, err) + } + return restoreTarget{kind: restoreTargetRegular, file: file}, nil +} + +func sha256OpenedFile(file *os.File) (string, error) { + if _, err := file.Seek(0, io.SeekStart); err != nil { + return "", err + } + hash := sha256.New() + if _, err := io.Copy(hash, file); err != nil { + return "", err + } + return hex.EncodeToString(hash.Sum(nil)), nil +} + func restoreActionIsAudio(action RestoreAction) bool { return restoreRelativePathIsAudio(action.LocalRelativePath) } diff --git a/internal/app/restore_plan_test.go b/internal/app/restore_plan_test.go index aeb43eb..8d4d528 100644 --- a/internal/app/restore_plan_test.go +++ b/internal/app/restore_plan_test.go @@ -2,6 +2,7 @@ package app import ( "context" + "os" "path/filepath" "reflect" "strings" @@ -267,6 +268,71 @@ func TestRestorePlanForceTurnsConflictsIntoDownloads(t *testing.T) { } } +func TestRestorePlanSymlinksRemainNonRegularConflicts(t *testing.T) { + for _, tt := range []struct { + name string + setup func(t *testing.T, sessionRoot string) + }{ + { + name: "leaf", + setup: func(t *testing.T, sessionRoot string) { + t.Helper() + outside := filepath.Join(t.TempDir(), "recap.md") + mustWriteTestFile(t, outside, "remote-content\n") + if err := os.MkdirAll(filepath.Join(sessionRoot, "artifacts"), 0o755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + if err := os.Symlink(outside, filepath.Join(sessionRoot, "artifacts", "session_recap.md")); err != nil { + t.Skipf("Symlink() unavailable: %v", err) + } + }, + }, + { + name: "ancestor", + setup: func(t *testing.T, sessionRoot string) { + t.Helper() + outside := t.TempDir() + mustWriteTestFile(t, filepath.Join(outside, "session_recap.md"), "remote-content\n") + if err := os.Symlink(outside, filepath.Join(sessionRoot, "artifacts")); err != nil { + t.Skipf("Symlink() unavailable: %v", err) + } + }, + }, + } { + t.Run(tt.name, func(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")) + sessionRoot := artifacts.SessionWorkDirForCampaign(cfg.Pipeline.Workspace.Root, cfg.Session.Campaign, cfg.Session.SessionID) + tt.setup(t, sessionRoot) + + for _, options := range []RestorePlanOptions{{DryRun: true}, {Force: true}} { + plan, err := buildRestorePlan(context.Background(), cfg, current, store, options) + if err != nil { + t.Fatalf("buildRestorePlan(%+v) error = %v", options, err) + } + action := restoreActionForPath(t, plan, "artifacts/session_recap.md") + if action.Kind != RestoreActionConflict || action.ConflictKind != RestoreConflictNonRegular { + t.Fatalf("action = %#v, want non-regular conflict", action) + } + } + }) + } +} + +func restoreActionForPath(t *testing.T, plan *RestorePlan, path string) RestoreAction { + t.Helper() + for _, action := range plan.Actions { + if action.LocalRelativePath == path { + return action + } + } + t.Fatalf("restore action for %q not found", path) + return RestoreAction{} +} + func TestRestorePlanTraversalUnsafeKeyFails(t *testing.T) { cfg := restorePlanConfig(t) current := restorePlanCurrentState(t, cfg)