23 KiB

Codebase Concerns

Analysis Date: 2026-05-07

Tech Debt

Multiple Lock Files Conflict Risk

Issue: Three conflicting lock files coexist: package-lock.json, pnpm-lock.yaml, and yarn.lock.

Files:

  • package-lock.json (177KB)
  • pnpm-lock.yaml (2.3KB)
  • yarn.lock (91KB)

Impact:

  • Risk of installing different dependency versions across environments
  • CI/CD, Docker builds, and local development may pull incompatible versions
  • Lock file merge conflicts during multi-developer collaboration
  • Dockerfile uses yarn exclusively, but npm/pnpm may be used locally, creating version drift

Current State:

  • Dockerfile (line 12, 34) explicitly uses yarn.lock*
  • .gitignore likely checks in all three files
  • CLAUDE.md explicitly warns "不要混用" but provides no automation to enforce it

Fix Approach:

  1. Choose single package manager (recommend yarn, per Dockerfile)
  2. Delete package-lock.json and pnpm-lock.yaml from git history
  3. Add enforcement to .gitignore and CI/CD (fail if multiple lock files modified)
  4. Document in CLAUDE.md that ONLY yarn should be used

Priority: High - impacts build reproducibility and team workflow


Production Build Ignores TypeScript & ESLint Errors

Issue: Build succeeds despite type/lint errors; production may contain undetected bugs.

Files:

  • next.config.mjs:16-21 — Both ignoreDuringBuilds: true

Current Configuration:

eslint: {
  ignoreDuringBuilds: true,
},
typescript: {
  ignoreBuildErrors: true,
},

Impact:

  • TypeScript errors (type mismatches, missing nullchecks) not caught at build time
  • ESLint warnings (unused imports, unreachable code) ignored
  • QA burden shifts to runtime discovery
  • Breaking changes in dependencies not detected early

Evidence: README.md (line 121) acknowledges this: "务必在本地执行 next linttsc --noEmit 保证质量" — manual step, not enforced

Fix Approach:

  1. Set both to false in production config
  2. Fix all current type/lint errors (likely small count given ~27K LOC)
  3. Add pre-commit hooks to husky to block commits with errors
  4. Make lint/type checks mandatory in CI/CD

Priority: High - gates correctness


Console Logging in Client Code (Debug Artifact)

Issue: Production code contains console.log/warn/error calls, exposing debug info and token prefixes.

Files & Line Numbers:

  • lib/api/client.ts:23console.log('🔍 Token检查:', ...)
  • lib/api/client.ts:29console.warn('⚠️ 未找到auth_token...')
  • lib/api/client.ts:35console.log('📡 发送请求:', ...)
  • lib/api/client.ts:36console.log('📋 请求头:', ...)
  • lib/api/client.ts:47console.log('✅ 响应成功:', ...)
  • lib/api/client.ts:51console.error('❌ 响应错误:', ...)
  • lib/api/client.ts:55console.warn('🚫 认证失败...')
  • lib/api/client.ts:56console.log('响应详情:', ...)
  • lib/api/upload.ts:33console.log('📤 开始上传文件:', ...)
  • lib/api/upload.ts:43console.log(...上传进度...)
  • lib/api/upload.ts:59console.log('✅ 文件上传成功:', ...)
  • lib/api/upload.ts:62console.error("文件上传失败:", error)
  • Similar issues in upload multifile (lines 83, 93)
  • components/sidebar.tsx:97console.error("退出登录失败:", error)
  • app/outfits/page.tsx:72, 117, 135, 154 — console.error calls
  • Many other pages: app/*/page.tsx contain error logging

Impact:

  • Token prefixes exposed in browser console → XSS/developer tools leak risk
  • Error details (e.g., API paths, status codes) visible to end users
  • Production debugging should use structured error handling, not console
  • Could be scraped by malicious actors with browser console access

Evidence: lib/api/token-debug.ts is entire debug utility with console.log throughout (lines 7-54), showing token prefix extraction logic

Fix Approach:

  1. Remove all console.log/warn/error in client code, replace with structured error handling
  2. Create error boundary/toast notification pattern for user-facing errors
  3. Optional: Create client-side error tracking service (Sentry) for production
  4. Add ESLint rule: no-console (already available in Next.js config, just not enabled)

Priority: High - security and code cleanliness


Security Concerns

Client-Side Permission Enforcement Only (Trust Boundary Violation)

Issue: Permissions are enforced ONLY on client; backend may not re-validate on API calls.

Files:

  • lib/permissions.ts:65-87getUserRole(), hasPermission() read from localStorage.user_role
  • components/sidebar.tsx:102-104 — Filter menu items client-side only
  • middleware.ts:17-44 — Token validation in middleware, but NO permission check against module being accessed

Risk Model:

User (attacker) → Modify localStorage.user_role="超级管理员" → 
  Sidebar shows all menus (useless) BUT 
  Direct API call to DELETE /api/v1/admin/users/123 → Backend accepts if no re-check

Current Evidence:

  • lib/api/users.ts:24-43getUsers() makes raw API call; no permission guard in client code
  • No evidence of backend role check in Django API (not inspectable from frontend code, but CRITICAL to verify)

Impact:

  • CRITICAL: If backend doesn't re-validate role on each endpoint, attackers can:
    • Modify localStorage role → make admin API calls
    • Bypass middleware token check (SSR can't access localStorage) → direct API calls with stolen token
    • Perform unauthorized operations (delete users, change settings, etc.)

Trust Boundary Rule: Backend MUST re-check permission for EVERY admin API call, regardless of client claims.

Fix Approach:

  1. URGENT: Audit Django backend API (qy_lty/lib/api/v1/admin/) to confirm all endpoints validate user role/permissions
  2. Document in CLAUDE.md: "Client-side permission filters are UI courtesy only; backend MUST validate on all admin/ endpoints"
  3. Add explicit checks in frontend error handling for 403 Forbidden responses
  4. Consider adding request logging/monitoring to catch unauthorized attempts

Priority: CRITICAL - security vulnerability if backend doesn't re-check


Token Storage in localStorage (XSS Vulnerability)

Issue: Auth token stored in localStorage instead of secure HttpOnly cookie; vulnerable to XSS.

Files:

  • lib/api/auth.ts:48-59 — Stores token in localStorage AND sets js-cookie
  • lib/api/client.ts:21-22 — Reads token from localStorage in request interceptor
  • middleware.ts:32 — Token read from Cookie (correct), but localStorage is fallback

Current Implementation:

// Stored in BOTH localStorage and cookie
localStorage.setItem("auth_token", token);  // XSS risk
Cookies.set("auth_token", token, { expires: 7, path: '/' });  // No HttpOnly flag

Impact:

  • XSS attack can steal token from localStorage
  • js-cookie (js-cookie, no HttpOnly flag) also vulnerable to XSS
  • 7-day expiry without rotation = long XSS window
  • localStorage.user_role also stored unencrypted (attackable)

Proper Secure Pattern:

  • HttpOnly, Secure, SameSite cookies (set by backend during login)
  • Token never exposed to JavaScript
  • Client reads from Bearer token response, never stores in accessible storage

Fix Approach:

  1. Backend should set HttpOnly cookie during login response
  2. Remove localStorage token storage entirely
  3. Keep only sealed HTTP-only cookie for auth
  4. If SSR requires token access, use secure session storage (not localStorage)
  5. Add CSP (Content-Security-Policy) headers to mitigate XSS

Priority: High - significant security risk


No Token Rotation / Refresh Strategy

Issue: Token expires in 7 days, no refresh token or rotation mechanism visible.

Files:

  • lib/api/auth.ts:63Cookies.set("auth_token", token, { expires: 7, path: '/' })
  • lib/api/client.ts:54-60 — 401 handler just redirects to /login, doesn't attempt refresh

Impact:

  • User activity > 7 days → auto-logout (UX degradation)
  • No short-lived access token + long-lived refresh token pattern
  • Stolen token valid for full 7 days
  • No token revocation (logout doesn't invalidate server-side if stateless JWT)

Fix Approach:

  1. Implement short-lived access token (15-30 min) + refresh token (7-14 days)
  2. Refresh interceptor: if 401, attempt token refresh before redirecting to /login
  3. Backend session should blacklist tokens on logout
  4. Document token lifecycle in CLAUDE.md

Priority: Medium - acceptable for internal admin tool, but should improve for production


No Input Validation on File Upload

Issue: File uploads accept any mimetype/size without validation.

Files:

  • lib/api/upload.ts:25-65uploadFile() accepts File object, no size/type checks in client
  • components/ui/file-upload.tsx — Likely has upload UI, needs validation

Impact:

  • Malicious files (executables, oversized) may reach backend
  • Backend must validate, but client-side check is first defense
  • No maximum file size enforced client-side

Fix Approach:

  1. Add file size limits (e.g., max 50MB)
  2. Whitelist MIME types (images: image/jpeg, image/png, image/webp only)
  3. Show user error before upload attempt
  4. Verify backend enforces same limits

Priority: Medium - backend should catch it, but client should be first defense


Fragile Areas

Large Pages with Mixed Concerns (1000+ line files)

Issue: Several pages handle data fetching, UI rendering, and business logic in single component.

Large Files:

  • app/affinity/page.tsx (1005 lines) — Affinity rules, levels, interactions all in one page
  • components/ui/sidebar.tsx (763 lines) — Navigation logic, permission filtering, logout in one component
  • app/songs/[id]/page.tsx (651 lines) — Song detail page, editing, deletion mixed
  • app/songs/page.tsx (644 lines) — Song list, search, pagination, CRUD operations

Fragility Risks:

  • Hard to test individual business logic
  • Changes to one feature (e.g., affinity rules) risk breaking another (levels)
  • Difficult to reuse UI logic across pages
  • High cognitive load for maintenance

Safe Modification Pattern:

  1. Extract data-fetching logic to lib/api/ hooks or services
  2. Create smaller sub-components (e.g., <AffinityRuleSection>, <AffinityLevelSection>)
  3. Use custom hooks for state management per feature
  4. Keep pages as composition only

Example Refactor:

// Current: 1005-line page
// After:
export default function AffinityPage() {
  return (
    <DashboardShell>
      <DashboardHeader />
      <AffinityRulesSection />
      <AffinityLevelsSection />
      <InteractionLogsSection />
    </DashboardShell>
  )
}

Priority: Medium - refactor as part of feature changes, not urgent


No Error Boundaries (Next.js error.tsx Files Missing)

Issue: No error boundary components for graceful error handling in App Router.

Files:

  • app/ has NO error.tsx files
  • Only some leaf routes have loading.tsx (e.g., app/outfits/loading.tsx)

Current Error Handling:

  • Pages catch errors in try/catch, show toast notifications
  • Uncaught errors propagate, may show raw error UI
  • No fallback UI for API failures

Impact:

  • User sees blank page or browser error on network failure
  • No recovery mechanism (retry button, fallback content)
  • Log data lost if error page crashes

Next.js Error Boundary Pattern:

// app/error.tsx (catches errors in all children)
'use client'
export default function Error({ error, reset }) {
  return (
    <div>
      <h1>Something went wrong</h1>
      <button onClick={() => reset()}>Try again</button>
    </div>
  )
}

Fix Approach:

  1. Add app/error.tsx for app-level errors
  2. Add segment-level error.tsx for feature-specific handling (e.g., app/outfits/error.tsx)
  3. Keep try/catch for API calls, render fallback UI
  4. Add app/not-found.tsx for 404 routes

Priority: Low - pages have basic error handling via toasts, but UX could improve


Hardcoded API Base URL Fallback

Issue: API client falls back to hardcoded URL if env var not set.

Files:

  • lib/api/client.ts:9API_BASE_URL = process.env.NEXT_PUBLIC_API_BASE_URL || "http://localhost:8000/api"

Risk:

  • If NEXT_PUBLIC_API_BASE_URL not set in production, silently uses localhost
  • Requests fail silently in production, confusing to debug
  • No warning log when fallback is used

Fix Approach:

  1. Remove fallback; throw error if env var not set at build time
  2. Use if (!process.env.NEXT_PUBLIC_API_BASE_URL) throw new Error("...")
  3. Document required env vars in deployment guide

Priority: Low - environmental issue, unlikely in production if properly configured


Performance Bottlenecks

No Memoization on Permission Checks

Issue: hasPermission(), getUserRole() called on every render without memoization.

Files:

  • components/sidebar.tsx:102-104 — Calls hasPermission(item.module) in .filter() on every render
  • lib/permissions.ts:65-87 — No caching; reads localStorage every time

Current Pattern:

const visibleAiItems = mounted ? aiMenuItems.filter((item) => hasPermission(item.module)) : []

Impact:

  • Sidebar re-renders, re-filters items, re-reads localStorage unnecessarily
  • Minimal impact on small arrays (3-15 items), but scales poorly

Fix Approach:

  1. Memoize getUserRole() result in sidebar:
const userRole = useMemo(() => getUserRole(), [mounted])
const visibleItems = useMemo(() => 
  aiMenuItems.filter(item => hasPermission(item.module)), 
  [userRole]
)
  1. Or move to custom hook: const { role, allowedModules } = usePermissions()

Priority: Low - not a visible performance issue, but good practice


Cross-Repo Coupling Risks

Strong Runtime Dependency on qy_lty Backend

Issue: Front-end depends on specific API contract from qy_lty/ Django backend; contract drift risk.

Files:

  • lib/api/users.ts:6-21 — Maps backend ParadiseUser to User type (field mapping)
  • lib/api/outfits.ts (not read, but expected) — Maps backend outfit schema
  • ALL API modules expect specific response structure from /api/v1/admin/*

Coupling Points:

  • Backend field names: username, email, phone_number, date_joined, last_login, is_superuser, is_staff
  • Response structure: { results: [...], count: number } (Django REST Framework default)
  • Endpoint paths: /v1/admin/login/, /user/, /common/upload/
  • Token storage key in Redis: admin_token:{token} (per CLAUDE.md)

Risk Scenario:

  1. Backend dev changes /v1/admin/users/ response format
  2. Frontend still sends old field names or expects old response shape
  3. Silent failures or type mismatches
  4. Detected late in testing or production

Mitigation in Place:

  • CLAUDE.md (line 34) requires both repos update docs/修改记录.md when contract changes
  • But no automated contract testing (OpenAPI, TypeScript codegen)

Fix Approach:

  1. Shared types/API contract (OpenAPI/GraphQL schema) versioned in git
  2. Generate TypeScript types from backend OpenAPI spec (e.g., with Swagger-Codegen)
  3. Add pre-commit hook to detect schema changes
  4. Document in CLAUDE.md: "Any API response structure change requires updating both repos' docs"

Priority: Medium - mitigated by manual process, but automatable


Dependencies at Risk

No Enforcement of Dependency Updates

Issue: Many dependencies pinned to latest, creating unpredictable updates.

Files:

  • package.json:12, 56, 63"latest" version specifiers

Current Versions Using "latest":

"@hookform/resolvers": "latest",
"react-hook-form": "latest",
"recharts": "latest",
"zod": "latest"

Impact:

  • npm install at different times installs different major versions
  • Breaking changes in react-hook-form or recharts not caught by lock file
  • Tests pass locally, fail in CI if new major version published

Fix Approach:

  1. Replace all "latest" with specific versions (e.g., "^7.0.0" for caret range)
  2. Use Dependabot or renovate to automate safe updates
  3. Pin devDependencies to exact versions

Example:

"@hookform/resolvers": "^3.3.4",
"react-hook-form": "^7.51.0",
"recharts": "^2.12.0",
"zod": "^3.22.0"

Priority: Medium - not urgent, but good practice


Unused Dependencies or Overlapping Packages

Issue: Unclear if all installed packages are actively used.

Potential Issues:

  • Multiple notification libraries: sonner + @radix-ui/react-toast (which is used?)
  • Figure out which is preferred and remove duplication

Files:

  • package.json:36, 59 — Both toast packages listed

Fix Approach:

  1. Audit each dependency in package.json
  2. Remove unused packages
  3. Consolidate duplicate functionality

Priority: Low - cleanup task


Test Coverage Gaps

No Tests Found

Issue: No Jest/Vitest configuration or test files detected in codebase.

Expected Test Files:

  • *.test.ts, *.spec.ts, *.test.tsx, *.spec.tsx not found
  • No jest.config.*, vitest.config.*

Impact:

  • Complex logic (permission matrix, API adapters) not tested
  • Refactoring risky (no regression protection)
  • Onboarding new developers without safety net
  • No confidence in critical paths (auth, user management)

Critical Areas Needing Tests:

  • lib/permissions.ts — Permission matrix logic (many branches)
  • lib/api/client.ts — Request/response interceptors (error handling paths)
  • lib/api/users.ts — User mapping logic (backend schema mismatches)
  • Custom hooks in pages (data fetching, state management)

Fix Approach:

  1. Set up Jest or Vitest (Vitest recommended for ESM + modern tooling)
  2. Create basic test for lib/permissions.ts:
// lib/permissions.test.ts
describe('Permission checks', () => {
  it('should allow super admin all modules', () => {
    // Mock localStorage with 超级管理员
    expect(hasPermission('users')).toBe(true)
    expect(hasPermission('ai-model')).toBe(true)
  })
  
  it('should restrict content admin to content modules', () => {
    // Mock localStorage with 内容管理员
    expect(hasPermission('users')).toBe(false)
    expect(hasPermission('outfits')).toBe(true)
  })
})
  1. Aim for 80%+ coverage of critical API functions

Priority: Medium - important for long-term maintainability


Missing Critical Features

No Logout Token Revocation

Issue: logout() clears client-side storage but doesn't invalidate server-side token.

Files:

  • lib/api/auth.ts:108-131logout() calls API endpoint, then clears localStorage

Problem:

  • If using stateless JWT, backend has no record of logout
  • Token remains valid until expiry (7 days)
  • Attacker with stolen token can still use it after target user "logs out"

Expected Behavior:

  • Server should blacklist token on logout (Redis blacklist or DB flag)
  • Verify endpoint /v1/admin/logout/ actually invalidates token

Fix Approach:

  1. Audit backend logout endpoint to confirm token is invalidated
  2. Add token blacklist to backend (Redis: token → blacklisted, TTL=7 days)
  3. Check token against blacklist on every request
  4. Document in CLAUDE.md

Priority: High - security issue if not handled on backend


No Rate Limiting on Client (Brute Force Risk)

Issue: Login page may be vulnerable to brute force attacks.

Files:

  • app/login/page.tsx — Likely has login form
  • lib/api/auth.ts:25-40 — No rate limiting before API call

Impact:

  • Attacker can spam login attempts to guess password
  • Backend must implement rate limiting, but client can't spam if it has throttling

Fix Approach:

  1. Client-side: Disable login button for 30 seconds after failed attempt
  2. Backend: Implement account lockout (3 failed attempts = 10 min lockout)
  3. Monitor login failures in logs

Priority: Medium - backend responsibility, but client should assist


Build Configuration Issues

Next.js Standalone Output May Have Deployment Pitfalls

Issue: Standalone mode used, but edge case behaviors not fully documented.

Files:

  • next.config.mjs:30output: 'standalone'

What Standalone Does:

  • Bundles all dependencies into .next/standalone folder
  • Reduces Docker image size (~100MB vs ~500MB)
  • Requires explicit COPY .next .next in Dockerfile (correctly done)

Potential Issues:

  • Public assets not automatically copied; must COPY separately (done in Dockerfile:44-45)
  • next.config.mjs must be manually copied (done in Dockerfile:46)
  • Environment variables must be passed at runtime, not build time
  • Debugging bundle issues harder (bundled deps not in node_modules)

Current Dockerfile (lines 44-46) handles this correctly:

COPY --from=builder /app/.next ./.next
COPY --from=builder /app/public ./public
COPY --from=builder /app/next.config.mjs ./

Risk:

  • If dependencies add new output files in future versions, they won't be copied
  • Node version mismatch between builder (22.10.0) and runner (22.10.0) is correct but fragile

Fix Approach:

  1. Document standalone output caveats in deployment guide
  2. Test Docker image monthly to ensure no drift in builder/runner
  3. Consider base image pinning: FROM node:22.10.0-alpine@sha256:...

Priority: Low - current setup is correct, just needs documentation


No Pre-commit Hooks or Husky Setup

Issue: No automated checks before commits; developers can commit broken code.

Files:

  • No .husky/ directory
  • No pre-commit config
  • CLAUDE.md doesn't mention hook setup

Risk:

  • Developer commits console.log, broken types, or lint errors
  • CI catches them later, but feedback loop is slow
  • No enforcement of modification record updates (CLAUDE.md requirement)

Fix Approach:

  1. Install husky: npm install husky --save-dev
  2. Setup hook: npx husky install
  3. Create .husky/pre-commit:
#!/bin/sh
npm run lint && npm run type-check
  1. Create .husky/prepare-commit-msg to remind user about docs/修改记录.md

Priority: Low - improvement for team workflow


Summary Table

Category Issue Severity Fix Effort
Security Client-only permissions (backend trust) CRITICAL High
Security Token in localStorage (XSS) High Medium
Security No token revocation on logout High High
Tech Debt Lock file conflicts High Low
Tech Debt Build ignores TS/ESLint errors High Low
Tech Debt Console logging in production High Low
Fragility Large 1000+ line pages Medium Medium
Performance No memoization on permissions Low Low
Testing Zero test coverage Medium High
Build Standalone output not fully documented Low Low

Concerns audit: 2026-05-07