Skip to content

Conversation

@LordofAvernus
Copy link
Collaborator

@LordofAvernus LordofAvernus commented Oct 30, 2025

User description

关联的 issue

#3162

描述你的变更

  1. 修复SSH的密钥校验问题
  2. 关闭SSL 证书验证,支持自签名证书

确认项(pr提交后操作)

Tip

请在指定复审人之前,确认并完成以下事项,完成后✅


  • 我已完成自测
  • 我已记录完整日志方便进行诊断
  • 我已在关联的issue里补充了实现方案
  • 我已在关联的issue里补充了测试影响面
  • 我已确认了变更的兼容性,如果不兼容则在issue里标记 not_compatible
  • 我已确认了是否要更新文档,如果要更新则在issue里标记 need_update_doc


Description

  • 修正 SSH 密钥判断逻辑错误

  • 在 ListGitBranches 中加入 InsecureSkipTLS 参数

  • 在 CloneGitRepository 中启用自签名证书支持

  • 确保 Git 连接兼容 HTTPS/SSH 多种协议


Diagram Walkthrough

flowchart LR
  A["修正 SSH 密钥判断错误"] --> B["添加 InsecureSkipTLS 参数"]
  B --> C["更新 ListGitBranches 和 CloneGitRepository"]
Loading

File Walkthrough

Relevant files
Bug fix
configuration.go
修复 SSH 校验及添加 TLS 跳过参数                                                                       

sqle/api/controller/v1/configuration.go

  • 修正 SSH 私钥判断逻辑(条件由 != 更改为 ==)
  • 在远程引用列表中加入 InsecureSkipTLS
  • 在仓库克隆选项中添加 InsecureSkipTLS 参数
+8/-5     

@github-actions
Copy link

PR Reviewer Guide 🔍

🎫 Ticket compliance analysis ✅

3162 - Fully compliant

Compliant requirements:

  • 修复 SSH 密钥校验问题
  • 在 Git 分支获取和仓库克隆中增加 InsecureSkipTLS 参数
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

SQL注入:No。Sensitive information exposure:
使用 InsecureSkipTLS 参数关闭证书验证可能导致中间人攻击风险,请充分评估该方案的安全性。

⚡ Recommended focus areas for review

SSH密钥校验

该更改通过修正条件判断(改为判断 SSH 私钥是否为空字符串)来修复 SSH 密钥校验逻辑,请确认新逻辑符合预期,不会误判或遗漏 SSH 密钥的存在性。

if systemVariable.Data.SystemVariableSSHPrimaryKey == "" {
	return nil, errors.New(errors.DataNotExist, fmt.Errorf("git ssh private key not found"))
SSL证书校验

在 ListGitBranches 函数中新增 InsecureSkipTLS 参数以关闭 SSL 证书验证,虽然支持自签名证书,但可能存在中间人攻击等安全风险,建议进一步评估风险。

	Auth:            auth,
	InsecureSkipTLS: true, // 跳过 SSL 证书验证,支持自签名证书
})
SSL证书校验

在 CloneGitRepository 函数中添加 InsecureSkipTLS 参数以支持自签名证书,同样关闭了 SSL 证书验证,需确认是否有后续方案以降低安全风险。

	URL:             url,
	InsecureSkipTLS: true, // 跳过 SSL 证书验证,支持自签名证书
}

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
审查TLS验证选项

建议检查在TLS验证中跳过安全验证是否为必要的选择,因此可能引发中间人攻击等安全风险。应确保仅在确实需要支持自签名证书时使用该选项,且最好通过配置进行控制。

sqle/api/controller/v1/configuration.go [454]

-InsecureSkipTLS: true, // 跳过 SSL 证书验证,支持自签名证书
+InsecureSkipTLS: true, // 注意:仅在自签名证书场景下启用,生产环境建议使用严格验证
Suggestion importance[1-10]: 6

__

Why: The suggestion enhances security awareness by advising a stricter TLS validation policy in production, though it only modifies a comment in the code.

Low
Possible issue
检查私钥判断逻辑

请确认此处逻辑是否符合预期。当前条件判断用于检测密钥是否为空,从而返回错误,逻辑的反转可能会导致认证问题。建议仔细检查业务逻辑,避免因判断错误引起的认证失败。

sqle/api/controller/v1/configuration.go [423-425]

+if systemVariable.Data.SystemVariableSSHPrimaryKey == "" {
+		return nil, errors.New(errors.DataNotExist, fmt.Errorf("git ssh private key not found"))
+}
 
-
Suggestion importance[1-10]: 4

__

Why: The suggestion prompts a review of the SSH private key check logic without modifying code behavior, offering a minimal verification reminder.

Low

@winfredLIN winfredLIN merged commit 458b782 into main Oct 31, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants