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

False positive, and problems with ignoring #13

Closed
aae42 opened this issue Apr 21, 2022 · 3 comments
Closed

False positive, and problems with ignoring #13

aae42 opened this issue Apr 21, 2022 · 3 comments

Comments

@aae42
Copy link

aae42 commented Apr 21, 2022

got a false positive on https://ola.hallengren.com/scripts/MaintenanceSolution.sql

./MaintenanceSolution.sql:84:)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON)

i went to ignore just this "secret" in the .secretsignore file like this:

[secrets]
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON)

but couldn't get it to ignore... had to add the whole file to .secretsignore

MaintenanceSolution.sql

[secrets]
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON)

this file's like 9100 lines long, so not ideal 😄

this is using secrets 0.1.2

@aae42 aae42 changed the title False positive, and how to ignore False positive, and problems with ignoring Apr 21, 2022
@sirwart
Copy link
Owner

sirwart commented Apr 21, 2022

Thanks for reporting! I'll investigate and fix soon!

@sirwart
Copy link
Owner

sirwart commented Apr 22, 2022

I just pushed a fix that will go out in 0.1.3 (e3f7bf0). The reason ignoring didn't work is because the key it detected was "ALLOW_ROW_LOCKS", so you would have had to add that to the secrets ignore. I think if the tool highlighted the subset of the row that was detected as the secret it would have been more obvious. The other issue is that ALLOW_ROW_LOCKS shouldn't have been detected as a non-random string in the first place, which I suspect was flagged because my common bigram dataset doesn't have enough capital letter bigrams.

@sirwart sirwart closed this as completed Apr 22, 2022
@aae42
Copy link
Author

aae42 commented Apr 22, 2022

I just pushed a fix that will go out in 0.1.3 (e3f7bf0). The reason ignoring didn't work is because the key it detected was "ALLOW_ROW_LOCKS", so you would have had to add that to the secrets ignore. I think if the tool highlighted the subset of the row that was detected as the secret it would have been more obvious. The other issue is that ALLOW_ROW_LOCKS shouldn't have been detected as a non-random string in the first place, which I suspect was flagged because my common bigram dataset doesn't have enough capital letter bigrams.

sounds good... highlight would be nice, but since you've already got the line number in the output, just simply outputting the secret would probably be sufficient too, maybe in single quotes or with some other delimiter, that might be more tool friendly than highlighting

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

No branches or pull requests

2 participants