feat: #31 DB設計とモデル実装(全6テーブル + ランク計算)#42
Conversation
issue #31で定義された6テーブル(Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree)+暗号化ユーティリティ+ランク計算のテストを先行作成。(TDD RED状態) OAuthテストではFernet暗号化/復号のラウンドトリップ、refresh_token暗号化、 UNIQUE制約違反を網羅。product-specのランク0-9(種子〜世界樹)に準拠。
OAuthトークン保護のためFernet対称暗号化を導入。 - app/core/encryption.py: encrypt_token/decrypt_token(遅延初期化、キー未設定時にValueError) - config.py: ENCRYPTION_KEY設定追加(.env必須) - conftest.py: テスト用暗号化キー設定とキャッシュリセットfixture 独自実装を避け、cryptographyライブラリの標準Fernetを使用(security SKILL準拠)。
…Status) product-specのバッジ5種(Commit/Days/Builder/Writer/Seeker)、 スキルツリー6カテゴリ(Web/AI/Security/Infrastructure/Design/Game)、 クエストステータス3種(NotStarted/InProgress/Completed)を定義。 QuestCategoryはSkillCategoryと同じ6種に統一。
Profile(1:1), OAuthAccount(1:N), Badge(1:N), Quest(独立), QuestProgress(User-Quest中間), SkillTree(1:6)のモデルを追加。 - OAuthAccount: encrypted_access_token/encrypted_refresh_tokenで暗号化保存 - Badge: UNIQUE(user_id, category, tier)で重複バッジ防止 - SkillTree: UNIQUE(user_id, category)で6カテゴリ固定 - User: 全テーブルへのrelationship定義 - base.py: Alembic自動検出用の全モデルimport
Create/Update/Response用スキーマを各テーブルに対して定義。 - OAuthAccountCreate: access_tokenを平文で受け取りCRUD層で暗号化 - OAuthAccount(Response): トークン値を含めないセキュアなレスポンス設計 - BadgeCreate/QuestCreate: Enumを型として受け取りバリデーション
- Profile: get_by_user_id, create, update(exclude_unsetで部分更新) - OAuthAccount: create時にaccess_token/refresh_tokenをFernet暗号化、update_tokens - Badge: award_badge, get_badges_by_user - Quest: get, list_by_category, create - QuestProgress: start_quest, complete_quest, get_quest_progress - SkillTree: initialize_for_user(6カテゴリ一括生成), update_tree, get_by_user_category 全操作でcommit失敗時のrollback処理を実装。
経験値からランク0-9(種子〜世界樹)を算出するロジックを実装。 閾値は暫定値(0,100,300,600,1000,1500,2500,4000,6000,9000)。 update_user_expで経験値加算+ランク再計算を一括処理。 MVP段階ではこの関数で十分、仕様確定後に閾値を調整する想定。
FK依存順にチェーン: 0002: profiles (User 1:1) 0003: oauth_accounts (User 1:N, UNIQUE user_id+provider) 0004: badges (User 1:N, UNIQUE user_id+category+tier) 0005: quests (独立テーブル) 0006: quest_progress (User-Quest中間, UNIQUE user_id+quest_id) 0007: skill_trees (User 1:6, UNIQUE user_id+category) 個別rollbackが可能な設計。autogenerateの結果を手動分割
- pytest-cov: カバレッジ計測のため追加(spec 12.テスト方針準拠)
coding-standards SKILLに準拠し、Pydanticのfield_validatorで入力値検証を追加: - Badge.tier: 1-3(Bronze/Silver/Gold)の範囲チェック - Quest.difficulty: 0-9(ランク: 種子〜世界樹)の範囲チェック - OAuthAccount.provider: githubのホワイトリスト検証(MVPスコープ) - Profile.portfolio_url: HttpUrl型でURL形式検証 HttpUrl→str変換をCRUD層で実施(SQLiteバインディング対応)。 スキーマバリデーションテスト10件追加(合計58テスト全GREEN)。
テストケースの肥大化を防ぐため、個別のスキーマバリデーションテスト ファイル(test_schema_*.py)を単一のtest_validation.pyに統合した。 変更内容: - 削除: test_schema_badge.py, test_schema_quest.py, test_schema_oauth_account.py, test_schema_profile.py - 作成: test_validation.py(4つのテスト関数) - テスト数: 58件 → 51件(妥当性は担保) 検証: - 全51件のテストがパス - ruffリントチェック通過
主キー(PRIMARY KEY)には自動的にインデックスが作成されるため、 明示的に `index=True` を指定すると無駄なインデックスが重複作成される。 変更内容: - User, Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree - `id = Column(Integer, primary_key=True, index=True)` → `id = Column(Integer, primary_key=True)` 検証: - 全51件のテストがパス - ruffリントチェック通過
There was a problem hiding this comment.
Pull request overview
このPRは、Issue #31に基づいて6つの新規テーブル(Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree)とランク計算機能、OAuth トークン暗号化機能を実装したものです。TDD(テスト駆動開発)アプローチを採用し、51件のテストが全てGREENの状態で実装が完了しています。
Changes:
- 6テーブルのモデル・スキーマ・CRUD実装(Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree)
- Fernet対称暗号化によるOAuthトークン保護機能
- 経験値ベースのランク計算ロジック(0-9の10段階)
- Alembicマイグレーション6本(依存関係順に分離、ロールバック可能)
- Pydanticバリデーション(tier 1-3, difficulty 0-9, provider whitelist, HttpUrl)
- 包括的なテストスイート(51件、正常系・異常系・バリデーションをカバー)
Reviewed changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/app/models/*.py | 6つの新規モデルとenums定義、Userモデルへのリレーションシップ追加 |
| backend/app/schemas/*.py | Pydanticスキーマ定義とfield_validatorによる入力検証 |
| backend/app/crud/*.py | CRUD操作実装(暗号化処理、トランザクション管理を含む) |
| backend/app/core/encryption.py | Fernet対称暗号化ユーティリティ |
| backend/app/core/rank.py | ランク計算ロジック(経験値ベース) |
| backend/app/core/config.py | 暗号化キーとランク閾値の設定追加 |
| backend/alembic/versions/000*.py | 6本のマイグレーションファイル(依存順: profiles→oauth_accounts→badges→quests→quest_progress→skill_trees) |
| backend/tests/test_*.py | 51件のテストケース(暗号化、ランク計算、CRUD、バリデーション) |
| backend/pyproject.toml | cryptography ^44.0.0 と pytest-cov ^7.0.0 の依存関係追加 |
| backend/tests/conftest.py | 暗号化キーのテスト環境設定とFernetキャッシュリセット |
| op.create_index(op.f("ix_oauth_accounts_id"), "oauth_accounts", ["id"], unique=False) | ||
| op.create_index(op.f("ix_oauth_accounts_user_id"), "oauth_accounts", ["user_id"], unique=False) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| op.drop_index(op.f("ix_oauth_accounts_user_id"), table_name="oauth_accounts") | ||
| op.drop_index(op.f("ix_oauth_accounts_id"), table_name="oauth_accounts") |
There was a problem hiding this comment.
主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_oauth_accounts_id インデックスは不要です。DB容量削減のため削除を推奨します。
| op.create_index(op.f("ix_oauth_accounts_id"), "oauth_accounts", ["id"], unique=False) | |
| op.create_index(op.f("ix_oauth_accounts_user_id"), "oauth_accounts", ["user_id"], unique=False) | |
| def downgrade() -> None: | |
| op.drop_index(op.f("ix_oauth_accounts_user_id"), table_name="oauth_accounts") | |
| op.drop_index(op.f("ix_oauth_accounts_id"), table_name="oauth_accounts") | |
| op.create_index(op.f("ix_oauth_accounts_user_id"), "oauth_accounts", ["user_id"], unique=False) | |
| def downgrade() -> None: | |
| op.drop_index(op.f("ix_oauth_accounts_user_id"), table_name="oauth_accounts") |
| op.create_index(op.f("ix_badges_id"), "badges", ["id"], unique=False) | ||
| op.create_index(op.f("ix_badges_user_id"), "badges", ["user_id"], unique=False) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| op.drop_index(op.f("ix_badges_user_id"), table_name="badges") | ||
| op.drop_index(op.f("ix_badges_id"), table_name="badges") |
There was a problem hiding this comment.
主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_badges_id インデックスは不要です。DB容量削減のため削除を推奨します。
| op.create_index(op.f("ix_badges_id"), "badges", ["id"], unique=False) | |
| op.create_index(op.f("ix_badges_user_id"), "badges", ["user_id"], unique=False) | |
| def downgrade() -> None: | |
| op.drop_index(op.f("ix_badges_user_id"), table_name="badges") | |
| op.drop_index(op.f("ix_badges_id"), table_name="badges") | |
| op.create_index(op.f("ix_badges_user_id"), "badges", ["user_id"], unique=False) | |
| def downgrade() -> None: | |
| op.drop_index(op.f("ix_badges_user_id"), table_name="badges") |
| def _get_fernet() -> Fernet: | ||
| """Fernetインスタンスを遅延初期化して返す。 | ||
|
|
||
| ENCRYPTION_KEYが未設定の場合はValueErrorを送出する。 | ||
| """ | ||
| global _fernet # noqa: PLW0603 | ||
| if _fernet is None: | ||
| key = settings.ENCRYPTION_KEY | ||
| if not key: | ||
| raise ValueError( | ||
| "ENCRYPTION_KEY is not set. " | ||
| "Generate one with: python -c \"from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())\"" | ||
| ) | ||
| _fernet = Fernet(key.encode()) | ||
| return _fernet |
There was a problem hiding this comment.
グローバル変数_fernetの初期化とリセットがスレッドセーフではありません。複数のリクエストが同時に_get_fernet()を呼び出した場合、競合状態(race condition)が発生する可能性があります。
マルチスレッド環境(uvicorn等)では、threading.Lockを使用して初期化を保護することを推奨します:
import threading
_fernet: Fernet | None = None
_fernet_lock = threading.Lock()
def _get_fernet() -> Fernet:
global _fernet
if _fernet is None:
with _fernet_lock:
if _fernet is None: # Double-checked locking
key = settings.ENCRYPTION_KEY
if not key:
raise ValueError(...)
_fernet = Fernet(key.encode())
return _fernet| def validate_encryption_key() -> None: | ||
| """ENCRYPTION_KEYが設定されているかを検証する。 | ||
|
|
||
| 未設定の場合はValueErrorを送出する。 | ||
| テスト環境ではダミーキーが自動設定されるため、本番環境のみチェック。 | ||
| """ | ||
| if not settings.ENCRYPTION_KEY: | ||
| raise ValueError( | ||
| "ENCRYPTION_KEY is not set. " | ||
| "Generate one with: python -c \"from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())\"" | ||
| ) |
There was a problem hiding this comment.
validate_encryption_key()関数が定義されていますが、アプリケーション起動時(main.py)やテスト初期化時に呼び出されていません。
本番環境でENCRYPTION_KEYが未設定のまま起動すると、初回のOAuthトークン暗号化時にエラーが発生します。これは実行時エラーとなり、デバッグが困難です。
アプリケーション起動時に検証を実行することを推奨します:
# backend/app/main.py
from app.core.config import settings, validate_encryption_key
# 本番環境のみ検証(テストではconftest.pyでENVが設定される)
import os
if os.getenv("ENV", "development") == "production":
validate_encryption_key()| id = Column(Integer, primary_key=True) | ||
| user_id = Column(Integer, ForeignKey("users.id"), nullable=False, index=True) | ||
|
|
||
| category = Column(String, nullable=False) |
There was a problem hiding this comment.
モデルのcategoryフィールドがString型として定義されていますが、スキーマではBadgeCategory Enumを使用しています。この不一致により、DB層とアプリケーション層の間で型の整合性が保証されません。
CRUDレイヤーで.valueを使用して文字列に変換していますが(例: badge.py:16)、モデル側でSQLAlchemy のEnum型を使用することで型安全性を向上できます:
from sqlalchemy import Enum as SAEnum
from app.models.enums import BadgeCategory
category = Column(SAEnum(BadgeCategory), nullable=False)ただし、現在の実装でも動作に問題はないため、オプショナルな改善提案です。
| op.create_index(op.f("ix_quests_id"), "quests", ["id"], unique=False) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| op.drop_index(op.f("ix_quests_id"), table_name="quests") |
There was a problem hiding this comment.
主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_quests_id インデックスは不要です。DB容量削減のため削除を推奨します。
| op.create_index(op.f("ix_quests_id"), "quests", ["id"], unique=False) | |
| def downgrade() -> None: | |
| op.drop_index(op.f("ix_quests_id"), table_name="quests") | |
| def downgrade() -> None: |
| sa.PrimaryKeyConstraint("id"), | ||
| sa.UniqueConstraint("user_id", "quest_id", name="uq_progress_user_quest"), | ||
| ) | ||
| op.create_index(op.f("ix_quest_progress_id"), "quest_progress", ["id"], unique=False) |
There was a problem hiding this comment.
主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_quest_progress_id インデックスは不要です。DB容量削減のため削除を推奨します。
| op.create_index(op.f("ix_skill_trees_id"), "skill_trees", ["id"], unique=False) | ||
| op.create_index(op.f("ix_skill_trees_user_id"), "skill_trees", ["user_id"], unique=False) | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees") | ||
| op.drop_index(op.f("ix_skill_trees_id"), table_name="skill_trees") |
There was a problem hiding this comment.
主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_skill_trees_id インデックスは不要です。DB容量削減のため削除を推奨します。
| op.create_index(op.f("ix_skill_trees_id"), "skill_trees", ["id"], unique=False) | |
| op.create_index(op.f("ix_skill_trees_user_id"), "skill_trees", ["user_id"], unique=False) | |
| def downgrade() -> None: | |
| op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees") | |
| op.drop_index(op.f("ix_skill_trees_id"), table_name="skill_trees") | |
| op.create_index(op.f("ix_skill_trees_user_id"), "skill_trees", ["user_id"], unique=False) | |
| def downgrade() -> None: | |
| op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees") |
| def update_profile(db: Session, profile_id: int, profile_in: ProfileUpdate) -> Profile: | ||
| db_profile = db.query(Profile).filter(Profile.id == profile_id).first() | ||
| if db_profile is None: | ||
| raise ValueError(f"Profile with id={profile_id} not found") | ||
| update_data = profile_in.model_dump(exclude_unset=True) | ||
| # HttpUrlをstrに変換 | ||
| if "portfolio_url" in update_data and update_data["portfolio_url"]: | ||
| update_data["portfolio_url"] = str(update_data["portfolio_url"]) | ||
| for field, value in update_data.items(): | ||
| setattr(db_profile, field, value) | ||
| try: | ||
| db.commit() | ||
| except Exception: | ||
| db.rollback() | ||
| raise | ||
| db.refresh(db_profile) | ||
| return db_profile |
There was a problem hiding this comment.
CRUD操作のValueErrorが発生するケース(存在しないレコードの更新等)のテストが不足しています。
例えば、以下のケースのテストを追加することを推奨します:
update_profile()で存在しないprofile_idを指定した場合update_skill_tree()で存在しないuser_id/categoryを指定した場合complete_quest()で存在しないquest_idを指定した場合
テスト例:
def test_update_profile_not_found(db):
with pytest.raises(ValueError, match="Profile with id=999 not found"):
update_profile(db, 999, ProfileUpdate(github_username="test"))現在51件のテストがGREENですが、エラーケースのカバレッジを追加することで、より堅牢なコードになります。
| def test_initialize_skill_trees_for_user(db): | ||
| """ユーザー作成時に6カテゴリ全てのSkillTreeが生成される""" | ||
| user = create_user(db, UserCreate(username="skill_tree_user")) | ||
| trees = initialize_skill_trees_for_user(db, user.id) | ||
|
|
||
| assert len(trees) == 6 | ||
| categories = {t.category for t in trees} | ||
| assert categories == { | ||
| SkillCategory.WEB, | ||
| SkillCategory.AI, | ||
| SkillCategory.SECURITY, | ||
| SkillCategory.INFRASTRUCTURE, | ||
| SkillCategory.DESIGN, | ||
| SkillCategory.GAME, | ||
| } | ||
| for tree in trees: | ||
| assert tree.tree_data == {} | ||
| assert tree.generated_at is None | ||
|
|
There was a problem hiding this comment.
テストでinitialize_skill_trees_for_userが正しく動作することは確認されていますが、実際のユーザー作成フロー(create_user)でこの関数が自動的に呼び出されることが確認されていません。
Issue #31の要件「SkillTree初期化: ユーザー作成時6カテゴリ自動生成」を完全に満たすには、create_user関数内でinitialize_skill_trees_for_userを自動的に呼び出す実装が必要です。
テストケースの追加を推奨します:
def test_create_user_initializes_skill_trees(db):
"""ユーザー作成時にSkillTreeが自動生成されることを確認"""
user = create_user(db, UserCreate(username="auto_tree_user"))
trees = db.query(SkillTree).filter(SkillTree.user_id == user.id).all()
assert len(trees) == 6be74f1c to
2d1096d
Compare
GitHub Copilotのレビュー指摘に基づき、以下の修正を実施:
1. Alembicマイグレーション - 主キーの冗長インデックス削除
- 0002, 0003, 0004のマイグレーションから`ix_*_id`削除
- PRIMARY KEY制約は自動的にインデックスを作成するため不要
2. encryption.py - スレッドセーフ対応
- threading.Lock追加、double-checked lockingパターン実装
- マルチスレッド環境での競合状態を防止
3. main.py - 本番環境での暗号化キー検証追加
- ENV=productionの場合のみvalidate_encryption_key()を実行
- テスト環境ではconftest.pyでダミーキー設定済み
4. skill_tree.py (model) - tree_dataデフォルト値修正
- lambda: {}をnullable=Falseに変更
- Pydanticスキーマ層でデフォルト値を設定
5. skill_tree.py (crud) - IntegrityError個別処理追加
- initialize_skill_trees_for_user()にIntegrityError処理追加
- UNIQUE制約違反時に明確なエラーメッセージを提供
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
backend/alembic/versions/0007_create_skill_trees_table.py:37
- 0007 の downgrade() が index を落とすだけで skill_trees テーブル自体を drop していません。ロールバック不能になるので、op.drop_table("skill_trees") まで含めて完全に元に戻せるようにしてください。
def downgrade() -> None:
op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees")
backend/app/api/admin.py
Outdated
| def verify_admin_key(x_admin_key: str = Header(...)) -> None: | ||
| """管理APIキーを検証する | ||
|
|
||
| リクエストヘッダー `X-Admin-Key` と環境変数 `ADMIN_API_KEY` を照合。 | ||
| 一致しない場合は401 Unauthorizedを返す。 | ||
| """ | ||
| if not settings.ADMIN_API_KEY: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| detail="ADMIN_API_KEY is not configured on server", | ||
| ) | ||
| if x_admin_key != settings.ADMIN_API_KEY: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_401_UNAUTHORIZED, | ||
| detail="Invalid admin API key", | ||
| ) |
There was a problem hiding this comment.
verify_admin_key が Header(...) で必須ヘッダーになっているため、ヘッダー未指定時に 401 ではなく 422 が返ります。認証として扱うなら Header(None) にして未指定/不一致ともに 401 を返すようにし、比較は secrets.compare_digest を使ってタイミング差分も抑える実装にしてください。
backend/app/main.py
Outdated
| from app.core.config import settings, validate_encryption_key | ||
| from app.api.admin import admin_app | ||
|
|
||
| # 本番環境のみ暗号化キーの検証を実行(テストではconftest.pyでENVが設定される) |
There was a problem hiding this comment.
コメントに「テストではconftest.pyでENVが設定される」とありますが、conftest.py で設定しているのは ENCRYPTION_KEY で、ENV は設定されていません。誤解を招くので記述を実態に合わせて修正してください。
| # 本番環境のみ暗号化キーの検証を実行(テストではconftest.pyでENVが設定される) | |
| # 本番環境のみ暗号化キーの検証を実行(テストでは conftest.py で ENCRYPTION_KEY が設定される) |
| sa.Column("id", sa.Integer(), nullable=False), | ||
| sa.Column("user_id", sa.Integer(), nullable=False), | ||
| sa.Column("category", sa.String(), nullable=False), | ||
| sa.Column("tree_data", sa.JSON(), nullable=True), |
There was a problem hiding this comment.
tree_data が nullable=True で作成されていますが、SQLAlchemy モデル側は tree_data を nullable=False にしており前提が不一致です。DB側も NOT NULL(必要ならデフォルト {} を持たせる)に合わせて、アプリとスキーマの整合性を取ってください。
| sa.Column("tree_data", sa.JSON(), nullable=True), | |
| sa.Column("tree_data", sa.JSON(), nullable=False, server_default=sa.text("'{}'")), |
backend/app/crud/user.py
Outdated
| db.commit() | ||
| db.refresh(db_user) | ||
| # ユーザー作成時に6カテゴリのSkillTreeを自動初期化 | ||
| initialize_skill_trees_for_user(db, db_user.id) |
There was a problem hiding this comment.
create_user() が users への commit 後に SkillTree 初期化を別トランザクションで行っており、SkillTree 側で失敗すると「ユーザーだけ作成され SkillTree が欠落した状態」が確定します。User 作成と SkillTree 6件生成は同一トランザクションで原子的に行えるよう、commit を1回にまとめる(flushで user_id を確定してから SkillTree を add し最後に commit する、もしくは db.begin() を使う)構成にしてください。
| db.commit() | |
| db.refresh(db_user) | |
| # ユーザー作成時に6カテゴリのSkillTreeを自動初期化 | |
| initialize_skill_trees_for_user(db, db_user.id) | |
| # user.id を確定させるが、まだトランザクションはコミットしない | |
| db.flush() | |
| # ユーザー作成時に6カテゴリのSkillTreeを自動初期化 | |
| initialize_skill_trees_for_user(db, db_user.id) | |
| # ユーザーと SkillTree 初期化をまとめて 1 回の commit で確定させる | |
| db.commit() | |
| db.refresh(db_user) |
| # Admin API Access Control | ||
| ADMIN_API_KEY: str = "test" # 本番では必ず.envで設定すること | ||
|
|
There was a problem hiding this comment.
ADMIN_API_KEY のデフォルト値が "test" になっているため、環境変数を設定し忘れても管理APIが既知キーで有効化されてしまいます。デフォルトは空文字(または None)にして本番起動時に必須検証する(ENCRYPTION_KEY と同様に validate_* を用意する)形に変更してください。
| # ENCRYPTION_KEYの検証(アプリ起動時にチェック) | ||
| def validate_encryption_key() -> None: | ||
| """ENCRYPTION_KEYが設定されているかを検証する。 | ||
|
|
||
| 未設定の場合はValueErrorを送出する。 | ||
| テスト環境ではダミーキーが自動設定されるため、本番環境のみチェック。 | ||
| """ | ||
| if not settings.ENCRYPTION_KEY: | ||
| raise ValueError( | ||
| "ENCRYPTION_KEY is not set. " | ||
| "Generate one with: python -c \"from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())\"" | ||
| ) |
There was a problem hiding this comment.
validate_encryption_key() が「未設定かどうか」しか見ていないため、形式不正のキーでも起動時検知できず、実際の暗号化/復号時に例外になります。ここで Fernet(key.encode()) を試して例外を握りつぶさず明示的に ValueError にする等、キー形式の検証も追加してください。
issue #31実装における主要な技術的意思決定を記録: 1. ADR 005: OAuth トークン暗号化戦略 - Fernet対称暗号 + スレッドセーフ対応を採用 - 独自実装、KMS、Vaultなどの代替案を検討・却下 - IPA基準準拠、DB漏洩時の二次被害防止 2. ADR 006: 管理用API認証設計 - API Key認証(環境変数)+ 独立FastAPI appを採用 - DBテーブル、JWT、認証なしなどの代替案を検討・却下 - ハッカソン期間中の優先度と責務分離を考慮
issue #31実装における技術的意思決定を記録: - ADR 007: SkillTree初期化タイミング - ユーザー作成時に自動初期化を採用 - 遅延初期化、手動初期化、CRON初期化などの代替案を検討・却下 - ADR 003(トランザクション管理はCRUD関数内で完結)に準拠 - LangChain連携時のオーバーヘッド削減、仕様準拠を優先
GitHub Copilotの再レビュー指摘6件に対応:
1. admin.py - 認証エラーを401に統一、secrets.compare_digest使用
- Header(None)でヘッダー未指定時も401を返すように修正
- タイミング攻撃対策としてsecrets.compare_digestを使用
2. main.py - コメント修正(ENV→ENCRYPTION_KEY)
- conftest.pyで設定されるのはENCRYPTION_KEYであることを明記
3. 0007マイグレーション - tree_dataをNOT NULL + デフォルト値
- nullable=False, server_default='{}'でモデルとの整合性を確保
4. user.py - db.flush()で同一トランザクション化(重要)
- User作成とSkillTree初期化を1つのトランザクションで実行
- SkillTree失敗時にUserも作成されない(原子性保証)
5. config.py - ADMIN_API_KEYデフォルト値を空文字に変更
- デフォルト"test"はセキュリティリスクのため削除
6. config.py - validate_encryption_key()にキー形式検証追加
- Fernet(key.encode())で形式不正のキーを起動時に検出
issue #31実装における技術的意思決定を記録: - ADR 008: Alembicマイグレーション粒度 - テーブル単位でマイグレーションを分離(6本) - 一括マイグレーション、機能単位グループ化などの代替案を検討・却下 - ロールバック粒度の細かさ、デバッグ容易性を優先
- User.skillsカラム削除(SkillTreeテーブルに移行) - マイグレーションを7本から1本に統合(issue単位一括) - ADR 008更新: MVP優先でissue単位一括マイグレーションに変更 理由: - SkillTreeテーブル実装によりskillsカラムは冗長 - 部分的ロールバックの必要性が薄く、all-or-nothingが適切 - マイグレーションの本質(環境同期)に集中 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
プロジェクト全体のDB設計方針を明確化するため、ADR 009を作成: - 正規化の程度: 第3正規形(3NF)を基本とし、パフォーマンスのためのキャッシュカラムは許容 - 型選択基準: JSON vs JSONB(SkillTreeはJSON型 = 全体読み込みのため)、Enum vs 整数、Text vs String - 外部キー制約: すべてのFK関係に外部キー制約を使用、ON DELETE CASCADEは使用しない - インデックス戦略: 主キー、外部キー、UNIQUE制約のみ - SQLite vs PostgreSQL互換性: JSON型使用、ORM中心でDB固有機能を避ける - NULL許容とデフォルト値、タイムスタンプ管理の方針
## 概要 ユーザーが学びたい技術のドキュメントを入力すると、 LLMを使用してランクに見合った実践的なハンズオン演習を自動生成する機能を実装。 概念実証を優先し、まずは動くことを重視。 ## 実装内容 ### 1. プロンプトテンプレート拡張 - `app/core/prompts.py`: QUEST_GENERATION_TEMPLATEを詳細化 - ランク別の難易度設定ガイドライン - JSON出力形式の厳密な指定 - ステップ構成の要件定義 ### 2. サービス層実装 - `app/services/quest_service.py`: generate_handson_quest関数 - LLM呼び出し (temperature=0.7で創造性を持たせる) - JSONパース+エラーハンドリング - デフォルト値フォールバック ### 3. スキーマ定義拡張 - `app/schemas/quest.py`: QuestGenerationRequest/Response追加 - QuestStep: 演習ステップの詳細定義 - リクエストバリデーション (document_content: 10KB制限) - Swagger UIのExample付き ### 4. APIエンドポイント実装 - `app/api/endpoints/quest.py`: POST /api/v1/quest/generate - リクエスト受け取り - サービス呼び出し - エラーハンドリング ### 5. ルーター登録 - `app/api/api.py`: questルーターを統合 ### 6. テスト実装 - `tests/test_services/test_quest_service.py`: サービス層テスト - `tests/test_api/test_quest.py`: APIエンドポイントテスト - LLM呼び出しをモック化してCI環境対応 - 正常系・エラー系・バリデーションをカバー ## MVP制限(概念実証優先) - ❌ DB保存なし: 生成された演習はAPIレスポンスのみ - ❌ 進捗管理なし: 演習の完了状態やスコアは後続Issue -⚠️ 生成品質: プロンプト調整は後続Issue ## セキュリティ考慮 - Request Bodyサイズ制限(document_content: 10KB) - LLMレスポンスのサニタイズ(JSONパースエラー時のフォールバック) - エラーメッセージの安全な返却 ## テスト要件 - モック使用でAPI キー不要(CI環境対応) - 正常系・エラー系・バリデーションを網羅 ## 完了条件(DoD) - [x] POST /api/v1/quest/generate実装完了 - [x] スキーマ定義とバリデーション実装 - [x] テスト5件以上実装(モック使用) - [x] エラーハンドリング実装 ## 依存 - Issue #32(AI基盤セットアップ)✅ - Issue #36(ランク判定AI)✅ ## 次のステップ(後続Issue) - Issue #41: 演習のDB保存機能 - Issue #42: 演習進捗管理機能 - Issue #43: 演習生成品質向上 - Issue #44: 生成コードのセキュリティチェック
Issue: #31
実装の概要
issue #31で定義された6テーブルとランク計算機能、および管理用APIを実装しました。
実装内容
🔧 技術的な意思決定とトレードオフ (最重要)
採用したアプローチ
1. TDD(テスト駆動開発)
2. Fernet対称暗号によるトークン暗号化(スレッドセーフ対応)
cryptography.fernet.Fernet+ 環境変数ENCRYPTION_KEY、threading.Lockによるdouble-checked locking3. 主キーの冗長なインデックス削除
index=Trueを削除4. SkillTree自動初期化
create_user()内で6カテゴリのSkillTreeを自動生成5. 管理用API(独立認証・Swagger UI)
/admin/docs)6. 本番環境での起動時検証
ENV=productionの場合のみvalidate_encryption_key()を実行却下したアプローチ(代替案)
1. 独自実装の暗号化アルゴリズム
2. テスト肥大化を許容
3. マイグレーション1本化
🧪 テスト戦略と範囲
追加したテストケース
正常系
異常系
テストしていないこと
セキュリティに関する自己評価
機密情報のハードコードはないか
ENCRYPTION_KEYによる暗号化ADMIN_API_KEYで管理.env管理、リポジトリに含めないTEST_ENCRYPTION_KEYとして明示(conftest.py)入力値の検証(バリデーション)は行っているか
field_validatorによる入力検証Badge.tier: 1-3範囲チェックQuest.difficulty: 0-9範囲チェックOAuthAccount.provider: ホワイトリスト(github)Profile.portfolio_url: HttpUrl型による形式検証User.rank: 経験値との整合性検証(model_validator)既知の脆弱性パターンへの対策は考慮したか
threading.LockによるFernetインスタンス初期化の競合状態防止レビュワー(人間)への申し送り事項
SwaggerUI確認
GitHub Copilot指摘対応済み
_get_fernet()のスレッドセーフ対応(threading.Lock + double-checked locking)tree_dataのデフォルト値修正(モデル層から削除、Pydanticで設定)テスト実行結果
注意点
.envで設定すること(32バイトのFernet互換base64キー)crud.skill_tree.initialize_skill_trees_for_user)/admin/docsはX-Admin-Key認証が必要(通常の/docsとは独立)補足
Closes #31