fix: require a deployment-specific session signing secret by 28Hus · Pull Request #1565 · apache/dubbo-admin

28Hus · 2026-09-21T01:31:58Z

Summary

This pull request addresses Issue #1557.

Dubbo Admin uses a client-side signed session cookie for authentication. The signing key must therefore be deployment-specific and unpredictable. The current implementation still falls back to the publicly known value secret when no sessionSecret is configured. This leaves the default password-authentication path vulnerable to forged session cookies.

Root Cause

The OAuth/OIDC work in PR #1542 introduced the sessionSecret configuration field and changed the cookie store to use it. However, the same change also retained a legacy fallback:

const DefaultSessionSecret = "secret"

When the configuration does not contain a session secret, validation restores this public value. The minimum-length check is applied only to release deployments with external providers, so a password-only deployment can still start with the known signing key.

As a result, adding a configuration field alone does not remediate the original issue. The insecure fallback remains reachable in the default authentication path.

Changes

This pull request:

removes the hard-coded default session secret;
requires a deployment-specific session secret;
rejects missing or shorter-than-32-byte secrets during startup;
supports DUBBO_ADMIN_SESSION_SECRET for Kubernetes and secret-manager based deployments;
updates the local and OAuth/OIDC configuration examples;
updates the Kubernetes deployment example to read the secret from a Kubernetes Secret;
keeps the session secret masked in displayed configuration and startup logs;
adds regression tests for password authentication, missing secrets, short secrets, environment injection, and secret sanitization.

Security Behavior

After this change:

a deployment without a configured session secret fails closed during startup;
the historical public value secret is rejected because it is too short;
a valid session cookie remains usable across restarts when the same configured secret is retained;
changing the configured secret invalidates existing session cookies, which provides a straightforward key-rotation mechanism.

Usability and Future Improvements

The current change intentionally prioritizes a secure default over zero-configuration startup. Generating a new secret only in process memory would make sessions invalid after every restart and would cause authentication failures between replicas using different keys.

To improve usability in a future change, Dubbo Admin could provide an initialization command that generates a cryptographically secure secret once and persists it to a protected configuration file or external secret store. This would preserve stable sessions while keeping runtime startup fail-closed. Such an initialization flow should remain separate from the runtime fallback logic.

Scope

This pull request focuses on removing the predictable session signing key and preserving the existing signed-cookie session design. It does not redesign the application around a server-side session store or add session revocation. Those would be separate architectural changes.

Testing

The following checks pass locally:

go test ./...
go vet ./...
YAML configuration parsing
Frontend production build

Fixes #1557

This work is part of my ongoing research, and I am very pleased to make a small contribution to improving the security of Apache Dubbo Admin.

Copilot

Copilot review overview

🟡 Changes recommended

A console: null configuration bypasses validation and starts with an empty, forgeable signing key.

Review effort: Balanced
Findings: 1 · 2

Open (3)

Validate the default console configuration after nil replacement · New
Clear session secret environment variable in missing-secret test · New
Isolate environment fallback in missing-secret validation test · New

What changed in this PR

Hardens session-cookie signing by requiring a deployment-specific secret.

Changes:

Removes the insecure default and enforces a 32-byte minimum.
Supports environment-based secret injection.
Updates tests, examples, Kubernetes configuration, and documentation.

File	Description
`pkg/config/console/auth/config.go`	Validates and loads session secrets.
`pkg/config/console/auth/config_test.go`	Tests secret validation and environment loading.
`pkg/config/console/config.go`	Removes the default signing secret.
`pkg/config/console/config_test.go`	Tests password and provider configurations.
`app/dubbo-admin/dubbo-admin.yaml`	Documents required local configuration.
`app/dubbo-admin/dubbo-admin-oauth-example.yaml`	Updates the OAuth example.
`release/kubernetes/dubbo-system/dubbo-admin.yaml`	Injects a Kubernetes Secret.
`docs/server-develop.md`	Documents secret generation and configuration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

28Hus · 2026-09-26T03:39:31Z

+	if strings.TrimSpace(c.SessionSecret) == "" {
+		c.SessionSecret = os.Getenv(SessionSecretEnvVar)
+	}
+	if strings.TrimSpace(c.SessionSecret) == "" || len([]byte(c.SessionSecret)) < MinimumSessionSecretLength {
+		return fmt.Errorf("auth: sessionSecret must contain at least %d bytes", MinimumSessionSecretLength)


Thanks for flagging this. However, the claimed authentication bypass is a false positive. In the reviewed revision, config.Load() invokes AdminConfig.PreProcess() before Validate(). With console: null, c.Console.PreProcess() panics on the nil pointer, so execution never reaches cookie.NewStore. In addition, Gorilla securecookie treats an empty hash key as unset and rejects cookie encoding and decoding; it cannot be used to forge a session.
I have since changed this path to fail closed explicitly: missing or null console configuration is rejected during preprocessing, and an empty console object is rejected during validation. Regression tests cover these cases, including when a valid session-secret environment variable is set.

+}
+
+func TestConfigValidateRequiresSessionSecret(t *testing.T) {
+	cfg := validConfig()


 }

+func TestPasswordAuthenticationRequiresStrongSessionSecret(t *testing.T) {
+	cfg := DefaultConsoleConfig()


robocanic · 2026-09-26T02:40:20Z

@28Hus please resolve the comments the copilot left and give feedbacks to me if there are any problems.

28Hus · 2026-09-26T02:52:39Z

Copilot review overview

🟡 Changes recommended

A console: null configuration bypasses validation and starts with an empty, forgeable signing key.

Review effort: Balanced Findings: 1 · 2

Open (3)

Validate the default console configuration after nil replacement · New

Clear session secret environment variable in missing-secret test · New

Isolate environment fallback in missing-secret validation test · New

What changed in this PR
Hardens session-cookie signing by requiring a deployment-specific secret.

Changes:

Removes the insecure default and enforces a 32-byte minimum.

Supports environment-based secret injection.

Updates tests, examples, Kubernetes configuration, and documentation.

File Description
pkg/config/console/auth/config.go Validates and loads session secrets.
pkg/config/console/auth/config_test.go Tests secret validation and environment loading.
pkg/config/console/config.go Removes the default signing secret.
pkg/config/console/config_test.go Tests password and provider configurations.
app/dubbo-admin/dubbo-admin.yaml Documents required local configuration.
app/dubbo-admin/dubbo-admin-oauth-example.yaml Updates the OAuth example.
release/kubernetes/dubbo-system/dubbo-admin.yaml Injects a Kubernetes Secret.
docs/server-develop.md Documents secret generation and configuration.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@28Hus please resolve the comments the copilot left and give feedbacks to me if there are any problems.

@robocanic Thanks for the reminder; I am looking into it and planning to make changes.

sonarqubecloud · 2026-09-26T03:35:58Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

28Hus · 2026-09-26T03:44:15Z

@robocanic
Thanks for the review. I pushed follow-up commits addressing the configuration handling and test concerns.

Regarding the console: null finding: the claimed authentication bypass is inaccurate. In the reviewed revision, config.Load() calls AdminConfig.PreProcess() before Validate(), so a nil Console panics before execution can reach cookie.NewStore. Also, Gorilla securecookie rejects an empty hash key for cookie encoding and decoding. Nevertheless, I updated the code to handle this configuration explicitly and safely: missing or null console now fails closed during preprocessing, and an empty console object fails validation.

I also made the missing-secret tests deterministic by explicitly clearing DUBBO_ADMIN_SESSION_SECRET. Regression tests cover these cases. The full Go test suite, go vet, and local startup and authentication checks pass.

robocanic · 2026-09-26T07:14:37Z

-          volumeMounts:
+          image: apache/dubbo-admin:0.7.0
+          imagePullPolicy: IfNotPresent
+          env:


Question: There is no secret defined in the deploy manifests. I think the secret defined in the ConfigMap is just fined, and if there is need to put it into secret, you need to bring up a new Secret Resource Definition.

Thanks for pointing this out. The dubbo-admin-auth Secret is created separately using the kubectl create secret generic command at the top of this manifest, before applying the Deployment. That command creates the Secret resource with a unique key for each installation. The Deployment then reads its session-secret key through secretKeyRef. We intentionally do not commit a Secret manifest containing a fixed signing key, since that would recreate the shared-key issue this PR fixes. Storing the signing key in the ConfigMap would also expose it as ordinary configuration data.
Of course, I have only considered the security aspect; regarding usability, we could later implement a feature that generates a strong, random key if the secret is missing from the YAML file.

we could later implement a feature that generates a strong, random key if the secret is missing from the YAML file.

Agree with that. The manifests in the directory is a one-stop deployment solution, so can you provide a solution more smoothly?

fix: require secure session signing secret

ad1282e

28Hus mentioned this pull request Sep 21, 2026

Hard-coded session signing key allows authentication bypass in Go-based Dubbo Admin #1557

Open

robocanic requested a balanced review from Copilot September 26, 2026 01:56

Copilot started reviewing on behalf of robocanic September 26, 2026 01:56 View session

Copilot AI reviewed Sep 26, 2026

View reviewed changes

28Hus added 3 commits September 26, 2026 11:05

fix: validate default console session secret

f04a519

fix: reject missing console configuration

0a1b8e9

test: cover fail-closed console loading

16af62f

robocanic reviewed Sep 26, 2026

View reviewed changes

Navigation Menu

fix: require a deployment-specific session signing secret - #1565

fix: require a deployment-specific session signing secret#1565
28Hus wants to merge 4 commits into
apache:developfrom
28Hus:codex/fix-session-secret

28Hus commented Sep 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

28Hus Sep 26, 2026

Uh oh!

robocanic commented Sep 26, 2026

Uh oh!

28Hus commented Sep 26, 2026

Copilot review overview

🟡 Changes recommended

Uh oh!

sonarqubecloud Bot commented Sep 26, 2026

Uh oh!

28Hus commented Sep 26, 2026

Uh oh!

robocanic Sep 26, 2026

Uh oh!

28Hus Sep 26, 2026

Uh oh!

robocanic Sep 27, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

28Hus commented Sep 21, 2026

Summary

Root Cause

Changes

Security Behavior

Usability and Future Improvements

Scope

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

28Hus Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

robocanic commented Sep 26, 2026

Uh oh!

28Hus commented Sep 26, 2026

Copilot review overview

🟡 Changes recommended

Uh oh!

sonarqubecloud Bot commented Sep 26, 2026

Quality Gate passed

Uh oh!

28Hus commented Sep 26, 2026

Uh oh!

robocanic Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

28Hus Sep 26, 2026

Choose a reason for hiding this comment

Uh oh!

robocanic Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robocanic Sep 27, 2026 •

edited

Loading