fix: make episode identity key collision-resistant and prototype-safe - #226
Conversation
getEpisodeKey built an episode's identity as `${podcastName}::${title}` with no
escaping, so distinct (podcastName, title) pairs collided: ("A", "B::C") and
("A::B", "C") both mapped to "A::B::C", and a boundary colon (("A:", "B") vs
("A", ":B")) collided too. Both fields come verbatim from RSS, so a malicious
feed could alias another subscribed feed's played-state / dedup key.
Keep the plain `name::title` format for ordinary pairs (byte-identical, so
existing data.json keys resolve with no migration) and escape only the rare
delimiter-forging pair into a NUL-prefixed, colon-free, injective form that is
disjoint from both the plain composite keys and the legacy title-only keys. The
manual escape never throws (unlike encodeURIComponent on lone surrogates), so a
hostile value cannot turn key generation into a crash.
findPlayedEpisodesInFeeds grouped played episodes onto a plain object keyed by
the feed-controlled podcastName, so names like "__proto__" / "constructor"
resolved to inherited prototype members and crashed `.push`, rejecting the
returned promise. Group with a Map so any feed-controlled name is a safe key.
deepsec: other-key-collision, other-unsafe-object-key
Deploying podnotes with
|
| Latest commit: |
440d91c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9cfd382a.podnotes.pages.dev |
| Branch Preview URL: | https://chhoumann-deepsec-key-safety.podnotes.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 440d91c83b
ℹ️ 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".
| // disjoint from the plain composite keys below; the NUL prefix keeps it | ||
| // disjoint from the legacy title-only keys. The encoding is injective, so | ||
| // no two distinct pairs can share a key. | ||
| return `${ESCAPED_KEY_PREFIX}${escapeKeyComponent(podcastName)}:${escapeKeyComponent(title)}`; |
There was a problem hiding this comment.
Preserve legacy lookups for escaped episode keys
When an existing episode has a delimiter-forging name/title, this now returns a new escaped key, but prior releases saved its progress under the raw ${podcastName}::${title} key. The restore path (getPlayedEpisode / restoreSavedPlaybackTime) only probes getEpisodeKey(episode) and the title-only legacy key, so a user who already has progress for a podcast like A / B::C will lose the saved position/finished state after upgrading until the episode is played again. Please keep the old raw composite key as an alias or migrate it for the ambiguous cases.
Useful? React with 👍 / 👎.
## [2.17.3](2.17.2...2.17.3) (2026-07-09) ### Bug Fixes * **feed/search:** parse feed once + content-based search cache ([#225](#225)) ([053d51f](053d51f)), closes [#149](#149) * make episode identity key collision-resistant and prototype-safe ([#226](#226)) ([a5683db](a5683db)) * **opml:** correct import progress math and saved-count reporting ([#221](#221)) ([a79e529](a79e529)) * **security:** validate feed/URI URLs and cap download size ([#223](#223)) ([edef281](edef281)) * **template:** neutralize feed-controlled note injection ([#228](#228)) ([ef4ecbd](ef4ecbd)) * **timestamp:** escape live table-cell pipe after an escaped backslash ([#227](#227)) ([a34dfca](a34dfca)) * **transcription:** resolve three deepsec transcription-pipeline bugs ([#224](#224)) ([83c34e7](83c34e7))
|
🎉 This PR is included in version 2.17.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Fixes two deepsec-confirmed findings in how an episode's identity key is built and used.
other-key-collision(src/utility/episodeKey.ts)getEpisodeKeybuilt identity as${podcastName}::${title}with no escaping, so the mapping from (podcastName, title) to key was not injective. Both fields come verbatim from a feed's RSS, so a malicious or merely unusual feed could alias another subscribed feed's episode:("A", "B::C")and("A::B", "C")both produced"A::B::C".("A:", "B")and("A", ":B")both produced"A:::B".These keys are the primary index for played-episode persistence (
playedEpisodes[key]) and for playlist/favorite dedup (isSameStoredEpisode), so a collision conflated one episode's finished/progress state with another's.Fix: keep the plain
name::titleformat for ordinary pairs and escape only the rare delimiter-forging pair (component contains::, or name ends with:, or title starts with:) into a collision-free form: a NUL-prefixed, colon-free, injective encoding."Serial: A Show") are byte-identical to the old key, so existingdata.jsonkeys resolve unchanged. Only inputs that were already ambiguous/colliding get a new key, and those re-key deterministically.::(so it can't collide with a plain composite key) and is NUL-prefixed (so it can't collide with a legacy title-only key - a feed with an empty<title>yieldspodcastName === ""and a raw-title key; a NUL byte can't occur in XML feed text). Verified injective by brute force over a delimiter-saturated alphabet (9025 pairs -> 9025 distinct keys).encodeURIComponent, which throwsURIErroron lone surrogates.other-unsafe-object-key(src/utility/findPlayedEpisodes.ts)findPlayedEpisodesInFeedsgrouped played episodes onto a plain object{}keyed by the feed-controlledpodcastName. For"__proto__"/"constructor",acc[name]resolved to an inherited prototype member (truthy, no.push), throwing aTypeErrorand rejecting the returned promise.Fix: group with a
Map<string, PlayedEpisode[]>so any feed-controlled name is a safe key. Behavior (grouping, title matching, ordering) is otherwise unchanged.Scope note
The
${podcastName}::${title}format is duplicated inepisodeStatus.ts(getPlayedEpisodeRecordKey) andepisodeListEntry.ts. To keep this change scoped to the finding's files, those were left as-is: for the common case they remain byte-identical togetEpisodeKey; for the rare ambiguous pair they diverge only in a benign display-layer dedup, never a crash or data corruption. Consolidating the key format into a single source of truth is a reasonable follow-up.Tests / validation
__proto__/constructorgrouping safety.deepsec slugs:
other-key-collision,other-unsafe-object-key