From 4bfe2900fb30c65d6140cc3d9249b27977bcdf09 Mon Sep 17 00:00:00 2001 From: v6ole Date: Tue, 28 Jul 2026 16:41:36 +0800 Subject: [PATCH] build(security): add migration and CI security gates --- .gitlab-ci.yml | 5 +- backend/Dockerfile | 2 +- .../versions/20260728_01_baseline_schema.py | 57 ++++++++++++++++ backend/alembic/versions/__init__.py | 1 + backend/app/api/wecom.py | 15 +++-- backend/app/config.py | 8 +-- backend/app/main.py | 67 +------------------ backend/app/services/wecom.py | 5 +- backend/app/utils/security.py | 5 +- backend/pyproject.toml | 11 +++ backend/requirements.txt | 13 ++-- backend/tests/test_api_security.py | 59 ++++++++++++++++ backend/tests/test_jwt_security.py | 10 +++ docker-compose.yml | 13 +++- 整改计划-中国电信研发规范.md | 12 ++-- 15 files changed, 186 insertions(+), 97 deletions(-) create mode 100644 backend/alembic/versions/20260728_01_baseline_schema.py create mode 100644 backend/alembic/versions/__init__.py create mode 100644 backend/pyproject.toml create mode 100644 backend/tests/test_api_security.py create mode 100644 backend/tests/test_jwt_security.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 40828e0..e3e85a4 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -6,9 +6,12 @@ backend_verify: stage: verify image: python:3.12-slim script: - - pip install --no-cache-dir -r backend/requirements.txt pytest + - pip install --no-cache-dir -r backend/requirements.txt pytest ruff bandit pip-audit - PYTHONPATH=backend python -m compileall -q backend/app + - ruff check backend/app backend/tests - PYTHONPATH=backend pytest -q backend/tests + - bandit -q -r backend/app -lll + - pip-audit -r backend/requirements.txt frontend_build: stage: build diff --git a/backend/Dockerfile b/backend/Dockerfile index 385ccd3..7a63d27 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -9,4 +9,4 @@ COPY . . EXPOSE 8000 -CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "8000"] +CMD ["sh", "-c", "alembic upgrade head && exec uvicorn app.main:app --host 0.0.0.0 --port 8000"] diff --git a/backend/alembic/versions/20260728_01_baseline_schema.py b/backend/alembic/versions/20260728_01_baseline_schema.py new file mode 100644 index 0000000..87d0051 --- /dev/null +++ b/backend/alembic/versions/20260728_01_baseline_schema.py @@ -0,0 +1,57 @@ +"""Establish the tracked baseline schema and replace application-startup DDL. + +Revision ID: 20260728_01 +Revises: +Create Date: 2026-07-28 +""" + +from collections.abc import Sequence + +from alembic import op + +from app.database import Base +import app.models # noqa: F401 # Register every model with Base.metadata. + + +revision: str = "20260728_01" +down_revision: str | None = None +branch_labels: Sequence[str] | None = None +depends_on: Sequence[str] | None = None + + +def upgrade() -> None: + """Create the current schema and upgrade tables created by older app versions.""" + bind = op.get_bind() + Base.metadata.create_all(bind=bind, checkfirst=True) + + # These columns were historically added in application startup code. Keep this + # compatibility path so existing deployments can adopt Alembic safely. + statements = ( + "ALTER TABLE customers ADD COLUMN IF NOT EXISTS remarks TEXT DEFAULT ''", + "ALTER TABLE visits ADD COLUMN IF NOT EXISTS visitor_name VARCHAR(50) DEFAULT ''", + "ALTER TABLE visits ADD COLUMN IF NOT EXISTS visitor_phone VARCHAR(20) DEFAULT ''", + "ALTER TABLE users ADD COLUMN IF NOT EXISTS require_report BOOLEAN DEFAULT TRUE", + "ALTER TABLE customers ADD COLUMN IF NOT EXISTS last_visit_date DATE", + "ALTER TABLE customers ADD COLUMN IF NOT EXISTS last_visit_manager_id UUID", + "ALTER TABLE visits ADD COLUMN IF NOT EXISTS companion_names TEXT[] DEFAULT '{}'", + "ALTER TABLE users ADD COLUMN IF NOT EXISTS color VARCHAR(7)", + "ALTER TABLE visits ADD COLUMN IF NOT EXISTS edit_log JSONB DEFAULT '[]'", + "ALTER TABLE daily_notes ADD COLUMN IF NOT EXISTS edit_log JSONB DEFAULT '[]'", + "ALTER TABLE work_plans ADD COLUMN IF NOT EXISTS edit_log JSONB DEFAULT '[]'", + "ALTER TABLE mini_business ADD COLUMN IF NOT EXISTS edit_log JSONB DEFAULT '[]'", + "ALTER TABLE key_visits ADD COLUMN IF NOT EXISTS edit_log JSONB DEFAULT '[]'", + "CREATE UNIQUE INDEX IF NOT EXISTS uq_users_wecom_userid " + "ON users (wecom_userid) WHERE wecom_userid IS NOT NULL", + ) + for statement in statements: + op.execute(statement) + + +def downgrade() -> None: + """Remove only objects introduced by the tracked security hardening revision. + + The historical base tables pre-date Alembic and may contain production data, so + they are deliberately not dropped by a downgrade operation. + """ + op.execute("DROP INDEX IF EXISTS uq_users_wecom_userid") + op.execute("DROP TABLE IF EXISTS wecom_bind_tokens") diff --git a/backend/alembic/versions/__init__.py b/backend/alembic/versions/__init__.py new file mode 100644 index 0000000..c8b57c4 --- /dev/null +++ b/backend/alembic/versions/__init__.py @@ -0,0 +1 @@ +"""Alembic migration revisions for the Qiji backend.""" diff --git a/backend/app/api/wecom.py b/backend/app/api/wecom.py index 79157a6..59b52ab 100644 --- a/backend/app/api/wecom.py +++ b/backend/app/api/wecom.py @@ -1,9 +1,11 @@ import hashlib +import hmac import struct import base64 import uuid -import xml.etree.ElementTree as ET from typing import Optional +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes +from defusedxml import ElementTree as ET from fastapi import APIRouter, Depends, HTTPException, Query, Request from fastapi.responses import PlainTextResponse from pydantic import BaseModel @@ -38,17 +40,20 @@ class BindConfirmRequest(BaseModel): def _verify_signature(token: str, timestamp: str, nonce: str, encrypted: str, signature: str) -> bool: items = sorted([token, timestamp, nonce, encrypted]) raw = "".join(items) - return hashlib.sha1(raw.encode()).hexdigest() == signature + # The WeCom callback protocol mandates SHA-1; compare_digest avoids timing leaks. + expected = hashlib.sha1(raw.encode(), usedforsecurity=False).hexdigest() + return hmac.compare_digest(expected, signature) def _decrypt_msg(encrypted: str) -> str: """Decrypt WeChat Work callback message. Returns plaintext XML.""" key = base64.b64decode(settings.WECOM_ENCODING_AES_KEY + "=") ciphertext = base64.b64decode(encrypted) - from Crypto.Cipher import AES - cipher = AES.new(key, AES.MODE_CBC, iv=key[:16]) - plaintext = cipher.decrypt(ciphertext) + decryptor = Cipher(algorithms.AES(key), modes.CBC(key[:16])).decryptor() + plaintext = decryptor.update(ciphertext) + decryptor.finalize() pad_len = plaintext[-1] + if pad_len < 1 or pad_len > 32 or plaintext[-pad_len:] != bytes([pad_len]) * pad_len: + raise ValueError("Invalid callback padding") plaintext = plaintext[:-pad_len] msg_len = struct.unpack(">I", plaintext[16:20])[0] return plaintext[20:20 + msg_len].decode("utf-8") diff --git a/backend/app/config.py b/backend/app/config.py index 3b60f75..2948e54 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -1,8 +1,9 @@ -from pydantic_settings import BaseSettings +from pydantic_settings import BaseSettings, SettingsConfigDict from typing import Optional class Settings(BaseSettings): + model_config = SettingsConfigDict(env_file=".env", env_file_encoding="utf-8") # App APP_NAME: str = "企迹-政企周报管理系统" ENVIRONMENT: str = "production" @@ -49,11 +50,6 @@ class Settings(BaseSettings): # CORS CORS_ORIGINS: list[str] = ["http://localhost:5173", "http://localhost:3000"] - class Config: - env_file = ".env" - env_file_encoding = "utf-8" - - settings = Settings() diff --git a/backend/app/main.py b/backend/app/main.py index 540b354..5996404 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -3,7 +3,7 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware from sqlalchemy import select from app.config import settings, validate_security_settings -from app.database import engine, Base, async_session +from app.database import engine, async_session from app.api import router as api_router from app.api import auth, users, customers, visits, work_plans, mini_business, key_visits from app.api import dashboard, upload, export, import_data, wecom, daily_notes, ai_summary, system_config, leaves @@ -15,71 +15,6 @@ from app.services.scheduler_manager import start_scheduler, shutdown_scheduler @asynccontextmanager async def lifespan(app: FastAPI): validate_security_settings() - # Startup: create tables if not exists (for dev convenience) - async with engine.begin() as conn: - await conn.run_sync(Base.metadata.create_all) - # Add columns that may be missing from older tables - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE customers ADD COLUMN IF NOT EXISTS remarks TEXT DEFAULT ''" - )) - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE visits ADD COLUMN IF NOT EXISTS visitor_name VARCHAR(50) DEFAULT ''" - )) - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE visits ADD COLUMN IF NOT EXISTS visitor_phone VARCHAR(20) DEFAULT ''" - )) - # edit_log columns for change tracking - for tbl in ["visits", "daily_notes", "work_plans", "mini_business", "key_visits"]: - await conn.run_sync(lambda c, t=tbl: c.exec_driver_sql( - f"ALTER TABLE {t} ADD COLUMN IF NOT EXISTS edit_log JSONB DEFAULT '[]'" - )) - # New columns for v0.3 - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE users ADD COLUMN IF NOT EXISTS require_report BOOLEAN DEFAULT TRUE" - )) - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE customers ADD COLUMN IF NOT EXISTS last_visit_date DATE" - )) - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE customers ADD COLUMN IF NOT EXISTS last_visit_manager_id UUID" - )) - # New columns for v0.4 - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE visits ADD COLUMN IF NOT EXISTS companion_names TEXT[] DEFAULT '{}'" - )) - # v0.6 — manager color tag - await conn.run_sync(lambda c: c.exec_driver_sql( - "ALTER TABLE users ADD COLUMN IF NOT EXISTS color VARCHAR(7)" - )) - await conn.run_sync(lambda c: c.exec_driver_sql( - "CREATE UNIQUE INDEX IF NOT EXISTS uq_users_wecom_userid " - "ON users (wecom_userid) WHERE wecom_userid IS NOT NULL" - )) - # system_config table for v0.5 - await conn.run_sync(lambda c: c.exec_driver_sql( - "CREATE TABLE IF NOT EXISTS system_config (key VARCHAR(64) PRIMARY KEY, value TEXT DEFAULT '')" - )) - # leaves table for v0.7 - await conn.run_sync(lambda c: c.exec_driver_sql( - "CREATE TABLE IF NOT EXISTS leaves (" - " id UUID PRIMARY KEY DEFAULT gen_random_uuid()," - " manager_id UUID NOT NULL REFERENCES users(id)," - " leave_type VARCHAR(20) NOT NULL DEFAULT '事假'," - " start_date DATE NOT NULL," - " end_date DATE NOT NULL," - " reason VARCHAR(500) DEFAULT ''," - " submitted_by UUID NOT NULL REFERENCES users(id)," - " created_at TIMESTAMPTZ DEFAULT now()," - " updated_at TIMESTAMPTZ DEFAULT now()" - ")" - )) - await conn.run_sync(lambda c: c.exec_driver_sql( - "CREATE INDEX IF NOT EXISTS idx_leaves_manager_id ON leaves(manager_id)" - )) - await conn.run_sync(lambda c: c.exec_driver_sql( - "CREATE INDEX IF NOT EXISTS idx_leaves_dates ON leaves(start_date, end_date)" - )) - # Read notification_time from DB (or use default 17:30) notification_hour, notification_minute = 17, 30 async with async_session() as db: diff --git a/backend/app/services/wecom.py b/backend/app/services/wecom.py index 00b0432..1094602 100644 --- a/backend/app/services/wecom.py +++ b/backend/app/services/wecom.py @@ -1,3 +1,4 @@ +import asyncio import time import uuid from datetime import datetime, timedelta, timezone @@ -91,14 +92,14 @@ class WecomClient: if errcode == 45009: if attempt < max_retries - 1: - time.sleep(1 * (attempt + 1)) + await asyncio.sleep(1 * (attempt + 1)) continue last_error = data except (httpx.TimeoutException, httpx.ConnectError) as e: last_error = {"errcode": -1, "errmsg": str(e)} if attempt < max_retries - 1: - time.sleep(0.5 * (attempt + 1)) + await asyncio.sleep(0.5 * (attempt + 1)) raise Exception(f"WeCom API error after {max_retries} retries: {last_error}") diff --git a/backend/app/utils/security.py b/backend/app/utils/security.py index bae475a..c9b9be9 100644 --- a/backend/app/utils/security.py +++ b/backend/app/utils/security.py @@ -1,6 +1,7 @@ from datetime import datetime, timedelta, timezone from typing import Optional -from jose import jwt, JWTError +import jwt +from jwt import InvalidTokenError from app.config import settings @@ -15,5 +16,5 @@ def decode_token(token: str) -> Optional[dict]: try: payload = jwt.decode(token, settings.SECRET_KEY, algorithms=[settings.JWT_ALGORITHM]) return payload - except JWTError: + except InvalidTokenError: return None diff --git a/backend/pyproject.toml b/backend/pyproject.toml new file mode 100644 index 0000000..3b1644f --- /dev/null +++ b/backend/pyproject.toml @@ -0,0 +1,11 @@ +[tool.ruff] +target-version = "py312" + +[tool.ruff.lint] +# Fail CI for parse errors and undefined names. Broader style cleanup is tracked +# separately because the existing codebase has a substantial legacy backlog. +select = ["E9", "F63", "F7", "F82"] + +[tool.ruff.lint.per-file-ignores] +# SQLAlchemy resolves these relationship targets from strings after model import. +"app/models/*.py" = ["F821"] diff --git a/backend/requirements.txt b/backend/requirements.txt index a189753..e78b262 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,16 +1,17 @@ -fastapi==0.115.6 +fastapi==0.140.7 uvicorn[standard]==0.34.0 sqlalchemy[asyncio]==2.0.36 asyncpg==0.30.0 alembic==1.14.0 pydantic==2.10.3 pydantic-settings==2.7.0 -python-jose[cryptography]==3.3.0 +PyJWT==2.13.0 httpx==0.28.1 -python-multipart==0.0.18 +python-multipart==0.0.32 minio==7.2.10 openpyxl==3.1.5 apscheduler==3.11.0 -python-dotenv==1.0.1 -pycryptodome -Pillow==11.1.0 +python-dotenv==1.2.2 +cryptography==49.0.0 +defusedxml==0.7.1 +Pillow==12.3.0 diff --git a/backend/tests/test_api_security.py b/backend/tests/test_api_security.py new file mode 100644 index 0000000..fabdb15 --- /dev/null +++ b/backend/tests/test_api_security.py @@ -0,0 +1,59 @@ +from io import BytesIO +from unittest.mock import patch + +from fastapi import FastAPI +from fastapi.testclient import TestClient +from PIL import Image + +from app.api import system_config, upload + + +def _png_bytes() -> bytes: + buffer = BytesIO() + Image.new("RGB", (8, 8), color=(10, 20, 30)).save(buffer, format="PNG") + return buffer.getvalue() + + +def _upload_client(authenticated: bool) -> TestClient: + app = FastAPI() + app.include_router(upload.router) + if authenticated: + app.dependency_overrides[upload.require_any_role] = lambda: { + "user_id": "a2d4447f-7b67-4304-ad9a-953007e75ef2", + "role": "manager", + } + return TestClient(app) + + +def test_upload_endpoint_requires_bearer_authentication(): + response = _upload_client(authenticated=False).post( + "/upload/image", + files={"file": ("photo.png", _png_bytes(), "image/png")}, + ) + + assert response.status_code == 401 + + +def test_upload_endpoint_validates_and_normalizes_before_storage(): + with patch("app.api.upload.put_validated_image") as put_image: + response = _upload_client(authenticated=True).post( + "/upload/image", + files={"file": ("photo.png", _png_bytes(), "image/png")}, + ) + + assert response.status_code == 200 + object_key = response.json()["object_key"] + assert object_key.startswith("uploads/a2d4447f-7b67-4304-ad9a-953007e75ef2/") + assert object_key.endswith(".jpg") + stored_bytes = put_image.call_args.args[1] + with Image.open(BytesIO(stored_bytes)) as image: + assert image.format == "JPEG" + + +def test_system_configuration_cannot_be_read_without_director_authentication(): + app = FastAPI() + app.include_router(system_config.router) + + response = TestClient(app).get("/system-config") + + assert response.status_code == 401 diff --git a/backend/tests/test_jwt_security.py b/backend/tests/test_jwt_security.py new file mode 100644 index 0000000..a4a39fc --- /dev/null +++ b/backend/tests/test_jwt_security.py @@ -0,0 +1,10 @@ +from app.config import settings +from app.utils.security import create_access_token, decode_token + + +def test_jwt_round_trip_and_tamper_rejection(monkeypatch): + monkeypatch.setattr(settings, "SECRET_KEY", "x" * 32) + token = create_access_token({"user_id": "user-1", "role": "manager"}) + + assert decode_token(token)["user_id"] == "user-1" + assert decode_token(f"{token}tampered") is None diff --git a/docker-compose.yml b/docker-compose.yml index a3ec8df..ed17140 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -13,6 +13,11 @@ services: - "127.0.0.1:5432:5432" volumes: - pgdata:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U $$POSTGRES_USER -d $$POSTGRES_DB"] + interval: 5s + timeout: 5s + retries: 12 minio: image: ${MINIO_IMAGE:?MINIO_IMAGE must be a pinned MinIO version or digest} @@ -41,9 +46,11 @@ services: MINIO_BUCKET: qiji-photos MINIO_SECURE: "false" depends_on: - - postgres - - minio - command: uvicorn app.main:app --host 0.0.0.0 --port 8000 + postgres: + condition: service_healthy + minio: + condition: service_started + command: sh -c "alembic upgrade head && exec uvicorn app.main:app --host 0.0.0.0 --port 8000" volumes: pgdata: diff --git a/整改计划-中国电信研发规范.md b/整改计划-中国电信研发规范.md index 6f82575..d5d7893 100644 --- a/整改计划-中国电信研发规范.md +++ b/整改计划-中国电信研发规范.md @@ -16,8 +16,10 @@ | 关闭不安全企微绑定 | 已完成 | 停用客户端提供 `wecom_userid` 的旧接口，仅保留一次性令牌确认流程 | | 管理配置鉴权 | 已完成 | 系统配置列表接口仅允许 director 访问 | | 上传与日志基础防护 | 已完成（仓库侧） | 上传经后端校验尺寸与图片内容、重编码为 JPEG 后才写入私有对象存储；回调和认证失败日志不再写入正文/token | -| 企微绑定令牌持久化 | 已完成（仓库侧） | 一次性令牌存入数据库并原子消费；启动时创建企微 ID 的部分唯一索引，防止重复绑定 | -| 测试与构建流水线 | 已完成 | 增加上传/配置安全测试和 `.gitlab-ci.yml` 的后端测试、前端构建任务 | +| 企微绑定令牌持久化 | 已完成（仓库侧） | 一次性令牌存入数据库并原子消费；通过 Alembic 迁移创建企微 ID 的部分唯一索引，防止重复绑定 | +| 测试与构建流水线 | 已完成（仓库侧） | 增加上传、配置和 JWT 安全 API 测试；CI 执行编译、Ruff 致命错误检查、pytest、高危 Bandit 扫描、依赖漏洞审计和前端构建 | +| 可追踪数据库变更 | 已完成（仓库侧） | 新增 Alembic 基线迁移；容器启动先执行 `alembic upgrade head`，不再由应用启动时直接改表 | +| 企微回调解析与 JWT 依赖加固 | 已完成（仓库侧） | 使用 defusedxml 解析回调、cryptography 解密、常量时间签名比较；移除存在无修复传递漏洞的 `python-jose`/`ecdsa` 依赖 | | 清理禁止的二进制文件 | 已完成 | 移除仓库根目录两份 `.xlsx` 模板；系统仍可由导入模板接口动态生成模板 | ## 剩余工作与责任边界 @@ -27,10 +29,10 @@ | P0 | 在密钥管理系统创建并注入数据库、MinIO、JWT、Casdoor、企微和 AI 密钥；轮换历史弱凭据 | 运维/安全 | 上线前 | 密钥轮换记录、部署变量清单、无明文密钥扫描报告 | | P0 | 为 PostgreSQL/MinIO 配置私网访问、TLS、最小权限服务账号和备份恢复演练 | 运维/DBA | 上线前 | 网络策略、TLS 证书、账号权限清单、恢复演练记录 | | P1 | 在生产环境接入恶意内容扫描，并确认对象存储桶策略不允许匿名读取 | 后端/安全 | 上线前 | 扫描日志、桶策略和抽样验证 | -| P1 | 在生产数据库验证 `users.wecom_userid` 唯一索引创建成功，并完成并发绑定演练 | 后端/DBA | 上线前 | 数据库迁移日志、并发绑定测试、令牌过期与审计记录 | +| P1 | 在生产数据库执行 Alembic 迁移，验证 `users.wecom_userid` 唯一索引创建成功，并完成并发绑定演练 | 后端/DBA | 上线前 | 数据库迁移日志、并发绑定测试、令牌过期与审计记录 | | P1 | 配置保护分支、MR 至少一名评审、禁止直接推送 `main`/`develop` | Git 平台管理员 | 2 个工作日 | 平台截图/导出配置、MR 审计记录 | -| P2 | 接入 SAST、依赖漏洞/SBOM 和容器镜像扫描，并将高危结果设为流水线阻断条件 | DevSecOps | 5 个工作日 | 流水线报告、阻断规则、漏洞例外审批 | -| P2 | 补齐鉴权、对象访问、企微绑定、文件上传的 API 集成测试；明确测试准入/准出及缺陷闭环 | 测试/后端 | 5 个工作日 | 测试计划、用例、报告、缺陷清单 | +| P2 | 补充 SBOM 和容器镜像扫描，并将高危结果设为流水线阻断条件 | DevSecOps | 5 个工作日 | 流水线报告、阻断规则、漏洞例外审批 | +| P2 | 扩展鉴权、对象访问、企微绑定、文件上传的 API 集成测试覆盖，并明确测试准入/准出及缺陷闭环 | 测试/后端 | 5 个工作日 | 测试计划、用例、报告、缺陷清单 | | P2 | 准备生产发布包：Tag、制品摘要、SBOM、测试报告、变更单、备份与回滚方案、上线验证用例 | 项目经理/运维 | 每次发布 | 发布审批记录和可回放部署记录 | ## 发布执行顺序