Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using assert in validate_config.py #1025

Merged
merged 1 commit into from
May 9, 2024
Merged

Conversation

micahflee
Copy link
Contributor

Status

Ready for review

Description of Changes

Fixes #683.

Here's how to reproduce the TOB-SDW-024 bug:

In sd-dev switch to the main branch in securedrop-workstation. In dom0, run make clone.

One of the checks in validate_config.py is that self.config["vmsizes"]["sd_log"] is an integer. So, in dom0, edit /usr/share/securedrop-workstation-dom0-config/config.json and make sd_log a string instead, like this:

{
    "submission_key_fpr": "65A1B5FF195B56353CC63DFFCC40EF1228271441",
    "hidserv": {
        "hostname": "pmlfte7wii27fi5ioyjwzbgdmjk2i5nx2gd6v3rk5bym7rattnvm36ad.onion",
        "key": "NDN4MWTGFCCJMJWXISZDN2RO3DZJKMVRBGIFCNYN2RKX4VDGKJTQ"
    },
    "environment": "dev",
    "vmsizes": {
        "sd_app": 10,
        "sd_log": "5"
    }
}

In dom0, change to ~/securedrop-workstation and run this to validate the config:

python3 files/sdw-admin.py --validate

It should fail:

[user@dom0 securedrop-workstation]$ python3 files/sdw-admin.py --validate
Validating...Traceback (most recent call last):
  File "/home/user/securedrop-workstation/files/sdw-admin.py", line 221, in <module>
    main()
  File "/home/user/securedrop-workstation/files/sdw-admin.py", line 172, in main
    validate_config(SCRIPTS_PATH)
  File "/home/user/securedrop-workstation/files/sdw-admin.py", line 108, in validate_config
    validator = SDWConfigValidator(path)  # noqa: F841
                ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/securedrop-workstation/files/validate_config.py", line 41, in __init__
    self.validate_existing_size()
  File "/home/user/securedrop-workstation/files/validate_config.py", line 132, in validate_existing_size
    assert isinstance(
AssertionError: Private volume size of sd-log must be an integer value.

Now run it with the -OO flag:

python3 -OO files/sdw-admin.py --validate

It will pass when really it should fail -- this is the bug:

[user@dom0 securedrop-workstation]$ python3 -OO files/sdw-admin.py --validate
Validating...OK

Now, switch to this PR's branch and run make clone in dom0. This time when you run it with the -OO flag, it will fail validation like it should:

[user@dom0 securedrop-workstation]$ python3 -OO files/sdw-admin.py --validate
Validating...Traceback (most recent call last):
  File "/home/user/securedrop-workstation/files/sdw-admin.py", line 108, in validate_config
    validator = SDWConfigValidator(path)  # noqa: F841
                ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/securedrop-workstation/files/validate_config.py", line 41, in __init__
    self.validate_existing_size()
  File "/home/user/securedrop-workstation/files/validate_config.py", line 151, in validate_existing_size
    raise ValidationError("Private volume size of sd-log must be an integer value.")
validate_config.ValidationError: Private volume size of sd-log must be an integer value.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/user/securedrop-workstation/files/sdw-admin.py", line 221, in <module>
    main()
  File "/home/user/securedrop-workstation/files/sdw-admin.py", line 172, in main
    validate_config(SCRIPTS_PATH)
  File "/home/user/securedrop-workstation/files/sdw-admin.py", line 110, in validate_config
    raise SDWAdminException("Error while validating configuration")
SDWAdminException: Error while validating configuration

@micahflee micahflee mentioned this pull request May 8, 2024
@zenmonkeykstop zenmonkeykstop self-requested a review May 8, 2024 23:09
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test plan and CI passes, visual review also good.

@zenmonkeykstop zenmonkeykstop added this pull request to the merge queue May 9, 2024
Merged via the queue into main with commit d686790 May 9, 2024
7 checks passed
@legoktm legoktm deleted the 683-avoid-assert branch May 14, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Consider avoiding assert statements in validate_config.py
2 participants