From 12d1d6e212670677c37592dcfe6522b11fb17523 Mon Sep 17 00:00:00 2001 From: Vinayak Mishra Date: Mon, 18 May 2026 11:55:25 +0545 Subject: [PATCH 1/5] feat(article): add strict ValidateSlug for operator-supplied slugs Exports a slug contract for compose/new-page writes (Phase 2 prereq). Stricter than FileSystemRepository.validateSlug, which stays permissive for already-stored slugs at admin surfaces. --- internal/services/article/predicate.go | 58 ++++++++++++++++++++- internal/services/article/predicate_test.go | 48 +++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/internal/services/article/predicate.go b/internal/services/article/predicate.go index 4c85418..4233ebe 100644 --- a/internal/services/article/predicate.go +++ b/internal/services/article/predicate.go @@ -1,6 +1,12 @@ package article -import "github.com/1mb-dev/markgo/internal/models" +import ( + "fmt" + "regexp" + "strings" + + "github.com/1mb-dev/markgo/internal/models" +) // TypePage is the explicit frontmatter type value for evergreen pages // served by the /p/:slug route. @@ -41,3 +47,53 @@ func CanonicalURLFor(a *models.Article) string { } return "/writing/" + a.Slug } + +// SlugMaxLength caps slug length for operator-supplied slugs. Filesystem +// safety + URL-readability ceiling. +const SlugMaxLength = 100 + +// slugCharClass enforces lowercase ASCII letters, digits, and hyphens. +// generateSlug() in the compose service already produces this shape, so +// auto-generated slugs are regex-compatible by construction. +var slugCharClass = regexp.MustCompile(`^[a-z0-9-]+$`) + +// reservedSlugs blocks slugs that would confuse readers if served from +// /p/ by shadowing feed-like names. The set is deliberately minimal: +// only names that collide with content-discovery conventions. Reserved +// route prefixes (/about, /writing, /admin, etc.) live at different paths +// and don't need explicit blocking here. +var reservedSlugs = map[string]struct{}{ + "index": {}, + "feed": {}, + "rss": {}, + "atom": {}, +} + +// ValidateSlug enforces the strict contract for operator-supplied slugs +// at the compose/new-page authoring boundary. Rejects anything that +// wouldn't be a clean URL component or would shadow a feed-like name. +// +// This is intentionally stricter than FileSystemRepository.validateSlug, +// which guards already-stored slugs from path traversal at admin-only +// surfaces. Historical articles may have looser slugs (e.g. uppercase +// from pre-validation eras) — those still work via the repository's +// permissive check. New writes through the compose form must satisfy +// this stricter contract. +func ValidateSlug(slug string) error { + if strings.TrimSpace(slug) == "" { + return fmt.Errorf("slug cannot be empty") + } + if strings.Contains(slug, "..") || strings.Contains(slug, "/") || strings.Contains(slug, "\\") { + return fmt.Errorf("slug contains path-traversal characters: %s", slug) + } + if len(slug) > SlugMaxLength { + return fmt.Errorf("slug exceeds %d characters: %d", SlugMaxLength, len(slug)) + } + if !slugCharClass.MatchString(slug) { + return fmt.Errorf("slug must match %s: %s", slugCharClass.String(), slug) + } + if _, blocked := reservedSlugs[slug]; blocked { + return fmt.Errorf("slug is reserved: %s", slug) + } + return nil +} diff --git a/internal/services/article/predicate_test.go b/internal/services/article/predicate_test.go index ac7f456..82cd053 100644 --- a/internal/services/article/predicate_test.go +++ b/internal/services/article/predicate_test.go @@ -1,6 +1,7 @@ package article import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -47,3 +48,50 @@ func TestCanonicalURLFor(t *testing.T) { }) } } + +func TestValidateSlug(t *testing.T) { + tests := []struct { + name string + slug string + wantErr bool + }{ + {"valid lowercase letters", "intro", false}, + {"valid with hyphens", "my-evergreen-page", false}, + {"valid with digits", "release-2026", false}, + {"valid digits only", "123", false}, + {"valid single char", "a", false}, + + {"empty", "", true}, + {"whitespace only", " ", true}, + + {"path traversal dots", "..", true}, + {"embedded traversal", "foo/../bar", true}, + {"forward slash", "foo/bar", true}, + {"backslash", "foo\\bar", true}, + + {"uppercase letter", "Intro", true}, + {"underscore", "my_page", true}, + {"space", "my page", true}, + {"unicode", "café", true}, + {"dot", "my.page", true}, + + {"too long", strings.Repeat("a", SlugMaxLength+1), true}, + {"at length limit", strings.Repeat("a", SlugMaxLength), false}, + + {"reserved index", "index", true}, + {"reserved feed", "feed", true}, + {"reserved rss", "rss", true}, + {"reserved atom", "atom", true}, + {"reserved-adjacent ok", "feed-2026", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateSlug(tt.slug) + if tt.wantErr { + assert.Error(t, err, "expected error for %q", tt.slug) + } else { + assert.NoError(t, err, "expected no error for %q", tt.slug) + } + }) + } +} From 7d261d79be63e32b1669d4f4a2ec6fab20d59b33 Mon Sep 17 00:00:00 2001 From: Vinayak Mishra Date: Mon, 18 May 2026 12:11:25 +0545 Subject: [PATCH 2/5] feat(compose): /compose/new-page authoring affordance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #79. Separate route renders the form in page mode: required slug input, type=page hidden, banner/tags/categories/link_url hidden. Edit of an existing page surfaces the same mode (header + hidden fields) while keeping the slug immutable. CreatePost branches on TypePage — explicit slug, no date frontmatter. Handler validates the operator's slug via article.ValidateSlug plus a uniqueness check against the in-memory store. FAB stays thought-scoped. --- internal/commands/serve/command.go | 1 + internal/handlers/compose.go | 66 +++++++++ internal/handlers/compose_test.go | 168 ++++++++++++++++++++++ internal/services/compose/service.go | 47 ++++-- internal/services/compose/service_test.go | 79 ++++++++++ web/templates/compose.html | 50 ++++++- 6 files changed, 394 insertions(+), 17 deletions(-) diff --git a/internal/commands/serve/command.go b/internal/commands/serve/command.go index 75f40af..4f98bcc 100644 --- a/internal/commands/serve/command.go +++ b/internal/commands/serve/command.go @@ -451,6 +451,7 @@ func setupRoutes(router *gin.Engine, h *handlers.Router, sessionStore *middlewar middleware.CSRF(secureCookie), ) registerGET(composeGroup, "", h.Compose.ShowCompose) + registerGET(composeGroup, "/new-page", h.Compose.ShowComposeNewPage) composeGroup.POST("", h.Compose.HandleSubmit) registerGET(composeGroup, "/edit/:slug", h.Compose.ShowEdit) composeGroup.POST("/edit/:slug", h.Compose.HandleEdit) diff --git a/internal/handlers/compose.go b/internal/handlers/compose.go index 78bed58..eeaba33 100644 --- a/internal/handlers/compose.go +++ b/internal/handlers/compose.go @@ -55,6 +55,21 @@ func NewComposeHandler( } } +// validatePageSlug returns a human-readable error message if the slug +// is not acceptable for a new type:page article, or empty string on +// success. Combines the strict ValidateSlug contract (charset, length, +// reserved set) with a uniqueness check against the in-memory article +// store. The handler renders the message in the error banner. +func (h *ComposeHandler) validatePageSlug(slug string) string { + if err := articlepkg.ValidateSlug(slug); err != nil { + return "Slug invalid: " + err.Error() + } + if existing, err := h.articleService.GetArticleBySlug(slug); err == nil && existing != nil { + return "Slug already in use: " + slug + } + return "" +} + // canonicalPathForSlug resolves the canonical URL for a composed or // edited post by looking up the article in the in-memory store. Existing // type:page articles edited via /compose/edit/:slug must redirect to @@ -93,6 +108,18 @@ func (h *ComposeHandler) ShowCompose(c *gin.Context) { h.renderHTML(c, http.StatusOK, "base.html", data) } +// ShowComposeNewPage renders the compose form in page-authoring mode. +// Pages need an explicit slug and skip date/tags/categories — the template +// branches on data["mode"] == "page" to surface a slug input and hide +// the article-shaped fields. +func (h *ComposeHandler) ShowComposeNewPage(c *gin.Context) { + data := h.buildBaseTemplateData("New page - " + h.config.Blog.Title) + data["template"] = templateCompose + data["mode"] = articlepkg.TypePage + data["csrf_token"] = csrfToken(c) + h.renderHTML(c, http.StatusOK, "base.html", data) +} + // ShowEdit renders the compose form pre-filled with an existing article. func (h *ComposeHandler) ShowEdit(c *gin.Context) { slug := c.Param("slug") @@ -112,6 +139,11 @@ func (h *ComposeHandler) ShowEdit(c *gin.Context) { data["input"] = input data["editing"] = true data["slug"] = slug + if input.Type == articlepkg.TypePage { + // Surface page-mode so the template can hide article-only fields + // (link_url, tags, categories, banner) when editing a page. + data["mode"] = articlepkg.TypePage + } data["csrf_token"] = csrfToken(c) h.renderHTML(c, http.StatusOK, "base.html", data) } @@ -165,6 +197,7 @@ func (h *ComposeHandler) HandleEdit(c *gin.Context) { Banner: c.PostForm("banner"), BannerAlt: c.PostForm("banner_alt"), Draft: c.PostForm("draft") == "on", + Type: c.PostForm("type"), } if input.Content == "" { @@ -174,6 +207,9 @@ func (h *ComposeHandler) HandleEdit(c *gin.Context) { data["input"] = input data["editing"] = true data["slug"] = slug + if input.Type == articlepkg.TypePage { + data["mode"] = articlepkg.TypePage + } data["csrf_token"] = refreshCSRFToken(c) if c.IsAborted() { return @@ -190,6 +226,9 @@ func (h *ComposeHandler) HandleEdit(c *gin.Context) { data["input"] = input data["editing"] = true data["slug"] = slug + if input.Type == articlepkg.TypePage { + data["mode"] = articlepkg.TypePage + } data["csrf_token"] = refreshCSRFToken(c) if c.IsAborted() { return @@ -224,6 +263,8 @@ func (h *ComposeHandler) HandleSubmit(c *gin.Context) { Banner: c.PostForm("banner"), BannerAlt: c.PostForm("banner_alt"), Draft: c.PostForm("draft") == "on", + Type: c.PostForm("type"), + Slug: c.PostForm("slug"), } if input.Content == "" { @@ -231,6 +272,9 @@ func (h *ComposeHandler) HandleSubmit(c *gin.Context) { data["template"] = templateCompose data["error"] = "Content is required" data["input"] = input + if input.Type == articlepkg.TypePage { + data["mode"] = articlepkg.TypePage + } data["csrf_token"] = refreshCSRFToken(c) if c.IsAborted() { return @@ -239,6 +283,25 @@ func (h *ComposeHandler) HandleSubmit(c *gin.Context) { return } + // Page-mode validation: pages need a valid, unique slug. Run before + // CreatePost so the operator sees a render with their input intact + // rather than a generic 500. + if input.Type == articlepkg.TypePage { + if errMsg := h.validatePageSlug(input.Slug); errMsg != "" { + data := h.buildBaseTemplateData("New page - " + h.config.Blog.Title) + data["template"] = templateCompose + data["mode"] = articlepkg.TypePage + data["error"] = errMsg + data["input"] = input + data["csrf_token"] = refreshCSRFToken(c) + if c.IsAborted() { + return + } + h.renderHTML(c, http.StatusBadRequest, "base.html", data) + return + } + } + slug, err := h.composeService.CreatePost(&input) if err != nil { h.logger.Error("Failed to create post", "error", err) @@ -246,6 +309,9 @@ func (h *ComposeHandler) HandleSubmit(c *gin.Context) { data["template"] = templateCompose data["error"] = "Failed to create post. Please try again." data["input"] = input + if input.Type == articlepkg.TypePage { + data["mode"] = articlepkg.TypePage + } data["csrf_token"] = refreshCSRFToken(c) if c.IsAborted() { return diff --git a/internal/handlers/compose_test.go b/internal/handlers/compose_test.go index c5bd1c6..ede87e4 100644 --- a/internal/handlers/compose_test.go +++ b/internal/handlers/compose_test.go @@ -1579,3 +1579,171 @@ func TestShowEdit(t *testing.T) { assert.Equal(t, http.StatusNotFound, w.Code) }) } + +// --------------------------------------------------------------------------- +// ShowComposeNewPage tests +// --------------------------------------------------------------------------- + +func TestShowComposeNewPage(t *testing.T) { + t.Run("renders compose form in page mode", func(t *testing.T) { + handler, _ := createFormComposeHandler(t) + + router := gin.New() + router.GET("/compose/new-page", handler.ShowComposeNewPage) + + req := httptest.NewRequest("GET", "/compose/new-page", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + }) +} + +// --------------------------------------------------------------------------- +// HandleSubmit — page-mode validation +// --------------------------------------------------------------------------- + +func TestHandleSubmit_PageMode(t *testing.T) { + t.Run("creates page and redirects to /p/", func(t *testing.T) { + handler, tmpDir := createFormComposeHandler(t) + + router := gin.New() + router.Use(func(c *gin.Context) { + c.Set("csrf_secure", false) + c.Next() + }) + router.POST("/compose", handler.HandleSubmit) + + form := url.Values{ + "content": {"Evergreen page body."}, + "title": {"About"}, + "type": {"page"}, + "slug": {"about-the-author"}, + "_csrf": {"test-token"}, + } + req := httptest.NewRequest("POST", "/compose", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusSeeOther, w.Code) + // MockArticleService doesn't know the new slug, so canonicalPathForSlug + // falls back to the synthetic empty-Type article — /writing/. + // In production the slug would be reloaded into the repo and resolve + // to /p/. The redirect path itself is exercised; the canonical + // resolution is covered by predicate tests. + assert.NotEmpty(t, w.Header().Get("Location")) + + entries, err := os.ReadDir(tmpDir) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, entries[0].Name(), "-about-the-author.md") + + content, err := os.ReadFile(tmpDir + "/" + entries[0].Name()) + require.NoError(t, err) + assert.Contains(t, string(content), "type: page") + assert.Contains(t, string(content), "slug: about-the-author") + assert.NotContains(t, string(content), "date:", "page frontmatter omits date") + }) + + t.Run("empty slug returns 400", func(t *testing.T) { + handler, _ := createFormComposeHandler(t) + + router := gin.New() + router.Use(func(c *gin.Context) { + c.Set("csrf_secure", false) + c.Next() + }) + router.POST("/compose", handler.HandleSubmit) + + form := url.Values{ + "content": {"Body."}, + "title": {"X"}, + "type": {"page"}, + "slug": {""}, + "_csrf": {"test-token"}, + } + req := httptest.NewRequest("POST", "/compose", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + }) + + t.Run("reserved slug returns 400", func(t *testing.T) { + handler, _ := createFormComposeHandler(t) + + router := gin.New() + router.Use(func(c *gin.Context) { + c.Set("csrf_secure", false) + c.Next() + }) + router.POST("/compose", handler.HandleSubmit) + + form := url.Values{ + "content": {"Body."}, + "title": {"X"}, + "type": {"page"}, + "slug": {"feed"}, + "_csrf": {"test-token"}, + } + req := httptest.NewRequest("POST", "/compose", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + }) + + t.Run("colliding slug returns 400", func(t *testing.T) { + handler, _ := createFormComposeHandler(t) + + router := gin.New() + router.Use(func(c *gin.Context) { + c.Set("csrf_secure", false) + c.Next() + }) + router.POST("/compose", handler.HandleSubmit) + + // MockArticleService.GetArticleBySlug returns a hit for "test-article". + form := url.Values{ + "content": {"Body."}, + "title": {"X"}, + "type": {"page"}, + "slug": {"test-article"}, + "_csrf": {"test-token"}, + } + req := httptest.NewRequest("POST", "/compose", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + }) + + t.Run("invalid slug charset returns 400", func(t *testing.T) { + handler, _ := createFormComposeHandler(t) + + router := gin.New() + router.Use(func(c *gin.Context) { + c.Set("csrf_secure", false) + c.Next() + }) + router.POST("/compose", handler.HandleSubmit) + + form := url.Values{ + "content": {"Body."}, + "title": {"X"}, + "type": {"page"}, + "slug": {"My_Page"}, + "_csrf": {"test-token"}, + } + req := httptest.NewRequest("POST", "/compose", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + }) +} diff --git a/internal/services/compose/service.go b/internal/services/compose/service.go index 5e453e4..1d06fcc 100644 --- a/internal/services/compose/service.go +++ b/internal/services/compose/service.go @@ -27,6 +27,11 @@ func NewService(articlesPath, defaultAuthor string) *Service { } } +// typePage mirrors article.TypePage. The article package imports compose +// (for ContainSlugPath), so we can't import the other direction; the +// constant is duplicated here as a synchronization point. +const typePage = "page" + // Input represents the compose form or API submission. type Input struct { Content string `json:"content"` @@ -41,22 +46,40 @@ type Input struct { Asker string `json:"asker"` AskerEmail string `json:"asker_email"` Type string `json:"type"` + // Slug is operator-supplied; required when Type == "page" because + // pages need stable, hand-picked URLs. Ignored otherwise — other + // types derive slugs from title or timestamp. + Slug string `json:"slug"` } // CreatePost creates a new markdown post file from compose input. // Returns the generated slug. +// +// Pages (Type == "page") use input.Slug verbatim and omit the date +// frontmatter — they're evergreen by definition and served by /p/:slug. +// All other types derive their slug from title or fall back to a +// timestamp-prefixed slug. func (s *Service) CreatePost(input *Input) (string, error) { now := time.Now() - // Generate slug (fall back to timestamp-prefixed slug if title produces - // empty slug, e.g. non-ASCII titles or empty Title field). Prefix - // reflects content type so `ls articles/` reveals intent at a glance. var slug string - if input.Title != "" { - slug = generateSlug(input.Title) - } - if slug == "" { - slug = fmt.Sprintf("%s-%d", slugPrefixFor(input.Type), now.UnixMilli()) + switch input.Type { + case typePage: + if strings.TrimSpace(input.Slug) == "" { + return "", fmt.Errorf("page requires an explicit slug") + } + slug = input.Slug + default: + // Generate slug from title; fall back to timestamp-prefixed slug + // if title produces empty slug (e.g. non-ASCII titles or empty + // Title field). Prefix reflects content type so `ls articles/` + // reveals intent at a glance. + if input.Title != "" { + slug = generateSlug(input.Title) + } + if slug == "" { + slug = fmt.Sprintf("%s-%d", slugPrefixFor(input.Type), now.UnixMilli()) + } } // Parse comma-separated tags @@ -68,10 +91,14 @@ func (s *Service) CreatePost(input *Input) (string, error) { } } - // Build frontmatter map (only non-empty fields) + // Build frontmatter map (only non-empty fields). Pages skip `date:` + // because they're evergreen and /p index sorts alphabetically, not + // by date. fm := map[string]any{ "slug": slug, - "date": now.Format(time.RFC3339), + } + if input.Type != typePage { + fm["date"] = now.Format(time.RFC3339) } setIfNonEmpty(fm, "title", input.Title) setIfNonEmpty(fm, "description", input.Description) diff --git a/internal/services/compose/service_test.go b/internal/services/compose/service_test.go index b800fe6..a78aff6 100644 --- a/internal/services/compose/service_test.go +++ b/internal/services/compose/service_test.go @@ -338,6 +338,85 @@ func TestLoadArticle_AMAFields(t *testing.T) { assert.Equal(t, "What is your favorite programming language?", input.Content) } +func TestCreatePost_PageType_RequiresSlug(t *testing.T) { + dir := t.TempDir() + svc := NewService(dir, "Test Author") + + _, err := svc.CreatePost(&Input{ + Type: "page", + Title: "About", + Content: "Page body.", + }) + + assert.Error(t, err, "page with empty slug should fail") + assert.Contains(t, err.Error(), "slug") + + // No file should have been written. + files, _ := filepath.Glob(filepath.Join(dir, "*.md")) + assert.Empty(t, files) +} + +func TestCreatePost_PageType_UsesExplicitSlug(t *testing.T) { + dir := t.TempDir() + svc := NewService(dir, "Test Author") + + slug, err := svc.CreatePost(&Input{ + Type: "page", + Slug: "my-evergreen", + Title: "Unrelated Title", + Content: "Page body.", + }) + + require.NoError(t, err) + assert.Equal(t, "my-evergreen", slug, "page slug should come from Input.Slug, not generateSlug(Title)") + + files, _ := filepath.Glob(filepath.Join(dir, "*.md")) + require.Len(t, files, 1) + assert.Contains(t, files[0], "-my-evergreen.md", "filename should still carry the date prefix + explicit slug") +} + +func TestCreatePost_PageType_OmitsDate(t *testing.T) { + dir := t.TempDir() + svc := NewService(dir, "Test Author") + + _, err := svc.CreatePost(&Input{ + Type: "page", + Slug: "about-the-site", + Title: "About", + Content: "Page body.", + }) + require.NoError(t, err) + + files, _ := filepath.Glob(filepath.Join(dir, "*.md")) + require.Len(t, files, 1) + content, err := os.ReadFile(files[0]) + require.NoError(t, err) + + s := string(content) + assert.Contains(t, s, "type: page") + assert.Contains(t, s, "slug: about-the-site") + assert.NotContains(t, s, "date:", "page frontmatter should omit date") +} + +func TestCreatePost_NonPage_PreservesDateBehavior(t *testing.T) { + dir := t.TempDir() + svc := NewService(dir, "Test Author") + + // Regression: non-page types continue to emit date frontmatter. + _, err := svc.CreatePost(&Input{ + Type: "article", + Title: "Regular Article", + Content: "Body.", + }) + require.NoError(t, err) + + files, _ := filepath.Glob(filepath.Join(dir, "*.md")) + require.Len(t, files, 1) + content, err := os.ReadFile(files[0]) + require.NoError(t, err) + assert.Contains(t, string(content), "date:") +} + func TestUpdateArticle_PreservesAMAFields(t *testing.T) { dir := t.TempDir() svc := NewService(dir, "Test Author") diff --git a/web/templates/compose.html b/web/templates/compose.html index 42abaef..a90312a 100644 --- a/web/templates/compose.html +++ b/web/templates/compose.html @@ -3,11 +3,22 @@
@@ -25,8 +36,31 @@

Compose

Draft saving unavailable — copy your work before leaving
-
+ + + + {{ if eq .mode "page" }} + {{ if .editing }}{{/* slug immutable on edit — no input field */}}{{ else }} +
+ + + Lowercase letters, digits, hyphens. URL becomes /p/<slug>. Cannot be changed after create. +
+ {{ end }} + {{ end }} +