Theme/plugin repository cleanup and quality control

[pamtbaau]

As in the past, I have again done some grep-ing on the theme repository, because of recurring posts on Discourse (and Grav repo) about incompatible themes.

Results

The results are not really encouraging:

• 109 available themes tested (excluding Typhoon and Gantry themes)
• 64 themes are not Grav 1.7 compatible with respect to autoescape
• 45 themes are Grav 1.7 compatible,
  • of which 27 are maintained by Team Grav/Trilby Media
  • only 18 (22%) out of 81 themes build by community members have been upgraded
• The chance to install an incompatible theme is 59%

Note: I've only tested for the absence of the |raw filter on {{ *.content|raw }}, {{ assets.css()|raw }} and {{ assets.js()|raw }}, because these are the main issues creating a frustrating user experience.
I've marked some themes as being compatible because they use filter |markdown, which also make the theme display correctly, but is in fact an incorrect use of the filter.
Yes, I've done my best to check for false positives.

What makes a theme incompatible with Grav 1.7?

Since Grav 1.7, all output from Twig templates is so called autoescaped, which prevents possible cross-site-scripting (XSS) security issues. The implementation of autoescape was a nescesseray decision, but unfortunately introduced a breaking change in themes which are not prepared to handle autoescape.

During the automated upgrade process of Grav 1.6 to Grav 1.7, autoescape is switched off to prevent existing sites to suffer from breaking themes.
This was nescessary, but is not ideal, because this leaves the security vulnerability still open.

Fresh installs of Grav 1.7.x leave autoescape switched on and hence do not display incompatible themes correctly. The suggested fix to manually switch off autoescape leaves the site again vulnerable to XSS attacks.

From mostly upgrades to mostly fresh installs of Grav 1.7

Time has passed since the introduction of Grav 1.7. I think there are nowadays more sites created using a fresh install of Grav 1.7, than sites being upgraded from Grav 1.6. For these new installs, incompatible themes (59%) create a frustrating first impression. And when incompatibilty is being "fixed" a site is again vulnerable to XSS attacks.

Suggestions

A few suggestions to prevent:

• user frustration when an installed theme "doesn't work"
• users from running sites with a known security vulnerability

Fixing incompatible themes

• Contact theme developers:
  • Create issue for repos of all incompatible themes and ask for upgrade. This can be done in bulk using the Github rest-API
  • If no upgrade within a certain period, mark the theme as being abandoned in the list of themes, and/or (preferably) remove the theme from the list.

Prevent installation of incompatible theme

• By default, only show compatible themes in theme lists (Admin/GPM/Download site) and add option to also list incompatible themes.

  • Or clearly mark all incompatible themes in the list of themes.
    Explain why the theme is incompatible, point to "fix" and explain security risk when applying "fix".

• Block users from installing an incompatible and vulnerable theme using Admin/GPM. Inform user about issues.

  • Provide a -force flag in case the user persists in installing the theme. The user might have good reasons.

• Show last-update date.
  This is an indication that theme might not be the best choice.

• Show for which Grav version the theme has been tested by developer (see below).

Quality control

• Auditing new themes/plugins:
  • Implement a clear auditing procedure for new themes/plugins
  • To relieve the Grav team, ask the community to perform audits.
  • Implement clear guidelines and requirements for submitting themes/plugins.
    For example:
    • A theme should have demo data to make it easy for a new user to get started.
      I think we can savely assume that every theme being developped already has data to test against.
    • A theme should implement a .dependency to install plugins required for the demo to work properly.
    • Add a new property in blueprints.yaml to indicate on which version of Grav the theme/plugin has been tested.
    • Should we perhaps setup/enforce coding standards?
      • To create uniformity among themes
      • To increase readability by auditors, clone-ers, extenders.
  • If no demo data available, a new theme should work properly on a fresh installation of latest Grav version.
  • Stretch: Create a new function in Devtools that runs check for:
    • Yaml issues
    • Code issues
    • Incomplete requirements
    • ...

Yes, this is quite a bit to read and digest, but nonetheless I would really appreciate feedback and/or suggestions...

[ghost]

Totally agree that it would be good to sort out this issue somehow. Especially to help with this: "user frustration when an installed theme doesn't work". The themes are presented in a way that suggests they are there to be used, and the first one on the list doesn't work, so new users inevitably think they have done something wrong and ask about it on the forum. This wastes everyone's time.

My simplest suggestion would be to add a line at the top of the theme page to alert users that some themes may be out of date. That will at least tell users that it's not something they are doing wrong. That message could also tell them to check the changelog, which was the best way I found to quickly assess a themes viability.

Another easy change would be to load Grav Team themes by default, instead of using the current "name" selector that shows them all. If a user chooses the name option they would then see a message warning them some themes may be outdated.

