cloudfoundry / java-buildpack

Cloud Foundry buildpack for running Java applications
Apache License 2.0
437 stars 2.58k forks source link

Handle Dynatrace API Token in the sanitizer #974

Closed rbamberger closed 1 year ago

rbamberger commented 1 year ago

Masking the private part of the API Token

Fix for Issue #967

rbamberger commented 1 year ago

I have also already thought of implementing a general solution. However, this is a problem for which we need a solution quickly. This was the least invasive fix from our point of view.

TimGerlach commented 1 year ago

To speed things up a little bit I would also appreciate if this could be implemented in two phases so that we get the Dynatrace API token masking with this PR and a more general solution with a follow-up PR. We do have some time pressure in the context of security requirements.

dmikusa commented 1 year ago

My vote would be for a proper fix, I don't think it's a huge request. That would also add in a solution that's actually generic, right now, this PR is modifying core parts of the buildpack with DT-specific logic.

meibensteiner commented 1 year ago

Or this PR is merged, satisfying a shared customer of VMware and Dynatrace. This way SAP can continue with their project. In return we will commit to adding this sanitizer in our own time, without throwing our current planning overboard.

TimGerlach commented 1 year ago

Hi @dmikusa, Is there any chance to get another PR review from you?

meibensteiner commented 1 year ago

@dmikusa Is there any new development here? We need to get this merged for a customer asap!

dmikusa commented 1 year ago

@meibensteiner Hi, posted some thoughts. Sorry for the delay. I don't work full time on this anymore, just spare time/OSS capacity.

@pivotal-david-osullivan - FYI for integration testing/release and also for a second opinion.

rbamberger commented 1 year ago

@dmikusa Hi, I've removed the special handling for the Dynatrace token. Please have another look.

meibensteiner commented 1 year ago

@pivotal-david-osullivan Or maybe you could have a look at it?

TimGerlach commented 1 year ago

Dear @dmikusa, @pivotal-david-osullivan, Can you please consider another review of this Pull Request? We're currently stuck in security audits and need to provide an ETA for masking the tokens in the staging logs any time soon.

Reviewing the PR should probably not take longer than 10 minutes.

Thanks for your support!

pivotal-david-osullivan commented 1 year ago

Yes, we'll take another look at this ASAP, thanks!