Staging into Main 20250430

.cursorrules

@@ -57,6 +57,7 @@
   - Implement proper authentication and authorization (e.g., Devise, Pundit).
   - Use strong parameters in controllers.
   - Protect against common web vulnerabilities (XSS, CSRF, SQL injection).
+  - Use Brakeman to scan for security vulnerabilities.
 
   Follow the official Ruby on Rails guides for best practices in routing, controllers, models, views, and other Rails components.
 

.github/workflows/brakeman.yml

@@ -0,0 +1,30 @@
+name: Brakeman Security Scan
+
+on:
+  push:
+    branches: [ main, master ]
+  pull_request:
+    branches: [ main, master ]
+  schedule:
+    - cron: '0 0 * * 0'  # Run weekly on Sunday
+
+jobs:
+  security:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: '3.3.4'
+          bundler-cache: true
+
+      - name: Run Brakeman
+        run: bundle exec brakeman -A -q -o tmp/brakeman-report.json
+
+      - name: Upload Brakeman report
+        uses: actions/upload-artifact@v3
+        with:
+          name: brakeman-report
+          path: tmp/brakeman-report.json

.gitignore

@@ -49,3 +49,6 @@ uploads/
 # Ignore working_files directory and all its contents
 /working_files/
 /working_files/**/*
+
+# Ignore Brakeman reports
+/tmp/brakeman-report.*

Gemfile

@@ -38,6 +38,7 @@ group :development do
   gem 'annotate'
   gem 'better_errors'
   gem 'binding_of_caller'
+  gem 'brakeman'
   gem 'capistrano', '~> 3.17', require: false
   gem 'capistrano-rails', '~> 1.6', '>= 1.6.1', require: false
   gem 'capistrano-asdf', require: false

Gemfile.lock

@@ -93,6 +93,8 @@ GEM
       debug_inspector (>= 1.2.0)
     bootsnap (1.18.4)
       msgpack (~> 1.2)
+    brakeman (7.0.2)
+      racc
     builder (3.3.0)
     byebug (11.1.3)
     capistrano (3.19.1)
@@ -258,7 +260,7 @@ GEM
     mysql2 (0.5.6)
     net-http (0.4.1)
       uri
-    net-imap (0.4.19)
+    net-imap (0.4.20)
       date
       net-protocol
     net-pop (0.1.2)
@@ -551,6 +553,7 @@ DEPENDENCIES
   better_errors
   binding_of_caller
   bootsnap
+  brakeman
   capistrano (~> 3.17)
   capistrano-asdf
   capistrano-rails (~> 1.6, >= 1.6.1)

app/controllers/addresses_controller.rb

@@ -60,7 +60,7 @@ def destroy
   private
 
   def set_address
-    @address = Address.find(params[:id])
+    @address = policy_scope(Address).find(params[:id])
   end
 
   def address_params

app/controllers/application_controller.rb

@@ -43,7 +43,7 @@ def handle_exceptions
     user_not_authorized(exception)
   rescue ActiveRecord::RecordNotFound => exception
     Rails.logger.info('!!!!!!! Handling ActiveRecord::RecordNotFound in ApplicationController')
-    redirect_to not_found_path
+    redirect_to root_path, alert: '!!! Not authorized !!!'
   end
 
   # Private method for handling Pundit not authorized errors

app/controllers/assignments_controller.rb

@@ -52,7 +52,7 @@ def destroy
   private
 
   def set_container
-    @container = Container.find(params[:container_id])
+    @container = policy_scope(Container).find(params[:container_id])
   end
 
   def assignment_params

app/controllers/bulk_contest_instances_controller.rb

@@ -38,7 +38,7 @@ def create
   private
 
   def set_container
-    @container = Container.find(params[:container_id])
+    @container = policy_scope(Container).find(params[:container_id])
   end
 
   def authorize_container_access

app/controllers/containers_controller.rb

@@ -1,9 +1,12 @@
 # app/controllers/containers_controller.rb
 class ContainersController < ApplicationController
   include ContestDescriptionsHelper
+  after_action :verify_authorized, except: :lookup_user
   before_action :set_container, only: %i[show edit update destroy description active_applicants_report]
-  before_action :authorize_container, only: %i[edit show update destroy description]
-  before_action :authorize_index, only: [ :index ]
+  before_action :authorize_container, only: %i[show edit destroy]
+  before_action :authorize_new_container, only: :new
+  before_action :authorize_admin_content, only: :admin_content
+  before_action :authorize_index, only: :index
 
   def index
     @containers = policy_scope(Container)
@@ -143,20 +146,28 @@ def active_applicants_report
 
   private
 
+  def set_container
+    @container = policy_scope(Container).find(params[:id])
+  end
+
+  def container_params
+    params.require(:container).permit(:name, :description, :notes, :contact_email, :department_id, :visibility_id,
+                                      assignments_attributes: %i[id user_id role_id _destroy])
+  end
+
   def authorize_container
     authorize @container
   end
 
-  def set_container
-    @container = Container.find(params[:id])
+  def authorize_new_container
+    authorize Container, :new?
   end
 
-  def authorize_index
-    authorize Container
+  def authorize_admin_content
+    authorize Container, :admin_content?
   end
 
-  def container_params
-    params.require(:container).permit(:name, :description, :notes, :contact_email, :department_id, :visibility_id,
-                                      assignments_attributes: %i[id user_id role_id _destroy])
+  def authorize_index
+    authorize Container
   end
 end

app/controllers/contest_descriptions_controller.rb

@@ -79,11 +79,11 @@ def handle_save(success, action)
   end
 
   def set_container
-    @container = Container.find(params[:container_id])
+    @container = policy_scope(Container).find(params[:container_id])
   end
 
   def set_contest_description
-    @contest_description = ContestDescription.find(params[:id])
+    @contest_description = policy_scope(ContestDescription).find(params[:id])
   end
 
   def contest_description_params

app/controllers/contest_instances_controller.rb

@@ -79,89 +79,101 @@ def destroy
   end
 
   def email_preferences
-    set_contest_instance
-    authorize @contest_instance, :send_round_results?
+    begin
+      set_contest_instance
+      authorize @contest_instance, :send_round_results?
 
-    round_id = params[:round_id]
-    @judging_round = @contest_instance.judging_rounds.find_by(id: round_id)
+      round_id = params[:round_id]
+      @judging_round = @contest_instance.judging_rounds.find_by(id: round_id)
 
-    if @judging_round.nil?
-      redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
-                 alert: 'Judging round not found.'
-      return
-    end
+      if @judging_round.nil?
+        redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
+                   alert: 'Judging round not found.'
+        return
+      end
 
-    if !@judging_round.completed?
-      redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
-                 alert: 'Cannot send results for an incomplete judging round.'
-      nil
+      if !@judging_round.completed?
+        redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
+                   alert: 'Cannot send results for an incomplete judging round.'
+        nil
+      end
+    rescue Pundit::NotAuthorizedError
+      flash[:alert] = 'Not authorized to perform this action'
+      redirect_to root_path
     end
   end
 
   def send_round_results
-    authorize @contest_instance, :send_round_results?
+    begin
+      authorize @contest_instance, :send_round_results?
 
-    round_id = params[:round_id]
-    judging_round = @contest_instance.judging_rounds.find_by(id: round_id)
+      round_id = params[:round_id]
+      judging_round = @contest_instance.judging_rounds.find_by(id: round_id)
 
-    if judging_round.nil?
-      redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
-                 alert: 'Judging round not found.'
-      return
-    end
+      if judging_round.nil?
+        redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
+                  alert: 'Judging round not found.'
+        return
+      end
 
-    if !judging_round.completed?
-      redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
-                 alert: 'Cannot send results for an incomplete judging round.'
-      return
-    end
+      if !judging_round.completed?
+        redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
+                  alert: 'Cannot send results for an incomplete judging round.'
+        return
+      end
 
-    # Update email preferences if they were provided
-    if params[:include_average_ranking].present? || params[:include_advancement_status].present?
-      judging_round.update(
-        include_average_ranking: params[:include_average_ranking] == '1',
-        include_advancement_status: params[:include_advancement_status] == '1'
-      )
-    end
+      # Update email preferences if they were provided
+      if params[:include_average_ranking].present? || params[:include_advancement_status].present?
+        judging_round.update(
+          include_average_ranking: params[:include_average_ranking] == '1',
+          include_advancement_status: params[:include_advancement_status] == '1'
+        )
+      end
 
-    # Get all entries for this round
-    entries = judging_round.entries.uniq
+      # Get all entries for this round
+      entries = judging_round.entries.uniq
 
-    email_count = 0
+      email_count = 0
 
-    # Send an email for each entry
-    entries.each do |entry|
-      mail = ResultsMailer.entry_evaluation_notification(entry, judging_round)
-      mail.deliver_later
-      email_count += 1
-    end
+      # Send an email for each entry
+      entries.each do |entry|
+        mail = ResultsMailer.entry_evaluation_notification(entry, judging_round)
+        mail.deliver_later
+        email_count += 1
+      end
 
-    # Increment the emails sent counter for this round
-    judging_round.increment!(:emails_sent_count)
+      # Increment the emails sent counter for this round
+      judging_round.increment!(:emails_sent_count)
 
-    redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
-               notice: "Successfully queued #{email_count} evaluation result emails for round #{judging_round.round_number}. This is email batch ##{judging_round.emails_sent_count}."
+      redirect_to container_contest_description_contest_instance_path(@container, @contest_description, @contest_instance),
+                notice: "Successfully queued #{email_count} evaluation result emails for round #{judging_round.round_number}. This is email batch ##{judging_round.emails_sent_count}."
+    rescue Pundit::NotAuthorizedError
+      flash[:alert] = 'Not authorized to perform this action'
+      redirect_to root_path
+    end
   end
 
   def export_entries
-    @contest_instance = ContestInstance.find(params[:id])
-    @contest_description = @contest_instance.contest_description
-    @container = @contest_description.container
-
-    authorize @contest_instance
+    begin
+      @contest_instance = @contest_description.contest_instances.find(params[:id])
+      authorize @contest_instance, :export_entries?
 
-    @entries = @contest_instance.entries.active.includes(:profile, :category)
+      @entries = @contest_instance.entries.active.includes(:profile, :category)
 
-    respond_to do |format|
-      format.csv do
-        filename = "#{@contest_description.name.parameterize}-entries_printed-#{Time.zone.today}.csv"
+      respond_to do |format|
+        format.csv do
+          filename = "#{@contest_description.name.parameterize}-entries_printed-#{Time.zone.today}.csv"
 
-        csv_data = generate_entries_csv(@entries, @contest_description, @contest_instance)
+          csv_data = generate_entries_csv(@entries, @contest_description, @contest_instance)
 
-        send_data csv_data,
-                  type: 'text/csv; charset=utf-8; header=present',
-                  disposition: "attachment; filename=#{filename}"
+          send_data csv_data,
+                    type: 'text/csv; charset=utf-8; header=present',
+                    disposition: "attachment; filename=#{filename}"
+        end
       end
+    rescue Pundit::NotAuthorizedError
+      flash[:alert] = 'Not authorized to access this contest instance'
+      redirect_to root_path
     end
   end
 
@@ -176,7 +188,7 @@ def set_contest_instance
   end
 
   def set_container
-    @container = Container.find(params[:container_id])
+    @container = policy_scope(Container).find(params[:container_id])
   end
 
   def set_contest_description

app/controllers/entries_controller.rb

@@ -1,6 +1,7 @@
 class EntriesController < ApplicationController
   include AvailableContestsConcern
-  before_action :set_entry, only: %i[ show edit update destroy soft_delete toggle_disqualified applicant_profile ]
+  before_action :set_entry, only: %i[ show edit update destroy soft_delete toggle_disqualified ]
+  before_action :set_entry_for_profile, only: %i[ applicant_profile ]
   before_action :authorize_entry, only: %i[show edit update destroy]
   before_action :authorize_index, only: [ :index ]
 
@@ -117,10 +118,10 @@ def applicant_profile
     if @profile.user == current_user || current_user.axis_mundi?
       @entries = Entry.active.where(profile: @profile)
     else
-      # For container administrators, only show entries from their containers
+      # For container administrators and managers, only show entries from their containers
+      admin_role_ids = Role.where(kind: ['Collection Administrator', 'Collection Manager']).pluck(:id)
       admin_container_ids = current_user.assignments
-                                      .where(role: 'container_admin')
-                                      .joins(:container)
+                                      .where(role_id: admin_role_ids)
                                       .pluck(:container_id)
 
       @entries = Entry.active
@@ -133,6 +134,11 @@ def applicant_profile
   private
     # Use callbacks to share common setup or constraints between actions.
     def set_entry
+      @entry = policy_scope(Entry).find(params[:id])
+    end
+
+    # For applicant_profile, we want to find the entry first, then authorize it
+    def set_entry_for_profile
       @entry = Entry.find(params[:id])
     end