-
Notifications
You must be signed in to change notification settings - Fork 929
(#3503, #3513) Handle arguments file decryption failures #3540
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
Conversation
This is WIP as I still need to add testing steps, as well as some tests to the repository. I also need to validate that it does fix 3513 as right now it's just suspicions that it will. |
d183bb2
to
8248dd0
Compare
This PR is now ready for review. |
I'm marking this as draft again so we don't merge it before we have some internal discussions about this. This appears that it may require changes in other projects, and we'll need to understand the repercussions of those changes. |
ok, back to ready for review as we now understand the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor things we should probably clean up but otherwise this looks good to me, great work!
8248dd0
to
ff7f4cf
Compare
c09b8c5
to
8399dcd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good here, great work! 💜
Currently we are decrypting the arguments file every time a list is run, even though we only care about them in scenarios where we might output them to the user (when Verbose is specified on the list command or with the info --local-only command). This updates the handling of the remembered arguments to be consistent between the List method and the scenario in an Upgrade where we want to use the remembered arguments. We now will only throw an error in a scenario where we can't decrypt the contents and we need them for the scenario to succeed. We are also stating what file we couldn't decrypt so the user can troubleshoot.
Add some Pester tests for the arguments file decryption handling. Ensuring that both invalid keys and base64 corruption works correctly.
- Use OrdinalIgnoreCase for command name comparison. - Use explicitly named parameters instead of true/false. - Remove comment that's no longer valid.
8399dcd
to
68b7916
Compare
Description Of Changes
Better handle failures to decrypt the arguments file.
Motivation and Context
When the file fails to decrypt, it should not block things unless it is actually a blocking failure.
Testing
Note: for my testing I downloaded the internal build from our build server. If you build these components yourself you'll need to use
--allow-unofficial
on all of thechoco
commands.choco install glab -y
will installglab
andglab.portable
)C:\ProgramData\Chocolatey\.chocolatey
.arguments
file within. (You'll need to run your editor as admin)+
or/
should suffice. I added!
to the start of theglab
file.)AQAAANCMnd8BFdERjHoAwE/Cl+sBAAAAn1/taDnOFUqGb17fBymxHQQAAAACAAAAAAAQZgAAAAEAACAAAAAU8gmqznJYKdkuj8bgk8sgg6Le3sbGoGkZOV3YtRFfwwAAAAAOgAAAAAIAACAAAAD1I9LYxrEhx9m71eF3VqyAike+XJTePhDAcrOilAFjQlAAAAA8lfiMR5Ns/AntLdVR3eBQSduCnipRCbdu/er/+YABMTzJDMGqnXuIsKwWoNIhrB14Yit4jVPipt3a/Nx18xx+YsnUewI4P6GlDL5do1y8mkAAAABMxvyPgCtN36BwAOXvJghIh9Hs8jUZOJtQIlWci8BnJkBmaaoSZ6pTGULk4TbFXMf/FK1NPo2mPM0YVL8QgJyK
(this is the string used in the text. It is from a VM that has long been discarded).choco list
choco pin list
choco pin add -n glab
choco pin list
choco pin remove -n glab
choco install gitbutler -y
choco upgrade all -y
(note this is not expected to find any updates, but it also does not error)choco list --verbose
choco info glab --local-only
choco info glab.portable --local-only
useRememberedArgumentsForUpgrades
feature:choco feature enable -n useRememberedArgumentsForUpgrades
choco upgrade glab -y
choco upgrade glab.portable -y
6.
choco uninstall glab -y
7.
choco uninstall glab.portable -y
8.
choco upgrade gitbutler -y
In addition to the above testing, I have also tested this in Test Kitchen against both this PR and against
2.3.0
to validate that the tests fail on2.3.0
Operating Systems Testing
Change Types Made
Change Checklist
Related Issue