prevent path traversal via filesystem scm

Setting up a filesystem scm now requires to specify a whitelist via the configuration

app/models/repository/filesystem.rb

@@ -33,6 +33,9 @@ class Repository::Filesystem < Repository
   attr_protected :root_url
   validates_presence_of :url
 
+  validate :validate_whitelisted_url,
+           :validate_url_is_dir
+
   ATTRIBUTE_KEY_NAMES = {
       "url"          => "Root directory",
     }
@@ -60,4 +63,30 @@ class Repository::Filesystem < Repository
     nil
   end
 
+  private
+
+  # validates that the url is a directory
+  def validate_url_is_dir
+    errors.add :url, :no_directory unless Dir.exists?(url)
+  end
+
+  # validate url against whitelisted urls as provided by the
+  # scm_filesystem_path_whitelist configuration parameter.
+  #
+  # The url needs to exist and needs to match one of the directories
+  # returned when globbing the configuration setting.
+  def validate_whitelisted_url
+    globbed_url = Dir.glob(url).first
+
+    unless globbed_url
+      errors.add :url, :not_whitelisted
+      return
+    end
+
+    globbed_whitelisted = Dir.glob(OpenProject::Configuration["scm_filesystem_path_whitelist"])
+
+    unless globbed_whitelisted.include?(globbed_url)
+      errors.add :url, :not_whitelisted
+    end
+  end
 end

config/configuration.yml.example

@@ -156,6 +156,24 @@ default:
   # scm_git_command:        /usr/local/bin/git                        # (default: git)
   # scm_subversion_command:
   # scm_git_command:
+  #
+  # Enable filesystem base SCM by specifying the directories that are available
+  # to serve as an SCM directory. The format in which the available directories
+  # can be specified is close to a shell glob.  Please see
+  # http://ruby-doc.org/core-2.2.0/Dir.html#method-c-glob for details.
+  #
+  # Multiple patterns can be configured.
+  #
+  # If no directory is specified, there will be no option to configure a
+  # filesystem based SCM within OpenProject.
+  #
+  # Please note that this allows users to browse the filesystem which could
+  # lead to sensitive information (e.g. db credentials) being made accessible
+  # to them.
+  #
+  # scm_filesystem_path_whitelist:
+  #   - "/opt/app/openproject/repositories/*"
+  #   - "/tmp/**/*"
 
   # Key used to encrypt sensitive data in the database (SCM and LDAP passwords).
   # If you don't want to enable data encryption, just leave it blank.

config/locales/de.yml

@@ -237,6 +237,8 @@ de:
         project_association:
           identical_projects: "kann nicht von einem Projekt auf sich selbst erstellt werden"
           project_association_not_allowed: "erlaubt keine Projekt-Abhängigkeiten"
+        repository:
+          not_whitelisted: "wird durch die Konfiguration nicht erlaubt"
         user:
           attributes:
             password:

config/locales/en.yml

@@ -222,6 +222,9 @@ en:
         project_association:
           identical_projects: "can not be created from one project to itself"
           project_association_not_allowed: "does not allow associations"
+        repository:
+          not_whitelisted: "is not allowed by the configuration"
+          no_directory: "is not a directory"
         work_package:
           is_not_a_valid_target_for_time_entries: "Work package #%{id} is not a valid target for reassigning the time entries."
           attributes:

features/step_definitions/repository_steps.rb

@@ -31,8 +31,15 @@ Given(/^the project "(.*?)" has a repository$/) do |project_name|
 
   project = Project.find(project_name)
 
+  repo_path = Rails.root.join 'tmp/filesystem_repository'
+
+  FileUtils.mkdir_p repo_path
+
+  OpenProject::Configuration['scm_filesystem_path_whitelist'] = [repo_path]
+
   repo = FactoryGirl.build(:repository,
-                           :project => project)
+                           url: repo_path,
+                           project: project)
 
   Setting.enabled_scm = Setting.enabled_scm << repo.scm_name
 

lib/open_project/configuration.rb

@@ -40,6 +40,7 @@ module OpenProject
       'autologin_cookie_path'   => '/',
       'autologin_cookie_secure' => false,
       'database_cipher_key'     => nil,
