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

bears/general: Add FileModeBear #2912

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bear-languages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ ESLintBear:
- Typescript
ElmLintBear:
- Elm
FileModeBear:
FilenameBear:
FormatRBear:
- R
Expand Down
48 changes: 48 additions & 0 deletions bears/general/FileModeBear.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import os
import stat

from coalib.bears.LocalBear import LocalBear
from coalib.results.Result import Result
from coalib.results.RESULT_SEVERITY import RESULT_SEVERITY


class FileModeBear(LocalBear):
LANGUAGES = {'All'}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'coala-devel@googlegroups.com'}
LICENSE = 'AGPL-3.0'

def run(self,
filename,
file,
filemode: str,
):
"""
The bear will check if the file has required permissions provided by
the user.

:param filemode:
Filemode to check, e.g. `rw`, `rwx`, etc.
"""
st = os.stat(filename)
permissions = {'r': stat.S_IRUSR,
'w': stat.S_IWUSR,
'x': stat.S_IXUSR,
}

invalid_chars = [ch for ch in filemode if ch not in permissions]

if invalid_chars:
raise ValueError('Unable to recognize character `{}` in filemode '
'`{}`.'.format(''.join(invalid_chars), filemode))

mode = st.st_mode
for char in filemode:
if not mode & permissions[char]:
message = ('The file permissions are not adequate. '
'The permissions are set to {}'
.format(stat.filemode(mode)))
return [Result.from_values(origin=self,
message=message,
severity=RESULT_SEVERITY.INFO,
file=filename)]
219 changes: 219 additions & 0 deletions tests/general/FileModeBearTest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
import os
import platform
import stat

from queue import Queue

from bears.general.FileModeBear import FileModeBear
from coalib.testing.LocalBearTestHelper import LocalBearTestHelper
from coalib.results.Result import RESULT_SEVERITY, Result
from coalib.settings.Section import Section
from coalib.settings.Setting import Setting


FILE_PATH = os.path.join(os.path.dirname(__file__),
'filemode_test_files', 'test_file.txt')
Copy link
Member

Choose a reason for hiding this comment

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

the issue asked for "...on scripts" . Why are you using .txt ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess having no extension at all would've been better. This is because the bear is supposed to check for all file permissions as mentioned here and is not just restricted scripts.

Copy link
Member

@jayvdb jayvdb Aug 22, 2019

Choose a reason for hiding this comment

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

Start with the realistic scenario of scripts having .sh / .bash etc extensions and make sure you can get that working and tested properly, before trying extensionless voodoo.



class FileModeBearTest(LocalBearTestHelper):

def setUp(self):
self.section = Section('')
self.uut = FileModeBear(self.section, Queue())

def test_r_to_r_permissions(self):
os.chmod(FILE_PATH, stat.S_IRUSR)
self.section.append(Setting('filemode', 'r'))
self.check_results(
self.uut,
[],
[],
filename=FILE_PATH,
)

