feat:add ai client#61
Conversation
- StorageSettings (env prefix QINIU_) in windup_framework/config - add qiniu>=7.14 to framework deps (resolved 7.18.0) - tests for config loading/defaults/env prefix (no network) - client wrapper deferred to server/asset domain
…enum - Response[T]: HTTP always 200, biz code in body, timestamp omitted when unset - BizException/ModelException hierarchy, auto-caught via MRO by global handler - BizCode/ModelErrorType enums for biz codes and model error classification
- handlers for BizException / RequestValidationError / HTTPException / Exception (fallback) - register_exception_handlers wired into create_app
- data: list[T], defaults to empty list (not null) for list endpoints - total/page/page_size fields; page_size=0 means unpaginated, total defaults to len(data)
Carries over the FennoAI F2/F3/F4 fixes (F1 sync session already present on this branch): URL.create for safe DSN, _env_flag for reload parsing, windup_app/__main__.py for 'python -m windup_app'. Adds targeted tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review follow-ups on 1024XEngineer#57: enforce a configurable upload size limit (Content-Length early-reject + chunked byte-count read) to prevent memory-exhaustion DoS; derive the object key from a MIME allowlist instead of the client filename suffix; rename create_at/update_at -> created_at/updated_at. Adds upload API tests (valid / non-image / oversized). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix：pr 3 upload hardening
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Found five concrete issues affecting tenant isolation, async request handling, upload-limit correctness, and AI client configuration. The full backend suite passes (ruff, import-linter, and 75 pytest tests), but targeted reproductions confirm the multipart-limit and factory-configuration failures.
| from windup_framework.db import get_session | ||
|
|
||
| from windup_app.server.project import service | ||
|
|
There was a problem hiding this comment.
[P1] Scope project access to the authenticated principal. The router accepts caller-controlled user_id values and the get/delete paths query only a global project_id; no authentication dependency or ownership predicate is applied. Any client can therefore list, fetch, or delete another user's projects (and create projects for an arbitrary user). Derive the user ID from authenticated context and include it in every query and mutation.
| ) | ||
| chunks.append(chunk) | ||
| return b"".join(chunks) | ||
|
|
There was a problem hiding this comment.
[P2] Compare the limit to file bytes, not the multipart request length. Content-Length covers the entire multipart body, including boundaries and part headers, so a valid file near the configured maximum is rejected before _read_capped examines it. For example, with a 512-byte limit, a 400-byte file produces a 566-byte request and is rejected. Remove this early check or rely on a request-layer limit that understands multipart framing.
|
|
||
| limit = _max_upload_bytes() | ||
| # 有 Content-Length 时先按声明体积早拒,省得白读一遍 | ||
| declared = request.headers.get("content-length") |
There was a problem hiding this comment.
[P2] Do not run the synchronous Kodo network call directly on the event loop. KodoStorage.upload() calls the synchronous Qiniu put_data, so every upload blocks all other async requests until the remote operation finishes. Offload it with run_in_threadpool/asyncio.to_thread, or restructure the endpoint so blocking work runs in FastAPI's thread pool.
| @@ -0,0 +1,22 @@ | |||
| # ── LLM ── | |||
| LLM_API_KEY=your-llm-key | |||
There was a problem hiding this comment.
[P1] Align the documented environment names with AIProviderSettings. The new settings class reads AI_API_KEY, AI_MODEL, and AI_BASE_URL, while this example instructs users to set LLM_API_KEY, LLM_MODEL_ID, and LLM_BASE_URL. Following the checked-in configuration leaves the key/model empty and keeps the OpenAI default URL, so the new factories cannot use the documented provider.
| base_url=config.normalized_base_url, | ||
| timeout=config.timeout, | ||
| max_retries=config.max_retries, | ||
| **kwargs, |
There was a problem hiding this comment.
[P2] Allow caller kwargs to override configured client options. Because model, base_url, timeout, and max_retries are passed explicitly before **kwargs, supplying any of those supported options raises TypeError (for example, create_chat_model(config, timeout=1)). Build a defaults dict and update it with kwargs; the image and video factories have the same collision for their client options.
There was a problem hiding this comment.
Review Summary
Found five concrete issues affecting tenant isolation, async request handling, upload-limit correctness, and AI client configuration. The full backend suite passes (ruff, import-linter, and 75 pytest tests), but targeted reproductions confirm the multipart-limit and factory-configuration failures.
| from windup_framework.db import get_session | ||
|
|
||
| from windup_app.server.project import service | ||
|
|
There was a problem hiding this comment.
[P1] Scope project access to the authenticated principal. The router accepts caller-controlled user_id values and the get/delete paths query only a global project_id; no authentication dependency or ownership predicate is applied. Any client can therefore list, fetch, or delete another user's projects (and create projects for an arbitrary user). Derive the user ID from authenticated context and include it in every query and mutation.
| ) | ||
| chunks.append(chunk) | ||
| return b"".join(chunks) | ||
|
|
There was a problem hiding this comment.
[P2] Compare the limit to file bytes, not the multipart request length. Content-Length covers the entire multipart body, including boundaries and part headers, so a valid file near the configured maximum is rejected before _read_capped examines it. For example, with a 512-byte limit, a 400-byte file produces a 566-byte request and is rejected. Remove this early check or rely on a request-layer limit that understands multipart framing.
|
|
||
| limit = _max_upload_bytes() | ||
| # 有 Content-Length 时先按声明体积早拒,省得白读一遍 | ||
| declared = request.headers.get("content-length") |
There was a problem hiding this comment.
[P2] Do not run the synchronous Kodo network call directly on the event loop. KodoStorage.upload() calls the synchronous Qiniu put_data, so every upload blocks all other async requests until the remote operation finishes. Offload it with run_in_threadpool/asyncio.to_thread, or restructure the endpoint so blocking work runs in FastAPI's thread pool.
| @@ -0,0 +1,22 @@ | |||
| # ── LLM ── | |||
| LLM_API_KEY=your-llm-key | |||
There was a problem hiding this comment.
[P1] Align the documented environment names with AIProviderSettings. The new settings class reads AI_API_KEY, AI_MODEL, and AI_BASE_URL, while this example instructs users to set LLM_API_KEY, LLM_MODEL_ID, and LLM_BASE_URL. Following the checked-in configuration leaves the key/model empty and keeps the OpenAI default URL, so the new factories cannot use the documented provider.
| base_url=config.normalized_base_url, | ||
| timeout=config.timeout, | ||
| max_retries=config.max_retries, | ||
| **kwargs, |
There was a problem hiding this comment.
[P2] Allow caller kwargs to override configured client options. Because model, base_url, timeout, and max_retries are passed explicitly before **kwargs, supplying any of those supported options raises TypeError (for example, create_chat_model(config, timeout=1)). Build a defaults dict and update it with kwargs; the image and video factories have the same collision for their client options.
2 participants
变更范围
次提交包含 AI Provider 配置、Chat/Image/Video 官方客户端工厂、依赖锁定及对应测试。