Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add secret masking for the telemetry.publish command #4461

Closed

Conversation

DenisNikulin5
Copy link
Contributor

@DenisNikulin5 DenisNikulin5 commented Oct 6, 2023

In this PR, I added the use of the SecretMasker for telemetry.publish command to prevent leaking tokens (including JWT).

By default, the SecretMasker hasn't used cred scan patterns, they could be enabled only using the environment variable.
To use them via a feature flag, I have to move the logic of using the knob from the HostContext to the ExecutionContext and added a runtime source for this knob as well.

P.S. This change must be under the feature flag definitely since I found a comment where it was written that the cred scan could affect the agent performance. But I couldn't confirm this after testing the changes.

Testing:
To test the changes I used the DevFabric environment (Microsoft DevBox).
Testing steps:

  1. Build and deploy DevFabric from the branch with the DistributedTask.Agent.MaskUsingCredScanRegexe feature flag.
  2. Build and connect the agent to the test organization from the PR branch.
  3. Add the test pipeline from this description and run it. It contains explicit telemetry.publish command with an exposed JWT token.
  4. Check the results in the VSODev database, the CustomerIntelligence table. The result could be filtered by the name of the Feature. It will contain the RAW JWT since the feature flag is turned off.
  5. Set the feature flag using LightRail tfs. Navigate to /tfs/DevFabric and run "Set-FeatureFlag -featureName DistributedTask.Agent.MaskUsingCredScanRegex -State ON".
  6. Repeat 3-4 steps. The results will contain the masked JWT token.
  • Test pipeline:
trigger:
- master

pool: Default

steps:
- powershell: |
    Write-Host "##vso[telemetry.publish area=AzurePipelinesAgent;feature=denikulin_Test]{"jwt" : "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTY5NTg4OTY3NCwiZXhwIjoxNjk1ODkzMjc0fQ.hdiPa_gTPATGWr6-5gF25QEMAWmHU2MXZ4ED6-w0HW8"}"
  displayName: 'Run script'

Results:

  • Before the change:
    image
  • After the change:
    image

@DenisNikulin5 DenisNikulin5 requested review from a team as code owners October 6, 2023 11:40
@DenisNikulin5

This comment was marked as resolved.

@DenisNikulin5 DenisNikulin5 deleted the users/denikulin/mask-secrets-for-telemetry branch March 12, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants