回滚tool#422
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| if info.IsDir() { | ||
| return snapshotState{}, fmt.Errorf("rollback snapshot target is a directory: %s", path) | ||
| } | ||
| if info.Size() > maxSnapshotBytes { |
There was a problem hiding this comment.
High: snapshot capture now hard-fails for any existing file larger than 5 MiB or any non-text file. Because Begin() propagates this error back through write_file, replace_in_file, and apply_patch, those tools can no longer modify files they previously could. That is a user-visible regression unrelated to whether rollback metadata is available; if a file is not snapshot-safe, the safer fallback is usually to skip rollback for that target rather than reject the edit entirely.
| if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { | ||
| return err | ||
| } | ||
| return os.WriteFile(path, data, 0o644) |
There was a problem hiding this comment.
Medium: rollback restores the previous bytes with a hard-coded 0644, so executable/read-only bits are lost after /rollback. readSnapshotCandidate() already captures the original mode, but FileChange never persists it and restoreSnapshot() does not use it. Rolling back a script or any non-0644 file will silently change its permissions.
| return paths | ||
| } | ||
|
|
||
| func validateOperationPaths(op Operation, workspace string, writableRoots ...string) error { |
There was a problem hiding this comment.
High: validateOperationPaths() only checks cleaned absolute paths and does not canonicalize symlinks the way normal tool access does in resolvePath(). If a recorded workspace path is replaced with a symlink before /rollback, this check still passes and the subsequent read/write/remove follows the link, which can target files outside the workspace or writable roots. Rollback needs the same canonical-path validation at execution time to preserve the trust boundary.
变更内容
write_file、replace_in_file、apply_patch接入回退操作记录/rollback命令,用于查看和执行回退