Skip to content

Commit d66b4fa

Browse files
authored
Merge pull request #424 from aurelianware/copilot/fix-security-scan-issues
2 parents 6856dac + 5f6903e commit d66b4fa

48 files changed

Lines changed: 405 additions & 122 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

containers/claims-publisher/Dockerfile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,10 @@ RUN apk add --no-cache \
1313
COPY publish-claims.sh /app/
1414
RUN chmod +x /app/publish-claims.sh
1515

16+
# Create non-root user
17+
RUN addgroup -S appuser && adduser -S -G appuser appuser \
18+
&& chown -R appuser:appuser /app
19+
USER appuser
20+
1621
# Set entrypoint
1722
ENTRYPOINT ["/app/publish-claims.sh"]

containers/x12-834-parser/Dockerfile

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@ COPY parse-834.js ./
1414
# Make script executable
1515
RUN chmod +x parse-834.js
1616

17-
# Create input/output directories
18-
RUN mkdir -p /input /output
17+
# Create input/output directories and set permissions
18+
RUN mkdir -p /input /output \
19+
&& chown -R node:node /app /input /output
1920

2021
# Set environment variables
2122
ENV INPUT_DIR=/input
2223
ENV OUTPUT_DIR=/output
2324

25+
# Run as non-root user
26+
USER node
27+
2428
CMD ["node", "parse-834.js"]

scripts/utils/template-helpers.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,9 @@ export function registerHelpers(): void {
209209

210210
Handlebars.registerHelper('replace', function(str: string, search: string, replacement: string) {
211211
if (!str) return '';
212-
return str.replace(new RegExp(search, 'g'), replacement);
212+
// Escape regex special characters from the search string to prevent regex injection
213+
const escapedSearch = search.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
214+
return str.replace(new RegExp(escapedSearch, 'g'), replacement);
213215
});
214216

215217
Handlebars.registerHelper('trim', function(str: string) {

src/ai/redaction.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,11 @@ export function isPHIFieldName(fieldName: string): boolean {
9797
if (lowerFieldName.includes('_' + lowerPhiName + '_')) return true;
9898
// CamelCase suffix match (e.g., "patientEmail" contains "Email")
9999
// Match if PHI name appears as a capitalized word at the end
100-
const camelCasePattern = new RegExp(lowerPhiName + '$', 'i');
100+
const escapedPhiName = lowerPhiName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
101+
const camelCasePattern = new RegExp(escapedPhiName + '$', 'i');
101102
if (camelCasePattern.test(lowerFieldName)) return true;
102103
// Word boundary match for other cases (e.g., "name" as a whole word)
103-
const wordBoundaryRegex = new RegExp(`\\b${lowerPhiName}\\b`, 'i');
104+
const wordBoundaryRegex = new RegExp(`\\b${escapedPhiName}\\b`, 'i');
104105
if (wordBoundaryRegex.test(fieldName)) return true;
105106
return false;
106107
});

src/fhir/patient-access-api.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,10 @@ export class PatientAccessApi {
699699
*/
700700
logAudit(entry: AuditLogEntry): void {
701701
this.auditLogs.push(entry);
702-
console.log(`[AUDIT] ${entry.timestamp} - ${entry.eventType} - ${entry.result} - User: ${entry.userId} - ${entry.details || ''}`);
702+
// Sanitize user-controlled values to prevent log injection
703+
const safeUserId = String(entry.userId || '').replace(/[\r\n]/g, '');
704+
const safeDetails = String(entry.details || '').replace(/[\r\n]/g, '');
705+
console.log(`[AUDIT] ${entry.timestamp} - ${entry.eventType} - ${entry.result} - User: ${safeUserId} - ${safeDetails}`);
703706
}
704707

705708
/**

src/fhir/provider-access-api.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -749,8 +749,10 @@ export class ProviderAccessApi {
749749
// In production: send to Azure Application Insights or Log Analytics
750750
this.auditLogs.push(entry);
751751

752-
// For development: log to console
753-
console.log(`[AUDIT] ${entry.timestamp} - ${entry.eventType} - ${entry.result} - User: ${entry.userId} - ${entry.details || ''}`);
752+
// For development: log to console (sanitize user-controlled values to prevent log injection)
753+
const safeUserId = String(entry.userId || '').replace(/[\r\n]/g, '');
754+
const safeDetails = String(entry.details || '').replace(/[\r\n]/g, '');
755+
console.log(`[AUDIT] ${entry.timestamp} - ${entry.eventType} - ${entry.result} - User: ${safeUserId} - ${safeDetails}`);
754756
}
755757

756758
/**

src/services/appeals-service/Controllers/AppealsController.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public AppealsController(
2828
[ProducesResponseType(StatusCodes.Status400BadRequest)]
2929
public async Task<ActionResult<Appeal>> SubmitAppeal([FromBody] Appeal appeal)
3030
{
31-
_logger.LogInformation("Submitting appeal for claim {ClaimId}", appeal.ClaimId);
31+
_logger.LogInformation("Submitting appeal for claim {ClaimId}", SanitizeForLog(appeal.ClaimId));
3232

3333
// Validation
3434
if (string.IsNullOrEmpty(appeal.ClaimId))
@@ -118,7 +118,7 @@ public async Task<ActionResult<IEnumerable<Appeal>>> SearchAppeals(
118118
[FromQuery] int pageSize = 50)
119119
{
120120
_logger.LogInformation("Searching appeals: member {Member}, provider {Provider}, status {Status}",
121-
memberId, providerNPI, status);
121+
SanitizeForLog(memberId), SanitizeForLog(providerNPI), status);
122122

123123
var appeals = await _appealRepository.SearchAsync(
124124
memberId, providerNPI, submittedFrom, submittedTo, status, lineOfBusiness, page, pageSize);
@@ -153,7 +153,7 @@ public async Task<ActionResult<Appeal>> AddAttachment(string id, [FromBody] Appe
153153
var updated = await _appealRepository.UpdateAsync(appeal);
154154

155155
_logger.LogInformation("Added attachment {AttachmentId} to appeal {AppealId}",
156-
attachment.AttachmentId, id);
156+
SanitizeForLog(attachment.AttachmentId), SanitizeForLog(id));
157157

158158
return Ok(updated);
159159
}
@@ -248,7 +248,7 @@ public async Task<ActionResult<Appeal>> SubmitDecision(string id, [FromBody] App
248248

249249
var updated = await _appealRepository.UpdateAsync(appeal);
250250

251-
_logger.LogInformation("Appeal {AppealId} decision: {Decision}", id, decision.DecisionType);
251+
_logger.LogInformation("Appeal {AppealId} decision: {Decision}", SanitizeForLog(id), decision.DecisionType);
252252

253253
return Ok(updated);
254254
}
@@ -291,6 +291,13 @@ public async Task<ActionResult<AppealsSummary>> GetAppealsSummary(
291291

292292
return Ok(summary);
293293
}
294+
295+
private static string SanitizeForLog(string? value)
296+
{
297+
if (string.IsNullOrEmpty(value))
298+
return string.Empty;
299+
return value.Replace("\r", string.Empty).Replace("\n", string.Empty);
300+
}
294301
}
295302

296303
public class UpdateAttachmentStatusRequest
@@ -309,3 +316,4 @@ public class UpdateStatusRequest
309316
{
310317
public AppealStatus Status { get; set; }
311318
}
319+

src/services/appeals-service/Middleware/TenantMiddleware.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ public async Task InvokeAsync(HttpContext context)
4444
if (string.IsNullOrEmpty(tenantId))
4545
{
4646
tenantId = "default-tenant";
47-
_logger.LogWarning("No TenantId found in JWT or headers, using default: {TenantId}", tenantId);
47+
_logger.LogWarning("No TenantId found in JWT or headers, using default: {TenantId}", SanitizeForLog(tenantId));
4848
}
4949

5050
// Store in HttpContext for repository access
5151
context.Items["TenantId"] = tenantId;
5252

53-
_logger.LogDebug("Request TenantId: {TenantId}", tenantId);
53+
_logger.LogDebug("Request TenantId: {TenantId}", SanitizeForLog(tenantId));
5454

5555
await _next(context);
5656
}
@@ -60,6 +60,13 @@ private static bool IsHealthCheckPath(PathString path)
6060
return path.StartsWithSegments("/health")
6161
|| path.StartsWithSegments("/swagger");
6262
}
63+
64+
private static string SanitizeForLog(string? value)
65+
{
66+
if (string.IsNullOrEmpty(value))
67+
return string.Empty;
68+
return value.Replace("\r", string.Empty).Replace("\n", string.Empty);
69+
}
6370
}
6471

6572
public static class TenantMiddlewareExtensions

src/services/appeals-service/Repositories/AppealRepository.cs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ public async Task<Appeal> CreateAsync(Appeal appeal)
249249
new PartitionKey(appeal.TenantId));
250250

251251
_logger.LogInformation("Created appeal {AppealId} for claim {ClaimId}",
252-
appeal.Id, appeal.ClaimId);
252+
SanitizeForLog(appeal.Id), SanitizeForLog(appeal.ClaimId));
253253

254254
return response.Resource;
255255
}
@@ -261,7 +261,7 @@ public async Task<Appeal> UpdateAsync(Appeal appeal)
261261
appeal.Id,
262262
new PartitionKey(appeal.TenantId));
263263

264-
_logger.LogInformation("Updated appeal {AppealId}", appeal.Id);
264+
_logger.LogInformation("Updated appeal {AppealId}", SanitizeForLog(appeal.Id));
265265

266266
return response.Resource;
267267
}
@@ -274,6 +274,13 @@ await _container.DeleteItemAsync<Appeal>(
274274
id,
275275
new PartitionKey(tenantId));
276276

277-
_logger.LogInformation("Deleted appeal {AppealId}", id);
277+
_logger.LogInformation("Deleted appeal {AppealId}", SanitizeForLog(id));
278+
}
279+
280+
private static string SanitizeForLog(string? value)
281+
{
282+
if (string.IsNullOrEmpty(value))
283+
return string.Empty;
284+
return value.Replace("\r", string.Empty).Replace("\n", string.Empty);
278285
}
279286
}

src/services/attachment-service/Controllers/AttachmentsController.cs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public async Task<ActionResult<Attachment>> CreateAttachment(
123123
created.Id,
124124
!string.IsNullOrWhiteSpace(created.ClaimId) ? "Claim" :
125125
!string.IsNullOrWhiteSpace(created.AuthorizationId) ? "Authorization" : "Appeal",
126-
created.ClaimId ?? created.AuthorizationId ?? created.AppealId);
126+
SanitizeForLog(created.ClaimId ?? created.AuthorizationId ?? created.AppealId));
127127

128128
return CreatedAtAction(nameof(GetAttachment), new { id = created.Id, tenantId = created.TenantId }, created);
129129
}
@@ -310,7 +310,7 @@ public async Task<ActionResult<AcknowledgmentResponse>> GenerateAcknowledgment(
310310
_logger.LogInformation(
311311
"Generated {AckType} acknowledgment for attachment {AttachmentId}",
312312
ackType,
313-
id);
313+
SanitizeForLog(id));
314314

315315
return Ok(new AcknowledgmentResponse
316316
{
@@ -325,7 +325,7 @@ public async Task<ActionResult<AcknowledgmentResponse>> GenerateAcknowledgment(
325325
}
326326
catch (Exception ex)
327327
{
328-
_logger.LogError(ex, "Error generating acknowledgment for attachment {AttachmentId}", id);
328+
_logger.LogError(ex, "Error generating acknowledgment for attachment {AttachmentId}", SanitizeForLog(id));
329329
return StatusCode(500, new { error = "Failed to generate acknowledgment", details = ex.Message });
330330
}
331331
}
@@ -344,6 +344,13 @@ private TradingPartner CreateDefaultTradingPartner(Attachment attachment)
344344
ApplicationReceiverId = attachment.ProviderId
345345
};
346346
}
347+
348+
private static string SanitizeForLog(string? value)
349+
{
350+
if (string.IsNullOrEmpty(value))
351+
return string.Empty;
352+
return value.Replace("\r", string.Empty).Replace("\n", string.Empty);
353+
}
347354
}
348355

349356
/// <summary>

0 commit comments

Comments
 (0)