nk3 set-config: Add warning and confirmation prompt #479
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robin-nitrokey
force-pushed
the
nk3-config
branch
from
9547c06 to
943357c
Compare
|
Maybe we can have an allowlist of config changes that don't have side effects, and only warn for the others ? |
With the upcoming firmware release for the Nitrokey 3, changing some configuration values can have side effects, namely a reset of the opcard application. We will add new commands with additional information and warnings in the future. For the time being, this patch adds a warning to the set-config command, indicating that it may cause data loss and should only be used for testing and development.
robin-nitrokey
force-pushed
the
nk3-config
branch
from
943357c to
1b473d5
Compare
|
I’ve added a whitelist that skips the warning and confirmation for known config values without side effects, currently only |
daringer
approved these changes
Nov 30, 2023
|
lgtm, I think this is a perfectly sane behavior for a cli tool to just warn, but keep a generic interface - also nicer to test - and for end-users (less-techy-users) nitrokey-app2 should be used anyways... |
sosthene-nitrokey
approved these changes
Nov 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the upcoming firmware release for the Nitrokey 3, changing some configuration values can have side effects, namely a reset of the opcard application. We will add new commands with additional information and warnings in the future. For the time being, this patch adds a warning to the set-config command, indicating that it may cause data loss and should only be used for testing and development.
@sosthene-nitrokey We want to release pynitrokey today for some NetHSM fixes. I’d like to include something like this even if we don’t have the proper commands for changing specific config values yet. What do you think?