Merge pull request #2707 from cloudfoundry/blobstore-id-handling

Blobstore id handling

Author: aramprice

.gitignore

@@ -1,5 +1,6 @@
 .DS_Store
 .bundle
+.cursor
 .idea
 *.iml
 

src/bosh-director/lib/bosh/director/agent_client.rb

@@ -1,4 +1,5 @@
 require 'bosh/director/agent_message_converter'
+require 'bosh/director/blobstore/uuid_validation'
 
 module Bosh::Director
   class AgentClient
@@ -317,8 +318,10 @@ def format_exception(exception)
 
       if exception['blobstore_id']
         blob = download_and_delete_blob(exception['blobstore_id'])
-        msg += "\n"
-        msg += blob.to_s
+        unless blob.nil?
+          msg += "\n"
+          msg += blob.to_s
+        end
       end
 
       msg
@@ -334,18 +337,27 @@ def inject_compile_log(response)
         response['value']['result'].is_a?(Hash) &&
         (blob_id = response['value']['result']['compile_log_id'])
         compile_log = download_and_delete_blob(blob_id)
-        response['value']['result']['compile_log'] = compile_log
+        response['value']['result']['compile_log'] = compile_log unless compile_log.nil?
       end
     end
 
     # Downloads blob and ensures it's deleted from the blobstore
     # @param [String] blob_id Blob id
     # @return [String] Blob contents
     def download_and_delete_blob(blob_id)
+      unless Blobstore::UuidValidation.valid_uuid?(blob_id)
+        @logger.warn(
+          "Skipping blob fetch for agent '#{@client_id}' (#{@instance_name}): object id is not a valid UUID",
+        )
+        return nil
+      end
+
       blob = @resource_manager.get_resource(blob_id)
       blob
     ensure
-      @resource_manager.delete_resource(blob_id)
+      if blob_id && Blobstore::UuidValidation.valid_uuid?(blob_id)
+        @resource_manager.delete_resource(blob_id)
+      end
     end
 
     def handle_message_with_retry(message_name, *args, &blk)

src/bosh-director/lib/bosh/director/api/controllers/deployments_controller.rb

@@ -67,6 +67,7 @@ def initialize(config)
           options['manifest_text'] = manifest
         else
           manifest_hash = validate_manifest_yml(request.body.read)
+          manifest_hash['name'] = deployment.name
           manifest = YAML.dump(manifest_hash)
           teams = deployment.teams
           latest_cloud_configs = Models::Config.latest_set_for_teams('cloud', *teams)
@@ -112,6 +113,7 @@ def initialize(config)
           options['manifest_text'] = manifest
         else
           manifest_hash = validate_manifest_yml(request.body.read)
+          manifest_hash['name'] = deployment.name
           manifest = YAML.dump(manifest_hash)
           teams = deployment.teams
           latest_cloud_configs = Models::Config.latest_set_for_teams('cloud', *teams)
@@ -440,9 +442,9 @@ def initialize(config)
           end
         end
 
-        # since authorizer does not look at manifest payload for deployment name
         @deployment = Models::Deployment[name: deployment_name]
         if @deployment
+          @permission_authorizer.granted_or_raise(@deployment, :admin, token_scopes)
           teams = @deployment.teams
         else
           teams = Bosh::Director::Models::Team.transform_admin_team_scope_to_teams(token_scopes)
@@ -488,6 +490,12 @@ def initialize(config)
           manifest_text = request.body.read
           manifest_hash = validate_manifest_yml(manifest_text)
 
+          if deployment
+            manifest_hash['name'] = deployment.name
+            # ensure diff will show `name` from `deployment.name` and not manifest YAML
+            manifest_text = YAML.dump(manifest_hash)
+          end
+
           if deployment
             before_manifest = Manifest.load_from_model(deployment, resolve_interpolation: false)
             before_manifest.resolve_aliases

src/bosh-director/lib/bosh/director/blobstore/local_client.rb

@@ -1,3 +1,5 @@
+require 'bosh/director/blobstore/uuid_validation'
+
 module Bosh::Director
   module Blobstore
     class LocalClient < Client
@@ -52,6 +54,10 @@ def required_credential_properties_list
       private
 
       def object_file_path(oid)
+        unless UuidValidation.valid_uuid?(oid)
+          raise BlobstoreError, "invalid blobstore object id format: #{oid.inspect}"
+        end
+
         File.join(@blobstore_path, oid)
       end
     end

src/bosh-director/lib/bosh/director/blobstore/uuid_validation.rb

@@ -0,0 +1,14 @@
+module Bosh::Director
+  module Blobstore
+    # Validates UUIDs are (8-4-4-4-12 hex):
+    # - Ruby: SecureRandom.uuid
+    # - Golang: github.com/nu7hatch/gouuid
+    module UuidValidation
+      UUID_PATTERN = /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i.freeze
+
+      def self.valid_uuid?(value)
+        UUID_PATTERN.match?(value.to_s)
+      end
+    end
+  end
+end

src/bosh-director/lib/bosh/director/deployment_plan/stages/package_compile_stage.rb

@@ -1,3 +1,4 @@
+require 'bosh/director/blobstore/uuid_validation'
 require 'bosh/director/compiled_package_requirement_generator'
 require 'digest/sha1'
 
@@ -131,6 +132,8 @@ def compile_package(requirement)
                 end
               end
 
+              validate_compiled_package_blobstore_id!(task_result['blobstore_id'])
+
               compiled_package = Models::CompiledPackage.create do |p|
                 p.package = package
                 p.stemcell_os = stemcell.os
@@ -158,6 +161,13 @@ def prepare_vm(...)
 
         private
 
+        def validate_compiled_package_blobstore_id!(blobstore_id)
+          return if Blobstore::UuidValidation.valid_uuid?(blobstore_id)
+
+          raise PackageCompilationInvalidTaskBlobstoreId,
+                'Compilation task result contained an invalid blobstore object id'
+        end
+
         def validate_packages(instance_groups_to_compile)
           instance_groups_to_compile.each do |instance_group|
             instance_group.jobs.each do |job|

src/bosh-director/lib/bosh/director/errors.rb

@@ -287,6 +287,7 @@ def self.err(error_code, response_code = BAD_REQUEST)
   AgentInvalidTaskResult = err(400011)
   AgentUnsupportedAction = err(400012)
   AgentUploadBlobUnableToOpenFile = err(400013)
+  AgentTaskInvalidBlobstoreId = err(400014)
 
   # Cloud check task errors
   CloudcheckTooManySimilarProblems = err(410001)
@@ -298,6 +299,7 @@ def self.err(error_code, response_code = BAD_REQUEST)
   DnsInvalidCanonicalName = err(420001)
 
   # PackageCompilation
+  PackageCompilationInvalidTaskBlobstoreId = err(430004)
   PackageCompilationNotEnoughWorkersForReuse = err(430002)
   PackageCompilationNotFound = err(430003)
 

src/bosh-director/lib/bosh/director/jobs/release/release_job.rb

@@ -1,3 +1,5 @@
+require 'open3'
+
 module Bosh::Director
   class ReleaseJob
 
@@ -11,6 +13,8 @@ def initialize(job_meta, release_model, release_dir, logger)
     end
 
     def update
+      validate_job_meta_identifiers!
+
       unpack
 
       job_manifest = load_manifest
@@ -30,11 +34,11 @@ def update
 
       if job_model.blobstore_id
         begin
-          @logger.info("Deleting blob for job '#{name}/#{@version}' with blobstore_id '#{job_model.blobstore_id}'")
+          @logger.info("Deleting blob for job '#{name}/#{version}' with blobstore_id '#{job_model.blobstore_id}'")
           BlobUtil.delete_blob(job_model.blobstore_id)
           job_model.blobstore_id = nil
         rescue Bosh::Director::Blobstore::BlobstoreError => e
-          @logger.info("Error deleting blob for job '#{name}/#{@version}' with blobstore_id '#{job_model.blobstore_id}': #{e.inspect}")
+          @logger.info("Error deleting blob for job '#{name}/#{version}' with blobstore_id '#{job_model.blobstore_id}': #{e.inspect}")
         end
       end
 
@@ -48,16 +52,26 @@ def update
     def unpack
       FileUtils.mkdir_p(job_dir)
 
-      desc = "job '#{name}/#{@version}'"
-      result = Bosh::Common::Exec.sh("tar -C #{job_dir} -xf #{job_tgz} 2>&1", :on_error => :return)
-      if result.failed?
+      desc = "job '#{name}/#{version}'"
+      out, err, status = Open3.capture3('tar', '-C', job_dir, '-xf', job_tgz)
+      combined = [out, err].map(&:to_s).join
+      if status.exitstatus != 0
         @logger.error("Extracting #{desc} archive failed in dir #{job_dir}, " +
-          "tar returned #{result.exit_status}, " +
-          "output: #{result.output}")
+          "tar returned #{status.exitstatus}, " +
+          "output: #{combined}")
         raise JobInvalidArchive, "Extracting #{desc} archive failed. Check task debug log for details."
       end
     end
 
