From 9ab29c91341bc25b83f7f28a9c11750992f1259c Mon Sep 17 00:00:00 2001 From: Arek Swidrak Date: Fri, 31 Jul 2026 12:55:45 +0200 Subject: [PATCH] feat(yes-core): stop decrypting on the write path (2.1.0) pg_eventstore 3.0 runs every registered middleware's #deserialize on the events returned by #append_to_stream, not only on reads. Our Encryptor#deserialize is two uncached HTTP calls to the encryptor service (GET encryption_keys/:id, then PATCH message/decrypt), so an encrypted append went from 2 to 4 round trips -- and fetched the same key id twice, each on a freshly built Faraday connection. Nothing on the write path reads that returned event's data: otl_record_response records only type, revision, stream and positions, and ReadModelUpdater always receives the command payload, so its payload_from_event branch is dead there. The work was pure waste, and inside PgEventstore#multiple it happened within the SERIALIZABLE transaction, widening the PG::TRSerializationFailure window that TransactionQueries#transaction retries. So every write site now appends with `middlewares: Middlewares.for_write`, which swaps :encryptor for :write_encryptor -- same #serialize (it subclasses Encryptor, so encryption at rest cannot drift), no-op #deserialize. Three things make the dangerous misconfiguration impossible rather than unlikely: * for_write is DERIVED from the live config. PgEventstore::Client resolves a passed list with config.middlewares.slice(*list), which silently drops unregistered names -- so a hard-coded %i[with_indifferent_access timestamp write_encryptor] would resolve to a list with NO encryptor against a config that registered it differently, and write plaintext at rest undetectably. Deriving it also keeps :timestamp (~20 aggregates read metadata['created_at']) and picks up future middlewares for free. * register_encryptor sets both keys at once, so the read encryptor cannot be registered without its write twin. It takes the config object rather than opening its own PgEventstore.configure block, which would deadlock on that method's non-reentrant mutex. The railtie warns at boot if only one is set; for_write then falls back to the full list, which is correct, just as slow as before. * Encryptor#serialize is now idempotent. With both registered, the DEFAULT list holds two serialize-capable encryptors, so an append that omits `middlewares:` would encrypt twice -- the second pass encrypting the first pass's sentinels and overwriting the real ciphertext, irrecoverably. The guard also makes re-appending an event read at rest safe, which it was not before. Specs assert the middleware list pg_eventstore actually applied (a real recording middleware at all four write sites) and the repository call counts, not just the resulting ciphertext. The integration spec's decrypt-on-append example, added as a tripwire in 2.0.0, is inverted here on purpose: that is the behaviour change. link_to is untouched -- it has defaulted to `middlewares: []` since v2, so link events never decrypted. Deferred follow-up from yousty-eventsourcing 2e486e8 ("a design decision worth making on its own rather than inside a dependency bump"). The same fix for yousty-eventsourcing (identity is the only legacy service with encrypted attributes) is a separate ticket. Refs B2BY-4684 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MUVcF4oySCLniej8aWGmUr --- CHANGELOG.md | 45 ++++++++++ Gemfile.lock | 16 ++-- README.md | 8 +- lib/yes/version.rb | 2 +- yes-auth/Gemfile.lock | 8 +- yes-auth/lib/yes/auth/version.rb | 2 +- yes-command-api/Gemfile.lock | 8 +- .../lib/yes/command/api/version.rb | 2 +- yes-core/Gemfile.lock | 12 +-- .../command_group_executor.rb | 3 +- .../core/command_handling/event_publisher.rb | 3 +- .../yes/core/commands/stateless/handler.rb | 3 +- .../commands/stateless/handler_helpers.rb | 2 +- yes-core/lib/yes/core/data_decryptor.rb | 4 +- yes-core/lib/yes/core/data_encryptor.rb | 8 +- yes-core/lib/yes/core/middlewares.rb | 45 ++++++++++ .../lib/yes/core/middlewares/encryptor.rb | 10 ++- .../yes/core/middlewares/write_encryptor.rb | 35 ++++++++ yes-core/lib/yes/core/railtie.rb | 15 ++++ .../yes/core/test_support/event_helpers.rb | 2 +- yes-core/lib/yes/core/version.rb | 2 +- .../middlewares/encryptor_integration_spec.rb | 83 +++++++++++++++---- .../yes/core/middlewares/encryptor_spec.rb | 11 +++ .../core/middlewares/write_encryptor_spec.rb | 51 ++++++++++++ .../yes/core/middlewares/write_path_spec.rb | 79 ++++++++++++++++++ .../spec/lib/yes/core/middlewares_spec.rb | 62 ++++++++++++++ yes-core/spec/support/dummy_repository.rb | 10 +++ yes-core/spec/support/recording_middleware.rb | 34 ++++++++ yes-read-api/Gemfile.lock | 8 +- yes-read-api/lib/yes/read/api/version.rb | 2 +- 30 files changed, 514 insertions(+), 61 deletions(-) create mode 100644 yes-core/lib/yes/core/middlewares/write_encryptor.rb create mode 100644 yes-core/spec/lib/yes/core/middlewares/write_encryptor_spec.rb create mode 100644 yes-core/spec/lib/yes/core/middlewares/write_path_spec.rb create mode 100644 yes-core/spec/lib/yes/core/middlewares_spec.rb create mode 100644 yes-core/spec/support/recording_middleware.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b3046e21..6171f3e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,51 @@ All notable changes to this project will be documented in this file. +## [2.1.0] - 2026-07-31 + +### yes-core + +#### Added +- `Middlewares::WriteEncryptor` — encrypts on `#serialize` exactly like `Middlewares::Encryptor` + (it subclasses it), but its `#deserialize` is a no-op. +- `Middlewares.register_encryptor(key_repository, config:)` — registers `:encryptor` and + `:write_encryptor` together. Host applications should call this instead of assigning + `config.middlewares[:encryptor]` by hand; registering the decrypting encryptor on its own doubles + the encryptor round trips of every encrypted append. +- `Middlewares.for_write` — the middleware keys to pass to `#append_to_stream`: every configured + middleware, with `:encryptor` swapped for `:write_encryptor`. Derived from the live config rather + than hard-coded, because `PgEventstore::Client` resolves a passed list with + `config.middlewares.slice(*list)`, which silently drops unregistered names — a literal list could + therefore resolve to one with no encryptor at all and write plaintext at rest. Falls back to the + full list when `:write_encryptor` is missing, and the railtie warns about that at boot. +- `Middlewares::ENCRYPTOR` / `Middlewares::WRITE_ENCRYPTOR` config-key constants, and + `DataEncryptor::CIPHERTEXT_KEY` for the `es_encrypted` data key. + +#### Changed +- Every write site now appends with `middlewares: Middlewares.for_write`, so an append no longer + decrypts the event it returns: `CommandHandling::EventPublisher`, + `CommandHandling::CommandGroupExecutor`, `Commands::Stateless::Handler` and + `TestSupport::EventHelpers#append_event`. This covers both `PgEventstore#multiple` paths, whose + sub-events publish through those same call sites. + + pg_eventstore 3.0 runs every registered middleware's `#deserialize` on the events returned by + `#append_to_stream`, not only on reads. Nothing in yes-core reads `data` off that returned event — + `otl_record_response` records only type, revision, stream and positions, and `ReadModelUpdater` + always receives the command payload on the write path — so each encrypted append was paying an + uncached key lookup plus a decrypt against the encryptor service for a payload it discarded. Those + calls also ran inside the SERIALIZABLE transaction opened by `#multiple`, widening the window for + `PG::TRSerializationFailure` and its retries. + + ⚠️ Consumers that relied on the appended event coming back decrypted must read the event instead. + +#### Fixed +- `Middlewares::Encryptor#serialize` is now idempotent: it returns the event untouched when the data + is already encrypted. Required because the default middleware list holds two serialize-capable + encryptors once `:write_encryptor` is registered, so an append that omits `middlewares:` would + otherwise encrypt twice — the second pass encrypting the first pass's sentinels and overwriting the + real ciphertext irrecoverably. It also makes re-appending an event that was read at rest + (`middlewares: Middlewares.without(:encryptor)`) safe, which it was not before. + ## [2.0.0] - 2026-07-28 Major bump because `yes-core` now requires `pg_eventstore` v3, whose schema is diff --git a/Gemfile.lock b/Gemfile.lock index 7d416d9e..a46252bf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,14 +1,14 @@ PATH remote: yes-auth specs: - yes-auth (2.0.0) + yes-auth (2.1.0) rails (>= 7.1) yes-core (~> 2.0) PATH remote: yes-command-api specs: - yes-command-api (2.0.0) + yes-command-api (2.1.0) message_bus (~> 4.0) rails (>= 7.1) yes-core @@ -16,7 +16,7 @@ PATH PATH remote: yes-core specs: - yes-core (2.0.0) + yes-core (2.1.0) cerbos (~> 0.8) dry-inflector (~> 1.2) dry-schema (~> 1.13) @@ -32,7 +32,7 @@ PATH PATH remote: yes-read-api specs: - yes-read-api (2.0.0) + yes-read-api (2.1.0) api-pagination (~> 5.0) pagy (~> 6.0) rails (>= 7.1) @@ -571,10 +571,10 @@ CHECKSUMS useragent (0.16.11) sha256=700e6413ad4bb954bb63547fa098dddf7b0ebe75b40cc6f93b8d54255b173844 websocket-driver (0.8.0) sha256=ed0dba4b943c22f17f9a734817e808bc84cdce6a7e22045f5315aa57676d4962 websocket-extensions (0.1.5) sha256=1c6ba63092cda343eb53fc657110c71c754c56484aad42578495227d717a8241 - yes-auth (2.0.0) - yes-command-api (2.0.0) - yes-core (2.0.0) - yes-read-api (2.0.0) + yes-auth (2.1.0) + yes-command-api (2.1.0) + yes-core (2.1.0) + yes-read-api (2.1.0) zeitwerk (2.7.5) sha256=d8da92128c09ea6ec62c949011b00ed4a20242b255293dd66bf41545398f73dd RUBY VERSION diff --git a/README.md b/README.md index 6af14bfa..c7243c47 100644 --- a/README.md +++ b/README.md @@ -365,14 +365,18 @@ command :change, :ssn, :string, encrypt: true Encryption is performed by a PgEventstore middleware that delegates the actual key management and cryptography to a `key_repository` object you provide. *Yes* does not ship a concrete implementation — you plug in any object that satisfies the interface below. -Register the middleware: +Register the middlewares: ```ruby PgEventstore.configure do |config| - config.middlewares[:encryptor] = Yes::Core::Middlewares::Encryptor.new(key_repository) + Yes::Core::Middlewares.register_encryptor(key_repository, config:) end ``` +This registers two middlewares against your repository: `:encryptor`, which decrypts events as they are read, and `:write_encryptor`, which encrypts identically but does not decrypt. pg_eventstore runs `#deserialize` on the event returned by `#append_to_stream` too, and nothing on the write path reads that event's data — so *Yes* appends with `middlewares: Yes::Core::Middlewares.for_write`, and your `key_repository` is never asked to decrypt a payload that is about to be discarded. Always register through `register_encryptor`: registering `:encryptor` alone silently costs a key lookup and a decrypt on every encrypted write. + +One consequence worth knowing: **the event returned by a command is encrypted**. Read the event back when you need its plaintext. + The `key_repository` must respond to the following methods, each returning a [`Dry::Monads::Result`](https://dry-rb.org/gems/dry-monads/) (or any object responding to `success?`, `failure?`, and `value!`): | Method | Purpose | Returns (on success) | diff --git a/lib/yes/version.rb b/lib/yes/version.rb index 694ce29a..a8485a1e 100644 --- a/lib/yes/version.rb +++ b/lib/yes/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Yes - VERSION = '2.0.0' + VERSION = '2.1.0' end diff --git a/yes-auth/Gemfile.lock b/yes-auth/Gemfile.lock index 65280516..e49c9cee 100644 --- a/yes-auth/Gemfile.lock +++ b/yes-auth/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../yes-core specs: - yes-core (2.0.0) + yes-core (2.1.0) cerbos (~> 0.8) dry-inflector (~> 1.2) dry-schema (~> 1.13) @@ -17,7 +17,7 @@ PATH PATH remote: . specs: - yes-auth (2.0.0) + yes-auth (2.1.0) rails (>= 7.1) yes-core (~> 2.0) @@ -542,8 +542,8 @@ CHECKSUMS useragent (0.16.11) sha256=700e6413ad4bb954bb63547fa098dddf7b0ebe75b40cc6f93b8d54255b173844 websocket-driver (0.8.0) sha256=ed0dba4b943c22f17f9a734817e808bc84cdce6a7e22045f5315aa57676d4962 websocket-extensions (0.1.5) sha256=1c6ba63092cda343eb53fc657110c71c754c56484aad42578495227d717a8241 - yes-auth (2.0.0) - yes-core (2.0.0) + yes-auth (2.1.0) + yes-core (2.1.0) zeitwerk (2.7.5) sha256=d8da92128c09ea6ec62c949011b00ed4a20242b255293dd66bf41545398f73dd RUBY VERSION diff --git a/yes-auth/lib/yes/auth/version.rb b/yes-auth/lib/yes/auth/version.rb index 2ac1b9f9..4eb078ae 100644 --- a/yes-auth/lib/yes/auth/version.rb +++ b/yes-auth/lib/yes/auth/version.rb @@ -2,6 +2,6 @@ module Yes module Auth - VERSION = '2.0.0' + VERSION = '2.1.0' end end diff --git a/yes-command-api/Gemfile.lock b/yes-command-api/Gemfile.lock index c9357eaa..eaaa8e5a 100644 --- a/yes-command-api/Gemfile.lock +++ b/yes-command-api/Gemfile.lock @@ -8,7 +8,7 @@ GIT PATH remote: ../yes-core specs: - yes-core (2.0.0) + yes-core (2.1.0) cerbos (~> 0.8) dry-inflector (~> 1.2) dry-schema (~> 1.13) @@ -24,7 +24,7 @@ PATH PATH remote: . specs: - yes-command-api (2.0.0) + yes-command-api (2.1.0) message_bus (~> 4.0) rails (>= 7.1) yes-core @@ -588,8 +588,8 @@ CHECKSUMS useragent (0.16.11) sha256=700e6413ad4bb954bb63547fa098dddf7b0ebe75b40cc6f93b8d54255b173844 websocket-driver (0.8.0) sha256=ed0dba4b943c22f17f9a734817e808bc84cdce6a7e22045f5315aa57676d4962 websocket-extensions (0.1.5) sha256=1c6ba63092cda343eb53fc657110c71c754c56484aad42578495227d717a8241 - yes-command-api (2.0.0) - yes-core (2.0.0) + yes-command-api (2.1.0) + yes-core (2.1.0) zeitwerk (2.7.5) sha256=d8da92128c09ea6ec62c949011b00ed4a20242b255293dd66bf41545398f73dd BUNDLED WITH diff --git a/yes-command-api/lib/yes/command/api/version.rb b/yes-command-api/lib/yes/command/api/version.rb index 5c219da0..88223dc4 100644 --- a/yes-command-api/lib/yes/command/api/version.rb +++ b/yes-command-api/lib/yes/command/api/version.rb @@ -3,7 +3,7 @@ module Yes module Command module Api - VERSION = '2.0.0' + VERSION = '2.1.0' end end end diff --git a/yes-core/Gemfile.lock b/yes-core/Gemfile.lock index 29b5ef74..c871ea31 100644 --- a/yes-core/Gemfile.lock +++ b/yes-core/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../yes-command-api specs: - yes-command-api (2.0.0) + yes-command-api (2.1.0) message_bus (~> 4.0) rails (>= 7.1) yes-core @@ -9,7 +9,7 @@ PATH PATH remote: ../yes-read-api specs: - yes-read-api (2.0.0) + yes-read-api (2.1.0) api-pagination (~> 5.0) pagy (~> 6.0) rails (>= 7.1) @@ -19,7 +19,7 @@ PATH PATH remote: . specs: - yes-core (2.0.0) + yes-core (2.1.0) cerbos (~> 0.8) dry-inflector (~> 1.2) dry-schema (~> 1.13) @@ -631,9 +631,9 @@ CHECKSUMS webmock (3.26.2) sha256=774556f2ea6371846cca68c01769b2eac0d134492d21f6d0ab5dd643965a4c90 websocket-driver (0.8.0) sha256=ed0dba4b943c22f17f9a734817e808bc84cdce6a7e22045f5315aa57676d4962 websocket-extensions (0.1.5) sha256=1c6ba63092cda343eb53fc657110c71c754c56484aad42578495227d717a8241 - yes-command-api (2.0.0) - yes-core (2.0.0) - yes-read-api (2.0.0) + yes-command-api (2.1.0) + yes-core (2.1.0) + yes-read-api (2.1.0) zeitwerk (2.7.5) sha256=d8da92128c09ea6ec62c949011b00ed4a20242b255293dd66bf41545398f73dd RUBY VERSION diff --git a/yes-core/lib/yes/core/command_handling/command_group_executor.rb b/yes-core/lib/yes/core/command_handling/command_group_executor.rb index 0d60bcfc..0d5b0881 100644 --- a/yes-core/lib/yes/core/command_handling/command_group_executor.rb +++ b/yes-core/lib/yes/core/command_handling/command_group_executor.rb @@ -169,7 +169,8 @@ def publish_subsequent_sub_event(sub_cmd) PgEventstore.client.append_to_stream( utils.build_stream(metadata: sub_cmd.metadata || {}), event, - options: { expected_revision: :any } + options: { expected_revision: :any }, + middlewares: Middlewares.for_write ) end diff --git a/yes-core/lib/yes/core/command_handling/event_publisher.rb b/yes-core/lib/yes/core/command_handling/event_publisher.rb index d292d64f..961b82e8 100644 --- a/yes-core/lib/yes/core/command_handling/event_publisher.rb +++ b/yes-core/lib/yes/core/command_handling/event_publisher.rb @@ -90,7 +90,8 @@ def publish_event PgEventstore.client.append_to_stream( command_utilities.build_stream(metadata:), event, - options: { expected_revision: } + options: { expected_revision: }, + middlewares: Middlewares.for_write ).tap { otl_record_response(_1) } end diff --git a/yes-core/lib/yes/core/commands/stateless/handler.rb b/yes-core/lib/yes/core/commands/stateless/handler.rb index 8884c4b5..67f2a1f0 100644 --- a/yes-core/lib/yes/core/commands/stateless/handler.rb +++ b/yes-core/lib/yes/core/commands/stateless/handler.rb @@ -118,7 +118,8 @@ def publish_event(event_name) PgEventstore.client.append_to_stream( stream, event, - options: { expected_revision: subject_stream_revision } + options: { expected_revision: subject_stream_revision }, + middlewares: Middlewares.for_write ).tap { otl_record_response(_1) } end otl_trackable :publish_event, OpenTelemetry::OtlSpan::OtlData.new(span_name: 'Publish Event', span_kind: :producer) diff --git a/yes-core/lib/yes/core/commands/stateless/handler_helpers.rb b/yes-core/lib/yes/core/commands/stateless/handler_helpers.rb index 11ea36cc..e428000a 100644 --- a/yes-core/lib/yes/core/commands/stateless/handler_helpers.rb +++ b/yes-core/lib/yes/core/commands/stateless/handler_helpers.rb @@ -287,7 +287,7 @@ def last_event(subject, event_name, options = {}, skip_decryption: true) # @return [Enumerator] def load_events(stream, options: {}, skip_decryption: true) options = { direction: 'Backwards' }.merge(options) - middlewares = Middlewares.without(:encryptor) if skip_decryption + middlewares = Middlewares.without(Middlewares::ENCRYPTOR) if skip_decryption PgEventstore.client.read_paginated(stream, options:, middlewares:) end diff --git a/yes-core/lib/yes/core/data_decryptor.rb b/yes-core/lib/yes/core/data_decryptor.rb index 9db68383..1330ca67 100644 --- a/yes-core/lib/yes/core/data_decryptor.rb +++ b/yes-core/lib/yes/core/data_decryptor.rb @@ -40,13 +40,13 @@ def initialize(data:, schema:, repository:) def decrypt_attributes(key:, data:, attributes: {}) # rubocop:disable Lint/UnusedMethodArgument return data unless key - res = key_repository.decrypt(key:, message: data['es_encrypted']) + res = key_repository.decrypt(key:, message: data[DataEncryptor::CIPHERTEXT_KEY]) return data if res.failure? decrypted_text = res.value! decrypted = JSON.parse(decrypted_text.attributes[:message]).transform_keys(&:to_s) decrypted.each { |k, value| data[k] = value if data.key?(k) } - data.delete('es_encrypted') + data.delete(DataEncryptor::CIPHERTEXT_KEY) data end diff --git a/yes-core/lib/yes/core/data_encryptor.rb b/yes-core/lib/yes/core/data_encryptor.rb index 3024c6d2..8b8818b9 100644 --- a/yes-core/lib/yes/core/data_encryptor.rb +++ b/yes-core/lib/yes/core/data_encryptor.rb @@ -10,6 +10,10 @@ module Core # encryptor.encrypted_data # encryptor.encryption_metadata class DataEncryptor + # Data key holding the ciphertext of all encrypted attributes. Its presence means the data is + # currently encrypted: it is written here and removed by {DataDecryptor}. + CIPHERTEXT_KEY = 'es_encrypted' + # @return [Hash] the encrypted data attr_reader :encrypted_data @@ -51,8 +55,8 @@ def initialize(data:, schema:, repository:) def encrypt_attributes(key:, data:, attributes:) text = JSON.generate(data.select { |hash_key, _value| attributes.include?(hash_key.to_s) }) encrypted = key_repository.encrypt(key:, message: text).value! - attributes.each { |att| data[att.to_s] = 'es_encrypted' if data.key?(att.to_s) } - data['es_encrypted'] = encrypted.attributes[:message] + attributes.each { |att| data[att.to_s] = CIPHERTEXT_KEY if data.key?(att.to_s) } + data[CIPHERTEXT_KEY] = encrypted.attributes[:message] data end end diff --git a/yes-core/lib/yes/core/middlewares.rb b/yes-core/lib/yes/core/middlewares.rb index 483d3205..7ba3ffb9 100644 --- a/yes-core/lib/yes/core/middlewares.rb +++ b/yes-core/lib/yes/core/middlewares.rb @@ -3,9 +3,54 @@ module Yes module Core module Middlewares + # Config key of the encryptor used on read paths: encrypts on #serialize, decrypts on #deserialize. + ENCRYPTOR = :encryptor + # Config key of the encryptor used on write paths: encrypts on #serialize, no-op on #deserialize. + WRITE_ENCRYPTOR = :write_encryptor + class << self + # Registers both encryptor middlewares against the same key repository. + # + # Always use this instead of assigning config.middlewares[:encryptor] by hand: registering the + # decrypting encryptor without its write-only twin silently doubles the encryptor round trips every + # encrypted append performs (see {WriteEncryptor}). + # + # Mutates the given config in place rather than opening its own PgEventstore.configure block, because + # PgEventstore.configure takes a non-reentrant mutex - nesting one inside another deadlocks. + # + # @param key_repository [#find, #create, #encrypt, #decrypt] + # @param config [PgEventstore::Config] the config yielded by PgEventstore.configure + # @return [void] + def register_encryptor(key_repository, config: PgEventstore.config) + config.middlewares[ENCRYPTOR] = Encryptor.new(key_repository) + config.middlewares[WRITE_ENCRYPTOR] = WriteEncryptor.new(key_repository) + end + + # Middleware keys to pass to #append_to_stream: every configured middleware, with the decrypting + # encryptor swapped for the write-only one. + # + # Derived from the live config, never hard-coded. PgEventstore::Client resolves a passed list with + # `config.middlewares.slice(*list)`, which silently drops names that are not registered - so a literal + # list would resolve to one with NO encryptor at all against a config that registered it under a + # different key, and would write plaintext at rest undetectably. Deriving the list makes that + # impossible, and picks up any middleware added later for free. + # + # Falls back to the full list when {WRITE_ENCRYPTOR} is not registered. That is correct, just as slow + # as before - unlike a hard-coded list, which would drop encryption altogether. + # + # @return [Array] + def for_write + keys = PgEventstore.config.middlewares.keys + return keys unless keys.include?(WRITE_ENCRYPTOR) + + keys - [ENCRYPTOR] + end + # Returns middleware keys excluding the specified one. # + # Note that excluding {ENCRYPTOR} still yields a list containing {WRITE_ENCRYPTOR}, whose #deserialize + # is a no-op - so `without(:encryptor)` keeps meaning "read the data as it is stored". + # # @param middleware_name [Symbol] the middleware key to exclude # @return [Array] remaining middleware keys def without(middleware_name) diff --git a/yes-core/lib/yes/core/middlewares/encryptor.rb b/yes-core/lib/yes/core/middlewares/encryptor.rb index 6ba74cf5..df6df9f2 100644 --- a/yes-core/lib/yes/core/middlewares/encryptor.rb +++ b/yes-core/lib/yes/core/middlewares/encryptor.rb @@ -5,9 +5,12 @@ module Core module Middlewares # PgEventstore middleware for encrypting/decrypting event data. # + # Register it through {Middlewares.register_encryptor} rather than by hand, so its write-only + # counterpart ({WriteEncryptor}) is always registered alongside it. + # # @example # PgEventstore.configure do |config| - # config.middlewares[:encryptor] = Yes::Core::Middlewares::Encryptor.new(key_repository) + # Yes::Core::Middlewares.register_encryptor(key_repository, config:) # end class Encryptor include PgEventstore::Middleware @@ -24,6 +27,11 @@ def initialize(key_repository) # @return [PgEventstore::Event] def serialize(event) return event unless event.class.respond_to?(:encryption_schema) + # Idempotence guard. With {WriteEncryptor} registered, the DEFAULT middleware list holds two + # serialize-capable encryptors, so an append that omits `middlewares:` would encrypt twice. The + # second pass would encrypt the sentinels written by the first and overwrite the real ciphertext, + # which cannot be recovered. It also guards re-appending an event that was read at rest. + return event if event.data[DataEncryptor::CIPHERTEXT_KEY].present? encryptor = DataEncryptor.new( data: event.data, schema: event.class.encryption_schema, repository: key_repository diff --git a/yes-core/lib/yes/core/middlewares/write_encryptor.rb b/yes-core/lib/yes/core/middlewares/write_encryptor.rb new file mode 100644 index 00000000..2f8d7704 --- /dev/null +++ b/yes-core/lib/yes/core/middlewares/write_encryptor.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Yes + module Core + module Middlewares + # Encrypts on the way in exactly like {Encryptor}, but does not decrypt on the way out. + # + # pg_eventstore 3.0 runs every registered middleware's #deserialize on the events returned by + # #append_to_stream, not only on the ones returned by reads. Nothing on the write path reads `data` off + # that returned event, so decrypting it costs two uncached HTTP calls to the encryptor service (a key + # lookup and a decrypt) per encrypted event, for a payload that is immediately discarded. + # + # #serialize is inherited rather than reimplemented on purpose: encryption at rest must never differ + # between the read and the write variant. + # + # Registered alongside {Encryptor} by {Middlewares.register_encryptor} and selected by + # {Middlewares.for_write}. The read path keeps using {Encryptor}, unchanged. + # + # @example + # PgEventstore.client.append_to_stream(stream, event, middlewares: Yes::Core::Middlewares.for_write) + class WriteEncryptor < Encryptor + # Deliberately does nothing: the caller does not read the returned event's data. + # + # Returns the event rather than nil. pg_eventstore itself ignores the return value, but middlewares + # are also invoked directly in places that use it. + # + # @param event [PgEventstore::Event] + # @return [PgEventstore::Event] + def deserialize(event) + event + end + end + end + end +end diff --git a/yes-core/lib/yes/core/railtie.rb b/yes-core/lib/yes/core/railtie.rb index 123c0e24..f37bf127 100644 --- a/yes-core/lib/yes/core/railtie.rb +++ b/yes-core/lib/yes/core/railtie.rb @@ -49,6 +49,21 @@ class Railtie < Rails::Railtie PgEventstore.logger ||= Rails.logger if ENV['PG_ES_LOGGING'] == 'true' end + # The one misconfiguration Middlewares.for_write cannot fix by itself: a decrypting encryptor registered + # without its write-only twin. for_write then falls back to the full list, so writes stay CORRECT - they + # just keep paying an encryptor key lookup and decrypt per encrypted event, for data nobody reads. + # Warned about once at boot rather than on every append. + config.after_initialize do + middlewares = PgEventstore.config.middlewares + next unless middlewares.key?(Yes::Core::Middlewares::ENCRYPTOR) + next if middlewares.key?(Yes::Core::Middlewares::WRITE_ENCRYPTOR) + + Rails.logger.warn( + 'PgEventstore middleware :encryptor is registered without :write_encryptor, so append_to_stream ' \ + 'decrypts every event it returns. Register both with Yes::Core::Middlewares.register_encryptor.' + ) + end + # Load aggregate shortcuts when Rails console starts console do Yes::Core::Utils::AggregateShortcuts.load! diff --git a/yes-core/lib/yes/core/test_support/event_helpers.rb b/yes-core/lib/yes/core/test_support/event_helpers.rb index 2bc2a3f9..f9bd3750 100644 --- a/yes-core/lib/yes/core/test_support/event_helpers.rb +++ b/yes-core/lib/yes/core/test_support/event_helpers.rb @@ -16,7 +16,7 @@ module EventHelpers # @param event [Yes::Core::Event] # @return [void] def append_event(stream, event) - PgEventstore.client.append_to_stream(stream, event) + PgEventstore.client.append_to_stream(stream, event, middlewares: Yes::Core::Middlewares.for_write) end # Appends an event to a stream and reloads it diff --git a/yes-core/lib/yes/core/version.rb b/yes-core/lib/yes/core/version.rb index 238d5349..7127c68e 100644 --- a/yes-core/lib/yes/core/version.rb +++ b/yes-core/lib/yes/core/version.rb @@ -2,6 +2,6 @@ module Yes module Core - VERSION = '2.0.0' + VERSION = '2.1.0' end end diff --git a/yes-core/spec/lib/yes/core/middlewares/encryptor_integration_spec.rb b/yes-core/spec/lib/yes/core/middlewares/encryptor_integration_spec.rb index e6ad0a70..bfe78611 100644 --- a/yes-core/spec/lib/yes/core/middlewares/encryptor_integration_spec.rb +++ b/yes-core/spec/lib/yes/core/middlewares/encryptor_integration_spec.rb @@ -1,13 +1,11 @@ # frozen_string_literal: true -# End-to-end coverage of the encryptor middleware as pg_eventstore actually applies it. +# End-to-end coverage of the encryptor middlewares as pg_eventstore actually applies them. # -# encryptor_spec.rb unit-tests #serialize/#deserialize by calling them directly, which -# cannot catch changes in WHEN pg_eventstore invokes them. pg_eventstore 3.0 started -# running #deserialize on append_to_stream as well as on reads, and no spec in this repo -# could see that: nothing here registers an encryptor in config.middlewares, and the host -# applications gate their registration on `!Rails.env.test?`, so their suites cannot see -# it either. +# encryptor_spec.rb unit-tests #serialize/#deserialize by calling them directly, which cannot catch changes in +# WHEN pg_eventstore invokes them. pg_eventstore 3.0 started running #deserialize on append_to_stream as well +# as on reads; every yes-core write site therefore passes `middlewares: Middlewares.for_write`, which swaps +# :encryptor for :write_encryptor so an append does not pay a decrypt round trip for a payload nobody reads. RSpec.describe 'Encryptor middleware integration' do let(:key_repository) { DummyRepository.new } let(:user_id) { SecureRandom.uuid } @@ -17,20 +15,27 @@ let(:data) do { 'user_id' => user_id, 'name' => 'Anakin Skywalker', 'secret_name' => 'Darth Vader' } end - let(:append) { PgEventstore.client.append_to_stream(stream, EncryptedEvent.new(data: data.dup)) } + let(:append) do + PgEventstore.client.append_to_stream( + stream, EncryptedEvent.new(data: data.dup), middlewares: Yes::Core::Middlewares.for_write + ) + end + let(:at_rest) do + PgEventstore.client.read(stream, middlewares: Yes::Core::Middlewares.without(Yes::Core::Middlewares::ENCRYPTOR)).last + end around do |example| - # Assigned last so :with_indifferent_access stays first, matching how the host - # applications register it. - PgEventstore.config.middlewares[:encryptor] = Yes::Core::Middlewares::Encryptor.new(key_repository) + # Registered last so :with_indifferent_access stays first, matching how the host applications register it. + Yes::Core::Middlewares.register_encryptor(key_repository) example.run ensure - PgEventstore.config.middlewares.delete(:encryptor) + PgEventstore.config.middlewares.except!( + Yes::Core::Middlewares::ENCRYPTOR, Yes::Core::Middlewares::WRITE_ENCRYPTOR + ) end it 'stores the protected attribute encrypted' do append - at_rest = PgEventstore.client.read(stream, middlewares: Yes::Core::Middlewares.without(:encryptor)).last aggregate_failures do expect(at_rest.data['secret_name']).to eq('es_encrypted') @@ -45,10 +50,52 @@ expect(PgEventstore.client.read(stream).last.data['secret_name']).to eq('Darth Vader') end - # pg_eventstore 3.0 runs #deserialize on append too, so the returned event comes back - # already decrypted. Pinned here so a future change to that behaviour fails in this - # spec rather than silently in a consumer. - it 'returns a decrypted event from append_to_stream' do - expect(append.data['secret_name']).to eq('Darth Vader') + # Was pinned the other way round while the write path used the default middleware list. The whole point of + # :write_encryptor is that the appended event comes back exactly as it went to disk. + it 'does not decrypt the event returned by append_to_stream' do + aggregate_failures do + expect(append.data['secret_name']).to eq('es_encrypted') + expect(append.data['es_encrypted']).to be_present + expect(append.metadata['encryption']).to be_present + end + end + + it 'keeps the other middlewares on the write list' do + aggregate_failures do + expect(append.metadata['created_at']).to be_present + expect(append.data).to be_a(ActiveSupport::HashWithIndifferentAccess) + end + end + + it 'performs no decrypt work on the write path' do + append + + aggregate_failures do + expect(DummyRepository.calls[:encrypt]).to eq(1) + expect(DummyRepository.calls[:decrypt]).to eq(0) + end + end + + it 'still decrypts once per event on the read path' do + append + PgEventstore.client.read(stream) + + expect(DummyRepository.calls[:decrypt]).to eq(1) + end + + # Fail-safe: for_write falls back to the full list rather than producing one with no encryptor in it, so + # encryption at rest survives the misconfiguration. + context 'when the write encryptor is not registered' do + before { PgEventstore.config.middlewares.delete(Yes::Core::Middlewares::WRITE_ENCRYPTOR) } + + it 'falls back to the full middleware list' do + expect(Yes::Core::Middlewares.for_write).to include(Yes::Core::Middlewares::ENCRYPTOR) + end + + it 'still encrypts at rest' do + append + + expect(at_rest.data['secret_name']).to eq('es_encrypted') + end end end diff --git a/yes-core/spec/lib/yes/core/middlewares/encryptor_spec.rb b/yes-core/spec/lib/yes/core/middlewares/encryptor_spec.rb index 2d9f862f..5844555d 100644 --- a/yes-core/spec/lib/yes/core/middlewares/encryptor_spec.rb +++ b/yes-core/spec/lib/yes/core/middlewares/encryptor_spec.rb @@ -45,6 +45,17 @@ end end end + + # With WriteEncryptor registered the DEFAULT middleware list holds two serialize-capable encryptors, so an + # append that omits `middlewares:` calls #serialize twice. Without the guard the second pass would encrypt + # the sentinels written by the first and overwrite the real ciphertext, which cannot be recovered. + context 'when the data is already encrypted' do + let(:event) { instance.serialize(EncryptedEvent.new(data: data.dup)) } + + it 'keeps the ciphertext untouched' do + expect { instance.serialize(event) }.not_to(change { event.data.to_h }) + end + end end describe '#deserialize' do diff --git a/yes-core/spec/lib/yes/core/middlewares/write_encryptor_spec.rb b/yes-core/spec/lib/yes/core/middlewares/write_encryptor_spec.rb new file mode 100644 index 00000000..5220ba46 --- /dev/null +++ b/yes-core/spec/lib/yes/core/middlewares/write_encryptor_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +RSpec.describe Yes::Core::Middlewares::WriteEncryptor do + let(:instance) { described_class.new(DummyRepository.new) } + let(:data) do + { + 'user_id' => 'dab48d26-e4f8-41fc-a9a8-59657e590716', + 'name' => 'Anakin Skywalker', + 'secret_name' => 'Darth Vader' + } + end + + describe '#serialize' do + subject { instance.serialize(event) } + + let(:event) { EncryptedEvent.new(data: data.dup) } + + it 'encrypts exactly like the decrypting encryptor' do + aggregate_failures do + expect(subject.data).to( + eq( + 'user_id' => 'dab48d26-e4f8-41fc-a9a8-59657e590716', + 'name' => 'Anakin Skywalker', + 'secret_name' => 'es_encrypted', + 'es_encrypted' => DummyRepository.encrypt(data.slice('secret_name').to_json) + ) + ) + expect(subject.metadata).to match(hash_including('encryption')) + end + end + end + + describe '#deserialize' do + subject { instance.deserialize(event) } + + let(:event) { instance.serialize(EncryptedEvent.new(data: data.dup)) } + + before do + event # encrypt first, so the counters below reflect #deserialize alone + DummyRepository.reset + end + + it 'returns the event with its data still encrypted, without touching the key repository' do + aggregate_failures do + expect(subject).to be(event) + expect(subject.data['secret_name']).to eq('es_encrypted') + expect(DummyRepository.calls).to eq(find: 0, encrypt: 0, decrypt: 0) + end + end + end +end diff --git a/yes-core/spec/lib/yes/core/middlewares/write_path_spec.rb b/yes-core/spec/lib/yes/core/middlewares/write_path_spec.rb new file mode 100644 index 00000000..55cfc6d6 --- /dev/null +++ b/yes-core/spec/lib/yes/core/middlewares/write_path_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +# test_support is ignored by the Zeitwerk loader (lib/yes/core.rb), so consumers require it explicitly. +require 'yes/core/test_support' + +# Every yes-core write site must append through Middlewares.for_write, so the decrypting :encryptor never runs +# on the write path. Asserted by standing a recording middleware in for each encryptor key and checking which +# one pg_eventstore invoked - the claim is about the middleware LIST, so no encrypted event class is needed. +RSpec.describe 'Write path middleware selection', integration: true do + let(:read_encryptor) { RecordingMiddleware.new } + let(:write_encryptor) { RecordingMiddleware.new } + + around do |example| + PgEventstore.config.middlewares[Yes::Core::Middlewares::ENCRYPTOR] = read_encryptor + PgEventstore.config.middlewares[Yes::Core::Middlewares::WRITE_ENCRYPTOR] = write_encryptor + example.run + ensure + PgEventstore.config.middlewares.except!( + Yes::Core::Middlewares::ENCRYPTOR, Yes::Core::Middlewares::WRITE_ENCRYPTOR + ) + end + + # write_encryptor.deserialized is expected to be positive: the write variant IS on the list, and it is its + # #deserialize that does nothing. The load-bearing assertions are the read encryptor's zeros. + shared_examples 'a write site that skips decryption' do + it 'appends through the write encryptor and never invokes the decrypting one' do + subject + + aggregate_failures do + expect(write_encryptor.serialized).to be_positive + expect(read_encryptor.serialized).to eq(0) + expect(read_encryptor.deserialized).to eq(0) + end + end + end + + describe 'EventPublisher, via a single aggregate command' do + subject { Test::User::Aggregate.new(aggregate_id).change_name(name: 'Jane', user_id: SecureRandom.uuid) } + + let(:aggregate_id) { SecureRandom.uuid } + + before { TestUser.create!(id: aggregate_id, name: 'John') } + + it_behaves_like 'a write site that skips decryption' + end + + describe 'CommandGroupExecutor, inside client.multiple' do + subject { Test::PersonalInfo::Aggregate.new(SecureRandom.uuid).update_personal_info_group(**payload) } + + let(:payload) do + { first_name: 'Ada', last_name: 'Lovelace', email: 'ada@example.com', birth_date: '1815-12-10' } + end + + it_behaves_like 'a write site that skips decryption' + end + + describe 'Stateless::Handler' do + subject { handler_class.new(cmd).call } + + let(:handler_class) do + Class.new(Yes::Core::Commands::Stateless::Handler).tap { |klass| klass.event_name = 'NameChanged' } + end + let(:cmd) { Dummy::Company::Commands::ChangeName::Command.new(name: 'some', company_id: SecureRandom.uuid) } + + it_behaves_like 'a write site that skips decryption' + end + + describe 'TestSupport::EventHelpers#append_event' do + include Yes::Core::TestSupport::EventHelpers + + subject { append_event(stream, Yes::Core::Event.new(type: 'Dummy::CompanyNameChanged', data: { 'name' => 'x' })) } + + let(:stream) do + PgEventstore::Stream.new(context: 'Dummy', stream_name: 'Company', stream_id: SecureRandom.uuid) + end + + it_behaves_like 'a write site that skips decryption' + end +end diff --git a/yes-core/spec/lib/yes/core/middlewares_spec.rb b/yes-core/spec/lib/yes/core/middlewares_spec.rb new file mode 100644 index 00000000..9fa1e0f7 --- /dev/null +++ b/yes-core/spec/lib/yes/core/middlewares_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +RSpec.describe Yes::Core::Middlewares do + let(:key_repository) { DummyRepository.new } + let(:base_keys) { %i[with_indifferent_access timestamp] } + + after { PgEventstore.config.middlewares.except!(described_class::ENCRYPTOR, described_class::WRITE_ENCRYPTOR) } + + describe '.register_encryptor' do + subject { described_class.register_encryptor(key_repository) } + + it 'registers both variants after the order-sensitive middlewares' do + subject + + expect(PgEventstore.config.middlewares.keys).to eq(base_keys + %i[encryptor write_encryptor]) + end + + it 'registers the right class under each key' do + subject + + aggregate_failures do + expect(PgEventstore.config.middlewares[described_class::ENCRYPTOR]). + to be_an_instance_of(described_class::Encryptor) + expect(PgEventstore.config.middlewares[described_class::WRITE_ENCRYPTOR]). + to be_an_instance_of(described_class::WriteEncryptor) + end + end + end + + describe '.for_write' do + subject { described_class.for_write } + + context 'when both encryptor variants are registered' do + before { described_class.register_encryptor(key_repository) } + + it { is_expected.to eq(base_keys + [described_class::WRITE_ENCRYPTOR]) } + end + + # A list that dropped :encryptor here would write plaintext at rest, so the fallback keeps it. + context 'when only the decrypting encryptor is registered' do + before do + described_class.register_encryptor(key_repository) + PgEventstore.config.middlewares.delete(described_class::WRITE_ENCRYPTOR) + end + + it { is_expected.to eq(base_keys + [described_class::ENCRYPTOR]) } + end + + context 'when no encryptor is registered' do + it { is_expected.to eq(base_keys) } + end + end + + describe '.without' do + subject { described_class.without(described_class::ENCRYPTOR) } + + before { described_class.register_encryptor(key_repository) } + + # Still means "read the data as stored": WriteEncryptor#deserialize is a no-op. + it { is_expected.to eq(base_keys + [described_class::WRITE_ENCRYPTOR]) } + end +end diff --git a/yes-core/spec/support/dummy_repository.rb b/yes-core/spec/support/dummy_repository.rb index a96a1421..f1882ad0 100644 --- a/yes-core/spec/support/dummy_repository.rb +++ b/yes-core/spec/support/dummy_repository.rb @@ -70,9 +70,16 @@ def attributes class << self attr_accessor :repository + # Counts calls per operation, so specs can assert how much encryptor work a code path performs without + # stubbing the repository. + # + # @return [Hash{Symbol => Integer}] + attr_accessor :calls + # @return [void] def reset self.repository = {} + self.calls = { find: 0, encrypt: 0, decrypt: 0 } end # @param str [String] @@ -92,6 +99,7 @@ def decrypt(str) # @param user_id [String] # @return [DummyRepository::Success] def find(user_id) + self.class.calls[:find] += 1 Success.new(Key.new(id: user_id)) end @@ -99,6 +107,7 @@ def find(user_id) # @param message [String] # @return [DummyRepository::Success] def encrypt(key:, message:) + self.class.calls[:encrypt] += 1 self.class.repository[key.id] = self.class.encrypt(message) message = Message.new({ message: self.class.repository[key.id] }) Success.new(message) @@ -108,6 +117,7 @@ def encrypt(key:, message:) # @param message [String] # @return [DummyRepository::Success] def decrypt(key:, message:) + self.class.calls[:decrypt] += 1 decrypted = if self.class.repository[key.id] self.class.decrypt(self.class.repository[key.id]) diff --git a/yes-core/spec/support/recording_middleware.rb b/yes-core/spec/support/recording_middleware.rb new file mode 100644 index 00000000..b6bd42eb --- /dev/null +++ b/yes-core/spec/support/recording_middleware.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Real (non-mock) PgEventstore middleware that records how often pg_eventstore invoked each hook. +# +# Lets specs assert WHICH middleware list a call site passed, without stubbing PgEventstore::Client and without +# needing an encrypted event class - the claim under test is about the list, not about ciphertext. +class RecordingMiddleware + include PgEventstore::Middleware + + # @return [Integer] + attr_reader :serialized + + # @return [Integer] + attr_reader :deserialized + + def initialize + @serialized = 0 + @deserialized = 0 + end + + # @param event [PgEventstore::Event] + # @return [PgEventstore::Event] + def serialize(event) + @serialized += 1 + event + end + + # @param event [PgEventstore::Event] + # @return [PgEventstore::Event] + def deserialize(event) + @deserialized += 1 + event + end +end diff --git a/yes-read-api/Gemfile.lock b/yes-read-api/Gemfile.lock index 6ee69342..f5a1e0b5 100644 --- a/yes-read-api/Gemfile.lock +++ b/yes-read-api/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: ../yes-core specs: - yes-core (2.0.0) + yes-core (2.1.0) cerbos (~> 0.8) dry-inflector (~> 1.2) dry-schema (~> 1.13) @@ -17,7 +17,7 @@ PATH PATH remote: . specs: - yes-read-api (2.0.0) + yes-read-api (2.1.0) api-pagination (~> 5.0) pagy (~> 6.0) rails (>= 7.1) @@ -583,8 +583,8 @@ CHECKSUMS useragent (0.16.11) sha256=700e6413ad4bb954bb63547fa098dddf7b0ebe75b40cc6f93b8d54255b173844 websocket-driver (0.8.0) sha256=ed0dba4b943c22f17f9a734817e808bc84cdce6a7e22045f5315aa57676d4962 websocket-extensions (0.1.5) sha256=1c6ba63092cda343eb53fc657110c71c754c56484aad42578495227d717a8241 - yes-core (2.0.0) - yes-read-api (2.0.0) + yes-core (2.1.0) + yes-read-api (2.1.0) zeitwerk (2.7.5) sha256=d8da92128c09ea6ec62c949011b00ed4a20242b255293dd66bf41545398f73dd BUNDLED WITH diff --git a/yes-read-api/lib/yes/read/api/version.rb b/yes-read-api/lib/yes/read/api/version.rb index 258ab49c..c6e1ff1c 100644 --- a/yes-read-api/lib/yes/read/api/version.rb +++ b/yes-read-api/lib/yes/read/api/version.rb @@ -3,7 +3,7 @@ module Yes module Read module Api - VERSION = '2.0.0' + VERSION = '2.1.0' end end end