Add configuration entry to allow adding additional elements.

[lhupfeldt]

I'm new to Jenkins plugin development and need some help with this.
There is an issue with initialization after Jenkins restart, causing a NullPointerException.
Everything works correctly after initially adding allowed elements, but after a Jenkins restart I need to visit the configuration page and click 'save'. The configuration is correctly saved and reloaded, but the class is not initialized correctly after restart.

[lhupfeldt]

[daniel-beck]

Not a maintainer, but IMO this feels like something best done in another plugin to limit the complexity of the "default option".

[lhupfeldt]

Not a maintainer, but IMO this feels like something best done in another plugin to limit the complexity of the "default option".

Do you mean code complexity or end user configuration complexity?
I don't think the code complexity of the "default option" is much increased, of cause there is extra code complexity to handle the user defined extra elements.

I'm not sure how it would work if this was put in another plugin, could I have that plugin .depend on this plugin and overwrite it's functionlaity? I use the Active Choices plugin which depends on this plugin. The total maintenance and dependency handling of having a split plugin solution (if possible) would also be bigger than having it in a single plugin.

I also think that it would be confusing for users to have to install a separate plugin, adding the configuration to this plugin makes it simple for users to discover, and if you don't need it there is really no added configuration complexity.
I think the functionality of adding allowed elements would be useful for a lot of users.

[malte-behrendt-gr]

We implemented the same functionality in a very similar fashion.

For us, this functionality is a must-have for being able to make changes/status/options visible in a readable manner to the entire team.

In my opinion, the complexity does not get increased in a significant way and I do not understand the benefit of creating another plugin yet. @daniel-beck can you expand on why this may be the better approach?

@lhupfeldt I installed your implementation in a docker container and it seems to work fine. Why is this PR in Draft mode aka how can I support you in finalizing this PR (without being a maintainer) (we like your implementation more than ours :-) )?

[lhupfeldt]

@malte-behrendt-gr Check my first comment. It does not work after restart, that's why it is in draft. Your help would be appreciated. Although it seems the maintainers are not accepting any pull requests.

[lhupfeldt]

@malte-behrendt-gr I have fixed the restart initialization problem.

[malte-behrendt-gr]

@lhupfeldt :-D fantastic

In the meantime we combined your solution and ours.
It also has the initialization issue fixed, although differently, and we added another input field as well as few basic tests.
It is not public yet (we also renamed the plugin for avoiding issues when another team updates the Jenkins and the plugin gets overwritten with the official version) but If you like I can PM you the code (LinkedIn, there I found, ok?)

I also noticed you seem to work on something similar as we do (Jenkins and Python) - I hope I'll get around to take a closer look on it soon.

[lhupfeldt]

Are any maintainers looking at pull requests?

[daniel-beck]

I use the Active Choices plugin which depends on this plugin.

Makes sense. I added this dependency years ago to help fix yet another security vulnerability in that plugin. It might be a better choice longer term to detach Active Choices formatting from the global markup formatter if more customization is regularly needed.

Given the frequency and severity of vulnerabilities in Active Choices, particularly XSS, as a reference use case I don't think it's particularly strong. Even ignoring that, this plugin has 10x the number of installs.

I don't think the code complexity of the "default option" is much increased, of cause there is extra code complexity to handle the user defined extra elements.

Depends on whether you want to help the admin make good decisions. title is a safe attribute to allow, tooltip is not. Nothing informs the admin they'd make a big mistake by allowing tooltip; superficially it seems safe, and more consistent with other UI elements' tooltips in Jenkins.

[lhupfeldt]

Depends on whether you want to help the admin make good decisions. title is a safe attribute to allow, tooltip is not. Nothing informs the admin they'd make a big mistake by allowing tooltip; superficially it seems safe, and more consistent with other UI elements' tooltips in Jenkins.

I think that it should be the admin's decision what to allow. Something inacceptable in a certain may be perfectly fine in another.
But a warning in the help text could be a good idea.

[daniel-beck]

I think that it should be the admin's decision what to allow.

Sure, but they should be equipped to make good decisions. There's already too many "gotchas" in Jenkins configuration, let's not add more.

Something inacceptable in a certain may be perfectly fine in another.

