Harden local image reading

CLAUDE.md

@@ -32,7 +32,7 @@ Requires Go (see `go.mod`/`mise.toml` for version) and, for packaging, `just`, `
 Everything happens in `main()` in `main.go`, in a straight-line pipeline:
 
 1. **Flag parsing** — `-o`, `-v/-version`, `-h/-help`, `-b/-bare`. First positional arg is the input markdown file; a single `-` reads the markdown from stdin instead, in which case `baseDir` for image resolution is the working directory rather than the input file's directory.
-2. **Image inlining** (`newMarkdown` → `imageTransformer` + `htmlImageRenderer` → `embedImage`/`imageToDataURI`) — rewrites relative image references into base64 `data:` URIs so the output HTML is fully self-contained/offline-viewable. Markdown images (inline and reference-style) are rewritten by an AST transformer, so images inside code spans and code blocks are left alone. Raw `<img>` tags (`src` and `srcset`) are rewritten by a custom renderer for `HTMLBlock`/`RawHTML` nodes. `embedImage` strips query/fragment and decodes percent-encoding. `getMimeType` uses the extension, then content sniffing, and skips files that are not images. This includes path-traversal guards (caps `..` traversal depth) and a 10MB per-image size cap.
+2. **Image inlining** (`newMarkdown` → `imageTransformer` + `htmlImageRenderer` → `embedImage`/`imageToDataURI`) — rewrites relative image references into base64 `data:` URIs so the output HTML is fully self-contained/offline-viewable. Markdown images (inline and reference-style) are rewritten by an AST transformer, so images inside code spans and code blocks are left alone. Raw `<img>` tags (`src` and `srcset`) are rewritten by a custom renderer for `HTMLBlock`/`RawHTML` nodes. `embedImage` strips query/fragment and decodes percent-encoding. `imageToDataURI` only reads files with an allowlisted image extension (`getMimeType`), resolves symlinks before checking the path (at most 3 `..` levels above the base directory), accepts only regular files, and caps images at 10MB.
 3. **Markdown parsing** via Goldmark (built by `newMarkdown`), configured with:
    - `extension.GFM` (tables, task lists, strikethrough, etc.)
    - `extension.Typographer` (smart quotes/dashes)

main.go

@@ -13,7 +13,6 @@ import (
 	"io"
 	"io/fs"
 	"log"
-	"net/http"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -373,88 +372,92 @@ func isRelativePath(path string) bool {
 	return !filepath.IsAbs(path)
 }
 
-// imageToDataURI reads an image file and converts it to a base64 data URI
+// maxImageSize is the largest image file that is embedded
+const maxImageSize = 10 * 1024 * 1024 // 10MB
+
+// maxParentLevels is how many directory levels above the base directory an image may be
+const maxParentLevels = 3
+
+// imageToDataURI reads an image file and converts it to a base64 data URI.
+// It returns "" and logs a warning when the file must not or cannot be embedded.
 func imageToDataURI(imagePath string, baseDir string) string {
-	// Resolve the full path relative to the markdown file
-	fullPath := filepath.Join(baseDir, imagePath)
-
-	// Clean and validate the path to prevent path traversal attacks
-	cleanedPath, err := filepath.Abs(fullPath)
+	// Check the extension first so that non-image files are never opened
+	mimeType := getMimeType(imagePath)
+	if mimeType == "" {
+		log.Printf("Warning: %s does not have a supported image extension, skipping", imagePath)
+		return ""
+	}
+
+	// Resolve symlinks in both paths so that a link cannot point outside the allowed area
+	resolvedBase, err := filepath.EvalSymlinks(baseDir)
 	if err != nil {
-		log.Printf("Warning: Invalid image path %s: %v", fullPath, err)
+		log.Printf("Warning: Invalid base directory %s: %v", baseDir, err)
 		return ""
 	}
-
-	// Ensure the resolved path is within or relative to the base directory
-	cleanedBase, err := filepath.Abs(baseDir)
+	resolvedBase, err = filepath.Abs(resolvedBase)
 	if err != nil {
 		log.Printf("Warning: Invalid base directory %s: %v", baseDir, err)
 		return ""
 	}
-
-	// Check if the cleaned path starts with the base directory or is a reasonable relative reference
-	// We allow accessing parent directories for flexibility with markdown repos
-	if !strings.HasPrefix(cleanedPath, cleanedBase) {
-		relPath, err := filepath.Rel(cleanedBase, cleanedPath)
-		if err != nil {
-			log.Printf("Warning: Unable to determine relative path for %s: %v", imagePath, err)
-			return ""
-		}
-
-		// If the path goes outside the base directory, check parent traversal limits
-		if strings.HasPrefix(relPath, "..") {
-			// Allow up to 3 levels of parent directory traversal for flexibility
-			// Count the number of ".." path components
-			components := strings.Split(filepath.ToSlash(relPath), "/")
-			parentLevels := 0
-			for _, component := range components {
-				if component == ".." {
-					parentLevels++
-				}
-			}
-			if parentLevels > 3 {
-				log.Printf("Warning: Image path %s goes too many levels above base directory", imagePath)
-				return ""
-			}
+	resolvedPath, err := filepath.EvalSymlinks(filepath.Join(resolvedBase, imagePath))
+	if err != nil {
+		log.Printf("Warning: Unable to resolve image file %s: %v", imagePath, err)
+		return ""
+	}
+
+	// Allow images in the base directory, below it, or a few levels above it
+	// (for markdown repos that keep images in a sibling directory)
+	relPath, err := filepath.Rel(resolvedBase, resolvedPath)
+	if err != nil {
+		log.Printf("Warning: Unable to determine relative path for %s: %v", imagePath, err)
+		return ""
+	}
+	parentLevels := 0
+	for _, component := range strings.Split(filepath.ToSlash(relPath), "/") {
+		if component != ".." {
+			break
 		}
+		parentLevels++
 	}
-
-	// Check file size before reading (limit to 10MB to prevent memory issues)
-	fileInfo, err := os.Stat(cleanedPath)
+	if parentLevels > maxParentLevels {
+		log.Printf("Warning: Image path %s goes too many levels above base directory", imagePath)
+		return ""
+	}
+
+	// Read at most one byte past the limit, so a file that grows after the check is still capped
+	f, err := os.Open(resolvedPath)
 	if err != nil {
-		log.Printf("Warning: Unable to stat image file %s: %v", cleanedPath, err)
+		log.Printf("Warning: Unable to read image file %s: %v", resolvedPath, err)
 		return ""
 	}
-
-	const maxSize = 10 * 1024 * 1024 // 10MB
-	if fileInfo.Size() > maxSize {
-		log.Printf("Warning: Image file %s is too large (%d bytes, max %d bytes)", cleanedPath, fileInfo.Size(), maxSize)
+	defer f.Close()
+
+	fileInfo, err := f.Stat()
+	if err != nil {
+		log.Printf("Warning: Unable to stat image file %s: %v", resolvedPath, err)
 		return ""
 	}
-
-	// Read the image file
-	data, err := os.ReadFile(cleanedPath)
+	if !fileInfo.Mode().IsRegular() {
+		log.Printf("Warning: Image file %s is not a regular file", resolvedPath)
+		return ""
+	}
+
+	data, err := io.ReadAll(io.LimitReader(f, maxImageSize+1))
 	if err != nil {
-		log.Printf("Warning: Unable to read image file %s: %v", cleanedPath, err)
+		log.Printf("Warning: Unable to read image file %s: %v", resolvedPath, err)
 		return ""
 	}
-
-	mimeType := getMimeType(cleanedPath, data)
-	if mimeType == "" {
-		log.Printf("Warning: File %s is not a recognized image type, skipping", cleanedPath)
+	if len(data) > maxImageSize {
+		log.Printf("Warning: Image file %s is too large (max %d bytes)", resolvedPath, maxImageSize)
 		return ""
 	}
 
-	// Encode to base64
-	encoded := base64.StdEncoding.EncodeToString(data)
-
-	// Return data URI
-	return fmt.Sprintf("data:%s;base64,%s", mimeType, encoded)
+	return fmt.Sprintf("data:%s;base64,%s", mimeType, base64.StdEncoding.EncodeToString(data))
 }
 
-// getMimeType returns the image MIME type for a file, or "" when it is not an image.
-// It trusts the file extension first, then sniffs the content.
-func getMimeType(path string, data []byte) string {
+// getMimeType returns the MIME type for a supported image extension, or "" for any other file.
+// The extension allowlist keeps non-image files (keys, configs) out of the output.
+func getMimeType(path string) string {
 	switch strings.ToLower(filepath.Ext(path)) {
 	case ".jpg", ".jpeg":
 		return "image/jpeg"
@@ -475,9 +478,6 @@ func getMimeType(path string, data []byte) string {
 	case ".ico":
 		return "image/x-icon"
 	}
-	if detected := http.DetectContentType(data); strings.HasPrefix(detected, "image/") {
-		return detected
-	}
 	return ""
 }
 

main_test.go

@@ -115,28 +115,100 @@ func TestNonEmbeddableImagesAreLeftAlone(t *testing.T) {
 }
 
 func TestGetMimeType(t *testing.T) {
-	png := []byte("\x89PNG\r\n\x1a\n")
+	tests := map[string]string{
+		"a.PNG":  "image/png",
+		"a.avif": "image/avif",
+		"a.tif":  "image/tiff",
+		"a.tiff": "image/tiff",
+		"a.svg":  "image/svg+xml",
+		"noext":  "",
+		"a.dat":  "",
+		"a.txt":  "",
+		"id_rsa": "",
+	}
+	for path, want := range tests {
+		if got := getMimeType(path); got != want {
+			t.Errorf("getMimeType(%q) = %q, want %q", path, got, want)
+		}
+	}
+}
+
+func TestImageToDataURIPathRules(t *testing.T) {
+	root := t.TempDir()
+	base := filepath.Join(root, "a", "b", "c", "d", "docs")
+	write := func(path string) {
+		t.Helper()
+		if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(path, []byte("image data"), 0644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := os.MkdirAll(base, 0755); err != nil {
+		t.Fatal(err)
+	}
+	write(filepath.Join(base, "in.png"))
+	write(filepath.Join(base, "sub", "in.png"))
+	write(filepath.Join(base, "..", "up1.png"))
+	write(filepath.Join(base, "..", "..", "..", "up3.png"))
+	write(filepath.Join(base, "..", "..", "..", "..", "up4.png"))
+	write(filepath.Join(base+"-secret", "x.png")) // sibling that shares the name prefix
+	write(filepath.Join(base, "key.pem"))
+	write(filepath.Join(root, "outside", "target.png"))
+
+	if err := os.Symlink(filepath.Join(root, "outside", "target.png"), filepath.Join(base, "link.png")); err != nil {
+		t.Skip("symlinks not supported:", err)
+	}
+	if err := os.Symlink(filepath.Join(base, "in.png"), filepath.Join(base, "inlink.png")); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(filepath.Join(base, "key.pem"), filepath.Join(base, "disguised.png")); err != nil {
+		t.Fatal(err)
+	}
+
 	tests := []struct {
-		path string
-		data []byte
-		want string
+		path  string
+		embed bool
 	}{
-		{"a.PNG", nil, "image/png"},
-		{"a.avif", nil, "image/avif"},
-		{"a.tif", nil, "image/tiff"},
-		{"a.tiff", nil, "image/tiff"},
-		{"a.svg", nil, "image/svg+xml"},
-		{"noext", png, "image/png"},
-		{"a.dat", png, "image/png"},
-		{"a.txt", []byte("hello"), ""},
+		{"in.png", true},
+		{"sub/in.png", true},
+		{"../up1.png", true},
+		{"../../../up3.png", true},
+		{"../../../../up4.png", false},
+		{"../docs-secret/x.png", true}, // one level up is allowed
+		{"key.pem", false},
+		{"missing.png", false},
+		{"inlink.png", true},
+		{"link.png", false}, // symlink to a file outside the allowed area
+		{"sub", false},      // directory
 	}
 	for _, tt := range tests {
-		if got := getMimeType(tt.path, tt.data); got != tt.want {
-			t.Errorf("getMimeType(%q) = %q, want %q", tt.path, got, tt.want)
+		got := imageToDataURI(tt.path, base) != ""
+		if got != tt.embed {
+			t.Errorf("imageToDataURI(%q) embedded = %v, want %v", tt.path, got, tt.embed)
 		}
 	}
 }
 
+func TestImageToDataURISizeLimit(t *testing.T) {
+	base := t.TempDir()
+	atLimit := filepath.Join(base, "limit.png")
+	tooBig := filepath.Join(base, "big.png")
+	if err := os.WriteFile(atLimit, make([]byte, maxImageSize), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(tooBig, make([]byte, maxImageSize+1), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if imageToDataURI("limit.png", base) == "" {
+		t.Error("file at the size limit was not embedded")
+	}
+	if imageToDataURI("big.png", base) != "" {
+		t.Error("file over the size limit was embedded")
+	}
+}
+
 func TestTitleIsEscaped(t *testing.T) {
 	tests := []struct {
 		name   string