Skip to content

fix: correct authorized table column name from enable to exist - #943

Closed
add-uos wants to merge 5 commits into
linuxdeepin:masterfrom
add-uos:agent/pms-bug-bot/1ef4550a2a0d
Closed

add-uos wants to merge 5 commits into
linuxdeepin:masterfrom
add-uos:agent/pms-bug-bot/1ef4550a2a0d

Conversation

@add-uos

@add-uos add-uos commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Root Cause Analysis

The authorized table's CREATE statement (line 402) only defines an exist boolean column — there is no enable column. However, clearEnableFromAuthorizedTable (line 101) and isUniqueIdEnabled (line 156) both reference a non-existent enable column in their SQL statements, causing SQLite to error with no such column: enable and silently fail. Key evidence: insertDataToAuthorizedTable writes to the exist column; controlinterface.cpp:629 passes an enable variable directly into the exist parameter position, confirming semantic confusion between the two names.

Fix

Replace enable with exist in both SQL statements. Change .arg(true) to .arg(1) in the DELETE statement to match SQLite's integer boolean storage format. Three pinning test cases updated to verify the fixed behavior.

Change Safety Assessment

Risk Level: LOW

  • Code Safety: No SQL injection risk (column name replacement only, no new user input). No data migration needed (exist column already exists and is populated by insertDataToAuthorizedTable). Behavioral change in isUniqueIdEnabled (from always-false to actual value) is the intended fix.
  • Business Impact: ControlInterface::isDeviceEnabled DBus interface will now correctly report device enabled status instead of always returning false. clearEnableFromAuthorizedTable will now correctly clear authorized records.
  • Verification: Test clearEnableFromAuthorizedTable with exist=true and exist=false rows to verify only exist=true rows are deleted. Test isUniqueIdEnabled with exist=true and exist=false records to verify correct boolean return values.

根因分析

authorized 表建表语句(line 402)仅定义了 exist boolean 列,不存在 enable 列。但 clearEnableFromAuthorizedTable(line 101)和 isUniqueIdEnabled(line 156)的 SQL 语句引用了不存在的 enable 列,导致 SQLite 报错 no such column: enable 并静默失败。关键证据:insertDataToAuthorizedTable 写入 exist 列;controlinterface.cpp:629 将 enable 变量直接传入 exist 参数位置,证实语义混淆。

修复方案

将两处 SQL 语句中的 enable 替换为 exist,并将 DELETE 语句中的 .arg(true) 改为 .arg(1) 以匹配 SQLite 布尔值的整数存储格式。3 个 pinning 测试用例同步更新。

改动安全评估

风险等级:LOW

  • 代码安全评估:无 SQL 注入风险(仅列名替换,无新用户输入拼接)。无需数据迁移(exist 列已存在且由 insertDataToAuthorizedTable 写入)。isUniqueIdEnabled 行为变更(从恒返回 false 到返回实际值)为预期修复行为。
  • 业务影响范围:ControlInterface::isDeviceEnabled DBus 接口修复后将正确报告设备启用状态。clearEnableFromAuthorizedTable 修复后可正确清理授权记录。
  • 验证建议:测试 clearEnableFromAuthorizedTable 在 exist=true 和 exist=false 行共存时仅删除 exist=true 行;测试 isUniqueIdEnabled 返回正确布尔值。

PMS: BUG-UT-DEF-001

Summary by Sourcery

Fix authorized-device state queries and expand isolated unit-test coverage for device and wake-control functionality.

Bug Fixes:

  • Correct authorized-table queries to use the existing exist column, restoring authorized-record cleanup and device-enabled status reporting.

Enhancements:

  • Add isolated SQLite database helpers and extensive unit coverage for SQL manager, device enablement, and wake-on-LAN behavior.

Tests:

  • Add comprehensive unit tests covering authorized-record state handling, device enablement paths, network control fallbacks, wake-on-LAN configuration, and error cases.

@sourcery-ai

sourcery-ai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Sorry @add-uos, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 11 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos

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

@sourcery-ai

sourcery-ai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Reviewer's Guide

Fixes authorized-table queries to match the existing exist schema column, restoring device-enabled status reporting and authorized-record cleanup, while adding temporary-database coverage for the regression and broad stub-based tests for related enable and wakeup control paths.

Sequence diagram for corrected device-enabled status lookup

sequenceDiagram
    participant ControlInterface
    participant EnableSqlManager
    participant SQLite

    ControlInterface->>EnableSqlManager: isUniqueIdEnabled(key)
    EnableSqlManager->>SQLite: SELECT exist FROM authorized WHERE unique_id=:key
    SQLite-->>EnableSqlManager: exist value
    EnableSqlManager-->>ControlInterface: enabled status
Loading

Flow diagram for authorized-record cleanup

flowchart LR
    A[clearEnableFromAuthorizedTable] --> B[DELETE FROM authorized WHERE exist=1]
    B --> C[SQLite removes enabled authorization records]
    C --> D[Records with exist=0 remain]
Loading

File-Level Changes

Change Details Files
Correct authorized-table SQL to use the schema’s exist column and store the boolean predicate in SQLite-compatible integer form.
  • Delete only authorized rows where exist=1 instead of querying the nonexistent enable column.
  • Select exist when determining whether a unique ID is enabled.
deepin-devicemanager-server/deepin-devicecontrol/src/enablecontrol/enablesqlmanager.cpp
Add isolated SQLite unit coverage for EnableSqlManager, including the corrected authorized-record behavior and SQL error paths.
  • Inject a temporary SQLite database with the production table schema and reset it between tests.
  • Verify true and false exist rows are handled correctly by clearing and status queries.
  • Cover CRUD behavior, singleton/database initialization, missing-table handling, and related wake/network/monitor operations.
deepin-devicemanager-server/tests/src/EnableControl/ut_enablesqlmanager.cpp
deepin-devicemanager-server/tests/src/ut_testdbhelper.h
Expand EnableUtils tests with stubbed DBus, ioctl, sysfs, file, and SQLite dependencies.
  • Exercise network DBus/ioctl fallback and failure behavior.
  • Cover device enable/disable flows, identifier variants, path safety, and file-write behavior.
  • Add a pinning test documenting early return when a USB authorized-file open fails.
deepin-devicemanager-server/tests/src/EnableControl/ut_enableutils.cpp
Expand WakeupUtils tests across WoL, TLP configuration, device discovery, and wakeup-file update paths.
  • Stub socket/ioctl and filesystem operations to test WoL state transitions and configuration writes/removal.
  • Cover unique-ID, MD5, PS/2, device-link, malformed-input, and error branches.
  • Verify wakeup state updates against temporary SQLite records.
deepin-devicemanager-server/tests/src/WakeControl/ut_wakeuputils.cpp

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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 98 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次提交修复了 authorized 表列名错误(enable→exist),核心修改仅 2 行,语法正确、逻辑清晰。同时新增大量单元测试覆盖 EnableSqlManager、EnableUtils、WakeupUtils 三个模块。测试代码整体质量良好但存在少量改进空间(全局变量未重置、冗余宏定义等),不构成安全漏洞。生产代码无安全风险。

🔍 详细分析

1. 语法逻辑 ✅

评价: 语法正确,逻辑清晰 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['无语法逻辑问题']


2. 代码质量 ✅

评价: 代码结构清晰,注释完整 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['在 disableOutDevice_moduleAliasOnly_stripsTrailingTag 和 disableOutDevice_noIdentifiableId_skipsItem 的 Arrange 阶段添加 ut_qfileOpen_calls = 0', '移除 ut_wakeuputils.cpp 中重复的 #define private public / #define protected public 宏定义', '在 noSuchTable 测试的 removePathList 调用后添加 EXPECT_EQ(paths.size(), 0) 断言', '考虑在 TearDown() 中统一重置全局桩变量,避免测试间状态污染']


3. 代码性能 ✅

评价: 性能良好,资源使用合理 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['无性能问题']


4. 代码安全 🔒

评价: 存在0个安全漏洞 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: ['ut_testdbhelper.h 中 tableHas 函数可考虑对 table 参数添加白名单校验,增强防御性编程', 'ut_qtSym 函数建议添加 dlopen/dlsym 返回值 nullptr 检查,避免测试中未定义行为']


💡 改进建议代码示例

// 建议修复:在 ut_enableutils.cpp 两个测试的 Arrange 阶段重置全局变量
// 1. disableOutDevice_moduleAliasOnly_stripsTrailingTag (约第 1561 行)
TEST_F(EnableUtilsTest, disableOutDevice_moduleAliasOnly_stripsTrailingTag)
{
    Stub stub;
    EnableSqlManager *mgr = EnableSqlManager::getInstance();
    ASSERT_TRUE(UTDB::setup(mgr));

    ut_isSafeSysfs_ret = true;
    ut_qfileOpen_ret = true;
    ut_qfileOpen_calls = 0;  // 新增:重置全局计数器
    ut_qfileWrites.clear();
    stub.set(isSafeSysfsPath, ut_isSafeSysfs_stub);
    stub.set((char *)ut_qtSym(UT_QSYM_QFILE_OPEN), ut_qfileOpen_stub);
    stub.set((char *)ut_qtSym(UT_QSYM_QIOWRITE), ut_qfileWriteData_stub);
    // ... 其余不变
}

// 2. disableOutDevice_noIdentifiableId_skipsItem (约第 1577 行)
TEST_F(EnableUtilsTest, disableOutDevice_noIdentifiableId_skipsItem)
{
    Stub stub;
    ASSERT_TRUE(UTDB::setup(EnableSqlManager::getInstance()));

    ut_qfileOpen_ret = true;
    ut_qfileOpen_calls = 0;  // 新增:重置全局计数器
    ut_qfileWrites.clear();
    stub.set((char *)ut_qtSym(UT_QSYM_QFILE_OPEN), ut_qfileOpen_stub);
    stub.set((char *)ut_qtSym(UT_QSYM_QIOWRITE), ut_qfileWriteData_stub);
    // ... 其余不变
}

本报告由 AI 代码审查工具自动生成

- 新增 ut_enablesqlmanager.cpp:31 个用例覆盖单例/initDB/authorized/
  remove/printer/wake/net_wake/monitor_dev 读写与错误分支
- 新增 ut_testdbhelper.h:进程内 QTemporaryDir SQLite 注入夹具,
  测试自建自清,不触碰 /var/lib 系统路径
- 标红疑似缺陷:isUniqueIdEnabled/clearEnableFromAuthorizedTable
  引用不存在的 enable 列(用例 pinning,未改源码)
- 在上游已有用例基础上补充 disableOutDevice/getMapInfo/
  enableNetworkByDBus/enableNetworkByIoctl 分支用例至 42 个
- 外部依赖全部桩化:DBus/socket/ioctl/QFile/sysfs,不依赖真实设备
- 标红疑似缺陷:USB authorized 打开失败 return 中断循环、
  USB 接口号 [1-9]$ 强制归一化为 0(用例 pinning,未改源码)
- 在上游已有用例基础上补充 wakeOnLanIsOpen/setWakeOnLan/
  writeTlpWolConfig/getPS2Syspath/updateWakeupDeviceInfo 等用例至 49 个
- ethtool ioctl/sysfs//proc/tlp 配置文件全部桩化,测试不写系统路径
- 三个目标文件行覆盖 97.5%、函数 100%、分支 84.3%,分级门禁全过
1. Root cause: authorized table CREATE statement only defines an exist
   boolean column, but clearEnableFromAuthorizedTable and
   isUniqueIdEnabled reference a non-existent enable column, causing
   SQLite error "no such column: enable" and silent failure
2. Fix: replace enable with exist in both SQL statements; change
   .arg(true) to .arg(1) to match SQLite boolean integer storage
3. Impact: isUniqueIdEnabled now returns actual exist value instead of
   always false; clearEnableFromAuthorizedTable now correctly deletes
   rows where exist=1; ControlInterface::isDeviceEnabled reports
   correct device enabled state

Influence:
1. Test clearEnableFromAuthorizedTable with exist=true and exist=false
   rows to verify only exist=true rows are deleted
2. Test isUniqueIdEnabled with exist=true and exist=false records to
   verify correct return values
3. Verify ControlInterface::isDeviceEnabled DBus interface returns
   correct device enabled state after fix

fix: 修复 authorized 表列名 enable 为 exist

1. 根因:authorized 表建表语句仅定义了 exist boolean 列,但
   clearEnableFromAuthorizedTable 和 isUniqueIdEnabled 引用了不存在
   的 enable 列,导致 SQLite 报错 no such column: enable 并静默失败
2. 方案:将两处 SQL 中的 enable 替换为 exist;.arg(true) 改为 .arg(1)
   以匹配 SQLite 布尔值的整数存储方式
3. 影响:isUniqueIdEnabled 修复后返回实际 exist 值而非恒返回 false;
   clearEnableFromAuthorizedTable 正确删除 exist=1 的行;
   ControlInterface::isDeviceEnabled 正确报告设备启用状态

Influence:
1. 测试 clearEnableFromAuthorizedTable 在 exist=true 和 exist=false 行
   共存时仅删除 exist=true 的行
2. 测试 isUniqueIdEnabled 在 exist=true 和 exist=false 记录下返回
   正确的布尔值
3. 验证 ControlInterface::isDeviceEnabled DBus 接口修复后返回正确的
   设备启用状态

PMS: BUG-UT-DEF-001
The license-check CI (SPDX Header Validation) requires the copyright year
range of touched sources to end with the current year. Extend the
SPDX-FileCopyrightText range of enablesqlmanager.cpp from 2019 ~ 2023 so
that 2026 is covered and the SPDX summary reports 0 failures.
@deepin-bot

deepin-bot Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

TAG Bot

New tag: 6.0.75
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #956

@add-uos add-uos closed this Oct 30, 2026
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