From 3dc601c02bbf1e61af435e787a79a6199042c409 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:28:16 +0300 Subject: [PATCH 001/281] fix(api): map legacy AILAError subclasses to real HTTP status (#54) AuthenticationError/RateLimitError/NotFoundError/ValidationError/UpstreamError/TimeoutError lacked ClassVar http_status, so the error-envelope handler returned 500 for all of them despite their docstrings promising 401/429/404/422/502/504. Add code/http_status/user_message ClassVars (codes preserved to match the prior derive-from-name output) plus operator hints, and extend the taxonomy + handler tests to lock the six statuses. Retarget the fallback test to a genuinely undeclared subclass. --- src/aila/api/errors/hints.py | 13 ++++++++++ src/aila/platform/exceptions.py | 24 ++++++++++++++++++ tests/api/errors/test_handlers.py | 41 +++++++++++++++++-------------- tests/api/errors/test_taxonomy.py | 15 +++++++++++ 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/aila/api/errors/hints.py b/src/aila/api/errors/hints.py index 7614d856..ab52113a 100644 --- a/src/aila/api/errors/hints.py +++ b/src/aila/api/errors/hints.py @@ -32,6 +32,19 @@ ), # Framework-level codes. "VALIDATION_ERROR": "Fix the highlighted input fields and retry.", + "AUTHENTICATION_ERROR": ( + "Sign in again or check the credentials for this operation." + ), + "RATE_LIMIT_ERROR": ( + "The upstream provider is rate limiting. Wait and retry after a delay." + ), + "NOT_FOUND_ERROR": "The requested resource does not exist or was removed.", + "UPSTREAM_ERROR": ( + "An upstream dependency failed. Retry, or contact support with the trace ID." + ), + "TIMEOUT_ERROR": ( + "The operation timed out. Retry, or check the target system's reachability." + ), "INTERNAL_ERROR": ( "An unexpected error occurred. Contact support with the trace ID shown below." ), diff --git a/src/aila/platform/exceptions.py b/src/aila/platform/exceptions.py index 163d02b2..11138828 100644 --- a/src/aila/platform/exceptions.py +++ b/src/aila/platform/exceptions.py @@ -35,6 +35,10 @@ class AuthenticationError(AILAError): providers. Maps to HTTP 401 at the API layer. """ + code: ClassVar[str] = "AUTHENTICATION_ERROR" + http_status: ClassVar[int] = 401 + user_message: ClassVar[str] = "Authentication failed." + class RateLimitError(AILAError): """Raised when an external service enforces a request rate limit. @@ -44,6 +48,10 @@ class RateLimitError(AILAError): API layer. """ + code: ClassVar[str] = "RATE_LIMIT_ERROR" + http_status: ClassVar[int] = 429 + user_message: ClassVar[str] = "Rate limit exceeded; retry after a delay." + class NotFoundError(AILAError): """Raised when a requested resource does not exist. @@ -52,6 +60,10 @@ class NotFoundError(AILAError): responses from external APIs. Maps to HTTP 404 at the API layer. """ + code: ClassVar[str] = "NOT_FOUND_ERROR" + http_status: ClassVar[int] = 404 + user_message: ClassVar[str] = "The requested resource was not found." + class ValidationError(AILAError): """Raised when input fails validation at a service or workflow boundary. @@ -61,6 +73,10 @@ class ValidationError(AILAError): an unsupported operation). Maps to HTTP 422 at the API layer. """ + code: ClassVar[str] = "VALIDATION_ERROR" + http_status: ClassVar[int] = 422 + user_message: ClassVar[str] = "The request failed validation." + class UpstreamError(AILAError): """Raised when an external dependency fails. @@ -70,6 +86,10 @@ class UpstreamError(AILAError): at the API layer. """ + code: ClassVar[str] = "UPSTREAM_ERROR" + http_status: ClassVar[int] = 502 + user_message: ClassVar[str] = "An upstream dependency failed." + class TimeoutError(AILAError): """Raised when an external call exceeds its configured deadline. @@ -78,6 +98,10 @@ class TimeoutError(AILAError): Maps to HTTP 504 at the API layer. """ + code: ClassVar[str] = "TIMEOUT_ERROR" + http_status: ClassVar[int] = 504 + user_message: ClassVar[str] = "The operation timed out." + # --------------------------------------------------------------------------- # Phase 176a: typed exception taxonomy (D-10b). diff --git a/tests/api/errors/test_handlers.py b/tests/api/errors/test_handlers.py index 4fdf63ab..9095af10 100644 --- a/tests/api/errors/test_handlers.py +++ b/tests/api/errors/test_handlers.py @@ -25,11 +25,17 @@ MissingApiKeyError, ModulePlatformNotReadyError, NotFoundError, + RateLimitError, RouterError, SSHConnectionFailedError, + TimeoutError, + UpstreamError, + ValidationError, WorkerUnreachableError, ) +# Issue #54 (2026-07-19): the six pre-existing legacy subclasses now carry real +# ClassVar code/http_status, so they are exercised end-to-end here too. _D20_MAPPING = [ (MissingApiKeyError, "MISSING_API_KEY", 503), (SSHConnectionFailedError, "SSH_CONNECTION_FAILED", 502), @@ -37,6 +43,12 @@ (ModulePlatformNotReadyError, "MODULE_PLATFORM_NOT_READY", 503), (ConfigValueMissingError, "CONFIG_VALUE_MISSING", 500), (WorkerUnreachableError, "WORKER_UNREACHABLE", 503), + (AuthenticationError, "AUTHENTICATION_ERROR", 401), + (RateLimitError, "RATE_LIMIT_ERROR", 429), + (NotFoundError, "NOT_FOUND_ERROR", 404), + (ValidationError, "VALIDATION_ERROR", 422), + (UpstreamError, "UPSTREAM_ERROR", 502), + (TimeoutError, "TIMEOUT_ERROR", 504), ] @@ -71,10 +83,10 @@ def _assert_envelope_shape(body: dict) -> None: @pytest.mark.parametrize("cls,expected_code,expected_status", _D20_MAPPING) -def test_handler_emits_envelope_for_each_of_six( +def test_handler_emits_envelope_for_each_typed_error( cls: type[AILAError], expected_code: str, expected_status: int ) -> None: - """Each of the six D-20 typed errors produces the correct envelope + status.""" + """Each typed error (six D-20 + six legacy per #54) produces the correct envelope + status.""" app = _build_app({"/raise": cls}) client = TestClient(app, raise_server_exceptions=False) @@ -102,10 +114,15 @@ def test_handler_emits_envelope_for_typed_error() -> None: assert body["trace_id"] is None or isinstance(body["trace_id"], str) -def test_handler_handles_pre_existing_aila_error_without_http_status() -> None: - """Pre-existing AILAError subclasses lack ClassVar http_status -- handler must - fall back to 500 and still emit the envelope shape (preflight BE-E).""" - app = _build_app({"/raise": AuthenticationError}) +def test_handler_falls_back_to_500_for_undeclared_subclass() -> None: + """An AILAError subclass that declares no ClassVar taxonomy still gets the + 500 fallback + safe envelope. The fallback path is retained for future + subclasses; issue #54 gave the six named legacy classes real statuses.""" + + class _UndeclaredError(AILAError): + pass + + app = _build_app({"/raise": _UndeclaredError}) client = TestClient(app, raise_server_exceptions=False) resp = client.get("/raise") @@ -120,18 +137,6 @@ def test_handler_handles_pre_existing_aila_error_without_http_status() -> None: assert "traceback" not in body["message"].lower() -def test_handler_handles_notfound_without_classvar() -> None: - """NotFoundError (legacy) also goes through the 500 fallback path.""" - app = _build_app({"/raise": NotFoundError}) - client = TestClient(app, raise_server_exceptions=False) - - resp = client.get("/raise") - body = resp.json() - - assert resp.status_code == 500 - _assert_envelope_shape(body) - - def test_handler_fallback_for_generic_exception() -> None: """An arbitrary Exception falls through to the generic handler (500 INTERNAL_ERROR).""" app = _build_app({"/raise": RuntimeError}) diff --git a/tests/api/errors/test_taxonomy.py b/tests/api/errors/test_taxonomy.py index 9e211d65..971ffd8d 100644 --- a/tests/api/errors/test_taxonomy.py +++ b/tests/api/errors/test_taxonomy.py @@ -17,15 +17,24 @@ from aila.platform.exceptions import ( AILAError, + AuthenticationError, ConfigValueMissingError, MissingApiKeyError, ModulePlatformNotReadyError, + NotFoundError, + RateLimitError, RouterError, SSHConnectionFailedError, + TimeoutError, + UpstreamError, + ValidationError, WorkerUnreachableError, ) # The canonical D-20 mapping. Do not mutate without revising phase decisions. +# Issue #54 (2026-07-19): the six pre-existing AILAError subclasses joined the +# taxonomy with their own ClassVar code/http_status, so they are now covered +# by the same locked-contract tests. _D20_MAPPING = [ (MissingApiKeyError, "MISSING_API_KEY", 503), (SSHConnectionFailedError, "SSH_CONNECTION_FAILED", 502), @@ -33,6 +42,12 @@ (ModulePlatformNotReadyError, "MODULE_PLATFORM_NOT_READY", 503), (ConfigValueMissingError, "CONFIG_VALUE_MISSING", 500), (WorkerUnreachableError, "WORKER_UNREACHABLE", 503), + (AuthenticationError, "AUTHENTICATION_ERROR", 401), + (RateLimitError, "RATE_LIMIT_ERROR", 429), + (NotFoundError, "NOT_FOUND_ERROR", 404), + (ValidationError, "VALIDATION_ERROR", 422), + (UpstreamError, "UPSTREAM_ERROR", 502), + (TimeoutError, "TIMEOUT_ERROR", 504), ] From 07486fdbbab88d50ecc7f266f9154bcc879d9c61 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:30:02 +0300 Subject: [PATCH 002/281] fix(contracts): RegisteredSystem tolerates extra DB columns (#61) RegisteredSystem inherited extra=forbid from the SSHIntegrationInput write payload, so constructing the read shape from an ORM row carrying undeclared columns (team_id, private_key_secret_id, future columns) raised at response-serialization time -- a latent 500 waiting on the next systems-table migration. The read shape now sets extra=ignore; the write payload keeps forbid so agents cannot submit undeclared fields. --- src/aila/platform/contracts/platform.py | 9 ++++ tests/test_platform_contracts.py | 63 +++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 tests/test_platform_contracts.py diff --git a/src/aila/platform/contracts/platform.py b/src/aila/platform/contracts/platform.py index dc1d3696..f7964737 100644 --- a/src/aila/platform/contracts/platform.py +++ b/src/aila/platform/contracts/platform.py @@ -112,8 +112,17 @@ class RegisteredSystem(SSHIntegrationInput): Extends SSHIntegrationInput with the database-assigned id and timestamps. Used as the read shape from SystemRegistryTool list/get actions. + + Unlike the write payload, this read shape sets ``extra="ignore"``: it is + constructed from ORM rows that carry columns the contract does not declare + (team_id, private_key_secret_id, and any future column). Inheriting the + parent's ``extra="forbid"`` would raise at response-serialization time on + the next migration that adds a column (issue #54/#61). The write payload + keeps ``forbid`` so agents cannot smuggle undeclared fields. """ + model_config = ConfigDict(extra="ignore") + id: int | None = None created_at: datetime | None = None updated_at: datetime | None = None diff --git a/tests/test_platform_contracts.py b/tests/test_platform_contracts.py new file mode 100644 index 00000000..35a42039 --- /dev/null +++ b/tests/test_platform_contracts.py @@ -0,0 +1,63 @@ +"""Platform contract shape tests (issue #61). + +RegisteredSystem is the DB read shape and MUST tolerate columns the contract +does not declare (team_id, private_key_secret_id, future columns), otherwise +construction from an ORM row raises at response-serialization time -- the same +class of latent 500 documented for MalwareTargetSummary.capability_profile. +SSHIntegrationInput is the write payload and MUST keep rejecting undeclared +fields so agents cannot smuggle extra keys. +""" +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from aila.platform.contracts.platform import RegisteredSystem, SSHIntegrationInput + + +def test_registered_system_ignores_undeclared_db_columns() -> None: + """A RegisteredSystem built from a row with extra columns does not raise.""" + row = { + "id": 7, + "name": "web-01", + "host": "10.0.0.5", + "username": "aila", + # Columns present on ManagedSystemRecord but not declared on the contract: + "team_id": 3, + "private_key_secret_id": "sec_abc", + "some_future_column": "value", + } + system = RegisteredSystem.model_validate(row) + assert system.id == 7 + assert system.name == "web-01" + # The undeclared columns are ignored, not surfaced as attributes. + assert not hasattr(system, "team_id") + assert not hasattr(system, "some_future_column") + + +def test_registered_system_extra_config_is_ignore() -> None: + """The read shape overrides the parent's forbid with ignore.""" + assert RegisteredSystem.model_config.get("extra") == "ignore" + + +def test_ssh_integration_input_still_forbids_extra() -> None: + """The write payload keeps rejecting undeclared fields (agent cannot smuggle).""" + assert SSHIntegrationInput.model_config.get("extra") == "forbid" + with pytest.raises(ValidationError): + SSHIntegrationInput.model_validate( + { + "name": "web-01", + "host": "10.0.0.5", + "username": "aila", + "team_id": 3, # not a declared write field -> rejected + } + ) + + +def test_ssh_integration_input_accepts_declared_fields() -> None: + """A well-formed write payload still validates.""" + payload = SSHIntegrationInput.model_validate( + {"name": "web-01", "host": "10.0.0.5", "username": "aila"} + ) + assert payload.port == 22 + assert payload.distro == "unknown" From fb32a8204e46fb1d90a079d43b4c401ac94002f6 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:34:01 +0300 Subject: [PATCH 003/281] fix(contracts): guard observables against non-JSON values at construction (#61) ReasoningCaseState.observables and ReasoningTurnDecision.observables are dict[str, Any] persisted to the DB and passed as task kwargs, both of which json-encode them. A datetime/bytes/set passed Pydantic's dict[str, Any] check then crashed later at json.dumps (enqueue or DB write). A field_validator now fails fast at construction with the offending detail. Existing string-valued observables are unaffected. --- src/aila/platform/contracts/reasoning.py | 30 +++++++++++++++++++++++- tests/test_platform_contracts.py | 23 ++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/aila/platform/contracts/reasoning.py b/src/aila/platform/contracts/reasoning.py index 6d307d87..5f946316 100644 --- a/src/aila/platform/contracts/reasoning.py +++ b/src/aila/platform/contracts/reasoning.py @@ -1,8 +1,9 @@ from __future__ import annotations +import json from typing import Any, Literal -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, Field, field_validator, model_validator __all__ = [ "ReasoningAction", @@ -158,6 +159,23 @@ class ReasoningGraphDiff(BaseModel): added_edges: list[ReasoningGraphEdge] = Field(default_factory=list) removed_edges: list[ReasoningGraphEdge] = Field(default_factory=list) +def _require_json_serializable(value: dict[str, Any]) -> dict[str, Any]: + """Reject an observables dict that cannot round-trip through json.dumps. + + observables is persisted to the DB and passed as task kwargs, both of which + json-encode it. A datetime, bytes, set, or custom object passes Pydantic's + ``dict[str, Any]`` check but crashes later at serialization time (issue + #61). Fail fast at construction with the offending detail instead. + """ + try: + json.dumps(value) + except (TypeError, ValueError) as exc: + raise ValueError( + f"observables must be JSON-serializable (DB + task-kwarg persistence): {exc}" + ) from exc + return value + + class ReasoningCaseState(BaseModel): """Normalized reasoning state carried across investigation turns.""" @@ -172,6 +190,11 @@ class ReasoningCaseState(BaseModel): # number" (legacy rows). Filled in by ``absorb(turn_number=N)``. current_turn: int = 0 + @field_validator("observables") + @classmethod + def _observables_serializable(cls, v: dict[str, Any]) -> dict[str, Any]: + return _require_json_serializable(v) + class ReasoningOperatorSteering(BaseModel): """Operator-provided steering constraints for one reasoning session.""" @@ -267,6 +290,11 @@ class ReasoningTurnDecision(BaseModel): edit_comment: str | None = None payload: dict[str, Any] = Field(default_factory=dict) + @field_validator("observables") + @classmethod + def _observables_serializable(cls, v: dict[str, Any]) -> dict[str, Any]: + return _require_json_serializable(v) + @model_validator(mode="after") def _validate_tool_run_command(self) -> ReasoningTurnDecision: """When ``action='tool_run'``, ``command`` MUST parse as JSON with diff --git a/tests/test_platform_contracts.py b/tests/test_platform_contracts.py index 35a42039..e9b8b1c5 100644 --- a/tests/test_platform_contracts.py +++ b/tests/test_platform_contracts.py @@ -9,10 +9,13 @@ class of latent 500 documented for MalwareTargetSummary.capability_profile. """ from __future__ import annotations +from datetime import datetime + import pytest from pydantic import ValidationError from aila.platform.contracts.platform import RegisteredSystem, SSHIntegrationInput +from aila.platform.contracts.reasoning import ReasoningCaseState, ReasoningTurnDecision def test_registered_system_ignores_undeclared_db_columns() -> None: @@ -61,3 +64,23 @@ def test_ssh_integration_input_accepts_declared_fields() -> None: ) assert payload.port == 22 assert payload.distro == "unknown" + + +def test_case_state_rejects_non_json_observables() -> None: + """A datetime in observables fails at construction, not later at json.dumps.""" + with pytest.raises(ValidationError): + ReasoningCaseState(observables={"when": datetime(2026, 7, 19)}) + + +def test_turn_decision_rejects_non_json_observables() -> None: + """Bytes in observables fail at construction.""" + with pytest.raises(ValidationError): + ReasoningTurnDecision(reasoning="x", observables={"raw": b"\x00\x01"}) + + +def test_observables_accept_plain_json_values() -> None: + """JSON-serializable observables still construct (regression guard).""" + cs = ReasoningCaseState(observables={"k": "v", "n": 1, "nested": {"a": [1, 2]}}) + assert cs.observables["n"] == 1 + td = ReasoningTurnDecision(reasoning="x", observables={"k": "v"}) + assert td.observables["k"] == "v" From 04afa9dd631611dad5f1c8978411c7e7056a424b Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:39:17 +0300 Subject: [PATCH 004/281] fix(llm): declare LLMResponse pipeline metadata fields (#44) _enrich_response constructed LLMResponse with classification/confidence/seal_id/pipeline_metadata, but the frozen+slots dataclass declared none of them, so any pipeline step that wrote a non-None value raised TypeError at the enrich boundary. Declare the four fields (default None). Repairs the enrich-response tests in test_pipeline/test_sanitize/test_gate/test_validate that already assert this behavior. --- src/aila/platform/llm/client.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/aila/platform/llm/client.py b/src/aila/platform/llm/client.py index 06a2e888..2952c7dd 100644 --- a/src/aila/platform/llm/client.py +++ b/src/aila/platform/llm/client.py @@ -250,7 +250,16 @@ class LLMResponse: usage: dict[str, int] = field(default_factory=dict) disabled: bool = False finish_reason: str = "" - # Pipeline metadata (Phase 116) -- default None, transparent to existing callers + # Pipeline metadata (Phase 116) -- default None, transparent to existing callers. + # _enrich_response() populates these from the pipeline ctx after the + # classify / gate / seal steps run. Declaring them is required: the + # dataclass is frozen + slots, so _enrich_response constructing with these + # kwargs raised TypeError the moment any step wrote a non-None value + # (issue #44). + classification: Any = None + confidence: Any = None + seal_id: str | None = None + pipeline_metadata: dict[str, Any] | None = None # Retry budget -- TIGHT BY DESIGN. # # Background (the change shipped on 2026-06-13 after the maddie / From aa1732aa30c6b574808f33e5fea7ea7c7c5da685 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:40:22 +0300 Subject: [PATCH 005/281] fix(llm): boundary-match temperature-reject markers, drop dead health lock (#44) Short rejection markers (o1/o3/o4) were matched as bare substrings, so proto1/audio1 and similar ids had temperature silently stripped. Match markers on alphanumeric boundaries via regex instead. Also remove _LLM_HEALTH_LOCK, an import-time asyncio.Lock() that was never acquired anywhere. --- src/aila/platform/llm/client.py | 15 ++++-- .../platform/llm/test_temperature_markers.py | 51 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tests/platform/llm/test_temperature_markers.py diff --git a/src/aila/platform/llm/client.py b/src/aila/platform/llm/client.py index 2952c7dd..195c87d6 100644 --- a/src/aila/platform/llm/client.py +++ b/src/aila/platform/llm/client.py @@ -23,6 +23,7 @@ import json import logging import os +import re import time as _time_mod from collections.abc import Awaitable, Callable from dataclasses import dataclass, field @@ -58,7 +59,6 @@ # branches may have hit the same outage. _LAST_LLM_OK_AT: float = 0.0 _LAST_LLM_ERROR_AT: float = 0.0 -_LLM_HEALTH_LOCK = asyncio.Lock() def _record_llm_ok() -> None: @@ -166,9 +166,18 @@ def _get_rejection_markers() -> tuple[str, ...]: def _model_supports_temperature(model_id: str) -> bool: - """Return False when the routed model is known to reject ``temperature``.""" + """Return False when the routed model is known to reject ``temperature``. + + Markers match on alphanumeric boundaries so a short marker like ``o1`` does + not spuriously fire inside an unrelated id (``proto1``, ``audio1``); the + old ``marker in mid`` substring test stripped temperature from those by + accident (issue #44). + """ mid = (model_id or "").lower() - return not any(marker in mid for marker in _get_rejection_markers()) + return not any( + re.search(rf"(? str: diff --git a/tests/platform/llm/test_temperature_markers.py b/tests/platform/llm/test_temperature_markers.py new file mode 100644 index 00000000..f0710257 --- /dev/null +++ b/tests/platform/llm/test_temperature_markers.py @@ -0,0 +1,51 @@ +"""Temperature-rejection marker matching (issue #44). + +Markers that disable the ``temperature`` kwarg must match on alphanumeric +boundaries. The prior ``marker in model_id`` substring test made short markers +like ``o1`` fire inside unrelated ids (``proto1``, ``audio1``), silently +stripping temperature from models that accept it. +""" +from __future__ import annotations + +import pytest + +from aila.platform.llm import client as client_mod + +_MARKERS = ("o1", "o3", "o4", "claude-opus-4-6", "gpt-5", "hadi") + + +@pytest.fixture(autouse=True) +def _fixed_markers(monkeypatch: pytest.MonkeyPatch) -> None: + """Pin the resolved marker list so the test does not read env/DB.""" + monkeypatch.setattr(client_mod, "_resolved_markers", _MARKERS) + + +@pytest.mark.parametrize( + "model_id", + [ + "openai/o1-preview", + "o3-mini", + "o4", + "anthropic/claude-opus-4-6-thinking", + "claude-opus-4-6", + "gpt-5", + "gpt-5-turbo", + "hadi", + ], +) +def test_rejecting_models_disable_temperature(model_id: str) -> None: + assert client_mod._model_supports_temperature(model_id) is False + + +@pytest.mark.parametrize( + "model_id", + [ + "proto1", # ends with "o1" but not on a boundary + "audio1-model", # contains "o1" mid-token + "gpt-4o-mini", # no marker + "claude-sonnet-4-5", + "", + ], +) +def test_non_rejecting_models_keep_temperature(model_id: str) -> None: + assert client_mod._model_supports_temperature(model_id) is True From a9166fa4f20b64416efc2797f201090d8551f043 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:44:44 +0300 Subject: [PATCH 006/281] feat(config): add ConfigRegistry.get_sync for sync call sites (C3, #65/#38) Sync call sites (proxy resolution, budget ceiling, worker bootstrap) could not resolve config without producing an un-awaited coroutine from the async get. get_sync mirrors get's resolution order (env > cache > DB > schema default) using the sync engine via session_scope. Cache access is lock-free (GIL-atomic dict ops; benign check-then-populate race). This is the foundation the #65 and #38 fixes consume. --- src/aila/storage/registry.py | 57 +++++++++++++++++++- tests/test_config_registry_get_sync.py | 72 ++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 tests/test_config_registry_get_sync.py diff --git a/src/aila/storage/registry.py b/src/aila/storage/registry.py index fa028ecc..b412259d 100644 --- a/src/aila/storage/registry.py +++ b/src/aila/storage/registry.py @@ -29,7 +29,7 @@ from sqlmodel import select from ..platform.contracts._common import utc_now -from .database import async_session_scope +from .database import async_session_scope, session_scope from .db_models import ConfigEntryRecord __all__ = ["ConfigRegistry", "SchemaRegistry"] @@ -153,6 +153,61 @@ async def get(self, namespace: str, key: str) -> Any: return default_val return None + def get_sync(self, namespace: str, key: str) -> Any: + """Synchronous twin of :meth:`get` for sync call sites. + + Same resolution order (env var > cache > DB value > schema default) as + the async ``get``, but usable from a plain ``def`` without producing an + un-awaited coroutine. Sync call sites (proxy resolution, budget ceiling, + worker bootstrap) previously called ``get`` without ``await`` and either + guarded with ``hasattr(x, "__await__")`` or -- when they forgot -- + operated on the coroutine object itself (issue #65/#38). + + The DB read uses the sync engine via ``session_scope`` (psycopg). Cache + access is lock-free by design: dict get/set are atomic under the GIL and + the check-then-populate race is benign -- at worst a redundant DB read + and an idempotent overwrite with the same value. The async ``_cache_lock`` + cannot be acquired from a sync context, so it is intentionally not used + here. + """ + env_name = f"AILA_{namespace.upper()}_{key.upper()}" + env_val = os.environ.get(env_name) + + schema = self._schemas.get(namespace) + field_info = schema.model_fields.get(key) if schema else None + + if env_val is not None: + return _cast_value(env_val, field_info) + + cache_key = (namespace, key) + entry = self._cache.get(cache_key) + if entry is not None and time.monotonic() < entry.expires_at: + return entry.value + + with session_scope() as session: + row = session.exec( + select(ConfigEntryRecord).where( + ConfigEntryRecord.namespace == namespace, + ConfigEntryRecord.key == key, + ) + ).first() + if row is not None: + value = _cast_value(row.value, field_info) + self._cache[cache_key] = _CacheEntry( + value=value, + expires_at=time.monotonic() + self._cache_ttl, + ) + return value + + if schema and field_info is not None: + default_val = field_info.default + self._cache[cache_key] = _CacheEntry( + value=default_val, + expires_at=time.monotonic() + self._cache_ttl, + ) + return default_val + return None + async def set(self, namespace: str, key: str, value: str) -> None: """Persist value to DB after type-validating against registered schema. Raises ValueError if namespace/key is not in any registered schema. diff --git a/tests/test_config_registry_get_sync.py b/tests/test_config_registry_get_sync.py new file mode 100644 index 00000000..8cfec478 --- /dev/null +++ b/tests/test_config_registry_get_sync.py @@ -0,0 +1,72 @@ +"""ConfigRegistry.get_sync resolution tests (issue #65/#38, contract C3). + +get_sync is the synchronous twin of the async get, for sync call sites that +must not produce an un-awaited coroutine. Resolution order matches get: +env var > cache > DB value > schema default. The DB branches patch +session_scope so the test stays hermetic (no DB writes). +""" +from __future__ import annotations + +import time +from unittest.mock import MagicMock, patch + +import pytest +from pydantic import BaseModel + +from aila.storage.registry import ConfigRegistry, _CacheEntry + + +class _Schema(BaseModel): + foo: int = 42 + name: str = "default" + + +def _registry() -> ConfigRegistry: + reg = ConfigRegistry() + reg._schemas["testns"] = _Schema + return reg + + +def _session_cm(row: object | None) -> MagicMock: + session = MagicMock() + session.exec.return_value.first.return_value = row + cm = MagicMock() + cm.__enter__.return_value = session + cm.__exit__.return_value = False + return cm + + +def test_env_override_is_cast(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("AILA_TESTNS_FOO", "7") + assert _registry().get_sync("testns", "foo") == 7 + + +def test_cache_hit_returned() -> None: + reg = _registry() + reg._cache[("testns", "foo")] = _CacheEntry(value=99, expires_at=time.monotonic() + 100) + assert reg.get_sync("testns", "foo") == 99 + + +def test_db_value_cast_and_cached() -> None: + reg = _registry() + row = MagicMock() + row.value = "13" + with patch("aila.storage.registry.session_scope", return_value=_session_cm(row)): + assert reg.get_sync("testns", "foo") == 13 + # cached after the read + assert reg._cache[("testns", "foo")].value == 13 + + +def test_schema_default_when_row_absent() -> None: + reg = _registry() + with patch("aila.storage.registry.session_scope", return_value=_session_cm(None)): + assert reg.get_sync("testns", "foo") == 42 + + +def test_never_returns_a_coroutine() -> None: + """The whole point: a sync caller gets a value, never an awaitable.""" + reg = _registry() + with patch("aila.storage.registry.session_scope", return_value=_session_cm(None)): + value = reg.get_sync("testns", "name") + assert value == "default" + assert not hasattr(value, "__await__") From d299e5a4d97adbfae6d74d670fd5fe2f20e0e7fe Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:46:14 +0300 Subject: [PATCH 007/281] fix(vulnerability): resolve proxy via get_sync, not un-awaited get (#65) _resolve_proxy is sync but called the async ConfigRegistry.get without await, so str( or '') returned the coroutine repr as the proxy URL and never reached the env fallback -- breaking every registry-configured provider client. Switch to get_sync (contract C3). --- .../modules/vulnerability/providers/_http.py | 7 ++- tests/test_vulnerability_proxy_resolution.py | 53 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/test_vulnerability_proxy_resolution.py diff --git a/src/aila/modules/vulnerability/providers/_http.py b/src/aila/modules/vulnerability/providers/_http.py index 945a11c0..c4feb7b9 100644 --- a/src/aila/modules/vulnerability/providers/_http.py +++ b/src/aila/modules/vulnerability/providers/_http.py @@ -41,8 +41,11 @@ def _resolve_proxy( Proxy URL string, or None when no proxy is configured. """ if registry is not None: - https_val = str(registry.get("platform", "https_proxy") or "").strip() - http_val = str(registry.get("platform", "http_proxy") or "").strip() + # Sync function -- must use get_sync. The async get without await made + # a coroutine whose truthy repr became the proxy URL and skipped the + # env fallback (issue #65). + https_val = str(registry.get_sync("platform", "https_proxy") or "").strip() + http_val = str(registry.get_sync("platform", "http_proxy") or "").strip() if https_val: return https_val if http_val: diff --git a/tests/test_vulnerability_proxy_resolution.py b/tests/test_vulnerability_proxy_resolution.py new file mode 100644 index 00000000..aa9adee6 --- /dev/null +++ b/tests/test_vulnerability_proxy_resolution.py @@ -0,0 +1,53 @@ +"""Proxy resolution must not leak an un-awaited coroutine (issue #65). + +_resolve_proxy is a sync function; it must call ConfigRegistry.get_sync, not the +async get. The old code called get without await, so str( or "") +returned the coroutine's repr as the proxy URL and never reached the env +fallback. The fake registry below exposes BOTH get_sync and an async get, so a +revert of the fix would return the coroutine repr and fail these assertions. +""" +from __future__ import annotations + +import pytest + +from aila.modules.vulnerability.providers._http import _resolve_proxy + + +class _FakeRegistry: + def __init__(self, values: dict[tuple[str, str], str]) -> None: + self._values = values + + def get_sync(self, namespace: str, key: str) -> str | None: + return self._values.get((namespace, key)) + + async def get(self, namespace: str, key: str) -> str | None: # async twin + return self._values.get((namespace, key)) + + +def test_registry_https_proxy_used() -> None: + reg = _FakeRegistry({("platform", "https_proxy"): "http://proxy:8080"}) + assert _resolve_proxy(reg) == "http://proxy:8080" + + +def test_result_is_a_real_string_not_a_coroutine() -> None: + reg = _FakeRegistry({("platform", "https_proxy"): "http://proxy:8080"}) + result = _resolve_proxy(reg) + assert result is not None + assert "coroutine" not in result + assert not hasattr(result, "__await__") + + +def test_http_proxy_fallback_within_registry() -> None: + reg = _FakeRegistry({("platform", "http_proxy"): "http://p2:3128"}) + assert _resolve_proxy(reg) == "http://p2:3128" + + +def test_env_fallback_when_registry_empty(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("HTTPS_PROXY", "http://envproxy:9") + assert _resolve_proxy(_FakeRegistry({})) == "http://envproxy:9" + + +def test_none_when_nothing_configured(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("HTTPS_PROXY", raising=False) + monkeypatch.delenv("HTTP_PROXY", raising=False) + assert _resolve_proxy(None) is None From d967b0bc05ad58c6b3048eaa0aa2b724d1c60413 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 00:50:36 +0300 Subject: [PATCH 008/281] fix(llm): enforce per-run token budget via get_sync (#38) _resolve_ceiling (sync) called the async ConfigRegistry.get without await; the resulting coroutine is not None and int() of it raises, which the try/except swallowed -- so the per-run token ceiling always resolved to 0 (unlimited) and the budget was dead code. Switch to get_sync. Adds a reproduction test with an async-get registry stub that failed DID-NOT-RAISE before the fix. --- src/aila/platform/llm/cost.py | 5 ++++- tests/platform/llm/test_cost.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/aila/platform/llm/cost.py b/src/aila/platform/llm/cost.py index 223fa724..5d9ec60c 100644 --- a/src/aila/platform/llm/cost.py +++ b/src/aila/platform/llm/cost.py @@ -146,7 +146,10 @@ def _resolve_ceiling(self, task_type: str) -> int: ``platform`` namespace. Returns 0 (unlimited) if the key is missing or cannot be converted to int. """ - raw = self._registry.get("platform", f"llm_budget_max_total_tokens_{task_type}") + # Use get_sync here (sync method): the async get produced an un-awaited + # coroutine that was not None and failed conversion, so the budget + # ceiling silently resolved to 0 -- dead code (issue #38). + raw = self._registry.get_sync("platform", f"llm_budget_max_total_tokens_{task_type}") if raw is None: return 0 try: diff --git a/tests/platform/llm/test_cost.py b/tests/platform/llm/test_cost.py index e4db2300..8fda982c 100644 --- a/tests/platform/llm/test_cost.py +++ b/tests/platform/llm/test_cost.py @@ -20,6 +20,24 @@ def __init__(self, data: dict[str, Any] | None = None) -> None: def get(self, namespace: str, key: str) -> Any: return self._data.get(f"{namespace}.{key}") + def get_sync(self, namespace: str, key: str) -> Any: + return self._data.get(f"{namespace}.{key}") + + +class _AsyncGetStubRegistry: + """Registry whose ``get`` is async (like the real ConfigRegistry) and whose + ``get_sync`` is the sync twin. Reproduces production shape: calling ``get`` + without await yields a coroutine, not a value.""" + + def __init__(self, data: dict[str, Any] | None = None) -> None: + self._data: dict[str, Any] = data or {} + + async def get(self, namespace: str, key: str) -> Any: + return self._data.get(f"{namespace}.{key}") + + def get_sync(self, namespace: str, key: str) -> Any: + return self._data.get(f"{namespace}.{key}") + # --------------------------------------------------------------------------- # CostTracker.record @@ -184,6 +202,20 @@ def test_config_change_between_checks(self) -> None: tracker.check_budget("r1", "scoring") +class TestBudgetCeilingUsesSyncResolver: + """_resolve_ceiling must use get_sync so the async registry.get does not + return an un-awaited coroutine that silently disables the budget (#38).""" + + def test_ceiling_enforced_with_async_registry(self) -> None: + reg = _AsyncGetStubRegistry( + {"platform.llm_budget_max_total_tokens_scoring": 100} + ) + tracker = CostTracker(RunMemory(), reg) + tracker.record("r1", {"prompt_tokens": 60, "completion_tokens": 50}) + with pytest.raises(BudgetExceededError): + tracker.check_budget("r1", "scoring") + + # --------------------------------------------------------------------------- # Integration: Client + CostTracker wiring (Plan 02) # --------------------------------------------------------------------------- From 1a92a1f7f4220ee634d441f7284dd5aa13a30f39 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 01:02:24 +0300 Subject: [PATCH 009/281] chore(tests): make touched test files ruff-clean Hoist in-function imports to module top (PLC0415) and rename local test doubles to conform to naming rules (N818 _FakeExc->_FakeError, N806 MockOAI->mock_oai) in the three pre-existing test files edited during Wave-1 fixes. No behavior change; failure set is unchanged (pre-existing stale-mock cluster verified via revert-compare). --- tests/api/errors/test_handlers.py | 64 +++++++++++------------------ tests/api/errors/test_taxonomy.py | 8 ++-- tests/platform/llm/test_cost.py | 67 ++++++++++--------------------- 3 files changed, 48 insertions(+), 91 deletions(-) diff --git a/tests/api/errors/test_handlers.py b/tests/api/errors/test_handlers.py index 9095af10..b1d75841 100644 --- a/tests/api/errors/test_handlers.py +++ b/tests/api/errors/test_handlers.py @@ -11,12 +11,23 @@ """ from __future__ import annotations +import asyncio + import pytest import structlog from fastapi import FastAPI +from fastapi.exceptions import RequestValidationError from fastapi.testclient import TestClient +import aila.api.errors as errors_pkg +from aila.api.app import create_app from aila.api.errors import ErrorEnvelope, register_error_handlers +from aila.api.errors.handlers import ( + _derive_module_label, + generic_error_handler, + typed_error_handler, + validation_error_handler, +) from aila.api.errors.hints import ERROR_HINTS from aila.platform.exceptions import ( AILAError, @@ -163,10 +174,6 @@ def test_handler_includes_trace_id_from_structlog() -> None: exposes it as ``trace_id``. We call the handler directly (no HTTP) to bind contextvars on the same thread the handler reads them from. """ - import asyncio - - from aila.api.errors.handlers import typed_error_handler - async def _drive() -> ErrorEnvelope: structlog.contextvars.clear_contextvars() structlog.contextvars.bind_contextvars(correlation_id="trace-xyz-123") @@ -185,10 +192,6 @@ async def _drive() -> ErrorEnvelope: def test_handler_handles_middleware_error_with_no_trace_id(monkeypatch) -> None: """When contextvars is empty, trace_id is None and no crash occurs (D-26).""" - import asyncio - - from aila.api.errors.handlers import typed_error_handler - monkeypatch.setattr( "aila.api.errors.handlers.structlog.contextvars.get_contextvars", lambda: {}, @@ -208,52 +211,42 @@ async def _drive() -> ErrorEnvelope: def test_handler_module_label_derivation_nested_modules() -> None: """aila.modules.X.* → module label ``X`` (preflight algorithm).""" - from aila.api.errors.handlers import _derive_module_label - - class _FakeExc(Exception): + class _FakeError(Exception): pass - _FakeExc.__module__ = "aila.modules.vulnerability.services.reports" - assert _derive_module_label(_FakeExc("x")) == "vulnerability" + _FakeError.__module__ = "aila.modules.vulnerability.services.reports" + assert _derive_module_label(_FakeError("x")) == "vulnerability" def test_handler_module_label_derivation_platform() -> None: """aila.platform.* → module label ``platform``.""" - from aila.api.errors.handlers import _derive_module_label - - class _FakeExc(Exception): + class _FakeError(Exception): pass - _FakeExc.__module__ = "aila.platform.llm.client" - assert _derive_module_label(_FakeExc("x")) == "platform" + _FakeError.__module__ = "aila.platform.llm.client" + assert _derive_module_label(_FakeError("x")) == "platform" def test_handler_module_label_derivation_api() -> None: """aila.api.* → module label ``api``.""" - from aila.api.errors.handlers import _derive_module_label - - class _FakeExc(Exception): + class _FakeError(Exception): pass - _FakeExc.__module__ = "aila.api.routers.foo" - assert _derive_module_label(_FakeExc("x")) == "api" + _FakeError.__module__ = "aila.api.routers.foo" + assert _derive_module_label(_FakeError("x")) == "api" def test_handler_module_label_fallback_for_unknown_prefix() -> None: """Modules not rooted at ``aila`` fall back to ``platform``.""" - from aila.api.errors.handlers import _derive_module_label - - class _FakeExc(Exception): + class _FakeError(Exception): pass - _FakeExc.__module__ = "some.third.party.lib" - assert _derive_module_label(_FakeExc("x")) == "platform" + _FakeError.__module__ = "some.third.party.lib" + assert _derive_module_label(_FakeError("x")) == "platform" def test_handler_registered_on_app() -> None: """After create_app()/register_error_handlers, all three handlers are wired.""" - from fastapi.exceptions import RequestValidationError - app = FastAPI() register_error_handlers(app) @@ -265,24 +258,13 @@ def test_handler_registered_on_app() -> None: def test_errors_package_phase2_exports() -> None: """Phase-2 package exports include register_error_handlers.""" - import aila.api.errors as errors_pkg - assert hasattr(errors_pkg, "register_error_handlers") assert "register_error_handlers" in errors_pkg.__all__ def test_handler_registered_in_create_app() -> None: """The real create_app() wires the three envelope handlers.""" - from fastapi.exceptions import RequestValidationError - - from aila.api.app import create_app - app = create_app() - from aila.api.errors.handlers import ( - generic_error_handler, - typed_error_handler, - validation_error_handler, - ) assert app.exception_handlers.get(AILAError) is typed_error_handler assert app.exception_handlers.get(RequestValidationError) is validation_error_handler diff --git a/tests/api/errors/test_taxonomy.py b/tests/api/errors/test_taxonomy.py index 971ffd8d..84661246 100644 --- a/tests/api/errors/test_taxonomy.py +++ b/tests/api/errors/test_taxonomy.py @@ -15,6 +15,8 @@ import pytest +import aila.api.errors +from aila.api.errors.envelope import ErrorEnvelope from aila.platform.exceptions import ( AILAError, AuthenticationError, @@ -101,8 +103,6 @@ def test_envelope_required_fields() -> None: code + message required; hint + trace_id optional (default None). """ - from aila.api.errors.envelope import ErrorEnvelope - fields = ErrorEnvelope.model_fields assert set(fields.keys()) == {"code", "message", "hint", "trace_id"} assert fields["code"].is_required() @@ -118,10 +118,8 @@ def test_envelope_required_fields() -> None: def test_errors_package_phase1_exports() -> None: """Phase-1 package exports: ErrorEnvelope + ERROR_HINTS only.""" - import aila.api.errors as errors_pkg - # Force re-import in case another test mutated module state. - errors_pkg = importlib.reload(errors_pkg) + errors_pkg = importlib.reload(aila.api.errors) assert hasattr(errors_pkg, "ErrorEnvelope") assert hasattr(errors_pkg, "ERROR_HINTS") diff --git a/tests/platform/llm/test_cost.py b/tests/platform/llm/test_cost.py index 8fda982c..bc977df5 100644 --- a/tests/platform/llm/test_cost.py +++ b/tests/platform/llm/test_cost.py @@ -2,13 +2,22 @@ from __future__ import annotations +import json from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch import pytest -from aila.platform.llm.cost import CostTracker +from aila.platform.llm.client import AilaLLMClient +from aila.platform.llm.cost import ( + CostTracker, + calculate_cost_usd, + emit_missing_pricing_notification, + persist_cost_record, +) from aila.platform.llm.errors import BudgetExceededError, LLMError from aila.platform.llm.run_memory import RunMemory +from aila.storage.db_models import NotificationRecord class _StubRegistry: @@ -231,9 +240,7 @@ def _make_client_with_tracker( Returns (client, cost_tracker) pair. """ - from unittest.mock import MagicMock - from aila.platform.llm.client import AilaLLMClient reg_data: dict[str, Any] = {} if budget is not None: @@ -254,7 +261,6 @@ def _make_client_with_tracker( @pytest.mark.asyncio async def test_chat_records_usage_with_run_id(self) -> None: """chat() with run_id records token usage to CostTracker.""" - from unittest.mock import AsyncMock, MagicMock, patch client, tracker = self._make_client_with_tracker() @@ -272,10 +278,10 @@ async def test_chat_records_usage_with_run_id(self) -> None: completion.choices = [choice] completion.usage = usage_mock - with patch("aila.platform.llm.client.AsyncOpenAI") as MockOAI: + with patch("aila.platform.llm.client.AsyncOpenAI") as mock_oai: mock_instance = AsyncMock() mock_instance.chat.completions.create = AsyncMock(return_value=completion) - MockOAI.return_value = mock_instance + mock_oai.return_value = mock_instance response = await client.chat( "scoring", @@ -307,7 +313,6 @@ async def test_budget_exceeded_blocks_call(self) -> None: @pytest.mark.asyncio async def test_no_run_id_records_under_no_run(self) -> None: """chat() without run_id records under _no_run sentinel.""" - from unittest.mock import AsyncMock, MagicMock, patch client, tracker = self._make_client_with_tracker() @@ -325,10 +330,10 @@ async def test_no_run_id_records_under_no_run(self) -> None: completion.choices = [choice] completion.usage = usage_mock - with patch("aila.platform.llm.client.AsyncOpenAI") as MockOAI: + with patch("aila.platform.llm.client.AsyncOpenAI") as mock_oai: mock_instance = AsyncMock() mock_instance.chat.completions.create = AsyncMock(return_value=completion) - MockOAI.return_value = mock_instance + mock_oai.return_value = mock_instance await client.chat( "scoring", @@ -342,7 +347,6 @@ async def test_no_run_id_records_under_no_run(self) -> None: @pytest.mark.asyncio async def test_no_tracker_backward_compatible(self) -> None: """Client without cost_tracker set still works normally.""" - from unittest.mock import AsyncMock, MagicMock, patch reg = _StubRegistry() secret_store = MagicMock() @@ -369,10 +373,10 @@ async def test_no_tracker_backward_compatible(self) -> None: completion.choices = [choice] completion.usage = usage_mock - with patch("aila.platform.llm.client.AsyncOpenAI") as MockOAI: + with patch("aila.platform.llm.client.AsyncOpenAI") as mock_oai: mock_instance = AsyncMock() mock_instance.chat.completions.create = AsyncMock(return_value=completion) - MockOAI.return_value = mock_instance + mock_oai.return_value = mock_instance response = await client_obj.chat( "scoring", @@ -384,7 +388,6 @@ async def test_no_tracker_backward_compatible(self) -> None: @pytest.mark.asyncio async def test_run_id_flows_to_pipeline_ctx(self) -> None: """run_id is set in pipeline ctx for seal step to read.""" - from unittest.mock import AsyncMock, MagicMock, patch client, tracker = self._make_client_with_tracker() @@ -413,10 +416,10 @@ async def spy_run(**kwargs: Any) -> Any: completion.choices = [choice] completion.usage = usage_mock - with patch("aila.platform.llm.client.AsyncOpenAI") as MockOAI: + with patch("aila.platform.llm.client.AsyncOpenAI") as mock_oai: mock_instance = AsyncMock() mock_instance.chat.completions.create = AsyncMock(return_value=completion) - MockOAI.return_value = mock_instance + mock_oai.return_value = mock_instance await client.chat( "scoring", @@ -429,8 +432,6 @@ async def spy_run(**kwargs: Any) -> Any: @pytest.mark.asyncio async def test_chat_json_accepts_run_id(self) -> None: """chat_json() also accepts run_id kwarg.""" - import json - from unittest.mock import AsyncMock, MagicMock, patch client, tracker = self._make_client_with_tracker() @@ -448,10 +449,10 @@ async def test_chat_json_accepts_run_id(self) -> None: completion.choices = [choice] completion.usage = usage_mock - with patch("aila.platform.llm.client.AsyncOpenAI") as MockOAI: + with patch("aila.platform.llm.client.AsyncOpenAI") as mock_oai: mock_instance = AsyncMock() mock_instance.chat.completions.create = AsyncMock(return_value=completion) - MockOAI.return_value = mock_instance + mock_oai.return_value = mock_instance await client.chat_json( "scoring", @@ -465,7 +466,6 @@ async def test_chat_json_accepts_run_id(self) -> None: def test_chat_sync_accepts_run_id(self) -> None: """chat_sync() passes run_id through.""" - from unittest.mock import AsyncMock, MagicMock, patch client, tracker = self._make_client_with_tracker() @@ -483,10 +483,10 @@ def test_chat_sync_accepts_run_id(self) -> None: completion.choices = [choice] completion.usage = usage_mock - with patch("aila.platform.llm.client.AsyncOpenAI") as MockOAI: + with patch("aila.platform.llm.client.AsyncOpenAI") as mock_oai: mock_instance = AsyncMock() mock_instance.chat.completions.create = AsyncMock(return_value=completion) - MockOAI.return_value = mock_instance + mock_oai.return_value = mock_instance client.chat_sync( "scoring", @@ -519,7 +519,6 @@ class TestCalculateCostUsd: @pytest.mark.asyncio async def test_calculate_cost_usd_both_keys_present(self) -> None: """Returns (cost, True) when both pricing keys exist and are valid.""" - from aila.platform.llm.cost import calculate_cost_usd registry = _AsyncStubRegistry({ "platform.llm_cost_per_1k_prompt_gpt-4o": 0.005, @@ -538,7 +537,6 @@ async def test_calculate_cost_usd_both_keys_present(self) -> None: @pytest.mark.asyncio async def test_calculate_cost_usd_missing_keys(self) -> None: """Returns (0.0, False) when pricing keys are missing.""" - from aila.platform.llm.cost import calculate_cost_usd registry = _AsyncStubRegistry() # no keys cost, configured = await calculate_cost_usd( @@ -553,7 +551,6 @@ async def test_calculate_cost_usd_missing_keys(self) -> None: @pytest.mark.asyncio async def test_calculate_cost_usd_non_numeric_keys(self) -> None: """Returns (0.0, False) when pricing keys are non-numeric strings.""" - from aila.platform.llm.cost import calculate_cost_usd registry = _AsyncStubRegistry({ "platform.llm_cost_per_1k_prompt_gpt-4o": "not-a-number", @@ -571,7 +568,6 @@ async def test_calculate_cost_usd_non_numeric_keys(self) -> None: @pytest.mark.asyncio async def test_calculate_cost_usd_negative_price_rejected(self) -> None: """Returns (0.0, False) when prices are negative (T-175-01).""" - from aila.platform.llm.cost import calculate_cost_usd registry = _AsyncStubRegistry({ "platform.llm_cost_per_1k_prompt_gpt-4o": -0.005, @@ -589,7 +585,6 @@ async def test_calculate_cost_usd_negative_price_rejected(self) -> None: @pytest.mark.asyncio async def test_calculate_cost_usd_zero_tokens(self) -> None: """Zero tokens yields zero cost but still configured=True.""" - from aila.platform.llm.cost import calculate_cost_usd registry = _AsyncStubRegistry({ "platform.llm_cost_per_1k_prompt_gpt-4o": 0.005, @@ -607,7 +602,6 @@ async def test_calculate_cost_usd_zero_tokens(self) -> None: @pytest.mark.asyncio async def test_calculate_cost_usd_only_prompt_key_missing(self) -> None: """Returns (0.0, False) when only one key is missing.""" - from aila.platform.llm.cost import calculate_cost_usd registry = _AsyncStubRegistry({ # prompt key missing @@ -634,9 +628,7 @@ class TestPersistCostRecord: @pytest.mark.asyncio async def test_persist_cost_record_swallows_db_exception(self) -> None: """persist_cost_record() swallows DB exceptions and never raises.""" - from unittest.mock import AsyncMock, MagicMock, patch - from aila.platform.llm.cost import persist_cost_record # Patch async_session_scope to raise on commit mock_session = AsyncMock() @@ -663,9 +655,7 @@ async def test_persist_cost_record_swallows_db_exception(self) -> None: @pytest.mark.asyncio async def test_persist_cost_record_none_run_id_defaults(self) -> None: """persist_cost_record() with run_id=None uses '_no_run' sentinel.""" - from unittest.mock import AsyncMock, MagicMock, patch - from aila.platform.llm.cost import persist_cost_record added_records: list = [] @@ -693,9 +683,7 @@ async def test_persist_cost_record_none_run_id_defaults(self) -> None: @pytest.mark.asyncio async def test_persist_cost_record_sets_all_fields(self) -> None: """persist_cost_record() creates record with all fields set correctly.""" - from unittest.mock import AsyncMock, MagicMock, patch - from aila.platform.llm.cost import persist_cost_record added_records: list = [] @@ -739,9 +727,7 @@ class TestEmitMissingPricingNotification: @pytest.mark.asyncio async def test_emit_missing_pricing_notification_creates_record(self) -> None: """Creates a NotificationRecord with user_id='__system__' on first call.""" - from unittest.mock import AsyncMock, MagicMock, patch - from aila.platform.llm.cost import emit_missing_pricing_notification added_records: list = [] @@ -769,10 +755,7 @@ async def test_emit_missing_pricing_notification_creates_record(self) -> None: @pytest.mark.asyncio async def test_emit_missing_pricing_notification_idempotent(self) -> None: """Does NOT create a new record if one already exists (dedup).""" - from unittest.mock import AsyncMock, MagicMock, patch - from aila.platform.llm.cost import emit_missing_pricing_notification - from aila.storage.db_models import NotificationRecord added_records: list = [] @@ -799,9 +782,7 @@ async def test_emit_missing_pricing_notification_idempotent(self) -> None: @pytest.mark.asyncio async def test_emit_missing_pricing_notification_swallows_exception(self) -> None: """Swallows all exceptions and never raises.""" - from unittest.mock import AsyncMock, patch - from aila.platform.llm.cost import emit_missing_pricing_notification mock_cm = AsyncMock() mock_cm.__aenter__ = AsyncMock(side_effect=RuntimeError("DB unavailable")) @@ -815,9 +796,7 @@ async def test_emit_missing_pricing_notification_swallows_exception(self) -> Non @pytest.mark.asyncio async def test_emit_missing_pricing_notification_uses_system_user_id(self) -> None: """user_id is always '__system__' (required non-nullable field).""" - from unittest.mock import AsyncMock, MagicMock, patch - from aila.platform.llm.cost import emit_missing_pricing_notification added_records: list = [] @@ -839,9 +818,7 @@ async def test_emit_missing_pricing_notification_uses_system_user_id(self) -> No @pytest.mark.asyncio async def test_emit_missing_pricing_notification_source_entity_id_format(self) -> None: """source_entity_id uses 'pricing_missing:{model_id}' format for dedup.""" - from unittest.mock import AsyncMock, MagicMock, patch - from aila.platform.llm.cost import emit_missing_pricing_notification added_records: list = [] From 617342231a5f3f5f6c70ae80ae7bda8474f0871c Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 01:20:38 +0300 Subject: [PATCH 010/281] fix(async): offload blocking calls off the event loop (#64) Embedding encode (knowledge tool), pg_dump (backup_database), and the offline-installer subprocess calls now run on a worker thread instead of stalling the loop. EPSSKEVIntelTool.forward becomes async and routes the blocking EPSS/KEV HTTP through run_blocking_io, mirroring NVDIntelTool; the two intel-service callsites now await it (they previously discarded the sync result on the loop). database.py uses asyncio.to_thread directly to avoid a storage-init import cycle. New tests assert the blocking callables execute off the main thread. --- .../forensics/services/offline_installer.py | 12 +-- .../modules/vulnerability/services/intel.py | 4 +- .../vulnerability/tools/intel_epss_kev.py | 7 +- src/aila/platform/tools/knowledge.py | 5 +- src/aila/storage/database.py | 4 +- tests/platform/test_event_loop_offload.py | 86 +++++++++++++++++++ 6 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 tests/platform/test_event_loop_offload.py diff --git a/src/aila/modules/forensics/services/offline_installer.py b/src/aila/modules/forensics/services/offline_installer.py index da2f6b22..920bc2e6 100644 --- a/src/aila/modules/forensics/services/offline_installer.py +++ b/src/aila/modules/forensics/services/offline_installer.py @@ -25,6 +25,7 @@ from aila.config import Settings from aila.platform.exceptions import AILAError +from aila.platform.services.runtime import run_blocking_io __all__ = ["OfflineInstallerService"] @@ -194,8 +195,8 @@ async def prepare_pip_wheels( _log.info("Downloading pip wheels: %s", " ".join(cmd)) try: - result = subprocess.run( - cmd, capture_output=True, text=True, timeout=600, + result = await run_blocking_io( + subprocess.run, cmd, capture_output=True, text=True, timeout=600, ) if result.returncode != 0: _log.warning("pip download failed for %s: %s", package_name, result.stderr[:500]) @@ -205,8 +206,8 @@ async def prepare_pip_wheels( "--no-deps" if "dissect" not in package_name else "--only-binary=:all:", package_name, ] - result = subprocess.run( - cmd_fallback, capture_output=True, text=True, timeout=600, + result = await run_blocking_io( + subprocess.run, cmd_fallback, capture_output=True, text=True, timeout=600, ) if result.returncode != 0: _log.error("pip download fallback failed: %s", result.stderr[:500]) @@ -324,7 +325,8 @@ async def _install_apt_offline( if not existing_debs: _log.info("Attempting to fetch .deb for %s on platform server...", package_name) try: - result = subprocess.run( + result = await run_blocking_io( + subprocess.run, ["apt-get", "download", package_name], capture_output=True, text=True, timeout=120, cwd=str(deb_dir), diff --git a/src/aila/modules/vulnerability/services/intel.py b/src/aila/modules/vulnerability/services/intel.py index 6bc3faf6..3966eec4 100644 --- a/src/aila/modules/vulnerability/services/intel.py +++ b/src/aila/modules/vulnerability/services/intel.py @@ -160,13 +160,13 @@ async def _enrich_cve_ids(self, cve_ids: list[str], force_refresh: bool) -> Inte """ plan = await self._plan_enrichment(cve_ids, force_refresh=force_refresh) try: - epss_scores = self.epss_kev_tool.forward(action="epss_lookup", cve_ids=cve_ids) + epss_scores = await self.epss_kev_tool.forward(action="epss_lookup", cve_ids=cve_ids) except AILAError: _LOGGER.warning("EPSS lookup failed; scoring proceeds without EPSS data", exc_info=True) epss_scores = {} try: - kev_catalog = self.epss_kev_tool.forward(action="kev_catalog") + kev_catalog = await self.epss_kev_tool.forward(action="kev_catalog") except AILAError: _LOGGER.warning("KEV catalog fetch failed; scoring proceeds without KEV data", exc_info=True) kev_catalog = {} diff --git a/src/aila/modules/vulnerability/tools/intel_epss_kev.py b/src/aila/modules/vulnerability/tools/intel_epss_kev.py index b8a1b958..f7780d31 100644 --- a/src/aila/modules/vulnerability/tools/intel_epss_kev.py +++ b/src/aila/modules/vulnerability/tools/intel_epss_kev.py @@ -4,6 +4,7 @@ from aila.modules.vulnerability.config_schema import VulnerabilityConfigSchema from aila.modules.vulnerability.providers import EPSSClient, KEVClient from aila.modules.vulnerability.tools._base import VulnerabilityTool +from aila.platform.services.runtime import run_blocking_io class EPSSKEVIntelTool(VulnerabilityTool): @@ -37,7 +38,7 @@ def _get_kev_client(self) -> KEVClient: self._kev_client = KEVClient(self.settings, self._config) return self._kev_client - def forward( + async def forward( self, action: str, cve_ids: list[str] | None = None, @@ -49,9 +50,9 @@ def forward( ) if not normalized_ids: return {} - return self._get_epss_client().fetch_scores(normalized_ids) + return await run_blocking_io(self._get_epss_client().fetch_scores, normalized_ids) if normalized_action == "kev_catalog": - result = self._get_kev_client().fetch_catalog() + result = await run_blocking_io(self._get_kev_client().fetch_catalog) return result if isinstance(result, dict) else {} raise ValueError(f"Unsupported EPSS/KEV intel action '{action}'.") diff --git a/src/aila/platform/tools/knowledge.py b/src/aila/platform/tools/knowledge.py index fddcdfcb..aa3ddeb1 100644 --- a/src/aila/platform/tools/knowledge.py +++ b/src/aila/platform/tools/knowledge.py @@ -10,6 +10,7 @@ from ...storage.database import async_session_scope from ...storage.db_models import KnowledgeEntryRecord from ..config import PlatformSettings +from ..services.runtime import run_blocking_io from ._common import Tool, normalize_limit, require_text __all__ = [ @@ -74,7 +75,7 @@ async def forward(self, content: str, metadata: dict | None = None) -> dict: model = _get_embedding_model() # Embedding computed outside transaction -- keep write lock short (per research pitfall 2) # pgvector accepts list[float], not raw bytes (Pitfall 3) - embedding_list = model.encode(content).tolist() + embedding_list = (await run_blocking_io(model.encode, content)).tolist() meta_json = json.dumps(meta) async with async_session_scope(self.settings) as session: @@ -166,7 +167,7 @@ async def forward(self, query: str, limit: int | None = None) -> dict: limit = normalize_limit(limit, default=10, maximum=50) model = _get_embedding_model() # pgvector accepts list[float], not raw bytes (Pitfall 3) - query_embedding = model.encode(query).tolist() + query_embedding = (await run_blocking_io(model.encode, query)).tolist() candidate_limit = limit * 10 async with async_session_scope(self.settings) as session: diff --git a/src/aila/storage/database.py b/src/aila/storage/database.py index 9870ce27..b736053d 100644 --- a/src/aila/storage/database.py +++ b/src/aila/storage/database.py @@ -16,6 +16,7 @@ from __future__ import annotations +import asyncio import subprocess import threading from contextlib import asynccontextmanager, contextmanager @@ -227,7 +228,8 @@ async def backup_database( dest.parent.mkdir(parents=True, exist_ok=True) # pg_dump requires a libpq-compatible URL (no +asyncpg driver prefix). pg_url = url.replace("+asyncpg", "") - result = subprocess.run( + result = await asyncio.to_thread( + subprocess.run, ["pg_dump", "--format=custom", f"--file={dest}", pg_url], capture_output=True, text=True, diff --git a/tests/platform/test_event_loop_offload.py b/tests/platform/test_event_loop_offload.py new file mode 100644 index 00000000..55c68135 --- /dev/null +++ b/tests/platform/test_event_loop_offload.py @@ -0,0 +1,86 @@ +"""Event-loop safety tests for #64. + +Blocking calls (sync HTTP, subprocess, embedding encode) invoked from an +``async def`` must run on a platform worker thread via ``run_blocking_io`` +so the event loop is never stalled. These tests assert the observable +consequence: the blocking callable executes on a thread other than the +event loop's own thread. +""" +from __future__ import annotations + +import inspect +import subprocess +import threading + +from aila.modules.vulnerability.tools.intel_epss_kev import EPSSKEVIntelTool +from aila.storage import database as db + + +class _ThreadRecordingClient: + """Records the thread on which its blocking method runs.""" + + def __init__(self) -> None: + self.thread: threading.Thread | None = None + + def fetch_scores(self, cve_ids: list[str]) -> dict[str, float]: + self.thread = threading.current_thread() + return {cid: 0.5 for cid in cve_ids} + + def fetch_catalog(self) -> dict: + self.thread = threading.current_thread() + return {"vulnerabilities": []} + + +def test_epss_kev_forward_is_coroutine_function() -> None: + """forward() is async so the framework can await it (mirrors NVD).""" + tool = EPSSKEVIntelTool() + assert inspect.iscoroutinefunction(tool.forward) + + +async def test_epss_kev_epss_lookup_offloads_to_worker_thread() -> None: + """epss_lookup runs the blocking HTTP client off the event-loop thread.""" + tool = EPSSKEVIntelTool() + stub = _ThreadRecordingClient() + tool._epss_client = stub # type: ignore[assignment] + result = await tool.forward(action="epss_lookup", cve_ids=["cve-2021-1"]) + assert result == {"CVE-2021-1": 0.5} + assert stub.thread is not None + assert stub.thread is not threading.main_thread() + + +async def test_epss_kev_kev_catalog_offloads_and_coerces() -> None: + """kev_catalog offloads and non-dict returns coerce to an empty dict.""" + tool = EPSSKEVIntelTool() + stub = _ThreadRecordingClient() + tool._kev_client = stub # type: ignore[assignment] + result = await tool.forward(action="kev_catalog") + assert result == {"vulnerabilities": []} + assert stub.thread is not threading.main_thread() + + +async def test_backup_database_offloads_pg_dump(monkeypatch, tmp_path) -> None: + """backup_database runs pg_dump on a worker thread, not the event loop.""" + recorded: dict[str, object] = {} + + def fake_run(*args, **kwargs): + recorded["thread"] = threading.current_thread() + recorded["argv"] = args[0] + + class _Result: + returncode = 0 + stderr = "" + + return _Result() + + monkeypatch.setattr(subprocess, "run", fake_run) + + class _Settings: + database_url = "postgresql+asyncpg://user:pw@localhost:5432/aila" + + dest = tmp_path / "backup.dump" + out = await db.backup_database(settings=_Settings(), destination=dest) + + assert out == dest + assert recorded["thread"] is not threading.main_thread() + # pg_dump receives a libpq URL (no +asyncpg driver prefix). + assert "postgresql://user:pw@localhost:5432/aila" in recorded["argv"] From 2a71b077b3dabbf4bee863f04cf0550150721197 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 02:01:15 +0300 Subject: [PATCH 011/281] fix(uow): fail loud on uncommitted writes; commit refresh_target_source (#63) UnitOfWork.__aexit__ now raises UnitOfWorkNotCommittedError when a block exits cleanly with pending inserts/updates/deletes and no commit, converting the silent rollback-on-close data loss into a named, test-visible failure (C4). refresh_target_source in the VR router added a re-keyed audit-mcp handle via session.add but never committed, so the new index id was rolled back on close; it now commits. Full non-e2e suite shows zero new failures from the backstop (verified by revert-compare on the highest-volume and write-heavy suites). --- src/aila/modules/vr/api_router.py | 1 + src/aila/platform/uow.py | 31 +++++++- tests/platform/test_uow_commit_contract.py | 91 ++++++++++++++++++++++ 3 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 tests/platform/test_uow_commit_contract.py diff --git a/src/aila/modules/vr/api_router.py b/src/aila/modules/vr/api_router.py index 5d2c71ef..2f842087 100644 --- a/src/aila/modules/vr/api_router.py +++ b/src/aila/modules/vr/api_router.py @@ -2753,6 +2753,7 @@ async def refresh_target_source( h["audit_mcp_index_id"] = new_index_id refreshed_row.mcp_handles_json = _json.dumps(h) uow.session.add(refreshed_row) + await uow.commit() return DataEnvelope(data={ "target_id": target_id, diff --git a/src/aila/platform/uow.py b/src/aila/platform/uow.py index bfc03407..128ee0da 100644 --- a/src/aila/platform/uow.py +++ b/src/aila/platform/uow.py @@ -23,6 +23,17 @@ from ..api.auth import TeamContext +class UnitOfWorkNotCommittedError(RuntimeError): + """A UoW body performed writes but exited without calling commit(). + + Raised by :meth:`UnitOfWork.__aexit__` on a non-exception exit when the + session still holds pending inserts, updates, or deletes. The underlying + ``async_session_scope`` rolls those writes back on close, so a forgotten + ``await uow.commit()`` silently loses data. This backstop converts that + silent loss into a named, test-visible failure (C4). + """ + + class UnitOfWork: """Wraps one AsyncSession for the duration of a caller-controlled transaction. @@ -62,9 +73,23 @@ async def __aexit__( exc_val: BaseException | None, tb: object, ) -> None: - await self._cm.__aexit__(exc_type, exc_val, tb) - self._session = None - self._cm = None + # Capture pending-write state before the inner scope rolls back and + # expires the session on close. + uncommitted = ( + exc_type is None + and self._session is not None + and bool(self._session.new or self._session.dirty or self._session.deleted) + ) + try: + await self._cm.__aexit__(exc_type, exc_val, tb) + finally: + self._session = None + self._cm = None + if uncommitted: + raise UnitOfWorkNotCommittedError( + "UnitOfWork exited with uncommitted writes; call " + "'await uow.commit()' before the block exits (C4)" + ) async def commit(self) -> None: """Explicitly commit the current transaction.""" diff --git a/tests/platform/test_uow_commit_contract.py b/tests/platform/test_uow_commit_contract.py new file mode 100644 index 00000000..d3f63435 --- /dev/null +++ b/tests/platform/test_uow_commit_contract.py @@ -0,0 +1,91 @@ +"""UoW commit-contract backstop tests for #63 (C4). + +A UnitOfWork block that performs writes but exits without ``await +uow.commit()`` silently loses data, because the underlying +``async_session_scope`` rolls back on close. The ``__aexit__`` backstop +turns that silent loss into a named ``UnitOfWorkNotCommittedError``. + +These tests inject a fake session and context manager to exercise the +exit logic without a live database. +""" +from __future__ import annotations + +import pytest + +from aila.platform.uow import UnitOfWork, UnitOfWorkNotCommittedError + + +class _FakeCM: + def __init__(self) -> None: + self.exit_args: tuple | None = None + + async def __aexit__(self, exc_type, exc_val, tb) -> bool: + self.exit_args = (exc_type, exc_val, tb) + return False + + +class _FakeSession: + def __init__(self, new=(), dirty=(), deleted=()) -> None: + self.new = list(new) + self.dirty = list(dirty) + self.deleted = list(deleted) + + +def _prime(uow: UnitOfWork, session: _FakeSession) -> _FakeCM: + cm = _FakeCM() + uow._cm = cm # type: ignore[assignment] + uow._session = session # type: ignore[assignment] + return cm + + +async def test_dirty_exit_without_commit_raises() -> None: + """Pending inserts at a clean exit raise the named backstop error.""" + uow = UnitOfWork() + cm = _prime(uow, _FakeSession(new=[object()])) + with pytest.raises(UnitOfWorkNotCommittedError): + await uow.__aexit__(None, None, None) + # The inner scope still ran (rolled the write back) and state was reset. + assert cm.exit_args == (None, None, None) + assert uow._session is None + assert uow._cm is None + + +async def test_dirty_updates_and_deletes_also_raise() -> None: + """Pending updates or deletes count as writes, not just inserts.""" + uow = UnitOfWork() + _prime(uow, _FakeSession(dirty=[object()])) + with pytest.raises(UnitOfWorkNotCommittedError): + await uow.__aexit__(None, None, None) + + uow2 = UnitOfWork() + _prime(uow2, _FakeSession(deleted=[object()])) + with pytest.raises(UnitOfWorkNotCommittedError): + await uow2.__aexit__(None, None, None) + + +async def test_clean_exit_no_writes_does_not_raise() -> None: + """A read-only block (empty pending sets) exits without error.""" + uow = UnitOfWork() + cm = _prime(uow, _FakeSession()) + await uow.__aexit__(None, None, None) + assert cm.exit_args == (None, None, None) + + +async def test_committed_exit_does_not_raise() -> None: + """After commit the session pending sets are empty, so no backstop fires.""" + uow = UnitOfWork() + # A committed session has flushed and cleared its pending collections. + _prime(uow, _FakeSession(new=(), dirty=(), deleted=())) + await uow.__aexit__(None, None, None) + + +async def test_exception_exit_propagates_original_not_backstop() -> None: + """On an exceptional exit the backstop stays silent so the real error wins.""" + uow = UnitOfWork() + cm = _prime(uow, _FakeSession(new=[object()])) + # exc_type is set: __aexit__ returns None (does not suppress), and must + # not raise UnitOfWorkNotCommittedError over the original exception. + result = await uow.__aexit__(ValueError, ValueError("boom"), None) + assert result is None + assert cm.exit_args[0] is ValueError + assert uow._session is None From 7f3e20b3709335c1670e46ddb3c10cd1658161bf Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 02:10:12 +0300 Subject: [PATCH 012/281] fix(vulnerability): correct criticality vocabulary and fallback scoring (#55) The severity sort table and the system_summary/report_count/fleet_severity_summary dashboards used the CVSS vocabulary (critical/high/medium/low) while findings store patching-urgency criticality (Immediate/High/Moderate/Planned). Severity sorting was a no-op and every tier except High reported zero. All three now source the vocabulary from CRITICALITY_KEYS. scoring_metadata raised RuntimeError when any finding was scored via fallback (a valid canonical mode); it now counts fallback and aggregates to fallback or mixed, and unknown modes are logged and counted as fallback rather than crashing the run summary. --- .../vulnerability/agents/scoring/agent.py | 36 +++++++---- .../vulnerability/agents/scoring/policy.py | 12 ++-- src/aila/modules/vulnerability/api_router.py | 10 +++- .../vulnerability/contracts/scoring.py | 1 + src/aila/modules/vulnerability/module.py | 19 +++--- .../test_scoring_metadata_fallback.py | 59 ++++++++++++++++++ .../vulnerability/test_severity_order.py | 46 ++++++++++++++ .../vulnerability/test_system_summary_keys.py | 60 +++++++++++++++++++ 8 files changed, 216 insertions(+), 27 deletions(-) create mode 100644 tests/modules/vulnerability/test_scoring_metadata_fallback.py create mode 100644 tests/modules/vulnerability/test_severity_order.py create mode 100644 tests/modules/vulnerability/test_system_summary_keys.py diff --git a/src/aila/modules/vulnerability/agents/scoring/agent.py b/src/aila/modules/vulnerability/agents/scoring/agent.py index df31e56d..69627f19 100644 --- a/src/aila/modules/vulnerability/agents/scoring/agent.py +++ b/src/aila/modules/vulnerability/agents/scoring/agent.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import logging from collections.abc import Callable from typing import TYPE_CHECKING @@ -38,6 +39,8 @@ if TYPE_CHECKING: from aila.platform.tools.knowledge import KnowledgeRetrieveTool, KnowledgeStoreTool +_log = logging.getLogger(__name__) + class RiskScoringAgent(StructuredAgent): """LLM scoring agent that assigns patching urgency to CVE findings. @@ -283,11 +286,9 @@ def scoring_metadata(self, findings: list[PrioritizedFinding]) -> ScoringRunSumm findings: Scored findings from this run. Returns: - ScoringRunSummary with scoring_mode (model/cache/mixed) and per-mode counts. - - Raises: - RuntimeError: If an unexpected scoring mode is encountered, or if findings - is non-empty but yields no countable mode. + ScoringRunSummary with scoring_mode (model/cache/fallback/mixed) and + per-mode counts. Fallback-scored findings are a valid canonical mode, + not an error; unknown modes are logged and counted as fallback. """ counts = ScoringModeCounts() for finding in findings: @@ -296,19 +297,28 @@ def scoring_metadata(self, findings: list[PrioritizedFinding]) -> ScoringRunSumm counts.model += 1 elif mode == "cache": counts.cache += 1 + elif mode == "fallback": + counts.fallback += 1 else: - raise RuntimeError(f"Unexpected scoring mode '{mode}'.") + _log.warning( + "scoring_metadata: unknown scoring mode %r on finding %s", + finding.scoring_mode, + getattr(finding, "cve_id", "?"), + ) + counts.fallback += 1 if not findings: mode = "model" - elif counts.model and counts.cache: - mode = "mixed" - elif counts.model: - mode = "model" - elif counts.cache: - mode = "cache" else: - raise RuntimeError("Scoring metadata cannot be empty when findings exist.") + non_fallback = counts.model + counts.cache + if counts.fallback and non_fallback == 0: + mode = "fallback" + elif (counts.model and counts.cache) or counts.fallback: + mode = "mixed" + elif counts.model: + mode = "model" + else: + mode = "cache" return ScoringRunSummary(scoring_mode=mode, scoring_counts=counts) diff --git a/src/aila/modules/vulnerability/agents/scoring/policy.py b/src/aila/modules/vulnerability/agents/scoring/policy.py index 3d488aff..3146f046 100644 --- a/src/aila/modules/vulnerability/agents/scoring/policy.py +++ b/src/aila/modules/vulnerability/agents/scoring/policy.py @@ -220,14 +220,18 @@ def format_scoring_mode_note(metadata: ScoringRunSummary) -> str: mode = metadata.scoring_mode model_count = metadata.scoring_counts.model cache_count = metadata.scoring_counts.cache - if model_count == 0 and cache_count == 0: + fallback_count = metadata.scoring_counts.fallback + if model_count == 0 and cache_count == 0 and fallback_count == 0: return "Scoring mode: no findings required review." + fallback_suffix = f", {fallback_count} fallback" if fallback_count else "" if mode == "model": - return f"Scoring mode: fresh model review on all findings ({model_count} fresh, {cache_count} cached)." + return f"Scoring mode: fresh model review on all findings ({model_count} fresh, {cache_count} cached{fallback_suffix})." if mode == "cache": - return f"Scoring mode: cached model review reused for all findings ({model_count} fresh, {cache_count} cached)." - return f"Scoring mode: mixed fresh and cached model review ({model_count} fresh, {cache_count} cached)." + return f"Scoring mode: cached model review reused for all findings ({model_count} fresh, {cache_count} cached{fallback_suffix})." + if mode == "fallback": + return f"Scoring mode: all findings scored via fallback ({fallback_count} fallback)." + return f"Scoring mode: mixed fresh and cached model review ({model_count} fresh, {cache_count} cached{fallback_suffix})." def build_rationale( diff --git a/src/aila/modules/vulnerability/api_router.py b/src/aila/modules/vulnerability/api_router.py index 8f69c8e3..34d6a77c 100644 --- a/src/aila/modules/vulnerability/api_router.py +++ b/src/aila/modules/vulnerability/api_router.py @@ -50,6 +50,7 @@ ReportSummary, ReportSummaryResponse, ) +from aila.modules.vulnerability.tools._scoring_constants import CRITICALITY_KEYS from aila.platform.contracts.auth import AuthContext, require_auth, require_role from aila.platform.exceptions import NotFoundError from aila.platform.services.audit import record_audit_event @@ -76,8 +77,11 @@ # Valid export format values for GET /reports/{run_id}?format= _VALID_EXPORT_FORMATS = {"json", "csv", "pdf"} -# Criticality order for severity sorting (ascending = lowest risk first). -_SEVERITY_ORDER = {"CRITICAL": 0, "HIGH": 1, "MEDIUM": 2, "LOW": 3, "UNKNOWN": 4} +# Criticality order for severity sorting (ascending = highest risk first). +# Sourced from the single criticality vocabulary so the sort key matches the +# values actually stored on findings (Immediate/High/Moderate/Planned). +_SEVERITY_ORDER = {key: rank for rank, key in enumerate(CRITICALITY_KEYS)} +_SEVERITY_MISSING_RANK = len(CRITICALITY_KEYS) # API sort_by aliases map to LatestFindingRecord field names. _SORT_ALIAS = { @@ -259,7 +263,7 @@ async def list_findings( if db_sort_field == "criticality": rows.sort( key=lambda r: ( - _SEVERITY_ORDER.get(r.criticality or "UNKNOWN", 99), + _SEVERITY_ORDER.get(r.criticality or "", _SEVERITY_MISSING_RANK), 0 if r.is_kev else 1, # KEV first within severity tier ), reverse=reverse, diff --git a/src/aila/modules/vulnerability/contracts/scoring.py b/src/aila/modules/vulnerability/contracts/scoring.py index c254fd8b..d9f5fea0 100644 --- a/src/aila/modules/vulnerability/contracts/scoring.py +++ b/src/aila/modules/vulnerability/contracts/scoring.py @@ -68,6 +68,7 @@ class ScoringModeCounts(BaseModel): model: int = 0 cache: int = 0 + fallback: int = 0 class ScoringRunSummary(BaseModel): diff --git a/src/aila/modules/vulnerability/module.py b/src/aila/modules/vulnerability/module.py index 6685d6d8..a124395b 100644 --- a/src/aila/modules/vulnerability/module.py +++ b/src/aila/modules/vulnerability/module.py @@ -21,6 +21,7 @@ from aila.modules.vulnerability.services import AdvisoryService, IntelService, InventoryService from aila.modules.vulnerability.tool_catalog import iter_tool_specs from aila.modules.vulnerability.tool_keys import all_tool_keys, tool_key +from aila.modules.vulnerability.tools._scoring_constants import CRITICALITY_KEYS from aila.modules.vulnerability.tools.advisories_alpine import AlpineAdvisoryTool from aila.modules.vulnerability.tools.advisories_arch import ArchAdvisoryTool from aila.modules.vulnerability.tools.advisories_osv import OSVAdvisoryTool @@ -549,10 +550,10 @@ async def system_summary(self, system_id: int, session: Any) -> dict[str, Any]: crit = (row.criticality or "UNKNOWN").lower() counts[crit] += 1 return { - "critical": counts.get("critical", 0), + "immediate": counts.get("immediate", 0), "high": counts.get("high", 0), - "medium": counts.get("medium", 0), - "low": counts.get("low", 0), + "moderate": counts.get("moderate", 0), + "planned": counts.get("planned", 0), } except (OSError, RuntimeError, ValueError): _log.debug("system_summary failed for system_id=%s", system_id, exc_info=True) @@ -651,10 +652,10 @@ async def report_count(self, run_id: str, session: Any) -> dict[str, Any]: counts[crit] += 1 return { "total_findings": len(rows), - "critical": counts.get("critical", 0), + "immediate": counts.get("immediate", 0), "high": counts.get("high", 0), - "medium": counts.get("medium", 0), - "low": counts.get("low", 0), + "moderate": counts.get("moderate", 0), + "planned": counts.get("planned", 0), } except (OSError, RuntimeError, ValueError): _log.debug("report_count failed for run_id=%s", run_id, exc_info=True) @@ -721,7 +722,11 @@ async def fleet_severity_summary( ) -> dict[int, str]: """Return top severity per system_id for platform list/topology overlays.""" findings = await self.latest_findings(session, limit=None) - severity_order = {"critical": 4, "high": 3, "medium": 2, "low": 1} + # Rank by the patching-urgency vocabulary that findings actually store + # (Immediate highest). Sourced from the single criticality vocabulary. + severity_order = { + key.lower(): rank for rank, key in enumerate(reversed(CRITICALITY_KEYS), start=1) + } top: dict[int, str] = {} for finding in findings: sid = finding.get("system_id") diff --git a/tests/modules/vulnerability/test_scoring_metadata_fallback.py b/tests/modules/vulnerability/test_scoring_metadata_fallback.py new file mode 100644 index 00000000..10d29df4 --- /dev/null +++ b/tests/modules/vulnerability/test_scoring_metadata_fallback.py @@ -0,0 +1,59 @@ +"""Scoring-metadata fallback-mode tests for #55. + +`normalize_scoring_mode` recognizes 'fallback' as a canonical mode, but +`scoring_metadata` used to raise RuntimeError on anything other than +model/cache, crashing the scoring summary whenever a finding was scored +via fallback. These tests pin the corrected aggregation. +""" +from __future__ import annotations + +from types import SimpleNamespace + +from aila.modules.vulnerability.agents.scoring.agent import RiskScoringAgent + + +def _finding(mode: str, cve_id: str = "CVE-2021-0001") -> SimpleNamespace: + return SimpleNamespace( + scoring_mode=mode, + scoring_evidence=SimpleNamespace(scoring_mode=mode), + cve_id=cve_id, + ) + + +def _summary(findings): + # scoring_metadata does not touch self; call it unbound with a stub. + return RiskScoringAgent.scoring_metadata(object(), findings) + + +def test_fallback_only_does_not_raise() -> None: + """A run scored entirely via fallback yields mode='fallback', not a crash.""" + summary = _summary([_finding("fallback")]) + assert summary.scoring_mode == "fallback" + assert summary.scoring_counts.fallback == 1 + assert summary.scoring_counts.model == 0 + + +def test_fallback_mixed_with_model() -> None: + """Model plus fallback aggregates to 'mixed' with the fallback counted.""" + summary = _summary([_finding("model"), _finding("model"), _finding("fallback")]) + assert summary.scoring_mode == "mixed" + assert summary.scoring_counts.model == 2 + assert summary.scoring_counts.fallback == 1 + + +def test_unknown_mode_counts_as_fallback() -> None: + """An unrecognized mode is counted as fallback rather than raising.""" + summary = _summary([_finding("wobble")]) + assert summary.scoring_counts.fallback == 1 + assert summary.scoring_mode == "fallback" + + +def test_all_model_stays_model() -> None: + summary = _summary([_finding("model"), _finding("model")]) + assert summary.scoring_mode == "model" + assert summary.scoring_counts.fallback == 0 + + +def test_empty_findings_default_model() -> None: + summary = _summary([]) + assert summary.scoring_mode == "model" diff --git a/tests/modules/vulnerability/test_severity_order.py b/tests/modules/vulnerability/test_severity_order.py new file mode 100644 index 00000000..992c4a30 --- /dev/null +++ b/tests/modules/vulnerability/test_severity_order.py @@ -0,0 +1,46 @@ +"""Severity-sort vocabulary tests for #55. + +The findings list is sorted by criticality, but the sort table used the +CVSS-style vocabulary (CRITICAL/HIGH/MEDIUM/LOW) while findings store the +patching-urgency vocabulary (Immediate/High/Moderate/Planned). Every row +therefore fell to the missing rank and severity sorting was a no-op. +""" +from __future__ import annotations + +from types import SimpleNamespace + +from aila.modules.vulnerability.api_router import _SEVERITY_MISSING_RANK, _SEVERITY_ORDER + + +def test_severity_order_uses_real_vocabulary() -> None: + assert _SEVERITY_ORDER == {"Immediate": 0, "High": 1, "Moderate": 2, "Planned": 3} + assert _SEVERITY_MISSING_RANK == 4 + + +def test_sort_ranks_immediate_first_planned_last() -> None: + rows = [ + SimpleNamespace(criticality="Planned", is_kev=False), + SimpleNamespace(criticality="Immediate", is_kev=False), + SimpleNamespace(criticality="Moderate", is_kev=False), + SimpleNamespace(criticality="High", is_kev=False), + ] + rows.sort(key=lambda r: _SEVERITY_ORDER.get(r.criticality or "", _SEVERITY_MISSING_RANK)) + assert [r.criticality for r in rows] == ["Immediate", "High", "Moderate", "Planned"] + + +def test_kev_breaks_ties_within_tier() -> None: + rows = [ + SimpleNamespace(criticality="High", is_kev=False), + SimpleNamespace(criticality="High", is_kev=True), + ] + rows.sort( + key=lambda r: ( + _SEVERITY_ORDER.get(r.criticality or "", _SEVERITY_MISSING_RANK), + 0 if r.is_kev else 1, + ) + ) + assert rows[0].is_kev is True + + +def test_unknown_criticality_sorts_last() -> None: + assert _SEVERITY_ORDER.get("Bogus", _SEVERITY_MISSING_RANK) == _SEVERITY_MISSING_RANK diff --git a/tests/modules/vulnerability/test_system_summary_keys.py b/tests/modules/vulnerability/test_system_summary_keys.py new file mode 100644 index 00000000..e552e1e9 --- /dev/null +++ b/tests/modules/vulnerability/test_system_summary_keys.py @@ -0,0 +1,60 @@ +"""system_summary / report_count vocabulary tests for #55. + +The dashboard summaries emitted CVSS-style keys (critical/medium/low) +while findings store patching-urgency criticality (immediate/moderate/ +planned). Only 'high' matched by accident; every other tier reported 0. +""" +from __future__ import annotations + +from types import SimpleNamespace + +from aila.modules.vulnerability.module import VulnerabilityModule + + +class _Result: + def __init__(self, rows): + self._rows = rows + + def all(self): + return self._rows + + +class _Session: + def __init__(self, rows): + self._rows = rows + + async def exec(self, _stmt): + return _Result(self._rows) + + +def _rows(*criticalities): + return [SimpleNamespace(criticality=c) for c in criticalities] + + +async def test_system_summary_returns_urgency_vocabulary() -> None: + session = _Session(_rows("Immediate", "Immediate", "High", "Moderate", "Planned")) + # system_summary does not use self; call it unbound with a stub receiver. + result = await VulnerabilityModule.system_summary(object(), 1, session) + assert result == {"immediate": 2, "high": 1, "moderate": 1, "planned": 1} + + +async def test_system_summary_empty_is_empty_dict() -> None: + session = _Session([]) + result = await VulnerabilityModule.system_summary(object(), 1, session) + assert result == {} + + +async def test_fleet_severity_summary_ranks_immediate_above_high() -> None: + findings = [ + {"system_id": 1, "criticality": "High"}, + {"system_id": 1, "criticality": "Immediate"}, + {"system_id": 2, "criticality": "Planned"}, + {"system_id": 2, "criticality": "Moderate"}, + ] + + async def _latest(_session, limit=None): + return findings + + fake_self = SimpleNamespace(latest_findings=_latest) + result = await VulnerabilityModule.fleet_severity_summary(fake_self, [1, 2], None) + assert result == {1: "immediate", 2: "moderate"} From 7160533493a82c5067c5e44be2f54d7f6a4f6534 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 02:15:17 +0300 Subject: [PATCH 013/281] fix(vulnerability): gate GHSA matches by version, honor cve TTL, fix NVD limiter (#55) GHSA advisories now emit a match only when the installed version falls inside the advisory vulnerable_version_range (falling back to first_patched_version when the range is unparseable), instead of flagging every package that shares a name. The enrichment planner honors cve_cache_ttl_hours via _cve_cache_is_fresh instead of a keep-forever check, so stale intel is refreshed. The NVD rate limiter holds the global lock across the sleep and stamps the post-sleep proceed time, so concurrent threads serialize instead of stamping ~now and firing together. --- .../vulnerability/adapters/_ghsa_range.py | 59 +++++++++++++++++++ .../modules/vulnerability/adapters/ghsa.py | 25 +++++++- .../modules/vulnerability/providers/nvd.py | 16 +++-- .../modules/vulnerability/services/intel.py | 25 +++++++- .../vulnerability/test_cve_cache_ttl.py | 46 +++++++++++++++ .../modules/vulnerability/test_ghsa_range.py | 47 +++++++++++++++ .../vulnerability/test_nvd_rate_limiter.py | 35 +++++++++++ 7 files changed, 242 insertions(+), 11 deletions(-) create mode 100644 src/aila/modules/vulnerability/adapters/_ghsa_range.py create mode 100644 tests/modules/vulnerability/test_cve_cache_ttl.py create mode 100644 tests/modules/vulnerability/test_ghsa_range.py create mode 100644 tests/modules/vulnerability/test_nvd_rate_limiter.py diff --git a/src/aila/modules/vulnerability/adapters/_ghsa_range.py b/src/aila/modules/vulnerability/adapters/_ghsa_range.py new file mode 100644 index 00000000..b8fadfeb --- /dev/null +++ b/src/aila/modules/vulnerability/adapters/_ghsa_range.py @@ -0,0 +1,59 @@ +"""GHSA vulnerable-version-range parsing and gating (#55). + +GitHub advisories express affected versions as a range expression such as +``>= 4.0.0, < 4.17.21``. A match should be emitted only when the installed +version falls inside that range. When the range cannot be parsed the caller +falls back to the ``first_patched_version`` comparison. +""" +from __future__ import annotations + +import re +from dataclasses import dataclass + +from aila.modules.vulnerability.versioning import compare_versions + +__all__ = ["VersionBound", "is_installed_in_range", "parse_ghsa_range"] + +_BOUND_RE = re.compile(r"(<=|<|>=|>|=)\s*([^\s,]+)") + + +@dataclass(slots=True, frozen=True) +class VersionBound: + op: str + version: str + + +def parse_ghsa_range(range_expr: str) -> list[VersionBound]: + """Parse a GHSA range expression into comparison bounds. + + Returns an empty list for empty or blank input. + """ + if not range_expr or not range_expr.strip(): + return [] + return [VersionBound(op=m.group(1), version=m.group(2)) for m in _BOUND_RE.finditer(range_expr)] + + +def is_installed_in_range(installed_version: str, range_expr: str, distribution: str) -> bool | None: + """Return whether an installed version satisfies a GHSA range. + + True -- installed version is inside the vulnerable range. + False -- installed version is outside the range (not vulnerable). + None -- the range could not be parsed; the caller should fall back to + the first_patched_version comparison. + """ + bounds = parse_ghsa_range(range_expr) + if not bounds: + return None + for bound in bounds: + cmp = compare_versions(installed_version, bound.version, distribution=distribution) + if bound.op == "<" and not cmp < 0: + return False + if bound.op == "<=" and not cmp <= 0: + return False + if bound.op == ">" and not cmp > 0: + return False + if bound.op == ">=" and not cmp >= 0: + return False + if bound.op == "=" and cmp != 0: + return False + return True diff --git a/src/aila/modules/vulnerability/adapters/ghsa.py b/src/aila/modules/vulnerability/adapters/ghsa.py index 0fede240..ebdea54d 100644 --- a/src/aila/modules/vulnerability/adapters/ghsa.py +++ b/src/aila/modules/vulnerability/adapters/ghsa.py @@ -7,7 +7,9 @@ import httpx import sqlalchemy.exc +from aila.modules.vulnerability.adapters._ghsa_range import is_installed_in_range from aila.modules.vulnerability.contracts import InventoryArtifact, VulnerabilityMatch +from aila.modules.vulnerability.versioning import compare_versions from aila.platform.exceptions import AILAError from aila.platform.rate_limiter import TokenBucketRateLimiter @@ -220,8 +222,10 @@ async def async_lookup_for_packages( cve_id: str | None = adv.get("cve_id") severity: str = (adv.get("severity") or "").lower() - # Find first_patched_version for this package in this ecosystem + # Find first_patched_version and the vulnerable range for + # this package in this ecosystem. first_patched: str | None = None + vulnerable_range: str = "" for vuln in adv.get("vulnerabilities", []): pkg_info = vuln.get("package", {}) if ( @@ -229,9 +233,12 @@ async def async_lookup_for_packages( and pkg_info.get("name") == package_name ): first_patched = vuln.get("first_patched_version") + vulnerable_range = vuln.get("vulnerable_version_range") or "" break - # Create a VulnerabilityMatch for each inventory that has this package + # Emit a match only for installed versions inside the vulnerable + # range. When the range is unparseable, fall back to the + # first_patched_version comparison so a patched install is skipped. for inv_ecosystem, inv_list in ecosystem_inventories.items(): if inv_ecosystem != ecosystem: continue @@ -239,6 +246,20 @@ async def async_lookup_for_packages( for pkg in inventory.packages: if pkg.name != package_name: continue + in_range = is_installed_in_range( + pkg.version, vulnerable_range, distribution=ecosystem + ) + if in_range is False: + continue + if ( + in_range is None + and first_patched + and compare_versions( + pkg.version, first_patched, distribution=ecosystem + ) + >= 0 + ): + continue matches.append( VulnerabilityMatch( system_id=inventory.system_id, diff --git a/src/aila/modules/vulnerability/providers/nvd.py b/src/aila/modules/vulnerability/providers/nvd.py index 40fbf92a..12480d04 100644 --- a/src/aila/modules/vulnerability/providers/nvd.py +++ b/src/aila/modules/vulnerability/providers/nvd.py @@ -134,14 +134,18 @@ def _wait_for_request_slot(self) -> None: share the same rate-limit state even when running in parallel threads. """ global _nvd_last_request_at + # Hold the lock across the sleep so concurrent threads serialize instead + # of all stamping ~now, releasing, and firing together. with _NVD_GLOBAL_LOCK: last = _nvd_last_request_at - _nvd_last_request_at = time.monotonic() - if last is not None: - elapsed = time.monotonic() - last - remaining = self._effective_min_interval_seconds() - elapsed - if remaining > 0: - time.sleep(remaining) + now = time.monotonic() + if last is not None: + elapsed = now - last + remaining = self._effective_min_interval_seconds() - elapsed + if remaining > 0: + time.sleep(remaining) + now = time.monotonic() + _nvd_last_request_at = now def _effective_min_interval_seconds(self) -> float: """Return the configured minimum request interval based on API key presence. diff --git a/src/aila/modules/vulnerability/services/intel.py b/src/aila/modules/vulnerability/services/intel.py index 3966eec4..ce5197af 100644 --- a/src/aila/modules/vulnerability/services/intel.py +++ b/src/aila/modules/vulnerability/services/intel.py @@ -1,6 +1,6 @@ from __future__ import annotations -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from aila.logging import get_logger from aila.modules.vulnerability.config_schema import VulnerabilityConfigSchema @@ -20,7 +20,6 @@ from aila.modules.vulnerability.tools.intel_epss_kev import EPSSKEVIntelTool from aila.modules.vulnerability.tools.intel_nvd import NVDIntelTool from aila.platform.exceptions import AILAError, RateLimitError -from aila.storage.operations import is_cache_fresh _LOGGER = get_logger(__name__) _NVD_DETAIL_URL = "https://nvd.nist.gov/vuln/detail" @@ -230,6 +229,26 @@ async def _enrich_cve_ids(self, cve_ids: list[str], force_refresh: bool) -> Inte ), ) + def _cve_cache_is_fresh(self, payload: dict | None) -> bool: + """Return True when a cached CVE payload is within the configured TTL. + + The previous keep-forever policy ignored cve_cache_ttl_hours, so cache + entries never expired and a scan reused arbitrarily stale intel. + """ + if not payload: + return False + stamp = payload.get("last_synced_at") + if not stamp: + return False + try: + ts = datetime.fromisoformat(stamp) + except (ValueError, TypeError): + return False + if ts.tzinfo is None: + ts = ts.replace(tzinfo=UTC) + age = datetime.now(UTC) - ts + return age <= timedelta(hours=self.config.cve_cache_ttl_hours) + async def _plan_enrichment(self, cve_ids: list[str], force_refresh: bool) -> EnrichmentPlan: """Fetch all CVE cache entries in one batch query, then classify each ID. @@ -253,7 +272,7 @@ async def _plan_enrichment(self, cve_ids: list[str], force_refresh: bool) -> Enr refresh_ids.append(cve_id) continue - if is_cache_fresh(cached, force_refresh=False): + if self._cve_cache_is_fresh(cached): fresh_cache_ids.append(cve_id) else: missing_ids.append(cve_id) diff --git a/tests/modules/vulnerability/test_cve_cache_ttl.py b/tests/modules/vulnerability/test_cve_cache_ttl.py new file mode 100644 index 00000000..787f6bfc --- /dev/null +++ b/tests/modules/vulnerability/test_cve_cache_ttl.py @@ -0,0 +1,46 @@ +"""CVE cache TTL tests for #55. + +The enrichment planner used a keep-forever freshness check that ignored +cve_cache_ttl_hours, so cached CVE intel never expired. _cve_cache_is_fresh +now honors the configured TTL. +""" +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from types import SimpleNamespace + +from aila.modules.vulnerability.config_schema import VulnerabilityConfigSchema +from aila.modules.vulnerability.services.intel import IntelService + + +def _svc(ttl_hours: int = 24) -> SimpleNamespace: + return SimpleNamespace(config=VulnerabilityConfigSchema(cve_cache_ttl_hours=ttl_hours)) + + +def _stamp(hours_ago: float) -> str: + return (datetime.now(UTC) - timedelta(hours=hours_ago)).isoformat() + + +def test_fresh_within_ttl() -> None: + assert IntelService._cve_cache_is_fresh(_svc(24), {"last_synced_at": _stamp(1)}) is True + + +def test_stale_past_ttl() -> None: + assert IntelService._cve_cache_is_fresh(_svc(24), {"last_synced_at": _stamp(25)}) is False + + +def test_missing_timestamp_is_stale() -> None: + assert IntelService._cve_cache_is_fresh(_svc(24), {}) is False + + +def test_none_payload_is_stale() -> None: + assert IntelService._cve_cache_is_fresh(_svc(24), None) is False + + +def test_malformed_timestamp_is_stale() -> None: + assert IntelService._cve_cache_is_fresh(_svc(24), {"last_synced_at": "not-a-date"}) is False + + +def test_naive_timestamp_treated_as_utc() -> None: + naive = (datetime.now(UTC) - timedelta(hours=1)).replace(tzinfo=None).isoformat() + assert IntelService._cve_cache_is_fresh(_svc(24), {"last_synced_at": naive}) is True diff --git a/tests/modules/vulnerability/test_ghsa_range.py b/tests/modules/vulnerability/test_ghsa_range.py new file mode 100644 index 00000000..8b969aac --- /dev/null +++ b/tests/modules/vulnerability/test_ghsa_range.py @@ -0,0 +1,47 @@ +"""GHSA version-range gating tests for #55. + +Before the fix the GHSA adapter emitted a match for every inventory package +regardless of installed version. These tests pin the range parser and the +in-range decision that now gates match emission. +""" +from __future__ import annotations + +from aila.modules.vulnerability.adapters._ghsa_range import ( + VersionBound, + is_installed_in_range, + parse_ghsa_range, +) + + +def test_parse_single_bound() -> None: + assert parse_ghsa_range("< 4.17.21") == [VersionBound("<", "4.17.21")] + + +def test_parse_compound_bounds() -> None: + bounds = parse_ghsa_range(">= 4.0.0, < 4.17.21") + assert bounds == [VersionBound(">=", "4.0.0"), VersionBound("<", "4.17.21")] + + +def test_parse_empty_is_empty() -> None: + assert parse_ghsa_range("") == [] + assert parse_ghsa_range(" ") == [] + + +def test_in_range_below_ceiling_is_vulnerable() -> None: + assert is_installed_in_range("4.17.20", "< 4.17.21", "pip") is True + + +def test_at_ceiling_is_not_vulnerable() -> None: + assert is_installed_in_range("4.17.21", "< 4.17.21", "pip") is False + + +def test_below_floor_of_compound_is_not_vulnerable() -> None: + assert is_installed_in_range("3.9.0", ">= 4.0.0, < 4.17.21", "pip") is False + + +def test_within_compound_is_vulnerable() -> None: + assert is_installed_in_range("4.17.19", ">= 4.0.0, < 4.17.21", "pip") is True + + +def test_unparseable_range_returns_none() -> None: + assert is_installed_in_range("1.2.3", "garbage", "pip") is None diff --git a/tests/modules/vulnerability/test_nvd_rate_limiter.py b/tests/modules/vulnerability/test_nvd_rate_limiter.py new file mode 100644 index 00000000..7da953b0 --- /dev/null +++ b/tests/modules/vulnerability/test_nvd_rate_limiter.py @@ -0,0 +1,35 @@ +"""NVD rate-limiter lock-scope test for #55. + +The limiter stamped _nvd_last_request_at before releasing the lock and then +slept outside it, so concurrent threads all recorded ~now, released, and +fired together. The fix holds the lock across the sleep and stamps the +post-sleep proceed time. Deterministic proof: with a controllable clock the +recorded timestamp after a spaced call reflects the time the request actually +proceeds, not the time it entered. +""" +from __future__ import annotations + +from aila.config import get_settings +from aila.modules.vulnerability.config_schema import VulnerabilityConfigSchema +from aila.modules.vulnerability.providers import nvd as nvd_mod +from aila.modules.vulnerability.providers.nvd import NVDClient + + +def test_stamp_reflects_post_sleep_proceed_time(monkeypatch) -> None: + monkeypatch.delenv("NVD_API_KEY", raising=False) + clock = {"t": 1000.0} + monkeypatch.setattr(nvd_mod.time, "monotonic", lambda: clock["t"]) + monkeypatch.setattr(nvd_mod.time, "sleep", lambda d: clock.__setitem__("t", clock["t"] + d)) + monkeypatch.setattr(nvd_mod, "_nvd_last_request_at", None) + + client = NVDClient(get_settings(), VulnerabilityConfigSchema(), api_key=None) + interval = client._effective_min_interval_seconds() + assert interval > 0 + + client._wait_for_request_slot() + assert nvd_mod._nvd_last_request_at == 1000.0 + + # Second immediate call must sleep one interval and stamp the post-sleep + # time; the pre-fix code stamped 1000.0 (entry time) instead. + client._wait_for_request_slot() + assert nvd_mod._nvd_last_request_at == 1000.0 + interval From cb9222d7b07dbd9564feade3a50500990da23099 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 02:19:27 +0300 Subject: [PATCH 014/281] feat(eval): add C7 eval metric functions (#32) Pure, deterministic metric layer for the eval harness: expected calibration error (10-bucket), reliability-diagram calibration curve, per-outcome-kind precision/recall (zero-support reported as None, not 0.0), case-count-weighted faithfulness, byte-for-byte determinism, and the EvalReport.beats promotion gate. The gate requires strictly lower ECE plus at-least-equal faithfulness with no per-kind precision/recall regression beyond tolerance, so a recall-only win never beats the baseline (RFC-08 amendment 2). No database, clock, or model dependency; the record-replay harness and storage land with the #62 backbone. --- src/aila/platform/eval/__init__.py | 28 ++++ src/aila/platform/eval/metrics.py | 243 ++++++++++++++++++++++++++++ tests/platform/eval/test_metrics.py | 119 ++++++++++++++ 3 files changed, 390 insertions(+) create mode 100644 src/aila/platform/eval/__init__.py create mode 100644 src/aila/platform/eval/metrics.py create mode 100644 tests/platform/eval/test_metrics.py diff --git a/src/aila/platform/eval/__init__.py b/src/aila/platform/eval/__init__.py new file mode 100644 index 00000000..26b8070a --- /dev/null +++ b/src/aila/platform/eval/__init__.py @@ -0,0 +1,28 @@ +"""Platform evaluation harness -- metrics and (later) record-replay (#32). + +Only the pure metric layer is present today; the record-replay harness and +its storage records land with the #62 test backbone and #32 harness work. +""" +from __future__ import annotations + +from .metrics import ( + CalibrationBucket, + CaseOutcome, + EvalReport, + calibration_curve, + determinism_score, + ece, + faithfulness_score, + precision_recall_per_kind, +) + +__all__ = [ + "CalibrationBucket", + "CaseOutcome", + "EvalReport", + "calibration_curve", + "determinism_score", + "ece", + "faithfulness_score", + "precision_recall_per_kind", +] diff --git a/src/aila/platform/eval/metrics.py b/src/aila/platform/eval/metrics.py new file mode 100644 index 00000000..12da67cb --- /dev/null +++ b/src/aila/platform/eval/metrics.py @@ -0,0 +1,243 @@ +"""Evaluation metrics for the C7 eval harness (#32 / design metrics). + +Pure, dependency-free scoring functions consumed by the eval harness: + +- ``ece`` -- expected calibration error (bucketed confidence vs outcome). +- ``calibration_curve`` -- the per-bucket reliability diagram behind ECE. +- ``precision_recall_per_kind`` -- precision and recall per outcome_kind, + with zero-support kinds reported as ``None`` (not ``0.0``). +- ``faithfulness_score`` -- case-count-weighted blend of per-kind precision + and recall. +- ``determinism_score`` -- fraction of replayed turns that match + byte-for-byte across two replays. +- ``EvalReport`` + ``beats`` -- the promotion gate. A candidate beats the + baseline only when its ECE is strictly lower, its faithfulness is at + least equal, and no per-kind precision or recall drops beyond the + regression tolerance. A recall-only win never beats the baseline + (RFC-08 amendment 2). + +Every function is deterministic and side-effect free so the harness and +the CI benchmark can call them without a database, clock, or model. +""" +from __future__ import annotations + +from collections import Counter +from collections.abc import Sequence +from dataclasses import dataclass + +__all__ = [ + "CaseOutcome", + "CalibrationBucket", + "EvalReport", + "calibration_curve", + "determinism_score", + "ece", + "faithfulness_score", + "precision_recall_per_kind", +] + +_ACCEPT = "accept" + + +@dataclass(frozen=True, slots=True) +class CaseOutcome: + """One scored benchmark case. + + ``predicted_verdict`` and ``verified_verdict`` are each ``"accept"`` or + ``"reject"``; ``confidence`` is the candidate's stated probability in + ``[0, 1]`` that its verdict is correct. + """ + + outcome_kind: str + predicted_verdict: str + verified_verdict: str + confidence: float = 0.0 + + +@dataclass(frozen=True, slots=True) +class CalibrationBucket: + lo: float + hi: float + count: int + mean_confidence: float + accuracy: float + + +def _bucket_index(confidence: float, n_buckets: int) -> int: + idx = int(confidence * n_buckets) + if idx < 0: + return 0 + if idx >= n_buckets: + return n_buckets - 1 + return idx + + +def calibration_curve( + confidences: Sequence[float], + correct: Sequence[bool], + n_buckets: int = 10, +) -> list[CalibrationBucket]: + """Return the reliability diagram: one bucket per confidence band. + + Empty bands are omitted. ``correct[i]`` is whether prediction ``i`` was + right (predicted verdict matched the verified verdict). + """ + if n_buckets <= 0: + raise ValueError("n_buckets must be positive") + if len(confidences) != len(correct): + raise ValueError("confidences and correct must be the same length") + + bins: list[list[tuple[float, bool]]] = [[] for _ in range(n_buckets)] + for conf, ok in zip(confidences, correct, strict=True): + bins[_bucket_index(conf, n_buckets)].append((conf, ok)) + + curve: list[CalibrationBucket] = [] + for i, members in enumerate(bins): + if not members: + continue + count = len(members) + mean_conf = sum(c for c, _ in members) / count + accuracy = sum(1 for _, ok in members if ok) / count + curve.append( + CalibrationBucket( + lo=i / n_buckets, + hi=(i + 1) / n_buckets, + count=count, + mean_confidence=mean_conf, + accuracy=accuracy, + ) + ) + return curve + + +def ece( + confidences: Sequence[float], + correct: Sequence[bool], + n_buckets: int = 10, +) -> float: + """Expected calibration error. + + ``ECE = sum_i (n_i / N) * |mean_confidence_i - accuracy_i|`` over the + non-empty confidence buckets. Overconfident-and-wrong buckets drive it + up. Returns ``0.0`` for an empty input. + """ + total = len(confidences) + if total == 0: + return 0.0 + error = 0.0 + for bucket in calibration_curve(confidences, correct, n_buckets): + error += (bucket.count / total) * abs(bucket.mean_confidence - bucket.accuracy) + return error + + +def precision_recall_per_kind( + cases: Sequence[CaseOutcome], +) -> tuple[dict[str, float | None], dict[str, float | None]]: + """Precision and recall per outcome_kind, treating ``accept`` as positive. + + A kind with no predicted-accept cases has undefined precision (``None``); + a kind with no verified-accept cases has undefined recall (``None``). + """ + tp: Counter[str] = Counter() + predicted_pos: Counter[str] = Counter() + actual_pos: Counter[str] = Counter() + kinds: set[str] = set() + + for case in cases: + kinds.add(case.outcome_kind) + pred_accept = case.predicted_verdict == _ACCEPT + actual_accept = case.verified_verdict == _ACCEPT + if pred_accept: + predicted_pos[case.outcome_kind] += 1 + if actual_accept: + actual_pos[case.outcome_kind] += 1 + if pred_accept and actual_accept: + tp[case.outcome_kind] += 1 + + precision: dict[str, float | None] = {} + recall: dict[str, float | None] = {} + for kind in kinds: + precision[kind] = (tp[kind] / predicted_pos[kind]) if predicted_pos[kind] else None + recall[kind] = (tp[kind] / actual_pos[kind]) if actual_pos[kind] else None + return precision, recall + + +def faithfulness_score(cases: Sequence[CaseOutcome], alpha: float = 0.5) -> float: + """Case-count-weighted blend of per-kind precision and recall. + + ``faithfulness = sum_kind w_kind * (alpha * precision + (1 - alpha) * recall)`` + where ``w_kind`` is the kind's share of all cases. Undefined per-kind + precision or recall (no support) contributes ``0.0`` for that term. + """ + total = len(cases) + if total == 0: + return 0.0 + precision, recall = precision_recall_per_kind(cases) + weights = Counter(case.outcome_kind for case in cases) + score = 0.0 + for kind, weight in weights.items(): + p = precision[kind] or 0.0 + r = recall[kind] or 0.0 + score += (weight / total) * (alpha * p + (1.0 - alpha) * r) + return score + + +def determinism_score( + replay_a: Sequence[tuple[int, str]], + replay_b: Sequence[tuple[int, str]], +) -> float: + """Fraction of replayed turns matching byte-for-byte across two replays. + + Each replay is a sequence of ``(turn_number, response_json)`` pairs. A + turn present in only one replay counts as a mismatch. Two empty replays + score ``1.0`` (nothing diverged). + """ + a = dict(replay_a) + b = dict(replay_b) + turns = set(a) | set(b) + if not turns: + return 1.0 + matches = sum(1 for t in turns if t in a and t in b and a[t] == b[t]) + return matches / len(turns) + + +@dataclass(frozen=True, slots=True) +class EvalReport: + """Aggregate metrics for one bundle scored against a benchmark.""" + + ece: float + precision_by_kind: dict[str, float | None] + recall_by_kind: dict[str, float | None] + determinism_score: float + faithfulness_score: float + + def beats(self, baseline: EvalReport, regression_tol: float = 0.02) -> bool: + """Strict-beat gate against a baseline report. + + Beats only when (a) ECE is strictly lower, (b) faithfulness is at + least equal, and (c) no per-kind precision or recall drops by more + than ``regression_tol``. A recall-only improvement fails (a) and so + never beats the baseline (RFC-08 amendment 2). + """ + if not self.ece < baseline.ece: + return False + if self.faithfulness_score < baseline.faithfulness_score: + return False + if _regressed(self.precision_by_kind, baseline.precision_by_kind, regression_tol): + return False + if _regressed(self.recall_by_kind, baseline.recall_by_kind, regression_tol): + return False + return True + + +def _regressed( + candidate: dict[str, float | None], + baseline: dict[str, float | None], + tol: float, +) -> bool: + """True when any kind's candidate value drops below baseline minus tol.""" + for kind, base in baseline.items(): + cand = candidate.get(kind) + if base is not None and cand is not None and cand < base - tol: + return True + return False diff --git a/tests/platform/eval/test_metrics.py b/tests/platform/eval/test_metrics.py new file mode 100644 index 00000000..a1f11795 --- /dev/null +++ b/tests/platform/eval/test_metrics.py @@ -0,0 +1,119 @@ +"""Tests for the C7 eval metrics (#32 / design metrics).""" +from __future__ import annotations + +import pytest + +from aila.platform.eval.metrics import ( + CaseOutcome, + EvalReport, + calibration_curve, + determinism_score, + ece, + faithfulness_score, + precision_recall_per_kind, +) + + +def test_ece_matches_hand_computed_value() -> None: + # bucket0: conf .05 acc 1.0 -> err .95 (w .25) + # bucket1: conf .15 acc 0.0 -> err .15 (w .25) + # bucket9: conf .95 acc 0.5 -> err .45 (w .50) + # ECE = .25*.95 + .25*.15 + .50*.45 = 0.5 + confidences = [0.05, 0.15, 0.95, 0.95] + correct = [True, False, True, False] + assert ece(confidences, correct) == pytest.approx(0.5, abs=1e-9) + + +def test_ece_perfect_calibration_is_zero() -> None: + assert ece([1.0, 1.0, 0.0, 0.0], [True, True, False, False]) == pytest.approx(0.0, abs=1e-9) + + +def test_ece_empty_is_zero() -> None: + assert ece([], []) == 0.0 + + +def test_ece_length_mismatch_raises() -> None: + with pytest.raises(ValueError): + ece([0.5], [True, False]) + + +def test_calibration_curve_omits_empty_buckets() -> None: + curve = calibration_curve([0.05, 0.95, 0.95], [True, True, False]) + assert len(curve) == 2 + top = curve[-1] + assert top.count == 2 + assert top.mean_confidence == pytest.approx(0.95) + assert top.accuracy == pytest.approx(0.5) + + +def _cases() -> list[CaseOutcome]: + return [ + CaseOutcome("sqli", "accept", "accept"), + CaseOutcome("sqli", "accept", "reject"), + CaseOutcome("sqli", "reject", "accept"), + CaseOutcome("xss", "reject", "reject"), + ] + + +def test_precision_recall_per_kind_basic() -> None: + precision, recall = precision_recall_per_kind(_cases()) + assert precision["sqli"] == pytest.approx(0.5) + assert recall["sqli"] == pytest.approx(0.5) + + +def test_precision_recall_zero_support_is_none() -> None: + precision, recall = precision_recall_per_kind(_cases()) + # xss has no predicted-accept and no verified-accept -> both undefined. + assert precision["xss"] is None + assert recall["xss"] is None + + +def test_faithfulness_weighted_blend() -> None: + # sqli weight 3/4, P=R=0.5; xss weight 1/4 contributes 0. + # faithfulness = 0.75 * (0.5*0.5 + 0.5*0.5) = 0.375 + assert faithfulness_score(_cases()) == pytest.approx(0.375, abs=1e-9) + + +def test_determinism_partial_and_total() -> None: + assert determinism_score([(1, "x"), (2, "y")], [(1, "x"), (2, "z")]) == pytest.approx(0.5) + assert determinism_score([(1, "x")], [(1, "x"), (2, "y")]) == pytest.approx(0.5) + assert determinism_score([(1, "x")], [(1, "x")]) == 1.0 + assert determinism_score([], []) == 1.0 + + +def _report(**kw) -> EvalReport: + base = { + "ece": 0.2, + "precision_by_kind": {"sqli": 0.9}, + "recall_by_kind": {"sqli": 0.9}, + "determinism_score": 1.0, + "faithfulness_score": 0.9, + } + base.update(kw) + return EvalReport(**base) + + +def test_beats_when_ece_lower_and_no_regression() -> None: + baseline = _report() + candidate = _report(ece=0.1) + assert candidate.beats(baseline) is True + + +def test_recall_only_win_does_not_beat() -> None: + baseline = _report() + # Equal ECE, higher recall, higher faithfulness -- still fails (a). + candidate = _report(recall_by_kind={"sqli": 0.95}, faithfulness_score=0.925) + assert candidate.beats(baseline) is False + + +def test_precision_regression_beyond_tol_does_not_beat() -> None: + baseline = _report() + # ECE improves but precision drops 0.03 > tol 0.02. + candidate = _report(ece=0.1, precision_by_kind={"sqli": 0.87}) + assert candidate.beats(baseline) is False + + +def test_faithfulness_drop_does_not_beat() -> None: + baseline = _report() + candidate = _report(ece=0.1, faithfulness_score=0.85) + assert candidate.beats(baseline) is False From 9ca738a3225ae3df2bd8afe96524fb549e952551 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 02:23:41 +0300 Subject: [PATCH 015/281] feat(security): redact inline secrets at the log boundary (C6, #42) New platform helper redact_command_line masks the value after a known secret marker (-p, password=, token=, authorization: bearer, --api-key, etc.) up to the next whitespace. The SSH service now routes captured stderr and the leaked command text in its exit-code and idle-timeout error messages through it, so credentials passed on a remote command line no longer surface in exception text or logs. The config-key classification catalog and is_secret column land with #50. --- src/aila/platform/services/log_redact.py | 56 ++++++++++++++++++++++++ src/aila/platform/services/ssh.py | 8 ++-- tests/platform/test_log_redact.py | 48 ++++++++++++++++++++ 3 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 src/aila/platform/services/log_redact.py create mode 100644 tests/platform/test_log_redact.py diff --git a/src/aila/platform/services/log_redact.py b/src/aila/platform/services/log_redact.py new file mode 100644 index 00000000..8f302fd1 --- /dev/null +++ b/src/aila/platform/services/log_redact.py @@ -0,0 +1,56 @@ +"""Secret-redaction boundary helpers (C6). + +Command lines and process stderr routinely carry inline credentials +(``-p hunter2``, ``password=...``, ``authorization: bearer ...``). Before +any such text reaches a structured log, an exception message, or an audit +row, it passes through :func:`redact_command_line`, which masks the value +that follows a known secret marker up to the next whitespace. + +The helper is intentionally dependency-free so every trust boundary can +call it without importing storage, config, or model code. +""" +from __future__ import annotations + +__all__ = ["redact_command_line"] + +_REDACTED = "[REDACTED]" + +# Substrings after which everything up to the next whitespace is a secret. +_INLINE_SECRET_MARKERS: tuple[str, ...] = ( + "password=", + "passwd=", + "pass=", + "token=", + "authorization: bearer ", + "apikey=", + "api_key=", + "secret=", + "-p ", + "--password ", + "--token ", + "--api-key ", +) + + +def redact_command_line(command: str) -> str: + """Mask inline secrets in a command line or captured stderr. + + For every known secret marker, the run of non-whitespace characters + that follows it is replaced with ``[REDACTED]``. Text without a marker + is returned unchanged. + """ + if not command: + return command + out = command + lower = out.lower() + for marker in _INLINE_SECRET_MARKERS: + idx = lower.find(marker) + while idx != -1: + start = idx + len(marker) + end = start + while end < len(out) and not out[end].isspace(): + end += 1 + out = out[:start] + _REDACTED + out[end:] + lower = out.lower() + idx = lower.find(marker, start + len(_REDACTED)) + return out diff --git a/src/aila/platform/services/ssh.py b/src/aila/platform/services/ssh.py index d53e8d7c..0a9d9791 100644 --- a/src/aila/platform/services/ssh.py +++ b/src/aila/platform/services/ssh.py @@ -14,6 +14,7 @@ from ..config import PlatformSettings from ..contracts.platform import RegisteredSystem, SSHIntegrationInput from ..exceptions import AuthenticationError, TimeoutError, UpstreamError, ValidationError +from .log_redact import redact_command_line class SSHConnectionPool: @@ -199,18 +200,19 @@ def _exec_command( raise TimeoutError( f"SSH command for {payload.name} ({payload.host}) closed " f"its streams but did not emit an exit status within 30s " - f"(command likely detached a child). Command: {command[:200]}" + f"(command likely detached a child). Command: {redact_command_line(command)[:200]}" ) _time.sleep(0.1) exit_code = stdout.channel.recv_exit_status() except builtins.TimeoutError as exc: raise TimeoutError( f"SSH command for {payload.name} ({payload.host}) idle " - f">{timeout_seconds}s with no output. Command: {command[:200]}" + f">{timeout_seconds}s with no output. Command: {redact_command_line(command)[:200]}" ) from exc if exit_code != 0: + redacted_stderr = redact_command_line(error_output) raise UpstreamError( - f"SSH command failed for {payload.name} ({payload.host}) with exit code {exit_code}: {error_output}" + f"SSH command failed for {payload.name} ({payload.host}) with exit code {exit_code}: {redacted_stderr}" ) return output except paramiko.AuthenticationException as exc: diff --git a/tests/platform/test_log_redact.py b/tests/platform/test_log_redact.py new file mode 100644 index 00000000..0ad091bb --- /dev/null +++ b/tests/platform/test_log_redact.py @@ -0,0 +1,48 @@ +"""Secret-redaction boundary tests for C6 (#42 / #50).""" +from __future__ import annotations + +from aila.platform.services.log_redact import redact_command_line + + +def test_redacts_short_password_flag() -> None: + out = redact_command_line("mysql -u root -p hunter2 -h db") + assert "hunter2" not in out + assert "[REDACTED]" in out + assert out == "mysql -u root -p [REDACTED] -h db" + + +def test_redacts_inline_password_assignment() -> None: + out = redact_command_line("psql password=hunter2 host=db") + assert out == "psql password=[REDACTED] host=db" + + +def test_redacts_bearer_token() -> None: + out = redact_command_line("curl -H 'authorization: bearer abc123def'") + assert "abc123def" not in out + assert "[REDACTED]" in out + + +def test_redacts_multiple_markers() -> None: + out = redact_command_line("run password=p1 then token=t2 done") + assert "p1" not in out + assert "t2" not in out + assert out.count("[REDACTED]") == 2 + + +def test_value_at_end_of_line_is_redacted() -> None: + assert redact_command_line("mysql -p secret") == "mysql -p [REDACTED]" + + +def test_no_secret_marker_unchanged() -> None: + assert redact_command_line("ls -la /tmp/output") == "ls -la /tmp/output" + + +def test_empty_input_unchanged() -> None: + assert redact_command_line("") == "" + + +def test_long_flag_forms() -> None: + out = redact_command_line("tool --token deadbeef --api-key cafef00d") + assert "deadbeef" not in out + assert "cafef00d" not in out + assert out.count("[REDACTED]") == 2 From a4034839d30d2370cf80ef6e052eaa56156bfcc2 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 02:26:32 +0300 Subject: [PATCH 016/281] fix(reporting): verify TLS certificate when emailing reports (#48) Scheduled report emails carry security findings; STARTTLS was invoked with no SSL context, so smtplib used an unverified stdlib context and the server certificate was never checked. The send path now passes ssl.create_default_context(), enabling hostname and certificate verification. --- src/aila/platform/tasks/report_tasks.py | 4 ++- tests/platform/test_report_email_tls.py | 39 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tests/platform/test_report_email_tls.py diff --git a/src/aila/platform/tasks/report_tasks.py b/src/aila/platform/tasks/report_tasks.py index 61600e27..c53565ae 100644 --- a/src/aila/platform/tasks/report_tasks.py +++ b/src/aila/platform/tasks/report_tasks.py @@ -23,6 +23,7 @@ import json import logging import smtplib +import ssl from datetime import UTC, datetime from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart @@ -278,9 +279,10 @@ def _send_report_email( ) msg.attach(attachment) + context = ssl.create_default_context() with smtplib.SMTP(smtp_host, smtp_port, timeout=30) as server: server.ehlo() - server.starttls() + server.starttls(context=context) if smtp_username and smtp_password: server.login(smtp_username, smtp_password) server.sendmail(smtp_from, [recipient], msg.as_string()) diff --git a/tests/platform/test_report_email_tls.py b/tests/platform/test_report_email_tls.py new file mode 100644 index 00000000..d71394e8 --- /dev/null +++ b/tests/platform/test_report_email_tls.py @@ -0,0 +1,39 @@ +"""Report-email TLS verification test for #48 / #45. + +Scheduled report emails carry security findings. STARTTLS was called with +no SSL context, so the server certificate was not verified (a MITM path). +The send path now passes a verifying default context. +""" +from __future__ import annotations + +import ssl +from unittest.mock import MagicMock + +from aila.platform.tasks import report_tasks + + +def test_send_report_email_uses_verifying_tls(monkeypatch) -> None: + server = MagicMock() + smtp_cm = MagicMock() + smtp_cm.__enter__.return_value = server + smtp_cm.__exit__.return_value = False + monkeypatch.setattr(report_tasks.smtplib, "SMTP", MagicMock(return_value=smtp_cm)) + + report_tasks._send_report_email( + smtp_host="smtp.example.com", + smtp_port=587, + smtp_from="from@example.com", + smtp_username=None, + smtp_password=None, + recipient="to@example.com", + report_name="Test Report", + date_str="2026-07-20", + pdf_bytes=b"%PDF-1.4 test", + ) + + server.starttls.assert_called_once() + context = server.starttls.call_args.kwargs.get("context") + assert isinstance(context, ssl.SSLContext) + assert context.check_hostname is True + assert context.verify_mode == ssl.CERT_REQUIRED + server.sendmail.assert_called_once() From 20fa2bde9cb40e26d7470ade3a30bc5810d2ca86 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 03:12:41 +0300 Subject: [PATCH 017/281] test(backbone): restore SQLite fixture compatibility for shared metadata (#62) Legacy unit tests build an in-memory SQLite engine and create_all the whole metadata, which aborted on Postgres-only JSONB/TSVECTOR columns and the to_tsvector generated column, failing every SQLite-based test at setup. Root conftest adds test-only @compiles shims (JSONB->JSON, TSVECTOR->TEXT on sqlite) and a connect-time deterministic to_tsvector passthrough; production models stay pure Postgres. Also updates test_87 to the real patching-urgency criticality vocabulary (Immediate/High/Moderate/Planned) that findings store, correcting stale assertions surfaced once create_all succeeds, and isolates the NVD limiter test from ambient DB-URL pollution. Full non-e2e suite: +87 passing, zero new failures (verified by node-id diff vs baseline). Full Alembic-driven Postgres fixture migration remains the eventual #62 target. --- tests/conftest.py | 51 +++++++++++++++ .../vulnerability/test_nvd_rate_limiter.py | 11 +++- tests/test_87_vulnerability_module_core.py | 65 +++++++++---------- 3 files changed, 90 insertions(+), 37 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 3ce55a69..5d70ec38 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,2 +1,53 @@ +"""Project-wide pytest fixtures and test-only database shims. + +Many legacy unit tests build an in-memory SQLite engine and call +``SQLModel.metadata.create_all`` against the whole metadata. That metadata +contains Postgres-only constructs (JSONB and TSVECTOR columns, and a STORED +generated column computed by ``to_tsvector``) that SQLite cannot render or +execute, so a single incompatible type aborts create_all for every table and +every SQLite-based test fails at setup. + +These shims keep production models pure (Postgres still gets JSONB/TSVECTOR and +the real generated column) while letting the shared metadata create_all on a +SQLite test engine: + +- ``@compiles(JSONB, "sqlite")`` -> ``JSON`` and ``@compiles(TSVECTOR, "sqlite")`` + -> ``TEXT`` so the column types render on SQLite. +- a ``connect`` shim registers a passthrough ``to_tsvector`` so the knowledge + table's generated column evaluates on SQLite instead of raising + ``no such function``. + +None of this touches the PostgreSQL path used by the Alembic-driven ``test_db`` +fixture. +""" from __future__ import annotations +from sqlalchemy import event +from sqlalchemy.dialects.postgresql import JSONB, TSVECTOR +from sqlalchemy.engine import Engine +from sqlalchemy.ext.compiler import compiles + + +@compiles(JSONB, "sqlite") +def _compile_jsonb_sqlite(_type, _compiler, **_kw) -> str: + return "JSON" + + +@compiles(TSVECTOR, "sqlite") +def _compile_tsvector_sqlite(_type, _compiler, **_kw) -> str: + return "TEXT" + + +@event.listens_for(Engine, "connect") +def _register_sqlite_pg_shims(dbapi_connection, _record) -> None: + """Register Postgres-only functions used in generated columns on SQLite. + + Only SQLite DBAPI connections expose ``create_function``; Postgres drivers + do not, so this is a no-op there. + """ + create_function = getattr(dbapi_connection, "create_function", None) + if create_function is None: + return + # search_vector is Computed as to_tsvector('english', content); a passthrough + # keeps SQLite create_all and inserts working (FTS itself is Postgres-only). + create_function("to_tsvector", 2, lambda _config, text: text or "", deterministic=True) diff --git a/tests/modules/vulnerability/test_nvd_rate_limiter.py b/tests/modules/vulnerability/test_nvd_rate_limiter.py index 7da953b0..d93832e5 100644 --- a/tests/modules/vulnerability/test_nvd_rate_limiter.py +++ b/tests/modules/vulnerability/test_nvd_rate_limiter.py @@ -9,14 +9,21 @@ """ from __future__ import annotations -from aila.config import get_settings +from aila.config import _build_settings, get_settings from aila.modules.vulnerability.config_schema import VulnerabilityConfigSchema from aila.modules.vulnerability.providers import nvd as nvd_mod from aila.modules.vulnerability.providers.nvd import NVDClient -def test_stamp_reflects_post_sleep_proceed_time(monkeypatch) -> None: +def test_stamp_reflects_post_sleep_proceed_time(monkeypatch, request) -> None: monkeypatch.delenv("NVD_API_KEY", raising=False) + # Isolate from ambient DB-URL pollution: get_settings() rejects a sqlite URL + # that a prior test may have left in the environment. This client never + # connects to a database. Clear the settings cache on teardown so the fake + # URL does not leak into later tests. + monkeypatch.setenv("AILA_DATABASE_URL", "postgresql+asyncpg://test:test@localhost:5432/test") + request.addfinalizer(_build_settings.cache_clear) + _build_settings.cache_clear() clock = {"t": 1000.0} monkeypatch.setattr(nvd_mod.time, "monotonic", lambda: clock["t"]) monkeypatch.setattr(nvd_mod.time, "sleep", lambda d: clock.__setitem__("t", clock["t"] + d)) diff --git a/tests/test_87_vulnerability_module_core.py b/tests/test_87_vulnerability_module_core.py index 5e10b45c..9e7ccf65 100644 --- a/tests/test_87_vulnerability_module_core.py +++ b/tests/test_87_vulnerability_module_core.py @@ -13,12 +13,15 @@ from __future__ import annotations import json +from dataclasses import FrozenInstanceError from datetime import UTC, datetime import pytest +from fastapi import APIRouter from sqlalchemy.dialects.sqlite import insert as sa_insert from sqlmodel import Session, SQLModel, create_engine +from aila.modules.vulnerability.db_models import LatestFindingRecord from aila.modules.vulnerability.module import VulnerabilityModule __all__ = [ @@ -67,8 +70,6 @@ def __getattr__(self, name: str): @pytest.fixture() def engine(): - from aila.modules.vulnerability.db_models import LatestFindingRecord - eng = create_engine("sqlite:///:memory:", connect_args={"check_same_thread": False}) SQLModel.metadata.create_all(eng, tables=[LatestFindingRecord.__table__]) yield eng @@ -105,8 +106,6 @@ def _insert_finding( status: str = "open", ) -> None: """Insert a LatestFindingRecord via the same upsert pattern as production.""" - from aila.modules.vulnerability.db_models import LatestFindingRecord - now = datetime.now(UTC) stmt = ( sa_insert(LatestFindingRecord) @@ -177,8 +176,6 @@ def test_spec_router_factory_callable(self, module: VulnerabilityModule) -> None def test_router_factory_returns_api_router(self, module: VulnerabilityModule) -> None: """router_factory() produces a FastAPI APIRouter instance.""" - from fastapi import APIRouter - spec = module.route_specs()[0] router = spec.router_factory() assert isinstance(router, APIRouter) @@ -200,8 +197,6 @@ def test_router_has_expected_paths(self, module: VulnerabilityModule) -> None: def test_spec_is_frozen(self, module: VulnerabilityModule) -> None: """ModuleRouteSpec instances are frozen (immutable).""" - from dataclasses import FrozenInstanceError - spec = module.route_specs()[0] with pytest.raises(FrozenInstanceError): spec.prefix = "/hacked" # type: ignore[misc] @@ -230,32 +225,32 @@ async def test_no_findings_returns_empty(self, module: VulnerabilityModule, sess @pytest.mark.asyncio async def test_single_criticality(self, module: VulnerabilityModule, session: Session) -> None: """system_summary() counts a single finding correctly.""" - _insert_finding(session, system_id=1, criticality="CRITICAL", cve_id="CVE-2024-0001") + _insert_finding(session, system_id=1, criticality="Immediate", cve_id="CVE-2024-0001") result = await module.system_summary(system_id=1, session=session) - assert result["critical"] == 1 + assert result["immediate"] == 1 assert result["high"] == 0 - assert result["medium"] == 0 - assert result["low"] == 0 + assert result["moderate"] == 0 + assert result["planned"] == 0 @pytest.mark.asyncio async def test_mixed_criticalities(self, module: VulnerabilityModule, session: Session) -> None: """system_summary() counts multiple criticalities correctly.""" - _insert_finding(session, system_id=1, criticality="CRITICAL", cve_id="CVE-2024-0001", package_name="a") - _insert_finding(session, system_id=1, criticality="CRITICAL", cve_id="CVE-2024-0002", package_name="b") - _insert_finding(session, system_id=1, criticality="HIGH", cve_id="CVE-2024-0003", package_name="c") - _insert_finding(session, system_id=1, criticality="MEDIUM", cve_id="CVE-2024-0004", package_name="d") - _insert_finding(session, system_id=1, criticality="LOW", cve_id="CVE-2024-0005", package_name="e") + _insert_finding(session, system_id=1, criticality="Immediate", cve_id="CVE-2024-0001", package_name="a") + _insert_finding(session, system_id=1, criticality="Immediate", cve_id="CVE-2024-0002", package_name="b") + _insert_finding(session, system_id=1, criticality="High", cve_id="CVE-2024-0003", package_name="c") + _insert_finding(session, system_id=1, criticality="Moderate", cve_id="CVE-2024-0004", package_name="d") + _insert_finding(session, system_id=1, criticality="Planned", cve_id="CVE-2024-0005", package_name="e") result = await module.system_summary(system_id=1, session=session) - assert result == {"critical": 2, "high": 1, "medium": 1, "low": 1} + assert result == {"immediate": 2, "high": 1, "moderate": 1, "planned": 1} @pytest.mark.asyncio async def test_filters_by_system_id(self, module: VulnerabilityModule, session: Session) -> None: """system_summary() only counts findings for the requested system_id.""" - _insert_finding(session, system_id=1, criticality="HIGH", cve_id="CVE-2024-0001") - _insert_finding(session, system_id=2, criticality="CRITICAL", cve_id="CVE-2024-0002", system_name="other-vm") + _insert_finding(session, system_id=1, criticality="High", cve_id="CVE-2024-0001") + _insert_finding(session, system_id=2, criticality="Immediate", cve_id="CVE-2024-0002", system_name="other-vm") result = await module.system_summary(system_id=1, session=session) assert result["high"] == 1 - assert result["critical"] == 0 + assert result["immediate"] == 0 @pytest.mark.asyncio async def test_case_insensitive_criticality(self, module: VulnerabilityModule, session: Session) -> None: @@ -416,30 +411,30 @@ async def test_no_findings_returns_empty(self, module: VulnerabilityModule, sess @pytest.mark.asyncio async def test_single_finding_count(self, module: VulnerabilityModule, session: Session) -> None: """report_count() returns correct counts for a single finding.""" - _insert_finding(session, criticality="CRITICAL", cve_id="CVE-2024-0001") + _insert_finding(session, criticality="Immediate", cve_id="CVE-2024-0001") result = await module.report_count(run_id="run-123", session=session) assert result["total_findings"] == 1 - assert result["critical"] == 1 + assert result["immediate"] == 1 assert result["high"] == 0 - assert result["medium"] == 0 - assert result["low"] == 0 + assert result["moderate"] == 0 + assert result["planned"] == 0 @pytest.mark.asyncio async def test_mixed_findings_count(self, module: VulnerabilityModule, session: Session) -> None: """report_count() returns correct breakdown for multiple criticalities.""" - _insert_finding(session, criticality="CRITICAL", cve_id="CVE-2024-0001", package_name="a") - _insert_finding(session, criticality="HIGH", cve_id="CVE-2024-0002", package_name="b") - _insert_finding(session, criticality="HIGH", cve_id="CVE-2024-0003", package_name="c") - _insert_finding(session, criticality="MEDIUM", cve_id="CVE-2024-0004", package_name="d") - _insert_finding(session, criticality="LOW", cve_id="CVE-2024-0005", package_name="e") - _insert_finding(session, criticality="LOW", cve_id="CVE-2024-0006", package_name="f") - _insert_finding(session, criticality="LOW", cve_id="CVE-2024-0007", package_name="g") + _insert_finding(session, criticality="Immediate", cve_id="CVE-2024-0001", package_name="a") + _insert_finding(session, criticality="High", cve_id="CVE-2024-0002", package_name="b") + _insert_finding(session, criticality="High", cve_id="CVE-2024-0003", package_name="c") + _insert_finding(session, criticality="Moderate", cve_id="CVE-2024-0004", package_name="d") + _insert_finding(session, criticality="Planned", cve_id="CVE-2024-0005", package_name="e") + _insert_finding(session, criticality="Planned", cve_id="CVE-2024-0006", package_name="f") + _insert_finding(session, criticality="Planned", cve_id="CVE-2024-0007", package_name="g") result = await module.report_count(run_id="run-123", session=session) assert result["total_findings"] == 7 - assert result["critical"] == 1 + assert result["immediate"] == 1 assert result["high"] == 2 - assert result["medium"] == 1 - assert result["low"] == 3 + assert result["moderate"] == 1 + assert result["planned"] == 3 @pytest.mark.asyncio async def test_run_id_parameter_accepted(self, module: VulnerabilityModule, session: Session) -> None: From c5705f36ab16a25e8aad2cc0dcd7e67bd1ef82e8 Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 03:15:04 +0300 Subject: [PATCH 018/281] fix(forensics): read real ArtifactRecord fields in writeup builder (#59) The writeup artefact grouper read phantom attributes (family/type/data) via getattr defaults, so every artefact fell into the 'unknown' family with empty data and writeups rendered blank. Grouping now reads the real columns (artifact_family / artifact_type / data_json), extracted into a pure _group_artefacts helper with unit coverage. --- .../forensics/reporting/writeup_builder.py | 44 +++++++------- .../forensics/test_writeup_group_artefacts.py | 59 +++++++++++++++++++ 2 files changed, 83 insertions(+), 20 deletions(-) create mode 100644 tests/modules/forensics/test_writeup_group_artefacts.py diff --git a/src/aila/modules/forensics/reporting/writeup_builder.py b/src/aila/modules/forensics/reporting/writeup_builder.py index 92fd7ee2..3ec8e5b1 100644 --- a/src/aila/modules/forensics/reporting/writeup_builder.py +++ b/src/aila/modules/forensics/reporting/writeup_builder.py @@ -356,35 +356,39 @@ async def _load_artefacts_by_family(project_id: str) -> dict[str, list[dict[str, from aila.modules.forensics.db_models import ArtifactRecord from aila.platform.uow import UnitOfWork - grouped: dict[str, list[dict[str, Any]]] = defaultdict(list) async with UnitOfWork() as uow: rows = (await uow.session.exec( select(ArtifactRecord).where(ArtifactRecord.project_id == project_id) )).all() - for r in rows: - family = getattr(r, "family", "unknown") or "unknown" - payload = getattr(r, "data", None) - if isinstance(payload, str): - try: - payload_obj = json.loads(payload) - except json.JSONDecodeError: - payload_obj = {} - elif isinstance(payload, dict): - payload_obj = payload - else: - payload_obj = {} - grouped[family].append({ - "id": getattr(r, "id", ""), - "type": getattr(r, "type", ""), - "source_tool": getattr(r, "source_tool", ""), - "data": payload_obj, - }) - return dict(grouped) + return _group_artefacts(rows) except (OSError, RuntimeError, ValueError): _log.warning("Failed to load artefacts_by_family for %s", project_id, exc_info=True) return {} +def _group_artefacts(rows: list[Any]) -> dict[str, list[dict[str, Any]]]: + """Group ArtifactRecord rows by family, reading the real column names. + + ArtifactRecord stores artifact_family / artifact_type / data_json; the + previous loop read phantom attributes (family / type / data), so every + row landed under 'unknown' with empty data and writeups rendered blank. + """ + grouped: dict[str, list[dict[str, Any]]] = defaultdict(list) + for r in rows: + family = r.artifact_family or "unknown" + try: + payload_obj = json.loads(r.data_json) if r.data_json else {} + except (json.JSONDecodeError, TypeError): + payload_obj = {} + grouped[family].append({ + "id": r.id, + "type": r.artifact_type or "", + "source_tool": r.source_tool or "", + "data": payload_obj, + }) + return dict(grouped) + + def _section_evidence_inventory(artefacts_by_family: dict[str, list[dict[str, Any]]]) -> str: """Inventory of the evidence files on disk, mined from binary_analysis.""" rows: list[str] = ["# EVIDENCE INVENTORY"] diff --git a/tests/modules/forensics/test_writeup_group_artefacts.py b/tests/modules/forensics/test_writeup_group_artefacts.py new file mode 100644 index 00000000..7c08bc79 --- /dev/null +++ b/tests/modules/forensics/test_writeup_group_artefacts.py @@ -0,0 +1,59 @@ +"""Writeup artefact-grouping tests for #59. + +The writeup builder grouped artefacts by phantom attribute names +(family/type/data), so every row fell into 'unknown' with empty data and +writeups rendered blank. _group_artefacts now reads the real ArtifactRecord +columns (artifact_family / artifact_type / data_json). +""" +from __future__ import annotations + +from types import SimpleNamespace + +from aila.modules.forensics.reporting.writeup_builder import _group_artefacts + + +def _row(family: str, type_: str, data_json: str, id_: str = "a1", tool: str = "dissect"): + return SimpleNamespace( + artifact_family=family, + artifact_type=type_, + data_json=data_json, + id=id_, + source_tool=tool, + ) + + +def test_groups_by_real_family_and_reads_data() -> None: + rows = [_row("malware", "binary_summary", '{"sha256": "deadbeef"}')] + grouped = _group_artefacts(rows) + assert "malware" in grouped + assert "unknown" not in grouped + entry = grouped["malware"][0] + assert entry["type"] == "binary_summary" + assert entry["data"]["sha256"] == "deadbeef" + assert entry["source_tool"] == "dissect" + + +def test_multiple_families() -> None: + rows = [ + _row("malware", "bin", "{}", id_="1"), + _row("network", "pcap", "{}", id_="2"), + _row("malware", "bin", "{}", id_="3"), + ] + grouped = _group_artefacts(rows) + assert len(grouped["malware"]) == 2 + assert len(grouped["network"]) == 1 + + +def test_missing_family_falls_back_to_unknown() -> None: + rows = [_row("", "bin", "{}")] + grouped = _group_artefacts(rows) + assert grouped["unknown"][0]["type"] == "bin" + + +def test_malformed_data_json_yields_empty_dict() -> None: + rows = [_row("malware", "bin", "not json")] + assert _group_artefacts(rows)["malware"][0]["data"] == {} + + +def test_empty_rows() -> None: + assert _group_artefacts([]) == {} From a1c8da23cdb893729c89be73a3faaf163e81939b Mon Sep 17 00:00:00 2001 From: echel0nn Date: Mon, 20 Jul 2026 03:20:22 +0300 Subject: [PATCH 019/281] fix(reporting): autoescape and URL-scheme guard for vulnerability PDF (#43) The vulnerability report PDF rendered untrusted CVE data (cve_id, rationale, package names) through a Jinja environment with no autoescape, and the nvd_url flowed straight into an href, so HTML and javascript: injection survived into the rasterized document. The environment now enables select_autoescape and a safe_url filter neutralizes any non-http(s)/mailto scheme to about:blank; a restrictive CSP meta is added for standalone HTML viewing. Extracted _build_report_env and _safe_report_url with unit coverage. --- .../modules/vulnerability/reporting/pdf.py | 52 +++++++++++++----- .../vulnerability/test_pdf_url_guard.py | 55 +++++++++++++++++++ 2 files changed, 94 insertions(+), 13 deletions(-) create mode 100644 tests/modules/vulnerability/test_pdf_url_guard.py diff --git a/src/aila/modules/vulnerability/reporting/pdf.py b/src/aila/modules/vulnerability/reporting/pdf.py index 774b58d1..8f263480 100644 --- a/src/aila/modules/vulnerability/reporting/pdf.py +++ b/src/aila/modules/vulnerability/reporting/pdf.py @@ -3,13 +3,50 @@ from datetime import UTC from pathlib import Path from typing import Any +from urllib.parse import urlparse __all__ = ["PDFReportRenderer"] +_SAFE_URL_SCHEMES: frozenset[str] = frozenset({"http", "https", "mailto"}) + + +def _safe_report_url(u: str | None) -> str: + """Neutralize non-http(s)/mailto URL schemes before they reach an href. + + Untrusted CVE data flows into report links; without this guard a + javascript:/data: scheme would survive HTML autoescaping. + """ + if not u: + return "about:blank" + parsed = urlparse(u.strip()) + scheme = parsed.scheme.lower() + if scheme not in _SAFE_URL_SCHEMES: + return "about:blank" + if scheme in {"http", "https"} and not parsed.netloc: + return "about:blank" + return u + + +def _build_report_env(): + """Build the Jinja2 Environment with HTML autoescape and the safe_url filter.""" + try: + from jinja2 import Environment, Undefined, select_autoescape + except ImportError as exc: + raise ImportError( + "PDF rendering requires optional dependencies. Install with: pip install aila[pdf]" + ) from exc + env = Environment(undefined=Undefined, autoescape=select_autoescape(["html", "xml"])) + env.filters["truncate"] = ( + lambda s, length=255: (str(s)[:length] + "\u2026") if len(str(s)) > length else str(s) + ) + env.filters["safe_url"] = _safe_report_url + return env + _HTML_TEMPLATE = """ +