Page MenuHomePhabricator
Create Task
Maniphest T346629

GrowthExperiments CI fails on master: mwgate-node16-docker
Closed, ResolvedPublic
Actions

Description

mwgate-node16-docker started to fail on GrowthExperiments's master branch with the following:

13:54:32 /src/i18n/mentorship/he.json
13:54:32   216:100  warning  Detected potential trojan source attack with unicode bidi introduced in this code: '"שינוי אפשרויות החונכות של $1‏: $2"'  security/detect-bidi-characters

The file was last changed with e0211b6f6 by the TWN bot. Since the TWN commit changes line 216, which is flagged by the failing check, I think the CI error was added in that commit, which passed CI, because it doesn't run the full set of jobs. Tagging Continuous-Integration-Config to help with finding a solution regarding the TWN commits (should we add mwgate-node16-docker there, which is a cheap job?) and translatewiki.net for awareness.

Details

ProjectBranchLines +/-Subject
mediawiki/extensions/GrowthExperimentswmf/1.41.0-wmf.27+10 -10build: Update eslint-config-wikimedia to 0.25.1
mediawiki/extensions/GrowthExperimentsmaster+10 -10build: Update eslint-config-wikimedia to 0.25.1
mediawiki/extensions/GrowthExperimentsmaster+0 -1Revert l10n update that breaks CI
Customize query in gerrit

Related Objects

Event Timeline

Urbanecm_WMF created this task.Mon, Sep 18, 12:05 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMon, Sep 18, 12:05 PM
gerritbot added a comment.Mon, Sep 18, 12:08 PM

Change 958466 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] Revert l10n update that breaks CI

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

gerritbot added a project: Patch-For-Review.Mon, Sep 18, 12:08 PM
Urbanecm_WMF triaged this task as High priority.Mon, Sep 18, 12:08 PM

This breaks merges to GrowthExperiments.

Urbanecm_WMF added a subscriber: Amire80.Mon, Sep 18, 12:11 PM

https://translatewiki.net/w/i.php?title=MediaWiki:Growthexperiments-manage-mentors-summary-change-admin-with-reason/he would probably need to be updated to remove whatever characters are considered "trojan source attack" by our CI (or deleted until that can be done). My patch uploaded above removes the offending entry from GrowthExperiments codebase.

CC @Amire80, who created the translation and who might know what's happening here :).

Urbanecm_WMF added projects: GrowthExperiments-Mentorship, Growth-Team (Current Sprint).Mon, Sep 18, 12:11 PM
Urbanecm_WMF added a comment.Mon, Sep 18, 12:13 PM

See https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/958407 (DNM CI testing patch).

gerritbot added a comment.Mon, Sep 18, 12:13 PM

Change 958466 abandoned by Urbanecm:

[mediawiki/extensions/GrowthExperiments@master] Revert l10n update that breaks CI

Reason:

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

gerritbot added a comment.Mon, Sep 18, 12:14 PM

Change 958466 restored by Urbanecm:

[mediawiki/extensions/GrowthExperiments@master] Revert l10n update that breaks CI

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

Maintenance_bot moved this task from New Tasks to In Progress on the GrowthExperiments-Mentorship board.Mon, Sep 18, 12:15 PM
Amire80 added a comment.Mon, Sep 18, 2:51 PM

That message has an invisible RLM character, and it should have it. I remember discussing it in some other task, but I am failing to find it; perhaps it is hidden for security reasons. But basically, our security experts said that it's OK to have RLM and other bidi control characters in messages, and that it shouldn't fail CI. If anyone disagrees with that, I can think of other ways to resolve the bidi issues, but using this control characters is a simple and efficient solution.

Nikerabbit moved this task from Backlog to External on the translatewiki.net board.Mon, Sep 18, 3:11 PM
Nikerabbit added a subscriber: Nikerabbit.
gerritbot added a comment.Tue, Sep 19, 12:56 AM

Change 958589 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] build: Update eslint-config-wikimedia to 0.25.1

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

Urbanecm_WMF added a comment.Tue, Sep 19, 12:58 AM
In T346629#9174994, @Amire80 wrote:

That message has an invisible RLM character, and it should have it. I remember discussing it in some other task, but I am failing to find it; perhaps it is hidden for security reasons.

Thanks for pointing out this was already discussed elsewhere. I found T338610: CheckUser: Detected potential trojan source attack with unicode bidi introduced in this code, which seems to be a recent issue in CheckUser for the same thing.

But basically, our security experts said that it's OK to have RLM and other bidi control characters in messages, and that it shouldn't fail CI. If anyone disagrees with that, I can think of other ways to resolve the bidi issues, but using this control characters is a simple and efficient solution.

No need to, this is fine. Thanks for pointing me towards the other ticket -- searching for "RLM character" did the trick :). Uploaded an actual fix!

gerritbot added a comment.Tue, Sep 19, 12:59 AM

Change 958466 abandoned by Urbanecm:

[mediawiki/extensions/GrowthExperiments@master] Revert l10n update that breaks CI

Reason:

in favour of https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/958589

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

Urbanecm_WMF removed a project: Continuous-Integration-Config.Tue, Sep 19, 1:01 AM
gerritbot added a comment.Tue, Sep 19, 11:19 AM

Change 958589 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] build: Update eslint-config-wikimedia to 0.25.1

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

Urbanecm_WMF closed this task as Resolved.Tue, Sep 19, 12:05 PM
Urbanecm_WMF claimed this task.

CI works now, closing.

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.28; 2023-09-26).Tue, Sep 19, 1:02 PM
hashar added a subscriber: hashar.Tue, Sep 19, 1:58 PM

Well done! :)

gerritbot added a comment.Tue, Sep 19, 9:14 PM

Change 959014 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@wmf/1.41.0-wmf.27] build: Update eslint-config-wikimedia to 0.25.1

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

gerritbot added a comment.Wed, Sep 20, 1:18 PM

Change 959014 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.41.0-wmf.27] build: Update eslint-config-wikimedia to 0.25.1

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

Stashbot added a comment.Wed, Sep 20, 1:21 PM

Mentioned in SAL (#wikimedia-operations) [2023-09-20T13:21:48Z] <urbanecm@deploy2002> Started scap: Backport for [[gerrit:959014|build: Update eslint-config-wikimedia to 0.25.1 (T346629)]], [[gerrit:959007|Change CSS selector for Minerva mobile menu icon (T346459)]]

Stashbot mentioned this in T346459: Mobile main menu icon missing when Growth home page enabled.Wed, Sep 20, 1:21 PM
Stashbot added a comment.Wed, Sep 20, 1:42 PM

Mentioned in SAL (#wikimedia-operations) [2023-09-20T13:42:17Z] <urbanecm@deploy2002> urbanecm and jdlrobson: Backport for [[gerrit:959014|build: Update eslint-config-wikimedia to 0.25.1 (T346629)]], [[gerrit:959007|Change CSS selector for Minerva mobile menu icon (T346459)]] synced to the testservers mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug2002.codfw.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experimental XWD option)

Stashbot added a comment.Wed, Sep 20, 1:56 PM

Mentioned in SAL (#wikimedia-operations) [2023-09-20T13:56:12Z] <urbanecm@deploy2002> Finished scap: Backport for [[gerrit:959014|build: Update eslint-config-wikimedia to 0.25.1 (T346629)]], [[gerrit:959007|Change CSS selector for Minerva mobile menu icon (T346459)]] (duration: 34m 21s)

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.27; 2023-09-19); removed MW-1.41-notes (1.41.0-wmf.28; 2023-09-26).Wed, Sep 20, 2:00 PM
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL