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
41 changes: 41 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -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.
24 changes: 24 additions & 0 deletions .github/instructions/pre-commit-testing.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
applyTo:
- "**"
---

# Pre-Commit Testing Requirements
Comment thread
MIchaelMainer marked this conversation as resolved.

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.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions src/kiota/kiota.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@
<PackageReference Include="OpenTelemetry.Instrumentation.Runtime" Version="1.15.1" />
<PackageReference Include="Spectre.Console" Version="0.55.2" />
<PackageReference Include="StreamJsonRpc" Version="2.24.92" />
<!-- StreamJsonRpc 2.24.92 uses a vulnerable version of MessagePack. https://github.com/advisories/GHSA-92vj-hp7m-gwcj
TODO: Replace with a non-vulnerable version once available and delete the explicit transitive reference of Nerdbank.MessagePack. -->
<PackageReference Include="Nerdbank.MessagePack" Version="1.2.4" />
<PackageReference Include="System.CommandLine" Version="2.0.8" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="10.0.300" PrivateAssets="All" />
<ProjectReference Include="..\Kiota.Generated\KiotaGenerated.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
Expand Down
65 changes: 65 additions & 0 deletions tests/Kiota.Builder.Tests/Writers/Python/CodeEnumWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
Comment thread
MIchaelMainer marked this conversation as resolved.
Dismissed
}
[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}");
}
Comment thread
MIchaelMainer marked this conversation as resolved.
Dismissed
// 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading