qiuyanjun888 commented on issue #17398:
URL: 
https://github.com/apache/dolphinscheduler/issues/17398#issuecomment-4766156217

   I checked the existing issues/PRs and did not find another active issue or 
PR that directly duplicates this one.
   
   I also noticed that there is already a task-log masking mechanism in 
`SensitiveDataConverter`, currently covering cases such as datasource 
`password`, DataX `post jdbc info`, Sqoop `--password`, and K8S `configYaml`. 
The maintainer comment also points to #17378, which suggests that we should 
first decide what should appear in task instance logs.
   
   I think there are two possible implementation directions here:
   
   ## Option A: avoid logging sensitive configuration objects
   
   If the leaked values come from printing a whole configuration object / 
initialization context, the safer fix may be to stop writing that object to the 
task instance log, or replace it with a smaller allowlisted summary. This 
follows the direction of #17378 and avoids relying only on masking after the 
sensitive text has already been formatted.
   
   ## Option B: extend the existing masking rules
   
   If the log lines are legitimate and only specific key/value pairs need 
masking, then we can extend `SensitiveDataConverter` with focused patterns for 
common key names such as:
   
   - `access.key.id`
   - `access.key.secret`
   - `access-key`
   - `secret-key`
   - `accessKey`
   - `secretKey`
   - possibly OSS/S3/COS-specific aliases if they appear in real logs
   
   This should be covered by tests in `SensitiveDataConverterTest`, using the 
actual log formats we want to support, such as JSON-like strings, Java 
`toString()` style, and `key=value` / properties-like output.
   
   ## Questions before implementation
   
   1. Which direction is preferred: suppress the containing configuration log, 
or keep the log and extend masking rules?
   2. Could the reporter provide one real leaked Dinky task log line, with 
values redacted? I could not find a Dinky task plugin in the current 
repository, so a concrete sample would help create a minimal failing test.
   3. Should access key IDs also be fully masked, or only secret keys? Some 
systems treat access key IDs as identifiers rather than secrets, but this issue 
mentions both access-key and secret-key.
   4. Should this first fix be limited to task instance logs, or should it also 
cover remote logging / storage plugin logs?
   
   If the preferred approach is Option B, I can start with focused unit tests 
in `SensitiveDataConverterTest` and then add the minimal masking rules.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to