From eb9f408dd71d2f16ecd71b2971bb301027dde59e Mon Sep 17 00:00:00 2001 From: bdeshi Date: Thu, 30 Jul 2026 01:30:12 +0600 Subject: [PATCH] content: read a site root into bundles Bundle loading with no HTTP: walk content/, split YAML frontmatter, derive an NFC-normalised key and a language from the filename, and lift only title out of frontmatter so every other key stays readable through Extra (ADR-0002). Path safety is os.Root rather than a hand-rolled cleaner (ADR-0031). os.DirFS documents that it does not prevent symlink escape; os.Root refuses any name resolving outside the root, so the guard is a property of the type instead of a check to remember at each call site. Test: a symlink to a file above the root cannot be read. This clears the traversal item off the latent list. A bundle that will not parse is logged and skipped, never fatal (ADR-0029), as is a key claimed by two spellings of one variant (ADR-0021). Bundle carries only Key, Lang, Path, Title, Body and Extra; Date, Slug, Draft and Aliases arrive with the features that read them. --- docs/decisions.md | 13 ++ docs/state.md | 40 +++++- go.mod | 4 +- go.sum | 1 + internal/content/content.go | 217 +++++++++++++++++++++++++++++++ internal/content/content_test.go | 155 ++++++++++++++++++++++ 6 files changed, 423 insertions(+), 7 deletions(-) create mode 100644 internal/content/content.go create mode 100644 internal/content/content_test.go diff --git a/docs/decisions.md b/docs/decisions.md index 33b8e4e..2e45de5 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -354,3 +354,16 @@ Consequence: cheap now, and permanent-ish once the module path is fetched by any a module rename after publication needs a redirect or a major-version bump, and a deployed binary name appears in service files and container tags. Revisit if: never. Renaming again costs strictly more than this did. + +## ADR-0031 — Path safety is `os.Root`, not a hand-rolled check +Date: 2026-07-30 · Status: accepted +Decision: every read of the site root goes through an `*os.Root` obtained by `os.OpenRoot` (Go 1.24+). +No code cleans, joins or validates a request path itself, and `os.DirFS` is not used for the site root. +Why: `os.Root` refuses any name resolving outside the root, including through a symlink; `os.DirFS` +documents that it does *not* prevent symlink escape. So the traversal guard that has sat on the latent +list becomes a property of the type rather than a check somebody has to remember at every call site — +which is the only version that survives twenty features. +Consequence: cheap — the guard cannot be forgotten, and the test is one symlink. Expensive — reads must +go through the root handle, so no helper may take a `string` path and open it directly, and the root is +held for the life of the process. +Revisit if: never. A hand-rolled cleaner is strictly worse. diff --git a/docs/state.md b/docs/state.md index b640de8..6780d69 100644 --- a/docs/state.md +++ b/docs/state.md @@ -7,9 +7,11 @@ If this file disagrees with the code, the code is right and this file is a bug. | File | Purpose | LOC | |---|---|---| -| `go.mod` | module `khosra`; `x/text` and `yaml.v3` required, not yet imported | 8 | +| `go.mod` | module `khosra`; `x/text`, `yaml.v3` direct | 8 | +| `internal/content/content.go` | site root → bundles: `os.Root` open, walk, frontmatter split, key/lang derivation, NFC, collision drop | 217 | +| `internal/content/content_test.go` | table-driven; symlink-escape evidence for the path guard | 155 | -No Go source yet. This repo holds engine source only — the site root is external and passed with `-site` +No HTTP yet. This repo holds engine source only — the site root is external and passed with `-site` (ADR-0011). Dependencies: none. @@ -28,9 +30,9 @@ this change*. | Effects | 0 | **2** | Effect runner + trigger wiring (change / schedule / demand) | | Extensions | 0 | **3** | Extension registry + wire file (`extensions.md`) | | Interface implementations | — | **2** | The interface itself | -| Non-stdlib dependencies | 0 | budget in `scripts/budgets.env` | — | +| Non-stdlib dependencies | 2 direct, 7 modules | budget in `scripts/budgets.env` | — | -Allowlisted but not yet required: `goldmark` (markdown), `golang.org/x/text` (NFC, ADR-0015), +Allowlisted, in use: `goldmark` is not yet imported. Allowlist: `goldmark` (markdown), `golang.org/x/text` (NFC, ADR-0015), `gopkg.in/yaml.v3` (frontmatter, ADR-0020). ## Latent items — known, deliberately unfixed @@ -41,7 +43,6 @@ with a stated reason. A list nothing drains is a graveyard of known defects. | Item | Why it waits | Trigger to fix | |---|---|---| -| Path traversal guard on URL → file mapping | Not yet internet-facing | **Before first public deploy — hard blocker; the target is a real server (ADR-0010). Nothing mechanical enforces this — `verify.sh` does not read this list. Make it a table-driven test when the first file read lands.** | | No mechanical check that the counters are *correct* | The coupling gate makes forgetting them impossible, which is the real failure mode; checking values needs code to count | 3rd transform or 2nd route | | No mechanical gate on the untrusted boundary (ADR-0003) | Nothing untrusted exists yet | The comment path, Arc 3 — a test that untrusted input reaches no shortcode or template evaluation | @@ -51,6 +52,35 @@ None. Every decision the engine needs before Arc 1 and before the first deploy i Every ADR in `decisions.md` is accepted; none is open or proposed. +## Build queue + +The prompt sequence to a Grav-level engine. **Completed through 1.** Update this line as each lands; a +context refresh loses the conversation, not the plan. + +- [x] 0 · ADRs: pagination shape (0028), parse-failure policy (0029), rename (0030), path guard (0031) +- [x] 1 · bundle loading: `os.Root`, frontmatter, key/lang, NFC, collisions, skip-loudly +- [ ] 2 · serve a bundle at its permalink: `-site`, trailing-slash redirect, goldmark, reference theme +- [ ] 3 · language variants: `/bn/…`, fallback chain, `/en/…` redirect *(2nd routing case → resolver)* +- [ ] 4 · aliases +- [ ] 5 · section index pages, paginated *(first collection page → Query)* +- [ ] 6 · settings cascade *(re-adopt from `ideas/deferred-decisions.md`)* +- [ ] 7 · declared content types *(re-adopt)* +- [ ] 8 · template composition: per-type sets, block override, site `templates/` +- [ ] 9 · tags: global namespace, tag listings *(re-adopt)* +- [ ] 10 · sequences: series, sparse order, prev/next/archive +- [ ] 11 · typography + Bengali numerals transforms +- [ ] 12 · shortcodes *(3rd transform → Stage pipeline)* +- [ ] 13 · image derivatives *(first Effect)* +- [ ] 14 · feeds: primary, per-section, per-tag *(2nd Effect → Effect runner)* +- [ ] 15 · sitemap, robots, OpenGraph/JSON-LD +- [ ] 16 · in-process page cache with the validity record *(re-adopt)* +- [ ] 17 · `check` command +- [ ] 18 · `new` command +- [ ] 19 · `-dev`: reveal drafts and future-dated, template reload +- [ ] 20 · extras *(re-adopt)* +- [ ] 21 · polled change detection + Dockerfile +- [ ] 22 · complete the reference theme against the full contract + ## Arc retro log One line per completed arc: what it cost, what it taught, what it made unnecessary. diff --git a/go.mod b/go.mod index 9af949b..7d0c834 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,6 @@ module khosra go 1.26.1 require ( - golang.org/x/text v0.40.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + golang.org/x/text v0.40.0 + gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index d9d8e1a..50764a7 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,6 @@ golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/content/content.go b/internal/content/content.go new file mode 100644 index 0000000..a6341e5 --- /dev/null +++ b/internal/content/content.go @@ -0,0 +1,217 @@ +// Package content reads a site root into bundles. It knows the disk and nothing about HTTP. +// +// Every read goes through an [os.Root] (ADR-0031), so no path — from a filename or later from a +// request — can escape the site root, even through a symlink. +package content + +import ( + "bytes" + "fmt" + "io/fs" + "log/slog" + "os" + "path" + "strings" + + "golang.org/x/text/unicode/norm" + "gopkg.in/yaml.v3" +) + +// DefaultLang is the locale served at the root of the URL space; every other language is served under a +// prefix (ADR-0009). A filename with no language suffix means this locale (ADR-0021). +const DefaultLang = "en" + +// Bundle is one addressable piece of content in one language. +// +// Only Title is lifted out of frontmatter; every other key lands in Extra, so a template or a later +// feature can read a field the parser has never heard of (ADR-0002). Absence is always the zero value, +// never an error. +type Bundle struct { + // Key identifies the bundle across languages: its path under content/, without language suffix or + // extension, NFC-normalised. This is the identity in ADR-0004 and never contains a language. + Key string + // Lang is always set; a file with no suffix reports DefaultLang. + Lang string + // Path is the file this bundle was read from, relative to the site root. + Path string + // Title is empty when frontmatter omits it. Whether that is legal depends on the type, which + // nothing decides yet, so the parser accepts it. + Title string + // Body is everything after the frontmatter, unrendered. + Body []byte + // Extra holds every frontmatter key other than title, exactly as YAML parsed it. + Extra map[string]any +} + +// OpenSite opens a site root for reading. +// +// The returned fs.FS is backed by [os.Root], which refuses any name that would resolve outside dir, +// including through a symlink — unlike os.DirFS, which does not (ADR-0031). The root is held for the +// life of the process. +func OpenSite(dir string) (fs.FS, error) { + root, err := os.OpenRoot(dir) + if err != nil { + return nil, fmt.Errorf("open site root %s: %w", dir, err) + } + return root.FS(), nil +} + +// Scan reads every bundle under content/ in fsys. +// +// A bundle that cannot be parsed, or that collides with another on the same key and language, is logged +// at error level and left out; neither is fatal, because one mistyped colon must not take down a site +// (ADR-0029). An error is returned only when the walk itself fails. +func Scan(fsys fs.FS) ([]Bundle, error) { + var found []Bundle + err := fs.WalkDir(fsys, "content", func(p string, d fs.DirEntry, err error) error { + switch { + case err != nil: + return err + case d.IsDir(): + if skipDir(path.Base(p)) { + return fs.SkipDir + } + return nil + case !strings.HasSuffix(p, ".md"): + return nil + } + data, err := fs.ReadFile(fsys, p) + if err != nil { + slog.Error("skipping unreadable bundle", "path", p, "err", err) + return nil + } + b, err := Parse(strings.TrimPrefix(p, "content/"), data) + if err != nil { + slog.Error("skipping unparseable bundle", "path", p, "err", err) + return nil + } + b.Path = p + found = append(found, b) + return nil + }) + if err != nil { + return nil, fmt.Errorf("scan content: %w", err) + } + return dropCollisions(found), nil +} + +// Parse reads one bundle from the bytes of a file, named relative to content/. +func Parse(name string, data []byte) (Bundle, error) { + key, lang, ok := splitName(name) + if !ok { + return Bundle{}, fmt.Errorf("not a bundle filename: %s", name) + } + front, body := splitFrontmatter(data) + b := Bundle{Key: key, Lang: lang, Body: body, Extra: map[string]any{}} + if len(front) > 0 { + if err := yaml.Unmarshal(front, &b.Extra); err != nil { + return Bundle{}, fmt.Errorf("frontmatter: %w", err) + } + } + if t, isStr := b.Extra["title"].(string); isStr { + b.Title = t + } + delete(b.Extra, "title") + return b, nil +} + +// Normalise puts s into NFC. +// +// Every identifier goes through this: Bengali conjuncts have several byte encodings for text that looks +// identical, and macOS hands back NFD, so without it two indistinguishable files take different keys +// (ADR-0015). +func Normalise(s string) string { return norm.NFC.String(s) } + +// splitName derives a bundle key and language from a path under content/. +// +// index.md and _index.md name the directory they sit in; anything else names itself. A trailing +// two- or three-letter lowercase segment is a language suffix. +func splitName(name string) (key, lang string, ok bool) { + if !strings.HasSuffix(name, ".md") || name == "" { + return "", "", false + } + dir, base := path.Split(strings.TrimSuffix(name, ".md")) + dir = strings.TrimSuffix(dir, "/") + if base == "" { + return "", "", false + } + lang = DefaultLang + if i := strings.LastIndex(base, "."); i > 0 && isLangTag(base[i+1:]) { + lang, base = base[i+1:], base[:i] + } + if base == "index" || base == "_index" { + key = dir + } else { + key = path.Join(dir, base) + } + return Normalise(key), lang, true +} + +// isLangTag reports whether s looks like a language suffix: two or three lowercase letters. +func isLangTag(s string) bool { + if len(s) < 2 || len(s) > 3 { + return false + } + for _, r := range s { + if r < 'a' || r > 'z' { + return false + } + } + return true +} + +// skipDir reports whether a directory is not content: hidden, or underscore-prefixed. +func skipDir(base string) bool { + return strings.HasPrefix(base, ".") && base != "." || strings.HasPrefix(base, "_") +} + +// splitFrontmatter separates a leading --- delimited YAML block from the body. A file without one is +// all body. +func splitFrontmatter(data []byte) (front, body []byte) { + const fence = "---" + rest, hasFence := trimLeadingFence(data, fence) + if !hasFence { + return nil, data + } + if i := bytes.Index(rest, []byte("\n"+fence)); i >= 0 { + front = rest[:i+1] + body = rest[i+1+len(fence):] + return front, bytes.TrimLeft(body, "\r\n") + } + return nil, data +} + +// trimLeadingFence removes an opening fence line, reporting whether one was there. +func trimLeadingFence(data []byte, fence string) ([]byte, bool) { + s := bytes.TrimLeft(data, "\ufeff") + if !bytes.HasPrefix(s, []byte(fence)) { + return data, false + } + s = s[len(fence):] + s = bytes.TrimLeft(s, "\r") + if !bytes.HasPrefix(s, []byte("\n")) { + return data, false + } + return s[1:], true +} + +// dropCollisions removes every bundle sharing a key and language with another. +// +// Two spellings of the same variant — about.md and about.en.md, or about.md and about/index.md — are +// ambiguous rather than harmless, so none of them is served (ADR-0021). +func dropCollisions(all []Bundle) []Bundle { + seen := map[string]int{} + for _, b := range all { + seen[b.Key+"\x00"+b.Lang]++ + } + kept := make([]Bundle, 0, len(all)) + for _, b := range all { + if seen[b.Key+"\x00"+b.Lang] > 1 { + slog.Error("skipping ambiguous bundle: two files claim one key and language", + "key", b.Key, "lang", b.Lang, "path", b.Path) + continue + } + kept = append(kept, b) + } + return kept +} diff --git a/internal/content/content_test.go b/internal/content/content_test.go new file mode 100644 index 0000000..ca90f6e --- /dev/null +++ b/internal/content/content_test.go @@ -0,0 +1,155 @@ +package content + +import ( + "io/fs" + "os" + "path/filepath" + "testing" + "testing/fstest" +) + +func TestSplitNameDerivesKeyAndLang(t *testing.T) { + cases := []struct { + name, key, lang string + ok bool + }{ + {name: "pages/about.md", key: "pages/about", lang: "en", ok: true}, + {name: "pages/about.en.md", key: "pages/about", lang: "en", ok: true}, + {name: "pages/about.bn.md", key: "pages/about", lang: "bn", ok: true}, + {name: "posts/hello/index.md", key: "posts/hello", lang: "en", ok: true}, + {name: "posts/hello/index.bn.md", key: "posts/hello", lang: "bn", ok: true}, + {name: "comics/monsoon/_index.md", key: "comics/monsoon", lang: "en", ok: true}, + {name: "posts/my.post.md", key: "posts/my.post", lang: "en", ok: true}, + {name: "posts/notes.txt", ok: false}, + } + for _, c := range cases { + key, lang, ok := splitName(c.name) + if ok != c.ok { + t.Errorf("%s: ok = %v, want %v", c.name, ok, c.ok) + continue + } + if ok && (key != c.key || lang != c.lang) { + t.Errorf("%s: got %q/%q, want %q/%q", c.name, key, lang, c.key, c.lang) + } + } +} + +func TestParseSplitsFrontmatterAndKeepsUnknownKeys(t *testing.T) { + b, err := Parse("posts/hello.md", []byte("---\ntitle: Hello\ntags: [a, b]\n---\n\nBody text.\n")) + if err != nil { + t.Fatal(err) + } + if b.Title != "Hello" { + t.Errorf("title = %q", b.Title) + } + if got := string(b.Body); got != "Body text.\n" { + t.Errorf("body = %q", got) + } + if _, ok := b.Extra["tags"]; !ok { + t.Error("tags did not land in Extra") + } + if _, ok := b.Extra["title"]; ok { + t.Error("title should be lifted out of Extra, not duplicated") + } +} + +func TestParseWithoutFrontmatterIsAllBody(t *testing.T) { + b, err := Parse("pages/now.md", []byte("Just prose.\n")) + if err != nil { + t.Fatal(err) + } + if b.Title != "" || string(b.Body) != "Just prose.\n" { + t.Errorf("got title %q body %q", b.Title, b.Body) + } +} + +func TestParseRejectsBrokenFrontmatter(t *testing.T) { + if _, err := Parse("posts/bad.md", []byte("---\ntitle: [unclosed\n---\nbody\n")); err == nil { + t.Fatal("want an error for unparseable YAML") + } +} + +func TestParseMissingTitleIsLegal(t *testing.T) { + b, err := Parse("status/note.md", []byte("---\ndate: 2026-07-30\n---\nhi\n")) + if err != nil { + t.Fatalf("a titleless bundle must parse: %v", err) + } + if b.Title != "" { + t.Errorf("title = %q, want empty", b.Title) + } +} + +func TestScanSkipsBadBundlesAndUnderscoreDirs(t *testing.T) { + fsys := fstest.MapFS{ + "content/pages/about.md": {Data: []byte("---\ntitle: About\n---\nx\n")}, + "content/posts/hello/index.md": {Data: []byte("---\ntitle: Hello\n---\ny\n")}, + "content/posts/broken.md": {Data: []byte("---\ntitle: [oops\n---\nz\n")}, + "content/_drafts/secret.md": {Data: []byte("---\ntitle: Secret\n---\nq\n")}, + "content/pages/notes.txt": {Data: []byte("not markdown")}, + } + got, err := Scan(fsys) + if err != nil { + t.Fatal(err) + } + keys := map[string]bool{} + for _, b := range got { + keys[b.Key] = true + } + if len(got) != 2 || !keys["pages/about"] || !keys["posts/hello"] { + t.Fatalf("got %d bundles %v, want pages/about and posts/hello only", len(got), keys) + } +} + +func TestScanDropsAmbiguousVariants(t *testing.T) { + fsys := fstest.MapFS{ + "content/pages/about.md": {Data: []byte("---\ntitle: A\n---\n")}, + "content/pages/about.en.md": {Data: []byte("---\ntitle: B\n---\n")}, + "content/pages/now.md": {Data: []byte("---\ntitle: Now\n---\n")}, + } + got, err := Scan(fsys) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].Key != "pages/now" { + t.Fatalf("got %+v, want only pages/now: two spellings of one variant are ambiguous", got) + } +} + +func TestNormaliseFoldsDecomposedBengali(t *testing.T) { + decomposed := "\u0995\u09c7\u09be" // ka + vowel sign e + vowel sign aa + composed := "\u0995\u09cb" // ka + vowel sign o + if decomposed == composed { + t.Skip("inputs are already identical; nothing to prove") + } + if Normalise(decomposed) != composed { + t.Errorf("NFC(%q) = %q, want %q", decomposed, Normalise(decomposed), composed) + } +} + +// TestOpenSiteRefusesSymlinkEscape is the path-traversal guard's evidence: os.DirFS would happily +// follow this symlink, os.Root does not (ADR-0031). +func TestOpenSiteRefusesSymlinkEscape(t *testing.T) { + tmp := t.TempDir() + site := filepath.Join(tmp, "site") + if err := os.MkdirAll(filepath.Join(site, "content"), 0o755); err != nil { + t.Fatal(err) + } + secret := filepath.Join(tmp, "secret.txt") + if err := os.WriteFile(secret, []byte("private"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../secret.txt", filepath.Join(site, "escape.txt")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + fsys, err := OpenSite(site) + if err != nil { + t.Fatal(err) + } + if data, err := fs.ReadFile(fsys, "escape.txt"); err == nil { + t.Fatalf("read outside the site root succeeded with %q", data) + } + if _, err := fs.ReadFile(fsys, "../secret.txt"); err == nil { + t.Fatal("traversal with .. succeeded") + } +}