Security fix: added is_shared parameter

10 files changed, 58 insertions, 19 deletions

diff --git a/CHANGELOG b/CHANGELOG
index beba00e..fdc1efe 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,9 @@
 v7.10.0
   - Projects sorting by last commit date
 
+v7.9.1
+  - [Security] Adding explicit is_shared parameter to runner
+
 v7.9.0
   - Reset user session if token is invalid
   - Runner delete api endpoint
diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb
index a6ef86e..8997d14 100644
--- a/app/controllers/admin/runners_controller.rb
+++ b/app/controllers/admin/runners_controller.rb
@@ -13,7 +13,7 @@ class Admin::RunnersController < Admin::ApplicationController
   end
 
   def update
-    @runner.update_attributes(params[:runner])
+    @runner.update_attributes(runner_params)
 
     respond_to do |format|
       format.js
@@ -56,4 +56,8 @@ class Admin::RunnersController < Admin::ApplicationController
   def runner
     @runner ||= Runner.find(params[:id])
   end
+
+  def runner_params
+    params.require(:runner).permit(:token, :description, :tag_list, :contacted_at, :active)
+  end
 end
diff --git a/app/controllers/runners_controller.rb b/app/controllers/runners_controller.rb
index 4780069..02465d6 100644
--- a/app/controllers/runners_controller.rb
+++ b/app/controllers/runners_controller.rb
@@ -15,9 +15,6 @@ class RunnersController < ApplicationController
   end
 
   def update
-    runner_params = params[:runner]
-    runner_params.delete(:token)
-
     if @runner.update_attributes(runner_params)
       redirect_to edit_project_runner_path(@project, @runner), notice: 'Runner was successfully updated.'
     else
@@ -58,4 +55,8 @@ class RunnersController < ApplicationController
   def set_runner
     @runner ||= @project.runners.find(params[:id])
   end
+
+  def runner_params
+    params.require(:runner).permit(:description, :tag_list, :contacted_at, :active)
+  end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index 629c4fe..89439ac 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -30,7 +30,7 @@ class Project < ActiveRecord::Base
     :default_ref, :gitlab_url, :always_build, :polling_interval,
     :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch, :skip_refs,
     :email_recipients, :email_add_pusher, :email_only_broken_builds, :coverage_regex,
-    :jobs_attributes
+    :jobs_attributes, :shared_runners_enabled
 
   has_many :commits, dependent: :destroy
   has_many :builds, through: :commits, dependent: :destroy
diff --git a/app/models/runner.rb b/app/models/runner.rb
index e1f685d..7b32cea 100644
--- a/app/models/runner.rb
+++ b/app/models/runner.rb
@@ -17,12 +17,10 @@ class Runner < ActiveRecord::Base
 
   has_one :last_build, ->() { order('id DESC') }, class_name: 'Build'
 
-  attr_accessible :token, :description, :tag_list, :contacted_at, :active
-
   before_validation :set_default_values
 
-  scope :specific, ->() { where(id: RunnerProject.select(:runner_id)) }
-  scope :shared, ->() { where.not(id: RunnerProject.select(:runner_id)) }
+  scope :specific, ->() { where(is_shared: false) }
+  scope :shared, ->() { where(is_shared: true) }
   scope :active, ->() { where(active: true) }
   scope :paused, ->() { where(active: false) }
 
@@ -43,7 +41,7 @@ class Runner < ActiveRecord::Base
   end
 
   def shared?
-    runner_projects.blank?
+    is_shared
   end
 
   def only_for?(project)
diff --git a/app/views/projects/_form.html.haml b/app/views/projects/_form.html.haml
index 3702f7e..f04ca33 100644
--- a/app/views/projects/_form.html.haml
+++ b/app/views/projects/_form.html.haml
@@ -61,6 +61,13 @@
             = f.check_box :public
             %span.light Anyone can see project and builds
     .form-group
