agnes-the-ai-analyst/app/api/marketplaces.py
ZdenekSrotyr 995e4cd366
fix(scheduler): HTTP marketplaces job + SCHEDULER_API_TOKEN shared secret (#127)
* fix(scheduler): HTTP marketplaces job + SCHEDULER_API_TOKEN shared secret

Two scheduler-reliability bugs surfaced after the v0.12.1 USER-agnes flip:

1. The marketplaces job called src.marketplace.sync_marketplaces() in-process
   from the scheduler container, racing the app's long-lived system.duckdb
   handle. DuckDB rejects cross-process writers — every cron tick 500-ed on
   "Could not set lock on file ... PID 0".

2. The data-refresh + new marketplaces jobs both 401-ed on the API because
   SCHEDULER_API_TOKEN was never propagated by the Terraform startup script.
   The scheduler had no credential to authenticate with.

Fix:
- New POST /api/marketplaces/sync-all (admin-only) drives the nightly refresh
  through the app process so it inherits the existing DB connection.
- Scheduler swaps fn->http for marketplaces; all jobs are now plain HTTP and
  the scheduler is reduced to a cron clock.
- New app/auth/scheduler_token.py adds a shared-secret auth path. The
  startup script generates a 256-bit secret on first boot, persists it
  across reboots, and writes it to /opt/agnes/.env. Both containers source
  the same .env. The app validates incoming Bearer tokens against the env
  var (constant-time, length-floored) and resolves matches to a synthetic
  scheduler@system.local user that's a member of the Admin system group.
  Audit-log entries from the scheduler are attributed to this user.
- app/main.py seeds the synthetic user at startup so the first cron tick
  has a valid actor; lazy seed in get_scheduler_user covers token rotation
  before the next app restart.

Tests: 5 new in tests/test_auth_scheduler_token.py covering empty/short
secret rejection, exact-match comparison, idempotent user seeding, and
lazy provisioning. 142 marketplace + scheduler tests + 96 auth tests
remain green.

Existing VMs with .env from before this change need a one-time
re-provisioning (re-run startup-script or rotate via openssl rand);
documented in CHANGELOG.

* fix(audit): use '_all' sentinel for bulk marketplace sync — Devin review #127

Avoids the literal string 'marketplace:None' in the audit_log resource
column when the bulk sync endpoint writes its summary row.

* fix(scheduler): unblock event loop + per-job timeouts — Devin review #127

Two findings from Devin re-review on commit 5fbad15:

1. BUG: trigger_sync_all was async def, so FastAPI ran it on the asyncio
   event loop. sync_marketplaces() does blocking I/O (subprocess git
   clones up to GIT_TIMEOUT_SEC=300 each, threading.Lock, DuckDB writes)
   and would freeze every concurrent request for the duration of a bulk
   sync. Switched to plain def so FastAPI auto-routes to the thread pool.

2. ANALYSIS: scheduler used a fixed 120s httpx timeout for every POST.
   Bulk marketplace sync iterates the registry under a single lock with
   up to 300s per repo — easily exceeds 120s on 2-3 slow repos. The
   scheduler then sees a timeout, doesn't update last_run, and re-fires
   on the next 30s tick, queueing redundant work. Per-job timeout
   override added to the JOBS tuple; marketplaces gets 900s (15 min),
   data-refresh keeps 120s, health-check 30s.

* fix(auth): require_session_token rejects scheduler shared secret — Devin review #127

require_session_token gates /auth/tokens (PAT minting). Pre-fix it only
rejected JWTs with typ=pat — but the scheduler shared secret is an opaque
string, so verify_token() returns None, payload becomes {}, and the
PAT-claim check silently passed. A caller bearing SCHEDULER_API_TOKEN
could mint persistent PATs that survive a secret rotation.

Added explicit is_scheduler_token() check before the PAT-claim check;
new regression test in tests/test_auth_scheduler_token.py.

Devin's other note (pre-existing async def trigger_sync at marketplaces.py:392
also calls blocking sync_one) — Devin flagged it as out-of-scope for this PR
and I agree; tracking separately.

* release(0.17.0): cut + clean up CHANGELOG duplicates

Cuts 0.17.0 (minor: scheduler shared-secret auth + sync-all endpoint
plus the deploy-shape fixes that landed since the last release tag).

Bumps pyproject from 0.15.0 — also corrects the missed bump from PR #120
(v0.16.0 was tagged on GitHub and shipped as :stable, but pyproject
stayed at 0.15.0, so /api/version, /cli/latest, and `da --version` had
been under-reporting the running release).

Removes the long-form duplicate entries for 0.13.0 / 0.14.0 / 0.15.0
above [0.16.0] — the canonical short summaries (with GitHub-release
links) already exist below 0.16.0, the long forms were leftover state
from before those versions were cut and have been silently shadowed
ever since.
2026-04-29 11:44:00 +02:00

455 lines
16 KiB
Python

"""Admin endpoints for marketplace git repositories.
CRUD + on-demand "Sync now" mirroring the /api/users shape. Tokens supplied
through the admin UI are persisted to data/state/.env_overlay (same pattern
as /api/admin/configure for Keboola/BigQuery) — never stored in the DB.
"""
import logging
import os
from datetime import datetime
from pathlib import Path
from typing import Any, List, Optional
import duckdb
from fastapi import APIRouter, Depends, HTTPException, Request
from pydantic import BaseModel
from app.auth.access import require_admin
from app.auth.dependencies import _get_db
from app.resource_types import ResourceType
from src.marketplace import (
MarketplaceNotFound,
delete_marketplace_dir,
is_valid_slug,
sync_marketplaces,
sync_one,
)
from src.repositories.audit import AuditRepository
from src.repositories.marketplace_plugins import MarketplacePluginsRepository
from src.repositories.marketplace_registry import MarketplaceRegistryRepository
logger = logging.getLogger(__name__)
router = APIRouter(prefix="/api/marketplaces", tags=["marketplaces"])
# ---------------------------------------------------------------------------
# Audit helper — same shape as app/api/users.py::_audit
# ---------------------------------------------------------------------------
def _audit(
conn: duckdb.DuckDBPyConnection,
actor_id: str,
action: str,
target_id: str,
params: Optional[dict] = None,
) -> None:
try:
safe_params = None
if params:
safe_params = {}
for k, v in params.items():
if isinstance(v, datetime):
safe_params[k] = v.isoformat()
else:
safe_params[k] = v
AuditRepository(conn).log(
user_id=actor_id,
action=action,
resource=f"marketplace:{target_id}",
params=safe_params,
)
except Exception:
pass
# ---------------------------------------------------------------------------
# Pydantic models
# ---------------------------------------------------------------------------
class CreateMarketplaceRequest(BaseModel):
name: str
slug: str
url: str
branch: Optional[str] = None
description: Optional[str] = None
token: Optional[str] = None
class UpdateMarketplaceRequest(BaseModel):
name: Optional[str] = None
url: Optional[str] = None
branch: Optional[str] = None
description: Optional[str] = None
# None = leave untouched; empty string = clear token; non-empty = rotate
token: Optional[str] = None
class MarketplaceResponse(BaseModel):
id: str
name: str
url: str
branch: Optional[str] = None
description: Optional[str] = None
registered_by: Optional[str] = None
registered_at: Optional[str] = None
last_synced_at: Optional[str] = None
last_commit_sha: Optional[str] = None
last_error: Optional[str] = None
has_token: bool = False
plugin_count: int = 0
def _to_response(row: dict, plugin_count: int = 0) -> MarketplaceResponse:
token_env = row.get("token_env") or ""
has_token = bool(token_env) and bool(os.environ.get(token_env, ""))
return MarketplaceResponse(
id=row["id"],
name=row["name"],
url=row["url"],
branch=row.get("branch"),
description=row.get("description"),
registered_by=row.get("registered_by"),
registered_at=str(row["registered_at"]) if row.get("registered_at") else None,
last_synced_at=str(row["last_synced_at"]) if row.get("last_synced_at") else None,
last_commit_sha=row.get("last_commit_sha"),
last_error=row.get("last_error"),
has_token=has_token,
plugin_count=plugin_count,
)
class PluginResponse(BaseModel):
name: str
description: Optional[str] = None
version: Optional[str] = None
author_name: Optional[str] = None
homepage: Optional[str] = None
category: Optional[str] = None
source_type: Optional[str] = None
source_spec: Optional[Any] = None
# ---------------------------------------------------------------------------
# Token persistence — mirrors app/api/admin.py::configure_instance
# ---------------------------------------------------------------------------
def _token_env_name(slug: str) -> str:
"""Derive a conventional env-var name from a slug.
"foundry-ai" -> "AGNES_MARKETPLACE_FOUNDRY_AI_TOKEN"
"""
normalized = slug.upper().replace("-", "_")
return f"AGNES_MARKETPLACE_{normalized}_TOKEN"
def _persist_token(env_name: str, value: str) -> None:
"""Write (or update) a single key in data/state/.env_overlay and os.environ."""
data_dir = Path(os.environ.get("DATA_DIR", "./data"))
overlay_path = data_dir / "state" / ".env_overlay"
overlay_path.parent.mkdir(parents=True, exist_ok=True)
existing: dict[str, str] = {}
if overlay_path.exists():
for line in overlay_path.read_text().splitlines():
if "=" in line and not line.startswith("#"):
k, v = line.split("=", 1)
existing[k.strip()] = v.strip()
if value:
existing[env_name] = value
os.environ[env_name] = value
else:
existing.pop(env_name, None)
os.environ.pop(env_name, None)
overlay_path.write_text(
"\n".join(f"{k}={v}" for k, v in existing.items()) + ("\n" if existing else "")
)
try:
overlay_path.chmod(0o600)
except OSError:
pass
# ---------------------------------------------------------------------------
# Endpoints
# ---------------------------------------------------------------------------
@router.get("", response_model=List[MarketplaceResponse])
async def list_marketplaces(
user: dict = Depends(require_admin),
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
):
counts = MarketplacePluginsRepository(conn).count_by_marketplace()
return [
_to_response(row, counts.get(row["id"], 0))
for row in MarketplaceRegistryRepository(conn).list_all()
]
@router.get("/{marketplace_id}/plugins", response_model=List[PluginResponse])
async def list_plugins(
marketplace_id: str,
user: dict = Depends(require_admin),
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
):
"""Return the cached plugin list for a marketplace.
Rows come from `marketplace_plugins`, which is refreshed from
`.claude-plugin/marketplace.json` on every successful sync. An
unsynced marketplace will return an empty list.
"""
if not MarketplaceRegistryRepository(conn).get(marketplace_id):
raise HTTPException(status_code=404, detail="marketplace not found")
rows = MarketplacePluginsRepository(conn).list_for_marketplace(marketplace_id)
return [
PluginResponse(
name=r["name"],
description=r.get("description"),
version=r.get("version"),
author_name=r.get("author_name"),
homepage=r.get("homepage"),
category=r.get("category"),
source_type=r.get("source_type"),
source_spec=r.get("source_spec"),
)
for r in rows
]
@router.post("", response_model=MarketplaceResponse, status_code=201)
async def create_marketplace(
payload: CreateMarketplaceRequest,
request: Request,
user: dict = Depends(require_admin),
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
):
slug = (payload.slug or "").strip().lower()
if not is_valid_slug(slug):
raise HTTPException(
status_code=400,
detail="slug must match [a-z0-9][a-z0-9_-]{0,63} (1-64 chars, start with alnum)",
)
if not (payload.url or "").strip().lower().startswith("https://"):
raise HTTPException(status_code=400, detail="url must start with https://")
if not (payload.name or "").strip():
raise HTTPException(status_code=400, detail="name is required")
repo = MarketplaceRegistryRepository(conn)
if repo.get(slug):
raise HTTPException(status_code=409, detail=f"marketplace '{slug}' already exists")
token_env: Optional[str] = None
if payload.token:
token_env = _token_env_name(slug)
_persist_token(token_env, payload.token)
repo.register(
id=slug,
name=payload.name.strip(),
url=payload.url.strip(),
branch=(payload.branch or "").strip() or None,
token_env=token_env,
description=payload.description,
registered_by=user.get("email"),
)
_audit(
conn,
user["id"],
"marketplace.create",
slug,
{"url": payload.url, "has_token": bool(payload.token)},
)
return _to_response(repo.get(slug))
@router.patch("/{marketplace_id}", response_model=MarketplaceResponse)
async def update_marketplace(
marketplace_id: str,
payload: UpdateMarketplaceRequest,
user: dict = Depends(require_admin),
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
):
repo = MarketplaceRegistryRepository(conn)
existing = repo.get(marketplace_id)
if not existing:
raise HTTPException(status_code=404, detail="marketplace not found")
# Start with the existing row; override fields the caller provided.
updated = {
"id": existing["id"],
"name": existing["name"],
"url": existing["url"],
"branch": existing.get("branch"),
"token_env": existing.get("token_env"),
"description": existing.get("description"),
"registered_by": existing.get("registered_by"),
}
changed: dict = {}
if payload.name is not None:
if not payload.name.strip():
raise HTTPException(status_code=400, detail="name cannot be empty")
updated["name"] = payload.name.strip()
changed["name"] = updated["name"]
if payload.url is not None:
url = payload.url.strip()
if not url.lower().startswith("https://"):
raise HTTPException(status_code=400, detail="url must start with https://")
updated["url"] = url
changed["url"] = url
if payload.branch is not None:
updated["branch"] = payload.branch.strip() or None
changed["branch"] = updated["branch"]
if payload.description is not None:
updated["description"] = payload.description
changed["description"] = payload.description
if payload.token is not None:
# None = untouched; "" = clear token_env binding; non-empty = rotate.
if payload.token == "":
if updated["token_env"]:
_persist_token(updated["token_env"], "")
updated["token_env"] = None
changed["token"] = "cleared"
else:
env_name = _token_env_name(marketplace_id)
_persist_token(env_name, payload.token)
updated["token_env"] = env_name
changed["token"] = "rotated"
repo.register(
id=updated["id"],
name=updated["name"],
url=updated["url"],
branch=updated["branch"],
token_env=updated["token_env"],
description=updated["description"],
registered_by=updated["registered_by"],
)
_audit(conn, user["id"], "marketplace.update", marketplace_id, changed)
counts = MarketplacePluginsRepository(conn).count_by_marketplace()
return _to_response(repo.get(marketplace_id), counts.get(marketplace_id, 0))
@router.delete("/{marketplace_id}", status_code=204)
async def delete_marketplace(
marketplace_id: str,
purge: bool = False,
user: dict = Depends(require_admin),
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
):
repo = MarketplaceRegistryRepository(conn)
existing = repo.get(marketplace_id)
if not existing:
raise HTTPException(status_code=404, detail="marketplace not found")
# Also clear any overlay token binding so a re-created marketplace of the
# same slug doesn't accidentally inherit the old PAT.
if existing.get("token_env"):
_persist_token(existing["token_env"], "")
repo.unregister(marketplace_id)
# Drop cached plugin rows and any resource grants that reference plugins
# from this marketplace. resource_grants stores resource_id as
# "<marketplace_slug>/<plugin_name>" — match the slash-prefix via
# starts_with(), not LIKE: marketplace slugs may contain '_' (validated
# by [a-z0-9][a-z0-9_-]{0,63}) and LIKE would interpret it as a
# single-char wildcard, silently dropping grants from sibling
# marketplaces whose slug differs by exactly one character.
try:
conn.execute(
"DELETE FROM marketplace_plugins WHERE marketplace_id = ?",
[marketplace_id],
)
conn.execute(
"DELETE FROM resource_grants "
"WHERE resource_type = ? AND starts_with(resource_id, ? || '/')",
[ResourceType.MARKETPLACE_PLUGIN.value, marketplace_id],
)
except Exception as e:
logger.warning("cleanup for marketplace %s failed: %s", marketplace_id, e)
purged = False
if purge:
try:
purged = delete_marketplace_dir(marketplace_id)
except Exception as e:
logger.warning("delete_marketplace_dir(%s) failed: %s", marketplace_id, e)
_audit(
conn,
user["id"],
"marketplace.delete",
marketplace_id,
{"purged_disk": purged},
)
@router.post("/{marketplace_id}/sync")
async def trigger_sync(
marketplace_id: str,
user: dict = Depends(require_admin),
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
):
try:
result = sync_one(marketplace_id)
except MarketplaceNotFound:
raise HTTPException(status_code=404, detail="marketplace not found")
except (RuntimeError, ValueError) as e:
_audit(conn, user["id"], "marketplace.sync_failed", marketplace_id, {"error": str(e)})
raise HTTPException(status_code=500, detail=str(e))
_audit(
conn,
user["id"],
"marketplace.sync",
marketplace_id,
{"commit": result["commit"], "action": result["action"]},
)
return result
@router.post("/sync-all")
def trigger_sync_all(
user: dict = Depends(require_admin),
conn: duckdb.DuckDBPyConnection = Depends(_get_db),
):
"""Sync every registered marketplace.
Wired up so the scheduler service can drive the nightly refresh over
HTTP. The previous implementation called ``src.marketplace.sync_marketplaces``
in-process from the scheduler container, which conflicted with the app's
long-lived ``system.duckdb`` handle (DuckDB allows only one writer per
file across processes). Routing through the app inherits the existing
connection without contention.
Declared ``def`` (not ``async def``) so FastAPI runs it in a thread
pool — :func:`sync_marketplaces` does blocking I/O (subprocess git
clones with ``GIT_TIMEOUT_SEC=300`` per repo, DuckDB writes, a
process-wide threading.Lock) and would freeze the event loop for the
duration of a bulk sync if it ran on the asyncio thread. Health
checks, login redirects, and every other concurrent request keep
serving while the bulk sync churns through the registry.
One audit row per call summarises the outcome — per-marketplace details
live in ``marketplace_registry`` and the per-call result payload below.
"""
result = sync_marketplaces()
# _audit appends "marketplace:" to the target id when writing the
# resource column. "_all" produces "marketplace:_all" — a stable,
# greppable sentinel for bulk-sync rows; the real per-marketplace
# commit/error breakdown is in the params payload.
_audit(
conn,
user["id"],
"marketplace.sync_all",
"_all",
{
"synced": [r.get("id") for r in result.get("synced", [])],
"errors": [{"id": e.get("id"), "error": e.get("error")} for e in result.get("errors", [])],
},
)
return result