def test_w_to_w_permissions(self):
os.chmod(FILE_PATH, stat.S_IWUSR)
self.section.append(Setting('filemode', 'w'))
self.check_results(
self.uut,
[],
[],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_x_to_x_permissions(self):
os.chmod(FILE_PATH, stat.S_IXUSR)
if platform.system() != 'Windows':
Copy link
Member

Choose a reason for hiding this comment

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

Mention the reason wherever you have done this.

Copy link
Member

Choose a reason for hiding this comment

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

No. Get rid of it. If this bear doesnt support Windows, that fact shouldnt be hidden inside the logic of the tests.

A .coafile is a cross-platform specification, and it must not cause failures on only one platform.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps create a separate test class for executable permissions, and the class should be skipped on Windows for tests which dont work on Windows, and potentially we need a second extra test class which does Windows specific testing to show that specifying executable bits in the .coafile results in a mild note in the coala log, but does not cause it to fail.

self.section.append(Setting('filemode', 'x'))
self.check_results(
self.uut,
[],
[],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_rw_to_rw_permissions(self):
os.chmod(FILE_PATH, stat.S_IRUSR | stat.S_IWUSR)
self.section.append(Setting('filemode', 'rw'))
self.check_results(
self.uut,
[],
[],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_wx_to_wx_permissions(self):
os.chmod(FILE_PATH, stat.S_IWUSR | stat.S_IXUSR)
if platform.system() != 'Windows':
self.section.append(Setting('filemode', 'wx'))
self.check_results(
self.uut,
[],
[],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_rx_to_rx_permissions(self):
os.chmod(FILE_PATH, stat.S_IRUSR | stat.S_IXUSR)
if platform.system() != 'Windows':
self.section.append(Setting('filemode', 'rx'))
self.check_results(
self.uut,
[],
[],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_rwx_to_rwx_permissions(self):
os.chmod(FILE_PATH, stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
if platform.system() != 'Windows':
self.section.append(Setting('filemode', 'rwx'))
self.check_results(
self.uut,
[],
[],
filename=FILE_PATH,
settings={'filemode': 'rwx'})
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_r_to_rwx_permissions(self):
os.chmod(FILE_PATH, stat.S_IRUSR)
filemode = '-r--------'
if platform.system() == 'Windows':
filemode = '-r--r--r--'
message = ('The file permissions are not adequate. The '
'permissions are set to {}'.format(filemode))
self.section.append(Setting('filemode', 'rwx'))
self.check_results(
self.uut,
[],
[Result.from_values('FileModeBear',
message,
file=FILE_PATH,
severity=RESULT_SEVERITY.INFO)],
filename=FILE_PATH,
settings={'filemode': 'rwx'})

def test_w_to_rwx_permissions(self):
os.chmod(FILE_PATH, stat.S_IWUSR)
filemode = '--w-------'
if platform.system() == 'Windows':
filemode = '-rw-rw-rw-'
message = ('The file permissions are not adequate. The '
'permissions are set to {}'.format(filemode))
self.section.append(Setting('filemode', 'rwx'))
self.check_results(
self.uut,
[],
[Result.from_values('FileModeBear',
message,
file=FILE_PATH,
severity=RESULT_SEVERITY.INFO)],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_x_to_rwx_permissions(self):
os.chmod(FILE_PATH, stat.S_IXUSR)
filemode = '---x------'
if platform.system() != 'Windows':
message = ('The file permissions are not adequate. The '
'permissions are set to {}'.format(filemode))
self.section.append(Setting('filemode', 'rwx'))
self.check_results(
self.uut,
[],
[Result.from_values('FileModeBear',
message,
file=FILE_PATH,
severity=RESULT_SEVERITY.INFO)],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_rx_to_rwx_permissions(self):
os.chmod(FILE_PATH, stat.S_IRUSR | stat.S_IXUSR)
filemode = '-r-x------'
if platform.system() != 'Windows':
message = ('The file permissions are not adequate. The '
'permissions are set to {}'.format(filemode))
self.section.append(Setting('filemode', 'rwx'))
self.check_results(
self.uut,
[],
[Result.from_values('FileModeBear',
message,
file=FILE_PATH,
severity=RESULT_SEVERITY.INFO)],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_wx_to_rwx_permissions(self):
os.chmod(FILE_PATH, stat.S_IWUSR | stat.S_IXUSR)
filemode = '--wx------'
if platform.system() != 'Windows':
message = ('The file permissions are not adequate. The '
'permissions are set to {}'.format(filemode))
self.section.append(Setting('filemode', 'rwx'))
self.check_results(
self.uut,
[],
[Result.from_values('FileModeBear',
message,
file=FILE_PATH,
severity=RESULT_SEVERITY.INFO)],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_rw_to_rwx_permissions(self):
os.chmod(FILE_PATH, stat.S_IRUSR | stat.S_IWUSR)
filemode = '-rw-------'
if platform.system() == 'Windows':
filemode = '-rw-rw-rw-'
message = ('The file permissions are not adequate. The '
'permissions are set to {}'.format(filemode))
self.section.append(Setting('filemode', 'rwx'))
self.check_results(
self.uut,
[],
[Result.from_values('FileModeBear',
message,
file=FILE_PATH,
severity=RESULT_SEVERITY.INFO)],
filename=FILE_PATH,
)
os.chmod(FILE_PATH, stat.S_IRUSR)

def test_invalid_char_in_filemode(self):
self.section.append(Setting('filemode', 'rwmc'))
error_msg = ('ValueError: Unable to recognize '
'character `mc` in filemode `rwmc`.')
with self.assertRaisesRegex(AssertionError, error_msg):
self.check_validity(self.uut, [], filename=FILE_PATH)
Empty file.