(PA-8346) Preserve sensitive parameters

[tvpartytonight]

When converting Puppet::Resource to an instance of Puppet::Type, the list of sensitive_parameters was lost. So later Puppet::Parameter#sensitive returned false.

There are multiple Puppet::Resource and Hash conversions taking place between the resource api and puppet:

1. resource-api calls Puppet::Resource#to_hash (line 78)
2. resource-api calls Puppet::Type#initialize(Hash) (line 108)
3. Puppet::Type#initialize calls hash2resource(Hash) returning Puppet::Resource[1]
4. Puppet::Type#initialize copies sensitive_parameters from Puppet::Resource to self[2]

We have to ensure the list of sensistive_parameters is preserved through this call chain, so that the resulting Puppet::Parameter#sensitive instance variables are set correctly.

[1] https://github.com/puppetlabs/puppet-private/blob/8361be778eb8bdda8b7335a98a4e7d11557f5f68/lib/puppet/type.rb#L2320
[2] https://github.com/puppetlabs/puppet-private/blob/8361be778eb8bdda8b7335a98a4e7d11557f5f68/lib/puppet/type.rb#L2347

Summary

Provide a detailed description of all the changes present in this pull request.

Additional Context

Add any additional context about the problem here.

• Root cause and the steps to reproduce. (If applicable)
• Thought process behind the implementation.

Related Issues (if any)

Mention any related issues or pull requests.

Checklist

• 🟢 Spec tests.
• 🟢 Acceptance tests.
• Manually verified.

[tvpartytonight]

Looking at the commits here, I believe this is the only one that needs to be add to main for parity...

[Copilot]

Pull request overview

Ensures sensitive_parameters is preserved when the Resource API converts a Puppet::Resource into a Puppet::Type instance, so that downstream Puppet::Parameter#sensitive is correctly set for sensitive attributes (preventing sensitive values from being persisted in transaction storage).

Changes:

• Preserve sensitive_parameters by re-attaching them to the attributes hash before invoking Puppet::Type initialization.
• Add spec coverage verifying Puppet::Parameter#sensitive is true when sensitive parameters are present, and not set when they are absent.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
lib/puppet/resource_api.rb	Propagates sensitive_parameters through the Puppet::Resource → Hash → Puppet::Type#initialize conversion chain.
spec/puppet/resource_api_spec.rb	Adds regression tests for Puppet::Parameter#sensitive behavior with/without sensitive_parameters.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gavindidrichsen]

Post-merge review: PA-8346 sensitive parameters fix

Four findings from reviewing this PR — the fix addresses a real cleartext leak, and these are suggestions to harden it further.

Finding 2 (Low): canonicalize can silently undo sensitive re-wrapping

The initialization sequence re-wraps sensitive values before calling canonicalize. If a provider's canonicalize implementation unwraps Sensitive objects (e.g. to normalize a password hash format), the re-wrapping is silently undone. No guard or re-wrap exists after canonicalize returns. This is a latent bug — no current provider likely does this, but the ordering makes it possible and the failure mode would be silent cleartext leakage. Consider either re-wrapping after canonicalize returns or documenting this as a provider contract constraint.

Finding 4 (High): No test covers the primary fix path

The PR tests verify the "no sensitive parameters" path and re-wrapping of unwrapped values, but no test covers the actual bug that was fixed: a Puppet::Resource input with non-empty sensitive_parameters should result in parameter(:secret).sensitive == true. A test like this would directly verify the fix:

it 'preserves sensitive_parameters so Puppet::Parameter#sensitive returns true' do
  resource = Puppet::Resource.new('test_sensitive', 'foo')
  resource[:secret] = Puppet::Pops::Types::PSensitiveType::Sensitive.new('s3cret')
  resource.sensitive_parameters = [:secret]

  instance = type_class.new(resource)
  expect(instance.parameter(:secret).sensitive).to be true
end

Without this, a future regression could reintroduce the cleartext leak with no test catching it.

[gavindidrichsen]

Finding 1 (Medium): Fix couples to hash2resource undocumented internals

This line works because Puppet::Type#hash2resource explicitly deletes :sensitive_parameters from the hash before processing remaining keys (puppet/type.rb:1213). That deletion behavior is an undocumented internal — if Puppet core changes how hash2resource handles this key, this fix silently breaks.

A more robust alternative would set sensitive_parameters explicitly after super completes, using Puppet's public API:

super(attributes)
set_sensitive_parameters(sensitives) unless sensitives.empty?

set_sensitive_parameters is a public method on Puppet::Type that iterates the list and marks each parameter as sensitive. This removes the dependency on hash2resource's internal key-deletion behavior.

Note: you can't remove set_sensitive_parameters from hash2resource itself — that's Puppet core code shared by all type implementations (native types, defined types, etc.), not just Resource API types. But the Resource API can safely add its own post-super call. The double-call (once inside hash2resource, once explicitly) is idempotent and harmless.

Trade-off is minimal — robustness over a negligible duplicate call.

[gavindidrichsen]

Finding 3 (Low): This test stubs an impossible state

This test creates a Puppet::Resource stub where sensitive_parameters returns [] but to_hash returns { secret: Puppet::Pops::Types::PSensitiveType::Sensitive.new("a password value") }. In real Puppet, if a value is wrapped in Sensitive in the catalog, its parameter name always appears in sensitive_parameters. The empty list + wrapped value combination can't happen.

The test passes but doesn't verify meaningful behavior — it's testing a code path that can't be reached in production. Consider either:

• Removing the Sensitive wrapper from the stub (plain string for :secret) to represent a genuinely non-sensitive parameter, or
• Adding the parameter name to sensitive_parameters to represent the realistic case (which then becomes a duplicate of the existing test above)