From ba3f2bcef4e5bbecf4577ef366a80111807c9ec0 Mon Sep 17 00:00:00 2001 From: rokuosanai <288084358+rokuosanai@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:48:39 +0000 Subject: [PATCH 1/4] fix: write downloaded images atomically --- pkg/core/filesystem_articles.go | 16 +++++++++---- pkg/core/filesystem_articles_test.go | 34 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/pkg/core/filesystem_articles.go b/pkg/core/filesystem_articles.go index b91de39..4fd274b 100644 --- a/pkg/core/filesystem_articles.go +++ b/pkg/core/filesystem_articles.go @@ -155,15 +155,23 @@ func (r *FileSystemArticleRepository) saveImage(ctx context.Context, image *Imag } fullPath := filepath.Join(imageDir, filename) - file, err := os.Create(fullPath) + tempFile, err := os.CreateTemp(imageDir, "."+filename+"-*") if err != nil { - return "", fmt.Errorf("failed to create file %s: %w", fullPath, err) + return "", fmt.Errorf("failed to create temporary image file in %s: %w", imageDir, err) } - defer file.Close() + tempPath := tempFile.Name() + defer os.Remove(tempPath) - if _, err := io.Copy(file, asset.Body); err != nil { + if _, err := io.Copy(tempFile, asset.Body); err != nil { + _ = tempFile.Close() return "", fmt.Errorf("failed to write image to %s: %w", fullPath, err) } + if err := tempFile.Close(); err != nil { + return "", fmt.Errorf("failed to close image file %s: %w", fullPath, err) + } + if err := os.Rename(tempPath, fullPath); err != nil { + return "", fmt.Errorf("failed to finalize image file %s: %w", fullPath, err) + } return filename, nil } diff --git a/pkg/core/filesystem_articles_test.go b/pkg/core/filesystem_articles_test.go index b5c8121..30f4aac 100644 --- a/pkg/core/filesystem_articles_test.go +++ b/pkg/core/filesystem_articles_test.go @@ -27,6 +27,40 @@ func (r *fakeImageRepository) Fetch(ctx context.Context, image *Image) (*ImageAs }, nil } +type failingImageRepository struct{} + +func (failingImageRepository) Fetch(context.Context, *Image) (*ImageAsset, error) { + return &ImageAsset{ + Body: io.NopCloser(&failingReader{}), + ContentType: "image/png", + }, nil +} + +type failingReader struct { + read bool +} + +func (r *failingReader) Read(p []byte) (int, error) { + if r.read { + return 0, assert.AnError + } + r.read = true + copy(p, "partial") + return len("partial"), nil +} + +func TestFileSystemArticleRepository_SaveImage_RemovesPartialFileOnFailure(t *testing.T) { + tempDir := t.TempDir() + conf := *config.NewConfig() + conf.Output.Images.Filename = "[:id].png" + repo := &FileSystemArticleRepository{imageRepo: failingImageRepository{}} + + _, err := repo.saveImage(context.Background(), NewImage("https://example.com/image.png", "", 0), tempDir, conf, time.Now()) + require.Error(t, err) + _, statErr := os.Stat(filepath.Join(tempDir, "0.png")) + assert.True(t, os.IsNotExist(statErr)) +} + func TestFileSystemArticleRepository_Save_RewritesImageURLs(t *testing.T) { tests := []struct { name string From 9294241e0c61a40a87f7d7526093ab0577777f67 Mon Sep 17 00:00:00 2001 From: rokuosanai <288084358+rokuosanai@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:00:05 +0000 Subject: [PATCH 2/4] fix: preserve image file permissions --- pkg/core/filesystem_articles.go | 3 +++ pkg/core/filesystem_articles_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/pkg/core/filesystem_articles.go b/pkg/core/filesystem_articles.go index 4fd274b..b8604df 100644 --- a/pkg/core/filesystem_articles.go +++ b/pkg/core/filesystem_articles.go @@ -166,6 +166,9 @@ func (r *FileSystemArticleRepository) saveImage(ctx context.Context, image *Imag _ = tempFile.Close() return "", fmt.Errorf("failed to write image to %s: %w", fullPath, err) } + if err := tempFile.Chmod(0o644); err != nil { + return "", fmt.Errorf("failed to set image file permissions for %s: %w", fullPath, err) + } if err := tempFile.Close(); err != nil { return "", fmt.Errorf("failed to close image file %s: %w", fullPath, err) } diff --git a/pkg/core/filesystem_articles_test.go b/pkg/core/filesystem_articles_test.go index 30f4aac..3c7514e 100644 --- a/pkg/core/filesystem_articles_test.go +++ b/pkg/core/filesystem_articles_test.go @@ -61,6 +61,19 @@ func TestFileSystemArticleRepository_SaveImage_RemovesPartialFileOnFailure(t *te assert.True(t, os.IsNotExist(statErr)) } +func TestFileSystemArticleRepository_SaveImage_UsesReadableFilePermissions(t *testing.T) { + tempDir := t.TempDir() + conf := *config.NewConfig() + conf.Output.Images.Filename = "[:id].png" + repo := &FileSystemArticleRepository{imageRepo: &fakeImageRepository{contentType: "image/png", body: "png"}} + + filename, err := repo.saveImage(context.Background(), NewImage("https://example.com/image.png", "", 0), tempDir, conf, time.Now()) + require.NoError(t, err) + info, err := os.Stat(filepath.Join(tempDir, filename)) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o644), info.Mode().Perm()) +} + func TestFileSystemArticleRepository_Save_RewritesImageURLs(t *testing.T) { tests := []struct { name string From bfd6c19015d5b055f90a1bc7d740c0b8aba927e2 Mon Sep 17 00:00:00 2001 From: rokuosanai <288084358+rokuosanai@users.noreply.github.com> Date: Fri, 24 Jul 2026 11:16:35 +0000 Subject: [PATCH 3/4] fix: respect image output umask --- pkg/core/filesystem_articles.go | 29 ++++++++++++++++++++++++---- pkg/core/filesystem_articles_test.go | 11 +++++++++-- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/pkg/core/filesystem_articles.go b/pkg/core/filesystem_articles.go index b8604df..25f5437 100644 --- a/pkg/core/filesystem_articles.go +++ b/pkg/core/filesystem_articles.go @@ -2,6 +2,8 @@ package core import ( "context" + "crypto/rand" + "encoding/hex" "fmt" "io" "log/slog" @@ -155,7 +157,7 @@ func (r *FileSystemArticleRepository) saveImage(ctx context.Context, image *Imag } fullPath := filepath.Join(imageDir, filename) - tempFile, err := os.CreateTemp(imageDir, "."+filename+"-*") + tempFile, err := createImageTempFile(imageDir, filename) if err != nil { return "", fmt.Errorf("failed to create temporary image file in %s: %w", imageDir, err) } @@ -166,9 +168,6 @@ func (r *FileSystemArticleRepository) saveImage(ctx context.Context, image *Imag _ = tempFile.Close() return "", fmt.Errorf("failed to write image to %s: %w", fullPath, err) } - if err := tempFile.Chmod(0o644); err != nil { - return "", fmt.Errorf("failed to set image file permissions for %s: %w", fullPath, err) - } if err := tempFile.Close(); err != nil { return "", fmt.Errorf("failed to close image file %s: %w", fullPath, err) } @@ -179,6 +178,28 @@ func (r *FileSystemArticleRepository) saveImage(ctx context.Context, image *Imag return filename, nil } +func createImageTempFile(directory, filename string) (*os.File, error) { + const maxAttempts = 100 + + for range maxAttempts { + var randomBytes [16]byte + if _, err := rand.Read(randomBytes[:]); err != nil { + return nil, fmt.Errorf("generate random suffix: %w", err) + } + + path := filepath.Join(directory, "."+filename+"-"+hex.EncodeToString(randomBytes[:])) + file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666) + if err == nil { + return file, nil + } + if !os.IsExist(err) { + return nil, err + } + } + + return nil, fmt.Errorf("could not create a unique temporary file") +} + func resolveImageFilename(conf config.Config, image *Image, datetime time.Time) string { filename := conf.Output.Images.Filename if filename == "" { diff --git a/pkg/core/filesystem_articles_test.go b/pkg/core/filesystem_articles_test.go index 3c7514e..5140dda 100644 --- a/pkg/core/filesystem_articles_test.go +++ b/pkg/core/filesystem_articles_test.go @@ -61,8 +61,15 @@ func TestFileSystemArticleRepository_SaveImage_RemovesPartialFileOnFailure(t *te assert.True(t, os.IsNotExist(statErr)) } -func TestFileSystemArticleRepository_SaveImage_UsesReadableFilePermissions(t *testing.T) { +func TestFileSystemArticleRepository_SaveImage_RespectsProcessUmask(t *testing.T) { tempDir := t.TempDir() + referencePath := filepath.Join(tempDir, "reference") + reference, err := os.OpenFile(referencePath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666) + require.NoError(t, err) + require.NoError(t, reference.Close()) + referenceInfo, err := os.Stat(referencePath) + require.NoError(t, err) + conf := *config.NewConfig() conf.Output.Images.Filename = "[:id].png" repo := &FileSystemArticleRepository{imageRepo: &fakeImageRepository{contentType: "image/png", body: "png"}} @@ -71,7 +78,7 @@ func TestFileSystemArticleRepository_SaveImage_UsesReadableFilePermissions(t *te require.NoError(t, err) info, err := os.Stat(filepath.Join(tempDir, filename)) require.NoError(t, err) - assert.Equal(t, os.FileMode(0o644), info.Mode().Perm()) + assert.Equal(t, referenceInfo.Mode().Perm(), info.Mode().Perm()) } func TestFileSystemArticleRepository_Save_RewritesImageURLs(t *testing.T) { From 97d6788ded635dd81b1394801f24dd846491132e Mon Sep 17 00:00:00 2001 From: rokuosanai <288084358+rokuosanai@users.noreply.github.com> Date: Fri, 24 Jul 2026 22:59:47 +0000 Subject: [PATCH 4/4] fix: shorten temporary image filenames --- pkg/core/filesystem_articles.go | 6 +++--- pkg/core/filesystem_articles_test.go | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/core/filesystem_articles.go b/pkg/core/filesystem_articles.go index 25f5437..32860e9 100644 --- a/pkg/core/filesystem_articles.go +++ b/pkg/core/filesystem_articles.go @@ -157,7 +157,7 @@ func (r *FileSystemArticleRepository) saveImage(ctx context.Context, image *Imag } fullPath := filepath.Join(imageDir, filename) - tempFile, err := createImageTempFile(imageDir, filename) + tempFile, err := createImageTempFile(imageDir) if err != nil { return "", fmt.Errorf("failed to create temporary image file in %s: %w", imageDir, err) } @@ -178,7 +178,7 @@ func (r *FileSystemArticleRepository) saveImage(ctx context.Context, image *Imag return filename, nil } -func createImageTempFile(directory, filename string) (*os.File, error) { +func createImageTempFile(directory string) (*os.File, error) { const maxAttempts = 100 for range maxAttempts { @@ -187,7 +187,7 @@ func createImageTempFile(directory, filename string) (*os.File, error) { return nil, fmt.Errorf("generate random suffix: %w", err) } - path := filepath.Join(directory, "."+filename+"-"+hex.EncodeToString(randomBytes[:])) + path := filepath.Join(directory, ".gic-image-"+hex.EncodeToString(randomBytes[:])) file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666) if err == nil { return file, nil diff --git a/pkg/core/filesystem_articles_test.go b/pkg/core/filesystem_articles_test.go index 5140dda..6a9686e 100644 --- a/pkg/core/filesystem_articles_test.go +++ b/pkg/core/filesystem_articles_test.go @@ -81,6 +81,20 @@ func TestFileSystemArticleRepository_SaveImage_RespectsProcessUmask(t *testing.T assert.Equal(t, referenceInfo.Mode().Perm(), info.Mode().Perm()) } +func TestFileSystemArticleRepository_SaveImage_SupportsMaximumLengthFilename(t *testing.T) { + tempDir := t.TempDir() + filename := strings.Repeat("a", 251) + ".png" + conf := *config.NewConfig() + conf.Output.Images.Filename = filename + repo := &FileSystemArticleRepository{imageRepo: &fakeImageRepository{contentType: "image/png", body: "png"}} + + savedFilename, err := repo.saveImage(context.Background(), NewImage("https://example.com/image.png", "", 0), tempDir, conf, time.Now()) + require.NoError(t, err) + assert.Equal(t, filename, savedFilename) + _, err = os.Stat(filepath.Join(tempDir, savedFilename)) + require.NoError(t, err) +} + func TestFileSystemArticleRepository_Save_RewritesImageURLs(t *testing.T) { tests := []struct { name string