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

Re-enable warning when a secret config is read as a non-secret #7127

Open
justinvp opened this issue May 24, 2021 · 6 comments
Open

Re-enable warning when a secret config is read as a non-secret #7127

justinvp opened this issue May 24, 2021 · 6 comments

Comments

@justinvp
Copy link
Member

justinvp commented May 24, 2021

We recently added a warning when reading a secret config value as a non-secret (#6896 #7078 #7079 #7080). However, one issue we've run into is that many of our provider SDKs greedily read config values in a config module using the non-secret config APIs, which results in a warning if such values happen to be saved as secrets -- a warning that users can't really do anything about (see #7126). We're going to temporarily disable the new warning for now. This tracks re-enabling the warning without this issue.

@farvour
Copy link

farvour commented Jun 14, 2021

Thank you. This was more of an annoyance than anything. Pulumi already detects with a value is secret encoded via the presence of the secure key on a value.. when the future is materialized. I don't see the purpose in telling users to specifically require a secret anyway since the underlying program should not care if it comes from a configuration whether it was sensitive or not [unless they care to check in the code themselves].

Examples where this was a nuisance:

warning: Configuration 'aws-rds-py:aws_rds_instances' value is a secret; use `require_secret_object` instead of `require_object

But we intentionally nest secrets within a value of the mapping/object:

    foobar:
      master_password:
        secure: v1:zspecialsecuretlao23thj08923fn092n10r91h20jn109cn0jt09j089ajsd90th98h23589
      parameter_group_parameters:
      - apply_method: pending-reboot
        name: max_connections
        value: 200```

@justinvp
Copy link
Member Author

I don't see the purpose in telling users to specifically require a secret

If you read the object via require_object, the object will be deserialized and available to the program with plaintext values. master_password will be plaintext and passing it as an input to any resource will likely result in the value being stored in the state unencrypted. To avoid this, use require_object_secret, which returns the object wrapped as Output[T] marked as a secret, and pass individual values in the object elsewhere with apply (e.g. foobar.apply(lambda fb: fb.master_password)) to maintain the value's secretness. Alternatively, continue using require_object, but use output.Secret(foobar.master_password) before passing it as an input to a resource, if you want it to to remain a secret as it flows throughout the rest of the system.

@farvour
Copy link

farvour commented Jun 15, 2021 via email

@justinvp
Copy link
Member Author

Part of addressing this will be seeing if we can make the variables in the config module lazy (#5582) as well as possibly providing alternative APIs that return the values as secrets.

@leviathanbadger
Copy link

Giving my two cents:

The issue occurs in both directions:

  • Using require when the config value is a secret.
  • Using requireSecret when the config value is not a secret. !!!

Both of these scenarios indicate that the config producers (users) and consumers (Pulumi programs or providers) are at odds about whether or not the value is a secret. To me, this isn't a "warning"-level offense; it's a blow-up-in-your-face offense. In the worst case, this indicates the secret is stored in plaintext in the repository; and in the best case it indicates the project or provider might not be cognizant of keeping the secret values safe.

Users can control their config and Pulumi programs, but not necessarily providers; so I'd love if this could be configured as either a warning or an error, on a per-stack basis.

Warnings might be annoying, but I'd much rather be annoyed than miss a potential security vulnerability.

@aureq
Copy link
Member

aureq commented Oct 11, 2021

Furthermore to this, using the non-secret getter allows for the result to be exported as a plain text value in the stack output (no warnings, no errors).

Maybe some thoughts here:

  1. a non-secret getter retrieving a secret: 1) generate a warning and 2) maintain the secret aspect of the value so it can't be directly revealed in the stack output.
  2. secret-getter on plain text: 1) should hard fail since the supposed secret is actually stored in plain text which is an anti-pattern

On 1. an extra option could be provided to allow a silent operation. The secret state of the value would be dependent of the way the value is stored in the configuration file.

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

No branches or pull requests

4 participants