Repository navigation
wallet: never lose the key: keep a copy outside the install dir, restore before creating - #47
Merged
Merged
Conversation
…ore before creating The identity files live in the install dir, which goes away on pilotctl appstore uninstall (and on every upgrade with a pilotctl older than v1.13.10). The wallet then created a fresh key on its next start, and funds at the old EVM address were out of reach. On every start the wallet now mirrors identity.json and identity-evm.json into ~/.pilot/keys/io.pilot.wallet (0700, files 0600). Before it would create a key, it restores the old one: from that mirror first, then from the newest copy in pilotctl's backups of replaced or uninstalled installs. Each candidate is loaded as a key before it is used, so a corrupt file is skipped. A key is created only when no copy exists anywhere. Nothing is ever deleted: a mirror copy holding a different key is kept as <name>.replaced-<time>. --key-vault sets the mirror dir (empty disables). The manifest declares the new file access. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…overwrite a copy Review of the key vault found: - One mirror for every wallet under a HOME: a dev run or a test replaced the real wallet's copy, and a lost install dir came back with the wrong key. The mirror is now per install path (<vault>/<dir>-<hash of its path>), and the package's tests run with a private HOME (a test run had copied a developer's real key into the vault). - A restore placed the copy and validated it afterwards, and a rename could replace a key created meanwhile. The copy is validated in a temp file and linked into place, which fails if the file appeared: a live key path is never removed or overwritten. - keep() overwrote a mirror copy it could not read, and replaced-<second> names collided. An unreadable copy is now left alone (keep fails), and every replaced copy gets a unique name, linked exclusively. - Another usable copy that disagrees with the one restored is logged as a WARNING, and kept. - Backups are ordered by their name stamp (mtime as a fallback, newer name first on a tie); symlinks and files owned by another user are never restored; a looser-mode copy is restored as 0600 with a warning; only <install>.previous and <install>.previous-* are searched. - The flag help says backups are searched even with the mirror off, and how to start over; the manifest grants list every path read or written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Second review of the key vault: - A copy that could not be read or put in place was treated like one that is not a key: the restore moved on to an older copy, or let a new key be created, with the right one still on disk. Now only content that is not a usable key, or a file that is not the user's own regular file, is skipped; any other error stops the wallet (the daemon retries). - Where hard links are not supported (network and FUSE homes, sandbox profiles), placing a copy fell into that path. It now falls back to an exclusive create, still never replacing a file. - A copy kept under another install path (HOME moved, another app root) was ignored and a new key created beside it. Such a copy is still not restored, but the wallet now refuses to create a key while it exists, and names it. - Candidates are opened without following a symlink, and checked on the open file. Root may restore a copy owned by the user. - The tests also clear PILOT_APPSTORE_BACKUP_ROOT and PILOT_APPSTORE_ROOT, and each run() test has its own vault; the manifest grants <install>.previous too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Third review: errors while finding copies were still read as "no copy". With the mirror dir unsearchable (a root-owned dir from a sudo run, EIO), the wallet started on a new key beside the old one. - Any error other than "does not exist" while listing or checking the mirror, the vault root, the backup dirs and their entries, or the <install>.previous dirs now stops the restore; the .previous dirs are listed rather than globbed so errors are not lost. - Before a backup is placed, a usable copy kept under another install path holding a different key stops the restore too, naming it. - As root, only copies owned by root or by the owner of HOME are restored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The other-path copy check validated its temp files in the identity's dir before anything had created it, and refused to start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Problem
The wallet keeps its two identity files in its install dir (
~/.pilot/apps/io.pilot.wallet): the overlay keyidentity.jsonand the EVM keyidentity-evm.json, which holds the funds.pilotctl appstore uninstall, and with a pilotctl older than v1.13.10 on every upgrade.Fix
A mirror per install path. On every start the wallet copies both identity files into
~/.pilot/keys/io.pilot.wallet/<dir>-<hash of the install path>/: dir 0700, files 0600. A second wallet with other paths (a dev run, a test) never sees or replaces this one's copies.Restore before create. Before the wallet would create a key, it puts the old one back. It looks in:
$PILOT_APPSTORE_BACKUP_ROOT,~/.pilot/app-backups/io.pilot.wallet/*,<root>/.app-backups/…,io.pilot.wallet.previous[-*]).How a candidate is handled:
A key is created only when no copy exists anywhere.
Nothing is deleted or overwritten without a copy.
<name>.replaced-<time>name.keepfails).Also:
--key-vaultsets the mirror root; empty turns the mirror off, and backups are still searched.~/.pilot.Tests
keyvault_test.go, using real key files made by the wallet's own loaders:Each fails with its fix reverted.
End to end, with real pilotctl builds (v1.13.10, v1.17.0, main + pilotprotocol#524), a test-signed catalogue and the real 0.3.3 bundle, on macOS arm64: catalogue install 0.3.3, run, upgrade to 0.4.0, run, uninstall, reinstall, run. The same EVM key is kept throughout. After the reinstall the wallet restores both identity files from its mirror, and it does so even with v1.17.0's deleting uninstall.
go test -race ./...passes, and the wallet cross-compiles for all four platforms.🤖 Generated with Claude Code