fix(bind): accept ~/ paths in OpenClaw secret references (#839)

OpenClaw stores secret file paths in user-authored ~/-relative form so
the configuration stays portable across machines. lark-cli config bind
previously rejected these as non-absolute, blocking users until they
rewrote the OpenClaw config with literal absolute paths.

bind now resolves ~ to the OpenClaw home directory (OPENCLAW_HOME if
set, otherwise the OS home) before the path audit runs, mirroring how
OpenClaw itself reads the same field. Cwd-relative paths and other
unsafe locations are still rejected as before.
This commit is contained in:
evandance
2026-05-13 12:34:43 +08:00
committed by GitHub
parent ce0b68dc0e
commit 62ff3d66a6
5 changed files with 575 additions and 2 deletions

View File

@@ -65,7 +65,11 @@ func AssertSecurePath(params AuditParams) (string, error) {
}
// requireAbsolutePath rejects relative paths; relative paths would depend on
// the process cwd and defeat the point of a static audit.
// the process cwd and defeat the point of a static audit. Shell-style
// shortcuts like `~` are home-relative, not cwd-relative — they are an
// orthogonal concern and the audit is intentionally Go-stdlib strict here.
// Callers that accept user-authored config (e.g. resolveFileRef) must
// pre-resolve any such shortcuts before passing the path in.
func requireAbsolutePath(target, label string) error {
if !filepath.IsAbs(target) {
return fmt.Errorf("%s: path must be absolute, got %q", label, target)

View File

@@ -23,9 +23,19 @@ func resolveFileRef(ref *SecretRef, pc *ProviderConfig) (string, error) {
return "", fmt.Errorf("file provider path is empty")
}
// OpenClaw preserves user-authored `~/...` paths verbatim on disk for
// portability and resolves them at read time. lark-cli reads the file
// raw, so we mirror that resolution here before the audit — otherwise
// an unambiguous home-relative path would be rejected by
// requireAbsolutePath, which is meant to guard against cwd-relative
// paths (a different concern). expandTildePath honours OPENCLAW_HOME so
// a tilde inside an OPENCLAW_HOME-overridden config resolves to the
// same absolute path OpenClaw itself would have used.
targetPath := expandTildePath(pc.Path)
// Security audit on file path
securePath, err := AssertSecurePath(AuditParams{
TargetPath: pc.Path,
TargetPath: targetPath,
Label: "secrets.providers file path",
TrustedDirs: pc.TrustedDirs,
AllowInsecurePath: pc.AllowInsecurePath,

View File

@@ -6,6 +6,7 @@ package binding
import (
"os"
"path/filepath"
"strings"
"testing"
)
@@ -230,3 +231,88 @@ func TestResolveFileRef_ExceedsMaxBytes(t *testing.T) {
t.Errorf("error = %q, want %q", err.Error(), want)
}
}
// TestResolveFileRef_TildePath_SingleValue is the end-to-end smoke test
// for the fix: a singleValue file provider with a ~/-relative path
// resolves correctly through resolveFileRef. Before this PR the audit
// would reject the path as "must be absolute".
func TestResolveFileRef_TildePath_SingleValue(t *testing.T) {
dir := t.TempDir()
setFakeOSHome(t, dir)
t.Setenv("OPENCLAW_HOME", "")
p := filepath.Join(dir, "secret.txt")
if err := os.WriteFile(p, []byte("tilde_secret\n"), 0o600); err != nil {
t.Fatalf("write temp file: %v", err)
}
ref := &SecretRef{Source: "file", ID: SingleValueFileRefID}
pc := &ProviderConfig{
Source: "file",
Path: "~/secret.txt",
Mode: "singleValue",
AllowInsecurePath: true,
}
got, err := resolveFileRef(ref, pc)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got != "tilde_secret" {
t.Errorf("got %q, want %q", got, "tilde_secret")
}
}
// TestResolveFileRef_RelativePath_StillRejected guards the absolute-path
// audit: cwd-relative input must still be rejected even though tilde was
// loosened. Catches regressions if expandTildePath is ever widened to
// also expand "./..." (which would weaken the audit's invariant).
func TestResolveFileRef_RelativePath_StillRejected(t *testing.T) {
ref := &SecretRef{Source: "file", ID: SingleValueFileRefID}
pc := &ProviderConfig{
Source: "file",
Path: "relative/secret.txt",
Mode: "singleValue",
AllowInsecurePath: true,
}
_, err := resolveFileRef(ref, pc)
if err == nil {
t.Fatal("expected error for relative path, got nil")
}
wantSub := "path must be absolute"
if !strings.Contains(err.Error(), wantSub) {
t.Errorf("error = %q, want substring %q", err.Error(), wantSub)
}
}
// TestResolveFileRef_TildePath_JSONMode verifies the tilde-expansion
// path works for json mode (where ref id is a JSON pointer) as well as
// singleValue mode — the mechanism is mode-agnostic.
func TestResolveFileRef_TildePath_JSONMode(t *testing.T) {
dir := t.TempDir()
setFakeOSHome(t, dir)
t.Setenv("OPENCLAW_HOME", "")
p := filepath.Join(dir, "secrets.json")
content := `{"providers":{"feishu":{"key":"json_via_tilde"}}}`
if err := os.WriteFile(p, []byte(content), 0o600); err != nil {
t.Fatalf("write temp file: %v", err)
}
ref := &SecretRef{Source: "file", ID: "/providers/feishu/key"}
pc := &ProviderConfig{
Source: "file",
Path: "~/secrets.json",
Mode: "json",
AllowInsecurePath: true,
}
got, err := resolveFileRef(ref, pc)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got != "json_via_tilde" {
t.Errorf("got %q, want %q", got, "json_via_tilde")
}
}

180
internal/binding/tilde.go Normal file
View File

@@ -0,0 +1,180 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT
package binding
import (
"os"
"os/user"
"path/filepath"
"strings"
"github.com/larksuite/cli/internal/vfs"
)
// hasTildePrefix reports whether s begins with `~` followed by end-of-string,
// `/`, or `\` — the form OpenClaw treats as home-relative.
func hasTildePrefix(s string) bool {
if s == "" || s[0] != '~' {
return false
}
if len(s) == 1 {
return true
}
return s[1] == '/' || s[1] == '\\'
}
// joinTildeSuffix expands a tilde-prefixed string against a resolved home
// directory. Replaces only the leading `~` so the original separator
// (forward or back slash) and suffix bytes are kept verbatim, matching
// OpenClaw's `input.replace(/^~(?=$|[\\/])/, home)` semantics rather than
// going through filepath.Join (which would silently drop a literal `\` on
// POSIX). filepath.Clean is applied so `..` and duplicate separators are
// collapsed in the same way Node's path.resolve does on each platform.
//
// Caller must ensure hasTildePrefix(s) is true and home is non-empty.
func joinTildeSuffix(s, home string) string {
if len(s) == 1 {
return home
}
return filepath.Clean(home + s[1:])
}
// normalizeSentinel applies OpenClaw's normalize() helper to a single
// string: trims whitespace and treats the JS-flavoured literals
// "undefined" / "null" (along with empty/whitespace-only) as unset.
func normalizeSentinel(v string) string {
v = strings.TrimSpace(v)
if v == "undefined" || v == "null" {
return ""
}
return v
}
// osHome returns the OS-level home directory by walking OpenClaw's
// resolution chain: HOME → USERPROFILE → OS user database (getpwuid on
// Unix / user32 on Windows, via os/user.Current). Each candidate is
// passed through normalizeSentinel so sentinel literals and blank
// strings fall through.
//
// Matches OpenClaw's resolveRawOsHomeDir env chain so the same tilde
// resolves against the same home under mixed shell environments and
// accidentally-stringified env values. Go's stdlib os.UserHomeDir on
// Unix only re-reads HOME and gives up; Node's os.homedir() still
// returns the account home via the user database, so the explicit
// user.Current() step is what keeps OpenClaw-authored `~/...` working
// in HOME-unset shells.
//
// Deliberate hybrid contract — neither a strict mirror of OpenClaw
// nor a strict reject-on-missing:
//
// - OpenClaw's final fallback is cwd (via resolveRequiredHomeDir →
// process.cwd()). We don't do that because requireAbsolutePath
// exists precisely to reject cwd-dependent paths; routing
// `~/secret` through cwd would defeat the audit invariant.
//
// - We still go through user.Current() before giving up, even when
// HOME is a sentinel literal ("undefined" / "null") and
// USERPROFILE is unset. At that point OpenClaw would land on cwd,
// and a strict implementation would reject; user.Current() lands
// on the account home instead — cwd-independent and user-bound,
// so it satisfies the audit's safety goal while still letting
// ~/-authored configs resolve in a malformed-env shell.
//
// - Only returns "" when the env chain AND user.Current() are all
// unresolvable, at which point the caller surfaces a clean
// "path must be absolute" error from the audit.
func osHome() string {
if v := normalizeSentinel(os.Getenv("HOME")); v != "" {
return v
}
if v := normalizeSentinel(os.Getenv("USERPROFILE")); v != "" {
return v
}
if u, err := user.Current(); err == nil {
return normalizeSentinel(u.HomeDir)
}
return ""
}
// explicitOpenClawHome reads OPENCLAW_HOME with OpenClaw's normalize()
// semantics applied.
func explicitOpenClawHome() string {
return normalizeSentinel(os.Getenv("OPENCLAW_HOME"))
}
// absolutize returns p as an absolute path, resolving against the process
// cwd when p is relative. Returns "" when the cwd cannot be resolved.
// Wraps filepath.Abs semantics via vfs.Getwd because forbidigo bans
// filepath.Abs inside internal/ packages.
func absolutize(p string) string {
if p == "" {
return ""
}
if filepath.IsAbs(p) {
return filepath.Clean(p)
}
wd, err := vfs.Getwd()
if err != nil {
return ""
}
return filepath.Join(wd, p)
}
// openClawHome returns the home directory used to resolve `~`-relative paths
// authored against OpenClaw's config. Closely mirrors OpenClaw's
// home-resolution semantics so the same tilde resolves to the same
// absolute path here as inside OpenClaw runtime under all normal
// conditions.
//
// Resolution order:
// 1. OPENCLAW_HOME env var, when set (sentinel-normalised).
// 2. If OPENCLAW_HOME itself has a tilde prefix, expand it against the OS
// home (see osHome); the result is empty when the OS home is
// unresolvable.
// 3. Otherwise fall back to the OS home.
//
// The returned path is absolute (relative OPENCLAW_HOME values are
// absolutised against the process cwd, matching Node path.resolve in
// OpenClaw's pipeline).
//
// Returns "" when no home can be resolved. This is a deliberate
// divergence from OpenClaw, whose read pipeline would fall back to
// cwd via resolveRequiredHomeDir — see osHome for the rationale.
func openClawHome() string {
raw := explicitOpenClawHome()
switch {
case raw == "":
raw = osHome()
case hasTildePrefix(raw):
h := osHome()
if h == "" {
return ""
}
raw = joinTildeSuffix(raw, h)
}
return absolutize(raw)
}
// expandTildePath resolves a leading `~` or `~/...` prefix to OpenClaw's
// effective home directory (see openClawHome).
//
// Returns the input unchanged when it lacks a tilde prefix or when
// openClawHome cannot resolve a home directory. The latter case is a
// deliberate divergence from OpenClaw, whose read pipeline falls back
// to cwd — see osHome. Surfacing a "path must be absolute" error from
// the audit is preferable to silently routing a user-authored
// `~/secret` through cwd resolution.
//
// `~user` shell-style expansion is intentionally not supported (OpenClaw
// does not support it either).
func expandTildePath(p string) string {
if !hasTildePrefix(p) {
return p
}
home := openClawHome()
if home == "" {
return p
}
return joinTildeSuffix(p, home)
}

View File

@@ -0,0 +1,293 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT
package binding
import (
"os"
"os/user"
"path/filepath"
"runtime"
"strings"
"testing"
)
// setFakeOSHome controls osHome's env-chain inputs (HOME and USERPROFILE)
// in one call so tests stay deterministic across platforms. osHome reads
// HOME first, then USERPROFILE, then user.Current(); setting only one of
// the two leaves the test sensitive to whichever the runner happens to
// have populated. Passing dir == "" disables both env entries so tests
// can exercise the user.Current() fallback or no-home edge cases.
func setFakeOSHome(t *testing.T, dir string) {
t.Helper()
t.Setenv("HOME", dir)
t.Setenv("USERPROFILE", dir)
}
// isolateRuntimeWrites parks the process cwd in a fresh TempDir for the
// test's duration. Tests that set HOME to a sentinel literal trigger Go
// runtime side effects — most visibly the telemetry subsystem, which
// calls os.UserConfigDir() (= "$HOME/Library/Application Support" on
// darwin) and happily writes through a relative result like
// "undefined/Library/...". Without isolation those files land in the
// package or repo dir and get accidentally staged. Chdir'ing into a
// TempDir routes the noise into a path testing.T auto-cleans.
func isolateRuntimeWrites(t *testing.T) {
t.Helper()
orig, err := os.Getwd()
if err != nil {
t.Fatalf("getwd: %v", err)
}
if err := os.Chdir(t.TempDir()); err != nil {
t.Fatalf("chdir: %v", err)
}
t.Cleanup(func() {
_ = os.Chdir(orig)
})
}
// TestOpenClawHome covers the openClawHome resolution table: empty /
// sentinel OPENCLAW_HOME falls back to the OS home, explicit absolute
// values are used verbatim (with whitespace trimmed), and tilde-prefixed
// values recurse through the OS home.
func TestOpenClawHome(t *testing.T) {
homeDir := t.TempDir()
explicit := t.TempDir()
setFakeOSHome(t, homeDir)
tests := []struct {
name string
openclawEnv string
want string
}{
{"unset falls back to OS home", "", homeDir},
{"undefined literal treated as unset", "undefined", homeDir},
{"null literal treated as unset", "null", homeDir},
{"whitespace-only treated as unset", " ", homeDir},
{"explicit absolute path used verbatim", explicit, explicit},
{"explicit absolute path is trimmed", " " + explicit + " ", explicit},
{"bare tilde resolves to OS home", "~", homeDir},
{"tilde-prefixed value recurses through OS home", "~/custom", filepath.Join(homeDir, "custom")},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Setenv("OPENCLAW_HOME", tc.openclawEnv)
got := openClawHome()
if got != tc.want {
t.Errorf("openClawHome() = %q, want %q", got, tc.want)
}
})
}
}
// TestOpenClawHome_RelativeIsAbsolutized confirms a relative
// OPENCLAW_HOME is resolved against the process cwd, mirroring Node's
// path.resolve behaviour in OpenClaw.
func TestOpenClawHome_RelativeIsAbsolutized(t *testing.T) {
t.Setenv("OPENCLAW_HOME", filepath.FromSlash("relative/dir"))
got := openClawHome()
if !filepath.IsAbs(got) {
t.Fatalf("openClawHome() = %q, want absolute path", got)
}
wantSuffix := filepath.FromSlash("relative/dir")
if !strings.HasSuffix(got, wantSuffix) {
t.Errorf("openClawHome() = %q, want suffix %q", got, wantSuffix)
}
}
// TestOpenClawHome_FallsBackToUserDatabase pins osHome's final fallback
// to the OS user database when HOME and USERPROFILE are both unset,
// matching Node's os.homedir() (which uses getpwuid). Cwd-independent
// and user-bound, so it does not conflict with the "no cwd fallback"
// rule documented on osHome.
func TestOpenClawHome_FallsBackToUserDatabase(t *testing.T) {
u, err := user.Current()
if err != nil || u.HomeDir == "" {
t.Skip("os/user.Current() unavailable on this runner")
}
setFakeOSHome(t, "")
t.Setenv("OPENCLAW_HOME", "")
got := openClawHome()
if got != u.HomeDir {
t.Errorf("openClawHome() = %q, want %q (account home from user.Current)", got, u.HomeDir)
}
}
// TestOpenClawHome_TildeOpenClawHomeUsesUserDatabaseFallback pins that
// a tilde-form OPENCLAW_HOME ("~/custom") expands against the
// user-database fallback when HOME and USERPROFILE are both unset.
// Without the user.Current() step in osHome this would have failed
// (returning "") and dropped the bind back to the audit's
// "path must be absolute" error.
func TestOpenClawHome_TildeOpenClawHomeUsesUserDatabaseFallback(t *testing.T) {
u, err := user.Current()
if err != nil || u.HomeDir == "" {
t.Skip("os/user.Current() unavailable on this runner")
}
setFakeOSHome(t, "")
t.Setenv("OPENCLAW_HOME", "~/custom")
got := openClawHome()
want := filepath.Join(u.HomeDir, "custom")
if got != want {
t.Errorf("openClawHome() = %q, want %q", got, want)
}
}
// TestExpandTildePath covers the full input grid for expandTildePath:
// bare tilde, tilde-slash, tilde + suffix, nested suffix, plain absolute
// and relative literals, and the intentionally-unchanged forms (~user,
// ~foo) that OpenClaw does not expand either.
func TestExpandTildePath(t *testing.T) {
fakeHome := t.TempDir()
absFixture := filepath.Join(fakeHome, "abs.json")
setFakeOSHome(t, fakeHome)
t.Setenv("OPENCLAW_HOME", "")
tests := []struct {
name string
in string
want string
}{
{"empty", "", ""},
{"bare tilde", "~", fakeHome},
{"tilde slash", "~/", fakeHome},
{"tilde with file", "~/secret.json", filepath.Join(fakeHome, "secret.json")},
{"tilde with nested path", "~/.openclaw/secret.json", filepath.Join(fakeHome, ".openclaw/secret.json")},
{"absolute unchanged", absFixture, absFixture},
{"relative unchanged", "foo/bar", "foo/bar"},
{"dot relative unchanged", "../foo", "../foo"},
{"tilde user form unchanged", "~root/foo", "~root/foo"},
{"tilde without separator unchanged", "~foo", "~foo"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := expandTildePath(tc.in)
if got != tc.want {
t.Errorf("expandTildePath(%q) = %q, want %q", tc.in, got, tc.want)
}
})
}
}
// TestExpandTildePath_RespectsOpenClawHome verifies that with
// OPENCLAW_HOME set, tilde expansion uses that custom home rather than
// the OS home — the integration-level invariant that closes the
// internal inconsistency CodeX's first review flagged.
func TestExpandTildePath_RespectsOpenClawHome(t *testing.T) {
homeDir := t.TempDir()
clawHome := t.TempDir()
setFakeOSHome(t, homeDir)
t.Setenv("OPENCLAW_HOME", clawHome)
got := expandTildePath("~/secret.json")
want := filepath.Join(clawHome, "secret.json")
if got != want {
t.Errorf("expandTildePath(%q) = %q, want %q (should use OPENCLAW_HOME)", "~/secret.json", got, want)
}
if got == filepath.Join(homeDir, "secret.json") {
t.Errorf("expandTildePath unexpectedly used OS home %q instead of OPENCLAW_HOME %q", homeDir, clawHome)
}
}
// TestExpandTildePath_FallsBackToUserDatabase is the end-to-end
// equivalent of TestOpenClawHome_FallsBackToUserDatabase: with HOME and
// USERPROFILE both unset, expandTildePath still resolves `~/foo` via
// osHome's user.Current() step. Matches Node os.homedir() and keeps
// OpenClaw-authored configs working in minimal-env shells.
func TestExpandTildePath_FallsBackToUserDatabase(t *testing.T) {
u, err := user.Current()
if err != nil || u.HomeDir == "" {
t.Skip("os/user.Current() unavailable on this runner")
}
setFakeOSHome(t, "")
t.Setenv("OPENCLAW_HOME", "")
got := expandTildePath("~/foo")
want := filepath.Join(u.HomeDir, "foo")
if got != want {
t.Errorf("expandTildePath(~/foo) = %q, want %q", got, want)
}
}
// TestOpenClawHome_OSHomeNormalization pins OpenClaw's sentinel
// normalisation on the env chain: the literals "undefined" / "null" /
// blank-or-whitespace are all treated as unset, so a JS-flavoured
// accidentally-stringified env value (e.g. `HOME=undefined` from a
// shell wrapper) doesn't end up as a literal directory component when
// the user authored `~/secret`. Combined with the user.Current()
// fallback further down (see TestOpenClawHome_FallsBackToUserDatabase),
// the contract is: a malformed HOME falls through to USERPROFILE first,
// and only if that's also unset/sentinel do we go to the user database.
func TestOpenClawHome_OSHomeNormalization(t *testing.T) {
isolateRuntimeWrites(t)
userProfileDir := t.TempDir()
homeWinsDir := t.TempDir()
tests := []struct {
name string
home string
userProfile string
want string
}{
{"HOME=undefined falls through to USERPROFILE", "undefined", userProfileDir, userProfileDir},
{"HOME=null falls through to USERPROFILE", "null", userProfileDir, userProfileDir},
{"HOME=whitespace falls through to USERPROFILE", " ", userProfileDir, userProfileDir},
{"HOME wins over USERPROFILE when both are valid", homeWinsDir, userProfileDir, homeWinsDir},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Setenv("HOME", tc.home)
t.Setenv("USERPROFILE", tc.userProfile)
t.Setenv("OPENCLAW_HOME", "")
if got := openClawHome(); got != tc.want {
t.Errorf("openClawHome() = %q, want %q", got, tc.want)
}
})
}
}
// TestOpenClawHome_SentinelHOMEFallsToUserDatabaseNotCwd pins the
// deliberate hybrid documented on osHome: with HOME a sentinel literal
// and USERPROFILE unset, OpenClaw would fall back to process.cwd();
// this implementation falls to the OS user database instead. The
// account home is both safer (cwd-independent) and more useful (it is
// where the user originally authored `~/...` against), so we prefer it
// over either OpenClaw's cwd fallback or a strict reject.
func TestOpenClawHome_SentinelHOMEFallsToUserDatabaseNotCwd(t *testing.T) {
isolateRuntimeWrites(t)
u, err := user.Current()
if err != nil || u.HomeDir == "" {
t.Skip("os/user.Current() unavailable on this runner")
}
t.Setenv("HOME", "undefined")
t.Setenv("USERPROFILE", "")
t.Setenv("OPENCLAW_HOME", "")
got := openClawHome()
if got != u.HomeDir {
t.Errorf("openClawHome() = %q, want %q (account home, not cwd)", got, u.HomeDir)
}
}
// TestExpandTildePath_BackslashPreservedOnPOSIX pins that `~\secret.json`
// expands by replacing only the `~` byte, leaving the backslash literally
// as part of the filename — matching OpenClaw's regex-replace semantics
// (`/^~(?=$|[\\/])/`) rather than going through filepath.Join (which would
// drop the backslash on POSIX). On Windows backslash is a real separator,
// so the literal-byte invariant doesn't apply.
func TestExpandTildePath_BackslashPreservedOnPOSIX(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("backslash is a path separator on Windows; invariant only applies on POSIX")
}
fakeHome := t.TempDir()
setFakeOSHome(t, fakeHome)
t.Setenv("OPENCLAW_HOME", "")
got := expandTildePath(`~\secret.json`)
want := fakeHome + `\secret.json`
if got != want {
t.Errorf("expandTildePath(%q) = %q, want %q (backslash should be preserved as filename byte)", `~\secret.json`, got, want)
}
}