Do not remove files on soft-delete

app/Http/Controllers/Accessories/AccessoriesController.php

@@ -9,7 +9,6 @@ use App\Models\Accessory;
 use App\Models\Company;
 use Illuminate\Contracts\View\View;
 use Illuminate\Http\RedirectResponse;
-use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;
 use Illuminate\Support\Facades\Validator;
 
@@ -218,14 +217,11 @@ class AccessoriesController extends Controller
         $accessory->loadCount('checkouts as checkouts_count');
 
         if ($accessory->isDeletable()) {
-            if ($accessory->image) {
-                try {
-                    Storage::disk('public')->delete('accessories'.'/'.$accessory->image);
-                } catch (\Exception $e) {
-                    Log::debug($e);
-                }
-            }
-
+            // Note: the image file is deliberately preserved across this
+            // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+            // removes it later when the row is force-deleted. Keeping
+            // the file here means a restored soft-deleted row still has
+            // its image.
             $accessory->delete();
 
             return redirect()->route('accessories.index')->with('success', trans('admin/accessories/message.delete.success'));

app/Http/Controllers/Api/AssetModelsController.php

@@ -16,7 +16,6 @@ use App\Models\Setting;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 use Illuminate\Http\Response;
-use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;
 
 /**
@@ -278,14 +277,13 @@ class AssetModelsController extends Controller
             return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/models/message.assoc_users')));
         }
 
-        if ($assetmodel->image) {
-            try {
-                Storage::disk('public')->delete('assetmodels/'.$assetmodel->image);
-            } catch (\Exception $e) {
-                Log::info($e);
-            }
-        }
-
+        // Note: the image file is deliberately preserved across this
+        // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+        // removes it later when the row is force-deleted. Keeping the
+        // file here means a restored soft-deleted row still has its
+        // image. Also fixes a latent path bug: the old delete used
+        // `assetmodels/` but handleImages stores under `models/`, so
+        // the unlink here was silently missing the file anyway.
         $assetmodel->delete();
 
         return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/models/message.delete.success')));

app/Http/Controllers/Assets/AssetsController.php

@@ -564,14 +564,11 @@ class AssetsController extends Controller
                 ->update(['assigned_to' => null, 'assigned_type' => null]);
         }
 
-        if ($asset->image) {
-            try {
-                Storage::disk('public')->delete('assets/'.basename($asset->image));
-            } catch (\Exception $e) {
-                Log::debug($e);
-            }
-        }
-
+        // Note: the image file is deliberately preserved across this
+        // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+        // removes it later when the row is force-deleted. Keeping the
+        // file here means a restored soft-deleted row still has its
+        // image.
         $asset->delete();
 
         return redirect()->route('hardware.index')->with('success', trans('admin/hardware/message.delete.success'));

app/Http/Controllers/BulkAccessoriesController.php

@@ -5,8 +5,6 @@ namespace App\Http\Controllers;
 use App\Models\Accessory;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Gate;
-use Illuminate\Support\Facades\Log;
-use Illuminate\Support\Facades\Storage;
 
 class BulkAccessoriesController extends Controller
 {
@@ -49,14 +47,11 @@ class BulkAccessoriesController extends Controller
                 continue;
             }
 
-            if ($accessory->image) {
-                try {
-                    Storage::disk('public')->delete('accessories/'.$accessory->image);
-                } catch (\Exception $e) {
-                    Log::debug($e);
-                }
-            }
-
+            // Note: the image file is deliberately preserved across this
+            // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+            // removes it later when the row is force-deleted. Keeping
+            // the file here means a restored soft-deleted row still has
+            // its image.
             $accessory->delete();
             $success_count++;
         }

app/Http/Controllers/CompaniesController.php

@@ -13,8 +13,6 @@ use App\Models\User;
 use Illuminate\Contracts\View\View;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Log;
-use Illuminate\Support\Facades\Storage;
 
 /**
  * This controller handles all actions related to Companies for
@@ -155,14 +153,11 @@ final class CompaniesController extends Controller
                 ->with('error', trans('admin/companies/message.assoc_users'));
         }
 
-        if ($company->image) {
-            try {
-                Storage::disk('public')->delete('companies'.'/'.$company->image);
-            } catch (\Exception $e) {
-                Log::debug($e);
-            }
-        }
-
+        // Note: the image file is deliberately preserved across this
+        // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+        // removes it later when the row is force-deleted. Keeping the
+        // file here means a restored soft-deleted row still has its
+        // image.
         $company->delete();
 
         return redirect()->route('companies.index')

app/Http/Controllers/Components/ComponentsController.php

@@ -11,8 +11,6 @@ use App\Models\Component;
 use Illuminate\Auth\Access\AuthorizationException;
 use Illuminate\Contracts\View\View;
 use Illuminate\Http\RedirectResponse;
-use Illuminate\Support\Facades\Log;
-use Illuminate\Support\Facades\Storage;
 
 /**
  * This class controls all actions related to Components for
@@ -151,7 +149,7 @@ class ComponentsController extends Controller
     public function update(UpdateComponentRequest $request, Component $component)
     {
         $this->authorize('update', $component);
-        
+
         // Update the component data
         $component->name = $request->input('name');
         $component->category_id = $request->input('category_id');
@@ -200,15 +198,11 @@ class ComponentsController extends Controller
 
         $this->authorize('delete', $component);
 
-        // Remove the image if one exists
-        if ($component->image && Storage::disk('public')->exists('components/'.$component->image)) {
-            try {
-                Storage::disk('public')->delete('components/'.$component->image);
-            } catch (\Exception $e) {
-                Log::debug($e);
-            }
-        }
-
+        // Note: the image file is deliberately preserved across this
+        // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+        // removes it later when the row is force-deleted. Keeping the
+        // file here means a restored soft-deleted row still has its
+        // image.
         if ($component->numCheckedOut() > 0) {
             return redirect()->route('components.index')->with('error', trans('admin/components/message.delete.error_qty'));
         }

app/Http/Controllers/DepartmentsController.php

@@ -8,7 +8,6 @@ use App\Models\Department;
 use Illuminate\Contracts\View\View;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;
 
 class DepartmentsController extends Controller
@@ -115,13 +114,11 @@ class DepartmentsController extends Controller
             return redirect()->to(route('departments.index'))->with('error', trans('admin/departments/message.assoc_users'));
         }
 
-        if ($department->image) {
-            try {
-                Storage::disk('public')->delete('departments'.'/'.$department->image);
-            } catch (\Exception $e) {
-                Log::debug($e);
-            }
-        }
+        // Note: the image file is deliberately preserved across this
+        // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+        // removes it later when the row is force-deleted. Keeping the
+        // file here means a restored soft-deleted row still has its
+        // image.
         $department->delete();
 
         return redirect()->back()->with('success', trans('admin/departments/message.delete.success'));

app/Http/Controllers/LocationsController.php

@@ -243,13 +243,11 @@ class LocationsController extends Controller
 
         if ($location->isDeletable()) {
 
-            if ($location->image) {
-                try {
-                    Storage::disk('public')->delete('locations/'.$location->image);
-                } catch (\Exception $e) {
-                    Log::error($e);
-                }
-            }
+            // Note: the image file is deliberately preserved across this
+            // soft-delete. Snipe-IT's `snipeit:purge` command permanently
+            // removes it later when the row is force-deleted. Keeping
+            // the file here means a restored soft-deleted row still has
+            // its image.
             $location->delete();
 
             return redirect()->to(route('locations.index'))->with('success', trans('admin/locations/message.delete.success'));

tests/Feature/Console/Commands/PurgeTest.php

@@ -5,11 +5,13 @@ namespace Tests\Feature\Console\Commands;
 use App\Models\Accessory;
 use App\Models\Actionlog;
 use App\Models\Asset;
+use App\Models\CheckoutAcceptance;
 use App\Models\License;
 use App\Models\Location;
 use App\Models\Maintenance;
 use App\Models\User;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Storage;
 use Tests\TestCase;
 
 /**
@@ -122,16 +124,233 @@ class PurgeTest extends TestCase
 
     public function test_soft_deleted_user_with_show_in_list_zero_is_preserved(): void
     {
-        // System users (LDAP-sync placeholders, etc.) set show_in_list=0
-        // and are excluded from purge even when soft-deleted. This filter
-        // was in the pre-refactor code and must be preserved.
-        $systemUser = User::factory()->create(['show_in_list' => 0]);
-        $systemUser->delete();
+        // show_in_list=0 excludes a user from checkout-target dropdowns
+        // in the UI. Purge preserves these users so they stick around
+        // even when soft-deleted (matches the pre-refactor behavior).
+        $nonCheckoutUser = User::factory()->create(['show_in_list' => 0]);
+        $nonCheckoutUser->delete();
 
         $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
 
         // Row is gone-from-index (soft-deleted) but still in the table.
-        $this->assertDatabaseHas('users', ['id' => $systemUser->id]);
+        $this->assertDatabaseHas('users', ['id' => $nonCheckoutUser->id]);
+    }
+
+    public function test_purge_removes_uploaded_files_for_soft_deleted_users(): void
+    {
+        // Regression guard: an intermediate refactor of Purge dropped the
+        // Storage::delete() step that removes uploaded avatars/documents
+        // under private_uploads/users/ when a user is purged. Without
+        // this test, that call could silently disappear again and leave
+        // orphan files on disk. The old inline code lived in a per-user
+        // loop; the current implementation batches via a single
+        // action_logs query, and either shape needs to actually unlink
+        // the file for the corresponding trashed user.
+        Storage::fake();
+        $user = User::factory()->create();
+        $filename = "u{$user->id}-avatar.png";
+        Storage::put("private_uploads/users/{$filename}", 'fake image bytes');
+
+        Actionlog::factory()->create([
+            'item_type' => User::class,
+            'item_id' => $user->id,
+            'action_type' => 'uploaded',
+            'filename' => $filename,
+        ]);
+
+        $user->delete();
+        Storage::assertExists("private_uploads/users/{$filename}");
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::assertMissing("private_uploads/users/{$filename}");
+        $this->assertDatabaseMissing('users', ['id' => $user->id]);
+    }
+
+    public function test_dry_run_does_not_delete_user_files(): void
+    {
+        // Companion guard: --dry-run must be side-effect-free on disk.
+        Storage::fake();
+        $user = User::factory()->create();
+        $filename = "u{$user->id}-avatar.png";
+        Storage::put("private_uploads/users/{$filename}", 'fake image bytes');
+
+        Actionlog::factory()->create([
+            'item_type' => User::class,
+            'item_id' => $user->id,
+            'action_type' => 'uploaded',
+            'filename' => $filename,
+        ]);
+
+        $user->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true', '--dry-run' => true])->assertExitCode(0);
+
+        Storage::assertExists("private_uploads/users/{$filename}");
+    }
+
+    public function test_purge_removes_image_files_for_soft_deleted_assets(): void
+    {
+        // Image column on the parent row itself. Snipe-IT stores these
+        // on the public disk under `{plural-type}/{filename}`. Removing
+        // them at purge time (rather than at soft-delete) means a
+        // restored soft-deleted asset still has its image intact.
+        Storage::fake('public');
+        $asset = Asset::factory()->create(['image' => 'asset-42.jpg']);
+        Storage::disk('public')->put('assets/asset-42.jpg', 'fake image bytes');
+
+        $asset->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::disk('public')->assertMissing('assets/asset-42.jpg');
+        $this->assertDatabaseMissing('assets', ['id' => $asset->id]);
+    }
+
+    public function test_purge_removes_avatar_files_for_soft_deleted_users(): void
+    {
+        // Users' avatar column has its own public-disk subpath (`avatars`)
+        // distinct from every other model's `image` column. Covered
+        // separately because `UsersController::destroy` used to NOT
+        // delete the avatar and now (correctly) still doesn't; purge
+        // is the sole avatar-unlink path.
+        Storage::fake('public');
+        $user = User::factory()->create(['avatar' => 'user-7.jpg']);
+        Storage::disk('public')->put('avatars/user-7.jpg', 'fake avatar bytes');
+
+        $user->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::disk('public')->assertMissing('avatars/user-7.jpg');
+        $this->assertDatabaseMissing('users', ['id' => $user->id]);
+    }
+
+    public function test_purge_removes_eula_pdfs_when_action_log_parent_is_purged(): void
+    {
+        // Signed-EULA PDFs live under `private_uploads/eula-pdfs/`.
+        // They're identified by an action_log with action_type of
+        // `accepted` or `declined`, not by item_type, so the routing
+        // logic in Purge has to key off action_type first.
+        Storage::fake();
+        $asset = Asset::factory()->create();
+        $eula = "eula-{$asset->id}.pdf";
+        Storage::put("private_uploads/eula-pdfs/{$eula}", 'fake pdf bytes');
+
+        Actionlog::factory()->create([
+            'item_type' => Asset::class,
+            'item_id' => $asset->id,
+            'action_type' => 'accepted',
+            'filename' => $eula,
+        ]);
+
+        $asset->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::assertMissing("private_uploads/eula-pdfs/{$eula}");
+    }
+
+    public function test_purge_removes_signature_files_from_action_logs(): void
+    {
+        // Signatures live under `private_uploads/signatures/` and are
+        // referenced by the `accept_signature` column on action_logs
+        // (not by the `filename` column and not by any specific
+        // action_type). Purge must read that column separately.
+        Storage::fake();
+        $asset = Asset::factory()->create();
+        $sig = "sig-{$asset->id}.png";
+        Storage::put("private_uploads/signatures/{$sig}", 'fake signature bytes');
+
+        Actionlog::factory()->create([
+            'item_type' => Asset::class,
+            'item_id' => $asset->id,
+            'action_type' => 'checkout',
+            'accept_signature' => $sig,
+        ]);
+
+        $asset->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::assertMissing("private_uploads/signatures/{$sig}");
+    }
+
+    public function test_purge_removes_audit_files_from_action_logs(): void
+    {
+        // Audit files (photos, notes attached during an audit) live
+        // under `private_uploads/audits/` and are keyed on
+        // `action_type = 'audit'` in the action_log.
+        Storage::fake();
+        $asset = Asset::factory()->create();
+        $auditFile = "audit-{$asset->id}.jpg";
+        Storage::put("private_uploads/audits/{$auditFile}", 'fake audit photo');
+
+        Actionlog::factory()->create([
+            'item_type' => Asset::class,
+            'item_id' => $asset->id,
+            'action_type' => 'audit',
+            'filename' => $auditFile,
+        ]);
+
+        $asset->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::assertMissing("private_uploads/audits/{$auditFile}");
+    }
+
+    public function test_purge_matches_action_log_files_via_target_columns_too(): void
+    {
+        // Signatures/EULAs for checkouts are recorded with the
+        // checkoutable item under `item_*` and the recipient user under
+        // `target_*`. Purging the recipient user must clean up their
+        // signature file, even though the action_log's `item_type`
+        // points at Asset (not User).
+        Storage::fake();
+        $user = User::factory()->create();
+        $sig = "user-{$user->id}-sig.png";
+        Storage::put("private_uploads/signatures/{$sig}", 'fake signature bytes');
+
+        Actionlog::factory()->create([
+            'item_type' => Asset::class,
+            'item_id' => Asset::factory()->create()->id,
+            'target_type' => User::class,
+            'target_id' => $user->id,
+            'action_type' => 'checkout',
+            'accept_signature' => $sig,
+        ]);
+
+        $user->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::assertMissing("private_uploads/signatures/{$sig}");
+    }
+
+    public function test_purge_removes_checkout_acceptance_signature_and_eula_files(): void
+    {
+        // CheckoutAcceptance stores its signature filename and the
+        // rendered EULA PDF inline on the row (not via a related
+        // action_log). Both need to be unlinked when the acceptance is
+        // itself purged.
+        Storage::fake();
+        $acceptance = CheckoutAcceptance::factory()
+            ->withoutActionLog()
+            ->accepted()
+            ->create([
+                'signature_filename' => 'acceptance-sig.png',
+                'stored_eula_file' => 'acceptance-eula.pdf',
+            ]);
+        Storage::put('private_uploads/signatures/acceptance-sig.png', 'sig bytes');
+        Storage::put('private_uploads/eula-pdfs/acceptance-eula.pdf', 'pdf bytes');
+
+        $acceptance->delete();
+
+        $this->artisan('snipeit:purge', ['--force' => 'true'])->assertExitCode(0);
+
+        Storage::assertMissing('private_uploads/signatures/acceptance-sig.png');
+        Storage::assertMissing('private_uploads/eula-pdfs/acceptance-eula.pdf');
     }
 
     public function test_soft_deleted_location_is_purged(): void