Validate ephermeral blobstore id format

Add shared UuidValidation helper matching SecureRandom.uuid format. AgentClient
only fetches ephemeral blobs when the id is a well-formed UUID; invalid ids are
logged and skipped. LocalClient restricts filesystem paths to UUID object names.

Reject non-UUID ids returned when fetching logs without signed URLs and when
persisting compiled package task results without signed URLs.

Introduce AgentTaskInvalidBlobstoreId for log fetch responses and
PackageCompilationInvalidTaskBlobstoreId for compile task results.

Update unit tests for AgentClient, LocalClient, and the new helper.

Made-with: Cursor

Author: aramprice

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

@@ -490,7 +490,11 @@ def initialize(config)
           manifest_text = request.body.read
           manifest_hash = validate_manifest_yml(manifest_text)
 
-          manifest_hash['name'] = deployment.name if deployment
+          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)

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/logs_fetcher.rb

@@ -1,3 +1,5 @@
+require 'bosh/director/blobstore/uuid_validation'
+
 module Bosh::Director
   class LogsFetcher
     # @param [Bosh::Director::EventLog::Log] event_log
@@ -41,6 +43,11 @@ def fetch(instance, log_type, filters, persist_blobstore_id = false)
             raise AgentTaskNoBlobstoreId,
                   "Agent didn't return a blobstore object id for packaged logs"
           end
+
+          unless Blobstore::UuidValidation.valid_uuid?(blobstore_id)
+            raise AgentTaskInvalidBlobstoreId,
+                  "Agent didn't return a valid blobstore object id for packaged logs"
+          end
         end
         sha_digest = fetch_logs_result['sha1']
       end

src/bosh-director/spec/unit/bosh/director/agent_client_spec.rb

@@ -549,20 +549,21 @@ def self.it_acts_as_message_with_timeout(message_name)
     end
 
     it 'should handle exceptions' do
+      blob_uuid = 'c0eebc99-9c0b-4ef8-bb6d-6bb9bd380a13'
       response = {
         'exception' => {
           'message' => 'test',
           'backtrace' => %w[a b c],
-          'blobstore_id' => 'deadbeef',
+          'blobstore_id' => blob_uuid,
         },
       }
 
       expect(@nats_rpc).to receive(:send_request)
         .with('foo.bar', 'bar', expected_rpc_args, options).and_yield(response)
 
       rm = double(Bosh::Director::Api::ResourceManager)
-      expect(rm).to receive(:get_resource).with('deadbeef').and_return('an exception')
-      expect(rm).to receive(:delete_resource).with('deadbeef')
+      expect(rm).to receive(:get_resource).with(blob_uuid).and_return('an exception')
+      expect(rm).to receive(:delete_resource).with(blob_uuid)
       expect(Bosh::Director::Api::ResourceManager).to receive(:new).and_return(rm)
 
       client = AgentClient.new('foo', 'bar', 'foo_instance/1')
@@ -715,11 +716,13 @@ def self.it_acts_as_message_with_timeout(message_name)
     end
 
     describe 'handling compilation log' do
+      let(:compile_log_uuid) { 'a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11' }
+
       it 'should inject compile log into response' do
         response = {
           'value' => {
             'result' => {
-              'compile_log_id' => 'cafe',
+              'compile_log_id' => compile_log_uuid,
             },
           },
         }
@@ -728,14 +731,36 @@ def self.it_acts_as_message_with_timeout(message_name)
           .with('foo.bar', 'bar', expected_rpc_args, options).and_yield(response)
 
         rm = instance_double('Bosh::Director::Api::ResourceManager')
-        expect(rm).to receive(:get_resource).with('cafe').and_return('blob')
-        expect(rm).to receive(:delete_resource).with('cafe')
+        expect(rm).to receive(:get_resource).with(compile_log_uuid).and_return('blob')
+        expect(rm).to receive(:delete_resource).with(compile_log_uuid)
         expect(Bosh::Director::Api::ResourceManager).to receive(:new).and_return(rm)
 
         client = AgentClient.new('foo', 'bar', 'foo_instance/1')
         value = client.baz(*test_args)
         expect(value['result']['compile_log']).to eq('blob')
       end
