Update check for Kirby and plugins

[lukasbestle]

Update check for Kirby and plugins

Our new update check in the enhanced system view makes it easy to stay informed about the version and security status of Kirby and your installed plugins. While feature updates are not always needed for finished sites, keeping an eye on security issues and important security messages is really important to keep your sites secure and healthy. We already publish all relevant information on our security page. The new update check brings this information and more right into the Panel so you can get a quick overview of the status of your particular site.

We designed the feature to work in your interest: It always prefers the minimum possible security update over feature updates to make security updates as easy as possible. It never performs an auto-update but just displays the relevant information. It never transmits any personal or site data to our server but processes the necessary data directly on your own server. And if you prefer to keep up to date with updates in another way, you can fine-tune the behavior or disable the feature all-together:

<?php

// global configuration
return [
  'updates' => true | 'security' | false
];

// separate configuration for Kirby and plugins
return [
  'updates.kirby'   => true | 'security' | false,
  'updates.plugins' => true | false,
];

// or even configuration by plugin
return [
  'updates.plugins' => [
    '*'                      => true | false,
    'superwoman/*'           => true | false,
    'superwoman/superplugin' => true | false
  ]
];

---

Related links

https://kirby.nolt.io/111

Testing information

To test different scenarios, please modify the version number in kirby/composer.json. Changes to the version number will be picked up immediately. After a reload of the system view you can verify if the output matches what you expect.

Unit tests are still missing for the main parts. I already wanted to create the PR today so you have time to look at it before RC.1. I will add complete unit and integration tests for all the logic until RC.2. My goal is to write detailed integration tests with lots of different raw inputs and their expected outputs, including edge cases.

Open questions

• [ ] I'm not 100 % happy with the TableUpdateStatusCell.vue component yet. Its logic works well, but it could use better styling. I'm mostly not sure about the link styling: The underline is consistent with UrlFieldPreview, but the visual semantics differ from the Kirby update status in the k-stats. In both cases the link leads to the release notes of the update (if an update is available) or of the current version (if no update is available), but the stat (Kirby) & table cell (plugin) always display the current version. So I'm wondering if we should make the whole table cell clickable. Then it's more clear that the link is about the version and its update, not just the current version.
• [ ] The plan is to use https://assets.getkirby.com (KeyCDN) to serve the update files. We need to verify that we can indeed use KeyCDN for this (bandwidth, cost, caching behavior...).

Ready?

• [ ] Unit tests for fixed bug/feature
• [x] In-code documentation (wherever needed)
• [x] Tests and checks all pass

For review team

• [ ] Add changes to release notes draft in Notion
• [ ] Add to website docs release checklist (if needed)
  • update option
  • Privacy policy
  • Privacy guide

[lukasbestle]

Regarding the config options: My thought was to make the options extensible in the future, e.g. with notify to send an update notification to the admins or autoupdate (in case this is ever needed/requested). But maybe I'm planning ahead too far. I'd be fine with your suggestion as it's indeed a lot simpler.

[bastianallgeier]

A few quick first thoughts from me as well.

• It already works really well. Nice job!
• I agree with @distantnative's thought on the available config flags (true, false, security) I think this could still be extended later.
• My first thought on the config option naming was, that it should be called updates instead of update
• The different text colors in the system view show one thing clearly: our colors our fucking ugly for text. Especially the blue and green. I think we need a different way to style this here.
• I'm not very happy with the info second line in our Stats component. I wonder if this could be done more elegantly, but that's definitely my job.
• When security updates are activated, there's the text "No known vulnerabilities" next to a plugin. I think we should just show a green / ok status here. It feels weird in combination with plugins without a version number. What about them? Do they have any vulnerabilities? I think sometimes too much positive information on first sight is worse than having none. It could still appear in a tooltip for example.

[lukasbestle]

I agree on the config flags and config option naming. I will change that.

Regarding styling and text colors: I'll leave this to you as I don't want to butcher the Panel design. :)

When security updates are activated, there's the text "No known vulnerabilities" next to a plugin. I think we should just show a green / ok status here.

I think this is as expected. If you only want security updates to be displayed, then this is what you get. "No known vulnerabilities" means that an update is available but that it isn't necessary to fix vulnerabilities.

With the current getkirby.com setup we can't publish vulnerabilities for plugins, but we could easily add this later when we extend our hub for plugin devs. That's why the core already treats plugins the same as Kirby regarding the update logic.

It feels weird in combination with plugins without a version number. What about them? Do they have any vulnerabilities?

Without a version number we cannot do any update checks. This is why the update check is only enabled if the plugin has a version number.

It could still appear in a tooltip for example.

Just the "No known vulnerabilities" text or every status value? TBH I wouldn't differentiate between the different status values. It wouldn't be consistent with the Kirby update status in the stats widget.

[distantnative]

"No known vulnerabilities" just feels like a lot of positive endorsement for plugins where we have no clue about their vulnerability status.

[lukasbestle]

If vulnerabilities were known, we would add them to the plugin.json on getkirby.com. If nothing is in there, then the core does know for sure that there are no known vulnerabilities.

[lukasbestle]

Idea for plugins without version number: What if we display the "Could not check for updates" text in this case? Then the only plugins without a status text would be those where the config option was explicitly set to false.

[distantnative]

I know that your are technically right. Nevertheless, we also know how users perceive "No known vulnerabilities". And it's not the technical understanding - "the Kirby team does not know about any vulnerabilities with this plugin" - but more a general one: there are no problems, this plugin is great. And given that we have not even a good setup and mechanism with plugin developers to handle security issues, I don't think it's a good UX, maybe even dangerous - even though the message is technically correct.

[lukasbestle]

I see what you mean.

Idea: We could give the plugin.json a new field securitySupport: true | false. The default for now would be false. Only if the dev explicitly opts in (once we have the hub), we would set it to true.

In mode false, the core would display a message like "Security status unknown".

[distantnative]

That would be an option. But I feel we are getting way ahead of ourselves. For the moment, I think the best experience for plugins (core is different) would be to only show a message when we know of an issue. If we don't know a thing (which is the case for 99% right now), no statement.

[lukasbestle]

Sounds good. So basically we hardcode the "Security status unknown" message for plugins while the core stays "No known vulnerabilities", right?

[lukasbestle]

If I think about it, maybe we should just not support the security-only mode for plugins at all until we have the hub. That would make it all a lot simpler. So plugin updates can only be configured true or false.

[distantnative]

Tbh I think for me personally the best experience would be if nothing is shown when no action is suggested - so up-to-date and no-vuln would just not show any message. And only if one wants to inform about an update available or a security issue to patch, then show a message.

[lukasbestle]

I feel like it could be confusing because the user won't know if the update check is enabled and working. It makes the status quite uncertain to the user.

[lukasbestle]

@bastianallgeier Quick comment on the version dropdown: The url is intended as a release notes URL, not specifically a download URL. Release notes often lead to the download, but I feel like a text like "Version information" and the icon open would better there.

Also the targetVersion is not necessarily the latest version. If there is a paid upgrade or a security issue, the targetVersion will be older.

[lukasbestle]

I've implemented what we discussed today. :)

[lukasbestle]

I think the TableUpdateStatusCell is now at a point where its logic covers all possible cases the backend could throw at it. I think it would be best if @bastianallgeier takes over from here again for styling finalization and cleanup.

[lukasbestle]

The unit tests currently fail. I'll fix them and add more unit tests when I come home.

[bastianallgeier]

I've added my design changes. Anything else we need to have a look at for tomorrow?

[lukasbestle]

Everything should be ready now. PHPUnit tests (unit and integration tests) for Plugin::updateStatus() and the UpdateStatus class are missing. I will add those in the coming days in a separate PR.

[bastianallgeier]

@distantnative @afbora do you want to have another look at it before we merge?

[afbora]

Let me try to few test please

[distantnative]

I won't get to it today, so please just wait for @afbora and I will test more during RC2

[afbora]

I loved PR ❤️ It's nice to see Kirby become more systematic. I've few points about PR. I think these are not critical. I'll more test while RC process.

1. Private repository behavior

Expectation

• Latest version: Unknown (or something like that)
• Update status: Could not check for updates

2. HTTP error 404 issue

I'm getting following error with following setup

Could not load update data for plugin jg/fields-block: HTTP error 404 Could not load update data for plugin test/test: HTTP error 404

Tested plugins

// test plugin
Kirby::plugin('test/test', []);

[lukasbestle]

Thanks for testing! 💛

Number 1 seems to be a bug in the plugin model of getkirby.com. If the latest version is not known, it should set the value to null instead of false. Then the Panel UI should work as expected.

Number 2 is as expected IMO. The errors only occur in debug mode. Otherwise the Panel will just display "Could not check for updates" without throwing an exception. The first plugin probably needs to be published to getkirby.com or has a different name there. For the second example (custom, unpublished plugin), one should disable the update check via the Kirby config.

[lukasbestle]

I've created a PR for jongacnik/fields-block: https://github.com/jongacnik/kirby-fields-block/pull/11

[lukasbestle]

I'll fix getkirby.com tonight for the private plugin repo.

[afbora]

Number 2 is as expected IMO. The errors only occur in debug mode. Otherwise the Panel will just display "Could not check for updates" without throwing an exception. The first plugin probably needs to be published to getkirby.com or has a different name there. For the second example (custom, unpublished plugin), one should disable the update check via the Kirby config.

@lukasbestle I don't fully agree. Because it is not a critical and useful error. For example, it throws an exception when it is not compatible with Kirby version and this is correct behavior. But I think it is not the right behavior to show an error screen because it just cannot find the version information. Because the lack of version information is of no use to the developer or user. It is sufficient to have an error log in the console.

[lukasbestle]

Console is a great idea! We could solve this by catching the exception and converting it to a prop for the system view. This makes the code of the system view a bit more complex, but it's certainly useful. I'll see what I can do.

[bastianallgeier]

@lukasbestle @afbora do you think this is something that needs to be ready for rc.2? Could be a good task for rc.3. Or would it lead to too many complaints?

[lukasbestle]

The error should only be displayed once. After another reload it's gone for three days. I think we can already ship it with RC.2 if you want to publish it today.