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

DodgyBear #836

Open
sils opened this issue Sep 19, 2016 · 31 comments · May be fixed by #2246
Open

DodgyBear #836

sils opened this issue Sep 19, 2016 · 31 comments · May be fixed by #2246

Comments

@sils
Copy link
Member

sils commented Sep 19, 2016

https://github.com/landscapeio/dodgy

difficulty/low type/bear proposal area/lintbears

@meetmangukiya
Copy link
Member

@sils1297 I'd be interested in writing a bear. Maybe this one?

@Makman2
Copy link
Member

Makman2 commented Sep 19, 2016

cool why not @sils1297 :)

@meetmangukiya
Copy link
Member

I think HappinessBear and implementation of this bear would be pretty much similar, right?

@sils
Copy link
Member Author

sils commented Sep 19, 2016

👍 see docs.coala.io docs about writing linter bears

@meetmangukiya
Copy link
Member

@sils1297 just in case you haven't noticed, the repo hasn't been updated in last 9 months that is as good as a year.

@jayvdb
Copy link
Member

jayvdb commented Sep 19, 2016

Ya. Unmaintained.
A language unaware regex bear would be sufficient for some of the dodgy checks, esp conflict markers.

@sils
Copy link
Member Author

sils commented Sep 19, 2016

Nice, let's do that.

@meetmangukiya
Copy link
Member

@sils1297 given that the repo is not maintained do we want to use it?

@sils
Copy link
Member Author

sils commented Sep 20, 2016

As @jayvdb proposed: we can rather make an own native bear with that functionality which is usable for all languages. Essentially we just "grep" through the code for suh things right?

@meetmangukiya
Copy link
Member

I think I'll have to get an idea of dodgy . Got to read up some stuff.

@meetmangukiya
Copy link
Member

Can we have some words that the bear should be dodging other then the ones in dodgy, I'll add those anyway, and also I was thinking of configuring the DodgyBear with a list of words that could be maybe mentioned in the coafile of the project. Some specific words for the CI or something they use, etc. Thoughts? CC @sils1297 @underyx

@jayvdb
Copy link
Member

jayvdb commented Sep 22, 2016

As @jayvdb proposed: we can rather make an own native bear with that functionality which is usable for all languages. Essentially we just "grep" through the code for suh things right?

Allowing regex config support to KeywordBear would suffice?
It already uses regex internally, but doesnt allow configuration using regex, which is #311.

@sils
Copy link
Member Author

sils commented Sep 22, 2016 via email

@sils
Copy link
Member Author

sils commented Sep 23, 2016 via email

@meetmangukiya
Copy link
Member

That'd be helpful

@meetmangukiya
Copy link
Member

I'd like to request an unassignment, sorry @sils1297 @Makman2 :( I hope the discussion would help the future assignee. Reason being I haven't been able to come up with even the little bit of code of dodgy bear, even after a week and it is getting uninteresting now :( .

@madhur-tandon
Copy link
Member

madhur-tandon commented Dec 5, 2016

Please assign me this, I would like to give it a shot

@swapagarwal
Copy link
Member

@sils Maybe we can use this? https://github.com/awslabs/git-secrets

@manankalra
Copy link
Contributor

manankalra commented Jan 12, 2018

Can someone guide me a little on what all understanding do I need other than getting familiar with writing a linter bear that the coala documentation demonstrates to take up this issue?

@manankalra
Copy link
Contributor

manankalra commented Jan 17, 2018

@sils @Makman2
What should create_arguments() return for a global linter bear which implements a custom processing function and accepts no command-line arguments of any sort?
I guess the docs do not cover this aspect and there is only a single global linter bear implemented till now (CPPCheckBear) which does accept a --template argument.

@Makman2
Copy link
Member

Makman2 commented Jan 17, 2018

Then you return an empty iterable ;)

I guess the docs do not cover this aspect and there is only a single global linter bear implemented till now (CPPCheckBear) which does accept a --template argument.

Hm I actually think it makes kind of sense like it is now, create_arguments returns an iterable / sequence of arguments to pass to the program. If you don't want to pass any additional arguments, just return [] or tuple() ;)
And the case not passing any arguments is pretty rare^^

manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 18, 2018
This bear checks Python code for possible
dodgy looking values such as secret keys or
passwords.

Closes coala#836
@manankalra
Copy link
Contributor

manankalra commented Jan 18, 2018

I'm facing problems while writing a test for it and couldn't come up with a solution.
The issue is -
Initialising file_dict with the absolute paths of the files that I need to test seems to have no impact on the uut object and the bear just checks for all the files in the current directory from where pytest -k DodgyBearTest ran. This is so because dodgy accepts no command-line arguments of any sort. That is, one cannot even specify a --path for files that need to be checked. The same thing is addressed in this issue. Also, whenever dodgy runs, it just looks for .py files present in the current working directory but not in the sub-directories. It being a global bear, I couldn't use LocalBearTestHelper for validating checks.

