Skip to content

fix(tm3): skip umask-masked modes and report permissive umask values #783

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.

Sign up for GitHub

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

Open
ilaigold wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(tm3): skip umask-masked modes and report permissive umask values #783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3eb3cd9
fix(tm3): skip umask-masked modes and report permissive umask values
ilaigold Oct 5, 2026
94547bd
fix(tm3): end the umask mask at the expression end and read umask val…
ilaigold Oct 7, 2026
c354b38
Merge remote-tracking branch 'origin/main' into fix/tm3-umask
ilaigold Oct 9, 2026
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
61 changes: 58 additions & 3 deletions src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,29 @@
TM2_PATTERNS = TM2_CODE_PATTERNS + TM2_PROSE_PATTERNS

# TM3: Unsafe Defaults — overly permissive default settings
#
# A umask leaves new files world-writable when its world-write bit (0o002) is unset.
# In octal digits that is a last digit of 0, 1, 4 or 5.
_UMASK_OCTAL_WORLD_WRITE = r"[0-7]*[0145]\b"
# Python, JavaScript and C read a number as octal only with a 0o prefix or a leading
# 0, so a bare 14 is 0o016 (world-write masked) and a bare 8 is 0o010 (unmasked). A
# decimal N leaves the bit unset when N % 4 is 0 or 1, which its last two digits decide.
_UMASK_NUMBER_WORLD_WRITE = (
rf"(?:0(?:o?{_UMASK_OCTAL_WORLD_WRITE})?\b"
r"|[1-9]\d*+(?<=[02468][014589]|[13579][2367]|\D[14589])\b)"
)
# umask 000, or an assigned value such as umask = 0o000 or umask = 8.
_TM3_UMASK = (
rf"umask(?:[ \t]+(?:0o)?{_UMASK_OCTAL_WORLD_WRITE}|\s*=\s*{_UMASK_NUMBER_WORLD_WRITE})",
0.8,
)
# Shell reads every umask value as octal, including a variable such as UMASK=14 that
# is later passed to the umask command.
_TM3_SHELL_UMASK = (rf"umask(?:\s*=\s*|[ \t]+)(?:0o)?{_UMASK_OCTAL_WORLD_WRITE}", 0.8)
# umask(0) as a statement or inside an expression. `old = os.umask(0)` is how Python
# reads the umask before restoring it, so analyze() skips a call whose result is
# assigned (see _is_assigned_value).
_TM3_UMASK_CALL = (rf"(?<![\w.])(?:\w+\.)*umask\(\s*{_UMASK_NUMBER_WORLD_WRITE}\s*\)", 0.8)
TM3_CODE_PATTERNS = [
# TLS/SSL verification disabled
(r"verify\s*=\s*False", 0.75),
Expand All @@ -338,8 +361,21 @@
# Overly permissive CORS / access
(r"(?:CORS|cors)[^=]*=\s*['\"]?\*['\"]?", 0.65),
(r"(?:allow|access)[_-]?(?:origin|hosts?)\s*=\s*['\"]?\*['\"]?", 0.7),
# Unsafe permissions
(r"(?:mode|permission|umask)\s*=\s*(?:0?o?777|0?o?666)", 0.8),
# Unsafe permissions. A mode masked with the umask (0o666 & ~umask) respects it,
# like a plain open(), when the mask ends the expression: a closing bracket, a
# separator, or the end of a line (LF or CRLF, after an optional # comment) whose
# next line does not continue it with an operator, `and` or `or`. Any other
# operator, such as | 0o777 or the floor division // 512, can change the result
# and is still reported. The comment is bounded so the check stays linear.
(
r"(?:mode|permission)\s*=\s*(?:0?o?777|0?o?666)"
r"(?![ \t]*&[ \t]*~[ \t]*[\w.]*umask\b(?:\([^()\n]*\))?[ \t]*"
r"(?:[),;\]}]|(?:#[^\n]{0,240}|\r)?$"
r"(?!\n[ \t]*(?:[-+*/%|&^<>=?:.]|(?:and|or)\b))))",
0.8,
),
_TM3_UMASK,
_TM3_UMASK_CALL,
(r"world[_-]?(?:readable|writable|executable)", 0.7),
# Debug/dev mode in production
(r"(?:debug|dev|development)[_-]?mode\s*=\s*(?:True|true|1|on|yes|enable)", 0.6),
Expand All @@ -364,6 +400,7 @@
),
]
TM3_PATTERNS = TM3_CODE_PATTERNS + TM3_PROSE_PATTERNS
_TM3_SHELL_PATTERNS = [_TM3_SHELL_UMASK if entry == _TM3_UMASK else entry for entry in TM3_PATTERNS]

# TM4: Privileged Kubernetes Workload — manifest/CLI primitives that grant
# node/host takeover (the cluster-scale counterpart of a privileged container).
Expand Down Expand Up @@ -4690,6 +4727,21 @@ def _line_containing(content: str, start: int, end: int) -> str:
return content[line_start:line_end]


