Skip to content

feat:add project model#57

Open
xiaocheny214 wants to merge 12 commits into
1024XEngineer:mainfrom
xiaocheny214:feat/add-project-model
Open

feat:add project model#57
xiaocheny214 wants to merge 12 commits into
1024XEngineer:mainfrom
xiaocheny214:feat/add-project-model

Conversation

@xiaocheny214

Copy link
Copy Markdown

项目背景

添加项目模块

项目内容

  • 项目模型及 CRUD 服务
  • 项目 CRUD API
  • 图片上传 API
  • 七牛 Kodo 存储适配器
  • 数据库 Session 同步化调整
  • python-multipart 依赖
  • 项目 API 测试

- 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)

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found several correctness, security, and operability issues in the new project/upload paths. The checked-out tree compiles (python3 -m compileall), but the configured CI commands could not be run because uv is not installed in this environment.

class Project(Base):
"""项目表,保存生成所需的全局约束。"""

__tablename__ = "windup_project"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds the windup_project model, but the PR does not add a migration, schema bootstrap, or any other path that creates this table. On a fresh database (and after deployment to a database without this table), every project CRUD endpoint will fail with relation windup_project does not exist. Please include the migration/deployment step or wire the application to apply the schema before exposing these routes.

def delete_project(
project_id: int, session: Session = Depends(get_session)
) -> Response[None]:
if not service.delete_project(session, project_id):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These endpoints do not establish an authenticated principal or constrain the lookup/delete to one. Because user_id is accepted directly from the request on create and is optional on list, an unauthenticated caller can create records for arbitrary users, enumerate all projects, read any project by ID, and delete any project by ID. Derive ownership from authentication and apply it to every project query/mutation before exposing this API.

else "bin"
)
key = f"projects/samples/{uuid4().hex}.{suffix}"
data = await file.read()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await file.read() loads the entire client-controlled upload into memory, with no size limit or streaming/backpressure. A single request can therefore allocate an arbitrarily large buffer and exhaust worker memory (and the bytes are then duplicated/held by the storage SDK). Enforce a maximum size while reading or stream the upload to storage.

data = await file.read()
logger.info("[WINDUP] 开始上传至 Kodo | key=%s size=%s", key, len(data))
try:
url = _storage.upload(data, key, file.content_type)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an async endpoint, but _storage.upload performs the synchronous network call through the Qiniu SDK. Each upload blocks the event-loop worker for the duration of the request, so concurrent uploads (or a slow storage response) stall unrelated async endpoints. Run the blocking adapter in a threadpool or provide an async storage path.

token = auth.upload_token(
self.config.bucket_name, key, expires=self.config.upload_expires
)
result, info = put_data(token, key, data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The adapter accepts mime_type and the caller passes file.content_type, but this call omits it, so Qiniu uses its default content type instead of the uploaded image's type. The returned object can consequently be served as application/octet-stream rather than an image. Pass the MIME type through to put_data (and test the request metadata).

key, info.status_code,
)
raise RuntimeError(f"Kodo 上传失败: {info.status_code}")
url = f"{self.config.download_base}/{key}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private_space is documented as requiring signed download URLs, but this path always returns a plain ${bucket_domain}/${key} URL and never uses download_expires or Auth.private_download_url. With QINIU_PRIVATE_SPACE=true, every successful upload returns an inaccessible URL. Branch on the setting and sign the returned URL for private buckets.

factory=True,
host=os.getenv("WINDUP_HOST", "127.0.0.1"),
port=int(os.getenv("WINDUP_PORT", "8000")),
reload=bool(os.getenv("WINDUP_RELOAD")),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool(os.getenv('WINDUP_RELOAD')) treats every non-empty value as true, including the documented/common setting WINDUP_RELOAD=false. This makes it impossible to explicitly disable reload through the environment and can unexpectedly enable the development reloader in production. Parse the value as a boolean (for example, with a settings field or an explicit true-value set).

johnnyzhang-eng and others added 2 commits July 27, 2026 10:01
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants