fix(privacy): protect private addresses adopted from sync

src/main.persistence.test.ts

@@ -225,6 +225,104 @@ describe("when a synced private feed address is in data.json during a save", ()
 	});
 });
 
+describe("private addresses adopted from sync", () => {
+	it("protects each incoming raw address on disk once without a local save", async () => {
+		const plugin = makePlugin();
+		await plugin.loadSettings();
+		const clock = manualClock();
+		attachDataFileWriter(plugin, clock);
+		const externalWrite = plugin.saveData.bind(plugin);
+		const save = vi.fn(externalWrite);
+		Object.assign(plugin, { saveData: save });
+
+		for (let incoming = 1; incoming <= 3; incoming++) {
+			const address = `https://priv3.supercast.com/feeds/ADOPTED_SENTINEL_${incoming}`;
+			await externalWrite({
+				...structuredClone(released2211),
+				savedFeeds: { Private: { title: "Private", url: address, artworkUrl: "" } },
+				copiedNote: `from ${address}`,
+			});
+
+			await plugin.onExternalSettingsChange();
+			await settled();
+
+			const disk = await plugin.loadData();
+			expect(JSON.stringify(disk)).not.toContain("ADOPTED_SENTINEL");
+			expect(disk).toMatchObject({
+				savedFeeds: { Private: { url: "podnotes-private-feed:Private" } },
+				copiedNote: "from podnotes-private-feed:Private",
+			});
+			expect(resolveFeedUrl(disk.savedFeeds.Private, plugin.feedUrls)).toBe(address);
+			expect(save).toHaveBeenCalledTimes(incoming);
+			await plugin.onExternalSettingsChange();
+			clock.advance(30_000);
+			clock.runAll();
+			await settled();
+			expect(save).toHaveBeenCalledTimes(incoming);
+			expect(clock.pending()).toBe(0);
+		}
+	});
+
+	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";
+		const externalWrite = plugin.saveData.bind(plugin);
+		await externalWrite({
+			...encodePodNotesData(structuredClone(DEFAULT_SETTINGS)),
+			savedFeeds: { Private: { title: "Private", url: address, artworkUrl: "" } },
+			copiedNote: `from ${address}`,
+		});
+		await plugin.loadSettings();
+		attachDataFileWriter(plugin);
+		const protectedData = await plugin.loadData();
+		await externalWrite({ ...protectedData, copiedNote: `from ${address}` });
+		const save = vi.fn(externalWrite);
+		Object.assign(plugin, { saveData: save });
+
+		await plugin.onExternalSettingsChange();
+		await settled();
+
+		expect(await plugin.loadData()).toEqual(protectedData);
+		expect(save).toHaveBeenCalledTimes(1);
+	});
+
+	it("does not rewrite a 2.21.1 document just to add its library id", async () => {
+		const plugin = makePlugin();
+		await plugin.loadSettings();
+		attachDataFileWriter(plugin);
+		const externalWrite = plugin.saveData.bind(plugin);
+		await externalWrite(structuredClone(released2211));
+		const save = vi.fn(externalWrite);
+		Object.assign(plugin, { saveData: save });
+
+		await plugin.onExternalSettingsChange();
+		await settled();
+
+		expect(plugin.settings.playedEpisodes["PRIV2 Both::PRIV2 Episode"].time).toBe(30);
+		expect(await plugin.loadData()).toEqual(released2211);
+		expect(save).not.toHaveBeenCalled();
+	});
+
+	it("does not save a public or already protected adoption", 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 });
+		for (const url of ["https://example.com/feed.xml", "podnotes-private-feed:Private"]) {
+			await externalWrite({
+				...encodePodNotesData(structuredClone(DEFAULT_SETTINGS)),
+				savedFeeds: { Private: { title: "Private", url, artworkUrl: "" } },
+			});
+			await plugin.onExternalSettingsChange();
+			await settled();
+			expect(plugin.settings.savedFeeds.Private.url).toBe(url);
+			expect(save).not.toHaveBeenCalled();
+		}
+	});
+});
+
 it("takes in the file a save read back before one that lands after it", async () => {
 	const notice = vi.spyOn(obsidian, "Notice");
 	const plugin = makePlugin();
@@ -1223,7 +1321,7 @@ describe("PodNotes persistence integration", () => {
 					persistenceMerge.jsonValuesEqual(result.value, repairs.mock.calls[index][1]),
 			),
 		).toBe(true);
-		expect(redactions).toHaveBeenCalledTimes(5);
+		expect(redactions).toHaveBeenCalledTimes(6);
 		expect(saveData).toHaveBeenCalledTimes(1);
 		expect(saveData.mock.lastCall?.[0]).toMatchObject({
 			defaultVolume: 0.4,

src/main.ts

@@ -625,6 +625,7 @@ export default class PodNotes extends Plugin implements IPodNotes {
 			doc: (restored
 				? redactPrivateFeedAddresses(doc, addresses)
 				: doc) as PersistedPodNotesDataV2,
+			rewrite: redactPrivateFeedAddresses(data, addresses) !== data,
 		};
 	}
 

src/persistence/writer.test.ts

@@ -93,6 +93,57 @@ afterEach(() => {
 	vi.restoreAllMocks();
 });
 
+describe("adopted documents that need a rewrite", () => {
+	it("rereads before writing and waits when the file becomes unreadable", async () => {
+		const { access, clock, state, writer } = setup();
+		const protectedRead = { kind: "file" as const, doc: doc(), rewrite: true };
+		state.disk = withFeed(doc(), "Raw");
+		access.read.mockImplementation(async () =>
+			state.disk?.savedFeeds.Raw ? protectedRead : found(state.disk),
+		);
+		access.read.mockResolvedValueOnce(protectedRead).mockResolvedValueOnce(refused(UNREADABLE));
+		vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+		await writer.onExternalChange();
+		await settled();
+
+		expect(access.read).toHaveBeenCalledTimes(2);
+		expect(state.writes).toEqual([]);
+		expect(clock.pending()).toBe(1);
+		clock.advance(10_000);
+		clock.runAll();
+		await settled();
+		expect(state.writes).toEqual([doc()]);
+	});
+
+	it("keeps rewrite work through a failed write and retries it once", async () => {
+		const { access, clock, state, writer } = setup();
+		const protectedRead = { kind: "file" as const, doc: doc(), rewrite: true };
+		state.disk = withFeed(doc(), "Raw");
+		access.read.mockImplementation(async () =>
+			state.disk?.savedFeeds.Raw ? protectedRead : found(state.disk),
+		);
+		access.read.mockResolvedValueOnce(protectedRead).mockResolvedValueOnce(protectedRead);
+		access.write.mockRejectedValueOnce(new Error("disk full"));
+		vi.spyOn(console, "error").mockImplementation(() => undefined);
+
+		await writer.onExternalChange();
+		await settled();
+
+		expect(access.write).toHaveBeenCalledTimes(1);
+		expect(state.writes).toEqual([]);
+		clock.advance(10_000);
+		clock.runAll();
+		await settled();
+		expect(state.writes).toEqual([doc()]);
+		await writer.flush();
+		expect(access.write).toHaveBeenCalledTimes(2);
+		await writer.onExternalChange();
+		await writer.flush();
+		expect(access.write).toHaveBeenCalledTimes(2);
+	});
+});
+
 describe("DataFileWriter saves", () => {
 	it("saves a change right away, once for a burst of changes", async () => {
 		const { clock, state, writer } = setup();

src/persistence/writer.ts

@@ -24,8 +24,9 @@ export interface Refusal {
 	readonly message: string;
 }
 
+/** `rewrite`: the file still holds a private address that `doc` protects. */
 export type ReadOutcome<Doc> =
-	| { readonly kind: "file"; readonly doc: Doc }
+	| { readonly kind: "file"; readonly doc: Doc; readonly rewrite?: boolean }
 	| { readonly kind: "missing" }
 	| { readonly kind: "refused"; readonly refusal: Refusal };
 
@@ -103,6 +104,7 @@ export class DataFileWriter<Doc> {
 	private lastSaveAt = Number.NEGATIVE_INFINITY;
 	/** A write that failed, and the memory it was made from. It may have reached disk anyway. */
 	private failedWrite: { local: Doc; written: Doc } | null = null;
+	private rewritePending = false;
 	/** What PodNotes last loaded or wrote; null until a readable file was loaded. */
 	private base: Doc | null;
 	private refusal: Refusal | null;
@@ -224,7 +226,12 @@ export class DataFileWriter<Doc> {
 	}
 
 	private pending(): boolean {
-		return this.refusal !== null || this.failedWrite !== null || this.hasUnsavedEdits();
+		return (
+			this.rewritePending ||
+			this.refusal !== null ||
+			this.failedWrite !== null ||
+			this.hasUnsavedEdits()
+		);
 	}
 
 	private refuse(refusal: Refusal): void {
@@ -330,6 +337,7 @@ export class DataFileWriter<Doc> {
 				this.scheduleCheckpoint();
 				throw error;
 			}
+			this.rewritePending = false;
 			if (outcome.kind === "verified") {
 				this.failedWrite = null;
 				this.settle(local, toWrite);
@@ -371,6 +379,7 @@ export class DataFileWriter<Doc> {
 			this.refuse(read.refusal);
 			return;
 		}
+		this.rewritePending = read.kind === "file" && read.rewrite === true;
 		const disk = read.kind === "file" ? read.doc : null;
 		if (this.refusal && this.base !== null) {
 			this.lastSaveAt = this.clock.now();
@@ -379,10 +388,13 @@ export class DataFileWriter<Doc> {
 		}
 		if (!disk) return;
 		this.settleFailedWrite(disk);
-		if (this.base && jsonValuesEqual(disk, this.base)) return;
+		if (this.base && jsonValuesEqual(disk, this.base)) {
+			if (this.rewritePending) this.request();
+			return;
+		}
 
 		const merged = this.adopt(disk, this.local());
-		if (jsonValuesEqual(merged, disk)) this.cancelTimer();
+		if (!this.rewritePending && jsonValuesEqual(merged, disk)) this.cancelTimer();
 		else this.request();
 	}
 

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

@@ -0,0 +1,119 @@
+import { mkdir, readFile, writeFile } from "node:fs/promises";
+import { resolve } from "node:path";
+import { expect, onTestFinished, test } from "vitest";
+import { SUPERCAST_FIXTURE_HOST } from "../../scripts/e2e-instance.mjs";
+import released2211 from "../fixtures/persistence/released-2.21.1.json";
+import { createPodNotesE2EHarness } from "./harness";
+import { showLibrary, showPodNotes } from "./journey/actions";
+import { recordWrites } from "./journey/library";
+import { type FixtureFeed, startFixtureServer } from "./journey/fixtures";
+import { holdsFor, shownPodcasts } from "./journey/wait";
+import { jsLiteral, RELOAD_OPTIONS, WAIT_OPTS } from "./runtime";
+
+const getContext = createPodNotesE2EHarness("sync-private-address");
+
+test.each(["raw event", "external settings handler"])(
+	"protects a synced 2.21.1 private address on disk once per incoming change through %s",
+	async (trigger) => {
+		const { obsidian, plugin } = getContext();
+		await plugin.updateDataAndReload<Record<string, unknown>>((data) => {
+			data.savedFeeds = {};
+			delete data.currentEpisode;
+		}, RELOAD_OPTIONS);
+		await showPodNotes(obsidian);
+		const path = await plugin.dataPath();
+		const sentinel = "FIX488_ADOPTED_SENTINEL";
+		const feed: FixtureFeed = {
+			title: "Private",
+			slug: "adopted-private",
+			host: SUPERCAST_FIXTURE_HOST,
+			path: `/feeds/${sentinel}`,
+			artwork: true,
+			episodes: [],
+		};
+		const server = await startFixtureServer([feed]);
+		onTestFinished(() => server.stop());
+		const address = server.feedUrl(feed);
+		const data: Record<string, unknown> = {
+			...structuredClone(released2211),
+			savedFeeds: {
+				Private: {
+					title: "Private",
+					url: address,
+					artworkUrl: server.artworkUrl(feed),
+				},
+			},
+			copiedNote: `from ${address}`,
+		};
+		delete data.currentEpisode;
+		const evidence = [];
+		for (let incoming = 1; incoming <= 3; incoming++) {
+			const recorder = await recordWrites(obsidian);
+			await writeFile(path, JSON.stringify({ ...data, incoming }), "utf8");
+			if (trigger === "external settings handler") {
+				await obsidian.dev.evalJsonAsync(
+					"(async () => { await app.plugins.plugins.podnotes.onExternalSettingsChange(); return true; })()",
+				);
+			}
+			await obsidian.waitFor(
+				() =>
+					obsidian.dev.evalJson<boolean>(
+						`app.plugins.plugins.podnotes.settings.savedFeeds.Private?.url === ${jsLiteral("podnotes-private-feed:Private")}`,
+					),
+				{ ...WAIT_OPTS, message: "the synced private feed was adopted" },
+			);
+			await expect
+				.poll(() => readFile(path, "utf8"), { timeout: 5_000, interval: 100 })
+				.not.toContain(sentinel);
+			const disk = await plugin.data<Record<string, unknown>>().read();
+			expect(disk).toMatchObject({
+				incoming,
+				savedFeeds: { Private: { url: "podnotes-private-feed:Private" } },
+				copiedNote: "from podnotes-private-feed:Private",
+			});
+			const text = await readFile(path, "utf8");
+			expect(await holdsFor(obsidian, () => readFile(path, "utf8"), text, 1_000)).toBe(text);
+			const writes = await recorder.stop();
+			expect(writes).toMatchObject({ attempted: 1, completed: 1, failed: 0, intact: true });
+			evidence.push({ incoming, writes, sentinelOnDisk: text.includes(sentinel) });
+		}
+		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") }),
+		);
+	},
+);
+
+test("adopts public and protected feeds without writing them back", async () => {
+	const { obsidian, plugin } = getContext();
+	await plugin.updateDataAndReload<Record<string, unknown>>((data) => {
+		data.savedFeeds = {};
+		delete data.currentEpisode;
+	}, RELOAD_OPTIONS);
+	const path = await plugin.dataPath();
+	for (const url of ["https://example.com/feed.xml", "podnotes-private-feed:Private"]) {
+		const incoming = await plugin.data<Record<string, unknown>>().read();
+		incoming.savedFeeds = { Private: { title: "Private", url, artworkUrl: "" } };
+		const text = JSON.stringify(incoming);
+		const recorder = await recordWrites(obsidian);
+		await writeFile(path, text, "utf8");
+		await obsidian.waitFor(
+			() =>
+				obsidian.dev.evalJson<boolean>(
+					`app.plugins.plugins.podnotes.settings.savedFeeds.Private?.url === ${jsLiteral(url)}`,
+				),
+			WAIT_OPTS,
+		);
+		expect(await holdsFor(obsidian, () => readFile(path, "utf8"), text, 1_000)).toBe(text);
+		expect(await recorder.stop()).toMatchObject({
+			attempted: 0,
+			completed: 0,
+			failed: 0,
+			intact: true,
+		});
+	}
+});