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

prTitle and prRef are now hashed - removed payload dump on log #1606

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

joshkay10
Copy link
Contributor

What's in this PR?
Adding prTitle and prRef to the sensitive data list

Removing a complete payload object in a log

Why
prTitle and prRef is UGC and needs to be hidden

an entire payload it too much logging! we need to be targeted, payload is ok during debug but not as a default, we cannot always be sure what is in it and could potentially leak PII/UGC

Added feature flags
nope

Affected issues
nope

How has this been tested?
locally only, changes are minor enough that I feel this is sufficient.

Whats Next?
Keep hunting for UGC!

@joshkay10 joshkay10 requested a review from a team as a code owner September 19, 2022 00:08
@@ -4,7 +4,7 @@ import bformat from "bunyan-format";
import { DEBUG } from "bunyan";
import { createHashWithSharedSecret } from "utils/encryption";

const SENSITIVE_DATA_FIELDS = ["jiraHost", "orgName", "repoName", "userGroup", "userGroup", "aaid", "username"];
const SENSITIVE_DATA_FIELDS = ["jiraHost", "orgName", "repoName", "userGroup", "userGroup", "aaid", "username", "prTitle", "prRef"];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we find more UGC/PII we continue to add to this list.
Its manageable at the moment but may need to consider moving it to a config file of some sort if it gets too massive!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants