Stop json/plain from allocating arbitrary classes (0.0.7) - #359
Merged
d2army merged 12 commits intoSep 3, 2026
Conversation
Reject arbitrary ^o instantiation while keeping Request/Response payloads and error serialization v2 working. Co-authored-by: Cursor <cursoragent@cursor.com>
Unconstrained bundle install was pulling protobuf 4, which cannot load the generated stubs, and ubuntu-latest no longer ships docker-compose. Co-authored-by: Cursor <cursoragent@cursor.com>
docker compose now starts, but auto-setup is not ready after a 10s sleep, so register_namespace hits connection refused. Co-authored-by: Cursor <cursoragent@cursor.com>
auto-setup: latest never bound gRPC with Cassandra 3.11 on GitHub runners. Dump compose logs if the wait still times out. Co-authored-by: Cursor <cursoragent@cursor.com>
Example workflows return Metadata::Workflow and Struct.new results. Those are first-party Oj encodings, not gadget classes. Co-authored-by: Cursor <cursoragent@cursor.com>
Oj puts ^o Thread::Backtrace on ~bt_locations. Rename the unit file to json_spec.rb so CI actually runs it. Co-authored-by: Cursor <cursoragent@cursor.com>
…trips. Oj::Saj rejects duplicate hash keys so JSON.parse and Oj.load cannot disagree on ^o. Allow Date/DateTime/Rational via ^O, widen InputDeserializer rescue for JSON::ParserError, and add regression specs. Co-authored-by: Cursor <cursoragent@cursor.com>
Record hash keys on Saj container entry, cap nesting in Saj and JSON.parse, walk directive trees iteratively, skip autoload stubs during validation, and declare google-protobuf ~> 3.25. Add regression specs for container duplicate keys and depth-5000 rejection. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove the 256-character hash key limit pending replay validation, freeze error message constants, document ^c usage for Exception ivars, and add scalar-first duplicate, sibling-key control, and MAX_NESTING boundary tests. Co-authored-by: Cursor <cursoragent@cursor.com>
google-protobuf ~> 3.25 is declared in temporal.gemspec; drop the extra root and examples Gemfile constraints. Remove the depth-5000 example now covered by the MAX_NESTING boundary tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Spell out what still works and what can break. Comment why Saj records container keys, why parse order is Saj then JSON then Oj, why ^c stays broad, and why InputDeserializer also rescues JSON::ParserError. Co-authored-by: Cursor <cursoragent@cursor.com>
File-level deserialize order on json.rb; why-comments on the new error, protobuf 3 pin, CI trigger, compose image pins, and spec groupings. Co-authored-by: Cursor <cursoragent@cursor.com>
d2army
force-pushed
the
ian-yap/secbugs-174-fail-closed-json-plain
branch
from
September 3, 2026 05:33
8703d60 to
58d40e5
Compare
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Oj object mode can allocate any Ruby class named in a
json/plainpayload. This PR allowlists what may round-trip, then loads the original bytes so existing encoded history still works. Encoding name staysjson/plain. Version is 0.0.7.deserializeruns three steps, in this order:Oj::Saj): reject duplicate object keys and nesting deeper than 512. Saj has to run first.JSON.parsekeeps the last duplicate key; Oj binds^oon the first, so a discarded object or array can still allocate a class.JSON.parsetree: only loadedTemporal::types, loaded::Request/::Response, loaded Exception subclasses, Date-like types, anonymous Structs, andTemporal::JSON.allow_class.Oj.loadof the original bytes: keep Time (^t) and symbol keys.Do not tighten
^c. It returns a Class object (not an instance) when an exception ivar holds a class.How to review
Start here:
lib/temporal/json.rb— whole guard. File header is the map. ThenPayloadStructureValidator#note(why keys are recorded on hash and array start), thenallowed_*/allow_class.lib/temporal/errors.rb—JSONDisallowedClassError.lib/temporal/concerns/input_deserializer.rb— also rescueJSON::ParserErrorso newline-split Go-client input still works. Do not swallowJSONDisallowedClassError.Then, only if you are checking packaging or CI:
temporal.gemspec—google-protobuf ~> 3.25because generated stubs do not load on protobuf 4..github/workflows/tests.yml— CI now runs ontransfers-masterPRs (it did not before).docker composev2, wait-for-7233, and image pins are so example CI actually starts Temporal.spec/unit/lib/temporal/json_spec.rb— first-party round-trips that broke on the first fail-closed pass, then attack/duplicate-key regressions.Test plan
bundle exec rspec spec/unit(643 examples, 0 failures)Gem::Requirement,Kernel) and duplicate keys (both value orderings, including^u) raise beforeOj.loadjson/plainencoding nametest_gemandtest_examplestemporal-ruby0.0.7 instead of a git SHA