Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the transfer creation flow to support specifying recipient email addresses and leverages returned recipient public keys to reduce/avoid requiring a transfer passphrase when recipients already have keys.
Changes:
- Extend
POST /sharerequest/response handling to include recipient emails and returnedpublic_keys. - Add
--to(repeatable) toretyc transfer createand display recipients in the confirmation summary. - Adjust encryption/passphrase flow to encrypt the session key for owner + keyed recipients, and only require prompting for a passphrase when needed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/api/transfer.go | Adds public_keys to ShareCreateResponse and threads recipient emails through CreateShare. |
| cmd/transfer.go | Adds --to recipients flag, parallelizes key/share creation, and updates passphrase + encryption behavior based on which recipients have keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fetch the user's own public key and create the share in parallel. | ||
| var titlePtr *string | ||
| if title != "" { | ||
| titlePtr = &title | ||
| } | ||
| if passphrase == "" { | ||
| return fmt.Errorf("transfer passphrase is required") | ||
|
|
||
| type keyResult struct { | ||
| v *api.UserKey | ||
| err error | ||
| } | ||
| type shareResult struct { | ||
| v *api.ShareCreateResponse | ||
| err error | ||
| } | ||
| keyCh := make(chan keyResult, 1) | ||
| shareCh := make(chan shareResult, 1) | ||
|
|
||
| // Fetch the user's own public key so recipients of transfer info can decrypt it later. | ||
| userKey, err := client.GetActiveKey(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("fetching encryption key: %w", err) | ||
| go func() { | ||
| v, err := client.GetActiveKey(ctx) | ||
| keyCh <- keyResult{v, err} | ||
| }() | ||
| go func() { | ||
| v, err := client.CreateShare(ctx, expire, titlePtr, true, toEmails) | ||
| shareCh <- shareResult{v, err} | ||
| }() | ||
|
|
||
| kr := <-keyCh | ||
| if kr.err != nil { | ||
| return fmt.Errorf("fetching encryption key: %w", kr.err) | ||
| } | ||
| userKey := kr.v | ||
| if userKey == nil { | ||
| return fmt.Errorf("no active encryption key — set up your key in the web interface first") | ||
| } |
There was a problem hiding this comment.
Creating the share concurrently with GetActiveKey means that if GetActiveKey fails (or returns nil), the goroutine calling CreateShare may still have created a transfer on the server, and the command then returns an error without cleaning it up. Consider fetching the active key first (then creating the share), or using a cancellable context and disabling/deleting the created share on early failure to avoid leaving orphan transfers behind.
| // Decide whether a transfer passphrase is needed. | ||
| // A passphrase is not needed only when all specified recipients already have a key. | ||
| allHaveKeys := len(toEmails) > 0 && len(share.PublicKeys) == len(toEmails) | ||
| needPassphrase := !allHaveKeys || passphraseExplicit | ||
|
|
||
| // Inform the user if some recipients have no key and a passphrase is therefore required. | ||
| if len(toEmails) > 0 && len(share.PublicKeys) < len(toEmails) { | ||
| fmt.Fprintf(os.Stderr, "Note: %d recipient(s) have no encryption key — a transfer passphrase is required.\n", | ||
| len(toEmails)-len(share.PublicKeys)) | ||
| } | ||
|
|
||
| // Prompt for transfer passphrase if required and not provided via flag. | ||
| if needPassphrase && passphrase == "" { | ||
| fmt.Fprint(os.Stderr, "Transfer passphrase: ") | ||
| pb, err := term.ReadPassword(int(os.Stdin.Fd())) | ||
| fmt.Fprint(os.Stderr, "\r\033[2K") | ||
| if err != nil { | ||
| return fmt.Errorf("reading passphrase: %w", err) | ||
| } | ||
| passphrase = string(pb) | ||
| } | ||
| if needPassphrase && passphrase == "" { | ||
| return fmt.Errorf("transfer passphrase is required") | ||
| } |
There was a problem hiding this comment.
CreateShare is executed before prompting for a required transfer passphrase. If the passphrase is required but cannot be read (non-interactive run, user aborts, etc.), the command returns an error after having created a transfer on the server but before uploading/completing it. Consider prompting/validating the passphrase earlier when it will be required, or explicitly disabling the share on this failure path once share.ID is known.
cf5ca79 to
d0c383f
Compare
d0c383f to
af0bd18
Compare
|
@copilot recheck, new code added, force push done |
No description provided.