From 76bef51b2332788dcc6070216743e658215e4e6a Mon Sep 17 00:00:00 2001 From: d2army Date: Wed, 2 Sep 2026 22:36:01 -0700 Subject: [PATCH 1/2] Stop json/plain from allocating arbitrary classes (0.0.7) Oj object mode can allocate any Ruby class named in a json/plain payload. Allowlist what may round-trip, reject duplicate object keys and excessive nesting, then Oj.load the original bytes so existing encoded history still works. Encoding name stays json/plain. Runtime depends on google-protobuf ~> 3.25. CI now runs on transfers-master. --- .github/workflows/tests.yml | 39 +- CHANGELOG.md | 10 + examples/docker-compose.yml | 7 +- lib/temporal/concerns/input_deserializer.rb | 4 +- lib/temporal/errors.rb | 3 + lib/temporal/json.rb | 278 +++++++++++++- lib/temporal/version.rb | 2 +- .../connection/converter/payload/json_spec.rb | 5 + spec/unit/lib/temporal/json.rb | 26 -- spec/unit/lib/temporal/json_spec.rb | 359 ++++++++++++++++++ temporal.gemspec | 2 + 11 files changed, 694 insertions(+), 41 deletions(-) delete mode 100644 spec/unit/lib/temporal/json.rb create mode 100644 spec/unit/lib/temporal/json_spec.rb diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2456cb5d9..f35c4f8de 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,9 +2,10 @@ name: Tests on: push: - branches: [ "master" ] + # This fork ships from transfers-master. Without it, PRs to that branch had no CI. + branches: [ "master", "transfers-master" ] pull_request: - branches: [ "master" ] + branches: [ "master", "transfers-master" ] jobs: test_gem: @@ -35,9 +36,10 @@ jobs: steps: - uses: actions/checkout@v3 + # ubuntu-latest no longer ships the docker-compose v1 binary. - name: Start dependencies run: | - docker-compose \ + docker compose \ -f examples/docker-compose.yml \ up -d @@ -50,13 +52,31 @@ jobs: run: | cd examples && bundle install --path vendor/bundle - - name: Wait for dependencies to settle + # auto-setup is not ready after a fixed sleep; wait until gRPC accepts connections. + - name: Wait for Temporal run: | - sleep 10 - + if timeout 300 bash -c 'until echo >/dev/tcp/127.0.0.1/7233; do sleep 2; done'; then + exit 0 + fi + echo "Temporal did not listen on 7233" + docker compose -f examples/docker-compose.yml ps -a + docker compose -f examples/docker-compose.yml logs --no-color --tail 400 + exit 1 + + # register_namespace can still race the server after 7233 is open. - name: Register namespace run: | - cd examples && bin/register_namespace ruby-samples + cd examples + for i in $(seq 1 30); do + if bin/register_namespace ruby-samples; then + exit 0 + fi + echo "register_namespace failed, retry ${i}" + sleep 5 + done + docker compose -f docker-compose.yml ps + docker compose -f docker-compose.yml logs --tail 200 + exit 1 - name: Wait for namespace to settle run: | @@ -78,6 +98,11 @@ jobs: run: | cd examples && bin/worker & + # Workers are started in the background; they are not ready at process spawn. + - name: Wait for workers + run: | + sleep 10 + - name: Run RSpec env: USE_ERROR_SERIALIZATION_V2: 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index cd9849848..477d8eb19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## 0.0.7 + +- `json/plain` no longer builds arbitrary Ruby classes from encoded payloads. That was possible because Oj object mode can allocate any constant named in the payload. +- Still round-trips first-party shapes: activity `Request` / `Response`, `Temporal::` types, Exception subclasses (including backtraces), `Date` / `DateTime` / `Rational`, anonymous Structs, and classes registered with `Temporal::JSON.allow_class`. +- Rejects duplicate JSON object keys. Oj and `JSON.parse` disagree on duplicates, so a discarded value could still allocate a class. +- Rejects constants that are only pending `autoload` until they are actually loaded. +- Rejects JSON nested deeper than 512 levels. +- Encoding name stays `json/plain`. +- Runtime depends on `google-protobuf` ~> 3.25 (generated stubs under `lib/gen/` require protobuf 3). + ## 0.0.6 - Defer gRPC loading via autoload for fork safety diff --git a/examples/docker-compose.yml b/examples/docker-compose.yml index 4bff724c6..f37c947f1 100644 --- a/examples/docker-compose.yml +++ b/examples/docker-compose.yml @@ -1,8 +1,9 @@ version: '3.5' +# Pin images. auto-setup:latest never bound gRPC :7233 with Cassandra 3.11 on GitHub runners. services: temporal: - image: temporalio/auto-setup:latest + image: temporalio/auto-setup:1.22.4 ports: - "7233:7233" environment: @@ -14,7 +15,7 @@ services: - cassandra temporal-web: - image: temporalio/web:latest + image: temporalio/web:1.15.0 environment: - "TEMPORAL_GRPC_ENDPOINT=temporal:7233" ports: @@ -23,6 +24,6 @@ services: - temporal cassandra: - image: cassandra:3.11 + image: cassandra:3.11.16 ports: - "9042:9042" diff --git a/lib/temporal/concerns/input_deserializer.rb b/lib/temporal/concerns/input_deserializer.rb index 8f636df8e..f3dbba746 100644 --- a/lib/temporal/concerns/input_deserializer.rb +++ b/lib/temporal/concerns/input_deserializer.rb @@ -3,7 +3,9 @@ module Concerns module InputDeserializer def deserialize(input) JSON.deserialize(input) - rescue Oj::ParseError + rescue Oj::ParseError, ::JSON::ParserError + # JSON.parse now runs before Oj.load, so newline-split Go-client input raises + # JSON::ParserError instead of Oj::ParseError. Do not rescue JSONDisallowedClassError. # Copied over from the Cadence side, similar situation happening with Temporal # # cadence official go-client serializes / deserializes input in a different format than this ruby client diff --git a/lib/temporal/errors.rb b/lib/temporal/errors.rb index 7aa114056..c3b8b2321 100644 --- a/lib/temporal/errors.rb +++ b/lib/temporal/errors.rb @@ -2,6 +2,9 @@ module Temporal # Superclass for all Temporal errors class Error < StandardError; end + # Raised when json/plain asks Oj to allocate a class that is not allowlisted. + class JSONDisallowedClassError < Error; end + # Superclass for errors specific to Temporal worker itself class InternalError < Error; end diff --git a/lib/temporal/json.rb b/lib/temporal/json.rb index 9784d1ead..1a7804b7e 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -1,20 +1,292 @@ -# Helper class for serializing/deserializing JSON +# json/plain codec for Temporal payloads. +# +# deserialize: +# 1. PayloadStructureValidator (Oj::Saj): reject duplicate keys and nesting > 512 +# 2. assert_safe! on JSON.parse: allowlist ^o / ^O / ^c / ^u class directives +# 3. Oj.load original bytes: keep Time (^t) and symbol keys +require 'json' require 'oj' +require 'set' +require 'temporal/errors' module Temporal module JSON OJ_OPTIONS = { mode: :object, - # use ruby's built-in serialization. If nil, OJ seems to default to ~15 decimal places of precision + # use ruby's built-in serialization. If nil, Oj seems to default to ~15 decimal places of precision float_precision: 0 }.freeze + MAX_NESTING = 512 + MAX_CLASS_NAME_LENGTH = 256 + + # ^o allocates instances. ^O encodes Date, DateTime, and Rational. + # ^c / ^C look up Class objects. Oj emits ^c when an Exception ivar holds a Class + # (see spec/unit/lib/temporal/connection/serializer/failure_spec.rb). + INSTANCE_DIRECTIVE_KEYS = %w[^o].freeze + ODD_MARSHALLER_KEYS = %w[^O].freeze + CLASS_REFERENCE_DIRECTIVE_KEYS = %w[^c ^C].freeze + STRUCT_DIRECTIVE_KEYS = %w[^u].freeze + ALLOWED_CLASS_SUFFIXES = %w[::Request ::Response].freeze + ALLOWED_ODD_CLASSES = %w[Date DateTime Rational].freeze + # Oj dumps these on a raised Exception (~bt_locations). They are not gadget classes. + ALLOWED_STDLIB_CLASSES = %w[ + Thread::Backtrace + Thread::Backtrace::Location + ].freeze + + ALLOWED_CLASSES = Set.new + ALLOWED_CLASSES_MUTEX = Mutex.new + private_constant :ALLOWED_CLASSES, :ALLOWED_CLASSES_MUTEX + + DUPLICATE_KEY_ERROR = 'json/plain payload contains duplicate hash key'.freeze + NESTING_DEPTH_ERROR = 'json/plain payload exceeds maximum nesting depth'.freeze + + # Walks raw bytes before JSON.parse / Oj.load. JSON.parse keeps the last duplicate + # key; Oj binds ^o on the first. A discarded object or array value can still allocate + # a class, so every hash key is recorded on scalar, object, and array entry. + class PayloadStructureValidator < Oj::Saj + def initialize + @hash_key_sets = [] + @depth = 0 + end + + def hash_start(key) + note(key) + bump_depth! + @hash_key_sets << Set.new + end + + def hash_end(_key) + @hash_key_sets.pop + @depth -= 1 + end + + def array_start(key) + note(key) + bump_depth! + end + + def array_end(_key) + @depth -= 1 + end + + def add_value(_value, key) + note(key) + end + + private + + def bump_depth! + @depth += 1 + return if @depth <= Temporal::JSON::MAX_NESTING + + raise Temporal::JSONDisallowedClassError, Temporal::JSON::NESTING_DEPTH_ERROR + end + + # Saj calls add_value for scalars and hash_start / array_start for containers. + # Skipping container entry is how duplicate ^u and duplicate "scope" slipped through. + def note(key) + return if key.nil? + + keys = @hash_key_sets.last + return if keys.nil? + + if keys.include?(key) + raise Temporal::JSONDisallowedClassError, Temporal::JSON::DUPLICATE_KEY_ERROR + end + + keys << key + end + end + private_constant :PayloadStructureValidator + def self.serialize(value) Oj.dump(value, OJ_OPTIONS) end + # Order matters: Saj first (duplicate keys / depth), then JSON.parse (allowlist on a + # collapsed tree), then Oj.load of the original bytes so ^t and symbol keys survive. def self.deserialize(value) - Oj.load(value.to_s, OJ_OPTIONS) + return nil if value.nil? + + raw = value.to_s + return nil if raw.empty? + + assert_payload_structure!(raw) + assert_safe!(::JSON.parse(raw, max_nesting: MAX_NESTING)) + Oj.load(raw, OJ_OPTIONS) + end + + # Register an extra class name for json/plain object reconstitution. Use this for + # application types that are not ::Request / ::Response, Temporal::, or Exception. + def self.allow_class(name) + with_allowed_classes { |set| set.add(name.to_s) } + name.to_s + end + + def self.allowed_class_names + with_allowed_classes(&:dup) + end + + def self.with_allowed_classes + ALLOWED_CLASSES_MUTEX.synchronize { yield ALLOWED_CLASSES } + end + private_class_method :with_allowed_classes + + def self.assert_payload_structure!(raw) + Oj.saj_parse(PayloadStructureValidator.new, raw) + end + private_class_method :assert_payload_structure! + + def self.assert_safe!(obj) + stack = [obj] + until stack.empty? + current = stack.pop + case current + when Hash + validate_hash_directives!(current) + current.each_value { |v| stack << v unless v.nil? } + when Array + current.each { |v| stack << v unless v.nil? } + end + end + end + private_class_method :assert_safe! + + def self.validate_hash_directives!(obj) + STRUCT_DIRECTIVE_KEYS.each do |key| + next unless obj.key?(key) + next if anonymous_struct_directive?(obj[key]) + + name = class_name_from_directive(obj[key]) + unless name && allowed_struct_class?(name) + raise Temporal::JSONDisallowedClassError, + "json/plain payload requested disallowed class #{safe_class_label(name)}" + end + end + + INSTANCE_DIRECTIVE_KEYS.each do |key| + next unless obj.key?(key) + + name = class_name_from_directive(obj[key]) + unless name && allowed_instance_class?(name) + raise Temporal::JSONDisallowedClassError, + "json/plain payload requested disallowed class #{safe_class_label(name)}" + end + end + + ODD_MARSHALLER_KEYS.each do |key| + next unless obj.key?(key) + + name = class_name_from_directive(obj[key]) + unless name && allowed_odd_class?(name) + raise Temporal::JSONDisallowedClassError, + "json/plain payload requested disallowed class #{safe_class_label(name)}" + end + end + + CLASS_REFERENCE_DIRECTIVE_KEYS.each do |key| + next unless obj.key?(key) + + name = class_name_from_directive(obj[key]) + unless name && allowed_class_reference?(name) + raise Temporal::JSONDisallowedClassError, + "json/plain payload requested disallowed class #{safe_class_label(name)}" + end + end + end + private_class_method :validate_hash_directives! + + def self.class_name_from_directive(value) + case value + when String + value + when Array + value.first if value.first.is_a?(String) + end + end + private_class_method :class_name_from_directive + + def self.safe_class_label(name) + return '?' unless valid_constant_name?(name) + + name + end + private_class_method :safe_class_label + + def self.valid_constant_name?(name) + return false unless name.is_a?(String) + return false if name.empty? || name.length > MAX_CLASS_NAME_LENGTH + + name.split('::').all? { |part| part.match?(/\A[A-Z]\w*\z/) } + end + private_class_method :valid_constant_name? + + def self.registered_class?(name) + with_allowed_classes { |set| set.include?(name) } + end + private_class_method :registered_class? + + def self.allowed_instance_class?(name) + return false unless valid_constant_name?(name) + return true if registered_class?(name) + return true if library_class?(name) + return true if ALLOWED_STDLIB_CLASSES.include?(name) && resolve_constant(name) + return true if ALLOWED_CLASS_SUFFIXES.any? { |suffix| name.end_with?(suffix) } && resolve_constant(name) + + klass = resolve_constant(name) + klass.is_a?(Class) && klass <= Exception + end + private_class_method :allowed_instance_class? + + def self.allowed_odd_class?(name) + ALLOWED_ODD_CLASSES.include?(name) && resolve_constant(name) + end + private_class_method :allowed_odd_class? + + def self.allowed_struct_class?(name) + return false unless valid_constant_name?(name) + + registered_class?(name) || library_class?(name) + end + private_class_method :allowed_struct_class? + + def self.library_class?(name) + name.start_with?('Temporal::') && resolve_constant(name).is_a?(Class) + end + private_class_method :library_class? + + # Oj encodes Struct.new(:a, :b).new(...) as ^u with member names, not a class. + def self.anonymous_struct_directive?(value) + value.is_a?(Array) && value.first.is_a?(Array) && value.first.all? { |member| member.is_a?(String) } + end + private_class_method :anonymous_struct_directive? + + # ^c reconstitutes a Class object, not an instance. Tightening this to Temporal:: / + # Exception / allow_class would break error v2 when an ivar holds an app class. + def self.allowed_class_reference?(name) + return false unless valid_constant_name?(name) + return true if registered_class?(name) + + resolve_constant(name).is_a?(Module) + end + private_class_method :allowed_class_reference? + + # Only resolve constants that are already loaded. const_get would trigger autoload. + def self.resolve_constant(name) + return nil unless valid_constant_name?(name) + + name.split('::').reduce(Object) do |mod, part| + return nil unless mod.is_a?(Module) + return nil if mod.autoload?(part) + return nil unless mod.const_defined?(part, false) + + mod.const_get(part, false) + end + rescue NameError + nil end + private_class_method :resolve_constant end end diff --git a/lib/temporal/version.rb b/lib/temporal/version.rb index 50451bd9b..5bdc1ec74 100644 --- a/lib/temporal/version.rb +++ b/lib/temporal/version.rb @@ -1,3 +1,3 @@ module Temporal - VERSION = '0.0.6'.freeze + VERSION = '0.0.7'.freeze end diff --git a/spec/unit/lib/temporal/connection/converter/payload/json_spec.rb b/spec/unit/lib/temporal/connection/converter/payload/json_spec.rb index ebdb6ce4e..f55f2cce5 100644 --- a/spec/unit/lib/temporal/connection/converter/payload/json_spec.rb +++ b/spec/unit/lib/temporal/connection/converter/payload/json_spec.rb @@ -3,6 +3,11 @@ describe Temporal::Connection::Converter::Payload::JSON do subject { described_class.new } + it 'keeps the json/plain encoding name' do + # Temporal stores this string on historical payloads. Renaming it would skip decode. + expect(subject.encoding).to eq('json/plain') + end + describe 'round trip' do it 'safely handles non-ASCII encodable UTF characters' do input = { 'one' => 'one', two: :two, ':three' => '☻' } diff --git a/spec/unit/lib/temporal/json.rb b/spec/unit/lib/temporal/json.rb deleted file mode 100644 index c2d00cd4d..000000000 --- a/spec/unit/lib/temporal/json.rb +++ /dev/null @@ -1,26 +0,0 @@ -require 'temporal/json' - -describe Temporal::JSON do - let(:hash) { { 'one' => 'one', two: :two, ':three' => ':three' } } - let(:json) { '{"one":"one",":two":":two","\u003athree":"\u003athree"}' } - - describe '.serialize' do - it 'generates JSON string' do - expect(described_class.serialize(hash)).to eq(json) - end - end - - describe '.deserialize' do - it 'parses JSON string' do - expect(described_class.deserialize(json)).to eq(hash) - end - - it 'parses empty string to nil' do - expect(described_class.deserialize('')).to eq(nil) - end - - it 'parses nil' do - expect(described_class.deserialize(nil)).to eq(nil) - end - end -end diff --git a/spec/unit/lib/temporal/json_spec.rb b/spec/unit/lib/temporal/json_spec.rb new file mode 100644 index 000000000..463eb1b9d --- /dev/null +++ b/spec/unit/lib/temporal/json_spec.rb @@ -0,0 +1,359 @@ +require 'temporal/json' +require 'temporal/concerns/input_deserializer' + +module TemporalJSONSpecFixtures + module DummyActivity + class Request + attr_accessor :scope + + def initialize(scope: nil) + @scope = scope + end + end + + class Response + attr_accessor :count + + def initialize(count: nil) + @count = count + end + end + end + + class DummyWidget + attr_accessor :name + + def initialize(name: nil) + @name = name + end + end + + class DummyError < StandardError + attr_reader :code + + def initialize(message = nil, code: nil) + super(message) + @code = code + end + end +end + +describe Temporal::JSON do + let(:hash) { { 'one' => 'one', two: :two, ':three' => ':three' } } + let(:json) { '{"one":"one",":two":":two","\u003athree":"\u003athree"}' } + + around do |example| + snapshot = described_class.allowed_class_names + example.run + ensure + described_class.send(:with_allowed_classes) { |set| set.replace(snapshot) } + end + + describe '.serialize' do + it 'generates JSON string' do + expect(described_class.serialize(hash)).to eq(json) + end + end + + describe '.deserialize' do + it 'parses JSON string' do + expect(described_class.deserialize(json)).to eq(hash) + end + + it 'parses empty string to nil' do + expect(described_class.deserialize('')).to eq(nil) + end + + it 'parses nil' do + expect(described_class.deserialize(nil)).to eq(nil) + end + + # First-party shapes that broke when deserialize became fail-closed. + it 'round-trips Time via ^t' do + time = Time.at(1_700_000_000) + loaded = described_class.deserialize(described_class.serialize(time)) + + expect(loaded).to be_a(Time) + expect(loaded.to_i).to eq(time.to_i) + end + + it 'reconstitutes a loaded ::Request from Go-style ^o JSON' do + payload = '{"^o":"TemporalJSONSpecFixtures::DummyActivity::Request","scope":"to_sync"}' + loaded = described_class.deserialize(payload) + + expect(loaded).to be_a(TemporalJSONSpecFixtures::DummyActivity::Request) + expect(loaded.scope).to eq('to_sync') + end + + it 'round-trips a loaded ::Request through serialize' do + request = TemporalJSONSpecFixtures::DummyActivity::Request.new(scope: 'to_sync') + loaded = described_class.deserialize(described_class.serialize(request)) + + expect(loaded).to be_a(TemporalJSONSpecFixtures::DummyActivity::Request) + expect(loaded.scope).to eq('to_sync') + end + + it 'round-trips a loaded ::Response through serialize' do + response = TemporalJSONSpecFixtures::DummyActivity::Response.new(count: 3) + loaded = described_class.deserialize(described_class.serialize(response)) + + expect(loaded).to be_a(TemporalJSONSpecFixtures::DummyActivity::Response) + expect(loaded.count).to eq(3) + end + + it 'reconstitutes nested Request objects' do + payload = '{"inner":{"^o":"TemporalJSONSpecFixtures::DummyActivity::Request","scope":"nested"}}' + loaded = described_class.deserialize(payload) + + expect(loaded['inner']).to be_a(TemporalJSONSpecFixtures::DummyActivity::Request) + expect(loaded['inner'].scope).to eq('nested') + end + + it 'round-trips a loaded Exception subclass' do + error = TemporalJSONSpecFixtures::DummyError.new('boom', code: 7) + loaded = described_class.deserialize(described_class.serialize(error)) + + expect(loaded).to be_a(TemporalJSONSpecFixtures::DummyError) + expect(loaded.message).to eq('boom') + expect(loaded.code).to eq(7) + end + + it 'round-trips a raised Exception including backtrace locations' do + begin + raise TemporalJSONSpecFixtures::DummyError.new('boom', code: 7) + rescue TemporalJSONSpecFixtures::DummyError => error + loaded = described_class.deserialize(described_class.serialize(error)) + + expect(loaded).to be_a(TemporalJSONSpecFixtures::DummyError) + expect(loaded.message).to eq('boom') + expect(loaded.code).to eq(7) + end + end + + it 'reconstitutes a loaded class reference via ^c' do + expect(described_class.deserialize('{"^c":"String"}')).to eq(String) + end + + # Attack and structure-guard regressions. Oj.load must not run. + it 'rejects ^c for an unloaded constant before Oj.load' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"^c":"DefinitelyNotAClassXYZ"}') + end.to raise_error(Temporal::JSONDisallowedClassError, /DefinitelyNotAClassXYZ/) + end + + it 'rejects Gem::Requirement gadget payloads before Oj.load' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"^o":"Gem::Requirement"}') + end.to raise_error(Temporal::JSONDisallowedClassError, /Gem::Requirement/) + end + + it 'rejects duplicate ^o keys before Oj.load' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize( + '{"^o":"Gem::Requirement","^o":"TemporalJSONSpecFixtures::DummyActivity::Request"}' + ) + end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate hash key/) + end + + it 'rejects duplicate ^u keys before Oj.load' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"^u":["Gem::Requirement",1],"^u":[["a"],1]}') + end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate hash key/) + end + + it 'rejects duplicate scope when the first value is an object' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize( + '{"^o":"TemporalJSONSpecFixtures::DummyActivity::Request","scope":{"^o":"Gem::Requirement"},"scope":"safe"}' + ) + end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate hash key/) + end + + it 'rejects duplicate scope when the first value is a scalar' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"scope":"safe","scope":{"^o":"Gem::Requirement"}}') + end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate hash key/) + end + + it 'allows the same key in separate objects' do + loaded = described_class.deserialize('[{"scope":"a"},{"scope":"b"}]') + + expect(loaded).to eq([{ 'scope' => 'a' }, { 'scope' => 'b' }]) + end + + it 'accepts nesting at MAX_NESTING' do + depth = Temporal::JSON::MAX_NESTING + raw = '[' * depth + '1' + ']' * depth + + node = described_class.deserialize(raw) + depth.times { node = node.first } + expect(node).to eq(1) + end + + it 'rejects nesting one level above MAX_NESTING' do + depth = Temporal::JSON::MAX_NESTING + 1 + raw = '[' * depth + '1' + ']' * depth + + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize(raw) + end.to raise_error(Temporal::JSONDisallowedClassError, /maximum nesting depth/) + end + + it 'rejects duplicate scope when the first value is an array' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"scope":[1,2],"scope":"safe"}') + end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate hash key/) + end + + it 'rejects excessive nesting before Oj.load' do + depth = Temporal::JSON::MAX_NESTING + 2 + raw = '[' * depth + '1' + ']' * depth + + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize(raw) + end.to raise_error(Temporal::JSONDisallowedClassError, /maximum nesting depth/) + end + + it 'does not autoload constants while validating directives' do + path = File.join(Dir.tmpdir, "temporal_json_autoload_#{Process.pid}.rb") + File.write(path, "$TEMPORAL_JSON_AUTOLOADED = true\nmodule TemporalJSONAutoloadProbe; class Widget; end; end\n") + $LOAD_PATH.unshift(File.dirname(path)) + Object.autoload(:TemporalJSONAutoloadProbe, path.sub(/\.rb$/, '')) + + expect do + described_class.deserialize('{"^o":"TemporalJSONAutoloadProbe::Widget"}') + end.to raise_error(Temporal::JSONDisallowedClassError, /Widget/) + + expect(defined?($TEMPORAL_JSON_AUTOLOADED)).to be_nil + ensure + $LOAD_PATH.delete(File.dirname(path)) + File.delete(path) if File.exist?(path) + end + + it 'rejects nested duplicate ^o keys before Oj.load' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize( + '{"^o":"TemporalJSONSpecFixtures::DummyActivity::Request","scope":{"^o":"Gem::Requirement","^o":"TemporalJSONSpecFixtures::DummyActivity::Request","requirements":[[">=","0"]]}}' + ) + end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate hash key/) + end + + it 'rejects Kernel gadget payloads before Oj.load' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"^o":"Kernel"}') + end.to raise_error(Temporal::JSONDisallowedClassError, /Kernel/) + end + + it 'rejects nested gadget payloads before Oj.load' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize( + '{"^o":"TemporalJSONSpecFixtures::DummyActivity::Request","scope":{"^o":"Gem::Requirement"}}' + ) + end.to raise_error(Temporal::JSONDisallowedClassError, /Gem::Requirement/) + end + + it 'rejects a ::Request name that is not a loaded constant' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"^o":"MissingActivity::Request"}') + end.to raise_error(Temporal::JSONDisallowedClassError, /MissingActivity::Request/) + end + + it 'rejects an unregistered non-Request class' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize( + '{"^o":"TemporalJSONSpecFixtures::DummyWidget","name":"x"}' + ) + end.to raise_error(Temporal::JSONDisallowedClassError, /DummyWidget/) + end + + it 'reconstitutes a class registered with allow_class' do + described_class.allow_class('TemporalJSONSpecFixtures::DummyWidget') + widget = TemporalJSONSpecFixtures::DummyWidget.new(name: 'ok') + loaded = described_class.deserialize(described_class.serialize(widget)) + + expect(loaded).to be_a(TemporalJSONSpecFixtures::DummyWidget) + expect(loaded.name).to eq('ok') + end + + it 'round-trips Temporal::Metadata::Workflow' do + require 'temporal/metadata/workflow' + + metadata = Temporal::Metadata::Workflow.new( + namespace: 'ns', + id: 'wid', + name: 'WorkflowName', + run_id: 'rid', + parent_id: nil, + parent_run_id: nil, + attempt: 1, + task_queue: 'default', + headers: {}, + run_started_at: Time.at(1_700_000_000), + memo: {} + ) + loaded = described_class.deserialize(described_class.serialize(metadata)) + + expect(loaded).to be_a(Temporal::Metadata::Workflow) + expect(loaded.id).to eq('wid') + expect(loaded.task_queue).to eq('default') + end + + it 'round-trips an anonymous Struct' do + response = Struct.new(:workflow_id, :run_id).new('wid', 'rid') + loaded = described_class.deserialize(described_class.serialize(response)) + + expect(loaded.workflow_id).to eq('wid') + expect(loaded.run_id).to eq('rid') + end + + it 'rejects a named Struct class that is not registered' do + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize('{"^u":["Range",1,7,false]}') + end.to raise_error(Temporal::JSONDisallowedClassError, /Range/) + end + + it 'round-trips Date via ^O odd marshaller' do + require 'date' + + date = Date.new(2026, 9, 2) + loaded = described_class.deserialize(described_class.serialize(date)) + + expect(loaded).to eq(date) + end + end +end + +describe Temporal::Concerns::InputDeserializer do + # JSON.parse now runs before Oj.load, so this path must rescue JSON::ParserError too. + let(:deserializer) do + Class.new do + include Temporal::Concerns::InputDeserializer + end.new + end + + it 'preserves newline-split go-client input' do + input = "1012474654\n\"second input\"" + expect(deserializer.deserialize(input)).to eq([1_012_474_654, 'second input']) + end + + it 'does not route JSONDisallowedClassError through the newline fallback' do + expect do + deserializer.deserialize('{"^o":"Gem::Requirement"}') + end.to raise_error(Temporal::JSONDisallowedClassError, /Gem::Requirement/) + end +end diff --git a/temporal.gemspec b/temporal.gemspec index 8c51adefd..e94398210 100644 --- a/temporal.gemspec +++ b/temporal.gemspec @@ -15,6 +15,8 @@ Gem::Specification.new do |spec| spec.files = Dir["{lib,rbi}/**/*.*"] + %w(temporal.gemspec Gemfile LICENSE README.md) spec.add_dependency 'grpc' + # lib/gen stubs use protobuf 3 DescriptorPool#build, which protobuf 4 removed. + spec.add_dependency 'google-protobuf', '~> 3.25' spec.add_dependency 'oj' spec.add_development_dependency 'pry' From 76b3b4295d1158740f2460ff78f87038effd74c0 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Thu, 3 Sep 2026 00:18:41 -0700 Subject: [PATCH 2/2] Honor allow_class for Oj ^O dumps (0.0.7) allow_class previously only applied to ^o instance payloads. Classes Oj encodes as ^O were still rejected even after registration. Co-authored-by: Cursor --- CHANGELOG.md | 2 +- lib/temporal/json.rb | 6 +++-- spec/unit/lib/temporal/json_spec.rb | 42 +++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 477d8eb19..5e5e3567b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## 0.0.7 - `json/plain` no longer builds arbitrary Ruby classes from encoded payloads. That was possible because Oj object mode can allocate any constant named in the payload. -- Still round-trips first-party shapes: activity `Request` / `Response`, `Temporal::` types, Exception subclasses (including backtraces), `Date` / `DateTime` / `Rational`, anonymous Structs, and classes registered with `Temporal::JSON.allow_class`. +- Still round-trips first-party shapes: activity `Request` / `Response`, `Temporal::` types, Exception subclasses (including backtraces), `Date` / `DateTime` / `Rational`, anonymous Structs, and classes registered with `Temporal::JSON.allow_class` (including Oj `^O` dumps). - Rejects duplicate JSON object keys. Oj and `JSON.parse` disagree on duplicates, so a discarded value could still allocate a class. - Rejects constants that are only pending `autoload` until they are actually loaded. - Rejects JSON nested deeper than 512 levels. diff --git a/lib/temporal/json.rb b/lib/temporal/json.rb index 1a7804b7e..de3c31937 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -20,7 +20,7 @@ module JSON MAX_NESTING = 512 MAX_CLASS_NAME_LENGTH = 256 - # ^o allocates instances. ^O encodes Date, DateTime, and Rational. + # ^o allocates instances. ^O encodes Date, DateTime, Rational, and allow_class names. # ^c / ^C look up Class objects. Oj emits ^c when an Exception ivar holds a Class # (see spec/unit/lib/temporal/connection/serializer/failure_spec.rb). INSTANCE_DIRECTIVE_KEYS = %w[^o].freeze @@ -241,7 +241,9 @@ def self.allowed_instance_class?(name) private_class_method :allowed_instance_class? def self.allowed_odd_class?(name) - ALLOWED_ODD_CLASSES.include?(name) && resolve_constant(name) + return false unless valid_constant_name?(name) + + (registered_class?(name) || ALLOWED_ODD_CLASSES.include?(name)) && resolve_constant(name) end private_class_method :allowed_odd_class? diff --git a/spec/unit/lib/temporal/json_spec.rb b/spec/unit/lib/temporal/json_spec.rb index 463eb1b9d..f13cdc6f3 100644 --- a/spec/unit/lib/temporal/json_spec.rb +++ b/spec/unit/lib/temporal/json_spec.rb @@ -28,6 +28,15 @@ def initialize(name: nil) end end + # Oj dumps this as ^O after Oj.register_odd. + class DummyOdd + attr_accessor :name + + def self.create(name) + new.tap { |obj| obj.name = name } + end + end + class DummyError < StandardError attr_reader :code @@ -289,6 +298,39 @@ def initialize(message = nil, code: nil) expect(loaded.name).to eq('ok') end + it 'rejects an unregistered ^O class' do + Oj.register_odd( + TemporalJSONSpecFixtures::DummyOdd, + TemporalJSONSpecFixtures::DummyOdd, + :create, + :name + ) + dumped = described_class.serialize( + TemporalJSONSpecFixtures::DummyOdd.create('x') + ) + + expect(dumped).to include('"^O"') + expect(Oj).not_to receive(:load) + expect do + described_class.deserialize(dumped) + end.to raise_error(Temporal::JSONDisallowedClassError, /DummyOdd/) + end + + it 'reconstitutes a ^O class registered with allow_class' do + Oj.register_odd( + TemporalJSONSpecFixtures::DummyOdd, + TemporalJSONSpecFixtures::DummyOdd, + :create, + :name + ) + described_class.allow_class('TemporalJSONSpecFixtures::DummyOdd') + odd = TemporalJSONSpecFixtures::DummyOdd.create('ok') + loaded = described_class.deserialize(described_class.serialize(odd)) + + expect(loaded).to be_a(TemporalJSONSpecFixtures::DummyOdd) + expect(loaded.name).to eq('ok') + end + it 'round-trips Temporal::Metadata::Workflow' do require 'temporal/metadata/workflow'