install imagemagick in CI

.github/workflows/ci.yml

@@ -26,10 +26,22 @@ jobs:
     env:
       RAILS_ENV: test
       DATABASE_URL: postgres://postgres:postgres@localhost:5432/theseus_test
+      # no master key in CI, so lockbox/blind_index need throwaway keys
+      LOCKBOX_MASTER_KEY: "0000000000000000000000000000000000000000000000000000000000000000"
+      BLIND_INDEX_MASTER_KEY: "0000000000000000000000000000000000000000000000000000000000000000"
+      HCB_USPS_ORG_ID: org_ci_usps
+      HCB_WAREHOUSE_ORG_ID: org_ci_warehouse
+      HCB_CLIENT_ID: ci_client_id
+      HCB_CLIENT_SECRET: ci_client_secret
 
     steps:
       - uses: actions/checkout@v4
 
+      - run: |
+          sudo apt-get update -qq
+          sudo apt-get install --no-install-recommends -y imagemagick libmagickwand-dev ghostscript
+          sudo sed -i '/disable ghostscript format types/,+6d' /etc/ImageMagick-6/policy.xml
+
       - uses: ruby/setup-ruby@v1
         with:
           ruby-version: "3.4.4"

app/controllers/api/v1/letter_queues_controller.rb

@@ -43,7 +43,6 @@ def create_letter
           letter_params.except(:address).merge(user: current_user),
         )
         render :create_letter, status: :created
-
       end
 
       def create_instant_letter

app/controllers/public_ids_controller.rb

@@ -50,7 +50,7 @@ def lookup
 
     # LSV fallback for tracking numbers (Airtable, not in PublicIdResolver)
     if params[:id].match?(/\A[A-Z0-9]{10,}\z/i)
-      escaped_id = params[:id].gsub('\\') { '\\\\' }.gsub("'") { "\\'" }
+      escaped_id = params[:id].gsub("\\") { "\\\\" }.gsub("'") { "\\'" }
       lsv = LSV::MarketingShipmentRequest.first_where("{Warehouse–Tracking Number} = '#{escaped_id}'")
       return redirect_to show_lsv_path(LSV.slug_for(lsv), lsv.id) if lsv
     end

app/controllers/warehouse/batches_controller.rb

@@ -13,7 +13,7 @@ def index
     batches = all_batches
     batches = batches.where(user_id: params[:user_id]) if params[:user_id].present? && current_user&.is_admin?
     batches = batches.search(params[:search]) if params[:search].present?
-    users = current_user&.is_admin? ? User.where(id: all_batches.reorder(nil).select(:user_id).distinct).order(:email) : []
+    users = current_user&.is_admin? ? User.where(id: all_batches.reorder(nil).unscope(:select).select(:user_id).distinct).order(:email) : []
     render Views::Warehouse::Batches::Index.new(
       batches: batches,
       search: params[:search],

app/controllers/warehouse/orders_controller.rb

@@ -183,5 +183,4 @@ def warehouse_order_params
       address_attributes: %i[first_name last_name line_1 line_2 city state postal_code country phone_number email],
     )
   end
-
 end

app/middleware/maintenance_mode.rb

@@ -18,10 +18,10 @@ def call(env)
     if env["PATH_INFO"].start_with?("/api/")
       body = { error: "service_unavailable", message: "Theseus is down for planned maintenance." }
       body[:message] += " Check the thread in #hq-hq for updates." unless env["PATH_INFO"].start_with?("/api/public/")
-      return [503, { "content-type" => "application/json", "retry-after" => "300" }, [body.to_json]]
+      return [ 503, { "content-type" => "application/json", "retry-after" => "300" }, [ body.to_json ] ]
     end
 
     html = env["PATH_INFO"].start_with?("/back_office") ? BACK_OFFICE_HTML : PUBLIC_HTML
-    [503, { "content-type" => "text/html", "retry-after" => "300" }, [html]]
+    [ 503, { "content-type" => "text/html", "retry-after" => "300" }, [ html ] ]
   end
 end

app/toolboxes/warehouse/orders_toolbox.rb

@@ -95,11 +95,10 @@
       param :address, :object, "Shipping address fields to update", optional: true
     end
     def update
-      billing_profile = resolve_billing_profile(params[:billing_profile_id])
       update_attrs = order_params
-      update_attrs[:billing_profile] = billing_profile if params.key?(:billing_profile_id)
+      update_attrs[:billing_profile] = resolve_billing_profile(params[:billing_profile_id]) if params.key?(:billing_profile_id)
 
       @order.update!(update_attrs)
       render :show
     end
 

app/views/billing/index.rb

@@ -112,7 +112,7 @@ def billing_profiles_table
 
     active = @billing_profiles.select { |p| profile_has_activity?(p) }
     inactive_count = @billing_profiles.size - active.size
