From 4842ad9a829fa0c35ce38c21c12e4669997c3f19 Mon Sep 17 00:00:00 2001 From: Anton Wiklund Date: Sun, 8 Nov 2026 16:33:12 +0100 Subject: [PATCH] fix(solid): publish the draft as its checks read it Publishing checked a draft on one read and then assembled the release from its documents read again, accepting the second read when each document's ETag was the same. A server may give no ETag, or the same one for other content, so a release could be written from content the release rules, the Markdown check and the shapes never saw, and the draft then marked released from that read too. assemble now makes the release from the read at the version it is given, as the repository holds it, without reading again, and refuses with changedElsewhere when that read is no longer the draft's last. A version now names the read itself: a document read again is a new read unless the pod answers 304, so a draft read anew in between is never taken for the one checked, whatever its ETags say. Refs: GHSA-438x-f779-7chm --- docs/data-model.md | 7 ++ docs/studio.md | 10 ++- packages/application/src/ports.ts | 16 ++-- packages/application/src/releasePublishing.ts | 3 +- .../src/solidReleaseDraftRepository.test.ts | 74 +++++++++++++++++++ .../solid/src/solidReleaseDraftRepository.ts | 60 ++++++++++++--- 6 files changed, 148 insertions(+), 22 deletions(-) diff --git a/docs/data-model.md b/docs/data-model.md index f5eeaef62..f13430cd4 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -1266,6 +1266,13 @@ own pod, as a draft, before publishing it ([deck-library.md](deck-library.md#a-release-outside-the-library)). `assemble` makes a draft a release's text, with every subject moved to the release's address, `@base` that address, and its release time set. + Publishing assembles the draft from the read its checks saw, not from + its documents read again. A draft's version names that read, never + only its documents' ETags: a document read again is a new read unless + the pod answers 304. A server can give no ETag, or the same one for + other content, so ETags alone can't show the draft is unchanged. A + read since, or an edit, makes publishing at that version + `changedElsewhere`. - **Publishing a release** ([solidReleasePublisher.ts](../packages/solid/src/solidReleasePublisher.ts), [studio.md](studio.md#publishing-a-release)) puts that text, as it is, diff --git a/docs/studio.md b/docs/studio.md index fcde4fcbc..c88165224 100644 --- a/docs/studio.md +++ b/docs/studio.md @@ -2347,10 +2347,12 @@ releases it, two ways. address is ignored, so such a draft reads as not released, as `data-model.md` says. - **One version.** What is checked, written and marked released is the - draft at the version read (If-Match). A draft changed elsewhere - meanwhile is `changedElsewhere`: before the release is written, - nothing is written; after, the release stays and the draft isn't - marked. + draft at the version read (If-Match). The release is made from the read + the checks saw, not from the documents read again, so it doesn't + rest on the server's ETags, which some servers leave out. A draft + read anew or changed elsewhere meanwhile is `changedElsewhere`: + before the release is written, nothing is written; after, the + release stays and the draft isn't marked. - **Finished again.** Publishing cut short after the release is written (the catalog, the draft or the network failed) is finished by publishing again, at the same address. A document there that states diff --git a/packages/application/src/ports.ts b/packages/application/src/ports.ts index 7b60265ab..8c11d5f15 100644 --- a/packages/application/src/ports.ts +++ b/packages/application/src/ports.ts @@ -589,10 +589,12 @@ export interface ReleaseDraftRepository { */ create(instanceUrl: string, draft: ReleaseDraft): Promise; /** - * The draft as its documents say now, with the version they are at - * together (opaque: it changes when any of them does, or one comes or - * goes). Nothing it published before is known here: `published` is - * empty. Throws draftGone when it has no release document, and + * The draft as its documents say now, with the version of this read + * of them (opaque: another when any of them is read anew rather than + * found unchanged, or one comes or goes; never only their ETags, which + * a server may leave out or keep for other content). Nothing it + * published before is known here: `published` is empty. Throws + * draftGone when it has no release document, and * draftIdUnsupported when it names a subject by an id no draft can * have (DRAFT_ID): a chapter's document is named after its id; and * draftHiddenValue when its documents state a predicate a subject's @@ -618,8 +620,10 @@ export interface ReleaseDraftRepository { * (`@base` its URL, the usual prefixes): every subject moved there, * released `issued` (also its time of change), written at library * deck format 6, with the series it starts described in it. With - * `version`, the draft as it was at that version (read), or - * changedElsewhere when its documents changed since. A draft that + * `version`, the draft as the read at that version had it, not read + * again, so what was checked of that read is what is published; or + * changedElsewhere when that read is not the draft's last (read again + * or edited since). A draft that * states anything of `targetUrl` or a subject of it (`#id`) * itself, which would say more of the release than its parts do, is * releaseForeignStatement. One whose documents name a subject of the diff --git a/packages/application/src/releasePublishing.ts b/packages/application/src/releasePublishing.ts index 56c2510ab..a3501b841 100644 --- a/packages/application/src/releasePublishing.ts +++ b/packages/application/src/releasePublishing.ts @@ -118,7 +118,8 @@ export function createReleasePublishingUseCases({ return { async publishRelease(draftUrl, targetUrl, markdownCheck) { if (isGuestUrl(draftUrl)) throw new AppError("guestCannotPublish"); - // Read once: what is checked is what is published, and what is marked released. + // Read once: what is checked is what is published (assembled from + // this read, not read again), and what is marked released. const { draft, version } = await drafts.readReleaseDraft(draftUrl); if (draft.root.releasedAs !== undefined) throw new AppError("draftReleased", { url: draft.root.releasedAs }); diff --git a/packages/solid/src/solidReleaseDraftRepository.test.ts b/packages/solid/src/solidReleaseDraftRepository.test.ts index 1befa75b3..a78b81976 100644 --- a/packages/solid/src/solidReleaseDraftRepository.test.ts +++ b/packages/solid/src/solidReleaseDraftRepository.test.ts @@ -62,6 +62,25 @@ const RDF = "http://www.w3.org/1999/02/22-rdf-syntax-ns#"; /** The course fixture as a draft of the instance. */ const course = (): ReleaseDraft => rebaseDraft(courseDraft(), DRAFT); +/** + * The pod's reads as a server answers them that gives no ETag (null), or + * always the same one, and ignores If-None-Match: every read a 200. + */ +function withEtag(fetch: typeof globalThis.fetch, etag: string | null): typeof globalThis.fetch { + return (async (input, init) => { + if ((init?.method ?? "GET") !== "GET") return fetch(input, init); + const asked = new Headers(init?.headers); + asked.delete("If-None-Match"); + const response = await fetch(input, { ...init, headers: asked }); + const headers = new Headers(response.headers); + if (etag === null) headers.delete("ETag"); + else headers.set("ETag", etag); + const answer = new Response(await response.text(), { status: response.status, headers }); + Object.defineProperty(answer, "url", { value: response.url || String(input) }); + return answer; + }) as typeof globalThis.fetch; +} + /** The draft with the changes made, which must be. */ function changed(draft: ReleaseDraft, ...changes: DraftChange[]): ReleaseDraft { const result = applyDraftChanges(draft, changes); @@ -999,6 +1018,61 @@ describe("assemble", () => { }); }); + describe.each([ + ["its own ETags", undefined], + ["no ETag", null], + ["the same ETag for other content", '"same"'], + ] as const)("on a pod that gives %s (GHSA-438x-f779-7chm)", (_, etag) => { + const target = `${INSTANCE}releases/solid/v1.ttl`; + const SWAPPED = "Changed after the check"; + + /** A draft read as publishing reads it before its checks, one card changed on the pod since. */ + async function checkedThenChanged() { + const pod = await draftPod(); + await pod.repository.create(INSTANCE, course()); + const repository = createSolidReleaseDraftRepository({ + fetch: etag === undefined ? pod.fetch : withEtag(pod.fetch, etag), + releaseFetch: siteFetch, + }); + const read = await repository.read(DRAFT); + await pod.put(CARDS, (await pod.text(CARDS)).replace('"loose!"@en', `"${SWAPPED}"@en`)); + return { repository, ...read }; + } + + it("makes the release of the draft as read at that version, not as it is now", async () => { + const { repository, version } = await checkedThenChanged(); + const turtle = await repository.assemble(DRAFT, target, NOW, version); + expect(turtle).toContain("loose!"); + expect(turtle).not.toContain(SWAPPED); + expect(await repository.assemble(DRAFT, target, NOW)).toContain(SWAPPED); + }); + + it("refuses to make it, or mark it released, at that version once the draft is read again", async () => { + const { repository, draft, version } = await checkedThenChanged(); + await repository.read(DRAFT); + await expect(repository.assemble(DRAFT, target, NOW, version)).rejects.toMatchObject({ + code: "changedElsewhere", + vars: { url: DRAFT }, + }); + await expect( + repository.applyChanges( + draft, + { ...draft, root: { ...draft.root, releasedAs: target } }, + version, + ), + ).rejects.toMatchObject({ code: "changedElsewhere" }); + }); + }); + + it("keeps the version of a draft read again unchanged, where the pod says so (304)", async () => { + const pod = await draftPod(); + await pod.repository.create(INSTANCE, course()); + const target = `${INSTANCE}releases/solid/v1.ttl`; + const { version } = await pod.repository.read(DRAFT); + expect((await pod.repository.read(DRAFT)).version).toBe(version); + await expect(pod.repository.assemble(DRAFT, target, NOW, version)).resolves.toContain("loose!"); + }); + it("refuses a draft that states anything of the release at its address already, for a pod or the library (GHSA-p2pr-x6fw-hrfx)", async () => { const pod = await draftPod(); const target = `${INSTANCE}releases/solid/v1.ttl`; diff --git a/packages/solid/src/solidReleaseDraftRepository.ts b/packages/solid/src/solidReleaseDraftRepository.ts index 4ba428fdb..cee6a2b96 100644 --- a/packages/solid/src/solidReleaseDraftRepository.ts +++ b/packages/solid/src/solidReleaseDraftRepository.ts @@ -54,7 +54,6 @@ import { removeOwned, readDataset, saveDataset, - versionOf, } from "./datasets"; import { draftEntries, @@ -156,11 +155,33 @@ export function createSolidReleaseDraftRepository({ const documentsOf = (draftUrl: string) => draftDocumentsIn(draftUrl, fetch); - /** A draft's documents, read now and remembered as read. */ - async function readDraft(draftUrl: string): Promise<{ draft: ReleaseDraft; version: string }> { + /** A draft's documents, read now. */ + async function readDocuments( + draftUrl: string, + ): Promise<{ urls: readonly string[]; datasets: readonly Dataset[] }> { const urls = await documentsOf(draftUrl); if (!urls.includes(draftUrl)) throw new AppError("draftGone"); - const datasets = await mapAtOnce(urls, (url) => readDataset(url, fetch)); + return { urls, datasets: await mapAtOnce(urls, (url) => readDataset(url, fetch)) }; + } + + /** + * A draft's documents as the read at `version` had them, not read + * again: what was checked of that read is what is published, which a + * server's ETags alone cannot vouch for (GHSA-438x-f779-7chm). + * changedElsewhere when that read is not the draft's last. + */ + function readAt( + draftUrl: string, + version: string, + ): { urls: readonly string[]; datasets: readonly Dataset[] } { + const read = reads.get(draftUrl); + if (read?.version !== version) throw new AppError("changedElsewhere", { url: draftUrl }); + return { urls: [...read.documents.keys()], datasets: [...read.documents.values()] }; + } + + /** A draft's documents, read now and remembered as read. */ + async function readDraft(draftUrl: string): Promise<{ draft: ReleaseDraft; version: string }> { + const { urls, datasets } = await readDocuments(draftUrl); const quads = draftNamedQuads(draftUrl, urls, datasets); // What its documents state otherwise than its records write it would be published, unseen and unchecked (GHSA-32jq-jh7h-r7v4). const draft = draftFromQuads(quads, draftUrl, { refuseHidden: true }); @@ -309,8 +330,8 @@ export function createSolidReleaseDraftRepository({ }, async assemble(draftUrl, targetUrl, issued, version) { - const urls = await documentsOf(draftUrl); - if (!urls.includes(draftUrl)) throw new AppError("draftGone"); + const { urls, datasets } = + version === undefined ? await readDocuments(draftUrl) : readAt(draftUrl, version); const iriOf = draftIriOf(draftContainerOf(draftUrl), draftUrl); const inPlace = inPlaceIn(draftContainerOf(draftUrl), draftUrl); const to = (iri: string) => { @@ -319,9 +340,6 @@ export function createSolidReleaseDraftRepository({ if (isIriPastBound(there)) throw termTooLong(); return there; }; - const datasets = await Promise.all(urls.map((url) => readDataset(url, fetch))); - if (version !== undefined && versionOfDocuments(urls, datasets) !== version) - throw new AppError("changedElsewhere", { url: draftUrl }); // A statement of the release at its address already would say more of it than the draft does, which no editor shows (GHSA-p2pr-x6fw-hrfx). const atTargetAlready = (iri: string) => iri === targetUrl || iri.startsWith(`${targetUrl}#`); const atTarget = datasets.flatMap((dataset, at) => @@ -435,9 +453,29 @@ export function createSolidReleaseDraftRepository({ }; } -/** The version of each of a draft's documents, in order: a draft changed when any of them did, or one came or went. */ +/** The number of each dataset read, given once: see readNumberOf. */ +const READ_NUMBERS = new WeakMap(); +let readsNumbered = 0; + +/** + * A dataset's own number among those read. A document read again is + * another dataset, so another number, unless the pod answered that it + * is unchanged (304), which gives the dataset read before. Its ETag + * alone would not tell two reads apart: a pod may give none, or the + * same one for other content (GHSA-438x-f779-7chm). + */ +function readNumberOf(dataset: object): number { + let number = READ_NUMBERS.get(dataset); + if (number === undefined) { + number = ++readsNumbered; + READ_NUMBERS.set(dataset, number); + } + return number; +} + +/** The version of a read of a draft's documents, in order: another when any of them was read anew, or one came or went. */ function versionOfDocuments(urls: readonly string[], datasets: readonly Dataset[]): string { - return JSON.stringify(urls.map((url, at) => [url, versionOf(datasets[at]!)])); + return JSON.stringify(urls.map((url, at) => [url, readNumberOf(datasets[at]!)])); } /**