+      'scm_filesystem_path_whitelist' => [],
       'scm_git_command'         => nil,
       'scm_subversion_command'  => nil,
       'disable_browser_cache'   => true,

spec/controllers/repositories_controller_spec.rb

@@ -29,15 +29,67 @@
 require 'spec_helper'
 
 describe RepositoriesController, :type => :controller do
-  let(:project) { FactoryGirl.create(:project) }
-  let(:user) { FactoryGirl.create(:user, :member_in_project => project,
-                                          :member_through_role => role) }
-  let(:repository) { FactoryGirl.create(:repository, :project => project) }
+  let(:project) do
+    project = FactoryGirl.create(:project)
+    allow(Project).to receive(:find).and_return(project)
+    project
+  end
 
-  before do
+  let(:user) do
+    FactoryGirl.create(:user, member_in_project: project,
+                              member_through_role: role)
+  end
+
+  let(:repository) do
     allow(Setting).to receive(:enabled_scm).and_return(['Filesystem'])
-    repository  # ensure repository is created after stubbing the setting
+    repo = FactoryGirl.build_stubbed(:repository,
+                                     :project => project)
+    allow(repo).to receive(:default_branch).and_return('master')
+    allow(repo).to receive(:branches).and_return(['master'])
+
+    repo
+  end
+
+  before do
     allow(User).to receive(:current).and_return(user)
+    allow(project).to receive(:repository).and_return(repository)
+  end
+
+  describe '#edit' do
+    let(:role) { FactoryGirl.create(:role, :permissions => [:manage_repository]) }
+
+    before do
+      # authorization checked in spec/permissions/manage_repositories_spec.rb
+      allow(controller).to receive(:authorize).and_return(true)
+    end
+
+    shared_examples_for 'successful response' do
+      it 'is successful' do
+        expect(response).to be_success
+      end
+
+      it 'renders the template' do
+        expect(response).to render_template 'projects/settings/repository'
+      end
+    end
+
+    context 'GET' do
+      before do
+        xhr :get, :edit
+      end
+
+      it_behaves_like 'successful response'
+    end
+
+    context 'POST' do
+      before do
+        allow(repository).to receive(:save).and_return(true)
+
+        xhr :post, :edit
+      end
+
+      it_behaves_like 'successful response'
+    end
   end
 
   describe 'commits per author graph' do

spec/factories/repository_factory.rb

@@ -27,7 +27,7 @@
 #++
 
 FactoryGirl.define do
-  factory :repository, :class => Repository::Filesystem do
+  factory :repository, class: Repository::Filesystem do
     # Setting.enabled_scm should include "Filesystem" to successfully save the created repository
     url 'file:///tmp/test_repo'
     project

spec/lib/open_project/text_formatting_spec.rb

@@ -67,13 +67,20 @@ describe OpenProject::TextFormatting do
     end
 
     context "Changeset links" do
-      let(:repository) { FactoryGirl.create :repository, :project => project }
-      let(:changeset1) { FactoryGirl.create :changeset,
-                                            :repository => repository,
-                                            :comments => 'My very first commit' }
-      let(:changeset2) { FactoryGirl.create :changeset,
-                                            :repository => repository,
-                                            :comments => 'This commit fixes #1, #2 and references #1 & #3' }
+      let(:repository) do
+        FactoryGirl.build_stubbed :repository,
+                                  project: project
+      end
+      let(:changeset1) do
+        FactoryGirl.build_stubbed :changeset,
+                                  repository: repository,
+                                  comments: 'My very first commit'
+      end
+      let(:changeset2) do
+        FactoryGirl.build_stubbed :changeset,
+                                  repository: repository,
+                                  comments: 'This commit fixes #1, #2 and references #1 & #3'
+      end
       let(:changeset_link) { link_to("r#{changeset1.revision}",
                                      {:controller => 'repositories', :action => 'revision', :project_id => identifier, :rev => changeset1.revision},
                                      :class => 'changeset', :title => 'My very first commit') }
@@ -82,7 +89,18 @@ describe OpenProject::TextFormatting do
                                       :class => 'changeset', :title => 'This commit fixes #1, #2 and references #1 & #3') }
 
       before do
