From b9dd4f1a8ac76b8c66d139d0745aadae691c4891 Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 18:27:01 -0600 Subject: [PATCH 1/4] File paths: a leading env reference resolves where it points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing on the way to a file tool is a shell, so "$env:USERPROFILE\.gitconfig" was joined onto the workspace as a literal "$env:USERPROFILE" directory: the read failed on a path the model never wrote, after the risk check had scored (and the user approved) the home directory it meant. Resolve now expands a leading $env:NAME, ${NAME}, $NAME or %NAME% to its value, and ~\ on Windows like ~/. Only a variable that is set, and only when a separator or the end follows — "Foo$Bar.class" and "$Recycle.Bin" are file names. The jail and the deny floor run on the expanded path. Fixes #389. Co-Authored-By: Claude Opus 5.5 --- internal/policy/policy.go | 32 ++++++++++++++++++++-- internal/policy/policy_test.go | 50 ++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 61c6226..0924b19 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -467,7 +467,7 @@ func (e *Engine) IsSecret(abs string) bool { return fileMatchFold(secretReadFloo // under the workspace, silently creating a literal "~" directory. The jail check // still runs after expansion, so ~ can't be used to escape it. func (e *Engine) Resolve(path string) (string, error) { - abs := expandTilde(path) + abs := expandEnvRef(expandTilde(path)) if !filepath.IsAbs(abs) { abs = filepath.Join(e.base(), abs) } @@ -507,7 +507,7 @@ func (e *Engine) Resolve(path string) (string, error) { // shell behavior so the model's "~/file" lands in $HOME instead of a literal "~" // directory under the workspace. func expandTilde(p string) string { - if p == "~" || strings.HasPrefix(p, "~/") { + if p == "~" || strings.HasPrefix(p, "~/") || strings.HasPrefix(p, "~"+string(filepath.Separator)) { // ~\ on Windows if home, err := os.UserHomeDir(); err == nil { return filepath.Join(home, strings.TrimPrefix(p, "~")) } @@ -515,6 +515,34 @@ func expandTilde(p string) string { return p } +// envRef matches an environment reference at the start of a path, in each +// shell's own spelling: PowerShell's $env:NAME, POSIX ${NAME} and $NAME, and +// cmd's %NAME% (whose names may hold parentheses: %ProgramFiles(x86)%). +var envRef = regexp.MustCompile(`^(?:\$env:([A-Za-z_][A-Za-z0-9_]*)|\$\{([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)|%([A-Za-z_][A-Za-z0-9_()]*)%)`) + +// expandEnvRef turns a leading environment reference into its value. Nothing on +// the way to a file tool is a shell, so "$env:USERPROFILE\.gitconfig" was joined +// onto the workspace as a literal "$env:USERPROFILE" directory: the read failed +// with a path the model never wrote, and the risk check had scored the home +// directory it meant. Only a variable that is set, and only when a separator or +// the end follows it — "$Recycle.Bin" and "Foo$Bar.class" are file names, and a +// reference to nothing is left for the path to fail on as written. +func expandEnvRef(p string) string { + m := envRef.FindStringSubmatch(p) + if m == nil { + return p + } + rest := p[len(m[0]):] + if rest != "" && rest[0] != '/' && rest[0] != filepath.Separator { + return p + } + name := m[1] + m[2] + m[3] + m[4] // exactly one group matched + if v := os.Getenv(name); v != "" { + return v + rest + } + return p +} + // maxSymlinkChase bounds resolveSymlinks' own-target chase (see below) against // a symlink cycle (a -> b -> a) looping forever. const maxSymlinkChase = 20 diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 29fbe61..62161c7 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -45,6 +45,50 @@ func TestResolveJailEscapeErrorExplainsItsAHardBoundary(t *testing.T) { } } +// A path opening with an environment reference lands where the variable points, +// in each shell's spelling — not in a literal "$env:USERPROFILE" directory under +// the workspace — and still inside the jail's rules. +func TestResolveExpandsALeadingEnvReference(t *testing.T) { + home := t.TempDir() + t.Setenv("IPCO_TEST_HOME", home) + c := config.Default() + c.Workspace = home + c.File = config.FilePolicy{Default: "allow", Jail: "."} + e := eng(t, c) + want := filepath.Join(home, "note.json") + for _, p := range []string{ + "$env:IPCO_TEST_HOME/note.json", + "${IPCO_TEST_HOME}/note.json", + "$IPCO_TEST_HOME/note.json", + "%IPCO_TEST_HOME%/note.json", + "$env:IPCO_TEST_HOME" + string(filepath.Separator) + "note.json", + } { + got, err := e.Resolve(p) + if err != nil || got != want { + t.Errorf("Resolve(%q) = %q, %v; want %q", p, got, err, want) + } + } + // Not references: a file name with a $ in it, a name followed by more name, + // and a variable that is not set — all stay as written, in the workspace. + for _, p := range []string{"Foo$Bar.class", "$IPCO_TEST_HOME.bak", "$IPCO_NOT_SET_ANYWHERE/note.json", "$Recycle.Bin"} { + got, err := e.Resolve(p) + if err != nil || got != filepath.Join(home, p) { + t.Errorf("Resolve(%q) = %q, %v; want it left as a name in the workspace", p, got, err) + } + } + // The jail still holds: a reference to somewhere outside it is outside it. + sub := filepath.Join(home, "sub") + if err := os.Mkdir(sub, 0o755); err != nil { + t.Fatal(err) + } + c2 := config.Default() + c2.Workspace = sub + c2.File = config.FilePolicy{Default: "allow", Jail: "."} + if _, err := eng(t, c2).Resolve("$IPCO_TEST_HOME/escape.json"); err == nil { + t.Error("an env reference must not escape a sub-directory jail") + } +} + func TestResolveExpandsTilde(t *testing.T) { home := t.TempDir() setHome(t, home) @@ -60,6 +104,12 @@ func TestResolveExpandsTilde(t *testing.T) { if strings.Contains(got, "~") || filepath.Base(got) != "note.json" { t.Errorf("Resolve(~/note.json) = %q, want /note.json", got) } + // ~\ is Windows' spelling of the same; elsewhere a backslash is a name character. + if filepath.Separator == '\\' { + if got, err := e.Resolve(`~\note.json`); err != nil || strings.Contains(got, "~") { + t.Errorf(`Resolve(~\note.json) = %q, %v; want \note.json`, got, err) + } + } // ~ must still respect the jail: from a sub-directory jail, ~ (the parent) escapes sub := filepath.Join(home, "sub") From 954a108e982d1274866f21c3bfc924e2db7409cd Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 18:34:38 -0600 Subject: [PATCH 2/4] File paths: refuse a leading env reference instead of expanding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found expanding one was a way to read a secret: "$OPENAI_API_KEY" as a read path resolved to /sk-…, and the "no such file" error handed the key to the model, with no prompt. It also let a call open something other than what the approval prompt, the risk check and git had seen. Resolve now refuses such a path with "file paths are not shell-expanded — pass the full path", naming the value only for a place (HOME, USERPROFILE, APPDATA, TEMP, …) so the model can retry at once. Any other variable's value never appears. PowerShell's env: matches in any case, and ${env:NAME} counts too. ~\ on Windows still expands, like ~/. Co-Authored-By: Claude Opus 5.5 --- internal/policy/policy.go | 56 ++++++++++++++++++++++------------ internal/policy/policy_test.go | 50 +++++++++++++----------------- 2 files changed, 58 insertions(+), 48 deletions(-) diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 0924b19..00c5c5a 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -467,7 +467,10 @@ func (e *Engine) IsSecret(abs string) bool { return fileMatchFold(secretReadFloo // under the workspace, silently creating a literal "~" directory. The jail check // still runs after expansion, so ~ can't be used to escape it. func (e *Engine) Resolve(path string) (string, error) { - abs := expandEnvRef(expandTilde(path)) + if err := unexpandedRef(path); err != nil { + return "", err + } + abs := expandTilde(path) if !filepath.IsAbs(abs) { abs = filepath.Join(e.base(), abs) } @@ -516,31 +519,44 @@ func expandTilde(p string) string { } // envRef matches an environment reference at the start of a path, in each -// shell's own spelling: PowerShell's $env:NAME, POSIX ${NAME} and $NAME, and -// cmd's %NAME% (whose names may hold parentheses: %ProgramFiles(x86)%). -var envRef = regexp.MustCompile(`^(?:\$env:([A-Za-z_][A-Za-z0-9_]*)|\$\{([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)|%([A-Za-z_][A-Za-z0-9_()]*)%)`) - -// expandEnvRef turns a leading environment reference into its value. Nothing on -// the way to a file tool is a shell, so "$env:USERPROFILE\.gitconfig" was joined -// onto the workspace as a literal "$env:USERPROFILE" directory: the read failed -// with a path the model never wrote, and the risk check had scored the home -// directory it meant. Only a variable that is set, and only when a separator or -// the end follows it — "$Recycle.Bin" and "Foo$Bar.class" are file names, and a -// reference to nothing is left for the path to fail on as written. -func expandEnvRef(p string) string { +// shell's own spelling: PowerShell's $env:NAME and ${env:NAME} (its "env:" in +// any case), POSIX ${NAME} and $NAME, and cmd's %NAME% (whose names may hold +// parentheses: %ProgramFiles(x86)%). +var envRef = regexp.MustCompile(`^(?:\$(?i:env):([A-Za-z_][A-Za-z0-9_]*)|\$\{(?:(?i:env):)?([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)|%([A-Za-z_][A-Za-z0-9_()]*)%)`) + +// dirVars are the variables an unexpanded path is told the value of: places, +// not secrets. Any other variable's value never appears — a model reading +// "$OPENAI_API_KEY" must learn nothing from the error. +var dirVars = map[string]bool{ + "HOME": true, "USERPROFILE": true, "APPDATA": true, "LOCALAPPDATA": true, + "TEMP": true, "TMP": true, "TMPDIR": true, "PROGRAMDATA": true, + "PROGRAMFILES": true, "PROGRAMFILES(X86)": true, "SYSTEMROOT": true, "WINDIR": true, + "XDG_CONFIG_HOME": true, "XDG_DATA_HOME": true, "XDG_CACHE_HOME": true, +} + +// unexpandedRef refuses a path that opens with an environment reference. +// Nothing on the way to a file tool is a shell, so "$env:USERPROFILE\.gitconfig" +// was joined onto the workspace as a literal "$env:USERPROFILE" directory: the +// read failed on a path the model never wrote, after the risk check had scored +// the home directory it meant. It is not expanded here either: the prompt, the +// risk check and git all see the path as written, and a call must open what +// they saw — and the value of an arbitrary variable, echoed back in an error, +// is a way to read a secret. A reference counts only when a separator or the +// end follows it: "$Recycle.Bin" and "Foo$Bar.class" are file names. +func unexpandedRef(p string) error { m := envRef.FindStringSubmatch(p) if m == nil { - return p + return nil } - rest := p[len(m[0]):] - if rest != "" && rest[0] != '/' && rest[0] != filepath.Separator { - return p + if rest := p[len(m[0]):]; rest != "" && rest[0] != '/' && rest[0] != filepath.Separator { + return nil } name := m[1] + m[2] + m[3] + m[4] // exactly one group matched - if v := os.Getenv(name); v != "" { - return v + rest + hint := "" + if v := os.Getenv(name); v != "" && dirVars[strings.ToUpper(name)] && filepath.IsAbs(v) { + hint = fmt.Sprintf(" (%s is %s)", m[0], v) } - return p + return fmt.Errorf("path %q starts with %s, but file paths are not shell-expanded — pass the full path%s", p, m[0], hint) } // maxSymlinkChase bounds resolveSymlinks' own-target chase (see below) against diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 62161c7..89a5c34 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -45,48 +45,42 @@ func TestResolveJailEscapeErrorExplainsItsAHardBoundary(t *testing.T) { } } -// A path opening with an environment reference lands where the variable points, -// in each shell's spelling — not in a literal "$env:USERPROFILE" directory under -// the workspace — and still inside the jail's rules. -func TestResolveExpandsALeadingEnvReference(t *testing.T) { +// A path opening with an environment reference is refused, not joined onto the +// workspace as a literal "$env:USERPROFILE" directory — and not expanded +// either: the prompt and the risk check saw the path as written. A place's +// value is offered so the model can retry with the full path; any other +// variable's value never appears. +func TestResolveRefusesALeadingEnvReference(t *testing.T) { home := t.TempDir() - t.Setenv("IPCO_TEST_HOME", home) + t.Setenv("HOME", home) + t.Setenv("IPCO_TEST_SECRET", "sk-do-not-leak") c := config.Default() c.Workspace = home c.File = config.FilePolicy{Default: "allow", Jail: "."} e := eng(t, c) - want := filepath.Join(home, "note.json") + sep := string(filepath.Separator) for _, p := range []string{ - "$env:IPCO_TEST_HOME/note.json", - "${IPCO_TEST_HOME}/note.json", - "$IPCO_TEST_HOME/note.json", - "%IPCO_TEST_HOME%/note.json", - "$env:IPCO_TEST_HOME" + string(filepath.Separator) + "note.json", + "$env:HOME/note.json", "$ENV:HOME" + sep + "note.json", "${env:HOME}/note.json", + "${HOME}/note.json", "$HOME/note.json", "%HOME%/note.json", "$HOME", } { - got, err := e.Resolve(p) - if err != nil || got != want { - t.Errorf("Resolve(%q) = %q, %v; want %q", p, got, err, want) + _, err := e.Resolve(p) + if err == nil || !strings.Contains(err.Error(), "not shell-expanded") || !strings.Contains(err.Error(), home) { + t.Errorf("Resolve(%q) error = %v; want it refused, naming where HOME is", p, err) + } + } + for _, p := range []string{"$IPCO_TEST_SECRET", "$env:IPCO_TEST_SECRET/x", "%IPCO_TEST_SECRET%/x"} { + _, err := e.Resolve(p) + if err == nil || strings.Contains(err.Error(), "sk-do-not-leak") { + t.Errorf("Resolve(%q) error = %v; want it refused without the value", p, err) } } - // Not references: a file name with a $ in it, a name followed by more name, - // and a variable that is not set — all stay as written, in the workspace. - for _, p := range []string{"Foo$Bar.class", "$IPCO_TEST_HOME.bak", "$IPCO_NOT_SET_ANYWHERE/note.json", "$Recycle.Bin"} { + // Not references: a file name with a $ in it, and a name followed by more name. + for _, p := range []string{"Foo$Bar.class", "$HOME.bak", "$Recycle.Bin"} { got, err := e.Resolve(p) if err != nil || got != filepath.Join(home, p) { t.Errorf("Resolve(%q) = %q, %v; want it left as a name in the workspace", p, got, err) } } - // The jail still holds: a reference to somewhere outside it is outside it. - sub := filepath.Join(home, "sub") - if err := os.Mkdir(sub, 0o755); err != nil { - t.Fatal(err) - } - c2 := config.Default() - c2.Workspace = sub - c2.File = config.FilePolicy{Default: "allow", Jail: "."} - if _, err := eng(t, c2).Resolve("$IPCO_TEST_HOME/escape.json"); err == nil { - t.Error("an env reference must not escape a sub-directory jail") - } } func TestResolveExpandsTilde(t *testing.T) { From 0d76dcba7c60e62ef544b215066c89b967b4fadf Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 18:42:12 -0600 Subject: [PATCH 3/4] File paths: review fixes for the env-reference refusal - A name's case is folded only on Windows: on Unix "home" is its own variable, and its value was named as if it were HOME's. - A backslash after the reference counts as a separator on every system: models write "$env:USERPROFILE\.gitconfig" on macOS too. - ${env:ProgramFiles(x86)} matches, like %ProgramFiles(x86)%. - git passes the refusal on, hint included, instead of calling the path outside the workspace. Co-Authored-By: Claude Opus 5.5 --- internal/policy/policy.go | 21 ++++++++++++++++----- internal/policy/policy_test.go | 9 +++++++-- internal/tool/ask_test.go | 10 ++++++++++ internal/tool/git.go | 3 +++ 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/internal/policy/policy.go b/internal/policy/policy.go index 00c5c5a..8cdf494 100644 --- a/internal/policy/policy.go +++ b/internal/policy/policy.go @@ -6,6 +6,7 @@ package policy import ( + "errors" "fmt" "os" "path/filepath" @@ -522,7 +523,11 @@ func expandTilde(p string) string { // shell's own spelling: PowerShell's $env:NAME and ${env:NAME} (its "env:" in // any case), POSIX ${NAME} and $NAME, and cmd's %NAME% (whose names may hold // parentheses: %ProgramFiles(x86)%). -var envRef = regexp.MustCompile(`^(?:\$(?i:env):([A-Za-z_][A-Za-z0-9_]*)|\$\{(?:(?i:env):)?([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)|%([A-Za-z_][A-Za-z0-9_()]*)%)`) +var envRef = regexp.MustCompile(`^(?:\$(?i:env):([A-Za-z_][A-Za-z0-9_]*)|\$\{(?:(?i:env):)?([A-Za-z_][A-Za-z0-9_()]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)|%([A-Za-z_][A-Za-z0-9_()]*)%)`) + +// ErrNotShellExpanded is the refusal of a path opening with an environment +// reference (see unexpandedRef), for a caller to pass on as it is. +var ErrNotShellExpanded = errors.New("file paths are not shell-expanded") // dirVars are the variables an unexpanded path is told the value of: places, // not secrets. Any other variable's value never appears — a model reading @@ -542,21 +547,27 @@ var dirVars = map[string]bool{ // risk check and git all see the path as written, and a call must open what // they saw — and the value of an arbitrary variable, echoed back in an error, // is a way to read a secret. A reference counts only when a separator or the -// end follows it: "$Recycle.Bin" and "Foo$Bar.class" are file names. +// end follows it: "$Recycle.Bin" and "Foo$Bar.class" are file names. A +// backslash counts as a separator everywhere: models write Windows paths on +// any system, and "$env:USERPROFILE\.gitconfig" is the same mistake on macOS. func unexpandedRef(p string) error { m := envRef.FindStringSubmatch(p) if m == nil { return nil } - if rest := p[len(m[0]):]; rest != "" && rest[0] != '/' && rest[0] != filepath.Separator { + if rest := p[len(m[0]):]; rest != "" && rest[0] != '/' && rest[0] != '\\' { return nil } name := m[1] + m[2] + m[3] + m[4] // exactly one group matched + key := name // "home" is not "HOME" on Unix: only Windows folds case + if runtime.GOOS == "windows" { + key = strings.ToUpper(name) + } hint := "" - if v := os.Getenv(name); v != "" && dirVars[strings.ToUpper(name)] && filepath.IsAbs(v) { + if v := os.Getenv(name); v != "" && dirVars[key] && filepath.IsAbs(v) { hint = fmt.Sprintf(" (%s is %s)", m[0], v) } - return fmt.Errorf("path %q starts with %s, but file paths are not shell-expanded — pass the full path%s", p, m[0], hint) + return fmt.Errorf("path %q starts with %s, but %w — pass the full path%s", p, m[0], ErrNotShellExpanded, hint) } // maxSymlinkChase bounds resolveSymlinks' own-target chase (see below) against diff --git a/internal/policy/policy_test.go b/internal/policy/policy_test.go index 89a5c34..6e6d793 100644 --- a/internal/policy/policy_test.go +++ b/internal/policy/policy_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" @@ -62,15 +63,19 @@ func TestResolveRefusesALeadingEnvReference(t *testing.T) { for _, p := range []string{ "$env:HOME/note.json", "$ENV:HOME" + sep + "note.json", "${env:HOME}/note.json", "${HOME}/note.json", "$HOME/note.json", "%HOME%/note.json", "$HOME", + `$env:HOME\note.json`, // Windows spelling on any system } { _, err := e.Resolve(p) if err == nil || !strings.Contains(err.Error(), "not shell-expanded") || !strings.Contains(err.Error(), home) { t.Errorf("Resolve(%q) error = %v; want it refused, naming where HOME is", p, err) } } - for _, p := range []string{"$IPCO_TEST_SECRET", "$env:IPCO_TEST_SECRET/x", "%IPCO_TEST_SECRET%/x"} { + if runtime.GOOS != "windows" { // "home" is its own variable on Unix, not HOME + t.Setenv("home", "/elsewhere/private") + } + for _, p := range []string{"$IPCO_TEST_SECRET", "$env:IPCO_TEST_SECRET/x", "%IPCO_TEST_SECRET%/x", "$home/x", "${env:ProgramFiles(x86)}/x"} { _, err := e.Resolve(p) - if err == nil || strings.Contains(err.Error(), "sk-do-not-leak") { + if err == nil || strings.Contains(err.Error(), "sk-do-not-leak") || strings.Contains(err.Error(), "/elsewhere/private") { t.Errorf("Resolve(%q) error = %v; want it refused without the value", p, err) } } diff --git a/internal/tool/ask_test.go b/internal/tool/ask_test.go index db08d01..7373eb9 100644 --- a/internal/tool/ask_test.go +++ b/internal/tool/ask_test.go @@ -65,3 +65,13 @@ func TestNoAnswerMarksOnlyANo(t *testing.T) { } NoAnswer(context.Background()) // outside an Ask: nothing to mark, and no panic } + +// git passes on the refusal of a path opening with an env reference — with +// its hint at where the path meant — instead of calling it outside the workspace. +func TestGitPassesOnTheUnexpandedPathHint(t *testing.T) { + tl := gitToolFor(t, initRepo(t), yes()) + r := tl.Call(context.Background(), "clone", map[string]any{"url": "$HOME/repo"}) + if !r.IsError || !strings.Contains(r.Content, "not shell-expanded") { + t.Errorf("clone $HOME/repo = %+v, want the not-shell-expanded refusal", r) + } +} diff --git a/internal/tool/git.go b/internal/tool/git.go index 6a12f66..1bf2abd 100644 --- a/internal/tool/git.go +++ b/internal/tool/git.go @@ -136,6 +136,9 @@ func (g *gitTool) checkLocalGitPath(v string) error { } p := strings.TrimPrefix(strings.TrimPrefix(v, "file://"), "file:") if _, err := g.pol.Resolve(p); err != nil { + if errors.Is(err, policy.ErrNotShellExpanded) { + return err // its own hint says where the path meant + } return errors.New("a local repository outside the workspace isn't reachable from here: " + v) } return nil From 103e60badd30f0bf11ddd94be233706f1cd07c0b Mon Sep 17 00:00:00 2001 From: Roman Romenskyi Date: Mon, 19 Oct 2026 19:04:47 -0600 Subject: [PATCH 4/4] =?UTF-8?q?File=20paths:=20git=20refuses=20$env:NAME/?= =?UTF-8?q?=E2=80=A6=20too,=20which=20reads=20as=20host:path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- internal/tool/ask_test.go | 8 +++++--- internal/tool/git.go | 7 +++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/tool/ask_test.go b/internal/tool/ask_test.go index 7373eb9..4d22b1d 100644 --- a/internal/tool/ask_test.go +++ b/internal/tool/ask_test.go @@ -70,8 +70,10 @@ func TestNoAnswerMarksOnlyANo(t *testing.T) { // its hint at where the path meant — instead of calling it outside the workspace. func TestGitPassesOnTheUnexpandedPathHint(t *testing.T) { tl := gitToolFor(t, initRepo(t), yes()) - r := tl.Call(context.Background(), "clone", map[string]any{"url": "$HOME/repo"}) - if !r.IsError || !strings.Contains(r.Content, "not shell-expanded") { - t.Errorf("clone $HOME/repo = %+v, want the not-shell-expanded refusal", r) + for _, url := range []string{"$HOME/repo", "$env:HOME/repo"} { // the second reads as host:path + r := tl.Call(context.Background(), "clone", map[string]any{"url": url}) + if !r.IsError || !strings.Contains(r.Content, "not shell-expanded") { + t.Errorf("clone %s = %+v, want the not-shell-expanded refusal", url, r) + } } } diff --git a/internal/tool/git.go b/internal/tool/git.go index 1bf2abd..6f31beb 100644 --- a/internal/tool/git.go +++ b/internal/tool/git.go @@ -131,6 +131,13 @@ func localGitPath(v string) bool { // fetching from one outside reads files the jail exists to keep out, and // pushing to one writes there (found by review). func (g *gitTool) checkLocalGitPath(v string) error { + if strings.HasPrefix(v, "$") || strings.HasPrefix(v, "%") { + // "$env:HOME/repo" reads as host:path to localGitPath, and git then + // failed on ssh to a host named "$env"; no server is named like that. + if _, err := g.pol.Resolve(v); errors.Is(err, policy.ErrNotShellExpanded) { + return err + } + } if !localGitPath(v) { return nil }