fix(ai): prevent SSRF in AI chat endpoints#306
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThis change introduces URL validation for AI API endpoints, enforcing a configurable allowlist of hosts and blocking requests to internal/private IP ranges. Configuration files are updated to support the new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/resources/application-dev.yml`:
- Around line 94-111: Remove the loopback addresses from the dev allowlist to
avoid weakening SSRF protection: delete the "localhost" and "127.0.0.1" entries
from the ai.allowed-hosts list in the dev config so validateFinalUrl() no longer
short-circuits to local services; rely on the separate local profile for
localhost access instead.
In
`@base/src/main/java/com/tinyengine/it/service/app/impl/v1/AiChatV1ServiceImpl.java`:
- Around line 270-287: The current AiChatV1ServiceImpl logic treats a null/empty
config.getAllowedHosts() as “allow all”, which is unsafe; change it to
fail-closed: if allowedHosts is null or empty then throw a ServiceException
indicating no allowed hosts configured (preserving the existing isLoopback
short-circuit if you want to allow local-only calls) unless an explicit,
well-named override flag on the config (e.g., config.isAllowAnyHost()) is true;
keep the existing host match flow and calls to enforceHttpsAndIpCheck(uri, host)
and update tests to cover the new behavior.
- Around line 296-308: The current DNS check uses resolveHostAddresses(host)
then later lets HttpClient.perform a fresh DNS lookup, creating a TOCTOU; fix by
pinning the validated IP into the request URI and preserving TLS/Host
verification: call resolveHostAddresses(host), validate each InetAddress with
isBlockedAddress, pick a validated IP (e.g. addresses[0].getHostAddress()),
build a new URI using that IP (keeping scheme, port, path, query, fragment) and
then build the HttpRequest with that IP-URI while adding a "Host" header set to
the original hostname so SNI/HTTP Host are preserved (use
HttpRequest.newBuilder(...) and then call HttpClient.send(...) with that
request). Ensure all references to resolveHostAddresses, isBlockedAddress,
HttpRequest.newBuilder, and HttpClient.send are updated accordingly to avoid any
re-resolution at connect time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9855d627-1249-410a-b42c-b7596f7bfed4
📒 Files selected for processing (6)
app/src/main/resources/application-alpha.ymlapp/src/main/resources/application-dev.ymlapp/src/main/resources/application-local.ymlbase/src/main/java/com/tinyengine/it/config/OpenAIConfig.javabase/src/main/java/com/tinyengine/it/service/app/impl/v1/AiChatV1ServiceImpl.javabase/src/test/java/com/tinyengine/it/service/app/impl/v1/AiChatV1ServiceImplTest.java
2 participants
Summary
简介
主要包括配置绑定改造、URL 白名单校验、重定向禁止、特殊网段拦截，以及对应测试补充
Testing
Summary by CodeRabbit
New Features
Tests