Repository navigation
Security audit: python-codex32 findings #20
Description
Security-audit umbrella originating from c118a834. The attached report.md remains 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.md disposition so the release gate has one exhaustive map rather than only the highest-severity implementation findings.
Material validated findings
- BL1 — documented dev setup failed: fixed by merged Wallet: use Core for setup and remove test crypto deps #7 removing the test-only
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. - BL2 — BSD notice omitted from distributions: fixed by merged build: Include all code license notices #15; current
pyproject.tomlincludesLICENSES/*andMANIFEST.inrecursively includesLICENSES. - H1 — artifact
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. - H2 — wallet import before recovery identity: tracked by Verify recovered wallet identity before import #26/Verify CLI recovery identity before wallet import #30. CLI/library release gate is wallet: Require the recorded fingerprint before import #57 plus cli: Remove unreachable recovery and search paths #105/wallet: Distinguish unavailable Bails checks #80/wallet: Check existing seed before sharing #81/cli: Announce recovery secret switch #95; GUI replay is draft gui: Require wallet identity before restore #118. Current reviewed heads enforce verify-before-mutate; final human integration and GUI/Tails qualification remain.
- H3 — overlong HRPs: bip93: Reject human-readable parts longer than 83 characters #32 / focused bip93: reject HRPs longer than 83 characters #33; current head is CI-green, all review threads are resolved, and it has a current-head Codex ACK.
- M1 — mixed-case correction reported no result: correct: Handle mixed-case damage explicitly #37 / correct: Interpret mixed-case damage #42, already integrated into
reviewability-v1; Define correction exit statuses #45 supplies the distinct exit-status contract. - M2 —
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. - M3 — supplying S mid-recovery silently discarded shares: cli: Announce recovery secret switch #95 keeps the deliberate mode switch but announces it. The focused patch is reviewed; human rewrite/squash/integration remains.
- M4 — reviewability/narrative gap: docs: Prepare the v1 review handoff #38 owns the frozen-candidate review guide, exact commit pins, review order and final fresh adversarial review. Draft Add comprehensive codex32 CLI, correction, and wallet integration #94 is only a moving placeholder and is not the final integration PR.
- M5 — inherited Bitcoin Core contributor boilerplate: draft docs: Replace contributor boilerplate #115 is focused, CI-green and content-ACKed by Codex; responsible-human rewrite/squash is the remaining policy step.
- M6 — stale benchmark numbers: draft docs: Label benchmark evidence historical #116 labels the measurements/raw files as historical evidence tied to commit
6802d86and directs reviewers to current CI/budget evidence. Exact-head CI is green and Codex ACKed it. - M7 — optimized tests/correction-constant verifier absent from CI: ci: Test optimized mode and correction constants #40 is closed; current
python-package.ymlruns bothpython -O -m pytest -qandtools/verify_correction_constants.py. - M8 — supplied-seed paths skipped BIP32-root validity: merged generation: Validate supplied BIP32 seeds #16 fixes supplied raw bytes, but did not validate an existing
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. - M9 — threshold digit
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
- L1 —
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. - L2 — unreachable CLI/search branches: Remove pre-review CLI dead code #46 is integrated; focused cli: Remove unreachable recovery and search paths #105 removes the remaining reviewed dead paths without behavior change.
- L3 —
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. - L4 — host-specific raw benchmark data: retained intentionally as historical evidence rather than current qualification data; docs: Label benchmark evidence historical #116 makes that status explicit.
- L5 — bespoke/nearly-full review budget: the maintainer-authorized library cap is
<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. - L6 — commit hygiene: final stacked AI-assisted follow-ups require responsible-human rewrite/squash under the repository AI policy; docs: Prepare the v1 review handoff #38 tracks this human-only integration step.
Tiny / edge findings
- package
__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__; - Core test-double signature drift: one-line draft tests: Match Core initializer signature #117, exact-head CI-green and Codex-ACKed;
- stale
MANIFEST.inprovenance exclusion: fixed on currentreviewability-v1(d6a9f99); - ignored provenance/plans: deliberate local-only unfinished work belongs under ignored
docs/planning/; finished security audit material belongs underdocs/security/; - Beta classifier with
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; - stray
pyproject.tomlblank line: no longer present on currentreviewability-v1; - repository
.codex/config.toml: removed by codex: Remove project-level agent overrides #41; - Bitcoin Core RC documentation links: docs: Trim offline signing to what Core's tutorial lacks #93 moves the user guidance to the maintained upstream tutorial path;
- budget-test discoverability: docs: Prepare the v1 review handoff #38 requires the final reviewer guide to point directly at the enforcement tests; no runtime move is required;
- private cross-module helper imports: api: Stop requiring private vector helpers #49 / api: Expose reference-vector helpers #53 publishes the vector helpers that reviewers/users actually need and documents the remaining intentional internal couplings;
- stale sibling branch names: repository housekeeping only, not a release/security gate; do not treat those names as fix provenance.
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.
Activity
BenWestgate commentedon Sep 22, 2026
report.md
docs: Sync audit finding tracking
docs: Record security audit verdict
BenWestgate commentedon Sep 25, 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.
docs: Record security audit verdict
docs: Record security audit verdict
BenWestgate commentedon Oct 4, 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.
.[dev]install plus 950 normal/optimized tests). #115 remains a human-authorship/review item.repr/strleaks recovery text.textis the reveal path.reviewability-v1, with normal/optimized and frozen differential coverage.ms32 secret/share/correctCore dependency is confusing/blocking<5250cap commit and therefore is not release-ready under the current<5200gate. Restack/drop that cap commit before freeze.Smid-recovery silently abandons accepted shares6802d86and the host-specific measurements as historical evidence and points reviewers to current CI/budgets. Human review remains.python -Osuite and constant verifier ran green.1differs from reference decodercorrectoverloads exit 1assert-dependent invariants under-O<5200and the separate GUI gate. The later<5250proposal in #99/#100/#101/#104 is not being inferred as approved from PR text.__all__to 23; #53 publishes vector helpers only at owning modules. #38 pins the expected final count.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.
BenWestgate commentedon Oct 4, 2026
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.BenWestgate commentedon Oct 5, 2026
Audit-closure verification update (Codex, maintainer-authorized):
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.00aae9bis green and has a current-head automated Codex review. Itsdocs/audits/adversarial-2026-10-04.mdremains the exhaustive four-report disposition ledger.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.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.
BenWestgate commentedon Oct 5, 2026
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.BenWestgate commentedon Oct 5, 2026
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.
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.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.docs: Refresh adversarial audit status
docs: Record post-audit release gates
BenWestgate commentedon 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:
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.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.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.
BenWestgate commentedon Oct 5, 2026
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.
chatgpt-codex-connector commentedon Oct 5, 2026
Summary
CorrectionEditdisclosure findings from the four supplied reports while requiring their fixes in the frozen candidate.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 --checkPYTHONPATH=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.View task →
BenWestgate commentedon Oct 6, 2026
Current-state reconciliation (2026-10-06):
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.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.