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

scrub message of secrets #4507

Merged
merged 7 commits into from
Dec 17, 2021
Merged

scrub message of secrets #4507

merged 7 commits into from
Dec 17, 2021

Conversation

emmyoop
Copy link
Member

@emmyoop emmyoop commented Dec 16, 2021

resolves #4508

Description

Scrub bad_package_spec() of secrets it may contain in output.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

@cla-bot cla-bot bot added the cla:yes label Dec 16, 2021
@emmyoop
Copy link
Member Author

emmyoop commented Dec 16, 2021

I have not checked that this works locally yet but wanted to get up the PR.

@jtcohen6
Copy link
Contributor

jtcohen6 commented Dec 17, 2021

To be safe, let's also add scrubbing logic to all potential exceptions raised in the dbt.deps.git + dbt.clients.git modules:

  • warn_or_error
  • raise_dependency_error
  • RuntimeException

There's two other worrying spots where we raise directly, here and here. Best move might be wrapping / converting those into real RuntimeException.

@emmyoop
Copy link
Member Author

emmyoop commented Dec 17, 2021

Converting this one into a RuntimeException caused a lot of test failures that I was having trouble tracking down. My fix still scrubs the output but is a CommandResultError.

@emmyoop emmyoop merged commit 7c46b78 into main Dec 17, 2021
@emmyoop emmyoop deleted the er/bad-package-spec-msg branch December 17, 2021 22:05
emmyoop added a commit that referenced this pull request Dec 17, 2021
* scrub message of secrets

* update changelog

* use new scrubbing and scrub more places using git

* fixed small miss of string conv and missing raise

* fix bug with cloning error

* resolving message issues

* better, more specific scrubbing
@emmyoop emmyoop mentioned this pull request Dec 17, 2021
leahwicz pushed a commit that referenced this pull request Dec 19, 2021
* scrub message of secrets

* update changelog

* use new scrubbing and scrub more places using git

* fixed small miss of string conv and missing raise

* fix bug with cloning error

* resolving message issues

* better, more specific scrubbing
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
* scrub message of secrets

* update changelog

* use new scrubbing and scrub more places using git

* fixed small miss of string conv and missing raise

* fix bug with cloning error

* resolving message issues

* better, more specific scrubbing

automatic commit by git-black, original commits:
  7c46b78
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
* scrub message of secrets

* update changelog

* use new scrubbing and scrub more places using git

* fixed small miss of string conv and missing raise

* fix bug with cloning error

* resolving message issues

* better, more specific scrubbing

automatic commit by git-black, original commits:
  7c46b78
  9cc7a7a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scrub secret env vars from targeted exceptions
4 participants