Hey all -

As noted above, @Mstyles deployed @Reedy's file-verification patch to Wikimedia production, so we should be good there, going forward. Some notes:

1. We do have some CSP report-only data for upload.wikimedia.org - I've filtered this to the past 90 days for SVGs: https://logstash.wikimedia.org/goto/76e0e3df1b7d7b2cee870bd2ce43dfc5. I'd forgotten we still have this VCL config in place, which only sets a report-only CSP for non-PDFs for the following sub-directories for upload.w.o: el, fr, ru, it, de, uk, ja, id, he, fi, zh. In light of this, I'm planning to roll this CSP out across all of upload.w.o (T117618#11974306) with the goal of setting it to enforce sooner than later, perhaps even as early as this week.
2. @Reedy is investigating how we can limit the storage of Timeline SVGs in Swift. We initially thought we could just stop storing them entirely, but I believe we did find an RTL use-case for them. We'll need to investigate if there is some way to just allow these SVGs only or perhaps transition them to PNGs or some other media format with a better security posture.
3. In discussing this issue with @Rsilvola and @roan today, we will want to perform some auditing of existing SVG files within Swift. We haven't seen any evidence of these types of XSSes being exploited on Wikimedia wikis, but we should at least get some idea of our potential exposure via the analysis methods discussed above. @Ladsgroup - would you or someone else be able to assist @Reedy with these efforts?

@Reedy and I are negotiating in IRC.

In T427611#11967747, @Reedy wrote:

T427611-v3.patch2 KBDownload

Abusing UploadFromFile and a FauxRequest to do some fakery to get the logging.

On mediawiki.org when pasting in the example at https://www.mediawiki.org/wiki/Extension:EasyTimeline#Code_example and changing the comment to get an uncached result, I now get the following error:

Locally I can reproduce this too, with the following verification logs:

MediaWiki\Upload\UploadVerification::checkSvgScriptCallback: Found href attribute <a 'http://www.w3.org/1999/xlink:href'='/wiki/special:mylanguage/lenin' in uploaded file.
MediaWiki\Upload\UploadVerification::checkSvgScriptCallback: Found href attribute <a 'http://www.w3.org/1999/xlink:href'='special:mylanguage/stalin' in uploaded file.
MediaWiki\Upload\UploadVerification::checkSvgScriptCallback: Found href attribute <a 'http://www.w3.org/1999/xlink:href'='special:mylanguage/stalin' in uploaded file.
MediaWiki\Upload\UploadVerification::checkSvgScriptCallback: Found href attribute <a 'http://www.w3.org/1999/xlink:href'='/wiki/special:mylanguage/khrushchev' in uploaded file.
MediaWiki\Upload\UploadVerification::checkSvgScriptCallback: Found href attribute <a 'http://www.w3.org/1999/xlink:href'='/wiki/special:mylanguage/brezhnev' in uploaded file.
MediaWiki\Upload\UploadVerification::checkSvgScriptCallback: Found href attribute <a 'http://www.w3.org/1999/xlink:href'='/wiki/special:mylanguage/andropov' in uploaded file.
MediaWiki\Upload\UploadVerification::checkSvgScriptCallback: Found href attribute <a 'http://www.w3.org/1999/xlink:href'='/wiki/special%27colon%27mylanguage/chernenko' in uploaded file.

Seems like UploadVerification doesn't support relative URLs?