-    profiles_to_show = @show_all_profiles ? @billing_profiles.sort_by { |p| p.organization_name.downcase } : active.sort_by { |p| [-p.hcb_transfers.where(state: [ :unknown, :failed ]).count, p.organization_name.downcase] }
+    profiles_to_show = @show_all_profiles ? @billing_profiles.sort_by { |p| p.organization_name.downcase } : active.sort_by { |p| [ -p.hcb_transfers.where(state: [ :unknown, :failed ]).count, p.organization_name.downcase ] }
 
     section(class: "mb-1h") do
       div(class: "toolbar mb-half") do

config/brakeman.ignore

@@ -9,7 +9,7 @@
       "note": "admin-only inspector; awesome_print output passed through sanitize with an explicit tag/attribute allowlist"
     },
     {
-      "fingerprint": "c3445cfef0c4723db5eabf5877e65e4c64a37158e3413a5fe96bad689c3c61b7",
+      "fingerprint": "5ebf484f7d05aa2b6ec7b4e78af7beade81858b7924e28897b8e8c3abe89fff8",
       "note": "tracking_url is built by Tracking.tracking_url_for from a fixed carrier template, not from user input"
     }
   ],

spec/jobs/batch_process_job_spec.rb

@@ -142,7 +142,7 @@ def perform_job
 
         sent = hcb_disbursements.sole
         expect(sent[:profile]).to eq(hcb_account)
-        expect(sent[:amount_cents]).to eq((letters.sum(&:postage) * 100).ceil)
+        expect(sent[:amount_cents]).to eq(letters.sum { |l| (l.postage_for(postage_type: "indicia").to_d * 100).ceil })
         expect(sent[:name]).to start_with("Postage for #{batch.public_id}")
 
         entry = batch.ledger_entries.sole

spec/requests/api/v1/tags_spec.rb

@@ -25,10 +25,10 @@ def body = JSON.parse(response.body)
     it "returns tags from common tags, letters, and warehouse orders" do
       # Create a common tag
       CommonTag.create!(tag: "urgent")
-      
+
       # Create a letter with a tag
-      create(:letter, user: user, tags: ["billing"])
-      
+      create(:letter, user: user, tags: [ "billing" ])
+
       get "/api/v1/tags", headers: headers
 
       expect(response).to have_http_status(:ok)
@@ -47,8 +47,8 @@ def body = JSON.parse(response.body)
   describe "GET /api/v1/tags/:id" do
     it "returns tag statistics for a tag with letters" do
       CommonTag.create!(tag: "marketing")
-      letter = create(:letter, user: user, tags: ["marketing"], postage: 0.58, aasm_state: "mailed")
-      
+      letter = create(:letter, user: user, tags: [ "marketing" ], postage: 0.58, aasm_state: "mailed")
+
       get "/api/v1/tags/marketing", headers: headers
 
       expect(response).to have_http_status(:ok)
@@ -63,12 +63,12 @@ def body = JSON.parse(response.body)
       sku = create(:warehouse_sku, declared_unit_cost_override: 2.0, actual_cost_to_hc: 1.0)
       template = Warehouse::Template.create!(name: "Test", user: user, public: true)
         .tap { |t| t.line_items.create!(sku: sku, quantity: 1) }
-      
+
       profile = create(:billing_profile, user: user)
       order = Warehouse::Order.from_template(
         template, user: user, recipient_email: "test@example.com",
         address: create(:address, country: "US"), billing_profile: profile
-      ).tap { |o| o.save!; o.update_columns(tags: ["fulfillment"], aasm_state: "dispatched") }
+      ).tap { |o| o.save!; o.update_columns(tags: [ "fulfillment" ], aasm_state: "dispatched") }
 
       get "/api/v1/tags/fulfillment", headers: headers
 
@@ -90,7 +90,7 @@ def body = JSON.parse(response.body)
 
     it "requires authentication" do
       CommonTag.create!(tag: "test")
-      
+
       get "/api/v1/tags/test"
 
       expect(response).not_to have_http_status(:ok)
@@ -100,20 +100,20 @@ def body = JSON.parse(response.body)
   describe "GET /api/v1/tags/:id/letters" do
     it "only returns the key owner's letters for a tag" do
       other_user = create(:user)
-      
-      mine = create(:letter, user: user, tags: ["archived"])
-      create(:letter, user: other_user, tags: ["archived"])
+
+      mine = create(:letter, user: user, tags: [ "archived" ])
+      create(:letter, user: other_user, tags: [ "archived" ])
 
       get "/api/v1/tags/archived/letters", headers: headers
 
       expect(response).to have_http_status(:ok)
       expect(body).to have_key("letters")
-      expect(body["letters"].map { |l| l["id"] }).to eq([mine.public_id])
+      expect(body["letters"].map { |l| l["id"] }).to eq([ mine.public_id ])
     end
 
     it "returns empty letters for a tag with no items visible to the user" do
       other_user = create(:user)
-      create(:letter, user: other_user, tags: ["secret"])
+      create(:letter, user: other_user, tags: [ "secret" ])
 
       get "/api/v1/tags/secret/letters", headers: headers
 

spec/requests/api/v1/warehouse_orders_spec.rb

