Skip to content

Commit 71b86c8

Browse files
committed
fix(certification): guard bonus_stardust against bad input and slim controller
1 parent c88a4d4 commit 71b86c8

3 files changed

Lines changed: 23 additions & 12 deletions

File tree

app/controllers/admin/certification/ships_controller.rb

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,10 @@ def set_project_type
8888

8989
def set_bonus_stardust
9090
authorize @ship
91-
value = params[:bonus_stardust].presence
92-
@ship.bonus_stardust = value.present? ? value.to_f : nil
93-
94-
# If the ship already has a verdict, recalculate stardust_earned to
95-
# include the updated bonus.
96-
if @ship.stardust_earned.present? && !@ship.pending?
97-
base = @ship.stardust_earned - (@ship.bonus_stardust_was || 0)
98-
@ship.stardust_earned = base + (@ship.bonus_stardust || 0)
99-
end
100-
101-
@ship.save!
91+
@ship.update_bonus_stardust!(params[:bonus_stardust].presence)
10292
redirect_to admin_certification_ship_path(@ship), notice: "Bonus stardust updated."
93+
rescue ArgumentError, ActiveRecord::RecordInvalid => e
94+
redirect_to admin_certification_ship_path(@ship), alert: "Invalid bonus stardust: #{e.message}"
10395
end
10496

10597
def update

app/models/certification/ship.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ def owner
9797
validates :feedback, length: { maximum: 10_000 }, allow_blank: true
9898
validates :verdict_video,
9999
content_type: { in: ACCEPTED_VIDEO_TYPES, spoofing_protection: true }
100+
validates :bonus_stardust,
101+
numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 100 },
102+
allow_nil: true
100103

101104
scope :for_reviewer, ->(user) {
102105
joins(:project)
@@ -358,6 +361,22 @@ def decided_on
358361
decided_at || updated_at
359362
end
360363

364+
365+
def update_bonus_stardust!(value)
366+
self.bonus_stardust = if value.present?
367+
parsed = Float(value)
368+
raise ArgumentError, "bonus_stardust must be >= 0" if parsed.negative?
369+
parsed
370+
end
371+
372+
if stardust_earned.present? && !pending?
373+
base = stardust_earned - (bonus_stardust_was || 0)
374+
self.stardust_earned = base + (bonus_stardust || 0)
375+
end
376+
377+
save!
378+
end
379+
361380
private
362381

363382
def assign_stardust_earned

app/views/admin/certification/ships/show.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@
109109
<%= form_with url: set_bonus_stardust_admin_certification_ship_path(@ship), method: :patch, class: "bonus-stardust__form" do %>
110110
<div class="bonus-stardust__field">
111111
<%= number_field_tag :bonus_stardust, @ship.bonus_stardust,
112-
step: 0.25, min: 0, placeholder: "0.0",
112+
step: 0.25, min: 0, max: 100, placeholder: "0.0",
113113
autocomplete: "off",
114114
class: "bonus-stardust__input",
115115
id: "bonus-stardust-input" %>

0 commit comments

Comments
 (0)