I'm leaning on a -1 here, var() is currently banned (awaiting a patch as it's fixed) as it caused a security issue. I'd like to know how the developer determined that it wasn't a security risk when they decided to allow this to enable it.

Spoken to another sysadmin, we are declining this as extending TemplateStyles is too much of a risk without high enough security standards.

We strongly encourage you to raise this upstream. You'll probably already find tasks for a few of them so it can be added to TemplateStyles.

In T8052#162343, @RhinosF1 wrote:

I'm leaning on a -1 here, var() is currently banned (awaiting a patch as it's fixed) as it caused a security issue. I'd like to know how the developer determined that it wasn't a security risk when they decided to allow this to enable it.

Is there more information on the security issue? If you're talking about T208881, it is fixed by browsers two years ago. Besides TS extender doesn't allow defining CSS variables, but only using it. So the variables have to be defined either through the MW namespace or other extensions, which are not sanitized anyways.

In T8052#162349, @RhinosF1 wrote:

Spoken to another sysadmin, we are declining this as extending TemplateStyles is too much of a risk without high enough security standards.

We strongly encourage you to raise this upstream. You'll probably already find tasks for a few of them so it can be added to TemplateStyles.

There are multiple years-old tasks upstream already on adding modern rules into TemplateStyles/CSS sanitizer. The current WMF stance is to only add rules when it becomes a published standard, which alienated many rules that are in the draft standards. With that being said, the whole purpose of developing this extension is an add-on to the original TS as upstream is not an option.

While the use case is good, I still want to ensure it's developed to high standard with risks taken into account.

How can we guarntee thats done by the developer?

In T8052#162854, @RhinosF1 wrote:

While the use case is good, I still want to ensure it's developed to high standard with risks taken into account.

How can we guarntee thats done by the developer?

What are the specific risks that you're referring to? I will be able to provide a clearer insight if I understand what you're concerned about.

The extension only adds a few additional rules to the allow list of TemplateStyles. CSS is a logicless language that is relatively safe compared to anything. Besides, TemplateStyles with TemplateStylesExtender is still enforce stricter than the Extension:CSS, which allows unsanitized input.

In T8052#162854, @RhinosF1 wrote:

While the use case is good, I still want to ensure it's developed to high standard with risks taken into account.

How can we guarntee thats done by the developer?

I do know this developer is active, and am confident in speedy reply for security issues, but regardless I still have some concerns. I can look into it though. I agree the use case is good enough to consider it though. However, nothing is good enough to risk a security issue, therefore it will be declined if I'm not 100% sure it is security safe. I also noticed now that the extension has configuration options to disable certain features of it. Therefore I am more willing to approve it. Nonetheless I will have to thoroughly review it.

We might be able to approve this since there is $wgTemplateStylesExtenderEnableCssVars, but I'd like a second opinion on that, and this extension's functionality first.

The extension code itself is fine, it's the functionality that concerns arise with. Moving out of security review needed, because it's technically approved from security, but leaving to the rest of SRE to decide whether the functionality is OK.

@RhinosF1, @Reception123 any thoughts on this?