gitmote

gitmote  ·  history  ·  branches

feat: rewrite rendered-markdown relative links to content verbs

fe077256d77b538915713a5dc295fc4e04671d76
Atanas Minev <atmin@protonmail.com> · 2026-07-05 21:21

diff --git a/docs/architecture/urls.md b/docs/architecture/urls.md
index 52a5d95..6fefda4 100644
--- a/docs/architecture/urls.md
+++ b/docs/architecture/urls.md
@@ -10,8 +10,9 @@ is no per-user namespace to disambiguate, so there is nothing to disambiguate.
> default-branch force-push) are implemented and reconciled into
> [storage.md](storage.md) and [auth.md](auth.md); the **`tree`/`blob`/`raw`
> content routing** (ref-in-path greedy resolution, unified `tree`, self-healing
-> `blob`, the bare `/{repo}` landing) is implemented per the sections below. Still
-> to land from the `tasks/urls-*` chain: rendered-markdown link rewriting, and the
+> `blob`, the bare `/{repo}` landing) and **rendered-markdown link rewriting**
+> (relative nav → `blob`/`tree`, embeds → `raw`, ref preserved) are implemented
+> per the sections below. Still to land from the `tasks/urls-*` chain: the
> dashboard UI on the new routes.
## The namespace
diff --git a/internal/render/render.go b/internal/render/render.go
index 749c684..f523fe4 100644
--- a/internal/render/render.go
+++ b/internal/render/render.go
@@ -14,6 +14,7 @@ package render
import (
"bytes"
"html/template"
+ "net/url"
"path"
"strings"
"sync"
@@ -23,7 +24,11 @@ import (
"github.com/alecthomas/chroma/v2/styles"
"github.com/microcosm-cc/bluemonday"
"github.com/yuin/goldmark"
+ "github.com/yuin/goldmark/ast"
"github.com/yuin/goldmark/extension"
+ "github.com/yuin/goldmark/parser"
+ "github.com/yuin/goldmark/text"
+ "github.com/yuin/goldmark/util"
highlighting "github.com/yuin/goldmark-highlighting/v2"
mermaid "go.abhg.dev/goldmark/mermaid"
@@ -109,8 +114,93 @@ var markdown = goldmark.New(
// by the sanitizer anyway.
&mermaid.Extender{RenderMode: mermaid.RenderModeClient, NoScript: true},
),
+ // Rewrite a rendered file's relative links/embeds into repo URLs. The
+ // transformer is a no-op unless the Convert call supplies a *LinkContext
+ // (MarkdownLinks does; plain Markdown does not).
+ goldmark.WithParserOptions(
+ parser.WithASTTransformers(util.Prioritized(linkTransformer{}, 100)),
+ ),
)
+// linkCtxKey carries the per-render *LinkContext through the parser context to
+// the link transformer.
+var linkCtxKey = parser.NewContextKey()
+
+// LinkContext supplies what the markdown link rewriter needs to turn a rendered
+// file's relative links and embeds into repo URLs: the repo name, the resolved
+// ref, the directory the rendered file lives in (relative paths resolve against
+// it), and a Stat callback reporting whether a repo-relative path is a "blob", a
+// "tree", or absent (ok=false).
+type LinkContext struct {
+ Repo string
+ Ref string
+ Dir string
+ Stat func(path string) (kind string, ok bool)
+}
+
+// rewrite maps one link/image destination to a repo URL, or returns it
+// unchanged. External (scheme/host), site-absolute, and pure-anchor targets are
+// left alone; a relative path is resolved against Dir and mapped to raw (images)
+// or blob/tree (nav, by the Stat lookup). A relative path that climbs out of the
+// repo, or a nav target that doesn't exist, is left as-is (visibly dangling)
+// rather than rewritten into a wrong URL.
+func (lc *LinkContext) rewrite(dest string, isImage bool) string {
+ u, err := url.Parse(dest)
+ if err != nil {
+ return dest
+ }
+ if u.Scheme != "" || u.Host != "" || u.Path == "" || path.IsAbs(u.Path) {
+ return dest
+ }
+ target := path.Join(lc.Dir, u.Path)
+ if target == ".." || strings.HasPrefix(target, "../") {
+ return dest
+ }
+ verb := "raw"
+ if !isImage {
+ kind, ok := lc.Stat(target)
+ if !ok {
+ return dest
+ }
+ if kind == "tree" {
+ verb = "tree"
+ } else {
+ verb = "blob"
+ }
+ }
+ out := "/" + lc.Repo + "/" + verb + "/" + lc.Ref + "/" + target
+ if u.RawQuery != "" {
+ out += "?" + u.RawQuery
+ }
+ if u.Fragment != "" {
+ out += "#" + u.Fragment
+ }
+ return out
+}
+
+// linkTransformer rewrites relative link and image destinations in the parsed
+// markdown when the Convert call supplies a *LinkContext.
+type linkTransformer struct{}
+
+func (linkTransformer) Transform(doc *ast.Document, _ text.Reader, pc parser.Context) {
+ lc, ok := pc.Get(linkCtxKey).(*LinkContext)
+ if !ok || lc == nil {
+ return
+ }
+ _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
+ if !entering {
+ return ast.WalkContinue, nil
+ }
+ switch node := n.(type) {
+ case *ast.Link:
+ node.Destination = []byte(lc.rewrite(string(node.Destination), false))
+ case *ast.Image:
+ node.Destination = []byte(lc.rewrite(string(node.Destination), true))
+ }
+ return ast.WalkContinue, nil
+ })
+}
+
// sanitizer strips dangerous constructs goldmark passes through, while allowing
// the class attribute chroma needs on the code elements it emits (so README
// code stays highlighted). UGCPolicy already forbids scripts, event handlers,
@@ -124,8 +214,19 @@ var sanitizer = func() *bluemonday.Policy {
// Markdown renders src to sanitized HTML. It never returns unsafe markup: even
// if goldmark errored, the (empty) output passes through the sanitizer.
func Markdown(src []byte) template.HTML {
+ return MarkdownLinks(src, nil)
+}
+
+// MarkdownLinks renders src like Markdown, additionally rewriting the file's
+// relative links and embeds into repo URLs via lc (see LinkContext). A nil lc
+// disables rewriting — it is exactly Markdown.
+func MarkdownLinks(src []byte, lc *LinkContext) template.HTML {
var buf bytes.Buffer
- if err := markdown.Convert(src, &buf); err != nil {
+ pc := parser.NewContext()
+ if lc != nil {
+ pc.Set(linkCtxKey, lc)
+ }
+ if err := markdown.Convert(src, &buf, parser.WithContext(pc)); err != nil {
return ""
}
return template.HTML(sanitizer.SanitizeBytes(buf.Bytes())) //nolint:gosec // sanitized above
diff --git a/internal/render/render_test.go b/internal/render/render_test.go
index 7895580..1875784 100644
--- a/internal/render/render_test.go
+++ b/internal/render/render_test.go
@@ -80,6 +80,71 @@ func TestMarkdownSanitizesXSS(t *testing.T) {
}
}
+func TestMarkdownLinks(t *testing.T) {
+ // A stat callback: doc.md and CONTRIBUTING.md are files, "sub" and "docs" are
+ // directories, everything else is absent.
+ lc := &LinkContext{
+ Repo: "app",
+ Ref: "main",
+ Dir: "docs",
+ Stat: func(p string) (string, bool) {
+ switch p {
+ case "docs/doc.md", "CONTRIBUTING.md":
+ return "blob", true
+ case "docs/sub", "sub":
+ return "tree", true
+ default:
+ return "", false
+ }
+ },
+ }
+
+ src := []byte(strings.Join([]string{
+ "[sib](./doc.md)", // sibling file → blob
+ "[dir](sub/)", // subdir → tree
+ "[up](../CONTRIBUTING.md)", // ../ up-link → blob at repo root
+ "![img](diagram.png)", // image → raw (no stat)
+ "[ext](https://x.example)", // external → unchanged
+ "[mail](mailto:a@b.co)", // mailto → unchanged
+ "[anchor](#section)", // in-page anchor → unchanged
+ "[abs](/other/thing)", // site-absolute → unchanged
+ "[gone](./missing.md)", // missing target → unchanged
+ }, "\n\n"))
+ out := string(MarkdownLinks(src, lc))
+
+ wantContains := []string{
+ `href="/app/blob/main/docs/doc.md"`,
+ `href="/app/tree/main/docs/sub"`,
+ `href="/app/blob/main/CONTRIBUTING.md"`,
+ `src="/app/raw/main/docs/diagram.png"`,
+ `href="https://x.example"`,
+ `href="mailto:a@b.co"`,
+ `href="#section"`,
+ `href="/other/thing"`,
+ `href="./missing.md"`, // dangling, left as-is
+ }
+ for _, w := range wantContains {
+ if !strings.Contains(out, w) {
+ t.Errorf("output missing %q:\n%s", w, out)
+ }
+ }
+ // A missing nav target must never be fabricated into a blob/tree URL.
+ if strings.Contains(out, "missing.md\"") && strings.Contains(out, "/app/blob/main/docs/missing.md") {
+ t.Errorf("missing target was rewritten into a false blob URL:\n%s", out)
+ }
+}
+
+func TestMarkdownLinksNilContextIsInert(t *testing.T) {
+ src := []byte("[x](./doc.md)\n\n![y](img.png)\n")
+ if a, b := MarkdownLinks(src, nil), Markdown(src); string(a) != string(b) {
+ t.Fatalf("nil LinkContext changed output:\n%s\nvs\n%s", a, b)
+ }
+ // Relative destinations pass through untouched with no context.
+ if out := string(Markdown(src)); !strings.Contains(out, `href="./doc.md"`) || !strings.Contains(out, `src="img.png"`) {
+ t.Fatalf("relative links altered without a context:\n%s", out)
+ }
+}
+
func TestMarkdownMermaid(t *testing.T) {
src := []byte("# Diagram\n\n```mermaid\ngraph TD\n A --> B\n```\n")
out := string(Markdown(src))
diff --git a/internal/webui/browse.go b/internal/webui/browse.go
index b2eafe8..ff0aed8 100644
--- a/internal/webui/browse.go
+++ b/internal/webui/browse.go
@@ -358,7 +358,7 @@ func (h *Handler) renderTree(w http.ResponseWriter, r *http.Request, c browseCtx
h.serverError(w, "list tree", err)
return
}
- readme := h.readme(r, c, entries)
+ readme := h.readme(r, c, treePath, entries)
data := treeData{
browseBase: h.browseHeader(r, c),
Path: treePath,
@@ -395,7 +395,7 @@ func (h *Handler) renderBlob(w http.ResponseWriter, r *http.Request, c browseCtx
case size > render.MaxSize:
data.Text = string(content)
case render.IsMarkdown(blobPath):
- data.Rendered = render.Markdown(content)
+ data.Rendered = render.MarkdownLinks(content, h.linkContext(r, c, path.Dir(blobPath)))
data.Mermaid = render.HasMermaid(data.Rendered)
default:
if hl, err := render.Highlight(content, blobPath); err == nil {
@@ -482,9 +482,10 @@ func (h *Handler) renderCommit(w http.ResponseWriter, r *http.Request, c browseC
}
// readme renders a directory's README.md (if any) to HTML for display below
-// the tree listing. It is best-effort: a missing, binary, oversize, or
-// unreadable README simply yields no rendered block.
-func (h *Handler) readme(r *http.Request, c browseCtx, entries []repo.TreeEntry) template.HTML {
+// the tree listing, with its relative links resolved against dir (the listed
+// directory). It is best-effort: a missing, binary, oversize, or unreadable
+// README simply yields no rendered block.
+func (h *Handler) readme(r *http.Request, c browseCtx, dir string, entries []repo.TreeEntry) template.HTML {
for _, e := range entries {
if e.Type != "blob" || !render.IsReadme(e.Name) {
continue
@@ -493,11 +494,29 @@ func (h *Handler) readme(r *http.Request, c browseCtx, entries []repo.TreeEntry)
if err != nil || binary || size > render.MaxSize {
return ""
}
- return render.Markdown(content)
+ return render.MarkdownLinks(content, h.linkContext(r, c, dir))
}
return ""
}
+// linkContext builds the markdown link rewriter's context for a file whose
+// relative links resolve against dir: the repo, the selected ref, and a Stat
+// callback backed by the materialized tree (so a nav link maps to blob vs tree).
+func (h *Handler) linkContext(r *http.Request, c browseCtx, dir string) *render.LinkContext {
+ return &render.LinkContext{
+ Repo: c.repo.Name,
+ Ref: c.ref,
+ Dir: dir,
+ Stat: func(p string) (string, bool) {
+ kind, err := repo.EntryType(r.Context(), c.dir, c.sha, p)
+ if err != nil {
+ return "", false
+ }
+ return kind, true
+ },
+ }
+}
+
// crumbs turns a path into its breadcrumb trail, each segment linking to the
// tree at that depth. The empty path yields no crumbs (the repo root link in
// the template stands alone).
diff --git a/internal/webui/browse_test.go b/internal/webui/browse_test.go
index 810aa3e..b23daac 100644
--- a/internal/webui/browse_test.go
+++ b/internal/webui/browse_test.go
@@ -43,6 +43,7 @@ func (x *harness) seedBrowseRepo(name, branch string) (head, first string) {
write(x.t, src, "README.md", "# App\n\nHello **world**.\n\n```go\nfunc main() {}\n```\n")
write(x.t, src, "diagram.md", "# Diagram\n\n```mermaid\ngraph TD\n A --> B\n```\n")
write(x.t, src, "sub/note.txt", "nested\n")
+ write(x.t, src, "links.md", "# Links\n\n[code](./hello.go)\n\n[dir](sub/)\n\n![img](bin.dat)\n\n[ext](https://example.com)\n\n[gone](./nope.md)\n")
write(x.t, src, "bin.dat", "a\x00b\x00c")
write(x.t, src, "big.go", "package main\n"+strings.Repeat("// filler line\n", 40000))
gitTest(x.t, src, "add", "-A")
@@ -92,6 +93,33 @@ func (x *harness) seedBrowseRepo(name, branch string) (head, first string) {
return head, first
}
+// TestBrowseMarkdownLinkRewriting: a rendered markdown file's relative links and
+// embeds are rewritten to the content verbs with the ref preserved (blob for a
+// file, tree for a directory, raw for an image), while external and dangling
+// links are left alone. This is the end-to-end fix for the "empty tree" bug.
+func TestBrowseMarkdownLinkRewriting(t *testing.T) {
+ x := newHarness(t)
+ x.seedBrowseRepo("app", "main")
+ session := x.login(x.mintTokenFor(x.admin.ID))
+
+ rec := x.do(http.MethodGet, "/app/blob/main/links.md", nil, session)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("links blob = %d (%s)", rec.Code, rec.Body)
+ }
+ body := rec.Body.String()
+ for _, want := range []string{
+ `href="/app/blob/main/hello.go"`, // sibling file → blob
+ `href="/app/tree/main/sub"`, // subdir → tree
+ `src="/app/raw/main/bin.dat"`, // embed → raw
+ `href="https://example.com"`, // external → unchanged
+ `href="./nope.md"`, // missing → left dangling
+ } {
+ if !strings.Contains(body, want) {
+ t.Errorf("rendered markdown missing %q:\n%s", want, body)
+ }
+ }
+}
+
// TestBrowseBlobOnDirRedirects: blob on a directory 301s to the tree URL,
// canonicalizing rather than 404ing (the self-healing verb).
func TestBrowseBlobOnDirRedirects(t *testing.T) {
diff --git a/tasks/README.md b/tasks/README.md
index e3560e7..c1578fe 100644
--- a/tasks/README.md
+++ b/tasks/README.md
@@ -18,15 +18,13 @@ namespace** (`host/<repo>`, no `owner/`, no `/-/`), content addressed in-path
visibility with spectators & collaborators, and rendered-markdown links that
actually resolve. Greenfield — **reset the bucket + meta**, no migration. Design +
rationale: [../docs/architecture/urls.md](../docs/architecture/urls.md). Order
-within the chain matters — the **flat namespace + visibility & access** and the
-**content routing** slices have landed (single-segment repos, `visibility`,
-repo-read browse authz, admin-only default-branch force-push; `tree`/`blob`/`raw`
-verbs with ref-in-path greedy resolution and the bare `/<repo>` landing); the
-rest remain:
+within the chain matters — the **flat namespace + visibility & access**, the
+**content routing**, and the **markdown link rewriting** slices have landed
+(single-segment repos, `visibility`, repo-read browse authz, admin-only
+default-branch force-push; `tree`/`blob`/`raw` verbs with ref-in-path greedy
+resolution and the bare `/<repo>` landing; rendered relative links → `blob`/`tree`
+and embeds → `raw`, ref preserved); one remains:
-- [urls-markdown-links.md](urls-markdown-links.md) — rewrite rendered-markdown
- relative links (nav → `blob`/`tree`) and embeds (→ `raw`), ref preserved.
- *(needs -content-routing.)*
- [urls-dashboard-ui.md](urls-dashboard-ui.md) — `/` dashboard, top-level globals
(`/login`, `/users`, …), per-repo `settings`/`access`/`secrets`. *(last —
reshapes the UI onto the new routes.)*
diff --git a/tasks/urls-markdown-links.md b/tasks/urls-markdown-links.md
deleted file mode 100644
index 330a8e4..0000000
--- a/tasks/urls-markdown-links.md
+++ /dev/null
@@ -1,56 +0,0 @@
-# Markdown link & embed rewriting
-
-Part of the **URLs** epic ([README](README.md)); implements the "relative links in
-rendered content" section of
-[../docs/architecture/urls.md](../docs/architecture/urls.md). Needs content
-routing ([urls-content-routing.md](urls-content-routing.md)) — it rewrites links
-*into* those verbs.
-
-## Spec
-
-Rendered markdown resolves in-repo relative links correctly, preserving the ref:
-
-- **Embedded assets** (`![](img.png)`, other embeds) → `/<repo>/raw/<ref>/<path>`
- (bytes, so the image actually renders).
-- **Navigation links** (`[x](./doc.md)`, `[y](../sub/)`) → the precise verb:
- `blob` for a file, `tree` for a directory (decided by a tree lookup), with the
- current ref baked in.
-- **Untouched:** absolute URLs, external `http(s)`/`mailto:`, and in-page anchors
- (`#section`).
-
-## Current
-
-The markdown renderer ([`internal/render`](../internal/render)) emits links
-verbatim. A relative link therefore resolves against the *browser's* current URL,
-which (pre-epic) carries `?ref=` in the query — dropped on a relative navigation —
-and can't distinguish tree from blob, so an in-repo link lands on "empty tree".
-Even after content routing puts the ref in the path, embedded images still need
-the `raw` verb, which a bare relative link can't express — so rewriting is
-required regardless.
-
-## Change
-
-- Give the render/browse seam the context to rewrite: the repo name, the resolved
- ref, the current file's directory, and a **tree-stat** callback (is `<path>` a
- blob, a tree, or absent?) — the materialized repo already backs this
- ([`internal/repo`](../internal/repo)).
-- In the markdown pipeline ([`internal/render`](../internal/render), consumed by
- [`browse.go`](../internal/webui/browse.go)), post-process the rendered tree: for
- each relative link/image, normalize it against the current dir, then map to
- `/<repo>/{raw|blob|tree}/<ref>/<path>` — `raw` for image/embed nodes, else
- `blob`/`tree` by the stat. Leave absolute/external/anchor targets alone.
-- A relative link to a **missing** path is left as-is (or rendered inert) rather
- than fabricating a wrong-verb URL — a dangling link stays visibly dangling.
-
-## Verify
-
-- A link to a sibling `*.md` → `/<repo>/blob/<ref>/…` with the ref preserved, and
- following it renders the target (the original "empty tree" bug is gone).
-- A link to a subdirectory → `/<repo>/tree/<ref>/…`; a `../` up-link resolves to
- the parent dir.
-- `![](diagram.png)` → `/<repo>/raw/<ref>/…` and the image renders.
-- External `https://…` links, `mailto:`, and `#anchor` are unchanged.
-- A link to a nonexistent path does not 500 and is not rewritten into a false
- blob/tree URL.
-- `gofmt`/`golangci-lint`/`go test ./...` clean; golden **and** failure paths
- (missing target, external-link passthrough, image→raw).