From 6d08d859465f396b0b919dbfb88f8fe144d2f0ec Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Wed, 2 Sep 2026 22:30:03 -0700 Subject: [PATCH 01/12] Fail-closed json/plain deserialize against Oj object-mode gadgets. Reject arbitrary ^o instantiation while keeping Request/Response payloads and error serialization v2 working. Co-authored-by: Cursor --- .github/workflows/tests.yml | 4 +- CHANGELOG.md | 4 + lib/temporal/errors.rb | 2 + lib/temporal/json.rb | 124 +++++++++++++- lib/temporal/version.rb | 2 +- .../connection/converter/payload/json_spec.rb | 4 + spec/unit/lib/temporal/json.rb | 152 ++++++++++++++++++ 7 files changed, 288 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2456cb5d9..9d7546101 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,9 +2,9 @@ name: Tests on: push: - branches: [ "master" ] + branches: [ "master", "transfers-master" ] pull_request: - branches: [ "master" ] + branches: [ "master", "transfers-master" ] jobs: test_gem: diff --git a/CHANGELOG.md b/CHANGELOG.md index cd9849848..8fadf19b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.0.7 + +- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Instances are reconstituted only for loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), and `Temporal::JSON.allow_class`. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged (SECBUGS-174). + ## 0.0.6 - Defer gRPC loading via autoload for fork safety diff --git a/lib/temporal/errors.rb b/lib/temporal/errors.rb index 7aa114056..a6090f734 100644 --- a/lib/temporal/errors.rb +++ b/lib/temporal/errors.rb @@ -2,6 +2,8 @@ module Temporal # Superclass for all Temporal errors class Error < StandardError; end + 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..e157a9741 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -1,5 +1,8 @@ # Helper class for serializing/deserializing JSON +require 'json' require 'oj' +require 'set' +require 'temporal/errors' module Temporal module JSON @@ -9,12 +12,131 @@ module JSON float_precision: 0 }.freeze + # ^o / ^O allocate instances. ^c / ^C look up Class objects (error serialization v2). + INSTANCE_DIRECTIVE_KEYS = %w[^o ^O].freeze + CLASS_REFERENCE_DIRECTIVE_KEYS = %w[^c ^C].freeze + STRUCT_DIRECTIVE_KEYS = %w[^u].freeze + ALLOWED_CLASS_SUFFIXES = %w[::Request ::Response].freeze + + ALLOWED_CLASSES = Set.new + ALLOWED_CLASSES_MUTEX = Mutex.new + private_constant :ALLOWED_CLASSES, :ALLOWED_CLASSES_MUTEX + def self.serialize(value) Oj.dump(value, OJ_OPTIONS) end + # SECBUGS-174: Oj mode: :object instantiates any constant named in ^o. + # Walk a JSON.parse tree first and only then Oj.load the original bytes so + # symbol keys and ^t stay compatible with encode. 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_safe!(::JSON.parse(raw)) + Oj.load(raw, OJ_OPTIONS) + end + + 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_safe!(obj) + case obj + when Hash + STRUCT_DIRECTIVE_KEYS.each do |key| + next unless obj.key?(key) + + name = class_name_from_directive(obj[key]) + unless name && registered_class?(name) + raise Temporal::JSONDisallowedClassError, + "json/plain payload requested disallowed class #{obj[key].inspect}" + 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 #{obj[key].inspect}" + 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 #{obj[key].inspect}" + end + end + + obj.each_value { |v| assert_safe!(v) } + when Array + obj.each { |v| assert_safe!(v) } + end + end + private_class_method :assert_safe! + + 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.registered_class?(name) + with_allowed_classes { |set| set.include?(name) } + end + private_class_method :registered_class? + + def self.allowed_instance_class?(name) + return true if registered_class?(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_class_reference?(name) + return true if registered_class?(name) + + resolve_constant(name).is_a?(Module) + end + private_class_method :allowed_class_reference? + + def self.resolve_constant(name) + parts = name.split('::') + parts.shift if parts.first.empty? + return nil if parts.empty? + + parts.reduce(Object) do |mod, part| + return nil unless mod.is_a?(Module) && 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..73278c781 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,10 @@ describe Temporal::Connection::Converter::Payload::JSON do subject { described_class.new } + it 'keeps the json/plain encoding name' do + 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 index c2d00cd4d..7d1ac203a 100644 --- a/spec/unit/lib/temporal/json.rb +++ b/spec/unit/lib/temporal/json.rb @@ -1,9 +1,53 @@ require 'temporal/json' +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) @@ -22,5 +66,113 @@ it 'parses nil' do expect(described_class.deserialize(nil)).to eq(nil) end + + 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 'reconstitutes a loaded class reference via ^c' do + expect(described_class.deserialize('{"^c":"String"}')).to eq(String) + end + + 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 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 end end From 3bc84caf2ad3440ea68dc873239270dc8ccae726 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Thu, 27 Aug 2026 15:44:36 -0700 Subject: [PATCH 02/12] Pin protobuf 3 in CI and use docker compose v2. 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 --- .github/workflows/tests.yml | 2 +- Gemfile | 3 +++ examples/Gemfile | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9d7546101..198ecc013 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -37,7 +37,7 @@ jobs: - name: Start dependencies run: | - docker-compose \ + docker compose \ -f examples/docker-compose.yml \ up -d diff --git a/Gemfile b/Gemfile index fa75df156..c03120191 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,6 @@ source 'https://rubygems.org' gemspec + +# Generated lib/gen stubs use protobuf 3 DescriptorPool#build, which protobuf 4 removed. +gem 'google-protobuf', '~> 3.25.0' diff --git a/examples/Gemfile b/examples/Gemfile index 9c543b774..45e0be6fc 100644 --- a/examples/Gemfile +++ b/examples/Gemfile @@ -1,6 +1,7 @@ source 'https://rubygems.org' gem 'temporal-ruby', path: '../' +gem 'google-protobuf', '~> 3.25.0' gem 'dry-types', '>= 1.2.0' gem 'dry-struct', '~> 1.1.1' From 3fce4e546e1421cc1626cd39b753d61b1c66cdeb Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Thu, 27 Aug 2026 15:47:30 -0700 Subject: [PATCH 03/12] Wait for Temporal on 7233 before example namespace registration. docker compose now starts, but auto-setup is not ready after a 10s sleep, so register_namespace hits connection refused. Co-authored-by: Cursor --- .github/workflows/tests.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 198ecc013..72b5c3ef6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -50,13 +50,23 @@ jobs: run: | cd examples && bundle install --path vendor/bundle - - name: Wait for dependencies to settle + - name: Wait for Temporal run: | - sleep 10 + timeout 180 bash -c 'until echo >/dev/tcp/127.0.0.1/7233; do sleep 2; done' - 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: | From 044ef474bc2d954827fd669c29b748398fe5395e Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Thu, 27 Aug 2026 16:38:23 -0700 Subject: [PATCH 04/12] Pin example Temporal images so CI can actually reach 7233. 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 --- .github/workflows/tests.yml | 8 +++++++- examples/docker-compose.yml | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 72b5c3ef6..2e7c85004 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -52,7 +52,13 @@ jobs: - name: Wait for Temporal run: | - timeout 180 bash -c 'until echo >/dev/tcp/127.0.0.1/7233; do sleep 2; done' + 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 - name: Register namespace run: | diff --git a/examples/docker-compose.yml b/examples/docker-compose.yml index 4bff724c6..d4a292bfb 100644 --- a/examples/docker-compose.yml +++ b/examples/docker-compose.yml @@ -2,7 +2,7 @@ version: '3.5' services: temporal: - image: temporalio/auto-setup:latest + image: temporalio/auto-setup:1.22.4 ports: - "7233:7233" environment: @@ -14,7 +14,7 @@ services: - cassandra temporal-web: - image: temporalio/web:latest + image: temporalio/web:1.15.0 environment: - "TEMPORAL_GRPC_ENDPOINT=temporal:7233" ports: @@ -23,6 +23,6 @@ services: - temporal cassandra: - image: cassandra:3.11 + image: cassandra:3.11.16 ports: - "9042:9042" From 04efe374bf477ed7513ad25c05edba00b6ddef30 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Thu, 27 Aug 2026 16:48:16 -0700 Subject: [PATCH 05/12] Allow Temporal library types and anonymous Structs through json/plain. Example workflows return Metadata::Workflow and Struct.new results. Those are first-party Oj encodings, not gadget classes. Co-authored-by: Cursor --- .github/workflows/tests.yml | 4 ++++ CHANGELOG.md | 2 +- lib/temporal/json.rb | 20 +++++++++++++++++- spec/unit/lib/temporal/json.rb | 38 ++++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2e7c85004..6846aab05 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -94,6 +94,10 @@ jobs: run: | cd examples && bin/worker & + - 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 8fadf19b2..ccb503cc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.0.7 -- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Instances are reconstituted only for loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), and `Temporal::JSON.allow_class`. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged (SECBUGS-174). +- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged (SECBUGS-174). ## 0.0.6 diff --git a/lib/temporal/json.rb b/lib/temporal/json.rb index e157a9741..d9c7f4cc6 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -58,9 +58,10 @@ def self.assert_safe!(obj) when Hash 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 && registered_class?(name) + unless name && allowed_struct_class?(name) raise Temporal::JSONDisallowedClassError, "json/plain payload requested disallowed class #{obj[key].inspect}" end @@ -110,6 +111,7 @@ def self.registered_class?(name) def self.allowed_instance_class?(name) return true if registered_class?(name) + return true if library_class?(name) return true if ALLOWED_CLASS_SUFFIXES.any? { |suffix| name.end_with?(suffix) } && resolve_constant(name) klass = resolve_constant(name) @@ -117,6 +119,22 @@ def self.allowed_instance_class?(name) end private_class_method :allowed_instance_class? + def self.allowed_struct_class?(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? + def self.allowed_class_reference?(name) return true if registered_class?(name) diff --git a/spec/unit/lib/temporal/json.rb b/spec/unit/lib/temporal/json.rb index 7d1ac203a..bf998d3a3 100644 --- a/spec/unit/lib/temporal/json.rb +++ b/spec/unit/lib/temporal/json.rb @@ -174,5 +174,43 @@ def initialize(message = nil, code: nil) 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 end end From 01a28b475c0b2f1eed2822a5a6e7d335467225c4 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Thu, 27 Aug 2026 16:55:13 -0700 Subject: [PATCH 06/12] Allow Thread::Backtrace through json/plain so raised errors round-trip. 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 --- CHANGELOG.md | 2 +- lib/temporal/json.rb | 6 ++++++ spec/unit/lib/temporal/{json.rb => json_spec.rb} | 12 ++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) rename spec/unit/lib/temporal/{json.rb => json_spec.rb} (93%) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb503cc5..8f4c275ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.0.7 -- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged (SECBUGS-174). +- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), Oj Exception backtrace types (`Thread::Backtrace`), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged (SECBUGS-174). ## 0.0.6 diff --git a/lib/temporal/json.rb b/lib/temporal/json.rb index d9c7f4cc6..5885bf904 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -17,6 +17,11 @@ module JSON CLASS_REFERENCE_DIRECTIVE_KEYS = %w[^c ^C].freeze STRUCT_DIRECTIVE_KEYS = %w[^u].freeze ALLOWED_CLASS_SUFFIXES = %w[::Request ::Response].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 @@ -112,6 +117,7 @@ def self.registered_class?(name) def self.allowed_instance_class?(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) diff --git a/spec/unit/lib/temporal/json.rb b/spec/unit/lib/temporal/json_spec.rb similarity index 93% rename from spec/unit/lib/temporal/json.rb rename to spec/unit/lib/temporal/json_spec.rb index bf998d3a3..6c81e18f1 100644 --- a/spec/unit/lib/temporal/json.rb +++ b/spec/unit/lib/temporal/json_spec.rb @@ -116,6 +116,18 @@ def initialize(message = nil, code: nil) 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 From 9f303f46bd6f7db42833c1729264aee0e1e76819 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Wed, 2 Sep 2026 19:18:21 -0700 Subject: [PATCH 07/12] Reject duplicate JSON keys before Oj.load and restore odd-type round-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 --- CHANGELOG.md | 2 +- lib/temporal/concerns/input_deserializer.rb | 2 +- lib/temporal/json.rb | 70 ++++++++++++++++++--- spec/unit/lib/temporal/json_spec.rb | 47 ++++++++++++++ 4 files changed, 110 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f4c275ae..dcf223250 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.0.7 -- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), Oj Exception backtrace types (`Thread::Backtrace`), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged (SECBUGS-174). +- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Duplicate hash keys are rejected before load. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), Oj odd-marshaller types (`Date`, `DateTime`, `Rational`), Oj Exception backtrace types (`Thread::Backtrace`), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged. ## 0.0.6 diff --git a/lib/temporal/concerns/input_deserializer.rb b/lib/temporal/concerns/input_deserializer.rb index 8f636df8e..b20118855 100644 --- a/lib/temporal/concerns/input_deserializer.rb +++ b/lib/temporal/concerns/input_deserializer.rb @@ -3,7 +3,7 @@ module Concerns module InputDeserializer def deserialize(input) JSON.deserialize(input) - rescue Oj::ParseError + rescue Oj::ParseError, ::JSON::ParserError # 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/json.rb b/lib/temporal/json.rb index 5885bf904..37c672183 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -12,11 +12,14 @@ module JSON float_precision: 0 }.freeze - # ^o / ^O allocate instances. ^c / ^C look up Class objects (error serialization v2). - INSTANCE_DIRECTIVE_KEYS = %w[^o ^O].freeze + # ^o allocates instances. ^O is Oj's odd marshaller (Date, DateTime, Rational). + # ^c / ^C look up Class objects (error serialization v2). + 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 @@ -27,20 +30,49 @@ module JSON ALLOWED_CLASSES_MUTEX = Mutex.new private_constant :ALLOWED_CLASSES, :ALLOWED_CLASSES_MUTEX + # Oj::Saj sees every hash key assignment. JSON.parse collapses duplicate keys. + class DuplicateKeyValidator < Oj::Saj + def initialize + @hash_key_sets = [] + end + + def hash_start(_key) + @hash_key_sets << Set.new + end + + def hash_end(_key) + @hash_key_sets.pop + end + + def add_value(_value, key) + return if key.nil? + + keys = @hash_key_sets.last + if keys.include?(key) + raise Temporal::JSONDisallowedClassError, + "json/plain payload contains duplicate key #{key.inspect}" + end + + keys << key + end + end + private_constant :DuplicateKeyValidator + def self.serialize(value) Oj.dump(value, OJ_OPTIONS) end - # SECBUGS-174: Oj mode: :object instantiates any constant named in ^o. - # Walk a JSON.parse tree first and only then Oj.load the original bytes so - # symbol keys and ^t stay compatible with encode. + # Fail-closed json/plain: Oj mode :object instantiates any constant named in ^o. + # Reject duplicate hash keys with Oj::Saj, validate directives on a JSON tree, then + # Oj.load the original bytes so symbol keys and ^t stay compatible with encode. def self.deserialize(value) return nil if value.nil? raw = value.to_s return nil if raw.empty? - assert_safe!(::JSON.parse(raw)) + assert_no_duplicate_hash_keys!(raw) + assert_safe!(::JSON.parse(raw, max_nesting: false)) Oj.load(raw, OJ_OPTIONS) end @@ -58,6 +90,11 @@ def self.with_allowed_classes end private_class_method :with_allowed_classes + def self.assert_no_duplicate_hash_keys!(raw) + Oj.saj_parse(DuplicateKeyValidator.new, raw) + end + private_class_method :assert_no_duplicate_hash_keys! + def self.assert_safe!(obj) case obj when Hash @@ -68,7 +105,7 @@ def self.assert_safe!(obj) name = class_name_from_directive(obj[key]) unless name && allowed_struct_class?(name) raise Temporal::JSONDisallowedClassError, - "json/plain payload requested disallowed class #{obj[key].inspect}" + "json/plain payload requested disallowed class #{name.inspect}" end end @@ -78,7 +115,17 @@ def self.assert_safe!(obj) name = class_name_from_directive(obj[key]) unless name && allowed_instance_class?(name) raise Temporal::JSONDisallowedClassError, - "json/plain payload requested disallowed class #{obj[key].inspect}" + "json/plain payload requested disallowed class #{name.inspect}" + 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 #{name.inspect}" end end @@ -88,7 +135,7 @@ def self.assert_safe!(obj) name = class_name_from_directive(obj[key]) unless name && allowed_class_reference?(name) raise Temporal::JSONDisallowedClassError, - "json/plain payload requested disallowed class #{obj[key].inspect}" + "json/plain payload requested disallowed class #{name.inspect}" end end @@ -125,6 +172,11 @@ def self.allowed_instance_class?(name) 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) registered_class?(name) || library_class?(name) end diff --git a/spec/unit/lib/temporal/json_spec.rb b/spec/unit/lib/temporal/json_spec.rb index 6c81e18f1..f31743809 100644 --- a/spec/unit/lib/temporal/json_spec.rb +++ b/spec/unit/lib/temporal/json_spec.rb @@ -1,4 +1,5 @@ require 'temporal/json' +require 'temporal/concerns/input_deserializer' module TemporalJSONSpecFixtures module DummyActivity @@ -146,6 +147,24 @@ def initialize(message = nil, code: nil) 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 key/) + 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 key/) + end + it 'rejects Kernel gadget payloads before Oj.load' do expect(Oj).not_to receive(:load) expect do @@ -224,5 +243,33 @@ def initialize(message = nil, code: nil) 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 + 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 From 423f197e92821ef241f4d41b39373120c4e0e6c8 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Wed, 2 Sep 2026 21:36:52 -0700 Subject: [PATCH 08/12] Harden json/plain deserialize against duplicate keys and deep nesting. 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 --- CHANGELOG.md | 3 +- lib/temporal/json.rb | 178 +++++++++++++++++++--------- spec/unit/lib/temporal/json_spec.rb | 63 +++++++++- temporal.gemspec | 1 + 4 files changed, 187 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcf223250..7d5ffb0ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ## 0.0.7 -- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Duplicate hash keys are rejected before load. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), Oj odd-marshaller types (`Date`, `DateTime`, `Rational`), Oj Exception backtrace types (`Thread::Backtrace`), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged. +- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Duplicate hash keys (including container-valued keys) and excessive nesting are rejected in an Oj::Saj pass before load. Directive validation resolves only already-loaded constants and does not trigger `autoload`. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), Oj odd-marshaller types (`Date`, `DateTime`, `Rational`), Oj Exception backtrace types (`Thread::Backtrace`), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged. +- Declare runtime dependency on `google-protobuf` ~> 3.25 (generated stubs under `lib/gen/` require protobuf 3). ## 0.0.6 diff --git a/lib/temporal/json.rb b/lib/temporal/json.rb index 37c672183..52a8b7adb 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -8,10 +8,14 @@ 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_KEY_LENGTH = 256 + MAX_CLASS_NAME_LENGTH = 256 + # ^o allocates instances. ^O is Oj's odd marshaller (Date, DateTime, Rational). # ^c / ^C look up Class objects (error serialization v2). INSTANCE_DIRECTIVE_KEYS = %w[^o].freeze @@ -30,49 +34,85 @@ module JSON ALLOWED_CLASSES_MUTEX = Mutex.new private_constant :ALLOWED_CLASSES, :ALLOWED_CLASSES_MUTEX - # Oj::Saj sees every hash key assignment. JSON.parse collapses duplicate keys. - class DuplicateKeyValidator < Oj::Saj + DUPLICATE_KEY_ERROR = 'json/plain payload contains duplicate hash key' + NESTING_DEPTH_ERROR = 'json/plain payload exceeds maximum nesting depth' + OVERSIZED_KEY_ERROR = 'json/plain payload contains oversized hash key' + + # Oj::Saj walks the raw bytes. JSON.parse collapses duplicate keys; Oj.load may + # instantiate discarded values. Reject duplicate keys and excessive nesting here. + class PayloadStructureValidator < Oj::Saj def initialize @hash_key_sets = [] + @depth = 0 end - def hash_start(_key) + 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 + + def note(key) return if key.nil? + if key.is_a?(String) && key.length > Temporal::JSON::MAX_KEY_LENGTH + raise Temporal::JSONDisallowedClassError, Temporal::JSON::OVERSIZED_KEY_ERROR + end + keys = @hash_key_sets.last + return if keys.nil? + if keys.include?(key) - raise Temporal::JSONDisallowedClassError, - "json/plain payload contains duplicate key #{key.inspect}" + raise Temporal::JSONDisallowedClassError, Temporal::JSON::DUPLICATE_KEY_ERROR end keys << key end end - private_constant :DuplicateKeyValidator + private_constant :PayloadStructureValidator def self.serialize(value) Oj.dump(value, OJ_OPTIONS) end # Fail-closed json/plain: Oj mode :object instantiates any constant named in ^o. - # Reject duplicate hash keys with Oj::Saj, validate directives on a JSON tree, then - # Oj.load the original bytes so symbol keys and ^t stay compatible with encode. + # Reject duplicate hash keys and deep nesting with Oj::Saj, validate directives on a + # JSON tree, then Oj.load the original bytes so symbol keys and ^t stay compatible. def self.deserialize(value) return nil if value.nil? raw = value.to_s return nil if raw.empty? - assert_no_duplicate_hash_keys!(raw) - assert_safe!(::JSON.parse(raw, max_nesting: false)) + assert_payload_structure!(raw) + assert_safe!(::JSON.parse(raw, max_nesting: MAX_NESTING)) Oj.load(raw, OJ_OPTIONS) end @@ -90,61 +130,69 @@ def self.with_allowed_classes end private_class_method :with_allowed_classes - def self.assert_no_duplicate_hash_keys!(raw) - Oj.saj_parse(DuplicateKeyValidator.new, raw) + def self.assert_payload_structure!(raw) + Oj.saj_parse(PayloadStructureValidator.new, raw) end - private_class_method :assert_no_duplicate_hash_keys! + private_class_method :assert_payload_structure! def self.assert_safe!(obj) - case obj - when Hash - 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 #{name.inspect}" - end + 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! - INSTANCE_DIRECTIVE_KEYS.each do |key| - next unless obj.key?(key) + 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_instance_class?(name) - raise Temporal::JSONDisallowedClassError, - "json/plain payload requested disallowed class #{name.inspect}" - end + 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 - ODD_MARSHALLER_KEYS.each do |key| - next unless obj.key?(key) + INSTANCE_DIRECTIVE_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 #{name.inspect}" - end + 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 - CLASS_REFERENCE_DIRECTIVE_KEYS.each do |key| - next unless obj.key?(key) + ODD_MARSHALLER_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 #{name.inspect}" - end + 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 - obj.each_value { |v| assert_safe!(v) } - when Array - obj.each { |v| assert_safe!(v) } + 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 :assert_safe! + private_class_method :validate_hash_directives! def self.class_name_from_directive(value) case value @@ -156,12 +204,28 @@ def self.class_name_from_directive(value) 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) @@ -178,6 +242,8 @@ def self.allowed_odd_class?(name) 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? @@ -194,19 +260,21 @@ def self.anonymous_struct_directive?(value) private_class_method :anonymous_struct_directive? 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) - parts = name.split('::') - parts.shift if parts.first.empty? - return nil if parts.empty? + return nil unless valid_constant_name?(name) - parts.reduce(Object) do |mod, part| - return nil unless mod.is_a?(Module) && mod.const_defined?(part, false) + 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 diff --git a/spec/unit/lib/temporal/json_spec.rb b/spec/unit/lib/temporal/json_spec.rb index f31743809..a848cb42e 100644 --- a/spec/unit/lib/temporal/json_spec.rb +++ b/spec/unit/lib/temporal/json_spec.rb @@ -153,7 +153,66 @@ def initialize(message = nil, code: nil) described_class.deserialize( '{"^o":"Gem::Requirement","^o":"TemporalJSONSpecFixtures::DummyActivity::Request"}' ) - end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate key/) + 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 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 'rejects very deep arrays without exhausting the Ruby stack' do + depth = 5_000 + 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 @@ -162,7 +221,7 @@ def initialize(message = nil, code: nil) described_class.deserialize( '{"^o":"TemporalJSONSpecFixtures::DummyActivity::Request","scope":{"^o":"Gem::Requirement","^o":"TemporalJSONSpecFixtures::DummyActivity::Request","requirements":[[">=","0"]]}}' ) - end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate key/) + end.to raise_error(Temporal::JSONDisallowedClassError, /duplicate hash key/) end it 'rejects Kernel gadget payloads before Oj.load' do diff --git a/temporal.gemspec b/temporal.gemspec index 8c51adefd..246b026a8 100644 --- a/temporal.gemspec +++ b/temporal.gemspec @@ -15,6 +15,7 @@ Gem::Specification.new do |spec| spec.files = Dir["{lib,rbi}/**/*.*"] + %w(temporal.gemspec Gemfile LICENSE README.md) spec.add_dependency 'grpc' + spec.add_dependency 'google-protobuf', '~> 3.25' spec.add_dependency 'oj' spec.add_development_dependency 'pry' From c1871792f671bbaa8b42356658736e66b2ad76e2 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Wed, 2 Sep 2026 21:48:45 -0700 Subject: [PATCH 09/12] Drop unvalidated key-length cap and extend duplicate-key specs. 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 --- lib/temporal/json.rb | 13 ++++-------- spec/unit/lib/temporal/json_spec.rb | 32 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/temporal/json.rb b/lib/temporal/json.rb index 52a8b7adb..fffa46e56 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -13,11 +13,11 @@ module JSON }.freeze MAX_NESTING = 512 - MAX_KEY_LENGTH = 256 MAX_CLASS_NAME_LENGTH = 256 # ^o allocates instances. ^O is Oj's odd marshaller (Date, DateTime, Rational). - # ^c / ^C look up Class objects (error serialization v2). + # ^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 @@ -34,9 +34,8 @@ module JSON ALLOWED_CLASSES_MUTEX = Mutex.new private_constant :ALLOWED_CLASSES, :ALLOWED_CLASSES_MUTEX - DUPLICATE_KEY_ERROR = 'json/plain payload contains duplicate hash key' - NESTING_DEPTH_ERROR = 'json/plain payload exceeds maximum nesting depth' - OVERSIZED_KEY_ERROR = 'json/plain payload contains oversized hash key' + DUPLICATE_KEY_ERROR = 'json/plain payload contains duplicate hash key'.freeze + NESTING_DEPTH_ERROR = 'json/plain payload exceeds maximum nesting depth'.freeze # Oj::Saj walks the raw bytes. JSON.parse collapses duplicate keys; Oj.load may # instantiate discarded values. Reject duplicate keys and excessive nesting here. @@ -82,10 +81,6 @@ def bump_depth! def note(key) return if key.nil? - if key.is_a?(String) && key.length > Temporal::JSON::MAX_KEY_LENGTH - raise Temporal::JSONDisallowedClassError, Temporal::JSON::OVERSIZED_KEY_ERROR - end - keys = @hash_key_sets.last return if keys.nil? diff --git a/spec/unit/lib/temporal/json_spec.rb b/spec/unit/lib/temporal/json_spec.rb index a848cb42e..74f090a47 100644 --- a/spec/unit/lib/temporal/json_spec.rb +++ b/spec/unit/lib/temporal/json_spec.rb @@ -172,6 +172,38 @@ def initialize(message = nil, code: nil) 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 From 834f3de4ae554ddfa608b5326c7c2e35c71cc315 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Wed, 2 Sep 2026 21:58:20 -0700 Subject: [PATCH 10/12] Trim redundant depth spec and duplicate protobuf Gemfile pins. 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 --- Gemfile | 3 --- examples/Gemfile | 1 - spec/unit/lib/temporal/json_spec.rb | 10 ---------- 3 files changed, 14 deletions(-) diff --git a/Gemfile b/Gemfile index c03120191..fa75df156 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,3 @@ source 'https://rubygems.org' gemspec - -# Generated lib/gen stubs use protobuf 3 DescriptorPool#build, which protobuf 4 removed. -gem 'google-protobuf', '~> 3.25.0' diff --git a/examples/Gemfile b/examples/Gemfile index 45e0be6fc..9c543b774 100644 --- a/examples/Gemfile +++ b/examples/Gemfile @@ -1,7 +1,6 @@ source 'https://rubygems.org' gem 'temporal-ruby', path: '../' -gem 'google-protobuf', '~> 3.25.0' gem 'dry-types', '>= 1.2.0' gem 'dry-struct', '~> 1.1.1' diff --git a/spec/unit/lib/temporal/json_spec.rb b/spec/unit/lib/temporal/json_spec.rb index 74f090a47..bc5556097 100644 --- a/spec/unit/lib/temporal/json_spec.rb +++ b/spec/unit/lib/temporal/json_spec.rb @@ -221,16 +221,6 @@ def initialize(message = nil, code: nil) end.to raise_error(Temporal::JSONDisallowedClassError, /maximum nesting depth/) end - it 'rejects very deep arrays without exhausting the Ruby stack' do - depth = 5_000 - 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") From 5ca889a06fd40020500c27c27f9907e520cdb0ca Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Wed, 2 Sep 2026 22:19:37 -0700 Subject: [PATCH 11/12] Clarify 0.0.7 changelog and reviewer comments on deserialize order. 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 --- CHANGELOG.md | 9 +++++++-- lib/temporal/concerns/input_deserializer.rb | 2 ++ lib/temporal/json.rb | 18 ++++++++++++------ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d5ffb0ee..477d8eb19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,13 @@ ## 0.0.7 -- Fail-closed `json/plain` deserialize: Oj `mode: :object` no longer instantiates arbitrary classes. Duplicate hash keys (including container-valued keys) and excessive nesting are rejected in an Oj::Saj pass before load. Directive validation resolves only already-loaded constants and does not trigger `autoload`. Instances are reconstituted only for loaded `Temporal::` types, loaded `::Request` / `::Response` types, loaded Exception subclasses (error serialization v2), Oj odd-marshaller types (`Date`, `DateTime`, `Rational`), Oj Exception backtrace types (`Thread::Backtrace`), and `Temporal::JSON.allow_class`. Anonymous Structs (`^u` with member names) round-trip. Class references (`^c`) require a loaded constant. Encode and the `json/plain` encoding name are unchanged. -- Declare runtime dependency on `google-protobuf` ~> 3.25 (generated stubs under `lib/gen/` require protobuf 3). +- `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 diff --git a/lib/temporal/concerns/input_deserializer.rb b/lib/temporal/concerns/input_deserializer.rb index b20118855..f3dbba746 100644 --- a/lib/temporal/concerns/input_deserializer.rb +++ b/lib/temporal/concerns/input_deserializer.rb @@ -4,6 +4,8 @@ module InputDeserializer def deserialize(input) JSON.deserialize(input) 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/json.rb b/lib/temporal/json.rb index fffa46e56..c3d6246bb 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -15,7 +15,7 @@ module JSON MAX_NESTING = 512 MAX_CLASS_NAME_LENGTH = 256 - # ^o allocates instances. ^O is Oj's odd marshaller (Date, DateTime, Rational). + # ^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 @@ -37,8 +37,9 @@ module JSON DUPLICATE_KEY_ERROR = 'json/plain payload contains duplicate hash key'.freeze NESTING_DEPTH_ERROR = 'json/plain payload exceeds maximum nesting depth'.freeze - # Oj::Saj walks the raw bytes. JSON.parse collapses duplicate keys; Oj.load may - # instantiate discarded values. Reject duplicate keys and excessive nesting here. + # 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 = [] @@ -78,6 +79,8 @@ def bump_depth! 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? @@ -97,9 +100,8 @@ def self.serialize(value) Oj.dump(value, OJ_OPTIONS) end - # Fail-closed json/plain: Oj mode :object instantiates any constant named in ^o. - # Reject duplicate hash keys and deep nesting with Oj::Saj, validate directives on a - # JSON tree, then Oj.load the original bytes so symbol keys and ^t stay compatible. + # 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) return nil if value.nil? @@ -111,6 +113,8 @@ def self.deserialize(value) 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 @@ -254,6 +258,8 @@ def self.anonymous_struct_directive?(value) 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) From 58d40e57b249b104fc4357e22fb1955b7e59e550 Mon Sep 17 00:00:00 2001 From: Ian Yap Date: Wed, 2 Sep 2026 22:27:10 -0700 Subject: [PATCH 12/12] Add reviewer maps on json.rb and the other 0.0.7 diffs. 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 --- .github/workflows/tests.yml | 5 +++++ examples/docker-compose.yml | 1 + lib/temporal/errors.rb | 1 + lib/temporal/json.rb | 7 ++++++- .../lib/temporal/connection/converter/payload/json_spec.rb | 1 + spec/unit/lib/temporal/json_spec.rb | 3 +++ temporal.gemspec | 1 + 7 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6846aab05..f35c4f8de 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -2,6 +2,7 @@ name: Tests on: push: + # This fork ships from transfers-master. Without it, PRs to that branch had no CI. branches: [ "master", "transfers-master" ] pull_request: branches: [ "master", "transfers-master" ] @@ -35,6 +36,7 @@ jobs: steps: - uses: actions/checkout@v3 + # ubuntu-latest no longer ships the docker-compose v1 binary. - name: Start dependencies run: | docker compose \ @@ -50,6 +52,7 @@ jobs: run: | cd examples && bundle install --path vendor/bundle + # auto-setup is not ready after a fixed sleep; wait until gRPC accepts connections. - name: Wait for Temporal run: | if timeout 300 bash -c 'until echo >/dev/tcp/127.0.0.1/7233; do sleep 2; done'; then @@ -60,6 +63,7 @@ jobs: 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 @@ -94,6 +98,7 @@ 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 diff --git a/examples/docker-compose.yml b/examples/docker-compose.yml index d4a292bfb..f37c947f1 100644 --- a/examples/docker-compose.yml +++ b/examples/docker-compose.yml @@ -1,5 +1,6 @@ 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:1.22.4 diff --git a/lib/temporal/errors.rb b/lib/temporal/errors.rb index a6090f734..c3b8b2321 100644 --- a/lib/temporal/errors.rb +++ b/lib/temporal/errors.rb @@ -2,6 +2,7 @@ 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 diff --git a/lib/temporal/json.rb b/lib/temporal/json.rb index c3d6246bb..1a7804b7e 100644 --- a/lib/temporal/json.rb +++ b/lib/temporal/json.rb @@ -1,4 +1,9 @@ -# 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' 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 73278c781..f55f2cce5 100644 --- a/spec/unit/lib/temporal/connection/converter/payload/json_spec.rb +++ b/spec/unit/lib/temporal/connection/converter/payload/json_spec.rb @@ -4,6 +4,7 @@ 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 diff --git a/spec/unit/lib/temporal/json_spec.rb b/spec/unit/lib/temporal/json_spec.rb index bc5556097..463eb1b9d 100644 --- a/spec/unit/lib/temporal/json_spec.rb +++ b/spec/unit/lib/temporal/json_spec.rb @@ -68,6 +68,7 @@ def initialize(message = nil, code: nil) 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)) @@ -133,6 +134,7 @@ def initialize(message = nil, code: nil) 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 @@ -337,6 +339,7 @@ def initialize(message = nil, code: nil) 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 diff --git a/temporal.gemspec b/temporal.gemspec index 246b026a8..e94398210 100644 --- a/temporal.gemspec +++ b/temporal.gemspec @@ -15,6 +15,7 @@ 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'