Skip to content

Fix random-password entropy and exclusions - #7

Merged
dk3yyyy merged 2 commits into
mainfrom
fix/random-password-entropy
Jul 24, 2026
Merged

Fix random-password entropy and exclusions#7
dk3yyyy merged 2 commits into
mainfrom
fix/random-password-entropy

Conversation

@dk3yyyy

@dk3yyyy dk3yyyy commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • generate random passwords uniformly from the valid output space using cryptographic rejection sampling
  • calculate exact constrained entropy with inclusion-exclusion
  • reject selected character classes emptied by exclusions
  • share generation and entropy logic between the CLI and web API
  • document the corrected entropy model and compile-check the new module

Verification

  • 91 tests passed
  • Python compilation passed for main.py, web.py, passphrases.py, and passwords.py
  • git diff --check passed
  • two independent reviews found no security concerns or logic errors

Summary by CodeRabbit

  • New Features

    • Added secure random-password generation with exact entropy calculations.
    • Entropy now accounts for selected character types, exclusions, and ambiguous-character filtering.
    • Added validation when exclusions empty a required character category or the requested length is too short.
    • Web and CLI password generation now report consistent constrained entropy.
  • Documentation

    • Clarified how displayed entropy is calculated and what it represents.
  • Tests

    • Expanded coverage for password generation, entropy calculations, validation, and web/CLI behavior.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@dk3yyyy, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 50 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 27d183bf-61d8-40d0-964a-a05978481496

📥 Commits

Reviewing files that changed from the base of the PR and between 2c344d5 and 39ececd.

📒 Files selected for processing (1)
  • README.md
📝 Walkthrough

Walkthrough

The change adds a shared password utility module with filtered generation and exact entropy calculation, integrates it into CLI and web flows, adds focused tests, and updates entropy documentation and compilation checks.

Changes

Password generation and entropy

Layer / File(s) Summary
Shared password utilities and validation
passwords.py, tests/test_passwords.py
Adds filtered character-set selection, length validation, rejection-sampled generation, inclusion–exclusion entropy calculation, and tests for valid and invalid configurations.
CLI and web entropy integration
main.py, web.py, tests/test_main.py, tests/test_web.py
Uses shared generation and entropy functions, reuses precomputed entropy for random outputs, and tests CLI constraints, validation, and web formatting.
Documentation and compilation coverage
.github/workflows/ci.yml, README.md
Documents constrained entropy semantics and includes passwords.py in compilation checks.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI_or_Web
  participant passwords
  participant Output
  CLI_or_Web->>passwords: calculate_password_entropy(length, flags, exclusions)
  passwords-->>CLI_or_Web: exact entropy
  CLI_or_Web->>passwords: generate_password(length, flags, exclusions)
  passwords-->>CLI_or_Web: generated password
  CLI_or_Web->>Output: render password and entropy
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing random-password entropy handling and exclusion behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/random-password-entropy

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (2)
README.md (1)

152-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Project structure tree is now stale.

This PR adds passwords.py and tests/test_passwords.py, but the documented tree still omits both.

📝 Proposed doc update
 password-generator/
 ├── .github/workflows/ci.yml
 ├── docs/web-ui.png
 ├── main.py
 ├── passphrases.py
+├── passwords.py
 ├── web.py
 ├── templates/index.html
 ├── tests/
 │   ├── test_main.py
 │   ├── test_passphrases.py
+│   ├── test_passwords.py
 │   └── test_web.py
 ├── wordlists/
 │   ├── eff_large_wordlist.txt
 │   └── README.md
 ├── pyproject.toml
 └── README.md
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 152 - 171, Update the Project structure tree in
README.md to include the new passwords.py module and tests/test_passwords.py
entry, placing each alongside the corresponding existing source and test files.
main.py (1)

216-237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate generate_passphrase implementation in main.py and web.py. Both files independently implement the same word-selection/capitalize/number-suffix logic; this PR already centralized the equivalent random-password logic into passwords.py, but passphrase generation was left duplicated.

  • main.py#L216-L237: extract this generate_passphrase implementation into a shared module (e.g. alongside calculate_passphrase_entropy in passphrases.py) and import it here instead of defining it locally.
  • web.py#L27-L39: remove this duplicate generate_passphrase and import the same shared implementation used by the CLI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@main.py` around lines 216 - 237, The passphrase-generation logic is
duplicated across the CLI and web paths. In main.py lines 216-237, move
generate_passphrase into the shared passphrases.py module alongside
calculate_passphrase_entropy and import it instead of defining it locally; in
web.py lines 27-39, remove the duplicate generate_passphrase and import the same
shared implementation, preserving its existing word selection, capitalization,
number suffix, validation, and return behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@main.py`:
- Around line 216-237: The passphrase-generation logic is duplicated across the
CLI and web paths. In main.py lines 216-237, move generate_passphrase into the
shared passphrases.py module alongside calculate_passphrase_entropy and import
it instead of defining it locally; in web.py lines 27-39, remove the duplicate
generate_passphrase and import the same shared implementation, preserving its
existing word selection, capitalization, number suffix, validation, and return
behavior.

In `@README.md`:
- Around line 152-171: Update the Project structure tree in README.md to include
the new passwords.py module and tests/test_passwords.py entry, placing each
alongside the corresponding existing source and test files.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 55968558-3e68-44cc-8f9a-1bceea7da493

📥 Commits

Reviewing files that changed from the base of the PR and between adc2eb5 and 2c344d5.

📒 Files selected for processing (8)
  • .github/workflows/ci.yml
  • README.md
  • main.py
  • passwords.py
  • tests/test_main.py
  • tests/test_passwords.py
  • tests/test_web.py
  • web.py

@dk3yyyy
dk3yyyy merged commit 5f2305f into main Jul 24, 2026
2 checks passed
@dk3yyyy
dk3yyyy deleted the fix/random-password-entropy branch July 24, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant