From 7c87c766688d737b895f91ebbb534a56691bc270 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 11 Jul 2026 14:26:31 +0200 Subject: [PATCH 1/2] feat(security): keep private feed URLs out of synced data A credentialed subscription URL (userinfo or an auth-bearing query parameter, e.g. Patreon's ?auth=...) is a durable secret; persisting it in data.json leaks it through vault sync, git-versioned vaults, and shared vaults. Private feeds now persist a non-fetchable placeholder plus a reference into Obsidian's vault-local SecretStorage, and the real URL is resolved only immediately before retrieval. - FeedUrlRepository: write-verified storage under content-free podnotes-feed-url IDs, deletion on unsubscribe, and an orphan sweep on load (covers settings imports replacing savedFeeds wholesale). - Existing subscriptions migrate on load, best-effort per feed; a SecretStorage failure keeps that feed exactly as it was. - Every ingress interns (subscribe, OPML import, load); every fetch seam resolves (episode refresh, feed-note enrichment, URI links). - FeedParser projects private-feed episodes against the SAVED feed so the persisted episode cache and currentEpisode carry the placeholder, never the resolved secret - including the URI-handler path. - A device without the secret gets an actionable notice; OPML export (a deliberate portability action) exports the real URL. Known residual: Supercast-style path-embedded tokens are not detectable and keep the status quo; enclosure URLs inside cached episodes are short-lived signatures and out of scope. --- src/URIHandler.ts | 41 +++++++- src/createFeedNote.ts | 5 +- src/main.ts | 19 ++++ src/opml.test.ts | 19 ++++ src/opml.ts | 12 ++- src/parser/feedParser.test.ts | 21 +++++ src/parser/feedParser.ts | 7 +- src/persistence/collectionCodecs.ts | 1 + src/services/FeedUrlRepository.test.ts | 110 ++++++++++++++++++++++ src/services/FeedUrlRepository.ts | 98 +++++++++++++++++++ src/services/privateFeeds.test.ts | 119 ++++++++++++++++++++++++ src/services/privateFeeds.ts | 82 ++++++++++++++++ src/types/PodcastFeed.ts | 6 ++ src/ui/PodcastView/PodcastView.svelte | 5 +- src/ui/settings/PodcastQueryGrid.svelte | 10 +- src/utility/privateFeedUrl.test.ts | 56 +++++++++++ src/utility/privateFeedUrl.ts | 61 ++++++++++++ 17 files changed, 660 insertions(+), 12 deletions(-) create mode 100644 src/services/FeedUrlRepository.test.ts create mode 100644 src/services/FeedUrlRepository.ts create mode 100644 src/services/privateFeeds.test.ts create mode 100644 src/services/privateFeeds.ts create mode 100644 src/utility/privateFeedUrl.test.ts create mode 100644 src/utility/privateFeedUrl.ts diff --git a/src/URIHandler.ts b/src/URIHandler.ts index 438b06f3..f25afce3 100644 --- a/src/URIHandler.ts +++ b/src/URIHandler.ts @@ -12,11 +12,15 @@ import { localFiles, playedEpisodes, plugin, + savedFeeds, requestedPlaybackTime, viewState, } from "./store"; import type { Episode } from "./types/Episode"; +import type { PodcastFeed } from "./types/PodcastFeed"; import { isFetchableUrl } from "./utility/assertFetchableUrl"; +import { resolveFeedUrl } from "./services/privateFeeds"; +import { isPrivateFeedPlaceholder, parsePrivateFeedPlaceholder } from "./utility/privateFeedUrl"; import { getEpisodeKey } from "./utility/episodeKey"; import { ViewState } from "./types/ViewState"; @@ -167,7 +171,10 @@ export default async function podNotesURIHandler( episode = nameCandidates .map((name) => localFiles.getLocalEpisode(name)) .find((ep) => ep !== undefined); - } else if (!candidateValues(url).some((u) => /^https?:\/\//i.test(u))) { + } else if ( + !isPrivateFeedPlaceholder(url) && + !candidateValues(url).some((u) => /^https?:\/\//i.test(u)) + ) { // The probe found no file, yet `url` has no http(s) scheme, so it can only be // a vault path whose file was moved/renamed. Resolve the episode by name from // the local-files store rather than handing the bare path to FeedParser (which @@ -176,11 +183,34 @@ export default async function podNotesURIHandler( .map((name) => localFiles.getLocalEpisode(name)) .find((ep) => ep !== undefined); } else { + // Links for a private feed carry the non-fetchable placeholder, not the + // secret URL. Resolution is local-only: the placeholder's feed name must + // match a saved feed on THIS device, and the real URL comes from + // SecretStorage. An attacker-crafted placeholder link can therefore only + // make the user's own device fetch the user's own subscribed feed - and + // the resolved URL still passes the fetchability gate below. + let fetchUrl = url; + let placeholderFeed: PodcastFeed | undefined; + const placeholderName = parsePrivateFeedPlaceholder(url); + if (placeholderName !== null) { + placeholderFeed = get(savedFeeds)[placeholderName]; + const resolved = placeholderFeed + ? resolveFeedUrl(placeholderFeed, get(plugin).feedUrls) + : null; + if (resolved === null) { + new Notice( + "This link points to a private feed that is not available on this device.", + ); + return; + } + fetchUrl = resolved; + } + // The url here came straight from an untrusted obsidian://podnotes deep link // (an attacker can put one behind on a web page), so a single click // must not be able to fetch an arbitrary internal host. Refuse anything that // isn't a public http(s) URL before handing it to FeedParser (blind SSRF). - if (!isFetchableUrl(url)) { + if (!isFetchableUrl(fetchUrl)) { new Notice("Refusing to load a feed from a private, local, or non-http(s) URL"); return; } @@ -190,8 +220,11 @@ export default async function podNotesURIHandler( // the legacy-candidate treatment. A '+' in a legacy feed URL is pre-existing and out of // scope. getEpisodes returns fully-populated episodes, so we match in memory rather than // re-fetching once per candidate. - const feedparser = new FeedParser(); - const episodes = await feedparser.getEpisodes(url); + // For a private feed, parse against the SAVED feed so episodes keep the + // placeholder identity - never the resolved secret - on url/feedUrl + // (the picked episode persists via currentEpisode). + const feedparser = new FeedParser(placeholderFeed); + const episodes = await feedparser.getEpisodes(fetchUrl); episode = findEpisodeByCandidates(episodes, nameCandidates); } catch (error) { // A fetch/parse failure is distinct from a genuine title miss; surface it instead of diff --git a/src/createFeedNote.ts b/src/createFeedNote.ts index a62c012c..432278d2 100644 --- a/src/createFeedNote.ts +++ b/src/createFeedNote.ts @@ -7,6 +7,7 @@ import addExtension from "./utility/addExtension"; import { enforceMaxPathLength } from "./utility/enforceMaxPathLength"; import { ensureFolderExists } from "./utility/ensureFolderExists"; import FeedParser from "./parser/feedParser"; +import { resolveFeedUrl } from "./services/privateFeeds"; /** * Resolve the on-disk path of a feed's note from the feed-note path template. @@ -84,9 +85,11 @@ async function enrichFeed(feed: PodcastFeed): Promise { if (!needsEnrichment || !feed.url) { return feed; } + const feedUrl = resolveFeedUrl(feed, get(plugin).feedUrls); + if (feedUrl === null) return feed; try { - const parsed = await new FeedParser(feed).getFeed(feed.url); + const parsed = await new FeedParser(feed).getFeed(feedUrl); // Keep the saved title/url/artwork; only backfill the new metadata fields // so the computed basename can never change mid-flight. Use `||` (not `??`) diff --git a/src/main.ts b/src/main.ts index 12bc8b06..b7f8b579 100644 --- a/src/main.ts +++ b/src/main.ts @@ -41,6 +41,8 @@ import { PodNotesDataError, } from "./persistence/podNotesData"; import { CredentialRepository } from "./services/CredentialRepository"; +import { FeedUrlRepository } from "./services/FeedUrlRepository"; +import { migratePrivateFeedUrls } from "./services/privateFeeds"; type MediaSessionActionName = | "previoustrack" @@ -73,6 +75,7 @@ export default class PodNotes extends Plugin implements IPodNotes { public override settings!: IPodNotesSettings; public override app!: PartialAppExtension; public credentials!: CredentialRepository; + public feedUrls!: FeedUrlRepository; private views = new Set(); @@ -102,6 +105,7 @@ export default class PodNotes extends Plugin implements IPodNotes { this.podcastViewMountEnabled = !this.isMobileRuntime(); plugin.set(this); this.credentials = new CredentialRepository(this.app.secretStorage); + this.feedUrls = new FeedUrlRepository(this.app.secretStorage); await this.loadSettings(); @@ -474,6 +478,21 @@ export default class PodNotes extends Plugin implements IPodNotes { } } + // Private feed URLs: move credential-bearing subscription URLs out of + // data.json into SecretStorage, then reap feed-url secrets no saved feed + // references anymore (e.g. after a settings import replaced savedFeeds). + // Best-effort: a failure keeps the URL where it was, never breaks loading. + this.feedUrls ??= new FeedUrlRepository(this.app.secretStorage); + const feedMigration = migratePrivateFeedUrls(settings.savedFeeds, this.feedUrls); + if (feedMigration.migrated > 0) { + settings = { ...settings, savedFeeds: feedMigration.savedFeeds }; + await this.saveData(encodePodNotesData(settings, decoded.unknownFields)); + new Notice( + `Moved ${feedMigration.migrated} private feed ${feedMigration.migrated === 1 ? "URL" : "URLs"} into Obsidian SecretStorage.`, + ); + } + this.feedUrls.sweepOrphans(settings.savedFeeds); + this.settings = settings; this.persistenceUnknownFields = decoded.unknownFields; diff --git a/src/opml.test.ts b/src/opml.test.ts index f61f16f2..60245fd8 100644 --- a/src/opml.test.ts +++ b/src/opml.test.ts @@ -18,10 +18,29 @@ const getFeed = vi.fn(defaultGetFeed); // accurate saved count). const noticeMessages = vi.hoisted(() => [] as string[]); +const feedUrlSecrets = vi.hoisted(() => new Map()); + vi.mock("./store", () => ({ get savedFeeds() { return savedFeeds; }, + // opml resolves/interns private feed URLs through the plugin's repository; + // a Map-backed stand-in keeps those paths inert for public-feed fixtures. + plugin: { + subscribe: (run: (value: unknown) => void) => { + run({ + feedUrls: { + resolve: (id: string) => feedUrlSecrets.get(id) ?? null, + store: (url: string) => { + feedUrlSecrets.set("podnotes-feed-url", url); + return "podnotes-feed-url"; + }, + delete: (id: string) => feedUrlSecrets.delete(id), + }, + }); + return () => {}; + }, + }, })); vi.mock("./parser/feedParser", () => ({ diff --git a/src/opml.ts b/src/opml.ts index 42dd5661..7789bf2d 100644 --- a/src/opml.ts +++ b/src/opml.ts @@ -1,6 +1,7 @@ import { type App, Notice } from "obsidian"; import FeedParser from "./parser/feedParser"; -import { savedFeeds } from "./store"; +import { plugin, savedFeeds } from "./store"; +import { internPrivateFeed, resolveFeedUrl } from "./services/privateFeeds"; import type { PodcastFeed } from "./types/PodcastFeed"; import { get } from "svelte/store"; @@ -163,7 +164,8 @@ async function importOPML(opml: string): Promise { savedFeeds.update((feeds) => { for (const pod of validPodcasts) { if (feeds[pod.title]) continue; - feeds[pod.title] = structuredClone(pod); + // Imported private URLs go straight to SecretStorage, never data.json. + feeds[pod.title] = internPrivateFeed(structuredClone(pod), get(plugin).feedUrls); savedCount++; } return feeds; @@ -216,8 +218,12 @@ async function exportOPML(app: App, feeds: PodcastFeed[], filePath = "PodNotes_E .replace(//g, ">") .replace(/"/g, """); + // An OPML export is a deliberate, user-initiated portability action, so a + // private feed exports its REAL url (an export with placeholders is useless). + // A private URL that cannot be resolved on this device exports as an empty + // xmlUrl rather than a placeholder. const feedOutline = (feed: PodcastFeed) => - ``; + ``; const feedsOutline = (_feeds: PodcastFeed[]) => `${feeds.map(feedOutline).join("")}`; diff --git a/src/parser/feedParser.test.ts b/src/parser/feedParser.test.ts index c0047772..12bcb087 100644 --- a/src/parser/feedParser.test.ts +++ b/src/parser/feedParser.test.ts @@ -206,6 +206,27 @@ describe("FeedParser", () => { vi.clearAllMocks(); }); + describe("private feed identity (getEpisodes with a resolved secret URL)", () => { + test("stamps episodes with the saved placeholder, never the resolved secret", async () => { + mockRequestWithTimeout.mockResolvedValueOnce(feedResponse(sampleRssFeed)); + const savedFeed: PodcastFeed = { + title: "Test Podcast", + url: "podnotes-private-feed:Test%20Podcast", + urlSecretId: "podnotes-feed-url", + artworkUrl: "", + }; + const resolvedSecretUrl = "https://www.patreon.com/rss/test?auth=se-cret"; + + const episodes = await new FeedParser(savedFeed).getEpisodes(resolvedSecretUrl); + + expect(episodes.length).toBeGreaterThan(0); + for (const episode of episodes) { + expect(episode.feedUrl).toBe("podnotes-private-feed:Test%20Podcast"); + expect(JSON.stringify(episode)).not.toContain("se-cret"); + } + }); + }); + describe("getFeed", () => { test("parses feed title and URL correctly", async () => { mockRequestWithTimeout.mockResolvedValueOnce(feedResponse(sampleRssFeed)); diff --git a/src/parser/feedParser.ts b/src/parser/feedParser.ts index 629bb45d..3ad7ae54 100644 --- a/src/parser/feedParser.ts +++ b/src/parser/feedParser.ts @@ -61,7 +61,12 @@ export default class FeedParser { async getEpisodes(sourceUrl: string): Promise { const xml = await this.source.load(sourceUrl); const existingFeed = this.feed; - if (existingFeed?.url === sourceUrl) { + // A private feed's sourceUrl is its RESOLVED secret while feed.url holds + // the placeholder, so the URLs never match. The caller resolved sourceUrl + // from this very feed, so the pairing holds - and projecting against the + // saved feed keeps the placeholder (never the secret) on every episode's + // url/feedUrl, which flow into the persisted episode cache. + if (existingFeed && (existingFeed.url === sourceUrl || existingFeed.urlSecretId)) { return defaultDocumentParser .parseEpisodeItems(xml) .map((episode) => projectLegacyEpisode(episode, existingFeed)); diff --git a/src/persistence/collectionCodecs.ts b/src/persistence/collectionCodecs.ts index 9b051aa8..f4c9405c 100644 --- a/src/persistence/collectionCodecs.ts +++ b/src/persistence/collectionCodecs.ts @@ -35,6 +35,7 @@ export function decodeSavedFeeds( feed.title = readString(candidate, "title", key, warnings, `savedFeeds.${key}`); feed.url = readString(candidate, "url", "", warnings, `savedFeeds.${key}`); feed.artworkUrl = readString(candidate, "artworkUrl", "", warnings, `savedFeeds.${key}`); + setOptionalString(feed, candidate, "urlSecretId", warnings, `savedFeeds.${key}`); setOptionalString(feed, candidate, "description", warnings, `savedFeeds.${key}`); setOptionalString(feed, candidate, "link", warnings, `savedFeeds.${key}`); setOptionalString(feed, candidate, "author", warnings, `savedFeeds.${key}`); diff --git a/src/services/FeedUrlRepository.test.ts b/src/services/FeedUrlRepository.test.ts new file mode 100644 index 00000000..6b7735fa --- /dev/null +++ b/src/services/FeedUrlRepository.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it, vi } from "vitest"; +import type { SecretStorage } from "obsidian"; +import type { PodcastFeed } from "src/types/PodcastFeed"; +import { FeedUrlRepository, isFeedUrlSecretId } from "./FeedUrlRepository"; + +function storage(initial: Record = {}) { + const values = new Map(Object.entries(initial)); + return { + values, + api: { + getSecret: vi.fn((id: string) => values.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => { + values.set(id, value); + }), + listSecrets: vi.fn(() => [...values.keys()]), + } as unknown as SecretStorage, + }; +} + +function feed(overrides: Partial = {}): PodcastFeed { + return { title: "Show", url: "https://example.com/rss", artworkUrl: "", ...overrides }; +} + +describe("FeedUrlRepository", () => { + it("stores a URL under the PodNotes feed-url ID and verifies the readback", () => { + const { api, values } = storage(); + const id = new FeedUrlRepository(api).store(" https://p.example/rss?auth=tok "); + expect(id).toBe("podnotes-feed-url"); + expect(values.get(id)).toBe("https://p.example/rss?auth=tok"); + }); + + it("reuses an identical value and suffixes past a conflicting one", () => { + const { api } = storage({ "podnotes-feed-url": "https://a.example/rss?auth=1" }); + const repository = new FeedUrlRepository(api); + expect(repository.store("https://a.example/rss?auth=1")).toBe("podnotes-feed-url"); + expect(repository.store("https://b.example/rss?auth=2")).toBe("podnotes-feed-url-2"); + }); + + it("reuses a cleared slot after deletion", () => { + const { api } = storage({ "podnotes-feed-url": "" }); + expect(new FeedUrlRepository(api).store("https://c.example/rss?auth=3")).toBe( + "podnotes-feed-url", + ); + }); + + it("throws when SecretStorage does not retain the value", () => { + const { api } = storage(); + (api.setSecret as ReturnType).mockImplementation(() => {}); + expect(() => new FeedUrlRepository(api).store("https://x.example/rss?auth=t")).toThrow( + /did not retain/, + ); + }); + + it("resolves only PodNotes-owned feed-url IDs and fails closed", () => { + const { api } = storage({ + "podnotes-feed-url": "https://p.example/rss?auth=tok", + "podnotes-openai-api-key": "sk-not-a-feed", + }); + const repository = new FeedUrlRepository(api); + expect(repository.resolve("podnotes-feed-url")).toBe("https://p.example/rss?auth=tok"); + expect(repository.resolve("podnotes-openai-api-key")).toBeNull(); + expect(repository.resolve("podnotes-feed-url-9")).toBeNull(); + expect(repository.resolve("")).toBeNull(); + }); + + it("returns null when the storage read throws", () => { + const { api } = storage(); + (api.getSecret as ReturnType).mockImplementation(() => { + throw new Error("locked"); + }); + expect(new FeedUrlRepository(api).resolve("podnotes-feed-url")).toBeNull(); + }); + + it("deletes by clearing and refuses foreign IDs", () => { + const { api, values } = storage({ + "podnotes-feed-url": "https://p.example/rss?auth=tok", + "podnotes-openai-api-key": "sk-keep", + }); + const repository = new FeedUrlRepository(api); + repository.delete("podnotes-feed-url"); + expect(values.get("podnotes-feed-url")).toBe(""); + repository.delete("podnotes-openai-api-key"); + expect(values.get("podnotes-openai-api-key")).toBe("sk-keep"); + }); + + it("sweeps unreferenced feed-url secrets and nothing else", () => { + const { api, values } = storage({ + "podnotes-feed-url": "https://kept.example/rss?auth=1", + "podnotes-feed-url-2": "https://orphan.example/rss?auth=2", + "podnotes-openai-api-key": "sk-keep", + }); + new FeedUrlRepository(api).sweepOrphans({ + Kept: feed({ urlSecretId: "podnotes-feed-url" }), + Public: feed(), + }); + expect(values.get("podnotes-feed-url")).toBe("https://kept.example/rss?auth=1"); + expect(values.get("podnotes-feed-url-2")).toBe(""); + expect(values.get("podnotes-openai-api-key")).toBe("sk-keep"); + }); +}); + +describe("isFeedUrlSecretId", () => { + it("accepts only the PodNotes feed-url grammar", () => { + expect(isFeedUrlSecretId("podnotes-feed-url")).toBe(true); + expect(isFeedUrlSecretId("podnotes-feed-url-2")).toBe(true); + expect(isFeedUrlSecretId("podnotes-feed-url-0")).toBe(false); + expect(isFeedUrlSecretId("podnotes-openai-api-key")).toBe(false); + expect(isFeedUrlSecretId("feed-url")).toBe(false); + }); +}); diff --git a/src/services/FeedUrlRepository.ts b/src/services/FeedUrlRepository.ts new file mode 100644 index 00000000..f3fc40dc --- /dev/null +++ b/src/services/FeedUrlRepository.ts @@ -0,0 +1,98 @@ +import type { SecretStorage } from "obsidian"; +import type { PodcastFeed } from "src/types/PodcastFeed"; +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. + * + * SecretStorage is device-local, so a second synced device sees the feed's + * placeholder without its secret - the feed is re-added there. IDs are + * content-free (`podnotes-feed-url`, `-2`, `-3`, ...) so neither the feed name + * nor the URL leaks through the ID namespace, which `listSecrets` exposes. + */ +const BASE_ID = "podnotes-feed-url"; +const FEED_URL_SECRET_ID = /^podnotes-feed-url(?:-[1-9]\d*)?$/; + +export function isFeedUrlSecretId(value: string): boolean { + return FEED_URL_SECRET_ID.test(value); +} + +export class FeedUrlRepository { + 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; + try { + const value = this.storage.getSecret(id); + return value?.trim() ? value.trim() : null; + } catch (error) { + console.error("PodNotes: failed to read a private feed URL", error); + return null; + } + } + + /** + * Store a private feed URL under a PodNotes-owned ID. An existing identical + * value is reused (idempotent retries); a conflicting value is never + * overwritten - the first free numeric suffix is used instead. + */ + store(url: string): string { + const value = url.trim(); + if (!value) throw new Error("A private feed URL must not be empty."); + + for (let suffix = 1; suffix <= 10_000; suffix++) { + const id = suffix === 1 ? BASE_ID : `${BASE_ID}-${suffix}`; + const existing = this.storage.getSecret(id); + + if (existing === value) return id; + if (existing !== null && existing !== "") continue; + + this.storage.setSecret(id, value); + if (this.storage.getSecret(id) !== value) { + throw new Error("SecretStorage did not retain the private feed URL."); + } + return id; + } + + throw new Error("Could not allocate a SecretStorage ID for the private feed URL."); + } + + /** Clearing to "" is SecretStorage's deletion idiom; reads treat "" as absent. */ + delete(urlSecretId: string): void { + const id = urlSecretId.trim(); + if (!id || !isFeedUrlSecretId(id)) return; + try { + this.storage.setSecret(id, ""); + } catch (error) { + console.error("PodNotes: failed to delete a private feed URL", error); + } + } + + /** + * 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). + */ + sweepOrphans(feeds: Record): void { + const referenced = new Set(); + for (const feed of Object.values(feeds)) { + if (feed.urlSecretId) referenced.add(feed.urlSecretId); + } + let ids: string[]; + try { + ids = this.storage.listSecrets(); + } catch (error) { + console.error("PodNotes: failed to list secrets for the orphan sweep", error); + return; + } + for (const id of ids) { + if (!isFeedUrlSecretId(id) || referenced.has(id)) continue; + if (!this.storage.getSecret(id)) continue; + this.delete(id); + } + } +} diff --git a/src/services/privateFeeds.test.ts b/src/services/privateFeeds.test.ts new file mode 100644 index 00000000..c6da93e1 --- /dev/null +++ b/src/services/privateFeeds.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it, vi } from "vitest"; +import type { SecretStorage } from "obsidian"; +import type { PodcastFeed } from "src/types/PodcastFeed"; +import { FeedUrlRepository } from "./FeedUrlRepository"; +import { internPrivateFeed, migratePrivateFeedUrls, resolveFeedUrl } from "./privateFeeds"; + +function repository(initial: Record = {}) { + const values = new Map(Object.entries(initial)); + const api = { + getSecret: vi.fn((id: string) => values.get(id) ?? null), + setSecret: vi.fn((id: string, value: string) => { + values.set(id, value); + }), + listSecrets: vi.fn(() => [...values.keys()]), + } as unknown as SecretStorage; + return { values, api, feedUrls: new FeedUrlRepository(api) }; +} + +function feed(overrides: Partial = {}): PodcastFeed { + return { title: "My Show", url: "https://feeds.example.com/rss", artworkUrl: "", ...overrides }; +} + +const PRIVATE_URL = "https://www.patreon.com/rss/show?auth=se-cret"; + +describe("internPrivateFeed", () => { + it("moves a credential-bearing URL into SecretStorage and persists a placeholder", () => { + const { feedUrls, values } = repository(); + const interned = internPrivateFeed(feed({ url: PRIVATE_URL }), feedUrls); + + expect(interned.urlSecretId).toBe("podnotes-feed-url"); + expect(values.get("podnotes-feed-url")).toBe(PRIVATE_URL); + expect(interned.url).toBe("podnotes-private-feed:My%20Show"); + expect(JSON.stringify(interned)).not.toContain("se-cret"); + }); + + it("passes public feeds and already-interned feeds through untouched", () => { + const { feedUrls } = repository(); + const publicFeed = feed(); + expect(internPrivateFeed(publicFeed, feedUrls)).toBe(publicFeed); + + const interned = feed({ + url: "podnotes-private-feed:My%20Show", + urlSecretId: "podnotes-feed-url", + }); + expect(internPrivateFeed(interned, feedUrls)).toBe(interned); + }); + + it("keeps the feed unchanged when SecretStorage fails, never half-migrated", () => { + const { api, feedUrls } = 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(); + }); +}); + +describe("resolveFeedUrl", () => { + it("resolves a private feed from SecretStorage", () => { + const { feedUrls } = repository({ "podnotes-feed-url": PRIVATE_URL }); + const privateFeed = feed({ + url: "podnotes-private-feed:My%20Show", + urlSecretId: "podnotes-feed-url", + }); + expect(resolveFeedUrl(privateFeed, feedUrls)).toBe(PRIVATE_URL); + }); + + it("returns null when the secret is absent on this device", () => { + const { feedUrls } = repository(); + const privateFeed = feed({ + url: "podnotes-private-feed:My%20Show", + urlSecretId: "podnotes-feed-url", + }); + expect(resolveFeedUrl(privateFeed, feedUrls)).toBeNull(); + }); + + it("never fetches a stranded placeholder without a reference", () => { + const { feedUrls } = repository(); + expect( + resolveFeedUrl(feed({ url: "podnotes-private-feed:My%20Show" }), feedUrls), + ).toBeNull(); + }); + + it("passes public URLs through", () => { + const { feedUrls } = repository(); + expect(resolveFeedUrl(feed(), feedUrls)).toBe("https://feeds.example.com/rss"); + }); +}); + +describe("migratePrivateFeedUrls", () => { + it("moves only credential-bearing feeds and reports the count", () => { + const { feedUrls, values } = repository(); + const { savedFeeds, migrated } = migratePrivateFeedUrls( + { + Public: feed({ title: "Public" }), + Patreon: feed({ title: "Patreon", url: PRIVATE_URL }), + Basic: feed({ title: "Basic", url: "https://user:pw@feeds.example.com/rss" }), + }, + feedUrls, + ); + + expect(migrated).toBe(2); + expect(savedFeeds.Public.url).toBe("https://feeds.example.com/rss"); + expect(savedFeeds.Patreon.url).toBe("podnotes-private-feed:Patreon"); + expect(savedFeeds.Basic.urlSecretId).toBe("podnotes-feed-url-2"); + expect(JSON.stringify(savedFeeds)).not.toMatch(/se-cret|user:pw/); + expect(values.get("podnotes-feed-url")).toBe(PRIVATE_URL); + }); + + it("is idempotent: a second run over migrated feeds changes nothing", () => { + const { feedUrls } = repository(); + const first = migratePrivateFeedUrls({ Patreon: feed({ url: PRIVATE_URL }) }, feedUrls); + const second = migratePrivateFeedUrls(first.savedFeeds, feedUrls); + expect(second.migrated).toBe(0); + expect(second.savedFeeds).toEqual(first.savedFeeds); + }); +}); diff --git a/src/services/privateFeeds.ts b/src/services/privateFeeds.ts new file mode 100644 index 00000000..931a99e6 --- /dev/null +++ b/src/services/privateFeeds.ts @@ -0,0 +1,82 @@ +import { Notice } from "obsidian"; +import type { PodcastFeed } from "src/types/PodcastFeed"; +import { + isCredentialBearingUrl, + isPrivateFeedPlaceholder, + privateFeedPlaceholder, +} from "src/utility/privateFeedUrl"; +import type { FeedUrlRepository } from "./FeedUrlRepository"; + +/** + * The single ingress/egress layer for private feed URLs. + * + * Ingress (subscribe, OPML import, load-time migration) interns a + * credential-bearing URL into SecretStorage and persists only a placeholder. + * Egress (`resolveFeedUrl`) turns a saved feed back into a fetchable URL + * immediately before retrieval and nowhere else - callers keep the resolved + * value confined to the fetch call and never persist or log it. + */ + +/** + * Return the feed as it may be persisted: a credential-bearing URL moves into + * SecretStorage and is replaced by a placeholder + reference. 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. + */ +export function internPrivateFeed(feed: PodcastFeed, feedUrls: FeedUrlRepository): PodcastFeed { + if (feed.urlSecretId || !isCredentialBearingUrl(feed.url)) return feed; + try { + const urlSecretId = feedUrls.store(feed.url); + return { ...feed, url: privateFeedPlaceholder(feed.title), urlSecretId }; + } catch (error) { + console.error(`PodNotes: could not protect the private feed "${feed.title}"`, error); + return feed; + } +} + +/** + * The URL to fetch this feed from. For private feeds this resolves the secret + * from SecretStorage; null means the secret is not available on this device + * (SecretStorage is device-local) and the caller must not fetch. + */ +export function resolveFeedUrl(feed: PodcastFeed, feedUrls: FeedUrlRepository): string | null { + if (feed.urlSecretId) { + return feedUrls.resolve(feed.urlSecretId); + } + return isPrivateFeedPlaceholder(feed.url) ? null : feed.url; +} + +/** resolveFeedUrl plus the standard user-facing failure Notice. */ +export function resolveFeedUrlWithNotice( + feed: PodcastFeed, + feedUrls: FeedUrlRepository, +): string | null { + const url = resolveFeedUrl(feed, feedUrls); + if (url === null) { + new Notice( + `The private feed URL for "${feed.title}" is not available on this device. ` + + "Remove the feed and add it again with its private URL.", + ); + } + return url; +} + +/** + * Move every credential-bearing saved-feed URL into SecretStorage. Returns the + * migrated savedFeeds and how many feeds moved; zero means nothing to persist. + * Best-effort per feed: one failing feed keeps its URL and the rest still move. + */ +export function migratePrivateFeedUrls( + savedFeeds: Record, + feedUrls: FeedUrlRepository, +): { savedFeeds: Record; migrated: number } { + let migrated = 0; + const result: Record = {}; + for (const [key, feed] of Object.entries(savedFeeds)) { + const interned = internPrivateFeed(feed, feedUrls); + if (interned !== feed) migrated += 1; + result[key] = interned; + } + return { savedFeeds: result, migrated }; +} diff --git a/src/types/PodcastFeed.ts b/src/types/PodcastFeed.ts index ab7155a8..81f0303b 100644 --- a/src/types/PodcastFeed.ts +++ b/src/types/PodcastFeed.ts @@ -1,6 +1,12 @@ export interface PodcastFeed { title: string; url: string; + /** + * Set for private (credentialed) feeds: the SecretStorage ID holding the + * real feed URL. When set, `url` holds a non-fetchable placeholder (see + * src/utility/privateFeedUrl.ts) so the secret never enters data.json. + */ + urlSecretId?: string; artworkUrl: string; collectionId?: string; /** Channel / . May contain HTML. */ diff --git a/src/ui/PodcastView/PodcastView.svelte b/src/ui/PodcastView/PodcastView.svelte index d0e3ae80..ffe9f74d 100644 --- a/src/ui/PodcastView/PodcastView.svelte +++ b/src/ui/PodcastView/PodcastView.svelte @@ -31,6 +31,7 @@ import spawnEpisodeContextMenu from "./spawnEpisodeContextMenu"; import { getCachedEpisodes, setCachedEpisodes } from "src/services/FeedCacheService"; import { get } from "svelte/store"; + import { resolveFeedUrlWithNotice } from "src/services/privateFeeds"; import { PLAYED_SETTINGS } from "src/constants"; import { getFinishedPlayedEpisodeRecords } from "src/utility/episodeStatus"; import { @@ -206,7 +207,9 @@ } try { - const episodes = await new FeedParser(feed).getEpisodes(feed.url); + const feedUrl = resolveFeedUrlWithNotice(feed, get(plugin).feedUrls); + if (feedUrl === null) return []; + const episodes = await new FeedParser(feed).getEpisodes(feedUrl); episodeCache.update((cache) => ({ ...cache, diff --git a/src/ui/settings/PodcastQueryGrid.svelte b/src/ui/settings/PodcastQueryGrid.svelte index d882e4a6..d3b777e5 100644 --- a/src/ui/settings/PodcastQueryGrid.svelte +++ b/src/ui/settings/PodcastQueryGrid.svelte @@ -2,7 +2,9 @@ import { debounce, Notice } from "obsidian"; import { queryiTunesPodcasts } from "src/iTunesAPIConsumer"; import FeedParser from "src/parser/feedParser"; - import { savedFeeds, podcastsUpdated } from "src/store"; + import { plugin, savedFeeds, podcastsUpdated } from "src/store"; + import { get } from "svelte/store"; + import { internPrivateFeed } from "src/services/privateFeeds"; import type { PodcastFeed } from "src/types/PodcastFeed"; import checkStringIsUrl from "src/utility/checkStringIsUrl"; import Text from "../obsidian/Text.svelte"; @@ -94,7 +96,9 @@ ); function addPodcast(event: CustomEvent<{ podcast: PodcastFeed }>) { - const { podcast } = event.detail; + // A pasted private feed URL must never reach persisted settings: intern it + // into SecretStorage and save the placeholder + reference instead. + const podcast = internPrivateFeed(event.detail.podcast, get(plugin).feedUrls); savedFeeds.update((feeds) => ({ ...feeds, [podcast.title]: podcast })); updateSearchResults(); } @@ -102,6 +106,8 @@ function removePodcast(event: CustomEvent<{ podcast: PodcastFeed }>) { const { podcast } = event.detail; savedFeeds.update((feeds) => { + const removed = feeds[podcast.title]; + if (removed?.urlSecretId) get(plugin).feedUrls.delete(removed.urlSecretId); const newFeeds = { ...feeds }; delete newFeeds[podcast.title]; return newFeeds; diff --git a/src/utility/privateFeedUrl.test.ts b/src/utility/privateFeedUrl.test.ts new file mode 100644 index 00000000..5628bcb3 --- /dev/null +++ b/src/utility/privateFeedUrl.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vitest"; +import { + isCredentialBearingUrl, + isPrivateFeedPlaceholder, + parsePrivateFeedPlaceholder, + privateFeedPlaceholder, +} from "./privateFeedUrl"; + +describe("isCredentialBearingUrl", () => { + it("detects userinfo credentials", () => { + expect(isCredentialBearingUrl("https://user:pass@feeds.example.com/rss")).toBe(true); + expect(isCredentialBearingUrl("https://user@feeds.example.com/rss")).toBe(true); + }); + + it("detects auth-bearing query parameters regardless of case", () => { + expect(isCredentialBearingUrl("https://www.patreon.com/rss/show?auth=se-cret")).toBe(true); + expect(isCredentialBearingUrl("https://example.com/feed?TOKEN=x")).toBe(true); + expect(isCredentialBearingUrl("https://example.com/feed?a=1&api_key=x")).toBe(true); + expect(isCredentialBearingUrl("https://example.com/feed?access-token=x")).toBe(true); + }); + + it("leaves ordinary public feeds alone", () => { + expect(isCredentialBearingUrl("https://feed.syntax.fm/rss")).toBe(false); + expect(isCredentialBearingUrl("https://example.com/feed?page=2&format=rss")).toBe(false); + // Substring matches must not trigger: "keyword" is not "key". + expect(isCredentialBearingUrl("https://example.com/feed?keyword=rust")).toBe(false); + expect(isCredentialBearingUrl("https://example.com/feed?authors=jane")).toBe(false); + }); + + it("rejects non-http and malformed inputs", () => { + expect(isCredentialBearingUrl("podnotes-private-feed:My%20Show")).toBe(false); + expect(isCredentialBearingUrl("not a url")).toBe(false); + expect(isCredentialBearingUrl("")).toBe(false); + }); +}); + +describe("private feed placeholder", () => { + it("round-trips a feed name, including URL-hostile characters", () => { + for (const name of ["My Show", "Show/With?Chars#&+", "ÆØÅ 播客"]) { + const placeholder = privateFeedPlaceholder(name); + expect(isPrivateFeedPlaceholder(placeholder)).toBe(true); + expect(parsePrivateFeedPlaceholder(placeholder)).toBe(name); + // The placeholder must never be a fetchable http(s) URL. + expect(placeholder.startsWith("http")).toBe(false); + } + }); + + it("returns null for non-placeholder values", () => { + expect(parsePrivateFeedPlaceholder("https://feeds.example.com/rss")).toBeNull(); + expect(parsePrivateFeedPlaceholder("")).toBeNull(); + }); + + it("returns null for a malformed percent-encoding instead of throwing", () => { + expect(parsePrivateFeedPlaceholder("podnotes-private-feed:%E0%A4%A")).toBeNull(); + }); +}); diff --git a/src/utility/privateFeedUrl.ts b/src/utility/privateFeedUrl.ts new file mode 100644 index 00000000..366e4b50 --- /dev/null +++ b/src/utility/privateFeedUrl.ts @@ -0,0 +1,61 @@ +/** + * Private (credentialed) podcast feeds embed a durable secret in their URL - + * userinfo (`user:pass@`) or an auth-bearing query parameter (Patreon's + * `?auth=...` and similar). Persisting such a URL in data.json leaks the + * secret through vault sync, git-versioned vaults, and shared vaults. + * + * Private feeds therefore persist a placeholder in `PodcastFeed.url` and keep + * the real URL in Obsidian's vault-local SecretStorage, referenced by + * `PodcastFeed.urlSecretId`. The placeholder deliberately uses a non-http + * scheme: if it ever reaches the network gate unresolved, the gate refuses it. + */ + +const PLACEHOLDER_SCHEME = "podnotes-private-feed:"; + +/** + * Query parameter names that carry a durable subscriber secret. Matched on the + * whole name, case-insensitively. Path-embedded tokens (Supercast-style) are + * indistinguishable from public URLs and are NOT detected; those feeds keep + * working, their URL just stays in data.json like before. + */ +const SECRET_QUERY_PARAM = + /^(auth|token|key|secret|pass|password|apikey|api[-_]key|access[-_]token|sig|signature)$/i; + +export function isCredentialBearingUrl(rawUrl: string): boolean { + let url: URL; + try { + url = new URL(rawUrl); + } catch { + return false; + } + if (url.protocol !== "http:" && url.protocol !== "https:") return false; + if (url.username.length > 0 || url.password.length > 0) return true; + for (const name of url.searchParams.keys()) { + if (SECRET_QUERY_PARAM.test(name)) return true; + } + return false; +} + +/** + * The persisted stand-in for a private feed's URL. Carries the feed's saved + * name (its savedFeeds key) so links and the URI handler can find the feed + * locally and resolve the real URL from SecretStorage - the name is identity, + * not a secret. + */ +export function privateFeedPlaceholder(podcastName: string): string { + return `${PLACEHOLDER_SCHEME}${encodeURIComponent(podcastName)}`; +} + +export function isPrivateFeedPlaceholder(value: string): boolean { + return value.startsWith(PLACEHOLDER_SCHEME); +} + +/** The savedFeeds key encoded in a placeholder, or null when not a placeholder. */ +export function parsePrivateFeedPlaceholder(value: string): string | null { + if (!isPrivateFeedPlaceholder(value)) return null; + try { + return decodeURIComponent(value.slice(PLACEHOLDER_SCHEME.length)); + } catch { + return null; + } +} From 64a3235b5dacf77201ad54f6e1536edf9b415c86 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Sat, 11 Jul 2026 14:50:43 +0200 Subject: [PATCH 2/2] fix(security): close review findings on private feed URL handling Adversarial review (Codex on-PR + local skeptic) found real gaps; all confirmed findings addressed: - Episode snapshots: migration now scrubs url/feedUrl in played history, queue, favorites, playlists, local files, downloads, and currentEpisode (episodes stamp feed URLs at fetch time and persist them), and evicts the migrated URL's entry from the local feed cache. - Cross-device references: secret IDs are now UUID-based. urlSecretId syncs through data.json while SecretStorage is device-local; sequential IDs could make device A's reference resolve to device B's unrelated secret and silently fetch the wrong private feed. - Settings transfer: urlSecretId is stripped on export AND import (references are device-local capabilities, not transfer data), and a raw data.json import interns credential-bearing URLs BEFORE the strict persist instead of waiting for the next reload. - Both-fields records: a feed carrying a stale reference plus a raw credentialed URL is re-interned instead of bypassing protection. - Unsubscribe keeps a secret still referenced by another feed; the orphan sweep trims persisted references like resolve() does and can no longer abort plugin loading. - URI links: a vault file named like a placeholder can't shadow private links; migration failures now name the affected feeds in a notice; OPML exports warn when they contain plaintext private URLs; detector covers jwt/credential parameters. --- src/URIHandler.ts | 11 +-- src/main.ts | 29 +++++-- src/opml.ts | 14 ++- src/services/FeedCacheService.ts | 19 +++++ src/services/FeedUrlRepository.test.ts | 76 +++++++++++------ src/services/FeedUrlRepository.ts | 63 ++++++-------- src/services/privateFeeds.test.ts | 109 ++++++++++++++++++++---- src/services/privateFeeds.ts | 92 ++++++++++++++++---- src/settingsTransfer.test.ts | 48 +++++++++++ src/settingsTransfer.ts | 29 +++++++ src/ui/settings/PodNotesSettingsTab.ts | 7 ++ src/ui/settings/PodcastQueryGrid.svelte | 9 +- src/utility/privateFeedUrl.ts | 2 +- 13 files changed, 395 insertions(+), 113 deletions(-) diff --git a/src/URIHandler.ts b/src/URIHandler.ts index f25afce3..fbb5f569 100644 --- a/src/URIHandler.ts +++ b/src/URIHandler.ts @@ -167,14 +167,15 @@ export default async function podNotesURIHandler( let episode: Episode | undefined; - if (localFile) { + // A private-feed placeholder always routes to placeholder resolution (the + // final branch): a vault file literally named like a placeholder must not + // shadow private links. + const isPlaceholderLink = isPrivateFeedPlaceholder(url); + if (!isPlaceholderLink && localFile) { episode = nameCandidates .map((name) => localFiles.getLocalEpisode(name)) .find((ep) => ep !== undefined); - } else if ( - !isPrivateFeedPlaceholder(url) && - !candidateValues(url).some((u) => /^https?:\/\//i.test(u)) - ) { + } else if (!isPlaceholderLink && !candidateValues(url).some((u) => /^https?:\/\//i.test(u))) { // The probe found no file, yet `url` has no http(s) scheme, so it can only be // a vault path whose file was moved/renamed. Resolve the episode by name from // the local-files store rather than handing the bare path to FeedParser (which diff --git a/src/main.ts b/src/main.ts index b7f8b579..4277ebbe 100644 --- a/src/main.ts +++ b/src/main.ts @@ -42,7 +42,8 @@ import { } from "./persistence/podNotesData"; import { CredentialRepository } from "./services/CredentialRepository"; import { FeedUrlRepository } from "./services/FeedUrlRepository"; -import { migratePrivateFeedUrls } from "./services/privateFeeds"; +import { migratePrivateFeedUrls, scrubMigratedEpisodeUrls } from "./services/privateFeeds"; +import { evictCachedFeedUrls } from "./services/FeedCacheService"; type MediaSessionActionName = | "previoustrack" @@ -479,16 +480,30 @@ export default class PodNotes extends Plugin implements IPodNotes { } // Private feed URLs: move credential-bearing subscription URLs out of - // data.json into SecretStorage, then reap feed-url secrets no saved feed - // references anymore (e.g. after a settings import replaced savedFeeds). - // Best-effort: a failure keeps the URL where it was, never breaks loading. + // data.json into SecretStorage, scrub the same URLs out of persisted + // episode snapshots (queue/favorites/playlists/downloads/history stamp + // url/feedUrl at fetch time) and the local feed cache, 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 = migratePrivateFeedUrls(settings.savedFeeds, this.feedUrls); - if (feedMigration.migrated > 0) { - settings = { ...settings, savedFeeds: feedMigration.savedFeeds }; + if (feedMigration.replacements.size > 0) { + settings = scrubMigratedEpisodeUrls( + { ...settings, savedFeeds: feedMigration.savedFeeds }, + feedMigration.replacements, + ); + evictCachedFeedUrls([...feedMigration.replacements.keys()]); await this.saveData(encodePodNotesData(settings, decoded.unknownFields)); + const count = feedMigration.replacements.size; new Notice( - `Moved ${feedMigration.migrated} private feed ${feedMigration.migrated === 1 ? "URL" : "URLs"} into Obsidian SecretStorage.`, + `Moved ${count} private feed ${count === 1 ? "URL" : "URLs"} into Obsidian SecretStorage.`, + ); + } + 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); diff --git a/src/opml.ts b/src/opml.ts index 7789bf2d..557a74e6 100644 --- a/src/opml.ts +++ b/src/opml.ts @@ -222,8 +222,12 @@ async function exportOPML(app: App, feeds: PodcastFeed[], filePath = "PodNotes_E // private feed exports its REAL url (an export with placeholders is useless). // A private URL that cannot be resolved on this device exports as an empty // xmlUrl rather than a placeholder. - const feedOutline = (feed: PodcastFeed) => - ``; + let exportedPrivateFeeds = 0; + const feedOutline = (feed: PodcastFeed) => { + const url = resolveFeedUrl(feed, get(plugin).feedUrls) ?? ""; + if (feed.urlSecretId && url) exportedPrivateFeeds += 1; + return ``; + }; const feedsOutline = (_feeds: PodcastFeed[]) => `${feeds.map(feedOutline).join("")}`; @@ -233,6 +237,12 @@ async function exportOPML(app: App, feeds: PodcastFeed[], filePath = "PodNotes_E await app.vault.create(filePath, doc); new Notice(`Exported ${feeds.length} podcast feeds to file "${filePath}".`); + if (exportedPrivateFeeds > 0) { + new Notice( + `The export contains ${exportedPrivateFeeds} private feed ${exportedPrivateFeeds === 1 ? "URL" : "URLs"} in plaintext. The file is inside your vault - delete it after importing it elsewhere.`, + 10000, + ); + } } catch (error) { if (error instanceof Error) { if (error.message.includes("Folder does not exist")) { diff --git a/src/services/FeedCacheService.ts b/src/services/FeedCacheService.ts index 9c8fb63e..7da236ed 100644 --- a/src/services/FeedCacheService.ts +++ b/src/services/FeedCacheService.ts @@ -324,6 +324,25 @@ export function setCachedEpisodes(feed: PodcastFeed, episodes: Episode[]): void persistCache(); } +/** + * Drop cache entries keyed by the given feed URLs. Used when a private feed's + * credential-bearing URL migrates to a placeholder: the old entry would never + * be read again (lookups now use the placeholder key), so its plaintext URL + * would otherwise sit in localStorage until eviction. + */ +export function evictCachedFeedUrls(urls: string[]): void { + if (!urls.length) return; + const store = loadCache(); + let changed = false; + for (const url of urls) { + if (url in store) { + delete store[url]; + changed = true; + } + } + if (changed) persistCache(); +} + export function clearFeedCache(): void { cache = {}; const storage = getStorage(); diff --git a/src/services/FeedUrlRepository.test.ts b/src/services/FeedUrlRepository.test.ts index 6b7735fa..6be56892 100644 --- a/src/services/FeedUrlRepository.test.ts +++ b/src/services/FeedUrlRepository.test.ts @@ -22,25 +22,21 @@ function feed(overrides: Partial = {}): PodcastFeed { } describe("FeedUrlRepository", () => { - it("stores a URL under the PodNotes feed-url ID and verifies the readback", () => { + it("stores a URL under a fresh device-unique ID and verifies the readback", () => { const { api, values } = storage(); const id = new FeedUrlRepository(api).store(" https://p.example/rss?auth=tok "); - expect(id).toBe("podnotes-feed-url"); + expect(isFeedUrlSecretId(id)).toBe(true); expect(values.get(id)).toBe("https://p.example/rss?auth=tok"); }); - it("reuses an identical value and suffixes past a conflicting one", () => { - const { api } = storage({ "podnotes-feed-url": "https://a.example/rss?auth=1" }); + it("allocates distinct IDs even for identical URLs (references sync, secrets don't)", () => { + const { api } = storage(); const repository = new FeedUrlRepository(api); - expect(repository.store("https://a.example/rss?auth=1")).toBe("podnotes-feed-url"); - expect(repository.store("https://b.example/rss?auth=2")).toBe("podnotes-feed-url-2"); - }); - - it("reuses a cleared slot after deletion", () => { - const { api } = storage({ "podnotes-feed-url": "" }); - expect(new FeedUrlRepository(api).store("https://c.example/rss?auth=3")).toBe( - "podnotes-feed-url", - ); + const first = repository.store("https://a.example/rss?auth=1"); + const second = repository.store("https://a.example/rss?auth=1"); + expect(first).not.toBe(second); + expect(isFeedUrlSecretId(first)).toBe(true); + expect(isFeedUrlSecretId(second)).toBe(true); }); it("throws when SecretStorage does not retain the value", () => { @@ -53,12 +49,16 @@ describe("FeedUrlRepository", () => { it("resolves only PodNotes-owned feed-url IDs and fails closed", () => { const { api } = storage({ - "podnotes-feed-url": "https://p.example/rss?auth=tok", + "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef": + "https://p.example/rss?auth=tok", "podnotes-openai-api-key": "sk-not-a-feed", }); const repository = new FeedUrlRepository(api); - expect(repository.resolve("podnotes-feed-url")).toBe("https://p.example/rss?auth=tok"); + expect(repository.resolve("podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef")).toBe( + "https://p.example/rss?auth=tok", + ); expect(repository.resolve("podnotes-openai-api-key")).toBeNull(); + expect(repository.resolve("podnotes-feed-url")).toBeNull(); expect(repository.resolve("podnotes-feed-url-9")).toBeNull(); expect(repository.resolve("")).toBeNull(); }); @@ -68,42 +68,62 @@ describe("FeedUrlRepository", () => { (api.getSecret as ReturnType).mockImplementation(() => { throw new Error("locked"); }); - expect(new FeedUrlRepository(api).resolve("podnotes-feed-url")).toBeNull(); + expect( + new FeedUrlRepository(api).resolve( + "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef", + ), + ).toBeNull(); }); it("deletes by clearing and refuses foreign IDs", () => { const { api, values } = storage({ - "podnotes-feed-url": "https://p.example/rss?auth=tok", + "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef": + "https://p.example/rss?auth=tok", "podnotes-openai-api-key": "sk-keep", }); const repository = new FeedUrlRepository(api); - repository.delete("podnotes-feed-url"); - expect(values.get("podnotes-feed-url")).toBe(""); + repository.delete("podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef"); + expect(values.get("podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef")).toBe(""); repository.delete("podnotes-openai-api-key"); expect(values.get("podnotes-openai-api-key")).toBe("sk-keep"); }); it("sweeps unreferenced feed-url secrets and nothing else", () => { const { api, values } = storage({ - "podnotes-feed-url": "https://kept.example/rss?auth=1", - "podnotes-feed-url-2": "https://orphan.example/rss?auth=2", + "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef": + "https://kept.example/rss?auth=1", + "podnotes-feed-url-22345678-9abc-4def-8123-456789abcdef": + "https://orphan.example/rss?auth=2", "podnotes-openai-api-key": "sk-keep", }); new FeedUrlRepository(api).sweepOrphans({ - Kept: feed({ urlSecretId: "podnotes-feed-url" }), + // An untrimmed persisted reference must still protect its secret. + Kept: feed({ urlSecretId: " podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef " }), Public: feed(), }); - expect(values.get("podnotes-feed-url")).toBe("https://kept.example/rss?auth=1"); - expect(values.get("podnotes-feed-url-2")).toBe(""); + expect(values.get("podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef")).toBe( + "https://kept.example/rss?auth=1", + ); + expect(values.get("podnotes-feed-url-22345678-9abc-4def-8123-456789abcdef")).toBe(""); expect(values.get("podnotes-openai-api-key")).toBe("sk-keep"); }); + + it("survives a listSecrets failure without throwing", () => { + const { api } = storage(); + (api.listSecrets as ReturnType).mockImplementation(() => { + throw new Error("locked"); + }); + expect(() => new FeedUrlRepository(api).sweepOrphans({})).not.toThrow(); + }); }); describe("isFeedUrlSecretId", () => { - it("accepts only the PodNotes feed-url grammar", () => { - expect(isFeedUrlSecretId("podnotes-feed-url")).toBe(true); - expect(isFeedUrlSecretId("podnotes-feed-url-2")).toBe(true); - expect(isFeedUrlSecretId("podnotes-feed-url-0")).toBe(false); + it("accepts only the PodNotes feed-url UUID grammar", () => { + expect(isFeedUrlSecretId("podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef")).toBe( + true, + ); + expect(isFeedUrlSecretId("podnotes-feed-url")).toBe(false); + expect(isFeedUrlSecretId("podnotes-feed-url-2")).toBe(false); expect(isFeedUrlSecretId("podnotes-openai-api-key")).toBe(false); expect(isFeedUrlSecretId("feed-url")).toBe(false); }); diff --git a/src/services/FeedUrlRepository.ts b/src/services/FeedUrlRepository.ts index f3fc40dc..06f62c3f 100644 --- a/src/services/FeedUrlRepository.ts +++ b/src/services/FeedUrlRepository.ts @@ -7,13 +7,17 @@ import { isValidSecretId } from "src/types/Credentials"; * contract: runtime reads fail closed and never cache secret values; writes * return an ID only after the exact value reads back. * - * SecretStorage is device-local, so a second synced device sees the feed's - * placeholder without its secret - the feed is re-added there. IDs are - * content-free (`podnotes-feed-url`, `-2`, `-3`, ...) so neither the feed name - * nor the URL leaks through the ID namespace, which `listSecrets` exposes. + * 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 - + * neither the feed name nor the URL leaks through the ID namespace, which + * `listSecrets` exposes. */ -const BASE_ID = "podnotes-feed-url"; -const FEED_URL_SECRET_ID = /^podnotes-feed-url(?:-[1-9]\d*)?$/; +const FEED_URL_SECRET_ID = + /^podnotes-feed-url-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/; export function isFeedUrlSecretId(value: string): boolean { return FEED_URL_SECRET_ID.test(value); @@ -35,30 +39,17 @@ export class FeedUrlRepository { } } - /** - * Store a private feed URL under a PodNotes-owned ID. An existing identical - * value is reused (idempotent retries); a conflicting value is never - * overwritten - the first free numeric suffix is used instead. - */ + /** 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."); - for (let suffix = 1; suffix <= 10_000; suffix++) { - const id = suffix === 1 ? BASE_ID : `${BASE_ID}-${suffix}`; - const existing = this.storage.getSecret(id); - - if (existing === value) return id; - if (existing !== null && existing !== "") continue; - - this.storage.setSecret(id, value); - if (this.storage.getSecret(id) !== value) { - throw new Error("SecretStorage did not retain the private feed URL."); - } - return id; + const id = `podnotes-feed-url-${crypto.randomUUID()}`; + this.storage.setSecret(id, value); + if (this.storage.getSecret(id) !== value) { + throw new Error("SecretStorage did not retain the private feed URL."); } - - throw new Error("Could not allocate a SecretStorage ID for the private feed URL."); + return id; } /** Clearing to "" is SecretStorage's deletion idiom; reads treat "" as absent. */ @@ -75,24 +66,24 @@ 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). + * replacing savedFeeds wholesale). Never throws: a sweep failure must not + * abort plugin loading. */ sweepOrphans(feeds: Record): void { const referenced = new Set(); for (const feed of Object.values(feeds)) { - if (feed.urlSecretId) referenced.add(feed.urlSecretId); + // 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()); } - let ids: string[]; try { - ids = this.storage.listSecrets(); + for (const id of this.storage.listSecrets()) { + if (!isFeedUrlSecretId(id) || referenced.has(id)) continue; + if (!this.storage.getSecret(id)) continue; + this.delete(id); + } } catch (error) { - console.error("PodNotes: failed to list secrets for the orphan sweep", error); - return; - } - for (const id of ids) { - if (!isFeedUrlSecretId(id) || referenced.has(id)) continue; - if (!this.storage.getSecret(id)) continue; - this.delete(id); + console.error("PodNotes: the private feed URL orphan sweep failed", error); } } } diff --git a/src/services/privateFeeds.test.ts b/src/services/privateFeeds.test.ts index c6da93e1..8538f9f1 100644 --- a/src/services/privateFeeds.test.ts +++ b/src/services/privateFeeds.test.ts @@ -2,7 +2,12 @@ import { describe, expect, it, vi } from "vitest"; import type { SecretStorage } from "obsidian"; import type { PodcastFeed } from "src/types/PodcastFeed"; import { FeedUrlRepository } from "./FeedUrlRepository"; -import { internPrivateFeed, migratePrivateFeedUrls, resolveFeedUrl } from "./privateFeeds"; +import { + internPrivateFeed, + migratePrivateFeedUrls, + resolveFeedUrl, + scrubMigratedEpisodeUrls, +} from "./privateFeeds"; function repository(initial: Record = {}) { const values = new Map(Object.entries(initial)); @@ -27,8 +32,8 @@ describe("internPrivateFeed", () => { const { feedUrls, values } = repository(); const interned = internPrivateFeed(feed({ url: PRIVATE_URL }), feedUrls); - expect(interned.urlSecretId).toBe("podnotes-feed-url"); - expect(values.get("podnotes-feed-url")).toBe(PRIVATE_URL); + expect(interned.urlSecretId).toMatch(/^podnotes-feed-url-/); + expect(values.get(interned.urlSecretId as string)).toBe(PRIVATE_URL); expect(interned.url).toBe("podnotes-private-feed:My%20Show"); expect(JSON.stringify(interned)).not.toContain("se-cret"); }); @@ -40,11 +45,29 @@ describe("internPrivateFeed", () => { const interned = feed({ url: "podnotes-private-feed:My%20Show", - urlSecretId: "podnotes-feed-url", + urlSecretId: "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef", }); expect(internPrivateFeed(interned, feedUrls)).toBe(interned); }); + it("re-interns a record carrying BOTH a stale reference and a raw credentialed URL", () => { + const { feedUrls, values } = repository(); + const smuggled = feed({ + url: PRIVATE_URL, + urlSecretId: "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef", + }); + const interned = internPrivateFeed(smuggled, feedUrls); + expect(interned.url).toBe("podnotes-private-feed:My%20Show"); + expect(interned.urlSecretId).not.toBe(smuggled.urlSecretId); + expect(values.get(interned.urlSecretId as string)).toBe(PRIVATE_URL); + }); + + it("builds the placeholder from the savedFeeds key when it differs from the title", () => { + const { feedUrls } = repository(); + const interned = internPrivateFeed(feed({ url: PRIVATE_URL }), feedUrls, "Renamed Key"); + expect(interned.url).toBe("podnotes-private-feed:Renamed%20Key"); + }); + it("keeps the feed unchanged when SecretStorage fails, never half-migrated", () => { const { api, feedUrls } = repository(); (api.setSecret as ReturnType).mockImplementation(() => { @@ -59,11 +82,9 @@ describe("internPrivateFeed", () => { describe("resolveFeedUrl", () => { it("resolves a private feed from SecretStorage", () => { - const { feedUrls } = repository({ "podnotes-feed-url": PRIVATE_URL }); - const privateFeed = feed({ - url: "podnotes-private-feed:My%20Show", - urlSecretId: "podnotes-feed-url", - }); + const id = "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef"; + const { feedUrls } = repository({ [id]: PRIVATE_URL }); + const privateFeed = feed({ url: "podnotes-private-feed:My%20Show", urlSecretId: id }); expect(resolveFeedUrl(privateFeed, feedUrls)).toBe(PRIVATE_URL); }); @@ -71,7 +92,7 @@ describe("resolveFeedUrl", () => { const { feedUrls } = repository(); const privateFeed = feed({ url: "podnotes-private-feed:My%20Show", - urlSecretId: "podnotes-feed-url", + urlSecretId: "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef", }); expect(resolveFeedUrl(privateFeed, feedUrls)).toBeNull(); }); @@ -92,7 +113,7 @@ describe("resolveFeedUrl", () => { describe("migratePrivateFeedUrls", () => { it("moves only credential-bearing feeds and reports the count", () => { const { feedUrls, values } = repository(); - const { savedFeeds, migrated } = migratePrivateFeedUrls( + const { savedFeeds, replacements, failed } = migratePrivateFeedUrls( { Public: feed({ title: "Public" }), Patreon: feed({ title: "Patreon", url: PRIVATE_URL }), @@ -101,19 +122,77 @@ describe("migratePrivateFeedUrls", () => { feedUrls, ); - expect(migrated).toBe(2); + expect(replacements.size).toBe(2); + expect(failed).toEqual([]); expect(savedFeeds.Public.url).toBe("https://feeds.example.com/rss"); expect(savedFeeds.Patreon.url).toBe("podnotes-private-feed:Patreon"); - expect(savedFeeds.Basic.urlSecretId).toBe("podnotes-feed-url-2"); + expect(savedFeeds.Basic.urlSecretId).toMatch(/^podnotes-feed-url-/); expect(JSON.stringify(savedFeeds)).not.toMatch(/se-cret|user:pw/); - expect(values.get("podnotes-feed-url")).toBe(PRIVATE_URL); + expect(values.get(savedFeeds.Patreon.urlSecretId as string)).toBe(PRIVATE_URL); + expect(replacements.get(PRIVATE_URL)).toBe("podnotes-private-feed:Patreon"); }); it("is idempotent: a second run over migrated feeds changes nothing", () => { const { feedUrls } = repository(); const first = migratePrivateFeedUrls({ Patreon: feed({ url: PRIVATE_URL }) }, feedUrls); const second = migratePrivateFeedUrls(first.savedFeeds, feedUrls); - expect(second.migrated).toBe(0); + expect(second.replacements.size).toBe(0); expect(second.savedFeeds).toEqual(first.savedFeeds); }); + + it("reports feeds whose URL could not be protected", () => { + const { api, feedUrls } = repository(); + (api.setSecret as ReturnType).mockImplementation(() => { + throw new Error("locked"); + }); + const { failed, replacements } = migratePrivateFeedUrls( + { Patreon: feed({ title: "Patreon", url: PRIVATE_URL }) }, + feedUrls, + ); + expect(failed).toEqual(["Patreon"]); + expect(replacements.size).toBe(0); + }); +}); + +describe("scrubMigratedEpisodeUrls", () => { + const replacements = new Map([[PRIVATE_URL, "podnotes-private-feed:Patreon"]]); + + it("rewrites url/feedUrl in nested episode snapshots and nothing else", () => { + const settings = { + currentEpisode: { title: "Ep 1", url: PRIVATE_URL, feedUrl: PRIVATE_URL }, + queue: { + episodes: [ + { + title: "Ep 2", + feedUrl: PRIVATE_URL, + streamUrl: "https://cdn.example/e2.mp3", + }, + ], + }, + playedEpisodes: { "Ep 3": { feedUrl: "https://public.example/rss", time: 10 } }, + }; + const scrubbed = scrubMigratedEpisodeUrls(settings, replacements); + expect(scrubbed.currentEpisode.url).toBe("podnotes-private-feed:Patreon"); + expect(scrubbed.currentEpisode.feedUrl).toBe("podnotes-private-feed:Patreon"); + expect(scrubbed.queue.episodes[0].feedUrl).toBe("podnotes-private-feed:Patreon"); + expect(scrubbed.queue.episodes[0].streamUrl).toBe("https://cdn.example/e2.mp3"); + expect(scrubbed.playedEpisodes["Ep 3"].feedUrl).toBe("https://public.example/rss"); + expect(JSON.stringify(scrubbed)).not.toContain("se-cret"); + }); + + it("preserves Date instances and untouched subtrees by reference", () => { + const date = new Date("2026-01-01T00:00:00Z"); + const settings = { + currentEpisode: { title: "Ep", feedUrl: PRIVATE_URL, episodeDate: date }, + untouched: { nested: { value: 1 } }, + }; + const scrubbed = scrubMigratedEpisodeUrls(settings, replacements); + expect(scrubbed.currentEpisode.episodeDate).toBe(date); + expect(scrubbed.untouched).toBe(settings.untouched); + }); + + it("is a no-op without replacements", () => { + const settings = { currentEpisode: { feedUrl: PRIVATE_URL } }; + expect(scrubMigratedEpisodeUrls(settings, new Map())).toBe(settings); + }); }); diff --git a/src/services/privateFeeds.ts b/src/services/privateFeeds.ts index 931a99e6..5c1c1e61 100644 --- a/src/services/privateFeeds.ts +++ b/src/services/privateFeeds.ts @@ -10,25 +10,36 @@ import type { FeedUrlRepository } from "./FeedUrlRepository"; /** * The single ingress/egress layer for private feed URLs. * - * Ingress (subscribe, OPML import, load-time migration) interns a - * credential-bearing URL into SecretStorage and persists only a placeholder. - * Egress (`resolveFeedUrl`) turns a saved feed back into a fetchable URL - * immediately before retrieval and nowhere else - callers keep the resolved - * value confined to the fetch call and never persist or log it. + * Ingress (subscribe, OPML import, settings import, load-time migration) + * interns a credential-bearing URL into SecretStorage and persists only a + * placeholder. Egress (`resolveFeedUrl`) turns a saved feed back into a + * fetchable URL immediately before retrieval and nowhere else - callers keep + * the resolved value confined to the fetch call and never persist or log it. */ /** * Return the feed as it may be persisted: a credential-bearing URL moves into - * SecretStorage and is replaced by a placeholder + reference. Non-credentialed + * SecretStorage and is replaced by a placeholder + reference. Applies even + * 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. + * than breaking the subscription; migration surfaces that failure to the user. + * + * `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 + * feeds are created, but persisted data may disagree). */ -export function internPrivateFeed(feed: PodcastFeed, feedUrls: FeedUrlRepository): PodcastFeed { - if (feed.urlSecretId || !isCredentialBearingUrl(feed.url)) return feed; +export function internPrivateFeed( + feed: PodcastFeed, + feedUrls: FeedUrlRepository, + savedKey: string = feed.title, +): PodcastFeed { + if (!isCredentialBearingUrl(feed.url)) return feed; try { const urlSecretId = feedUrls.store(feed.url); - return { ...feed, url: privateFeedPlaceholder(feed.title), urlSecretId }; + return { ...feed, url: privateFeedPlaceholder(savedKey), urlSecretId }; } catch (error) { console.error(`PodNotes: could not protect the private feed "${feed.title}"`, error); return feed; @@ -62,21 +73,66 @@ export function resolveFeedUrlWithNotice( return url; } +export interface PrivateFeedMigration { + savedFeeds: Record; + /** Old credential-bearing URL -> the placeholder that replaced it. */ + replacements: Map; + /** Feeds whose URL could NOT be protected (SecretStorage failure). */ + failed: string[]; +} + /** - * Move every credential-bearing saved-feed URL into SecretStorage. Returns the - * migrated savedFeeds and how many feeds moved; zero means nothing to persist. - * Best-effort per feed: one failing feed keeps its URL and the rest still move. + * Move every credential-bearing saved-feed URL into SecretStorage. + * Best-effort per feed: one failing feed keeps its URL, is reported in + * `failed`, and the rest still move. */ export function migratePrivateFeedUrls( savedFeeds: Record, feedUrls: FeedUrlRepository, -): { savedFeeds: Record; migrated: number } { - let migrated = 0; +): PrivateFeedMigration { + const replacements = new Map(); + const failed: string[] = []; const result: Record = {}; for (const [key, feed] of Object.entries(savedFeeds)) { - const interned = internPrivateFeed(feed, feedUrls); - if (interned !== feed) migrated += 1; + const interned = internPrivateFeed(feed, feedUrls, key); + if (interned !== feed) { + replacements.set(feed.url, interned.url); + } else if (isCredentialBearingUrl(feed.url)) { + failed.push(key); + } result[key] = interned; } - return { savedFeeds: result, migrated }; + return { savedFeeds: result, replacements, failed }; +} + +/** + * Rewrite persisted Episode snapshots that still carry a migrated feed's old + * credential-bearing URL (played history, queue, favorites, playlists, local + * files, downloads, and the current episode all persist full episode objects + * whose `url`/`feedUrl` were stamped at fetch time). + */ +export function scrubMigratedEpisodeUrls(value: T, replacements: Map): T { + if (replacements.size === 0) return value; + if (Array.isArray(value)) { + return value.map((entry) => scrubMigratedEpisodeUrls(entry, replacements)) as T; + } + if (typeof value !== "object" || value === null) return value; + // Decoded settings carry Date instances (episodeDate); recursing into one + // would flatten it into {}. Non-plain objects hold no episode URLs. + if (Object.getPrototypeOf(value) !== Object.prototype) return value; + + const record = value as Record; + let changed = false; + const next: Record = {}; + for (const [key, entry] of Object.entries(record)) { + let scrubbed: unknown; + if ((key === "url" || key === "feedUrl") && typeof entry === "string") { + scrubbed = replacements.get(entry) ?? entry; + } else { + scrubbed = scrubMigratedEpisodeUrls(entry, replacements); + } + if (scrubbed !== entry) changed = true; + next[key] = scrubbed; + } + return (changed ? next : value) as T; } diff --git a/src/settingsTransfer.test.ts b/src/settingsTransfer.test.ts index 7458fe34..7a985850 100644 --- a/src/settingsTransfer.test.ts +++ b/src/settingsTransfer.test.ts @@ -17,6 +17,54 @@ 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 () => { + const { serializeSettings } = await import("./settingsTransfer"); + const { DEFAULT_SETTINGS } = await import("./constants"); + const settings = { + ...structuredClone(DEFAULT_SETTINGS), + savedFeeds: { + Private: { + title: "Private", + url: "podnotes-private-feed:Private", + urlSecretId: "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef", + artworkUrl: "", + }, + }, + }; + const envelope = serializeSettings(settings, {}, "0.0.0", "2026-07-11T00:00:00Z"); + const exported = ( + envelope.settings as unknown as Record>> + ).savedFeeds.Private; + expect(exported.urlSecretId).toBeUndefined(); + expect(exported.url).toBe("podnotes-private-feed:Private"); + }); + + it("drops urlSecretId arriving through an import", async () => { + const { parseImport } = await import("./settingsTransfer"); + const result = parseImport( + JSON.stringify({ + savedFeeds: { + Foreign: { + title: "Foreign", + url: "podnotes-private-feed:Foreign", + urlSecretId: "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef", + artworkUrl: "", + }, + }, + }), + ); + expect(result.ok).toBe(true); + if (result.ok) { + const feed = ( + result.settings.savedFeeds as unknown as Record> + ).Foreign; + expect(feed.urlSecretId).toBeUndefined(); + expect(feed.url).toBe("podnotes-private-feed:Foreign"); + } + }); +}); + describe("serializeSettings", () => { it("wraps settings in a versioned envelope", () => { const envelope = serializeSettings(makeSettings(), {}, "2.16.0", NOW); diff --git a/src/settingsTransfer.ts b/src/settingsTransfer.ts index f9675527..4ef26a39 100644 --- a/src/settingsTransfer.ts +++ b/src/settingsTransfer.ts @@ -122,6 +122,29 @@ export type ParseResult = * SecretStorage references are always excluded. Plaintext values appear only in * the separate payload explicitly passed by the caller. */ +/** + * SecretStorage references are device-local capabilities and never transfer + * data: an imported urlSecretId is at best meaningless on this device. The + * private URL itself stays behind (placeholder) unless the source device + * exports it another way; the feed is re-added on the target device. + */ +function stripFeedUrlSecretReferences(savedFeeds: unknown): unknown { + if (typeof savedFeeds !== "object" || savedFeeds === null || Array.isArray(savedFeeds)) { + return savedFeeds; + } + const out: Record = {}; + for (const [key, feed] of Object.entries(savedFeeds)) { + if (typeof feed === "object" && feed !== null && !Array.isArray(feed)) { + const rest = { ...(feed as Record) }; + delete rest.urlSecretId; + out[key] = rest; + } else { + out[key] = feed; + } + } + return out; +} + export function serializeSettings( settings: IPodNotesSettings, opts: ExportOptions, @@ -136,6 +159,7 @@ export function serializeSettings( if (!IMPORTABLE_KEYS.has(key)) continue; out[key] = persisted[key]; } + out.savedFeeds = stripFeedUrlSecretReferences(out.savedFeeds); const secrets = normalizeSecrets(opts.secrets ?? {}); return { @@ -223,6 +247,11 @@ export function parseImport(jsonText: string): ParseResult { } const settings = sanitizeImportedSettings(source); + if ("savedFeeds" in settings) { + (settings as Record).savedFeeds = stripFeedUrlSecretReferences( + settings.savedFeeds, + ); + } if (Object.keys(settings).length === 0 && Object.keys(secrets).length === 0) { return { diff --git a/src/ui/settings/PodNotesSettingsTab.ts b/src/ui/settings/PodNotesSettingsTab.ts index 98a0e271..b2d55836 100644 --- a/src/ui/settings/PodNotesSettingsTab.ts +++ b/src/ui/settings/PodNotesSettingsTab.ts @@ -39,6 +39,7 @@ import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import { get } from "svelte/store"; import { exportOPML, importOPML } from "src/opml"; import { clearFeedCache } from "src/services/FeedCacheService"; +import { migratePrivateFeedUrls } from "src/services/privateFeeds"; import { describeSecrets, mergeImportedSettings, @@ -920,6 +921,12 @@ export class PodNotesSettingsTab extends PluginSettingTab { } const merged = mergeImportedSettings(previous, { ...imported, ...secretReferences }); + // A raw data.json import can carry credential-bearing feed URLs; intern + // them BEFORE the strict persist below so the secret never reaches disk. + merged.savedFeeds = migratePrivateFeedUrls( + merged.savedFeeds, + this.plugin.feedUrls, + ).savedFeeds; const failedCandidate = structuredClone(merged); const openAIReferenceChanged = merged.openAISecretId !== previous.openAISecretId; if (openAIReferenceChanged) this.plugin.invalidateTranscriptionCredentialCache(); diff --git a/src/ui/settings/PodcastQueryGrid.svelte b/src/ui/settings/PodcastQueryGrid.svelte index d3b777e5..0728c777 100644 --- a/src/ui/settings/PodcastQueryGrid.svelte +++ b/src/ui/settings/PodcastQueryGrid.svelte @@ -107,9 +107,16 @@ const { podcast } = event.detail; savedFeeds.update((feeds) => { const removed = feeds[podcast.title]; - if (removed?.urlSecretId) get(plugin).feedUrls.delete(removed.urlSecretId); const newFeeds = { ...feeds }; delete newFeeds[podcast.title]; + // 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 stillReferenced = Object.values(newFeeds).some( + (feed) => feed.urlSecretId === removed?.urlSecretId, + ); + if (removed?.urlSecretId && !stillReferenced) { + get(plugin).feedUrls.delete(removed.urlSecretId); + } return newFeeds; }); updateSearchResults(); diff --git a/src/utility/privateFeedUrl.ts b/src/utility/privateFeedUrl.ts index 366e4b50..7a584565 100644 --- a/src/utility/privateFeedUrl.ts +++ b/src/utility/privateFeedUrl.ts @@ -19,7 +19,7 @@ const PLACEHOLDER_SCHEME = "podnotes-private-feed:"; * working, their URL just stays in data.json like before. */ const SECRET_QUERY_PARAM = - /^(auth|token|key|secret|pass|password|apikey|api[-_]key|access[-_]token|sig|signature)$/i; + /^(auth|token|key|secret|pass|password|apikey|api[-_]key|access[-_]token|sig|signature|jwt|credential|credentials)$/i; export function isCredentialBearingUrl(rawUrl: string): boolean { let url: URL;