fix: removing EnvConfigSource keys that are not intended by shawkins · Pull Request #51291 · keycloak/keycloak

shawkins · 2026-07-29T17:00:55Z

Copilot

Pull request overview

Fixes #51113 by separating Keycloak environment variables from Quarkus environment handling and masking SPI values in show-config.

Changes:

Filters Keycloak-specific environment variables from Quarkus configuration.
Masks all SPI option values in show-config.
Adds regression tests and upgrade documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`KcEnvConfigSource.java`	Splits Keycloak and Quarkus environment sources.
`Configuration.java`	Manually registers default configuration sources.
`ShowConfig.java`	Masks SPI configuration values.
`PicocliTest.java`	Tests SPI secret masking.
`changes-26_8_0.adoc`	Documents the masking change.

shawkins · 2026-08-04T20:11:42Z

+                    .withSources(new DotEnvConfigSourceProvider()
+                            .getConfigSources(Thread.currentThread().getContextClassLoader()))


This is not an issue for two reasons. 1. all spi options are now masked. 2. .env files are not a documented configsource, it is not expected that they will be used.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java:279

The negative-only assertion can also pass if this valid SPI property is accidentally dropped from show-config, so it does not verify the documented “mask” behavior. Assert the property is still emitted with the mask as well as checking that the secret is absent.

        NonRunningPicocli nonRunningPicocli = pseudoLaunch("show-config");
        assertThat(nonRunningPicocli.getOutString(), not(containsString("custom-secret")));

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

docs/documentation/upgrading/topics/changes/changes-26_8_0.adoc:57

This repeats the release-note section already added at lines 41–43, causing the same change to appear twice. Remove this second section.

=== `show-conifg` will mask all SPI option values

The `show-config` command will mask all SPI option values. The command is not currently aware of provider factory configuration property metadata, so it cannot determine if a property is marked as `isSecret`.

shawkins · 2026-08-05T11:10:41Z

Should be ready for a review. Given that we are now masking all SPI options the logic to change how the env properties are discovered could be reverted - if it seems too risky to take responsibility for this in our code.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Pepo48 · 2026-08-05T20:52:43Z

Given that we are now masking all SPI options the logic to change how the env properties are discovered could be reverted - if it seems too risky to take responsibility for this in our code.

@shawkins, I'm not sure the env filtering can be safely reverted. The SPI masking catches kc.spi-* (hyphenated) properties, but SmallRye's EnvConfigSource aliases KC_SPI_VAULT__KEYSTORE__PASS to kc.spi.vault."keystore".pass (dotted), which bypasses the kc.spi- prefix check entirely.

So I guess it's not redundant, but rather complementary.

shawkins · 2026-08-05T21:14:02Z

So I guess it's not redundant, but rather complementary.

That is correct, the spi masking would need to check for both prefixes.

Pepo48 · 2026-08-06T08:04:37Z

+        if (property.startsWith(MicroProfileConfigProvider.SPI_PREFIX)) {
+            // could be marked as ProviderConfigProperty.isSecret, so the simplest option for now
+            // is to just mask all direct usage of spi options.
+            // the most straight-forward alternative is to move show-config to be run after the quarkus start


@shawkins, correct. To be specific, if we decide to revert the changes in KcEnvConfigSource.java and Configuration.java, I would adjust:

Suggested change

// the most straight-forward alternative is to move show-config to be run after the quarkus start

if (property.startsWith(MicroProfileConfigProvider.SPI_PREFIX)

|| property.startsWith(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + "spi.")) {

During the review yesterday, I ran the added tests with this change on top and everything seemed to work.

Correct, I'm not saying that we have to take this approach, just that it's an option if anyone feels the changes to the Configuration creation seem too risky.

shawkins · 2026-08-07T14:00:35Z

Squashed and rebased to see if the doc check error will get cleared.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

keycloak-github-bot · 2026-08-07T15:08:53Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.cluster.RealmInvalidationClusterTest#crudWithFailover

Keycloak CI - Store IT (oracle)

org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
	at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

closes:keycloak#51113 Signed-off-by: Steve Hawkins <shawkins@redhat.com> # Conflicts: # docs/documentation/upgrading/topics/changes/changes-26_8_0.adoc

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

keycloak-github-bot Bot added the team/cloud-native label Jul 29, 2026

shawkins mentioned this pull request Jul 29, 2026

Usage of second class configuration may not be masked in show-config #51113

Open

shawkins marked this pull request as ready for review August 4, 2026 20:03

Copilot AI balanced review requested due to automatic review settings August 4, 2026 20:03

shawkins requested review from a team as code owners August 4, 2026 20:03

Copilot started reviewing on behalf of shawkins August 4, 2026 20:05 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 4, 2026 20:10

Copilot started reviewing on behalf of shawkins August 4, 2026 20:11 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 4, 2026 22:43

Copilot started reviewing on behalf of shawkins August 4, 2026 22:43 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 5, 2026 11:08

shawkins force-pushed the iss51113 branch from c1fee52 to e11efdf Compare August 5, 2026 11:08

Copilot started reviewing on behalf of shawkins August 5, 2026 11:09 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

Pepo48 previously approved these changes Aug 5, 2026

View reviewed changes

Pepo48 reviewed Aug 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 7, 2026 14:00

shawkins force-pushed the iss51113 branch from e11efdf to 88b6a22 Compare August 7, 2026 14:00

Copilot started reviewing on behalf of shawkins August 7, 2026 14:00 View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

keycloak-github-bot Bot added the flaky-test label Aug 7, 2026

keycloak-github-bot Bot reviewed Aug 7, 2026

View reviewed changes

fix: masking all spi options in show-config and refining env handling

9aa7246

closes:keycloak#51113 Signed-off-by: Steve Hawkins <shawkins@redhat.com> # Conflicts: # docs/documentation/upgrading/topics/changes/changes-26_8_0.adoc

Copilot AI review requested due to automatic review settings August 8, 2026 11:26

shawkins dismissed Pepo48’s stale review via 9aa7246 August 8, 2026 11:26

shawkins force-pushed the iss51113 branch from 88b6a22 to 9aa7246 Compare August 8, 2026 11:26

Copilot started reviewing on behalf of shawkins August 8, 2026 11:27 View session

Copilot AI reviewed Aug 8, 2026

View reviewed changes

		.withSources(new DotEnvConfigSourceProvider()
		.getConfigSources(Thread.currentThread().getContextClassLoader()))

	// the most straight-forward alternative is to move show-config to be run after the quarkus start
	if (property.startsWith(MicroProfileConfigProvider.SPI_PREFIX)
	\|\| property.startsWith(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + "spi.")) {

Uh oh!

Conversation

shawkins commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

shawkins Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

shawkins commented Aug 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Pepo48 commented Aug 5, 2026

Uh oh!

shawkins commented Aug 5, 2026

Uh oh!

Pepo48 Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

shawkins Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

shawkins commented Aug 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

keycloak-github-bot Bot commented Aug 7, 2026

Unreported flaky test detected

org.keycloak.testsuite.cluster.RealmInvalidationClusterTest#crudWithFailover

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants