Develop

[ucswift]

No description provided.

[request-info]

Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details?

[coderabbitai]

Walkthrough

Warning

Review details and warnings were omitted to fit the comment limit.

[Resgrid-Bot]

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

⚠️ Note: some checks couldn't be completed in this run, so auto-approval (when enabled) was skipped. Check the details at http://localhost:3000/pull-requests and then comment @kody review on this PR to retry once the issue is resolved.

Kody Guide: Usage and Configuration

Interacting with Kody

• Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

• Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

• Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration

Review Options

The following review options are enabled or disabled:

Options	Enabled
Bug	✅
Performance	❌
Security	✅
Business Logic	❌

Access your configuration settings here.

​

[github-advanced-security]

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[Resgrid-Bot]

BrokerLegacySharedKeyEnabled enables a full-authority legacy shared credential by default, violating deny-by-default and least-privilege principles during migration. Disable it by default and require an explicit, scoped authorization path.

Kody rule violation: Implement RBAC with least privilege and deny-by-default

public static bool BrokerLegacySharedKeyEnabled = false;

Prompt for LLM

File Core/Resgrid.Config/DataProtectionConfig.cs:

Line 67:

`BrokerLegacySharedKeyEnabled` enables a full-authority legacy shared credential by default, violating deny-by-default and least-privilege principles during migration. Disable it by default and require an explicit, scoped authorization path.

Suggested Code:

public static bool BrokerLegacySharedKeyEnabled = false;

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The documented shared-session absolute timeout permits up to 24 hours, exceeding the required maximum of 12 hours. Change the documentation and corresponding limits in Core/Resgrid.Config/PasskeyConfig.cs:89 and Core/Resgrid.Model/DepartmentSecurityPolicy.cs:131 to a maximum of 12 hours.

Kody rule violation: Harden session management with idle and absolute timeouts

<value>Nombre d'heures après la connexion au bout desquelles une session partagée se termine, quelle que soit l'activité, de 1 à 12. Une valeur plus courte termine plus tôt les sessions en cours ; une valeur plus longue ne les prolonge jamais. Membre gestionnaire uniquement.</value>

Prompt for LLM

File Core/Resgrid.Localization/Areas/User/AdminAssist/AdminAssist.fr.resx:

Line 5521:

The documented shared-session absolute timeout permits up to 24 hours, exceeding the required maximum of 12 hours. Change the documentation and corresponding limits in `Core/Resgrid.Config/PasskeyConfig.cs:89` and `Core/Resgrid.Model/DepartmentSecurityPolicy.cs:131` to a maximum of 12 hours.

Suggested Code:

    <value>Nombre d'heures après la connexion au bout desquelles une session partagée se termine, quelle que soit l'activité, de 1 à 12. Une valeur plus courte termine plus tôt les sessions en cours ; une valeur plus longue ne les prolonge jamais. Membre gestionnaire uniquement.</value>

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

RegisterForSessionEvents lacks an explicit error handler and deterministic unsubscribe mechanism, so session-event failures and cleanup cannot be managed reliably. Accept an error callback and return an IDisposable cleanup handle.

Kody rule violation: Provide error handlers to subscription/listener APIs

IDisposable RegisterForSessionEvents(Func<string, string, Task> sessionEvent, Func<Exception, Task> errorHandler);

Prompt for LLM

File Core/Resgrid.Model/Providers/IRabbitInboundEventProvider.cs:

Line 34:

RegisterForSessionEvents lacks an explicit error handler and deterministic unsubscribe mechanism, so session-event failures and cleanup cannot be managed reliably. Accept an error callback and return an IDisposable cleanup handle.

Suggested Code:

		IDisposable RegisterForSessionEvents(Func<string, string, Task> sessionEvent, Func<Exception, Task> errorHandler);

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The awaited ProtectedGrantBinding.CheckAsync call in AdpReleaseService and the listed call sites can reject without guarded handling, propagating unhandled exceptions. Wrap the call in try/catch or apply an equivalent error-handling strategy that maps or logs the failure appropriately.

Kody rule violation: Handle async operations with proper error handling

Prompt for LLM

File Core/Resgrid.Services/AdpReleaseService.cs:

Line 30:

The awaited `ProtectedGrantBinding.CheckAsync` call in AdpReleaseService and the listed call sites can reject without guarded handling, propagating unhandled exceptions. Wrap the call in try/catch or apply an equivalent error-handling strategy that maps or logs the failure appropriately.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

AuthenticationChallengeService returns null from a Task-returning method, which can cause callers to receive a null Task and fail before awaiting a result. Return an explicit Task result, or change the contract to a nullable result type with a completed Task.

Kody rule violation: Avoid Returning Null in Non-Async Task Methods

return Task.FromResult<AuthenticationChallenge>(null);

Prompt for LLM

File Core/Resgrid.Services/AuthenticationChallengeService.cs:

Line 34:

AuthenticationChallengeService returns null from a Task-returning method, which can cause callers to receive a null Task and fail before awaiting a result. Return an explicit Task result, or change the contract to a nullable result type with a completed Task.

Suggested Code:

return Task.FromResult<AuthenticationChallenge>(null);

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

AuthenticationChallengeService catches every exception and converts it to AuthenticationChallengeOutcome.Unavailable, masking non-transient database failures and preventing them from surfacing for remediation. Catch only exceptions identified by IsTransientDatabaseException and rethrow non-transient failures after logging them.

Kody rule violation: Implement proper database error checking

catch (Exception ex) when (IsTransientDatabaseException(ex))
{
	Logging.LogException(ex, "Transient authentication challenge lookup failure.");
	return AuthenticationChallengeResult.Of(AuthenticationChallengeOutcome.Unavailable);
}
catch (Exception ex)
{
	Logging.LogException(ex, "Non-transient authentication challenge lookup failure.");
	throw;
}

Prompt for LLM

File Core/Resgrid.Services/AuthenticationChallengeService.cs:

Line 70 to 74:

AuthenticationChallengeService catches every exception and converts it to AuthenticationChallengeOutcome.Unavailable, masking non-transient database failures and preventing them from surfacing for remediation. Catch only exceptions identified by IsTransientDatabaseException and rethrow non-transient failures after logging them.

Suggested Code:

catch (Exception ex) when (IsTransientDatabaseException(ex))
{
	Logging.LogException(ex, "Transient authentication challenge lookup failure.");
	return AuthenticationChallengeResult.Of(AuthenticationChallengeOutcome.Unavailable);
}
catch (Exception ex)
{
	Logging.LogException(ex, "Non-transient authentication challenge lookup failure.");
	throw;
}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

RecordFailedAttemptAsync swallows every non-cancellation repository failure and returns success, so FactorRecoveryController continues using the same pending transaction when the attempt counter cannot be persisted, allowing unlimited replacement-authenticator guesses during a storage outage. Propagate a refusal or failure result and stop the recovery endpoint until the guarded attempt update succeeds.

public async Task RecordFailedAttemptAsync(FactorRecoveryTransaction transaction, CancellationToken cancellationToken = default)
{
	await _transactions.RecordFailedAttemptAsync(transaction.FactorRecoveryTransactionId, cancellationToken);
}

Prompt for LLM

File Core/Resgrid.Services/FactorRecoveryService.cs:

Line 99 to 109:

RecordFailedAttemptAsync swallows every non-cancellation repository failure and returns success, so FactorRecoveryController continues using the same pending transaction when the attempt counter cannot be persisted, allowing unlimited replacement-authenticator guesses during a storage outage. Propagate a refusal or failure result and stop the recovery endpoint until the guarded attempt update succeeds.

Suggested Code:

		public async Task RecordFailedAttemptAsync(FactorRecoveryTransaction transaction, CancellationToken cancellationToken = default)
		{
			await _transactions.RecordFailedAttemptAsync(transaction.FactorRecoveryTransactionId, cancellationToken);
		}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

MfaActivityService issues a repository query when userId or mfaActivityId is missing, allowing invalid identifiers to reach the data layer. Validate both identifiers first and return MfaActivityReportOutcome.NotFound without querying when either is blank.

Kody rule violation: Order validations before database queries

if (string.IsNullOrWhiteSpace(userId) || string.IsNullOrWhiteSpace(mfaActivityId))
				return new MfaActivityReport { Outcome = MfaActivityReportOutcome.NotFound };
			var activity = await _rows.GetAsync(mfaActivityId, cancellationToken);

Prompt for LLM

File Core/Resgrid.Services/MfaActivityService.cs:

Line 68:

MfaActivityService issues a repository query when userId or mfaActivityId is missing, allowing invalid identifiers to reach the data layer. Validate both identifiers first and return MfaActivityReportOutcome.NotFound without querying when either is blank.

Suggested Code:

if (string.IsNullOrWhiteSpace(userId) || string.IsNullOrWhiteSpace(mfaActivityId))
				return new MfaActivityReport { Outcome = MfaActivityReportOutcome.NotFound };
			var activity = await _rows.GetAsync(mfaActivityId, cancellationToken);

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

NamesCredential dereferences the potentially null grant before validating it, which can cause a null reference failure in the protected-grant flow. Check grant before accessing its properties and use GrantVersionWithCredential for the version comparison.

Kody rule violation: Add null checks to prevent NullReferenceException

private static bool NamesCredential(ProtectedDataGrant grant) =>
			grant != null && grant.Version >= GrantVersionWithCredential && grant.MfaMethod != ProtectedDataGrantMfaMethods.Totp && grant.MfaMethod != ProtectedDataGrantMfaMethods.None;

Prompt for LLM

File Core/Resgrid.Services/ProtectedGrantBinding.cs:

Line 64 to 65:

`NamesCredential` dereferences the potentially null `grant` before validating it, which can cause a null reference failure in the protected-grant flow. Check `grant` before accessing its properties and use `GrantVersionWithCredential` for the version comparison.

Suggested Code:

private static bool NamesCredential(ProtectedDataGrant grant) =>
			grant != null && grant.Version >= GrantVersionWithCredential && grant.MfaMethod != ProtectedDataGrantMfaMethods.Totp && grant.MfaMethod != ProtectedDataGrantMfaMethods.None;

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

Android origins are accepted solely because they match a 43-character shape, without checking an allowlisted signing-certificate hash, so any Android application with its own signing key can present an android:apk-key-hash: origin and bypass per-application passkey binding. Require Android origins to be explicitly configured and validate the normalized origin against the configured per-client allowlist instead of returning true for every regex match.

private static bool IsAllowedOrigin(string origin, string rpId, IReadOnlySet<string> allowedAndroidOrigins)
{
	if (AndroidOriginPattern.IsMatch(origin))
		return allowedAndroidOrigins.Contains(origin);
	// validate HTTPS web origins as before
}

Prompt for LLM

File Core/Resgrid.Services/RelyingPartyRegistry.cs:

Line 131 to 134:

Android origins are accepted solely because they match a 43-character shape, without checking an allowlisted signing-certificate hash, so any Android application with its own signing key can present an `android:apk-key-hash:` origin and bypass per-application passkey binding. Require Android origins to be explicitly configured and validate the normalized origin against the configured per-client allowlist instead of returning true for every regex match.

Suggested Code:

private static bool IsAllowedOrigin(string origin, string rpId, IReadOnlySet<string> allowedAndroidOrigins)
{
	if (AndroidOriginPattern.IsMatch(origin))
		return allowedAndroidOrigins.Contains(origin);
	// validate HTTPS web origins as before
}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

SecurityNoticeService performs delivery and related I/O once per notice inside a loop, serializing independent operations and increasing latency. Batch the work or safely parallelize it with Task.WhenAll, or add an aggregate delivery API.

Kody rule violation: Detect N+1 style queries and suggest batching

var deliveries = due.Select(notice => DeliverAsync(notice, cancellationToken));
foreach (var delivered in await Task.WhenAll(deliveries))
{
	if (delivered)
		sent++;
}

Prompt for LLM

File Core/Resgrid.Services/SecurityNoticeService.cs:

Line 120:

SecurityNoticeService performs delivery and related I/O once per notice inside a loop, serializing independent operations and increasing latency. Batch the work or safely parallelize it with Task.WhenAll, or add an aggregate delivery API.

Suggested Code:

			var deliveries = due.Select(notice => DeliverAsync(notice, cancellationToken));
			foreach (var delivered in await Task.WhenAll(deliveries))
			{
				if (delivered)
					sent++;
			}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The loop uses an equality-based termination expression and does not explicitly access the nullable value, reducing termination clarity. Use pattern matching with entries is null || i < entries.Value for deterministic relational termination.

Kody rule violation: Avoid equality operators in loop termination conditions

for (int i = 0; entries is null || i < entries.Value; i++)

Prompt for LLM

File Providers/Resgrid.Providers.Authentication/Fido2PasskeyProvider.cs:

Line 221:

The loop uses an equality-based termination expression and does not explicitly access the nullable value, reducing termination clarity. Use pattern matching with `entries is null || i < entries.Value` for deterministic relational termination.

Suggested Code:

for (int i = 0; entries is null || i < entries.Value; i++)

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The CBOR algorithm value is converted from Int64 to int without checked arithmetic, so an out-of-range value can silently overflow. Use checked((int)reader.ReadInt64()).

Kody rule violation: Prevent Numeric Overflow in Calculations

return checked((int)reader.ReadInt64());

Prompt for LLM

File Providers/Resgrid.Providers.Authentication/Fido2PasskeyProvider.cs:

Line 230:

The CBOR algorithm value is converted from Int64 to int without checked arithmetic, so an out-of-range value can silently overflow. Use `checked((int)reader.ReadInt64())`.

Suggested Code:

return checked((int)reader.ReadInt64());

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The Rabbit/message-bus publish operation in RabbitTopicProvider and the listed call sites lacks guarded failure context, making session-event delivery failures difficult to diagnose. Wrap the operation in try/catch, log the operation and session identifier with structured fields, and rethrow or map the error appropriately.

Kody rule violation: Add try-catch blocks for external calls

public async Task<bool> SessionEvent(string sessionId, string payload)
{
	try
	{
		return await SendMessage(Topics.EventingTopic, new EventingMessage
		{
			Id = Guid.NewGuid(), Type = (int)EventingTypes.SessionEvent, TimeStamp = DateTime.UtcNow, ItemId = sessionId, Payload = payload
		}.SerializeJson());
	}
	catch (Exception exception)
	{
		_logger.Error(exception, "Failed to publish session event for session {SessionId}", sessionId);
		throw;
	}
}

Prompt for LLM

File Providers/Resgrid.Providers.Bus.Rabbit/RabbitTopicProvider.cs:

Line 90 to 93:

The Rabbit/message-bus publish operation in RabbitTopicProvider and the listed call sites lacks guarded failure context, making session-event delivery failures difficult to diagnose. Wrap the operation in try/catch, log the operation and session identifier with structured fields, and rethrow or map the error appropriately.

Suggested Code:

public async Task<bool> SessionEvent(string sessionId, string payload)
{
	try
	{
		return await SendMessage(Topics.EventingTopic, new EventingMessage
		{
			Id = Guid.NewGuid(), Type = (int)EventingTypes.SessionEvent, TimeStamp = DateTime.UtcNow, ItemId = sessionId, Payload = payload
		}.SerializeJson());
	}
	catch (Exception exception)
	{
		_logger.Error(exception, "Failed to publish session event for session {SessionId}", sessionId);
		throw;
	}
}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

Adding a non-null column with a default in one migration can lock or rewrite a large table, creating deployment risk across the listed migrations. Use an expand/contract strategy: add the column nullable, backfill existing rows in batches, add the default for new writes, and enforce NOT NULL later with an online migration and documented rollback plan.

Kody rule violation: Block risky database migrations (locking ops, downtime risk)

Prompt for LLM

File Providers/Resgrid.Providers.Migrations/Migrations/M0251_AddFederatedMfaMapping.cs:

Line 19:

Adding a non-null column with a default in one migration can lock or rewrite a large table, creating deployment risk across the listed migrations. Use an expand/contract strategy: add the column nullable, backfill existing rows in batches, add the default for new writes, and enforce NOT NULL later with an online migration and documented rollback plan.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The _postgres field is initialized once and never reassigned, so its declaration should remain readonly to enforce immutability. Keep private readonly bool _postgres; or verify that the field is intentionally mutable throughout the type.

Kody rule violation: Use `readonly` or `const` for Immutable Data

Prompt for LLM

File Repositories/Resgrid.Repositories.DataRepository/AuthenticationChallengeRepository.cs:

Line 25:

The `_postgres` field is initialized once and never reassigned, so its declaration should remain readonly to enforce immutability. Keep `private readonly bool _postgres;` or verify that the field is intentionally mutable throughout the type.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

CurrentTimeStep rounds Unix seconds before integer division, advancing the TOTP step by up to 0.5 seconds and allowing the server to consume a future step before the actual 30-second boundary, which causes intermittent valid-code failures. Use floor/integer division of elapsed Unix seconds, such as (long)(utcNow - UnixEpoch).TotalSeconds / StepSeconds, to match RFC 6238 and Identity's time-step calculation.

public static long CurrentTimeStep(DateTime utcNow)
	=> (long)(utcNow - UnixEpoch).TotalSeconds / StepSeconds;

Prompt for LLM

File Repositories/Resgrid.Repositories.DataRepository/Stores/TotpCalculator.cs:

Line 23 to 25:

`CurrentTimeStep` rounds Unix seconds before integer division, advancing the TOTP step by up to 0.5 seconds and allowing the server to consume a future step before the actual 30-second boundary, which causes intermittent valid-code failures. Use floor/integer division of elapsed Unix seconds, such as `(long)(utcNow - UnixEpoch).TotalSeconds / StepSeconds`, to match RFC 6238 and Identity's time-step calculation.

Suggested Code:

public static long CurrentTimeStep(DateTime utcNow)
	=> (long)(utcNow - UnixEpoch).TotalSeconds / StepSeconds;

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The related database inserts execute without a transaction, so a failure can leave the test database partially seeded. Enclose the inserts in a transaction, commit only after all succeed, and roll back on failure.

Kody rule violation: Handle transaction rollbacks properly

using var transaction = db.BeginTransaction();
try
{
    await db.ExecuteAsync($"INSERT INTO {Q("AspNetUsers")} (...) VALUES (...); INSERT INTO {Q("DepartmentSsoConfigs")} VALUES (...); INSERT INTO {Q("DepartmentSecurityPolicies")} VALUES (...)", args, transaction);
    transaction.Commit();
}
catch
{
    transaction.Rollback();
    throw;
}

Prompt for LLM

File Tests/Resgrid.Tests/AdminAssist/AdminAssistDatabaseTests.cs:

Line 355:

The related database inserts execute without a transaction, so a failure can leave the test database partially seeded. Enclose the inserts in a transaction, commit only after all succeed, and roll back on failure.

Suggested Code:

using var transaction = db.BeginTransaction();
try
{
    await db.ExecuteAsync($"INSERT INTO {Q("AspNetUsers")} (...) VALUES (...); INSERT INTO {Q("DepartmentSsoConfigs")} VALUES (...); INSERT INTO {Q("DepartmentSecurityPolicies")} VALUES (...)", args, transaction);
    transaction.Commit();
}
catch
{
    transaction.Rollback();
    throw;
}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The listed Regex instances, including LocalizerInject in Tests/Resgrid.Tests/Localization/RazorOutputEncodingTests.cs, omit a timeout and can be exposed to regular-expression denial-of-service attacks. Define an explicit timeout for every regex that processes untrusted or externally sourced input.

Kody rule violation: Specify Timeout for Regular Expressions

Prompt for LLM

File Tests/Resgrid.Tests/Localization/RazorOutputEncodingTests.cs:

Line 45:

The listed Regex instances, including `LocalizerInject` in `Tests/Resgrid.Tests/Localization/RazorOutputEncodingTests.cs`, omit a timeout and can be exposed to regular-expression denial-of-service attacks. Define an explicit timeout for every regex that processes untrusted or externally sourced input.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The listed tests use int.Parse to convert string input, which can throw on invalid or unexpected formats. Use a TryParse-style API and validate the culture and format where applicable.

Kody rule violation: Use TryParse for string conversions

Prompt for LLM

File Tests/Resgrid.Tests/Localization/ResourceKeyParityTests.cs:

Line 64:

The listed tests use `int.Parse` to convert string input, which can throw on invalid or unexpected formats. Use a TryParse-style API and validate the culture and format where applicable.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The listed database tests build SQL with unsanitized _database input, allowing SQL injection through the concatenated CREATE DATABASE statement. Use parameterized queries or a validated identifier-quoting mechanism for database names.

Kody rule violation: Prevent SQL Injection in Queries

Prompt for LLM

File Tests/Resgrid.Tests/Security/BrokerReplayDatabaseTests.cs:

Line 70:

The listed database tests build SQL with unsanitized `_database` input, allowing SQL injection through the concatenated `CREATE DATABASE` statement. Use parameterized queries or a validated identifier-quoting mechanism for database names.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The async setup method calls the migration runner's synchronous MigrateUp() API, blocking the execution thread during database work. Use the asynchronous MigrateUpAsync() API.

Kody rule violation: Use Awaitable Methods in Async Code

await _runner.GetRequiredService<IMigrationRunner>().MigrateUpAsync();

Prompt for LLM

File Tests/Resgrid.Tests/Security/BrokerReplayDatabaseTests.cs:

Line 85:

The async setup method calls the migration runner's synchronous `MigrateUp()` API, blocking the execution thread during database work. Use the asynchronous `MigrateUpAsync()` API.

Suggested Code:

await _runner.GetRequiredService<IMigrationRunner>().MigrateUpAsync();

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

BrokerReplayDatabaseTests uses an async-capable primary-constructor pattern for lifecycle initialization, which obscures synchronous construction and setup ordering. Keep the constructor synchronous and move asynchronous initialization into the setup method.

Kody rule violation: Avoid asynchronous operations in constructors

public class BrokerReplayDatabaseTests
	{
		public BrokerReplayDatabaseTests(DatabaseTypes type) { ... }

Prompt for LLM

File Tests/Resgrid.Tests/Security/BrokerReplayDatabaseTests.cs:

Line 32:

BrokerReplayDatabaseTests uses an async-capable primary-constructor pattern for lifecycle initialization, which obscures synchronous construction and setup ordering. Keep the constructor synchronous and move asynchronous initialization into the setup method.

Suggested Code:

public class BrokerReplayDatabaseTests
	{
		public BrokerReplayDatabaseTests(DatabaseTypes type) { ... }

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The listed tests block asynchronous operations with .Result or .Wait(), which can cause deadlocks and prevent efficient asynchronous execution. Convert the call paths to async and use await instead of blocking.

Kody rule violation: Avoid Blocking Calls to Async Methods

Prompt for LLM

File Tests/Resgrid.Tests/Security/DepartmentLockAuthFlowsTests.cs:

Line 63:

The listed tests block asynchronous operations with `.Result` or `.Wait()`, which can cause deadlocks and prevent efficient asynchronous execution. Convert the call paths to async and use `await` instead of blocking.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

DepartmentLockAuthFlowsTests and the listed MfaApprovalServiceTests, MfaPolicyEnforcementTests, MfaStepUpIssuanceTests, SsoBrokerServiceTests, and ConnectControllerSsoTests block on asynchronous operations instead of awaiting them. Use async/await end-to-end, avoid .Result and .Wait(), and configure awaits appropriately.

Kody rule violation: Await async operations properly

Prompt for LLM

File Tests/Resgrid.Tests/Security/DepartmentLockAuthFlowsTests.cs:

Line 63:

DepartmentLockAuthFlowsTests and the listed MfaApprovalServiceTests, MfaPolicyEnforcementTests, MfaStepUpIssuanceTests, SsoBrokerServiceTests, and ConnectControllerSsoTests block on asynchronous operations instead of awaiting them. Use async/await end-to-end, avoid `.Result` and `.Wait()`, and configure awaits appropriately.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The listed code uses an inline lambda in Should().ContainSingle(u => u.Web), although the cited rule targets .bind() or arrow functions in JSX props and does not apply to this C# test expression. Apply the rule only to JSX props, or provide a specific concern for this LINQ predicate.

Kody rule violation: Avoid using .bind() or arrow functions in JSX props

Prompt for LLM

File Tests/Resgrid.Tests/Security/LegacyAppCallbacksTests.cs:

Line 122:

The listed code uses an inline lambda in `Should().ContainSingle(u => u.Web)`, although the cited rule targets `.bind()` or arrow functions in JSX props and does not apply to this C# test expression. Apply the rule only to JSX props, or provide a specific concern for this LINQ predicate.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

LiveSignInServer.StartAsync starts the externally reachable test server over plaintext HTTP, leaving transport security and HSTS behavior untested. Require HTTPS with TLS 1.2 or later.

Kody rule violation: Enforce TLS 1.2+ and HSTS on all external endpoints

public static async Task<LiveSignInServer> StartAsync(string url = "https://127.0.0.1:0")

Prompt for LLM

File Tests/Resgrid.Tests/Security/Live/LiveSignInServer.cs:

Line 593:

LiveSignInServer.StartAsync starts the externally reachable test server over plaintext HTTP, leaving transport security and HSTS behavior untested. Require HTTPS with TLS 1.2 or later.

Suggested Code:

public static async Task<LiveSignInServer> StartAsync(string url = "https://127.0.0.1:0")

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The disposable ECDsa instance in SoftPasskeyAuthenticator and the listed call sites is never deterministically released, which can leak cryptographic resources. Implement IDisposable on the owning type and dispose the field during teardown.

Kody rule violation: Use using statements for disposable resources

private readonly ECDsa _key = ECDsa.Create(ECCurve.NamedCurves.nistP256);

public void Dispose() => _key.Dispose();

Prompt for LLM

File Tests/Resgrid.Tests/Security/SoftPasskeyAuthenticator.cs:

Line 19:

The disposable `ECDsa` instance in SoftPasskeyAuthenticator and the listed call sites is never deterministically released, which can leak cryptographic resources. Implement IDisposable on the owning type and dispose the field during teardown.

Suggested Code:

private readonly ECDsa _key = ECDsa.Create(ECCurve.NamedCurves.nistP256);

public void Dispose() => _key.Dispose();

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The listed files use a plain <img> element for app assets without the required Next.js Image component, explicit dimensions, and meaningful alt text. Replace the element with the appropriate image component and provide dimensions and accessible alternative text.

Kody rule violation: Use next/image with explicit dimensions and alt

Prompt for LLM

File Tests/Resgrid.Tests/Web/User/ContactsIndexEncodingTests.cs:

Line 41:

The listed files use a plain `<img>` element for app assets without the required Next.js Image component, explicit dimensions, and meaningful alt text. Replace the element with the appropriate image component and provide dimensions and accessible alternative text.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The delayed response timer remains active when the request closes, allowing a response write after the connection has ended. Retain the timeout handle and clear it from the request's close handler.

Kody rule violation: Clear timers on teardown/unmount

const timer = setTimeout(() => response.writeHead(code, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' }).end(json), state.delay);
request.on('close', () => clearTimeout(timer));

Prompt for LLM

File Tests/Resgrid.Tests/Web/resgrid-shared-session.test.cjs:

Line 71:

The delayed response timer remains active when the request closes, allowing a response write after the connection has ended. Retain the timeout handle and clear it from the request's close handler.

Suggested Code:

const timer = setTimeout(() => response.writeHead(code, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' }).end(json), state.delay);
request.on('close', () => clearTimeout(timer));

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The connection is registered before the awaited SignalR group join, and AddToGroupAsync failures leave dead connections in SessionConnectionRegistry even though OnConnectedAsync never completed. Wrap registration, group joining, and the downstream callback in try/finally or equivalent cleanup logic, and unregister when setup or the callback fails after registration.

var registered = false;\ntry\n{\n\t_connections.Register(context.Context.ConnectionId, sessionId, context.Context.Abort);\n\tregistered = true;\n\tawait context.Hub.Groups.AddToGroupAsync(context.Context.ConnectionId, SessionEvents.GroupFor(sessionId));\n\tawait next(context);\n}\ncatch\n{\n\tif (registered)\n\t\t_connections.Unregister(context.Context.ConnectionId);\n\tthrow;\n}

Prompt for LLM

File Web/Resgrid.Web.Eventing/Middleware/SessionValidationHubFilter.cs:

Line 49 to 53:

The connection is registered before the awaited SignalR group join, and AddToGroupAsync failures leave dead connections in SessionConnectionRegistry even though OnConnectedAsync never completed. Wrap registration, group joining, and the downstream callback in try/finally or equivalent cleanup logic, and unregister when setup or the callback fails after registration.

Suggested Code:

var registered = false;\ntry\n{\n\t_connections.Register(context.Context.ConnectionId, sessionId, context.Context.Abort);\n\tregistered = true;\n\tawait context.Hub.Groups.AddToGroupAsync(context.Context.ConnectionId, SessionEvents.GroupFor(sessionId));\n\tawait next(context);\n}\ncatch\n{\n\tif (registered)\n\t\t_connections.Unregister(context.Context.ConnectionId);\n\tthrow;\n}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

Sso/Redeem routes every redeemed SSO purpose other than Reauthentication to LoginAsync, so session-bound StepUp and AdpStepUp transactions are treated as fresh SSO logins and create login MFA/completion artifacts with incorrect policy and audit semantics. Dispatch only Login transactions to LoginAsync, route Reauthentication to ReauthenticatedAsync, and reject or handle StepUp/AdpStepUp exclusively in their dedicated consumers.

var transaction = redeemed.Transaction;
return transaction.TransactionPurpose switch
{
	SsoTransactionPurpose.Login => await LoginAsync(transaction, client, input.ClientId, cancellationToken),
	SsoTransactionPurpose.Reauthentication => await ReauthenticatedAsync(transaction, cancellationToken),
	_ => Refuse(SsoBrokerOutcome.InvalidRequest)
};

Prompt for LLM

File Web/Resgrid.Web.Services/Controllers/v4/SsoController.cs:

Line 148 to 151:

Sso/Redeem routes every redeemed SSO purpose other than Reauthentication to LoginAsync, so session-bound StepUp and AdpStepUp transactions are treated as fresh SSO logins and create login MFA/completion artifacts with incorrect policy and audit semantics. Dispatch only Login transactions to LoginAsync, route Reauthentication to ReauthenticatedAsync, and reject or handle StepUp/AdpStepUp exclusively in their dedicated consumers.

Suggested Code:

var transaction = redeemed.Transaction;
return transaction.TransactionPurpose switch
{
	SsoTransactionPurpose.Login => await LoginAsync(transaction, client, input.ClientId, cancellationToken),
	SsoTransactionPurpose.Reauthentication => await ReauthenticatedAsync(transaction, cancellationToken),
	_ => Refuse(SsoBrokerOutcome.InvalidRequest)
};

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

SessionValidationMiddleware logs only a message string for the failure, omitting the exception and identifiers needed to correlate the operation. Emit a structured error log containing the operation name, exception, session ID, request trace ID, and user or actor ID, with the same context applied to the listed call sites.

Kody rule violation: Include error context in structured logs

Prompt for LLM

File Web/Resgrid.Web.Services/Middleware/SessionValidationMiddleware.cs:

Line 132:

SessionValidationMiddleware logs only a message string for the failure, omitting the exception and identifiers needed to correlate the operation. Emit a structured error log containing the operation name, exception, session ID, request trace ID, and user or actor ID, with the same context applied to the listed call sites.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

model.Groups can be null when passed to BSTreeModel.ForDepartmentGroups, causing a null-related failure in PersonnelController and the other listed call sites. Use Enumerable.Empty<DepartmentGroup>() as the default collection.

Kody rule violation: Add null checks before accessing properties

trees.AddRange(BSTreeModel.ForDepartmentGroups(model.Groups ?? Enumerable.Empty<DepartmentGroup>()));

Prompt for LLM

File Web/Resgrid.Web/Areas/User/Controllers/PersonnelController.cs:

Line 325:

`model.Groups` can be null when passed to `BSTreeModel.ForDepartmentGroups`, causing a null-related failure in PersonnelController and the other listed call sites. Use `Enumerable.Empty<DepartmentGroup>()` as the default collection.

Suggested Code:

trees.AddRange(BSTreeModel.ForDepartmentGroups(model.Groups ?? Enumerable.Empty<DepartmentGroup>()));

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The federated MFA mapping audit record in SecurityController uses legacy fields and omits tamper-evident metadata required for traceability. Populate UTC timestamp, actor role, action, resource, result, trace ID, IP, and user agent, then ensure the service writes to immutable/WORM storage and forwards the record to the SIEM.

Kody rule violation: Emit tamper-evident audit logs with required fields

await _systemAuditsService.SaveSystemAuditAsync(new SystemAudit
{
	TimestampUtc = DateTime.UtcNow,
	ActorUserId = UserId,
	ActorRole = CurrentRole,
	Action = removing ? "FEDERATED_MFA_MAPPING_REMOVE" : "FEDERATED_MFA_MAPPING_SAVE",
	ResourceId = config.DepartmentSsoConfigId,
	Result = "success",
	TraceId = HttpContext.TraceIdentifier,
	IpAddress = IpAddressHelper.GetRequestIP(Request, true),
	UserAgent = Request.Headers["User-Agent"].ToString()
}, cancellationToken);

Prompt for LLM

File Web/Resgrid.Web/Areas/User/Controllers/SecurityController.cs:

Line 1484 to 1497:

The federated MFA mapping audit record in SecurityController uses legacy fields and omits tamper-evident metadata required for traceability. Populate UTC timestamp, actor role, action, resource, result, trace ID, IP, and user agent, then ensure the service writes to immutable/WORM storage and forwards the record to the SIEM.

Suggested Code:

await _systemAuditsService.SaveSystemAuditAsync(new SystemAudit
{
	TimestampUtc = DateTime.UtcNow,
	ActorUserId = UserId,
	ActorRole = CurrentRole,
	Action = removing ? "FEDERATED_MFA_MAPPING_REMOVE" : "FEDERATED_MFA_MAPPING_SAVE",
	ResourceId = config.DepartmentSsoConfigId,
	Result = "success",
	TraceId = HttpContext.TraceIdentifier,
	IpAddress = IpAddressHelper.GetRequestIP(Request, true),
	UserAgent = Request.Headers["User-Agent"].ToString()
}, cancellationToken);

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

Disable2FA awaits SecurityNoticeService before revoking all sessions and signing out, so a notice-delivery failure can leave the current and other old sessions authenticated after MFA is disabled. Make NoticeAsync best-effort by catching and logging its exception, or revoke sessions and sign out before awaiting the notice.

await _systemAuditsService.SaveSystemAuditAsync(new SystemAudit
{
    System = (int)SystemAuditSystems.Website,
    Type = (int)SystemAuditTypes.TwoFactorDisabled,
    UserId = user.Id,
    Username = user.UserName,
    Successful = true,
    IpAddress = IpAddressHelper.GetRequestIP(Request, true),
    ServerName = Environment.MachineName,
    Data = $"2FA disabled via web; all sessions revoked. {Request.Headers["User-Agent"]}"
}, cancellationToken);

await _signInManager.ForgetTwoFactorClientAsync();
await _userSessionService.RevokeAllAfterCredentialChangeAsync(user.Id, user.Id,
    UserSessionRevocationReason.MfaChanged, now, cancellationToken);
try
{
    await NoticeAsync(user, SecurityNoticeKind.TotpDisabled, cancellationToken);
}
catch (Exception ex) when (!(ex is OperationCanceledException))
{
    Logging.LogException(ex, "Failed to queue MFA-disabled security notice.");
}
await HttpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);

Prompt for LLM

File Web/Resgrid.Web/Areas/User/Controllers/TwoFactorController.cs:

Line 412 to 421:

Disable2FA awaits SecurityNoticeService before revoking all sessions and signing out, so a notice-delivery failure can leave the current and other old sessions authenticated after MFA is disabled. Make NoticeAsync best-effort by catching and logging its exception, or revoke sessions and sign out before awaiting the notice.

Suggested Code:

await _systemAuditsService.SaveSystemAuditAsync(new SystemAudit
{
    System = (int)SystemAuditSystems.Website,
    Type = (int)SystemAuditTypes.TwoFactorDisabled,
    UserId = user.Id,
    Username = user.UserName,
    Successful = true,
    IpAddress = IpAddressHelper.GetRequestIP(Request, true),
    ServerName = Environment.MachineName,
    Data = $"2FA disabled via web; all sessions revoked. {Request.Headers["User-Agent"]}"
}, cancellationToken);

await _signInManager.ForgetTwoFactorClientAsync();
await _userSessionService.RevokeAllAfterCredentialChangeAsync(user.Id, user.Id,
    UserSessionRevocationReason.MfaChanged, now, cancellationToken);
try
{
    await NoticeAsync(user, SecurityNoticeKind.TotpDisabled, cancellationToken);
}
catch (Exception ex) when (!(ex is OperationCanceledException))
{
    Logging.LogException(ex, "Failed to queue MFA-disabled security notice.");
}
await HttpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The Enable2FA and ReplaceAuthenticator POST actions accept staged authenticator codes without rechecking the fresh first-factor requirement enforced by their GET actions, allowing credential-change authorization to outlive its intended window. Re-run HasFreshFirstFactorAsync at the start of each POST action and redirect to reauthentication when it is no longer valid.

if (await _userManager.GetTwoFactorEnabledAsync(user))
    return RedirectToAction(nameof(Index));

if (!await HasFreshFirstFactorAsync(user, TwoFactorConfig.FirstFactorReauthWindowMinutes))
    return RedirectToReauthenticate(Url.Action(nameof(Enable2FA)));

var stagedKey = await GetStagedAuthenticatorKeyAsync(user);

Prompt for LLM

File Web/Resgrid.Web/Areas/User/Controllers/TwoFactorController.cs:

Line 177 to 182:

The Enable2FA and ReplaceAuthenticator POST actions accept staged authenticator codes without rechecking the fresh first-factor requirement enforced by their GET actions, allowing credential-change authorization to outlive its intended window. Re-run HasFreshFirstFactorAsync at the start of each POST action and redirect to reauthentication when it is no longer valid.

Suggested Code:

if (await _userManager.GetTwoFactorEnabledAsync(user))
    return RedirectToAction(nameof(Index));

if (!await HasFreshFirstFactorAsync(user, TwoFactorConfig.FirstFactorReauthWindowMinutes))
    return RedirectToReauthenticate(Url.Action(nameof(Enable2FA)));

var stagedKey = await GetStagedAuthenticatorKeyAsync(user);

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The multi-stage passkey query combines retrieval, ordering, projection, and materialization in one expression, making each stage difficult to inspect independently. Split it into named intermediate expressions for activePasskeys, orderedPasskeys, and the final Passkeys projection.

Kody rule violation: Limit Lengthy LINQ Chains

var activePasskeys = await _passkeys.GetActiveForUserAsync(user.Id, cancellationToken) ?? Array.Empty<UserPasskey>();
var orderedPasskeys = activePasskeys.OrderBy(p => p.ClientApplication).ThenBy(p => p.CreatedOnUtc);
Passkeys = orderedPasskeys.Select(p => new PasskeyRowView { ... }).ToList()

Prompt for LLM

File Web/Resgrid.Web/Areas/User/Controllers/TwoFactorController.cs:

Line 126 to 136:

The multi-stage passkey query combines retrieval, ordering, projection, and materialization in one expression, making each stage difficult to inspect independently. Split it into named intermediate expressions for activePasskeys, orderedPasskeys, and the final Passkeys projection.

Suggested Code:

var activePasskeys = await _passkeys.GetActiveForUserAsync(user.Id, cancellationToken) ?? Array.Empty<UserPasskey>();
var orderedPasskeys = activePasskeys.OrderBy(p => p.ClientApplication).ThenBy(p => p.CreatedOnUtc);
Passkeys = orderedPasskeys.Select(p => new PasskeyRowView { ... }).ToList()

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

DepartmentGroupId is interpolated into raw HTML through Html.Raw without encoding, allowing model-derived values to inject markup or attributes. Encode DepartmentGroupId before inserting it into the attribute, and apply the same protection to the listed views.

Kody rule violation: Always sanitize user inputs

@Html.Raw("<li role='presentation' class='active'><a href='#unitsTab" + Html.Encode(Model.Groups[i].DepartmentGroupId) + "' aria-controls='home' role='tab' data-toggle='tab'>" + Html.Encode(Model.Groups[i].Name) + "</a></li>")

Prompt for LLM

File Web/Resgrid.Web/Areas/User/Views/Dispatch/NewCall.cshtml:

Line 369:

DepartmentGroupId is interpolated into raw HTML through Html.Raw without encoding, allowing model-derived values to inject markup or attributes. Encode DepartmentGroupId before inserting it into the attribute, and apply the same protection to the listed views.

Suggested Code:

@Html.Raw("<li role='presentation' class='active'><a href='#unitsTab" + Html.Encode(Model.Groups[i].DepartmentGroupId) + "' aria-controls='home' role='tab' data-toggle='tab'>" + Html.Encode(Model.Groups[i].Name) + "</a></li>")

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

LoginMfaSetup and the listed POST actions process the submitted model and may issue downstream operations before validating ModelState. Return the view immediately when ModelState.IsValid is false.

Kody rule violation: Always Validate `ModelState.IsValid` in Controllers

if (!ModelState.IsValid) return View(model);

Prompt for LLM

File Web/Resgrid.Web/Controllers/AccountController.Recovery.cs:

Line 71:

LoginMfaSetup and the listed POST actions process the submitted model and may issue downstream operations before validating ModelState. Return the view immediately when `ModelState.IsValid` is false.

Suggested Code:

if (!ModelState.IsValid) return View(model);

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The controller-wide AllowDuringDepartmentLock exemption bypasses the department operation-lock filter for every AccountController action, including ForcePasswordChange, which writes department membership state and defeats the filter's data-entry boundary. Remove the controller-level exemption and apply AllowDuringDepartmentLock only to the required sign-in, sign-out, and MFA/shared-session actions.

public partial class AccountController : Controller
{
    // Apply [AllowDuringDepartmentLock] only to the individual authentication/MFA actions that require it.

Prompt for LLM

File Web/Resgrid.Web/Controllers/AccountController.cs:

Line 39 to 42:

The controller-wide AllowDuringDepartmentLock exemption bypasses the department operation-lock filter for every AccountController action, including ForcePasswordChange, which writes department membership state and defeats the filter's data-entry boundary. Remove the controller-level exemption and apply AllowDuringDepartmentLock only to the required sign-in, sign-out, and MFA/shared-session actions.

Suggested Code:

public partial class AccountController : Controller
{
    // Apply [AllowDuringDepartmentLock] only to the individual authentication/MFA actions that require it.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The passkey-removal checkbox always renders a boolean checked attribute, including when the model value is false; HTML treats checked="False" as checked, so recovery submits every displayed passkey in RemovePasskeyIds. Render the attribute only when Model.RemovePasskeyIds.Contains(passkey.Id) is true.

<input type="checkbox" name="RemovePasskeyIds" value="@passkey.Id" @(Model.RemovePasskeyIds.Contains(passkey.Id) ? "checked=\"checked\"" : "") />

Prompt for LLM

File Web/Resgrid.Web/Views/Account/Recovery.cshtml:

Line 58:

The passkey-removal checkbox always renders a boolean checked attribute, including when the model value is false; HTML treats `checked="False"` as checked, so recovery submits every displayed passkey in RemovePasskeyIds. Render the attribute only when `Model.RemovePasskeyIds.Contains(passkey.Id)` is true.

Suggested Code:

<input type="checkbox" name="RemovePasskeyIds" value="@passkey.Id" @(Model.RemovePasskeyIds.Contains(passkey.Id) ? "checked=\"checked\"" : "") />

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The SsoLogOn view and listed views serve only a PNG without explicit dimensions or accessible text, which can reduce image performance and cause layout shifts. Serve AVIF/WebP with a PNG fallback and provide meaningful alt text, explicit width and height, loading="lazy", and decoding="async".

Kody rule violation: Serve responsive images with modern formats and lazy-load

<picture><source srcset="~/images/Resgrid_JustText.avif" type="image/avif" /><source srcset="~/images/Resgrid_JustText.webp" type="image/webp" /><img src="~/images/Resgrid_JustText.png" alt="Resgrid" width="200" height="40" loading="lazy" decoding="async" /></picture>

Prompt for LLM

File Web/Resgrid.Web/Views/Account/SsoLogOn.cshtml:

Line 23:

The SsoLogOn view and listed views serve only a PNG without explicit dimensions or accessible text, which can reduce image performance and cause layout shifts. Serve AVIF/WebP with a PNG fallback and provide meaningful alt text, explicit width and height, `loading="lazy"`, and `decoding="async"`.

Suggested Code:

<picture><source srcset="~/images/Resgrid_JustText.avif" type="image/avif" /><source srcset="~/images/Resgrid_JustText.webp" type="image/webp" /><img src="~/images/Resgrid_JustText.png" alt="Resgrid" width="200" height="40" loading="lazy" decoding="async" /></picture>

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

The empty catch in Web/Resgrid.Web/wwwroot/js/app/common/shared/resgrid.shared.locked.js silently swallows exceptions, obscuring failures and preventing recovery. Log the exception with relevant context and either rethrow it or handle it explicitly.

Kody rule violation: Avoid empty catch blocks

Prompt for LLM

File Web/Resgrid.Web/wwwroot/js/app/common/shared/resgrid.shared.locked.js:

Line 51:

The empty catch in `Web/Resgrid.Web/wwwroot/js/app/common/shared/resgrid.shared.locked.js` silently swallows exceptions, obscuring failures and preventing recovery. Log the exception with relevant context and either rethrow it or handle it explicitly.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[Resgrid-Bot]

Assigning xhr.responseJSON.redirectUrl directly to window.location.href permits arbitrary or untrusted redirections. Parse the URL and allow navigation only when its origin matches window.location.origin and its protocol is https:.

Kody rule violation: Avoid unprotected HTTP request redirections

const redirectUrl = new URL(xhr.responseJSON.redirectUrl, window.location.origin);
if (redirectUrl.origin === window.location.origin && redirectUrl.protocol === 'https:') {
	window.location.href = redirectUrl.href;
}

Prompt for LLM

File Web/Resgrid.Web/wwwroot/js/app/internal/profile/resgrid.profile.yourdepartments.js:

Line 58:

Assigning xhr.responseJSON.redirectUrl directly to window.location.href permits arbitrary or untrusted redirections. Parse the URL and allow navigation only when its origin matches window.location.origin and its protocol is `https:`.

Suggested Code:

const redirectUrl = new URL(xhr.responseJSON.redirectUrl, window.location.origin);
if (redirectUrl.origin === window.location.origin && redirectUrl.protocol === 'https:') {
	window.location.href = redirectUrl.href;
}

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[ucswift]

@CodeRabbit

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@ucswift What would you like me to do with this PR? I can review the changes, answer code questions, or plan fixes.

You are interacting with an AI system.

[ucswift]

@CodeRabbit Review

[coderabbitai]

⚠️ Action not completed

Review skipped: 409 files exceed the limit of 150.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.