From 1a7033d38714337642c740b4648475538fbc03bf Mon Sep 17 00:00:00 2001 From: "Michael Mainer (from Dev Box)" <8527305+MIchaelMainer@users.noreply.github.com> Date: Tue, 26 May 2026 12:19:35 -0700 Subject: [PATCH 1/6] fix(writers/python): prevent code injection via x-ms-enum description newlines The RemoveInvalidDescriptionCharacters method in PythonConventionService did not strip newline characters (\r\n, \r, \n). A malicious OpenAPI spec could inject arbitrary Python code via x-ms-enum description fields by embedding newlines that break out of inline comments (# ...) or docstrings. This fix replaces newlines with spaces, keeping all description content on a single line where it remains safely within comments/docstrings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Kiota.Builder/KiotaBuilder.cs | 2 +- .../Writers/Python/PythonConventionService.cs | 8 +++- .../Writers/Python/CodeEnumWriterTests.cs | 40 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index b6990808bd..889759a84b 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -2082,7 +2082,7 @@ private static void SetEnumOptions(IOpenApiSchema schema, CodeEnum target) SerializationName = x, Documentation = new() { - DescriptionTemplate = optionDescription?.Description ?? string.Empty, + DescriptionTemplate = optionDescription?.Description?.CleanupDescription() ?? string.Empty, }, }; }) diff --git a/src/Kiota.Builder/Writers/Python/PythonConventionService.cs b/src/Kiota.Builder/Writers/Python/PythonConventionService.cs index 2df914ef7a..504d41c749 100644 --- a/src/Kiota.Builder/Writers/Python/PythonConventionService.cs +++ b/src/Kiota.Builder/Writers/Python/PythonConventionService.cs @@ -96,7 +96,13 @@ public override string GetTypeString(CodeTypeBase code, CodeElement targetElemen throw new InvalidOperationException($"type of type {code.GetType()} is unknown"); } #pragma warning restore CA1822 // Method should be static - internal static string RemoveInvalidDescriptionCharacters(string originalDescription) => originalDescription.Replace("\\", "/", StringComparison.OrdinalIgnoreCase).Replace("\"\"\"", "\\\"\\\"\\\"", StringComparison.OrdinalIgnoreCase); + internal static string RemoveInvalidDescriptionCharacters(string originalDescription) => + string.IsNullOrEmpty(originalDescription) ? string.Empty : + originalDescription.Replace("\\", "/", StringComparison.OrdinalIgnoreCase) + .Replace("\t", " ", StringComparison.OrdinalIgnoreCase) + .Replace("\r", " ", StringComparison.OrdinalIgnoreCase) + .Replace("\n", " ", StringComparison.OrdinalIgnoreCase) + .Replace("\"\"\"", "\\\"\\\"\\\"", StringComparison.OrdinalIgnoreCase); public override string TranslateType(CodeType type) { ArgumentNullException.ThrowIfNull(type); diff --git a/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs index f43dd34522..f0d3099da8 100644 --- a/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs @@ -66,4 +66,44 @@ public void EscapesEnumWireValues() var result = tw.ToString(); Assert.Contains("Option1 = \"line1\\\"\\nline2\",", result); } + [Fact] + public void SanitizesEnumOptionDescriptionWithNewlines() + { + currentEnum.AddOption(new CodeEnumOption + { + Name = "Option1", + SerializationName = "option1", + Documentation = new() + { + DescriptionTemplate = "line1\nimport os; os.system('evil')\r\nline3", + }, + }); + writer.Write(currentEnum); + var result = tw.ToString(); + // Newlines replaced with spaces: entire payload is on one comment line, not executable code + Assert.Contains("# line1 import os; os.system('evil') line3", result); + // Verify no raw newlines exist within the comment content itself + var commentLine = result.Split('\n').First(l => l.TrimStart().StartsWith("# line1")).TrimEnd('\r'); + Assert.DoesNotContain("\n", commentLine); + Assert.DoesNotContain("\r", commentLine); + } + [Fact] + public void SanitizesEnumOptionDescriptionWithTripleQuotes() + { + currentEnum.AddOption(new CodeEnumOption + { + Name = "Option1", + SerializationName = "option1", + Documentation = new() + { + DescriptionTemplate = "before\"\"\"\nimport os\nafter", + }, + }); + writer.Write(currentEnum); + var result = tw.ToString(); + // Triple quotes escaped, newlines replaced with spaces — all on one comment line + Assert.Contains("# before\\\"\\\"\\\" import os after", result); + var commentLine = result.Split('\n').First(l => l.TrimStart().StartsWith("# before")); + Assert.DoesNotContain("\"\"\"", commentLine); + } } From fd6f19c2485d6658280612375a52bd2fc29ab4ee Mon Sep 17 00:00:00 2001 From: "Michael Mainer (from Dev Box)" <8527305+MIchaelMainer@users.noreply.github.com> Date: Tue, 26 May 2026 12:19:35 -0700 Subject: [PATCH 2/6] fix(writers/python): prevent code injection via x-ms-enum description newlines The RemoveInvalidDescriptionCharacters method in PythonConventionService did not strip newline characters (\r\n, \r, \n). A malicious OpenAPI spec could inject arbitrary Python code via x-ms-enum description fields by embedding newlines that break out of inline comments (# ...) or docstrings. This fix replaces newlines with spaces, keeping all description content on a single line where it remains safely within comments/docstrings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Kiota.Builder/KiotaBuilder.cs | 2 +- .../Writers/Python/PythonConventionService.cs | 8 ++- .../Writers/Python/CodeEnumWriterTests.cs | 65 +++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index b6990808bd..889759a84b 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -2082,7 +2082,7 @@ private static void SetEnumOptions(IOpenApiSchema schema, CodeEnum target) SerializationName = x, Documentation = new() { - DescriptionTemplate = optionDescription?.Description ?? string.Empty, + DescriptionTemplate = optionDescription?.Description?.CleanupDescription() ?? string.Empty, }, }; }) diff --git a/src/Kiota.Builder/Writers/Python/PythonConventionService.cs b/src/Kiota.Builder/Writers/Python/PythonConventionService.cs index 2df914ef7a..504d41c749 100644 --- a/src/Kiota.Builder/Writers/Python/PythonConventionService.cs +++ b/src/Kiota.Builder/Writers/Python/PythonConventionService.cs @@ -96,7 +96,13 @@ public override string GetTypeString(CodeTypeBase code, CodeElement targetElemen throw new InvalidOperationException($"type of type {code.GetType()} is unknown"); } #pragma warning restore CA1822 // Method should be static - internal static string RemoveInvalidDescriptionCharacters(string originalDescription) => originalDescription.Replace("\\", "/", StringComparison.OrdinalIgnoreCase).Replace("\"\"\"", "\\\"\\\"\\\"", StringComparison.OrdinalIgnoreCase); + internal static string RemoveInvalidDescriptionCharacters(string originalDescription) => + string.IsNullOrEmpty(originalDescription) ? string.Empty : + originalDescription.Replace("\\", "/", StringComparison.OrdinalIgnoreCase) + .Replace("\t", " ", StringComparison.OrdinalIgnoreCase) + .Replace("\r", " ", StringComparison.OrdinalIgnoreCase) + .Replace("\n", " ", StringComparison.OrdinalIgnoreCase) + .Replace("\"\"\"", "\\\"\\\"\\\"", StringComparison.OrdinalIgnoreCase); public override string TranslateType(CodeType type) { ArgumentNullException.ThrowIfNull(type); diff --git a/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs index f43dd34522..1e4326db29 100644 --- a/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs @@ -66,4 +66,69 @@ public void EscapesEnumWireValues() var result = tw.ToString(); Assert.Contains("Option1 = \"line1\\\"\\nline2\",", result); } + [Fact] + public void SanitizesEnumOptionDescriptionWithNewlines() + { + currentEnum.AddOption(new CodeEnumOption + { + Name = "Option1", + SerializationName = "option1", + Documentation = new() + { + DescriptionTemplate = "line1\nimport os; os.system('evil')\r\nline3", + }, + }); + writer.Write(currentEnum); + var result = tw.ToString(); + Assert.Contains("Option1 = \"option1\"", result); + // Verify no injected code appears as executable Python (on its own line outside a comment) + var lines = result.Split('\n').Select(l => l.TrimEnd('\r')).Where(l => !string.IsNullOrWhiteSpace(l)).ToArray(); + foreach (var line in lines) + { + var trimmed = line.TrimStart(); + // Every line must be a comment, import, class declaration, or assignment — never bare injected code + Assert.True( + trimmed.StartsWith('#') || trimmed.StartsWith("from ") || trimmed.StartsWith("class ") || trimmed.Contains('=') || trimmed == "pass", + $"Unexpected executable line in output: {trimmed}"); + } + } + [Fact] + public void SanitizesEnumOptionDescriptionWithTripleQuotes() + { + currentEnum.AddOption(new CodeEnumOption + { + Name = "Option1", + SerializationName = "option1", + Documentation = new() + { + DescriptionTemplate = "before\"\"\"\nimport os\nafter", + }, + }); + writer.Write(currentEnum); + var result = tw.ToString(); + Assert.Contains("Option1 = \"option1\"", result); + // Verify no injected code appears as executable Python + var lines = result.Split('\n').Select(l => l.TrimEnd('\r')).Where(l => !string.IsNullOrWhiteSpace(l)).ToArray(); + foreach (var line in lines) + { + var trimmed = line.TrimStart(); + Assert.True( + trimmed.StartsWith('#') || trimmed.StartsWith("from ") || trimmed.StartsWith("class ") || trimmed.Contains('=') || trimmed == "pass", + $"Unexpected executable line in output: {trimmed}"); + } + // Verify no unescaped triple quotes that could break out of a docstring + Assert.DoesNotContain("\"\"\"\nimport", result); + } + [Theory] + [InlineData("line1\nimport os\nline3", "line1 import os line3")] + [InlineData("line1\r\nimport os\r\nline3", "line1 import os line3")] + [InlineData("before\"\"\"\nimport os\nafter", "before\\\"\\\"\\\" import os after")] + [InlineData("normal description", "normal description")] + [InlineData("", "")] + [InlineData(null, "")] + public void RemoveInvalidDescriptionCharactersHandlesInjection(string input, string expected) + { + var result = Kiota.Builder.Writers.Python.PythonConventionService.RemoveInvalidDescriptionCharacters(input!); + Assert.Equal(expected, result); + } } From 3592adf408895791ef21c61b709907657b2a37a6 Mon Sep 17 00:00:00 2001 From: "Michael Mainer (from Dev Box)" <8527305+MIchaelMainer@users.noreply.github.com> Date: Tue, 26 May 2026 14:02:31 -0700 Subject: [PATCH 3/6] docs: add instructions for agents to run tests and fix failing tests before commit --- .../pre-commit-testing.instructions.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .github/instructions/pre-commit-testing.instructions.md diff --git a/.github/instructions/pre-commit-testing.instructions.md b/.github/instructions/pre-commit-testing.instructions.md new file mode 100644 index 0000000000..cfae6a25d4 --- /dev/null +++ b/.github/instructions/pre-commit-testing.instructions.md @@ -0,0 +1,24 @@ +--- +applyTo: + - "**" +--- + +# Pre-Commit Testing Requirements + +Before creating any git commit, agents **must** run the relevant tests and verify they pass. + +## Rules + +1. **Always run tests before committing.** Never commit code that has not been validated by running the relevant test suite. +2. **Scope tests appropriately.** Run at minimum the tests related to the files you changed. If unsure which tests cover your changes, run the full test project that contains the modified code. +3. **Fix failing tests before committing.** If tests fail, diagnose and fix the issue. Do not commit with known test failures unless explicitly instructed by the user. +4. **Re-run tests after fixing failures.** After making corrections, run tests again to confirm they pass. + +## Test Commands + +| Project | Command | +|---------|---------| +| Kiota.Builder | `dotnet test tests/Kiota.Builder.Tests/Kiota.Builder.Tests.csproj` | +| VS Code Extension | `cd vscode/packages && npm test` | + +Use `--filter "FullyQualifiedName~ClassName"` to scope .NET tests to specific test classes when a full run is unnecessary. From b91b284bf2ba5b02b1b7f355cc9e93031e9fa5ea Mon Sep 17 00:00:00 2001 From: "Michael Mainer (from Dev Box)" <8527305+MIchaelMainer@users.noreply.github.com> Date: Mon, 1 Jun 2026 11:09:33 -0700 Subject: [PATCH 4/6] test(python): align deprecation warning assertion with description sanitization The Python writer now normalizes description control characters (newline, carriage return, tab) to spaces as part of injection hardening. This caused EscapesDeprecationWarningStringLiteral to expect an outdated \n sequence in the generated warning output. --- .../Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs index c38cdc5487..c05f3f317b 100644 --- a/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs @@ -2551,7 +2551,7 @@ public void EscapesDeprecationWarningStringLiteral() method.Deprecation = new("line1\"\nline2"); writer.Write(method); var result = tw.ToString(); - Assert.Contains("warn(\"line1\\\"\\nline2", result); + Assert.Contains("warn(\"line1\\\" line2", result); Assert.DoesNotContain("line1\"\nline2", result); } [Fact] From 75943340a9e1ac3a925f952f589479af49100e4c Mon Sep 17 00:00:00 2001 From: "Michael Mainer (from Dev Box)" <8527305+MIchaelMainer@users.noreply.github.com> Date: Mon, 1 Jun 2026 11:38:43 -0700 Subject: [PATCH 5/6] docs: add central Copilot policies and validation checklist for repository-wide defaults --- .github/copilot-instructions.md | 41 +++++++++++++++++++ .../writer-literal-security.instructions.md | 4 +- 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..667f906bd9 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,41 @@ +--- +applyTo: + - "**" +--- + +# Kiota Central Copilot Policies + +This file defines repository-wide defaults for Copilot behavior. It is always applied. + +## Scope and precedence + +1. Apply this file for all work in the repository. +2. Also apply any matching files under `.github/instructions/`. +3. If guidance conflicts, follow the more specific instruction file for the target paths. + +## Required defaults + +1. Keep changes minimal and targeted. Avoid unrelated refactors. +2. Preserve existing style, naming, and public behavior unless the task requires changes. +3. Add or update tests for behavior changes or bug fixes. +4. Before proposing a commit, run relevant tests and ensure they pass. +5. If tests fail, fix issues and re-run tests before considering the work complete. +6. Prefer non-destructive actions and do not revert unrelated local changes. + +## Security review defaults + +When modifying code generation or writer/refiner logic, treat schema-derived values as untrusted and ensure literal-context sanitization at the emission site. + +Use language/literal-appropriate sanitizers from `src/Kiota.Builder/Writers/StringExtensions.cs` (and Dart convention helpers where applicable). + +## Writer hardening reminder + +For writer changes that emit schema-derived text into generated code, ensure hostile content is escaped and covered by regression tests in `tests/Kiota.Builder.Tests/Writers/`. + +## Validation checklist + +Before finishing implementation work: + +1. Build affected projects. +2. Run targeted tests first, then broader tests if impact is unclear. +3. Confirm no new warnings/errors were introduced by the change. diff --git a/.github/instructions/writer-literal-security.instructions.md b/.github/instructions/writer-literal-security.instructions.md index 204839ba50..c90a286dcb 100644 --- a/.github/instructions/writer-literal-security.instructions.md +++ b/.github/instructions/writer-literal-security.instructions.md @@ -44,6 +44,8 @@ Sanitizers are in `src/Kiota.Builder/Writers/StringExtensions.cs` and `DartConve ## Test expectations -Changes to writer emission paths should include regression tests in `tests/Kiota.Builder.Tests/Writers/` that: +Always follow `.github/instructions/pre-commit-testing.instructions.md` for test execution, scoping, and pass/fail validation before commit. + +For writer emission changes, include regression tests in `tests/Kiota.Builder.Tests/Writers/` that: - Use hostile payloads containing `"`, `'`, `\n`, `\r`, `\t`, `\\`, and `$`. - Assert the output contains **escaped** characters, not raw injection payloads. From 7580cf24ec1acd6ce5f67a4c0a8fda79a261c3cb Mon Sep 17 00:00:00 2001 From: "Michael Mainer (from Dev Box)" <8527305+MIchaelMainer@users.noreply.github.com> Date: Mon, 1 Jun 2026 14:03:25 -0700 Subject: [PATCH 6/6] fix: add Nerdbank.MessagePack explicit reference and note vulnerability in StreamJsonRpc --- src/kiota/kiota.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/kiota/kiota.csproj b/src/kiota/kiota.csproj index cdcf6522e9..efd269b1ca 100644 --- a/src/kiota/kiota.csproj +++ b/src/kiota/kiota.csproj @@ -62,6 +62,9 @@ + +