From b97aa85eedb81f9700bde4784ec23e5d324cd211 Mon Sep 17 00:00:00 2001 From: "Adam C. Stephens" Date: Fri, 9 Oct 2026 03:01:29 +0000 Subject: [PATCH] encryption: migrate credentials to fields on boot Refs: GS-49 Assisted-By: OpenAI Codex GPT-6.1 Sol --- README.md | 39 ++++-- config/dev.exs | 7 - config/runtime.exs | 13 -- config/test.exs | 7 - lib/git_sync/application.ex | 9 +- lib/git_sync/encrypted/binary.ex | 15 ++- lib/git_sync/vault.ex | 7 - mix.exs | 1 - mix.lock | 2 - nix/deps.nix | 41 ------ nix/module.nix | 4 +- nix/test.nix | 1 - ...09025209_migrate_credentials_to_fields.exs | 57 ++++++++ test/git_sync/encryption_upgrade_test.exs | 122 ++++++++++++++++++ test/support/upgrade_repo.ex | 5 + 15 files changed, 232 insertions(+), 98 deletions(-) delete mode 100644 lib/git_sync/vault.ex create mode 100644 priv/repo/migrations/20261009025209_migrate_credentials_to_fields.exs create mode 100644 test/git_sync/encryption_upgrade_test.exs create mode 100644 test/support/upgrade_repo.ex diff --git a/README.md b/README.md index 95df4aa..6e6c56b 100644 --- a/README.md +++ b/README.md @@ -30,11 +30,34 @@ screens. ## Credential encryption upgrade -The preparation release adds Fields without changing credential storage. Before -deploying it, add `ENCRYPTION_KEYS` to the service environment with exactly the -existing `CLOAK_KEY` value, and retain `CLOAK_KEY`. Do not generate a replacement -key for existing data. - -The cutover release will convert credentials automatically at boot, before -workers and HTTP serving start. Stop the old service and take a consistent SQLite -backup before deploying that release. Both commits may be deployed together. +Two commits separate preparation from cutover; they may be deployed together. + +1. Stop the old service and take a consistent SQLite backup, including its WAL + state. Preserve the old release and encryption key with the backup. +2. Set `ENCRYPTION_KEYS` in the service environment to exactly the existing + `CLOAK_KEY` value. Do not generate a new key for existing data. +3. Deploy the preparation commit if desired. It still requires `CLOAK_KEY` and + continues writing Cloak credentials. +4. Deploy the cutover commit and start the service. It no longer requires + `CLOAK_KEY`; remove that variable after cutover. + +The cutover converts `connections.token`, `refresh_token`, `ssh_key`, +`client_secret`, and `sources.webhook_secret` automatically on application boot, +before webhook reconciliation, sync workers, and HTTP serving start. It replaces +the ciphertext header while preserving the encrypted payload and authenticates +every non-null value using key index zero. Nulls remain null. + +Conversion runs in one SQLite migration transaction, reading batches of 100 rows. +A wrong key, failed authentication, or unexpected format aborts startup and rolls +back all updates. Correct the problem and restart; failed migrations are not +recorded. Successful migrations are recorded and are not repeated on restart. +Only one service instance may access the database during cutover. + +Rollback requires stopping the service and restoring the pre-upgrade backup +together with the old release and key. Starting the old binary against converted +data is not supported. Restoring the backup discards writes made after cutover. + +After migration, `ENCRYPTION_KEYS` may contain comma-separated Base64-encoded +32-byte keys. Fields encrypts with the last key and stores its numeric index. +Append keys to rotate; never reorder or remove keys still needed by stored data +or retained backups. Do not combine rotation with this upgrade. diff --git a/config/dev.exs b/config/dev.exs index 3b8243d..1bf8ab9 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -73,11 +73,4 @@ config :phoenix_live_view, # Enable helpful, but potentially expensive runtime checks enable_expensive_runtime_checks: true -config :git_sync, GitSync.Vault, - ciphers: [ - default: - {Cloak.Ciphers.AES.GCM, - tag: "AES.GCM.V1", key: Base.decode64!("dVf0aGNPJiqYMT12JyzbtgqEkyHev079E3dNIwCn0UU=")} - ] - config :fields, encryption_keys: "dVf0aGNPJiqYMT12JyzbtgqEkyHev079E3dNIwCn0UU=" diff --git a/config/runtime.exs b/config/runtime.exs index 1ed4dd2..b2158a9 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -54,19 +54,6 @@ if config_env() == :prod do database: database_path, pool_size: String.to_integer(System.get_env("POOL_SIZE") || "5") - cloak_key = - System.get_env("CLOAK_KEY") || - raise """ - environment variable CLOAK_KEY is missing. - You can generate one by calling: - elixir -e 'IO.puts(Base.encode64(:crypto.strong_rand_bytes(32)))' - """ - - config :git_sync, GitSync.Vault, - ciphers: [ - default: {Cloak.Ciphers.AES.GCM, tag: "AES.GCM.V1", key: Base.decode64!(cloak_key)} - ] - config :fields, encryption_keys: System.fetch_env!("ENCRYPTION_KEYS") # The secret key base is used to sign/encrypt cookies and other secrets. diff --git a/config/test.exs b/config/test.exs index 47e607e..d5079bc 100644 --- a/config/test.exs +++ b/config/test.exs @@ -31,13 +31,6 @@ config :phoenix_live_view, config :phoenix, sort_verified_routes_query_params: true -config :git_sync, GitSync.Vault, - ciphers: [ - default: - {Cloak.Ciphers.AES.GCM, - tag: "AES.GCM.V1", key: Base.decode64!("aqk/Lo+yqt9cBQo0Pi4TPYgfP/8hC9axhT3DMJecN5I=")} - ] - config :fields, encryption_keys: "aqk/Lo+yqt9cBQo0Pi4TPYgfP/8hC9axhT3DMJecN5I=" # Outbound HTTP is stubbed per test process; see `Req.Test`. diff --git a/lib/git_sync/application.ex b/lib/git_sync/application.ex index b1a23e5..e816cb0 100644 --- a/lib/git_sync/application.ex +++ b/lib/git_sync/application.ex @@ -11,10 +11,8 @@ defmodule GitSync.Application do children = [ GitSyncWeb.Telemetry, - GitSync.Vault, GitSync.Repo, - {Ecto.Migrator, - repos: Application.fetch_env!(:git_sync, :ecto_repos), skip: skip_migrations?()}, + {Ecto.Migrator, repos: Application.fetch_env!(:git_sync, :ecto_repos)}, Supervisor.child_spec( {Task, &GitSync.Sources.reconcile_webhooks/0}, id: GitSync.WebhookReconciler @@ -51,9 +49,4 @@ defmodule GitSync.Application do GitSync.Sync.start_enabled() end end - - defp skip_migrations?() do - # By default, sqlite migrations are run when using a release - System.get_env("RELEASE_NAME") == nil - end end diff --git a/lib/git_sync/encrypted/binary.ex b/lib/git_sync/encrypted/binary.ex index bd324dc..3aa821b 100644 --- a/lib/git_sync/encrypted/binary.ex +++ b/lib/git_sync/encrypted/binary.ex @@ -1,5 +1,18 @@ defmodule GitSync.Encrypted.Binary do @moduledoc false - use Cloak.Ecto.Binary, vault: GitSync.Vault + use Ecto.Type + + defdelegate type(), to: Fields.Encrypted + defdelegate cast(value), to: Fields.Encrypted + defdelegate dump(value), to: Fields.Encrypted + defdelegate embed_as(format), to: Fields.Encrypted + defdelegate equal?(left, right), to: Fields.Encrypted + + def load(value) do + case Fields.AES.decrypt(value) do + plaintext when is_binary(plaintext) -> {:ok, plaintext} + :error -> :error + end + end end diff --git a/lib/git_sync/vault.ex b/lib/git_sync/vault.ex deleted file mode 100644 index 761e6a4..0000000 --- a/lib/git_sync/vault.ex +++ /dev/null @@ -1,7 +0,0 @@ -defmodule GitSync.Vault do - @moduledoc """ - Cloak vault backing the encrypted credential fields on `GitSync.Connection`. - """ - - use Cloak.Vault, otp_app: :git_sync -end diff --git a/mix.exs b/mix.exs index cc522e9..6f3d7e1 100644 --- a/mix.exs +++ b/mix.exs @@ -58,7 +58,6 @@ defmodule GitSync.MixProject do {:jason, "~> 1.2"}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, {:bandit, "~> 1.5"}, - {:cloak_ecto, "~> 1.3.0"}, {:fields, "~> 2.12"}, {:oidcc, "~> 3.8"}, {:oidcc_plug, "~> 0.5.1"}, diff --git a/mix.lock b/mix.lock index 21329ce..b9fc8ab 100644 --- a/mix.lock +++ b/mix.lock @@ -3,8 +3,6 @@ "bandit": {:hex, :bandit, "1.12.5", "af205a8e550f304caae09a97d29fd3c79a7f337526ea7cd772d2ff11d2f7c800", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.18", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.5", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "c5684ca062fa407cac115aec3256383f3e2ec9fdced7904d59cf5a7bb7ed6181"}, "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "cc_precompiler": {:hex, :cc_precompiler, "0.1.11", "8c844d0b9fb98a3edea067f94f616b3f6b29b959b6b3bf25fee94ffe34364768", [:mix], [{:elixir_make, "~> 0.7", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "3427232caf0835f94680e5bcf082408a70b48ad68a5f5c0b02a3bea9f3a075b9"}, - "cloak": {:hex, :cloak, "1.1.4", "aba387b22ea4d80d92d38ab1890cc528b06e0e7ef2a4581d71c3fdad59e997e7", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}], "hexpm", "92b20527b9aba3d939fab0dd32ce592ff86361547cfdc87d74edce6f980eb3d7"}, - "cloak_ecto": {:hex, :cloak_ecto, "1.3.0", "0de127c857d7452ba3c3367f53fb814b0410ff9c680a8d20fbe8b9a3c57a1118", [:mix], [{:cloak, "~> 1.1.1", [hex: :cloak, repo: "hexpm", optional: false]}, {:ecto, "~> 3.0", [hex: :ecto, repo: "hexpm", optional: false]}], "hexpm", "314beb0c123b8a800418ca1d51065b27ba3b15f085977e65c0f7b2adab2de1cc"}, "comeonin": {:hex, :comeonin, "5.5.1", "5113e5f3800799787de08a6e0db307133850e635d34e9fab23c70b6501669510", [:mix], [], "hexpm", "65aac8f19938145377cee73973f192c5645873dcf550a8a6b18187d17c13ccdb"}, "credo": {:hex, :credo, "1.7.19", "cc52129665fc7c15143d47838fda0f9cd6dac9ceced7bf4da6f85fcbfe64b12a", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "2d8bc95d5a7bb99dd2613621d4f08c6a3575c3fd4b62e6a2b48a100352a557b8"}, "db_connection": {:hex, :db_connection, "2.10.2", "ae391e803a5adff104da913c2fc1c0c14a37f8b10001dcef568796e1fb7bf95c", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "510b14482330f1af6490a2fa0efd8d4f1435d1529b165647df22ac0f2df0fa93"}, diff --git a/nix/deps.nix b/nix/deps.nix index baf875a..aeaa3ae 100644 --- a/nix/deps.nix +++ b/nix/deps.nix @@ -251,47 +251,6 @@ let in drv.override (workarounds.elixirMake { } drv); - cloak = - let - version = "1.1.4"; - drv = buildMix { - inherit version; - name = "cloak"; - - src = fetchHex { - inherit version; - pkg = "cloak"; - sha256 = "92b20527b9aba3d939fab0dd32ce592ff86361547cfdc87d74edce6f980eb3d7"; - }; - - beamDeps = [ - jason - ]; - }; - in - drv; - - cloak_ecto = - let - version = "1.3.0"; - drv = buildMix { - inherit version; - name = "cloak_ecto"; - - src = fetchHex { - inherit version; - pkg = "cloak_ecto"; - sha256 = "314beb0c123b8a800418ca1d51065b27ba3b15f085977e65c0f7b2adab2de1cc"; - }; - - beamDeps = [ - cloak - ecto - ]; - }; - in - drv; - comeonin = let version = "5.5.1"; diff --git a/nix/module.nix b/nix/module.nix index 1673b7c..b0eeef4 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -39,8 +39,8 @@ in secretsFile = lib.mkOption { type = lib.types.path; description = '' - EnvironmentFile holding `SECRET_KEY_BASE`, `CLOAK_KEY`, and `ENCRYPTION_KEYS`. - Set `ENCRYPTION_KEYS` to the existing `CLOAK_KEY` for the encryption upgrade. + EnvironmentFile holding `SECRET_KEY_BASE` and `ENCRYPTION_KEYS`. + For the encryption upgrade, set `ENCRYPTION_KEYS` to the old `CLOAK_KEY` value. Generate new secrets with `mix phx.gen.secret` and `elixir -e 'IO.puts(Base.encode64(:crypto.strong_rand_bytes(32)))'`. diff --git a/nix/test.nix b/nix/test.nix index d98cbc8..4d5508b 100644 --- a/nix/test.nix +++ b/nix/test.nix @@ -15,7 +15,6 @@ address = "127.0.0.1"; secretsFile = pkgs.writeText "git-sync.env" '' SECRET_KEY_BASE=${lib.strings.replicate 64 "a"} - CLOAK_KEY=${lib.strings.replicate 43 "b"}= ENCRYPTION_KEYS=${lib.strings.replicate 43 "b"}= ''; }; diff --git a/priv/repo/migrations/20261009025209_migrate_credentials_to_fields.exs b/priv/repo/migrations/20261009025209_migrate_credentials_to_fields.exs new file mode 100644 index 0000000..b9440a3 --- /dev/null +++ b/priv/repo/migrations/20261009025209_migrate_credentials_to_fields.exs @@ -0,0 +1,57 @@ +defmodule GitSync.Repo.Migrations.MigrateCredentialsToFields do + use Ecto.Migration + + def up do + migrate("connections", ["token", "refresh_token", "ssh_key", "client_secret"], 0) + migrate("sources", ["webhook_secret"], 0) + end + + def down do + raise Ecto.MigrationError, "restore the pre-upgrade database backup to roll back encryption" + end + + defp migrate(table, columns, after_id) do + selected = Enum.join(["id" | columns], ", ") + + %{rows: rows} = + repo().query!( + "SELECT #{selected} FROM #{table} WHERE id > ? ORDER BY id LIMIT 100", + [after_id], + log: false + ) + + for [id | values] <- rows do + converted = Enum.map(values, &convert(&1, table, id)) + assignments = Enum.map_join(columns, ", ", &"#{&1} = ?") + + repo().query!( + "UPDATE #{table} SET #{assignments} WHERE id = ?", + converted ++ [id], + log: false + ) + end + + case List.last(rows) do + nil -> :ok + [id | _] -> migrate(table, columns, id) + end + end + + defp convert(nil, _table, _id), do: nil + + defp convert(<<1, 10, "AES.GCM.V1", payload::binary>>, table, id) do + converted = "0000" <> payload + + case Fields.AES.decrypt(converted) do + plaintext when is_binary(plaintext) -> + converted + + :error -> + raise Ecto.MigrationError, "credential authentication failed in #{table} row #{id}" + end + end + + defp convert(_value, table, id) do + raise Ecto.MigrationError, "unexpected credential format in #{table} row #{id}" + end +end diff --git a/test/git_sync/encryption_upgrade_test.exs b/test/git_sync/encryption_upgrade_test.exs new file mode 100644 index 0000000..1f0489c --- /dev/null +++ b/test/git_sync/encryption_upgrade_test.exs @@ -0,0 +1,122 @@ +defmodule GitSync.EncryptionUpgradeTest do + use ExUnit.Case, async: false + + alias GitSync.UpgradeRepo + + @version 20_261_009_025_209 + @columns ["token", "refresh_token", "ssh_key", "client_secret"] + @migrations (for file <- Path.wildcard(Path.expand("../../priv/repo/migrations/*.exs", __DIR__)) do + [version, name] = String.split(Path.basename(file, ".exs"), "_", parts: 2) + module = Module.concat(GitSync.Repo.Migrations, Macro.camelize(name)) + unless Code.ensure_loaded?(module), do: Code.compile_file(file) + {String.to_integer(version), module} + end) + + setup do + database = + Path.join(System.tmp_dir!(), "git-sync-upgrade-#{System.unique_integer([:positive])}.db") + + start_supervised!({UpgradeRepo, database: database, pool_size: 1}) + + on_exit(fn -> + for suffix <- ["", "-wal", "-shm"], do: File.rm(database <> suffix) + end) + + Ecto.Migrator.run(UpgradeRepo, @migrations, :up, to: @version - 1, log: false) + :ok + end + + test "converts every credential, preserves nulls and payloads, and runs only once" do + old = legacy_encrypt("credential\nwith bytes\0") + insert_connection(old) + + UpgradeRepo.query!( + "INSERT INTO sources (connection_id, repo, interval_seconds, enabled, webhook_secret, inserted_at, updated_at) VALUES (1, 'repo', 60, 0, ?, datetime('now'), datetime('now'))", + [old] + ) + + assert Ecto.Migrator.run(UpgradeRepo, @migrations, :up, all: true, log: false) == [@version] + + %{rows: [values]} = + UpgradeRepo.query!( + "SELECT token, refresh_token, ssh_key, client_secret FROM connections WHERE id = 1" + ) + + for value <- values do + assert Fields.AES.decrypt(value) == "credential\nwith bytes\0" + assert value == "0000" <> binary_part(old, 12, byte_size(old) - 12) + end + + assert %{rows: [[nil, nil, nil, nil]]} = + UpgradeRepo.query!( + "SELECT token, refresh_token, ssh_key, client_secret FROM connections WHERE id = 2" + ) + + assert %{rows: [[secret]]} = UpgradeRepo.query!("SELECT webhook_secret FROM sources") + assert secret == hd(values) + assert Ecto.Migrator.run(UpgradeRepo, @migrations, :up, all: true, log: false) == [] + + assert %{rows: [^values]} = + UpgradeRepo.query!( + "SELECT token, refresh_token, ssh_key, client_secret FROM connections WHERE id = 1" + ) + end + + test "authentication failure rolls back earlier updates and does not record the migration" do + old = legacy_encrypt("valid") + insert_connection(old) + bad = legacy_encrypt("invalid", :crypto.strong_rand_bytes(32)) + UpgradeRepo.query!("UPDATE connections SET token = ? WHERE id = 2", [bad]) + + assert_raise Ecto.MigrationError, fn -> + Ecto.Migrator.run(UpgradeRepo, @migrations, :up, all: true, log: false) + end + + assert %{rows: [[^old]]} = UpgradeRepo.query!("SELECT token FROM connections WHERE id = 1") + refute @version in Ecto.Migrator.migrated_versions(UpgradeRepo) + end + + test "a fresh database can migrate and store Fields credentials" do + Ecto.Migrator.run(UpgradeRepo, @migrations, :up, all: true, log: false) + encrypted = Fields.AES.encrypt("") + + UpgradeRepo.query!( + "INSERT INTO connections (kind, base_url, token, inserted_at, updated_at) VALUES ('github', 'https://example.com', ?, datetime('now'), datetime('now'))", + [encrypted] + ) + + assert %{rows: [[value]]} = UpgradeRepo.query!("SELECT token FROM connections") + assert Fields.AES.decrypt(value) == "" + end + + test "the credential type rejects failed authentication" do + good = Fields.AES.encrypt("secret") + <> = good + assert GitSync.Encrypted.Binary.load(good) == {:ok, "secret"} + + assert GitSync.Encrypted.Binary.load(prefix <> <>) == + :error + end + + defp insert_connection(value) do + columns = Enum.join(@columns, ", ") + + UpgradeRepo.query!( + "INSERT INTO connections (id, kind, base_url, #{columns}, inserted_at, updated_at) VALUES (1, 'github', 'https://example.com', ?, ?, ?, ?, datetime('now'), datetime('now'))", + List.duplicate(value, 4) + ) + + UpgradeRepo.query!( + "INSERT INTO connections (id, kind, base_url, inserted_at, updated_at) VALUES (2, 'tangled', 'https://example.com', datetime('now'), datetime('now'))" + ) + end + + defp legacy_encrypt( + value, + key \\ Base.decode64!(Application.fetch_env!(:fields, :encryption_keys)) + ) do + iv = :crypto.strong_rand_bytes(16) + {body, tag} = :crypto.crypto_one_time_aead(:aes_256_gcm, key, iv, value, "AES256GCM", true) + <<1, 10>> <> "AES.GCM.V1" <> iv <> tag <> body + end +end diff --git a/test/support/upgrade_repo.ex b/test/support/upgrade_repo.ex new file mode 100644 index 0000000..51db472 --- /dev/null +++ b/test/support/upgrade_repo.ex @@ -0,0 +1,5 @@ +defmodule GitSync.UpgradeRepo do + @moduledoc false + + use Ecto.Repo, otp_app: :git_sync, adapter: Ecto.Adapters.SQLite3 +end -- 2.51.2