From dd263d3fa23481b12f62913a3f4416f7485ed362 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 6 Aug 2026 12:33:24 +0100 Subject: [PATCH 1/2] fix(security): enforce input validation boundaries Signed-off-by: Tanvir Farhad --- api/app.py | 3 +- api/observability.py | 5 +- api/routes/ai.py | 100 +++++++++-------- api/routes/findings.py | 43 +++++++- api/routes/scans.py | 14 ++- api/validation.py | 137 ++++++++++++++++++++++++ docs/api-reference.md | 19 +++- docs/input-validation-audit.md | 52 +++++++++ docs/openssf-silver-evidence.md | 2 +- docs/security-requirements.md | 3 + sentinel/ingest.py | 91 ++++++++++++---- tests/test_error_exposure.py | 2 +- tests/test_input_validation.py | 136 +++++++++++++++++++++++ tests/test_scans_enrich.py | 24 +++-- tests/test_sentinel_input_validation.py | 38 +++++++ 15 files changed, 582 insertions(+), 87 deletions(-) create mode 100644 api/validation.py create mode 100644 docs/input-validation-audit.md create mode 100644 tests/test_input_validation.py create mode 100644 tests/test_sentinel_input_validation.py diff --git a/api/app.py b/api/app.py index 217742f..bf0c971 100644 --- a/api/app.py +++ b/api/app.py @@ -29,6 +29,7 @@ _INSECURE_JWT_DEFAULT = "change-me-in-production" _MIN_JWT_SECRET_LENGTH = 32 +_MAX_AUTHORIZATION_HEADER_LENGTH = 8192 _GENERATE_CMD = 'python -c "import secrets; print(secrets.token_urlsafe(32))"' @@ -167,7 +168,7 @@ def verify_jwt() -> None: return None auth = request.headers.get("Authorization", "") - if not auth.startswith("Bearer "): + if len(auth) > _MAX_AUTHORIZATION_HEADER_LENGTH or not auth.startswith("Bearer "): return jsonify( { "error": "Missing or malformed Authorization header", diff --git a/api/observability.py b/api/observability.py index 8f48ecd..4b7bcfa 100644 --- a/api/observability.py +++ b/api/observability.py @@ -13,6 +13,7 @@ import logging import os +import re import time import uuid @@ -35,6 +36,7 @@ logger = logging.getLogger(__name__) REQUEST_ID_HEADER = "X-Request-ID" +_REQUEST_ID_RE = re.compile(r"^[A-Za-z0-9._:-]{1,128}$") # --------------------------------------------------------------------------- # # Prometheus metrics # @@ -160,7 +162,8 @@ def init_app(app: Flask) -> None: @app.before_request def _start_observability() -> None: - g.request_id = request.headers.get(REQUEST_ID_HEADER) or str(uuid.uuid4()) + supplied_request_id = request.headers.get(REQUEST_ID_HEADER, "") + g.request_id = supplied_request_id if _REQUEST_ID_RE.fullmatch(supplied_request_id) else str(uuid.uuid4()) g.request_start_time = time.perf_counter() @app.after_request diff --git a/api/routes/ai.py b/api/routes/ai.py index 776796a..22ac101 100644 --- a/api/routes/ai.py +++ b/api/routes/ai.py @@ -8,6 +8,18 @@ from api.rate_limit import rate_limit from api.services.ai_provider import PROVIDERS as SUPPORTED_PROVIDERS from api.services.ai_provider import get_completion +from api.validation import ( + MAX_API_KEY_LENGTH, + MAX_MODEL_LENGTH, + MAX_QUESTION_LENGTH, + MODEL_RE, + ValidationError, + bounded_string, + choice, + findings_list, + reject_unknown_fields, + require_json_object, +) from ai.retriever import retrieve, VectorStoreNotBuilt ai_bp = Blueprint("ai", __name__) @@ -148,14 +160,18 @@ def _context_for(query): def _read_request(): - body = request.get_json(silent=True) - if not body: - return None, (jsonify({"error": "Request body must be JSON"}), 400) - if not body.get("provider"): - return None, (jsonify({"error": "provider is required"}), 400) - if not body.get("api_key"): - return None, (jsonify({"error": "api_key is required"}), 400) - return body, None + try: + body = require_json_object(request.get_json(silent=True)) + reject_unknown_fields(body, {"provider", "api_key", "model", "findings", "question"}) + body["provider"] = choice(body.get("provider"), "provider", SUPPORTED_PROVIDERS, case="lower") + body["api_key"] = bounded_string(body.get("api_key"), "api_key", maximum=MAX_API_KEY_LENGTH) + if body.get("model") is not None: + body["model"] = bounded_string(body["model"], "model", maximum=MAX_MODEL_LENGTH, pattern=MODEL_RE) + if ".." in body["model"]: + raise ValidationError("model has an invalid format") + return body, None + except ValidationError as exc: + return None, (jsonify({"error": str(exc)}), 400) _AI_ERROR_MESSAGES = { @@ -180,27 +196,21 @@ def _ai_error_response(exc: Exception, status: int, log_context: str): @ai_bp.post("/api/ai/insights") @rate_limit(_AI_RATE_LIMIT) def insights(): - data = request.get_json(silent=True) - if data is None: - return jsonify({"error": "Request body must be valid JSON"}), 400 - - provider = str(data.get("provider") or "").strip().lower() - api_key = str(data.get("api_key") or "").strip() - findings = data.get("findings") - question = str(data.get("question") or "").strip() - - if not provider: - return jsonify({"error": "Missing required field: provider"}), 400 - if provider not in SUPPORTED_PROVIDERS: - return jsonify({"error": f"Unsupported provider: {provider}"}), 400 - if not api_key: - return jsonify({"error": "Missing required field: api_key"}), 400 - if findings is None: - return jsonify({"error": "Missing required field: findings"}), 400 - if not isinstance(findings, list): - return jsonify({"error": "findings must be a list"}), 400 - if len(findings) == 0: - return jsonify({"error": "findings must not be empty"}), 400 + data, error = _read_request() + if error: + return error + try: + provider = data["provider"] + api_key = data["api_key"] + findings = findings_list(data.get("findings"), required=True) + question = "" + if data.get("question") is not None: + if not isinstance(data["question"], str): + raise ValidationError("question must be a string") + if data["question"].strip(): + question = bounded_string(data["question"], "question", maximum=MAX_QUESTION_LENGTH) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 sorted_findings = sorted(findings, key=severity_rank, reverse=True) @@ -234,9 +244,10 @@ def ai_summary(): body, error = _read_request() if error: return error - findings = body.get("findings", []) - if not isinstance(findings, list): - return jsonify({"error": "findings must be a list"}), 400 + try: + findings = findings_list(body.get("findings")) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 findings_text = _findings_to_text(findings) try: @@ -273,9 +284,10 @@ def ai_prioritise(): body, error = _read_request() if error: return error - findings = body.get("findings", []) - if not isinstance(findings, list): - return jsonify({"error": "findings must be a list"}), 400 + try: + findings = findings_list(body.get("findings")) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 findings_text = _findings_to_text(findings) try: @@ -319,16 +331,17 @@ def ai_ask(): body, error = _read_request() if error: return error - question = body.get("question", "") - if not question or not question.strip(): - return jsonify({"error": "question is required"}), 400 + try: + question = bounded_string(body.get("question"), "question", maximum=MAX_QUESTION_LENGTH) + findings = findings_list(body.get("findings")) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 try: context, sources = _context_for(question) except VectorStoreNotBuilt as exc: return _ai_error_response(exc, 503, "Vector store unavailable in ai_ask") - findings = body.get("findings", []) findings_text = _findings_to_text(findings) if findings else "Not provided." prompt = ( @@ -361,11 +374,10 @@ def ai_threat_simulation(): body, error = _read_request() if error: return error - findings = body.get("findings", []) - if not isinstance(findings, list): - return jsonify({"error": "findings must be a list"}), 400 - if not findings: - return jsonify({"error": "findings must not be empty"}), 400 + try: + findings = findings_list(body.get("findings"), required=True) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 findings_text = _findings_to_text(findings) try: diff --git a/api/routes/findings.py b/api/routes/findings.py index 91f6afb..1b1d7e4 100644 --- a/api/routes/findings.py +++ b/api/routes/findings.py @@ -2,19 +2,26 @@ import logging import os -import re from pathlib import Path from flask import Blueprint, g, jsonify, request from api.models.finding import DatabaseManager +from api.validation import ( + CATEGORIES, + RULE_ID_RE, + SEVERITIES, + ValidationError, + bounded_string, + choice, + positive_integer, + uuid_string, +) _PLAYBOOKS_DIR = (Path(__file__).parent.parent.parent / "playbooks" / "cli").resolve() # Known rule_id shape, e.g. AZ-STOR-001. Anything else is rejected before it # ever reaches the filesystem, closing off path traversal via a crafted or # corrupted rule_id. -_RULE_ID_RE = re.compile(r"^[A-Z0-9]+(?:-[A-Z0-9]+)*$") - findings_bp = Blueprint("findings", __name__) logger = logging.getLogger(__name__) @@ -37,10 +44,30 @@ def list_findings(): scan_id - UUID of a specific scan """ try: - filters = {k: v for k, v in request.args.items() if k in ("severity", "category", "rule_id", "scan_id")} + allowed = {"severity", "category", "rule_id", "scan_id"} + unknown = set(request.args) - allowed + if unknown: + raise ValidationError(f"Unsupported query parameter: {sorted(unknown)[0]}") + for key in request.args: + if len(request.args.getlist(key)) != 1: + raise ValidationError(f"Query parameter {key} must be provided once") + + filters = {} + if "severity" in request.args: + filters["severity"] = choice(request.args["severity"], "severity", SEVERITIES, case="upper") + if "category" in request.args: + filters["category"] = choice(request.args["category"], "category", CATEGORIES) + if "rule_id" in request.args: + filters["rule_id"] = bounded_string( + request.args["rule_id"].upper(), "rule_id", maximum=64, pattern=RULE_ID_RE + ) + if "scan_id" in request.args: + filters["scan_id"] = uuid_string(request.args["scan_id"], "scan_id") db = _get_db() findings = db.get_findings(filters) return jsonify({"count": len(findings), "findings": findings}) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 except Exception as exc: logger.error("Failed to list findings: %s", exc) return jsonify({"error": "Failed to retrieve findings"}), 500 @@ -50,11 +77,14 @@ def list_findings(): def get_finding(finding_id: int): """Return a single finding by its integer ID.""" try: + finding_id = positive_integer(finding_id, "finding_id") db = _get_db() finding = db.get_finding_by_id(finding_id) if not finding: return jsonify({"error": "Finding not found"}), 404 return jsonify(finding) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 except Exception as exc: logger.error("Failed to get finding %d: %s", finding_id, exc) return jsonify({"error": "Database error"}), 500 @@ -68,6 +98,7 @@ def get_playbook(finding_id: int): and combines it with the finding's remediation guidance and any CVE references. """ try: + finding_id = positive_integer(finding_id, "finding_id") db = _get_db() finding = db.get_finding_by_id(finding_id) if not finding: @@ -79,7 +110,7 @@ def get_playbook(finding_id: int): cli_commands = [] script_path = None - if _RULE_ID_RE.match(rule_id or ""): + if RULE_ID_RE.match(rule_id or ""): # Map rule_id (e.g. AZ-STOR-001) to script filename (fix_az_stor_001.sh) script_name = "fix_" + rule_id.lower().replace("-", "_") + ".sh" candidate = (_PLAYBOOKS_DIR / script_name).resolve() @@ -124,6 +155,8 @@ def get_playbook(finding_id: int): } ) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 except Exception as exc: logger.error("Failed to get playbook for finding %d: %s", finding_id, exc) return jsonify({"error": "Failed to retrieve playbook"}), 500 diff --git a/api/routes/scans.py b/api/routes/scans.py index 17c2c59..f2a8254 100644 --- a/api/routes/scans.py +++ b/api/routes/scans.py @@ -7,6 +7,7 @@ from flask import Blueprint, g, jsonify, request from api.models.finding import DatabaseManager +from api.validation import ValidationError, reject_unknown_fields, require_json_object, uuid_string from scanner.cve_correlator import enrich_findings scans_bp = Blueprint("scans", __name__) @@ -39,11 +40,14 @@ def list_scans(): def get_scan_status(scan_id): """Return the details and status of a specific scan.""" try: + scan_id = uuid_string(scan_id, "scan_id") db = _get_db() scan = db.get_scan(scan_id) if not scan: return jsonify({"error": "Scan not found"}), 404 return jsonify(scan) + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 except Exception as exc: logger.error("Failed to get scan status: %s", exc) return jsonify({"error": "Database error"}), 500 @@ -59,11 +63,14 @@ def trigger_scan(): Returns 202 Accepted with the scan_id immediately. """ try: - body = request.get_json(silent=True) or {} + raw_body = request.get_json(silent=True) + body = {} if raw_body is None and not request.data else require_json_object(raw_body) + reject_unknown_fields(body, {"subscription_id"}) subscription_id = body.get("subscription_id") or os.environ.get("AZURE_SUBSCRIPTION_ID") if not subscription_id: return jsonify({"error": "subscription_id is required"}), 400 + subscription_id = uuid_string(subscription_id, "subscription_id") scan_id = str(uuid.uuid4()) logger.info("Async scan triggered for subscription %s (id: %s)", subscription_id, scan_id) @@ -79,6 +86,8 @@ def trigger_scan(): {"scan_id": scan_id, "status": "pending", "message": "Scan has been queued and will start shortly."} ), 202 + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 except Exception as exc: logger.error("Critical error in trigger_scan route: %s", exc, exc_info=True) return jsonify({"error": "Critical route failure"}), 500 @@ -153,6 +162,7 @@ def enrich_scan(scan_id): rate-limited to one every ~7 seconds. """ try: + scan_id = uuid_string(scan_id, "scan_id") db = _get_db() # Check current status to avoid redundant NVD calls @@ -189,6 +199,8 @@ def enrich_scan(scan_id): } ), 202 + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 except Exception as exc: logger.error("Failed to start enrichment for scan %s: %s", scan_id, exc) return jsonify({"error": "Internal server error"}), 500 diff --git a/api/validation.py b/api/validation.py new file mode 100644 index 0000000..85a950a --- /dev/null +++ b/api/validation.py @@ -0,0 +1,137 @@ +"""Reusable allowlist and shape validation for untrusted API input.""" + +from __future__ import annotations + +import re +import uuid +from typing import Any, Iterable + + +class ValidationError(ValueError): + """Raised when a client-controlled value violates the public API contract.""" + + +RULE_ID_RE = re.compile(r"^[A-Z0-9]+(?:-[A-Z0-9]+)*$") +MODEL_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:/-]*$") + +SEVERITIES = frozenset({"CRITICAL", "HIGH", "MEDIUM", "LOW", "INFO", "INFORMATIONAL"}) +CATEGORIES = frozenset( + { + "Backup", + "Compute", + "Database", + "Identity", + "Key Vault", + "KeyVault", + "Kubernetes", + "Network", + "PostQuantum", + "Serverless", + "Storage", + "Supply Chain", + } +) + +MAX_API_KEY_LENGTH = 4096 +MAX_MODEL_LENGTH = 128 +MAX_QUESTION_LENGTH = 4000 +MAX_FINDINGS = 1000 +MAX_FINDING_TEXT_LENGTH = 8192 + + +def require_json_object(value: Any) -> dict[str, Any]: + if not isinstance(value, dict): + raise ValidationError("Request body must be a JSON object") + return value + + +def reject_unknown_fields(value: dict[str, Any], allowed: Iterable[str]) -> None: + unknown = set(value) - set(allowed) + if unknown: + raise ValidationError(f"Unsupported field: {sorted(unknown)[0]}") + + +def bounded_string( + value: Any, + field: str, + *, + minimum: int = 1, + maximum: int, + pattern: re.Pattern[str] | None = None, +) -> str: + if not isinstance(value, str): + raise ValidationError(f"{field} must be a string") + result = value.strip() + if len(result) < minimum: + raise ValidationError(f"{field} is required") + if len(result) > maximum: + raise ValidationError(f"{field} must be at most {maximum} characters") + if pattern is not None and pattern.fullmatch(result) is None: + raise ValidationError(f"{field} has an invalid format") + return result + + +def choice(value: Any, field: str, allowed: Iterable[str], *, case: str = "preserve") -> str: + result = bounded_string(value, field, maximum=128) + if case == "upper": + result = result.upper() + elif case == "lower": + result = result.lower() + allowed_set = set(allowed) + if result not in allowed_set: + raise ValidationError(f"Unsupported {field}") + return result + + +def uuid_string(value: Any, field: str) -> str: + result = bounded_string(value, field, maximum=36) + try: + parsed = uuid.UUID(result) + except (ValueError, AttributeError) as exc: + raise ValidationError(f"{field} must be a valid UUID") from exc + if str(parsed) != result.lower(): + raise ValidationError(f"{field} must use canonical UUID format") + return str(parsed) + + +def positive_integer(value: int, field: str) -> int: + if value <= 0: + raise ValidationError(f"{field} must be a positive integer") + return value + + +def findings_list(value: Any, *, required: bool = False) -> list[dict[str, Any]]: + if value is None: + if required: + raise ValidationError("findings is required") + return [] + if not isinstance(value, list): + raise ValidationError("findings must be a list") + if required and not value: + raise ValidationError("findings must not be empty") + if len(value) > MAX_FINDINGS: + raise ValidationError(f"findings must contain at most {MAX_FINDINGS} items") + + text_fields = ( + "rule_id", + "rule_name", + "title", + "severity", + "resource_name", + "description", + "remediation", + ) + validated: list[dict[str, Any]] = [] + for index, finding in enumerate(value): + if not isinstance(finding, dict): + raise ValidationError(f"findings[{index}] must be an object") + for key in text_fields: + field_value = finding.get(key) + if field_value is not None and ( + not isinstance(field_value, str) or len(field_value) > MAX_FINDING_TEXT_LENGTH + ): + raise ValidationError( + f"findings[{index}].{key} must be a string of at most {MAX_FINDING_TEXT_LENGTH} characters" + ) + validated.append(finding) + return validated diff --git a/docs/api-reference.md b/docs/api-reference.md index 82a2084..1d5fbbf 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1,13 +1,26 @@ # API Reference -The OpenShield API is a Flask app registered in `api/app.py`. All `GET` requests (including `/health` and all `/api/*` GET routes) are public — no token needed. `POST` endpoints (`/api/scans/trigger`, `/api/ai/*`) require an `Authorization: Bearer ` header signed with `JWT_SECRET`. - -The OpenShield API is a Flask app registered in `api/app.py`. +The OpenShield API is a Flask app registered in `api/app.py`. By default, every +`/api/*` route requires an `Authorization: Bearer ` header signed with +`JWT_SECRET`; only the explicitly listed health and observability endpoints are +public. Read-only API routes become public only when the deliberate demo-mode +setting is enabled. ## Authentication `/health` and `/` are always public. All other routes — including all `/api/*` GET endpoints — require an `Authorization: Bearer ` header signed with `JWT_SECRET`. +## Input limits + +- Request bodies are limited to 2 MiB. +- Scan and subscription identifiers use canonical UUID format. +- Finding filters accept only `severity`, `category`, `rule_id`, and `scan_id`; + unknown or repeated parameters return `400`. +- AI routes accept a supported provider, an API key of at most 4,096 characters, + an optional model identifier of at most 128 characters, questions of at most + 4,000 characters, and at most 1,000 finding objects. +- Full boundary details are maintained in `docs/input-validation-audit.md`. + ### Public demo mode Set `OPENSHIELD_PUBLIC_DEMO=true` to allow unauthenticated GET requests to `/api/*`. This is intended for local development and public demo dashboards where the data is not sensitive. POST endpoints (scan trigger, AI) always require a valid JWT regardless of this setting. diff --git a/docs/input-validation-audit.md b/docs/input-validation-audit.md new file mode 100644 index 0000000..a000dd9 --- /dev/null +++ b/docs/input-validation-audit.md @@ -0,0 +1,52 @@ +# Input Validation Audit + +This audit records every untrusted input boundary reviewed for OpenSSF Silver +issue #201. Validation occurs before values reach PostgreSQL, filesystem paths, +Azure/Sentinel integrations, subprocesses, or external AI providers. + +## API-wide controls + +- JSON request bodies are limited to 2 MiB by Flask. +- Protected routes require a bounded Bearer token and reject malformed tokens. +- Client request IDs accept only 1-128 letters, digits, `.`, `_`, `:` or `-`; + invalid values are replaced with a server-generated UUID before logging. +- Validation failures return a consistent `400` response without database or + provider access. Authentication failures remain `401`. + +## Boundary inventory + +| Boundary | Accepted input | Enforcement | +|---|---|---| +| `POST /api/scans/trigger` | Optional JSON object; Azure subscription UUID | Object/field allowlist and canonical UUID validation before queue insertion | +| `GET /api/scans/` | Scan UUID | Canonical UUID validation before database access | +| `POST /api/scans//enrich` | Scan UUID | Canonical UUID validation before lookup or background work | +| `GET /api/findings` | `severity`, `category`, `rule_id`, `scan_id` | Unknown/duplicate parameters rejected; severity/category allowlists; bounded rule pattern; UUID scan ID | +| Finding and playbook paths | Positive integer finding ID; stored rule ID | Positive ID check, strict rule pattern, resolved-path containment check | +| `GET /api/compliance/` | Named compliance framework | Existing framework allowlist; unknown frameworks rejected | +| AI POST routes | Provider, API key, optional model/question and findings | Field/provider allowlists; bounded strings; model pattern; maximum 1,000 object findings; bounded prompt fields | +| JWT header | HS256 Bearer token | 8 KiB header ceiling, exact prefix and PyJWT signature/expiry validation | +| Sentinel ingestion CLI | JSON file, scan ID, finding records and environment configuration | Existing regular `.json` file under 10 MiB; at most 1,000 object findings; bounded fields; severity/config format checks | +| Azure resource data | Management-plane SDK objects | Typed SDK accessors; failures preserved as unknown; no subprocess interpolation | +| Playbook selection | Rule ID derived from stored finding | Allowlisted identifier converted to a filename and constrained beneath `playbooks/cli` | +| Website media URLs | User-entered video URL | HTTPS host allowlist and embed conversion tests in `website/test_toEmbedUrl.mjs` | +| Website editor text | Titles, excerpts, names and Markdown content | Intentionally free-form client-side content; repository write still requires the operator's GitHub token and GitHub authorization | + +## Intentionally unrestricted text + +AI questions, finding descriptions, remediation text and website article content +cannot use semantic allowlists without breaking legitimate use. They are instead +type checked, length bounded, kept out of SQL/file paths, and passed only through +parameterized or fixed-destination interfaces. AI prompts explicitly treat +findings as evidence and instruct providers not to invent unsupported facts. + +## Regression evidence + +`tests/test_input_validation.py` covers malformed JSON shapes, invalid and +oversized AI fields, injection-style identifiers, unknown and duplicated query +parameters, UUID enforcement, request-ID sanitization and authorization-header +limits. `tests/test_sentinel_input_validation.py` covers file, record, severity +and field-shape rejection. Existing authentication, error-exposure, playbook and +route tests protect prior behavior. + +Re-run the audit when a route, query parameter, upload, CLI input, filesystem +selection, subprocess call, or external-provider integration is added. diff --git a/docs/openssf-silver-evidence.md b/docs/openssf-silver-evidence.md index 921d1c5..bc3cb23 100644 --- a/docs/openssf-silver-evidence.md +++ b/docs/openssf-silver-evidence.md @@ -43,6 +43,7 @@ submit the public URL or justification. | `crypto_certificate_verification` | Standard verification defaults; no disabled verification in source | | `crypto_verification_private` | Verification occurs in the TLS client before HTTP data is sent | | `hardening` | Website/frontend CSP and security headers, production fail-closed configuration | +| `input_validation` | `docs/input-validation-audit.md`, centralized validators and security regression tests | | `assurance_case` | `docs/security-assurance-case.md` | | `static_analysis_common_vulnerabilities` | CodeQL, Bandit and Semgrep | | `dynamic_analysis_unsafe` | N/A: project code is Python/JavaScript, not C/C++ | @@ -66,7 +67,6 @@ submit the public URL or justification. | `internationalization` | English-only UI; implement localization or mark Unmet with justification | | `regression_tests_added50` | Preliminary audit shows 9 of 12 fixes with test changes; verify behavioral assertions before marking Met | | `interfaces_current` | Review deprecated API warnings and document the periodic check | -| `input_validation` | Complete route-by-route allowlist audit and close discovered gaps | | `crypto_algorithm_agility` | Review JWT and signing algorithm agility; document supported migration path | | `build_repeatable` | Demonstrate repeatable frontend/release output or provide an accurate scripting-language N/A rationale | diff --git a/docs/security-requirements.md b/docs/security-requirements.md index 5254935..6dd4792 100644 --- a/docs/security-requirements.md +++ b/docs/security-requirements.md @@ -47,3 +47,6 @@ The security policy, architecture, assurance case, automated test suite, SAST, secret scanning, dependency review, SBOM generation and container scanning form the public evidence for these requirements. Known defects must be tracked and resolved through GitHub issues or private advisories as appropriate. + +The reviewed input boundaries, limits, allowlists and intentional free-text +exceptions are recorded in `docs/input-validation-audit.md`. diff --git a/sentinel/ingest.py b/sentinel/ingest.py index c642a68..c056ea8 100644 --- a/sentinel/ingest.py +++ b/sentinel/ingest.py @@ -4,14 +4,58 @@ import hmac import json import os +import re import sys import time +from pathlib import Path import requests +from api.validation import ValidationError, bounded_string, uuid_string + WORKSPACE_ID = os.environ.get("SENTINEL_WORKSPACE_ID", "") SHARED_KEY = os.environ.get("SENTINEL_SHARED_KEY", "") LOG_TYPE = os.environ.get("SENTINEL_LOG_TYPE", "OpenShieldFindings") +_LOG_TYPE_RE = re.compile(r"^[A-Za-z][A-Za-z0-9_]{0,99}$") +_MAX_INPUT_BYTES = 10 * 1024 * 1024 +_MAX_RECORDS = 1000 +_MAX_FIELD_LENGTH = 8192 + + +def _safe_text(value, field, *, maximum=_MAX_FIELD_LENGTH): + if value in (None, ""): + return "" + return bounded_string(value, field, maximum=maximum) + + +def validate_config(): + uuid_string(WORKSPACE_ID, "SENTINEL_WORKSPACE_ID") + bounded_string(SHARED_KEY, "SENTINEL_SHARED_KEY", maximum=16384) + bounded_string(LOG_TYPE, "SENTINEL_LOG_TYPE", maximum=100, pattern=_LOG_TYPE_RE) + try: + base64.b64decode(SHARED_KEY, validate=True) + except (ValueError, TypeError) as exc: + raise ValidationError("SENTINEL_SHARED_KEY must be valid base64") from exc + + +def load_findings(path_value): + path = Path(path_value).expanduser().resolve() + if path.suffix.lower() != ".json" or not path.is_file(): + raise ValidationError("input path must be an existing JSON file") + if path.stat().st_size > _MAX_INPUT_BYTES: + raise ValidationError(f"input file must be at most {_MAX_INPUT_BYTES} bytes") + try: + with path.open(encoding="utf-8") as handle: + data = json.load(handle) + except (json.JSONDecodeError, UnicodeDecodeError) as exc: + raise ValidationError("input file must contain valid UTF-8 JSON") from exc + findings = data if isinstance(data, list) else data.get("findings", []) if isinstance(data, dict) else None + if not isinstance(findings, list): + raise ValidationError("input JSON must be a findings list or contain a findings list") + if len(findings) > _MAX_RECORDS: + raise ValidationError(f"input must contain at most {_MAX_RECORDS} findings") + return findings + def build_signature(date, content_length): x_headers = f"x-ms-date:{date}" @@ -24,28 +68,38 @@ def build_signature(date, content_length): def normalise(raw, scan_id): + if not isinstance(raw, dict): + raise ValidationError("each Sentinel finding must be an object") + scan_id = bounded_string(scan_id, "scan_id", maximum=128, pattern=re.compile(r"^[A-Za-z0-9._:-]+$")) sev_map = {"CRITICAL": 4, "HIGH": 3, "MEDIUM": 2, "LOW": 1, "INFO": 0} - sev = str(raw.get("severity", "MEDIUM")).upper() + sev = _safe_text(raw.get("severity", "MEDIUM"), "severity", maximum=16).upper() + if sev not in sev_map: + raise ValidationError("severity must be CRITICAL, HIGH, MEDIUM, LOW, or INFO") + compliance = raw.get("compliance", {}) + if not isinstance(compliance, dict): + raise ValidationError("compliance must be an object") return { "ScanId": scan_id, - "FindingId": raw.get("id", ""), - "TimeGenerated": raw.get("detected_at", datetime.datetime.utcnow().isoformat() + "Z"), - "ResourceId": raw.get("resource_id", ""), - "ResourceType": raw.get("resource_type", ""), - "ResourceName": raw.get("resource_name", ""), - "SubscriptionId": raw.get("subscription_id", ""), - "ResourceGroup": raw.get("resource_group", ""), - "Region": raw.get("region", ""), - "RuleId": raw.get("rule_id", ""), - "RuleName": raw.get("rule_name", ""), + "FindingId": _safe_text("" if raw.get("id") is None else str(raw.get("id", "")), "id", maximum=128), + "TimeGenerated": _safe_text( + raw.get("detected_at", datetime.datetime.now(datetime.UTC).isoformat()), "detected_at", maximum=64 + ), + "ResourceId": _safe_text(raw.get("resource_id", ""), "resource_id"), + "ResourceType": _safe_text(raw.get("resource_type", ""), "resource_type", maximum=256), + "ResourceName": _safe_text(raw.get("resource_name", ""), "resource_name", maximum=512), + "SubscriptionId": _safe_text(raw.get("subscription_id", ""), "subscription_id", maximum=128), + "ResourceGroup": _safe_text(raw.get("resource_group", ""), "resource_group", maximum=256), + "Region": _safe_text(raw.get("region", ""), "region", maximum=128), + "RuleId": _safe_text(raw.get("rule_id", ""), "rule_id", maximum=64), + "RuleName": _safe_text(raw.get("rule_name", ""), "rule_name", maximum=512), "Severity": sev.capitalize(), "SeverityScore": sev_map.get(sev, 0), - "Description": raw.get("description", ""), - "Remediation": raw.get("remediation", ""), - "CisControl": raw.get("compliance", {}).get("cis", ""), - "NistControl": raw.get("compliance", {}).get("nist", ""), + "Description": _safe_text(raw.get("description", ""), "description"), + "Remediation": _safe_text(raw.get("remediation", ""), "remediation"), + "CisControl": _safe_text(compliance.get("cis", ""), "compliance.cis", maximum=128), + "NistControl": _safe_text(compliance.get("nist", ""), "compliance.nist", maximum=128), "Source": "OpenShield", - "ToolVersion": raw.get("tool_version", "0.1.0"), + "ToolVersion": _safe_text(raw.get("tool_version", "0.1.0"), "tool_version", maximum=64), } @@ -79,9 +133,8 @@ def main(): path = sys.argv[1] if len(sys.argv) > 1 else "scanner/output/test_findings.json" scan_id = sys.argv[2] if len(sys.argv) > 2 else datetime.datetime.utcnow().strftime("scan-%Y%m%d-%H%M") print(f"[INFO] Scan ID: {scan_id}") - with open(path) as f: - data = json.load(f) - findings = data if isinstance(data, list) else data.get("findings", []) + validate_config() + findings = load_findings(path) print(f"[INFO] Loaded {len(findings)} findings") records = [normalise(f, scan_id) for f in findings] send(records) diff --git a/tests/test_error_exposure.py b/tests/test_error_exposure.py index 2322c78..2610ffd 100644 --- a/tests/test_error_exposure.py +++ b/tests/test_error_exposure.py @@ -41,7 +41,7 @@ def test_list_scans_error_does_not_leak_exception(client, auth_headers): def test_get_scan_status_error_does_not_leak_exception(client, auth_headers): with patch.object(scans_route, "_get_db", return_value=_raising_db("get_scan")): - resp = client.get("/api/scans/some-id", headers=auth_headers) + resp = client.get("/api/scans/00000000-0000-0000-0000-000000000001", headers=auth_headers) _assert_no_leak(resp, 500) diff --git a/tests/test_input_validation.py b/tests/test_input_validation.py new file mode 100644 index 0000000..e5a9d51 --- /dev/null +++ b/tests/test_input_validation.py @@ -0,0 +1,136 @@ +"""Security regression tests for public input boundaries tracked by #201.""" + +from unittest.mock import MagicMock, patch + +import pytest + +import api.routes.findings as findings_route +import api.routes.scans as scans_route +from api.validation import MAX_API_KEY_LENGTH, MAX_FINDINGS, MAX_QUESTION_LENGTH + +_SCAN_ID = "00000000-0000-0000-0000-000000000001" +_SUBSCRIPTION_ID = "00000000-0000-0000-0000-000000000002" + + +@pytest.mark.parametrize( + "path", + [ + "/api/scans/not-a-uuid", + "/api/scans/not-a-uuid/enrich", + ], +) +def test_scan_paths_reject_non_uuid_before_database(client, auth_headers, path): + with patch.object(scans_route, "_get_db") as get_db: + response = ( + client.get(path, headers=auth_headers) + if not path.endswith("/enrich") + else client.post(path, headers=auth_headers) + ) + assert response.status_code == 400 + get_db.assert_not_called() + + +def test_trigger_rejects_non_object_json(client, auth_headers): + with patch.object(scans_route, "_get_db") as get_db: + response = client.post("/api/scans/trigger", json=[_SUBSCRIPTION_ID], headers=auth_headers) + assert response.status_code == 400 + get_db.assert_not_called() + + +def test_trigger_rejects_unknown_json_field(client, auth_headers): + with patch.object(scans_route, "_get_db") as get_db: + response = client.post( + "/api/scans/trigger", + json={"subscription_id": _SUBSCRIPTION_ID, "command": "ignored-before-fix"}, + headers=auth_headers, + ) + assert response.status_code == 400 + get_db.assert_not_called() + + +def test_trigger_rejects_malformed_subscription_id(client, auth_headers): + with patch.object(scans_route, "_get_db") as get_db: + response = client.post("/api/scans/trigger", json={"subscription_id": "../../etc/passwd"}, headers=auth_headers) + assert response.status_code == 400 + get_db.assert_not_called() + + +def test_trigger_accepts_canonical_subscription_uuid(client, auth_headers): + db = MagicMock() + with patch.object(scans_route, "_get_db", return_value=db): + response = client.post("/api/scans/trigger", json={"subscription_id": _SUBSCRIPTION_ID}, headers=auth_headers) + assert response.status_code == 202 + assert db.create_pending_scan.call_args.args[1] == _SUBSCRIPTION_ID + + +@pytest.mark.parametrize( + "query", + [ + "severity=INVALID", + "category=Unknown", + "rule_id=../../secret", + "scan_id=not-a-uuid", + "limit=1000000", + "severity=HIGH&severity=LOW", + ], +) +def test_finding_filters_reject_values_outside_contract(client, auth_headers, query): + with patch.object(findings_route, "_get_db") as get_db: + response = client.get(f"/api/findings?{query}", headers=auth_headers) + assert response.status_code == 400 + get_db.assert_not_called() + + +def test_finding_filters_are_normalised_before_database(client, auth_headers): + db = MagicMock() + db.get_findings.return_value = [] + with patch.object(findings_route, "_get_db", return_value=db): + response = client.get( + f"/api/findings?severity=high&category=Network&rule_id=az-net-001&scan_id={_SCAN_ID}", + headers=auth_headers, + ) + assert response.status_code == 200 + db.get_findings.assert_called_once_with( + {"severity": "HIGH", "category": "Network", "rule_id": "AZ-NET-001", "scan_id": _SCAN_ID} + ) + + +@pytest.mark.parametrize( + "payload", + [ + [], + {"provider": ["groq"], "api_key": "secret", "findings": [{}]}, + {"provider": "groq", "api_key": "secret", "findings": [{}], "unexpected": True}, + {"provider": "groq", "api_key": "x" * (MAX_API_KEY_LENGTH + 1), "findings": [{}]}, + {"provider": "groq", "api_key": "secret", "model": "../model", "findings": [{}]}, + {"provider": "groq", "api_key": "secret", "findings": ["not-an-object"]}, + {"provider": "groq", "api_key": "secret", "findings": [{}] * (MAX_FINDINGS + 1)}, + { + "provider": "groq", + "api_key": "secret", + "findings": [{}], + "question": "x" * (MAX_QUESTION_LENGTH + 1), + }, + ], +) +def test_ai_insights_rejects_invalid_shapes_and_limits(client, auth_headers, payload): + response = client.post("/api/ai/insights", json=payload, headers=auth_headers) + assert response.status_code == 400 + + +def test_invalid_request_id_is_replaced(client): + supplied = "contains spaces " + ("x" * 200) + response = client.get("/health", headers={"X-Request-ID": supplied}) + returned = response.headers["X-Request-ID"] + assert returned != supplied + assert len(returned) == 36 + + +def test_safe_request_id_is_preserved(client): + response = client.get("/health", headers={"X-Request-ID": "client-request_123"}) + assert response.headers["X-Request-ID"] == "client-request_123" + + +def test_oversized_authorization_header_is_rejected(client): + response = client.get("/api/findings", headers={"Authorization": "Bearer " + ("x" * 9000)}) + assert response.status_code == 401 diff --git a/tests/test_scans_enrich.py b/tests/test_scans_enrich.py index c57b184..44c42c8 100644 --- a/tests/test_scans_enrich.py +++ b/tests/test_scans_enrich.py @@ -4,6 +4,8 @@ import api.routes.scans as scans_route +_SCAN_ID = "00000000-0000-0000-0000-000000000001" + def _mock_db(current_scan=None, findings=None): db = MagicMock() @@ -30,7 +32,7 @@ def start(self): def test_enrich_returns_202_and_schedules_background_thread(client, auth_headers, monkeypatch): monkeypatch.setenv("DATABASE_URL", "postgresql://mock/mock") - scan = {"scan_id": "scan-1", "cve_enrichment_status": "PENDING"} + scan = {"scan_id": _SCAN_ID, "cve_enrichment_status": "PENDING"} findings = [{"id": 1, "rule_id": "AZ-STOR-001"}] db = _mock_db(current_scan=scan, findings=findings) @@ -38,36 +40,36 @@ def test_enrich_returns_202_and_schedules_background_thread(client, auth_headers patch.object(scans_route, "_get_db", return_value=db), patch.object(scans_route.threading, "Thread", _FakeThread), ): - resp = client.post("/api/scans/scan-1/enrich", headers=auth_headers) + resp = client.post(f"/api/scans/{_SCAN_ID}/enrich", headers=auth_headers) assert resp.status_code == 202 body = resp.get_json() assert body["status"] == "ENRICHING" - db.update_scan_enrichment_status.assert_called_once_with("scan-1", "ENRICHING") + db.update_scan_enrichment_status.assert_called_once_with(_SCAN_ID, "ENRICHING") thread = _FakeThread.last_instance assert thread.started is True assert thread.daemon is True assert thread.target is scans_route._run_enrichment_in_background - assert thread.args[0] == "scan-1" + assert thread.args[0] == _SCAN_ID assert thread.args[1] == findings def test_enrich_already_completed_returns_200(client, auth_headers): - scan = {"scan_id": "scan-1", "cve_enrichment_status": "COMPLETED"} + scan = {"scan_id": _SCAN_ID, "cve_enrichment_status": "COMPLETED"} db = _mock_db(current_scan=scan) with patch.object(scans_route, "_get_db", return_value=db): - resp = client.post("/api/scans/scan-1/enrich", headers=auth_headers) + resp = client.post(f"/api/scans/{_SCAN_ID}/enrich", headers=auth_headers) assert resp.status_code == 200 assert "already enriched" in resp.get_json()["message"] def test_enrich_already_in_progress_returns_202(client, auth_headers): - scan = {"scan_id": "scan-1", "cve_enrichment_status": "ENRICHING"} + scan = {"scan_id": _SCAN_ID, "cve_enrichment_status": "ENRICHING"} db = _mock_db(current_scan=scan) with patch.object(scans_route, "_get_db", return_value=db): - resp = client.post("/api/scans/scan-1/enrich", headers=auth_headers) + resp = client.post(f"/api/scans/{_SCAN_ID}/enrich", headers=auth_headers) assert resp.status_code == 202 assert "in progress" in resp.get_json()["message"] @@ -75,15 +77,15 @@ def test_enrich_already_in_progress_returns_202(client, auth_headers): def test_enrich_missing_scan_returns_404(client, auth_headers): db = _mock_db(current_scan=None) with patch.object(scans_route, "_get_db", return_value=db): - resp = client.post("/api/scans/missing-scan/enrich", headers=auth_headers) + resp = client.post(f"/api/scans/{_SCAN_ID}/enrich", headers=auth_headers) assert resp.status_code == 404 def test_enrich_no_findings_returns_404(client, auth_headers): - scan = {"scan_id": "scan-1", "cve_enrichment_status": "PENDING"} + scan = {"scan_id": _SCAN_ID, "cve_enrichment_status": "PENDING"} db = _mock_db(current_scan=scan, findings=[]) with patch.object(scans_route, "_get_db", return_value=db): - resp = client.post("/api/scans/scan-1/enrich", headers=auth_headers) + resp = client.post(f"/api/scans/{_SCAN_ID}/enrich", headers=auth_headers) assert resp.status_code == 404 diff --git a/tests/test_sentinel_input_validation.py b/tests/test_sentinel_input_validation.py new file mode 100644 index 0000000..5da1747 --- /dev/null +++ b/tests/test_sentinel_input_validation.py @@ -0,0 +1,38 @@ +"""Validation tests for the Sentinel JSON ingestion boundary.""" + +import json + +import pytest + +from api.validation import ValidationError +from sentinel.ingest import load_findings, normalise + + +def test_load_findings_rejects_non_json_path(tmp_path): + path = tmp_path / "findings.txt" + path.write_text("[]", encoding="utf-8") + with pytest.raises(ValidationError, match="JSON file"): + load_findings(path) + + +def test_load_findings_rejects_wrong_shape(tmp_path): + path = tmp_path / "findings.json" + path.write_text(json.dumps({"findings": "not-a-list"}), encoding="utf-8") + with pytest.raises(ValidationError, match="findings list"): + load_findings(path) + + +def test_normalise_rejects_non_object_and_invalid_severity(): + with pytest.raises(ValidationError, match="object"): + normalise("finding", "scan-1") + with pytest.raises(ValidationError, match="severity"): + normalise({"severity": "urgent"}, "scan-1") + + +def test_normalise_accepts_bounded_finding(): + record = normalise( + {"id": 1, "severity": "HIGH", "rule_id": "AZ-NET-001", "compliance": {"cis": "1.1"}}, + "scan-1", + ) + assert record["Severity"] == "High" + assert record["RuleId"] == "AZ-NET-001" From d8469a28823b383889e77418af2c0254c078d888 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 6 Aug 2026 12:52:50 +0100 Subject: [PATCH 2/2] fix(api): avoid reflecting validation exceptions Signed-off-by: Tanvir Farhad --- api/routes/ai.py | 25 +++++++++++++------------ api/routes/findings.py | 13 +++++++------ api/routes/scans.py | 20 +++++++++++++------- api/validation.py | 3 +++ docs/input-validation-audit.md | 5 +++-- tests/test_input_validation.py | 4 +++- 6 files changed, 42 insertions(+), 28 deletions(-) diff --git a/api/routes/ai.py b/api/routes/ai.py index 22ac101..7f9673c 100644 --- a/api/routes/ai.py +++ b/api/routes/ai.py @@ -13,6 +13,7 @@ MAX_MODEL_LENGTH, MAX_QUESTION_LENGTH, MODEL_RE, + VALIDATION_ERROR_MESSAGE, ValidationError, bounded_string, choice, @@ -170,8 +171,8 @@ def _read_request(): if ".." in body["model"]: raise ValidationError("model has an invalid format") return body, None - except ValidationError as exc: - return None, (jsonify({"error": str(exc)}), 400) + except ValidationError: + return None, (jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400) _AI_ERROR_MESSAGES = { @@ -209,8 +210,8 @@ def insights(): raise ValidationError("question must be a string") if data["question"].strip(): question = bounded_string(data["question"], "question", maximum=MAX_QUESTION_LENGTH) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 sorted_findings = sorted(findings, key=severity_rank, reverse=True) @@ -246,8 +247,8 @@ def ai_summary(): return error try: findings = findings_list(body.get("findings")) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 findings_text = _findings_to_text(findings) try: @@ -286,8 +287,8 @@ def ai_prioritise(): return error try: findings = findings_list(body.get("findings")) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 findings_text = _findings_to_text(findings) try: @@ -334,8 +335,8 @@ def ai_ask(): try: question = bounded_string(body.get("question"), "question", maximum=MAX_QUESTION_LENGTH) findings = findings_list(body.get("findings")) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 try: context, sources = _context_for(question) @@ -376,8 +377,8 @@ def ai_threat_simulation(): return error try: findings = findings_list(body.get("findings"), required=True) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 findings_text = _findings_to_text(findings) try: diff --git a/api/routes/findings.py b/api/routes/findings.py index 1b1d7e4..0343453 100644 --- a/api/routes/findings.py +++ b/api/routes/findings.py @@ -10,6 +10,7 @@ CATEGORIES, RULE_ID_RE, SEVERITIES, + VALIDATION_ERROR_MESSAGE, ValidationError, bounded_string, choice, @@ -66,8 +67,8 @@ def list_findings(): db = _get_db() findings = db.get_findings(filters) return jsonify({"count": len(findings), "findings": findings}) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 except Exception as exc: logger.error("Failed to list findings: %s", exc) return jsonify({"error": "Failed to retrieve findings"}), 500 @@ -83,8 +84,8 @@ def get_finding(finding_id: int): if not finding: return jsonify({"error": "Finding not found"}), 404 return jsonify(finding) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 except Exception as exc: logger.error("Failed to get finding %d: %s", finding_id, exc) return jsonify({"error": "Database error"}), 500 @@ -155,8 +156,8 @@ def get_playbook(finding_id: int): } ) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 except Exception as exc: logger.error("Failed to get playbook for finding %d: %s", finding_id, exc) return jsonify({"error": "Failed to retrieve playbook"}), 500 diff --git a/api/routes/scans.py b/api/routes/scans.py index f2a8254..6147b0f 100644 --- a/api/routes/scans.py +++ b/api/routes/scans.py @@ -7,7 +7,13 @@ from flask import Blueprint, g, jsonify, request from api.models.finding import DatabaseManager -from api.validation import ValidationError, reject_unknown_fields, require_json_object, uuid_string +from api.validation import ( + VALIDATION_ERROR_MESSAGE, + ValidationError, + reject_unknown_fields, + require_json_object, + uuid_string, +) from scanner.cve_correlator import enrich_findings scans_bp = Blueprint("scans", __name__) @@ -46,8 +52,8 @@ def get_scan_status(scan_id): if not scan: return jsonify({"error": "Scan not found"}), 404 return jsonify(scan) - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 except Exception as exc: logger.error("Failed to get scan status: %s", exc) return jsonify({"error": "Database error"}), 500 @@ -86,8 +92,8 @@ def trigger_scan(): {"scan_id": scan_id, "status": "pending", "message": "Scan has been queued and will start shortly."} ), 202 - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 except Exception as exc: logger.error("Critical error in trigger_scan route: %s", exc, exc_info=True) return jsonify({"error": "Critical route failure"}), 500 @@ -199,8 +205,8 @@ def enrich_scan(scan_id): } ), 202 - except ValidationError as exc: - return jsonify({"error": str(exc)}), 400 + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 except Exception as exc: logger.error("Failed to start enrichment for scan %s: %s", scan_id, exc) return jsonify({"error": "Internal server error"}), 500 diff --git a/api/validation.py b/api/validation.py index 85a950a..b971449 100644 --- a/api/validation.py +++ b/api/validation.py @@ -11,6 +11,9 @@ class ValidationError(ValueError): """Raised when a client-controlled value violates the public API contract.""" +VALIDATION_ERROR_MESSAGE = "Invalid request parameters" + + RULE_ID_RE = re.compile(r"^[A-Z0-9]+(?:-[A-Z0-9]+)*$") MODEL_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._:/-]*$") diff --git a/docs/input-validation-audit.md b/docs/input-validation-audit.md index a000dd9..bec41a8 100644 --- a/docs/input-validation-audit.md +++ b/docs/input-validation-audit.md @@ -10,8 +10,9 @@ Azure/Sentinel integrations, subprocesses, or external AI providers. - Protected routes require a bounded Bearer token and reject malformed tokens. - Client request IDs accept only 1-128 letters, digits, `.`, `_`, `:` or `-`; invalid values are replaced with a server-generated UUID before logging. -- Validation failures return a consistent `400` response without database or - provider access. Authentication failures remain `401`. +- Validation failures return a fixed `400` response without reflecting + exception details and without database or provider access. Authentication + failures remain `401`. ## Boundary inventory diff --git a/tests/test_input_validation.py b/tests/test_input_validation.py index e5a9d51..4929685 100644 --- a/tests/test_input_validation.py +++ b/tests/test_input_validation.py @@ -6,7 +6,7 @@ import api.routes.findings as findings_route import api.routes.scans as scans_route -from api.validation import MAX_API_KEY_LENGTH, MAX_FINDINGS, MAX_QUESTION_LENGTH +from api.validation import MAX_API_KEY_LENGTH, MAX_FINDINGS, MAX_QUESTION_LENGTH, VALIDATION_ERROR_MESSAGE _SCAN_ID = "00000000-0000-0000-0000-000000000001" _SUBSCRIPTION_ID = "00000000-0000-0000-0000-000000000002" @@ -52,6 +52,8 @@ def test_trigger_rejects_malformed_subscription_id(client, auth_headers): with patch.object(scans_route, "_get_db") as get_db: response = client.post("/api/scans/trigger", json={"subscription_id": "../../etc/passwd"}, headers=auth_headers) assert response.status_code == 400 + assert response.get_json() == {"error": VALIDATION_ERROR_MESSAGE} + assert "../../etc/passwd" not in response.get_data(as_text=True) get_db.assert_not_called()