From 576b1f58344db0ffa61129b8856770e1def6a399 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Sun, 26 Jul 2026 15:02:10 +0300 Subject: [PATCH] Enforce install-dir containment on uninstall; resolve manifest verification key Uninstall walked plugin file names straight from the manifest and passed the joined path to removeOwnedFile, while the install path (reconcile- PluginFiles) rejected any name that resolved outside the plugin install directory. Both sides now share a pathWithin helper, so a name that escapes installDir is reported as a RemovalError instead of being acted on. Manifest signature verification previously required the caller to supply Config.ManifestPublicKey; a nil value silently skipped verification. The key is now resolved from Config, then PILOT_SKILLINJECT_PUBKEY, then ~/.pilot/skillinject.pub, then the compiled-in DefaultManifestPublicKey- Hex. Config.RequireSignedManifest (and PILOT_SKILLINJECT_REQUIRE_SIG) makes a resolved key mandatory. Both default off, so behaviour with no key configured is unchanged. Co-Authored-By: Claude Opus 5 --- manifest.go | 144 +++++++++++++++++- skillinject.go | 15 +- uninstall.go | 14 ++ zz_manifest_key_test.go | 268 +++++++++++++++++++++++++++++++++ zz_uninstall_traversal_test.go | 139 +++++++++++++++++ 5 files changed, 573 insertions(+), 7 deletions(-) create mode 100644 zz_manifest_key_test.go create mode 100644 zz_uninstall_traversal_test.go diff --git a/manifest.go b/manifest.go index 14d687d..795b86d 100644 --- a/manifest.go +++ b/manifest.go @@ -5,6 +5,8 @@ package skillinject import ( "context" "crypto/ed25519" + "encoding/base64" + "encoding/hex" "encoding/json" "fmt" "io" @@ -123,13 +125,33 @@ type ManifestPluginAllowList struct { EntriesJsonPath string `json:"entriesJsonPath"` } +// EnvManifestPublicKey names the environment variable holding an +// Ed25519 public key (hex or base64) used to verify fetched resources. +const EnvManifestPublicKey = "PILOT_SKILLINJECT_PUBKEY" + +// EnvRequireSignedManifest names the environment variable that makes a +// verification key mandatory. Recognised true values: "1", "true", "yes". +const EnvRequireSignedManifest = "PILOT_SKILLINJECT_REQUIRE_SIG" + +// manifestPublicKeyRel is the path under ~/.pilot holding a trusted +// Ed25519 public key in hex or base64 form, one key per file. +const manifestPublicKeyRel = "skillinject.pub" + +// DefaultManifestPublicKeyHex is the built-in verification key, hex +// encoded. Empty means no built-in key is compiled in, in which case the +// Config field, the environment, and the on-disk trust file are the only +// sources. +const DefaultManifestPublicKeyHex = "" + // fetcher is a small wrapper around http.Client that returns response // bodies. Pulled out so tests can inject a fake. type fetcher struct { httpClient *http.Client manifestURL string repoBase string - publicKey ed25519.PublicKey // nil = skip verification (backward compat) + publicKey ed25519.PublicKey // nil = no key resolved + keyErr error // non-nil when a configured key failed to decode + requireSig bool // fail fetches when publicKey is nil } func newFetcher(cfg Config) *fetcher { @@ -148,7 +170,102 @@ func newFetcher(cfg Config) *fetcher { if !strings.HasSuffix(rb, "/") { rb += "/" } - return &fetcher{httpClient: c, manifestURL: mu, repoBase: rb, publicKey: cfg.ManifestPublicKey} + key, keyErr := resolveManifestPublicKey(cfg) + return &fetcher{ + httpClient: c, + manifestURL: mu, + repoBase: rb, + publicKey: key, + keyErr: keyErr, + requireSig: requireSignedManifest(cfg), + } +} + +// requireSignedManifest reports whether a verification key is mandatory, +// combining the Config field with EnvRequireSignedManifest. +func requireSignedManifest(cfg Config) bool { + if cfg.RequireSignedManifest { + return true + } + switch strings.ToLower(strings.TrimSpace(os.Getenv(EnvRequireSignedManifest))) { + case "1", "true", "yes": + return true + } + return false +} + +// resolveManifestPublicKey returns the Ed25519 public key used to verify +// fetched resources, checking the Config field, then the environment, +// then the on-disk trust file, then the built-in default. A nil key with +// a nil error means no source supplied one. +func resolveManifestPublicKey(cfg Config) (ed25519.PublicKey, error) { + if len(cfg.ManifestPublicKey) > 0 { + if len(cfg.ManifestPublicKey) != ed25519.PublicKeySize { + return nil, fmt.Errorf("configured manifest public key is %d bytes, want %d", + len(cfg.ManifestPublicKey), ed25519.PublicKeySize) + } + return cfg.ManifestPublicKey, nil + } + if v := strings.TrimSpace(os.Getenv(EnvManifestPublicKey)); v != "" { + k, err := decodeEd25519PublicKey(v) + if err != nil { + return nil, fmt.Errorf("%s: %w", EnvManifestPublicKey, err) + } + return k, nil + } + if p := manifestPublicKeyPath(cfg.Home); p != "" { + if raw, err := os.ReadFile(p); err == nil { + if v := strings.TrimSpace(string(raw)); v != "" { + k, derr := decodeEd25519PublicKey(v) + if derr != nil { + return nil, fmt.Errorf("%s: %w", p, derr) + } + return k, nil + } + } + } + if DefaultManifestPublicKeyHex != "" { + k, err := decodeEd25519PublicKey(DefaultManifestPublicKeyHex) + if err != nil { + return nil, fmt.Errorf("built-in manifest public key: %w", err) + } + return k, nil + } + return nil, nil +} + +// manifestPublicKeyPath returns the on-disk trust file location, or "" +// when the home directory cannot be determined. +func manifestPublicKeyPath(home string) string { + if home == "" { + h, err := os.UserHomeDir() + if err != nil { + return "" + } + home = h + } + return filepath.Join(home, ".pilot", manifestPublicKeyRel) +} + +// decodeEd25519PublicKey parses a public key encoded as hex, standard +// base64, or raw (unpadded) base64. +func decodeEd25519PublicKey(s string) (ed25519.PublicKey, error) { + s = strings.TrimSpace(s) + decoders := []func(string) ([]byte, error){ + hex.DecodeString, + base64.StdEncoding.DecodeString, + base64.RawStdEncoding.DecodeString, + } + for _, dec := range decoders { + b, err := dec(s) + if err != nil { + continue + } + if len(b) == ed25519.PublicKeySize { + return ed25519.PublicKey(b), nil + } + } + return nil, fmt.Errorf("not a %d-byte hex or base64 ed25519 public key", ed25519.PublicKeySize) } func (f *fetcher) get(ctx context.Context, url string) ([]byte, error) { @@ -201,9 +318,16 @@ func (f *fetcher) fetchRepoFile(ctx context.Context, relPath string) ([]byte, er // getOrVerify returns the body at url. When f.publicKey is set, it also // fetches .sig and verifies the detached Ed25519 signature before -// returning. Without a public key, behavior matches get() exactly -// (backward compatible). +// returning. Without a public key, behavior matches get() exactly unless +// f.requireSig is set, in which case the fetch fails. func (f *fetcher) getOrVerify(ctx context.Context, url string) ([]byte, error) { + if f.keyErr != nil { + return nil, f.keyErr + } + if f.publicKey == nil && f.requireSig { + return nil, fmt.Errorf("signed manifest required but no public key resolved (set %s, %s, or Config.ManifestPublicKey)", + EnvManifestPublicKey, manifestPublicKeyPath("")) + } body, err := f.get(ctx, url) if err != nil { return nil, err @@ -221,6 +345,18 @@ func (f *fetcher) getOrVerify(ctx context.Context, url string) ([]byte, error) { return body, nil } +// pathWithin reports whether path, after lexical cleaning, resolves to +// dir itself or to a location beneath it. Both arguments are treated as +// plain lexical paths; symlinks are not resolved. +func pathWithin(dir, path string) bool { + cleanDir := filepath.Clean(dir) + cleanPath := filepath.Clean(path) + if cleanPath == cleanDir { + return true + } + return strings.HasPrefix(cleanPath, cleanDir+string(os.PathSeparator)) +} + // expandHome resolves "~/" in a manifest path against the user's home dir. func expandHome(p, home string) string { if strings.HasPrefix(p, "~/") { diff --git a/skillinject.go b/skillinject.go index 71e86d1..a6b54d3 100644 --- a/skillinject.go +++ b/skillinject.go @@ -75,8 +75,17 @@ type Config struct { // ManifestPublicKey, when set, enables Ed25519 detached-signature // verification on manifest + all fetched repo files. The daemon // fetches .sig alongside each resource and verifies before - // accepting. Nil (default) preserves the pre-verification behavior. + // accepting. When nil, the key is resolved from the environment + // (EnvManifestPublicKey), then the on-disk trust file + // (~/.pilot/skillinject.pub), then the built-in + // DefaultManifestPublicKeyHex. If none of those yield a key, + // verification is skipped, preserving the pre-verification behavior. ManifestPublicKey ed25519.PublicKey + // RequireSignedManifest makes a verification key mandatory: when true + // and no key resolves from any source, every fetch fails instead of + // falling back to unverified transport trust. Also settable via + // EnvRequireSignedManifest. Default false. + RequireSignedManifest bool } // Run blocks running scan/reconcile ticks until ctx is cancelled. The @@ -358,8 +367,8 @@ func reconcilePluginFiles(f *fetcher, ctx context.Context, p *ManifestPlugin, ho out := make([]Outcome, 0, len(p.Files)) for _, pf := range p.Files { dst := filepath.Join(installDir, pf.Name) - // Reject path-traversal in pf.Name (e.g. "../../.ssh/authorized_keys") - if clean := filepath.Clean(dst); !strings.HasPrefix(clean, filepath.Clean(installDir)+string(os.PathSeparator)) && clean != filepath.Clean(installDir) { + // Only names that stay inside installDir are written. + if !pathWithin(installDir, dst) { out = append(out, Outcome{ Tool: p.ID, Kind: KindPluginFile, Path: dst, Action: ActionError, diff --git a/uninstall.go b/uninstall.go index ee3a943..b9ecf7b 100644 --- a/uninstall.go +++ b/uninstall.go @@ -134,6 +134,20 @@ func Uninstall(ctx context.Context, cfg Config) (*RemovalReport, error) { installDir := expandHome(mt.Plugin.InstallPath, home) for _, pf := range mt.Plugin.Files { dst := filepath.Join(installDir, pf.Name) + // Mirrors reconcilePluginFiles: only names that stay + // inside installDir are acted on. Anything that resolves + // outside was never written by the install path, so it is + // reported rather than removed. + if !pathWithin(installDir, dst) { + report.Removals = append(report.Removals, Removal{ + Tool: mt.Plugin.ID, + Kind: KindPluginFile, + Path: dst, + Action: RemovalError, + Err: fmt.Sprintf("path traversal: %q escapes install dir", pf.Name), + }) + continue + } report.Removals = append(report.Removals, removeOwnedFile(mt.Plugin.ID, KindPluginFile, dst)) } // Try to remove the plugin install dir if empty after. diff --git a/zz_manifest_key_test.go b/zz_manifest_key_test.go new file mode 100644 index 0000000..7b036ce --- /dev/null +++ b/zz_manifest_key_test.go @@ -0,0 +1,268 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package skillinject + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "encoding/base64" + "encoding/hex" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// newTestKey returns a deterministic-enough Ed25519 keypair for tests. +func newTestKey(t *testing.T) (ed25519.PublicKey, ed25519.PrivateKey) { + t.Helper() + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("generate key: %v", err) + } + return pub, priv +} + +func TestPathWithin(t *testing.T) { + t.Parallel() + cases := []struct { + dir, path string + want bool + }{ + {"/a/b", "/a/b", true}, + {"/a/b", "/a/b/c", true}, + {"/a/b", "/a/b/c/d.txt", true}, + {"/a/b/", "/a/b/c", true}, + {"/a/b", "/a/b/../c", false}, + {"/a/b", "/a/bc", false}, + {"/a/b", "/a", false}, + {"/a/b", "/a/b/../../etc/passwd", false}, + {"/a/b", "/etc/passwd", false}, + } + for _, c := range cases { + if got := pathWithin(c.dir, c.path); got != c.want { + t.Errorf("pathWithin(%q, %q) = %v, want %v", c.dir, c.path, got, c.want) + } + } +} + +func TestDecodeEd25519PublicKey(t *testing.T) { + t.Parallel() + pub, _ := newTestKey(t) + + for name, encoded := range map[string]string{ + "hex": hex.EncodeToString(pub), + "base64": base64.StdEncoding.EncodeToString(pub), + "rawbase64": base64.RawStdEncoding.EncodeToString(pub), + "padded": " " + hex.EncodeToString(pub) + "\n", + } { + got, err := decodeEd25519PublicKey(encoded) + if err != nil { + t.Errorf("%s: unexpected error: %v", name, err) + continue + } + if !got.Equal(pub) { + t.Errorf("%s: decoded key mismatch", name) + } + } + + for _, bad := range []string{"", "not-a-key", hex.EncodeToString(pub[:16])} { + if _, err := decodeEd25519PublicKey(bad); err == nil { + t.Errorf("decodeEd25519PublicKey(%q) succeeded, want error", bad) + } + } +} + +func TestResolveManifestPublicKey_ConfigWins(t *testing.T) { + pub, _ := newTestKey(t) + other, _ := newTestKey(t) + home := t.TempDir() + + t.Setenv(EnvManifestPublicKey, hex.EncodeToString(other)) + writeKeyFile(t, home, hex.EncodeToString(other)) + + got, err := resolveManifestPublicKey(Config{Home: home, ManifestPublicKey: pub}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !got.Equal(pub) { + t.Fatalf("Config key did not take precedence") + } +} + +func TestResolveManifestPublicKey_EnvBeatsFile(t *testing.T) { + pub, _ := newTestKey(t) + other, _ := newTestKey(t) + home := t.TempDir() + + t.Setenv(EnvManifestPublicKey, base64.StdEncoding.EncodeToString(pub)) + writeKeyFile(t, home, hex.EncodeToString(other)) + + got, err := resolveManifestPublicKey(Config{Home: home}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !got.Equal(pub) { + t.Fatalf("environment key did not take precedence over the trust file") + } +} + +func TestResolveManifestPublicKey_FileFallback(t *testing.T) { + pub, _ := newTestKey(t) + home := t.TempDir() + + t.Setenv(EnvManifestPublicKey, "") + writeKeyFile(t, home, hex.EncodeToString(pub)) + + got, err := resolveManifestPublicKey(Config{Home: home}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if !got.Equal(pub) { + t.Fatalf("trust file key not picked up") + } +} + +func TestResolveManifestPublicKey_NoSourceIsNilNoError(t *testing.T) { + t.Setenv(EnvManifestPublicKey, "") + got, err := resolveManifestPublicKey(Config{Home: t.TempDir()}) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if got != nil { + t.Fatalf("expected nil key when no source supplies one, got %d bytes", len(got)) + } +} + +func TestResolveManifestPublicKey_BadEncodingErrors(t *testing.T) { + t.Setenv(EnvManifestPublicKey, "zzzz-not-a-key") + if _, err := resolveManifestPublicKey(Config{Home: t.TempDir()}); err == nil { + t.Fatalf("expected an error for an undecodable environment key") + } +} + +func TestResolveManifestPublicKey_WrongSizeConfigKeyErrors(t *testing.T) { + if _, err := resolveManifestPublicKey(Config{Home: t.TempDir(), ManifestPublicKey: ed25519.PublicKey("short")}); err == nil { + t.Fatalf("expected an error for a wrong-size Config key") + } +} + +func TestRequireSignedManifest_EnvAndConfig(t *testing.T) { + t.Setenv(EnvRequireSignedManifest, "") + if requireSignedManifest(Config{}) { + t.Fatalf("default should not require a signature") + } + if !requireSignedManifest(Config{RequireSignedManifest: true}) { + t.Fatalf("Config field should require a signature") + } + for _, v := range []string{"1", "true", "YES"} { + t.Setenv(EnvRequireSignedManifest, v) + if !requireSignedManifest(Config{}) { + t.Errorf("%q should require a signature", v) + } + } + t.Setenv(EnvRequireSignedManifest, "0") + if requireSignedManifest(Config{}) { + t.Fatalf("%q should not require a signature", "0") + } +} + +// TestGetOrVerify_DefaultUnverified pins the backward-compatible default: +// with no key anywhere, the body is returned and no .sig is requested. +func TestGetOrVerify_DefaultUnverified(t *testing.T) { + t.Setenv(EnvManifestPublicKey, "") + t.Setenv(EnvRequireSignedManifest, "") + + var sigRequests int + srv := newBodyServer(t, []byte("hello"), &sigRequests) + + f := newFetcher(Config{Home: t.TempDir(), HTTPClient: srv.Client()}) + body, err := f.getOrVerify(context.Background(), srv.URL+"/x") + if err != nil { + t.Fatalf("getOrVerify: %v", err) + } + if string(body) != "hello" { + t.Fatalf("body = %q", body) + } + if sigRequests != 0 { + t.Fatalf("expected no signature fetch, got %d", sigRequests) + } +} + +// TestGetOrVerify_RequireWithoutKeyFails pins the opt-in strict mode. +func TestGetOrVerify_RequireWithoutKeyFails(t *testing.T) { + t.Setenv(EnvManifestPublicKey, "") + t.Setenv(EnvRequireSignedManifest, "") + + var sigRequests int + srv := newBodyServer(t, []byte("hello"), &sigRequests) + + f := newFetcher(Config{Home: t.TempDir(), HTTPClient: srv.Client(), RequireSignedManifest: true}) + if _, err := f.getOrVerify(context.Background(), srv.URL+"/x"); err == nil { + t.Fatalf("expected an error when a signature is required and no key resolves") + } else if !strings.Contains(err.Error(), EnvManifestPublicKey) { + t.Fatalf("error should name the environment override, got %v", err) + } +} + +// TestGetOrVerify_ResolvedKeyVerifies checks that a key supplied purely +// via the trust file is picked up and used to verify. +func TestGetOrVerify_ResolvedKeyVerifies(t *testing.T) { + t.Setenv(EnvManifestPublicKey, "") + t.Setenv(EnvRequireSignedManifest, "") + + pub, priv := newTestKey(t) + home := t.TempDir() + writeKeyFile(t, home, hex.EncodeToString(pub)) + + body := []byte("signed body") + sig := ed25519.Sign(priv, body) + + mux := http.NewServeMux() + mux.HandleFunc("/x", func(w http.ResponseWriter, _ *http.Request) { w.Write(body) }) + mux.HandleFunc("/x.sig", func(w http.ResponseWriter, _ *http.Request) { w.Write(sig) }) + mux.HandleFunc("/bad", func(w http.ResponseWriter, _ *http.Request) { w.Write([]byte("tampered")) }) + mux.HandleFunc("/bad.sig", func(w http.ResponseWriter, _ *http.Request) { w.Write(sig) }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + + f := newFetcher(Config{Home: home, HTTPClient: srv.Client()}) + got, err := f.getOrVerify(context.Background(), srv.URL+"/x") + if err != nil { + t.Fatalf("getOrVerify: %v", err) + } + if string(got) != string(body) { + t.Fatalf("body = %q", got) + } + if _, err := f.getOrVerify(context.Background(), srv.URL+"/bad"); err == nil { + t.Fatalf("expected verification failure on a mismatched body") + } +} + +func writeKeyFile(t *testing.T, home, contents string) { + t.Helper() + p := manifestPublicKeyPath(home) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(p), err) + } + if err := os.WriteFile(p, []byte(contents+"\n"), 0o600); err != nil { + t.Fatalf("write key file: %v", err) + } +} + +func newBodyServer(t *testing.T, body []byte, sigRequests *int) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, ".sig") { + *sigRequests++ + http.NotFound(w, r) + return + } + w.Write(body) + })) + t.Cleanup(srv.Close) + return srv +} diff --git a/zz_uninstall_traversal_test.go b/zz_uninstall_traversal_test.go new file mode 100644 index 0000000..d593731 --- /dev/null +++ b/zz_uninstall_traversal_test.go @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package skillinject_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/pilot-protocol/skillinject" +) + +// Uninstall walks plugin file names straight from the manifest. Both the +// install and the disable path must agree on which names are in scope: +// a name that resolves outside the plugin install dir is reported, not +// acted on. These tests pin that symmetry. + +// traversalManifest builds a manifest whose plugin declares one in-scope +// file plus one name that climbs out of the install directory. +func traversalManifest(r *fakeRepo) { + r.manifest = skillinject.Manifest{ + Version: 1, + Entrypoint: "pilotctl", + Tools: []skillinject.ManifestTool{{ + Name: "openclaw", + RootDir: "~/.openclaw", + SkillsDir: "~/.openclaw/skills", + HeartbeatPath: "~/.openclaw/workspace/AGENTS.md", + HeartbeatTemplate: "heartbeats/openclaw.md", + Plugin: &skillinject.ManifestPlugin{ + ID: "pilot", + InstallPath: "~/.openclaw/extensions/pilotprotocol", + Files: []skillinject.ManifestPluginFile{ + {Name: "index.mjs", Src: "plugin/index.mjs"}, + {Name: "../../../.ssh/authorized_keys", Src: "plugin/index.mjs"}, + }, + }, + }}, + } + r.files["skills/pilotctl/SKILL.md"] = []byte(testContent) + r.files["heartbeats/openclaw.md"] = []byte(testHeartbeat) + r.files["plugin/index.mjs"] = []byte("// pilot plugin\n") +} + +// TestUninstall_PluginFileEscapingInstallDirIsNotRemoved seeds a file +// outside the plugin install dir at the location an escaping manifest +// name resolves to, then runs Uninstall and asserts the file survives +// and the removal is reported as an error. +func TestUninstall_PluginFileEscapingInstallDirIsNotRemoved(t *testing.T) { + t.Parallel() + home := t.TempDir() + + outside := filepath.Join(home, ".ssh", "authorized_keys") + mustMkdirAll(t, filepath.Dir(outside)) + const sentinel = "ssh-ed25519 AAAA user@host\n" + if err := os.WriteFile(outside, []byte(sentinel), 0o600); err != nil { + t.Fatalf("seed outside file: %v", err) + } + + r := newFakeRepo(t) + traversalManifest(r) + + rep, err := skillinject.Uninstall(context.Background(), r.cfg(home)) + if err != nil { + t.Fatalf("Uninstall: %v", err) + } + + got, err := os.ReadFile(outside) + if err != nil { + t.Fatalf("file outside install dir was removed by Uninstall: %v", err) + } + if string(got) != sentinel { + t.Fatalf("file outside install dir was modified\nwant=%q\ngot =%q", sentinel, got) + } + + var sawError bool + for _, rm := range rep.Removals { + if rm.Kind == skillinject.KindPluginFile && rm.Action == skillinject.RemovalError { + sawError = true + } + } + if !sawError { + t.Fatalf("expected a RemovalError for the escaping plugin file, got %+v", rep.Removals) + } +} + +// TestUninstall_InScopePluginFileStillRemoved guards against the guard +// being too broad: a normal in-dir plugin file must still be deleted. +func TestUninstall_InScopePluginFileStillRemoved(t *testing.T) { + t.Parallel() + home := t.TempDir() + + r := newFakeRepo(t) + traversalManifest(r) + + installed := filepath.Join(home, ".openclaw", "extensions", "pilotprotocol", "index.mjs") + mustMkdirAll(t, filepath.Dir(installed)) + if err := os.WriteFile(installed, []byte("// pilot plugin\n"), 0o644); err != nil { + t.Fatalf("seed plugin file: %v", err) + } + + rep, err := skillinject.Uninstall(context.Background(), r.cfg(home)) + if err != nil { + t.Fatalf("Uninstall: %v", err) + } + if _, err := os.Stat(installed); !os.IsNotExist(err) { + t.Fatalf("in-scope plugin file was not removed: stat err=%v", err) + } + + var sawDeleted bool + for _, rm := range rep.Removals { + if rm.Path == installed && rm.Action == skillinject.RemovalDeleted { + sawDeleted = true + } + } + if !sawDeleted { + t.Fatalf("expected RemovalDeleted for %s, got %+v", installed, rep.Removals) + } +} + +// TestReconcile_PluginFileEscapingInstallDirIsNotWritten mirrors the +// disable-path test on the install side. +func TestReconcile_PluginFileEscapingInstallDirIsNotWritten(t *testing.T) { + t.Parallel() + home := t.TempDir() + + r := newFakeRepo(t) + traversalManifest(r) + + if _, err := skillinject.Tick(context.Background(), r.cfg(home)); err != nil { + t.Fatalf("Tick: %v", err) + } + + outside := filepath.Join(home, ".ssh", "authorized_keys") + if _, err := os.Stat(outside); !os.IsNotExist(err) { + t.Fatalf("install wrote outside the plugin install dir: stat err=%v", err) + } +}