From 65bf4d41180157a7ac65a66f5da93c738ae12a2b Mon Sep 17 00:00:00 2001
From: matthew-pilot <matthew@vulturelabs.io>
Date: Fri, 29 May 2026 21:36:25 +0000
Subject: [PATCH] fix(skillinject): reject path-traversal in plugin file names
 (PILOT-250)

reconcilePluginFiles computes dst := filepath.Join(installDir, pf.Name)
where pf.Name comes from the remote manifest. A malicious name like
"../../.ssh/authorized_keys" would escape installDir and write outside
the plugin sandbox.

Add filepath.Clean + strings.HasPrefix guard that rejects traversal
sequences before fetching or writing.

Verified: build + vet + test all green. New test exercises both the
rejection and happy path for non-traversal names.

Closes PILOT-250
---
 skillinject.go            | 10 +++++++
 zz_extra_branches_test.go | 55 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/skillinject.go b/skillinject.go
index 5a8750d..45d8804 100644
--- a/skillinject.go
+++ b/skillinject.go
@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 )
@@ -262,6 +263,15 @@ 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) {
+			out = append(out, Outcome{
+				Tool: p.ID, Kind: KindPluginFile, Path: dst,
+				Action: ActionError,
+				Err:    fmt.Sprintf("path traversal: %q escapes install dir", pf.Name),
+			})
+			continue
+		}
 		body, err := f.fetchRepoFile(ctx, pf.Src)
 		if err != nil {
 			out = append(out, Outcome{
diff --git a/zz_extra_branches_test.go b/zz_extra_branches_test.go
index d99a380..8fcd677 100644
--- a/zz_extra_branches_test.go
+++ b/zz_extra_branches_test.go
@@ -458,3 +458,58 @@ func TestReconcilePluginAllowList_MergeErrorSurfaced(t *testing.T) {
 		t.Errorf("expected non-empty Err on merge failure: %+v", out)
 	}
 }
+
+// TestReconcilePluginFiles_PathTraversalRejected verifies that a
+// pf.Name containing "../" is rejected with an error Outcome instead
+// of writing outside the install directory.
+func TestReconcilePluginFiles_PathTraversalRejected(t *testing.T) {
+	t.Parallel()
+	home := t.TempDir()
+	installDir := filepath.Join(home, ".openclaw", "plugins", "pilot")
+
+	p := &ManifestPlugin{
+		ID:          "pilot",
+		InstallPath: "~/.openclaw/plugins/pilot",
+		Files: []ManifestPluginFile{
+			{Name: "../../.ssh/authorized_keys", Src: "plugin/authorized_keys"},
+			{Name: "index.mjs", Src: "plugin/index.mjs"},
+		},
+	}
+
+	// Must set up a real fetcher so the loop doesn't fail on the
+	// good file before we reach the path traversal guard.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/plugin/index.mjs" {
+			w.Write([]byte("export default {}\n"))
+			return
+		}
+		http.NotFound(w, r)
+	}))
+	t.Cleanup(srv.Close)
+
+	f := newFetcher(Config{RepoBaseURL: srv.URL + "/"})
+
+	out := reconcilePluginFiles(f, context.Background(), p, home)
+
+	// First entry (path traversal) should be an error.
+	if len(out) < 2 {
+		t.Fatalf("expected at least 2 outcomes, got %d: %+v", len(out), out)
+	}
+	first := out[0]
+	if first.Action != ActionError {
+		t.Errorf("first outcome action = %v, want ActionError", first.Action)
+	}
+	if !strings.Contains(first.Err, "path traversal") {
+		t.Errorf("first outcome error = %q, want 'path traversal'", first.Err)
+	}
+	// Second entry (clean filename) should succeed.
+	second := out[1]
+	if second.Action == ActionError {
+		t.Errorf("second outcome should not be error, got: %+v", second)
+	}
+	// Verify that the traversal file was NOT written.
+	traversalPath := filepath.Join(installDir, "../../.ssh/authorized_keys")
+	if _, err := os.Stat(traversalPath); err == nil {
+		t.Errorf("path traversal file was written at %s", traversalPath)
+	}
+}