mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 06:52:04 +00:00
fix(webhooks): rate-limit every verb that checks the shared secret
The Withings entrypoints, the Telegram bot webhook and the Coolify deploy hook limited POST and left the verification verbs unlimited. GET and HEAD compare the same secret and answer 200 against 401, which is an oracle for guessing it and a free load channel besides; timingSafeEqual protects the timing channel, not the status code. The project's rule is that the per-source limit runs before secret verification, and it was being applied per handler rather than per file. Each verification verb now runs the same limit its POST sibling used, on the same bucket. Telegram and the deploy hook grow the small local helper the Withings routes already share. HEAD re-shapes the limiter's JSON envelope into a bodyless 429 and keeps the rate-limit headers. The new guard walks every webhook route from the filesystem, resolves which local helpers reach a comparison and which reach the limiter, and fails when an exported verb reaches a comparison without the limiter in front of it. It asserts a non-zero match count so an empty match set cannot read as green. WHOOP already passed: it exports POST only.
This commit is contained in:
1 parent
214b268b41
commit
20812b54f5
10 files changed
+556
-22
No files matched your search
@@ -329,6 +329,31 @@ describe("GET /api/internal/deploy-webhook (reachability check)", () => {
|
||||
const res = await GET(req);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
// The reachability GET compares the same shared secret the POST does and
|
||||
// answers 200 against 401. Unlimited, that is a free oracle for guessing it.
|
||||
// A VALID secret over the limit must still be refused — that proves the
|
||||
// limiter runs before the comparison, not merely beside it.
|
||||
it("returns 429 for a valid secret when over the limit, on the POST bucket", async () => {
|
||||
vi.mocked(checkRateLimit).mockResolvedValue({
|
||||
allowed: false,
|
||||
remaining: 0,
|
||||
resetAt: Date.now() + 60_000,
|
||||
} as never);
|
||||
|
||||
const res = await GET(
|
||||
new NextRequest("http://localhost/api/internal/deploy-webhook", {
|
||||
method: "GET",
|
||||
headers: { "x-deploy-webhook-secret": "test-secret" },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(res.status).toBe(429);
|
||||
expect(checkRateLimit).toHaveBeenCalledTimes(1);
|
||||
expect(String(vi.mocked(checkRateLimit).mock.calls[0][0])).toMatch(
|
||||
/^deploy-webhook:/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /api/internal/deploy-webhook (timestamp replay protection)", () => {
|
||||
|
||||
@@ -202,24 +202,39 @@ async function notifyAdminsOfFailure(event: NormalizedEvent): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
annotate({ action: { name: "deploy.webhook" } });
|
||||
|
||||
// Rate-limit by client IP. 60 requests / minute is comfortably above
|
||||
// Coolify's per-event burst (typically 1-2 per deploy) but well below
|
||||
// a hostile actor flooding to fish for a working secret.
|
||||
/**
|
||||
* Rate-limit by client IP. 60 requests / minute is comfortably above
|
||||
* Coolify's per-event burst (typically 1-2 per deploy) but well below a
|
||||
* hostile actor flooding to fish for a working secret.
|
||||
*
|
||||
* Both verbs share the bucket, and both run it before the comparison: the
|
||||
* reachability GET checks the same secret and answers 200 against 401, so
|
||||
* leaving it unlimited would hand back the free guessing channel the POST
|
||||
* limit exists to close.
|
||||
*/
|
||||
async function applyWebhookRateLimit(
|
||||
request: NextRequest,
|
||||
): Promise<NextResponse | null> {
|
||||
const ip = getClientIp(request);
|
||||
const rl = await checkRateLimit(
|
||||
`deploy-webhook:${ip ?? "unknown"}`,
|
||||
60,
|
||||
60_000,
|
||||
);
|
||||
if (!rl.allowed) {
|
||||
return NextResponse.json(
|
||||
{ status: "rate_limited" },
|
||||
{ status: 429, headers: rateLimitHeaders(rl) },
|
||||
);
|
||||
}
|
||||
if (rl.allowed) return null;
|
||||
return NextResponse.json(
|
||||
{ status: "rate_limited" },
|
||||
{ status: 429, headers: rateLimitHeaders(rl) },
|
||||
);
|
||||
}
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
annotate({ action: { name: "deploy.webhook" } });
|
||||
|
||||
const limited = await applyWebhookRateLimit(request);
|
||||
if (limited) return limited;
|
||||
|
||||
const ip = getClientIp(request);
|
||||
|
||||
if (!hasValidSecret(request)) {
|
||||
return NextResponse.json({ status: "unauthorized" }, { status: 401 });
|
||||
@@ -293,6 +308,10 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
*/
|
||||
export const GET = apiHandler(async (request: NextRequest) => {
|
||||
annotate({ action: { name: "deploy.webhook.verify" } });
|
||||
|
||||
const limited = await applyWebhookRateLimit(request);
|
||||
if (limited) return limited;
|
||||
|
||||
if (!hasValidSecret(request)) {
|
||||
return NextResponse.json({ status: "unauthorized" }, { status: 401 });
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user