fix: fixed TD-01 and TD-25

lib/bds/ai/one_shot.ex

@@ -406,7 +406,9 @@ defmodule BDS.AI.OneShot do
        title: media.title || "",
        alt: media.alt || "",
        caption: media.caption || "",
-       image_url: Map.get(media, :image_url),
+       # A stored media row has no remote URL; resolve_image_data_url/1 fills
+       # this from file_path before an :analyze_image request is built.
+       image_url: nil,
        file_path: media.file_path,
        project_id: media.project_id,
        language: media.language || ""

lib/bds/ai/secret_backend.ex

@@ -1,25 +1,91 @@
 defmodule BDS.AI.SecretBackend do
-  @moduledoc false
+  @moduledoc """
+  Encrypts and decrypts AI provider secrets (AES-256-GCM) with the
+  machine-local master key resolved by `BDS.AI.SecretKey`.
+
+  Values written by earlier releases — encrypted with key material that
+  shipped in the repository, or with the deterministic node-name fallback —
+  are still readable: `decrypt/1` falls back to the legacy keys, and
+  `BDS.AI.SecretMigration` re-encrypts such rows at boot. When no master key
+  can be obtained, both operations return `{:error, :secret_key_unavailable}`
+  instead of degrading to a weaker key.
+  """
+
+  require Logger
+
+  alias BDS.AI.SecretKey
 
   @aad "bds-ai-secret"
 
+  # Key material shipped in the repository before TD-01. Retained only so
+  # existing user databases can be read and re-encrypted by
+  # BDS.AI.SecretMigration; remove both together in a future release.
+  @legacy_repo_key binary_part(
+                     "bds_desktop_shell_secret_key_base_64_chars_minimum_seed_value_001",
+                     0,
+                     32
+                   )
+
+  @spec encrypt(String.t()) :: {:ok, String.t()} | {:error, term()}
   def encrypt(value) when is_binary(value) do
-    key = secret_key()
-    iv = :crypto.strong_rand_bytes(12)
+    with {:ok, key} <- secret_key() do
+      iv = :crypto.strong_rand_bytes(12)
 
-    {ciphertext, tag} =
-      :crypto.crypto_one_time_aead(:aes_256_gcm, key, iv, value, @aad, true)
+      {ciphertext, tag} =
+        :crypto.crypto_one_time_aead(:aes_256_gcm, key, iv, value, @aad, true)
 
-    {:ok, Base.encode64(iv <> tag <> ciphertext)}
+      {:ok, Base.encode64(iv <> tag <> ciphertext)}
+    end
   end
 
+  @spec decrypt(String.t()) :: {:ok, String.t()} | {:error, term()}
   def decrypt(encoded) when is_binary(encoded) do
+    with {:ok, key} <- secret_key() do
+      case decrypt_with(encoded, key) do
+        {:ok, plaintext} -> {:ok, plaintext}
+        {:error, :invalid_ciphertext} -> decrypt_legacy(encoded)
+      end
+    end
+  end
+
+  @doc """
+  Decrypts strictly with the current master key — no legacy fallback. Used by
+  `BDS.AI.SecretMigration` to detect rows that still need re-encryption.
+  """
+  @spec decrypt_with_current_key(String.t()) :: {:ok, String.t()} | {:error, term()}
+  def decrypt_with_current_key(encoded) when is_binary(encoded) do
+    with {:ok, key} <- secret_key() do
+      decrypt_with(encoded, key)
+    end
+  end
+
+  @doc """
+  Attempts decryption with the legacy keys used by earlier releases.
+  """
+  @spec decrypt_legacy(String.t()) :: {:ok, String.t()} | {:error, :invalid_ciphertext}
+  def decrypt_legacy(encoded) when is_binary(encoded) do
+    Enum.find_value(legacy_keys(), {:error, :invalid_ciphertext}, fn key ->
+      case decrypt_with(encoded, key) do
+        {:ok, plaintext} -> {:ok, plaintext}
+        {:error, :invalid_ciphertext} -> nil
+      end
+    end)
+  end
+
+  defp legacy_keys do
+    [
+      @legacy_repo_key,
+      :crypto.hash(:sha256, Atom.to_string(node()) <> ":bds:ai")
+    ]
+  end
+
+  defp decrypt_with(encoded, key) do
     with {:ok, binary} <- Base.decode64(encoded),
          <<iv::binary-size(12), tag::binary-size(16), ciphertext::binary>> <- binary,
          plaintext when is_binary(plaintext) <-
            :crypto.crypto_one_time_aead(
              :aes_256_gcm,
-             secret_key(),
+             key,
              iv,
              ciphertext,
              @aad,
@@ -33,9 +99,13 @@ defmodule BDS.AI.SecretBackend do
   end
 
   defp secret_key do
-    case Application.get_env(:bds, :ai_secret_key) do
-      key when is_binary(key) and byte_size(key) >= 32 -> binary_part(key, 0, 32)
-      _other -> :crypto.hash(:sha256, Atom.to_string(node()) <> ":bds:ai")
+    case SecretKey.fetch() do
+      {:ok, key} ->
+        {:ok, key}
+
+      {:error, reason} ->
+        Logger.error("AI secret key unavailable: #{inspect(reason)}")
+        {:error, :secret_key_unavailable}
     end
   end
 end

lib/bds/ai/secret_key.ex

@@ -0,0 +1,233 @@
+defmodule BDS.AI.SecretKey do
+  @moduledoc """
+  Resolves the 32-byte machine-local master key that encrypts AI provider
+  secrets at rest (the `SecureKeyStore` entity in `specs/ai.allium`).
+
+  Resolution order:
+
+  1. `config :bds, :ai_secret_key` — explicit override. Used by the test
+     suite for determinism; must be at least 32 bytes (the first 32 are
+     used). An invalid value is an error, never a silent fallback.
+  2. A previously resolved key cached in `:persistent_term`.
+  3. The OS keyring: on macOS the login Keychain via the `security` CLI; on
+     other platforms — or when the Keychain is unavailable — a random key
+     file under the private app dir, written with `0600` permissions.
+
+  A fresh random key is generated and stored on first use. There is no
+  deterministic fallback: when no key can be obtained or persisted, `fetch/0`
+  returns `{:error, reason}` and secret encryption/decryption fails loudly
+  instead of degrading to obfuscation.
+
+  Config (`config :bds, BDS.AI.SecretKey`):
+
+    * `:strategy` — `:auto` (default; Keychain on macOS, key file elsewhere),
+      `:keychain`, or `:file`
+    * `:key_file_path` — overrides the key file location
+    * `:command_runner` — 3-arity replacement for `System.cmd/3` (tests)
+  """
+
+  require Logger
+
+  @key_bytes 32
+  @cache_key {__MODULE__, :key}
+  @keychain_service "bDS2"
+  @keychain_account "ai-secret-key"
+  @keychain_not_found_status 44
+  @key_file_name "ai_secret.key"
+
+  @spec fetch() :: {:ok, binary()} | {:error, term()}
+  def fetch do
+    case configured_key() do
+      {:ok, key} -> {:ok, key}
+      {:error, _detail} = error -> error
+      :unset -> cached_or_resolve()
+    end
+  end
+
+  @doc "Clears the cached key so the next fetch re-resolves it (test helper)."
+  @spec reset_cache() :: :ok
+  def reset_cache do
+    :persistent_term.erase(@cache_key)
+    :ok
+  end
+
+  defp configured_key do
+    case Application.get_env(:bds, :ai_secret_key) do
+      nil ->
+        :unset
+
+      key when is_binary(key) and byte_size(key) >= @key_bytes ->
+        {:ok, binary_part(key, 0, @key_bytes)}
+
+      other ->
+        {:error, {:invalid_configured_key, "expected a binary of at least #{@key_bytes} bytes, got: #{inspect(other)}"}}
+    end
+  end
+
+  defp cached_or_resolve do
+    case :persistent_term.get(@cache_key, nil) do
+      key when is_binary(key) ->
+        {:ok, key}
+
+      nil ->
+        with {:ok, key} <- resolve() do
+          :persistent_term.put(@cache_key, key)
+          {:ok, key}
+        end
+    end
+  end
+
+  defp resolve do
+    case strategy() do
+      :keychain -> resolve_keychain()
+      :file -> resolve_file()
+    end
+  end
+
+  defp strategy do
+    case config(:strategy, :auto) do
+      :auto ->
+        if match?({:unix, :darwin}, :os.type()), do: :keychain, else: :file
+
+      explicit when explicit in [:keychain, :file] ->
+        explicit
+    end
+  end
+
+  # ─── macOS Keychain ─────────────────────────────────────────
+
+  defp resolve_keychain do
+    case keychain_find() do
+      {:ok, key} ->
+        {:ok, key}
+
+      :not_found ->
+        keychain_create()
+
+      {:error, reason} ->
+        Logger.warning(
+          "AI secret key: macOS Keychain unavailable (#{inspect(reason)}); falling back to the key file"
+        )
+
+        resolve_file()
+    end
+  end
+
+  defp keychain_find do
+    case run_security([
+           "find-generic-password",
+           "-s",
+           @keychain_service,
+           "-a",
+           @keychain_account,
+           "-w"
+         ]) do
+      {output, 0} ->
+        case output |> String.trim() |> Base.decode64() do
+          {:ok, key} when byte_size(key) == @key_bytes -> {:ok, key}
+          _other -> {:error, :corrupt_keychain_item}
+        end
+
+      {_output, @keychain_not_found_status} ->
+        :not_found
+
+      {output, status} ->
+        {:error, {:security_failed, status, String.trim(output)}}
+    end
+  end
+
+  # The generated key passes through `security`'s argv, which is briefly
+  # visible to other local processes of the same user. Accepted trade-off for
+  # a single-user desktop app; `security` offers no non-interactive way to
+  # take the password on stdin in one shot.
+  defp keychain_create do
+    key = :crypto.strong_rand_bytes(@key_bytes)
+
+    case run_security([
+           "add-generic-password",
+           "-U",
+           "-s",
+           @keychain_service,
+           "-a",
+           @keychain_account,
+           "-w",
+           Base.encode64(key)
+         ]) do
+      {_output, 0} ->
+        {:ok, key}
+
+      {output, status} ->
+        Logger.warning(
+          "AI secret key: could not store the key in the Keychain " <>
+            "(status #{status}: #{String.trim(output)}); falling back to the key file"
+        )
+
+        resolve_file()
+    end
+  end
+
+  defp run_security(args) do
+    runner = config(:command_runner, &default_runner/3)
+    runner.("security", args, stderr_to_stdout: true)
+  end
+
+  defp default_runner(command, args, opts) do
+    System.cmd(command, args, opts)
+  rescue
+    error in ErlangError -> {"#{command} unavailable: #{inspect(error.original)}", 127}
+  end
+
+  # ─── Key file ───────────────────────────────────────────────
+
+  defp resolve_file do
+    path = key_file_path()
+
+    case File.read(path) do
+      {:ok, contents} -> decode_key_file(contents, path)
+      {:error, :enoent} -> create_key_file(path)
+      {:error, reason} -> {:error, {:key_file_unreadable, path, reason}}
+    end
+  end
+
+  defp decode_key_file(contents, path) do
+    case contents |> String.trim() |> Base.decode64() do
+      {:ok, key} when byte_size(key) == @key_bytes -> {:ok, key}
+      _other -> {:error, {:key_file_corrupt, path}}
+    end
+  end
+
+  defp create_key_file(path) do
+    key = :crypto.strong_rand_bytes(@key_bytes)
+    temp_path = path <> ".tmp." <> Integer.to_string(System.unique_integer([:positive]))
+
+    with :ok <- File.mkdir_p(Path.dirname(path)),
+         :ok <- File.write(temp_path, Base.encode64(key) <> "\n"),
+         :ok <- File.chmod(temp_path, 0o600),
+         :ok <- File.rename(temp_path, path) do
+      {:ok, key}
+    else
+      {:error, reason} ->
+        _ = File.rm(temp_path)
+        {:error, {:key_file_write_failed, path, reason}}
+    end
+  end
+
+  defp key_file_path do
+    config(:key_file_path, nil) || Path.join(private_app_dir(), @key_file_name)
+  end
+
+  # Same private app dir as BDS.Projects.private_app_dir/0 — on macOS
+  # ~/Library/Application Support/BDS2. Duplicated to keep this module free of
+  # project/DB dependencies.
+  defp private_app_dir do
+    case :filename.basedir(:user_config, "BDS2") do
+      path when is_list(path) -> List.to_string(path)
+      path -> path
+    end
+    |> Path.expand()
+  end
+
+  defp config(key, default) do
+    Application.get_env(:bds, __MODULE__, []) |> Keyword.get(key, default)
+  end
+end

lib/bds/ai/secret_migration.ex

@@ -0,0 +1,67 @@
+defmodule BDS.AI.SecretMigration do
+  @moduledoc """
+  Idempotent boot-time re-encryption of stored AI secrets.
+
+  Earlier releases encrypted secrets with key material shipped in the
+  repository (or a deterministic node-name fallback). This pass finds the
+  `__encrypted_*` rows in `settings`, decrypts them with the legacy keys, and
+  re-encrypts them with the machine-local key from `BDS.AI.SecretKey`. Rows
+  already encrypted with the current key are left untouched; rows no known
+  key can decrypt are left in place and reported, so the user can re-enter
+  the secret. Runs from `BDS.RepoBootstrap` on every boot; on a migrated
+  database it is a cheap no-op.
+  """
+
+  import Ecto.Query
+
+  require Logger
+
+  alias BDS.AI.SecretBackend
+  alias BDS.Persistence
+  alias BDS.Repo
+  alias BDS.Settings.Setting
+
+  @encrypted_prefix "__encrypted_"
+
+  @spec migrate_legacy_secrets(module()) ::
+          {:ok, %{migrated: non_neg_integer(), failed: non_neg_integer()}}
+  def migrate_legacy_secrets(repo \\ Repo) do
+    summary =
+      from(setting in Setting, where: like(setting.key, ^"#{@encrypted_prefix}%"))
+      |> repo.all()
+      |> Enum.reduce(%{migrated: 0, failed: 0}, fn setting, acc ->
+        case migrate_row(repo, setting) do
+          :current -> acc
+          :migrated -> %{acc | migrated: acc.migrated + 1}
+          :failed -> %{acc | failed: acc.failed + 1}
+        end
+      end)
+
+    {:ok, summary}
+  end
+
+  defp migrate_row(repo, setting) do
+    with {:error, _no_current_key_match} <- SecretBackend.decrypt_with_current_key(setting.value),
+         {:ok, plaintext} <- SecretBackend.decrypt_legacy(setting.value),
+         {:ok, reencrypted} <- SecretBackend.encrypt(plaintext) do
+      repo.update_all(
+        from(s in Setting, where: s.key == ^setting.key),
+        set: [value: reencrypted, updated_at: Persistence.now_ms()]
+      )
+
+      Logger.info("AI secret #{setting.key} re-encrypted with the machine-local key")
+      :migrated
+    else
+      {:ok, _already_current} ->
+        :current
+
+      {:error, reason} ->
+        Logger.warning(
+          "AI secret #{setting.key} could not be re-encrypted (#{inspect(reason)}); " <>
+            "leaving it unchanged — the secret may need to be entered again"
+        )
+
+        :failed
+    end
+  end
+end