Fix path traversal vulnerability in ActiveStorage blob key import

BlobRecordSet#import_batch preserved blob keys from exported ZIP files
verbatim. A crafted ZIP with a traversal key like "../../config/deploy.yml"
would create a blob whose key resolves to an arbitrary filesystem path
when served by ActiveStorage's DiskService, allowing an attacker to read
local files.

Fix: generate fresh blob keys on import, discarding whatever key was in
the ZIP. This is safe because nothing else in the import pipeline
references blobs by key — attachments use blob_id, ActionText uses GIDs
based on record IDs, and only FileRecordSet used the old key for file
data lookup.

Update FileRecordSet to build an old_key→blob_id mapping from the blob
JSON metadata in the ZIP, then look up blobs by ID instead of by key.
Both check_record and import_batch are now fail-closed: they raise
IntegrityError for unmapped storage files, missing blobs, and duplicate
keys in the export.

Backfill test coverage for BlobRecordSet and FileRecordSet import, and
add a round-trip test verifying blob data survives export/import with
regenerated keys.

app/models/account/data_transfer/active_storage/blob_record_set.rb

@@ -13,6 +13,7 @@ class Account::DataTransfer::ActiveStorage::BlobRecordSet < Account::DataTransfe
         data = load(file)
         data.slice(*attributes).merge(
           "account_id" => account.id,
+          "key" => ::ActiveStorage::Blob.generate_unique_secure_token(length: ::ActiveStorage::Blob::MINIMUM_TOKEN_LENGTH),
           "service_name" => ::ActiveStorage::Blob.service.name
         )
       end

app/models/account/data_transfer/active_storage/file_record_set.rb

@@ -22,9 +22,12 @@ class Account::DataTransfer::ActiveStorage::FileRecordSet < Account::DataTransfe
 
     def import_batch(files)
       files.each do |file|
-        key = File.basename(file)
-        blob = ::ActiveStorage::Blob.find_by(key: key, account: account)
-        next unless blob
+        old_key = file.delete_prefix("storage/")
+        blob_id = old_key_to_blob_id[old_key]
+        raise IntegrityError, "Storage file #{old_key} has no matching blob metadata in export" unless blob_id
+
+        blob = ::ActiveStorage::Blob.find_by(id: blob_id, account: account)
+        raise IntegrityError, "Blob #{blob_id} not found for storage key #{old_key}" unless blob
 
         zip.read(file) do |stream|
           blob.upload(stream)
@@ -32,12 +35,31 @@ class Account::DataTransfer::ActiveStorage::FileRecordSet < Account::DataTransfe
       end
     end
 
-    def check_record(file_path)
-      key = File.basename(file_path)
+    def old_key_to_blob_id
+      @old_key_to_blob_id ||= build_old_key_to_blob_id
+    end
 
-      unless zip.exists?("data/active_storage_blobs/#{key}.json") || ::ActiveStorage::Blob.exists?(key: key, account: account)
-        # File exists without corresponding blob record - could be orphaned or blob not yet imported
-        # We allow this since blob metadata is imported before files
+    def build_old_key_to_blob_id
+      zip.glob("data/active_storage_blobs/*.json").each_with_object({}) do |file, map|
+        data = load(file)
+        old_key = data["key"]
+        if map.key?(old_key)
+          raise IntegrityError, "Duplicate blob key in export: #{old_key}"
+        end
+        map[old_key] = data["id"]
+      end
+    end
+
+    def with_zip(zip)
+      @old_key_to_blob_id = nil
+      super
+    end
+
+    def check_record(file_path)
+      old_key = file_path.delete_prefix("storage/")
+
+      unless old_key_to_blob_id.key?(old_key)
+        raise IntegrityError, "Storage file #{old_key} has no matching blob metadata in export"
       end
     end
 end

test/integration/blob_key_traversal_test.rb

