Open only viewable attachments, and mark every download

docs/tui.md

@@ -202,7 +202,7 @@ the iOS and Android apps each keep their own local choice too, and this is the s
 
 ## Attachments
 
-Thread attachments always appear with their filename, media type, and size. Use `[` and `]` to select an attachment, `s` to save it without replacing an existing file, and Enter to download and open it in an external application — once Tab has selected a link, Enter opens the link instead, and Escape clears it. Attachments never open automatically. Kitty and Ghostty can show inline images. In the message itself, a file is marked by its kind: 📷 an image, 🎬 a video, 🎵 audio, 📄 a PDF or document, 📎 anything else. An image shows by name rather than by its URL, unless its name looks like a URL, when the real destination is shown beside it; an image on the web is a link you can select with Tab, and the row above the shortcut bar shows its whole destination.
+Thread attachments always appear with their filename, media type, and size. Use `[` and `]` to select an attachment, `s` to save it without replacing an existing file, and Enter to download and open it in an external application — once Tab has selected a link, Enter opens the link instead, and Escape clears it. Enter only opens documents, images, audio and video (on Linux, only when the file's contents are one too, since the system picks the application from them); anything that could run a program, such as a script, a shortcut or a Terminal profile, has to be saved with `s` and opened by hand. Attachments never open automatically, and on macOS and Windows every saved attachment is marked as downloaded from the internet, so the system warns before running one. Kitty and Ghostty can show inline images. In the message itself, a file is marked by its kind: 📷 an image, 🎬 a video, 🎵 audio, 📄 a PDF or document, 📎 anything else. An image shows by name rather than by its URL, unless its name looks like a URL, when the real destination is shown beside it; an image on the web is a link you can select with Tab, and the row above the shortcut bar shows its whole destination.
 
 ## Contacts
 

internal/attachments/mark_downloaded_darwin.go

@@ -0,0 +1,17 @@
+//go:build darwin
+
+package attachments
+
+import (
+	"fmt"
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+// markDownloaded sets the quarantine flag a browser sets on a download, so
+// macOS asks before opening a file that can run a program.
+func markDownloaded(path string) error {
+	value := fmt.Sprintf("0081;%08x;HEY CLI;", time.Now().Unix())
+	return unix.Setxattr(path, "com.apple.quarantine", []byte(value), 0)
+}

internal/attachments/mark_downloaded_other.go

@@ -0,0 +1,9 @@
+//go:build !darwin && !windows
+
+package attachments
+
+// markDownloaded does nothing where the system keeps no record of where a
+// file came from. Saved attachments are still never executable.
+func markDownloaded(string) error {
+	return nil
+}

internal/attachments/mark_downloaded_windows.go

@@ -0,0 +1,14 @@
+//go:build windows
+
+package attachments
+
+import "os"
+
+// markDownloaded records that the file came from the internet, as a browser
+// does, so Windows warns before running it. Volumes without alternate data
+// streams, such as FAT drives, can't carry the mark; the file is saved anyway,
+// as a browser would save it.
+func markDownloaded(path string) error {
+	_ = os.WriteFile(path+":Zone.Identifier", []byte("[ZoneTransfer]\r\nZoneId=3\r\n"), 0o600)
+	return nil
+}

internal/attachments/save.go

@@ -81,7 +81,7 @@ func PortableFilename(filename string) (string, error) {
 
 // SaveBytes safely writes data to destination. Existing paths are preserved unless force is set.
 func SaveBytes(destination string, data []byte, force bool) (int64, error) {
-	return save(context.Background(), byteDownloader(data), destination, "", force)
+	return save(context.Background(), byteDownloader(data), destination, "", force, nil)
 }
 
 type byteDownloader []byte
@@ -95,10 +95,12 @@ func Save(ctx context.Context, downloader Downloader, destination, sourceURL str
 	if !IsHEYBlobURL(sourceURL) {
 		return 0, apierr.ErrAPI(0, "attachment URL does not identify a HEY blob")
 	}
-	return save(ctx, downloader, destination, sourceURL, force)
+	return save(ctx, downloader, destination, sourceURL, force, markDownloaded)
 }
 
-func save(ctx context.Context, downloader Downloader, destination, sourceURL string, force bool) (int64, error) {
+// save writes the download beside destination and moves it into place. mark,
+// when given, runs on the finished file before anything can see it there.
+func save(ctx context.Context, downloader Downloader, destination, sourceURL string, force bool, mark func(string) error) (int64, error) {
 	if !force {
 		if _, err := os.Lstat(destination); err == nil {
 			return 0, apierr.ErrUsage(fmt.Sprintf("destination already exists: %s (use --force to replace it)", destination))
@@ -123,6 +125,11 @@ func save(ctx context.Context, downloader Downloader, destination, sourceURL str
 	if err := temporary.Close(); err != nil {
 		return written, apierr.ErrAPI(0, fmt.Sprintf("could not close temporary file: %v", err))
 	}
+	if mark != nil {
+		if err := mark(temporaryPath); err != nil {
+			return written, apierr.ErrAPI(0, fmt.Sprintf("could not mark file as downloaded: %v", err))
+		}
+	}
 
 	if force {
 		if err := replaceFile(temporaryPath, destination); err != nil {

internal/attachments/save_darwin_test.go

@@ -0,0 +1,28 @@
+//go:build darwin
+
+package attachments
+
+import (
+	"context"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"golang.org/x/sys/unix"
+)
+
+func TestSaveSetsQuarantineOnDownloadedAttachment(t *testing.T) {
+	destination := filepath.Join(t.TempDir(), "Invoice.terminal")
+	if _, err := Save(context.Background(), writeDownload("<plist/>"), destination, "/rails/active_storage/blobs/invoice", false); err != nil {
+		t.Fatal(err)
+	}
+	value := make([]byte, 256)
+	size, err := unix.Getxattr(destination, "com.apple.quarantine", value)
+	if err != nil {
+		t.Fatalf("saved attachment has no quarantine flag: %v", err)
+	}
+	flag := string(value[:size])
+	if !strings.HasPrefix(flag, "0081;") || !strings.HasSuffix(flag, ";HEY CLI;") {
+		t.Errorf("quarantine flag = %q", value[:size])
+	}
+}

internal/attachments/save_test.go

@@ -210,3 +210,35 @@ func assertNoTemporaryFiles(t *testing.T, directory string) {
 		}
 	}
 }
+
+func TestSaveMarksDownloadBeforeItReachesDestination(t *testing.T) {
+	destination := filepath.Join(t.TempDir(), "agenda.pdf")
+	var marked string
+	mark := func(path string) error {
+		if _, err := os.Lstat(destination); !os.IsNotExist(err) {
+			t.Errorf("destination existed before the download was marked: %v", err)
+		}
+		marked = path
+		return nil
+	}
+	if _, err := save(context.Background(), writeDownload("agenda"), destination, "/rails/active_storage/blobs/agenda", false, mark); err != nil {
+		t.Fatal(err)
+	}
+	if marked == "" || filepath.Dir(marked) != filepath.Dir(destination) {
+		t.Errorf("marked path = %q", marked)
+	}
+}
+
+func TestSaveKeepsNothingWhenDownloadCannotBeMarked(t *testing.T) {
+	directory := t.TempDir()
+	destination := filepath.Join(directory, "agenda.pdf")
+	mark := func(string) error { return errors.New("no extended attributes") }
+	_, err := save(context.Background(), writeDownload("agenda"), destination, "/rails/active_storage/blobs/agenda", false, mark)
+	if err == nil || !strings.Contains(err.Error(), "could not mark file as downloaded") {
+		t.Fatalf("save error = %v", err)
+	}
+	entries, readErr := os.ReadDir(directory)
+	if readErr != nil || len(entries) != 0 {
+		t.Errorf("directory after failed mark = %v, %v", entries, readErr)
+	}
+}

internal/attachments/save_unix_test.go

@@ -0,0 +1,24 @@
+//go:build unix
+
+package attachments
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestSavedAttachmentIsNeverExecutable(t *testing.T) {
+	destination := filepath.Join(t.TempDir(), "install.sh")
+	if _, err := Save(context.Background(), writeDownload("#!/bin/sh\n"), destination, "/rails/active_storage/blobs/install", false); err != nil {
+		t.Fatal(err)
+	}
+	info, err := os.Stat(destination)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if info.Mode().Perm()&0o111 != 0 {
+		t.Errorf("saved attachment mode = %v, want no execute bits", info.Mode().Perm())
+	}
+}

internal/attachments/viewable.go

@@ -0,0 +1,94 @@
+package attachments
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+// viewableExtensions are the kinds of file the system hands to a document,
+// image or media viewer. Anything else might be run rather than shown: a
+// Terminal profile, a script, a shortcut, an installer. The list is closed on
+// purpose, because the kinds of file an operating system will run keep growing.
+var viewableExtensions = map[string]bool{
+	// Documents and text
+	".pdf": true, ".txt": true, ".md": true, ".csv": true, ".rtf": true,
+	".docx": true, ".xlsx": true, ".pptx": true,
+	".odt": true, ".ods": true, ".odp": true,
+	".pages": true, ".numbers": true, ".key": true,
+	".ics": true,
+
+	// Images
+	".png": true, ".jpg": true, ".jpeg": true, ".gif": true, ".webp": true,
+	".heic": true, ".heif": true, ".bmp": true, ".tif": true, ".tiff": true,
+
+	// Audio and video
+	".mp3": true, ".m4a": true, ".wav": true, ".aac": true, ".flac": true, ".ogg": true,
+	".mp4": true, ".m4v": true, ".mov": true, ".webm": true,
+}
+
+// viewableContentTypes are the media types, as a content sniffer names them,
+// of the same documents, images and media. Some systems pick the application
+// for a file from what is in it rather than from its name, so a Windows
+// program called report.pdf reaches whatever runs Windows programs. Both
+// xdg-mime backends' spellings are here: file(1)'s and shared-mime-info's.
+var viewableContentTypes = map[string]bool{
+	// Documents and text
+	"application/pdf": true, "text/plain": true, "text/markdown": true, "text/x-markdown": true,
+	"text/csv": true, "application/rtf": true, "text/rtf": true, "text/calendar": true,
+	"application/vnd.openxmlformats-officedocument.wordprocessingml.document":   true,
+	"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet":         true,
+	"application/vnd.openxmlformats-officedocument.presentationml.presentation": true,
+	"application/vnd.oasis.opendocument.text":                                   true,
+	"application/vnd.oasis.opendocument.spreadsheet":                            true,
+	"application/vnd.oasis.opendocument.presentation":                           true,
+	"application/vnd.apple.pages":                                               true, "application/vnd.apple.numbers": true, "application/vnd.apple.keynote": true,
+
+	// Images
+	"image/png": true, "image/jpeg": true, "image/gif": true, "image/webp": true,
+	"image/heic": true, "image/heif": true, "image/bmp": true, "image/x-ms-bmp": true, "image/tiff": true,
+
+	// Audio and video
+	"audio/mpeg": true, "audio/mp4": true, "audio/x-m4a": true, "audio/wav": true, "audio/x-wav": true,
+	"audio/vnd.wave": true, "audio/aac": true, "audio/x-aac": true, "audio/x-hx-aac-adts": true,
+	"audio/flac": true, "audio/x-flac": true, "audio/ogg": true, "audio/x-vorbis+ogg": true,
+	"audio/x-opus+ogg": true, "application/ogg": true,
+	"video/mp4": true, "video/x-m4v": true, "video/quicktime": true, "video/webm": true,
+}
+
+// zipDocumentExtensions are documents stored as zip archives. A sniffer that
+// cannot tell one from a plain archive calls it application/zip, which then
+// opens in an archive manager rather than anything that runs it.
+var zipDocumentExtensions = map[string]bool{
+	".docx": true, ".xlsx": true, ".pptx": true,
+	".odt": true, ".ods": true, ".odp": true,
+	".pages": true, ".numbers": true, ".key": true,
+}
+
+// OpensInViewer reports whether an attachment called filename is a kind of
+// file the system shows in a viewer, so it is safe to open directly. Other
+// attachments can still be saved and opened deliberately.
+func OpensInViewer(filename string) bool {
+	return viewableExtensions[portableExtension(filename)]
+}
+
+// ContentOpensInViewer reports whether a file called filename whose contents
+// a sniffer named contentType is still a document, image or media file, so an
+// opener that chooses by contents shows it rather than runs it.
+func ContentOpensInViewer(filename, contentType string) bool {
+	if !OpensInViewer(filename) {
+		return false
+	}
+	contentType = strings.ToLower(strings.TrimSpace(contentType))
+	if contentType == "application/zip" {
+		return zipDocumentExtensions[portableExtension(filename)]
+	}
+	return viewableContentTypes[contentType]
+}
+
+func portableExtension(filename string) string {
+	portable, err := PortableFilename(filename)
+	if err != nil {
+		return ""
+	}
+	return strings.ToLower(filepath.Ext(portable))
+}

internal/attachments/viewable_test.go

@@ -0,0 +1,78 @@
+package attachments
+
+import "testing"
+
+func TestOpensInViewerOnlyForDocumentsImagesAndMedia(t *testing.T) {
+	for filename, want := range map[string]bool{
+		"agenda.pdf":          true,
+		"Chart.PNG":           true,
+		"minutes.docx":        true,
+		"invite.ics":          true,
+		"recording.m4a":       true,
+		"../../notes.txt":     true,
+		"Invoice.terminal":    false,
+		"setup.command":       false,
+		"report.pdf.command":  false,
+		"report.pdf.terminal": false,
+		"Invoice.terminal. ":  false,
+		"Shortcut.webloc":     false,
+		"Server.inetloc":      false,
+		"Folder.fileloc":      false,
+		"Script.scpt":         false,
+		"Installer.pkg":       false,
+		"Disk.dmg":            false,
+		"launcher.desktop":    false,
+		"install.sh":          false,
+		"tool.AppImage":       false,
+		"invoice.exe":         false,
+		"invoice.bat":         false,
+		"invoice.cmd":         false,
+		"invoice.lnk":         false,
+		"invoice.url":         false,
+		"invoice.hta":         false,
+		"invoice.js":          false,
+		"invoice.vbs":         false,
+		"invoice.ps1":         false,
+		"invoice.msi":         false,
+		"macro.docm":          false,
+		"page.html":           false,
+		"drawing.svg":         false,
+		"archive.zip":         false,
+		"README":              false,
+		"":                    false,
+		"..":                  false,
+	} {
+		if got := OpensInViewer(filename); got != want {
+			t.Errorf("OpensInViewer(%q) = %v, want %v", filename, got, want)
+		}
+	}
+}
+
+func TestContentOpensInViewerOnlyWhenTheContentsAgree(t *testing.T) {
+	for _, tc := range []struct {
+		filename, contentType string
+		want                  bool
+	}{
+		{"agenda.pdf", "application/pdf", true},
+		{"notes.md", "text/plain", true},
+		{"Chart.PNG", " Image/PNG ", true},
+		{"photo.jpg", "image/png", true},
+		{"minutes.docx", "application/zip", true},
+		{"budget.ods", "application/zip", true},
+		{"agenda.pdf", "application/vnd.microsoft.portable-executable", false},
+		{"agenda.md", "application/x-ms-dos-executable", false},
+		{"notes.txt", "application/x-shellscript", false},
+		{"notes.txt", "text/x-script.python", false},
+		{"photo.jpg", "application/zip", false},
+		{"photo.png", "image/svg+xml", false},
+		{"photo.png", "application/x-desktop", false},
+		{"photo.png", "application/octet-stream", false},
+		{"photo.png", "", false},
+		{"Invoice.terminal", "text/plain", false},
+		{"report.pdf.command", "application/pdf", false},
+	} {
+		if got := ContentOpensInViewer(tc.filename, tc.contentType); got != tc.want {
+			t.Errorf("ContentOpensInViewer(%q, %q) = %v, want %v", tc.filename, tc.contentType, got, tc.want)
+		}
+	}
+}

internal/tui/mail.go

@@ -2252,6 +2252,12 @@ func (v *mailView) openSelectedAttachment() tea.Cmd {
 		return nil
 	}
 	topicID := v.topicID
+	if !attachmentfiles.OpensInViewer(attachment.Filename) {
+		err := apierr.ErrUsageHint("it could run a program", "press s to save it")
+		return func() tea.Msg {
+			return attachmentOpenedMsg{topicID: topicID, attachmentID: attachment.ID, filename: attachment.Filename, err: err}
+		}
+	}
 	return func() tea.Msg {
 		directory, err := v.vc.newAttachmentTempDir()
 		if err != nil {