From d1a99ca5006e48a8273cb949e8b16dab1fc259b6 Mon Sep 17 00:00:00 2001 From: Zef Hemel Date: Tue, 28 Jul 2026 13:39:15 +0200 Subject: [PATCH] Fixes #2028: user attributes could overwrite an indexed object's source offsets --- docs/CHANGELOG.md | 7 ++++--- plugs/index/data.test.ts | 21 +++++++++++++++++++++ plugs/index/data.ts | 3 ++- plugs/index/frontmatter.test.ts | 28 ++++++++++++++++++++++++++++ plugs/index/frontmatter.ts | 6 +++++- plugs/index/header.test.ts | 20 ++++++++++++++++++++ plugs/index/header.ts | 3 ++- plugs/index/item.test.ts | 22 ++++++++++++++++++++++ plugs/index/item.ts | 3 +++ plugs/index/paragraph.test.ts | 12 ++++++++++++ plugs/index/paragraph.ts | 3 ++- plugs/index/position_attributes.ts | 30 ++++++++++++++++++++++++++++++ plugs/index/table.test.ts | 30 ++++++++++++++++++++++++++++++ plugs/index/table.ts | 4 +++- 14 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 plugs/index/frontmatter.test.ts create mode 100644 plugs/index/position_attributes.ts diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 98e3f157..bf351c99 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,9 +4,6 @@ An attempt at documenting the changes/new features introduced in each release. Whenever a commit is pushed to the `main` branch, within ~5 minutes, it will be released as a docker image with the `:v2` tag, and a binary in the [edge release](https://github.com/silverbulletmd/silverbullet/releases/tag/edge). If you want to live on the bleeding edge of SilverBullet goodness (or regression) this is where to do it. * [[Space Manager]]: multi-space hosting with multiple accounts is here. A fresh install pointed at an empty folder opens a browser-based first-run **setup wizard** that creates an admin account and your first space, then serves it in place with no restart. One server can host any number of [[Space|spaces]], each bound to a URL prefix or hostname. -* Fix: `SB_SHELL_BACKEND=local` disabled shell command execution instead of enabling it. -* Fix: only the first entry of a frontmatter list of wiki links formed a [[Object/relation]] (e.g. an `authors:` list of `[[...]]` items), so every entry after the first was missing from [[Object Graph]], linked mentions and rename refactoring. -* Fix: writing to a read-only path (anything served from the bundled library, a `SB_READ_ONLY` server) returned a 500, which clients could not tell apart from a temporary server fault — so a syncing client retried it forever. Read-only refusals now return 403, and the sync engine records the path and stops re-attempting it until the local file changes. This most often bit spaces holding a stale copy of a `Library/Std` page that a later release had dropped from the bundle. * [[Baked Sections]]: bake `${...}` Lua expressions and widgets into HTML-comment-delimited markdown (`` … ``). * Space Lua: **code complete now shows documentation** (where available), all available via [[API/spacelua]] reflection APIs. @@ -17,6 +14,10 @@ Whenever a commit is pushed to the `main` branch, within ~5 minutes, it will be * [[Frontmatter]] in the editor now has configurable folding: by default long frontmatter blocks fold automatically, and `frontmatterFolding` options let you disable auto-folding, always fold frontmatter, or change the line threshold. A subtle right-side marker folds or unfolds the whole block, and folded frontmatter previews any `tags` value as tag chips. This is configurable via the [[Configuration Manager]] as well. * [[Page Picker]]: type `$` to switch to [[Markdown/Anchor|anchor]] navigation * Pulling the "this was experimental card" for the CLI: removed the `sb get` command and the `/.runtime/objects/*` REST API, including their dedicated client-side query bridge. Use `sb query`, `sb eval`, or `sb script` for indexed-object access. This added too much complexity and another query language. +* Fix: `SB_SHELL_BACKEND=local` disabled shell command execution instead of enabling it. +* Fix: a `range` (or `pos`) attribute in your own content could overwrite the source offsets an indexed object uses to point back at its place in the page. +* Fix: only the first entry of a frontmatter list of wiki links formed a [[Object/relation]] (e.g. an `authors:` list of `[[...]]` items), so every entry after the first was missing from [[Object Graph]], linked mentions and rename refactoring. +* Fix: writing to a read-only path (anything served from the bundled library, a `SB_READ_ONLY` server) returned a 500, which clients could not tell apart from a temporary server fault — so a syncing client retried it forever. Read-only refusals now return 403, and the sync engine records the path and stops re-attempting it until the local file changes. This most often bit spaces holding a stale copy of a `Library/Std` page that a later release had dropped from the bundle. * Fix: major typing/navigation slowdown on pages with many internal links in large spaces. * Fix: the service worker precached client assets *through* the browser's HTTP cache, so a stale client could be copied into its cache and then served as though it were the current build — leaving a "A new version of SilverBullet client is available." notification that no reload could clear (only a hard reload, which bypasses the service worker, showed the real client; the next normal reload brought the notification back). Precaching now bypasses the HTTP cache. * The server now sets `Cache-Control` on client assets, which matters if you run SilverBullet behind a CDN. diff --git a/plugs/index/data.test.ts b/plugs/index/data.test.ts index 0bbd9a49..0163d40a 100644 --- a/plugs/index/data.test.ts +++ b/plugs/index/data.test.ts @@ -142,3 +142,24 @@ describe("range covers the YAML content, not the fence markers", () => { expect(md.slice(b.range[0], b.range[1])).toContain("Hank"); }); }); + +test("user `pos`/`range` attributes cannot overwrite the block's source offsets", async () => { + createMockSystem(); + const md = [ + "```#person", + "name: Pete", + "range: 0, 36", + "pos: nope", + "```", + ].join("\n"); + const results = await indexDataForTest(md, "Page"); + const person = results.find((o) => o.tag === "person")!; + expect(person).toBeTruthy(); + expect(person.range.every((n: unknown) => typeof n === "number")).toBe(true); + expect(typeof person.pos).toBe("number"); + expect(person.range[0]).toBe(md.indexOf("name:")); + expect(md.slice(person.range[0], person.range[1])).toContain("Pete"); + expect(md.slice(person.range[0], person.range[1])).not.toContain("```"); + // The user's own attributes are still indexed as data, just not as offsets. + expect(person.name).toBe("Pete"); +}); diff --git a/plugs/index/data.ts b/plugs/index/data.ts index f7458d6a..b80b958d 100644 --- a/plugs/index/data.ts +++ b/plugs/index/data.ts @@ -7,6 +7,7 @@ import { import type { TagObject } from "./tags.ts"; import type { FrontMatter } from "./frontmatter.ts"; import { updateITags } from "./tags.ts"; +import { stripPositionAttributes } from "./position_attributes.ts"; import type { ObjectValue, PageMeta, @@ -77,7 +78,7 @@ export function indexData( itags: ["data"], pos: docStart, range: [docStart, docEnd] as [number, number], - ...doc, + ...stripPositionAttributes(doc), page: pageMeta.name, }; updateITags(dataObj, frontmatter); diff --git a/plugs/index/frontmatter.test.ts b/plugs/index/frontmatter.test.ts new file mode 100644 index 00000000..be971a01 --- /dev/null +++ b/plugs/index/frontmatter.test.ts @@ -0,0 +1,28 @@ +import { expect, test } from "vitest"; +import { parseMarkdown } from "../../client/markdown_parser/parser.ts"; +import { extractFrontMatter } from "./frontmatter.ts"; + +test("extracts the frontmatter block's own range", () => { + const text = `---\nstuff: things\n---\n\nBody\n`; + const fm = extractFrontMatter(parseMarkdown(text)); + expect(fm.range).toEqual([0, text.indexOf("---\n\nBody") + 3]); +}); + +test("a user `range` attribute cannot overwrite the frontmatter's own range", () => { + // Regression for #2028: `range` reaches CodeMirror as a position pair + // (X-Ray diagnostics, lint), so a user-authored value took the tab down. + const text = `---\nstuff: things\nrange: 0, 36\notherstuff: more things\n---\n\nBody\n`; + const fm = extractFrontMatter(parseMarkdown(text)); + expect(Array.isArray(fm.range)).toBe(true); + expect(fm.range!.every((n) => typeof n === "number")).toBe(true); + expect(text.slice(fm.range![0], fm.range![1])).toContain("stuff: things"); + // Other frontmatter data is untouched. + expect(fm.stuff).toEqual("things"); + expect(fm.otherstuff).toEqual("more things"); +}); + +test("a user `pos` attribute cannot overwrite a synthesized position either", () => { + const text = `---\npos: not a number\n---\n\nBody\n`; + const fm = extractFrontMatter(parseMarkdown(text)); + expect(fm.pos).toBeUndefined(); +}); diff --git a/plugs/index/frontmatter.ts b/plugs/index/frontmatter.ts index dc9e14fc..f7665cb0 100644 --- a/plugs/index/frontmatter.ts +++ b/plugs/index/frontmatter.ts @@ -11,6 +11,7 @@ import YAML from "js-yaml"; import { extractHashtag } from "@silverbulletmd/silverbullet/lib/tags"; import type { CompleteEvent } from "@silverbulletmd/silverbullet/type/client"; import { determineTags } from "./cheap_yaml.ts"; +import { stripPositionAttributes } from "./position_attributes.ts"; import { attributeCompletion } from "./complete.ts"; export type FrontMatter = { @@ -99,7 +100,10 @@ export function extractFrontMatter( const parsedData: any = cleanupJSON(YAML.load(yamlText)); // console.log("Parsed front matter", parsedData); const newData = { ...parsedData }; - frontmatter = { ...frontmatter, ...parsedData }; + frontmatter = { + ...frontmatter, + ...stripPositionAttributes(parsedData), + }; // Make sure we have a tags array if (!frontmatter.tags) { frontmatter.tags = []; diff --git a/plugs/index/header.test.ts b/plugs/index/header.test.ts index 46cdcafd..a13b0725 100644 --- a/plugs/index/header.test.ts +++ b/plugs/index/header.test.ts @@ -71,3 +71,23 @@ test("header without anchor keeps Page@pos ref shape", async () => { expect(headers.length).toEqual(1); expect(headers[0].ref).toMatch(/^MyPage@\d+$/); }); + +test("user `range` attribute cannot overwrite a header's source offsets", async () => { + createMockSystem(); + const md = `# Real header [range: 0, 36]`; + const tree = parseMarkdown(md); + const frontmatter = extractFrontMatter(tree); + const pageMeta: PageMeta = { + ref: "test", + name: "test", + tag: "page", + created: "", + lastModified: "", + perm: "rw", + }; + const headers = await indexHeaders(pageMeta, frontmatter, tree); + const header = headers.find((h: any) => h.tag === "header")! as any; + expect(header).toBeTruthy(); + expect(header.range).toEqual([0, md.length]); + expect(typeof header.pos).toBe("number"); +}); diff --git a/plugs/index/header.ts b/plugs/index/header.ts index 8c66870a..3f5118d1 100644 --- a/plugs/index/header.ts +++ b/plugs/index/header.ts @@ -18,6 +18,7 @@ import type { FrontMatter } from "./frontmatter.ts"; import { cleanAttributes, collectAttributes } from "./attribute.ts"; import { cleanTags, collectTags } from "./tags.ts"; import { cleanAnchor, collectAnchor } from "./anchor.ts"; +import { stripPositionAttributes } from "./position_attributes.ts"; type HeaderObject = ObjectValue< { @@ -63,7 +64,7 @@ export function indexHeaders( page: pageMeta.name, pos: n.from!, range: [n.from!, n.to!], - ...attributes, + ...stripPositionAttributes(attributes), }); } diff --git a/plugs/index/item.test.ts b/plugs/index/item.test.ts index f64aba13..676151bf 100644 --- a/plugs/index/item.test.ts +++ b/plugs/index/item.test.ts @@ -151,3 +151,25 @@ test("Test item indexing with nested lists in list items", async () => { const regularItem = items.find((i) => i.name === "Regular item"); expect(regularItem).toBeDefined(); }); + +test("user `range` attribute cannot overwrite an item's source offsets", async () => { + createMockSystem(); + const md = `* An item [range: 0, 36]`; + const tree = parseMarkdown(md); + const frontmatter = extractFrontMatter(tree); + const pageMeta: PageMeta = { + ref: "test", + name: "test", + tag: "page", + created: "", + lastModified: "", + perm: "rw", + }; + const items = await indexItems(pageMeta, frontmatter, tree); + const item = items.find((i: any) => i.tag === "item")! as any; + expect(item).toBeTruthy(); + expect(Array.isArray(item.range)).toBe(true); + expect(item.range.every((n: unknown) => typeof n === "number")).toBe(true); + expect(md.slice(item.range[0], item.range[1])).toContain("An item"); + expect(typeof item.pos).toBe("number"); +}); diff --git a/plugs/index/item.ts b/plugs/index/item.ts index b710a5cb..2be5b989 100644 --- a/plugs/index/item.ts +++ b/plugs/index/item.ts @@ -7,6 +7,7 @@ import { } from "@silverbulletmd/silverbullet/lib/tree"; import { cleanTags, collectTags, updateITags } from "./tags.ts"; import { cleanAnchor, collectAnchor } from "./anchor.ts"; +import { isPositionAttribute } from "./position_attributes.ts"; import type { FrontMatter } from "./frontmatter.ts"; import type { ObjectValue, @@ -177,6 +178,8 @@ export function extractItemFromNode( } for (const [key, value] of Object.entries(attributes)) { + // `pos`/`range` are the item's own source offsets, not user data (#2028). + if (isPositionAttribute(key)) continue; item[key] = value; } diff --git a/plugs/index/paragraph.test.ts b/plugs/index/paragraph.test.ts index 602b5171..c40b89a3 100644 --- a/plugs/index/paragraph.test.ts +++ b/plugs/index/paragraph.test.ts @@ -86,3 +86,15 @@ test("paragraph without anchor keeps Page@pos ref", async () => { const para = objects.find((o) => o.tag === "paragraph")!; expect(para.ref).toMatch(/^MyPage@\d+$/); }); + +test("a `range` attribute does not overwrite a paragraph's source offsets", async () => { + // Tagged so it is indexed with the default `index.paragraph.all = false`. + const paragraphs = await indexParagraphsForTest( + "A #noted paragraph with [range: 0, 36] in it.\n", + ); + const p = paragraphs[0] as any; + expect(p).toBeTruthy(); + expect(Array.isArray(p.range)).toBe(true); + expect(p.range.every((n: unknown) => typeof n === "number")).toBe(true); + expect(typeof p.pos).toBe("number"); +}); diff --git a/plugs/index/paragraph.ts b/plugs/index/paragraph.ts index 2fcb4ce2..87189074 100644 --- a/plugs/index/paragraph.ts +++ b/plugs/index/paragraph.ts @@ -14,6 +14,7 @@ import type { } from "@silverbulletmd/silverbullet/type/index"; import { system } from "@silverbulletmd/silverbullet/syscalls"; import { cleanAttributes, collectAttributes } from "./attribute.ts"; +import { stripPositionAttributes } from "./position_attributes.ts"; /** ParagraphObject An index object for the top level text nodes */ export type ParagraphObject = ObjectValue< @@ -81,7 +82,7 @@ export async function indexParagraphs( page: pageMeta.name, pos, range: [p.from!, p.to!], - ...attrs, + ...stripPositionAttributes(attrs), }; if (tags.length > 0) { paragraph.tags = [...tags]; diff --git a/plugs/index/position_attributes.ts b/plugs/index/position_attributes.ts new file mode 100644 index 00000000..a274784d --- /dev/null +++ b/plugs/index/position_attributes.ts @@ -0,0 +1,30 @@ +/** + * Internally used attribute names that cannot be overriden by the user. + */ +const POSITION_ATTRIBUTES = ["pos", "range"]; + +/** + * Returns `data` without the reserved position attributes, so it is safe to + * merge over a synthesized object. Returns the original object untouched when + * it carries neither, which is the overwhelmingly common case. + */ +export function stripPositionAttributes>( + data: T, +): T { + if (!POSITION_ATTRIBUTES.some((name) => name in data)) { + return data; + } + const cleaned = { ...data }; + for (const name of POSITION_ATTRIBUTES) { + delete cleaned[name]; + } + return cleaned; +} + +/** + * Whether `name` is a reserved position attribute (for attribute-by-attribute + * merges). + */ +export function isPositionAttribute(name: string): boolean { + return POSITION_ATTRIBUTES.includes(name); +} diff --git a/plugs/index/table.test.ts b/plugs/index/table.test.ts index bc6adfea..31ed4428 100644 --- a/plugs/index/table.test.ts +++ b/plugs/index/table.test.ts @@ -34,3 +34,33 @@ test("Test table indexing", async () => { expect(datas[1].name).toEqual("Fred"); expect(datas[1].age).toEqual("21"); }); + +test("a `range` column does not overwrite the row's source offsets", async () => { + createMockSystem(); + // A table of date ranges is an entirely reasonable thing to write; the + // column must not land on the row object's own offsets (#2028). + const md = ` +| name | range | +|----------|----------| +| Frank | 0, 36 | +`.trim(); + const tree = parseMarkdown(md); + const frontmatter = extractFrontMatter(tree); + const pageMeta: PageMeta = { + ref: "test", + name: "test", + tag: "page", + created: "", + lastModified: "", + perm: "rw", + }; + + const rows = await indexTables(pageMeta, frontmatter, tree); + expect(rows).toHaveLength(1); + const row = rows[0] as any; + expect(row.name).toEqual("Frank"); + expect(Array.isArray(row.range)).toBe(true); + expect(row.range.every((n: unknown) => typeof n === "number")).toBe(true); + expect(md.slice(row.range[0], row.range[1])).toContain("Frank"); + expect(typeof row.pos).toBe("number"); +}); diff --git a/plugs/index/table.ts b/plugs/index/table.ts index 19a46d55..7720e037 100644 --- a/plugs/index/table.ts +++ b/plugs/index/table.ts @@ -14,6 +14,7 @@ import type { } from "@silverbulletmd/silverbullet/type/index"; import { extractHashtag } from "@silverbulletmd/silverbullet/lib/tags"; import type { FrontMatter } from "./frontmatter.ts"; +import { isPositionAttribute } from "./position_attributes.ts"; type TableRowObject = ObjectValue<{ tableref: string; @@ -102,7 +103,8 @@ export function indexTables( }); const content = concatChildrenTextsPreserveLinks(c.children!); const label = headerLabels[i]; - tableRow[label!] = content; + if (!label || isPositionAttribute(label)) return; + tableRow[label] = content; }); result.push(tableRow); }