Skip to content

Conversation

@yileicn
Copy link
Collaborator

@yileicn yileicn commented Nov 20, 2025

PR Type

Bug fix


Description

  • Fix NullReferenceException when HttpContext is unavailable

  • Replace direct HttpContext access with safe null-coalescing pattern

  • Use Context.GetHttpContext() for proper SignalR context retrieval


Diagram Walkthrough

flowchart LR
  A["Direct _context.HttpContext access"] -->|"causes NullReferenceException"| B["Unsafe code path"]
  C["Context.GetHttpContext() with null-coalescing"] -->|"safely handles null"| D["Robust code path"]
Loading

File Walkthrough

Relevant files
Bug fix
SignalRHub.cs
Safe HttpContext access with null-coalescing operators     

src/Plugins/BotSharp.Plugin.ChatHub/SignalRHub.cs

  • Replace direct _context.HttpContext access with
    Context.GetHttpContext() method call
  • Use null-coalescing operator (?.) for safe property access on
    potentially null HttpContext
  • Extract conversationId using safe query parameter retrieval with
    FirstOrDefault()
  • Prevent NullReferenceException when HttpContext is unavailable in
    SignalR connections
+2/-1     

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Nov 20, 2025

PR Compliance Guide 🔍

(Compliance updated until commit 078d685)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Logging context: New code adds logging about connections but does not clearly include all required audit
fields (e.g., explicit user ID and outcome) for a critical connect event.

Referred Code
public override async Task OnConnectedAsync()
{
    _logger.LogInformation($"SignalR Hub: {_user.FirstName} {_user.LastName} ({Context.User.Identity.Name}) connected in {Context.ConnectionId}");

    var convService = _services.GetRequiredService<IConversationService>();
    var httpContext = Context.GetHttpContext();
    string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault();

    if (!string.IsNullOrEmpty(conversationId))
    {
        _logger.LogInformation($"Connection {Context.ConnectionId} is with conversation {conversationId}");

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Null handling: While null-coalescing access is added for HttpContext and query, there is no
handling/logging for the case when conversation-id is missing or malformed to aid
debugging.

Referred Code
var httpContext = Context.GetHttpContext();
string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault();

if (!string.IsNullOrEmpty(conversationId))
{

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Sensitive logging: Logs include personal identifiers (first and last name) which may be considered sensitive
depending on policy, and exposure in user-visible contexts is unclear.

Referred Code
_logger.LogInformation($"SignalR Hub: {_user.FirstName} {_user.LastName} ({Context.User.Identity.Name}) connected in {Context.ConnectionId}");

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
PII in logs: Logging of _user.FirstName, _user.LastName, and Context.User.Identity.Name may include
PII; consider redaction or user IDs depending on logging policy.

Referred Code
_logger.LogInformation($"SignalR Hub: {_user.FirstName} {_user.LastName} ({Context.User.Identity.Name}) connected in {Context.ConnectionId}");

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 078d685
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Action logging: The new code logs connection events and conversation linkage but does not clearly ensure
that all critical actions are logged with user ID, timestamp, and outcome; additional
context may be provided elsewhere and needs verification.

Referred Code
_logger.LogInformation($"SignalR Hub: {_user.FirstName} {_user.LastName} ({Context.User.Identity.Name}) connected in {Context.ConnectionId}");

var convService = _services.GetRequiredService<IConversationService>();
var httpContext = Context.GetHttpContext();
string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault();

if (!string.IsNullOrEmpty(conversationId))
{
    _logger.LogInformation($"Connection {Context.ConnectionId} is with conversation {conversationId}");

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
PII in logs: The log message includes the user's first and last name and identity name, which may
be considered PII; verify policy compliance or redact in logs.

Referred Code
_logger.LogInformation($"SignalR Hub: {_user.FirstName} {_user.LastName} ({Context.User.Identity.Name}) connected in {Context.ConnectionId}");

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input validation: The query parameter 'conversation-id' is read and used for logging without
visible validation or authorization checks in this diff; validation may exist elsewhere
and needs verification.

Referred Code
string? conversationId = httpContext?.Request?.Query["conversation-id"].FirstOrDefault();

if (!string.IsNullOrEmpty(conversationId))
{
    _logger.LogInformation($"Connection {Context.ConnectionId} is with conversation {conversationId}");

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

@JackJiang1234
Copy link
Contributor

reviewed

@adenchen123
Copy link
Contributor

Reviewed

@yileicn yileicn merged commit 5f2083f into SciSharp:master Nov 20, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants