Skip to content

Commit

Permalink
Merge branch 'fix-brakeman-audit-failures' of github.com:fiveNinePlus…
Browse files Browse the repository at this point in the history
…R/pbm
  • Loading branch information
RyanTG committed Oct 1, 2024
2 parents fc8ff4d + 5d17168 commit af6dd2e
Show file tree
Hide file tree
Showing 19 changed files with 178 additions and 32 deletions.
6 changes: 3 additions & 3 deletions app/controllers/api/v1/location_machine_xrefs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ def create

return return_response(AUTH_REQUIRED_MSG, 'errors') if user.nil?

location_id = params[:location_id]
machine_id = params[:machine_id]
location_id = params[:location_id].to_i
machine_id = params[:machine_id].to_i
condition = params[:condition]
status_code = 200

return return_response('Failed to find machine', 'errors') if machine_id.nil? || location_id.nil? || !Machine.exists?(machine_id) || !Location.exists?(location_id)
return return_response('Failed to find machine', 'errors') if machine_id.zero? || location_id.zero? || !Machine.exists?(machine_id) || !Location.exists?(location_id)

lmx = LocationMachineXref.find_by_location_id_and_machine_id(location_id, machine_id)

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/location_picture_xrefs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def show
def create
return return_response(AUTH_REQUIRED_MSG, 'errors') if current_user.nil?

location_id = params[:location_id]
return return_response('Failed to find location', 'errors') if location_id.nil? || !Location.exists?(location_id)
location_id = params[:location_id].to_i
return return_response('Failed to find location', 'errors') if location_id.zero? || !Location.exists?(location_id)

photo = params[:photo]
return return_response('Missing photo to add', 'errors') if photo.nil?
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/machines_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ def autocomplete
sanitized_sql = ActiveRecord::Base.sanitize_sql_array([sql, { term: params[:term] }])

results = ActiveRecord::Base.connection.select_all(sanitized_sql)
.map do |m|
name_year = "#{m['name']} (#{m['manufacturer']}, #{m['year']})"
.map do |m|
name_year = "#{m['name']} (#{m['manufacturer']}, #{m['year']})"

{ label: name_year, value: name_year, id: m['id'], group_id: m['machine_group_id'] }
end
{ label: name_year, value: name_year, id: m['id'], group_id: m['machine_group_id'] }
end

end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def map

params[:user_faved] = user.id if user && !params[:user_faved].blank?

if !params[:by_location_id].blank? && loc = Location.where(id: params[:by_location_id]).first
if !params[:by_location_id].blank? && (loc = Location.where(id: params[:by_location_id]).first)
@title_params[:title] = loc.name
location_type = loc.location_type.name + ' - ' unless loc.location_type.nil?
machine_list = ' - ' + loc.machine_names_first_no_year.join(', ') unless loc.machine_names_first_no_year.empty?
Expand All @@ -74,7 +74,7 @@ def region
@location_count = @locations.count
@lmx_count = @region.machines_count

if !params[:by_location_id].blank? && loc = Location.where(id: params[:by_location_id]).first
if !params[:by_location_id].blank? && (loc = Location.where(id: params[:by_location_id]).first)
@title_params[:title] = loc.name
location_type = loc.location_type.name + ' - ' unless loc.location_type.nil?
machine_list = ' - ' + loc.machine_names_first_no_year.join(', ') unless loc.machine_names_first_no_year.empty?
Expand Down
4 changes: 2 additions & 2 deletions app/models/location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ class Location < ApplicationRecord

validates_presence_of :name, :street, :city, :country
validates :phone, phone: { possible: true, allow_blank: true, message: 'Phone format not valid.' }
validates :website, format: { with: %r{http(s?)://}, message: 'must begin with http:// or https://' }, if: :website?
validates :name, :street, :city, format: { with: /^\S.*/, message: "Can't start with a blank", multiline: true }
validates :website, format: { with: %r{\Ahttp(s?)://}, message: 'must begin with http:// or https://' }, if: :website?
validates :name, :street, :city, format: { with: /\A\S.*/, message: "Can't start with a blank", multiline: true }
validates :lat, :lon, presence: { message: 'Latitude/Longitude failed to generate. Please double check address and try again, or manually enter the lat/lon' }

belongs_to :location_type, optional: true
Expand Down
8 changes: 4 additions & 4 deletions app/models/machine_score_xref.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ class MachineScoreXref < ApplicationRecord

scope :zone_id, lambda { |id|
joins(:location_machine_xref).joins(:location).where("
locations.zone_id = #{id}
")
locations.zone_id = ?
", id)
}

scope :region, lambda { |name|
r = Region.find_by_name(name.downcase)
joins(:location_machine_xref).joins(:location).where("
location_machine_xrefs.id = machine_score_xrefs.location_machine_xref_id
and locations.id = location_machine_xrefs.location_id
and locations.region_id = #{r.id}
")
and locations.region_id = ?
", r.id)
}

def username
Expand Down
12 changes: 7 additions & 5 deletions app/models/suggested_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ class SuggestedLocation < ApplicationRecord
validates_presence_of :name, :machines, on: :create
validates_presence_of :street, :city, :zip, on: :update

validates :website, format: { with: %r{http(s?)://}, message: 'must begin with http:// or https://' }, if: :website?, on: :update
validates :name, :street, :city, format: { with: /^\S.*/, message: "Can't start with a blank", multiline: true }, on: :update
validates :website, format: { with: %r{\Ahttp(s?)://}, message: 'must begin with http:// or https://' }, if: :website?, on: :update
validates :name, :street, :city, format: { with: /\A\S.*/, message: "Can't start with a blank", multiline: true }, on: :update
validates :lat, :lon, presence: { message: 'Latitude/Longitude failed to generate. Please double check address and try again, or manually enter the lat/lon' }, on: :update

belongs_to :region, optional: true
Expand Down Expand Up @@ -97,19 +97,21 @@ def convert_to_location(user_email)

delete

ActiveRecord::Base.connection.execute(<<HERE)
sql = <<HERE
insert into versions values (
nextval('versions_id_seq'),
'Location',
#{location.id},
:location_id,
'converted from suggested location',
'#{user_email}',
:user_email,
NULL,
now(),
NULL,
NULL
)
HERE
sanitized_sql = ActiveRecord::Base.sanitize_sql_array([sql, { location_id: location.id, user_email: user_email }])
ActiveRecord::Base.connection.execute(sanitized_sql)
end
end
end
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class User < ApplicationRecord

validates :username, presence: true, uniqueness: { case_sensitive: false }

validates_format_of :username, with: /^[a-zA-Z0-9_\.]*$/, multiline: true
validates_format_of :username, with: /\A[a-zA-Z0-9_\.]*\z/, multiline: true
validates :username, length: { maximum: 20 }
strip_attributes only: %i[username password]

Expand Down
2 changes: 1 addition & 1 deletion app/views/locations/_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
:javascript
$(function () {
$('#by_#{key}_name').autocomplete({
source: '/#{key}s/autocomplete?region_level_search=1;region=#{params[:region]}',
source: '/#{key}s/autocomplete?region_level_search=1;region=#{h(params[:region])}',
minLength: 2,
delay: 500
});
Expand Down
2 changes: 1 addition & 1 deletion app/views/locations/_locations.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
[#{[@lat, @lon].join(', ')}]
);

var hrefOrig = '#{request.scheme}://#{request.host_with_port}/#{@region ? @region.name.downcase : @operators_map ? @operators_map : "map"}?';
var hrefOrig = '#{request.scheme}://#{request.host_with_port}/#{@region ? h(@region.name.downcase) : @operators_map ? @operators_map : "map"}?';
var def_value = window.location.href;

var url = '';
Expand Down
27 changes: 27 additions & 0 deletions bin/bundle-audit
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

#
# This file was generated by Bundler.
#
# The application 'bundle-audit' is installed as part of a gem, and
# this file is here to facilitate running it.
#

ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__)

bundle_binstub = File.expand_path("bundle", __dir__)

if File.file?(bundle_binstub)
if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
load(bundle_binstub)
else
abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
end
end

require "rubygems"
require "bundler/setup"

load Gem.bin_path("bundler-audit", "bundle-audit")
27 changes: 27 additions & 0 deletions bin/bundler-audit
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

#
# This file was generated by Bundler.
#
# The application 'bundler-audit' is installed as part of a gem, and
# this file is here to facilitate running it.
#

ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__)

bundle_binstub = File.expand_path("bundle", __dir__)

if File.file?(bundle_binstub)
if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
load(bundle_binstub)
else
abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
end
end

require "rubygems"
require "bundler/setup"

load Gem.bin_path("bundler-audit", "bundler-audit")
94 changes: 94 additions & 0 deletions config/brakeman.ignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
{
"ignored_warnings": [
{
"warning_type": "Format Validation",
"warning_code": 30,
"fingerprint": "06f9be88a8af832b07095516dcafa96909e3f7770c63c803906d29d94ed9f5fc",
"check_name": "ValidationRegex",
"message": "Insufficient validation for `website` using `/\\Ahttp(s?):\\/\\//`. Use `\\A` and `\\z` as anchors",
"file": "app/models/location.rb",
"line": 6,
"link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
"code": null,
"render_path": null,
"location": {
"type": "model",
"model": "Location"
},
"user_input": null,
"confidence": "High",
"cwe_id": [
777
],
"note": ""
},
{
"warning_type": "Format Validation",
"warning_code": 30,
"fingerprint": "06f9be88a8af832b07095516dcafa96909e3f7770c63c803906d29d94ed9f5fc",
"check_name": "ValidationRegex",
"message": "Insufficient validation for `name` using `/\\A\\S.*/`. Use `\\A` and `\\z` as anchors",
"file": "app/models/location.rb",
"line": 7,
"link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
"code": null,
"render_path": null,
"location": {
"type": "model",
"model": "Location"
},
"user_input": null,
"confidence": "High",
"cwe_id": [
777
],
"note": ""
},
{
"warning_type": "Format Validation",
"warning_code": 30,
"fingerprint": "8b632b14ee9e862130a926be342087e1f1ec9d174f244c2d61fc2a4514afd2f7",
"check_name": "ValidationRegex",
"message": "Insufficient validation for `website` using `/\\Ahttp(s?):\\/\\//`. Use `\\A` and `\\z` as anchors",
"file": "app/models/suggested_location.rb",
"line": 8,
"link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
"code": null,
"render_path": null,
"location": {
"type": "model",
"model": "SuggestedLocation"
},
"user_input": null,
"confidence": "High",
"cwe_id": [
777
],
"note": ""
},
{
"warning_type": "Format Validation",
"warning_code": 30,
"fingerprint": "8b632b14ee9e862130a926be342087e1f1ec9d174f244c2d61fc2a4514afd2f7",
"check_name": "ValidationRegex",
"message": "Insufficient validation for `name` using `/\\A\\S.*/`. Use `\\A` and `\\z` as anchors",
"file": "app/models/suggested_location.rb",
"line": 9,
"link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
"code": null,
"render_path": null,
"location": {
"type": "model",
"model": "SuggestedLocation"
},
"user_input": null,
"confidence": "High",
"cwe_id": [
777
],
"note": ""
}
],
"updated": "2024-09-29 19:42:12 -0700",
"brakeman_version": "6.2.1"
}
2 changes: 1 addition & 1 deletion config/initializers/cookies_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

# Specify a serializer for the signed and encrypted cookie jars.
# Valid options are :json, :marshal, and :hybrid.
Rails.application.config.action_dispatch.cookies_serializer = :hybrid
Rails.application.config.action_dispatch.cookies_serializer = :json
1 change: 0 additions & 1 deletion spec/features/location_machine_xrefs_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,6 @@
end

it 'allows case insensitive searches of a region' do

chicago_region = FactoryBot.create(:region, name: 'chicago', full_name: 'Chicago')
FactoryBot.create(:location, id: 23, region: chicago_region, name: 'Chicago Location')

Expand Down
2 changes: 0 additions & 2 deletions spec/features/locations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@
before(:each) do
@location = FactoryBot.create(:location, region_id: @region.id, name: 'Cleo')
@machine = FactoryBot.create(:machine, name: 'Bawb')

end

it 'removes a machine from a location' do
Expand Down Expand Up @@ -531,7 +530,6 @@
@user = FactoryBot.create(:user)
login(@user)


@location = FactoryBot.create(:location, region: @region, name: 'Cleo')
end

Expand Down
2 changes: 1 addition & 1 deletion spec/features/pages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@
visit "/#{@region.name}/high_rollers"
expect(page).to have_title('High Scores')

visit "/map/?by_location_id=1234"
visit '/map/?by_location_id=1234'
expect(page).to have_title('Pinball Map')

visit "/map/?by_location_id=#{@location.id}"
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/api/v1/locations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@
it 'respects no_details and shows fewer location fields' do
lmx = FactoryBot.create(:location_machine_xref, location: @location, machine: FactoryBot.create(:machine, id: 7777, name: 'Cleo'))
FactoryBot.create(:machine_condition, location_machine_xref_id: lmx.id, comment: 'foo bar')
FactoryBot.create(:machine_score_xref, location_machine_xref: lmx, score: 567890)
FactoryBot.create(:machine_score_xref, location_machine_xref: lmx, score: 567_890)
get "/api/v1/locations/#{@location.id}.json", params: { no_details: 1 }

expect(response.body).to include('Satchmo')
Expand Down
1 change: 0 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
include Sprockets::Rails::Helper
include ActiveSupport::Testing::TimeHelpers


SimpleCov.start
Coveralls.wear!

Expand Down

0 comments on commit af6dd2e

Please sign in to comment.