Twig templates can call delete() on entities and other objects

Created on 27 June 2015, over 9 years ago
Updated 12 September 2023, about 1 year ago

Beta phase evaluation

Problem/Motivation

We are passing fully functional entity objects to Twig templates instead of read-only ViewModels. This allows templates to do bad things, such as node.delete() which defeats a major benefit of using Twig over the PHPTemplate engine.

Proposed resolution

Twig Sandbox policy with whitelisted methods and properties. The list of methods and properties will be configurable as we will undoubtedly miss something a contrib (or even core) module needs access to.

The following methods are whitelisted:

  • id()
  • label()
  • bundle()
  • get()
  • __toString()

Also, methods that start with the following strings are whitelisted:

  • get
  • has
  • is

Finally, any method on the Attribute class is whitelisted as that class is designed to be changed from Twig templates (e.g.: <div {{ attributes.addClass("myClassName") }}

These options are configurable in settings.php (or settings.local.php) as such:

$settings['twig_sandbox_whitelisted_methods'] = [
  'foo',
];
$settings['twig_sandbox_whitelisted_prefixes'] = [
  'bar',
];
$settings['twig_sandbox_whitelisted_classes] = [
  'BazClass',
];

The above code will allow only methods named foo or methods that begin with bar (such as barGetSomeValue), overriding the existing method and prefix whitelists. Any method call on the BazClass class will be allowed.

The current implementation allows access to any public properties on any class. This can be easily changed in future implementations.

Remaining tasks

  1. Use Sandbox policy
  2. While adding $settings to a settings.php is not ideal, it does allow these whitelists to be customized.
  3. Follow-up issue: Allow Sandbox policies to be added as services
  4. Now configurable via $settings.
  5. Done, though the sooner we get this in, the sooner we can figure out other methods/prefixes that should be added to the default lists.

User interface changes

n/a

API changes

None. Though contrib modules that provide objects for use in Twig templates will need to either adjust their API to use the default whitelisted methods and prefixes or advise site builders using their module to add the appropriate methods and/or prefixes to $settings as indicated above.

Data model changes

None.

Original report

I tried running {{ node.delete }} within node.html.twig and the node deleted. Bug or feature? I was talking with Larry Garfield and Lewis Nyman at Drupal Camp Twin Cities about the idea of only passing "ViewModels" or otherwise stripped down data into our templates rather than full entities with all methods available. We agreed that the presence of a delete method in Twig cuts against one of the original Twig selling points. The discussion around Twig originally (and still) often highlights that queries and other "bad" methods and functions simply can't be called from a template file. Someone shouldn't be able to destroy data on their site by editing a template.

Which entity methods (if any) should be available from a Twig template?

πŸ› Bug report
Status

Fixed

Version

8.0 ⚰️

Component
RenderΒ  β†’

Last updated 3 days ago

Created by

πŸ‡ΊπŸ‡ΈUnited States stevector

Live updates comments and jobs are added and updated live.
  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Production build 0.71.5 2024