Conversation
|
Important Review skippedToo many files! This PR contains 170 files, which is 70 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (60)
📒 Files selected for processing (170)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Thanks for opening this, but we'd appreciate a little more information. Could you update it with more details? |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
| /// subdomains ("example.org, .ciffc.ca"). Empty allows any public host: the address checks below still | ||
| /// apply, so an empty list is not the same as no restriction. | ||
| /// </summary> | ||
| public static string AllowedFeedHosts = ""; |
There was a problem hiding this comment.
Mutable configuration state in Core/Resgrid.Config/RecordsConnectorConfig.cs: AllowedFeedHosts appears to be configuration metadata initialized once but remains reassignable. Mark it readonly to communicate immutability and prevent accidental mutation.
Kody rule violation: Use `readonly` or `const` for Immutable Data
public static readonly string AllowedFeedHosts = string.Empty;Prompt for LLM
File Core/Resgrid.Config/RecordsConnectorConfig.cs:
Line 37:
Mutable configuration state in Core/Resgrid.Config/RecordsConnectorConfig.cs: AllowedFeedHosts appears to be configuration metadata initialized once but remains reassignable. Mark it readonly to communicate immutability and prevent accidental mutation.
Suggested Code:
public static readonly string AllowedFeedHosts = string.Empty;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <data name="CandidateLinked" xml:space="preserve"><value>Candidato collegato.</value></data> | ||
| <data name="CandidateRejected" xml:space="preserve"><value>Candidato rifiutato.</value></data> | ||
| <data name="Candidates" xml:space="preserve"><value>Candidati</value></data> | ||
| <data name="CaseAccessNotice" xml:space="preserve"><value>L'accesso a questo caso è limitato ai suoi membri e ogni lettura viene registrata.</value></data> |
There was a problem hiding this comment.
Audit wording mismatch in Core/Resgrid.Localization/Areas/User/Records/Records.it.resx and the related locations listed: CaseAccessNotice promises that every read is recorded, but the stated requirement is an append-only immutable audit log for ePHI access. Tighten the text to immutable audit logging and verify the implementation enforces that stronger guarantee.
Kody rule violation: Write immutable audit logs for all ePHI access
<data name="CaseAccessNotice" xml:space="preserve"><value>L'accesso a questo caso è limitato ai suoi membri e ogni lettura genera un audit record immutabile.</value></data>Prompt for LLM
File Core/Resgrid.Localization/Areas/User/Records/Records.it.resx:
Line 1461:
Audit wording mismatch in Core/Resgrid.Localization/Areas/User/Records/Records.it.resx and the related locations listed: CaseAccessNotice promises that every read is recorded, but the stated requirement is an append-only immutable audit log for ePHI access. Tighten the text to immutable audit logging and verify the implementation enforces that stronger guarantee.
Suggested Code:
<data name="CaseAccessNotice" xml:space="preserve"><value>L'accesso a questo caso è limitato ai suoi membri e ogni lettura genera un audit record immutabile.</value></data>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <data name="AllPrograms" xml:space="preserve"><value>Tutti i programmi</value></data> | ||
| <data name="AmendmentsRecommended" xml:space="preserve"><value>Modifiche consigliate</value></data> | ||
| <data name="Applicant" xml:space="preserve"><value>Richiedente</value></data> | ||
| <data name="ApplicantEmail" xml:space="preserve"><value>E-mail del richiedente</value></data> |
There was a problem hiding this comment.
PII labeling risk in Core/Resgrid.Localization/Areas/User/Records/Records.it.resx and the related locations listed: ApplicantEmail names raw email directly, which conflicts with default redaction or hashing requirements for telemetry and diagnostics. Prefer terminology and downstream usage that indicate hashed or redacted representations.
Kody rule violation: Redact PII in logs and metrics by default
<data name="ApplicantEmail" xml:space="preserve"><value>Hash e-mail richiedente</value></data>Prompt for LLM
File Core/Resgrid.Localization/Areas/User/Records/Records.it.resx:
Line 1440:
PII labeling risk in Core/Resgrid.Localization/Areas/User/Records/Records.it.resx and the related locations listed: ApplicantEmail names raw email directly, which conflicts with default redaction or hashing requirements for telemetry and diagnostics. Prefer terminology and downstream usage that indicate hashed or redacted representations.
Suggested Code:
<data name="ApplicantEmail" xml:space="preserve"><value>Hash e-mail richiedente</value></data>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| /// <summary>Address folding shared by the crosswalk inventory and the occupancy master so both sides match the same way.</summary> | ||
| public static class AddressNormalizer | ||
| { | ||
| private static readonly Regex NonAlphanumeric = new Regex("[^A-Z0-9 ]", RegexOptions.Compiled); |
There was a problem hiding this comment.
Regex DoS risk in Core/Resgrid.Model/Records/RecordsPreventionContracts.cs and the related locations listed: NonAlphanumeric uses new Regex("[^A-Z0-9 ]", RegexOptions.Compiled) without a timeout, allowing untrusted input to consume unbounded processing time. Define an explicit regex timeout.
Kody rule violation: Specify Timeout for Regular Expressions
Prompt for LLM
File Core/Resgrid.Model/Records/RecordsPreventionContracts.cs:
Line 60:
Regex DoS risk in Core/Resgrid.Model/Records/RecordsPreventionContracts.cs and the related locations listed: NonAlphanumeric uses new Regex("[^A-Z0-9 ]", RegexOptions.Compiled) without a timeout, allowing untrusted input to consume unbounded processing time. Define an explicit regex timeout.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| auditEvent.UserAgent = userAgent; | ||
| var auditEvent = NewAuditEvent(departmentId, userId, AuditLogTypes.ContactRemoved, ipAddress, userAgent); | ||
|
|
||
| var contact = await _contactsRepository.GetByIdAsync(contactId); |
There was a problem hiding this comment.
Missing service-level exception context in Core/Resgrid.Services/ContactsService.cs and the related locations listed: await _contactsRepository.GetByIdAsync(contactId) performs external data access without a surrounding try/catch, so repository failures bubble up without operation context. Catch exceptions around the repository call, annotate them with service context, and map or rethrow deliberately.
Kody rule violation: Add try-catch blocks for external calls
try
{
var contact = await _contactsRepository.GetByIdAsync(contactId);
}
catch (Exception ex)
{
// add context/logging and map to an application-level error
throw;
}Prompt for LLM
File Core/Resgrid.Services/ContactsService.cs:
Line 225:
Missing service-level exception context in Core/Resgrid.Services/ContactsService.cs and the related locations listed: await _contactsRepository.GetByIdAsync(contactId) performs external data access without a surrounding try/catch, so repository failures bubble up without operation context. Catch exceptions around the repository call, annotate them with service context, and map or rethrow deliberately.
Suggested Code:
try
{
var contact = await _contactsRepository.GetByIdAsync(contactId);
}
catch (Exception ex)
{
// add context/logging and map to an application-level error
throw;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| foreach (var contactId in (contactIds ?? Enumerable.Empty<string>()).Where(x => !string.IsNullOrWhiteSpace(x)).Distinct()) | ||
| { | ||
| var notes = await _contactNotesRepository.GetContactNotesByContactIdAsync(contactId); | ||
| result[contactId] = (notes ?? Enumerable.Empty<ContactNote>()) | ||
| .Where(x => IsLiveAlertNote(x, departmentId, now)) | ||
| .ToList(); | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
N+1 query pattern in Core/Resgrid.Services/ContactsService.cs: GetAlertNotesByContactIdsAsync and LoadContactsAsync issue one repository call per distinct contact, adding O(contacts) round-trips to GetActiveCalls and GetCall on top of the existing batched reads. Batch contact and alert-note retrieval with repository methods such as GetContactNotesByContactIdsAsync and GetByIdsAsync so each request uses one query per dataset.
public async Task<Dictionary<string, List<ContactNote>>> GetAlertNotesByContactIdsAsync(int departmentId, IEnumerable<string> contactIds)
{
var ids = (contactIds ?? Enumerable.Empty<string>()).Where(x => !string.IsNullOrWhiteSpace(x)).Distinct().ToList();
if (!ids.Any())
return new Dictionary<string, List<ContactNote>>();
var now = DateTime.UtcNow;
var notes = await _contactNotesRepository.GetContactNotesByContactIdsAsync(departmentId, ids);
return (notes ?? Enumerable.Empty<ContactNote>())
.Where(x => IsLiveAlertNote(x, departmentId, now))
.GroupBy(x => x.ContactId)
.ToDictionary(g => g.Key, g => g.ToList());
}
private async Task<Dictionary<string, Contact>> LoadContactsAsync(int departmentId, IEnumerable<string> contactIds)
{
var ids = contactIds.Where(x => !string.IsNullOrWhiteSpace(x)).Distinct().ToList();
if (!ids.Any())
return new Dictionary<string, Contact>();
return ((await _contactsRepository.GetByIdsAsync(departmentId, ids)) ?? Enumerable.Empty<Contact>())
.Where(c => !c.IsDeleted)
.ToDictionary(c => c.ContactId, c => c);
}Prompt for LLM
File Core/Resgrid.Services/ContactsService.cs:
Line 590 to 598:
N+1 query pattern in Core/Resgrid.Services/ContactsService.cs: GetAlertNotesByContactIdsAsync and LoadContactsAsync issue one repository call per distinct contact, adding O(contacts) round-trips to GetActiveCalls and GetCall on top of the existing batched reads. Batch contact and alert-note retrieval with repository methods such as GetContactNotesByContactIdsAsync and GetByIdsAsync so each request uses one query per dataset.
Suggested Code:
public async Task<Dictionary<string, List<ContactNote>>> GetAlertNotesByContactIdsAsync(int departmentId, IEnumerable<string> contactIds)
{
var ids = (contactIds ?? Enumerable.Empty<string>()).Where(x => !string.IsNullOrWhiteSpace(x)).Distinct().ToList();
if (!ids.Any())
return new Dictionary<string, List<ContactNote>>();
var now = DateTime.UtcNow;
var notes = await _contactNotesRepository.GetContactNotesByContactIdsAsync(departmentId, ids);
return (notes ?? Enumerable.Empty<ContactNote>())
.Where(x => IsLiveAlertNote(x, departmentId, now))
.GroupBy(x => x.ContactId)
.ToDictionary(g => g.Key, g => g.ToList());
}
private async Task<Dictionary<string, Contact>> LoadContactsAsync(int departmentId, IEnumerable<string> contactIds)
{
var ids = contactIds.Where(x => !string.IsNullOrWhiteSpace(x)).Distinct().ToList();
if (!ids.Any())
return new Dictionary<string, Contact>();
return ((await _contactsRepository.GetByIdsAsync(departmentId, ids)) ?? Enumerable.Empty<Contact>())
.Where(c => !c.IsDeleted)
.ToDictionary(c => c.ContactId, c => c);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| foreach (var key in OnePerOperationalPeriod()) | ||
| { | ||
| var definition = pack.Definitions.SingleOrDefault(d => d.Key == IncidentSupportPackKey + "." + key) |
There was a problem hiding this comment.
Invariant mismatch in Core/Resgrid.Services/Records/RecordTemplateCatalog.IncidentSupport.cs and Tests/Resgrid.Tests/Rms/RmsPreventionFakes.cs: SingleOrDefault() implies that no match is acceptable, but the code throws when the definition is missing, so exactly one match is required. Use Single() to make that contract explicit.
Kody rule violation: Use `First`/`Single` Instead of `FirstOrDefault`/`SingleOrDefault` for Non-Empty Collections
var definition = pack.Definitions.Single(d => d.Key == IncidentSupportPackKey + "." + key)Prompt for LLM
File Core/Resgrid.Services/Records/RecordTemplateCatalog.IncidentSupport.cs:
Line 123:
Invariant mismatch in Core/Resgrid.Services/Records/RecordTemplateCatalog.IncidentSupport.cs and Tests/Resgrid.Tests/Rms/RmsPreventionFakes.cs: SingleOrDefault() implies that no match is acceptable, but the code throws when the definition is missing, so exactly one match is required. Use Single() to make that contract explicit.
Suggested Code:
var definition = pack.Definitions.Single(d => d.Key == IncidentSupportPackKey + "." + key)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| inspection.SignatureName = RecordsPreventionGate.Trim(signatureName, 200); | ||
| inspection.SignedOn = inspection.SignatureName == null ? null : now; | ||
| inspection.CompletedOn = now; inspection.StartedOn ??= now; inspection.InspectorUserId ??= userId; | ||
| inspection.Result = failed.Count == 0 ? (int)RmsInspectionResult.Pass : failedRequired ? (int)RmsInspectionResult.Fail : (int)RmsInspectionResult.Conditional; |
There was a problem hiding this comment.
Blocking async call identified in Core/Resgrid.Services/Records/RecordsInspectionsService.cs and the related locations listed: .Result or .Wait() can deadlock and waste threads, although the sample line does not show the offending call directly. Replace blocking access with await.
Kody rule violation: Avoid Blocking Calls to Async Methods
Prompt for LLM
File Core/Resgrid.Services/Records/RecordsInspectionsService.cs:
Line 313:
Blocking async call identified in Core/Resgrid.Services/Records/RecordsInspectionsService.cs and the related locations listed: .Result or .Wait() can deadlock and waste threads, although the sample line does not show the offending call directly. Replace blocking access with await.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| inspection.SignatureName = RecordsPreventionGate.Trim(signatureName, 200); | ||
| inspection.SignedOn = inspection.SignatureName == null ? null : now; | ||
| inspection.CompletedOn = now; inspection.StartedOn ??= now; inspection.InspectorUserId ??= userId; | ||
| inspection.Result = failed.Count == 0 ? (int)RmsInspectionResult.Pass : failedRequired ? (int)RmsInspectionResult.Fail : (int)RmsInspectionResult.Conditional; |
There was a problem hiding this comment.
Async rule violation in Core/Resgrid.Services/Records/RecordsInspectionsService.cs and the related locations listed: the issue is blocking async work with .Result or .Wait(), despite the sample line not showing the violation directly. Replace blocking calls with await and keep async/await flow end-to-end.
Kody rule violation: Await async operations properly
Prompt for LLM
File Core/Resgrid.Services/Records/RecordsInspectionsService.cs:
Line 313:
Async rule violation in Core/Resgrid.Services/Records/RecordsInspectionsService.cs and the related locations listed: the issue is blocking async work with .Result or .Wait(), despite the sample line not showing the violation directly. Replace blocking calls with await and keep async/await flow end-to-end.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| entity.Title = RecordsPreventionGate.Require(input.Title, 200, "A hazard needs a title."); | ||
| entity.HazardType = input.HazardType; entity.Severity = Math.Clamp(input.Severity, 1, 4); | ||
| entity.Description = RecordsPreventionGate.Trim(input.Description, 4000); | ||
| entity.LocationDescription = RecordsPreventionGate.Trim(input.LocationDescription, 1000); | ||
| entity.GpsCoordinates = RecordsPreventionGate.Trim(input.GpsCoordinates, 100); | ||
| entity.ShouldAlert = input.ShouldAlert; entity.ModifiedOn = now; |
There was a problem hiding this comment.
Severity enum mismatch in Core/Resgrid.Services/Records/RecordsOccupancyService.cs: RMS occupancy hazards now accept severity values 3 and 4, but the ownership-cutover projection still maps through ContactPreplanHazard, whose contact-facing enum defines only 0..2. Clamp severity to the ContactPreplanHazardSeverities range or add an explicit translation before projecting RMS hazards into the ContactPreplan view.
entity.Title = RecordsPreventionGate.Require(input.Title, 200, "A hazard needs a title.");
entity.HazardType = input.HazardType;
entity.Severity = Math.Clamp(input.Severity,
(int)ContactPreplanHazardSeverities.Info,
(int)ContactPreplanHazardSeverities.Danger);
entity.Description = RecordsPreventionGate.Trim(input.Description, 4000);Prompt for LLM
File Core/Resgrid.Services/Records/RecordsOccupancyService.cs:
Line 291 to 296:
Severity enum mismatch in Core/Resgrid.Services/Records/RecordsOccupancyService.cs: RMS occupancy hazards now accept severity values 3 and 4, but the ownership-cutover projection still maps through ContactPreplanHazard, whose contact-facing enum defines only 0..2. Clamp severity to the ContactPreplanHazardSeverities range or add an explicit translation before projecting RMS hazards into the ContactPreplan view.
Suggested Code:
entity.Title = RecordsPreventionGate.Require(input.Title, 200, "A hazard needs a title.");
entity.HazardType = input.HazardType;
entity.Severity = Math.Clamp(input.Severity,
(int)ContactPreplanHazardSeverities.Info,
(int)ContactPreplanHazardSeverities.Danger);
entity.Description = RecordsPreventionGate.Trim(input.Description, 4000);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return entity; | ||
| } | ||
|
|
||
| public async Task<List<RmsPermit>> ListAsync(int departmentId, string userId, RmsPermitQuery query) |
There was a problem hiding this comment.
Responsibility overload in Core/Resgrid.Services/Records/RecordsPermitsService.cs: the service combines listing, counting, retrieval, mutations, transitions, auditing, protection, and expiry workflows in one class. Split permit query operations from command and workflow operations to restore single-responsibility boundaries.
Kody rule violation: Ensure Controllers Follow Single Responsibility Principle
// Move listing/query orchestration into a dedicated query service and keep controller/service responsibilities narrower.Prompt for LLM
File Core/Resgrid.Services/Records/RecordsPermitsService.cs:
Line 64:
Responsibility overload in Core/Resgrid.Services/Records/RecordsPermitsService.cs: the service combines listing, counting, retrieval, mutations, transitions, auditing, protection, and expiry workflows in one class. Split permit query operations from command and workflow operations to restore single-responsibility boundaries.
Suggested Code:
// Move listing/query orchestration into a dedicated query service and keep controller/service responsibilities narrower.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public async Task<RecordsReleaseTelemetry> LogSnapshotAsync(int departmentId, CancellationToken cancellationToken = default) | ||
| { | ||
| var snapshot = await BuildAsync(departmentId, 24); | ||
| Logging.LogInfo(JsonConvert.SerializeObject(new { rms_release_telemetry = snapshot })); |
There was a problem hiding this comment.
Unstructured telemetry logging in Core/Resgrid.Services/Records/RecordsReleaseTelemetryService.cs and the related locations listed: Logging.LogInfo(JsonConvert.SerializeObject(new { rms_release_telemetry = snapshot })) collapses operation context into an opaque string. Emit a structured log entry with fields such as operation = "LogSnapshotAsync" and departmentId.
Kody rule violation: Include error context in structured logs
Logging.LogInfo("Release telemetry snapshot", new { operation = "LogSnapshotAsync", departmentId, rms_release_telemetry = snapshot });Prompt for LLM
File Core/Resgrid.Services/Records/RecordsReleaseTelemetryService.cs:
Line 59:
Unstructured telemetry logging in Core/Resgrid.Services/Records/RecordsReleaseTelemetryService.cs and the related locations listed: Logging.LogInfo(JsonConvert.SerializeObject(new { rms_release_telemetry = snapshot })) collapses operation context into an opaque string. Emit a structured log entry with fields such as operation = "LogSnapshotAsync" and departmentId.
Suggested Code:
Logging.LogInfo("Release telemetry snapshot", new { operation = "LogSnapshotAsync", departmentId, rms_release_telemetry = snapshot });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| i["program_name"] = "Annual life-safety inspection"; | ||
| i["state"] = "ReinspectionRequired"; | ||
| i["result"] = "Fail"; | ||
| i["scheduled_on"] = DateTime.Now.AddDays(-3); |
There was a problem hiding this comment.
Timing source misuse in Core/Resgrid.Services/WorkflowSampleDataGenerator.cs and the related locations listed: DateTime.Now is used for timing-sensitive behavior and is vulnerable to daylight savings and system clock changes. Use Stopwatch for elapsed-time measurement.
Kody rule violation: Avoid `DateTime.Now` for Timing Operations
Prompt for LLM
File Core/Resgrid.Services/WorkflowSampleDataGenerator.cs:
Line 888:
Timing source misuse in Core/Resgrid.Services/WorkflowSampleDataGenerator.cs and the related locations listed: DateTime.Now is used for timing-sensitive behavior and is vulnerable to daylight savings and system clock changes. Use Stopwatch for elapsed-time measurement.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .WithColumn("IsProtected").AsBoolean().NotNullable().WithDefaultValue(false); | ||
|
|
||
| Create.Index("IX_ContactAttachments_Contact").OnTable("ContactAttachments") | ||
| .OnColumn("DepartmentId").Ascending().OnColumn("ContactId").Ascending().OnColumn("IsDeleted").Ascending(); |
There was a problem hiding this comment.
Index validation gap in Providers/Resgrid.Providers.Migrations/Migrations/M0184_AddContactAttachments.cs: the index on DepartmentId, ContactId, and IsDeleted is added without evidence that those columns match real query predicates or production workload plans. Verify the access patterns and confirm the database index strategy against actual usage.
Kody rule violation: Add database indexes for query optimization
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0184_AddContactAttachments.cs:
Line 34:
Index validation gap in Providers/Resgrid.Providers.Migrations/Migrations/M0184_AddContactAttachments.cs: the index on DepartmentId, ContactId, and IsDeleted is added without evidence that those columns match real query predicates or production workload plans. Verify the access patterns and confirm the database index strategy against actual usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| Create.Index("IX_RmsInvestigationReferrals_Case").OnTable("RmsInvestigationReferrals").OnColumn("DepartmentId").Ascending().OnColumn("RmsInvestigationCaseId").Ascending(); | ||
| } | ||
|
|
||
| Execute.Sql("IF NOT EXISTS (SELECT 1 FROM [FeatureFlags] WHERE [FlagKey] = 'Records.Prevention.Occupancy') INSERT INTO [FeatureFlags] ([FlagKey], [Name], [Description], [Category], [IsEnabledGlobally]) VALUES ('Records.Prevention.Occupancy', 'Records Prevention - Occupancies', 'RMS-5 occupancy/property master and the Contacts pre-plan crosswalk. Requires Records.System. Seeded off.', 'Records', 0);"); |
There was a problem hiding this comment.
Missing audit trail in Providers/Resgrid.Providers.Migrations/Migrations/M0186_AddRmsPreventionAndInvestigations.cs: the migration seeds the security-relevant flag Records.Prevention.Occupancy without an immutable audit record containing timestamp, actor.user_id, actor.role, action, resource.id, result, trace_id, ip, and user_agent. Add an append-only audit write for the flag change or move the mutation through audited application code.
Kody rule violation: Emit tamper-evident audit logs with required fields
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0186_AddRmsPreventionAndInvestigations.cs:
Line 709:
Missing audit trail in Providers/Resgrid.Providers.Migrations/Migrations/M0186_AddRmsPreventionAndInvestigations.cs: the migration seeds the security-relevant flag Records.Prevention.Occupancy without an immutable audit record containing timestamp, actor.user_id, actor.role, action, resource.id, result, trace_id, ip, and user_agent. Add an append-only audit write for the flag change or move the mutation through audited application code.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public override void Up() | ||
| { | ||
| if (Schema.Table("rmsoperationalrecords").Exists() && !Schema.Table("rmsoperationalrecords").Index("IX_RmsOperationalRecords_Department_State_Modified").Exists()) | ||
| Create.Index("IX_RmsOperationalRecords_Department_State_Modified").OnTable("rmsoperationalrecords").OnColumn("departmentid").Ascending().OnColumn("state").Ascending().OnColumn("modifiedon").Ascending(); |
There was a problem hiding this comment.
Migration locking risk in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0185_RmsReleaseReadinessPg.cs and the related locations listed: Create.Index("IX_RmsOperationalRecords_Department_State_Modified") on rmsoperationalrecords shows no concurrent or online creation strategy for an existing PostgreSQL table. Use a non-blocking approach such as CONCURRENTLY where supported and document rollback and downtime expectations.
Kody rule violation: Block risky database migrations (locking ops, downtime risk)
Prompt for LLM
File Providers/Resgrid.Providers.MigrationsPg/Migrations/M0185_RmsReleaseReadinessPg.cs:
Line 19:
Migration locking risk in Providers/Resgrid.Providers.MigrationsPg/Migrations/M0185_RmsReleaseReadinessPg.cs and the related locations listed: Create.Index("IX_RmsOperationalRecords_Department_State_Modified") on rmsoperationalrecords shows no concurrent or online creation strategy for an existing PostgreSQL table. Use a non-blocking approach such as CONCURRENTLY where supported and document rollback and downtime expectations.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public Task<IEnumerable<RmsAccessAudit>> GetForAggregateAsync(int departmentId, string aggregateId, int take) | ||
| { | ||
| return QueryAsync<RmsAccessAudit>($"SELECT * FROM {Tbl("RmsAccessAudits")} WHERE {Col("DepartmentId")} = {P}DepartmentId AND {Col("RecordId")} = {P}Id ORDER BY {Col("OccurredOn")} DESC, {Col("RmsAccessAuditId")} DESC {Paging()}", new { DepartmentId = departmentId, Id = aggregateId, Skip = 0, Take = Math.Clamp(take, 1, 1000) }); |
There was a problem hiding this comment.
Audit lookup mismatch in Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs: GetForAggregateAsync filters on RecordId, but the new prevention/investigation audit writer stores case IDs in CorrelationId and leaves RecordId null. Query CorrelationId instead so the RecordInvestigationsService access-audit view returns the reads and exports audited by RecordsPreventionGate.AuditAsync.
public Task<IEnumerable<RmsAccessAudit>> GetForAggregateAsync(int departmentId, string aggregateId, int take)
{
return QueryAsync<RmsAccessAudit>($"SELECT * FROM {Tbl("RmsAccessAudits")} WHERE {Col("DepartmentId")} = {P}DepartmentId AND {Col("CorrelationId")} = {P}Id ORDER BY {Col("OccurredOn")} DESC, {Col("RmsAccessAuditId")} DESC {Paging()}", new { DepartmentId = departmentId, Id = aggregateId, Skip = 0, Take = Math.Clamp(take, 1, 1000) });
}Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs:
Line 844 to 846:
Audit lookup mismatch in Repositories/Resgrid.Repositories.DataRepository/RmsRepositories.cs: GetForAggregateAsync filters on RecordId, but the new prevention/investigation audit writer stores case IDs in CorrelationId and leaves RecordId null. Query CorrelationId instead so the RecordInvestigationsService access-audit view returns the reads and exports audited by RecordsPreventionGate.AuditAsync.
Suggested Code:
public Task<IEnumerable<RmsAccessAudit>> GetForAggregateAsync(int departmentId, string aggregateId, int take)
{
return QueryAsync<RmsAccessAudit>($"SELECT * FROM {Tbl("RmsAccessAudits")} WHERE {Col("DepartmentId")} = {P}DepartmentId AND {Col("CorrelationId")} = {P}Id ORDER BY {Col("OccurredOn")} DESC, {Col("RmsAccessAuditId")} DESC {Paging()}", new { DepartmentId = departmentId, Id = aggregateId, Skip = 0, Take = Math.Clamp(take, 1, 1000) });
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public int Inserts { get; private set; } | ||
| public int Updates { get; private set; } | ||
|
|
||
| protected static int DeptOf(T e) => (int)e.GetType().GetProperty("DepartmentId").GetValue(e); |
There was a problem hiding this comment.
Null reflection dereference in Tests/Resgrid.Tests/Rms/RmsPreventionFakes.cs and the related locations listed: GetProperty("DepartmentId") can return null, making .GetValue(e) a potential null reference. Add a null-safe check with ?. and a clear fallback or exception.
Kody rule violation: Add null checks before accessing properties
protected static int DeptOf(T e) => (int?)(e.GetType().GetProperty("DepartmentId")?.GetValue(e)) ?? 0;Prompt for LLM
File Tests/Resgrid.Tests/Rms/RmsPreventionFakes.cs:
Line 22:
Null reflection dereference in Tests/Resgrid.Tests/Rms/RmsPreventionFakes.cs and the related locations listed: GetProperty("DepartmentId") can return null, making .GetValue(e) a potential null reference. Add a null-safe check with ?. and a clear fallback or exception.
Suggested Code:
protected static int DeptOf(T e) => (int?)(e.GetType().GetProperty("DepartmentId")?.GetValue(e)) ?? 0;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| .Concat(Resgrid.Model.RmsProtectedFields.Permits.Keys).Concat(Resgrid.Model.RmsProtectedFields.PlanReviews.Keys) | ||
| .Concat(Resgrid.Model.RmsProtectedFields.InvestigationCases.Keys).Concat(Resgrid.Model.RmsProtectedFields.InvestigationNotes.Keys) | ||
| .Concat(Resgrid.Model.RmsProtectedFields.InvestigationEvidence.Keys).Concat(Resgrid.Model.RmsProtectedFields.InvestigationCustody.Keys) | ||
| .Concat(Resgrid.Model.RmsProtectedFields.InvestigationReferrals.Keys).Concat(Resgrid.Model.RmsProtectedFields.QualityReviews.Keys) |
There was a problem hiding this comment.
Readability issue in Tests/Resgrid.Tests/Services/RemainingCandidateProtectionTests.cs and the related locations listed: the long chained query obscures the business assertion. Introduce intermediate variables or helper methods so the test names the expected field set instead of embedding the full chain inline.
Kody rule violation: Limit Lengthy LINQ Chains
expectedRms5FieldIds = expectedRms5FieldIds
.Concat(Resgrid.Model.RmsProtectedFields.InvestigationReferrals.Keys)
.Concat(Resgrid.Model.RmsProtectedFields.QualityReviews.Keys);Prompt for LLM
File Tests/Resgrid.Tests/Services/RemainingCandidateProtectionTests.cs:
Line 37:
Readability issue in Tests/Resgrid.Tests/Services/RemainingCandidateProtectionTests.cs and the related locations listed: the long chained query obscures the business assertion. Introduce intermediate variables or helper methods so the test names the expected field set instead of embedding the full chain inline.
Suggested Code:
expectedRms5FieldIds = expectedRms5FieldIds
.Concat(Resgrid.Model.RmsProtectedFields.InvestigationReferrals.Keys)
.Concat(Resgrid.Model.RmsProtectedFields.QualityReviews.Keys);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| var attachment = meta; | ||
| if (includeData) | ||
| attachment = await _contactsService.GetContactAttachmentByIdAsync(meta.ContactAttachmentId) ?? meta; |
There was a problem hiding this comment.
N+1 attachment fetch in Web/Resgrid.Web.Services/Controllers/v4/ContactFilesController.cs and the related locations listed: await _contactsService.GetContactAttachmentByIdAsync(meta.ContactAttachmentId) executes once per item during iteration. Batch the attachment IDs and resolve them with a single GetContactAttachmentsByIdsAsync call.
Kody rule violation: Detect N+1 style queries and suggest batching
var ids = attachments.Select(a => a.ContactAttachmentId).ToList();
var fullAttachments = await _contactsService.GetContactAttachmentsByIdsAsync(ids);
// map results by id and reuse in the loopPrompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/ContactFilesController.cs:
Line 90:
N+1 attachment fetch in Web/Resgrid.Web.Services/Controllers/v4/ContactFilesController.cs and the related locations listed: await _contactsService.GetContactAttachmentByIdAsync(meta.ContactAttachmentId) executes once per item during iteration. Batch the attachment IDs and resolve them with a single GetContactAttachmentsByIdsAsync call.
Suggested Code:
var ids = attachments.Select(a => a.ContactAttachmentId).ToList();
var fullAttachments = await _contactsService.GetContactAttachmentsByIdsAsync(ids);
// map results by id and reuse in the loop
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public async Task<ActionResult<StandardApiResponseV4Base>> ImportCodeSections(string codeSetId, [FromBody] HydrantImportInput input, CancellationToken cancellationToken) | ||
| { | ||
| if (!await FlagOnAsync()) return NotFound(); | ||
| try { var created = await _inspections.ImportCodeSectionsAsync(DepartmentId, UserId, codeSetId, input?.Csv, cancellationToken); return Ok(Done(new StandardApiResponseV4Base { PageSize = created })); } |
There was a problem hiding this comment.
Input validation order bug in Web/Resgrid.Web.Services/Controllers/v4/RecordInspectionsController.cs and the related locations listed: _inspections.ImportCodeSectionsAsync(DepartmentId, UserId, codeSetId, input?.Csv, cancellationToken) is invoked before confirming that codeSetId and input?.Csv are non-empty. Add guard clauses for codeSetId and input?.Csv before the service call and return BadRequest() on invalid input.
Kody rule violation: Order validations before database queries
if (string.IsNullOrWhiteSpace(codeSetId) || string.IsNullOrWhiteSpace(input?.Csv)) return BadRequest();
try { var created = await _inspections.ImportCodeSectionsAsync(DepartmentId, UserId, codeSetId, input.Csv, cancellationToken); return Ok(Done(new StandardApiResponseV4Base { PageSize = created })); }Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/RecordInspectionsController.cs:
Line 78:
Input validation order bug in Web/Resgrid.Web.Services/Controllers/v4/RecordInspectionsController.cs and the related locations listed: _inspections.ImportCodeSectionsAsync(DepartmentId, UserId, codeSetId, input?.Csv, cancellationToken) is invoked before confirming that codeSetId and input?.Csv are non-empty. Add guard clauses for codeSetId and input?.Csv before the service call and return BadRequest() on invalid input.
Suggested Code:
if (string.IsNullOrWhiteSpace(codeSetId) || string.IsNullOrWhiteSpace(input?.Csv)) return BadRequest();
try { var created = await _inspections.ImportCodeSectionsAsync(DepartmentId, UserId, codeSetId, input.Csv, cancellationToken); return Ok(Done(new StandardApiResponseV4Base { PageSize = created })); }
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public async Task<ActionResult<OccupancySavedResult>> Save([FromBody] OccupancyInput input, CancellationToken cancellationToken) | ||
| { | ||
| if (!await FlagOnAsync()) return NotFound(); | ||
| if (input == null) return BadRequest(); |
There was a problem hiding this comment.
Opaque client error in Web/Resgrid.Web.Services/Controllers/v4/RecordOccupanciesController.cs and the related locations listed: if (input == null) return BadRequest(); returns HTTP 400 without a reason payload. Return a minimal error body such as { error = "input is required" } so callers can diagnose the validation failure.
Kody rule violation: Use appropriate HTTP status codes
if (input == null) return BadRequest(new { error = "input is required" });Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/RecordOccupanciesController.cs:
Line 74:
Opaque client error in Web/Resgrid.Web.Services/Controllers/v4/RecordOccupanciesController.cs and the related locations listed: if (input == null) return BadRequest(); returns HTTP 400 without a reason payload. Return a minimal error body such as { error = "input is required" } so callers can diagnose the validation failure.
Suggested Code:
if (input == null) return BadRequest(new { error = "input is required" });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| if (!await FlagOnAsync()) return NotFound(); | ||
| if (input == null) return BadRequest(); | ||
| try { return Ok(Done(new PermitTypeResult { Data = RecordsRms5ApiMapper.ToPermitType(await _permits.SaveTypeAsync(DepartmentId, UserId, new RmsPermitType { RmsPermitTypeId = input.PermitTypeId, Name = input.Name, Code = input.Code, Description = input.Description, DefaultValidityDays = input.DefaultValidityDays, RequiresPlanReview = input.RequiresPlanReview, FeeAmount = input.FeeAmount, ConditionsTemplate = input.ConditionsTemplate, IsActive = input.IsActive }, cancellationToken)), PageSize = 1 })); } |
There was a problem hiding this comment.
Controller mapping leakage in Web/Resgrid.Web.Services/Controllers/v4/RecordPermitsController.cs: the action constructs RmsPermitType inline inside the controller, mixing transport mapping with request handling. Delegate object construction to RecordsRms5ApiMapper.FromPermitType(input) or a service method to keep the controller thin.
Kody rule violation: Separate UI logic from business logic
try
{
var permitType = RecordsRms5ApiMapper.FromPermitType(input);
var saved = await _permits.SaveTypeAsync(DepartmentId, UserId, permitType, cancellationToken);
return Ok(Done(new PermitTypeResult { Data = RecordsRms5ApiMapper.ToPermitType(saved), PageSize = 1 }));
}Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/RecordPermitsController.cs:
Line 46:
Controller mapping leakage in Web/Resgrid.Web.Services/Controllers/v4/RecordPermitsController.cs: the action constructs RmsPermitType inline inside the controller, mixing transport mapping with request handling. Delegate object construction to RecordsRms5ApiMapper.FromPermitType(input) or a service method to keep the controller thin.
Suggested Code:
try
{
var permitType = RecordsRms5ApiMapper.FromPermitType(input);
var saved = await _permits.SaveTypeAsync(DepartmentId, UserId, permitType, cancellationToken);
return Ok(Done(new PermitTypeResult { Data = RecordsRms5ApiMapper.ToPermitType(saved), PageSize = 1 }));
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!await ModuleOnAsync(Flag)) return NotFound(); | ||
| try | ||
| { | ||
| var saved = await _hydrants.SaveAsync(DepartmentId, UserId, model.Hydrant, cancellationToken); |
There was a problem hiding this comment.
Model validation bypass in Web/Resgrid.Web/Areas/User/Controllers/RecordHydrantsController.cs and the related locations listed: the action saves model.Hydrant through _hydrants.SaveAsync(DepartmentId, UserId, model.Hydrant, cancellationToken) without first checking ModelState. Return early on !ModelState.IsValid, prepare the model, and redisplay the view before persisting.
Kody rule violation: Always Validate `ModelState.IsValid` in Controllers
if (!ModelState.IsValid)
{
Prepare(model);
return View(model);
}
var saved = await _hydrants.SaveAsync(DepartmentId, UserId, model.Hydrant, cancellationToken);Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordHydrantsController.cs:
Line 84:
Model validation bypass in Web/Resgrid.Web/Areas/User/Controllers/RecordHydrantsController.cs and the related locations listed: the action saves model.Hydrant through _hydrants.SaveAsync(DepartmentId, UserId, model.Hydrant, cancellationToken) without first checking ModelState. Return early on !ModelState.IsValid, prepare the model, and redisplay the view before persisting.
Suggested Code:
if (!ModelState.IsValid)
{
Prepare(model);
return View(model);
}
var saved = await _hydrants.SaveAsync(DepartmentId, UserId, model.Hydrant, cancellationToken);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| foreach (var o in await _occupancies.ListAsync(DepartmentId, UserId, new RmsOccupancyQuery { Take = 2000 })) | ||
| if (wanted.Contains(o.RmsOccupancyId)) map[o.RmsOccupancyId] = (o.OccupancyNumber + " " + o.Name).Trim(); | ||
| } | ||
| catch (RecordsModuleDisabledException) { } |
There was a problem hiding this comment.
Exception swallowing in Web/Resgrid.Web/Areas/User/Controllers/RecordInspectionsController.cs and the related locations listed: catch (RecordsModuleDisabledException) { } suppresses failures without logging or explicit handling. Log the exception with context and either rethrow or convert it to a deliberate application response.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordInspectionsController.cs:
Line 288:
Exception swallowing in Web/Resgrid.Web/Areas/User/Controllers/RecordInspectionsController.cs and the related locations listed: catch (RecordsModuleDisabledException) { } suppresses failures without logging or explicit handling. Log the exception with context and either rethrow or convert it to a deliberate application response.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (wanted.Count == 0) return; | ||
| try | ||
| { | ||
| foreach (var o in await _occupancies.ListAsync(DepartmentId, UserId, new RmsOccupancyQuery { Take = 2000 })) |
There was a problem hiding this comment.
Error-handling ambiguity in Web/Resgrid.Web/Areas/User/Controllers/RecordInspectionsController.cs and the related locations listed: awaited external calls run within methods that swallow a specific exception and do not add contextual handling for other async failures. Handle those failures with explicit logging or application mapping instead of silent suppression or context-free propagation.
Kody rule violation: Handle async operations with proper error handling
var occupancies = await _occupancies.ListAsync(DepartmentId, UserId, new RmsOccupancyQuery { Take = OccupancyQueryLimit });
foreach (var o in occupancies)Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordInspectionsController.cs:
Line 285:
Error-handling ambiguity in Web/Resgrid.Web/Areas/User/Controllers/RecordInspectionsController.cs and the related locations listed: awaited external calls run within methods that swallow a specific exception and do not add contextual handling for other async failures. Handle those failures with explicit logging or application mapping instead of silent suppression or context-free propagation.
Suggested Code:
var occupancies = await _occupancies.ListAsync(DepartmentId, UserId, new RmsOccupancyQuery { Take = OccupancyQueryLimit });
foreach (var o in occupancies)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!await ModuleOnAsync(Flag)) return NotFound(); | ||
| try | ||
| { | ||
| var bytes = await _investigations.ExportAsync(DepartmentId, UserId, id, Ip, cancellationToken); |
There was a problem hiding this comment.
Export control gap in Web/Resgrid.Web/Areas/User/Controllers/RecordInvestigationsController.cs: _investigations.ExportAsync(DepartmentId, UserId, id, Ip, cancellationToken) executes a bulk export path without visible approval, step-up MFA, rate limiting, watermarking, or export_id audit capture. Enforce those controls before invoking the export operation.
Kody rule violation: Define data export controls and watermarking
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordInvestigationsController.cs:
Line 270:
Export control gap in Web/Resgrid.Web/Areas/User/Controllers/RecordInvestigationsController.cs: _investigations.ExportAsync(DepartmentId, UserId, id, Ip, cancellationToken) executes a bulk export path without visible approval, step-up MFA, rate limiting, watermarking, or export_id audit capture. Enforce those controls before invoking the export operation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| byte[] bytes; | ||
| using (var stream = new System.IO.MemoryStream()) { await file.CopyToAsync(stream, cancellationToken); bytes = stream.ToArray(); } | ||
| await _attachments.AddAsync(DepartmentId, UserId, (RmsPreventionParentKind)parentKind, parentId, file.FileName, file.ContentType, bytes, description, restricted, cancellationToken); |
There was a problem hiding this comment.
Filename propagation risk in Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs and the related locations listed: file.FileName may contain sensitive or identifying content and is passed through directly to _attachments.AddAsync. Normalize to a safe basename with System.IO.Path.GetFileName(file.FileName) and avoid emitting raw filenames into logs or telemetry.
Kody rule violation: Mask PII and secrets in logs
var safeFileName = System.IO.Path.GetFileName(file.FileName);
await _attachments.AddAsync(DepartmentId, UserId, (RmsPreventionParentKind)parentKind, parentId, safeFileName, file.ContentType, bytes, description, restricted, cancellationToken);Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs:
Line 220:
Filename propagation risk in Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs and the related locations listed: file.FileName may contain sensitive or identifying content and is passed through directly to _attachments.AddAsync. Normalize to a safe basename with System.IO.Path.GetFileName(file.FileName) and avoid emitting raw filenames into logs or telemetry.
Suggested Code:
var safeFileName = System.IO.Path.GetFileName(file.FileName);
await _attachments.AddAsync(DepartmentId, UserId, (RmsPreventionParentKind)parentKind, parentId, safeFileName, file.ContentType, bytes, description, restricted, cancellationToken);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| private async Task PopulateAsync(RecordsQualityRubricView model) | ||
| { | ||
| model.Definitions = new[] { new SelectListItem { Value = "", Text = Localizer["AllDefinitions"].Value } } | ||
| .Concat((await _definitions.ListAsync(DepartmentId)).Select(d => new SelectListItem { Value = d.Key, Text = d.Name, Selected = d.Key == model.Rubric?.DefinitionKey })) |
There was a problem hiding this comment.
Async LINQ composition issue in Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs: await _definitions.ListAsync(DepartmentId) is embedded directly inside a larger Concat projection, which obscures control flow and mixes I/O with query composition. Fetch definitions first, then build model.Definitions from the materialized result.
Kody rule violation: Use Awaitable Methods in Async Code
var definitions = await _definitions.ListAsync(DepartmentId);
model.Definitions = new[] { new SelectListItem { Value = "", Text = Localizer["AllDefinitions"].Value } }
.Concat(definitions.Select(d => new SelectListItem { Value = d.Key, Text = d.Name, Selected = d.Key == model.Rubric?.DefinitionKey }))
.ToList();Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs:
Line 149:
Async LINQ composition issue in Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs: await _definitions.ListAsync(DepartmentId) is embedded directly inside a larger Concat projection, which obscures control flow and mixes I/O with query composition. Fetch definitions first, then build model.Definitions from the materialized result.
Suggested Code:
var definitions = await _definitions.ListAsync(DepartmentId);
model.Definitions = new[] { new SelectListItem { Value = "", Text = Localizer["AllDefinitions"].Value } }
.Concat(definitions.Select(d => new SelectListItem { Value = d.Key, Text = d.Name, Selected = d.Key == model.Rubric?.DefinitionKey }))
.ToList();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| [HttpGet] | ||
| public async Task<IActionResult> Index(int windowHours = 24) |
There was a problem hiding this comment.
Routing ambiguity in Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs: Index(int windowHours = 24) lacks an explicit HTTP verb attribute. Add [HttpGet] to make the action contract unambiguous.
Kody rule violation: Annotate REST API Actions with HTTP Verb Attributes
[HttpGet]
public async Task<IActionResult> Index(int windowHours = 24)Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs:
Line 184:
Routing ambiguity in Web/Resgrid.Web/Areas/User/Controllers/RecordsQualityController.cs: Index(int windowHours = 24) lacks an explicit HTTP verb attribute. Add [HttpGet] to make the action contract unambiguous.
Suggested Code:
[HttpGet]
public async Task<IActionResult> Index(int windowHours = 24)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <tbody> | ||
| @foreach (var hazard in Model.Hazards) | ||
| { | ||
| <tr style="background-color:@Resgrid.Web.Areas.User.Controllers.ContactsController.HazardSeverityColor(hazard.Severity)"> |
There was a problem hiding this comment.
Layering violation in Web/Resgrid.Web/Areas/User/Views/Contacts/View.cshtml: the view calls Resgrid.Web.Areas.User.Controllers.ContactsController.HazardSeverityColor(hazard.Severity) directly, coupling presentation to controller implementation. Move HazardSeverityColor into the view model, a UI helper, or a dedicated service.
Kody rule violation: Enforce architecture boundaries and layering rules
<tr style="background-color:@HazardSeverityColor(hazard.Severity)">Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Contacts/View.cshtml:
Line 532:
Layering violation in Web/Resgrid.Web/Areas/User/Views/Contacts/View.cshtml: the view calls Resgrid.Web.Areas.User.Controllers.ContactsController.HazardSeverityColor(hazard.Severity) directly, coupling presentation to controller implementation. Move HazardSeverityColor into the view model, a UI helper, or a dedicated service.
Suggested Code:
<tr style="background-color:@HazardSeverityColor(hazard.Severity)">
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| points.forEach(function (p) { | ||
| var color = p.InService ? colors[p.FlowClass] || '#999' : '#000'; | ||
| var m = L.circleMarker([p.Lat, p.Lon], { radius: 7, color: color, fillColor: color, fillOpacity: 0.8 }).addTo(map); | ||
| m.bindPopup('<a href="@Url.Action("Details", "RecordHydrants", new { area = "User" })?id=' + p.HydrantId + '">' + p.HydrantNumber + '</a>' + (p.FlowGpm ? '<br/>' + p.FlowGpm + ' gpm' : '') + (p.InService ? '' : '<br/><b>@L["OutOfService"]</b>')); |
There was a problem hiding this comment.
Stored XSS in Web/Resgrid.Web/Areas/User/Views/RecordHydrants/Index.cshtml: HydrantNumber is concatenated directly into the Leaflet popup HTML string, so markup or event handlers in stored data are injected by bindPopup. HTML-encode the popup content or build the popup with DOM text nodes instead of raw HTML.
var link = document.createElement('a');
link.href = '@Url.Action("Details", "RecordHydrants", new { area = "User" })?id=' + encodeURIComponent(p.HydrantId);
link.textContent = p.HydrantNumber || '';
var container = document.createElement('div');
container.appendChild(link);
if (p.FlowGpm) { container.appendChild(document.createElement('br')); container.appendChild(document.createTextNode(p.FlowGpm + ' gpm')); }
if (!p.InService) { container.appendChild(document.createElement('br')); var status = document.createElement('b'); status.textContent = '@L["OutOfService"]'; container.appendChild(status); }
m.bindPopup(container);Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/RecordHydrants/Index.cshtml:
Line 98:
Stored XSS in Web/Resgrid.Web/Areas/User/Views/RecordHydrants/Index.cshtml: HydrantNumber is concatenated directly into the Leaflet popup HTML string, so markup or event handlers in stored data are injected by bindPopup. HTML-encode the popup content or build the popup with DOM text nodes instead of raw HTML.
Suggested Code:
var link = document.createElement('a');
link.href = '@Url.Action("Details", "RecordHydrants", new { area = "User" })?id=' + encodeURIComponent(p.HydrantId);
link.textContent = p.HydrantNumber || '';
var container = document.createElement('div');
container.appendChild(link);
if (p.FlowGpm) { container.appendChild(document.createElement('br')); container.appendChild(document.createTextNode(p.FlowGpm + ' gpm')); }
if (!p.InService) { container.appendChild(document.createElement('br')); var status = document.createElement('b'); status.textContent = '@L["OutOfService"]'; container.appendChild(status); }
m.bindPopup(container);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @{ | ||
| ViewBag.Title = "Resgrid | " + L["Inspections"]; | ||
| var now = DateTime.UtcNow; | ||
| string Occ(string id) => Model.OccupancyNames.TryGetValue(id ?? "", out var n) ? n : id; |
There was a problem hiding this comment.
Nullability mismatch in Web/Resgrid.Web/Areas/User/Views/RecordInspections/Index.cshtml and the related locations listed: Occ(string id) accepts a non-null parameter but already compensates with id ?? "", which indicates nullable input and risks unsafe rendering paths. Make the parameter string? and coalesce both lookup and fallback output to string.Empty.
Kody rule violation: Add null checks to prevent NullReferenceException
string Occ(string? id) => Model.OccupancyNames.TryGetValue(id ?? string.Empty, out var n) ? n : (id ?? string.Empty);Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/RecordInspections/Index.cshtml:
Line 8:
Nullability mismatch in Web/Resgrid.Web/Areas/User/Views/RecordInspections/Index.cshtml and the related locations listed: Occ(string id) accepts a non-null parameter but already compensates with id ?? "", which indicates nullable input and risks unsafe rendering paths. Make the parameter string? and coalesce both lookup and fallback output to string.Empty.
Suggested Code:
string Occ(string? id) => Model.OccupancyNames.TryGetValue(id ?? string.Empty, out var n) ? n : (id ?? string.Empty);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <div class="form-group"><label class="col-sm-3 control-label">@L["FrequencyMonths"]</label><div class="col-sm-9"><input class="form-control" type="number" min="1" max="120" asp-for="Editing.FrequencyMonths" /></div></div> | ||
| <div class="form-group"><label class="col-sm-3 control-label">@L["OccupancyTypesApply"]</label><div class="col-sm-9"><input class="form-control" asp-for="Editing.OccupancyTypesCsv" maxlength="200" placeholder="1,2,5" /><span class="help-block">@L["OccupancyTypesApplyHelp"] @string.Join(", ", Model.OccupancyTypes.Select(t => t.Value + "=" + t.Text))</span></div></div> | ||
| <div class="form-group"><label class="col-sm-3 control-label">@L["CodeSets"]</label><div class="col-sm-9"><select class="form-control" asp-for="Editing.RmsCodeSetId" asp-items="Model.CodeSetItems"></select></div></div> | ||
| <div class="form-group"><label class="col-sm-3 control-label">@L["Checklist"]</label><div class="col-sm-9"><textarea class="form-control" asp-for="ChecklistText" rows="10" style="font-family:monospace"></textarea><span class="help-block">@L["ChecklistHelp"]</span></div></div> |
There was a problem hiding this comment.
Inline style leakage in Web/Resgrid.Web/Areas/User/Views/RecordInspections/Programs.cshtml and the related locations listed: style="font-family:monospace" embeds presentation details directly in the view. Move the styling to a scoped CSS class such as checklist-textarea.
Kody rule violation: Use component-scoped styling
<div class="form-group"><label class="col-sm-3 control-label">@L["Checklist"]</label><div class="col-sm-9"><textarea class="form-control checklist-textarea" asp-for="ChecklistText" rows="10"></textarea><span class="help-block">@L["ChecklistHelp"]</span></div></div>Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/RecordInspections/Programs.cshtml:
Line 48:
Inline style leakage in Web/Resgrid.Web/Areas/User/Views/RecordInspections/Programs.cshtml and the related locations listed: style="font-family:monospace" embeds presentation details directly in the view. Move the styling to a scoped CSS class such as checklist-textarea.
Suggested Code:
<div class="form-group"><label class="col-sm-3 control-label">@L["Checklist"]</label><div class="col-sm-9"><textarea class="form-control checklist-textarea" asp-for="ChecklistText" rows="10"></textarea><span class="help-block">@L["ChecklistHelp"]</span></div></div>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @Html.AntiForgeryToken()<input type="hidden" name="id" value="@c.RmsInvestigationCaseId" /><input type="hidden" name="rowVersion" value="@c.RowVersion" /> | ||
| <div class="form-group"><label class="col-sm-3 control-label">@L["Title"]</label><div class="col-sm-9"><input class="form-control" name="title" value="@Text(c.Title)" maxlength="200" readonly="@(!Model.CanWrite)" /></div></div> | ||
| <div class="form-group"><label class="col-sm-3 control-label">@L["OpenedOn"]</label><div class="col-sm-9"><p class="form-control-static">@RmsEnumDisplay.Utc(c.OpenedOn) · @Model.UserName(c.OpenedByUserId)</p></div></div> | ||
| <div class="form-group"><label class="col-sm-3 control-label">@L["LeadInvestigator"]</label><div class="col-sm-9">@if (Model.IsLead && !c.IsClosed) { <select class="form-control" name="leadInvestigatorUserId">@foreach (var m in Model.Members) { <option value="@m.Value" selected="@(m.Value == c.LeadInvestigatorUserId)">@m.Text</option> }</select> } else { <p class="form-control-static">@Model.UserName(c.LeadInvestigatorUserId)</p><input type="hidden" name="leadInvestigatorUserId" value="@c.LeadInvestigatorUserId" /> }</div></div> |
There was a problem hiding this comment.
Privilege escalation in Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Details.cshtml: the form posts leadInvestigatorUserId for non-lead investigators through a hidden field, and RecordInvestigationsController.Update forwards that value into RecordsInvestigationsService.UpdateAsync before the service overwrites LeadInvestigatorUserId. Stop posting leadInvestigatorUserId for non-leads and enforce the lead-only authorization check server-side before applying any lead-investigator change.
<div class="form-group"><label class="col-sm-3 control-label">@L["LeadInvestigator"]</label><div class="col-sm-9">@if (Model.IsLead && !c.IsClosed) { <select class="form-control" name="leadInvestigatorUserId">@foreach (var m in Model.Members) { <option value="@m.Value" selected="@(m.Value == c.LeadInvestigatorUserId)">@m.Text</option> }</select> } else { <p class="form-control-static">@Model.UserName(c.LeadInvestigatorUserId)</p> }</div></div>Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Details.cshtml:
Line 40:
Privilege escalation in Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Details.cshtml: the form posts leadInvestigatorUserId for non-lead investigators through a hidden field, and RecordInvestigationsController.Update forwards that value into RecordsInvestigationsService.UpdateAsync before the service overwrites LeadInvestigatorUserId. Stop posting leadInvestigatorUserId for non-leads and enforce the lead-only authorization check server-side before applying any lead-investigator change.
Suggested Code:
<div class="form-group"><label class="col-sm-3 control-label">@L["LeadInvestigator"]</label><div class="col-sm-9">@if (Model.IsLead && !c.IsClosed) { <select class="form-control" name="leadInvestigatorUserId">@foreach (var m in Model.Members) { <option value="@m.Value" selected="@(m.Value == c.LeadInvestigatorUserId)">@m.Text</option> }</select> } else { <p class="form-control-static">@Model.UserName(c.LeadInvestigatorUserId)</p> }</div></div>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| </div> | ||
| <div class="col-sm-6"> | ||
| <div class="btn-group top-page-buttons" style="float:right;padding-right:15px;"> | ||
| <a class="btn btn-default" asp-controller="RecordInvestigations" asp-action="Export" asp-route-area="User" asp-route-id="@c.RmsInvestigationCaseId"><i class="fa fa-download"></i> @L["ExportCase"]</a> |
There was a problem hiding this comment.
Sensitive export authorization gap in Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Details.cshtml: the ExportCase action is exposed without any visible consent or lawful-basis workflow for sensitive investigation or health-related data. Require an explicit consent record, or equivalent authorization artifact, and pass its identifier into the export operation.
Kody rule violation: Require explicit consent before processing sensitive data
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Details.cshtml:
Line 24:
Sensitive export authorization gap in Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Details.cshtml: the ExportCase action is exposed without any visible consent or lawful-basis workflow for sensitive investigation or health-related data. Require an explicit consent record, or equivalent authorization artifact, and pass its identifier into the export operation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| </div> | ||
| <div class="col-sm-6"> | ||
| <div class="btn-group top-page-buttons" style="float:right;padding-right:15px;"> | ||
| <a class="btn btn-success" asp-controller="RecordInvestigations" asp-action="Open" asp-route-area="User"><i class="fa fa-plus"></i> @L["OpenCase"]</a> |
There was a problem hiding this comment.
Authorization bypass in Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Index.cshtml: the Open case entry point is visible to any user who can reach the investigations index, while RecordInvestigationsController is limited to RecordRestricted_View and RecordsInvestigationsService.OpenAsync only calls RequireRestrictedAsync before creating the case and assigning the caller as lead. Gate the Open link behind the same write-role check used elsewhere and require stronger server-side authorization than restricted-view for Open.
@if (Model.CanAdminister)
{
<a class="btn btn-success" asp-controller="RecordInvestigations" asp-action="Open" asp-route-area="User"><i class="fa fa-plus"></i> @L["OpenCase"]</a>
}Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Index.cshtml:
Line 19:
Authorization bypass in Web/Resgrid.Web/Areas/User/Views/RecordInvestigations/Index.cshtml: the Open case entry point is visible to any user who can reach the investigations index, while RecordInvestigationsController is limited to RecordRestricted_View and RecordsInvestigationsService.OpenAsync only calls RequireRestrictedAsync before creating the case and assigning the caller as lead. Gate the Open link behind the same write-role check used elsewhere and require stronger server-side authorization than restricted-view for Open.
Suggested Code:
@if (Model.CanAdminister)
{
<a class="btn btn-success" asp-controller="RecordInvestigations" asp-action="Open" asp-route-area="User"><i class="fa fa-plus"></i> @L["OpenCase"]</a>
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| @if (Has(Model.TacticalSummary)) | ||
| { | ||
| <div class="alert alert-info"><strong>@localizer["TacticalSummary"]:</strong> <span data-adp-field="@Adp("tacticalsummary")">@Model.TacticalSummary</span></div> |
There was a problem hiding this comment.
Sensitive data exposure in Web/Resgrid.Web/Areas/User/Views/Shared/_ContactPreplanSummary.cshtml and the related locations listed: @Model.TacticalSummary renders operational preplan content that can qualify as PHI/ePHI without minimization. Render only the minimum necessary metadata or a redacted placeholder unless the view is explicitly covered by PHI handling and auditing controls.
Kody rule violation: Do not log PHI; mask and drop sensitive fields
<div class="alert alert-info"><strong>@localizer["TacticalSummary"]:</strong> <span data-adp-field="@Adp("tacticalsummary")">[REDACTED]</span></div>Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Shared/_ContactPreplanSummary.cshtml:
Line 23:
Sensitive data exposure in Web/Resgrid.Web/Areas/User/Views/Shared/_ContactPreplanSummary.cshtml and the related locations listed: @Model.TacticalSummary renders operational preplan content that can qualify as PHI/ePHI without minimization. Render only the minimum necessary metadata or a redacted placeholder unless the view is explicitly covered by PHI handling and auditing controls.
Suggested Code:
<div class="alert alert-info"><strong>@localizer["TacticalSummary"]:</strong> <span data-adp-field="@Adp("tacticalsummary")">[REDACTED]</span></div>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| try | ||
| { | ||
| var prevention = await scope.Resolve<IRecordsPreventionSweepService>().SweepAsync(cancellationToken); | ||
| Logging.LogInfo($"Prevention sweep: departments={prevention.DepartmentsEvaluated} inspections={prevention.InspectionsGenerated} violationsOverdue={prevention.ViolationsBecameOverdue} permitsNotified={prevention.PermitsExpiringNotified} permitsExpired={prevention.PermitsExpired} errors={prevention.Errors}"); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logging.LogException(ex, "Prevention sweep failed; the due-state result still stands."); |
There was a problem hiding this comment.
Error suppression in Workers/Resgrid.Workers.Framework/Logic/RmsDueStateEvaluationLogic.cs: the prevention sweep exception is caught, logged, and discarded, so Process still reports success while inspections, violations, and permit expiry processing remain stale. Propagate the failure through the returned tuple or rethrow after logging so job monitoring and retries can detect the failed prevention sweep.
var prevention = await scope.Resolve<IRecordsPreventionSweepService>().SweepAsync(cancellationToken);
Logging.LogInfo($"Prevention sweep: departments={prevention.DepartmentsEvaluated} inspections={prevention.InspectionsGenerated} violationsOverdue={prevention.ViolationsBecameOverdue} permitsNotified={prevention.PermitsExpiringNotified} permitsExpired={prevention.PermitsExpired} errors={prevention.Errors}");Prompt for LLM
File Workers/Resgrid.Workers.Framework/Logic/RmsDueStateEvaluationLogic.cs:
Line 26 to 33:
Error suppression in Workers/Resgrid.Workers.Framework/Logic/RmsDueStateEvaluationLogic.cs: the prevention sweep exception is caught, logged, and discarded, so Process still reports success while inspections, violations, and permit expiry processing remain stale. Propagate the failure through the returned tuple or rethrow after logging so job monitoring and retries can detect the failed prevention sweep.
Suggested Code:
var prevention = await scope.Resolve<IRecordsPreventionSweepService>().SweepAsync(cancellationToken);
Logging.LogInfo($"Prevention sweep: departments={prevention.DepartmentsEvaluated} inspections={prevention.InspectionsGenerated} violationsOverdue={prevention.ViolationsBecameOverdue} permitsNotified={prevention.PermitsExpiringNotified} permitsExpired={prevention.PermitsExpired} errors={prevention.Errors}");
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 pull request adds a major new RMS prevention and investigations foundation, expands Contacts with structured pre-plans and site files, hardens external records connectors, and improves release safety/telemetry for Records.
What changed
Contacts now support structured pre-plans, hazards, and site files
Call site information now surfaces linked contact/site context
RMS prevention modules were introduced
Added new Records modules and service layers for prevention and related workflows, behind feature flags:
Occupancies
Inspections
Hydrants
Permits
Community Risk Reduction
Investigations
Quality Review
New permissions and feature flags
Protected data coverage was expanded
Records release telemetry and health were added
Record definition cardinality is now configurable and enforced
Connector hardening and concurrency protections
Field records resilience improvements
Functional impact
These changes introduce the core prevention/investigations RMS surface, let departments manage structured site pre-plan data directly on contacts, expose richer site context during dispatch, strengthen outbound connector safety, and improve operational safety through telemetry, concurrency guards, and retry-friendly failure handling.