Fix Broken Access Control (CWE-285) and update CLAUDE.md by ry2811 · Pull Request #163 · ozdemirburak/simple-cms

ry2811 · 2026-03-10T17:02:32Z

Description

This PR addresses a critical Broken Access Control vulnerability (CWE-285) where any user with an 'Editor' role could delete Media and Pages created by an Administrator.

As requested, I have also updated CLAUDE.md to include the new security guidelines and build instructions to assist AI-based workflows.

Vulnerability Breakdown

Type: Broken Access Control (OWASP A01:2021).
Root Cause: Lack of object-level authorization. Resources like Article, Page, and Media were not tracking ownership, and global DeleteAction was enabled without permission checks.

Changes Implemented

Database: Added user_id to articles, pages, and media tables via migration.
Models: Implemented ownership tracking using Model Observers/Booted methods.
Authorization: - Created Laravel Policies for Article, Page, and Media.
- Restricted delete and update actions to owners and administrators only.
Filament Resources: Updated getEloquentQuery() to scope data access based on user roles.
Documentation: Updated CLAUDE.md with the new security-first coding standards.

Proof of Concept (PoC)

Login as a standard 'Editor'.
Navigate to /admin/pages.
An Editor can no longer see or delete Pages created by an Administrator. (Before this fix, an Editor could permanently delete any system Page).

Documentation Update

Updated CLAUDE.md to ensure future development (especially via Claude Code or Copilot) follows these authorization patterns.

Note: Since this fix addresses a security vulnerability, I would appreciate it if we could coordinate on a CVE assignment once merged.

ry2811 · 2026-03-10T17:03:05Z

Hi! I've implemented the requested security enhancements and updated CLAUDE.md with the new guidelines. Looking forward to your review!

Copilot

Pull request overview

This PR aims to remediate a broken object-level access control issue by introducing per-record ownership (user_id) for Articles/Pages/Media, enforcing ownership checks via Laravel Policies, and scoping Filament resource queries so non-admins only see their own records.

Changes:

Add user_id ownership columns to articles, pages, and media.
Introduce ArticlePolicy, PagePolicy, and MediaPolicy, and register them via Gate::policy(...).
Scope Filament getEloquentQuery() for Articles/Pages/Media to restrict non-admin visibility; add user()/hasMany() relationships and auto-assign user_id on create.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
database/seeders/ContentSeeder.php	Seed content adjusted (currently contains a PHP parse error in the new `content` value).
database/migrations/2026_03_10_000000_add_user_id_to_articles_pages_media_tables.php	Adds nullable ownership FK columns and indexes (needs index/backfill considerations).
config/media-library.php	Configures Spatie Media Library to use `App\Models\Media`.
app/Providers/AppServiceProvider.php	Registers policy mappings via `Gate::policy`.
app/Policies/{Article,Page,Media}Policy.php	Adds ownership-based authorization rules for update/delete/etc.
app/Models/{User,Article,Page,Media}.php	Adds relationships, makes `user_id` fillable, and auto-assigns `user_id` on create.
app/Filament/Resources/{Articles,Pages,Media}/*Resource.php	Scopes queries by `user_id` for non-admins (Media resource model class mismatch needs correction).
CLAUDE.md	Documents the new ownership/policy/scoping guidelines.

Comments suppressed due to low confidence (1)

app/Filament/Resources/Media/MediaResource.php:17

MediaResource is using Spatie's base Media model, but the PR introduces App\Models\Media (and registers MediaPolicy for that class, plus config points to it). With the current import/model, policy enforcement and ownership assignment hooks won't apply consistently to records managed in this resource. Switch the resource model (and import) to App\Models\Media so policies/scoping apply to the same class used throughout the app.

use Spatie\MediaLibrary\MediaCollections\Models\Media;

class MediaResource extends Resource
{
    protected static ?string $model = Media::class;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

database/seeders/ContentSeeder.php

Copilot · 2026-03-10T17:19:06Z

database/migrations/2026_03_10_000000_add_user_id_to_articles_pages_media_tables.php

+        Schema::table('articles', function (Blueprint $table) {
+            $table->foreignId('user_id')->nullable()->after('id')->constrained()->onDelete('cascade');
+            $table->index('user_id');
+        });


This migration adds an extra index('user_id') right after foreignId(...)->constrained(). Depending on the database, the FK may already create an index; adding another can cause duplicate-index issues and the down() only drops one index. Prefer a single index definition (e.g., foreignId(...)->constrained()->cascadeOnDelete()->index() or omit the explicit index if it's already created).

database/migrations/2026_03_10_000000_add_user_id_to_articles_pages_media_tables.php

Copilot · 2026-03-10T17:19:07Z

app/Policies/ArticlePolicy.php

+    public function update(User $user, Article $article): bool
+    {
+        // Admin can update anything, Editor can only update their own
+        return $user->isAdmin() || $article->user_id === $user->id;
+    }
+
+    /**
+     * Determine whether the user can delete the model.
+     */
+    public function delete(User $user, Article $article): bool
+    {
+        // Admin can delete anything, Editor can only delete their own
+        return $user->isAdmin() || $article->user_id === $user->id;
+    }


New ownership-based authorization is introduced here, but there are no corresponding feature tests asserting that an Editor cannot update/delete an Article owned by an Admin (and can still manage their own). Add coverage (e.g., in tests/Feature/AdminCrudTest.php or a dedicated security test) that exercises the Filament delete/edit actions and verifies 403/denial for non-owners.

Copilot · 2026-03-10T17:19:07Z

app/Policies/MediaPolicy.php

+    public function update(User $user, Media $media): bool
+    {
+        // Admin can update anything, Editor can only update their own
+        return $user->isAdmin() || $media->user_id === $user->id;
+    }
+
+    /**
+     * Determine whether the user can delete the model.
+     */
+    public function delete(User $user, Media $media): bool
+    {
+        // Admin can delete anything, Editor can only delete their own
+        return $user->isAdmin() || $media->user_id === $user->id;
+    }


Please add feature coverage for the new Media ownership rules (e.g., Editor cannot delete media uploaded by another user/admin, but can delete their own). This is especially important because Media is managed through Filament actions/bulk actions and the vulnerability being fixed is broken object-level authorization.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ozdemirburak · 2026-03-15T11:23:22Z

Thanks, @ry2811, merged it. There was one weird empty file called git in the commit, any reason for that?

Also asking out of curiosity, how did you find this project, did you just stumble upon it randomly?

ozdemirburak · 2026-03-15T11:28:18Z

git

What is this file @ry2811?

ry2811 · 2026-03-16T03:36:44Z

Thanks for merging the PR!

About the weird git file: That was definitely a typo on my part while running some CLI commands. My apologies for that clutter!

To answer your curiosity: I’m currently a first-year Information Security student. I've been specifically looking into CMS projects built with Laravel and Filament to study their architecture and security patterns. I came across your project and noticed the access control logic could be tightened up, so I thought I'd contribute.

Since the fix for the Broken Access Control (CWE-285) is now merged, would you be open to coordinating a GitHub Security Advisory and assigning a CVE for this? I’d be happy to help with the write-up and documentation to credit the discovery properly and inform the users.

Khoi added 4 commits March 10, 2026 23:17

Your commit message

c4ce745

fix: implement object-level authorization and update CLAUDE.md

ff0d467

fix: implement object-level authorization and update CLAUDE.md

4a7360d

fix: implement object-level authorization and update CLAUDE.md

9133e00

Copilot AI review requested due to automatic review settings March 10, 2026 17:02

Copilot started reviewing on behalf of ry2811 March 10, 2026 17:03 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Update database/seeders/ContentSeeder.php

7485bb8

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ozdemirburak merged commit 54174b6 into ozdemirburak:master Mar 15, 2026

ozdemirburak reviewed Mar 15, 2026

View reviewed changes

git

Copy link

Owner

ozdemirburak Mar 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this file @ry2811?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Broken Access Control (CWE-285) and update CLAUDE.md#163

Fix Broken Access Control (CWE-285) and update CLAUDE.md#163
ozdemirburak merged 5 commits intoozdemirburak:masterfrom
ry2811:fix/access-control

ry2811 commented Mar 10, 2026

Uh oh!

ry2811 commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

ozdemirburak commented Mar 15, 2026 •

edited

Loading

Uh oh!

ozdemirburak Mar 15, 2026

Uh oh!

ry2811 commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ry2811 commented Mar 10, 2026

Description

Vulnerability Breakdown

Changes Implemented

Proof of Concept (PoC)

Documentation Update

Uh oh!

ry2811 commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

ozdemirburak commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ozdemirburak Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

ry2811 commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ozdemirburak commented Mar 15, 2026 •

edited

Loading