Add size, created_at, and download to the attachments API

CHANGELOG

@@ -1,6 +1,5 @@
 [v#.#.#] ([month] [YYYY])
-  - [entity]:
-    - [future tense verb] [feature]
+  - Attachments: Add size, created_at, and download link to the API
   - Upgraded gems:
     - nokogiri, rails
   - Bugs fixes:

config/brakeman.ignore

@@ -7,7 +7,7 @@
       "check_name": "FileAccess",
       "message": "Model attribute used in file name",
       "file": "app/models/methodology.rb",
-      "line": 45,
+      "line": 46,
       "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
       "code": "Pathname.new(Configuration.paths_templates_methodologies)",
       "render_path": null,
@@ -76,7 +76,7 @@
       "check_name": "FileAccess",
       "message": "Model attribute used in file name",
       "file": "engines/dradis-api/app/controllers/dradis/ce/api/v3/attachments_controller.rb",
-      "line": 56,
+      "line": 58,
       "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
       "code": "File.rename(Attachment.find(params[:filename], :conditions => ({ :node_id => current_project.nodes.find(params[:node_id]).id })).fullpath, Attachment.pwd.join(current_project.nodes.find(params[:node_id]).id.to_s, CGI.unescape(attachment_params[:filename])).to_s)",
       "render_path": null,
@@ -169,7 +169,7 @@
       "check_name": "FileAccess",
       "message": "Model attribute used in file name",
       "file": "app/models/methodology.rb",
-      "line": 127,
+      "line": 123,
       "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
       "code": "FileUtils.mkdir_p(Methodology.pwd)",
       "render_path": null,
@@ -185,6 +185,29 @@
       ],
       "note": "False positive: Methodology.pwd is set by the admin to specify the directory for the methodologies"
     },
+    {
+      "warning_type": "File Access",
+      "warning_code": 16,
+      "fingerprint": "8ab35d05ae235702bf80402025acb8093cc59c0d280b55baf56d771af3780018",
+      "check_name": "SendFile",
+      "message": "Model attribute used in file name",
+      "file": "engines/dradis-api/app/controllers/dradis/ce/api/v3/attachments_controller.rb",
+      "line": 79,
+      "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
+      "code": "send_file(Attachment.find(params[:filename], :conditions => ({ :node_id => current_project.nodes.find(params[:node_id]).id })).fullpath)",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Dradis::CE::API::V3::AttachmentsController",
+        "method": "download"
+      },
+      "user_input": "Attachment.find(params[:filename], :conditions => ({ :node_id => current_project.nodes.find(params[:node_id]).id })).fullpath",
+      "confidence": "Medium",
+      "cwe_id": [
+        22
+      ],
+      "note": "False positive: The destination filename is prepended by the Attachments directory and validated as such to prevent being moved to the other directories"
+    },
     {
       "warning_type": "Denial of Service",
       "warning_code": 76,
@@ -216,7 +239,7 @@
       "check_name": "UnsafeReflection",
       "message": "Unsafe reflection method `constantize` called on parameter value",
       "file": "app/controllers/upload_controller.rb",
-      "line": 136,
+      "line": 134,
       "link": "https://brakemanscanner.org/docs/warning_types/remote_code_execution/",
       "code": "params[:uploader].constantize",
       "render_path": null,
@@ -239,7 +262,7 @@
       "check_name": "RegexDoS",
       "message": "Model attribute used in regular expression",
       "file": "engines/dradis-api/app/controllers/dradis/ce/api/v3/attachments_controller.rb",
-      "line": 55,
+      "line": 57,
       "link": "https://brakemanscanner.org/docs/warning_types/denial_of_service/",
       "code": "/^#{Attachment.pwd}/",
       "render_path": null,
@@ -314,6 +337,6 @@
       "note": "False positive: The params is used to fetch the boards and cannot be manipulated by user input"
     }
   ],
-  "updated": "2023-08-18 10:40:16 -0400",
+  "updated": "2024-03-07 16:33:01 +0800",
   "brakeman_version": "5.4.0"
 }

engines/dradis-api/app/controllers/dradis/ce/api/v3/attachments_controller.rb

@@ -15,6 +15,8 @@ def index
       def show
         begin
           @attachment = Attachment.find(params[:filename], conditions: { node_id: @node.id })
+          @file_size = File.size(@attachment.fullpath)
+          @created_at = File.ctime(@attachment.fullpath)
         rescue
           raise ActiveRecord::RecordNotFound, "Couldn't find attachment with filename '#{params[:filename]}'"
         end
@@ -71,6 +73,15 @@ def destroy
         render_successful_destroy_message
       end
 
