Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update default_regexes.json to expand Password in URL detection #376

Merged
merged 7 commits into from Aug 30, 2022

Conversation

afammartino-godaddy
Copy link
Contributor

@afammartino-godaddy afammartino-godaddy commented Aug 5, 2022

To help us get this pull request reviewed and merged quickly, please be sure to include the following items:

  • Tests (if applicable)
  • Documentation (if applicable)
  • Changelog entry
  • A full explanation here in the PR description of the work done

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Backward Compatibility

Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the API in any way?

  • Yes (backward compatible)
  • No (breaking changes)

Issue Linking

#375

What's new?

Update the "Password in URL" default_regexes.json to allow for usernames/passwords of length 40 and remove the requirement of an fqdn/port/location/query_params of length less than 100.

Notes

I would like additional help/guidance on testing the performance impact of this change. I did some runtime testing and saw minimal increase in cpu time.

Copy link
Contributor

@rbailey-godaddy rbailey-godaddy left a comment

Choose a reason for hiding this comment

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

I like what you did, but I believe I spotted another problem while I was trying to understand it. Does the comment below make sense?

tartufo/data/default_regexes.json Outdated Show resolved Hide resolved
Co-authored-by: Scott Bailey <72747501+rbailey-godaddy@users.noreply.github.com>
@sonarcloud
Copy link

sonarcloud bot commented Aug 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@afammartino-godaddy
Copy link
Contributor Author

Hey just wondering if anyone else had some thoughts on how to handle tests for these regex changes?

@afammartino-godaddy afammartino-godaddy requested review from rbailey-godaddy and removed request for irodelta and jmink-godaddy August 13, 2022 01:17
pyproject.toml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
afammartino-godaddy and others added 2 commits August 28, 2022 14:20
Co-authored-by: Esha Mayuri <88688228+emayuri-godaddy@users.noreply.github.com>
@emayuri-godaddy emayuri-godaddy merged commit 8cb779b into godaddy:main Aug 30, 2022
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.

None yet

4 participants