From 4b4fdf58c7f358fb06bed6dae166b758465850d0 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 4 Jan 2016 23:18:23 -0800
Subject: [PATCH] Fix Error 500 when visiting build page of project with nil
runners_token
Properly ensure that the token exists and add defensively check for a
non-nil value.
Closes #4294
---
CHANGELOG | 1 +
app/models/ci/build.rb | 2 +-
app/models/project.rb | 10 ++++++----
spec/models/ci/build_spec.rb | 22 ++++++++++++++++++++++
4 files changed, 30 insertions(+), 5 deletions(-)
create mode 100644 spec/models/ci/build_spec.rb
diff --git a/CHANGELOG b/CHANGELOG
index f7c278823ee..a7d352b53aa 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ v 8.4.0 (unreleased)
- The default GitLab logo now acts as a loading indicator
- Accept 2xx status codes for successful Web hook triggers (Stan Hu)
- Fix missing date of month in network graph when commits span a month (Stan Hu)
+ - Fix Error 500 when visiting build page of project with nil runners_token (Stan Hu)
- Expire view caches when application settings change (e.g. Gravatar disabled) (Stan Hu)
- Don't notify users twice if they are both project watchers and subscribers (Stan Hu)
- Implement new UI for group page
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 30f79fd3bfa..a4779d06de8 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -206,7 +206,7 @@ module Ci
def trace
trace = raw_trace
- if project && trace.present?
+ if project && trace.present? && project.runners_token.present?
trace.gsub(project.runners_token, 'xxxxxx')
else
trace
diff --git a/app/models/project.rb b/app/models/project.rb
index 7626c698816..0d7341e1384 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -50,6 +50,7 @@ class Project < ActiveRecord::Base
include Sortable
include AfterCommitQueue
include CaseSensitivity
+ include TokenAuthenticatable
extend Gitlab::ConfigHelper
@@ -193,10 +194,7 @@ class Project < ActiveRecord::Base
if: ->(project) { project.avatar.present? && project.avatar_changed? }
validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
- before_validation :set_runners_token_token
- def set_runners_token_token
- self.runners_token = SecureRandom.hex(15) if self.runners_token.blank?
- end
+ add_authentication_token_field :runners_token
mount_uploader :avatar, AvatarUploader
@@ -900,4 +898,8 @@ class Project < ActiveRecord::Base
return true unless forked?
Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i)
end
+
+ def runners_token
+ ensure_runners_token!
+ end
end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
new file mode 100644
index 00000000000..36d10636ae9
--- /dev/null
+++ b/spec/models/ci/build_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe Ci::Build, models: true do
+ let(:build) { create(:ci_build) }
+ let(:test_trace) { 'This is a test' }
+
+ describe '#trace' do
+ it 'obfuscates project runners token' do
+ allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}")
+
+ expect(build.trace).to eq("Test: xxxxxx")
+ end
+
+ it 'empty project runners token' do
+ allow(build).to receive(:raw_trace).and_return(test_trace)
+ # runners_token can't normally be set to nil
+ allow(build.project).to receive(:runners_token).and_return(nil)
+
+ expect(build.trace).to eq(test_trace)
+ end
+ end
+end
--
GitLab