apache / apache/dolphinscheduler
[DSIP-105][Feature][Parent] Sensitive Variable Support — Masking in API/UI & Encrypted Storage
- Dominant language
- Java
- Stars
- 14.5k
- Forks
- 5.1k
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 29
Description
### Search before asking
- [x] I had searched in the [DSIP](https://github.com/apache/dolphinscheduler/issues/14102) and found no similar DSIP.
## Subtasks
Each subtask is one issue and one PR (same pattern as #6407). Design stays on this parent.
- [ ] **API/UI masking** — `Property.sensitive`, mask `******` on read, keep-original on write — #18586 / PR #18585
- [ ] **Definition encryption** — reuse `PasswordUtils` (no datasource CRUD change) — #18587
- [ ] **Worker log masking** — dynamic stdout redaction + cleanup — #18588
## Motivation
Currently, DolphinScheduler variables do not distinguish between sensitive and non-sensitive information. When users store secrets such as passwords or API keys as workflow global parameters or task local parameters, these values may be exposed in plaintext via:
1. UI pages that display parameter definitions
2. API responses (e.g., viewVariables, getWorkflowDefinition, queryWorkflowDefinitionByCode, etc.)
3. Database definition JSON (`global_params` / `task_params` value fields)
This change adds a variable-level sensitive flag on the existing `Property` model: mask values in API/UI, and apply datasource-level at-rest protection for sensitive definition values before persist (reuse `PasswordUtils`).
Delivery is split into **three subtasks** (see Subtasks): #18586 API/UI masking, #18587 definition encryption, #18588 Worker log masking. Until #18588 lands, task logs may still print sensitive values.
### Out of scope
| Item | Notes |
|------|--------|
| Project parameters (`ProjectParameter` / `t_ds_project_parameter`) | Different storage model than `Property`; not in this DSIP. UI/docs should state this is unsupported for now |
| Workflow definition **Export / Import** | Feature removed or unmaintained; **not considered** |
| KMS / envelope encryption / key rotation | Out of scope |
| Encrypting instance `global_params` | This DSIP uses plaintext materialization at runtime; encrypting instance params can be a follow-up if the community requires it |
Task stdout log masking: **not in #18586 / #18587**; implemented in **#18588** (still part of this DSIP delivery — see Subtasks).
---
## Goals (acceptance)
### #18586 — API/UI masking
- Definition parameters marked `sensitive=true` never return real values in any external API/UI response.
- Unmarked parameters behave exactly as today.
- Saving with `******` keeps the stored value (keep-original). Empty string is a real empty value.
### #18587 — Definition encryption
- When `datasource.encryption.enable=true`, sensitive definition values are not stored as plaintext.
- Saving must not double-encrypt unchanged sensitive values.
- After same-cluster Copy of a definition, sensitive parameters still work at runtime.
### #18588 — Worker log masking
- Task stdout (and agreed task log paths) must not contain plaintext values of that task’s sensitive parameters.
- Dynamic mask patterns must be cleared when the task ends (success / failure / kill), so later tasks are not polluted.
### Explicitly not claimed
- With encryption disabled (default), definition storage may still be plaintext.
- Protection is at the same level as datasource passwords (salt + Base64 obfuscation), not compliance-grade encryption.
- Plaintext remains visible in Master/Worker memory and the task process.
- #18586 / #18587 alone do **not** claim log safety.
---
## Delivery (three subtasks)
| Subtask | Issue | Scope | Depends on |
|---------|-------|--------|------------|
| **API/UI masking** | #18586 | `Property.sensitive`; API/UI masking; save merge (keep-original `******` only); start-time merge; UI checkbox; unit tests | None |
| **Definition encryption** | #18587 | Reuse `PasswordUtils` (no datasource CRUD); no double-encrypt; Copy keeps ciphertext as-is | #18586 |
| **Worker log masking** | #18588 | Dynamically register log mask patterns from this task’s sensitive param values; mandatory cleanup on task end; unit tests + log assertions | #18586 |
**Merge order:** #18586 first, then #18587 and #18588 (both can follow #18586 independently).
---
## Design Details
### 1. Add `sensitive` boolean to `Property`
Do **not** introduce a new data type (e.g. `SENSITIVE_VARCHAR`). Only add a field on existing `Property`:
```java
@Builder.Default
private boolean sensitive = false;
```
- Missing / null in JSON → treat as `false` (backward compatible).
- Serialized into existing JSON columns; **no schema change**.
- Applies to workflow global params and task `localParams`.
### 2. Placeholder semantics
Reuse existing constant `Constants.XXXXXX` (`"******"`).
| Scenario | Meaning |
|----------|---------|
| API / UI display | Sensitive values are always `******` |
| Update submit | Sensitive value `******` **or empty string** → “unchanged; keep DB value” |
| Real secret | Document that users must not use `******` as a real password (same convention as datasource) |
UI: use a checkbox for `sensitive`; do not rely on `type=password` alone. Always echo `******`; on edit, replace the whole value; if unchanged, submit `******`.
### 3. Storage layers and threat model
| Layer | Stored content | Notes |
|-------|----------------|--------|
| **Definition** (workflow/task definition and definition log) | `sensitive=true` values are **ciphertext** when encryption is enabled | Protects static config |
| **Runtime** (workflow instance `global_params`, task execution context) | **Plaintext** after decrypt + merge at start (materialized) | For param curing and task dispatch |
| **External API / UI** | Always `******` | Mask both definition and instance queries |
Why runtime plaintext: curing, complement, rerun, and context dispatch already assume readable values; encrypting instance params would scatter decrypt across the execution path. This DSIP covers the main goals with **definition encryption + API masking** (+ PR2 log masking).
### 4. Encrypt / decrypt (definition write path only, PR1)
- Reuse `PasswordUtils.encodePassword` / `decodePassword`.
- Same switch as datasource: `datasource.encryption.enable` (default `false`).
- After salt change, old ciphertext cannot be decrypted; users must re-enter (same as datasource).
**Encode only when all of the following hold:**
- `sensitive == true`
- value is non-empty
- value is **not** a keep-original marker (not `******`, not empty)
- value is **new plaintext** from the client (see §5)
### 5. Write path (forbid double encryption, PR1)
```
Client submit
→ for each field:
if sensitive && value is keep-original (****** or empty):
write back the DB value for the same prop **as-is**
(if already ciphertext, keep ciphertext)
**do not** encode again
else if sensitive && value is new plaintext:
encode (if encryption enabled) → write
else:
write as non-sensitive as-is
→ persist
```
- Match key: param name `prop`.
- Creating a new `prop` with only a keep-original marker is invalid (nothing to merge → reject; require plaintext).
**`sensitive` flag transitions:**
| Change | Behavior |
|--------|----------|
| `false → true`, new plaintext submitted | encode then store |
| `false → true`, keep-original submitted | **reject**; require re-entering plaintext |
| `true → false` | decode to plaintext, then store with `sensitive=false` |
| `true → true`, keep-original | keep DB ciphertext as-is; do not encode |
Suggested hooks: before workflow definition save for global params; before task definition save for local params. Exact class names are implementation details; **behavior above is normative**.
### 6. Read path (copies only; do not mutate shared entities, PR1)
Shared helpers (names illustrative):
- `maskSensitiveProperties(...)` → **deep copy** then mask, for API responses
- `decryptSensitiveProperties(...)` → **deep copy** then decrypt, for internal execution/merge
- `mergeKeepOriginal(submitted, existingFromDb)` → merge keep-original only; **does not** encrypt
Rules:
1. After loading definition from DB, use decrypt **copies** only on internal paths (start merge / task dispatch).
2. Any path returning to UI / OpenAPI / token clients must return mask **copies** only.
3. **Never** in-place replace values with `******` on a shared `WorkflowDefinition` / `TaskDefinition` entity that is later used for execution.
Masking must cover every API that returns `Property` / `globalParams` / `localParams` / DagData (including list/paging/detail/variables). Do not assume `genDagData` alone is sufficient.
**Order of operations:**
- Write: merge keep-original → encode **only new plaintext** → persist
- Read (external): DB → decrypt to copy if needed → mask → response
- Read (execution): DB → decrypt to copy → merge/cure/dispatch (plaintext)
### 7. Start & Command (PR1)
1. Load definition globalParams → decrypt copy.
2. Merge with command / startParams: same name with keep-original → use decrypted definition value; new plaintext → may override.
3. Write merged plaintext into workflow instance `global_params` (runtime materialization).
4. Later `prepareParamsMap` uses instance/context plaintext; **do not** apply definition-style encode on instance fields.
Start UI: show `******` for sensitive items; submit keep-original if unchanged; allow override with a new value.
### 8. OUT parameters (PR1 API masking; PR2 for logs)
- `sensitive=true` is allowed on OUT params.
- Values written at runtime are masked in instance/variable APIs (PR1).
- Sensitive values appearing in stdout are handled by PR2.
### 9. Copy (Export / Import not considered)
| Operation | This DSIP |
|-----------|-----------|
| **Export / Import** | **Out of scope** (removed / unmaintained) |
| **Copy** (same-cluster definition copy, e.g. `batchCopyWorkflowDefinition`) | Server copies JSON as-is (including ciphertext and `sensitive`); no UI round-trip; **no double encryption** (PR1) |
### 10. External surfaces that must be masked (PR1 principles)
Any response containing global/local `Property` must be masked, including but not limited to:
- `viewVariables` (definition / instance)
- `queryWorkflowDefinitionByCode` / `getWorkflowDefinition` / list / paging / byName
- `queryWorkflowInstanceById` and other APIs returning dag/params
- `getTaskDefinition` / `queryTaskDefinitionDetail`
Save APIs: merge keep-original first, then encrypt per §5, then persist.
### 11. UI (PR1)
- Add `sensitive` checkbox on global params and task local params forms.
- Echo sensitive values as `******`; submit `******` when unchanged.
- Leave project parameters unchanged; tip that sensitive marking is only for workflow global and task local params for now.
### 12. Task stdout log masking (PR2)
**Goal:** task logs must not contain plaintext values of this task’s sensitive parameters.
**Design points:**
- Input: plaintext values of `sensitive=true` entries in this task’s `prepareParamsMap` (or equivalent context).
- Behavior: when those values appear in logs → replace with `******` (may extend existing `SensitiveDataConverter` with **per-task dynamic** pattern registration).
- Lifecycle: register at task start; **must** clear on task end (success / failure / kill); no static global leakage into later unrelated tasks.
- Scope: stdout / task logs on the physical task execution path; details in the PR2 description.
- Non-goals: project parameters; does not replace definition encryption; does not claim coverage of every plugin custom log file.
### 13. Risks & mitigations
| Risk | Mitigation |
|------|------------|
| `******` used as a real password | Document the prohibition; same as datasource |
| Double-encoding keep-original corrupts the secret | §5: write DB value as-is; never encode again |
| In-place mask pollutes execution | §6: deep copies required; tests cover this |
| Legacy data lacks `sensitive` | Default `false` |
| Salt change | Same as datasource: re-enter secrets |
| Legacy unencrypted data | `decodePassword` returns original when undecodable |
| Encryption off by default | Document honestly; same expectation as datasource |
| PR2 mask not cleared → cross-task pollution | Unified cleanup on all end paths (including kill); concurrency tests |
| Users assume logs are safe after PR1 only | Motivation / Goals + release notes point to PR2 |
---
## Compatibility, Deprecation, and Migration Plan
- Backward compatible: `sensitive` defaults to `false`; existing workflows unchanged.
- No DB migration: field lives in existing JSON.
- When `datasource.encryption.enable=false`, do not encrypt; old plaintext remains readable.
- Rollout: backend (PR1) → frontend (PR1) → logging (PR2).
- Rollback: PR2 and PR1 frontend can be rolled back independently; old clients ignore unknown fields.
---
## Test Plan
### PR1
- **Unit:** `Property.sensitive` serialization; merge keep-original without double encrypt; `false↔true` transitions; mask/decrypt deep copies do not mutate originals; encryption enable true/false.
- **API integration:** create definition with sensitive global/local params → queries return `******` → update only non-sensitive fields then run task still sees real secret → with `enable=true`, definition DB value is not plaintext → Copy still runs correctly.
- Frontend: lint / prettier; variable views show no plaintext.
- Regression: unmarked params match current behavior.
### PR2
- Unit: dynamic register/clear; consecutive or concurrent tasks do not cross-contaminate masks.
- Integration or repeatable task-log case: when a sensitive value is printed by the script, logs show `******`; cleanup still works after failure/kill.
---
## Separate / Follow-up (not in these three subtasks)
- Project parameter sensitive flag
- Export / Import strategy
- Encrypted instance `global_params`
- Optional encryption switch independent of `datasource.encryption`
---
## Design decisions (summary)
1. Definition ciphertext + runtime plaintext materialization + API masking
2. Export/Import not considered; Copy copies ciphertext as-is
3. `false→true` requires re-entering plaintext
4. Keep-original = `******` or empty string; never re-encode keep-original
5. Three subtasks: #18586 API/UI masking; #18587 definition encryption; #18588 stdout dynamic masking
---
## Code of Conduct
- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
Contributor guide
Assessment
This issue has not been assessed yet.