eric/narratio
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 27 Wiki Activity

Protect restore planning from symlinks

Browse Source
This commit is contained in:
Eric Rakestraw
2026-09-02 00:51:19 +00:00
parent 1e418d6929
commit 45b27ad562
6 changed files with 201 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
docs/internal/command-restore.md
View File

@@ -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.

6
docs/operations.md
View File

@@ -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`.

2
docs/roadmap/implementation.md
View File

@@ -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

5
docs/troubleshooting.md
View File

@@ -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).

139
internal/app/restore_plan.go
View File

@@ -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)
}

66
internal/app/restore_plan_test.go
View File

@@ -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)