[DSIP-105][API][UI] Add Property.sensitive with API/UI masking

[det101]

Summary

Implements #18586 (subtask of DSIP-105 / #17937): add Property.sensitive and mask sensitive values as ****** on API/UI, with keep-original merge on write/start.

In scope (#18586)

• Property.sensitive (default false)
• HTTP outbound mask: SensitivePropertyResponseAdvice copy-then-masks Result.data that carries workflow/task/instance Property (query, version lists, and create/update replies). Persistence / in-process stay plaintext.
• Write-path merge: only ****** means keep original; empty string is a real empty value
• Reject creating/enabling/disabling sensitive with placeholder-only ******
• UI: Sensitive checkbox on global params and task localParams
• Start/command path skips ****** startParams so they do not overwrite definition secrets

Out of scope (follow-up subtasks)

• [DSIP-105][Feature][API] Encrypt sensitive definition params with PasswordUtils #18587: definition-time encrypt/decrypt via PasswordUtils
• [DSIP-105][Feature][Worker] Mask sensitive parameter values in task logs #18588: Worker stdout dynamic redaction + cleanup

Related

• Fixes [DSIP-105][Feature][API] Add Property.sensitive and mask values in API/UI #18586
• Refs [DSIP-105][Feature][Parent] Sensitive Variable Support — Masking in API/UI & Encrypted Storage #17937 (parent DSIP; do not close)

Test plan

• Unit: PropertySensitiveUtilsTest, SensitivePropertyUtilsTest
• Local standalone: create workflow with sensitive global + local params
• API query returns masked ****** (globalParamList / localParams / view-variables)
• Create/update Result is masked without mutating the persisted object
• UI save modal shows Sensitive checked and value ****** after reload
• CI

Verification screenshots

UI — reload workflow: sensitive global param masked as ******

UI — save modal: Sensitive checkbox available

API — same workflow masked on query / view-variables

=== Workflow Definition Query (masked) ===
globalParamList: [{"prop": "db_pwd", "direct": "IN", "type": "VARCHAR", "value": "******", "sensitive": true}]
task localParams: [{"prop": "token", "direct": "IN", "type": "VARCHAR", "value": "******", "sensitive": true}]

=== view-variables (masked) ===
{
  "globalParams": [
    {
      "prop": "db_pwd",
      "direct": "IN",
      "type": "VARCHAR",
      "value": "******",
      "sensitive": true
    }
  ],
  "localParams": {
    "shell_node_1": {
      "localParamsList": [
        {
          "prop": "token",
          "direct": "IN",
          "type": "VARCHAR",
          "value": "******",
          "sensitive": true
        }
      ],
      "taskType": "SHELL"
    }
  }
}

Screenshot branch (fork only, not part of review diff): det101:pr1-17937-verification-screenshots

[det101]

UI / API verification screenshots (PR1)

1) Reload existing sensitive workflow — global params masked

Save modal shows:

• db_pwd value = ******
• Sensitive checkbox checked

2) Create / save modal — Sensitive checkbox present

3) API mask proof (same workflow)

=== Workflow Definition Query (masked) ===
globalParamList: [{"prop": "db_pwd", "direct": "IN", "type": "VARCHAR", "value": "******", "sensitive": true}]
task localParams: [{"prop": "token", "direct": "IN", "type": "VARCHAR", "value": "******", "sensitive": true}]

=== view-variables (masked) ===
{
  "globalParams": [
    {
      "prop": "db_pwd",
      "direct": "IN",
      "type": "VARCHAR",
      "value": "******",
      "sensitive": true
    }
  ],
  "localParams": {
    "shell_node_1": {
      "localParamsList": [
        {
          "prop": "token",
          "direct": "IN",
          "type": "VARCHAR",
          "value": "******",
          "sensitive": true
        }
      ],
      "taskType": "SHELL"
    }
  }
}

[det101]

Verification screenshots (hosted on fork evidence branch)

Reload existing sensitive workflow — value masked + Sensitive checked

Save modal — Sensitive checkbox present

API mask proof

=== Workflow Definition Query (masked) ===
globalParamList: [{"prop": "db_pwd", "direct": "IN", "type": "VARCHAR", "value": "******", "sensitive": true}]
task localParams: [{"prop": "token", "direct": "IN", "type": "VARCHAR", "value": "******", "sensitive": true}]

=== view-variables (masked) ===
{
  "globalParams": [
    {
      "prop": "db_pwd",
      "direct": "IN",
      "type": "VARCHAR",
      "value": "******",
      "sensitive": true
    }
  ],
  "localParams": {
    "shell_node_1": {
      "localParamsList": [
        {
          "prop": "token",
          "direct": "IN",
          "type": "VARCHAR",
          "value": "******",
          "sensitive": true
        }
      ],
      "taskType": "SHELL"
    }
  }
}

[SbloodyS]

I found two blocking issues:

1. [P1] Sensitive values are still exposed by the definition-version APIs

The new masking is applied to the current workflow/task definition query paths, but the version-list endpoints still return the persisted entities directly:

• GET /projects/{projectCode}/task-definition/{code}/versions returns TaskDefinitionLog.taskParams.
• GET /projects/{projectCode}/workflow-definition/{code}/versions returns WorkflowDefinitionLog.globalParams.

Both SQL projections include these fields, and the service methods put the records into the response without masking them. Therefore, a parameter with sensitive=true can still be read in plaintext by querying its version history, which violates the acceptance criterion that sensitive values must never be returned through external APIs.

Please either mask the returned version entities using the same deep-copy strategy or use dedicated summary DTOs that omit these fields. Regression tests should cover both version endpoints.

2. [P1] Updating an old workflow instance can restore the secret from the wrong task version

WorkflowInstanceServiceImpl.mergeSensitiveLocalParams() resolves ****** using taskDefinitionDao.queryByCodes(), which loads the current task definitions. However, the workflow-instance detail shown to the user may have been generated from an older workflow/task version.

If the task's sensitive value changed after that instance was created, editing and saving the old instance will silently replace ****** with the latest task definition's value instead of preserving the value belonging to the displayed instance version.

Please resolve each original value using the matching task definition code and version associated with the instance/submitted task definition, rather than querying only the current definition by code. A regression test should cover updating an old instance after its task secret has changed in a newer version.

[det101]

@SbloodyS Thanks for the review. Both P1s are addressed, with a few extra hardening changes.

1. Version-list APIs
   GET .../task-definition/{code}/versions and GET .../workflow-definition/{code}/versions now mask sensitive taskParams / globalParams. Following the deep-copy approach you suggested, masking is applied to JSON copies (copyAndMask*) so MyBatis-mapped log entities are not mutated in place. Regression tests cover both endpoints and assert the original records stay unmasked.

2. Old workflow instance update
   WorkflowInstanceServiceImpl.mergeSensitiveLocalParams no longer uses taskDefinitionDao.queryByCodes(). It restores ****** from TaskDefinitionLog by code + version. To avoid N+1 queries it uses a single queryByTaskDefinitions call, with a List of keys (TaskDefinition.equals() ignores code/version, so a Set would collapse distinct versions). A regression test updates an old instance after the current task secret has changed and asserts the matching version value is kept.

3. Extra hardening

   • maskWorkflowDefinition / maskTaskDefinition now clear the cached globalParamMap / taskParamMap, because setGlobalParams does not rebuild the map and getters could still return plaintext.
   • Workflow definition update still merges against the current definition via queryByCodes(), because that path edits the live definition rather than an old instance.

[SbloodyS]

Disabling sensitive can silently overwrite the real value with ******

There is still a data-loss path when an existing sensitive parameter is changed to non-sensitive:

1. The API returns the parameter as value = "******", sensitive = true.
2. The user unchecks Sensitive without editing the value.
3. The UI submits value = "******", sensitive = false.
4. findInvalidSensitivePlaceholderProp() skips the parameter because it is no longer sensitive.
5. mergeSensitiveValuePlaceholder() also skips it for the same reason.
6. The update therefore persists the literal ****** and permanently overwrites the original value.

This affects both workflow global parameters and task localParams. Besides losing the secret, subsequent task executions will receive ****** instead of the configured credential.

Please reject ****** when an existing sensitive parameter is being changed to non-sensitive, requiring the user to enter an explicit replacement value. This validation must be enforced by the backend rather than only by the UI. Restoring the original value while setting sensitive = false would not be safe either, because the next read would expose that value as non-sensitive.

[det101]

@SbloodyS Thank you for the careful review — this is a real data-loss path, and your analysis is correct.

Unchecking Sensitive while leaving ****** must not persist the mask. Restoring the original value with sensitive = false would also be unsafe, as you noted, because the next read would return the secret as non-sensitive. We followed that guidance.

The backend now rejects this for both workflow global params and task localParams. findInvalidSensitivePlaceholderProp treats true → false with value ****** as invalid and returns REQUEST_PARAMS_NOT_VALID_ERROR, asking the user to enter an explicit replacement. Toggle sequences are judged only by the final (sensitive, value) pair, so uncheck → check again → save with true + ****** still keeps the original secret.

On the UI we chose not to auto-clear ****** when the checkbox is unchecked. Clearing it would make it too easy to save an empty string and wipe the secret by accident. Leaving the mask in the field means save fails until the user types a real new value, or checks Sensitive again (keep-original). Empty string remains a real empty value; only ****** means keep-original.

Unit tests cover the reject path and the toggle-back keep-original path.

As a related hardening, start/backfill also skips a startParam whose value is ****** instead of overlaying it onto workflow globals. That includes map-format {"pwd":"******"}, which loses sensitive=true. Please let us know if you would prefer a different approach here.

Thanks again for the detailed write-up.

[SbloodyS]

Sensitive global parameters are exposed by create/update responses

The read/query endpoints now mask sensitive parameters, but the write endpoints still return the in-memory WorkflowDefinition containing the merged plaintext globalParams:

• WorkflowDefinitionServiceImpl#createWorkflowDefinition() returns the object created from the submitted plaintext parameters.
• WorkflowDefinitionServiceImpl#updateWorkflowDefinition() returns the object after mergeGlobalParams() has restored the real value.
• WorkflowInstanceServiceImpl#updateWorkflowInstance() also returns a WorkflowDefinition containing the merged plaintext value.

The corresponding controllers serialize these objects directly in their Result<WorkflowDefinition> responses. Therefore, creating a sensitive global parameter or updating it with a new value returns the real value through an external API, which violates the acceptance criterion that sensitive parameters must only be returned as ******.

Please return a masked deep copy from these external write paths, for example by applying copyAndMaskWorkflowDefinition() after persistence. Do not mask the object before it has finished being persisted or reuse a masked object for execution.

Please add regression coverage for:

1. Creating a workflow with a sensitive global parameter.
2. Updating a workflow with a new sensitive value.
3. Updating a workflow instance containing a sensitive global parameter.

Each response should contain ******, while the persisted/internal value must remain unchanged and usable for execution.

[det101]

@SbloodyS Thank you — you are right that create/update responses were still serializing the merged plaintext WorkflowDefinition.

We did not only wrap those three service returns. Query-time masking was too easy to miss on write endpoints (and on any new Result that carries Property), so masking now happens at the HTTP outbound boundary:

• SensitivePropertyResponseAdvice copy-then-masks Result.data when it contains workflow/task/instance Property payloads. That includes createWorkflowDefinition, updateWorkflowDefinition, updateWorkflowInstance, version lists, and the existing query/view-variables paths.
• Query/write services keep returning plaintext. Persistence and in-process objects are not mutated; only a JSON copy is masked before Jackson writes the body.
• Unrelated Result payloads (login, cluster, user, …) are left unchanged.

Acceptance is unchanged: API/UI still show ******; the metadata DB is still plaintext in this PR (encryption remains #18587).