bindbox-game/.planning/codebase/CONCERNS.md

Codebase Concerns

Analysis Date: 2026-03-21

---

Security Considerations

WeChat AppSecret logged in plaintext:

• Risk: WeChat AppSecret (OAuth credentials) are written to application logs at Info/Error level, exposing secrets in log files and any log aggregation systems.
• Files:
  • internal/service/user/login_weixin.go:52 — s.logger.Info("DEBUG: LoginWeixin Config", zap.String("AppSecret", wcfg.AppSecret))
  • internal/api/user/phone_bind.go:59 — h.logger.Error("...", zap.String("app_secret", wxCfg.AppSecret))
  • internal/api/common/openid_app.go:44 — h.logger.Info("GetOpenID Config", zap.String("AppSecret", wxcfg.AppSecret))
• Current mitigation: None. These are active, non-conditional log calls.
• Recommendations: Remove all AppSecret fields from log calls immediately. If debugging is needed, log only AppID (never AppSecret).

Hardcoded internal API key fallback:

• Risk: If Internal.ApiKey config is missing or empty, the system falls back to the hardcoded literal "bindbox-internal-secret-2024". Any attacker with knowledge of this default can call all internal game settlement endpoints.
• Files: internal/router/router.go:99
• Current mitigation: Config key overrides the default when set.
• Recommendations: Remove the hardcoded default. Fail-closed: if expectedKey == "", reject all requests with 503 rather than falling back to a known string.

Hardcoded Nakama server key:

• Risk: internal/api/game/handler.go:207 contains nakamaKey := "defaultkey" which is the well-known Nakama default key, used when config is absent.
• Files: internal/api/game/handler.go:207
• Current mitigation: Config value overrides when present.
• Recommendations: Panic at startup if Nakama key is not configured in production mode.

CORS allows all origins with credentials:

• Risk: AllowedOrigins: []string{"*"} combined with AllowCredentials: true is an invalid CORS configuration per spec (browsers block it) and signals the intent to allow arbitrary origins was not fully thought through.
• Files: internal/pkg/cors/cors.go:14,34
• Current mitigation: Browsers enforce the spec restriction, partially preventing exploitation.
• Recommendations: Replace "*" with an explicit allowlist of trusted origins.

pprof profiling endpoint exposed in production:

• Risk: core.WithEnablePProf() is unconditionally passed in internal/router/router.go:45. The Go pprof endpoints at /debug/pprof/* expose heap dumps, goroutine stacks, CPU profiles, and memory layout — high-value information for attackers.
• Files: internal/router/router.go:45, internal/pkg/core/core.go:252-258
• Current mitigation: None — pprof is always enabled.
• Recommendations: Gate WithEnablePProf() behind an environment check (ENV != "pro").

---

Tech Debt

Skipped Redis ticket validation in game token service:

• Issue: internal/service/game/token.go:127-136 has a commented-out return statement under a // TODO: 临时跳过 Redis 验证. When the Redis key is not found, validation is bypassed and the game token is accepted regardless. This permanently disables single-use ticket enforcement.
• Files: internal/service/game/token.go:127-136
• Impact: A valid JWT can be replayed indefinitely once issued; single-use semantics are broken.
• Fix approach: Restore the return nil, fmt.Errorf("ticket not found or expired") line. Investigate why tickets expire from Redis before use (likely TTL too short or Redis key prefix mismatch).

Stub minesweeper game handlers:

• Issue: internal/api/internal/minesweeper/handler.go contains two handlers (VerifyTicket, SettleGame) that are entirely mock implementations. Both contain // TODO: 实际验证逻辑 and return hardcoded responses. The settle handler always returns success: true and a mock reward string.
• Files: internal/api/internal/minesweeper/handler.go:46-78
• Impact: Minesweeper game settlement and ticket verification are non-functional. Any caller receives a fake success response regardless of actual state.
• Fix approach: Implement proper Redis ticket lookup (matching the game token service), deduct tickets, and grant actual rewards on win.

TODO counts in douyin order sync:

• Issue: internal/api/admin/douyin_orders_admin.go:302,337 have GrantedCount: 0 and RefundedCount: 0 with TODO comments noting these should return actual counts from the sync/grant functions. Admin UI shows incorrect stats (always 0) for these operations.
• Files: internal/api/admin/douyin_orders_admin.go:302,337
• Impact: Douyin sync reports are inaccurate. Operators cannot confirm how many prizes were actually granted or refunded per sync run.
• Fix approach: Update SyncRefundStatus and GrantLivestreamPrizes to return counts, propagate to response.

Duplicate user handler registration with undocumented intent:

• Issue: internal/router/router.go:83 has an explicit // TODO: Check if userHandler and userAppHandler are redundant or distinct. comment. Two user handler instances exist with unclear separation of responsibility.
• Files: internal/router/router.go:82-86
• Impact: Risk of inconsistent behavior — changes to one handler may be expected to apply to both but don't.
• Fix approach: Audit both handler paths; consolidate or document the distinction.

---

Known Bugs

Debug fmt.Printf statements left in production code:

• Symptoms: Over 35 fmt.Printf("[DEBUG]...") calls across production code path are unconditionally executed in all environments, writing to stdout rather than the structured Zap logger. This pollutes logs, degrades performance, and leaks business data.
• Files (representative):
  • internal/service/douyin/order_sync.go:648,710,731,800,812,824,829,840,846,850,855,859,872
  • internal/service/sysconfig/dynamic_config.go:198
  • internal/service/activity/activity_order_service.go:150,158,163,207,235,265
  • internal/service/user/order_timeout.go:51,60,150
  • internal/service/user/coupon_transfer.go:97
  • internal/api/user/login_app.go:59
  • internal/api/admin/lottery_admin.go:535
  • internal/api/activity/lottery_app.go:67
  • internal/api/activity/issue_choices_app.go:76,79,82
  • internal/service/game/token.go:78,131,141,147,152,159
  • internal/pkg/wechat/code2session.go:22
• Trigger: Always — these are unconditional print calls in hot paths.
• Fix: Remove all fmt.Printf calls; replace necessary observability with s.logger.Debug(...) calls gated by log level.

Silently discarded errors in critical paths:

• Symptoms: Multiple writes/inserts ignore errors via _ = h.repo.GetDbW().Exec(...) and _ = h.repo.GetDbR().Raw(...).Scan(...). Failed inventory updates, ledger entries, and coupon operations produce no error response.
• Files (representative):
  • internal/api/admin/pay_refund_admin.go:155,174,195,198,224,227,235,239,262
  • internal/api/activity/lottery_app.go:429,639,667
  • internal/api/activity/issues_app.go:80
  • internal/api/admin/users_profile.go:172,188,197,222,246,249
  • internal/api/admin/activity_commitment_admin.go:62,63,64,66,96
• Trigger: On database errors in refund, inventory, and query flows.
• Fix: Wrap these in proper error handling; at minimum log errors; for writes in financial flows, propagate errors to callers.

---

Performance Bottlenecks

Dashboard admin handler is a 2,666-line monolith:

• Problem: internal/api/admin/dashboard_admin.go contains the entire dashboard implementation in a single file. Many dashboard queries perform multiple full-table scans on orders, inventory, and draw_logs tables without pagination constraints.
• Files: internal/api/admin/dashboard_admin.go
• Cause: Dashboard endpoints aggregate across all-time data. Complex multi-table JOINs (up to 7 tables in dashboard_spending.go) run inline per request.
• Improvement path: Introduce materialized summaries or scheduled background computation for dashboard aggregates. Split the file into per-widget files (max 400 lines each).

Unguarded Find() calls without LIMIT in handler layer:

• Problem: Several handlers call .Find() on potentially unbounded result sets.
• Files:
  • internal/api/app/categories.go:44 — all active categories loaded at once
  • internal/api/activity/issue_choices_app.go:50 — all reward settings for an issue
  • internal/api/activity/draw_logs_app.go:133,146,164 — users, rewards, products hydration in loops
  • internal/api/app/product_category.go:47
• Cause: Missing Limit() calls; no pagination on supporting queries.
• Improvement path: Add .Limit(500) guards and paginate public-facing endpoints. For in-memory hydration loops, batch-query by IN clause (already done in some places) but add bounds.

time.Sleep in hot production paths:

• Problem: internal/pkg/wechat/shipping.go uses time.Sleep(time.Second) and time.Sleep(2 * time.Second) between WeChat API calls, blocking goroutines for up to 4 seconds per operation. Under load, this exhausts the goroutine pool.
• Files: internal/pkg/wechat/shipping.go:138,165,215,246
• Cause: Naive retry/rate-limit handling.
• Improvement path: Replace with exponential backoff using time.After or context-aware wait, and move to a worker pool pattern.

---

Fragile Areas

Handler layer bypasses service layer for DB writes:

• Files: internal/api/admin/pay_refund_admin.go, internal/api/activity/lottery_app.go, internal/api/activity/issue_choices_app.go
• Why fragile: 113 direct GetDbW() calls exist in the API handler layer. Business logic (inventory updates, ledger entries, item card resets) is scattered across handlers and services, making transactional consistency hard to enforce and audit.
• Safe modification: Any change to refund or inventory logic must trace all three locations (handler, service, scheduler). Do not add new business writes in handlers.
• Test coverage: No dedicated tests for pay_refund_admin.go refund flows.

Douyin order sync with goroutine fan-out and mutex:

• Files: internal/service/douyin/order_sync.go
• Why fragile: The sync loop at line 1056 spawns a goroutine per order item with a shared sync.Mutex for counter updates. Errors from individual goroutines are collected in a slice protected by mutex, but goroutine lifecycle is managed only through a sync.WaitGroup. A single panicking goroutine will be caught by the task center worker recover, but may leave the mutex in an inconsistent state.
• Safe modification: Add goroutine-level panic recovery inside the fan-out goroutine (line 1056). Do not increase fan-out concurrency without adding semaphore limiting.
• Test coverage: 1 test file for a 1,094-line service.

Task center service is a 1,665-line file with embedded BUG FIX comments:

• Files: internal/service/task_center/service.go
• Why fragile: The file contains multiple // BUG FIX: comments at lines 715, 809, 1558, 1581, 1609, 1629 indicating patch-on-patch fixes. The quantity resolution logic for reward payloads has been fixed three times (lines 1558, 1581, 1609, 1629) with slight variations — these must be kept consistent.
• Safe modification: When modifying reward quantity parsing, update all four sites. Do not add a fifth variant.
• Test coverage: 4 test files cover primarily list filtering and invite logic, not the reward claim path.

Dynamic config service with global singleton and panic:

• Files: internal/service/sysconfig/global.go:34-37, internal/service/sysconfig/dynamic_config.go
• Why fragile: GetGlobalDynamicConfig() panics if called before InitGlobalDynamicConfig(). Any package that calls this at init time or before main.go initialization order completes will crash the process.
• Safe modification: Always call InitGlobalDynamicConfig() as the first step after DB initialization in main.go. Do not call GetGlobalDynamicConfig() at package init() scope.
• Test coverage: No tests in internal/service/sysconfig/.

---

Test Coverage Gaps

User service layer (37 source files, 2 test files):

• What's not tested: Login flow, coupon add/transfer, order timeout, address share, expiration task, WeChat integration wrappers.
• Files: internal/service/user/ (35 untested files including login_weixin.go, coupon_add.go, order_timeout.go, address_share.go)
• Risk: Regressions in payment-adjacent and user lifecycle code go undetected.
• Priority: High

Activity service (24 source files, 3 test files):

• What's not tested: lottery_process.go, activity_order_service.go, scheduler.go, matching_game.go, most of the 24 files.
• Files: internal/service/activity/
• Risk: Core lottery and order creation flows have no automated test coverage. Any refactor risks silent breakage.
• Priority: High

Admin service (6 source files, 0 test files):

• What's not tested: All of internal/service/admin/.
• Files: internal/service/admin/
• Risk: Admin-level business logic untested.
• Priority: Medium

Snapshot and recycle services:

• What's not tested: internal/service/snapshot/ (2 files, 0 tests), internal/service/recycle/ (1 file, 0 tests).
• Risk: Snapshot replay and recycle operations silently broken.
• Priority: Medium

Sysconfig service:

• What's not tested: internal/service/sysconfig/ (3 files, 0 tests) including the global singleton and dynamic config loader.
• Risk: Config loading failures or key formatting bugs go undetected.
• Priority: Medium

---

Scaling Limits

Matching game in-memory state:

• Current capacity: Game state for the matching card game (MatchingGameState) is stored in internal/service/activity/matching_game.go using an in-memory sync.Mutex-protected struct. This is per-process state.
• Limit: Cannot scale horizontally — a second server instance has no visibility into game state of the first.
• Scaling path: Migrate game state to Redis using atomic operations or use a dedicated game state store (e.g., Nakama, which is already referenced in the codebase).

Scheduler goroutines without stop signal:

• Current capacity: internal/service/activity/scheduler.go:38 and internal/service/douyin/scheduler.go:29 spawn bare goroutines that loop forever on time.Sleep(30 * time.Second).
• Limit: Cannot be stopped gracefully on shutdown — goroutines outlive the shutdown context.
• Scaling path: Thread a context.Context into the scheduler loop and break on ctx.Done().

---

Dependencies at Risk

Proliferation of debug command tools in cmd/:

• Risk: cmd/ contains 9+ one-off debug/diagnostic tools (debug_task_270, debug_check_coupon_22, fix_openid, exploit_verify, check_order, etc.) that have hardcoded database credentials or connection strings for one-time use. These tools may be committed with active credentials and are not maintained.
• Impact: Security exposure if credentials are embedded; build confusion if these break the CI pipeline.
• Migration plan: Move all one-off tools to a tools/ directory with a clear no-deploy policy, or delete after use.

---

Concerns audit: 2026-03-21