Sandbox incident template rendering - #403
Conversation
Twig incident templates were rendered by a bare Environment, so a template body could reach any PHP function through the filters that take a callable. Register SandboxExtension with a SecurityPolicy that allows only the tags and filters a status update needs. Blade bodies compile to PHP and Blade has no sandbox, so rendering them is now opt in through cachet.incident_templates.allow_blade. While the option is off, the engine fails validation on create and update, is disabled in the form, and BladeRenderer refuses to render.
| | application, so they are only rendered when enabled here. | ||
| | | ||
| */ | ||
| 'incident_templates' => [ |
There was a problem hiding this comment.
I'd prefer to make this more generic as renderers or something.
There was a problem hiding this comment.
Renamed in 5e5439d. The block is now renderers and the value cachet.renderers.blade, with CACHET_BLADE_RENDERER as the env variable. Availability moved onto the engine: isAvailable() reads the config for its own renderer and available() derives the list from that, and BladeRenderer now checks the config value directly instead of asking the incident template enum. Twig stays unconditional, so an installation carrying an older published config/cachet.php keeps rendering Twig and still gets no Blade.
If you meant a registry that maps an engine to a renderer class, 'renderers' => ['twig' => TwigRenderer::class] with Blade left out by default, I am happy to redo it that way. I stayed with the boolean because a published config predating the key would then resolve to no renderers at all rather than to Twig.
Rename cachet.incident_templates.allow_blade to cachet.renderers.blade, so the config block is named after what it configures rather than after the one feature that uses those renderers. The env variable becomes CACHET_BLADE_RENDERER. Availability now lives on the engine itself: isAvailable() reads the config for its own renderer and available() derives the list from it. BladeRenderer reads the config value directly instead of asking the incident template enum.
2 participants
What
IncidentTemplate::render()hands the stored template body to a bare TwigEnvironment(src/Renderers/TwigRenderer.php:16) and toBlade::render()(src/Renderers/BladeRenderer.php:14). Both compile and execute that body, so whoever authors a template decides which PHP runs when an incident is created from it (src/Actions/Incident/CreateIncident.php:77).Why
A template body reaches those renderers through
POST /api/incident-templatesand the MCPcreate_incident_templatetool, where it is validated as['required', 'string'](src/Data/Requests/IncidentTemplate/CreateIncidentTemplateRequestData.php:24), and both paths are authorised by theincident-templates.managetoken ability alone (src/Http/Controllers/Api/IncidentTemplateController.php:48). Twig ships a sandbox for exactly this situation and it was not switched on. Blade has no equivalent: an echo tag holds an arbitrary PHP expression, so a Blade body cannot be made safe by filtering directives.How
TwigRendererregistersSandboxExtensionwith aSecurityPolicythat allows the tags and filters a status update needs. Filters that take a callable (map,filter,reduce,sort) are left out, because a template can pass them the name of any PHP function. Methods, properties and functions are all denied, which also closes template introspection through_self.cachet.renderers.bladeoption (CACHET_BLADE_RENDERER, defaultfalse). While it is off,engine: bladefails validation on create and update, the option is disabled in the Filament form, andBladeRendererrefuses to render. Availability sits onIncidentTemplateEngineEnum:isAvailable()reads the config for its own renderer andavailable()derives the list from that. Twig is always available, so an installation carrying an older publishedconfig/cachet.phpkeeps rendering Twig and still gets no Blade.src/Filament/Resources/IncidentTemplates/IncidentTemplateResource.php:49also callsBlade::render(), but on hardcoded translation strings rather than on stored input. The scope ofincident-templates.manageis untouched as well; who may author a template is a separate question from what a template may do.Behaviour change
An installation that already stores Blade templates needs
CACHET_BLADE_RENDERER=true, otherwise creating an incident from such a template fails. Twig templates that use the callable filters,includeor method calls stop rendering; interpolation,if,forand the allowed filters keep working.Testing
PHP 8.3.30 in a container, dependencies from
composer update, assets fromnpm ciandnpm run build, workbench prepared withcomposer build.composer test:unit: 955 passed, 2 todos, 0 failed.composer test:lint(Pint): PASS, 807 files.vendor/bin/phpstan analyse: no errors.Psalm (security)workflow runs it: no errors found.tests/Unit/Models/IncidentTemplateTest.phpgains one case for a Blade template while the renderer is disabled and two datasets for a Twig template that reaches for PHP functions;tests/Feature/Api/IncidentTemplateTest.phpgains the rejected and the acceptedengine: bladerequest. All four fail onmainat78b41c4and pass with this change.References
GHSA-34gh-p78x-w96h (reported privately, in triage)