fix(privacy): reuse the reference of a private address this device holds

src/main.persistence.test.ts

@@ -263,6 +263,47 @@ describe("private addresses adopted from sync", () => {
 		}
 	});
 
+	it("keeps one reference for an address that keeps arriving raw, also after a restart", async () => {
+		const plugin = makePlugin();
+		await plugin.loadSettings();
+		attachDataFileWriter(plugin);
+		const externalWrite = plugin.saveData.bind(plugin);
+		const save = vi.fn(externalWrite);
+		Object.assign(plugin, { saveData: save });
+		const storage = plugin.app.secretStorage;
+		const address = "https://priv3.supercast.com/feeds/REPEATED_SENTINEL";
+
+		for (let incoming = 1; incoming <= 3; incoming++) {
+			await externalWrite({
+				...structuredClone(released2211),
+				savedFeeds: { Private: { title: "Private", url: address, artworkUrl: "" } },
+				incoming,
+			});
+			await Promise.all([
+				plugin.onExternalSettingsChange(),
+				plugin.onExternalSettingsChange(),
+			]);
+			await settled();
+			expect(save).toHaveBeenCalledTimes(incoming);
+		}
+
+		const disk = await plugin.loadData();
+		expect(JSON.stringify(disk)).not.toContain("REPEATED_SENTINEL");
+		const reference = disk.savedFeeds.Private.urlSecretId;
+		expect(storage.listSecrets()).toEqual([reference]);
+
+		const restarted = makePlugin(vi.fn().mockResolvedValue(undefined), {
+			dataFileExists: true,
+		});
+		Object.assign(restarted, { app: { ...restarted.app, secretStorage: storage } });
+		await restarted.saveData(disk);
+		await restarted.loadSettings();
+		expect(storage.listSecrets()).toEqual([reference]);
+		expect(resolveFeedUrl(restarted.settings.savedFeeds.Private, restarted.feedUrls)).toBe(
+			address,
+		);
+	});
+
 	it("protects a copied address even when the decoded document equals memory", async () => {
 		const plugin = makePlugin();
 		const address = "https://private.example/feed.xml?auth=COPIED_ADOPTION_SENTINEL";

src/main.ts

@@ -521,8 +521,8 @@ export default class PodNotes extends Plugin implements IPodNotes {
 	}
 
 	private dataFileAccess(): DataFileAccess<PersistedPodNotesDataV2> {
-		// As found: reading a private feed URL again stores it under a new reference, so
-		// the same file never reads as the same document twice.
+		// The file as found, before decoding protects or repairs it. A readback still equal
+		// to it means the write did not land.
 		let lastRead: unknown;
 		return {
 			read: async (base): Promise<ReadOutcome<PersistedPodNotesDataV2>> => {

src/services/FeedUrlRepository.test.ts

@@ -29,11 +29,46 @@ describe("FeedUrlRepository", () => {
 		expect(values.get(id)).toBe("https://p.example/rss?auth=tok");
 	});
 
-	it("allocates distinct IDs even for identical URLs (references sync, secrets don't)", () => {
-		const { api } = storage();
+	it("stores a URL this device holds under its reference, and another URL under its own", () => {
+		const { api, values } = storage();
 		const repository = new FeedUrlRepository(api);
 		const first = repository.store("https://a.example/rss?auth=1");
-		const second = repository.store("https://a.example/rss?auth=1");
+		expect(repository.store(" https://a.example/rss?auth=1 ")).toBe(first);
+		const other = repository.store("https://a.example/rss?auth=2");
+		expect(other).not.toBe(first);
+		expect([...values]).toEqual([
+			[first, "https://a.example/rss?auth=1"],
+			[other, "https://a.example/rss?auth=2"],
+		]);
+		expect(api.setSecret).toHaveBeenCalledTimes(2);
+	});
+
+	it("never reuses a cleared reference, one holding another URL here, or a foreign ID", () => {
+		const elsewhere = "podnotes-feed-url-12345678-9abc-4def-8123-456789abcdef";
+		const { api, values } = storage({
+			"other-plugin-feed": "https://a.example/rss?auth=1",
+			[elsewhere]: "https://a.example/rss?auth=other",
+		});
+		const repository = new FeedUrlRepository(api);
+		const removed = repository.store("https://a.example/rss?auth=1");
+		repository.delete(removed);
+
+		const id = repository.store("https://a.example/rss?auth=1");
+
+		expect(() => repository.store(" ")).toThrow(/must not be empty/);
+		expect([removed, elsewhere]).not.toContain(id);
+		expect([...values]).toEqual([
+			["other-plugin-feed", "https://a.example/rss?auth=1"],
+			[elsewhere, "https://a.example/rss?auth=other"],
+			[removed, ""],
+			[id, "https://a.example/rss?auth=1"],
+		]);
+	});
+
+	it("allocates IDs that two devices storing one URL never share (references sync, secrets don't)", () => {
+		const url = "https://a.example/rss?auth=1";
+		const first = new FeedUrlRepository(storage().api).store(url);
+		const second = new FeedUrlRepository(storage().api).store(url);
 		expect(first).not.toBe(second);
 		expect(isFeedUrlSecretId(first)).toBe(true);
 		expect(isFeedUrlSecretId(second)).toBe(true);

src/services/FeedUrlRepository.ts

@@ -16,6 +16,11 @@ import { isValidSecretId } from "src/types/Credentials";
  * one reference names each device's own copy. Content-free by construction -
  * neither the feed name nor the URL leaks through the ID namespace, which
  * `listSecrets` exposes.
+ *
+ * SecretStorage's API has no delete, so a cleared ID stays listed for good.
+ * Storing a URL this device already holds therefore returns the ID holding it.
+ * Only a PodNotes ID that resolves here to that URL qualifies. A cleared ID
+ * stays retired, since another device may still reference it.
  */
 const FEED_URL_SECRET_ID =
 	/^podnotes-feed-url-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/;
@@ -45,8 +50,12 @@ export class FeedUrlRepository {
 		}
 	}
 
-	/** Store a private feed URL under a fresh PodNotes-owned ID and verify the readback. */
+	/** Store a private feed URL under its ID here, else a fresh one, and verify the readback. */
 	store(url: string): string {
+		const held = this.storage
+			.listSecrets()
+			.find((candidate) => this.resolve(candidate) === url.trim());
+		if (held) return held;
 		const id = `podnotes-feed-url-${crypto.randomUUID()}`;
 		this.attach(id, url);
 		return id;

src/services/library.test.ts

@@ -13,7 +13,7 @@ import { deferred, MemoryStorage } from "../../tests/mocks/feeds";
 import { FeedRepository, loadFeed } from "./FeedRepository";
 import { FeedUrlRepository } from "./FeedUrlRepository";
 import { createLibraryActions } from "./library";
-import { resolveFeedUrl } from "./privateFeeds";
+import { protectPrivateFeeds, resolveFeedUrl } from "./privateFeeds";
 
 vi.mock("src/utility/networkRequest", async (importOriginal) => {
 	const actual = await importOriginal<typeof import("src/utility/networkRequest")>();
@@ -203,6 +203,26 @@ describe("removing a podcast", () => {
 		expect(get(savedFeeds)).toEqual({});
 		expect(secrets.getSecret(reference)).toBeFalsy();
 	});
+
+	test("keeps the address of a podcast protected with the same private URL as a removed one", () => {
+		const { actions, feedUrls } = actionsOn(new obsidian.SecretStorage());
+		const raw = { title: "Private Show", url: PRIVATE_URL, artworkUrl: "" };
+		const { settings } = protectPrivateFeeds(
+			{
+				...DEFAULT_SETTINGS,
+				savedFeeds: { "Private Show": raw, Alias: { ...raw, title: "Alias" } },
+			},
+			feedUrls,
+		);
+		savedFeeds.set(settings.savedFeeds);
+		expect(settings.savedFeeds.Alias.urlSecretId).toBe(
+			settings.savedFeeds["Private Show"].urlSecretId,
+		);
+
+		actions.removePodcast("Alias");
+
+		expect(resolveFeedUrl(get(savedFeeds)["Private Show"], feedUrls)).toBe(PRIVATE_URL);
+	});
 });
 
 describe("undoing Remove podcast", () => {

tests/e2e/sync-private-address.test.ts

@@ -12,8 +12,13 @@ import { jsLiteral, RELOAD_OPTIONS, WAIT_OPTS } from "./runtime";
 
 const getContext = createPodNotesE2EHarness("sync-private-address");
 
+const feedUrlSecretIds = (obsidian: ReturnType<typeof getContext>["obsidian"]) =>
+	obsidian.dev.evalJson<string[]>(
+		`app.secretStorage.listSecrets().filter((id) => id.startsWith("podnotes-feed-url-"))`,
+	);
+
 test.each(["raw event", "external settings handler"])(
-	"protects a synced 2.21.1 private address on disk once per incoming change through %s",
+	"protects a synced 2.21.1 private address on disk once per incoming change, under one reference, through %s",
 	async (trigger) => {
 		const { obsidian, plugin } = getContext();
 		await plugin.updateDataAndReload<Record<string, unknown>>((data) => {
@@ -46,6 +51,7 @@ test.each(["raw event", "external settings handler"])(
 			copiedNote: `from ${address}`,
 		};
 		delete data.currentEpisode;
+		const before = await feedUrlSecretIds(obsidian);
 		const evidence = [];
 		for (let incoming = 1; incoming <= 3; incoming++) {
 			const recorder = await recordWrites(obsidian);
@@ -77,13 +83,32 @@ test.each(["raw event", "external settings handler"])(
 			expect(writes).toMatchObject({ attempted: 1, completed: 1, failed: 0, intact: true });
 			evidence.push({ incoming, writes, sentinelOnDisk: text.includes(sentinel) });
 		}
+		const disk = await plugin
+			.data<{ savedFeeds: { Private: { urlSecretId: string } } }>()
+			.read();
+		const listed = await feedUrlSecretIds(obsidian);
+		expect(listed.filter((id) => !before.includes(id))).toEqual([
+			disk.savedFeeds.Private.urlSecretId,
+		]);
+		await plugin.reload(RELOAD_OPTIONS);
+		expect(await feedUrlSecretIds(obsidian)).toEqual(listed);
+		expect(
+			await obsidian.dev.evalJson<string | null>(
+				"(() => { const p = app.plugins.plugins.podnotes; return p.feedUrls.resolve(p.settings.savedFeeds.Private.urlSecretId); })()",
+			),
+		).toBe(address);
 		await showLibrary(obsidian);
 		expect(await shownPodcasts(obsidian)).toContain("Private");
 		const directory = resolve(".obsidian-e2e-artifacts/fix488");
 		await mkdir(directory, { recursive: true });
 		await obsidian.dev.screenshot(resolve(directory, `${trigger.replaceAll(" ", "-")}.png`));
 		console.info(
-			JSON.stringify({ trigger, evidence, version: await obsidian.exec("version") }),
+			JSON.stringify({
+				trigger,
+				evidence,
+				feedUrlSecretIds: { before: before.length, after: listed.length },
+				version: await obsidian.exec("version"),
+			}),
 		);
 	},
 );