Repository navigation
Security audit: python-codex32 findings #20
Description
Activity
- added a commit that references this issue
on Sep 22, 2026 - added a commit that references this issue
on Sep 23, 2026 Release-gate threat-model correction: the restore ordering finding remains valid, but its v1 requirement is accident safety, not malicious-threshold-share authentication. PRs #57/#28 require the operator to identify the recovered wallet before mutation using a typed record fingerprint or the explicit no-record visual fingerprint + codex32/Bails identifier fallback. A 32-bit fingerprint is not claimed to resist deliberate grinding/replacement. The separate malicious-tampering design is #55: authenticate/decrypt the full descriptor backup and require the recovered seed to reproduce it before mutation. PR #23's audit verdict has been updated to make this separation explicit.
- added 2 commits that reference this issue
on Sep 28, 2026 Audit disposition refresh (Codex, 2026-10-04). I rechecked the four supplied review reports against the current issue/PR stack. This is the item-by-item mapping for the consolidated findings so the umbrella does not rely on an implicit “covered somewhere” claim.
Consolidated finding Current disposition BL1 / Kimi F4 — documented dev setup cannot run tests Runtime/test dependency cause is removed by #7; the actual contributor setup is replaced and verified in #115 (disposable Python 3.14 .[dev]install plus 950 normal/optimized tests). #115 remains a human-authorship/review item.BL2 — BSD-3-Clause notice absent from distributions Fixed by focused #15; artifacts were rebuilt/checked with the notice included. H1 / Kimi F2 — artifact repr/strleaks recovery text#22, fixed by merged #25; explicit .textis the reveal path.H2 / Kimi F3 — wallet import before recovery identity gate #26/#30. CLI/library path is #57 → #105 → #80 → #81 → #95; GUI clean replay is #118 after #65/#66/#77/#78/#119. CLI heads are reviewed/verified; final GUI replay + Tails/manual qualification still gate closure. H3 / Kimi F1 — HRP >83 accepted #32 / focused #33; current head is green, mergeable and has a current-head Codex ACK. M1 / GLM 2 — mixed-case damage cannot be corrected #37 / #42; implemented and integrated into reviewability-v1, with normal/optimized and frozen differential coverage.M2 / GLM 3 — ms32 secret/share/correctCore dependency is confusing/blocking#84 / #99 plus the Core-boundary handoff text in #38. The behavioral fix is reviewed, but #99 currently carries the unrelated <5250cap commit and therefore is not release-ready under the current<5200gate. Restack/drop that cap commit before freeze.M3 / Kimi F6 — entering Smid-recovery silently abandons accepted shares#95 makes the deliberate mode switch explicit; reviewed stack follow-up, normal/optimized verified. M4 — branch defeats its reviewability purpose Addressed as a release-process finding rather than one runtime defect: #7 removes the native/test crypto dependency chain, #46/#105 reduce dead review surface, #64/#53 tighten the supported API, and #38 owns the frozen reviewer handoff. M5 / DeepSeek D3 — inherited Bitcoin Core contributor boilerplate #115 replaces it with the actual python-codex32 workflow and verifies the instructions. Human authorship/review remains. M6 / DeepSeek D1 — benchmark document presents stale figures as current #116 labels commit 6802d86and the host-specific measurements as historical evidence and points reviewers to current CI/budgets. Human review remains.M7 / Kimi F5 — CI omits optimized mode / correction-constant verification #40 / merged #47. Claude ACKed its exact head; the added python -Osuite and constant verifier ran green.M8 / Kimi F7 — caller-supplied seeds skip BIP32-root validity Focused #16 applies the existing stdlib root check to supplied seeds; validated normal and optimized. M9 / Kimi F8 — threshold digit 1differs from reference decoderDeliberate v1 interoperability divergence, not an accidental parser path. #38 requires the final reviewer handoff to state that v1 accepts only the documented unshared/shared header forms. L1 / DeepSeek C3 — correctoverloads exit 1#39 / #45 defines stable statuses (0 valid, 1 suggestion, 2 usage/input, 3 no usable suggestion); already carried by #42. L2 / DeepSeek C1/C2/C4 — unreachable CLI paths, permanently-false ambiguity state, stale test double #46 removed the reviewed dead paths; final-stack cleanup is #105. The remaining fake-Core signature drift is isolated in #117. L3 / DeepSeek C5 — assert-dependent invariants under-OThe release assurance is tracked by #40/#47 and the current stack’s full optimized runs. This is an accepted internal-invariant style only where optimized behavior is independently verified; it is not being represented as wholesale assert removal. Record that disposition in the final #38 handoff. L4 / DeepSeek D6 — ~400 KB host-specific benchmark data #116 explicitly retains it as historical evidence rather than current reproducible qualification; current qualification comes from CI/regenerated checks. L5 — bespoke/nearly exhausted size budget #38 records the maintainer-approved v1 library gate as <5200and the separate GUI gate. The later<5250proposal in #99/#100/#101/#104 is not being inferred as approved from PR text.L6 / DeepSeek D5 — commit hygiene vs repository policy Remaining agent-authored commits are explicitly marked for responsible-human rewrite/squash and review before integration; #38 tracks this as a human-only integration requirement. DeepSeek D2 — public API count stale #64 reduces package-level __all__to 23; #53 publishes vector helpers only at owning modules. #38 pins the expected final count.DeepSeek D4 — no reviewer-facing narrative Intentionally deferred to the last handoff change in #38 so it can pin the actual frozen commits; opening it earlier would make the identifiers stale. GLM 5 — private-name imports blur API boundary #49 / #53 publishes the vector helpers that external reviewers need and documents the remaining intentional internal couplings. The report candidates that failed re-verification remain non-defects; in particular, the historical opaque-HRP deadline-bypass candidate is not reopened.
Post-audit field-test work is tracked separately from the original audit: #102 and #103 are current-head Codex-reviewed, and #103 now also has a focused security ACK. #104 now has a focused recovery-security ACK, but #99/#100/#101/#104 still carry the disputed
<5250cap change and are not release-ready under the current checklist.Result: every validated consolidated finding now has an explicit tracker/fix/accepted-defer disposition in this umbrella. The remaining work is integration/qualification plus the specific #99 cap/restack blocker, not discovery of an untracked original finding.
Follow-up to the disposition table: the cap-policy blocker on #99 has now been removed. #99 was reset to its behavior-only head
c583b24, retargeted onto #105, and received a current-head Codex ACK; its old cap-bearing tip is preserved onarchive/99-pre-restack-20261004. The same cleanup was applied to later field-test #100 and #101: cap commits removed, old tips archived, bases moved to #105, and current behavior heads received Codex ACKs. All three now preserve the<5200policy. Final combined-tip CI is still required after the stack is actually integrated.Audit-closure verification update (Codex, maintainer-authorized):
- generation: Validate roots before re-sharing #126 current head
8cd584eis green in the Python-package workflow and now has a current-head Codex ACK. It closes the existing-MasterSeedBIP32-root path missed by merged generation: Validate supplied BIP32 seeds #16; the runtime test proves rejection before entropy/identifier selection, and the follow-up docs state the boundary. - docs: Close adversarial audit tracking gaps #127 current head
00aae9bis green and has a current-head automated Codex review. Itsdocs/audits/adversarial-2026-10-04.mdremains the exhaustive four-report disposition ledger. - docs: Trim offline signing to what Core's tutorial lacks #93 current head
2fa7d8ais green and now has a current-head Codex ACK after retaining only the offline-payment verification that Core's maintained tutorial does not explicitly state. - docs: Answer first-time questions in the user guide #97 current head
1cf1a17is green and already has a Codex follow-up ACK for its latest documentation fixes.
No supplied material finding currently lacks an issue plus a focused fix/defer path. Release closure is still pending human integration of the reviewed stack, GUI/Tails/manual qualification, the frozen-tip reviewer handoff, exact-artifact qualification, and the fresh adversarial pass over the final library/CLI/GUI/user-documentation candidate.
- generation: Validate roots before re-sharing #126 current head
Current code-scanning note: the two medium “Workflow does not contain permissions” alerts visible in the repository UI are separate from the four supplied adversarial reports and appear stale relative to the current workflow content.
masternow has explicitpermissions: contents: readin both.github/workflows/python-package.ymland.github/workflows/pylint.yml; the review stack's Python-package workflow also declarescontents: read, and that stack no longer carries the Pylint workflow. The failedgithub-advanced-securitycheck seen on #53 is a dynamic “Code scanning AI findings” agent job whose Processing Request step failed; the ordinary CodeQL analysis on that commit succeeded. Do not count that dynamic-agent failure as a package-test failure or as an additional supplied-audit finding. The connector available here does not expose code-scanning alert dismissal, so the UI alert state itself may still need repository-side cleanup.Audit-closure update from Codex review of the moving v1 integration PR: two post-audit findings were reproduced after the four supplied reports were dispositioned.
- wallet: public_descriptors ignores the seed it's given #128: the exported
core_descriptors(..., private=False, ...)path could return descriptors for whatever seed was already in the named Core wallet, ignoring thesecretargument. This API has no repository caller. - correct: CorrectionEdit repr shows corrected characters #129:
CorrectionEditdefault dataclass repr exposed recoveredobserved/replacementcharacters inside a redactedCorrectionCandidate.
Focused draft #130 fixes both. It removes the unused descriptor API and other unused fresh-CL-generation entry points, while preserving Core-native wallet initialization and existing-CL parse/recover/re-share behavior; it also marks the two edit-character fields
repr=Falsewith a regression. #130 supersedes #64/#63 for the API cleanup.I reviewed exact head
3133c8e96ee241ec1b3427e214a5fada5babc84fand posted a Codex security ACK. The Bitcoin Core fixture is green; the full Python-package matrix is still queued as of this comment. These are not findings from the four supplied reports, but they should be included before any broader claim that every currently validated material finding is resolved.- wallet: public_descriptors ignores the seed it's given #128: the exported
- added 2 commits that reference this issue
on Oct 5, 2026 2026-10-05 release-gate refresh after checking current GitHub heads, check rollups, review threads, and labels across the audit/release stack:
- docs: Replace contributor boilerplate #115: maintainer-requested contributor-guide rewrite is now at
2511af8. Codex reviewed substantive head3c13060and found one broken GitHub task-list URL; the one-line follow-up fixes it and that thread is resolved. Exact-head Python-package run 821 is queued. The remaining unresolved docs: Replace contributor boilerplate #115 thread is intentionally the responsible-human authorship/rewrite requirement. - docs: Trim offline signing to what Core's tutorial lacks #93: the latest maintainer cNACK is addressed at
1dcee71; the CipherStick/Tails-cloning paragraph is reverted, the requested permanent-offline boundary/tutorial wording is retained, and both fresh inline threads are resolved. Exact-head run 823 and one current-head Codex review are pending. - gui: Add optional graphical interface #65: its one unresolved thread remains intentionally open because the standalone parent really does drop restore mode on manual refresh. Mandatory stacked gui: Refresh empty wallets automatically #66 removes that transition and is green; the final GUI candidate must include both.
- Every other PR in the current docs: Prepare the v1 review handoff #38 audit/release matrix has a SUCCESS check rollup and zero unresolved review threads. Every PR in that matrix carries
gate: adversarial review; the area labels are also present on the security/runtime/GUI/API/release work.
This comment supersedes the umbrella body’s stale statement that current #115 is already CI-green. No material finding was reclassified by this refresh.
- docs: Replace contributor boilerplate #115: maintainer-requested contributor-guide rewrite is now at
Audit closeout update, 2026-10-05: the four supplied reports remain exhaustively mapped by #127 /
docs/security/adversarial-2026-10-04.md(hashes and source qualifications included). No material validated finding currently lacks a tracker or a focused fix/disposition. The final reviewer-handoff process finding (M4) intentionally remains issue #38 until the candidate tip is frozen; opening that PR earlier would immediately stale its required commit pins.Current machine state: the remaining runtime/security audit PR heads (#33, #57, #105, #99, #80, #81, #95, #126, #130, #132) are CI-green with no unresolved review threads and have current-head Codex/disclosed AI-assisted review evidence as recorded in #127. #116/#117 are also green and thread-clean. #115's contributor-guide content has been corrected through
2511af8; its only intentionally open thread is the repository policy requiring responsible-human authorship/rewrite, and a cancelled-job rerun is settling. #93 is release-documentation follow-up rather than a new audit finding: all maintainer review threads are resolved at1dcee71, Codex reviewed that exact head and reported no major issues, all executed matrix jobs pass, and the cancelled compatibility-job rerun is settling.Post-audit release-gate findings remain separately tracked: #128/#130 (unused unauthenticated descriptor API) and #129/#132 (
CorrectionEditdefault-rendering disclosure). Both focused fixes are mechanically verified/reviewed and must be included in the frozen candidate.Do not run the next full adversarial review yet. Human review/rewrite/integration remains the gating step, followed by the final handoff document, composed full-suite/Core/artifact qualification, GUI/Tails manual qualification, and then the fresh adversarial pass over the frozen library + CLI + GUI + recovery documentation.
Summary
- Added a dated security-audit status document recording the exhaustive supplied-report disposition, current candidate inputs, unresolved human-review requirements, and M4’s intentionally deferred frozen-tip handoff.
- Distinguished the post-audit descriptor API and
CorrectionEditdisclosure findings from the four supplied reports while requiring their fixes in the frozen candidate. - Documented the required closeout sequence and explicitly deferred the next full adversarial review until after human integration, candidate freeze, exact-artifact qualification, and GUI/Tails qualification.
- Linked the point-in-time status from the README while clarifying that it is not a completed-release or final-review claim.
- Committed the changes as
fe0f0ae(docs: record audit closeout gates) and created the requested PR metadata titled “Docs: record the 2026-10-05 audit closeout gates.”
Testing
- ✅
git diff --check - ✅
PYTHONPATH=src python -m pytest -q— 24 tests passed. ⚠️ python -m pytest -q— initial environment-only collection warning because the src-layout package was not installed or placed onPYTHONPATH; the explicitPYTHONPATH=srcrun passed.- ✅
git status --short --branch— clean working tree after commit.
Current-state reconciliation (2026-10-06):
- Merged fixes from the supplied four reports: Wallet: use Core for setup and remove test crypto deps #7 (BL1), build: Include all code license notices #15 (BL2), generation: Validate supplied BIP32 seeds #16 (supplied-byte part of M8), bip93: Redact artifact string rendering #25 (H1), correct: Interpret mixed-case damage #42 (M1/L1), and wallet: Require the recorded fingerprint before import #57 (H2 CLI/library verify-before-mutate).
- Exact-head reviewed + green and ready for responsible-human integration: bip93: reject HRPs longer than 83 characters #33 (H3, after its foundation stack), wallet: Distinguish unavailable Bails checks #80, wallet: Check existing seed before sharing #81, cli: Announce recovery secret switch #95 (M3), docs: Label benchmark evidence historical #116 (M6), tests: Match Core initializer signature #117, generation: Validate roots before re-sharing #126 (remaining M8 path), docs: Close adversarial audit tracking gaps #127 (audit ledger/contract dispositions), and correct: Redact correction edit characters #132 (post-audit disclosure hardening).
- cli: Remove unreachable recovery and search paths #105 and cli: Name the real Bitcoin Core requirement when it is missing #99 were just rewritten/restacked onto the merged wallet: Require the recorded fingerprint before import #57 base. Their live heads are now
9ea48deand27f4cc6; exact-head package CI is green for both and cli: Name the real Bitcoin Core requirement when it is missing #99's Core fixture is green. One fresh Codex review has been requested on each rewritten head because the prior ACKs named the superseded SHAs. - docs: Replace contributor boilerplate #115 is content-ACKed and CI-green but still has the existing human-authorship P1; it needs a responsible-human rewrite/squash before merge.
- gui: Require wallet identity before restore #118 has a current-head ACK and green package CI; supported-Tails/manual GUI qualification remains before the frozen adversarial pass.
- api: Remove unused public entry points #130/correct: Redact correction edit characters #132 are post-audit findings, not findings from the supplied reviewers. api: Remove unused public entry points #130 remains draft/conflicted after wallet: Require the recorded fingerprint before import #57 and needs restacking before frozen-candidate integration; correct: Redact correction edit characters #132 is clean.
- docs: Label benchmark evidence historical #116 is no longer draft; its historical-evidence wording is ready for human review.
Remaining integration order for the restore follow-ups is now #105 → #99 → #80 → #81 → #95; #57 is already merged.
No supplied material finding lacks a tracker or a focused fix/defer path. The remaining work is human integration, the #115 authorship repair, the #130 restack, frozen-tip full/GUI qualification, and then one fresh adversarial review of the frozen library/CLI/GUI/docs candidate.
Security-audit umbrella originating from
c118a834. The attachedreport.mdremains historical evidence; its Finding 1 (opaque-HRP deadline bypass) was subsequently falsified and is not tracked as a defect.This issue also tracks the three independent adversarial reviews and their
consolidated-review.mddisposition so the release gate has one exhaustive map rather than only the highest-severity implementation findings.Material validated findings
bip32/Coincurve dependency; current CI installs.[dev]and runs the suite. Draft docs: Replace contributor boilerplate #115 rewrites the contributor on-ramp around that actual setup.pyproject.tomlincludesLICENSES/*andMANIFEST.inrecursively includesLICENSES.repr/strdisclosure: fixed by merged bip93: Redact artifact string rendering #25 / Redact secret-bearing artifact string representations #22; rendering is redacted and.textis the explicit export path.reviewability-v1; Define correction exit statuses #45 supplies the distinct exit-status contract.ms32Core requirement during recovery/correction: retained as an explicitms32trust-boundary choice rather than silently weakened. cli: Name the real Bitcoin Core requirement when it is missing #84 / cli: Name the real Bitcoin Core requirement when it is missing #99 now names Bitcoin Core 32+/RPC, explains what Core supplies, pointssecret/share/correctto the Core-independentcodex32fallback, connects before damaged-seed search, stays under<5200, has current-head Codex ACK, and exact-head Python-package + Core-fixture CI are green.6802d86and directs reviewers to current CI/budget evidence. Exact-head CI is green and Codex ACKed it.python-package.ymlruns bothpython -O -m pytest -qandtools/verify_correction_constants.py.MasterSeedpassed toCreationCeremony.from_secret. New generation: Validate existing Bitcoin roots before sharing #125 / focused generation: Validate roots before re-sharing #126 rejects an invalid BIP32 root before identifier randomness or share creation; CL secrets are unchanged. The behavioral patch passed all 953 tests normally and underpython -O, Ruff, strict mypy, and fresh wheel/sdist checks. Current head8cd584eis CI-green and has a current-head Codex ACK; human integration remains.1differs from the BIP-93 reference decoder: accepted v1 parser divergence, not a secret-safety defect. docs: Close adversarial audit tracking gaps #127 records it in the API guide; docs: Prepare the v1 review handoff #38 requires the frozen reviewer handoff to carry that qualification.Low / reviewability findings
correctoverloaded exit 1: cli: Define stable correct exit statuses #39 / Define correction exit statuses #45 defines stable 0/1/2/3 statuses; integrated with correct: Interpret mixed-case damage #42.assertstatements vanish under-O: no reproduced security failure. The release control is ci: Test optimized mode and correction constants #40's full optimized suite in CI; docs: Close adversarial audit tracking gaps #127 explicitly documents that public input/identity gates use exceptions and remaining assertions concern validated internal state. docs: Prepare the v1 review handoff #38 must call out the optimized gate in reviewer evidence.<5200; docs: Replace contributor boilerplate #115 documents the metric/change-control contract and the existing enforcement test remains authoritative. GUI has a separate authorized cap.Tiny / edge findings
__all__count: api: Remove unused public entry points #130 supersedes wallet: Privatize Core descriptor records #64 by removing obsoletecore_descriptorsplus unused fresh Core Lightning generation exports; its intended package surface is 22 names, while the api: Expose reference-vector helpers #53 supported reference-vector helpers remain module-level and do not expandcodex32.__all__;MANIFEST.inprovenance exclusion: fixed on currentreviewability-v1(d6a9f99);docs/planning/; finished security audit material belongs underdocs/security/;1.0.0rc1: final release metadata remains governed by Gate v1 publication on final-RC integration and built-artifact verification #5/release: Qualify exact artifacts before publish #52; no security behavior depends on this Trove label;pyproject.tomlblank line: no longer present on currentreviewability-v1;.codex/config.toml: removed by codex: Remove project-level agent overrides #41;Current release-gate disposition
The audit verdict itself is focused PR #23. Security-contract wording is #59 / #4. Exact-artifact release qualification is #52 / #5. No material validated finding from the supplied reviews now lacks a tracker or a focused fix/defer path. The newly reproduced existing-secret part of M8 is #125/#126, not already fixed by #16. #127 adds the exhaustive original/consolidated finding ledger at
docs/security/adversarial-2026-10-04.md, with all numbered findings, nits, source qualifications, and explicit limitations.Post-audit Codex review found two additional release-gate issues that are tracked separately rather than attributed to the supplied reviewers: #128 / focused #130 removes an unused descriptor API that could return data from a mismatched Core wallet, and #129 / focused #132 redacts correction-edit characters from default candidate rendering. Both must be integrated before the frozen-candidate adversarial pass.
Keep this umbrella open until the remaining focused PRs are human-integrated, the GUI restore delta is replayed and manually qualified, #23 is on the frozen candidate, and a fresh adversarial review covers the frozen library/CLI/GUI/user-documentation tips.
Local composed proof on 2026-10-04 applied the reviewed #99 delta to the #53 + #126 behavioral tree and #127's AST-equivalent tuple cleanup: 59 focused Core/CLI/disclosure tests passed; the library measured 5,198 logical review lines; the official Bitcoin Core 32.0rc2 regtest fixture passed (
/Satoshi:32.0.0/). This is limited interaction evidence, not a frozen-candidate full-suite or GUI qualification. The downloaded archive's SHA-256 matched the official HTTPS checksum list; this run did not newly verify its PGP signature.