mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix(security): security audit and hardening (#207)
* fix(security): harden SVG sanitizer, rate limiting, and analytics defaults - SVG: add control-char stripping in href values to block whitespace/null-byte obfuscated javascript: URIs; block <feImage> with external href (SSRF via SVG filter primitives); expand test suite to 32 inline bypass payloads - Rate limiting: add per-route limits on tool endpoints (60/min) and batch (20/min); fix compose files defaulting RATE_LIMIT_PER_MIN to 0 which mapped to 50,000 in code; simplify rate limit registration to use env.ts default - Analytics: default ANALYTICS_ENABLED to false so self-hosters do not unknowingly send telemetry - Docker: add --max-time 5 and -s flags to compose healthcheck curl commands * fix: remove stale login limit bypass, reduce error log noise, clean up fixtures - Fix getLoginAttemptLimit() ignoring LOGIN_ATTEMPT_LIMIT when global rate limit exceeded 1000/min, which let the global limit override the stricter per-route login brute-force protection - Downgrade rate limit 429 responses from error to warn level in the global error handler to avoid log noise and unnecessary Sentry reports - Log 4xx client errors at warn level instead of error level - Remove 11 orphaned SVG attack fixture files replaced by inline test payloads
This commit is contained in:
+10
-7
@@ -144,12 +144,16 @@ app.addContentTypeParser("application/json", { parseAs: "string" }, (_request, b
|
||||
|
||||
app.setErrorHandler((error: Error & { statusCode?: number }, request, reply) => {
|
||||
const statusCode = error.statusCode ?? 500;
|
||||
request.log.error(
|
||||
{ err: error, url: request.url, method: request.method },
|
||||
"Unhandled request error",
|
||||
);
|
||||
if (statusCode >= 500) {
|
||||
if (statusCode === 429) {
|
||||
request.log.warn({ url: request.url, method: request.method }, "Rate limit exceeded");
|
||||
} else if (statusCode >= 500) {
|
||||
request.log.error(
|
||||
{ err: error, url: request.url, method: request.method },
|
||||
"Unhandled request error",
|
||||
);
|
||||
captureException(error, request);
|
||||
} else {
|
||||
request.log.warn({ err: error, url: request.url, method: request.method }, "Request error");
|
||||
}
|
||||
reply.status(statusCode).send({
|
||||
error: statusCode >= 500 ? "Internal server error" : error.message,
|
||||
@@ -178,9 +182,8 @@ app.addHook("onSend", async (_request, reply) => {
|
||||
});
|
||||
|
||||
// Always register rate-limit plugin so per-route limits (login brute-force protection) work.
|
||||
// RATE_LIMIT_PER_MIN=0 means no global limit (per-route limits still apply).
|
||||
await app.register(rateLimit, {
|
||||
max: env.RATE_LIMIT_PER_MIN > 0 ? env.RATE_LIMIT_PER_MIN : 50_000,
|
||||
max: env.RATE_LIMIT_PER_MIN,
|
||||
timeWindow: "1 minute",
|
||||
allowList: (request) => !request.url.startsWith("/api/"),
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user