Skip to content

fix(update): restrict to global admin, register commands, add error handling #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ModerRAS merged 1 commit into from
May 11, 2026
Merged

fix(update): restrict to global admin, register commands, add error handling #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 5 additions & 29 deletions TelegramSearchBot.Test/Controller/Manage/UpdateControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,25 @@
using Telegram.Bot;
using Telegram.Bot.Requests.Abstractions;
using Telegram.Bot.Types;
using TelegramSearchBot.Common;
using TelegramSearchBot.Controller.Manage;
using TelegramSearchBot.Model;
using TelegramSearchBot.Service.Common;
using TelegramSearchBot.Service.Manage;
using TelegramSearchBot.Service.Scheduler;
using Xunit;

namespace TelegramSearchBot.Test.Controller.Manage;

/// <summary>
/// RED-phase tests for UpdateController bot command handling.
/// Tests for UpdateController bot command handling.
///
/// These tests validate the command routing logic, null-safety guards,
/// admin permission checks, and the full ExecuteAsync flow.
/// admin permission checks (global admin via Env.AdminId), and the full ExecuteAsync flow.
///
/// NOTE: Many integration-style tests are EXPECTED to fail at runtime
/// in the RED phase because:
/// 1. AdminService.IsNormalAdmin is non-virtual and cannot be mocked.
/// 2. SelfUpdateBootstrap is a static class with platform-dependent partial methods.
/// 3. Telegram.Bot v22 API methods are extension methods (not interface methods)
/// and cannot be mocked by Moq.
///
/// The test structure is correct and compiles; the GREEN phase (Task 10)
/// will make the system-under-test fully mockable.
/// NOTE: Some integration-style tests call SelfUpdateBootstrap static methods
/// which depend on network access and platform-specific functionality.
/// </summary>
public class UpdateControllerTests
{
private readonly Mock<ITelegramBotClient> _mockBotClient;
private readonly Mock<AdminService> _mockAdminService;
private readonly Mock<IHostApplicationLifetime> _mockAppLifetime;
private readonly UpdateController _controller;

Expand All @@ -43,19 +33,6 @@ public UpdateControllerTests()
_mockBotClient = new Mock<ITelegramBotClient>();
_mockAppLifetime = new Mock<IHostApplicationLifetime>();

// AdminService constructor requires several dependencies.
// IsNormalAdmin is now virtual — mock returns true for admin permission.
_mockAdminService = new Mock<AdminService>(
Mock.Of<Microsoft.Extensions.Logging.ILogger<AdminService>>(),
null!, // DataDbContext — not needed when IsNormalAdmin is mocked
Mock.Of<IAppConfigurationService>(),
Mock.Of<StackExchange.Redis.IConnectionMultiplexer>(),
Mock.Of<ISchedulerService>());

_mockAdminService
.Setup(x => x.IsNormalAdmin(It.IsAny<long>()))
.ReturnsAsync(true);

// SendMessage in Telegram.Bot v22 is an extension method that calls
// ITelegramBotClient.SendRequest<T>() internally. Mock the underlying
// interface method to avoid real HTTP calls to Telegram API.
Expand All @@ -67,7 +44,6 @@ public UpdateControllerTests()

_controller = new UpdateController(
_mockBotClient.Object,
_mockAdminService.Object,
_mockAppLifetime.Object);
}

Expand Down
98 changes: 52 additions & 46 deletions TelegramSearchBot/Controller/Manage/UpdateController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,22 @@
using Microsoft.Extensions.Hosting;
using Telegram.Bot;
using Telegram.Bot.Types;
using TelegramSearchBot.Common;
using TelegramSearchBot.Interface.Controller;
using TelegramSearchBot.Model;
using TelegramSearchBot.Service.AppUpdate;
using TelegramSearchBot.Service.Manage;

namespace TelegramSearchBot.Controller.Manage {
public class UpdateController : IOnUpdate {
private readonly ITelegramBotClient _botClient;
private readonly AdminService _adminService;
private readonly IHostApplicationLifetime _applicationLifetime;

public List<Type> Dependencies => new List<Type>() { typeof(AdminController) };

public UpdateController(
ITelegramBotClient botClient,
AdminService adminService,
IHostApplicationLifetime applicationLifetime) {
_botClient = botClient;
_adminService = adminService;
_applicationLifetime = applicationLifetime;
}

Expand All @@ -39,8 +36,9 @@ public async Task ExecuteAsync(PipelineContext p) {
return;
}

if (!await _adminService.IsNormalAdmin(message.From.Id)) {
await ReplyAsync(message, "❌ 权限不足：只有管理员才能使用更新指令。");
// 仅允许全局管理员使用更新指令
if (message.From.Id != Env.AdminId) {
await ReplyAsync(message, "❌ 权限不足：只有全局管理员才能使用更新指令。");
return;
}
Comment on lines +39 to 43

@coderabbitai coderabbitai Bot May 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify Env.AdminId initialization and usage patterns

# Check Env.AdminId declaration and initialization
ast-grep --pattern 'class Env {
  $$$
  AdminId
  $$$
}'

# Check for validation or initialization of AdminId
rg -nP --type=cs 'AdminId\s*[=:]' -A 2 -B 2

# Check if there's startup validation
rg -nP --type=cs 'AdminId.*==\s*0|AdminId.*validation|ValidateConfig' -C 3

Repository: ModerRAS/TelegramSearchBot

Length of output: 475

🏁 Script executed:

# Read full Env.cs to see AdminId property definition and type
cat -n TelegramSearchBot.Common/Env.cs | head -60

Repository: ModerRAS/TelegramSearchBot

Length of output: 3507

🏁 Script executed:

# Find where config is loaded and how it's validated
rg -n --type=cs 'config\s*=|ConfigProvider|LoadConfig|BotConfig' -A 3 -B 1 | head -100

Repository: ModerRAS/TelegramSearchBot

Length of output: 9677

🏁 Script executed:

# Check for BotConfig class definition to see AdminId field
fd -t f '*.cs' -x grep -l 'class.*BotConfig' {}

Repository: ModerRAS/TelegramSearchBot

Length of output: 354

🏁 Script executed:

# Find Config class - likely in same directory as Env.cs
fd -t f -x grep -l 'class Config' {} \; | head -5

Repository: ModerRAS/TelegramSearchBot

Length of output: 104

🏁 Script executed:

# Check for Config class in Common namespace
rg -n --type=cs 'class Config' -A 10 TelegramSearchBot.Common/

Repository: ModerRAS/TelegramSearchBot

Length of output: 1103

🏁 Script executed:

# Also check what the default Config looks like and AdminId initialization
rg -n --type=cs 'AdminId' -B 5 -A 5 | head -80

Repository: ModerRAS/TelegramSearchBot

Length of output: 7323

🏁 Script executed:

# Verify if there's any startup validation for AdminId
rg -n --type=cs 'AdminId.*0|AdminId.*validation|AdminId.*required' -B 2 -A 2

Repository: ModerRAS/TelegramSearchBot

Length of output: 52

🏁 Script executed:

# Check if Config.json existence is validated at startup
rg -n --type=cs 'Config\.json|WorkDir.*Config' -B 2 -A 2 | head -50

Repository: ModerRAS/TelegramSearchBot

Length of output: 523

Critical security issue: Env.AdminId defaults to zero when Config.json is missing, inverting permission checks across all management controllers.

The permission check if (message.From.Id != Env.AdminId) becomes if (message.From.Id != 0) when AdminId defaults to 0, allowing access to all users except user ID 0 instead of denying access to all except the configured admin. This occurs silently when Config.json is missing or contains invalid JSON (lines 14-21 of Env.cs catch exceptions without logging).

Add validation in the Env static constructor to: (1) require Config.json to exist and be valid JSON, (2) require AdminId > 0 with an explicit error message, and (3) log configuration failures. This vulnerability affects UpdateController, EditOCRConfController, EditVisionConfController, EditLLMConfController, EditMcpConfController, RefreshController, and CheckBanGroupController.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@TelegramSearchBot/Controller/Manage/UpdateController.cs` around lines 39 -
43, The Env static constructor must stop silently defaulting AdminId=0: validate
that Config.json exists and parses as valid JSON, log any exceptions (including
parse errors and missing file), and enforce AdminId>0 (log a clear error and
throw/terminate startup if invalid) so controllers like UpdateController,
EditOCRConfController, EditVisionConfController, EditLLMConfController,
EditMcpConfController, RefreshController and CheckBanGroupController cannot
invert permission checks; update Env (static ctor and any config-loading helper)
to perform these checks and emit explicit error logs before failing fast rather
than leaving AdminId at 0.


Expand Down Expand Up @@ -70,50 +68,58 @@ private static bool IsUpdateCommand(string text) {
}

private async Task HandleCheckUpdateAsync(Message message) {
var result = await SelfUpdateBootstrap.GetUpdateStatusAsync();
var sb = new StringBuilder();
sb.AppendLine("更新状态");
sb.AppendLine($"当前版本: {result.CurrentVersion}");
sb.AppendLine($"运行位置: {( result.RunningManagedInstall ? "独立安装目录" : "桥接启动实例" )}");
sb.AppendLine($"独立安装目录: {( result.ManagedInstallExists ? "已存在" : "尚未建立" )}");

if (!string.IsNullOrWhiteSpace(result.LatestVersion)) {
sb.AppendLine($"最新版本: {result.LatestVersion}");
try {
var result = await SelfUpdateBootstrap.GetUpdateStatusAsync();
var sb = new StringBuilder();
sb.AppendLine("更新状态");
sb.AppendLine($"当前版本: {result.CurrentVersion}");
sb.AppendLine($"运行位置: {( result.RunningManagedInstall ? "独立安装目录" : "桥接启动实例" )}");
sb.AppendLine($"独立安装目录: {( result.ManagedInstallExists ? "已存在" : "尚未建立" )}");

if (!string.IsNullOrWhiteSpace(result.LatestVersion)) {
sb.AppendLine($"最新版本: {result.LatestVersion}");
}

switch (result.State) {
case ManagedUpdateState.UpToDate:
sb.AppendLine("状态: 已是最新版本。");
break;
case ManagedUpdateState.UpdateAvailable:
sb.AppendLine($"状态: 可更新到 {result.TargetVersion ?? result.LatestVersion}。");
sb.AppendLine("发送「更新」或 /update 立即开始升级。");
break;
case ManagedUpdateState.UpdateUnavailable:
case ManagedUpdateState.NoPathFound:
sb.AppendLine($"状态: {result.Message ?? "当前没有可用更新路径。"}");
break;
default:
sb.AppendLine($"状态: {result.Message ?? result.State.ToString()}");
break;
}

await ReplyAsync(message, sb.ToString());
} catch (Exception ex) {
await ReplyAsync(message, $"❌ 检查更新时发生异常：{ex.Message}");
}

switch (result.State) {
case ManagedUpdateState.UpToDate:
sb.AppendLine("状态: 已是最新版本。");
break;
case ManagedUpdateState.UpdateAvailable:
sb.AppendLine($"状态: 可更新到 {result.TargetVersion ?? result.LatestVersion}。");
sb.AppendLine("发送“更新”或 /update 立即开始升级。");
break;
case ManagedUpdateState.UpdateUnavailable:
case ManagedUpdateState.NoPathFound:
sb.AppendLine($"状态: {result.Message ?? "当前没有可用更新路径。"}");
break;
default:
sb.AppendLine($"状态: {result.Message ?? result.State.ToString()}");
break;
}

await ReplyAsync(message, sb.ToString());
}

private async Task HandleStartUpdateAsync(Message message) {
var result = await SelfUpdateBootstrap.StartUpdateAsync();
string responseText = result.State switch {
ManagedUpdateState.UpdateScheduled => $"✅ 已开始准备更新到 {result.TargetVersion ?? result.LatestVersion}，机器人将退出并在更新后自动重启。",
ManagedUpdateState.UpToDate => "✅ 当前已经是最新版本，无需更新。",
ManagedUpdateState.UpdateAvailable => $"ℹ️ 已检测到新版本 {result.TargetVersion ?? result.LatestVersion}，但暂未成功调度更新。",
_ => $"❌ 无法启动更新：{result.Message ?? result.State.ToString()}"
};

await ReplyAsync(message, responseText);

if (result.ShouldStopApplication) {
_applicationLifetime.StopApplication();
try {
var result = await SelfUpdateBootstrap.StartUpdateAsync();
string responseText = result.State switch {
ManagedUpdateState.UpdateScheduled => $"✅ 已开始准备更新到 {result.TargetVersion ?? result.LatestVersion}，机器人将退出并在更新后自动重启。",
ManagedUpdateState.UpToDate => "✅ 当前已经是最新版本，无需更新。",
ManagedUpdateState.UpdateAvailable => $"ℹ️ 已检测到新版本 {result.TargetVersion ?? result.LatestVersion}，但暂未成功调度更新。",
_ => $"❌ 无法启动更新：{result.Message ?? result.State.ToString()}"
};

await ReplyAsync(message, responseText);

if (result.ShouldStopApplication) {
_applicationLifetime.StopApplication();
}
} catch (Exception ex) {
await ReplyAsync(message, $"❌ 启动更新时发生异常：{ex.Message}");
}
}

Expand Down
3 changes: 2 additions & 1 deletion TelegramSearchBot/Service/BotAPI/TelegramCommandRegistryService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) {
var commands = new List<BotCommand>
{
new BotCommand { Command = "resolveurls", Description = "解析文本中的链接并存储原始链接与解析后链接的映射。" },
// 可以根据需要添加更多命令
new BotCommand { Command = "checkupdate", Description = "检查系统更新状态。" },
new BotCommand { Command = "update", Description = "执行系统更新（如果存在新版本）。" },
};

_logger.LogInformation($"Registering {commands.Count} commands...");
Expand Down
Loading