feat(security): keep private feed URLs out of synced data#295
Merged
Conversation
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.
Deploying podnotes with
|
| Latest commit: |
64a3235
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ca391940.podnotes.pages.dev |
| Branch Preview URL: | https://feat-private-feed-secrets.podnotes.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c87c76668
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A credentialed subscription URL - userinfo (
user:pass@) or an auth-bearing query parameter like Patreon's?auth=...- is a durable subscriber secret. Persisting it indata.jsonleaks it through vault sync, git-versioned vaults, and shared vaults. This is PR 3 of the plan that replaced the capability architecture: the same protection goal, solved with a feed ID + secret reference instead of opaque handles and sealed records.Design. Private feeds persist a non-fetchable placeholder (
podnotes-private-feed:<name>- deliberately not http(s), so the network gate refuses it if it ever leaks into a fetch) plus aurlSecretIdreference. The real URL lives in Obsidian's vault-local SecretStorage under content-freepodnotes-feed-urlIDs and is resolved only immediately before retrieval, confined to the fetch call.obsidian://podnoteslinks, whose placeholder resolution is local-only (the name must match a saved feed on this device; the resolved URL still passes the gate).FeedParsernow projects private-feed episodes against the SAVED feed, soepisode.feedUrl/episode.url- which persist via the episode cache andcurrentEpisode- carry the placeholder, never the resolved secret. Regression-tested on both the refresh and URI-handler paths.listSecrets) reaps references dropped by wholesale settings imports. A second synced device gets an actionable notice and re-adds the feed - SecretStorage is device-local by design and secrets are never synced.xmlUrl.Documented residuals: Supercast-style path-embedded tokens are indistinguishable from public URLs and keep today's behavior (a future "mark as private" toggle can cover them); enclosure URLs inside cached episodes are typically short-lived CDN signatures and out of scope.
Verification. 1,116 tests (26 new + parser leak regressions), lint/format/typecheck/build green. Live in an isolated Obsidian vault: seeded a credentialed URL into
data.json, loaded the plugin, and confirmed the secret was scrubbed into SecretStorage (placeholder + reference left behind), the feed fetches through the resolved URL (119 ms, real network), an unknown placeholder link shows the device-availability notice, unsubscribe clears the secret, anddev:errorsstayed clean throughout.