refactor: pass explicit dest_dir for vault backup export#282
Merged
paolodamico merged 3 commits intomainfrom Mar 11, 2026
Merged
refactor: pass explicit dest_dir for vault backup export#282paolodamico merged 3 commits intomainfrom
dest_dir for vault backup export#282paolodamico merged 3 commits intomainfrom
Conversation
thomas-waite
changed the title
dest_dir for vault backup export
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
paolodamico
approved these changes
Mar 11, 2026
| let backup_path = src_inner.export_vault_for_backup().expect("export vault"); | ||
| // Export plaintext vault to a separate directory | ||
| let export_dir = temp_root_path(); | ||
| std::fs::create_dir_all(&export_dir).expect("create export dir"); |
Contributor
There was a problem hiding this comment.
not sure we should panic here, why not expose the error?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates
export_vault_for_backup()to take a file path to which the plaintext exported vault will be written -dest_dir.Motivation
Previously, the plaintext vault export was written to WalletKit's own storage directory under
Documents/worldid/. This meant that any orchestrator callingexport_vault_for_backup(), e.g. an iOS native app, needs to copy the exported file themselves to the correct location from which any existing backup process would occur. This creates two copies of the unencrypted vault data which both need to be cleaned up.By making
dest_diran arg, the caller can write the exported plaintext vault directly to the correct place. e.g. in a reference World App implementation, World App could write the vault export directly to Bedrock's filesystem without having to first copy the file over from WalletKit storage.Further, this change means an integrator could pass a temporary directory as the location to write the vault export: #280
Note
Medium Risk
Public API/UniFFI signature change can break downstream callers and introduces new path-handling expectations (e.g., invalid or unwritable
dest_dir).Overview
Refactors plaintext vault backup export to require an explicit destination directory:
CredentialStore::export_vault_for_backup(dest_dir)now writesvault_backup_plaintext.sqliteinto the caller-provided folder instead of WalletKit’s own storage directory.Updates internal implementation to build the export path from
dest_dir+PLAINTEXT_VAULT_BACKUP_FILENAME, removes theStoragePaths::plaintext_vault_backup_path*helpers, and adjusts tests to create/clean up a dedicated export directory.Written by Cursor Bugbot for commit ffba8c3. This will update automatically on new commits. Configure here.