From fb6c113310fa7f8af62d2eaa9a8fa354bea0afcd Mon Sep 17 00:00:00 2001 From: Bryan Matthew Simonson <7519963+bryanmatthewsimonson@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:07:58 -0700 Subject: [PATCH] =?UTF-8?q?fix(archive):=20"Load=20archive"=20served=20the?= =?UTF-8?q?=20wrong=20article=20=E2=80=94=20gate=20on=20identity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reconstruct handler asked relays for {kinds:[30023], '#r':[url]} and rendered the newest hit, with no authors filter and no identity check. But `#r` is not an identity index: buildArticleEvent co-emits an indexed `r` for responds-to targets and for the first 25 OUTBOUND LINKS of every article, so an event matches merely by LINKING to the probed URL. Every article you publish poisons the `#r` index for up to 25 URLs it links to, and a cross-linked corpus maximizes it. The worst case needs no race: with no published capture of X, a linking article is the ONLY candidate — it wins 100% of the time, and altCount 0 means the "N relay versions found" tell never fires. loadArchivedArticle pins url: state.article.url, so the foreign body renders under X's address and claims made on it key to X. NIP_DRAFT.md already mandated the disambiguation this handler never did. articleAnswersTo (url-identity.js, pure + tested): an article answers to its identity URL (the first `r`, per the reconstruct invariant) and its capture_url mirror, and to nothing else. The handler reconstructs first, keeps only events that ARE the requested URL, and reports found: false when none survive. Probing from a mirror address still resolves. Normalization is applied only to an article's own addresses, on both sides. Normalizing the probe against the raw `#r` set instead would widen the over-match — the identity check and the normalization are one change. No wire-format change. 1737 tests green; lint 0 errors. Co-Authored-By: Claude Fable 5 --- docs/JOURNAL.md | 48 ++++++++++ src/background/index.js | 45 +++++++++- src/shared/url-identity.js | 45 ++++++++++ tests/archive-identity.test.mjs | 152 ++++++++++++++++++++++++++++++++ 4 files changed, 286 insertions(+), 4 deletions(-) create mode 100644 tests/archive-identity.test.mjs diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 77f4b73..637f00e 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -19,6 +19,54 @@ or files, and the "so-what" for future readers. --- +## 2026-07-17 — "Load archive" served the WRONG ARTICLE: `#r` is not an identity index + +Tags: `bug`, `design`. + +Reported from daily use: *"Once, I went ahead and clicked 'Load Archive' +and it loaded a completely different article."* It reproduces, it needs +no race, and the mechanism is entirely ours. + +`xray:archive:reconstruct` (`background/index.js`) asked relays for +`{kinds:[30023], '#r':[url], limit:20}`, sorted by `created_at` desc, +took `events[0]`, and reconstructed it — with **no authors filter and no +identity check**. But `buildArticleEvent` co-emits an indexed `r` tag for +`responds-to` targets and for the **first 25 outbound links** of every +article (`event-builder.js`, the `if (linked < 25) pushR(link.url)` +line). So **every article you publish poisons the `#r` index for up to 25 +URLs it merely links to.** A cross-linked corpus — which is the whole +point of the tool — maximizes the over-match. + +The worst case needs no concurrency at all: if you never published a +capture of X, a *linking* article is the **only** candidate, so it wins +**100% of the time**, and `altCount` is 0 — the "N relay versions found" +tell never fires. The swap is then disguised, because +`loadArchivedArticle` pins `url: state.article.url`: the foreign body +renders under X's address, and any claims or comments made on it key to +X. `docs/NIP_DRAFT.md` already *mandated* the disambiguation this handler +never did. + +**Fix:** `articleAnswersTo` in `url-identity.js` (pure, tested). An +article answers to exactly two addresses — its identity URL (the FIRST +`r`, per the `reconstructArticleFromEvent` invariant) and its +`capture_url` mirror. Everything else in the `r` set is a reference to +someone *else's* article. The handler now reconstructs first, keeps only +events that ARE the requested URL, and reports `found: false` when none +survive. + +**The trap worth recording:** link `r` targets are normalized +(`content-extractor.js`) while the primary `r` and `capture-url` are +emitted raw, and the probe URL is raw. Normalizing the *probe* against +the raw `#r` set — the obvious "fix" — would have made the bug fire +**more often**, not less. Normalization here is applied only to an +article's OWN addresses, on both sides. The identity check and the +normalization are one change and must not be separated. + +`tests/archive-identity.test.mjs` runs the real builder and asserts the +poisoned `r` tag IS present (so the relay filter really would return the +event) *before* asserting the gate rejects it — the repro is pinned, not +just the fix. + ## 2026-07-16 — xray-network stays out of WORKSPACE_DATABASES (25.2b) Tags: `design`. diff --git a/src/background/index.js b/src/background/index.js index fc79bc3..096750f 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -28,6 +28,7 @@ import { fetchSubstackPost, fetchSubstackComments } from '../shared/platforms/su import { handleScreenshotCapture } from '../shared/screenshot.js'; import { runSuggestionPass, runAuditPass, runAuditModulePass, getLlmConfig, runLensPass, getLensConfig, runCorpusMapPass, runCorpusReducePass, getCorpusConfig } from '../shared/llm-client.js'; import { pdfDocumentUrl } from '../shared/pdf-detect.js'; +import { articleAnswersTo } from '../shared/url-identity.js'; import { Signer } from '../shared/signer.js'; import { loadFlags, isEnabled } from '../shared/metadata/feature-flags.js'; import { publishConfirmed, IDENTITY_KINDS } from '../shared/confirmed-publish.js'; @@ -789,11 +790,47 @@ chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { sendResponse({ ok: true, found: false }); return; } + // `#r` is NOT an identity index, and treating it as one + // served the WRONG ARTICLE. buildArticleEvent co-emits an + // indexed `r` for `responds-to` targets and for the first + // 25 OUTBOUND LINKS of every article, so an event matches + // this filter merely by LINKING to `url` — and when no + // capture of `url` was ever published, a linking article + // is the only candidate, so it won every time, with + // altCount 0 offering no tell. The reader then rendered + // that foreign body under the requested URL. + // + // An article has exactly two addresses of its own: the + // primary `r` (the identity URL — reconstruct reads the + // FIRST `r`) and `capture-url` (the mirror it was fetched + // from). Everything else in the `r` set is a reference to + // someone ELSE's article. Reconstruct first, keep only + // events that ARE the requested URL, and let the rest of + // the pipeline see nothing. + // + // `articleAnswersTo` normalizes both sides — the primary + // `r` and `capture-url` are emitted raw while the probe + // URL comes from a live capture — but it only ever + // normalizes an article's OWN addresses. Normalizing the + // probe against the raw `#r` set instead would widen the + // over-match rather than close it, so the identity check + // and the normalization must stay together. + // Mandated by docs/NIP_DRAFT.md (archive reconstruction: + // the link/capture-url/responds-to tags disambiguate). + const candidates = []; + for (const ev of events) { + const rebuilt = EventBuilder.reconstructArticleFromEvent(ev); + if (!rebuilt || !articleAnswersTo(rebuilt, url)) continue; + candidates.push({ ev, article: rebuilt }); + } + if (candidates.length === 0) { + sendResponse({ ok: true, found: false }); + return; + } // Most recent event wins. kind-30023 is NIP-33 replaceable // per (pubkey, d-tag), so ties should be rare but real. - events.sort((a, b) => (b.created_at || 0) - (a.created_at || 0)); - const pick = events[0]; - const article = EventBuilder.reconstructArticleFromEvent(pick); + candidates.sort((a, b) => (b.ev.created_at || 0) - (a.ev.created_at || 0)); + const { ev: pick, article } = candidates[0]; sendResponse({ ok: true, found: true, @@ -801,7 +838,7 @@ chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { eventId: pick.id, authorPubkey: pick.pubkey, createdAt: pick.created_at, - altCount: events.length - 1 + altCount: candidates.length - 1 }); } catch (err) { sendResponse({ ok: false, error: err && err.message ? err.message : String(err) }); diff --git a/src/shared/url-identity.js b/src/shared/url-identity.js index f817b7a..8ab902f 100644 --- a/src/shared/url-identity.js +++ b/src/shared/url-identity.js @@ -434,4 +434,49 @@ export function rewriteArchivedLinks(links, ownHost) { return [...out.values()]; } +/** + * The URLs an article ANSWERS TO: its identity URL and, when the + * capture came from a mirror, the address it was actually fetched from + * (`capture_url`). Normalized, so an archive capture and a direct + * capture of the same piece answer to the same canonical strings. + * + * Deliberately NOT "every URL the event mentions". `buildArticleEvent` + * co-emits indexed `r` tags for `responds-to` targets and for the first + * 25 outbound links, so a relay `#r=` query also returns articles + * that merely REFERENCE `url` — someone else's article. Answering an + * archive probe with one of those renders a foreign body under the + * requested URL. These two fields are the only addresses that are the + * article itself. + * + * @param {{url?: string, capture_url?: string|null}} article + * @returns {string[]} normalized addresses, identity first, deduped + */ +export function articleAddresses(article) { + if (!article) return []; + const out = []; + for (const raw of [article.url, article.capture_url]) { + if (!raw || typeof raw !== 'string') continue; + let n = ''; + try { n = normalize(raw); } catch (_) { continue; } + if (n && !out.includes(n)) out.push(n); + } + return out; +} + +/** + * Is `article` the article located at `url` — as opposed to one that + * merely links to it? The identity gate for archive reconstruction. + * + * @param {{url?: string, capture_url?: string|null}} article + * @param {string} url + * @returns {boolean} + */ +export function articleAnswersTo(article, url) { + if (!url || typeof url !== 'string') return false; + let wanted = ''; + try { wanted = normalize(url); } catch (_) { return false; } + if (!wanted) return false; + return articleAddresses(article).includes(wanted); +} + export { ARCHIVE_TODAY_HOSTS as _ARCHIVE_TODAY_HOSTS }; diff --git a/tests/archive-identity.test.mjs b/tests/archive-identity.test.mjs new file mode 100644 index 0000000..e8447a1 --- /dev/null +++ b/tests/archive-identity.test.mjs @@ -0,0 +1,152 @@ +// Archive-reconstruction identity — the `#r` over-match that served +// the WRONG ARTICLE. +// +// `xray:archive:reconstruct` asks relays for `{kinds:[30023], +// '#r':[url]}` and renders the newest hit under `url`. But `#r` is not +// an identity index: buildArticleEvent co-emits an indexed `r` for +// `responds-to` targets and for the first 25 OUTBOUND LINKS of every +// article, so an event matches merely by LINKING to `url`. When no +// capture of `url` was ever published, a linking article is the ONLY +// candidate — it wins 100% of the time, with `altCount: 0` offering no +// tell — and its body renders under the requested URL, where claims and +// comments then key to it. +// +// `articleAnswersTo` is the gate: an article answers to its identity +// URL and its `capture_url` mirror address, and to nothing else. +// +// The end-to-end test below is the real repro: it runs the actual +// builder and the actual reconstruct inverse, asserts the poisoned `r` +// tag IS present (so the relay filter really would return this event), +// and then asserts the gate rejects it. + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +// EventBuilder transitively imports Storage, which probes +// `chrome.storage.local` at module-load time. +globalThis.chrome = globalThis.chrome || { + storage: { local: { get(_k, cb) { cb({}); }, set(_o, cb) { cb && cb(); }, remove(_k, cb) { cb && cb(); } } } +}; + +const { articleAddresses, articleAnswersTo } = await import('../src/shared/url-identity.js'); +const { EventBuilder } = await import('../src/shared/event-builder.js'); +const { normalize } = await import('../src/shared/metadata/url-normalizer.js'); + +const PUBKEY = '6daa7f3b0f5a4c8e9b2d1a7c3e5f80916d4b2a8c7e1f3059d8b6a4c2e0f19375'; + +// --- the gate, as a unit ------------------------------------------------------ + +test('an article answers to its identity URL', () => { + assert.equal(articleAnswersTo({ url: 'https://example.com/story' }, + 'https://example.com/story'), true); +}); + +test('an article answers to its capture_url mirror, so a probe from the mirror still finds it', () => { + // Original-as-identity (url-identity policy, 2026-07-09): the piece + // keys to the recovered original, and archive.is is provenance. A + // reader sitting ON the archive page probes with the archive + // address — that must still resolve to this article. + const archived = { + url: 'https://www.wsj.com/articles/three-researchers', + capture_url: 'https://archive.is/7gYpy' + }; + assert.equal(articleAnswersTo(archived, 'https://archive.is/7gYpy'), true); + assert.equal(articleAnswersTo(archived, 'https://www.wsj.com/articles/three-researchers'), true); + assert.deepEqual(articleAddresses(archived), [ + normalize('https://www.wsj.com/articles/three-researchers'), + normalize('https://archive.is/7gYpy') + ], 'identity first, mirror second, both normalized'); +}); + +test('an article does NOT answer to a URL it merely links to', () => { + const linking = { + url: 'https://substack.example/p/a-critique', + links: [{ url: 'https://example.com/story', text: 'the piece' }] + }; + assert.equal(articleAnswersTo(linking, 'https://example.com/story'), false, + 'linking to X does not make you X'); +}); + +test('normalization applies to BOTH sides — a tracking param does not fork identity', () => { + assert.equal(articleAnswersTo({ url: 'https://example.com/story' }, + 'https://example.com/story?utm_source=twitter'), true); + assert.equal(articleAnswersTo({ url: 'https://example.com/story?utm_source=rss' }, + 'https://example.com/story'), true); +}); + +test('a different path is not the same article, however similar', () => { + assert.equal(articleAnswersTo({ url: 'https://example.com/story-2' }, + 'https://example.com/story'), false); +}); + +test('junk degrades to false, never to a false match', () => { + assert.equal(articleAnswersTo(null, 'https://example.com/x'), false); + assert.equal(articleAnswersTo({}, 'https://example.com/x'), false); + assert.equal(articleAnswersTo({ url: 'https://example.com/x' }, ''), false); + assert.equal(articleAnswersTo({ url: 'https://example.com/x' }, null), false); + assert.equal(articleAnswersTo({ url: 123 }, 'https://example.com/x'), false); + // `normalize` is fail-open: an unparseable string passes through + // unchanged rather than throwing. So an unparseable address matches + // ITSELF (harmless and consistent — the probe URL always comes from + // a live capture), but never a real URL. + assert.equal(articleAnswersTo({ url: 'not a url' }, 'https://example.com/x'), false); + assert.deepEqual(articleAddresses(null), []); + assert.deepEqual(articleAddresses({ url: null, capture_url: null }), []); +}); + +test('addresses dedupe when capture_url equals the identity URL', () => { + assert.deepEqual( + articleAddresses({ url: 'https://example.com/a', capture_url: 'https://example.com/a' }), + [normalize('https://example.com/a')]); +}); + +// --- the repro: real builder → real inverse → the gate ------------------------- + +test('REPRO: a linking article carries the victim URL in `r`, yet is rejected by the gate', async () => { + const VICTIM = 'https://www.wsj.com/articles/three-researchers-sick'; + + // An ordinary article that merely cites the WSJ piece. Nothing + // unusual: this is what every commentary capture looks like. + const critique = { + url: 'https://substack.example/p/why-that-wsj-piece-is-wrong', + title: 'Why that WSJ piece is wrong', + content: '

The reporting rests on unnamed sources.

', + markdown: 'The reporting rests on unnamed sources.', + links: [{ url: VICTIM, text: 'the WSJ article', internal: false }] + }; + + const ev = await EventBuilder.buildArticleEvent(critique, [], PUBKEY); + + // 1. The poison is real: this event WOULD come back from + // `{kinds:[30023], '#r':[VICTIM]}`. + const rTags = ev.tags.filter((t) => t[0] === 'r').map((t) => t[1]); + assert.ok(rTags.includes(normalize(VICTIM)), + 'the outbound link co-emits an indexed r for the victim URL — this is the over-match'); + assert.equal(rTags[0], critique.url, + 'the FIRST r is still the article\'s own URL (reconstruct invariant)'); + + // 2. The inverse recovers the critique — NOT the WSJ piece. + const rebuilt = EventBuilder.reconstructArticleFromEvent(ev); + assert.equal(rebuilt.url, critique.url); + + // 3. The gate rejects it for the victim URL. Before this gate, the + // handler took events[0] and served THIS body under VICTIM. + assert.equal(articleAnswersTo(rebuilt, VICTIM), false, + 'the critique must never be served as the WSJ article'); + assert.equal(articleAnswersTo(rebuilt, critique.url), true, + 'and it must still be findable as itself'); +}); + +test('REPRO: a genuine capture of the victim URL still resolves', async () => { + const VICTIM = 'https://www.wsj.com/articles/three-researchers-sick'; + const real = { + url: VICTIM, + title: 'Three researchers fell ill', + content: '

Body.

', + markdown: 'Body.' + }; + const ev = await EventBuilder.buildArticleEvent(real, [], PUBKEY); + const rebuilt = EventBuilder.reconstructArticleFromEvent(ev); + assert.equal(articleAnswersTo(rebuilt, VICTIM), true, + 'the gate must not cost us the true positive'); +});