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

typo fix in security hardening #1036

Merged
merged 4 commits into from
Nov 5, 2020
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Secrets use [Libsodium sealed boxes](https://libsodium.gitbook.io/doc/public-key

To help prevent accidental disclosure, {% data variables.product.product_name %} uses a mechanism that attempts to redact any secrets that appear in run logs. This redaction looks for exact matches of any configured secrets, as well as common encodings of the values, such as Base64. However, because there are multiple ways a secret value can be transformed, this redaction is not guaranteed. As a result, there are certain proactive steps and good practices you should follow to help ensure secrets are redacted, and to limit other risks associated with secrets:

- **Never use structured data as a secret**
- **Never use unstructured data as a secret**
Copy link
Contributor

@lucascosti lucascosti Nov 4, 2020

Choose a reason for hiding this comment

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

@brentjo This change makes sense to me, but I wanted to also get your 👀 just to make sure 🙂

Copy link
Member

Choose a reason for hiding this comment

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

👋 @lucasilverentand thanks for opening this up, and for catching this. Structured was the intended word here though, so the typo's actually with how we go on to say Unstructured in the first sentence. As in: don't use "structured" data such as a blob of JSON as a secret, because secret redaction mostly looks for exact matches to the configured secrets, so doing so can cause redaction to fail. You want to configure the secret in its rawest form that you would want redacted in the event that value ever gets written to the logs.

"Structured" was the best term I could think of that captures that meaning, but definitely open to suggestions on that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brentjo that makes a lot of sense. I do agree with you that the term makes sense. Is there any chance someone else could make the change? Since I'm currently a bit busy with other projects.

Copy link
Member

Choose a reason for hiding this comment

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

@lucasilverentand Can do! Thanks again for bringing this to our attention!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brentjo glad I could help! 😇

- Unstructured data can cause secret redaction within logs to fail, because redaction largely relies on finding an exact match for the specific secret value. For example, do not use a blob of JSON, XML, or YAML (or similar) to encapsulate a secret value, as this significantly reduces the probability the secrets will be properly redacted. Instead, create individual secrets for each sensitive value.
- **Register all secrets used within workflows**
- If a secret is used to generate another sensitive value within a workflow, that generated value should be formally [registered as a secret](https://github.com/actions/toolkit/tree/main/packages/core#setting-a-secret), so that it will be redacted if it ever appears in the logs. For example, if using a private key to generate a signed JWT to access a web API, be sure to register that JWT as a secret or else it won’t be redacted if it ever enters the log output.
Expand Down