Skip to content

Conversation

@SeanKilleen
Copy link

What this PR does / why we need it: Updates KubernetesClient to the earliest version which is not impacted by a security vulnerability (which happens to be the latest version).

Which issue(s) this PR fixes: #2434

Please reference the issue this PR will close: #2434

Special notes for your reviewer:

Does this PR introduce a user-facing change?: No

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Unit tests passing
  • End-to-end tests passing
  • Extended the documentation
  • Provided sample for the feature

@SeanKilleen
Copy link
Author

I want to be clear -- this is still pretty early on. I'm looking for feedback from the GitHub Actions pipelines, once they're approved.

@SeanKilleen SeanKilleen marked this pull request as draft September 18, 2025 02:37
@vip32
Copy link

vip32 commented Sep 23, 2025

maybe we can get rid of the KubernetesClient dependency in the AspNetCore.HealthChecks.UI, does not seem logical that the UI part needs a Kubernetes dependency.

@SeanKilleen
Copy link
Author

@vip32 I am trying to keep it as simple as possible. This repository is actively seeking new maintainers so I'm not trying to make anything too complicated so as to reduce the burden to review and maintain my change.

@AntiPasha
Copy link

Updating k8s client version will cause error AspNetCore.Diagnostics.HealthChecks\src\HealthChecks.UI\Core\HostedService\HealthCheckReportCollector.cs(137,64,137,94): error CS0246: The type or namespace name 'BasicAuthenticationHeaderValue' could not be found (are you missing a using directive or an assembly reference?) Previous version of client uses IdentityModel.* package and now it doesn't exist anymore, while this class comes from it

@SeanKilleen
Copy link
Author

SeanKilleen commented Sep 26, 2025

@AntiPasha I figured I'd look into that once there was enough interest in this PR to at least approve the CI workflow to run. 👍

image

@AntiPasha
Copy link

Ok, thank you. Faced with this problem when updated KubernetesClient on our project and it causes failure of HealthChecks.UI

@gleb-osokin
Copy link

Hi @SeanKilleen , thanks for taking care of this! 🙏

Do you think you could update src/HealthChecks.UI/Core/HostedService/HealthCheckReportCollector.cs:

@@ -1,4 +1,6 @@
+using System.Net.Http.Headers;
 using System.Net.Http.Json;
+using System.Text;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 using HealthChecks.UI.Configuration;
@@ -134,7 +136,7 @@ internal sealed class HealthCheckReportCollector : IHealthCheckReportCollector,
                     // means you can't use _httpClient.GetAsync and have to use _httpClient.SendAsync
 
                     using var requestMessage = new HttpRequestMessage(HttpMethod.Get, absoluteUri);
-                    requestMessage.Headers.Authorization = new BasicAuthenticationHeaderValue(userInfoArr[0], userInfoArr[1]);
+                    requestMessage.Headers.Authorization = new AuthenticationHeaderValue("Basic", Convert.ToBase64String(Encoding.UTF8.GetBytes($"{userInfoArr[0]}:{userInfoArr[1]}")));
                     response = await _httpClient.SendAsync(requestMessage, HttpCompletionOption.ResponseHeadersRead).ConfigureAwait(false);
                 }
             }

TL;DR: This removes dependency on IdentityModel and fixes the build.

Longer explanation:

Previous KubernetesClient 15.0.1 had a transitive dependency on IdentityModel package.
That dependency was removed from KubernetesClient 17.0.14.
Unfortunately, HealthChecks.UI uses a class from that package, namely BasicAuthenticationHeaderValue.

To makes things worse, the IdentityModel package is now decommissioned and removed from Nuget. Luckily, the source code for this single class is still available in the archive.
If you check the source code for the class, it is just a tiny wrapper around AuthenticationHeaderValue, specifically for Basic authentication.
And, since we don't need to repeat null/empty strings checks, the rest of the logic is a simple one-liner replacement.

@SeanKilleen
Copy link
Author

SeanKilleen commented Sep 30, 2025

@gleb-osokin I am happy to do that, but as I mentioned before, this repository is a bit dormant so I'm holding off on doing anything until someone approves the GitHub Actions to run, so that I can get better feedback from the official build system as I proceed. If I can't get someone to do that, the likelihood of this being merged is very slim, so I can't spend more time on it.

@Lithrun
Copy link

Lithrun commented Nov 3, 2025

Any updates on this? As this vulnerability is still there

@SeanKilleen
Copy link
Author

@Lithrun please see my comment just prior to yours. It reflects the latest update.

@Lithrun
Copy link

Lithrun commented Nov 4, 2025

Thanks for your reply Sean! Your changes seem fine to me, just a package update. It was more directed towards the maintainers of this repo (i.e. those who can trigger the build). As I think that's what we are waiting for?

Apologies for the ping, but according to the ReadMe this should be: @unaizorrilla, @lurumad, @CarlosLanderas , @eiximenis, @evacrespob and @sungam3r. Could any of you take a look into this, as we need to resolve the vulnerability.

@arr1v3d
Copy link

arr1v3d commented Nov 26, 2025

Any updates on this?

@Lithrun
Copy link

Lithrun commented Dec 1, 2025

@SeanKilleen could you move it out of draft? As it seems like someone approved it

@SeanKilleen
Copy link
Author

SeanKilleen commented Dec 1, 2025

@Lithrun

  • The approval was done by @extgarib who is not a contributor to this project as far as I can tell. Anyone can approve a PR.
  • Regardless of whether it was approved, nobody with the appropriate permissions has enabled the CI/CD pipeline for this PR, which must be done explicitly by a maintainer as I am a new contributor. The pipeline must pass before I would consider this ready for review.

@SeanKilleen SeanKilleen mentioned this pull request Dec 1, 2025
6 tasks
@extgarib
Copy link

extgarib commented Dec 4, 2025

Hi all,
If we could speed up with review this PR it would be great!

Yeah that's true only maintainers can approve the PR to trigger build

@AlexeyRaga
Copy link

Perhaps the maintainers are awaiting (and not even notified by GitHub) until the PR is out of the draft mode?
This is my experience with opensource anywhere elsewhere.
Can we try moving it out of the draft to attract some attention from maintainers?

@SeanKilleen
Copy link
Author

Moving this to review, even though it is not quite ready for review, in the hope that it attracts the attention of a maintainer to enable the build process.

However, I also understand that this project is largely dormant and unmaintained at this point, based on #1714 and the fact that there hasn't been a release since February 2024. So I do not see a merge being very likely at this point.

@SeanKilleen SeanKilleen marked this pull request as ready for review December 8, 2025 13:39
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.

KubernetesClient has a moderate security vulnerability

8 participants