diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index f1bc6b82..2ae27381 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, manualClock } 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)); @@ -273,43 +276,315 @@ 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: "", + 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(); + const written = JSON.stringify(saveData.mock.calls.at(-1)?.[0]); + expect(written).toContain('"feedUrl":"podnotes-private-feed:Private"'); + expect(written).not.toContain("auth=leaked"); + + const writes = saveData.mock.calls.length; + await plugin.loadSettings(); + 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", 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 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(resolveFeedUrl(plugin.settings.savedFeeds["PRIV2 Both"], plugin.feedUrls)).toBe( + secretUrl, + ); + }); + + it.each(["throws", "drops the write"])( + "keeps saving with a session-only private feed when storage %s", + async (failure) => { + const rawUrl = "https://private.example/feed.xml?auth=SESSION_SENTINEL"; + const { storage } = memorySecretStorage(); + vi.mocked(storage.setSecret).mockImplementation(() => { + if (failure === "throws") throw new Error("locked"); + }); + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData, { dataFileExists: true }); + Object.assign(plugin, { app: { ...plugin.app, secretStorage: storage } }); + const raw = { + ...encodePodNotesData(structuredClone(DEFAULT_SETTINGS)), + savedFeeds: { Private: { title: "Private", url: rawUrl, artworkUrl: "" } }, + }; + await plugin.saveData(raw); + saveData.mockClear(); + const notice = vi.spyOn(obsidian, "Notice"); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await plugin.loadSettings(); + attachDataFileWriter(plugin); + plugin.settings.defaultVolume = 0.4; + await plugin.saveSettingsStrict(); + plugin.settings.skipForwardLength = 42; + await plugin.saveSettingsStrict(); + + expect(resolveFeedUrl(plugin.settings.savedFeeds.Private, plugin.feedUrls)).toBe( + rawUrl, + ); + const disk = await plugin.loadData(); + expect(disk).toMatchObject({ + defaultVolume: 0.4, + skipForwardLength: 42, + savedFeeds: { Private: { url: "podnotes-private-feed:Private" } }, + }); + for (const [written] of saveData.mock.calls) + expect(JSON.stringify(written)).not.toContain("SESSION_SENTINEL"); + expect(notice.mock.calls).toEqual([ + ["Could not store the private feed address on this device."], + ]); + + const restarted = makePlugin(); + Object.assign(restarted, { app: { ...restarted.app, secretStorage: storage } }); + await restarted.saveData(disk); + await restarted.loadSettings(); + expect(restarted.settings.savedFeeds.Private).toEqual( + plugin.settings.savedFeeds.Private, + ); + expect( + resolveFeedUrl(restarted.settings.savedFeeds.Private, restarted.feedUrls), + ).toBeNull(); + }, + ); + + 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", - url: "", + streamUrl: "https://media.example/ep.mp3?sig=media", + url: rawUrl, description: "", content: "", podcastName: "Private", - feedUrl: secretUrl, + 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("saves other changes and a session-only reference when a synced private address cannot be stored", 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); - await plugin.loadSettings(); - const written = JSON.stringify(saveData.mock.calls.at(-1)?.[0]); - expect(written).toContain('"feedUrl":"podnotes-private-feed:Private"'); - expect(written).not.toContain("auth=leaked"); + plugin.settings.defaultVolume = 0.4; - const writes = saveData.mock.calls.length; + await plugin.saveSettingsStrict(); + expect(await plugin.loadData()).toMatchObject({ + defaultVolume: 0.4, + savedFeeds: { Private: { url: "podnotes-private-feed:Private" } }, + }); + expect(resolveFeedUrl(plugin.settings.savedFeeds.Private, plugin.feedUrls)).toBe(rawUrl); + expect(JSON.stringify(saveData.mock.calls)).not.toContain(rawUrl); + }); + + 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(); - expect(saveData).toHaveBeenCalledTimes(writes); + 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 () => { diff --git a/src/main.ts b/src/main.ts index eb650db1..d9ceb4b2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -48,7 +48,8 @@ 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 = @@ -458,12 +459,17 @@ 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); + 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 +482,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 +502,12 @@ export default class PodNotes extends Plugin implements IPodNotes { `Moved your ${migratedProviders.join(" and ")} API ${migratedProviders.length === 1 ? "key" : "keys"} into Obsidian SecretStorage.`, ); } - } - - // 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) { + if (movedFeeds && feedProtection.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; @@ -610,7 +601,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 { @@ -645,9 +640,15 @@ 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); + 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. @@ -657,9 +658,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), }; @@ -708,8 +713,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 a7bf503d..698c869f 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), }, }); @@ -230,7 +233,7 @@ describe("importOPML (IE-01)", () => { expect(channelAt).not.toHaveBeenCalled(); expect(get(savedFeeds)).toEqual({ Private: saved }); - expect(noticeMessages.some((m) => m.includes("Skipped 1 existing podcasts"))).toBe(true); + expect(noticeMessages.some((m) => m.includes("Skipped 1 existing podcast"))).toBe(true); }); it("skips a saved private feed listed by its real url with an uppercase host", async () => { @@ -250,15 +253,19 @@ describe("importOPML (IE-01)", () => { expect(channelAt).not.toHaveBeenCalled(); expect(get(savedFeeds)).toEqual({ Private: saved }); - expect(noticeMessages.some((m) => m.includes("Skipped 1 existing podcasts"))).toBe(true); + expect(noticeMessages.some((m) => m.includes("Skipped 1 existing podcast"))).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,61 @@ 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("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 = { + 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 () => { @@ -294,7 +352,7 @@ describe("importOPML (IE-01)", () => { // Nothing to fetch, so the 0/0 progress division must not surface as NaN. expect(channelAt).not.toHaveBeenCalled(); expect(noticeMessages.some((m) => m.includes("NaN"))).toBe(false); - expect(noticeMessages.some((m) => m.includes("Saved 0 new podcasts"))).toBe(true); + expect(noticeMessages.some((m) => m.includes("Saved 0 podcasts"))).toBe(true); }); it("reports the saved count, not the fetched count, on duplicate titles", async () => { @@ -317,8 +375,8 @@ describe("importOPML (IE-01)", () => { expect(channelAt).toHaveBeenCalledTimes(2); expect(Object.keys(get(savedFeeds))).toEqual(["Same Title"]); // Exactly one was written, so the summary must say "Saved 1", not "Saved 2". - expect(noticeMessages.some((m) => m.includes("Saved 1 new podcasts"))).toBe(true); - expect(noticeMessages.some((m) => m.includes("Saved 2 new podcasts"))).toBe(false); + expect(noticeMessages.some((m) => m.includes("Saved 1 podcast"))).toBe(true); + expect(noticeMessages.some((m) => m.includes("Saved 2 podcasts"))).toBe(false); expect(noticeMessages.some((m) => m.includes("Skipped 1 with duplicate titles"))).toBe( true, ); @@ -351,7 +409,7 @@ describe("importOPML (IE-01)", () => { expect(Object.keys(get(savedFeeds))).toEqual(["Same Title"]); // The original feed must be preserved, not overwritten. expect(get(savedFeeds)["Same Title"].url).toBe("https://old.test/feed"); - expect(noticeMessages.some((m) => m.includes("Saved 0 new podcasts"))).toBe(true); + expect(noticeMessages.some((m) => m.includes("Saved 0 podcasts"))).toBe(true); expect(noticeMessages.some((m) => m.includes("Skipped 1 with duplicate titles"))).toBe( true, ); @@ -385,8 +443,8 @@ describe("importOPML (IE-01)", () => { expect( noticeMessages.some( (m) => - m.includes("Saved 1 new podcasts") && - m.includes("Skipped 1 existing podcasts") && + m.includes("Saved 1 podcast") && + m.includes("Skipped 1 existing podcast") && m.includes("Skipped 1 with duplicate titles"), ), ).toBe(true); diff --git a/src/opml.ts b/src/opml.ts index 29bdf6e0..9c38cc12 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,14 +136,21 @@ 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.`; + let summary = `OPML import complete. Saved ${savedCount} ${savedCount === 1 ? "podcast" : "podcasts"}. Skipped ${skippedExisting} existing ${skippedExisting === 1 ? "podcast" : "podcasts"}.`; if (droppedDuplicateTitle > 0) { summary += ` Skipped ${droppedDuplicateTitle} with duplicate titles.`; } 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 80046dfd..78d6613a 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -60,6 +60,183 @@ function settingsWithEveryEpisodeContainer(): IPodNotesSettings { } describe("PodNotes data schema", () => { + 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 }; + 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; + 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 [ + 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); + 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("keeps distinct resource URLs that share a private feed address prefix, including quoted links", () => { + const feedUrl = "https://private.example/resource?token=FEED_TOKEN"; + const streamUrl = `${feedUrl}_DISTINCT_MEDIA_SIGNATURE&kind=audio`; + const item = { + ...episode("Private"), + feedUrl, + streamUrl, + artworkUrl: `${feedUrl}.`, + chaptersUrl: `${feedUrl}&sig=CHAPTERS`, + url: `${feedUrl}&token=ARTICLE`, + description: `Audio Feed: "${feedUrl}"`, + }; + const encoded = encodePodNotesData({ + ...structuredClone(DEFAULT_SETTINGS), + currentEpisode: item, + }); + expect(encoded.currentEpisode).toEqual({ + ...item, + episodeDate: episodeDate.toISOString(), + feedUrl: "podnotes-private-feed:Test%20Podcast", + description: `Audio Feed: "podnotes-private-feed:Test%20Podcast"`, + }); + }); + + it.each([ + ["See URL.", "See PLACEHOLDER."], + ["See URL, then listen.", "See PLACEHOLDER, then listen."], + ["[Feed](URL)", "[Feed](PLACEHOLDER)"], + ["See URL; listen next.", "See PLACEHOLDER; listen next."], + ["URL#section", "PLACEHOLDER"], + ])("puts no private feed address in data.json when quoted as %s", (template, expected) => { + const feedUrl = "https://private.example/feed?auth=PUNCTUATION_SENTINEL"; + const item = { + ...episode("Private"), + feedUrl, + description: template.replace("URL", feedUrl), + }; + const encoded = encodePodNotesData({ + ...structuredClone(DEFAULT_SETTINGS), + currentEpisode: item, + }); + expect(encoded.currentEpisode?.description).toBe( + expected.replace("PLACEHOLDER", "podnotes-private-feed:Test%20Podcast"), + ); + expect(JSON.stringify(encoded)).not.toContain("PUNCTUATION_SENTINEL"); + }); + + it("puts no private feed address in data.json object keys without losing unknown entries", () => { + const feedUrl = "https://private.example/feed?token=KEY_SENTINEL"; + const item = { ...episode("Private"), feedUrl }; + const raw = { + ...encodePodNotesData(DEFAULT_SETTINGS), + futureByFeed: JSON.parse( + JSON.stringify({ [feedUrl]: item, ["__proto__"]: { keep: true } }), + ), + }; + const decoded = decodePodNotesData(raw); + const encoded = encodePodNotesData(decoded.settings, decoded.unknownFields); + expect(JSON.stringify(encoded)).not.toContain("KEY_SENTINEL"); + expect(encoded.futureByFeed).toEqual({ + ["podnotes-private-feed:Test%20Podcast"]: { + ...item, + feedUrl: "podnotes-private-feed:Test%20Podcast", + episodeDate: episodeDate.toISOString(), + }, + ["__proto__"]: { keep: true }, + }); + expect(JSON.stringify(raw)).toContain("KEY_SENTINEL"); + }); + + it("refuses colliding private feed keys instead of overwriting preserved data", () => { + const feedUrl = "https://private.example/feed?token=COLLISION"; + const settings = { + ...structuredClone(DEFAULT_SETTINGS), + currentEpisode: { ...episode("Private"), feedUrl }, + }; + expect(() => + encodePodNotesData(settings, { + futureByFeed: { + [feedUrl]: { keep: "first" }, + ["podnotes-private-feed:Test%20Podcast"]: { keep: "second" }, + }, + }), + ).toThrow("Private feed address protection would overwrite a saved field."); + }); + it("loads fresh data as independent deep-cloned defaults", () => { const first = decodePodNotesData(undefined); const second = decodePodNotesData(null); diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index bca9b335..11c7aba8 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 + * have a stored or session-only reference first; raw addresses are refused. + */ 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..a07df432 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": @@ -108,6 +154,15 @@ describe("FeedUrlRepository", () => { expect(values.get("podnotes-openai-api-key")).toBe("sk-keep"); }); + it("keeps an ID that data.json references as an unknown map key", () => { + const id = "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef"; + const url = "https://private.example/feed?token=KEEP"; + const { api } = storage({ [id]: url }); + const repository = new FeedUrlRepository(api); + repository.sweepOrphans({ futureFeedsByReference: { [id]: { title: "Private" } } }); + expect(repository.resolve(id)).toBe(url); + }); + it("survives a listSecrets failure without throwing", () => { const { api } = storage(); (api.listSecrets as ReturnType).mockImplementation(() => { diff --git a/src/services/FeedUrlRepository.ts b/src/services/FeedUrlRepository.ts index 06f62c3f..1a293025 100644 --- a/src/services/FeedUrlRepository.ts +++ b/src/services/FeedUrlRepository.ts @@ -1,18 +1,19 @@ -import type { SecretStorage } from "obsidian"; -import type { PodcastFeed } from "src/types/PodcastFeed"; +import { Notice, type SecretStorage } from "obsidian"; import { isValidSecretId } from "src/types/Credentials"; /** * Vault-local storage for private feed URLs, following CredentialRepository's - * contract: runtime reads fail closed and never cache secret values; writes - * return an ID only after the exact value reads back. + * contract: stored references resolve from SecretStorage; writes return an ID + * only after the exact value reads back. Failed moves from data.json can retain + * the address for this session under a separate, temporary reference. * * IDs are `podnotes-feed-url-`. The UUID matters: `urlSecretId` * 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. */ @@ -24,12 +25,17 @@ export function isFeedUrlSecretId(value: string): boolean { } export class FeedUrlRepository { + private readonly sessionUrls = new Map(); + private warnedAboutStorage = false; + constructor(private readonly storage: SecretStorage) {} /** The real feed URL, or null when the reference is invalid or absent on this device. */ resolve(urlSecretId: string): string | null { const id = urlSecretId.trim(); if (!id || !isFeedUrlSecretId(id) || !isValidSecretId(id)) return null; + const sessionUrl = this.sessionUrls.get(id); + if (sessionUrl) return sessionUrl; try { const value = this.storage.getSecret(id); return value?.trim() ? value.trim() : null; @@ -41,21 +47,52 @@ export class FeedUrlRepository { /** Store a private feed URL under a fresh PodNotes-owned ID and verify the readback. */ store(url: string): string { - const value = url.trim(); - if (!value) throw new Error("A private feed URL must not be empty."); + const id = `podnotes-feed-url-${crypto.randomUUID()}`; + this.attach(id, url); + return id; + } + /** A failed move still removes the raw URL from disk and works until restart. */ + retainForSession(url: string): string { + for (const [id, value] of this.sessionUrls) { + if (value === url) return id; + } const id = `podnotes-feed-url-${crypto.randomUUID()}`; + this.sessionUrls.set(id, url); + if (!this.warnedAboutStorage) { + this.warnedAboutStorage = true; + new Notice("Could not store the private feed address on this device."); + } + 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 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. */ delete(urlSecretId: string): void { const id = urlSecretId.trim(); if (!id || !isFeedUrlSecretId(id)) return; + this.sessionUrls.delete(id); try { this.storage.setSecret(id, ""); } catch (error) { @@ -64,18 +101,25 @@ 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 [key, entry] of Object.entries(value)) { + collect(key); + 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 e683699a..bfa7bda0 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(); @@ -70,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); }); }); @@ -298,20 +312,24 @@ describe("protectPrivateFeeds", () => { expect(safe.queue.episodes[0].url).toBe(link); }); - it("reports feeds whose URL could not be protected", () => { + it("keeps a failed move available only for the session and redacts its snapshots", () => { 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.savedFeeds.Patreon.url).toBe(PRIVATE_URL); - expect(result.queue.episodes[0].feedUrl).toBe("podnotes-private-feed:Patreon"); + expect(JSON.stringify(result)).not.toContain(PRIVATE_URL); + expect(result.savedFeeds.Patreon.url).toBe("podnotes-private-feed:Patreon"); + expect(resolveFeedUrl(result.savedFeeds.Patreon, feedUrls)).toBe(PRIVATE_URL); + expect(resolveFeedUrl(result.savedFeeds.Patreon, new FeedUrlRepository(api))).toBeNull(); }); }); diff --git a/src/services/privateFeeds.ts b/src/services/privateFeeds.ts index 1fc0bb61..83b0fa14 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"; @@ -21,8 +22,9 @@ import type { FeedUrlRepository } from "./FeedUrlRepository"; * value confined to the fetch call and never persist or log it. Outside a * fetch, a secret is resolved only to compare it with a URL already in hand * (`isSavedFeed`, which returns just the answer) or to check that it exists - * on this device. OPML export is the one deliberate exception: the user asks - * for a file that carries the real feed URLs. + * on this device. The persistence boundary also resolves addresses to replace + * their copies with placeholders. OPML export deliberately carries the real + * feed URLs at the user's request. */ /** @@ -31,9 +33,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 +46,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 +98,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 addresses are available only for this session. */ failed: string[]; } @@ -105,57 +124,46 @@ 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); + interned = { + ...feed, + url: privateFeedPlaceholder(key), + urlSecretId: feedUrls.retainForSession(feed.url), + }; } + 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); 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/settingsTransfer.test.ts b/src/settingsTransfer.test.ts index f5f157e4..5f7c0394 100644 --- a/src/settingsTransfer.test.ts +++ b/src/settingsTransfer.test.ts @@ -17,8 +17,67 @@ function makeSettings(overrides: Partial = {}): IPodNotesSett const NOW = "2026-06-14T00:00:00.000Z"; -describe("private feed reference stripping", () => { - it("drops device-local urlSecretId from exported savedFeeds but keeps the placeholder", async () => { +describe("private feed reference transfer", () => { + it("puts no private feed address in a settings export, including quoted episode text", () => { + const address = "https://private.example/feed?auth=EXPORT_SENTINEL"; + const placeholder = "podnotes-private-feed:Private"; + const settings = makeSettings({ + savedFeeds: { + Private: { + title: "Private", + url: placeholder, + artworkUrl: "", + urlSecretId: "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef", + }, + }, + favorites: { + ...DEFAULT_SETTINGS.favorites, + episodes: [ + { + title: "Private episode", + podcastName: "Private", + feedUrl: placeholder, + streamUrl: "https://media.example/audio.mp3?sig=KEEP_MEDIA", + url: "https://example.com/episode", + content: "", + description: `Your feed: ${address}`, + }, + ], + }, + }); + const envelope = serializeSettings( + settings, + {}, + "2.21.1", + NOW, + new Map([[address, placeholder]]), + ); + expect(JSON.stringify(envelope)).not.toContain("EXPORT_SENTINEL"); + expect(envelope.settings.favorites?.episodes[0]).toMatchObject({ + description: `Your feed: ${placeholder}`, + streamUrl: "https://media.example/audio.mp3?sig=KEEP_MEDIA", + }); + expect(settings.favorites.episodes[0].description).toContain("EXPORT_SENTINEL"); + }); + it("drops a foreign secret reference instead of transferring it as a feed identity", () => { + const result = parseImport( + JSON.stringify({ + savedFeeds: { + Private: { + title: "Private", + url: "podnotes-private-feed:Private", + artworkUrl: "", + urlSecretId: "podnotes-openai-api-key", + }, + }, + }), + ); + expect(result.ok).toBe(true); + if (!result.ok) throw new Error(result.error); + expect(result.settings.savedFeeds?.Private.urlSecretId).toBeUndefined(); + }); + + it("keeps the existing private feed ID and placeholder in settings exports", async () => { const { serializeSettings } = await import("./settingsTransfer"); const { DEFAULT_SETTINGS } = await import("./constants"); const settings = { @@ -36,11 +95,11 @@ describe("private feed reference stripping", () => { const exported = ( envelope.settings as unknown as Record>> ).savedFeeds.Private; - expect(exported.urlSecretId).toBeUndefined(); + expect(exported.urlSecretId).toBe(settings.savedFeeds.Private.urlSecretId); expect(exported.url).toBe("podnotes-private-feed:Private"); }); - it("drops urlSecretId arriving through an import", async () => { + it("keeps an imported private feed ID for setup on this device", async () => { const { parseImport } = await import("./settingsTransfer"); const result = parseImport( JSON.stringify({ @@ -59,7 +118,7 @@ describe("private feed reference stripping", () => { const feed = ( result.settings.savedFeeds as unknown as Record> ).Foreign; - expect(feed.urlSecretId).toBeUndefined(); + expect(feed.urlSecretId).toBe("podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef"); expect(feed.url).toBe("podnotes-private-feed:Foreign"); } }); diff --git a/src/settingsTransfer.ts b/src/settingsTransfer.ts index 1a2bd452..4ab37ec4 100644 --- a/src/settingsTransfer.ts +++ b/src/settingsTransfer.ts @@ -12,6 +12,7 @@ import type { Episode } from "./types/Episode"; import type { IPodNotesSettings } from "./types/IPodNotesSettings"; import { ownRecord } from "./utility/ownRecord"; import { isLocalFile } from "./utility/isLocalFile"; +import { isFeedUrlSecretId } from "./services/FeedUrlRepository"; /** * Serialization for the "import/export settings & templates" feature (issue #162). @@ -33,7 +34,7 @@ export const VAULT_LOCAL_KEYS: readonly (keyof IPodNotesSettings)[] = [ "currentEpisode", ]; -/** SecretStorage references are device-local implementation details, not settings transfer data. */ +/** Provider API-key references are device-local, not settings transfer data. */ const SECRET_REFERENCE_KEYS: ReadonlySet = new Set([ "openAISecretId", "deepgramSecretId", @@ -113,10 +114,18 @@ export type ParseResult = function portable(settings: T): T { const out = { ...settings } as Record; - // SecretStorage is device-local: an imported urlSecretId is at best meaningless here. if (isPlainObject(out.savedFeeds)) { out.savedFeeds = mapRecord(out.savedFeeds, (feed) => - isPlainObject(feed) ? { ...feed, urlSecretId: undefined } : feed, + isPlainObject(feed) + ? { + ...feed, + urlSecretId: + typeof feed.urlSecretId === "string" && + isFeedUrlSecretId(feed.urlSecretId) + ? feed.urlSecretId + : undefined, + } + : feed, ); } for (const key of ["queue", "favorites"]) { @@ -161,9 +170,10 @@ export function serializeSettings( opts: ExportOptions, pluginVersion: string, nowISO: string, + privateFeedAddresses?: ReadonlyMap, ): SettingsEnvelope { const out: Record = {}; - const persisted = encodePodNotesData(settings); + const persisted = encodePodNotesData(settings, {}, privateFeedAddresses); for (const key of Object.keys(settings)) { if (DANGEROUS_KEYS.has(key)) continue; 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/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/settings/FeedAddressModal.ts b/src/ui/settings/FeedAddressModal.ts new file mode 100644 index 00000000..7079e5a7 --- /dev/null +++ b/src/ui/settings/FeedAddressModal.ts @@ -0,0 +1,47 @@ +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(); + }; + const setting = new Setting(this.contentEl).setName("Feed address"); + setting.nameEl.id = `podnotes-feed-address-${crypto.randomUUID()}`; + setting.addText((text) => { + input = text.inputEl; + input.setAttribute("aria-labelledby", setting.nameEl.id); + input.addEventListener("keydown", (event) => { + if (event.key !== "Enter") return; + event.preventDefault(); + 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/PodNotesSettingsTab.transfer.test.ts b/src/ui/settings/PodNotesSettingsTab.transfer.test.ts index c3791c9c..225a8f85 100644 --- a/src/ui/settings/PodNotesSettingsTab.transfer.test.ts +++ b/src/ui/settings/PodNotesSettingsTab.transfer.test.ts @@ -142,9 +142,8 @@ describe("importing a settings file", () => { expect(disk().queue.episodes[0].feedUrl).toBe("podnotes-private-feed:Synced"); }); - it("refuses an import whose private feed URL SecretStorage cannot keep", async () => { + it("imports other settings and keeps a failed private-address move for this session", async () => { const { plugin, notice, disk, chooseFile } = setup(); - const before = { settings: plugin.settings, disk: JSON.stringify(disk()) }; const setSecret = vi .spyOn(obsidian.SecretStorage.prototype, "setSecret") .mockImplementation(() => { @@ -156,21 +155,22 @@ describe("importing a settings file", () => { consoleError.mockRestore(); }); - chooseFile({ + await chooseFile({ ...structuredClone(DEFAULT_SETTINGS), schemaVersion: 2, + defaultVolume: 0.4, savedFeeds: { Private: { title: "Private", url: privateUrl, artworkUrl: "" } }, - }).click(); + }).confirm(); - await vi.waitFor(() => - expect(notice).toHaveBeenCalledWith( - "Could not finish importing secrets into Obsidian SecretStorage. Existing settings were kept, and retrying is safe.", - 10000, - ), + expect(notice).toHaveBeenCalledWith( + "Could not store the private feed address on this device.", + ); + expect(disk().defaultVolume).toBe(0.4); + expect(disk().savedFeeds.Private.url).toBe("podnotes-private-feed:Private"); + expect(JSON.stringify(disk())).not.toContain("s3cret"); + expect(plugin.feedUrls.resolve(disk().savedFeeds.Private.urlSecretId ?? "")).toBe( + privateUrl, ); - expect(notice).not.toHaveBeenCalledWith("Imported PodNotes settings."); - expect(plugin.settings).toBe(before.settings); - expect(JSON.stringify(disk())).toBe(before.disk); }); it("never writes a private feed URL that an older build left in the imported file", async () => { diff --git a/src/ui/settings/PodNotesSettingsTab.ts b/src/ui/settings/PodNotesSettingsTab.ts index e43d16f9..4116dc52 100644 --- a/src/ui/settings/PodNotesSettingsTab.ts +++ b/src/ui/settings/PodNotesSettingsTab.ts @@ -37,7 +37,7 @@ import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import { get } from "svelte/store"; import { exportOPML, importOPML } from "src/opml"; import { decodePodNotesData, PODNOTES_DATA_SCHEMA_VERSION } from "src/persistence/podNotesData"; -import { protectPrivateFeeds } from "src/services/privateFeeds"; +import { privateFeedAddresses, protectPrivateFeeds } from "src/services/privateFeeds"; import { describeSecrets, mergeImportedSettings, @@ -839,6 +839,7 @@ export class PodNotesSettingsTab extends PluginSettingTab { { secrets: includeSecret ? secrets : undefined }, this.plugin.manifest.version, new Date().toISOString(), + privateFeedAddresses(this.plugin.settings.savedFeeds, this.plugin.feedUrls), ); const contents = JSON.stringify(envelope, null, 2); @@ -934,7 +935,6 @@ export class PodNotesSettingsTab extends PluginSettingTab { mergeImportedSettings(previous, { ...imported, ...secretReferences }), this.plugin.feedUrls, ); - if (protection.failed.length > 0) throw new Error(); merged = protection.settings; } catch (error) { new Notice( diff --git a/src/ui/settings/PodcastQueryGrid.svelte b/src/ui/settings/PodcastQueryGrid.svelte index 707e9c31..af56cbfc 100644 --- a/src/ui/settings/PodcastQueryGrid.svelte +++ b/src/ui/settings/PodcastQueryGrid.svelte @@ -8,14 +8,18 @@ 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 { isCredentialBearingUrl } from "src/utility/privateFeedUrl"; 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 +127,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 +165,61 @@ } // 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; + } + if (!isCredentialBearingUrl(url.href)) { + new Notice("Enter a private feed 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 +250,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 c5307a34..b035dc54 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. @@ -197,6 +209,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")); @@ -221,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(); @@ -231,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.", @@ -241,6 +268,119 @@ 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("refuses a public recovery address before fetching it", async () => { + const notice = vi.spyOn(obsidian, "Notice"); + const { saved } = followPrivateShow(); + const thisDevice = new obsidian.SecretStorage(); + usePlugin(new FeedUrlRepository(thisDevice)); + const view = renderGrid(); + await setFeedAddress(view, "Private Show", "https://public.example/feed.xml"); + expect(fetchTextMock).not.toHaveBeenCalled(); + expect(notice).toHaveBeenCalledWith("Enter a private feed address."); + expect(thisDevice.listSecrets()).toEqual([]); + expect(get(savedFeeds)).toEqual({ "Private Show": saved }); + }); + + 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("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..ccc404e0 100644 --- a/src/ui/settings/PodcastResultCard.svelte +++ b/src/ui/settings/PodcastResultCard.svelte @@ -6,11 +6,16 @@ export let podcast: PodcastFeed; export let isSaved = false; + export let needsAddress = false; const dispatch = createEventDispatcher(); -
+
+ {#if needsAddress} +