Only redact the non-sensitive bits of build args and env vars.

* `-e [REDACTED]` → `-e SOME_SECRET=[REDACTED]`
* Replaces `Utils.redact` with `Utils.sensitive` to clarify that we're
  indicating redactability, not actually performing redaction.
* Redacts from YAML output, including `mrsk config` (fixes #96)
This commit is contained in:
Jeremy Daer
2023-03-23 03:49:05 -07:00
parent f851d6528d
commit c137b38c87
10 changed files with 102 additions and 26 deletions

View File

@@ -119,7 +119,7 @@ class Mrsk::Cli::Main < Mrsk::Cli::Base
desc "config", "Show combined config (including secrets!)" desc "config", "Show combined config (including secrets!)"
def config def config
run_locally do run_locally do
puts MRSK.config.to_h.to_yaml puts Mrsk::Utils.redacted(MRSK.config.to_h).to_yaml
end end
end end

View File

@@ -1,6 +1,6 @@
module Mrsk::Commands module Mrsk::Commands
class Base class Base
delegate :redact, :argumentize, to: Mrsk::Utils delegate :sensitive, :argumentize, to: Mrsk::Utils
attr_accessor :config attr_accessor :config

View File

@@ -28,7 +28,7 @@ class Mrsk::Commands::Builder::Base < Mrsk::Commands::Base
end end
def build_args def build_args
argumentize "--build-arg", args, redacted: true argumentize "--build-arg", args, sensitive: true
end end
def build_secrets def build_secrets

View File

@@ -2,7 +2,7 @@ class Mrsk::Commands::Registry < Mrsk::Commands::Base
delegate :registry, to: :config delegate :registry, to: :config
def login def login
docker :login, registry["server"], "-u", redact(lookup("username")), "-p", redact(lookup("password")) docker :login, registry["server"], "-u", sensitive(lookup("username")), "-p", sensitive(lookup("password"))
end end
def logout def logout

View File

@@ -2,11 +2,12 @@ module Mrsk::Utils
extend self extend self
# Return a list of escaped shell arguments using the same named argument against the passed attributes (hash or array). # Return a list of escaped shell arguments using the same named argument against the passed attributes (hash or array).
def argumentize(argument, attributes, redacted: false) def argumentize(argument, attributes, sensitive: false)
Array(attributes).flat_map do |key, value| Array(attributes).flat_map do |key, value|
if value.present? if value.present?
escaped_pair = [ key, escape_shell_value(value) ].join("=") attr = "#{key}=#{escape_shell_value(value)}"
[ argument, redacted ? redact(escaped_pair) : escaped_pair ] attr = self.sensitive(attr, redaction: "#{key}=[REDACTED]") if sensitive
[ argument, attr]
else else
[ argument, key ] [ argument, key ]
end end
@@ -17,7 +18,7 @@ module Mrsk::Utils
# but redacts and expands secrets. # but redacts and expands secrets.
def argumentize_env_with_secrets(env) def argumentize_env_with_secrets(env)
if (secrets = env["secret"]).present? if (secrets = env["secret"]).present?
argumentize("-e", secrets.to_h { |key| [ key, ENV.fetch(key) ] }, redacted: true) + argumentize("-e", env["clear"]) argumentize("-e", secrets.to_h { |key| [ key, ENV.fetch(key) ] }, sensitive: true) + argumentize("-e", env["clear"])
else else
argumentize "-e", env.fetch("clear", env) argumentize "-e", env.fetch("clear", env)
end end
@@ -39,9 +40,37 @@ module Mrsk::Utils
args.flat_map { |key, value| value.try(:map) { |entry| [key, entry] } || [ [ key, value ] ] } args.flat_map { |key, value| value.try(:map) { |entry| [key, entry] } || [ [ key, value ] ] }
end end
# Copied from SSHKit::Backend::Abstract#redact to be available inside Commands classes # Marks sensitive values for redaction in logs and human-visible output.
def redact(arg) # Used in execute_command to hide redact() args a user passes in # Pass `redaction:` to change the default `"[REDACTED]"` redaction, e.g.
arg.to_s.extend(SSHKit::Redaction) # to_s due to our inability to extend Integer, etc # `sensitive "#{arg}=#{secret}", redaction: "#{arg}=xxxx"
def sensitive(...)
Mrsk::Utils::Sensitive.new(...)
end
def redacted(value)
case
when value.respond_to?(:redaction)
value.redaction
when value.respond_to?(:transform_values)
value.transform_values { |value| redacted value }
when value.respond_to?(:map)
value.map { |element| redacted element }
else
value
end
end
def unredacted(value)
case
when value.respond_to?(:unredacted)
value.unredacted
when value.respond_to?(:transform_values)
value.transform_values { |value| unredacted value }
when value.respond_to?(:map)
value.map { |element| unredacted element }
else
value
end
end end
# Escape a value to make it safe for shell use. # Escape a value to make it safe for shell use.

View File

@@ -0,0 +1,19 @@
require "active_support/core_ext/module/delegation"
class Mrsk::Utils::Sensitive
# So SSHKit knows to redact these values.
include SSHKit::Redaction
attr_reader :unredacted, :redaction
delegate :to_s, to: :unredacted
delegate :inspect, to: :redaction
def initialize(value, redaction: "[REDACTED]")
@unredacted, @redaction = value, redaction
end
# Sensitive values won't leak into YAML output.
def encode_with(coder)
coder.represent_scalar nil, redaction
end
end

View File

@@ -112,8 +112,11 @@ class ConfigurationAccessoryTest < ActiveSupport::TestCase
test "env args with secret" do test "env args with secret" do
ENV["MYSQL_ROOT_PASSWORD"] = "secret123" ENV["MYSQL_ROOT_PASSWORD"] = "secret123"
assert_equal ["-e", "MYSQL_ROOT_PASSWORD=\"secret123\"", "-e", "MYSQL_ROOT_HOST=\"%\""], @config.accessory(:mysql).env_args
assert @config.accessory(:mysql).env_args[1].is_a?(SSHKit::Redaction) @config.accessory(:mysql).env_args.tap do |env_args|
assert_equal ["-e", "MYSQL_ROOT_PASSWORD=\"secret123\"", "-e", "MYSQL_ROOT_HOST=\"%\""], Mrsk::Utils.unredacted(env_args)
assert_equal ["-e", "MYSQL_ROOT_PASSWORD=[REDACTED]", "-e", "MYSQL_ROOT_HOST=\"%\""], Mrsk::Utils.redacted(env_args)
end
ensure ensure
ENV["MYSQL_ROOT_PASSWORD"] = nil ENV["MYSQL_ROOT_PASSWORD"] = nil
end end

View File

@@ -97,7 +97,10 @@ class ConfigurationRoleTest < ActiveSupport::TestCase
ENV["REDIS_PASSWORD"] = "secret456" ENV["REDIS_PASSWORD"] = "secret456"
ENV["DB_PASSWORD"] = "secret&\"123" ENV["DB_PASSWORD"] = "secret&\"123"
assert_equal ["-e", "REDIS_PASSWORD=\"secret456\"", "-e", "DB_PASSWORD=\"secret&\\\"123\"", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], @config_with_roles.role(:workers).env_args @config_with_roles.role(:workers).env_args.tap do |env_args|
assert_equal ["-e", "REDIS_PASSWORD=\"secret456\"", "-e", "DB_PASSWORD=\"secret&\\\"123\"", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], Mrsk::Utils.unredacted(env_args)
assert_equal ["-e", "REDIS_PASSWORD=[REDACTED]", "-e", "DB_PASSWORD=[REDACTED]", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], Mrsk::Utils.redacted(env_args)
end
ensure ensure
ENV["REDIS_PASSWORD"] = nil ENV["REDIS_PASSWORD"] = nil
ENV["DB_PASSWORD"] = nil ENV["DB_PASSWORD"] = nil
@@ -116,7 +119,10 @@ class ConfigurationRoleTest < ActiveSupport::TestCase
ENV["DB_PASSWORD"] = "secret123" ENV["DB_PASSWORD"] = "secret123"
assert_equal ["-e", "DB_PASSWORD=\"secret123\"", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], @config_with_roles.role(:workers).env_args @config_with_roles.role(:workers).env_args.tap do |env_args|
assert_equal ["-e", "DB_PASSWORD=\"secret123\"", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], Mrsk::Utils.unredacted(env_args)
assert_equal ["-e", "DB_PASSWORD=[REDACTED]", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], Mrsk::Utils.redacted(env_args)
end
ensure ensure
ENV["DB_PASSWORD"] = nil ENV["DB_PASSWORD"] = nil
end end
@@ -133,7 +139,10 @@ class ConfigurationRoleTest < ActiveSupport::TestCase
ENV["REDIS_PASSWORD"] = "secret456" ENV["REDIS_PASSWORD"] = "secret456"
assert_equal ["-e", "REDIS_PASSWORD=\"secret456\"", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], @config_with_roles.role(:workers).env_args @config_with_roles.role(:workers).env_args.tap do |env_args|
assert_equal ["-e", "REDIS_PASSWORD=\"secret456\"", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], Mrsk::Utils.unredacted(env_args)
assert_equal ["-e", "REDIS_PASSWORD=[REDACTED]", "-e", "REDIS_URL=\"redis://a/b\"", "-e", "WEB_CONCURRENCY=\"4\""], Mrsk::Utils.redacted(env_args)
end
ensure ensure
ENV["REDIS_PASSWORD"] = nil ENV["REDIS_PASSWORD"] = nil
end end

View File

@@ -113,12 +113,13 @@ class ConfigurationTest < ActiveSupport::TestCase
test "env args with clear and secrets" do test "env args with clear and secrets" do
ENV["PASSWORD"] = "secret123" ENV["PASSWORD"] = "secret123"
config = Mrsk::Configuration.new(@deploy.tap { |c| c.merge!({ config = Mrsk::Configuration.new(@deploy.tap { |c| c.merge!({
env: { "clear" => { "PORT" => "3000" }, "secret" => [ "PASSWORD" ] } env: { "clear" => { "PORT" => "3000" }, "secret" => [ "PASSWORD" ] }
}) }) }) })
assert_equal [ "-e", "PASSWORD=\"secret123\"", "-e", "PORT=\"3000\"" ], config.env_args assert_equal [ "-e", "PASSWORD=\"secret123\"", "-e", "PORT=\"3000\"" ], Mrsk::Utils.unredacted(config.env_args)
assert config.env_args[1].is_a?(SSHKit::Redaction) assert_equal [ "-e", "PASSWORD=[REDACTED]", "-e", "PORT=\"3000\"" ], Mrsk::Utils.redacted(config.env_args)
ensure ensure
ENV["PASSWORD"] = nil ENV["PASSWORD"] = nil
end end
@@ -133,12 +134,13 @@ class ConfigurationTest < ActiveSupport::TestCase
test "env args with only secrets" do test "env args with only secrets" do
ENV["PASSWORD"] = "secret123" ENV["PASSWORD"] = "secret123"
config = Mrsk::Configuration.new(@deploy.tap { |c| c.merge!({ config = Mrsk::Configuration.new(@deploy.tap { |c| c.merge!({
env: { "secret" => [ "PASSWORD" ] } env: { "secret" => [ "PASSWORD" ] }
}) }) }) })
assert_equal [ "-e", "PASSWORD=\"secret123\"" ], config.env_args assert_equal [ "-e", "PASSWORD=\"secret123\"" ], Mrsk::Utils.unredacted(config.env_args)
assert config.env_args[1].is_a?(SSHKit::Redaction) assert_equal [ "-e", "PASSWORD=[REDACTED]" ], Mrsk::Utils.redacted(config.env_args)
ensure ensure
ENV["PASSWORD"] = nil ENV["PASSWORD"] = nil
end end

View File

@@ -8,13 +8,16 @@ class UtilsTest < ActiveSupport::TestCase
test "argumentize with redacted" do test "argumentize with redacted" do
assert_kind_of SSHKit::Redaction, \ assert_kind_of SSHKit::Redaction, \
Mrsk::Utils.argumentize("--label", { foo: "bar" }, redacted: true).last Mrsk::Utils.argumentize("--label", { foo: "bar" }, sensitive: true).last
end end
test "argumentize_env_with_secrets" do test "argumentize_env_with_secrets" do
ENV.expects(:fetch).with("FOO").returns("secret") ENV.expects(:fetch).with("FOO").returns("secret")
assert_equal [ "-e", "FOO=\"secret\"", "-e", "BAZ=\"qux\"" ], \
Mrsk::Utils.argumentize_env_with_secrets({ "secret" => [ "FOO" ], "clear" => { BAZ: "qux" } }) args = Mrsk::Utils.argumentize_env_with_secrets({ "secret" => [ "FOO" ], "clear" => { BAZ: "qux" } })
assert_equal [ "-e", "FOO=[REDACTED]", "-e", "BAZ=\"qux\"" ], Mrsk::Utils.redacted(args)
assert_equal [ "-e", "FOO=\"secret\"", "-e", "BAZ=\"qux\"" ], Mrsk::Utils.unredacted(args)
end end
test "optionize" do test "optionize" do
@@ -27,9 +30,20 @@ class UtilsTest < ActiveSupport::TestCase
Mrsk::Utils.optionize({ foo: "bar", baz: "qux", quux: true }, with: "=") Mrsk::Utils.optionize({ foo: "bar", baz: "qux", quux: true }, with: "=")
end end
test "redact" do test "no redaction from #to_s" do
assert_kind_of SSHKit::Redaction, Mrsk::Utils.redact("secret") assert_equal "secret", Mrsk::Utils.sensitive("secret").to_s
assert_equal "secret", Mrsk::Utils.redact("secret") end
test "redact from #inspect" do
assert_equal "[REDACTED]".inspect, Mrsk::Utils.sensitive("secret").inspect
end
test "redact from SSHKit output" do
assert_kind_of SSHKit::Redaction, Mrsk::Utils.sensitive("secret")
end
test "redact from YAML output" do
assert_equal "--- ! '[REDACTED]'\n", YAML.dump(Mrsk::Utils.sensitive("secret"))
end end
test "escape_shell_value" do test "escape_shell_value" do