From b87a0da3baef0cf80d173738ac8b23bb45184d25 Mon Sep 17 00:00:00 2001
From: Snider <snider@host.uk.com>
Date: Wed, 15 Apr 2026 23:07:20 +0100
Subject: [PATCH] Harden marketplace install inputs

---
 pkg/display/marketplace_test.go     |  4 ++-
 pkg/marketplace/marketplace.go      | 46 +++++++++++++++++++++++++++--
 pkg/marketplace/marketplace_test.go | 23 ++++++++++++++-
 3 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/pkg/display/marketplace_test.go b/pkg/display/marketplace_test.go
index 8ba6c39d..687ca677 100644
--- a/pkg/display/marketplace_test.go
+++ b/pkg/display/marketplace_test.go
@@ -124,7 +124,9 @@ func TestMarketplace_registerMarketplaceActions_Good(t *testing.T) {
 	installed, ok := installResult.Value.(map[string]any)
 	require.True(t, ok)
 	assert.Equal(t, installDir, installed["install_dir"])
-	assert.Equal(t, filepath.Join(installDir, "core-ui"), installed["target_dir"])
+	resolvedInstallDir, err := filepath.EvalSymlinks(installDir)
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Join(resolvedInstallDir, "core-ui"), installed["target_dir"])
 
 	contents, err := os.ReadFile(gitLog)
 	require.NoError(t, err)
diff --git a/pkg/marketplace/marketplace.go b/pkg/marketplace/marketplace.go
index 57145b45..1ce0559d 100644
--- a/pkg/marketplace/marketplace.go
+++ b/pkg/marketplace/marketplace.go
@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -130,7 +131,7 @@ func (i Installer) Install(ctx context.Context, manifest Manifest) (string, erro
 	if err := validateManifestName(manifest.Name); err != nil {
 		return "", err
 	}
-	if err := validateCloneArg("repository", manifest.Repository); err != nil {
+	if err := validateRepositorySource(manifest.Repository); err != nil {
 		return "", err
 	}
 	if err := validateCloneArgOptional("ref", manifest.Ref); err != nil {
@@ -139,16 +140,20 @@ func (i Installer) Install(ctx context.Context, manifest Manifest) (string, erro
 	if err := os.MkdirAll(i.InstallDir, 0o755); err != nil {
 		return "", err
 	}
-	targetDir := filepath.Join(i.InstallDir, safeName(manifest.Name))
 	rootAbs, err := filepath.Abs(i.InstallDir)
 	if err != nil {
 		return "", err
 	}
+	rootResolved, err := filepath.EvalSymlinks(rootAbs)
+	if err != nil {
+		return "", err
+	}
+	targetDir := filepath.Join(rootResolved, safeName(manifest.Name))
 	targetAbs, err := filepath.Abs(targetDir)
 	if err != nil {
 		return "", err
 	}
-	rel, err := filepath.Rel(rootAbs, targetAbs)
+	rel, err := filepath.Rel(rootResolved, targetAbs)
 	if err != nil {
 		return "", err
 	}
@@ -238,6 +243,41 @@ func validateCloneArgOptional(label, value string) error {
 	return validateCloneArg(label, trimmed)
 }
 
+func validateRepositorySource(value string) error {
+	trimmed := strings.TrimSpace(value)
+	if trimmed == "" {
+		return errors.New("repository is required")
+	}
+	if strings.ContainsAny(trimmed, "\x00\r\n") {
+		return errors.New("repository contains invalid control characters")
+	}
+	if strings.HasPrefix(strings.ToLower(trimmed), "ext::") {
+		return errors.New("repository must not use git remote helper protocols")
+	}
+	if strings.HasPrefix(trimmed, "-") {
+		return errors.New("repository must not begin with a dash")
+	}
+	if strings.Contains(trimmed, "://") {
+		parsed, err := url.Parse(trimmed)
+		if err != nil {
+			return err
+		}
+		switch strings.ToLower(parsed.Scheme) {
+		case "http", "https", "ssh", "git":
+		default:
+			return fmt.Errorf("repository scheme %q is not allowed", parsed.Scheme)
+		}
+		return nil
+	}
+	if strings.Contains(trimmed, string(filepath.Separator)) || filepath.IsAbs(trimmed) {
+		return errors.New("repository path clones are not allowed")
+	}
+	if !strings.Contains(trimmed, ":") {
+		return errors.New("repository must be a URL or scp-style remote")
+	}
+	return nil
+}
+
 func DigestManifest(manifest Manifest) string {
 	hash := sha256.Sum256([]byte(manifest.Name + ":" + manifest.Version + ":" + manifest.Repository + ":" + manifest.Ref))
 	return hex.EncodeToString(hash[:])
diff --git a/pkg/marketplace/marketplace_test.go b/pkg/marketplace/marketplace_test.go
index d0fb8358..9c93e621 100644
--- a/pkg/marketplace/marketplace_test.go
+++ b/pkg/marketplace/marketplace_test.go
@@ -185,7 +185,9 @@ func TestMarketplace_Install_Good(t *testing.T) {
 		Ref:        "main",
 	}))
 	require.NoError(t, err)
-	assert.Equal(t, filepath.Join(targetRoot, "core-ui"), targetDir)
+	resolvedRoot, err := filepath.EvalSymlinks(targetRoot)
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Join(resolvedRoot, "core-ui"), targetDir)
 	_, err = os.Stat(targetDir)
 	require.NoError(t, err)
 
@@ -366,6 +368,25 @@ func TestMarketplace_validateCloneArg_Ugly(t *testing.T) {
 	assert.Contains(t, err.Error(), "invalid control characters")
 }
 
+func TestMarketplace_validateRepositorySource_Good(t *testing.T) {
+	require.NoError(t, validateRepositorySource("https://example.com/core-ui.git"))
+	require.NoError(t, validateRepositorySource("git@example.com:core-ui.git"))
+}
+
+func TestMarketplace_validateRepositorySource_Bad(t *testing.T) {
+	err := validateRepositorySource("ext::sh -c id")
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "remote helper")
+}
+
+func TestMarketplace_validateRepositorySource_Ugly(t *testing.T) {
+	err := validateRepositorySource("file:///tmp/core-ui.git")
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "not allowed")
+}
+
 func TestMarketplace_decodeManifestList_Good(t *testing.T) {
 	manifests, err := decodeManifestList([]byte(`[{"name":"core-ui","version":"1.2.3"},{"name":"core-chat","version":"0.9.0"}]`))