+      def download
+        begin
+          @attachment = Attachment.find(params[:filename], conditions: { node_id: @node.id })
+          send_file(@attachment.fullpath)
+        rescue
+          raise ActiveRecord::RecordNotFound, "Couldn't find attachment with filename '#{params[:filename]}'"
+        end
+      end
+
       private
 
       def set_node

engines/dradis-api/app/views/dradis/ce/api/v3/attachments/_attachment.json.jbuilder

@@ -1,2 +1,5 @@
 json.filename attachment.filename
+json.size @file_size
+json.created_at @created_at
 json.link main_app.project_node_attachment_path(current_project, @node, attachment.filename)
+json.download download_node_attachment_url(@node, attachment.filename)

engines/dradis-api/config/routes.rb

@@ -24,7 +24,9 @@
           resources :evidence
           resources :notes
           constraints(filename: /.*/) do
-            resources :attachments, param: :filename
+            resources :attachments, param: :filename do
+              get :download, on: :member
+            end
           end
         end
       end

engines/dradis-api/spec/requests/dradis/ce/api/v3/attachments_spec.rb

@@ -67,15 +67,24 @@
 
         expect(attachment_0).to eq({
           'filename' => 'image0.png',
-          'link' => "/projects/#{current_project.id}/nodes/#{node.id}/attachments/image0.png"
+          'size' => nil,
+          'created_at' => nil,
+          'link' => "/projects/#{current_project.id}/nodes/#{node.id}/attachments/image0.png",
+          'download' => "https://www.example.com/api/nodes/#{node.id}/attachments/#{attachment_0['filename']}/download"
         })
         expect(attachment_1).to eq({
           'filename' => 'image1.png',
-          'link' => "/projects/#{current_project.id}/nodes/#{node.id}/attachments/image1.png"
+          'size' => nil,
+          'created_at' => nil,
+          'link' => "/projects/#{current_project.id}/nodes/#{node.id}/attachments/image1.png",
+          'download' => "https://www.example.com/api/nodes/#{node.id}/attachments/#{attachment_1['filename']}/download"
         })
         expect(attachment_2).to eq({
           'filename' => 'image2.png',
-          'link' => "/projects/#{current_project.id}/nodes/#{node.id}/attachments/image2.png"
+          'size' => nil,
+          'created_at' => nil,
+          'link' => "/projects/#{current_project.id}/nodes/#{node.id}/attachments/image2.png",
+          'download' => "https://www.example.com/api/nodes/#{node.id}/attachments/#{attachment_2['filename']}/download"
         })
       end
 
@@ -104,9 +113,11 @@
 
       it 'returns JSON information about the attachment' do
         retrieved_attachment = JSON.parse(response.body)
-        expect(retrieved_attachment.keys).to match_array(%w[filename link])
+        expect(retrieved_attachment.keys).to match_array(%w[filename size created_at link download])
         expect(retrieved_attachment['filename']).to eq 'image.png'
+        expect(retrieved_attachment['size']).to eq 1787
         expect(retrieved_attachment['link']).to eq "/projects/#{current_project.id}/nodes/#{node.id}/attachments/image.png"
+        expect(retrieved_attachment['download']).to eq "https://www.example.com/api/nodes/#{node.id}/attachments/image.png/download"
       end
     end
 
@@ -158,10 +169,10 @@
         attachment_0 = retrieved_attachments.detect { |n| n['filename'] == 'rails.png' }
         attachment_1 = retrieved_attachments.detect { |n| n['filename'] == 'rails_copy-01.png' }
 
-        expect(attachment_0.keys).to match_array %w[filename link]
+        expect(attachment_0.keys).to match_array %w[filename size created_at link download]
         expect(attachment_0['filename']).to eq 'rails.png'
         expect(attachment_0['link']).to eq "/projects/#{current_project.id}/nodes/#{node.id}/attachments/rails.png"
-        expect(attachment_1.keys).to match_array %w[filename link]
+        expect(attachment_1.keys).to match_array %w[filename size created_at link download]
         expect(attachment_1['filename']).to eq 'rails_copy-01.png'
         expect(attachment_1['link']).to eq "/projects/#{current_project.id}/nodes/#{node.id}/attachments/rails_copy-01.png"
       end
@@ -289,5 +300,17 @@
           'Resource deleted successfully'
       end
     end
+
+    describe 'GET /api/nodes/:node_id/attachments/:filename/download' do
+      it 'returns with the file' do
+        create(:attachment, filename: 'image.png', node: node)
+
+        get "/api/nodes/#{node.id}/attachments/image.png/download", env: @env
+
+        expect(response.headers['Content-Type']).to eq('image/png')
+        expect(response.headers['Content-Length']).to eq('1787')
+        expect(response.headers['Content-Disposition']).to eq("attachment; filename=\"image.png\"; filename*=UTF-8''image.png")
+      end
+    end
   end
 end