Skip to content
Open
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
changes:
- section: "Bugs Fixed"
description: "Fixed directory traversal vulnerability in OneLake file operations. Paths containing .. sequences are now rejected before any HTTP request is made."
46 changes: 45 additions & 1 deletion tools/Fabric.Mcp.Tools.OneLake/src/Services/OneLakeService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ private async Task<Workspace> GetWorkspaceAsync(string workspaceId, Cancellation
// Data Operations (OneLake Data Plane)
public async Task<OneLakeFileInfo> GetFileInfoAsync(string workspaceId, string itemId, string filePath, CancellationToken cancellationToken = default)
{
ValidatePathForTraversal(filePath, nameof(filePath));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
var url = $"{OneLakeEndpoints.OneLakeDataPlaneBaseUrl}/{normalizedWorkspaceId}/{normalizedItemId}/Files/{filePath.TrimStart('/')}";
var response = await SendDataPlaneRequestAsync(HttpMethod.Head, url, cancellationToken: cancellationToken);
Expand All @@ -221,6 +222,8 @@ public async Task<OneLakeFileInfo> GetFileInfoAsync(string workspaceId, string i

public async Task<IEnumerable<OneLakeFileInfo>> ListBlobsAsync(string workspaceId, string itemId, string? path = null, bool recursive = false, CancellationToken cancellationToken = default)
{
if (path is not null)
ValidatePathForTraversal(path, nameof(path));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);

// If no path is specified, intelligently discover and search top-level folders
Expand Down Expand Up @@ -707,6 +710,8 @@ private List<FileSystemItem> ParsePathListResponse(string jsonContent)

public async Task<List<FileSystemItem>> ListPathAsync(string workspaceId, string itemId, string? path = null, bool recursive = false, CancellationToken cancellationToken = default)
{
if (path is not null)
ValidatePathForTraversal(path, nameof(path));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
// If no path is specified, intelligently discover and search top-level folders
if (string.IsNullOrEmpty(path))
Expand Down Expand Up @@ -874,6 +879,8 @@ public async Task<IEnumerable<OneLakeItem>> ListOneLakeItemsAsync(string workspa

public async Task<string> ListBlobsRawAsync(string workspaceId, string itemId, string? path = null, bool recursive = false, CancellationToken cancellationToken = default)
{
if (path is not null)
ValidatePathForTraversal(path, nameof(path));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
// If no path is specified, intelligently discover and search top-level folders
if (string.IsNullOrEmpty(path))
Expand Down Expand Up @@ -945,6 +952,8 @@ public async Task<string> ListBlobsRawAsync(string workspaceId, string itemId, s

public async Task<string> ListPathRawAsync(string workspaceId, string itemId, string? path = null, bool recursive = false, CancellationToken cancellationToken = default)
{
if (path is not null)
ValidatePathForTraversal(path, nameof(path));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
// If no path is specified, intelligently discover and search top-level folders
if (string.IsNullOrEmpty(path))
Expand Down Expand Up @@ -1256,6 +1265,7 @@ private static List<OneLakeItem> ParseGenericElements(IEnumerable<XElement> elem

public Task<BlobGetResult> ReadFileAsync(string workspaceId, string itemId, string filePath, BlobDownloadOptions? downloadOptions = null, CancellationToken cancellationToken = default)
{
ValidatePathForTraversal(filePath, nameof(filePath));
return DownloadBlobAsync(
OneLakeEndpoints.OneLakeDataPlaneDfsBaseUrl,
workspaceId,
Expand All @@ -1268,6 +1278,7 @@ public Task<BlobGetResult> ReadFileAsync(string workspaceId, string itemId, stri

public async Task WriteFileAsync(string workspaceId, string itemId, string filePath, Stream content, bool overwrite = false, CancellationToken cancellationToken = default)
{
ValidatePathForTraversal(filePath, nameof(filePath));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
// Use DFS endpoint for file operations (similar to directory creation)
var url = $"{OneLakeEndpoints.OneLakeDataPlaneDfsBaseUrl}/{normalizedWorkspaceId}/{normalizedItemId}/{filePath.TrimStart('/')}";
Expand Down Expand Up @@ -1303,6 +1314,7 @@ public async Task WriteFileAsync(string workspaceId, string itemId, string fileP
public async Task<BlobPutResult> PutBlobAsync(string workspaceId, string itemId, string blobPath, Stream content, long contentLength, string? contentType = null, bool overwrite = false, CancellationToken cancellationToken = default)
{
ArgumentNullException.ThrowIfNull(content);
ValidatePathForTraversal(blobPath, nameof(blobPath));

var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
var url = $"{OneLakeEndpoints.OneLakeDataPlaneBlobBaseUrl}/{normalizedWorkspaceId}/{normalizedItemId}/{blobPath.TrimStart('/')}";
Expand Down Expand Up @@ -1388,6 +1400,7 @@ public async Task<BlobPutResult> PutBlobAsync(string workspaceId, string itemId,

public Task<BlobGetResult> GetBlobAsync(string workspaceId, string itemId, string blobPath, BlobDownloadOptions? downloadOptions = null, CancellationToken cancellationToken = default)
{
ValidatePathForTraversal(blobPath, nameof(blobPath));
return DownloadBlobAsync(
OneLakeEndpoints.OneLakeDataPlaneBlobBaseUrl,
workspaceId,
Expand Down Expand Up @@ -1573,9 +1586,9 @@ private async Task<BlobGetResult> DownloadBlobAsync(

public async Task<BlobDeleteResult> DeleteBlobAsync(string workspaceId, string itemId, string blobPath, CancellationToken cancellationToken = default)
{
ValidatePathForTraversal(blobPath, nameof(blobPath));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
var url = $"{OneLakeEndpoints.OneLakeDataPlaneBlobBaseUrl}/{normalizedWorkspaceId}/{normalizedItemId}/{blobPath.TrimStart('/')}";

using var request = new HttpRequestMessage(HttpMethod.Delete, url);
using var response = await SendDataPlaneRequestAsync(request, cancellationToken: cancellationToken);

Expand All @@ -1598,13 +1611,15 @@ public async Task<BlobDeleteResult> DeleteBlobAsync(string workspaceId, string i

public async Task DeleteFileAsync(string workspaceId, string itemId, string filePath, CancellationToken cancellationToken = default)
{
ValidatePathForTraversal(filePath, nameof(filePath));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
var url = $"{OneLakeEndpoints.OneLakeDataPlaneBaseUrl}/{normalizedWorkspaceId}/{normalizedItemId}/{filePath.TrimStart('/')}";
await SendDataPlaneRequestAsync(HttpMethod.Delete, url, cancellationToken: cancellationToken);
}

public async Task DeleteDirectoryAsync(string workspaceId, string itemId, string directoryPath, bool recursive = false, CancellationToken cancellationToken = default)
{
ValidatePathForTraversal(directoryPath, nameof(directoryPath));
// Use blob endpoint for directory deletion, similar to file deletion
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
var url = $"{OneLakeEndpoints.OneLakeDataPlaneBaseUrl}/{normalizedWorkspaceId}/{normalizedItemId}/{directoryPath.TrimStart('/')}";
Expand All @@ -1625,6 +1640,7 @@ public async Task CreateDirectoryAsync(string workspaceId, string itemId, string
{
// In OneLake/Azure Data Lake Storage, directories are created implicitly when files are created
// However, we can create an empty directory using a PUT request with the appropriate headers
ValidatePathForTraversal(directoryPath, nameof(directoryPath));
var (normalizedWorkspaceId, normalizedItemId) = await GetNormalizedIdentifiersAsync(workspaceId, itemId, cancellationToken);
var url = $"{OneLakeEndpoints.OneLakeDataPlaneDfsBaseUrl}/{normalizedWorkspaceId}/{normalizedItemId}/{directoryPath.TrimStart('/')}?resource=directory";

Expand All @@ -1635,6 +1651,34 @@ public async Task CreateDirectoryAsync(string workspaceId, string itemId, string
}

// Private helper methods
private static void ValidatePathForTraversal(string path, string paramName)
{
// Decode percent-encoding so that %2e%2e or %2E%2E variants are caught
// before checking for traversal segments.
string decoded;
try
{
decoded = Uri.UnescapeDataString(path);
}
catch (UriFormatException)
{
// Malformed percent-encoding — treat the raw string as-is.
decoded = path;
}

// Normalise both forward- and back-slash separators and reject any
// segment that resolves to "." or "..".
var segments = decoded.Split('/', '\\');
foreach (var segment in segments)
{
var trimmed = segment.Trim();
if (trimmed is "." or "..")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe consider blocking "~" character as well?

{
throw new ArgumentException("Path cannot contain directory traversal sequences.", paramName);
}
}
}

private async Task<Stream> SendFabricApiRequestAsync(HttpMethod method, string url, string? jsonContent = null, string? tenant = null, CancellationToken cancellationToken = default)
{
var tokenContext = new TokenRequestContext(new[] { OneLakeEndpoints.GetFabricScope() });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.Mcp.Core.Areas.Server.Options;
using Microsoft.Mcp.Core.Models.Command;
using NSubstitute;
using NSubstitute.ExceptionExtensions;

namespace Fabric.Mcp.Tools.OneLake.Tests.Commands;

Expand Down Expand Up @@ -257,6 +258,32 @@ public async Task ExecuteAsync_RejectsDownloadPath_WhenTransportIsHttp()
await service.DidNotReceive().GetBlobAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<BlobDownloadOptions>(), Arg.Any<CancellationToken>());
}

[Theory]
[InlineData("../../secret.txt")]
[InlineData("Files/../../other-item/data")]
[InlineData("../credentials.env")]
public async Task ExecuteAsync_RejectsTraversalPath_ReturnsErrorResponse(string traversalPath)
{
var service = Substitute.For<IOneLakeService>();
var command = new BlobGetCommand(NullLogger<BlobGetCommand>.Instance, service);

service
.GetBlobAsync(
Arg.Any<string>(),
Arg.Any<string>(),
Arg.Is<string>(p => p.Contains("..", StringComparison.Ordinal)),
Arg.Any<BlobDownloadOptions>(),
Arg.Any<CancellationToken>())
.ThrowsAsync(new ArgumentException("Path cannot contain directory traversal sequences.", "blobPath"));

var parseResult = command.GetCommand().Parse($"--workspace-id workspace --item-id item --file-path {traversalPath}");
var context = CreateContext();

var response = await command.ExecuteAsync(context, parseResult, CancellationToken.None);

Assert.NotEqual(HttpStatusCode.OK, response.Status);
}

private static string SerializeResult(ResponseResult? result)
{
if (result is null)
Expand Down Expand Up @@ -285,3 +312,4 @@ private static CommandContext CreateContext(string transport = "stdio")
return new CommandContext(serviceProvider);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Mcp.Core.Models.Command;
using NSubstitute;
using NSubstitute.ExceptionExtensions;
using Xunit;

namespace Fabric.Mcp.Tools.OneLake.Tests.Commands;
Expand Down Expand Up @@ -212,4 +213,34 @@ private static string SerializeResult(ResponseResult? result)

return Encoding.UTF8.GetString(stream.ToArray());
}

[Theory]
[InlineData("../../secret.txt")]
[InlineData("Files/../../other-item/data")]
[InlineData("../credentials.env")]
public async Task ExecuteAsync_RejectsTraversalPath_ReturnsErrorResponse(string traversalPath)
{
var oneLakeService = Substitute.For<IOneLakeService>();
var command = new BlobPutCommand(NullLogger<BlobPutCommand>.Instance, oneLakeService);

oneLakeService
.PutBlobAsync(
Arg.Any<string>(),
Arg.Any<string>(),
Arg.Is<string>(p => p.Contains("..", StringComparison.Ordinal)),
Arg.Any<Stream>(),
Arg.Any<long>(),
Arg.Any<string?>(),
Arg.Any<bool>(),
Arg.Any<CancellationToken>())
.ThrowsAsync(new ArgumentException("Path cannot contain directory traversal sequences.", "blobPath"));

var serviceProvider = Substitute.For<IServiceProvider>();
var parseResult = command.GetCommand().Parse($"--workspace-id workspace --item-id item --file-path {traversalPath} --content data");
var context = new CommandContext(serviceProvider);

var response = await command.ExecuteAsync(context, parseResult, CancellationToken.None);

Assert.NotEqual(HttpStatusCode.Created, response.Status);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using Fabric.Mcp.Tools.OneLake.Commands.File;
using Fabric.Mcp.Tools.OneLake.Services;
using Microsoft.Extensions.Logging;
using Microsoft.Mcp.Core.Models.Command;
using NSubstitute;
using NSubstitute.ExceptionExtensions;

namespace Fabric.Mcp.Tools.OneLake.Tests.Commands;

Expand Down Expand Up @@ -100,4 +102,32 @@ public void Metadata_HasCorrectProperties()
Assert.False(metadata.ReadOnly);
Assert.False(metadata.Secret);
}

[Theory]
[InlineData("../../dir")]
[InlineData("Files/../../other-item")]
[InlineData("../subdir")]
public async Task ExecuteAsync_RejectsTraversalPath_ReturnsErrorResponse(string traversalPath)
{
var logger = LoggerFactory.Create(builder => { }).CreateLogger<DirectoryCreateCommand>();
var oneLakeService = Substitute.For<IOneLakeService>();
var command = new DirectoryCreateCommand(logger, oneLakeService);

oneLakeService
.CreateDirectoryAsync(
Arg.Any<string>(),
Arg.Any<string>(),
Arg.Is<string>(p => p.Contains("..", StringComparison.Ordinal)),
Arg.Any<CancellationToken>())
.ThrowsAsync(new ArgumentException("Path cannot contain directory traversal sequences.", "directoryPath"));

var serviceProvider = Substitute.For<IServiceProvider>();
var systemCommand = command.GetCommand();
var parseResult = systemCommand.Parse($"--workspace-id workspace --item-id item --directory-path {traversalPath}");
var context = new CommandContext(serviceProvider);

var response = await command.ExecuteAsync(context, parseResult, CancellationToken.None);

Assert.NotEqual(System.Net.HttpStatusCode.OK, response.Status);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
using Fabric.Mcp.Tools.OneLake.Commands.File;
using Fabric.Mcp.Tools.OneLake.Services;
using Microsoft.Extensions.Logging;
using Microsoft.Mcp.Core.Models.Command;
using NSubstitute;
using NSubstitute.ExceptionExtensions;

namespace Fabric.Mcp.Tools.OneLake.Tests.Commands;

Expand Down Expand Up @@ -100,4 +102,33 @@ public void Metadata_HasCorrectProperties()
Assert.False(metadata.ReadOnly);
Assert.False(metadata.Secret);
}

[Theory]
[InlineData("../../dir")]
[InlineData("Files/../../other-item")]
[InlineData("../subdir")]
public async Task ExecuteAsync_RejectsTraversalPath_ReturnsErrorResponse(string traversalPath)
{
var logger = LoggerFactory.Create(builder => { }).CreateLogger<DirectoryDeleteCommand>();
var oneLakeService = Substitute.For<IOneLakeService>();
var command = new DirectoryDeleteCommand(logger, oneLakeService);

oneLakeService
.DeleteDirectoryAsync(
Arg.Any<string>(),
Arg.Any<string>(),
Arg.Is<string>(p => p.Contains("..", StringComparison.Ordinal)),
Arg.Any<bool>(),
Arg.Any<CancellationToken>())
.ThrowsAsync(new ArgumentException("Path cannot contain directory traversal sequences.", "directoryPath"));

var serviceProvider = Substitute.For<IServiceProvider>();
var systemCommand = command.GetCommand();
var parseResult = systemCommand.Parse($"--workspace-id workspace --item-id item --directory-path {traversalPath}");
var context = new CommandContext(serviceProvider);

var response = await command.ExecuteAsync(context, parseResult, CancellationToken.None);

Assert.NotEqual(System.Net.HttpStatusCode.OK, response.Status);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,32 @@ public async Task ExecuteAsync_WithMissingIdentifiers_ReturnsValidationError()
Assert.Equal(HttpStatusCode.BadRequest, response.Status);
await oneLakeService.DidNotReceive().DeleteFileAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CancellationToken>());
}

[Theory]
[InlineData("../../secret.txt")]
[InlineData("Files/../../other-item/data")]
[InlineData("../credentials.env")]
public async Task ExecuteAsync_RejectsTraversalPath_ReturnsErrorResponse(string traversalPath)
{
var logger = LoggerFactory.Create(builder => { }).CreateLogger<FileDeleteCommand>();
var oneLakeService = Substitute.For<IOneLakeService>();
var command = new FileDeleteCommand(logger, oneLakeService);

oneLakeService
.DeleteFileAsync(
Arg.Any<string>(),
Arg.Any<string>(),
Arg.Is<string>(p => p.Contains("..", StringComparison.Ordinal)),
Arg.Any<CancellationToken>())
.ThrowsAsync(new ArgumentException("Path cannot contain directory traversal sequences.", "filePath"));

var serviceProvider = Substitute.For<IServiceProvider>();
var systemCommand = command.GetCommand();
var parseResult = systemCommand.Parse($"--workspace-id workspace --item-id item --file-path {traversalPath}");
var context = new CommandContext(serviceProvider);

var response = await command.ExecuteAsync(context, parseResult, CancellationToken.None);

Assert.NotEqual(HttpStatusCode.OK, response.Status);
}
}
Loading