Skip to content

Add first-run backup security mode selection#12

Open
thatdaveguy1 wants to merge 1 commit intoLampese:mainfrom
thatdaveguy1:codex/backup-security
Open

Add first-run backup security mode selection#12
thatdaveguy1 wants to merge 1 commit intoLampese:mainfrom
thatdaveguy1:codex/backup-security

Conversation

@thatdaveguy1
Copy link
Copy Markdown
Contributor

Why

Full backup encryption currently depends on a single built-in fallback secret. That preserves compatibility, but it does not give new users a meaningful security choice.

This PR adds an explicit first-run decision so new users can pick a stronger backup protection model while preserving the existing behavior as a compatibility option.

What changed

  • add a first-run backup security chooser in src/App.tsx
  • keep the existing behavior available, labeled Less Secure
  • persist the selected backup security mode in app settings
  • extend full export and import flows to support:
    • OS keychain-backed secrets
    • user-supplied passphrases
    • the legacy built-in fallback mode
  • add backend support in settings.rs, account.rs, and types.rs

Reviewer notes

  • this PR is intentionally limited to backup security selection and export/import behavior
  • process restart behavior and storage hardening are handled in separate follow-up PRs

Verification

  • cargo check in src-tauri passes on this branch

@Lampese
Copy link
Copy Markdown
Owner

Lampese commented Mar 7, 2026

Sorry, I don't quite understand what this PR is about. Do we have any security issues?

@E9C50
Copy link
Copy Markdown
Contributor

E9C50 commented Mar 9, 2026

Sorry, I don't quite understand what this PR is about. Do we have any security issues?

Currently, when exporting backup files, the account information is encrypted using a key that is hardcoded in the source code. The contributor's intention is to allow users to set their own custom keys to enhance the security of these backup files.

When I originally implemented this feature, I thought managing custom keys might be a bit cumbersome for users, as they would need to ensure they use the exact same key when importing/exporting across different machines.

@Lampese
Copy link
Copy Markdown
Owner

Lampese commented Mar 9, 2026

Sorry, I don't quite understand what this PR is about. Do we have any security issues?

Currently, when exporting backup files, the account information is encrypted using a key that is hardcoded in the source code. The contributor's intention is to allow users to set their own custom keys to enhance the security of these backup files.

When I originally implemented this feature, I thought managing custom keys might be a bit cumbersome for users, as they would need to ensure they use the exact same key when importing/exporting across different machines.

Is this encryption necessary? I mean, what impact would it have on a user exporting the plaintext?

Copy link
Copy Markdown
Owner

@Lampese Lampese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two behavioral issues:

  1. Keychain mode does not produce a portable backup. The secret is generated and stored only in the current machine's OS keychain, and import can only read that same local keychain back. The file itself does not carry any portable recovery material, so the backup becomes unrecoverable on a different machine, a different OS user, or after losing the original keychain. Labeling this option as Recommended is likely to mislead users into treating it as a real backup strategy.

  2. Existing users effectively cannot access the new security mode selection. The selection UI only appears on first-run when accounts.length === 0 and export_security_mode is still unset. Users upgrading with existing accounts will never see this entry point, while the backend still falls back to less_secure by default for exports. In practice, that means this PR barely changes behavior for current installations.

I think these two issues should be resolved before merging, either by changing the product semantics or by changing the file format and entry-point design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants