Conversation
# Reviewed, transaction id: 53999
* feat: webhook支持 --story=124942044 # Reviewed, transaction id: 54016 * feat: webhook支持相关的逻辑优化 --story=124942044 # Reviewed, transaction id: 54063 * feat: webhook支持相关的逻辑优化 --story=124942044 # Reviewed, transaction id: 54073
# Reviewed, transaction id: 54194
feat: 模板接口返回新增通知信息 --story=126257473
# Reviewed, transaction id: 54439
refactor: 修复定时任务trace不连通的问题 --story=126421543
# Reviewed, transaction id: 54441
refactor: 修复定时任务trace不连通的问题 --story=126421543
# Reviewed, transaction id: 54524
# Reviewed, transaction id: 54662
refactor: 修复部分drf请求获取trace属性报错的问题 --story=126421543
refactor: 去掉postcss依赖 #ignore
# Reviewed, transaction id: 54982
fix: yaml导出兼容旧版本流程 #7986
# Reviewed, transaction id: 55467
* fix: 恢复异步队列消息序列化方式 --story=126514126 # Reviewed, transaction id: 54999 * fix: webhook功能验收问题修复 --story=126514126 # Reviewed, transaction id: 55529
# Reviewed, transaction id: 55530
# Reviewed, transaction id: 55571
# Reviewed, transaction id: 55589
fix: 修复变量ip选择器手动拓扑方式过滤问题 --story=147302266
# Reviewed, transaction id: 55786
Feature template webhook
# Reviewed, transaction id: 55950
# Reviewed, transaction id: 55953
# Reviewed, transaction id: 72225
# Reviewed, transaction id: 72281
# Reviewed, transaction id: 72282
…e' into merge_master_to_humming_bird
# Reviewed, transaction id: 72402
# Reviewed, transaction id: 72359
# Reviewed, transaction id: 72402 # Reviewed, transaction id: 72414
…_bird Merge master to humming bird
# Reviewed, transaction id: 72472
# Reviewed, transaction id: 72472 # Reviewed, transaction id: 72473
…King/bk-sops into merge_master_to_humming_bird
…_bird Merge master to humming bird
定期同步master分支到hb
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| name: 安全检查 | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| is_safe: ${{ steps.check.outputs.is_safe }} | ||
| author_association: ${{ steps.check.outputs.author_association }} | ||
| steps: | ||
| - name: 检查贡献者身份 | ||
| id: check | ||
| run: | | ||
| echo "作者关联: ${{ github.event.pull_request.author_association }}" | ||
| # 允许的身份：OWNER（所有者）、MEMBER（成员）、COLLABORATOR（协作者） | ||
| if [[ "${{ github.event.pull_request.author_association }}" == "OWNER" ]] || \ | ||
| [[ "${{ github.event.pull_request.author_association }}" == "MEMBER" ]] || \ | ||
| [[ "${{ github.event.pull_request.author_association }}" == "COLLABORATOR" ]]; then | ||
| echo "✅ 可信任的贡献者，允许自动审查" | ||
| echo "is_safe=true" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "⚠️ 外部贡献者，需要手动批准" | ||
| echo "is_safe=false" >> $GITHUB_OUTPUT | ||
| fi | ||
| echo "author_association=${{ github.event.pull_request.author_association }}" >> $GITHUB_OUTPUT | ||
| # 第二步：代码审查（只对可信贡献者自动运行） | ||
| code-review: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the problem, add an explicit permissions block to the security-check job so that its GITHUB_TOKEN has only the minimal required scope. This job only reads event context and writes to GITHUB_OUTPUT; it does not interact with repository contents, issues, or pull requests directly, so we can safely set contents: read (or even contents: none if no repo content is accessed). Using contents: read is a common minimal baseline and matches the recommendation pattern.
Concretely:
- In
.github/workflows/code_review.yml, within thesecurity-checkjob definition, add apermissions:section at the same indentation level asruns-onandoutputs. - Set:
contents: readto allow read access to repository contents if ever needed for simple checks.- Optionally
pull-requests: readis not strictly necessary here because the job is usinggithub.eventcontext, but it’s safe to omit; a minimal fix is justcontents: read.
No new imports or external libraries are needed; this is purely a YAML configuration change.
| @@ -11,6 +11,8 @@ | ||
| security-check: | ||
| name: 安全检查 | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| outputs: | ||
| is_safe: ${{ steps.check.outputs.is_safe }} | ||
| author_association: ${{ steps.check.outputs.author_association }} |
|
hanouyang seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 代码审查总结
这是一个大规模合并 PR（479 个文件，+44,457/-22,180 行），将 hb 分支合入多租户分支。由于变更规模巨大，以下重点关注高严重度安全和逻辑问题。
🚨 关键发现
1. 安全：JSON 解析缺少空 body 检查
位置: gcloud/apigw/decorators.py:84
- ✅ 已修复: 代码已添加
if request.body检查，避免空 body 导致 JSON 解析异常 - 原问题:
params = json.loads(request.body)→ 已修复为params = json.loads(request.body) if request.body else {}
2. 安全：MCP 网关装饰器的权限控制逻辑
位置: gcloud/apigw/decorators.py:164-327
- 新增
mcp_apigw装饰器用于过滤敏感字段 ⚠️ 注意: 该装饰器基于app_code前缀或 HTTP header 来判断是否过滤响应- 建议：确保
settings.APIGW_MCP_APP_CODE_PREFIX和APIGW_MCP_SERVER_ID_HEADER配置正确，防止绕过过滤
3. 逻辑：Webhook 配置批量操作的事务完整性
位置: gcloud/apigw/views/apply_webhook_configs.py:70-128
- ✅ 良好实践: 使用
transaction.atomic()确保原子性 - ✅ 使用
ignore_conflicts=True避免唯一约束冲突 - ✅ 先删除旧订阅再批量创建新订阅，逻辑清晰
4. 性能：深度优先搜索可能导致栈溢出
位置: gcloud/apigw/views/get_task_effective_time.py:187-197
⚠️ 递归函数_dfs_calculate_effective_time和_find_converge_gateway处理复杂流程图时可能导致递归深度过大- 建议：对于超大流程图，考虑使用迭代+显式栈代替递归
5. 验证：Webhook Serializer 缺少 None 值检查
位置: gcloud/apigw/serializers.py:52-63
- 🔒
validate_extra_info方法中raw_value > max_val未检查raw_value是否为 None - 风险: 如果
extra_info中字段缺失或为 None，会导致 TypeError - 建议: 添加
if raw_value is not None and raw_value > max_val:
6. 安全：IAM 权限拦截器缺少异常处理
位置: gcloud/iam_auth/view_interceptors/apigw/apply_webhook_configs.py:28-51
⚠️ allow_or_raise_immediate_response_for_resources_list可能抛出权限异常- 当前代码未捕获 IAM 异常，可能导致用户看到不友好的错误信息
- 建议：添加异常处理并返回标准化错误响应
✨ 积极改进
- ✅ 添加了 OpenTelemetry 追踪支持 (
gcloud/core/trace.py) - ✅ 新增 Coverage 配置 (
.coveragerc) - ✅ 添加 GitHub Actions 自动化代码审查工作流
📌 建议
- 优先修复: Serializer 中的 None 值检查（第 5 项）
- 代码审查: 确保 MCP 网关装饰器的配置安全（第 2 项）
- 性能测试: 验证递归函数在大流程图下的性能（第 4 项）
- 单元测试: 为新增的 webhook 配置和 MCP 装饰器添加测试覆盖
注: 由于 PR 规模巨大，建议分批审查。本次审查聚焦于安全和逻辑关键问题，其他代码质量问题建议通过 linting 工具自动化检测。
| for field, rule in extra_info_mappings.items(): | ||
| raw_value = extra_info.get(field) | ||
| max_val = rule["max_value"] | ||
| if raw_value > max_val: |
There was a problem hiding this comment.
🔒 None 值检查缺失: raw_value 可能为 None，直接与 max_val 比较会导致 TypeError。建议改为：if raw_value is not None and raw_value > max_val:
| if app_code: | ||
| # 从 settings 获取 v_mcp 前缀 | ||
| v_mcp_prefix = getattr(settings, "APIGW_MCP_APP_CODE_PREFIX", "v_mcp") | ||
| # 检查 app_code 是否以指定前缀开头 |
There was a problem hiding this comment.
app_code 前缀或 HTTP header 判断是否过滤敏感字段。需确保 settings.APIGW_MCP_APP_CODE_PREFIX (默认 v_mcp) 和 APIGW_MCP_SERVER_ID_HEADER 配置安全，防止攻击者伪造绕过过滤。
|
|
||
|
|
||
| def _dfs_calculate_effective_time( | ||
| node_id, |
There was a problem hiding this comment.
⚡ 性能风险: 递归函数处理复杂流程图时可能导致栈溢出。对于超大流程图（节点数 >1000），建议使用迭代+显式栈替代递归实现。
8 participants
No description provided.