https://plugins.jenkins.io/anything-goes-formatter/ exists; if you're fine with XSS, you're fine with everything and can allow everything (rather than pretend to only allow a subset).

[lhupfeldt]

I still have the Active Choices plugin which depends on this plugin. How would it work if I installed anything-goes-formatter as well? And I don't need/want anything goes. I really think that allowing admin to configure (given appropriate warning) what is allowed is the best solution. If you are admin and don't have the dependency issue, then you could install anything-goes-formatter, but I don't see how forcing people to go that way, would that be better for security than allowing configuration of the standard plugin?
Could you explain how a basic span with plain text title is a security risk? (I read your comment again, you actually said it is not)

[daniel-beck]

How would it work if I installed anything-goes-formatter as well?

Good point, this regularly trips me up, including in an earlier response above -- Active Choices always uses this plugin rather than whatever is the configured markup formatter.

This means Active Choices would need to change how it sanitizes output by switching to a different (perhaps more configurable) sanitizer, possibly provided on its own. Alternatively, it could allow configuring the markup formatter it uses from among all installed ones (rather than whichever is configured for all other text).

(Personally I would still consider this use case to be too narrow to justify changing this markup formatter installed on most Jenkins instances.)

[lhupfeldt]

The dependency problem is not the only use case. There are several people asking for support of different things which are not allowed by the plugin. E.g. why was font disallowed? True, it is deprecated, but it is not a security risk. Making this plugin configurable is really the only way to stop breaking (at least allowing an easy fix) peoples setup. Those who are happy with the defaults probably will not even discover that the plugin is configurable. I think a proper warning in the help text should be enough for admins to decide on their own what to allow.
Btw. what is the prodedure for updating the language files?

[daniel-beck]

The dependency problem is not the only use case. There are several people asking for support of different things which are not allowed by the plugin.

It's the only use case I can think of that cannot currently be solved by having people migrate to a new customizable-html-markup-formatter-plugin (or similar) that implements this feature. This keeps the default simple to use, while enabling customization for everyone needing it.

Plugin hell can be real with Jenkins, but sufficiently narrow plugins are IMO a clear benefit. See e.g. https://plugins.jenkins.io/strict-crumb-issuer/ which has a bunch of options most people won't care about (and are lacking in the built-in implementation).

Btw. what is the prodedure for updating the language files?

Depends on scope, I mostly do it manually but https://github.com/jenkinsci/jenkins/blob/master/translation-tool.pl may help with some content.

[lhupfeldt]

I don't agree that the defaults are kept any simpler by not adding a configuration possibility. The defaults are exactly the same - you do not have to configure enything is you want the defaults. On the other hand splitting this into multiple plugins requires more total maintenance and the risk of a plugin not receiving the necessary security updates will be higher. And we still have not solved the problem of plugin dependency.
The proper solution for the dependency mechanism should be dependencies on plugin "interfaces", allowing to swap in different implementations, but that is a different story. And even in that case I would still argue that having the "default" plugin be configurable is better than having multiple implemetations.

[daniel-beck]

The proper solution for the dependency mechanism should be dependencies on plugin "interfaces", allowing to swap in different implementations, but that is a different story. And even in that case I would still argue that having the "default" plugin be configurable is better than having multiple implemetations.

For the general case (i.e. not Active Choices), that's basically how it works. There's nothing "default" about OWASP Markup Formatter other than it being recommended by the setup wizard (and it is recommended because it's useful and simple).

I don't agree that the defaults are kept any simpler by not adding a configuration possibility.

People have been complaining for at least a decade now (which is when I started using Jenkins, so probably for longer) about the confusing number of seemingly irrelevant options exposed in Jenkins configuration, so I disagree with this stance. Adding options for things only few people care about in this manner isn't a solution, it just adds work for everyone who needs to determine whether something is relevant to them or not.

[mdealer]

This would a non-bureaucratic solution to start using the

tag: https://www.w3schools.com/tags/tag_details.asp

But alas, we first need to discuss it in 10 rounds and then actually make things way worse by providing a yet another plugin that does the same thing. It's circular logic.

We could go ahead and remove URL support, because admins might misclick something. Jenkins might not be the place for such admins...

[daniel-beck]

FYI a plugin allowing customizing the set of allowed elements is currently in the hosting process: jenkins-infra/repository-permissions-updater#3364