Seems like it indeed doesn't...

			// Do not allow relative links, or unsafe url schemas.
			// For <a> tags, only data:, http: and https: and same-document
			// fragment links are allowed.
			// For all other tags, only 'data:' and fragments (#) are allowed.
			if (
				$stripped === 'href'
				&& $value !== ''
				&& !str_starts_with( $value, 'data:' )
				&& !str_starts_with( $value, '#' )
				&& !( $strippedElement === 'a' && preg_match( '!^https?://!i', $value ) )
			) {

https://github.com/wikimedia/mediawiki/blob/deb8af8b9f14ad17537b41b1e3616a7852e02f83/includes/Upload/UploadVerification.php#L732-L741

Seems like UploadVerification doesn't support relative URLs?

A couple workaround ideas:

1. fork checkSvgScriptCallback somewhere else and remove the rel link validation - this seems like a bad approach
2. make the rel link check bypassable with a config variable or similar - e.g. && !( $strippedElement === 'a' && preg_match( '!^https?://!i', $value ) && $wgSVGValidateRelLinks ). Or a new function param that defaults to true may be cleaner and more targeted. - seems ok maybe?
3. use DOMDocument/xpath or svg-sanitize to write a more minimal SVG sanitization function - not as bad as option 1 IMO, but still not great as it creates two ways of doing the same thing, duping a problem MediaWiki already solved
4. just don't support relative links - would break certain legitimate usages, I'd guess

Any other ideas?

T428063: "Timeline error. Could not store output files" is probably fallout from this.

+			'name' => 'file.svg',
+			'type' => 'image/png',

def sus

In T427611#11982030, @Reedy wrote:

T427611-v4.patch2 KBDownload

So basically s/'type' => 'image/png',/'type' => 'image/svg',/? Do we want to try an updated deployment here? This makes sense to me but I don't have a great way to test it locally atm.

In T427611#11982128, @sbassett wrote:

In T427611#11982030, @Reedy wrote:

T427611-v4.patch2 KBDownload

So basically s/'type' => 'image/png',/'type' => 'image/svg',/? Do we want to try an updated deployment here? This makes sense to me but I don't have a great way to test it locally atm.

I think T428063 is caused by T427611#11974701, as the SVG sanitizer currently breaks all timelines with internal links (which are always relative in the SVG AFAIK), so we need an additional patch (e.g. to add an option to allow relative links in UploadVerification) to fix that task.

Would it make sense to handle this the same way fixMap() already handles image map hrefs, i.e. check xlink:href values against $wgUrlProtocols, convert relative wiki links to absolute URLs (distinguishing namespace prefixes from URI schemes via NamespaceInfo), and neutralize anything else? That way both the XSS (T427611) and the relative-link regression (T428063) would be fixed without needing to route through UploadBase::verifyUpload().

So basically borrow this code from from Timeline::fixMap and then run the processed SVG source through UploadBase::verifyUpload()? It looks like that code wants relative links to start with a / and some of the ones @SomeRandomDeveloper mentioned above do not. So we might need to optimize for that, and we also wouldn't want to allow .. etc. But this might get us what we need and be simpler than digging through all of the abstraction layers to allow for a less strict regular expression in UploadVerification::checkSvgScriptCallback.

One thing to keep in mind: the relative wiki links (like Special:MyLanguage/Foo) would need to be rewritten to absolute URLs before the $wgUrlProtocols check, otherwise fixMap()-style filtering would strip them too.

I didn't want to make a public connection but I'm deleting any svg file older than 2021: https://sal.toolforge.org/log/9Of_j54B8tZ8Ohr0U7L4 I will eventually move to a more recent time and then run a maint script to force purge all pages that are using the extension.

Note: people on discord are complaining easy timeline is broken. I dont know if its related to this or not.

In T427611#11986043, @Bawolff wrote:

Note: people on discord are complaining easy timeline is broken. I dont know if its related to this or not.

Thank you for notifying. It could be. is it fixed by just purging the page? if so, then it's because of the deletions (and I co go around and mass purge all pages in the tracking category). If it stays broken even after the purge, then it's something else.

In T427611#11986043, @Bawolff wrote:

Note: people on discord are complaining easy timeline is broken. I dont know if its related to this or not.

I'd say it very likely is as we know the relative link issue is currently causing errors.

Based upon the fixMap() suggestion above, I worked on something that I think can at least unbreak the current relative URL issue in Wikimedia production:

1. Keeps all of the same UploadFromFile verification changes from the previous patches, including the recent image/svg change in v4.
2. Introduces a basic SVG link converter in convertSVGToAbsoluteURLs() inspired by what fixMap() does. Some notes:
   1. I don't think we actually need to worry about $wgUrlProtocols (even though I left the code in there) as I don't think Timeline ever supported absolute URL links in generated SVGs. When testing locally with unpatched Timeline, things like text:[https://en.wikipedia.org/wiki/...|some_link] were already being sanitized in an albeit strange way, but sanitized none the less. And just turned into nonsense links in the ploticus-generated SVGs.
   2. As per the comment in the patch, the $dangerous_protocols stripping is not meant to be remotely foolproof, but instead intended to strip away some dangerous/unwanted parts of a URL as a basic URL-processing step.
   3. For now, most URL strings eventually get forced to an MW Title/local URL and then expanded via UrlUtils::expand to get the absolute URL. Some junk can obviously slip through here, but it should just end up as a link to a non-existent wiki page. Or, at worst, an exception thrown from $title->getLocalURL() if a user is trying to abuse .. etc, which we could make a bit more user-friendly, I suppose.

Also, some handy steps to build ploticus in mw-docker, within the mediawiki container:

$ docker compose exec -u root mediawiki bash
$ apt-get update && apt-get install -y ploticus perl fonts-freefont-ttf wget libx11-dev libpng-dev zlib1g-dev
$ cd /tmp
$ wget -O "pl.tar.gz" https://sourceforge.net/projects/ploticus/files/ploticus/2.42/ploticus242_src.tar.gz/download
$ tar -xvf pl.tar.gz
$ cd ploticus242/src/
$ make -f Makefile
$ mv pl /usr/local/bin/ploticus
$ chmod +x /usr/local/bin/ploticus

and then set $wgTimelinePloticusCommand = '/usr/local/bin/ploticus'; in LocalSettings.php

Seems like UploadVerification doesn't support relative URLs?

For context, this is because it is difficult to sanitize relative urls correctly and there is basically no valid use case for a relative url in an uploaded file.

Have you guys considered reverting the patch if it doesn't work? The issue itself is still secret after all.

A rather clean way to clean up old files would be to make the urls for timeline svgs be prefixed with a version number. So it'll be https://upload.wikimedia.org/wikipedia/en/timeline/v2-46e9y6gya1583ddr1q98xxe7uu2ahfd.svg instead. After a month, when all parser cache entries expire, then we can delete anything that doesn't start with v2-, later when we find a new issue and need to invalidate all the svgs, we can just bump the version. That being said, this feels too much like lipstick on a pig for an extension that doesn't even support unicode and better to be undeployed and replaced with something like charts or similar ASAP.

I'm open to ideas.

[subscribing because T428063 came to my swift pile, and I think this is the likely underlying issue]

In T427611#11986973, @sbassett wrote:

Based upon the fixMap() suggestion above, I worked on something that I think can at least unbreak the current relative URL issue in Wikimedia production:

T427611-v5.patch4 KBDownload

• I don't think we should try to strip javascript: URLs in the method if we're going to run the SVG through UploadVerification anyways afterwards, especially since the way this is done in the patch can easily be bypassed as the check is case-sensitive and (presumably, haven't tested this) does not work if there is whitespace in front of the URL. I think we should instead only try to fix code that is generated by normal usage of EasyTimeline and does not pass the SVG filters; otherwise we also don't have any logging if somebody tries to use javascript: URLs
• The continue; in the $wgUrlProtocols loop applies to the inner loop, so it does nothing; I think you can use continue 2; instead
• The $wgUrlProtocols checks should maybe be case-insensitive. See Skin::makeInternalOrExternalUrl() (https://github.com/wikimedia/mediawiki/blob/c6a5fb8c27904b9c575e6b6c1e042a0649ca3185/includes/Skin/Skin.php#L1171), it uses regex for this
• (nit) The new method should use a PHPDoc instead of a normal comment block; also it should be marked as @throws TimelineException because it's a checked exception
• When testing this with the example at https://www.mediawiki.org/wiki/Extension:EasyTimeline#Code_example I get Error: Call to a member function getLocalURL() on null because $currentUrl is wiki/Special:MyLanguage/Lenin (after going through ltrim()), which is not a valid Title text. Given that EasyTimeline sometimes already uses the article path, maybe we should try to expand the URL if it starts with a slash, and only otherwise fall back to trying to constructing a Title and getting the URL from it?

(haven't fully tested the patch but wanted to point out those issues already as the patch doesn't work properly in its current form)

In T427611#11987159, @Bawolff wrote:

Have you guys considered reverting the patch if it doesn't work? The issue itself is still secret after all.

I'd rather try to find some reasonable compromise to better secure Timeline SVGs from XSS even if we don't perfectly handle all relative or even malicious links. Especially before we get T117618 in place, which I'm hoping can be next week-ish.

In T427611#11988089, @SomeRandomDeveloper wrote:

...
(haven't fully tested the patch but wanted to point out those issues already as the patch doesn't work properly in its current form)

Thanks for the CR. If there's consensus that this is a reasonable (though imperfect) compromise approach, I'll clean these issues up and get a new patch posted soon.

In T427611#11988089, @SomeRandomDeveloper wrote:

• I don't think we should try to strip javascript: URLs in the method if we're going to run the SVG through UploadVerification anyways afterwards, especially since the way this is done in the patch can easily be bypassed as the check is case-sensitive and (presumably, haven't tested this) does not work if there is whitespace in front of the URL. I think we should instead only try to fix code that is generated by normal usage of EasyTimeline and does not pass the SVG filters; otherwise we also don't have any logging if somebody tries to use javascript: URLs

I've removed the trivial $dangerous_protocols checks, including for javascript: schemes, to just let UploadFromFile::verifyFile() Title::newFromText() handle anything potentially malicious.

• The continue; in the $wgUrlProtocols loop applies to the inner loop, so it does nothing; I think you can use continue 2; instead

Changed since the patch is now handling URL protocols similar to Skin::makeInternalOrExternalUrl() as you suggested below.

• The $wgUrlProtocols checks should maybe be case-insensitive. See Skin::makeInternalOrExternalUrl() (https://github.com/wikimedia/mediawiki/blob/c6a5fb8c27904b9c575e6b6c1e042a0649ca3185/includes/Skin/Skin.php#L1171), it uses regex for this

The v6 patch now uses this approach.

• (nit) The new method should use a PHPDoc instead of a normal comment block; also it should be marked as @throws TimelineException because it's a checked exception

Done.

• When testing this with the example at https://www.mediawiki.org/wiki/Extension:EasyTimeline#Code_example I get Error: Call to a member function getLocalURL() on null because $currentUrl is wiki/Special:MyLanguage/Lenin (after going through ltrim()), which is not a valid Title text. Given that EasyTimeline sometimes already uses the article path, maybe we should try to expand the URL if it starts with a slash, and only otherwise fall back to trying to constructing a Title and getting the URL from it?

The v6 patch above works for me with this example now, and the bad example in the task description, and many other conditions that I tested locally but: this is still brittle code IMO. I had to handle some odd Timeline behaviors (see lines 122 - 129 in the patch). Timeline likes to throw random forward-slashes and $wgArticlePaths in front of various relative URLs. And then it does odd things in certain situations, particularly after modifiers like fontsize:XS, where it performs "colon-escaping". I still think trying to coerce various strings to Title objects makes sense, though that sanitizes things like javascript:... as bad page titles in absolute URLs as opposed to catching and throwing an exception based upon verifyUpload()'s output. But as was implied, it's not feasible to check every possible way javascript: and other dangerous schemes could be bypassed by naive href filtering. This code also defaults to an empty URL if it cannot create a valid title object e.g. if someone tries ../A_Page_Title and similar, which would throw an exception in getCanonicalUrl, parseTitle, etc. I also noticed that for the PNGs that actually get used (and the resultant image maps that are created via the current Timeline code), ../A_Page_Title does get through, which isn't great, but seems to have always been the case.

This patch is the same as -v6 above but introduces some tests for convertSVGToAbsoluteURLs. Also open to other approaches, but I'd really like to unbreak the relative URL issue in production sooner than later.

While reviewing your patch, I noticed that $articlepath in EasyTimeline.pl defaults to a full URL and that gave me an idea for an easier solution, at least for wiki links:

diff --git a/includes/Timeline.php b/includes/Timeline.php
index 4ac2673..5e6fc33 100644
--- a/includes/Timeline.php
+++ b/includes/Timeline.php
@@ -173,7 +173,7 @@ class Timeline implements ParserFirstCallInitHook {
 			->outputFileToString( 'file.err' )
 			->includeStderr()
 			->environment( [
-				'ET_ARTICLEPATH' => $wgArticlePath,
+				'ET_ARTICLEPATH' => MediaWikiServices::getInstance()->getUrlUtils()->expand( $wgArticlePath ),
 				'ET_FONTFILE' => $options['font']['file'],
 				'ET_PERL' => $wgTimelinePerlCommand,
 				'ET_PLOTICUS' => $wgTimelinePloticusCommand,

This will cause EasyTimeline to always emit full URLs for wiki links.

I then tried a few examples from the parent task and noticed that links like link:PageName still cause an error. This can be fixed by always prefixing links with the article path if they're not http(s) links (which are the only protocols supported by UploadVerification):

diff --git a/scripts/EasyTimeline.pl b/scripts/EasyTimeline.pl
index 363f987..d33aba1 100755
--- a/scripts/EasyTimeline.pl
+++ b/scripts/EasyTimeline.pl
@@ -5119,6 +5119,11 @@ sub NormalizeURL {
     $url =~ s/(https?)\:?\/?\/?/$1:\/\//
         ;    # add possibly missing special characters
     $url =~ s/ /%20/g;
+    if ( $url !~ /^https?:\/\// ) {
+        my $newurl = $articlepath;
+        $newurl =~ s/\$1/$url/;
+        $url = $newurl;
+    }
     return ($url);
 }

This breaks relative URLs like text:[/Test], but I'm not sure how commonly they're used in timelines, and I think this is fine temporarily as the majority of timelines will be working properly again, and the patch is way smaller and easier to review than the PHP-only version.
We could probably prefix relative URLs with $wgServer but it's late for me right now so I don't have the time to do that.

Amended to the v4 patch:

This breaks relative URLs like text:[/Test], but I'm not sure how commonly they're used in timelines,

If this turns out to be important, one potential solution is to prefix with ./ instead (This is how parsoid solved this issue). However, I suspect that use case is extremely rare, and prefixing with the actual article path like you did seems more proper to me.

Actually, maybe it would be better to only expand URLs in SVGs for now:

(I noticed a bug with some links like the ones in https://web.archive.org/web/20241222050514/https://de.wikipedia.org/wiki/Vorlage:Zeitleiste_Ligazugeh%C3%B6rigkeit_FC_Ingolstadt_04 which are not present in the image map HTML because they don't start with a / (map source) and are therefore sanitized by fixMap(), and the previous version of my security patch would have fixed that by prepending the article path to those URLs as well, but I think it's better to make changes like this in a public gerrit patch after the security patch is made public)

In T427611#12012955, @SomeRandomDeveloper wrote:

Amended to the v4 patch:

T427611-v5-alternative.patch3 KBDownload

This is definitely cleaner and does partially unbreak the relative URLs issue. Some notes from testing locally, primarily with the mediawiki.org example:

1. "Normal" text links like text:Vladimir~Ilyich~[[Special:MyLanguage/Lenin|Lenin]] seem to work fine with the new v5 patch and are processed as expected.
2. But the next line - text:[[Special:MyLanguage/Stalin|Josef~Stalin]] - actually gets caught by the upload verification. This appears to be due to the tilde in Josef~Stalin. When I remove it, the verification issue goes away. I'm guessing this is from one the many cryptic code paths Timeline.pl takes when processing ploticus text data to URLs. But this example - as published on mediawiki.org - fails for me locally.
3. I also noticed that text:Konstantin~[[Special:MyLanguage/Chernenko|Chernenko]] fontsize:XS gets mangled, again I'm guessing due to the extra fontsize:XS directive having a colon in it. It gets processed as http://localhost:8081/wiki/Special%27colon%27MyLanguage/Chernenko for me. But this is clearly due to a long-existing bug in Timeline.
4. text:Mikhail~[[../Special:MyLanguage/Gorbachev|Gorbachev]] does get processed with the directory traversal, so it becomes http://localhost:8081/Special:MyLanguage/Gorbachev. Again, long-standing bug within Timeline, I'm sure.

But the important pieces are there: non-forward-slash-prefixed relative URLs get processed to the correct canonical URLs and the upload verification catches other stuff, particularly javascript:... URLs.

In T427611#12014845, @sbassett wrote:

1. "Normal" text links like text:Vladimir~Ilyich~[[Special:MyLanguage/Lenin|Lenin]] seem to work fine with the new v5 patch and are processed as expected.
2. But the next line - text:[[Special:MyLanguage/Stalin|Josef~Stalin]] - actually gets caught by the upload verification. This appears to be due to the tilde in Josef~Stalin. When I remove it, the verification issue goes away. I'm guessing this is from one the many cryptic code paths Timeline.pl takes when processing ploticus text data to URLs. But this example - as published on mediawiki.org - fails for me locally.

That's weird, are you sure it's not a cached result? I copied the entire example from mediawiki.org and tried it with my latest patch (while adding a few random characters to the comment in the first line to bypass the cache) and it works fine (and if it works with my latest patch I would also expect it to work with the one before which I assume was the one you tested).

Oh, if UploadVerification fails, it won't get cached, so that can't be the issue here...

In T427611#12014900, @SomeRandomDeveloper wrote:

Oh, if UploadVerification fails, it won't get cached, so that can't be the issue here...

Hmm, with the latest patch, I'm not getting the Stalin issue to trigger anymore, so maybe that was just a testing issue on my part. It seems to generate the correct, canonical URL for the SVG but not for the image map for the PNG. It just doesn't create a link/URL at all for me in the image map. I can still replicate the other issues though. Again, this stuff has likely been an issue in Timeline for quite a while. It would be great if that extension eventually gets a proper maintainer, or its functionality migrated into ext:Chart or something.

FWIW, I tested v5-alternative-2 on a local MW 1.47 instance and both SVG and MAP links generated correctly as absolute URLs for me.

In T427611#12014941, @sbassett wrote:

In T427611#12014900, @SomeRandomDeveloper wrote:

Oh, if UploadVerification fails, it won't get cached, so that can't be the issue here...

Hmm, with the latest patch, I'm not getting the Stalin issue to trigger anymore, so maybe that was just a testing issue on my part. It seems to generate the correct, canonical URL for the SVG but not for the image map for the PNG. It just doesn't create a link/URL at all for me in the image map.

That's also visible at https://www.mediawiki.org/wiki/Extension:EasyTimeline#Code_example:

Again, this stuff has likely been an issue in Timeline for quite a while. It would be great if that extension eventually gets a proper maintainer, or its functionality migrated into ext:Chart or something.

Agreed, especially given T426631#11938015 and the fact that Ploticus was last updated 12 years ago (T338098), I personally would prefer a replacement... though given the complexity of EasyTimeline + ploticus, it would probably take some time to actually implement a proper replacement and then replace all on-wiki usages with it

In T427611#12014960, @HakanIST wrote:

FWIW, I tested v5-alternative-2 on a local MW 1.47 instance and both SVG and MAP links generated correctly as absolute URLs for me.

Interesting. I'm testing on a fairly recent version of MW core master (just updated it actually), recent ext:timline w/ security patches from T426631 applied as well and ploticus 2.42-May2013. I get the strange image map and SVG linking issues I mentioned above for the Stalin text in the mediawiki.org example every time.

In T427611#12014972, @SomeRandomDeveloper wrote:

That's also visible at https://www.mediawiki.org/wiki/Extension:EasyTimeline#Code_example:

Which is likely due to the href in the .map file being Special:MyLanguage/Stalin for some reason.

Ah, ok.

Still, the v5 patch from T427611#12014840 seems to address most relative link issues while still maintaining the security mitigations that I think we actually care about. So I'm fine with deploying that during next Monday's (2026-06-15) security window.

In T427611#12015397, @sbassett wrote:

In T427611#12014960, @HakanIST wrote:

FWIW, I tested v5-alternative-2 on a local MW 1.47 instance and both SVG and MAP links generated correctly as absolute URLs for me.

Interesting. I'm testing on a fairly recent version of MW core master (just updated it actually), recent ext:timline w/ security patches from T426631 applied as well and ploticus 2.42-May2013. I get the strange image map and SVG linking issues I mentioned above for the Stalin text in the mediawiki.org example every time.

Fair point, I had tested without T426631 patches. With both applied I can now reproduce the relative MAP links. Looks like NormalizeURL (used for MAP) doesn't prefix $articlepath, only NormalizeURLForSVG does.

Just FYI - I plan to deploy @SomeRandomDeveloper's updated patch from T427611#12014840 during today's security deployment window unless there are any objections.

Deployed the updated fix from T427611#12014840: https://sal.toolforge.org/log/24VRzZ4BffdvpiTrEgvG. The mediawiki.org example now works for me here: https://test.wikipedia.org/wiki/User:SBassett_(WMF)/Sandbox/Test, so the relative links issue is definitely partially-unbroken now.

In T427611#12021620, @sbassett wrote:

[...] so the relative links issue is definitely partially-unbroken now.

By partially, are you referring to the existing problems with relative links that were already present in the extension before any patches were deployed, or is there still something that's broken right now because of the security patch?

In T427611#12021642, @SomeRandomDeveloper wrote:

By partially, are you referring to the existing problems with relative links that were already present in the extension before any patches were deployed, or is there still something that's broken right now because of the security patch?

Mostly the former, and that /URLs do not work.

While testing the patch on a new local environment (to backport it to 1.45), I noticed that $wgFileExtensions[] = 'svg'; is now required for EasyTimeline to work, as otherwise UploadVerification returns an error when uploading the SVG. This will need to be documented at https://www.mediawiki.org/wiki/Extension:EasyTimeline/installation once the patch is public, unless there's a workaround to bypass this check when Timeline uploads its files.

As reported in T428063#12032091, there is still a broken timeline on ruwiki. Minimal PoC:

{{#tag:timeline|
ImageSize = width:1000 height:250
PlotArea = top:20 bottom:65 right:50 left:20

DateFormat = dd/mm/yyyy
Period = from:01/01/1970 till:01/01/2028
TimeAxis = orientation:horizontal

TextData =
  pos:(10,25)
  text: "х"
}}

Replacing the character х (CYRILLIC SMALL LETTER HA (U+0445); not the same as x) with any other character causes the timeline to render. I can reproduce this on enwiki as well, but not locally.

Unfortunately, the timeline log channel is not configured in wmgMonologChannels, so I assume we don't store the log entries (which use the info level) that are generated by the security patch if UploadVerification fails...

Maybe we could add 'timeline' => 'info' to $wmgMonologChannels in a public config patch (the log channel is already used in the existing extension code, and I don't think this would give away any information about this task), and then somebody with access could check the logs?

In T427611#12032127, @SomeRandomDeveloper wrote:

Maybe we could add 'timeline' => 'info' to $wmgMonologChannels in a public config patch (the log channel is already used in the existing extension code, and I don't think this would give away any information about this task), and then somebody with access could check the logs?

T429654

As a more permanent solution for the issues reported in T428063, if UploadVerification fails, maybe we could only skip storing the SVG file for now, log it but still store and display the timeline properly?

Probably either the svg is not marked as utf-8, or the cyrillic character is not being encoded properly.

Change #1306185 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/timeline@master] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306185

Change #1306194 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/timeline@REL1_46] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306194

Change #1306197 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/timeline@REL1_45] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306197

Change #1306200 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/timeline@REL1_44] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306200

Change #1306185 merged by jenkins-bot:

[mediawiki/extensions/timeline@master] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306185

Change #1306203 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/timeline@REL1_43] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306203

Change #1306194 merged by jenkins-bot:

[mediawiki/extensions/timeline@REL1_46] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306194

Change #1306200 merged by jenkins-bot:

[mediawiki/extensions/timeline@REL1_44] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306200

Change #1306197 merged by jenkins-bot:

[mediawiki/extensions/timeline@REL1_45] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306197

Change #1306203 merged by jenkins-bot:

[mediawiki/extensions/timeline@REL1_43] SECURITY: Run timeline generated SVG through MW upload checks

https://gerrit.wikimedia.org/r/1306203