remove more sensitive logs in pipeline/responseProcessor logs #2130
Conversation
Signed-off-by: Xun Zhang <[email protected]>
Zhangxunmt
requested review from
b4sjoo,
dhrubo-os,
jngz-es,
model-collapse,
rbhavna,
ylwu-amzn,
zane-neo,
austintlee,
HenryL27 and
sam-herman
as code owners
Zhangxunmt
had a problem deploying
to
ml-commons-cicd-env
GitHub Actions
Error
Zhangxunmt
had a problem deploying
to
ml-commons-cicd-env
GitHub Actions
Failure
Zhangxunmt
had a problem deploying
to
ml-commons-cicd-env
GitHub Actions
Error
Zhangxunmt
temporarily deployed
to
ml-commons-cicd-env
GitHub Actions
Inactive
Zhangxunmt
temporarily deployed
to
ml-commons-cicd-env
GitHub Actions
Inactive
Zhangxunmt
temporarily deployed
to
ml-commons-cicd-env
GitHub Actions
Inactive
| @@ -120,7 +120,6 @@ public SearchResponse processResponse(SearchRequest request, SearchResponse resp | |||
| if (timeout == null || timeout == GenerativeQAParameters.SIZE_NULL_VALUE) { | |||
| timeout = DEFAULT_PROCESSOR_TIME_IN_SECONDS; | |||
| } | |||
| log.info("Timeout for this request: {} seconds.", timeout); | |||
There was a problem hiding this comment.
Why this should be treated as a sensitive information?
There was a problem hiding this comment.
This one is not directly pointed out in the report but the timeout parameter is a configuration provided by customer in their pipeline. So to be on the safe side we should avoid logging it. Just in case this could be a follow-up in the next round.
There was a problem hiding this comment.
I don't quite agree with the reason. But approving anyway as this is final hour....
There was a problem hiding this comment.
The system and user prompts are returned directly by just the GET search/pipeline endpoint - they're not meant to be secure. Those variables don't include any index or user data - basically just templates.
i.e. I don't think any of the GenerativeQAResponseProcessor log-deletions are necessary?
Approving but would like to keep these logs if possible
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2130 +/- ##
============================================
- Coverage 81.86% 81.86% -0.01%
- Complexity 5644 5659 +15
============================================
Files 543 543
Lines 22790 22843 +53
Branches 2333 2347 +14
============================================
+ Hits 18658 18700 +42
- Misses 3195 3203 +8
- Partials 937 940 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Zhangxunmt
temporarily deployed
to
ml-commons-cicd-env
GitHub Actions
Inactive
Zhangxunmt
temporarily deployed
to
ml-commons-cicd-env
GitHub Actions
Inactive
Zhangxunmt
temporarily deployed
to
ml-commons-cicd-env
GitHub Actions
Inactive
Signed-off-by: Xun Zhang <[email protected]> (cherry picked from commit 896caac)
Signed-off-by: Xun Zhang <[email protected]> (cherry picked from commit 896caac)
…#2133) Signed-off-by: Xun Zhang <[email protected]> (cherry picked from commit 896caac) Co-authored-by: Xun Zhang <[email protected]>
…#2132) Signed-off-by: Xun Zhang <[email protected]> (cherry picked from commit 896caac) Co-authored-by: Xun Zhang <[email protected]>
…arch-project#2130) Signed-off-by: Xun Zhang <[email protected]>
Description
Pen testers still pointed out "Finding: Search Pipeline / Response Processor Logs Contains Sensitive Data" not resolved. This PR is to delete the leftovers in the sensitive logs required by the pen tests. #1965
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.