From 53c091bafa695ef6e002041b2e612ea2b955a2b8 Mon Sep 17 00:00:00 2001
From: Rob Zolkos <rob@zolkos.com>
Date: Sat, 24 Jan 2026 15:32:50 -0500
Subject: [PATCH] fix: prevent path traversal in attachment downloads

Sanitize filename using filepath.Base() to prevent malicious server
responses from writing files to arbitrary locations on the filesystem.

While Fizzy's API is trusted, this follows defense-in-depth principles
by ensuring the CLI never writes outside the current directory regardless
of what filename the server returns.

Adds comprehensive unit tests covering various path traversal attempts.
---
 internal/commands/attachment.go      |  4 +-
 internal/commands/attachment_test.go | 81 ++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/internal/commands/attachment.go b/internal/commands/attachment.go
index 6aa6430..50e3907 100644
--- a/internal/commands/attachment.go
+++ b/internal/commands/attachment.go
@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"path/filepath"
 	"regexp"
 	"strconv"
 
@@ -112,7 +113,8 @@ Use 'fizzy card attachments show CARD_NUMBER' to see available attachments and t
 		// Download the files
 		var results []map[string]interface{}
 		for _, attachment := range toDownload {
-			outputPath := attachment.Filename
+			// Sanitize filename to prevent path traversal attacks
+			outputPath := filepath.Base(attachment.Filename)
 			// If downloading single file with custom output name
 			if len(toDownload) == 1 && attachmentDownloadOutput != "" {
 				outputPath = attachmentDownloadOutput
diff --git a/internal/commands/attachment_test.go b/internal/commands/attachment_test.go
index 1bb6cd9..d2023fa 100644
--- a/internal/commands/attachment_test.go
+++ b/internal/commands/attachment_test.go
@@ -415,3 +415,84 @@ func stringContains(s, substr string) bool {
 	}
 	return false
 }
+
+func TestAttachmentDownloadSanitizesFilename(t *testing.T) {
+	tests := []struct {
+		name               string
+		maliciousFilename  string
+		expectedOutputPath string
+	}{
+		{
+			name:               "path traversal with ../",
+			maliciousFilename:  "../../../etc/passwd",
+			expectedOutputPath: "passwd",
+		},
+		{
+			name:               "path traversal with subdirectory",
+			maliciousFilename:  "../../.bashrc",
+			expectedOutputPath: ".bashrc",
+		},
+		{
+			name:               "absolute path attempt",
+			maliciousFilename:  "/etc/shadow",
+			expectedOutputPath: "shadow",
+		},
+		{
+			name:               "normal filename unchanged",
+			maliciousFilename:  "safe-file.png",
+			expectedOutputPath: "safe-file.png",
+		},
+		{
+			name:               "filename with spaces",
+			maliciousFilename:  "my document.pdf",
+			expectedOutputPath: "my document.pdf",
+		},
+		{
+			name:               "deeply nested traversal",
+			maliciousFilename:  "../../../../../../../../tmp/malware.sh",
+			expectedOutputPath: "malware.sh",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cardData := map[string]interface{}{
+				"id":     "card-id",
+				"number": 241,
+				"description_html": `<action-text-attachment sgid="test-sgid" content-type="image/png" filename="` + tt.maliciousFilename + `" filesize="1000">
+					<a href="/blobs/blob/file?disposition=attachment">Download</a>
+				</action-text-attachment>`,
+			}
+
+			mock := NewMockClient().WithGetData(cardData)
+			result := SetTestMode(mock)
+			SetTestConfig("test-token", "test-account", "https://api.test.com")
+			defer ResetTestMode()
+
+			rootCmd.SetArgs([]string{"card", "attachments", "download", "241", "1"})
+
+			RunTestCommand(func() {
+				_ = rootCmd.Execute()
+			})
+
+			if result.Response == nil {
+				t.Fatal("expected response, got nil")
+			}
+
+			if !result.Response.Success {
+				t.Errorf("expected success, got error: %v", result.Response)
+				return
+			}
+
+			// Verify the output path was sanitized
+			if len(mock.DownloadFileCalls) != 1 {
+				t.Fatalf("expected 1 download call, got %d", len(mock.DownloadFileCalls))
+			}
+
+			actualOutputPath := mock.DownloadFileCalls[0].DestPath
+			if actualOutputPath != tt.expectedOutputPath {
+				t.Errorf("expected sanitized output path %q, got %q", tt.expectedOutputPath, actualOutputPath)
+			}
+		})
+	}
+}