Repository navigation
codex32: Add Bitcoin Core restore flow - #230
Conversation
This comment has been minimized.
This comment has been minimized.
BenWestgate
left a comment
There was a problem hiding this comment.
Concept NACK.
CipherStick needs a GUI as easy-to-use GUIs are the standard for software included in Tails.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
4fd6853 to
12d5c49
Compare
|
Addressed the Concept NACK: this no longer opens the |
|
cACK @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12d5c4962f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd0f0638de
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 336a3dae86
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4144c0b91c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd6ed693c8
ℹ️ 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".
bd6ed69 to
859c4b7
Compare
|
Rebuilt as The branch therefore carried a stale copy of the whole tree alongside the feature. Replayed onto the current base is
Not replayed, because the snapshot was simply older than master and would have reverted it: Diff is now +431/-2 against the base, from 523 insertions and 87 deletions before. The previous history is kept as #237 is unaffected: it was already merged, and its content is carried into the rebuild. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 859c4b7de8
ℹ️ 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".
|
Triage: this is the replacement path for #215, but it is not ready for human approval yet. The pinned GUI still imports into Bitcoin Core before a strong wallet-identity check, and the persistent virtualenv can break after a Tails Python minor-version upgrade. Fix those two points, update the recovery text to match the final flow, then run one end-to-end restore test on current Tails. |
This comment was marked as outdated.
This comment was marked as outdated.
|
Update: python-codex32#29 is closed. python-codex32#28 now carries the whole gate on its own: a typed fingerprint, plus a no-record path that accepts Bails' RIPEMD-160 backup identifiers. Once it's reviewed, repin to it. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3d459ff4d
ℹ️ 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".
|
The red "Analyze (python)" CodeQL check isn't this PR's failure. It fails on master too: #202 removed the last Python file, so CodeQL finds no Python source to scan. #299 switches the scan to GitHub Actions workflows. This PR's diff doesn't touch CodeQL, so it's fine to merge with that check red. Generated by Claude Code |
A corrupt .git/index made the install check offer a repair, but the repair's own fetch and checkout then failed on the same index and set -e aborted. Remove the index first; checkout --force rebuilds it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PjUU3NvvEE8SGD3RdpXWDr
This comment has been minimized.
This comment has been minimized.
BenWestgate
left a comment
There was a problem hiding this comment.
crACK changes since my last review look good.
This comment has been minimized.
This comment has been minimized.
BenWestgate
left a comment
There was a problem hiding this comment.
Code review found one blocking source-integrity issue on the current head. I reproduced it against Git: a modified tracked file hidden with git update-index --assume-unchanged leaves git status --porcelain=v1 --untracked-files=all --ignored=matching empty, and even git diff --quiet HEAD -- file returns success. Because both launcher/installer treat that status result as proof that the persistent checkout matches the reviewed pin, modified recovery code can be imported/executed while HEAD still equals the pinned revision. Please make the cleanliness check independent of mutable index flags (and apply the same fix in open-codex32) before human merge.
git status trusts index flags, so a tracked file marked assume-unchanged could be modified while the checkout still looked clean, and the launcher would import it. Both the installer and the launcher now hash every file under the checkout and compare it with the pinned tree, rejecting missing, extra, ignored and replaced files without reading the index. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PjUU3NvvEE8SGD3RdpXWDr
This comment has been minimized.
This comment has been minimized.
BenWestgate
left a comment
There was a problem hiding this comment.
ACK 8006c5a after re-review of the source-integrity fix.
The P1 index-flag bypass is closed at both entry points. I reproduced the original condition with a modified tracked module hidden by assume-unchanged (empty porcelain status) and also tested skip-worktree; source_tree_clean rejects both because it hashes the actual filesystem bytes against the pinned tree rather than trusting the index. Clean pinned source passes, while a missing tracked file, extra ignored file, and a symlink replacing a tracked file are rejected. install-codex32 and open-codex32 carry the same verifier.
Regression review: the pinned python-codex32 tree has no executable tracked source files, so the verifier intentionally comparing blob contents/types rather than ordinary executable-bit changes does not alter the launch boundary. git ls-tree failure is fail-closed under pipefail. bash -n and git diff --check pass; exact-head Lint CI, CodeQL and dependency review are green.
No remaining code blocker found for this PR. Human review/merge is still required.
Run the hash verifier with python3 -I so packages in the user site directory cannot change its result, and drop the user site from the import check and the launch. Fail the check when a folder cannot be read instead of skipping it. Document that the check guards against corruption, not a local attacker who can already edit these scripts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PjUU3NvvEE8SGD3RdpXWDr
This comment has been minimized.
This comment has been minimized.
Revert the content-hash verifier (8006c5a, 2378fad). The checkout check guards against corruption and accidents, which git status already catches; the hash verifier only defended against a local attacker who can edit these scripts anyway. Document that scope. The corrupt-index repair fix stays. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PjUU3NvvEE8SGD3RdpXWDr
This comment has been minimized.
This comment has been minimized.
Resolve conflicts with the squash-merged #230: keep this branch's Core datadir probe and wait loop, and take master's corrupt-index repair fix and checkout-check comment. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PjUU3NvvEE8SGD3RdpXWDr
…y-claims Bring in the merged codex32 restore flow (#230). Keep this branch's bounded wording and list the pinned python-codex32 application as supported, replacing the stale "tracked in #215" notes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PjUU3NvvEE8SGD3RdpXWDr
Shorten Bitcoin Core verification and synchronization guidance to fit the supported Tails desktop, use current Zenity icon arguments on the touched surfaces, keep signer identity visible without clipping, and describe synchronization as starting only after the user acknowledges the modal dialog. Keep this presentation-only: no fixed release-specific geometry, new UI framework, or dependency is added. Refs #256 Refs #230 Refs #249
Shorten Bitcoin Core verification and synchronization guidance to fit the supported Tails desktop, use current Zenity icon arguments on the touched surfaces, keep signer identity visible without clipping, and describe synchronization as starting only after the user acknowledges the modal dialog. Keep this presentation-only: no fixed release-specific geometry, new UI framework, or dependency is added. Refs #256 Refs #230 Refs #249
Status
Current head
fddc710is mergeable, the stacked diff is one coherent commit / 12 files, Lint CI is green, and all current review threads are resolved. The human Tails 7.13 create/restore runtime gate passed on 2026-09-29.What
python-codex32GTK4/libadwaita GUI at1938b604182b5553f4e724fd2aea814496a24cd4codex32-guiapplication directly; Bails adds no wallet UIWhy
This replaces the broken legacy GTK3 wallet flow reported in #240 while keeping wallet/recovery logic in
python-codex32.Refs #215, #240
Dependency
#244 is merged. #202 is now rebased on current
master; merge #202 first, retarget this PR tomaster, then merge it immediately so the legacy wallet removal and replacement codex32 flow land in sequence.Testing
git diff --checkandbash -npassed for the integration scripts before the rebase; the final file content is unchangedgit statuschecks; clean trees pass and dirty/broken-index trees failpython-codex32revision: 987 tests pass normally and 987 underpython -O; Ruff and mypy pass64e6b96d…kgxis present butgnome-terminalis not; the launcher avoids adding a terminal dependencyFollow-up tracking from hands-on review
The broader findings are intentionally outside this integration PR and are tracked in focused work under #249:
codex32naming sweep: docs: write codex32 in lowercase everywhere users see it #239The latest codex32 GUI-specific tester findings are tracked in
python-codex32: #43 (retype the recorded fingerprint), #71 (neutral card-count choices), #72 (paper-card group layout), #73 (wallet-encryption guidance), #74 (fit final wallet identity on one screen), #75 (compact home-window sizing), and #76 (distinct home-action artwork). The reported extra-group repair, recovery-vs-repair highlighting, and readback-prefix/cursor behavior are already corrected in the current #65 GUI candidate and were not duplicated as new issues.