diff --git a/docs/ISOLATED_INSPECTION_SECURITY.md b/docs/ISOLATED_INSPECTION_SECURITY.md new file mode 100644 index 0000000000..0f43babc37 --- /dev/null +++ b/docs/ISOLATED_INSPECTION_SECURITY.md @@ -0,0 +1,63 @@ +--- +owner: @SivanCola +backup: @esengine +status: active +reviewed: 2026-10-25 +--- + +# Isolated result inspection / 隔离结果检查 + +## Cause and repair / 根因与修复 + +A restarted store reconstructs a pending result by asking Git for its candidate +tree and diff statistics. Recovery previously treated an inspection error and an +empty result as the same cleanup condition. Cleanup forgot the durable marker +before attempting worktree removal, losing the result's discoverable identity. + +重启后的存储通过 Git 重建候选树和变更统计。此前检查失败与无修改共用清理分支, +先删除恢复标记再尝试移除工作树。即使 Git 只是暂时不可用,原结果 ID 也会丢失。 + +Recovery now returns inspection errors before any cleanup. The lookup owner +reports the existing `isolation.unavailable` refusal with the result ID and +retained-files guidance. A caller can retry the same ID when Git is available. +Genuinely unknown IDs retain their existing refusal. + +修复将检查错误直接返回,禁止错误触发清理。共用查询入口使用已有的不可用错误码, +说明结果 ID 和文件保留情况。Git 恢复后可用同一 ID 重试;不存在的 ID 仍按原规则拒绝。 + +## Inputs, authority and effects / 输入、权限与执行 + +- Result IDs and durable markers retain their existing workspace scope. This + change adds no path grant, root, Git command or model-controlled execution. +- All three callers share the fix: apply path discovery, apply execution and + explicit discard. An inspection error mutates neither the marker nor the + store's recovered-entry map and does not start apply/discard cleanup. +- Inspection can still invoke the existing Git snapshot machinery. This is not + a promise that arbitrary Git internals, external writers or commands are + confined, atomic or side-effect-free. +- Error details come from the existing local Git operations. The durable JSON + shape, tool schema, refusal vocabulary and compatibility format are unchanged. + +本批不增加路径授权、Git 命令或模型可控执行点。三个调用入口均复用同一修复; +失败时不删除标记、不登记错误重建结果,也不开始应用或丢弃。检查仍使用原 Git +快照流程,不宣称其具备系统沙箱、全局原子性或消除外部写入竞争的能力。 + +## Evidence and remaining bounds / 证据与未完成边界 + +Regressions create real disposable Git worktrees, restart the store and remove +Git from the test process's PATH. Every caller must preserve marker bytes, +candidate content, workspace content and pending identity. Restoring PATH must +allow the same operation to complete using the original ID. + +回归使用真实临时 Git 工作树,在存储重建后暂时移除测试进程的 Git 搜索路径。 +测试要求恢复标记逐字节保留,候选内容和主工作区不变,原 ID 仍可发现;恢复路径 +后同一操作必须成功。没有修改用户 Git 配置或运行生产资源。 + +Age-based expiration, successful-empty inspection policy, interrupted Finish, +versioned lifecycle records, recovery cancellation, journaled application and +cross-process settlement remain separate review units. This repair does not +claim to preserve results from an older process still using the old policy. + +按年龄清理、成功但为空的判定、Finish 中断、版本化生命周期、恢复取消、整合日志 +与跨进程整合互斥仍须分批处理。本批不能约束仍使用旧清理策略的旧程序,也不代表 +完整恢复层已经验收。作者说明不能替代独立安全审查。 diff --git a/internal/runtime/isolation/durable.go b/internal/runtime/isolation/durable.go index bdbf133313..1d0a581091 100644 --- a/internal/runtime/isolation/durable.go +++ b/internal/runtime/isolation/durable.go @@ -3,6 +3,7 @@ package isolation import ( "context" "encoding/json" + "fmt" "os" "path/filepath" "time" @@ -70,9 +71,9 @@ func (s *Store) durableIDs() []string { return ids } -// recover rebuilds a result from its worktree. One whose worktree no longer -// holds a change is removed rather than offered. -func (s *Store) recover(id string) (*Entry, bool) { +// recover rebuilds a result from its worktree. Inspection errors retain the +// result for retry; a successfully inspected empty result is removed. +func (s *Store) recover(id string) (*Entry, error) { for _, d := range s.durable() { if d.ID != id { continue @@ -83,17 +84,20 @@ func (s *Store) recover(id string) (*Entry, bool) { if err == nil && len(changes) > 0 { e.Stats, err = worktree.DiffStat(ctx, e.Snap, tree, changes) } - if err != nil || len(changes) == 0 { + if err != nil { + return nil, fmt.Errorf("inspect isolated result %s; files retained: %w", id, err) + } + if len(changes) == 0 { s.remove(ctx, e) - return nil, false + return nil, nil } e.Tree, e.Changes = tree, changes s.mu.Lock() s.entries[e.ID] = e s.mu.Unlock() - return e, true + return e, nil } - return nil, false + return nil, nil } // SweepExpired removes this workspace's results nobody settled within maxAge. diff --git a/internal/runtime/isolation/recover_inspection_test.go b/internal/runtime/isolation/recover_inspection_test.go new file mode 100644 index 0000000000..a9aad98771 --- /dev/null +++ b/internal/runtime/isolation/recover_inspection_test.go @@ -0,0 +1,89 @@ +package isolation + +import ( + "bytes" + "errors" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "reasonix/internal/base/testenv" + "reasonix/internal/contract/tool" +) + +func TestRecoveryInspectionFailureRetainsResultAndCanRetry(t *testing.T) { + for _, operation := range []string{"paths", "apply", "discard"} { + t.Run(operation, func(t *testing.T) { + repo := gitRepo(t) + managed := testenv.TempDir(t) + store := NewStore(managed, repo, opened(t, repo)) + report, err := store.Execute(t.Context(), writes(map[string]string{"a.txt": "isolated work\n"}), repo, "change a.txt", "") + if err != nil { + t.Fatal(err) + } + id := idOf(t, report) + e, err := store.lookup(id) + if err != nil { + t.Fatal(err) + } + marker := entryPath(e.Cand) + before, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + restarted := NewStore(managed, repo, opened(t, repo)) + invoke := func() error { + switch operation { + case "paths": + _, err := NewApplyTool(restarted).DeclaredWritePaths(t.Context(), args(id)) + return err + case "apply": + _, err := NewApplyTool(restarted).Execute(t.Context(), args(id)) + return err + default: + _, err := NewDiscardTool(restarted).Execute(t.Context(), args(id)) + return err + } + } + // No Git process can inspect or clean the worktree while PATH is empty. + // This is a temporary tool failure, not an empty or discarded result. + previousPath := os.Getenv("PATH") + t.Setenv("PATH", testenv.TempDir(t)) + err = invoke() + after, readErr := os.ReadFile(marker) + if readErr != nil || !bytes.Equal(after, before) { + t.Fatalf("inspection failure lost or changed the recovery marker: %v", readErr) + } + if got := read(t, e.Cand.WorkspaceRoot, "a.txt"); got != "isolated work\n" { + t.Fatalf("candidate work was changed: %q", got) + } + if got := read(t, repo, "a.txt"); got != "one\n" { + t.Fatalf("failed recovery changed workspace: %q", got) + } + var refusal tool.Refusal + if !errors.As(err, &refusal) || refusal.Code != CodeUnavailable || !strings.Contains(refusal.Message, id) || !strings.Contains(refusal.Message, "retained") { + t.Fatalf("inspection error = %v, want unavailable result with retained-files guidance", err) + } + if !slices.Contains(restarted.Pending(), id) { + t.Fatal("pending result became undiscoverable") + } + t.Setenv("PATH", previousPath) + if err := invoke(); err != nil { + t.Fatalf("same result could not retry after Git returned: %v", err) + } + if operation == "apply" && read(t, repo, "a.txt") != "isolated work\n" { + t.Fatal("retried apply did not integrate retained work") + } + if operation == "paths" { + if _, err := NewDiscardTool(restarted).Execute(t.Context(), args(id)); err != nil { + t.Fatal(err) + } + } + if _, err := os.Stat(filepath.Join(filepath.Dir(e.Cand.WorktreeRoot), entryFile)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("settled result marker remains: %v", err) + } + }) + } +} diff --git a/internal/runtime/isolation/store.go b/internal/runtime/isolation/store.go index a51c2b4295..c037b0f3c5 100644 --- a/internal/runtime/isolation/store.go +++ b/internal/runtime/isolation/store.go @@ -170,7 +170,11 @@ func (s *Store) lookup(id string) (*Entry, error) { e, ok := s.entries[id] s.mu.Unlock() if !ok { - e, ok = s.recover(id) + recovered, err := s.recover(id) + if err != nil { + return nil, tool.Refusal{Code: CodeUnavailable, Message: err.Error()} + } + e, ok = recovered, recovered != nil } if !ok { pending := s.Pending() diff --git a/release-notes/studio/unreleased/14025-isolated-inspection.md b/release-notes/studio/unreleased/14025-isolated-inspection.md new file mode 100644 index 0000000000..0956f2f764 --- /dev/null +++ b/release-notes/studio/unreleased/14025-isolated-inspection.md @@ -0,0 +1,2 @@ +- Fixed: temporary Git inspection failures after restart retain an isolated result's files and ID, so path discovery, apply or discard can retry when Git returns. Refs #14025, #14133. +- 修复:重启后 Git 暂时不可用时保留隔离结果的文件和 ID;Git 恢复后可重新查询、应用或丢弃。关联 #14025、#14133。