Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 39 additions & 5 deletions src/URIHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -163,11 +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 (!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
Expand All @@ -176,11 +184,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 <a href> 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;
}
Expand All @@ -190,8 +221,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
Expand Down
5 changes: 4 additions & 1 deletion src/createFeedNote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -84,9 +85,11 @@ async function enrichFeed(feed: PodcastFeed): Promise<PodcastFeed> {
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 `??`)
Expand Down
34 changes: 34 additions & 0 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ import {
PodNotesDataError,
} from "./persistence/podNotesData";
import { CredentialRepository } from "./services/CredentialRepository";
import { FeedUrlRepository } from "./services/FeedUrlRepository";
import { migratePrivateFeedUrls, scrubMigratedEpisodeUrls } from "./services/privateFeeds";
import { evictCachedFeedUrls } from "./services/FeedCacheService";

type MediaSessionActionName =
| "previoustrack"
Expand Down Expand Up @@ -73,6 +76,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<MainView>();

Expand Down Expand Up @@ -102,6 +106,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();

Expand Down Expand Up @@ -474,6 +479,35 @@ export default class PodNotes extends Plugin implements IPodNotes {
}
}

// 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) 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.replacements.size > 0) {
settings = scrubMigratedEpisodeUrls(
{ ...settings, savedFeeds: feedMigration.savedFeeds },
feedMigration.replacements,
);
evictCachedFeedUrls([...feedMigration.replacements.keys()]);
await this.saveData(encodePodNotesData(settings, decoded.unknownFields));
Comment thread
chhoumann marked this conversation as resolved.
const count = feedMigration.replacements.size;
new Notice(
`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);

this.settings = settings;
this.persistenceUnknownFields = decoded.unknownFields;

Expand Down
19 changes: 19 additions & 0 deletions src/opml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,29 @@ const getFeed = vi.fn(defaultGetFeed);
// accurate saved count).
const noticeMessages = vi.hoisted(() => [] as string[]);

const feedUrlSecrets = vi.hoisted(() => new Map<string, string>());

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", () => ({
Expand Down
24 changes: 20 additions & 4 deletions src/opml.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -163,7 +164,8 @@ async function importOPML(opml: string): Promise<void> {
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;
Expand Down Expand Up @@ -216,8 +218,16 @@ async function exportOPML(app: App, feeds: PodcastFeed[], filePath = "PodNotes_E
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;");
const feedOutline = (feed: PodcastFeed) =>
`<outline text="${escAttr(feed.title)}" type="rss" xmlUrl="${escAttr(feed.url)}" />`;
// 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.
let exportedPrivateFeeds = 0;
const feedOutline = (feed: PodcastFeed) => {
const url = resolveFeedUrl(feed, get(plugin).feedUrls) ?? "";
if (feed.urlSecretId && url) exportedPrivateFeeds += 1;
return `<outline text="${escAttr(feed.title)}" type="rss" xmlUrl="${escAttr(url)}" />`;
};
const feedsOutline = (_feeds: PodcastFeed[]) =>
`<outline text="feeds">${feeds.map(feedOutline).join("")}</outline>`;

Expand All @@ -227,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")) {
Expand Down
21 changes: 21 additions & 0 deletions src/parser/feedParser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
7 changes: 6 additions & 1 deletion src/parser/feedParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ export default class FeedParser {
async getEpisodes(sourceUrl: string): Promise<Episode[]> {
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));
Expand Down
1 change: 1 addition & 0 deletions src/persistence/collectionCodecs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down
19 changes: 19 additions & 0 deletions src/services/FeedCacheService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading