Skip to content

bip93: Reject non-ASCII normalized input - #13

Open
BenWestgate wants to merge 2 commits into
codex/v1-core-state-validationfrom
codex/v1-ascii-normalization
Open

BenWestgate wants to merge 2 commits into
codex/v1-core-state-validationfrom
codex/v1-ascii-normalization

Conversation

@BenWestgate

@BenWestgate BenWestgate commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner

What

  • reject non-ASCII identifiers, share indices, derivation targets, and raw CLI headers before lowercasing;
  • prevent non-ASCII confirmation input from aliasing valid ASCII card text;
  • retain ASCII case-insensitive and whitespace-tolerant confirmation behavior, including per-character progressive feedback.

Why

Unicode Kelvin sign K lowercases to ASCII k. It could therefore cross header/generation validation boundaries or make a card transcription appear valid even though the parser rejects it. This adapts Rob1Ham#9 onto the current target.

Current head 4f01561 is the same two human-authored patches mechanically replayed onto refreshed #12; both stable patch-ids are unchanged/; all inline review threads are resolved. The small duplicated ASCII-fold helper is intentionally left to #53, which already owns the overlapping vector/API cleanup and will centralize it once this behavior fix lands.

Validation

  • full suite: 864 passed;
  • optimized suite: 864 passed under python -O;
  • Ruff check/format and strict mypy: pass;
  • exact-head GitHub Python-package run 335: success.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 364e3d5060

ℹ️ 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".

Comment thread src/codex32/generation.py Outdated
Comment thread src/codex32/generation.py
@BenWestgate
BenWestgate force-pushed the codex/v1-ascii-normalization branch from 364e3d5 to b064fb5 Compare September 21, 2026 09:39
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@BenWestgate

Copy link
Copy Markdown
Owner Author

Not a very likely situation bot OK Rob

@BenWestgate

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b064fb5aa2

ℹ️ 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".

Comment thread src/codex32/generation.py Outdated

@BenWestgate BenWestgate left a comment •

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, outside of the nit codex found

@BenWestgate BenWestgate added gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. area: bip93 BIP93 encoding, checksum, parsing, and format rules. bug Something isn't working labels Sep 24, 2026

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated review (Claude), posted at the maintainer's request.

ACK 1b17d8a, one nit.

  • ASCII checks land before every .lower() on header/index/target/confirmation input; bech32_decode was already guarded by _validate_single_case_ascii.
  • Merges cleanly with #11 and #33.

Comment thread src/codex32/generation.py

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated review, posted at the maintainer's request.

ACK d2f8589.

@BenWestgate BenWestgate added the area: correction Correction engine and correction UX. label Sep 30, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Owner Author

Agent release-gate review at exact head d2f8589: the Unicode-normalization fix closes the Kelvin-sign/lookalike boundary consistently. Non-ASCII identifiers, indices, derivation targets and raw CLI headers are rejected before normalization; confirmation folds ASCII case only, so Unicode lookalikes cannot alias valid card text while ordinary ASCII case-insensitive confirmation remains intact. Exact-head Python-package run 36361133118 succeeded. No remaining code-review blocker found. The duplicated ASCII-only lower helper is appropriately deferred to #53's final API-boundary refresh.

@BenWestgate
BenWestgate force-pushed the codex/v1-ascii-normalization branch from d2f8589 to 4f24148 Compare October 1, 2026 18:52
@BenWestgate
BenWestgate changed the base branch from reviewability-v1 to codex/v1-core-state-validation October 1, 2026 18:52
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

1 similar comment
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@BenWestgate
BenWestgate force-pushed the codex/v1-core-state-validation branch from afee360 to a860035 Compare October 1, 2026 19:08
@BenWestgate
BenWestgate force-pushed the codex/v1-ascii-normalization branch from 4f24148 to 49f5cdc Compare October 1, 2026 19:08
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Reject non-ASCII identifiers, indices, and raw CLI creation headers before lowercasing so Unicode lookalikes cannot normalize into valid Bech32 symbols.

Make whole-card confirmation fold ASCII case only. Non-ASCII lookalikes now stay visibly mismatched and enter the normal retry flow for shared, unshared, and derived cards instead of being accepted or reaching the ceremony callback as an apparent match.

Document the ASCII-only confirmation boundary in the API and security model.

Validation: 867 tests passed normally and under python -O; Ruff check and format passed; strict mypy passed; git diff --check passed.
@BenWestgate
BenWestgate force-pushed the codex/v1-core-state-validation branch from a860035 to bbcfb56 Compare October 2, 2026 00:42
@BenWestgate
BenWestgate force-pushed the codex/v1-ascii-normalization branch from 49f5cdc to 4f01561 Compare October 2, 2026 00:42
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: bip93 BIP93 encoding, checksum, parsing, and format rules. area: correction Correction engine and correction UX. bug Something isn't working gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant