diff --git a/dependencies/auth.py b/dependencies/auth.py index b721a092..c6a58b29 100644 --- a/dependencies/auth.py +++ b/dependencies/auth.py @@ -111,7 +111,10 @@ async def get_current_user( email_verified = user.email_verified if user else False structlog.contextvars.bind_contextvars( - user_id=str(key.user_id), auth_method="api_key" + user_id=str(key.user_id), + auth_method="api_key", + key_id=str(key.id), + key_prefix=key.token_prefix, ) return CurrentUser( user_id=key.user_id, diff --git a/infrastructure/logging.py b/infrastructure/logging.py index 0a377d7f..e692fbde 100644 --- a/infrastructure/logging.py +++ b/infrastructure/logging.py @@ -54,6 +54,18 @@ "key", } +# Field names that trip the substring heuristic in redact_sensitive_fields +# but carry no secret material: derived booleans, display prefixes, and +# identifiers. These pass through unredacted. +SAFE_FIELDS = { + "has_password", + "password_protected", + "key_id", + "key_prefix", + "token_prefix", + "query_keys", +} + # --------------------------------------------------------------------------- # IP hashing @@ -99,6 +111,8 @@ def redact_sensitive_fields( ) -> EventDict: """Redact sensitive fields from logs.""" for key in list(event_dict.keys()): + if key.lower() in SAFE_FIELDS: + continue if ( key.lower() in REDACTED_FIELDS or any( diff --git a/middleware/logging.py b/middleware/logging.py index ec567769..5a096edd 100644 --- a/middleware/logging.py +++ b/middleware/logging.py @@ -7,6 +7,7 @@ from __future__ import annotations +import re import time import uuid @@ -23,17 +24,37 @@ # Paths to skip detailed logging (high-volume, low-value) _SKIP_PATHS = frozenset({"/health", "/favicon.ico"}) +# X-Spoo-Client value: "" or "/", e.g. "snap/2.1.0". +# First-party clients send dashboard/landing/snap/raycast/cli/bot; anything +# not matching the shape is treated as absent rather than rejected. +_CLIENT_TAG_RE = re.compile(r"^([a-z0-9_-]{1,32})(?:/([A-Za-z0-9._-]{1,16}))?$") + + +def _client_tag(request: Request) -> tuple[str | None, str | None]: + """Parse the X-Spoo-Client header into (client, client_version).""" + match = _CLIENT_TAG_RE.match(request.headers.get("x-spoo-client", "").strip()) + if match is None: + return None, None + return match.group(1), match.group(2) + def _auth_kind(request: Request) -> str: - """Coarse "who is calling" tag without leaking creds.""" + """Coarse "who is calling" tag without leaking creds. + + Mirrors the token extraction in dependencies/auth.py (case-insensitive + scheme, stripped token) so the tag can't disagree with how the request + actually authenticates. + """ auth = request.headers.get("authorization", "") - if auth.startswith("Bearer "): - token = auth[7:] + if auth.lower().startswith("bearer "): + token = auth.split(" ", 1)[1].strip() if token.startswith("spoo_"): return "api_key" if token.count(".") == 2: return "jwt" return "bearer_other" + if request.cookies.get("access_token"): + return "jwt_cookie" if request.cookies.get("session"): return "session_cookie" return "anonymous" @@ -76,6 +97,7 @@ async def dispatch( referrer = request.headers.get("referer") ip_hash = hash_ip(get_client_ip(request)) auth_kind = _auth_kind(request) + client, client_version = _client_tag(request) cf_ray = request.headers.get("cf-ray", "") # cf-ray format: <12-hex-id>-<3-letter-pop>, e.g. "9f80a96e7a07f934-SIN" @@ -101,6 +123,8 @@ async def dispatch( referrer=referrer, user_agent=ua[:200] if ua else None, auth_kind=auth_kind, + client=client, + client_version=client_version, cf_ray=cf_ray or None, cf_pop=cf_pop, query_keys=query_keys, diff --git a/middleware/security.py b/middleware/security.py index 5776cc20..3b005e2d 100644 --- a/middleware/security.py +++ b/middleware/security.py @@ -18,7 +18,7 @@ _PUBLIC_PREFIXES = ("/api/v1", "/auth/device", "/stats", "/export", "/metric") _ALLOWED_METHODS = "GET, POST, PUT, PATCH, DELETE, OPTIONS" -_ALLOWED_HEADERS = "Authorization, Content-Type, Accept, X-Request-ID" +_ALLOWED_HEADERS = "Authorization, Content-Type, Accept, X-Request-ID, X-Spoo-Client" def _classify_path(path: str) -> str: diff --git a/tests/unit/infrastructure/test_logging_redaction.py b/tests/unit/infrastructure/test_logging_redaction.py new file mode 100644 index 00000000..40338f3d --- /dev/null +++ b/tests/unit/infrastructure/test_logging_redaction.py @@ -0,0 +1,73 @@ +"""Unit tests for redact_sensitive_fields — secrets scrubbed, analytics fields kept.""" + +from __future__ import annotations + +import pytest + +from infrastructure.logging import redact_sensitive_fields + +REDACTED = "***REDACTED***" + + +def _redact(event_dict: dict) -> dict: + return redact_sensitive_fields(None, "info", dict(event_dict)) + + +@pytest.mark.parametrize( + "field", + [ + "password", + "password_hash", + "token", + "api_key", + "authorization", + "cookie", + "refresh_token", + "access_token", + "secret", + "key", + # substring heuristic + "jwt_secret", + "client_secret", + "device_token", + "raw_password", + ], +) +def test_secret_fields_redacted(field: str): + assert _redact({field: "s3cr3t"})[field] == REDACTED + + +@pytest.mark.parametrize( + ("field", "value"), + [ + ("has_password", True), + ("password_protected", False), + ("key_id", "6a065366720e95786b0608fb"), + ("key_prefix", "abcd1234"), + ("token_prefix", "abcd1234"), + ("query_keys", ["password", "alias"]), + ], +) +def test_safe_fields_pass_through(field: str, value): + assert _redact({field: value})[field] == value + + +def test_structural_keys_untouched(): + event = {"level": "info", "event": "url_created", "timestamp": "t", "logger": "x"} + assert _redact(event) == event + + +def test_mixed_event_dict(): + out = _redact( + { + "event": "api_key_created", + "key_id": "abc", + "key_prefix": "abcd1234", + "api_key": "spoo_raw", + "has_password": True, + } + ) + assert out["key_id"] == "abc" + assert out["key_prefix"] == "abcd1234" + assert out["has_password"] is True + assert out["api_key"] == REDACTED diff --git a/tests/unit/middleware/test_logging.py b/tests/unit/middleware/test_logging.py new file mode 100644 index 00000000..0d712b81 --- /dev/null +++ b/tests/unit/middleware/test_logging.py @@ -0,0 +1,118 @@ +"""Unit tests for request logging middleware helpers (_client_tag, _auth_kind).""" + +from __future__ import annotations + +import pytest +from starlette.requests import Request + +from middleware.logging import _auth_kind, _client_tag + + +def _request(headers: dict[str, str] | None = None) -> Request: + raw_headers = [(k.lower().encode(), v.encode()) for k, v in (headers or {}).items()] + scope = { + "type": "http", + "method": "GET", + "path": "/", + "headers": raw_headers, + "query_string": b"", + } + return Request(scope) + + +# ── _client_tag ────────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ("dashboard", ("dashboard", None)), + ("landing", ("landing", None)), + ("snap/2.1.0", ("snap", "2.1.0")), + ("cli/0.3.0-beta.1", ("cli", "0.3.0-beta.1")), + ("bot", ("bot", None)), + ], +) +def test_client_tag_valid(value: str, expected: tuple): + assert _client_tag(_request({"X-Spoo-Client": value})) == expected + + +@pytest.mark.parametrize( + "value", + [ + "", + "Dashboard", # uppercase slug + "a" * 33, # slug too long + "snap/" + "1" * 17, # version too long + "snap/2.1.0/extra", + "sn ap", + "snap;DROP", + ], +) +def test_client_tag_invalid_treated_as_absent(value: str): + assert _client_tag(_request({"X-Spoo-Client": value})) == (None, None) + + +def test_client_tag_missing_header(): + assert _client_tag(_request()) == (None, None) + + +def test_client_tag_strips_whitespace(): + assert _client_tag(_request({"X-Spoo-Client": " raycast "})) == ( + "raycast", + None, + ) + + +# ── _auth_kind ─────────────────────────────────────────────────────────────── + + +def test_auth_kind_api_key(): + req = _request({"Authorization": "Bearer spoo_abc123"}) + assert _auth_kind(req) == "api_key" + + +def test_auth_kind_jwt_bearer(): + req = _request({"Authorization": "Bearer aaa.bbb.ccc"}) + assert _auth_kind(req) == "jwt" + + +def test_auth_kind_bearer_other(): + req = _request({"Authorization": "Bearer something-else"}) + assert _auth_kind(req) == "bearer_other" + + +def test_auth_kind_access_token_cookie(): + req = _request({"Cookie": "access_token=aaa.bbb.ccc"}) + assert _auth_kind(req) == "jwt_cookie" + + +def test_auth_kind_legacy_session_cookie(): + req = _request({"Cookie": "session=xyz"}) + assert _auth_kind(req) == "session_cookie" + + +def test_auth_kind_access_token_beats_session_cookie(): + req = _request({"Cookie": "session=xyz; access_token=aaa.bbb.ccc"}) + assert _auth_kind(req) == "jwt_cookie" + + +def test_auth_kind_bearer_beats_cookie(): + req = _request( + {"Authorization": "Bearer spoo_abc", "Cookie": "access_token=aaa.bbb.ccc"} + ) + assert _auth_kind(req) == "api_key" + + +def test_auth_kind_anonymous(): + assert _auth_kind(_request()) == "anonymous" + + +def test_auth_kind_lowercase_bearer_scheme(): + # Must match dependencies/auth.py, which accepts the scheme + # case-insensitively; a lowercase bearer API key with a session + # cookie present must not be classified as jwt_cookie. + req = _request( + {"Authorization": "bearer spoo_abc123", "Cookie": "access_token=a.b.c"} + ) + assert _auth_kind(req) == "api_key"