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

securedrop-admin update returns indistinguishable error messages for distinct error conditions #7200

Open
cfm opened this issue Jul 10, 2024 · 3 comments · May be fixed by #7272
Open

securedrop-admin update returns indistinguishable error messages for distinct error conditions #7200

cfm opened this issue Jul 10, 2024 · 3 comments · May be fixed by #7272

Comments

@cfm
Copy link
Member

cfm commented Jul 10, 2024

Description

securedrop-admin update returns the same error message Signature verification failed. for multiple distinct error conditions. It's therefore necessary to step through the logic of the update() function to identify the cause of the error.

sdlog.info("Signature verification failed.")
return 1
except subprocess.CalledProcessError as e:
if "not a valid ref" in e.output.decode("utf-8"):
# Then there is no duplicate branch.
sdlog.info("Signature verification successful.")
else: # If any other exception occurs, we bail.
sdlog.info("Signature verification failed.")
return 1
else: # If anything else happens, fail and exit 1
sdlog.info("Signature verification failed.")
return 1
except subprocess.CalledProcessError:
# If there is no signature, or if the signature does not verify,
# then git tag -v exits subprocess.check_output will exit 1
# and subprocess.check_output will throw a CalledProcessError
sdlog.info("Signature verification failed.")

Steps to Reproduce

Discovered in the course of #7168.

Expected Behavior

Error messages either (a) are self-explanatory or (b) can be traced to a unique point in the code.

Actual Behavior

Neither.

Comments

It would be interesting to run a search for duplicated logging and exception strings across the codebase and see if this is a problem elsewhere.

@xavierlwr
Copy link

xavierlwr commented Jul 12, 2024

interested, will look into this

@rudradeep22
Copy link

If no one is working on this, can I take this up?

@legoktm
Copy link
Member

legoktm commented Oct 2, 2024

Hey @rudradeep22, you're welcome to. If you have any questions or need help feel free to join our Matrix room.

@timini timini linked a pull request Oct 23, 2024 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants