feat: Add iup-tool for testing the internal update platform

[electricface]

Accelerate issue debugging

[deepin-ci-robot]

deepin pr auto review

Git Diff 代码审查报告

我已对提供的 git diff 进行了详细审查，以下是关于语法逻辑、代码质量、性能和安全方面的分析和改进建议。

1. Makefile 修改

审查意见

• 语法正确，添加了新的 dev-tools 目标和 .PHONY 声明
• 建议将 bin/iup-tool 也标记为 .PHONY，虽然它是一个文件目标，但为了避免与同名文件冲突

改进建议

.PHONY: dev-tools bin/iup-tool

2. cmd_get_current_packages.go

审查意见

• 代码逻辑基本正确，使用 cobra 实现命令行接口
• HTTP 请求超时设置为 40 秒，较为合理

改进建议

1. 安全性：Token 使用 base64 编码传输，建议考虑更安全的传输方式
2. 性能：每次请求都创建新的 HTTP 客户端，可以复用客户端
3. 错误处理：错误信息可以更详细，便于调试

// 建议改进的代码
var httpClient = &http.Client{
    Timeout: 40 * time.Second,
}

func genCurrentPkgListsResponse(requestUrl, token, baseline string) (*http.Response, error) {
    policyUrl := requestUrl + Urls[GetCurrentPkgLists].path
    values := url.Values{}
    values.Add("baseline", baseline)
    policyUrl = policyUrl + "?" + values.Encode()
    
    request, err := http.NewRequest(Urls[GetCurrentPkgLists].method, policyUrl, nil)
    if err != nil {
        return nil, fmt.Errorf("%v new request failed: %v", GetCurrentPkgLists.string(), err)
    }
    
    request.Header.Set("X-Repo-Token", base64.RawStdEncoding.EncodeToString([]byte(token)))
    logRequestHeaders(request)
    
    return httpClient.Do(request)
}

3. cmd_get_cve_info.go

审查意见

• 代码结构与 cmd_get_current_packages.go 类似
• CVE 信息显示逻辑合理，限制显示数量避免输出过多

改进建议

1. 性能：同上，复用 HTTP 客户端
2. 代码复用：与 cmd_get_current_packages.go 的 HTTP 请求逻辑重复，可以提取公共函数

// 建议提取的公共函数
func makeRequest(requestUrl, token, method string, queryParams url.Values) (*http.Response, error) {
    policyUrl := requestUrl + queryParams.Encode()
    request, err := http.NewRequest(method, policyUrl, nil)
    if err != nil {
        return nil, fmt.Errorf("new request failed: %v", err)
    }
    
    request.Header.Set("X-Repo-Token", base64.RawStdEncoding.EncodeToString([]byte(token)))
    logRequestHeaders(request)
    
    return httpClient.Do(request)
}

4. cmd_get_update_log.go

审查意见

• 代码逻辑正确
• 使用了 spew 库进行调试输出

改进建议

1. 性能：复用 HTTP 客户端
2. 代码质量：调试输出可以条件化，避免生产环境输出过多信息

// 建议改进的代码
if globalDebug {
    logger.Debugf("Update Log Data: %v", spew.Sdump(logs))
}

5. cmd_get_version.go

审查意见

• 代码结构清晰
• 版本信息获取逻辑正确

改进建议

1. 性能：复用 HTTP 客户端
2. 代码质量：genUpdatePolicyByToken 方法获取响应后未关闭响应体，可能导致资源泄漏

// 建议改进的代码
func (m *UpdatePlatformManager) genUpdatePolicyByToken() error {
    response, err := m.genVersionResponse()
    if err != nil {
        return fmt.Errorf("failed get version data %v", err)
    }
    defer response.Body.Close() // 添加这一行
    
    logger.Debugf("response: %v", response)
    data, err := getResponseData(response, GetVersion)
    if err != nil {
        return fmt.Errorf("failed get version data %v", err)
    }
    msg := getVersionData(data)
    logger.Infof("msg: %s", spew.Sdump(msg))
    return nil
}

6. cmd_post_process.go

审查意见

• 代码逻辑复杂，实现了状态消息和日志文件上传功能
• 使用 xz 压缩文件，签名机制合理

