refactor: collapse CORS/preflight duplication in local_server

[piazzatron]

Summary

• Replaces the per-endpoint CORS header dicts and separate preflight handlers with one middleware. The three browser-facing paths (/auth/callback, /subscription/refresh, /ping) share a single implementation of preflight handling and response decoration, including the Access-Control-Allow-Private-Network header.
• Drops the origin allowlist. The old code restricted /auth/callback and /subscription/refresh to smart-notes.xyz. The CSRF attack this defended against — a malicious page overwriting the user's JWT — would only redirect API costs to the attacker's own account, which isn't a threat model worth the code. Any origin now gets echoed back and the request is served.
• Replaces the old TypedDict + .get()-and-string-check param extraction with dacite-validated dataclasses that mirror the camelCase JSON wire contract. Missing or wrong-typed fields now raise ParamError at the boundary with a clear message (e.g. "noteType is required").
• Splits the / RPC action dispatch into a module-level ACTIONS: dict[str, ActionHandler] registry; each action becomes a top-level async function. The external contract ({action, version, params} → {result, error}) is unchanged.

Note: option/model string fields on the request dataclasses are deliberately Optional[str] rather than the Literal unions from models.py — a strict dacite check there would reject any newly-added model before the add-on updates.

dacite is added to the vendored-deps list in scripts/build.sh (already in requirements.txt).

Verification

Automated

• ./scripts/build.sh check — ruff format --check, ruff check, pyright: all green.
• pytest — 96/96 tests pass, including the rewritten test_local_server.py suite.
• TCP-level smoke test: spun up the real LocalServer in its background thread on 127.0.0.1:8766 and hit every endpoint with urllib. Confirmed: /ping, /auth/callback, /subscription/refresh all echo whatever Origin the caller sent (including unknown origins), OPTIONS preflights return 204 with the CORS + PNA headers, the RPC / endpoint remains free of CORS headers (intentional — not browser-facing), and server.stop() shuts down cleanly.

Not performed

• Anki launch via /anki — headless Linux host with no Anki install. The LocalServer lifecycle and all HTTP behavior was verified via the TCP smoke test above, but the plugin-in-Anki launch path was not exercised end-to-end.

Claude session

• Working directory: /home/michael/development/anki-smart-notes-master/anki-smart-notes
• Session ID: 6fb6c964-c1aa-492e-9b9d-78696c8a5809

[claude]

Claude finished @piazzatron's task —— View job

---

Code Review - Bug and Vulnerability Analysis

Todo List:

• Read and analyze changed files for bugs and vulnerabilities
• Review CORS policy implementation for security issues
• Examine dacite parameter validation for edge cases
• Check RPC action dispatch for vulnerabilities
• Review error handling and information leakage
• Post findings

Security Findings

After thorough analysis of the refactored local server code, I found 1 potential security issue and 1 edge case bug:

🔴 Security Issue: JWT Validation Bypass

• Location: src/local_server.py:379-380
• Issue: The /auth/callback endpoint accepts empty string JWTs
• Code:

  if not body.jwt:
      return web.json_response({"ok": False, "error": "missing_jwt"}, status=400)

• Problem: In Python, not "" evaluates to True, but an empty string passes dacite validation as a valid str. An attacker could send {"jwt": ""} and it would be stored as the auth token.
• Impact: Medium - could lead to authentication bypass scenarios
• Fix: Change to explicit length check: if not body.jwt or not body.jwt.strip():

🟡 Edge Case Bug: CORS Origin Header Handling

• Location: src/local_server.py:292-294
• Issue: Potential None value in CORS response headers when policy allows "*"
• Code:

  echoed = origin if policy.allowed_origins != "*" else (origin or "*")
  return {
      "Access-Control-Allow-Origin": echoed or "*",  # Double fallback needed

• Problem: If policy.allowed_origins == "*" and origin is None, echoed becomes None, requiring the second or "*" fallback
• Impact: Low - mainly a code clarity issue, functionality preserved
• Fix: Simplify to origin if policy.allowed_origins != "*" else "*"

Positive Security Observations

✅ CORS Implementation: Properly restricts origins and rejects unauthorized requests
✅ Parameter Validation: dacite provides strong typing validation with clear error messages
✅ Error Handling: No sensitive information leakage in error responses
✅ Action Dispatch: Clean separation with proper error boundaries
✅ Thread Safety: Main thread operations properly queued via run_on_main_sync

The refactor significantly improves code organization while maintaining security. The JWT validation issue should be addressed before merging.

[piazzatron]

Code review

Found 1 issue:

1. Multiple underscore-prefixed top-level functions. The project CLAUDE.md says: "Don't prefix top level defs with an _ ever. Save those for private methods on classes." Offenders in src/local_server.py: _envelope, _ok, _err (shown below), plus _cors_response_headers (L291), _register_routes (L344), _options_stub (L359), _resolve_deck_id (L725), _run_on_main_sync (L731), and _save_smart_field (L746). Rename to drop the leading underscore.

anki-smart-notes/src/local_server.py

Lines 252 to 265 in dd372d4

	def _envelope(result: Any = None, error: Optional[str] = None) -> web.Response:
	return web.json_response({"result": result, "error": error})
	def _ok(result: Any = None) -> web.Response:
	return _envelope(result=result)
	def _err(message: str) -> web.Response:
	return _envelope(error=message)

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[claude]

Claude finished @piazzatron's task —— View job

---

Bug and Security Review

Todo List:

• Read and analyze changed files for bugs and vulnerabilities
• Review CORS policy implementation for security issues
• Examine parameter validation for edge cases
• Check RPC action dispatch for vulnerabilities
• Review error handling and information leakage
• Post findings

Security Findings

After analyzing the refactored local server code, I found 1 potential security issue:

🔴 JWT Validation Bypass in /auth/callback

• Location: src/local_server.py:337-338
• Issue: The endpoint accepts empty string JWTs as valid
• Code:

  if not body.jwt:
      return web.json_response({"ok": False, "error": "missing_jwt"}, status=400)

• Problem: In Python, not "" evaluates to True, but dacite validates "" as a valid str. An attacker could send {"jwt": ""} and it would bypass this check and be stored as the auth token.
• Impact: Could lead to authentication bypass scenarios depending on downstream JWT validation
• Fix: Add explicit length/content validation: if not body.jwt or not body.jwt.strip():

Code Quality Notes

The refactor significantly improves code organization and maintainability. The CORS implementation properly restricts access, dacite provides strong parameter validation, and error handling doesn't leak sensitive information. The action dispatch system is clean and well-organized.