@@ -0,0 +1,45 @@
+require "test_helper"
+
+class Account::DataTransfer::ActiveStorage::BlobKeyTraversalTest < ActionDispatch::IntegrationTest
+  test "import with path traversal blob key does not leak local files" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+    traversal_key = "../../config/deploy.yml"
+    zip = build_zip_with_blob(id: blob_id, key: traversal_key)
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+    blob = ActiveStorage::Blob.find(blob_id)
+
+    assert_not_equal traversal_key, blob.key
+
+    sign_in_as identities(:david)
+    get rails_blob_path(blob, disposition: "inline")
+
+    assert_response :redirect
+
+    follow_redirect!
+
+    assert_response :not_found
+  end
+
+  private
+    def build_zip_with_blob(id:, key:)
+      tempfile = Tempfile.new([ "malicious", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+      writer.add_file("data/active_storage_blobs/#{id}.json", {
+        id: id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: 32,
+        checksum: "",
+        content_type: "text/plain",
+        created_at: Time.current.iso8601,
+        filename: "traversal.txt",
+        key: key,
+        metadata: {}
+      }.to_json)
+      writer.close
+
+      tempfile.rewind
+      ZipFile::Reader.new(tempfile)
+    end
+end

test/models/account/data_transfer/active_storage/blob_record_set_test.rb

@@ -0,0 +1,58 @@
+require "test_helper"
+
+class Account::DataTransfer::ActiveStorage::BlobRecordSetTest < ActiveSupport::TestCase
+  test "import generates fresh keys instead of using exported keys" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+    exported_key = "original-exported-key-abc123"
+
+    zip = build_zip_with_blob(id: blob_id, key: exported_key)
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+
+    blob = ActiveStorage::Blob.find(blob_id)
+    assert_not_equal exported_key, blob.key
+    assert_equal 28, blob.key.length
+  end
+
+  test "import preserves blob metadata" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+
+    zip = build_zip_with_blob(
+      id: blob_id,
+      key: "some-key",
+      filename: "report.pdf",
+      content_type: "application/pdf",
+      byte_size: 12345,
+      checksum: "abc123checksum"
+    )
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+
+    blob = ActiveStorage::Blob.find(blob_id)
+    assert_equal "report.pdf", blob.filename.to_s
+    assert_equal "application/pdf", blob.content_type
+    assert_equal 12345, blob.byte_size
+    assert_equal "abc123checksum", blob.checksum
+  end
+
+  private
+    def build_zip_with_blob(id:, key:, filename: "test.txt", content_type: "text/plain", byte_size: 32, checksum: "")
+      tempfile = Tempfile.new([ "test_export", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+      writer.add_file("data/active_storage_blobs/#{id}.json", {
+        id: id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: byte_size,
+        checksum: checksum,
+        content_type: content_type,
+        created_at: Time.current.iso8601,
+        filename: filename,
+        key: key,
+        metadata: {}
+      }.to_json)
+      writer.close
+
+      tempfile.rewind
+      ZipFile::Reader.new(tempfile)
+    end
+end

test/models/account/data_transfer/active_storage/file_record_set_test.rb

@@ -0,0 +1,177 @@
+require "test_helper"
+
+class Account::DataTransfer::ActiveStorage::FileRecordSetTest < ActiveSupport::TestCase
+  test "import uploads file data to blobs with regenerated keys" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+    old_key = "original-key-for-file"
+    file_content = "hello world file content"
+
+    zip = build_zip_with_blob_and_file(blob_id: blob_id, old_key: old_key, file_content: file_content)
+
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+    Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).import(from: zip)
+
+    blob = ActiveStorage::Blob.find(blob_id)
+    assert_not_equal old_key, blob.key
+    assert_equal file_content, blob.download
+  end
+
+  test "import handles keys containing path separators" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+    old_key = "folder/subfolder/file-key"
+    file_content = "nested key content"
+
+    zip = build_zip_with_blob_and_file(blob_id: blob_id, old_key: old_key, file_content: file_content)
+
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+    Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).import(from: zip)
+
+    blob = ActiveStorage::Blob.find(blob_id)
+    assert_not_equal old_key, blob.key
+    assert_equal file_content, blob.download
+  end
+
+  test "import raises IntegrityError for storage file without matching blob metadata" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+    old_key = "key-with-metadata"
+    orphan_key = "orphaned-storage-key"
+
+    zip = build_zip_with_orphaned_storage_file(
+      blob_id: blob_id,
+      old_key: old_key,
+      orphan_key: orphan_key
+    )
+
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+
+    assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do
+      Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).import(from: zip)
+    end
+  end
+
+  test "import raises IntegrityError when mapped blob is not found in database" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+    old_key = "key-for-missing-blob"
+
+    zip = build_zip_with_blob_and_file(blob_id: blob_id, old_key: old_key, file_content: "data")
+
+    # Import file data WITHOUT importing blob metadata first
+    assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do
+      Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).import(from: zip)
+    end
+  end
+
+  test "check raises IntegrityError for storage file without matching blob metadata" do
+    blob_id = ActiveRecord::Type::Uuid.generate
+    old_key = "key-with-metadata"
+    orphan_key = "orphaned-storage-key"
+
+    zip = build_zip_with_orphaned_storage_file(
+      blob_id: blob_id,
+      old_key: old_key,
+      orphan_key: orphan_key
+    )
+
+    assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do
+      Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).check(from: zip)
+    end
+  end
+
+  test "import raises IntegrityError for duplicate blob keys in export" do
+    blob_id_1 = ActiveRecord::Type::Uuid.generate
+    blob_id_2 = ActiveRecord::Type::Uuid.generate
+    duplicate_key = "same-key-for-both"
+
+    zip = build_zip_with_duplicate_keys(
+      blob_id_1: blob_id_1,
+      blob_id_2: blob_id_2,
+      key: duplicate_key
+    )
+
+    assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do
+      Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).import(from: zip)
+    end
+  end
+
+  private
+    def build_zip_with_blob_and_file(blob_id:, old_key:, file_content:)
+      tempfile = Tempfile.new([ "test_export", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+      writer.add_file("data/active_storage_blobs/#{blob_id}.json", {
+        id: blob_id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: file_content.bytesize,
+        checksum: Digest::MD5.base64digest(file_content),
+        content_type: "text/plain",
+        created_at: Time.current.iso8601,
+        filename: "test.txt",
+        key: old_key,
+        metadata: {}
+      }.to_json)
+      writer.add_file("storage/#{old_key}", file_content, compress: false)
+      writer.close
+
+      tempfile.rewind
+      ZipFile::Reader.new(tempfile)
+    end
+
+    def build_zip_with_orphaned_storage_file(blob_id:, old_key:, orphan_key:)
+      tempfile = Tempfile.new([ "test_export", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+      writer.add_file("data/active_storage_blobs/#{blob_id}.json", {
+        id: blob_id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: 10,
+        checksum: "",
+        content_type: "text/plain",
+        created_at: Time.current.iso8601,
+        filename: "test.txt",
+        key: old_key,
+        metadata: {}
+      }.to_json)
+      writer.add_file("storage/#{old_key}", "file data", compress: false)
+      writer.add_file("storage/#{orphan_key}", "orphan data", compress: false)
+      writer.close
+
+      tempfile.rewind
+      ZipFile::Reader.new(tempfile)
+    end
+
+    def build_zip_with_duplicate_keys(blob_id_1:, blob_id_2:, key:)
+      tempfile = Tempfile.new([ "test_export", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+      writer.add_file("data/active_storage_blobs/#{blob_id_1}.json", {
+        id: blob_id_1,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: 10,
+        checksum: "",
+        content_type: "text/plain",
+        created_at: Time.current.iso8601,
+        filename: "file1.txt",
+        key: key,
+        metadata: {}
+      }.to_json)
+      writer.add_file("data/active_storage_blobs/#{blob_id_2}.json", {
+        id: blob_id_2,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: 10,
+        checksum: "",
+        content_type: "text/plain",
+        created_at: Time.current.iso8601,
+        filename: "file2.txt",
+        key: key,
+        metadata: {}
+      }.to_json)
+      writer.add_file("storage/#{key}", "file data", compress: false)
+      writer.close
+
+      tempfile.rewind
+      ZipFile::Reader.new(tempfile)
+    end
+end

test/models/account/import_test.rb

@@ -174,6 +174,52 @@ class Account::ImportTest < ActiveSupport::TestCase
     export_tempfile&.unlink
   end
 
+  test "export and import round-trip preserves blobs and attachments" do
+    source_account = accounts("37s")
+    exporter = users(:david)
+    identity = exporter.identity
+
+    ActiveStorage::Blob.create_and_upload!(
+      io: StringIO.new("test image data"),
+      filename: "logo.png",
+      content_type: "image/png"
+    )
+
+    source_blob_count = ActiveStorage::Blob.where(account: source_account).count
+    source_blob_keys = ActiveStorage::Blob.where(account: source_account).pluck(:key)
+
+    assert_operator source_blob_count, :>, 0
+
+    export = Account::Export.create!(account: source_account, user: exporter)
+    export.build
+
+    export_tempfile = Tempfile.new([ "export", ".zip" ])
+    export.file.open { |f| FileUtils.cp(f.path, export_tempfile.path) }
+
+    ActiveStorage::Blob.where(account: source_account).delete_all
+    source_account.destroy!
+
+    target_account = Account.create_with_owner(account: { name: "Import Test" }, owner: { identity: identity, name: exporter.name })
+    import = Account::Import.create!(identity: identity, account: target_account)
+    Current.set(account: target_account) do
+      import.file.attach(io: File.open(export_tempfile.path), filename: "export.zip", content_type: "application/zip")
+    end
+
+    import.check
+    assert_not import.failed?
+
+    import.process
+    assert import.completed?
+
+    imported_blob = ActiveStorage::Blob.find_by(account: target_account, filename: "logo.png")
+    assert_not_nil imported_blob
+    assert_not_includes source_blob_keys, imported_blob.key
+    assert_equal "test image data", imported_blob.download
+  ensure
+    export_tempfile&.close
+    export_tempfile&.unlink
+  end
+
   private
     def account_digest(account)
       {