From c0da89e6742fec71a92a41c4abf491a02317a936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=89=E6=BE=9C?= Date: Fri, 14 Aug 2026 11:33:55 +0800 Subject: [PATCH] fix(skills): make app-bundle detection gate HOME-independent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The allowSystemApps gate (homeDir == systemHome) was effectively a no-op in production: systemHome came from os.UserHomeDir, which honors the $HOME env override just like homeDir, so the two were always equal and the gate never fired when $HOME was overridden. ResolveSystemHomeDir now prefers the OS user database (getpwuid on Unix), which is independent of $HOME, falling back to $HOME only when the user record cannot be resolved. Production behavior is unchanged (a real $HOME still matches); an isolated/overridden HOME now correctly skips machine-wide /Applications discovery for zcode/minimax. The app surface references the same shared resolver. This is the correct fix for the hermeticity concern (machine-wide state leaking into an isolated HOME): there is no cross-surface production inconsistency to port — script installers always operate on the real user HOME in practice, so they need no gate. --- internal/app/skill_setup.go | 2 +- internal/upgrade/paths.go | 20 +++++++++++- internal/upgrade/system_home_test.go | 49 ++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 internal/upgrade/system_home_test.go diff --git a/internal/app/skill_setup.go b/internal/app/skill_setup.go index 6c0becf4..2d2abf24 100644 --- a/internal/app/skill_setup.go +++ b/internal/app/skill_setup.go @@ -127,7 +127,7 @@ var ( skillSetupExecutable = os.Executable skillSetupGetwd = os.Getwd skillSetupUserHomeDir = os.UserHomeDir - skillSetupSystemHomeDir = os.UserHomeDir + skillSetupSystemHomeDir = upgrade.ResolveSystemHomeDir skillSetupRemoveAll = os.RemoveAll skillSetupBackupAndRemove = upgrade.BackupAndRemoveSkillDir skillSetupRestoreBackup = upgrade.RestoreSkillPath diff --git a/internal/upgrade/paths.go b/internal/upgrade/paths.go index f0424ae6..d0edb495 100644 --- a/internal/upgrade/paths.go +++ b/internal/upgrade/paths.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "os" + "os/user" "path/filepath" "runtime" "sort" @@ -139,9 +140,26 @@ var universalSkillDirs = map[string]bool{ ".amp/skills": true, } +// upgradeCurrentUser is the test seam backing ResolveSystemHomeDir. +var upgradeCurrentUser = user.Current + +// ResolveSystemHomeDir returns the operating-system user's real home directory +// independent of the $HOME environment variable. $HOME may be overridden in +// isolated or verifier contexts, so the app-bundle detection gate +// (allowSystemApps = homeDir == systemHome) must compare against a HOME- +// independent source to actually mean "operating on the real user profile". +// It prefers the OS user database (getpwuid on Unix) and falls back to $HOME +// only when the user record cannot be resolved. +func ResolveSystemHomeDir() (string, error) { + if u, err := upgradeCurrentUser(); err == nil && u.HomeDir != "" { + return u.HomeDir, nil + } + return os.UserHomeDir() +} + var ( upgradeUserHomeDir = os.UserHomeDir - upgradeSystemHomeDir = os.UserHomeDir + upgradeSystemHomeDir = ResolveSystemHomeDir upgradeExecutable = os.Executable upgradeEvalSymlinks = filepath.EvalSymlinks upgradeCopyDir = copyDir diff --git a/internal/upgrade/system_home_test.go b/internal/upgrade/system_home_test.go new file mode 100644 index 00000000..7870742f --- /dev/null +++ b/internal/upgrade/system_home_test.go @@ -0,0 +1,49 @@ +package upgrade + +import ( + "errors" + "os/user" + "testing" + + "github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/testseam" +) + +// TestCrossPlatformCoverageSystemHomeIndependentOfHOME pins that the app-bundle +// detection gate compares against a HOME-independent system home: ResolveSystemHomeDir +// prefers the OS user record (getpwuid on Unix) and only falls back to $HOME when that +// record cannot be resolved. Without this, allowSystemApps = homeDir == systemHome is a +// no-op whenever $HOME is overridden, because both sides would honor the same override. +func TestCrossPlatformCoverageSystemHomeIndependentOfHOME(t *testing.T) { + // Success path: the OS user database home is returned as-is. + testseam.Swap(t, &upgradeCurrentUser, func() (*user.User, error) { + return &user.User{HomeDir: "/tmp/dws-real-system-home"}, nil + }) + got, err := ResolveSystemHomeDir() + if err != nil || got != "/tmp/dws-real-system-home" { + t.Fatalf("ResolveSystemHomeDir success = (%q, %v), want /tmp/dws-real-system-home", got, err) + } + + // Fallback path: when the user record is unavailable, fall back to $HOME. + testseam.Swap(t, &upgradeCurrentUser, func() (*user.User, error) { + return nil, errors.New("user record unavailable") + }) + got, err = ResolveSystemHomeDir() + if err != nil { + t.Fatalf("ResolveSystemHomeDir fallback returned error: %v", err) + } + if got == "" { + t.Fatalf("ResolveSystemHomeDir fallback returned empty home") + } + + // Empty HomeDir is also treated as "unresolved" and falls back. + testseam.Swap(t, &upgradeCurrentUser, func() (*user.User, error) { + return &user.User{HomeDir: ""}, nil + }) + got, err = ResolveSystemHomeDir() + if err != nil { + t.Fatalf("ResolveSystemHomeDir empty-home fallback returned error: %v", err) + } + if got == "" { + t.Fatalf("ResolveSystemHomeDir empty-home fallback returned empty home") + } +}