fix(BUG-3185): an item export's file name whose stem is a Windows device gets a "_" prefix, on the server and in the CLI (#1495)

`pad item export` saves to "<slug>.pad.md", so an item slugged nul, con,
aux, prn, com1-9 or lpt1-9 named a Windows device, and the CLI on Windows
wrote the artifact nowhere.

- attachments.WindowsDeviceName is exported (a rename, no behaviour
  change): the one device rule from BUG-2822 (#1493), where the stem ends
  at the first "." or ":". Its doc says not to write a second one.
- artifactExportFilename prefixes "_" to a device name, so the
  Content-Disposition both clients save under is safe.
- The CLI applies the same rule to the path it will write
  (defaultExportPath): it is the process writing the file, an older server
  still suggests the device name, and the "<ref>.pad.md" fallback never
  passed through the server.

Population (every Content-Disposition and client default name): the
bundle export ("<slug>-export.tar.gz", whose stem is never bare), the account
export (a fixed name) and the edit temp/recovery files (the "pad-edit-"
prefix) are safe; attachments are #1493. Import has no mirror: nothing it
creates is named from content.

Claude-Session: https://claude.ai/code/session_01Vf8uZmYdsrQqJtbAmjUFSX

cmd/pad/cmd_item.go

@@ -17,6 +17,7 @@ import (
 	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 
+	"github.com/PerpetualSoftware/pad/internal/attachments"
 	"github.com/PerpetualSoftware/pad/internal/cli"
 	"github.com/PerpetualSoftware/pad/internal/items"
 
@@ -2845,13 +2846,9 @@ Examples:
 
 			// Default filename: the server-suggested Content-Disposition
 			// filename (<slug>.pad.md), falling back to the ref when the
-			// header is missing.
+			// header is missing. See defaultExportPath for the device rule.
 			if outPath == "" {
-				if res.Filename != "" {
-					outPath = res.Filename
-				} else {
-					outPath = args[0] + ".pad.md"
-				}
+				outPath = defaultExportPath(res.Filename, args[0])
 			}
 
 			if err := writeFileAtomic(outPath, res.Body, 0o644); err != nil {
@@ -2865,6 +2862,25 @@ Examples:
 	return cmd
 }
 
+// defaultExportPath is the file `pad item export` writes when no -o is given:
+// the server-suggested name, else "<ref>.pad.md".
+//
+// BUG-3185: when that name's stem is a Windows device ("nul.pad.md",
+// "con.pad.md"), it is prefixed "_" here as well as on the server. The CLI is
+// the process that writes the file, and a server older than the fix still
+// suggests the device name; the ref fallback never passed through the server
+// at all.
+func defaultExportPath(serverName, ref string) string {
+	name := serverName
+	if name == "" {
+		name = ref + ".pad.md"
+	}
+	if attachments.WindowsDeviceName(name) {
+		return "_" + name
+	}
+	return name
+}
+
 // writeFileAtomic writes data to a sibling temp file, fsyncs it, then
 // atomically renames it onto path. Unlike os.WriteFile it never
 // truncates an existing destination before the bytes are known-good, so

cmd/pad/item_export_device_test.go

@@ -0,0 +1,26 @@
+package main
+
+import "testing"
+
+// BUG-3185: the CLI writes the export file itself, so it applies the device
+// rule to the name it will write, whether that name came from the server
+// (an older server still suggests "nul.pad.md") or from the ref fallback.
+func TestDefaultExportPath_BUG3185(t *testing.T) {
+	cases := []struct{ server, ref, want string }{
+		// Server-suggested names.
+		{"nul.pad.md", "PLAYB-1", "_nul.pad.md"},
+		{"CON.pad.md", "PLAYB-1", "_CON.pad.md"},
+		{"_nul.pad.md", "PLAYB-1", "_nul.pad.md"}, // a fixed server's name is not prefixed twice
+		{"ship.pad.md", "PLAYB-1", "ship.pad.md"},
+		// The ref fallback, when the header is absent.
+		{"", "nul", "_nul.pad.md"},
+		{"", "com1", "_com1.pad.md"},
+		{"", "PLAYB-1", "PLAYB-1.pad.md"},
+		{"", "nulls", "nulls.pad.md"},
+	}
+	for _, c := range cases {
+		if got := defaultExportPath(c.server, c.ref); got != c.want {
+			t.Errorf("defaultExportPath(%q, %q) = %q, want %q", c.server, c.ref, got, c.want)
+		}
+	}
+}

internal/attachments/filename.go

@@ -72,7 +72,7 @@ const fallbackFilename = "upload.bin"
 //     becomes "upload.bin".
 //  6. A Windows reserved device name gets a "_" prefix (BUG-2822): Windows
 //     opens "nul.txt" as the null device, so a download saved under the stored
-//     name is lost or refused. See windowsDeviceName for the match.
+//     name is lost or refused. See WindowsDeviceName for the match.
 func NormalizeFilename(raw string) string {
 	name := filepath.Base(raw)
 	if i := strings.LastIndexByte(name, '\\'); i >= 0 {
@@ -99,7 +99,7 @@ func NormalizeFilename(raw string) string {
 	if name == "" || name == "." || name == ".." || name == "/" {
 		return fallbackFilename
 	}
-	if windowsDeviceName(name) {
+	if WindowsDeviceName(name) {
 		return "_" + name
 	}
 	return name
@@ -118,12 +118,16 @@ var windowsReservedStems = func() map[string]bool {
 	return m
 }()
 
-// windowsDeviceName reports whether Windows would open name as a device. It
+// WindowsDeviceName reports whether Windows would open name as a device. It
 // reads the name as Windows does: the stem is everything before the FIRST dot
 // or colon, so "con.tar.gz" and "nul:x.txt" are devices (a colon opens the
 // stream syntax, codex round 1), and trailing spaces in the stem are ignored,
 // so "con .txt" is one too. The match folds case.
-func windowsDeviceName(name string) bool {
+//
+// Exported for BUG-3185: every name offered for download or written by the
+// CLI that is derived from user data goes through this one rule. Do not write
+// a second predicate; the stem rule has already been corrected once.
+func WindowsDeviceName(name string) bool {
 	stem := name
 	if i := strings.IndexAny(name, ".:"); i >= 0 {
 		stem = name[:i]

internal/server/handlers_artifact_export.go

@@ -9,6 +9,7 @@ import (
 	"github.com/go-chi/chi/v5"
 
 	"github.com/PerpetualSoftware/pad/internal/artifact"
+	"github.com/PerpetualSoftware/pad/internal/attachments"
 	"github.com/PerpetualSoftware/pad/internal/models"
 )
 
@@ -147,5 +148,12 @@ func (s *Server) handleExportItemArtifact(w http.ResponseWriter, r *http.Request
 // artifactExportFilename builds the download filename for an exported item:
 // "<slug>.pad.md".
 func artifactExportFilename(item *models.Item) string {
-	return item.Slug + ".pad.md"
+	name := item.Slug + ".pad.md"
+	// BUG-3185: the stem is the slug, so an item slugged "nul", "con", "com1"
+	// and so on names a Windows device, and a client saving the suggested name
+	// writes nowhere. Prefixed "_" as attachment names are (BUG-2822).
+	if attachments.WindowsDeviceName(name) {
+		return "_" + name
+	}
+	return name
 }

internal/server/handlers_artifact_export_device_test.go

@@ -0,0 +1,55 @@
+package server
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// BUG-3185: an export's suggested file name is "<slug>.pad.md", so an item
+// slugged like a Windows device named a device. The name is prefixed "_",
+// by the one device rule attachments use (BUG-2822).
+func TestArtifactExportFilename_BUG3185_DeviceStemIsPrefixed(t *testing.T) {
+	cases := []struct{ slug, want string }{
+		{"nul", "_nul.pad.md"},
+		{"con", "_con.pad.md"},
+		{"com1", "_com1.pad.md"},
+		{"lpt9", "_lpt9.pad.md"},
+		{"aux", "_aux.pad.md"},
+		// Near misses: a longer stem is not a device.
+		{"nulls", "nulls.pad.md"},
+		{"com10", "com10.pad.md"},
+		{"con-x", "con-x.pad.md"},
+		{"ship-it", "ship-it.pad.md"},
+	}
+	for _, c := range cases {
+		if got := artifactExportFilename(&models.Item{Slug: c.slug}); got != c.want {
+			t.Errorf("slug %q: got %q, want %q", c.slug, got, c.want)
+		}
+	}
+}
+
+// The binding leg: the export ENDPOINT carries the prefixed name in its
+// Content-Disposition, which is what both clients save under.
+func TestExportArtifact_BUG3185_DeviceSlugHeaderIsPrefixed(t *testing.T) {
+	srv := testServer(t)
+	ws := createWSForTest(t, srv)
+	pb := createItem(t, srv, ws, "playbooks", map[string]interface{}{
+		"title":   "NUL",
+		"content": "## Steps\n\n1. Nothing\n",
+		"fields":  `{"status":"active","trigger":"manual","scope":"all"}`,
+	})
+	if pb.Slug != "nul" {
+		t.Fatalf("precondition: the title must slug to exactly %q to exercise the device rule, got %q", "nul", pb.Slug)
+	}
+	rr := doRequest(srv, "GET", "/api/v1/workspaces/"+ws+"/items/"+pb.Slug+"/export", nil)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("export: expected 200, got %d: %s", rr.Code, rr.Body.String())
+	}
+	cd := rr.Header().Get("Content-Disposition")
+	if !strings.Contains(cd, `filename="_nul.pad.md"`) {
+		t.Errorf("Content-Disposition %q does not name _nul.pad.md", cd)
+	}
+}