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

Add JSON filetype #455

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add JSON filetype #455

wants to merge 1 commit into from

Conversation

hashier
Copy link

@hashier hashier commented Apr 22, 2021

I disable the filter for the comparison, since the key/secret would be filtered out otherwise.
Though, without the patch not every valid line inside JSON would be scanned.

# Current master

$ PYTHONPATH="" python3 -m detect_secrets scan test.json --disable-filter detect_secrets.filters.heuristic.is_not_alphanumeric_string | jq '.results'
{}
# My branch

$ PYTHONPATH="/src/detect-secrets/" python3 -m detect_secrets scan test.json --disable-filter detect_secrets.filters.heuristic.is_not_alphanumeric_string | jq '.results'
{
  "test.json": [
    {
      "type": "Secret Keyword",
      "filename": "test.json",
      "hashed_secret": "3c8567b928837d1264eec832a7e59fdcb9470f8b",
      "is_verified": false,
      "line_number": 2
    }
  ]
}
# file to be scanned, valid JSON

$ cat test.json
{
  "secret": 78193672857003048172616861202423,
}

@@ -227,6 +227,10 @@
FOLLOWED_BY_EQUAL_SIGNS_QUOTES_REQUIRED_REGEX: 5,
FOLLOWED_BY_QUOTES_AND_SEMICOLON_REGEX: 3,
}
JSON_DENYLIST_REGEX_TO_GROUP = {
FOLLOWED_BY_COLON_REGEX: 4,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this?

I'm assuming that secrets in JSON files (found by the KeywordDetector) will be in string fields. aka.

{
    ...
    "secret": "blah"
}

By having FOLLOWED_BY_COLON_REGEX, we would also include things like numbers (which we want to ignore), and nested values (which is debatably useful, given that this is a line-based scanner).

Copy link
Author

Choose a reason for hiding this comment

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

Ah ok, if you want to ignore numbers then of course this isn't a rule you need.

I just followed more the "what is legal json" and added those rules.

I guess the PR without the FOLLOWED_BY_COLON_REGEX might still be useful, just to improve scanning times of JSON files.

@lorenzodb1 lorenzodb1 closed this Sep 26, 2022
@lorenzodb1 lorenzodb1 reopened this Sep 26, 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

3 participants