mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(github-app): PAT fallback covers every mint failure; video/motion/x cleanups
mint_installation_token now wraps JWT build + HTTP + parsing so raw httpx/jwt failures surface as GitHubAppError and the existing PAT fallback catches them all (a GitHub outage no longer crashes git operations for App-bound projects). The PEM is validated at credential-set time instead of first mint, and the installation-token cache is cleared when credentials are deleted. Also: the video preview root resolves symlinks like its sibling route (frames under a symlinked workspaces_root no longer false-404), the tracked dangling motion/node_modules symlink is removed and the gitignore gains a slash-less entry that actually matches symlinks, changelog caption input strips GHSA refs like PR refs, and the edit-project dialog hides the GitHub App section for auto-detected gitlab.com projects and warns before a save that would clear both auth sources.
This commit is contained in:
@@ -10,6 +10,8 @@ from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from cryptography.hazmat.primitives import serialization
|
||||
from cryptography.hazmat.primitives.asymmetric import rsa
|
||||
from roboco.db.tables import GitHubAppCredentialsTable
|
||||
from roboco.services.github_app_credentials import (
|
||||
GitHubAppCredentialsService,
|
||||
@@ -23,9 +25,19 @@ if TYPE_CHECKING:
|
||||
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
|
||||
def _generate_rsa_pem() -> str:
|
||||
private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
|
||||
return private_key.private_bytes(
|
||||
encoding=serialization.Encoding.PEM,
|
||||
format=serialization.PrivateFormat.PKCS8,
|
||||
encryption_algorithm=serialization.NoEncryption(),
|
||||
).decode()
|
||||
|
||||
|
||||
_CREDS = {
|
||||
"app_id": "123456",
|
||||
"private_key": "-----BEGIN PRIVATE KEY-----\nfake-pem\n-----END PRIVATE KEY-----",
|
||||
"private_key": _generate_rsa_pem(),
|
||||
}
|
||||
|
||||
|
||||
@@ -88,9 +100,16 @@ async def test_rotate_overwrites_previous_values(
|
||||
svc: GitHubAppCredentialsService,
|
||||
) -> None:
|
||||
await svc.set_credentials(**_CREDS)
|
||||
rotated = {"app_id": "654321", "private_key": _CREDS["private_key"] + "-rotated"}
|
||||
rotated = {"app_id": "654321", "private_key": _generate_rsa_pem()}
|
||||
await svc.set_credentials(**rotated)
|
||||
decrypted = await svc.get_decrypted()
|
||||
assert decrypted is not None
|
||||
assert decrypted.app_id == rotated["app_id"]
|
||||
assert decrypted.private_key == rotated["private_key"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_malformed_pem_is_rejected(svc: GitHubAppCredentialsService) -> None:
|
||||
with pytest.raises(GitHubAppCredentialsValidationError):
|
||||
await svc.set_credentials(app_id="123456", private_key="not-a-pem-at-all")
|
||||
assert await svc.has_credentials() is False
|
||||
|
||||
@@ -15,6 +15,8 @@ from uuid import UUID, uuid4
|
||||
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from cryptography.hazmat.primitives import serialization
|
||||
from cryptography.hazmat.primitives.asymmetric import rsa
|
||||
from fastapi import FastAPI
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
from roboco.api.deps import get_agent_context, get_db
|
||||
@@ -34,6 +36,18 @@ if TYPE_CHECKING:
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
|
||||
def _generate_rsa_pem() -> str:
|
||||
private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
|
||||
return private_key.private_bytes(
|
||||
encoding=serialization.Encoding.PEM,
|
||||
format=serialization.PrivateFormat.PKCS8,
|
||||
encryption_algorithm=serialization.NoEncryption(),
|
||||
).decode()
|
||||
|
||||
|
||||
_VALID_PEM = _generate_rsa_pem()
|
||||
|
||||
|
||||
def _build_app(db_session: AsyncSession, role: AgentRole, agent_id: UUID) -> FastAPI:
|
||||
app = FastAPI()
|
||||
app.include_router(github_app_router, prefix="/api/github-app")
|
||||
@@ -71,14 +85,11 @@ async def test_set_credentials_reports_status_never_plaintext(
|
||||
) -> None:
|
||||
resp = await ceo_client.put(
|
||||
"/api/github-app/credentials",
|
||||
json={
|
||||
"app_id": "123456",
|
||||
"private_key": "-----BEGIN KEY-----\nsecretpem\n-----END KEY-----",
|
||||
},
|
||||
json={"app_id": "123456", "private_key": _VALID_PEM},
|
||||
)
|
||||
assert resp.status_code == HTTPStatus.OK
|
||||
assert resp.json() == {"has_credentials": True}
|
||||
assert "secretpem" not in resp.text
|
||||
assert _VALID_PEM.splitlines()[1] not in resp.text
|
||||
|
||||
status_resp = await ceo_client.get("/api/github-app/credentials")
|
||||
assert status_resp.json()["has_credentials"] is True
|
||||
@@ -93,11 +104,28 @@ async def test_partial_credentials_is_400(ceo_client: AsyncClient) -> None:
|
||||
assert resp.status_code == HTTPStatus.BAD_REQUEST
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_malformed_pem_is_400(ceo_client: AsyncClient) -> None:
|
||||
# Compares before/after rather than asserting an absolute False: routes
|
||||
# commit directly (unlike the service-layer tests' rolled-back flush),
|
||||
# so an earlier test in this module may have already left credentials
|
||||
# set — the contract under test is "a rejected PUT changes nothing".
|
||||
before = await ceo_client.get("/api/github-app/credentials")
|
||||
resp = await ceo_client.put(
|
||||
"/api/github-app/credentials",
|
||||
json={"app_id": "123456", "private_key": "not-a-real-pem"},
|
||||
)
|
||||
assert resp.status_code == HTTPStatus.BAD_REQUEST
|
||||
|
||||
after = await ceo_client.get("/api/github-app/credentials")
|
||||
assert after.json() == before.json()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_clear_credentials(ceo_client: AsyncClient) -> None:
|
||||
await ceo_client.put(
|
||||
"/api/github-app/credentials",
|
||||
json={"app_id": "123456", "private_key": "pem-body"},
|
||||
json={"app_id": "123456", "private_key": _VALID_PEM},
|
||||
)
|
||||
resp = await ceo_client.delete("/api/github-app/credentials")
|
||||
assert resp.status_code == HTTPStatus.OK
|
||||
@@ -107,6 +135,17 @@ async def test_clear_credentials(ceo_client: AsyncClient) -> None:
|
||||
assert status_resp.json()["has_credentials"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_clear_credentials_clears_token_cache(ceo_client: AsyncClient) -> None:
|
||||
"""Deleting the App credentials must drop any cached installation
|
||||
token — otherwise a revoked/replaced App keeps serving a stale token
|
||||
until the cache entry's own expiry."""
|
||||
with patch("roboco.api.routes.github_app.clear_token_cache") as mock_clear:
|
||||
resp = await ceo_client.delete("/api/github-app/credentials")
|
||||
assert resp.status_code == HTTPStatus.OK
|
||||
mock_clear.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_installations_not_configured_is_409(ceo_client: AsyncClient) -> None:
|
||||
with patch(
|
||||
|
||||
@@ -1456,18 +1456,33 @@ async def test_preview_frame_non_ceo_is_forbidden(db_session: AsyncSession) -> N
|
||||
app.dependency_overrides.clear()
|
||||
|
||||
|
||||
def test_resolve_preview_path_confines_frame_orientation_traversal(
|
||||
tmp_path: Path,
|
||||
def test_previews_root_confines_frame_orientation_traversal_through_symlink(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""The preview-frame route composes ``f"{orientation}/{filename}"`` before
|
||||
calling ``_resolve_preview_path`` — same confinement the composition-HTML
|
||||
proxy uses, applied to a task's .previews/ dir instead of its read-clone."""
|
||||
root = (tmp_path / "roboco-x" / ".previews" / "abcd1234").resolve()
|
||||
(root / "vertical").mkdir(parents=True)
|
||||
frame = root / "vertical" / "frame-01-of-1-at-0.5s.png"
|
||||
"""Drives the REAL ``_previews_root`` -> ``_resolve_preview_path`` chain
|
||||
(the preview-frame route composes ``f"{orientation}/{filename}"`` before
|
||||
calling ``_resolve_preview_path``) with an UNRESOLVED, symlinked
|
||||
``workspaces_root`` — the container-mount shape. Before ``_previews_root``
|
||||
resolved its own path, the ``is_relative_to`` confinement check compared a
|
||||
resolved candidate against an unresolved root and 404'd every legit
|
||||
frame; this proves a real frame under the symlink still serves while
|
||||
traversal is still rejected."""
|
||||
real_root = tmp_path / "real-workspaces"
|
||||
real_root.mkdir()
|
||||
symlinked_root = tmp_path / "workspaces-symlink"
|
||||
symlinked_root.symlink_to(real_root)
|
||||
monkeypatch.setattr(cfg, "workspaces_root", str(symlinked_root))
|
||||
|
||||
task_id = uuid4()
|
||||
project_slug = "roboco-x"
|
||||
frame_dir = real_root / project_slug / ".previews" / task_id.hex[:8] / "vertical"
|
||||
frame_dir.mkdir(parents=True)
|
||||
frame = frame_dir / "frame-01-of-1-at-0.5s.png"
|
||||
frame.write_bytes(b"png")
|
||||
secret = tmp_path / "secret.png"
|
||||
secret.write_bytes(b"nope")
|
||||
|
||||
root = video_module._previews_root(project_slug, task_id)
|
||||
assert (
|
||||
video_module._resolve_preview_path(root, "vertical/frame-01-of-1-at-0.5s.png")
|
||||
== frame.resolve()
|
||||
|
||||
@@ -13,6 +13,7 @@ from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import httpx
|
||||
import jwt
|
||||
import pytest
|
||||
from cryptography.hazmat.primitives import serialization
|
||||
@@ -20,7 +21,9 @@ from cryptography.hazmat.primitives.asymmetric import rsa
|
||||
from roboco.services import github_app_auth
|
||||
from roboco.services.github_app_auth import (
|
||||
GitHubAppAPIError,
|
||||
GitHubAppError,
|
||||
GitHubAppNotConfiguredError,
|
||||
clear_token_cache,
|
||||
list_installation_repositories,
|
||||
list_installations,
|
||||
mint_installation_token,
|
||||
@@ -62,7 +65,7 @@ def _patch_creds(*, configured: bool = True) -> Any:
|
||||
|
||||
|
||||
def _client(
|
||||
*, get: list[MagicMock] | None = None, post: list[MagicMock] | None = None
|
||||
*, get: list[Any] | None = None, post: list[Any] | None = None
|
||||
) -> MagicMock:
|
||||
client = MagicMock()
|
||||
client.__aenter__ = AsyncMock(return_value=client)
|
||||
@@ -163,6 +166,57 @@ async def test_mint_failure_raises_api_error() -> None:
|
||||
await mint_installation_token(MagicMock(), 5)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mint_network_error_wrapped_as_github_app_error() -> None:
|
||||
"""A raw httpx failure (connection refused, DNS, timeout) must not escape
|
||||
as an unexpected exception type — ProjectService._resolve_token only
|
||||
catches GitHubAppError to fall back to the PAT."""
|
||||
client = _client(post=[httpx.ConnectError("connection refused")])
|
||||
with (
|
||||
_patch_creds(),
|
||||
patch("roboco.services.github_app_auth.httpx.AsyncClient", return_value=client),
|
||||
pytest.raises(GitHubAppError) as exc_info,
|
||||
):
|
||||
await mint_installation_token(MagicMock(), 55)
|
||||
assert not isinstance(exc_info.value, GitHubAppAPIError)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mint_corrupted_pem_wrapped_as_github_app_error() -> None:
|
||||
"""A corrupted stored private key makes ``jwt.encode`` raise
|
||||
``InvalidKeyError`` — must also surface as GitHubAppError, not the raw
|
||||
PyJWT exception, so the PAT fallback still triggers."""
|
||||
fake_service = MagicMock()
|
||||
fake_service.get_decrypted = AsyncMock(
|
||||
return_value=GitHubAppCredentialsData(app_id=_APP_ID, private_key="not-a-pem")
|
||||
)
|
||||
with (
|
||||
patch(
|
||||
"roboco.services.github_app_auth.get_github_app_credentials_service",
|
||||
return_value=fake_service,
|
||||
),
|
||||
pytest.raises(GitHubAppError) as exc_info,
|
||||
):
|
||||
await mint_installation_token(MagicMock(), 66)
|
||||
assert not isinstance(exc_info.value, GitHubAppAPIError)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_clear_token_cache_forces_a_fresh_mint() -> None:
|
||||
client = _client(post=[_token_resp("tok-first"), _token_resp("tok-second")])
|
||||
with (
|
||||
_patch_creds(),
|
||||
patch("roboco.services.github_app_auth.httpx.AsyncClient", return_value=client),
|
||||
):
|
||||
first = await mint_installation_token(MagicMock(), 77)
|
||||
clear_token_cache()
|
||||
second = await mint_installation_token(MagicMock(), 77)
|
||||
|
||||
assert first == "tok-first"
|
||||
assert second == "tok-second"
|
||||
assert client.post.call_count == _SECOND_CALL_COUNT
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_installations_maps_account_login() -> None:
|
||||
payload = [
|
||||
|
||||
@@ -13,12 +13,28 @@ from __future__ import annotations
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from uuid import uuid4
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from cryptography.hazmat.primitives import serialization
|
||||
from cryptography.hazmat.primitives.asymmetric import rsa
|
||||
from roboco.services.github_app_auth import GitHubAppAPIError
|
||||
from roboco.services.github_app_credentials import GitHubAppCredentialsData
|
||||
from roboco.services.project import ProjectService
|
||||
from roboco.utils.crypto import encrypt_token
|
||||
|
||||
|
||||
def _generate_rsa_pem() -> str:
|
||||
private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
|
||||
return private_key.private_bytes(
|
||||
encoding=serialization.Encoding.PEM,
|
||||
format=serialization.PrivateFormat.PKCS8,
|
||||
encryption_algorithm=serialization.NoEncryption(),
|
||||
).decode()
|
||||
|
||||
|
||||
_VALID_PEM = _generate_rsa_pem()
|
||||
|
||||
|
||||
def _project(*, installation_id: int | None, pat: str | None) -> MagicMock:
|
||||
p = MagicMock()
|
||||
p.id = uuid4()
|
||||
@@ -99,6 +115,62 @@ async def test_mint_failure_falls_back_to_pat() -> None:
|
||||
assert token == "ghp_fallback"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mint_network_error_falls_back_to_pat() -> None:
|
||||
"""A raw httpx failure inside the REAL mint path (not mocked away) must
|
||||
still fall back to the PAT — github_app_auth wraps it as GitHubAppError
|
||||
at its own chokepoint, which _resolve_token's existing except clause
|
||||
already catches."""
|
||||
svc = _svc_with_get(_project(installation_id=42, pat="ghp_fallback"))
|
||||
fake_creds_svc = MagicMock()
|
||||
fake_creds_svc.has_credentials = AsyncMock(return_value=True)
|
||||
fake_creds_svc.get_decrypted = AsyncMock(
|
||||
return_value=GitHubAppCredentialsData(app_id="1", private_key=_VALID_PEM)
|
||||
)
|
||||
client = MagicMock()
|
||||
client.__aenter__ = AsyncMock(return_value=client)
|
||||
client.__aexit__ = AsyncMock(return_value=False)
|
||||
client.post = AsyncMock(side_effect=httpx.ConnectError("connection refused"))
|
||||
with (
|
||||
patch(
|
||||
"roboco.services.project.get_github_app_credentials_service",
|
||||
return_value=fake_creds_svc,
|
||||
),
|
||||
patch(
|
||||
"roboco.services.github_app_auth.get_github_app_credentials_service",
|
||||
return_value=fake_creds_svc,
|
||||
),
|
||||
patch("roboco.services.github_app_auth.httpx.AsyncClient", return_value=client),
|
||||
):
|
||||
token = await svc.get_decrypted_token(uuid4())
|
||||
assert token == "ghp_fallback"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mint_corrupted_pem_falls_back_to_pat() -> None:
|
||||
"""A corrupted stored private key makes the REAL mint path's
|
||||
``jwt.encode`` raise ``InvalidKeyError`` before any network call — must
|
||||
also fall back to the PAT."""
|
||||
svc = _svc_with_get(_project(installation_id=42, pat="ghp_fallback"))
|
||||
fake_creds_svc = MagicMock()
|
||||
fake_creds_svc.has_credentials = AsyncMock(return_value=True)
|
||||
fake_creds_svc.get_decrypted = AsyncMock(
|
||||
return_value=GitHubAppCredentialsData(app_id="1", private_key="not-a-pem")
|
||||
)
|
||||
with (
|
||||
patch(
|
||||
"roboco.services.project.get_github_app_credentials_service",
|
||||
return_value=fake_creds_svc,
|
||||
),
|
||||
patch(
|
||||
"roboco.services.github_app_auth.get_github_app_credentials_service",
|
||||
return_value=fake_creds_svc,
|
||||
),
|
||||
):
|
||||
token = await svc.get_decrypted_token(uuid4())
|
||||
assert token == "ghp_fallback"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_mint_failure_with_no_pat_returns_none() -> None:
|
||||
svc = _svc_with_get(_project(installation_id=42, pat=None))
|
||||
|
||||
@@ -1425,19 +1425,21 @@ def test_changelog_highlights_extracts_feature_headlines() -> None:
|
||||
entry = (
|
||||
"## [0.26.0] - 2026-07-20\n\n"
|
||||
"### Security\n\n"
|
||||
"- **Orchestrator API is off the public internet (GHSA-4f7g).** Both "
|
||||
"composes published :8000 on 0.0.0.0.\n\n"
|
||||
"- **Orchestrator API is off the public internet (GHSA-4f7g-w95g-5q2c).** "
|
||||
"Both composes published :8000 on 0.0.0.0.\n\n"
|
||||
"### Added\n\n"
|
||||
"- **Telegram Mini App V5 — brand voice and an operations ring (#583).** "
|
||||
"Share Tech Mono becomes the display face.\n"
|
||||
"- **Forge program: GitHub, Gitea, and GitLab (#575, #581).** One API.\n"
|
||||
)
|
||||
hl = x_engine_module.changelog_highlights(entry)
|
||||
assert hl[0] == "Orchestrator API is off the public internet (GHSA-4f7g)"
|
||||
# A GHSA advisory ref is stripped exactly like a PR ref — neither belongs
|
||||
# in a caption prompt's feature headline.
|
||||
assert hl[0] == "Orchestrator API is off the public internet"
|
||||
assert hl[1] == "Telegram Mini App V5 — brand voice and an operations ring"
|
||||
assert hl[2] == "Forge program: GitHub, Gitea, and GitLab"
|
||||
# No raw commit-subject noise, no trailing PR refs or periods.
|
||||
assert all("#" not in h.split("(GHSA")[0] for h in hl)
|
||||
# No raw commit-subject noise, no trailing PR/GHSA refs or periods.
|
||||
assert all("#" not in h and "GHSA" not in h for h in hl)
|
||||
|
||||
|
||||
def test_changelog_highlights_empty_on_no_leads() -> None:
|
||||
|
||||
Reference in New Issue
Block a user