diff --git a/config/config.exs b/config/config.exs index 278d47b..3fc1813 100644 --- a/config/config.exs +++ b/config/config.exs @@ -44,12 +44,15 @@ config :phoenix, :filter_parameters, [ "refresh_token", "id_token", "device_code", + "device_auth_id", "user_code", + "code_verifier", "client_secret", "authorization", "encrypted_credentials", "account_id", "chatgpt_account_id", + "chatgpt_user_id", "organization_id", "workspace_id" ] diff --git a/config/runtime.exs b/config/runtime.exs index b561fc8..6009b71 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -71,8 +71,15 @@ end if config_env() == :prod do current_key_version = case System.get_env("KODO_CREDENTIAL_ENCRYPTION_CURRENT_KEY_VERSION") do - version when is_binary(version) and version != "" -> version - _invalid -> raise "KODO_CREDENTIAL_ENCRYPTION_CURRENT_KEY_VERSION must be set" + version when is_binary(version) -> + if Regex.match?(~r/\A[A-Za-z0-9][A-Za-z0-9._-]{0,63}\z/, version) do + version + else + raise "KODO_CREDENTIAL_ENCRYPTION_CURRENT_KEY_VERSION is invalid" + end + + _invalid -> + raise "KODO_CREDENTIAL_ENCRYPTION_CURRENT_KEY_VERSION must be set" end encoded_key_ring = @@ -85,13 +92,19 @@ if config_env() == :prod do {:ok, keys} <- Enum.reduce_while(encoded_keys, {:ok, %{}}, fn {version, encoded_key}, {:ok, keys} - when is_binary(version) and version != "" and is_binary(encoded_key) -> - case Base.decode64(encoded_key) do - {:ok, key} when byte_size(key) == 32 -> - {:cont, {:ok, Map.put(keys, version, key)}} - - _invalid -> - {:halt, :error} + when is_binary(version) and is_binary(encoded_key) -> + with true <- + Regex.match?(~r/\A[A-Za-z0-9][A-Za-z0-9._-]{0,63}\z/, version), + {:ok, key} <- Base.decode64(encoded_key) do + case key do + key when byte_size(key) == 32 -> + {:cont, {:ok, Map.put(keys, version, key)}} + + _invalid -> + {:halt, :error} + end + else + _invalid -> {:halt, :error} end _invalid_entry, _keys -> diff --git a/docs/operations.org b/docs/operations.org index 7fe5a6a..e5c0f9b 100644 --- a/docs/operations.org +++ b/docs/operations.org @@ -108,7 +108,8 @@ Generate key material with a cryptographically secure generator, store it in the deployment secret manager, and inject the same ring into every replica. Do not derive it from ~SECRET_KEY_BASE~ or any provider credential. For example, the shape is ~{"2026-09-v1":""}~; never commit the -real value. +real value. Key versions must be 1–64 ASCII letters, digits, dots, underscores, +or hyphens, and must begin with a letter or digit. Startup validates every configured key and refuses to proceed unless the current version exists. After PostgreSQL starts, Kodo also verifies that every diff --git a/lib/kodo/integrations.ex b/lib/kodo/integrations.ex index defb12e..c1e4159 100644 --- a/lib/kodo/integrations.ex +++ b/lib/kodo/integrations.ex @@ -34,13 +34,15 @@ defmodule Kodo.Integrations do end end - def connect(%Scope{user: user}, provider, authentication_type, credentials, opts \\ []) do + def connect(scope, provider, authentication_type, credentials, opts \\ []) + + def connect(%Scope{user: user}, provider, "api_key", credentials, opts) do integration = %Integration{id: Ecto.UUID.generate(), user_id: user.id} changeset = Integration.create_changeset(integration, %{ provider: provider, - authentication_type: authentication_type + authentication_type: "api_key" }) with true <- changeset.valid?, @@ -56,12 +58,16 @@ defmodule Kodo.Integrations do ) |> Integration.constraint_changeset() |> Repo.insert() + |> normalize_insert_result() else false -> {:error, changeset} {:error, _reason} = error -> error end end + def connect(%Scope{}, _provider, _authentication_type, _credentials, _opts), + do: {:error, :authentication_type_mismatch} + def replace_credentials(%Scope{} = scope, id, generation, credentials, opts \\ []) do install_credentials( scope, @@ -70,7 +76,7 @@ defmodule Kodo.Integrations do credentials, opts, Integration.connection_statuses(), - :any + "api_key" ) end @@ -191,8 +197,6 @@ defmodule Kodo.Integrations do defp require_generation(%Integration{credential_generation: generation}, generation), do: :ok defp require_generation(%Integration{}, _generation), do: {:error, :stale_credential_generation} - defp require_authentication_type(%Integration{}, :any), do: :ok - defp require_authentication_type(%Integration{authentication_type: type}, type), do: :ok defp require_authentication_type(%Integration{}, _type), @@ -228,5 +232,21 @@ defmodule Kodo.Integrations do end end + defp normalize_insert_result({:error, changeset} = error) do + cond do + constraint_error?(changeset, :foreign) -> {:error, :integration_owner_not_found} + constraint_error?(changeset, :unique) -> {:error, :integration_already_exists} + true -> error + end + end + + defp normalize_insert_result(result), do: result + + defp constraint_error?(changeset, type) do + Enum.any?(changeset.errors, fn {_field, {_message, metadata}} -> + metadata[:constraint] == type + end) + end + defp now, do: DateTime.utc_now() end diff --git a/lib/kodo/integrations/credential_encryption.ex b/lib/kodo/integrations/credential_encryption.ex index 968591e..5fabd35 100644 --- a/lib/kodo/integrations/credential_encryption.ex +++ b/lib/kodo/integrations/credential_encryption.ex @@ -7,12 +7,13 @@ defmodule Kodo.Integrations.CredentialEncryption do @nonce_bytes 12 @tag_bytes 16 @key_bytes 32 + @key_version ~r/\A[A-Za-z0-9][A-Za-z0-9._-]{0,63}\z/ @doc "Encrypts a credential payload using the configured current key." def encrypt(%Integration{} = integration, payload) when is_map(payload) do with {:ok, ring} <- key_ring(), {:ok, associated_data} <- associated_data(integration, @format_version), - {:ok, plaintext} <- Jason.encode(payload) do + {:ok, plaintext} <- encode_payload(payload) do nonce = :crypto.strong_rand_bytes(@nonce_bytes) key = Map.fetch!(ring.keys, ring.current_key_version) @@ -34,10 +35,13 @@ defmodule Kodo.Integrations.CredentialEncryption do credential_format_version: @format_version }} else + {:error, :credential_payload_invalid} = error -> error _error -> {:error, :credential_encryption_unavailable} end end + def encrypt(_integration, _payload), do: {:error, :credential_payload_invalid} + @doc "Decrypts and authenticates an integration's credential payload." def decrypt(%Integration{credential_format_version: version}) when version != @format_version, do: {:error, :credential_payload_version_unsupported} @@ -101,7 +105,18 @@ defmodule Kodo.Integrations.CredentialEncryption do defp valid_keys?(_keys), do: false - defp valid_version?(version), do: is_binary(version) and version != "" + defp valid_version?(version), do: is_binary(version) and Regex.match?(@key_version, version) + + defp encode_payload(payload) do + try do + case Jason.encode(payload) do + {:ok, encoded} -> {:ok, encoded} + {:error, _reason} -> {:error, :credential_payload_invalid} + end + rescue + _exception -> {:error, :credential_payload_invalid} + end + end defp associated_data(integration, format_version) do values = [ diff --git a/lib/kodo/integrations/integration.ex b/lib/kodo/integrations/integration.ex index 4b0f018..39a20d9 100644 --- a/lib/kodo/integrations/integration.ex +++ b/lib/kodo/integrations/integration.ex @@ -45,6 +45,7 @@ defmodule Kodo.Integrations.Integration do @doc false def constraint_changeset(changeset) do changeset + |> foreign_key_constraint(:user_id) |> check_constraint(:provider, name: :provider_integrations_provider_valid) |> check_constraint(:authentication_type, name: :provider_integrations_authentication_type_valid diff --git a/test/kodo/integrations/credential_encryption_test.exs b/test/kodo/integrations/credential_encryption_test.exs index de1e0c2..272102a 100644 --- a/test/kodo/integrations/credential_encryption_test.exs +++ b/test/kodo/integrations/credential_encryption_test.exs @@ -117,6 +117,29 @@ defmodule Kodo.Integrations.CredentialEncryptionTest do CredentialEncryption.encrypt(%Integration{}, %{"api_key" => "secret"}) end + test "rejects malformed payloads without raising or exposing them in diagnostics" do + sentinel = "plaintext-provider-secret" + + malformed_payloads = [sentinel, %{"api_key" => ["value" | {sentinel}]}] + + for payload <- malformed_payloads do + assert result = {:error, :credential_payload_invalid} + assert ^result = CredentialEncryption.encrypt(integration(), payload) + refute inspect(result) =~ sentinel + end + end + + test "rejects key versions that cannot be persisted" do + invalid_versions = ["contains spaces", "é", String.duplicate("a", 65)] + + for version <- invalid_versions do + put_config(version, %{version => :binary.copy(<<1>>, 32)}) + + assert {:error, :credential_encryption_config_invalid} = + CredentialEncryption.validate_config() + end + end + defp integration do %Integration{ id: Ecto.UUID.generate(), diff --git a/test/kodo/integrations_test.exs b/test/kodo/integrations_test.exs index 867ef68..f3da2b0 100644 --- a/test/kodo/integrations_test.exs +++ b/test/kodo/integrations_test.exs @@ -4,6 +4,8 @@ defmodule Kodo.IntegrationsTest do alias Kodo.AccountsFixtures alias Kodo.Integrations alias Kodo.Integrations.CredentialEncryption + alias Kodo.Integrations.Integration + alias Kodo.Test.BlockingJSONValue describe "scoped credential lifecycle" do setup do @@ -35,8 +37,13 @@ defmodule Kodo.IntegrationsTest do test "enforces one integration per user and provider", %{scope: scope} do assert {:ok, _integration} = connect(scope) - assert {:error, changeset} = connect(scope) - assert "has already been taken" in errors_on(changeset).user_id + assert {:error, :integration_already_exists} = connect(scope) + end + + test "returns a bounded error when connection races account deletion", %{scope: scope} do + Repo.delete!(scope.user) + + assert {:error, :integration_owner_not_found} = connect(scope) end test "replaces credentials with a new nonce and advances the generation", %{scope: scope} do @@ -117,8 +124,10 @@ defmodule Kodo.IntegrationsTest do end test "retains provisional OAuth credentials until a fenced refresh succeeds", %{scope: scope} do + integration = oauth_integration(scope) + assert {:ok, integration} = - Integrations.connect(scope, "openai_codex", "oauth", %{ + Integrations.oauth_succeeded(scope, integration.id, 0, %{ "access_token" => "old-access", "refresh_token" => "old-refresh", "account_id" => "account" @@ -174,9 +183,35 @@ defmodule Kodo.IntegrationsTest do ) end + test "rejects raw connect and replacement APIs for OAuth credentials", %{scope: scope} do + assert {:error, :authentication_type_mismatch} = + Integrations.connect(scope, "openai_codex", "oauth", %{ + "access_token" => "raw-access", + "refresh_token" => "raw-refresh" + }) + + integration = oauth_integration(scope) + + assert {:ok, connected} = + Integrations.oauth_succeeded(scope, integration.id, 0, %{ + "access_token" => "authorized", + "refresh_token" => "refresh" + }) + + assert {:error, :authentication_type_mismatch} = + Integrations.replace_credentials( + scope, + connected.id, + connected.credential_generation, + %{"access_token" => "raw-replacement"} + ) + end + test "installs a generation-fenced OAuth authorization after disconnection", %{scope: scope} do + integration = oauth_integration(scope) + assert {:ok, integration} = - Integrations.connect(scope, "openai_codex", "oauth", %{ + Integrations.oauth_succeeded(scope, integration.id, 0, %{ "access_token" => "old-access", "refresh_token" => "old-refresh" }) @@ -218,9 +253,224 @@ defmodule Kodo.IntegrationsTest do assert {:ok, current} = Integrations.get_integration(scope, integration.id) assert current.connection_status == "connected" end + + test "admits exactly one of two replacements at the same generation", %{scope: scope} do + assert {:ok, integration} = connect(scope) + supervisor = start_supervised!(Task.Supervisor) + owner = self() + + tasks = + for suffix <- ["first", "second"] do + ref = make_ref() + + task = + async_operation(supervisor, fn -> + Integrations.replace_credentials( + scope, + integration.id, + integration.credential_generation, + %{"api_key" => blocking_value(owner, ref, "replacement-#{suffix}")} + ) + end) + + {task, ref} + end + + Enum.each(tasks, fn {task, ref} -> assert_encoding_blocked(task, ref) end) + Enum.each(tasks, fn {task, ref} -> send(task.pid, {:continue_json_encoding, ref}) end) + + results = Enum.map(tasks, fn {task, _ref} -> Task.await(task) end) + + assert Enum.count(results, &match?({:ok, _integration}, &1)) == 1 + + assert Enum.count(results, &(&1 == {:error, :stale_credential_generation})) == 1 + + assert {:ok, current} = Integrations.get_integration(scope, integration.id) + assert current.credential_generation == integration.credential_generation + 1 + end + + test "delayed credential and validation results cannot undo disconnection", %{scope: scope} do + assert {:ok, api_integration} = connect(scope) + api_generation = api_integration.credential_generation + supervisor = start_supervised!(Task.Supervisor) + owner = self() + replace_ref = make_ref() + + replace_task = + async_operation(supervisor, fn -> + Integrations.replace_credentials( + scope, + api_integration.id, + api_generation, + %{"api_key" => blocking_value(owner, replace_ref, "delayed")} + ) + end) + + assert_encoding_blocked(replace_task, replace_ref) + + assert {:ok, _disconnected} = + Integrations.disconnect(scope, api_integration.id, api_generation) + + send(replace_task.pid, {:continue_json_encoding, replace_ref}) + + assert {:error, :stale_credential_generation} = + Task.await(replace_task) + + assert {:error, :stale_credential_generation} = + Integrations.validation_invalid(scope, api_integration.id, api_generation) + + oauth = oauth_integration(scope) + oauth_ref = make_ref() + + oauth_task = + async_operation(supervisor, fn -> + Integrations.oauth_succeeded(scope, oauth.id, 0, %{ + "access_token" => blocking_value(owner, oauth_ref, "access"), + "refresh_token" => "refresh" + }) + end) + + assert_encoding_blocked(oauth_task, oauth_ref) + assert {:ok, oauth_disconnected} = Integrations.disconnect(scope, oauth.id, 0) + send(oauth_task.pid, {:continue_json_encoding, oauth_ref}) + + assert {:error, :stale_credential_generation} = Task.await(oauth_task) + assert oauth_disconnected.connection_status == "disconnected" + + refresh_scope = AccountsFixtures.user_scope_fixture() + refresh = oauth_integration(refresh_scope) + + assert {:ok, refresh} = + Integrations.oauth_succeeded(refresh_scope, refresh.id, 0, %{ + "access_token" => "access", + "refresh_token" => "refresh" + }) + + refresh_ref = make_ref() + + refresh_task = + async_operation(supervisor, fn -> + Integrations.refresh_succeeded( + refresh_scope, + refresh.id, + refresh.credential_generation, + %{ + "access_token" => blocking_value(owner, refresh_ref, "new-access"), + "refresh_token" => "new-refresh" + } + ) + end) + + assert_encoding_blocked(refresh_task, refresh_ref) + + assert {:ok, _disconnected} = + Integrations.disconnect( + refresh_scope, + refresh.id, + refresh.credential_generation + ) + + send(refresh_task.pid, {:continue_json_encoding, refresh_ref}) + assert {:error, :stale_credential_generation} = Task.await(refresh_task) + + assert {:ok, current} = Integrations.get_integration(refresh_scope, refresh.id) + assert current.connection_status == "disconnected" + assert is_nil(current.encrypted_credentials) + end + + test "deletion after credential lookup cannot recreate integration state", %{scope: scope} do + assert {:ok, integration} = connect(scope) + supervisor = start_supervised!(Task.Supervisor) + owner = self() + ref = make_ref() + + task = + async_operation(supervisor, fn -> + Integrations.replace_credentials( + scope, + integration.id, + integration.credential_generation, + %{"api_key" => blocking_value(owner, ref, "late-replacement")} + ) + end) + + assert_encoding_blocked(task, ref) + Repo.delete!(scope.user) + send(task.pid, {:continue_json_encoding, ref}) + + assert {:error, :stale_credential_generation} = Task.await(task) + + refute Repo.get(Integration, integration.id) + end + + test "rejects validation and refresh transitions from illegal connection states", %{ + scope: scope + } do + oauth = oauth_integration(scope) + + assert {:error, :stale_credential_generation} = + Integrations.validation_succeeded(scope, oauth.id, 0) + + assert {:error, :stale_credential_generation} = + Integrations.refresh_succeeded(scope, oauth.id, 0, %{ + "access_token" => "access", + "refresh_token" => "refresh" + }) + + assert {:ok, connected} = + Integrations.oauth_succeeded(scope, oauth.id, 0, %{ + "access_token" => "access", + "refresh_token" => "refresh" + }) + + assert {:ok, reauthorization} = + Integrations.refresh_invalid_grant( + scope, + connected.id, + connected.credential_generation + ) + + assert {:error, :stale_credential_generation} = + Integrations.validation_succeeded( + scope, + reauthorization.id, + reauthorization.credential_generation + ) + + assert {:error, :stale_credential_generation} = + Integrations.refresh_invalid_grant( + scope, + reauthorization.id, + reauthorization.credential_generation + ) + end end defp connect(scope) do Integrations.connect(scope, "openai", "api_key", %{"api_key" => "provider-secret"}) end + + defp oauth_integration(scope) do + %Integration{user_id: scope.user.id} + |> Integration.create_changeset(%{ + provider: "openai_codex", + authentication_type: "oauth" + }) + |> Repo.insert!() + end + + defp async_operation(supervisor, operation) do + task = Task.Supervisor.async_nolink(supervisor, operation) + Ecto.Adapters.SQL.Sandbox.allow(Repo, self(), task.pid) + task + end + + defp blocking_value(owner, ref, value) do + %BlockingJSONValue{owner: owner, ref: ref, value: value} + end + + defp assert_encoding_blocked(task, ref) do + task_pid = task.pid + assert_receive {:json_encoding_blocked, ^ref, ^task_pid} + end end diff --git a/test/kodo_web/parameter_filtering_test.exs b/test/kodo_web/parameter_filtering_test.exs index c95e91b..4e2866b 100644 --- a/test/kodo_web/parameter_filtering_test.exs +++ b/test/kodo_web/parameter_filtering_test.exs @@ -9,12 +9,15 @@ defmodule KodoWeb.ParameterFilteringTest do "refresh_token", "id_token", "device_code", + "device_auth_id", "user_code", + "code_verifier", "client_secret", "authorization", "encrypted_credentials", "account_id", "chatgpt_account_id", + "chatgpt_user_id", "organization_id", "workspace_id" ] diff --git a/test/support/blocking_json_value.ex b/test/support/blocking_json_value.ex new file mode 100644 index 0000000..08a4b54 --- /dev/null +++ b/test/support/blocking_json_value.ex @@ -0,0 +1,5 @@ +defmodule Kodo.Test.BlockingJSONValue do + @moduledoc false + + defstruct [:owner, :ref, :value] +end diff --git a/test/support/blocking_json_value_encoder.ex b/test/support/blocking_json_value_encoder.ex new file mode 100644 index 0000000..c66ff88 --- /dev/null +++ b/test/support/blocking_json_value_encoder.ex @@ -0,0 +1,10 @@ +defimpl Jason.Encoder, for: Kodo.Test.BlockingJSONValue do + def encode(value, opts) do + send(value.owner, {:json_encoding_blocked, value.ref, self()}) + + receive do + {:continue_json_encoding, ref} when ref == value.ref -> + Jason.Encoder.encode(value.value, opts) + end + end +end