From 6c22561e964d08887a8f15e4f0cae7f74ae66bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Wed, 2 Sep 2026 14:35:12 -0600 Subject: [PATCH 1/2] Serve the lockfile page as HTML only A request for /lockfiles/:id.json reached the web controller, which rendered :show_new unconditionally and raised ActionView::MissingTemplate (500), since only show_new.html.erb exists. The JSON representation is served by API::LockfilesController on the api subdomain, so the web action now responds to HTML only. Other formats raise ActionController::UnknownFormat, which Rails maps to 406 and sentry-rails excludes from reporting by default. --- app/controllers/lockfiles_controller.rb | 14 +++++++++----- spec/controllers/lockfiles_controller_spec.rb | 10 +++++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/controllers/lockfiles_controller.rb b/app/controllers/lockfiles_controller.rb index d80a540..ca27182 100644 --- a/app/controllers/lockfiles_controller.rb +++ b/app/controllers/lockfiles_controller.rb @@ -12,11 +12,15 @@ def create end def show - if FeatureFlags.new_check_flow? - @lockfile = Lockfile.includes(lockfile_checks: [:rails_release, :gem_checks]).find_by!(slug: params[:id]) - render :show_new - else - @lockfile = Lockfile.find_by!(slug: params[:id]) + respond_to do |format| + format.html do + if FeatureFlags.new_check_flow? + @lockfile = Lockfile.includes(lockfile_checks: [:rails_release, :gem_checks]).find_by!(slug: params[:id]) + render :show_new + else + @lockfile = Lockfile.find_by!(slug: params[:id]) + end + end end end diff --git a/spec/controllers/lockfiles_controller_spec.rb b/spec/controllers/lockfiles_controller_spec.rb index 01807b8..6f6fdeb 100644 --- a/spec/controllers/lockfiles_controller_spec.rb +++ b/spec/controllers/lockfiles_controller_spec.rb @@ -178,5 +178,13 @@ get :show, params: { id: lockfile.to_param } expect(response).to be_successful end + + # UnknownFormat is mapped to 406 by Rails and excluded from Sentry by + # default, unlike the MissingTemplate 500 this replaces. + it "raises UnknownFormat (406) for non-HTML formats instead of MissingTemplate" do + expect { + get :show, params: { id: lockfile.to_param }, format: :json + }.to raise_error(ActionController::UnknownFormat) + end end -end \ No newline at end of file +end From ff9c95e9208eef3599ee4fad1eaa2e7220cfdecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Wed, 2 Sep 2026 14:51:56 -0600 Subject: [PATCH 2/2] Stop crashing on a scalar lockfile param API::LockfilesController#create called fetch on the result of params.require(:lockfile), which returns a String when a request sends lockfile as a bare scalar instead of the nested lockfile[content] hash, raising NoMethodError (500). It now accepts both shapes and lets Lockfile::Inspection answer with 422 invalid_content. The web controller had the same crash through String#permit, reachable by a hand-crafted POST though not by the form. It now uses params.expect, which raises ParameterMissing (400) for unexpected shapes. ParameterMissing is in sentry-rails' default ignore list, so these requests stop paging us. --- app/controllers/api/lockfiles_controller.rb | 10 +++++++++- app/controllers/lockfiles_controller.rb | 2 +- .../api/lockfiles_controller_spec.rb | 20 +++++++++++++++++++ spec/controllers/lockfiles_controller_spec.rb | 8 ++++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/lockfiles_controller.rb b/app/controllers/api/lockfiles_controller.rb index 0674a32..670eaaa 100644 --- a/app/controllers/api/lockfiles_controller.rb +++ b/app/controllers/api/lockfiles_controller.rb @@ -51,8 +51,16 @@ def poll_after_seconds(lockfile) estimate.clamp(MIN_POLL_SECONDS, MAX_POLL_SECONDS) end + # Some requests arrive with `lockfile` as a bare scalar instead of the + # nested `lockfile[content]` hash. Accept both and let Inspection + # reject anything that is not a Gemfile.lock. def lockfile_content - params.require(:lockfile).fetch(:content, "").to_s.strip + lockfile = params[:lockfile] + + case lockfile + when ActionController::Parameters then lockfile[:content] + when String then lockfile + end.to_s.strip end end end diff --git a/app/controllers/lockfiles_controller.rb b/app/controllers/lockfiles_controller.rb index ca27182..78852dd 100644 --- a/app/controllers/lockfiles_controller.rb +++ b/app/controllers/lockfiles_controller.rb @@ -53,6 +53,6 @@ def create_legacy_flow end def lockfile_params - params.require(:lockfile).permit(:content) + params.expect(lockfile: [:content]) end end diff --git a/spec/controllers/api/lockfiles_controller_spec.rb b/spec/controllers/api/lockfiles_controller_spec.rb index 672ea87..170b666 100644 --- a/spec/controllers/api/lockfiles_controller_spec.rb +++ b/spec/controllers/api/lockfiles_controller_spec.rb @@ -76,6 +76,26 @@ end end + context "when lockfile is sent as a scalar instead of a hash" do + it "returns 422 with reason invalid_content instead of raising" do + expect { + post :create, params: { lockfile: "not a hash" }, as: :json + }.not_to raise_error + + expect(response).to have_http_status(:unprocessable_content) + expect(JSON.parse(response.body)["reason"]).to eq("invalid_content") + end + end + + context "when the lockfile param is missing entirely" do + it "returns 422 with reason invalid_content" do + post :create, params: {}, as: :json + + expect(response).to have_http_status(:unprocessable_content) + expect(JSON.parse(response.body)["reason"]).to eq("invalid_content") + end + end + context "when the lockfile is already on the latest known Rails" do it "returns 200 with reason up_to_date and errors, and does not persist the lockfile" do FactoryBot.create(:rails_release, version: "7.1") diff --git a/spec/controllers/lockfiles_controller_spec.rb b/spec/controllers/lockfiles_controller_spec.rb index 6f6fdeb..ae39a16 100644 --- a/spec/controllers/lockfiles_controller_spec.rb +++ b/spec/controllers/lockfiles_controller_spec.rb @@ -187,4 +187,12 @@ }.to raise_error(ActionController::UnknownFormat) end end + + describe "POST #create with a scalar lockfile param" do + it "raises ParameterMissing (400) instead of NoMethodError" do + expect { + post :create, params: { lockfile: "not a hash" } + }.to raise_error(ActionController::ParameterMissing) + end + end end