Skip to content

Conversation

@asterwyx
Copy link
Contributor

No description provided.

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: asterwyx

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

Details Needs approval from an approver in each of these files:

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

Synchronizing is no more needed.
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这段 git diff 显示的是一个删除操作,即删除了名为 call-synchronize-to-dtk6.yml 的 GitHub Actions 工作流文件。由于文件已被删除,我将基于该文件原本的内容进行审查,分析其逻辑、安全性及潜在风险。

1. 代码审查总结

文件功能
该工作流定义了一个可复用的工作流调用。当针对仓库发起 Pull Request (pull_request_target) 时,它会触发并调用外部仓库 linuxdeepin/dtk 中的另一个工作流 synchronize-to-dtk6.yml。目的是将 PR 中的更改同步到另一个名为 linuxdeepin/qt6integration 的仓库。

主要风险
该配置存在严重的安全隐患,这很可能是该文件被删除的主要原因。

2. 详细审查意见

2.1 安全性 - 严重风险

  • 使用了 pull_request_target 触发器

    • 问题pull_request_target 会在 PR 的基础分支(通常是 mastermain)上下文中运行,而不是 PR 的贡献者分支上下文。这意味着它拥有仓库的写入权限(如果仓库启用了写入权限)。
    • 风险:虽然这个文件本身只是调用了另一个工作流,但结合 secrets: inherit,它会将仓库的 Secrets 传递给被调用的工作流。
    • 代码注入风险:被调用的外部工作流 synchronize-to-dtk6.yml 位于 linuxdeepin/dtk 仓库的 master 分支。如果外部工作流的逻辑被修改(例如通过恶意 PR 合并到该仓库),或者该工作流 checkout 了本仓库的代码并执行了其中的脚本,攻击者可以通过提交恶意代码到本仓库的 PR 中,利用这个拥有高权限的工作流执行恶意操作(如窃取 Secrets、篡改代码)。
  • 使用了 secrets: inherit

    • 问题:这会将当前仓库的所有 Secrets 传递给 linuxdeepin/dtk 仓库中的工作流。
    • 风险:这扩大了攻击面。如果 linuxdeepin/dtk 仓库中的工作流存在漏洞或被劫持,当前仓库的敏感信息(如 Token、密钥)将面临泄露风险。
  • 使用了 uses: ...@master

    • 问题:直接引用 master 分支是不稳定的做法。
    • 建议:应该引用具体的 Commit SHA 或 Tag(例如 @v1.0.0@a1b2c3d),以确保调用的代码是固定且经过审计的版本,防止上游 master 分支的变动引入意外行为或恶意代码。

2.2 逻辑与语法

  • 路径过滤 (paths-ignore)

    • 逻辑:配置了忽略特定路径(如 debian, archlinux, rpm, .obs, .github)的变更。
    • 评价:逻辑清晰,意图是仅当核心代码变更时才触发同步,忽略打包相关的文件变更,这是合理的。
  • 参数传递 (with)

    • 逻辑:将当前 PR 的源仓库、源分支和 PR 编号传递给下游工作流。
    • 评价:参数传递逻辑正确,符合同步操作的需求。

2.3 代码质量与维护性

  • 硬编码仓库
    • dest_repo: linuxdeepin/qt6integration 被硬编码在文件中。虽然对于特定用途可能没问题,但如果需要同步到多个仓库,灵活性较差。

3. 改进建议

如果必须保留此类同步功能,建议进行以下修改以增强安全性:

  1. 限制权限
    在工作流级别添加 permissions 块,仅授予必要的最小权限(例如 contents: read),除非绝对必要,否则不要授予 write 权限。

  2. 固定引用版本
    uses: linuxdeepin/dtk/.github/workflows/synchronize-to-dtk6.yml@master 修改为引用具体的 Tag 或 Commit SHA。

  3. 审查 Secrets 传递
    除非被调用的外部工作流严格必需,否则移除 secrets: inherit。如果需要,仅传递特定的 Secret(使用 secrets: MY_SECRET: ${{ secrets.MY_SECRET }})。

  4. 代码执行隔离
    确保被调用的外部工作流在执行任何脚本前,显式 checkout 目标仓库(dest_repo)的受信代码,而不是直接运行 PR 中可能包含的恶意脚本。

4. 结论

删除该文件是正确的决定
该文件通过 pull_request_target 结合 secrets: inherit 调用外部可变工作流,构成了严重的安全风险(类似于 pwn request 攻击向量)。如果不进行严格的安全加固(如固定版本、最小权限、不传递 Secrets),不应恢复此类工作流。如果需要代码同步功能,建议使用更安全的方式,例如在 PR 合并后通过 pull_request 事件触发,或者使用经过严格审计的、具有严格输入检查的 Bot 账户进行操作。

@zccrs
Copy link
Member

zccrs commented Jan 20, 2026

同步到gitee的这个失败了不影响这个提交,这个改动是github action的变化,本次未同步过去不要紧,直接合入了。

@zccrs zccrs merged commit 8e83934 into master Jan 20, 2026
12 of 16 checks 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.

4 participants