Skip to content

feat: add util fp-finder command #219

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 26, 2025
Merged

feat: add util fp-finder command #219

merged 1 commit into from
Apr 26, 2025

Conversation

S0obi
Copy link
Contributor

@S0obi S0obi commented Apr 8, 2025 •
edited
Loading

As proposed in #181 (comment), I would like to propose a new command, that can be used like crs-toolchain util fp-finder restricted-files.data.

Here are the main steps :

  1. Download https://raw.githubusercontent.com/dwyl/english-words/master/words_alpha.txt a popular english dictionnary in "~/.crs-toolchain"
  2. Remove any words from input file that is contained in the dictionnary (+ extended in a static file)
  3. Sort and/or remove duplicates

Main goal of this command is also to fix coreruleset/coreruleset#3916 (which will be done if this PR is merged).

Resolves: #181

theseion
theseion requested changes Apr 12, 2025
View reviewed changes
Copy link
Contributor

@theseion theseion left a comment

Choose a reason for hiding this comment

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

Thanks! Looks pretty good already.

@S0obi
Copy link
Contributor Author

S0obi commented Apr 13, 2025

Thanks! Looks pretty good already.

Cool! I addressed last comments in different commits for easier review. Let me know if you have any other remarks 👍

@S0obi S0obi requested a review from theseion April 13, 2025 12:31
theseion
theseion requested changes Apr 13, 2025
View reviewed changes
@fzipi fzipi requested a review from theseion April 20, 2025 13:23
@S0obi S0obi force-pushed the feature/add-fp-finder-util-command branch 3 times, most recently from ad86a7a to 23fe1e2 Compare April 23, 2025 18:42
theseion
theseion requested changes Apr 24, 2025
View reviewed changes
@theseion
Copy link
Contributor

That's pretty good @S0obi, just some cleanup.

@S0obi S0obi force-pushed the feature/add-fp-finder-util-command branch from f419685 to 46a11a2 Compare April 24, 2025 19:49
@S0obi
Copy link
Contributor Author

S0obi commented Apr 24, 2025

That's pretty good @S0obi, just some cleanup.

Thanks! I addressed your comments and fixed the tests

@S0obi S0obi requested a review from theseion April 24, 2025 19:49
theseion
theseion requested changes Apr 25, 2025
View reviewed changes
@S0obi S0obi force-pushed the feature/add-fp-finder-util-command branch from 46a11a2 to 48a14d9 Compare April 25, 2025 12:26
@S0obi S0obi requested a review from theseion April 25, 2025 12:26
@theseion
Copy link
Contributor

Thank you very much @S0obi! Really appreciate another Go dev helping out :).
I have one request for your (hopefully) next contribution: please do not rebase unless you're cleaning up at the end or it is necessary for the builds. Rebasing breaks the review functionality, which makes it hard to review individual changes (the larger the PR the worse).

@theseion theseion merged commit 0b85893 into coreruleset:main Apr 26, 2025
3 checks passed
@theseion theseion added the enhancement New feature or request label Apr 26, 2025
@S0obi S0obi deleted the feature/add-fp-finder-util-command branch April 26, 2025 05:58
This was referenced Apr 26, 2025
Closed
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fzipi fzipi fzipi left review comments

@theseion theseion theseion approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Invalid bash snippet in restricted-upload.data fp-finder: tool to find potential false positives in data files
3 participants
@S0obi @theseion @fzipi