Skip to content

Commit 3111ddc

Browse files
cardmagicclaude
andcommitted
fix: authorize a pruned key like the message it replaces
Greptile found a real disclosure. The pruned answer ran `authorize_query` against the synthetic `__snapshot__` operation, while a lookup whose row survives runs the hook for the stored operation. A caller allowed to read state but denied the operation could therefore tell `MessagePruned` from nil and learn that the operation had run. `snapshot` does not expose the remembered keys, so the gate granted more than the thing it borrowed. An actor now remembers the operation beside each key, and the pruned answer runs the same hook against the same operation a surviving row would. Without that the new test raises `SolidObjects::MessagePruned` where it expects nil. `authorized_to_invoke?` carries the check both paths share, and `authorized_to_read?` passes a message to it. Validation: bundle exec rake (795 runs, 0 failures). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d6d35f6 commit 3111ddc

7 files changed

Lines changed: 65 additions & 38 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,11 @@
2323
store and no second write. `reference.find_by(idempotency_key:)` raises
2424
`SolidObjects::MessagePruned` for a key the actor remembers and whose message
2525
retention removed, and still answers `nil` for a key no caller ever sent.
26-
The memory is actor state, so `authorize_query` gates the pruned answer and a
27-
caller the policy refuses reads `nil` for both.
26+
An actor remembers the operation beside each key, so the pruned answer runs
27+
the same hook against the same operation a lookup of the surviving row would,
28+
and a caller the policy refuses reads `nil` for both. Gating it on `snapshot`
29+
would have told a caller who may read state, but not the operation, that the
30+
operation had run.
2831
`retained_idempotency_keys` bounds the memory and defaults to 64 keys for
2932
each actor. A lookup by request id cannot make the distinction, because the
3033
runtime, not the caller, generates a request id and no actor remembers one.

‎docs/architecture.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,9 +442,12 @@ than in a separate tombstone table, which needs no second store, no second
442442
write, and no separate retention. `reference.find_by(idempotency_key:)` raises
443443
`MessagePruned` for a key the actor remembers and whose message retention
444444
removed, and answers `nil` for a key no caller ever sent, so a client can tell
445-
a lost result from a request that never arrived. The memory is actor state, so
446-
`authorize_query` gates the pruned answer the way it gates `snapshot`, and a
447-
caller the policy refuses reads `nil` for both.
445+
a lost result from a request that never arrived. An actor remembers the operation beside each key, so
446+
the pruned answer runs the same hook against the same operation that a lookup
447+
of the surviving row would, and a caller the policy refuses reads `nil` whether
448+
the message is pruned or never existed. Gating it on `snapshot` instead would
449+
tell a caller who may read state, but not the operation, that the operation had
450+
run.
448451
`retained_idempotency_keys` bounds the memory and defaults to 64 keys for each
449452
actor, and `retained_idempotency_keys_bytes` bounds its serialized size at 16 KB,
450453
because an idempotency key has no length limit on every adapter and the memory

‎docs/operations.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,8 +567,9 @@ observe it.
567567
names survives retention. A lookup by idempotency key still tells the two cases
568568
apart after pruning, because the actor remembers the keys of its own last
569569
`retained_idempotency_keys` finished turns: it raises `MessagePruned` for a key
570-
the actor remembers and answers `nil` for a key no caller ever sent. The
571-
memory is actor state, so `authorize_query` gates the pruned answer. Raise
570+
the actor remembers and answers `nil` for a key no caller ever sent. The actor
571+
remembers the operation beside each key, so the pruned answer runs the same
572+
authorization the surviving row would. Raise
572573
`retained_idempotency_keys` above the default of 64 when an actor finishes more
573574
keyed turns than that inside the window in which a caller may retry. A lookup
574575
by request id answers `nil` in both cases, so a caller that must tell them apart

‎lib/solid_objects/client.rb‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -196,24 +196,19 @@ def remembered_message(reference, idempotency_key, authorization_context:)
196196

197197
message = Message.uncached { Message.find_by(instance_id: instance.id, idempotency_key:) }
198198
return message if message
199-
return nil unless Array(instance.completed_idempotency_keys).include?(idempotency_key)
200-
return nil unless readable_state?(reference, authorization_context:)
201199

202-
raise MessagePruned, idempotency_key
203-
end
204-
205-
# @rbs (Reference, authorization_context: untyped) -> bool
206-
def readable_state?(reference, authorization_context:)
207-
authorize!(
208-
hook: SolidObjects.configuration.authorize_query,
209-
reference:,
210-
operation: "__snapshot__",
200+
remembered = Array(instance.completed_idempotency_keys)
201+
.find { |entry| entry["key"] == idempotency_key }
202+
return nil unless remembered
203+
return nil unless authorized_to_invoke?(
204+
actor_type: reference.actor_type,
205+
actor_id: reference.actor_id,
206+
operation: remembered["operation"],
211207
arguments: {},
212208
authorization_context:
213209
)
214-
true
215-
rescue Unauthorized
216-
false
210+
211+
raise MessagePruned, idempotency_key
217212
end
218213

219214
# @rbs (Message?, authorization_context: untyped) -> MessageReference?
@@ -226,21 +221,32 @@ def readable_message(message, authorization_context:)
226221

227222
# @rbs (Message, authorization_context: untyped) -> bool
228223
def authorized_to_read?(message, authorization_context:)
229-
actor_class = SolidObjects.registry.fetch(message.actor_type)
230-
operation = message.operation.to_sym
231-
query = actor_class.definition.queries.key?(operation)
232-
return false unless query || actor_class.definition.messages.key?(operation)
224+
authorized_to_invoke?(
225+
actor_type: message.actor_type,
226+
actor_id: message.actor_id,
227+
operation: message.operation,
228+
arguments: message.arguments,
229+
authorization_context:
230+
)
231+
end
232+
233+
# @rbs (actor_type: String, actor_id: String, operation: String, arguments: Hash[String, untyped], authorization_context: untyped) -> bool
234+
def authorized_to_invoke?(actor_type:, actor_id:, operation:, arguments:, authorization_context:)
235+
actor_class = SolidObjects.registry.fetch(actor_type)
236+
operation_symbol = operation.to_sym
237+
query = actor_class.definition.queries.key?(operation_symbol)
238+
return false unless query || actor_class.definition.messages.key?(operation_symbol)
233239

234240
hook = if query
235241
SolidObjects.configuration.authorize_query
236242
else
237243
SolidObjects.configuration.authorize_message
238244
end
239245
hook.call(
240-
actor_type: message.actor_type,
241-
actor_id: message.actor_id,
242-
operation: message.operation.to_s,
243-
arguments: message.arguments,
246+
actor_type:,
247+
actor_id:,
248+
operation: operation.to_s,
249+
arguments:,
244250
authorization_context:
245251
)
246252
rescue UnknownActorType

‎lib/solid_objects/executor.rb‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,9 +487,11 @@ def remembered_keys(instance, message)
487487
remembered = Array(instance.completed_idempotency_keys)
488488
key = message.idempotency_key
489489
return remembered unless key
490-
return remembered if remembered.last == key
491490

492-
bounded(remembered - [ key ] + [ key ])
491+
entry = { "key" => key, "operation" => message.operation }
492+
return remembered if remembered.last == entry
493+
494+
bounded(remembered.reject { |value| value["key"] == key } + [ entry ])
493495
end
494496

495497
# @rbs (Array[String]) -> Array[String]

‎sig/generated/lib/solid_objects/client.rbs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,15 @@ module SolidObjects
3535
# @rbs (Reference, String, authorization_context: untyped) -> Message?
3636
def remembered_message: (Reference, String, authorization_context: untyped) -> Message?
3737

38-
# @rbs (Reference, authorization_context: untyped) -> bool
39-
def readable_state?: (Reference, authorization_context: untyped) -> bool
40-
4138
# @rbs (Message?, authorization_context: untyped) -> MessageReference?
4239
def readable_message: (Message?, authorization_context: untyped) -> MessageReference?
4340

4441
# @rbs (Message, authorization_context: untyped) -> bool
4542
def authorized_to_read?: (Message, authorization_context: untyped) -> bool
4643

44+
# @rbs (actor_type: String, actor_id: String, operation: String, arguments: Hash[String, untyped], authorization_context: untyped) -> bool
45+
def authorized_to_invoke?: (actor_type: String, actor_id: String, operation: String, arguments: Hash[String, untyped], authorization_context: untyped) -> bool
46+
4747
# @rbs (MessageReference, Message) -> void
4848
def validate_message_reference!: (MessageReference, Message) -> void
4949

‎test/integration/result_lookup_test.rb‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ def reject_checkout
289289
reference.async(idempotency_key: "checkout-7f3a").checkout(order_id: 1)
290290
run_actors
291291
SolidObjects::Message.delete_all
292+
SolidObjects.configuration.authorize_message = ->(**) { false }
292293
SolidObjects.configuration.authorize_query = ->(**) { false }
293294

294295
assert_nil reference.find_by(
@@ -297,6 +298,17 @@ def reject_checkout
297298
)
298299
end
299300

301+
test "does not tell a snapshot-only caller that a key was pruned" do
302+
reference = CartActor.ref("alice")
303+
reference.async(idempotency_key: "checkout-7f3a").checkout(order_id: 1)
304+
run_actors
305+
SolidObjects::Message.delete_all
306+
SolidObjects.configuration.authorize_query = ->(**) { true }
307+
SolidObjects.configuration.authorize_message = ->(**) { false }
308+
309+
assert_nil reference.find_by(idempotency_key: "checkout-7f3a")
310+
end
311+
300312
test "remembers a key whose message was rejected" do
301313
reference = CartActor.ref("alice")
302314
reference.async(idempotency_key: "rejected-7f3a").reject_checkout
@@ -345,16 +357,16 @@ def reject_checkout
345357
end
346358

347359
test "bounds what an instance remembers by size" do
348-
SolidObjects.configuration.retained_idempotency_keys_bytes = 64
360+
SolidObjects.configuration.retained_idempotency_keys_bytes = 128
349361
reference = CartActor.ref("alice")
350362
keys = 3.times.map { |index| "#{index}-#{"k" * 20}" }
351363
keys.each_with_index { |key, index| reference.async(idempotency_key: key).checkout(order_id: index) }
352364
run_actors
353365

354366
remembered = SolidObjects::Instance.sole.completed_idempotency_keys
355367

356-
assert_equal keys.last(2), remembered
357-
assert_operator remembered.to_json.bytesize, :<=, 64
368+
assert_equal keys.last(2), remembered.map { |entry| entry["key"] }
369+
assert_operator remembered.to_json.bytesize, :<=, 128
358370
end
359371

360372
test "remembers nothing for a key larger than what it retains" do
@@ -379,7 +391,7 @@ def reject_checkout
379391
run_actors
380392

381393
assert_equal [ "second", "first" ],
382-
SolidObjects::Instance.sole.completed_idempotency_keys
394+
SolidObjects::Instance.sole.completed_idempotency_keys.map { |entry| entry["key"] }
383395
end
384396

385397
test "remembers nothing for a message that carried no key" do

0 commit comments

Comments
 (0)