-
Notifications
You must be signed in to change notification settings - Fork 580
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -146,6 +146,7 @@ ESLintBear: | |
- Typescript | ||
ElmLintBear: | ||
- Elm | ||
FileModeBear: | ||
FilenameBear: | ||
FormatRBear: | ||
- R | ||
|
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)] |
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') | ||
|
||
|
||
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': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mention the reason wherever you have done this. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) |
There was a problem hiding this comment.
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
?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.