feat(yes-core): stop decrypting on the write path (2.1.0) - #35
Merged
Conversation
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MUVcF4oySCLniej8aWGmUr
aroeczek
force-pushed
the
B2BY-4684-write-path-encryptor
branch
from
July 31, 2026 11:00
2bf0336 to
9ab29c9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
pg_eventstore 3.0 runs every registered middleware's
#deserializeon the events returned by#append_to_stream, not only on reads.Yes::Core::Middlewares::Encryptor#deserializemakes two uncached HTTP calls to the encryptor service (GET encryption_keys/:id, thenPATCH message/decrypt), so an encrypted append went from 2 to 4 round trips — fetching 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_responserecords only type, revision, stream and positions;ReadModelUpdater#callalways receives the command payload on the write path, so itspayload_from_eventbranch is dead there;update_stateblock in either host app that readsevent.datadoes so on an unencrypted event.So the work was pure waste. Worse, inside
PgEventstore#multipleit happened within the SERIALIZABLE transaction, widening thePG::TRSerializationFailurewindow thatTransactionQueries#transactionretries — underMAX_RETRIES = 10above it, while pinning a pool connection for up to 10s per call.Measured blast radius in the host apps:
company_manager's team-member batch sync issues ~4 encrypted appends per member per batch (so 16 encryptor calls per member, 8 of them the identical GET), and AMS seeds ~4 encrypted appends per demo application.This is the follow-up that
yousty-eventsourcing2e486e8deferred as "a design decision worth making on its own rather than inside a dependency bump".How
Every write site appends with
middlewares: Middlewares.for_write, which swaps:encryptorfor the new:write_encryptor— same#serialize(it subclassesEncryptor, so encryption at rest cannot drift), no-op#deserialize.Three safeguards make the dangerous misconfiguration impossible rather than merely unlikely:
for_writeis derived from the live config, never hard-coded.PgEventstore::Clientresolves a passed list withconfig.middlewares.slice(*list), which silently drops unregistered names — so a literal%i[with_indifferent_access timestamp write_encryptor]would resolve to a list with no encryptor at all against a config that registered it differently, and write plaintext PII at rest, undetectably. Deriving the list also keeps:timestamp(~20 aggregates readmetadata['created_at']) and picks up future middlewares for free.register_encryptorsets both keys at once, so the read encryptor cannot be registered without its write twin. It takes theconfigobject rather than opening its ownPgEventstore.configureblock, which would deadlock on that method's non-reentrant mutex. If only one ends up registered,for_writefalls back to the full list (correct, just as slow as before) and the railtie warns once at boot.Encryptor#serializeis now idempotent. With both registered, the default list holds two serialize-capable encryptors, so an append that omitsmiddlewares:would encrypt twice — the second pass encrypting the first pass'ses_encryptedsentinels and overwriting the real ciphertext, irrecoverably. The guard also makes re-appending an event read at rest safe, which it was not before.link_tois untouched: it has defaulted tomiddlewares: []since v2, so link events never decrypted.Breaking-ish
#append_to_stream(and therefore by a command) is now encrypted. Nothing in this repo or incompany_manager/AMS consumes it — audited call site by call site — but consumers relying on it must read the event instead. Documented in the CHANGELOG and README.The integration spec's
it 'returns a decrypted event from append_to_stream', added in 2.0.0 as a deliberate tripwire, is inverted here. That is the point of the PR, not an accident.Tests
middlewares/write_path_spec.rb— stands a real recording middleware in for each encryptor key at all four write sites (single command →EventPublisher; command group → insideclient.multiple;Stateless::Handler;TestSupport::EventHelpers#append_event) and asserts the decrypting one was never invoked. This asserts which list pg_eventstore actually applied, which is the thing that can regress.middlewares_spec.rb—register_encryptorordering and classes;for_writein all three registration states, including the fail-safe fallback;without(:encryptor)still means "read at rest".write_encryptor_spec.rb—#serializeidentical toEncryptor,#deserializetouches no repository.encryptor_spec.rb— new context for the idempotence guard.encryptor_integration_spec.rb— append return is not decrypted, data still encrypted at rest, reads still decrypt,:timestamp/:with_indifferent_accesssurvive the write list, repository call counts (1 encrypt / 0 decrypts on write, 1 decrypt on read), and encryption at rest survives a missing:write_encryptor.DummyRepositorygainedfind/encrypt/decryptcall counters, so cost claims are asserted with a real object rather than mocks.Ran locally in a Ruby 3.4.5 container matching CI: yes-core 1205 examples, 0 failures; yes-command-api 190 examples, 0 failures;
bundle exec rubocopclean for these changes (the 6 remainingRails/Exitoffenses are pre-existing, inrails_helperfiles this PR does not touch).Rollout
Version bumped to 2.1.0 across all five gems (
~> 2.0in both host apps accepts it, so no Gemfile edits). After merge,company_managerandapplication_management_systembump their lockfiles and switch their initializers toregister_encryptor— landing on thestaging-es-v3branch set so the fix is included in the cherry-pick to production, and production never runs a day at the doubled encryptor write cost.🤖 Generated with Claude Code
https://claude.ai/code/session_01MUVcF4oySCLniej8aWGmUr