+
+      it 'does not fetch compile log when compile_log_id is not a UUID' do
+        response = {
+          'value' => {
+            'result' => {
+              'compile_log_id' => 'cafe',
+            },
+          },
+        }
+
+        expect(@nats_rpc).to receive(:send_request)
+          .with('foo.bar', 'bar', expected_rpc_args, options).and_yield(response)
+
+        rm = instance_double('Bosh::Director::Api::ResourceManager')
+        expect(rm).not_to receive(:get_resource)
+        expect(rm).not_to receive(:delete_resource)
+        expect(Bosh::Director::Api::ResourceManager).to receive(:new).and_return(rm)
+
+        client = AgentClient.new('foo', 'bar', 'foo_instance/1')
+        value = client.baz(*test_args)
+        expect(value['result']).not_to have_key('compile_log')
+      end
     end
 
     describe 'formatting RPC remote exceptions' do
@@ -745,23 +770,39 @@ def self.it_acts_as_message_with_timeout(message_name)
       end
 
       it 'supports new style (Hash)' do
+        blob_uuid = 'b0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12'
         exception = {
           'message' => 'something happened',
           'backtrace' => ['in zbb.rb:35', 'in zbb.rb:26'],
-          'blobstore_id' => 'deadbeef',
+          'blobstore_id' => blob_uuid,
         }
 
         rm = instance_double('Bosh::Director::Api::ResourceManager')
         allow(Bosh::Director::Api::ResourceManager).to receive(:new).and_return(rm)
-        expect(rm).to receive(:get_resource).with('deadbeef')
+        expect(rm).to receive(:get_resource).with(blob_uuid)
                                             .and_return("Failed to compile: no such file 'zbb'")
-        expect(rm).to receive(:delete_resource).with('deadbeef')
+        expect(rm).to receive(:delete_resource).with(blob_uuid)
 
         expected_error = "something happened\nin zbb.rb:35\nin zbb.rb:26\nFailed to compile: no such file 'zbb'"
 
         client = AgentClient.new('foo', 'bar', 'foo_instance/1')
         expect(client.format_exception(exception)).to eq(expected_error)
       end
+
+      it 'does not append blob content when blobstore_id is not a UUID' do
+        exception = {
+          'message' => 'something happened',
+          'blobstore_id' => 'deadbeef',
+        }
+
+        rm = instance_double('Bosh::Director::Api::ResourceManager')
+        allow(Bosh::Director::Api::ResourceManager).to receive(:new).and_return(rm)
+        expect(rm).not_to receive(:get_resource)
+        expect(rm).not_to receive(:delete_resource)
+
+        client = AgentClient.new('foo', 'bar', 'foo_instance/1')
+        expect(client.format_exception(exception)).to eq('something happened')
+      end
     end
 
     describe '#run_errand' do

src/bosh-director/spec/unit/bosh/director/api/controllers/deployments_controller_spec.rb

@@ -2737,10 +2737,14 @@ def perform
 
             it 'uses the deployment from the URL when the YAML manifest names a different deployment' do
               manifest = YAML.dump('name' => 'other_deployment', 'releases' => [], 'instance_groups' => [])
+              expect_any_instance_of(DeploymentManager)
+                .to receive(:create_deployment) do |_, _, rewritten_manifest, *_|
+                expect(YAML.load(rewritten_manifest)).to include('name' => 'owned_deployment')
+                OpenStruct.new(id: 1)
+              end
               put '/owned_deployment/jobs/dea?state=recreate', manifest, { 'CONTENT_TYPE' => 'text/yaml' }
               expect(last_response.status).to eq(302)
             end
-
           end
 
           context 'PUT /:deployment/jobs/:job/:index_or_id' do