-
-
Notifications
You must be signed in to change notification settings - Fork 181
fix: allow internal URLs in admin configs #516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||||||||||||||||||||||||
| export type ProviderUrlValidationError = { | ||||||||||||||||||||||||||||
| message: string; | ||||||||||||||||||||||||||||
| details: { | ||||||||||||||||||||||||||||
| error: string; | ||||||||||||||||||||||||||||
| errorType: "InvalidProviderUrl"; | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * 验证供应商地址是否是可用于连通性测试的 URL(仅做基础格式校验) | ||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
| * 说明:此处不再限制内网地址/端口,统一交由管理员配置策略控制。 | ||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] [SECURITY-VULNERABILITY] Cloud metadata SSRF endpoints are now allowed Why this is a problem: Suggested fix: // src/lib/validation/provider-url.ts
const hostname = parsedProviderUrl.hostname.toLowerCase().replace(/\.$/, "");
const normalizedHost = hostname.replace(/^\[|\]$/g, "");
const blockedMetadataHosts = new Set([
"169.254.169.254",
"metadata.google.internal",
"100.100.100.200",
"fd00:ec2::254",
]);
if (blockedMetadataHosts.has(normalizedHost)) {
return {
valid: false,
error: {
message: "供应商地址安全检查失败",
details: {
error: "不允许访问云厂商元数据服务地址",
errorType: "InvalidProviderUrl",
},
},
};
} |
||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
|
Comment on lines
+9
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 文档明确了验证范围,但需考虑特殊风险地址 文档清楚说明了仅做基础格式校验,不限制内网地址。这与 PR 的"管理员责任模型"一致。 然而,即使在管理员配置场景下,某些特殊地址(如云厂商元数据服务)也应当被阻止,因为:
建议在文档中补充安全考量说明: /**
* 验证供应商地址是否是可用于连通性测试的 URL(仅做基础格式校验)
*
* 说明:此处不再限制内网地址/端口,统一交由管理员配置策略控制。
+ *
+ * 注意:即使允许内网地址,仍应阻止云厂商元数据服务地址(如 169.254.169.254)
+ * 以防止管理员会话被盗用时的权限提升攻击。
*/📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
| export function validateProviderUrlForConnectivity( | ||||||||||||||||||||||||||||
| providerUrl: string | ||||||||||||||||||||||||||||
| ): { valid: true; normalizedUrl: string } | { valid: false; error: ProviderUrlValidationError } { | ||||||||||||||||||||||||||||
| const trimmedUrl = providerUrl.trim(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||
| const parsedProviderUrl = new URL(trimmedUrl); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (!["https:", "http:"].includes(parsedProviderUrl.protocol)) { | ||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||
| valid: false, | ||||||||||||||||||||||||||||
| error: { | ||||||||||||||||||||||||||||
| message: "供应商地址格式无效", | ||||||||||||||||||||||||||||
| details: { | ||||||||||||||||||||||||||||
| error: "仅支持 HTTP 和 HTTPS 协议", | ||||||||||||||||||||||||||||
| errorType: "InvalidProviderUrl", | ||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return { valid: true, normalizedUrl: trimmedUrl }; | ||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||
| valid: false, | ||||||||||||||||||||||||||||
| error: { | ||||||||||||||||||||||||||||
| message: "供应商地址格式无效", | ||||||||||||||||||||||||||||
| details: { | ||||||||||||||||||||||||||||
| error: error instanceof Error ? error.message : "URL 解析失败", | ||||||||||||||||||||||||||||
| errorType: "InvalidProviderUrl", | ||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial 考虑 DNS 重绑定攻击防护 当前验证仅在调用时解析一次 URL,不防止 DNS 重绑定攻击: 攻击场景:
缓解建议:
由于这属于运行时防护而非配置验证范畴,可作为后续改进项。 💡 DNS pinning 实现参考在 // 首次解析时记录 IP
const resolvedIp = await dns.lookup(hostname);
// 发送请求时使用固定 IP,但保留 Host header
fetch(`http://${resolvedIp}:${port}${path}`, {
headers: { Host: hostname }
});
// 或使用支持 DNS pinning 的 HTTP 客户端库🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
错误处理方式不一致可能导致问题。
getNotificationSettingsAction在权限检查失败时抛出错误(line 21),而同文件中的其他两个函数(updateNotificationSettingsAction和testWebhookAction)返回错误对象。这种不一致可能导致:🔎 建议统一错误处理方式
export async function getNotificationSettingsAction(): Promise<NotificationSettings> { const session = await getSession(); if (!session || session.user.role !== "admin") { - throw new Error("无权限执行此操作"); + throw new Error("无权限执行此操作"); // 如果保持抛出,确保调用方有 try-catch } return getNotificationSettings(); }或者修改返回类型以与其他函数保持一致:
🤖 Prompt for AI Agents