Skip to content

Commit 7cca790

Browse files
committed
fix: address CodeQL security findings
1 parent e8f62c3 commit 7cca790

22 files changed

Lines changed: 507 additions & 79 deletions

README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ uv run --python 3.11 python -m alembic upgrade heads
8383
uv run --python 3.11 python scripts/init_user.py --email you@example.com
8484
```
8585

86+
Pass `--api-key-output-file ./standalone-api-key.txt` if you need the generated
87+
plaintext key written to a local `0600` file. The default console output only
88+
reports that the credential was created.
89+
8690
If you plan to use the dashboard, start the combined self-hosted stack and
8791
register through the dashboard instead of using `scripts/init_user.py`.
8892

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
"""key api key hashes
2+
3+
Revision ID: f6a7b8c9d0e1
4+
Revises: e5f6a7b8c9d0
5+
Create Date: 2026-05-01 05:55:00.000000
6+
7+
"""
8+
9+
from typing import Sequence, Union
10+
11+
import sqlalchemy as sa
12+
from alembic import op
13+
14+
15+
revision: str = "f6a7b8c9d0e1"
16+
down_revision: Union[str, Sequence[str], None] = "e5f6a7b8c9d0"
17+
branch_labels: Union[str, Sequence[str], None] = None
18+
depends_on: Union[str, Sequence[str], None] = None
19+
20+
21+
def upgrade() -> None:
22+
op.execute("UPDATE api_keys SET is_active = false")
23+
op.add_column(
24+
"api_keys",
25+
sa.Column("hash_version", sa.String(length=16), nullable=False, server_default="hmac-v1"),
26+
)
27+
op.alter_column("api_keys", "hash_version", server_default=None)
28+
29+
30+
def downgrade() -> None:
31+
op.drop_column("api_keys", "hash_version")

apps/api/app/api/v1/health.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,23 @@
1717
@router.get("/database/health")
1818
async def check_database_health():
1919
"""Check database health status."""
20-
return await get_database_health()
20+
health = await get_database_health()
21+
if "error" in health:
22+
return {
23+
"status": health.get("status", "unhealthy"),
24+
"error": "Database health check failed",
25+
"last_check": health.get("last_check"),
26+
}
27+
return health
2128

2229

2330
@router.get("/database/info")
2431
async def get_database_information():
2532
"""Return database connection information."""
26-
return await get_database_info()
33+
info = await get_database_info()
34+
if "error" in info:
35+
return {"error": "Database information unavailable"}
36+
return info
2737

2838

2939
@router.get("/database/performance")

apps/api/app/api/v1/routes/s3_events.py

Lines changed: 81 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
import base64
66
import json
77
import os
8+
import socket
89
from typing import Any, Dict
910

1011
import aiohttp
12+
from aiohttp.abc import AbstractResolver
1113
from app.repositories.job_repository import JobRepository
1214
from app.services.knowledge.kb_orchestrator import KBOrchestrator
1315
from app.services.state_machine import JobStateMachine
@@ -19,9 +21,13 @@
1921
from shared.core.state_machine.states import JobStatus
2022
from shared.models.schemas.oss_event import OSSEvent
2123
from shared.models.schemas.s3_event import S3Event
24+
from shared.services.webhook.validator import validate_webhook_url_async
25+
from shared.utils.url_security import SafePublicHTTPURL
2226

2327
router = APIRouter(tags=["Internal"])
2428

29+
SNS_SUBSCRIPTION_TIMEOUT_SECONDS = 10
30+
2531

2632
def verify_sns_signature(request_body: bytes, signature: str, message: str) -> bool:
2733
"""
@@ -207,22 +213,7 @@ async def handle_sns_event(body: bytes):
207213
if subscribe_url:
208214
logger.info(f"SNS subscription confirmation URL: {subscribe_url}")
209215
# Visit the URL to confirm the subscription.
210-
try:
211-
async with aiohttp.ClientSession() as session:
212-
async with session.get(subscribe_url) as response:
213-
if response.status == 200:
214-
logger.info("SNS subscription confirmed successfully")
215-
return {"message": "SNS subscription confirmed"}
216-
else:
217-
logger.error(
218-
f"SNS subscription confirmation failed, status={response.status}"
219-
)
220-
return {
221-
"message": "SNS subscription confirmation failed"
222-
}
223-
except Exception as e:
224-
logger.error(f"Failed to reach the SNS confirmation URL: {e}")
225-
return {"message": "SNS subscription confirmation failed"}
216+
return await confirm_sns_subscription(subscribe_url)
226217
else:
227218
logger.warning(
228219
"SNS subscription confirmation did not include SubscribeURL"
@@ -274,6 +265,80 @@ async def handle_sns_event(body: bytes):
274265
raise
275266

276267

268+
async def confirm_sns_subscription(subscribe_url: str) -> dict[str, str]:
269+
"""Confirm an SNS subscription after SSRF validation and IP pinning."""
270+
validation = await validate_webhook_url_async(subscribe_url)
271+
if not validation.is_valid:
272+
logger.warning(
273+
f"SNS subscription confirmation URL failed validation: {validation.error_message}"
274+
)
275+
return {"message": "SNS subscription confirmation failed"}
276+
277+
if not validation.validated_ip:
278+
logger.warning("SNS subscription confirmation URL validation returned no IP")
279+
return {"message": "SNS subscription confirmation failed"}
280+
281+
try:
282+
validated_subscribe_url = SafePublicHTTPURL(subscribe_url)
283+
connector = aiohttp.TCPConnector(
284+
resolver=_PinnedSNSResolver(validation.validated_ip),
285+
)
286+
timeout = aiohttp.ClientTimeout(total=SNS_SUBSCRIPTION_TIMEOUT_SECONDS)
287+
async with aiohttp.ClientSession(
288+
connector=connector,
289+
timeout=timeout,
290+
) as session:
291+
async with session.get(
292+
validated_subscribe_url,
293+
allow_redirects=False,
294+
) as response:
295+
if response.status == 200:
296+
logger.info("SNS subscription confirmed successfully")
297+
return {"message": "SNS subscription confirmed"}
298+
299+
if 300 <= response.status < 400:
300+
logger.warning(
301+
f"SNS subscription confirmation redirect blocked, status={response.status}"
302+
)
303+
else:
304+
logger.error(
305+
f"SNS subscription confirmation failed, status={response.status}"
306+
)
307+
return {"message": "SNS subscription confirmation failed"}
308+
except Exception as e:
309+
logger.error(f"Failed to reach the SNS confirmation URL: {e}")
310+
return {"message": "SNS subscription confirmation failed"}
311+
312+
313+
class _PinnedSNSResolver(AbstractResolver):
314+
"""Resolver that pins SNS confirmation to a pre-validated public IP."""
315+
316+
def __init__(self, pinned_ip: str) -> None:
317+
self.pinned_ip = pinned_ip
318+
319+
async def resolve(
320+
self,
321+
host: str,
322+
port: int = 0,
323+
family: int = socket.AF_INET,
324+
) -> list[dict[str, Any]]:
325+
parsed_ip = self.pinned_ip
326+
pinned_family = socket.AF_INET6 if ":" in parsed_ip else socket.AF_INET
327+
return [
328+
{
329+
"hostname": host,
330+
"host": parsed_ip,
331+
"port": port,
332+
"family": pinned_family,
333+
"proto": 0,
334+
"flags": socket.AI_NUMERICHOST,
335+
}
336+
]
337+
338+
async def close(self) -> None:
339+
pass
340+
341+
277342
async def handle_minio_event(body: bytes, auth_token: str):
278343
"""
279344
Handle a MinIO webhook event.

apps/api/app/core/dependencies.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import hashlib
21
import threading
32
from datetime import timedelta
43
from fnmatch import fnmatch
@@ -18,6 +17,7 @@
1817
AuthException,
1918
PermissionDeniedException,
2019
)
20+
from shared.utils.api_key_hashing import hash_api_key
2121

2222
# Standard JWKS endpoint path (fixed, following OpenID Connect convention)
2323
JWKS_ENDPOINT_PATH = "/api/auth/jwks"
@@ -186,7 +186,7 @@ async def get_current_user_id(
186186
# Mode 1: API Key verification (for external clients)
187187
if token.startswith("sk_"):
188188
# Check identity cache first — skip DB on cache hit
189-
api_key_hash = hashlib.sha256(token.encode()).hexdigest()
189+
api_key_hash = hash_api_key(token)
190190
try:
191191
cached = await identity_cache.get_cached_identity(
192192
redis_pool_manager.get_redis_service(),

apps/api/app/services/auth/api_key_service.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
"""API key management service."""
22

33
import asyncio
4-
import hashlib
54
import uuid
65
from dataclasses import dataclass
76
from datetime import datetime
@@ -23,6 +22,7 @@
2322
)
2423
from shared.models.database.api_key import APIKey
2524
from shared.models.database.user_balance import UserBalance
25+
from shared.utils.api_key_hashing import hash_api_key
2626

2727
_DEFAULT_USER_TIER: str = "free"
2828

@@ -85,7 +85,7 @@ async def create_api_key(
8585

8686
# 3. Generate a secure API key (sk_ + a 32-char UUID without hyphens).
8787
api_key = f"sk_{str(uuid.uuid4()).replace('-', '')}"
88-
key_hash = hashlib.sha256(api_key.encode()).hexdigest()
88+
key_hash = hash_api_key(api_key)
8989
key_mask = self._mask_api_key(api_key)
9090

9191
# 4. Store it in the database.
@@ -116,7 +116,7 @@ async def validate_api_key_identity(
116116
api_key: str,
117117
) -> Optional[APIKeyIdentity]:
118118
"""Validate API key and return the authenticated identity."""
119-
key_hash = hashlib.sha256(api_key.encode()).hexdigest()
119+
key_hash = hash_api_key(api_key)
120120
api_key_record = await self.repository.get_by_key_hash(session, key_hash)
121121

122122
if not api_key_record or not api_key_record.is_valid():
@@ -236,7 +236,7 @@ async def regenerate_api_key(
236236

237237
# 2. Generate a new API key (sk_ + a 32-char UUID without hyphens).
238238
new_api_key = f"sk_{str(uuid.uuid4()).replace('-', '')}"
239-
new_key_hash = hashlib.sha256(new_api_key.encode()).hexdigest()
239+
new_key_hash = hash_api_key(new_api_key)
240240
new_key_mask = self._mask_api_key(new_api_key)
241241

242242
# 3. Update the database record.
@@ -267,7 +267,7 @@ async def check_module_permission(
267267
self, session: AsyncSession, api_key: str, module: str
268268
) -> bool:
269269
"""Check whether an API key can access the requested module."""
270-
key_hash = hashlib.sha256(api_key.encode()).hexdigest()
270+
key_hash = hash_api_key(api_key)
271271
api_key_record = await self.repository.get_by_key_hash(session, key_hash)
272272

273273
if not api_key_record or not api_key_record.is_valid():

apps/api/app/services/guest/guest_registration_service.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Guest registration business logic."""
22

33
import hashlib
4+
import uuid
45
from datetime import datetime
56
from typing import NoReturn
67
from uuid import uuid4
@@ -25,6 +26,7 @@
2526
GuestRegisterResponse,
2627
)
2728
from shared.services.billing.credits_service import CreditsService
29+
from shared.utils.api_key_hashing import hash_api_key
2830

2931
_GUEST_TIER: str = "guest"
3032
_GUEST_KEY_NAME_PREFIX: str = "guest-device"
@@ -153,13 +155,10 @@ async def _create_api_key_without_commit(
153155
This avoids the internal commit inside APIKeyService.create_api_key()
154156
which would make the key durable before the device row is inserted.
155157
"""
156-
import hashlib
157-
import uuid
158-
159158
from shared.models.database.api_key import APIKey
160159

161160
api_key = f"sk_{str(uuid.uuid4()).replace('-', '')}"
162-
key_hash = hashlib.sha256(api_key.encode()).hexdigest()
161+
key_hash = hash_api_key(api_key)
163162
key_mask = self._api_key_service._mask_api_key(api_key)
164163

165164
api_key_record = APIKey(
@@ -264,13 +263,11 @@ def _raise_existing_device_conflict(cls, device_id: str) -> NoReturn:
264263
@staticmethod
265264
async def _resolve_api_key_id(session: AsyncSession, api_key: str) -> str | None:
266265
"""Resolve the DB id for a just-created API key by its hash."""
267-
import hashlib
268-
269266
from sqlalchemy import select
270267

271268
from shared.models.database.api_key import APIKey
272269

273-
key_hash = hashlib.sha256(api_key.encode()).hexdigest()
270+
key_hash = hash_api_key(api_key)
274271
result = await session.execute(
275272
select(APIKey.id).where(APIKey.key_hash == key_hash).limit(1)
276273
)

apps/api/app/services/rate_limit/dependencies.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
only when billing is enabled.
1515
"""
1616

17-
import hashlib
1817
import math
1918
from datetime import datetime, timezone
2019
from typing import AsyncGenerator
@@ -42,6 +41,7 @@
4241
from shared.core.logging import log_context
4342
from shared.core.state_machine.states import JobStatus
4443
from shared.models.database.api_key import APIKey
44+
from shared.utils.api_key_hashing import hash_api_key
4545
from shared.models.database.job import Job
4646
from shared.models.database.user_balance import UserBalance
4747

@@ -175,7 +175,7 @@ async def with_current_user(
175175
api_key_hash = None
176176
is_api_key_auth = isinstance(token, str) and token.startswith("sk_")
177177
if token is not None and is_api_key_auth:
178-
api_key_hash = hashlib.sha256(token.encode()).hexdigest()
178+
api_key_hash = hash_api_key(token)
179179
if is_api_key_auth and api_key_hash:
180180
try:
181181
ttl_seconds = await _resolve_apikey_cache_ttl_seconds(api_key_hash)
@@ -197,7 +197,7 @@ async def with_current_user(
197197
api_key_hash = None
198198
is_api_key_auth = isinstance(token, str) and token.startswith("sk_")
199199
if token is not None and is_api_key_auth:
200-
api_key_hash = hashlib.sha256(token.encode()).hexdigest()
200+
api_key_hash = hash_api_key(token)
201201
cache_key: str = (
202202
identity_cache._apikey_key(api_key_hash)
203203
if is_api_key_auth and api_key_hash

apps/api/scripts/bootstrap_local_dev.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,11 @@ async def _run(mode: str) -> int:
4545

4646

4747
def _print_profile() -> None:
48-
profile = LocalDevelopmentBootstrapService.get_local_developer_profile()
49-
print(f"user_id={profile['user_id']}")
50-
print(f"name={profile['name']}")
51-
print(f"email={profile['email']}")
52-
print(f"tier={profile['tier']}")
53-
print(f"api_key={profile['api_key']}")
48+
print("user_id=local-dev-user")
49+
print("name=Local Development User")
50+
print("email=local-dev-user@knowhere.local")
51+
print("tier=tier_5")
52+
print("local_developer_key_seeded=true")
5453

5554

5655
def main() -> int:

0 commit comments

Comments
 (0)