Conversation
Add a new `mask_on_submit` flag that shows input in cleartext while typing but masks it with asterisks after submission. Useful for PINs where the user needs to see what they're typing but the value should not remain visible in terminal scrollback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a mask_on_submit option to the Input component, a useful feature for handling sensitive inputs like PINs. However, the implementation of the masking logic in render_success introduces an information disclosure vulnerability, leaking the length and byte-count of passwords in the terminal scrollback, which is a regression from previous fixed-length masking. The change also contradicts the pull request's own test plan regarding password functionality. A specific suggestion has been provided to address this while preserving the new functionality.
| if self.password || self.mask_on_submit { | ||
| "*".repeat(self.input.len().max(1)) | ||
| } else { |
There was a problem hiding this comment.
The new masking logic in render_success introduces an information disclosure vulnerability, contradicting the test plan's goal of preserving password(true) behavior. Specifically: 1. Password Length Leak: The current implementation leaks the exact password length by using the input's actual length instead of the previous fixed-length mask (12 asterisks). 2. Byte-Count Leak: Using .len() instead of .chars().count() for mask_on_submit reveals information about character types (e.g., multi-byte characters like Emojis). 3. Misleading Empty Input: The .max(1) logic renders an empty input as a single asterisk, which is inconsistent. The logic should differentiate between password and mask_on_submit. For password, a fixed-length mask should be used. For mask_on_submit, .chars().count() should be used for accurate character counting.
Greptile SummaryThis PR adds a Key implementation details:
Confidence Score: 5/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User types input] --> B{password?}
B -- yes --> C[render_input: mask each char with '*']
B -- no --> D[render_input: show plaintext]
C --> E[User submits]
D --> E
E --> F[render_success]
F --> G{password?}
G -- yes --> H["Display '************' (12 fixed asterisks)"]
G -- no --> I{mask_on_submit?}
I -- yes --> J["Display '*' × chars().count()"]
I -- no --> K[Display plaintext input]
Last reviewed commit: 0779446 |
Password mode now shows 12 asterisks (original behavior) to avoid revealing password length. mask_on_submit uses actual input length without .max(1) so empty input correctly shows nothing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Byte length produces too many asterisks for multi-byte UTF-8 input. Use character count to match the rest of the codebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Also document that password takes precedence over mask_on_submit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
mask_on_submitflag toInputthat shows input in cleartext while typing but masks with asterisks after submissionpasswordwhich masks during typing tooTest plan
.mask_on_submit(true), verify text is visible while typing****password(true)behavior is unchanged🤖 Generated with Claude Code
Note
Low Risk
Small, localized change to terminal rendering behavior with added tests; minimal risk aside from potential expectations around how masking appears after submit.
Overview
Adds a new
mask_on_submitflag (and builder method) toInputto support post-submit-only masking, intended for cases like PIN entry where terminal scrollback shouldn’t reveal the value.Updates
render_successto mask output based on precedence (passwordmasks with a fixed 12*, otherwisemask_on_submitmasks to input length), and adds unit tests covering masked/emptymask_on_submitoutput and verifyingpasswordsuccess rendering remains consistent.Written by Cursor Bugbot for commit 0779446. This will update automatically on new commits. Configure here.