feat: download-attachments for comments & documents, shared download helper (#148)

Credit to @brianevanmiller for this contribution.

feat: download-attachments for comments & documents, shared download helper

Author: brigleb

CHANGELOG.md

@@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Added
+- `comment download-attachments` command for downloading attachments from comments
+- `document download-attachments` command for downloading attachments from documents
+- `--include-comments` flag on card/todo/message/document download-attachments to also download comment attachments
+- Attachment display in `comment view` output
+
+### Changed
+- Refactored download logic into shared `internal/download` package, reducing code duplication
+
 ## [0.13.0] - 2026-01-19
 
 ### Added

README.md

@@ -357,6 +357,10 @@ bc4 document create "Spec Document" --content "# Overview\n\nThis is the spec...
 # Edit an existing document
 bc4 document edit 12345
 bc4 document edit 12345 --title "Updated Title"
+
+# Download attachments from a document
+bc4 document download-attachments 12345
+bc4 document download-attachments 12345 --include-comments
 ```
 
 ### Card Management
@@ -503,12 +507,16 @@ bc4 comment attach 12345 --attach ./log.txt
 
 # Append an attachment to a specific comment by ID
 bc4 comment attach 12345 --comment-id 67890 --attach ./screenshot.png
+
+# Download attachments from a comment
+bc4 comment download-attachments 67890
+bc4 comment download-attachments 67890 --output-dir ~/Downloads
 ```
 
 
 ### Downloading Attachments
 
-bc4 can download images and files attached to cards, todos, and messages using OAuth authentication:
+bc4 can download images and files attached to cards, todos, messages, documents, and comments using OAuth authentication:
 
 ```bash
 # Download all attachments from a card
@@ -520,6 +528,13 @@ bc4 todo download-attachments 789012 --output-dir ~/Downloads
 # Download from a message (only first attachment)
 bc4 message download-attachments 345678 --attachment 1
 
+# Download from a comment or document
+bc4 comment download-attachments 456789
+bc4 document download-attachments 567890
+
+# Include attachments from comments on a resource
+bc4 card download-attachments 123456 --include-comments
+
 # Overwrite existing files
 bc4 card download-attachments 123456 --overwrite
 
@@ -531,8 +546,9 @@ bc4 card download-attachments https://3.basecamp.com/123/buckets/456/card_tables
 - `--output-dir, -o` - Directory to save attachments (default: current directory)
 - `--attachment N` - Download only the Nth attachment (1-based index)
 - `--overwrite` - Replace existing files without prompting
+- `--include-comments` - Also download attachments from comments (available on card, todo, message, document)
 
-**Note:** Comment attachments use blob storage that requires browser authentication and cannot be downloaded via OAuth. To download comment attachments, access them through your web browser while logged into Basecamp.
+**Note:** Some Basecamp attachments use blob storage URLs that require browser session authentication and cannot be downloaded via the API. When encountered, bc4 will display the URL so you can open it in your browser while logged into Basecamp.
 
 ### Activity & Events
 
@@ -832,4 +848,4 @@ Thanks to everyone who has contributed to bc4!
 
 #### Known Limitations
 
-**Comment Attachments:** Comment attachments use blob storage URLs that require browser session cookies and do not support OAuth Bearer token authentication. The `download-attachments` command works for card body attachments, todo attachments, and message attachments, but not for attachments added in comments. To download comment attachments, use a web browser while authenticated to Basecamp.
+**Blob Storage URLs:** Some Basecamp attachments use blob storage URLs (containing `/blobs/` or `preview.3.basecamp.com`) that require browser session cookies and cannot be downloaded via OAuth. When bc4 encounters these URLs, it displays a helpful message with the URL so you can open it in your browser while logged into Basecamp.

cmd/card/download_attachments.go

@@ -2,14 +2,11 @@ package card
 
 import (
 	"fmt"
-	"os"
-	"path/filepath"
 	"strconv"
-	"strings"
 
 	"github.com/spf13/cobra"
 
-	"github.com/needmore/bc4/internal/attachments"
+	"github.com/needmore/bc4/internal/download"
 	"github.com/needmore/bc4/internal/factory"
 	"github.com/needmore/bc4/internal/parser"
 )
@@ -20,6 +17,7 @@ func newDownloadAttachmentsCmd(f *factory.Factory) *cobra.Command {
 	var outputDir string
 	var overwrite bool
 	var attachmentIndex int
+	var includeComments bool
 
 	cmd := &cobra.Command{
 		Use:   "download-attachments [card-id or URL]",
@@ -45,27 +43,25 @@ You can specify the card using either:
   # Overwrite existing files
   bc4 card download-attachments 123456 --overwrite
 
+  # Include attachments from comments
+  bc4 card download-attachments 123456 --include-comments
+
   # Using Basecamp URL
   bc4 card download-attachments https://3.basecamp.com/123/buckets/456/card_tables/cards/789`,
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// Apply account override if specified
 			if accountID != "" {
 				f = f.WithAccount(accountID)
 			}
-
-			// Apply project override if specified
 			if projectID != "" {
 				f = f.WithProject(projectID)
 			}
 
-			// Parse card ID (could be numeric ID or URL)
 			cardID, parsedURL, err := parser.ParseArgument(args[0])
 			if err != nil {
 				return fmt.Errorf("invalid card ID or URL: %s", args[0])
 			}
 
-			// If a URL was parsed, override account and project IDs if provided
 			var bucketID string
 			if parsedURL != nil {
 				if parsedURL.ResourceType != parser.ResourceTypeCard {
@@ -80,15 +76,13 @@ You can specify the card using either:
 				}
 			}
 
-			// Get API client from factory
 			client, err := f.ApiClient()
 			if err != nil {
 				return err
 			}
 			cardOps := client.Cards()
 			uploadOps := client.Uploads()
 
-			// Get resolved project ID (bucket ID)
 			resolvedProjectID, err := f.ProjectID()
 			if err != nil {
 				return err
@@ -97,165 +91,43 @@ You can specify the card using either:
 				bucketID = resolvedProjectID
 			}
 
-			// Fetch the card details
 			card, err := cardOps.GetCard(f.Context(), resolvedProjectID, cardID)
 			if err != nil {
 				return fmt.Errorf("failed to get card: %w", err)
 			}
 
-			// Parse attachments from card content
-			atts := attachments.ParseAttachments(card.Content)
-			if len(atts) == 0 {
-				fmt.Println("No attachments found in this card")
-				return nil
-			}
-
-			// Store original count before filtering
-			originalCount := len(atts)
-
-			// Filter to specific attachment if requested
-			if attachmentIndex > 0 {
-				if attachmentIndex > originalCount {
-					return fmt.Errorf("attachment index %d out of range (card has %d attachments)", attachmentIndex, originalCount)
-				}
-				atts = []attachments.Attachment{atts[attachmentIndex-1]}
-			}
-
-			// Use current directory if no output directory specified
-			if outputDir == "" {
-				outputDir = "."
+			sources := []download.AttachmentSource{
+				{Label: "card", Content: card.Content},
 			}
 
-			// Download each attachment
-			successful := 0
-			failed := 0
-			ctx := f.Context()
-
-			for i, att := range atts {
-				displayIndex := i + 1
-				if attachmentIndex > 0 {
-					displayIndex = attachmentIndex
-				}
-
-				// Show appropriate progress message based on whether filtering
-				if attachmentIndex > 0 {
-					fmt.Printf("Downloading attachment %d: %s\n", displayIndex, att.GetDisplayName())
-				} else {
-					fmt.Printf("Downloading attachment %d/%d: %s\n", displayIndex, originalCount, att.GetDisplayName())
-				}
-
-				// Try to extract upload ID from URL or Href
-				result, err := attachments.TryExtractUploadID(&att)
+			if includeComments {
+				comments, err := client.ListComments(f.Context(), resolvedProjectID, card.ID)
 				if err != nil {
-					if result != nil && result.IsBlobURL {
-						// This is a blob URL - provide helpful guidance
-						fmt.Println("  ✗ Cannot download via API: This attachment uses a browser-only URL")
-						fmt.Printf("    URL: %s\n", result.BlobURL)
-						fmt.Println("    Tip: Open this URL in your browser while logged into Basecamp to download")
-					} else {
-						fmt.Printf("  ✗ Failed: %v\n", err)
-					}
-					failed++
-					continue
+					return fmt.Errorf("failed to fetch comments: %w", err)
 				}
-
-				// Get full upload details including download URL
-				upload, err := uploadOps.GetUpload(ctx, bucketID, result.UploadID)
-				if err != nil {
-					fmt.Printf("  ✗ Failed to get upload details: %v\n", err)
-					failed++
-					continue
+				for _, c := range comments {
+					sources = append(sources, download.AttachmentSource{
+						Label:   fmt.Sprintf("comment #%d by %s", c.ID, c.Creator.Name),
+						Content: c.Content,
+					})
 				}
-
-				// Sanitize filename for filesystem safety
-				filename := sanitizeFilename(upload.Filename)
-				destPath := filepath.Join(outputDir, filename)
-
-				// Check if file exists
-				if !overwrite {
-					if _, err := os.Stat(destPath); err == nil {
-						fmt.Printf("  ⚠ File already exists: %s (use --overwrite to replace)\n", destPath)
-						fmt.Println("  Skipping...")
-						continue
-					}
-				}
-
-				// Download the attachment
-				err = uploadOps.DownloadAttachment(ctx, upload.DownloadURL, destPath)
-				if err != nil {
-					fmt.Printf("  ✗ Failed to download: %v\n", err)
-					failed++
-					continue
-				}
-
-				// Format file size
-				sizeStr := formatByteSize(upload.ByteSize)
-				fmt.Printf("  ✓ Downloaded: %s (%s)\n", destPath, sizeStr)
-				successful++
 			}
 
-			// Print summary
-			fmt.Println()
-			if successful > 0 {
-				fmt.Printf("Successfully downloaded: %d/%d attachments\n", successful, len(atts))
-			}
-			if failed > 0 {
-				fmt.Printf("Failed: %d attachments\n", failed)
-				return fmt.Errorf("some attachments failed to download")
-			}
-
-			return nil
+			_, err = download.DownloadFromSources(f.Context(), uploadOps, bucketID, sources, download.Options{
+				OutputDir:       outputDir,
+				Overwrite:       overwrite,
+				AttachmentIndex: attachmentIndex,
+			})
+			return err
 		},
 	}
 
-	// Add flags
 	cmd.Flags().StringVarP(&accountID, "account", "a", "", "Specify account ID")
 	cmd.Flags().StringVarP(&projectID, "project", "p", "", "Specify project ID")
 	cmd.Flags().StringVarP(&outputDir, "output-dir", "o", "", "Directory to save attachments (default: current directory)")
 	cmd.Flags().BoolVar(&overwrite, "overwrite", false, "Overwrite existing files without prompting")
 	cmd.Flags().IntVar(&attachmentIndex, "attachment", 0, "Download only specified attachment (1-based index)")
+	cmd.Flags().BoolVar(&includeComments, "include-comments", false, "Also download attachments from comments on this card")
 
 	return cmd
 }
-
-// sanitizeFilename removes or replaces characters that are unsafe for filenames
-// to prevent path traversal attacks and filesystem errors
-func sanitizeFilename(filename string) string {
-	// Remove path separators to prevent directory traversal
-	cleaned := filepath.Base(filename)
-
-	// Remove null bytes and other control characters
-	cleaned = strings.Map(func(r rune) rune {
-		if r < 32 || r == 127 {
-			return -1
-		}
-		return r
-	}, cleaned)
-
-	// Replace filesystem-unsafe characters with underscores
-	unsafe := []string{"<", ">", ":", "\"", "|", "?", "*"}
-	for _, char := range unsafe {
-		cleaned = strings.ReplaceAll(cleaned, char, "_")
-	}
-
-	// Prevent empty filenames
-	if cleaned == "" || cleaned == "." || cleaned == ".." {
-		cleaned = "attachment"
-	}
-
-	return cleaned
-}
-
-// formatByteSize formats a byte size in a human-readable format
-func formatByteSize(bytes int64) string {
-	const unit = 1024
-	if bytes < unit {
-		return fmt.Sprintf("%d B", bytes)
-	}
-	div, exp := int64(unit), 0
-	for n := bytes / unit; n >= unit; n /= unit {
-		div *= unit
-		exp++
-	}
-	return fmt.Sprintf("%.1f %cB", float64(bytes)/float64(div), "KMGTPE"[exp])
-}

cmd/comment/comment.go

@@ -33,6 +33,7 @@ on your team's work.`,
 	cmd.AddCommand(newEditCmd(f))
 	cmd.AddCommand(newAttachCmd(f))
 	cmd.AddCommand(newDeleteCmd(f))
+	cmd.AddCommand(newDownloadAttachmentsCmd(f))
 
 	return cmd
 }