+    def validate_job_meta_identifiers!
+      if !@job_meta['name'].is_a?(String) || !Models::VALID_ID.match?(@job_meta['name'])
+        raise JobInvalidName, "Invalid job name in release manifest: #{@job_meta['name'].inspect}"
+      end
+      if !@job_meta['version'].is_a?(String) || !Models::VALID_ID.match?(@job_meta['version'])
+        raise ValidationInvalidValue, "Invalid job version in release manifest: #{@job_meta['version'].inspect}"
+      end
+    end
+
     def job_tgz
       @job_tgz ||= File.join(@release_dir, 'jobs', "#{name}.tgz")
     end
@@ -176,5 +190,9 @@ def validate_consume_links(consume_links)
     def name
       @job_meta['name']
     end
+
+    def version
+      @job_meta['version']
+    end
   end
 end

src/bosh-director/lib/bosh/director/jobs/update_release.rb

@@ -1,4 +1,5 @@
 require 'pp' # for #pretty_inspect
+require 'open3'
 require 'securerandom'
 require 'bosh/version/release_version'
 
@@ -71,9 +72,10 @@ def download_remote_release
       def extract_release
         release_dir = Dir.mktmpdir
 
-        result = Bosh::Common::Exec.sh("tar -C #{release_dir} -xf #{release_path} 2>&1", on_error: :return)
-        if result.failed?
-          logger.error("Failed to extract release archive '#{release_path}' into dir '#{release_dir}', tar returned #{result.exit_status}, output: #{result.output})")
+        out, err, status = Open3.capture3('tar', '-C', release_dir, '-xf', release_path)
+        combined = [out, err].map(&:to_s).join
+        if status.exitstatus != 0
+          logger.error("Failed to extract release archive '#{release_path}' into dir '#{release_dir}', tar returned #{status.exitstatus}, output: #{combined}")
           FileUtils.rm_rf(release_dir)
           raise ReleaseInvalidArchive, 'Extracting release archive failed. Check task debug log for details.'
         end
@@ -176,6 +178,29 @@ def normalize_manifest
 
         manifest_packages.each { |p| hash_string_vals(p, 'name', 'version', 'sha1') }
         manifest_jobs.each { |j| hash_string_vals(j, 'name', 'version', 'sha1') }
+
+        validate_manifest_release_identifiers!
+      end
+
+      def validate_manifest_release_identifiers!
+        validate_manifest_identifier!(@manifest['name'], 'release name')
+        validate_manifest_identifier!(@manifest['version'], 'release version')
+
+        manifest_packages.each_with_index do |p, i|
+          validate_manifest_identifier!(p['name'], "package #{i + 1} name")
+          validate_manifest_identifier!(p['version'], "package #{i + 1} version")
+        end
+
+        manifest_jobs.each_with_index do |j, i|
+          validate_manifest_identifier!(j['name'], "job #{i + 1} name")
+          validate_manifest_identifier!(j['version'], "job #{i + 1} version")
+        end
+      end
+
+      def validate_manifest_identifier!(value, label)
+        if !value.is_a?(String) || !Models::VALID_ID.match?(value)
+          raise ValidationInvalidValue, "Invalid #{label} in release manifest: #{value.inspect}"
+        end
       end
 
       # Replace values for keys in a hash with their to_s.

src/bosh-director/lib/bosh/director/jobs/update_release/package_persister.rb

@@ -1,3 +1,5 @@
+require 'open3'
+
 module Bosh::Director
   module Jobs
     class UpdateRelease < BaseJob
@@ -186,9 +188,10 @@ def delete_package_blob(logger, desc, package)
           end
 
           def validate_tgz(logger, tgz, desc)
-            result = Bosh::Common::Exec.sh("tar -tf #{tgz} 2>&1", on_error: :return)
-            if result.failed?
-              logger.error("Extracting #{desc} archive failed, tar returned #{result.exit_status}, output: #{result.output}")
+            out, err, status = Open3.capture3('tar', '-tf', tgz)
+            combined = [out, err].map(&:to_s).join
+            if status.exitstatus != 0
+              logger.error("Extracting #{desc} archive failed, tar returned #{status.exitstatus}, output: #{combined}")
               raise PackageInvalidArchive, "Extracting #{desc} archive failed. Check task debug log for details."
             end
           end