LEGLINK-651: Add Automated & Manual Data Cleanup - #1740
Open
nvmLantana wants to merge 5 commits into
Open
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds normalization operation, sequence, and suite models with MongoDB persistence, startup seeding, MVC management screens, scenario suite selection, and suite-driven normalization configuration during automation runs. ChangesNormalization suite lifecycle
Sequence Diagram(s)sequenceDiagram
participant ScenarioEditor
participant StartScenarioRequestResolver
participant RunExecutor
participant NormalizationSuiteResolver
participant NormalizationAPI
ScenarioEditor->>StartScenarioRequestResolver: Submit NormalizationSuiteId
StartScenarioRequestResolver->>RunExecutor: Provide resolved run options
RunExecutor->>NormalizationSuiteResolver: Resolve requested or default suite
NormalizationSuiteResolver-->>RunExecutor: Return ordered operations
RunExecutor->>NormalizationAPI: Create operations and sequences
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
DotNet/Automation.UI/Services/RunExecutor.cs (1)
962-989: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd focused XUnit coverage for every operation mapping branch.
Cover CopyProperty/default, ConditionalTransform, CodeMap, and RemoveExtensions. Mock
INormalizationServiceClientwith Moq; extracting the mapper would keep these tests small and network-free.As per path instructions, each introduced switch branch requires a unit test, and unit tests must mock external communication with Moq.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNet/Automation.UI/Services/RunExecutor.cs` around lines 962 - 989, Add focused XUnit tests for the operation-mapping logic around the switch in RunExecutor, covering CopyProperty/default, ConditionalTransform, CodeMap, and RemoveExtensions. Mock INormalizationServiceClient with Moq so tests remain network-free; extract the mapper into a testable method if needed, and assert each branch produces the expected API model fields and nested mappings.Source: Path instructions
DotNet/Automation.UI/Services/Persistence/MongoNormalizationStore.cs (1)
49-52: 🗄️ Data Integrity & Integration | 🔵 TrivialNo referential-integrity check when deleting operations/sequences still referenced by a sequence/suite.
DeleteOperationAsync/DeleteSequenceAsyncunconditionally remove the document even if it's still referenced from aNormalizationSequenceDefinition.EntriesorNormalizationSuiteDefinition.OperationIds/SequenceIds.NormalizationSuiteResolver(context snippet) skips missing lookups gracefully, so this won't crash, but a suite silently loses an operation with no warning surfaced to the admin managing it.Also applies to: 76-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DotNet/Automation.UI/Services/Persistence/MongoNormalizationStore.cs` around lines 49 - 52, Update DeleteOperationAsync and DeleteSequenceAsync to check existing NormalizationSequenceDefinition.Entries and NormalizationSuiteDefinition.OperationIds/SequenceIds references before deleting. Reject or otherwise prevent deletion when references exist, while preserving deletion for unreferenced documents and surfacing the reference conflict to the caller.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DotNet/Automation.UI/Services/NormalizationSuiteResolver.cs`:
- Around line 43-59: The suite resolution logic around sequenceIds and
operationIds must reject dangling references instead of silently skipping them.
Update the sequence lookup and opLookup branches to fail with the unresolved
sequence or operation ID (or perform equivalent referential-integrity validation
before persistence), while preserving successful resolution for valid
references. Add XUnit coverage for both missing-reference branches and ensure
each introduced or modified conditional branch is tested.
In `@DotNet/Automation.UI/Services/NormalizationSuiteSeedService.cs`:
- Around line 111-124: Update the default-suite seeding flow around defaultSuite
and UpsertSuiteAsync so the system suite is not assigned IsDefault = true during
every startup. Preserve any existing administrator-selected default, then call
SetDefaultSuiteAsync only when no default suite exists, ensuring the system
suite becomes default only as the initial fallback.
In `@DotNet/Automation.UI/Services/Persistence/MongoNormalizationStore.cs`:
- Around line 109-116: Update SetDefaultSuiteAsync to make replacing the default
suite atomic and concurrency-safe, using a MongoDB transaction or equivalent
single atomic operation so failures cannot leave zero defaults and concurrent
calls cannot create multiple defaults. Also update idx_isDefault in
MongoIndexManager to a partial unique index covering only documents where
IsDefault is true, preserving GetDefaultSuiteAsync’s existing behavior.
In `@DotNet/Automation.UI/Services/RunExecutor.cs`:
- Around line 929-935: Update the existing-configuration handling in RunExecutor
around SearchFacilityOperationsAsync so it reconciles records against the
resolved operation suite instead of returning whenever any record exists.
Identify missing or disabled operations and sequences, then idempotently create
or update them; only skip processing once the complete required normalization
configuration is present.
- Around line 1017-1049: The sequence-building logic around opsByResourceType
must preserve the resolved suite order from resolution.Operations instead of
using the unordered, potentially truncated SearchFacilityOperationsAsync
records; correlate the created API operation IDs to resolution.Operations and
assign each resource sequence in that order. If the API re-query is still
required for correlation, page through all results before building the
sequences.
- Around line 1000-1004: Update the normalization setup flow in RunExecutor
around the operation and sequence creation checks so any unsuccessful creation
response fails the run instead of continuing with partial configuration. Throw a
contextual exception that identifies the failed normalization item and HTTP
status, or roll back all partial normalization configuration before proceeding;
apply the same behavior to both failure branches.
In `@DotNet/Automation.UI/Views/Normalizations/Index.cshtml`:
- Around line 456-468: Remove the stored-XSS risk from the dynamic row builders
`addConditionRow`, `addSeqEntryRow`, `addSuiteSeqRow`, and `addSuiteOpRow` by
escaping every untrusted `fhirPathSource`, condition `value`, and
operation/sequence `name` before template interpolation, or by constructing
these elements with safe DOM APIs and assigning text through `textContent`.
Ensure option labels, input values, and generated markup cannot interpret
admin-supplied HTML while preserving the existing row and selection behavior.
---
Nitpick comments:
In `@DotNet/Automation.UI/Services/Persistence/MongoNormalizationStore.cs`:
- Around line 49-52: Update DeleteOperationAsync and DeleteSequenceAsync to
check existing NormalizationSequenceDefinition.Entries and
NormalizationSuiteDefinition.OperationIds/SequenceIds references before
deleting. Reject or otherwise prevent deletion when references exist, while
preserving deletion for unreferenced documents and surfacing the reference
conflict to the caller.
In `@DotNet/Automation.UI/Services/RunExecutor.cs`:
- Around line 962-989: Add focused XUnit tests for the operation-mapping logic
around the switch in RunExecutor, covering CopyProperty/default,
ConditionalTransform, CodeMap, and RemoveExtensions. Mock
INormalizationServiceClient with Moq so tests remain network-free; extract the
mapper into a testable method if needed, and assert each branch produces the
expected API model fields and nested mappings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6189e0ca-c823-406f-ac71-382e4279af91
📒 Files selected for processing (20)
DotNet/Automation.UI/Controllers/NormalizationsController.csDotNet/Automation.UI/Controllers/ScenariosController.csDotNet/Automation.UI/Models/NormalizationModels.csDotNet/Automation.UI/Models/StartScenarioRequest.csDotNet/Automation.UI/Models/TestScenarioDefinition.csDotNet/Automation.UI/Program.csDotNet/Automation.UI/Services/AutomationRunManager.csDotNet/Automation.UI/Services/NormalizationSuiteResolver.csDotNet/Automation.UI/Services/NormalizationSuiteSeedService.csDotNet/Automation.UI/Services/Persistence/INormalizationStore.csDotNet/Automation.UI/Services/Persistence/MongoIndexManager.csDotNet/Automation.UI/Services/Persistence/MongoNormalizationStore.csDotNet/Automation.UI/Services/ResolvedRunOptions.csDotNet/Automation.UI/Services/RunExecutor.csDotNet/Automation.UI/Services/StartScenarioRequestResolver.csDotNet/Automation.UI/Views/Normalizations/Index.cshtmlDotNet/Automation.UI/Views/Shared/_Layout.cshtmlDotNet/Automation.UI/Views/Shared/_ScenarioEditorModal.cshtmlDotNet/ServiceTests/IntegrationTests/Automation.UI/AutomationUIIntegrationTestFixture.csDotNet/Shared/Application/Models/Integration/Normalization/NormalizationApiModels.cs
Comment on lines
+43
to
+59
| foreach (var seqId in suite.SequenceIds) | ||
| { | ||
| var seq = allSequences.FirstOrDefault(s => s.Id == seqId); | ||
| if (seq == null) continue; | ||
|
|
||
| foreach (var entry in seq.Entries.OrderBy(e => e.Sequence)) | ||
| { | ||
| if (opLookup.TryGetValue(entry.OperationId, out var op)) | ||
| resolvedOps.Add(op); | ||
| } | ||
| } | ||
|
|
||
| // Add standalone operations from the suite. | ||
| foreach (var opId in suite.OperationIds) | ||
| { | ||
| if (opLookup.TryGetValue(opId, out var op)) | ||
| resolvedOps.Add(op); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject suites containing dangling sequence or operation references.
Missing IDs are silently skipped, so a run can apply only part of the selected suite while reporting successful resolution. Fail with the unresolved IDs or validate referential integrity before persistence, and add XUnit coverage for both missing-reference branches.
As per path instructions, “If/Else or Switch/Case blocks are introduced or modified — ensure each branch has a corresponding unit test.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNet/Automation.UI/Services/NormalizationSuiteResolver.cs` around lines 43
- 59, The suite resolution logic around sequenceIds and operationIds must reject
dangling references instead of silently skipping them. Update the sequence
lookup and opLookup branches to fail with the unresolved sequence or operation
ID (or perform equivalent referential-integrity validation before persistence),
while preserving successful resolution for valid references. Add XUnit coverage
for both missing-reference branches and ensure each introduced or modified
conditional branch is tested.
Source: Path instructions
Comment on lines
+111
to
+124
| var defaultSuite = new NormalizationSuiteDefinition | ||
| { | ||
| Id = SuiteSystemDefaultId, | ||
| Name = "System Default", | ||
| Description = "Built-in normalization suite that applies location normalization and extension cleanup.", | ||
| OperationIds = [], | ||
| SequenceIds = [SeqDefaultLocationId, SeqDefaultCleanupId], | ||
| IsSystem = true, | ||
| IsDefault = true, | ||
| UpdatedAt = DateTimeOffset.UtcNow | ||
| }; | ||
|
|
||
| await _store.UpsertSuiteAsync(defaultSuite, cancellationToken); | ||
| _logger.LogInformation("Seeded/refreshed system default normalization suite: {Id}", SuiteSystemDefaultId); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not reset the administrator-selected default during every startup.
Upserting the system suite with IsDefault = true can either replace a custom default or leave multiple defaults. Preserve the current default and call SetDefaultSuiteAsync only when no default exists.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNet/Automation.UI/Services/NormalizationSuiteSeedService.cs` around lines
111 - 124, Update the default-suite seeding flow around defaultSuite and
UpsertSuiteAsync so the system suite is not assigned IsDefault = true during
every startup. Preserve any existing administrator-selected default, then call
SetDefaultSuiteAsync only when no default suite exists, ensuring the system
suite becomes default only as the initial fallback.
Comment on lines
+109
to
+116
| public async Task SetDefaultSuiteAsync(Guid id, CancellationToken ct = default) | ||
| { | ||
| var clearUpdate = Builders<NormalizationSuiteDocument>.Update.Set(d => d.IsDefault, false); | ||
| await _suites.UpdateManyAsync(_ => true, clearUpdate, cancellationToken: ct); | ||
|
|
||
| var setUpdate = Builders<NormalizationSuiteDocument>.Update.Set(d => d.IsDefault, true); | ||
| await _suites.UpdateOneAsync(d => d.Id == id, setUpdate, cancellationToken: ct); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
Non-atomic default-suite swap can leave zero or multiple defaults.
SetDefaultSuiteAsync clears all IsDefault flags, then sets the target one, as two separate writes with no transaction. A failure between the two calls (or two concurrent admin calls) can leave no default suite, or — since idx_isDefault (MongoIndexManager.cs) is not a partial-unique index — multiple suites marked default. GetDefaultSuiteAsync's unsorted FirstOrDefaultAsync would then return whichever doc happens to match, silently changing which normalization suite a run applies.
🔧 Suggested fix: enforce a single default at the data layer
- var clearUpdate = Builders<NormalizationSuiteDocument>.Update.Set(d => d.IsDefault, false);
- await _suites.UpdateManyAsync(_ => true, clearUpdate, cancellationToken: ct);
-
- var setUpdate = Builders<NormalizationSuiteDocument>.Update.Set(d => d.IsDefault, true);
- await _suites.UpdateOneAsync(d => d.Id == id, setUpdate, cancellationToken: ct);
+ // Clear the previous default(s) first, then set the new one. Pair this with a
+ // partial unique index on { IsDefault: true } (see MongoIndexManager) so the DB
+ // itself rejects a second concurrent "set default" from ever landing.
+ var clearUpdate = Builders<NormalizationSuiteDocument>.Update.Set(d => d.IsDefault, false);
+ await _suites.UpdateManyAsync(d => d.IsDefault, clearUpdate, cancellationToken: ct);
+
+ var setUpdate = Builders<NormalizationSuiteDocument>.Update.Set(d => d.IsDefault, true);
+ var result = await _suites.UpdateOneAsync(d => d.Id == id, setUpdate, cancellationToken: ct);
+ if (result.MatchedCount == 0)
+ throw new InvalidOperationException($"Suite {id} not found; default was cleared but not reassigned.");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNet/Automation.UI/Services/Persistence/MongoNormalizationStore.cs` around
lines 109 - 116, Update SetDefaultSuiteAsync to make replacing the default suite
atomic and concurrency-safe, using a MongoDB transaction or equivalent single
atomic operation so failures cannot leave zero defaults and concurrent calls
cannot create multiple defaults. Also update idx_isDefault in MongoIndexManager
to a partial unique index covering only documents where IsDefault is true,
preserving GetDefaultSuiteAsync’s existing behavior.
Comment on lines
+929
to
+935
| // Check if the facility already has operations configured. | ||
| var existingResp = await normalizationClient.SearchFacilityOperationsAsync(facilityId); | ||
| if (existingResp.IsSuccessStatusCode && existingResp.Body?.Records?.Count > 0) | ||
| { | ||
| output.WriteLine($"Normalization config for facility '{facilityId}' already exists. Skipping create."); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Reconcile existing normalization configuration instead of treating any record as complete.
A partial previous attempt—or only a disabled operation—causes an immediate return, leaving missing operations and sequences unrepaired. Compare existing state against the resolved suite and idempotently create or update the missing configuration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNet/Automation.UI/Services/RunExecutor.cs` around lines 929 - 935, Update
the existing-configuration handling in RunExecutor around
SearchFacilityOperationsAsync so it reconciles records against the resolved
operation suite instead of returning whenever any record exists. Identify
missing or disabled operations and sequences, then idempotently create or update
them; only skip processing once the complete required normalization
configuration is present.
Comment on lines
+1000
to
+1004
| if (!createResp.IsSuccessStatusCode) | ||
| { | ||
| output.WriteLine($" WARNING: Failed to create normalization operation '{opDef.Name}': HTTP {createResp.StatusCode}"); | ||
| continue; | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail the run when normalization setup is incomplete.
Continuing after an operation or sequence creation failure runs the scenario with partial normalization and produces misleading results. Throw a contextual exception or roll back the partial configuration.
Also applies to: 1051-1055
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNet/Automation.UI/Services/RunExecutor.cs` around lines 1000 - 1004,
Update the normalization setup flow in RunExecutor around the operation and
sequence creation checks so any unsuccessful creation response fails the run
instead of continuing with partial configuration. Throw a contextual exception
that identifies the failed normalization item and HTTP status, or roll back all
partial normalization configuration before proceeding; apply the same behavior
to both failure branches.
Comment on lines
+1017
to
+1049
| // Create sequences per resource type. | ||
| // We need to get operations back from the API since the create response may not give IDs directly. | ||
| // Instead, re-search to find newly created ops and build sequences. | ||
| var opsResp = await normalizationClient.SearchFacilityOperationsAsync(facilityId, pageSize: 100, cancellationToken: cancellationToken); | ||
| if (opsResp.IsSuccessStatusCode && opsResp.Body?.Records?.Count > 0) | ||
| { | ||
| // Group by resource type and create sequences in order. | ||
| var opsByResourceType = new Dictionary<string, List<(Guid OpId, int Seq)>>(StringComparer.OrdinalIgnoreCase); | ||
| int seq = 0; | ||
| foreach (var op in opsResp.Body.Records) | ||
| { | ||
| seq++; | ||
| foreach (var ort in op.OperationResourceTypes) | ||
| { | ||
| var resourceName = ort.Resource?.ResourceName; | ||
| if (string.IsNullOrEmpty(resourceName)) continue; | ||
|
|
||
| if (!opsByResourceType.ContainsKey(resourceName)) | ||
| opsByResourceType[resourceName] = []; | ||
| opsByResourceType[resourceName].Add((op.Id, seq)); | ||
| } | ||
| } | ||
|
|
||
| foreach (var (resourceType, ops) in opsByResourceType) | ||
| { | ||
| var sequences = ops | ||
| .OrderBy(o => o.Seq) | ||
| .Select((o, idx) => new CreateNormalizationOperationSequenceApiModel | ||
| { | ||
| OperationId = o.OpId, | ||
| Sequence = idx + 1 | ||
| }) | ||
| .ToList(); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve resolved suite order when creating API sequences.
The code discards globalOrder and assigns sequence numbers from the API search result order, which is not guaranteed and is truncated at 100 records. Correlate created API IDs with resolution.Operations and build each resource sequence directly in that order; page all results if re-querying remains necessary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNet/Automation.UI/Services/RunExecutor.cs` around lines 1017 - 1049, The
sequence-building logic around opsByResourceType must preserve the resolved
suite order from resolution.Operations instead of using the unordered,
potentially truncated SearchFacilityOperationsAsync records; correlate the
created API operation IDs to resolution.Operations and assign each resource
sequence in that order. If the API re-query is still required for correlation,
page through all results before building the sequences.
Comment on lines
+456
to
+468
| function addConditionRow(cond) { | ||
| const container = document.getElementById('opConditionsContainer'); | ||
| const idx = container.children.length; | ||
| const operators = ['Equal','NotEqual','GreaterThan','GreaterThanOrEqual','LessThan','LessThanOrEqual','Exists','NotExists']; | ||
| const row = document.createElement('div'); | ||
| row.className = 'row g-2 mb-1 align-items-center'; | ||
| row.innerHTML = ` | ||
| <div class="col-4"><input class="form-control form-control-sm cond-fhirpath" placeholder="FhirPath" value="${cond?.fhirPathSource || ''}"/></div> | ||
| <div class="col-3"><select class="form-select form-select-sm cond-operator">${operators.map(o => `<option value="${o}" ${o === (cond?.operator || 'Equal') ? 'selected' : ''}>${o}</option>`).join('')}</select></div> | ||
| <div class="col-3"><input class="form-control form-control-sm cond-value" placeholder="Value" value="${cond?.value ?? ''}"/></div> | ||
| <div class="col-2"><button type="button" class="btn btn-sm btn-outline-danger" onclick="this.closest('.row').remove()"><i class="bi bi-x-lg"></i></button></div>`; | ||
| container.appendChild(row); | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Stored XSS via unescaped innerHTML interpolation of operation/sequence names and values.
addConditionRow, addSeqEntryRow, addSuiteSeqRow, and addSuiteOpRow all build HTML via template literals that interpolate untrusted, admin-supplied strings (Name, FhirPathSource, condition Value) and assign the result to .innerHTML. Any operation/sequence Name or condition value containing markup (e.g. "><img src=x onerror=alert(document.cookie)>) executes for every user who later opens a Sequence/Suite editor, since these functions rebuild option lists from the entire allOperations/allSequences arrays — not just the record being edited. This can be used to exfiltrate the page's anti-forgery token or perform actions as another admin.
🔒 Suggested fix: escape before interpolating (or use safe DOM APIs)
+ function escapeHtml(s) {
+ return String(s ?? '').replace(/[&<>"']/g, c => ({ '&':'&', '<':'<', '>':'>', '"':'"', "'":'&`#39`;' }[c]));
+ }
+
function addConditionRow(cond) {
...
row.innerHTML = `
- <div class="col-4"><input class="form-control form-control-sm cond-fhirpath" placeholder="FhirPath" value="${cond?.fhirPathSource || ''}"/></div>
- <div class="col-3"><select class="form-select form-select-sm cond-operator">${operators.map(o => `<option value="${o}" ${o === (cond?.operator || 'Equal') ? 'selected' : ''}>${o}</option>`).join('')}</select></div>
- <div class="col-3"><input class="form-control form-control-sm cond-value" placeholder="Value" value="${cond?.value ?? ''}"/></div>
+ <div class="col-4"><input class="form-control form-control-sm cond-fhirpath" placeholder="FhirPath" value="${escapeHtml(cond?.fhirPathSource)}"/></div>
+ <div class="col-3"><select class="form-select form-select-sm cond-operator">${operators.map(o => `<option value="${o}" ${o === (cond?.operator || 'Equal') ? 'selected' : ''}>${o}</option>`).join('')}</select></div>
+ <div class="col-3"><input class="form-control form-control-sm cond-value" placeholder="Value" value="${escapeHtml(cond?.value)}"/></div>
<div class="col-2">...</div>`;
}Apply the same escapeHtml(...) wrapping to the o.name/s.name interpolations in addSeqEntryRow, addSuiteSeqRow, and addSuiteOpRow.
Also applies to: 554-565, 643-663
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DotNet/Automation.UI/Views/Normalizations/Index.cshtml` around lines 456 -
468, Remove the stored-XSS risk from the dynamic row builders `addConditionRow`,
`addSeqEntryRow`, `addSuiteSeqRow`, and `addSuiteOpRow` by escaping every
untrusted `fhirPathSource`, condition `value`, and operation/sequence `name`
before template interpolation, or by constructing these elements with safe DOM
APIs and assigning text through `textContent`. Ensure option labels, input
values, and generated markup cannot interpret admin-supplied HTML while
preserving the existing row and selection behavior.
1 task
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠️ Description of Changes
Please provide a high-level overview of the changes included in this PR.
🧪 Testing Performed
Please describe the testing that was performed on the changes included in this PR.
🧑🔬 Unit Testing
📓 Documentation Updated
Please update any relevant sections in the project documentation that were impacted by the changes in the PR.
Summary by CodeRabbit