@@ -114,7 +114,7 @@ def payload(idempotency_key: nil, **overrides)
       expect(response).to have_http_status(:ok)
       expect(body).to have_key("warehouse_orders")
       expect(body["warehouse_orders"].count).to eq(2)
-      expect(body["warehouse_orders"].map { |o| o["id"] }).to match_array([order1.hc_id, order2.hc_id])
+      expect(body["warehouse_orders"].map { |o| o["id"] }).to match_array([ order1.hc_id, order2.hc_id ])
     end
 
     it "respects API key scoping — only shows the key owner's orders" do

spec/requests/edge_cases_spec.rb

@@ -123,7 +123,7 @@
       }
 
       # Should fail validation or redirect on error
-      expect([302, 400, 422]).to include(response.status)
+      expect([ 302, 400, 422 ]).to include(response.status)
     end
 
     it "handles CSV with bad UTF-8 encoding gracefully" do
@@ -154,7 +154,7 @@
       # Should not return 500 error
       expect(response.status).not_to eq(500)
       # Should be either validation error (400/422) or redirect (302)
-      expect([302, 400, 422]).to include(response.status)
+      expect([ 302, 400, 422 ]).to include(response.status)
     end
 
     it "accepts valid CSV file upload to letter batch" do
@@ -183,7 +183,7 @@
       }
 
       # Should redirect or show form, not crash (500)
-      expect([302, 400, 422]).to include(response.status)
+      expect([ 302, 400, 422 ]).to include(response.status)
     end
   end
 
@@ -205,7 +205,7 @@
       }
 
       # Should fail validation or redirect on error
-      expect([302, 400, 422]).to include(response.status)
+      expect([ 302, 400, 422 ]).to include(response.status)
     end
 
     it "handles warehouse CSV with bad UTF-8 encoding gracefully" do
@@ -231,7 +231,7 @@
       # Should not return 500 error
       expect(response.status).not_to eq(500)
       # Should be either validation error (400/422) or redirect (302)
-      expect([302, 400, 422]).to include(response.status)
+      expect([ 302, 400, 422 ]).to include(response.status)
     end
   end
 
@@ -240,13 +240,13 @@
       get letter_path(id: "nonexistent")
 
       # Should handle gracefully (either 404 or redirect)
-      expect([302, 404]).to include(response.status)
+      expect([ 302, 404 ]).to include(response.status)
     end
 
     it "does not crash when accessing invalid batch ID" do
       get letter_batch_path(id: "nonexistent")
 
-      expect([302, 404]).to include(response.status)
+      expect([ 302, 404 ]).to include(response.status)
     end
 
     it "returns error when submitting batch with invalid return address" do
@@ -259,7 +259,7 @@
         }
       }
 
-      expect([302, 422]).to include(response.status)
+      expect([ 302, 422 ]).to include(response.status)
       # Error should be present in response
       expect(response.body).to include("error") if response.status == 422
     end
@@ -272,7 +272,7 @@
       get "/letters/#{letter.id}"
 
       # Should render or redirect, not crash (500)
-      expect([200, 302, 403, 404]).to include(response.status)
+      expect([ 200, 302, 403, 404 ]).to include(response.status)
     end
 
     it "renders public package page without crashing" do
@@ -291,7 +291,7 @@
       get "/packages/#{order.id}"
 
       # Should render or redirect, not crash (500)
-      expect([200, 302, 403, 404]).to include(response.status)
+      expect([ 200, 302, 403, 404 ]).to include(response.status)
     end
   end
 end

spec/requests/iv_mtr_webhook_spec.rb

@@ -3,8 +3,8 @@
 require "rails_helper"
 
 RSpec.describe "USPS IV-MTR webhook", type: :request do
+  let(:password) { "iv_mtr_spec_password" }
   let(:headers) do
-    password = Rails.application.credentials.dig(:usps, :iv_mtr, :webhook_password)
     {
       "CONTENT_TYPE" => "application/json",
       "HTTP_AUTHORIZATION" => ActionController::HttpAuthentication::Basic.encode_credentials("my_best_friend_the_informed_visibility_robot", password)
@@ -15,7 +15,11 @@ def ingest(raw) = post("/webhooks/usps/iv_mtr", params: raw, headers: headers)
 
   def body = JSON.parse(response.body)
 
-  before { allow(USPS::IVMTR::ImportEventsJob).to receive(:perform_later) }
+  before do
+    allow(USPS::IVMTR::ImportEventsJob).to receive(:perform_later)
+    allow(Rails.application.credentials).to receive(:dig).and_call_original
+    allow(Rails.application.credentials).to receive(:dig).with(:usps, :iv_mtr, :webhook_password).and_return(password)
+  end
 
   it "accepts a real batch" do
     expect {

spec/requests/letters_spec.rb

@@ -254,7 +254,7 @@ def purchase_indicium!
 
       delete letter_path(standalone)
 
-      expect(response.status).to be_in([302, 303])
+      expect(response.status).to be_in([ 302, 303 ])
       expect { Letter.find(letter_id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end