cli: correct keyring new and --keyring-path help text#4729
cli: correct keyring new and --keyring-path help text#4729SAY-5 wants to merge 1 commit intosourcenetwork:developfrom
Conversation
Two small-but-confusing inaccuracies in the keyring CLI help:
1. `keyring new --help` claimed that DEFRA_KEYRING_SECRET "must be set
to unlock the keyring" and that the command "will overwrite existing
keys". Neither is true in the default flow:
- On a first run with no DEFRA_KEYRING_SECRET set, the keyring is
initialised with "secret" as its password rather than erroring
out, so the env var is only needed to unlock an *existing*
keyring.
- Existing keys are *preserved* unless --force is passed: the
second run returns "key ... already exists, use --force to
overwrite".
2. `--keyring-path` help text was `Path to store encrypted keys when
using the file backend`. The default shown to the user is "keys",
which looks like a cwd-relative path but is actually resolved
against --rootdir by LoadConfig (relative paths are Join'd with
rootdir). Users on first run routinely guess wrong about where
keys/ lands (we've seen `./keys` assumptions in issues).
Rewrite both help strings to match behaviour. No code-path changes,
CLI-reference docs will regenerate the same way they already do.
Fixes sourcenetwork#4704
Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-03-27T08:24:16.083ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughDocumentation updates to CLI help text for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
What
Two small-but-confusing inaccuracies in the keyring CLI help, per #4704:
keyring new --helpclaimed thatDEFRA_KEYRING_SECRET"must be set to unlock the keyring" and that the command "will overwrite existing keys". Neither matches the default flow:DEFRA_KEYRING_SECRETset, the keyring is initialised with"secret"as its password rather than erroring out. The env var is only needed to unlock an existing keyring.--forceis passed. Running the command a second time without--forcereturnskey ... already exists, use --force to overwrite.--keyring-pathhelp text wasPath to store encrypted keys when using the file backend. The default shown to the user is"keys", which looks like a cwd-relative path but is actually resolved against--rootdirbyLoadConfig(relative paths areJoin'd withrootdir). Users on a first run routinely guess wrong about wherekeys/lands.Fix
Rewrite both help strings to match the actual behaviour:
keyring new --helpnow says the env var unlocks an existing keyring, that a first run defaults to"secret"and should be rotated, and that existing keys are preserved unless--forceis passed.--keyring-pathhelp now notes that the default"keys"is rootdir-relative and typically resolves to~/.defradb/keys.No behaviour change. Only help strings. CLI reference docs regenerate from the same metadata.
Verification
On
developbefore this PR:— works fine without the env var, contradicting the old help text. After this PR the help text matches.
— the "WARNING: This will overwrite existing keys" line in the old help was also contradicted by this behaviour.
Fixes #4704