Conversation
|
Important Review skippedToo many files! This PR contains 163 files, which is 13 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 (60)
📒 Files selected for processing (163)
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}
| AttestationStatementVersion = string.IsNullOrWhiteSpace(input.AttestationStatementVersion) ? "1" : input.AttestationStatementVersion, | ||
| input.NewOwnerUserId, Origin = origin | ||
| })); | ||
| if (key != null) |
| [ResponseCache(NoStore = true, Location = ResponseCacheLocation.None)] | ||
| public async Task<ActionResult<RecordResult>> CreateRunCall(CreateRunCallInput input, CancellationToken cancellationToken) | ||
| { | ||
| if (input == null || string.IsNullOrWhiteSpace(input.RecordId)) return BadRequest(); |
| [HttpPost("CreateRunCall")] | ||
| [Authorize(Policy = ResgridResources.Record_Create)] | ||
| [ResponseCache(NoStore = true, Location = ResponseCacheLocation.None)] | ||
| public async Task<ActionResult<RecordResult>> CreateRunCall(CreateRunCallInput input, CancellationToken cancellationToken) |
| </form> | ||
| @if (selection.NextSequence.HasValue) | ||
| { | ||
| <p><a asp-action="Select" asp-route-recordId="@context.RecordId" asp-route-recordKind="@context.RecordKind" asp-route-sourceKind="@selection.SourceKind" asp-route-channelId="@selection.ChannelId" asp-route-afterSequence="@selection.NextSequence">Next message page (uncaptured selections will be cleared)</a></p> |
| </form> | ||
| @if (selection.NextSequence.HasValue) | ||
| { | ||
| <p><a asp-action="Select" asp-route-recordId="@context.RecordId" asp-route-recordKind="@context.RecordKind" asp-route-sourceKind="@selection.SourceKind" asp-route-channelId="@selection.ChannelId" asp-route-afterSequence="@selection.NextSequence">Next message page (uncaptured selections will be cleared)</a></p> |
| } | ||
| <div class="wrapper wrapper-content"> | ||
| <h2>Supporting evidence — @context.RecordNumber</h2> | ||
| <p><a asp-controller="@recordController" asp-action="Details" asp-route-id="@context.RecordId">Return to report</a></p> |
|
|
||
| [HttpPost("Submissions/{submissionId}/Reconcile")] | ||
| [Authorize(Policy = ResgridResources.Record_Submit)] | ||
| public async Task<IActionResult> Reconcile(string submissionId, [FromBody] RmsSubmissionReconciliationInput input, CancellationToken cancellationToken) |
| catch (Exception ex) when (ex is ArgumentException || ex is InvalidOperationException) { return Problem(ex.Message, statusCode: 409); } | ||
| } | ||
| [HttpPost("Release")] | ||
| public async Task<IActionResult> Release([FromBody] ReleaseHoldInput input, CancellationToken cancellationToken) |
| try { Response.Headers["Cache-Control"] = "no-store"; return Ok(await _holds.GetAsync(DepartmentId, UserId)); } catch (UnauthorizedAccessException) { return Forbid(); } | ||
| } | ||
| [HttpPost("Place")] | ||
| public async Task<IActionResult> Place([FromBody] RmsRecordLegalHold input, CancellationToken cancellationToken) |
| return await Allowed(recordId) ? Ok(usage) : Forbid(); | ||
| } | ||
| [HttpPost("Consume")] | ||
| public async Task<IActionResult> Consume(ConsumeInput input,CancellationToken cancellationToken) |
| /// <summary>Serializes each index mutation with retention and verifies the current SQL source before invoking the synchronous index writer.</summary> | ||
| public interface IRmsSearchWriteFence | ||
| { | ||
| Task<int> WithLiveSourceAsync(RecordsSearchDocumentSource source, Func<RecordsSearchDocumentSource, int> write, CancellationToken cancellationToken = default); |
There was a problem hiding this comment.
Async API mismatch in Core/Resgrid.Model/Repositories/IRmsSearchWriteFence.cs and the listed implementations and callers: WithLiveSourceAsync accepts Func<RecordsSearchDocumentSource, int> write even though the API itself is async, which forces synchronous delegates into an asynchronous flow and blocks non-blocking I/O implementations. Change the delegate to Func<RecordsSearchDocumentSource, Task> or ValueTask and await it in implementations.
Existing code:
Task<int> WithLiveSourceAsync(RecordsSearchDocumentSource source, Func<RecordsSearchDocumentSource, int> write, CancellationToken cancellationToken = default);
Improved code:
Task<int> WithLiveSourceAsync(RecordsSearchDocumentSource source, Func<RecordsSearchDocumentSource, Task<int>> write, CancellationToken cancellationToken = default);
Kody rule violation: Use Awaitable Methods in Async Code
Task<int> WithLiveSourceAsync(RecordsSearchDocumentSource source, Func<RecordsSearchDocumentSource, Task<int>> write, CancellationToken cancellationToken = default);Prompt for LLM
File Core/Resgrid.Model/Repositories/IRmsSearchWriteFence.cs:
Line 10:
Async API mismatch in Core/Resgrid.Model/Repositories/IRmsSearchWriteFence.cs and the listed implementations and callers: WithLiveSourceAsync accepts Func<RecordsSearchDocumentSource, int> write even though the API itself is async, which forces synchronous delegates into an asynchronous flow and blocks non-blocking I/O implementations. Change the delegate to Func<RecordsSearchDocumentSource, Task<int>> or ValueTask<int> and await it in implementations.
Existing code:
Task<int> WithLiveSourceAsync(RecordsSearchDocumentSource source, Func<RecordsSearchDocumentSource, int> write, CancellationToken cancellationToken = default);
Improved code:
Task<int> WithLiveSourceAsync(RecordsSearchDocumentSource source, Func<RecordsSearchDocumentSource, Task<int>> write, CancellationToken cancellationToken = default);
Suggested Code:
Task<int> WithLiveSourceAsync(RecordsSearchDocumentSource source, Func<RecordsSearchDocumentSource, Task<int>> write, CancellationToken cancellationToken = default);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| private readonly object _sync = new object(); | ||
| private readonly Directory _directory; | ||
| private readonly bool _ownsDirectory; | ||
| private Directory _directory; |
There was a problem hiding this comment.
Mutability regression in Core/Resgrid.Search/LuceneRecordsIndexHost.cs, Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs:82-83, Core/Resgrid.Model/Records/RmsRecordAttachment.cs:86-86, Repositories/Resgrid.Repositories.DataRepository/RmsRetentionRepository.cs:25-25, Tests/Resgrid.Tests/Rms/FakeRmsStore.cs:257-257, Core/Resgrid.Services/Records/RecordsDocumentService.cs:179-179, Tests/Resgrid.Tests/Rms/FakeRmsStore.cs:290-290, and Tests/Resgrid.Tests/Rms/FakeRmsStore.cs:337-337: _directory was changed from readonly to mutable without evidence that reassignment is required after construction. Restore readonly to communicate immutability and prevent accidental reference mutation.
Existing code:
private Directory _directory;
Improved code:
private readonly Directory _directory;
Kody rule violation: Use `readonly` or `const` for Immutable Data
private readonly Directory _directory;Prompt for LLM
File Core/Resgrid.Search/LuceneRecordsIndexHost.cs:
Line 24:
Mutability regression in Core/Resgrid.Search/LuceneRecordsIndexHost.cs, Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs:82-83, Core/Resgrid.Model/Records/RmsRecordAttachment.cs:86-86, Repositories/Resgrid.Repositories.DataRepository/RmsRetentionRepository.cs:25-25, Tests/Resgrid.Tests/Rms/FakeRmsStore.cs:257-257, Core/Resgrid.Services/Records/RecordsDocumentService.cs:179-179, Tests/Resgrid.Tests/Rms/FakeRmsStore.cs:290-290, and Tests/Resgrid.Tests/Rms/FakeRmsStore.cs:337-337: _directory was changed from readonly to mutable without evidence that reassignment is required after construction. Restore readonly to communicate immutability and prevent accidental reference mutation.
Existing code:
private Directory _directory;
Improved code:
private readonly Directory _directory;
Suggested Code:
private readonly Directory _directory;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| var text = (string)property.Value; | ||
| if (text?.TrimStart().StartsWith("{") == true || text?.TrimStart().StartsWith("[") == true) | ||
| try { property.Value = JToken.Parse(text); } catch (JsonException) { } |
There was a problem hiding this comment.
Silent exception suppression in Core/Resgrid.Services/Records/DisclosureContentPolicy.cs, Core/Resgrid.Services/Records/RecordsDocumentService.cs:233-233, Core/Resgrid.Services/Records/RecordsSubmissionService.cs:198-198, Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:273-273, and Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:371-371: catch (JsonException) { } discards parse failures without context, obscuring malformed JSON and data-quality defects. Log the JsonException with payload context and either rethrow or handle the invalid value explicitly.
Existing code:
try { property.Value = JToken.Parse(text); } catch (JsonException) { }
Improved code:
(none)
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File Core/Resgrid.Services/Records/DisclosureContentPolicy.cs:
Line 27:
Silent exception suppression in Core/Resgrid.Services/Records/DisclosureContentPolicy.cs, Core/Resgrid.Services/Records/RecordsDocumentService.cs:233-233, Core/Resgrid.Services/Records/RecordsSubmissionService.cs:198-198, Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:273-273, and Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:371-371: catch (JsonException) { } discards parse failures without context, obscuring malformed JSON and data-quality defects. Log the JsonException with payload context and either rethrow or handle the invalid value explicitly.
Existing code:
try { property.Value = JToken.Parse(text); } catch (JsonException) { }
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| public async Task<RecordEvidenceCapture> CaptureAsync(RecordEvidenceCaptureRequest request, CancellationToken cancellationToken = default) | ||
| { | ||
| var units = (request.UnitIds ?? new List<int>()).Where(u => u > 0).Distinct().ToList(); | ||
| if (units.Count > EvidenceLimits.MaxItems / SamplesPerUnit) throw new ArgumentException("Select at most " + EvidenceLimits.MaxItems / SamplesPerUnit + " units per tracking capture."); |
There was a problem hiding this comment.
Incorrect rule classification in Core/Resgrid.Services/Records/Evidence/RecordEvidenceAdapters.cs and RecordEvidenceAdapters.cs:292-292: the flagged line only builds an ArgumentException message using EvidenceLimits.MaxItems / SamplesPerUnit and units.Count, not a SQL query or user-controlled SQL fragment. Remove or correct this SQL injection finding so review noise does not obscure real query risks.
Existing code:
if (units.Count > EvidenceLimits.MaxItems / SamplesPerUnit) throw new ArgumentException("Select at most " + EvidenceLimits.MaxItems / SamplesPerUnit + " units per tracking capture.");
Improved code:
(none)
Kody rule violation: Prevent SQL Injection in Queries
Prompt for LLM
File Core/Resgrid.Services/Records/Evidence/RecordEvidenceAdapters.cs:
Line 186:
Incorrect rule classification in Core/Resgrid.Services/Records/Evidence/RecordEvidenceAdapters.cs and RecordEvidenceAdapters.cs:292-292: the flagged line only builds an ArgumentException message using EvidenceLimits.MaxItems / SamplesPerUnit and units.Count, not a SQL query or user-controlled SQL fragment. Remove or correct this SQL injection finding so review noise does not obscure real query risks.
Existing code:
if (units.Count > EvidenceLimits.MaxItems / SamplesPerUnit) throw new ArgumentException("Select at most " + EvidenceLimits.MaxItems / SamplesPerUnit + " units per tracking capture.");
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| await _audits.InsertAsync(new RmsAccessAudit { DepartmentId = departmentId, RecordId = reportId, ActorUserId = userId, Action = (int)RmsAccessAuditAction.Change, | ||
| Purpose = "Incident attachment uploaded", Successful = true, OccurredOn = now, DetailJson = JsonConvert.SerializeObject(new { attachment.RmsRecordAttachmentId, attachment.Checksum, attachment.ByteSize }) }, cancellationToken, true); |
There was a problem hiding this comment.
Tamper-evident audit gap in Core/Resgrid.Services/Records/IncidentAttachmentsService.cs and IncidentAttachmentsService.cs:109-110: the attachment upload audit record omits required structured fields such as actor.role, resource.id shape, result, trace_id, ip, and user_agent, reducing forensic value for a security-relevant event. Add the mandated fields to DetailJson and preserve immutable audit storage.
Existing code:
await _audits.InsertAsync(new RmsAccessAudit { DepartmentId = departmentId, RecordId = reportId, ActorUserId = userId, Action = (int)RmsAccessAuditAction.Change,
Purpose = "Incident attachment uploaded", Successful = true, OccurredOn = now, DetailJson = JsonConvert.SerializeObject(new { attachment.RmsRecordAttachmentId, attachment.Checksum, attachment.ByteSize }) }, cancellationToken, true);
Improved code:
await _audits.InsertAsync(new RmsAccessAudit {
DepartmentId = departmentId,
RecordId = reportId,
ActorUserId = userId,
Action = (int)RmsAccessAuditAction.Change,
Purpose = "Incident attachment uploaded",
Successful = true,
OccurredOn = now,
DetailJson = JsonConvert.SerializeObject(new {
action = "incident_attachment.upload",
resource = new { id = reportId },
actor = new { user_id = userId, role = /* role */ null },
result = "success",
trace_id = /* trace id */ "",
ip = /* ip */ "",
user_agent = /* user agent */ "",
attachmentId = attachment.RmsRecordAttachmentId,
checksum = attachment.Checksum,
byteSize = attachment.ByteSize
})
}, cancellationToken, true);
Kody rule violation: Emit tamper-evident audit logs with required fields
await _audits.InsertAsync(new RmsAccessAudit {
DepartmentId = departmentId,
RecordId = reportId,
ActorUserId = userId,
Action = (int)RmsAccessAuditAction.Change,
Purpose = "Incident attachment uploaded",
Successful = true,
OccurredOn = now,
DetailJson = JsonConvert.SerializeObject(new {
action = "incident_attachment.upload",
resource = new { id = reportId },
actor = new { user_id = userId, role = /* role */ null },
result = "success",
trace_id = /* trace id */ "",
ip = /* ip */ "",
user_agent = /* user agent */ "",
attachmentId = attachment.RmsRecordAttachmentId,
checksum = attachment.Checksum,
byteSize = attachment.ByteSize
})
}, cancellationToken, true);Prompt for LLM
File Core/Resgrid.Services/Records/IncidentAttachmentsService.cs:
Line 68 to 69:
Tamper-evident audit gap in Core/Resgrid.Services/Records/IncidentAttachmentsService.cs and IncidentAttachmentsService.cs:109-110: the attachment upload audit record omits required structured fields such as actor.role, resource.id shape, result, trace_id, ip, and user_agent, reducing forensic value for a security-relevant event. Add the mandated fields to DetailJson and preserve immutable audit storage.
Existing code:
await _audits.InsertAsync(new RmsAccessAudit { DepartmentId = departmentId, RecordId = reportId, ActorUserId = userId, Action = (int)RmsAccessAuditAction.Change,
Purpose = "Incident attachment uploaded", Successful = true, OccurredOn = now, DetailJson = JsonConvert.SerializeObject(new { attachment.RmsRecordAttachmentId, attachment.Checksum, attachment.ByteSize }) }, cancellationToken, true);
Improved code:
await _audits.InsertAsync(new RmsAccessAudit {
DepartmentId = departmentId,
RecordId = reportId,
ActorUserId = userId,
Action = (int)RmsAccessAuditAction.Change,
Purpose = "Incident attachment uploaded",
Successful = true,
OccurredOn = now,
DetailJson = JsonConvert.SerializeObject(new {
action = "incident_attachment.upload",
resource = new { id = reportId },
actor = new { user_id = userId, role = /* role / null },
result = "success",
trace_id = / trace id / "",
ip = / ip / "",
user_agent = / user agent */ "",
attachmentId = attachment.RmsRecordAttachmentId,
checksum = attachment.Checksum,
byteSize = attachment.ByteSize
})
}, cancellationToken, true);
Suggested Code:
await _audits.InsertAsync(new RmsAccessAudit {
DepartmentId = departmentId,
RecordId = reportId,
ActorUserId = userId,
Action = (int)RmsAccessAuditAction.Change,
Purpose = "Incident attachment uploaded",
Successful = true,
OccurredOn = now,
DetailJson = JsonConvert.SerializeObject(new {
action = "incident_attachment.upload",
resource = new { id = reportId },
actor = new { user_id = userId, role = /* role */ null },
result = "success",
trace_id = /* trace id */ "",
ip = /* ip */ "",
user_agent = /* user agent */ "",
attachmentId = attachment.RmsRecordAttachmentId,
checksum = attachment.Checksum,
byteSize = attachment.ByteSize
})
}, cancellationToken, true);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // Recheck earlier choices after the last awaited source read. No caller-provided IDs grant access. | ||
| foreach (var choice in selection.Choices) | ||
| { | ||
| if (sourceKind == RmsEvidenceKind.TrackingFix && !await _sourceAuthorization.Value.CanUserViewUnitLocationAsync(userId, int.Parse(choice.Id, CultureInfo.InvariantCulture), departmentId)) throw new UnauthorizedAccessException(); |
There was a problem hiding this comment.
Unsafe parsing in Core/Resgrid.Services/Records/RecordEvidenceSelectionService.cs and Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:110-110: int.Parse(choice.Id, CultureInfo.InvariantCulture) treats choice.Id as trusted input and can throw on malformed values before UnauthorizedAccessException handling runs. Use a TryParse-based validation path for choice.Id and reject invalid format explicitly.
Existing code:
if (sourceKind == RmsEvidenceKind.TrackingFix && !await _sourceAuthorization.Value.CanUserViewUnitLocationAsync(userId, int.Parse(choice.Id, CultureInfo.InvariantCulture), departmentId)) throw new UnauthorizedAccessException();
Improved code:
(none)
Kody rule violation: Use TryParse for string conversions
Prompt for LLM
File Core/Resgrid.Services/Records/RecordEvidenceSelectionService.cs:
Line 120:
Unsafe parsing in Core/Resgrid.Services/Records/RecordEvidenceSelectionService.cs and Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:110-110: int.Parse(choice.Id, CultureInfo.InvariantCulture) treats choice.Id as trusted input and can throw on malformed values before UnauthorizedAccessException handling runs. Use a TryParse-based validation path for choice.Id and reject invalid format explicitly.
Existing code:
if (sourceKind == RmsEvidenceKind.TrackingFix && !await _sourceAuthorization.Value.CanUserViewUnitLocationAsync(userId, int.Parse(choice.Id, CultureInfo.InvariantCulture), departmentId)) throw new UnauthorizedAccessException();
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (sourceKind == RmsEvidenceKind.TrackingFix) | ||
| { | ||
| foreach (var unit in (await _units.GetUnitsForDepartmentAsync(departmentId)).Where(u => u.DepartmentId == departmentId)) | ||
| if (await _sourceAuthorization.Value.CanUserViewUnitLocationAsync(userId, unit.UnitId, departmentId)) | ||
| selection.Choices.Add(new RecordEvidenceChoice { Id = unit.UnitId.ToString(CultureInfo.InvariantCulture), Label = unit.Name }); | ||
| } | ||
| else if (sourceKind == RmsEvidenceKind.CertificationSnapshot) | ||
| { | ||
| foreach (var person in await _departments.GetAllPersonnelNamesForDepartmentAsync(departmentId)) | ||
| if (await _sourceAuthorization.Value.CanUserViewPersonAsync(userId, person.UserId, departmentId)) | ||
| selection.Choices.Add(new RecordEvidenceChoice { Id = person.UserId, Label = person.Name }); |
There was a problem hiding this comment.
N+1 authorization pattern in Core/Resgrid.Services/Records/RecordEvidenceSelectionService.cs: opening the RmsEvidenceKind.TrackingFix or RmsEvidenceKind.CertificationSnapshot picker iterates all department units or personnel and awaits _sourceAuthorization.Value.CanUserViewUnitLocationAsync or CanUserViewPersonAsync for each row, making selection-page load O(N) backend calls. Fetch caller-scoped candidates up front or add a batched authorization API so large departments do not incur avoidable UI latency and authorization-service load.
Existing code:
if (sourceKind == RmsEvidenceKind.TrackingFix)
{
foreach (var unit in (await _units.GetUnitsForDepartmentAsync(departmentId)).Where(u => u.DepartmentId == departmentId))
if (await _sourceAuthorization.Value.CanUserViewUnitLocationAsync(userId, unit.UnitId, departmentId))
selection.Choices.Add(new RecordEvidenceChoice { Id = unit.UnitId.ToString(CultureInfo.InvariantCulture), Label = unit.Name });
}
else if (sourceKind == RmsEvidenceKind.CertificationSnapshot)
{
foreach (var person in await _departments.GetAllPersonnelNamesForDepartmentAsync(departmentId))
if (await _sourceAuthorization.Value.CanUserViewPersonAsync(userId, person.UserId, departmentId))
selection.Choices.Add(new RecordEvidenceChoice { Id = person.UserId, Label = person.Name });
}
Improved code:
// Example direction: replace per-row checks with a batched or pre-scoped query.
var visibleUnits = await _sourceAuthorization.Value.GetVisibleUnitLocationsAsync(userId, departmentId);
selection.Choices.AddRange(visibleUnits.Select(unit => new RecordEvidenceChoice
{
Id = unit.UnitId.ToString(CultureInfo.InvariantCulture),
Label = unit.Name
}));
var visiblePeople = await _sourceAuthorization.Value.GetVisiblePeopleAsync(userId, departmentId);
selection.Choices.AddRange(visiblePeople.Select(person => new RecordEvidenceChoice
{
Id = person.UserId,
Label = person.Name
}));
// Example direction: replace per-row checks with a batched or pre-scoped query.
var visibleUnits = await _sourceAuthorization.Value.GetVisibleUnitLocationsAsync(userId, departmentId);
selection.Choices.AddRange(visibleUnits.Select(unit => new RecordEvidenceChoice
{
Id = unit.UnitId.ToString(CultureInfo.InvariantCulture),
Label = unit.Name
}));
var visiblePeople = await _sourceAuthorization.Value.GetVisiblePeopleAsync(userId, departmentId);
selection.Choices.AddRange(visiblePeople.Select(person => new RecordEvidenceChoice
{
Id = person.UserId,
Label = person.Name
}));Prompt for LLM
File Core/Resgrid.Services/Records/RecordEvidenceSelectionService.cs:
Line 83 to 93:
N+1 authorization pattern in Core/Resgrid.Services/Records/RecordEvidenceSelectionService.cs: opening the RmsEvidenceKind.TrackingFix or RmsEvidenceKind.CertificationSnapshot picker iterates all department units or personnel and awaits _sourceAuthorization.Value.CanUserViewUnitLocationAsync or CanUserViewPersonAsync for each row, making selection-page load O(N) backend calls. Fetch caller-scoped candidates up front or add a batched authorization API so large departments do not incur avoidable UI latency and authorization-service load.
Existing code:
if (sourceKind == RmsEvidenceKind.TrackingFix)
{
foreach (var unit in (await _units.GetUnitsForDepartmentAsync(departmentId)).Where(u => u.DepartmentId == departmentId))
if (await _sourceAuthorization.Value.CanUserViewUnitLocationAsync(userId, unit.UnitId, departmentId))
selection.Choices.Add(new RecordEvidenceChoice { Id = unit.UnitId.ToString(CultureInfo.InvariantCulture), Label = unit.Name });
}
else if (sourceKind == RmsEvidenceKind.CertificationSnapshot)
{
foreach (var person in await _departments.GetAllPersonnelNamesForDepartmentAsync(departmentId))
if (await _sourceAuthorization.Value.CanUserViewPersonAsync(userId, person.UserId, departmentId))
selection.Choices.Add(new RecordEvidenceChoice { Id = person.UserId, Label = person.Name });
}
Improved code:
// Example direction: replace per-row checks with a batched or pre-scoped query.
var visibleUnits = await _sourceAuthorization.Value.GetVisibleUnitLocationsAsync(userId, departmentId);
selection.Choices.AddRange(visibleUnits.Select(unit => new RecordEvidenceChoice
{
Id = unit.UnitId.ToString(CultureInfo.InvariantCulture),
Label = unit.Name
}));
var visiblePeople = await _sourceAuthorization.Value.GetVisiblePeopleAsync(userId, departmentId);
selection.Choices.AddRange(visiblePeople.Select(person => new RecordEvidenceChoice
{
Id = person.UserId,
Label = person.Name
}));
Suggested Code:
// Example direction: replace per-row checks with a batched or pre-scoped query.
var visibleUnits = await _sourceAuthorization.Value.GetVisibleUnitLocationsAsync(userId, departmentId);
selection.Choices.AddRange(visibleUnits.Select(unit => new RecordEvidenceChoice
{
Id = unit.UnitId.ToString(CultureInfo.InvariantCulture),
Label = unit.Name
}));
var visiblePeople = await _sourceAuthorization.Value.GetVisiblePeopleAsync(userId, departmentId);
selection.Choices.AddRange(visiblePeople.Select(person => new RecordEvidenceChoice
{
Id = person.UserId,
Label = person.Name
}));
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (produced.Count == 0 || produced.Count != production.RecordCount || manifest == null || manifest.Count != produced.Count) return null; | ||
| var restricted = (bool?)artifact["restricted_content_included"] ?? production.RedactionProfile == RmsRedactionProfiles.FullDisclosure; | ||
| var visibilityRequired=(int?)artifact["udf_visibility_required"] ?? 0; | ||
| var containsUdf=(artifact["documents"] as JArray ?? new JArray()).OfType<JObject>().Any(d=>(((d["content"] as JObject)?["CustomFields"] as JObject)?["Fields"] as JArray)?.Count>0); |
There was a problem hiding this comment.
Readability debt in Core/Resgrid.Services/Records/RecordsDisclosureService.cs and the listed controllers: the containsUdf expression compresses JArray navigation, JObject casts, and null-handling into a single dense statement, which makes review and maintenance error-prone. Split the artifact["documents"] traversal into intermediate expressions so the null-handling path remains verifiable.
Existing code:
var containsUdf=(artifact["documents"] as JArray ?? new JArray()).OfType<JObject>().Any(d=>(((d["content"] as JObject)?["CustomFields"] as JObject)?["Fields"] as JArray)?.Count>0);
Improved code:
var documents = artifact["documents"] as JArray ?? new JArray();
var containsUdf = documents
.OfType<JObject>()
.Any(d => ((((d["content"] as JObject)?["CustomFields"] as JObject)?["Fields"] as JArray)?.Count ?? 0) > 0);
Kody rule violation: Limit Lengthy LINQ Chains
var documents = artifact["documents"] as JArray ?? new JArray();
var containsUdf = documents
.OfType<JObject>()
.Any(d => ((((d["content"] as JObject)?["CustomFields"] as JObject)?["Fields"] as JArray)?.Count ?? 0) > 0);Prompt for LLM
File Core/Resgrid.Services/Records/RecordsDisclosureService.cs:
Line 231:
Readability debt in Core/Resgrid.Services/Records/RecordsDisclosureService.cs and the listed controllers: the containsUdf expression compresses JArray navigation, JObject casts, and null-handling into a single dense statement, which makes review and maintenance error-prone. Split the artifact["documents"] traversal into intermediate expressions so the null-handling path remains verifiable.
Existing code:
var containsUdf=(artifact["documents"] as JArray ?? new JArray()).OfType<JObject>().Any(d=>(((d["content"] as JObject)?["CustomFields"] as JObject)?["Fields"] as JArray)?.Count>0);
Improved code:
var documents = artifact["documents"] as JArray ?? new JArray();
var containsUdf = documents
.OfType<JObject>()
.Any(d => ((((d["content"] as JObject)?["CustomFields"] as JObject)?["Fields"] as JArray)?.Count ?? 0) > 0);
Suggested Code:
var documents = artifact["documents"] as JArray ?? new JArray();
var containsUdf = documents
.OfType<JObject>()
.Any(d => ((((d["content"] as JObject)?["CustomFields"] as JObject)?["Fields"] as JArray)?.Count ?? 0) > 0);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return bytes; | ||
| } | ||
| private static string E(string text) => WebUtility.HtmlEncode(text ?? ""); | ||
| private static string Label(string text) => Regex.Replace(text.Replace('_', ' '), "([a-z])([A-Z])", "$1 $2"); |
There was a problem hiding this comment.
Regex DoS risk in Core/Resgrid.Services/Records/RecordsDocumentService.cs, RecordsDocumentService.cs:263-263, and Web/Resgrid.Web/Areas/User/Controllers/DisclosuresController.cs:181-181: Label(string text) calls Regex.Replace without a timeout on untrusted input. Specify a timeout in the Regex operation to bound pathological backtracking.
Existing code:
private static string Label(string text) => Regex.Replace(text.Replace('_', ' '), "([a-z])([A-Z])", "$1 $2");
Improved code:
(none)
Kody rule violation: Specify Timeout for Regular Expressions
Prompt for LLM
File Core/Resgrid.Services/Records/RecordsDocumentService.cs:
Line 178:
Regex DoS risk in Core/Resgrid.Services/Records/RecordsDocumentService.cs, RecordsDocumentService.cs:263-263, and Web/Resgrid.Web/Areas/User/Controllers/DisclosuresController.cs:181-181: Label(string text) calls Regex.Replace without a timeout on untrusted input. Specify a timeout in the Regex operation to bound pathological backtracking.
Existing code:
private static string Label(string text) => Regex.Replace(text.Replace('_', ' '), "([a-z])([A-Z])", "$1 $2");
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (config.ShowLogo && branding?.HasLogo == true) | ||
| { | ||
| var logo = await _branding.GetMediaAsync(departmentId, DepartmentProfileMediaKind.PrintHeader); | ||
| if (logo?.Data?.Length > 0 && new[] { "image/png", "image/jpeg" }.Contains(logo.ContentType)) html.Append("<img alt=\"Department logo\" style=\"max-width:180px;max-height:85px\" src=\"data:").Append(logo.ContentType).Append(";base64,").Append(Convert.ToBase64String(logo.Data)).Append("\">"); |
There was a problem hiding this comment.
Framework-rule mismatch in Core/Resgrid.Services/Records/RecordsDocumentService.cs, Tests/Resgrid.Tests/Rms/RecordNarrativeFormatterTests.cs:10-10, and Tests/Resgrid.Tests/Web/neris-guided-form.test.cjs:51-51: the flagged code generates HTML for a .NET document service and cannot use Next.js Image. Remove or replace this finding with a rule applicable to server-side HTML generation.
Existing code:
if (logo?.Data?.Length > 0 && new[] { "image/png", "image/jpeg" }.Contains(logo.ContentType)) html.Append("<img alt=\"Department logo\" style=\"max-width:180px;max-height:85px\" src=\"data:").Append(logo.ContentType).Append(";base64,").Append(Convert.ToBase64String(logo.Data)).Append("\">");
Improved code:
(none)
Kody rule violation: Use next/image with explicit dimensions and alt
Prompt for LLM
File Core/Resgrid.Services/Records/RecordsDocumentService.cs:
Line 121:
Framework-rule mismatch in Core/Resgrid.Services/Records/RecordsDocumentService.cs, Tests/Resgrid.Tests/Rms/RecordNarrativeFormatterTests.cs:10-10, and Tests/Resgrid.Tests/Web/neris-guided-form.test.cjs:51-51: the flagged code generates HTML for a .NET document service and cannot use Next.js Image. Remove or replace this finding with a rule applicable to server-side HTML generation.
Existing code:
if (logo?.Data?.Length > 0 && new[] { "image/png", "image/jpeg" }.Contains(logo.ContentType)) html.Append("<img alt=\"Department logo\" style=\"max-width:180px;max-height:85px\" src=\"data:").Append(logo.ContentType).Append(";base64,").Append(Convert.ToBase64String(logo.Data)).Append("\">");
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| catch { _unitOfWork.DiscardChanges(); throw; } | ||
| } | ||
| private Task AuditAsync(RmsRecordLegalHold hold, string userId, string purpose, string reason, CancellationToken ct) => _audits.InsertAsync(new RmsAccessAudit { DepartmentId = hold.DepartmentId, RecordId = hold.RecordId, | ||
| ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true); |
There was a problem hiding this comment.
Incomplete ePHI audit record in Core/Resgrid.Services/Records/RecordsLegalHoldService.cs and the listed controllers and services: the audit write omits patient or record identifier shape and request-level traceability, preventing downstream compliance reporting for protected-record access. Include PatientId = hold.RecordId, an action such as READ_PHI or WRITE_PHI, and RequestId = requestId in the immutable audit payload.
Existing code:
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);
Improved code:
ActorUserId = userId, PatientId = hold.RecordId, Action = purpose.Contains("released") ? "WRITE_PHI" : "READ_PHI", Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, RequestId = requestId, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);
Kody rule violation: Write immutable audit logs for all ePHI access
ActorUserId = userId, PatientId = hold.RecordId, Action = purpose.Contains("released") ? "WRITE_PHI" : "READ_PHI", Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, RequestId = requestId, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);Prompt for LLM
File Core/Resgrid.Services/Records/RecordsLegalHoldService.cs:
Line 84:
Incomplete ePHI audit record in Core/Resgrid.Services/Records/RecordsLegalHoldService.cs and the listed controllers and services: the audit write omits patient or record identifier shape and request-level traceability, preventing downstream compliance reporting for protected-record access. Include PatientId = hold.RecordId, an action such as READ_PHI or WRITE_PHI, and RequestId = requestId in the immutable audit payload.
Existing code:
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);
Improved code:
ActorUserId = userId, PatientId = hold.RecordId, Action = purpose.Contains("released") ? "WRITE_PHI" : "READ_PHI", Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, RequestId = requestId, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);
Suggested Code:
ActorUserId = userId, PatientId = hold.RecordId, Action = purpose.Contains("released") ? "WRITE_PHI" : "READ_PHI", Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, RequestId = requestId, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| catch { _unitOfWork.DiscardChanges(); throw; } | ||
| } | ||
| private Task AuditAsync(RmsRecordLegalHold hold, string userId, string purpose, string reason, CancellationToken ct) => _audits.InsertAsync(new RmsAccessAudit { DepartmentId = hold.DepartmentId, RecordId = hold.RecordId, | ||
| ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true); |
There was a problem hiding this comment.
Data-minimization gap in Core/Resgrid.Services/Records/RecordsLegalHoldService.cs and the listed telemetry and view sites: DetailJson stores raw hold.ReferenceNumber and reason without privacy context, increasing exposure of potentially personal data in audit or diagnostic payloads. Redact or hash personal fields by default and attach lawful-basis metadata such as purpose = "records_retention" and lawful_basis = "legal_obligation".
Existing code:
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);
Improved code:
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, Gdpr = new { purpose = "records_retention", lawful_basis = "legal_obligation" }, DetailJson = JsonConvert.SerializeObject(new { holdId = hold.RmsRecordLegalHoldId, referenceNumberHash = Hash(hold.ReferenceNumber) }) }, ct, true);
Kody rule violation: Redact PII in logs and metrics by default
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, Gdpr = new { purpose = "records_retention", lawful_basis = "legal_obligation" }, DetailJson = JsonConvert.SerializeObject(new { holdId = hold.RmsRecordLegalHoldId, referenceNumberHash = Hash(hold.ReferenceNumber) }) }, ct, true);Prompt for LLM
File Core/Resgrid.Services/Records/RecordsLegalHoldService.cs:
Line 84:
Data-minimization gap in Core/Resgrid.Services/Records/RecordsLegalHoldService.cs and the listed telemetry and view sites: DetailJson stores raw hold.ReferenceNumber and reason without privacy context, increasing exposure of potentially personal data in audit or diagnostic payloads. Redact or hash personal fields by default and attach lawful-basis metadata such as purpose = "records_retention" and lawful_basis = "legal_obligation".
Existing code:
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, DetailJson = JsonConvert.SerializeObject(new { hold.RmsRecordLegalHoldId, hold.ReferenceNumber, reason }) }, ct, true);
Improved code:
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, Gdpr = new { purpose = "records_retention", lawful_basis = "legal_obligation" }, DetailJson = JsonConvert.SerializeObject(new { holdId = hold.RmsRecordLegalHoldId, referenceNumberHash = Hash(hold.ReferenceNumber) }) }, ct, true);
Suggested Code:
ActorUserId = userId, Action = (int)RmsAccessAuditAction.Admin, Successful = true, OccurredOn = DateTime.UtcNow, Purpose = purpose, Gdpr = new { purpose = "records_retention", lawful_basis = "legal_obligation" }, DetailJson = JsonConvert.SerializeObject(new { holdId = hold.RmsRecordLegalHoldId, referenceNumberHash = Hash(hold.ReferenceNumber) }) }, ct, true);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| Create.Table("RmsRecordLegalHoldMembers").WithColumn("DepartmentId").AsInt32().NotNullable() | ||
| .WithColumn("HoldId").AsString(36).NotNullable().PrimaryKey().WithColumn("RecordId").AsString(36).NotNullable().PrimaryKey().WithColumn("MatchedOn").AsDateTime2().NotNullable(); | ||
| Create.Index("IX_RmsLegalHoldMembers_Record").OnTable("RmsRecordLegalHoldMembers").OnColumn("DepartmentId").Ascending().OnColumn("RecordId").Ascending(); |
There was a problem hiding this comment.
Migration lock-risk in Providers/Resgrid.Providers.Migrations/Migrations/M0173_RmsReleaseHardening.cs, M0173_RmsReleaseHardening.cs:77-77, M0173_RmsReleaseHardening.cs:79-79, and Providers/Resgrid.Providers.MigrationsPg/Migrations/M0173_RmsReleaseHardeningPg.cs:56-56 and :57-57: Create.Index("IX_RmsLegalHoldMembers_Record") on RmsRecordLegalHoldMembers does not show an online or concurrent build strategy, which can take strong locks on large tables. Use the database-specific concurrent or online option where available, or document a safe expand-contract rollback plan.
Existing code:
Create.Index("IX_RmsLegalHoldMembers_Record").OnTable("RmsRecordLegalHoldMembers").OnColumn("DepartmentId").Ascending().OnColumn("RecordId").Ascending();
Improved code:
(none)
Kody rule violation: Block risky database migrations (locking ops, downtime risk)
Prompt for LLM
File Providers/Resgrid.Providers.Migrations/Migrations/M0173_RmsReleaseHardening.cs:
Line 39:
Migration lock-risk in Providers/Resgrid.Providers.Migrations/Migrations/M0173_RmsReleaseHardening.cs, M0173_RmsReleaseHardening.cs:77-77, M0173_RmsReleaseHardening.cs:79-79, and Providers/Resgrid.Providers.MigrationsPg/Migrations/M0173_RmsReleaseHardeningPg.cs:56-56 and :57-57: Create.Index("IX_RmsLegalHoldMembers_Record") on RmsRecordLegalHoldMembers does not show an online or concurrent build strategy, which can take strong locks on large tables. Use the database-specific concurrent or online option where available, or document a safe expand-contract rollback plan.
Existing code:
Create.Index("IX_RmsLegalHoldMembers_Record").OnTable("RmsRecordLegalHoldMembers").OnColumn("DepartmentId").Ascending().OnColumn("RecordId").Ascending();
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var contract = JObject.Parse(reader.ReadToEnd()); | ||
| if ((string)contract["info"]?["version"] != NerisValueSetCatalog.Instance.ContractVersion) | ||
| throw new InvalidOperationException("The NERIS schema and value-set versions do not match."); | ||
| return new NerisContractCatalog((JObject)contract["components"]["schemas"]); |
There was a problem hiding this comment.
Nested JSON dereference risk in Providers/Resgrid.Providers.Neris/NerisContractCatalog.cs and the listed files: contract["components"]["schemas"] throws if components is missing, turning malformed or partial contracts into runtime failures. Use null-safe access with a fallback or explicit validation before constructing NerisContractCatalog.
Existing code:
return new NerisContractCatalog((JObject)contract["components"]["schemas"]);
Improved code:
return new NerisContractCatalog((JObject?)contract["components"]?["schemas"] ?? new JObject());
Kody rule violation: Add null checks before accessing properties
return new NerisContractCatalog((JObject?)contract["components"]?["schemas"] ?? new JObject());Prompt for LLM
File Providers/Resgrid.Providers.Neris/NerisContractCatalog.cs:
Line 122:
Nested JSON dereference risk in Providers/Resgrid.Providers.Neris/NerisContractCatalog.cs and the listed files: contract["components"]["schemas"] throws if components is missing, turning malformed or partial contracts into runtime failures. Use null-safe access with a fallback or explicit validation before constructing NerisContractCatalog.
Existing code:
return new NerisContractCatalog((JObject)contract["components"]["schemas"]);
Improved code:
return new NerisContractCatalog((JObject?)contract["components"]?["schemas"] ?? new JObject());
Suggested Code:
return new NerisContractCatalog((JObject?)contract["components"]?["schemas"] ?? new JObject());
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var rows = (await QueryAsync<RmsSearchErasureTarget>( | ||
| $"SELECT * FROM ({Pending("RmsOperationalRecords", "RmsOperationalRecordId", RmsRecordKind.Operational)} UNION ALL {Pending("RmsIncidentReports", "RmsIncidentReportId", RmsRecordKind.IncidentReport)}) pending {cursor} ORDER BY {Col("DepartmentId")}, {Col("RecordKind")}, {Col("RecordId")} {Paging()}", | ||
| new { Skip = 0, Take = Math.Max(1, Math.Min(1000, take)), AfterDepartment = after?.DepartmentId, AfterKind = after?.RecordKind, AfterId = after?.RecordId }, cancellationToken)).ToList(); |
There was a problem hiding this comment.
Missing contextual error mapping in Repositories/Resgrid.Repositories.DataRepository/RmsRetentionRepository.cs and the listed database-backed call sites: the QueryAsync(...) call can fail transiently or due to schema or data issues, but the current code lets the exception surface without operation context. Wrap the query in try/catch and rethrow with repository-level context such as "Failed to query pending search erasures.".
Existing code:
var rows = (await QueryAsync<RmsSearchErasureTarget>(
$"SELECT * FROM ({Pending("RmsOperationalRecords", "RmsOperationalRecordId", RmsRecordKind.Operational)} UNION ALL {Pending("RmsIncidentReports", "RmsIncidentReportId", RmsRecordKind.IncidentReport)}) pending {cursor} ORDER BY {Col("DepartmentId")}, {Col("RecordKind")}, {Col("RecordId")} {Paging()}",
new { Skip = 0, Take = Math.Max(1, Math.Min(1000, take)), AfterDepartment = after?.DepartmentId, AfterKind = after?.RecordKind, AfterId = after?.RecordId }, cancellationToken)).ToList();
Improved code:
List<RmsSearchErasureTarget> rows;
try
{
rows = (await QueryAsync<RmsSearchErasureTarget>(
$"SELECT * FROM ({Pending("RmsOperationalRecords", "RmsOperationalRecordId", RmsRecordKind.Operational)} UNION ALL {Pending("RmsIncidentReports", "RmsIncidentReportId", RmsRecordKind.IncidentReport)}) pending {cursor} ORDER BY {Col("DepartmentId")}, {Col("RecordKind")}, {Col("RecordId")} {Paging()}",
new { Skip = 0, Take = Math.Max(1, Math.Min(1000, take)), AfterDepartment = after?.DepartmentId, AfterKind = after?.RecordKind, AfterId = after?.RecordId }, cancellationToken)).ToList();
}
catch (Exception ex)
{
throw new InvalidOperationException("Failed to query pending search erasures.", ex);
}
Kody rule violation: Add try-catch blocks for external calls
List<RmsSearchErasureTarget> rows;
try
{
rows = (await QueryAsync<RmsSearchErasureTarget>(
$"SELECT * FROM ({Pending("RmsOperationalRecords", "RmsOperationalRecordId", RmsRecordKind.Operational)} UNION ALL {Pending("RmsIncidentReports", "RmsIncidentReportId", RmsRecordKind.IncidentReport)}) pending {cursor} ORDER BY {Col("DepartmentId")}, {Col("RecordKind")}, {Col("RecordId")} {Paging()}",
new { Skip = 0, Take = Math.Max(1, Math.Min(1000, take)), AfterDepartment = after?.DepartmentId, AfterKind = after?.RecordKind, AfterId = after?.RecordId }, cancellationToken)).ToList();
}
catch (Exception ex)
{
throw new InvalidOperationException("Failed to query pending search erasures.", ex);
}Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/RmsRetentionRepository.cs:
Line 38 to 40:
Missing contextual error mapping in Repositories/Resgrid.Repositories.DataRepository/RmsRetentionRepository.cs and the listed database-backed call sites: the QueryAsync<RmsSearchErasureTarget>(...) call can fail transiently or due to schema or data issues, but the current code lets the exception surface without operation context. Wrap the query in try/catch and rethrow with repository-level context such as "Failed to query pending search erasures.".
Existing code:
var rows = (await QueryAsync<RmsSearchErasureTarget>(
$"SELECT * FROM ({Pending("RmsOperationalRecords", "RmsOperationalRecordId", RmsRecordKind.Operational)} UNION ALL {Pending("RmsIncidentReports", "RmsIncidentReportId", RmsRecordKind.IncidentReport)}) pending {cursor} ORDER BY {Col("DepartmentId")}, {Col("RecordKind")}, {Col("RecordId")} {Paging()}",
new { Skip = 0, Take = Math.Max(1, Math.Min(1000, take)), AfterDepartment = after?.DepartmentId, AfterKind = after?.RecordKind, AfterId = after?.RecordId }, cancellationToken)).ToList();
Improved code:
List<RmsSearchErasureTarget> rows;
try
{
rows = (await QueryAsync<RmsSearchErasureTarget>(
$"SELECT * FROM ({Pending("RmsOperationalRecords", "RmsOperationalRecordId", RmsRecordKind.Operational)} UNION ALL {Pending("RmsIncidentReports", "RmsIncidentReportId", RmsRecordKind.IncidentReport)}) pending {cursor} ORDER BY {Col("DepartmentId")}, {Col("RecordKind")}, {Col("RecordId")} {Paging()}",
new { Skip = 0, Take = Math.Max(1, Math.Min(1000, take)), AfterDepartment = after?.DepartmentId, AfterKind = after?.RecordKind, AfterId = after?.RecordId }, cancellationToken)).ToList();
}
catch (Exception ex)
{
throw new InvalidOperationException("Failed to query pending search erasures.", ex);
}
Suggested Code:
List<RmsSearchErasureTarget> rows;
try
{
rows = (await QueryAsync<RmsSearchErasureTarget>(
$"SELECT * FROM ({Pending("RmsOperationalRecords", "RmsOperationalRecordId", RmsRecordKind.Operational)} UNION ALL {Pending("RmsIncidentReports", "RmsIncidentReportId", RmsRecordKind.IncidentReport)}) pending {cursor} ORDER BY {Col("DepartmentId")}, {Col("RecordKind")}, {Col("RecordId")} {Paging()}",
new { Skip = 0, Take = Math.Max(1, Math.Min(1000, take)), AfterDepartment = after?.DepartmentId, AfterKind = after?.RecordKind, AfterId = after?.RecordId }, cancellationToken)).ToList();
}
catch (Exception ex)
{
throw new InvalidOperationException("Failed to query pending search erasures.", ex);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| private void Complete(bool commit) | ||
| { | ||
| _semaphore.Wait(); |
There was a problem hiding this comment.
Blocking async call in Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs and UnitOfWork.cs:92-92: _semaphore.Wait() blocks an asynchronous flow and can cause deadlocks or thread starvation. Replace the blocking call with await-based semaphore usage throughout the call chain.
Existing code:
_semaphore.Wait();
Improved code:
(none)
Kody rule violation: Avoid Blocking Calls to Async Methods
Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs:
Line 71:
Blocking async call in Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs and UnitOfWork.cs:92-92: _semaphore.Wait() blocks an asynchronous flow and can cause deadlocks or thread starvation. Replace the blocking call with await-based semaphore usage throughout the call chain.
Existing code:
_semaphore.Wait();
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| private void Complete(bool commit) | ||
| { | ||
| _semaphore.Wait(); |
There was a problem hiding this comment.
Blocking async call in Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs and UnitOfWork.cs:92-92: _semaphore.Wait() violates the await-only async rule and can deadlock or stall asynchronous execution. Convert the path to async/await end-to-end instead of blocking on the semaphore.
Existing code:
_semaphore.Wait();
Improved code:
(none)
Kody rule violation: Await async operations properly
Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs:
Line 71:
Blocking async call in Repositories/Resgrid.Repositories.DataRepository/Transactions/UnitOfWork.cs and UnitOfWork.cs:92-92: _semaphore.Wait() violates the await-only async rule and can deadlock or stall asynchronous execution. Convert the path to async/await end-to-end instead of blocking on the semaphore.
Existing code:
_semaphore.Wait();
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (owns) UnitOfWork.CommitChanges(); | ||
| return result; | ||
| } | ||
| catch { if (owns) UnitOfWork.DiscardChanges(); throw; } |
There was a problem hiding this comment.
Diagnostic gap in Repositories/Resgrid.Repositories.DataRepository/WorkflowRunLogRepository.cs and the listed catch sites: catch { if (owns) UnitOfWork.DiscardChanges(); throw; } rethrows without structured logging, which prevents correlation of persistence failures to WorkflowRunId, insert mode, DepartmentId, or AggregateId. Log the exception with operation metadata before rethrowing.
Existing code:
catch { if (owns) UnitOfWork.DiscardChanges(); throw; }
Improved code:
catch (Exception ex)
{
if (owns) UnitOfWork.DiscardChanges();
_logger.Error(ex, "WriteLogAsync failed", new { operation = "WriteLogAsync", workflowRunId = entity.WorkflowRunId, insert, departmentId = run?.DepartmentId, aggregateId = run?.AggregateId });
throw;
}
Kody rule violation: Include error context in structured logs
catch (Exception ex)
{
if (owns) UnitOfWork.DiscardChanges();
_logger.Error(ex, "WriteLogAsync failed", new { operation = "WriteLogAsync", workflowRunId = entity.WorkflowRunId, insert, departmentId = run?.DepartmentId, aggregateId = run?.AggregateId });
throw;
}Prompt for LLM
File Repositories/Resgrid.Repositories.DataRepository/WorkflowRunLogRepository.cs:
Line 41:
Diagnostic gap in Repositories/Resgrid.Repositories.DataRepository/WorkflowRunLogRepository.cs and the listed catch sites: catch { if (owns) UnitOfWork.DiscardChanges(); throw; } rethrows without structured logging, which prevents correlation of persistence failures to WorkflowRunId, insert mode, DepartmentId, or AggregateId. Log the exception with operation metadata before rethrowing.
Existing code:
catch { if (owns) UnitOfWork.DiscardChanges(); throw; }
Improved code:
catch (Exception ex)
{
if (owns) UnitOfWork.DiscardChanges();
_logger.Error(ex, "WriteLogAsync failed", new { operation = "WriteLogAsync", workflowRunId = entity.WorkflowRunId, insert, departmentId = run?.DepartmentId, aggregateId = run?.AggregateId });
throw;
}
Suggested Code:
catch (Exception ex)
{
if (owns) UnitOfWork.DiscardChanges();
_logger.Error(ex, "WriteLogAsync failed", new { operation = "WriteLogAsync", workflowRunId = entity.WorkflowRunId, insert, departmentId = run?.DepartmentId, aggregateId = run?.AggregateId });
throw;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| LocationUse = "RESIDENTIAL||DETATCHED_SINGLE_FAMILY_DWELLING", ConstructionType = "TYPE_VB", DamageType = "MAJOR_DAMAGE", | ||
| FireSpread = "BUILDING", EstimatedValue = 300000m, EstimatedLoss = 120000m, ContentsValue = 50000m, ContentsLoss = 20000m | ||
| FireSpread = "BUILDING", EstimatedValue = 300000m, EstimatedLoss = 120000m, ContentsValue = 50000m, ContentsLoss = 20000m, DetailJson = "{\"parcel_id\":\"test-parcel\",\"structures\":[{\"ignition_source\":true,\"location\":{\"number\":100,\"street\":\"Main St\",\"incorporated_municipality\":\"Springfield\",\"state\":\"IL\"}}]}" |
There was a problem hiding this comment.
PII in test fixture in Tests/Resgrid.Tests/Rms/IncidentAnalysisServiceTests.cs and the listed files: DetailJson embeds address-like fields such as number, street, incorporated_municipality, and state, which introduces raw location data into payload-shaped test artifacts. Replace those fields with synthetic or tokenized placeholders to keep fixtures non-identifying.
Existing code:
FireSpread = "BUILDING", EstimatedValue = 300000m, EstimatedLoss = 120000m, ContentsValue = 50000m, ContentsLoss = 20000m, DetailJson = "{\"parcel_id\":\"test-parcel\",\"structures\":[{\"ignition_source\":true,\"location\":{\"number\":100,\"street\":\"Main St\",\"incorporated_municipality\":\"Springfield\",\"state\":\"IL\"}}]}"
Improved code:
(none)
Kody rule violation: Mask PII and secrets in logs
Prompt for LLM
File Tests/Resgrid.Tests/Rms/IncidentAnalysisServiceTests.cs:
Line 96:
PII in test fixture in Tests/Resgrid.Tests/Rms/IncidentAnalysisServiceTests.cs and the listed files: DetailJson embeds address-like fields such as number, street, incorporated_municipality, and state, which introduces raw location data into payload-shaped test artifacts. Replace those fields with synthetic or tokenized placeholders to keep fixtures non-identifying.
Existing code:
FireSpread = "BUILDING", EstimatedValue = 300000m, EstimatedLoss = 120000m, ContentsValue = 50000m, ContentsLoss = 20000m, DetailJson = "{\"parcel_id\":\"test-parcel\",\"structures\":[{\"ignition_source\":true,\"location\":{\"number\":100,\"street\":\"Main St\",\"incorporated_municipality\":\"Springfield\",\"state\":\"IL\"}}]}"
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var input = TrainingInput(); input.Participants[0].UnitId = 5; input.Participants[0].Role = "Instructor"; | ||
| var created = await _service.CreateDraftAsync(Dept, "author", input); | ||
| var model = new Resgrid.Web.Areas.User.Models.Records.RecordEditView { ParticipantRows = created.Participants.Select(p => new Resgrid.Web.Areas.User.Models.Records.RecordParticipantEditRow { Selected = true, UserId = p.UserId, UnitId = p.UnitId, Role = p.Role }).ToList() }; | ||
| var updated = TrainingInput("Unrelated narrative correction"); updated.Participants = Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model); |
There was a problem hiding this comment.
Architecture-layer violation in Tests/Resgrid.Tests/Rms/RecordsServiceTests.cs: Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model) pulls controller mapping logic into a service-oriented test, making UI code the dependency boundary for record logic. Move BuildParticipantInput into a shared application or service-layer mapper and reference that from tests.
Existing code:
var updated = TrainingInput("Unrelated narrative correction"); updated.Participants = Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model);
Improved code:
(none)
Kody rule violation: Separate UI logic from business logic
Prompt for LLM
File Tests/Resgrid.Tests/Rms/RecordsServiceTests.cs:
Line 208:
Architecture-layer violation in Tests/Resgrid.Tests/Rms/RecordsServiceTests.cs: Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model) pulls controller mapping logic into a service-oriented test, making UI code the dependency boundary for record logic. Move BuildParticipantInput into a shared application or service-layer mapper and reference that from tests.
Existing code:
var updated = TrainingInput("Unrelated narrative correction"); updated.Participants = Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model);
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var input = TrainingInput(); input.Participants[0].UnitId = 5; input.Participants[0].Role = "Instructor"; | ||
| var created = await _service.CreateDraftAsync(Dept, "author", input); | ||
| var model = new Resgrid.Web.Areas.User.Models.Records.RecordEditView { ParticipantRows = created.Participants.Select(p => new Resgrid.Web.Areas.User.Models.Records.RecordParticipantEditRow { Selected = true, UserId = p.UserId, UnitId = p.UnitId, Role = p.Role }).ToList() }; | ||
| var updated = TrainingInput("Unrelated narrative correction"); updated.Participants = Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model); |
There was a problem hiding this comment.
Cross-layer dependency in Tests/Resgrid.Tests/Rms/RecordsServiceTests.cs: depending on Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model) couples this test or module to web-controller code instead of application-layer mapping. Extract BuildParticipantInput into a shared utility or service-layer mapper so record logic does not depend on the UI layer.
Existing code:
var updated = TrainingInput("Unrelated narrative correction"); updated.Participants = Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model);
Improved code:
(none)
Kody rule violation: Enforce architecture boundaries and layering rules
Prompt for LLM
File Tests/Resgrid.Tests/Rms/RecordsServiceTests.cs:
Line 208:
Cross-layer dependency in Tests/Resgrid.Tests/Rms/RecordsServiceTests.cs: depending on Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model) couples this test or module to web-controller code instead of application-layer mapping. Extract BuildParticipantInput into a shared utility or service-layer mapper so record logic does not depend on the UI layer.
Existing code:
var updated = TrainingInput("Unrelated narrative correction"); updated.Participants = Resgrid.Web.Areas.User.Controllers.RecordsController.BuildParticipantInput(model);
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| var winner = reservations[0] ? first : second; var loser = reservations[0] ? second : first; | ||
| using var db = new SqlConnection(_connection); | ||
| await db.ExecuteAsync("UPDATE RmsCommandReceipts SET CreatedOn=DATEADD(day,-2,SYSUTCDATETIME()) WHERE DepartmentId=11 AND KeyHash=@Key", new { Key = key }); | ||
| var pending = await restarted.TryGetCommandAsync(11, "officer", clientKey, "Reassign"); |
There was a problem hiding this comment.
Null-result handling gap in Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs and Core/Resgrid.Services/Records/IncidentAttachmentsService.cs:81-81, :88-88, and :91-91: await restarted.TryGetCommandAsync(11, "officer", clientKey, "Reassign") may return null, so later property dereferences can fail if callers assume a non-null result. Preserve a non-null Task result and guard pending before accessing nested state.
Existing code:
var pending = await restarted.TryGetCommandAsync(11, "officer", clientKey, "Reassign");
Improved code:
(none)
Kody rule violation: Avoid Returning Null in Non-Async Task Methods
Prompt for LLM
File Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:
Line 414:
Null-result handling gap in Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs and Core/Resgrid.Services/Records/IncidentAttachmentsService.cs:81-81, :88-88, and :91-91: await restarted.TryGetCommandAsync(11, "officer", clientKey, "Reassign") may return null, so later property dereferences can fail if callers assume a non-null result. Preserve a non-null Task result and guard pending before accessing nested state.
Existing code:
var pending = await restarted.TryGetCommandAsync(11, "officer", clientKey, "Reassign");
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| using var db = new SqlConnection(_connection); | ||
| await db.ExecuteAsync("UPDATE RmsOperationalRecords SET State=@State, OwnerUserId=CASE WHEN RmsOperationalRecordId=@Owned THEN @Viewer ELSE 'another-officer' END WHERE RmsOperationalRecordId IN @Ids", new { State = (int)RmsRecordState.Draft, Owned = operational[2], Viewer = viewer, Ids = operational }); | ||
| await db.ExecuteAsync("INSERT RmsRecordParticipants (RmsRecordParticipantId,DepartmentId,ProtectionId,RecordId,UserId,CreatedOn,ModifiedOn) VALUES (@Id,11,@Id,@RecordId,@Viewer,@Now,@Now)", new { Id = Guid.NewGuid().ToString(), RecordId = operational[3], Viewer = viewer, Now = DateTime.UtcNow }); | ||
| for (var i = 0; i < 2; i++) |
There was a problem hiding this comment.
No issue in Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs and Core/Resgrid.Services/Records/RecordsUdfService.cs:75-75: for (var i = 0; i < 2; i++) already uses a relational loop-termination condition and complies with the stated rule. Remove this finding rather than suggesting a change.
Existing code:
for (var i = 0; i < 2; i++)
Improved code:
(none)
Kody rule violation: Avoid equality operators in loop termination conditions
Prompt for LLM
File Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:
Line 164:
No issue in Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs and Core/Resgrid.Services/Records/RecordsUdfService.cs:75-75: for (var i = 0; i < 2; i++) already uses a relational loop-termination condition and complies with the stated rule. Remove this finding rather than suggesting a change.
Existing code:
for (var i = 0; i < 2; i++)
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| #endregion | ||
| private Task<bool> CanViewRestrictedAsync() => _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords); |
There was a problem hiding this comment.
Authorization regression in Web/Resgrid.Web.Services/Controllers/v4/RecordEvidenceController.cs: CanViewRestrictedAsync now trusts _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords) alone, so a caller without the current restricted-records claim can still read, verify, or capture restricted evidence if the live permission check returns true. Require both ClaimsAuthorizationHelper.CanViewRestrictedRecords() and the live permission check to preserve the existing authorization boundary.
Existing code:
private Task<bool> CanViewRestrictedAsync() => _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords);
Improved code:
private async Task<bool> CanViewRestrictedAsync() => ClaimsAuthorizationHelper.CanViewRestrictedRecords()
&& await _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords);
private async Task<bool> CanViewRestrictedAsync() => ClaimsAuthorizationHelper.CanViewRestrictedRecords()
&& await _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords);Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/RecordEvidenceController.cs:
Line 308 to 309:
Authorization regression in Web/Resgrid.Web.Services/Controllers/v4/RecordEvidenceController.cs: CanViewRestrictedAsync now trusts _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords) alone, so a caller without the current restricted-records claim can still read, verify, or capture restricted evidence if the live permission check returns true. Require both ClaimsAuthorizationHelper.CanViewRestrictedRecords() and the live permission check to preserve the existing authorization boundary.
Existing code:
private Task CanViewRestrictedAsync() => _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords);
Improved code:
private async Task CanViewRestrictedAsync() => ClaimsAuthorizationHelper.CanViewRestrictedRecords()
&& await _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords);
Suggested Code:
private async Task<bool> CanViewRestrictedAsync() => ClaimsAuthorizationHelper.CanViewRestrictedRecords()
&& await _recordsAuthorizationService.HasPermissionAsync(UserId, DepartmentId, PermissionTypes.ViewRestrictedRecords);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @@ -179,6 +186,8 @@ public async Task<ActionResult<RecordEvidenceResult>> Capture(CaptureRecordEvide | |||
| { | |||
| if (input == null || string.IsNullOrWhiteSpace(input.RecordId) || string.IsNullOrWhiteSpace(input.CaptureReason)) | |||
| return BadRequest(); | |||
| input.ExpectedRowVersion = RecordsApiHelper.ResolveRowVersion(input.ExpectedRowVersion, Request); | |||
| if (!input.ExpectedRowVersion.HasValue) return Problem(statusCode: 428, title: "Supply the current record version in ExpectedRowVersion or If-Match."); | |||
There was a problem hiding this comment.
Client-error response clarity issue in Web/Resgrid.Web.Services/Controllers/v4/RecordEvidenceController.cs and the listed endpoints: Problem(statusCode: 428, ...) returns a generic problem body for a client precondition failure, which is less explicit than the API's minimal 4xx payloads. Return StatusCodes.Status428PreconditionRequired with a small non-sensitive error object instead.
Existing code:
if (!input.ExpectedRowVersion.HasValue) return Problem(statusCode: 428, title: "Supply the current record version in ExpectedRowVersion or If-Match.");
Improved code:
if (!input.ExpectedRowVersion.HasValue)
return StatusCode(StatusCodes.Status428PreconditionRequired, new { error = "Supply the current record version in ExpectedRowVersion or If-Match." });
Kody rule violation: Use appropriate HTTP status codes
if (!input.ExpectedRowVersion.HasValue)
return StatusCode(StatusCodes.Status428PreconditionRequired, new { error = "Supply the current record version in ExpectedRowVersion or If-Match." });Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/RecordEvidenceController.cs:
Line 190:
Client-error response clarity issue in Web/Resgrid.Web.Services/Controllers/v4/RecordEvidenceController.cs and the listed endpoints: Problem(statusCode: 428, ...) returns a generic problem body for a client precondition failure, which is less explicit than the API's minimal 4xx payloads. Return StatusCodes.Status428PreconditionRequired with a small non-sensitive error object instead.
Existing code:
if (!input.ExpectedRowVersion.HasValue) return Problem(statusCode: 428, title: "Supply the current record version in ExpectedRowVersion or If-Match.");
Improved code:
if (!input.ExpectedRowVersion.HasValue)
return StatusCode(StatusCodes.Status428PreconditionRequired, new { error = "Supply the current record version in ExpectedRowVersion or If-Match." });
Suggested Code:
if (!input.ExpectedRowVersion.HasValue)
return StatusCode(StatusCodes.Status428PreconditionRequired, new { error = "Supply the current record version in ExpectedRowVersion or If-Match." });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (!await Allowed(input.RecordId)) return NotFound(); | ||
| try | ||
| { | ||
| var usage=await _usage.ConsumeAsync(DepartmentId,UserId,input.RecordId,input.Kind,input.ExpectedRowVersion,input.TypeId,input.GroupId,input.UnitId,input.Quantity,input.Note,cancellationToken); |
There was a problem hiding this comment.
No functional issue is demonstrated in Web/Resgrid.Web.Services/Controllers/v4/RecordInventoryController.cs and the listed locations: the awaited _usage.ConsumeAsync(...) call is already inside a surrounding try/catch, and the proposed change is formatting-only. Retain the guarded placement and address logging quality in the catch block if needed, but this line itself does not warrant a code-change comment.
Existing code:
var usage=await _usage.ConsumeAsync(DepartmentId,UserId,input.RecordId,input.Kind,input.ExpectedRowVersion,input.TypeId,input.GroupId,input.UnitId,input.Quantity,input.Note,cancellationToken);
Improved code:
var usage = await _usage.ConsumeAsync(DepartmentId, UserId, input.RecordId, input.Kind, input.ExpectedRowVersion, input.TypeId, input.GroupId, input.UnitId, input.Quantity, input.Note, cancellationToken);
Kody rule violation: Handle async operations with proper error handling
var usage = await _usage.ConsumeAsync(DepartmentId, UserId, input.RecordId, input.Kind, input.ExpectedRowVersion, input.TypeId, input.GroupId, input.UnitId, input.Quantity, input.Note, cancellationToken);Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/RecordInventoryController.cs:
Line 50:
No functional issue is demonstrated in Web/Resgrid.Web.Services/Controllers/v4/RecordInventoryController.cs and the listed locations: the awaited _usage.ConsumeAsync(...) call is already inside a surrounding try/catch, and the proposed change is formatting-only. Retain the guarded placement and address logging quality in the catch block if needed, but this line itself does not warrant a code-change comment.
Existing code:
var usage=await _usage.ConsumeAsync(DepartmentId,UserId,input.RecordId,input.Kind,input.ExpectedRowVersion,input.TypeId,input.GroupId,input.UnitId,input.Quantity,input.Note,cancellationToken);
Improved code:
var usage = await _usage.ConsumeAsync(DepartmentId, UserId, input.RecordId, input.Kind, input.ExpectedRowVersion, input.TypeId, input.GroupId, input.UnitId, input.Quantity, input.Note, cancellationToken);
Suggested Code:
var usage = await _usage.ConsumeAsync(DepartmentId, UserId, input.RecordId, input.Kind, input.ExpectedRowVersion, input.TypeId, input.GroupId, input.UnitId, input.Quantity, input.Note, cancellationToken);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| [HttpPost("Place")] | ||
| public async Task<IActionResult> Place([FromBody] RmsRecordLegalHold input, CancellationToken cancellationToken) | ||
| { | ||
| if (!(await _cutover.GetModuleStateAsync(DepartmentId)).FlagEnabled) return NotFound(); |
There was a problem hiding this comment.
Precondition ordering issue in Web/Resgrid.Web.Services/Controllers/v4/RecordLegalHoldsController.cs and the listed RecordLegalHoldsController and RecordInventoryController actions: Place currently performs await _cutover.GetModuleStateAsync(DepartmentId) before validating the request body, causing a service or database-backed lookup for an invalid request. Validate input == null before the async cutover check.
Existing code:
if (!(await _cutover.GetModuleStateAsync(DepartmentId)).FlagEnabled) return NotFound();
Improved code:
if (input == null) return BadRequest();
if (!(await _cutover.GetModuleStateAsync(DepartmentId)).FlagEnabled) return NotFound();
Kody rule violation: Order validations before database queries
if (input == null) return BadRequest();
if (!(await _cutover.GetModuleStateAsync(DepartmentId)).FlagEnabled) return NotFound();Prompt for LLM
File Web/Resgrid.Web.Services/Controllers/v4/RecordLegalHoldsController.cs:
Line 28:
Precondition ordering issue in Web/Resgrid.Web.Services/Controllers/v4/RecordLegalHoldsController.cs and the listed RecordLegalHoldsController and RecordInventoryController actions: Place currently performs await _cutover.GetModuleStateAsync(DepartmentId) before validating the request body, causing a service or database-backed lookup for an invalid request. Validate input == null before the async cutover check.
Existing code:
if (!(await _cutover.GetModuleStateAsync(DepartmentId)).FlagEnabled) return NotFound();
Improved code:
if (input == null) return BadRequest();
if (!(await _cutover.GetModuleStateAsync(DepartmentId)).FlagEnabled) return NotFound();
Suggested Code:
if (input == null) return BadRequest();
if (!(await _cutover.GetModuleStateAsync(DepartmentId)).FlagEnabled) return NotFound();
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| model.ActiveCalls = calls.OrderByDescending(c => c.LoggedOn) | ||
| var readable = new List<Call>(); | ||
| foreach (var call in calls) | ||
| if (await _recordsAuthorizationService.CanReadSourceCallAsync(UserId, DepartmentId, call)) readable.Add(call); |
There was a problem hiding this comment.
N+1 service call in Web/Resgrid.Web/Areas/User/Controllers/IncidentReportsController.cs and Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:231-231: awaiting _recordsAuthorizationService.CanReadSourceCallAsync(UserId, DepartmentId, call) inside a loop scales latency with the number of calls. Batch readability evaluation or fetch only authorized calls up front.
Existing code:
if (await _recordsAuthorizationService.CanReadSourceCallAsync(UserId, DepartmentId, call)) readable.Add(call);
Improved code:
(none)
Kody rule violation: Detect N+1 style queries and suggest batching
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/IncidentReportsController.cs:
Line 202:
N+1 service call in Web/Resgrid.Web/Areas/User/Controllers/IncidentReportsController.cs and Tests/Resgrid.Tests/Rms/RmsRetentionDatabaseTests.cs:231-231: awaiting _recordsAuthorizationService.CanReadSourceCallAsync(UserId, DepartmentId, call) inside a loop scales latency with the number of calls. Batch readability evaluation or fetch only authorized calls up front.
Existing code:
if (await _recordsAuthorizationService.CanReadSourceCallAsync(UserId, DepartmentId, call)) readable.Add(call);
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| await Audit(right, RmsAccessAuditAction.Read, "Compare departmental revisions " + left.RevisionNumber + " and " + right.RevisionNumber); | ||
| ViewData["From"] = left.RevisionNumber; ViewData["To"] = right.RevisionNumber; | ||
| ViewData["RecordId"] = id; ViewData["Kind"] = kind; ViewData["FromId"] = from; ViewData["ToId"] = to; | ||
| ViewData["Withheld"] = left.WithheldFields.Count > 0 || right.WithheldFields.Count > 0; |
There was a problem hiding this comment.
Null dereference risk in Web/Resgrid.Web/Areas/User/Controllers/RecordDocumentsController.cs and the listed sites: left.WithheldFields.Count and right.WithheldFields.Count assume WithheldFields is always initialized, which can throw NullReferenceException during diff rendering. Use null-conditional and null-coalescing access when evaluating the counts.
Existing code:
ViewData["Withheld"] = left.WithheldFields.Count > 0 || right.WithheldFields.Count > 0;
Improved code:
ViewData["Withheld"] = (left.WithheldFields?.Count ?? 0) > 0 || (right.WithheldFields?.Count ?? 0) > 0;
Kody rule violation: Add null checks to prevent NullReferenceException
ViewData["Withheld"] = (left.WithheldFields?.Count ?? 0) > 0 || (right.WithheldFields?.Count ?? 0) > 0;Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordDocumentsController.cs:
Line 94:
Null dereference risk in Web/Resgrid.Web/Areas/User/Controllers/RecordDocumentsController.cs and the listed sites: left.WithheldFields.Count and right.WithheldFields.Count assume WithheldFields is always initialized, which can throw NullReferenceException during diff rendering. Use null-conditional and null-coalescing access when evaluating the counts.
Existing code:
ViewData["Withheld"] = left.WithheldFields.Count > 0 || right.WithheldFields.Count > 0;
Improved code:
ViewData["Withheld"] = (left.WithheldFields?.Count ?? 0) > 0 || (right.WithheldFields?.Count ?? 0) > 0;
Suggested Code:
ViewData["Withheld"] = (left.WithheldFields?.Count ?? 0) > 0 || (right.WithheldFields?.Count ?? 0) > 0;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| if (format == "pdf") { bytes = await _documents.RenderPdfAsync(DepartmentId, UserId, document); contentType = "application/pdf"; } | ||
| else if (format == "json") { bytes = Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(new { document.Format, document.RecordId, document.RecordKind, document.RecordNumber, document.RevisionId, document.RevisionNumber, document.OriginalChecksum, document.ContentChecksum, document.FinalizedOn, document.AttestedBy, document.AttestationVersion, document.WithheldFields, Content = JObject.Parse(document.ContentJson) }, Formatting.Indented)); contentType = "application/json"; } | ||
| else { bytes = Encoding.UTF8.GetBytes(Csv(document)); contentType = "text/csv"; } | ||
| await Audit(document, RmsAccessAuditAction.Export, "Departmental " + format + " export"); |
There was a problem hiding this comment.
Missing export controls in Web/Resgrid.Web/Areas/User/Controllers/RecordDocumentsController.cs and the listed export paths: await Audit(document, RmsAccessAuditAction.Export, "Departmental " + format + " export") records an export event but does not show approval, step-up MFA, rate limiting, watermarking, or an export_id required for bulk PII or ePHI export flows. Enforce those controls before export and write a dedicated export identifier to the audit log.
Existing code:
await Audit(document, RmsAccessAuditAction.Export, "Departmental " + format + " export");
Improved code:
var exportId = await exportControlService.CreateApprovedExportAsync(UserId, document.RecordId, format);
await auditLog.WriteAsync(new { action = "export.create", export_id = exportId, userId = UserId, timestamp = DateTime.UtcNow });
Kody rule violation: Define data export controls and watermarking
var exportId = await exportControlService.CreateApprovedExportAsync(UserId, document.RecordId, format);
await auditLog.WriteAsync(new { action = "export.create", export_id = exportId, userId = UserId, timestamp = DateTime.UtcNow });Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordDocumentsController.cs:
Line 72:
Missing export controls in Web/Resgrid.Web/Areas/User/Controllers/RecordDocumentsController.cs and the listed export paths: await Audit(document, RmsAccessAuditAction.Export, "Departmental " + format + " export") records an export event but does not show approval, step-up MFA, rate limiting, watermarking, or an export_id required for bulk PII or ePHI export flows. Enforce those controls before export and write a dedicated export identifier to the audit log.
Existing code:
await Audit(document, RmsAccessAuditAction.Export, "Departmental " + format + " export");
Improved code:
var exportId = await exportControlService.CreateApprovedExportAsync(UserId, document.RecordId, format);
await auditLog.WriteAsync(new { action = "export.create", export_id = exportId, userId = UserId, timestamp = DateTime.UtcNow });
Suggested Code:
var exportId = await exportControlService.CreateApprovedExportAsync(UserId, document.RecordId, format);
await auditLog.WriteAsync(new { action = "export.create", export_id = exportId, userId = UserId, timestamp = DateTime.UtcNow });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| [HttpPost] | ||
| [ValidateAntiForgeryToken] | ||
| [Authorize(Policy = ResgridResources.Record_Create)] | ||
| public async Task<IActionResult> Capture(RecordEvidenceForm input, CancellationToken cancellationToken) |
There was a problem hiding this comment.
Validation ordering issue in Web/Resgrid.Web/Areas/User/Controllers/RecordEvidenceController.cs and Web/Resgrid.Web.Services/Controllers/v4/RecordInventoryController.cs:43-43: Capture(RecordEvidenceForm input, CancellationToken cancellationToken) should treat ModelState.IsValid as a first-class gate before using model-bound values or returning flow-dependent responses. Move the invalid-model check to the start of the action.
Existing code:
public async Task<IActionResult> Capture(RecordEvidenceForm input, CancellationToken cancellationToken)
Improved code:
public async Task<IActionResult> Capture(RecordEvidenceForm input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return BadRequest(ModelState);
...
Kody rule violation: Always Validate `ModelState.IsValid` in Controllers
public async Task<IActionResult> Capture(RecordEvidenceForm input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return BadRequest(ModelState);
...Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordEvidenceController.cs:
Line 74:
Validation ordering issue in Web/Resgrid.Web/Areas/User/Controllers/RecordEvidenceController.cs and Web/Resgrid.Web.Services/Controllers/v4/RecordInventoryController.cs:43-43: Capture(RecordEvidenceForm input, CancellationToken cancellationToken) should treat ModelState.IsValid as a first-class gate before using model-bound values or returning flow-dependent responses. Move the invalid-model check to the start of the action.
Existing code:
public async Task<IActionResult> Capture(RecordEvidenceForm input, CancellationToken cancellationToken)
Improved code:
public async Task<IActionResult> Capture(RecordEvidenceForm input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return BadRequest(ModelState);
...
Suggested Code:
public async Task<IActionResult> Capture(RecordEvidenceForm input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid) return BadRequest(ModelState);
...
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| catch (UnauthorizedAccessException) { return Forbid(); } | ||
| } | ||
| [HttpPost, ValidateAntiForgeryToken] | ||
| public async Task<IActionResult> Place(RmsRecordLegalHold input, CancellationToken cancellationToken) |
There was a problem hiding this comment.
Missing server-side validation gate in Web/Resgrid.Web/Areas/User/Controllers/RecordLegalHoldsController.cs and RecordLegalHoldsController.cs:35-35: Place(RmsRecordLegalHold input, CancellationToken cancellationToken) accepts POSTed input without an upfront ModelState.IsValid check, allowing invalid payloads to reach the service layer. Reject invalid models before processing.
Existing code:
public async Task<IActionResult> Place(RmsRecordLegalHold input, CancellationToken cancellationToken)
Improved code:
public async Task<IActionResult> Place(RmsRecordLegalHold input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid)
return BadRequest(ModelState);
// continue
}
Kody rule violation: Validate inputs on the server (zod) in Route Handlers/Actions
public async Task<IActionResult> Place(RmsRecordLegalHold input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid)
return BadRequest(ModelState);
// continue
}Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordLegalHoldsController.cs:
Line 26:
Missing server-side validation gate in Web/Resgrid.Web/Areas/User/Controllers/RecordLegalHoldsController.cs and RecordLegalHoldsController.cs:35-35: Place(RmsRecordLegalHold input, CancellationToken cancellationToken) accepts POSTed input without an upfront ModelState.IsValid check, allowing invalid payloads to reach the service layer. Reject invalid models before processing.
Existing code:
public async Task Place(RmsRecordLegalHold input, CancellationToken cancellationToken)
Improved code:
public async Task Place(RmsRecordLegalHold input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid)
return BadRequest(ModelState);
// continue
}
Suggested Code:
public async Task<IActionResult> Place(RmsRecordLegalHold input, CancellationToken cancellationToken)
{
if (!ModelState.IsValid)
return BadRequest(ModelState);
// continue
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| { | ||
| if (RmsDefinitionKeys.RestrictedClass.Contains(model.DefinitionKey ?? string.Empty)) | ||
| foreach (var name in RecordSnapshotSerializer.RestrictedDetailFields) | ||
| typeof(RmsOperationalRecordDetail).GetProperty(name).SetValue(model.Details, null); |
There was a problem hiding this comment.
Reflection-driven mutation in Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs and Core/Resgrid.Services/Records/IncidentReportsService.cs:717-717: typeof(RmsOperationalRecordDetail).GetProperty(name).SetValue(model.Details, null) selects a property from variable input at runtime, which is brittle and can modify unintended members. Replace reflection with an explicit allowlisted mapping from field names to strongly typed setters.
Existing code:
typeof(RmsOperationalRecordDetail).GetProperty(name).SetValue(model.Details, null);
Improved code:
var allowedRestrictedFields = new Dictionary<string, Action<RmsOperationalRecordDetail>>
{
[nameof(RmsOperationalRecordDetail.SomeField)] = d => d.SomeField = null,
};
if (allowedRestrictedFields.TryGetValue(name, out var clearField))
{
clearField(model.Details);
}
Kody rule violation: Prevent Reflection Injection Attacks
var allowedRestrictedFields = new Dictionary<string, Action<RmsOperationalRecordDetail>>
{
[nameof(RmsOperationalRecordDetail.SomeField)] = d => d.SomeField = null,
};
if (allowedRestrictedFields.TryGetValue(name, out var clearField))
{
clearField(model.Details);
}Prompt for LLM
File Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs:
Line 431:
Reflection-driven mutation in Web/Resgrid.Web/Areas/User/Controllers/RecordsController.cs and Core/Resgrid.Services/Records/IncidentReportsService.cs:717-717: typeof(RmsOperationalRecordDetail).GetProperty(name).SetValue(model.Details, null) selects a property from variable input at runtime, which is brittle and can modify unintended members. Replace reflection with an explicit allowlisted mapping from field names to strongly typed setters.
Existing code:
typeof(RmsOperationalRecordDetail).GetProperty(name).SetValue(model.Details, null);
Improved code:
var allowedRestrictedFields = new Dictionary<string, Action<RmsOperationalRecordDetail>>
{
[nameof(RmsOperationalRecordDetail.SomeField)] = d => d.SomeField = null,
};
if (allowedRestrictedFields.TryGetValue(name, out var clearField))
{
clearField(model.Details);
}
Suggested Code:
var allowedRestrictedFields = new Dictionary<string, Action<RmsOperationalRecordDetail>>
{
[nameof(RmsOperationalRecordDetail.SomeField)] = d => d.SomeField = null,
};
if (allowedRestrictedFields.TryGetValue(name, out var clearField))
{
clearField(model.Details);
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <label>Delivery method<input class="form-control" name="deliveryMethod" required maxlength="500" /></label> | ||
| <label>Receipt or delivery reference<input class="form-control" name="deliveryReference" required maxlength="1000" /></label> |
There was a problem hiding this comment.
Validation mismatch in Web/Resgrid.Web/Areas/User/Views/Disclosures/Details.cshtml: the deliveryMethod input allows maxlength="500", but the release service rejects values longer than 200, so client-side acceptance guarantees a server-side failure. Align the view limit with the 200-character service constraint.
Existing code:
<label>Delivery method<input class="form-control" name="deliveryMethod" required maxlength="500" /></label>
<label>Receipt or delivery reference<input class="form-control" name="deliveryReference" required maxlength="1000" /></label>
Improved code:
<label>Delivery method<input class="form-control" name="deliveryMethod" required maxlength="200" /></label>
<label>Receipt or delivery reference<input class="form-control" name="deliveryReference" required maxlength="1000" /></label>
<label>Delivery method<input class="form-control" name="deliveryMethod" required maxlength="200" /></label>
<label>Receipt or delivery reference<input class="form-control" name="deliveryReference" required maxlength="1000" /></label>Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Disclosures/Details.cshtml:
Line 217 to 218:
Validation mismatch in Web/Resgrid.Web/Areas/User/Views/Disclosures/Details.cshtml: the deliveryMethod input allows maxlength="500", but the release service rejects values longer than 200, so client-side acceptance guarantees a server-side failure. Align the view limit with the 200-character service constraint.
Existing code:
Delivery method
Receipt or delivery reference
Improved code:
Delivery method
Receipt or delivery reference
Suggested Code:
<label>Delivery method<input class="form-control" name="deliveryMethod" required maxlength="200" /></label>
<label>Receipt or delivery reference<input class="form-control" name="deliveryReference" required maxlength="1000" /></label>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @for (var a = 0; a < Model.Records[i].Attachments.Count; a++) | ||
| { | ||
| <div class="well"><input type="hidden" asp-for="Records[i].Attachments[a].AttachmentId" /><input type="hidden" asp-for="Records[i].Attachments[a].Checksum" /> | ||
| <p><a asp-action="ReviewAttachment" asp-route-id="@Model.RequestId" asp-route-recordId="@Model.Records[i].RecordId" asp-route-revisionId="@Model.Records[i].RevisionId" asp-route-attachmentId="@Model.Records[i].Attachments[a].AttachmentId" asp-route-profile="@Model.Profile">@Model.Records[i].Attachments[a].FileName</a></p> |
There was a problem hiding this comment.
Missing consent context in Web/Resgrid.Web/Areas/User/Views/Disclosures/Review.cshtml and Web/Resgrid.Web/Areas/User/Views/Records/Details.cshtml:145-145: the ReviewAttachment link can expose attachments that may contain sensitive personal data without an explicit consent or authorization identifier in the flow. Require an explicit consent context before review and propagate a consent identifier to the server-side action.
Existing code:
<p><a asp-action="ReviewAttachment" asp-route-id="@Model.RequestId" asp-route-recordId="@Model.Records[i].RecordId" asp-route-revisionId="@Model.Records[i].RevisionId" asp-route-attachmentId="@Model.Records[i].Attachments[a].AttachmentId" asp-route-profile="@Model.Profile">@Model.Records[i].Attachments[a].FileName</a></p>
Improved code:
(none)
Kody rule violation: Require explicit consent before processing sensitive data
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Disclosures/Review.cshtml:
Line 35:
Missing consent context in Web/Resgrid.Web/Areas/User/Views/Disclosures/Review.cshtml and Web/Resgrid.Web/Areas/User/Views/Records/Details.cshtml:145-145: the ReviewAttachment link can expose attachments that may contain sensitive personal data without an explicit consent or authorization identifier in the flow. Require an explicit consent context before review and propagate a consent identifier to the server-side action.
Existing code:
@Model.Records[i].Attachments[a].FileName
``` Improved code: ``` (none) ```
</details>
<sub>Talk to Kody by mentioning @kody</sub>
<sub>Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.</sub>
<!-- kody-codereview -->​
​
| <strong>#@revision.RevisionNumber</strong> @(((RmsRevisionTransition)revision.Transition).ToString()) | ||
| <a asp-controller="RecordDocuments" asp-action="Revision" asp-route-id="@a.Analysis.RmsIncidentAnalysisId" asp-route-kind="IncidentAnalysis" asp-route-revisionId="@revision.RmsRevisionId"><strong>#@revision.RevisionNumber</strong></a> @(((RmsRevisionTransition)revision.Transition).ToString()) | ||
| @if (revision.PriorRevisionId != null) { <a asp-controller="RecordDocuments" asp-action="Diff" asp-route-id="@a.Analysis.RmsIncidentAnalysisId" asp-route-kind="IncidentAnalysis" asp-route-from="@revision.PriorRevisionId" asp-route-to="@revision.RmsRevisionId">Compare</a> } | ||
| @if (ClaimsAuthorizationHelper.CanExportRecords()) { <a asp-controller="RecordDocuments" asp-action="Export" asp-route-id="@a.Analysis.RmsIncidentAnalysisId" asp-route-kind="IncidentAnalysis" asp-route-revisionId="@revision.RmsRevisionId" asp-route-format="pdf">PDF</a> } |
There was a problem hiding this comment.
Insufficient step-up authorization in Web/Resgrid.Web/Areas/User/Views/IncidentAnalysis/Details.cshtml: ClaimsAuthorizationHelper.CanExportRecords() alone does not demonstrate fresh MFA for a privileged PII or record-export action. Require step-up MFA within the last 5 minutes and record mfa_verified_at in the audit trail before rendering or honoring the export link.
Existing code:
@if (ClaimsAuthorizationHelper.CanExportRecords()) { <a asp-controller="RecordDocuments" asp-action="Export" asp-route-id="@a.Analysis.RmsIncidentAnalysisId" asp-route-kind="IncidentAnalysis" asp-route-revisionId="@revision.RmsRevisionId" asp-route-format="pdf">PDF</a> }
Improved code:
(none)
Kody rule violation: Require step-up MFA for privileged operations
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/IncidentAnalysis/Details.cshtml:
Line 270:
Insufficient step-up authorization in Web/Resgrid.Web/Areas/User/Views/IncidentAnalysis/Details.cshtml: ClaimsAuthorizationHelper.CanExportRecords() alone does not demonstrate fresh MFA for a privileged PII or record-export action. Require step-up MFA within the last 5 minutes and record mfa_verified_at in the audit trail before rendering or honoring the export link.
Existing code:
@if (ClaimsAuthorizationHelper.CanExportRecords()) { <a asp-controller="RecordDocuments" asp-action="Export" asp-route-id="@a.Analysis.RmsIncidentAnalysisId" asp-route-kind="IncidentAnalysis" asp-route-revisionId="@revision.RmsRevisionId" asp-route-format="pdf">PDF</a> }
Improved code:
(none)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @foreach (var attachment in a.Attachments.Where(x => !x.IsProtected || Model.CanViewRestricted)) | ||
| { | ||
| <p><a asp-controller="IncidentReports" asp-action="Attachment" asp-route-id="@r.RmsIncidentReportId" asp-route-attachmentId="@attachment.RmsRecordAttachmentId">@attachment.FileName</a> — @attachment.Description <small>@((RmsAttachmentScanState)attachment.ScanState) · @attachment.ByteSize bytes</small></p> |
There was a problem hiding this comment.
Authorization gap in Web/Resgrid.Web/Areas/User/Views/IncidentReports/Details.cshtml: the attachment filter checks only IsProtected, but classification-restricted attachments also depend on RequiresRestrictedAccess, which exposes restricted filenames, descriptions, and sizes to users without ViewRestrictedRecords until the download endpoint blocks access. Filter on attachment.RequiresRestrictedAccess, or pre-project attachments the same way the service and API do.
Existing code:
@foreach (var attachment in a.Attachments.Where(x => !x.IsProtected || Model.CanViewRestricted))
{
<p><a asp-controller="IncidentReports" asp-action="Attachment" asp-route-id="@r.RmsIncidentReportId" asp-route-attachmentId="@attachment.RmsRecordAttachmentId">@attachment.FileName</a> — @attachment.Description <small>@((RmsAttachmentScanState)attachment.ScanState) · @attachment.ByteSize bytes</small></p>
}
Improved code:
@foreach (var attachment in a.Attachments.Where(x => !x.RequiresRestrictedAccess || Model.CanViewRestricted))
{
<p><a asp-controller="IncidentReports" asp-action="Attachment" asp-route-id="@r.RmsIncidentReportId" asp-route-attachmentId="@attachment.RmsRecordAttachmentId">@attachment.FileName</a> — @attachment.Description <small>@((RmsAttachmentScanState)attachment.ScanState) · @attachment.ByteSize bytes</small></p>
}
@foreach (var attachment in a.Attachments.Where(x => !x.RequiresRestrictedAccess || Model.CanViewRestricted))
{
<p><a asp-controller="IncidentReports" asp-action="Attachment" asp-route-id="@r.RmsIncidentReportId" asp-route-attachmentId="@attachment.RmsRecordAttachmentId">@attachment.FileName</a> — @attachment.Description <small>@((RmsAttachmentScanState)attachment.ScanState) · @attachment.ByteSize bytes</small></p>
}Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/IncidentReports/Details.cshtml:
Line 62 to 64:
Authorization gap in Web/Resgrid.Web/Areas/User/Views/IncidentReports/Details.cshtml: the attachment filter checks only IsProtected, but classification-restricted attachments also depend on RequiresRestrictedAccess, which exposes restricted filenames, descriptions, and sizes to users without ViewRestrictedRecords until the download endpoint blocks access. Filter on attachment.RequiresRestrictedAccess, or pre-project attachments the same way the service and API do.
Existing code:
@foreach (var attachment in a.Attachments.Where(x => !x.IsProtected || Model.CanViewRestricted))
{
@attachment.FileName — @attachment.Description @((RmsAttachmentScanState)attachment.ScanState) · @attachment.ByteSize bytes
}
Improved code:
@foreach (var attachment in a.Attachments.Where(x => !x.RequiresRestrictedAccess || Model.CanViewRestricted))
{
@attachment.FileName — @attachment.Description @((RmsAttachmentScanState)attachment.ScanState) · @attachment.ByteSize bytes
}
Suggested Code:
@foreach (var attachment in a.Attachments.Where(x => !x.RequiresRestrictedAccess || Model.CanViewRestricted))
{
<p><a asp-controller="IncidentReports" asp-action="Attachment" asp-route-id="@r.RmsIncidentReportId" asp-route-attachmentId="@attachment.RmsRecordAttachmentId">@attachment.FileName</a> — @attachment.Description <small>@((RmsAttachmentScanState)attachment.ScanState) · @attachment.ByteSize bytes</small></p>
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| </form> | ||
|
|
||
| <link rel="stylesheet" href="~/css/neris-guided-form.css" asp-append-version="true" /> |
There was a problem hiding this comment.
Style-isolation issue in Web/Resgrid.Web/Areas/User/Views/IncidentReports/Edit.cshtml and the listed views: adding from the view introduces a global stylesheet that can leak across unrelated pages. Move these styles into a view-scoped or component-specific mechanism.
Existing code:
<link rel="stylesheet" href="~/css/neris-guided-form.css" asp-append-version="true" />
Improved code:
(none)
Kody rule violation: Use component-scoped styling
Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/IncidentReports/Edit.cshtml:
Line 588:
Style-isolation issue in Web/Resgrid.Web/Areas/User/Views/IncidentReports/Edit.cshtml and the listed views: adding <link rel="stylesheet" href="~/css/neris-guided-form.css" asp-append-version="true" /> from the view introduces a global stylesheet that can leak across unrelated pages. Move these styles into a view-scoped or component-specific mechanism.
Existing code:
</details>
<sub>Talk to Kody by mentioning @kody</sub>
<sub>Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.</sub>
<!-- kody-codereview -->​
​
| { | ||
| <h3>@artifact.Title</h3> | ||
| <p>@artifact.Reason</p> | ||
| <p>@artifact.Items source item(s) · captured @artifact.CapturedOn.ToString("yyyy-MM-dd HH:mm:ss") UTC · @(artifact.Superseded ? "Superseded" : "Current") · @(artifact.RevisionId == null ? "Draft" : "Signed revision: " + artifact.RevisionId)</p> |
There was a problem hiding this comment.
Sensitive metadata exposure in Web/Resgrid.Web/Areas/User/Views/RecordEvidence/Index.cshtml and the listed record views and services: rendering artifact.RevisionId discloses a detailed revision identifier for evidence-associated records. Replace the identifier with non-identifying status text such as signed-revision availability unless explicit access control requires the raw value.
Existing code:
<p>@artifact.Items source item(s) · captured @artifact.CapturedOn.ToString("yyyy-MM-dd HH:mm:ss") UTC · @(artifact.Superseded ? "Superseded" : "Current") · @(artifact.RevisionId == null ? "Draft" : "Signed revision: " + artifact.RevisionId)</p>
Improved code:
<p>@artifact.Items source item(s) · captured @artifact.CapturedOn.ToString("yyyy-MM-dd HH:mm:ss") UTC · @(artifact.Superseded ? "Superseded" : "Current") · @(artifact.RevisionId == null ? "Draft" : "Signed revision available")</p>
Kody rule violation: Do not log PHI; mask and drop sensitive fields
<p>@artifact.Items source item(s) · captured @artifact.CapturedOn.ToString("yyyy-MM-dd HH:mm:ss") UTC · @(artifact.Superseded ? "Superseded" : "Current") · @(artifact.RevisionId == null ? "Draft" : "Signed revision available")</p>Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/RecordEvidence/Index.cshtml:
Line 34:
Sensitive metadata exposure in Web/Resgrid.Web/Areas/User/Views/RecordEvidence/Index.cshtml and the listed record views and services: rendering artifact.RevisionId discloses a detailed revision identifier for evidence-associated records. Replace the identifier with non-identifying status text such as signed-revision availability unless explicit access control requires the raw value.
Existing code:
<p>@artifact.Items source item(s) · captured @artifact.CapturedOn.ToString("yyyy-MM-dd HH:mm:ss") UTC · @(artifact.Superseded ? "Superseded" : "Current") · @(artifact.RevisionId == null ? "Draft" : "Signed revision: " + artifact.RevisionId)</p>
Improved code:
<p>@artifact.Items source item(s) · captured @artifact.CapturedOn.ToString("yyyy-MM-dd HH:mm:ss") UTC · @(artifact.Superseded ? "Superseded" : "Current") · @(artifact.RevisionId == null ? "Draft" : "Signed revision available")</p>
Suggested Code:
<p>@artifact.Items source item(s) · captured @artifact.CapturedOn.ToString("yyyy-MM-dd HH:mm:ss") UTC · @(artifact.Superseded ? "Superseded" : "Current") · @(artifact.RevisionId == null ? "Draft" : "Signed revision available")</p>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| @@ -104,7 +104,7 @@ | |||
| <tr><td class="label">@localizer["OtherPersonnel"]</td><td>@Text(details.OtherPersonnel)</td></tr> | |||
| } | |||
| </table> | |||
| <p class="narrative">@details.Narrative</p> | |||
| <div class="narrative">@Html.Raw(Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative))</div> | |||
There was a problem hiding this comment.
XSS risk in Web/Resgrid.Web/Areas/User/Views/Records/Print.cshtml and Web/Resgrid.Web/Areas/User/Views/Records/Details.cshtml:145-145: Html.Raw(Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative)) bypasses Razor output encoding for potentially user-controlled narrative content. Render encoded output unless RecordNarrativeFormatter.Render(details.Narrative) guarantees sanitized HTML.
Existing code:
<div class="narrative">@Html.Raw(Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative))</div>
Improved code:
<div class="narrative">@Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative)</div>
Kody rule violation: Always sanitize user inputs
<div class="narrative">@Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative)</div>Prompt for LLM
File Web/Resgrid.Web/Areas/User/Views/Records/Print.cshtml:
Line 107:
XSS risk in Web/Resgrid.Web/Areas/User/Views/Records/Print.cshtml and Web/Resgrid.Web/Areas/User/Views/Records/Details.cshtml:145-145: Html.Raw(Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative)) bypasses Razor output encoding for potentially user-controlled narrative content. Render encoded output unless RecordNarrativeFormatter.Render(details.Narrative) guarantees sanitized HTML.
Existing code:
<div class="narrative">@Html.Raw(Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative))</div>
Improved code:
<div class="narrative">@Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative)</div>
Suggested Code:
<div class="narrative">@Resgrid.Framework.RecordNarrativeFormatter.Render(details.Narrative)</div>
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return new Tuple<bool, string>(true, "Search host disabled; nothing to do."); | ||
|
|
||
| var maintenance = Bootstrapper.GetKernel().Resolve<IRecordsSearchIndexMaintenanceService>(); | ||
| using var scope = Bootstrapper.GetKernel().BeginLifetimeScope(); |
There was a problem hiding this comment.
Async disposal risk in Workers/Resgrid.Workers.Framework/Logic/RecordsSearchIndexLogic.cs and Tests/Resgrid.Tests/Rms/IncidentOfficerJourneyTests.cs:131-131: if Bootstrapper.GetKernel().BeginLifetimeScope() returns an async-disposable scope, using var can block or skip asynchronous cleanup in an async method. Use await using for deterministic async disposal.
Existing code:
using var scope = Bootstrapper.GetKernel().BeginLifetimeScope();
Improved code:
await using var scope = Bootstrapper.GetKernel().BeginLifetimeScope();
Kody rule violation: Use using statements for disposable resources
await using var scope = Bootstrapper.GetKernel().BeginLifetimeScope();Prompt for LLM
File Workers/Resgrid.Workers.Framework/Logic/RecordsSearchIndexLogic.cs:
Line 22:
Async disposal risk in Workers/Resgrid.Workers.Framework/Logic/RecordsSearchIndexLogic.cs and Tests/Resgrid.Tests/Rms/IncidentOfficerJourneyTests.cs:131-131: if Bootstrapper.GetKernel().BeginLifetimeScope() returns an async-disposable scope, using var can block or skip asynchronous cleanup in an async method. Use await using for deterministic async disposal.
Existing code:
using var scope = Bootstrapper.GetKernel().BeginLifetimeScope();
Improved code:
await using var scope = Bootstrapper.GetKernel().BeginLifetimeScope();
Suggested Code:
await using var scope = Bootstrapper.GetKernel().BeginLifetimeScope();
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 a broad RMS hardening update across records, incident reporting, evidence, disclosures, retention, authorization, and NERIS submission workflows.
What changed
Records and incident reports
Departmental record output and review
Evidence and inventory
Disclosures
Retention and legal hold
Search
Authorization and visibility
NERIS submission hardening
API and idempotency
Infrastructure and workers
Terminology and localization
CallArrivalmeans the call arriving at dispatch/PSAP, not first unit on scene.CallArrivallabel across supported languages to reflect that meaning.Functional impact
These changes make RMS workflows safer and more auditable by preserving immutable revision content, enforcing live authorization at read/write time, preventing unsafe retries or stale overwrites, supporting departmental custom fields and richer disclosures, and strengthening retention, evidence, and NERIS submission handling.