Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 4 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,7 @@
## 2024-05-19 - Pathlib Instantiation in Hot Loops
**Learning:** Blindly instantiating `pathlib.Path` objects in hot loops (like file discovery loops or display formatters such as `detect_language_axes` and `_display_path`) creates measurable performance bottlenecks due to object allocation and potential system calls. When checking file extensions or processing path strings, Python's native string methods like `str.rfind()` and `str.replace()` are vastly more efficient.
**Action:** Replace `pathlib.Path` usage with fast C-level string operations (`replace("\\", "/")`, `rfind()`, `split()`) in performance-critical areas, particularly when traversing thousands of files, formatting paths, or extracting file extensions.

## 2024-08-08 - Pathlib Instantiation and string operations in Hot Loops
**Learning:** Blindly checking `isinstance(file_path, Path)` or instantiating paths, alongside performing allocating string operations like `replace("\\", "/")`, in hot loops (like file discovery loops or display formatters such as `detect_language_axes` and `_display_path`) creates measurable performance bottlenecks due to object allocation and potential system calls. When checking file extensions or processing path strings, Python's native string methods like `str.rfind()` and `max()` are vastly more efficient.
**Action:** Replace `isinstance(..., Path)` with `type(...) is not str` and avoid `replace("\\", "/")` in favor of fast C-level string operations (`max(path.rfind("/"), path.rfind("\\"))`, `rfind()`, `split()`) in performance-critical areas, particularly when traversing thousands of files, formatting paths, or extracting file extensions.
48 changes: 29 additions & 19 deletions appguardrail_core/language.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,12 @@ def detect_language_axes(files: Iterable[str | Path]) -> set[str]:
"""Return language axes found in a scan target without requiring user flags."""
languages: set[str] = set()
for file_path in files:
if isinstance(file_path, Path):
if not isinstance(file_path, str):
name = file_path.name
suffix = file_path.suffix.lower()
else:
file_path_posix = file_path.replace("\\", "/")
idx = file_path_posix.rfind("/")
name = file_path_posix[idx + 1 :] if idx != -1 else file_path_posix
idx = max(file_path.rfind("/"), file_path.rfind("\\"))
name = file_path[idx + 1 :] if idx != -1 else file_path

dot_idx = name.rfind(".")
suffix = name[dot_idx:].lower() if dot_idx > 0 else ""
Expand Down Expand Up @@ -202,14 +201,28 @@ def detect_stack_profile(files: Iterable[str | Path]) -> StackProfile:
)


def _iter_lower_path_components(path: str) -> Iterable[str]:
"""Yield lowercase non-empty path components for either slash convention."""
start = 0
for index, character in enumerate(path):
if character not in "/\\":
continue
if index > start:
yield path[start:index].lower()
start = index + 1
if start < len(path):
yield path[start:].lower()


def _detect_framework_markers(paths: list[str]) -> set[str]:
markers: set[str] = set()
for path in paths:
posix_path = path.replace("\\", "/")
idx = posix_path.rfind("/")
name = posix_path[idx + 1 :] if idx != -1 else posix_path
lowered_path = posix_path.lower()
if "templates/" in lowered_path or "/views/" in lowered_path:
idx = max(path.rfind("/"), path.rfind("\\"))
name = path[idx + 1 :] if idx != -1 else path
if any(
component in {"templates", "views"}
for component in _iter_lower_path_components(path)
):
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
markers.add("templates")
if name not in MANIFEST_NAMES:
continue
Expand All @@ -223,15 +236,13 @@ def _detect_framework_markers(paths: list[str]) -> set[str]:
def _detect_signals(paths: list[str], frameworks: set[str]) -> set[str]:
signals = set(frameworks)
for path in paths:
posix_path = path.replace("\\", "/")
idx = posix_path.rfind("/")
name = posix_path[idx + 1 :] if idx != -1 else posix_path
idx = max(path.rfind("/"), path.rfind("\\"))
name = path[idx + 1 :] if idx != -1 else path
if name in MANIFEST_NAMES:
signals.add(name)
parts = posix_path.split("/")
for part in parts:
if part.lower() in WEB_SIGNAL_DIRS:
signals.add(part.lower())
for component in _iter_lower_path_components(path):
if component in WEB_SIGNAL_DIRS:
signals.add(component)
return signals


Expand Down Expand Up @@ -269,8 +280,7 @@ def _is_web_reachable(
):
return True
for path in paths:
parts = path.replace("\\", "/").split("/")
for part in parts:
if part.lower() in WEB_SIGNAL_DIRS:
for component in _iter_lower_path_components(path):
if component in WEB_SIGNAL_DIRS:
return True
return False
4 changes: 2 additions & 2 deletions scanner/cli/appguardrail.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ def _console_print(*values, **kwargs) -> None:
"id": "dangerous-eval",
"pattern": re.compile(r"\beval\s*\(", re.MULTILINE),
"severity": "CRITICAL",
"message": "Use of eval() detected. This is a critical risk for arbitrary code execution and injection attacks. [OWASP A03:2021 - Injection]",
"message": "Use of eval detected. This is a critical risk for arbitrary code execution and injection attacks. [OWASP A03:2021 - Injection]",
"extensions": [".js", ".jsx", ".ts", ".tsx", ".py"],
},
{
Expand Down Expand Up @@ -1174,7 +1174,7 @@ def _load_packaged_regex_rules():

def _display_path(path: str | Path) -> str:
"""Return a stable, slash-separated path for CLI output and reports."""
return path.as_posix() if isinstance(path, Path) else path.replace("\\", "/")
return path.as_posix() if type(path) is not str else path.replace("\\", "/")


# ---------------------------------------------------------------------------
Expand Down
48 changes: 47 additions & 1 deletion tests/test_language_path_optimization_contract.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
"""Regression tests for string-based language-profile path handling."""

import inspect
from pathlib import Path

import pytest

from appguardrail_core.language import detect_language_axes, detect_stack_profile
from appguardrail_core.language import (
_detect_signals,
detect_language_axes,
detect_stack_profile,
)
from scanner.cli.appguardrail import _display_path


class StringPath(str):
"""String path subtype used to preserve the public ``str`` input contract."""


@pytest.mark.parametrize(
Expand All @@ -28,6 +38,42 @@ def test_string_path_language_detection_matches_path_objects(path_text: str) ->
assert detect_language_axes([path_text]) == detect_language_axes([Path(path_text)])


def test_string_subclass_uses_string_language_detection_branch() -> None:
"""A ``str`` subtype must not be mistaken for a ``Path``-like object."""
assert detect_language_axes([StringPath(r"src\main.py")]) == {"python"}


def test_string_subclass_uses_string_display_path_branch() -> None:
"""CLI display formatting must accept ``str`` subtypes without Path methods."""
assert _display_path(StringPath(r"src\main.py")) == "src/main.py"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved


@pytest.mark.parametrize(
("path_text", "expects_template_marker"),
[
("mytemplates/page.tsx", False),
("views/page.tsx", True),
(r"src\views\page.tsx", True),
(r"src/views\page.tsx", True),
],
)
def test_template_and_view_markers_use_exact_path_components(
path_text: str, expects_template_marker: bool
) -> None:
"""Template detection must handle both separators without substring matches."""
profile = detect_stack_profile([path_text])

assert ("templates" in profile.frameworks) is expects_template_marker


def test_signal_detection_avoids_replace_split_hot_loop_allocations() -> None:
"""Signal extraction must not rebuild and split every path in the scan loop."""
source = inspect.getsource(_detect_signals)

assert ".replace(" not in source
assert ".split(" not in source


Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
def test_generator_input_is_materialized_once_for_profile_detection(tmp_path: Path) -> None:
"""One-shot iterables must feed language, framework, and signal detection once."""
manifest = tmp_path / "package.json"
Expand Down
Loading