From db7556ed9953f79dfee7d831d107127ca6361b9b Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 12:05:28 +0000 Subject: [PATCH 1/7] Fix enpass adapter There were changes in main that meant the tests failed after merging. Adding the new `requires_account?` method to the enpass adapter fixed it. --- lib/kamal/secrets/adapters/enpass.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/kamal/secrets/adapters/enpass.rb b/lib/kamal/secrets/adapters/enpass.rb index 68fe41a9..17788f1d 100644 --- a/lib/kamal/secrets/adapters/enpass.rb +++ b/lib/kamal/secrets/adapters/enpass.rb @@ -14,6 +14,10 @@ class Kamal::Secrets::Adapters::Enpass < Kamal::Secrets::Adapters::Base fetch_secrets(secrets, from) end + def requires_account? + false + end + private def fetch_secrets(secrets, vault) secrets_titles = fetch_secret_titles(secrets) From a1708f687f6f3a5bb16b4f65e57fb3d311cc0c0b Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 12:24:46 +0000 Subject: [PATCH 2/7] Prefix secrets in fetch_secrets This allows us to remove the custom fetch method for enpass. --- .../secrets/adapters/aws_secrets_manager.rb | 4 ++-- lib/kamal/secrets/adapters/base.rb | 7 ++++-- lib/kamal/secrets/adapters/bitwarden.rb | 4 ++-- .../adapters/bitwarden_secrets_manager.rb | 23 +++++++++++-------- lib/kamal/secrets/adapters/doppler.rb | 4 +++- lib/kamal/secrets/adapters/enpass.rb | 13 +++++------ .../secrets/adapters/gcp_secret_manager.rb | 4 ++-- lib/kamal/secrets/adapters/last_pass.rb | 3 ++- lib/kamal/secrets/adapters/one_password.rb | 4 ++-- lib/kamal/secrets/adapters/test.rb | 4 ++-- 10 files changed, 40 insertions(+), 30 deletions(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index 4bcac21d..48add1ac 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -4,9 +4,9 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Ba nil end - def fetch_secrets(secrets, account:, session:) + def fetch_secrets(secrets, from:, account:, session:) {}.tap do |results| - get_from_secrets_manager(secrets, account: account).each do |secret| + get_from_secrets_manager(prefixed_secrets(secrets, from: from), account: account).each do |secret| secret_name = secret["Name"] secret_string = JSON.parse(secret["SecretString"]) diff --git a/lib/kamal/secrets/adapters/base.rb b/lib/kamal/secrets/adapters/base.rb index fc66bb34..c74f7c41 100644 --- a/lib/kamal/secrets/adapters/base.rb +++ b/lib/kamal/secrets/adapters/base.rb @@ -7,8 +7,7 @@ class Kamal::Secrets::Adapters::Base check_dependencies! session = login(account) - full_secrets = secrets.map { |secret| [ from, secret ].compact.join("/") } - fetch_secrets(full_secrets, account: account, session: session) + fetch_secrets(secrets, from: from, account: account, session: session) end def requires_account? @@ -27,4 +26,8 @@ class Kamal::Secrets::Adapters::Base def check_dependencies! raise NotImplementedError end + + def prefixed_secrets(secrets, from:) + secrets.map { |secret| [ from, secret ].compact.join("/") } + end end diff --git a/lib/kamal/secrets/adapters/bitwarden.rb b/lib/kamal/secrets/adapters/bitwarden.rb index 5dfb72db..6bd4fb25 100644 --- a/lib/kamal/secrets/adapters/bitwarden.rb +++ b/lib/kamal/secrets/adapters/bitwarden.rb @@ -21,9 +21,9 @@ class Kamal::Secrets::Adapters::Bitwarden < Kamal::Secrets::Adapters::Base session end - def fetch_secrets(secrets, account:, session:) + def fetch_secrets(secrets, from:, account:, session:) {}.tap do |results| - items_fields(secrets).each do |item, fields| + items_fields(prefixed_secrets(secrets, from: from)).each do |item, fields| item_json = run_command("get item #{item.shellescape}", session: session, raw: true) raise RuntimeError, "Could not read #{item} from Bitwarden" unless $?.success? item_json = JSON.parse(item_json) diff --git a/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb b/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb index f0a19caa..7cbc093e 100644 --- a/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb @@ -9,17 +9,11 @@ class Kamal::Secrets::Adapters::BitwardenSecretsManager < Kamal::Secrets::Adapte LIST_COMMAND = "secret list -o env" GET_COMMAND = "secret get -o env" - def fetch_secrets(secrets, account:, session:) + def fetch_secrets(secrets, from:, account:, session:) raise RuntimeError, "You must specify what to retrieve from Bitwarden Secrets Manager" if secrets.length == 0 - if secrets.length == 1 - if secrets[0] == LIST_ALL_SELECTOR - command = LIST_COMMAND - elsif secrets[0].end_with?(LIST_ALL_FROM_PROJECT_SUFFIX) - project = secrets[0].split(LIST_ALL_FROM_PROJECT_SUFFIX).first - command = "#{LIST_COMMAND} #{project}" - end - end + secrets = prefixed_secrets(secrets, from: from) + command, project = extract_command_and_project(secrets) {}.tap do |results| if command.nil? @@ -40,6 +34,17 @@ class Kamal::Secrets::Adapters::BitwardenSecretsManager < Kamal::Secrets::Adapte end end + def extract_command_and_project(secrets) + if secrets.length == 1 + if secrets[0] == LIST_ALL_SELECTOR + [ LIST_COMMAND, nil ] + elsif secrets[0].end_with?(LIST_ALL_FROM_PROJECT_SUFFIX) + project = secrets[0].split(LIST_ALL_FROM_PROJECT_SUFFIX).first + [ "#{LIST_COMMAND} #{project}", project ] + end + end + end + def parse_secret(secret) key, value = secret.split("=", 2) value = value.gsub(/^"|"$/, "") diff --git a/lib/kamal/secrets/adapters/doppler.rb b/lib/kamal/secrets/adapters/doppler.rb index 64d644f7..40eeeab5 100644 --- a/lib/kamal/secrets/adapters/doppler.rb +++ b/lib/kamal/secrets/adapters/doppler.rb @@ -16,8 +16,10 @@ class Kamal::Secrets::Adapters::Doppler < Kamal::Secrets::Adapters::Base $?.success? end - def fetch_secrets(secrets, **) + def fetch_secrets(secrets, from:, **) + secrets = prefixed_secrets(secrets, from: from) project_and_config_flags = "" + unless service_token_set? project, config, _ = secrets.first.split("/") diff --git a/lib/kamal/secrets/adapters/enpass.rb b/lib/kamal/secrets/adapters/enpass.rb index 17788f1d..96dea11a 100644 --- a/lib/kamal/secrets/adapters/enpass.rb +++ b/lib/kamal/secrets/adapters/enpass.rb @@ -9,20 +9,15 @@ # Fetch only DB_PASSWORD from FooBar item # `kamal secrets fetch --adapter enpass --from /Users/YOUR_USERNAME/Library/Containers/in.sinew.Enpass-Desktop/Data/Documents/Vaults/primary FooBar/DB_PASSWORD` class Kamal::Secrets::Adapters::Enpass < Kamal::Secrets::Adapters::Base - def fetch(secrets, account: nil, from:) - check_dependencies! - fetch_secrets(secrets, from) - end - def requires_account? false end private - def fetch_secrets(secrets, vault) + def fetch_secrets(secrets, from:, account:, session:) secrets_titles = fetch_secret_titles(secrets) - result = `enpass-cli -json -vault #{vault.shellescape} show #{secrets_titles.map(&:shellescape).join(" ")}`.strip + result = `enpass-cli -json -vault #{from.shellescape} show #{secrets_titles.map(&:shellescape).join(" ")}`.strip parse_result_and_take_secrets(result, secrets) end @@ -36,6 +31,10 @@ class Kamal::Secrets::Adapters::Enpass < Kamal::Secrets::Adapters::Base $?.success? end + def login(account) + nil + end + def fetch_secret_titles(secrets) secrets.reduce(Set.new) do |secret_titles, secret| # Sometimes secrets contain a '/', when the intent is to fetch a single password for an item. Example: FooBar/DB_PASSWORD diff --git a/lib/kamal/secrets/adapters/gcp_secret_manager.rb b/lib/kamal/secrets/adapters/gcp_secret_manager.rb index b8dfebf3..9f4ed41c 100644 --- a/lib/kamal/secrets/adapters/gcp_secret_manager.rb +++ b/lib/kamal/secrets/adapters/gcp_secret_manager.rb @@ -26,11 +26,11 @@ class Kamal::Secrets::Adapters::GcpSecretManager < Kamal::Secrets::Adapters::Bas nil end - def fetch_secrets(secrets, account:, session:) + def fetch_secrets(secrets, from:, account:, session:) user, service_account = parse_account(account) {}.tap do |results| - secrets_with_metadata(secrets).each do |secret, (project, secret_name, secret_version)| + secrets_with_metadata(prefixed_secrets(secrets, from: from)).each do |secret, (project, secret_name, secret_version)| item_name = "#{project}/#{secret_name}" results[item_name] = fetch_secret(project, secret_name, secret_version, user, service_account) raise RuntimeError, "Could not read #{item_name} from Google Secret Manager" unless $?.success? diff --git a/lib/kamal/secrets/adapters/last_pass.rb b/lib/kamal/secrets/adapters/last_pass.rb index 2f95148b..be10d0d7 100644 --- a/lib/kamal/secrets/adapters/last_pass.rb +++ b/lib/kamal/secrets/adapters/last_pass.rb @@ -11,7 +11,8 @@ class Kamal::Secrets::Adapters::LastPass < Kamal::Secrets::Adapters::Base `lpass status --color never`.strip == "Logged in as #{account}." end - def fetch_secrets(secrets, account:, session:) + def fetch_secrets(secrets, from:, account:, session:) + secrets = prefixed_secrets(secrets, from: from) items = `lpass show #{secrets.map(&:shellescape).join(" ")} --json` raise RuntimeError, "Could not read #{secrets} from LastPass" unless $?.success? diff --git a/lib/kamal/secrets/adapters/one_password.rb b/lib/kamal/secrets/adapters/one_password.rb index c7f9d5ab..fe454342 100644 --- a/lib/kamal/secrets/adapters/one_password.rb +++ b/lib/kamal/secrets/adapters/one_password.rb @@ -15,9 +15,9 @@ class Kamal::Secrets::Adapters::OnePassword < Kamal::Secrets::Adapters::Base $?.success? end - def fetch_secrets(secrets, account:, session:) + def fetch_secrets(secrets, from:, account:, session:) {}.tap do |results| - vaults_items_fields(secrets).map do |vault, items| + vaults_items_fields(prefixed_secrets(secrets, from: from)).map do |vault, items| items.each do |item, fields| fields_json = JSON.parse(op_item_get(vault, item, fields, account: account, session: session)) fields_json = [ fields_json ] if fields.one? diff --git a/lib/kamal/secrets/adapters/test.rb b/lib/kamal/secrets/adapters/test.rb index 82577a76..ac48960b 100644 --- a/lib/kamal/secrets/adapters/test.rb +++ b/lib/kamal/secrets/adapters/test.rb @@ -4,8 +4,8 @@ class Kamal::Secrets::Adapters::Test < Kamal::Secrets::Adapters::Base true end - def fetch_secrets(secrets, account:, session:) - secrets.to_h { |secret| [ secret, secret.reverse ] } + def fetch_secrets(secrets, from:, account:, session:) + prefixed_secrets(secrets, from: from).to_h { |secret| [ secret, secret.reverse ] } end def check_dependencies! From 5e2678decebe46d4f04ba17985845165039856dc Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 12:28:59 +0000 Subject: [PATCH 3/7] Ensure external input is shell escaped --- lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb b/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb index 7cbc093e..66afbe70 100644 --- a/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/bitwarden_secrets_manager.rb @@ -18,7 +18,7 @@ class Kamal::Secrets::Adapters::BitwardenSecretsManager < Kamal::Secrets::Adapte {}.tap do |results| if command.nil? secrets.each do |secret_uuid| - secret = run_command("#{GET_COMMAND} #{secret_uuid}") + secret = run_command("#{GET_COMMAND} #{secret_uuid.shellescape}") raise RuntimeError, "Could not read #{secret_uuid} from Bitwarden Secrets Manager" unless $?.success? key, value = parse_secret(secret) results[key] = value @@ -40,7 +40,7 @@ class Kamal::Secrets::Adapters::BitwardenSecretsManager < Kamal::Secrets::Adapte [ LIST_COMMAND, nil ] elsif secrets[0].end_with?(LIST_ALL_FROM_PROJECT_SUFFIX) project = secrets[0].split(LIST_ALL_FROM_PROJECT_SUFFIX).first - [ "#{LIST_COMMAND} #{project}", project ] + [ "#{LIST_COMMAND} #{project.shellescape}", project ] end end end From 10dafc058aff4b635dd816ec9d4660d7b02aeaa9 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 12:31:03 +0000 Subject: [PATCH 4/7] Extract secrets_get_flags --- lib/kamal/secrets/adapters/doppler.rb | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/kamal/secrets/adapters/doppler.rb b/lib/kamal/secrets/adapters/doppler.rb index 40eeeab5..90b2c63b 100644 --- a/lib/kamal/secrets/adapters/doppler.rb +++ b/lib/kamal/secrets/adapters/doppler.rb @@ -18,8 +18,19 @@ class Kamal::Secrets::Adapters::Doppler < Kamal::Secrets::Adapters::Base def fetch_secrets(secrets, from:, **) secrets = prefixed_secrets(secrets, from: from) - project_and_config_flags = "" + flags = secrets_get_flags(secrets) + secret_names = secrets.collect { |s| s.split("/").last } + + items = `doppler secrets get #{secret_names.map(&:shellescape).join(" ")} --json #{flags}` + raise RuntimeError, "Could not read #{secrets} from Doppler" unless $?.success? + + items = JSON.parse(items) + + items.transform_values { |value| value["computed"] } + end + + def secrets_get_flags(secrets) unless service_token_set? project, config, _ = secrets.first.split("/") @@ -29,15 +40,6 @@ class Kamal::Secrets::Adapters::Doppler < Kamal::Secrets::Adapters::Base project_and_config_flags = "-p #{project.shellescape} -c #{config.shellescape}" end - - secret_names = secrets.collect { |s| s.split("/").last } - - items = `doppler secrets get #{secret_names.map(&:shellescape).join(" ")} --json #{project_and_config_flags}` - raise RuntimeError, "Could not read #{secrets} from Doppler" unless $?.success? - - items = JSON.parse(items) - - items.transform_values { |value| value["computed"] } end def service_token_set? From f9a78f4fcbaa244c27f804a34bb9f0e64305ee9d Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 12:34:38 +0000 Subject: [PATCH 5/7] gcloud login tidy Use unless instead of if !, don't suggest running gcloud auth login, we've just tried that. --- lib/kamal/secrets/adapters/gcp_secret_manager.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/kamal/secrets/adapters/gcp_secret_manager.rb b/lib/kamal/secrets/adapters/gcp_secret_manager.rb index 9f4ed41c..8ce381ff 100644 --- a/lib/kamal/secrets/adapters/gcp_secret_manager.rb +++ b/lib/kamal/secrets/adapters/gcp_secret_manager.rb @@ -18,9 +18,9 @@ class Kamal::Secrets::Adapters::GcpSecretManager < Kamal::Secrets::Adapters::Bas # - "default|my-service-user@example.com" will use the default user, and enable service account impersonation as my-service-user # - "default|my-service-user@example.com,another-service-user@example.com" same as above, but with an impersonation delegation chain - if !logged_in? + unless logged_in? `gcloud auth login` - raise RuntimeError, "gcloud is not authenticated, please run `gcloud auth login`" if !logged_in? + raise RuntimeError, "could not login to gcloud" unless logged_in? end nil From 2bd716ece40ef552f2da52aa20888553e23aaec7 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 12:37:12 +0000 Subject: [PATCH 6/7] Drop the TestOptionalAccount adapter It's included in the gem lib which is best to avoid and we can infer that it works account optional adapters. --- lib/kamal/secrets/adapters/test_optional_account.rb | 5 ----- test/cli/secrets_test.rb | 6 ------ 2 files changed, 11 deletions(-) delete mode 100644 lib/kamal/secrets/adapters/test_optional_account.rb diff --git a/lib/kamal/secrets/adapters/test_optional_account.rb b/lib/kamal/secrets/adapters/test_optional_account.rb deleted file mode 100644 index 3a252e68..00000000 --- a/lib/kamal/secrets/adapters/test_optional_account.rb +++ /dev/null @@ -1,5 +0,0 @@ -class Kamal::Secrets::Adapters::TestOptionalAccount < Kamal::Secrets::Adapters::Test - def requires_account? - false - end -end diff --git a/test/cli/secrets_test.rb b/test/cli/secrets_test.rb index bd412862..74f309f7 100644 --- a/test/cli/secrets_test.rb +++ b/test/cli/secrets_test.rb @@ -13,12 +13,6 @@ class CliSecretsTest < CliTestCase run_command("fetch", "foo", "bar", "baz", "--adapter", "test") end - test "fetch without required --account" do - assert_equal \ - "\\{\\\"foo\\\":\\\"oof\\\",\\\"bar\\\":\\\"rab\\\",\\\"baz\\\":\\\"zab\\\"\\}", - run_command("fetch", "foo", "bar", "baz", "--adapter", "test_optional_account") - end - test "extract" do assert_equal "oof", run_command("extract", "foo", "{\"foo\":\"oof\", \"bar\":\"rab\", \"baz\":\"zab\"}") end From 9f1688da7aeab35fa6a760119b68ebfad3ed4b78 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 12:52:23 +0000 Subject: [PATCH 7/7] Fix test --- test/secrets/gcp_secret_manager_adapter_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/secrets/gcp_secret_manager_adapter_test.rb b/test/secrets/gcp_secret_manager_adapter_test.rb index 341f42e9..682db1f4 100644 --- a/test/secrets/gcp_secret_manager_adapter_test.rb +++ b/test/secrets/gcp_secret_manager_adapter_test.rb @@ -23,7 +23,7 @@ class GcpSecretManagerAdapterTest < SecretAdapterTestCase JSON.parse(shellunescape(run_command("fetch", "mypassword"))) end - assert_match(/not authenticated/, error.message) + assert_match(/could not login to gcloud/, error.message) end test "fetch with from" do