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 d80a540..78852dd 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 @@ -49,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 01807b8..ae39a16 100644 --- a/spec/controllers/lockfiles_controller_spec.rb +++ b/spec/controllers/lockfiles_controller_spec.rb @@ -178,5 +178,21 @@ 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 + + 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 \ No newline at end of file +end