mirror of
https://github.com/HolgerHatGarKeineNode/einundzwanzig-nostr.git
synced 2026-02-15 03:23:17 +00:00
[P0 Security] Mass Assignment Protection – $fillable für alle 18 Models (vibe-kanban 4a764a11)
## Security Audit: Mass Assignment Protection ### Problem Alle 18 Eloquent Models verwenden `protected $guarded = [];` – das bedeutet **kein Schutz** gegen Mass Assignment. Ein Angreifer könnte über manipulierte Requests sensible Felder wie `accepted`, `sats_paid`, `association_status`, `paid` oder `created_by` direkt setzen. ### Betroffene Dateien und empfohlene Änderungen Ersetze in **jedem** der folgenden Models `protected $guarded = [];` durch ein explizites `protected $fillable = [...]` Array. Hier die Analyse pro Model: **Höchstes Risiko (Finanzen & Identity):** 1. **`app/Models/PaymentEvent.php`** – Finanz-kritisch! - Sensible Felder (NICHT fillable): `einundzwanzig_pleb_id`, `year`, `amount`, `event_id`, `paid`, `btc_pay_invoice` - `$fillable` sollte leer oder minimal sein – alle Felder werden programmatisch gesetzt 2. **`app/Models/EinundzwanzigPleb.php`** - Sensible Felder: `association_status`, `application_for`, `nip05_handle` - `$fillable = ['npub', 'pubkey', 'email', 'no_email', 'application_text', 'archived_application_text']` 3. **`app/Models/Vote.php`** - Sensible Felder: `einundzwanzig_pleb_id`, `project_proposal_id`, `value` - `$fillable = ['reason']` – alle anderen Felder müssen programmatisch gesetzt werden 4. **`app/Models/ProjectProposal.php`** - Sensible Felder: `einundzwanzig_pleb_id`, `accepted`, `sats_paid`, `slug` - `$fillable = ['name', 'support_in_sats', 'description', 'website']` 5. **`app/Models/Election.php`** - Sensible Felder: `year`, `candidates`, `end_time` - `$fillable` sollte leer sein – nur Admin-gesteuert **Mittleres Risiko (mit `created_by` auto-fill in boot):** 6. **`app/Models/Venue.php`** – `$fillable = ['name']` (slug & created_by auto-generiert) 7. **`app/Models/MeetupEvent.php`** – `$fillable = ['start']` (meetup_id, created_by, attendees guarded) 8. **`app/Models/CourseEvent.php`** – `$fillable = ['from', 'to']` (course_id, venue_id, created_by guarded) 9. **`app/Models/Course.php`** – `$fillable = ['name', 'description']` (lecturer_id, created_by guarded) 10. **`app/Models/Meetup.php`** – `$fillable = ['name']` (city_id, created_by, slug, github_data, simplified_geojson guarded) 11. **`app/Models/Lecturer.php`** – `$fillable = ['name']` (active, created_by, slug guarded) 12. **`app/Models/City.php`** – `$fillable = ['name']` (country_id, created_by, slug, osm_relation, simplified_geojson guarded) **Niedrigeres Risiko (Lookup/Reference-Daten):** 13. **`app/Models/Event.php`** – `$fillable = []` (alle Felder: event_id, parent_event_id, pubkey, json, type sind extern gesteuert) 14. **`app/Models/RenderedEvent.php`** – `$fillable = []` (event_id, html, profile_image, profile_name alle system-generiert) 15. **`app/Models/Profile.php`** – `$fillable = ['name', 'display_name', 'picture', 'banner', 'website', 'about']` (pubkey, deleted, nip05, lud16, lud06 guarded) 16. **`app/Models/Category.php`** – `$fillable = ['name']` 17. **`app/Models/Country.php`** – `$fillable = ['name']` (code, language_codes guarded) 18. **`app/Models/Notification.php`** – `$fillable = ['name', 'description']` (einundzwanzig_pleb_id, category guarded) ### Vorgehen 1. Jedes Model öffnen und `$guarded = []` durch das oben definierte `$fillable` Array ersetzen 2. Prüfen, ob bestehende `::create()` oder `::update()` Aufrufe noch funktionieren – ggf. müssen explizite Feld-Zuweisungen ergänzt werden 3. Für jedes geänderte Model einen Pest-Test schreiben, der verifiziert, dass Mass Assignment von sensiblen Feldern blockiert wird 4. `vendor/bin/pint --dirty` ausführen 5. Bestehende Tests laufen lassen: `php artisan test --compact` ### Akzeptanzkriterien - Kein Model hat mehr `$guarded = []` - Alle sensiblen Felder (status, paid, accepted, created_by, slug, IDs) sind NICHT in `$fillable` - Bestehende Features funktionieren weiterhin (Tests grün) - Neue Tests verifizieren Mass Assignment Protection
This commit is contained in:
288
tests/Feature/MassAssignmentProtectionTest.php
Normal file
288
tests/Feature/MassAssignmentProtectionTest.php
Normal file
@@ -0,0 +1,288 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Models\Category;
|
||||
use App\Models\City;
|
||||
use App\Models\Country;
|
||||
use App\Models\Course;
|
||||
use App\Models\CourseEvent;
|
||||
use App\Models\EinundzwanzigPleb;
|
||||
use App\Models\Election;
|
||||
use App\Models\Event;
|
||||
use App\Models\Lecturer;
|
||||
use App\Models\Meetup;
|
||||
use App\Models\MeetupEvent;
|
||||
use App\Models\Notification;
|
||||
use App\Models\PaymentEvent;
|
||||
use App\Models\Profile;
|
||||
use App\Models\ProjectProposal;
|
||||
use App\Models\RenderedEvent;
|
||||
use App\Models\Venue;
|
||||
use App\Models\Vote;
|
||||
use Illuminate\Database\Eloquent\MassAssignmentException;
|
||||
|
||||
it('ensures no model uses guarded empty array', function () {
|
||||
$models = [
|
||||
PaymentEvent::class,
|
||||
EinundzwanzigPleb::class,
|
||||
Vote::class,
|
||||
ProjectProposal::class,
|
||||
Election::class,
|
||||
Venue::class,
|
||||
MeetupEvent::class,
|
||||
CourseEvent::class,
|
||||
Course::class,
|
||||
Meetup::class,
|
||||
Lecturer::class,
|
||||
City::class,
|
||||
Event::class,
|
||||
RenderedEvent::class,
|
||||
Profile::class,
|
||||
Category::class,
|
||||
Country::class,
|
||||
Notification::class,
|
||||
];
|
||||
|
||||
foreach ($models as $modelClass) {
|
||||
$reflection = new ReflectionClass($modelClass);
|
||||
$property = $reflection->getProperty('guarded');
|
||||
$instance = $reflection->newInstanceWithoutConstructor();
|
||||
$guarded = $property->getValue($instance);
|
||||
|
||||
expect($guarded)
|
||||
->not->toBe([], "{$modelClass} still uses \$guarded = [] which is insecure");
|
||||
}
|
||||
});
|
||||
|
||||
it('ensures all models have explicit fillable arrays', function () {
|
||||
$models = [
|
||||
PaymentEvent::class,
|
||||
EinundzwanzigPleb::class,
|
||||
Vote::class,
|
||||
ProjectProposal::class,
|
||||
Election::class,
|
||||
Venue::class,
|
||||
MeetupEvent::class,
|
||||
CourseEvent::class,
|
||||
Course::class,
|
||||
Meetup::class,
|
||||
Lecturer::class,
|
||||
City::class,
|
||||
Event::class,
|
||||
RenderedEvent::class,
|
||||
Profile::class,
|
||||
Category::class,
|
||||
Country::class,
|
||||
Notification::class,
|
||||
];
|
||||
|
||||
foreach ($models as $modelClass) {
|
||||
$reflection = new ReflectionClass($modelClass);
|
||||
$property = $reflection->getProperty('fillable');
|
||||
$instance = $reflection->newInstanceWithoutConstructor();
|
||||
|
||||
expect($property->getValue($instance))
|
||||
->toBeArray("{$modelClass} should have an explicit \$fillable array");
|
||||
}
|
||||
});
|
||||
|
||||
it('blocks mass assignment of einundzwanzig_pleb_id on PaymentEvent', function () {
|
||||
$paymentEvent = new PaymentEvent;
|
||||
$paymentEvent->fill(['einundzwanzig_pleb_id' => 999]);
|
||||
|
||||
expect($paymentEvent->einundzwanzig_pleb_id)->toBeNull();
|
||||
});
|
||||
|
||||
it('verifies EinundzwanzigPleb fillable does not contain application_for', function () {
|
||||
$reflection = new ReflectionClass(EinundzwanzigPleb::class);
|
||||
$property = $reflection->getProperty('fillable');
|
||||
$instance = $reflection->newInstanceWithoutConstructor();
|
||||
$fillable = $property->getValue($instance);
|
||||
|
||||
expect($fillable)->not->toContain('application_for');
|
||||
expect($fillable)->not->toContain('id');
|
||||
expect($fillable)->toContain('npub');
|
||||
expect($fillable)->toContain('pubkey');
|
||||
expect($fillable)->toContain('email');
|
||||
expect($fillable)->toContain('no_email');
|
||||
expect($fillable)->toContain('nip05_handle');
|
||||
});
|
||||
|
||||
it('blocks mass assignment of accepted and sats_paid on ProjectProposal', function () {
|
||||
$proposal = new ProjectProposal;
|
||||
$proposal->fill([
|
||||
'name' => 'Test',
|
||||
'accepted' => true,
|
||||
'sats_paid' => 100000,
|
||||
'einundzwanzig_pleb_id' => 1,
|
||||
'slug' => 'injected-slug',
|
||||
]);
|
||||
|
||||
expect($proposal->accepted)->toBeNull();
|
||||
expect($proposal->sats_paid)->toBeNull();
|
||||
expect($proposal->einundzwanzig_pleb_id)->toBeNull();
|
||||
expect($proposal->slug)->toBeNull();
|
||||
expect($proposal->name)->toBe('Test');
|
||||
});
|
||||
|
||||
it('blocks mass assignment of all fields on Election', function () {
|
||||
$election = new Election;
|
||||
|
||||
expect(fn () => $election->fill(['year' => 2025]))
|
||||
->toThrow(MassAssignmentException::class);
|
||||
});
|
||||
|
||||
it('blocks mass assignment of created_by and slug on Venue', function () {
|
||||
$venue = new Venue;
|
||||
$venue->fill([
|
||||
'name' => 'Test Venue',
|
||||
'created_by' => 999,
|
||||
'slug' => 'injected-slug',
|
||||
]);
|
||||
|
||||
expect($venue->name)->toBe('Test Venue');
|
||||
expect($venue->created_by)->toBeNull();
|
||||
expect($venue->slug)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of meetup_id and created_by on MeetupEvent', function () {
|
||||
$event = new MeetupEvent;
|
||||
$event->fill([
|
||||
'start' => '2025-01-01',
|
||||
'meetup_id' => 999,
|
||||
'created_by' => 999,
|
||||
'attendees' => ['a'],
|
||||
]);
|
||||
|
||||
expect($event->start)->not->toBeNull();
|
||||
expect($event->meetup_id)->toBeNull();
|
||||
expect($event->created_by)->toBeNull();
|
||||
expect($event->attendees)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of course_id venue_id and created_by on CourseEvent', function () {
|
||||
$event = new CourseEvent;
|
||||
$event->fill([
|
||||
'from' => '2025-01-01',
|
||||
'to' => '2025-01-02',
|
||||
'course_id' => 999,
|
||||
'venue_id' => 999,
|
||||
'created_by' => 999,
|
||||
]);
|
||||
|
||||
expect($event->from)->not->toBeNull();
|
||||
expect($event->to)->not->toBeNull();
|
||||
expect($event->course_id)->toBeNull();
|
||||
expect($event->venue_id)->toBeNull();
|
||||
expect($event->created_by)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of lecturer_id and created_by on Course', function () {
|
||||
$course = new Course;
|
||||
$course->fill([
|
||||
'name' => 'Test Course',
|
||||
'description' => 'Test',
|
||||
'lecturer_id' => 999,
|
||||
'created_by' => 999,
|
||||
]);
|
||||
|
||||
expect($course->name)->toBe('Test Course');
|
||||
expect($course->description)->toBe('Test');
|
||||
expect($course->lecturer_id)->toBeNull();
|
||||
expect($course->created_by)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of city_id created_by and slug on Meetup', function () {
|
||||
$meetup = new Meetup;
|
||||
$meetup->fill([
|
||||
'name' => 'Test Meetup',
|
||||
'city_id' => 999,
|
||||
'created_by' => 999,
|
||||
'slug' => 'injected',
|
||||
'github_data' => '{}',
|
||||
'simplified_geojson' => '{}',
|
||||
]);
|
||||
|
||||
expect($meetup->name)->toBe('Test Meetup');
|
||||
expect($meetup->city_id)->toBeNull();
|
||||
expect($meetup->created_by)->toBeNull();
|
||||
expect($meetup->slug)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of active created_by and slug on Lecturer', function () {
|
||||
$lecturer = new Lecturer;
|
||||
$lecturer->fill([
|
||||
'name' => 'Test Lecturer',
|
||||
'active' => true,
|
||||
'created_by' => 999,
|
||||
'slug' => 'injected',
|
||||
]);
|
||||
|
||||
expect($lecturer->name)->toBe('Test Lecturer');
|
||||
expect($lecturer->active)->toBeNull();
|
||||
expect($lecturer->created_by)->toBeNull();
|
||||
expect($lecturer->slug)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of country_id created_by and slug on City', function () {
|
||||
$city = new City;
|
||||
$city->fill([
|
||||
'name' => 'Test City',
|
||||
'country_id' => 999,
|
||||
'created_by' => 999,
|
||||
'slug' => 'injected',
|
||||
'osm_relation' => '{}',
|
||||
'simplified_geojson' => '{}',
|
||||
]);
|
||||
|
||||
expect($city->name)->toBe('Test City');
|
||||
expect($city->country_id)->toBeNull();
|
||||
expect($city->created_by)->toBeNull();
|
||||
expect($city->slug)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of einundzwanzig_pleb_id and category on Notification', function () {
|
||||
$notification = new Notification;
|
||||
$notification->fill([
|
||||
'name' => 'Test News',
|
||||
'description' => 'Test',
|
||||
'einundzwanzig_pleb_id' => 999,
|
||||
'category' => 1,
|
||||
]);
|
||||
|
||||
expect($notification->name)->toBe('Test News');
|
||||
expect($notification->description)->toBe('Test');
|
||||
expect($notification->einundzwanzig_pleb_id)->toBeNull();
|
||||
expect($notification->category)->toBeNull();
|
||||
});
|
||||
|
||||
it('blocks mass assignment of code and language_codes on Country', function () {
|
||||
$country = new Country;
|
||||
$country->fill([
|
||||
'name' => 'Test',
|
||||
'code' => 'XX',
|
||||
'language_codes' => ['en'],
|
||||
]);
|
||||
|
||||
expect($country->name)->toBe('Test');
|
||||
expect($country->code)->toBeNull();
|
||||
expect($country->language_codes)->toBeNull();
|
||||
});
|
||||
|
||||
it('allows fillable fields on PaymentEvent', function () {
|
||||
$paymentEvent = new PaymentEvent;
|
||||
$paymentEvent->fill([
|
||||
'year' => 2025,
|
||||
'event_id' => 'test-event',
|
||||
'amount' => 21000,
|
||||
'paid' => true,
|
||||
'btc_pay_invoice' => 'inv-123',
|
||||
]);
|
||||
|
||||
expect($paymentEvent->year)->toBe(2025);
|
||||
expect($paymentEvent->event_id)->toBe('test-event');
|
||||
expect($paymentEvent->amount)->toBe(21000);
|
||||
expect($paymentEvent->paid)->toBeTrue();
|
||||
expect($paymentEvent->btc_pay_invoice)->toBe('inv-123');
|
||||
});
|
||||
Reference in New Issue
Block a user