Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 54 additions & 54 deletions dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ internal sealed partial class AgentFileSkillsSource : AgentSkillsSource
private const string SkillFileName = "SKILL.md";
private const int MaxSearchDepth = 2;

// "." means the skill directory root itself (no sub-folder descent constraint)
private const string RootFolderIndicator = ".";
// "." means the skill directory root itself (no subdirectory descent constraint)
private const string RootDirectoryIndicator = ".";

private static readonly string[] s_defaultScriptExtensions = [".py", ".js", ".sh", ".ps1", ".cs", ".csx"];
private static readonly string[] s_defaultResourceExtensions = [".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt"];

// Standard sub-folder names per https://agentskills.io/specification#directory-structure
private static readonly string[] s_defaultScriptFolders = ["scripts"];
private static readonly string[] s_defaultResourceFolders = ["references", "assets"];
// Standard subdirectory names per https://agentskills.io/specification#directory-structure
private static readonly string[] s_defaultScriptDirectories = ["scripts"];
private static readonly string[] s_defaultResourceDirectories = ["references", "assets"];

// Matches YAML frontmatter delimited by "---" lines. Group 1 = content between delimiters.
// Multiline makes ^/$ match line boundaries; Singleline makes . match newlines across the block.
Expand All @@ -63,8 +63,8 @@ internal sealed partial class AgentFileSkillsSource : AgentSkillsSource
private readonly IEnumerable<string> _skillPaths;
private readonly HashSet<string> _allowedResourceExtensions;
private readonly HashSet<string> _allowedScriptExtensions;
private readonly IReadOnlyList<string> _scriptFolders;
private readonly IReadOnlyList<string> _resourceFolders;
private readonly IReadOnlyList<string> _scriptDirectories;
private readonly IReadOnlyList<string> _resourceDirectories;
private readonly AgentFileSkillScriptRunner? _scriptRunner;
private readonly ILogger _logger;

Expand Down Expand Up @@ -111,13 +111,13 @@ public AgentFileSkillsSource(
options?.AllowedScriptExtensions ?? s_defaultScriptExtensions,
StringComparer.OrdinalIgnoreCase);

this._scriptFolders = options?.ScriptFolders is not null
? [.. ValidateAndNormalizeFolderNames(options.ScriptFolders, this._logger)]
: s_defaultScriptFolders;
this._scriptDirectories = options?.ScriptDirectories is not null
? [.. ValidateAndNormalizeDirectoryNames(options.ScriptDirectories, this._logger)]
: s_defaultScriptDirectories;

this._resourceFolders = options?.ResourceFolders is not null
? [.. ValidateAndNormalizeFolderNames(options.ResourceFolders, this._logger)]
: s_defaultResourceFolders;
this._resourceDirectories = options?.ResourceDirectories is not null
? [.. ValidateAndNormalizeDirectoryNames(options.ResourceDirectories, this._logger)]
: s_defaultResourceDirectories;

this._scriptRunner = scriptRunner;
}
Expand Down Expand Up @@ -303,41 +303,41 @@ private bool TryParseFrontmatter(string content, string skillFilePath, [NotNullW
}

/// <summary>
/// Scans configured resource folders within a skill directory for resource files matching the configured extensions.
/// Scans configured resource directories within a skill directory for resource files matching the configured extensions.
/// </summary>
/// <remarks>
/// By default, scans <c>references/</c> and <c>assets/</c> sub-folders as specified by the
/// By default, scans <c>references/</c> and <c>assets/</c> subdirectories as specified by the
/// <see href="https://agentskills.io/specification">Agent Skills specification</see>.
/// Configure <see cref="AgentFileSkillsSourceOptions.ResourceFolders"/> to scan different or
/// Configure <see cref="AgentFileSkillsSourceOptions.ResourceDirectories"/> to scan different or
/// additional directories, including <c>"."</c> for the skill root itself.
/// Each file is validated against path-traversal and symlink-escape checks; unsafe files are skipped.
/// </remarks>
private List<AgentFileSkillResource> DiscoverResourceFiles(string skillDirectoryFullPath, string skillName)
{
var resources = new List<AgentFileSkillResource>();

foreach (string folder in this._resourceFolders.Distinct(StringComparer.OrdinalIgnoreCase))
foreach (string directory in this._resourceDirectories.Distinct(StringComparer.OrdinalIgnoreCase))
{
bool isRootFolder = string.Equals(folder, RootFolderIndicator, StringComparison.Ordinal);
bool isRootDirectory = string.Equals(directory, RootDirectoryIndicator, StringComparison.Ordinal);

// GetFullPath normalizes mixed separators (e.g. "C:\skill\scripts/f1" → "C:\skill\scripts\f1")
string targetDirectory = isRootFolder
string targetDirectory = isRootDirectory
? skillDirectoryFullPath
: Path.GetFullPath(Path.Combine(skillDirectoryFullPath, folder)) + Path.DirectorySeparatorChar;
: Path.GetFullPath(Path.Combine(skillDirectoryFullPath, directory)) + Path.DirectorySeparatorChar;

if (!Directory.Exists(targetDirectory))
{
continue;
}

// Directory-level symlink check: skip if targetDirectory (or any intermediate
// segment) is a reparse point. The root folder is excluded — it's a caller-supplied
// segment) is a reparse point. The root directory is excluded — it's a caller-supplied
// trusted path, and the security boundary guards files within it, not the path itself.
if (!isRootFolder && HasSymlinkInPath(targetDirectory, skillDirectoryFullPath))
if (!isRootDirectory && HasSymlinkInPath(targetDirectory, skillDirectoryFullPath))
{
if (this._logger.IsEnabled(LogLevel.Warning))
{
LogResourceSymlinkFolder(this._logger, skillName, SanitizePathForLog(folder));
LogResourceSymlinkDirectory(this._logger, skillName, SanitizePathForLog(directory));
}

continue;
Expand Down Expand Up @@ -380,7 +380,7 @@ private List<AgentFileSkillResource> DiscoverResourceFiles(string skillDirectory
// e.g. "references/../../../etc/shadow" → "/etc/shadow"
string resolvedFilePath = Path.GetFullPath(filePath);

// Path containment: reject if the resolved path escapes the target folder.
// Path containment: reject if the resolved path escapes the target directory.
// e.g. "/etc/shadow".StartsWith("/skills/myskill/references/") → false → skip
if (!resolvedFilePath.StartsWith(targetDirectory, StringComparison.OrdinalIgnoreCase))
{
Expand Down Expand Up @@ -416,41 +416,41 @@ private List<AgentFileSkillResource> DiscoverResourceFiles(string skillDirectory
}

/// <summary>
/// Scans configured script folders within a skill directory for script files matching the configured extensions.
/// Scans configured script directories within a skill directory for script files matching the configured extensions.
/// </summary>
/// <remarks>
/// By default, scans the <c>scripts/</c> sub-folder as specified by the
/// By default, scans the <c>scripts/</c> subdirectory as specified by the
/// <see href="https://agentskills.io/specification">Agent Skills specification</see>.
/// Configure <see cref="AgentFileSkillsSourceOptions.ScriptFolders"/> to scan different or
/// Configure <see cref="AgentFileSkillsSourceOptions.ScriptDirectories"/> to scan different or
/// additional directories, including <c>"."</c> for the skill root itself.
/// Each file is validated against path-traversal and symlink-escape checks; unsafe files are skipped.
/// </remarks>
private List<AgentFileSkillScript> DiscoverScriptFiles(string skillDirectoryFullPath, string skillName)
{
var scripts = new List<AgentFileSkillScript>();

foreach (string folder in this._scriptFolders.Distinct(StringComparer.OrdinalIgnoreCase))
foreach (string directory in this._scriptDirectories.Distinct(StringComparer.OrdinalIgnoreCase))
{
bool isRootFolder = string.Equals(folder, RootFolderIndicator, StringComparison.Ordinal);
bool isRootDirectory = string.Equals(directory, RootDirectoryIndicator, StringComparison.Ordinal);

// GetFullPath normalizes mixed separators (e.g. "C:\skill\scripts/f1" → "C:\skill\scripts\f1")
string targetDirectory = isRootFolder
string targetDirectory = isRootDirectory
? skillDirectoryFullPath
: Path.GetFullPath(Path.Combine(skillDirectoryFullPath, folder)) + Path.DirectorySeparatorChar;
: Path.GetFullPath(Path.Combine(skillDirectoryFullPath, directory)) + Path.DirectorySeparatorChar;

if (!Directory.Exists(targetDirectory))
{
continue;
}

// Directory-level symlink check: skip if targetDirectory (or any intermediate
// segment) is a reparse point. The root folder is excluded — it's a caller-supplied
// segment) is a reparse point. The root directory is excluded — it's a caller-supplied
// trusted path, and the security boundary guards files within it, not the path itself.
if (!isRootFolder && HasSymlinkInPath(targetDirectory, skillDirectoryFullPath))
if (!isRootDirectory && HasSymlinkInPath(targetDirectory, skillDirectoryFullPath))
{
if (this._logger.IsEnabled(LogLevel.Warning))
{
LogScriptSymlinkFolder(this._logger, skillName, SanitizePathForLog(folder));
LogScriptSymlinkDirectory(this._logger, skillName, SanitizePathForLog(directory));
}

continue;
Expand Down Expand Up @@ -480,7 +480,7 @@ private List<AgentFileSkillScript> DiscoverScriptFiles(string skillDirectoryFull
// e.g. "scripts/../../../etc/shadow" → "/etc/shadow"
string resolvedFilePath = Path.GetFullPath(filePath);

// Path containment: reject if the resolved path escapes the target folder.
// Path containment: reject if the resolved path escapes the target directory.
// e.g. "/etc/shadow".StartsWith("/skills/myskill/scripts/") → false → skip
if (!resolvedFilePath.StartsWith(targetDirectory, StringComparison.OrdinalIgnoreCase))
{
Expand Down Expand Up @@ -541,8 +541,8 @@ private static bool HasSymlinkInPath(string pathToCheck, string trustedBasePath)
}

/// <summary>
/// Normalizes a relative path or folder name by stripping a leading "./"/".\",
/// trimming trailing directory separators, and replacing backslashes with forward
/// Normalizes a relative path or directory name by stripping a leading "./"/".\",
/// trimming trailing separators, and replacing backslashes with forward
/// slashes.
/// </summary>
private static string NormalizePath(string path)
Expand Down Expand Up @@ -602,36 +602,36 @@ private static void ValidateExtensions(IEnumerable<string>? extensions)
}
}

private static IEnumerable<string> ValidateAndNormalizeFolderNames(IEnumerable<string> folders, ILogger logger)
private static IEnumerable<string> ValidateAndNormalizeDirectoryNames(IEnumerable<string> directories, ILogger logger)
{
foreach (string folder in folders)
foreach (string directory in directories)
{
if (string.IsNullOrWhiteSpace(folder))
if (string.IsNullOrWhiteSpace(directory))
{
throw new ArgumentException("Folder names must not be null or whitespace.", nameof(folders));
throw new ArgumentException("Directory names must not be null or whitespace.", nameof(directories));
}

// "." is valid — it means the skill root directory.
if (string.Equals(folder, RootFolderIndicator, StringComparison.Ordinal))
if (string.Equals(directory, RootDirectoryIndicator, StringComparison.Ordinal))
{
yield return folder;
yield return directory;
continue;
}

// Reject absolute paths and any path segments that escape upward.
if (Path.IsPathRooted(folder) || ContainsParentTraversalSegment(folder))
if (Path.IsPathRooted(directory) || ContainsParentTraversalSegment(directory))
{
LogFolderNameSkippedInvalid(logger, folder);
LogDirectoryNameSkippedInvalid(logger, directory);
continue;
}

yield return NormalizePath(folder);
yield return NormalizePath(directory);
}
}

private static bool ContainsParentTraversalSegment(string folder)
private static bool ContainsParentTraversalSegment(string directory)
{
foreach (string segment in folder.Split('/', '\\'))
foreach (string segment in directory.Split('/', '\\'))
{
if (segment == "..")
{
Expand Down Expand Up @@ -666,8 +666,8 @@ private static bool ContainsParentTraversalSegment(string folder)
[LoggerMessage(LogLevel.Warning, "Skipping resource in skill '{SkillName}': '{ResourcePath}' is a symlink that resolves outside the skill directory")]
private static partial void LogResourceSymlinkEscape(ILogger logger, string skillName, string resourcePath);

[LoggerMessage(LogLevel.Warning, "Skipping resource folder '{FolderName}' in skill '{SkillName}': folder path contains a symlink")]
private static partial void LogResourceSymlinkFolder(ILogger logger, string skillName, string folderName);
[LoggerMessage(LogLevel.Warning, "Skipping resource directory '{DirectoryName}' in skill '{SkillName}': directory path contains a symlink")]
private static partial void LogResourceSymlinkDirectory(ILogger logger, string skillName, string directoryName);

[LoggerMessage(LogLevel.Debug, "Skipping file '{FilePath}' in skill '{SkillName}': extension '{Extension}' is not in the allowed list")]
private static partial void LogResourceSkippedExtension(ILogger logger, string skillName, string filePath, string extension);
Expand All @@ -678,9 +678,9 @@ private static bool ContainsParentTraversalSegment(string folder)
[LoggerMessage(LogLevel.Warning, "Skipping script in skill '{SkillName}': '{ScriptPath}' is a symlink that resolves outside the skill directory")]
private static partial void LogScriptSymlinkEscape(ILogger logger, string skillName, string scriptPath);

[LoggerMessage(LogLevel.Warning, "Skipping script folder '{FolderName}' in skill '{SkillName}': folder path contains a symlink")]
private static partial void LogScriptSymlinkFolder(ILogger logger, string skillName, string folderName);
[LoggerMessage(LogLevel.Warning, "Skipping script directory '{DirectoryName}' in skill '{SkillName}': directory path contains a symlink")]
private static partial void LogScriptSymlinkDirectory(ILogger logger, string skillName, string directoryName);

[LoggerMessage(LogLevel.Warning, "Skipping invalid folder name '{FolderName}': must be a relative path with no '..' segments")]
private static partial void LogFolderNameSkippedInvalid(ILogger logger, string folderName);
[LoggerMessage(LogLevel.Warning, "Skipping invalid directory name '{DirectoryName}': must be a relative path with no '..' segments")]
private static partial void LogDirectoryNameSkippedInvalid(ILogger logger, string directoryName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public sealed class AgentFileSkillsSourceOptions
public IEnumerable<string>? AllowedScriptExtensions { get; set; }

/// <summary>
/// Gets or sets relative folder paths to scan for script files within each skill directory.
/// Gets or sets relative directory paths to scan for script files within each skill directory.
/// Values may be single-segment names (e.g., <c>"scripts"</c>) or multi-segment relative
/// paths (e.g., <c>"sub/scripts"</c>). Use <c>"."</c> to include files directly at the
/// skill root. Leading <c>"./"</c> prefixes, trailing separators, and backslashes are
Expand All @@ -42,10 +42,10 @@ public sealed class AgentFileSkillsSourceOptions
/// <see href="https://agentskills.io/specification">Agent Skills specification</see>).
/// When set, replaces the defaults entirely.
/// </summary>
public IEnumerable<string>? ScriptFolders { get; set; }
public IEnumerable<string>? ScriptDirectories { get; set; }

/// <summary>
/// Gets or sets relative folder paths to scan for resource files within each skill directory.
/// Gets or sets relative directory paths to scan for resource files within each skill directory.
/// Values may be single-segment names (e.g., <c>"references"</c>) or multi-segment relative
/// paths (e.g., <c>"sub/resources"</c>). Use <c>"."</c> to include files directly at the
/// skill root. Leading <c>"./"</c> prefixes, trailing separators, and backslashes are
Expand All @@ -55,5 +55,5 @@ public sealed class AgentFileSkillsSourceOptions
/// <see href="https://agentskills.io/specification">Agent Skills specification</see>).
/// When set, replaces the defaults entirely.
/// </summary>
public IEnumerable<string>? ResourceFolders { get; set; }
public IEnumerable<string>? ResourceDirectories { get; set; }
}
Loading
Loading