[api] trying to fix some things brakeman complains about

src/api/app/controllers/admin_controller.rb

@@ -7,7 +7,7 @@ class AdminController < ApplicationController
 
   def list_blacklist_tags
 
-    @tags = BlacklistTag.find(:all)  
+    @tags = BlacklistTag.all
     @tags ||= []
 
     @number_of_tags = @tags.size
@@ -31,34 +31,34 @@ def list_tags
 
     session[:column] = params[:column] if params[:column]
 
-
     order_by = (session[:column] ||= 'id')
     sort_by = (session[:sort] ||= 'ASC')
 
-
     unless allowed_order_by_arguments.include? order_by
       raise ArgumentError.new( "unknown argument '#{session[:column]}'" )
+    else
+      order = order_by
     end
 
     unless allowed_sort_by_arguments.include? sort_by
       raise ArgumentError.new( "unknown argument '#{session[:sort]}'" )
+    else
+      order = order + " " + sort_by
     end
 
-    logger.debug "[TAG: order_by: #{order_by}"
+    logger.debug "[TAG: order_by: #{order}"
 
-    if order_by == 'count' and sort_by == "ASC"
-      tags = Tag.find(:all)
-
-      @tags = tags.sort { |x,y| x.count <=> y.count }
+    if order_by == 'count'
+      tags = Tag.all
 
-    elsif order_by == 'count' and sort_by == "DESC"
-      tags = Tag.find(:all)
-
-      @tags = tags.sort { |x,y| y.count <=> x.count }
+      if sort_by == "ASC"
+        @tags = tags.sort { |x,y| x.count <=> y.count }
+      else
+        @tags = tags.sort { |x,y| x.count <=> y.count }
+      end
 
     else
-
-      @tags = Tag.find(:all, :order => order_by + ' ' + sort_by)
+      @tags = Tag.order(order).all
 
     end
 
@@ -68,7 +68,7 @@ def list_tags
 
 
   def tags_summary
-    tags = Tag.find(:all)
+    tags = Tag.all
     unused_tags = []
     tags.each do |tag|
       unused_tags << tag if tag.count == 0
@@ -112,8 +112,8 @@ def create_blacklist_tag
 
   def show_tag
     @tag = Tag.find(params[:id])
-    @tagged_projects = @tag.db_projects.find(:all, :group => 'name')
-    @tagged_packages = @tag.db_packages.find(:all, :group => 'name')
+    @tagged_projects = @tag.db_projects.group(:name).all
+    @tagged_packages = @tag.db_packages.group(:name).all
   rescue
     invalid_tag
   end

src/api/app/controllers/apidocs_controller.rb

@@ -16,8 +16,9 @@ def index
   def file
     file = params[:file]
     if ( file =~ /\.(xml|xsd|rng)$/ )
-      if File.exist?( File.expand_path(CONFIG['schema_location']) + "/" + file )
-        send_file( File.expand_path(CONFIG['schema_location']) + "/" + file, :type => "text/xml",
+      file = File.expand( File.join(CONFIG['schema_location'], file) )
+      if File.exist?( file )
+        send_file( file, :type => "text/xml",
           :disposition => "inline" )
       else
         render_error :status => 404, :errorcode => 'file_not_found', :message => 'file was not found'

src/api/app/controllers/application_controller.rb

@@ -22,6 +22,9 @@ class IssueNotFoundError < Exception; end
 
 class ApplicationController < ActionController::Base
 
+  # cross site scripting is rather unlikely for us, but we better play safe
+  protect_from_forgery
+
   # Do never use a layout here since that has impact on every controller
   layout nil
   # session :disabled => true

src/api/app/models/role.rb

@@ -8,7 +8,7 @@
 class Role < ActiveRecord::Base
 
   validates_format_of :title,
-                      :with => %r{^[\w \$\^\-\.#\*\+&'"]*$},
+                      :with => %r{^\w*$},
                       :message => 'must not contain invalid characters.'
   validates_length_of :title,
                       :in => 2..100, :allow_nil => true,
@@ -35,9 +35,8 @@ class Role < ActiveRecord::Base
   has_and_belongs_to_many :groups, :uniq => true
   # roles have n:m relations to permissions
   has_and_belongs_to_many :static_permissions, :uniq => true
-  # protect users and groups from mass assigning - we want to do those
-  # manually
-  attr_protected :users, :static_permissions
+
+  attr_accessible :title, :global
 
   scope :global, where(:global => true)
 

src/api/app/models/tag.rb

@@ -2,18 +2,14 @@ class Tag < ActiveRecord::Base
 
   has_many :taggings, :dependent => :destroy
   has_many :db_projects, :through => :taggings,
-  :conditions => "taggings.taggable_type = 'DbProject'"
+           conditions: "taggings.taggable_type = 'DbProject'"
   has_many :db_packages, :through => :taggings,
-  :conditions => "taggings.taggable_type = 'DbPackage'"
+           conditions: "taggings.taggable_type = 'DbPackage'"
 
   has_many :users, :through => :taggings
 
   attr_accessor :cached_count
 
-  def before_save
-  end
-
-
   def count(opt={})
     if @cached_count 
       #logger.debug "[TAG:] tag usage count is already calculated. count: #{@cached_count}"

src/api/app/views/admin/list_tags.html.erb

@@ -36,16 +36,16 @@
 
     <tr>
      <td>
-        <%= h(tag.id) %>
+        <%= tag.id %>
     </td>
     <td>  
-        <%= h(tag.name) %>
+        <%= tag.name %>
     </td>
     <td>  
-        <%= h(tag.count) %>
+        <%= tag.count %>
     </td>
     <td>  
-        <%= h(tag.created_at) %>
+        <%= tag.created_at %>
     </td>
     <td>
         <%= link_to 'Show', :action => 'show_tag', :id => tag %>