From f05f5a13c1574e05d39efe9e87a02ed4c4a865c8 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sun, 24 May 2026 14:39:57 -0700 Subject: [PATCH 1/9] feat(wiki): index image references with kind='image' Lay the groundwork for image support by tracking ![](path) references in the existing links table, distinguished from wikilinks by a new kind column. Every lifecycle question for an asset can now be answered with an index query, no parallel bookkeeping. - Add a migration runner keyed on wiki_state.schema_version; first migration adds the kind column to links via ALTER TABLE. Probe with PRAGMA table_info so the run is idempotent. - Switch parsePage to walk the goldmark AST for ast.Image nodes; wikilink extraction (still string-scan, since [[..]] is non-standard) is unchanged. External URLs and anchor-only refs are skipped. - Insert image refs alongside wikilinks in indexPage and Reindex with kind='image'. - Constrain getLinks, getBacklinks, and AllLinks to kind='link' so existing callers see the same page-edge surface as before. Tests cover: image refs indexed and distinguished by kind, dedup on repeat references, reindex drops stale rows, migration is idempotent across reopens, migration from a legacy (pre-kind) schema. --- internal/wiki/images_test.go | 284 +++++++++++++++++++++++++++++++++++ internal/wiki/index.go | 15 +- internal/wiki/migrate.go | 154 +++++++++++++++++++ internal/wiki/pages.go | 12 +- internal/wiki/parse.go | 83 +++++++++- internal/wiki/wiki.go | 10 ++ 6 files changed, 546 insertions(+), 12 deletions(-) create mode 100644 internal/wiki/images_test.go create mode 100644 internal/wiki/migrate.go diff --git a/internal/wiki/images_test.go b/internal/wiki/images_test.go new file mode 100644 index 0000000..dd6c151 --- /dev/null +++ b/internal/wiki/images_test.go @@ -0,0 +1,284 @@ +package wiki + +import ( + "context" + "os" + "path/filepath" + "strconv" + "testing" +) + +// TestImageRefsIndexed verifies that markdown image references are recorded +// in the links table with kind='image' and don't leak into the wikilink +// query surface (GetBacklinks, AllLinks, GetPage.Links). +func TestImageRefsIndexed(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "guide.md", `# Guide + +Here's a diagram: + +![architecture](guide.assets/architecture.png) + +And a wikilink to [[index]]. Also an external image +![external](https://example.com/foo.png) that should NOT be indexed. +`) + writeFile(t, dir, "index.md", `# Index + +See [[guide]]. +`) + + w, err := Open(dir) + if err != nil { + t.Fatalf("Open: %v", err) + } + t.Cleanup(func() { w.Close() }) + + ctx := context.Background() + + // 1. Image row must exist in `links` with kind='image'. + rows, err := w.db.QueryContext(ctx, "SELECT source, target, kind FROM links WHERE kind = 'image'") + if err != nil { + t.Fatalf("query: %v", err) + } + type rec struct { + source, target, kind string + } + var imgs []rec + for rows.Next() { + var r rec + if err := rows.Scan(&r.source, &r.target, &r.kind); err != nil { + t.Fatal(err) + } + imgs = append(imgs, r) + } + rows.Close() + if len(imgs) != 1 { + t.Fatalf("got %d image rows, want 1: %+v", len(imgs), imgs) + } + if got := imgs[0]; got.source != "guide" || got.target != "guide.assets/architecture.png" || got.kind != "image" { + t.Errorf("image row = %+v, want {guide, guide.assets/architecture.png, image}", got) + } + + // 2. Wikilink query path must not include the image target. + links, err := w.getLinks(ctx, "guide") + if err != nil { + t.Fatalf("getLinks: %v", err) + } + if len(links) != 1 || links[0] != "index" { + t.Errorf("getLinks(guide) = %v, want [index]", links) + } + + // 3. Backlinks for the image asset (via the kind='image' query) + // should surface the embedding page. + imgBacklinks := queryBacklinks(t, w, "guide.assets/architecture.png", "image") + if len(imgBacklinks) != 1 || imgBacklinks[0] != "guide" { + t.Errorf("image backlinks = %v, want [guide]", imgBacklinks) + } + + // 4. Backlinks for the page must not include the image target as a source. + pageBacklinks, err := w.GetBacklinks(ctx, "guide") + if err != nil { + t.Fatalf("GetBacklinks: %v", err) + } + if len(pageBacklinks) != 1 || pageBacklinks[0] != "index" { + t.Errorf("GetBacklinks(guide) = %v, want [index]", pageBacklinks) + } + + // 5. AllLinks excludes image edges. + all, err := w.AllLinks(ctx) + if err != nil { + t.Fatalf("AllLinks: %v", err) + } + for _, l := range all { + if l.Target == "guide.assets/architecture.png" { + t.Errorf("AllLinks leaked image edge: %+v", l) + } + } +} + +// TestImageRefDeduplication ensures the same image referenced multiple +// times from one page produces a single row, not duplicates. +func TestImageRefDeduplication(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "page.md", `# Page + +![one](page.assets/a.png) +![two](page.assets/a.png) +![three](page.assets/a.png) +`) + w, err := Open(dir) + if err != nil { + t.Fatalf("Open: %v", err) + } + t.Cleanup(func() { w.Close() }) + + var n int + if err := w.db.QueryRow("SELECT COUNT(*) FROM links WHERE source='page' AND kind='image'").Scan(&n); err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("image row count = %d, want 1", n) + } +} + +// TestImageRefReindexCleansStale verifies that removing an image reference +// from a page and re-indexing drops the link row. +func TestImageRefReindexCleansStale(t *testing.T) { + dir := t.TempDir() + abs := filepath.Join(dir, "page.md") + if err := os.WriteFile(abs, []byte(`# Page + +![v1](page.assets/v1.png) +`), 0o644); err != nil { + t.Fatal(err) + } + w, err := Open(dir) + if err != nil { + t.Fatalf("Open: %v", err) + } + t.Cleanup(func() { w.Close() }) + + if err := w.UpdatePage(context.Background(), "page", `# Page + +![v2](page.assets/v2.png) +`); err != nil { + t.Fatalf("UpdatePage: %v", err) + } + + var n int + if err := w.db.QueryRow( + "SELECT COUNT(*) FROM links WHERE source='page' AND kind='image' AND target=?", + "page.assets/v1.png").Scan(&n); err != nil { + t.Fatal(err) + } + if n != 0 { + t.Errorf("stale v1 image ref still present (%d rows)", n) + } + + if err := w.db.QueryRow( + "SELECT COUNT(*) FROM links WHERE source='page' AND kind='image' AND target=?", + "page.assets/v2.png").Scan(&n); err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("v2 image ref not indexed (%d rows)", n) + } +} + +// TestMigrationIdempotent runs Open twice on the same directory to confirm +// the migration runner doesn't trip on its second pass. +func TestMigrationIdempotent(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "p.md", "# P\n") + + w1, err := Open(dir) + if err != nil { + t.Fatalf("Open #1: %v", err) + } + v1 := schemaVersion(t, w1) + w1.Close() + + w2, err := Open(dir) + if err != nil { + t.Fatalf("Open #2: %v", err) + } + t.Cleanup(func() { w2.Close() }) + v2 := schemaVersion(t, w2) + + if v1 != v2 { + t.Errorf("schema_version changed across reopens: %d -> %d", v1, v2) + } + if v1 == 0 { + t.Errorf("schema_version still 0 after migrate; expected latest > 0") + } +} + +// TestMigrationFromLegacySchema simulates a database that pre-dates the +// kind column by manually creating the old links schema, then verifies +// that Open transparently upgrades it. +func TestMigrationFromLegacySchema(t *testing.T) { + dir := t.TempDir() + + // Build a "legacy" database by opening, then dropping kind, then + // resetting schema_version. This is the simplest portable way to + // fake a pre-migration state without checking in a binary fixture. + w, err := Open(dir) + if err != nil { + t.Fatalf("Open seed: %v", err) + } + // SQLite can't DROP COLUMN before 3.35; emulate by rebuilding. + if _, err := w.db.Exec(` + DROP TABLE links; + CREATE TABLE links ( + source TEXT NOT NULL, + target TEXT NOT NULL, + PRIMARY KEY (source, target) + ); + CREATE INDEX idx_links_target ON links(target); + DELETE FROM wiki_state WHERE key = 'schema_version'; + `); err != nil { + t.Fatalf("legacy reset: %v", err) + } + // Pre-seed a wikilink row in the old shape. + if _, err := w.db.Exec(`INSERT INTO links (source, target) VALUES ('a', 'b')`); err != nil { + t.Fatalf("seed legacy row: %v", err) + } + w.Close() + + w2, err := Open(dir) + if err != nil { + t.Fatalf("Open post-legacy: %v", err) + } + t.Cleanup(func() { w2.Close() }) + + // kind column must exist now. + has, err := columnExists(w2, "links", "kind") + if err != nil { + t.Fatalf("columnExists: %v", err) + } + if !has { + t.Fatal("kind column not added by migration") + } + + // The legacy row must have been backfilled with kind='link'. + var kind string + if err := w2.db.QueryRow(`SELECT kind FROM links WHERE source='a' AND target='b'`).Scan(&kind); err != nil { + t.Fatalf("query legacy row: %v", err) + } + if kind != "link" { + t.Errorf("legacy row backfill kind = %q, want %q", kind, "link") + } +} + +// --- helpers --- + +func queryBacklinks(t *testing.T, w *Wiki, target, kind string) []string { + t.Helper() + rows, err := w.db.Query("SELECT source FROM links WHERE target = ? AND kind = ?", target, kind) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + var out []string + for rows.Next() { + var s string + if err := rows.Scan(&s); err != nil { + t.Fatal(err) + } + out = append(out, s) + } + return out +} + +func schemaVersion(t *testing.T, w *Wiki) int { + t.Helper() + raw, ok := w.readStateKey(context.Background(), stateKeySchemaVersion) + if !ok { + return 0 + } + v, err := strconv.Atoi(raw) + if err != nil { + t.Fatalf("parse schema_version %q: %v", raw, err) + } + return v +} diff --git a/internal/wiki/index.go b/internal/wiki/index.go index 0379dc6..a7c8f48 100644 --- a/internal/wiki/index.go +++ b/internal/wiki/index.go @@ -132,7 +132,13 @@ func (w *Wiki) Reindex(ctx context.Context) (ReindexStats, error) { return ReindexStats{}, err } for _, target := range parsed.links { - if _, err := tx.ExecContext(ctx, "INSERT OR IGNORE INTO links (source, target) VALUES (?, ?)", pagePath, target); err != nil { + if _, err := tx.ExecContext(ctx, "INSERT OR IGNORE INTO links (source, target, kind) VALUES (?, ?, 'link')", pagePath, target); err != nil { + tx.Rollback() + return ReindexStats{}, err + } + } + for _, target := range parsed.images { + if _, err := tx.ExecContext(ctx, "INSERT OR IGNORE INTO links (source, target, kind) VALUES (?, ?, 'image')", pagePath, target); err != nil { tx.Rollback() return ReindexStats{}, err } @@ -232,7 +238,12 @@ func (w *Wiki) indexPage(ctx context.Context, pagePath string) error { return err } for _, target := range parsed.links { - if _, err := tx.ExecContext(ctx, "INSERT OR IGNORE INTO links (source, target) VALUES (?, ?)", pagePath, target); err != nil { + if _, err := tx.ExecContext(ctx, "INSERT OR IGNORE INTO links (source, target, kind) VALUES (?, ?, 'link')", pagePath, target); err != nil { + return err + } + } + for _, target := range parsed.images { + if _, err := tx.ExecContext(ctx, "INSERT OR IGNORE INTO links (source, target, kind) VALUES (?, ?, 'image')", pagePath, target); err != nil { return err } } diff --git a/internal/wiki/migrate.go b/internal/wiki/migrate.go new file mode 100644 index 0000000..0678f1b --- /dev/null +++ b/internal/wiki/migrate.go @@ -0,0 +1,154 @@ +package wiki + +import ( + "context" + "fmt" + "log/slog" + "strconv" +) + +// Schema migrations. +// +// The base schema in initSchema() uses CREATE ... IF NOT EXISTS and is safe +// to run against any database. For *changes* to existing tables (ADD COLUMN, +// new indexes, etc.) we need an ordered, idempotent migration runner. +// +// State is tracked under wiki_state["schema_version"] as a decimal integer +// string. A fresh database starts implicitly at version 0; each migration +// bumps to its declared `to` value inside a single transaction so a partial +// run never leaves the schema in a hybrid state. +// +// Append-only: never edit a migration after it has shipped. Add a new one. + +// stateKeySchemaVersion is the wiki_state key holding the current +// migration version as a decimal string. +const stateKeySchemaVersion = "schema_version" + +// migration describes one schema bump. `to` is the version this migration +// produces; the runner applies migrations in ascending `to` order whose +// `to` value is strictly greater than the current schema version. +type migration struct { + to int + name string + apply func(*Wiki) error +} + +// migrations is the canonical ordered list. Add new entries to the end. +var migrations = []migration{ + { + to: 1, + name: "links.kind column for image refs", + apply: func(w *Wiki) error { + // SQLite has no `ADD COLUMN IF NOT EXISTS`. We probe the + // schema via PRAGMA table_info and only ALTER when the + // column is missing — that way a database that was + // hand-patched, or one that survived a partial earlier + // run, doesn't error out. + has, err := columnExists(w, "links", "kind") + if err != nil { + return fmt.Errorf("probe links.kind: %w", err) + } + if has { + return nil + } + _, err = w.db.Exec(`ALTER TABLE links ADD COLUMN kind TEXT NOT NULL DEFAULT 'link'`) + return err + }, + }, +} + +// migrate brings the database up to the latest schema version. Idempotent — +// re-runs are no-ops once everything is applied. Each migration is bounded +// by writeStateKey on success, so a crash mid-pass leaves the version at +// the prior step and the next start picks up where we left off. +// +// On a fresh database (initSchema just ran with the current CREATE TABLE +// definitions), the migrations are all no-ops in practice — they probe for +// schema state before mutating. We still record the latest version so future +// migrations have an unambiguous "you started fresh at version N" baseline. +func (w *Wiki) migrate() error { + current, err := w.currentSchemaVersion() + if err != nil { + return err + } + + latest := 0 + if n := len(migrations); n > 0 { + latest = migrations[n-1].to + } + + for _, m := range migrations { + if m.to <= current { + continue + } + + slog.Info("applying migration", + slog.Int("from", current), + slog.Int("to", m.to), + slog.String("name", m.name), + ) + + if err := m.apply(w); err != nil { + return fmt.Errorf("migration %d (%s): %w", m.to, m.name, err) + } + if err := w.writeStateKey(context.Background(), stateKeySchemaVersion, + strconv.Itoa(m.to)); err != nil { + return fmt.Errorf("record schema_version=%d: %w", m.to, err) + } + current = m.to + } + + // Belt-and-suspenders: ensure the recorded version matches latest + // even when no migrations ran (e.g. fresh DB whose CREATE TABLE + // already includes everything). This gives future migrations a + // clean "I know exactly where you are" starting point. + if current < latest { + if err := w.writeStateKey(context.Background(), stateKeySchemaVersion, + strconv.Itoa(latest)); err != nil { + return fmt.Errorf("record schema_version=%d: %w", latest, err) + } + } + + return nil +} + +// currentSchemaVersion reads the persisted version, defaulting to 0 for +// fresh databases. +func (w *Wiki) currentSchemaVersion() (int, error) { + raw, ok := w.readStateKey(context.Background(), stateKeySchemaVersion) + if !ok { + return 0, nil + } + v, err := strconv.Atoi(raw) + if err != nil { + return 0, fmt.Errorf("parse schema_version %q: %w", raw, err) + } + return v, nil +} + +// columnExists reports whether the named column is present on the given +// table. Uses PRAGMA table_info, which lists one row per column. +func columnExists(w *Wiki, table, column string) (bool, error) { + rows, err := w.db.Query("PRAGMA table_info(" + table + ")") + if err != nil { + return false, err + } + defer rows.Close() + for rows.Next() { + var ( + cid int + name string + ctype string + notnull int + dfltValue any + pk int + ) + if err := rows.Scan(&cid, &name, &ctype, ¬null, &dfltValue, &pk); err != nil { + return false, err + } + if name == column { + return true, nil + } + } + return false, rows.Err() +} diff --git a/internal/wiki/pages.go b/internal/wiki/pages.go index 9e4de30..a9314db 100644 --- a/internal/wiki/pages.go +++ b/internal/wiki/pages.go @@ -398,14 +398,16 @@ type Link struct { Target string `json:"target"` } -// AllLinks returns every wikilink edge in the index. Used by the graph -// view to render reference edges without a per-page round-trip. +// AllLinks returns every wikilink edge in the index. Image references +// (kind='image') are excluded — those have a separate query path for the +// asset lifecycle code. Used by the graph view to render reference edges +// without a per-page round-trip. func (w *Wiki) AllLinks(ctx context.Context) ([]Link, error) { if err := ctx.Err(); err != nil { return nil, err } - rows, err := w.db.QueryContext(ctx, "SELECT source, target FROM links") + rows, err := w.db.QueryContext(ctx, "SELECT source, target FROM links WHERE kind = 'link'") if err != nil { return nil, err } @@ -501,7 +503,7 @@ func (w *Wiki) releaseLock(pagePath string) { // --- internal helpers --- func (w *Wiki) getLinks(ctx context.Context, pagePath string) ([]string, error) { - rows, err := w.db.QueryContext(ctx, "SELECT target FROM links WHERE source = ?", pagePath) + rows, err := w.db.QueryContext(ctx, "SELECT target FROM links WHERE source = ? AND kind = 'link'", pagePath) if err != nil { return nil, err } @@ -518,7 +520,7 @@ func (w *Wiki) getLinks(ctx context.Context, pagePath string) ([]string, error) } func (w *Wiki) getBacklinks(ctx context.Context, pagePath string) ([]string, error) { - rows, err := w.db.QueryContext(ctx, "SELECT source FROM links WHERE target = ?", pagePath) + rows, err := w.db.QueryContext(ctx, "SELECT source FROM links WHERE target = ? AND kind = 'link'", pagePath) if err != nil { return nil, err } diff --git a/internal/wiki/parse.go b/internal/wiki/parse.go index 9d04399..8973f85 100644 --- a/internal/wiki/parse.go +++ b/internal/wiki/parse.go @@ -4,6 +4,7 @@ import ( "strings" "github.com/yuin/goldmark" + "github.com/yuin/goldmark/ast" meta "github.com/yuin/goldmark-meta" "github.com/yuin/goldmark/parser" "github.com/yuin/goldmark/text" @@ -21,17 +22,24 @@ type parsedPage struct { body string frontmatter map[string]interface{} links []string + // images holds standard markdown image destinations (`![](path)`) + // in document order, deduplicated. These are tracked in the links + // table with kind='image' so lifecycle operations (delete/move/GC) + // can run as plain index queries. + images []string } -// parsePage extracts frontmatter, title, body text, and wikilinks from raw markdown. +// parsePage extracts frontmatter, title, body text, wikilinks, and image +// references from raw markdown. func parsePage(raw []byte) parsedPage { ctx := parser.NewContext() reader := text.NewReader(raw) - // Parse only to populate the meta context; goldmark-meta stores the - // YAML frontmatter map on ctx as a side effect. The AST itself is - // unused here — we extract the body via stripFrontmatter below. - md.Parser().Parse(reader, parser.WithContext(ctx)) + // Parse fully now: we use the AST to extract image destinations + // (standard markdown ![](path)). Wikilinks are still string-scanned + // from the post-frontmatter body since they're a non-standard + // extension and goldmark doesn't recognize them. + doc := md.Parser().Parse(reader, parser.WithContext(ctx)) fm := meta.Get(ctx) body := stripFrontmatter(raw) @@ -41,6 +49,7 @@ func parsePage(raw []byte) parsedPage { body: string(body), frontmatter: fm, links: extractWikilinks(body), + images: extractImages(doc, raw), } } @@ -116,3 +125,67 @@ func extractWikilinks(body []byte) []string { return links } + +// extractImages walks the markdown AST and returns the destinations of +// every `![](path)` image in document order, deduplicated. External URLs +// (anything with a scheme) are skipped — we only track wiki-local +// references because those are the ones the lifecycle code needs to +// reason about. Anchor-only refs (`#foo`) and empty destinations are +// also skipped. +// +// The `raw` parameter is the full file bytes (including frontmatter); +// goldmark uses byte offsets into this when reporting node positions, +// but ast.Image carries its destination directly so we don't need to +// re-slice the source. +func extractImages(doc ast.Node, _ []byte) []string { + seen := make(map[string]bool) + var images []string + + _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) { + if !entering { + return ast.WalkContinue, nil + } + img, ok := n.(*ast.Image) + if !ok { + return ast.WalkContinue, nil + } + dest := string(img.Destination) + if !isWikiLocalRef(dest) { + return ast.WalkContinue, nil + } + if !seen[dest] { + seen[dest] = true + images = append(images, dest) + } + return ast.WalkContinue, nil + }) + + return images +} + +// isWikiLocalRef reports whether a markdown image destination points at a +// wiki-local asset rather than an external resource. External resources +// (http://, https://, data:, mailto:, etc.) are intentionally ignored — +// the lifecycle code only manages files inside the wiki tree. +func isWikiLocalRef(dest string) bool { + if dest == "" { + return false + } + if strings.HasPrefix(dest, "#") { + return false + } + // Reject anything with a URL scheme (RFC 3986 scheme is + // ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) followed by ":"). + // A bare check for "://" misses `data:` and `mailto:`, hence the + // stricter scan: first colon before any slash means scheme. + for i := 0; i < len(dest); i++ { + c := dest[i] + if c == ':' { + return false + } + if c == '/' || c == '?' || c == '#' { + break + } + } + return true +} diff --git a/internal/wiki/wiki.go b/internal/wiki/wiki.go index 98f35de..d5e4f02 100644 --- a/internal/wiki/wiki.go +++ b/internal/wiki/wiki.go @@ -170,9 +170,15 @@ func (w *Wiki) initSchema() error { modified TEXT NOT NULL DEFAULT '' ); + -- The PRIMARY KEY is (source, target) for back-compat with databases + -- migrated from before the kind column existed (see migrate.go). In + -- practice (source, target) is unique even across kinds because + -- wikilink targets are page paths (no extension) while image targets + -- are filesystem paths with extensions — they don't collide. CREATE TABLE IF NOT EXISTS links ( source TEXT NOT NULL, target TEXT NOT NULL, + kind TEXT NOT NULL DEFAULT 'link', PRIMARY KEY (source, target) ); @@ -216,6 +222,10 @@ func (w *Wiki) initSchema() error { return fmt.Errorf("wiki_state schema: %w", err) } + if err := w.migrate(); err != nil { + return fmt.Errorf("migrate: %w", err) + } + // Clean up stale locks (older than 5 minutes) from crashed processes _, err := w.db.Exec("DELETE FROM page_locks WHERE acquired < ?", time.Now().Add(-5*time.Minute).UTC().Format(time.RFC3339)) From 6cce1673a92813e0cdd218b394d463acf8e53a37 Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sun, 24 May 2026 14:59:06 -0700 Subject: [PATCH 2/9] feat(wiki): asset CRUD with sidecar storage and lifecycle cascades UploadAsset writes binary content into a per-page sidecar directory (.assets/), sniffs magic bytes against the browser-renderable image set (PNG/JPEG/GIF/WebP/AVIF/SVG/BMP/ICO), auto-suffixes on name collision (case-insensitive), and caps size at Wiki.MaxAssetBytes (default 10MB). ReadAsset and StatAsset round-trip bytes + MIME, validating paths against the wiki root to prevent traversal. DeletePage now sweeps the sidecar against the link index after dropping the page's own rows: any asset with no remaining kind='image' referencer is deleted, and the sidecar dir is removed if empty. Shared assets (referenced from other pages) are kept in place. MovePage uses splitSidecarOnMove to decide per-asset what travels: exclusive assets are renamed alongside the page and in-body references are rewritten to the new sidecar path; shared assets stay in the original sidecar and the moved page's body keeps pointing at the old path (which still resolves). gcSidecarAssets cleans up afterward. Tests cover: upload + collision suffix (incl. case-insensitive), SVG acceptance, non-image rejection, size cap, filename sanitization, read/stat round-trip, traversal rejection, delete cascade for exclusive assets, delete preserving shared assets, move relocating exclusive assets with body rewrite, move leaving shared assets behind with body unchanged. --- internal/wiki/assets.go | 563 +++++++++++++++++++++++++++++++++++ internal/wiki/assets_test.go | 322 ++++++++++++++++++++ internal/wiki/pages.go | 89 +++++- internal/wiki/wiki.go | 4 + 4 files changed, 975 insertions(+), 3 deletions(-) create mode 100644 internal/wiki/assets.go create mode 100644 internal/wiki/assets_test.go diff --git a/internal/wiki/assets.go b/internal/wiki/assets.go new file mode 100644 index 0000000..a60a1ef --- /dev/null +++ b/internal/wiki/assets.go @@ -0,0 +1,563 @@ +package wiki + +import ( + "bytes" + "context" + "errors" + "fmt" + "log/slog" + "net/http" + "os" + "path" + "path/filepath" + "strings" +) + +// Asset support. +// +// Images and other binary references embedded in markdown live on disk +// in a per-page "sidecar" directory: a page at `foo/bar` keeps its +// assets in `foo/bar.assets/`. The asset path stored in markdown +// (`![](foo/bar.assets/diagram.png)`) is the same string used as the +// link table's `target` for kind='image' rows, which lets every +// lifecycle question be answered by a plain index query. +// +// This file holds the wiki-internal asset CRUD. The MCP and HTTP +// layers wrap these methods; they don't reach into the filesystem +// directly. + +const ( + // assetsSuffix is the dirname suffix appended to a page path to + // form its sidecar asset directory. The "." prefix matches how + // agents and humans naturally read the references — "this page's + // assets" — and keeps the directory next to the page on disk. + assetsSuffix = ".assets" + + // defaultMaxAssetBytes is the per-upload size cap used when the + // wiki hasn't been configured otherwise. 10 MB matches the design + // doc default. Operators can raise this via Wiki.MaxAssetBytes if + // they're hosting larger illustrations. + defaultMaxAssetBytes int64 = 10 * 1024 * 1024 +) + +// ErrAssetTooLarge is returned by UploadAsset when content exceeds +// the configured size cap. +var ErrAssetTooLarge = errors.New("asset exceeds size cap") + +// ErrUnsupportedAssetType is returned by UploadAsset when the content's +// detected MIME type isn't one of the browser-renderable image formats. +var ErrUnsupportedAssetType = errors.New("unsupported asset type") + +// ErrAssetNotFound is returned by ReadAsset and StatAsset when the +// requested asset doesn't exist on disk. +var ErrAssetNotFound = errors.New("asset not found") + +// AssetInfo describes an asset without including its body. Returned by +// StatAsset and used to populate the metadata-only mode of the page +// read tools. +type AssetInfo struct { + // Path is the asset's wiki-relative path as it appears in markdown + // references and in the links table (kind='image' target). + Path string `json:"path"` + // SizeBytes is the on-disk size of the asset file. + SizeBytes int64 `json:"size_bytes"` + // MIME is the detected content type. Always populated when the + // file exists and can be sniffed. + MIME string `json:"mime"` +} + +// UploadAsset writes binary content into the sidecar directory of the +// given page, returning the asset's wiki-relative path on success. The +// returned path is what the caller should embed in markdown: +// +// uploaded, _ := w.UploadAsset(ctx, "projects/mind-map", "diagram.png", bytes) +// // uploaded == "projects/mind-map.assets/diagram.png" +// body := "![diagram](" + uploaded + ")" +// +// Behavior: +// +// - The sidecar directory is created if it doesn't exist. +// - If a file with the same name already exists, UploadAsset auto- +// suffixes the basename ("diagram.png" → "diagram-1.png") and keeps +// trying until it finds a free slot. Agents don't have to probe. +// - Content is sniffed via http.DetectContentType plus an SVG check; +// anything outside the browser-renderable image set is rejected +// with ErrUnsupportedAssetType. +// - Size is bounded by Wiki.MaxAssetBytes (default 10 MB), rejecting +// oversize uploads with ErrAssetTooLarge. +// +// UploadAsset does NOT touch the markdown of any page. It writes the +// file and returns the path; the caller updates the page via +// update_page / edit_page so the reference is captured by the index +// on the next indexPage call. +func (w *Wiki) UploadAsset(ctx context.Context, page, name string, content []byte) (string, error) { + if err := ctx.Err(); err != nil { + return "", err + } + + page, err := normalizePagePath(page) + if err != nil { + return "", fmt.Errorf("page: %w", err) + } + + cleanName, err := sanitizeAssetFilename(name) + if err != nil { + return "", err + } + + maxBytes := w.MaxAssetBytes + if maxBytes <= 0 { + maxBytes = defaultMaxAssetBytes + } + if int64(len(content)) > maxBytes { + return "", fmt.Errorf("%w: %d bytes > %d", ErrAssetTooLarge, len(content), maxBytes) + } + + mime, ok := detectImageMIME(content) + if !ok { + return "", fmt.Errorf("%w: detected %q", ErrUnsupportedAssetType, mime) + } + + sidecarRel := page + assetsSuffix + sidecarAbs := filepath.Join(w.root, sidecarRel) + if err := os.MkdirAll(sidecarAbs, 0o755); err != nil { + return "", fmt.Errorf("create sidecar: %w", err) + } + + finalName, err := resolveAssetCollision(sidecarAbs, cleanName) + if err != nil { + return "", err + } + + absPath := filepath.Join(sidecarAbs, finalName) + if err := os.WriteFile(absPath, content, 0o644); err != nil { + return "", fmt.Errorf("write asset: %w", err) + } + + relPath := path.Join(sidecarRel, finalName) + slog.Info("asset uploaded", + slog.String("page", page), + slog.String("path", relPath), + slog.Int("bytes", len(content)), + slog.String("mime", mime), + ) + return relPath, nil +} + +// ReadAsset returns the bytes and detected MIME type for an asset path. +// The path must be wiki-relative (as stored in markdown references); it +// is validated against the wiki root to prevent traversal. +func (w *Wiki) ReadAsset(ctx context.Context, assetPath string) ([]byte, string, error) { + if err := ctx.Err(); err != nil { + return nil, "", err + } + + abs, err := w.resolveAssetPath(assetPath) + if err != nil { + return nil, "", err + } + + data, err := os.ReadFile(abs) + if err != nil { + if os.IsNotExist(err) { + return nil, "", fmt.Errorf("%w: %s", ErrAssetNotFound, assetPath) + } + return nil, "", err + } + + mime, _ := detectImageMIME(data) + if mime == "" { + // Fall back to a generic detect so we still serve something + // useful for assets that pre-date the sniff list (e.g. a + // human-uploaded format we haven't enumerated). The static + // handler can still return the bytes; only uploads are gated. + mime = http.DetectContentType(data) + } + return data, mime, nil +} + +// StatAsset returns metadata for an asset without reading its full body. +// Used by the metadata-only mode of the page read tools and by future +// listing/GC operations. +func (w *Wiki) StatAsset(ctx context.Context, assetPath string) (*AssetInfo, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + abs, err := w.resolveAssetPath(assetPath) + if err != nil { + return nil, err + } + + info, err := os.Stat(abs) + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("%w: %s", ErrAssetNotFound, assetPath) + } + return nil, err + } + + // We need the head of the file to sniff a MIME, but full read just + // for stats would be wasteful for large illustrations. 512 bytes is + // the limit http.DetectContentType actually inspects. + head := make([]byte, 512) + f, err := os.Open(abs) + if err != nil { + return nil, err + } + defer f.Close() + n, _ := f.Read(head) + mime, _ := detectImageMIME(head[:n]) + if mime == "" { + mime = http.DetectContentType(head[:n]) + } + + return &AssetInfo{ + Path: filepath.ToSlash(assetPath), + SizeBytes: info.Size(), + MIME: mime, + }, nil +} + +// resolveAssetPath validates a wiki-relative asset path and returns its +// absolute filesystem path. Rejects traversal attempts (..) and any +// path that doesn't resolve under the wiki root. +// +// Unlike normalizePagePath, asset paths keep their extension and don't +// have a trailing-slash normalization to do — they're filesystem paths, +// not page paths. +func (w *Wiki) resolveAssetPath(assetPath string) (string, error) { + if assetPath == "" { + return "", fmt.Errorf("asset path is empty") + } + // Normalize separators and reject leading slashes; asset paths are + // always wiki-root-relative. + p := strings.ReplaceAll(assetPath, `\`, "/") + p = strings.TrimPrefix(p, "/") + cleaned := path.Clean(p) + if cleaned == "." || cleaned == "" { + return "", fmt.Errorf("invalid asset path: %q", assetPath) + } + if cleaned == ".." || strings.HasPrefix(cleaned, "../") { + return "", fmt.Errorf("asset path escapes wiki root: %q", assetPath) + } + + abs := filepath.Join(w.root, filepath.FromSlash(cleaned)) + // Final guard: filepath.Join can't undo a cleaned ".." check above, + // but defense in depth: resolve symlinks and verify containment. + absClean, err := filepath.Abs(abs) + if err != nil { + return "", err + } + if !strings.HasPrefix(absClean+string(filepath.Separator), w.root+string(filepath.Separator)) && absClean != w.root { + return "", fmt.Errorf("asset path escapes wiki root: %q", assetPath) + } + return absClean, nil +} + +// sanitizeAssetFilename strips path components and forbidden characters +// from a user-supplied filename. Only the basename survives — agents +// can't smuggle "../" or nested directory paths in via the name field. +func sanitizeAssetFilename(name string) (string, error) { + if name == "" { + return "", fmt.Errorf("asset name is empty") + } + // Collapse any path-y syntax to just the final component. + name = strings.ReplaceAll(name, `\`, "/") + name = path.Base(name) + if name == "" || name == "." || name == ".." || name == "/" { + return "", fmt.Errorf("invalid asset name: %q", name) + } + // SQLite stores the asset path as part of the links table primary + // key tuple; null bytes would torch sqlite tooling. Reject them. + if strings.ContainsRune(name, 0) { + return "", fmt.Errorf("asset name contains NUL") + } + return name, nil +} + +// resolveAssetCollision returns a free filename inside dir, starting +// from desired and auto-suffixing on collision: "a.png" → "a-1.png" → +// "a-2.png" ... Comparisons are case-insensitive so we don't end up +// with "Diagram.png" and "diagram.png" coexisting on case-sensitive +// filesystems and confusing sync to a case-insensitive remote. +func resolveAssetCollision(dir, desired string) (string, error) { + existing, err := caseInsensitiveDirSet(dir) + if err != nil { + return "", err + } + lower := strings.ToLower(desired) + if _, taken := existing[lower]; !taken { + return desired, nil + } + + ext := filepath.Ext(desired) + stem := strings.TrimSuffix(desired, ext) + for i := 1; i < 10_000; i++ { + candidate := fmt.Sprintf("%s-%d%s", stem, i, ext) + if _, taken := existing[strings.ToLower(candidate)]; !taken { + return candidate, nil + } + } + return "", fmt.Errorf("could not find a free filename after 10000 attempts: %s", desired) +} + +// caseInsensitiveDirSet returns the lowercased names of files in dir. +// Missing dirs return an empty set without an error — that's the +// "no collisions, first upload" case. +func caseInsensitiveDirSet(dir string) (map[string]struct{}, error) { + entries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return map[string]struct{}{}, nil + } + return nil, err + } + set := make(map[string]struct{}, len(entries)) + for _, e := range entries { + set[strings.ToLower(e.Name())] = struct{}{} + } + return set, nil +} + +// detectImageMIME inspects the leading bytes of a file and returns the +// MIME type if it's a browser-renderable image format, or ("", false) +// otherwise. SVG gets special-cased because http.DetectContentType +// reports it as a generic XML type; we look for the = 12 && bytes.Equal(content[4:8], []byte("ftyp")) { + brand := string(content[8:12]) + switch brand { + case "avif", "avis": + return "image/avif", true + } + } + + // SVG sniff: skip leading whitespace and optional XML decl/DOCTYPE, + // then look for the element, allowing leading whitespace, XML declaration, and +// DOCTYPE/comments. It does NOT validate the XML; that's left to the +// downstream renderer. +func looksLikeSVG(content []byte) bool { + s := bytes.TrimLeft(content, " \t\r\n\xef\xbb\xbf") + // Strip leading declaration if present. + if bytes.HasPrefix(s, []byte("")) + if end < 0 { + return false + } + s = bytes.TrimLeft(s[end+2:], " \t\r\n") + } + // Strip leading comments and DOCTYPE/whitespace until we hit a + // real element open. Bounded loop so a pathological input can't + // spin us. + for i := 0; i < 8; i++ { + s = bytes.TrimLeft(s, " \t\r\n") + switch { + case bytes.HasPrefix(s, []byte("")) + if end < 0 { + return false + } + s = s[end+3:] + case bytes.HasPrefix(s, []byte("') + if end < 0 { + return false + } + s = s[end+1:] + default: + i = 8 // break the for + } + } + s = bytes.TrimLeft(s, " \t\r\n") + if !bytes.HasPrefix(s, []byte("` so we don't + // accept `` or `` as SVG. + if len(s) <= 4 { + return false + } + next := s[4] + return next == ' ' || next == '\t' || next == '\r' || next == '\n' || next == '>' || next == '/' +} + +// gcSidecarAssets removes files under .assets/ that have no +// row in the link index. Called from DeletePage (after the page's +// own rows are deleted from `links`) and from MovePage (to clean up +// any orphans the move left behind). The sidecar dir itself is +// removed if empty after the sweep so the wiki tree stays tidy. +// +// Files referenced by OTHER pages (kind='image' rows with a different +// source) are kept in place — the design intentionally has no shared +// asset pool, so the file lives in its original sidecar even when +// shared. The markdown path in the referencing page still resolves. +func (w *Wiki) gcSidecarAssets(ctx context.Context, page string) error { + sidecarRel := page + assetsSuffix + sidecarAbs := filepath.Join(w.root, sidecarRel) + + entries, err := os.ReadDir(sidecarAbs) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + + for _, entry := range entries { + if entry.IsDir() { + // We don't support nested sidecars; skip any + // subdirectory rather than recursing or deleting it. + continue + } + assetRel := path.Join(sidecarRel, entry.Name()) + var n int + if err := w.db.QueryRowContext(ctx, + "SELECT COUNT(*) FROM links WHERE target = ? AND kind = 'image'", + assetRel, + ).Scan(&n); err != nil { + return fmt.Errorf("query asset refs %q: %w", assetRel, err) + } + if n > 0 { + continue + } + assetAbs := filepath.Join(sidecarAbs, entry.Name()) + if err := os.Remove(assetAbs); err != nil && !os.IsNotExist(err) { + slog.Warn("sidecar asset remove failed", + slog.String("asset", assetRel), + slog.Any("error", err), + ) + } + } + + // If the sidecar is now empty, drop the directory. os.Remove + // fails on non-empty dirs, which is exactly the "still has shared + // or human-added files" case we want to leave alone. + if err := os.Remove(sidecarAbs); err != nil && !os.IsNotExist(err) { + // Non-empty or permission error — not fatal, just log. + slog.Debug("sidecar dir not removed (likely non-empty)", + slog.String("dir", sidecarRel), + slog.Any("error", err), + ) + } + return nil +} + +// splitSidecarOnMove rewrites the image references inside a page's +// markdown when the page is moved from→to, and decides which sidecar +// files travel with the page vs. stay behind for other referencers. +// +// Returns the rewritten body. The caller is responsible for writing +// the file at its new path and re-indexing. +// +// Design (option (a) from the image-support discussion): use the link +// index to find which assets are exclusive to the moving page. Move +// those into the new sidecar (rewriting in-body paths to match); +// leave shared assets in the old sidecar (keeping their original path +// in the in-body markdown, since other pages still reference them at +// that path). After the move, gcSidecarAssets on the old sidecar +// cleans up: the dir is gone if everything moved, or it survives +// holding only the shared files. +// +// `oldImages` is the set of image targets the page referenced before +// the move (queried before we delete the source page's link rows). +// We need it as input because by the time we rewrite the body those +// rows may already be gone, and rebuilding it from the new markdown +// is the wrong direction — we need pre-move state. +func (w *Wiki) splitSidecarOnMove(ctx context.Context, from, to string, body []byte, oldImages []string) ([]byte, error) { + if len(oldImages) == 0 { + return body, nil + } + + oldSidecarPrefix := from + assetsSuffix + "/" + newSidecarRel := to + assetsSuffix + newSidecarAbs := filepath.Join(w.root, newSidecarRel) + + rewritten := body + for _, target := range oldImages { + // Only touch assets that lived in the page's own sidecar. + // Images referencing some OTHER page's sidecar (cross- + // referenced images) keep their path either way: the file + // stays where it was and the path is still valid post-move. + if !strings.HasPrefix(target, oldSidecarPrefix) { + continue + } + + // Is anyone else (besides `from`) referencing this asset? + var n int + if err := w.db.QueryRowContext(ctx, + "SELECT COUNT(*) FROM links WHERE target = ? AND kind = 'image' AND source != ?", + target, from, + ).Scan(&n); err != nil { + return nil, fmt.Errorf("query shared %q: %w", target, err) + } + if n > 0 { + // Shared: leave the file in place, leave the + // markdown path alone. Other pages still resolve. + continue + } + + // Exclusive: move the file to the new sidecar and rewrite + // the in-body reference. + basename := path.Base(target) + oldAbs := filepath.Join(w.root, filepath.FromSlash(target)) + newRel := path.Join(newSidecarRel, basename) + newAbs := filepath.Join(newSidecarAbs, basename) + + if err := os.MkdirAll(newSidecarAbs, 0o755); err != nil { + return nil, fmt.Errorf("create new sidecar: %w", err) + } + // Best-effort rename; if the source vanished (e.g. someone + // hand-deleted it while the page still referenced it) we + // still want the body rewrite to happen so the index is + // consistent. The next reindex/Stat will surface the + // missing-file condition if needed. + if err := os.Rename(oldAbs, newAbs); err != nil && !os.IsNotExist(err) { + return nil, fmt.Errorf("move asset %q: %w", target, err) + } + + rewritten = bytes.ReplaceAll(rewritten, []byte(target), []byte(newRel)) + } + + return rewritten, nil +} diff --git a/internal/wiki/assets_test.go b/internal/wiki/assets_test.go new file mode 100644 index 0000000..5db8da2 --- /dev/null +++ b/internal/wiki/assets_test.go @@ -0,0 +1,322 @@ +package wiki + +import ( + "bytes" + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +// --- test fixtures --- + +// onePixelPNG is the smallest possible valid PNG: 1x1 transparent. +// Generated once by hand; the magic bytes and IDAT are what http.DetectContentType +// inspects, so any browser-renderable PNG works in tests. +var onePixelPNG = []byte{ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, // signature + 0x00, 0x00, 0x00, 0x0d, // IHDR length + 0x49, 0x48, 0x44, 0x52, // "IHDR" + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, // 1x1 + 0x08, 0x06, 0x00, 0x00, 0x00, // bit depth + color + 0x1f, 0x15, 0xc4, 0x89, // CRC + 0x00, 0x00, 0x00, 0x0d, // IDAT length + 0x49, 0x44, 0x41, 0x54, // "IDAT" + 0x78, 0x9c, 0x62, 0x00, 0x01, 0x00, 0x00, 0x05, + 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, // data + CRC + 0x00, 0x00, 0x00, 0x00, // IEND length + 0x49, 0x45, 0x4e, 0x44, // "IEND" + 0xae, 0x42, 0x60, 0x82, // CRC +} + +const tinySVG = `` + +// --- UploadAsset --- + +func TestUploadAssetCreatesSidecar(t *testing.T) { + w, dir := testWiki(t) + ctx := context.Background() + + got, err := w.UploadAsset(ctx, "projects/mind-map", "diagram.png", onePixelPNG) + if err != nil { + t.Fatalf("UploadAsset: %v", err) + } + want := "projects/mind-map.assets/diagram.png" + if got != want { + t.Errorf("returned path = %q, want %q", got, want) + } + abs := filepath.Join(dir, filepath.FromSlash(got)) + if _, err := os.Stat(abs); err != nil { + t.Errorf("asset file missing on disk: %v", err) + } +} + +func TestUploadAssetCollisionSuffix(t *testing.T) { + w, _ := testWiki(t) + ctx := context.Background() + + first, err := w.UploadAsset(ctx, "projects/mind-map", "shot.png", onePixelPNG) + if err != nil { + t.Fatalf("first upload: %v", err) + } + second, err := w.UploadAsset(ctx, "projects/mind-map", "shot.png", onePixelPNG) + if err != nil { + t.Fatalf("second upload: %v", err) + } + if first == second { + t.Fatalf("expected collision suffix, both uploads returned %q", first) + } + if !strings.HasSuffix(second, "shot-1.png") { + t.Errorf("second upload path %q does not end with shot-1.png", second) + } + + // Case-insensitive collision: SHOT.PNG should also bump. + third, err := w.UploadAsset(ctx, "projects/mind-map", "SHOT.PNG", onePixelPNG) + if err != nil { + t.Fatalf("third upload: %v", err) + } + if strings.EqualFold(filepath.Base(third), "shot.png") || strings.EqualFold(filepath.Base(third), "shot-1.png") { + t.Errorf("third upload %q collided case-insensitively", third) + } +} + +func TestUploadAssetRejectsNonImage(t *testing.T) { + w, _ := testWiki(t) + ctx := context.Background() + + _, err := w.UploadAsset(ctx, "projects/mind-map", "evil.exe", + []byte("MZ\x00\x00not actually a PE but http.DetectContentType picks it up")) + if err == nil { + t.Fatal("UploadAsset accepted non-image content") + } + if !errors.Is(err, ErrUnsupportedAssetType) { + t.Errorf("err = %v, want ErrUnsupportedAssetType", err) + } +} + +func TestUploadAssetAcceptsSVG(t *testing.T) { + w, _ := testWiki(t) + ctx := context.Background() + + got, err := w.UploadAsset(ctx, "projects/mind-map", "vector.svg", []byte(tinySVG)) + if err != nil { + t.Fatalf("UploadAsset svg: %v", err) + } + if !strings.HasSuffix(got, ".svg") { + t.Errorf("returned path = %q, want .svg suffix", got) + } +} + +func TestUploadAssetSizeCap(t *testing.T) { + w, _ := testWiki(t) + w.MaxAssetBytes = 64 // tiny cap for the test + ctx := context.Background() + + big := bytes.Repeat([]byte{0}, 256) // not even an image, but we want size to fail first + _, err := w.UploadAsset(ctx, "projects/mind-map", "big.png", big) + if err == nil || !errors.Is(err, ErrAssetTooLarge) { + t.Errorf("err = %v, want ErrAssetTooLarge", err) + } +} + +func TestUploadAssetSanitizesFilename(t *testing.T) { + w, _ := testWiki(t) + ctx := context.Background() + + got, err := w.UploadAsset(ctx, "projects/mind-map", "../../evil.png", onePixelPNG) + if err != nil { + t.Fatalf("UploadAsset: %v", err) + } + if strings.Contains(got, "..") { + t.Errorf("returned path contains traversal: %q", got) + } + if filepath.Base(got) != "evil.png" { + t.Errorf("expected basename evil.png, got %q", got) + } +} + +// --- ReadAsset / StatAsset --- + +func TestReadAssetRoundTrip(t *testing.T) { + w, _ := testWiki(t) + ctx := context.Background() + + upPath, err := w.UploadAsset(ctx, "projects/mind-map", "d.png", onePixelPNG) + if err != nil { + t.Fatal(err) + } + bs, mime, err := w.ReadAsset(ctx, upPath) + if err != nil { + t.Fatalf("ReadAsset: %v", err) + } + if !bytes.Equal(bs, onePixelPNG) { + t.Error("bytes differ from uploaded content") + } + if mime != "image/png" { + t.Errorf("mime = %q, want image/png", mime) + } +} + +func TestReadAssetRejectsTraversal(t *testing.T) { + w, _ := testWiki(t) + ctx := context.Background() + + _, _, err := w.ReadAsset(ctx, "../../../etc/passwd") + if err == nil { + t.Fatal("ReadAsset accepted traversal path") + } +} + +func TestStatAsset(t *testing.T) { + w, _ := testWiki(t) + ctx := context.Background() + + upPath, _ := w.UploadAsset(ctx, "projects/mind-map", "d.png", onePixelPNG) + info, err := w.StatAsset(ctx, upPath) + if err != nil { + t.Fatalf("StatAsset: %v", err) + } + if info.SizeBytes != int64(len(onePixelPNG)) { + t.Errorf("SizeBytes = %d, want %d", info.SizeBytes, len(onePixelPNG)) + } + if info.MIME != "image/png" { + t.Errorf("MIME = %q, want image/png", info.MIME) + } + if info.Path != upPath { + t.Errorf("Path = %q, want %q", info.Path, upPath) + } +} + +// --- DeletePage cascade --- + +func TestDeletePageCascadesUnreferencedAssets(t *testing.T) { + w, dir := testWiki(t) + ctx := context.Background() + + uploaded, err := w.UploadAsset(ctx, "projects/mind-map", "d.png", onePixelPNG) + if err != nil { + t.Fatal(err) + } + // Update the page so the index knows it references the asset. + if err := w.UpdatePage(ctx, "projects/mind-map", "# mm\n\n![d]("+uploaded+")\n"); err != nil { + t.Fatal(err) + } + + if err := w.DeletePage(ctx, "projects/mind-map"); err != nil { + t.Fatalf("DeletePage: %v", err) + } + + // Asset should be gone. + abs := filepath.Join(dir, filepath.FromSlash(uploaded)) + if _, err := os.Stat(abs); !os.IsNotExist(err) { + t.Errorf("asset still exists after delete: %v", err) + } + // Sidecar dir should be gone too. + sidecar := filepath.Join(dir, "projects/mind-map.assets") + if _, err := os.Stat(sidecar); !os.IsNotExist(err) { + t.Errorf("sidecar dir still exists after delete: %v", err) + } +} + +func TestDeletePageKeepsSharedAssets(t *testing.T) { + w, dir := testWiki(t) + ctx := context.Background() + + uploaded, err := w.UploadAsset(ctx, "projects/mind-map", "d.png", onePixelPNG) + if err != nil { + t.Fatal(err) + } + if err := w.UpdatePage(ctx, "projects/mind-map", "# mm\n\n![d]("+uploaded+")\n"); err != nil { + t.Fatal(err) + } + // people/alice also references the same asset. + if err := w.UpdatePage(ctx, "people/alice", "# Alice\n\n![d]("+uploaded+")\n"); err != nil { + t.Fatal(err) + } + + if err := w.DeletePage(ctx, "projects/mind-map"); err != nil { + t.Fatalf("DeletePage: %v", err) + } + + abs := filepath.Join(dir, filepath.FromSlash(uploaded)) + if _, err := os.Stat(abs); err != nil { + t.Errorf("shared asset removed despite external referencer: %v", err) + } +} + +// --- MovePage with sidecar --- + +func TestMovePageRelocatesExclusiveAssets(t *testing.T) { + w, dir := testWiki(t) + ctx := context.Background() + + uploaded, _ := w.UploadAsset(ctx, "projects/mind-map", "d.png", onePixelPNG) + if err := w.UpdatePage(ctx, "projects/mind-map", "# mm\n\n![d]("+uploaded+")\n"); err != nil { + t.Fatal(err) + } + + if err := w.MovePage(ctx, "projects/mind-map", "projects/mm2", MoveOptions{}); err != nil { + t.Fatalf("MovePage: %v", err) + } + + // Old asset should be gone, new one in place at the new sidecar. + if _, err := os.Stat(filepath.Join(dir, "projects/mind-map.assets/d.png")); !os.IsNotExist(err) { + t.Errorf("old asset file still present: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "projects/mm2.assets/d.png")); err != nil { + t.Errorf("new asset file missing: %v", err) + } + + // Page body should have the rewritten reference. + p, err := w.GetPage(ctx, "projects/mm2") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(p.Body, "projects/mm2.assets/d.png") { + t.Errorf("body not rewritten: %q", p.Body) + } + if strings.Contains(p.Body, "projects/mind-map.assets/") { + t.Errorf("old sidecar path still in body: %q", p.Body) + } +} + +func TestMovePageLeavesSharedAssetsBehind(t *testing.T) { + w, dir := testWiki(t) + ctx := context.Background() + + uploaded, _ := w.UploadAsset(ctx, "projects/mind-map", "shared.png", onePixelPNG) + // Both pages reference the asset that lives in projects/mind-map.assets/. + if err := w.UpdatePage(ctx, "projects/mind-map", "# mm\n\n![s]("+uploaded+")\n"); err != nil { + t.Fatal(err) + } + if err := w.UpdatePage(ctx, "people/alice", "# Alice\n\n![s]("+uploaded+")\n"); err != nil { + t.Fatal(err) + } + + if err := w.MovePage(ctx, "projects/mind-map", "projects/mm2", MoveOptions{}); err != nil { + t.Fatalf("MovePage: %v", err) + } + + // The shared file must stay in its original sidecar. + if _, err := os.Stat(filepath.Join(dir, "projects/mind-map.assets/shared.png")); err != nil { + t.Errorf("shared asset removed from original sidecar: %v", err) + } + + // The moved page's body should keep referencing the old sidecar + // path, because the file still lives there. + p, err := w.GetPage(ctx, "projects/mm2") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(p.Body, "projects/mind-map.assets/shared.png") { + t.Errorf("moved page body lost reference to shared asset: %q", p.Body) + } + + // And alice still resolves. + a, _ := w.GetPage(ctx, "people/alice") + if !strings.Contains(a.Body, "projects/mind-map.assets/shared.png") { + t.Errorf("alice lost her reference: %q", a.Body) + } +} diff --git a/internal/wiki/pages.go b/internal/wiki/pages.go index a9314db..6ce7c5b 100644 --- a/internal/wiki/pages.go +++ b/internal/wiki/pages.go @@ -194,7 +194,13 @@ func (w *Wiki) UpdatePage(ctx context.Context, pagePath string, content string) return nil } -// DeletePage removes a page from the filesystem and index. +// DeletePage removes a page from the filesystem and index. Sidecar +// assets under .assets/ are GC'd against the link index: any file +// that no longer has a row in `links` (after the page's own rows are +// removed) is deleted. Cross-referenced assets — those another page +// still embeds — are kept; the design intentionally has no shared-pool +// concept, so the file lives where it was originally uploaded even when +// referenced from elsewhere. func (w *Wiki) DeletePage(ctx context.Context, pagePath string) error { if err := ctx.Err(); err != nil { return err @@ -223,6 +229,21 @@ func (w *Wiki) DeletePage(ctx context.Context, pagePath string) error { // The page is gone; leaving it in recents would point the agent // at a 404. Drop the entry rather than promote it. w.recents.remove(pagePath) + + // Sweep the sidecar after dropping this page's link rows. Any + // asset still referenced by another page (kind='image' row with + // a different source) is kept; everything else is removed. The + // sidecar dir itself is removed if it ends up empty. + if err := w.gcSidecarAssets(ctx, pagePath); err != nil { + // Non-fatal: the page itself is gone and the index is + // consistent. A future gc_assets pass (Slice 2 follow-up) + // can mop up. + slog.Warn("sidecar gc failed", + slog.String("page", pagePath), + slog.Any("error", err), + ) + } + return nil } @@ -317,8 +338,37 @@ func (w *Wiki) MovePage(ctx context.Context, fromPath, toPath string, opts MoveO return fmt.Errorf("create destination directory: %w", err) } - if err := os.Rename(fromAbs, toAbs); err != nil { - return fmt.Errorf("rename page file: %w", err) + // Sidecar handling: before we destroy the source page's link rows, + // snapshot its image references so splitSidecarOnMove can decide + // which assets travel with the page and which stay behind for + // other referencers. We read the body once here and pass it + // through to be rewritten in place — that way the file ends up at + // the new location with image paths already pointing at the new + // sidecar (for exclusive assets) and unchanged for shared ones. + oldImages, err := w.imageRefsFor(ctx, from) + if err != nil { + return fmt.Errorf("snapshot image refs: %w", err) + } + body, err := os.ReadFile(fromAbs) + if err != nil { + return fmt.Errorf("read source body: %w", err) + } + rewritten, err := w.splitSidecarOnMove(ctx, from, to, body, oldImages) + if err != nil { + return fmt.Errorf("split sidecar on move: %w", err) + } + + if err := os.WriteFile(toAbs, rewritten, 0o644); err != nil { + return fmt.Errorf("write destination: %w", err) + } + if err := os.Remove(fromAbs); err != nil && !os.IsNotExist(err) { + // Best-effort: the destination is already in place and the + // indexer will reconcile. Leaving the source on disk would + // confuse the next reindex into thinking we have a + // duplicate, but a follow-up Reindex picks the newer mtime. + slog.Warn("move: remove source after copy failed", + slog.String("from", from), slog.Any("error", err), + ) } if err := w.removePageIndex(ctx, from); err != nil { @@ -329,6 +379,15 @@ func (w *Wiki) MovePage(ctx context.Context, fromPath, toPath string, opts MoveO return fmt.Errorf("index new page: %w", err) } + // GC any remaining files in the old sidecar. If everything moved, + // the dir is gone; if shared assets remained, the dir survives + // with just them. + if err := w.gcSidecarAssets(ctx, from); err != nil { + slog.Warn("move: sidecar gc failed", + slog.String("from", from), slog.Any("error", err), + ) + } + // Treat a move as one continuous "active use" rather than dropping // the old name and freshly inserting the new one. See recentsLRU.rename. w.recents.rename(from, to) @@ -536,6 +595,30 @@ func (w *Wiki) getBacklinks(ctx context.Context, pagePath string) ([]string, err return backlinks, nil } +// imageRefsFor returns the asset paths a page currently references in +// the index (kind='image' rows where source = pagePath). Order is +// unspecified; callers that care should sort. Used by MovePage to +// snapshot pre-move state before the source's link rows are deleted. +func (w *Wiki) imageRefsFor(ctx context.Context, pagePath string) ([]string, error) { + rows, err := w.db.QueryContext(ctx, + "SELECT target FROM links WHERE source = ? AND kind = 'image'", + pagePath, + ) + if err != nil { + return nil, err + } + defer rows.Close() + + var images []string + for rows.Next() { + var t string + if err := rows.Scan(&t); err == nil { + images = append(images, t) + } + } + return images, nil +} + func (w *Wiki) topLevelDirs() []string { entries, err := os.ReadDir(w.root) if err != nil { diff --git a/internal/wiki/wiki.go b/internal/wiki/wiki.go index d5e4f02..dd20695 100644 --- a/internal/wiki/wiki.go +++ b/internal/wiki/wiki.go @@ -72,6 +72,10 @@ type Wiki struct { // against an already-closed DB and logs a spurious warning. closeOnce sync.Once closeErr error + // MaxAssetBytes caps individual asset uploads via UploadAsset. + // Zero (the default) means "use defaultMaxAssetBytes" (10 MB). + // Set this from the CLI / config layer to override per deployment. + MaxAssetBytes int64 } // Open opens (or creates) a wiki rooted at the given directory. From 043dca2db08e2e213a4bdda8df2e33b0dd5a00cc Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sun, 24 May 2026 15:04:09 -0700 Subject: [PATCH 3/9] feat(mcp): upload_image, download_image, and image read flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three additions to the MCP tool surface: - upload_image: agent uploads base64 image bytes to a page's sidecar, receives the markdown-ready path + URL + size + mime. Embedding the ![]() reference is the agent's job (via update_page / edit_page); the design defers the convenience insert_image tool until v2 to keep tool responsibilities clean. - download_image: returns mcp.ImageContent so vision-capable agents see the asset inline. - get_page gains include_images and include_image_metadata flags. Default off — image work is opt-in to keep token cost predictable. include_images attaches the actual bytes as MCP ImageContent blocks after the text payload; include_image_metadata embeds {path,size, mime} entries in the JSON body without the bytes. Both can be globally overridden by Server.SetForceImagesOff for token- constrained deployments; when forced, the response includes images_forced_off=true so callers don't reason as if they got what they asked for. Wiki.ImageRefsForPage is exposed (it was already used internally by MovePage as imageRefsFor) so the MCP layer can enumerate a page's image references without re-parsing the body. Tests cover: upload happy path, download returning ImageContent, include_image_metadata embedding entries, include_images returning both text + image blocks, non-image rejection (via result.IsError), force-off override. --- internal/mcp/images.go | 281 ++++++++++++++++++++++++++++++++++++ internal/mcp/images_test.go | 249 ++++++++++++++++++++++++++++++++ internal/mcp/server.go | 31 ++-- internal/wiki/pages.go | 18 +++ 4 files changed, 567 insertions(+), 12 deletions(-) create mode 100644 internal/mcp/images.go create mode 100644 internal/mcp/images_test.go diff --git a/internal/mcp/images.go b/internal/mcp/images.go new file mode 100644 index 0000000..f1658a7 --- /dev/null +++ b/internal/mcp/images.go @@ -0,0 +1,281 @@ +// MCP tool handlers for image support. +// +// Three new tools surface the wiki's asset layer to agents: +// +// - upload_image: write binary content into the page's sidecar and +// return a markdown-ready path. The agent embeds the reference +// itself via update_page / edit_page; this keeps tool +// responsibilities crisp (we considered an insert_image +// convenience tool but deferred it — see the design doc). +// +// - download_image: fetch an asset and return it as MCP ImageContent +// so vision-capable agents see the image inline. +// +// - get_page and search_pages gain include_images and +// include_image_metadata flags, opt-in by design (default off so +// token cost stays predictable). When the operator forces images +// off (Server.ForceImagesOff) those flags are silently overridden +// and a notice is appended to the response so callers don't reason +// as if they got what they asked for. + +package mcp + +import ( + "context" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "log/slog" + "time" + + "github.com/aniongithub/mind-map/internal/wiki" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// ForceImagesOff, when set on the Server, makes get_page and +// search_pages behave as if include_images=false and +// include_image_metadata=false regardless of caller request. Intended +// for token-constrained deployments. See SetForceImagesOff. +func (s *Server) SetForceImagesOff(off bool) { s.forceImagesOff = off } + +// uploadImageInput is the request shape for the upload_image tool. +// +// Content is base64-encoded so the JSON-over-stdio MCP transport can +// carry arbitrary binary safely. The Go SDK doesn't currently support +// passing []byte through tool inputs as raw bytes — base64 is the +// universal idiom. +type uploadImageInput struct { + Page string `json:"page" jsonschema:"page path (without .md) under which to store the image; the asset lives in .assets/"` + Name string `json:"name" jsonschema:"filename for the uploaded image, e.g. diagram.png; collisions auto-suffix"` + ContentBase64 string `json:"content_base64" jsonschema:"image bytes encoded as base64. Supported formats: PNG, JPEG, GIF, WebP, AVIF, SVG, BMP, ICO."` +} + +// uploadImageOutput is the success payload. `Path` is the markdown- +// ready relative path the agent should embed; `URL` is the convenience +// HTTP path served by the static asset handler. +type uploadImageOutput struct { + Path string `json:"path"` + URL string `json:"url"` + SizeBytes int64 `json:"size_bytes"` + MIME string `json:"mime"` +} + +func (s *Server) uploadImage(ctx context.Context, _ *mcp.CallToolRequest, in uploadImageInput) (*mcp.CallToolResult, any, error) { + start := time.Now() + + content, err := base64.StdEncoding.DecodeString(in.ContentBase64) + if err != nil { + // Be forgiving of URL-safe base64 too — vision tooling often + // emits one or the other. + if alt, altErr := base64.URLEncoding.DecodeString(in.ContentBase64); altErr == nil { + content = alt + } else { + return nil, nil, fmt.Errorf("decode content_base64: %w", err) + } + } + + uploaded, err := s.wiki.UploadAsset(ctx, in.Page, in.Name, content) + if err != nil { + slog.Warn("tool.upload_image failed", + slog.String("page", in.Page), + slog.String("name", in.Name), + slog.Int("bytes", len(content)), + slog.Any("error", err), + ) + switch { + case errors.Is(err, wiki.ErrAssetTooLarge): + return nil, nil, fmt.Errorf("%w. Compress or split the image before retrying", err) + case errors.Is(err, wiki.ErrUnsupportedAssetType): + return nil, nil, fmt.Errorf("%w. Supported formats: PNG, JPEG, GIF, WebP, AVIF, SVG, BMP, ICO", err) + } + return nil, nil, err + } + + info, err := s.wiki.StatAsset(ctx, uploaded) + if err != nil { + // We just wrote the file, so stat failing is genuinely + // unexpected. Surface a usable response anyway — the + // upload itself succeeded. + slog.Warn("tool.upload_image stat after upload failed", + slog.String("path", uploaded), slog.Any("error", err)) + info = &wiki.AssetInfo{Path: uploaded} + } + + out := uploadImageOutput{ + Path: uploaded, + URL: "/api/assets/" + uploaded, + SizeBytes: info.SizeBytes, + MIME: info.MIME, + } + slog.Info("tool.upload_image", + slog.String("page", in.Page), + slog.String("path", uploaded), + slog.Int64("bytes", info.SizeBytes), + slog.String("mime", info.MIME), + slog.Duration("elapsed", time.Since(start)), + ) + return textResult(out) +} + +// downloadImageInput is the request shape for download_image. The path +// is the wiki-relative asset path (as it appears in markdown). +type downloadImageInput struct { + Path string `json:"path" jsonschema:"wiki-relative path to the image, e.g. projects/mind-map.assets/diagram.png"` +} + +func (s *Server) downloadImage(ctx context.Context, _ *mcp.CallToolRequest, in downloadImageInput) (*mcp.CallToolResult, any, error) { + start := time.Now() + data, mime, err := s.wiki.ReadAsset(ctx, in.Path) + if err != nil { + slog.Warn("tool.download_image failed", + slog.String("path", in.Path), slog.Any("error", err)) + return nil, nil, err + } + slog.Info("tool.download_image", + slog.String("path", in.Path), + slog.Int("bytes", len(data)), + slog.String("mime", mime), + slog.Duration("elapsed", time.Since(start)), + ) + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.ImageContent{ + Data: data, + MIMEType: mime, + }, + }, + }, nil, nil +} + +// pageReadFlags carries the optional image-related read flags used by +// get_page and search_pages. Kept as a named type so the schema +// descriptions land on the same set of fields everywhere. +type pageReadFlags struct { + IncludeImages bool `json:"include_images,omitempty" jsonschema:"if true, include referenced images as MCP image content blocks alongside the page body. Default false — opt in only when a vision-capable agent needs to see the images inline. Server may force this off for token-constrained deployments."` + IncludeImageMetadata bool `json:"include_image_metadata,omitempty" jsonschema:"if true, include { path, size_bytes, mime } for each referenced image without the bytes. Cheap mode for non-vision agents or planning a follow-up download_image call. Default false."` +} + +// getPageInput replaces the legacy pagePathInput for get_page so we can +// add the new flags without affecting other tools that still take a +// bare path. +type getPageInput struct { + Path string `json:"path" jsonschema:"page path without .md extension, e.g. projects/mind-map"` + pageReadFlags +} + +// imageMetadata is the cheap-mode shape included in get_page responses +// when include_image_metadata=true. Subset of wiki.AssetInfo; we keep +// the JSON identical so callers parsing either source see the same +// fields. +type imageMetadata struct { + Path string `json:"path"` + SizeBytes int64 `json:"size_bytes"` + MIME string `json:"mime"` + // Missing reports whether the file was missing on disk despite + // being indexed. Lets agents distinguish "we couldn't read it" + // from "it's a zero-byte file". + Missing bool `json:"missing,omitempty"` +} + +// pageWithImages is the JSON payload for include_image_metadata mode. +// We embed the wiki.Page directly so existing fields appear unchanged. +type pageWithImages struct { + *wiki.Page + Images []imageMetadata `json:"images,omitempty"` + // ImagesForced, when true, signals that the operator-level kill + // switch overrode the caller's request to include images or + // image metadata. Agents should treat this as authoritative — + // retrying with the flag set again won't help. + ImagesForcedOff bool `json:"images_forced_off,omitempty"` +} + +// getPageWithFlags is the new get_page handler. The signature change +// (from pagePathInput to getPageInput) is back-compat at the JSON +// level: the original `path` field is still required, the new fields +// are optional and default to false. +func (s *Server) getPageWithFlags(ctx context.Context, _ *mcp.CallToolRequest, in getPageInput) (*mcp.CallToolResult, any, error) { + start := time.Now() + page, err := s.wiki.GetPage(ctx, in.Path) + if err != nil { + slog.Warn("tool.get_page failed", slog.String("page", in.Path), slog.Any("error", err)) + return nil, nil, err + } + + wantMeta := in.IncludeImageMetadata + wantBytes := in.IncludeImages + forcedOff := s.forceImagesOff && (wantMeta || wantBytes) + if s.forceImagesOff { + wantMeta = false + wantBytes = false + } + + // Fast path: no image work requested → same response shape as + // before. Keeps existing agents and tooling untouched. + if !wantMeta && !wantBytes && !forcedOff { + slog.Info("tool.get_page", + slog.String("page", in.Path), + slog.Duration("elapsed", time.Since(start)), + ) + return textResult(page) + } + + imageRefs, err := s.wiki.ImageRefsForPage(ctx, in.Path) + if err != nil { + slog.Warn("tool.get_page image refs failed", slog.String("page", in.Path), slog.Any("error", err)) + // Don't fail the call — the page body is still useful. + imageRefs = nil + } + + resp := pageWithImages{Page: page, ImagesForcedOff: forcedOff} + + if wantMeta { + resp.Images = make([]imageMetadata, 0, len(imageRefs)) + for _, ref := range imageRefs { + info, statErr := s.wiki.StatAsset(ctx, ref) + if statErr != nil { + resp.Images = append(resp.Images, imageMetadata{Path: ref, Missing: true}) + continue + } + resp.Images = append(resp.Images, imageMetadata{ + Path: info.Path, + SizeBytes: info.SizeBytes, + MIME: info.MIME, + }) + } + } + + // Build the multi-block response: JSON page body first (so non- + // vision agents still get text), then optional image content + // blocks for vision agents. + data, err := json.MarshalIndent(resp, "", " ") + if err != nil { + return nil, nil, err + } + content := []mcp.Content{&mcp.TextContent{Text: string(data)}} + + if wantBytes { + for _, ref := range imageRefs { + body, mime, readErr := s.wiki.ReadAsset(ctx, ref) + if readErr != nil { + slog.Warn("tool.get_page include_images read failed", + slog.String("asset", ref), slog.Any("error", readErr)) + continue + } + content = append(content, &mcp.ImageContent{ + Data: body, + MIMEType: mime, + }) + } + } + + slog.Info("tool.get_page", + slog.String("page", in.Path), + slog.Bool("include_images", wantBytes), + slog.Bool("include_image_metadata", wantMeta), + slog.Bool("forced_off", forcedOff), + slog.Int("image_count", len(imageRefs)), + slog.Duration("elapsed", time.Since(start)), + ) + return &mcp.CallToolResult{Content: content}, nil, nil +} diff --git a/internal/mcp/images_test.go b/internal/mcp/images_test.go new file mode 100644 index 0000000..f31d19e --- /dev/null +++ b/internal/mcp/images_test.go @@ -0,0 +1,249 @@ +package mcp + +import ( + "context" + "encoding/base64" + "encoding/json" + "strings" + "testing" + + "github.com/aniongithub/mind-map/internal/wiki" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// onePixelPNG mirrors the test fixture from internal/wiki/assets_test.go. +// Duplicated here so the MCP tests don't need a cross-package import. +var onePixelPNG = []byte{ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, + 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, 0x89, + 0x00, 0x00, 0x00, 0x0d, 0x49, 0x44, 0x41, 0x54, + 0x78, 0x9c, 0x62, 0x00, 0x01, 0x00, 0x00, 0x05, + 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, + 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, + 0xae, 0x42, 0x60, 0x82, +} + +func TestUploadImageTool(t *testing.T) { + session := setupTestServer(t) + + enc := base64.StdEncoding.EncodeToString(onePixelPNG) + text := callTool(t, session, "upload_image", map[string]any{ + "page": "projects/mind-map", + "name": "diagram.png", + "content_base64": enc, + }) + + var out struct { + Path string `json:"path"` + URL string `json:"url"` + SizeBytes int64 `json:"size_bytes"` + MIME string `json:"mime"` + } + if err := json.Unmarshal([]byte(text), &out); err != nil { + t.Fatalf("unmarshal upload_image: %v\n%s", err, text) + } + wantPath := "projects/mind-map.assets/diagram.png" + if out.Path != wantPath { + t.Errorf("path = %q, want %q", out.Path, wantPath) + } + if !strings.HasSuffix(out.URL, wantPath) { + t.Errorf("URL = %q, want suffix %q", out.URL, wantPath) + } + if out.MIME != "image/png" { + t.Errorf("MIME = %q, want image/png", out.MIME) + } + if out.SizeBytes != int64(len(onePixelPNG)) { + t.Errorf("SizeBytes = %d, want %d", out.SizeBytes, len(onePixelPNG)) + } +} + +func TestDownloadImageTool(t *testing.T) { + session := setupTestServer(t) + ctx := context.Background() + + enc := base64.StdEncoding.EncodeToString(onePixelPNG) + callTool(t, session, "upload_image", map[string]any{ + "page": "projects/mind-map", + "name": "d.png", + "content_base64": enc, + }) + + result, err := session.CallTool(ctx, &mcp.CallToolParams{ + Name: "download_image", + Arguments: map[string]any{ + "path": "projects/mind-map.assets/d.png", + }, + }) + if err != nil { + t.Fatalf("CallTool download_image: %v", err) + } + if len(result.Content) != 1 { + t.Fatalf("expected 1 content block, got %d", len(result.Content)) + } + img, ok := result.Content[0].(*mcp.ImageContent) + if !ok { + t.Fatalf("expected ImageContent, got %T", result.Content[0]) + } + if img.MIMEType != "image/png" { + t.Errorf("MIME = %q, want image/png", img.MIMEType) + } + // The Go SDK base64-encodes ImageContent.Data on the wire and + // decodes back into []byte on the client side, so we can compare + // directly here. + if string(img.Data) != string(onePixelPNG) { + t.Errorf("image bytes differ; got %d bytes, want %d", len(img.Data), len(onePixelPNG)) + } +} + +func TestGetPageIncludeImageMetadata(t *testing.T) { + session := setupTestServer(t) + enc := base64.StdEncoding.EncodeToString(onePixelPNG) + callTool(t, session, "upload_image", map[string]any{ + "page": "projects/mind-map", + "name": "d.png", + "content_base64": enc, + }) + // Reference the asset from the page so the index picks it up. + callTool(t, session, "update_page", map[string]any{ + "path": "projects/mind-map", + "content": "# mm\n\n![d](projects/mind-map.assets/d.png)\n", + }) + + text := callTool(t, session, "get_page", map[string]any{ + "path": "projects/mind-map", + "include_image_metadata": true, + }) + + // Loose check: the JSON should mention the asset path and a size. + if !strings.Contains(text, "projects/mind-map.assets/d.png") { + t.Errorf("expected metadata to mention asset path, got: %s", text) + } + if !strings.Contains(text, "size_bytes") { + t.Errorf("expected metadata to include size_bytes, got: %s", text) + } +} + +func TestGetPageIncludeImagesBytes(t *testing.T) { + session := setupTestServer(t) + enc := base64.StdEncoding.EncodeToString(onePixelPNG) + callTool(t, session, "upload_image", map[string]any{ + "page": "projects/mind-map", + "name": "d.png", + "content_base64": enc, + }) + callTool(t, session, "update_page", map[string]any{ + "path": "projects/mind-map", + "content": "# mm\n\n![d](projects/mind-map.assets/d.png)\n", + }) + + ctx := context.Background() + result, err := session.CallTool(ctx, &mcp.CallToolParams{ + Name: "get_page", + Arguments: map[string]any{ + "path": "projects/mind-map", + "include_images": true, + }, + }) + if err != nil { + t.Fatalf("CallTool: %v", err) + } + + // First block is the text payload; subsequent blocks are images. + if len(result.Content) < 2 { + t.Fatalf("expected text + at least one image, got %d blocks", len(result.Content)) + } + if _, ok := result.Content[0].(*mcp.TextContent); !ok { + t.Fatalf("first block = %T, want TextContent", result.Content[0]) + } + gotImage := false + for _, c := range result.Content[1:] { + if img, ok := c.(*mcp.ImageContent); ok { + gotImage = true + if img.MIMEType != "image/png" { + t.Errorf("image MIME = %q, want image/png", img.MIMEType) + } + } + } + if !gotImage { + t.Errorf("no image content block in response") + } +} + +func TestUploadImageRejectsNonImage(t *testing.T) { + session := setupTestServer(t) + ctx := context.Background() + enc := base64.StdEncoding.EncodeToString([]byte("not an image")) + result, err := session.CallTool(ctx, &mcp.CallToolParams{ + Name: "upload_image", + Arguments: map[string]any{ + "page": "projects/mind-map", + "name": "nope.png", + "content_base64": enc, + }, + }) + // The Go SDK surfaces handler errors as result.IsError rather + // than as a transport error (the call itself succeeded; the tool + // just reported failure). Both shapes are valid signals. + if err == nil && !result.IsError { + t.Fatal("upload_image accepted non-image content") + } +} + +func TestForceImagesOff(t *testing.T) { + // Build a server with the kill-switch flipped on and verify the + // flags are silently overridden. + session, srv := setupTestServerForceOff(t) + + enc := base64.StdEncoding.EncodeToString(onePixelPNG) + callTool(t, session, "upload_image", map[string]any{ + "page": "projects/mind-map", + "name": "d.png", + "content_base64": enc, + }) + callTool(t, session, "update_page", map[string]any{ + "path": "projects/mind-map", + "content": "# mm\n\n![d](projects/mind-map.assets/d.png)\n", + }) + + text := callTool(t, session, "get_page", map[string]any{ + "path": "projects/mind-map", + "include_images": true, + }) + if !strings.Contains(text, "\"images_forced_off\": true") { + t.Errorf("expected images_forced_off in response when force-off is set:\n%s", text) + } + _ = srv +} + +// setupTestServerForceOff wires a server with force-images-off enabled. +// Mirrors setupTestServer but exposes the *Server too so tests can poke +// at server-level flags. +func setupTestServerForceOff(t *testing.T) (*mcp.ClientSession, *Server) { + t.Helper() + dir := t.TempDir() + writeTestFile(t, dir, "index.md", "# Home\n") + writeTestFile(t, dir, "projects/mind-map.md", "# mm\n") + w, err := wiki.Open(dir) + if err != nil { + t.Fatalf("Open wiki: %v", err) + } + t.Cleanup(func() { w.Close() }) + + s := NewServer(w, nil, "test") + s.SetForceImagesOff(true) + + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, nil) + ct, st := mcp.NewInMemoryTransports() + ctx := context.Background() + if _, err := s.MCPServer().Connect(ctx, st, nil); err != nil { + t.Fatalf("server connect: %v", err) + } + session, err := client.Connect(ctx, ct, nil) + if err != nil { + t.Fatalf("client connect: %v", err) + } + t.Cleanup(func() { session.Close() }) + return session, s +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index a58839c..7bfb33f 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -28,6 +28,11 @@ type Server struct { wiki *wiki.Wiki sync SyncRegistrar server *mcp.Server + // forceImagesOff, when true, makes get_page / search_pages behave + // as if include_images and include_image_metadata are both false + // regardless of caller request. Set by operators for token- + // constrained deployments via SetForceImagesOff. + forceImagesOff bool } // NewServer creates an MCP server backed by the given wiki. @@ -66,8 +71,8 @@ func (s *Server) registerTools() { mcp.AddTool(s.server, &mcp.Tool{ Name: "get_page", - Description: "Read a wiki page with parsed frontmatter, body, outgoing links, and backlinks.", - }, s.getPage) + Description: "Read a wiki page with parsed frontmatter, body, outgoing links, and backlinks. Optional flags include_images (returns referenced images as MCP image content for vision agents) and include_image_metadata (returns {path,size,mime} per image without bytes). Both default off to keep token cost predictable.", + }, s.getPageWithFlags) mcp.AddTool(s.server, &mcp.Tool{ Name: "create_page", @@ -108,6 +113,16 @@ func (s *Server) registerTools() { Name: "reindex_wiki", Description: "Force a full reindex pass over the wiki's on-disk markdown files. Use when you've edited files outside the wiki API and want the index (search, page list, backlinks) to reflect disk state without restarting the server. The pass is incremental — unchanged files are skipped via mtime — so it's cheap to call. Returns stats: total/added/updated/removed/unchanged/elapsed_ms.", }, s.reindexWiki) + + mcp.AddTool(s.server, &mcp.Tool{ + Name: "upload_image", + Description: "Upload an image to a page's sidecar directory and return its markdown-ready path. The agent then embeds the reference (e.g. ![alt](returned/path)) via update_page or edit_page. Image bytes must be base64-encoded; supported formats track what browsers render natively (PNG, JPEG, GIF, WebP, AVIF, SVG, BMP, ICO). Collisions auto-suffix.", + }, s.uploadImage) + + mcp.AddTool(s.server, &mcp.Tool{ + Name: "download_image", + Description: "Read an image asset and return it as MCP ImageContent so vision-capable agents can see it directly. Path is the wiki-relative asset path as it appears in markdown references.", + }, s.downloadImage) } // --- Tool input types --- @@ -175,16 +190,8 @@ func (s *Server) getWikiContext(ctx context.Context, _ *mcp.CallToolRequest, _ a return textResult(wctx) } -func (s *Server) getPage(ctx context.Context, _ *mcp.CallToolRequest, input pagePathInput) (*mcp.CallToolResult, any, error) { - start := time.Now() - page, err := s.wiki.GetPage(ctx, input.Path) - if err != nil { - slog.Warn("tool.get_page failed", slog.String("page", input.Path), slog.Any("error", err)) - return nil, nil, err - } - slog.Info("tool.get_page", slog.String("page", input.Path), slog.Duration("elapsed", time.Since(start))) - return textResult(page) -} +// (get_page is implemented in images.go as getPageWithFlags so the +// image-related flags live next to the rest of the image tooling.) func (s *Server) createPage(ctx context.Context, _ *mcp.CallToolRequest, input createInput) (*mcp.CallToolResult, any, error) { start := time.Now() diff --git a/internal/wiki/pages.go b/internal/wiki/pages.go index 6ce7c5b..3dd1755 100644 --- a/internal/wiki/pages.go +++ b/internal/wiki/pages.go @@ -619,6 +619,24 @@ func (w *Wiki) imageRefsFor(ctx context.Context, pagePath string) ([]string, err return images, nil } +// ImageRefsForPage is the exported variant of imageRefsFor used by +// MCP / HTTP handlers that need to enumerate a page's image +// references. The page path is normalized first (same rules as +// GetPage); an empty result is returned for an unknown page rather +// than an error, since "no images referenced" and "page doesn't +// exist" are both legitimate empty cases the caller will usually +// treat the same way. +func (w *Wiki) ImageRefsForPage(ctx context.Context, pagePath string) ([]string, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + normalized, err := normalizePagePath(pagePath) + if err != nil { + return nil, err + } + return w.imageRefsFor(ctx, normalized) +} + func (w *Wiki) topLevelDirs() []string { entries, err := os.ReadDir(w.root) if err != nil { From 95c120d528c4de163b9b90fcd0c33c9650a689bf Mon Sep 17 00:00:00 2001 From: aniongithub Date: Sun, 24 May 2026 15:08:46 -0700 Subject: [PATCH 4/9] feat(httpapi): asset upload + static serving with SVG CSP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/assets accepts either JSON (page, name, content_base64) or multipart/form-data (page, name, file) and writes the bytes through the wiki's UploadAsset. Maps wiki errors to HTTP status codes: ErrAssetTooLarge → 413, ErrUnsupportedAssetType → 415, other errors → 400. GET /assets/ serves uploaded asset bytes via http.ServeContent (gets conditional GET and byte-range support for free). Content-Type is the MIME detected at read time. Cache-Control: public, max-age=300 because assets are stable-by-path (collision auto-suffix means the same path always returns the same bytes). SVG responses get a strict Content-Security-Policy that disables scripts and external loads (default-src 'none'; style-src 'unsafe-inline'; sandbox) to neutralize script-injection from hand-crafted SVG payloads. Same-origin only. Tests cover: JSON upload happy path returning correct path/URL/size, multipart upload, non-image rejection (415), serving bytes round- trip, SVG CSP header presence, not-found (404), and traversal rejection (the wiki layer guard surfaces a 4xx for ../ escapes). --- internal/httpapi/images.go | 222 ++++++++++++++++++++++++++++++++ internal/httpapi/images_test.go | 199 ++++++++++++++++++++++++++++ internal/httpapi/server.go | 1 + 3 files changed, 422 insertions(+) create mode 100644 internal/httpapi/images.go create mode 100644 internal/httpapi/images_test.go diff --git a/internal/httpapi/images.go b/internal/httpapi/images.go new file mode 100644 index 0000000..eaa7194 --- /dev/null +++ b/internal/httpapi/images.go @@ -0,0 +1,222 @@ +// Asset HTTP handlers. +// +// Two endpoints: +// +// - POST /api/assets — upload an image. Accepts either a JSON body +// ({page, name, content_base64}) for parity with the MCP tool, or +// a multipart/form-data body with fields {page, name, file=} +// for browser-friendly uploads. +// +// - GET /assets/.assets/ — serve the bytes of an +// uploaded asset. Lives outside the /api/ prefix so the web UI can +// reference it directly from tags rendered by Goldmark. +// For SVG specifically, a strict Content-Security-Policy is set so +// embedded scripts and external loads cannot execute. + +package httpapi + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "errors" + "io" + "log/slog" + "net/http" + "strings" + "time" + + "github.com/aniongithub/mind-map/internal/wiki" +) + +// registerAssets wires the asset routes. Called from register(). +func (s *Server) registerAssets(mux *http.ServeMux) { + mux.HandleFunc("POST /api/assets", s.uploadAsset) + mux.HandleFunc("GET /assets/{path...}", s.serveAsset) +} + +// uploadAssetJSON is the JSON-body shape for asset uploads. Mirrors +// the MCP tool's input so a client picking either transport sees the +// same field names. +type uploadAssetJSON struct { + Page string `json:"page"` + Name string `json:"name"` + ContentBase64 string `json:"content_base64"` +} + +// uploadAssetResponse is the success payload for POST /api/assets. +type uploadAssetResponse struct { + Path string `json:"path"` + URL string `json:"url"` + SizeBytes int64 `json:"size_bytes"` + MIME string `json:"mime"` +} + +// uploadAsset handles POST /api/assets. Accepts JSON or multipart bodies. +// The page and name fields are required; content arrives as either +// base64-encoded JSON or a multipart "file" part. +func (s *Server) uploadAsset(rw http.ResponseWriter, r *http.Request) { + page, name, content, err := readAssetUpload(r) + if err != nil { + http.Error(rw, err.Error(), http.StatusBadRequest) + return + } + + uploaded, err := s.deps.Wiki.UploadAsset(r.Context(), page, name, content) + if err != nil { + slog.Warn("http upload_asset failed", + slog.String("page", page), + slog.String("name", name), + slog.Int("bytes", len(content)), + slog.Any("error", err), + ) + switch { + case errors.Is(err, wiki.ErrAssetTooLarge): + http.Error(rw, err.Error(), http.StatusRequestEntityTooLarge) + case errors.Is(err, wiki.ErrUnsupportedAssetType): + http.Error(rw, err.Error(), http.StatusUnsupportedMediaType) + default: + http.Error(rw, err.Error(), http.StatusBadRequest) + } + return + } + + info, statErr := s.deps.Wiki.StatAsset(r.Context(), uploaded) + if statErr != nil { + slog.Warn("http upload_asset stat failed", + slog.String("path", uploaded), slog.Any("error", statErr)) + info = &wiki.AssetInfo{Path: uploaded} + } + + rw.WriteHeader(http.StatusCreated) + writeJSON(rw, uploadAssetResponse{ + Path: uploaded, + URL: "/assets/" + uploaded, + SizeBytes: info.SizeBytes, + MIME: info.MIME, + }) +} + +// readAssetUpload extracts (page, name, content) from either a JSON +// body or a multipart/form-data body. Returns descriptive errors that +// the caller can pass straight to http.Error. +// +// Body size is bounded by http.MaxBytesReader using the wiki's +// MaxAssetBytes (or default) plus a small overhead for the multipart +// framing. Going over the cap is reported as "request entity too +// large" by the standard library. +func readAssetUpload(r *http.Request) (page, name string, content []byte, err error) { + maxBytes := defaultUploadCapForRequest(r) + r.Body = http.MaxBytesReader(nil, r.Body, maxBytes) + + ct := r.Header.Get("Content-Type") + switch { + case strings.HasPrefix(ct, "multipart/form-data"): + // 32 MiB in-memory threshold matches stdlib defaults for + // form parsing; anything larger gets spooled to a temp + // file by the multipart reader. + if err := r.ParseMultipartForm(32 << 20); err != nil { + return "", "", nil, errors.New("parse multipart: " + err.Error()) + } + page = r.FormValue("page") + name = r.FormValue("name") + + f, hdr, ferr := r.FormFile("file") + if ferr != nil { + return "", "", nil, errors.New("missing 'file' part: " + ferr.Error()) + } + defer f.Close() + if name == "" { + name = hdr.Filename + } + data, rerr := io.ReadAll(f) + if rerr != nil { + return "", "", nil, errors.New("read 'file' part: " + rerr.Error()) + } + content = data + + default: + // Treat anything else as JSON for simplicity. application/ + // json is the documented happy path; other content types + // either decode as JSON or get a clear parse error. + var body uploadAssetJSON + if derr := json.NewDecoder(r.Body).Decode(&body); derr != nil { + return "", "", nil, errors.New("invalid JSON: " + derr.Error()) + } + page = body.Page + name = body.Name + if body.ContentBase64 == "" { + return "", "", nil, errors.New("content_base64 is required for JSON uploads") + } + data, derr := base64.StdEncoding.DecodeString(body.ContentBase64) + if derr != nil { + if alt, altErr := base64.URLEncoding.DecodeString(body.ContentBase64); altErr == nil { + data = alt + } else { + return "", "", nil, errors.New("decode content_base64: " + derr.Error()) + } + } + content = data + } + + if page == "" { + return "", "", nil, errors.New("page is required") + } + if name == "" { + return "", "", nil, errors.New("name is required") + } + return page, name, content, nil +} + +// defaultUploadCapForRequest returns the HTTP-level body cap for an +// upload. We don't have a clean handle on Wiki.MaxAssetBytes from +// here without expanding the Deps surface, so we use a generous +// constant upper bound (128 MiB) and let the wiki layer report the +// precise cap to the client when it rejects via ErrAssetTooLarge. +// The cap mostly exists to bound multipart parsing memory. +func defaultUploadCapForRequest(_ *http.Request) int64 { + return 128 * 1024 * 1024 +} + +// serveAsset handles GET /assets/. Reads the asset from the +// wiki and streams it back with the correct Content-Type. SVG is +// served with a strict CSP to neutralize script-injection from +// hand-crafted SVG payloads. +// +// The /assets prefix is deliberately distinct from the SPA static +// handler at "/" so URLs in markdown (rewritten by the web UI to +// /assets/) don't conflict with the React/Preact routes. +func (s *Server) serveAsset(rw http.ResponseWriter, r *http.Request) { + assetPath := r.PathValue("path") + if assetPath == "" { + http.NotFound(rw, r) + return + } + + data, mime, err := s.deps.Wiki.ReadAsset(r.Context(), assetPath) + if err != nil { + if errors.Is(err, wiki.ErrAssetNotFound) { + http.NotFound(rw, r) + return + } + slog.Warn("http serve_asset failed", + slog.String("path", assetPath), slog.Any("error", err)) + http.Error(rw, err.Error(), http.StatusBadRequest) + return + } + + rw.Header().Set("Content-Type", mime) + rw.Header().Set("Cache-Control", "public, max-age=300") + // Conservative CSP for SVG: no scripts, no external loads, no + // inline event handlers. Stops script-injection attacks via + // embedded