Skip to content

chore: security-only backport of critical CVE fixes to v1.29.x - #8038

Merged
Vincent Biret (baywet) merged 32 commits into
microsoft:security/1-29-xfrom
EelcoLos:security-backport-1.29.x
Aug 13, 2026
Merged

chore: security-only backport of critical CVE fixes to v1.29.x#8038
Vincent Biret (baywet) merged 32 commits into
microsoft:security/1-29-xfrom
EelcoLos:security-backport-1.29.x

Conversation

@EelcoLos

@EelcoLos Eelco Los (EelcoLos) commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #7999. Security-only backport of fixes for Kiota.Builder CVEs onto v1.29.0, targeting security/1-29-x. Nothing on the Microsoft.OpenApi 3.x line, so it doesn't conflict with Microsoft.AspNetCore.OpenApi still being pinned to 2.x.

Consumers stuck on 1.29.x because they can't take the OpenAPI v3 major bump yet (see #7999 and the FastEndpoints side of this, FastEndpoints/FastEndpoints#1101) currently have no way to clear NU1903/NuGetAudit on Kiota.Builder without pulling in an incompatible major. This branch is a proof that a 1.29.x-based patch is buildable and passes the full test suite, in case the team decides a backport is worth shipping.

This started as a 3-CVE backport; querying the advisory database turned up 8 more disclosed CVEs affecting Kiota.Builder, all confirmed present on v1.29.0 by checking the actual pre-fix code, not just the advisory's version range. All 11 are now ported.

What's in it

Dependency/CI:

  • 8cc28ced9: GHSA-v5pm-xwqc-g5wc, bump Microsoft.OpenApi/Microsoft.OpenApi.YamlReader 2.0.0 to 2.7.6. Same major, first patched version in the 2.x line. Blocked from release: needs another bump once chore(support/v2): release 2.12.0 OpenAPI.NET#2978 ships in a published NuGet version (not merged yet as of this writing, being watched).
  • bd18b5d5c: adapts .azure-pipelines/ci-build.yml to this branch's source tree (kiota.sln not .slnx, net9.0 not net10.0, npm-package/runtime.json path), pulling in main's private-feed routing and release idempotency guards otherwise unchanged.
  • d604dbd70: GHSA-4625-4j76-fww9, GHSA-mr8r-92fq-pj8p, GHSA-q834-8qmm-v933, bump OpenTelemetry/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Extensions.Hosting 1.13.1 to 1.15.3. Unrelated to the Kiota CVEs, but was failing CI (NU1902) once the pipeline actually ran.
  • f8660d8cb: removes leftover git conflict markers accidentally committed into CHANGELOG.md in an earlier commit on this branch.
  • 37b3f04e7: reorganizes CHANGELOG.md, moving all new entries under [Unreleased] and removing the [1.30.0]-[1.32.3] headings that described main's release history rather than this branch's.
  • b661129df: dotnet format across the whole tree (250 files, whitespace/blank-line only, no behavioral changes). dotnet format --verify-no-changes is clean now.
  • b61800293: fixes two is not A or B pattern-matching precedence bugs (parses as (is not A) or B, not the intended is not (A or B)) that a newer Roslyn flags as CS9336-redundant. Build no longer needs -p:NoWarn=CS9336.

CVE fixes, security-only:

Conflicts against most upstream fixes were pure indentation drift or unrelated features this branch doesn't have (an intervening default-value formatting feature in Ruby, an AllowedExternalOriginsOption-adjacent ServiceProvider DI refactor, etc.), not semantic divergence. Details are in each commit message.

Testing

  • Full test suite: 1955/1958 passed locally, 2 skipped, 1 known failure: AllowedExternalOriginsLoadMatchingReferences hits a Microsoft.OpenApi 2.7.6 circular-reference resolution quirk in the test's mock fixture; the actual AllowedExternalOriginsStreamLoader security logic and its other 8 dedicated tests all pass.
  • CI failures on this PR are all pre-existing/environmental, not caused by any commit here: the same AllowedExternalOriginsLoadMatchingReferences gap above, an SSLConnection/network blip on a live-endpoint test (KiotaSearcherTests.GetsMicrosoftGraphBetaAsync), and 404s on external fixture URLs that moved/vanished (googlemaps/openapi-specification, msgraph-sdk-powershell for the VS Code extension's jest suite, unrelated to any C# changes here).
  • dotnet format --verify-no-changes and the CS9336 Roslyn warning, both previously flagged as pre-existing drift, are now fixed in b661129df/b61800293 respectively.

Open question

Is a 1.29.x security-only patch line something the team would take, or is Microsoft.OpenApi 3.x support in Microsoft.AspNetCore.OpenApi close enough that this isn't worth maintaining in parallel? Asked the same thing on #7999.

Per Vincent Biret (@baywet)'s review: before merging/releasing, we need microsoft/OpenAPI.NET#2978 to ship in a published NuGet version so Microsoft.OpenApi/Microsoft.OpenApi.YamlReader can be bumped again. Watching for that.

…soft#7883)

Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
… generation (backport of microsoft#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.
…qc-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.
@EelcoLos
Eelco Los (EelcoLos) requested a review from a team as a code owner August 11, 2026 06:45
@EelcoLos

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@EelcoLos
Eelco Los (EelcoLos) marked this pull request as draft August 11, 2026 06:48
@EelcoLos

Copy link
Copy Markdown
Contributor Author

Converting to draft. Marking this because the diff here is against v1.29.0, not main (which is already on Microsoft.OpenApi 3.x), so the conflicts CI is flagging aren't a few lines to resolve, they're the whole 2.x-to-3.x migration. There's no maintenance branch for the 1.29.x line to target instead.

This PR is meant as a concrete, working reference (branch, commits, test results) for the question asked on #7999, not something to merge into main as-is. If the team decides a 1.29.x security line is worth maintaining, it'd need its own branch to target and I'm happy to rebase onto that. Leaving as draft until there's a response there.

@baywet

Copy link
Copy Markdown
Member

Hi Eelco Los (@EelcoLos)
Thanks for the contribution!

Can you please target the branch I just created security/1-29-x instead (based off the 1.29.0 tag) ? Once we merge in there, we should be able to create a servicing tag.

What would be interesting to do is compare the azure devops pipeline, to ensure releasing goes smoothly.

@EelcoLos
Eelco Los (EelcoLos) changed the base branch from main to security/1-29-x August 11, 2026 11:25
@EelcoLos
Eelco Los (EelcoLos) marked this pull request as ready for review August 11, 2026 11:25
@EelcoLos

Copy link
Copy Markdown
Contributor Author

Retargeted to security/1-29-x and marked ready, thanks for cutting that. CI now shows clean/mergeable against it.

Happy to help compare against the Azure DevOps release pipeline if useful, just point me at what to look at.

@baywet

Copy link
Copy Markdown
Member

https://github.com/microsoft/kiota/tree/main/.azure-pipelines is what we want to compare here

@EelcoLos

Copy link
Copy Markdown
Contributor Author

Compared .azure-pipelines/ci-build.yml on main against the v1.29.0 source tree this branch is built from. If the pipeline definition on main is what runs against a tag cut from security/1-29-x, a couple of things look like they'd break the build outright, not just warn:

  • Solution file: the pipeline's build/test/publish steps reference kiota.slnx. v1.29.0 still has kiota.sln (no .slnx in the tree), so those DotNetCoreCLI@2 steps would fail to find the project.
  • Target framework for the single-file publish: the pipeline publishes with -f net10.0 (previously net9.0). v1.29.0's src/kiota/kiota.csproj only targets net8.0;net9.0, no net10.0 TFM, so that publish step would fail too.
  • VS Code runtime.json path: the pipeline now writes to vscode/packages/npm-package/lib/runtime.json, but on v1.29.0 the file lives at vscode/packages/npm-package/runtime.json (no lib/ folder), so update-vscode-releases.ps1 would either fail or silently write to the wrong path depending on how it handles a missing target.

Smaller/likely-fine items that are still worth knowing about:

  • The pipeline now does git rm -r it/ right after checkout (integration-test fixtures). v1.29.0 still has it/, so this step would just delete it at build time, that's probably harmless for a servicing build but means those fixtures never get exercised.
  • New Azure Artifacts private-feed routing (NuGetAuthenticate, .npmrc/nuget.config generation, GitHub token extraction for rate limits) is additive and should work against an older source tree without changes, since it only affects how packages are restored, not what's being restored.
  • Idempotency guards for re-triggered releases (skip if the GitHub release/NuGet/npm version already exists) are also additive and unrelated to the source tree's shape.

So: as-is, running main's current pipeline against security/1-29-x would likely fail at the build/test step (solution file mismatch) before it even gets to publishing. If the pipeline used for a servicing tag is meant to be whatever's on main at release time, either the branch needs a kiota.slnx (and the net10.0 TFM / runtime.json path resolved), or the servicing release needs to run off a pinned/older copy of the pipeline that matches v1.29.0's shape. Let me know which direction makes more sense and I can help make the change on my end.

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.
@EelcoLos

Copy link
Copy Markdown
Contributor Author

Pushed a commit that adapts the release pipeline for this branch: pulled in main's current ci-build.yml (private feed routing, release idempotency guards) and corrected the three places that assumed a newer source layout than what v1.29.0 actually has:

  • kiota.slnx -> kiota.sln
  • publish TFM net10.0 -> net9.0 (this branch's kiota.csproj only targets net8.0/net9.0)
  • vscode/packages/npm-package/lib/runtime.json -> npm-package/runtime.json

No other changes. git rm -r it/ at checkout still applies since those fixtures exist on this branch too, same as on main, just means they don't get exercised in this pipeline run, consistent with how main runs it.

@baywet

Copy link
Copy Markdown
Member

Thanks! Now we probably need a changelog entry under changed unreleased, and then to run this script for patch https://github.com/microsoft/kiota/blob/main/scripts/bump-version.ps1
And then that script to align things https://github.com/microsoft/kiota/blob/main/scripts/prepare-release.ps1

@EelcoLos

Copy link
Copy Markdown
Contributor Author

One more thing worth surfacing before this goes further: querying the advisory database for Microsoft.OpenApi.Kiota.Builder (not just the two CVEs this PR already covers) turns up 8 more disclosed CVEs that also affect v1.29.0, all HIGH severity:

I checked each against the actual v1.29.0 source rather than trusting the advisory's version range at face value, all 8 are genuinely present (the pre-fix vulnerable code is there verbatim in most cases).

I also test-applied each upstream fix commit against v1.29.0 to gauge feasibility: 7 of the 8 cherry-pick cleanly or with only trivial one-line conflicts. The SSRF fix (#7888) is the exception, its core library change applies clean, but the CLI wiring conflicts because main upgraded System.CommandLine to a new API somewhere after this branch point, so that one needs hand-porting the new option through the older InvocationContext/GetValueForOption pattern across ~13 command handler files. Still doable, just not a clean cherry-pick.

Not proposing to add all of these to this PR unprompted, wanted to flag it before this goes further since it changes what "security backport of v1.29.0" actually means in practice. Happy to add some or all of these as follow-up commits here if that is useful, just say the word.

@baywet

Copy link
Copy Markdown
Member

Thank you for the additional information. I think there's only value in releasing a security patch if all the known security issues are addressed, addressing only a subset would lead to a bizarre security posture.

Also I want to outline this as the goal here (why are we patching an older version)

Consumers stuck on 1.29.x because they can't take the OpenAPI v3 major bump yet

That has the potential to unblock quite a few consumers from ASP.NET and other horizons.

Let's start by porting the trivial patches onto this branch, and pause before the more complex ones, so I get time to do an initial review pass. If you can keep a clean/explicit commit history for those patches (i.e. one commit per security fix), that'd help as well in the review process.

@baywet

Copy link
Copy Markdown
Member

Additionally the CI is also reporting this

/home/runner/work/kiota/kiota/src/kiota/kiota.csproj : error NU1902: Warning As Error: Package 'OpenTelemetry.Exporter.OpenTelemetryProtocol' 1.13.1 has a known moderate severity vulnerability, GHSA-4625-4j76-fww9 [/home/runner/work/kiota/kiota/kiota.sln]

, 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.
Unresolved conflict markers from an earlier cherry-pick were
accidentally committed. No content change beyond removing the markers
and one stray blank line.
…microsoft#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.
… newlines (backport of microsoft#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, microsoft#7621).
…(backport of microsoft#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.
…ection (backport of microsoft#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.
…njection (backport of microsoft#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.
…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).
…icrosoft#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.
@EelcoLos

Eelco Los (EelcoLos) commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Pushed the 7 trivial CVE fixes, one commit each as requested:

Also included d604dbd70: bumped OpenTelemetry.Exporter.OpenTelemetryProtocol (and the two other lockstep OpenTelemetry packages) 1.13.1 -> 1.15.3, that's what was behind the NU1902 failure you flagged, three moderate advisories on 1.13.1, 1.15.3 is the first version patching all of them.

Each commit builds clean and the full test suite passes locally (net8.0/net9.0, 1949/1952 relevant tests, the rest are the pre-existing flaky/network-dependent tests already noted in the PR description). Conflicts against upstream's fixes were mostly indentation drift or unrelated features this branch doesn't have (noted per-commit in each message); details on request.

Pausing here before the SSRF/allow-list fix (#7888) as discussed, that's the one needing the CLI hand-port due to the System.CommandLine API change.

On the changelog/version-bump ask: changelog entries are in under Unreleased > Changed (one per commit above). Holding off on bump-version.ps1/prepare-release.ps1 until after your review pass, in case scope changes.

@EelcoLos

Copy link
Copy Markdown
Contributor Author

Thanks for approving the run, that gave real signal. CI is red, but for reasons unrelated to the 7 commits: both failures are pre-existing on v1.29.0 itself, reproducible on files none of these commits touch.

  1. dotnet-build / Check formatting fails repo-wide (exit code 2, hundreds of files, e.g. src/kiota/Authentication/GitHub/DeviceCode/AccessTokenProvider.cs, src/Kiota.Builder/CodeDOM/CodeClass.cs). I confirmed this reproduces identically against the untouched v1.29.0 tag: the formatting tool/ruleset has moved on since this tag was cut, and the original source was never re-formatted to match. Not something introduced here.
  2. CS9336 ("The pattern is redundant") fires in test-release's publish step on every platform (macOS, Windows, Windows ARM, Linux ARM), same two lines each time: CodeDOM/CodeClass.cs:181 and Writers/CSharp/CodeMethodWriter.cs:563. This is the same issue already noted in the PR description, a newer Roslyn than what v1.29.0 was built against flags this pattern. -p:NoWarn=CS9336 was needed locally for the same reason.

The good news: no NU1902 errors anywhere in this run, so the OpenTelemetry bump fixed the restore failure cleanly.

Both remaining issues are tooling/vintage-drift on the branch baseline, not regressions from any of the 7 security commits. Happy to fix either or both (re-format the whole tree to satisfy the current formatter, and/or work around or fix the CS9336 sites) if that's useful, just didn't want to bundle unrelated formatting churn into the security commits without checking first.

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 (microsoft#7821, microsoft#7862) and excluding
just that one endpoint for java (microsoft#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 (microsoft#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.
@EelcoLos

This comment was marked as outdated.

@EelcoLos

This comment was marked as outdated.

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.
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.
@EelcoLos

Eelco Los (EelcoLos) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Both items from my last two comments turned out to be fixable. Pushed 64fc4d686 and 5bed3f52c.

1. Python mypy failure. Not a naming divergence as I first thought. CodeScanningVariantAnalysisStatus (component) and CodeScanningVariantAnalysis_status (inline) both snake case to the same .py file, so one silently overwrites the other and the loser's importers reference a symbol that is gone. Which one wins is nondeterministic, hence CI seeing it in both directions. Fixed with a DisambiguateSnakeCasedNames pass in PythonRefiner. Broken relative imports across the generated client go from 3 to 0 of 13302. main has the same latent bug, so nothing to backport.

2. Stack overflow on 2.12.0. It is a kiota bug, not an OpenAPI.NET one: GetDiscriminatorMappings to GetCodeTypeForMapping to CreateModelDeclarations to CreateComposedModelDeclaration and back, with no cycle detection anywhere on that path. 2.7.6 never produced a shape that closes the loop, 2.12.0 does. Fixed by tracking the components currently being resolved. Output is byte identical on the current pin across all 3704 generated files, so it is safe to carry now.

3. Still blocking the bump, and it needs your call Vincent Biret (@baywet) . 2.12.0's V3 reader folds anyOf: [string, integer] into Type = Integer|String and nulls AnyOf, so SelectFirstAnyOneOfVisitor never fires and the output keeps both types instead of narrowing to string. JsonSchemaType is a flags enum, so authoring order is gone and the old behaviour cannot be restored, only re decided. That is a deliberate output change, so I did not want to pick on a security only backport.

Relevant to you as maintainer of both repos: that fold shipped in 2.12.0 and 3.10.0 together, main pins 3.9.0, and main's copy of the failing test (GeneratesManifestAndCleansUpInputDescriptionAsync) is byte for byte identical to this branch's. So main hits the same failure on its next bump.

Pin is still 2.7.6. Tell me which way you want the anyOf question resolved and I will do that and move the pin in one change.

Bisect table and evidence

Each row is a clean pin, restore, build and full Kiota.Builder.Tests run:

Microsoft.OpenApi Result
2.7.6 (current pin) 1965 passed, 1 failed (the known AllowedExternalOriginsLoadMatchingReferences)
2.10.0 1966 passed, 0 failed
2.11.0 1966 passed, 0 failed
2.12.0 stack overflow, test host crashes, run aborts

2.10.0 and 2.11.0 both also clear the AllowedExternalOriginsLoadMatchingReferences gap this PR description documents.

Python fix: verified with an AST scan resolving every relative import against the symbols actually present in the target module, the same class of check as mypy's attr-defined, over the whole generated client. Added test is genuinely red without the fix (Model_status vs Model_status_). Ruby and Dart segment file names the same way so they share the latent bug; left alone to keep this scoped.

Cycle guard: main's ThreadLocal<int> modelCreationDepth does not cover this, it wraps CreatePropertiesForModelClass and this cycle never goes through there. The guard trips 108 times on github/rest-api-description while changing nothing, which is why it logs at debug rather than warning.

The KiotaSearcherTests failures I see locally are live network flakes, they vary run to run and fail on a clean 2.7.6 tree too.

@baywet

Copy link
Copy Markdown
Member

Eelco Los (@EelcoLos) Thank you for the additional information.

So if I understand things correctly, both 3.10 and 2.12 reveal a new bug that we don't have a fix for at all in any branch?

If my understanding is correct, it'd be interesting to start another pull request to main to address the upgrade + bugfix, and then backport that here as a way to unblock this release. What do you think?

Eelco Los (EelcoLos) and others added 3 commits August 13, 2026 14:02
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
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
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
@EelcoLos

Eelco Los (EelcoLos) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Your understanding is right about the anyOf fold: it lands in 2.12.0 and 3.10.0 at the same time, main is still on 3.9.0, so no branch has a fix and main will hit the same wall on its next bump.

Good news though, this release does not have to wait for that. I found we never actually needed 2.12: 2.11.0 is enough, and it is the last version before the fold.

CI ran overnight and surfaced a real bug I had missed. AllowedExternalOriginsLoadMatchingReferences failed with:

System.InvalidOperationException : Circular reference detected while resolving schema:
  https://contoso.com/schemas/pet.yaml#/components/schemas/Pet
   at Microsoft.OpenApi.OpenApiWorkspace.ResolveSubSchema(...)
   at Microsoft.OpenApi.OpenApiWorkspace.RegisterComponents(OpenApiDocument document)

That is not our code, it is OpenAPI.NET 2.7.6 failing to resolve a component level external $ref during workspace registration. The practical impact is that the deny half of the CVE-2026-59867 allow list worked, but the allow half could not actually load a permitted external reference on the version 1.29.0 ships. So the security fix was only half functional.

2.11.0 fixes that resolution bug, and it predates both breaking changes in 2.12.0 (the anyOf fold, and IOpenApiSchema.Example becoming obsolete, which does not even compile against our source). Full suite on 2.11.0: 1967 passed, 0 failed, 2 skipped, with the anyOf tests passing unchanged. So the pin moves 2.7.6 to 2.11.0 here, and the 2.12/3.10 upgrade plus the anyOf decision stays a main concern with no release pressure behind it.

I also fixed the two integration failures, both pre-existing and unrelated to the security changes:

  1. idempotency (typescript, github) crashed generating with Function deserializeIntoWithPath not found in namespace ApiSdk.models. Main excludes /repos/{owner}/{repo}/contents/{path}#GET for typescript as well as java ([TypeScript] Error "Function deserializeIntoWithPath not found" for GITHUB.COM api #7803), my earlier backport only carried the java entry. The integration suppression does not help here because idempotency generates before it compares.
  2. integration (java, github) failed on cannot find symbol: byOrgId. The github description now names that path parameter owner rather than org, so the generated builder exposes byOwnerId. Same one line change main already carries.

Pushed as three separate commits.

@baywet

Copy link
Copy Markdown
Member

While I can't share the specifics just yet, we'll need to be on version 2.12.0 or above to address all the security issues.

There's has been conflicting security priorities, and issues with coordination on OpenAPI.net which lead to delays in publishing patches and other information, but I'm working to get all of that resolved by today, or tomorrow at the latest.

@EelcoLos

Copy link
Copy Markdown
Contributor Author

Follow up on my previous comment, and a correction to it. I opened the main PR you asked for: #8050.

While preparing it I checked the anyOf fold against the 3.x line properly instead of assuming it behaved like 2.12, and it does not affect 3.10.0. I had that part wrong.

GeneratesManifestAndCleansUpInputDescriptionAsync asserts AnyOf.Count == 2 on the freshly loaded source document. On main at 3.10.0 that still passes. On this branch pinned to 2.12.0 the same assertion throws a NullReferenceException because AnyOf is null, against a byte identical fixture. So the fold is specific to 2.12.0, not something the 3.x line inherited.

That makes the whole thing much smaller than either of us thought:

  • main (build(deps): bump Microsoft.OpenApi to 3.10.0 #8050): the upgrade to 3.10.0 is two lines. The bump itself, plus IOpenApiSchema.Example becoming obsolete in favour of Examples in the schema copy in PluginsGenerationService. Full suite green, format clean. There is no bug to fix and no anyOf decision to make.
  • this branch: 2.11.0, which fixes the external reference resolution bug that broke the CVE-2026-59867 allow list and predates the fold entirely. Full suite green.

So there is nothing here that needs a fix invented in one branch and carried to the other. Sorry for pointing you at a decision that turned out not to exist.

@baywet

Copy link
Copy Markdown
Member

Thank you for the additional information.

So we should be safe to upgrade this branch to 2.12 based on your latest analysis? let's do so then please.

@EelcoLos

Copy link
Copy Markdown
Contributor Author

Thank you for the additional information.

So we should be safe to upgrade this branch to 2.12 based on your latest analysis? let's do so then please.

I'm looking into it as we speak

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
@EelcoLos

Copy link
Copy Markdown
Contributor Author

Done, pushed. One correction before you read too much comfort into my last comment though: 2.12 is not free for this branch. My "it does not affect 3.10" finding was about main, and I should have been clearer that the reverse is what matters here. The fold does bite us on 2.12, so I fixed it rather than dodged it.

Two things change on 2.12.0.

IOpenApiSchema.Example becomes obsolete in favour of Examples, which under warnaserror is a build break. Same one line fix as #8050.

The reader folds a primitive union. I probed it directly:

input:  anyOf: [ {type: string}, {type: integer} ]
2.12.0: AnyOf = null, Type = Integer | String
3.10.0: AnyOf = 2 entries

So SelectFirstAnyOneOfVisitor never sees a union, and the emitted plugin description kept Integer | String where it previously narrowed to String. That is a real output change, not just a test assertion, which is why I want it on your radar.

I added NarrowMultipleTypes to the same visitor to collapse a multi flag Type back down to one. For anyOf: [string, integer] the output is byte identical to before.

The bit that is genuinely your call: JsonSchemaType is a flags enum, so by the time we see the folded value the authoring order is gone. "Take the first entry" cannot be honoured literally any more, it has to become a rule. I picked String when present, since every JSON scalar round trips through it, and otherwise the lowest flag so the result stays deterministic. That reproduces today's behaviour for the common case, but for something like anyOf: [integer, string] we now emit String where 1.29.0 would have emitted Integer. If you would rather have a different precedence, or would rather push OpenAPI.NET to stop folding on read in a 2.12.x so the 2.x line matches 3.10, say the word and I will follow that instead.

Full suite on 2.12.0: 1967 passed, 0 failed, 2 skipped, dotnet format clean.

Also added the changelog entry CI wanted on #8050, thanks for the quick approve there.

Eelco Los (EelcoLos) and others added 2 commits August 13, 2026 14:48
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 (microsoft#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
@baywet

Copy link
Copy Markdown
Member

reproduces today's behaviour for the common case, but for something like anyOf: [integer, string] we now emit String where 1.29.0 would have emitted Integer

This is essentially special casing because in JSON a number value can be represented as a JSON number or as a JSON string. That behaviour comes from the fact that some older parsers would attempt to parse numbers as int32/64 or float32/64 and get out of bound exceptions when the number is larger than that.

In practical sense if we see type int | string, we should consider that as a numeric type, and attempt to correlate with the format BEFORE we fall back to a string type. And if special casing is required in the code to avoid a regression, let's implement that.

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
@EelcoLos

Copy link
Copy Markdown
Contributor Author

Two things.

First, a correction. My earlier claim here that the fold is specific to 2.12.0 and that 3.10.0 is unaffected was wrong. Both readers fold. I had been reading local test runs that reported green while the test host was crashing partway through, exit code -1073741571, with the runner still printing Passed! for the tests that completed before the crash. CI on #8050 caught it at the same assertion I had cited as proof. Details and the fix are on that PR, which now needs a fresh look since you approved it before this came out.

Nothing changes for this branch in practice. 2.12.0 is still the right target, we still need the narrowing, and CI here was green on the previous commit. The difference is only that the same fix belongs on main too, rather than main being naturally immune as I claimed.

Second, your point about numeric unions is implemented. You are right that int | string is the JsonNumberHandling.AllowReadingFromString shape rather than a genuine union. Instead of inventing a precedence I reused the rule GetPrimitiveType already applies for clients on main from #6541: correlate the format first, resolve to the numeric type when the format is numeric, and only fall back to a string when there is nothing numeric to go on. Both this branch and #8050 now use that same rule, so the plugin path and the client path agree.

That also retires the caveat from my earlier comment. anyOf: [integer, string] with a numeric format resolves to the numeric type, not to String.

Suite here: 1967 passed, 0 failed, 2 skipped, dotnet format clean.

@baywet

Copy link
Copy Markdown
Member

Thank you for making the changes!

I believe we need to go ahead with the version bump I was referring to here and we should be good to merge this.

#8038 (comment)

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
@EelcoLos

Copy link
Copy Markdown
Contributor Author

Done, 24e037743.

Ran scripts/bump-version.ps1 -kind patch, so 1.29.0 becomes 1.29.1 in both src/kiota/kiota.csproj and src/Kiota.Builder/Kiota.Builder.csproj, then scripts/prepare-release.ps1 -date 2026-08-13, which moved the security entries under ## [1.29.1] and opened a fresh empty ## [Unreleased].

Two small things I reverted by hand, both side effects of the scripts rather than intended changes: prepare-release.ps1 strips the UTF-8 BOM from CHANGELOG.md and appends a trailing blank line. The committed diff is only the six added heading lines plus the two version lines.

Let me know if you would rather the release date be the actual merge date, I can amend.

@baywet Vincent Biret (baywet) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making the changes!

@baywet
Vincent Biret (baywet) merged commit 70cd8bc into microsoft:security/1-29-x Aug 13, 2026
289 checks passed
Dĵ ΝιΓΞΗΛψΚ (dj-nitehawk) pushed a commit to FastEndpoints/FastEndpoints that referenced this pull request Aug 15, 2026
Fixes the codegen bug from microsoft/kiota#8038, now released as
Microsoft.OpenApi.Kiota.Builder 1.29.1 (Microsoft.OpenApi 2.12.0).
Verified via `Tests/IntegrationTests/FastEndpoints.OpenApi.Kiota`
(client generation + command-mode export), which is otherwise
excluded from CI.


Claude-Session: https://claude.ai/code/session_016gczAbMsgW4deQYkFpxdTZ

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants