maintenance: classify service credentials as passwords

[zqr10159]

Summary

This update completes the storage and API lifecycle for the Ollama API key and HTTP service-discovery access token.

• classifies both template parameters as passwords
• runs an ordered, transactional startup migration before job scheduling
• encrypts legacy plaintext database values and changes their stored parameter type
• leaves already encrypted values unchanged, including ciphertext written with the legacy default root
• masks password parameters as ****** in monitor API responses and exports
• restores the stored value only for an authenticated edit of the same monitor
• requires credential re-entry when the Ollama host/port/SSL destination or HTTP service-discovery URL/authentication type changes
• rejects the response mask as a new credential value
• documents the upgrade and edit behavior

The migration is idempotent and logs only the number of migrated rows. It never logs a credential value. It executes before SchedulerInit, so collectors receive encrypted type-2 config and decrypt it only inside the runtime collection path.

Regression evidence

The previous head had no database migration, no response-mask contract, and no mask-resolution behavior. The added contracts cover:

• real H2 rows containing legacy Ollama and HTTP service-discovery plaintext values
• database ciphertext and parameter type after migration
• idempotent handling of already encrypted and unrelated parameters
• API JSON masking without returning plaintext or ciphertext
• unchanged-destination edit behavior
• rejection when a masked secret is combined with a changed destination
• actual WheelTimerTask runtime decryption and protocol placeholder replacement

Validation

• Manager credential, controller, service, template, and validator suites: 53 tests passed.
• Collector runtime credential replacement: 1 test passed.
• Startup source package proof: 24 reactor modules passed.
• Maven Checkstyle, added-line no-CJK scan, and Git whitespace checks passed.

AI assistance: used for draft implementation and test iteration.
Human validation: ran the real H2 migration contract, API and edit-path regressions, collector runtime decryption proof, and the 24-module startup source package proof; all completed successfully.
Risk notes: startup performs a bounded query for only the two reclassified parameter identities. A migration failure aborts before scheduling rather than dispatching an ambiguously typed credential. Exported masked credentials must be re-entered when imported as a new monitor.

[zqr10159]

Author remediation update:

The change now covers existing data as well as new definitions. Stored Ollama and HTTP service-discovery credentials migrate through the existing encryption boundary, API responses remain masked, masked edits preserve the prior secret, runtime use decrypts it, and migration is destination-bound so copied ciphertext is not accepted for another parameter.

Persistence, masking, edit, runtime-decrypt, and legacy-upgrade contracts passed (53 focused tests plus startup packaging). The current GitHub backend, E2E, docs, license, and label checks are green. The requested migration gap is resolved; maintainer review is still required.

[Duansg]

Thanks for tackling this — apiKey and sd_token being stored as text is a real problem worth fixing. However, I think the current implementation has three blocking issues.

1. sd_token can never be restored — the literal ****** gets persisted

MonitorServiceImpl.java:309 only resolves param definitions from monitor.app:

List paramDefines = appService.getAppParamDefines(monitor.getApp());

For an HTTP-SD monitor, app is the discovered application (e.g. linux), while sd_token is declared in app-http_sd.yml and keyed off monitor.scrape. So sd_token never appears in this paramDefines loop, which means:

• the restoresMaskedCredential pre-check never sees it;
• the mask-restore branch (guarded by "password".equals(paramDefine.getType())) never fires.

Meanwhile MonitorParam.fromEntity masks on the stored Param.type, which this PR's migration sets to PARAM_TYPE_PASSWORD. Net effect: edit an HTTP-SD monitor and the string ****** is written back as the real access token, breaking service discovery.

sd_password (app-http_sd.yml:102) is already type: password, so it hits the same path today.

Repro: create an HTTP-SD monitor with an access token → run the migration → open and save the monitor → inspect hzb_param.

Suggestion: resolve param definitions for the scrape/SD app as well, not just monitor.app.

2. Legacy-key ciphertext gets encrypted twice

The new restore branch deliberately accepts ciphertext under the legacy default key:

boolean legacyCiphertext = !AesUtil.DEFAULT_ENCODE_RULES.equals(AesUtil.getDefaultSecretKey())
&& AesUtil.isCiphertext(storedValue, AesUtil.DEFAULT_ENCODE_RULES);

But the restored value continues through the same loop into paramValidatorManager.validate(...), and PasswordParamValidator.java:40 re-checks with the current key:

if (!AesUtil.isCiphertext(passwordValue)) { // false for legacy-key ciphertext
passwordValue = AesUtil.aesEncode(passwordValue); // encrypted a second time

WheelTimerTask.initJobMetrics only performs one AesUtil.aesDecode(), so the collector ends up with the legacy ciphertext instead of the plaintext credential. This affects exactly the key-rotation path the branch was added to support.

Suggestion: skip the password validator for values restored from storage, or make the ciphertext check key-aware.

3. Export → import round-trip is broken for every app with a password param

Export goes exportConfig → getMonitorDto() → MonitorServiceImpl.java:521 setParams() → MonitorParam.fromEntity (masks) → AbstractImExportServiceImpl.java:126, so exported files now contain ******.

Import goes importConfig → validateImportBatch → validate(dto, false), and with isModify != TRUE a masked value hits "The credential mask cannot be used as a new value." — the whole batch fails.

This is not limited to Ollama and HTTP-SD; it applies to MySQL, Redis, SSH, Oracle and every other app with a password param, so the existing export/migrate workflow stops working. Masking secrets in exports may well be the right call, but import needs a matching story (an explicit "include secrets" option, or skip-and-prompt on import).

Non-blocking

ServiceCredentialMigration runs as a CommandLineRunner and rescans the table on every startup. A versioned migration would be a better fit and would align with #4280, which uses Flyway V182.

[zqr10159]

Addressed the review findings in commit 65977a3462.

Changes:

• dynamic service-discovery credential validation now resolves the monitor.scrape definition rather than the monitored application;
• legacy-default-key ciphertext is recognized and is not encrypted a second time;
• public API DTOs remain masked, while configuration export uses a separate ciphertext-preserving path so same-secret export/import never needs plaintext;
• the historical credential migration now writes a version marker and does not rescan the full parameter table on every startup.

Human validation:

• focused migration, validator, monitor service/controller, and JSON import/export suites (57 tests passed);
• git diff --check.

The new-head backend, Maven E2E, license, docs, and label checks are currently queued by GitHub and have not started yet.

AI assistance: used for draft implementation and test iteration.
Risk notes: encrypted exports remain tied to the deployment secret by design.

@Duansg, please re-review this head when convenient.

[Duansg]

Swapping the parameter definitions for non-static monitors drops validation of the app's own params — including credential encryption

validate() previously always resolved definitions from monitor.getApp(). This PR changes it to:

String parameterDefinitionApp = isStatic ? monitor.getApp() : monitor.getScrape();
List paramDefines = appService.getAppParamDefines(parameterDefinitionApp);
...
checkJobFields(parameterDefinitionApp);

The motivation is clear and correct — sd_token lives in app-http_sd.yml, so without this it never enters the loop and the mask-restore branch can't fire. But this is a swap rather than an addition, and the submitted payload contains both sets of params. monitor-new.component.ts:285:

params: info.params.concat(info.advancedParams).concat(info.sdParams),

So for any non-static monitor, the loop now iterates the sd definitions only and never sees the app's own params. Three things follow.

1. App credentials are stored in plaintext and returned unmasked. The loop body ends with:

if (param != null && StringUtils.hasText(param.getParamValue())) {
paramValidatorManager.validate(paramDefine, param);
}

PasswordParamValidator is the only call site of AesUtil.aesEncode under hertzbeat-manager/src/main, and it is also what sets param.setType(CommonConstants.PARAM_TYPE_PASSWORD). If the app's password params are never visited:

• the value is persisted in cleartext, and
• Param.type is never set to PARAM_TYPE_PASSWORD, so the new masking in MonitorParam.fromEntity — which keys off exactly that — never applies either.

Create a mysql (or ssh, redis, oracle …) monitor with scrape=http_sd and its password goes to the database in the clear and comes back out of the API in the clear. That inverts the goal of this PR on the very path it was extended to cover.

2. Required-param validation is lost for the app's fields (port, username, and so on) on non-static monitors. The pre-existing if (!isStatic && "host".equals(field)) continue; is itself evidence that the loop was always meant to run over the app's definitions, with host as the single discovery-supplied exception.

3. checkJobFields now checks the wrong template. It walks job.getMetrics() field names through JexlCheckerUtil.verifyKeywords / verifyStartCharacter / verifySpaces. Passing http_sd instead of the real app means the app template's field names are no longer keyword-checked on this path.

Suggested fix

Concatenate rather than replace — keep monitor.getApp() definitions and append the monitor.getScrape() ones when the monitor is not static, and pass monitor.getApp() to checkJobFields as before (optionally checking the scrape define too). The sd definitions use _sd* / _nacos_sd* style field names with no overlap against any app define, so a plain concatenation needs no dedupe.