swt/dev updateplatform

[electricface]

• fix: check display-manager.service instead of lightdm (fix: check display-manager.service instead of lightdm #286)
• chore: bump version to 6.2.45 (chore: bump version to 6.2.45 #287)
• feat: Add gatherinfo and posthardware commands to lastore-tools
• feat(updateplatform): Add Helper support for machineID retrieval and policy check interval configuration

[deepin-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface

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

[deepin-ci-robot]

deepin pr auto review

这份代码 diff 主要涉及系统更新平台（UpdatePlatform）的功能增强，包括从外部 helper 获取硬件 ID、任务 ID 管理、更新过程事件上报以及私有化版本的配置调整。以下是从语法逻辑、代码质量、代码性能和代码安全四个方面的审查意见：

1. 语法逻辑

1. GetHardwareId 函数签名变更的影响范围

   • 问题：GetHardwareId 函数签名由 (includeDiskInfo bool) string 变更为 (includeDiskInfo bool, getByHelper bool) string。diff 中修改了所有调用点（如 manager.go, check_policy.go 等），这是一个好的做法，确保了编译通过。
   • 建议：在调用该函数时，如果上下文明确 getByHelper 的值（例如在 UpdateTokenConfigFile 中已经传入了 getHardwareIdByHelper），应确保参数传递的一致性，避免在调用处重复计算或从配置中读取（虽然 diff 中看起来做得不错）。

2. PostStatusMessage 函数签名变更

   • 问题：PostStatusMessage 增加了 forceUpload bool 参数。
   • 逻辑检查：新增逻辑 if system.IsPrivateLastore && !forceUpload { return } 意味着私有版默认不上报状态，除非强制。这符合私有化部署通常不向公网平台上报数据的逻辑。
   • 建议：检查所有调用 PostStatusMessage 的地方，确认 forceUpload 的赋值是否符合业务预期。例如，manager_check.go 中报错时传 false，而 manager_upgrade.go 中某些报错传 true。这种区分需要明确的文档说明，哪些错误是必须上报（即使私有版），哪些不是。

3. ClientPollSetting 结构体字段更新

   • 问题：在 genUpdatePolicyByToken 中，从 msg.ClientPollSetting 读取配置并更新到 m.config。
   • 逻辑检查：

     if !utils.IsElementEqual(m.config.CheckPolicyInterval, msg.ClientPollSetting.CheckPolicyInterval) && msg.ClientPollSetting.CheckPolicyInterval > 0 {
         // ...
     }

     这里使用了 utils.IsElementEqual。对于 int 类型比较，直接使用 != 可能更直观且性能更好，除非 IsElementEqual 处理了特殊的边界情况（如指针或未初始化）。建议确认该工具函数的必要性。

2. 代码质量

1. 硬编码字符串与魔法数字

   • 问题：代码中存在多处硬编码。
     • DBus 相关：helperDBusName, helperDBusPath 等定义为常量是好的。
     • 文件路径：cacheTaskInfo = "/var/lib/lastore/os-task-info"。
     • 字符串截取：body.EventContent[:950]。
   • 建议：
     • 对于 950 这个长度限制，建议定义为一个常量，例如 const MaxEventContentLength = 950，并添加注释说明为何限制在此长度（例如数据库字段限制或协议限制）。

2. 错误处理

   • 问题：在 getHardwareIdByHelper 中：

     err = helperObj.Call(helperDBusInterface+".GetHardware", 0).Store(&hw)
     if err != nil {
         logger.Warning("failed to get hardware info by helper:", err)
         return ""
     }

     如果 DBus 调用失败，函数返回空字符串 ""。这可能导致后续逻辑（如生成 Token）使用空的 MachineID。
   • 建议：考虑当 Helper 获取失败时，是否应该回退到原有的 hhardware.GenMachineID() 方法？或者明确返回 error，由上层决定是重试还是回退，而不是静默返回空字符串。

3. 代码注释与文档

   • 问题：ReplaceVersionCache 增加了注释，但 UpdateBaseline 的修改逻辑（直接写内存数据到文件）比较复杂，仅有一行注释。
   • 建议：UpdateBaseline 中新增了三行写入操作，建议补充注释解释为什么要这样做（正如注释所说："防止其他渠道更新了baseline.b影响了系统的baseline"），以及为什么顺序是先写缓存再复制。

3. 代码性能

1. 文件 I/O 操作

   • 问题：saveTaskId 和 getTaskId 涉及文件读写。每次获取或保存 TaskID 都会触发磁盘 I/O。
   • 建议：getTaskId 目前在 NewUpdatePlatformManager 中调用一次，这是合理的。如果后续有高频调用场景，建议增加内存缓存层。

2. JSON 序列化

   • 问题：PostProcessEventMessage 中对 ProcessEvent 进行了 json.Marshal，然后进行了 EncryptMsg 和 base64 编码。
   • 建议：目前的流程是标准的。如果 EncryptMsg 耗时较长，且调用频率高，可以考虑异步化上报，或者使用对象池复用 bytes.Buffer（虽然 Go 的 json.Marshal 内部已经做了优化）。

4. 代码安全

1. 敏感信息泄露

   • 问题：在 getHardwareIdByHelper 中，日志打印了 logger.Info("hardwareId is:", hw.ID)。
   • 风险：HardwareID 通常用于唯一标识设备，属于敏感信息。直接打印到标准日志可能会泄露。
   • 建议：在生产环境中，应避免打印完整的 HardwareID，或者确保日志文件的权限设置严格（仅 root 可读）。建议改为 Debug 级别，或者只打印前几位/后几位。

2. DBus 调用安全

   • 问题：通过 DBus 调用 com.deepin.sync.Helper 获取硬件信息。
   • 风险：依赖外部 DBus 服务。如果该服务被恶意替换，可能返回伪造的硬件 ID。
   • 建议：虽然这是系统内部服务，但在安全要求较高的场景下，可以考虑验证 DBus 服务的调用者身份或签名（如果 DBus 守护进程配置支持）。

3. TODO 注释

   • 问题：UpdateTokenConfigFile 中有一行 // TODO: may need to use chattr command to set tokenPath as immutable。
   • 风险：Token 文件 /etc/apt/apt.conf.d/99lastore-token.conf 目前权限是 0644（所有者读写，组和其他用户只读）。
   • 建议：
     • 确认 0644 是否符合安全基线要求。通常配置文件不应让其他用户可读，尤其是包含 Token 的文件。建议考虑 0600 或 0640。
     • 关于 chattr +i：这确实可以防止文件被意外修改，但在需要更新 Token 时会带来麻烦（需要先 chattr -i）。如果决定实现，务必做好异常处理，防止死锁或文件锁定。

4. 输入验证

   • 问题：PostProcessEventMessage 中截取字符串 body.EventContent[:950]。
   • 建议：虽然 Go 的切片操作在长度不足时不会 panic，但显式检查 len(body.EventContent) 并进行截断逻辑会更清晰。

总结

这段代码整体结构清晰，功能扩展（如通过 Helper 获取 ID、私有化适配、事件上报）逻辑基本正确。主要改进点在于：

1. 安全性：处理敏感日志打印，检查 Token 文件权限。
2. 健壮性：GetHardwareId 在 Helper 失败时的回退策略。
3. 可维护性：消除魔法数字，补充关键逻辑注释。

代码中对于 IsPrivateLastore 的判断贯穿多处，建议将其封装为一个统一的方法或接口，避免在业务逻辑中散落大量的 if system.IsPrivateLastore 判断，以提高代码的内聚性。