Potential fix for code scanning alert no. 46: Prototype-polluting assignment#1357
Conversation
…ignment Most JavaScript objects inherit the properties of the built-in Object.prototype object. Prototype pollution is a type of vulnerability in which an attacker can modify Object.prototype. Since most objects inherit from the compromised Object.prototype object, the attacker can use this to tamper with the application logic, and often escalate to remote code execution or cross-site scripting. Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Steve Scargall <37674041+sscargal@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Hardens the TS client’s semantic-memory formatting against prototype pollution by switching the intermediate “structured” accumulator (and nested tag buckets) to prototype-less objects.
Changes:
- Use
Object.create(null)for the top-levelstructureddictionary informatSemanticMemories. - Use
Object.create(null)for each nestedstructured[feature.tag]bucket to prevent dangerous keys from mutating prototypes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const structured = Object.create(null) as Record<string, Record<string, string>> | ||
| for (const feature of features) { | ||
| if (!(feature.tag in structured)) { | ||
| structured[feature.tag] = {} | ||
| structured[feature.tag] = Object.create(null) as Record<string, string> | ||
| } |
There was a problem hiding this comment.
This change is addressing prototype-pollution hardening, but the existing unit tests for formatSemanticMemories don't appear to cover the new security-relevant behavior (e.g., tags/feature names like __proto__, constructor, or prototype). Adding a regression test for these keys would help ensure the alert fix doesn't regress and that the output remains safe to consume.
| const structured = Object.create(null) as Record<string, Record<string, string>> | ||
| for (const feature of features) { | ||
| if (!(feature.tag in structured)) { | ||
| structured[feature.tag] = {} | ||
| structured[feature.tag] = Object.create(null) as Record<string, string> |
There was a problem hiding this comment.
The as Record<...> assertions here are avoidable and reduce type-safety (type assertions can hide mismatches). Consider keeping an explicit variable type annotation instead (e.g., const structured: Record<...> = Object.create(null) and similarly for the nested bucket) so TypeScript checks the value without needing as.
…ignment (MemMachine#1357) Most JavaScript objects inherit the properties of the built-in Object.prototype object. Prototype pollution is a type of vulnerability in which an attacker can modify Object.prototype. Since most objects inherit from the compromised Object.prototype object, the attacker can use this to tamper with the application logic, and often escalate to remote code execution or cross-site scripting. Signed-off-by: Steve Scargall <37674041+sscargal@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Potential fix for https://github.com/MemMachine/MemMachine/security/code-scanning/46
Use a prototype-less dictionary for
structuredand for each nested tag bucket, so dangerous keys like__proto__are treated as ordinary data keys rather than prototype mutators.Best fix in this file: in
packages/ts-client/src/memory/format.ts, updateformatSemanticMemoriesto:structuredwithObject.create(null)(typed asRecord<string, Record<string, string>>);structured[feature.tag]bucket withObject.create(null)as well.This preserves existing functionality and output shape (
JSON.stringifystill produces the same JSON structure), while removing prototype-chain pollution behavior. No new imports or dependencies are required.Suggested fixes powered by Copilot Autofix. Review carefully before merging.