Repository navigation
fix(conformance): reject Symbol-keyed object properties in the interop cross-check (LAB-8754) - #191
Conversation
…p cross-check The cross-check listed a plain object's keys with Object.keys, which skips Symbol keys, so an object with an enumerable Symbol key encoded as if the key were absent. IOP-8 forbids silently skipping a value and IOP-13 requires a non-string key to be rejected. The plain-object branch now rejects an own enumerable Symbol key, and nonStringKeySelftest pins it. IOP-13's gap note no longer claims JavaScript objects carry string keys only.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
Summary by CodeRabbit
WalkthroughThe interop cross-check now rejects plain objects with own enumerable Symbol keys. A new self-test covers top-level and nested keys, as well as non-enumerable Symbols. The IOP-13 records and changelog describe the updated checks; published vectors are reported unchanged. ChangesSymbol-key validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The changelog links work after collection into CHANGELOG.md, and no merge-blocking issue remains. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely identifies the main change: rejecting Symbol-keyed object properties in the interop cross-check. |
| Description check | |
The description directly explains the defect, implementation, self-tests, conformance updates, and verification results. It is fully related to the changeset. |
| Docstring Coverage | |
Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (3 skipped: 3 u… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
📝 Generate docstrings
-
-
- Commit to this branch
- Create a new PR
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
What
tools/interop-crosscheck.mjslisted a plain object's keys withObject.keys, which skips Symbol keys. So[{ [Symbol('k')]: 1n, a: 2n }]encoded to9181a16102, the same bytes as[{ a: 2n }], and[{ [Symbol('k')]: 1n }]encoded as an empty map. IOP-8 says a value outside the data model must be rejected, never skipped, and IOP-13 says a non-string map key is an error.encodeCanonicalnow throwsOutOfModelErrorwhen the object has an own enumerable Symbol-keyed property, before it encodes any key.propertyIsEnumerableis called throughObject.prototype, because a null-prototype object has no such method. A non-enumerable Symbol key is ignored, as a non-enumerable string key already is.nonStringKeySelftest, the counterpart of the Python reference's_non_string_key_selftest: rejection at the top level, on a null-prototype object, in a nested map, in a list and in a set element, plus two controls (a non-enumerable Symbol key encodes to9181a16102;[{ "1": "a" }]encodes to9181a131a161).outOfModelSelftestno longer says a JavaScript non-string map key needs a Map.conformance/requirements.json: IOP-13 cites the new self-test, and its gap note no longer says JavaScript objects have string keys only.conformance/coverage.mdregenerated withtools/conformance.py report.No spec text, vector or reference-implementation change. Portable JSON cannot carry a Symbol key, so a self-test is the only way to pin this.
Verification
node tools/interop-crosscheck.mjsprints the same line asmain:OK: 44 key, 6 value, 6 reader accept, 1 reader reject, 1 AAD, 1 encryption, 34 error vectors verified independently.FAIL nonStringKeySelftest (...)lines. With the enumerable filter removed, it exits 1 on the non-enumerable control.python3 tools/conformance.py checkOK;python3 tools/test_conformance.py: all 284 cases pass. IOP-13 stayspartial (gap)for all three SDKs.Closes LAB-8754