Files
CFDivePlatform/docs/analysis/2026-10-14-project-review-todo.md
T
2026-10-17 23:06:28 +08:00

20 KiB
Raw Blame History

CFDivePlatform 專案審視:高/中優先待辦

審視日期:2026-10-14 範圍:後端授權、部署設定、程式結構、資料一致性。 低優先項目未列入本文(分頁、範本殘留檔、CHANGELOG 等),需要時另行整理。 搭配閱讀:《2026-06-11-spec-implementation-audit.md》。 實作原則:以下每一項只要涉及行為變更,都必須先走 OpenSpec(/opsx:propose),AC 對應測試後才可實作。 更新紀錄:2026-10-14 新增 M7、M8(撰寫 hardener skill 時,閱讀預約相關程式碼發現;以程式碼閱讀判斷,尚未重現驗證);2026-10-17 新增「功能與延後項目」F1–F7(彙整既有規劃文件與已歸檔 change 的未完成任務)。


總覽

# 優先 項目 是否走 OpenSpec 建議 change 名稱
H1 🔴 高 member/provider 路由缺角色 middleware 是 add-role-middleware
H2 🔴 高 正式環境 APP_DEBUG 與預設值風險 否(設定確認)+ 視情況改 .env.example —
M1 🟡 中 AuthController 過大、無 FormRequest/Service 層 是 refactor-auth-controller
M2 🟡 中 預約名額邏輯重複 是 extract-booking-service
M3 🟡 中(已下修,見內文) 通知派送的例外處理與失敗可觀測性 視範圍而定 harden-notification-dispatch
M4 🟡 中 前端 token 儲存不一致(admin 用 localStorage) 是 unify-frontend-token-storage
M5 🟡 中 CORS 在正式環境仍允許 localhost 是(小) restrict-cors-by-env
M6 🟡 中 CI 使用 SQLite,鎖定/超賣測試可能假綠 是 ci-mysql-test-job
M7 🟡 中 預約流程 catch (RuntimeException) 把例外訊息直接回給使用者 是(小) fix-booking-exception-handling
M8 🟡 中(影響資料正確性,需並發才觸發) 預約取消/確認的狀態在鎖之前讀取,並發下名額可能重複增減 是 fix-booking-state-race(建議併入 M2)

建議順序:H1 → H2 → M7 → M6 → M5 → M8 與 M2 合併處理 → M1 → M4 → M3(M3 已下修,可視驗證結果延後或關閉)。 理由:H1 是安全漏洞;M7 範圍小、可立即降低資訊洩漏;M6 先建立可信的測試基礎,後續 M8、M2、M1 才有安全網;M8 的修法(鎖內重讀狀態)本來就是 M2 抽出 BookingService 時要統一的邏輯,建議同一個 change 處理。


H1 member/provider 路由缺角色 middleware

現況 routes/api.php 中 member 與 provider 兩個群組只有 auth:sanctum,沒有角色檢查;只有 admin 群組有 EnsureAdmin。

已確認的影響

  • 任何已登入的 member 或 admin 可呼叫 POST /api/provider/offers、/provider/schedules、/provider/offers/{id}/images,provider_id 會被設成呼叫者 id,產生非教練帳號的課程。
  • 任何 provider 或 admin 可呼叫 POST /api/member/bookings 建立預約。
  • 目前只有 ReviewController、refresh 端點自己檢查角色,其餘端點無防護。

