mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-12 08:41:32 -10:00
* docs: spec for additional write roots (--add-dir / /add-dir) Co-Authored-By: Claude Fable 5 <[email protected]> * docs: implementation plan for additional write roots Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: add Scope type for multi-root write access Co-Authored-By: Claude Fable 5 <[email protected]> * docs: write roots honored from global config only, union merge semantics Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: fix multi-root violation reporting and pin prefix-alias behavior - validate: prefer ViolationSymlinkTraversal over ViolationOutsideWorkspace when all roots deny; return original requestedPath in violation; append --add-dir hint only on outside_workspace results; extend doc comment to state that a symlink resolving inside any root is allowed. - normalizePrefixForRoot: dedupe insideRoot check via pathWithinRoot (Fix 3); widen EvalSymlinks comment to cover jump-over case; add POSIX-only note. - WorkspaceRoot: add doc comment noting immutability/lock safety (Fix 4). - Tests: pin multi-root traversal-preferred behavior, deterministic alias test (TestValidateResolvesAliasedPathPrefixes), Add symlink normalization test, and tilde-expansion error path test. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: make engine path validation and risk scope-aware Thread *Scope through EngineOptions/Engine so Evaluate and Classify use multi-root scope validation instead of single-root validateWorkspacePaths, letting extra write roots (--add-dir / /add-dir) be honoured by the policy gate and risk classifier. Remove now-dead validateWorkspacePaths helper. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pin override-root scope isolation, tidy Evaluate scope handling Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: widen seatbelt/bubblewrap profiles and command cwd to scope roots Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pin extra-root chdir behavior and document bind invariants Co-Authored-By: Claude Fable 5 <[email protected]> * tools: resolve paths against the shared write scope Add PathScope interface and scoped resolver helpers to workspace.go, then thread a scope field through all 8 file tools (read_file, list_directory, glob, grep, write_file, edit_file, apply_patch, bash). Each existing constructor delegates to a new NewScoped* variant; nil scope is byte-identical to the original behavior. Registry gains CoreReadOnlyToolsScoped, CoreWriteToolsScoped, CoreShellToolsScoped, and CoreToolsScoped. Co-Authored-By: Claude Fable 5 <[email protected]> * tools: restore fail-closed write-target symlink checks, share prefix normalization * tools: report absolute paths for extra-root changes, pin scoped traversal denial - resolveScopedPath and resolveScopedTargetPath now return the absolute path as the second value when the matched root is an extra (non-workspace) root, eliminating ambiguity in ChangedFiles, cwd meta, and display summaries downstream - Add TestScopedWriteRefusesSameRootSymlinkTraversal to pin that same-root symlink traversal within an extra root is denied (pre-existing behavior) - Add TestScopedWriteReportsAbsolutePathForExtraRoot to enforce the new absolute-path contract for ChangedFiles on extra-root writes - Extend doc comments on PathScope, ChangedFiles, changedFilesFromPatch, and both scoped resolvers to capture the workspace-vs-absolute invariant Co-Authored-By: Claude Fable 5 <[email protected]> * config: add sandbox.additionalWriteRoots (global config only, union merge) * cli: add repeatable --add-dir flag wiring scope into registry and engine Co-Authored-By: Claude Fable 5 <[email protected]> * cli: forward --add-dir across dispatch paths, pin scope wiring with tests Co-Authored-By: Claude Fable 5 <[email protected]> * cli: pin --add-dir dispatch forwarding end-to-end, close guard gaps Review fixes for the --add-dir dispatch commit: - Add TestRunAddDirDispatchForwardsGrantIntoExecScope: drives runWithDeps through both the "exec" and "-p" dispatch shapes with a provider that calls write_file inside the extra root, asserting the grant reaches the exec sandbox scope (and a negative control without --add-dir is denied fail-closed). Mutation-verified: dropping addDirFlagArgs forwarding from either case fails the test. - Remove help/version from the --add-dir allowlist so any non-TUI/ non-exec leading --add-dir hard-errors, matching the stated requirement; extend the rejection test with those cases. - Fail loud when --add-dir is hidden behind a stray non-flag arg on the --skip-permissions-unsafe path instead of silently dropping the grant. - Reject flag-like values in the inline --add-dir= spelling to match the space form (a directory named -foo stays reachable as ./-foo). - Move TestExecScopeReRegistrationSwapsCoreToolsByName out of the parse test file into the new exec_scope_test.go alongside the e2e test. Co-Authored-By: Claude Fable 5 <[email protected]> * tui: add /add-dir command for session write-root grants Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: surface write roots in zero sandbox status and plan snapshots Co-Authored-By: Claude Fable 5 <[email protected]> * cli: surface write-roots error in --effective --json, normalize fallback root Review fixes for the Task8 observability range: - runSandboxPolicyEffective JSON payload gains writeRootsError (omitempty) so --json consumers see the same fail-soft signal as the text write_roots_error line instead of a misleading workspace-only writeRoots list. - The fail-soft fallback now derives its write roots from a workspace-only Scope (which cannot fail) so the error path renders the same symlink-resolved root as the success path. - TestRunSandboxPolicyEffectiveWriteRootsFailSoft gains a --json leg asserting writeRootsError names the stale root with a workspace-only fallback; the configured-roots test asserts the key is absent for valid roots. - SandboxPlanSnapshot doc comment reworded to match the converter: no current builder populates WriteRoots. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: seatbelt integration test for extra write roots Co-Authored-By: Claude Fable 5 <[email protected]> * docs: document --add-dir and /add-dir write-root grants Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pass volume-qualified paths through prefix normalization verbatim NormalizePrefixForRoot's POSIX component walk mangled Windows drive paths (C:\Users -> C:Users), which downstream single-root checks treated as relative and joined under the workspace — failing the policy gate OPEN on Windows. Volume-qualified paths now bypass the walk entirely (the macOS /var alias problem it solves has no Windows equivalent) and windows-gated tests pin both the verbatim pass-through and the engine denial. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: make prefix normalization volume-aware for Windows alias resolution The previous commit disabled NormalizePrefixForRoot on Windows, which broke it the other direction: a workspace created under an 8.3 short path (C:\Users\RUNNER~1\...) resolves via EvalSymlinks to its long form (runneradmin), so raw short-form requests escaped the long-form root and legitimate extra-root writes were denied. This is the same alias problem as macOS /var -> /private/var, which the helper already solves. Start the component walk from the volume root (C:\ or //host/share/) instead of "/" so it resolves the prefix on Windows too; POSIX VolumeName is empty so the walk reduces to the original "/"-rooted behavior byte-for-byte. Windows test now pins allow-inside-root + deny-outside both directions. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: address PR review findings for additional write roots Resolve the CHANGES_REQUESTED review on PR #162: - engine: derive workspaceRoot from scope.Roots()[0] when NewEngine is given a Scope without an explicit WorkspaceRoot, so Evaluate's EnforceWorkspace/classification guards no longer silently skip (+ regression test). - tools/workspace: scopedRoots now fails closed (returns an error) when a non-nil PathScope exposes empty Roots(), instead of returning success with an empty path; threaded through the scoped resolve/recheck helpers and resolveGrepRoot. - tools/glob: emit absolute matches when cwd resolves into an extra root so results can't be fed back and resolve to a same-named workspace file (+ regression test). - tools/bash,grep,apply_patch: schema descriptions mention granted extra roots (relative -> workspace, absolute -> granted root). - docs/plan: fix markdownlint heading increment, malformed rule+heading, and bare code fence. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * tools/grep, tui: address PR review for additional write roots - grep now emits absolute, symlink-resolved paths for matches in a granted extra (non-workspace) root, mirroring the glob fix. A bare workspace-relative name like "report.go" otherwise resolves back under the workspace when fed to read_file/edit_file and hits the wrong file when the name exists in both roots. Covers content and files_with_matches output. - tui: only reset chatScrollOffset for a real submission, not an empty Enter (a no-op), so the viewport is no longer yanked to the bottom when nothing was submitted. Real submissions still snap back. - glob: schema description now mentions granted extra roots, matching the bash/grep/apply_patch wording so the --add-dir/-add-dir flow is discoverable. Regression tests: TestScopedGrepReturnsAbsoluteMatchesForExtraRoot, TestEmptySubmitKeepsChatScrollOffset. * docs: add /add-dir to the TUI commands quick-reference table The command was documented further down but missing from the scannable commands table, so users browsing the table wouldn't discover the additional-write-roots flow. --------- Co-authored-by: Claude Fable 5 <[email protected]>
347 lines
11 KiB
Go
347 lines
11 KiB
Go
package tools
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
|
|
"github.com/Gitlawb/zero/internal/sandbox"
|
|
"github.com/Gitlawb/zero/internal/workspaceindex"
|
|
)
|
|
|
|
var ignoredDirectories = map[string]bool{
|
|
".git": true,
|
|
"node_modules": true,
|
|
"dist": true,
|
|
"build": true,
|
|
".next": true,
|
|
".turbo": true,
|
|
"coverage": true,
|
|
".cache": true,
|
|
"tmp": true,
|
|
"temp": true,
|
|
}
|
|
|
|
func normalizeWorkspaceRoot(workspaceRoot string) string {
|
|
root, err := filepath.Abs(workspaceRoot)
|
|
if err != nil {
|
|
return workspaceRoot
|
|
}
|
|
return root
|
|
}
|
|
|
|
func resolveWorkspacePath(workspaceRoot string, requestedPath string) (string, string, error) {
|
|
if requestedPath == "" {
|
|
requestedPath = "."
|
|
}
|
|
|
|
root, err := filepath.Abs(workspaceRoot)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
root, err = filepath.EvalSymlinks(root)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
|
|
target := requestedPath
|
|
if !filepath.IsAbs(target) {
|
|
target = filepath.Join(root, target)
|
|
}
|
|
|
|
target, err = filepath.Abs(target)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
target, err = filepath.EvalSymlinks(target)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
|
|
relative, err := filepath.Rel(root, target)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) {
|
|
return "", "", fmt.Errorf("%s must stay inside the workspace", requestedPath)
|
|
}
|
|
if relative == "." {
|
|
return target, ".", nil
|
|
}
|
|
return target, filepath.ToSlash(relative), nil
|
|
}
|
|
|
|
func resolveWorkspaceTargetPath(workspaceRoot string, requestedPath string) (string, string, error) {
|
|
if requestedPath == "" {
|
|
requestedPath = "."
|
|
}
|
|
|
|
root, err := filepath.Abs(workspaceRoot)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
root, err = filepath.EvalSymlinks(root)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
|
|
target := requestedPath
|
|
if !filepath.IsAbs(target) {
|
|
target = filepath.Join(root, target)
|
|
}
|
|
target, err = filepath.Abs(target)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
if err := recheckWorkspaceWriteTarget(root, target); err != nil {
|
|
return "", "", err
|
|
}
|
|
|
|
existing := target
|
|
missingSegments := []string{}
|
|
for {
|
|
if _, err := os.Lstat(existing); err == nil {
|
|
break
|
|
} else if os.IsNotExist(err) {
|
|
parent := filepath.Dir(existing)
|
|
if parent == existing {
|
|
return "", "", err
|
|
}
|
|
missingSegments = append([]string{filepath.Base(existing)}, missingSegments...)
|
|
existing = parent
|
|
continue
|
|
} else {
|
|
return "", "", err
|
|
}
|
|
}
|
|
|
|
resolved, err := filepath.EvalSymlinks(existing)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
for _, segment := range missingSegments {
|
|
resolved = filepath.Join(resolved, segment)
|
|
}
|
|
|
|
relative, err := filepath.Rel(root, resolved)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) {
|
|
return "", "", fmt.Errorf("%s must stay inside the workspace", requestedPath)
|
|
}
|
|
if relative == "." {
|
|
return resolved, ".", nil
|
|
}
|
|
return resolved, filepath.ToSlash(relative), nil
|
|
}
|
|
|
|
func recheckWorkspaceWriteTarget(workspaceRoot string, requestedPath string) error {
|
|
if requestedPath == "" {
|
|
requestedPath = "."
|
|
}
|
|
|
|
root, err := filepath.Abs(workspaceRoot)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
root, err = filepath.EvalSymlinks(root)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
target := requestedPath
|
|
if !filepath.IsAbs(target) {
|
|
target = filepath.Join(root, target)
|
|
}
|
|
target, err = filepath.Abs(target)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
relative, err := filepath.Rel(root, target)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) || filepath.IsAbs(relative) {
|
|
return fmt.Errorf("%s must stay inside the workspace", requestedPath)
|
|
}
|
|
if relative == "." {
|
|
return nil
|
|
}
|
|
|
|
current := root
|
|
for _, segment := range strings.Split(filepath.Clean(relative), string(filepath.Separator)) {
|
|
if segment == "." || segment == "" {
|
|
continue
|
|
}
|
|
|
|
current = filepath.Join(current, segment)
|
|
info, err := os.Lstat(current)
|
|
if err != nil {
|
|
if os.IsNotExist(err) {
|
|
return nil
|
|
}
|
|
return err
|
|
}
|
|
if info.Mode()&os.ModeSymlink != 0 {
|
|
symlinkRelative, err := filepath.Rel(root, current)
|
|
if err != nil {
|
|
symlinkRelative = current
|
|
}
|
|
return fmt.Errorf("%s must not traverse symlink %s", requestedPath, filepath.ToSlash(symlinkRelative))
|
|
}
|
|
}
|
|
|
|
return nil
|
|
}
|
|
|
|
func shouldSkipDirectory(name string) bool {
|
|
return ignoredDirectories[name] || workspaceindex.ShouldSkipDir(name)
|
|
}
|
|
|
|
func shouldSkipWorkspaceFile(path string) bool {
|
|
return workspaceindex.ShouldSkipFile(path)
|
|
}
|
|
|
|
// PathScope is the multi-root write scope shared with the sandbox engine.
|
|
// *sandbox.Scope satisfies it; nil means workspace-only (today's behavior).
|
|
// Roots()[0] must be the workspace root (sandbox.Scope guarantees this
|
|
// ordering); relative paths and error messages key off it.
|
|
type PathScope interface {
|
|
Roots() []string
|
|
}
|
|
|
|
// scopedRoots returns the ordered roots to try for an absolute path: the
|
|
// scope's roots when present, else just the workspace root. A non-nil scope
|
|
// must expose at least one root (the workspace root first, per PathScope); an
|
|
// empty Roots() is a contract violation that fails closed with an error so the
|
|
// scoped helpers never silently accept a path with no root to validate against.
|
|
// Returning success there would resolve to an empty path and, e.g., run bash
|
|
// with Cmd.Dir == "" (the process cwd) instead of an allowed root.
|
|
func scopedRoots(workspaceRoot string, scope PathScope) ([]string, error) {
|
|
if scope == nil {
|
|
return []string{workspaceRoot}, nil
|
|
}
|
|
roots := scope.Roots()
|
|
if len(roots) == 0 {
|
|
return nil, fmt.Errorf("invalid path scope: no write roots configured")
|
|
}
|
|
return roots, nil
|
|
}
|
|
|
|
// resolveScopedPath is resolveWorkspacePath generalized to a scope: relative
|
|
// paths resolve against the workspace root only; an absolute path resolves
|
|
// against the first root that contains it. The workspace root's error is
|
|
// returned when no root matches so messages stay stable. Prefix symlinks
|
|
// outside a root are resolved per root (macOS /var aliasing); symlinks INSIDE
|
|
// a root stay visible to the single-root checks, so a write target may resolve
|
|
// through a symlink only when its final location lies inside a DIFFERENT
|
|
// granted root — mirroring sandbox.Scope.validate's documented widening.
|
|
// When the matched root is an extra (non-workspace) root the second return
|
|
// value is the absolute path rather than a per-root-relative path; "relative
|
|
// to which root" is ambiguous for downstream consumers (ChangedFiles, cwd
|
|
// meta, display summaries) that document workspace-relative paths.
|
|
// When all roots deny, the workspace root's error is returned; unlike
|
|
// sandbox.Scope.validate this does not prefer traversal violations — the
|
|
// engine layer reports those with full fidelity before tools run.
|
|
func resolveScopedPath(workspaceRoot string, scope PathScope, requestedPath string) (string, string, error) {
|
|
if requestedPath == "" || !filepath.IsAbs(requestedPath) || scope == nil {
|
|
return resolveWorkspacePath(workspaceRoot, requestedPath)
|
|
}
|
|
roots, err := scopedRoots(workspaceRoot, scope)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
var firstErr error
|
|
for index, root := range roots {
|
|
// Normalize platform-level symlinks (e.g. macOS /var -> /private/var)
|
|
// in the prefix outside this root only, leaving in-root components
|
|
// verbatim for the single-root symlink checks.
|
|
candidate := sandbox.NormalizePrefixForRoot(requestedPath, root)
|
|
absolute, relative, err := resolveWorkspacePath(root, candidate)
|
|
if err == nil {
|
|
if index > 0 {
|
|
// Extra-root matches report the absolute path: "relative to
|
|
// which root" is ambiguous downstream (ChangedFiles, cwd
|
|
// meta, display), and consumers document workspace-relative.
|
|
return absolute, absolute, nil
|
|
}
|
|
return absolute, relative, nil
|
|
}
|
|
if firstErr == nil {
|
|
firstErr = err
|
|
}
|
|
}
|
|
return "", "", firstErr
|
|
}
|
|
|
|
// resolveScopedTargetPath mirrors resolveWorkspaceTargetPath for write targets
|
|
// (the target may not exist yet) across all scope roots. When the matched root
|
|
// is an extra (non-workspace) root the second return value is the absolute
|
|
// path rather than a per-root-relative path; "relative to which root" is
|
|
// ambiguous for downstream consumers (ChangedFiles, cwd meta, display
|
|
// summaries) that document workspace-relative paths.
|
|
// When all roots deny, the workspace root's error is returned; unlike
|
|
// sandbox.Scope.validate this does not prefer traversal violations — the
|
|
// engine layer reports those with full fidelity before tools run.
|
|
func resolveScopedTargetPath(workspaceRoot string, scope PathScope, requestedPath string) (string, string, error) {
|
|
if requestedPath == "" || !filepath.IsAbs(requestedPath) || scope == nil {
|
|
return resolveWorkspaceTargetPath(workspaceRoot, requestedPath)
|
|
}
|
|
roots, err := scopedRoots(workspaceRoot, scope)
|
|
if err != nil {
|
|
return "", "", err
|
|
}
|
|
var firstErr error
|
|
for index, root := range roots {
|
|
// Normalize platform-level symlinks (e.g. macOS /var -> /private/var)
|
|
// in the prefix outside this root only; in-root components stay
|
|
// verbatim so the per-segment write-target symlink checks apply.
|
|
candidate := sandbox.NormalizePrefixForRoot(requestedPath, root)
|
|
absolute, relative, err := resolveWorkspaceTargetPath(root, candidate)
|
|
if err == nil {
|
|
if index > 0 {
|
|
// Extra-root matches report the absolute path: "relative to
|
|
// which root" is ambiguous downstream (ChangedFiles, cwd
|
|
// meta, display), and consumers document workspace-relative.
|
|
return absolute, absolute, nil
|
|
}
|
|
return absolute, relative, nil
|
|
}
|
|
if firstErr == nil {
|
|
firstErr = err
|
|
}
|
|
}
|
|
return "", "", firstErr
|
|
}
|
|
|
|
// recheckScopedWriteTarget mirrors recheckWorkspaceWriteTarget across roots.
|
|
// Prefix symlinks outside a root are resolved per root (macOS /var aliasing);
|
|
// symlinks INSIDE a root stay visible to the single-root checks, so a write
|
|
// target may resolve through a symlink only when its final location lies
|
|
// inside a DIFFERENT granted root — mirroring sandbox.Scope.validate's
|
|
// documented widening.
|
|
func recheckScopedWriteTarget(workspaceRoot string, scope PathScope, requestedPath string) error {
|
|
if requestedPath == "" || !filepath.IsAbs(requestedPath) || scope == nil {
|
|
return recheckWorkspaceWriteTarget(workspaceRoot, requestedPath)
|
|
}
|
|
roots, err := scopedRoots(workspaceRoot, scope)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
var firstErr error
|
|
for _, root := range roots {
|
|
candidate := sandbox.NormalizePrefixForRoot(requestedPath, root)
|
|
err := recheckWorkspaceWriteTarget(root, candidate)
|
|
if err == nil {
|
|
return nil
|
|
}
|
|
if firstErr == nil {
|
|
firstErr = err
|
|
}
|
|
}
|
|
return firstErr
|
|
}
|