Skip to content

Commit c7fa70d

Browse files
resolved copilot comments
1 parent c7d1d9f commit c7fa70d

7 files changed

Lines changed: 72 additions & 10 deletions

File tree

App/backend-api/Microsoft.GS.DPS.Host/API/KernelMemory/KernelMemory.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,12 @@ DPS.API.KernelMemory kernelMemory
9494
return Results.Ok(new DocumentDeletedResult() { IsDeleted = true });
9595
}
9696
#pragma warning disable CA1031 // Must catch all to log and keep the process alive
97-
catch (Exception)
97+
catch (Exception ex)
9898
{
99+
app.Logger.LogError(ex, "An error occurred while deleting document {DocumentId}.", documentId);
99100
return Results.BadRequest(new DocumentDeletedResult() { IsDeleted = false });
100101
}
102+
#pragma warning restore CA1031
101103
})
102104
.DisableAntiforgery();
103105

App/frontend-app/src/components/documentViewer/documentViewer.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import { PagesTab } from "./PagesTab";
1717
import { PageNumberTab } from "./pageNumberTab";
1818
import { DialogTitleBar } from "./dialogTitleBar";
1919
import { AIKnowledgeTab } from "./aIKnowledgeTab";
20-
import { useTranslation } from "react-i18next";
2120

2221
const useStyles = makeStyles({
2322
dialog: {

App/frontend-app/src/components/filter/filter.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import React, { useEffect, useState, memo, useRef } from "react";
22
import { Checkbox } from "@fluentui/react-checkbox";
3-
import { useTranslation } from "react-i18next";
43
import { Accordion, AccordionHeader, AccordionItem, AccordionPanel, makeStyles } from "@fluentui/react-components";
54

65
const useStyles = makeStyles({

App/frontend-app/src/components/headerBar/headerBar.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ import React, { MouseEventHandler, useMemo, useState } from "react";
22
import { useTranslation } from "react-i18next";
33
import { Link, useNavigate } from "react-router-dom";
44
import { useMsal } from "@azure/msal-react";
5-
import { Auth } from "../../utils/auth/auth";
6-
import { RedirectRequest } from "@azure/msal-browser";
75
import {
86
Avatar,
97
makeStyles,
@@ -16,7 +14,6 @@ import {
1614
} from "@fluentui/react-components";
1715
import resolveConfig from "tailwindcss/resolveConfig";
1816
import TailwindConfig from "../../../tailwind.config";
19-
import { isPlatformAdmin } from "../../utils/auth/roles";
2017

2118
const fullConfig = resolveConfig(TailwindConfig);
2219
const useStylesAvatar = makeStyles({

App/kernel-memory/extensions/SQLServer/SQLServer/SqlServerMemory.cs

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -490,9 +490,13 @@ public void Dispose()
490490

491491
#region private ================================================================================
492492

493-
// Note: "_" is allowed in SQL Server, but we normalize it to "-" for consistency with other DBs
494-
private static readonly Regex s_replaceIndexNameCharsRegex = new(@"[\s|\\|/|.|_|:]");
495-
private const string ValidSeparator = "-";
493+
// Note: "_" is allowed in SQL Server and used as the safe separator
494+
private static readonly Regex s_replaceIndexNameCharsRegex = new(@"[\s|\\|/|.|:]");
495+
private const string ValidSeparator = "_";
496+
497+
// Validation regex to ensure normalized index names contain only safe SQL identifier characters
498+
private static readonly Regex s_validIndexNameRegex = new(@"^[a-z0-9_]+$");
499+
private const int MaxIndexNameLength = 128;
496500

497501
/// <summary>
498502
/// Prepare instance, ensuring tables exist and reusable info is cached.
@@ -586,6 +590,7 @@ private async Task<bool> DoesIndexExistsAsync(string indexName,
586590
/// <returns></returns>
587591
private string GetFullTableName(string tableName)
588592
{
593+
ValidateTableName(tableName);
589594
return $"[{this._config.Schema}].[{tableName}]";
590595
}
591596

@@ -601,6 +606,9 @@ private string GenerateFilters(
601606
SqlParameterCollection parameters,
602607
ICollection<MemoryFilter>? filters = null)
603608
{
609+
// Validate index before using it in SQL construction (defense in depth)
610+
ValidateIndexName(index);
611+
604612
var filterBuilder = new StringBuilder();
605613

606614
if (filters is null || filters.Count <= 0 || filters.All(f => f.Count <= 0))
@@ -684,8 +692,61 @@ private static string NormalizeIndexName(string index)
684692

685693
index = s_replaceIndexNameCharsRegex.Replace(index.Trim().ToLowerInvariant(), ValidSeparator);
686694

695+
// Validate the normalized index name
696+
ValidateIndexName(index);
697+
687698
return index;
688699
}
689700

701+
/// <summary>
702+
/// Validates that an index name contains only safe SQL identifier characters.
703+
/// This prevents SQL injection when index names are used in dynamic SQL.
704+
/// </summary>
705+
/// <param name="index">The index name to validate.</param>
706+
/// <exception cref="ConfigurationException">Thrown if the index name is invalid.</exception>
707+
private static void ValidateIndexName(string index)
708+
{
709+
if (string.IsNullOrEmpty(index))
710+
{
711+
throw new ConfigurationException("The index name is empty after normalization");
712+
}
713+
714+
if (index.Length > MaxIndexNameLength)
715+
{
716+
throw new ConfigurationException($"The index name '{index}' exceeds the maximum allowed length of {MaxIndexNameLength} characters");
717+
}
718+
719+
if (!s_validIndexNameRegex.IsMatch(index))
720+
{
721+
throw new ConfigurationException($"The index name '{index}' contains invalid characters. Only lowercase letters, numbers, and underscores are allowed");
722+
}
723+
}
724+
725+
/// <summary>
726+
/// Validates that a table name contains only safe SQL identifier characters.
727+
/// This prevents SQL injection when table names are used in dynamic SQL.
728+
/// </summary>
729+
/// <param name="tableName">The table name to validate.</param>
730+
/// <exception cref="ConfigurationException">Thrown if the table name is invalid.</exception>
731+
private static void ValidateTableName(string tableName)
732+
{
733+
if (string.IsNullOrWhiteSpace(tableName))
734+
{
735+
throw new ConfigurationException("The table name is empty");
736+
}
737+
738+
if (tableName.Length > MaxIndexNameLength)
739+
{
740+
throw new ConfigurationException($"The table name '{tableName}' exceeds the maximum allowed length of {MaxIndexNameLength} characters");
741+
}
742+
743+
// Table names can contain letters, digits, underscores, and hyphens
744+
// This covers both base table names and index-suffixed table names like "TableName_indexname"
745+
if (!s_validIndexNameRegex.IsMatch(tableName))
746+
{
747+
throw new ConfigurationException($"The table name '{tableName}' contains invalid characters. Only lowercase letters, numbers, and underscores are allowed");
748+
}
749+
}
750+
690751
#endregion
691752
}

App/kernel-memory/service/Abstractions/Diagnostics/ArgumentOutOfRangeExceptionEx.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public static void ThrowIfEqualToOrGreaterThan(float value, float other, string
195195

196196
public static void ThrowIfZero(double value, string paramName, string message)
197197
{
198-
if (Math.Abs(value) > 0) { return; }
198+
if (value != 0d) { return; }
199199

200200
throw new ArgumentOutOfRangeException(paramName, message);
201201
}

App/kernel-memory/service/Core/Pipeline/Queue/DevTools/SimpleQueues.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,10 @@ private void PopulateQueue(object? sender, ElapsedEventArgs elapsedEventArgs)
235235
{
236236
this._log.LogError(e, "Unexpected error while polling the queue");
237237
}
238+
catch (Exception e)
239+
{
240+
this._log.LogError(e, "Unexpected error while populating queue {QueueName}", this._queueName);
241+
}
238242
finally
239243
{
240244
this._busy = false;

0 commit comments

Comments
 (0)