diff --git a/docs/WORKTREE_SNAPSHOT_SECURITY.md b/docs/WORKTREE_SNAPSHOT_SECURITY.md new file mode 100644 index 0000000000..be850b7a47 --- /dev/null +++ b/docs/WORKTREE_SNAPSHOT_SECURITY.md @@ -0,0 +1,65 @@ +--- +owner: @SivanCola +backup: @esengine +status: active +reviewed: 2026-10-24 +--- + +# Worktree content snapshot review / Worktree 内容快照审查 + +## Purpose / 目的 + +This is the author's security assessment of the content-snapshot slice extracted +from #14133 at `467a61668a0781cab171536158536283d931e4d9`. It is independent of +the transaction-library PR #14368. It is not independent security approval. + +本批修复已有快照入口受 Git stat 缓存影响的问题，独立于事务基础库。本文是 +作者审查，不代表独立安全审批，也不替代后续隔离、整合及恢复层的审查。 + +## Reference tables / 边界对照 + +| Value / 输入 | Owner and use / 所有者及用途 | +| --- | --- | +| Repository and Git index path | Existing resolved `gitcmd.Repo` and `rev-parse --git-path index`. Existing copy reads that index. No model-provided arbitrary index path is added. | +| Private index | Existing `os.MkdirTemp` directory and fixed `index` leaf. `GIT_INDEX_FILE` is passed as environment data to every new Git command. Only this private index is removed/rebuilt. | +| Tree identity | `write-tree` output from the private index is passed as one argument to `read-tree`. No shell string concatenation or new shell executable is introduced. | +| Workspace bytes | Existing `Repo.StageAll` reads the authorized source through the established Git driver. Git configuration/filter policy is not broadened. | +| Source Git state | The real index, HEAD and refs are not updated. Writing unreachable tree/commit objects remains an existing snapshot side effect governed by Git's own object publication. | + +各路径来自现有宿主解析的仓库身份或新建的私有临时目录。新命令仍经现有 Git +驱动执行；参数不作为 shell 脚本拼接。真实索引和 refs 不参与私有索引重建。 + +## Rules and failure / 规则与失败 + +1. The copied index MUST first preserve its staged tree, including explicitly + staged ignored files. Rebuild from that tree before staging current bytes, + so trusted size/mtime cache entries cannot select stale content. +2. An unmerged copied index MUST be staged from working bytes before its tree + can be written. This records conflict markers as bytes; it does not resolve + or rewrite the user's actual conflicted index. +3. Every new Git invocation MUST retain the existing driver and deadline. + Failure returns an error. The temporary directory is removed by its existing + owner; Git object cleanup remains ordinary Git garbage collection. +4. Callers MUST still coordinate the source during capture. This repair does + not make a directory scan atomic against concurrent external writers, grant + access, isolate execution or issue a verification receipt. +5. Existing snapshot/result formats remain unchanged. No metadata field, + migration, settings change or automatic-isolation policy is introduced. + +规则保证私有索引重建时保留已暂存的忽略文件，并处理真实冲突状态。它没有把 +目录扫描变成跨文件原子快照；调用者仍需协调源文件写入。失败必须返回错误， +不能把不完整捕获当作有效验证证据。 + +## Verification / 验证 + +1. `TestSnapshotHashesBytesDespiteTrustedCachedStat` covers trusted size/mtime, + `assume-unchanged` and `core.ignorestat`. Old code captures old bytes; fixed + code captures new bytes, keeps staged ignored input, and preserves index/HEAD. +2. `TestSnapshotRecordsAWorkspaceMidConflict` uses a real merge conflict and + checks both captured conflict-marker bytes and unchanged real index bytes. +3. The complete worktree race suite covers neighbouring creation, cancellation, + filtering, relative roots, application and rollback behavior. Platform and + broader assembly results are reported separately in the PR. + +回归先证明旧实现在真实 Git 仓库中捕获旧内容，再验证修复后内容、忽略文件、 +索引及 HEAD 均符合预期。其他平台和装配层结论按各自实际测试结果记录。 diff --git a/docs/design/FIRST_RUN_WORKSPACE.md b/docs/design/FIRST_RUN_WORKSPACE.md index 5a616a5d9d..432a4b7d3a 100644 --- a/docs/design/FIRST_RUN_WORKSPACE.md +++ b/docs/design/FIRST_RUN_WORKSPACE.md @@ -1,13 +1,18 @@ --- -owner: @Oliver-Rui -status: proposed +owner: @esengine +backup: @SivanCola +author: @Oliver-Rui +status: active +reviewed: 2026-10-24 --- # First run without a picked workspace Status: proposed. Nothing in this document is shipped unless §8 says so. It lives under `docs/design/` so the product-doc retrieval corpus (`docs/*.md`) cannot present it to a model as a feature. -Reference: how MiMo Desktop (observed 2026-10-24 on a machine running it) handles the same three first-run decisions — connect a model, pick a workspace, start a session. Its behaviour below is read off installed artifacts (config schema, directory layout, state store), not source; the mapping onto this repository in §6 is grounded in source. +Reference: how MiMo Desktop (observed 2026-10-24 on a machine running it) handles the same three first-run decisions — connect a model, pick a workspace, start a session. + +Its behaviour below is read off installed artifacts (config schema, directory layout, state store), not source; the mapping onto this repository in §6 is grounded in source. ## 1. The problem @@ -19,9 +24,13 @@ Studio's first launch asks three things at once, and only one of them is honestl | Start a session | the composer is reachable as soon as the workbench opens | yes | | Pick a workspace | `pickLaunchWorkspace(cwd, appExe, remembered, home)` — with nothing remembered, the first pane's root is the **user's home directory** | no | -The fallback chain in `cmd/reasonix-studio-host/launchroot.go` is careful about *which* cwd it trusts — a Start-menu launch inherits the install directory, and taking that as a project "puts the app's own folder in the sidebar again after every removal" — but its last resort is the home directory. That folder was never chosen as a project. It is broad, it is sensitive, and it is not represented in the sidebar as anything the user picked, so a session running there reads as a temporary session: not in the user's project, scoped to a folder they did not choose, and removing it does not stick. +The fallback chain in `cmd/reasonix-studio-host/launchroot.go` is careful about *which* cwd it trusts — a Start-menu launch inherits the install directory, and taking that as a project "puts the app's own folder in the sidebar again after every removal" — but its last resort is the home directory. -Everything else is already in place for a decoupled first run. Session transcripts persist in the global store (`SessionDir()` = `/sessions`), so a session without a chosen project is not lost. Each logical session already owns a private scratch directory (`SessionTemp`, rotated per session, deleted when it ends, surfaced to the model as `$TMPDIR`). Provider setup already refuses to block. The workspace is the last coupled decision. +That folder was never chosen as a project. It is broad, it is sensitive, and it is not represented in the sidebar as anything the user picked, so a session running there reads as a temporary session: not in the user's project, scoped to a folder they did not choose, and removing it does not stick. + +Everything else is already in place for a decoupled first run. Session transcripts persist in the global store (`SessionDir()` = `/sessions`), so a session without a chosen project is not lost. + +Each logical session already owns a private scratch directory (`SessionTemp`, rotated per session, deleted when it ends, surfaced to the model as `$TMPDIR`). Provider setup already refuses to block. The workspace is the last coupled decision. ## 2. What MiMo does instead diff --git a/internal/platform/worktree/candidate.go b/internal/platform/worktree/candidate.go index bde3380c19..cba5ced81e 100644 --- a/internal/platform/worktree/candidate.go +++ b/internal/platform/worktree/candidate.go @@ -193,8 +193,8 @@ func RemoveCandidate(ctx context.Context, snap Snapshot, c Candidate) error { return nil } -// workingTree writes repo's working state as a tree through a private copy of -// its index, so only changed files are hashed and the real index is untouched. +// workingTree hashes actual workspace bytes through a rebuilt private index. +// Git's cached stat identity is not a content version; the real index is untouched. func workingTree(ctx context.Context, repo gitcmd.Repo) (string, error) { indexPath, _, err := runGit(ctx, repo, "rev-parse", "--git-path", "index") if err != nil { @@ -216,6 +216,29 @@ func workingTree(ctx context.Context, repo gitcmd.Repo) (string, error) { env := []string{"GIT_INDEX_FILE=" + private} stageCtx, cancel := context.WithTimeout(ctx, gitWorktreeAddTimeout) defer cancel() + if _, err := os.Stat(private); err == nil { + unmerged, stderr, err := runGitEnv(stageCtx, repo, env, "ls-files", "--unmerged", "-z") + if err != nil { + return "", fmt.Errorf("read private index: %w%s", err, stderrSuffix(stderr)) + } + if unmerged != "" { + if err := repo.StageAll(stageCtx, env...); err != nil { + return "", fmt.Errorf("stage conflicted workspace snapshot: %w", err) + } + } + indexTree, stderr, err := runGitEnv(stageCtx, repo, env, "write-tree") + if err != nil { + return "", fmt.Errorf("record private index: %w%s", err, stderrSuffix(stderr)) + } + if err := os.Remove(private); err != nil { + return "", err + } + if _, stderr, err := runGitEnv(stageCtx, repo, env, "read-tree", strings.TrimSpace(indexTree)); err != nil { + return "", fmt.Errorf("rebuild private index: %w%s", err, stderrSuffix(stderr)) + } + } else if !os.IsNotExist(err) { + return "", err + } if err := repo.StageAll(stageCtx, env...); err != nil { return "", fmt.Errorf("stage workspace snapshot: %w", err) } @@ -257,9 +280,8 @@ func runGitEnv(parent context.Context, repo gitcmd.Repo, env []string, args ...s return outBuf.String(), strings.TrimSpace(errBuf.String()), err } -// copyFile copies an index keeping its modification time: git re-hashes an -// entry written in the index's own second only while the index file's mtime -// says so, and a fresh mtime would hide a same-size edit made in that second. +// copyFile preserves the index bytes and timestamp. workingTree rebuilds its +// cached entries before staging the current workspace content. func copyFile(from, to string) error { in, err := os.Open(from) if err != nil { diff --git a/internal/platform/worktree/content_snapshot_test.go b/internal/platform/worktree/content_snapshot_test.go new file mode 100644 index 0000000000..2ac31755e7 --- /dev/null +++ b/internal/platform/worktree/content_snapshot_test.go @@ -0,0 +1,73 @@ +package worktree + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestSnapshotHashesBytesDespiteTrustedCachedStat(t *testing.T) { + for _, policy := range []string{"stat-cache", "assume-unchanged", "ignore-stat"} { + t.Run(policy, func(t *testing.T) { testSnapshotCurrentBytes(t, policy) }) + } +} + +func testSnapshotCurrentBytes(t *testing.T, policy string) { + t.Helper() + requireGit(t) + root := t.TempDir() + gitIn(t, root, "init", "-q") + gitIn(t, root, "config", "core.trustctime", "false") + gitIn(t, root, "config", "core.checkstat", "minimal") + if policy == "ignore-stat" { + gitIn(t, root, "config", "core.ignorestat", "true") + } + writeRepoFile(t, root, "input", "old\n") + writeRepoFile(t, root, ".gitignore", "staged-ignored\n") + path := filepath.Join(root, "input") + stamp := time.Now().Add(-time.Hour).Truncate(time.Second) + if err := os.Chtimes(path, stamp, stamp); err != nil { + t.Fatal(err) + } + gitIn(t, root, "add", "-A") + gitIn(t, root, "commit", "-qm", "base") + if policy == "assume-unchanged" { + gitIn(t, root, "update-index", "--assume-unchanged", "input") + } + writeRepoFile(t, root, "staged-ignored", "must survive\n") + gitIn(t, root, "add", "-f", "staged-ignored") + index := filepath.Join(root, ".git", "index") + before, err := os.ReadFile(index) + if err != nil { + t.Fatal(err) + } + writeRepoFile(t, root, "input", "new\n") + if err := os.Chtimes(path, stamp, stamp); err != nil { + t.Fatal(err) + } + // The ordinary stat cache really does hide this edit; otherwise the test + // would not distinguish hashing bytes from copying Git's cached version. + if diff := gitIn(t, root, "diff", "--", "input"); diff != "" { + t.Fatalf("fixture did not reproduce a trusted stale stat cache: %s", diff) + } + snapshot, err := TakeSnapshot(t.Context(), opened(t, root)) + if err != nil { + t.Fatal(err) + } + if got := gitIn(t, root, "show", snapshot.Tree+":input"); got != "new\n" { + t.Fatalf("snapshot reused stale stat cache: %q", got) + } + if got := gitIn(t, root, "show", snapshot.Tree+":staged-ignored"); got != "must survive\n" { + t.Fatalf("private index rebuild lost staged ignored input: %q", got) + } + after, err := os.ReadFile(index) + if err != nil || !bytes.Equal(before, after) { + t.Fatalf("snapshot modified the user's index: %v", err) + } + if got := strings.TrimSpace(gitIn(t, root, "show", "HEAD:input")); got != "old" { + t.Fatalf("snapshot changed HEAD: %s", got) + } +} diff --git a/internal/platform/worktree/snapshot_unmerged_test.go b/internal/platform/worktree/snapshot_unmerged_test.go new file mode 100644 index 0000000000..d311e5b629 --- /dev/null +++ b/internal/platform/worktree/snapshot_unmerged_test.go @@ -0,0 +1,60 @@ +package worktree + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestSnapshotRecordsAWorkspaceMidConflict(t *testing.T) { + requireGit(t) + repo := initRepo(t) + git := func(args ...string) error { + cmd := exec.Command("git", append([]string{"-C", repo}, args...)...) + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", + "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t") + return cmd.Run() + } + write := func(body string) { + if err := os.WriteFile(filepath.Join(repo, "README.md"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + if err := git("checkout", "-q", "-b", "other"); err != nil { + t.Fatal(err) + } + write("other\n") + if err := git("commit", "-qam", "other"); err != nil { + t.Fatal(err) + } + if err := git("checkout", "-q", "-"); err != nil { + t.Fatal(err) + } + write("mine\n") + if err := git("commit", "-qam", "mine"); err != nil { + t.Fatal(err) + } + if err := git("merge", "-q", "other"); err == nil { + t.Fatal("fixture merge did not conflict") + } + index := filepath.Join(repo, ".git", "index") + before, err := os.ReadFile(index) + if err != nil { + t.Fatal(err) + } + snap, err := TakeSnapshot(t.Context(), opened(t, repo)) + if err != nil { + t.Fatalf("a conflicted index blocked the snapshot: %v", err) + } + out, err := exec.Command("git", "-C", repo, "cat-file", "-p", snap.Tree+":README.md").Output() + if err != nil || !strings.Contains(string(out), "<<<<<<<") { + t.Fatalf("snapshot did not record the working file: %q %v", out, err) + } + after, err := os.ReadFile(index) + if err != nil || !bytes.Equal(before, after) { + t.Fatalf("snapshot rewrote the user's conflicted index: %v", err) + } +} diff --git a/release-notes/studio/unreleased/14025-snapshot-content.md b/release-notes/studio/unreleased/14025-snapshot-content.md new file mode 100644 index 0000000000..677f1373b1 --- /dev/null +++ b/release-notes/studio/unreleased/14025-snapshot-content.md @@ -0,0 +1,2 @@ +- 修复文件大小和修改时间未变时，候选快照可能复用 Git 缓存中的旧内容；同时覆盖 `assume-unchanged` 和 `core.ignorestat` 设置。关联 #14025。 +- 私有索引重建保留用户暂存区、已暂存的忽略文件和未解决的合并冲突现场。关联 #14025。