[Audit][Low+Info bundle] Six lower-severity findings from the GraphCoder red-team audit

[docxology]

Title: [Security][Low+Info bundle] Six lower-severity findings from the GraphCoder red-team audit

Severity: Low / Informational (bundle — each item independently actionable)

1. PATCH /api/annotations partial geometry silently erases existing points (low, CWE-708)

updateAnnotationSchema declares geometry.points with .default([]). A PATCH body containing geometry: {anchor:{x,y}} (anchor-only update) materializes points: [] after zod parsing; routes/annotations.ts:227-229 then does {...existing, ...updates} and saveAnnotation — the user's region points are wiped on disk.
Fix: merge geometry per-field (preserve points when absent) or make points optional without default in the update schema.
Evidence: routes/annotations.ts:227-229 {const updates = parsed.data; const updated = { ...existing, ...updates }; saveAnnotation(root, updated)}; store.ts:162-168 copies geometry wholesale.

2. saveAnnotation writes paths derived from annotation.id without validation (low, CWE-22)

normalizeAnnotation copies raw.id verbatim (id: (raw.id as string) ?? randomUUID()), and saveAnnotation builds annotationPath(projectRoot, annotation.id) from it. An on-disk annotation file with id: '../x' round-trips into a write outside the directory; the kind-rename cascade (routes/annotations.ts:119-125) re-saves every matching annotation, amplifying it. Same root cause as the high-severity annotation-id traversal — apply the same UUID/basename validation in the write path.

3. Kind-rename cascade bypasses request schema + case-handling inconsistency (low, CWE-20)

PATCH /api/annotation-kinds/:name mutates every annotation of that kind via direct ann.kind = updated.name; saveAnnotation(root, ann) — skipping zod validation. updateKind sets existing.name = updates.name.trim() case-sensitively while findKind dedupes case-insensitively: renames across case collisions can produce registry entries the case-insensitive lookup treats as the same kind with different stored names.

4. Git refs from HTTP passed unsanitized to simple-git (low, CWE-88)

resolveRef (git/index.ts:110-119), discoverPrStack revparse, and listCommits pass client strings (req.query.branch, req.body.base/target) directly as git argv. simple-git uses argument arrays (no shell injection), but leading - values parse as git OPTIONS (argument injection). Validate refs (reject leading -, prefer --end-of-options).

5. Node navigation history grows unboundedly (low, CWE-770)

packages/client/src/state/selection.ts:19-63 — module-level navHistory array, pushHistory() appends without cap; only the forward branch is truncated. Cap it (e.g. 500, drop oldest) the way state/annotations.ts caps undo/redo at UNDO_LIMIT=100.

6. E2E base URL ignores the client PORT override (low, config drift)

tests/config.ts:9-18 hardcodes CLIENT_PORT 3356 while vite.config.ts:15-18 honors an env-provided PORT via loadEnv — running the client with PORT=4000 in .env leaves the Playwright readiness check polling :3356 forever. Derive both from one shared constants module.

7. Debug global window.__graphcoder ships in production builds (info, CWE-489)

packages/client/src/App.tsx:183-239 unconditionally attaches the full reactive store snapshot (including selected node source code) plus mutation functions to window.__graphcoder. E2E tests depend on it — gate behind import.meta.env.DEV or a test-only build flag.

8. WebSocket client trusts server payloads without schema validation (info, CWE-20)

packages/client/src/state/project.ts:213-277 JSON.parses every WS message and applies fields via type casts straight into the store (ELK worker, ThreeRenderer buffers, SDF glyphs, command palette). Currently a same-trust-boundary correctness issue (degraded rendering/crash on malformed payloads), but worth minimal shape checks (finite numbers, array caps) as defense-in-depth — pairs with the server-side WS validation issue.

9. SSE multi-data-line concatenation deviates from the SSE spec (info)

computeDiff SSE handling in the client concatenates data: lines per spec... currently later lines overwrite instead of joining; and patchAnnotation sends the full annotation snapshot (id/version included) as the PATCH body. No confirmed exploit; worth aligning with the spec.