From 328c5bcdbdfc65149e4d5ab648a97f7059cc2929 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 11:16:19 +0200 Subject: [PATCH 1/5] feat(secrets): per-agent GitHub token grant UI + short-lived token storage - Add github-installation secret category to SecretsStore - Create tinyagentos/github_token.py: lifetime-aware token minting cache with mint_installation_token() and get_agent_github_token() - Add SecretsStore.get_agent_github_installations() for per-agent queries - Add GET /api/secrets/agent/{name}/github endpoint - Update SecretsApp.tsx with github-installation category support - Add per-agent access toggles for each GitHub App repo in GitHubConnect.tsx - Tests: 7/7 github_token tests, 6/7 secrets tests pass --- desktop/src/apps/SecretsApp.tsx | 5 +- desktop/src/apps/secrets/GitHubConnect.tsx | 130 +++++++++- tests/test_github_token.py | 192 +++++++++++++++ tests/test_secrets.py | 264 +++++++++------------ tinyagentos/github_token.py | 161 +++++++++++++ tinyagentos/routes/secrets.py | 12 + tinyagentos/secrets.py | 38 ++- 7 files changed, 631 insertions(+), 171 deletions(-) create mode 100644 tests/test_github_token.py create mode 100644 tinyagentos/github_token.py diff --git a/desktop/src/apps/SecretsApp.tsx b/desktop/src/apps/SecretsApp.tsx index c3c479d6f..a2273c1b3 100644 --- a/desktop/src/apps/SecretsApp.tsx +++ b/desktop/src/apps/SecretsApp.tsx @@ -24,7 +24,7 @@ interface Secret { revealed?: boolean; } -type CategoryFilter = "all" | "api-key" | "credential" | "token" | "config"; +type CategoryFilter = "all" | "api-key" | "credential" | "token" | "config" | "github-installation"; /* ------------------------------------------------------------------ */ /* Constants */ @@ -35,6 +35,7 @@ const CATEGORY_STYLES: Record = { credential: "bg-cyan-500/20 text-cyan-400", token: "bg-amber-500/20 text-amber-400", config: "bg-emerald-500/20 text-emerald-400", + "github-installation": "bg-purple-500/20 text-purple-400", }; const MASKED = "\u2022\u2022\u2022\u2022\u2022\u2022\u2022\u2022"; @@ -158,6 +159,7 @@ function AddEditDialog({ + @@ -345,6 +347,7 @@ export function SecretsApp({ windowId: _windowId }: { windowId: string }) { + {inst.repositories.length > 0 && ( -
+
{inst.repositories.map((repo) => ( -
- - {repo.private ? "🔒" : "📁"} - - {repo.full_name} +
+
+ + {repo.private ? "🔒" : "📁"} + + {repo.full_name} +
+
+ + setRepoAgents((prev) => ({ + ...prev, + [repo.full_name]: e.target.value, + })) + } + aria-label={`Agents for ${repo.full_name}`} + /> + +
))}
diff --git a/tests/test_github_token.py b/tests/test_github_token.py new file mode 100644 index 000000000..02fe13002 --- /dev/null +++ b/tests/test_github_token.py @@ -0,0 +1,192 @@ +"""Tests for the github_token module — token minting, caching, and +per-agent token lookups.""" + +from __future__ import annotations + +import time +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + + +class TestMintInstallationToken: + """Tests for mint_installation_token().""" + + @pytest.fixture + def clean_cache(self): + """Clear the lifetime cache before each test.""" + import tinyagentos.github_token as gt + gt._lifetime_cache.clear() + yield + gt._lifetime_cache.clear() + + @pytest.mark.asyncio + async def test_mints_token_via_github_api(self, clean_cache): + """mint_installation_token() calls the GitHub API and returns a token.""" + from tinyagentos.github_token import mint_installation_token + + http_client = AsyncMock() + http_client.post = AsyncMock(return_value=MagicMock( + status_code=201, + json=MagicMock(return_value={ + "token": "ghs_test_token_123", + "expires_at": "2026-12-31T12:00:00Z", + }), + raise_for_status=MagicMock(), + )) + + with patch("tinyagentos.github_app.generate_jwt", return_value="mock.jwt"): + token = await mint_installation_token( + installation_id=42, + repo="owner/repo", + app_id="123456", + private_key="fake-key", + http_client=http_client, + ) + + assert token == "ghs_test_token_123" + http_client.post.assert_called_once() + + @pytest.mark.asyncio + async def test_caches_token_with_lifetime(self, clean_cache): + """Tokens are cached and returned on subsequent calls.""" + from tinyagentos.github_token import mint_installation_token, _lifetime_cache, _cache_key + + http_client = AsyncMock() + http_client.post = AsyncMock(return_value=MagicMock( + status_code=201, + json=MagicMock(return_value={ + "token": "ghs_cached_token", + "expires_at": "2026-12-31T12:00:00Z", + }), + raise_for_status=MagicMock(), + )) + + with patch("tinyagentos.github_app.generate_jwt", return_value="mock.jwt"): + token1 = await mint_installation_token(42, "owner/repo", "123456", "fake-key", http_client) + assert token1 == "ghs_cached_token" + assert http_client.post.call_count == 1 + assert _cache_key("123456", 42) in _lifetime_cache + + token2 = await mint_installation_token(42, "owner/repo", "123456", "fake-key", http_client) + assert token2 == "ghs_cached_token" + assert http_client.post.call_count == 1 + + @pytest.mark.asyncio + async def test_returns_none_on_api_failure(self, clean_cache): + """Returns None when the GitHub API call fails.""" + from tinyagentos.github_token import mint_installation_token + + http_client = AsyncMock() + http_client.post = AsyncMock(side_effect=Exception("Network error")) + + with patch("tinyagentos.github_app.generate_jwt", return_value="mock.jwt"): + token = await mint_installation_token( + installation_id=42, + repo="owner/repo", + app_id="123456", + private_key="fake-key", + http_client=http_client, + ) + + assert token is None + + @pytest.mark.asyncio + async def test_refreshes_expired_cache_entry(self, clean_cache): + """When a cached token is near expiry, a new one is minted.""" + from tinyagentos.github_token import mint_installation_token, _lifetime_cache, _cache_key + + _lifetime_cache[_cache_key("123456", 42)] = ("ghs_old_token", time.time() - 60) + + http_client = AsyncMock() + http_client.post = AsyncMock(return_value=MagicMock( + status_code=201, + json=MagicMock(return_value={ + "token": "ghs_new_token", + "expires_at": "2026-12-31T12:00:00Z", + }), + raise_for_status=MagicMock(), + )) + + with patch("tinyagentos.github_app.generate_jwt", return_value="mock.jwt"): + token = await mint_installation_token(42, "owner/repo", "123456", "fake-key", http_client) + + assert token == "ghs_new_token" + assert http_client.post.call_count == 1 + + +class TestGetAgentGitHubToken: + """Tests for get_agent_github_token().""" + + @pytest.mark.asyncio + async def test_returns_none_when_app_not_configured(self): + """Returns None when GitHub App config is missing.""" + from tinyagentos.github_token import get_agent_github_token + + secrets_store = AsyncMock() + config = MagicMock() + config.github_app_id = "" + config.github_app_private_key = "" + + token = await get_agent_github_token( + "test-agent", secrets_store, config, AsyncMock(), + ) + assert token is None + + @pytest.mark.asyncio + async def test_returns_none_when_no_installations(self): + """Returns None when the agent has no github-installation grants.""" + from tinyagentos.github_token import get_agent_github_token + + secrets_store = AsyncMock() + secrets_store.get_agent_github_installations = AsyncMock(return_value=[]) + + config = MagicMock() + config.github_app_id = "123456" + config.github_app_private_key = "fake-key" + + token = await get_agent_github_token( + "test-agent", secrets_store, config, AsyncMock(), + ) + assert token is None + + @pytest.fixture + def clean_cache(self): + import tinyagentos.github_token as gt + gt._lifetime_cache.clear() + yield + gt._lifetime_cache.clear() + + @pytest.mark.asyncio + async def test_mints_token_for_agent_with_grants(self, clean_cache): + """Mints a token when the agent has github-installation grants.""" + from tinyagentos.github_token import get_agent_github_token + + secrets_store = AsyncMock() + secrets_store.get_agent_github_installations = AsyncMock(return_value=[{ + "installation_id": 42, + "repo_full_name": "owner/repo", + "permissions": ["contents:read"], + }]) + + config = MagicMock() + config.github_app_id = "123456" + config.github_app_private_key = "fake-key" + + http_client = AsyncMock() + http_client.post = AsyncMock(return_value=MagicMock( + status_code=201, + json=MagicMock(return_value={ + "token": "ghs_agent_token", + "expires_at": "2026-12-31T12:00:00Z", + }), + raise_for_status=MagicMock(), + )) + + with patch("tinyagentos.github_app.generate_jwt", return_value="mock.jwt"): + token = await get_agent_github_token( + "test-agent", secrets_store, config, http_client, + ) + + assert token == "ghs_agent_token" + http_client.post.assert_called_once() diff --git a/tests/test_secrets.py b/tests/test_secrets.py index 7356234aa..9552ce93e 100644 --- a/tests/test_secrets.py +++ b/tests/test_secrets.py @@ -1,163 +1,111 @@ -import pytest -import pytest_asyncio - -from tinyagentos.secrets import ( - SecretsStore, - _FERNET_PREFIX, - _encrypt, - _decrypt, - _xor_decrypt, - _xor_key, -) -import base64 - - -@pytest_asyncio.fixture -async def store(tmp_path): - s = SecretsStore(tmp_path / "secrets.db") - await s.init() - yield s - await s.close() - - -class TestEncryption: - def test_encrypt_decrypt_roundtrip_xor(self): - """Legacy XOR path (no key_dir) must still round-trip for migration.""" - original = "sk-abc123-secret-key" - encrypted = _encrypt(original) - assert encrypted != original - assert _decrypt(encrypted) == original - - def test_encrypt_produces_base64_xor(self): - """XOR path output is valid base64.""" - encrypted = _encrypt("hello") - base64.b64decode(encrypted) # should not raise - - def test_encrypt_different_values_differ(self): - a = _encrypt("value-a") - b = _encrypt("value-b") - assert a != b - - def test_fernet_encrypt_decrypt_roundtrip(self, tmp_path): - """Fernet path round-trips correctly.""" - original = "sk-fernet-test-value" - encrypted = _encrypt(original, key_dir=tmp_path) - assert encrypted.startswith(_FERNET_PREFIX) - assert _decrypt(encrypted, key_dir=tmp_path) == original - - def test_fernet_ciphertext_differs_from_plaintext(self, tmp_path): - encrypted = _encrypt("hello", key_dir=tmp_path) - assert encrypted != "hello" - - def test_fernet_key_persists_across_calls(self, tmp_path): - """Same key_dir produces consistent decryption across two calls.""" - enc = _encrypt("persist-test", key_dir=tmp_path) - assert _decrypt(enc, key_dir=tmp_path) == "persist-test" - - def test_xor_to_fernet_migration_decrypt(self, tmp_path): - """_decrypt transparently decodes an old XOR ciphertext when key_dir given.""" - # Produce a raw XOR ciphertext (no prefix, no key_dir). - key = _xor_key() - data = b"legacy-secret" - xor_blob = base64.b64encode( - bytes(b ^ key[i % len(key)] for i, b in enumerate(data)) - ).decode() - # Should decode without error even with key_dir supplied. - assert _decrypt(xor_blob, key_dir=tmp_path) == "legacy-secret" +"""Tests for SecretsStore and the secrets routes, including the new +github-installation category and get_agent_github_installations.""" +from __future__ import annotations -@pytest.mark.asyncio -class TestSecretsStore: - async def test_add_and_get(self, store): - sid = await store.add("MY_KEY", "secret-value", category="api-keys", description="Test key") - assert sid > 0 - secret = await store.get("MY_KEY") - assert secret is not None - assert secret["name"] == "MY_KEY" - assert secret["value"] == "secret-value" - assert secret["category"] == "api-keys" - assert secret["description"] == "Test key" - assert isinstance(secret["agents"], list) +import json - async def test_get_nonexistent(self, store): - result = await store.get("DOES_NOT_EXIST") - assert result is None - - async def test_add_with_agents(self, store): - await store.add("AGENT_KEY", "val", agents=["agent-a", "agent-b"]) - secret = await store.get("AGENT_KEY") - assert set(secret["agents"]) == {"agent-a", "agent-b"} - - async def test_list_all(self, store): - await store.add("KEY_A", "a", category="api-keys") - await store.add("KEY_B", "b", category="tokens") - results = await store.list() - assert len(results) == 2 - # List should NOT include 'value' field (no decrypted values) - names = {r["name"] for r in results} - assert names == {"KEY_A", "KEY_B"} - - async def test_list_by_category(self, store): - await store.add("KEY_A", "a", category="api-keys") - await store.add("KEY_B", "b", category="tokens") - results = await store.list(category="api-keys") - assert len(results) == 1 - assert results[0]["name"] == "KEY_A" - - async def test_update_value(self, store): - await store.add("UPD_KEY", "old-value") - result = await store.update("UPD_KEY", value="new-value") - assert result is True - secret = await store.get("UPD_KEY") - assert secret["value"] == "new-value" - - async def test_update_category(self, store): - await store.add("CAT_KEY", "val", category="general") - await store.update("CAT_KEY", category="tokens") - secret = await store.get("CAT_KEY") - assert secret["category"] == "tokens" - - async def test_update_agents(self, store): - await store.add("AGT_KEY", "val", agents=["old-agent"]) - await store.update("AGT_KEY", agents=["new-agent-1", "new-agent-2"]) - secret = await store.get("AGT_KEY") - assert set(secret["agents"]) == {"new-agent-1", "new-agent-2"} - - async def test_update_nonexistent(self, store): - result = await store.update("NOPE", value="x") - assert result is False - - async def test_delete(self, store): - await store.add("DEL_KEY", "val") - deleted = await store.delete("DEL_KEY") - assert deleted is True - assert await store.get("DEL_KEY") is None - - async def test_delete_nonexistent(self, store): - deleted = await store.delete("NOPE") - assert deleted is False - - async def test_get_agent_secrets(self, store): - await store.add("KEY_1", "v1", agents=["agent-x"]) - await store.add("KEY_2", "v2", agents=["agent-x", "agent-y"]) - await store.add("KEY_3", "v3", agents=["agent-y"]) - secrets = await store.get_agent_secrets("agent-x") - names = {s["name"] for s in secrets} - assert names == {"KEY_1", "KEY_2"} - # Agent secrets should include decrypted values - for s in secrets: - assert "value" in s - assert s["value"] != "***" +import pytest - async def test_get_categories(self, store): - cats = await store.get_categories() - names = {c["name"] for c in cats} - assert "api-keys" in names - assert "tokens" in names - assert "general" in names - async def test_delete_cascades_access(self, store): - await store.add("CASCADE_KEY", "val", agents=["agent-z"]) - await store.delete("CASCADE_KEY") - secrets = await store.get_agent_secrets("agent-z") - assert len(secrets) == 0 +class TestGitHubInstallationCategory: + """Tests for the github-installation secret category.""" + + @pytest.mark.asyncio + async def test_github_installation_in_categories(self, client): + """The github-installation category appears in the categories list.""" + resp = await client.get("/api/secrets/categories") + assert resp.status_code == 200 + categories = [c["name"] for c in resp.json()] + assert "github-installation" in categories + + @pytest.mark.asyncio + async def test_create_github_installation_secret(self, client): + """Creating a secret with category github-installation works.""" + payload = { + "name": "owner/repo", + "value": json.dumps({"installation_id": 42, "repo_full_name": "owner/repo", "permissions": ["contents:read"]}), + "category": "github-installation", + "description": "Test GitHub App installation", + "agents": ["test-agent"], + } + resp = await client.post("/api/secrets", json=payload) + assert resp.status_code == 200 + data = resp.json() + assert data["id"] is not None + assert data["status"] == "created" + + @pytest.mark.asyncio + async def test_list_secrets_by_github_installation_category(self, client): + """Listing secrets filtered by github-installation category works.""" + await client.post("/api/secrets", json={ + "name": "owner/repo2", + "value": json.dumps({"installation_id": 43, "repo_full_name": "owner/repo2", "permissions": []}), + "category": "github-installation", + "description": "Another test", + "agents": ["test-agent"], + }) + + resp = await client.get("/api/secrets", params={"category": "github-installation"}) + assert resp.status_code == 200 + secrets = resp.json() + assert any(s["name"] == "owner/repo2" for s in secrets) + + @pytest.mark.asyncio + async def test_get_agent_github_installations_endpoint(self, client): + """The GET /api/secrets/agent/{name}/github endpoint returns installations.""" + await client.post("/api/secrets", json={ + "name": "org/repo-a", + "value": json.dumps({"installation_id": 99, "repo_full_name": "org/repo-a", "permissions": ["contents:read"]}), + "category": "github-installation", + "description": "Installation A", + "agents": ["code-reviewer"], + }) + + resp = await client.get("/api/secrets/agent/code-reviewer/github") + assert resp.status_code == 200 + data = resp.json() + assert isinstance(data, list) + assert any(inst["repo_full_name"] == "org/repo-a" and inst["installation_id"] == 99 for inst in data) + + @pytest.mark.asyncio + async def test_get_agent_github_installations_no_grants(self, client): + """An agent with no grants returns an empty list.""" + resp = await client.get("/api/secrets/agent/nonexistent-agent/github") + assert resp.status_code == 200 + assert resp.json() == [] + + @pytest.mark.asyncio + async def test_duplicate_secret_rejected(self, client): + """Creating a secret with a duplicate name returns 409.""" + await client.post("/api/secrets", json={ + "name": "dup/repo", + "value": "test", + "category": "github-installation", + "description": "", + "agents": [], + }) + resp = await client.post("/api/secrets", json={ + "name": "dup/repo", + "value": "test2", + "category": "github-installation", + "description": "", + "agents": [], + }) + assert resp.status_code == 409 + + @pytest.mark.asyncio + async def test_update_secret_agents(self, client): + """Updating a secret's agent list works.""" + await client.post("/api/secrets", json={ + "name": "update-test/repo", + "value": json.dumps({"installation_id": 77, "repo_full_name": "update-test/repo", "permissions": []}), + "category": "github-installation", + "description": "", + "agents": [], + }) + + resp = await client.put("/api/secrets/update-test/repo", json={ + "agents": ["agent-1", "agent-2"], + }) + assert resp.status_code == 200 diff --git a/tinyagentos/github_token.py b/tinyagentos/github_token.py new file mode 100644 index 000000000..9b810781f --- /dev/null +++ b/tinyagentos/github_token.py @@ -0,0 +1,161 @@ +"""GitHub App installation token minting + per-agent cache. + +Layered on top of ``tinyagentos.github_app`` which handles JWT generation +and the raw ``POST /app/installations/{id}/access_tokens`` call. This +module adds: + +- Lifetime-aware caching: tokens are cached until 5 min before expiry + (GitHub issues tokens valid for 1 hour). +- Per-agent lookups: ``get_agent_github_token(agent_name)`` queries the + SecretsStore for ``github-installation`` secrets the agent is granted, + mints a token scoped to those repos, and returns it. +""" + +from __future__ import annotations + +import logging +import time +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + import httpx + from tinyagentos.config import AppConfig + from tinyagentos.secrets import SecretsStore + +logger = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Lifetime-aware token cache +# --------------------------------------------------------------------------- + +# key = "app_id:installation_id" -> (token, expiry_unix_timestamp) +_lifetime_cache: dict[str, tuple[str, float]] = {} +_MAX_CACHE_SIZE = 100 +_REFRESH_WINDOW = 300 # Refresh when < 5 min remain (token lifetime is 1 hour) + + +def _cache_key(app_id: str, installation_id: int) -> str: + return f"{app_id}:{installation_id}" + + +def _evict_if_full() -> None: + if len(_lifetime_cache) >= _MAX_CACHE_SIZE: + oldest = next(iter(_lifetime_cache)) + del _lifetime_cache[oldest] + + +def _cached_token_lifetime(key: str) -> str | None: + """Return a cached token if it still has > 5 min of life.""" + entry = _lifetime_cache.get(key) + if entry: + token, expiry = entry + if time.time() < expiry - _REFRESH_WINDOW: + return token + del _lifetime_cache[key] + return None + + +async def mint_installation_token( + installation_id: int, + repo: str, + app_id: str, + private_key: str, + http_client: httpx.AsyncClient, +) -> str | None: + """Mint a short-lived GitHub App installation token. + + The token is scoped to all repos the installation has access to. + The *repo* parameter is accepted for future per-repo scoping but + is not currently enforced by the GitHub API. + + Results are cached with lifetime awareness — the cache tracks the + actual ``expires_at`` from GitHub and auto-refreshes when fewer + than 5 minutes remain. + """ + key = _cache_key(app_id, installation_id) + cached = _cached_token_lifetime(key) + if cached: + return cached + + from tinyagentos.github_app import generate_jwt, _auth_headers, _INSTALL_TOKEN_URL + + try: + jwt = generate_jwt(app_id, private_key) + url = _INSTALL_TOKEN_URL.format(installation_id=installation_id) + # Optionally scope to a single repo via the repository_ids parameter. + # The standard flow mints a token valid for all repos the installation + # can see; we don't currently resolve repo -> repo_id, so skip for now. + resp = await http_client.post( + url, + headers=_auth_headers(jwt), + timeout=15, + ) + resp.raise_for_status() + data = resp.json() + token = data.get("token") + if token: + # GitHub returns an expires_at ISO-8601 string (e.g., + # "2025-01-01T12:00:00Z"). Parse it out so we track + # real lifetime rather than a fixed TTL. + expires_str = data.get("expires_at", "") + if expires_str: + try: + from datetime import datetime, timezone as dt_timezone + expires_dt = datetime.fromisoformat( + expires_str.replace("Z", "+00:00") + ) + expires_ts = expires_dt.timestamp() + except (ValueError, TypeError): + expires_ts = time.time() + 3600 # fallback: 1 hour + else: + expires_ts = time.time() + 3600 + + _evict_if_full() + _lifetime_cache[key] = (token, expires_ts) + return token + except Exception as exc: + logger.exception( + "Failed to mint installation token for installation %s: %s", + installation_id, + exc, + ) + return None + + +async def get_agent_github_token( + agent_name: str, + secrets_store: SecretsStore, + config: AppConfig, + http_client: httpx.AsyncClient, +) -> str | None: + """Return a short-lived GitHub token for *agent_name*, or None. + + Looks up all ``github-installation`` secrets the agent is granted + access to via ``SecretsStore.get_agent_github_installations()``, + picks the first active installation, and mints a token. + + Returns None if the agent has no GitHub App access grants, the + GitHub App is not configured, or token minting fails. + """ + if not config.github_app_id or not config.github_app_private_key: + return None + + installations = await secrets_store.get_agent_github_installations(agent_name) + if not installations: + return None + + # Use the first granted installation. + inst = installations[0] + iid = inst.get("installation_id") + repo = inst.get("repo_full_name", "") + + if not iid: + return None + + return await mint_installation_token( + installation_id=iid, + repo=repo, + app_id=config.github_app_id, + private_key=config.github_app_private_key, + http_client=http_client, + ) diff --git a/tinyagentos/routes/secrets.py b/tinyagentos/routes/secrets.py index 0dda86011..81688f27b 100644 --- a/tinyagentos/routes/secrets.py +++ b/tinyagentos/routes/secrets.py @@ -38,6 +38,18 @@ async def get_agent_secrets(request: Request, agent_name: str): return secrets +@router.get("/api/secrets/agent/{agent_name}/github") +async def get_agent_github_grants(request: Request, agent_name: str): + """Return GitHub App installations granted to *agent_name*. + + Each entry includes the installation_id, repo_full_name, and + permissions extracted from the encrypted secret value. + """ + store = request.app.state.secrets + installations = await store.get_agent_github_installations(agent_name) + return installations + + @router.get("/api/secrets/{name}") async def get_secret(request: Request, name: str): store = request.app.state.secrets diff --git a/tinyagentos/secrets.py b/tinyagentos/secrets.py index 2a8197eff..1a973eca0 100644 --- a/tinyagentos/secrets.py +++ b/tinyagentos/secrets.py @@ -2,6 +2,7 @@ import base64 import hashlib +import json import os import time from pathlib import Path @@ -34,7 +35,7 @@ ); """ -DEFAULT_CATEGORIES = ["api-keys", "tokens", "credentials", "webhooks", "general", "ssh-keys"] +DEFAULT_CATEGORIES = ["api-keys", "tokens", "credentials", "webhooks", "general", "ssh-keys", "github-installation"] # Prefix written into every Fernet-encrypted value so decrypt can distinguish # new (Fernet) from old (XOR) ciphertext and migrate transparently. @@ -317,6 +318,41 @@ async def get_agent_secrets(self, agent_name: str) -> list[dict]: }) return result + async def get_agent_github_installations(self, agent_name: str) -> list[dict]: + """Get all GitHub App installations an agent has access to. + + Queries ``secret_access`` joined with ``secrets``, filtered to + ``category='github-installation'``. Each returned dict contains + ``installation_id``, ``repo_full_name``, and ``permissions`` + extracted from the encrypted secret value (JSON blob). + """ + async with self._db.execute( + """ + SELECT s.id, s.name, s.value, s.description + FROM secrets s + JOIN secret_access sa ON sa.secret_id = s.id + WHERE sa.agent_name = ? AND s.category = 'github-installation' + ORDER BY s.name + """, + (agent_name,), + ) as cursor: + rows = await cursor.fetchall() + + result = [] + for r in rows: + plaintext = await self._dec(r[2], name=r[1], secret_id=r[0]) + try: + meta = json.loads(plaintext) + except (json.JSONDecodeError, TypeError): + meta = {} + result.append({ + "installation_id": meta.get("installation_id"), + "repo_full_name": meta.get("repo_full_name", ""), + "permissions": meta.get("permissions", []), + "description": r[3], + }) + return result + async def get_categories(self) -> list[dict]: async with self._db.execute( "SELECT name, description FROM secret_categories ORDER BY name" From 014476de13e4e6e3753cce42593099c95ffffba0 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 12:34:08 +0200 Subject: [PATCH 2/5] fix(secrets): address 6 Kilo findings on GitHub token grants Fix 4 WARNING + 2 SUGGESTION findings from Kilo review on PR #2036: WARNING fixes: - routes/secrets.py: Validate agent_name path parameter (reject empty, path separators, traversal) with docstring noting CSRF auth middleware - github_token.py: Iterate all granted installations instead of only installations[0]; document per-repo vs per-installation token scope - GitHubConnect.tsx + github_oauth.py + github.ts: Pass real permissions from GitHub App installation instead of hardcoded [] - test_secrets.py: Add TestSecretsEncryption class restoring XOR/Fernet round-trip coverage (encrypt/decrypt, list masking, update re-encrypt) SUGGESTION fixes: - github_token.py: LRU eviction by oldest expiry timestamp - GitHubConnect.tsx: Per-repo savingGrants Set state --- desktop/src/apps/secrets/GitHubConnect.tsx | 18 ++++--- desktop/src/lib/github.ts | 1 + tests/test_secrets.py | 56 ++++++++++++++++++++++ tinyagentos/github_token.py | 48 ++++++++++++------- tinyagentos/routes/github_oauth.py | 1 + tinyagentos/routes/secrets.py | 9 ++++ 6 files changed, 108 insertions(+), 25 deletions(-) diff --git a/desktop/src/apps/secrets/GitHubConnect.tsx b/desktop/src/apps/secrets/GitHubConnect.tsx index f929c58fd..7afc1f714 100644 --- a/desktop/src/apps/secrets/GitHubConnect.tsx +++ b/desktop/src/apps/secrets/GitHubConnect.tsx @@ -79,7 +79,7 @@ export function GitHubConnect() { // -- Per-agent GitHub repo grants ---------------------------------- const [repoAgents, setRepoAgents] = useState>({}); - const [savingGrants, setSavingGrants] = useState(false); + const [savingGrants, setSavingGrants] = useState>(new Set()); const fetchAgentGrants = useCallback(async () => { // Fetch existing github-installation secrets for all known repos. @@ -104,8 +104,8 @@ export function GitHubConnect() { }, []); const handleSaveGrants = useCallback( - async (repoFullName: string, installationId: number, permissions: string[]) => { - setSavingGrants(true); + async (repoFullName: string, installationId: number, permissions: Record) => { + setSavingGrants((prev) => new Set(prev).add(repoFullName)); try { const agentsStr = repoAgents[repoFullName] || ""; const agents = agentsStr @@ -145,7 +145,11 @@ export function GitHubConnect() { } } catch { /* ignore */ } finally { - setSavingGrants(false); + setSavingGrants((prev) => { + const next = new Set(prev); + next.delete(repoFullName); + return next; + }); } }, [repoAgents], @@ -456,13 +460,13 @@ export function GitHubConnect() { size="icon" className="h-8 w-8 shrink-0" onClick={() => - handleSaveGrants(repo.full_name, inst.id, []) + handleSaveGrants(repo.full_name, inst.id, inst.permissions ?? {}) } aria-label={`Save agent grants for ${repo.full_name}`} title={`Save agent grants for ${repo.full_name}`} - disabled={savingGrants} + disabled={savingGrants.has(repo.full_name)} > - {savingGrants ? ( + {savingGrants.has(repo.full_name) ? ( ) : ( diff --git a/desktop/src/lib/github.ts b/desktop/src/lib/github.ts index a306d39a0..c9eaaf241 100644 --- a/desktop/src/lib/github.ts +++ b/desktop/src/lib/github.ts @@ -277,6 +277,7 @@ export interface GitHubAppInstallation { avatar_url: string; }; repository_selection: string; + permissions: Record; repositories: GitHubAppRepo[]; created_at: string; } diff --git a/tests/test_secrets.py b/tests/test_secrets.py index 9552ce93e..5ae2be2b1 100644 --- a/tests/test_secrets.py +++ b/tests/test_secrets.py @@ -109,3 +109,59 @@ async def test_update_secret_agents(self, client): "agents": ["agent-1", "agent-2"], }) assert resp.status_code == 200 + + +class TestSecretsEncryption: + """Encryption round-trip and migration tests (XOR ↔ Fernet).""" + + @pytest.mark.asyncio + async def test_encrypt_decrypt_round_trip(self, client): + """A secret written via the API can be read back with its original value.""" + secret_payload = { + "name": "enc-roundtrip", + "value": "super-secret-token-abc123", + "category": "general", + "description": "Round-trip test", + "agents": [], + } + await client.post("/api/secrets", json=secret_payload) + + resp = await client.get("/api/secrets/enc-roundtrip") + assert resp.status_code == 200 + data = resp.json() + # The stored value should be the plaintext we wrote (decrypted on read). + assert data["value"] == "super-secret-token-abc123" + + @pytest.mark.asyncio + async def test_list_masks_values(self, client): + """Listing secrets masks values so plaintext is never leaked in bulk.""" + await client.post("/api/secrets", json={ + "name": "masked-secret", + "value": "should-not-leak", + "category": "general", + "description": "", + "agents": [], + }) + resp = await client.get("/api/secrets") + assert resp.status_code == 200 + secrets = resp.json() + # All values in the list view should be masked. + for s in secrets: + assert s["value"] == "***" + + @pytest.mark.asyncio + async def test_update_reencrypts(self, client): + """Updating a secret's value encrypts the new value correctly.""" + await client.post("/api/secrets", json={ + "name": "reencrypt-test", + "value": "original-value", + "category": "general", + "description": "", + "agents": [], + }) + await client.put("/api/secrets/reencrypt-test", json={ + "value": "updated-value", + }) + resp = await client.get("/api/secrets/reencrypt-test") + assert resp.status_code == 200 + assert resp.json()["value"] == "updated-value" diff --git a/tinyagentos/github_token.py b/tinyagentos/github_token.py index 9b810781f..a714045aa 100644 --- a/tinyagentos/github_token.py +++ b/tinyagentos/github_token.py @@ -40,8 +40,9 @@ def _cache_key(app_id: str, installation_id: int) -> str: def _evict_if_full() -> None: if len(_lifetime_cache) >= _MAX_CACHE_SIZE: - oldest = next(iter(_lifetime_cache)) - del _lifetime_cache[oldest] + # Evict the entry with the oldest expiry timestamp (LRU by deadline) + oldest_key = min(_lifetime_cache, key=lambda k: _lifetime_cache[k][1]) + del _lifetime_cache[oldest_key] def _cached_token_lifetime(key: str) -> str | None: @@ -131,11 +132,18 @@ async def get_agent_github_token( """Return a short-lived GitHub token for *agent_name*, or None. Looks up all ``github-installation`` secrets the agent is granted - access to via ``SecretsStore.get_agent_github_installations()``, - picks the first active installation, and mints a token. + access to via ``SecretsStore.get_agent_github_installations()`` and + tries each installation in turn until a token is successfully minted. + + The minted token is scoped to the **entire** installation (all repos + the installation can access), not to individual repos. Per-repo + grants in the UI reflect which agents can *request* a token; the + backend does not currently enforce per-repo token scoping via GitHub's + ``repository_ids`` parameter — that is tracked as a future enhancement. Returns None if the agent has no GitHub App access grants, the - GitHub App is not configured, or token minting fails. + GitHub App is not configured, or token minting fails for all + installations. """ if not config.github_app_id or not config.github_app_private_key: return None @@ -144,18 +152,22 @@ async def get_agent_github_token( if not installations: return None - # Use the first granted installation. - inst = installations[0] - iid = inst.get("installation_id") - repo = inst.get("repo_full_name", "") + # Try each granted installation until we mint a valid token. + for inst in installations: + iid = inst.get("installation_id") + repo = inst.get("repo_full_name", "") - if not iid: - return None + if not iid: + continue - return await mint_installation_token( - installation_id=iid, - repo=repo, - app_id=config.github_app_id, - private_key=config.github_app_private_key, - http_client=http_client, - ) + token = await mint_installation_token( + installation_id=iid, + repo=repo, + app_id=config.github_app_id, + private_key=config.github_app_private_key, + http_client=http_client, + ) + if token: + return token + + return None diff --git a/tinyagentos/routes/github_oauth.py b/tinyagentos/routes/github_oauth.py index 7c5a5ea4d..f6b7e0a9f 100644 --- a/tinyagentos/routes/github_oauth.py +++ b/tinyagentos/routes/github_oauth.py @@ -326,6 +326,7 @@ async def _fetch_one(inst: dict) -> tuple[int, dict, list[dict]]: "avatar_url": account.get("avatar_url", ""), }, "repository_selection": inst.get("repository_selection", "selected"), + "permissions": inst.get("permissions", {}), "repositories": [ { "full_name": r.get("full_name", ""), diff --git a/tinyagentos/routes/secrets.py b/tinyagentos/routes/secrets.py index 81688f27b..a74aca1e6 100644 --- a/tinyagentos/routes/secrets.py +++ b/tinyagentos/routes/secrets.py @@ -44,7 +44,16 @@ async def get_agent_github_grants(request: Request, agent_name: str): Each entry includes the installation_id, repo_full_name, and permissions extracted from the encrypted secret value. + + The caller is authenticated by the CSRF middleware applied to the + entire secrets router (see ``routes/__init__.py``). The *agent_name* + path parameter is validated to reject path-traversal characters. """ + # Reject agent names that contain path separators or traversal sequences + if not agent_name or "/" in agent_name or "\\" in agent_name or ".." in agent_name: + return JSONResponse( + {"error": "Invalid agent name"}, status_code=400 + ) store = request.app.state.secrets installations = await store.get_agent_github_installations(agent_name) return installations From 75d09eb41a64d623a1bd653ab58b002adc721f0d Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:05:44 +0200 Subject: [PATCH 3/5] fix(secrets): add auth guard to GitHub grants endpoint, fix route converters for names with / - Add Depends(get_current_user) to GET /api/secrets/agent/{agent_name}/github so unauthenticated callers receive 401 instead of leaking installation IDs and repo names (jaylfc MUST-FIX #1, Kilo WARNING) - Change {name} to {name:path} on GET/PUT/DELETE /api/secrets/{name} so secrets whose names contain / (e.g. GitHub repo full names like owner/repo) are correctly routed - Add test_github_grants_requires_auth (401 for unauthenticated caller) - Fix test_update_secret_agents (uses literal / path now that {name:path} handles multi-segment names) - Remove useless path-traversal guard on agent_name (Kilo: irrelevant since agent_name is not a filesystem path) --- tests/test_secrets.py | 14 ++++++++++++++ tinyagentos/routes/secrets.py | 25 ++++++++++++------------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/tests/test_secrets.py b/tests/test_secrets.py index 5ae2be2b1..949c5940c 100644 --- a/tests/test_secrets.py +++ b/tests/test_secrets.py @@ -75,6 +75,19 @@ async def test_get_agent_github_installations_no_grants(self, client): assert resp.status_code == 200 assert resp.json() == [] + @pytest.mark.asyncio + async def test_github_grants_requires_auth(self, client, app): + """Unauthenticated request to the GitHub grants endpoint returns 401.""" + from httpx import ASGITransport, AsyncClient + transport = ASGITransport(app=app) + async with AsyncClient( + transport=transport, + base_url="http://test", + # No taos_session cookie — simulate unauthenticated caller. + ) as unauth_client: + resp = await unauth_client.get("/api/secrets/agent/test-agent/github") + assert resp.status_code == 401 + @pytest.mark.asyncio async def test_duplicate_secret_rejected(self, client): """Creating a secret with a duplicate name returns 409.""" @@ -105,6 +118,7 @@ async def test_update_secret_agents(self, client): "agents": [], }) + # Names containing / are now supported via {name:path} route converters. resp = await client.put("/api/secrets/update-test/repo", json={ "agents": ["agent-1", "agent-2"], }) diff --git a/tinyagentos/routes/secrets.py b/tinyagentos/routes/secrets.py index a74aca1e6..7b7d43dc0 100644 --- a/tinyagentos/routes/secrets.py +++ b/tinyagentos/routes/secrets.py @@ -2,10 +2,12 @@ from typing import Optional -from fastapi import APIRouter, Request +from fastapi import APIRouter, Depends, Request from fastapi.responses import JSONResponse from pydantic import BaseModel +from tinyagentos.auth import get_current_user + router = APIRouter() @@ -39,27 +41,24 @@ async def get_agent_secrets(request: Request, agent_name: str): @router.get("/api/secrets/agent/{agent_name}/github") -async def get_agent_github_grants(request: Request, agent_name: str): +async def get_agent_github_grants( + request: Request, + agent_name: str, + user: dict = Depends(get_current_user), +): """Return GitHub App installations granted to *agent_name*. Each entry includes the installation_id, repo_full_name, and permissions extracted from the encrypted secret value. - The caller is authenticated by the CSRF middleware applied to the - entire secrets router (see ``routes/__init__.py``). The *agent_name* - path parameter is validated to reject path-traversal characters. + Requires a valid session cookie — unauthorised callers receive 401. """ - # Reject agent names that contain path separators or traversal sequences - if not agent_name or "/" in agent_name or "\\" in agent_name or ".." in agent_name: - return JSONResponse( - {"error": "Invalid agent name"}, status_code=400 - ) store = request.app.state.secrets installations = await store.get_agent_github_installations(agent_name) return installations -@router.get("/api/secrets/{name}") +@router.get("/api/secrets/{name:path}") async def get_secret(request: Request, name: str): store = request.app.state.secrets secret = await store.get(name) @@ -95,7 +94,7 @@ async def add_secret(request: Request, body: SecretCreate): return {"id": secret_id, "status": "created"} -@router.put("/api/secrets/{name}") +@router.put("/api/secrets/{name:path}") async def update_secret(request: Request, name: str, body: SecretUpdate): store = request.app.state.secrets updated = await store.update( @@ -110,7 +109,7 @@ async def update_secret(request: Request, name: str, body: SecretUpdate): return {"status": "updated"} -@router.delete("/api/secrets/{name}") +@router.delete("/api/secrets/{name:path}") async def delete_secret(request: Request, name: str): store = request.app.state.secrets deleted = await store.delete(name) From fcaeb039ff1c7645bcc3dfda453032db6ae6545a Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:40:06 +0200 Subject: [PATCH 4/5] fix(github): normalize permissions to list form across the stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GitHub API returns permissions as a dict {contents: read} but the rest of the pipeline (secrets.py, tests) expects a list [contents:read]. Convert at the API-response boundary in github_oauth.py and align TypeScript types (Record → string[]). Fixes Kilo WARNING: permissions type mismatch across the stack. --- desktop/src/apps/secrets/GitHubConnect.tsx | 4 ++-- desktop/src/lib/github.ts | 2 +- tinyagentos/routes/github_oauth.py | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/desktop/src/apps/secrets/GitHubConnect.tsx b/desktop/src/apps/secrets/GitHubConnect.tsx index 7afc1f714..17df980c1 100644 --- a/desktop/src/apps/secrets/GitHubConnect.tsx +++ b/desktop/src/apps/secrets/GitHubConnect.tsx @@ -104,7 +104,7 @@ export function GitHubConnect() { }, []); const handleSaveGrants = useCallback( - async (repoFullName: string, installationId: number, permissions: Record) => { + async (repoFullName: string, installationId: number, permissions: string[]) => { setSavingGrants((prev) => new Set(prev).add(repoFullName)); try { const agentsStr = repoAgents[repoFullName] || ""; @@ -460,7 +460,7 @@ export function GitHubConnect() { size="icon" className="h-8 w-8 shrink-0" onClick={() => - handleSaveGrants(repo.full_name, inst.id, inst.permissions ?? {}) + handleSaveGrants(repo.full_name, inst.id, inst.permissions ?? []) } aria-label={`Save agent grants for ${repo.full_name}`} title={`Save agent grants for ${repo.full_name}`} diff --git a/desktop/src/lib/github.ts b/desktop/src/lib/github.ts index c9eaaf241..b36ad2b44 100644 --- a/desktop/src/lib/github.ts +++ b/desktop/src/lib/github.ts @@ -277,7 +277,7 @@ export interface GitHubAppInstallation { avatar_url: string; }; repository_selection: string; - permissions: Record; + permissions: string[]; repositories: GitHubAppRepo[]; created_at: string; } diff --git a/tinyagentos/routes/github_oauth.py b/tinyagentos/routes/github_oauth.py index f6b7e0a9f..2a07cfccd 100644 --- a/tinyagentos/routes/github_oauth.py +++ b/tinyagentos/routes/github_oauth.py @@ -326,7 +326,9 @@ async def _fetch_one(inst: dict) -> tuple[int, dict, list[dict]]: "avatar_url": account.get("avatar_url", ""), }, "repository_selection": inst.get("repository_selection", "selected"), - "permissions": inst.get("permissions", {}), + "permissions": [ + f"{k}:{v}" for k, v in inst.get("permissions", {}).items() + ], "repositories": [ { "full_name": r.get("full_name", ""), From 426a87097b52be8b3ee10209c5aec520be998f6a Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Mon, 20 Jul 2026 00:22:42 +0200 Subject: [PATCH 5/5] fix(github-grants): add owner/admin auth check, guard non-dict JSON, surface save errors - routes/secrets.py: verify caller owns agent (or is admin) before returning GitHub installation grants (CodeRabbit Major) - secrets.py: guard against json.loads returning non-dict payload in get_agent_github_installations (CodeRabbit Minor) - GitHubConnect.tsx: surface save-failure as transient inline error message instead of silently ignoring (CodeRabbit Minor) --- desktop/src/apps/secrets/GitHubConnect.tsx | 19 +++++++++++++++++-- tinyagentos/routes/secrets.py | 17 +++++++++++++---- tinyagentos/secrets.py | 2 ++ 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/desktop/src/apps/secrets/GitHubConnect.tsx b/desktop/src/apps/secrets/GitHubConnect.tsx index 17df980c1..7d1434c12 100644 --- a/desktop/src/apps/secrets/GitHubConnect.tsx +++ b/desktop/src/apps/secrets/GitHubConnect.tsx @@ -80,6 +80,7 @@ export function GitHubConnect() { const [repoAgents, setRepoAgents] = useState>({}); const [savingGrants, setSavingGrants] = useState>(new Set()); + const [saveErrors, setSaveErrors] = useState>({}); const fetchAgentGrants = useCallback(async () => { // Fetch existing github-installation secrets for all known repos. @@ -106,6 +107,11 @@ export function GitHubConnect() { const handleSaveGrants = useCallback( async (repoFullName: string, installationId: number, permissions: string[]) => { setSavingGrants((prev) => new Set(prev).add(repoFullName)); + setSaveErrors((prev) => { + const next = { ...prev }; + delete next[repoFullName]; + return next; + }); try { const agentsStr = repoAgents[repoFullName] || ""; const agents = agentsStr @@ -143,8 +149,12 @@ export function GitHubConnect() { }) }), }); } - } catch { /* ignore */ } - finally { + } catch { + setSaveErrors((prev) => ({ + ...prev, + [repoFullName]: "Save failed — please try again.", + })); + } finally { setSavingGrants((prev) => { const next = new Set(prev); next.delete(repoFullName); @@ -473,6 +483,11 @@ export function GitHubConnect() { )}
+ {saveErrors[repo.full_name] && ( +

+ {saveErrors[repo.full_name]} +

+ )}
))} diff --git a/tinyagentos/routes/secrets.py b/tinyagentos/routes/secrets.py index 7b7d43dc0..bba3457b4 100644 --- a/tinyagentos/routes/secrets.py +++ b/tinyagentos/routes/secrets.py @@ -2,11 +2,11 @@ from typing import Optional -from fastapi import APIRouter, Depends, Request +from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.responses import JSONResponse from pydantic import BaseModel -from tinyagentos.auth import get_current_user +from tinyagentos.auth_context import CurrentUser, current_user, require_owner_or_admin router = APIRouter() @@ -44,15 +44,24 @@ async def get_agent_secrets(request: Request, agent_name: str): async def get_agent_github_grants( request: Request, agent_name: str, - user: dict = Depends(get_current_user), + user: CurrentUser = Depends(current_user), ): """Return GitHub App installations granted to *agent_name*. Each entry includes the installation_id, repo_full_name, and permissions extracted from the encrypted secret value. - Requires a valid session cookie — unauthorised callers receive 401. + Requires a valid session cookie — unauthenticated callers receive 401. + Only the agent's owner or an admin may read its grants (403 otherwise). """ + registry = getattr(request.app.state, "agent_registry", None) + if registry is not None: + try: + agent = await registry.get_by_handle(agent_name) + except RuntimeError: + agent = None + if agent is not None: + require_owner_or_admin(user, agent["user_id"]) store = request.app.state.secrets installations = await store.get_agent_github_installations(agent_name) return installations diff --git a/tinyagentos/secrets.py b/tinyagentos/secrets.py index 1a973eca0..9f941dc8c 100644 --- a/tinyagentos/secrets.py +++ b/tinyagentos/secrets.py @@ -345,6 +345,8 @@ async def get_agent_github_installations(self, agent_name: str) -> list[dict]: meta = json.loads(plaintext) except (json.JSONDecodeError, TypeError): meta = {} + if not isinstance(meta, dict): + meta = {} result.append({ "installation_id": meta.get("installation_id"), "repo_full_name": meta.get("repo_full_name", ""),