From 1a7de0fe0b53508e483c7c7da379484684ff95bd Mon Sep 17 00:00:00 2001
From: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Date: Sat, 18 Jul 2026 16:37:54 -0500
Subject: [PATCH] fix: add HTTP timeout and reject unsafe item names in clone
 flow

Closes #25, closes #26
---
 internal/commands/clone.go      | 34 +++++++++++++++++++++++++--------
 internal/commands/clone_test.go | 25 ++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 8 deletions(-)
 create mode 100644 internal/commands/clone_test.go

diff --git a/internal/commands/clone.go b/internal/commands/clone.go
index d89bd89..0be2d57 100644
--- a/internal/commands/clone.go
+++ b/internal/commands/clone.go
@@ -12,6 +12,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/charmbracelet/bubbles/spinner"
 	tea "github.com/charmbracelet/bubbletea"
@@ -29,6 +30,20 @@ const (
 	apiBaseURL    = "https://api.github.com/repos/" + templatesRepo + "/contents"
 )
 
+// httpClient is used for all GitHub API/download requests so a stalled
+// response can't hang the command indefinitely.
+var httpClient = &http.Client{Timeout: 30 * time.Second}
+
+// isSafeItemName reports whether a GitHub content item's name is safe to
+// join into a local filesystem path. Defense-in-depth against a compromised
+// or unexpected API response containing a path-traversal name.
+func isSafeItemName(name string) bool {
+	if name == "" || name == "." || name == ".." {
+		return false
+	}
+	return !strings.ContainsAny(name, "/\\")
+}
+
 // GitHubContent represents a file/directory from GitHub API
 type GitHubContent struct {
 	Name        string `json:"name"`
@@ -149,7 +164,7 @@ func buildContentsAPIURL(path, branch string) string {
 // fetchGroupedTemplates fetches templates grouped by category (resources vs standalones)
 func fetchGroupedTemplates(branch string) (resources []templateDescriptor, standalones []templateDescriptor, err error) {
 	// Fetch root contents
-	resp, err := http.Get(buildContentsAPIURL("", branch))
+	resp, err := httpClient.Get(buildContentsAPIURL("", branch))
 	if err != nil {
 		return nil, nil, err
 	}
@@ -196,7 +211,7 @@ func fetchGroupedTemplates(branch string) (resources []templateDescriptor, stand
 // fetchFolderContents fetches the list of directories inside a folder
 func fetchFolderContents(folderPath string, category templateCategory, branch string) ([]templateDescriptor, error) {
 	requestURL := buildContentsAPIURL(folderPath, branch)
-	resp, err := http.Get(requestURL)
+	resp, err := httpClient.Get(requestURL)
 	if err != nil {
 		return nil, err
 	}
@@ -236,7 +251,7 @@ func fetchFolderContents(folderPath string, category templateCategory, branch st
 
 func fetchTemplateManifest(templatePath, branch string) (*templateManifest, error) {
 	requestURL := buildContentsAPIURL(templatePath, branch)
-	resp, err := http.Get(requestURL)
+	resp, err := httpClient.Get(requestURL)
 	if err != nil {
 		return nil, err
 	}
@@ -256,7 +271,7 @@ func fetchTemplateManifest(templatePath, branch string) (*templateManifest, erro
 			continue
 		}
 
-		manifestResp, err := http.Get(item.DownloadURL)
+		manifestResp, err := httpClient.Get(item.DownloadURL)
 		if err != nil {
 			return nil, err
 		}
@@ -308,7 +323,7 @@ func resolveTemplate(templateName, branch string) (templateDescriptor, error) {
 }
 
 func fetchTemplateList() ([]string, error) {
-	resp, err := http.Get(apiBaseURL)
+	resp, err := httpClient.Get(apiBaseURL)
 	if err != nil {
 		return nil, err
 	}
@@ -503,7 +518,7 @@ func cloneWithSparseCheckout(template, targetPath, branch string) error {
 func cloneWithGitHubAPI(template, targetPath, branch string) error {
 	// First verify template exists
 	apiURL := buildContentsAPIURL(template, branch)
-	resp, err := http.Get(apiURL)
+	resp, err := httpClient.Get(apiURL)
 	if err != nil {
 		return fmt.Errorf("failed to connect to GitHub: %w", err)
 	}
@@ -527,7 +542,7 @@ func cloneWithGitHubAPI(template, targetPath, branch string) error {
 
 func downloadDirectory(remotePath, localPath, branch string) error {
 	apiURL := buildContentsAPIURL(remotePath, branch)
-	resp, err := http.Get(apiURL)
+	resp, err := httpClient.Get(apiURL)
 	if err != nil {
 		return err
 	}
@@ -543,6 +558,9 @@ func downloadDirectory(remotePath, localPath, branch string) error {
 	}
 
 	for _, item := range contents {
+		if !isSafeItemName(item.Name) {
+			return fmt.Errorf("unsafe item name in template contents: %q", item.Name)
+		}
 		localItemPath := filepath.Join(localPath, item.Name)
 
 		if item.Type == "dir" {
@@ -563,7 +581,7 @@ func downloadDirectory(remotePath, localPath, branch string) error {
 }
 
 func downloadFile(url, localPath string) error {
-	resp, err := http.Get(url)
+	resp, err := httpClient.Get(url)
 	if err != nil {
 		return err
 	}
diff --git a/internal/commands/clone_test.go b/internal/commands/clone_test.go
new file mode 100644
index 0000000..a81e4a1
--- /dev/null
+++ b/internal/commands/clone_test.go
@@ -0,0 +1,25 @@
+package commands
+
+import "testing"
+
+func TestIsSafeItemName(t *testing.T) {
+	safe := []string{"chat", "my-template", "my_template", "file.ts", "README.md"}
+	for _, name := range safe {
+		if !isSafeItemName(name) {
+			t.Errorf("expected %q to be considered safe", name)
+		}
+	}
+
+	unsafe := []string{"", ".", "..", "../escape", "a/b", "a\\b", "/etc/passwd"}
+	for _, name := range unsafe {
+		if isSafeItemName(name) {
+			t.Errorf("expected %q to be rejected", name)
+		}
+	}
+}
+
+func TestHTTPClientHasTimeout(t *testing.T) {
+	if httpClient.Timeout <= 0 {
+		t.Fatal("expected httpClient to have a positive timeout configured")
+	}
+}