Skip to content

fix: throttle writes, bound mcp inputs, harden oauth consent#118

Merged
FrkAk merged 11 commits into
mainfrom
security-review-repo-bmn2el
Jun 11, 2026
Merged

fix: throttle writes, bound mcp inputs, harden oauth consent#118
FrkAk merged 11 commits into
mainfrom
security-review-repo-bmn2el

Conversation

@FrkAk

@FrkAk FrkAk commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Summary

Task Reference: none (security-review hardening; no associated Mymir task)

Addresses the high/medium findings from the security review that map to hosted-cost abuse and OAuth client spoofing, plus the UI reliability fixes that fell out of the same review (silent mutation failures, delete-undo data loss). Dynamic client registration stays open (the standard MCP onboarding pattern); the fixes target what rides along with it rather than registration itself. All changes are Neon + Cloudflare Workers compatible (no pg_cron dependency — see Notes).

Cost / abuse (hosted):

  • Throttle the web write path with per-action budgets on the mutation wrappers (generous for humans, tight for scripts), since server-action POSTs bypass the /api/* middleware limiter. The per-IP limb runs before the session lookup so an unauthenticated flood cannot farm free DB session lookups. Team/invite actions carry their own budgets and surface a typed rate_limited result.
  • Cap MCP tool inputs: an explicit request body-size limit on /api/mcp (MCP_MAX_BODY_BYTES, streamed with early cancel via readBodyBounded so a chunked over-limit body cannot fill isolate memory) plus generous per-field/array ceilings — anti-abuse, not content policy; long unabridged plans still pass. An explicit 0 hard-freezes the endpoint and logs a warning at module init.
  • Per-project task cap (MAX_TASKS_PER_PROJECT, default 50k) as a growth backstop, checked race-free under the existing per-project advisory lock; agents hitting it get a dedicated TaskLimitError telling them to stop retrying.
  • Rate-limit open OAuth client registration on the strict auth binding. Pre-auth rules (sign-in, sign-up, oauth2/register) key on the client IP rather than the unvalidated session cookie — a forged cookie would otherwise mint a fresh bucket per request — and the matcher normalizes trailing slashes so exact-pattern rules cannot be dodged.

OAuth consent spoofing:

  • Brand-name normalization is gated on a server-side verified client_id allowlist (MYMIR_VERIFIED_OAUTH_CLIENT_IDS, empty by default), so an unverified dynamically-registered client renders its raw registered name and cannot impersonate a trusted brand on the consent screen or the devices list. The verified flag is a required parameter on formatOAuthClientName so no future call site can silently default into brand polish.
  • Invisible Unicode (zero-width space/joiner, RTL override, control chars) is stripped from displayed client names so lookalike names cannot render as a trusted brand.

Agent prompt-injection defense-in-depth:

  • Untrusted-content framing notice prepended to all six agent-facing context bundles (agent, planning, review, working, summary, overview) — teammate-authored fields are reference data, not instructions.

UI reliability (review fallout, ~half the diff):

  • Previously-silent mutation failures now surface inline errors and revert optimistic state: criteria, decisions, description, title, plan lifecycle, relationships, and prop-rail field updates.
  • The delete-undo stack moved from StructureView to WorkspaceClient so it survives layout remounts (view switches, breakpoint flips, selection transitions) — the stack holds the only copy of a deleted task's body, and remount-scoped state was permanent data loss. The layout was restructured into stable slot components so the element type no longer changes when the selection flips between null and non-null.
  • Undo entries are pushed only after a successful delete (no more bogus "undo" for a task that was never removed). A failed restore re-pushes the entry (useUndo rejection handling with a generation guard against stale re-pushes) and shows an error in the undo strip; a failed delete keeps the confirm dialog open with a visible error.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation

Testing

  • Tested locally with bun run dev
  • Linting passes (bun run lint)
  • Typecheck passes (bun run typecheck)

Notes for reviewer

  • Full suite run locally against Postgres: 665 pass / 0 fail, including the 6 context-bundle snapshots (earlier drafts hand-edited those in a DB-less sandbox; they have since been validated by running the suite).
  • New tests on this branch: register rate-limit rule + trailing-slash normalization, verbatim / invisible-Unicode client-name rendering, parseEnvInt (explicit-zero semantics), readBodyBounded (inclusive cap boundary, chunked over-limit reject with stream cancellation), and the verified-client allowlist (memoization re-keys on env change, no test-only reset hook).
  • Compatibility: an earlier draft added a pg_cron job to purge abandoned dynamically-registered clients, but Neon does not support pg_cron and there is no Cloudflare cron trigger configured, so it was dropped. The register rate-limit is the compatible bound on client growth; a Cloudflare Cron Trigger could add active purging later if wanted.
  • Action write budgets run on the per-isolate memory backend by design (documented in lib/api/rate-limit.ts): exact on self-host, a per-isolate soft bound on Workers.
  • Deferred follow-ups: align team-action rate-limit checks to the pre-auth IP ordering used by the mutation wrappers; drop the stale content-length header on the reconstructed MCP request (harmless today — runtimes derive length from the byte body).
  • Tunable env knobs: MCP_MAX_BODY_BYTES, MAX_TASKS_PER_PROJECT, MYMIR_VERIFIED_OAUTH_CLIENT_IDS.

@FrkAk FrkAk force-pushed the security-review-repo-bmn2el branch from 8a5e3ae to 656cd60 Compare June 10, 2026 13:09
@FrkAk FrkAk self-assigned this Jun 10, 2026
@FrkAk FrkAk merged commit f6048ae into main Jun 11, 2026
4 checks passed
@FrkAk FrkAk deleted the security-review-repo-bmn2el branch June 11, 2026 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants