Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| @if (Model.ModuleState.RecordsUsable && ClaimsAuthorizationHelper.CanExportRecords()) | ||
| { | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="csv"><i class="fa fa-download"></i> @localizer["ExportCsv"]</a> |
| } | ||
| @if (Model.ModuleState.RecordsUsable && ClaimsAuthorizationHelper.CanExportRecords()) | ||
| { | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="csv"><i class="fa fa-download"></i> @localizer["ExportCsv"]</a> |
| } | ||
| @if (Model.ModuleState.RecordsUsable && ClaimsAuthorizationHelper.CanExportRecords()) | ||
| { | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="csv"><i class="fa fa-download"></i> @localizer["ExportCsv"]</a> |
| @if (Model.ModuleState.RecordsUsable && ClaimsAuthorizationHelper.CanExportRecords()) | ||
| { | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="csv"><i class="fa fa-download"></i> @localizer["ExportCsv"]</a> | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="json"><i class="fa fa-download"></i> @localizer["ExportJson"]</a> |
| @if (Model.ModuleState.RecordsUsable && ClaimsAuthorizationHelper.CanExportRecords()) | ||
| { | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="csv"><i class="fa fa-download"></i> @localizer["ExportCsv"]</a> | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="json"><i class="fa fa-download"></i> @localizer["ExportJson"]</a> |
| @if (Model.ModuleState.RecordsUsable && ClaimsAuthorizationHelper.CanExportRecords()) | ||
| { | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="csv"><i class="fa fa-download"></i> @localizer["ExportCsv"]</a> | ||
| <a class="btn btn-default" asp-controller="Records" asp-action="ExportList" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-format="json"><i class="fa fa-download"></i> @localizer["ExportJson"]</a> |
| @for (var p = 1; p <= Model.TotalPages; p++) | ||
| { | ||
| <li class="@(p == Model.Page ? "active" : string.Empty)"> | ||
| <a asp-controller="Records" asp-action="Index" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-owner="@Model.OwnerFilter" asp-route-group="@Model.GroupFilter" asp-route-page="@p">@p</a> |
| @for (var p = 1; p <= Model.TotalPages; p++) | ||
| { | ||
| <li class="@(p == Model.Page ? "active" : string.Empty)"> | ||
| <a asp-controller="Records" asp-action="Index" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-owner="@Model.OwnerFilter" asp-route-group="@Model.GroupFilter" asp-route-page="@p">@p</a> |
| @for (var p = 1; p <= Model.TotalPages; p++) | ||
| { | ||
| <li class="@(p == Model.Page ? "active" : string.Empty)"> | ||
| <a asp-controller="Records" asp-action="Index" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-owner="@Model.OwnerFilter" asp-route-group="@Model.GroupFilter" asp-route-page="@p">@p</a> |
| @for (var p = 1; p <= Model.TotalPages; p++) | ||
| { | ||
| <li class="@(p == Model.Page ? "active" : string.Empty)"> | ||
| <a asp-controller="Records" asp-action="Index" asp-route-area="User" asp-route-year="@Model.Year" asp-route-definitionKey="@Model.DefinitionKey" asp-route-state="@Model.StateFilter" asp-route-q="@Model.Query" asp-route-owner="@Model.OwnerFilter" asp-route-group="@Model.GroupFilter" asp-route-page="@p">@p</a> |
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
|
Important Review skippedToo many files! This PR contains 196 files, which is 46 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to Team to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: ⛔ Files ignored due to path filters (118)
📒 Files selected for processing (196)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ❌ |
| public static class AttachmentScanningConfig | ||
| { | ||
| /// <summary>Master switch. When false the scanner never opens a connection.</summary> | ||
| public static bool Enabled = false; |
There was a problem hiding this comment.
Mutable configuration state in Core/Resgrid.Config/AttachmentScanningConfig.cs because public static bool Enabled = false; declares immutable metadata as a writable field. Mark it const, as shown, or readonly if runtime assignment is required.
Kody rule violation: Use `readonly` or `const` for Immutable Data
public const bool Enabled = false;Prompt for LLM
File Core/Resgrid.Config/AttachmentScanningConfig.cs:
Line 11:
Mutable configuration state in Core/Resgrid.Config/AttachmentScanningConfig.cs because public static bool Enabled = false; declares immutable metadata as a writable field. Mark it const, as shown, or readonly if runtime assignment is required.
Suggested Code:
public const bool Enabled = false;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| namespace Resgrid.Localization.Areas.User.Records | ||
| { | ||
| public class Records |
There was a problem hiding this comment.
Suspicious empty type in Core/Resgrid.Localization/Areas/User/Records/Records.cs because public class Records has no instance state or behavior and exposes only an implicit public constructor. Make Records static, as shown, or remove it until it has a concrete instantiation purpose.
Kody rule violation: Avoid Private-Only Constructors
public static class RecordsPrompt for LLM
File Core/Resgrid.Localization/Areas/User/Records/Records.cs:
Line 7:
Suspicious empty type in Core/Resgrid.Localization/Areas/User/Records/Records.cs because public class Records has no instance state or behavior and exposes only an implicit public constructor. Make Records static, as shown, or remove it until it has a concrete instantiation purpose.
Suggested Code:
public static class Records
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return descriptor; | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
Null return contract in Core/Resgrid.Model/Records/RecordPermissionCatalog.cs because return null; from a reference-returning API forces callers onto a null-handling path, with matching occurrences in Core/Resgrid.Model/Events/RecordsWorkflowEvent.cs:37-37, Tests/Resgrid.Tests/Rms/RecordsReportingServiceTests.cs:158-158, and Repositories/Resgrid.Repositories.DataRepository/WorkflowRunRepository.cs:60-60. Return a non-null default or use an explicit nullable or exception-based absence pattern.
Kody rule violation: Avoid Returning Null in Non-Async Task Methods
return default;Prompt for LLM
File Core/Resgrid.Model/Records/RecordPermissionCatalog.cs:
Line 81:
Null return contract in Core/Resgrid.Model/Records/RecordPermissionCatalog.cs because return null; from a reference-returning API forces callers onto a null-handling path, with matching occurrences in Core/Resgrid.Model/Events/RecordsWorkflowEvent.cs:37-37, Tests/Resgrid.Tests/Rms/RecordsReportingServiceTests.cs:158-158, and Repositories/Resgrid.Repositories.DataRepository/WorkflowRunRepository.cs:60-60. Return a non-null default or use an explicit nullable or exception-based absence pattern.
Suggested Code:
return default;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| try { _directory.Dispose(); } catch (Exception ex) { Logging.LogException(ex); } | ||
| } | ||
| Analyzer.Dispose(); |
There was a problem hiding this comment.
Fragile disposal in Core/Resgrid.Search/LuceneRecordsIndexHost.cs because Analyzer.Dispose() is not guarded even though other IDisposable cleanup follows a deterministic protected pattern, with a matching occurrence in Tests/Resgrid.Tests/Rms/RecordAttachmentHygieneTests.cs:22-22. Wrap disposal in exception handling and log the failure with operation="Dispose" and indexPath = IndexPath.
Kody rule violation: Use using statements for disposable resources
try { Analyzer.Dispose(); } catch (Exception ex) { Logging.LogException(ex, "Disposing analyzer failed.", new { operation = "Dispose", indexPath = IndexPath }); }Prompt for LLM
File Core/Resgrid.Search/LuceneRecordsIndexHost.cs:
Line 153:
Fragile disposal in Core/Resgrid.Search/LuceneRecordsIndexHost.cs because Analyzer.Dispose() is not guarded even though other IDisposable cleanup follows a deterministic protected pattern, with a matching occurrence in Tests/Resgrid.Tests/Rms/RecordAttachmentHygieneTests.cs:22-22. Wrap disposal in exception handling and log the failure with operation="Dispose" and indexPath = IndexPath.
Suggested Code:
try { Analyzer.Dispose(); } catch (Exception ex) { Logging.LogException(ex, "Disposing analyzer failed.", new { operation = "Dispose", indexPath = IndexPath }); }
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var searcher = manager.Acquire(); | ||
| try | ||
| { | ||
| var hits = searcher.Search(new TermQuery(new Term(RecordsIndexFields.DepartmentId, departmentId.ToString())), 1); |
There was a problem hiding this comment.
Misleading async contract in Core/Resgrid.Search/LuceneRecordsIndexer.cs because CountDocumentsAsync performs the synchronous call searcher.Search(new TermQuery(new Term(RecordsIndexFields.DepartmentId, departmentId.ToString())), 1). Use an awaitable API if one exists, or make the method synchronous or rename it to avoid implying non-blocking behavior.
Kody rule violation: Use Awaitable Methods in Async Code
Prompt for LLM
File Core/Resgrid.Search/LuceneRecordsIndexer.cs:
Line 81:
Misleading async contract in Core/Resgrid.Search/LuceneRecordsIndexer.cs because CountDocumentsAsync performs the synchronous call searcher.Search(new TermQuery(new Term(RecordsIndexFields.DepartmentId, departmentId.ToString())), 1). Use an awaitable API if one exists, or make the method synchronous or rename it to avoid implying non-blocking behavior.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| await _accessAuditsRepository.InsertAsync(new RmsAccessAudit | ||
| { | ||
| DepartmentId = departmentId, | ||
| Action = (int)RmsAccessAuditAction.Activation, | ||
| ActorUserId = userId, | ||
| Purpose = "Records activation", | ||
| Successful = true, | ||
| OccurredOn = now, | ||
| OriginClient = (int)RmsOriginClient.Web, | ||
| DetailJson = JsonConvert.SerializeObject(new { cutover.RmsDepartmentCutoverId, reason }) | ||
| }, cancellationToken, true); |
There was a problem hiding this comment.
Incomplete audit payload in Core/Resgrid.Services/Records/RecordsCutoverService.cs because the RmsAccessAudit activation record omits required fields such as trace_id, actor.role, result, ip, and user_agent, including the similar block at lines 307-317. Emit a tamper-evident DetailJson payload that includes trace_id, actor, action, resource, result, ip, user_agent, and reason_token.
Kody rule violation: Emit tamper-evident audit logs with required fields
await _accessAuditsRepository.InsertAsync(new RmsAccessAudit
{
DepartmentId = departmentId,
Action = (int)RmsAccessAuditAction.Activation,
ActorUserId = userId,
Purpose = "Records activation",
Successful = true,
OccurredOn = now,
OriginClient = (int)RmsOriginClient.Web,
DetailJson = JsonConvert.SerializeObject(new { trace_id, actor = new { user_id = userId, role }, action = "records.activation", resource = new { id = cutover.RmsDepartmentCutoverId }, result = "success", ip, user_agent, reason_token })
}, cancellationToken, true);Prompt for LLM
File Core/Resgrid.Services/Records/RecordsCutoverService.cs:
Line 234 to 244:
Incomplete audit payload in Core/Resgrid.Services/Records/RecordsCutoverService.cs because the RmsAccessAudit activation record omits required fields such as trace_id, actor.role, result, ip, and user_agent, including the similar block at lines 307-317. Emit a tamper-evident DetailJson payload that includes trace_id, actor, action, resource, result, ip, user_agent, and reason_token.
Suggested Code:
await _accessAuditsRepository.InsertAsync(new RmsAccessAudit
{
DepartmentId = departmentId,
Action = (int)RmsAccessAuditAction.Activation,
ActorUserId = userId,
Purpose = "Records activation",
Successful = true,
OccurredOn = now,
OriginClient = (int)RmsOriginClient.Web,
DetailJson = JsonConvert.SerializeObject(new { trace_id, actor = new { user_id = userId, role }, action = "records.activation", resource = new { id = cutover.RmsDepartmentCutoverId }, result = "success", ip, user_agent, reason_token })
}, cancellationToken, true);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| UsageSnapshot snapshot; | ||
| try | ||
| { | ||
| snapshot = string.IsNullOrWhiteSpace(reference.SnapshotJson) ? new UsageSnapshot() : JsonConvert.DeserializeObject<UsageSnapshot>(reference.SnapshotJson) ?? new UsageSnapshot(); |
There was a problem hiding this comment.
Null-reference risk in Core/Resgrid.Services/Records/RmsInventoryUsageAdapter.cs because reference.SnapshotJson is dereferenced even though the reference parameter may be absent, with matching occurrences at lines 111, 106, 118, 117, and 113 and in the listed callers. Guard the access with reference?.SnapshotJson or an explicit null check before deserialization.
Kody rule violation: Add null checks before accessing properties
snapshot = string.IsNullOrWhiteSpace(reference?.SnapshotJson)
? new UsageSnapshot()
: JsonConvert.DeserializeObject<UsageSnapshot>(reference.SnapshotJson) ?? new UsageSnapshot();Prompt for LLM
File Core/Resgrid.Services/Records/RmsInventoryUsageAdapter.cs:
Line 99:
Null-reference risk in Core/Resgrid.Services/Records/RmsInventoryUsageAdapter.cs because reference.SnapshotJson is dereferenced even though the reference parameter may be absent, with matching occurrences at lines 111, 106, 118, 117, and 113 and in the listed callers. Guard the access with reference?.SnapshotJson or an explicit null check before deserialization.
Suggested Code:
snapshot = string.IsNullOrWhiteSpace(reference?.SnapshotJson)
? new UsageSnapshot()
: JsonConvert.DeserializeObject<UsageSnapshot>(reference.SnapshotJson) ?? new UsageSnapshot();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| recordNumber = "TRN-2026-0042"; | ||
| revisionNumber = 1; | ||
| revisionId = "0f3c1d2e-5b6a-4c7d-8e9f-0a1b2c3d4e5f"; | ||
| finalizedOn = DateTime.Now; |
There was a problem hiding this comment.
Timing accuracy issue in Core/Resgrid.Services/WorkflowSampleDataGenerator.cs because DateTime.Now is affected by daylight savings and system clock adjustments, including the occurrences at lines 561, 571, 582, 605, 606, 607, 631, 632, and 633. Use Stopwatch for elapsed timing operations.
Kody rule violation: Avoid `DateTime.Now` for Timing Operations
Prompt for LLM
File Core/Resgrid.Services/WorkflowSampleDataGenerator.cs:
Line 551:
Timing accuracy issue in Core/Resgrid.Services/WorkflowSampleDataGenerator.cs because DateTime.Now is affected by daylight savings and system clock adjustments, including the occurrences at lines 561, 571, 582, 605, 606, 607, 631, 632, and 633. Use Stopwatch for elapsed timing operations.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// </summary> | ||
| private static void AddDepartmentBranding(Dictionary<string, object> templateModel, DepartmentEmailBranding branding) | ||
| { | ||
| var enabled = branding != null && branding.Enabled && !String.IsNullOrWhiteSpace(branding.LogoUrl); |
There was a problem hiding this comment.
Null-reference risk in Providers/Resgrid.Providers.Email/PostmarkTemplateProvider.cs because branding is dereferenced through branding.Enabled and branding.LogoUrl after a manual null check. Use explicit null-safe access with branding?.Enabled == true and branding?.LogoUrl to make the nullable contract consistent.
Kody rule violation: Add null checks to prevent NullReferenceException
var enabled = branding?.Enabled == true && !String.IsNullOrWhiteSpace(branding?.LogoUrl);Prompt for LLM
File Providers/Resgrid.Providers.Email/PostmarkTemplateProvider.cs:
Line 721:
Null-reference risk in Providers/Resgrid.Providers.Email/PostmarkTemplateProvider.cs because branding is dereferenced through branding.Enabled and branding.LogoUrl after a manual null check. Use explicit null-safe access with branding?.Enabled == true and branding?.LogoUrl to make the nullable contract consistent.
Suggested Code:
var enabled = branding?.Enabled == true && !String.IsNullOrWhiteSpace(branding?.LogoUrl);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <tr> | ||
| {{#department_branding}}<tr> | ||
| <td class="email-masthead"> | ||
| <a href="{{department_website}}" class="email-masthead_name"><img src="{{department_logo_url}}" alt="{{department_display_name}}" class="email-masthead_logo" width="94" style="width: 94px; max-width: 94px; height: auto; border: 0; display: block; margin: 0 auto 6px auto;" /><span>{{department_display_name}}</span></a> |
There was a problem hiding this comment.
Rule mismatch in Providers/Resgrid.Providers.Email/Template/Call.html because the Next.js image requirement does not apply to plain HTML email templates and cshtml views, including the listed occurrences in email templates, tests, and Razor files. Enforce only requirements that are valid for each rendering platform.
Kody rule violation: Use next/image with explicit dimensions and alt
Prompt for LLM
File Providers/Resgrid.Providers.Email/Template/Call.html:
Line 395:
Rule mismatch in Providers/Resgrid.Providers.Email/Template/Call.html because the Next.js image requirement does not apply to plain HTML email templates and cshtml views, including the listed occurrences in email templates, tests, and Razor files. Enforce only requirements that are valid for each rendering platform.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <tr> | ||
| {{#department_branding}}<tr> | ||
| <td class="email-masthead"> | ||
| <a href="{{department_website}}" class="email-masthead_name"><img src="{{department_logo_url}}" alt="{{department_display_name}}" class="email-masthead_logo" width="94" style="width: 94px; max-width: 94px; height: auto; border: 0; display: block; margin: 0 auto 6px auto;" /><span>{{department_display_name}}</span></a> |
There was a problem hiding this comment.
Incomplete image attributes in Providers/Resgrid.Providers.Email/Template/ReportDelivery.html because the defines width="94" but not an explicit height or loading hints, with matching occurrences in Providers/Resgrid.Providers.Email/Template/Call.html:395-395 and Providers/Resgrid.Providers.Email/Template/Message.html:395-395. Add height="94", loading="lazy", and decoding="async".
Kody rule violation: Serve responsive images with modern formats and lazy-load
<a href="{{department_website}}" class="email-masthead_name"><img src="{{department_logo_url}}" alt="{{department_display_name}}" class="email-masthead_logo" width="94" height="94" loading="lazy" decoding="async" style="width: 94px; max-width: 94px; height: auto; border: 0; display: block; margin: 0 auto 6px auto;" /><span>{{department_display_name}}</span></a>Prompt for LLM
File Providers/Resgrid.Providers.Email/Template/ReportDelivery.html:
Line 395:
Incomplete image attributes in Providers/Resgrid.Providers.Email/Template/ReportDelivery.html because the <img> defines width="94" but not an explicit height or loading hints, with matching occurrences in Providers/Resgrid.Providers.Email/Template/Call.html:395-395 and Providers/Resgrid.Providers.Email/Template/Message.html:395-395. Add height="94", loading="lazy", and decoding="async".
Suggested Code:
<a href="{{department_website}}" class="email-masthead_name"><img src="{{department_logo_url}}" alt="{{department_display_name}}" class="email-masthead_logo" width="94" height="94" loading="lazy" decoding="async" style="width: 94px; max-width: 94px; height: auto; border: 0; display: block; margin: 0 auto 6px auto;" /><span>{{department_display_name}}</span></a>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Create.Index("IX_RmsOperationalRecords_Department_Owner").OnTable("RmsOperationalRecords") | ||
| .OnColumn("DepartmentId").Ascending().OnColumn("OwnerUserId").Ascending(); | ||
|
|
||
| Execute.Sql("CREATE NONCLUSTERED INDEX IX_RmsOperationalRecords_Department_Call ON RmsOperationalRecords (DepartmentId, CallId) WHERE CallId IS NOT NULL;"); |
There was a problem hiding this comment.
Unsafe index migration strategy in Providers/Resgrid.Providers.Migrations/Migrations/M0150_AddRmsRecordsCore.cs because Execute.Sql("CREATE NONCLUSTERED INDEX IX_RmsOperationalRecords_Department_Call ON RmsOperationalRecords (DepartmentId, CallId) WHERE CallId IS NOT NULL;") creates an index with raw SQL without showing an online or rollback-safe path, with matching occurrences in the listed migration files. Use the database's online or concurrent option where supported, or document why the operation is safe for the target table size and rollback plan.
Kody rule violation: Block risky database migrations (locking ops, downtime risk)
// Use an online migration strategy for index creation on large tables, or document table size/rollback plan.
Execute.Sql("CREATE NONCLUSTERED INDEX IX_RmsOperationalRecords_Department_Call ON RmsOperationalRecords (DepartmentId, CallId) WHERE CallId IS NOT NULL;");Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0150_AddRmsRecordsCore.cs:
Line 84:
Unsafe index migration strategy in Providers/Resgrid.Providers.Migrations/Migrations/M0150_AddRmsRecordsCore.cs because Execute.Sql("CREATE NONCLUSTERED INDEX IX_RmsOperationalRecords_Department_Call ON RmsOperationalRecords (DepartmentId, CallId) WHERE CallId IS NOT NULL;") creates an index with raw SQL without showing an online or rollback-safe path, with matching occurrences in the listed migration files. Use the database's online or concurrent option where supported, or document why the operation is safe for the target table size and rollback plan.
Suggested Code:
// Use an online migration strategy for index creation on large tables, or document table size/rollback plan.
Execute.Sql("CREATE NONCLUSTERED INDEX IX_RmsOperationalRecords_Department_Call ON RmsOperationalRecords (DepartmentId, CallId) WHERE CallId IS NOT NULL;");
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Execute.Sql("CREATE UNIQUE NONCLUSTERED INDEX UX_RmsOperationalRecords_Department_DraftReference ON RmsOperationalRecords (DepartmentId, DraftReference);"); | ||
| } | ||
|
|
||
| if (!Schema.Table("RmsOperationalRecordDetails").Exists()) |
There was a problem hiding this comment.
Missing audit-path requirement in Providers/Resgrid.Providers.Migrations/Migrations/M0150_AddRmsRecordsCore.cs because the RmsOperationalRecordDetails table appears to store ePHI-sensitive operational record data without a corresponding append-only access audit mechanism in the surrounding application flows. Ensure reads and writes emit immutable audit entries with user id, subject id when applicable, action, purpose-of-use, timestamp, and request id.
Kody rule violation: Write immutable audit logs for all ePHI access
// ePHI-bearing records should be paired with append-only audit logging in application flows.
if (!Schema.Table("RmsOperationalRecordDetails").Exists())Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0150_AddRmsRecordsCore.cs:
Line 92:
Missing audit-path requirement in Providers/Resgrid.Providers.Migrations/Migrations/M0150_AddRmsRecordsCore.cs because the RmsOperationalRecordDetails table appears to store ePHI-sensitive operational record data without a corresponding append-only access audit mechanism in the surrounding application flows. Ensure reads and writes emit immutable audit entries with user id, subject id when applicable, action, purpose-of-use, timestamp, and request id.
Suggested Code:
// ePHI-bearing records should be paired with append-only audit logging in application flows.
if (!Schema.Table("RmsOperationalRecordDetails").Exists())
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| Execute.Sql( | ||
| "IF NOT EXISTS (SELECT 1 FROM [FeatureFlags] WHERE [FlagKey] = '" + flag[0] + "') " + | ||
| "INSERT INTO [FeatureFlags] ([FlagKey], [Name], [Description], [Category], [IsEnabledGlobally]) " + |
There was a problem hiding this comment.
SQL injection pattern in Providers/Resgrid.Providers.Migrations/Migrations/M0152_SeedRecordsFeatureFlags.cs because the INSERT INTO [FeatureFlags] statement is built as raw SQL text, with matching occurrences in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0152_SeedRecordsFeatureFlagsPg.cs:28-28 and :29-29. Use parameterized SQL or migration APIs so values are not concatenated into the query.
Kody rule violation: Prevent SQL Injection in Queries
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0152_SeedRecordsFeatureFlags.cs:
Line 33:
SQL injection pattern in Providers/Resgrid.Providers.Migrations/Migrations/M0152_SeedRecordsFeatureFlags.cs because the INSERT INTO [FeatureFlags] statement is built as raw SQL text, with matching occurrences in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0152_SeedRecordsFeatureFlagsPg.cs:28-28 and :29-29. Use parameterized SQL or migration APIs so values are not concatenated into the query.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| Create.Table("RmsDepartmentCutoverEvents") | ||
| .WithColumn("RmsDepartmentCutoverEventId").AsInt32().NotNullable().PrimaryKey().Identity() | ||
| .WithColumn("DepartmentId").AsInt32().NotNullable() |
There was a problem hiding this comment.
Potential missing single-column index in Providers/Resgrid.Providers.Migrations/Migrations/M0154_AddRmsDepartmentCutover.cs because DepartmentId is likely filtered or joined independently even though only a composite index is added later, with a matching occurrence in Providers/Resgrid.Providers.Migrations/Migrations/M0172_AddDepartmentProfileMediaAndBranding.cs:42-42. Verify query plans and add a dedicated DepartmentId index if standalone filtering is part of the workload.
Kody rule violation: Add database indexes for query optimization
.WithColumn("DepartmentId").AsInt32().NotNullable()
// add an index for frequent filtering/joining on DepartmentIdPrompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0154_AddRmsDepartmentCutover.cs:
Line 45:
Potential missing single-column index in Providers/Resgrid.Providers.Migrations/Migrations/M0154_AddRmsDepartmentCutover.cs because DepartmentId is likely filtered or joined independently even though only a composite index is added later, with a matching occurrence in Providers/Resgrid.Providers.Migrations/Migrations/M0172_AddDepartmentProfileMediaAndBranding.cs:42-42. Verify query plans and add a dedicated DepartmentId index if standalone filtering is part of the workload.
Suggested Code:
.WithColumn("DepartmentId").AsInt32().NotNullable()
// add an index for frequent filtering/joining on DepartmentId
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| Execute.Sql( | ||
| "INSERT INTO featureflags (flagkey, name, description, category, isenabledglobally) " + | ||
| "SELECT '" + flag[0] + "', '" + flag[1] + "', '" + flag[2] + "', '" + flag[3] + "', false " + |
There was a problem hiding this comment.
SQL concatenation in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0152_SeedRecordsFeatureFlagsPg.cs because "SELECT '" + flag[0] + "', '" + flag[1] + "', '" + flag[2] + "', '" + flag[3] + "', false " interpolates values into SQL text, with a matching occurrence at :30-30. Use parameterized placeholders such as @key, @name, @description, and @category instead of string concatenation.
Kody rule violation: Always sanitize user inputs
"SELECT @key, @name, @description, @category, false " +Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0152_SeedRecordsFeatureFlagsPg.cs:
Line 29:
SQL concatenation in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0152_SeedRecordsFeatureFlagsPg.cs because "SELECT '" + flag[0] + "', '" + flag[1] + "', '" + flag[2] + "', '" + flag[3] + "', false " interpolates values into SQL text, with a matching occurrence at :30-30. Use parameterized placeholders such as @key, @name, @description, and @category instead of string concatenation.
Suggested Code:
"SELECT @key, @name, @description, @category, false " +
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
| catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) | ||
| { | ||
| Logging.LogError($"ClamAV scan of '{fileName}' timed out after {AttachmentScanningConfig.TimeoutSeconds} seconds ({AttachmentScanningConfig.Host}:{AttachmentScanningConfig.Port})."); |
There was a problem hiding this comment.
Unstructured error logging in Providers/Resgrid.Providers.Scanning/ClamAvAttachmentScanner.cs because Logging.LogError($"ClamAV scan of '{fileName}' timed out after {AttachmentScanningConfig.TimeoutSeconds} seconds ({AttachmentScanningConfig.Host}:{AttachmentScanningConfig.Port}).") encodes operation context into a message string only. Emit structured fields such as operation="ClamAvScan", fileName, host, port, timeoutSeconds, and the error type so failures remain filterable and correlatable.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File Providers/Resgrid.Providers.Scanning/ClamAvAttachmentScanner.cs:
Line 43:
Unstructured error logging in Providers/Resgrid.Providers.Scanning/ClamAvAttachmentScanner.cs because Logging.LogError($"ClamAV scan of '{fileName}' timed out after {AttachmentScanningConfig.TimeoutSeconds} seconds ({AttachmentScanningConfig.Host}:{AttachmentScanningConfig.Port}).") encodes operation context into a message string only. Emit structured fields such as operation="ClamAvScan", fileName, host, port, timeoutSeconds, and the error type so failures remain filterable and correlatable.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (scope.CreatedOn == default) | ||
| scope.CreatedOn = DateTime.UtcNow; | ||
|
|
||
| await InsertAsync(scope, cancellationToken, true); |
There was a problem hiding this comment.
N+1 database write pattern in Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs because await InsertAsync(scope, cancellationToken, true); executes inside a loop, with matching occurrences in Core/Resgrid.Services/Records/RecordsService.cs:1119-1120 and :1149-1149. Batch the scopes into a single InsertManyAsync or another aggregated write operation.
Kody rule violation: Detect N+1 style queries and suggest batching
// Batch inserts or use a single multi-row write instead of awaiting one DB call per scope
await InsertManyAsync(scopes, cancellationToken);Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs:
Line 787:
N+1 database write pattern in Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs because await InsertAsync(scope, cancellationToken, true); executes inside a loop, with matching occurrences in Core/Resgrid.Services/Records/RecordsService.cs:1119-1120 and :1149-1149. Batch the scopes into a single InsertManyAsync or another aggregated write operation.
Suggested Code:
// Batch inserts or use a single multi-row write instead of awaiting one DB call per scope
await InsertManyAsync(scopes, cancellationToken);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| return await work(UnitOfWork.CreateOrGetConnection()); | ||
| } | ||
| catch (Exception ex) |
There was a problem hiding this comment.
Overbroad exception handling in Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs because catch (Exception ex) wraps database operations without classifying provider-specific failures. Catch DbException explicitly, distinguish transient from non-transient errors, add database context, and only then fall back to a general exception handler.
Kody rule violation: Implement proper database error checking
catch (DbException ex)
{
// add context, classify transient vs non-transient, and rethrow/handle appropriately
throw;
}
catch (Exception ex)Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs:
Line 107:
Overbroad exception handling in Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs because catch (Exception ex) wraps database operations without classifying provider-specific failures. Catch DbException explicitly, distinguish transient from non-transient errors, add database context, and only then fall back to a general exception handler.
Suggested Code:
catch (DbException ex)
{
// add context, classify transient vs non-transient, and rethrow/handle appropriately
throw;
}
catch (Exception ex)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| public void Dispose() | ||
| { | ||
| try { _listener.Stop(); } catch { } |
There was a problem hiding this comment.
Exception suppression in Tests/Resgrid.Tests/Providers/ClamAvAttachmentScannerTests.cs hides failures from _listener.Stop() and makes test cleanup behavior unverifiable. Log the exception with cleanup context and either rethrow it or handle it explicitly.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File Tests/Resgrid.Tests/Providers/ClamAvAttachmentScannerTests.cs:
Line 272:
Exception suppression in Tests/Resgrid.Tests/Providers/ClamAvAttachmentScannerTests.cs hides failures from _listener.Stop() and makes test cleanup behavior unverifiable. Log the exception with cleanup 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.
| _neverReply = neverReply; | ||
| _listener = new TcpListener(IPAddress.Loopback, 0); | ||
| _listener.Start(); | ||
| _ = Task.Run(ServeAsync); |
There was a problem hiding this comment.
Async work in constructor in Tests/Resgrid.Tests/Providers/ClamAvAttachmentScannerTests.cs because _ = Task.Run(ServeAsync); starts background execution during construction and prevents deterministic awaiting and failure handling. Move ServeAsync startup into an explicit StartAsync or InitializeAsync method.
Kody rule violation: Avoid asynchronous operations in constructors
// start async serving from an explicit StartAsync/InitializeAsync method instead of the constructorPrompt for LLM
File Tests/Resgrid.Tests/Providers/ClamAvAttachmentScannerTests.cs:
Line 219:
Async work in constructor in Tests/Resgrid.Tests/Providers/ClamAvAttachmentScannerTests.cs because _ = Task.Run(ServeAsync); starts background execution during construction and prevents deterministic awaiting and failure handling. Move ServeAsync startup into an explicit StartAsync or InitializeAsync method.
Suggested Code:
// start async serving from an explicit StartAsync/InitializeAsync method instead of the constructor
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// <summary>The call export link carries an encrypted token that is not stable across runs; nothing else varies.</summary> | ||
| private static string Stable(string html) | ||
| { | ||
| return Regex.Replace(html, @"query=[^""&]*", "query=TOKEN"); |
There was a problem hiding this comment.
Regular expression denial-of-service risk in Tests/Resgrid.Tests/Providers/DepartmentEmailBrandingTemplateTests.cs because Regex.Replace(html, @"query=[^""&]*", "query=TOKEN") executes without a timeout on potentially untrusted input, with matching occurrences in Tests/Resgrid.Tests/Rms/ClaimsAuthorizationHelperParityTests.cs:88-88, Tests/Resgrid.Tests/Rms/SystemApiKeyRecordPolicyTests.cs:47-47, and :63-63. Specify an explicit regex timeout.
Kody rule violation: Specify Timeout for Regular Expressions
Prompt for LLM
File Tests/Resgrid.Tests/Providers/DepartmentEmailBrandingTemplateTests.cs:
Line 67:
Regular expression denial-of-service risk in Tests/Resgrid.Tests/Providers/DepartmentEmailBrandingTemplateTests.cs because Regex.Replace(html, @"query=[^""&]*", "query=TOKEN") executes without a timeout on potentially untrusted input, with matching occurrences in Tests/Resgrid.Tests/Rms/ClaimsAuthorizationHelperParityTests.cs:88-88, Tests/Resgrid.Tests/Rms/SystemApiKeyRecordPolicyTests.cs:47-47, and :63-63. Specify an explicit regex timeout.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Postmark automatically inlines all CSS properties for maximum email client | ||
| compatibility. You can just update styles here, and Postmark does the rest. | ||
| --> | ||
| <style type="text/css" rel="stylesheet" media="all"> |
There was a problem hiding this comment.
Style-scope leakage in Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/ReportDelivery.html because <style type="text/css" rel="stylesheet" media="all"> introduces a global style block, with matching occurrences in Web/Resgrid.Web/Areas/User/Views/Records/Print.cshtml:20-20, PrintDiff.cshtml:12-12, Diff.cshtml:19-19, Workflows/New.cshtml:68-68, and Diff.cshtml:53-54. Scope selectors to the template or component so they do not leak beyond this markup.
Kody rule violation: Use component-scoped styling
Prompt for LLM
File Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/ReportDelivery.html:
Line 12:
Style-scope leakage in Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/ReportDelivery.html because <style type="text/css" rel="stylesheet" media="all"> introduces a global style block, with matching occurrences in Web/Resgrid.Web/Areas/User/Views/Records/Print.cshtml:20-20, PrintDiff.cshtml:12-12, Diff.cshtml:19-19, Workflows/New.cshtml:68-68, and Diff.cshtml:53-54. Scope selectors to the template or component so they do not leak beyond this markup.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| (await _service.SendReminderAsync(Dept, "chief", "b2")).Reason.Should().Be(RecordsReminderResult.ReasonNotOpen); | ||
|
|
||
| for (var i = 0; i < 30; i++) |
There was a problem hiding this comment.
False positive in Tests/Resgrid.Tests/Rms/RecordsAccountabilityServiceTests.cs because for (var i = 0; i < 30; i++) uses < rather than == or != in the loop termination condition. Do not flag this line under rule 19.
Kody rule violation: Avoid equality operators in loop termination conditions
for (var i = 0; i < 30; i++)Prompt for LLM
File Tests/Resgrid.Tests/Rms/RecordsAccountabilityServiceTests.cs:
Line 182:
False positive in Tests/Resgrid.Tests/Rms/RecordsAccountabilityServiceTests.cs because for (var i = 0; i < 30; i++) uses < rather than == or != in the loop termination condition. Do not flag this line under rule 19.
Suggested Code:
for (var i = 0; i < 30; i++)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| private static Dictionary<string, string> Load(string path) | ||
| { | ||
| return XDocument.Load(path) |
There was a problem hiding this comment.
Unhandled external IO and XML parsing in Tests/Resgrid.Tests/Rms/RecordsPermissionRowsTests.cs because XDocument.Load(path) can fail on file access or malformed XML across the listed call sites. Wrap the load in contextual exception handling and rethrow an InvalidOperationException that includes the path and operation context.
Kody rule violation: Add try-catch blocks for external calls
try
{
return XDocument.Load(path)
.Root!
.Elements("data")
.ToDictionary(
x => (string)x.Attribute("name")!,
x => (string)x.Element("value") ?? string.Empty);
}
catch (IOException ex)
{
throw new InvalidOperationException($"Failed to load resx file: {path}", ex);
}
catch (System.Xml.XmlException ex)
{
throw new InvalidOperationException($"Invalid XML in resx file: {path}", ex);
}Prompt for LLM
File Tests/Resgrid.Tests/Rms/RecordsPermissionRowsTests.cs:
Line 154:
Unhandled external IO and XML parsing in Tests/Resgrid.Tests/Rms/RecordsPermissionRowsTests.cs because XDocument.Load(path) can fail on file access or malformed XML across the listed call sites. Wrap the load in contextual exception handling and rethrow an InvalidOperationException that includes the path and operation context.
Suggested Code:
try
{
return XDocument.Load(path)
.Root!
.Elements("data")
.ToDictionary(
x => (string)x.Attribute("name")!,
x => (string)x.Element("value") ?? string.Empty);
}
catch (IOException ex)
{
throw new InvalidOperationException($"Failed to load resx file: {path}", ex);
}
catch (System.Xml.XmlException ex)
{
throw new InvalidOperationException($"Invalid XML in resx file: {path}", ex);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| Narrative = "Scene narrative", | ||
| Cause = "Natural", | ||
| ContactName = "Jane Doe", |
There was a problem hiding this comment.
Personal data in test and fixture diagnostics in Tests/Resgrid.Tests/Rms/RecordsWorkflowTriggerTests.cs because ContactName = "Jane Doe" stores a raw name, with matching occurrences in Web/Resgrid.Web/Areas/User/Models/Records/RecordsListExport.cs:111-111, Core/Resgrid.Services/Records/RecordsCutoverService.cs:119-119, Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/Message.html:452-452, Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/Call.html:420-420, and Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs:1407-1407. Replace it with a hashed or tokenized surrogate unless the test explicitly validates name handling.
Kody rule violation: Redact PII in logs and metrics by default
Prompt for LLM
File Tests/Resgrid.Tests/Rms/RecordsWorkflowTriggerTests.cs:
Line 165:
Personal data in test and fixture diagnostics in Tests/Resgrid.Tests/Rms/RecordsWorkflowTriggerTests.cs because ContactName = "Jane Doe" stores a raw name, with matching occurrences in Web/Resgrid.Web/Areas/User/Models/Records/RecordsListExport.cs:111-111, Core/Resgrid.Services/Records/RecordsCutoverService.cs:119-119, Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/Message.html:452-452, Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/Call.html:420-420, and Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs:1407-1407. Replace it with a hashed or tokenized surrogate unless the test explicitly validates name handling.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!typeof(IEnumerable).IsAssignableFrom(propertyType)) | ||
| return false; | ||
|
|
||
| var elementType = propertyType.IsGenericType ? propertyType.GetGenericArguments().FirstOrDefault() : propertyType.GetElementType(); |
There was a problem hiding this comment.
Ambiguous generic argument access in Tests/Resgrid.Tests/Rms/RmsCascadeSafetyTests.cs because propertyType.IsGenericType guarantees GetGenericArguments() for a constructed generic type, so FirstOrDefault() implies an empty case that should not occur. Use First() to reflect that invariant more accurately.
Kody rule violation: Use `First`/`Single` Instead of `FirstOrDefault`/`SingleOrDefault` for Non-Empty Collections
var elementType = propertyType.IsGenericType ? propertyType.GetGenericArguments().First() : propertyType.GetElementType();Prompt for LLM
File Tests/Resgrid.Tests/Rms/RmsCascadeSafetyTests.cs:
Line 77:
Ambiguous generic argument access in Tests/Resgrid.Tests/Rms/RmsCascadeSafetyTests.cs because propertyType.IsGenericType guarantees GetGenericArguments() for a constructed generic type, so FirstOrDefault() implies an empty case that should not occur. Use First() to reflect that invariant more accurately.
Suggested Code:
var elementType = propertyType.IsGenericType ? propertyType.GetGenericArguments().First() : propertyType.GetElementType();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!ClaimsAuthorizationHelper.CanViewRestrictedRecords() && snapshot.Details != null) | ||
| { | ||
| foreach (var field in RecordSnapshotSerializer.RestrictedDetailFields) | ||
| typeof(RmsOperationalRecordDetail).GetProperty(field)?.SetValue(snapshot.Details, null); |
There was a problem hiding this comment.
Reflection injection risk in Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs because typeof(RmsOperationalRecordDetail).GetProperty(field)?.SetValue(snapshot.Details, null); resolves a property name from field at runtime. Validate field against a strict allowlist or replace the reflective setter with explicit property handling.
Kody rule violation: Prevent Reflection Injection Attacks
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs:
Line 594:
Reflection injection risk in Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs because typeof(RmsOperationalRecordDetail).GetProperty(field)?.SetValue(snapshot.Details, null); resolves a property name from field at runtime. Validate field against a strict allowlist or replace the reflective setter with explicit property handling.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| format = JsonFormat, | ||
| exportedOn = DateTime.UtcNow, | ||
| exportedByUserId, |
There was a problem hiding this comment.
Personal data export in Web/Resgrid.Web/Areas/User/Models/Records/RecordsListExport.cs because exportedByUserId emits a raw user identifier into serialized metadata, with matching occurrences in Tests/Resgrid.Tests/Rms/RecordsWorkflowTriggerTests.cs:165-165, Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/Call.html:420-420, Web/Resgrid.Web/Areas/User/Views/Records/Edit.cshtml:44-44, Core/Resgrid.Services/Records/RecordsService.cs:306-306, Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs:1407-1407, and Core/Resgrid.Services/Records/RecordsCutoverService.cs:119-119. Omit the raw ID or replace it with a stable pseudonymized token or hash.
Kody rule violation: Mask PII and secrets in logs
// omit raw user id or replace with a stable non-PII token/hashPrompt for LLM
File Web/Resgrid.Web/Areas/User/Models/Records/RecordsListExport.cs:
Line 111:
Personal data export in Web/Resgrid.Web/Areas/User/Models/Records/RecordsListExport.cs because exportedByUserId emits a raw user identifier into serialized metadata, with matching occurrences in Tests/Resgrid.Tests/Rms/RecordsWorkflowTriggerTests.cs:165-165, Tests/Resgrid.Tests/Providers/Fixtures/EmailBaseline/Call.html:420-420, Web/Resgrid.Web/Areas/User/Views/Records/Edit.cshtml:44-44, Core/Resgrid.Services/Records/RecordsService.cs:306-306, Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs:1407-1407, and Core/Resgrid.Services/Records/RecordsCutoverService.cs:119-119. Omit the raw ID or replace it with a stable pseudonymized token or hash.
Suggested Code:
// omit raw user id or replace with a stable non-PII token/hash
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| @foreach (var row in Model.Preview.PermissionMapping) |
There was a problem hiding this comment.
Implicit typing and enumeration safety issue in Web/Resgrid.Web/Areas/User/Views/Records/Activate.cshtml because @foreach (var row in Model.Preview.PermissionMapping) uses var and assumes PermissionMapping is non-null. Use an explicit, descriptive loop variable name and null-guard the sequence before enumeration.
Kody rule violation: A module should not import itself
@foreach (var permissionRow in Model.Preview?.PermissionMapping ?? Enumerable.Empty<object>())Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Records/Activate.cshtml:
Line 67:
Implicit typing and enumeration safety issue in Web/Resgrid.Web/Areas/User/Views/Records/Activate.cshtml because @foreach (var row in Model.Preview.PermissionMapping) uses var and assumes PermissionMapping is non-null. Use an explicit, descriptive loop variable name and null-guard the sequence before enumeration.
Suggested Code:
@foreach (var permissionRow in Model.Preview?.PermissionMapping ?? Enumerable.Empty<object>())
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <tbody> | ||
| @for (var i = 0; i < Model.AvailableUnits.Count; i++) | ||
| { | ||
| var unitId = int.Parse(Model.AvailableUnits[i].Value); |
There was a problem hiding this comment.
Format exception risk in Web/Resgrid.Web/Areas/User/Views/Records/Edit.cshtml because int.Parse(Model.AvailableUnits[i].Value) assumes valid user or IO input. Use a TryParse-style conversion and validate the expected culture or format before accepting the value.
Kody rule violation: Use TryParse for string conversions
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Records/Edit.cshtml:
Line 223:
Format exception risk in Web/Resgrid.Web/Areas/User/Views/Records/Edit.cshtml because int.Parse(Model.AvailableUnits[i].Value) assumes valid user or IO input. Use a TryParse-style conversion and validate the expected culture or format before accepting the value.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| @if (isCoroner) | ||
| { | ||
| <div class="section restricted"> |
There was a problem hiding this comment.
Missing consent gate in Web/Resgrid.Web/Areas/User/Views/Records/PrintRevision.cshtml because
Kody rule violation: Require explicit consent before processing sensitive data
<div class="section restricted" data-consent-required="true">Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Records/PrintRevision.cshtml:
Line 85:
Missing consent gate in Web/Resgrid.Web/Areas/User/Views/Records/PrintRevision.cshtml because <div class="section restricted"> marks a sensitive restricted coroner section without evidence that a consent record is verified, with a matching occurrence in Web/Resgrid.Web/Areas/User/Views/Records/Details.cshtml:152-152. Gate the section on a valid consent check and propagate the consent identifier through the request or model.
Suggested Code:
<div class="section restricted" data-consent-required="true">
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| <span>· @localizer["Revision"] @Model.RevisionNumber</span> | ||
| } | ||
| <span>· @localizer["PrintedBy"]: @Model.PrintedByName</span> |
There was a problem hiding this comment.
PII exposure in Web/Resgrid.Web/Areas/User/Views/Records/_PrintFooter.cshtml because @Model.PrintedByName renders a personal identifier in a records footer, with similar occurrences in the listed record views, migrations, tests, and templates. Replace it with minimized non-identifying metadata or route identity display through the required audited access path.
Kody rule violation: Do not log PHI; mask and drop sensitive fields
@* Avoid rendering/logging identifying PHI/PII unless strictly required and authorized; prefer non-identifying metadata or a tokenized user reference. *@Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Records/_PrintFooter.cshtml:
Line 19:
PII exposure in Web/Resgrid.Web/Areas/User/Views/Records/_PrintFooter.cshtml because @Model.PrintedByName renders a personal identifier in a records footer, with similar occurrences in the listed record views, migrations, tests, and templates. Replace it with minimized non-identifying metadata or route identity display through the required audited access path.
Suggested Code:
@* Avoid rendering/logging identifying PHI/PII unless strictly required and authorized; prefer non-identifying metadata or a tokenized user reference. *@
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var eventTypes = Enum.GetValues(typeof(Resgrid.Model.WorkflowTriggerEventType)) | ||
| .Cast<Resgrid.Model.WorkflowTriggerEventType>() | ||
| .Where(e => !usedEventTypes.Contains((int)e)) | ||
| .Where(e => recordsTriggersAvailable || !Resgrid.Model.WorkflowTriggerEventTypes.IsRecordsTrigger(e)) |
There was a problem hiding this comment.
Logic change in Web/Resgrid.Web/Areas/User/Views/Workflows/New.cshtml because replacing .Where(e => recordsTriggersAvailable || !Resgrid.Model.WorkflowTriggerEventTypes.IsRecordsTrigger(e)) with two chained Where calls changes semantics and filters out all results when recordsTriggersAvailable is false, with a matching occurrence in Tests/Resgrid.Tests/Rms/RecordsGroupScopePreviewTests.cs:70-70. Preserve the original predicate or extract it into a named expression without altering behavior.
Kody rule violation: Limit Lengthy LINQ Chains
.Where(e => recordsTriggersAvailable)
.Where(e => !Resgrid.Model.WorkflowTriggerEventTypes.IsRecordsTrigger(e))Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Workflows/New.cshtml:
Line 11:
Logic change in Web/Resgrid.Web/Areas/User/Views/Workflows/New.cshtml because replacing .Where(e => recordsTriggersAvailable || !Resgrid.Model.WorkflowTriggerEventTypes.IsRecordsTrigger(e)) with two chained Where calls changes semantics and filters out all results when recordsTriggersAvailable is false, with a matching occurrence in Tests/Resgrid.Tests/Rms/RecordsGroupScopePreviewTests.cs:70-70. Preserve the original predicate or extract it into a named expression without altering behavior.
Suggested Code:
.Where(e => recordsTriggersAvailable)
.Where(e => !Resgrid.Model.WorkflowTriggerEventTypes.IsRecordsTrigger(e))
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @@ -24,8 +26,10 @@ var resgrid; | |||
| } | |||
| }); | |||
| $(selector).on('change', function () { | |||
| var url = resgrid.absoluteBaseUrl + '/User/Security/SetPermissionData?type=' + permType + '&data=' + encodeURIComponent(($(selector).val() || []).join(',')); | |||
There was a problem hiding this comment.
Unhandled async failure in Web/Resgrid.Web/wwwroot/js/app/internal/security/resgrid.security.permissions.js because the AJAX request in this permission update flow has no rejection handling, which can surface failed writes as unhandled async errors across the listed occurrences. Await the request inside try/catch or attach a .fail/.catch handler and log contextual fields such as op: 'SetPermissionData', permType, and selector.
Kody rule violation: Handle async operations with proper error handling
var url = resgrid.absoluteBaseUrl + '/User/Security/SetPermissionData?type=' + permType + '&data=' + encodeURIComponent(($(selector).val() || []).join(','));
try {
await $.ajax({
url: url + (typeof lockValue === 'function' ? '&lockToGroup=' + lockValue() : ''),
type: 'POST',
headers: { 'RequestVerificationToken': antiForgeryToken() }
});
} catch (err) {
logger.error('set permission data failed', { op: 'SetPermissionData', permType: permType, selector: selector, err: err });
}Prompt for LLM
File Web/Resgrid.Web/wwwroot/js/app/internal/security/resgrid.security.permissions.js:
Line 29:
Unhandled async failure in Web/Resgrid.Web/wwwroot/js/app/internal/security/resgrid.security.permissions.js because the AJAX request in this permission update flow has no rejection handling, which can surface failed writes as unhandled async errors across the listed occurrences. Await the request inside try/catch or attach a .fail/.catch handler and log contextual fields such as op: 'SetPermissionData', permType, and selector.
Suggested Code:
var url = resgrid.absoluteBaseUrl + '/User/Security/SetPermissionData?type=' + permType + '&data=' + encodeURIComponent(($(selector).val() || []).join(','));
try {
await $.ajax({
url: url + (typeof lockValue === 'function' ? '&lockToGroup=' + lockValue() : ''),
type: 'POST',
headers: { 'RequestVerificationToken': antiForgeryToken() }
});
} catch (err) {
logger.error('set permission data failed', { op: 'SetPermissionData', permType: permType, selector: selector, err: err });
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
3 participants
Summary
This PR delivers the first major pass of the new Records (RMS) module as the successor to legacy Logs, including department activation/cutover, record lifecycle management, revisions, search, permissions, workflows, reporting, attachment hygiene/scanning, and department branding for records and operational emails.
What changed
Added the new Records module foundation
IRecordsServiceand related services for:Added department-level Records activation and cutover
Added Records permissions and claims
Added Records UI in the web app
RecordsControllerand Records views for:Added immutable revision history and diff support
Added Records search infrastructure
Added workflow integration for Records
LogAddedworkflows can continue firing for eligible finalized Records during migration.Added reporting and accountability across cutover
Added attachment hygiene and malware scanning
Added department profile branding and email mastheads
Added persistence and worker support
Added localization and tests
Functional impact
This PR establishes the first production-ready pass of the Records/RMS platform inside Resgrid, allowing departments to begin transitioning from legacy Logs to a new record-based system with stronger auditing, lifecycle control, search, branding, permissions, and workflow integration, while preserving read-only access to historical Logs after cutover.