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

Consider avoiding assert statements in validate_config.py #683

Closed
eloquence opened this issue Mar 30, 2021 · 4 comments · Fixed by #1025
Closed

Consider avoiding assert statements in validate_config.py #683

eloquence opened this issue Mar 30, 2021 · 4 comments · Fixed by #1025

Comments

@eloquence
Copy link
Member

Informational finding TOB-SDW-024 from the 2020 SecureDrop Workstation audit (PDF) notes that assert statements are optimized out if Python is ever run with the -O or -OO flags and recommends eventually factoring them out in favor of if conditions and custom exceptions in validate_config.py, which is not test code but used during the installation.

@eloquence eloquence changed the title Consider avoiding assert statements in validate_config Consider avoiding assert statements in validate_config.py Mar 30, 2021
@eloquence eloquence changed the title Consider avoiding assert statements in validate_config.py Consider avoiding assert statements in validate_config.py Mar 30, 2021
@pierwill
Copy link
Contributor

pierwill commented Oct 4, 2021

I'd be happy to help with this. Would we need a small helper function that tests for truth and exits on false?

@conorsch
Copy link
Contributor

conorsch commented Oct 4, 2021

That's right, simply refactoring the logic not to use assert would be sufficient. Over in the SD repo we have a rather complex validator for server config https://github.com/freedomofpress/securedrop/blob/c267329e5bad9c3396a5087b41275bb324fbf4f8/admin/securedrop_admin/__init__.py#L119 but I don't think you need to go quick so far to satisfy this issue! =)

@gonzalo-bulnes
Copy link
Contributor

Would we need a small helper function that tests for truth and exits on false?

I was thinking along the same lines. I think I'd make all those functions return a bool and either exit early with a function like @pierwill suggests, or collect all the steps and exit 0 or 1 if any of the checks returned False.

@zenmonkeykstop zenmonkeykstop self-assigned this Apr 25, 2024
@zenmonkeykstop
Copy link
Contributor

Planning to investigate config validation in the near future, will take this ticket on as a first step.

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 a pull request may close this issue.

7 participants