Improve consistency: use file_path for API create/update/delete files

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

6 files changed, 18 insertions, 18 deletions

diff --git a/app/contexts/files/create_context.rb b/app/contexts/files/create_context.rb
index ae73b11bd54..10273138559 100644
--- a/app/contexts/files/create_context.rb
+++ b/app/contexts/files/create_context.rb
@@ -15,18 +15,13 @@ module Files
         return error("You can only create files if you are on top of a branch")
       end
 
-      file_name = params[:file_name]
+      file_name = File.basename(path)
+      file_path = path
 
       unless file_name =~ Gitlab::Regex.path_regex
         return error("Your changes could not be commited, because file name contains not allowed characters")
       end
 
-      file_path = if path.blank?
-                    file_name
-                  else
-                    File.join(path, file_name)
-                  end
-
       blob = repository.blob_at(ref, file_path)
 
       if blob
diff --git a/app/controllers/projects/new_tree_controller.rb b/app/controllers/projects/new_tree_controller.rb
index 9f9e0191e98..d6d474cf9c5 100644
--- a/app/controllers/projects/new_tree_controller.rb
+++ b/app/controllers/projects/new_tree_controller.rb
@@ -5,11 +5,12 @@ class Projects::NewTreeController < Projects::BaseTreeController
   end
 
   def update
-    result = Files::CreateContext.new(@project, current_user, params, @ref, @path).execute
+    file_path = File.join(@path, File.basename(params[:file_name]))
+    result = Files::CreateContext.new(@project, current_user, params, @ref, file_path).execute
 
     if result[:status] == :success
       flash[:notice] = "Your changes have been successfully commited"
-      redirect_to project_blob_path(@project, File.join(@id, params[:file_name]))
+      redirect_to project_blob_path(@project, File.join(@ref, file_path))
     else
       flash[:alert] = result[:error]
       render :show
diff --git a/doc/api/repositories.md b/doc/api/repositories.md
index af760795d00..af7b82ca76d 100644
--- a/doc/api/repositories.md
+++ b/doc/api/repositories.md
@@ -379,8 +379,7 @@ POST /projects/:id/repository/files
 
 Parameters:
 
-+ `file_name` (required) - The name of new file. Ex. class.rb
-+ `file_path` (optional) - The path to new file. Ex. lib/
++ `file_path` (optional) - Full path to new file. Ex. lib/class.rb
 + `branch_name` (required) - The name of branch
 + `content` (required) - File content
 + `commit_message` (required) - Commit message
diff --git a/lib/api/files.rb b/lib/api/files.rb
index 588c27d5692..6a5419a580f 100644
--- a/lib/api/files.rb
+++ b/lib/api/files.rb
@@ -8,8 +8,7 @@ module API
       # Create new file in repository
       #
       # Parameters:
-      #   file_name (required) - The name of new file. Ex. class.rb
-      #   file_path (optional) - The path to new file. Ex. lib/
+      #   file_path (optional) - The path to new file. Ex. lib/class.rb
       #   branch_name (required) - The name of branch
       #   content (required) - File content
       #   commit_message (required) - Commit message
@@ -18,8 +17,8 @@ module API
       #   POST /projects/:id/repository/files
       #
       post ":id/repository/files" do
-        required_attributes! [:file_name, :branch_name, :content, :commit_message]
-        attrs = attributes_for_keys [:file_name, :file_path, :branch_name, :content, :commit_message]
+        required_attributes! [:file_path, :branch_name, :content, :commit_message]
+        attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message]
         branch_name = attrs.delete(:branch_name)
         file_path = attrs.delete(:file_path)
         result = ::Files::CreateContext.new(user_project, current_user, attrs, branch_name, file_path).execute
@@ -28,7 +27,6 @@ module API
           status(201)
 
           {
-            file_name: attrs[:file_name],
             file_path: file_path,
             branch_name: branch_name
           }
diff --git a/lib/gitlab/satellite/files/new_file_action.rb b/lib/gitlab/satellite/files/new_file_action.rb
index 833a3777158..91f7175c2ac 100644
--- a/lib/gitlab/satellite/files/new_file_action.rb
+++ b/lib/gitlab/satellite/files/new_file_action.rb
@@ -18,6 +18,13 @@ module Gitlab
 
           # update the file in the satellite's working dir
           file_path_in_satellite = File.join(repo.working_dir, file_path)
+
+          # Prevent relative links
+          unless File.absolute_path(file_path_in_satellite) == file_path_in_satellite
+            Gitlab::GitLogger.error("NewFileAction: Relative path not allowed")
+            return false
+          end
+
           File.open(file_path_in_satellite, 'w') { |f| f.write(content) }
 
           # add new file
diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb
index 0e2a48689ac..2d1f8df47dd 100644
--- a/spec/requests/api/files_spec.rb
+++ b/spec/requests/api/files_spec.rb
@@ -12,7 +12,7 @@ describe API::API do
   describe "POST /projects/:id/repository/files" do
     let(:valid_params) {
       {
-        file_name: 'newfile.rb',
+        file_path: 'newfile.rb',
         branch_name: 'master',
         content: 'puts 8',
         commit_message: 'Added newfile'
@@ -26,7 +26,7 @@ describe API::API do
 
       post api("/projects/#{project.id}/repository/files", user), valid_params
       response.status.should == 201
-      json_response['file_name'].should == 'newfile.rb'
+      json_response['file_path'].should == 'newfile.rb'
     end
 
     it "should return a 400 bad request if no params given" do