fix(api): authenticate and harden the operator API surface#15
Merged
Conversation
The Fastify API had no authentication on any route: anyone who could reach the port could read every tenant's data (including PostHog API keys via /api/clients), enqueue paid jobs, and approve queued irreversible actions. Registration also accepted anonymous tenant upserts when SEO_BOT_API_KEY was unset, there was no rate limiting, CORS reflected any origin, and DB/LLM strings were interpolated into the dashboard HTML unescaped. - Add operator auth (HTTP Basic password or Bearer, constant-time compare) as a global onRequest hook, fail-closed when OPERATOR_API_KEY is unset; exempt /health and the machine-authenticated /api/clients/register. - Add a dependency-free per-IP fixed-window rate limiter, stricter on the expensive /register and /trigger routes. - Redact posthogApiKey/posthogProjectId behind an explicit column allow-list on the client read endpoints. - Make /api/clients/register fail closed (503) when SEO_BOT_API_KEY is unset. - Lock CORS to an allow-list (default same-origin) and stop returning 5xx error detail to clients. - HTML-escape every DB/LLM-derived value in the dashboard renderers and guard the approvals JSON.parse. - Document OPERATOR_API_KEY / DASHBOARD_ALLOWED_ORIGINS in .env.example. Tests: operator auth hook (Basic/Bearer/fail-closed), escapeHtml, and the register fail-closed contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpFtUpb53Lj1QHSMxaX774
There was a problem hiding this comment.
Pull request overview
This PR hardens the operator-facing Fastify API by adding global authentication and rate limiting, preventing secret leakage from tenant endpoints, and sanitizing the HTML dashboard surface against injection.
Changes:
- Introduces global
onRequestsecurity hooks (fixed-window per-IP rate limiting + operator auth), with fail-closed behavior when secrets are unset. - Prevents sensitive tenant secrets from being serialized via
/api/clientsendpoints by switching to explicit column allow-lists. - Escapes all dynamic dashboard HTML interpolations and guards parsing of untrusted JSON fields; updates/extends tests for the new security contracts.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/api/security.ts |
Adds rate limiting + operator authentication hooks and exports helper utilities for testing. |
src/api/index.ts |
Wires security hooks globally, tightens CORS configuration, hardens error responses, and allow-lists client columns. |
src/api/clients/register.ts |
Makes /api/clients/register fail-closed when SEO_BOT_API_KEY is unset, enforcing machine-auth. |
src/api/dashboard.ts |
Adds escapeHtml() and applies escaping/encoding to all interpolated dashboard values; guards JSON.parse. |
src/core/config.ts |
Adds optional config entries for operator auth and dashboard CORS allow-list; updates comments to reflect fail-closed behavior. |
.env.example |
Documents new OPERATOR_API_KEY and DASHBOARD_ALLOWED_ORIGINS, and the fail-closed registration/auth behavior. |
tests/api/security.test.ts |
New unit tests for auth parsing, constant-time compare, strict-route classification, and operator auth hook behavior. |
tests/api/dashboard.test.ts |
New unit tests for escapeHtml() behavior. |
tests/api/register.test.ts |
Updates registration tests to reflect the new fail-closed key gate contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+92
to
+95
| // Lazy prune to bound memory under IP churn. | ||
| if (buckets.size > RATE_MAX_BUCKETS) { | ||
| for (const [k, b] of buckets) if (b.resetAt <= now) buckets.delete(k); | ||
| } |
Comment on lines
+45
to
+46
| // Rate limiting + operator authentication, before any route. | ||
| registerApiSecurity(app); |
This was referenced Jul 18, 2026
Addresses Copilot review on #15: - Rate limiter: after pruning expired buckets, hard-evict oldest-inserted entries so buckets.size stays <= RATE_MAX_BUCKETS even under a burst of unique IPs within one window (previously only pruned expired, so the map could still grow unbounded mid-window). - Make proxy trust explicit: new TRUST_PROXY env (default false) sets Fastify trustProxy, so request.ip and the per-IP limiter use X-Forwarded-For behind a reverse proxy/tunnel instead of bucketing all traffic under the proxy IP. Documented in .env.example. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpFtUpb53Lj1QHSMxaX774
cryptoxdog
added a commit
that referenced
this pull request
Jul 19, 2026
…retry on total dispatch failure (#20) * fix(site-deploy/executor): escape frontmatter, add network timeouts, don't lose work on total dispatch failure Three autonomous-action correctness/robustness fixes: - M2 (injection): updateMetaTitle/Description wrote `title: "${value}"` into Astro YAML frontmatter with no escaping, from LLM-generated surpass-plan text. A `"` breaks the build; a newline injects an arbitrary frontmatter key. Add yamlDoubleQuoted() (escapes \ and ", collapses newlines) and use it for title/og_title/description; collapse newlines in the H1 heading write. - M9 (robustness): no axios call in site-deployment (GitHub read/write, Vercel hook, build-site dispatch) or the Telegram sender set a timeout, so a hung endpoint stalled a BullMQ worker forever. Add a 15s timeout to all of them. - M5 (correctness): the surpass-plan executor advanced a gap to status='executing' even when every action dispatch threw (errors were only logged), permanently losing the planned work (the next run only re-selects status='planned'). Track per-gap dispatch attempts/successes and leave the gap 'planned' for retry when all attempted dispatches failed. Deferred (documented): M10 (client-scoped approve/reject) is low-value now that the dashboard is authenticated with a single global operator (PR #15); M11 (per-client deploy target) is a larger refactor behind the disabled serp:execute-surpass-plans job and its existing TODO. Tests: yamlDoubleQuoted escaping/newline-collapse; existing site-deploy tests still pass (timeout added to opts doesn't change asserted headers). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpFtUpb53Lj1QHSMxaX774 * review(#20): widen yamlDoubleQuoted signature, clarify comment, test nullish Addresses Copilot review on #20: - yamlDoubleQuoted accepts string|null|undefined (matches its nullish-safe body). - Reword the plan-executor retry comment to match the actual condition. - Assert null/undefined in the yamlDoubleQuoted test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpFtUpb53Lj1QHSMxaX774 --------- Co-authored-by: Igor Beylin <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
From the code audit — PR-1 of the remediation set (Critical: API security). The Fastify API had no authentication on any route: anyone able to reach the port could read every tenant's data (including PostHog API keys via
/api/clients), enqueue paid jobs, and approve queued irreversible actions. This PR closes that cluster.Findings addressed
onRequesthook (Basic password or Bearer, constant-time), fail-closed whenOPERATOR_API_KEYunset;/health+/api/clients/registerexemptGET /api/clients[/:id]returned full rows incl.posthogApiKey→ explicit column allow-list (secret columns never serialized)/api/clients/registeraccepted anonymous upserts whenSEO_BOT_API_KEYunset → fail closed (503)/register+/triggercors({origin:true})+ raw 5xxerror.messageleak → CORS allow-list (default same-origin), generic 5xx bodyescapeHtml()on every value; guarded the approvalsJSON.parseChanges
src/api/security.ts(new) — rate-limit + operator-auth hooks; exported pure helpers (parseAuthSecret,constantTimeEqual,isStrictRateLimited).src/api/index.ts— register security hooks; lock CORS; sanitize error handler; column allow-list on client endpoints.src/api/clients/register.ts— fail-closed key check.src/api/dashboard.ts—escapeHtmlhelper + escape all interpolated values; guardJSON.parse.src/core/config.ts/.env.example— addOPERATOR_API_KEY,DASHBOARD_ALLOWED_ORIGINS.tests/api/security.test.ts(new),tests/api/dashboard.test.ts(new),tests/api/register.test.ts(updated to the fail-closed contract).This is intentionally fail-closed:
OPERATOR_API_KEYor the dashboard/API returns401(that's the point — it's no longer open).SEO_BOT_API_KEYmust be set or/api/clients/registerreturns503.Verification
tsc --noEmit+vitestcould not be run in the authoring sandbox (the private@quantum-l9/llm-routerneedsNODE_AUTH_TOKEN, unavailable there). CI runs both. New/updated tests cover the auth hook (Basic/Bearer/fail-closed),escapeHtml, and the register fail-closed contract. Changes aresrc/-only + tests; no dependencies added.🤖 Generated with Claude Code
Generated by Claude Code