Skip to content

feat: export deepin-elf-verify paths for base and runtime packages#1685

Draft
dengbo11 wants to merge 1 commit into
OpenAtom-Linyaps:masterfrom
dengbo11:fix-base-sign-export
Draft

feat: export deepin-elf-verify paths for base and runtime packages#1685
dengbo11 wants to merge 1 commit into
OpenAtom-Linyaps:masterfrom
dengbo11:fix-base-sign-export

Conversation

@dengbo11

Copy link
Copy Markdown
Collaborator
  1. Add exportReferencePaths method to OSTreeRepo for exporting specific paths only
  2. Add optional exportPathsFilter parameter to exportEntries for filtered exports
  3. Export share/deepin-elf-verify directory from base and runtime packages during updates
  4. Namespace deepin-elf-verify exports by commit hash to avoid conflicts between versions
  5. Update exportAllEntries to also export deepin-elf-verify for base and runtime kinds
  6. Bump LINGLONG_EXPORT_VERSION from 1.0.0.2 to 1.0.0.3
  7. Add comprehensive tests for filtered export functionality and package update scenarios

Influence:

  1. Test updating a base package and verify share/deepin-elf-verify is exported to entries directory
  2. Test updating a runtime package and verify share/deepin-elf-verify is exported
  3. Verify that multiple base/runtime versions export deepin-elf-verify under separate commit-named subdirectories
  4. Test app-only updates and confirm exportReferencePaths is not called
  5. Verify that non-existent filtered paths are skipped gracefully without errors
  6. Test exportAllEntries exports deepin-elf-verify for both app and base/runtime packages
  7. Verify that existing app entries (desktop files, icons, etc.) are still exported correctly alongside the new filtered paths
  8. Test that ELF verification data remains accessible after package upgrades

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the ability to selectively export specific paths (specifically 'share/deepin-elf-verify') for 'base' and 'runtime' package kinds during updates, rather than exporting all entries. It adds the 'exportReferencePaths' method to 'OSTreeRepo' and updates 'exportEntries' to support an optional path filter, along with comprehensive unit tests. Feedback on the changes includes addressing a potential crash by checking if 'modules' is empty before accessing its front element, handling errors from 'getPackageInfo()', and adding the 'override' specifier to the overridden 'exportReferencePaths' method in the test mock.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread libs/linglong/src/linglong/package_manager/package_update.cpp Outdated
Comment thread libs/linglong/tests/ll-tests/src/linglong/package_manager/package_update_test.cpp Outdated
@dengbo11 dengbo11 marked this pull request as draft June 24, 2026 07:21
@dengbo11 dengbo11 force-pushed the fix-base-sign-export branch from 56c501c to 67dd6f5 Compare June 24, 2026 08:14
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 27.27273% with 32 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
libs/linglong/src/linglong/repo/ostree_repo.cpp 28.94% 23 Missing and 4 partials ⚠️
...ng/src/linglong/package_manager/package_update.cpp 16.66% 1 Missing and 4 partials ⚠️
Files with missing lines Coverage Δ
libs/linglong/src/linglong/repo/ostree_repo.h 9.09% <ø> (ø)
...ng/src/linglong/package_manager/package_update.cpp 30.90% <16.66%> (+0.06%) ⬆️
libs/linglong/src/linglong/repo/ostree_repo.cpp 17.51% <28.94%> (+0.86%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dengbo11

Copy link
Copy Markdown
Collaborator Author

pre-commit.ci autofix

@dengbo11 dengbo11 force-pushed the fix-base-sign-export branch 3 times, most recently from af3800c to 32ab683 Compare June 24, 2026 08:58
@dengbo11 dengbo11 marked this pull request as ready for review June 24, 2026 09:09
@dengbo11 dengbo11 requested a review from reddevillg June 24, 2026 09:45
@dengbo11 dengbo11 force-pushed the fix-base-sign-export branch from 32ab683 to c9f8683 Compare June 24, 2026 12:44
1. Add exportReferencePaths method to OSTreeRepo for exporting specific
paths only
2. Add optional exportPathsFilter parameter to exportEntries for
filtered exports
3. Export share/deepin-elf-verify directory from base and runtime
packages during updates
4. Namespace deepin-elf-verify exports by commit hash to avoid conflicts
between versions
5. Update exportAllEntries to also export deepin-elf-verify for base and
runtime kinds
6. Bump LINGLONG_EXPORT_VERSION from 1.0.0.2 to 1.0.0.3
7. Add comprehensive tests for filtered export functionality and package
update scenarios

Influence:
1. Test updating a base package and verify share/deepin-elf-verify is
exported to entries directory
2. Test updating a runtime package and verify share/deepin-elf-verify
is exported
3. Verify that multiple base/runtime versions export deepin-elf-verify
under separate commit-named subdirectories
4. Test app-only updates and confirm exportReferencePaths is not called
5. Verify that non-existent filtered paths are skipped gracefully
without errors
6. Test exportAllEntries exports deepin-elf-verify for both app and
base/runtime packages
7. Verify that existing app entries (desktop files, icons, etc.) are
still exported correctly alongside the new filtered paths
8. Test that ELF verification data remains accessible after package
upgrades
@dengbo11 dengbo11 force-pushed the fix-base-sign-export branch from c9f8683 to bf3af19 Compare June 24, 2026 13:35
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

★ 总体评分:40分

■ 【总体评价】

代码实现了base和runtime包更新时导出ELF签名文件的功能,但存在严重的高危路径遍历安全漏洞
逻辑正确但因路径遍历漏洞及错误处理缺失扣60分

■ 【详细分析】

  • 1.语法逻辑(存在错误)✕

package_update.cpp 第 252 行调用 repo.exportReferencePaths 时,该函数返回值为 void。当内部 getLayerItemexportEntries 失败时,函数仅通过 LogE 打印日志并直接返回,不会向上层传递错误。这导致 updateApp 函数在签名文件导出失败的情况下仍会继续执行并返回成功,使得升级流程状态不一致。
潜在问题:升级状态误报为成功;签名验证文件缺失导致后续运行时校验失败
建议:将 exportReferencePaths 的返回值修改为 utils::error::Result<void>,并在 updateApp 中检查其返回值,失败时直接中断返回错误

  • 2.代码质量(一般)✕

ostree_repo.cpp 第 2080 行对 share/deepin-elf-verify 进行了硬编码的字符串比较与特殊路径拼接处理。这种将业务逻辑与特定目录名强耦合的做法降低了代码的扩展性,若未来增加其他需要按 commit 隔离的目录,必须修改底层导出函数。同时 exportReferencePaths 接口设计为 void 违反了 C++ 处理潜在失败操作的最佳实践。
潜在问题:代码可扩展性差;错误处理机制不符合规范
建议:考虑引入配置或策略模式管理需要命名空间隔离的路径;重构接口返回值类型以反映真实执行状态

  • 3.代码性能(无性能问题)✓

代码中使用的 std::filesystem::path::lexically_normalstd::filesystem::exists 以及目录遍历操作均在可接受的复杂度范围内,未发现不必要的系统调用、死循环或资源泄漏等性能瓶颈。

  • 4.代码安全(存在 1 个安全漏洞)✕

漏洞对比统计:新增漏洞 1 个,减少漏洞 0 个,持平 0 个
总体风险描述,在 exportEntries 函数的路径过滤器逻辑中,仅校验了路径是否为绝对路径,未防范相对路径穿越,攻击面覆盖所有能间接控制 paths 参数的外部调用链

  • 安全漏洞1(高危):[路径遍历] 在 [OSTreeRepo::exportEntries / ostree_repo.cpp] 中,[由于仅使用 is_absolute() 检查过滤路径,未防范 ../ 等相对路径跳转。攻击者若能控制传入 exportReferencePathspaths 参数,可构造如 ../../etc 的字符串。该字符串经 lexically_normal() 后仍为相对路径,绕过绝对路径检查,随后通过 appEntriesDir / normalizedPath 拼接导致目录穿越,越权读取或覆盖系统任意目录文件] ——非常重要

  • 建议:在路径规范化后,增加对路径分量的遍历检查,拒绝包含 .. 的路径请求

■ 【改进建议代码示例】

// 文件: libs/linglong/src/linglong/repo/ostree_repo.cpp (第 2068-2099 行附近)
    // 如果指定了导出路径过滤器,仅导出指定的路径
    if (exportPathsFilter.has_value()) {
        for (const auto &path : *exportPathsFilter) {
            auto normalizedPath = std::filesystem::path(path).lexically_normal();
            if (normalizedPath.is_absolute()) {
                return LINGLONG_ERR(fmt::format(
                  "Failed to export {}: absolute path is not allowed in export filter: {}",
                  path));
            }

            // 防范路径遍历攻击:确保路径不包含 ".." 分量
            for (const auto &component : normalizedPath) {
                if (component == "..") {
                    return LINGLONG_ERR(fmt::format(
                      "Failed to export {}: path traversal (..) is not allowed in export filter: {}",
                      path));
                }
            }

            auto source = appEntriesDir / normalizedPath;
            auto destination = rootEntriesDir / normalizedPath;

            if (normalizedPath == "share/deepin-elf-verify") {
                destination = rootEntriesDir / "share/deepin-elf-verify" / item.commit;
            }

            exists = std::filesystem::exists(source, ec);
            if (ec) {
                return LINGLONG_ERR(fmt::format("Failed to check file existence: {}", source), ec);
            }
            if (!exists) {
                continue;
            }
            auto ret = this->exportDir(item.info.id, source, destination, 10);
            if (!ret.has_value()) {
                return ret;
            }
        }
        return LINGLONG_OK;
    }

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

@dengbo11: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
github-pr-review-ci bf3af19 link true /test github-pr-review-ci

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@dengbo11 dengbo11 marked this pull request as draft June 24, 2026 14:38
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.

2 participants