Skip to content

Commit 5c388db

Browse files
committed
Resolve #5091 final adversarial findings
1 parent 2f9af72 commit 5c388db

7 files changed

Lines changed: 414 additions & 27 deletions

changelog.d/unreleased/5091.fixed.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,16 @@ affected:
3232
- src/CodeIndex/Indexer/Scanning/FileIndexer.Types.cs
3333
- src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs
3434
- src/CodeIndex/Indexer/Symbols/SymbolExtractor.TypeScriptPathAliases.cs
35+
- src/CodeIndex/Mcp/McpToolHandlers.Indexing.Execution.cs
36+
- src/CodeIndex/Mcp/McpToolHandlers.Indexing.ExtractedFilePersistence.cs
37+
- src/CodeIndex/Mcp/McpToolHandlers.Indexing.FileLoop.cs
3538
- tests/CodeIndex.Tests/ConsoleUiTests.cs
3639
- tests/CodeIndex.Tests/FileIndexerTests.cs
3740
- tests/CodeIndex.Tests/IndexCommandRunnerDryRunTests.cs
3841
- tests/CodeIndex.Tests/IndexCommandRunnerTests.cs
3942
- tests/CodeIndex.Tests/IndexCommandRunnerUpdateTests.cs
4043
- tests/CodeIndex.Tests/IndexWatchRunnerTests.cs
44+
- tests/CodeIndex.Tests/McpServerToolsCallTests.cs
4145
- tests/CodeIndex.Tests/PathCasingTests.cs
4246
---
4347

src/CodeIndex/Cli/IndexCommandRunner.ExplicitFilesPreflight.cs

Lines changed: 162 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ internal static ExplicitFilesIndexedPathSnapshot ReadFailure()
2929
=> CreateExplicitFilesIndexedPathSnapshot([], readFailed: true);
3030
}
3131

32+
internal readonly record struct ExplicitFilesIndexedPathLookupMetrics(
33+
int ExactQueryCount,
34+
int CaseFoldScanCount,
35+
long CaseFoldScannedRowCount);
36+
37+
internal sealed record ExplicitFilesIndexedPathLookupResult(
38+
IReadOnlySet<string> Paths,
39+
ExplicitFilesIndexedPathLookupMetrics Metrics);
40+
3241
private static int? RunExplicitFilesPreflight(
3342
IndexCommandOptions options,
3443
string resolvedDbPath,
@@ -154,7 +163,9 @@ void Reject(int inputIndex, string path, string reason)
154163
continue;
155164
}
156165

157-
if (!FileIndexer.IsFilePathSyntaxIndexable(absolutePath))
166+
if (!IsExplicitFilePathComponentSyntaxValid(
167+
relativePath,
168+
useWindowsRules: OperatingSystem.IsWindows()))
158169
{
159170
Reject(
160171
inputIndex,
@@ -523,29 +534,11 @@ private static ExplicitFilesIndexedPathSnapshot ReadExplicitFilesIndexedPathSnap
523534
if (!DryRunTableExists(connection, "files"))
524535
return ExplicitFilesIndexedPathSnapshot.Empty();
525536

526-
var paths = new HashSet<string>(StringComparer.Ordinal);
527-
ReadExplicitFilesIndexedPathMatches(
537+
var lookup = ReadExplicitFilesIndexedPathMatches(
528538
connection,
529539
candidates,
530-
paths,
531-
pathExpression: "path",
532-
cancellationToken: cancellationToken);
533-
var unmatched = candidates.Where(path => !paths.Contains(path)).ToArray();
534-
if (unmatched.Length > 0)
535-
{
536-
const string collationName = "cdidx_explicit_files_path";
537-
connection.CreateCollation(
538-
collationName,
539-
(left, right) => StringComparer.OrdinalIgnoreCase.Compare(left, right));
540-
ReadExplicitFilesIndexedPathMatches(
541-
connection,
542-
unmatched,
543-
paths,
544-
pathExpression: $"path COLLATE {collationName}",
545-
cancellationToken: cancellationToken);
546-
}
547-
548-
return CreateExplicitFilesIndexedPathSnapshot(paths, readFailed: false);
540+
cancellationToken);
541+
return CreateExplicitFilesIndexedPathSnapshot(lookup.Paths, readFailed: false);
549542
}
550543
catch (SqliteException)
551544
{
@@ -607,13 +600,20 @@ private static IReadOnlyCollection<string> CollectExplicitFilesIndexedPathCandid
607600
return candidates;
608601
}
609602

610-
private static void ReadExplicitFilesIndexedPathMatches(
603+
internal static ExplicitFilesIndexedPathLookupResult
604+
ReadExplicitFilesIndexedPathMatchesForTesting(
605+
SqliteConnection connection,
606+
IReadOnlyCollection<string> candidates,
607+
CancellationToken cancellationToken = default)
608+
=> ReadExplicitFilesIndexedPathMatches(connection, candidates, cancellationToken);
609+
610+
private static ExplicitFilesIndexedPathLookupResult ReadExplicitFilesIndexedPathMatches(
611611
SqliteConnection connection,
612-
IEnumerable<string> candidates,
613-
HashSet<string> matches,
614-
string pathExpression,
612+
IReadOnlyCollection<string> candidates,
615613
CancellationToken cancellationToken)
616614
{
615+
var matches = new HashSet<string>(StringComparer.Ordinal);
616+
var exactQueryCount = 0;
617617
foreach (var batch in candidates.Chunk(ExplicitFilesIndexedPathQueryBatchSize))
618618
{
619619
cancellationToken.ThrowIfCancellationRequested();
@@ -626,7 +626,8 @@ private static void ReadExplicitFilesIndexedPathMatches(
626626
SqliteCommandPolicy.Add(command, parameterName, batch[index]);
627627
}
628628

629-
command.CommandText = $"SELECT path FROM files WHERE {pathExpression} IN ({string.Join(", ", parameterNames)})";
629+
command.CommandText = $"SELECT path FROM files WHERE path IN ({string.Join(", ", parameterNames)})";
630+
exactQueryCount++;
630631
using var reader = command.ExecuteReader();
631632
while (reader.Read())
632633
{
@@ -635,6 +636,140 @@ private static void ReadExplicitFilesIndexedPathMatches(
635636
matches.Add(FileIndexer.NormalizeIndexPath(reader.GetString(0)));
636637
}
637638
}
639+
640+
var unmatched = candidates
641+
.Where(path => !matches.Contains(path))
642+
.ToHashSet(StringComparer.OrdinalIgnoreCase);
643+
var caseFoldScanCount = 0;
644+
long caseFoldScannedRowCount = 0;
645+
if (unmatched.Count > 0)
646+
{
647+
// A custom collation on files.path prevents SQLite from using the ordinary
648+
// binary path index. Batching that expression therefore rescanned the entire
649+
// files table once per 256 candidates. Scan it at most once, retain only the
650+
// case-fold candidates, and let TryGetExplicitFileIndexedPath apply the target
651+
// directory namespace policy before treating a spelling as indexed.
652+
using var command = connection.CreateCommand();
653+
command.CommandText = "SELECT path FROM files";
654+
caseFoldScanCount = 1;
655+
using var reader = command.ExecuteReader();
656+
while (reader.Read())
657+
{
658+
cancellationToken.ThrowIfCancellationRequested();
659+
caseFoldScannedRowCount++;
660+
if (reader.IsDBNull(0))
661+
continue;
662+
663+
var path = FileIndexer.NormalizeIndexPath(reader.GetString(0));
664+
if (unmatched.Contains(path))
665+
matches.Add(path);
666+
}
667+
}
668+
669+
return new ExplicitFilesIndexedPathLookupResult(
670+
matches,
671+
new ExplicitFilesIndexedPathLookupMetrics(
672+
exactQueryCount,
673+
caseFoldScanCount,
674+
caseFoldScannedRowCount));
675+
}
676+
677+
internal static bool IsExplicitFilePathComponentSyntaxValid(
678+
string path,
679+
bool useWindowsRules)
680+
{
681+
if (!FileIndexer.IsFilePathSyntaxIndexable(path))
682+
return false;
683+
if (!useWindowsRules)
684+
return true;
685+
686+
if (FileIndexer.IsWindowsDevicePath(path)
687+
|| IsExplicitFileWindowsSuperscriptDevicePath(path))
688+
return false;
689+
690+
// Path.GetFullPath no longer rejects wildcard and other invalid filename
691+
// characters on modern .NET. Check Windows filename components explicitly so a
692+
// bad direct --files token cannot reach the mutation pipeline as an inconclusive
693+
// filesystem probe. A drive designator is valid only as the first component; the
694+
// production caller normally supplies a project-relative spelling, but accepting
695+
// the rooted form here keeps the validation seam faithful to Win32 syntax.
696+
var span = path.AsSpan();
697+
var componentIndex = 0;
698+
for (var start = 0; start < span.Length;)
699+
{
700+
while (start < span.Length && span[start] is '/' or '\\')
701+
start++;
702+
if (start >= span.Length)
703+
break;
704+
705+
var end = start;
706+
while (end < span.Length && span[end] is not ('/' or '\\'))
707+
end++;
708+
709+
var component = span[start..end];
710+
var isDriveDesignator = componentIndex == 0
711+
&& component.Length == 2
712+
&& ((component[0] >= 'A' && component[0] <= 'Z')
713+
|| (component[0] >= 'a' && component[0] <= 'z'))
714+
&& component[1] == ':';
715+
var hasInvalidCharacter = false;
716+
if (!isDriveDesignator)
717+
{
718+
foreach (var character in component)
719+
{
720+
if (character is '<' or '>' or ':' or '"' or '|' or '?' or '*')
721+
{
722+
hasInvalidCharacter = true;
723+
break;
724+
}
725+
}
726+
}
727+
if (hasInvalidCharacter)
728+
return false;
729+
730+
if (!component.SequenceEqual(".".AsSpan())
731+
&& !component.SequenceEqual("..".AsSpan())
732+
&& component[^1] is ' ' or '.')
733+
{
734+
return false;
735+
}
736+
737+
componentIndex++;
738+
start = end + 1;
739+
}
740+
741+
return true;
742+
}
743+
744+
private static bool IsExplicitFileWindowsSuperscriptDevicePath(string path)
745+
{
746+
var span = path.AsSpan();
747+
for (var start = 0; start < span.Length;)
748+
{
749+
while (start < span.Length && span[start] is '/' or '\\')
750+
start++;
751+
if (start >= span.Length)
752+
break;
753+
754+
var end = start;
755+
while (end < span.Length && span[end] is not ('/' or '\\'))
756+
end++;
757+
758+
var component = span[start..end];
759+
var extensionIndex = component.IndexOf('.');
760+
var name = extensionIndex >= 0 ? component[..extensionIndex] : component;
761+
if (name.Length == 4
762+
&& (name.StartsWith("COM".AsSpan(), StringComparison.OrdinalIgnoreCase)
763+
|| name.StartsWith("LPT".AsSpan(), StringComparison.OrdinalIgnoreCase))
764+
&& name[3] is '\u00b9' or '\u00b2' or '\u00b3')
765+
{
766+
return true;
767+
}
768+
769+
start = end + 1;
770+
}
771+
772+
return false;
638773
}
639774

640775
private static int WriteExplicitFilesPreflightError(

src/CodeIndex/Mcp/McpToolHandlers.Indexing.Execution.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,7 @@ bool CanReuseCSharpPrepassTargetWithoutRead(CSharpStaticInterfacePrepass.FileTar
826826
CancellationToken = requestToken,
827827
MaxSymbolsPerFile = maxSymbolsPerFile,
828828
MaxReferencesPerFile = maxReferencesPerFile,
829+
SymlinkPolicy = symlinkPolicy,
829830
Rebuild = rebuild,
830831
StartedWithNoIndexedFiles = startedWithNoIndexedFiles,
831832
UseFullRunBatchMarker = useFullRunBatchMarker,

src/CodeIndex/Mcp/McpToolHandlers.Indexing.ExtractedFilePersistence.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ private static (List<SymbolRecord> Symbols, FileIssue? TimeoutIssue)
8686
using var regexTimeouts = BoundedRegex.CaptureTimeouts(
8787
record.Lang,
8888
"symbol_extraction");
89+
using var typeScriptPathAliasFileSystemPolicy =
90+
SymbolExtractor.EnterTypeScriptPathAliasFileSystemPolicy(
91+
context.SymlinkPolicy,
92+
context.ProjectPath);
8993
var symbols = SymbolExtractor.ExtractNormalized(
9094
fileId,
9195
record.Lang,

src/CodeIndex/Mcp/McpToolHandlers.Indexing.FileLoop.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ private sealed class McpIndexFileLoopContext
2424
internal required CancellationToken CancellationToken { get; init; }
2525
internal required int MaxSymbolsPerFile { get; init; }
2626
internal required int MaxReferencesPerFile { get; init; }
27+
internal required FileIndexer.SymlinkPolicy SymlinkPolicy { get; init; }
2728
internal required bool Rebuild { get; init; }
2829
internal required bool StartedWithNoIndexedFiles { get; init; }
2930
internal required bool UseFullRunBatchMarker { get; init; }

0 commit comments

Comments
 (0)