Skip to content

Commit

Permalink
test: Use permissions from git in lint-files.py
Browse files Browse the repository at this point in the history
Instead of using permissions from the local file system, which might
depend on the umask, directly check the permissions from git's metadata.
  • Loading branch information
laanwj committed May 23, 2022
1 parent 48d2e80 commit 908fb7e
Showing 1 changed file with 42 additions and 30 deletions.
72 changes: 42 additions & 30 deletions test/lint/lint-files.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,36 @@
import re
import sys
from subprocess import check_output
from typing import Optional, NoReturn
from typing import Dict, Optional, NoReturn

CMD_TOP_LEVEL = ["git", "rev-parse", "--show-toplevel"]
CMD_ALL_FILES = ["git", "ls-files", "-z", "--full-name"]
CMD_SOURCE_FILES = ["git", "ls-files", "-z", "--full-name", "--", "*.[cC][pP][pP]", "*.[hH]", "*.[pP][yY]", "*.[sS][hH]"]
CMD_ALL_FILES = ["git", "ls-files", "-z", "--full-name", "--stage"]
CMD_SHEBANG_FILES = ["git", "grep", "--full-name", "--line-number", "-I", "^#!"]

ALL_SOURCE_FILENAMES_REGEXP = r"^.*\.(cpp|h|py|sh)$"
ALLOWED_FILENAME_REGEXP = "^[a-zA-Z0-9/_.@][a-zA-Z0-9/_.@-]*$"
ALLOWED_SOURCE_FILENAME_REGEXP = "^[a-z0-9_./-]+$"
ALLOWED_SOURCE_FILENAME_EXCEPTION_REGEXP = (
"^src/(secp256k1/|minisketch/|univalue/|test/fuzz/FuzzedDataProvider.h)"
)
ALLOWED_PERMISSION_NON_EXECUTABLES = 644
ALLOWED_PERMISSION_EXECUTABLES = 755
ALLOWED_PERMISSION_NON_EXECUTABLES = 0o644
ALLOWED_PERMISSION_EXECUTABLES = 0o755
ALLOWED_EXECUTABLE_SHEBANG = {
"py": [b"#!/usr/bin/env python3"],
"sh": [b"#!/usr/bin/env bash", b"#!/bin/sh"],
}


class FileMeta(object):
def __init__(self, file_path: str):
self.file_path = file_path
def __init__(self, file_spec: str):
'''Parse a `git ls files --stage` output line.'''
# 100755 5a150d5f8031fcd75e80a4dd9843afa33655f579 0 ci/test/00_setup_env.sh
meta, self.file_path = file_spec.split('\t', 2)
meta = meta.split()
# The octal file permission of the file. Internally, git only
# keeps an 'executable' bit, so this will always be 0o644 or 0o755.
self.permissions = int(meta[0], 8) & 0o7777
# We don't currently care about the other fields

@property
def extension(self) -> Optional[str]:
Expand Down Expand Up @@ -61,20 +68,24 @@ def full_extension(self) -> Optional[str]:
except IndexError:
return None

@property
def permissions(self) -> int:
"""
Returns the octal file permission of the file
"""
return int(oct(os.stat(self.file_path).st_mode)[-3:])

def get_git_file_metadata() -> Dict[str, FileMeta]:
'''
Return a dictionary mapping the name of all files in the repository to git tree metadata.
'''
files_raw = check_output(CMD_ALL_FILES).decode("utf8").rstrip("\0").split("\0")
files = {}
for file_spec in files_raw:
meta = FileMeta(file_spec)
files[meta.file_path] = meta
return files

def check_all_filenames() -> int:
def check_all_filenames(files) -> int:
"""
Checks every file in the repository against an allowed regexp to make sure only lowercase or uppercase
alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.
"""
filenames = check_output(CMD_ALL_FILES).decode("utf8").rstrip("\0").split("\0")
filenames = files.keys()
filename_regex = re.compile(ALLOWED_FILENAME_REGEXP)
failed_tests = 0
for filename in filenames:
Expand All @@ -86,14 +97,14 @@ def check_all_filenames() -> int:
return failed_tests


def check_source_filenames() -> int:
def check_source_filenames(files) -> int:
"""
Checks only source files (*.cpp, *.h, *.py, *.sh) against a stricter allowed regexp to make sure only lowercase
alphanumerics (a-z0-9), underscores (_), hyphens (-) and dots (.) are used in source code filenames.
Additionally there is an exception regexp for directories or files which are excepted from matching this regexp.
"""
filenames = check_output(CMD_SOURCE_FILES).decode("utf8").rstrip("\0").split("\0")
filenames = [filename for filename in files.keys() if re.match(ALL_SOURCE_FILENAMES_REGEXP, filename, re.IGNORECASE)]
filename_regex = re.compile(ALLOWED_SOURCE_FILENAME_REGEXP)
filename_exception_regex = re.compile(ALLOWED_SOURCE_FILENAME_EXCEPTION_REGEXP)
failed_tests = 0
Expand All @@ -106,24 +117,22 @@ def check_source_filenames() -> int:
return failed_tests


def check_all_file_permissions() -> int:
def check_all_file_permissions(files) -> int:
"""
Checks all files in the repository match an allowed executable or non-executable file permission octal.
Additionally checks that for executable files, the file contains a shebang line
"""
filenames = check_output(CMD_ALL_FILES).decode("utf8").rstrip("\0").split("\0")
failed_tests = 0
for filename in filenames:
file_meta = FileMeta(filename)
for filename, file_meta in files.items():
if file_meta.permissions == ALLOWED_PERMISSION_EXECUTABLES:
with open(filename, "rb") as f:
shebang = f.readline().rstrip(b"\n")

# For any file with executable permissions the first line must contain a shebang
if not shebang.startswith(b"#!"):
print(
f"""File "{filename}" has permission {ALLOWED_PERMISSION_EXECUTABLES} (executable) and is thus expected to contain a shebang '#!'. Add shebang or do "chmod {ALLOWED_PERMISSION_NON_EXECUTABLES} {filename}" to make it non-executable."""
f"""File "{filename}" has permission {ALLOWED_PERMISSION_EXECUTABLES:03o} (executable) and is thus expected to contain a shebang '#!'. Add shebang or do "chmod {ALLOWED_PERMISSION_NON_EXECUTABLES:03o} {filename}" to make it non-executable."""
)
failed_tests += 1

Expand All @@ -146,14 +155,14 @@ def check_all_file_permissions() -> int:
continue
else:
print(
f"""File "{filename}" has unexpected permission {file_meta.permissions}. Do "chmod {ALLOWED_PERMISSION_NON_EXECUTABLES} {filename}" (if non-executable) or "chmod {ALLOWED_PERMISSION_EXECUTABLES} {filename}" (if executable)."""
f"""File "{filename}" has unexpected permission {file_meta.permissions:03o}. Do "chmod {ALLOWED_PERMISSION_NON_EXECUTABLES:03o} {filename}" (if non-executable) or "chmod {ALLOWED_PERMISSION_EXECUTABLES:03o} {filename}" (if executable)."""
)
failed_tests += 1

return failed_tests


def check_shebang_file_permissions() -> int:
def check_shebang_file_permissions(files_meta) -> int:
"""
Checks every file that contains a shebang line to ensure it has an executable permission
"""
Expand All @@ -165,7 +174,7 @@ def check_shebang_file_permissions() -> int:

failed_tests = 0
for filename in filenames:
file_meta = FileMeta(filename)
file_meta = files_meta[filename]
if file_meta.permissions != ALLOWED_PERMISSION_EXECUTABLES:
# These file types are typically expected to be sourced and not executed directly
if file_meta.full_extension in ["bash", "init", "openrc", "sh.in"]:
Expand All @@ -179,7 +188,7 @@ def check_shebang_file_permissions() -> int:
continue

print(
f"""File "{filename}" contains a shebang line, but has the file permission {file_meta.permissions} instead of the expected executable permission {ALLOWED_PERMISSION_EXECUTABLES}. Do "chmod {ALLOWED_PERMISSION_EXECUTABLES} {filename}" (or remove the shebang line)."""
f"""File "{filename}" contains a shebang line, but has the file permission {file_meta.permissions:03o} instead of the expected executable permission {ALLOWED_PERMISSION_EXECUTABLES:03o}. Do "chmod {ALLOWED_PERMISSION_EXECUTABLES:03o} {filename}" (or remove the shebang line)."""
)
failed_tests += 1
return failed_tests
Expand All @@ -188,11 +197,14 @@ def check_shebang_file_permissions() -> int:
def main() -> NoReturn:
root_dir = check_output(CMD_TOP_LEVEL).decode("utf8").strip()
os.chdir(root_dir)

files = get_git_file_metadata()

failed_tests = 0
failed_tests += check_all_filenames()
failed_tests += check_source_filenames()
failed_tests += check_all_file_permissions()
failed_tests += check_shebang_file_permissions()
failed_tests += check_all_filenames(files)
failed_tests += check_source_filenames(files)
failed_tests += check_all_file_permissions(files)
failed_tests += check_shebang_file_permissions(files)

if failed_tests:
print(
Expand Down

0 comments on commit 908fb7e

Please sign in to comment.