mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(api): rate-limit feedback endpoint + drop overly broad cache invalidation (H5/H6 code-review)
H5 — `POST /api/insights/feedback` had no rate limit. The unique
index `(userId, recommendationId, recommendationText)` blocks exact
replays but a malicious client could distort the daily aggregator's
bucket slice by varying recommendationId/text per request. Added
`checkRateLimit("insights-feedback:<user.id>", 60, 60 * 60 * 1000)`.
60/h is generous given a comprehensive insight rarely has >10 recs.
H6 — `<RecommendationFeedback>` invalidated the entire `["insights"]`
prefix on every thumbs click, which matches all 8 insights queries
(comprehensive + 7 per-status). None of them depend on the feedback
row. The 24h server cache absorbs the actual LLM round-trip but the
network thrash + stale-cache refetch was waste. Dropped the
invalidation; user-visible verdict state lives in localStorage +
local React state. v1.4.17's prompt-version ratchet will scope its
own invalidation when feedback feeds the next generation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
f3025a8dd5
commit
6863ecb87d
2 files changed
+28
-3
No files matched your search
@@ -8,6 +8,7 @@ import { Prisma } from "@/generated/prisma/client";
|
||||
import { resolveFeedbackAttribution } from "@/lib/ai/feedback-attribution";
|
||||
import { withIdempotency } from "@/lib/idempotency";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { recommendationFeedbackRequestSchema } from "@/lib/validations/recommendation-feedback";
|
||||
|
||||
/**
|
||||
@@ -30,6 +31,25 @@ import { recommendationFeedbackRequestSchema } from "@/lib/validations/recommend
|
||||
async function handlePost(request: NextRequest) {
|
||||
const { user } = await requireAuth();
|
||||
|
||||
// v1.4.16 phase D reconcile (code-review H5 / security M1) — bound
|
||||
// the per-user write rate. Without this, a single client can
|
||||
// distort the daily aggregator's bucket slice by varying
|
||||
// recommendationId/text per request (the unique index only catches
|
||||
// exact replays). 60/h is generous: a comprehensive insight rarely
|
||||
// has >10 recs, so ≥6 regenerations/h before throttle hits.
|
||||
const rl = await checkRateLimit(
|
||||
`insights-feedback:${user.id}`,
|
||||
60,
|
||||
60 * 60 * 1000,
|
||||
);
|
||||
if (!rl.allowed) {
|
||||
annotate({
|
||||
action: { name: "insights.recommendation.feedback" },
|
||||
meta: { outcome: "rate_limited" },
|
||||
});
|
||||
return apiError("Too many feedback submissions, try again later", 429);
|
||||
}
|
||||
|
||||
const { data: rawBody, error } = await safeJson<unknown>(request);
|
||||
if (error) return error;
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@
|
||||
*/
|
||||
|
||||
import { useMemo, useState } from "react";
|
||||
import { useMutation, useQueryClient } from "@tanstack/react-query";
|
||||
import { useMutation } from "@tanstack/react-query";
|
||||
import { ThumbsUp, ThumbsDown, Loader2, Check } from "lucide-react";
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
@@ -132,7 +132,6 @@ export function RecommendationFeedback({
|
||||
}: RecommendationFeedbackProps) {
|
||||
const { t } = useTranslations();
|
||||
const { user } = useAuth();
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
const [state, setState] = useState<FeedbackState>(() => {
|
||||
if (initialState) return initialState;
|
||||
@@ -189,7 +188,13 @@ export function RecommendationFeedback({
|
||||
const verdict = result.helpful ? "up" : "down";
|
||||
writeLocalState(user.id, recId, recText, verdict);
|
||||
setState(`submitted-${verdict}` as const);
|
||||
queryClient.invalidateQueries({ queryKey: ["insights"] });
|
||||
// v1.4.16 phase D reconcile (code-review H6) — feedback doesn't
|
||||
// affect the displayed insight content, so invalidating the entire
|
||||
// ["insights"] tree (8 queries: comprehensive + 7 per-status)
|
||||
// would force expensive refetches on a stale cache. The user-
|
||||
// visible state lives in localStorage + the local React state
|
||||
// above. The v1.4.17 ratchet that consumes feedback into the
|
||||
// next generation will scope its own invalidation.
|
||||
},
|
||||
onError: () => {
|
||||
// Optimistic-rollback: drop back to default so the user can
|
||||
|
||||
Reference in new issue
Block a user