fix: sanitizes the client class and namespace names to avoid code injection - #7884
Merged
Gavin Barron (gavinbarron) merged 4 commits intoJul 1, 2026
Merged
Conversation
…ection Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Gavin Barron (gavinbarron)
changed the title
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Gavin Barron (gavinbarron)
enabled auto-merge (squash)
July 1, 2026 18:40
Code Coverage OverviewLanguages: C# C# / code-coverage/dotnetThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Peter Ombwa (peombwa)
previously approved these changes
Jul 1, 2026
Peter Ombwa (peombwa)
approved these changes
Jul 1, 2026
Eelco Los (EelcoLos)
added a commit
to EelcoLos/kiota
that referenced
this pull request
Aug 11, 2026
…ection (backport of microsoft#7884 / GHSA-4vv7-jj25-4gh6 / CVE-2026-59866) ClientClassName and ClientNamespaceName loaded from the x-ms-kiota-info OpenAPI extension (or settings) were used unsanitized in generated code and file paths, allowing an attacker-controlled OpenAPI description to inject arbitrary paths or code via those values. Test-file TestContext.Current.CancellationToken usages (xunit v3 API, unavailable on this branch's xunit 2.9.3) replaced with this branch's existing convention (new CancellationToken() / omitted).
Vincent Biret (baywet)
added a commit
that referenced
this pull request
Aug 13, 2026
* fix: do not use the install command from the OpenAPI extension (#7883) Signed-off-by: Vincent Biret <vibiret@microsoft.com> Co-authored-by: Vincent Biret <vibiret@microsoft.com> * fix: reject unsafe static_template.file references in plugin manifest generation (backport of #7892 / GHSA-4jwf-m4wg-8p66) Hand-ported against the pre-3.x extension shape (JsonObject/plain string) since upstream's fix depends on the typed ExtensionResponseSemanticsStaticTemplate model and an unrelated LoggerMessage refactor that don't exist on this line. * chore: bump Microsoft.OpenApi/YamlReader 2.0.0 -> 2.7.6 (GHSA-v5pm-xwqc-g5wc) Same-major patch bump, no Microsoft.AspNetCore.OpenApi conflict. Bisected an apparent DivergentResponseSchemaTests failure at this version to a pre-existing race on DivergentResponseSchema's shared static validStatusCodes HashSet under xUnit's parallel test execution, unrelated to this bump. * ci: adapt release pipeline to the v1.29.x source tree Pulls in main's current ci-build.yml (private feed routing, release idempotency guards) but corrects the three references that assume a newer source layout than what security/1-29-x actually has: - kiota.slnx -> kiota.sln (this branch predates the .sln to .slnx move) - publish TFM net10.0 -> net9.0 (kiota.csproj here targets net8.0/net9.0 only) - vscode/packages/npm-package/lib/runtime.json -> npm-package/runtime.json (the lib/ subfolder doesn't exist on this branch) No other changes. * chore: bump OpenTelemetry packages 1.13.1 -> 1.15.3 (GHSA-4625-4j76-fww9, GHSA-mr8r-92fq-pj8p, GHSA-q834-8qmm-v933) OpenTelemetry.Exporter.OpenTelemetryProtocol 1.13.1 has three known moderate-severity advisories (local blob injection via disk-retry temp path, unbounded grpc-status-details-bin parsing, unbounded OTLP HTTP response body reads). 1.15.3 is the first version patching all three. Bumps OpenTelemetry and OpenTelemetry.Extensions.Hosting to the same 1.15.3 to keep the OpenTelemetry core packages version-aligned, same pattern as the existing Microsoft.OpenApi same-major bump on this branch. Instrumentation.Http/Runtime are versioned independently upstream and unaffected by these advisories, left as-is. * fix: remove leftover git conflict markers from CHANGELOG.md Unresolved conflict markers from an earlier cherry-pick were accidentally committed. No content change beyond removing the markers and one stray blank line. * fix: escape literal injection across all language writers (backport of #7603 / GHSA-2hx3-vp6r-mg3f / CVE-2026-41134) Attacker-controlled OpenAPI description content (defaults, enum values, wire names, discriminator mappings, doc comments, deprecation info) emitted as string literals into generated client code could break out of the literal and inject arbitrary code in the generated client's language, across every writer (C#, Dart, Go, HTTP, Java, PHP, Python, Ruby, TypeScript). Adds/uses per-language literal sanitizers at all emission sites. Hand-resolved conflicts against upstream's fix (Go/Java/Python getter bodies, TypeScript composed-type deserializer) were pure indentation drift and one unrelated call to SortTypesByInheritance (a feature not present on this branch); kept this branch's existing formatting/logic and applied only the sanitizer calls. Dropped one unrelated test (GetFactoryMethodName_ReturnsCamelCase_WhenTypeIsAliasedButMethodIsNot) that depends on aliased-type factory-method resolution not present on this branch. * fix(writers/python): prevent code injection via x-ms-enum description newlines (backport of #7735 / GHSA-7f3j-j7jj-r3vr / CVE-2026-59862) PythonConventionService.RemoveInvalidDescriptionCharacters did not strip newline characters. 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. Dropped an unrelated dependency bump (StreamJsonRpc/Nerdbank.MessagePack/ System.CommandLine) that rode along in the upstream commit's "Secondary" changes, out of scope for this branch. Also dropped .github/instructions/writer-literal-security.instructions.md since it modifies a Copilot review-instructions file this branch doesn't have (added by an unrelated, un-backported commit, #7621). * fix(ruby): escape interpolation markers in generated string literals (backport of #7746 / GHSA-xg2h-5xr2-29jw / CVE-2026-59861) Prevent Ruby code injection in generated clients by escaping # in schema-derived values emitted into Ruby double-quoted literals. Dropped upstream's enum/datetime/date/time/guid-specific default-value formatting for backing-store/additional-data properties, an unrelated feature this branch doesn't have; kept the existing plain default-value assignment shape and applied only the Ruby sanitizer swap. * fix(csharp): strip newlines from doc comment text to prevent code injection (backport of #7831 / GHSA-3hrf-2gc2-mx32 / CVE-2026-59860) Newlines in an OpenAPI externalDocs.description (or other doc-comment sources) could break out of the C# /// doc comment and inject arbitrary code. Strips \r, \n, NEL, LS, PS from doc-comment text and replaces tabs with spaces before emission. * fix(php): escape $ in double-quoted string literals to prevent code injection (backport of #7863 / GHSA-jqwh-526h-c92j / CVE-2026-59859) PHP interpolates $var, ${...} and {$...} inside double-quoted strings. The shared SanitizeDoubleQuote helper does not escape $, so attacker-controlled OpenAPI input (descriptions, enum values, content types, discriminator/query names) emitted into PHP double-quoted literals could inject and execute arbitrary PHP/OS commands when the generated client runs. Adds SanitizePhpDoubleQuoteLiteral on PhpConventionService, mirroring the existing SanitizeDartDoubleQuoteLiteral, used at all PHP double-quote emission sites. * fix: sanitizes the client class and namespace names to avoid code injection (backport of #7884 / GHSA-4vv7-jj25-4gh6 / CVE-2026-59866) ClientClassName and ClientNamespaceName loaded from the x-ms-kiota-info OpenAPI extension (or settings) were used unsanitized in generated code and file paths, allowing an attacker-controlled OpenAPI description to inject arbitrary paths or code via those values. Test-file TestContext.Current.CancellationToken usages (xunit v3 API, unavailable on this branch's xunit 2.9.3) replaced with this branch's existing convention (new CancellationToken() / omitted). * fix: validate workspace output paths are sub-directories (backport of #7885 / GHSA-4rj6-vrwv-wr8m / CVE-2026-59863) A workspace configuration file (kiota-config.json) committed to a repo could set a client or plugin's outputPath to an absolute path or a path escaping the workspace directory. Running kiota against that workspace would then write generated files outside the intended project tree. Validates every client/plugin outputPath against the workspace root when loading the workspace configuration, rejecting rooted or escaping paths. * chore: reorganize CHANGELOG.md for the 1.29.x line Removed the [1.30.0] through [1.32.3] version headings: they describe main's release history, not this branch's, and their content either doesn't apply here (unrelated bug fixes never backported to this line) or duplicates entries already listed under [Unreleased] for the CVE fixes actually ported here. This branch will release as a 1.29.x patch, so nothing newer than [1.29.0] should appear as if released on this line. * style: apply dotnet format across the repo Brings the tree in line with the current formatting ruleset, mostly missing blank lines after namespace declarations and using blocks. No behavioral changes. Fixes the "Check formatting" CI gate, which was failing repo-wide on this branch's baseline (v1.29.0 predates the current formatter/ruleset) independent of the security fixes. * feat: add an allow list parameter for external references resolution (backport of #7888 / GHSA-rg4h-fpcp-2qm8 / CVE-2026-59867) External $ref resolution during OpenAPI parsing was previously unrestricted, allowing a malicious or compromised OpenAPI description to trigger SSRF or local/remote file inclusion via $ref targets kiota would follow and load. Adds AllowedExternalOriginsStreamLoader and a new --allowed-external-origins CLI parameter (default: deny all external refs) to restrict resolution to an explicit allow list ('*', full URIs, URI patterns, full/relative paths, path patterns). Hand-ported the CLI wiring (13 command/handler files) against this branch's System.CommandLine 2.0.0-beta4 API (InvocationContext / GetValueForOption), since upstream's commit rides on main's later System.CommandLine 2.0.8 API (ParseResult / GetValue) migration, which this branch doesn't have. GetAllowedExternalOriginsOption similarly rewritten against the old Option<T> constructor API (no DefaultValueFactory/HelpName init-only properties on this version). Core library changes (AllowedExternalOriginsStreamLoader, OpenApiDocumentDownloadService) applied with no changes needed. Known gap: one test, AllowedExternalOriginsLoadMatchingReferences, fails locally with a "circular reference detected" error from Microsoft.OpenApi's schema resolver. This appears to be a behavior difference between Microsoft.OpenApi 2.7.6 (this branch, same-major bump from 2.0.0) and whatever 3.x version upstream's test was written against, triggered by the test's mock HTTP response reusing the same component name ("Pet") as the referencing document. The AllowedExternalOriginsStreamLoader class itself (the actual security-relevant code) and its 8 other dedicated tests all pass; this looks like a test-fixture/library-version interaction, not a flaw in the allow-list logic. Flagging rather than silently fixing the fixture, since I'm not confident yet whether adjusting the mock masks a real resolution-order issue or not. Unpushed: prepared in case this fix is approved, per discussion on the PR. Not yet pushed to keep the reviewed 7-commit set untouched pending review. * Fix redundant pattern-matching precedence flagged by CS9336 `is not A or B` parses as `(is not A) or B`, not the intended `is not (A or B)`. Parenthesize per baywet's review comment on #8038. * ci: fix stale msgraph-sdk-powershell dev-branch URL to main The dev branch no longer exists; Mail.yml (and the other openApiDocs fixtures referenced in launch.json) moved to main, causing 404s in test_and_generate_binaries. Same fix upstream applied in beb99d9 (#7364). * ci: pin googlemaps openapi spec to tag, dist/ removed from main googlemaps/openapi-specification dropped the dist/ folder from main, causing 404s on download-openapi-specs. Same fix main applied in 0361427 (#7774): pin to refs/tags/v1.22.5 where dist/ still exists. * ci: replace apisguru proxy for api.github.com, sync config.json keys apisguru::github.com:api.github.com routes through the APIs.guru proxy, same class of flakiness main moved off in 0361427 (#7774). Swapped to the direct rest-api-description URL main uses. Also fixes it/config.json: its suppression/mock-folder lookup is keyed by the exact matrix string, so the googlemaps tag-pin from the prior commit and this api.github.com swap both needed their config.json keys updated to match or the suppressions would silently stop applying. Verified download-openapi-specs.ps1 against the new URL locally. * fix: escape backslashes in C# property default value string literals WriteConstructorBody hand-rolled quote/newline escaping for string default values, missing backslashes. A default like "\z|[^0-9A-Za-z]" (regex pattern, e.g. GitHub's rest-api-description secret-scanning custom pattern schema) round-tripped its raw backslashes straight into the generated literal, producing invalid C# (CS1009: Unrecognized escape sequence). Switched to SanitizeQuotedStringLiteral(), the same literal-escaping helper already used elsewhere in this file and added across writers by 6dbee0f's literal-injection CVE fix, which already escapes backslashes correctly. Same fix main applied in ee39d2a (#7414), scoped down to just the string-escaping change since that commit's DateTime/numeric/boolean default-value handling is an unrelated feature, not a bug on this branch's own baseline. Reproduced and verified locally: generated + compiled the C# client for GitHub's rest-api-description spec (the spec that surfaced this), 0 errors after the fix vs 2x CS1009 before. * ci: match main's IT suppressions for dart/typescript/java WithPath bug The github/rest-api-description spec's /repos/{owner}/{repo}/contents/{path} GET response is a union type that generates a "WithPath" model class main's generator doesn't correctly emit (missing symbol in java, missing file in dart, missing deserializer registration in typescript). This is an open, unfixed kiota generator bug (main doesn't generate correct code for it either) that main works around by suppressing dart/typescript entirely for this spec (#7821, #7862) and excluding just that one endpoint for java (#7829), rather than a real fix to backport. Added the matching per-language Suppressions/ExcludePatterns entries to it/config.json, and backported the "Language" field filtering for ExcludePatterns from main's 0361427 (#7774) to get-additional-arguments.ps1 -- our version applied every ExcludePattern to every language regardless of a Language field, so the java-only exclude would otherwise have leaked to every language's generation command. Verified locally: get-additional-arguments.ps1 emits the contents/path exclude only for java, not csharp; get-is-suppressed.ps1 suppresses dart/typescript but not java/csharp/python for this description. * fix(python): rename types that collide once snake cased Python modules are named after the snake cased type name, so the inline schema class CodeScanningVariantAnalysis_status and the component schema class CodeScanningVariantAnalysisStatus both resolve to the same file, code_scanning_variant_analysis_status.py. One silently overwrote the other and the losing type's importers kept referencing a symbol that no longer existed, which is the mypy "has no attribute ... [attr-defined]" failure on the python integration test. Which of the two wins is nondeterministic, which is why CI reported the error in both directions (CodeScanningVariantAnalysis_status missing in one module, RepositoryRulesetConditions missing in the other). Adds a DisambiguateSnakeCasedNames pass to PythonRefiner, run right after CorrectCommonNames so it lands before imports are materialized. Sibling classes/enums are grouped by snake cased name and later collisions get a trailing underscore, ordered with StringComparer.Ordinal for determinism. Renaming the type is sufficient because CodeType.Name is computed from TypeDefinition.Name, so the file name, import path, import symbol and every type reference follow automatically; no writer changes. No upstream fix to backport: main has the same latent bug, its PythonRefiner and PythonPathSegmenter differ from this branch only by the unrelated ShortenOversizedNamespaceSegments work. Verified against the github/rest-api-description description used by the integration test. An AST scan resolving every relative import against the symbols actually present in the target module goes from 3 broken pairs to 0 of 13302. Both previously lost types are emitted again. The added test fails without the fix (Model_status vs Model_status_). Ruby and Dart segment file names the same way and so share this latent bug; left alone here to keep the change scoped to the reported failure. * fix: guard against circular discriminator resolution A discriminator mapping can point at a composed (oneOf/anyOf) schema whose own discriminator maps back to an ancestor, so GetDiscriminatorMappings calls GetCodeTypeForMapping calls CreateModelDeclarations calls CreateComposedModelDeclaration calls GetDiscriminatorMappings with nothing to break the loop. There is no cycle detection anywhere on that path. Tracks the component schemas currently being resolved as discriminator targets, pushed before recursing and popped in a finally, so only genuine ancestor cycles are cut while the same component referenced from sibling branches still resolves. Returning null is the already established "skip this mapping" path, GetDiscriminatorMappings filters nulls out. Prerequisite for moving the Microsoft.OpenApi pin to 2.12.0: the schema shapes 2.12.0 produces close this loop and overflow the stack, aborting the whole test run. Bisected to confirm, 2.7.6, 2.10.0 and 2.11.0 are all fine and 2.12.0 is where it starts. main's ThreadLocal modelCreationDepth guard does not cover this, it wraps CreatePropertiesForModelClass and this cycle never passes through that method. Behaviour preserving on the current pin: generating the C# client for github/rest-api-description before and after gives byte identical output across all 3704 files including the log, even though the guard trips 108 times on that description. Logged at debug rather than warning for that reason, tripping here is normal rather than anomalous. * fix: bump Microsoft.OpenApi to 2.11.0 so external ref allow list works AllowedExternalOriginsLoadMatchingReferences failed on 2.7.6 with "Circular reference detected while resolving schema" thrown from OpenApiWorkspace.ResolveSubSchema while RegisterComponents resolved the root document's component-level external $ref. The deny path of the CVE-2026-59867 allow list was fine, but the allow path could not actually load a permitted external reference on the version 1.29.0 shipped. 2.11.0 fixes that resolution bug and is the last 2.x before the breaking changes: 2.12.0 folds anyOf: [string, integer] into a single flags-enum Type and obsoletes IOpenApiSchema.Example, which is the upgrade work that belongs on main rather than on a security branch. Full suite on 2.11.0: 1967 passed, 0 failed, 2 skipped (was 1 failure on 2.7.6). The anyOf tests pass unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka * ci: exclude the WithPath endpoint for typescript too Main excludes /repos/{owner}/{repo}/contents/{path}#GET for both java and typescript; the previous backport only carried the java entry, so typescript idempotency against the github description still crashed generation with "Function deserializeIntoWithPath not found in namespace ApiSdk.models". The integration suppression did not cover it because idempotency generates the client before any comparison. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka * ci: follow the github spec rename of byOrgId to byOwnerId The github/rest-api-description spec is tracked from its main branch, and the /repos/{owner}/{repo} path parameter is now owner rather than org, so the generated ReposRequestBuilder exposes byOwnerId. Matches the same change on kiota main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka * fix: move to Microsoft.OpenApi 2.12.0 and narrow folded union types 2.12.0 is the floor for the remaining security fixes, so 2.11.0 is not an option after all. Two things change on 2.12.0. IOpenApiSchema.Example becomes obsolete in favour of Examples, so the schema copy in PluginsGenerationService copies Examples now. The reader also folds a union of primitive types, anyOf: [string, integer], into a single Type carrying both flags and clears AnyOf. SelectFirstAnyOneOfVisitor therefore never sees a union, and the generated plugin description kept "Integer | String" where it used to narrow to "String". NarrowMultipleTypes restores a single type. Worth flagging: JsonSchemaType is a flags enum, so the order the types were authored in is gone by the time we see it, and "take the first entry" cannot be honoured literally any more. The rule is String when present, since every JSON scalar round trips through it, otherwise the lowest flag so the result stays deterministic. For anyOf: [string, integer] that reproduces the previous output exactly. Full suite on 2.12.0: 1967 passed, 0 failed, 2 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka * docs: changelog for the OpenApi 2.12.0 bump and union narrowing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka * docs: reference the advisories and cover the missing security fixes The changelog described the fixes but never named them, so nobody reading it could line an entry up with an advisory. Each security entry now carries its GHSA link and CVE. Two fixes on this branch had no entry at all: - the static_template.file path escape rejection (#7892 / GHSA-4jwf-m4wg-8p66) - the OpenTelemetry 1.13.1 to 1.15.3 bump (GHSA-4625-4j76-fww9, GHSA-mr8r-92fq-pj8p, GHSA-q834-8qmm-v933) Also folded GHSA-v5pm-xwqc-g5wc into the Microsoft.OpenApi entry, since that bump started as a fix for it and has since moved on to 2.12.0. All of these advisories were published in July 2026 or earlier, so naming them here discloses nothing new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka * fix: prefer the numeric type when a numeric union carries a format Per review feedback: int | string is what System.Text.Json's JsonNumberHandling.AllowReadingFromString advertises, the ASP.NET Core default, so the pair should resolve to the numeric type rather than collapsing to a string. The format is correlated first, and a string only wins when there is no numeric format to go on. This matches the rule GetPrimitiveType already applies for clients on main, backported here for the plugin path. Suite: 1967 passed, 0 failed, 2 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka * chore: prepare the 1.29.1 patch release Ran scripts/bump-version.ps1 -kind patch and scripts/prepare-release.ps1 as requested, which moves the unreleased security entries under 1.29.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUifZksSV3pi2zWxvs29ka --------- Signed-off-by: Vincent Biret <vibiret@microsoft.com> Co-authored-by: Gavin Barron <gavinbarron@microsoft.com> Co-authored-by: Vincent Biret <vibiret@microsoft.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.