feat(`validate_locale_configuration`): discard and warn on configured but unsupported locale #7443

cfm · 2025-02-20T00:19:24Z

Status

Ready for review

Description of Changes

Closes #7442: If validate_locale_configuration() doesn't check the authoritative list of supported locales in the i18n.json file installed by securedrop-app-code, then a language for which support has been revoked won't be disabled until config.SUPPORTED_LOCALES is updated on the next securedrop-admin {sdconfig,install} run.

This isn't a bug; it was simply out of scope of #6557. Now #7442 motivates us to at least consider: Do we want an unsupported language to be disabled immediately on upgrade of securedrop-app-code, not just on the next securedrop-admin {sdconfig,install}? If yes, then this diff handles this case. If not, then we can just close this pull request.

Testing

Under make dev with:

--- a/securedrop/bin/dev-config
+++ b/securedrop/bin/dev-config
@@ -26,7 +26,7 @@ with open('securedrop/config.py', 'w') as f:
     text = env.get_template("securedrop/config.py.example").render(ctx)
     text += '\n'
     supported_locales = subprocess.check_output(['make', '--quiet', 'supported-locales']).decode().strip()
-    text += f'SUPPORTED_LOCALES = {supported_locales}\n'
+    text += f'SUPPORTED_LOCALES = {"de_DE", "ro", "en_US"}\n'
     f.write(text)
 
 with open('securedrop/rq_config.py', 'w') as f:
--- a/securedrop/i18n.json
+++ b/securedrop/i18n.json
@@ -63,10 +63,6 @@
       "name": "Portuguese, Portugal",
       "desktop": "pt_PT"
     },
