fix(cli): default include_keys to False in soul export (#273)#274
fix(cli): default include_keys to False in soul export (#273)#274sshekhar563 wants to merge 2 commits into
Conversation
prakashUXtech
left a comment
There was a problem hiding this comment.
Good catch on this one. Exporting the private signing key by default was a real leak, and the fix is right. I went through all the include_keys=True call sites on dev to check nothing else was affected, and the flip is complete: soul export was the only share-intended path that leaked the key, and the in-place saves (remember, evolve, and the rest) correctly keep it. --include-keys still works for migration. Clean change.
Two things before merge, both packaging rather than substance:
1. A test. Since this is a security default, it's worth locking in. A CliRunner test asserting the default export zip contains keys/public.key but not keys/private.key, and that --include-keys puts the private key back. tests/test_cli/ is the spot.
2. Docs. docs/cli-reference.md under soul export doesn't list --include-keys, and the "Portable transfer between machines" line now needs the flag to carry signing power. Worth updating both here.
One suggestion, non-blocking: the migrate-then-delete workflow now silently drops signing power, and with the quiet verification-only mode it's easy to miss until signing stops working. A one-line console notice on default export ("private key not included; pass --include-keys to migrate signing power") would save someone a confusing afternoon.
Also a stray extra blank line snuck in around main.py:705. Drop it while you're in there.
Thanks for the threat write-up on the issue too. Made this easy to verify.
Add --include-keys flag to the export command. Private signing keys are no longer included by default when exporting .soul files, preventing accidental key leaks when sharing files. Users must explicitly pass --include-keys when migrating between their own devices.
- Add 6 tests for export key behavior (tests/test_cli/test_export_keys.py) - Default export has public.key but NOT private.key - --include-keys adds private.key back - Console notice on default export: 'private key not included' - Update docs/cli-reference.md with --include-keys option - Remove stray blank line at main.py:707
55865de to
98d2df4
Compare
What
Fixes #273
The
soul exportcommand was passinginclude_keys=Trueby default, which included the private Ed25519 signing key in every exported.soulfile.Changes
--include-keysflag tosoul export(default:False)--include-keysUsage