Skip to content

fix(audit): enhance SQL audit middleware to utilize session context from streamExecute requests (cherry-pick to 4.2601.x)#624

Merged
LordofAvernus merged 1 commit into
release-4.2601.xfrom
fix-odc-audit-4.2601.x
May 20, 2026
Merged

fix(audit): enhance SQL audit middleware to utilize session context from streamExecute requests (cherry-pick to 4.2601.x)#624
LordofAvernus merged 1 commit into
release-4.2601.xfrom
fix-odc-audit-4.2601.x

Conversation

@LordofAvernus
Copy link
Copy Markdown
Collaborator

@LordofAvernus LordofAvernus commented May 20, 2026

User description

What this PR does

将 dms@c3c45cc37f0a2f7ae0916c5a1c7a996e2234d883(main 上由 #623 跟进合并)的修复同步到 release-4.2601.x。详细问题描述见 #622

同步策略

由于 fix-odc-audit 相对 release-4.2601.x 领先 157 个 commit(含大量与本 issue 无关的特性),无法直接合并。本 PR 通过 git cherry-pick c3c45cc3 单独把 ODC SQL 审核中间件的修复挑出来,仅触及 internal/sql_workbench/service/sql_workbench_service.go 一个文件。

cherry-pick 时合并冲突已按以下原则解决:只把 c3c45cc 直接涉及的语义同步过来,不夹带 main 上其他无关重构——

  • 保留 4.2601.x 上 fmt.Errorf 形式的中间错误返回(getDMSUserId / cache / GetDBService),不替换为 errors.New(locale.Bundle.LocalizeMsgByCtx(...)) i18n 风格(那是 main 上另一条独立改造);
  • 仅按 commit 注释「解析仅服务于审核辅助路径,解析失败不应直接阻塞用户的 SQL 执行」的语义,把:
    • parseStreamExecuteRequest 错误,
    • sql == "" || sidInfo == nil || datasourceID == "",
    • !isEnableSQLAudit
      这三处从 fmt.Errorf 改为 return next(c)
  • 重命名 parseSidToDatasourceIDparseStreamExecuteSid,引入 streamExecuteSidInfo{datasourceID, schemaName, dbID}fillStreamExecuteSidFromBase64getODCDatabaseName
  • base64 解码由 StdEncoding 改为 URLEncoding(ODC 用 Base64.getUrlEncoder(),sid 含 -/_)。
  • callSQLEAudit 增加 schemaName 参数,在 AuditSQLReq 中带上 SchemaName

缺陷与修复(同 main PR)

  1. 未启用审核反向阻塞 SQL 执行next(c) 放行。
  2. SQLE 审核缺少 schema 上下文 → 透传 SchemaName
  3. sid 解析不完整且会误切 :did: → 按 ODC pathUtil.generateDatabaseSid 三态有序解析(:did: 先于 :d:,再回退 base64 JSON);仅给 dbID 时通过 ODC /api/v2/database/databases/{id} 反查库名。

验证

  • go build ./... 通过
  • go vet ./internal/sql_workbench/... 通过

Related

Notes

合并顺序建议:先合 #623main,再合本 PR 进 release-4.2601.x,避免后续从 main 回拣到 4.2601.x 时再次冲突。


Description

  • 修复未启用审核时阻塞 SQL 执行问题

  • 重构 sid 解析逻辑,支持 ":did:" 与 ":d:" 格式

  • 新增 ODC 库名获取(getODCDatabaseName)逻辑

  • 调整 base64 解码方式与错误处理机制


Diagram Walkthrough

flowchart LR
  A["AuditMiddleware"]
  B["parseStreamExecuteRequest"]
  C["parseStreamExecuteSid"]
  D["getODCDatabaseName"]
  E["callSQLEAudit"]
  A -- "解析请求体" --> B
  B -- "解析 sid 字段" --> C
  C -- "返回 sidInfo" --> A
  A -- "schema 为空时查询库名" --> D
  D -- "获取 schemaName" --> E
  A -- "调用审核接口" --> E
Loading

File Walkthrough

Relevant files
Bug fix
sql_workbench_service.go
优化 SQL 审核中间件逻辑及 sid 解析方式                                                                 

internal/sql_workbench/service/sql_workbench_service.go

  • 修改 AuditMiddleware 中解析失败时直接放行请求
  • 将 parseStreamExecuteRequest 修改为返回 SQL 与 sidInfo 结构体
  • 新增 parseStreamExecuteSid 与 fillStreamExecuteSidFromBase64 方法支持多种 sid
    格式
  • 调整 callSQLEAudit 增加 schemaName 参数,并新增 getODCDatabaseName 实现库名反查
+118/-48

…ext from streamExecute requests and improve error handling
@actiontech-bot actiontech-bot requested a review from BugsGuru May 20, 2026 07:14
@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🎫 Ticket compliance analysis 🔶

623 - Partially compliant

Compliant requirements:

  • 放行未启用审核情况下的 SQL 请求
  • 按顺序处理三种 sid 格式
  • 透传 schema 上下文给 SQLE 审核接口

Non-compliant requirements:

[]

Requires further human verification:

[]

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

拼写错误

在解析 sid 的逻辑中,错误提示字符串中将 "sid" 错误写成 "sibd",容易引起混淆,建议修正该拼写问题。

dbID, err := strconv.Atoi(dbPart)
if err != nil {
	return nil, fmt.Errorf("invalid database id in sibd: %v", err)
}

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
传播SID解析错误

建议将对 fillStreamExecuteSidFromBase64 返回错误的处理改为向上返回错误,而不是仅记录 Debug 信息。这可以确保 SID
解析失败时不会产生不完整的会话信息,从而避免审核功能绕过或其他潜在的致命问题。此处同样适用于处理 :d: 分支的代码,应保持错误处理的一致性。

internal/sql_workbench/service/sql_workbench_service.go [1145-1157]

 if didMarker := strings.LastIndex(rest, ":did:"); didMarker != -1 {
     dbPart := rest[didMarker+5:]
     prefix := rest[:didMarker]
     dbID, err := strconv.Atoi(dbPart)
     if err != nil {
-        return nil, fmt.Errorf("invalid database id in sibd: %v", err)
+        return nil, fmt.Errorf("invalid database id in sid: %v", err)
     }
     info.dbID = dbID
     if err := sqlWorkbenchService.fillStreamExecuteSidFromBase64(prefix, info); err != nil {
-        sqlWorkbenchService.log.Debugf("sid prefix is not base64 session JSON, skip: %v", err)
+        sqlWorkbenchService.log.Warnf("failed to decode sid prefix: %v", err)
+        return nil, err
     }
     return info, nil
 }
Suggestion importance[1-10]: 8

__

Why: The suggestion strengthens error handling by propagating errors from fillStreamExecuteSidFromBase64 instead of only logging them, which is crucial for preventing incomplete session information during SID parsing. This change improves reliability and consistency in error management within the critical SID parsing logic.

Medium

@LordofAvernus LordofAvernus merged commit 32f64d5 into release-4.2601.x May 20, 2026
1 check 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.

1 participant