Explorar o código

test(release-v1): Phase 3 — HTTP-level integration test skeleton

audit/AUDIT_2026-06-05.md critical #6 baseline:

  $ grep -rln "TestClient\|httpx" tests/
  (no matches)

3,477 LOC of FastAPI route handlers had ZERO route-level coverage. A
`tenant_id` filter typo could ship and CI would still be green. This
file is the bootstrap skeleton — covers the *most expensive* failure
modes (auth bypass, cross-tenant read, loopback guard) so the framework
is in place; subsequent PRs fan out per-route happy + sad paths from here.

## 7 tests, all passing locally (Python 3.11)

  test_setup_health_no_auth                  ✓
  test_list_agents_requires_auth             ✓  (401 on missing auth)
  test_list_agents_rejects_bad_key           ✓  (401 on invalid Bearer)
  test_list_agents_accepts_valid_key         ✓  (200 happy path)
  test_bootstrap_remote_caller_hidden        ✓  ← Phase 0 loopback guard
                                                  verified via TestClient
                                                  (host = "testclient" ≠
                                                  loopback → 404, never
                                                  leaks admin key)
  test_cross_tenant_get_agent_blocked        ✓  ← audit #14/#30 防 IDOR
  test_cross_tenant_list_does_not_include    ✓  ← list scoping verified
    _other_tenant

## Fixture design

  fixture `db`            — fresh sqlite:///:memory: per test, swap into
                            agentpaas.db.session._db singleton so both
                            Depends(get_database) AND direct get_db()
                            calls inside authenticate() see the same DB.
  fixture `client`        — TestClient(app) bound to the isolated DB.
  fixture `tenant_and_key`— insert tenant + user + admin api_key,
                            return (tenant_id, raw_key).

  Helper `_insert_tenant_with_agent(db, tname, aname)` keeps cross-tenant
  setup tight.

## Side effect: fixed a Database init order bug

In writing the fixture, hit `sqlite3.OperationalError: no such table:
knowledge_bases` on a fresh `:memory:` DB. Cause: the `_migrate` loop at
models.py:217 ran BEFORE `_ensure_table("knowledge_bases", ...)` at :257,
so the migration ALTERs a table that doesn't yet exist. Production hid
this because the table persists across boots.

Fix: pre-fetch sqlite_master table list and skip migrations whose target
table doesn't yet exist — `_ensure_table` lazy-creates them with the new
column already in its CREATE statement, so nothing is lost.

## CI update

`.github/workflows/ci.yml` now installs `agentpaas/[dev]` instead of
just `agentpaas/`. The [dev] extra pulls in httpx — required by
fastapi.testclient.TestClient.

## What's covered now vs. before

  Before: lambdagent kernel tests (175 passing) but ZERO route-level
          tests. Auth, tenant scoping, loopback guard — all structurally
          untested.
  After:  7 integration tests. Auth dependency proven, tenant scoping
          proven on GET-by-id + LIST, loopback guard proven on
          /bootstrap. Critical #6 reduced from "completely untested"
          to "skeleton in place, gaps named in future PRs".

## What's NOT covered yet (deferred to follow-up PRs)

  - POST/PUT/DELETE on /agents (create → rollback round-trip)
  - Knowledge endpoint full CRUD + tenant scoping
  - RBAC denial paths (read-only key trying agents:write)
  - rollback_agent (audit #14 specific repro test)
  - Rate limiting behavior under burst

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kenny67nju hai 4 meses
pai
achega
7d629b1686
Modificáronse 3 ficheiros con 271 adicións e 2 borrados
  1. 4 2
      .github/workflows/ci.yml
  2. 16 0
      agentpaas/src/agentpaas/db/models.py
  3. 251 0
      tests/test_api_endpoints.py

+ 4 - 2
.github/workflows/ci.yml

@@ -41,10 +41,12 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
           cache: pip
-      - name: Install lambdagent (src-layout) + agentpaas
+      - name: Install lambdagent (src-layout) + agentpaas[dev]
         run: |
           pip install -e lambdagent/
-          pip install -e agentpaas/   # agentpaas + fastapi/uvicorn/pydantic 依赖
+          # [dev] pulls in httpx — required by fastapi.testclient.TestClient
+          # used by tests/test_api_endpoints.py (audit critical #6 coverage).
+          pip install -e "agentpaas/[dev]"
           pip install pytest pytest-asyncio
       - name: Run tests
         run: python -m pytest tests/ -v --tb=short

+ 16 - 0
agentpaas/src/agentpaas/db/models.py

@@ -214,7 +214,23 @@ class Database:
             # the run existed.
             ("runs", "subprocess_pid", "ALTER TABLE runs ADD COLUMN subprocess_pid INTEGER"),
         ]
