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

Implement AZP_IGNORE_SECRETS_SHORTER_THAN knob #4073

Conversation

KonstantinTyukalov
Copy link
Contributor

@KonstantinTyukalov KonstantinTyukalov commented Nov 30, 2022

Description: Implemented agent knob which allows to skip short secrets by setting min secret length of each secret.

Note that maximum length of secrets filter is 4

Changelog:

  • Add AZP_IGNORE_SECRETS_SHORTER_THAN knob
  • Updated vss-api-netcore to 0.5.181
  • Update LoggedSecretMasker
  • Added unit tests
  • Some formatting
  • Temporarily disabled removing of .pdb files from build
  • Fixed api conflicts in VssAadToken and VssClientCredentials

Added unit tests:

  • Yes

Attached related issue:

Previous PR - #3962

Checklist:

  • There are no risky dependency updates
  • Changes have been tested

KonstantinTyukalov and others added 25 commits August 26, 2022 15:36
…-agent into users/KonstantinTyukalov/Implement_min_secrets_len_knob
…-agent into users/KonstantinTyukalov/Implement_min_secrets_len_knob
…-agent into users/KonstantinTyukalov/Implement_min_secrets_len_knob
…' of github.com:microsoft/azure-pipelines-agent into users/KonstantinTyukalov/Implement_min_secrets_len_knob
…s package version 0.1.0-20220517.1.1 contains pdb and application doesn't run without it
…-agent into users/KonstantinTyukalov/Implement_min_secrets_len_knob
@KonstantinTyukalov KonstantinTyukalov changed the title Implement AGENT_MIN_SECRET_LENGTH knob Implement AZP_IGNORE_SECRETS_SHORTER_THAN knob Nov 30, 2022
@StingyJack
Copy link

StingyJack commented Dec 1, 2022

@KonstantinTyukalov please be sure that the solution is not going to be worse than the original problem. Nobody asked for secrets to be unmasked - that is worse than the problem reported where we can guess the secret values.

What if I made a build pipeline that used secrets I am not supposed to know and set the knob to 1000? I'm going to see all of the things I'm not supposed to see.

Also make sure its a feasible and reasonable suggestion. This proposal will require my org to put a variable into like 4K build pipelines. We should have to do nothing and change nothing - no knobulation should be required to have secrets masked appropriately. It just needs to work properly.

I know this is not an easy ask but its possible and there really are no shortcuts that are OK when it comes to security and diagnostics. I dont normally comment on another org's PR's but it doesnt look like you are incorporating what is said in the issue thread or the dev comm issues into these solutions, and any potential reviewer needs to understand that

@StingyJack
Copy link

@kirill-ivlev please take note of the comment above. This does not solve the problem and actually makes the problem worse.

@KonstantinTyukalov KonstantinTyukalov merged commit 4debafc into master Dec 6, 2022
@kirill-ivlev
Copy link
Contributor

@StingyJack
thank you for your input, regarding long secrets we limited this knob to a maximum value of 4 characters.
For now, it should resolve cases when short secrets might leak from the logs.
We will check the other ways of resolving it in a more elegant way in the future.

LiliaSabitova added a commit that referenced this pull request Jan 6, 2023
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.

None yet

7 participants