Skip to content
Merged
12 changes: 12 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@ AMA_THANKYOU_COPY=Question submitted! It will appear once answered.
# Strict-Transport-Security, Permissions-Policy) always ship.
CSP_DISABLE=false

# =============================================================================
# UPLOADS
# =============================================================================

# Set ORPHAN_SWEEP_DISABLED=true to skip the post-boot orphan-upload sweep.
# Default false: once per startup, MarkGo walks <UPLOAD_PATH>/<slug>/ and
# removes files no article references (by banner frontmatter field or body
# content — markdown image, raw HTML img/src, or plain link). Set to true
# if you manage files in the uploads tree independently of article
# references — otherwise unreferenced files will be cleaned on next boot.
ORPHAN_SWEEP_DISABLED=false

# =============================================================================
# LOGGING CONFIGURATION
# =============================================================================
Expand Down
38 changes: 38 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [3.18.1] - 2026-05-19

Wave-5 operator feedback patch. Four issues from log.1mb.dev.

### Fixed

- **CSRF cookie now stable across GETs (#101).** The middleware rotated
`_csrf` on every GET, even when a valid cookie was already present.
SPA navigation only swaps `<main>`, so the meta token rendered in
`<head>` went stale relative to the rotated cookie and subsequent
XHRs (banner upload, etc.) 403'd. The middleware now reuses the
existing cookie when its value decodes as 32 raw bytes of hex;
rotation only on absence or malformed value.
- **Banner uploads no longer orphan on swap or remove (#103).**
`HandleEdit` unlinks the previous banner file when it matches
`/uploads/<slug>/<filename>` shape and the operator swaps or clears
the field. External URLs, `/static/...` paths, and bare frontmatter
filenames are operator-managed and left alone. ENOENT silent; other
os.Remove failures warn-logged.
- **Autosave warning element contract pinned (#102).** The reported
"Draft saving unavailable" at edit-mode load is not reproducible
from the codebase: the template renders the warning element with
`hidden`, and `compose.js` only flips it visible after a
`localStorage.setItem` failure. Regression test added at the
template layer.

### Added

- **Startup orphan-upload sweep (#103).** One pass per boot walks
`<UPLOAD_PATH>/<slug>/` and removes files no article references —
via banner field or body content (markdown image, raw HTML img/src,
or plain link). URL-decoded, traversal-rejected. Async post-bind,
60s timeout bound. Disable with `ORPHAN_SWEEP_DISABLED=true` if
the operator manages files in the uploads tree independently of
article references.
- **`data-count` on `.tag-cloud-item` (#100).** Lets forkers size or
weight tags via CSS attribute selectors without JS.

## [3.18.0] - 2026-05-19

Compose readability pass + "Save Draft" regression fix.
Expand Down
2 changes: 2 additions & 0 deletions internal/commands/serve/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ func setupServer(cfg *config.Config, logger *slog.Logger) (*gin.Engine, *service
return nil, nil, nil, fmt.Errorf("article service: %w", err)
}

go runOrphanSweep(cfg, articleService, logger)

emailService := services.NewEmailService(&cfg.Email, cfg.Blog.Title, logger)
composeService := compose.NewService(cfg.ArticlesPath, cfg.Blog.Author)

Expand Down
74 changes: 74 additions & 0 deletions internal/commands/serve/sweep.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package serve

import (
"context"
"log/slog"
"time"

"github.com/1mb-dev/markgo/internal/config"
"github.com/1mb-dev/markgo/internal/models"
"github.com/1mb-dev/markgo/internal/services/article"
)

// sweepTimeout bounds the post-boot orphan sweep so a misconfigured upload
// directory cannot wedge the goroutine indefinitely.
const sweepTimeout = 60 * time.Second

// articleLister is the slice of ArticleServiceInterface that the sweep
// needs. Declared locally to avoid importing the parent services package.
//
// GetPages is included alongside GetAllArticles because GetAllArticles
// applies the DedicatedRouteArticle predicate (v3.13.0) and excludes
// published type:page content from its result. Without GetPages in the
// reference set, Page banner uploads would appear unreferenced and be
// swept on every restart.
type articleLister interface {
GetAllArticles() []*models.Article
GetDraftArticles() []*models.Article
GetPages() []*models.Article
}

// runOrphanSweep launches a single post-boot pass that removes upload
// files no article references. Honors ORPHAN_SWEEP_DISABLED. Logs a
// single slog.Info on completion with cleanup count and duration; errors
// during the walk are warn-logged inside the sweep itself.
//
// Recovers from any panic in the sweep path: best-effort cleanup must
// never bring down the server. A panic is logged at Error level so it
// surfaces in operator triage without losing the running process.
func runOrphanSweep(cfg *config.Config, articleSvc articleLister, logger *slog.Logger) {
defer func() {
if r := recover(); r != nil {
logger.Error("orphan sweep panicked", "panic", r)
}
}()

if cfg.OrphanSweepDisabled {
logger.Info("orphan sweep disabled via ORPHAN_SWEEP_DISABLED")
return
}
if cfg.Upload.Path == "" {
return
}

ctx, cancel := context.WithTimeout(context.Background(), sweepTimeout)
defer cancel()

// GetAllArticles excludes dedicated-route content (type:page + about
// slug) via the v3.13.0 predicate. Bring Pages back in explicitly
// so Page banner uploads don't appear unreferenced. The /about
// article remains a known gap — operators using a banner there
// should rely on per-action unlink (Phase 3) and ORPHAN_SWEEP_DISABLED
// for the startup pass.
articles := articleSvc.GetAllArticles()
articles = append(articles, articleSvc.GetDraftArticles()...)
articles = append(articles, articleSvc.GetPages()...)

start := time.Now()
cleaned, errs := article.OrphanSweep(ctx, articles, cfg.Upload.Path, logger)
logger.Info("orphan sweep complete",
"cleaned", cleaned,
"errors", len(errs),
"duration", time.Since(start),
)
}
132 changes: 132 additions & 0 deletions internal/commands/serve/sweep_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package serve

import (
"io"
"log/slog"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/1mb-dev/markgo/internal/config"
"github.com/1mb-dev/markgo/internal/models"
)

type stubArticleLister struct {
published []*models.Article
drafts []*models.Article
pages []*models.Article
}

func (s *stubArticleLister) GetAllArticles() []*models.Article { return s.published }
func (s *stubArticleLister) GetDraftArticles() []*models.Article { return s.drafts }
func (s *stubArticleLister) GetPages() []*models.Article { return s.pages }

func stageSweepFile(t *testing.T, root, slug, filename string) string {
t.Helper()
dir := filepath.Join(root, slug)
require.NoError(t, os.MkdirAll(dir, 0o755))
p := filepath.Join(dir, filename)
require.NoError(t, os.WriteFile(p, []byte("x"), 0o600))
return p
}

func TestRunOrphanSweep_RemovesOrphansWhenEnabled(t *testing.T) {
uploadPath := t.TempDir()
orphan := stageSweepFile(t, uploadPath, "stale", "orphan.png")

cfg := &config.Config{
Upload: config.UploadConfig{Path: uploadPath},
OrphanSweepDisabled: false,
}
lister := &stubArticleLister{}
logger := slog.New(slog.NewTextHandler(io.Discard, nil))

runOrphanSweep(cfg, lister, logger)

_, err := os.Stat(orphan)
assert.True(t, os.IsNotExist(err), "orphan must be removed when sweep enabled")
}

func TestRunOrphanSweep_SkippedWhenDisabled(t *testing.T) {
uploadPath := t.TempDir()
orphan := stageSweepFile(t, uploadPath, "stale", "orphan.png")

cfg := &config.Config{
Upload: config.UploadConfig{Path: uploadPath},
OrphanSweepDisabled: true,
}
lister := &stubArticleLister{}
logger := slog.New(slog.NewTextHandler(io.Discard, nil))

runOrphanSweep(cfg, lister, logger)

_, err := os.Stat(orphan)
assert.NoError(t, err, "orphan must survive when sweep disabled")
}

func TestRunOrphanSweep_SkippedWhenUploadPathEmpty(t *testing.T) {
cfg := &config.Config{
Upload: config.UploadConfig{Path: ""},
OrphanSweepDisabled: false,
}
lister := &stubArticleLister{}
logger := slog.New(slog.NewTextHandler(io.Discard, nil))

// Must not panic on empty upload path.
runOrphanSweep(cfg, lister, logger)
}

// TestRunOrphanSweep_PreservesPageBanners is the regression test for the
// dedicated-route exclusion bug: GetAllArticles() filters out type:page
// articles via the DedicatedRouteArticle predicate (v3.13.0). Without
// GetPages() in the lister union, banner uploads referenced by published
// Page articles would appear unreferenced and be swept on next startup.
func TestRunOrphanSweep_PreservesPageBanners(t *testing.T) {
uploadPath := t.TempDir()
pageBanner := stageSweepFile(t, uploadPath, "info", "page-banner.png")

cfg := &config.Config{
Upload: config.UploadConfig{Path: uploadPath},
OrphanSweepDisabled: false,
}
lister := &stubArticleLister{
pages: []*models.Article{
{Slug: "info", Banner: "/uploads/info/page-banner.png"},
},
}
logger := slog.New(slog.NewTextHandler(io.Discard, nil))

runOrphanSweep(cfg, lister, logger)

_, err := os.Stat(pageBanner)
assert.NoError(t, err,
"banner upload owned by a published Page article must survive the sweep")
}

// panickyLister forces a panic when the sweep tries to enumerate articles,
// proving that runOrphanSweep's defer/recover keeps the goroutine alive.
type panickyLister struct{}

func (panickyLister) GetAllArticles() []*models.Article {
panic("simulated lister failure")
}
func (panickyLister) GetDraftArticles() []*models.Article { return nil }
func (panickyLister) GetPages() []*models.Article { return nil }

func TestRunOrphanSweep_RecoversFromPanic(t *testing.T) {
cfg := &config.Config{
Upload: config.UploadConfig{Path: t.TempDir()},
OrphanSweepDisabled: false,
}
logger := slog.New(slog.NewTextHandler(io.Discard, nil))

// If recover() weren't in place, this call would propagate the panic
// up to the runtime — fatal in a goroutine. The fact that the test
// reaches the next line is the assertion.
assert.NotPanics(t, func() {
runOrphanSweep(cfg, panickyLister{}, logger)
})
}
9 changes: 9 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ type Config struct {
Upload UploadConfig `json:"upload"`
AMA AMAConfig `json:"ama"`
Security SecurityConfig `json:"security"`

// OrphanSweepDisabled skips the post-boot orphan-upload sweep when true.
// Default false: the sweep runs once per startup, removing files in
// <UPLOAD_PATH>/<slug>/ that no article (banner field or body content)
// references. Operators who manually drop assets into the uploads tree
// without referencing them from an article should set this to true.
OrphanSweepDisabled bool `json:"orphan_sweep_disabled"`
}

// SecurityConfig holds security-header configuration. Most security headers
Expand Down Expand Up @@ -353,6 +360,8 @@ func Load() (*Config, error) {
Security: SecurityConfig{
CSPDisable: getEnvBool("CSP_DISABLE", false),
},

OrphanSweepDisabled: getEnvBool("ORPHAN_SWEEP_DISABLED", false),
}

// Validate the configuration
Expand Down
2 changes: 1 addition & 1 deletion internal/handlers/ama.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (h *AMAHandler) Answer(c *gin.Context) {
input.Content = input.Content + "\n\n---\n\n" + form.Answer
input.Draft = false

if err := h.composeService.UpdateArticle(slug, input); err != nil {
if _, err := h.composeService.UpdateArticle(slug, input); err != nil {
h.handleError(c, err, "Failed to publish answer")
return
}
Expand Down
6 changes: 4 additions & 2 deletions internal/handlers/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ func (h *ComposeHandler) HandleEdit(c *gin.Context) {
return
}

if err := h.composeService.UpdateArticle(slug, &input); err != nil {
prevBanner, err := h.composeService.UpdateArticle(slug, &input)
if err != nil {
h.logger.Error("Failed to update post", "error", err, "slug", slug)
data := h.buildBaseTemplateData("Edit - " + h.config.Blog.Title)
data["template"] = templateCompose
Expand All @@ -244,6 +245,7 @@ func (h *ComposeHandler) HandleEdit(c *gin.Context) {
h.renderHTML(c, http.StatusInternalServerError, "base.html", data)
return
}
compose.UnlinkOwnedUpload(prevBanner, input.Banner, slug, h.config.Upload.Path, h.logger)

reloadOK := true
if err := h.articleService.ReloadArticles(); err != nil {
Expand Down Expand Up @@ -477,7 +479,7 @@ func (h *ComposeHandler) PublishDraft(c *gin.Context) {
}

input.Draft = false
if err := h.composeService.UpdateArticle(slug, input); err != nil {
if _, err := h.composeService.UpdateArticle(slug, input); err != nil {
h.logger.Error("Failed to publish draft", "error", err, "slug", slug)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to publish draft"})
return
Expand Down
Loading