Skip to content

Commit 192a46d

Browse files
gavinbarronCopilot
gavinbarron
and
Copilot
committed
Fix path traversal via workspace consumer identifiers in DescriptionStorageService
Workspace consumer identifiers (clientName/pluginName) and workspace configuration keys were used directly as filesystem path components when caching OpenAPI descriptions under .kiota/documents, with no canonicalization or containment validation. A crafted name such as junk/../Victim could escape the intended consumer namespace and overwrite a sibling consumer's cached description, which is later trusted by client/plugin edit operations. Adds defense-in-depth validation: - DescriptionStorageService: normalized-containment check + consumer-name validation at the storage sink. - WorkspaceManagementService: early consumer-name validation at all entry points (is-present, update-state, get-description-copy, remove client/plugin). - WorkspaceConfigurationStorageService: validates consumer keys on config load. Adds regression tests for all three layers. Fixes #7919 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4cc79c9 commit 192a46d

7 files changed

Lines changed: 212 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313

1414
### Changed
1515

16+
- Fixed a path traversal vulnerability where workspace consumer identifiers (`clientName`/`pluginName`) and workspace configuration keys were used as filesystem path components without containment validation, allowing a crafted name (e.g. `junk/../Victim`) to overwrite another consumer's cached OpenAPI description. [#7919](https://github.com/microsoft/kiota/issues/7919)
17+
1618
## [1.33.0] - 2026-07-06
1719

1820
### Added

‎src/Kiota.Builder/WorkspaceManagement/DescriptionStorageService.cs‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.IO;
3+
using System.Linq;
34
using System.Threading;
45
using System.Threading.Tasks;
56
using AsyncKeyedLock;
@@ -21,7 +22,29 @@ public DescriptionStorageService(string targetDirectory)
2122
o.PoolSize = 20;
2223
o.PoolInitialFill = 1;
2324
});
24-
private string GetDescriptionFilePath(string clientName, string extension) => Path.Combine(TargetDirectory, DescriptionsSubDirectoryRelativePath, clientName, $"openapi.{extension}");
25+
private string GetDescriptionFilePath(string clientName, string extension)
26+
{
27+
ValidateConsumerName(clientName);
28+
var documentsDirectory = Path.Combine(TargetDirectory, DescriptionsSubDirectoryRelativePath);
29+
var descriptionFilePath = Path.GetFullPath(Path.Combine(documentsDirectory, clientName, $"openapi.{extension}"));
30+
var documentsFullPath = Path.GetFullPath(documentsDirectory);
31+
var documentsFullPathWithSeparator = Path.EndsInDirectorySeparator(documentsFullPath) ? documentsFullPath : documentsFullPath + Path.DirectorySeparatorChar;
32+
var comparison = OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
33+
if (!descriptionFilePath.StartsWith(documentsFullPathWithSeparator, comparison))
34+
throw new InvalidOperationException($"The consumer name '{clientName}' resolves to a path outside of the documents directory.");
35+
return descriptionFilePath;
36+
}
37+
internal static void ValidateConsumerName(string clientName)
38+
{
39+
if (string.IsNullOrWhiteSpace(clientName))
40+
throw new InvalidOperationException("The consumer name must not be empty or whitespace.");
41+
if (Path.IsPathRooted(clientName) ||
42+
clientName.Contains('/', StringComparison.Ordinal) ||
43+
clientName.Contains('\\', StringComparison.Ordinal) ||
44+
clientName.Split('/', '\\').Contains("..", StringComparer.Ordinal) ||
45+
clientName is "." or "..")
46+
throw new InvalidOperationException($"The consumer name '{clientName}' is not a valid single path segment and cannot navigate the file system.");
47+
}
2548
public async Task UpdateDescriptionAsync(string clientName, Stream description, string extension = "yml", CancellationToken cancellationToken = default)
2649
{
2750
ArgumentNullException.ThrowIfNull(clientName);

‎src/Kiota.Builder/WorkspaceManagement/WorkspaceConfigurationStorageService.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,20 +111,22 @@ private void ValidateConsumerOutputPaths(WorkspaceConfiguration? configuration)
111111
foreach (var client in configuration.Clients)
112112
try
113113
{
114+
DescriptionStorageService.ValidateConsumerName(client.Key);
114115
BaseApiConsumerConfiguration.ValidateOutputPath(client.Value.OutputPath, workspaceDirectory);
115116
}
116117
catch (InvalidOperationException ex)
117118
{
118-
throw new InvalidOperationException($"The client {client.Key} has an invalid output path: {ex.Message}", ex);
119+
throw new InvalidOperationException($"The client {client.Key} has an invalid configuration: {ex.Message}", ex);
119120
}
120121
foreach (var plugin in configuration.Plugins)
121122
try
122123
{
124+
DescriptionStorageService.ValidateConsumerName(plugin.Key);
123125
BaseApiConsumerConfiguration.ValidateOutputPath(plugin.Value.OutputPath, workspaceDirectory);
124126
}
125127
catch (InvalidOperationException ex)
126128
{
127-
throw new InvalidOperationException($"The plugin {plugin.Key} has an invalid output path: {ex.Message}", ex);
129+
throw new InvalidOperationException($"The plugin {plugin.Key} has an invalid configuration: {ex.Message}", ex);
128130
}
129131
}
130132
public async Task BackupConfigAsync(CancellationToken cancellationToken = default)

