Skip to content

Commit e572fe4

Browse files
committed
refactor(ai): centralize plugin crypto on the host KeystoreSecretStore
Also switches ChatViewModel's terminal-tool dedup to isTerminalToolName and adds JVM coverage for MCP keep-alive, the McpServerStore lock and McpPlugin scope cancellation.
1 parent 69c950f commit e572fe4

29 files changed

Lines changed: 686 additions & 573 deletions

File tree

CLAUDE.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,30 @@ There is also **one shared Gradle wrapper at the repo root** (`gradlew` + `gradl
4646

4747
Both jars are referenced via `../libs/*.jar`. **Always use the repo-root `libs/` jars and the repo-root Gradle wrapper — never bundle per-plugin copies.** A plugin that ships its own `libs/plugin-api.jar` / `libs/gradle-plugin.jar` (e.g. copied from another plugin) can drift out of sync with the rest of the repo; point `build.gradle.kts` (`compileOnly`) and `settings.gradle.kts` (buildscript `classpath`) at `../libs/*.jar` and delete any local `libs/`. The root `plugin-api.jar` already carries the full API surface (including `IdeTemplateService`/`CgtTemplateBuilder`), so newer sub-APIs do not justify a local copy. **A plugin folder is not standalone in isolation** — copy the root `libs/` along if you move one elsewhere. When CoGo's API changes, refresh via the script above or the **Update libs from CodeOnTheGo** GitHub Action (which also commits the refreshed jars, cuts a release, and deploys `.cgp` files to the website).
4848

49+
### Credentials: use the host's `KeystoreSecretStore`, never your own crypto
50+
51+
A plugin that stores a credential encrypts it with `com.itsaky.androidide.plugins.security.KeystoreSecretStore`
52+
from `plugin-api.jar` (**26.35+** — set `plugin.min_ide_version` accordingly). It is `compileOnly`
53+
like the rest of the API, so there is one implementation in the IDE's process rather than a copy
54+
compiled into each `.cgp`. Do not re-implement AES/GCM in a plugin; three AI plugins each grew a
55+
copy that started to diverge, which is what ADFA-5255 removed.
56+
57+
Construct it with **this plugin's own alias** (`KeystoreSecretStore(ALIAS)`) as a single
58+
top-level `val` in a `SecureApiKeyStore.kt`/`SecureTokenStore.kt` that holds nothing but the alias;
59+
callers use that instance directly. The store logs under its own name — it takes no log tag, and
60+
the second constructor parameter is a `SecretKeySource` override that plugins do not pass.
61+
`ai-agent-mcp`, `ai-agent-gemini` and `ai-agent-openai` are the reference shape. Do **not** wrap
62+
it in an object of forwarding methods — that is just a second copy of the store's contract to keep
63+
in step. The alias must be unique per
64+
plugin (all plugins share the host's UID and Keystore, so a shared alias lets one plugin's
65+
invalidated-key recovery delete another's secret) and must never change across releases.
66+
67+
`readAndMigrate` returns a three-way `Stored` (`Absent` / `Value` / `Unreadable`) rather than a
68+
nullable String on purpose: "never saved" and "saved but this device's Keystore can no longer open
69+
it" need opposite advice, and a plugin that collapses them tells a user their credential was
70+
refused when it was never sent. Collapse it only where the caller genuinely has one answer for
71+
both, and say so in a comment.
72+
4973
### Plugin shape
5074

5175
A plugin is an Android *application* module (despite installing as a library) with:

ai-agent-gemini/README.md

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,14 @@ The key is entered in **AI Core → Agent settings**, not here. It is stored
2727
encrypted (AES/GCM under a hardware-backed Android Keystore secret) and sent as
2828
an `x-goog-api-key` **header**, never in a URL query string.
2929

30-
`security/SecureApiKeyStore.kt` is the only copy of the crypto — this plugin owns
31-
both the write and the read, so there are no constants to keep in sync with
32-
another plugin. A key written under an earlier plugin id is adopted once by
33-
`preferences/GeminiPreferences.kt` and re-encrypted here.
30+
`security/SecureApiKeyStore.kt` holds only this plugin's Keystore alias
31+
(`cotg_ai_gemini_key_v1`); the AES/GCM itself is the IDE's `KeystoreSecretStore`
32+
(`plugin-api`, since **26.35** — hence this plugin's `min_ide_version`), so there
33+
is one implementation in the process rather than a copy per plugin. The alias
34+
stays per plugin: they all share the host's Keystore, so a shared alias would let
35+
one plugin's invalidated-key recovery delete another's secret. A key written under
36+
an earlier plugin id is adopted once by `preferences/GeminiPreferences.kt` and
37+
re-encrypted here.
3438

3539
## Installation
3640

@@ -57,7 +61,7 @@ root of `com/itsaky/androidide/plugins/aiagentgemini/`.
5761
- `plugin/GeminiPlugin.kt` — plugin entry point; registers the backend with ai-core
5862
- `backend/GeminiBackend.kt` — the REST transport, streaming (SSE) and model catalog
5963
- `errors/GeminiErrorFormatter.kt` — turns an API failure into one translated sentence
60-
- `security/SecureApiKeyStore.kt`AES/GCM at rest
64+
- `security/SecureApiKeyStore.kt`this plugin's Keystore alias, over the IDE's `KeystoreSecretStore`
6165
- `preferences/GeminiPreferences.kt` — this plugin's settings store, plus the
6266
one-time adoption of settings written under earlier plugin ids
6367
- `prompt/GeminiSystemPrompt.kt` — the system prompt this cloud model is given

ai-agent-gemini/ai-agent-gemini.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ <h2>Technical architecture</h2>
8282
<tr><td><code>GeminiErrorFormatter</code></td><td>Classifies a failure
8383
(retired model, quota, refused key, outage, unreachable) so it can be
8484
reported as one translated sentence.</td></tr>
85-
<tr><td><code>SecureApiKeyStore</code></td><td>AES/GCM encryption of the API
86-
key under a hardware-backed Android Keystore secret owned by this
87-
plugin.</td></tr>
85+
<tr><td><code>SecureApiKeyStore</code></td><td>Binds this plugin's Keystore
86+
alias to the IDE's <code>KeystoreSecretStore</code>, which AES/GCM-encrypts
87+
the API key under a hardware-backed Android Keystore secret.</td></tr>
8888
<tr><td><code>GeminiSettingsFragment</code></td><td>The settings pane AI Core
8989
mounts: key entry and verification, visibility toggle, and the model picker
9090
driven by the live catalog.</td></tr>

ai-agent-gemini/build.gradle.kts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ dependencies {
7171
testImplementation("org.json:json:20231013")
7272
}
7373

74-
// SecureApiKeyStore is no longer duplicated: this plugin holds the only copy, so there is nothing
75-
// left to drift against. The parity check that guarded the ai-assistant copy went with that plugin.
74+
// No SecureApiKeyStore parity check any more: the AES/GCM core is the host's KeystoreSecretStore
75+
// (plugin-api), so there is one implementation in the process rather than copies to keep in step.
7676

7777
// AAR metadata checks are disabled by convention for these application-as-library plugins.
7878
tasks.matching {

ai-agent-gemini/src/main/AndroidManifest.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
and pairs with ai-core, which requires the same release. -->
3434
<meta-data
3535
android:name="plugin.min_ide_version"
36-
android:value="26.32" />
36+
android:value="26.35" />
3737

3838
<meta-data
3939
android:name="plugin.max_ide_version"

ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/backend/GeminiBackend.kt

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import com.itsaky.androidide.plugins.aiagentgemini.errors.GeminiErrorFormatter
88
import com.itsaky.androidide.plugins.aiagentgemini.errors.GeminiFailure
99
import com.itsaky.androidide.plugins.aiagentgemini.preferences.GeminiPreferences
1010
import com.itsaky.androidide.plugins.aiagentgemini.prompt.GeminiSystemPrompt
11-
import com.itsaky.androidide.plugins.aiagentgemini.security.SecureApiKeyStore
11+
import com.itsaky.androidide.plugins.aiagentgemini.security.secureApiKeyStore
12+
import com.itsaky.androidide.plugins.security.KeystoreSecretStore
1213
import com.itsaky.androidide.plugins.services.LlmInferenceService.*
1314
import java.io.IOException
1415
import java.net.HttpURLConnection
@@ -116,8 +117,19 @@ class GeminiBackend(
116117
*/
117118
private fun refreshKeyCache(): String? {
118119
val prefs = agentPrefs()
119-
val plain = SecureApiKeyStore.readAndMigrate(prefs, GeminiPreferences.KEY_API_KEY)
120-
?.trim()?.takeIf { it.isNotBlank() }
120+
val plain = when (val stored = secureApiKeyStore.readAndMigrate(prefs, GeminiPreferences.KEY_API_KEY)) {
121+
is KeystoreSecretStore.Stored.Value -> stored.plain.trim().takeIf { it.isNotBlank() }
122+
KeystoreSecretStore.Stored.Absent -> null
123+
// Reported here rather than passed on as "no key": generation fails either way, but a
124+
// lost Keystore entry needs the key entering again, and the log is all that says so.
125+
KeystoreSecretStore.Stored.Unreadable -> {
126+
context.logger.warn(
127+
"GeminiBackend: the saved API key cannot be decrypted on this device; " +
128+
"it has to be entered again in settings"
129+
)
130+
null
131+
}
132+
}
121133
val raw = prefs?.getString(GeminiPreferences.KEY_API_KEY, null)
122134
keyCache = raw?.let { it to plain }
123135
return plain

ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/preferences/GeminiPreferences.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ internal object GeminiPreferences {
5959
* Copies this backend's settings out of every store in [LEGACY_FILES], once.
6060
*
6161
* The API key moves as ciphertext and stays readable: it is encrypted under a Keystore alias
62-
* (see [SecureApiKeyStore]) rather than under anything plugin-specific, and every plugin runs
62+
* (see [secureApiKeyStore]) rather than under anything plugin-specific, and every plugin runs
6363
* in the host's process and UID. Copies rather than moves, so downgrading still finds the old
6464
* values. Call before anything reads a setting.
6565
*
Lines changed: 11 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -1,146 +1,17 @@
11
package com.itsaky.androidide.plugins.aiagentgemini.security
22

3-
import android.content.SharedPreferences
4-
import android.security.keystore.KeyGenParameterSpec
5-
import android.security.keystore.KeyPermanentlyInvalidatedException
6-
import android.security.keystore.KeyProperties
7-
import android.util.Base64
8-
import android.util.Log
9-
import com.itsaky.androidide.plugins.aiagentgemini.logging.LOG_PREFIX
10-
import java.security.GeneralSecurityException
11-
import java.security.KeyStore
12-
import javax.crypto.Cipher
13-
import javax.crypto.KeyGenerator
14-
import javax.crypto.SecretKey
15-
import javax.crypto.spec.GCMParameterSpec
3+
import com.itsaky.androidide.plugins.security.KeystoreSecretStore
4+
5+
/** Unique to this plugin and fixed across releases; see [KeystoreSecretStore] for why both matter. */
6+
private const val ALIAS = "cotg_ai_gemini_key_v1"
167

178
/**
18-
* AES/GCM encryption for sensitive settings (currently the Gemini API key),
19-
* keyed by a hardware-backed Android Keystore secret. Only ciphertext is
20-
* written to SharedPreferences, so a copied prefs file (root, `adb backup`,
21-
* forensic dump) is useless without this device's Keystore.
9+
* This plugin's binding of [KeystoreSecretStore]: its API key, encrypted under this plugin's own
10+
* Keystore alias.
2211
*
23-
* The [ALIAS] must stay stable across releases: a key encrypted under one
24-
* alias cannot be read under another, so changing it silently invalidates
25-
* every stored key. It is also what lets a key written before the AI plugins
26-
* were reorganised still decrypt today — every plugin runs in the host app's
27-
* process and UID, so they all share one Android Keystore.
12+
* The store is the IDE's, from plugin-api, and callers use it directly. A forwarding object per
13+
* method would only be a second copy of its contract to keep in step — and one that had to pick a
14+
* single answer for "absent" and "no longer decryptable", which callers here do not share. The
15+
* thing this file owns is the alias.
2816
*/
29-
object SecureApiKeyStore {
30-
private const val TAG = "$LOG_PREFIX.SecureApiKeyStore"
31-
private const val KEYSTORE = "AndroidKeyStore"
32-
private const val ALIAS = "cotg_ai_gemini_key_v1"
33-
private const val TRANSFORM = "AES/GCM/NoPadding"
34-
private const val IV_LEN = 12
35-
private const val TAG_BITS = 128
36-
37-
/** Marks a stored value as ciphertext; anything without it is treated as legacy plaintext. */
38-
const val ENC_PREFIX = "enc:v1:"
39-
40-
private fun getOrCreateKey(): SecretKey {
41-
val ks = KeyStore.getInstance(KEYSTORE).apply { load(null) }
42-
(ks.getEntry(ALIAS, null) as? KeyStore.SecretKeyEntry)?.let { return it.secretKey }
43-
val generator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, KEYSTORE)
44-
generator.init(
45-
KeyGenParameterSpec.Builder(
46-
ALIAS,
47-
KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT
48-
)
49-
.setBlockModes(KeyProperties.BLOCK_MODE_GCM)
50-
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE)
51-
.build()
52-
)
53-
return generator.generateKey()
54-
}
55-
56-
private fun deleteKey() {
57-
try {
58-
KeyStore.getInstance(KEYSTORE).apply { load(null) }.deleteEntry(ALIAS)
59-
} catch (e: Exception) {
60-
Log.w(TAG, "Failed to delete Keystore alias $ALIAS", e)
61-
}
62-
}
63-
64-
private fun encryptWith(key: SecretKey, plain: String): String {
65-
val cipher = Cipher.getInstance(TRANSFORM)
66-
cipher.init(Cipher.ENCRYPT_MODE, key)
67-
val iv = cipher.iv
68-
val ciphertext = cipher.doFinal(plain.toByteArray(Charsets.UTF_8))
69-
val combined = ByteArray(iv.size + ciphertext.size)
70-
System.arraycopy(iv, 0, combined, 0, iv.size)
71-
System.arraycopy(ciphertext, 0, combined, iv.size, ciphertext.size)
72-
return ENC_PREFIX + Base64.encodeToString(combined, Base64.NO_WRAP)
73-
}
74-
75-
/**
76-
* Encrypt [plain] into a self-describing string: [ENC_PREFIX] + base64(iv | ciphertext).
77-
*
78-
* The key is not auth-bound, so a credential change does not invalidate it; an alias an
79-
* OEM Keystore drops anyway is regenerated once before retrying.
80-
*
81-
* @param plain the value to encrypt
82-
* @throws GeneralSecurityException on any other Keystore/cipher failure, so the caller can
83-
* inform the user instead of crashing the IDE on Save
84-
*/
85-
@Throws(GeneralSecurityException::class)
86-
fun encrypt(plain: String): String {
87-
return try {
88-
encryptWith(getOrCreateKey(), plain)
89-
} catch (e: KeyPermanentlyInvalidatedException) {
90-
Log.w(TAG, "Keystore key invalidated; regenerating and retrying encrypt", e)
91-
deleteKey()
92-
encryptWith(getOrCreateKey(), plain)
93-
}
94-
}
95-
96-
/**
97-
* Return the plaintext for a stored value, handling both formats transparently:
98-
* an [ENC_PREFIX] value is decrypted; anything else is returned unchanged as
99-
* legacy plaintext (use [readAndMigrate] to upgrade it in place). Returns
100-
* null if a ciphertext value can't be decrypted — e.g. the Keystore key was
101-
* lost or invalidated — in which case the user must re-enter the key.
102-
*/
103-
fun decrypt(stored: String?): String? {
104-
if (stored == null) return null
105-
if (!stored.startsWith(ENC_PREFIX)) return stored
106-
return try {
107-
val combined = Base64.decode(stored.removePrefix(ENC_PREFIX), Base64.NO_WRAP)
108-
val iv = combined.copyOfRange(0, IV_LEN)
109-
val ciphertext = combined.copyOfRange(IV_LEN, combined.size)
110-
val cipher = Cipher.getInstance(TRANSFORM)
111-
cipher.init(Cipher.DECRYPT_MODE, getOrCreateKey(), GCMParameterSpec(TAG_BITS, iv))
112-
String(cipher.doFinal(ciphertext), Charsets.UTF_8)
113-
} catch (e: Exception) {
114-
Log.w(TAG, "Failed to decrypt stored API key", e)
115-
null
116-
}
117-
}
118-
119-
/**
120-
* Read [key] from [prefs], upgrading a legacy plaintext value to ciphertext in place.
121-
*
122-
* Keys written before this store existed are still plaintext on disk, and [decrypt] alone
123-
* hands them back unchanged forever — so an install that configured its key earlier would
124-
* never actually gain encryption. Re-encrypting on the first read closes that gap without
125-
* making the user re-enter the key.
126-
*
127-
* The value is trimmed on migration, so the stored, displayed and sent forms all agree.
128-
*
129-
* Keystore IPC + AES/GCM, so call this off the main thread.
130-
*
131-
* @return the trimmed plaintext value, or null when nothing is stored or decryption failed.
132-
*/
133-
fun readAndMigrate(prefs: SharedPreferences?, key: String): String? {
134-
val stored = prefs?.getString(key, null) ?: return null
135-
if (stored.startsWith(ENC_PREFIX)) return decrypt(stored)
136-
val plain = stored.trim()
137-
if (plain.isEmpty()) return plain
138-
try {
139-
prefs.edit().putString(key, encrypt(plain)).apply()
140-
Log.i(TAG, "Upgraded legacy plaintext value for '$key' to ciphertext")
141-
} catch (e: Exception) {
142-
Log.w(TAG, "Could not upgrade legacy plaintext value for '$key' to ciphertext", e)
143-
}
144-
return plain
145-
}
146-
}
17+
val secureApiKeyStore = KeystoreSecretStore(ALIAS)

ai-agent-gemini/src/main/kotlin/com/itsaky/androidide/plugins/aiagentgemini/settings/GeminiSettingsFragment.kt

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import com.itsaky.androidide.plugins.PluginContext
3131
import com.itsaky.androidide.plugins.aiagentgemini.plugin.GeminiPlugin
3232
import com.itsaky.androidide.plugins.aiagentgemini.R
3333
import com.itsaky.androidide.plugins.base.PluginFragmentHelper
34+
import com.itsaky.androidide.plugins.security.KeystoreSecretStore
3435
import com.itsaky.androidide.plugins.services.IdeTooltipService
3536
import kotlinx.coroutines.launch
3637
import java.text.SimpleDateFormat
@@ -189,15 +190,17 @@ class GeminiSettingsFragment : Fragment() {
189190
}
190191

191192
viewLifecycleOwner.lifecycleScope.launch {
192-
val savedApiKey = viewModel.getGeminiApiKey()
193+
val stored = viewModel.getGeminiApiKey()
194+
val savedApiKey = (stored as? KeystoreSecretStore.Stored.Value)?.plain
193195
val hasKey = !savedApiKey.isNullOrBlank()
194196
updateUiState(isEditing = !hasKey)
195197
if (hasKey) {
196198
statusTextView.text = savedApiKeyStatusText()
197199
} else {
198200
apiKeyInput.setText("")
199-
// A stored-but-undecryptable key also reads as null; warn as the Edit path does.
200-
if (viewModel.hasStoredGeminiApiKey()) {
201+
// Only for a key that is there and will not decrypt; an empty box alone looks like
202+
// data loss. Nothing stored at all is the ordinary first run and says nothing.
203+
if (stored is KeystoreSecretStore.Stored.Unreadable) {
201204
Toast.makeText(
202205
requireContext(),
203206
getString(R.string.msg_api_key_unreadable),
@@ -383,20 +386,22 @@ class GeminiSettingsFragment : Fragment() {
383386
editButton.setOnClickListener {
384387
editButton.isEnabled = false
385388
viewLifecycleOwner.lifecycleScope.launch {
386-
val apiKey = try {
389+
val stored = try {
387390
viewModel.getGeminiApiKey()
388391
} finally {
389392
editButton.isEnabled = true
390393
}
391-
// null = a key IS stored but won't decrypt; an empty box alone looks like data loss.
392-
if (apiKey == null) {
394+
// A key that is stored and will not decrypt; an empty box alone looks like data
395+
// loss. Told apart from "nothing stored" here, which this button rarely sees but
396+
// must not report as a lost Keystore entry when it does.
397+
if (stored is KeystoreSecretStore.Stored.Unreadable) {
393398
Toast.makeText(
394399
requireContext(),
395400
getString(R.string.msg_api_key_unreadable),
396401
Toast.LENGTH_LONG
397402
).show()
398403
}
399-
revealEditMode(apiKey.orEmpty())
404+
revealEditMode((stored as? KeystoreSecretStore.Stored.Value)?.plain.orEmpty())
400405
}
401406
}
402407

0 commit comments

Comments
 (0)