改进建议

1. 安全性：secret 常量硬编码在代码中，建议从安全配置中获取
2. 性能：复用 HTTP 客户端
3. 代码质量：临时文件路径硬编码为 /tmp，应使用系统临时目录

// 建议改进的代码
// 从安全配置中获取 secret
func getSecret() string {
    // 实现从安全配置中获取 secret 的逻辑
    // 可以考虑从环境变量或安全存储中获取
    secret := os.Getenv("IUP_TOOL_SECRET")
    if secret == "" {
        logger.Warning("IUP_TOOL_SECRET not set, using default")
        return "DflXyFwTmaoGmbDkVj8uD62XGb01pkJn"
    }
    return secret
}

// 使用系统临时目录
func runPostProcess(cmd *cobra.Command, args []string) {
    // ...
    filePath := filepath.Join(os.TempDir(), fmt.Sprintf("%s_%s.xz", "update", time.Now().Format("20060102150405")))
    // ...
}

7. cmd_post_process_event.go

审查意见

• 代码逻辑正确
• 事件类型验证合理

改进建议

1. 安全性：事件内容长度限制为 950 字符，建议在文档中说明
2. 性能：复用 HTTP 客户端
3. 代码质量：事件类型验证逻辑可以提取为单独函数

// 建议提取的函数
func validateEventType(eventType ProcessEventType) error {
    if !eventType.IsValid() {
        var validTypes []string
        for i := CheckEnv; i < MaxProcessEventType; i++ {
            validTypes = append(validTypes, fmt.Sprintf("%d=%s", i, i.String()))
        }
        return fmt.Errorf("invalid event type: %d, must be between %d and %d. Valid types: %s", 
            eventType, CheckEnv, MaxProcessEventType-1, strings.Join(validTypes, ", "))
    }
    return nil
}

// 使用提取的函数
func runPostProcessEvent(cmd *cobra.Command, args []string) {
    // ...
    if err := validateEventType(eventType); err != nil {
        logger.Warningf("%v", err)
        os.Exit(1)
    }
    // ...
}

8. cmd_post_result.go

审查意见

• 代码逻辑正确
• 升级结果上报功能完整

改进建议

1. 性能：复用 HTTP 客户端
2. 代码质量：多个字段标记为 TODO，需要实现
3. 安全性：使用 AES-CBC 加密，但 IV 生成方式可能不够安全

// 建议改进的加密部分
func EncryptMsg(data []byte) ([]byte, error) {
    // 获取 16 字节随机字符串并添加到明文前
    replyMsgBytes, err := GetRandomBytes(randomLen)
    if err != nil {
        return nil, err
    }
    replyMsgBytes = append(replyMsgBytes, data...)
    
    // 生成随机 IV
    iv, err := GetRandomBytes(16)
    if err != nil {
        return nil, err
    }
    
    // 生成密文
    block, err := aes.NewCipher([]byte(encodingAesKey))
    if err != nil {
        logger.Warningf("[encrypt] aes-cbc encrypt data failed, error:%v", err)
        return nil, err
    }
    
    encodeBytes := replyMsgBytes
    encodeBytes = PKCS7Encode(encodeBytes, BlockSize)
    blockMode := cipher.NewCBCEncrypter(block, iv)
    crypted := make([]byte, len(encodeBytes))
    if len(crypted)%block.BlockSize() != 0 {
        return nil, errors.New("encrypt failed, input not full blocks")
    }
    blockMode.CryptBlocks(crypted, encodeBytes)
    
    // 将 IV 添加到密文前，以便解密时使用
    result := append(iv, crypted...)
    return result, nil
}

9. iup-tool.go

审查意见

• 主程序结构清晰
• 使用 cobra 实现命令行框架

改进建议

1. 代码质量：日志初始化可以更灵活，支持日志文件输出
2. 错误处理：主程序错误处理可以更详细

// 建议改进的日志初始化
var rootCmd = &cobra.Command{
    Use:   "iup-tool",
    Short: "tool for intranet update platform operations",
    Long:  "A tool for interacting with the IUP (Intranet Update Platform)",
    PersistentPreRun: func(cmd *cobra.Command, args []string) {
        // 设置日志级别和输出
        if globalDebug {
            logger.SetLogLevel(log.LevelDebug)
        } else {
            logger.SetLogLevel(log.LevelInfo)
        }
        
        // 如果需要，可以添加日志文件输出
        if logFile := os.Getenv("IUP_TOOL_LOG_FILE"); logFile != "" {
            file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
            if err == nil {
                logger.SetOutput(file)
                defer file.Close()
            }
        }
        
        logger.Debug("Starting iup-tool")
        
        // 初始化更新平台管理器
        initUpdatePlatform()
    },
}

10. types.go

审查意见

• 类型定义完整
• 枚举类型和字符串转换方法合理

改进建议

1. 代码质量：可以添加更多的验证方法
2. 文档：可以添加更详细的类型注释

// 建议添加的验证方法
func (r UpgradeResult) IsValid() bool {
    switch r {
    case UpgradeSucceed, UpgradeFailed, CheckFailed:
        return true
    default:
        return false
    }
}

// 在 runPostResult 中使用
func runPostResult(cmd *cobra.Command, args []string) {
    var result *UpgradePostMsg
    
    // 从文件读取或从命令行参数构建
    if resultDataFile != "" {
        // ...
    } else {
        // ...
        result = &UpgradePostMsg{
            // ...
            UpgradeStatus: UpgradeResult(resultStatus),
            // ...
        }
    }
    
    // 验证升级结果状态
    if !result.UpgradeStatus.IsValid() {
        logger.Warningf("invalid upgrade status: %d", resultStatus)
        os.Exit(1)
    }
    
    logger.Debugf("Result: %s", spew.Sdump(result))
    // ...
}

11. utils.go

审查意见

• 工具函数实现完整
• 加密解密功能实现正确

改进建议

1. 安全性：Token 和 machine ID 提取逻辑可以更健壮
2. 性能：HTTP 客户端可以复用
3. 代码质量：字符串处理函数可以优化

// 建议改进的 Token 提取逻辑
func extractMachineIDFromToken(token string) string {
    if token == "" {
        return ""
    }
    
    // Token 格式: a=value;b=value;i=machineID;...
    fields := strings.Split(token, ";")
    for _, field := range fields {
        field = strings.TrimSpace(field)
        parts := strings.SplitN(field, "=", 2)
        if len(parts) == 2 && parts[0] == "i" {
            machineID := strings.TrimSpace(parts[1])
            if machineID != "" {
                logger.Debugf("Machine ID extracted from token: %s", machineID)
                return machineID
            }
        }
    }
    
    logger.Warning("Machine ID (i=) not found in token")
    return ""
}

// 建议改进的字符串处理函数
func Substr(str string, start int, length int) string {
    if start < 0 {
        start = 0
    }
    if length < 0 {
        length = 0
    }
    
    runeStr := []rune(str)
    runeLen := len(runeStr)
    
    if start >= runeLen {
        return ""
    }
    
    end := start + length
    if end > runeLen {
        end = runeLen
    }
    
    return string(runeStr[start:end])
}

总体建议

1. HTTP 客户端复用：所有 HTTP 请求都应复用同一个客户端，以提高性能
2. 错误处理：统一错误处理模式，提供更详细的错误信息
3. 代码复用：提取公共函数，减少代码重复
4. 安全性：
   • Token 和 secret 应从安全配置中获取，而非硬编码
   • 考虑使用更安全的加密方式
   • 临时文件应使用系统临时目录
5. 性能：
   • 复用 HTTP 客户端
   • 考虑添加请求缓存机制
6. 代码质量：
   • 添加更多验证函数
   • 完善文档注释
   • 统一代码风格

安全性特别关注

1. Token 传输：当前使用 base64 编码传输 Token，建议考虑更安全的传输方式
2. Secret 管理：Secret 硬编码在代码中，存在安全风险，应从安全配置中获取
3. 加密方式：当前使用 AES-CBC 加密，IV 生成方式可以改进
4. 临时文件：临时文件路径硬编码为 /tmp，应使用系统临时目录

这些改进建议旨在提高代码的安全性、性能和可维护性，同时保持代码的功能完整性。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface, zhaohuiw42

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment