Skip to content

feat: add Linglong uninstaller script support#54

Merged
wjyrich merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-291703
Aug 26, 2025
Merged

feat: add Linglong uninstaller script support#54
wjyrich merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-291703

Conversation

@wjyrich

@wjyrich wjyrich commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

Added a new script dde-appwiz-linglong-uninstaller.sh for handling Linglong bundle uninstallation with proper error checking and validation Modified uninstallLinglongBundle function to use pkexec with the new script instead of direct ll-cli execution
Updated Polkit policy to include the new script path for proper privilege escalation
Added installation of the new script in CMakeLists.txt This change improves security by using Polkit for privilege escalation and provides better error handling for Linglong uninstallation

feat: 添加玲珑卸载脚本支持

新增 dde-appwiz-linglong-uninstaller.sh 脚本用于处理玲珑包卸载,包含完善 的错误检查和验证
修改 uninstallLinglongBundle 函数使用 pkexec 调用新脚本,替代直接执行 ll-cli
更新 Polkit 策略以包含新脚本路径,实现正确的权限提升
在 CMakeLists.txt 中添加新脚本的安装配置
此改进通过使用 Polkit 进行权限提升提高了安全性,并为玲珑卸载提供了更好的
错误处理

Pms: BUG-291703

Summary by Sourcery

Introduce a dedicated Linglong bundle uninstaller script invoked through Polkit for secure privilege escalation and improved error handling

New Features:

  • Add dde-appwiz-linglong-uninstaller.sh script for handling Linglong bundle uninstallation with validation and error checking

Enhancements:

  • Modify uninstallLinglongBundle to invoke the uninstaller via pkexec instead of direct ll-cli calls
  • Update Polkit policy to allow privilege escalation for the new Linglong uninstaller script

Build:

  • Install the new Linglong uninstaller script via CMakeLists.txt

@sourcery-ai

sourcery-ai Bot commented Aug 26, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR secures Linglong bundle uninstallation by introducing a dedicated Polkit-enabled script and adapting the application code and build configuration to invoke it via pkexec, enhancing error handling and permission control.

Sequence diagram for Linglong bundle uninstallation via Polkit script

sequenceDiagram
    actor User
    participant App as Application
    participant PKExec as pkexec
    participant Script as dde-appwiz-linglong-uninstaller.sh
    participant LLCLI as ll-cli

    User->>App: Request to uninstall Linglong bundle
    App->>PKExec: Start dde-appwiz-linglong-uninstaller.sh with appId
    PKExec->>Script: Run script with elevated privileges
    Script->>LLCLI: Check if ll-cli is available
    Script->>LLCLI: Check if bundle is installed
    Script->>LLCLI: Uninstall bundle
    Script-->>PKExec: Return success or error
    PKExec-->>App: Return exit code
    App-->>User: Notify result
Loading

Class diagram for updated uninstallLinglongBundle logic

classDiagram
    class Application {
        +uninstallLinglongBundle(entry: DDesktopEntry): bool
    }
    class DDesktopEntry {
        +rawValue(key: QString): QString
    }
    Application --> DDesktopEntry: uses

    class dde-appwiz-linglong-uninstaller.sh {
        +main(appId: string)
        +error checking
        +calls ll-cli
    }
    Application --> dde-appwiz-linglong-uninstaller.sh: invokes via pkexec
Loading

File-Level Changes

Change Details Files
Invoke Linglong uninstallation via pkexec script
  • Replaced direct ll-cli execution with QProcess start of pkexec and script path
  • Added debug logging and waitForFinished call
  • Checked process exit code instead of return code
dbus/launcher1compat.cpp
Add Linglong uninstaller script with robust error handling
  • Validate presence of app ID argument
  • Check for ll-cli availability
  • Verify bundle installation before uninstall
  • Perform uninstall with success and error messaging
scripts/dde-appwiz-linglong-uninstaller.sh
Update CMake installation to include new script
  • Added install rule for the new uninstaller script
CMakeLists.txt
Extend Polkit policy for new script privilege escalation
  • Included new script path in Polkit action definition
polkit-1/actions/org.deepin.dde.appwiz.uninstall.policy

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • Add a timeout to QProcess::waitForFinished in uninstallLinglongBundle to prevent potential hangs if the uninstaller script hangs or never exits.
  • Use a more precise check for the installed bundle in the shell script (for example exact matching or structured output parsing) instead of a generic grep to avoid false positives.
  • Ensure the Polkit policy file is updated to explicitly authorize the new linglong uninstaller script path so that pkexec can properly escalate privileges.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a timeout to QProcess::waitForFinished in uninstallLinglongBundle to prevent potential hangs if the uninstaller script hangs or never exits.
- Use a more precise check for the installed bundle in the shell script (for example exact matching or structured output parsing) instead of a generic grep to avoid false positives.
- Ensure the Polkit policy file is updated to explicitly authorize the new linglong uninstaller script path so that pkexec can properly escalate privileges.

## Individual Comments

### Comment 1
<location> `scripts/dde-appwiz-linglong-uninstaller.sh:22` </location>
<code_context>
+fi
+
+# Check if the Linglong bundle is installed
+if ! ll-cli list | grep -q "$LINGLONG_APP_ID"; then
+    echo "Error: Linglong bundle '$LINGLONG_APP_ID' is not installed."
+    exit 1
</code_context>

<issue_to_address>
Using grep for bundle existence may yield false positives if app IDs are substrings of others.

Use grep -w or another method to ensure only exact app ID matches are detected.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
# Check if the Linglong bundle is installed
if ! ll-cli list | grep -q "$LINGLONG_APP_ID"; then
    echo "Error: Linglong bundle '$LINGLONG_APP_ID' is not installed."
    exit 1
=======
# Check if the Linglong bundle is installed
if ! ll-cli list | grep -w -q "$LINGLONG_APP_ID"; then
    echo "Error: Linglong bundle '$LINGLONG_APP_ID' is not installed."
    exit 1
>>>>>>> REPLACE

</suggested_fix>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread scripts/dde-appwiz-linglong-uninstaller.sh
Added a new script dde-appwiz-linglong-uninstaller.sh for handling
Linglong bundle uninstallation with proper error checking and validation
Modified uninstallLinglongBundle function to use pkexec with the new
script instead of direct ll-cli execution
Updated Polkit policy to include the new script path for proper
privilege escalation
Added installation of the new script in CMakeLists.txt
This change improves security by using Polkit for privilege escalation
and provides better error handling for Linglong uninstallation

feat: 添加玲珑卸载脚本支持

新增 dde-appwiz-linglong-uninstaller.sh 脚本用于处理玲珑包卸载,包含完善
的错误检查和验证
修改 uninstallLinglongBundle 函数使用 pkexec 调用新脚本,替代直接执行
ll-cli
更新 Polkit 策略以包含新脚本路径,实现正确的权限提升
在 CMakeLists.txt 中添加新脚本的安装配置
此改进通过使用 Polkit 进行权限提升提高了安全性,并为玲珑卸载提供了更好的
错误处理

Pms: BUG-291703
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

我来对这段代码进行审查,从多个方面提出改进意见:

1. 语法逻辑

  • launcher1compat.cpp 中,uninstallLinglongBundle 函数的改进是好的,使用了更安全的 QProcess 方式而不是直接调用 QProcess::execute
  • 但是,建议添加错误处理逻辑,比如检查 process.start() 是否成功启动:
if (!process.waitForStarted()) {
    qDebug() << "Failed to start process";
    return false;
}

2. 代码质量

  • scripts/dde-appwiz-linglong-uninstaller.sh 中的错误处理很好,但可以考虑使用更严格的错误检查:
set -euo pipefail

这会在任何命令失败时立即退出,并且未设置的变量会报错。

  • 建议在脚本中添加日志记录功能,记录卸载操作的详细信息,便于追踪问题。

3. 代码性能

  • launcher1compat.cpp 中,process.waitForFinished() 会阻塞当前线程,如果卸载过程较久,可能会影响用户体验。建议考虑使用异步处理方式。

  • 在脚本中,ll-cli list | grep -q 的方式检查应用是否安装,可能需要遍历所有应用,效率较低。可以考虑使用 ll-cli info <app_id> 直接检查特定应用。

4. 代码安全

  • launcher1compat.cpp 中,直接使用 pkexec 提升权限是合适的,但建议添加参数验证,确保 appId 是合法的:
if (appId.isEmpty() || appId.contains(QRegularExpression("[^a-zA-Z0-9_-]"))) {
    qDebug() << "Invalid app ID:" << appId;
    return false;
}
  • 在脚本中,虽然检查了参数是否为空,但建议进一步验证参数格式是否符合 Linglong 应用 ID 的规范。

  • 考虑在脚本中添加更多的安全检查,比如:

    • 检查当前用户是否有权限执行卸载操作
    • 添加操作确认步骤
    • 实现回滚机制,以防卸载失败

5. 其他建议

  • 建议在 CMakeLists.txt 中为新增的脚本文件添加权限设置:
install(FILES scripts/dde-appwiz-linglong-uninstaller.sh 
        DESTINATION ${CMAKE_INSTALL_LIBEXECDIR}
        PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE)
  • 建议在代码中添加更多的注释,解释关键步骤和决策原因。

  • 考虑添加单元测试,特别是对于 uninstallLinglongBundle 函数,确保各种边界条件都能正确处理。

总体而言,这次的改进是朝着正确的方向,但还有进一步优化的空间,特别是在错误处理、性能优化和安全性方面。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, wjyrich

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

@wjyrich
wjyrich merged commit c308025 into linuxdeepin:master Aug 26, 2025
9 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.

3 participants