diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..3ed92d2 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,14 @@ +.git +.env +.venv +__pycache__/ +*.py[cod] +.pytest_cache/ + +logs/ +docs/ +migration/ +README.md +read.md +AGENTS.md +AGENTS.md.bak-* diff --git a/.gitignore b/.gitignore index 44d4d55..ecf95cb 100644 --- a/.gitignore +++ b/.gitignore @@ -9,16 +9,11 @@ __pycache__/ *.log # Local docs and agent instructions -/docs/* -!/docs/ai_integration.md -!/docs/mysql_integration.md +/docs/ +/README.md /read.md /AGENTS.md /AGENTS.md.bak-* # Local migration workspace /migration/ - -# Local generated docs -/docs/ -/README.md diff --git a/Dockerfile b/Dockerfile index 0982cf4..968103e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,6 +16,7 @@ RUN pip install --no-cache-dir \ python-dotenv==1.0.1 \ alembic==1.14.0 \ httpx==0.28.1 \ + lark-oapi==1.6.8 \ apscheduler==3.10.4 \ redis==5.2.1 \ celery==5.4.0 \ diff --git a/app/core/config.py b/app/core/config.py index 93d6460..1dd7e6f 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -64,10 +64,18 @@ class Settings(BaseSettings): @field_validator("cors_origins", mode="before") @classmethod - def parse_cors_origins(cls, value: str | list[str]) -> list[str]: + def parse_cors_origins(cls, value: Any) -> list[str]: if isinstance(value, list): - return value - return [item.strip() for item in value.split(",") if item.strip()] + return [str(item).strip() for item in value if str(item).strip()] + if value is None: + return [] + text = str(value).strip() + if text.startswith("["): + data = json.loads(text) + if not isinstance(data, list): + raise ValueError("CORS_ORIGINS must be a CSV string or JSON list") + return [str(item).strip() for item in data if str(item).strip()] + return [item.strip() for item in text.split(",") if item.strip()] @field_validator("openclaw_allowed_tools", "openclaw_allowed_actions", mode="before") @classmethod diff --git a/app/core/pagination.py b/app/core/pagination.py new file mode 100644 index 0000000..135b731 --- /dev/null +++ b/app/core/pagination.py @@ -0,0 +1,14 @@ +DEFAULT_MAX_LIMIT = 500 +DEFAULT_MIN_LIMIT = 1 + + +def bounded_limit(limit: int, max_limit: int = DEFAULT_MAX_LIMIT) -> int: + """Clamp database query limits to a safe positive range.""" + + return max(DEFAULT_MIN_LIMIT, min(int(limit), max_limit)) + + +def bounded_offset(offset: int) -> int: + """Clamp pagination offsets to zero or above.""" + + return max(0, int(offset)) diff --git a/app/main.py b/app/main.py index e433e30..58c3975 100644 --- a/app/main.py +++ b/app/main.py @@ -6,6 +6,10 @@ from app.core.config import get_settings from app.core.scheduler import attach_scheduler +def _allow_cors_credentials(cors_origins: list[str]) -> bool: + return "*" not in cors_origins + + def create_app() -> FastAPI: """Create and configure the FastAPI application.""" @@ -23,7 +27,7 @@ def create_app() -> FastAPI: app.add_middleware( CORSMiddleware, allow_origins=settings.cors_origins, - allow_credentials=True, + allow_credentials=_allow_cors_credentials(settings.cors_origins), allow_methods=["*"], allow_headers=["*"], ) diff --git a/app/modules/ai_agent/adapters.py b/app/modules/ai_agent/adapters.py index 438f00e..92d9e7f 100644 --- a/app/modules/ai_agent/adapters.py +++ b/app/modules/ai_agent/adapters.py @@ -350,9 +350,18 @@ class DirectLLMAdapter(AIAdapter): if response.status_code >= 400: raise HTTPException(status_code=502, detail={AIErrorKey.DIRECT_LLM: response.text}) data = response.json() - answer = data[AIHttpPayloadKey.CHOICES][0][AIHttpPayloadKey.MESSAGE][ - AIHttpPayloadKey.CONTENT - ] + try: + answer = data[AIHttpPayloadKey.CHOICES][0][AIHttpPayloadKey.MESSAGE][ + AIHttpPayloadKey.CONTENT + ] + except (KeyError, IndexError, TypeError) as exc: + raise HTTPException( + status_code=502, + detail={ + AIErrorKey.DIRECT_LLM: UNEXPECTED_HERMES_RESPONSE, + AIResponseKey.RAW: data, + }, + ) from exc return {AIResponseKey.ANSWER: answer, AIResponseKey.RAW: data} diff --git a/app/modules/approvals/routes.py b/app/modules/approvals/routes.py index 99c9104..1b1fa44 100644 --- a/app/modules/approvals/routes.py +++ b/app/modules/approvals/routes.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.core.database import get_db @@ -21,7 +21,7 @@ def create_approval( @router.get("", response_model=list[ApprovalRead]) def list_approvals( status: str | None = None, - limit: int = 100, + limit: int = Query(default=100, ge=1, le=500), db: Session = Depends(get_db), ): return ApprovalService(db).list(status_filter=status, limit=limit) diff --git a/app/modules/approvals/service.py b/app/modules/approvals/service.py index f52f48e..0ff406b 100644 --- a/app/modules/approvals/service.py +++ b/app/modules/approvals/service.py @@ -8,6 +8,7 @@ from fastapi import HTTPException, status from sqlalchemy import select from sqlalchemy.orm import Session +from app.core.pagination import bounded_limit from app.core.time import utc_now from app.modules.approvals.constants import ( ApprovalActionValue, @@ -56,7 +57,11 @@ class ApprovalService: return ticket def list(self, status_filter: str | None = None, limit: int = 100) -> list[ApprovalRequest]: - stmt = select(ApprovalRequest).order_by(ApprovalRequest.id.desc()).limit(min(limit, 500)) + stmt = ( + select(ApprovalRequest) + .order_by(ApprovalRequest.id.desc()) + .limit(bounded_limit(limit)) + ) if status_filter: stmt = stmt.where(ApprovalRequest.status == status_filter) return list(self.db.execute(stmt).scalars()) diff --git a/app/modules/audit/routes.py b/app/modules/audit/routes.py index 13478cd..86fb0e9 100644 --- a/app/modules/audit/routes.py +++ b/app/modules/audit/routes.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.core.database import get_db @@ -10,5 +10,8 @@ router = APIRouter(dependencies=[Depends(require_api_key)]) @router.get("/logs", response_model=list[AuditLogRead]) -def list_audit_logs(limit: int = 100, db: Session = Depends(get_db)) -> list: +def list_audit_logs( + limit: int = Query(default=100, ge=1, le=500), + db: Session = Depends(get_db), +) -> list: return AuditService(db).list_logs(limit=limit) diff --git a/app/modules/audit/service.py b/app/modules/audit/service.py index 6fce6e3..81a5383 100644 --- a/app/modules/audit/service.py +++ b/app/modules/audit/service.py @@ -4,6 +4,7 @@ from typing import Any from sqlalchemy import select from sqlalchemy.orm import Session +from app.core.pagination import bounded_limit from app.modules.audit.models import AuditLog from app.modules.audit.schemas import AuditLogCreate @@ -42,5 +43,5 @@ class AuditService: return record def list_logs(self, limit: int = 100) -> list[AuditLog]: - stmt = select(AuditLog).order_by(AuditLog.id.desc()).limit(min(limit, 500)) + stmt = select(AuditLog).order_by(AuditLog.id.desc()).limit(bounded_limit(limit)) return list(self.db.execute(stmt).scalars()) diff --git a/app/modules/business/routes.py b/app/modules/business/routes.py index 76683b6..8a78b81 100644 --- a/app/modules/business/routes.py +++ b/app/modules/business/routes.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy.orm import Session from app.core.database import get_db @@ -18,8 +18,8 @@ def list_domains() -> dict[str, list[str]]: @router.get("/{domain}", response_model=DomainListRead) def list_records( domain: str, - limit: int = 50, - offset: int = 0, + limit: int = Query(default=50, ge=1, le=500), + offset: int = Query(default=0, ge=0), status: str | None = None, db: Session = Depends(get_db), ) -> dict: diff --git a/app/modules/business/service.py b/app/modules/business/service.py index ed3ada9..244fe71 100644 --- a/app/modules/business/service.py +++ b/app/modules/business/service.py @@ -11,6 +11,7 @@ from sqlalchemy.sql.schema import Column from sqlalchemy.orm import Session from app.core.constants import ActorValue +from app.core.pagination import bounded_limit, bounded_offset from app.modules.audit.constants import AuditRiskLevel, AuditSource from app.modules.audit.schemas import AuditLogCreate from app.modules.audit.service import AuditService @@ -89,7 +90,9 @@ class BusinessService: if status_filter and hasattr(model, "status"): stmt = stmt.where(model.status == status_filter) count_stmt = count_stmt.where(model.status == status_filter) - stmt = stmt.order_by(model.id.desc()).limit(min(limit, 500)).offset(max(offset, 0)) + stmt = stmt.order_by(model.id.desc()).limit(bounded_limit(limit)).offset( + bounded_offset(offset) + ) total = int(self.db.execute(count_stmt).scalar() or 0) return total, [serialize_model(item) for item in self.db.execute(stmt).scalars()] diff --git a/app/modules/feishu/service.py b/app/modules/feishu/service.py index eeedfee..bd443ed 100644 --- a/app/modules/feishu/service.py +++ b/app/modules/feishu/service.py @@ -1,3 +1,4 @@ +from secrets import compare_digest from typing import Any from fastapi import HTTPException, status @@ -30,7 +31,7 @@ class FeishuService: status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="FEISHU_VERIFICATION_TOKEN is required", ) - if not token or token != expected: + if not token or not compare_digest(str(token), expected): raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid Feishu token", diff --git a/app/modules/legacy_mysql/routes.py b/app/modules/legacy_mysql/routes.py index 5575b18..b128359 100644 --- a/app/modules/legacy_mysql/routes.py +++ b/app/modules/legacy_mysql/routes.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.core.database import get_db @@ -38,7 +38,10 @@ def readonly_query(payload: ReadonlyQueryRequest, db: Session = Depends(get_db)) @router.get("/projects", response_model=QueryResult) -def default_project_query(limit: int = 100, db: Session = Depends(get_db)) -> dict: +def default_project_query( + limit: int = Query(default=100, ge=1, le=500), + db: Session = Depends(get_db), +) -> dict: return LegacyMySQLService(db).fetch_default_projects(limit=limit) diff --git a/app/modules/legacy_mysql/service.py b/app/modules/legacy_mysql/service.py index 597d90a..0f9e0fe 100644 --- a/app/modules/legacy_mysql/service.py +++ b/app/modules/legacy_mysql/service.py @@ -12,6 +12,7 @@ from sqlalchemy.orm import Session from app.core.constants import ActorValue from app.core.config import get_settings from app.core.database import legacy_engine +from app.core.pagination import bounded_limit from app.core.time import utc_now from app.modules.audit.constants import AuditAction, AuditRiskLevel, AuditSource, AuditStatus from app.modules.audit.schemas import AuditLogCreate @@ -174,7 +175,7 @@ class LegacyMySQLService: self._ensure_readonly(sql) engine = self._ensure_engine() params = dict(params or {}) - params.setdefault("limit", min(limit, 500)) + params.setdefault("limit", bounded_limit(limit)) limited_sql = sql if " limit " not in sql.lower(): limited_sql = f"{sql.rstrip(';')} LIMIT :limit" diff --git a/app/modules/reports/service.py b/app/modules/reports/service.py index 22e915f..75be158 100644 --- a/app/modules/reports/service.py +++ b/app/modules/reports/service.py @@ -6,6 +6,7 @@ from sqlalchemy import func, or_, select from sqlalchemy.orm import Session from app.core.constants import ActorValue +from app.core.pagination import bounded_limit from app.core.time import utc_now from app.modules.audit.constants import AuditSource, AuditTargetType from app.modules.audit.schemas import AuditLogCreate @@ -137,7 +138,7 @@ class ReportService: stmt = stmt.order_by(order_by) else: stmt = stmt.order_by(model.id.desc()) - return list(self.db.execute(stmt.limit(limit)).scalars()) + return list(self.db.execute(stmt.limit(bounded_limit(limit))).scalars()) def daily_brief(self) -> dict: project_count = self._count(Project) diff --git a/app/modules/risk/routes.py b/app/modules/risk/routes.py index ed45c4f..7f68c29 100644 --- a/app/modules/risk/routes.py +++ b/app/modules/risk/routes.py @@ -1,4 +1,4 @@ -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session from app.core.database import get_db @@ -40,7 +40,7 @@ def supplier_risks(db: Session = Depends(get_db)) -> dict: @router.get("/events") def risk_events( - limit: int = 100, + limit: int = Query(default=100, ge=1, le=500), status: str | None = None, db: Session = Depends(get_db), ) -> dict: diff --git a/app/modules/risk/service.py b/app/modules/risk/service.py index db96ab9..42f4c84 100644 --- a/app/modules/risk/service.py +++ b/app/modules/risk/service.py @@ -6,6 +6,7 @@ from sqlalchemy import select from sqlalchemy.orm import Session from app.core.constants import ActorValue +from app.core.pagination import bounded_limit from app.core.time import utc_now from app.modules.audit.constants import ( AuditAction, @@ -75,13 +76,14 @@ class RiskService: limit: int = 100, status_filter: str | None = None, ) -> list[dict[str, Any]]: - stmt = select(RiskEvent).order_by(RiskEvent.id.desc()).limit(min(limit, 500)) + limit_value = bounded_limit(limit) + stmt = select(RiskEvent).order_by(RiskEvent.id.desc()).limit(limit_value) if status_filter: stmt = ( select(RiskEvent) .where(RiskEvent.status == status_filter) .order_by(RiskEvent.id.desc()) - .limit(min(limit, 500)) + .limit(limit_value) ) return [serialize_model(item) for item in self.db.execute(stmt).scalars()] diff --git a/scripts/verify_smoke.py b/scripts/verify_smoke.py index bd42e41..91ac3e2 100644 --- a/scripts/verify_smoke.py +++ b/scripts/verify_smoke.py @@ -11,6 +11,9 @@ db.close() os.environ["DATABASE_URL"] = "sqlite:///" + db.name.replace("\\", "/") os.environ["API_KEY"] = "test-key" +os.environ["LEGACY_ALLOWED_QUERIES"] = "{}" +os.environ["LEGACY_DATABASE_URL"] = "" +os.environ["LEGACY_PROJECT_QUERY"] = "" os.environ["MODEL_PROVIDER"] = "noop" os.environ["SCHEDULER_ENABLED"] = "false" diff --git a/tests/test_ai_adapters.py b/tests/test_ai_adapters.py index ea180e4..b3698b0 100644 --- a/tests/test_ai_adapters.py +++ b/tests/test_ai_adapters.py @@ -9,6 +9,7 @@ from app.modules.ai_agent.constants import ( OPENCLAW_HERMES_PIPELINE, AIChatRole, AIContextKey, + AIErrorKey, AIHttpHeader, AIHttpPath, AIHttpPayloadKey, @@ -237,3 +238,30 @@ def test_openclaw_hermes_adapter_fails_when_requested_tool_is_blocked(monkeypatc "http://openclaw.local/healthz", "http://openclaw.local/readyz", ] + + +def test_direct_llm_adapter_wraps_unexpected_chat_response(monkeypatch) -> None: + class BadChatClient(DummyClient): + def post( + self, + url: str, + json: dict[str, Any], + headers: dict[str, str], + ) -> DummyResponse: + self.calls.append({"method": "POST", "url": url, "json": json, "headers": headers}) + return DummyResponse({AIHttpPayloadKey.CHOICES: []}) + + DummyClient.calls = [] + monkeypatch.setattr(adapters.httpx, "Client", BadChatClient) + settings = Settings( + direct_llm_base_url="http://llm.local/v1", + direct_llm_api_key="direct-key", + direct_llm_model="company-model", + ) + + with pytest.raises(HTTPException) as exc_info: + adapters.DirectLLMAdapter(settings).ask("summarize") + + assert exc_info.value.status_code == 502 + assert exc_info.value.detail[AIErrorKey.DIRECT_LLM] == "Unexpected chat completion response" + assert exc_info.value.detail[AIResponseKey.RAW] == {AIHttpPayloadKey.CHOICES: []} diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 22c655d..c0efac5 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -20,15 +20,19 @@ os.environ["APPROVAL_API_ACTOR"] = "approval-manager" os.environ["FEISHU_APP_ID"] = "" os.environ["FEISHU_APP_SECRET"] = "" os.environ["FEISHU_VERIFICATION_TOKEN"] = "test-feishu-token" +os.environ["LEGACY_ALLOWED_QUERIES"] = "{}" +os.environ["LEGACY_DATABASE_URL"] = "" +os.environ["LEGACY_PROJECT_QUERY"] = "" os.environ["MODEL_PROVIDER"] = AIProviderName.NOOP os.environ["SCHEDULER_ENABLED"] = "false" from fastapi.testclient import TestClient -from app.core.config import get_settings +from app.core.config import Settings, get_settings from app.core.database import Base, engine +from app.core.pagination import bounded_limit, bounded_offset from app.core.security import require_api_key, require_approval_api_key -from app.main import app +from app.main import _allow_cors_credentials, app from app.modules.legacy_mysql.service import LegacyMySQLService from app.modules.reports.constants import ( LifecycleAttentionKey, @@ -133,6 +137,26 @@ def test_api_key_and_feishu_webhook_fail_closed(monkeypatch) -> None: get_settings.cache_clear() +def test_config_and_pagination_guardrails() -> None: + settings = Settings(cors_origins='["https://app.example.com", "https://admin.example.com"]') + + assert settings.cors_origins == ["https://app.example.com", "https://admin.example.com"] + assert _allow_cors_credentials(["*"]) is False + assert _allow_cors_credentials(["https://app.example.com"]) is True + assert bounded_limit(-1) == 1 + assert bounded_limit(1000) == 500 + assert bounded_offset(-10) == 0 + + negative_limit_response = client.get("/api/v1/business/projects?limit=-1", headers=headers) + assert negative_limit_response.status_code == 422 + + oversized_limit_response = client.get("/api/v1/risks/events?limit=501", headers=headers) + assert oversized_limit_response.status_code == 422 + + negative_offset_response = client.get("/api/v1/business/projects?offset=-1", headers=headers) + assert negative_offset_response.status_code == 422 + + def test_approval_gate_for_high_risk_update() -> None: create_payload = { "code": "FUND-SMOKE-001",