[det101]

Was this PR generated or assisted by AI?

YES. The code changes (sensitive field rule validation, log masking refactoring) and tests were AI-assisted; the design decisions and review were human-driven.

Purpose of the pull request

The worker server prints sensitive parameter values (e.g. passwords, API keys) in its own log output, which is a security risk. This PR:

1. Masks sensitive parameter values in the worker's own log output, uniformly via SensitiveDataConverter (no per-log-statement handling).
2. Adds a sensitive field rule: a sensitive value must be at least 6 characters, enforced on both the web forms and the backend create/update paths.

Related issue: #18588

Brief change log

• Worker log masking (uniform, via SensitiveDataConverter)

  • PhysicalTaskExecutor registers the task's sensitive plaintext values via SensitiveDataConverter.registerTaskSensitiveValues(taskInstanceId, prepareParamsMap) when the task context is initialized, and clears them via clearTaskSensitiveValues in finalizeTask (success / failure / kill), so values never leak across tasks.
  • SensitiveDataConverter (logback MessageConverter) masks both the static regex patterns (e.g. datasource password) and the per-task registered values, resolving the task from MDC.
  • Values shorter than MIN_SENSITIVE_LITERAL_LENGTH (6) are skipped to avoid over-masking common log text.
  • The task process's own output (stdout / K8s pod logs) is intentionally NOT masked — it is the task's own output; only logs printed by the worker server itself are masked.
  • No point-level changes at log statements: no injection of sensitive values into shell builders, no per-statement masking of the final script content or task context dump.

• Sensitive field rule (min length 6)

  • PropertySensitiveUtils.SENSITIVE_FIELD_RULE is defined as a regex (.{6,}, i.e. at least 6 characters) so the rule can be extended later; findInvalidSensitiveField returns the first offending prop name (empty values and the ****** keep-original placeholder are skipped).
  • Backend: SensitivePropertyUtils.requireNoPlaceholder (create) and SensitivePropertyUtils.merge (update) reject a too-short sensitive value with ServiceException, covering both workflow global params and task local params.
  • Frontend: workflow global params (dag-save-modal.tsx) and task custom/local params (use-custom-params.ts) validate the value when the sensitive checkbox is checked; the input is marked red and the submission is blocked. i18n messages sensitive_value_invalid added (zh_CN / en_US).

Verify this pull request

This change added tests and can be verified as follows:

• PropertySensitiveUtilsTest (task-api): length boundary covered — a 5-char sensitive value is rejected, a 6-char value passes; placeholder / empty values are skipped.
• SensitivePropertyUtilsTest (api): create and update paths reject a too-short sensitive value and accept values satisfying the rule.
• SensitiveDataConverterTest (task-api): per-task register / mask / clear behavior.
• BaseLinuxShellInterceptorBuilderTest (task-api): final script content is written to the shell file as-is (only the logged content is masked by the converter).
• Frontend: vue-tsc type check passes for the changed files.

Pull Request Notice

Pull Request Notice: https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md

Compatibility note: saving a sensitive parameter whose value is shorter than 6 characters is now rejected (web form shows an error, backend returns an error). Existing stored values are not affected; the restriction only applies to new saves / updates. If this is considered an incompatible change, docs/docs/en/guide/upgrade/incompatible.md should be updated accordingly.

[ruanwenjun]

We should only ensure that the content printed by the worker server itself does not contain sensitive information; there is no need to encrypt the output of the shell tasks themselves, as this should be handled by the shell itself.

[det101]

Thanks @ruanwenjun, addressed. Reverted the taskInstanceId MDC injection in AbstractCommandExecutor — task process stdout and K8s pod logs are now collected unmasked, same as dev. Masking now applies only to logs printed by the worker server itself (task context dumps, plugin logs on the executor thread), where sensitive prepareParams plaintext would otherwise leak into worker logs.

[det101]

Verified end-to-end on a standalone instance (official 3.4.3 image + this branch's jars + rebuilt UI), with a SHELL task carrying secret_token (sensitive) and normal_msg (normal) prepareParams:

Log location	Result
Task log: localParams JSON / Final Script Content	value: "******" ✅
Task log: process output	plaintext SuperSecretToken123 kept, per review intent ✅
Worker log: Initialized Task Context{...} dump (taskParams + prepareParamsMap)	******, MDC [TI-1] ✅
Normal params	never masked, no over-masking ✅

During this E2E run I also found a pre-existing leak unrelated to this PR's changes: PhysicalTaskExecutorOperatorImpl (introduced in #16790) logged the whole TaskExecutorDispatchRequest on the RPC dispatch thread — before per-task masks are registered and without taskInstanceId MDC, so both masking mechanisms missed it. Fixed in 48c469e by logging only taskInstanceId and taskName; after the fix the worker log contains no sensitive plaintext outside the process-output forwarding.

[ruanwenjun]

Thanks for working on this. I tested the current masking path and found three gaps:

• Secrets containing quotes, backslashes, or newlines remain recoverable in JSON context logs because their escaped forms do not match the registered plaintext.
• A sensitive global parameter overridden by a local parameter disappears from prepareParamsMap, but its original value is still printed through globalParams in the context dump.
• Exception messages bypass MessageConverter. For example, an HTTP body of ["${token}"] causes HttpTask to throw with the resolved body, which is then printed unmasked in the stack trace.

Could we use a combination of structured redaction and output-level masking?

• Build a redacted copy of the context for logging, masking sensitive fields before JSON serialization and keeping the execution context intact.
• Keep task-scoped literal masking as a fallback, collecting both original and resolved sensitive values. Apply it after the complete log event is rendered so it also covers exception output.
• Use an explicit marker for process stdout / pod logs to skip this task-parameter masking, rather than relying on those threads having no task MDC.

I'd also suggest dropping the six-character validation from this PR. It changes which business parameter values are accepted and still cannot prevent accidental matches in unrelated log text.

Please add tests using the actual Logback layout for these cases, including process-output preservation.

[det101]

Thanks @ruanwenjun, addressed in the latest commit.

• Added structured redaction via TaskExecutionContextLogRedactor for context dumps (execution context stays plaintext; copy-failure falls back to a safe stub without parameter payloads).
• Switched worker/master/standalone logback to SensitiveDataLayout so masking runs after the full log event is rendered (covers exception/stack traces). Task-scoped literal masking now registers prepareParams + globalParams + varPool, including JSON-escaped forms.
• Marked process stdout / pod logs with TaskLogMarkers.skipTaskParamMask() so task-parameter literal masking is skipped explicitly.
• Dropped the six-character sensitive-value validation from API/UI.
• Added Logback layout tests for JSON-escaped secrets, exception masking, process-output preservation, and redactor copy-failure safety.

Please take another look when you have a chance.