From 7c6cffbcdecbba60f6667b017c9142aab0ac5586 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Mon, 27 Apr 2026 09:44:07 +0300 Subject: fix(processor): handle os.Remove errors for markdown inbox extras When removing markdown inbox extras (embedded local images), os.Remove errors were silently ignored. If removal failed, the image remained in the inbox and was later published as a standalone image post. - Check os.Remove errors and return a wrapped error. - Rollback the already-saved post directory when extra removal fails. - Deduplicate extras to avoid failing on duplicate references (e.g. same image referenced twice in one markdown). - Add a negative test that verifies the error is returned and no post is persisted when removing an embedded image fails. Fixes duplicate image posts caused by leftover inbox extras. --- internal/processor/processor.go | 11 +++++++- internal/processor/processor_test.go | 55 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/internal/processor/processor.go b/internal/processor/processor.go index 0bbc28e..9620e62 100644 --- a/internal/processor/processor.go +++ b/internal/processor/processor.go @@ -169,8 +169,17 @@ func commitPlan(plan postPlan, postsDir string, now time.Time) error { return err } + // Deduplicate extras in case the same file is referenced multiple times. + seen := make(map[string]bool, len(inboxExtras)) for _, path := range inboxExtras { - _ = os.Remove(path) + if seen[path] { + continue + } + seen[path] = true + if err := os.Remove(path); err != nil { + _ = os.RemoveAll(postDir) + return fmt.Errorf("remove inbox extra %s: %w", path, err) + } } return os.Remove(plan.srcPath) diff --git a/internal/processor/processor_test.go b/internal/processor/processor_test.go index b5e8f20..df39008 100644 --- a/internal/processor/processor_test.go +++ b/internal/processor/processor_test.go @@ -6,6 +6,7 @@ import ( "image/png" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -318,3 +319,57 @@ func TestRun_duplicateImageClaimsInSameMarkdownAllowed(t *testing.T) { t.Fatalf("n=%d; want 1", n) } } + +func TestRun_markdownWithLocalImage_removeFails(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("chmod does not reliably deny removal on Windows") + } + + in := t.TempDir() + out := t.TempDir() + + pngPath := filepath.Join(in, "embed.png") + f, err := os.Create(pngPath) + if err != nil { + t.Fatal(err) + } + if err := png.Encode(f, image.NewRGBA(image.Rect(0, 0, 2, 2))); err != nil { + f.Close() + t.Fatal(err) + } + f.Close() + + md := `![x](embed.png) +text` + if err := os.WriteFile(filepath.Join(in, "post.md"), []byte(md), 0o644); err != nil { + t.Fatal(err) + } + + // Remove write permission from the inbox so os.Remove on the extra fails. + if err := os.Chmod(in, 0o555); err != nil { + t.Fatal(err) + } + defer os.Chmod(in, 0o755) // restore for cleanup + + _, err = Run(ctx, &config.Config{InputDir: in, OutputDir: out, BaseURL: "https://x"}) + if err == nil { + t.Fatal("expected error when inbox extra removal fails") + } + if !strings.Contains(err.Error(), "embed.png") { + t.Fatalf("error should mention embed.png, got: %v", err) + } + + // Ensure nothing was persisted: post directory should have been rolled back. + entries, _ := os.ReadDir(filepath.Join(out, "posts")) + if len(entries) != 0 { + t.Fatalf("expected no posts after rollback, got %d", len(entries)) + } + + // Source files should still be in the inbox. + for _, name := range []string{"post.md", "embed.png"} { + if _, err := os.Stat(filepath.Join(in, name)); err != nil { + t.Fatalf("source %s should still exist: %v", name, err) + } + } +} -- cgit v1.2.3