def _is_assigned_value(content: str, start: int) -> bool:
"""Return whether the expression at *start* is the value of an assignment.

Whitespace, line continuations and opening parentheses after the ``=`` are
skipped, so ``old = (os.umask(0))`` reads like ``old = os.umask(0)``.
``==``, ``!=``, ``<=`` and ``>=`` compare rather than assign.
"""
index = start
while index and content[index - 1] in " \t\r\n\\(":
index -= 1
if not index or content[index - 1] != "=":
return False
return index < 2 or content[index - 2] not in "=!<>"


def _classify_tm1(
context: str,
matched_text: str,
Expand Down Expand Up @@ -4830,13 +4882,16 @@ def ctx(start: int) -> str:
complete_match=match.group(0),
)
)
for pattern, confidence in TM3_PATTERNS:
for pattern, confidence in _TM3_SHELL_PATTERNS if file_type == "shell" else TM3_PATTERNS:
matches = (
static_runner.iter_paragraph_matches
if (pattern, confidence) in TM3_PROSE_PATTERNS
else re.finditer
)
skip_assigned = (pattern, confidence) == _TM3_UMASK_CALL
for match in matches(pattern, content, re.IGNORECASE | re.MULTILINE):
if skip_assigned and _is_assigned_value(content, match.start()):
continue
line_num = get_line_number(content, match.start())
findings.append(
AnalyzerFinding(
Expand Down
194 changes: 194 additions & 0 deletions tests/unit/test_patterns_new.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,200 @@ def test_tm1_rm_outside_dockerfile_stays_high(self) -> None:
def test_tm3_detected(self, content: str, filename: str, filetype: str) -> None:
assert any(f.rule_id == "TM3" for f in tm_mod.analyze(content, filename, filetype))

@pytest.mark.parametrize(
"content,filename,filetype,expected",
[
pytest.param("mode = 0o777", "fs.py", "python", "mode = 0o777", id="mode_777"),
pytest.param(
"permission = 0o666", "fs.py", "python", "permission = 0o666", id="permission_666"
),
pytest.param(
"mode = 0o777 & ~umask | 0o777",
"fs.py",
"python",
"mode = 0o777",
id="bits_added_after_umask_mask",
),
pytest.param(
"mode = 0o777 & ~0", "fs.py", "python", "mode = 0o777", id="mask_is_not_the_umask"
),
pytest.param("umask 000", "run.sh", "shell", "umask 000", id="shell_umask_000"),
pytest.param("UMASK=0000", "env.sh", "shell", "UMASK=0000", id="env_umask_0000"),
pytest.param("umask = 0o000", "cfg.py", "python", "umask = 0o000", id="assign_0o000"),
pytest.param("umask 004", "run.sh", "shell", "umask 004", id="world_write_unmasked"),
pytest.param(
"umask=0o7770", "cfg.py", "python", "umask=0o7770", id="long_value_ending_in_0"
),
pytest.param("os.umask(0)", "fs.py", "python", "os.umask(0)", id="py_umask_call"),
pytest.param(
"process.umask(0o000);", "fs.js", "javascript", "process.umask(0o000)", id="js_call"
),
pytest.param("umask(0);", "main.c", "c", "umask(0)", id="c_umask_call"),
pytest.param(
"fd = os.open(p, flags, mode=0o666 & ~os.umask(0))",
"fs.py",
"python",
"os.umask(0)",
id="umask_zero_inside_mask_expression",
),
pytest.param(
"mode = 0o666 & ~current_umask // 512",
"fs.py",
"python",
"mode = 0o666",
id="floor_division_after_umask_mask",
),
pytest.param(
"os.chmod(path, mode=0o666 & ~umask\n | 0o666)",
"fs.py",
"python",
"mode=0o666",
id="bits_added_on_continued_line",
),
pytest.param(
"os.chmod(path, mode=0o666 & ~umask\r\n | 0o666)",
"fs.py",
"python",
"mode=0o666",
id="bits_added_on_continued_crlf_line",
),
pytest.param(
"os.chmod(path, mode=0o666 & ~umask # keep the umask\n | 0o666)",
"fs.py",
"python",
"mode=0o666",
id="bits_added_after_comment_line",
),
pytest.param(
"os.chmod(path, mode=0o666 & ~umask\n or 0o777)",
"fs.py",
"python",
"mode=0o666",
id="or_on_continued_line",
),
pytest.param(
"changed = old == os.umask(0)",
"fs.py",
"python",
"os.umask(0)",
id="umask_call_compared_not_assigned",
),
pytest.param("umask = 8", "cfg.py", "python", "umask = 8", id="decimal_8_is_0o010"),
pytest.param("os.umask(8)", "fs.py", "python", "os.umask(8)", id="decimal_call_0o010"),
pytest.param("umask 14", "run.sh", "shell", "umask 14", id="shell_umask_is_octal"),
pytest.param("UMASK=14", "env.sh", "shell", "UMASK=14", id="shell_variable_is_octal"),
],
)
def test_tm3_world_writable_mode_or_umask_detected(
self, content: str, filename: str, filetype: str, expected: str
) -> None:
"""A world-writable mode, or a umask that leaves world-write unmasked, is TM3."""
findings = tm_mod.analyze(content, filename, filetype)
assert [f.matched_text for f in findings if f.rule_id == "TM3"] == [expected]

@pytest.mark.parametrize(
"content,filename,filetype",
[
pytest.param("mode = 0o666 & ~umask", "fs.py", "python", id="mode_masked_by_umask"),
pytest.param(
"os.chmod(path, mode=0o777 & ~current_umask)",
"fs.py",
"python",
id="mode_masked_in_call",
),
pytest.param(
"const mode = 0o666 & ~process.umask();",
"fs.js",
"javascript",
id="mode_masked_by_process_umask",
),
pytest.param("umask = 0o777", "cfg.py", "python", id="most_restrictive_umask"),
pytest.param("UMASK=0777", "env.sh", "shell", id="restrictive_env_umask"),
pytest.param("umask 077", "run.sh", "shell", id="umask_077"),
pytest.param("umask 022", "run.sh", "shell", id="umask_022"),
pytest.param("os.umask(0o022)", "fs.py", "python", id="umask_call_022"),
pytest.param("os.umask(18)", "fs.py", "python", id="decimal_umask_call_022"),
pytest.param(
"umask = os.umask(0)\nos.umask(umask)\nos.chmod(tmp, 0o666 & ~umask)",
"fs.py",
"python",
id="read_and_restore_umask",
),
pytest.param(
"const previous = process.umask(0);", "fs.js", "javascript", id="js_read_umask"
),
pytest.param(
"mode = 0o666 & ~umask\r\nfd = os.open(path, flags, mode)\r\n",
"fs.py",
"python",
id="mode_masked_by_umask_crlf",
),
pytest.param(
"mode = 0o666 & ~umask # respect the umask\nfd = os.open(path, flags, mode)\n",
"fs.py",
"python",
id="mode_masked_by_umask_with_comment",
),
pytest.param(
"mode = 0o666 & ~umask # respect the umask\r\nif exists:\r\n pass\r\n",
"fs.py",
"python",
id="mode_masked_by_umask_with_comment_crlf",
),
pytest.param(
"os.chmod(path, mode=0o666 & ~umask\n )",
"fs.py",
"python",
id="mask_ends_before_closing_line",
),
pytest.param(
"old = os.umask(0)\nos.umask(old)", "fs.py", "python", id="read_umask_two_spaces"
),
pytest.param(
"old =\tos.umask(0)\nos.umask(old)", "fs.py", "python", id="read_umask_tab"
),
pytest.param(
"old = (os.umask(0))\nos.umask(old)",
"fs.py",
"python",
id="read_umask_parenthesized",
),
pytest.param("umask = 14", "cfg.py", "python", id="decimal_14_is_0o016"),
],
)
def test_tm3_umask_masked_mode_or_restrictive_umask_not_flagged(
self, content: str, filename: str, filetype: str
) -> None:
"""A mode masked with the umask and a restrictive umask are not unsafe defaults."""
findings = tm_mod.analyze(content, filename, filetype)
assert not [f.matched_text for f in findings if f.rule_id == "TM3"]

@pytest.mark.parametrize(
"content",
[
pytest.param("mode=0o777 & ~" + "a" * 50_000, id="long_mask_operand"),
pytest.param("mode=0o777 &" + " " * 50_000 + "~umask |", id="long_gap_before_mask"),
pytest.param("mode=0o777 & ~os.umask(" + "0" * 50_000, id="unclosed_mask_call"),
pytest.param("umask " + "0" * 50_000 + "2", id="long_umask_value"),
pytest.param("umask(0" + "0" * 50_000, id="unclosed_umask_call"),
pytest.param("a." * 25_000 + "umask(", id="long_dotted_name"),
pytest.param("umask umask " * 4_000, id="repeated_umask_words"),
pytest.param("old =" + " " * 50_000 + "os.umask(0)", id="long_gap_after_assignment"),
pytest.param("(" * 50_000 + "os.umask(0)", id="long_paren_run_before_call"),
pytest.param("old = (os.umask(0))\n" * 10_000, id="repeated_assigned_umask_calls"),
pytest.param("umask = 1" + "0" * 50_000 + "4", id="long_decimal_umask"),
pytest.param(
"mode=0o777 & ~umask\n" + " " * 50_000 + "|", id="long_indent_on_continued_line"
),
pytest.param("mode=0o777 & ~umask #" + "c" * 50_000, id="long_comment_after_mask"),
pytest.param("mode=0o777 & ~umask # c\n" * 5_000, id="repeated_commented_masks"),
],
)
def test_tm3_permission_patterns_are_linear(self, content: str) -> None:
started = time.perf_counter()
tm_mod.analyze(content, "fs.py", "python")
assert time.perf_counter() - started < 1.0

@pytest.mark.parametrize(
"content,filename,filetype",
[
Expand Down
Loading