‎src/Kiota.Builder/WorkspaceManagement/WorkspaceManagementService.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public WorkspaceManagementService(ILogger logger, HttpClient httpClient, bool us
4242
private readonly DescriptionStorageService descriptionStorageService;
4343
public async Task<bool> IsConsumerPresentAsync(string clientName, CancellationToken cancellationToken = default)
4444
{
45+
DescriptionStorageService.ValidateConsumerName(clientName);
4546
if (!UseKiotaConfig) return false;
4647
var (wsConfig, _) = await workspaceConfigurationStorageService.GetWorkspaceConfigurationAsync(cancellationToken).ConfigureAwait(false);
4748
return wsConfig is not null && (wsConfig.Clients.ContainsKey(clientName) || wsConfig.Plugins.ContainsKey(clientName));
@@ -68,6 +69,7 @@ private BaseApiConsumerConfiguration UpdateConsumerConfiguration(GenerationConfi
6869
public async Task UpdateStateFromConfigurationAsync(GenerationConfiguration generationConfiguration, string descriptionHash, Dictionary<string, HashSet<string>> templatesWithOperations, Stream descriptionStream, CancellationToken cancellationToken = default)
6970
{
7071
ArgumentNullException.ThrowIfNull(generationConfiguration);
72+
DescriptionStorageService.ValidateConsumerName(generationConfiguration.ClientClassName);
7173
if (UseKiotaConfig)
7274
{
7375
var (wsConfig, manifest) = await LoadConfigurationAndManifestAsync(cancellationToken).ConfigureAwait(false);
@@ -154,6 +156,7 @@ public async Task<bool> ShouldGenerateAsync(GenerationConfiguration inputConfig,
154156
}
155157
public async Task<Stream?> GetDescriptionCopyAsync(string clientName, string inputPath, bool cleanOutput, CancellationToken cancellationToken = default)
156158
{
159+
DescriptionStorageService.ValidateConsumerName(clientName);
157160
if (!UseKiotaConfig || cleanOutput)
158161
return null;
159162
return await descriptionStorageService.GetDescriptionAsync(clientName, new Uri(inputPath).GetFileExtension(), cancellationToken).ConfigureAwait(false);
@@ -178,6 +181,7 @@ public Task RemovePluginAsync(string clientName, bool cleanOutput = false, Cance
178181
}
179182
private async Task RemoveConsumerInternalAsync<T>(string consumerName, Func<WorkspaceConfiguration, Dictionary<string, T>> consumerRetrieval, bool cleanOutput, string consumerDisplayName, CancellationToken cancellationToken) where T : BaseApiConsumerConfiguration
180183
{
184+
DescriptionStorageService.ValidateConsumerName(consumerName);
181185
if (!UseKiotaConfig)
182186
throw new InvalidOperationException($"Cannot remove a {consumerDisplayName} in lock mode");
183187
var (wsConfig, manifest) = await workspaceConfigurationStorageService.GetWorkspaceConfigurationAsync(cancellationToken).ConfigureAwait(false);

‎tests/Kiota.Builder.Tests/WorkspaceManagement/DescriptionStorageServiceTests.cs‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,62 @@ public async Task DefensiveAsync()
4646
await Assert.ThrowsAsync<ArgumentNullException>(() => service.UpdateDescriptionAsync("foo", null, cancellationToken: TestContext.Current.CancellationToken));
4747
await Assert.ThrowsAsync<ArgumentNullException>(() => service.GetDescriptionAsync(null, cancellationToken: TestContext.Current.CancellationToken));
4848
}
49+
50+
[Theory]
51+
[InlineData("junk/../Victim")]
52+
[InlineData("../Victim")]
53+
[InlineData("..")]
54+
[InlineData("a/b")]
55+
[InlineData("a\\b")]
56+
[InlineData("./Victim")]
57+
[InlineData(".")]
58+
[InlineData(" ")]
59+
public async Task UpdateDescriptionRejectsTraversalNamesAsync(string clientName)
60+
{
61+
var service = new DescriptionStorageService(tempPath);
62+
using var stream = new MemoryStream();
63+
stream.WriteByte(0x1);
64+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.UpdateDescriptionAsync(clientName, stream, cancellationToken: TestContext.Current.CancellationToken));
65+
}
66+
67+
[Fact]
68+
public async Task UpdateDescriptionTraversalDoesNotEscapeConsumerNamespaceAsync()
69+
{
70+
var service = new DescriptionStorageService(tempPath);
71+
using var victimStream = new MemoryStream();
72+
victimStream.WriteByte(0x2);
73+
await service.UpdateDescriptionAsync("Victim", victimStream, cancellationToken: TestContext.Current.CancellationToken);
74+
75+
using var maliciousStream = new MemoryStream();
76+
maliciousStream.WriteByte(0x9);
77+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.UpdateDescriptionAsync("junk/../Victim", maliciousStream, cancellationToken: TestContext.Current.CancellationToken));
78+
79+
// The victim's cached description must remain untouched (single byte 0x2 written above).
80+
var victimFilePath = Path.Combine(tempPath, DescriptionStorageService.DescriptionsSubDirectoryRelativePath, "Victim", "openapi.yml");
81+
Assert.True(File.Exists(victimFilePath));
82+
var contents = await File.ReadAllBytesAsync(victimFilePath, TestContext.Current.CancellationToken);
83+
Assert.Equal([0x2], contents);
84+
}
85+
86+
[Theory]
87+
[InlineData("junk/../Victim")]
88+
[InlineData("../Victim")]
89+
[InlineData("a/b")]
90+
[InlineData("a\\b")]
91+
public async Task GetDescriptionRejectsTraversalNamesAsync(string clientName)
92+
{
93+
var service = new DescriptionStorageService(tempPath);
94+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.GetDescriptionAsync(clientName, cancellationToken: TestContext.Current.CancellationToken));
95+
}
96+
97+
[Theory]
98+
[InlineData("junk/../Victim")]
99+
[InlineData("../Victim")]
100+
[InlineData("a/b")]
101+
[InlineData("a\\b")]
102+
public void RemoveDescriptionRejectsTraversalNames(string clientName)
103+
{
104+
var service = new DescriptionStorageService(tempPath);
105+
Assert.Throws<InvalidOperationException>(() => service.RemoveDescription(clientName));
106+
}
49107
}

‎tests/Kiota.Builder.Tests/WorkspaceManagement/WorkspaceConfigurationStorageServiceTests.cs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,50 @@ await WriteWorkspaceConfigurationAsync($$"""
113113

114114
await Assert.ThrowsAsync<InvalidOperationException>(() => service.GetWorkspaceConfigurationAsync(cancellationToken: TestContext.Current.CancellationToken));
115115
}
116+
[InlineData("../Victim")]
117+
[InlineData("junk/../Victim")]
118+
[InlineData("a/b")]
119+
[InlineData("a\\b")]
120+
[Theory]
121+
public async Task RejectsClientNameWithTraversalKeyAsync(string clientName)
122+
{
123+
var service = new WorkspaceConfigurationStorageService(tempPath);
124+
await WriteWorkspaceConfigurationAsync($$"""
125+
{
126+
"version": "1.0.0",
127+
"clients": {
128+
"{{clientName.Replace("\\", "\\\\", StringComparison.Ordinal)}}": {
129+
"outputPath": "./client"
130+
}
131+
},
132+
"plugins": {}
133+
}
134+
""");
135+
136+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.GetWorkspaceConfigurationAsync(cancellationToken: TestContext.Current.CancellationToken));
137+
}
138+
[InlineData("../Victim")]
139+
[InlineData("junk/../Victim")]
140+
[InlineData("a/b")]
141+
[InlineData("a\\b")]
142+
[Theory]
143+
public async Task RejectsPluginNameWithTraversalKeyAsync(string pluginName)
144+
{
145+
var service = new WorkspaceConfigurationStorageService(tempPath);
146+
await WriteWorkspaceConfigurationAsync($$"""
147+
{
148+
"version": "1.0.0",
149+
"clients": {},
150+
"plugins": {
151+
"{{pluginName.Replace("\\", "\\\\", StringComparison.Ordinal)}}": {
152+
"outputPath": "./plugin"
153+
}
154+
}
155+
}
156+
""");
157+
158+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.GetWorkspaceConfigurationAsync(cancellationToken: TestContext.Current.CancellationToken));
159+
}
116160
private async Task WriteWorkspaceConfigurationAsync(string content)
117161
{
118162
var configurationDirectory = Path.Combine(tempPath, WorkspaceConfigurationStorageService.KiotaDirectorySegment);

‎tests/Kiota.Builder.Tests/WorkspaceManagement/WorkspaceManagementServiceTests.cs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,82 @@ await File.WriteAllTextAsync(descriptionPath, @$"openapi: 3.0.1
264264
Assert.NotNull(descriptionCopy);
265265
}
266266

267+
[Theory]
268+
[InlineData("junk/../Victim")]
269+
[InlineData("../Victim")]
270+
[InlineData("a/b")]
271+
[InlineData("a\\b")]
272+
public async Task RemoveClientRejectsTraversalNamesAsync(string clientName)
273+
{
274+
var mockLogger = Mock.Of<ILogger>();
275+
Directory.CreateDirectory(tempPath);
276+
var service = new WorkspaceManagementService(mockLogger, httpClient, true, tempPath);
277+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.RemoveClientAsync(clientName, cancellationToken: TestContext.Current.CancellationToken));
278+
}
279+
[Theory]
280+
[InlineData("junk/../Victim")]
281+
[InlineData("../Victim")]
282+
[InlineData("a/b")]
283+
[InlineData("a\\b")]
284+
public async Task RemovePluginRejectsTraversalNamesAsync(string clientName)
285+
{
286+
var mockLogger = Mock.Of<ILogger>();
287+
Directory.CreateDirectory(tempPath);
288+
var service = new WorkspaceManagementService(mockLogger, httpClient, true, tempPath);
289+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.RemovePluginAsync(clientName, cancellationToken: TestContext.Current.CancellationToken));
290+
}
291+
[Theory]
292+
[InlineData("junk/../Victim")]
293+
[InlineData("../Victim")]
294+
[InlineData("a/b")]
295+
[InlineData("a\\b")]
296+
public async Task IsConsumerPresentRejectsTraversalNamesAsync(string clientName)
297+
{
298+
var mockLogger = Mock.Of<ILogger>();
299+
Directory.CreateDirectory(tempPath);
300+
var service = new WorkspaceManagementService(mockLogger, httpClient, true, tempPath);
301+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.IsConsumerPresentAsync(clientName, cancellationToken: TestContext.Current.CancellationToken));
302+
}
303+
[Theory]
304+
[InlineData("junk/../Victim")]
305+
[InlineData("../Victim")]
306+
[InlineData("a/b")]
307+
[InlineData("a\\b")]
308+
public async Task GetDescriptionCopyRejectsTraversalNamesAsync(string clientName)
309+
{
310+
var mockLogger = Mock.Of<ILogger>();
311+
Directory.CreateDirectory(tempPath);
312+
var service = new WorkspaceManagementService(mockLogger, httpClient, true, tempPath);
313+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.GetDescriptionCopyAsync(clientName, Path.Combine(tempPath, "openapi.yml"), false, cancellationToken: TestContext.Current.CancellationToken));
314+
}
315+
[Theory]
316+
[InlineData("junk/../Victim")]
317+
[InlineData("../Victim")]
318+
[InlineData("a/b")]
319+
[InlineData("a\\b")]
320+
public async Task UpdateStateRejectsTraversalClientNamesAsync(string clientName)
321+
{
322+
var mockLogger = Mock.Of<ILogger>();
323+
Directory.CreateDirectory(tempPath);
324+
var service = new WorkspaceManagementService(mockLogger, httpClient, true, tempPath);
325+
var configuration = new GenerationConfiguration
326+
{
327+
ClientClassName = clientName,
328+
OutputPath = Path.Combine(tempPath, "client"),
329+
OpenAPIFilePath = Path.Combine(tempPath, "openapi.yaml"),
330+
ApiRootUrl = "https://graph.microsoft.com",
331+
};
332+
using var stream = new MemoryStream();
333+
stream.WriteByte(0x1);
334+
await Assert.ThrowsAsync<InvalidOperationException>(() => service.UpdateStateFromConfigurationAsync(
335+
configuration,
336+
"foo",
337+
new Dictionary<string, HashSet<string>> {
338+
{ "/foo", new HashSet<string> { "GET" } }
339+
},
340+
stream, cancellationToken: TestContext.Current.CancellationToken));
341+
}
342+
267343
public void Dispose()
268344
{
269345
if (Directory.Exists(tempPath))

0 commit comments

Comments
 (0)