The client code for this service can be seen at https://github.com/wikimedia/mediawiki-services-cxserver/tree/master/mt

While reviewing this, similar code: https://phabricator.wikimedia.org/T90208 should also be reviewed.

In T143185#2590013, @KartikMistry wrote:

While reviewing this, similar code: https://phabricator.wikimedia.org/T90208 should also be reviewed.

Note: ^^This isn't priority. Youdao should be done first.

In T143185#2590027, @KartikMistry wrote:

In T143185#2590013, @KartikMistry wrote:

While reviewing this, similar code: https://phabricator.wikimedia.org/T90208 should also be reviewed.

Note: ^^This isn't priority. Youdao should be done first.

@KartikMistry, would you mind creating a separate phab ticket for this one? Thanks!

@dpatrick Done at, https://phabricator.wikimedia.org/T144467

Note that the MT clients has a common pattern. Mostly the API url and key changes. Sometimes the error handling too. And it is potentially possible that more of this kind of clients will be added.

Security review:

• If the source wikitext has something like [[Javascript:foo()]], then the translated version will have an a tag with a href="Javascript:foo()". Its unclear if this is exploitable, as click events don't actually navigate you to the link, but its definitely not a good thing. [Note: I did not review that part of the app in general, I was primarily only looking at the Youdao part]
• I don't understand the security goals of the JWT-token in the Authorization header. There should be a code comment somewhere explaining what threat it is meant to stop. I'm concerned that it may not be accomplishing what you want it to, but I'm not really sure, as I'm not sure what the intention is.

Otherwise I think this is fine.

The purpose of the JWT-token is two-fold:

• To reliably identify the requesting user for monitoring abuse and rate-limiting (not currently implemented)
• To make it more difficult to use the service outside CX extension. Clearly, leaving it all open is not a good idea, and clearly, we cannot completely prevent translation of arbitrary content because people could save mostly arbitrary content on their user page on a wiki which is not actively monitored.

If the source wikitext has something like [[Javascript:foo()]], then the translated version will have an a tag with a href="Javascript:foo()". Its unclear if this is exploitable, as click events don't actually navigate you to the link, but its definitely not a good thing. [Note: I did not review that part of the app in general, I was primarily only looking at the Youdao part]

The input to MT engines is not Wikitext. Infact, cxserver does not know about wikitext at all. All the data it accept as input and output is HTML. The mediawiki extension Content translation also works with HTML. The only place it produce wikitext is at the end of workflow, when translator decide to publish an article to a wiki

In the case of Youdao, this MT engine is not capable of translating HTML. So the translateHtml method passes the input HTML to an HTML to plain text conversion module and then to translateText method. So what goes to MT engine hosted outside is this plain text(not wikitext, just plain text) and we get translation also in plain text.

In T143185#2635711, @santhosh wrote:

If the source wikitext has something like [[Javascript:foo()]], then the translated version will have an a tag with a href="Javascript:foo()". Its unclear if this is exploitable, as click events don't actually navigate you to the link, but its definitely not a good thing. [Note: I did not review that part of the app in general, I was primarily only looking at the Youdao part]

That issue wasnt related to Youado (it was discovered when i was testing the extension on wikipedia with languages fr and en just to get a sense how the various pieces fit together). Content translation seems to be taking safe parsoid html and outputting slightly unsafe html in the preview window.

In T143185#2632835, @Nikerabbit wrote:

The purpose of the JWT-token is two-fold:

• To reliably identify the requesting user for monitoring abuse nd rate-limiting (not currently implemented
• To make it more difficult to use the service outside CX extension. Clearly, leaving it all open is not a good idea, and clearly, we cannot completely prevent translation of arbitrary content because people could save mostly arbitrary content on their user page on a wiki which is not actively monitored.

Ok. Just as long as you're aware its mostly trivial to get around (since any non-globally blocked user can get a token and anyone can create an account). If the goal is to deter third party users, the cxserver auto-generated help pages should probably say on them that you are not allowed to use it externally.

Do you mean this page: https://cxserver.wikimedia.org/v1/ ?

For me it's nearly impossible to find anything on cxserver.wikimedia.org unless you know the address already. I doubt it has much readers.

@dpatrick @Bawolff Is there anything else needed from Language team on this review? Please let us know.

In T143185#2656681, @KartikMistry wrote:

@dpatrick @Bawolff Is there anything else needed from Language team on this review? Please let us know.

Sorry I wasn't clear before (And sorry I wasn't more responsive). The review should is considered "Done" generally, you can go ahead and deploy Youdao MT thing.

The bug is still open because I have concerns about how internal links starting with javascript: are transformed during "preview" of the autotranslated text. However that's unrelated to Youdao, so that does not block you deploying it.

In T143185#2651471, @Nikerabbit wrote:

Do you mean this page: https://cxserver.wikimedia.org/v1/ ?

For me it's nearly impossible to find anything on cxserver.wikimedia.org unless you know the address already. I doubt it has much readers.

Yeah. Mostly because I suspect legal/political obstacles are more likely to stop someone then the token, which is trivial to figure out once you watch things in firebug. (The v1 thing follows a standard pattern, so I suspect people will actually read it) However, this isn't a real security issue, so if you think things are fine the way things are, then that's ok too.

In T143185#2635711, @santhosh wrote:

If the source wikitext has something like [[Javascript:foo()]], then the translated version will have an a tag with a href="Javascript:foo()". Its unclear if this is exploitable, as click events don't actually navigate you to the link, but its definitely not a good thing. [Note: I did not review that part of the app in general, I was primarily only looking at the Youdao part]

The input to MT engines is not Wikitext. Infact, cxserver does not know about wikitext at all. All the data it accept as input and output is HTML. The mediawiki extension Content translation also works with HTML. The only place it produce wikitext is at the end of workflow, when translator decide to publish an article to a wiki

Sorry I wasn't clear before. To clarify, I mean how cxserver translates parsoid html -> translated html (In general, not Youdao specific). It starts with html in the left-hand column like

<a class="cx-link cx-source-link new" data-linkid="7" href="//en.wikipedia.org/wiki/Javascript:alert(1)" id="mwBA" rel="mw:WikiLink" title="Javascript:alert(1)">

and after you tell it to machine translate, the html in the right-hand column looks like:

<a class="cx-link cx-target-link-unadapted cx-target-link" data-linkid="7" href="Javascript:alert(1)" id="mwBA" rel="mw:WikiLink" title="Javascript:alert(1)">

Note, how before the href attribute is a full url, and then later it becomes a relative url. The relative url isn't safe by itself when a page name contains a colon, as the browser interprets it as a protocol. This is probably a minor issue, as click events are taken over, but nonetheless this is concerning. One possible fix would be prefixing local wiki pages with a /wiki/, so that its href="/wiki/javascript:alert(1)". I'm also concerned a similar thing is possible with the insert external link dialog