Conversation
still not sure if this is a good path fprward
🦋 Changeset detectedLatest commit: 9270bf7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } else if (typeof oldVal === "string" && typeof newVal === "string") { | ||
| accumulator[key] = oldVal + newVal; | ||
| } else { | ||
| accumulator[key] = newVal; |
Check warning
Code scanning / CodeQL
Prototype-polluting function Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 days ago
In general, to fix prototype pollution in deep merge / accumulate helpers, you should prevent assignments to dangerous prototype-related keys (__proto__, constructor, and optionally prototype) and/or only recurse into properties that are own properties of the destination object. This preserves existing semantics for normal keys while blocking the vectors used to reach Object.prototype or constructor prototypes.
For this specific code, the minimal-impact fix is to introduce a small helper like isSafeKey and use it to skip dangerous keys both at the top-level merge loop (for (const key of Object.keys(item))) and when recursing into nested objects via deepMergeStream. This avoids prototype mutation while keeping the existing merge behavior (arrays, objects, strings, overwrite) for all legitimate keys. We should implement isSafeKey directly in accumulate.ts above deepMergeStream, and add checks:
- At line 17–18: skip any key that is not safe before reading
item[key]. - Before any recursive call to
deepMergeStreamonoldElem/newElem(array branch) andoldVal/newVal(object branch), rely on the top-level key check to ensure the key is safe; nested recursion works on plain objects that are now free from dangerous keys at each level. - Also ensure we don’t assign
accumulator[key] = newValoraccumulator[key] = oldVal + newValfor unsafe keys.
The simplest way to implement this while touching as little code as possible is:
- Add:
function isSafeKey(key: string): boolean {
return key !== "__proto__" && key !== "constructor" && key !== "prototype";
}- At the start of the
for (const key of Object.keys(item))loop, immediatelycontinueif!isSafeKey(key).
Because all further uses of key in this function are in that loop, this single guard prevents any writes under dangerous keys and, transitively, prevents nested dangerous keys from being merged as well.
| @@ -10,11 +10,16 @@ | ||
| * - Both strings → concatenate | ||
| * - Otherwise → overwrite with the new value | ||
| */ | ||
| function isSafeKey(key: string): boolean { | ||
| return key !== "__proto__" && key !== "constructor" && key !== "prototype"; | ||
| } | ||
|
|
||
| function deepMergeStream( | ||
| accumulator: Record<string, unknown>, | ||
| item: Record<string, unknown>, | ||
| ): void { | ||
| for (const key of Object.keys(item)) { | ||
| if (!isSafeKey(key)) continue; | ||
| const newVal = item[key]; | ||
| if (newVal === undefined || newVal === null) continue; | ||
| const oldVal = accumulator[key]; |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bridge | 9270bf7 | Commit Preview URL Branch Preview URL |
Mar 10 2026, 09:07 AM |
|
Bencher Report| Branch | stream |
| Testbed | ubuntu-latest |
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
🚨 22 Alerts
🐰 View full continuous benchmarking report in Bencher
1 participant
o <- data[] as {}- can be a async generatorPlayground: https://stream-bridge.aarne-laur.workers.dev/