diff by dengyh · Pull Request #567 · TencentBlueKing/BKFlow

dengyh · 2026-01-07T08:10:45Z

No description provided.

* feat: 网关增加流程版本管理sdk接口 --story=129266074 # Reviewed, transaction id: 68622 * fix: 修改删除接口逻辑 --story=129266074 # Reviewed, transaction id: 68626

* feat: 补充第三方插件网关接口 --story=129266074 * feat: 补充接口文档 --story=129266074 * feat: 补充流程版本关联接口文档 --story=129266074 # Reviewed, transaction id: 68892

* fix: 修复网关接口请求方法错误 --story=129266074 * fix: 修复接口文档错误 --story=129266074

* fix: 版本快照列表协议修改 --story=128050351 # Reviewed, transaction id: 69031 * fix: 添加异常捕获 --story=128050351 # Reviewed, transaction id: 69036

* feat: 补充模板发布免认证接口 --story=129543443 # Reviewed, transaction id: 69289 * fix: 修复接口逻辑 --story=129543443 # Reviewed, transaction id: 69317

# Reviewed, transaction id: 69369

* fix: 修复网关接口发布流程重复问题 --story=129266074 * feat: 调试功能增加作用域校验 --story=129266074 # Reviewed, transaction id: 69579 * fix: 修复权限校验问题 --story=129266074 # Reviewed, transaction id: 69580

tests/engine/utils/test_validate.py

+            assert DomainValidator.validate("http://sub.example.com")[0] is True
+            is_valid, err = DomainValidator.validate("http://other.com")
+            assert is_valid is False
+            assert "example.com" in err


tests/plugin_service/test_plugin_client.py

+                        ["system", "bk_plugins"], environment="prod"
+                    )
+
+                    assert "http://paas.example.com" in url


In general, instead of checking whether a trusted base URL is a substring of another URL string, parse the URL and assert on its structured components (scheme, hostname, path, query). This avoids issues where the expected host appears in an unexpected place or is combined with other content.

For this specific test, we can keep the existing behavior expectations while making the check precise by parsing url with urllib.parse.urlparse and asserting that scheme == "http" and netloc == "paas.example.com". This removes the substring check and aligns with the recommendation to operate on parsed URLs. We only need to modify the test method test_prepare_paas_api_request_with_token in tests/plugin_service/test_plugin_client.py (line 702 onward) and add a standard-library import of urlparse at the top of that file. No application logic needs to change, only the way the test validates the URL.

Concretely:

Add from urllib.parse import urlparse near the other imports at the top of tests/plugin_service/test_plugin_client.py.

Replace assert "http://paas.example.com" in url with:

parsed_url = urlparse(url)

assert parsed_url.scheme == "http"

assert parsed_url.netloc == "paas.example.com"
This maintains the same intention (ensuring the base host is used) while avoiding substring sanitization.

tests/plugin_service/test_plugin_client.py

+                                        ["system", "bk_plugins"]
+                                    )
+
+                                    assert "example.com" in url


bkflow/apigw/views/release_template.py

+        bump_custom(new_version, instance.version)
+    except ValueError as e:
+        logger.error(str(e))
+        return JsonResponse({"result": False, "message": f"版本号不符合规范: {str(e)}", "code": err_code.VALIDATION_ERROR.code})


In general, the fix is to avoid returning raw exception details to the client. Instead, log the exception (optionally with a stack trace) on the server and send a generic, user-friendly error message that does not expose internal implementation details.

For this specific case in bkflow/apigw/views/release_template.py, we should change the except ValueError as e: block so that it no longer interpolates str(e) into the message field of the JSON response. Instead:

Keep logging the error, ideally with stack information.

Return a generic validation message like “版本号不符合规范” without including str(e).

Concretely:

At lines 66–68:

Replace logger.error(str(e)) with a form that logs the exception more fully without affecting the client response (e.g. logger.exception("Invalid version format")), though we can also keep the original message if desired.

Replace return JsonResponse({"result": False, "message": f"版本号不符合规范: {str(e)}", ...}) with a generic message that omits str(e), e.g. {"result": False, "message": "版本号不符合规范", ...}.

No new imports or additional methods are required; the file already imports logging and configures logger.

github-actions

代码审查总结

本次审查重点关注新增的并发请求、线程池、API客户端以及权限验证逻辑。发现以下关键问题需要处理：

🚨 严重问题

并发请求缺少超时控制 - batch_request 中线程池执行可能无限期等待
错误处理逻辑不一致 - 第一次请求和后续批量请求的错误检查存在差异

🔒 安全问题

敏感信息泄露风险 - Cookie 中的 bk_ticket 未经验证直接传递到 API
线程本地存储清理不完整 - 异常时可能导致数据泄露

⚡ 性能问题

线程数未限制 - ThreadPool() 可能创建大量线程导致资源耗尽

⚠️ 逻辑问题

数据库连接处理 - 仅在 finally 块关闭，异常场景可能遗留连接
响应数据校验缺失 - release_template API 未验证空间版本管理配置时机不当

建议优先修复严重和安全问题，性能问题在高并发场景下需要关注。

github-actions

代码审查总结

本次审查重点关注新增的免登录发布流程、删除模板逻辑变更、权限控制以及前端错误处理。发现以下需要关注的问题：

🔒 安全问题

免登录接口缺少关键权限验证 - release_template 使用 @login_exempt 但权限检查可能不足
软删除未清理关联数据 - delete_template 改为软删除但未处理快照等关联数据

⚠️ 逻辑问题

版本管理开关检查不一致 - 部分接口缺少版本管理功能开关验证
前端错误处理不完整 - try-catch 块捕获异常后仅 console.log，未向用户展示错误

✨ 改进建议

权限验证逻辑可优化 - has_object_permission 中存在重复的权限检查调用

建议优先修复安全问题，逻辑问题会影响功能一致性。

# Reviewed, transaction id: 70027

* fix: 更换接口修复权限问题 # Reviewed, transaction id: 70128 * fix: 更换接口修复权限问题 # Reviewed, transaction id: 70130

# Reviewed, transaction id: 70746

github-actions

代码审查总结

本次审查发现以下关键问题需要处理：

🚨 严重问题

软删除缺少关联数据清理 - delete_template 改为软删除但未处理快照等关联数据，可能导致数据泄露
权限控制逻辑存在漏洞 - 新增免登录接口 release_template 使用 @login_exempt，需要额外验证权限检查是否充分

🔒 安全问题

敏感信息可能泄露 - AJAX 请求新增 withCredentials: true 可能导致跨域传递敏感凭证
版本管理权限检查不一致 - 部分接口缺少版本管理开关验证

⚠️ 逻辑问题

错误处理不完整 - 多处前端 try-catch 仅输出 console.log，用户无法感知错误
API 响应结构不一致 - getTemplateVersionSnapshotList 返回值从 res 改为 res.data，需确保后端一致性

建议优先修复严重和安全问题。

* refactor: 使用AI补充存量功能的测试用例 --story=129211203 * refactor: 删除冗余的无意义的测试用例 --story=129211203 * refactor: 测试用例排除非代码文件 --story=129211203 * refactor: 测试覆盖率提升到80%以上 --story=129211203

github-actions

代码审查总结

已审查此 PR 的主要变更，发现以下关键问题需要处理：

🚨 严重问题

release_template.py:48 - JSON 解析缺少异常处理，可能导致 500 错误
release_template.py:52 - 返回错误响应时未设置正确的 HTTP 状态码
models.py:309-320 - force 参数存在安全隐患，可绕过草稿验证直接发布任意版本

⚠️ 需改进

delete_template.py:38 - 删除操作记录被移除，影响审计追踪
models.py:311 - 控制流不清晰，return 语句位置会让人困惑

✨ 积极改进

软删除替代硬删除（delete_template.py）
权限系统增强支持 MOCK 权限
新增版本管理和发布功能

建议优先修复前 3 个严重问题后再合并。

github-actions · 2026-01-07T08:30:13Z

bkflow/apigw/views/release_template.py

+    try:
+        instance = Template.objects.get(id=template_id, space_id=space_id)
+    except Template.DoesNotExist:
+        return JsonResponse({"result": False, "message": "Template not found"})


⚠️ 应设置正确的 HTTP 状态码（如 404），当前返回 200 会误导客户端

github-actions · 2026-01-07T08:30:13Z

bkflow/template/models.py

        except TemplateSnapshot.DoesNotExist:
-            logger.warning(f"未找到模板（ID: {self.id}）的草稿快照（draft=True）")
-            raise ValidationError(f"该模板{self.id}没有草稿版本")
+            if data.get("force", False):


🔒 force 参数可绕过草稿验证直接发布任意 pipeline_tree，建议加权限检查或审核流程

github-actions · 2026-01-07T08:30:13Z

bkflow/apigw/views/release_template.py

+@check_jwt_and_space
+@return_json_response
+def release_template(request, space_id, template_id):
+    data = json.loads(request.body)


🚨 缺少异常处理：json.loads 可能抛出 JSONDecodeError，建议添加 try-except 或使用 serializer 统一验证

github-actions · 2026-01-07T08:30:13Z

bkflow/apigw/views/delete_template.py

-    TemplateOperationSource.api.name,
-    extra_info={"tag": "apigw"},
-)
 def delete_template(request, space_id, template_id):


⚠️ 删除操作记录被移除，影响合规审计。建议保留 record_operation 装饰器

github-actions · 2026-01-07T08:30:29Z

bkflow/apigw/views/release_template.py

+        instance.snapshot_id = snapshot.id
+        instance.save()
+
+    TemplateOperationRecord.objects.create(


⚡ TemplateOperationRecord 创建在事务外，若此步骤失败会导致数据不一致。建议移入事务内

github-actions · 2026-01-07T08:30:29Z

bkflow/template/models.py

        except Exception as e:
            logger.error(f"发布模板草稿时发生错误（template_id={self.id}）: {e}", exc_info=True)
            raise ValidationError("发布模板失败，请稍后重试")
+        return template_snapshot


⚠️ return 语句移到 try-except 外部，代码流不清晰。建议在每个分支明确 return

github-actions · 2026-01-07T08:30:29Z

bkflow/apigw/views/delete_template.py

-    count, _ = Template.objects.filter(space_id=space_id, id=template_id).delete()
-    return {"result": True, "data": {"count": count}, "code": err_code.SUCCESS.code}
+    Template.objects.filter(space_id=space_id, id=template_id).update(is_deleted=True)
+    return {"result": True, "data": {}, "code": err_code.SUCCESS.code}


⚠️ 软删除缺少状态验证：应检查 is_deleted 避免重复删除；考虑检查是否有关联任务运行中

github-actions · 2026-01-07T08:30:40Z

bkflow/apigw/views/release_template.py

+    if TemplateSnapshot.objects.filter(template_id=instance.id, version=new_version).exists():
+        return JsonResponse({"result": False, "message": "版本已存在", "code": err_code.VALIDATION_ERROR.code})
+
+    try:


⚠️ 版本验证在快照存在性检查之后，顺序不佳。建议先验证版本格式，再检查重复，减少不必要的数据库查询

guohelu and others added 10 commits December 19, 2025 10:37

feat: 网关增加流程版本管理sdk接口 --story=129266074 (#547)

bb44e30

* feat: 网关增加流程版本管理sdk接口 --story=129266074 # Reviewed, transaction id: 68622 * fix: 修改删除接口逻辑 --story=129266074 # Reviewed, transaction id: 68626

refactor: 修复api-resources文档 --story=129023052 (#551)

445078a

feat: 补充第三方插件网关接口 --story=129266074 (#552)

28bee4b

* feat: 补充第三方插件网关接口 --story=129266074 * feat: 补充接口文档 --story=129266074 * feat: 补充流程版本关联接口文档 --story=129266074 # Reviewed, transaction id: 68892

fix: 修复网关接口请求方法错误 --story=129266074 (#553)

2003af0

* fix: 修复网关接口请求方法错误 --story=129266074 * fix: 修复接口文档错误 --story=129266074

fix: 版本快照列表协议修改 --story=128050351 (#554)

6229a39

* fix: 版本快照列表协议修改 --story=128050351 # Reviewed, transaction id: 69031 * fix: 添加异常捕获 --story=128050351 # Reviewed, transaction id: 69036

fix: 修复网关接口文档问题 --story=129266074 (#555)

0e9ad05

feat: 补充模板发布免认证接口 --story=129543443 (#557)

bcdd564

* feat: 补充模板发布免认证接口 --story=129543443 # Reviewed, transaction id: 69289 * fix: 修复接口逻辑 --story=129543443 # Reviewed, transaction id: 69317

fix: 子流程来回切换任务id未变导致画布状态丢失问题修复 #ignore

52fa939

# Reviewed, transaction id: 69369

fix: 修改网关sdk版本 --story=129266074 (#560)

fc92e23

github-advanced-security bot found potential problems Jan 7, 2026

View reviewed changes

github-actions bot reviewed Jan 7, 2026

View reviewed changes

dengyh force-pushed the master branch from a9918bd to fc92e23 Compare January 7, 2026 08:16

github-actions bot reviewed Jan 7, 2026

View reviewed changes

guohelu and others added 4 commits January 7, 2026 16:20

feat: 网关api发布支持强制发布 --story=129266074 (#561)

450f5ae

fix: 子流程节点详情更换模板名称获取方式 --story=127611598

b5e7f5f

# Reviewed, transaction id: 70027

Feat/stage canvas 20251231 (#564)

a08c2c6

* fix: 更换接口修复权限问题 # Reviewed, transaction id: 70128 * fix: 更换接口修复权限问题 # Reviewed, transaction id: 70130

fix: TagSelect组件使用远程数据源时，请求支持携带cookie凭证信息 --story=129798291

ae0c58e

# Reviewed, transaction id: 70746

github-actions bot reviewed Jan 7, 2026

View reviewed changes

dengyh closed this Jan 7, 2026

@@ -23,6 +23,7 @@
             import pytest
             from django.core.files.uploadedfile import UploadedFile
             from requests import HTTPError
+            from urllib.parse import urlparse
             from plugin_service import env
             from plugin_service.exceptions import PluginServiceException, PluginServiceNotUse
@@ -708,7 +709,9 @@
                                     ["system", "bk_plugins"], environment="prod"
                                 )
-                                assert "http://paas.example.com" in url
+                                parsed_url = urlparse(url)
+                                assert parsed_url.scheme == "http"
+                                assert parsed_url.netloc == "paas.example.com"
                                 assert params["private_token"] == "test_token"
                 def test_prepare_paas_api_request_without_token(self):

@@ -64,8 +64,8 @@
                 try:
                     bump_custom(new_version, instance.version)
                 except ValueError as e:
-                    logger.error(str(e))
-                    return JsonResponse({"result": False, "message": f"版本号不符合规范: {str(e)}", "code": err_code.VALIDATION_ERROR.code})
+                    logger.error("Invalid version format: %s", str(e))
+                    return JsonResponse({"result": False, "message": "版本号不符合规范", "code": err_code.VALIDATION_ERROR.code})
                 with transaction.atomic():
                     data = {"username": request.user.username, **ser.validated_data}

Comments

Conversation

dengyh commented Jan 7, 2026

Uh oh!

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Copilot Autofix

github-actions bot left a comment

Choose a reason for hiding this comment

代码审查总结

🚨 严重问题

🔒 安全问题

⚡ 性能问题

⚠️ 逻辑问题

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

代码审查总结

🔒 安全问题

⚠️ 逻辑问题

✨ 改进建议

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

代码审查总结

🚨 严重问题

🔒 安全问题

⚠️ 逻辑问题

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

代码审查总结

🚨 严重问题

⚠️ 需改进

✨ 积极改进

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants