From dffbe450e42e6a61d306845f5e533d2e77669b0f Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Tue, 13 Oct 2026 01:26:46 +0200 Subject: [PATCH 1/8] test(feeds): reproduce duplicate subscriptions and first-write secret leaks --- src/persistence/podNotesData.test.ts | 36 ++++++++++++++++++++++++ src/services/privateFeeds.test.ts | 16 ++++++++++- src/ui/settings/PodcastQueryGrid.test.ts | 16 +++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index 80046dfd..9e27628e 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -60,6 +60,42 @@ function settingsWithEveryEpisodeContainer(): IPodNotesSettings { } describe("PodNotes data schema", () => { + it("protects private feed URLs in every episode container on the first encode without a device secret", () => { + const sentinel = "PRIV2_ENCODE_SENTINEL"; + const rawUrl = `https://private.example/feed?auth=${sentinel}`; + const item = { ...episode("Private"), url: rawUrl, feedUrl: rawUrl }; + const history = { + title: item.title, + podcastName: item.podcastName, + time: 12, + duration: 180, + finished: false, + episode: item, + }; + const settings = { + ...structuredClone(DEFAULT_SETTINGS), + currentEpisode: item, + queue: playlist("Queue", item), + favorites: playlist("Favorites", item), + localFiles: playlist("Local files", { ...item, streamUrl: "audio.mp3" }), + playlists: { Custom: playlist("Custom", item) }, + downloadedEpisodes: { + "Test Podcast": [{ ...item, filePath: "audio.mp3", size: 42 }], + }, + playedEpisodes: { "Test Podcast::Private": history }, + } satisfies IPodNotesSettings; + const before = structuredClone(settings); + + const encoded = encodePodNotesData(settings); + + expect(JSON.stringify(encoded)).not.toContain(sentinel); + expect(encoded.currentEpisode?.feedUrl).toBe("podnotes-private-feed:Test%20Podcast"); + expect(encoded.queue.episodes[0].streamUrl).toBe(item.streamUrl); + expect(encoded.downloadedEpisodes["Test Podcast"][0].filePath).toBe("audio.mp3"); + expect(encoded.playedEpisodes["Test Podcast::Private"].time).toBe(12); + expect(settings).toEqual(before); + }); + it("loads fresh data as independent deep-cloned defaults", () => { const first = decodePodNotesData(undefined); const second = decodePodNotesData(null); diff --git a/src/services/privateFeeds.test.ts b/src/services/privateFeeds.test.ts index e683699a..95068394 100644 --- a/src/services/privateFeeds.test.ts +++ b/src/services/privateFeeds.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import type { SecretStorage } from "obsidian"; +import * as obsidian from "obsidian"; import { DEFAULT_SETTINGS } from "src/constants"; import { decodePodNotesData, @@ -9,7 +10,12 @@ import { import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import type { PodcastFeed } from "src/types/PodcastFeed"; import { FeedUrlRepository } from "./FeedUrlRepository"; -import { internPrivateFeed, protectPrivateFeeds, resolveFeedUrl } from "./privateFeeds"; +import { + internPrivateFeed, + notifyPrivateFeedUnavailable, + protectPrivateFeeds, + resolveFeedUrl, +} from "./privateFeeds"; function repository(initial: Record = {}) { const values = new Map(Object.entries(initial)); @@ -29,6 +35,14 @@ function feed(overrides: Partial = {}): PodcastFeed { const PRIVATE_URL = "https://www.patreon.com/rss/show?auth=se-cret"; +it("reports a missing device address in one sentence without destructive recovery instructions", () => { + const notice = vi.spyOn(obsidian, "Notice"); + notifyPrivateFeedUnavailable("My Show"); + expect(notice).toHaveBeenCalledWith( + 'The private feed address for "My Show" is not available on this device.', + ); +}); + describe("internPrivateFeed", () => { it("moves a credential-bearing URL into SecretStorage and persists a placeholder", () => { const { feedUrls, values } = repository(); diff --git a/src/ui/settings/PodcastQueryGrid.test.ts b/src/ui/settings/PodcastQueryGrid.test.ts index c5307a34..dee818ac 100644 --- a/src/ui/settings/PodcastQueryGrid.test.ts +++ b/src/ui/settings/PodcastQueryGrid.test.ts @@ -197,6 +197,22 @@ describe("PodcastQueryGrid", () => { expect(queryByRole("button", { name: "Add Private Show podcast" })).toBeNull(); }); + test("a changed channel title still removes the existing private subscription by its saved key", async () => { + const { saved, secrets } = followPrivateShow(); + fetchTextMock.mockResolvedValue( + "Renamed ShowPatron Host", + ); + + const { findByText, getByRole, queryByRole } = paste(PRIVATE_URL); + await findByText("Patron Host"); + + expect(queryByRole("button", { name: "Add Renamed Show podcast" })).toBeNull(); + expect(get(savedFeeds)).toEqual({ "Private Show": saved }); + await fireEvent.click(getByRole("button", { name: "Remove Renamed Show podcast" })); + expect(get(savedFeeds)).toEqual({}); + expect(secrets.getSecret(saved.urlSecretId)).toBeFalsy(); + }); + test("shows a private feed followed with an uppercase host as saved when its URL is pasted", async () => { followPrivateShow(PRIVATE_URL.replace("www.patreon.com", "WWW.PATREON.COM")); From 1014152cea1fc918752bfcd0351d71b1d8cfed26 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Tue, 13 Oct 2026 01:30:39 +0200 Subject: [PATCH 2/8] test(feeds): preserve episode containers while checking URL privacy --- src/persistence/podNotesData.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index 9e27628e..4891d062 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -90,6 +90,20 @@ describe("PodNotes data schema", () => { expect(JSON.stringify(encoded)).not.toContain(sentinel); expect(encoded.currentEpisode?.feedUrl).toBe("podnotes-private-feed:Test%20Podcast"); + for (const list of [ + encoded.queue, + encoded.favorites, + encoded.localFiles, + encoded.playlists.Custom, + ]) { + expect(list.episodes).toHaveLength(1); + expect(list.episodes[0]).toMatchObject({ + title: item.title, + url: "podnotes-private-feed:Test%20Podcast", + feedUrl: "podnotes-private-feed:Test%20Podcast", + }); + expect(list.currentEpisode?.feedUrl).toBe("podnotes-private-feed:Test%20Podcast"); + } expect(encoded.queue.episodes[0].streamUrl).toBe(item.streamUrl); expect(encoded.downloadedEpisodes["Test Podcast"][0].filePath).toBe("audio.mp3"); expect(encoded.playedEpisodes["Test Podcast::Private"].time).toBe(12); From b6244c9cd91fe30e8673b96435715b98365ffc65 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Tue, 13 Oct 2026 01:45:18 +0200 Subject: [PATCH 3/8] test(feeds): reproduce second-device recovery, OPML revival and sweep losses Set feed address must keep the synced reference, OPML must reuse a saved private feed's reference and never let a public namesake replace it, the orphan sweep must keep every ID data.json references, and released 2.20.0 and 2.21.1 libraries must load unchanged. --- src/main.persistence.test.ts | 108 +++++++++++++++++++++++ src/opml.test.ts | 47 ++++++++-- src/persistence/podNotesData.test.ts | 38 +++++++- src/ui/settings/PodcastQueryGrid.test.ts | 53 ++++++++++- 4 files changed, 233 insertions(+), 13 deletions(-) diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index ebb710c3..003ee8b2 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -305,6 +305,114 @@ describe("PodNotes persistence integration", () => { expect(saveData).toHaveBeenCalledTimes(writes); }); + it("keeps every private feed secret data.json references, even where this version cannot read it", async () => { + const unknownFieldId = "podnotes-feed-url-00000000-0000-4000-8000-000000000001"; + const droppedFeedId = "podnotes-feed-url-00000000-0000-4000-8000-000000000002"; + const orphanId = "podnotes-feed-url-00000000-0000-4000-8000-000000000003"; + const { storage, values } = memorySecretStorage({ + [unknownFieldId]: "https://private.example/a.xml?auth=a", + [droppedFeedId]: "https://private.example/b.xml?auth=b", + [orphanId]: "https://private.example/c.xml?auth=c", + }); + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { + app: { ...plugin.app, secretStorage: storage }, + loadData: vi.fn().mockResolvedValue({ + ...encodePodNotesData(structuredClone(DEFAULT_SETTINGS), { libraryId: "library" }), + savedFeeds: [ + { title: "B", url: "podnotes-private-feed:B", urlSecretId: droppedFeedId }, + ], + futureFeeds: { + A: { url: "podnotes-private-feed:A", urlSecretId: ` ${unknownFieldId} ` }, + }, + }), + }); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + + await plugin.loadSettings(); + + expect(saveData).not.toHaveBeenCalled(); + expect(values.get(unknownFieldId)).toBe("https://private.example/a.xml?auth=a"); + expect(values.get(droppedFeedId)).toBe("https://private.example/b.xml?auth=b"); + expect(values.get(orphanId)).toBe(""); + }); + + it.each([ + ["2.20.0", "- {{time}} ", {}], + ["2.21.1", "- {{linktime}} ", { downloadedByPodNotes: true }], + ])("loads a %s library with a private feed unchanged", async (_version, template, download) => { + const id = "podnotes-feed-url-00000000-0000-4000-8000-000000000001"; + const secretUrl = "https://private.example/feed.xml?auth=released"; + const { storage, values } = memorySecretStorage({ [id]: secretUrl }); + const episode = { + title: "Ep", + streamUrl: "https://media.example/ep.mp3?sig=media", + url: "podnotes-private-feed:Private", + description: "", + content: "", + podcastName: "Private", + episodeDate: "2026-01-01T00:00:00.000Z", + feedUrl: "podnotes-private-feed:Private", + }; + const released = { + ...JSON.parse(JSON.stringify(encodePodNotesData(structuredClone(DEFAULT_SETTINGS)))), + timestamp: { template, offset: 0 }, + savedFeeds: { + Private: { + title: "Private", + url: "podnotes-private-feed:Private", + urlSecretId: id, + artworkUrl: "https://media.example/art.jpg?sig=art", + }, + Public: { + title: "Public", + url: "https://feeds.example/public.xml", + artworkUrl: "", + }, + }, + queue: { ...DEFAULT_SETTINGS.queue, episodes: [episode] }, + currentEpisode: episode, + downloadedEpisodes: { + Private: [ + { ...episode, filePath: "PodNotes/Private/Ep.mp3", size: 42, ...download }, + ], + }, + playedEpisodes: { + "Private::Ep": { + title: "Ep", + podcastName: "Private", + time: 30, + duration: 60, + finished: false, + }, + }, + }; + delete released.libraryId; + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { + app: { ...plugin.app, secretStorage: storage }, + loadData: vi.fn().mockResolvedValue(structuredClone(released)), + }); + + await plugin.loadSettings(); + attachDataFileWriter(plugin); + plugin.settings.defaultVolume = 0.4; + await plugin.saveSettingsStrict(); + + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData.mock.calls[0][0]).toEqual({ + ...released, + defaultVolume: 0.4, + libraryId: "", + }); + expect(values.get(id)).toBe(secretUrl); + expect(plugin.feedUrls.resolve(plugin.settings.savedFeeds.Private.urlSecretId ?? "")).toBe( + secretUrl, + ); + }); + it("starts on an empty data.json as on a new install", async () => { const plugin = makePlugin(undefined, { dataFileExists: true }); let text = ""; diff --git a/src/opml.test.ts b/src/opml.test.ts index a7bf503d..a11b11fc 100644 --- a/src/opml.test.ts +++ b/src/opml.test.ts @@ -37,6 +37,9 @@ vi.mock("./store", () => ({ feedUrlSecrets.set("podnotes-feed-url", url); return "podnotes-feed-url"; }, + attach: (id: string, url: string) => { + feedUrlSecrets.set(id, url); + }, delete: (id: string) => feedUrlSecrets.delete(id), }, }); @@ -253,12 +256,16 @@ describe("importOPML (IE-01)", () => { expect(noticeMessages.some((m) => m.includes("Skipped 1 existing podcasts"))).toBe(true); }); - it("gives a saved private feed whose URL is not on this device the listed URL", async () => { + it("sets the listed private URL under the reference of a saved private feed missing on this device", async () => { const privateUrl = "https://private.test/feed?auth=s3cret"; feedUrlSecrets.clear(); - savedFeeds.set({ - Private: { title: "Private", url: "podnotes-private-feed:Private", artworkUrl: "" }, - }); + const saved = { + title: "Private", + url: "podnotes-private-feed:Private", + urlSecretId: "podnotes-feed-url-00000000-0000-4000-8000-000000000001", + artworkUrl: "", + }; + savedFeeds.set({ Private: saved }); channelAt.mockImplementation(async ({ url }: { url: string }) => ({ title: "Private", url, @@ -269,10 +276,34 @@ describe("importOPML (IE-01)", () => { ``, ); - const revived = get(savedFeeds).Private; - expect(Object.keys(get(savedFeeds))).toEqual(["Private"]); - expect(revived.url).toBe("podnotes-private-feed:Private"); - expect(feedUrlSecrets.get(revived.urlSecretId as string)).toBe(privateUrl); + expect(get(savedFeeds)).toEqual({ Private: saved }); + expect([...feedUrlSecrets]).toEqual([[saved.urlSecretId, privateUrl]]); + }); + + it("never replaces a saved private feed missing on this device with a public namesake", async () => { + feedUrlSecrets.clear(); + const saved = { + title: "Private", + url: "podnotes-private-feed:Private", + urlSecretId: "podnotes-feed-url-00000000-0000-4000-8000-000000000001", + artworkUrl: "", + }; + savedFeeds.set({ Private: saved }); + channelAt.mockImplementation(async ({ url }: { url: string }) => ({ + title: "Private", + url, + artworkUrl: "", + })); + + await importOPML( + '', + ); + + expect(get(savedFeeds)).toEqual({ Private: saved }); + expect(feedUrlSecrets.size).toBe(0); + expect(noticeMessages.some((m) => m.includes("Skipped 1 with duplicate titles"))).toBe( + true, + ); }); it("never renders 'NaN%' when every feed is already subscribed", async () => { diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index 4891d062..c3d05d42 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -60,7 +60,7 @@ function settingsWithEveryEpisodeContainer(): IPodNotesSettings { } describe("PodNotes data schema", () => { - it("protects private feed URLs in every episode container on the first encode without a device secret", () => { + it("puts no private feed address in data.json, from the first encode and without a device secret", () => { const sentinel = "PRIV2_ENCODE_SENTINEL"; const rawUrl = `https://private.example/feed?auth=${sentinel}`; const item = { ...episode("Private"), url: rawUrl, feedUrl: rawUrl }; @@ -84,11 +84,21 @@ describe("PodNotes data schema", () => { }, playedEpisodes: { "Test Podcast::Private": history }, } satisfies IPodNotesSettings; - const before = structuredClone(settings); + settings.queue.episodes[0] = { ...item, sourceFeed: rawUrl } as Episode; + const unknownFields = { libraryId: "library", nextVersion: { lastEpisode: item } }; + const before = structuredClone({ settings, unknownFields }); - const encoded = encodePodNotesData(settings); + const encoded = encodePodNotesData(settings, unknownFields); expect(JSON.stringify(encoded)).not.toContain(sentinel); + expect(encoded.queue.episodes[0].sourceFeed).toBe("podnotes-private-feed:Test%20Podcast"); + expect(encoded.nextVersion).toEqual({ + lastEpisode: { + ...item, + url: "podnotes-private-feed:Test%20Podcast", + feedUrl: "podnotes-private-feed:Test%20Podcast", + }, + }); expect(encoded.currentEpisode?.feedUrl).toBe("podnotes-private-feed:Test%20Podcast"); for (const list of [ encoded.queue, @@ -107,7 +117,27 @@ describe("PodNotes data schema", () => { expect(encoded.queue.episodes[0].streamUrl).toBe(item.streamUrl); expect(encoded.downloadedEpisodes["Test Podcast"][0].filePath).toBe("audio.mp3"); expect(encoded.playedEpisodes["Test Podcast::Private"].time).toBe(12); - expect(settings).toEqual(before); + expect({ settings, unknownFields }).toEqual(before); + }); + + it("keeps signed media and an episode's own tokened link of a private feed's episode", () => { + const signed = { + ...episode("Signed"), + streamUrl: "https://media.example/ep.mp3?sig=MEDIA&token=MEDIA", + url: "https://private.example/post?token=ARTICLE", + artworkUrl: "https://media.example/art.jpg?signature=ART", + chaptersUrl: "https://media.example/chapters.json?key=CHAPTERS", + feedUrl: "https://private.example/feed?auth=FEED", + }; + const settings = { ...structuredClone(DEFAULT_SETTINGS), currentEpisode: signed }; + + const encoded = encodePodNotesData(settings); + + expect(encoded.currentEpisode).toEqual({ + ...signed, + episodeDate: episodeDate.toISOString(), + feedUrl: "podnotes-private-feed:Test%20Podcast", + }); }); it("loads fresh data as independent deep-cloned defaults", () => { diff --git a/src/ui/settings/PodcastQueryGrid.test.ts b/src/ui/settings/PodcastQueryGrid.test.ts index dee818ac..9bed7b48 100644 --- a/src/ui/settings/PodcastQueryGrid.test.ts +++ b/src/ui/settings/PodcastQueryGrid.test.ts @@ -63,7 +63,19 @@ function usePlugin(feedUrls: FeedUrlRepository | object) { cache: () => ({ enabled: false, ttlHours: 6 }), now: Date.now, }); - plugin.set({ feedUrls, feeds } as unknown as PodNotes); + plugin.set({ app: new obsidian.App(), feedUrls, feeds } as unknown as PodNotes); +} + +async function setFeedAddress(view: ReturnType, title: string, url: string) { + await fireEvent.click( + await view.findByRole("button", { name: `Set feed address for ${title}` }), + ); + const dialog = within(document.body); + await fireEvent.input(dialog.getByRole("textbox", { name: "Feed address" }), { + target: { value: url }, + }); + await fireEvent.click(dialog.getByRole("button", { name: "Save" })); + await settle(); } // Runs the debounced handler and every response already resolved. @@ -257,6 +269,45 @@ describe("PodcastQueryGrid", () => { expect(thisDevice.listSecrets()).toEqual([]); }); + test("sets a synced private feed's address on this device under the reference data.json holds", async () => { + const { saved, secrets: firstDevice } = followPrivateShow(); + const thisDevice = new obsidian.SecretStorage(); + usePlugin(new FeedUrlRepository(thisDevice)); + + const view = renderGrid(); + await setFeedAddress(view, "Private Show", PRIVATE_URL); + + expect(thisDevice.listSecrets()).toEqual([saved.urlSecretId]); + expect(thisDevice.getSecret(saved.urlSecretId)).toBe(PRIVATE_URL); + expect(firstDevice.getSecret(saved.urlSecretId)).toBe(PRIVATE_URL); + expect(get(savedFeeds)).toEqual({ "Private Show": saved }); + expect( + view.queryByRole("button", { name: "Set feed address for Private Show" }), + ).toBeNull(); + expect( + view.getByRole("button", { name: "Remove Private Show podcast" }), + ).toBeInTheDocument(); + }); + + test("keeps the reference when the feed at the entered address cannot be loaded", async () => { + const notice = vi.spyOn(obsidian, "Notice"); + vi.spyOn(console, "error").mockImplementation(() => {}); + const { saved } = followPrivateShow(); + const thisDevice = new obsidian.SecretStorage(); + usePlugin(new FeedUrlRepository(thisDevice)); + fetchTextMock.mockRejectedValue(new NetworkError("timeout")); + + const view = renderGrid(); + await setFeedAddress(view, "Private Show", PRIVATE_URL); + + expect(notice).toHaveBeenCalledWith(expect.stringMatching(/^Could not load feed: /)); + expect(thisDevice.listSecrets()).toEqual([]); + expect(get(savedFeeds)).toEqual({ "Private Show": saved }); + expect( + view.getByRole("button", { name: "Set feed address for Private Show" }), + ).toBeInTheDocument(); + }); + test("keeps the newest search when an older one answers last", async () => { const older = deferred(); const newer = deferred(); From 6709e5acfb3f0cea1907c9221e13740cff575f57 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Tue, 13 Oct 2026 02:02:39 +0200 Subject: [PATCH 4/8] fix(ui): dispatch one click per Button press Button registered its click listener on every update, so a press after any prop change dispatched once per update. Register it once when the button is created. --- src/ui/obsidian/Button.svelte | 7 +++---- src/ui/obsidian/Button.test.ts | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 src/ui/obsidian/Button.test.ts diff --git a/src/ui/obsidian/Button.svelte b/src/ui/obsidian/Button.svelte index e2f4c826..c59528ec 100644 --- a/src/ui/obsidian/Button.svelte +++ b/src/ui/obsidian/Button.svelte @@ -28,6 +28,9 @@ function createButton(container: HTMLElement) { button = new ButtonComponent(container); + button.onClick((event: MouseEvent) => { + dispatch("click", { event }); + }); updateButtonAttributes(button); } @@ -43,10 +46,6 @@ if (cta) btn.setCta(); else btn.removeCta(); - btn.onClick((event: MouseEvent) => { - dispatch("click", { event }); - }); - if (styles) { btn.buttonEl.setAttr("style", extractStylesFromObj(styles)); } diff --git a/src/ui/obsidian/Button.test.ts b/src/ui/obsidian/Button.test.ts new file mode 100644 index 00000000..c876148f --- /dev/null +++ b/src/ui/obsidian/Button.test.ts @@ -0,0 +1,17 @@ +// @vitest-environment jsdom +import { fireEvent, render } from "@testing-library/svelte"; +import { expect, test, vi } from "vitest"; +import Button from "./Button.svelte"; + +test("dispatches one click per press, also after its props change", async () => { + const onClick = vi.fn(); + const view = render(Button, { + props: { text: "Save", ariaLabel: "Save" }, + events: { click: onClick }, + } as never); + await view.rerender({ text: "Saved", ariaLabel: "Saved" }); + + await fireEvent.click(view.getByRole("button", { name: "Saved" })); + + expect(onClick).toHaveBeenCalledTimes(1); +}); From 0b8f52d2c10caaa5731cdfdba13062ff498f9b3b Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Tue, 13 Oct 2026 02:02:39 +0200 Subject: [PATCH 5/8] fix(feeds): set up private feeds on another device without breaking the first A private feed's synced reference now stays its identity. Settings offers Set feed address on a saved private feed whose URL is not on this device, and OPML import sets a listed private URL the same way, both under the existing reference, so every device keeps resolving its own copy. A public namesake never replaces the feed. The orphan sweep keeps every ID data.json still references, also in records this version drops or fields it does not know. Pasting a feed under a title it no longer has offers Remove for its saved key instead of adding a duplicate. No private feed address reaches data.json: the encoder redacts it in every field, from the first write, keeping signed media and an episode's own link. A URL SecretStorage cannot keep is never saved in its place; startup then leaves data.json as it is and refuses saves. The missing-address notice no longer advises removing the feed. Closes #391 --- src/main.persistence.test.ts | 297 +++++++++++++----- src/main.ts | 93 ++++-- src/opml.test.ts | 27 ++ src/opml.ts | 53 +++- src/persistence/podNotesData.test.ts | 15 +- src/persistence/podNotesData.ts | 63 ++-- src/services/FeedUrlRepository.test.ts | 46 +++ src/services/FeedUrlRepository.ts | 52 ++- src/services/privateFeeds.test.ts | 30 +- src/services/privateFeeds.ts | 123 ++++---- .../PodcastView.integration.test.ts | 5 +- src/ui/settings/FeedAddressModal.ts | 43 +++ src/ui/settings/PodcastQueryGrid.svelte | 87 +++-- src/ui/settings/PodcastQueryGrid.test.ts | 66 +++- src/ui/settings/PodcastResultCard.svelte | 9 + src/utility/privateFeedUrl.ts | 76 +++++ tests/fixtures/persistence/README.md | 10 + .../fixtures/persistence/released-2.20.0.json | 114 +++++++ .../fixtures/persistence/released-2.21.1.json | 114 +++++++ 19 files changed, 1067 insertions(+), 256 deletions(-) create mode 100644 src/ui/settings/FeedAddressModal.ts create mode 100644 tests/fixtures/persistence/released-2.20.0.json create mode 100644 tests/fixtures/persistence/released-2.21.1.json diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index 003ee8b2..d3568c9e 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -10,8 +10,11 @@ import { PlayerController } from "./player/PlayerController"; import { downloadedEpisodes, localFiles } from "./store"; import { bindStoresToSettings } from "./store/persistence"; import { CredentialRepository } from "./services/CredentialRepository"; +import { resolveFeedUrl } from "./services/privateFeeds"; import { attachDataFileWriter, dataFileText } from "../tests/mocks/dataFileWriter"; import { FakeDocument } from "../tests/mocks/fakeMedia"; +import released2200 from "../tests/fixtures/persistence/released-2.20.0.json"; +import released2211 from "../tests/fixtures/persistence/released-2.21.1.json"; function memorySecretStorage(initial: Record = {}) { const values = new Map(Object.entries(initial)); @@ -266,33 +269,36 @@ describe("PodNotes persistence integration", () => { const secretUrl = "https://private.example/feed.xml?auth=leaked"; const { storage } = memorySecretStorage({ [id]: secretUrl }); Object.assign(plugin, { app: { ...plugin.app, secretStorage: storage } }); - await plugin.saveData( - encodePodNotesData({ - ...structuredClone(DEFAULT_SETTINGS), - savedFeeds: { - Private: { - title: "Private", - url: "podnotes-private-feed:Private", - urlSecretId: id, - artworkUrl: "", - }, - }, - queue: { - ...DEFAULT_SETTINGS.queue, - episodes: [ - { - title: "Ep", - streamUrl: "https://media.example/ep.mp3", - url: "", - description: "", - content: "", - podcastName: "Private", - feedUrl: secretUrl, + const leaked = JSON.parse( + JSON.stringify( + encodePodNotesData({ + ...structuredClone(DEFAULT_SETTINGS), + savedFeeds: { + Private: { + title: "Private", + url: "podnotes-private-feed:Private", + urlSecretId: id, + artworkUrl: "", }, - ], - }, - }), + }, + queue: { + ...DEFAULT_SETTINGS.queue, + episodes: [ + { + title: "Ep", + streamUrl: "https://media.example/ep.mp3", + url: "", + description: "", + content: "", + podcastName: "Private", + }, + ], + }, + }), + ), ); + leaked.queue.episodes[0].feedUrl = secretUrl; + await plugin.saveData(leaked); saveData.mockClear(); await plugin.loadSettings(); @@ -339,56 +345,12 @@ describe("PodNotes persistence integration", () => { }); it.each([ - ["2.20.0", "- {{time}} ", {}], - ["2.21.1", "- {{linktime}} ", { downloadedByPodNotes: true }], - ])("loads a %s library with a private feed unchanged", async (_version, template, download) => { - const id = "podnotes-feed-url-00000000-0000-4000-8000-000000000001"; - const secretUrl = "https://private.example/feed.xml?auth=released"; + ["2.20.0", released2200], + ["2.21.1", released2211], + ])("loads a %s library with a private feed unchanged", async (_version, released) => { + const id = "podnotes-feed-url-00000000-0000-4000-8000-000000000211"; + const secretUrl = "http://podnotes.test:41627/private.xml?auth=released"; const { storage, values } = memorySecretStorage({ [id]: secretUrl }); - const episode = { - title: "Ep", - streamUrl: "https://media.example/ep.mp3?sig=media", - url: "podnotes-private-feed:Private", - description: "", - content: "", - podcastName: "Private", - episodeDate: "2026-01-01T00:00:00.000Z", - feedUrl: "podnotes-private-feed:Private", - }; - const released = { - ...JSON.parse(JSON.stringify(encodePodNotesData(structuredClone(DEFAULT_SETTINGS)))), - timestamp: { template, offset: 0 }, - savedFeeds: { - Private: { - title: "Private", - url: "podnotes-private-feed:Private", - urlSecretId: id, - artworkUrl: "https://media.example/art.jpg?sig=art", - }, - Public: { - title: "Public", - url: "https://feeds.example/public.xml", - artworkUrl: "", - }, - }, - queue: { ...DEFAULT_SETTINGS.queue, episodes: [episode] }, - currentEpisode: episode, - downloadedEpisodes: { - Private: [ - { ...episode, filePath: "PodNotes/Private/Ep.mp3", size: 42, ...download }, - ], - }, - playedEpisodes: { - "Private::Ep": { - title: "Ep", - podcastName: "Private", - time: 30, - duration: 60, - finished: false, - }, - }, - }; - delete released.libraryId; const saveData = vi.fn().mockResolvedValue(undefined); const plugin = makePlugin(saveData, { dataFileExists: true }); Object.assign(plugin, { @@ -408,11 +370,196 @@ describe("PodNotes persistence integration", () => { libraryId: "", }); expect(values.get(id)).toBe(secretUrl); - expect(plugin.feedUrls.resolve(plugin.settings.savedFeeds.Private.urlSecretId ?? "")).toBe( + expect(resolveFeedUrl(plugin.settings.savedFeeds["PRIV2 Both"], plugin.feedUrls)).toBe( secretUrl, ); }); + it("keeps data.json and every secret, and saves nothing, when a private feed URL cannot move", async () => { + const orphanId = "podnotes-feed-url-00000000-0000-4000-8000-000000000003"; + const rawUrl = "https://private.example/feed.xml?auth=only-copy"; + const values = new Map([[orphanId, "https://private.example/old.xml?auth=old"]]); + const storage = { + getSecret: vi.fn((id: string) => values.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => { + if (id.startsWith("podnotes-feed-url-") && value) throw new Error("locked"); + values.set(id, value); + }), + listSecrets: vi.fn(() => [...values.keys()]), + } as unknown as SecretStorage; + const legacy = { + openAIApiKey: "sk-legacy", + savedFeeds: { Private: { title: "Private", url: rawUrl, artworkUrl: "" } }, + }; + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { + app: { ...plugin.app, secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue(structuredClone(legacy)), + }); + const notice = vi.spyOn(obsidian, "Notice"); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await plugin.loadSettings(); + attachDataFileWriter(plugin); + plugin.settings.defaultVolume = 0.4; + + await expect(plugin.saveSettingsStrict()).rejects.toThrow(/private feed URL/); + expect(saveData).not.toHaveBeenCalled(); + expect(values.get(orphanId)).toBe("https://private.example/old.xml?auth=old"); + expect(resolveFeedUrl(plugin.settings.savedFeeds.Private, plugin.feedUrls)).toBe(rawUrl); + expect(notice).toHaveBeenCalledWith( + "PodNotes could not move the private feed URL for Private into SecretStorage, so it saves nothing until you restart Obsidian.", + 0, + ); + }); + + it("moves legacy API keys and private feed URLs out of data.json in one write", async () => { + const rawUrl = "https://private.example/feed.xml?auth=legacy"; + const { storage, values } = memorySecretStorage(); + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { + app: { ...plugin.app, secretStorage: storage }, + credentials: new CredentialRepository(storage), + loadData: vi.fn().mockResolvedValue({ + openAIApiKey: "sk-legacy", + savedFeeds: { Private: { title: "Private", url: rawUrl, artworkUrl: "" } }, + queue: { + ...DEFAULT_SETTINGS.queue, + episodes: [ + { + title: "Ep", + streamUrl: "https://media.example/ep.mp3?sig=media", + url: rawUrl, + description: "", + content: "", + podcastName: "Private", + feedUrl: rawUrl, + }, + ], + }, + }), + }); + + await plugin.loadSettings(); + + expect(saveData).toHaveBeenCalledTimes(1); + const written = saveData.mock.calls[0][0]; + expect(JSON.stringify(written)).not.toContain("auth=legacy"); + const { urlSecretId } = written.savedFeeds.Private; + expect(written).toMatchObject({ + schemaVersion: 2, + openAISecretId: "podnotes-openai-api-key", + savedFeeds: { Private: { url: "podnotes-private-feed:Private" } }, + queue: { + episodes: [ + { + streamUrl: "https://media.example/ep.mp3?sig=media", + url: "podnotes-private-feed:Private", + feedUrl: "podnotes-private-feed:Private", + }, + ], + }, + }); + expect(values.get(urlSecretId)).toBe(rawUrl); + expect(values.get("podnotes-openai-api-key")).toBe("sk-legacy"); + }); + + it("never writes a private feed URL that arrives raw in a synced data.json it cannot protect", async () => { + const rawUrl = "https://private.example/feed.xml?auth=synced"; + const values = new Map(); + let locked = false; + const storage = { + getSecret: vi.fn((id: string) => values.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => { + if (locked) throw new Error("locked"); + values.set(id, value); + }), + listSecrets: vi.fn(() => [...values.keys()]), + } as unknown as SecretStorage; + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { app: { ...plugin.app, secretStorage: storage } }); + await plugin.loadSettings(); + attachDataFileWriter(plugin); + locked = true; + const synced = JSON.parse( + JSON.stringify(encodePodNotesData(structuredClone(DEFAULT_SETTINGS))), + ); + synced.savedFeeds = { Private: { title: "Private", url: rawUrl, artworkUrl: "" } }; + await plugin.saveData(synced); + saveData.mockClear(); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + plugin.settings.defaultVolume = 0.4; + + await expect(plugin.saveSettingsStrict()).rejects.toThrow(/SecretStorage/); + expect(saveData).not.toHaveBeenCalled(); + }); + + it("names a renamed private feed's episodes by its saved key and leaves its URL nowhere", async () => { + const id = "podnotes-feed-url-00000000-0000-4000-8000-000000000001"; + const rawUrl = "https://private.example/feed.xml?auth=renamed"; + const { storage } = memorySecretStorage({ [id]: rawUrl }); + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { + app: { ...plugin.app, secretStorage: storage }, + loadData: vi.fn().mockResolvedValue({ + ...JSON.parse( + JSON.stringify(encodePodNotesData(structuredClone(DEFAULT_SETTINGS))), + ), + savedFeeds: { + "Saved Key": { + title: "Channel Title", + url: "podnotes-private-feed:Saved%20Key", + urlSecretId: id, + artworkUrl: "", + }, + }, + }), + }); + await plugin.loadSettings(); + attachDataFileWriter(plugin); + const linked = { + title: "Linked", + streamUrl: "https://media.example/linked.mp3?sig=media", + url: rawUrl, + description: "", + content: "", + podcastName: "Channel Title", + feedUrl: rawUrl, + }; + const listed = { + ...linked, + title: "Listed", + url: rawUrl.replace("private.example", "PRIVATE.EXAMPLE"), + feedUrl: "podnotes-private-feed:Saved%20Key", + description: `Your feed: ${rawUrl}`, + sourceFeed: rawUrl, + }; + plugin.settings.currentEpisode = linked; + plugin.settings.queue = { ...plugin.settings.queue, episodes: [listed] }; + + await plugin.saveSettingsStrict(); + + const written = saveData.mock.calls.at(-1)?.[0]; + expect(JSON.stringify(written)).not.toContain("auth=renamed"); + expect(written.currentEpisode).toMatchObject({ + url: "podnotes-private-feed:Saved%20Key", + feedUrl: "podnotes-private-feed:Saved%20Key", + streamUrl: "https://media.example/linked.mp3?sig=media", + }); + expect(written.queue.episodes[0]).toMatchObject({ + url: "podnotes-private-feed:Saved%20Key", + description: "Your feed: podnotes-private-feed:Saved%20Key", + sourceFeed: "podnotes-private-feed:Saved%20Key", + }); + expect(plugin.settings.currentEpisode?.feedUrl).toBe(rawUrl); + }); + it("starts on an empty data.json as on a new install", async () => { const plugin = makePlugin(undefined, { dataFileExists: true }); let text = ""; diff --git a/src/main.ts b/src/main.ts index 903613d9..4baa1022 100644 --- a/src/main.ts +++ b/src/main.ts @@ -48,18 +48,22 @@ import { } from "./persistence/writer"; import { CredentialRepository } from "./services/CredentialRepository"; import { FeedUrlRepository } from "./services/FeedUrlRepository"; -import { protectPrivateFeeds, resolveFeedUrl } from "./services/privateFeeds"; +import { privateFeedAddresses, protectPrivateFeeds, resolveFeedUrl } from "./services/privateFeeds"; +import { redactPrivateFeedAddresses } from "./utility/privateFeedUrl"; import { FeedRepository, loadFeed } from "./services/FeedRepository"; type WriteRefusal = | "unreadable-data-file" | "older-data-file" + | "unprotected-private-feed" | "window-unloading" | "obsidian-closing"; const WRITE_REFUSALS: Record = { "unreadable-data-file": "PodNotes does not save over a data.json it could not read.", "older-data-file": "PodNotes does not save over a data.json from an older version.", + "unprotected-private-feed": + "PodNotes does not save while a private feed URL could not move into SecretStorage.", "window-unloading": "PodNotes does not start a save while the window unloads.", "obsidian-closing": "PodNotes does not start a save while Obsidian closes.", }; @@ -458,12 +462,27 @@ export default class PodNotes extends Plugin implements IPodNotes { } const decoded = decodePodNotesData(data); - let settings = decoded.settings; this.credentials ??= new CredentialRepository(this.app.secretStorage); + this.feedUrls ??= new FeedUrlRepository(this.app.secretStorage); + + const feedProtection = protectPrivateFeeds(decoded.settings, this.feedUrls); + if (feedProtection.failed.length > 0) { + this.settings = decoded.settings; + this.persistenceUnknownFields = decoded.unknownFields; + const { failed } = feedProtection; + this.refuseSaves( + "unprotected-private-feed", + `PodNotes could not move the private feed ${failed.length === 1 ? "URL" : "URLs"} for ${failed.join(", ")} into SecretStorage, so it saves nothing until you restart Obsidian.`, + ); + return; + } + let settings = feedProtection.settings; + const movedFeeds = settings !== decoded.settings; if ( decoded.sourceVersion < PODNOTES_DATA_SCHEMA_VERSION || - decoded.retiredPlaintextPresent + decoded.retiredPlaintextPresent || + movedFeeds ) { // SecretStorage writes are synchronous. Store every legacy value and // verify it first, then write the v2 snapshot directly because the normal @@ -476,7 +495,11 @@ export default class PodNotes extends Plugin implements IPodNotes { ? this.credentials.storeValues(decoded.legacySecrets) : {}; settings = { ...settings, ...references }; - await this.writeDataFile(encodePodNotesData(settings, decoded.unknownFields)); + const addresses = privateFeedAddresses(settings.savedFeeds, this.feedUrls); + await this.writeProtectedDataFile( + encodePodNotesData(settings, decoded.unknownFields, addresses), + addresses, + ); } catch (error) { throw new PodNotesSchemaMigrationError( error, @@ -492,31 +515,10 @@ export default class PodNotes extends Plugin implements IPodNotes { `Moved your ${migratedProviders.join(" and ")} API ${migratedProviders.length === 1 ? "key" : "keys"} into Obsidian SecretStorage.`, ); } + if (movedFeeds) new Notice("Moved private feed URLs out of data.json."); } - // Private feed URLs: move credential-bearing subscription URLs out of - // data.json into SecretStorage, scrub the same URLs out of persisted - // episode snapshots (queue/favorites/playlists/downloads/history stamp - // url/feedUrl at fetch time), then reap feed-url secrets no saved feed - // references anymore (e.g. after a settings import replaced savedFeeds). - // Best-effort: a failure keeps that feed's URL where it was, is - // reported, and never breaks loading. - this.feedUrls ??= new FeedUrlRepository(this.app.secretStorage); - const feedMigration = protectPrivateFeeds(settings, this.feedUrls); - if (feedMigration.settings !== settings) { - settings = feedMigration.settings; - await this.writeDataFile(encodePodNotesData(settings, decoded.unknownFields)); - if (feedMigration.failed.length === 0) { - new Notice("Moved private feed URLs out of data.json."); - } - } - if (feedMigration.failed.length > 0) { - new Notice( - `PodNotes could not protect the private feed ${feedMigration.failed.length === 1 ? "URL" : "URLs"} for ${feedMigration.failed.join(", ")}; ${feedMigration.failed.length === 1 ? "it stays" : "they stay"} in data.json for now. Restart PodNotes to retry.`, - 0, - ); - } - this.feedUrls.sweepOrphans(settings.savedFeeds); + this.feedUrls.sweepOrphans(data, settings); this.settings = settings; this.persistenceUnknownFields = decoded.unknownFields; @@ -581,7 +583,7 @@ export default class PodNotes extends Plugin implements IPodNotes { /** No save until `refusal` lifts, if it does, with a Notice saying why. */ private refuseSaves( - refusal: "unreadable-data-file" | "older-data-file", + refusal: "unreadable-data-file" | "older-data-file" | "unprotected-private-feed", message: string, ): void { // Closing refusals never lift; this one would. @@ -608,7 +610,11 @@ export default class PodNotes extends Plugin implements IPodNotes { } private localDocument(): PersistedPodNotesDataV2 { - return toDocument(this.settings, this.persistenceUnknownFields); + return toDocument( + this.settings, + this.persistenceUnknownFields, + privateFeedAddresses(this.settings.savedFeeds, this.feedUrls), + ); } private dataFileAccess(): DataFileAccess { @@ -643,9 +649,20 @@ export default class PodNotes extends Plugin implements IPodNotes { } this.liftUnreadableRefusal(); // A private feed synced here raw is protected before it can be merged and written. + const protection = protectPrivateFeeds(decoded.settings, this.feedUrls); + if (protection.failed.length > 0) { + throw new Error( + `PodNotes could not move a private feed URL in ${path} into SecretStorage`, + ); + } + const addresses = new Map([ + ...privateFeedAddresses(protection.settings.savedFeeds, this.feedUrls), + ...privateFeedAddresses(this.settings.savedFeeds, this.feedUrls), + ]); const doc: Record = toDocument( - protectPrivateFeeds(decoded.settings, this.feedUrls).settings, + protection.settings, decoded.unknownFields, + addresses, ); // An entry the codec dropped while repairing a value (an invalid episode, a // collection reset) is not a deletion on the other device. Keep it. @@ -655,9 +672,13 @@ export default class PodNotes extends Plugin implements IPodNotes { doc[key] = restoreDropped(value, doc[key], raw[key], [key]); } } - return doc as PersistedPodNotesDataV2; + return redactPrivateFeedAddresses(doc, addresses) as PersistedPodNotesDataV2; }, - write: (doc) => this.writeDataFile(doc), + write: (doc) => + this.writeProtectedDataFile( + doc, + privateFeedAddresses(this.settings.savedFeeds, this.feedUrls), + ), local: () => this.localDocument(), publish: (doc) => this.publishDocument(doc), }; @@ -706,8 +727,14 @@ export default class PodNotes extends Plugin implements IPodNotes { } } - private async writeDataFile(data: unknown): Promise { + private async writeProtectedDataFile( + data: unknown, + privateFeedAddresses: ReadonlyMap, + ): Promise { if (this.writeRefusal) throw new WriteRefusedError(WRITE_REFUSALS[this.writeRefusal]); + if (redactPrivateFeedAddresses(data, privateFeedAddresses) !== data) { + throw new Error("PodNotes does not write a private feed URL to data.json."); + } const write = this.saveData(data); this.writeInFlight = write.catch(() => undefined); try { diff --git a/src/opml.test.ts b/src/opml.test.ts index a11b11fc..23de53b5 100644 --- a/src/opml.test.ts +++ b/src/opml.test.ts @@ -280,6 +280,33 @@ describe("importOPML (IE-01)", () => { expect([...feedUrlSecrets]).toEqual([[saved.urlSecretId, privateUrl]]); }); + it("sets no address on a saved private feed a sync replaced while the import loaded", async () => { + const privateUrl = "https://private.test/feed?auth=s3cret"; + feedUrlSecrets.clear(); + const saved = { + title: "Private", + url: "podnotes-private-feed:Private", + urlSecretId: "podnotes-feed-url-00000000-0000-4000-8000-000000000001", + artworkUrl: "", + }; + const replaced = { + ...saved, + urlSecretId: "podnotes-feed-url-00000000-0000-4000-8000-000000000002", + }; + savedFeeds.set({ Private: saved }); + channelAt.mockImplementation(async ({ url }: { url: string }) => { + savedFeeds.set({ Private: replaced }); + return { title: "Private", url, artworkUrl: "" }; + }); + + await importOPML( + ``, + ); + + expect(get(savedFeeds)).toEqual({ Private: replaced }); + expect(feedUrlSecrets.size).toBe(0); + }); + it("never replaces a saved private feed missing on this device with a public namesake", async () => { feedUrlSecrets.clear(); const saved = { diff --git a/src/opml.ts b/src/opml.ts index 29bdf6e0..1b57f54f 100644 --- a/src/opml.ts +++ b/src/opml.ts @@ -1,6 +1,13 @@ import { type App, Notice } from "obsidian"; import { plugin, savedFeeds } from "./store"; -import { internPrivateFeed, isSavedFeed, resolveFeedUrl } from "./services/privateFeeds"; +import { + internPrivateFeed, + isSavedFeed, + needsPrivateFeedAddress, + resolveFeedUrl, + setPrivateFeedAddress, +} from "./services/privateFeeds"; +import { isCredentialBearingUrl } from "./utility/privateFeedUrl"; import type { PodcastFeed } from "./types/PodcastFeed"; import { TimerNotice } from "./ui/TimerNotice"; import { get } from "svelte/store"; @@ -44,7 +51,8 @@ async function importOPML(opml: string): Promise { throw new Error("No valid podcast entries found in OPML"); } - const existingSavedFeeds = Object.values(get(savedFeeds)); + const savedAtStart = get(savedFeeds); + const existingSavedFeeds = Object.values(savedAtStart); const { feeds, feedUrls } = get(plugin); const newPodcastsToAdd = incompletePodcastsToAdd.filter( (pod) => !existingSavedFeeds.some((saved) => isSavedFeed(saved, pod.url, feedUrls)), @@ -92,14 +100,34 @@ async function importOPML(opml: string): Promise { // from an earlier import in this batch or a previously saved feed) are // dropped. Count what is actually written so the summary doesn't over-report. let savedCount = 0; + const notStored: string[] = []; savedFeeds.update((saved) => { for (const pod of validPodcasts) { - // A saved private feed whose URL is not on this device takes the listed URL. - if (saved[pod.title] && resolveFeedUrl(saved[pod.title], feedUrls) !== null) - continue; - // Imported private URLs go straight to SecretStorage, never data.json. - saved[pod.title] = internPrivateFeed(structuredClone(pod), feedUrls); - savedCount++; + try { + const existing = Object.prototype.hasOwnProperty.call(saved, pod.title) + ? saved[pod.title] + : undefined; + if (existing) { + if ( + !needsPrivateFeedAddress(existing, feedUrls) || + !isCredentialBearingUrl(pod.url) || + savedAtStart[pod.title]?.urlSecretId !== existing.urlSecretId + ) { + continue; + } + setPrivateFeedAddress(existing, pod.url, feedUrls); + } else { + // Imported private URLs go straight to SecretStorage, never data.json. + saved[pod.title] = internPrivateFeed(structuredClone(pod), feedUrls); + } + savedCount++; + } catch (error) { + console.error( + `PodNotes: could not store the private feed "${pod.title}"`, + error, + ); + notStored.push(pod.title); + } } return saved; }); @@ -108,7 +136,7 @@ async function importOPML(opml: string): Promise { const skippedExisting = incompletePodcastsToAdd.length - newPodcastsToAdd.length; // Feeds that fetched fine but collided with an existing/earlier title and // were therefore silently dropped by the title-keyed store above. - const droppedDuplicateTitle = validPodcasts.length - savedCount; + const droppedDuplicateTitle = validPodcasts.length - savedCount - notStored.length; let summary = `OPML import complete. Saved ${savedCount} new podcasts. Skipped ${skippedExisting} existing podcasts.`; if (droppedDuplicateTitle > 0) { @@ -116,6 +144,13 @@ async function importOPML(opml: string): Promise { } notice.update(summary); + if (notStored.length > 0) { + new Notice( + `Could not store the private feed ${notStored.length === 1 ? "address" : "addresses"} for ${notStored.join(", ")} on this device.`, + 10000, + ); + } + if (validPodcasts.length !== newPodcastsToAdd.length) { const failedImports = newPodcastsToAdd.length - validPodcasts.length; console.error(`Failed to import ${failedImports} podcasts.`); diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index c3d05d42..edf17bc8 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -84,20 +84,31 @@ describe("PodNotes data schema", () => { }, playedEpisodes: { "Test Podcast::Private": history }, } satisfies IPodNotesSettings; - settings.queue.episodes[0] = { ...item, sourceFeed: rawUrl } as Episode; - const unknownFields = { libraryId: "library", nextVersion: { lastEpisode: item } }; + settings.queue.episodes[0] = { + ...item, + sourceFeed: rawUrl, + description: `

Subscribe: your feed

`, + } as Episode; + const unknownFields = { + libraryId: "library", + nextVersion: { lastEpisode: item, note: `Moved from ${rawUrl} today` }, + }; const before = structuredClone({ settings, unknownFields }); const encoded = encodePodNotesData(settings, unknownFields); expect(JSON.stringify(encoded)).not.toContain(sentinel); expect(encoded.queue.episodes[0].sourceFeed).toBe("podnotes-private-feed:Test%20Podcast"); + expect(encoded.queue.episodes[0].description).toBe( + '

Subscribe: your feed

', + ); expect(encoded.nextVersion).toEqual({ lastEpisode: { ...item, url: "podnotes-private-feed:Test%20Podcast", feedUrl: "podnotes-private-feed:Test%20Podcast", }, + note: "Moved from podnotes-private-feed:Test%20Podcast today", }); expect(encoded.currentEpisode?.feedUrl).toBe("podnotes-private-feed:Test%20Podcast"); for (const list of [ diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index bca9b335..ca8b1b45 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -9,6 +9,7 @@ import { } from "./episodeCodec"; import { decodeSettings } from "./settingsCodec"; import type { CredentialValues } from "src/types/Credentials"; +import { isCredentialBearingUrl, redactPrivateFeedAddresses } from "src/utility/privateFeedUrl"; export const PODNOTES_DATA_SCHEMA_VERSION = 2; @@ -113,43 +114,63 @@ export function decodePodNotesData(value: unknown): DecodedPodNotesData { }; } -/** Serialize a validated runtime snapshot with canonical dates and schema. */ +/** + * Serialize a validated runtime snapshot with canonical dates and schema. No + * private feed address reaches the result. A saved feed's own private URL must + * be in SecretStorage first: this throws rather than drop its only copy. + */ export function encodePodNotesData( settings: IPodNotesSettings, unknownFields: UnknownRecord = {}, + /** Saved private feeds' URLs on this device, to their placeholders. */ + privateFeedAddresses?: ReadonlyMap, ): PersistedPodNotesDataV2 { const validated = decodeSettings( settings as unknown as UnknownRecord, new Set(), PODNOTES_DATA_SCHEMA_VERSION, ); + for (const [key, feed] of Object.entries(validated.savedFeeds)) { + if (isCredentialBearingUrl(feed.url)) { + throw new PodNotesDataError( + `PodNotes does not write the private feed URL of "${key}" to data.json.`, + "invalid-data", + ); + } + } - return { - ...copyUnknownRootFields(unknownFields), - ...validated, - schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, - libraryId: - typeof unknownFields.libraryId === "string" - ? unknownFields.libraryId - : LIBRARY_BEFORE_IDS, - currentEpisode: validated.currentEpisode - ? encodeEpisode(validated.currentEpisode) - : undefined, - favorites: encodePlaylist(validated.favorites), - queue: encodePlaylist(validated.queue), - localFiles: encodePlaylist(validated.localFiles), - playlists: mapRecord(validated.playlists, (playlist) => encodePlaylist(playlist)), - downloadedEpisodes: mapRecord(validated.downloadedEpisodes, (episodes) => - episodes.map((episode) => encodeEpisode(episode)), - ), - }; + return redactPrivateFeedAddresses( + { + ...copyUnknownRootFields(unknownFields), + ...validated, + schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, + libraryId: + typeof unknownFields.libraryId === "string" + ? unknownFields.libraryId + : LIBRARY_BEFORE_IDS, + currentEpisode: validated.currentEpisode + ? encodeEpisode(validated.currentEpisode) + : undefined, + favorites: encodePlaylist(validated.favorites), + queue: encodePlaylist(validated.queue), + localFiles: encodePlaylist(validated.localFiles), + playlists: mapRecord(validated.playlists, (playlist) => encodePlaylist(playlist)), + downloadedEpisodes: mapRecord(validated.downloadedEpisodes, (episodes) => + episodes.map((episode) => encodeEpisode(episode)), + ), + }, + privateFeedAddresses, + ); } export function toDocument( settings: IPodNotesSettings, unknownFields?: UnknownRecord, + privateFeedAddresses?: ReadonlyMap, ): PersistedPodNotesDataV2 { - return JSON.parse(JSON.stringify(encodePodNotesData(settings, unknownFields))); + return JSON.parse( + JSON.stringify(encodePodNotesData(settings, unknownFields, privateFeedAddresses)), + ); } function readSchemaVersion(root: UnknownRecord): PodNotesDataSchemaVersion { diff --git a/src/services/FeedUrlRepository.test.ts b/src/services/FeedUrlRepository.test.ts index 6be56892..1ff544f3 100644 --- a/src/services/FeedUrlRepository.test.ts +++ b/src/services/FeedUrlRepository.test.ts @@ -47,6 +47,52 @@ describe("FeedUrlRepository", () => { ); }); + it("sets a URL on a second device under the reference the first device created", () => { + const firstDevice = storage(); + const secondDevice = storage(); + const id = new FeedUrlRepository(firstDevice.api).store("https://p.example/rss?auth=tok"); + const repository = new FeedUrlRepository(secondDevice.api); + + repository.attach(` ${id} `, "https://p.example/rss?auth=tok"); + repository.attach(id, "https://p.example/rss?auth=tok"); + + expect([...secondDevice.values]).toEqual([[id, "https://p.example/rss?auth=tok"]]); + expect(firstDevice.values.get(id)).toBe("https://p.example/rss?auth=tok"); + }); + + it("never attaches over another URL, a foreign ID, an unreadable slot or an unkept write", () => { + const id = "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef"; + const occupied = storage({ [id]: "https://p.example/rss?auth=other" }); + expect(() => + new FeedUrlRepository(occupied.api).attach(id, "https://p.example/rss?auth=tok"), + ).toThrow(/another URL/); + expect(occupied.values.get(id)).toBe("https://p.example/rss?auth=other"); + + const foreign = storage(); + expect(() => + new FeedUrlRepository(foreign.api).attach( + "podnotes-openai-api-key", + "https://p.example/rss?auth=tok", + ), + ).toThrow(/Not a PodNotes/); + expect(foreign.values.size).toBe(0); + + const unreadable = storage(); + (unreadable.api.getSecret as ReturnType).mockImplementation(() => { + throw new Error("locked"); + }); + expect(() => + new FeedUrlRepository(unreadable.api).attach(id, "https://p.example/rss?auth=tok"), + ).toThrow("locked"); + expect(unreadable.api.setSecret).not.toHaveBeenCalled(); + + const unkept = storage(); + (unkept.api.setSecret as ReturnType).mockImplementation(() => {}); + expect(() => + new FeedUrlRepository(unkept.api).attach(id, "https://p.example/rss?auth=tok"), + ).toThrow(/did not retain/); + }); + it("resolves only PodNotes-owned feed-url IDs and fails closed", () => { const { api } = storage({ "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef": diff --git a/src/services/FeedUrlRepository.ts b/src/services/FeedUrlRepository.ts index 06f62c3f..a0e775e9 100644 --- a/src/services/FeedUrlRepository.ts +++ b/src/services/FeedUrlRepository.ts @@ -1,5 +1,4 @@ import type { SecretStorage } from "obsidian"; -import type { PodcastFeed } from "src/types/PodcastFeed"; import { isValidSecretId } from "src/types/Credentials"; /** @@ -11,8 +10,9 @@ import { isValidSecretId } from "src/types/Credentials"; * references sync through data.json while SecretStorage stays device-local, * so IDs must never collide across devices. With sequential IDs, device A's * synced reference could resolve on device B to an UNRELATED feed's secret - * and silently fetch the wrong private feed; with UUIDs a foreign reference - * simply misses and the user re-adds the feed. Content-free by construction - + * and silently fetch the wrong private feed. With UUIDs a reference misses on + * a device until its user sets the feed's address there, under the same ID, so + * 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. */ @@ -41,15 +41,31 @@ export class FeedUrlRepository { /** Store a private feed URL under a fresh PodNotes-owned ID and verify the readback. */ store(url: string): string { + const id = `podnotes-feed-url-${crypto.randomUUID()}`; + this.attach(id, url); + return id; + } + + /** + * Store a private feed URL under `urlSecretId`, a reference data.json already + * holds, and verify the readback. Refuses an ID PodNotes does not own, and a + * slot that holds another URL on this device. + */ + attach(urlSecretId: string, url: string): void { + const id = urlSecretId.trim(); const value = url.trim(); + if (!isFeedUrlSecretId(id) || !isValidSecretId(id)) { + throw new Error("Not a PodNotes private feed reference."); + } if (!value) throw new Error("A private feed URL must not be empty."); - - const id = `podnotes-feed-url-${crypto.randomUUID()}`; + const current = this.storage.getSecret(id)?.trim(); + if (current && current !== value) { + throw new Error("This device holds another URL for this private feed."); + } this.storage.setSecret(id, value); if (this.storage.getSecret(id) !== value) { throw new Error("SecretStorage did not retain the private feed URL."); } - return id; } /** Clearing to "" is SecretStorage's deletion idiom; reads treat "" as absent. */ @@ -64,18 +80,22 @@ export class FeedUrlRepository { } /** - * Delete every PodNotes-owned feed-url secret no saved feed references. - * Covers removals that bypassed the remove flow (e.g. a settings import - * replacing savedFeeds wholesale). Never throws: a sweep failure must not - * abort plugin loading. + * Delete every PodNotes-owned feed-url secret none of `documents` references + * anywhere, so an ID in a record this version could not read, or in a field it + * does not know, keeps its secret. Covers removals that bypassed the remove + * flow (e.g. a settings import replacing savedFeeds wholesale). Never throws: + * a sweep failure must not abort plugin loading. */ - sweepOrphans(feeds: Record): void { + sweepOrphans(...documents: unknown[]): void { const referenced = new Set(); - for (const feed of Object.values(feeds)) { - // Trim to match resolve(): a reference that resolves must also protect - // its secret from the sweep. - if (feed.urlSecretId?.trim()) referenced.add(feed.urlSecretId.trim()); - } + const collect = (value: unknown): void => { + if (typeof value === "string") { + if (isFeedUrlSecretId(value.trim())) referenced.add(value.trim()); + } else if (typeof value === "object" && value !== null) { + for (const entry of Object.values(value)) collect(entry); + } + }; + for (const document of documents) collect(document); try { for (const id of this.storage.listSecrets()) { if (!isFeedUrlSecretId(id) || referenced.has(id)) continue; diff --git a/src/services/privateFeeds.test.ts b/src/services/privateFeeds.test.ts index 95068394..efdf9318 100644 --- a/src/services/privateFeeds.test.ts +++ b/src/services/privateFeeds.test.ts @@ -84,15 +84,15 @@ describe("internPrivateFeed", () => { expect(interned.url).toBe("podnotes-private-feed:Renamed%20Key"); }); - it("keeps the feed unchanged when SecretStorage fails, never half-migrated", () => { - const { api, feedUrls } = repository(); + it("throws when SecretStorage fails, so the URL is never saved in its place", () => { + const { api, feedUrls, values } = repository(); (api.setSecret as ReturnType).mockImplementation(() => { throw new Error("locked"); }); const original = feed({ url: PRIVATE_URL }); - const result = internPrivateFeed(original, feedUrls); - expect(result).toBe(original); - expect(result.urlSecretId).toBeUndefined(); + expect(() => internPrivateFeed(original, feedUrls)).toThrow("locked"); + expect(original).toEqual(feed({ url: PRIVATE_URL })); + expect(values.size).toBe(0); }); }); @@ -312,20 +312,22 @@ describe("protectPrivateFeeds", () => { expect(safe.queue.episodes[0].url).toBe(link); }); - it("reports feeds whose URL could not be protected", () => { + it("reports feeds whose URL could not be protected and changes nothing", () => { const { api, feedUrls } = repository(); + vi.spyOn(console, "error").mockImplementation(() => undefined); (api.setSecret as ReturnType).mockImplementation(() => { throw new Error("locked"); }); - const { failed, settings: result } = protectPrivateFeeds( - settings({ - savedFeeds: { Patreon: feed({ title: "Patreon", url: PRIVATE_URL }) }, - queue: { ...DEFAULT_SETTINGS.queue, episodes: [episode("Ep 2", PRIVATE_URL)] }, - }), - feedUrls, - ); + const input = settings({ + savedFeeds: { + Public: feed({ title: "Public" }), + Patreon: feed({ title: "Patreon", url: PRIVATE_URL }), + }, + queue: { ...DEFAULT_SETTINGS.queue, episodes: [episode("Ep 2", PRIVATE_URL)] }, + }); + const { failed, settings: result } = protectPrivateFeeds(input, feedUrls); expect(failed).toEqual(["Patreon"]); + expect(result).toBe(input); expect(result.savedFeeds.Patreon.url).toBe(PRIVATE_URL); - expect(result.queue.episodes[0].feedUrl).toBe("podnotes-private-feed:Patreon"); }); }); diff --git a/src/services/privateFeeds.ts b/src/services/privateFeeds.ts index 1fc0bb61..1f695259 100644 --- a/src/services/privateFeeds.ts +++ b/src/services/privateFeeds.ts @@ -1,12 +1,13 @@ import { Notice } from "obsidian"; import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import type { PodcastFeed } from "src/types/PodcastFeed"; -import checkStringIsUrl from "src/utility/checkStringIsUrl"; import { ownRecord } from "src/utility/ownRecord"; import { + canonicalUrl, isCredentialBearingUrl, isPrivateFeedPlaceholder, privateFeedPlaceholder, + redactPrivateFeedAddresses, } from "src/utility/privateFeedUrl"; import type { FeedAddress } from "./FeedRepository"; import type { FeedUrlRepository } from "./FeedUrlRepository"; @@ -31,9 +32,8 @@ import type { FeedUrlRepository } from "./FeedUrlRepository"; * when a reference already exists (an imported or hand-edited record can * carry BOTH a reference and a raw URL - the raw URL is the visible ground * truth, so it is re-interned and the placeholder restored). Non-credentialed - * feeds pass through untouched. On a SecretStorage failure the feed is - * returned unchanged - keeping the URL in data.json (the status quo) rather - * than breaking the subscription; migration surfaces that failure to the user. + * feeds pass through untouched. Throws when SecretStorage fails: the URL must + * never be saved in its place. * * `savedKey` is the feed's savedFeeds key, which links and the URI handler * use to find the feed again; it defaults to the title (they match everywhere @@ -45,13 +45,30 @@ export function internPrivateFeed( savedKey: string = feed.title, ): PodcastFeed { if (!isCredentialBearingUrl(feed.url)) return feed; - try { - const urlSecretId = feedUrls.store(feed.url); - return { ...feed, url: privateFeedPlaceholder(savedKey), urlSecretId }; - } catch (error) { - console.error(`PodNotes: could not protect the private feed "${feed.title}"`, error); - return feed; + const urlSecretId = feedUrls.store(feed.url); + return { ...feed, url: privateFeedPlaceholder(savedKey), urlSecretId }; +} + +/** Whether this saved private feed waits for its URL to be set on this device. */ +export function needsPrivateFeedAddress(feed: PodcastFeed, feedUrls: FeedUrlRepository): boolean { + return Boolean(feed.urlSecretId?.trim()) && resolveFeedUrl(feed, feedUrls) === null; +} + +/** + * Set a saved private feed's URL on this device, under the reference data.json + * already holds. Every device keeps its own copy under that one reference, so + * setting it up here leaves the feed working on the others. Throws when `url` + * is not a private feed URL or SecretStorage cannot keep it. + */ +export function setPrivateFeedAddress( + feed: PodcastFeed, + url: string, + feedUrls: FeedUrlRepository, +): void { + if (!feed.urlSecretId || !isCredentialBearingUrl(url)) { + throw new Error("Not a private feed URL for this feed."); } + feedUrls.attach(feed.urlSecretId, url); } /** @@ -80,22 +97,23 @@ export function isSavedFeed(saved: PodcastFeed, url: string, feedUrls: FeedUrlRe return resolved !== null && canonicalUrl(resolved) === wanted; } -/** `url` as the URL parser serializes it, or as given when it is not a URL. */ -function canonicalUrl(url: string): string { - return checkStringIsUrl(url)?.href ?? url; +/** The savedFeeds key of the feed at `url`, whatever the feed is called now. */ +export function findSavedFeed( + feeds: Record, + url: string, + feedUrls: FeedUrlRepository, +): string | undefined { + return Object.keys(feeds).find((key) => isSavedFeed(feeds[key], url, feedUrls)); } /** Tells the user that a saved private feed's URL is not on this device. */ export function notifyPrivateFeedUnavailable(title: string): void { - new Notice( - `The private feed URL for "${title}" is not available on this device. ` + - "Remove the feed and add it again with its private URL.", - ); + new Notice(`The private feed address for "${title}" is not available on this device.`); } export interface PrivateFeedProtection { settings: IPodNotesSettings; - /** Feeds whose URL could NOT be protected (SecretStorage failure). */ + /** Feeds whose URL could NOT be protected (SecretStorage failure); `settings` is then the input. */ failed: string[]; } @@ -105,57 +123,42 @@ export function protectPrivateFeeds( ): PrivateFeedProtection { const failed: string[] = []; const savedFeeds = ownRecord(); - // A private feed's real URL maps to the placeholder naming its saved key, - // which links resolve by. - const byUrl = new Map(); let moved = false; for (const [key, feed] of Object.entries(settings.savedFeeds)) { - const interned = internPrivateFeed(feed, feedUrls, key); - if (interned !== feed) moved = true; - else if (isCredentialBearingUrl(feed.url)) failed.push(key); - if (isPrivateFeedPlaceholder(interned.url)) { - byUrl.set(feed.url, interned.url); - const url = interned.urlSecretId && feedUrls.resolve(interned.urlSecretId); - if (url) byUrl.set(url, interned.url); + let interned = feed; + try { + interned = internPrivateFeed(feed, feedUrls, key); + } catch (error) { + console.error(`PodNotes: could not protect the private feed "${key}"`, error); + failed.push(key); } + if (interned !== feed) moved = true; savedFeeds[key] = interned; } - // Snapshots stamp `feedUrl`, and an episode without a link of its own takes - // the feed URL as `url`. A private URL no saved feed matches still goes: its - // podcast's placeholder stands for it. - const placeholderFor = (url: string, episode: Record) => - byUrl.get(url) ?? - (url === episode.feedUrl && - typeof episode.podcastName === "string" && - isCredentialBearingUrl(url) - ? privateFeedPlaceholder(episode.podcastName) - : url); + if (failed.length > 0) return { settings, failed }; return { - settings: scrubEpisodeUrls(moved ? { ...settings, savedFeeds } : settings, placeholderFor), + settings: redactPrivateFeedAddresses( + moved ? { ...settings, savedFeeds } : settings, + privateFeedAddresses(savedFeeds, feedUrls), + ), failed, }; } -/** Returns `value` itself when no URL changed, so callers can tell whether to save. */ -function scrubEpisodeUrls( - value: T, - placeholderFor: (url: string, episode: Record) => string, -): T { - if (typeof value !== "object" || value === null) return value; - const isArray = Array.isArray(value); - const prototype = Object.getPrototypeOf(value) as object | null; - if (!isArray && prototype !== Object.prototype && prototype !== null) return value; - - const record = value as Record; - let changed = false; - const next = (isArray ? [] : Object.create(prototype)) as Record; - for (const [key, entry] of Object.entries(record)) { - const scrubbed = - (key === "url" || key === "feedUrl") && typeof entry === "string" - ? placeholderFor(entry, record) - : scrubEpisodeUrls(entry, placeholderFor); - if (scrubbed !== entry) changed = true; - next[key] = scrubbed; +/** + * Each saved private feed's URL on this device, to the placeholder naming its + * saved key, which links resolve by. An episode snapshot of the feed takes that + * placeholder even when the feed is now called something else. + */ +export function privateFeedAddresses( + feeds: Record, + feedUrls: FeedUrlRepository, +): Map { + const addresses = new Map(); + for (const feed of Object.values(feeds)) { + if (!isPrivateFeedPlaceholder(feed.url)) continue; + const url = resolveFeedUrl(feed, feedUrls); + if (url) addresses.set(url, feed.url); } - return (changed ? next : value) as T; + return addresses; } diff --git a/src/ui/PodcastView/PodcastView.integration.test.ts b/src/ui/PodcastView/PodcastView.integration.test.ts index ca54635c..fae77f12 100644 --- a/src/ui/PodcastView/PodcastView.integration.test.ts +++ b/src/ui/PodcastView/PodcastView.integration.test.ts @@ -439,10 +439,7 @@ describe("PodcastView integration flow", () => { await waitFor(() => expect(notice.mock.calls).toEqual([ - [ - 'The private feed URL for "Patron Show" is not available on this device. ' + - "Remove the feed and add it again with its private URL.", - ], + ['The private feed address for "Patron Show" is not available on this device.'], ]), ); expect(mockLoad).toHaveBeenCalledTimes(1); diff --git a/src/ui/settings/FeedAddressModal.ts b/src/ui/settings/FeedAddressModal.ts new file mode 100644 index 00000000..25c8875e --- /dev/null +++ b/src/ui/settings/FeedAddressModal.ts @@ -0,0 +1,43 @@ +import { type App, Modal, Setting } from "obsidian"; + +class FeedAddressModal extends Modal { + private address: string | null = null; + + constructor( + app: App, + private readonly podcastTitle: string, + private readonly onAnswer: (address: string | null) => void, + ) { + super(app); + } + + override onOpen(): void { + this.titleEl.setText(this.podcastTitle); + let input: HTMLInputElement | undefined; + const save = () => { + this.address = input?.value ?? null; + this.close(); + }; + new Setting(this.contentEl).setName("Feed address").addText((text) => { + input = text.inputEl; + input.setAttribute("aria-label", "Feed address"); + input.addEventListener("keydown", (event) => { + if (event.key === "Enter") save(); + }); + }); + new Setting(this.contentEl).addButton((button) => + button.setButtonText("Save").setCta().onClick(save), + ); + input?.focus(); + } + + override onClose(): void { + this.contentEl.empty(); + this.onAnswer(this.address); + } +} + +/** The address the user enters for a feed, or null when they close the dialog. */ +export function askFeedAddress(app: App, podcastTitle: string): Promise { + return new Promise((resolve) => new FeedAddressModal(app, podcastTitle, resolve).open()); +} diff --git a/src/ui/settings/PodcastQueryGrid.svelte b/src/ui/settings/PodcastQueryGrid.svelte index 707e9c31..ebc87241 100644 --- a/src/ui/settings/PodcastQueryGrid.svelte +++ b/src/ui/settings/PodcastQueryGrid.svelte @@ -8,14 +8,17 @@ import { plugin, savedFeeds } from "src/store"; import { get } from "svelte/store"; import { + findSavedFeed, internPrivateFeed, - isSavedFeed, + needsPrivateFeedAddress, notifyPrivateFeedUnavailable, resolveFeedUrl, + setPrivateFeedAddress, } from "src/services/privateFeeds"; import type { PodcastFeed } from "src/types/PodcastFeed"; import checkStringIsUrl from "src/utility/checkStringIsUrl"; import Text from "../obsidian/Text.svelte"; + import { askFeedAddress } from "./FeedAddressModal"; import PodcastResultCard from "./PodcastResultCard.svelte"; import { onMount } from "svelte"; import { fade } from "svelte/transition"; @@ -123,26 +126,35 @@ // Saved feeds are keyed by title. Only own keys count, so a show called // "toString" is not mistaken for a saved one. - function savedFeedTitled( - feeds: Record, - title: string, - ): PodcastFeed | undefined { - return Object.prototype.hasOwnProperty.call(feeds, title) ? feeds[title] : undefined; + function savedFeedAt(feeds: Record, key: string): PodcastFeed | undefined { + return Object.prototype.hasOwnProperty.call(feeds, key) ? feeds[key] : undefined; } - function isSavedResult(feeds: Record, podcast: PodcastFeed): boolean { - const saved = savedFeedTitled(feeds, podcast.title); - return saved !== undefined && isSavedFeed(saved, podcast.url, get(plugin).feedUrls); + // Saved private feeds whose URL is not on this device. SecretStorage is not a + // store, so setting an address here recomputes it. + let missingAddresses = new Set(); + $: missingAddresses = feedsMissingAddress($savedFeeds); + + function feedsMissingAddress(feeds: Record): Set { + const { feedUrls } = get(plugin); + return new Set( + Object.keys(feeds).filter((key) => needsPrivateFeedAddress(feeds[key], feedUrls)), + ); + } + + function notStored(title: string, error: unknown) { + console.error("PodNotes: could not store a private feed URL", error); + new Notice(`Could not store the private feed address for "${title}" on this device.`); } function addPodcast(event: CustomEvent<{ podcast: PodcastFeed }>) { const { podcast } = event.detail; const { feedUrls } = get(plugin); + if (findSavedFeed($savedFeeds, podcast.url, feedUrls) !== undefined) return; // A namesake must never replace a saved feed. A saved private feed whose // URL is missing on this device cannot be compared, so say that instead. - const saved = savedFeedTitled($savedFeeds, podcast.title); + const saved = savedFeedAt($savedFeeds, podcast.title); if (saved) { - if (isSavedFeed(saved, podcast.url, feedUrls)) return; if (resolveFeedUrl(saved, feedUrls) === null) { notifyPrivateFeedUnavailable(saved.title); return; @@ -152,23 +164,57 @@ } // A pasted private feed URL must never reach persisted settings: intern it // into SecretStorage and save the placeholder + reference instead. - const feed = internPrivateFeed(podcast, feedUrls); + let feed: PodcastFeed; + try { + feed = internPrivateFeed(podcast, feedUrls); + } catch (error) { + notStored(podcast.title, error); + return; + } savedFeeds.update((feeds) => ({ ...feeds, [feed.title]: feed })); updateSearchResults(); } + async function setFeedAddress(key: string) { + const feed = savedFeedAt(get(savedFeeds), key); + if (!feed) return; + const { app, feeds, feedUrls } = get(plugin); + const entered = await askFeedAddress(app, feed.title); + if (entered === null || entered.trim() === "") return; + const url = checkStringIsUrl(entered.trim()); + if (url?.protocol !== "http:" && url?.protocol !== "https:") { + new Notice("Enter the feed's http or https address."); + return; + } + try { + await feeds.channelAt({ url: url.href }); + } catch (e) { + new Notice(`Could not load feed: ${e instanceof Error ? e.message : String(e)}`); + return; + } + const current = savedFeedAt(get(savedFeeds), key); + if (!current || current.urlSecretId !== feed.urlSecretId) return; + try { + setPrivateFeedAddress(current, url.href, feedUrls); + } catch (error) { + notStored(feed.title, error); + return; + } + missingAddresses = feedsMissingAddress(get(savedFeeds)); + void feeds.load([key], "network"); + } + function removePodcast(key: string) { savedFeeds.update((feeds) => { const removed = feeds[key]; const newFeeds = { ...feeds }; delete newFeeds[key]; - // Identical private URLs dedupe to one secret ID, so delete only when no - // remaining feed still references it (the load-time sweep is the backstop). + const reference = removed?.urlSecretId?.trim(); const stillReferenced = Object.values(newFeeds).some( - (feed) => feed.urlSecretId === removed?.urlSecretId, + (feed) => feed.urlSecretId?.trim() === reference, ); - if (removed?.urlSecretId && !stillReferenced) { - get(plugin).feedUrls.delete(removed.urlSecretId); + if (reference && !stillReferenced) { + get(plugin).feedUrls.delete(reference); } return newFeeds; }); @@ -199,12 +245,15 @@
{#each searchResults as [key, podcast] (key ?? podcast)} + {@const savedKey = key ?? findSavedFeed($savedFeeds, podcast.url, $plugin.feedUrls)}
removePodcast(key ?? podcast.title)} + on:removePodcast={() => savedKey !== undefined && removePodcast(savedKey)} + on:setFeedAddress={() => savedKey !== undefined && setFeedAddress(savedKey)} />
{/each} diff --git a/src/ui/settings/PodcastQueryGrid.test.ts b/src/ui/settings/PodcastQueryGrid.test.ts index 9bed7b48..914a4c1c 100644 --- a/src/ui/settings/PodcastQueryGrid.test.ts +++ b/src/ui/settings/PodcastQueryGrid.test.ts @@ -249,7 +249,7 @@ describe("PodcastQueryGrid", () => { expect(secrets.listSecrets()).toEqual([saved.urlSecretId]); }); - test("a followed private feed without its URL on this device asks to add it again", async () => { + test("a followed private feed without its URL on this device says so instead of adding it", async () => { const notice = vi.spyOn(obsidian, "Notice"); const { saved } = followPrivateShow(); const thisDevice = new obsidian.SecretStorage(); @@ -259,8 +259,7 @@ describe("PodcastQueryGrid", () => { await fireEvent.click(await findByRole("button", { name: "Add Private Show podcast" })); expect(notice).toHaveBeenCalledWith( - 'The private feed URL for "Private Show" is not available on this device. ' + - "Remove the feed and add it again with its private URL.", + 'The private feed address for "Private Show" is not available on this device.', ); expect(notice).not.toHaveBeenCalledWith( "You already follow a podcast called Private Show from another feed.", @@ -308,6 +307,67 @@ describe("PodcastQueryGrid", () => { ).toBeInTheDocument(); }); + test("sets no address on a feed that a sync replaced while the address loaded", async () => { + followPrivateShow(); + const thisDevice = new obsidian.SecretStorage(); + usePlugin(new FeedUrlRepository(thisDevice)); + const response = deferred(); + fetchTextMock.mockReturnValue(response.promise); + const replaced = { + title: "Private Show", + url: privateFeedPlaceholder("Private Show"), + urlSecretId: "podnotes-feed-url-00000000-0000-4000-8000-000000000009", + artworkUrl: "", + }; + + const view = renderGrid(); + await setFeedAddress(view, "Private Show", PRIVATE_URL); + savedFeeds.set({ "Private Show": replaced }); + response.resolve("Private Show"); + await settle(); + + expect(thisDevice.listSecrets()).toEqual([]); + expect(get(savedFeeds)).toEqual({ "Private Show": replaced }); + }); + + test("adds nothing when this device cannot store a pasted private feed's address", async () => { + const notice = vi.spyOn(obsidian, "Notice"); + vi.spyOn(console, "error").mockImplementation(() => {}); + const secrets = new obsidian.SecretStorage(); + vi.spyOn(secrets, "setSecret").mockImplementation(() => { + throw new Error("locked"); + }); + usePlugin(new FeedUrlRepository(secrets)); + fetchTextMock.mockResolvedValue( + "Private Show", + ); + + const { findByRole } = paste(PRIVATE_URL); + await fireEvent.click(await findByRole("button", { name: "Add Private Show podcast" })); + + expect(get(savedFeeds)).toEqual({}); + expect(notice).toHaveBeenCalledWith( + 'Could not store the private feed address for "Private Show" on this device.', + ); + }); + + test("removing a private feed saved under two names keeps its address for the other", async () => { + const { saved, secrets } = followPrivateShow(); + const alias = { ...saved, title: "Alias", urlSecretId: ` ${saved.urlSecretId} ` }; + savedFeeds.set({ "Private Show": saved, Alias: alias }); + + const { findByRole } = renderGrid(); + await fireEvent.click(await findByRole("button", { name: "Remove Alias podcast" })); + + expect(get(savedFeeds)).toEqual({ "Private Show": saved }); + expect(secrets.getSecret(saved.urlSecretId)).toBe(PRIVATE_URL); + + await fireEvent.click(await findByRole("button", { name: "Remove Private Show podcast" })); + + expect(get(savedFeeds)).toEqual({}); + expect(secrets.getSecret(saved.urlSecretId)).toBeFalsy(); + }); + test("keeps the newest search when an older one answers last", async () => { const older = deferred(); const newer = deferred(); diff --git a/src/ui/settings/PodcastResultCard.svelte b/src/ui/settings/PodcastResultCard.svelte index ca3311e9..f4394da0 100644 --- a/src/ui/settings/PodcastResultCard.svelte +++ b/src/ui/settings/PodcastResultCard.svelte @@ -6,6 +6,7 @@ export let podcast: PodcastFeed; export let isSaved = false; + export let needsAddress = false; const dispatch = createEventDispatcher(); @@ -25,6 +26,13 @@ {/if}
+ {#if needsAddress} +