+      = f.label :shared_runners_enabled, 'Allow shared runners', class: 'control-label'
+      .col-sm-10
+        .checkbox
+          = f.label :shared_runners_enabled do
+            = f.check_box :shared_runners_enabled
+            %span.light Allow run builds on shared runners
+    .form-group
       = f.label :coverage_regex, "Test coverage parsing", class: 'control-label'
       .col-sm-10
         .input-group
diff --git a/db/migrate/20150324001123_add_settings_for_shared_runners.rb b/db/migrate/20150324001123_add_settings_for_shared_runners.rb
new file mode 100644
index 0000000..559ca20
--- /dev/null
+++ b/db/migrate/20150324001123_add_settings_for_shared_runners.rb
@@ -0,0 +1,6 @@
+class AddSettingsForSharedRunners < ActiveRecord::Migration
+  def change
+    add_column :projects, :shared_runners_enabled, :boolean, default: false
+    add_column :runners, :is_shared, :boolean, default: false
+  end
+end
diff --git a/db/migrate/20150324001227_migrate_shared_runners.rb b/db/migrate/20150324001227_migrate_shared_runners.rb
new file mode 100644
index 0000000..1d86aa7
--- /dev/null
+++ b/db/migrate/20150324001227_migrate_shared_runners.rb
@@ -0,0 +1,11 @@
+class MigrateSharedRunners < ActiveRecord::Migration
+  def up
+    #all shared runners should remain to be shared
+    execute("UPDATE runners SET is_shared = true WHERE id NOT IN (SELECT runner_id FROM runner_projects)");
+
+    Project.update_all(shared_runners_enabled: true)
+  end
+
+  def down
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index b76eaa0..7ed96ff 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,27 +11,24 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150310001733) do
-
-  # These are extensions that must be enabled in order to support this database
-  enable_extension "plpgsql"
+ActiveRecord::Schema.define(version: 20150324001227) do
 
   create_table "builds", force: true do |t|
     t.integer  "project_id"
     t.string   "ref"
     t.string   "status"
     t.datetime "finished_at"
-    t.text     "trace"
+    t.text     "trace",       limit: 2147483647
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "sha"
     t.datetime "started_at"
     t.string   "tmp_file"
     t.string   "before_sha"
-    t.text     "push_data"
+    t.text     "push_data",   limit: 16777215
     t.integer  "runner_id"
     t.integer  "commit_id"
-    t.float    "coverage"
+    t.float    "coverage",    limit: 24
     t.text     "commands"
     t.integer  "job_id"
   end
@@ -57,6 +54,15 @@ ActiveRecord::Schema.define(version: 20150310001733) do
   add_index "commits", ["project_id"], name: "index_commits_on_project_id", using: :btree
   add_index "commits", ["sha"], name: "index_commits_on_sha", using: :btree
 
+  create_table "events", force: true do |t|
+    t.integer  "project_id"
+    t.integer  "user_id"
+    t.integer  "is_admin"
+    t.text     "description"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+  end
+
   create_table "jobs", force: true do |t|
     t.integer  "project_id",                          null: false
     t.text     "commands"
@@ -91,6 +97,7 @@ ActiveRecord::Schema.define(version: 20150310001733) do
     t.boolean  "email_only_broken_builds", default: true,  null: false
     t.string   "skip_refs"
     t.string   "coverage_regex"
+    t.boolean  "shared_runners_enabled",   default: false
   end
 
   create_table "runner_projects", force: true do |t|
@@ -109,7 +116,8 @@ ActiveRecord::Schema.define(version: 20150310001733) do
     t.datetime "updated_at"
     t.string   "description"
     t.datetime "contacted_at"
-    t.boolean  "active",       default: true, null: false
+    t.boolean  "active",       default: true,  null: false
+    t.boolean  "is_shared",    default: false
   end
 
   create_table "services", force: true do |t|
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index 34cf041..0ec4871 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -47,7 +47,8 @@ module API
             # Create shared runner. Requires admin access
             Runner.create(
               description: params[:description],
-              tag_list: params[:tag_list]
+              tag_list: params[:tag_list],
+              is_shared: true
             )
           elsif project = Project.find_by(token: params[:token])
             # Create a specific runner for project.