From 549881b4112bab6d8aca48c1279a8ab1e29c1552 Mon Sep 17 00:00:00 2001 From: fullstackjam Date: Mon, 1 Jun 2026 22:50:09 +0800 Subject: [PATCH] feat: clone external oh-my-zsh plugins on snapshot restore OpenBoot only wrote plugin names into .zshrc's plugins=() array but never cloned external plugins (zsh-autosuggestions, zsh-syntax-highlighting, ...) into $ZSH_CUSTOM/plugins, so those plugins silently failed to load on a restored machine. Add a curated embedded catalog (internal/config/data/zsh-plugins.yaml, mirroring the screen-recording-packages.yaml precedent) mapping known external plugin names to https git repos. On RestoreFromSnapshot, any plugin present in the catalog is git-cloned into $ZSH_CUSTOM/plugins before plugins=() is written; built-in and unknown names are left untouched. - Snapshot/contract unchanged: plugins stays []string, URL never persisted to .zshrc and never enters the snapshot. - The only exec stays in internal/system via a cloneRunner seam, so no new no-direct-exec violation (baseline diff is a line shift from the import). - cfg.DryRun gated, https-only, idempotent skip-if-exists, non-fatal clone. Baseline edits are line-number shifts only (import added one line); the hand-authored reason annotations are preserved. --- internal/archtest/baseline/dryrun.txt | 6 +- internal/archtest/baseline/no-direct-exec.txt | 2 +- internal/config/data/zsh-plugins.yaml | 15 ++ internal/config/types.go | 12 ++ internal/config/zshplugins.go | 43 +++++ internal/config/zshplugins_test.go | 37 +++++ internal/shell/clone_plugins_test.go | 156 ++++++++++++++++++ internal/shell/shell.go | 66 ++++++++ 8 files changed, 333 insertions(+), 4 deletions(-) create mode 100644 internal/config/data/zsh-plugins.yaml create mode 100644 internal/config/zshplugins.go create mode 100644 internal/config/zshplugins_test.go create mode 100644 internal/shell/clone_plugins_test.go diff --git a/internal/archtest/baseline/dryrun.txt b/internal/archtest/baseline/dryrun.txt index 2a2bc53..2184e82 100644 --- a/internal/archtest/baseline/dryrun.txt +++ b/internal/archtest/baseline/dryrun.txt @@ -8,9 +8,9 @@ internal/doctor/doctor.go:54 # read-only diagnostic runner; doctor does not muta internal/dotfiles/dotfiles.go:260 # helper writes conflict backups only from linkWithStow after Link dry-run gate internal/dotfiles/dotfiles.go:270 # helper restores conflict backups only after a non-dry-run stow failure internal/dotfiles/dotfiles.go:327 # helper removes conflict files only from linkWithStow after Link dry-run gate -internal/shell/shell.go:248 # helper patches .zshrc only after RestoreFromSnapshot dry-run gate -internal/shell/shell.go:251 # helper patches .zshrc only after RestoreFromSnapshot dry-run gate -internal/shell/shell.go:252 # helper cleanup only after RestoreFromSnapshot dry-run gate +internal/shell/shell.go:249 # helper patches .zshrc only after RestoreFromSnapshot dry-run gate +internal/shell/shell.go:252 # helper patches .zshrc only after RestoreFromSnapshot dry-run gate +internal/shell/shell.go:253 # helper cleanup only after RestoreFromSnapshot dry-run gate internal/snapshot/capture.go:200 # read-only package inventory probe internal/snapshot/capture.go:239 # read-only package inventory probe internal/snapshot/capture.go:269 # read-only macOS defaults probe diff --git a/internal/archtest/baseline/no-direct-exec.txt b/internal/archtest/baseline/no-direct-exec.txt index 7bda143..32c0775 100644 --- a/internal/archtest/baseline/no-direct-exec.txt +++ b/internal/archtest/baseline/no-direct-exec.txt @@ -14,7 +14,7 @@ internal/dotfiles/dotfiles.go:379 internal/installer/step_system.go:85 internal/npm/npm.go:22 internal/permissions/screen_recording_cgo.go:21 -internal/shell/shell.go:177 +internal/shell/shell.go:178 internal/updater/updater.go:205 internal/updater/updater.go:212 internal/updater/updater.go:219 diff --git a/internal/config/data/zsh-plugins.yaml b/internal/config/data/zsh-plugins.yaml new file mode 100644 index 0000000..64671d3 --- /dev/null +++ b/internal/config/data/zsh-plugins.yaml @@ -0,0 +1,15 @@ +# External oh-my-zsh plugins that are NOT bundled with the OMZ install and +# require a git clone into $ZSH_CUSTOM/plugins before they work. +# Built-in plugins (git, docker, kubectl, ...) are deliberately absent: they +# ship with oh-my-zsh and only need a name in the .zshrc plugins=() array. +# Source of truth for now is this embedded file; a server overlay can be added +# later via /api/zsh-plugins (mirror packages_remote.go) without a contract change. +plugins: + - name: zsh-autosuggestions + repo: https://github.com/zsh-users/zsh-autosuggestions + - name: zsh-syntax-highlighting + repo: https://github.com/zsh-users/zsh-syntax-highlighting + - name: zsh-completions + repo: https://github.com/zsh-users/zsh-completions + - name: zsh-history-substring-search + repo: https://github.com/zsh-users/zsh-history-substring-search diff --git a/internal/config/types.go b/internal/config/types.go index fbac230..54f5207 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -182,3 +182,15 @@ type presetsData struct { type screenRecordingData struct { Packages []string `yaml:"packages"` } + +// zshPluginsData mirrors data/zsh-plugins.yaml: external oh-my-zsh plugins that +// require a git clone into $ZSH_CUSTOM/plugins, keyed by the name used in the +// .zshrc plugins=() array. +type zshPluginsData struct { + Plugins []zshPluginEntry `yaml:"plugins"` +} + +type zshPluginEntry struct { + Name string `yaml:"name"` + Repo string `yaml:"repo"` +} diff --git a/internal/config/zshplugins.go b/internal/config/zshplugins.go new file mode 100644 index 0000000..5d4492f --- /dev/null +++ b/internal/config/zshplugins.go @@ -0,0 +1,43 @@ +package config + +import ( + "embed" + "log" + + "gopkg.in/yaml.v3" +) + +//go:embed data/zsh-plugins.yaml +var zshPluginsYAML embed.FS + +// ZshPluginRepoURL returns the git repo URL for a known external oh-my-zsh +// plugin name, and whether the name is a known external plugin at all. +// +// The lookup is deliberately conservative: only names with an explicit curated +// URL are treated as external. Built-in OMZ plugins (git, docker, kubectl, ...) +// and unknown/typo'd names are not in the catalog and return ("", false) — +// callers leave those untouched in plugins=() and never attempt a clone. +func ZshPluginRepoURL(name string) (string, bool) { + for _, p := range loadZshPlugins() { + if p.Name == name { + return p.Repo, true + } + } + return "", false +} + +func loadZshPlugins() []zshPluginEntry { + data, err := zshPluginsYAML.ReadFile("data/zsh-plugins.yaml") + if err != nil { + log.Printf("Warning: failed to read zsh-plugins.yaml: %v", err) + return nil + } + + var zpd zshPluginsData + if err := yaml.Unmarshal(data, &zpd); err != nil { + log.Printf("Warning: failed to parse zsh-plugins.yaml: %v", err) + return nil + } + + return zpd.Plugins +} diff --git a/internal/config/zshplugins_test.go b/internal/config/zshplugins_test.go new file mode 100644 index 0000000..79c62d8 --- /dev/null +++ b/internal/config/zshplugins_test.go @@ -0,0 +1,37 @@ +package config + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestZshPluginRepoURL_KnownExternal(t *testing.T) { + url, ok := ZshPluginRepoURL("zsh-autosuggestions") + assert.True(t, ok, "zsh-autosuggestions should be a known external plugin") + assert.True(t, strings.HasPrefix(url, "https://"), "repo URL must be https, got %q", url) +} + +func TestZshPluginRepoURL_BuiltinReturnsFalse(t *testing.T) { + for _, builtin := range []string{"git", "docker", "kubectl", "z"} { + url, ok := ZshPluginRepoURL(builtin) + assert.False(t, ok, "%s is a built-in OMZ plugin and must not be in the catalog", builtin) + assert.Empty(t, url) + } +} + +func TestZshPluginRepoURL_UnknownReturnsFalse(t *testing.T) { + url, ok := ZshPluginRepoURL("totally-made-up-plugin-xyz") + assert.False(t, ok) + assert.Empty(t, url) +} + +func TestZshPluginRepoURL_AllCatalogEntriesAreHTTPS(t *testing.T) { + entries := loadZshPlugins() + assert.NotEmpty(t, entries, "catalog must not be empty") + for _, e := range entries { + assert.NotEmpty(t, e.Name, "every entry needs a name") + assert.True(t, strings.HasPrefix(e.Repo, "https://"), "%s repo must be https, got %q", e.Name, e.Repo) + } +} diff --git a/internal/shell/clone_plugins_test.go b/internal/shell/clone_plugins_test.go new file mode 100644 index 0000000..dfafc93 --- /dev/null +++ b/internal/shell/clone_plugins_test.go @@ -0,0 +1,156 @@ +package shell + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeCatalog returns a resolver that treats only the given names as external. +func fakeCatalog(m map[string]string) func(string) (string, bool) { + return func(name string) (string, bool) { + url, ok := m[name] + return url, ok + } +} + +// withFakes swaps the package-level seams for the duration of a test and +// returns a pointer to a slice that records (url, dest) clone invocations. +func withFakes(t *testing.T, catalog map[string]string) *[][2]string { + t.Helper() + var calls [][2]string + + origResolve := resolvePluginURL + origClone := cloneRunner + t.Cleanup(func() { + resolvePluginURL = origResolve + cloneRunner = origClone + }) + + resolvePluginURL = fakeCatalog(catalog) + cloneRunner = func(url, dest string) error { + calls = append(calls, [2]string{url, dest}) + // Simulate a successful clone by creating the destination dir, so a + // second pass exercises the idempotent skip-if-exists path. + _ = os.MkdirAll(dest, 0700) + return nil + } + return &calls +} + +func customPluginsDir(home string) string { + return filepath.Join(home, ".oh-my-zsh", "custom", "plugins") +} + +func TestCloneExternalPlugins_ClonesKnownExternal(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + calls := withFakes(t, map[string]string{ + "zsh-autosuggestions": "https://github.com/zsh-users/zsh-autosuggestions", + }) + + err := cloneExternalPlugins([]string{"git", "zsh-autosuggestions"}, false) + require.NoError(t, err) + + require.Len(t, *calls, 1, "only the external plugin should be cloned") + assert.Equal(t, "https://github.com/zsh-users/zsh-autosuggestions", (*calls)[0][0]) + assert.Equal(t, filepath.Join(customPluginsDir(home), "zsh-autosuggestions"), (*calls)[0][1]) +} + +func TestCloneExternalPlugins_BuiltinIsNotCloned(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + calls := withFakes(t, map[string]string{ + "zsh-autosuggestions": "https://github.com/zsh-users/zsh-autosuggestions", + }) + + err := cloneExternalPlugins([]string{"git", "docker", "kubectl"}, false) + require.NoError(t, err) + assert.Empty(t, *calls, "built-in plugins must never trigger a clone") +} + +func TestCloneExternalPlugins_SkipsWhenDestExists(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + calls := withFakes(t, map[string]string{ + "zsh-autosuggestions": "https://github.com/zsh-users/zsh-autosuggestions", + }) + // Pre-create the destination — already installed. + require.NoError(t, os.MkdirAll(filepath.Join(customPluginsDir(home), "zsh-autosuggestions"), 0700)) + + err := cloneExternalPlugins([]string{"zsh-autosuggestions"}, false) + require.NoError(t, err) + assert.Empty(t, *calls, "an already-cloned plugin must be skipped idempotently") +} + +func TestCloneExternalPlugins_DryRunDoesNotClone(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + calls := withFakes(t, map[string]string{ + "zsh-autosuggestions": "https://github.com/zsh-users/zsh-autosuggestions", + }) + + err := cloneExternalPlugins([]string{"zsh-autosuggestions"}, true) + require.NoError(t, err) + assert.Empty(t, *calls, "dry-run must not clone") + // And no destination directory should be created. + _, statErr := os.Stat(filepath.Join(customPluginsDir(home), "zsh-autosuggestions")) + assert.True(t, os.IsNotExist(statErr), "dry-run must not touch the filesystem") +} + +func TestCloneExternalPlugins_NonHTTPSSkipped(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + calls := withFakes(t, map[string]string{ + "evil-plugin": "git@github.com:attacker/evil.git", + }) + + err := cloneExternalPlugins([]string{"evil-plugin"}, false) + require.NoError(t, err) + assert.Empty(t, *calls, "non-https repo URLs must be skipped") +} + +func TestCloneExternalPlugins_CloneFailureIsNonFatal(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + origResolve := resolvePluginURL + origClone := cloneRunner + t.Cleanup(func() { + resolvePluginURL = origResolve + cloneRunner = origClone + }) + resolvePluginURL = fakeCatalog(map[string]string{ + "zsh-autosuggestions": "https://github.com/zsh-users/zsh-autosuggestions", + }) + cloneRunner = func(url, dest string) error { return errors.New("network down") } + + // A failed clone must not abort the restore. + err := cloneExternalPlugins([]string{"zsh-autosuggestions"}, false) + require.NoError(t, err) +} + +func TestCloneExternalPlugins_RestoreWritesBareNames(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + withFakes(t, map[string]string{ + "zsh-autosuggestions": "https://github.com/zsh-users/zsh-autosuggestions", + }) + // Pre-create ~/.oh-my-zsh so RestoreFromSnapshot's install gate is skipped. + require.NoError(t, os.MkdirAll(filepath.Join(home, ".oh-my-zsh"), 0700)) + zshrcPath := filepath.Join(home, ".zshrc") + require.NoError(t, os.WriteFile(zshrcPath, []byte("plugins=(git)\n"), 0600)) + + err := RestoreFromSnapshot(true, "agnoster", []string{"git", "zsh-autosuggestions"}, false) + require.NoError(t, err) + + out, err := os.ReadFile(zshrcPath) + require.NoError(t, err) + // The URL never leaks into .zshrc — only bare names. + assert.Contains(t, string(out), "plugins=(git zsh-autosuggestions)") + assert.NotContains(t, string(out), "https://") +} diff --git a/internal/shell/shell.go b/internal/shell/shell.go index e94cf3a..3155cd7 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/openbootdotdev/openboot/internal/config" "github.com/openbootdotdev/openboot/internal/httputil" "github.com/openbootdotdev/openboot/internal/system" "github.com/openbootdotdev/openboot/internal/ui" @@ -255,6 +256,64 @@ func patchZshrcBlock(zshrcPath, theme string, plugins []string) error { return nil } +// resolvePluginURL maps a plugins=() entry to its external git repo URL. +// It is a var so tests can inject a fake catalog without the embedded data. +var resolvePluginURL = config.ZshPluginRepoURL + +// cloneRunner clones an external plugin repo into dest. It is a var so tests +// can record invocations without a real git binary or network. The only exec +// call lives in internal/system, so no execAllowedPaths baseline change is +// needed. +var cloneRunner = func(url, dest string) error { + return system.RunCommand("git", "clone", "--depth", "1", url, dest) +} + +// cloneExternalPlugins git-clones any plugins in the list that are known +// external oh-my-zsh plugins (present in the catalog with a repo URL) into +// $ZSH_CUSTOM/plugins. Built-in and unknown plugins are left untouched — they +// stay as bare names in plugins=(). A failed clone is non-fatal: it warns and +// continues so one bad repo never aborts the whole restore and the plugins=() +// block is still written. +func cloneExternalPlugins(plugins []string, dryRun bool) error { + home, err := system.HomeDir() + if err != nil { + return fmt.Errorf("clone external plugins: %w", err) + } + customPlugins := filepath.Join(home, ".oh-my-zsh", "custom", "plugins") + + for _, name := range plugins { + url, ok := resolvePluginURL(name) + if !ok { + continue // built-in or unknown — leave untouched + } + // Defense in depth: the catalog is curated, but a server overlay may + // one day supply URLs. Only ever clone over https. + if !strings.HasPrefix(url, "https://") { + ui.Warn(fmt.Sprintf("Skipping plugin %s: non-https repo URL %q", name, url)) + continue + } + + dest := filepath.Join(customPlugins, name) + if _, err := os.Stat(dest); err == nil { + continue // already cloned — idempotent skip + } + + if dryRun { + ui.DryRunMsg("Would clone %s to %s", url, dest) + continue + } + + if err := os.MkdirAll(customPlugins, 0700); err != nil { + return fmt.Errorf("create %s: %w", customPlugins, err) + } + if err := cloneRunner(url, dest); err != nil { + ui.Warn(fmt.Sprintf("Failed to clone plugin %s: %v", name, err)) + continue + } + } + return nil +} + func RestoreFromSnapshot(ohMyZsh bool, theme string, plugins []string, dryRun bool) error { if !ohMyZsh { return nil @@ -270,6 +329,13 @@ func RestoreFromSnapshot(ohMyZsh bool, theme string, plugins []string, dryRun bo } } + // External plugins (zsh-autosuggestions, ...) are not bundled with OMZ and + // must be git-cloned into $ZSH_CUSTOM/plugins before plugins=() references + // them. Built-in plugins are left untouched. + if err := cloneExternalPlugins(plugins, dryRun); err != nil { + return fmt.Errorf("clone external plugins: %w", err) + } + home, err := system.HomeDir() if err != nil { return fmt.Errorf("configure zshrc: %w", err)