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/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. 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. 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/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 @@ + + 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); + } } 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]