Replies: 1 comment 1 reply
|
Hi rennf93, Thanks for the proposal — it's a well-thought-out design, and we're happy to work with it. We had discussed this before, and we're on board with building it on gin-guard v1.5.0 and guard-core-go. One thing to consider: RAGFlow may run multiple Go API services. If the guard settings are configured through config files, that means every Go API service's config file would need to be modified, and some of those settings can't be changed dynamically. With that in mind, here are my thoughts on your proposal:
Best, |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Disclosure: I maintain gin-guard and guard-core-go, the two libraries this proposal builds on.
Problem
RAGFlow API servers are often exposed to the internet, and the Go API server (
internal/server/api/server.go) has no inbound limiter: no per-IP rate limit, no IP allow or block list, and nothing that drops scanner traffic before it reaches a handler. Two things make this matter for RAGFlow in particular:The trusted proxy work (the fix for #19549, the spoofed X-Forwarded-For bypass of the agent webhook IP whitelist) already gives the server a correct client address. I build on that address instead of adding a second notion of it.
Proposal
An optional middleware, default off, built on gin-guard v1.5.0 (MIT) and guard-core-go v4.4.0. When
api.guard.enabledisn't set nothing is registered and behavior is byte for byte unchanged.When it's on:
The choices I made on purpose:
api.trusted_proxieslist thatcommon.ConfigureTrustedProxiesgives gin (loopback by default, the bundled nginx), so it resolves the client the same wayc.ClientIP()does. A spoofed X-Forwarded-For from an untrusted peer is ignored by both, and a rotating header can't evade the limit. The guard side is covered by tests (loopback peer honored, other peers ignored, rotating header can't evade the limit). The gin side is already covered bytrusted_proxies_test.go.gin.Recovery()and before the routes. The Go API has no CORS middleware today (a few handlers set headers themselves), so there's nothing to keep outside of it. If one is added later and registered before the guard, 429 and 403 responses carry its headers, and the tests exercise that ordering with a stand-in middleware./health,/api/v1/system/ping,/api/v1/system/healthz) skip the guard entirely, so probes are never limited or blocked.api.guard.use_kvrocks: truekeeps them in the Kvrocks instance the Go services already use, so replicas share them. It fails open if Kvrocks is unreachable.What else the engine offers
I kept the branch to a small, safe subset. The rest is already in gin-guard and I can wire whatever you find useful:
/api/v1/admin/*, including user management), a strict budget with bans onPOST /api/v1/admin/login, request size and content-type limits on document uploads, and per-client budgets on/api/v1/chatsand dataset search.Configuration
Under the existing
api:section ofservice_conf.yaml, next totrusted_proxies, withRAGFLOW_API_GUARD_*environment overrides (same mechanism asRAGFLOW_API_HTTP_PORT):Documented in
docs/administrator/configurations/configurations.md,conf/service_conf.yamlanddocker/service_conf.yaml.template. It applies to the API server and, with its own counters, to the admin server.Tests
Unit tests in
internal/guardandinternal/server/config: disabled registers nothing; a burst returns 429 and the response keeps the outer middleware's CORS header; allowlist returns 403; spoofed and rotating X-Forwarded-For from an untrusted peer is ignored; an attack in the query is blocked while the same text in a body or an excluded parameter passes; screening is off unless asked for; an SSE stream is delivered incrementally; a 64 MiB upload passes; config defaults, YAML and environment parsing. I couldn't start the full server locally (it needs MySQL, a doc engine, MinIO and the native libraries), so I also ran the real middleware in a small standalone gin server and probed it with curl: 429 after the limit, health unaffected, SSE ticks arriving 700 ms apart, attack query rejected.Dependency impact
go.modgainsgithub.com/rennf93/gin-guard v1.5.0andgithub.com/rennf93/guard-core-go/v4 v4.4.0plus one transitive module,github.com/oschwald/maxminddb-golang v1.13.1(GeoIP reader, unused unless country rules are configured). It also raises modules already there:go-redis/v99.18.0 to 9.23.0,zeebo/xxh31.0.2 to 1.1.0,dlclark/regexp21.10.0 to 1.12.0,klauspost/cpuid/v22.3.0 to 2.4.0,go.uber.org/atomic1.11.0 to 1.12.0 andgo.yaml.in/yaml/v33.0.4 to 3.0.5, and drops the now-unneeded indirectdgryski/go-rendezvous. Everything else the libraries need is already in the module graph (gin v1.12.0 matches).One known follow-up: guard-core-go v4.4.0 prints a few log lines on every request through the standard
logpackage (for example "No countries blocked or whitelisted"). The fix that routes them throughslogat Debug level (hidden by default) is merged upstream (Guard-Core/guard-core-go#111) and not released yet, and I'll bump the dependency in the PR once it ships. I didn't work around it, and it only affects deployments that turn the guard on.Alternatives
limit_req, a WAF or a CDN): they protect the edge and I'd document them either way, but it's a different layer. This is the application layer inside the Go server, using the same client-address resolution as the rest of the Go code, so the two complement each other. It also gives deployments that run the bundled stack as is a default they can switch on.Status
A tested branch is ready: https://github.com/rennf93/ragflow/tree/feat/gin-guard-security-middleware. I'll open a PR referencing this discussion if there's interest. I'm happy to trim it (drop screening or the Kvrocks option, say) or move the wiring elsewhere in the server if you want a different shape.
Who I am: I'm rennf93. I wrote fastapi-guard, public since August 2024 (first PyPI release, 0.1.0, on 2024-08-26), now at 825 GitHub stars, 65 releases and about 62,000 PyPI downloads last month (pypistats.org). The engines and adapters for other languages and frameworks came later and run the same detection rules, checked in CI against shared conformance fixtures.
All reactions