Showing the "last updated" information might be another easy solution. It's pretty clear that if it was last updated 5 years ago you shouldn't touch it.

Frankly, I think any theme that has a security issue shouldn't be there at all. I don't know the mechanism for having themes added to that page but I would be contacting the developer and asking them to update within 14 days or it will be removed.

[wernerjoss]

well, here are my 0.02€ .
I don't think it is a good idea not to list incompatible/outdated themes, but would rather opt for clearly marking them.
as for the fix, I think it would be helpful to create a helper script e.g. in the bin folder, named fix_theme or the like, which just does the mentioned filter additions.
then point the fix proposal to that script in the first place, and only in second, to switch off autoescape.
I also opt for showing the "last updated" information for themes.
plus, show for which Grav version the theme has been tested.
these are 2 things where Wordpress does better than Grav 😄

[pamtbaau]

@wernerjoss, Thanks for your feedback.

I don't think it is a good idea not to list incompatible/outdated themes, but would rather opt for clearly marking them.

That depends...

• Responsibility:
  Out of responsibility, I would not expose themes that have a security vulnerability to users. Especially inexperienced users (see target audience).
• Impression:
  If a new user wants to change the theme and sees a list of which 59% is clearly marked with a red flag, it would not look good. I, for one, would seriously doubt my choice for Grav. It would be a telltale about quality control, responsibility towards users and maturity.
  For a good first impression, a clean list of themes will be better.
• Target audience of Grav:
  If Grav is targeted at every level of skills/experience, it should present a user experience aimed at the user with the lowest skills.
  If Grav is targeted at developers (as been said elsewhere), the required skills level should be clearly communicated. Judging from Discourse, there are many inexperienced users using Grav.

I also opt for showing the "last updated" information for themes.
plus, show for which Grav version the theme has been tested.
these are 2 things where Wordpress does better than Grav

Apart from "last update" and tested version, you could also mention:

• How many downloads the plugin/theme has
• How many ratings the plugin/theme has and how good they are
• If there are any good reviews available for the plugin/theme
• How responsive the plugin/theme author is to issues and requests

[mahagr]

I and Andy have been adding some tools in my free time to detect compatibility and quality issues in both plugins and themes, but so far I've mostly used those to improve the main plugins.

One of them is phpstan, which basically compiles all the PHP code and finds deprecated and badly written code. It can already be used to test plugins as long as you use git version of grav, install dev dependencies from composer and symlink tests/ folder into the site. There's also yaml linter for detecting issues in yaml files.

In the future, I hope to have basic tests run for all plugins and themes, but there's still some work to do in order to make it happen.

[pamtbaau]

@mahagr, Thanks for your feedback.

Personally I consider static code analysers (Phpstan/Psalm) a "must use" when developing plugins. Themes however are not that heavy on raw PHP, but instead mostly on Twig, for which, afaik, Phpstan/Psalm do not offer much help.

• Would it be possible to add Phpstan to composer.json together with default config files when generated by Devtools? Together with a suitable script?
• Are these tools used for evaluating new plugins being offered?
• What is the current evaluation procedure for themes/plugins?

[mahagr]

The tests for plugins are actually part of Grav composer, though I need to improve it so it allows you to test individual plugins. I've been using it for a long time already.

Basically, you need to have git version of grav and particularly test/ folder in it. After that, you can just run the command to test your plugin.

We already have tool to check YAML files, but twig should also be tested -- preferably against Twig 2 or 3.

[gizmecano]

• A theme should implement a .dependency to install plugins required for the demo to work properly.
• Add a new property in blueprints.yaml to indicate on which version of Grav the theme/plugin has been tested.

These two points seem particularly interesting: in this way, it would be possible to really display the compatibility of a theme or a module with one (or more) specific versions of Grav (a bit like the WordPress directory, as in the screenshot below).

[aaronxn]

Explanation

I started https://discourse.getgrav.org/t/plugin-theme-maintenance/22992 in order to try to get the ball rolling in improving Grav by dealing with the broken theme and plugin issue. It got some attention, then it was closed. Yes, it is going to be a lot of work, but isn't that worth it to keep Grav a good and useful application? Instead of closing it, it should have been used as a starting point and improved upon. A new thread or way of organizing the task should have been at least discussed.

Moving on ...

For Discussion

• A system for marking themes and plugins with what version of Grav they were tested on should be discussed and implemented.
• A protocol for removing old themes and plugins should be discussed and implemented. According to https://learn.getgrav.org/17/advanced/grav-development#abandoned-resource-protocol there are no protocols for removing broken plugins.

An example: the Resize Images v0.2.2 plugin has not been updated since 2016. This info is not readily available as it is only displayed on the Github site, and you have to fish for it because the Github link on the plugin page pulls up a 404 not found error.

All I am trying to do is get the ball rolling. I like Grav, and I want to do what I can to keep it up-to-date and useful.