fix: warn that copied ecash is spendable - #560
Conversation
The copy button on the ecash send screen hands a bearer instrument to the clipboard, which other apps, input methods and clipboard-sync services can read. Nothing on screen said so (fixes fedimint#534, security audit L23). Adds a persistent warning directly above the token and its copy button, so it is read before the tap rather than shown as a toast afterwards. A persistent note is used instead of a one-time dialog because the risk applies to every copy, and because it needs no persisted "already warned" state. The screen is presented in a fixed-height bottom sheet and was a plain non-scrolling Column already holding a square QR, so the outer padding becomes a SingleChildScrollView to keep the added banner from overflowing on shorter screens.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3259bbf1e6
ℹ️ 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".
| Expanded( | ||
| child: Text( | ||
| context.l10n.ecashClipboardWarning, | ||
| style: const TextStyle(color: Colors.orange), |
There was a problem hiding this comment.
The Colors.orange foreground is rendered on Colors.orange.shade100, producing a contrast ratio of only about 1.7:1—well below the 4.5:1 expected for normal-sized text. This makes the new security warning difficult to read, especially for users with low vision, undermining the purpose of warning users before they copy spendable ecash; use a substantially darker foreground color.
Useful? React with 👍 / 👎.
Colors.orange on an orange.shade100 background gives a contrast ratio of roughly 1.7:1, well under the 4.5:1 WCAG AA asks for body text. A security warning that is hard to read defeats its own purpose, so use brown.shade900 for the text and icon instead, which keeps the warm tone at about 10.9:1. A darker orange is not sufficient: orange.shade900 only reaches 3.0:1 and deepOrange.shade900 4.4:1, both still short of AA. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 participants
The copy button on the ecash send screen hands a bearer instrument to the clipboard, which other apps, input methods and clipboard-sync services can read. Nothing on screen said so (fixes #534, security audit L23).
Adds a persistent warning directly above the token and its copy button, so it is read before the tap rather than shown as a toast afterwards. A persistent note is used instead of a one-time dialog because the risk applies to every copy, and because it needs no persisted "already warned" state.
The screen is presented in a fixed-height bottom sheet and was a plain non-scrolling Column already holding a square QR, so the outer padding becomes a SingleChildScrollView to keep the added banner from overflowing on shorter screens.