From b7218588c725b46b1eb929b2411c2b0f5fd98a77 Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Thu, 30 Apr 2026 23:13:54 -0500 Subject: [PATCH] feat(runtime): add HostPath for jail-aware host path translation Promote the previously private OsFS jail-translation to a public API on the FileSystem interface and forward it from Runtime.HostPath, so callers that need a real on-disk path (subprocess argv, file handles handed to external libraries, logging) have a single stable entry point instead of re-implementing TrimPrefix + filepath.Join(jail, ...) at each call site. - FileSystem.HostPath(virtual) (string, error) added to the interface - (*OsFS).HostPath canonicalizes via EvalSymlinks on the jail root with a raw fallback so darwin /var -> /private/var resolves correctly at the API boundary instead of leaking into every caller - Runtime.HostPath forwarder applies the standard validate -> ExpandEnv -> ExpandPath -> wd-prefix chain before delegating to the FS - toolkit.Edit migrated from inline TrimPrefix + filepath.Join(jail, ...) to the canonical ResolvePath + HostPath two-call form, with godoc noting the rationale; pinned by the existing TestEdit_UsesRuntimeAndResolvesSymlinkPath regression - ErrSubprocessNotSupported sentinel exported for backends that cannot host a subprocess (TestFS, jailed FSes without a real host mapping) - Tests: 5 TestOsFS_HostPath_* (including a darwin /var regression and a lexical-contract case for parent-symlink chasing) plus 4 TestRuntime_HostPath_* covering the forwarder chain --- toolkit/filesystem/errors.go | 11 +++ toolkit/filesystem/filesystem.go | 23 ++++++ toolkit/filesystem/os_fs.go | 50 ++++++++++++ toolkit/filesystem/os_fs_test.go | 126 +++++++++++++++++++++++++++++++ toolkit/runtime.go | 47 ++++++++++++ toolkit/runtime_test.go | 61 +++++++++++++++ toolkit/user.go | 18 +++-- 7 files changed, 331 insertions(+), 5 deletions(-) create mode 100644 toolkit/filesystem/errors.go create mode 100644 toolkit/filesystem/os_fs_test.go diff --git a/toolkit/filesystem/errors.go b/toolkit/filesystem/errors.go new file mode 100644 index 0000000..fea1bb5 --- /dev/null +++ b/toolkit/filesystem/errors.go @@ -0,0 +1,11 @@ +package filesystem + +import "errors" + +// ErrSubprocessNotSupported is the sentinel returned by FileSystem +// implementations that cannot launch host-level subprocesses for the +// caller (e.g. an in-memory FS with no real host mapping). No +// production codepath returns it yet; it is exported for future +// consumers that need to distinguish "no host available" from other +// failure modes. +var ErrSubprocessNotSupported = errors.New("filesystem: subprocess execution not supported") diff --git a/toolkit/filesystem/filesystem.go b/toolkit/filesystem/filesystem.go index 6af4f43..288f43b 100644 --- a/toolkit/filesystem/filesystem.go +++ b/toolkit/filesystem/filesystem.go @@ -85,6 +85,29 @@ type FileSystem interface { // ResolvePath resolves path to an absolute normalized path, optionally following symlinks. // Relative paths are resolved from the current working directory. ResolvePath(path string, followSymlinks bool) (string, error) + // HostPath translates a virtual (jail-relative) path into the + // equivalent absolute host filesystem path that an external program + // (subprocess, editor, etc.) needs to operate on. + // + // Implementations canonicalize the jail prefix via filepath.EvalSymlinks + // before joining so platform-level symlinks (e.g. macOS /var -> + // /private/var) do not produce phantom escapes when callers later + // re-canonicalize the returned path. Implementations do NOT + // canonicalize intermediate symlinks in the virtual path; callers + // that need parent-traversal-symlink defense should resolve the + // path through ResolvePath(_, true) first. + // + // HostPath is the public surface of the in-tree host-translation + // helper used by the FileSystem implementation itself. It is + // intended for callers that need to hand a host path to code outside + // the FileSystem abstraction (e.g. exec.Command). + // + // The input may be absolute (treated as virtual when a jail is + // configured) or relative (resolved against the current working + // directory). Returns [jail.ErrEscapeAttempt] when the lexical jail + // check rejects the resulting path. When no jail is configured, the + // cleaned absolute host path is returned unchanged. + HostPath(virtual string) (string, error) } func atomicWriteFile(path string, data []byte, perm os.FileMode) error { diff --git a/toolkit/filesystem/os_fs.go b/toolkit/filesystem/os_fs.go index 58a90b9..3a681f0 100644 --- a/toolkit/filesystem/os_fs.go +++ b/toolkit/filesystem/os_fs.go @@ -674,4 +674,54 @@ func (fs *OsFS) hostPath(path string) string { return filepath.Clean(filepath.Join(jailPath, path)) } +// HostPath returns the absolute host filesystem path corresponding to a +// virtual (jail-relative) path. It is the public surface of the in-tree +// resolveHost translation used internally by FileSystem methods, intended +// for callers that need to hand a host path to code outside the FileSystem +// abstraction (e.g. exec.CommandContext). +// +// Semantics: +// +// 1. Resolve the input lexically through resolveVirtual with +// followSymlinks=false. This applies the working directory for +// relative inputs and re-checks IsInJail on the lexical jail join, +// returning [jail.ErrEscapeAttempt] for parent-traversal attempts. +// 2. When a jail is configured, canonicalize the jail prefix via +// filepath.EvalSymlinks. On macOS, /var resolves to /private/var; on +// Linux, jail directories created under /tmp on certain distros may +// resolve through symlinks too. Without this canonicalization, +// downstream consumers that re-canonicalize the returned path would +// see a different prefix and falsely flag the path as outside the +// jail. Falls back to the raw jail when EvalSymlinks fails (e.g. the +// jail does not yet exist), matching resolveHostForCreate. +// 3. Join the canonical jail with the cleaned virtual path and return +// the result. The final segment is intentionally NOT EvalSymlinks'd +// — callers may pass paths whose final component does not yet exist +// (e.g. files about to be created by an editor). +// +// Returns [jail.ErrEscapeAttempt] when the virtual path would resolve +// outside the configured jail. When no jail is configured, returns the +// cleaned absolute host path unchanged. +func (fs *OsFS) HostPath(virtual string) (string, error) { + resolved, err := fs.resolveVirtual(virtual, false) + if err != nil { + return "", err + } + + jailPath := fs.GetJail() + if strings.TrimSpace(jailPath) == "" { + return filepath.Clean(resolved), nil + } + + // Canonicalize the jail prefix. Falling back to the raw jail keeps + // the contract usable when the jail directory has not yet been + // created on disk (matches resolveHostForCreate at lines 466-470). + canonicalJail := jailPath + if evaledJail, evalErr := filepath.EvalSymlinks(jailPath); evalErr == nil { + canonicalJail = evaledJail + } + + return filepath.Clean(filepath.Join(canonicalJail, resolved)), nil +} + var _ FileSystem = (*OsFS)(nil) diff --git a/toolkit/filesystem/os_fs_test.go b/toolkit/filesystem/os_fs_test.go new file mode 100644 index 0000000..64811ce --- /dev/null +++ b/toolkit/filesystem/os_fs_test.go @@ -0,0 +1,126 @@ +package filesystem_test + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/jlrickert/cli-toolkit/toolkit/filesystem" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOsFS_HostPath_InJailVirtualPath(t *testing.T) { + t.Parallel() + + jailDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + fs, err := filesystem.NewOsFS(jailDir, "/") + require.NoError(t, err) + + got, err := fs.HostPath("/home/alice/notes") + require.NoError(t, err) + assert.Equal(t, filepath.Join(jailDir, "home", "alice", "notes"), got) +} + +func TestOsFS_HostPath_LexicalContract_ParentSymlinkNotChased(t *testing.T) { + t.Parallel() + + // Plant a parent-traversal symlink: /jail/sneaky -> outside. + // HostPath performs the same lexical IsInJail check as + // resolveHostForCreate's parent-walk uses, but without the symlink + // canonicalization of intermediate components — so the LEXICAL form + // of an in-jail path stays in jail. This test confirms the lexical + // jail-escape contract: paths that resolve outside the jail at the + // LEXICAL layer return jail.ErrEscapeAttempt. + // + // Intermediate-symlink canonicalization is intentionally out of + // scope for HostPath; callers that need parent-traversal-symlink + // defense must route through ResolvePath(_, true) first. HostPath + // canonicalizes only the JAIL prefix (to fix the macOS /var case). + jailDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + outsideDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + if err := os.Symlink(outsideDir, filepath.Join(jailDir, "sneaky")); err != nil { + t.Skipf("symlink unavailable: %v", err) + } + + fs, err := filesystem.NewOsFS(jailDir, "/") + require.NoError(t, err) + + // HostPath does NOT canonicalize the intermediate "sneaky" symlink, + // so the returned lexical host path stays under the jail prefix even + // though OS-level traversal would escape. This test pins the + // documented lexical contract; tightening it would be a contract + // change, not a bug fix. + got, err := fs.HostPath("/sneaky/file.txt") + require.NoError(t, err) + assert.Equal(t, filepath.Join(jailDir, "sneaky", "file.txt"), got, + "HostPath returns lexical jail-prefixed path; parent-symlink defense is the caller's responsibility via ResolvePath") +} + +func TestOsFS_HostPath_NoJail(t *testing.T) { + t.Parallel() + + cwd, err := os.Getwd() + require.NoError(t, err) + + fs, err := filesystem.NewOsFS("", cwd) + require.NoError(t, err) + + abs := filepath.Join(cwd, "some", "path") + got, err := fs.HostPath(abs) + require.NoError(t, err) + assert.Equal(t, filepath.Clean(abs), got) +} + +func TestOsFS_HostPath_RelativeUsesWd(t *testing.T) { + t.Parallel() + + jailDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + fs, err := filesystem.NewOsFS(jailDir, "/work") + require.NoError(t, err) + + got, err := fs.HostPath("file.txt") + require.NoError(t, err) + assert.Equal(t, filepath.Join(jailDir, "work", "file.txt"), got) +} + +// TestOsFS_HostPath_DarwinVarCanonicalization pins the macOS jail-prefix +// canonicalization. On macOS, t.TempDir() lives under /var, which is +// itself a symlink to /private/var. Without canonicalizing the jail +// prefix in HostPath, downstream consumers that re-canonicalize would +// see the /private/var form and falsely flag the path as outside the +// jail. +func TestOsFS_HostPath_DarwinVarCanonicalization(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("regression test specific to macOS /var -> /private/var symlink") + } + t.Parallel() + + // Use the raw t.TempDir() (NOT EvalSymlinks'd) so the jail still + // contains /var/...; HostPath itself must do the canonicalization. + rawJail := t.TempDir() + require.Contains(t, rawJail, "/var/", "expected macOS TempDir under /var") + + fs, err := filesystem.NewOsFS(rawJail, "/") + require.NoError(t, err) + + got, err := fs.HostPath("/notes/keg") + require.NoError(t, err) + + // The returned path must be under /private/var (the canonical form), + // not /var (the symlink form), so that re-canonicalization by + // callers stays inside the same jail prefix. + canonicalJail, err := filepath.EvalSymlinks(rawJail) + require.NoError(t, err) + assert.Equal(t, filepath.Join(canonicalJail, "notes", "keg"), got) + assert.Contains(t, got, "/private/var/", + "expected canonical /private/var prefix, got %q", got) +} diff --git a/toolkit/runtime.go b/toolkit/runtime.go index daf9324..e8b77b0 100644 --- a/toolkit/runtime.go +++ b/toolkit/runtime.go @@ -824,5 +824,52 @@ func (rt *Runtime) Rel(basePath, targetPath string) (string, error) { return rt.fs.Rel(baseResolved, targetResolved) } +// HostPath translates a caller-supplied virtual path into the absolute +// host filesystem path that an external program (subprocess, editor) +// must be invoked against. +// +// Input is run through the same expansion chain as other Runtime +// forwarders so callers may pass tilde-prefixed (~/foo), env-variable +// ($HOME/foo), or relative paths and get the same result they would +// from ReadFile or Stat. The expanded virtual path is then handed to +// FS().HostPath, which canonicalizes the jail prefix (see +// [filesystem.OsFS.HostPath]) and joins it with the cleaned virtual. +// +// Returns [jail.ErrEscapeAttempt] when the path would resolve outside +// the configured jail. With no jail configured, returns the cleaned +// absolute host path. +func (rt *Runtime) HostPath(rel string) (string, error) { + if err := rt.Validate(); err != nil { + return "", err + } + + // Empty input is treated like "." in other forwarders: resolve to + // the current working directory's host equivalent. + var virtual string + if strings.TrimSpace(rel) == "" || rel == "." { + cwd, err := rt.Getwd() + if err != nil { + return "", err + } + virtual = cwd + } else { + expanded := ExpandEnv(rt, rel) + parsed, err := ExpandPath(rt, expanded) + if err != nil { + return "", err + } + virtual = parsed + if !filepath.IsAbs(virtual) { + cwd, err := rt.Getwd() + if err != nil { + return "", err + } + virtual = filepath.Join(cwd, virtual) + } + } + + return rt.fs.HostPath(filepath.Clean(virtual)) +} + var _ Env = (*Runtime)(nil) var _ FileSystem = (*Runtime)(nil) diff --git a/toolkit/runtime_test.go b/toolkit/runtime_test.go index c50b0b1..7ddf6fd 100644 --- a/toolkit/runtime_test.go +++ b/toolkit/runtime_test.go @@ -248,3 +248,64 @@ func TestNewRuntime_NilOptionSkipped(t *testing.T) { require.NoError(t, err) require.NotNil(t, rt) } + +func TestRuntime_HostPath_TildeExpansion(t *testing.T) { + t.Parallel() + + jailDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + rt, err := toolkit.NewTestRuntime(jailDir, "/home/alice", "alice") + require.NoError(t, err) + + got, err := rt.HostPath("~/notes/keg") + require.NoError(t, err) + assert.Equal(t, filepath.Join(jailDir, "home", "alice", "notes", "keg"), got) +} + +func TestRuntime_HostPath_EnvVarExpansion(t *testing.T) { + t.Parallel() + + jailDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + rt, err := toolkit.NewTestRuntime(jailDir, "/home/alice", "alice") + require.NoError(t, err) + require.NoError(t, rt.Set("KEG_ROOT", "/home/alice/notes")) + + got, err := rt.HostPath("$KEG_ROOT/keg") + require.NoError(t, err) + assert.Equal(t, filepath.Join(jailDir, "home", "alice", "notes", "keg"), got) +} + +func TestRuntime_HostPath_AbsoluteVirtualPath(t *testing.T) { + t.Parallel() + + jailDir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + + rt, err := toolkit.NewTestRuntime(jailDir, "/home/alice", "alice") + require.NoError(t, err) + + got, err := rt.HostPath("/etc/config") + require.NoError(t, err) + assert.Equal(t, filepath.Join(jailDir, "etc", "config"), got) +} + +func TestRuntime_HostPath_NoJail(t *testing.T) { + t.Parallel() + + // No jail: HostPath should return the cleaned absolute host path. + cwd, err := os.Getwd() + require.NoError(t, err) + + rt, err := toolkit.NewRuntime( + toolkit.WithRuntimeFileSystem(&toolkit.OsFS{}), + ) + require.NoError(t, err) + + abs := filepath.Join(cwd, "some", "file.txt") + got, err := rt.HostPath(abs) + require.NoError(t, err) + assert.Equal(t, filepath.Clean(abs), got) +} diff --git a/toolkit/user.go b/toolkit/user.go index 3e68296..e863c5e 100644 --- a/toolkit/user.go +++ b/toolkit/user.go @@ -122,19 +122,27 @@ func UserStatePath(env Env) (string, error) { var DefaultEditor = "nano" // Edit launches the user's editor to edit the provided file path. +// +// The two-call form (ResolvePath(_, true) followed by HostPath) is intentional +// and pinned by TestEdit_UsesRuntimeAndResolvesSymlinkPath; do not consolidate. func Edit(ctx context.Context, rt *Runtime, path string) error { if path == "" { return fmt.Errorf("empty filepath") } - resolvedPath, err := rt.ResolvePath(path, true) + // Follow symlinks at the virtual layer so editors are pointed at the + // real file behind any in-jail symlink (matches the prior behavior + // pinned by TestEdit_UsesRuntimeAndResolvesSymlinkPath). HostPath then + // translates the canonicalized virtual path to its host form, + // applying jail-prefix canonicalization (e.g. macOS /var -> + // /private/var) so the editor receives the real on-disk path. + resolvedVirtual, err := rt.ResolvePath(path, true) if err != nil { return fmt.Errorf("resolve edit path: %w", err) } - editorPath := resolvedPath - if jail := strings.TrimSpace(rt.GetJail()); jail != "" { - trimmed := strings.TrimPrefix(resolvedPath, string(filepath.Separator)) - editorPath = filepath.Join(jail, trimmed) + editorPath, err := rt.HostPath(resolvedVirtual) + if err != nil { + return fmt.Errorf("resolve edit path: %w", err) } editor := rt.Get("VISUAL")