Skip to content

fix(sync): keep credentials out of configuration and sync logs (#8220) - #8226

Open
konlue wants to merge 1 commit into
apache:masterfrom
konlue:fix/sync-log-credentials
Open

konlue wants to merge 1 commit into
apache:masterfrom
konlue:fix/sync-log-credentials

Conversation

@konlue

@konlue konlue commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Fixes #8220.

Problem

Configuration synchronization logs operational credentials: full plugin/selector/rule configuration and handles at INFO, the accepted Admin access token, changed ConfigData, AI proxy keys, raw WebSocket frames and the Admin-side WebSocket payload. Log readers or aggregation/export copies can obtain credentials; lowering these messages to DEBUG would not fix the exposure.

Change

  • Subscriber and logging-plugin handlers now log a bounded allowlist only: plugin/selector identity, enabled state and namespace; no config / handle JSON. Subscription and snapshot error paths log the data class/type plus the exception, not the serialized DTO.
  • HTTP synchronization logs token issuance and expiry (never the token), refresh group/md5/modify-time/item count, and payload size (md5 only while DEBUG is enabled). Listener responses log size rather than the body.
  • WebSocket synchronization drops the raw frame DEBUG message; the existing bounded group/event/namespace/size line remains. AI proxy-key updates log selector/namespace/enabled instead of the key.
  • Admin delivery no longer reparses, redacts and re-serializes the whole WebSocket payload at DEBUG, and DataChangedEvent logging no longer serializes the event source.
  • Authentication and synchronization still receive the real values. No payload parsing, recursive redaction, request-path scan, or extra serialization when logging is disabled was added; the ordinary forwarding path is untouched.

Tests

  • Logger-capture regressions with unmistakable fake credentials assert the emitted output for plugin subscription and its error fallback, logging-plugin enable/disable, HTTP login, initial/update/refresh delivery, AI proxy-key delivery and WebSocket frames.
  • The Admin collector and dispatcher tests assert that DEBUG adds no payload diagnostic and that no secret is emitted; the old payload masking helper and its direct test are removed.
  • Guard tests assert the logging path never calls PluginData#getConfig, so a disabled/unconfigured log path cannot re-introduce DTO serialization.

Verification

shenyu-plugin-base: 337/337
shenyu-plugin-logging-common: 147/147
shenyu-sync-data-http: 67/67
shenyu-sync-data-websocket: 150/150
shenyu-admin: 2571 (9 skipped)
shenyu-common: 626 (4 skipped)
loadbalancer: 83

Checkstyle clean on the touched modules. No measurable hot-path change: only configuration/sync log statements changed, removing serialization rather than adding work; md5 is computed only when DEBUG is enabled.

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed f256d35.

The changed log sites use delivery metadata rather than token/configuration/handle/frame contents, and the Admin WebSocket path no longer reparses and serializes a payload solely for DEBUG logging. Real credentials still reach the authentication/synchronization code, and the forwarding path is untouched.

The targeted log-capture and delivery regressions pass. I inspected the failed CI jobs: the Dubbo job failed resolving org.apache:apache:pom:21 with Maven Central HTTP 429, while the Kubernetes job stopped because its local example image was missing. I have not attributed either to this logging change; the required jobs still need successful reruns before merge.

Targeted test report: 194 cases, 0 failures, 0 errors, 0 skipped.

Validation used ./mvnw, Amazon Corretto 21 and project target Java 17. Targeted Checkstyle, separately executed Apache RAT and git diff --check pass. No full-repository or Docker E2E pass is claimed; required CI must succeed before merge.

This branch has not been deployed

No deployments
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.

[BUG][Bootstrap] Keep credentials out of configuration and sync logs

2 participants