fix(jail,tui): emit OSC 7 cwd and stop false-positive cd jail errors
#38: emit OSC 7 (ESC ]7;file://host/path) on TUI setup and /cd so terminals like kitty open new tabs/splits in the launch cwd instead of inheriting a stale extension-subprocess directory. Verified end-to-end against kitty 0.46.2. #39: stop blanket-rejecting cd into subdirectories of the sandbox root. CheckCommand now resolves the cd target and rejects only real escapes. Add Sandbox.DisplayPath to present jailed tool-result/error paths relative to root, reducing the absolute-path bias that pushed the model toward unjailing.
- commit
- bfd8c07cfa8ac29047fbce11349a6ec4d39c5ea6
- author
- patriceckhart · · 2026-06-17 15:15 UTC
- parent
- 5a4e492
- signature
- unsigned
6 files · +286 -7
M packages/agent/modes/interactive.go +14 -0
| @@ -533,6 +533,13 @@ func (i *Interactive) Run(ctx context.Context) error { | |||
| 533 | 533 | // chat as normal terminal flow/scrollback and redraws only the live | |
| 534 | 534 | // input/status block on normal typing. | |
| 535 | 535 | _, _ = term.Write([]byte(tui.SeqBracketedPasteOn + tui.SeqEnhancedKeyboardOn + tui.SeqResetScrollRegion + tui.SeqDeleteKittyImages + tui.SeqClearScreenNoHome + tui.SeqClearScrollback + tui.MoveTo(1, 1))) | |
| 536 | + | // Tell the terminal our working directory (OSC 7) so "new tab / | |
| 537 | + | // split here" opens in the launch cwd instead of inheriting a stale | |
| 538 | + | // directory from an extension subprocess (see issue #38). Harmless | |
| 539 | + | // on terminals that ignore it. | |
| 540 | + | if seq := tui.ReportCWD(i.cfg.CWD); seq != "" { | |
| 541 | + | _, _ = term.Write([]byte(seq)) | |
| 542 | + | } | |
| 536 | 543 | defer term.Write([]byte(tui.SeqResetScrollRegion + tui.SeqDeleteKittyImages + tui.SeqEnhancedKeyboardOff + tui.SeqBracketedPasteOff + tui.SeqShowCursor)) | |
| 537 | 544 | ||
| 538 | 545 | // Streaming pacer: drains buffered text deltas at a steady rate | |
| @@ -2463,6 +2470,13 @@ func (i *Interactive) ApplyChangedCWD(ag *core.Agent, provider, model, cwd strin | |||
| 2463 | 2470 | i.mu.Lock() | |
| 2464 | 2471 | i.agent = ag | |
| 2465 | 2472 | i.cfg.CWD = cwd | |
| 2473 | + | // Re-report the working directory to the terminal so "new tab here" | |
| 2474 | + | // tracks the /cd change (OSC 7, see issue #38). | |
| 2475 | + | if i.cfg.Terminal != nil { | |
| 2476 | + | if seq := tui.ReportCWD(cwd); seq != "" { | |
| 2477 | + | _, _ = i.cfg.Terminal.Write([]byte(seq)) | |
| 2478 | + | } | |
| 2479 | + | } | |
| 2466 | 2480 | i.cfg.Provider = provider | |
| 2467 | 2481 | i.cfg.Model = model | |
| 2468 | 2482 | i.toolCalls = map[string]*tui.ToolCallView{} | |
| @@ -533,6 +533,13 @@ func (i *Interactive) Run(ctx context.Context) error { | |||||
| 533 | // chat as normal terminal flow/scrollback and redraws only the live | 533 | // chat as normal terminal flow/scrollback and redraws only the live | ||
| 534 | // input/status block on normal typing. | 534 | // input/status block on normal typing. | ||
| 535 | _, _ = term.Write([]byte(tui.SeqBracketedPasteOn + tui.SeqEnhancedKeyboardOn + tui.SeqResetScrollRegion + tui.SeqDeleteKittyImages + tui.SeqClearScreenNoHome + tui.SeqClearScrollback + tui.MoveTo(1, 1))) | 535 | _, _ = term.Write([]byte(tui.SeqBracketedPasteOn + tui.SeqEnhancedKeyboardOn + tui.SeqResetScrollRegion + tui.SeqDeleteKittyImages + tui.SeqClearScreenNoHome + tui.SeqClearScrollback + tui.MoveTo(1, 1))) | ||
| 536 | + | // Tell the terminal our working directory (OSC 7) so "new tab / | |||
| 537 | + | // split here" opens in the launch cwd instead of inheriting a stale | |||
| 538 | + | // directory from an extension subprocess (see issue #38). Harmless | |||
| 539 | + | // on terminals that ignore it. | |||
| 540 | + | if seq := tui.ReportCWD(i.cfg.CWD); seq != "" { | |||
| 541 | + | _, _ = term.Write([]byte(seq)) | |||
| 542 | + | } | |||
| 536 | defer term.Write([]byte(tui.SeqResetScrollRegion + tui.SeqDeleteKittyImages + tui.SeqEnhancedKeyboardOff + tui.SeqBracketedPasteOff + tui.SeqShowCursor)) | 543 | defer term.Write([]byte(tui.SeqResetScrollRegion + tui.SeqDeleteKittyImages + tui.SeqEnhancedKeyboardOff + tui.SeqBracketedPasteOff + tui.SeqShowCursor)) | ||
| 537 | 544 | ||||
| 538 | // Streaming pacer: drains buffered text deltas at a steady rate | 545 | // Streaming pacer: drains buffered text deltas at a steady rate | ||
| @@ -2463,6 +2470,13 @@ func (i *Interactive) ApplyChangedCWD(ag *core.Agent, provider, model, cwd strin | |||||
| 2463 | i.mu.Lock() | 2470 | i.mu.Lock() | ||
| 2464 | i.agent = ag | 2471 | i.agent = ag | ||
| 2465 | i.cfg.CWD = cwd | 2472 | i.cfg.CWD = cwd | ||
| 2473 | + | // Re-report the working directory to the terminal so "new tab here" | |||
| 2474 | + | // tracks the /cd change (OSC 7, see issue #38). | |||
| 2475 | + | if i.cfg.Terminal != nil { | |||
| 2476 | + | if seq := tui.ReportCWD(cwd); seq != "" { | |||
| 2477 | + | _, _ = i.cfg.Terminal.Write([]byte(seq)) | |||
| 2478 | + | } | |||
| 2479 | + | } | |||
| 2466 | i.cfg.Provider = provider | 2480 | i.cfg.Provider = provider | ||
| 2467 | i.cfg.Model = model | 2481 | i.cfg.Model = model | ||
| 2468 | i.toolCalls = map[string]*tui.ToolCallView{} | 2482 | i.toolCalls = map[string]*tui.ToolCallView{} | ||
M packages/agent/tools/read.go +4 -1
| @@ -51,13 +51,16 @@ func (t *ReadTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 51 | 51 | if err := t.Sandbox.CheckPath(path); err != nil { | |
| 52 | 52 | return core.ToolResult{}, err | |
| 53 | 53 | } | |
| 54 | + | // What the model sees in errors: relative to the sandbox root when | |
| 55 | + | // jailed, so absolute paths stay out of the context window. | |
| 56 | + | shown := t.Sandbox.DisplayPath(path, a.Path) | |
| 54 | 57 | ||
| 55 | 58 | info, err := os.Stat(path) | |
| 56 | 59 | if err != nil { | |
| 57 | 60 | return core.ToolResult{}, err | |
| 58 | 61 | } | |
| 59 | 62 | if info.IsDir() { | |
| 60 | - | return core.ToolResult{}, fmt.Errorf("%s is a directory", path) | |
| 63 | + | return core.ToolResult{}, fmt.Errorf("%s is a directory", shown) | |
| 61 | 64 | } | |
| 62 | 65 | ||
| 63 | 66 | // Image handling. | |
| @@ -51,13 +51,16 @@ func (t *ReadTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 51 | if err := t.Sandbox.CheckPath(path); err != nil { | 51 | if err := t.Sandbox.CheckPath(path); err != nil { | ||
| 52 | return core.ToolResult{}, err | 52 | return core.ToolResult{}, err | ||
| 53 | } | 53 | } | ||
| 54 | + | // What the model sees in errors: relative to the sandbox root when | |||
| 55 | + | // jailed, so absolute paths stay out of the context window. | |||
| 56 | + | shown := t.Sandbox.DisplayPath(path, a.Path) | |||
| 54 | 57 | ||||
| 55 | info, err := os.Stat(path) | 58 | info, err := os.Stat(path) | ||
| 56 | if err != nil { | 59 | if err != nil { | ||
| 57 | return core.ToolResult{}, err | 60 | return core.ToolResult{}, err | ||
| 58 | } | 61 | } | ||
| 59 | if info.IsDir() { | 62 | if info.IsDir() { | ||
| 60 | - | return core.ToolResult{}, fmt.Errorf("%s is a directory", path) | 63 | + | return core.ToolResult{}, fmt.Errorf("%s is a directory", shown) |
| 61 | } | 64 | } | ||
| 62 | 65 | ||||
| 63 | // Image handling. | 66 | // Image handling. | ||
M packages/agent/tools/sandbox.go +114 -6
| @@ -2,6 +2,7 @@ package tools | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "fmt" | |
| 5 | + | "os" | |
| 5 | 6 | "path/filepath" | |
| 6 | 7 | "strings" | |
| 7 | 8 | "sync/atomic" | |
| @@ -56,6 +57,42 @@ func (s *Sandbox) CheckPath(path string) error { | |||
| 56 | 57 | return nil | |
| 57 | 58 | } | |
| 58 | 59 | ||
| 60 | + | // DisplayPath returns the path the model should see in tool results | |
| 61 | + | // and error messages. When jailed, an absolute path inside the | |
| 62 | + | // sandbox root is rewritten relative to that root ("./pkg/foo.go"), | |
| 63 | + | // which keeps absolute paths out of the context window so the model | |
| 64 | + | // is nudged toward relative paths instead of trying to escape the | |
| 65 | + | // jail (see issue #39). Paths outside root, unjailed sessions, and | |
| 66 | + | // already-relative inputs are returned unchanged. | |
| 67 | + | // | |
| 68 | + | // abs should be the resolved absolute path (resolvePath output); | |
| 69 | + | // given is the path exactly as the model supplied it, used as the | |
| 70 | + | // fallback when no better relative form is available. | |
| 71 | + | func (s *Sandbox) DisplayPath(abs, given string) string { | |
| 72 | + | if !s.Locked() { | |
| 73 | + | return given | |
| 74 | + | } | |
| 75 | + | rootAbs, err := canonical(s.Root) | |
| 76 | + | if err != nil { | |
| 77 | + | return given | |
| 78 | + | } | |
| 79 | + | target, err := canonicalOrParent(abs) | |
| 80 | + | if err != nil { | |
| 81 | + | return given | |
| 82 | + | } | |
| 83 | + | if !isUnder(rootAbs, target) { | |
| 84 | + | return given | |
| 85 | + | } | |
| 86 | + | rel, err := filepath.Rel(rootAbs, target) | |
| 87 | + | if err != nil || rel == "" || strings.HasPrefix(rel, "..") { | |
| 88 | + | return given | |
| 89 | + | } | |
| 90 | + | if rel == "." { | |
| 91 | + | return "." | |
| 92 | + | } | |
| 93 | + | return "./" + filepath.ToSlash(rel) | |
| 94 | + | } | |
| 95 | + | ||
| 59 | 96 | // CheckCommand applies a lightweight sanity check to a bash command | |
| 60 | 97 | // when jailed. We cannot fully sandbox a shell, but we can reject the | |
| 61 | 98 | // most obvious escapes so the model does not accidentally touch files | |
| @@ -81,19 +118,90 @@ func (s *Sandbox) CheckCommand(cmd string) error { | |||
| 81 | 118 | return fmt.Errorf("jailed: command contains banned pattern %q (use /unjail to disable)", b) | |
| 82 | 119 | } | |
| 83 | 120 | } | |
| 84 | - | // Heuristic: reject a leading `cd /` or `cd ~` that tries to move | |
| 85 | - | // the shell out of the sandbox. Note this only catches simple cases; | |
| 86 | - | // a determined adversary can still escape. This is a speed bump for | |
| 87 | - | // the model, not a security boundary. | |
| 121 | + | // Heuristic: reject a leading `cd` that tries to move the shell out | |
| 122 | + | // of the sandbox. We only reject when the target actually resolves | |
| 123 | + | // outside root; a `cd` into a subdirectory of root (even spelled as | |
| 124 | + | // an absolute path) is allowed, because the model frequently does | |
| 125 | + | // `cd /abs/path/inside/root && build` and blanket-rejecting that | |
| 126 | + | // wastes turns and nudges the model toward trying to break out. | |
| 127 | + | // Note this only catches simple cases; a determined adversary can | |
| 128 | + | // still escape. This is a speed bump for the model, not a security | |
| 129 | + | // boundary. | |
| 88 | 130 | first := strings.TrimSpace(strings.SplitN(cmd, ";", 2)[0]) | |
| 89 | 131 | first = strings.TrimSpace(strings.SplitN(first, "&&", 2)[0]) | |
| 90 | - | if strings.HasPrefix(first, "cd /") || strings.HasPrefix(first, "cd ~") || | |
| 91 | - | strings.HasPrefix(first, "cd $HOME") || strings.HasPrefix(first, "cd ..") { | |
| 132 | + | if target, ok := cdTarget(first); ok { | |
| 133 | + | if err := s.checkCDTarget(target); err != nil { | |
| 134 | + | return err | |
| 135 | + | } | |
| 136 | + | } | |
| 137 | + | return nil | |
| 138 | + | } | |
| 139 | + | ||
| 140 | + | // cdTarget extracts the destination of a leading `cd <dir>` command. | |
| 141 | + | // Returns ok=false when seg is not a `cd` invocation or has no explicit | |
| 142 | + | // target (bare `cd` / `cd -` go home / previous dir; we leave those to | |
| 143 | + | // the path checks on subsequent tool calls rather than guessing). | |
| 144 | + | func cdTarget(seg string) (string, bool) { | |
| 145 | + | seg = strings.TrimSpace(seg) | |
| 146 | + | if seg != "cd" && !strings.HasPrefix(seg, "cd ") { | |
| 147 | + | return "", false | |
| 148 | + | } | |
| 149 | + | arg := strings.TrimSpace(strings.TrimPrefix(seg, "cd")) | |
| 150 | + | if arg == "" || arg == "-" { | |
| 151 | + | return "", false | |
| 152 | + | } | |
| 153 | + | // Drop surrounding quotes if the model wrapped the path. | |
| 154 | + | if len(arg) >= 2 { | |
| 155 | + | if (arg[0] == '"' && arg[len(arg)-1] == '"') || (arg[0] == '\'' && arg[len(arg)-1] == '\'') { | |
| 156 | + | arg = arg[1 : len(arg)-1] | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | return arg, true | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | // checkCDTarget resolves a `cd` destination (relative to the sandbox | |
| 163 | + | // root, with ~ and $HOME expansion) and rejects it only when it lands | |
| 164 | + | // outside the root. | |
| 165 | + | func (s *Sandbox) checkCDTarget(dir string) error { | |
| 166 | + | rootAbs, err := canonical(s.Root) | |
| 167 | + | if err != nil { | |
| 168 | + | return fmt.Errorf("sandbox root: %w", err) | |
| 169 | + | } | |
| 170 | + | expanded := expandHome(dir) | |
| 171 | + | if !filepath.IsAbs(expanded) { | |
| 172 | + | // Relative targets (including `..`) resolve against the sandbox | |
| 173 | + | // root, which is the bash tool's working directory when jailed. | |
| 174 | + | expanded = filepath.Join(s.Root, expanded) | |
| 175 | + | } | |
| 176 | + | target, err := canonicalOrParent(expanded) | |
| 177 | + | if err != nil { | |
| 178 | + | return fmt.Errorf("sandbox path: %w", err) | |
| 179 | + | } | |
| 180 | + | if !isUnder(rootAbs, target) { | |
| 92 | 181 | return fmt.Errorf("jailed: cd outside sandbox root is not allowed (use /unjail to disable)") | |
| 93 | 182 | } | |
| 94 | 183 | return nil | |
| 95 | 184 | } | |
| 96 | 185 | ||
| 186 | + | // expandHome replaces a leading ~, ~/, or $HOME with the user's home | |
| 187 | + | // directory so cd-target resolution matches what the shell would do. | |
| 188 | + | func expandHome(p string) string { | |
| 189 | + | home, err := os.UserHomeDir() | |
| 190 | + | if err != nil || home == "" { | |
| 191 | + | return p | |
| 192 | + | } | |
| 193 | + | switch { | |
| 194 | + | case p == "~" || p == "$HOME": | |
| 195 | + | return home | |
| 196 | + | case strings.HasPrefix(p, "~/"): | |
| 197 | + | return filepath.Join(home, p[2:]) | |
| 198 | + | case strings.HasPrefix(p, "$HOME/"): | |
| 199 | + | return filepath.Join(home, p[len("$HOME/"):]) | |
| 200 | + | default: | |
| 201 | + | return p | |
| 202 | + | } | |
| 203 | + | } | |
| 204 | + | ||
| 97 | 205 | // canonical returns an absolute, symlink-resolved path. Errors on missing files. | |
| 98 | 206 | func canonical(p string) (string, error) { | |
| 99 | 207 | abs, err := filepath.Abs(p) | |
| @@ -2,6 +2,7 @@ package tools | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "fmt" | 4 | "fmt" | ||
| 5 | + | "os" | |||
| 5 | "path/filepath" | 6 | "path/filepath" | ||
| 6 | "strings" | 7 | "strings" | ||
| 7 | "sync/atomic" | 8 | "sync/atomic" | ||
| @@ -56,6 +57,42 @@ func (s *Sandbox) CheckPath(path string) error { | |||||
| 56 | return nil | 57 | return nil | ||
| 57 | } | 58 | } | ||
| 58 | 59 | ||||
| 60 | + | // DisplayPath returns the path the model should see in tool results | |||
| 61 | + | // and error messages. When jailed, an absolute path inside the | |||
| 62 | + | // sandbox root is rewritten relative to that root ("./pkg/foo.go"), | |||
| 63 | + | // which keeps absolute paths out of the context window so the model | |||
| 64 | + | // is nudged toward relative paths instead of trying to escape the | |||
| 65 | + | // jail (see issue #39). Paths outside root, unjailed sessions, and | |||
| 66 | + | // already-relative inputs are returned unchanged. | |||
| 67 | + | // | |||
| 68 | + | // abs should be the resolved absolute path (resolvePath output); | |||
| 69 | + | // given is the path exactly as the model supplied it, used as the | |||
| 70 | + | // fallback when no better relative form is available. | |||
| 71 | + | func (s *Sandbox) DisplayPath(abs, given string) string { | |||
| 72 | + | if !s.Locked() { | |||
| 73 | + | return given | |||
| 74 | + | } | |||
| 75 | + | rootAbs, err := canonical(s.Root) | |||
| 76 | + | if err != nil { | |||
| 77 | + | return given | |||
| 78 | + | } | |||
| 79 | + | target, err := canonicalOrParent(abs) | |||
| 80 | + | if err != nil { | |||
| 81 | + | return given | |||
| 82 | + | } | |||
| 83 | + | if !isUnder(rootAbs, target) { | |||
| 84 | + | return given | |||
| 85 | + | } | |||
| 86 | + | rel, err := filepath.Rel(rootAbs, target) | |||
| 87 | + | if err != nil || rel == "" || strings.HasPrefix(rel, "..") { | |||
| 88 | + | return given | |||
| 89 | + | } | |||
| 90 | + | if rel == "." { | |||
| 91 | + | return "." | |||
| 92 | + | } | |||
| 93 | + | return "./" + filepath.ToSlash(rel) | |||
| 94 | + | } | |||
| 95 | + | ||||
| 59 | // CheckCommand applies a lightweight sanity check to a bash command | 96 | // CheckCommand applies a lightweight sanity check to a bash command | ||
| 60 | // when jailed. We cannot fully sandbox a shell, but we can reject the | 97 | // when jailed. We cannot fully sandbox a shell, but we can reject the | ||
| 61 | // most obvious escapes so the model does not accidentally touch files | 98 | // most obvious escapes so the model does not accidentally touch files | ||
| @@ -81,19 +118,90 @@ func (s *Sandbox) CheckCommand(cmd string) error { | |||||
| 81 | return fmt.Errorf("jailed: command contains banned pattern %q (use /unjail to disable)", b) | 118 | return fmt.Errorf("jailed: command contains banned pattern %q (use /unjail to disable)", b) | ||
| 82 | } | 119 | } | ||
| 83 | } | 120 | } | ||
| 84 | - | // Heuristic: reject a leading `cd /` or `cd ~` that tries to move | 121 | + | // Heuristic: reject a leading `cd` that tries to move the shell out |
| 85 | - | // the shell out of the sandbox. Note this only catches simple cases; | 122 | + | // of the sandbox. We only reject when the target actually resolves |
| 86 | - | // a determined adversary can still escape. This is a speed bump for | 123 | + | // outside root; a `cd` into a subdirectory of root (even spelled as |
| 87 | - | // the model, not a security boundary. | 124 | + | // an absolute path) is allowed, because the model frequently does |
| 125 | + | // `cd /abs/path/inside/root && build` and blanket-rejecting that | |||
| 126 | + | // wastes turns and nudges the model toward trying to break out. | |||
| 127 | + | // Note this only catches simple cases; a determined adversary can | |||
| 128 | + | // still escape. This is a speed bump for the model, not a security | |||
| 129 | + | // boundary. | |||
| 88 | first := strings.TrimSpace(strings.SplitN(cmd, ";", 2)[0]) | 130 | first := strings.TrimSpace(strings.SplitN(cmd, ";", 2)[0]) | ||
| 89 | first = strings.TrimSpace(strings.SplitN(first, "&&", 2)[0]) | 131 | first = strings.TrimSpace(strings.SplitN(first, "&&", 2)[0]) | ||
| 90 | - | if strings.HasPrefix(first, "cd /") || strings.HasPrefix(first, "cd ~") || | 132 | + | if target, ok := cdTarget(first); ok { |
| 91 | - | strings.HasPrefix(first, "cd $HOME") || strings.HasPrefix(first, "cd ..") { | 133 | + | if err := s.checkCDTarget(target); err != nil { |
| 134 | + | return err | |||
| 135 | + | } | |||
| 136 | + | } | |||
| 137 | + | return nil | |||
| 138 | + | } | |||
| 139 | + | ||||
| 140 | + | // cdTarget extracts the destination of a leading `cd <dir>` command. | |||
| 141 | + | // Returns ok=false when seg is not a `cd` invocation or has no explicit | |||
| 142 | + | // target (bare `cd` / `cd -` go home / previous dir; we leave those to | |||
| 143 | + | // the path checks on subsequent tool calls rather than guessing). | |||
| 144 | + | func cdTarget(seg string) (string, bool) { | |||
| 145 | + | seg = strings.TrimSpace(seg) | |||
| 146 | + | if seg != "cd" && !strings.HasPrefix(seg, "cd ") { | |||
| 147 | + | return "", false | |||
| 148 | + | } | |||
| 149 | + | arg := strings.TrimSpace(strings.TrimPrefix(seg, "cd")) | |||
| 150 | + | if arg == "" || arg == "-" { | |||
| 151 | + | return "", false | |||
| 152 | + | } | |||
| 153 | + | // Drop surrounding quotes if the model wrapped the path. | |||
| 154 | + | if len(arg) >= 2 { | |||
| 155 | + | if (arg[0] == '"' && arg[len(arg)-1] == '"') || (arg[0] == '\'' && arg[len(arg)-1] == '\'') { | |||
| 156 | + | arg = arg[1 : len(arg)-1] | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | return arg, true | |||
| 160 | + | } | |||
| 161 | + | ||||
| 162 | + | // checkCDTarget resolves a `cd` destination (relative to the sandbox | |||
| 163 | + | // root, with ~ and $HOME expansion) and rejects it only when it lands | |||
| 164 | + | // outside the root. | |||
| 165 | + | func (s *Sandbox) checkCDTarget(dir string) error { | |||
| 166 | + | rootAbs, err := canonical(s.Root) | |||
| 167 | + | if err != nil { | |||
| 168 | + | return fmt.Errorf("sandbox root: %w", err) | |||
| 169 | + | } | |||
| 170 | + | expanded := expandHome(dir) | |||
| 171 | + | if !filepath.IsAbs(expanded) { | |||
| 172 | + | // Relative targets (including `..`) resolve against the sandbox | |||
| 173 | + | // root, which is the bash tool's working directory when jailed. | |||
| 174 | + | expanded = filepath.Join(s.Root, expanded) | |||
| 175 | + | } | |||
| 176 | + | target, err := canonicalOrParent(expanded) | |||
| 177 | + | if err != nil { | |||
| 178 | + | return fmt.Errorf("sandbox path: %w", err) | |||
| 179 | + | } | |||
| 180 | + | if !isUnder(rootAbs, target) { | |||
| 92 | return fmt.Errorf("jailed: cd outside sandbox root is not allowed (use /unjail to disable)") | 181 | return fmt.Errorf("jailed: cd outside sandbox root is not allowed (use /unjail to disable)") | ||
| 93 | } | 182 | } | ||
| 94 | return nil | 183 | return nil | ||
| 95 | } | 184 | } | ||
| 96 | 185 | ||||
| 186 | + | // expandHome replaces a leading ~, ~/, or $HOME with the user's home | |||
| 187 | + | // directory so cd-target resolution matches what the shell would do. | |||
| 188 | + | func expandHome(p string) string { | |||
| 189 | + | home, err := os.UserHomeDir() | |||
| 190 | + | if err != nil || home == "" { | |||
| 191 | + | return p | |||
| 192 | + | } | |||
| 193 | + | switch { | |||
| 194 | + | case p == "~" || p == "$HOME": | |||
| 195 | + | return home | |||
| 196 | + | case strings.HasPrefix(p, "~/"): | |||
| 197 | + | return filepath.Join(home, p[2:]) | |||
| 198 | + | case strings.HasPrefix(p, "$HOME/"): | |||
| 199 | + | return filepath.Join(home, p[len("$HOME/"):]) | |||
| 200 | + | default: | |||
| 201 | + | return p | |||
| 202 | + | } | |||
| 203 | + | } | |||
| 204 | + | ||||
| 97 | // canonical returns an absolute, symlink-resolved path. Errors on missing files. | 205 | // canonical returns an absolute, symlink-resolved path. Errors on missing files. | ||
| 98 | func canonical(p string) (string, error) { | 206 | func canonical(p string) (string, error) { | ||
| 99 | abs, err := filepath.Abs(p) | 207 | abs, err := filepath.Abs(p) | ||
M packages/agent/tools/sandbox_test.go +67 -0
| @@ -56,6 +56,73 @@ func TestSandboxCommandBanned(t *testing.T) { | |||
| 56 | 56 | } | |
| 57 | 57 | } | |
| 58 | 58 | ||
| 59 | + | // TestSandboxAllowsCDIntoSubdir is the regression for issue #39: a `cd` | |
| 60 | + | // into a subdirectory of the sandbox root, spelled as an absolute path, | |
| 61 | + | // must be allowed. The old guard rejected any `cd /...` outright, which | |
| 62 | + | // wasted turns and nudged the model toward trying to break out of jail. | |
| 63 | + | func TestSandboxAllowsCDIntoSubdir(t *testing.T) { | |
| 64 | + | root := t.TempDir() | |
| 65 | + | sub := filepath.Join(root, "packages", "provider") | |
| 66 | + | if err := os.MkdirAll(sub, 0o755); err != nil { | |
| 67 | + | t.Fatal(err) | |
| 68 | + | } | |
| 69 | + | sb := NewSandbox(root) | |
| 70 | + | sb.Lock() | |
| 71 | + | ||
| 72 | + | allowed := []string{ | |
| 73 | + | "cd " + sub + " && go build ./...", | |
| 74 | + | "cd " + root + " && go build ./...", | |
| 75 | + | "cd " + root, // bare cd to root | |
| 76 | + | "cd packages/provider && go build", | |
| 77 | + | "cd \"" + sub + "\" && ls", // quoted absolute path | |
| 78 | + | } | |
| 79 | + | for _, c := range allowed { | |
| 80 | + | if err := sb.CheckCommand(c); err != nil { | |
| 81 | + | t.Fatalf("expected %q to be allowed: %v", c, err) | |
| 82 | + | } | |
| 83 | + | } | |
| 84 | + | ||
| 85 | + | blocked := []string{ | |
| 86 | + | "cd /etc", | |
| 87 | + | "cd / && ls", | |
| 88 | + | "cd ..", // parent of root escapes | |
| 89 | + | "cd " + filepath.Dir(root), // explicit parent | |
| 90 | + | } | |
| 91 | + | for _, c := range blocked { | |
| 92 | + | if err := sb.CheckCommand(c); err == nil { | |
| 93 | + | t.Fatalf("expected %q to be blocked", c) | |
| 94 | + | } | |
| 95 | + | } | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | // TestSandboxDisplayPath covers issue #39's secondary cause: tool | |
| 99 | + | // results / errors should present paths relative to the sandbox root | |
| 100 | + | // when jailed, so the model isn't biased toward absolute paths. | |
| 101 | + | func TestSandboxDisplayPath(t *testing.T) { | |
| 102 | + | root := t.TempDir() | |
| 103 | + | sub := filepath.Join(root, "pkg", "foo.go") | |
| 104 | + | outside := filepath.Join(t.TempDir(), "x.go") | |
| 105 | + | ||
| 106 | + | sb := NewSandbox(root) | |
| 107 | + | ||
| 108 | + | // Unlocked: returns the given form verbatim. | |
| 109 | + | if got := sb.DisplayPath(sub, "pkg/foo.go"); got != "pkg/foo.go" { | |
| 110 | + | t.Fatalf("unlocked DisplayPath = %q; want verbatim", got) | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | sb.Lock() | |
| 114 | + | if got := sb.DisplayPath(sub, sub); got != "./pkg/foo.go" { | |
| 115 | + | t.Fatalf("DisplayPath(abs inside) = %q; want ./pkg/foo.go", got) | |
| 116 | + | } | |
| 117 | + | if got := sb.DisplayPath(root, root); got != "." { | |
| 118 | + | t.Fatalf("DisplayPath(root) = %q; want .", got) | |
| 119 | + | } | |
| 120 | + | // Outside root: fall back to the given form (don't fabricate a path). | |
| 121 | + | if got := sb.DisplayPath(outside, "x.go"); got != "x.go" { | |
| 122 | + | t.Fatalf("DisplayPath(outside) = %q; want given fallback", got) | |
| 123 | + | } | |
| 124 | + | } | |
| 125 | + | ||
| 59 | 126 | func TestReadToolRejectsOutsideWhenLocked(t *testing.T) { | |
| 60 | 127 | root := t.TempDir() | |
| 61 | 128 | outside := t.TempDir() | |
| @@ -56,6 +56,73 @@ func TestSandboxCommandBanned(t *testing.T) { | |||||
| 56 | } | 56 | } | ||
| 57 | } | 57 | } | ||
| 58 | 58 | ||||
| 59 | + | // TestSandboxAllowsCDIntoSubdir is the regression for issue #39: a `cd` | |||
| 60 | + | // into a subdirectory of the sandbox root, spelled as an absolute path, | |||
| 61 | + | // must be allowed. The old guard rejected any `cd /...` outright, which | |||
| 62 | + | // wasted turns and nudged the model toward trying to break out of jail. | |||
| 63 | + | func TestSandboxAllowsCDIntoSubdir(t *testing.T) { | |||
| 64 | + | root := t.TempDir() | |||
| 65 | + | sub := filepath.Join(root, "packages", "provider") | |||
| 66 | + | if err := os.MkdirAll(sub, 0o755); err != nil { | |||
| 67 | + | t.Fatal(err) | |||
| 68 | + | } | |||
| 69 | + | sb := NewSandbox(root) | |||
| 70 | + | sb.Lock() | |||
| 71 | + | ||||
| 72 | + | allowed := []string{ | |||
| 73 | + | "cd " + sub + " && go build ./...", | |||
| 74 | + | "cd " + root + " && go build ./...", | |||
| 75 | + | "cd " + root, // bare cd to root | |||
| 76 | + | "cd packages/provider && go build", | |||
| 77 | + | "cd \"" + sub + "\" && ls", // quoted absolute path | |||
| 78 | + | } | |||
| 79 | + | for _, c := range allowed { | |||
| 80 | + | if err := sb.CheckCommand(c); err != nil { | |||
| 81 | + | t.Fatalf("expected %q to be allowed: %v", c, err) | |||
| 82 | + | } | |||
| 83 | + | } | |||
| 84 | + | ||||
| 85 | + | blocked := []string{ | |||
| 86 | + | "cd /etc", | |||
| 87 | + | "cd / && ls", | |||
| 88 | + | "cd ..", // parent of root escapes | |||
| 89 | + | "cd " + filepath.Dir(root), // explicit parent | |||
| 90 | + | } | |||
| 91 | + | for _, c := range blocked { | |||
| 92 | + | if err := sb.CheckCommand(c); err == nil { | |||
| 93 | + | t.Fatalf("expected %q to be blocked", c) | |||
| 94 | + | } | |||
| 95 | + | } | |||
| 96 | + | } | |||
| 97 | + | ||||
| 98 | + | // TestSandboxDisplayPath covers issue #39's secondary cause: tool | |||
| 99 | + | // results / errors should present paths relative to the sandbox root | |||
| 100 | + | // when jailed, so the model isn't biased toward absolute paths. | |||
| 101 | + | func TestSandboxDisplayPath(t *testing.T) { | |||
| 102 | + | root := t.TempDir() | |||
| 103 | + | sub := filepath.Join(root, "pkg", "foo.go") | |||
| 104 | + | outside := filepath.Join(t.TempDir(), "x.go") | |||
| 105 | + | ||||
| 106 | + | sb := NewSandbox(root) | |||
| 107 | + | ||||
| 108 | + | // Unlocked: returns the given form verbatim. | |||
| 109 | + | if got := sb.DisplayPath(sub, "pkg/foo.go"); got != "pkg/foo.go" { | |||
| 110 | + | t.Fatalf("unlocked DisplayPath = %q; want verbatim", got) | |||
| 111 | + | } | |||
| 112 | + | ||||
| 113 | + | sb.Lock() | |||
| 114 | + | if got := sb.DisplayPath(sub, sub); got != "./pkg/foo.go" { | |||
| 115 | + | t.Fatalf("DisplayPath(abs inside) = %q; want ./pkg/foo.go", got) | |||
| 116 | + | } | |||
| 117 | + | if got := sb.DisplayPath(root, root); got != "." { | |||
| 118 | + | t.Fatalf("DisplayPath(root) = %q; want .", got) | |||
| 119 | + | } | |||
| 120 | + | // Outside root: fall back to the given form (don't fabricate a path). | |||
| 121 | + | if got := sb.DisplayPath(outside, "x.go"); got != "x.go" { | |||
| 122 | + | t.Fatalf("DisplayPath(outside) = %q; want given fallback", got) | |||
| 123 | + | } | |||
| 124 | + | } | |||
| 125 | + | ||||
| 59 | func TestReadToolRejectsOutsideWhenLocked(t *testing.T) { | 126 | func TestReadToolRejectsOutsideWhenLocked(t *testing.T) { | ||
| 60 | root := t.TempDir() | 127 | root := t.TempDir() | ||
| 61 | outside := t.TempDir() | 128 | outside := t.TempDir() | ||
A packages/tui/report_cwd_test.go +42 -0
| @@ -0,0 +1,42 @@ | |||
| 1 | + | package tui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "os" | |
| 5 | + | "strings" | |
| 6 | + | "testing" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestReportCWDEmptyIsBlank(t *testing.T) { | |
| 10 | + | if got := ReportCWD(""); got != "" { | |
| 11 | + | t.Fatalf("ReportCWD(\"\") = %q; want empty", got) | |
| 12 | + | } | |
| 13 | + | } | |
| 14 | + | ||
| 15 | + | func TestReportCWDFormat(t *testing.T) { | |
| 16 | + | host, _ := os.Hostname() | |
| 17 | + | got := ReportCWD("/tmp/some dir/work") | |
| 18 | + | if !strings.HasPrefix(got, "\x1b]7;file://"+host+"/") { | |
| 19 | + | t.Fatalf("missing OSC 7 prefix: %q", got) | |
| 20 | + | } | |
| 21 | + | if !strings.HasSuffix(got, "\x07") { | |
| 22 | + | t.Fatalf("missing BEL terminator: %q", got) | |
| 23 | + | } | |
| 24 | + | // Spaces must be percent-encoded, slashes preserved. | |
| 25 | + | if !strings.Contains(got, "/tmp/some%20dir/work") { | |
| 26 | + | t.Fatalf("path not encoded as expected: %q", got) | |
| 27 | + | } | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | func TestOSC7EscapePath(t *testing.T) { | |
| 31 | + | cases := map[string]string{ | |
| 32 | + | "/a/b": "/a/b", | |
| 33 | + | "/a b/c": "/a%20b/c", | |
| 34 | + | "/weird#?&": "/weird%23%3F%26", | |
| 35 | + | "/unreserved-_.~": "/unreserved-_.~", | |
| 36 | + | } | |
| 37 | + | for in, want := range cases { | |
| 38 | + | if got := osc7EscapePath(in); got != want { | |
| 39 | + | t.Errorf("osc7EscapePath(%q) = %q; want %q", in, got, want) | |
| 40 | + | } | |
| 41 | + | } | |
| 42 | + | } | |
| @@ -0,0 +1,42 @@ | |||||
| 1 | + | package tui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "os" | |||
| 5 | + | "strings" | |||
| 6 | + | "testing" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestReportCWDEmptyIsBlank(t *testing.T) { | |||
| 10 | + | if got := ReportCWD(""); got != "" { | |||
| 11 | + | t.Fatalf("ReportCWD(\"\") = %q; want empty", got) | |||
| 12 | + | } | |||
| 13 | + | } | |||
| 14 | + | ||||
| 15 | + | func TestReportCWDFormat(t *testing.T) { | |||
| 16 | + | host, _ := os.Hostname() | |||
| 17 | + | got := ReportCWD("/tmp/some dir/work") | |||
| 18 | + | if !strings.HasPrefix(got, "\x1b]7;file://"+host+"/") { | |||
| 19 | + | t.Fatalf("missing OSC 7 prefix: %q", got) | |||
| 20 | + | } | |||
| 21 | + | if !strings.HasSuffix(got, "\x07") { | |||
| 22 | + | t.Fatalf("missing BEL terminator: %q", got) | |||
| 23 | + | } | |||
| 24 | + | // Spaces must be percent-encoded, slashes preserved. | |||
| 25 | + | if !strings.Contains(got, "/tmp/some%20dir/work") { | |||
| 26 | + | t.Fatalf("path not encoded as expected: %q", got) | |||
| 27 | + | } | |||
| 28 | + | } | |||
| 29 | + | ||||
| 30 | + | func TestOSC7EscapePath(t *testing.T) { | |||
| 31 | + | cases := map[string]string{ | |||
| 32 | + | "/a/b": "/a/b", | |||
| 33 | + | "/a b/c": "/a%20b/c", | |||
| 34 | + | "/weird#?&": "/weird%23%3F%26", | |||
| 35 | + | "/unreserved-_.~": "/unreserved-_.~", | |||
| 36 | + | } | |||
| 37 | + | for in, want := range cases { | |||
| 38 | + | if got := osc7EscapePath(in); got != want { | |||
| 39 | + | t.Errorf("osc7EscapePath(%q) = %q; want %q", in, got, want) | |||
| 40 | + | } | |||
| 41 | + | } | |||
| 42 | + | } | |||
M packages/tui/terminal.go +45 -0
| @@ -3,6 +3,7 @@ package tui | |||
| 3 | 3 | import ( | |
| 4 | 4 | "io" | |
| 5 | 5 | "os" | |
| 6 | + | "path/filepath" | |
| 6 | 7 | "time" | |
| 7 | 8 | ||
| 8 | 9 | "golang.org/x/term" | |
| @@ -133,6 +134,50 @@ const ( | |||
| 133 | 134 | SeqEraseToEnd = "\x1b[J" | |
| 134 | 135 | ) | |
| 135 | 136 | ||
| 137 | + | // ReportCWD returns the OSC 7 sequence that tells the terminal the | |
| 138 | + | // current working directory, formatted as a file URL. Terminals such | |
| 139 | + | // as kitty, WezTerm, iTerm2, and GNOME Terminal use this to open new | |
| 140 | + | // tabs / splits in the same directory. Returns "" for an empty path so | |
| 141 | + | // callers can write it unconditionally. The path is percent-encoded | |
| 142 | + | // per RFC 3986 (only unreserved characters and the path separator are | |
| 143 | + | // left bare) and prefixed with the local hostname. | |
| 144 | + | func ReportCWD(dir string) string { | |
| 145 | + | if dir == "" { | |
| 146 | + | return "" | |
| 147 | + | } | |
| 148 | + | abs, err := filepath.Abs(dir) | |
| 149 | + | if err != nil { | |
| 150 | + | abs = dir | |
| 151 | + | } | |
| 152 | + | host, _ := os.Hostname() | |
| 153 | + | return "\x1b]7;file://" + host + osc7EscapePath(abs) + "\x07" | |
| 154 | + | } | |
| 155 | + | ||
| 156 | + | // osc7EscapePath percent-encodes a filesystem path for an OSC 7 file | |
| 157 | + | // URL. Forward slashes (the URL path separator) and the unreserved set | |
| 158 | + | // (A-Z a-z 0-9 - _ . ~) pass through; everything else is %XX-encoded. | |
| 159 | + | // On Windows the backslash separators are normalised to forward | |
| 160 | + | // slashes and a leading slash is added so "C:\foo" becomes "/C:/foo". | |
| 161 | + | func osc7EscapePath(p string) string { | |
| 162 | + | p = filepath.ToSlash(p) | |
| 163 | + | if len(p) > 1 && p[1] == ':' { | |
| 164 | + | p = "/" + p | |
| 165 | + | } | |
| 166 | + | const hex = "0123456789ABCDEF" | |
| 167 | + | var b []byte | |
| 168 | + | for i := 0; i < len(p); i++ { | |
| 169 | + | c := p[i] | |
| 170 | + | unreserved := (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || | |
| 171 | + | (c >= '0' && c <= '9') || c == '-' || c == '_' || c == '.' || c == '~' || c == '/' | |
| 172 | + | if unreserved { | |
| 173 | + | b = append(b, c) | |
| 174 | + | continue | |
| 175 | + | } | |
| 176 | + | b = append(b, '%', hex[c>>4], hex[c&0x0f]) | |
| 177 | + | } | |
| 178 | + | return string(b) | |
| 179 | + | } | |
| 180 | + | ||
| 136 | 181 | // SetScrollRegion returns the DECSTBM sequence to set the terminal's | |
| 137 | 182 | // scroll region (1-indexed, inclusive). Lines that scroll out of this | |
| 138 | 183 | // region still flow into the terminal's scrollback, but content above | |
| @@ -3,6 +3,7 @@ package tui | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "io" | 4 | "io" | ||
| 5 | "os" | 5 | "os" | ||
| 6 | + | "path/filepath" | |||
| 6 | "time" | 7 | "time" | ||
| 7 | 8 | ||||
| 8 | "golang.org/x/term" | 9 | "golang.org/x/term" | ||
| @@ -133,6 +134,50 @@ const ( | |||||
| 133 | SeqEraseToEnd = "\x1b[J" | 134 | SeqEraseToEnd = "\x1b[J" | ||
| 134 | ) | 135 | ) | ||
| 135 | 136 | ||||
| 137 | + | // ReportCWD returns the OSC 7 sequence that tells the terminal the | |||
| 138 | + | // current working directory, formatted as a file URL. Terminals such | |||
| 139 | + | // as kitty, WezTerm, iTerm2, and GNOME Terminal use this to open new | |||
| 140 | + | // tabs / splits in the same directory. Returns "" for an empty path so | |||
| 141 | + | // callers can write it unconditionally. The path is percent-encoded | |||
| 142 | + | // per RFC 3986 (only unreserved characters and the path separator are | |||
| 143 | + | // left bare) and prefixed with the local hostname. | |||
| 144 | + | func ReportCWD(dir string) string { | |||
| 145 | + | if dir == "" { | |||
| 146 | + | return "" | |||
| 147 | + | } | |||
| 148 | + | abs, err := filepath.Abs(dir) | |||
| 149 | + | if err != nil { | |||
| 150 | + | abs = dir | |||
| 151 | + | } | |||
| 152 | + | host, _ := os.Hostname() | |||
| 153 | + | return "\x1b]7;file://" + host + osc7EscapePath(abs) + "\x07" | |||
| 154 | + | } | |||
| 155 | + | ||||
| 156 | + | // osc7EscapePath percent-encodes a filesystem path for an OSC 7 file | |||
| 157 | + | // URL. Forward slashes (the URL path separator) and the unreserved set | |||
| 158 | + | // (A-Z a-z 0-9 - _ . ~) pass through; everything else is %XX-encoded. | |||
| 159 | + | // On Windows the backslash separators are normalised to forward | |||
| 160 | + | // slashes and a leading slash is added so "C:\foo" becomes "/C:/foo". | |||
| 161 | + | func osc7EscapePath(p string) string { | |||
| 162 | + | p = filepath.ToSlash(p) | |||
| 163 | + | if len(p) > 1 && p[1] == ':' { | |||
| 164 | + | p = "/" + p | |||
| 165 | + | } | |||
| 166 | + | const hex = "0123456789ABCDEF" | |||
| 167 | + | var b []byte | |||
| 168 | + | for i := 0; i < len(p); i++ { | |||
| 169 | + | c := p[i] | |||
| 170 | + | unreserved := (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || | |||
| 171 | + | (c >= '0' && c <= '9') || c == '-' || c == '_' || c == '.' || c == '~' || c == '/' | |||
| 172 | + | if unreserved { | |||
| 173 | + | b = append(b, c) | |||
| 174 | + | continue | |||
| 175 | + | } | |||
| 176 | + | b = append(b, '%', hex[c>>4], hex[c&0x0f]) | |||
| 177 | + | } | |||
| 178 | + | return string(b) | |||
| 179 | + | } | |||
| 180 | + | ||||
| 136 | // SetScrollRegion returns the DECSTBM sequence to set the terminal's | 181 | // SetScrollRegion returns the DECSTBM sequence to set the terminal's | ||
| 137 | // scroll region (1-indexed, inclusive). Lines that scroll out of this | 182 | // scroll region (1-indexed, inclusive). Lines that scroll out of this | ||
| 138 | // region still flow into the terminal's scrollback, but content above | 183 | // region still flow into the terminal's scrollback, but content above | ||