+        # Discovered while writing test_api_endpoints.py against a fresh
+        # `:memory:` DB: `knowledge_bases` is created by `_ensure_table`
+        # later in this function, so the probe and the ALTER both fire
+        # against a non-existent table on first init → unhandled
+        # OperationalError. Production hides this because the table
+        # exists from a prior boot. Defensive fix: skip a migration when
+        # its target table is missing — `_ensure_table` will lazy-create
+        # the table next, with the new column already in its CREATE
+        # statement.
+        existing_tables = {
+            row[0] for row in self.conn.execute(
+                "SELECT name FROM sqlite_master WHERE type='table'"
+            ).fetchall()
+        }
         for table, column, sql in migrations:
+            if table not in existing_tables:
+                continue
             try:
                 self.conn.execute(f"SELECT {column} FROM {table} LIMIT 0")
             except sqlite3.OperationalError:

+ 251 - 0
tests/test_api_endpoints.py

@@ -0,0 +1,251 @@
+"""
+HTTP-level integration tests for the agentpaas FastAPI surface.
+
+audit/AUDIT_2026-06-05.md critical #6 baseline:
+
+  $ grep -rln "TestClient\|httpx" tests/
+  (no matches)
+
+3,477 LOC of route handlers had ZERO route-level coverage before this
+file existed. A `tenant_id` filter typo could ship to production and
+CI would still be green. This file is the bootstrap skeleton — it
+covers the *most expensive* failure modes (auth bypass, cross-tenant
+read, loopback guard) so the framework is in place; subsequent PRs
+should fan out per-route happy + sad paths from here.
+
+Conventions for new tests:
+  - Use the `client` fixture (TestClient bound to an isolated in-memory
+    SQLite DB).
+  - Use `tenant_and_key` for an authenticated request.
+  - For cross-tenant tests, build the second tenant inline via the `db`
+    fixture exposed alongside.
+  - Assertions favor *what must not happen* over *exact response shape*;
+    we are testing security boundaries, not API ergonomics.
+"""
+from __future__ import annotations
+
+# IMPORTANT: the env var has to be set BEFORE agentpaas.config is imported,
+# because settings.database_url is read at module-import time. conftest.py
+# runs first (Python imports), and *this* module's top-level os.environ
+# call lands before the `from agentpaas...` lines below.
+import os
+os.environ.setdefault("AGENTPAAS_DATABASE_URL", "sqlite:///:memory:")
+
+import json
+import secrets
+
+import pytest
+
+try:
+    from fastapi.testclient import TestClient
+except ImportError:  # httpx not installed — skip entire file gracefully
+    pytest.skip(
+        "fastapi.testclient requires httpx — `pip install -e ./agentpaas/[dev]`",
+        allow_module_level=True,
+    )
+
+from agentpaas.api.app import app
+from agentpaas.api.middleware.auth import hash_key
+from agentpaas.db.models import Database, gen_id, now_utc
+import agentpaas.db.session as _session_mod
+
+
+# ─────────────────────────────────────────────────────────────────────────────
+# Fixtures
+# ─────────────────────────────────────────────────────────────────────────────
+
+@pytest.fixture
+def db():
+    """Fresh in-memory SQLite per test. Database.__init__ re-runs all
+    migrations + the zombie-reaper migration; a brand-new `:memory:` DB
+    is guaranteed clean. We swap the module-level singleton in
+    `agentpaas.db.session` so both `Depends(get_database)` AND the
+    direct `get_db()` calls inside `authenticate()` see the same DB."""
+    prev = _session_mod._db
+    _session_mod._db = Database("sqlite:///:memory:")
+    try:
+        yield _session_mod._db
+    finally:
+        _session_mod._db = prev
+
+
+@pytest.fixture
+def client(db):
+    """TestClient bound to the isolated DB. We don't override any
+    dependencies — by patching `_session_mod._db`, every code path that
+    calls `get_db()` (Depends, or direct) hits the test DB."""
+    with TestClient(app) as c:
+        yield c
+
+
+@pytest.fixture
+def tenant_and_key(db):
+    """Insert a tenant + a user + an active admin-scope API key.
+    Returns (tenant_id, raw_api_key). The raw key is shown ONCE — same
+    contract as production /bootstrap."""
+    tid = gen_id("tn_")
+    uid = gen_id("usr_")
+    key = f"ap_{secrets.token_hex(16)}"
+    now = now_utc()
+
+    db.execute(
+        "INSERT INTO tenants (id, name, plan, status, created_at) "
+        "VALUES (?, ?, 'free', 'active', ?)",
+        (tid, "test-tenant", now),
+    )
+    db.execute(
+        "INSERT INTO users (id, tenant_id, email, role, created_at) "
+        "VALUES (?, ?, '', 'admin', ?)",
+        (uid, tid, now),
+    )
+    db.execute(
+        "INSERT INTO api_keys "
+        "(id, tenant_id, user_id, key_hash, key_prefix, name, scopes, "
+        " rate_limit, status, created_at) "
+        "VALUES (?, ?, ?, ?, ?, 'test-key', ?, 600, 'active', ?)",
+        (
+            gen_id("key_"), tid, uid, hash_key(key), key[:8],
+            json.dumps(["agents:*", "keys:*", "admin:*"]), now,
+        ),
+    )
+    db.commit()
+    return tid, key
+
+
+def _auth(key: str) -> dict:
+    return {"Authorization": f"Bearer {key}"}
+
+
+# ─────────────────────────────────────────────────────────────────────────────
+# Health / no-auth surface
+# ─────────────────────────────────────────────────────────────────────────────
+
+def test_setup_health_no_auth(client):
+    """SetupWizard polls /api/v1/setup/health before bootstrap — must be
+    reachable without a key, and must not leak DB state."""
+    r = client.get("/api/v1/setup/health")
+    assert r.status_code == 200
+    body = r.json()
+    assert body.get("ok") is True
+    # Should not include things like raw env vars / paths / tenant counts
+    assert "secret" not in str(body).lower()
+    assert "password" not in str(body).lower()
+
+
+# ─────────────────────────────────────────────────────────────────────────────
+# Auth dependency — critical surface, was untested
+# ─────────────────────────────────────────────────────────────────────────────
+
+def test_list_agents_requires_auth(client):
+    """No Authorization header → 401. audit #14 root cause: handlers
+    without `Depends(get_tenant)` slip through this gate silently;
+    this test enforces it on /agents."""
+    r = client.get("/api/v1/agents")
+    assert r.status_code == 401, (
+        f"Expected 401 for missing auth, got {r.status_code}: {r.text[:300]}"
+    )
+
+
+def test_list_agents_rejects_bad_key(client):
+    """Garbage Bearer → 401, not 500. Catches both 'key absent in DB'
+    and 'authenticate() raised unhandled' regressions."""
+    r = client.get("/api/v1/agents", headers=_auth("ap_doesnotexist_0000000000000000"))
+    assert r.status_code == 401, (
+        f"Expected 401 for invalid key, got {r.status_code}: {r.text[:300]}"
+    )
+
+
+def test_list_agents_accepts_valid_key(client, tenant_and_key):
+    """Happy path: real key → 200 + list payload. Anchors the
+    authenticate() success path; without this, the legacy-hash auto-
+    migration logic (audit SEC-01 leftover at api/middleware/auth.py
+    around _hash_key_legacy) is structurally untested."""
+    _, key = tenant_and_key
+    r = client.get("/api/v1/agents", headers=_auth(key))
+    assert r.status_code == 200, r.text[:300]
+    body = r.json()
+    # tolerant: response may be a bare list or {"items": [...]} or {"agents": [...]}
+    items = body if isinstance(body, list) else (
+        body.get("items") or body.get("agents") or []
+    )
+    assert isinstance(items, list)
+
+
+# ─────────────────────────────────────────────────────────────────────────────
+# Phase 0 verification — loopback guard on /bootstrap
+# ─────────────────────────────────────────────────────────────────────────────
+
+def test_bootstrap_remote_caller_hidden(client):
+    """audit critical #1 fix (commit 9a3f22b): /bootstrap from a non-
+    loopback client returns 404, never 200-with-admin-key.
+
+    `TestClient`'s default client host is 'testclient', not '127.0.0.1'
+    (https://www.starlette.io/testclient — the host is fabricated). So
+    this test runs against the same code path a remote attacker would
+    hit, and our `_require_loopback` guard should refuse."""
+    r = client.post("/api/v1/setup/bootstrap")
+    assert r.status_code == 404, (
+        f"/bootstrap leaked from non-loopback host! "
+        f"status={r.status_code}, body={r.text[:300]}"
+    )
+
+
+# ─────────────────────────────────────────────────────────────────────────────
+# Multi-tenant isolation — audit critical/high category
+# ─────────────────────────────────────────────────────────────────────────────
+
+def _insert_tenant_with_agent(db, tenant_name: str, agent_name: str) -> tuple[str, str]:
+    """Helper: drop in a tenant + a single agent row. Returns (tid, aid)."""
+    tid = gen_id("tn_")
+    aid = gen_id("ag_")
+    now = now_utc()
+    db.execute(
+        "INSERT INTO tenants (id, name, plan, status, created_at) "
+        "VALUES (?, ?, 'free', 'active', ?)",
+        (tid, tenant_name, now),
+    )
+    db.execute(
+        "INSERT INTO agents "
+        "(id, tenant_id, name, description, status, created_at, updated_at) "
+        "VALUES (?, ?, ?, '', 'active', ?, ?)",
+        (aid, tid, agent_name, now, now),
+    )
+    db.commit()
+    return tid, aid
+
+
+def test_cross_tenant_get_agent_blocked(client, db, tenant_and_key):
+    """Tenant A's key must NOT be able to GET tenant B's agent.
+
+    audit #14 + #30: the `agents` and KB endpoints largely DO scope by
+    `tenant_id`, but a single missing `AND tenant_id = ?` in any WHERE
+    clause leaks cross-tenant data. Without this test the regression is
+    invisible to CI."""
+    _, key_a = tenant_and_key
+    _, aid_b = _insert_tenant_with_agent(db, "tenant-b", "tenant-b-private")
+
+    r = client.get(f"/api/v1/agents/{aid_b}", headers=_auth(key_a))
+    # 403 (you're not allowed) OR 404 (we pretend it doesn't exist) — both
+    # acceptable defenses; only 200 is a security regression.
+    assert r.status_code in (403, 404), (
+        f"Cross-tenant read leaked! status={r.status_code}, body={r.text[:300]}"
+    )
+
+
+def test_cross_tenant_list_does_not_include_other_tenant(client, db, tenant_and_key):
+    """Tenant A's /agents list must not include tenant B's rows.
+    Catches the easier 'forgot WHERE tenant_id=?' typo on the list
+    endpoint."""
+    _, key_a = tenant_and_key
+    _, aid_b = _insert_tenant_with_agent(db, "tenant-b", "tenant-b-private")
+
+    r = client.get("/api/v1/agents", headers=_auth(key_a))
+    assert r.status_code == 200, r.text[:300]
+    body = r.json()
+    items = body if isinstance(body, list) else (
+        body.get("items") or body.get("agents") or []
+    )
+    leaked = [a for a in items if a.get("id") == aid_b]
+    assert not leaked, (
+        f"Tenant A's list leaked tenant B's agent! Got: {leaked}"
+    )