diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..70af451 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,68 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + plugin: + name: Plugin (typecheck + bundle) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 22 + cache: npm + - run: npm ci + - run: npx tsc --noEmit + - run: npm run bundle + + ui: + name: UI (typecheck + build) + runs-on: ubuntu-latest + defaults: + run: + working-directory: ui + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 22 + cache: npm + cache-dependency-path: ui/package-lock.json + - run: npm ci + - run: npm run build + + backend: + name: Backend (typecheck + build) + runs-on: ubuntu-latest + defaults: + run: + working-directory: backend + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version: 22 + cache: npm + cache-dependency-path: backend/package-lock.json + - run: npm ci + - run: npx tsc --noEmit + - run: npm run build + + docker: + name: Docker build + runs-on: ubuntu-latest + needs: [plugin, ui, backend] + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3 + - uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 + with: + context: backend + push: false + cache-from: type=gha + cache-to: type=gha,mode=max diff --git a/.gitignore b/.gitignore index e1ec484..3758186 100644 --- a/.gitignore +++ b/.gitignore @@ -53,8 +53,16 @@ scripts/test-*.js .serena/ settings.local.json -# Development UI files -# ui-enhanced.html is now our main UI file, not a dev file +# Data / SQLite databases +data/ +*.db +*.db-wal +*.db-shm + +# Environment variables +.env +.env.* +!.env.example # Personal/Internal documentation MCP_INTEGRATION_SUCCESS.md diff --git a/AUDIT-REPORT.md b/AUDIT-REPORT.md new file mode 100644 index 0000000..44894c1 --- /dev/null +++ b/AUDIT-REPORT.md @@ -0,0 +1,573 @@ +# FigmaLint — Consolidated Audit Report + +**Date:** 2026-03-13 +**Agents:** 16 (Code Reviewer, Security Engineer, Backend Architect, Frontend Developer, DevOps Automator, Technical Writer, Software Architect, UX Researcher, UI Designer, Accessibility Auditor, UX Architect, AI Engineer, MCP Builder, Database Optimizer, Performance Benchmarker, API Tester) + +## Summary + +- **~200 unique findings** (after cross-agent deduplication) +- **Snyk SAST:** 0 automated issues +- **WCAG 2.1 AA:** DOES NOT CONFORM (28 accessibility issues, 8 critical) +- **Documentation:** 4/10 + +--- + +## TOP-10 Critical / Must Fix + +| # | Issue | Source | Effort | +|---|-------|--------|--------| +| 1 | **No backend authentication** — open Claude proxy, anyone can burn API credits | Security, Backend, Code Review, Architect | Medium | +| 2 | **API keys logged to console** (`save-api-key` handler logs plaintext `sk-ant-...`) | Security, Code Review | Trivial | +| 3 | **Google API key in URL** (`?key=...`) — appears in proxy logs, browser history | Security, Code Review | Medium | +| 4 | **XSS in legacy `ui-enhanced.html`** — innerHTML with unsanitized AI content | Security | Medium | +| 5 | **Chat invisible to screen readers** — no `aria-live`, `role="log"`, landmarks | Accessibility | Low | +| 6 | **Settings panel is not a modal** — no focus trap, Escape key, `role="dialog"` | Accessibility | Low | +| 7 | **No AbortController for streams** — concurrent streams interleave into one bubble | Frontend | Low | +| 8 | **Race condition in `appendConversation`** — read-modify-write without transaction | Backend | Low | +| 9 | **Fix preview not wired to UI** — `previewFix()` + `dryRun` infrastructure exists but unused | UX Research | Medium | +| 10 | **No onboarding** — blank screen + Analyze button, zero explanation | UX Research | Medium | + +--- + +## Security (12 findings) + +### HIGH + +**S-1: API keys logged to console** +- File: `src/ui/message-handler.ts:87` +- The `save-api-key` handler logs `data` including the full API key in plaintext. +- Fix: `const safeData = type === 'save-api-key' ? { ...data, apiKey: '***REDACTED***' } : data;` + +**S-2: Google API key in URL query parameter** +- File: `src/api/providers/index.ts:181`, `src/api/providers/google.ts:332` +- Key appended as `?key=...`, logged via `console.log`, visible in proxy logs and browser history. +- Fix: Route Google calls through backend proxy; redact URL before logging. + +### MEDIUM + +**S-3: No authentication or rate limiting on backend** +- File: `backend/src/index.ts` +- All routes open. Anyone who discovers the URL can call `/api/analyze` and burn Anthropic credits. +- Fix: Add `bearerAuth` middleware from Hono + rate limiting. + +**S-4: Session ID enumeration** +- Files: `backend/src/routes/session.ts`, `backend/src/services/session.ts` +- nanoid(12) provides good entropy (~71 bits), but without auth any valid ID leaks full session data. +- Fix: Implement auth (S-3), then bind sessions to client identifier. + +**S-5: XSS in legacy HTML via innerHTML** +- File: `ui-enhanced.html:6854-6894` +- AI-generated content rendered via `innerHTML` after markdown conversion without escaping source text. +- Note: React UI (`AiMessage.tsx`) is clean — no `dangerouslySetInnerHTML`. +- Fix: Escape HTML before markdown transformation, or delete legacy file. + +**S-6: No request body size limits** +- Files: `backend/src/routes/analyze.ts`, `backend/src/routes/flow.ts` +- Base64 screenshots accepted with no size cap. OOM risk. +- Fix: `app.use('/api/analyze', bodyLimit({ maxSize: 25 * 1024 * 1024 }));` + +**S-7: MCP clients lack timeout configuration** +- Files: `backend/src/mcp/client.ts`, `backend/src/mcp/design-systems-client.ts` +- No connection or request timeouts. Unresponsive MCP server = indefinite hang. +- Fix: Wrap `client.callTool()` with `Promise.race` timeout (15s). + +**S-8: CORS returns `'null'` for blocked origins** +- File: `backend/src/index.ts:30-31` +- `Access-Control-Allow-Origin: null` allows any sandboxed iframe to access the API. +- Fix: Return empty string or omit header for non-allowed origins. + +### LOW + +**S-9: `.env` not in `.gitignore`** +- File: `.gitignore` +- Fix: Add `.env`, `.env.*`, `!.env.example`, `backend/.env`, `data/`. + +**S-10: Missing security headers in Caddyfile** +- File: `backend/Caddyfile` +- No `X-Content-Type-Options`, `X-Frame-Options`, `Strict-Transport-Security`, `Referrer-Policy`. +- Fix: Add `header` block to Caddyfile. + +**S-11: Error messages may leak internal details** +- Files: `backend/src/routes/*.ts` +- `error.message` passed directly to client. Could expose Anthropic SDK internals. +- Fix: Generic 500 message to client; log full error server-side. + +**S-12: SQL column name interpolation (mitigated)** +- File: `backend/src/db/queries.ts:60-71` +- Column names from `ALLOWED_COLUMNS` interpolated into SQL. Safe due to allowlist but fragile. +- Fix: Add comment documenting the security invariant. + +--- + +## Architecture (15 findings) + +### P0 — Critical + +**A-1: No backend authentication** +- See S-3. The backend is an open Claude proxy. + +### P1 — High + +**A-2: Duplicated types between plugin and UI** +- Plugin `src/types.ts` defines `LintErrorType` including `'visualQuality' | 'microcopy'` +- UI `ui/src/lib/messages.ts` defines `LintErrorType` missing these two types +- Impact: `buildFullReport` fails to report visual quality and microcopy issues; `byType` record incomplete. +- Fix: Create shared types package or single source of truth. + +**A-3: `message-handler.ts` is an 800-line monolith** +- File: `src/ui/message-handler.ts` +- Handles all 33 message types in one giant switch/if chain. Highest coupling point. +- Fix: Split into per-feature handlers: `lint-commands.ts`, `fix-commands.ts`, `screenshot-commands.ts`, `chat-commands.ts`, `settings-commands.ts`. + +**A-4: No test suite** +- `package.json`: `"test": "echo \"Error: no test specified\" && exit 1"` +- Lint rules, scoring, batch fixer, JSON normalization — all pure functions, highly testable. +- Fix: Add vitest with unit tests for deterministic logic. + +### P2 — Medium + +**A-5: Triple Anthropic client singletons** +- `backend/src/services/claude.ts:7-12`, `analyzer.ts:8-13`, `flow-analyzer.ts:5-10` +- Fix: Export single shared `getAnthropicClient()` from `claude.ts`. + +**A-6: Duplicate MCP client code** +- `backend/src/mcp/client.ts` and `design-systems-client.ts` are nearly identical. +- Fix: Create `createMcpClient(name, url)` factory. + +**A-7: MODEL constant defined 3 times** +- `claude.ts:15`, `refero.ts:6`, `flow-analyzer.ts:13` — all `'claude-sonnet-4-20250514'` +- Fix: Export from `claude.ts`, import elsewhere. + +**A-8: `src/fix/` vs `src/fixes/` overlap** +- Two directories for fix-related code with overlapping responsibility. +- Fix: Merge into single `src/fixes/` directory. + +**A-9: Plugin-side LLM providers are dead code** +- `src/api/providers/` — multi-provider abstraction predates the backend. Backend now handles all AI calls. +- Fix: Evaluate for removal or document as optional direct-mode feature. + +**A-10: Cache doesn't account for lint settings** +- File: `src/core/consistency-engine.ts` +- Hash omits `lintSettings`, `severityOverrides`, `ignorePatterns`. Changed settings → stale cache. +- Fix: Include settings hash in cache key. + +**A-11: No protection against huge selections** +- `traverseAndLint` is synchronous and recursive. Thousands of nodes = frozen sandbox. +- Fix: Add node count limit or yield between chunks. + +**A-12: Greedy JSON regex for AI response parsing** +- Pattern: `text.match(/\{[\s\S]*\}/)` — matches first `{` to last `}`. +- Fix: Balanced brace matcher or use Claude's structured output / tool-use mode. + +**A-13: Unbounded consistency engine cache** +- File: `src/core/consistency-engine.ts:12-13` +- Map grows without limit. No LRU eviction, no max size. +- Fix: Add max-entries cap (e.g., 100) with LRU eviction. + +**A-14: Conversation history sent to Claude twice** +- `backend/src/prompts/chat-followup.ts` embeds last 8 messages in system prompt. +- `chat.ts` and `stream.ts` also pass full history as API messages. +- Fix: Remove history from system prompt; let messages array carry it. + +**A-15: 3 sequential Claude API calls could be optimized** +- `detectPageType` + `generateReview` are sequential. +- Fix: Fold page type detection into the review prompt to save one round trip. + +--- + +## Frontend (34 findings) + +### HIGH — Correctness Bugs + +**F-1: Stale closure in `handleRescan`** +- File: `ui/src/hooks/useChat.ts:222-261` +- Reads `state.score` outside `setState` updater — captures stale reference. +- Fix: Use `prev.score` inside the `setState` callback. + +**F-2: No AbortController for streams** +- File: `ui/src/lib/api.ts:97-165` +- No abort on new analysis or navigation. Old stream calls `onChunk` on stale state. +- Fix: Pass `AbortSignal` to `fetch`; expose cancellation to caller. + +**F-3: Concurrent streams interleave** +- File: `ui/src/App.tsx:190-212` +- No guard against sending while `isStreaming`. Two streams write to same bubble. +- Fix: Disable input during streaming or abort previous stream. + +**F-4: Polling interval leaks on unmount** +- File: `ui/src/App.tsx:72-99` +- `referoPollingRef` interval never cleared on component unmount. +- Fix: Add `useEffect` cleanup. + +**F-5: `text-10` class undefined in Tailwind** +- Files: `ui/src/components/messages/AiReviewCard.tsx:16`, `ReferoGallery.tsx:54` +- Class silently ignored; elements inherit parent font size. +- Fix: Add `'10': ['10px', '14px']` to Tailwind fontSize config. + +**F-6: `bg-bg-success/10` opacity modifier broken with CSS variables** +- File: `ui/src/components/messages/ReferoGallery.tsx:9-10` +- Tailwind opacity modifiers require decomposable color values; CSS vars can't be decomposed. +- Fix: Use `style` prop with `color-mix()` or define explicit semi-transparent tokens. + +**F-7: `focus:ring-fg-brand` references nonexistent token** +- File: `ui/src/components/shared/SettingsPanel.tsx:98` +- Fix: Change to `focus:ring-bg-brand` or add `fg-brand` alias. + +### MEDIUM — Type Safety / Robustness + +**F-8: `PluginEvent` catch-all defeats exhaustive checking** +- File: `ui/src/lib/messages.ts:197` +- `| { type: string; data: unknown }` matches everything. +- Fix: Remove catch-all member. + +**F-9: Pervasive `as any` casts in message handler** +- File: `ui/src/App.tsx:118,128,134,139,144,164,167` +- Fix: Narrow to correct union members. + +**F-10: Two divergent code paths for Re-scan** +- `ChatContainer.tsx:35-38` routes to `onAnalyze`; `App.tsx:299-302` posts `rescan-lint`. +- Fix: Unify to single path. + +**F-11: Module-level mutable `messageIdCounter`** +- File: `ui/src/hooks/useChat.ts:5` +- Fix: Use `crypto.randomUUID()` or `useId()`. + +**F-12: Dead `postToPlugin` function** +- File: `ui/src/lib/messages.ts:200-202` — standalone function duplicates hook; appears unused. +- Fix: Remove. + +### LOW — Architecture / Polish + +**F-13: App.tsx monolith (485 lines)** +- Fix: Extract `handleAction` into `useActions` hook; move `buildFullReport` to `lib/`. + +**F-14: Inline SVG duplication** +- Settings gear in `App.tsx` and `StickyHeader.tsx`; checkmark in `FixResult.tsx` and `MessageList.tsx`. +- Fix: Extract into shared `icons/` module. + +**F-15: No `React.memo` on message components** +- `appendStreamChunk` called per SSE chunk → entire MessageList re-renders. +- Fix: Wrap `AiMessage`, `ScoreCard`, `IssuesList` etc. in `React.memo()`. + +**F-16: `parseBold` regex runs on every render without memoization** +- File: `ui/src/components/messages/AiMessage.tsx:10` +- Fix: `useMemo` or wrap component in `memo`. + +**F-17: Redundant CSS variables in globals.css** +- `:root` variables may conflict with Figma-injected values. +- Fix: Remove or mark as development-only fallbacks. + +--- + +## Accessibility (28 findings) + +**WCAG 2.1 AA Conformance: FAIL** + +### CRITICAL (8) + +| # | Issue | WCAG | File | +|---|-------|------|------| +| AC-1 | No landmark regions (`
`, `
`, `