Skip to content

feat: implement centralized passphrase validation system#44

Merged
S4tvara merged 5 commits intoS4tvara:mainfrom
Deepam02:feature/centralized-passphrase-validation
Oct 6, 2025
Merged

feat: implement centralized passphrase validation system#44
S4tvara merged 5 commits intoS4tvara:mainfrom
Deepam02:feature/centralized-passphrase-validation

Conversation

@Deepam02
Copy link
Copy Markdown
Contributor

@Deepam02 Deepam02 commented Oct 3, 2025

Closes #38

Add centralized passphrase validation

Fixes scattered weak validation across the app. Now passphrases need 12+ chars with proper character requirements and common password detection.

Changes:

  • Upgraded min length: 8 → 12 chars
  • Added character requirements (upper/lower/digits/symbols)
  • Detects common passwords with zxcvbn
  • Works on all passphrase prompts

Comment thread internal/passphrase/validation.go Outdated
@S4tvara
Copy link
Copy Markdown
Owner

S4tvara commented Oct 3, 2025

image

The go.mod is missing the required packages, which is causing all the CI builds to fail.
Please add them, and I’ll do another review.

@MrKeiKun
Copy link
Copy Markdown
Contributor

MrKeiKun commented Oct 3, 2025

Remove 231f6e9 on source branch

@S4tvara
Copy link
Copy Markdown
Owner

S4tvara commented Oct 4, 2025

Hi @Deepam02,
Thanks for the contribution! Could you please fix all the linter issues reported by the CI?
Happy to help if you get stuck anywhere.

Cheers,
Nilay

@S4tvara
Copy link
Copy Markdown
Owner

S4tvara commented Oct 5, 2025

Hey @Deepam02,
Are you still working on this issue, or should I go ahead and close the PR?

Cheers,
Nilay

@Deepam02
Copy link
Copy Markdown
Contributor Author

Deepam02 commented Oct 5, 2025

yes i am still working on this issue.
to fix the lint issues i just need to use gofmt right?
or is there some other tool i can use.

@Deepam02
Copy link
Copy Markdown
Contributor Author

Deepam02 commented Oct 5, 2025

hi @SubstantialCattle5
i have fixed the lint and dependency issues.
the checks should now pass.

@S4tvara S4tvara requested a review from MrKeiKun October 5, 2025 14:10
@Deepam02
Copy link
Copy Markdown
Contributor Author

Deepam02 commented Oct 5, 2025

the tests should pass now

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 5, 2025

Codecov Report

❌ Patch coverage is 80.09479% with 42 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/ui/passphrase.go 0.00% 20 Missing ⚠️
internal/passphrase/hybrid.go 87.50% 8 Missing and 5 partials ⚠️
internal/passphrase/validation.go 92.85% 5 Missing and 1 partial ⚠️
internal/encryption/passphrase/prompt.go 0.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@MrKeiKun
Copy link
Copy Markdown
Contributor

MrKeiKun commented Oct 5, 2025

@Deepam02 rebase the branch to remove merge commits due to using git merge.

@Deepam02
Copy link
Copy Markdown
Contributor Author

Deepam02 commented Oct 5, 2025

@Deepam02 rebase the branch to remove merge commits due to using git merge.

sure, will do that.
also do i need to add tests?

@Deepam02 Deepam02 force-pushed the feature/centralized-passphrase-validation branch from 1e5d8a5 to ab808f8 Compare October 5, 2025 15:54
@MrKeiKun
Copy link
Copy Markdown
Contributor

MrKeiKun commented Oct 6, 2025

@Deepam02 rebase the branch to remove merge commits due to using git merge.

sure, will do that.
also do i need to add tests?

Sure thing. Most likely in the future, we will enforce all new function define must have its unit test to exist as well.

@S4tvara
Copy link
Copy Markdown
Owner

S4tvara commented Oct 6, 2025

Hi @Deepam02 ,
The current PR has 0% code coverage please note that the target coverage is 11.60%, rest looks good to me.
Cheers!
Nilay

@Deepam02
Copy link
Copy Markdown
Contributor Author

Deepam02 commented Oct 6, 2025

Hi @Deepam02 , The current PR has 0% code coverage please note that the target coverage is 11.60%, rest looks good to me. Cheers! Nilay

yes i was working on this only now the codecov check should pass

@S4tvara S4tvara merged commit 1b3142f into S4tvara:main Oct 6, 2025
28 checks passed
@S4tvara
Copy link
Copy Markdown
Owner

S4tvara commented Oct 6, 2025

Hi @Deepam02,
I've reviewed the code and found no issues.
Merging it into the main branch.

I was also planning on updating the UI to support showing the new validation passphrase requirements to the user. Let me know if you're interested in picking it up — it will be created as a new issue, of course.

btw Thanks for the stellar contribution xD

Cheers,
Nilay

@Deepam02 Deepam02 deleted the feature/centralized-passphrase-validation branch October 7, 2025 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add common validation for passphrase length and requirements

3 participants