Skip to content

Commit dc4a854

Browse files
Mpdreamzclaude
Mpdreamz
and
claude
authored
Harden path security across all file-reading directives (#3000)
* Harden path security across all file-reading directives Previously only `IncludeBlock` validated user-supplied paths. The other three directives (`SettingsBlock`, `CsvIncludeBlock`, `ChangelogBlock`) had no boundary or symlink checks, leaving them open to path traversal. All four now enforce the same four-layer protection: 1. `Path.IsPathRooted()` — reject absolute paths 2. `Path.GetFullPath()` — canonicalize, resolving any `..` segments 3. `IsSubPathOf()` — path must stay within the documentation source directory 4. `SymlinkValidator.ValidateFileAccess/DirectoryAccess()` — no symlinks, no hidden ancestor directories `SymlinkValidator` gains two new shared methods (`ValidateFileAccess` and `ValidateDirectoryAccess`) so the symlink + hidden-dir ancestor walk is no longer duplicated inline in `IncludeBlock`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address CodeRabbit review: symlink order and dangling symlinks - Move symlink validation before existence check in IncludeBlock so a dangling symlink doesn't fall through to emit confusing extra errors, matching the order used in SettingsBlock and CsvIncludeBlock. - Drop the `Exists: true` guard on all `LinkTarget != null` checks in SymlinkValidator so dangling symlinks (target missing) are rejected unconditionally, both on the file/directory itself and on ancestor dirs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ce8705a commit dc4a854

5 files changed

Lines changed: 156 additions & 43 deletions

File tree

‎src/Elastic.Documentation.Configuration/SymlinkValidator.cs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
using System.IO.Abstractions;
66
using System.Security;
7+
using Elastic.Documentation.Extensions;
78

89
namespace Elastic.Documentation.Configuration;
910

@@ -38,4 +39,62 @@ public static void EnsureNotSymlink(IFileSystem fileSystem, string filePath)
3839
if (fileInfo.Exists)
3940
EnsureNotSymlink(fileInfo);
4041
}
42+
43+
/// <summary>
44+
/// Validates that a file and its ancestor directories (up to <paramref name="docRoot"/>) are not symlinks
45+
/// and that no ancestor directory is hidden (starts with <c>.</c>).
46+
/// </summary>
47+
/// <returns>An error message if validation fails, or <c>null</c> if the file is safe to read.</returns>
48+
public static string? ValidateFileAccess(IFileInfo file, IDirectoryInfo docRoot)
49+
{
50+
if (file.LinkTarget != null)
51+
return "Path must not point to a symlink.";
52+
53+
var cmp = IDirectoryInfoExtensions.IsCaseSensitiveFileSystem
54+
? StringComparison.Ordinal
55+
: StringComparison.OrdinalIgnoreCase;
56+
57+
var dir = file.Directory;
58+
while (dir != null && !string.Equals(dir.FullName, docRoot.FullName, cmp))
59+
{
60+
if (dir.Name.StartsWith('.'))
61+
return "Path must not traverse hidden directories.";
62+
63+
if (dir.LinkTarget != null)
64+
return "Path must not traverse symlinked directories.";
65+
66+
dir = dir.Parent;
67+
}
68+
69+
return null;
70+
}
71+
72+
/// <summary>
73+
/// Validates that a directory and its ancestor directories (up to <paramref name="docRoot"/>) are not symlinks
74+
/// and that no ancestor directory is hidden (starts with <c>.</c>).
75+
/// </summary>
76+
/// <returns>An error message if validation fails, or <c>null</c> if the directory is safe to access.</returns>
77+
public static string? ValidateDirectoryAccess(IDirectoryInfo directory, IDirectoryInfo docRoot)
78+
{
79+
if (directory.LinkTarget != null)
80+
return "Path must not point to a symlinked directory.";
81+
82+
var cmp = IDirectoryInfoExtensions.IsCaseSensitiveFileSystem
83+
? StringComparison.Ordinal
84+
: StringComparison.OrdinalIgnoreCase;
85+
86+
var dir = directory.Parent;
87+
while (dir != null && !string.Equals(dir.FullName, docRoot.FullName, cmp))
88+
{
89+
if (dir.Name.StartsWith('.'))
90+
return "Path must not traverse hidden directories.";
91+
92+
if (dir.LinkTarget != null)
93+
return "Path must not traverse symlinked directories.";
94+
95+
dir = dir.Parent;
96+
}
97+
98+
return null;
99+
}
41100
}

‎src/Elastic.Markdown/Myst/Directives/Changelog/ChangelogBlock.cs‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the LICENSE file in the project root for more information
44

55
using Elastic.Documentation;
6+
using Elastic.Documentation.Configuration;
67
using Elastic.Documentation.Configuration.Assembler;
78
using Elastic.Documentation.Configuration.ReleaseNotes;
89
using Elastic.Documentation.Extensions;
@@ -214,9 +215,29 @@ private void ExtractBundlesFolderPath()
214215
if (string.IsNullOrWhiteSpace(folderPath))
215216
folderPath = DefaultBundlesFolder;
216217

217-
BundlesFolderPath = Build.DocumentationSourceDirectory.ResolvePathFrom(folderPath);
218+
var trimmedPath = folderPath.TrimStart('/');
219+
if (Path.IsPathRooted(trimmedPath))
220+
{
221+
this.EmitError("Changelog bundles path must not be an absolute path.");
222+
return;
223+
}
224+
225+
BundlesFolderPath = Path.GetFullPath(Build.DocumentationSourceDirectory.ResolvePathFrom(folderPath));
218226
BundlesFolderRelativeToSource = Path.GetRelativePath(Build.DocumentationSourceDirectory.FullName, BundlesFolderPath);
219227

228+
var dir = Build.ReadFileSystem.DirectoryInfo.New(BundlesFolderPath);
229+
if (!dir.IsSubPathOf(Build.DocumentationSourceDirectory))
230+
{
231+
this.EmitError("Changelog bundles path must resolve within the documentation source directory.");
232+
return;
233+
}
234+
235+
if (SymlinkValidator.ValidateDirectoryAccess(dir, Build.DocumentationSourceDirectory) is { } accessError)
236+
{
237+
this.EmitError(accessError);
238+
return;
239+
}
240+
220241
if (!Build.ReadFileSystem.Directory.Exists(BundlesFolderPath))
221242
{
222243
this.EmitError($"Changelog bundles folder `{BundlesFolderRelativeToSource}` does not exist.");
@@ -246,9 +267,28 @@ private void LoadConfiguration()
246267
if (string.IsNullOrWhiteSpace(ConfigPath))
247268
return;
248269

249-
var fileSystem = Build.ReadFileSystem;
250-
var explicitPath = Build.DocumentationSourceDirectory.ResolvePathFrom(ConfigPath);
251-
if (!fileSystem.File.Exists(explicitPath))
270+
var trimmedPath = ConfigPath.TrimStart('/');
271+
if (Path.IsPathRooted(trimmedPath))
272+
{
273+
this.EmitError("Changelog config path must not be an absolute path.");
274+
return;
275+
}
276+
277+
var explicitPath = Path.GetFullPath(Build.DocumentationSourceDirectory.ResolvePathFrom(ConfigPath));
278+
var file = Build.ReadFileSystem.FileInfo.New(explicitPath);
279+
if (!file.IsSubPathOf(Build.DocumentationSourceDirectory))
280+
{
281+
this.EmitError("Changelog config path must resolve within the documentation source directory.");
282+
return;
283+
}
284+
285+
if (SymlinkValidator.ValidateFileAccess(file, Build.DocumentationSourceDirectory) is { } accessError)
286+
{
287+
this.EmitError(accessError);
288+
return;
289+
}
290+
291+
if (!Build.ReadFileSystem.File.Exists(explicitPath))
252292
this.EmitWarning($"Specified changelog config path '{ConfigPath}' not found.");
253293
}
254294

‎src/Elastic.Markdown/Myst/Directives/CsvInclude/CsvIncludeBlock.cs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// See the LICENSE file in the project root for more information
44

55
using System.IO.Abstractions;
6+
using Elastic.Documentation.Configuration;
7+
using Elastic.Documentation.Extensions;
68
using Elastic.Markdown.Diagnostics;
79

810
namespace Elastic.Markdown.Myst.Directives.CsvInclude;
@@ -50,9 +52,29 @@ private void ExtractCsvPath(ParserContext context)
5052
if (csvPath.StartsWith('/'))
5153
csvFrom = Build.DocumentationSourceDirectory.FullName;
5254

53-
CsvFilePath = Path.Join(csvFrom, csvPath.TrimStart('/'));
55+
var trimmedPath = csvPath.TrimStart('/');
56+
if (Path.IsPathRooted(trimmedPath))
57+
{
58+
this.EmitError("CSV path must not be an absolute path.");
59+
return;
60+
}
61+
62+
CsvFilePath = Path.GetFullPath(Path.Join(csvFrom, trimmedPath));
5463
CsvFilePathRelativeToSource = Path.GetRelativePath(Build.DocumentationSourceDirectory.FullName, CsvFilePath);
5564

65+
var file = Build.ReadFileSystem.FileInfo.New(CsvFilePath);
66+
if (!file.IsSubPathOf(Build.DocumentationSourceDirectory))
67+
{
68+
this.EmitError("CSV path must resolve within the documentation source directory.");
69+
return;
70+
}
71+
72+
if (SymlinkValidator.ValidateFileAccess(file, Build.DocumentationSourceDirectory) is { } accessError)
73+
{
74+
this.EmitError(accessError);
75+
return;
76+
}
77+
5678
if (Build.ReadFileSystem.File.Exists(CsvFilePath))
5779
{
5880
ValidateFileSize();

‎src/Elastic.Markdown/Myst/Directives/Include/IncludeBlock.cs‎

Lines changed: 8 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the LICENSE file in the project root for more information
44

55
using System.IO.Abstractions;
6+
using Elastic.Documentation.Configuration;
67
using Elastic.Documentation.Extensions;
78
using Elastic.Markdown.Diagnostics;
89

@@ -76,13 +77,18 @@ private void ExtractInclusionPath(ParserContext context)
7677
return;
7778
}
7879

80+
if (SymlinkValidator.ValidateFileAccess(file, Build.DocumentationSourceDirectory) is { } accessError)
81+
{
82+
this.EmitError(accessError);
83+
Found = false;
84+
return;
85+
}
86+
7987
if (Build.ReadFileSystem.File.Exists(IncludePath))
8088
Found = true;
8189
else
8290
this.EmitError($"`{IncludePath}` does not exist.");
8391

84-
ValidateIncludePath(file);
85-
8692
if (Literal)
8793
return;
8894

@@ -98,39 +104,4 @@ private void ExtractInclusionPath(ParserContext context)
98104
Found = false;
99105
}
100106
}
101-
102-
private void ValidateIncludePath(IFileInfo file)
103-
{
104-
if (file is { Exists: true, LinkTarget: not null })
105-
{
106-
this.EmitError("Include path must not point to a symlink.");
107-
Found = false;
108-
return;
109-
}
110-
111-
var cmp = IDirectoryInfoExtensions.IsCaseSensitiveFileSystem
112-
? StringComparison.Ordinal
113-
: StringComparison.OrdinalIgnoreCase;
114-
115-
var docRootFullName = Build.DocumentationSourceDirectory.FullName;
116-
var dir = file.Directory;
117-
while (dir != null && !string.Equals(dir.FullName, docRootFullName, cmp))
118-
{
119-
if (dir.Name.StartsWith('.'))
120-
{
121-
this.EmitError("Include path must not traverse hidden directories.");
122-
Found = false;
123-
return;
124-
}
125-
126-
if (dir is { Exists: true, LinkTarget: not null })
127-
{
128-
this.EmitError("Include path must not traverse symlinked directories.");
129-
Found = false;
130-
return;
131-
}
132-
133-
dir = dir.Parent;
134-
}
135-
}
136107
}

‎src/Elastic.Markdown/Myst/Directives/Settings/SettingsBlock.cs‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
using System.IO.Abstractions;
66
using Elastic.Documentation.Configuration;
7+
using Elastic.Documentation.Extensions;
78
using Elastic.Markdown.Diagnostics;
89
using Elastic.Markdown.Helpers;
910
using Elastic.Markdown.Myst;
@@ -129,7 +130,27 @@ private void ExtractInclusionPath(ParserContext context)
129130
if (includePath.StartsWith('/'))
130131
includeFrom = Build.DocumentationSourceDirectory.FullName;
131132

132-
IncludePath = Path.Join(includeFrom, includePath.TrimStart('/'));
133+
var trimmedPath = includePath.TrimStart('/');
134+
if (Path.IsPathRooted(trimmedPath))
135+
{
136+
this.EmitError("Settings path must not be an absolute path.");
137+
return;
138+
}
139+
140+
IncludePath = Path.GetFullPath(Path.Join(includeFrom, trimmedPath));
141+
var file = Build.ReadFileSystem.FileInfo.New(IncludePath);
142+
if (!file.IsSubPathOf(Build.DocumentationSourceDirectory))
143+
{
144+
this.EmitError("Settings path must resolve within the documentation source directory.");
145+
return;
146+
}
147+
148+
if (SymlinkValidator.ValidateFileAccess(file, Build.DocumentationSourceDirectory) is { } accessError)
149+
{
150+
this.EmitError(accessError);
151+
return;
152+
}
153+
133154
if (Build.ReadFileSystem.File.Exists(IncludePath))
134155
Found = true;
135156
else

0 commit comments

Comments
 (0)