-    "ro": {
-      "name": "Romanian",
-      "desktop": "ro"
-    },
     "ru": {
       "name": "Russian",
       "desktop": "ru"

Observe on startup:

[2025-02-19 23:51:46,846] ERROR in i18n: Configured locales {Locale('ro')} are not in the set of usable locales {Locale('de', territory='DE'), Locale('en', territory='US')}

Romanian is not offered in the locale switcher
/?l=ro defaults to en_US

Deployment

As above:

Do we want an unsupported language to be disabled immediately on upgrade of securedrop-app-code, not just on the next securedrop-admin {sdconfig,install}?
Do we want an error to be logged and an OSSEC alert to be sent in this case?

Checklist

If you made changes to the server application code:

Linting (make lint) and tests (make test) pass in the development container

NB. This is already covered at the implementation (as opposed to configuration) level by test_valid_but_unusable_locales().

If you made changes to `securedrop-admin`:

Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

Configuration tests pass

If you added or removed a file deployed with the application:

I have updated AppArmor rules to include the change

If you made non-trivial code changes:

I have written a test plan and validated it for this PR

Choose one of the following:

I have opened a PR in the docs repo for these changes, or will do so later
I would appreciate help with the documentation
These changes do not require documentation

…ut unsupported locale If validate_locale_configuration() doesn't check the authoritative list of supported locales in the "i18n.json" file installed by securedrop-app-code, then a language for which support has been revoked won't be disabled until config.SUPPORTED_LOCALES is updated on the next "securedrop-admin {sdconfig,install}" run.

…under test

cfm · 2025-02-20T01:46:42Z

Noting for the record (from way back in 9c11950): Configured locales [...] are not in the set of usable locales [...] is probably a warning, not an error, but the decision then was that we did want this case to trigger an OSSEC alert.

cfm · 2025-02-20T02:13:36Z

Clarified out of band with @zenmonkeykstop: We can just close this pull request if we don't care about this disablement case—or defer to v2.13.

legoktm · 2025-02-20T15:40:50Z

IMO the ideal behavior would be:

any language we disable, is disabled on all SDs that have it explicitly enabled.
if you're using ./securedrop-admin sdconfig/install, an unsupported language is an error
the existing server configuration is not an OSSEC alert, and if/when Hindi and Romanian are supported again, those servers will automatically re-enable the language without any manual interaction needed.

Would that be reasonable?

Also, I'm not sure where in your plan it was, but can we also disable hi/ro before the rc is cut so that behavior can be QA'd at the same time?

legoktm · 2025-02-20T15:41:27Z

securedrop/i18n.py

@@ -29,6 +30,8 @@
 from flask_babel import Babel
 from sdconfig import FALLBACK_LOCALE, SecureDropConfig

+I18N_CONF = "i18n.json"


Let's use an absolute path please, like Path(__file__).parent / "i18n.json"

legoktm · 2025-02-20T15:42:17Z

securedrop/i18n.py

+        with open(I18N_CONF) as i18n_conf_file:
+            i18n_conf = json.load(i18n_conf_file)
+        supported = parse_locale_set(i18n_conf["supported_locales"].keys())
+    except FileNotFoundError:


In what case would the file not be found? If i18n.json is missing, that feels like a fatal error to me.

nathandyer · 2025-02-20T16:11:44Z

I agree with @legoktm's ideal scenario above, and disabling a removed language on update rather than running ./securedrop-admin feels like the most straightforward / discoverable path. By which I mean, if I were an admin and the language suddenly reverted to a different language, I would be able to see there was an upgrade and find the release notes.

If we do decide to go this route, I think adding a short blurb in our docs saying "in the event that you are using a language that is no longer supported, your SecureDrop will disable it during the upgrade process for that release" (or similar).

cfm · 2025-02-21T05:42:08Z

Thanks both. Replying to @legoktm in #7443 (comment):

IMO the ideal behavior would be:

any language we disable, is disabled on all SDs that have it explicitly enabled.

if you're using ./securedrop-admin sdconfig/install, an unsupported language is an error

the existing server configuration is not an OSSEC alert

and if/when Hindi and Romanian are supported again, those servers will automatically re-enable the language without any manual interaction needed.

Would that be reasonable?

Very reasonable, and I've done some slicing and checklisting here to show what we have currently (checked) and as of this pull request (checked and in bold).

Changing from error (with OSSEC alert) to warning (no OSSEC alert) should be a one-line change I'll tack on tomorrow along with responding to your review feedback.

cfm added the i18n Anything related to translation or internationalization of SecureDrop label Feb 20, 2025

cfm added this to the SecureDrop 2.12.0 milestone Feb 20, 2025

cfm requested a review from a team as a code owner February 20, 2025 00:19

cfm requested review from legoktm and nathandyer February 20, 2025 00:20

fix(validate_locale_configuration): "i18n.json" may not be available …

ac79d45

…under test

zenmonkeykstop self-requested a review February 20, 2025 01:33

legoktm requested changes Feb 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(`validate_locale_configuration`): discard and warn on configured but unsupported locale #7443

feat(`validate_locale_configuration`): discard and warn on configured but unsupported locale #7443

cfm commented Feb 20, 2025 •

edited

Loading

cfm commented Feb 20, 2025

cfm commented Feb 20, 2025

legoktm commented Feb 20, 2025

legoktm Feb 20, 2025

legoktm Feb 20, 2025

nathandyer commented Feb 20, 2025 •

edited

Loading

cfm commented Feb 21, 2025 •

edited

Loading

feat(validate_locale_configuration): discard and warn on configured but unsupported locale #7443

Are you sure you want to change the base?

feat(validate_locale_configuration): discard and warn on configured but unsupported locale #7443

Conversation

cfm commented Feb 20, 2025 • edited Loading

Status

Description of Changes

Testing

Deployment

Checklist

If you made changes to the server application code:

If you made changes to securedrop-admin:

If you made changes to the system configuration:

If you added or removed a file deployed with the application:

If you made non-trivial code changes:

cfm commented Feb 20, 2025

cfm commented Feb 20, 2025

legoktm commented Feb 20, 2025

legoktm Feb 20, 2025

Choose a reason for hiding this comment

legoktm Feb 20, 2025

Choose a reason for hiding this comment

nathandyer commented Feb 20, 2025 • edited Loading

cfm commented Feb 21, 2025 • edited Loading

feat(`validate_locale_configuration`): discard and warn on configured but unsupported locale #7443

feat(`validate_locale_configuration`): discard and warn on configured but unsupported locale #7443

cfm commented Feb 20, 2025 •

edited

Loading

If you made changes to `securedrop-admin`:

nathandyer commented Feb 20, 2025 •

edited

Loading

cfm commented Feb 21, 2025 •

edited

Loading