From aa8a155685eb5c6774f5f4e446923897951c163a Mon Sep 17 00:00:00 2001 From: lemone112 Date: Fri, 13 Mar 2026 13:49:20 +0300 Subject: [PATCH 01/10] =?UTF-8?q?feat:=20P0+P1=20pipeline=20improvements?= =?UTF-8?q?=20=E2=80=94=20wire=20hidden=20features,=20add=20selection=20aw?= =?UTF-8?q?areness,=20progressive=20loading?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 (coherence fixes): - Wire all 7 AI review categories to UI (was 4 — 40% of Claude tokens wasted) - Add labels for visualQuality + microcopy lint categories in IssuesList - Expand Design Health Score to 7 weighted categories (25/18/10/25/7/8/7) - Forward token analysis summary to backend for AI context - Fix hardcoded hasAutoLayout/childCount — now reads real values from Figma node - Add selection change detection with stale results banner + Re-analyze button P1 (UX improvements): - Expose radius fixes in Fix All + walkthrough (was spacing-only) - Progressive 4-phase loading indicator (rules → screenshot → AI → Refero) - Persistent ignore state via figma.root.setPluginData (survives restarts) - 3-step onboarding flow replacing blank empty state - "AI-generated" badge on AiReviewCard to distinguish facts from opinions Audit reports: - AUDIT-REPORT.md: 16-agent consolidated audit (~200 findings) - PIPELINE-EVALUATION.md: 3-agent pipeline coherence/completeness evaluation - backend/TEST_PLAN.md: 107 test cases across 7 endpoints Co-Authored-By: Claude Opus 4.6 --- AUDIT-REPORT.md | 573 +++ PIPELINE-EVALUATION.md | 390 ++ backend/TEST_PLAN.md | 753 ++++ backend/src/services/analyzer.ts | 4 +- dist/code.js | 3984 +++++++++++++------ dist/ui.html | 70 +- src/code.ts | 13 + src/ui/message-handler.ts | 51 +- ui/src/App.tsx | 106 +- ui/src/components/chat/ChatContainer.tsx | 2 +- ui/src/components/chat/MessageList.tsx | 70 +- ui/src/components/messages/AiReviewCard.tsx | 14 +- ui/src/components/messages/IssuesList.tsx | 4 + ui/src/components/messages/ScoreCard.tsx | 10 +- ui/src/hooks/useChat.ts | 29 +- ui/src/lib/api.ts | 6 + ui/src/lib/messages.ts | 15 +- 17 files changed, 4707 insertions(+), 1387 deletions(-) create mode 100644 AUDIT-REPORT.md create mode 100644 PIPELINE-EVALUATION.md create mode 100644 backend/TEST_PLAN.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 (`
`, `
`, `