Skip to content

fix: harden codexcli-mcp converters against prototype pollution and add regression tests#1630

Open
dyoshikawa-claw wants to merge 2 commits into
mainfrom
fix/1626-harden-codexcli-mcp-removeEmptyEntries
Open

fix: harden codexcli-mcp converters against prototype pollution and add regression tests#1630
dyoshikawa-claw wants to merge 2 commits into
mainfrom
fix/1626-harden-codexcli-mcp-removeEmptyEntries

Conversation

@dyoshikawa-claw
Copy link
Copy Markdown
Collaborator

Summary

Closes #1626

Security hardening

  • Skip __proto__ / constructor / prototype keys in removeEmptyEntries, convertToCodexFormat, and convertFromCodexFormat to prevent prototype pollution
  • Add isPlainObject guard to replace broad typeof === 'object' check, avoiding misclassification of Date/Map/Set as empty plain objects
  • Add isRecord type guard helper to replace as-casts and eslint-disable comments

Robustness

  • Add recursion depth cap (MAX_REMOVE_EMPTY_ENTRIES_DEPTH = 32) to prevent stack overflow on deeply nested input
  • Warn when a server entry collapses to empty after removeEmptyEntries

Test coverage

  • Add TOML-string-level regression test asserting getFileContent() never contains .env] for server with env: {}
  • Add regression test for arrays-of-objects not recursed into (args: [{}])
  • Tighten test assertions: replace optional chaining with non-null assertion after expect().toBeDefined()

Related

root added 2 commits May 12, 2026 14:16
…dd regression tests

- Add PROTOTYPE_POLLUTION_KEYS skip for __proto__/constructor/prototype in
  removeEmptyEntries, convertToCodexFormat, and convertFromCodexFormat
- Add isPlainObject guard to replace broad object type check, avoiding
  misclassification of Date/Map/Set as empty plain objects
- Add isRecord type guard helper to replace as-casts
- Add MAX_REMOVE_EMPTY_ENTRIES_DEPTH (32) recursion depth cap
- Warn when a server entry collapses to empty after removeEmptyEntries
- Add TOML-string-level regression test asserting getFileContent() never
  contains .env] for server with env: {}
- Add regression test for arrays-of-objects not recursed into
- Tighten test assertions: replace optional chaining with non-null
  assertion after expect().toBeDefined()

Closes #1626
…ing, and improve depth warning

- Import isRecord from shared utils/type-guards.js instead of local definition
- Add PROTOTYPE_POLLUTION_KEYS guard for server names in both converters
- Use Object.hasOwn instead of 'in' operator for accurate warning check
- Warn when removeEmptyEntries depth cap is exceeded
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden codexcli-mcp removeEmptyEntries: prototype pollution, recursion depth, and test coverage gaps

1 participant