flowey: make values secret, not variables #1338
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Nice! Pushing secrecy to the value level seems like it should work better than having it at the variable level. My thinking was to lift secrecy to the type system (similar to claimed/not-claimed), but wrangling that extra type param would be far more annoying than this approach. Also, glad to see some the cleanup in/around some of the more gnarly bits of internal plumbing! |
|
Yeah, I had thought about putting it into the type system so that callers were aware they were using something that (might be) secret. But I think that makes it hard to auto-propagate the secret marker without playing lots of type system games or making everything extremely explicit. This approach seems the cleanest. |
justus-camp-microsoft
previously approved these changes
May 13, 2025
Currently, a `ReadVar`/`WriteVar` pair can be marked as secret, in which case flowey is careful never to display it in logs. To mark a variable, the user must remember to create the variable pair with `new_secret_var()`, and the user must ensure that users of the variable to do not rewrite its contents into some other, non-secret variable. This is hard to do accurately, especially as we change the code to create more variables implicitly. Change the model so that _variables_ are not secret but their _values_ can be--when any variable is written to, the caller can specify that the value is secret. Propagate this to readers of the variable, even if this variable is converted into and back from a CI environment variable. By default, be conservative: once a Rust step reads a secret from a variable, treat all future values written as secret. Add specific `write_secret` and `write_non_secret` methods for overriding this default.
benhillis
approved these changes
May 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, a
ReadVar/WriteVarpair can be marked as secret, in which case flowey is careful never to display its value in logs. To mark a variable as such, the user must remember to create the variable pair withnew_secret_var(), and the user must ensure that users of the variable to do not rewrite its contents into some other, non-secret variable. This is hard to do accurately, especially as we change the code to create more variables implicitly (via<foo>v-style methods such asreqvandemit_rust_stepv).Change the model so that variables are not secret but their values can be--when any variable is written to, the caller can specify that the value is secret. Propagate this to readers of the variable, even if this variable is converted into and back from a CI environment variable.
By default, be conservative in marking values as secret: once a Rust step reads a secret value from a variable, mark all future values written by that step as secret. Add specific
write_secretandwrite_non_secretmethods for overriding this default.