From cab7dad3d653be8b2530b2ef8aac1b26b4f73f34 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 14:05:36 +0200 Subject: [PATCH 1/4] test(privacy): cover reusing a synced private address's reference --- src/main.persistence.test.ts | 41 ++++++++++++++++++++++++++ src/services/FeedUrlRepository.test.ts | 35 ++++++++++++++++++++-- src/services/library.test.ts | 22 +++++++++++++- tests/e2e/sync-private-address.test.ts | 29 ++++++++++++++++-- 4 files changed, 121 insertions(+), 6 deletions(-) diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index 24fa6790..9639d145 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -263,6 +263,47 @@ describe("private addresses adopted from sync", () => { } }); + it("keeps one reference for an address that keeps arriving raw, also after a restart", async () => { + const plugin = makePlugin(); + await plugin.loadSettings(); + attachDataFileWriter(plugin); + const externalWrite = plugin.saveData.bind(plugin); + const save = vi.fn(externalWrite); + Object.assign(plugin, { saveData: save }); + const storage = plugin.app.secretStorage; + const address = "https://priv3.supercast.com/feeds/REPEATED_SENTINEL"; + + for (let incoming = 1; incoming <= 3; incoming++) { + await externalWrite({ + ...structuredClone(released2211), + savedFeeds: { Private: { title: "Private", url: address, artworkUrl: "" } }, + incoming, + }); + await Promise.all([ + plugin.onExternalSettingsChange(), + plugin.onExternalSettingsChange(), + ]); + await settled(); + expect(save).toHaveBeenCalledTimes(incoming); + } + + const disk = await plugin.loadData(); + expect(JSON.stringify(disk)).not.toContain("REPEATED_SENTINEL"); + const reference = disk.savedFeeds.Private.urlSecretId; + expect(storage.listSecrets()).toEqual([reference]); + + const restarted = makePlugin(vi.fn().mockResolvedValue(undefined), { + dataFileExists: true, + }); + Object.assign(restarted, { app: { ...restarted.app, secretStorage: storage } }); + await restarted.saveData(disk); + await restarted.loadSettings(); + expect(storage.listSecrets()).toEqual([reference]); + expect(resolveFeedUrl(restarted.settings.savedFeeds.Private, restarted.feedUrls)).toBe( + address, + ); + }); + it("protects a copied address even when the decoded document equals memory", async () => { const plugin = makePlugin(); const address = "https://private.example/feed.xml?auth=COPIED_ADOPTION_SENTINEL"; diff --git a/src/services/FeedUrlRepository.test.ts b/src/services/FeedUrlRepository.test.ts index a07df432..d85ae895 100644 --- a/src/services/FeedUrlRepository.test.ts +++ b/src/services/FeedUrlRepository.test.ts @@ -29,11 +29,40 @@ describe("FeedUrlRepository", () => { expect(values.get(id)).toBe("https://p.example/rss?auth=tok"); }); - it("allocates distinct IDs even for identical URLs (references sync, secrets don't)", () => { - const { api } = storage(); + it("stores a URL this device holds under its reference, and another URL under its own", () => { + const { api, values } = storage(); const repository = new FeedUrlRepository(api); const first = repository.store("https://a.example/rss?auth=1"); - const second = repository.store("https://a.example/rss?auth=1"); + expect(repository.store(" https://a.example/rss?auth=1 ")).toBe(first); + const other = repository.store("https://a.example/rss?auth=2"); + expect(other).not.toBe(first); + expect([...values]).toEqual([ + [first, "https://a.example/rss?auth=1"], + [other, "https://a.example/rss?auth=2"], + ]); + }); + + it("never reuses a cleared reference or one holding another URL on this device", () => { + const elsewhere = "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef"; + const { api, values } = storage({ [elsewhere]: "https://a.example/rss?auth=other" }); + const repository = new FeedUrlRepository(api); + const removed = repository.store("https://a.example/rss?auth=1"); + repository.delete(removed); + + const id = repository.store("https://a.example/rss?auth=1"); + + expect([removed, elsewhere]).not.toContain(id); + expect([...values]).toEqual([ + [elsewhere, "https://a.example/rss?auth=other"], + [removed, ""], + [id, "https://a.example/rss?auth=1"], + ]); + }); + + it("allocates IDs that two devices storing one URL never share (references sync, secrets don't)", () => { + const url = "https://a.example/rss?auth=1"; + const first = new FeedUrlRepository(storage().api).store(url); + const second = new FeedUrlRepository(storage().api).store(url); expect(first).not.toBe(second); expect(isFeedUrlSecretId(first)).toBe(true); expect(isFeedUrlSecretId(second)).toBe(true); diff --git a/src/services/library.test.ts b/src/services/library.test.ts index 89020231..7e4de3ca 100644 --- a/src/services/library.test.ts +++ b/src/services/library.test.ts @@ -13,7 +13,7 @@ import { deferred, MemoryStorage } from "../../tests/mocks/feeds"; import { FeedRepository, loadFeed } from "./FeedRepository"; import { FeedUrlRepository } from "./FeedUrlRepository"; import { createLibraryActions } from "./library"; -import { resolveFeedUrl } from "./privateFeeds"; +import { protectPrivateFeeds, resolveFeedUrl } from "./privateFeeds"; vi.mock("src/utility/networkRequest", async (importOriginal) => { const actual = await importOriginal(); @@ -203,6 +203,26 @@ describe("removing a podcast", () => { expect(get(savedFeeds)).toEqual({}); expect(secrets.getSecret(reference)).toBeFalsy(); }); + + test("keeps the address of a podcast protected with the same private URL as a removed one", () => { + const { actions, feedUrls } = actionsOn(new obsidian.SecretStorage()); + const raw = { title: "Private Show", url: PRIVATE_URL, artworkUrl: "" }; + const { settings } = protectPrivateFeeds( + { + ...DEFAULT_SETTINGS, + savedFeeds: { "Private Show": raw, Alias: { ...raw, title: "Alias" } }, + }, + feedUrls, + ); + savedFeeds.set(settings.savedFeeds); + expect(settings.savedFeeds.Alias.urlSecretId).toBe( + settings.savedFeeds["Private Show"].urlSecretId, + ); + + actions.removePodcast("Alias"); + + expect(resolveFeedUrl(get(savedFeeds)["Private Show"], feedUrls)).toBe(PRIVATE_URL); + }); }); describe("undoing Remove podcast", () => { diff --git a/tests/e2e/sync-private-address.test.ts b/tests/e2e/sync-private-address.test.ts index 9a1feb61..b143f92d 100644 --- a/tests/e2e/sync-private-address.test.ts +++ b/tests/e2e/sync-private-address.test.ts @@ -12,8 +12,13 @@ import { jsLiteral, RELOAD_OPTIONS, WAIT_OPTS } from "./runtime"; const getContext = createPodNotesE2EHarness("sync-private-address"); +const feedUrlSecretIds = (obsidian: ReturnType["obsidian"]) => + obsidian.dev.evalJson( + `app.secretStorage.listSecrets().filter((id) => id.startsWith("podnotes-feed-url-"))`, + ); + test.each(["raw event", "external settings handler"])( - "protects a synced 2.21.1 private address on disk once per incoming change through %s", + "protects a synced 2.21.1 private address on disk once per incoming change, under one reference, through %s", async (trigger) => { const { obsidian, plugin } = getContext(); await plugin.updateDataAndReload>((data) => { @@ -46,6 +51,7 @@ test.each(["raw event", "external settings handler"])( copiedNote: `from ${address}`, }; delete data.currentEpisode; + const before = await feedUrlSecretIds(obsidian); const evidence = []; for (let incoming = 1; incoming <= 3; incoming++) { const recorder = await recordWrites(obsidian); @@ -77,13 +83,32 @@ test.each(["raw event", "external settings handler"])( expect(writes).toMatchObject({ attempted: 1, completed: 1, failed: 0, intact: true }); evidence.push({ incoming, writes, sentinelOnDisk: text.includes(sentinel) }); } + const disk = await plugin + .data<{ savedFeeds: { Private: { urlSecretId: string } } }>() + .read(); + const listed = await feedUrlSecretIds(obsidian); + expect(listed.filter((id) => !before.includes(id))).toEqual([ + disk.savedFeeds.Private.urlSecretId, + ]); + await plugin.reload(RELOAD_OPTIONS); + expect(await feedUrlSecretIds(obsidian)).toEqual(listed); + expect( + await obsidian.dev.evalJson( + "(() => { const p = app.plugins.plugins.podnotes; return p.feedUrls.resolve(p.settings.savedFeeds.Private.urlSecretId); })()", + ), + ).toBe(address); await showLibrary(obsidian); expect(await shownPodcasts(obsidian)).toContain("Private"); const directory = resolve(".obsidian-e2e-artifacts/fix488"); await mkdir(directory, { recursive: true }); await obsidian.dev.screenshot(resolve(directory, `${trigger.replaceAll(" ", "-")}.png`)); console.info( - JSON.stringify({ trigger, evidence, version: await obsidian.exec("version") }), + JSON.stringify({ + trigger, + evidence, + feedUrlSecretIds: { before: before.length, after: listed.length }, + version: await obsidian.exec("version"), + }), ); }, ); From 7acf2c777be83deee1f1e82bd7e8fe7d3fdbfce5 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 14:07:56 +0200 Subject: [PATCH 2/4] fix(privacy): reuse the reference of a private address this device holds --- src/services/FeedUrlRepository.test.ts | 8 ++++++-- src/services/FeedUrlRepository.ts | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/services/FeedUrlRepository.test.ts b/src/services/FeedUrlRepository.test.ts index d85ae895..be8bd905 100644 --- a/src/services/FeedUrlRepository.test.ts +++ b/src/services/FeedUrlRepository.test.ts @@ -42,9 +42,12 @@ describe("FeedUrlRepository", () => { ]); }); - it("never reuses a cleared reference or one holding another URL on this device", () => { + it("never reuses a cleared reference, one holding another URL here, or a foreign ID", () => { const elsewhere = "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef"; - const { api, values } = storage({ [elsewhere]: "https://a.example/rss?auth=other" }); + const { api, values } = storage({ + "other-plugin-feed": "https://a.example/rss?auth=1", + [elsewhere]: "https://a.example/rss?auth=other", + }); const repository = new FeedUrlRepository(api); const removed = repository.store("https://a.example/rss?auth=1"); repository.delete(removed); @@ -53,6 +56,7 @@ describe("FeedUrlRepository", () => { expect([removed, elsewhere]).not.toContain(id); expect([...values]).toEqual([ + ["other-plugin-feed", "https://a.example/rss?auth=1"], [elsewhere, "https://a.example/rss?auth=other"], [removed, ""], [id, "https://a.example/rss?auth=1"], diff --git a/src/services/FeedUrlRepository.ts b/src/services/FeedUrlRepository.ts index 1a293025..c2fe0a38 100644 --- a/src/services/FeedUrlRepository.ts +++ b/src/services/FeedUrlRepository.ts @@ -16,6 +16,11 @@ import { isValidSecretId } from "src/types/Credentials"; * one reference names each device's own copy. Content-free by construction - * neither the feed name nor the URL leaks through the ID namespace, which * `listSecrets` exposes. + * + * SecretStorage cannot delete an ID, only clear its value, so storing a URL + * this device already holds reuses that ID. Only an ID whose value here is + * exactly the URL qualifies. A cleared ID stays retired, since another device + * may still reference it. */ const FEED_URL_SECRET_ID = /^podnotes-feed-url-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/; @@ -45,9 +50,15 @@ export class FeedUrlRepository { } } - /** Store a private feed URL under a fresh PodNotes-owned ID and verify the readback. */ + /** Store a private feed URL under its ID here, else a fresh one, and verify the readback. */ store(url: string): string { - const id = `podnotes-feed-url-${crypto.randomUUID()}`; + const value = url.trim(); + const id = + this.storage + .listSecrets() + .find( + (held) => isFeedUrlSecretId(held) && this.storage.getSecret(held) === value, + ) ?? `podnotes-feed-url-${crypto.randomUUID()}`; this.attach(id, url); return id; } From de18bb8606e844f1375d232b74e692478788e15e Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 14:29:03 +0200 Subject: [PATCH 3/4] fix(privacy): return a held private address reference without rewriting it --- src/main.ts | 4 ++-- src/services/FeedUrlRepository.test.ts | 2 ++ src/services/FeedUrlRepository.ts | 18 +++++++++++------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main.ts b/src/main.ts index a19028bf..181aa9a2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -521,8 +521,8 @@ export default class PodNotes extends Plugin implements IPodNotes { } private dataFileAccess(): DataFileAccess { - // As found: reading a private feed URL again stores it under a new reference, so - // the same file never reads as the same document twice. + // The file as found, before decoding protects or repairs it. A readback still equal + // to it means the write did not land. let lastRead: unknown; return { read: async (base): Promise> => { diff --git a/src/services/FeedUrlRepository.test.ts b/src/services/FeedUrlRepository.test.ts index be8bd905..db837c69 100644 --- a/src/services/FeedUrlRepository.test.ts +++ b/src/services/FeedUrlRepository.test.ts @@ -40,6 +40,7 @@ describe("FeedUrlRepository", () => { [first, "https://a.example/rss?auth=1"], [other, "https://a.example/rss?auth=2"], ]); + expect(api.setSecret).toHaveBeenCalledTimes(2); }); it("never reuses a cleared reference, one holding another URL here, or a foreign ID", () => { @@ -54,6 +55,7 @@ describe("FeedUrlRepository", () => { const id = repository.store("https://a.example/rss?auth=1"); + expect(() => repository.store(" ")).toThrow(/must not be empty/); expect([removed, elsewhere]).not.toContain(id); expect([...values]).toEqual([ ["other-plugin-feed", "https://a.example/rss?auth=1"], diff --git a/src/services/FeedUrlRepository.ts b/src/services/FeedUrlRepository.ts index c2fe0a38..1d5fc082 100644 --- a/src/services/FeedUrlRepository.ts +++ b/src/services/FeedUrlRepository.ts @@ -17,10 +17,10 @@ import { isValidSecretId } from "src/types/Credentials"; * neither the feed name nor the URL leaks through the ID namespace, which * `listSecrets` exposes. * - * SecretStorage cannot delete an ID, only clear its value, so storing a URL - * this device already holds reuses that ID. Only an ID whose value here is - * exactly the URL qualifies. A cleared ID stays retired, since another device - * may still reference it. + * SecretStorage's API has no delete, so a cleared ID stays listed for good. + * Storing a URL this device already holds therefore returns the ID holding it. + * Only an ID whose value here is exactly the URL qualifies. A cleared ID stays + * retired, since another device may still reference it. */ const FEED_URL_SECRET_ID = /^podnotes-feed-url-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/; @@ -53,12 +53,16 @@ export class FeedUrlRepository { /** Store a private feed URL under its ID here, else a fresh one, and verify the readback. */ store(url: string): string { const value = url.trim(); - const id = + const held = + value && this.storage .listSecrets() .find( - (held) => isFeedUrlSecretId(held) && this.storage.getSecret(held) === value, - ) ?? `podnotes-feed-url-${crypto.randomUUID()}`; + (candidate) => + isFeedUrlSecretId(candidate) && this.storage.getSecret(candidate) === value, + ); + if (held) return held; + const id = `podnotes-feed-url-${crypto.randomUUID()}`; this.attach(id, url); return id; } From 3b25e6389a4b656f808b1ed2021e1f68946b6008 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Thu, 15 Oct 2026 14:37:07 +0200 Subject: [PATCH 4/4] fix(privacy): match a held private address through resolve --- src/services/FeedUrlRepository.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/services/FeedUrlRepository.ts b/src/services/FeedUrlRepository.ts index 1d5fc082..6f0efc95 100644 --- a/src/services/FeedUrlRepository.ts +++ b/src/services/FeedUrlRepository.ts @@ -19,8 +19,8 @@ import { isValidSecretId } from "src/types/Credentials"; * * SecretStorage's API has no delete, so a cleared ID stays listed for good. * Storing a URL this device already holds therefore returns the ID holding it. - * Only an ID whose value here is exactly the URL qualifies. A cleared ID stays - * retired, since another device may still reference it. + * Only a PodNotes ID that resolves here to that URL qualifies. A cleared ID + * stays retired, since another device may still reference it. */ const FEED_URL_SECRET_ID = /^podnotes-feed-url-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/; @@ -52,15 +52,9 @@ export class FeedUrlRepository { /** Store a private feed URL under its ID here, else a fresh one, and verify the readback. */ store(url: string): string { - const value = url.trim(); - const held = - value && - this.storage - .listSecrets() - .find( - (candidate) => - isFeedUrlSecretId(candidate) && this.storage.getSecret(candidate) === value, - ); + const held = this.storage + .listSecrets() + .find((candidate) => this.resolve(candidate) === url.trim()); if (held) return held; const id = `podnotes-feed-url-${crypto.randomUUID()}`; this.attach(id, url);