Skip to content

fix: make episode identity key collision-resistant and prototype-safe - #226

Merged
chhoumann merged 1 commit into
masterfrom
chhoumann/deepsec-key-safety
Jun 29, 2026
Merged

chhoumann merged 1 commit into
masterfrom
chhoumann/deepsec-key-safety

Conversation

@chhoumann

Copy link
Copy Markdown
Owner

Fixes two deepsec-confirmed findings in how an episode's identity key is built and used.

other-key-collision (src/utility/episodeKey.ts)

getEpisodeKey built 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 boundary colon collided too: ("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::title format 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.

  • Backward compatible / no migration: ordinary pairs (including names/titles with single colons like "Serial: A Show") are byte-identical to the old key, so existing data.json keys resolve unchanged. Only inputs that were already ambiguous/colliding get a new key, and those re-key deterministically.
  • Disjoint namespaces: the escaped form has no :: (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> yields podcastName === "" 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).
  • No new crash vector: the manual escape never throws, unlike encodeURIComponent, which throws URIError on lone surrogates.

other-unsafe-object-key (src/utility/findPlayedEpisodes.ts)

findPlayedEpisodesInFeeds grouped played episodes onto a plain object {} keyed by the feed-controlled podcastName. For "__proto__"/"constructor", acc[name] resolved to an inherited prototype member (truthy, no .push), throwing a TypeError and 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 in episodeStatus.ts (getPlayedEpisodeRecordKey) and episodeListEntry.ts. To keep this change scoped to the finding's files, those were left as-is: for the common case they remain byte-identical to getEpisodeKey; 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

  • New + extended unit tests for injectivity (incl. the finding's exact pairs), backward-compat byte-identity, escaped-vs-legacy disjointness, lone-surrogate no-throw, and __proto__/constructor grouping safety.
  • Full gates green: lint, format:check, typecheck, build, 784 tests.
  • Loaded the built plugin in an isolated Obsidian runtime: plugin live, no captured errors.

deepsec slugs: other-key-collision, other-unsafe-object-key

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
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying podnotes with  Cloudflare Pages  Cloudflare Pages

Latest commit: 440d91c
Status: ✅  Deploy successful!
Preview URL: https://9cfd382a.podnotes.pages.dev
Branch Preview URL: https://chhoumann-deepsec-key-safety.podnotes.pages.dev

View logs

@chhoumann
chhoumann marked this pull request as ready for review June 29, 2026 07:29

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/utility/episodeKey.ts
// 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)}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chhoumann
chhoumann merged commit a5683db into master Jun 29, 2026
3 checks passed
@chhoumann
chhoumann deleted the chhoumann/deepsec-key-safety branch June 29, 2026 10:01
github-actions Bot pushed a commit that referenced this pull request Jul 9, 2026
## [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))
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.17.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant