Content Checker Plugin #118

TharunMohandoss · 2023-03-07T04:38:09Z

No description provided.

victoria-miltcheva · 2023-03-20T16:14:51Z

Hi @TharunMohandoss, thank you for your pull request! I will review it this week.

victoria-miltcheva · 2023-04-17T15:11:59Z

@TharunMohandoss sorry for the wait on the PR review, unfortunately I've been very busy lately. I will make some time this week to review your PR!

victoria-miltcheva · 2023-04-24T22:50:18Z

Hi @TharunMohandoss, you will want to make sure that your commits are signed. See this check for instructions on how to fix the unsigned commit: https://github.com/IBM/detect-secrets/runs/11811883373.

victoria-miltcheva · 2023-04-24T22:52:17Z

detect_secrets/core/usage.py

+            ._add_no_verify_flag() \
            ._add_output_verified_false_flag()\
-            ._add_fail_on_unaudited_flag()
+            ._add_fail_on_non_audited_flag()


This line should be ._add_fail_on_unaudited_flag(): https://github.com/IBM/detect-secrets/blob/master/detect_secrets/core/usage.py#L88

victoria-miltcheva · 2023-04-24T22:53:49Z

detect_secrets/core/usage.py

+    def _add_fail_on_non_audited_flag(self):
        self.parser.add_argument(
-            '--fail-on-unaudited',
+            '--fail-on-non-audited',


Can this change please be undone? This renames the --fail-on-unaudited flag and will break detect-secrets for users who are using this flag in their builds and pre-commit config files.

I think I understand what happened, you're probably working off an outdated version of master (see #118 (comment)).

victoria-miltcheva · 2023-04-24T22:56:40Z

detect_secrets/plugins/content_checker.py

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+


Suggested change

victoria-miltcheva · 2023-04-24T22:58:27Z

tests/plugins/content_checker_test.py

+			"content": 'String jwtSecret = "dasdasdasdasdasdasdasdasdasddaasdasdasdasdsdhashdsahdhsadhhasdhahdhashdhahdhah";',
+			"want":    secretExpected,
+		},
+		# {


It looks like some of the values in java_source_file_tests are commented out. Was this intentional?

victoria-miltcheva · 2023-04-24T22:58:37Z

tests/plugins/content_checker_test.py

+
+
+
+
+
+
+
+
+
+
+
+


Suggested change

victoria-miltcheva · 2023-04-24T22:59:06Z

detect_secrets/plugins/content_checker.py

+
+
+
+
+
+
+
+
+
+
+
+


Suggested change

victoria-miltcheva · 2023-04-24T22:59:26Z

detect_secrets/plugins/content_checker.py

+
+        self.secret_type = 'Content Checker'
+
+        #print('initialization')


This line can probably be removed.

victoria-miltcheva · 2023-04-24T22:59:45Z

detect_secrets/plugins/content_checker.py

+            # print('pattern : ',pattern.name)
+            # print("pattern : ",pattern.pattern)
+            # print('string : ',string.strip())
+            if pattern.extensions and (file_extension not in pattern.extensions):
+                # print('skipping no in extensions list')
+                # print()
+                # print()
+                continue
+            elif file_extension in pattern.excluded_extensions:
+                # print('in excluded extensions list')
+                # print()
+                # print()
+                continue
+
+            match = pattern.regex.search(string)
+            # print('match : ',match)
+            # print()
+            # print()


These print statements can probably be removed as well.

victoria-miltcheva · 2023-04-24T23:00:34Z

detect_secrets/plugins/content_checker.py

+Adaptation from : https://github.ibm.com/cognitive-data-platform/cognitive-data-platform/blob/master/tools/cedp_ci/check/passwords.go
+
+Searches for the following named patterns. 
+2. SecretFoundInString


Why does this list start with 2?

victoria-miltcheva · 2023-04-24T23:02:09Z

detect_secrets/core/usage.py

 class AuditOptions:
+
    def __init__(self, subparser):
-        # Override the default audit parser usage message since the arguments within


Question: why was the report parser code removed? I wonder if you were working off an outdated version of master. I'd recommend merging upstream master into your branch to ensure that you're working with the latest code.

victoria-miltcheva · 2023-04-24T23:04:06Z

It looks like the tests are failing in the build:

================ 33 failed, 1275 passed, 117 warnings in 30.92s ================

py37: exit 1 (31.40 seconds) /home/travis/build/IBM/detect-secrets> coverage run -m pytest tests pid=5073

.pkg: _exit> python /home/travis/virtualenv/python3.7.13/lib/python3.7/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__

  py37: FAIL code 1 (69.40=setup[37.88]+cmd[0.11,31.40] seconds)

  evaluation failed :( (69.54 seconds)

Makefile:16: recipe for target 'test' failed

make: *** [test] Error 1

The command "make setup-trivy && make trivy-scan-python-vulnerabilities && make test" exited with 2.
cache.2

store build cache

Can you please fix them after merging upstream master into your branch?

content checker additions for code and tests

97cccac

victoria-miltcheva self-assigned this Mar 24, 2023

victoria-miltcheva reviewed Apr 24, 2023

View reviewed changes

detect_secrets/plugins/content_checker.py

Comment on lines +206 to +232

Copy link

victoria-miltcheva Apr 24, 2023 •

edited

Loading

Choose a reason for hiding this comment

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

Suggested change

victoria-miltcheva reviewed Apr 24, 2023

View reviewed changes

tests/plugins/content_checker_test.py

Comment on lines +123 to +134

Copy link

victoria-miltcheva Apr 24, 2023

Choose a reason for hiding this comment

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

Suggested change

victoria-miltcheva reviewed Apr 24, 2023

View reviewed changes

detect_secrets/plugins/content_checker.py

Comment on lines +114 to +125

Copy link

victoria-miltcheva Apr 24, 2023

Choose a reason for hiding this comment

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

Suggested change

victoria-miltcheva reviewed Apr 24, 2023

View reviewed changes


		self.secret_type = 'Content Checker'

		#print('initialization')

Content Checker Plugin #118

Are you sure you want to change the base?

Content Checker Plugin #118

Uh oh!

Conversation

TharunMohandoss commented Mar 7, 2023

Uh oh!

victoria-miltcheva commented Mar 20, 2023

Uh oh!

victoria-miltcheva commented Apr 17, 2023

Uh oh!

victoria-miltcheva commented Apr 24, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victoria-miltcheva Apr 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

victoria-miltcheva commented Apr 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

victoria-miltcheva Apr 24, 2023 •

edited

Loading