Skip to content

Commit b60dec5

Browse files
committed
fix: preserve pruned lookup authorization
Retain original arguments within the history byte bound and deny legacy entries without them. Inline the single-use lookup, bound, and dead-row helpers requested by review. See #79
1 parent 863dea6 commit b60dec5

10 files changed

Lines changed: 81 additions & 67 deletions

File tree

‎docs/architecture.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,12 +442,17 @@ 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. An actor remembers the operation beside each key, so
446-
the pruned answer runs the same hook against the same operation that a lookup
445+
a lost result from a request that never arrived. An actor remembers the operation and original arguments beside each key, so
446+
the pruned answer runs the same hook against the same operation and arguments that a lookup
447447
of the surviving row would, and a caller the policy refuses reads `nil` whether
448448
the message is pruned or never existed. Gating it on `snapshot` instead would
449449
tell a caller who may read state, but not the operation, that the operation had
450450
run.
451+
Remembered arguments count toward the serialized memory limit and remain until
452+
the entry is evicted or the instance is removed. Entries from older versions
453+
that lack arguments return absence after pruning because their original
454+
authorization cannot be reproduced.
455+
451456
`retained_idempotency_keys` bounds the memory and defaults to 64 keys for each
452457
actor, and `retained_idempotency_keys_bytes` bounds its serialized size at 16 KB,
453458
because an idempotency key has no length limit on every adapter and the memory

‎docs/operations.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,13 +568,18 @@ 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
570570
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
571+
remembers the operation and original arguments beside each key, so the pruned answer runs the same
572572
authorization the surviving row would. Raise
573573
`retained_idempotency_keys` above the default of 64 when an actor finishes more
574574
keyed turns than that inside the window in which a caller may retry. A lookup
575575
by request id answers `nil` in both cases, so a caller that must tell them apart
576576
sends its own idempotency key.
577577

578+
Remembered arguments count toward the serialized memory limit and remain until
579+
the entry is evicted or the instance is removed. Entries from older versions
580+
that lack arguments return absence after pruning because their original
581+
authorization cannot be reproduced.
582+
578583
`retained_idempotency_keys_bytes` bounds the serialized memory as well, because
579584
an idempotency key has no length limit on every adapter and the memory outlives
580585
the message row. An actor drops its oldest keys until the list fits, so a key

‎docs/roadmap.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@
126126
lookup by key raises `MessagePruned` for a message that retention removed and
127127
answers `nil` for a message that never existed. A lookup by request ID cannot
128128
make that distinction, because the runtime generates a request ID and no
129-
actor remembers one
129+
actor remembers one. Pruned lookups retain the original arguments for authorization
130+
within the byte limit; older entries without arguments answer `nil`
130131

131132
## Partially implemented
132133

‎lib/solid_objects/client.rb‎

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,27 @@ def find_by(reference: nil, request_id: nil, idempotency_key: nil, authorization
110110
)
111111
end
112112

113-
readable_message(
114-
remembered_message(reference, idempotency_key, authorization_context:),
113+
instance = Instance.find_by(
114+
actor_type: reference.actor_type,
115+
actor_id: reference.actor_id
116+
)
117+
return nil unless instance
118+
119+
message = Message.uncached { Message.find_by(instance_id: instance.id, idempotency_key:) }
120+
return readable_message(message, authorization_context:) if message
121+
122+
remembered = Array(instance.completed_idempotency_keys)
123+
.find { |entry| entry["key"] == idempotency_key }
124+
return nil unless remembered && remembered["arguments"].is_a?(Hash)
125+
return nil unless authorized_to_invoke?(
126+
actor_type: reference.actor_type,
127+
actor_id: reference.actor_id,
128+
operation: remembered["operation"],
129+
arguments: remembered["arguments"],
115130
authorization_context:
116131
)
132+
133+
raise MessagePruned, idempotency_key
117134
end
118135

119136
# @rbs (Reference, ?authorization_context: untyped) -> StateSnapshot
@@ -186,31 +203,6 @@ def enqueue_sync(reference:, operation:, arguments:, idempotency_key:, timeout:)
186203
)
187204
end
188205

189-
# @rbs (Reference, String, authorization_context: untyped) -> Message?
190-
def remembered_message(reference, idempotency_key, authorization_context:)
191-
instance = Instance.find_by(
192-
actor_type: reference.actor_type,
193-
actor_id: reference.actor_id
194-
)
195-
return nil unless instance
196-
197-
message = Message.uncached { Message.find_by(instance_id: instance.id, idempotency_key:) }
198-
return message if message
199-
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"],
207-
arguments: {},
208-
authorization_context:
209-
)
210-
211-
raise MessagePruned, idempotency_key
212-
end
213-
214206
# @rbs (Message?, authorization_context: untyped) -> MessageReference?
215207
def readable_message(message, authorization_context:)
216208
return nil unless message

‎lib/solid_objects/dead_letter_scope.rb‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,19 @@ def self.for_kind(kind)
3030
# @rbs (?authorization_context: untyped) -> Array[DeadRow]
3131
def all(authorization_context: nil)
3232
authorize!(:inspect, authorization_context:)
33-
dead.includes(:instance).order(updated_at: :desc, id: :desc).map { |row| dead_row(row) }
34-
end
35-
36-
# @rbs (untyped) -> DeadRow
37-
def dead_row(row)
38-
DeadRow.new(
39-
id: row.public_send(identifier),
40-
kind:,
41-
actor_type: row.instance.actor_type,
42-
actor_id: row.instance.actor_id,
43-
status: row.status,
44-
attempt_count: row.attempt_count,
45-
available_at: row.available_at,
46-
failed_at: row.updated_at,
47-
error: row.error
48-
)
33+
dead.includes(:instance).order(updated_at: :desc, id: :desc).map do |row|
34+
DeadRow.new(
35+
id: row.public_send(identifier),
36+
kind:,
37+
actor_type: row.instance.actor_type,
38+
actor_id: row.instance.actor_id,
39+
status: row.status,
40+
attempt_count: row.attempt_count,
41+
available_at: row.available_at,
42+
failed_at: row.updated_at,
43+
error: row.error
44+
)
45+
end
4946
end
5047

5148
# @rbs (String, ?authorization_context: untyped) -> untyped

‎lib/solid_objects/executor.rb‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -482,21 +482,17 @@ def remember_key(instance, message)
482482
instance.update!(completed_idempotency_keys: remembered_keys(instance, message))
483483
end
484484

485-
# @rbs (Instance, Message) -> Array[String]
485+
# @rbs (Instance, Message) -> Array[Hash[String, untyped]]
486486
def remembered_keys(instance, message)
487487
remembered = Array(instance.completed_idempotency_keys)
488488
key = message.idempotency_key
489489
return remembered unless key
490490

491-
entry = { "key" => key, "operation" => message.operation }
491+
entry = { "key" => key, "operation" => message.operation, "arguments" => message.arguments }
492492
return remembered if remembered.last == entry
493493

494-
bounded(remembered.reject { |value| value["key"] == key } + [ entry ])
495-
end
496-
497-
# @rbs (Array[String]) -> Array[String]
498-
def bounded(keys)
499-
kept = keys.last(SolidObjects.configuration.retained_idempotency_keys)
494+
kept = (remembered.reject { |value| value["key"] == key } + [ entry ])
495+
.last(SolidObjects.configuration.retained_idempotency_keys)
500496
limit = SolidObjects.configuration.retained_idempotency_keys_bytes
501497
kept.shift while kept.any? && kept.to_json.bytesize > limit
502498
kept

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ module SolidObjects
3232
# @rbs (reference: Reference, operation: Symbol | String, arguments: Hash[Symbol | String, untyped], idempotency_key: String?, timeout: Numeric) -> MessageReference
3333
def enqueue_sync: (reference: Reference, operation: Symbol | String, arguments: Hash[Symbol | String, untyped], idempotency_key: String?, timeout: Numeric) -> MessageReference
3434

35-
# @rbs (Reference, String, authorization_context: untyped) -> Message?
36-
def remembered_message: (Reference, String, authorization_context: untyped) -> Message?
37-
3835
# @rbs (Message?, authorization_context: untyped) -> MessageReference?
3936
def readable_message: (Message?, authorization_context: untyped) -> MessageReference?
4037

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ module SolidObjects
2525
# @rbs (?authorization_context: untyped) -> Array[DeadRow]
2626
def all: (?authorization_context: untyped) -> Array[DeadRow]
2727

28-
# @rbs (untyped) -> DeadRow
29-
def dead_row: (untyped) -> DeadRow
30-
3128
# @rbs (String, ?authorization_context: untyped) -> untyped
3229
def retry: (String, ?authorization_context: untyped) -> untyped
3330

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,8 @@ module SolidObjects
9797
# @rbs (Instance, Message) -> void
9898
def remember_key: (Instance, Message) -> void
9999

100-
# @rbs (Instance, Message) -> Array[String]
101-
def remembered_keys: (Instance, Message) -> Array[String]
102-
103-
# @rbs (Array[String]) -> Array[String]
104-
def bounded: (Array[String]) -> Array[String]
100+
# @rbs (Instance, Message) -> Array[Hash[String, untyped]]
101+
def remembered_keys: (Instance, Message) -> Array[Hash[String, untyped]]
105102

106103
# @rbs (Exception) -> Hash[String, untyped]
107104
def serialized_error: (Exception) -> Hash[String, untyped]

‎test/integration/result_lookup_test.rb‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,33 @@ def reject_checkout
309309
assert_nil reference.find_by(idempotency_key: "checkout-7f3a")
310310
end
311311

312+
test "authorizes pruned keys with the original arguments" do
313+
reference = CartActor.ref("alice")
314+
reference.async(idempotency_key: "protected").checkout(order_id: 1)
315+
run_actors
316+
SolidObjects::Message.delete_all
317+
seen = []
318+
SolidObjects.configuration.authorize_message = lambda do |arguments:, **|
319+
seen << arguments
320+
arguments["order_id"] != 1
321+
end
322+
323+
assert_nil reference.find_by(idempotency_key: "protected")
324+
assert_equal [ { "order_id" => 1 } ], seen
325+
SolidObjects.configuration.authorize_message = ->(arguments:, **) { arguments["order_id"] == 1 }
326+
assert_raises(SolidObjects::MessagePruned) { reference.find_by(idempotency_key: "protected") }
327+
end
328+
329+
test "does not disclose legacy keys without authorization arguments" do
330+
reference = CartActor.ref("alice")
331+
reference.async(idempotency_key: "legacy").checkout(order_id: 1)
332+
run_actors
333+
SolidObjects::Message.delete_all
334+
SolidObjects::Instance.sole.update!(completed_idempotency_keys: [ { "key" => "legacy", "operation" => "checkout" } ])
335+
336+
assert_nil reference.find_by(idempotency_key: "legacy")
337+
end
338+
312339
test "remembers a key whose message was rejected" do
313340
reference = CartActor.ref("alice")
314341
reference.async(idempotency_key: "rejected-7f3a").reject_checkout
@@ -365,7 +392,7 @@ def reject_checkout
365392

366393
remembered = SolidObjects::Instance.sole.completed_idempotency_keys
367394

368-
assert_equal keys.last(2), remembered.map { |entry| entry["key"] }
395+
assert_equal keys.last(1), remembered.map { |entry| entry["key"] }
369396
assert_operator remembered.to_json.bytesize, :<=, 128
370397
end
371398

0 commit comments

Comments
 (0)