Sanitize exported stack state by scrubbing secrets #107
Conversation
thomas11
requested review from
danielrbradley,
t0yv0,
VenelinMartinov and
a team
| "github.com/pulumi/pulumi/sdk/v3/go/common/apitype" | ||
| ) | ||
|
|
||
| const plaintextSub = "REDACTED BY PROVIDERTEST" |
There was a problem hiding this comment.
Won't this break the tests upon reading the state back?
There was a problem hiding this comment.
Perhaps some tests are sensitive to the secret values and some aren't. Make it optional so we can opt in or out on a per-test basis. I'd prefer eventually pulumi/pulumi#17158 as I have a hunch that "official" encrypting should also be able to decrypt upon reading, given a fixed secrets provider.
There was a problem hiding this comment.
pulumi/pulumi#17158 would be great, but we should never persist and commit secrets. GH will reject them on push anyway. Not sure how a test could be not sensitive to them?
There was a problem hiding this comment.
With 17158 they're persisted in encrypted form, but you're right that it might be that the passphrase or encrypting key is plaintexted somewhere in our libs, we'd need to sanity check them. To elaborate the test being sensitive, I mean that you might get a test failing if you replace a value with "REDACTED BY PROVIDERTEST" sometimes, it would seem? If the test actually cares about the value? Maybe it's rare in practice.
There was a problem hiding this comment.
The passphrase is definitely hard coded. I think, for the time being a simple hard-coded value is fine for most situations. If this causes an issue in the future then we could try do do something smarter where we mirror the shape of the secret data while entirely replacing it. E.g. if it's not a string but instead an object, then sanitise all the values within the object. In practice, in tests I think 99% of secrets will be individual strings, and might not even be used.
There was a problem hiding this comment.
Not sure, IMO the cleanest is pulumi/pulumi#17158 + keeping your passphrase in GH secrets instead of sources gets you there as Pulumi already has the sanitization machinery for stack state. As to storing gRPC logs, encrypt-at-rest could do the job.
There was a problem hiding this comment.
But thanks for getting something working quickly!
Sanitize by default when previewing provider upgrades.
|
This PR has been shipped in release v0.1.2. |
When upgrade tests persist stack.json, it contains secrets, making it unsuitable for committing. This PR adds a sanitization step that scrubs secrets, as identified by the p/p signature, from the state.
Fixes #106