Add interactive defaults and password storage#3
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| // Save stores or updates a password under the provided label. | ||
| func (s *FileStore) Save(label, password string) (StoredPassword, error) { | ||
| cleanLabel := strings.TrimSpace(label) | ||
| if cleanLabel == "" { | ||
| return StoredPassword{}, errors.New("label cannot be empty") | ||
| } | ||
| if password == "" { | ||
| return StoredPassword{}, errors.New("password cannot be empty") | ||
| } | ||
|
|
||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
|
|
||
| entries, err := s.readAll() | ||
| if err != nil { | ||
| return StoredPassword{}, err | ||
| } | ||
|
|
||
| now := time.Now().UTC() | ||
| for idx := range entries { | ||
| if strings.EqualFold(entries[idx].Label, cleanLabel) { | ||
| entries[idx].Label = cleanLabel | ||
| entries[idx].Password = password | ||
| entries[idx].UpdatedAt = now | ||
| if err := s.writeAll(entries); err != nil { | ||
| return StoredPassword{}, err | ||
| } | ||
| return entries[idx], nil | ||
| } | ||
| } | ||
|
|
||
| record := StoredPassword{ | ||
| Label: cleanLabel, | ||
| Password: password, | ||
| CreatedAt: now, | ||
| UpdatedAt: now, | ||
| } | ||
| entries = append(entries, record) | ||
| if err := s.writeAll(entries); err != nil { | ||
| return StoredPassword{}, err | ||
| } | ||
|
|
||
| return record, nil |
There was a problem hiding this comment.
Prevent concurrent saves from clobbering password store
The new file-backed password store only guards Save with an in-process mutex and rewrites the entire JSON file after loading it into memory. Because the mutex is not shared across processes, two CLI invocations executed in parallel will both read the same snapshot, append their own entry, and then writeAll replaces the file with whichever write finishes last. The earlier save is silently discarded, resulting in lost credentials whenever users run concurrent save commands or the CLI auto-prompts from multiple shells at once. Consider adding a filesystem lock or another inter-process synchronization mechanism to ensure writes are serialized.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f7e36cbc8c832cb00f2086c912a0c5