-        project.repository = repository
+        allow(project).to receive(:repository).and_return(repository)
+
+        changesets = [changeset1, changeset2]
+
+        allow(Changeset).to receive(:visible).and_return(changesets)
+
+        changesets.each do |changeset|
+          allow(changesets)
+            .to receive(:find_by_repository_id_and_revision)
+            .with(project.repository.id, changeset.revision)
+            .and_return(changeset)
+        end
       end
 
       context "Single link" do
@@ -393,12 +411,12 @@ describe OpenProject::TextFormatting do
     end
 
     context "Redmine links" do
-      let(:repository) { FactoryGirl.create :repository, :project => project }
+      let(:repository) { FactoryGirl.build_stubbed :repository, :project => project }
       let(:source_url) { {:controller => 'repositories', :action => 'entry', :project_id => identifier, :path => 'some/file'} }
       let(:source_url_with_ext) { {:controller => 'repositories', :action => 'entry', :project_id => identifier, :path => 'some/file.ext'} }
 
       before do
-        project.repository = repository
+        allow(project).to receive(:repository).and_return(repository)
 
         @to_test = {
           # source

spec/models/changeset_spec.rb

@@ -30,14 +30,14 @@ require 'spec_helper'
 
 describe Changeset, :type => :model do
   let(:email) { "bob@bobbit.org" }
-  let(:repo) { FactoryGirl.create(:repository) }
+
+  with_created_filesystem_repository do
     let(:changeset) { FactoryGirl.build(:changeset,
-                                      repository: repo,
+                                        repository: repository,
                                         revision: '1',
                                         committer: email,
                                         comments: "Initial commit") }
-
-  before { allow(Setting).to receive(:enabled_scm).and_return(['Filesystem']) }
+  end
 
   shared_examples_for 'valid changeset' do
     it { expect(changeset.revision).to eq('1') }

spec/models/repository/filesystem_spec.rb

@@ -0,0 +1,128 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2014 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe Repository::Filesystem, type: :model do
+  before do
+    allow(Setting).to receive(:enabled_scm).and_return(['Filesystem'])
+  end
+
+  let(:instance) { described_class.new }
+
+  describe '#valid?' do
+    def mock_dirs_exist(input, output)
+      allow(Dir).to receive(:glob).with(input).and_return(output)
+      allow(Dir).to receive(:exists?).with(input).and_return(true)
+    end
+
+    let(:desired_url) { 'something' }
+    let(:whitelisted_urls) { 'another_thing' }
+
+    let(:valid_args) do
+      { url: desired_url,
+        path_encoding: 'US-ASCII' }
+    end
+    let(:expected_url_whitelist_error_message) do
+      [I18n.t('activerecord.errors.models.repository.not_whitelisted')]
+    end
+    let(:expected_url_not_directory_error_message) do
+      [I18n.t('activerecord.errors.models.repository.no_directory')]
+    end
+
+    before do
+      mock_dirs_exist(desired_url, ['/this/will/match'])
+      mock_dirs_exist(whitelisted_urls, ['/this/will/match'])
+
+      allow(OpenProject::Configuration).to receive(:[])
+                                       .with('scm_filesystem_path_whitelist')
+                                       .and_return(whitelisted_urls)
+
+      instance.attributes = valid_args
+    end
+
+    subject { instance }
+
+    it 'is valid' do
+      is_expected.to be_valid
+    end
+
+    context 'url not whitelisted' do
+      before do
+        mock_dirs_exist(desired_url, ['/desired/dir'])
+        mock_dirs_exist(whitelisted_urls, ['/desired',
+                                           '/desired/*',
+                                           '/desired/di',
+                                           '/desired/dir/1',
+                                           '*'])
+      end
+
+      it 'is invalid' do
+        instance.attributes = valid_args
+
+        is_expected.to be_invalid
+        expect(subject.errors[:url]).to eql(expected_url_whitelist_error_message)
+      end
+    end
+
+    context 'url is not a directory' do
+      before do
+        allow(Dir).to receive(:exists?).with(desired_url).and_return(false)
+      end
+
+      it 'is invalid' do
+        is_expected.to be_invalid
+        expect(subject.errors[:url]).to eql(expected_url_not_directory_error_message)
+      end
+    end
+
+    context 'url does not exist' do
+      before do
+        mock_dirs_exist(desired_url, [])
+      end
+
+      it 'is invalid' do
+        is_expected.to be_invalid
+        expect(subject.errors[:url]).to eql(expected_url_whitelist_error_message)
+      end
+    end
+
+    context 'nothing is whitelisted' do
+      before do
+        mock_dirs_exist(whitelisted_urls, [])
+      end
+
+      it 'is invalid' do
+        instance.attributes = valid_args
+
+        is_expected.to be_invalid
+        expect(subject.errors[:url]).to eql(expected_url_whitelist_error_message)
+      end
+    end
+  end
+end

spec/models/user_deletion_spec.rb

@@ -379,24 +379,28 @@ describe User, 'deletion', :type => :model do
   end
 
   describe "WHEN the user has created a changeset" do
-    let(:repository) { FactoryGirl.create(:repository) }
-    let(:associated_instance) { FactoryGirl.build(:changeset, :repository_id => repository.id,
-                                                          :committer => user.login) }
+    with_created_filesystem_repository do
+      let(:associated_instance) do
+        FactoryGirl.build(:changeset,
+                          repository_id: repository.id,
+                          committer: user.login)
+      end
+
 
       let(:associated_class) { Changeset }
       let(:associations) { [:user] }
 
-    before do
-      Setting.enabled_scm = Setting.enabled_scm << "Filesystem"
-    end
-
       it_should_behave_like "created journalized associated object"
     end
+  end
 
   describe "WHEN the user has updated a changeset" do
-    let(:repository) { FactoryGirl.create(:repository) }
-    let(:associated_instance) { FactoryGirl.build(:changeset, :repository_id => repository.id,
-                                                          :committer => user2.login) }
+    with_created_filesystem_repository do
+      let(:associated_instance) do
+        FactoryGirl.build(:changeset,
+                          repository_id: repository.id,
+                          committer: user2.login)
+      end
 
       let(:associated_class) { Changeset }
       let(:associations) { [:user] }
@@ -429,6 +433,7 @@ describe User, 'deletion', :type => :model do
         expect(associated_instance.journals.last.changed_data[:user_id].last).to eq(substitute_user.id)
       end
     end
+  end
 
   describe "WHEN the user is responsible for a project" do
     before do

spec/permissions/manage_repositories_spec.rb

@@ -0,0 +1,36 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2014 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+require File.expand_path('../../spec_helper', __FILE__)
+require File.expand_path('../../support/permission_specs', __FILE__)
+
+describe RepositoriesController, "manage_repository permission", type: :controller do
+  include PermissionSpecs
+
+  check_permission_required_for('repositories#edit', :manage_repository)
+end

spec/support/filesystem_repo.rb

@@ -0,0 +1,51 @@
+#-- encoding: UTF-8
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2014 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+def with_created_filesystem_repository(&block)
+  let(:repository) do
+    repo = FactoryGirl.build(:repository)
+
+    # ignoring the bugs on url as those are expected:
+    # 1) directory is not existing
+    # 2) configuration is not whitelisting the directory
+    if repo.valid? || (repo.errors.keys - [:url]).empty?
+      repo.save(validate: false)
+    else
+      repo.save!
+    end
+
+    repo
+  end
+
+  before do
+    allow(Setting).to receive(:enabled_scm).and_return(["Filesystem"])
+  end
+
+  block.call
+end

test/functional/repositories_filesystem_controller_test.rb

@@ -35,8 +35,6 @@ class RepositoriesController; def rescue_action(e) raise e end; end
 class RepositoriesFilesystemControllerTest < ActionController::TestCase
   fixtures :all
 
-  # No '..' in the repository path
-  REPOSITORY_PATH = Rails.root.to_s.gsub(%r{config\/\.\.}, '') + '/tmp/test/filesystem_repository'
   PRJ_ID = 3
 
   def setup
@@ -45,17 +43,20 @@ class RepositoriesFilesystemControllerTest < ActionController::TestCase
     @request    = ActionController::TestRequest.new
     @response   = ActionController::TestResponse.new
     User.current = nil
-    Setting.enabled_scm = Setting.enabled_scm.dup << 'Filesystem' unless Setting.enabled_scm.include?('Filesystem')
+
+    with_existing_filesystem_scm do |repo_path|
       @repository = Repository::Filesystem.create(
           :project => Project.find(PRJ_ID),
-                      :url     => REPOSITORY_PATH,
+          :url     => repo_path,
           :path_encoding => nil
           )
+    end
+
     assert @repository
   end
 
-  if File.directory?(REPOSITORY_PATH)
   def test_browse_root
+    with_existing_filesystem_scm do
       @repository.fetch_changesets
       @repository.reload
       get :show, :project_id => PRJ_ID
@@ -66,8 +67,10 @@ class RepositoriesFilesystemControllerTest < ActionController::TestCase
       assert_not_nil assigns(:changesets)
       assert assigns(:changesets).size == 0
     end
+  end
 
   def test_show_no_extension
+    with_existing_filesystem_scm do
       get :entry, :project_id => PRJ_ID, :path => 'test'
       assert_response :success
       assert_template 'entry'
@@ -76,14 +79,18 @@ class RepositoriesFilesystemControllerTest < ActionController::TestCase
                  :attributes => { :class => 'line-num' },
                  :sibling => { :tag => 'td', :content => /TEST CAT/ }
     end
+  end
 
   def test_entry_download_no_extension
+    with_existing_filesystem_scm do |repo_path|
       get :entry, :project_id => PRJ_ID, :path => 'test', :format => 'raw'
       assert_response :success
       assert_equal 'application/octet-stream', @response.content_type
     end
+  end
 
   def test_show_non_ascii_contents
+    with_existing_filesystem_scm do
       with_settings :repositories_encodings => 'UTF-8,EUC-JP' do
         get :entry, :project_id => PRJ_ID, :path => 'japanese/euc-jp.txt'
         assert_response :success
@@ -94,8 +101,10 @@ class RepositoriesFilesystemControllerTest < ActionController::TestCase
                    :sibling => { :tag => 'td', :content => /japanese/ }
       end
     end
+  end
 
   def test_show_utf16
+    with_existing_filesystem_scm do
       with_settings :repositories_encodings => 'UTF-16' do
         get :entry, :project_id => PRJ_ID, :path => 'japanese/utf-16.txt'
         assert_response :success
@@ -106,19 +115,17 @@ class RepositoriesFilesystemControllerTest < ActionController::TestCase
           end
           assert_select "td", :content => /japanese/
         end
-
+      end
     end
   end
 
   def test_show_text_file_should_send_if_too_big
+    with_existing_filesystem_scm do
       with_settings :file_max_size_displayed => 1 do
         get :entry, :project_id => PRJ_ID, :path => 'japanese/big-file.txt'
         assert_response :success
         assert_equal 'text/plain', @response.content_type
       end
     end
-  else
-    puts "Filesystem test repository NOT FOUND. Skipping functional tests !!!"
-    def test_fake; assert true end
   end
 end

test/functional/user_mailer_test.rb

@@ -106,7 +106,8 @@ class UserMailerTest < ActionMailer::TestCase
                     "https://mydomain.foo/work_packages/#{related_issue.id}",
                     "My related Ticket (#{related_issue.status})",
                     :text => "##{related_issue.id}"
-      # link to a changeset
+      # link to a changeset if present
+      if changeset
         assert_select 'a[href=?][title=?]',
                       url_for(:controller => 'repositories',
                               :action => 'revision',
@@ -114,6 +115,7 @@ class UserMailerTest < ActionMailer::TestCase
                               :rev => changeset.revision),
                       'This commit fixes #1, #2 and references #1 and #3',
                       :text => "r#{changeset.revision}"
+      end
       # link to an attachment
       assert_select 'a[href=?]',
                     "https://mydomain.foo/attachments/#{attachment.id}/download",
@@ -147,7 +149,8 @@ class UserMailerTest < ActionMailer::TestCase
                     "http://mydomain.foo/rdm/work_packages/#{related_issue.id}",
                     "My related Ticket (#{related_issue.status})",
                     :text => "##{related_issue.id}"
-      # link to a changeset
+      # link to a changeset if present
+      if changeset
         assert_select 'a[href=?][title=?]',
                       url_for(:controller => 'repositories',
                               :action => 'revision',
@@ -155,6 +158,7 @@ class UserMailerTest < ActionMailer::TestCase
                               :rev => changeset.revision),
                       'This commit fixes #1, #2 and references #1 and #3',
                       :text => "r#{changeset.revision}"
+      end
       # link to an attachment
       assert_select 'a[href=?]',
                     "http://mydomain.foo/rdm/attachments/#{attachment.id}/download",
@@ -192,6 +196,7 @@ class UserMailerTest < ActionMailer::TestCase
                     "My related Ticket (#{related_issue.status})",
                     :text => "##{related_issue.id}"
       # link to a changeset
+      if changeset
         assert_select 'a[href=?][title=?]',
                       url_for(:controller => 'repositories',
                               :action => 'revision',
@@ -199,6 +204,7 @@ class UserMailerTest < ActionMailer::TestCase
                               :rev => changeset.revision),
                       'This commit fixes #1, #2 and references #1 and #3',
                       :text => "r#{changeset.revision}"
+      end
       # link to an attachment
       assert_select 'a[href=?]',
                     "http://mydomain.foo/rdm/attachments/#{attachment.id}/download",
@@ -532,16 +538,28 @@ private
         :project => project,
         :description => "nothing here yet")
 
