properly escape quotes in passwords by calling to_ruby

[evgeni]

database passwords can contain special characters, especially " and '
so we can't just print the value of the field enclosed by double quotes
as that would break whenever the user uses a literal " in their password

using to_ruby here and not to_yaml, as the former gives us correct escaping
without the whole --- and \n enclosing that to_yaml forces.
using to_yaml would require to pass the whole config hash to it

[ekohl]

I just realized this also goes for the pulpcore PR I already merged

[ekohl]

On my phone so tricky to check, but can this be a Sensitive and does it handle that well?

[evgeni]

Technically, it can. No idea how to test this tho 🙈

In pulpcore, we're lucky, it's a String.

[bastelfreak]

I think you can pass Sensitive to epp and it will automatically unwrap it, so that's fine.

[evgeni]

added a test, let's see

[evgeni]

lol. init.pp says db_password is a String, while the epp file allows Sensitive[String]:

puppet-foreman/manifests/init.pp

Line 229 in b1a2286

String[1] $db_password = $foreman::params::db_password,

puppet-foreman/templates/database.yml.epp

Line 9 in b1a2286

Variant[Undef, String[1], Sensitive[String[1]]] $password,

[evgeni]

going to drop the test (as it's failing now) and add sensitive support later

[evgeni]

#1190 says this does not work for Sensitive

[evgeni]

and this not even works properly, what the hell

[evgeni]

oh I'm stupid, I was running a non-patched installer 🙈

[alexjfisher]

to_ruby? Does this really work with all inputs? Don't you need something with specific yaml knowledge?

It'd be a stupid password, but if your password was previously the string no, it would have been correctly quoted. Now it wouldn't be and yaml would treat it as a boolean.

[alexjfisher]

to_ruby? Does this really work with all inputs? Don't you need something with specific yaml knowledge?

It'd be a stupid password, but if your password was previously the string no, it would have been correctly quoted. Now it wouldn't be and yaml would treat it as a boolean.

or maybe it will be quoted actually. Just looks a bit funky.

[evgeni]

It does work, partially by accident (don't look at the implementation in stdlib).

Yes to_yaml would be better, but that requires more work as then we need to generate the whole hash with it.

[alexjfisher]

to_yaml for the whole file would have its advantages, but you wouldn't be able to include the comments you currently do.

I'm thinking a new stdlib function just for escaping strings in a yaml specific way would be beneficial. Something that uses the low level Psych API.

For a PoC...

require 'psych'
require 'stringio'

def string_to_yaml_escaped_string(str)
  out = StringIO.new
  emitter = Psych::Emitter.new(out)

  emitter.start_stream(Psych::Nodes::Stream::UTF8)
  emitter.start_document([], [], true) # implicit flag set to true to suppress document start marker

  emitter.scalar(
    str,
    nil,
    nil,
    false,
    true,
    Psych::Nodes::Scalar::DOUBLE_QUOTED
  )

  emitter.end_document(true) # implicit flag set to true to suppress document end marker
  emitter.end_stream

  out.string
end

str = 'ao&"$bc\ndef'
puts string_to_yaml_escaped_string(str)

[ekohl]

I had hoped there was some flag to omit the document header and an easier API to write out fragments. This doesn't deal with any datastructure, does it?

[alexjfisher]

I had hoped there was some flag to omit the document header and an easier API to write out fragments. This doesn't deal with any datastructure, does it?

Unfortunately, it looks like you've got to use the low level API to get this control. YAML scalars are strings and numbers, not arrays and hashes, so this would only really be useful for safely encoding individual strings. You could make the style configurable though using a different style constant from this list. https://docs.ruby-lang.org/en/master/Psych/Nodes/Scalar.html

[ekohl]

I think in the short term this is good enough. If we need to wait for a new stdlib function it'll take too long before it's really available and I expect this will simply always quote strings instead of sometimes, which I actually prefer. The bare strings that sometimes turn into booleans or other data types scare me.