Skip to content

[security] Sensitive type for gitlab_rails#487

Open
m8t wants to merge 2 commits intovoxpupuli:masterfrom
m8t:m8t/sensitive
Open

[security] Sensitive type for gitlab_rails#487
m8t wants to merge 2 commits intovoxpupuli:masterfrom
m8t:m8t/sensitive

Conversation

@m8t
Copy link
Copy Markdown

@m8t m8t commented Feb 16, 2026

Pull Request (PR) description

LDAP crendentials can be passed through gitlab_rails, which will appear in clear in Puppetdb for the class attributes.

This commit permits to hide from the class attributes inside PuppetDB, but still renders the password visible inside PuppetDB with the generated content of the template.

This Pull Request (PR) fixes the following issues

#486

@TheMeier
Copy link
Copy Markdown
Contributor

REFERENCE.md is outdated; to regenerate: bundle exec rake strings:generate:reference

@m8t
Copy link
Copy Markdown
Author

m8t commented Feb 17, 2026

REFERENCE.md is outdated; to regenerate: bundle exec rake strings:generate:reference

Indeed, updated!

LDAP crendentials can be passed through gitlab_rails, which will appear
in clear in Puppetdb for the class attributes.

This commit permits to hide from the class attributes inside PuppetDB,
but still renders the password visible inside PuppetDB with the
generated content of the template.
Comment thread templates/gitlab.rb.erb Outdated
<%- if @gitlab_rails -%>
<%-
if @gitlab_rails
if @gitlab_rails.is_a?(Puppet::Pops::Types::PSensitiveType::Sensitive)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this conditional needed? I thought perhaps for some time, unwrap was a noop if the data wasn't Sensitive?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

puppet apply -t -e 'notice("foo".unwrap)'
Notice: Scope(Class[main]): foo

Copy link
Copy Markdown
Author

@m8t m8t Feb 17, 2026

Choose a reason for hiding this comment

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

On a type String it bailed out with an error, none-existing method

edit fails with type Hash, String is fine in ERB as long as you call it through scope.call_function

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thing is, in a ERB template it fails, in Puppet code it's fine, I think.

Copy link
Copy Markdown
Author

@m8t m8t Feb 17, 2026

Choose a reason for hiding this comment

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

Tested and fails both by calling Puppet function or plain ERB:

gitlab_rails = scope.call_function('unwrap', @gitlab_rails)

  Filepath: /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/pops/functions/dispatcher.rb
  Line: 40
  Detail: 'unwrap' expects 1 argument, got 16
gitlab_rails = @gitlab_rails.unwrap

  Filepath: /etc/puppetlabs/code/environments/production/modules/gitlab/templates/gitlab.rb.erb
  Line: 68
  Detail: undefined method `unwrap' for #<Hash:0x37327d95>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and fails both by calling Puppet function or plain ERB:

gitlab_rails = scope.call_function('unwrap', @gitlab_rails)

Close! But the second argument needs to be an array of arguments to the function.
ie scope.call_function('unwrap', [@gitlab_rails]) should work I believe.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oddly this works fine puppet apply -t -e 'notice({"foo"=>0,"bar"=>1}.unwrap)'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Works fine through Puppet code only to unwrap a type Hash.

diff --git a/manifests/omnibus_config.pp b/manifests/omnibus_config.pp
index bcef084..1720747 100644
--- a/manifests/omnibus_config.pp
+++ b/manifests/omnibus_config.pp
@@ -27,3 +27,3 @@ class gitlab::omnibus_config (
   $gitlab_pages = $gitlab::gitlab_pages
-  $gitlab_rails = $gitlab::gitlab_rails
+  $_gitlab_rails = $gitlab::gitlab_rails
   $gitlab_sshd = $gitlab::gitlab_sshd
@@ -121,2 +121,3 @@ class gitlab::omnibus_config (
     } else {
+      $gitlab_rails = $_gitlab_rails.unwrap
       file { $config_file:

works fine. I can send a new patch, avoids having type check in ERB

Copy link
Copy Markdown
Author

@m8t m8t Feb 17, 2026

Choose a reason for hiding this comment

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

Close! But the second argument needs to be an array of arguments to the function. ie scope.call_function('unwrap', [@gitlab_rails]) should work I believe.

Ah true, thought about it, and forgot, I prefer this variant. I already sent a different commit, where unwrap is called from the manifest, reused in the template. In the end both are fine, and the odd condition around type checking is not needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Either works for me. I've approved it as-is, but if you wanted to change it, I'm happy to reapprove.

@alexjfisher alexjfisher added the enhancement New feature or request label Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants