feat: key generation and storage#27
Conversation
paolodamico
requested review from
Takaros999,
andy-t-wang and
m1guelpf
as code owners
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca64d442d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match backend { | ||
| Backend::Keyring => Ok(Box::new(KeyringStore::new()?)), | ||
| Backend::File => Ok(Box::new(FileStore::new(data_dir.join(FILE_NAME)))), |
There was a problem hiding this comment.
When backend is Keyring, this always returns KeyringStore and there is no fallback path if the platform keyring cannot actually be accessed (for example, Linux sessions without a running Secret Service). In that case store.read()/store.write() fails and enroll exits, even though the code/docs describe file storage as the fallback for unusable keyrings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is deliberate so the user makes an explicit decision
| let cfg = Config { backend }; | ||
| cfg.save(paths)?; |
There was a problem hiding this comment.
The selected backend is written to config.json before verifying it can be used; if a user accepts the recommended keychain option in an environment where keyring access fails, the broken keyring preference is persisted and subsequent runs reuse it without re-prompting. This leaves enrollment stuck until the config is manually edited or deleted.
Useful? React with 👍 / 👎.
| Ok(()) | ||
| } | ||
|
|
||
| fn resolve_config(paths: &Paths) -> Result<Config> { |
There was a problem hiding this comment.
nit. i feel like this doesn't belong here, it's not specific to the enroll comamnd, maybe move it to config.rs
2 participants
Initiates the
enrollcommand by generating a random secret and storing it. The secret can be stored in keychain (or equivalent) by default with a fallback to filesystem if there's no keychain available.Also adds a configuration json file stored in the home path of the user.