待辦

  • 新增 EnsureMember、EnsureProvider middleware(仿 app/Http/Middleware/EnsureAdmin.php),並在 bootstrap/app.php 註冊 alias
  • 套用到 member、provider 路由群組
  • 通知、即時訊息這類「member + provider 共用」群組維持現狀,但需確認 admin 是否應被排除
  • 新增 Feature 測試(仿 tests/Feature/AdminEndpointAuthTest.php):每個角色 × 每組路由,期望 403
  • 更新 spec:provider-auth、admin-auth;視需要新增 role-authorization spec
  • 更新 Swagger 註解(app/Docs/*)的 403 回應

驗收條件(草案,需轉為 spec Scenario)

  1. member 呼叫任一 /api/provider/* 回 403
  2. provider 呼叫任一 /api/member/* 回 403
  3. admin 呼叫 /api/member/*、/api/provider/* 回 403
  4. 既有合法角色的行為不變(既有測試全綠)

安全確認(CLAUDE.md 必答)

  • 權限檢查層級:middleware(路由層),Controller 內既有所有權檢查保留
  • 未授權觸發可能性:修正後不可

H2 正式環境 APP_DEBUG 與預設值

現況

  • 本機 .env:APP_ENV=local、APP_DEBUG=true
  • .env.example:APP_DEBUG=true、APP_ENV=local
  • docker-compose.yml 以 env_file: .env 注入容器

風險 若 VPS 上 .env 沿用相同值,錯誤頁會洩漏堆疊追蹤、環境變數與 SQL。

待辦

  • 需要你確認:登入 VPS 檢查實際 .env 的 APP_DEBUG、APP_ENV(我無法從此環境確認)
  • 確認 config:cache 後的實際值:docker compose exec app php artisan tinker --execute="dump(config('app.debug'));"
  • .env.example 加註解說明正式環境必須 APP_DEBUG=false
  • 評估是否在 deploy.yml 加入啟動檢查:APP_ENV=production 且 APP_DEBUG 為 false,否則部署失敗
  • 本機 .env 中的 DB 密碼為弱密碼(cfdivepass),確認正式環境未使用

備註 這項大多是設定確認,不一定需要 OpenSpec change;若加入部署檢查則屬 env-cloud-annotations spec 範圍。


M1 AuthController 過大,缺 FormRequest 與 Service 層

現況

  • app/Http/Controllers/API/AuthController.php 1136 行
  • 三種角色的 register/login/logout/profile/change-password/refresh 大量複製
  • app/Http/Requests、app/Services 目錄不存在,驗證與業務邏輯都在 Controller
  • 已有 commonLogout、commonChangePassword 等雛形,但 refreshMember/refreshProvider/refreshAdmin 幾乎相同

違反的 CLAUDE.md 規則 單一職責、Function 超過 20 行需說明、Controller 只處理資源、參數過多應封裝 DTO。

待辦

  • 先補足現有行為的測試(AuthLoginTest、AuthLockoutTest、TokenRefreshTest 已有基礎),重構前先確認覆蓋率
  • 拆分:依角色或依功能拆成多個 Controller(例如 MemberAuthController),共用邏輯抽成 AuthService
  • 引入 FormRequest(RegisterMemberRequest 等)
  • 合併三個 refresh 方法
  • 重構期間 API 路徑、回應格式、Swagger 輸出不得改變
  • 重構完成後 app/Docs/AuthApiDoc.php(1261 行)註解位置需一併檢視

風險 改動面大,務必在 M6(CI MySQL)之後進行,並分多個小 PR。


M2 預約名額邏輯重複

現況 「取消後遞減 current_participants,若由 Full 變回 Open」的邏輯同時出現在:

  • MemberBookingController::destroy
  • ProviderBookingController::cancel
  • AdminBookingController(需確認)

同時 MemberBookingController::store 約 70 行,包含驗證、鎖定、建立、通知。

待辦

  • 抽出 BookingService(或 CourseSchedule 的 releaseSpots() 方法)集中名額增減與狀態轉換
  • 確認所有取消路徑使用同一入口
  • 保留 BookingOversellTest、BookingLifecycleTest 全綠
  • 補單元測試:名額釋放、Full → Open 的邊界

M3 通知派送的例外處理與失敗可觀測性

2026-10-14 更新:初版誤判為「通知同步發送」。實際檢查後,9 個 Notification 中 8 個已實作 ShouldQueue 並設有 public int $tries = 3,cfdive-queue 容器也已在跑。原本的風險大幅降低,本項由「中」下修,並縮小範圍。

現況

  • BookingCreated、BookingConfirmed、BookingRejected、BookingCancelled、BookingCompleted、ProviderVerificationApproved、ProviderVerificationRejected、ReviewReceived:皆為 ShouldQueue,tries = 3
  • NewBookingMessageNotification:未實作 ShouldQueue,檔案內註解說明是刻意同步,讓後續的 NotificationCreated broadcast 能立刻讓前端拉到正確的未讀數
  • MemberBookingController、ProviderBookingController 仍以 try { notify() } catch (\Throwable) { Log::error } 包住派送

仍存在的風險

  • try/catch 現在只會攔到「派送入列」階段的失敗(例如 Redis 無法連線),這可能是刻意讓預約流程不因通知失敗而中斷;但例外只進 log,不會送 Sentry,也沒有補送機制
  • 8 個 Notification 都只設了 tries,沒有 backoff,重試會立刻連續發生,對暫時性故障(SMTP、Redis 抖動)幫助有限
  • 沒有 failed() 處理:重試用完後只留在 failed_jobs,是否會送到 Sentry 我不確定,需要驗證
  • NewBookingMessageNotification 同步發送,聊天頻繁時會增加請求延遲(且有 Redis broadcast 依賴)

待辦

  • 先驗證:讓一個 Notification job 失敗,確認 Sentry 是否收到;確認 failed_jobs 是否有人監看
  • 為 8 個 queued Notification 補上 backoff(例如 [10, 60, 300])
  • 將 Controller 內的 Log::error 改為 report($e),讓 Sentry 收到派送失敗,並保留「不中斷主流程」的行為
  • 評估 NewBookingMessageNotification 是否能在不破壞未讀數即時性的前提下改為 queued(可能需調整 broadcast 時序),不可行則在 spec 註明為刻意設計
  • 更新 NotificationTriggerTest 涵蓋失敗情境
  • 如行為有變更,更新 spec:notification-core、notification-triggers、notification-email

備註 若驗證後發現 Sentry 已能收到失敗,且 Redis 故障非實際風險,本項可降為低優先或直接關閉。


M4 前端 token 儲存不一致

現況

  • member:sessionStorage(frontend/src/api/axios.js)
  • admin:localStorage(frontend/src/api/adminAxios.js)
  • openspec/config.yaml 仍寫 localStorage(見另一份 prompt 文件)

風險 管理員權限最高,卻用持久儲存,XSS 時外洩影響更大。好消息是前端目前沒有 v-html 或 innerHTML。

待辦

  • 確認 coach 端(coachAxios.js)實際使用哪種儲存
  • 決定統一策略(至少 admin 改為 sessionStorage)
  • 評估是否改用 HttpOnly Cookie(需搭配 Sanctum SPA 模式,改動大,另案評估)
  • 更新 member-portal-ui、coach-portal-ui、admin-panel-ui spec 中關於儲存的描述

M5 CORS 在正式環境仍允許 localhost

現況 config/cors.php 的 allowed_origins 固定包含 http://localhost:5173 與 http://127.0.0.1:5173。

待辦

  • 改為依 APP_ENV 決定:local 才加入 localhost,production 僅允許 FRONTEND_URL
  • 補一個 Feature 測試驗證 production 設定下的 CORS 標頭
  • 同步檢查 Reverb(WebSocket)的 allowed origins 設定

M6 CI 使用 SQLite,鎖定/超賣測試可能假綠

現況

  • .gitea/workflows/test.yml 以 cp .env.example .env 執行,.env.example 為 DB_CONNECTION=sqlite
  • BookingOversellTest 等測試依賴 lockForUpdate(),SQLite 不支援行鎖,該方法在 SQLite 上實質無效
  • 另外 SQLite 與 MySQL 在 enum、JSON、時間函式上行為不同

風險 CI 綠燈不代表正式環境(MySQL)行為正確,尤其是併發情境。

待辦

  • 在 CI 增加 MySQL service,至少讓預約、超賣相關測試在 MySQL 上跑一次
  • 檢查 phpunit.xml 的 DB 設定,確認 CI 與本機一致
  • 評估 BookingOversellTest 是否真的測到併發(單程序測試無法驗證行鎖,可能需要多程序測試或接受其限制並在文件註明)
  • deploy.yml 與 test.yml 目前互相獨立;評估是否讓部署依賴測試通過
  • 更新 spec:feature-test-coverage 或新增 ci-pipeline spec

M7 預約流程 catch (RuntimeException) 把例外訊息直接回給使用者

來源:撰寫 hardener skill 時閱讀程式碼發現。以下為程式碼閱讀結果,尚未實際觸發驗證。

現況

  • app/Http/Controllers/API/MemberBookingController.php:92(store)與 app/Http/Controllers/API/ProviderBookingController.php:57(confirm)都是: catch (\RuntimeException $e) { return response()->json(['status' => false, 'message' => $e->getMessage()], 422); }
  • 閉包內刻意拋出的業務訊息(「名額不足」「您已預約此時段」)依賴這個 catch 轉成 422

風險

  • Illuminate\Database\QueryException 繼承 PDOException,而 PDOException 是 RuntimeException 的子類別。交易內的資料庫錯誤(死結、逾時、約束違反、連線中斷)也會被這個 catch 接住
  • 結果是資料庫錯誤訊息(可能含 SQL 與資料表結構)被當成 422 回給使用者,屬於資訊洩漏
  • 這類錯誤被當成「可預期的業務失敗」處理,不會經過 Laravel 例外處理器,不會被 Sentry 收到,營運端看不到
  • 狀態碼為 422(客戶端錯誤),實際是伺服器端故障,監控與前端錯誤處理都會誤判

待辦

  • 定義單一窄型的業務例外(例如 BookingRuleViolationException),交易閉包內改拋它;Controller 只 catch 該型別,其他例外讓它往外傳播
  • 回應內容只使用自己拋出的、已知安全的訊息;不再回傳任意 $e->getMessage()
  • 確認資料庫類例外會被 Sentry 收到、回應為 500 且訊息通用
  • 檢查其他 Controller 是否有相同寫法(目前 grep 僅發現這兩處 RuntimeException)
  • 補測試:讓交易內拋出 QueryException(例如以 DB::listen 或 mock 模擬),斷言回應為 500 且 body 不含 SQL 與資料表名稱;業務失敗(名額不足、重複預約)仍回 422 與原訊息
  • 更新 spec:booking-lifecycle(錯誤處理行為);注意該 spec 有一條 Requirement 超過 500 字元,修改時可順便拆分

驗收條件(草案,需轉為 spec Scenario)

  1. 名額不足、重複預約仍回 422 與既有訊息
  2. 交易內發生資料庫例外時回 500,回應不含 SQL、資料表或欄位資訊
  3. 上述資料庫例外會被記錄並送到 Sentry

安全確認(CLAUDE.md 必答)

  • 輸入驗證位置:既有 $request->validate(),不變
  • 資料庫存取:Eloquent,不變
  • 權限:不變;本項只改例外處理與回應內容

M8 預約取消/確認的狀態在鎖之前讀取,並發下名額可能重複增減

來源:撰寫 hardener skill 時閱讀程式碼發現。以下為程式碼閱讀判斷,尚未重現驗證;而且 phpunit.xml 使用 SQLite,無法在現有測試中重現行鎖行為(見 M6)。

現況(皆為「交易外讀狀態、交易內才鎖 schedule」的模式)

  • MemberBookingController::destroy:在交易外以 $booking->status 判斷可否取消,並記下 $wasConfirmed;進入交易後才 lockForUpdate 鎖 schedule,booking 本身沒有鎖,且仍使用鎖之前讀到的舊狀態
  • ProviderBookingController::cancel、confirm、reject、complete:同樣先用 canTransitionTo() 判斷(交易外、無鎖),交易內才鎖 schedule
  • AdminBookingController::complete、排程 app:complete-finished-bookings、app:expire-pending-bookings:以交易外或批次方式直接改狀態,沒有與其他寫入者序列化

可能的後果(需並發才觸發)

  • 同一筆 confirmed 預約被會員與教練(或同一方重複點擊)同時取消:兩個請求都通過檢查,各自扣減一次 current_participants,名額被重複釋放,後續可能超賣;current_participants 為 unsignedInteger,若剛好扣到負數,在 MySQL 嚴格模式下會報錯變成 500
  • 同一筆 pending 預約被同時確認兩次:各自增加一次 current_participants,名額被重複占用
  • confirm 與 reject/expire 同時發生:預約可能最後是 confirmed 卻已被標成 expired,或名額已增加但狀態被覆蓋
  • 既有的 BookingOversellTest 只涵蓋單程序情境,無法抓到這類問題

待辦

  • 先驗證:以兩個程序或 MySQL 上的並發請求重現「同一預約重複取消」與「同一預約重複確認」,確認名額是否真的重複增減(驗證環境需為 MySQL,見 M6)
  • 狀態轉換統一在交易內:先鎖住預約本身(Booking::lockForUpdate()->find(...))或先鎖 schedule 再重讀預約,鎖之後再以最新狀態判斷 canTransitionTo() 與 $wasConfirmed
  • 以條件式更新輔助:例如 where('status', 'confirmed')->update(...) 並檢查受影響列數,為 0 代表別人已先處理
  • 取得鎖的順序需一致(例如一律先 schedule 後 booking),避免新增鎖後產生死結
  • 統一抽到單一入口(併入 M2 的 BookingService),所有取消與確認路徑共用
  • 評估排程指令(complete-finished-bookings、expire-pending-bookings)是否也需要用同樣的條件式更新,避免與使用者操作互相覆蓋
  • 補測試:雖然 SQLite 無法驗證行鎖,仍應補「同一預約連續取消兩次,第二次回 422 且名額只釋放一次」等可確定性的測試;真正的並發驗證放在 MySQL CI(M6)或人工驗證
  • 更新 spec:booking-lifecycle(狀態轉換與名額一致性)

驗收條件(草案,需轉為 spec Scenario)

  1. 同一筆預約重複取消,名額只釋放一次,第二次請求回 422
  2. 同一筆預約重複確認,名額只占用一次,第二次請求回 422
  3. 狀態轉換的判斷使用鎖之後的最新狀態
  4. 現有 BookingLifecycleTest、BookingOversellTest 全綠

安全確認(CLAUDE.md 必答)

  • 資料庫存取:Eloquent,不使用 raw SQL
  • 是否需要悲觀鎖:是;需序列化的資源是「單一預約」與其所屬的 schedule
  • 權限檢查:不變,各端點既有的擁有權檢查保留

備註

  • 此問題的完整驗證依賴 M6(MySQL CI)。在 M6 完成之前,只能以程式碼審查與可確定性測試降低風險,不能宣稱已驗證併發安全

功能與延後項目(非本次審視來源)

來源:既有規劃文件與已歸檔 change 的未完成任務,2026-10-17 彙整並對照程式碼確認仍未完成。優先順序未排入上方建議順序。

# 項目 來源 狀態/前置條件
F1 金流整合(awaiting_payment 或獨立 payments 表、Webhook 冪等) 2026-06-11-future-roadmap-feasibility.md §2.2 未開工;需先定案金流商(評估建議綠界)與退款政策;建議 M6、H1 先完成
F2 上傳檔案正式切換 S3 相容儲存(R2) 已歸檔 2026-08-02-cloud-ready-s3-storage tasks 4–6 程式碼已完成,VPS 仍為 FILESYSTEM_DISK=local;需先申請帳號,接續時另開新 change
F3 預約列表端點補分頁(member/provider/admin bookings 皆 ->get() 全量) 2026-06-11-performance-optimization-plan.md O3.2 未開工;建議金流上線前完成
F4 課程搜尋強化(地區、價格區間、可預約日期篩選) 可行性評估 §2.5 未開工;cache key 需納入新篩選參數
F5 前端載入體感(列表 skeleton、搜尋 debounce 300ms) 效能計畫 O3.3 未開工(frontend/src 無 skeleton/debounce)
F6 聊天圖片壓縮改用 CompressesImages trait O3.1 歸檔時留待後續 BookingMessageController 仍自行 scaleDown/toJpeg;屬重複邏輯,改動前需先補聊天圖片上傳測試
F7 正式環境 OPcache validate_timestamps=0 效能計畫「生產環境註記」 目前僅有 docker/php/local.ini(=1);程式碼已烘進 image,可評估於正式環境關閉

其他提醒

  • Git remote vps 的 URL 內含明文 Gitea token(存在 .git/config,未進版控)。建議輪換 token,改用 credential helper 或 SSH。
  • 本文件為待辦清單,不代表已排入開發;每項啟動前仍需依 CLAUDE.md 完成「Task 執行前」檢查。