diff --git a/docs/09-advanced/06-security.md b/docs/09-advanced/06-security.md index 22c0586937..d7587dd130 100644 --- a/docs/09-advanced/06-security.md +++ b/docs/09-advanced/06-security.md @@ -55,7 +55,9 @@ TextColumn::make('website') }) ``` -Similarly, the `icon()` method expects either a Blade icon name (like `heroicon-o-user`) or a valid image URL. If you pass unsanitized user input, it could be used to break out of HTML attributes. Always ensure icon values are either known icon names or validated URLs. +The `icon()` method expects either a Blade icon name (like `heroicon-o-user`) or an image URL (any string containing `/`). Icon name strings are resolved via Blade's icon system, and URL strings are escaped before rendering into `src` attributes. However, passing an invalid icon name from user input will cause a rendering error, so you should still validate icon values against a known allowlist if they are user-controlled. + +Methods like `extraAttributes()`, `extraInputAttributes()`, `extraCellAttributes()`, and other `extra*Attributes()` methods render their values into HTML without escaping. This is by design, as these methods are often used to pass Alpine.js directives and Livewire attributes that must not be escaped. However, if you pass user-controlled data as attribute names or values, an attacker could break out of the HTML attribute and inject arbitrary markup, leading to XSS. Always ensure that any dynamic values passed to these methods are validated or sourced from trusted data. As a general rule: whenever you pass user-controlled data into a Filament configuration method, treat it with the same caution you would when rendering it directly in a Blade template. @@ -117,6 +119,10 @@ By default, all `App\Models\User` records can access Filament panels in local en If your application has multiple panels (e.g. an admin panel and a user-facing panel), ensure that `canAccessPanel()` checks the `$panel` argument and returns the appropriate result for each one. +### Multi-factor authentication + +Filament supports [multi-factor authentication](../users/multi-factor-authentication) via TOTP apps and email codes, but it is not enabled by default. MFA is enforced within the Filament panel authentication flow — if your application has other authentication paths (such as API routes or non-Filament login pages), MFA will not be enforced on those paths unless you implement it separately. + ## Model attribute exposure Filament exposes all non-`$hidden` model attributes to JavaScript via Livewire's model binding. This is necessary for dynamic form functionality, and only attributes with corresponding form fields are actually editable — this is not a mass assignment vulnerability. However, if your model contains sensitive attributes that should not be visible in the browser (such as API keys or internal flags), you should either add them to the model's `$hidden` property or remove them using the `mutateFormDataBeforeFill()` method on your Edit or View page. See the [resources documentation](../resources/overview#protecting-model-attributes) for more details. @@ -125,7 +131,7 @@ Filament exposes all non-`$hidden` model attributes to JavaScript via Livewire's Filament's `FileUpload` component uses Livewire's file upload mechanism. There are important security considerations when allowing users to upload files, particularly around file names, storage visibility, and accepted file types. -By default, Filament generates random file names and stores files with `private` visibility. If you use `preserveFilenames()` or `storeFileNamesIn()` with local or public disks, an attacker could upload a PHP file with a deceptive MIME type that gets executed by your server. See the [file upload documentation](../forms/file-upload#security-implications-of-controlling-file-names) for a full explanation of these risks and recommended mitigations. +By default, Filament generates random file names and stores files with `private` visibility. If you use `preserveFilenames()` or `getUploadedFileNameForStorageUsing()` with local or public disks, an attacker could upload a PHP file with a deceptive MIME type that gets executed by your server. The safer alternative is to use `storeFileNamesIn()`, which stores original file names in a separate database column while keeping randomly generated file names on disk. See the [file upload documentation](../forms/file-upload#security-implications-of-controlling-file-names) for a full explanation of these risks and recommended mitigations. You should always use `acceptedFileTypes()` to restrict the types of files users can upload, and validate file sizes with `maxSize()`. These constraints are enforced server-side, not just in the browser. diff --git a/packages/actions/src/Action.php b/packages/actions/src/Action.php index 730d4a49d4..a311edd6f1 100644 --- a/packages/actions/src/Action.php +++ b/packages/actions/src/Action.php @@ -309,6 +309,9 @@ class Action extends ViewComponent implements Arrayable public function alpineClickHandler(string | Closure | null $handler): static { + // Security: This JavaScript expression is evaluated on the client. + // Never pass user input — only developer-defined expressions. + $this->alpineClickHandler = $handler; $this->livewireClickHandlerEnabled(blank($handler)); @@ -317,6 +320,9 @@ class Action extends ViewComponent implements Arrayable public function actionJs(string | Closure | null $action): static { + // Security: This JavaScript expression is evaluated on the client. + // Never pass user input — only developer-defined expressions. + $this->alpineClickHandler($action); return $this; diff --git a/packages/actions/src/ActionGroup.php b/packages/actions/src/ActionGroup.php index e3bf94bf9e..d9414b07a9 100644 --- a/packages/actions/src/ActionGroup.php +++ b/packages/actions/src/ActionGroup.php @@ -744,6 +744,9 @@ class ActionGroup extends ViewComponent implements Arrayable, HasEmbeddedView */ public function extraDropdownAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraDropdownAttributes[] = $attributes; } else { diff --git a/packages/actions/src/Concerns/CanBeAuthorized.php b/packages/actions/src/Concerns/CanBeAuthorized.php index 1759fbca81..6af9d14858 100644 --- a/packages/actions/src/Concerns/CanBeAuthorized.php +++ b/packages/actions/src/Concerns/CanBeAuthorized.php @@ -12,6 +12,11 @@ use LogicException; trait CanBeAuthorized { + // Security: Actions do not have automatic policy-based authorization. + // Authorization defaults to `null` (allowed for all users). + // You must explicitly use `authorize()`, `visible()`, or + // `hidden()` to restrict access to custom actions. + protected mixed $authorization = null; protected string | Closure | null $authorizationMessage = null; diff --git a/packages/actions/src/Concerns/CanExportRecords.php b/packages/actions/src/Concerns/CanExportRecords.php index 2da0d06dc5..8c9e194522 100644 --- a/packages/actions/src/Concerns/CanExportRecords.php +++ b/packages/actions/src/Concerns/CanExportRecords.php @@ -284,7 +284,8 @@ trait CanExportRecords // Temporary save to obtain the sequence number of the export file. $export->save(); - // Delete the export directory to prevent data contamination from previous exports with the same ID. + // Delete the export directory to prevent data contamination + // from previous exports with the same ID. $export->deleteFileDirectory(); $export->file_name = $action->getFileName($export) ?? $exporter->getFileName($export); @@ -301,8 +302,9 @@ trait CanExportRecords $jobConnection = $exporter->getJobConnection(); $jobBatchName = $exporter->getJobBatchName(); - // We do not want to send the loaded user relationship to the queue in job payloads, - // in case it contains attributes that are not serializable, such as binary columns. + // We do not want to send the loaded user relationship to the + // queue in job payloads, in case it contains attributes that + // are not serializable, such as binary columns. $export->unsetRelation('user'); $makeCreateXlsxFileJob = fn (): CreateXlsxFile => app(CreateXlsxFile::class, [ @@ -549,6 +551,9 @@ trait CanExportRecords public function modifyQueryUsing(?Closure $callback): static { + // Security: Exports do not check per-record policies. Use this + // to scope the query to records the user is authorized to see. + $this->modifyQueryUsing = $callback; return $this; diff --git a/packages/actions/src/Concerns/CanOpenUrl.php b/packages/actions/src/Concerns/CanOpenUrl.php index 32cd34f37e..c5b47ac1be 100644 --- a/packages/actions/src/Concerns/CanOpenUrl.php +++ b/packages/actions/src/Concerns/CanOpenUrl.php @@ -21,6 +21,10 @@ trait CanOpenUrl public function url(string | Closure | null $url, bool | Closure | null $shouldOpenInNewTab = null): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` or `data:` protocol URLs + // rendered in `href` attributes. + if ($shouldOpenInNewTab !== null) { $this->openUrlInNewTab($shouldOpenInNewTab); } diff --git a/packages/actions/src/Concerns/HasExtraModalWindowAttributes.php b/packages/actions/src/Concerns/HasExtraModalWindowAttributes.php index 204182e030..db30cdb4c6 100644 --- a/packages/actions/src/Concerns/HasExtraModalWindowAttributes.php +++ b/packages/actions/src/Concerns/HasExtraModalWindowAttributes.php @@ -17,6 +17,9 @@ trait HasExtraModalWindowAttributes */ public function extraModalWindowAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraModalWindowAttributes[] = $attributes; } else { diff --git a/packages/actions/src/Exports/Concerns/CanFormatState.php b/packages/actions/src/Exports/Concerns/CanFormatState.php index 8aa3bb6252..75d3a9a90f 100644 --- a/packages/actions/src/Exports/Concerns/CanFormatState.php +++ b/packages/actions/src/Exports/Concerns/CanFormatState.php @@ -63,6 +63,10 @@ trait CanFormatState public function formatState(mixed $state): mixed { + // Security: Export values are written to CSV/XLSX as-is after + // formatting. Use `formatStateUsing()` to sanitize values that + // may trigger formula injection (`=`, `+`, `-`, `@`). + $state = $this->evaluate($this->formatStateUsing ?? $state, [ 'state' => $state, ]); diff --git a/packages/actions/src/Exports/ExportColumn.php b/packages/actions/src/Exports/ExportColumn.php index 88ed6decfb..3ab3979ec5 100644 --- a/packages/actions/src/Exports/ExportColumn.php +++ b/packages/actions/src/Exports/ExportColumn.php @@ -13,6 +13,12 @@ use InvalidArgumentException; class ExportColumn extends Component { + // Security: Export column values are written to CSV/XLSX without + // transformation. Values starting with `=`, `+`, `-`, or `@` + // may be interpreted as formulas by spreadsheet software. + // Use `formatStateUsing()` to sanitize untrusted user + // content, e.g. by prefixing with a single quote. + use CanAggregateRelatedModels; use Concerns\CanFormatState; use HasCellState; diff --git a/packages/actions/src/Exports/Exporter.php b/packages/actions/src/Exports/Exporter.php index d0e6d66f41..8afc3aa44b 100644 --- a/packages/actions/src/Exports/Exporter.php +++ b/packages/actions/src/Exports/Exporter.php @@ -19,6 +19,15 @@ use OpenSpout\Writer\XLSX\Writer; abstract class Exporter { + // Security: Exports do not perform per-record authorization checks. + // All records matching the query are included without consulting + // Laravel policies. Use `modifyQueryUsing()` on the export action + // to scope the query. Data is written to CSV/XLSX as-is — values + // starting with `=`, `+`, `-`, or `@` may be interpreted as + // formulas by spreadsheet software (CSV formula injection). + // Sanitize via `formatStateUsing()` if exporting + // untrusted user content. + /** @var array */ protected array $cachedColumns; diff --git a/packages/actions/src/Imports/Importer.php b/packages/actions/src/Imports/Importer.php index e5cbc7b6ca..0797bc2782 100644 --- a/packages/actions/src/Imports/Importer.php +++ b/packages/actions/src/Imports/Importer.php @@ -14,6 +14,13 @@ use Illuminate\Validation\ValidationException; abstract class Importer { + // Security: Imports do not perform per-record authorization checks. + // Each CSV row is processed by `resolveRecord()`, `fillRecord()`, + // and `saveRecord()` without consulting Laravel policies. Add + // manual checks in lifecycle hooks (`beforeCreate()`, etc.) + // if needed. Failure CSVs contain original data unchanged — + // formula injection risk applies to those files too. + /** @var array */ protected array $cachedColumns; @@ -145,6 +152,9 @@ abstract class Importer public function resolveRecord(): ?Model { + // Security: This method runs without policy checks. + // Override to add authorization logic if needed. + $keyName = app(static::getModel())->getKeyName(); $keyColumnName = $this->columnMap[$keyName] ?? $keyName; diff --git a/packages/forms/src/Components/BaseFileUpload.php b/packages/forms/src/Components/BaseFileUpload.php index 3f14546d7b..12b5ecef11 100644 --- a/packages/forms/src/Components/BaseFileUpload.php +++ b/packages/forms/src/Components/BaseFileUpload.php @@ -329,6 +329,12 @@ class BaseFileUpload extends Field implements Contracts\HasNestedRecursiveValida public function preserveFilenames(bool | Closure $condition = true): static { + // Security: Preserving user-provided filenames on local or public + // disks can allow PHP file execution (e.g. uploading `.php` + // files). `acceptedFileTypes()` validates MIME type but not + // extension. Use S3 or keep the default random filenames. + // Only use this with trusted users. + $this->shouldPreserveFilenames = $condition; return $this; @@ -431,6 +437,10 @@ class BaseFileUpload extends Field implements Contracts\HasNestedRecursiveValida public function visibility(string | Closure | null $visibility): static { + // Security: Default visibility is `private` (except on the `public` + // disk). Always use `acceptedFileTypes()` and `maxSize()` for + // server-side validation regardless of visibility setting. + $this->visibility = $visibility; return $this; @@ -916,6 +926,10 @@ class BaseFileUpload extends Field implements Contracts\HasNestedRecursiveValida public function getUploadedFileNameForStorageUsing(?Closure $callback): static { + // Security: Custom storage filenames carry the same risk as + // `preserveFilenames()` — user-controlled names on local + // or public disks can enable PHP execution. + $this->getUploadedFileNameForStorageUsing = $callback; return $this; diff --git a/packages/forms/src/Components/Concerns/CanAllowHtml.php b/packages/forms/src/Components/Concerns/CanAllowHtml.php index 04c3855b73..5d6072577b 100644 --- a/packages/forms/src/Components/Concerns/CanAllowHtml.php +++ b/packages/forms/src/Components/Concerns/CanAllowHtml.php @@ -6,6 +6,10 @@ use Closure; trait CanAllowHtml { + // Security: Enabling HTML rendering on form components means the content + // will not be escaped. Only enable for trusted content you control — + // never for raw user input without proper sanitization. + protected bool | Closure $isHtmlAllowed = false; public function allowHtml(bool | Closure $condition = true): static diff --git a/packages/forms/src/Components/Concerns/CanBeValidated.php b/packages/forms/src/Components/Concerns/CanBeValidated.php index 9622166878..dcb6525f99 100644 --- a/packages/forms/src/Components/Concerns/CanBeValidated.php +++ b/packages/forms/src/Components/Concerns/CanBeValidated.php @@ -738,6 +738,10 @@ trait CanBeValidated public function allowHtmlValidationMessages(bool | Closure $condition = true): static { + // Security: Enabling HTML in validation messages means they are + // rendered with `{!! !!}`. If messages include user input + // (e.g. via custom rules or placeholders), XSS is possible. + $this->areHtmlValidationMessagesAllowed = $condition; return $this; @@ -1036,8 +1040,9 @@ trait CanBeValidated $state = parent::mutateStateForValidation($state); } - // Laravel's `in` validation rule expects state values to be scalar, so we need to convert any - // enum objects to their backed values before they are passed to the Laravel validator. + // Laravel's `in` validation rule expects state values to be + // scalar, so we need to convert any enum objects to their + // backed values before passing to the validator. if (is_array($state)) { foreach ($state as $key => $value) { if ($value instanceof BackedEnum) { diff --git a/packages/forms/src/Components/Concerns/HasExtraFieldWrapperAttributes.php b/packages/forms/src/Components/Concerns/HasExtraFieldWrapperAttributes.php index f8b4ca3fff..5f709ef97b 100644 --- a/packages/forms/src/Components/Concerns/HasExtraFieldWrapperAttributes.php +++ b/packages/forms/src/Components/Concerns/HasExtraFieldWrapperAttributes.php @@ -17,6 +17,9 @@ trait HasExtraFieldWrapperAttributes */ public function extraFieldWrapperAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraFieldWrapperAttributes[] = $attributes; } else { diff --git a/packages/forms/src/Components/Concerns/HasExtraInputAttributes.php b/packages/forms/src/Components/Concerns/HasExtraInputAttributes.php index c393d763d5..84210a6bfa 100644 --- a/packages/forms/src/Components/Concerns/HasExtraInputAttributes.php +++ b/packages/forms/src/Components/Concerns/HasExtraInputAttributes.php @@ -17,6 +17,9 @@ trait HasExtraInputAttributes */ public function extraInputAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraInputAttributes[] = $attributes; } else { diff --git a/packages/forms/src/Components/DateTimePicker.php b/packages/forms/src/Components/DateTimePicker.php index 9889cbef8d..6f13d96e59 100644 --- a/packages/forms/src/Components/DateTimePicker.php +++ b/packages/forms/src/Components/DateTimePicker.php @@ -136,6 +136,9 @@ class DateTimePicker extends Field implements HasAffixActions */ public function extraTriggerAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraAttributes[] = $attributes; } else { diff --git a/packages/forms/src/Components/MarkdownEditor.php b/packages/forms/src/Components/MarkdownEditor.php index 55ecbe548e..15a486e746 100644 --- a/packages/forms/src/Components/MarkdownEditor.php +++ b/packages/forms/src/Components/MarkdownEditor.php @@ -9,6 +9,11 @@ use LogicException; class MarkdownEditor extends Field implements Contracts\CanBeLengthConstrained { + // Security: Like the rich editor, the markdown editor sends raw content + // to the backend. When rendering in Blade views, always sanitize with + // `sanitizeHtml()` and `markdown()` together. Never use `{!! !!}` + // with unsanitized content. + use CanConfigureCommonMark; use Concerns\CanBeLengthConstrained; use Concerns\HasFileAttachments; diff --git a/packages/forms/src/Components/ModalTableSelect.php b/packages/forms/src/Components/ModalTableSelect.php index f8678905e7..fc4f4555a8 100644 --- a/packages/forms/src/Components/ModalTableSelect.php +++ b/packages/forms/src/Components/ModalTableSelect.php @@ -326,8 +326,8 @@ class ModalTableSelect extends Field $relatedRecords = $relationship->getResults(); $component->state( - // Cast the related keys to a string, otherwise JavaScript does not - // know how to handle deselection. + // Cast the related keys to a string, otherwise + // JavaScript can't handle deselection. // // https://github.com/filamentphp/filament/issues/1111 $relatedRecords @@ -357,8 +357,8 @@ class ModalTableSelect extends Field $relatedRecords = $relationship->getResults(); $component->state( - // Cast the related keys to a string, otherwise JavaScript does not - // know how to handle deselection. + // Cast the related keys to a string, otherwise + // JavaScript can't handle deselection. // // https://github.com/filamentphp/filament/issues/1111 $relatedRecords @@ -593,9 +593,9 @@ class ModalTableSelect extends Field } if (! $relationship instanceof BelongsToMany) { - // If the model is new and the foreign key is already filled, we don't need to fill it again. - // This could be a security issue if the foreign key was mutated in some way before it - // was saved, and we don't want to overwrite that value. + // Security: If the model is new and the foreign key is already + // filled, don't overwrite it — the key may have been set by + // authorization logic or event listeners before save. if ( $record->wasRecentlyCreated && filled($record->getAttributeValue($relationship->getForeignKeyName())) diff --git a/packages/forms/src/Components/RichEditor.php b/packages/forms/src/Components/RichEditor.php index 75dd7e70a3..454eb97d7d 100644 --- a/packages/forms/src/Components/RichEditor.php +++ b/packages/forms/src/Components/RichEditor.php @@ -38,6 +38,13 @@ use Tiptap\Editor; class RichEditor extends Field implements Contracts\CanBeLengthConstrained { + // Security: The rich editor outputs raw HTML. Attackers can intercept + // the value and send arbitrary HTML to the backend. When rendering + // in Blade views, always sanitize using `sanitizeHtml()` or the + // `RichContentRenderer`. Never use `{!! $content !!}` unsanitized. + // The default sanitizer permits inline `style` attributes — + // configure a restrictive one for untrusted user content. + use Concerns\CanBeLengthConstrained; use Concerns\HasExtraInputAttributes; use Concerns\HasFileAttachments; @@ -729,9 +736,10 @@ class RichEditor extends Field implements Contracts\CanBeLengthConstrained public function getContentAttribute(): ?RichContentAttribute { - // Do not read content attributes from the model when the rich editor is nested - // inside a custom block action modal, since the content attribute should only - // be used to configure the parent rich editor. + // Do not read content attributes from the model when the + // rich editor is nested inside a custom block action + // modal — the content attribute should only be used + // to configure the parent rich editor. if ($this->getRootContainer()->getOperation() === CustomBlockAction::NAME) { return null; } diff --git a/packages/forms/src/Components/RichEditor/MentionProvider.php b/packages/forms/src/Components/RichEditor/MentionProvider.php index d79e9b42d0..fadd16a063 100644 --- a/packages/forms/src/Components/RichEditor/MentionProvider.php +++ b/packages/forms/src/Components/RichEditor/MentionProvider.php @@ -72,6 +72,10 @@ class MentionProvider */ public function url(?Closure $callback): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` protocol URLs rendered + // in `href` attributes. + $this->getUrlUsing = $callback; return $this; @@ -82,6 +86,9 @@ class MentionProvider */ public function extraAttributes(array | Closure $attributes): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + $this->extraAttributes = $attributes; return $this; diff --git a/packages/forms/src/Components/RichEditor/RichContentRenderer.php b/packages/forms/src/Components/RichEditor/RichContentRenderer.php index f70c7be0c6..66470e7dc6 100644 --- a/packages/forms/src/Components/RichEditor/RichContentRenderer.php +++ b/packages/forms/src/Components/RichEditor/RichContentRenderer.php @@ -485,6 +485,9 @@ class RichContentRenderer implements Htmlable public function toUnsafeHtml(): string { + // Security: This method returns unsanitized HTML. Only use for + // internal processing — never render in Blade. Use `toHtml()`. + $editor = $this->getEditor(); $this->processCustomBlocks($editor); @@ -498,6 +501,10 @@ class RichContentRenderer implements Htmlable public function toHtml(): string { + // Security: Always use `toHtml()` (not `toUnsafeHtml()`) when + // rendering user-provided rich content. This applies + // Symfony's `HtmlSanitizer` to prevent XSS. + return Str::sanitizeHtml($this->toUnsafeHtml()); } diff --git a/packages/forms/src/Components/Select.php b/packages/forms/src/Components/Select.php index f61ae6e055..af0f60c9ce 100644 --- a/packages/forms/src/Components/Select.php +++ b/packages/forms/src/Components/Select.php @@ -1043,8 +1043,8 @@ class Select extends Field implements Contracts\CanDisableOptions, Contracts\Has /** @var Collection $relatedRecords */ $relatedRecords = $relationship->getResults(); - // Cast the related keys to a string, otherwise JavaScript does not - // know how to handle deselection. + // Cast the related keys to a string, otherwise + // JavaScript can't handle deselection. // // https://github.com/filamentphp/filament/issues/1111 $relatedKeys = $relatedRecords @@ -1309,9 +1309,9 @@ class Select extends Field implements Contracts\CanDisableOptions, Contracts\Has } if (! $relationship instanceof BelongsToMany) { - // If the model is new and the foreign key is already filled, we don't need to fill it again. - // This could be a security issue if the foreign key was mutated in some way before it - // was saved, and we don't want to overwrite that value. + // Security: If the model is new and the foreign key is already + // filled, don't overwrite it — the key may have been set by + // authorization logic or event listeners before save. if ( $record->wasRecentlyCreated && filled($record->getAttributeValue($relationship->getForeignKeyName())) diff --git a/packages/forms/src/Components/TableSelect.php b/packages/forms/src/Components/TableSelect.php index 366664f478..be1c6663f9 100644 --- a/packages/forms/src/Components/TableSelect.php +++ b/packages/forms/src/Components/TableSelect.php @@ -165,8 +165,8 @@ class TableSelect extends Field $relatedRecords = $relationship->getResults(); $component->state( - // Cast the related keys to a string, otherwise JavaScript does not - // know how to handle deselection. + // Cast the related keys to a string, otherwise + // JavaScript can't handle deselection. // // https://github.com/filamentphp/filament/issues/1111 $relatedRecords @@ -260,9 +260,9 @@ class TableSelect extends Field } if (! $relationship instanceof BelongsToMany) { - // If the model is new and the foreign key is already filled, we don't need to fill it again. - // This could be a security issue if the foreign key was mutated in some way before it - // was saved, and we don't want to overwrite that value. + // Security: If the model is new and the foreign key is already + // filled, don't overwrite it — the key may have been set by + // authorization logic or event listeners before save. if ( $record->wasRecentlyCreated && filled($record->getAttributeValue($relationship->getForeignKeyName())) diff --git a/packages/infolists/src/Components/Concerns/CanFormatState.php b/packages/infolists/src/Components/Concerns/CanFormatState.php index 610d4abc6b..2ecf1be4cb 100644 --- a/packages/infolists/src/Components/Concerns/CanFormatState.php +++ b/packages/infolists/src/Components/Concerns/CanFormatState.php @@ -52,6 +52,9 @@ trait CanFormatState public function markdown(bool | Closure $condition = true): static { + // Security: Markdown is converted to HTML and then sanitized via + // `Str::sanitizeHtml()`. Same inline `style` caveat as `html()`. + $this->isMarkdown = $condition; return $this; @@ -348,6 +351,12 @@ trait CanFormatState public function html(bool | Closure $condition = true): static { + // Security: Content is automatically sanitized via Symfony's + // `HtmlSanitizer`. The default config permits inline `style` + // attributes, which can enable CSS-based attacks (e.g. + // `background: url(...)`). Configure a custom sanitizer + // if rendering untrusted user content. + $this->isHtml = $condition; return $this; diff --git a/packages/infolists/src/Components/Concerns/HasExtraEntryWrapperAttributes.php b/packages/infolists/src/Components/Concerns/HasExtraEntryWrapperAttributes.php index 548f783d70..862b27b9c2 100644 --- a/packages/infolists/src/Components/Concerns/HasExtraEntryWrapperAttributes.php +++ b/packages/infolists/src/Components/Concerns/HasExtraEntryWrapperAttributes.php @@ -17,6 +17,9 @@ trait HasExtraEntryWrapperAttributes */ public function extraEntryWrapperAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraEntryWrapperAttributes[] = $attributes; } else { diff --git a/packages/infolists/src/Components/ImageEntry.php b/packages/infolists/src/Components/ImageEntry.php index 64291998d9..c34ad04f61 100644 --- a/packages/infolists/src/Components/ImageEntry.php +++ b/packages/infolists/src/Components/ImageEntry.php @@ -288,6 +288,9 @@ class ImageEntry extends Entry implements HasEmbeddedView */ public function extraImgAttributes(array | Closure $attributes): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + $this->extraImgAttributes = $attributes; return $this; diff --git a/packages/notifications/src/Notification.php b/packages/notifications/src/Notification.php index 19c1637aa5..0297dea445 100644 --- a/packages/notifications/src/Notification.php +++ b/packages/notifications/src/Notification.php @@ -138,6 +138,9 @@ class Notification extends ViewComponent implements Arrayable, HasEmbeddedView protected function isViewSafe(string $view): bool { + // Security: Only explicitly whitelisted views can be rendered in + // notifications, preventing view injection from stored data. + return in_array($view, $this->safeViews, strict: true); } diff --git a/packages/panels/src/Http/Middleware/Authenticate.php b/packages/panels/src/Http/Middleware/Authenticate.php index 24dc4ceab8..95f8a6a679 100644 --- a/packages/panels/src/Http/Middleware/Authenticate.php +++ b/packages/panels/src/Http/Middleware/Authenticate.php @@ -29,6 +29,9 @@ class Authenticate extends Middleware $panel = Filament::getCurrentOrDefaultPanel(); + // Security: If the user model does not implement `FilamentUser`, + // access is only allowed in local environments. In production, + // implement `FilamentUser` with `canAccessPanel()`. abort_if( $user instanceof FilamentUser ? (! $user->canAccessPanel($panel)) : diff --git a/packages/panels/src/Http/Middleware/IdentifyTenant.php b/packages/panels/src/Http/Middleware/IdentifyTenant.php index c4f20ea9ea..95d0566abf 100644 --- a/packages/panels/src/Http/Middleware/IdentifyTenant.php +++ b/packages/panels/src/Http/Middleware/IdentifyTenant.php @@ -12,6 +12,12 @@ class IdentifyTenant { public function handle(Request $request, Closure $next): mixed { + // Security: Tenant identification occurs in this middleware. Global + // scopes for tenant isolation are only active AFTER this runs. + // Queries in earlier middleware or service providers will not + // be tenant-scoped. Ensure tenant-aware middleware uses + // `isPersistent: true` for Livewire AJAX enforcement. + $panel = Filament::getCurrentOrDefaultPanel(); if (! $panel->hasTenancy()) { diff --git a/packages/panels/src/Models/Contracts/FilamentUser.php b/packages/panels/src/Models/Contracts/FilamentUser.php index 84a7c398c5..0989f65f08 100644 --- a/packages/panels/src/Models/Contracts/FilamentUser.php +++ b/packages/panels/src/Models/Contracts/FilamentUser.php @@ -6,5 +6,10 @@ use Filament\Panel; interface FilamentUser { + // Security: You must implement this interface on your User model in + // production. Without it, all authenticated users can access your + // panel when `APP_ENV` is not `local`. For multi-panel apps, + // check `$panel->getId()` to restrict access per panel. + public function canAccessPanel(Panel $panel): bool; } diff --git a/packages/panels/src/Navigation/Concerns/HasExtraSidebarAttributes.php b/packages/panels/src/Navigation/Concerns/HasExtraSidebarAttributes.php index 4db48a98be..4f01d3b5c9 100644 --- a/packages/panels/src/Navigation/Concerns/HasExtraSidebarAttributes.php +++ b/packages/panels/src/Navigation/Concerns/HasExtraSidebarAttributes.php @@ -17,6 +17,9 @@ trait HasExtraSidebarAttributes */ public function extraSidebarAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraSidebarAttributes[] = $attributes; } else { diff --git a/packages/panels/src/Navigation/Concerns/HasExtraTopbarAttributes.php b/packages/panels/src/Navigation/Concerns/HasExtraTopbarAttributes.php index 85d4d3ccfc..288bc841ae 100644 --- a/packages/panels/src/Navigation/Concerns/HasExtraTopbarAttributes.php +++ b/packages/panels/src/Navigation/Concerns/HasExtraTopbarAttributes.php @@ -17,6 +17,9 @@ trait HasExtraTopbarAttributes */ public function extraTopbarAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraTopbarAttributes[] = $attributes; } else { diff --git a/packages/panels/src/Navigation/MenuItem.php b/packages/panels/src/Navigation/MenuItem.php index 4aa8dfe7a9..65b04265c9 100644 --- a/packages/panels/src/Navigation/MenuItem.php +++ b/packages/panels/src/Navigation/MenuItem.php @@ -80,6 +80,10 @@ class MenuItem extends Component public function url(string | Closure | null $url, bool | Closure | null $shouldOpenInNewTab = null): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` protocol URLs rendered + // in `href` attributes. + if ($shouldOpenInNewTab !== null) { $this->openUrlInNewTab($shouldOpenInNewTab); } diff --git a/packages/panels/src/Navigation/NavigationItem.php b/packages/panels/src/Navigation/NavigationItem.php index 372cde2143..289c49a709 100644 --- a/packages/panels/src/Navigation/NavigationItem.php +++ b/packages/panels/src/Navigation/NavigationItem.php @@ -149,6 +149,10 @@ class NavigationItem extends Component public function url(string | Closure | null $url, bool | Closure | null $shouldOpenInNewTab = null): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` protocol URLs rendered + // in `href` attributes. + $this->url = $url; if ($shouldOpenInNewTab !== null) { diff --git a/packages/panels/src/Pages/Concerns/CanAuthorizeAccess.php b/packages/panels/src/Pages/Concerns/CanAuthorizeAccess.php index 4358763e52..06e94016d6 100644 --- a/packages/panels/src/Pages/Concerns/CanAuthorizeAccess.php +++ b/packages/panels/src/Pages/Concerns/CanAuthorizeAccess.php @@ -11,6 +11,10 @@ trait CanAuthorizeAccess public static function canAccess(): bool { + // Security: Custom pages default to allowing access for all + // authenticated panel users. Override this method to restrict + // access based on roles, permissions, or other logic. + return true; } } diff --git a/packages/panels/src/Resources/Pages/EditRecord.php b/packages/panels/src/Resources/Pages/EditRecord.php index a79dc4deb9..56efd7ee6d 100644 --- a/packages/panels/src/Resources/Pages/EditRecord.php +++ b/packages/panels/src/Resources/Pages/EditRecord.php @@ -142,6 +142,12 @@ class EditRecord extends Page */ protected function mutateFormDataBeforeFill(array $data): array { + // Security: All non-`$hidden` model attributes are sent to the + // browser via Livewire. Override this to `unset()` sensitive + // attributes (API keys, internal flags, etc.). Only form + // field attributes are writable — not a mass assignment + // issue, but a data exposure concern. + return $data; } diff --git a/packages/panels/src/Resources/Pages/ViewRecord.php b/packages/panels/src/Resources/Pages/ViewRecord.php index 6591afb772..3bc1eb4bfe 100644 --- a/packages/panels/src/Resources/Pages/ViewRecord.php +++ b/packages/panels/src/Resources/Pages/ViewRecord.php @@ -127,6 +127,11 @@ class ViewRecord extends Page */ protected function mutateFormDataBeforeFill(array $data): array { + // Security: All non-`$hidden` model attributes are sent to the + // browser via Livewire. Override this to `unset()` sensitive + // attributes (API keys, etc.) that should not be exposed + // to client-side JavaScript. + return $data; } diff --git a/packages/panels/src/Resources/RelationManagers/RelationManager.php b/packages/panels/src/Resources/RelationManagers/RelationManager.php index 187d2536f3..826e93a1d9 100644 --- a/packages/panels/src/Resources/RelationManagers/RelationManager.php +++ b/packages/panels/src/Resources/RelationManagers/RelationManager.php @@ -331,6 +331,12 @@ class RelationManager extends Component implements HasActions, HasRenderHookScop public function getDefaultActionAuthorizationResponse(Action $action): ?Response { + // Security: `AssociateAction`, `AttachAction`, `DetachAction`, and + // `DissociateAction` only check `isReadOnly()` — they do not check + // specific policy methods. `DeleteBulkAction`, `ForceDeleteBulkAction`, + // and `RestoreBulkAction` use `*Any()` policy methods for performance. + // Use `authorizeIndividualRecords()` if per-record checks are needed. + if ($action instanceof ViewAction) { return $this->getViewAuthorizationResponse($action->getRecord()); } diff --git a/packages/panels/src/Resources/Resource.php b/packages/panels/src/Resources/Resource.php index 2c2cbf7025..550e4cc483 100644 --- a/packages/panels/src/Resources/Resource.php +++ b/packages/panels/src/Resources/Resource.php @@ -75,6 +75,11 @@ abstract class Resource */ public static function getEloquentQuery(): Builder { + // Security: Override this method to scope queries to the current + // user's permissions. By default all records are returned + // (subject to tenant scoping if active). Failing to scope + // in multi-user apps can expose unauthorized records. + $query = static::getModel()::query(); if (! static::isScopedToTenant()) { diff --git a/packages/panels/src/Resources/Resource/Concerns/BelongsToTenant.php b/packages/panels/src/Resources/Resource/Concerns/BelongsToTenant.php index 1daec7f2ed..bb2ead01c9 100644 --- a/packages/panels/src/Resources/Resource/Concerns/BelongsToTenant.php +++ b/packages/panels/src/Resources/Resource/Concerns/BelongsToTenant.php @@ -18,6 +18,15 @@ use Znck\Eloquent\Relations\BelongsToThrough; */ trait BelongsToTenant { + // Security: Tenant query scoping is applied via global scopes registered + // after tenant identification in middleware. Queries before identification + // (early middleware, service providers) will NOT be scoped. Custom queries + // outside the panel must be manually scoped. Laravel's `unique()` / + // `exists()` validation rules bypass global scopes — use + // `scopedUnique()` / `scopedExists()` instead. Filament does + // not guarantee multi-tenant security; it is your + // responsibility to implement correctly. + protected static bool $isScopedToTenant = true; protected static ?string $tenantOwnershipRelationshipName = null; @@ -62,6 +71,10 @@ trait BelongsToTenant public static function scopeToTenant(bool $condition = true): void { + // Security: Disabling tenant scoping means this resource's queries + // will not be filtered by tenant. All tenants' data will be + // accessible. Only disable for shared / cross-tenant resources. + static::$isScopedToTenant = $condition; } diff --git a/packages/panels/src/Resources/Resource/Concerns/HasAuthorization.php b/packages/panels/src/Resources/Resource/Concerns/HasAuthorization.php index fc0c569f6c..67d96d5687 100644 --- a/packages/panels/src/Resources/Resource/Concerns/HasAuthorization.php +++ b/packages/panels/src/Resources/Resource/Concerns/HasAuthorization.php @@ -10,6 +10,16 @@ use function Filament\get_authorization_response; trait HasAuthorization { + // Security: Resource authorization delegates to Laravel Model Policies. + // Standard CRUD operations (`viewAny`, `create`, `update`, `view`, + // `delete`, `forceDelete`, `restore`, `reorder`) are checked + // automatically. Bulk actions use `*Any()` policy methods + // (`deleteAny`, `forceDeleteAny`, `restoreAny`) for performance — + // use `authorizeIndividualRecords()` if per-record checks are + // needed. Inline editable table columns bypass these checks — + // they only respect `disabled()`. Custom actions require manual + // authorization via `authorize()`, `visible()`, or `hidden()`. + protected static bool $shouldCheckPolicyExistence = true; protected static bool $shouldSkipAuthorization = false; @@ -48,6 +58,10 @@ trait HasAuthorization public static function skipAuthorization(bool $condition = true): void { + // Security: Disabling authorization removes all policy checks for + // this resource. All panel users will be able to perform any + // operation. Not recommended for production. + static::$shouldSkipAuthorization = $condition; } diff --git a/packages/panels/src/Resources/Resource/Concerns/HasNavigation.php b/packages/panels/src/Resources/Resource/Concerns/HasNavigation.php index 07fec2b22d..9fede49520 100644 --- a/packages/panels/src/Resources/Resource/Concerns/HasNavigation.php +++ b/packages/panels/src/Resources/Resource/Concerns/HasNavigation.php @@ -181,6 +181,10 @@ trait HasNavigation public static function shouldRegisterNavigation(): bool { + // Security: Hiding a resource from navigation does NOT prevent + // direct URL access. Use resource authorization (Model + // Policies) to control who can access pages. + return static::$shouldRegisterNavigation; } diff --git a/packages/schemas/src/Components/Concerns/CanBeDisabled.php b/packages/schemas/src/Components/Concerns/CanBeDisabled.php index 7b777f895b..51e9863936 100644 --- a/packages/schemas/src/Components/Concerns/CanBeDisabled.php +++ b/packages/schemas/src/Components/Concerns/CanBeDisabled.php @@ -15,6 +15,12 @@ trait CanBeDisabled public function disabled(bool | Closure $condition = true): static { + // Security: Disabling a field prevents it from being saved, but + // skilled users can manipulate Livewire's JavaScript to bypass + // the disabled state on the client. Always enforce authorization + // on the backend (e.g. in `mutateFormDataBeforeSave()` or + // via Model Policies) for sensitive fields. + $this->isDisabled = $condition; $this->saved(fn (Component $component): bool => ! $component->evaluate($condition)); diff --git a/packages/schemas/src/Components/Concerns/CanBeHidden.php b/packages/schemas/src/Components/Concerns/CanBeHidden.php index d00929964d..495dda5ea5 100644 --- a/packages/schemas/src/Components/Concerns/CanBeHidden.php +++ b/packages/schemas/src/Components/Concerns/CanBeHidden.php @@ -192,6 +192,9 @@ trait CanBeHidden public function visibleJs(string | Closure | null $condition): static { + // Security: This JavaScript is evaluated on the client via `eval()`. + // Never pass user input — only developer-defined expressions. + $this->visibleJs = $condition; return $this; @@ -199,6 +202,9 @@ trait CanBeHidden public function hiddenJs(string | Closure | null $condition): static { + // Security: This JavaScript is evaluated on the client via `eval()`. + // Never pass user input — only developer-defined expressions. + $this->hiddenJs = $condition; return $this; diff --git a/packages/schemas/src/Components/Concerns/CanOpenUrl.php b/packages/schemas/src/Components/Concerns/CanOpenUrl.php index 258f73f304..323db01b08 100644 --- a/packages/schemas/src/Components/Concerns/CanOpenUrl.php +++ b/packages/schemas/src/Components/Concerns/CanOpenUrl.php @@ -19,6 +19,10 @@ trait CanOpenUrl public function url(string | Closure | null $url, bool | Closure | null $shouldOpenInNewTab = null): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` protocol URLs rendered + // in `href` attributes. + if ($shouldOpenInNewTab !== null) { $this->openUrlInNewTab($shouldOpenInNewTab); } diff --git a/packages/schemas/src/Components/Concerns/HasState.php b/packages/schemas/src/Components/Concerns/HasState.php index 0c85546380..03733dffe1 100644 --- a/packages/schemas/src/Components/Concerns/HasState.php +++ b/packages/schemas/src/Components/Concerns/HasState.php @@ -138,6 +138,9 @@ trait HasState public function afterStateUpdatedJs(string | Closure | null $js): static { + // Security: This JavaScript is evaluated on the client via `eval()`. + // Never pass user input — only developer-defined expressions. + if (blank($js)) { $this->afterStateUpdatedJs = []; @@ -481,9 +484,10 @@ trait HasState $isStatePathMatching = in_array($statePathToCheck, $statePaths); - // Even if the current component's state path is not present in the array of state paths to hydrate, - // a parent state path may be present. In this case, we still need to hydrate the field as it is - // nested inside the parent state that was hydrated. + // Even if the current component's state path is not in the + // array of state paths to hydrate, a parent path may be. + // In that case we still need to hydrate the field since + // it is nested inside the parent state. while ((! $isStatePathMatching) && str($statePathToCheck)->contains('.')) { $statePathToCheck = (string) str($statePathToCheck)->beforeLast('.'); @@ -639,9 +643,11 @@ trait HasState data_set($livewire, $this->getStatePath(), $this->evaluate($state)); - // For components such as repeaters and builders, the default child schemas depend on the state of the component. - // When loading state into these fields after the state is already present, the cached child schemas need to be - // cleared so that they can be re-evaluated based on the new state. `rawState()` is called during this process. + // For components like repeaters and builders, child schemas + // depend on the component's state. When loading state after + // it is already present, cached child schemas must be + // cleared so they can be re-evaluated. `rawState()` + // is called during this process. $this->clearCachedDefaultChildSchemas(); return $this; diff --git a/packages/schemas/src/Components/Image.php b/packages/schemas/src/Components/Image.php index b225d06a7b..c30fe94a0a 100644 --- a/packages/schemas/src/Components/Image.php +++ b/packages/schemas/src/Components/Image.php @@ -37,6 +37,10 @@ class Image extends Component public function url(string | Closure $url): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` protocol URLs rendered + // in `src` attributes. + $this->url = $url; return $this; diff --git a/packages/schemas/src/Components/Tabs/Tab.php b/packages/schemas/src/Components/Tabs/Tab.php index 7a145b42da..a3ae74c6f4 100644 --- a/packages/schemas/src/Components/Tabs/Tab.php +++ b/packages/schemas/src/Components/Tabs/Tab.php @@ -147,6 +147,11 @@ class Tab extends Component implements CanConcealComponents public function excludeQueryWhenResolvingRecord(bool | Closure $condition = true): static { + // Security: Do NOT use this on tabs that enforce authorization + // scopes (e.g. restricting records by tenant or user ownership). + // Excluding the query allows direct URL access to records + // that the tab's scope would otherwise prevent. + $this->shouldExcludeQueryWhenResolvingRecord = $condition; return $this; diff --git a/packages/schemas/src/Concerns/InteractsWithSchemas.php b/packages/schemas/src/Concerns/InteractsWithSchemas.php index 75f525bef2..1ebb50980f 100644 --- a/packages/schemas/src/Concerns/InteractsWithSchemas.php +++ b/packages/schemas/src/Concerns/InteractsWithSchemas.php @@ -62,6 +62,10 @@ trait InteractsWithSchemas */ public function callSchemaComponentMethod(string $componentKey, string $method, array $arguments = []): mixed { + // Security: This method is callable from the frontend and dispatches + // to `#[ExposedLivewireMethod]` methods on schema components. + // Only methods marked with that attribute are allowed. + $component = $this->getSchemaComponent($componentKey); if (! $component) { @@ -203,7 +207,8 @@ trait InteractsWithSchemas return $this->cachedSchemas[$name] = $schema->key($name); } - // If null was explicitly passed as the schema, unset the cached schema. + // If null was explicitly passed as the schema, + // unset the cached schema. if (func_num_args() === 2) { unset($this->cachedSchemas[$name]); diff --git a/packages/schemas/src/JsContent.php b/packages/schemas/src/JsContent.php index 5dd42194d4..8616c1a3b1 100644 --- a/packages/schemas/src/JsContent.php +++ b/packages/schemas/src/JsContent.php @@ -7,6 +7,10 @@ use Illuminate\Support\Js; class JsContent implements Htmlable { + // Security: This class evaluates its content as JavaScript via + // `eval()` in the browser. Only use with developer-defined + // expressions — never with user input. + protected string $content; public function __construct(string $content) diff --git a/packages/support/src/Assets/Js.php b/packages/support/src/Assets/Js.php index adf6dde191..5d98a1a15a 100644 --- a/packages/support/src/Assets/Js.php +++ b/packages/support/src/Assets/Js.php @@ -97,6 +97,9 @@ class Js extends Asset */ public function extraAttributes(array $attributes): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + $this->extraAttributes = $attributes; return $this; diff --git a/packages/support/src/Concerns/HasExtraAlpineAttributes.php b/packages/support/src/Concerns/HasExtraAlpineAttributes.php index fe23f8440a..ed983e4a95 100644 --- a/packages/support/src/Concerns/HasExtraAlpineAttributes.php +++ b/packages/support/src/Concerns/HasExtraAlpineAttributes.php @@ -17,6 +17,9 @@ trait HasExtraAlpineAttributes */ public function extraAlpineAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraAlpineAttributes[] = $attributes; } else { diff --git a/packages/support/src/Concerns/HasExtraAttributes.php b/packages/support/src/Concerns/HasExtraAttributes.php index e06c623f96..ab8216e59c 100644 --- a/packages/support/src/Concerns/HasExtraAttributes.php +++ b/packages/support/src/Concerns/HasExtraAttributes.php @@ -17,6 +17,9 @@ trait HasExtraAttributes */ public function extraAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraAttributes[] = $attributes; } else { diff --git a/packages/support/src/Concerns/HasIcon.php b/packages/support/src/Concerns/HasIcon.php index 4fdc261ffb..b148319dea 100644 --- a/packages/support/src/Concerns/HasIcon.php +++ b/packages/support/src/Concerns/HasIcon.php @@ -14,6 +14,10 @@ trait HasIcon public function icon(string | BackedEnum | Htmlable | Closure | null $icon): static { + // Security: Icon strings are escaped when rendered as URLs, but + // invalid icon names from user input will cause rendering errors. + // Validate against a known allowlist if user-controlled. + $this->icon = filled($icon) ? $icon : false; return $this; diff --git a/packages/tables/src/Columns/CheckboxColumn.php b/packages/tables/src/Columns/CheckboxColumn.php index 762f898aee..a0abd26158 100644 --- a/packages/tables/src/Columns/CheckboxColumn.php +++ b/packages/tables/src/Columns/CheckboxColumn.php @@ -13,6 +13,10 @@ use Illuminate\Support\Js; class CheckboxColumn extends Column implements Editable, HasEmbeddedView { + // Security: This column saves directly without checking Laravel + // Model Policies. Use `disabled()` to restrict editing + // based on your own authorization logic. + use Concerns\CanBeValidated; use Concerns\CanUpdateState; use HasExtraInputAttributes; diff --git a/packages/tables/src/Columns/Concerns/CanFormatState.php b/packages/tables/src/Columns/Concerns/CanFormatState.php index 63a6e29050..822b3757c4 100644 --- a/packages/tables/src/Columns/Concerns/CanFormatState.php +++ b/packages/tables/src/Columns/Concerns/CanFormatState.php @@ -52,6 +52,9 @@ trait CanFormatState public function markdown(bool | Closure $condition = true): static { + // Security: Markdown is converted to HTML and then sanitized via + // `Str::sanitizeHtml()`. Same inline `style` caveat as `html()`. + $this->isMarkdown = $condition; return $this; @@ -348,6 +351,12 @@ trait CanFormatState public function html(bool | Closure $condition = true): static { + // Security: Content is automatically sanitized via Symfony's + // `HtmlSanitizer`. The default config permits inline `style` + // attributes, which can enable CSS-based attacks (e.g. + // `background: url(...)`). Configure a custom sanitizer + // if rendering untrusted user content. + $this->isHtml = $condition; return $this; diff --git a/packages/tables/src/Columns/Concerns/CanOpenUrl.php b/packages/tables/src/Columns/Concerns/CanOpenUrl.php index 025eb30995..5978071473 100644 --- a/packages/tables/src/Columns/Concerns/CanOpenUrl.php +++ b/packages/tables/src/Columns/Concerns/CanOpenUrl.php @@ -19,6 +19,10 @@ trait CanOpenUrl public function url(string | Closure | null $url, bool | Closure | null $shouldOpenInNewTab = null): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` protocol URLs rendered + // in `href` attributes. + if ($shouldOpenInNewTab !== null) { $this->openUrlInNewTab($shouldOpenInNewTab); } diff --git a/packages/tables/src/Columns/Concerns/CanUpdateState.php b/packages/tables/src/Columns/Concerns/CanUpdateState.php index bc485823d4..c460e3d12d 100644 --- a/packages/tables/src/Columns/Concerns/CanUpdateState.php +++ b/packages/tables/src/Columns/Concerns/CanUpdateState.php @@ -9,6 +9,12 @@ use Illuminate\Support\Arr; trait CanUpdateState { + // Security: Inline editable columns (`ToggleColumn`, `TextInputColumn`, + // `SelectColumn`, `CheckboxColumn`) do not automatically check Laravel + // Model Policies before saving. Only the `disabled()` state is + // checked. Use `disabled()` with a closure, or use a full edit + // page / modal action where resource authorization is enforced. + protected ?Closure $updateStateUsing = null; protected ?Closure $beforeStateUpdated = null; diff --git a/packages/tables/src/Columns/Concerns/HasExtraCellAttributes.php b/packages/tables/src/Columns/Concerns/HasExtraCellAttributes.php index 9e7c584409..70b915819f 100644 --- a/packages/tables/src/Columns/Concerns/HasExtraCellAttributes.php +++ b/packages/tables/src/Columns/Concerns/HasExtraCellAttributes.php @@ -17,6 +17,9 @@ trait HasExtraCellAttributes */ public function extraCellAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraCellAttributes[] = $attributes; } else { diff --git a/packages/tables/src/Columns/Concerns/HasExtraHeaderAttributes.php b/packages/tables/src/Columns/Concerns/HasExtraHeaderAttributes.php index 75e142de28..e8b3507f82 100644 --- a/packages/tables/src/Columns/Concerns/HasExtraHeaderAttributes.php +++ b/packages/tables/src/Columns/Concerns/HasExtraHeaderAttributes.php @@ -17,6 +17,9 @@ trait HasExtraHeaderAttributes */ public function extraHeaderAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraHeaderAttributes[] = $attributes; } else { diff --git a/packages/tables/src/Columns/ImageColumn.php b/packages/tables/src/Columns/ImageColumn.php index 5930ac4eac..03b9bef7c3 100644 --- a/packages/tables/src/Columns/ImageColumn.php +++ b/packages/tables/src/Columns/ImageColumn.php @@ -282,6 +282,9 @@ class ImageColumn extends Column implements HasEmbeddedView */ public function extraImgAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraImgAttributes[] = $attributes; } else { diff --git a/packages/tables/src/Columns/SelectColumn.php b/packages/tables/src/Columns/SelectColumn.php index 922356383e..aaab2e432d 100644 --- a/packages/tables/src/Columns/SelectColumn.php +++ b/packages/tables/src/Columns/SelectColumn.php @@ -39,6 +39,10 @@ use function Filament\Support\generate_search_term_expression; class SelectColumn extends Column implements Editable, HasEmbeddedView { + // Security: This column saves directly without checking Laravel + // Model Policies. Use `disabled()` to restrict editing + // based on your own authorization logic. + use CanDisableOptions; use CanSelectPlaceholder; use Concerns\CanBeValidated { @@ -329,6 +333,9 @@ class SelectColumn extends Column implements Editable, HasEmbeddedView public function allowOptionsHtml(bool | Closure $condition = true): static { + // Security: Enabling HTML in options renders them without escaping. + // Only use with trusted content — never with raw user input. + $this->isOptionsHtmlAllowed = $condition; return $this; diff --git a/packages/tables/src/Columns/TextInputColumn.php b/packages/tables/src/Columns/TextInputColumn.php index b6e3a5e216..53e4b6286c 100644 --- a/packages/tables/src/Columns/TextInputColumn.php +++ b/packages/tables/src/Columns/TextInputColumn.php @@ -22,6 +22,10 @@ use function Filament\Support\generate_icon_html; class TextInputColumn extends Column implements Editable, HasEmbeddedView { + // Security: This column saves directly without checking Laravel + // Model Policies. Use `disabled()` to restrict editing + // based on your own authorization logic. + use Concerns\CanBeValidated; use Concerns\CanUpdateState; use HasExtraInputAttributes; diff --git a/packages/tables/src/Columns/ToggleColumn.php b/packages/tables/src/Columns/ToggleColumn.php index 4a2e99b9ab..0a0efdcb5c 100644 --- a/packages/tables/src/Columns/ToggleColumn.php +++ b/packages/tables/src/Columns/ToggleColumn.php @@ -21,6 +21,10 @@ use function Filament\Support\get_component_color_classes; class ToggleColumn extends Column implements Editable, HasEmbeddedView { + // Security: This column saves directly without checking Laravel + // Model Policies. Use `disabled()` to restrict editing + // based on your own authorization logic. + use Concerns\CanBeValidated; use Concerns\CanUpdateState; use HasToggleColors; diff --git a/packages/tables/src/Concerns/HasColumns.php b/packages/tables/src/Concerns/HasColumns.php index 8354acc403..92af772966 100644 --- a/packages/tables/src/Concerns/HasColumns.php +++ b/packages/tables/src/Concerns/HasColumns.php @@ -80,6 +80,11 @@ trait HasColumns */ public function callTableColumnMethod(string $name, string $recordKey, string $method, array $arguments = []): mixed { + // Security: This method is callable from the frontend and dispatches + // to `#[ExposedLivewireMethod]` methods on table columns. It does + // not perform per-record policy checks. Inline editable columns + // called through here bypass Model Policies. + $column = $this->getTable()->getColumn($name); if (! $column) { diff --git a/packages/tables/src/Table/Concerns/HasRecordUrl.php b/packages/tables/src/Table/Concerns/HasRecordUrl.php index ddc5666dd0..5b83139066 100644 --- a/packages/tables/src/Table/Concerns/HasRecordUrl.php +++ b/packages/tables/src/Table/Concerns/HasRecordUrl.php @@ -28,6 +28,10 @@ trait HasRecordUrl public function recordUrl(string | Closure | null $url, bool | Closure | null $shouldOpenInNewTab = null): static { + // Security: If this URL is derived from user input, validate it + // to prevent XSS via `javascript:` protocol URLs rendered + // in `href` attributes. + if ($shouldOpenInNewTab !== null) { $this->openRecordUrlInNewTab($shouldOpenInNewTab); } @@ -82,6 +86,9 @@ trait HasRecordUrl */ public function extraRecordLinkAttributes(array | Closure $attributes, bool $merge = false): static { + // Security: Attribute values are not escaped when rendered. Never + // pass unsanitized user input as attribute names or values. + if ($merge) { $this->extraRecordLinkAttributes[] = $attributes; } else {