sailfishos-chum / sailfishos-chum-gui

GUI application for utilising the SailfishOS:Chum community repository
https://openrepos.net/content/olf/sailfishoschum-gui-installer
MIT License
13 stars 17 forks source link

Move this command out from translation #173

Closed martonmiklos closed 9 months ago

martonmiklos commented 1 year ago

Hi folks,

I came across this string during translation: https://github.com/sailfishos-chum/sailfishos-chum-gui/blob/5d0ce0aa0d7b1c80f88a216d367cd6676aa3c839/src/chum.cpp#L334

I would recommend to make the command untranslatable by adding a placeholder to it. For security reasons and to ease the translations. What do you think?

Olf0 commented 1 year ago

Yes, I also wondered once, if there is better way to handle this code line embedded in a text string.

For security reasons and to ease the translations.

Can you please explain this a bit, specifically the security aspect. I perceived it just as a small nuisance ("a bit ugly"), hence I did not mind to research and address this.

I would recommend to make the command untranslatable by adding a placeholder to it.

I know there is a notranslate tag, but have no idea, if this is the right thing to use and how.

As always, PRs are welcome, then I might also understand better what solution you think of and how that looks like in practice. Side note: I am merely little experienced with Qt's l10n infrastructure.

martonmiklos commented 1 year ago

Can you please explain this a bit, specifically the security aspect.

Well if I 'mis-translate' this string to end with an rm -rf / there could be an issue if someone copies it over and execute it.

I know there is a notranslate tag, but have no idea, if this is the right thing to use and how.

I am not an expert on the localization, but I would use the qsTr somehow like this:

qsTr("The SailfishOS:Chum GUI application failed to manage the SailfishOS:Chum repository! "
                    "You probably have multiple SailfishOS:Chum repositories defined for SSU or disabled a "
                    "SailfishOS:Chum repository.\n\n"
                    "Please remove all SailfishOS:Chum repositories by executing this command line as root user:\n"
                    "%s\n"
                    "This SailfishOS:Chum GUI application will add any missing SailfishOS:Chum repository when started again.").arg("for i in $(ssu lr | fgrep chum | cut -f 3 -d ' '); do ssu rr $i; done");

I do no have SFOS SDK at hand at the moment, but I will submit a PR once get back to my dev box.

Olf0 commented 1 year ago

Can you please explain this a bit, specifically the security aspect.

Well, if one "mis-translates" this string to end with an rm -rf / there could be an issue if someone copies it over and executes it.

So this is about safety (not security), nevertheless the reasoning is valid. :+1:

Olf0 commented 10 months ago

@martonmiklos, as your idea for rendering the shell code as immutable for translators is not ID-based, your implementation suggestion has to be transformed to suite ID-based, translatable Qt strings, first.

Olf0 commented 10 months ago

Should be fixed by PR #215.

Olf0 commented 10 months ago

PR #215 seems to work fine, but by looking at this text I realise to have never seen it before. Thus I played bit with it and lastly created PR #253.

Before

Screenshot_20231110-1-v Screenshot_20231110-2-v

Screenshot_20231110-2-h Screenshot_20231110-1-h

After

<Screenshots are missing, as one wold have to misconfigure SSU's SailfishOS:Chum repository entry for that. If someone wants to provide corresponding screenshots, please post them here.>