-    # now change the issue, to get a nice journal
-    # we create a Filesystem repository for our changeset, so we have to enable it
-    Setting.enabled_scm = Setting.enabled_scm.dup << 'Filesystem' unless Setting.enabled_scm.include?('Filesystem')
-    changeset = FactoryGirl.create :changeset,
-                                   :repository => FactoryGirl.create(:repository, :project => project),
+    issue.description = "This is related to issue ##{related_issue.id}\n"
+
+    changeset = with_existing_filesystem_scm do |repo_url|
+      repository = FactoryGirl.build(:repository,
+                                     url: repo_url,
+                                     project: project)
+
+      repository.save!
+
+      FactoryGirl.create :changeset,
+                         :repository => repository,
                          :comments => 'This commit fixes #1, #2 and references #1 and #3'
+    end
+
+    issue.description += " A reference to a changeset r#{changeset.revision}\n" if changeset
+
     attachment = FactoryGirl.create(:attachment,
         :container => issue,
         :author => issue.author)
-    issue.description = "This is related to issue ##{related_issue.id}\n A reference to a changeset r#{changeset.revision}\n A reference to an attachment attachment:#{attachment.filename}"
+
+    issue.description += " A reference to an attachment attachment:#{attachment.filename}"
+
     assert issue.save
     issue.reload
     journal = issue.journals.last

test/test_helper.rb

@@ -171,6 +171,20 @@ class ActiveSupport::TestCase
     saved_settings.each {|k, v| Setting[k] = v}
   end
 
+  REPOSITORY_PATH = Rails.root.to_s.gsub(%r{config\/\.\.}, '') + '/tmp/test/filesystem_repository'
+
+  def with_existing_filesystem_scm(&block)
+    if Dir.exists?(REPOSITORY_PATH)
+      Setting.enabled_scm = Setting.enabled_scm << "Filesystem" unless Setting.enabled_scm.include? "Filesystem"
+      OpenProject::Configuration["scm_filesystem_path_whitelist"] = [REPOSITORY_PATH]
+
+      block.call(REPOSITORY_PATH)
+    else
+      warn "Filesystem test repository NOT FOUND. Skipping tests !!! See doc/RUNNING_TESTS."
+      nil
+    end
+  end
+
   def change_user_password(login, new_password)
     user = User.first(:conditions => {:login => login})
     user.password, user.password_confirmation = new_password, new_password

test/unit/changeset_test.rb

@@ -235,10 +235,12 @@ class ChangesetTest < ActiveSupport::TestCase
 
   def test_comments_should_be_converted_to_utf8
     with_settings :enabled_scm => ['Filesystem'] do
+      with_existing_filesystem_scm do |repo_url|
         proj = Project.find(3)
         str = File.read(Rails.root.join('test/fixtures/encoding/iso-8859-1.txt'))
         r = Repository::Filesystem.create!(
-            :project => proj, :url => '/tmp/test/filesystem_repository',
+              :project => proj,
+              :url => repo_url,
               :log_encoding => 'ISO-8859-1' )
         assert r
         c = Changeset.new(:repository => r,
@@ -250,14 +252,16 @@ class ChangesetTest < ActiveSupport::TestCase
         assert_equal "Texte encodé en ISO-8859-1.", c.comments
       end
     end
+  end
 
   def test_invalid_utf8_sequences_in_comments_should_be_replaced_latin1
     with_settings :enabled_scm => ['Filesystem'] do
+      with_existing_filesystem_scm do |repo_url|
         proj = Project.find(3)
         str = File.read(Rails.root.join('test/fixtures/encoding/iso-8859-1.txt'))
         r = Repository::Filesystem.create!(
               :project => proj,
-            :url => '/tmp/test/filesystem_repository',
+              :url => repo_url,
               :log_encoding => 'UTF-8' )
         assert r
         c = Changeset.new(:repository   => r,
@@ -269,9 +273,11 @@ class ChangesetTest < ActiveSupport::TestCase
         assert_equal "Texte encod? en ISO-8859-1.", c.comments
       end
     end
+  end
 
   def test_invalid_utf8_sequences_in_comments_should_be_replaced_ja_jis
     with_settings :enabled_scm => ['Filesystem'] do
+      with_existing_filesystem_scm do |repo_url|
         proj = Project.find(3)
         str = "test\xb5\xfetest\xb5\xfe"
         if str.respond_to?(:force_encoding)
@@ -279,7 +285,7 @@ class ChangesetTest < ActiveSupport::TestCase
         end
         r = Repository::Filesystem.create!(
               :project => proj,
-            :url     => '/tmp/test/filesystem_repository',
+              :url     => repo_url,
               :log_encoding => 'ISO-2022-JP' )
         assert r
         c = Changeset.new(:repository   => r,
@@ -291,9 +297,11 @@ class ChangesetTest < ActiveSupport::TestCase
         assert_equal "test??test??", c.comments
       end
     end
+  end
 
   def test_comments_should_be_converted_all_latin1_to_utf8
     with_settings :enabled_scm => ['Filesystem'] do
+      with_existing_filesystem_scm do |repo_url|
         s1 = "\xC2\x80"
         s2 = "\xc3\x82\xc2\x80"
         s4 = s2.dup
@@ -307,7 +315,8 @@ class ChangesetTest < ActiveSupport::TestCase
         end
         proj = Project.find(3)
         r = Repository::Filesystem.create!(
-            :project => proj, :url => '/tmp/test/filesystem_repository',
+              :project => proj,
+              :url => repo_url,
               :log_encoding => 'ISO-8859-1' )
         assert r
         c = Changeset.new(:repository => r,
@@ -319,12 +328,15 @@ class ChangesetTest < ActiveSupport::TestCase
         assert_equal s4, c.comments
       end
     end
+  end
 
   def test_comments_nil
     with_settings :enabled_scm => ['Filesystem'] do
+      with_existing_filesystem_scm do |repo_url|
         proj = Project.find(3)
         r = Repository::Filesystem.create!(
-            :project => proj, :url => '/tmp/test/filesystem_repository',
+              :project => proj,
+              :url => repo_url,
               :log_encoding => 'ISO-8859-1' )
         assert r
         c = Changeset.new(:repository => r,
@@ -339,12 +351,15 @@ class ChangesetTest < ActiveSupport::TestCase
         end
       end
     end
+  end
 
   def test_comments_empty
     with_settings :enabled_scm => ['Filesystem'] do
+      with_existing_filesystem_scm do |repo_url|
         proj = Project.find(3)
         r = Repository::Filesystem.create!(
-            :project => proj, :url => '/tmp/test/filesystem_repository',
+              :project => proj,
+              :url => repo_url,
               :log_encoding => 'ISO-8859-1' )
         assert r
         c = Changeset.new(:repository => r,
@@ -359,6 +374,7 @@ class ChangesetTest < ActiveSupport::TestCase
         end
       end
     end
+  end
 
   def test_identifier
     c = Changeset.find_by_revision('1')

test/unit/helpers/application_helper_test.rb

@@ -192,9 +192,17 @@ RAW
     version = FactoryGirl.create :version,
                  :name => '1.0',
                  :project => @project
-    Setting.enabled_scm = Setting.enabled_scm << "Filesystem" unless Setting.enabled_scm.include? "Filesystem"
+    to_test = {
+      # versions
+      'version:"1.0"'                         => 'version:"1.0"',
+      "#{@project.identifier}:version:\"1.0\""         => "<a href=\"/versions/#{version.id}\" class=\"version\">1.0</a>",
+      'invalid:version:"1.0"'                 => 'invalid:version:"1.0"'
+    }
+
+    with_existing_filesystem_scm do |repo_path|
       repository = FactoryGirl.create :repository,
-                 :project => @project
+                                      url: repo_path,
+                                      project: @project
       changeset = FactoryGirl.create :changeset,
                    :repository => repository,
                    :comments => 'This commit fixes #1, #2 and references #1 & #3'
@@ -206,27 +214,28 @@ RAW
                                                                  :path => 'some/file'} ,
                                                                :class => 'source')
       changeset_link = link_to("#{identifier}:r#{changeset.revision}",
-      {:controller => 'repositories', :action => 'revision', :project_id => identifier, :rev => changeset.revision},
-      :class => 'changeset', :title => 'This commit fixes #1, #2 and references #1 & #3')
-
-
-    # format_text "sees" the text is parses from the_other_project (and not @project)
-    the_other_project = FactoryGirl.create :valid_project
+                               { :controller => 'repositories',
+                                 :action => 'revision',
+                                 :project_id => identifier,
+                                 :rev => changeset.revision },
+                               :class => 'changeset',
+                               :title => 'This commit fixes #1, #2 and references #1 & #3')
 
-    to_test = {
-      # versions
-      'version:"1.0"'                         => 'version:"1.0"',
-      "#{identifier}:version:\"1.0\""         => "<a href=\"/versions/#{version.id}\" class=\"version\">1.0</a>",
-      'invalid:version:"1.0"'                 => 'invalid:version:"1.0"',
+      to_test.merge!({
         # changeset
         "r#{changeset.revision}"                => "r#{changeset.revision}",
-      "#{identifier}:r#{changeset.revision}"  => changeset_link,
+        "#{@project.identifier}:r#{changeset.revision}"  => changeset_link,
         "invalid:r#{changeset.revision}"        => "invalid:r#{changeset.revision}",
         # source
         'source:/some/file'                     => 'source:/some/file',
-      "#{identifier}:source:/some/file"       => source_link,
+        "#{@project.identifier}:source:/some/file"       => source_link,
         'invalid:source:/some/file'             => 'invalid:source:/some/file',
-    }
+      })
+    end
+
+    # format_text "sees" the text is parses from the_other_project (and not @project)
+    the_other_project = FactoryGirl.create :valid_project
+
     to_test.each { |text, result| assert_equal "<p>#{result}</p>", format_text(text, :project => the_other_project), "#{text} failed" }
   end
 

test/unit/repository_filesystem_test.rb

@@ -31,37 +31,37 @@ require File.expand_path('../../test_helper', __FILE__)
 class RepositoryFilesystemTest < ActiveSupport::TestCase
   fixtures :all
 
-  # No '..' in the repository path
-  REPOSITORY_PATH = Rails.root.to_s.gsub(%r{config\/\.\.}, '') + '/tmp/test/filesystem_repository'
-
   def setup
     super
     @project = Project.find(3)
-    Setting.enabled_scm = Setting.enabled_scm.dup << 'Filesystem' unless Setting.enabled_scm.include?('Filesystem')
+
+    with_existing_filesystem_scm do |repo_path|
       assert @repository = Repository::Filesystem.create(
-                            :project => @project, :url => REPOSITORY_PATH)
+                              :project => @project,
+                              :url => repo_path)
+    end
   end
 
-  if File.directory?(REPOSITORY_PATH)
   def test_fetch_changesets
+    with_existing_filesystem_scm do
       @repository.fetch_changesets
       @repository.reload
 
       assert_equal 0, @repository.changesets.count
       assert_equal 0, @repository.changes.count
     end
+  end
 
   def test_entries
+    with_existing_filesystem_scm do
       assert_equal 3, @repository.entries("", 2).size
       assert_equal 2, @repository.entries("dir", 3).size
     end
+  end
 
   def test_cat
+    with_existing_filesystem_scm do
       assert_equal "TEST CAT\n", @repository.scm.cat("test")
     end
-
-  else
-    puts "Filesystem test repository NOT FOUND. Skipping unit tests !!! See doc/RUNNING_TESTS."
-    def test_fake; assert true end
   end
 end