From 3d6996a2dfee8540afccb7f463e4f509ca888543 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 00:42:15 -0400 Subject: [PATCH 01/17] feat(gateway): scaffold service + /health + auth shims + deploy/CI (#59) --- .github/workflows/ci.yml | 20 ++++++++++++ services/gateway/.dockerignore | 5 +++ services/gateway/.env.example | 5 +++ services/gateway/Dockerfile | 11 +++++++ services/gateway/alembic.ini | 39 +++++++++++++++++++++++ services/gateway/docker-compose.yml | 8 +++++ services/gateway/pyproject.toml | 44 ++++++++++++++++++++++++++ services/gateway/railway.json | 10 ++++++ services/gateway/src/__init__.py | 0 services/gateway/src/api/__init__.py | 0 services/gateway/src/api/app.py | 24 ++++++++++++++ services/gateway/src/api/hashing.py | 15 +++++++++ services/gateway/src/api/middleware.py | 3 ++ services/gateway/src/config.py | 39 +++++++++++++++++++++++ services/gateway/tests/test_health.py | 10 ++++++ uv.lock | 43 +++++++++++++++++++++++++ 16 files changed, 276 insertions(+) create mode 100644 services/gateway/.dockerignore create mode 100644 services/gateway/.env.example create mode 100644 services/gateway/Dockerfile create mode 100644 services/gateway/alembic.ini create mode 100644 services/gateway/docker-compose.yml create mode 100644 services/gateway/pyproject.toml create mode 100644 services/gateway/railway.json create mode 100644 services/gateway/src/__init__.py create mode 100644 services/gateway/src/api/__init__.py create mode 100644 services/gateway/src/api/app.py create mode 100644 services/gateway/src/api/hashing.py create mode 100644 services/gateway/src/api/middleware.py create mode 100644 services/gateway/src/config.py create mode 100644 services/gateway/tests/test_health.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 67af12c..6a0a4a8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -213,6 +213,22 @@ jobs: - name: Ruff format check run: uv run ruff format --check . + gateway-test: + runs-on: ubuntu-latest + defaults: + run: + working-directory: services/gateway + steps: + - uses: actions/checkout@v4 + - name: Install uv + uses: astral-sh/setup-uv@v5 + - name: Install dependencies + run: uv sync --extra dev + - name: Run tests + run: uv run pytest + - name: Ruff check + run: uv run ruff check . + node-test: runs-on: ubuntu-latest defaults: @@ -253,6 +269,10 @@ jobs: run: docker build -f services/llm/Dockerfile -t llm:ci . - name: Smoke-test llm image (imports resolve at boot) run: docker run --rm llm:ci python -c "import src.api.app" + - name: Build gateway image + run: docker build -f services/gateway/Dockerfile -t gateway:ci . + - name: Smoke-test gateway image (imports incl. contracts/ resolve at boot) + run: docker run --rm gateway:ci python -c "import src.api.app" - name: Build discord-bot image run: docker build -t discord-bot:ci ./discord-bot - name: Smoke-test discord-bot image (source parses) diff --git a/services/gateway/.dockerignore b/services/gateway/.dockerignore new file mode 100644 index 0000000..c579947 --- /dev/null +++ b/services/gateway/.dockerignore @@ -0,0 +1,5 @@ +.venv +__pycache__ +*.pyc +.env +.pytest_cache diff --git a/services/gateway/.env.example b/services/gateway/.env.example new file mode 100644 index 0000000..4bac9a7 --- /dev/null +++ b/services/gateway/.env.example @@ -0,0 +1,5 @@ +DATABASE_URL=postgresql+psycopg://gateway:dev_password@localhost:5435/gateway +API_KEY=dev-api-key-change-me +DIRECTORY_BASE_URL=http://localhost:8000 +DIRECTORY_API_KEY=dev-api-key-change-me +GATEWAY_ENV=local diff --git a/services/gateway/Dockerfile b/services/gateway/Dockerfile new file mode 100644 index 0000000..f1bbf5f --- /dev/null +++ b/services/gateway/Dockerfile @@ -0,0 +1,11 @@ +FROM python:3.11-slim +COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv +WORKDIR /app +COPY pyproject.toml uv.lock ./ +COPY packages/ ./packages/ +COPY services/gateway/ ./services/gateway/ +RUN uv sync --frozen --no-dev --package gateway +ENV PATH="/app/.venv/bin:$PATH" +WORKDIR /app/services/gateway +EXPOSE 8000 +CMD ["uvicorn", "src.api.app:app", "--host", "0.0.0.0", "--port", "8000"] diff --git a/services/gateway/alembic.ini b/services/gateway/alembic.ini new file mode 100644 index 0000000..01e994c --- /dev/null +++ b/services/gateway/alembic.ini @@ -0,0 +1,39 @@ +[alembic] +script_location = migrations +prepend_sys_path = . +version_path_separator = os +sqlalchemy.url = + +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/services/gateway/docker-compose.yml b/services/gateway/docker-compose.yml new file mode 100644 index 0000000..14a40a7 --- /dev/null +++ b/services/gateway/docker-compose.yml @@ -0,0 +1,8 @@ +services: + postgres: + image: postgres:16 + environment: + POSTGRES_USER: gateway + POSTGRES_PASSWORD: dev_password + POSTGRES_DB: gateway + ports: ["5435:5432"] diff --git a/services/gateway/pyproject.toml b/services/gateway/pyproject.toml new file mode 100644 index 0000000..f1a8de6 --- /dev/null +++ b/services/gateway/pyproject.toml @@ -0,0 +1,44 @@ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["src"] + +[tool.uv] +package = true + +[project] +name = "gateway" +version = "0.1.0" +description = "UTMIST external API gateway" +requires-python = ">=3.11" +dependencies = [ + "fastapi>=0.115", + "pydantic>=2.9", + "pydantic-settings>=2.5", + "sqlalchemy>=2.0.35", + "psycopg[binary]>=3.2", + "alembic>=1.13", + "uvicorn[standard]>=0.32", + "argon2-cffi>=23.1", + "httpx>=0.27", + "platform-auth", +] + +[tool.uv.sources] +platform-auth = { workspace = true } + +[project.optional-dependencies] +dev = ["pytest>=8.3", "httpx>=0.27", "ruff>=0.6"] + +[project.scripts] +gateway-keys = "src.cli:main" + +[tool.pytest.ini_options] +testpaths = ["tests"] +pythonpath = [".", "tests"] + +[tool.ruff] +line-length = 100 +target-version = "py311" diff --git a/services/gateway/railway.json b/services/gateway/railway.json new file mode 100644 index 0000000..f3255b3 --- /dev/null +++ b/services/gateway/railway.json @@ -0,0 +1,10 @@ +{ + "$schema": "https://railway.com/railway.schema.json", + "build": { "builder": "DOCKERFILE", "dockerfilePath": "services/gateway/Dockerfile" }, + "deploy": { + "startCommand": "sh -c 'uvicorn src.api.app:app --host 0.0.0.0 --port ${PORT}'", + "preDeployCommand": "alembic upgrade head", + "healthcheckPath": "/health", + "restartPolicyType": "ON_FAILURE" + } +} diff --git a/services/gateway/src/__init__.py b/services/gateway/src/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/services/gateway/src/api/__init__.py b/services/gateway/src/api/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py new file mode 100644 index 0000000..32a742a --- /dev/null +++ b/services/gateway/src/api/app.py @@ -0,0 +1,24 @@ +from fastapi import FastAPI + +from src.api.middleware import AuditLogMiddleware +from src.config import verify_production_secrets + + +def create_app() -> FastAPI: + verify_production_secrets() + app = FastAPI( + title="UTMIST gateway", + version="0.1.0", + description="External API gateway.", + docs_url="/docs", + ) + app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") + + @app.get("/health") + def health() -> dict[str, str]: + return {"status": "ok"} + + return app + + +app = create_app() diff --git a/services/gateway/src/api/hashing.py b/services/gateway/src/api/hashing.py new file mode 100644 index 0000000..9aa4663 --- /dev/null +++ b/services/gateway/src/api/hashing.py @@ -0,0 +1,15 @@ +"""Thin shim over platform_auth, binding the gateway key envelope.""" + +from platform_auth import PREFIX_LENGTH, verify_key # noqa: F401 +from platform_auth import generate_key as _generate_key +from platform_auth import parse_prefix as _parse_prefix + +KEY_ENVELOPE = "gw_" + + +def generate_key() -> tuple[str, str, str]: + return _generate_key(KEY_ENVELOPE) + + +def parse_prefix(candidate: str) -> str | None: + return _parse_prefix(candidate, KEY_ENVELOPE) diff --git a/services/gateway/src/api/middleware.py b/services/gateway/src/api/middleware.py new file mode 100644 index 0000000..ef1de7d --- /dev/null +++ b/services/gateway/src/api/middleware.py @@ -0,0 +1,3 @@ +"""Thin shim re-exporting the shared audit middleware.""" + +from platform_auth import AuditLogMiddleware # noqa: F401 diff --git a/services/gateway/src/config.py b/services/gateway/src/config.py new file mode 100644 index 0000000..5c3c2ef --- /dev/null +++ b/services/gateway/src/config.py @@ -0,0 +1,39 @@ +from functools import lru_cache +from typing import Literal + +from pydantic_settings import BaseSettings, SettingsConfigDict + +DEFAULT_DEV_API_KEY = "dev-api-key-change-me" + + +class Settings(BaseSettings): + model_config = SettingsConfigDict(env_file=".env", extra="ignore") + + database_url: str = "postgresql+psycopg://gateway:dev_password@localhost:5435/gateway" + # Env grace-period admin key for platform_auth's bootstrap path (local dev). + api_key: str = DEFAULT_DEV_API_KEY + # Outbound: the gateway's own team-tracking key (scoped identifiers:read). + directory_base_url: str = "http://localhost:8000" + directory_api_key: str = DEFAULT_DEV_API_KEY + gateway_env: Literal["local", "staging", "production"] = "local" + + +@lru_cache(maxsize=1) +def get_settings() -> Settings: + return Settings() + + +def verify_production_secrets(settings: Settings | None = None) -> None: + settings = settings or get_settings() + if settings.gateway_env == "local": + return + insecure: list[str] = [] + if settings.api_key == DEFAULT_DEV_API_KEY: + insecure.append("API_KEY") + if settings.directory_api_key == DEFAULT_DEV_API_KEY: + insecure.append("DIRECTORY_API_KEY") + if insecure: + raise RuntimeError( + f"Refusing to start in gateway_env={settings.gateway_env!r}: " + f"{', '.join(insecure)} still set to the built-in dev default." + ) diff --git a/services/gateway/tests/test_health.py b/services/gateway/tests/test_health.py new file mode 100644 index 0000000..b9c6887 --- /dev/null +++ b/services/gateway/tests/test_health.py @@ -0,0 +1,10 @@ +from fastapi.testclient import TestClient + +from src.api.app import create_app + + +def test_health_ok(): + client = TestClient(create_app()) + r = client.get("/health") + assert r.status_code == 200 + assert r.json() == {"status": "ok"} diff --git a/uv.lock b/uv.lock index d1a34b5..506b201 100644 --- a/uv.lock +++ b/uv.lock @@ -11,6 +11,7 @@ resolution-markers = [ members = [ "connectors", "documentation-system", + "gateway", "llm", "meeting", "platform-auth", @@ -656,6 +657,48 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/66/0a/cf50ecffa1e3747ed9380a3adfc829259f1f86b3fdbd9e505af789003141/fpdf2-2.8.7-py3-none-any.whl", hash = "sha256:d391fc508a3ce02fc43a577c830cda4fe6f37646f2d143d489839940932fbc19", size = 327056, upload-time = "2026-02-28T05:39:14.619Z" }, ] +[[package]] +name = "gateway" +version = "0.1.0" +source = { editable = "services/gateway" } +dependencies = [ + { name = "alembic" }, + { name = "argon2-cffi" }, + { name = "fastapi" }, + { name = "httpx" }, + { name = "platform-auth" }, + { name = "psycopg", extra = ["binary"] }, + { name = "pydantic" }, + { name = "pydantic-settings" }, + { name = "sqlalchemy" }, + { name = "uvicorn", extra = ["standard"] }, +] + +[package.optional-dependencies] +dev = [ + { name = "httpx" }, + { name = "pytest" }, + { name = "ruff" }, +] + +[package.metadata] +requires-dist = [ + { name = "alembic", specifier = ">=1.13" }, + { name = "argon2-cffi", specifier = ">=23.1" }, + { name = "fastapi", specifier = ">=0.115" }, + { name = "httpx", specifier = ">=0.27" }, + { name = "httpx", marker = "extra == 'dev'", specifier = ">=0.27" }, + { name = "platform-auth", editable = "packages/auth" }, + { name = "psycopg", extras = ["binary"], specifier = ">=3.2" }, + { name = "pydantic", specifier = ">=2.9" }, + { name = "pydantic-settings", specifier = ">=2.5" }, + { name = "pytest", marker = "extra == 'dev'", specifier = ">=8.3" }, + { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.6" }, + { name = "sqlalchemy", specifier = ">=2.0.35" }, + { name = "uvicorn", extras = ["standard"], specifier = ">=0.32" }, +] +provides-extras = ["dev"] + [[package]] name = "google-api-core" version = "2.31.0" From 41ced608a658b21a9f55a16a3beb0d14ce1bce14 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 00:47:23 -0400 Subject: [PATCH 02/17] feat(gateway): api_keys store (in-memory + postgres) + migration (#59) --- services/gateway/contracts/__init__.py | 0 services/gateway/contracts/storage.py | 15 +++ services/gateway/contracts/types.py | 27 +++++ services/gateway/migrations/env.py | 42 +++++++ services/gateway/migrations/script.py.mako | 28 +++++ .../migrations/versions/001_api_keys.py | 51 ++++++++ services/gateway/src/storage/__init__.py | 0 services/gateway/src/storage/in_memory.py | 57 +++++++++ services/gateway/src/storage/postgres.py | 111 ++++++++++++++++++ services/gateway/src/storage/schema.py | 27 +++++ services/gateway/tests/test_storage.py | 24 ++++ 11 files changed, 382 insertions(+) create mode 100644 services/gateway/contracts/__init__.py create mode 100644 services/gateway/contracts/storage.py create mode 100644 services/gateway/contracts/types.py create mode 100644 services/gateway/migrations/env.py create mode 100644 services/gateway/migrations/script.py.mako create mode 100644 services/gateway/migrations/versions/001_api_keys.py create mode 100644 services/gateway/src/storage/__init__.py create mode 100644 services/gateway/src/storage/in_memory.py create mode 100644 services/gateway/src/storage/postgres.py create mode 100644 services/gateway/src/storage/schema.py create mode 100644 services/gateway/tests/test_storage.py diff --git a/services/gateway/contracts/__init__.py b/services/gateway/contracts/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/services/gateway/contracts/storage.py b/services/gateway/contracts/storage.py new file mode 100644 index 0000000..434504d --- /dev/null +++ b/services/gateway/contracts/storage.py @@ -0,0 +1,15 @@ +from typing import Protocol +from uuid import UUID + +from contracts.types import ApiKey + + +class StorageAdapter(Protocol): + def create_api_key( + self, *, name: str, prefix: str, key_hash: str, scopes: list[str], actor: str + ) -> ApiKey: ... + def get_api_key_by_prefix(self, prefix: str) -> ApiKey | None: ... + def get_api_key_hash(self, prefix: str) -> str | None: ... + def list_api_keys(self, *, active_only: bool = False) -> list[ApiKey]: ... + def revoke_api_key(self, api_key_id: UUID, *, actor: str) -> ApiKey | None: ... + def touch_api_key_last_used(self, api_key_id: UUID) -> None: ... diff --git a/services/gateway/contracts/types.py b/services/gateway/contracts/types.py new file mode 100644 index 0000000..bae3a75 --- /dev/null +++ b/services/gateway/contracts/types.py @@ -0,0 +1,27 @@ +from datetime import datetime +from uuid import UUID + +from pydantic import BaseModel, ConfigDict + + +class ApiKey(BaseModel): + model_config = ConfigDict(extra="forbid") + id: UUID + name: str + prefix: str + scopes: list[str] + active: bool = True + revoked_at: datetime | None = None + last_used_at: datetime | None = None + + +class ApiKeyCreate(BaseModel): + model_config = ConfigDict(extra="forbid") + name: str + scopes: list[str] = [] + + +class IssuedApiKey(BaseModel): + model_config = ConfigDict(extra="forbid") + plaintext: str + api_key: ApiKey diff --git a/services/gateway/migrations/env.py b/services/gateway/migrations/env.py new file mode 100644 index 0000000..04e25ea --- /dev/null +++ b/services/gateway/migrations/env.py @@ -0,0 +1,42 @@ +from logging.config import fileConfig + +from alembic import context +from sqlalchemy import engine_from_config, pool + +from src.config import get_settings +from src.storage.schema import metadata as target_metadata + +config = context.config +if config.config_file_name is not None: + fileConfig(config.config_file_name) + +config.set_main_option("sqlalchemy.url", get_settings().database_url) + + +def run_migrations_offline() -> None: + context.configure( + url=config.get_main_option("sqlalchemy.url"), + target_metadata=target_metadata, + literal_binds=True, + dialect_opts={"paramstyle": "named"}, + ) + with context.begin_transaction(): + context.run_migrations() + + +def run_migrations_online() -> None: + connectable = engine_from_config( + config.get_section(config.config_ini_section, {}), + prefix="sqlalchemy.", + poolclass=pool.NullPool, + ) + with connectable.connect() as connection: + context.configure(connection=connection, target_metadata=target_metadata) + with context.begin_transaction(): + context.run_migrations() + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/services/gateway/migrations/script.py.mako b/services/gateway/migrations/script.py.mako new file mode 100644 index 0000000..1101630 --- /dev/null +++ b/services/gateway/migrations/script.py.mako @@ -0,0 +1,28 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision: str = ${repr(up_revision)} +down_revision: Union[str, Sequence[str], None] = ${repr(down_revision)} +branch_labels: Union[str, Sequence[str], None] = ${repr(branch_labels)} +depends_on: Union[str, Sequence[str], None] = ${repr(depends_on)} + + +def upgrade() -> None: + """Upgrade schema.""" + ${upgrades if upgrades else "pass"} + + +def downgrade() -> None: + """Downgrade schema.""" + ${downgrades if downgrades else "pass"} diff --git a/services/gateway/migrations/versions/001_api_keys.py b/services/gateway/migrations/versions/001_api_keys.py new file mode 100644 index 0000000..14ce8f3 --- /dev/null +++ b/services/gateway/migrations/versions/001_api_keys.py @@ -0,0 +1,51 @@ +"""api_keys table + +Revision ID: 001 +Revises: +Create Date: 2026-07-05 +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects.postgresql import ARRAY, UUID + +revision = "001" +down_revision = None +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "api_keys", + sa.Column( + "id", UUID(as_uuid=True), primary_key=True, server_default=sa.text("gen_random_uuid()") + ), + sa.Column("name", sa.Text, nullable=False, unique=True), + sa.Column("prefix", sa.Text, nullable=False, unique=True), + sa.Column("key_hash", sa.Text, nullable=False), + sa.Column( + "scopes", ARRAY(sa.Text), nullable=False, server_default=sa.text("ARRAY[]::text[]") + ), + sa.Column("active", sa.Boolean, nullable=False, server_default=sa.text("true")), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + nullable=False, + server_default=sa.text("now()"), + ), + sa.Column( + "updated_at", + sa.DateTime(timezone=True), + nullable=False, + server_default=sa.text("now()"), + ), + sa.Column("created_by", sa.Text, nullable=False), + sa.Column("updated_by", sa.Text, nullable=False), + sa.Column("revoked_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("last_used_at", sa.DateTime(timezone=True), nullable=True), + ) + + +def downgrade() -> None: + op.drop_table("api_keys") diff --git a/services/gateway/src/storage/__init__.py b/services/gateway/src/storage/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/services/gateway/src/storage/in_memory.py b/services/gateway/src/storage/in_memory.py new file mode 100644 index 0000000..bfe852b --- /dev/null +++ b/services/gateway/src/storage/in_memory.py @@ -0,0 +1,57 @@ +from datetime import datetime, timezone +from uuid import UUID, uuid4 + +from contracts.types import ApiKey + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +class InMemoryStorageAdapter: + """In-process adapter for tests. Not persistent, not thread-safe.""" + + def __init__(self) -> None: + self._api_keys: dict[UUID, ApiKey] = {} + self._hashes: dict[UUID, str] = {} + + def create_api_key( + self, *, name: str, prefix: str, key_hash: str, scopes: list[str], actor: str + ) -> ApiKey: + if any(k.name == name for k in self._api_keys.values()): + raise ValueError(f"api key name already exists: {name}") + if any(k.prefix == prefix for k in self._api_keys.values()): + raise ValueError(f"api key prefix already exists: {prefix}") + key = ApiKey(id=uuid4(), name=name, prefix=prefix, scopes=list(scopes), active=True) + self._api_keys[key.id] = key + self._hashes[key.id] = key_hash + return key + + def _by_prefix(self, prefix: str) -> ApiKey | None: + return next((k for k in self._api_keys.values() if k.prefix == prefix), None) + + def get_api_key_by_prefix(self, prefix: str) -> ApiKey | None: + return self._by_prefix(prefix) + + def get_api_key_hash(self, prefix: str) -> str | None: + row = self._by_prefix(prefix) + return self._hashes.get(row.id) if row else None + + def list_api_keys(self, *, active_only: bool = False) -> list[ApiKey]: + keys = list(self._api_keys.values()) + if active_only: + keys = [k for k in keys if k.active and k.revoked_at is None] + return keys + + def revoke_api_key(self, api_key_id: UUID, *, actor: str) -> ApiKey | None: + key = self._api_keys.get(api_key_id) + if key is None: + return None + updated = key.model_copy(update={"active": False, "revoked_at": _now()}) + self._api_keys[api_key_id] = updated + return updated + + def touch_api_key_last_used(self, api_key_id: UUID) -> None: + key = self._api_keys.get(api_key_id) + if key is not None: + self._api_keys[api_key_id] = key.model_copy(update={"last_used_at": _now()}) diff --git a/services/gateway/src/storage/postgres.py b/services/gateway/src/storage/postgres.py new file mode 100644 index 0000000..5fb053f --- /dev/null +++ b/services/gateway/src/storage/postgres.py @@ -0,0 +1,111 @@ +from datetime import datetime, timezone +from uuid import UUID + +from sqlalchemy import insert, select, update +from sqlalchemy.engine import Engine +from sqlalchemy.exc import IntegrityError + +from contracts.types import ApiKey +from src.storage.schema import api_keys + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +def _api_key_row_to_model(row) -> ApiKey: + return ApiKey( + id=row.id, + name=row.name, + prefix=row.prefix, + scopes=list(row.scopes), + active=row.active, + revoked_at=row.revoked_at, + last_used_at=row.last_used_at, + ) + + +class PostgresStorageAdapter: + """Postgres-backed StorageAdapter using SQLAlchemy Core. + + Every method returns Pydantic domain models (never raw rows, and never the + stored `key_hash`). Callers should not need to know this backend exists — + they see StorageAdapter (Protocol). + """ + + def __init__(self, engine: Engine) -> None: + self._engine = engine + + def create_api_key( + self, + *, + name: str, + prefix: str, + key_hash: str, + scopes: list[str], + actor: str, + ) -> ApiKey: + try: + with self._engine.begin() as conn: + row = conn.execute( + insert(api_keys) + .values( + name=name, + prefix=prefix, + key_hash=key_hash, + scopes=scopes, + created_by=actor, + updated_by=actor, + ) + .returning(api_keys) + ).one() + except IntegrityError as e: + raise ValueError(f"name or prefix already exists: {name!r} / {prefix!r}") from e + return _api_key_row_to_model(row) + + def get_api_key_by_prefix(self, prefix: str) -> ApiKey | None: + with self._engine.connect() as conn: + row = conn.execute(select(api_keys).where(api_keys.c.prefix == prefix)).one_or_none() + return _api_key_row_to_model(row) if row else None + + def get_api_key_hash(self, prefix: str) -> str | None: + with self._engine.connect() as conn: + row = conn.execute( + select(api_keys.c.key_hash).where( + api_keys.c.prefix == prefix, + api_keys.c.active.is_(True), + api_keys.c.revoked_at.is_(None), + ) + ).one_or_none() + return row.key_hash if row else None + + def list_api_keys(self, *, active_only: bool = False) -> list[ApiKey]: + stmt = select(api_keys) + if active_only: + stmt = stmt.where( + api_keys.c.active.is_(True), + api_keys.c.revoked_at.is_(None), + ) + with self._engine.connect() as conn: + rows = conn.execute(stmt).all() + return [_api_key_row_to_model(r) for r in rows] + + def revoke_api_key(self, api_key_id: UUID, *, actor: str) -> ApiKey | None: + now = _now() + with self._engine.begin() as conn: + row = conn.execute( + update(api_keys) + .where(api_keys.c.id == api_key_id) + .values(active=False, revoked_at=now, updated_at=now, updated_by=actor) + .returning(api_keys) + ).one_or_none() + return _api_key_row_to_model(row) if row else None + + def touch_api_key_last_used(self, api_key_id: UUID) -> None: + try: + with self._engine.begin() as conn: + conn.execute( + update(api_keys).where(api_keys.c.id == api_key_id).values(last_used_at=_now()) + ) + except Exception: + pass # best-effort; DB blips must not fail the auth path diff --git a/services/gateway/src/storage/schema.py b/services/gateway/src/storage/schema.py new file mode 100644 index 0000000..0b05251 --- /dev/null +++ b/services/gateway/src/storage/schema.py @@ -0,0 +1,27 @@ +"""SQLAlchemy Core Table definitions for the gateway's api_keys store. + +This module defines TABLES, not ORM classes. All queries in +PostgresStorageAdapter use core-style expressions against these tables. +""" + +from sqlalchemy import Boolean, Column, DateTime, MetaData, Table, Text, text +from sqlalchemy.dialects.postgresql import ARRAY, UUID + +metadata = MetaData() + +api_keys = Table( + "api_keys", + metadata, + Column("id", UUID(as_uuid=True), primary_key=True, server_default=text("gen_random_uuid()")), + Column("name", Text, nullable=False, unique=True), + Column("prefix", Text, nullable=False, unique=True), + Column("key_hash", Text, nullable=False), + Column("scopes", ARRAY(Text), nullable=False, server_default=text("ARRAY[]::text[]")), + Column("active", Boolean, nullable=False, server_default=text("true")), + Column("created_at", DateTime(timezone=True), nullable=False, server_default=text("now()")), + Column("updated_at", DateTime(timezone=True), nullable=False, server_default=text("now()")), + Column("created_by", Text, nullable=False), + Column("updated_by", Text, nullable=False), + Column("revoked_at", DateTime(timezone=True), nullable=True), + Column("last_used_at", DateTime(timezone=True), nullable=True), +) diff --git a/services/gateway/tests/test_storage.py b/services/gateway/tests/test_storage.py new file mode 100644 index 0000000..48df1e6 --- /dev/null +++ b/services/gateway/tests/test_storage.py @@ -0,0 +1,24 @@ +from src.storage.in_memory import InMemoryStorageAdapter + + +def test_create_get_verify_revoke_roundtrip(): + a = InMemoryStorageAdapter() + key = a.create_api_key(name="gh-action", prefix="abcd1234", key_hash="HASH", + scopes=["resolve:discord"], actor="cli") + assert key.name == "gh-action" and key.active is True + assert a.get_api_key_hash("abcd1234") == "HASH" + row = a.get_api_key_by_prefix("abcd1234") + assert row.scopes == ["resolve:discord"] + assert [k.name for k in a.list_api_keys()] == ["gh-action"] + a.touch_api_key_last_used(key.id) + revoked = a.revoke_api_key(key.id, actor="cli") + assert revoked.active is False and revoked.revoked_at is not None + # revoked key: hash still returns, but active=false (auth layer rejects) + assert a.get_api_key_by_prefix("abcd1234").active is False + + +def test_unknown_prefix_returns_none(): + a = InMemoryStorageAdapter() + assert a.get_api_key_hash("nope") is None + assert a.get_api_key_by_prefix("nope") is None + assert a.revoke_api_key(__import__("uuid").uuid4(), actor="cli") is None From 5016b95e031aa5a53b9fd98ecb1a232eadc05a11 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 00:52:40 -0400 Subject: [PATCH 03/17] feat(gateway): platform_auth wiring for external keys (#59) --- services/gateway/src/api/auth.py | 17 +++++++++++++++ services/gateway/src/api/deps.py | 25 ++++++++++++++++++++++ services/gateway/tests/test_auth.py | 32 +++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+) create mode 100644 services/gateway/src/api/auth.py create mode 100644 services/gateway/src/api/deps.py create mode 100644 services/gateway/tests/test_auth.py diff --git a/services/gateway/src/api/auth.py b/services/gateway/src/api/auth.py new file mode 100644 index 0000000..bb33bfc --- /dev/null +++ b/services/gateway/src/api/auth.py @@ -0,0 +1,17 @@ +"""Thin shim: builds the gateway's auth deps from platform_auth (external keys).""" + +from platform_auth import ADMIN_SCOPE, AuthedKey, build_auth # noqa: F401 + +from src.api.deps import get_storage +from src.config import get_settings + +_deps = build_auth( + get_storage, + envelope="gw_", + get_env_key=lambda: get_settings().api_key, + audit_logger_name="gateway.audit", +) + +require_api_key = _deps.require_api_key +require_scope = _deps.require_scope +get_actor = _deps.get_actor diff --git a/services/gateway/src/api/deps.py b/services/gateway/src/api/deps.py new file mode 100644 index 0000000..f27b007 --- /dev/null +++ b/services/gateway/src/api/deps.py @@ -0,0 +1,25 @@ +from functools import lru_cache + +from sqlalchemy import create_engine +from sqlalchemy.engine import Engine + +from contracts.storage import StorageAdapter +from src.config import get_settings + + +@lru_cache(maxsize=1) +def _default_engine() -> Engine: + return create_engine(get_settings().database_url, future=True, pool_pre_ping=True) + + +def get_storage() -> StorageAdapter: + from src.storage.postgres import PostgresStorageAdapter + + return PostgresStorageAdapter(_default_engine()) + + +def get_directory(): + from src.directory.http_client import HttpDirectoryClient + + s = get_settings() + return HttpDirectoryClient(s.directory_base_url, s.directory_api_key) diff --git a/services/gateway/tests/test_auth.py b/services/gateway/tests/test_auth.py new file mode 100644 index 0000000..e33e86b --- /dev/null +++ b/services/gateway/tests/test_auth.py @@ -0,0 +1,32 @@ +from fastapi import Depends, FastAPI +from fastapi.testclient import TestClient + +from src.api.auth import require_scope +from src.api.deps import get_storage +from src.api.hashing import generate_key +from src.storage.in_memory import InMemoryStorageAdapter + + +def _client_with_key(scopes): + store = InMemoryStorageAdapter() + plaintext, prefix, key_hash = generate_key() + store.create_api_key(name="c", prefix=prefix, key_hash=key_hash, scopes=scopes, actor="t") + app = FastAPI() + + @app.get("/probe") + def probe(_=Depends(require_scope("resolve:discord"))): + return {"ok": True} + + app.dependency_overrides[get_storage] = lambda: store + return TestClient(app), plaintext + + +def test_valid_scope_200(): + client, key = _client_with_key(["resolve:discord"]) + assert client.get("/probe", headers={"X-API-Key": key}).status_code == 200 + + +def test_missing_scope_403_and_no_key_401(): + client, key = _client_with_key(["other:scope"]) + assert client.get("/probe", headers={"X-API-Key": key}).status_code == 403 + assert client.get("/probe").status_code == 401 From 6189bb7281b67594dddbf264a9c87b764f8a2b7b Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 00:55:23 -0400 Subject: [PATCH 04/17] feat(gateway): gateway-keys CLI (issue/list/revoke) (#59) --- services/gateway/src/cli.py | 88 ++++++++++++++++++++++++++++++ services/gateway/tests/test_cli.py | 15 +++++ 2 files changed, 103 insertions(+) create mode 100644 services/gateway/src/cli.py create mode 100644 services/gateway/tests/test_cli.py diff --git a/services/gateway/src/cli.py b/services/gateway/src/cli.py new file mode 100644 index 0000000..09b7631 --- /dev/null +++ b/services/gateway/src/cli.py @@ -0,0 +1,88 @@ +"""gateway-keys — direct-to-DB management of the gateway's EXTERNAL API keys. + + gateway-keys issue --name --scopes resolve:discord + gateway-keys list [--active-only] + gateway-keys revoke + +`issue` PRINTS THE PLAINTEXT KEY ONCE to stdout; the argon2 hash is all that's stored. +""" + +import argparse +import sys +from uuid import UUID + +from sqlalchemy import create_engine + +from src.api.hashing import generate_key +from src.config import get_settings +from src.storage.postgres import PostgresStorageAdapter + + +def _adapter() -> PostgresStorageAdapter: + return PostgresStorageAdapter(create_engine(get_settings().database_url, future=True)) + + +def cmd_issue(args: argparse.Namespace) -> int: + plaintext, prefix, key_hash = generate_key() + key = _adapter().create_api_key( + name=args.name, prefix=prefix, key_hash=key_hash, scopes=list(args.scopes or []), + actor=args.actor, + ) + print("=" * 70, file=sys.stderr) + print("EXTERNAL API KEY ISSUED (shown once)", file=sys.stderr) + print(f" Name: {key.name}", file=sys.stderr) + print(f" Scopes: {', '.join(key.scopes) if key.scopes else '(none)'}", file=sys.stderr) + print(f" Key id: {key.id}", file=sys.stderr) + print(plaintext) + return 0 + + +def cmd_list(args: argparse.Namespace) -> int: + keys = _adapter().list_api_keys(active_only=args.active_only) + if not keys: + print("(no keys)", file=sys.stderr) + return 0 + for k in keys: + active = "yes" if (k.active and k.revoked_at is None) else "no" + print(f"{k.name:<25} {k.prefix:<10} {active:<7} {', '.join(k.scopes) or '(none)'}") + return 0 + + +def cmd_revoke(args: argparse.Namespace) -> int: + try: + key_id = UUID(args.api_key_id) + except ValueError: + print(f"error: not a valid UUID: {args.api_key_id}", file=sys.stderr) + return 2 + revoked = _adapter().revoke_api_key(key_id, actor=args.actor) + if revoked is None: + print(f"error: no such api key: {key_id}", file=sys.stderr) + return 1 + print(f"revoked {revoked.name} ({revoked.prefix})", file=sys.stderr) + return 0 + + +def build_parser() -> argparse.ArgumentParser: + p = argparse.ArgumentParser(prog="gateway-keys", description="Manage gateway external API keys.") + p.add_argument("--actor", default="cli") + subs = p.add_subparsers(dest="cmd", required=True) + pi = subs.add_parser("issue") + pi.add_argument("--name", required=True) + pi.add_argument("--scopes", nargs="*", default=[]) + pi.set_defaults(func=cmd_issue) + pl = subs.add_parser("list") + pl.add_argument("--active-only", action="store_true") + pl.set_defaults(func=cmd_list) + pr = subs.add_parser("revoke") + pr.add_argument("api_key_id") + pr.set_defaults(func=cmd_revoke) + return p + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + return args.func(args) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/services/gateway/tests/test_cli.py b/services/gateway/tests/test_cli.py new file mode 100644 index 0000000..00ad30b --- /dev/null +++ b/services/gateway/tests/test_cli.py @@ -0,0 +1,15 @@ +from src import cli +from src.api.hashing import parse_prefix, verify_key +from src.storage.in_memory import InMemoryStorageAdapter + + +def test_issue_mints_verifiable_key(monkeypatch, capsys): + store = InMemoryStorageAdapter() + monkeypatch.setattr(cli, "_adapter", lambda: store) + rc = cli.main(["issue", "--name", "gh-action", "--scopes", "resolve:discord"]) + assert rc == 0 + plaintext = capsys.readouterr().out.strip() + prefix = parse_prefix(plaintext) + assert store.get_api_key_hash(prefix) is not None + assert verify_key(plaintext, store.get_api_key_hash(prefix)) is True + assert store.get_api_key_by_prefix(prefix).scopes == ["resolve:discord"] From cb3709360f3773ffb4b408b8dffebf43e09636cf Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 00:58:42 -0400 Subject: [PATCH 05/17] feat(gateway): directory http client (github lookup + identifiers) (#59) --- services/gateway/contracts/directory.py | 10 +++++ services/gateway/src/directory/__init__.py | 0 services/gateway/src/directory/http_client.py | 38 +++++++++++++++++++ services/gateway/tests/test_directory.py | 29 ++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 services/gateway/contracts/directory.py create mode 100644 services/gateway/src/directory/__init__.py create mode 100644 services/gateway/src/directory/http_client.py create mode 100644 services/gateway/tests/test_directory.py diff --git a/services/gateway/contracts/directory.py b/services/gateway/contracts/directory.py new file mode 100644 index 0000000..d391f6c --- /dev/null +++ b/services/gateway/contracts/directory.py @@ -0,0 +1,10 @@ +from typing import Protocol + + +class DirectoryUnavailable(Exception): + """Raised when team-tracking is unreachable or returns 5xx.""" + + +class DirectoryClient(Protocol): + def get_person_by_github(self, github_login: str) -> dict | None: ... + def list_identifiers(self, person_id: str) -> list[dict]: ... diff --git a/services/gateway/src/directory/__init__.py b/services/gateway/src/directory/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/services/gateway/src/directory/http_client.py b/services/gateway/src/directory/http_client.py new file mode 100644 index 0000000..519c5ec --- /dev/null +++ b/services/gateway/src/directory/http_client.py @@ -0,0 +1,38 @@ +import httpx + +from contracts.directory import DirectoryUnavailable + +_TIMEOUT = httpx.Timeout(5.0) + + +class HttpDirectoryClient: + """Looks up people and identifiers over team-tracking's HTTP API. A 404 + means 'no such record' (returns None); connection failure or 5xx means + 'directory unavailable' (raises DirectoryUnavailable).""" + + def __init__(self, base_url: str, api_key: str, client: httpx.Client | None = None) -> None: + self._base_url = base_url.rstrip("/") + self._api_key = api_key + self._client = client + + def _get(self, path: str): + client = self._client or httpx.Client(timeout=_TIMEOUT) + try: + resp = client.get(f"{self._base_url}{path}", headers={"X-API-Key": self._api_key}) + except httpx.HTTPError as e: + raise DirectoryUnavailable(f"directory unreachable: {e}") from e + finally: + if self._client is None: + client.close() + if resp.status_code == 404: + return None + if not (200 <= resp.status_code < 300): + raise DirectoryUnavailable(f"directory returned {resp.status_code}") + return resp.json() + + def get_person_by_github(self, github_login: str) -> dict | None: + return self._get(f"/people/by-identifier/github/{github_login}") + + def list_identifiers(self, person_id: str) -> list[dict]: + result = self._get(f"/people/{person_id}/identifiers") + return result or [] diff --git a/services/gateway/tests/test_directory.py b/services/gateway/tests/test_directory.py new file mode 100644 index 0000000..3a521d9 --- /dev/null +++ b/services/gateway/tests/test_directory.py @@ -0,0 +1,29 @@ +import httpx + +from contracts.directory import DirectoryUnavailable +from src.directory.http_client import HttpDirectoryClient + + +def _client(handler): + return HttpDirectoryClient("http://d", "k", client=httpx.Client(transport=httpx.MockTransport(handler))) + + +def test_get_person_by_github_found_and_404(): + def h(req): + if req.url.path == "/people/by-identifier/github/octocat": + return httpx.Response(200, json={"id": "p1"}) + return httpx.Response(404) + c = _client(h) + assert c.get_person_by_github("octocat") == {"id": "p1"} + assert c.get_person_by_github("ghost") is None + + +def test_list_identifiers_and_5xx_raises(): + c = _client(lambda req: httpx.Response(200, json=[{"provider": "discord", "external_id": "42"}])) + assert c.list_identifiers("p1") == [{"provider": "discord", "external_id": "42"}] + c2 = _client(lambda req: httpx.Response(503)) + try: + c2.get_person_by_github("x") + assert False + except DirectoryUnavailable: + pass From 429b8642e68e0ca801416be9f5643eb11ec60ff9 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 01:01:47 -0400 Subject: [PATCH 06/17] feat(gateway): /v1/resolve/discord endpoint (#59) --- services/gateway/src/api/app.py | 11 +++- services/gateway/src/api/routers/__init__.py | 0 .../__pycache__/__init__.cpython-314.pyc | Bin 0 -> 220 bytes .../__pycache__/resolve.cpython-314.pyc | Bin 0 -> 2080 bytes services/gateway/src/api/routers/resolve.py | 26 ++++++++ services/gateway/tests/test_resolve.py | 57 ++++++++++++++++++ 6 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 services/gateway/src/api/routers/__init__.py create mode 100644 services/gateway/src/api/routers/__pycache__/__init__.cpython-314.pyc create mode 100644 services/gateway/src/api/routers/__pycache__/resolve.cpython-314.pyc create mode 100644 services/gateway/src/api/routers/resolve.py create mode 100644 services/gateway/tests/test_resolve.py diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 32a742a..1d7a3bc 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -1,6 +1,9 @@ -from fastapi import FastAPI +from fastapi import FastAPI, Request +from fastapi.responses import JSONResponse +from contracts.directory import DirectoryUnavailable from src.api.middleware import AuditLogMiddleware +from src.api.routers import resolve from src.config import verify_production_secrets @@ -18,6 +21,12 @@ def create_app() -> FastAPI: def health() -> dict[str, str]: return {"status": "ok"} + app.include_router(resolve.router) + + @app.exception_handler(DirectoryUnavailable) + async def _directory_unavailable(request: Request, exc: DirectoryUnavailable): + return JSONResponse(status_code=503, content={"detail": "directory temporarily unavailable"}) + return app diff --git a/services/gateway/src/api/routers/__init__.py b/services/gateway/src/api/routers/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/services/gateway/src/api/routers/__pycache__/__init__.cpython-314.pyc b/services/gateway/src/api/routers/__pycache__/__init__.cpython-314.pyc new file mode 100644 index 0000000000000000000000000000000000000000..25520ce3369d8b18d33d9d089be4c8422b44457a GIT binary patch literal 220 zcmX|*!4APd5QcXN5eZih;3QJ>0^;Q2KmaYcp}MscF@EyC5%(GjwOH;0s}B-km3S5*MNy6Bjlzd$jlOaqymPc1}| fPPL1oQC5_#@yTrS1SizsBtEP4@UBZKsm}BP%;r5U literal 0 HcmV?d00001 diff --git a/services/gateway/src/api/routers/__pycache__/resolve.cpython-314.pyc b/services/gateway/src/api/routers/__pycache__/resolve.cpython-314.pyc new file mode 100644 index 0000000000000000000000000000000000000000..57a891e986ffdd320fe210ee6a345e20928dfdf5 GIT binary patch literal 2080 zcmb7FO>7%Q6rSC+*Z=u#>oom|n>I;JsJCfTHI%d>YN+fM(y*j1m6EJByAx+ycXu;8 zPLm*zsX%b(fn&I!UXX&s1#tlhi7Q8^u9UC}su1+hn@cJX7sMNT*9j6AM%tM-Kkv=F z?|tv>U`ry3pmjgZ-Hrqh`jt)m7V?Ch??EV|StN5gl;tRwdWw&^;NGx5no{)M}v93JKxvl;#v znu+~u76CgEf5jiq@^UnZsY-j1XjV1Ve-y9I+Tl;)J8 zRWQu=R$IOO3*NrQ@bEA}EM@|HJGY@Lqdc1BM?OSIhqc4#EpCC6xoOyGG?}0F&_+lW zrXkASdSF`g<=xY}(pmoHZhs#kDF8SIpD{~reD71;NYiJRQ(Tc?S|TQ3EjW}A#vu}B zN&(%_D8MAnTjZ`v9p+1~yDZ&sFmWVI7gTf6C`sekxkD{mx-s=;?(L~llf;m~YRS-Gr@25hNwp1$)MM`Wpl9qAH?AnE zX<86$tSF=lUK4}Q8G){h{zR=@|8v%DckHhTF#gthv=)T%g^0rKwGflSNQ^!>{=oU< zjLN|tbZr2k2s#Q$avYr#3=TLP zwu!Z5=&W3LdFaM`)+vTQ0elu9?ZE*)bK2z$hk@3gB1stB$Y=qZc-bbG++Hu6p+3~J zJu_OgG_~kl$~4dH0~IF?-7|2ja^U9Io$F&i1iuS@d%HT4`|;MsNd9qh;M2vmBb&*A zN6CS2I{$K5UfoYh+~@5Qw>oHADbIQeUTjih-oOL^CMn1*n%WD!8}&|t4{6ce$10Z% zX=s_s+=|jTNf>ttqnz8pDx@-edRUpbKBZi_eq&D;sh@NG7lb!wJLiKG1~!Bqnf~R+!J)MiY`7y(i1fF zGwS~((y`X}aOL5h$B{E-u_huu{BidO-Ie&MDwlaKqTclSk?OJYKXrD0cCnJaP(Ac& UwR7|jR@8O`#7n>PAk}aF2VREl*Z=?k literal 0 HcmV?d00001 diff --git a/services/gateway/src/api/routers/resolve.py b/services/gateway/src/api/routers/resolve.py new file mode 100644 index 0000000..86afdd7 --- /dev/null +++ b/services/gateway/src/api/routers/resolve.py @@ -0,0 +1,26 @@ +from fastapi import APIRouter, Depends, HTTPException, status + +from contracts.directory import DirectoryClient +from src.api.auth import AuthedKey, require_scope +from src.api.deps import get_directory + +router = APIRouter(prefix="/v1", tags=["resolve"]) + + +@router.get("/resolve/discord/{github_login}") +def resolve_discord( + github_login: str, + directory: DirectoryClient = Depends(get_directory), + _: AuthedKey = Depends(require_scope("resolve:discord")), +) -> dict[str, str]: + person = directory.get_person_by_github(github_login) + if person is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="github login not found") + ids = directory.list_identifiers(person["id"]) + discord_id = next((i["external_id"] for i in ids if i.get("provider") == "discord"), None) + if discord_id is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="no discord identifier for that github login", + ) + return {"discord_id": discord_id} diff --git a/services/gateway/tests/test_resolve.py b/services/gateway/tests/test_resolve.py new file mode 100644 index 0000000..1ccf73e --- /dev/null +++ b/services/gateway/tests/test_resolve.py @@ -0,0 +1,57 @@ +from fastapi.testclient import TestClient + +from contracts.directory import DirectoryUnavailable +from src.api.app import create_app +from src.api.deps import get_directory, get_storage +from src.api.hashing import generate_key +from src.storage.in_memory import InMemoryStorageAdapter + + +class FakeDir: + def __init__(self, person=None, idents=None, down=False): + self._p, self._i, self._down = person, idents or [], down + def get_person_by_github(self, login): + if self._down: + raise DirectoryUnavailable("x") + return self._p + def list_identifiers(self, pid): + return self._i + + +def _client(fake): + store = InMemoryStorageAdapter() + plaintext, prefix, key_hash = generate_key() + store.create_api_key(name="c", prefix=prefix, key_hash=key_hash, + scopes=["resolve:discord"], actor="t") + app = create_app() + app.dependency_overrides[get_storage] = lambda: store + app.dependency_overrides[get_directory] = lambda: fake + return TestClient(app), {"X-API-Key": plaintext} + + +def test_resolves_discord_id(): + c, h = _client(FakeDir(person={"id": "p1"}, + idents=[{"provider": "github", "external_id": "octocat"}, + {"provider": "discord", "external_id": "42"}])) + r = c.get("/v1/resolve/discord/octocat", headers=h) + assert r.status_code == 200 and r.json() == {"discord_id": "42"} + + +def test_login_not_found_404(): + c, h = _client(FakeDir(person=None)) + assert c.get("/v1/resolve/discord/ghost", headers=h).status_code == 404 + + +def test_no_discord_identifier_404(): + c, h = _client(FakeDir(person={"id": "p1"}, idents=[{"provider": "github", "external_id": "x"}])) + assert c.get("/v1/resolve/discord/octocat", headers=h).status_code == 404 + + +def test_directory_down_503(): + c, h = _client(FakeDir(down=True)) + assert c.get("/v1/resolve/discord/octocat", headers=h).status_code == 503 + + +def test_requires_scope_and_key(): + c, h = _client(FakeDir(person={"id": "p1"}, idents=[{"provider": "discord", "external_id": "42"}])) + assert c.get("/v1/resolve/discord/octocat").status_code == 401 From 52e86bc07752a9734d15f88c0317cfdec24ef74c Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 01:01:50 -0400 Subject: [PATCH 07/17] chore(gateway): remove accidentally committed __pycache__ --- .../routers/__pycache__/__init__.cpython-314.pyc | Bin 220 -> 0 bytes .../routers/__pycache__/resolve.cpython-314.pyc | Bin 2080 -> 0 bytes 2 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 services/gateway/src/api/routers/__pycache__/__init__.cpython-314.pyc delete mode 100644 services/gateway/src/api/routers/__pycache__/resolve.cpython-314.pyc diff --git a/services/gateway/src/api/routers/__pycache__/__init__.cpython-314.pyc b/services/gateway/src/api/routers/__pycache__/__init__.cpython-314.pyc deleted file mode 100644 index 25520ce3369d8b18d33d9d089be4c8422b44457a..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 220 zcmX|*!4APd5QcXN5eZih;3QJ>0^;Q2KmaYcp}MscF@EyC5%(GjwOH;0s}B-km3S5*MNy6Bjlzd$jlOaqymPc1}| fPPL1oQC5_#@yTrS1SizsBtEP4@UBZKsm}BP%;r5U diff --git a/services/gateway/src/api/routers/__pycache__/resolve.cpython-314.pyc b/services/gateway/src/api/routers/__pycache__/resolve.cpython-314.pyc deleted file mode 100644 index 57a891e986ffdd320fe210ee6a345e20928dfdf5..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2080 zcmb7FO>7%Q6rSC+*Z=u#>oom|n>I;JsJCfTHI%d>YN+fM(y*j1m6EJByAx+ycXu;8 zPLm*zsX%b(fn&I!UXX&s1#tlhi7Q8^u9UC}su1+hn@cJX7sMNT*9j6AM%tM-Kkv=F z?|tv>U`ry3pmjgZ-Hrqh`jt)m7V?Ch??EV|StN5gl;tRwdWw&^;NGx5no{)M}v93JKxvl;#v znu+~u76CgEf5jiq@^UnZsY-j1XjV1Ve-y9I+Tl;)J8 zRWQu=R$IOO3*NrQ@bEA}EM@|HJGY@Lqdc1BM?OSIhqc4#EpCC6xoOyGG?}0F&_+lW zrXkASdSF`g<=xY}(pmoHZhs#kDF8SIpD{~reD71;NYiJRQ(Tc?S|TQ3EjW}A#vu}B zN&(%_D8MAnTjZ`v9p+1~yDZ&sFmWVI7gTf6C`sekxkD{mx-s=;?(L~llf;m~YRS-Gr@25hNwp1$)MM`Wpl9qAH?AnE zX<86$tSF=lUK4}Q8G){h{zR=@|8v%DckHhTF#gthv=)T%g^0rKwGflSNQ^!>{=oU< zjLN|tbZr2k2s#Q$avYr#3=TLP zwu!Z5=&W3LdFaM`)+vTQ0elu9?ZE*)bK2z$hk@3gB1stB$Y=qZc-bbG++Hu6p+3~J zJu_OgG_~kl$~4dH0~IF?-7|2ja^U9Io$F&i1iuS@d%HT4`|;MsNd9qh;M2vmBb&*A zN6CS2I{$K5UfoYh+~@5Qw>oHADbIQeUTjih-oOL^CMn1*n%WD!8}&|t4{6ce$10Z% zX=s_s+=|jTNf>ttqnz8pDx@-edRUpbKBZi_eq&D;sh@NG7lb!wJLiKG1~!Bqnf~R+!J)MiY`7y(i1fF zGwS~((y`X}aOL5h$B{E-u_huu{BidO-Ie&MDwlaKqTclSk?OJYKXrD0cCnJaP(Ac& UwR7|jR@8O`#7n>PAk}aF2VREl*Z=?k From e56b6e4fc5cf5343110a80a3727bfb24cdbb78c8 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 01:05:45 -0400 Subject: [PATCH 08/17] feat(gateway): per-key rate limiting (#59) --- services/gateway/src/api/app.py | 2 ++ services/gateway/src/api/ratelimit.py | 29 ++++++++++++++++++++++++ services/gateway/tests/test_ratelimit.py | 21 +++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 services/gateway/src/api/ratelimit.py create mode 100644 services/gateway/tests/test_ratelimit.py diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 1d7a3bc..6874e91 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -3,6 +3,7 @@ from contracts.directory import DirectoryUnavailable from src.api.middleware import AuditLogMiddleware +from src.api.ratelimit import RateLimitMiddleware from src.api.routers import resolve from src.config import verify_production_secrets @@ -16,6 +17,7 @@ def create_app() -> FastAPI: docs_url="/docs", ) app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") + app.add_middleware(RateLimitMiddleware, limit=60, window_s=60) @app.get("/health") def health() -> dict[str, str]: diff --git a/services/gateway/src/api/ratelimit.py b/services/gateway/src/api/ratelimit.py new file mode 100644 index 0000000..db3adcc --- /dev/null +++ b/services/gateway/src/api/ratelimit.py @@ -0,0 +1,29 @@ +"""Per-key fixed-window rate limit. In-memory (process-local): correct because the +gateway runs a single replica. A shared store (Redis) is needed only if scaled >1.""" + +import time + +from starlette.middleware.base import BaseHTTPMiddleware +from starlette.requests import Request +from starlette.responses import JSONResponse + + +class RateLimitMiddleware(BaseHTTPMiddleware): + def __init__(self, app, limit: int = 60, window_s: int = 60): + super().__init__(app) + self._limit = limit + self._window = window_s + self._hits: dict[str, tuple[int, float]] = {} # key -> (count, window_start) + + async def dispatch(self, request: Request, call_next): + key = request.headers.get("X-API-Key") + if key: + now = time.monotonic() + count, start = self._hits.get(key, (0, now)) + if now - start >= self._window: + count, start = 0, now + count += 1 + self._hits[key] = (count, start) + if count > self._limit: + return JSONResponse(status_code=429, content={"detail": "rate limit exceeded"}) + return await call_next(request) diff --git a/services/gateway/tests/test_ratelimit.py b/services/gateway/tests/test_ratelimit.py new file mode 100644 index 0000000..e040619 --- /dev/null +++ b/services/gateway/tests/test_ratelimit.py @@ -0,0 +1,21 @@ +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from src.api.ratelimit import RateLimitMiddleware + + +def test_limits_per_key(): + app = FastAPI() + app.add_middleware(RateLimitMiddleware, limit=2, window_s=60) + + @app.get("/x") + def x(): + return {"ok": True} + + c = TestClient(app) + h = {"X-API-Key": "k1"} + assert c.get("/x", headers=h).status_code == 200 + assert c.get("/x", headers=h).status_code == 200 + assert c.get("/x", headers=h).status_code == 429 + # a different key is unaffected + assert c.get("/x", headers={"X-API-Key": "k2"}).status_code == 200 From 656e30a5d3a5db7dc0ff96c7fc02ea137190cca2 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 01:10:22 -0400 Subject: [PATCH 09/17] fix(gateway): audit outermost so 429s are logged (#59) --- services/gateway/src/api/app.py | 2 +- services/gateway/tests/test_ratelimit.py | 28 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 6874e91..15e807e 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -16,8 +16,8 @@ def create_app() -> FastAPI: description="External API gateway.", docs_url="/docs", ) - app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") app.add_middleware(RateLimitMiddleware, limit=60, window_s=60) + app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") @app.get("/health") def health() -> dict[str, str]: diff --git a/services/gateway/tests/test_ratelimit.py b/services/gateway/tests/test_ratelimit.py index e040619..3ebc2c6 100644 --- a/services/gateway/tests/test_ratelimit.py +++ b/services/gateway/tests/test_ratelimit.py @@ -1,6 +1,9 @@ +import json + from fastapi import FastAPI from fastapi.testclient import TestClient +from src.api.middleware import AuditLogMiddleware from src.api.ratelimit import RateLimitMiddleware @@ -19,3 +22,28 @@ def x(): assert c.get("/x", headers=h).status_code == 429 # a different key is unaffected assert c.get("/x", headers={"X-API-Key": "k2"}).status_code == 200 + + +def test_429_is_still_audited(capsys): + # Mount in the same order as src.api.app.create_app(): RateLimitMiddleware + # added first (inner), AuditLogMiddleware added last (outer). Audit must + # be outermost so it observes the 429 short-circuit from the rate limiter. + app = FastAPI() + app.add_middleware(RateLimitMiddleware, limit=1, window_s=60) + app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") + + @app.get("/x") + def x(): + return {"ok": True} + + c = TestClient(app) + h = {"X-API-Key": "k1"} + assert c.get("/x", headers=h).status_code == 200 + assert c.get("/x", headers=h).status_code == 429 + + lines = [line for line in capsys.readouterr().out.strip().splitlines() if line.startswith("{")] + entries = [json.loads(line) for line in lines] + statuses = [entry.get("status") for entry in entries] + + assert 429 in statuses # the 429 response WAS audited (audit is outermost) + assert 200 in statuses From 18c5818b324e0ce5904b55d080b803961e5d1537 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 01:15:11 -0400 Subject: [PATCH 10/17] docs(gateway): document external gateway + first endpoint (#59) --- README.md | 13 ++- docs/ARCHITECTURE.md | 30 +++++++ services/gateway/README.md | 176 +++++++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+), 4 deletions(-) create mode 100644 services/gateway/README.md diff --git a/README.md b/README.md index 4784a96..897f630 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,7 @@ Every domain has a first-class HTTP API — build your own dashboard, sync job, - **[team-tracking](services/team-tracking/README.md)** — 26 endpoints across `people`, `teams`, `role_kinds`, `team_memberships`, `providers`, `person_identifiers`, `api_keys`. Full point-in-time roster queries. Scoped API keys, per-request audit log. **Actively consumed** by the Discord bot in production. - **[documentation-system](services/documentation-system/README.md)** — endpoints over `docs` and `sources`; ingest a URL and it's normalized, dedup'd, fetched (title + snapshot for supported sources), and owner-validated against team-tracking. Ownership degrades gracefully if the directory is unreachable. **Consumed** by the Discord bot's `/doc` command group (`add`, `list`, `show`, `remove`). +- **[gateway](services/gateway/README.md)** — the one **public** service. A narrow, scoped, rate-limited door onto the directory for external consumers (e.g. a GitHub Action) that shouldn't hold an internal team-tracking key. First endpoint: `GET /v1/resolve/discord/{github_login}`, returning only a Discord id. Every service speaks OpenAPI. Point Swagger UI or codegen at them. (`meeting`'s WebSocket route isn't representable in OpenAPI — its wire format is documented in [`services/meeting/README.md`](services/meeting/README.md).) @@ -64,6 +65,7 @@ The other four are internal-facing: **[llm](services/llm/README.md)** (`POST /ch | [`services/verification/`](services/verification/README.md) | Email verification: request a one-time code and confirm it, linking a subject (e.g. `discord:`) to a verified email; requires the `verification:write` scope | **Deployed** (staging + prod). | | [`services/meeting/`](services/meeting/README.md) | Meeting recording: transcribes a Discord voice session (Amazon Transcribe) and returns LLM-generated minutes as a branded PDF; no DB, nothing persisted | **Deployed** (staging). Consumed by the bot's `/record` command group; requires the `meetings` scope. | | [`services/connectors/`](services/connectors/README.md) | Stateless outbound adapter: fetches document content (Google Docs/Sheets/Slides/Drive) on behalf of internal consumers via a service account; no DB | **Deployed** (staging). Consumed by documentation-system's Google source fetches; requires the `fetch` scope. | +| [`services/gateway/`](services/gateway/README.md) | The one **public** service — a scoped, rate-limited external gateway onto the directory, with its own external key registry and one internal team-tracking key | Built. First endpoint: `GET /v1/resolve/discord/{github_login}` (used by #34's reviewer-ping GitHub Action). | | [`discord-bot/`](discord-bot/README.md) | Discord slash-command frontend + a browser-based "web playground" for iterating on commands without a Discord token | **Deployed** (staging + prod). All slash commands are stable and registered globally; 0 beta. | | Search / retrieval | Full-text + semantic search over the catalog's snapshots | Deferred (not built) | @@ -123,14 +125,17 @@ Misty/ │ ├── llm/ Bedrock /chat proxy — 8002, NO database │ ├── meeting/ Live meeting transcription — 8004, NO database, │ │ stateful (in-memory sessions) -│ └── connectors/ Google source fetch adapter — 8005, NO database +│ ├── connectors/ Google source fetch adapter — 8005, NO database +│ └── gateway/ External API gateway — 8006, own Postgres (external +│ key registry). The one PUBLIC service: scoped, +│ rate-limited, curated read surface │ (every service above has the same docs/ set: │ API.md, ARCHITECTURE.md, CONTRIBUTING.md, DEPLOYMENT.md) │ ├── packages/ │ └── auth/ platform_auth — shared API-key auth lib (argon2 hashing, │ scopes, FastAPI deps, audit middleware); a pure leaf -│ consumed by all six services via thin shims +│ consumed by all seven services via thin shims │ ├── discord-bot/ Discord frontend + web playground │ ├── src/ Node.js + discord.js @@ -149,7 +154,7 @@ Misty/ ├── PULL_REQUEST_TEMPLATE.md Zone, verification steps, deployment notes ├── ISSUE_TEMPLATE/ Bug / feature / epic issue forms (Blocked by + Zone fields) └── workflows/ - ├── ci.yml Tests + lint + Docker builds on every PR (10 jobs) + ├── ci.yml Tests + lint + Docker builds on every PR (11 jobs) ├── main-source-guard.yml Enforces "PRs to main come from staging" ├── pr-zone-check.yml Warns on PRs spanning multiple CODEOWNERS zones ├── label-consistency.yml Fails when the zone or area list drifts (runs check-labels.mjs) @@ -161,7 +166,7 @@ Misty/ └── blocked-ready-automation.yml Syncs blocked/ready issue labels ``` -Each service is self-contained: its own tests, its own docs, and its own database *if it needs one* — `llm`, `meeting`, and `connectors` deliberately have none. Dependencies are managed as one uv workspace rooted at this repo's `pyproject.toml`/`uv.lock`, and all six services share one leaf, `packages/auth` (`platform_auth`), for API-key auth — a shared *library* dependency, not a dependency between services, which remain independent of each other. Add a new service by dropping it in `services/` following the same shape (and adding its CI job in the same PR). +Each service is self-contained: its own tests, its own docs, and its own database *if it needs one* — `llm`, `meeting`, and `connectors` deliberately have none. Dependencies are managed as one uv workspace rooted at this repo's `pyproject.toml`/`uv.lock`, and all seven services share one leaf, `packages/auth` (`platform_auth`), for API-key auth — a shared *library* dependency, not a dependency between services, which remain independent of each other. Add a new service by dropping it in `services/` following the same shape (and adding its CI job in the same PR). --- diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e36b804..3001c4c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -212,6 +212,36 @@ describes how it applies them concretely. driven by an actor supplied via `X-On-Behalf-Of` rather than by the key alone. No other service needs this today. +## Access architecture: two doors, not one gateway + +The platform has exactly two ways in, and they're deliberately asymmetric — there is +**no internal gateway**. Internal services trust each other directly; only external +callers go through a gateway at all. + +- **Internal door.** team-tracking, documentation-system, and the gateway's own + outbound call to team-tracking all authenticate the same way: a per-consumer key, + argon2-hashed, scoped, issued by the target service's own CLI (`team-tracking-keys`, + `doc-keys`). The machinery is the shared [`packages/auth`](../packages/auth) + (`platform_auth`) library described above — each internal service is its own + authority over its own keys. There is no shared internal proxy standing in front of + them; adding a service that trusts another means issuing it a key on that service, + nothing more. +- **External door — [`services/gateway/`](../services/gateway/README.md), implemented.** + Callers outside the org's trust boundary (a GitHub Action, a future third-party + integration) never get a team-tracking key. Instead they hold a key issued by the + gateway's own external registry (`gateway-keys`, scoped e.g. `resolve:discord`), and + the gateway holds exactly **one** internal team-tracking key + (`identifiers:read`) for its own outbound calls. It composes and curates — it never + passes an internal response straight through — and adds the protections an + externally-facing surface needs that internal services don't: a public Railway + domain, per-key rate limiting, and audit logging of every external request. + +The gateway's first (and so far only) endpoint is the resolver, +`GET /v1/resolve/discord/{github_login}` — it turns a GitHub login into the Discord id +linked to the same person in the directory, and nothing else, for #34's reviewer-ping +GitHub Action. New external use cases get new narrow endpoints on the gateway, not +broader access to team-tracking itself. + ## Why the directory is built first The build order — **directory → docs catalog → search** — isn't arbitrary. It follows diff --git a/services/gateway/README.md b/services/gateway/README.md new file mode 100644 index 0000000..5ea92a2 --- /dev/null +++ b/services/gateway/README.md @@ -0,0 +1,176 @@ +# gateway + +The **public** external API gateway — a thin, scoped, rate-limited door onto UTMIST's +internal directory for third-party consumers (GitHub Actions, external integrations) +that should never see the internal `team-tracking` API directly. + +## What this service does + +team-tracking is the internal source of truth for the org, but not every consumer of +that data is inside the org's trust boundary. A GitHub Action, for instance, needs to +turn a GitHub login into a Discord id (to @-mention a reviewer) without holding a +team-tracking key or seeing anything else in the directory. + +The gateway exists for exactly that shape of consumer: + +- It holds **one internal team-tracking key** (scoped `identifiers:read`) for its own + outbound calls — external callers never see it. +- It issues and manages its **own, separate registry of external API keys** (via the + `gateway-keys` CLI), scoped to gateway-specific permissions like `resolve:discord`. +- It exposes a **narrow, curated surface** — today, one endpoint — that returns only + what the consumer needs (a Discord id), never a raw pass-through of team-tracking's + response. +- It rate-limits and audit-logs every request, since (unlike the internal services) + its callers are outside UTMIST's control. + +This is the "external door" half of the [access architecture](../../docs/ARCHITECTURE.md): +internal services trust each other via the shared `packages/auth` library and their own +keys; anything reaching in from outside the org goes through the gateway instead. + +## Quick start + +Prerequisites: Docker, Python 3.11+, [uv](https://github.com/astral-sh/uv). + +```bash +# 0. From the repo root, enter the service directory (all commands below run here) +cd services/gateway + +# 1. Copy environment config and start Postgres +cp .env.example .env +docker compose up -d postgres + +# 2. Install dependencies (including dev tools) +uv sync --extra dev + +# 3. Apply database migrations (creates the api_keys table) +uv run alembic upgrade head + +# 4. Start the API server +uv run uvicorn src.api.app:app --reload --port 8002 +``` + +> The repo is a single [uv workspace](https://docs.astral.sh/uv/concepts/workspaces/) (root `pyproject.toml` with `[tool.uv.workspace] members = ["services/*", "packages/*"]`, one root `uv.lock`). gateway depends on the shared `platform-auth` package (`[tool.uv.sources] platform-auth = { workspace = true }`) but the commands above are unchanged — `uv sync`, `uv run pytest`, `uv run alembic` still work exactly as shown when run from this directory. + +The API is now at `http://localhost:8002`. Interactive Swagger UI is at +`http://localhost:8002/docs`; the machine-readable schema is at +`http://localhost:8002/openapi.json`. + +### Configuration (`.env`) + +| Variable | Purpose | +|---|---| +| `DATABASE_URL` | The gateway's **own** Postgres — it holds only `api_keys` (its external key registry), never a copy of team-tracking's data. | +| `API_KEY` | Env grace-period bootstrap key for `platform_auth` (local dev only). | +| `DIRECTORY_BASE_URL` | team-tracking's base URL — where the gateway makes its **outbound** call. | +| `DIRECTORY_API_KEY` | The gateway's **one internal** team-tracking key, scoped `identifiers:read`. Issued on team-tracking via `team-tracking-keys issue --name gateway --scopes identifiers:read`. | +| `GATEWAY_ENV` | `local` / `staging` / `production`. In non-`local` envs, startup refuses to boot if `API_KEY` or `DIRECTORY_API_KEY` are still the built-in dev default. | + +## The resolver endpoint + +``` +GET /v1/resolve/discord/{github_login} +``` + +Resolves a GitHub login to the Discord id linked to the same person in team-tracking's +directory. Requires `X-API-Key` on a key scoped `resolve:discord`. + +**Response — `200 OK`:** + +```json +{ "discord_id": "123456789012345678" } +``` + +Only the Discord id is returned — no name, no team, no other identifiers. That's the +whole point of the curated surface: the gateway composes two internal calls +(`get_person_by_github` → `list_identifiers`) and hands back exactly one field. + +**Error responses:** + +| Status | Meaning | +|---|---| +| `401` | Missing or invalid `X-API-Key`. | +| `403` | Key valid but lacks the `resolve:discord` scope. | +| `404` | No person with that GitHub login, or that person has no linked Discord identifier. | +| `429` | Caller's key has exceeded the rate limit (60 requests / 60s per key, in-memory, single-replica). | +| `503` | team-tracking (the internal directory) is unreachable — the gateway doesn't guess; it fails closed. | + +## Managing external keys (`gateway-keys`) + +The gateway keeps its own key registry, separate from team-tracking's. Manage it with +the bundled CLI: + +```bash +# Issue an external key for a consumer (e.g. a GitHub Action) +uv run gateway-keys issue --name reviewer-ping --scopes resolve:discord +# Prints: gw__ (shown ONCE — capture it now) + +# List existing keys (metadata only, never plaintext) +uv run gateway-keys list --active-only + +# Revoke a compromised key (soft-delete; history preserved) +uv run gateway-keys revoke +``` + +Scopes recognized today: + +- `resolve:discord` — the only external-facing scope so far. +- `admin` — wildcard, grants all scopes. + +## Repo layout + +``` +gateway/ +├── contracts/ The domain boundary — no framework imports +│ ├── types.py Pydantic ApiKey type +│ ├── storage.py StorageAdapter Protocol +│ └── directory.py DirectoryClient Protocol + DirectoryUnavailable +│ +├── src/ +│ ├── api/ +│ │ ├── app.py App factory; mounts the resolver router + rate limit + audit middleware +│ │ ├── auth.py Thin shim over `platform_auth`: require_scope, get_actor +│ │ ├── hashing.py Thin shim over `platform_auth`: argon2 key hashing + gw__ generation +│ │ ├── middleware.py Thin shim over `platform_auth`: AuditLogMiddleware +│ │ ├── ratelimit.py Per-key fixed-window rate limit (in-memory, single-replica) +│ │ ├── deps.py get_storage() / get_directory() dependencies +│ │ └── routers/resolve.py `GET /v1/resolve/discord/{github_login}` +│ │ +│ ├── directory/http_client.py HTTP DirectoryClient — calls team-tracking with DIRECTORY_API_KEY +│ ├── storage/ StorageAdapter implementations (in-memory + Postgres) for the gateway's own api_keys table +│ ├── cli.py gateway-keys CLI (issue / list / revoke external keys) +│ └── config.py Settings (DATABASE_URL, API_KEY, DIRECTORY_*, GATEWAY_ENV) +│ +├── migrations/ Alembic — 001_api_keys +├── tests/ pytest — auth, cli, directory client, health, rate limit, resolver, storage +├── Dockerfile, railway.json Production image + Railway config (repo-root Docker context) +└── docker-compose.yml Local Postgres on port 5435 +``` + +## Testing + +```bash +uv run pytest +``` + +Lint and format with ruff: + +```bash +uv run ruff check . +uv run ruff format . +``` + +## Status + +Public gateway with its own `api_keys` registry (migration 001), one internal +team-tracking key for outbound calls, and one endpoint: +`GET /v1/resolve/discord/{github_login}`. Rate-limited (60 req/min/key) and +audit-logged. Built on `packages/auth` (`platform_auth`) — no auth logic is +reimplemented here. + +**Not implemented (by design):** + +- **More endpoints** — the gateway only exposes what an external consumer has an + actual need for; new endpoints are added deliberately, not by mirroring + team-tracking's surface. +- **Multi-replica rate limiting** — the in-memory limiter is correct for a single + Railway replica; a shared store (Redis) would be needed to scale horizontally. From 099a4d9639ac90550b984c5ab06658186034c0a5 Mon Sep 17 00:00:00 2001 From: Eeetan Date: Sun, 5 Jul 2026 02:13:02 -0400 Subject: [PATCH 11/17] harden(gateway): rate-limit eviction + login url-encoding + 503 logging (#59) Co-Authored-By: Claude Opus 4.8 --- services/gateway/src/api/app.py | 5 ++++ services/gateway/src/api/ratelimit.py | 13 ++++++++ services/gateway/src/directory/http_client.py | 4 ++- services/gateway/tests/test_directory.py | 19 ++++++++++++ services/gateway/tests/test_ratelimit.py | 30 +++++++++++++++++++ 5 files changed, 70 insertions(+), 1 deletion(-) diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 15e807e..9b43daf 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -1,3 +1,5 @@ +import logging + from fastapi import FastAPI, Request from fastapi.responses import JSONResponse @@ -7,6 +9,8 @@ from src.api.routers import resolve from src.config import verify_production_secrets +logger = logging.getLogger("gateway") + def create_app() -> FastAPI: verify_production_secrets() @@ -27,6 +31,7 @@ def health() -> dict[str, str]: @app.exception_handler(DirectoryUnavailable) async def _directory_unavailable(request: Request, exc: DirectoryUnavailable): + logger.warning("directory unavailable: %s", exc) return JSONResponse(status_code=503, content={"detail": "directory temporarily unavailable"}) return app diff --git a/services/gateway/src/api/ratelimit.py b/services/gateway/src/api/ratelimit.py index db3adcc..b3283ae 100644 --- a/services/gateway/src/api/ratelimit.py +++ b/services/gateway/src/api/ratelimit.py @@ -9,21 +9,34 @@ class RateLimitMiddleware(BaseHTTPMiddleware): + _SWEEP_THRESHOLD = 1024 + def __init__(self, app, limit: int = 60, window_s: int = 60): super().__init__(app) self._limit = limit self._window = window_s self._hits: dict[str, tuple[int, float]] = {} # key -> (count, window_start) + def _sweep(self, now: float) -> None: + """Opportunistically evict entries whose window has fully expired. + Only runs once the dict grows large, so it stays O(1) amortized.""" + if len(self._hits) < self._SWEEP_THRESHOLD: + return + expired = [k for k, (_, start) in self._hits.items() if now - start >= self._window] + for k in expired: + del self._hits[k] + async def dispatch(self, request: Request, call_next): key = request.headers.get("X-API-Key") if key: now = time.monotonic() + self._sweep(now) count, start = self._hits.get(key, (0, now)) if now - start >= self._window: count, start = 0, now count += 1 self._hits[key] = (count, start) + self._sweep(now) if count > self._limit: return JSONResponse(status_code=429, content={"detail": "rate limit exceeded"}) return await call_next(request) diff --git a/services/gateway/src/directory/http_client.py b/services/gateway/src/directory/http_client.py index 519c5ec..41d503b 100644 --- a/services/gateway/src/directory/http_client.py +++ b/services/gateway/src/directory/http_client.py @@ -1,3 +1,5 @@ +from urllib.parse import quote + import httpx from contracts.directory import DirectoryUnavailable @@ -31,7 +33,7 @@ def _get(self, path: str): return resp.json() def get_person_by_github(self, github_login: str) -> dict | None: - return self._get(f"/people/by-identifier/github/{github_login}") + return self._get(f"/people/by-identifier/github/{quote(github_login, safe='')}") def list_identifiers(self, person_id: str) -> list[dict]: result = self._get(f"/people/{person_id}/identifiers") diff --git a/services/gateway/tests/test_directory.py b/services/gateway/tests/test_directory.py index 3a521d9..4796896 100644 --- a/services/gateway/tests/test_directory.py +++ b/services/gateway/tests/test_directory.py @@ -18,6 +18,25 @@ def h(req): assert c.get_person_by_github("ghost") is None +def test_get_person_by_github_percent_encodes_login(): + captured = {} + + def h(req): + # raw_path is the on-the-wire (percent-encoded) path; req.url.path is decoded. + captured["path"] = req.url.raw_path.decode() + return httpx.Response(404) + + c = _client(h) + c.get_person_by_github("a b/c#d") + + path = captured["path"] + segment = path.removeprefix("/people/by-identifier/github/") + assert " " not in segment + assert "#" not in segment + assert "/" not in segment + assert segment == "a%20b%2Fc%23d" + + def test_list_identifiers_and_5xx_raises(): c = _client(lambda req: httpx.Response(200, json=[{"provider": "discord", "external_id": "42"}])) assert c.list_identifiers("p1") == [{"provider": "discord", "external_id": "42"}] diff --git a/services/gateway/tests/test_ratelimit.py b/services/gateway/tests/test_ratelimit.py index 3ebc2c6..95c70a7 100644 --- a/services/gateway/tests/test_ratelimit.py +++ b/services/gateway/tests/test_ratelimit.py @@ -1,4 +1,5 @@ import json +import time from fastapi import FastAPI from fastapi.testclient import TestClient @@ -47,3 +48,32 @@ def x(): assert 429 in statuses # the 429 response WAS audited (audit is outermost) assert 200 in statuses + + +def test_evicts_expired_entries(): + # Small window + low sweep threshold makes it easy to force a sweep + # without needing to spray thousands of distinct keys. + app = FastAPI() + + @app.get("/x") + def x(): + return {"ok": True} + + # Wrap the app directly with the middleware instance (BaseHTTPMiddleware + # is itself a valid ASGI app) so we can inspect/seed its internal state. + mw = RateLimitMiddleware(app, limit=60, window_s=1) + mw._SWEEP_THRESHOLD = 3 + + # Seed several distinct keys with an already-expired window. + stale_start = time.monotonic() - 10 + for i in range(5): + mw._hits[f"stale{i}"] = (1, stale_start) + assert len(mw._hits) == 5 + + c = TestClient(mw) + # A fresh request for a new key pushes len(_hits) >= threshold and + # triggers the sweep, which should clear all the expired stale entries. + assert c.get("/x", headers={"X-API-Key": "fresh"}).status_code == 200 + + assert all(not k.startswith("stale") for k in mw._hits) + assert "fresh" in mw._hits From 1998b151e55fe0ca674d8cfb2241838f6ff6a4ab Mon Sep 17 00:00:00 2001 From: Ethan Qiu Date: Sun, 16 Aug 2026 20:20:43 -0400 Subject: [PATCH 12/17] fix(gateway): no env-bootstrap admin key on the public door (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit platform_auth's env-bootstrap path mints an AuthedKey carrying ADMIN_SCOPE, and ADMIN_SCOPE is a wildcard — has_scope() returns True for every scope once it is present. On the internal services that is a deliberate grace path: it authenticates you to the admin API that issues the first real key, and it is only reachable from the private network. The gateway has neither half of that justification. Its keys are issued direct-to-DB by gateway-keys, so there is no admin API to bootstrap, and it is the one service exposed to the internet. Wiring API_KEY into it put a single wildcard credential on the public door for no gain. Pass get_env_key=lambda: None to disable the path, and drop the api_key setting so it cannot be quietly re-added. Also wrap directory_api_key in SecretStr, matching the convention the other services adopted after this branch forked: a plain str prints in full on any repr/diff/traceback. verify_production_secrets unwraps it explicitly, since a SecretStr never compares equal to a str and the guard would otherwise stop firing in silence — the failure mode platform_auth's secret_guard docstring asks each service to pin. tests/test_config.py pins it. Co-Authored-By: Claude Opus 5 --- services/gateway/.env.example | 13 ++++- services/gateway/src/api/auth.py | 18 +++++- services/gateway/src/api/deps.py | 22 +++++-- services/gateway/src/config.py | 43 +++++++++++--- services/gateway/src/directory/http_client.py | 20 ++++--- services/gateway/tests/test_auth.py | 34 +++++++++++ services/gateway/tests/test_config.py | 58 +++++++++++++++++++ 7 files changed, 184 insertions(+), 24 deletions(-) create mode 100644 services/gateway/tests/test_config.py diff --git a/services/gateway/.env.example b/services/gateway/.env.example index 4bac9a7..a521621 100644 --- a/services/gateway/.env.example +++ b/services/gateway/.env.example @@ -1,5 +1,16 @@ DATABASE_URL=postgresql+psycopg://gateway:dev_password@localhost:5435/gateway -API_KEY=dev-api-key-change-me + +# No API_KEY. Unlike the internal services, the gateway has no env-bootstrap +# admin key — inbound callers must present a scoped key issued by `gateway-keys` +# and stored in the api_keys table. See src/api/auth.py. + +# Outbound: the gateway's own team-tracking key, scoped identifiers:read. DIRECTORY_BASE_URL=http://localhost:8000 DIRECTORY_API_KEY=dev-api-key-change-me + GATEWAY_ENV=local + +# Set to true wherever a proxy terminates TLS in front of the gateway (Railway +# does). Off by default so a directly-exposed deploy can't be handed a spoofed +# X-Forwarded-For and slip the per-IP rate limit. +TRUST_PROXY_HEADERS=false diff --git a/services/gateway/src/api/auth.py b/services/gateway/src/api/auth.py index bb33bfc..79a71d8 100644 --- a/services/gateway/src/api/auth.py +++ b/services/gateway/src/api/auth.py @@ -1,14 +1,26 @@ """Thin shim: builds the gateway's auth deps from platform_auth (external keys).""" -from platform_auth import ADMIN_SCOPE, AuthedKey, build_auth # noqa: F401 +from platform_auth import AuthedKey, build_auth # noqa: F401 from src.api.deps import get_storage -from src.config import get_settings _deps = build_auth( get_storage, envelope="gw_", - get_env_key=lambda: get_settings().api_key, + # No env-bootstrap key, unlike every other service's shim. That path mints + # an AuthedKey carrying ADMIN_SCOPE, and ADMIN_SCOPE is a wildcard — + # AuthedKey.has_scope() returns True for every scope once it is present. On + # the internal services that is a deliberate grace path: it is how you + # authenticate to the admin API that issues the first real key, and it is + # only reachable from the private network. + # + # The gateway has neither half of that justification. Its keys are issued + # direct-to-DB by the gateway-keys CLI, so there is no admin API to + # bootstrap; and it is the one service exposed to the internet, so the env + # key would be a single wildcard credential on the public door. Returning + # None disables the path outright: every caller must present a scoped key + # from the api_keys table. tests/test_auth.py pins this. + get_env_key=lambda: None, audit_logger_name="gateway.audit", ) diff --git a/services/gateway/src/api/deps.py b/services/gateway/src/api/deps.py index f27b007..fe244d3 100644 --- a/services/gateway/src/api/deps.py +++ b/services/gateway/src/api/deps.py @@ -3,6 +3,7 @@ from sqlalchemy import create_engine from sqlalchemy.engine import Engine +from contracts.directory import DirectoryClient from contracts.storage import StorageAdapter from src.config import get_settings @@ -12,14 +13,25 @@ def _default_engine() -> Engine: return create_engine(get_settings().database_url, future=True, pool_pre_ping=True) +@lru_cache(maxsize=1) +def _default_directory() -> DirectoryClient: + from src.directory.http_client import HttpDirectoryClient + + s = get_settings() + # .get_secret_value() at the boundary: HttpDirectoryClient puts this + # straight into an outbound header, which needs the raw str. This is one of + # the two sanctioned unwrap sites (the other is verify_production_secrets); + # everywhere else the field stays wrapped. + return HttpDirectoryClient(s.directory_base_url, s.directory_api_key.get_secret_value()) + + def get_storage() -> StorageAdapter: from src.storage.postgres import PostgresStorageAdapter return PostgresStorageAdapter(_default_engine()) -def get_directory(): - from src.directory.http_client import HttpDirectoryClient - - s = get_settings() - return HttpDirectoryClient(s.directory_base_url, s.directory_api_key) +def get_directory() -> DirectoryClient: + # Cached, not per-request: the client owns a connection pool (see + # HttpDirectoryClient), which is worthless if it is rebuilt every request. + return _default_directory() diff --git a/services/gateway/src/config.py b/services/gateway/src/config.py index 5c3c2ef..a48e38f 100644 --- a/services/gateway/src/config.py +++ b/services/gateway/src/config.py @@ -1,8 +1,12 @@ from functools import lru_cache from typing import Literal +from pydantic import SecretStr from pydantic_settings import BaseSettings, SettingsConfigDict +# The built-in dev secret for the gateway's OUTBOUND team-tracking key. Only +# acceptable when gateway_env == "local"; any other environment must override +# DIRECTORY_API_KEY with the real issued key (see verify_production_secrets). DEFAULT_DEV_API_KEY = "dev-api-key-change-me" @@ -10,13 +14,30 @@ class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") database_url: str = "postgresql+psycopg://gateway:dev_password@localhost:5435/gateway" - # Env grace-period admin key for platform_auth's bootstrap path (local dev). - api_key: str = DEFAULT_DEV_API_KEY + + # NOTE: there is deliberately no inbound `api_key` here. Every other service + # carries one to feed platform_auth's env-bootstrap path; the gateway does + # not, because that path grants ADMIN_SCOPE and this is the only service + # reachable from the internet. See src/api/auth.py for the full reasoning. + # Outbound: the gateway's own team-tracking key (scoped identifiers:read). + # SecretStr, not str: a plain str field prints in full on any repr/diff/ + # traceback, and this is a live credential for the private directory. + # SecretStr makes that structurally impossible — repr/str always render + # "**********" — so don't revert this to str. Only the two boundaries that + # must see the raw value (verify_production_secrets below, and + # src/api/deps.py building the outbound header) call .get_secret_value(). directory_base_url: str = "http://localhost:8000" - directory_api_key: str = DEFAULT_DEV_API_KEY + directory_api_key: SecretStr = SecretStr(DEFAULT_DEV_API_KEY) + gateway_env: Literal["local", "staging", "production"] = "local" + # Whether an upstream proxy sets X-Forwarded-For / X-Real-IP. Off by default + # so a direct deploy can't be fed a spoofed client IP; the Railway deploy + # sets TRUST_PROXY_HEADERS=true. Read by the per-IP rate limiter, which is + # only as good as its notion of "who is calling" (see src/api/ratelimit.py). + trust_proxy_headers: bool = False + @lru_cache(maxsize=1) def get_settings() -> Settings: @@ -24,16 +45,24 @@ def get_settings() -> Settings: def verify_production_secrets(settings: Settings | None = None) -> None: + """Fail fast if a non-local environment is still using built-in dev secrets. + + Called from create_app(), so a misconfigured deploy dies at startup rather + than on the first request that needs the directory. + """ settings = settings or get_settings() if settings.gateway_env == "local": return insecure: list[str] = [] - if settings.api_key == DEFAULT_DEV_API_KEY: - insecure.append("API_KEY") - if settings.directory_api_key == DEFAULT_DEV_API_KEY: + # .get_secret_value() is required: SecretStr never compares equal to a str, + # so `settings.directory_api_key == DEFAULT_DEV_API_KEY` would silently be + # False forever and this guard would stop firing without any test noticing. + # tests/test_config.py pins exactly that. + if settings.directory_api_key.get_secret_value() == DEFAULT_DEV_API_KEY: insecure.append("DIRECTORY_API_KEY") if insecure: raise RuntimeError( f"Refusing to start in gateway_env={settings.gateway_env!r}: " - f"{', '.join(insecure)} still set to the built-in dev default." + f"{', '.join(insecure)} still set to the built-in dev default. " + "Set a strong, unique value via environment variables." ) diff --git a/services/gateway/src/directory/http_client.py b/services/gateway/src/directory/http_client.py index 41d503b..ae4bdef 100644 --- a/services/gateway/src/directory/http_client.py +++ b/services/gateway/src/directory/http_client.py @@ -10,22 +10,26 @@ class HttpDirectoryClient: """Looks up people and identifiers over team-tracking's HTTP API. A 404 means 'no such record' (returns None); connection failure or 5xx means - 'directory unavailable' (raises DirectoryUnavailable).""" + 'directory unavailable' (raises DirectoryUnavailable). + + Holds one pooled httpx.Client for its whole lifetime. The resolver makes two + directory calls per request, so a client built (and closed) per call would + mean two fresh TCP + TLS handshakes on the hot path. Construct this once — + src/api/deps.py caches the instance — rather than per request. + """ def __init__(self, base_url: str, api_key: str, client: httpx.Client | None = None) -> None: self._base_url = base_url.rstrip("/") - self._api_key = api_key - self._client = client + # Set once as a default header so the key never has to be rebuilt (or + # accidentally logged) per call. It is never read back out. + self._client = client or httpx.Client(timeout=_TIMEOUT) + self._client.headers["X-API-Key"] = api_key def _get(self, path: str): - client = self._client or httpx.Client(timeout=_TIMEOUT) try: - resp = client.get(f"{self._base_url}{path}", headers={"X-API-Key": self._api_key}) + resp = self._client.get(f"{self._base_url}{path}") except httpx.HTTPError as e: raise DirectoryUnavailable(f"directory unreachable: {e}") from e - finally: - if self._client is None: - client.close() if resp.status_code == 404: return None if not (200 <= resp.status_code < 300): diff --git a/services/gateway/tests/test_auth.py b/services/gateway/tests/test_auth.py index e33e86b..2610a9a 100644 --- a/services/gateway/tests/test_auth.py +++ b/services/gateway/tests/test_auth.py @@ -30,3 +30,37 @@ def test_missing_scope_403_and_no_key_401(): client, key = _client_with_key(["other:scope"]) assert client.get("/probe", headers={"X-API-Key": key}).status_code == 403 assert client.get("/probe").status_code == 401 + + +def test_revoked_key_401(): + store = InMemoryStorageAdapter() + plaintext, prefix, key_hash = generate_key() + row = store.create_api_key( + name="c", prefix=prefix, key_hash=key_hash, scopes=["resolve:discord"], actor="t" + ) + app = FastAPI() + + @app.get("/probe") + def probe(_=Depends(require_scope("resolve:discord"))): + return {"ok": True} + + app.dependency_overrides[get_storage] = lambda: store + client = TestClient(app) + assert client.get("/probe", headers={"X-API-Key": plaintext}).status_code == 200 + store.revoke_api_key(row.id, actor="t") + assert client.get("/probe", headers={"X-API-Key": plaintext}).status_code == 401 + + +def test_no_env_bootstrap_admin_path(): + """The gateway must not honour an env key. See src/api/auth.py. + + platform_auth's bootstrap path hands out ADMIN_SCOPE, which satisfies every + require_scope check. src/api/auth.py disables it by passing + get_env_key=lambda: None, and this pins that: nothing that isn't a live row + in api_keys gets through, including the value other services use as their + env key. Without the guard, `API_KEY=` in the gateway's + environment would be a wildcard credential on the public door. + """ + client, _ = _client_with_key(["resolve:discord"]) + for candidate in ("dev-api-key-change-me", "", "admin", "gw_notarealkey"): + assert client.get("/probe", headers={"X-API-Key": candidate}).status_code == 401 diff --git a/services/gateway/tests/test_config.py b/services/gateway/tests/test_config.py new file mode 100644 index 0000000..f104d7a --- /dev/null +++ b/services/gateway/tests/test_config.py @@ -0,0 +1,58 @@ +import pytest +from pydantic import SecretStr + +from src.config import DEFAULT_DEV_API_KEY, Settings, verify_production_secrets + + +def _settings(**overrides) -> Settings: + # _env_file=None so a developer's local .env can't leak into these + # assertions; every field under test is passed explicitly. + base = { + "gateway_env": "production", + "directory_api_key": SecretStr("a-real-issued-key"), + } + base.update(overrides) + return Settings(_env_file=None, **base) + + +def test_no_inbound_api_key_field(): + """The gateway must not carry an env-bootstrap key at all. + + Re-adding an `api_key` field is the first half of re-enabling the wildcard + admin path this service deliberately does without; src/api/auth.py is the + second half, pinned by tests/test_auth.py. + """ + assert "api_key" not in Settings.model_fields + + +def test_local_tolerates_the_dev_default(): + verify_production_secrets( + _settings(gateway_env="local", directory_api_key=SecretStr(DEFAULT_DEV_API_KEY)) + ) + + +@pytest.mark.parametrize("env", ["staging", "production"]) +def test_non_local_refuses_the_dev_default(env): + """The guard fires — and keeps firing once directory_api_key is a SecretStr. + + This is the test platform_auth's secret_guard docstring asks every service + to have. A SecretStr never compares equal to a str, so writing the check as + `settings.directory_api_key == DEFAULT_DEV_API_KEY` (without + .get_secret_value()) makes it False forever: the service boots happily in + production with a publicly-known key and nothing else notices. + """ + with pytest.raises(RuntimeError, match="DIRECTORY_API_KEY"): + verify_production_secrets( + _settings(gateway_env=env, directory_api_key=SecretStr(DEFAULT_DEV_API_KEY)) + ) + + +def test_non_local_accepts_a_real_secret(): + verify_production_secrets(_settings()) + + +def test_directory_key_is_redacted_in_repr(): + s = _settings(directory_api_key=SecretStr("super-secret-value")) + assert "super-secret-value" not in repr(s) + assert "super-secret-value" not in str(s.directory_api_key) + assert s.directory_api_key.get_secret_value() == "super-secret-value" From 61b8fe587cdbc5f57662abb61c9ad72c2d4af8a1 Mon Sep 17 00:00:00 2001 From: Ethan Qiu Date: Sun, 16 Aug 2026 20:25:23 -0400 Subject: [PATCH 13/17] fix(gateway): bound the rate limiter and meter issued keys, not headers (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old limiter kept an unbounded dict keyed on the raw X-API-Key header, in front of auth. Three problems, all of which matter more here than they would on a private service: - anyone could grow it without limit inside a window, since the sweep only evicted entries whose window had already expired; - past 1024 entries every request paid a full O(n) scan of the dict, and _sweep was called twice per request, so the cost of a flood landed on everyone else; - rotating the header value defeated the limit outright, which is what makes the first two exploitable rather than theoretical — and a well-formed gw_ key forces an argon2 verification, so unmetered attempts are unmetered CPU. Split it into the two things it was conflating. The per-consumer quota is now a dependency running after require_api_key, keyed on AuthedKey.name: that set is bounded by the keys we have issued, so it cannot be grown from outside, and no plaintext key sits in process memory. In front of auth sits a per-IP flood guard, whose only job is to cap how much argon2 work one address can demand; /health is exempt so the liveness probe cannot be throttled. Both use FixedWindowCounter, which is bounded by construction: entries are ordered by window start, so eviction takes the oldest at O(1) instead of scanning. When trusting proxy headers it reads the rightmost X-Forwarded-For hop, the only one a client cannot write itself — the leftmost would have been per-request bucket rotation by design. Co-Authored-By: Claude Opus 5 --- services/gateway/src/api/app.py | 14 +- services/gateway/src/api/ratelimit.py | 182 ++++++++++++++--- services/gateway/tests/conftest.py | 16 ++ services/gateway/tests/test_ratelimit.py | 243 ++++++++++++++++++----- 4 files changed, 375 insertions(+), 80 deletions(-) create mode 100644 services/gateway/tests/conftest.py diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 9b43daf..9abd43c 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -5,9 +5,9 @@ from contracts.directory import DirectoryUnavailable from src.api.middleware import AuditLogMiddleware -from src.api.ratelimit import RateLimitMiddleware +from src.api.ratelimit import ClientRateLimitMiddleware from src.api.routers import resolve -from src.config import verify_production_secrets +from src.config import get_settings, verify_production_secrets logger = logging.getLogger("gateway") @@ -20,7 +20,15 @@ def create_app() -> FastAPI: description="External API gateway.", docs_url="/docs", ) - app.add_middleware(RateLimitMiddleware, limit=60, window_s=60) + # Order matters, and add_middleware prepends: the last one added is the + # outermost. Audit must be outermost so it still records the requests the + # flood guard short-circuits — a 429 storm is exactly what you want in the + # log. The per-key quota is not here; it runs as a router dependency, after + # auth has resolved the key (see src/api/ratelimit.py). + app.add_middleware( + ClientRateLimitMiddleware, + trust_proxy=get_settings().trust_proxy_headers, + ) app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") @app.get("/health") diff --git a/services/gateway/src/api/ratelimit.py b/services/gateway/src/api/ratelimit.py index b3283ae..f617aa6 100644 --- a/services/gateway/src/api/ratelimit.py +++ b/services/gateway/src/api/ratelimit.py @@ -1,42 +1,170 @@ -"""Per-key fixed-window rate limit. In-memory (process-local): correct because the -gateway runs a single replica. A shared store (Redis) is needed only if scaled >1.""" +"""Rate limiting for the public door. + +Two layers, because they defend different things. + +`enforce_key_rate_limit` is the real per-consumer quota. It runs as a FastAPI +dependency *after* platform_auth has resolved the key, and counts against +`AuthedKey.name` — a value that can only have come from a row in `api_keys`. +That matters as much for the bookkeeping as for the quota: the set of names is +bounded by the number of keys we have issued, so nobody on the internet can grow +the counter, and no plaintext key is held in process memory. + +`ClientRateLimitMiddleware` sits in front of auth and counts per client IP. +Authentication is itself the expensive step — a well-formed `gw_` key forces an +argon2 verification, which is deliberately slow — so something has to bound how +fast an unauthenticated caller can demand one, and per-key limiting cannot: an +attacker simply varies the key. Its limit is loose on purpose. It is a +floodgate, not a quota. + +Both sit on FixedWindowCounter, which is bounded by construction. The first +version of this module keyed an unbounded dict on the raw `X-API-Key` header, in +front of auth: anyone could grow it without limit inside a window, and every +request past 1024 entries then paid a full O(n) scan (twice). + +In-memory and process-local, which is correct because the gateway runs a single +replica. A shared store (Redis) is needed only if it is ever scaled past one. +""" import time +from collections import OrderedDict +from fastapi import Depends, HTTPException, status +from platform_auth import AuthedKey from starlette.middleware.base import BaseHTTPMiddleware from starlette.requests import Request from starlette.responses import JSONResponse +from src.api.auth import require_api_key -class RateLimitMiddleware(BaseHTTPMiddleware): - _SWEEP_THRESHOLD = 1024 +_TOO_MANY = "rate limit exceeded" - def __init__(self, app, limit: int = 60, window_s: int = 60): - super().__init__(app) +# Per issued key. The quota an external consumer actually gets. +KEY_LIMIT = 60 +KEY_WINDOW_S = 60.0 + +# Per client IP, in front of auth. Set well above the per-key quota: this is not +# trying to meter anyone, only to cap how much argon2 work one address can +# demand. Note that with TRUST_PROXY_HEADERS unset behind a proxy, every caller +# shares one bucket — another reason to keep it loose. +IP_LIMIT = 120 +IP_WINDOW_S = 60.0 + + +class FixedWindowCounter: + """Per-identity fixed-window counter with a hard capacity. + + The capacity is the point. `hit()` takes an identity supplied — directly or + indirectly — by the caller, so an unbounded dict is a memory-growth vector + for anyone who can vary it. Entries are held in insertion order and an entry + is re-inserted whenever its window restarts, so the front of the dict is + always the oldest window: evicting from the front drops the entry closest to + expiring, which is what a scan for expired entries would have picked anyway, + at O(1) instead of O(n). + + Eviction under pressure is deliberately permissive — a flood of fresh + identities can push a legitimate one out and hand it a fresh window. That + trade is on purpose: the alternative, refusing new identities once full, + would let an attacker lock everyone else out, turning a rate limiter into a + denial-of-service tool. + """ + + def __init__(self, *, limit: int, window_s: float, capacity: int = 8192) -> None: self._limit = limit self._window = window_s - self._hits: dict[str, tuple[int, float]] = {} # key -> (count, window_start) + self._capacity = capacity + self._hits: OrderedDict[str, tuple[int, float]] = OrderedDict() + + def hit(self, identity: str, now: float | None = None) -> bool: + """Record a request for `identity`. False means it is over the limit.""" + now = time.monotonic() if now is None else now + entry = self._hits.get(identity) + if entry is not None: + count, start = entry + if now - start < self._window: + self._hits[identity] = (count + 1, start) + return count + 1 <= self._limit + # Window elapsed. Drop it so the re-insert below puts it at the + # back, keeping the dict ordered by window start. + del self._hits[identity] + while len(self._hits) >= self._capacity: + self._hits.popitem(last=False) + self._hits[identity] = (1, now) + return 1 <= self._limit + + def clear(self) -> None: + self._hits.clear() - def _sweep(self, now: float) -> None: - """Opportunistically evict entries whose window has fully expired. - Only runs once the dict grows large, so it stays O(1) amortized.""" - if len(self._hits) < self._SWEEP_THRESHOLD: - return - expired = [k for k, (_, start) in self._hits.items() if now - start >= self._window] - for k in expired: - del self._hits[k] + def __len__(self) -> int: + return len(self._hits) + + +def client_identity(request: Request, *, trust_proxy: bool) -> str: + """Best available identifier for the caller, for per-IP limiting. + + With `trust_proxy`, take the *rightmost* X-Forwarded-For entry. A proxy + appends the address it actually saw, so the rightmost hop is the one value + in that header the client could not have written itself. Taking the leftmost + — the usual "original client" reading — would let anyone reset their own + bucket by sending a fresh X-Forwarded-For on every request, which defeats + the whole layer. + + Without `trust_proxy` the headers are ignored entirely and the socket peer + is used. Behind an unacknowledged proxy that collapses every caller into one + bucket, which is why IP_LIMIT is set loose enough not to bite a real + consumer. + """ + if trust_proxy: + forwarded = request.headers.get("X-Forwarded-For") + if forwarded: + hops = [hop.strip() for hop in forwarded.split(",") if hop.strip()] + if hops: + return hops[-1] + real_ip = request.headers.get("X-Real-IP") + if real_ip and real_ip.strip(): + return real_ip.strip() + return request.client.host if request.client else "unknown" + + +class ClientRateLimitMiddleware(BaseHTTPMiddleware): + """Per-IP flood guard, mounted in front of auth. See the module docstring.""" + + def __init__( + self, + app, + *, + limit: int = IP_LIMIT, + window_s: float = IP_WINDOW_S, + trust_proxy: bool = False, + capacity: int = 8192, + ) -> None: + super().__init__(app) + self._counter = FixedWindowCounter(limit=limit, window_s=window_s, capacity=capacity) + self._trust_proxy = trust_proxy async def dispatch(self, request: Request, call_next): - key = request.headers.get("X-API-Key") - if key: - now = time.monotonic() - self._sweep(now) - count, start = self._hits.get(key, (0, now)) - if now - start >= self._window: - count, start = 0, now - count += 1 - self._hits[key] = (count, start) - self._sweep(now) - if count > self._limit: - return JSONResponse(status_code=429, content={"detail": "rate limit exceeded"}) + # /health is exempt: it is unauthenticated, costs nothing, and Railway's + # liveness probe hits it on a fixed interval. Letting the flood guard + # 429 the probe would restart a service that is answering fine. + if request.url.path == "/health": + return await call_next(request) + if not self._counter.hit(client_identity(request, trust_proxy=self._trust_proxy)): + return JSONResponse(status_code=429, content={"detail": _TOO_MANY}) return await call_next(request) + + +def build_key_rate_limit(counter: FixedWindowCounter): + """FastAPI dependency enforcing `counter` against the authenticated key.""" + + def _dep(key: AuthedKey = Depends(require_api_key)) -> AuthedKey: + if not counter.hit(key.name): + raise HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, detail=_TOO_MANY + ) + return key + + return _dep + + +# The process-wide per-key quota, mounted on the /v1 router (see routers/resolve.py). +key_rate_limit_counter = FixedWindowCounter(limit=KEY_LIMIT, window_s=KEY_WINDOW_S) +enforce_key_rate_limit = build_key_rate_limit(key_rate_limit_counter) diff --git a/services/gateway/tests/conftest.py b/services/gateway/tests/conftest.py new file mode 100644 index 0000000..b5441cb --- /dev/null +++ b/services/gateway/tests/conftest.py @@ -0,0 +1,16 @@ +import pytest + +from src.api.ratelimit import key_rate_limit_counter + + +@pytest.fixture(autouse=True) +def _reset_key_rate_limit(): + """The per-key quota is process-wide, so it would otherwise carry across tests. + + Every test builds its key with the same name, which means without this they + all share one bucket and the suite starts failing once it grows past + KEY_LIMIT requests — a confusing failure a long way from its cause. + """ + key_rate_limit_counter.clear() + yield + key_rate_limit_counter.clear() diff --git a/services/gateway/tests/test_ratelimit.py b/services/gateway/tests/test_ratelimit.py index 95c70a7..611ef75 100644 --- a/services/gateway/tests/test_ratelimit.py +++ b/services/gateway/tests/test_ratelimit.py @@ -1,79 +1,222 @@ import json -import time -from fastapi import FastAPI +from fastapi import Depends, FastAPI from fastapi.testclient import TestClient +from starlette.requests import Request +from src.api.auth import require_scope +from src.api.deps import get_storage +from src.api.hashing import generate_key from src.api.middleware import AuditLogMiddleware -from src.api.ratelimit import RateLimitMiddleware +from src.api.ratelimit import ( + ClientRateLimitMiddleware, + FixedWindowCounter, + build_key_rate_limit, + client_identity, +) +from src.storage.in_memory import InMemoryStorageAdapter -def test_limits_per_key(): +# --- FixedWindowCounter ------------------------------------------------------- + + +def test_counts_within_a_window_then_refuses(): + c = FixedWindowCounter(limit=2, window_s=60) + assert c.hit("a", now=100.0) is True + assert c.hit("a", now=100.5) is True + assert c.hit("a", now=100.9) is False + + +def test_identities_are_independent(): + c = FixedWindowCounter(limit=1, window_s=60) + assert c.hit("a", now=100.0) is True + assert c.hit("a", now=100.1) is False + assert c.hit("b", now=100.1) is True + + +def test_window_rolls_over(): + c = FixedWindowCounter(limit=1, window_s=10) + assert c.hit("a", now=100.0) is True + assert c.hit("a", now=105.0) is False + assert c.hit("a", now=110.0) is True # new window + + +def test_capacity_is_hard(): + """The whole point: an attacker varying the identity cannot grow this. + + The previous implementation kept an unbounded dict keyed on the raw + X-API-Key header, in front of auth, so anyone could add entries without + limit inside a window — and past 1024 entries each request paid a full scan. + """ + c = FixedWindowCounter(limit=10, window_s=60, capacity=32) + for i in range(10_000): + c.hit(f"attacker-{i}", now=100.0) + assert len(c) == 32 + + +def test_eviction_drops_the_oldest_window_first(): + c = FixedWindowCounter(limit=10, window_s=60, capacity=3) + c.hit("oldest", now=100.0) + c.hit("middle", now=101.0) + c.hit("newest", now=102.0) + c.hit("arrival", now=103.0) + assert "oldest" not in c._hits + assert set(c._hits) == {"middle", "newest", "arrival"} + + +def test_restarting_a_window_moves_an_entry_to_the_back(): + # Ordering is what makes O(1) eviction correct: an entry whose window + # restarts is no longer the oldest and must not be the next one evicted. + c = FixedWindowCounter(limit=10, window_s=10, capacity=2) + c.hit("a", now=100.0) + c.hit("b", now=101.0) + c.hit("a", now=120.0) # a's window restarts; a is now the newest + c.hit("c", now=121.0) # evicts one entry + assert "b" not in c._hits + assert set(c._hits) == {"a", "c"} + + +# --- client_identity ---------------------------------------------------------- + + +def _request(headers: dict, peer: str = "10.0.0.1") -> Request: + raw = [(k.lower().encode(), v.encode()) for k, v in headers.items()] + return Request({"type": "http", "headers": raw, "client": (peer, 1234)}) + + +def test_untrusted_proxy_headers_are_ignored(): + r = _request({"X-Forwarded-For": "1.2.3.4", "X-Real-IP": "5.6.7.8"}) + assert client_identity(r, trust_proxy=False) == "10.0.0.1" + + +def test_trusted_proxy_takes_the_rightmost_hop(): + """The rightmost entry is the only one the client could not have written. + + A client that sends `X-Forwarded-For: 1.2.3.4` gets the proxy's observed + address appended after it. Reading the leftmost value — the usual "original + client" convention — would let anyone rotate their own bucket per request + and walk straight through this layer. + """ + r = _request({"X-Forwarded-For": "1.2.3.4, 9.9.9.9"}) + assert client_identity(r, trust_proxy=True) == "9.9.9.9" + + +def test_trusted_proxy_falls_back_to_real_ip_then_peer(): + assert client_identity(_request({"X-Real-IP": "5.6.7.8"}), trust_proxy=True) == "5.6.7.8" + assert client_identity(_request({}), trust_proxy=True) == "10.0.0.1" + + +# --- per-IP middleware -------------------------------------------------------- + + +def _ip_app(**kwargs) -> TestClient: app = FastAPI() - app.add_middleware(RateLimitMiddleware, limit=2, window_s=60) @app.get("/x") def x(): return {"ok": True} - c = TestClient(app) - h = {"X-API-Key": "k1"} - assert c.get("/x", headers=h).status_code == 200 - assert c.get("/x", headers=h).status_code == 200 - assert c.get("/x", headers=h).status_code == 429 - # a different key is unaffected - assert c.get("/x", headers={"X-API-Key": "k2"}).status_code == 200 + @app.get("/health") + def health(): + return {"status": "ok"} + app.add_middleware(ClientRateLimitMiddleware, **kwargs) + return TestClient(app) -def test_429_is_still_audited(capsys): - # Mount in the same order as src.api.app.create_app(): RateLimitMiddleware - # added first (inner), AuditLogMiddleware added last (outer). Audit must - # be outermost so it observes the 429 short-circuit from the rate limiter. - app = FastAPI() - app.add_middleware(RateLimitMiddleware, limit=1, window_s=60) - app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") - @app.get("/x") - def x(): +def test_ip_limit_applies_without_any_key(): + # The point of this layer: it bounds unauthenticated callers, who by + # definition have no key to meter. + c = _ip_app(limit=2, window_s=60) + assert c.get("/x").status_code == 200 + assert c.get("/x").status_code == 200 + assert c.get("/x").status_code == 429 + + +def test_rotating_the_api_key_does_not_escape_the_ip_limit(): + c = _ip_app(limit=2, window_s=60) + assert c.get("/x", headers={"X-API-Key": "gw_one"}).status_code == 200 + assert c.get("/x", headers={"X-API-Key": "gw_two"}).status_code == 200 + assert c.get("/x", headers={"X-API-Key": "gw_three"}).status_code == 429 + + +def test_health_is_exempt(): + # Railway's liveness probe hits /health on a fixed interval; 429ing it would + # restart a service that is answering perfectly well. + c = _ip_app(limit=1, window_s=60) + for _ in range(5): + assert c.get("/health").status_code == 200 + + +# --- per-key quota ------------------------------------------------------------ + + +def _keyed_app(limit: int): + store = InMemoryStorageAdapter() + plaintext, prefix, key_hash = generate_key() + store.create_api_key( + name="consumer", prefix=prefix, key_hash=key_hash, scopes=["resolve:discord"], actor="t" + ) + other_plain, other_prefix, other_hash = generate_key() + store.create_api_key( + name="other", prefix=other_prefix, key_hash=other_hash, scopes=["resolve:discord"], actor="t" + ) + + counter = FixedWindowCounter(limit=limit, window_s=60) + app = FastAPI(dependencies=[Depends(build_key_rate_limit(counter))]) + + @app.get("/probe") + def probe(_=Depends(require_scope("resolve:discord"))): return {"ok": True} - c = TestClient(app) - h = {"X-API-Key": "k1"} - assert c.get("/x", headers=h).status_code == 200 - assert c.get("/x", headers=h).status_code == 429 + app.dependency_overrides[get_storage] = lambda: store + return TestClient(app), plaintext, other_plain - lines = [line for line in capsys.readouterr().out.strip().splitlines() if line.startswith("{")] - entries = [json.loads(line) for line in lines] - statuses = [entry.get("status") for entry in entries] - assert 429 in statuses # the 429 response WAS audited (audit is outermost) - assert 200 in statuses +def test_key_quota_is_per_issued_key(): + c, key, other = _keyed_app(limit=2) + assert c.get("/probe", headers={"X-API-Key": key}).status_code == 200 + assert c.get("/probe", headers={"X-API-Key": key}).status_code == 200 + assert c.get("/probe", headers={"X-API-Key": key}).status_code == 429 + # A different issued key has its own bucket. + assert c.get("/probe", headers={"X-API-Key": other}).status_code == 200 + + +def test_unauthenticated_requests_never_reach_the_key_counter(): + """401 must win over 429, and an anonymous caller must not create a bucket. + + This is the structural fix: the quota runs behind require_api_key, so the + only strings it can ever be keyed on are names of keys we issued. + """ + c, key, _ = _keyed_app(limit=1) + for _ in range(5): + assert c.get("/probe").status_code == 401 + assert c.get("/probe", headers={"X-API-Key": "gw_bogus"}).status_code == 401 + # The real key still has its full quota — the noise above consumed none. + assert c.get("/probe", headers={"X-API-Key": key}).status_code == 200 -def test_evicts_expired_entries(): - # Small window + low sweep threshold makes it easy to force a sweep - # without needing to spray thousands of distinct keys. +def test_429_is_still_audited(capsys): + # Audit is added last, so it is outermost and observes the flood guard's + # short-circuit. Mirrors the order in src.api.app.create_app(). app = FastAPI() @app.get("/x") def x(): return {"ok": True} - # Wrap the app directly with the middleware instance (BaseHTTPMiddleware - # is itself a valid ASGI app) so we can inspect/seed its internal state. - mw = RateLimitMiddleware(app, limit=60, window_s=1) - mw._SWEEP_THRESHOLD = 3 - - # Seed several distinct keys with an already-expired window. - stale_start = time.monotonic() - 10 - for i in range(5): - mw._hits[f"stale{i}"] = (1, stale_start) - assert len(mw._hits) == 5 - - c = TestClient(mw) - # A fresh request for a new key pushes len(_hits) >= threshold and - # triggers the sweep, which should clear all the expired stale entries. - assert c.get("/x", headers={"X-API-Key": "fresh"}).status_code == 200 + app.add_middleware(ClientRateLimitMiddleware, limit=1, window_s=60) + app.add_middleware(AuditLogMiddleware, logger_name="gateway.audit") - assert all(not k.startswith("stale") for k in mw._hits) - assert "fresh" in mw._hits + c = TestClient(app) + assert c.get("/x").status_code == 200 + assert c.get("/x").status_code == 429 + + entries = [ + json.loads(line) + for line in capsys.readouterr().out.strip().splitlines() + if line.startswith("{") + ] + statuses = [e.get("status") for e in entries] + assert 429 in statuses + assert 200 in statuses From 33e00a4e88b3120e8f40e881b36503bf7a6e2ad5 Mon Sep 17 00:00:00 2001 From: Ethan Qiu Date: Sun, 16 Aug 2026 20:25:34 -0400 Subject: [PATCH 14/17] fix(gateway): stop the resolver's 404s revealing directory membership (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two misses had different bodies — "github login not found" versus "no discord identifier for that github login" — which let any holder of a resolve:discord key walk a list of GitHub logins and learn which of them belong to UTMIST members. That is a membership oracle on a public endpoint, and it is precisely the disclosure this service exists to prevent. Both now return one body; the distinction survives in the audit log, where only we can read it. While in here: - declare the response as a model, so the "only ever discord_id" contract is stated in the OpenAPI schema rather than only in prose; - read `person["id"]` as `.get("id")` and fail closed to 503 when it is absent. An unrecognised upstream shape is an upstream fault, not a missing record, and blind indexing surfaced it as a 500; - record why `github_login` reaches the audit log, since it is a path segment and the middleware logs the path. That is deliberate — it is what makes abuse of the public door investigable — but the branch had claimed the opposite, so it needed saying where someone will read it. Co-Authored-By: Claude Opus 5 --- services/gateway/src/api/routers/resolve.py | 60 ++++++++++++++++----- services/gateway/tests/test_resolve.py | 43 +++++++++++++++ 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/services/gateway/src/api/routers/resolve.py b/services/gateway/src/api/routers/resolve.py index 86afdd7..e893bc3 100644 --- a/services/gateway/src/api/routers/resolve.py +++ b/services/gateway/src/api/routers/resolve.py @@ -1,26 +1,62 @@ from fastapi import APIRouter, Depends, HTTPException, status +from pydantic import BaseModel, ConfigDict -from contracts.directory import DirectoryClient +from contracts.directory import DirectoryClient, DirectoryUnavailable from src.api.auth import AuthedKey, require_scope from src.api.deps import get_directory +from src.api.ratelimit import enforce_key_rate_limit -router = APIRouter(prefix="/v1", tags=["resolve"]) +# One body for both misses. Splitting them — "github login not found" versus +# "no discord identifier for that github login" — told an external caller +# whether a given GitHub user is in the directory at all. That is a membership +# oracle on a public endpoint, and enumerating it is cheap: exactly the kind of +# leak this service exists to prevent. The two cases stay distinguishable in the +# audit log, where only we can read them. +_NOT_FOUND = "no discord id for that github login" -@router.get("/resolve/discord/{github_login}") +class DiscordId(BaseModel): + """The entire public response. Nothing else about the person leaves here.""" + + model_config = ConfigDict(extra="forbid") + discord_id: str + + +router = APIRouter( + prefix="/v1", + tags=["resolve"], + # The per-consumer quota, applied to every /v1 route. It runs after + # require_api_key resolves the caller, so it meters an issued key rather + # than an attacker-supplied header; see src/api/ratelimit.py. Wrong-scope + # requests are metered too — they are still requests we had to authenticate. + dependencies=[Depends(enforce_key_rate_limit)], +) + + +@router.get("/resolve/discord/{github_login}", response_model=DiscordId) def resolve_discord( github_login: str, directory: DirectoryClient = Depends(get_directory), _: AuthedKey = Depends(require_scope("resolve:discord")), -) -> dict[str, str]: +) -> DiscordId: + # `github_login` reaches the audit log, because it is a path segment and + # AuditLogMiddleware records request.url.path. That is deliberate: it is + # what makes abuse of the public door investigable, the value is public and + # pseudonymous, and the caller supplied it in the first place. What never + # joins it there is anything the directory told us back. person = directory.get_person_by_github(github_login) if person is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="github login not found") - ids = directory.list_identifiers(person["id"]) - discord_id = next((i["external_id"] for i in ids if i.get("provider") == "discord"), None) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=_NOT_FOUND) + person_id = person.get("id") + if person_id is None: + # The directory answered, but not in a shape we understand. That is an + # upstream fault rather than a missing record, so fail closed to 503 + # instead of raising KeyError into a 500. + raise DirectoryUnavailable("person record has no id") + identifiers = directory.list_identifiers(person_id) + discord_id = next( + (i.get("external_id") for i in identifiers if i.get("provider") == "discord"), None + ) if discord_id is None: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail="no discord identifier for that github login", - ) - return {"discord_id": discord_id} + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=_NOT_FOUND) + return DiscordId(discord_id=discord_id) diff --git a/services/gateway/tests/test_resolve.py b/services/gateway/tests/test_resolve.py index 1ccf73e..4fe2081 100644 --- a/services/gateway/tests/test_resolve.py +++ b/services/gateway/tests/test_resolve.py @@ -47,11 +47,54 @@ def test_no_discord_identifier_404(): assert c.get("/v1/resolve/discord/octocat", headers=h).status_code == 404 +def test_the_two_misses_are_indistinguishable(): + """"Not in the directory" and "in it, but no Discord link" must look identical. + + Otherwise the endpoint is a membership oracle: anyone holding a + resolve:discord key could walk a list of GitHub logins and learn which of + them belong to UTMIST members, which is more than this endpoint is meant to + disclose about anyone. + """ + absent, absent_h = _client(FakeDir(person=None)) + present, present_h = _client( + FakeDir(person={"id": "p1"}, idents=[{"provider": "github", "external_id": "x"}]) + ) + a = absent.get("/v1/resolve/discord/octocat", headers=absent_h) + b = present.get("/v1/resolve/discord/octocat", headers=present_h) + assert a.status_code == b.status_code == 404 + assert a.json() == b.json() + + def test_directory_down_503(): c, h = _client(FakeDir(down=True)) assert c.get("/v1/resolve/discord/octocat", headers=h).status_code == 503 +def test_person_without_an_id_fails_closed_to_503(): + # An unrecognised upstream shape is an upstream fault, not a missing record. + # Indexing it blindly would raise KeyError and surface as a 500. + c, h = _client(FakeDir(person={"name": "no id here"})) + assert c.get("/v1/resolve/discord/octocat", headers=h).status_code == 503 + + +def test_response_carries_only_the_discord_id(): + c, h = _client( + FakeDir( + person={"id": "p1", "primary_email": "someone@example.com", "full_name": "Someone"}, + idents=[ + {"provider": "discord", "external_id": "42"}, + {"provider": "uoft_email", "external_id": "someone@utoronto.ca"}, + ], + ) + ) + r = c.get("/v1/resolve/discord/octocat", headers=h) + assert r.status_code == 200 + assert r.json() == {"discord_id": "42"} + body = r.text + for leaked in ("someone@example.com", "Someone", "utoronto.ca", "p1"): + assert leaked not in body + + def test_requires_scope_and_key(): c, h = _client(FakeDir(person={"id": "p1"}, idents=[{"provider": "discord", "external_id": "42"}])) assert c.get("/v1/resolve/discord/octocat").status_code == 401 From 537abfa21db57e0342ecccae49b5332f1a92f681 Mon Sep 17 00:00:00 2001 From: Ethan Qiu Date: Sun, 16 Aug 2026 20:26:52 -0400 Subject: [PATCH 15/17] chore(gateway): pool the outbound client, and tidy the rough edges (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five small things, none of them worth its own commit: - HttpDirectoryClient now holds one pooled httpx.Client for its lifetime, and deps caches the instance. It built and closed a client per call, and the resolver makes two calls per request, so the hot path was paying two fresh TCP + TLS handshakes. - touch_api_key_last_used logs instead of swallowing silently. Best-effort is right — a DB blip must not fail auth over a bookkeeping write — but in silence the only symptom is a key that looks unused while in daily use. - gateway-keys issue reports a duplicate --name as a sentence and exit 1, matching revoke, rather than an argparse-free traceback. Nothing goes to stdout on that path: a caller piping stdout into a secrets store must not be handed a key that was never persisted. - CI runs `ruff format --check`, like every other Python job in this workflow. It did not, which is why the format pass below touches files this branch had otherwise left alone. - `ruff format` over the service, to make that check pass. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 2 ++ services/gateway/src/api/app.py | 4 +++- services/gateway/src/api/ratelimit.py | 4 +--- services/gateway/src/cli.py | 21 +++++++++++++---- services/gateway/src/storage/postgres.py | 11 ++++++++- services/gateway/tests/test_cli.py | 14 ++++++++++++ services/gateway/tests/test_directory.py | 9 ++++++-- services/gateway/tests/test_ratelimit.py | 6 ++++- services/gateway/tests/test_resolve.py | 29 +++++++++++++++++------- services/gateway/tests/test_storage.py | 9 ++++++-- 10 files changed, 86 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a0a4a8..1a3a8d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -228,6 +228,8 @@ jobs: run: uv run pytest - name: Ruff check run: uv run ruff check . + - name: Ruff format check + run: uv run ruff format --check . node-test: runs-on: ubuntu-latest diff --git a/services/gateway/src/api/app.py b/services/gateway/src/api/app.py index 9abd43c..563675e 100644 --- a/services/gateway/src/api/app.py +++ b/services/gateway/src/api/app.py @@ -40,7 +40,9 @@ def health() -> dict[str, str]: @app.exception_handler(DirectoryUnavailable) async def _directory_unavailable(request: Request, exc: DirectoryUnavailable): logger.warning("directory unavailable: %s", exc) - return JSONResponse(status_code=503, content={"detail": "directory temporarily unavailable"}) + return JSONResponse( + status_code=503, content={"detail": "directory temporarily unavailable"} + ) return app diff --git a/services/gateway/src/api/ratelimit.py b/services/gateway/src/api/ratelimit.py index f617aa6..fbf25a8 100644 --- a/services/gateway/src/api/ratelimit.py +++ b/services/gateway/src/api/ratelimit.py @@ -157,9 +157,7 @@ def build_key_rate_limit(counter: FixedWindowCounter): def _dep(key: AuthedKey = Depends(require_api_key)) -> AuthedKey: if not counter.hit(key.name): - raise HTTPException( - status_code=status.HTTP_429_TOO_MANY_REQUESTS, detail=_TOO_MANY - ) + raise HTTPException(status_code=status.HTTP_429_TOO_MANY_REQUESTS, detail=_TOO_MANY) return key return _dep diff --git a/services/gateway/src/cli.py b/services/gateway/src/cli.py index 09b7631..cb54243 100644 --- a/services/gateway/src/cli.py +++ b/services/gateway/src/cli.py @@ -24,10 +24,19 @@ def _adapter() -> PostgresStorageAdapter: def cmd_issue(args: argparse.Namespace) -> int: plaintext, prefix, key_hash = generate_key() - key = _adapter().create_api_key( - name=args.name, prefix=prefix, key_hash=key_hash, scopes=list(args.scopes or []), - actor=args.actor, - ) + try: + key = _adapter().create_api_key( + name=args.name, + prefix=prefix, + key_hash=key_hash, + scopes=list(args.scopes or []), + actor=args.actor, + ) + except ValueError as e: + # Almost always a duplicate --name. Matching cmd_revoke, an operator + # error gets a sentence and an exit code, not a traceback. + print(f"error: {e}", file=sys.stderr) + return 1 print("=" * 70, file=sys.stderr) print("EXTERNAL API KEY ISSUED (shown once)", file=sys.stderr) print(f" Name: {key.name}", file=sys.stderr) @@ -63,7 +72,9 @@ def cmd_revoke(args: argparse.Namespace) -> int: def build_parser() -> argparse.ArgumentParser: - p = argparse.ArgumentParser(prog="gateway-keys", description="Manage gateway external API keys.") + p = argparse.ArgumentParser( + prog="gateway-keys", description="Manage gateway external API keys." + ) p.add_argument("--actor", default="cli") subs = p.add_subparsers(dest="cmd", required=True) pi = subs.add_parser("issue") diff --git a/services/gateway/src/storage/postgres.py b/services/gateway/src/storage/postgres.py index 5fb053f..659d257 100644 --- a/services/gateway/src/storage/postgres.py +++ b/services/gateway/src/storage/postgres.py @@ -1,3 +1,4 @@ +import logging from datetime import datetime, timezone from uuid import UUID @@ -8,6 +9,8 @@ from contracts.types import ApiKey from src.storage.schema import api_keys +logger = logging.getLogger("gateway.storage") + def _now() -> datetime: return datetime.now(timezone.utc) @@ -108,4 +111,10 @@ def touch_api_key_last_used(self, api_key_id: UUID) -> None: update(api_keys).where(api_keys.c.id == api_key_id).values(last_used_at=_now()) ) except Exception: - pass # best-effort; DB blips must not fail the auth path + # Best-effort: a DB blip must not fail the auth path over a + # bookkeeping write. Logged rather than swallowed outright, because + # silence here means last_used_at can go stale indefinitely and the + # only symptom is a key that looks unused while it is in daily use. + logger.warning( + "could not update last_used_at for api key %s", api_key_id, exc_info=True + ) diff --git a/services/gateway/tests/test_cli.py b/services/gateway/tests/test_cli.py index 00ad30b..ef0b602 100644 --- a/services/gateway/tests/test_cli.py +++ b/services/gateway/tests/test_cli.py @@ -13,3 +13,17 @@ def test_issue_mints_verifiable_key(monkeypatch, capsys): assert store.get_api_key_hash(prefix) is not None assert verify_key(plaintext, store.get_api_key_hash(prefix)) is True assert store.get_api_key_by_prefix(prefix).scopes == ["resolve:discord"] + + +def test_issue_reports_a_duplicate_name_without_a_traceback(monkeypatch, capsys): + store = InMemoryStorageAdapter() + monkeypatch.setattr(cli, "_adapter", lambda: store) + assert cli.main(["issue", "--name", "gh-action"]) == 0 + capsys.readouterr() + + assert cli.main(["issue", "--name", "gh-action"]) == 1 + captured = capsys.readouterr() + assert "error:" in captured.err + # Nothing on stdout: a caller piping stdout to a secrets store must not be + # handed a key that was never persisted. + assert captured.out.strip() == "" diff --git a/services/gateway/tests/test_directory.py b/services/gateway/tests/test_directory.py index 4796896..47b1c77 100644 --- a/services/gateway/tests/test_directory.py +++ b/services/gateway/tests/test_directory.py @@ -5,7 +5,9 @@ def _client(handler): - return HttpDirectoryClient("http://d", "k", client=httpx.Client(transport=httpx.MockTransport(handler))) + return HttpDirectoryClient( + "http://d", "k", client=httpx.Client(transport=httpx.MockTransport(handler)) + ) def test_get_person_by_github_found_and_404(): @@ -13,6 +15,7 @@ def h(req): if req.url.path == "/people/by-identifier/github/octocat": return httpx.Response(200, json={"id": "p1"}) return httpx.Response(404) + c = _client(h) assert c.get_person_by_github("octocat") == {"id": "p1"} assert c.get_person_by_github("ghost") is None @@ -38,7 +41,9 @@ def h(req): def test_list_identifiers_and_5xx_raises(): - c = _client(lambda req: httpx.Response(200, json=[{"provider": "discord", "external_id": "42"}])) + c = _client( + lambda req: httpx.Response(200, json=[{"provider": "discord", "external_id": "42"}]) + ) assert c.list_identifiers("p1") == [{"provider": "discord", "external_id": "42"}] c2 = _client(lambda req: httpx.Response(503)) try: diff --git a/services/gateway/tests/test_ratelimit.py b/services/gateway/tests/test_ratelimit.py index 611ef75..d328c0d 100644 --- a/services/gateway/tests/test_ratelimit.py +++ b/services/gateway/tests/test_ratelimit.py @@ -159,7 +159,11 @@ def _keyed_app(limit: int): ) other_plain, other_prefix, other_hash = generate_key() store.create_api_key( - name="other", prefix=other_prefix, key_hash=other_hash, scopes=["resolve:discord"], actor="t" + name="other", + prefix=other_prefix, + key_hash=other_hash, + scopes=["resolve:discord"], + actor="t", ) counter = FixedWindowCounter(limit=limit, window_s=60) diff --git a/services/gateway/tests/test_resolve.py b/services/gateway/tests/test_resolve.py index 4fe2081..fbbf125 100644 --- a/services/gateway/tests/test_resolve.py +++ b/services/gateway/tests/test_resolve.py @@ -10,10 +10,12 @@ class FakeDir: def __init__(self, person=None, idents=None, down=False): self._p, self._i, self._down = person, idents or [], down + def get_person_by_github(self, login): if self._down: raise DirectoryUnavailable("x") return self._p + def list_identifiers(self, pid): return self._i @@ -21,8 +23,9 @@ def list_identifiers(self, pid): def _client(fake): store = InMemoryStorageAdapter() plaintext, prefix, key_hash = generate_key() - store.create_api_key(name="c", prefix=prefix, key_hash=key_hash, - scopes=["resolve:discord"], actor="t") + store.create_api_key( + name="c", prefix=prefix, key_hash=key_hash, scopes=["resolve:discord"], actor="t" + ) app = create_app() app.dependency_overrides[get_storage] = lambda: store app.dependency_overrides[get_directory] = lambda: fake @@ -30,9 +33,15 @@ def _client(fake): def test_resolves_discord_id(): - c, h = _client(FakeDir(person={"id": "p1"}, - idents=[{"provider": "github", "external_id": "octocat"}, - {"provider": "discord", "external_id": "42"}])) + c, h = _client( + FakeDir( + person={"id": "p1"}, + idents=[ + {"provider": "github", "external_id": "octocat"}, + {"provider": "discord", "external_id": "42"}, + ], + ) + ) r = c.get("/v1/resolve/discord/octocat", headers=h) assert r.status_code == 200 and r.json() == {"discord_id": "42"} @@ -43,12 +52,14 @@ def test_login_not_found_404(): def test_no_discord_identifier_404(): - c, h = _client(FakeDir(person={"id": "p1"}, idents=[{"provider": "github", "external_id": "x"}])) + c, h = _client( + FakeDir(person={"id": "p1"}, idents=[{"provider": "github", "external_id": "x"}]) + ) assert c.get("/v1/resolve/discord/octocat", headers=h).status_code == 404 def test_the_two_misses_are_indistinguishable(): - """"Not in the directory" and "in it, but no Discord link" must look identical. + """ "Not in the directory" and "in it, but no Discord link" must look identical. Otherwise the endpoint is a membership oracle: anyone holding a resolve:discord key could walk a list of GitHub logins and learn which of @@ -96,5 +107,7 @@ def test_response_carries_only_the_discord_id(): def test_requires_scope_and_key(): - c, h = _client(FakeDir(person={"id": "p1"}, idents=[{"provider": "discord", "external_id": "42"}])) + c, h = _client( + FakeDir(person={"id": "p1"}, idents=[{"provider": "discord", "external_id": "42"}]) + ) assert c.get("/v1/resolve/discord/octocat").status_code == 401 diff --git a/services/gateway/tests/test_storage.py b/services/gateway/tests/test_storage.py index 48df1e6..ef09d7d 100644 --- a/services/gateway/tests/test_storage.py +++ b/services/gateway/tests/test_storage.py @@ -3,8 +3,13 @@ def test_create_get_verify_revoke_roundtrip(): a = InMemoryStorageAdapter() - key = a.create_api_key(name="gh-action", prefix="abcd1234", key_hash="HASH", - scopes=["resolve:discord"], actor="cli") + key = a.create_api_key( + name="gh-action", + prefix="abcd1234", + key_hash="HASH", + scopes=["resolve:discord"], + actor="cli", + ) assert key.name == "gh-action" and key.active is True assert a.get_api_key_hash("abcd1234") == "HASH" row = a.get_api_key_by_prefix("abcd1234") From 84e651fa52d9e88c10c3f3694520c45a111bd7e8 Mon Sep 17 00:00:00 2001 From: Ethan Qiu Date: Sun, 16 Aug 2026 20:28:59 -0400 Subject: [PATCH 16/17] docs(gateway): say what the public door actually does (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch's docs claimed the audit log "never carries person data". It does: the GitHub login is a path segment, and AuditLogMiddleware records request.url.path, so every call writes the login the caller asked about. That is the right behaviour — an audit trail for a public endpoint that omitted what was requested would be useless for investigating abuse, and the value is public, pseudonymous, and caller-supplied — but the docs said the opposite, and a security claim nobody has checked is worse than none. Now stated, along with what genuinely never appears there: anything the directory answered back. Also documents a constraint the resolver inherits and cannot fix locally: team-tracking matches person_identifiers.external_id exactly for every provider but email, so GitHub logins resolve case-sensitively even though GitHub's are not. The gateway deliberately does not lowercase the login — that only helps if stored values are already lowercase, and breaks the mixed-case links that work today. The fix is upstream (normalise github identifiers on write, plus a migration) and is tracked separately. Plus the fallout from the preceding commits: no API_KEY, the two rate-limit layers, TRUST_PROXY_HEADERS, the single 404 body, and a warning never to issue a gateway key scoped `admin`. The local dev port moves 8002 → 8006, which was colliding with llm. Co-Authored-By: Claude Opus 5 --- docs/ARCHITECTURE.md | 13 ++++- services/gateway/README.md | 112 ++++++++++++++++++++++++++++++------- 2 files changed, 104 insertions(+), 21 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3001c4c..01fd6bc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -234,7 +234,18 @@ callers go through a gateway at all. (`identifiers:read`) for its own outbound calls. It composes and curates — it never passes an internal response straight through — and adds the protections an externally-facing surface needs that internal services don't: a public Railway - domain, per-key rate limiting, and audit logging of every external request. + domain, per-key rate limiting behind a per-IP flood guard, and audit logging of + every external request. + + The asymmetry runs one level deeper than the key registries. Every internal + service also accepts an env-bootstrap key (`API_KEY`), which `platform_auth` + resolves to the wildcard `admin` scope — the grace path you use to reach the + admin API that issues the first real key, and safe because only the private + network can reach it. **The gateway does not.** It has no admin API to + bootstrap (`gateway-keys` writes to its database directly) and it is the one + service on the public internet, so it passes `get_env_key=lambda: None` and + every caller must present an issued, scoped key. When adding an + externally-reachable service, copy that, not the internal shim. The gateway's first (and so far only) endpoint is the resolver, `GET /v1/resolve/discord/{github_login}` — it turns a GitHub login into the Discord id diff --git a/services/gateway/README.md b/services/gateway/README.md index 5ea92a2..bbcd0d7 100644 --- a/services/gateway/README.md +++ b/services/gateway/README.md @@ -46,24 +46,34 @@ uv sync --extra dev uv run alembic upgrade head # 4. Start the API server -uv run uvicorn src.api.app:app --reload --port 8002 +uv run uvicorn src.api.app:app --reload --port 8006 ``` > The repo is a single [uv workspace](https://docs.astral.sh/uv/concepts/workspaces/) (root `pyproject.toml` with `[tool.uv.workspace] members = ["services/*", "packages/*"]`, one root `uv.lock`). gateway depends on the shared `platform-auth` package (`[tool.uv.sources] platform-auth = { workspace = true }`) but the commands above are unchanged — `uv sync`, `uv run pytest`, `uv run alembic` still work exactly as shown when run from this directory. -The API is now at `http://localhost:8002`. Interactive Swagger UI is at -`http://localhost:8002/docs`; the machine-readable schema is at -`http://localhost:8002/openapi.json`. +The API is now at `http://localhost:8006` (8000–8005 are the internal services). +Interactive Swagger UI is at `http://localhost:8006/docs`; the machine-readable +schema is at `http://localhost:8006/openapi.json`. ### Configuration (`.env`) | Variable | Purpose | |---|---| | `DATABASE_URL` | The gateway's **own** Postgres — it holds only `api_keys` (its external key registry), never a copy of team-tracking's data. | -| `API_KEY` | Env grace-period bootstrap key for `platform_auth` (local dev only). | | `DIRECTORY_BASE_URL` | team-tracking's base URL — where the gateway makes its **outbound** call. | | `DIRECTORY_API_KEY` | The gateway's **one internal** team-tracking key, scoped `identifiers:read`. Issued on team-tracking via `team-tracking-keys issue --name gateway --scopes identifiers:read`. | -| `GATEWAY_ENV` | `local` / `staging` / `production`. In non-`local` envs, startup refuses to boot if `API_KEY` or `DIRECTORY_API_KEY` are still the built-in dev default. | +| `GATEWAY_ENV` | `local` / `staging` / `production`. In non-`local` envs, startup refuses to boot if `DIRECTORY_API_KEY` is still the built-in dev default. | +| `TRUST_PROXY_HEADERS` | Whether a proxy in front of the gateway sets `X-Forwarded-For` / `X-Real-IP`. **Set this to `true` on Railway.** Left `false`, the per-IP flood guard buckets every caller behind the proxy together; set `true` where no proxy exists, and a caller can forge their own source address. | + +**There is deliberately no `API_KEY`.** Every other service in this repo carries +one to feed `platform_auth`'s env-bootstrap path, which authenticates you as a +key holding `admin` — a wildcard scope. On the internal services that is a +sanctioned grace path, reachable only from the private network, and it is how +you authenticate to the admin API that issues the first real key. The gateway +has neither justification: its keys are issued straight into the database by +`gateway-keys`, and it is the only service exposed to the internet. Setting +`API_KEY` here does nothing — `src/api/auth.py` passes `get_env_key=lambda: +None`, and `tests/test_auth.py` pins that it stays that way. ## The resolver endpoint @@ -90,9 +100,61 @@ whole point of the curated surface: the gateway composes two internal calls |---|---| | `401` | Missing or invalid `X-API-Key`. | | `403` | Key valid but lacks the `resolve:discord` scope. | -| `404` | No person with that GitHub login, or that person has no linked Discord identifier. | -| `429` | Caller's key has exceeded the rate limit (60 requests / 60s per key, in-memory, single-replica). | -| `503` | team-tracking (the internal directory) is unreachable — the gateway doesn't guess; it fails closed. | +| `404` | No Discord id for that GitHub login. | +| `429` | Rate limit exceeded — either the caller's key quota or the per-IP flood guard (see below). | +| `503` | team-tracking (the internal directory) is unreachable, or answered in a shape we don't recognise — the gateway doesn't guess; it fails closed. | + +The `404` is deliberately one message for two different situations: "that login +isn't in the directory" and "it is, but there's no Discord account linked". Told +apart, they let anyone holding a `resolve:discord` key feed in a list of GitHub +logins and learn which of them are UTMIST members — a membership oracle on a +public endpoint. The distinction is still visible in the audit log, where only +we can read it. `tests/test_resolve.py` pins that the two responses are byte-identical. + +### Two things to know about this endpoint + +**The lookup is case-sensitive, and GitHub logins are not.** team-tracking +matches `person_identifiers.external_id` exactly for every provider except +`email`, so a person stored as `octocat` will **not** be found by a request for +`OctoCat`. Whoever links the identifier and whoever calls the endpoint have to +agree on casing. The gateway does not lowercase the login, because that would +only help if stored values were already lowercase and would break the mixed-case +links that currently work. The real fix belongs upstream — normalise `github` +identifiers on write in team-tracking, plus a migration for existing rows — and +is tracked separately. Until then, link GitHub identifiers using the exact login +GitHub reports. + +**The GitHub login appears in the audit log.** It's a path segment, and +`AuditLogMiddleware` records `request.url.path`, so every call writes the login +the caller asked about to stdout. That's intentional: an audit trail for the +public door that omitted *what was requested* would be close to useless for +investigating abuse, and a GitHub login is public, pseudonymous, and supplied by +the caller in the first place. What is never logged is anything the directory +answered back — no person id, no name, no other identifiers, and not the Discord +id itself. + +### Rate limiting + +Two layers, because they defend different things: + +| Layer | Where | Limit | Keyed on | +|---|---|---|---| +| Per-consumer quota | Router dependency, **after** auth | 60 / 60s | `AuthedKey.name` — an issued key | +| Flood guard | Middleware, **in front of** auth | 120 / 60s | Client IP (`/health` exempt) | + +The quota is the one a consumer notices. It runs after `require_api_key`, so it +can only ever be keyed on the name of a key we issued — the set is bounded by +our own key registry, no plaintext key sits in process memory, and nobody +outside can grow it. + +The flood guard exists because authentication is itself the expensive step: a +well-formed `gw_` key forces an argon2 verification, which is slow by design. +Per-key limiting can't bound that — an attacker just varies the key — so +something in front of auth has to. Its limit is loose on purpose; it is a +floodgate, not a quota. + +Both are in-memory and process-local, which is correct for a single replica. A +shared store (Redis) is needed only if the gateway is ever scaled past one. ## Managing external keys (`gateway-keys`) @@ -113,8 +175,13 @@ uv run gateway-keys revoke Scopes recognized today: -- `resolve:discord` — the only external-facing scope so far. -- `admin` — wildcard, grants all scopes. +- `resolve:discord` — the only external-facing scope, and the only one to issue. + +`platform_auth` also treats `admin` as a wildcard that satisfies every scope +check. **Never issue a gateway key with it.** There is no bootstrap path that +needs one here (see the note on `API_KEY` above), and on a service reachable +from the internet a wildcard key is a standing invitation. Scope every external +key to exactly the endpoint its consumer calls. ## Repo layout @@ -127,21 +194,21 @@ gateway/ │ ├── src/ │ ├── api/ -│ │ ├── app.py App factory; mounts the resolver router + rate limit + audit middleware -│ │ ├── auth.py Thin shim over `platform_auth`: require_scope, get_actor +│ │ ├── app.py App factory; mounts the resolver router + flood guard + audit middleware +│ │ ├── auth.py Thin shim over `platform_auth`: require_scope, get_actor — env-bootstrap path OFF │ │ ├── hashing.py Thin shim over `platform_auth`: argon2 key hashing + gw__ generation │ │ ├── middleware.py Thin shim over `platform_auth`: AuditLogMiddleware -│ │ ├── ratelimit.py Per-key fixed-window rate limit (in-memory, single-replica) -│ │ ├── deps.py get_storage() / get_directory() dependencies +│ │ ├── ratelimit.py Bounded fixed-window counter; per-key quota + per-IP flood guard +│ │ ├── deps.py get_storage() / get_directory() dependencies (pooled directory client) │ │ └── routers/resolve.py `GET /v1/resolve/discord/{github_login}` │ │ │ ├── directory/http_client.py HTTP DirectoryClient — calls team-tracking with DIRECTORY_API_KEY │ ├── storage/ StorageAdapter implementations (in-memory + Postgres) for the gateway's own api_keys table │ ├── cli.py gateway-keys CLI (issue / list / revoke external keys) -│ └── config.py Settings (DATABASE_URL, API_KEY, DIRECTORY_*, GATEWAY_ENV) +│ └── config.py Settings (DATABASE_URL, DIRECTORY_*, GATEWAY_ENV, TRUST_PROXY_HEADERS) │ ├── migrations/ Alembic — 001_api_keys -├── tests/ pytest — auth, cli, directory client, health, rate limit, resolver, storage +├── tests/ pytest — auth, cli, config guards, directory client, health, rate limit, resolver, storage ├── Dockerfile, railway.json Production image + Railway config (repo-root Docker context) └── docker-compose.yml Local Postgres on port 5435 ``` @@ -163,9 +230,10 @@ uv run ruff format . Public gateway with its own `api_keys` registry (migration 001), one internal team-tracking key for outbound calls, and one endpoint: -`GET /v1/resolve/discord/{github_login}`. Rate-limited (60 req/min/key) and -audit-logged. Built on `packages/auth` (`platform_auth`) — no auth logic is -reimplemented here. +`GET /v1/resolve/discord/{github_login}`. Rate-limited (60 req/min/key, plus a +per-IP flood guard) and audit-logged. Every caller must present a key issued by +`gateway-keys` — there is no env-bootstrap admin path. Built on `packages/auth` +(`platform_auth`) — no auth logic is reimplemented here. **Not implemented (by design):** @@ -174,3 +242,7 @@ reimplemented here. team-tracking's surface. - **Multi-replica rate limiting** — the in-memory limiter is correct for a single Railway replica; a shared store (Redis) would be needed to scale horizontally. + +**Known constraint:** GitHub logins resolve case-sensitively, because that is how +team-tracking matches identifiers. See "Two things to know about this endpoint" +above — the fix belongs upstream, not here. From 65a85e8254dab40b554220333366aa6a0bf64340 Mon Sep 17 00:00:00 2001 From: Ethan Qiu Date: Sun, 16 Aug 2026 20:34:37 -0400 Subject: [PATCH 17/17] chore: give services/gateway its own ownership zone (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit label-consistency fails a services/ directory that has no zone of its own, because it falls through to the services/* catch-all — the bucket that hid the August 2026 drift, where several services shared one zone and a PR spanning two of them looked single-zone. The gateway is a new services/ member and this machinery landed after this branch forked, so it arrived unregistered. All five hand-maintained lists, per docs/CODE-OWNERSHIP.md: zone_for() in pr-zone-check.yml (canonical), CODEOWNERS, labeler.yml (the zone key *and* its negation in the services/other catch-all, or the zone gets two labels), the PR template, and the table in CODE-OWNERSHIP.md. Its CI job is already in this branch. `node scripts/check-labels.mjs` passes. Still needs `gh label create "zone: services/gateway" --color BFD4F2` — nothing checks that the label exists, and an uncreated one is created on first use in a random colour. Co-Authored-By: Claude Opus 5 --- .github/CODEOWNERS | 1 + .github/PULL_REQUEST_TEMPLATE.md | 2 +- .github/labeler.yml | 6 ++++++ .github/workflows/pr-zone-check.yml | 1 + docs/CODE-OWNERSHIP.md | 1 + 5 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 6bb0899..629c183 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -48,6 +48,7 @@ /packages/auth/ @qiuethan /services/connectors/ @qiuethan /services/documentation-system/ @qiuethan +/services/gateway/ @qiuethan /services/llm/ @qiuethan /services/meeting/ @qiuethan /services/team-tracking/ @qiuethan diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 3a5532a..d74b0d2 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -30,7 +30,7 @@ label-consistency.yml fails the build if you miss one — docs/CODE-OWNERSHIP.md Full list, and what each zone covers: docs/CODE-OWNERSHIP.md --> -`discord-bot` · `packages/auth` · `services/connectors` · `services/documentation-system` · `services/llm` · `services/meeting` · `services/team-tracking` · `services/verification` · `docs` · `scripts` · `.github` · `root` +`discord-bot` · `packages/auth` · `services/connectors` · `services/documentation-system` · `services/gateway` · `services/llm` · `services/meeting` · `services/team-tracking` · `services/verification` · `docs` · `scripts` · `.github` · `root` diff --git a/.github/labeler.yml b/.github/labeler.yml index a7573ac..20be217 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -44,6 +44,11 @@ - any-glob-to-any-file: - services/documentation-system/** +"zone: services/gateway": + - changed-files: + - any-glob-to-any-file: + - services/gateway/** + "zone: services/llm": - changed-files: - any-glob-to-any-file: @@ -74,6 +79,7 @@ - services/** - "!services/connectors/**" - "!services/documentation-system/**" + - "!services/gateway/**" - "!services/llm/**" - "!services/meeting/**" - "!services/team-tracking/**" diff --git a/.github/workflows/pr-zone-check.yml b/.github/workflows/pr-zone-check.yml index 97a335f..5450c17 100644 --- a/.github/workflows/pr-zone-check.yml +++ b/.github/workflows/pr-zone-check.yml @@ -41,6 +41,7 @@ jobs: packages/*) echo packages/other ;; services/connectors/*) echo services/connectors ;; services/documentation-system/*) echo services/documentation-system ;; + services/gateway/*) echo services/gateway ;; services/llm/*) echo services/llm ;; services/meeting/*) echo services/meeting ;; services/team-tracking/*) echo services/team-tracking ;; diff --git a/docs/CODE-OWNERSHIP.md b/docs/CODE-OWNERSHIP.md index 068618f..3693cda 100644 --- a/docs/CODE-OWNERSHIP.md +++ b/docs/CODE-OWNERSHIP.md @@ -24,6 +24,7 @@ Fourteen buckets. Every tracked file lands in exactly one. | `packages/other` | `packages/*` | `/packages/` | @qiuethan | | `services/connectors` | `services/connectors/*` | `/services/connectors/` | @qiuethan | | `services/documentation-system` | `services/documentation-system/*` | `/services/documentation-system/` | @qiuethan | +| `services/gateway` | `services/gateway/*` | `/services/gateway/` | @qiuethan | | `services/llm` | `services/llm/*` | `/services/llm/` | @qiuethan | | `services/meeting` | `services/meeting/*` | `/services/meeting/` | @qiuethan | | `services/team-tracking` | `services/team-tracking/*` | `/services/team-tracking/` | @qiuethan |