Here's a snippet of what I've done so far for the DodgyBearTest:

import os
import unittest
from queue import Queue

from bears.python.DodgyBear import DodgyBear
from coalib.settings.Section import Section
from coalib.testing.BearTestHelper import generate_skip_decorator
from coalib.testing.LocalBearTestHelper import LocalBearTestHelper


def get_absolute_file_path(file):
    return os.path.join(os.path.dirname(__file__),
                        'dodgy_test_files', file)

@generate_skip_decorator(DodgyBear)
class DodgyBearTest(unittest.TestCase):

    def setUp(self):
        self.file_dict = {}
        self.queue = Queue()
        self.section = Section('dodgy')
        self.test_files = [get_absolute_file_path(f) for f in os.listdir(get_absolute_file_path(""))]
        for filename in self.test_files:
            with open(filename, 'r', encoding='utf-8') as content:
               self.file_dict[filename] = tuple(content.readlines())
        self.uut = DodgyBear(self.file_dict,
                             self.section,
                             self.queue)

    def get_results(self):
        return list(result.message for result in self.uut.run())

    def test(self):
        self.assertEqual(self.get_results()[0], 'Amazon Web Services secret key')

The test passes or raises an AssertionError only if the files being tested are present in the coala-bears or coala-bears/bears/python directory.

To sum up, I guess this is an issue with dodgy. Thoughts?

Other than this, I've added a commit for the implementation of DodgyBear and it seems to work fine for me.

dodgy

@Makman2
Copy link
Member

Makman2 commented Jan 18, 2018

If dodgy really accepts no command line arguments, that's actually a huge flaw imo of dodgy itself.
We should really consider using dodgy at all then.

However, to make it work anyway and integrate into the coala-ecosystem, we can now deploy some OS capabilities: We could use temporary files. You create a temporary directory and symbolic mappings to the files that shall be analyzed, all ending with .py. The working directory is then this temporary directory (not sure how we can properly set it inside linter)

But really let's first consider using dodgy, and rather make them improve their CLI, because that is just bad design.

@manankalra
Copy link
Contributor

manankalra commented Jan 19, 2018

So, what you mean is:

  • create temporary files in the root directory i.e. coala-bears.
  • run DodgyBearTest.
  • delete those temp files under tearDown().

manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 19, 2018
Tests DodgyBear for passwords,
secret keys, ssh keys and diff
check-ins.

Closes coala#836
manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 19, 2018
This bear checks Python code for possible
dodgy looking values such as secret keys or
passwords.

Closes coala#836
@manankalra manankalra linked a pull request Jan 19, 2018 that will close this issue
2 tasks
@Makman2
Copy link
Member

Makman2 commented Jan 20, 2018

This won't apply just for tests, but how the bear operates in general. But I would really suggest you sync up with the dodgy-guys and ask why they don't have any arguments to control the linting process.

@manankalra
Copy link
Contributor

manankalra commented Jan 20, 2018

@Makman2 I have already written an email to one of the dodgy-guys; didn't get a reply.
Any idea why my tests are not being executed during the build? I'm getting complete coverage locally.

@Makman2
Copy link
Member

Makman2 commented Jan 20, 2018

Probably dodgy not installed in CI.

manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 20, 2018
This bear checks Python code for possible
dodgy looking values such as secret keys or
passwords.

Closes coala#836
@manankalra
Copy link
Contributor

Anything I need to take care of?

@Makman2
Copy link
Member

Makman2 commented Jan 20, 2018

Yes, you have to install it ;)

manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 20, 2018
This bear checks Python code for possible
dodgy looking values such as secret keys or
passwords.

Closes coala#836
manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 22, 2018
This bear checks Python code for possible
dodgy looking values such as secret keys or
passwords.

Closes coala#836
@manankalra
Copy link
Contributor

@Makman2 I know this will be closed in favour of a better alternative but any guesses on why the tests are failing? They are all passing locally. Is writing and deleting new files in the coala-bears directory or any other directory allowed during the ci build process?
I've added dodgy in bear-requirements.txt.

@Makman2
Copy link
Member

Makman2 commented Jan 22, 2018

Is writing and deleting new files in the coala-bears directory or any other directory allowed during the ci build process?

It definitely is.

And I don't know the problem on CI. However this is something you should discuss on the PR itself because it's implementation detail ;)

manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 26, 2018
This bear checks Python code for possible
dodgy looking values such as secret keys or
passwords.

Closes coala#836
manankalra added a commit to manankalra/coala-bears that referenced this issue Jan 26, 2018
This bear checks Python code for possible
dodgy looking values such as secret keys or
passwords.

Closes coala#836
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

8 participants