diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a8e0f8e86..4d81791ae 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -18,6 +18,180 @@ entry. See `CONTRIBUTING.md` § Releases & changelog. ## [Unreleased] +### Added — API keys as a first-class authentication method, with per-key scopes (#439) + +- New workspace package `@omadia/api-key-auth` + (`middleware/packages/harness-api-key-auth/`). The API-key primitives + #438 shipped inside `@omadia/channel-api` — mint/sha256-hash/constant-time + verify, the vault-backed key store, the per-key rate limiter, the usage + audit log — moved here unchanged, so there is exactly **one** + implementation of the credential. A shared workspace package is the only + home both sides can reach: the kernel must never import a channel plugin, + and a plugin cannot import kernel source (`middleware/src/auth/` is not + resolvable from a package whose `tsconfig` has `rootDir: src`). Same role + `@omadia/plugin-api` and `@omadia/channel-sdk` already play. The package is + dependency-free apart from an `express` peer — its storage dependency is a + structural subset (`ApiKeySecretStorage`) that `SecretsAccessor` satisfies + without an adapter. No new npm dependencies, matching #438. +- New mountable Express middleware `requireApiKey({ apiKeys, rateLimiter, + auditLog, scope })`: any route or plugin can apply it and be authenticated + by a server-to-server bearer key instead of the `omadia_session` cookie + (driving use case: a Laravel/PHP integration with no human session behind + it). It attaches an `ApiKeyPrincipal` to `req.apiKey` and deliberately does + **not** populate `req.session` — `SessionClaims.role` is hard-typed + `'admin'`, so synthesizing a session for a machine would make every + session-reading route downstream silently treat a key as an operator. + 401/403/429 use the `{ error, message }` shape #438 established for the + public API surface, not the session gate's `{ code, message }`, so the wire + format of `POST /api/public/v1/chat` is unchanged. +- Per-key **scopes**: `:` strings (or the global `*`), + matched exactly — no prefix wildcards, which are how "I thought `admin:*` + didn't cover `admin:delete`" happens. A route declares the scope it needs; + a key without it gets `403 forbidden` and a `forbidden` audit entry. + Backward compatible: a key persisted before scopes existed carries no + `scopes` field and is normalized to `['chat:write']` — exactly the one + capability it had when it was minted. Defaulting such keys to `*` would + also keep them working and would silently widen every existing key to + whatever scoped surface lands next, so it is not what we do. + `POST /api/public/v1/admin/keys` accepts a `scopes` array (validated, 400 + on a malformed scope) and `GET` lists it. +- `normalizeScopes` distinguishes an **absent** `scopes` field from a + **malformed** one, because collapsing the two turns a read error into a + capability grant. Absent (`undefined`) → the legacy `['chat:write']`. + Present but unreadable — not an array (`"memory:read"` stored as a bare + string), an empty array, or an array with any invalid entry + (`['Chat:Write']`, `['chat:write','nonsense']`) → the **empty** scope set: + the key still authenticates, and every scope check on it fails closed with + `403`. A malformed record is at least as likely to be a key an operator + deliberately restricted *away* from chat as it is to be a lost pre-#439 + key, and defaulting it to `chat:write` would hand back exactly the access + that was removed. Partially-valid arrays deny rather than silently narrow. + Every such case logs `[api-key-auth] malformed persisted scopes` so an + operator can tell a corrupt record from a revoked key. The scope set is + always persisted explicitly at `create()` time, so nothing this store + writes can be mistaken for a pre-#439 record. +- Creation agrees with that read path on the same value. Only an **omitted** + `scopes` field resolves to the legacy default; an explicitly supplied `[]` + is rejected — `400` at the admin route, and a throw from `create()` for + callers using the package directly. Otherwise one field would mean "deny + everything" on read and "grant `chat:write`" on write, so an operator asking + for a zero-capability key would have been handed a chat-capable one. +- `@omadia/channel-api` now consumes the shared package instead of owning + the code: `chatRouter.ts` mounts `requireApiKey` with `scope: 'chat:write'` + rather than parsing bearer headers itself. Behaviour and wire format of + `POST /api/public/v1/chat` are unchanged, and its existing test suite + passes as written (only the moved modules' import paths were repointed). +- `middleware/src/auth/publicPaths.ts` is deliberately **not** broadened — + `/api/public/v1/chat` is still the only exempted API-key route. Its comment + now records what a future route that mounts `requireApiKey` has to do. +- The `scopes` additions to `/api/public/v1/admin/keys` sit **on top of** the + kernel-level `ctx.operatorAuth` session gate that the entry below adds to + that router, not beside it: an anonymous `POST` carrying `scopes: ['*']` + is rejected `401` before any handler runs, covered by its own regression + test in `adminKeysRouter.test.ts`. +- Tests: `test/auth/requireApiKey.test.ts`, `test/auth/apiKeyScopes.test.ts`, + and `test/channelApi/apiKeyAuthReuseSeam.test.ts` — the last one is a + structural guard on the seam itself (the plugin holds no second copy of the + primitives, and `middleware/src` imports no channel plugin), because + "where does this code live" is a property no runtime assertion can express + and the cheapest one to regress. + +### Added — public API channel: chat over HTTP with per-key auth (#438) + +- New built-in channel package `@omadia/channel-api` + (`middleware/packages/harness-channel-api/`) exposes `POST + /api/public/v1/chat` — a documented, self-authenticating HTTP entry point + external systems can drive without a channel adapter or the operator UI. + Streams the SAME NDJSON event framing as `/chat/stream` and dispatches + through `CoreApi.handleTurnStream`, so PII masking (privacy-guard), memory, + and the knowledge graph all apply exactly as they do for every other + channel — no second response-masking path. +- Credential model (locked design decision on the issue): each API key **is** + its own identity — `ChannelUserRef{ channel: 'api', id: 'key:' }` — + not a delegate for a human end-user. No impersonation surface. +- Full v1 security posture, not deferred: API keys are vault-backed (this + plugin's own `ctx.secrets` namespace, no DB migration) and verified with + `crypto.timingSafeEqual` against a sha256 hash — the plaintext is shown + exactly once, at creation; per-key configurable rate limits (fixed-window, + 429 on overage); an explicit revoke endpoint (`POST + /api/public/v1/admin/keys/:id/revoke`) that fails the next request + immediately; and a usage audit log (who/what/when) recorded on every + authenticated call. +- Key lifecycle (`GET`/`POST /api/public/v1/admin/keys`, revoke) is + deliberately mounted under the SAME `/api/public/v1` prefix but NOT added + to `middleware/src/auth/publicPaths.ts`'s exemption list — only `.../chat` + is public. Key management stays behind the normal operator session, like + every other admin surface in this app — see the security-fixup entry below + for how that's enforced both implicitly (the kernel's broad `/api` + session gate) and, after that entry's change, explicitly as well. +- Review fixups: the internal `conversationId` handed to `CoreApi` is now + namespaced by key id (`${key.id}:${callerConversationId}`) so two + different API keys can never collide on the same core-side scope, even + when they send an identical caller-supplied `conversationId` — closes a + cross-key transcript/context leak. The usage audit log now records one + entry for every authenticated call (not just the success path) with a + status reflecting the real outcome — `ok` | `rate_limited` | + `invalid_request` | `error` — instead of writing `status: 'ok'` + optimistically before dispatch. +- Second review fixup round: the audit-status fix above still had a gap — + `deps.core.handleTurnStream` can yield an in-band `{type:'error', + message}` event on the already-open stream WITHOUT throwing (same bug + class as #403), and the loop completing normally was still recorded as + `ok`. `chatRouter.ts` now tracks whether an `error`-type event was + forwarded during iteration and audits `error` in that case too, with a + regression test covering the no-throw path. Docs: the README's event + table no longer claims `agent_bound` is emitted on this route (it isn't — + that event is synthesized by the kernel's own `/api/chat/stream` handler, + not by `CoreApi.handleTurnStream`) and now documents the verifier-mode + `{type:'verifier'}` event that can follow `done`. `docs/security-architecture.md` + § 8 and the README's rate-limiting section now say explicitly that the + limiter is in-memory and per-process, not shared across replicas + (accepted v1 trade-off, no code change). `harness-channel-api`'s + `peerDependencies` on `@omadia/channel-sdk` / `@omadia/plugin-api` are now + pinned to `^0.1.0` instead of `"*"`, per `CONTRIBUTING.md`'s dependency + hardening policy. +- Security fixup: an earlier note here overstated this as a live + authentication bypass. It wasn't — `/api/public/v1/admin/keys` was + already covered by the kernel's pre-existing broad `app.use('/api', + requireAuth, ...)` mount (`src/index.ts`), which runs ahead of + `pluginRouteRegistry.mountAll(app)` in boot order and gates every + `/api/*` path not listed in `publicPaths.ts`, same as any other + non-exempted channel route. That coverage is real but implicit — it + depends on mount order and on this path never being added to + `publicPaths.ts`, either of which a future refactor could break silently. + Hardened at the kernel level so the guarantee doesn't depend on that + coincidence, and so future plugins needing an admin surface get a + reusable, explicit check: `PluginContext` gains an optional `ctx.operatorAuth` + (`OperatorAuthAccessor`, `packages/plugin-api/src/pluginContext.ts`), + published by the kernel and threaded into every plugin runtime + (`ToolPluginRuntime`, `DynamicAgentRuntime`, `DefaultChannelRegistry`) so + any future plugin needing an operator-only admin surface can reuse it. + `hasValidSession(cookieHeader)` reuses the EXACT SAME session-verification + logic `requireAuth` runs (extracted to `evaluateSessionToken` in + `src/auth/requireAuth.ts`) — one code path, not two that can drift apart. + `adminKeysRouter.ts` now applies it as router-level middleware ahead of + every route: missing/invalid session → `401`; `ctx.operatorAuth` itself + unavailable → `503` (fail closed, never silently unauthenticated). New + end-to-end coverage in `adminKeysRouter.test.ts` mounts the router behind + the REAL accessor (not a stub) and asserts the no-cookie / invalid-cookie + / valid-cookie and fail-closed paths. `docs/security-architecture.md` § 9, + this package's `README.md`, and `docs/middleware-agent-handoff.md` are + corrected to describe the real mechanism. +- Third review fixup: the key-id namespacing above (`${key.id}:${callerConversationId}`) + was itself still lossy. `SessionLogger`'s `sanitizeScope` collapses any run + of punctuation to a single `-`, lowercases, and truncates to 80 chars + before persisting — so two DIFFERENT caller-supplied `conversationId`s + under the SAME key could still land on the identical sanitized scope (for + example `"case/a"` and `"case?a"`, or two long ids differing only past the + truncation cutoff), letting one conversation thread recall another + thread's memory/graph content. `chatRouter.ts` now derives the internal + `conversationId` as `sha256(key.id:callerConversationId)` (hex digest — + fixed-width, already lowercase alphanumeric, so nothing about it can be + mangled or truncated into colliding with a different digest) instead of + plain concatenation. Regression coverage in `chatRouter.test.ts` sends + both collision shapes through the real `createApiChatRouter` and asserts + the resulting scopes differ after being run through the real + `graphScopeFor`/`sanitizeScope`. ### Added — pluggable embedding provider (#440) - The `EmbeddingClient` contract moved from `@omadia/embeddings` (the Ollama diff --git a/docs/creating-plugins.md b/docs/creating-plugins.md index 00de79ff1..a9bdc348f 100644 --- a/docs/creating-plugins.md +++ b/docs/creating-plugins.md @@ -456,6 +456,7 @@ lokal und startet dann den normalen Install-Job. - Admin-UI-Constraints: `middleware/assets/boilerplate/agent-integration/assets/admin-ui/CLAUDE.md` - Kanonischer Agent: `middleware/packages/agent-seo-analyst/` - Channel-SDK: `middleware/packages/harness-channel-sdk/src/` (`@omadia/channel-sdk`) — inkl. `getChatAgent(ctx)` +- API-Key-Auth (server-to-server Bearer statt Session-Cookie): `middleware/packages/harness-api-key-auth/src/` (`@omadia/api-key-auth`) — `requireApiKey(...)` als mountbare Express-Middleware, inkl. Key-Store, Scopes, Rate-Limit, Audit-Log - Öffentliches Channel-Referenz-Plugin: `byte5ai/omadia-channel-whatsapp` - Runtime-Contract: `middleware/packages/plugin-api/src/pluginContext.ts` (`@omadia/plugin-api`) - Manifest-Schema (inkl. `channel:`-Block §14): `docs/harness-platform/manifest-schema.v1.yaml` diff --git a/docs/middleware-agent-handoff.md b/docs/middleware-agent-handoff.md index 535d0f902..21b743c69 100644 --- a/docs/middleware-agent-handoff.md +++ b/docs/middleware-agent-handoff.md @@ -835,6 +835,148 @@ die verbleibenden Phasen: `specs/470-dev-platform-plugin/plan.md`. --- +### Public API Channel (issue #438) + +Neues Built-in-Channel-Plugin `packages/harness-channel-api/` +(`@omadia/channel-api`, `kind: channel`), erster nicht-Session-Cookie-Ingress +für externe Systeme: **`POST /api/public/v1/chat`** treibt einen Turn genau +wie jeder andere Channel — über `core.registerRouter` + +`CoreApi.handleTurnStream` —, authentifiziert aber per **API-Key** +(`Authorization: Bearer omk_…`) statt Session-Cookie. NDJSON-Framing +identisch zu `/chat/stream` (`src/routes/chat.ts`); da der Turn über +`CoreApi.handleTurnStream` läuft, greifen PII-Masking (Privacy-Guard), +Memory und Knowledge-Graph unverändert — **kein zweiter Masking-Pfad**. + +- **Credential-Modell** (geklärte Design-Entscheidung im Issue): ein API-Key + **ist** seine eigene Identität — `ChannelUserRef{ kind: 'custom', id: + 'key:' }` —, kein Delegat für einen menschlichen Endnutzer. Keine + Impersonation-Fläche. +- **Storage:** vault-backed über `ctx.secrets` (eigener Plugin-Namespace, + `permissions.secrets.runtime_write: true`) — kein DB-Migration nötig. Nur + der sha256-Hash landet im Vault; der Klartext-Key wird genau einmal beim + `create()` zurückgegeben (`packages/harness-channel-api/src/apiKeyToken.ts`, + spiegelt `src/devplatform/jobToken.ts`s Mint/Hash/Verify-Muster — + `crypto.timingSafeEqual`, kein früh-abbrechender String-Vergleich). +- **Rate-Limiting:** Fixed-Window-Token-Bucket pro Key + (`rateLimiter.ts`, spiegelt `platform/httpAccessor.ts`s `TokenBucket`), + Kapazität pro Key konfigurierbar (`create({ rateLimitPerMinute })`), + Default 60/min. Über Budget → `429`. +- **Audit-Log:** jeder authentifizierte Call schreibt einen Eintrag + (`keyId`, `route`, `method`, `at`, `status`) — vault-backed, auf die + letzten `MAX_ENTRIES` (200) gedeckelt, Writes seriell über eine interne + Promise-Queue (`auditLog.ts`). +- **Key-Lifecycle** (`GET`/`POST /api/public/v1/admin/keys`, `POST + /api/public/v1/admin/keys/:id/revoke`) liegt bewusst unter demselben + `/api/public/v1`-Prefix, ist aber **nicht** in + `src/auth/publicPaths.ts`s Exemption-Liste — nur `.../chat` ist public. + Ein früherer Stand dieser Notiz behauptete, das sei ein kompletter + Auth-Bypass gewesen (jeder anonyme Caller könnte Keys minten/listen/ + revoken); das war empirisch falsch. `src/index.ts` mountet früh im Boot + `app.use('/api', requireAuth, createChatRouter(...))` (der OB-106-Hotfix) + — lange bevor `pluginRouteRegistry.mountAll(app)` später im selben Boot + läuft. Express wertet Middleware in Mount-Reihenfolge für den gesamten + `/api`-Prefix aus, unabhängig davon, welcher Router den Pfad am Ende + bedient — `requireAuth` lief also bereits vor JEDEM `/api/*`-Request, + auch plugin-gemounteten, außer der Pfad steht in + `publicPaths.ts`. `/api/public/v1/admin/keys` stand dort nie, war also + schon durch dieses Gate geschützt — genau wie jede andere + nicht-exemptierte Channel-Route. Eine Minimal-Reproduktion mit dem + echten Mount-Order (echtes `createRequireAuth` + `publicPaths`) bestätigt: + ein anonymer Request auf `/api/public/v1/admin/keys` bekommt `401 + {code:'auth.missing'}` von diesem Gate, bevor er überhaupt den + Plugin-Router (der selbst keine eigene Auth hat, da `core.registerRouter` + nur active/inactive prüft) erreicht. + + Diese Absicherung ist real, aber implizit — sie hängt an der Mount- + Reihenfolge und daran, dass der Pfad nie in `publicPaths.ts` landet. + Beides kann ein künftiger Refactor versehentlich brechen, ohne dass + etwas sichtbar fehlschlägt. Deshalb der reale Fix (Kernel-Ebene, + Security-Nachbesserung), der die Absicherung explizit statt implizit + macht: `PluginContext` bekommt ein optionales `ctx.operatorAuth` + (`OperatorAuthAccessor`), vom Kernel published und in jede + Plugin-Runtime durchgereicht (`ToolPluginRuntime`, `DynamicAgentRuntime`, + `DefaultChannelRegistry`). `hasValidSession(cookieHeader)` nutzt exakt + dieselbe Verifikationslogik wie `requireAuth` + (`evaluateSessionToken` in `src/auth/requireAuth.ts`) — ein Code-Pfad, + keine zwei, die auseinanderlaufen können. `adminKeysRouter.ts` wendet das + jetzt als Router-Middleware VOR jedem Handler an: fehlende/ungültige + Session → `401`; kein `ctx.operatorAuth` verfügbar → `503` (fail closed, + nie stillschweigend offen). Der Vorteil ist, dass die Garantie nicht mehr + an der Mount-Reihenfolge hängt und künftige Plugins mit Admin-Fläche den + Accessor wiederverwenden können, statt sich auf dieselbe Koinzidenz zu + verlassen. Siehe `docs/security-architecture.md` § 9 für die volle + Mechanik. +- **Scope:** nur `chat` in v1 (Issue #438 explizit: "Start with chat …, then + extend to other flows" — weitere Flows sind Folge-Issues). + +Tests: `test/channelApi/` — u.a. eine echte Orchestrator- + echte +Privacy-Guard-Integration (`chatRouterPrivacyIntegration.test.ts`, spiegelt +`test/orchestrator/promptMaskPipeline.test.ts`s "realer Turn, gefakter LLM"- +Muster), Auth/Rate-Limit/Revoke/Audit-Wiring (`chatRouter.test.ts`), Key-CRUD ++ die reale `ctx.operatorAuth`-Verifikation inkl. Fail-closed-Pfad +(`adminKeysRouter.test.ts`), und die `publicPaths`-Exemption +(`publicPathsExemption.test.ts`). + +--- + +### API-Keys als eigenständige Auth-Methode (issue #439) + +Issue #438 hatte die Bearer-Auth plugin-intern gebaut und genau **eine** Route +abgesichert. #439 macht daraus eine allgemeine Authentifizierungs-Methode +neben dem Session-Cookie — Zielfall: eine Laravel/PHP-Integration, die omadia +vom eigenen Server aus aufruft, ohne menschliche Session. + +- **Neues Workspace-Package `packages/harness-api-key-auth/` + (`@omadia/api-key-auth`).** `apiKeyToken.ts`, `apiKeyStore.ts`, + `rateLimiter.ts` und `auditLog.ts` sind aus `harness-channel-api/` + hierher gezogen; es gibt danach **genau eine** Implementierung von + Mint/Hash/Verify/Store. Warum ein Package und nicht `src/auth/`: der Kernel + darf nie aus einem Channel-Plugin importieren, und ein Plugin kann keinen + Kernel-Source importieren (eigenes `tsconfig` mit `rootDir: src`, Auflösung + ausschließlich über `@omadia/*`). Ein Workspace-Package ist die einzige + Stelle, die beide Richtungen bedient — dieselbe Rolle, die + `@omadia/plugin-api` und `@omadia/channel-sdk` schon spielen. + Das Package ist bewusst dependency-frei (nur `express` als Peer): die + Storage-Abhängigkeit ist ein strukturelles Subset (`ApiKeySecretStorage` in + `secretStorage.ts`), das `SecretsAccessor` ohne Adapter erfüllt. +- **`requireApiKey(...)`** (`requireApiKey.ts`) ist die mountbare + Express-Middleware: Bearer-Parsing → `verify()` → Rate-Limit → Scope-Check, + danach `req.apiKey: ApiKeyPrincipal`. Sie setzt **nicht** `req.session` — + `SessionClaims.role` ist hart `'admin'`, eine synthetische Session würde + jeden session-lesenden Downstream-Handler einen Key für einen Operator + halten lassen. Fehlerform `{ error, message }` wie in #438 (nicht + `{ code, message }` wie `createRequireAuth`), damit die Wire-Form von + `POST /api/public/v1/chat` unverändert bleibt. +- **Scopes** (`apiKeyScopes.ts`): `:` oder globales `*`, + exakter Match, keine Prefix-Wildcards. Keys ohne persistiertes `scopes`-Feld + (alles aus #438) werden auf `['chat:write']` normalisiert — genau die eine + Fähigkeit, die sie beim Minten hatten. `*` als Default wäre eine per Upgrade + ausgelieferte Rechteausweitung. Admin-Route nimmt `scopes` bei `POST` + entgegen (Zod-validiert → 400 statt 500) und zeigt sie im `GET`. + **`normalizeScopes` unterscheidet dabei *fehlend* von *kaputt*:** nur ein + komplett fehlendes Feld (`undefined`) bekommt den Legacy-Default; ein + vorhandenes, aber unlesbares Feld (kein Array, leeres Array, ungültige oder + teilweise ungültige Einträge wie `"memory:read"` als String oder + `['Chat:Write']`) ergibt die **leere** Scope-Menge — der Key + authentifiziert weiter, ist aber für nichts autorisiert, jeder + `hasScope`-Check schlägt fail-closed fehl. Beides in einen Grant zu + kollabieren würde einem Key, den ein Operator bewusst von Chat + weggeschnitten hat, genau diesen Chat-Zugriff zurückgeben. Jeder solche + Fall loggt eine `[api-key-auth] malformed persisted scopes`-Warnung. +- **`publicPaths.ts` bleibt unverändert eng:** weiterhin nur + `/api/public/v1/chat`. Wer `requireApiKey` auf eine neue Route mountet, + braucht dort einen eigenen, möglichst engen Eintrag. + +Tests: `test/auth/requireApiKey.test.ts` (Auth/Scope/Rate-Limit/Audit der +Middleware), `test/auth/apiKeyScopes.test.ts` (Scope-Modell inkl. +Legacy-Default), `test/channelApi/apiKeyAuthReuseSeam.test.ts` (strukturelle +Zusicherung, dass das Plugin keine zweite Kopie der Primitive hält und der +Kernel kein Channel-Plugin importiert). Die bestehenden `test/channelApi/`- +Suites laufen inhaltlich unverändert weiter, nur die Importpfade der +verschobenen Module zeigen jetzt auf `packages/harness-api-key-auth/`. + +--- + ## 4. Migration Managed Agents → Lokal ### Warum migriert diff --git a/docs/security-architecture.md b/docs/security-architecture.md index a0b9fd0f9..914adfb2a 100644 --- a/docs/security-architecture.md +++ b/docs/security-architecture.md @@ -212,7 +212,188 @@ At a minimum, your deployment vault holds: Nothing from this list should appear in `git grep` output of this repository. If it does, that is a bug — file an issue and rotate. -## 9. Reviewer checklist +## 9. API-key authentication (`@omadia/api-key-auth`, issues #438 / #439) + +API keys are omadia's **second authentication method**, alongside the +human-bound `omadia_session` cookie. A server-to-server caller (the driving +use case is a Laravel/PHP integration) has no human behind it and no cookie +to present; it authenticates with a bearer key instead. + +**Where the code lives.** All of it — mint/hash/verify, the key store, the +per-key rate limiter, the usage audit log, and the mountable `requireApiKey` +Express middleware — lives in the workspace package +`middleware/packages/harness-api-key-auth` (`@omadia/api-key-auth`). Issue +#438 shipped these inside the `@omadia/channel-api` plugin; issue #439 moved +them out so the kernel can use them too. The kernel must never import a +channel plugin, and a plugin cannot import kernel source, so a shared package +is the only home that lets both consume the same implementation. **There is +exactly one implementation of the credential** — a second one, however small, +is how a security-critical primitive quietly diverges. + +**Mounting it.** Any Express route, kernel or plugin, can apply +`requireApiKey({ apiKeys, rateLimiter, auditLog, scope })`. It attaches an +`ApiKeyPrincipal` to `req.apiKey` and deliberately does **not** populate +`req.session`: a `SessionClaims` value means "a human logged in", and its +`role` is hard-typed `'admin'`, so synthesizing one for a machine would make +every downstream session-reading route silently treat a key as an operator. +A route has to opt in to machine callers by reading `req.apiKey`. + +**Scopes (issue #439).** Every key carries a scope set — `:` +strings, or the global `*`. `requireApiKey` answers `403 forbidden` when the +key lacks the scope the route declares. Matching is exact; there are no +prefix wildcards (`chat:*`), because a prefix matcher invites the "I thought +that didn't cover delete" mistake scopes exist to prevent. A key persisted +before scopes existed has **no** `scopes` field at all and is normalized to +`['chat:write']` — precisely the one capability it had when it was minted. +Defaulting such keys to `*` would also keep them working, and would silently +widen every existing key to whatever scoped surface lands next; that is a +privilege escalation delivered by an upgrade, so it is not what we do. + +**Malformed persisted scopes deny, they do not default.** `normalizeScopes` +distinguishes *absent* from *malformed*. Absent (`scopes === undefined`, the +genuine pre-#439 record) → the legacy default above. Present but not an +array, or an array containing anything that is not a valid scope string +(`"memory:read"` stored as a bare string, `["Chat:Write"]` with the wrong +case, `[]`) → the **empty** scope set: the key still authenticates, and every +`hasScope` check on it fails closed, so it is authorized for nothing. This +matters because a malformed field is at least as likely to be a key an +operator deliberately restricted *away* from chat as it is to be corruption, +and falling back to a capability grant in that case hands the key exactly the +access the operator removed. Partially-valid arrays deny too rather than +silently narrowing to the valid subset — a record we cannot read faithfully +is a record we must not guess at. Each such case emits a +`[api-key-auth] malformed persisted scopes` warning so an operator can see +why a key stopped working. + +**Session-gate exemption stays narrow.** `POST /api/public/v1/chat` is the +only API-key route exempted from the session middleware +(`middleware/src/auth/publicPaths.ts`). Mounting `requireApiKey` on a new +route requires adding that route to `publicPaths.ts` — add the narrowest +regex that covers the one route, never a prefix that also catches its +siblings. Note that omission from `publicPaths.ts` is *necessary but not +sufficient* for a plugin-contributed router to be authenticated; see the +admin-keys discussion immediately below for why. `POST /api/public/v1/chat` +remains the first and only ingress this app exposes that is **not** cookie- +or provider-JWT-gated. + +**Key administration (`/api/public/v1/admin/keys`) — kernel-published +`ctx.operatorAuth`, in addition to the broad `/api` session gate.** +`middleware/src/index.ts` mounts `app.use('/api', requireAuth, +createChatRouter(...))` (the OB-106 hotfix) early in server boot, well +before `pluginRouteRegistry.mountAll(app)` runs later in the same boot +sequence. Express evaluates middleware in mount order for the whole `/api` +prefix regardless of which router ultimately answers a given path, so +`requireAuth` already runs in front of every `/api/*` request — including +plugin-mounted routes — unless that specific path is listed in +`middleware/src/auth/publicPaths.ts`'s exemption list. `/api/public/v1/admin/keys` +was never added to that list (only `.../chat` was, deliberately), so it was +already covered by this session gate, the same mechanism that protects +every other channel's non-exempted routes (see `publicPaths.ts`'s own doc +comment). An earlier revision of this document instead described the admin +routes as reachable by any anonymous caller; that was wrong — it read +`core.registerRouter` (`middleware/src/channels/routeRegistry.ts`, which +does only gate on the channel's active/inactive state) as the sole gate in +front of the router, without accounting for the broad `/api` mount that +Express already applies ahead of it. A minimal reproduction mirroring the +real mount order (real `createRequireAuth` + `publicPaths`, same mount +sequence as `index.ts`) confirms an anonymous request to +`/api/public/v1/admin/keys` gets `401 {code:'auth.missing'}` from that gate +before ever reaching the plugin router. + +That coverage is real, but it depends on an *implicit* invariant: the +broad `/api` mount happening to run before this plugin's router is mounted, +and this path happening not to be added to `publicPaths.ts`. Either one is +easy to break by accident in a future refactor — reordering mounts, moving +this plugin behind a different prefix, or a well-meaning future PR adding +`/api/public/v1/admin` to the exemption list by pattern-matching too +broadly against the neighboring `/chat` entry. None of that would raise an +error; the admin surface would just quietly stop being gated. So the fix +below adds an *explicit* check inside the plugin itself, so the guarantee +travels with the router regardless of where or in what order it gets +mounted — and publishes a reusable accessor so future plugins that need an +admin surface don't have to rely on the same mount-order coincidence. + +The real fix: `PluginContext` now exposes an optional `ctx.operatorAuth` +(`OperatorAuthAccessor`, `middleware/packages/plugin-api/src/pluginContext.ts`), +published by the kernel (`middleware/src/auth/operatorAuthAccessor.ts`) and +wired into every plugin-context factory +(`middleware/src/platform/pluginContext.ts`, threaded through +`ToolPluginRuntime`, `DynamicAgentRuntime`, and `DefaultChannelRegistry`). +`hasValidSession(cookieHeader)` reuses `evaluateSessionToken` — the EXACT +SAME session-verification logic `requireAuth` runs (same cookie name, same +signing key, same Entra-whitelist rule) — extracted into +`middleware/src/auth/requireAuth.ts` so there is exactly one code path that +decides session validity, never two that can drift apart. +`adminKeysRouter.ts` applies this as router-level middleware ahead of every +route: missing/invalid session → `401` (same `{code, message}` shape as +`requireAuth`); `ctx.operatorAuth` itself unavailable (an older host that +never wired it) → `503`, so the router **fails closed** rather than +silently mounting unauthenticated. See `adminKeysRouter.test.ts`'s +"operator-session auth" and "fails closed" test blocks for the coverage +that was missing before this fix. + +**Credential model — per-key service identity.** Each API key *is* its own +identity, not a delegate for a human end-user: `ChannelUserRef{ kind: +'custom', id: 'key:' }`. Every action traces to exactly one key; +there is no impersonation trust boundary to design or police, and no +"act on behalf of a user" surface in v1. + +**Storage — vault-backed, hash-only-at-rest.** Keys are minted as +`omk_<32 random bytes, base64url>` (`apiKeyToken.ts`). The plaintext is +returned to the operator exactly once, at creation time, and is never +persisted; only its sha256 hex digest is written to this plugin's own +`ctx.secrets` vault namespace (`apiKeyStore.ts`, one vault entry per key — +no DB migration for v1). Hashing is deliberately unsalted: the key itself +is a 256-bit high-entropy random value, not a low-entropy human-chosen +secret, so there is no dictionary/rainbow-table surface for a salt to +defend against — the same reasoning applies to GitHub PATs and Stripe API +keys, which are also hashed unsalted. + +**Verification — constant-time.** `verify()` walks every stored, +non-revoked key and compares each one's hash against the presented token's +hash with `crypto.timingSafeEqual`, deliberately without an early return on +the first match, so total work (and the timing signal) never depends on +which key, if any, matched. + +**Rate limiting — fixed-window, per key, in-memory and per-process.** Each +key gets its own in-memory fixed-window counter (`rateLimiter.ts`, 60s +window, capacity = `rateLimitPerMinute` set at key-creation time). This +state lives in a single Node process's memory only — it is **not** shared +across multiple replicas/instances of this app, and a restart clears every +counter. If this app is ever run with more than one replica behind a load +balancer, each replica enforces the limit independently, so the effective +ceiling for a key is `rateLimitPerMinute × replica count`, not the +configured value. This is an accepted v1 trade-off, same bar as the +`TokenBucket` in `httpAccessor.ts` elsewhere in this codebase — "good enough +to stop a runaway caller", not a precise distributed quota. A shared/ +distributed limiter (e.g. Redis-backed) was explicitly considered and +declined for v1; revisit only if multi-replica deployment of this app +becomes real. + +**Revocation.** `POST /api/public/v1/admin/keys/:id/revoke` sets +`revokedAt` on the key's vault record (idempotent — revoking an +already-revoked key is a no-op that returns its unchanged view). `verify()` +skips any record with `revokedAt` set, so a revoked key starts failing +immediately on its very next call — no propagation delay, no cache to +invalidate. + +**Usage audit.** Every call that gets *past key verification* — i.e. every +authenticated call, regardless of what happens next — is recorded as one +entry (`auditLog.ts`) with a status reflecting the real outcome: `ok`, +`rate_limited`, `forbidden` (scope check failed), `invalid_request`, or +`error` (the handler failed). `requireApiKey` records the outcomes it +produces itself; the route handler records its own via +`req.apiKey.audit(...)`, because only the handler knows whether the work +succeeded. Unauthenticated calls (missing/invalid/revoked key) are not +audited here — they never got the caller identity that makes an audit +entry meaningful. + +**PII masking.** Chat turns from this ingress go through the exact same +`CoreApi.handleTurnStream` dispatch as every other channel (Teams, +Telegram, Omadia UI) — no second, parallel response path — so +privacy-guard's prompt masking and receipt behavior apply identically. + +## 10. Reviewer checklist Before merging a PR that touches credentials, prompts, or proxy routes: diff --git a/middleware/package-lock.json b/middleware/package-lock.json index ca8f20aab..7678bcecf 100644 --- a/middleware/package-lock.json +++ b/middleware/package-lock.json @@ -18,6 +18,7 @@ "@azure/msal-node": "^5.3.1", "@microsoft/microsoft-graph-client": "^3.0.7", "@types/better-sqlite3": "^7.6.13", + "@types/cookie": "^0.6.0", "@types/cookie-parser": "^1.4.10", "@types/multer": "^2.1.0", "@types/yauzl": "^3.4.0", @@ -25,6 +26,7 @@ "better-sqlite3": "^12.11.1", "bonjour-service": "^1.4.3", "botbuilder": "^4.23.3", + "cookie": "^0.7.2", "cookie-parser": "^1.4.7", "croner": "^9.1.0", "dotenv": "^17.4.2", @@ -2113,10 +2115,18 @@ "resolved": "packages/agent-seo-analyst", "link": true }, + "node_modules/@omadia/api-key-auth": { + "resolved": "packages/harness-api-key-auth", + "link": true + }, "node_modules/@omadia/canvas-core": { "resolved": "packages/canvas-core", "link": true }, + "node_modules/@omadia/channel-api": { + "resolved": "packages/harness-channel-api", + "link": true + }, "node_modules/@omadia/channel-sdk": { "resolved": "packages/harness-channel-sdk", "link": true @@ -2691,6 +2701,12 @@ "@types/node": "*" } }, + "node_modules/@types/cookie": { + "version": "0.6.0", + "resolved": "https://registry.npmjs.org/@types/cookie/-/cookie-0.6.0.tgz", + "integrity": "sha512-4Kh9a6B2bQciAhf7FSuMRRkUWecJgJu9nPnx3yzpsfXX/c50REIqpHY4C82bXP90qrLtXtkDxTZosYO3UpOwlA==", + "license": "MIT" + }, "node_modules/@types/cookie-parser": { "version": "1.4.10", "resolved": "https://registry.npmjs.org/@types/cookie-parser/-/cookie-parser-1.4.10.tgz", @@ -9295,6 +9311,32 @@ "undici": "^8.0.0" } }, + "packages/harness-api-key-auth": { + "name": "@omadia/api-key-auth", + "version": "0.1.0", + "license": "MIT", + "engines": { + "node": ">=20" + }, + "peerDependencies": { + "express": "^5.1.0" + } + }, + "packages/harness-channel-api": { + "name": "@omadia/channel-api", + "version": "0.1.0", + "license": "MIT", + "engines": { + "node": ">=20" + }, + "peerDependencies": { + "@omadia/api-key-auth": "^0.1.0", + "@omadia/channel-sdk": "^0.1.0", + "@omadia/plugin-api": "^0.1.0", + "express": "^5.1.0", + "zod": "^4.0.0" + } + }, "packages/harness-channel-sdk": { "name": "@omadia/channel-sdk", "version": "0.1.0", diff --git a/middleware/package.json b/middleware/package.json index 777526cb7..9fdca3bb2 100644 --- a/middleware/package.json +++ b/middleware/package.json @@ -20,14 +20,14 @@ ], "scripts": { "preinstall": "node scripts/check-node-version.mjs", - "build": "npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/embedding-adapter-openai && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsc && node scripts/copy-build-assets.mjs", + "build": "npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/api-key-auth && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/embedding-adapter-openai && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/channel-api && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsc && node scripts/copy-build-assets.mjs", "start": "node dist/index.js", - "dev": "node scripts/ensure-native-abi.mjs && npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/embedding-adapter-openai && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsx watch --ignore './.memory/**' --ignore './.uploaded-packages/**' --ignore './data/**' --ignore './dist/**' --ignore './packages/*/dist/**' --ignore './seed/**' src/index.ts", + "dev": "node scripts/ensure-native-abi.mjs && npm run build -w @omadia/plugin-api && npm run build -w @omadia/llm-provider-api && npm run build -w @omadia/llm-provider && npm run build -w @omadia/llm-adapter-anthropic && npm run build -w @omadia/llm-adapter-openai && npm run build -w @omadia/canvas-core && npm run build -w @omadia/conductor-core && npm run build -w @omadia/plugin-ui-helpers && npm run build -w @omadia/api-key-auth && npm run build -w @omadia/channel-sdk && npm run build -w @omadia/diagrams && npm run build -w @omadia/memory && npm run build -w @omadia/memory-postgres && npm run build -w @omadia/embeddings && npm run build -w @omadia/embedding-adapter-openai && npm run build -w @omadia/knowledge-graph-inmemory && npm run build -w @omadia/knowledge-graph-neon && npm run build -w @omadia/usage-telemetry && npm run build -w @omadia/orchestrator-extras && npm run build -w @omadia/verifier && npm run build -w @omadia/plugin-privacy-guard && npm run build -w @omadia/orchestrator && npm run build -w @omadia/ui-orchestrator && npm run build -w @omadia/ui-channel && npm run build -w @omadia/channel-api && npm run build -w @omadia/plugin-office && npm run build -w @omadia/plugin-web-search && npm run build -w @omadia/plugin-quality-guard && npm run build -w @omadia/agent-seo-analyst && npm run build -w @omadia/agent-reference-maximum && npm run build -w @omadia/plugin-plan-runner && tsx watch --ignore './.memory/**' --ignore './.uploaded-packages/**' --ignore './data/**' --ignore './dist/**' --ignore './packages/*/dist/**' --ignore './seed/**' src/index.ts", "dev:clean": "node scripts/dev-clean.mjs && npm run dev", "ensure-native-abi": "node scripts/ensure-native-abi.mjs", - "lint": "eslint src/ packages/plugin-api/src/ packages/llm-provider-api/src/ packages/llm-provider/src/ packages/llm-adapter-anthropic/src/ packages/llm-adapter-openai/src/ packages/harness-ui-helpers/src/ packages/harness-channel-sdk/src/ packages/harness-diagrams/src/ packages/harness-memory/src/ packages/harness-memory-postgres/src/ packages/harness-embeddings/src/ packages/embedding-adapter-openai/src/ packages/harness-knowledge-graph-inmemory/src/ packages/harness-knowledge-graph-neon/src/ packages/harness-usage-telemetry/src/ packages/harness-orchestrator-extras/src/ packages/harness-verifier/src/ packages/harness-orchestrator/src/ packages/harness-plugin-web-search/src/ packages/harness-plugin-quality-guard/src/ packages/harness-plugin-privacy-guard/src/ packages/harness-plugin-office/src/ packages/omadia-ui-orchestrator/src/ packages/omadia-ui-channel/src/ packages/harness-plugin-plan-runner/src/", - "lint:fix": "eslint src/ packages/plugin-api/src/ packages/llm-provider-api/src/ packages/llm-provider/src/ packages/llm-adapter-anthropic/src/ packages/llm-adapter-openai/src/ packages/harness-ui-helpers/src/ packages/harness-channel-sdk/src/ packages/harness-diagrams/src/ packages/harness-memory/src/ packages/harness-memory-postgres/src/ packages/harness-embeddings/src/ packages/embedding-adapter-openai/src/ packages/harness-knowledge-graph-inmemory/src/ packages/harness-knowledge-graph-neon/src/ packages/harness-usage-telemetry/src/ packages/harness-orchestrator-extras/src/ packages/harness-verifier/src/ packages/harness-orchestrator/src/ packages/harness-plugin-web-search/src/ packages/harness-plugin-quality-guard/src/ packages/harness-plugin-privacy-guard/src/ packages/harness-plugin-office/src/ packages/omadia-ui-orchestrator/src/ packages/omadia-ui-channel/src/ packages/harness-plugin-plan-runner/src/ --fix", - "typecheck": "npm run typecheck -w @omadia/plugin-api && npm run typecheck -w @omadia/llm-provider-api && npm run typecheck -w @omadia/llm-provider && npm run typecheck -w @omadia/llm-adapter-anthropic && npm run typecheck -w @omadia/llm-adapter-openai && npm run typecheck -w @omadia/canvas-core && npm run typecheck -w @omadia/conductor-core && npm run typecheck -w @omadia/plugin-ui-helpers && npm run typecheck -w @omadia/channel-sdk && npm run typecheck -w @omadia/diagrams && npm run typecheck -w @omadia/memory && npm run typecheck -w @omadia/memory-postgres && npm run typecheck -w @omadia/embeddings && npm run typecheck -w @omadia/embedding-adapter-openai && npm run typecheck -w @omadia/knowledge-graph-inmemory && npm run typecheck -w @omadia/knowledge-graph-neon && npm run typecheck -w @omadia/orchestrator-extras && npm run typecheck -w @omadia/verifier && npm run typecheck -w @omadia/plugin-privacy-guard && npm run typecheck -w @omadia/orchestrator && npm run typecheck -w @omadia/ui-orchestrator && npm run typecheck -w @omadia/ui-channel && npm run typecheck -w @omadia/plugin-office && npm run typecheck -w @omadia/plugin-web-search && npm run typecheck -w @omadia/plugin-quality-guard && npm run typecheck -w @omadia/agent-seo-analyst && npm run typecheck -w @omadia/agent-reference-maximum && npm run typecheck -w @omadia/plugin-plan-runner && tsc --noEmit", + "lint": "eslint src/ packages/plugin-api/src/ packages/llm-provider-api/src/ packages/llm-provider/src/ packages/llm-adapter-anthropic/src/ packages/llm-adapter-openai/src/ packages/harness-ui-helpers/src/ packages/harness-api-key-auth/src/ packages/harness-channel-sdk/src/ packages/harness-diagrams/src/ packages/harness-memory/src/ packages/harness-memory-postgres/src/ packages/harness-embeddings/src/ packages/embedding-adapter-openai/src/ packages/harness-knowledge-graph-inmemory/src/ packages/harness-knowledge-graph-neon/src/ packages/harness-usage-telemetry/src/ packages/harness-orchestrator-extras/src/ packages/harness-verifier/src/ packages/harness-orchestrator/src/ packages/harness-plugin-web-search/src/ packages/harness-plugin-quality-guard/src/ packages/harness-plugin-privacy-guard/src/ packages/harness-plugin-office/src/ packages/omadia-ui-orchestrator/src/ packages/omadia-ui-channel/src/ packages/harness-channel-api/src/ packages/harness-plugin-plan-runner/src/", + "lint:fix": "eslint src/ packages/plugin-api/src/ packages/llm-provider-api/src/ packages/llm-provider/src/ packages/llm-adapter-anthropic/src/ packages/llm-adapter-openai/src/ packages/harness-ui-helpers/src/ packages/harness-api-key-auth/src/ packages/harness-channel-sdk/src/ packages/harness-diagrams/src/ packages/harness-memory/src/ packages/harness-memory-postgres/src/ packages/harness-embeddings/src/ packages/embedding-adapter-openai/src/ packages/harness-knowledge-graph-inmemory/src/ packages/harness-knowledge-graph-neon/src/ packages/harness-usage-telemetry/src/ packages/harness-orchestrator-extras/src/ packages/harness-verifier/src/ packages/harness-orchestrator/src/ packages/harness-plugin-web-search/src/ packages/harness-plugin-quality-guard/src/ packages/harness-plugin-privacy-guard/src/ packages/harness-plugin-office/src/ packages/omadia-ui-orchestrator/src/ packages/omadia-ui-channel/src/ packages/harness-channel-api/src/ packages/harness-plugin-plan-runner/src/ --fix", + "typecheck": "npm run typecheck -w @omadia/plugin-api && npm run typecheck -w @omadia/llm-provider-api && npm run typecheck -w @omadia/llm-provider && npm run typecheck -w @omadia/llm-adapter-anthropic && npm run typecheck -w @omadia/llm-adapter-openai && npm run typecheck -w @omadia/canvas-core && npm run typecheck -w @omadia/conductor-core && npm run typecheck -w @omadia/plugin-ui-helpers && npm run typecheck -w @omadia/api-key-auth && npm run typecheck -w @omadia/channel-sdk && npm run typecheck -w @omadia/diagrams && npm run typecheck -w @omadia/memory && npm run typecheck -w @omadia/memory-postgres && npm run typecheck -w @omadia/embeddings && npm run typecheck -w @omadia/embedding-adapter-openai && npm run typecheck -w @omadia/knowledge-graph-inmemory && npm run typecheck -w @omadia/knowledge-graph-neon && npm run typecheck -w @omadia/orchestrator-extras && npm run typecheck -w @omadia/verifier && npm run typecheck -w @omadia/plugin-privacy-guard && npm run typecheck -w @omadia/orchestrator && npm run typecheck -w @omadia/ui-orchestrator && npm run typecheck -w @omadia/ui-channel && npm run typecheck -w @omadia/channel-api && npm run typecheck -w @omadia/plugin-office && npm run typecheck -w @omadia/plugin-web-search && npm run typecheck -w @omadia/plugin-quality-guard && npm run typecheck -w @omadia/agent-seo-analyst && npm run typecheck -w @omadia/agent-reference-maximum && npm run typecheck -w @omadia/plugin-plan-runner && tsc --noEmit", "format": "prettier --write \"src/**/*.ts\"", "format:check": "prettier --check \"src/**/*.ts\"", "smoke:entity-refs": "tsx scripts/smoke-entity-refs.ts", @@ -51,6 +51,7 @@ "@azure/msal-node": "^5.3.1", "@microsoft/microsoft-graph-client": "^3.0.7", "@types/better-sqlite3": "^7.6.13", + "@types/cookie": "^0.6.0", "@types/cookie-parser": "^1.4.10", "@types/multer": "^2.1.0", "@types/yauzl": "^3.4.0", @@ -58,6 +59,7 @@ "better-sqlite3": "^12.11.1", "bonjour-service": "^1.4.3", "botbuilder": "^4.23.3", + "cookie": "^0.7.2", "cookie-parser": "^1.4.7", "croner": "^9.1.0", "dotenv": "^17.4.2", diff --git a/middleware/packages/harness-api-key-auth/package.json b/middleware/packages/harness-api-key-auth/package.json new file mode 100644 index 000000000..51cbdfe99 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/package.json @@ -0,0 +1,30 @@ +{ + "name": "@omadia/api-key-auth", + "version": "0.1.0", + "private": true, + "type": "module", + "main": "dist/index.js", + "types": "dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + } + }, + "files": [ + "dist" + ], + "description": "Server-to-server API-key authentication for omadia (issue #439). Owns the ONE implementation of mint/hash/verify, the vault-backed key store with per-key scopes, the per-key rate limiter, the usage audit log, and the mountable `requireApiKey` Express middleware. Deliberately dependency-free (except express types) so both the kernel and any plugin can consume it without a layering inversion.", + "license": "MIT", + "scripts": { + "build": "tsc", + "typecheck": "tsc --noEmit", + "clean": "rm -rf dist" + }, + "peerDependencies": { + "express": "^5.1.0" + }, + "engines": { + "node": ">=20" + } +} diff --git a/middleware/packages/harness-api-key-auth/src/apiKeyScopes.ts b/middleware/packages/harness-api-key-auth/src/apiKeyScopes.ts new file mode 100644 index 000000000..4ce831051 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/apiKeyScopes.ts @@ -0,0 +1,153 @@ +/** + * Issue #439 — per-key scopes. + * + * Authentication alone stopped being sufficient the moment API keys became a + * first-class auth method usable by more than the one chat route: a key that + * authenticates must also be restrictable to the capabilities its holder + * actually needs. A scope is a plain `:` string so a plugin + * can mint its own vocabulary without this package having to know about it. + * + * Deliberately no hierarchy matching (`chat:*`): the only wildcard is the + * global `*`, and everything else is an exact match. A prefix matcher invites + * exactly the kind of "I thought `admin:*` didn't cover `admin:delete`" + * mistake that scopes exist to prevent. + */ + +/** A scope string. Kept as `string` rather than a closed union so plugins can + * define their own capability names — validity is enforced by shape + * (`isValidScope`), not by an allow-list this package would have to own. */ +export type ApiKeyScope = string; + +/** Grants every scope. Only ever set explicitly by an operator — never a + * default, and never inferred for a key that predates scopes. */ +export const WILDCARD_SCOPE = '*'; + +/** The capability the public chat ingress requires (`@omadia/channel-api`). */ +export const CHAT_WRITE_SCOPE = 'chat:write'; + +/** + * What a key with no persisted `scopes` field is treated as. + * + * Backward compatibility with a security floor: keys minted by issue #438 + * predate scopes entirely, and the only thing they could ever reach was + * `POST /api/public/v1/chat`. Defaulting them to `chat:write` keeps every one + * of them working exactly as before. Defaulting them to `*` would also "keep + * them working" — and would silently widen every existing key to whatever + * scoped surface gets added next, which is a privilege escalation delivered + * by an upgrade. + */ +export const LEGACY_DEFAULT_SCOPES: readonly ApiKeyScope[] = [CHAT_WRITE_SCOPE]; + +/** `:`, lowercase, or the bare global wildcard. */ +const SCOPE_PATTERN = /^[a-z][a-z0-9_-]*:[a-z][a-z0-9_-]*$/; + +export function isValidScope(value: unknown): value is ApiKeyScope { + if (typeof value !== 'string') return false; + return value === WILDCARD_SCOPE || SCOPE_PATTERN.test(value); +} + +/** Grants nothing. Every `hasScope` check against it is false. */ +export const DENY_ALL_SCOPES: readonly ApiKeyScope[] = []; + +/** + * Normalizes the `scopes` field of a PERSISTED record. + * + * The distinction that matters here is *absent* versus *malformed*, because + * collapsing the two turns a read error into a capability GRANT: + * + * - **Absent** (`undefined` — the field was never written) is a genuine + * pre-#439 record. It gets `LEGACY_DEFAULT_SCOPES`: exactly what that key + * could already do, no more. + * - **Present but malformed** — not an array (`"memory:read"` stored as a + * bare string), or an array holding anything that is not a valid scope + * (`["Chat:Write"]`, `["nonsense"]`), or an empty array — is corruption or + * a writer bug. It gets `DENY_ALL_SCOPES`. Such a record is at least as + * likely to be a key an operator deliberately restricted AWAY from chat as + * it is to be a lost pre-#439 key, and handing it `chat:write` would grant + * precisely the access the operator removed. + * + * Partially-valid arrays deny too, rather than silently narrowing to the + * valid subset: a record we cannot read faithfully is one we must not guess + * at, and a key that looks like it has half its capabilities is worse to + * debug than one that plainly has none. + * + * The key still AUTHENTICATES in the deny case — `verify()` is unaffected — + * it is simply authorized for nothing, so every scope check fails closed and + * the caller gets `403`, not a silently-widened `200`. + * + * The empty array earns its place in the malformed group by an invariant on + * the write side: no writer here can persist one. `assertValidScopes` throws + * on `[]`, so `create()` cannot store it, and the HTTP boundary + * (`CreateKeyRequestSchema` in `@omadia/channel-api`'s `adminKeysRouter.ts`) + * answers `400` before that. A persisted `[]` is therefore corruption or a + * foreign writer by construction, and denying is the only honest reading. + * Keep the two halves in step — if `[]` ever became persistable, the same + * value would mean "grant the default" on write and "grant nothing" on read. + */ +export function normalizeScopes(raw: unknown): readonly ApiKeyScope[] { + if (raw === undefined) return LEGACY_DEFAULT_SCOPES; + if (!Array.isArray(raw)) { + warnMalformedScopes('scopes is not an array', raw); + return DENY_ALL_SCOPES; + } + if (raw.length === 0) { + warnMalformedScopes('scopes is an empty array', raw); + return DENY_ALL_SCOPES; + } + const invalid = raw.filter((entry) => !isValidScope(entry)); + if (invalid.length > 0) { + warnMalformedScopes( + `scopes contains ${String(invalid.length)} invalid entr${invalid.length === 1 ? 'y' : 'ies'}`, + raw, + ); + return DENY_ALL_SCOPES; + } + return Array.from(new Set(raw as readonly ApiKeyScope[])); +} + +/** A malformed persisted `scopes` field silently stops a key from working; + * without this line an operator has no way to tell that from a revoke. The + * raw value is summarized, never dumped — the record it came from also + * holds a key hash. */ +function warnMalformedScopes(reason: string, raw: unknown): void { + console.warn( + `[api-key-auth] malformed persisted scopes (${reason}, type=${ + Array.isArray(raw) ? 'array' : typeof raw + }) — key denied all scopes until the record is repaired`, + ); +} + +/** + * Validates operator-supplied scopes at CREATION time. Strict by design — + * silently dropping a typo'd scope would hand back a key that looks right and + * is quietly missing a capability. Callers accepting HTTP input should + * validate first and answer 400; reaching this throw is a programmer error. + * + * An EXPLICIT empty array is rejected here rather than resolved to any default. + * The read path (`normalizeScopes`) treats a persisted `[]` as corruption and + * returns `DENY_ALL_SCOPES`; if creation quietly turned the same value into + * `LEGACY_DEFAULT_SCOPES` instead, one field would mean "deny everything" going + * out and "grant chat" going in — and the grant is the dangerous direction. + * Omitting `scopes` entirely remains the way to ask for the legacy default. + */ +export function assertValidScopes(scopes: readonly unknown[]): readonly ApiKeyScope[] { + if (scopes.length === 0) { + throw new Error( + 'API-key scopes must not be empty; omit the field entirely to accept the default', + ); + } + const invalid = scopes.filter((s) => !isValidScope(s)); + if (invalid.length > 0) { + throw new Error(`invalid API-key scope(s): ${invalid.map((s) => String(s)).join(', ')}`); + } + return Array.from(new Set(scopes as readonly ApiKeyScope[])); +} + +/** True when `granted` covers `required` — exact match, or the global `*`. */ +export function hasScope( + granted: readonly ApiKeyScope[] | undefined, + required: ApiKeyScope, +): boolean { + if (!granted) return false; + return granted.includes(WILDCARD_SCOPE) || granted.includes(required); +} diff --git a/middleware/packages/harness-api-key-auth/src/apiKeyStore.ts b/middleware/packages/harness-api-key-auth/src/apiKeyStore.ts new file mode 100644 index 000000000..49d831016 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/apiKeyStore.ts @@ -0,0 +1,225 @@ +/** + * Vault-backed API-key store. Introduced by issue #438 inside + * `@omadia/channel-api`, moved here by issue #439 (plus per-key scopes) so + * there is exactly ONE key store in the codebase. + * + * Design decision (locked on issue #438): API keys are vault-backed via the + * owning plugin's OWN `ctx.secrets` namespace — no DB migration for v1. Only + * the sha256 hash of a key ever lands in the vault (see `apiKeyToken.ts`); + * the plaintext is returned to the caller exactly once, at `create()` time. + * + * Each key is its own vault entry (`key:` → JSON `ApiKeyRecord`) rather + * than one growing blob, so create/revoke touch only their own entry. Reads + * scale with the number of installed keys (`FileSecretVault`'s own docs cite + * O(10) keys per agent as the target scale for v1) — acceptable for an + * operator-managed credential list; a future revision can move to durable + * storage if that stops being true. + */ + +import { randomUUID } from 'node:crypto'; + +import type { ApiKeyScope } from './apiKeyScopes.js'; +import { assertValidScopes, LEGACY_DEFAULT_SCOPES, normalizeScopes } from './apiKeyScopes.js'; +import { mintApiKey, verifyApiKey } from './apiKeyToken.js'; +import type { ApiKeySecretStorage } from './secretStorage.js'; + +export interface ApiKeyRecord { + readonly id: string; + readonly label?: string; + /** sha256 hex of the plaintext key. Never exposed outside this module. */ + readonly hash: string; + readonly rateLimitPerMinute: number; + /** Capabilities this key may exercise. Always populated on read — a record + * persisted before scopes existed (no `scopes` field at all) is normalized + * to `LEGACY_DEFAULT_SCOPES`, and a record whose `scopes` field is present + * but malformed is normalized to the EMPTY set, so consumers never have to + * handle `undefined` and a corrupt record never yields a grant. */ + readonly scopes: readonly ApiKeyScope[]; + readonly createdAt: number; + readonly revokedAt?: number; +} + +/** `ApiKeyRecord` minus the hash — the shape every caller outside this + * module (admin route, tests) is allowed to see. */ +export type ApiKeyPublicView = Omit; + +export interface CreateApiKeyOptions { + readonly label?: string; + readonly rateLimitPerMinute?: number; + /** Omitted → `LEGACY_DEFAULT_SCOPES`, i.e. exactly what a pre-scopes key + * could do. Throws on a malformed scope rather than dropping it silently + * (see `assertValidScopes`). */ + readonly scopes?: readonly ApiKeyScope[]; +} + +export interface CreatedApiKey { + readonly record: ApiKeyPublicView; + /** Plaintext — returned exactly once. Callers must show/copy it now. */ + readonly token: string; +} + +export interface ApiKeyStore { + create(opts: CreateApiKeyOptions): Promise; + list(): Promise; + /** Idempotent: revoking an already-revoked key returns its (unchanged) + * view. Returns `undefined` when no key with that id exists. */ + revoke(id: string): Promise; + /** Resolves the presented plaintext key to its record — only if a + * matching, non-revoked key exists. Every stored hash is compared in + * constant time via `verifyApiKey`; no early return keyed off which + * record is checked first. */ + verify(token: string): Promise; +} + +const VAULT_KEY_PREFIX = 'key:'; + +const DEFAULT_RATE_LIMIT_PER_MINUTE = 60; +const MIN_RATE_LIMIT_PER_MINUTE = 1; +const MAX_RATE_LIMIT_PER_MINUTE = 6000; + +function clampRateLimit(value: number | undefined): number { + if (typeof value !== 'number' || !Number.isFinite(value)) { + return DEFAULT_RATE_LIMIT_PER_MINUTE; + } + return Math.min( + MAX_RATE_LIMIT_PER_MINUTE, + Math.max(MIN_RATE_LIMIT_PER_MINUTE, Math.floor(value)), + ); +} + +function toPublicView(record: ApiKeyRecord): ApiKeyPublicView { + const { hash: _hash, ...view } = record; + return view; +} + +/** + * Every path that deserializes a vault entry funnels through here, so a + * record written before scopes existed comes back with the legacy default + * filled in instead of `undefined` leaking into scope checks — and a record + * whose `scopes` field is present but unreadable comes back denying + * everything rather than falling back to a grant (see `normalizeScopes`). + */ +function hydrate(raw: unknown): ApiKeyRecord { + const record = raw as ApiKeyRecord; + return { ...record, scopes: normalizeScopes((record as { scopes?: unknown }).scopes) }; +} + +/** + * Narrows an optional `SecretsAccessor` write method to its non-optional + * function type. A plain `if (!fn) throw` guard on a captured variable does + * NOT narrow that variable's type inside nested closures defined afterwards + * (TypeScript re-widens captured bindings across function boundaries) — this + * helper gives `write`/`del` below a genuinely non-optional TYPE instead of + * relying on control-flow narrowing that closures can't see. + */ +function requireWriter(fn: T | undefined, name: string): T { + if (!fn) { + throw new Error( + `createApiKeyStore requires a write-capable SecretsAccessor (missing ${name}) — declare permissions.secrets.runtime_write in the manifest`, + ); + } + return fn; +} + +/** + * Builds the store. `secrets` must be write-capable (for a plugin, the + * manifest declares `permissions.secrets.runtime_write`) — the caller checks + * this once at wiring time and never mounts the routes at all otherwise, so + * this throwing here is a programmer error, not a runtime condition callers + * need to handle. + */ +export function createApiKeyStore(secrets: ApiKeySecretStorage): ApiKeyStore { + const write = requireWriter(secrets.set, 'set'); + requireWriter(secrets.delete, 'delete'); + + async function readRecord(id: string): Promise { + const raw = await secrets.get(VAULT_KEY_PREFIX + id); + if (!raw) return undefined; + try { + return hydrate(JSON.parse(raw)); + } catch { + return undefined; + } + } + + async function writeRecord(record: ApiKeyRecord): Promise { + await write(VAULT_KEY_PREFIX + record.id, JSON.stringify(record)); + } + + async function allRecords(): Promise { + const vaultKeys = await secrets.keys(); + const records: ApiKeyRecord[] = []; + for (const vaultKey of vaultKeys) { + if (!vaultKey.startsWith(VAULT_KEY_PREFIX)) continue; + const raw = await secrets.get(vaultKey); + if (!raw) continue; + try { + records.push(hydrate(JSON.parse(raw))); + } catch { + // Corrupt entry — skip it rather than fail the whole listing. + } + } + return records; + } + + return { + async create(opts) { + const { token, hash } = mintApiKey(); + const record: ApiKeyRecord = { + id: randomUUID(), + ...(opts.label ? { label: opts.label } : {}), + hash, + rateLimitPerMinute: clampRateLimit(opts.rateLimitPerMinute), + // OMITTED (`undefined`) is the only input that resolves to the legacy + // default. Anything explicitly supplied — including `[]` — goes through + // `assertValidScopes`, which throws on an empty array. That keeps this + // path in agreement with `normalizeScopes` on the read side, where a + // persisted `[]` is corruption and denies everything: the same value + // must never mean "deny" coming out and "grant chat" going in. Callers + // taking HTTP input reject `[]` at their own boundary first (see + // `adminKeysRouter.ts`, which answers 400) so this throw stays a + // programmer error rather than a 500. + // + // The scope set is always persisted EXPLICITLY, which is what lets + // `hydrate` read a missing `scopes` field as "genuinely pre-#439" + // rather than "written by us and lost". + scopes: + opts.scopes === undefined ? LEGACY_DEFAULT_SCOPES : assertValidScopes(opts.scopes), + createdAt: Date.now(), + }; + await writeRecord(record); + return { record: toPublicView(record), token }; + }, + + async list() { + const records = await allRecords(); + return records + .slice() + .sort((a, b) => a.createdAt - b.createdAt) + .map(toPublicView); + }, + + async revoke(id) { + const record = await readRecord(id); + if (!record) return undefined; + if (record.revokedAt !== undefined) return toPublicView(record); + const revoked: ApiKeyRecord = { ...record, revokedAt: Date.now() }; + await writeRecord(revoked); + return toPublicView(revoked); + }, + + async verify(token) { + if (typeof token !== 'string' || token.length === 0) return undefined; + const records = await allRecords(); + let match: ApiKeyRecord | undefined; + // Deliberately do NOT `break`/`return` on the first hit — walk every + // record so the total work (and therefore the timing signal) doesn't + // depend on which key, if any, matched. + for (const record of records) { + if (record.revokedAt !== undefined) continue; + if (verifyApiKey(token, record.hash)) match = record; + } + return match; + }, + }; +} diff --git a/middleware/packages/harness-api-key-auth/src/apiKeyToken.ts b/middleware/packages/harness-api-key-auth/src/apiKeyToken.ts new file mode 100644 index 000000000..c62880e8d --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/apiKeyToken.ts @@ -0,0 +1,67 @@ +/** + * API key: mint / hash / verify. Introduced by issue #438 inside + * `@omadia/channel-api`, moved here unchanged by issue #439 so the kernel and + * any plugin can share the SAME implementation (core must never import from a + * channel plugin). + * + * Mirrors `middleware/src/devplatform/jobToken.ts` (the dev-runner's one-time + * job token), the closest existing precedent for a bearer credential this + * codebase hashes at rest and verifies in constant time. The plaintext exists + * exactly once — at creation time, returned to the operator — and is never + * persisted. Only its sha256 hex lands in the vault (`apiKeyStore.ts`). + * Verification hashes the presented token and compares digests with + * `crypto.timingSafeEqual`, so a wrong key cannot be distinguished by timing. + */ + +import { createHash, randomBytes, timingSafeEqual } from 'node:crypto'; + +/** Every minted key carries this prefix so it is greppable in logs/incidents + * and visually distinct from other omadia tokens (e.g. `djr_`). */ +export const API_KEY_PREFIX = 'omk_'; + +/** 32 random bytes → 43 base64url chars; with the prefix, a 47-char key. */ +const API_KEY_RANDOM_BYTES = 32; + +export interface MintedApiKey { + /** Plaintext — hand to the operator once, never store, never log. */ + token: string; + /** sha256 hex — the ONLY thing that goes into the vault. */ + hash: string; +} + +/** sha256 hex of a UTF-8 string. */ +export function sha256Hex(input: string): string { + return createHash('sha256').update(input, 'utf8').digest('hex'); +} + +/** Mint a fresh API key and its stored hash. */ +export function mintApiKey(): MintedApiKey { + const token = API_KEY_PREFIX + randomBytes(API_KEY_RANDOM_BYTES).toString('base64url'); + return { token, hash: sha256Hex(token) }; +} + +/** + * Constant-time check of a presented key against a stored sha256 hash. Both + * operands are sha256 digests (fixed 32 bytes) once decoded, so `expected` + * and `actual` always match in length for a well-formed stored hash; + * `timingSafeEqual` never throws for that case. A malformed/empty stored + * hash, or a non-string input, is a plain `false` rather than an exception — + * the length check is a guard, not a shortcut that skips comparing the + * secret material itself. + */ +export function verifyApiKey(token: string, storedHash: string | null | undefined): boolean { + if (typeof token !== 'string' || token.length === 0) return false; + if (typeof storedHash !== 'string' || storedHash.length === 0) return false; + + const actual = Buffer.from(sha256Hex(token), 'hex'); + let expected: Buffer; + try { + expected = Buffer.from(storedHash, 'hex'); + } catch { + return false; + } + // Non-hex / truncated stored hash decodes to a different length — reject + // without ever calling timingSafeEqual on mismatched-length buffers (throws). + if (expected.length !== actual.length) return false; + return timingSafeEqual(actual, expected); +} diff --git a/middleware/packages/harness-api-key-auth/src/auditLog.ts b/middleware/packages/harness-api-key-auth/src/auditLog.ts new file mode 100644 index 000000000..34a4640f0 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/auditLog.ts @@ -0,0 +1,84 @@ +/** + * Usage audit log: who (which key) called what, when. Introduced by issue + * #438 inside `@omadia/channel-api`, moved here by issue #439. + * + * Vault-backed like `apiKeyStore.ts` (no DB migration for v1), stored as one + * JSON array under a single fixed vault key. Capped at `MAX_ENTRIES` — this + * is an operator-visible recent-activity trail, not a durable compliance + * log; a future revision can move it to durable storage if that need shows + * up. Writes are serialized through an internal promise chain so concurrent + * requests append rather than clobber each other's read-modify-write. + */ + +import type { ApiKeySecretStorage } from './secretStorage.js'; + +/** + * `ok` — the request was handled and the handler reported success. + * `rate_limited` — the key authenticated but was over its per-minute quota; + * the handler was never invoked. + * `forbidden` — the key authenticated but lacked the scope the route + * requires (issue #439); the handler was never invoked. + * `invalid_request` — the key authenticated but the request body failed + * schema validation; the handler was never invoked. + * `error` — the key authenticated, the handler ran, and it failed. + */ +export type AuditStatus = 'ok' | 'rate_limited' | 'forbidden' | 'error' | 'invalid_request'; + +export interface AuditEntry { + readonly keyId: string; + readonly route: string; + readonly method: string; + readonly at: number; + readonly status: AuditStatus; +} + +export interface AuditLog { + record(entry: AuditEntry): Promise; + list(): Promise; +} + +const VAULT_KEY = 'usage-audit-log'; +/** Exported so tests can assert the cap without hardcoding a magic number. */ +export const MAX_ENTRIES = 200; + +export function createAuditLog(secrets: ApiKeySecretStorage): AuditLog { + const write = secrets.set; + if (!write) { + throw new Error( + 'createAuditLog requires a write-capable SecretsAccessor — declare permissions.secrets.runtime_write in the manifest', + ); + } + + async function readAll(): Promise { + const raw = await secrets.get(VAULT_KEY); + if (!raw) return []; + try { + const parsed = JSON.parse(raw) as unknown; + return Array.isArray(parsed) ? (parsed as AuditEntry[]) : []; + } catch { + return []; + } + } + + // Serializes append operations so two in-flight requests don't both read + // the same snapshot and each write back an array missing the other's entry. + let writeQueue: Promise = Promise.resolve(); + + return { + record(entry) { + writeQueue = writeQueue + .then(async () => { + const current = await readAll(); + const next = [...current, entry].slice(-MAX_ENTRIES); + await write(VAULT_KEY, JSON.stringify(next)); + }) + // A logging failure must never break the caller's request; the next + // append still gets a fresh chain link. + .catch(() => undefined); + return writeQueue; + }, + list() { + return readAll(); + }, + }; +} diff --git a/middleware/packages/harness-api-key-auth/src/index.ts b/middleware/packages/harness-api-key-auth/src/index.ts new file mode 100644 index 000000000..452168522 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/index.ts @@ -0,0 +1,58 @@ +// @omadia/api-key-auth — public barrel (issue #439). +// +// The single implementation of omadia's server-to-server API-key credential: +// mint/hash/verify, the vault-backed store with per-key scopes, the per-key +// rate limiter, the usage audit log, and the mountable `requireApiKey` +// Express middleware. Consumed by the kernel and by plugins alike — neither +// direction is a layering inversion, which is exactly why this lives in its +// own workspace package rather than in `middleware/src/auth/` (a plugin +// cannot import kernel source) or in `@omadia/channel-api` (the kernel must +// never import a channel plugin). + +export { + API_KEY_PREFIX, + mintApiKey, + sha256Hex, + verifyApiKey, + type MintedApiKey, +} from './apiKeyToken.js'; + +export { + assertValidScopes, + CHAT_WRITE_SCOPE, + DENY_ALL_SCOPES, + hasScope, + isValidScope, + LEGACY_DEFAULT_SCOPES, + normalizeScopes, + WILDCARD_SCOPE, + type ApiKeyScope, +} from './apiKeyScopes.js'; + +export { + createApiKeyStore, + type ApiKeyPublicView, + type ApiKeyRecord, + type ApiKeyStore, + type CreateApiKeyOptions, + type CreatedApiKey, +} from './apiKeyStore.js'; + +export { + createAuditLog, + MAX_ENTRIES as AUDIT_LOG_MAX_ENTRIES, + type AuditEntry, + type AuditLog, + type AuditStatus, +} from './auditLog.js'; + +export { createRateLimiter, type RateLimiter } from './rateLimiter.js'; + +export { + bearerToken, + requireApiKey, + type ApiKeyPrincipal, + type RequireApiKeyOptions, +} from './requireApiKey.js'; + +export type { ApiKeySecretStorage } from './secretStorage.js'; diff --git a/middleware/packages/harness-api-key-auth/src/rateLimiter.ts b/middleware/packages/harness-api-key-auth/src/rateLimiter.ts new file mode 100644 index 000000000..f04b07f87 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/rateLimiter.ts @@ -0,0 +1,57 @@ +/** + * Per-key rate limiting. Introduced by issue #438 inside + * `@omadia/channel-api`, moved here unchanged by issue #439. + * + * Fixed-window token bucket, mirroring `middleware/src/platform/httpAccessor.ts`'s + * `TokenBucket` (same "not wall-clock accurate, good enough to stop a runaway + * caller" trade-off), but keyed per API key with a per-key configurable + * capacity instead of one shared plugin-wide limit — the design decision on + * issue #438 calls for per-key limits, not a single global one. + * + * In-memory, per-process. A restart clears every bucket; acceptable for v1 + * (a burst right after deploy is not the threat this defends against). + */ + +const WINDOW_MS = 60_000; + +class TokenBucket { + private count = 0; + private windowStart = Date.now(); + + constructor(private readonly capacity: number) {} + + tryConsume(): boolean { + const now = Date.now(); + if (now - this.windowStart >= WINDOW_MS) { + this.count = 0; + this.windowStart = now; + } + if (this.count >= this.capacity) return false; + this.count++; + return true; + } +} + +export interface RateLimiter { + /** Returns `true` when the call is within budget (and consumes one unit + * of it), `false` when the key is over its per-minute limit. */ + tryConsume(keyId: string, capacityPerMinute: number): boolean; +} + +export function createRateLimiter(): RateLimiter { + const buckets = new Map(); + + return { + tryConsume(keyId, capacityPerMinute) { + let bucket = buckets.get(keyId); + // A key's configured limit can change between calls (revoke + recreate + // reuses a fresh id, so this is really "first sight of this id"); pin + // the bucket's capacity to what was configured when we first saw it. + if (!bucket) { + bucket = new TokenBucket(capacityPerMinute); + buckets.set(keyId, bucket); + } + return bucket.tryConsume(); + }, + }; +} diff --git a/middleware/packages/harness-api-key-auth/src/requireApiKey.ts b/middleware/packages/harness-api-key-auth/src/requireApiKey.ts new file mode 100644 index 000000000..1509c624f --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/requireApiKey.ts @@ -0,0 +1,142 @@ +/** + * Issue #439 — `requireApiKey`: the mountable authentication middleware for + * server-to-server callers. + * + * omadia's auth model was built around human-bound sessions (`omadia_session` + * cookie → `createRequireAuth`). A Laravel/PHP integration calling omadia from + * its own server has no human behind it and no cookie to present. This + * middleware is the second authentication method: any Express route — kernel + * or plugin — can mount it and be authenticated by a bearer API key instead. + * + * It deliberately does NOT populate `req.session`. A `SessionClaims` value + * means "a human logged in and these are their claims"; synthesizing one for + * a machine would make every downstream `req.session`-reading route silently + * treat a key as an operator (`role: 'admin'` is hard-typed on those claims). + * The principal lands on its own `req.apiKey` so a route has to opt in. + * + * Error shape follows the public API surface issue #438 established + * (`{ error, message }`), NOT the kernel session gate's `{ code, message }` — + * these routes answer API clients, and `POST /api/public/v1/chat`'s wire + * format must not change. + */ + +import type { NextFunction, Request, RequestHandler, Response } from 'express'; + +import type { ApiKeyScope } from './apiKeyScopes.js'; +import { hasScope } from './apiKeyScopes.js'; +import type { ApiKeyStore } from './apiKeyStore.js'; +import type { AuditLog, AuditStatus } from './auditLog.js'; +import type { RateLimiter } from './rateLimiter.js'; + +/** The authenticated machine caller, attached to the request. */ +export interface ApiKeyPrincipal { + readonly keyId: string; + readonly label?: string; + readonly scopes: readonly ApiKeyScope[]; + readonly rateLimitPerMinute: number; + /** + * Records one audit entry for THIS request. Fire-and-forget — a logging + * failure must never fail the caller's request. The middleware itself + * audits only the outcomes it produces (`rate_limited`, `forbidden`); the + * route handler owns its own outcome, because only it knows whether the + * work actually succeeded. + */ + readonly audit: (status: AuditStatus) => void; +} + +declare module 'express-serve-static-core' { + interface Request { + apiKey?: ApiKeyPrincipal; + } +} + +export interface RequireApiKeyOptions { + readonly apiKeys: ApiKeyStore; + /** Optional — omit to authenticate without enforcing a per-key quota. */ + readonly rateLimiter?: RateLimiter; + /** Optional — omit to authenticate without a usage trail. */ + readonly auditLog?: AuditLog; + /** Scope the guarded routes require. Omit to require authentication only. */ + readonly scope?: ApiKeyScope; + /** Value recorded as `route` in audit entries. Defaults to `req.path`, + * which is relative to the router's mount point and therefore stable. */ + readonly routeLabel?: string; +} + +/** `Authorization: Bearer ` → the token, or undefined. */ +export function bearerToken(req: Request): string | undefined { + const header = req.headers['authorization']; + if (typeof header !== 'string' || !header.startsWith('Bearer ')) { + return undefined; + } + const token = header.slice('Bearer '.length).trim(); + return token.length > 0 ? token : undefined; +} + +export function requireApiKey(opts: RequireApiKeyOptions): RequestHandler { + return async function requireApiKeyHandler( + req: Request, + res: Response, + next: NextFunction, + ): Promise { + const token = bearerToken(req); + if (!token) { + res.status(401).json({ + error: 'unauthorized', + message: 'missing Authorization: Bearer header', + }); + return; + } + + const key = await opts.apiKeys.verify(token); + if (!key) { + res.status(401).json({ error: 'unauthorized', message: 'invalid or revoked API key' }); + return; + } + + // From here on the caller is AUTHENTICATED, so every outcome is + // attributable to a key and worth auditing. Unauthenticated rejections + // above are deliberately not audited — there is no caller identity that + // would make such an entry meaningful. + const route = opts.routeLabel ?? req.path; + const audit = (status: AuditStatus): void => { + if (!opts.auditLog) return; + void opts.auditLog.record({ + keyId: key.id, + route, + method: req.method, + at: Date.now(), + status, + }); + }; + + if (opts.rateLimiter && !opts.rateLimiter.tryConsume(key.id, key.rateLimitPerMinute)) { + audit('rate_limited'); + res.status(429).json({ + error: 'rate_limited', + message: `this key is limited to ${key.rateLimitPerMinute} requests/minute`, + }); + return; + } + + // Scope is checked AFTER the rate limit on purpose: a caller probing for + // scopes it doesn't have should burn quota like any other request. + if (opts.scope !== undefined && !hasScope(key.scopes, opts.scope)) { + audit('forbidden'); + res.status(403).json({ + error: 'forbidden', + message: `this API key is not scoped for '${opts.scope}'`, + }); + return; + } + + req.apiKey = { + keyId: key.id, + ...(key.label ? { label: key.label } : {}), + scopes: key.scopes, + rateLimitPerMinute: key.rateLimitPerMinute, + audit, + }; + next(); + }; +} diff --git a/middleware/packages/harness-api-key-auth/src/secretStorage.ts b/middleware/packages/harness-api-key-auth/src/secretStorage.ts new file mode 100644 index 000000000..51a894430 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/src/secretStorage.ts @@ -0,0 +1,21 @@ +/** + * Issue #439 — the storage surface this package needs, expressed structurally. + * + * The stores originally typed their dependency as `SecretsAccessor` from + * `@omadia/plugin-api`. That would make this package depend on the plugin + * contract package purely to name four methods, and would drag every kernel + * consumer of `requireApiKey` into that dependency too. A structural subset + * keeps the package dependency-free; `SecretsAccessor` (and the kernel's own + * vault accessors) satisfy it without any adapter, because TypeScript + * structural typing already accepts the wider interface. + */ +export interface ApiKeySecretStorage { + /** Returns the stored value, or undefined if absent. */ + get(key: string): Promise; + /** Keys present in this namespace. Never returns values. */ + keys(): Promise; + /** Present only on a write-capable accessor — guard before use. */ + set?(key: string, value: string): Promise; + /** Present only on a write-capable accessor — guard before use. */ + delete?(key: string): Promise; +} diff --git a/middleware/packages/harness-api-key-auth/tsconfig.json b/middleware/packages/harness-api-key-auth/tsconfig.json new file mode 100644 index 000000000..4c3e17230 --- /dev/null +++ b/middleware/packages/harness-api-key-auth/tsconfig.json @@ -0,0 +1,25 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "lib": ["ES2022"], + "outDir": "dist", + "rootDir": "src", + "strict": true, + "noImplicitAny": true, + "strictNullChecks": true, + "noUncheckedIndexedAccess": true, + "noImplicitOverride": true, + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true, + "skipLibCheck": true, + "resolveJsonModule": true, + "declaration": true, + "declarationMap": true, + "sourceMap": true, + "composite": true + }, + "include": ["src/**/*.ts"], + "exclude": ["node_modules", "dist"] +} diff --git a/middleware/packages/harness-channel-api/README.md b/middleware/packages/harness-channel-api/README.md new file mode 100644 index 000000000..4885cde5f --- /dev/null +++ b/middleware/packages/harness-channel-api/README.md @@ -0,0 +1,277 @@ +# Public API Channel (`@omadia/channel-api`) + +Built-in channel plugin that exposes omadia's chat flow over a documented, +public HTTP API (issue #438) so external systems can integrate without +building a channel adapter or driving the operator UI. This document is for +**external API consumers** — if you are looking for how the plugin itself is +built, see the source under `src/`. + +## Tools & capability + +| Surface | What it does | +|---|---| +| `POST /api/public/v1/chat` | The one public route. Send a message, stream the turn back as NDJSON. Self-authenticating (bearer API key) — no session cookie. | +| `GET`/`POST /api/public/v1/admin/keys`, `POST /api/public/v1/admin/keys/:id/revoke` | Key lifecycle (create/list/revoke). **Not part of the public API** — see "Getting an API key" below. | + +## Getting an API key + +API keys are issued and managed by the omadia operator, not by external +callers. The `/api/public/v1/admin/keys` endpoints that create, list, and +revoke keys stay behind the same **operator session cookie** as every other +admin surface in this app — they are not reachable with a bearer token and +are out of scope for an external integrator. If you need a key, ask the +operator running the omadia instance to create one for you from the admin +UI/API and hand you the plaintext token; it is shown to the operator exactly +once, at creation time, and is never recoverable afterwards (only its hash is +stored). + +Mechanically, that session check is enforced by `adminKeysRouter.ts` itself +via the kernel-published `ctx.operatorAuth` accessor (`@omadia/plugin-api`), +not by an absence from `publicPaths.ts` — see `docs/security-architecture.md` +§ 9 for why that distinction matters and the full mechanism. + +## Authentication + +Every call to `/api/public/v1/chat` must carry the key as a bearer token: + +``` +Authorization: Bearer omk_<...> +``` + +- Missing header, malformed header, or an empty token → `401 Unauthorized`. +- A key that doesn't match any stored key, or that has been revoked → `401 + Unauthorized`. Revocation takes effect immediately — a revoked key fails on + its very next call, no propagation delay. +- A key that is valid but not scoped for the route → `403 Forbidden` (see + "Scopes" below). +- Keys are per-caller identities in their own right (not a delegate for a + human end-user) — every request is attributed to the key that made it. + +### Scopes + +Each key carries a set of scopes — `:` strings, or the +global `*` — and every route states the scope it requires. `/chat` requires +`chat:write`, which is also what a key gets when the operator creates it +without naming any scopes, so an integration that only chats never has to +think about this. Ask your operator for `*` only if you actually need every +current and future capability. + +Matching is exact: `chat:write` grants `chat:write` and nothing else. There +are no prefix wildcards (`chat:*`). + +Omit `scopes` entirely to accept the default. Sending `"scopes": []` is a +`400`, not a key with no capabilities — a zero-capability key can never do +anything, so an empty array is treated as a mistake rather than silently +resolved in either direction. + +```json +{ "error": "forbidden", "message": "this API key is not scoped for 'memory:read'" } +``` + +If a key suddenly answers `403` on a route it used to reach, ask your operator +to check the server log for `[api-key-auth] malformed persisted scopes`. A key +whose stored scope set cannot be read is denied every capability rather than +falling back to a default — it still authenticates, so `401` versus `403` +tells you which of the two happened. + +## Server-to-server integration + +This API is designed for calls from *your server*, not from a browser: the +key is a server credential and must never be shipped to a client. There is no +session, no cookie, and no user consent step — the key is the whole identity. + +The credential is a plain bearer token, so any HTTP client works. curl: + +```bash +curl -sS -N -X POST https:///api/public/v1/chat \ + -H "Authorization: Bearer $OMADIA_API_KEY" \ + -H "Content-Type: application/json" \ + -d '{"message": "What is our current MRR?", "conversationId": "crm-42"}' +``` + +PHP (Laravel's HTTP client, streaming the NDJSON line by line): + +```php +use Illuminate\Support\Facades\Http; + +$response = Http::withToken(config('services.omadia.api_key')) + ->withOptions(['stream' => true]) + ->post(config('services.omadia.url').'/api/public/v1/chat', [ + 'message' => 'What is our current MRR?', + 'conversationId' => 'crm-'.$customer->id, + ]); + +if ($response->status() === 401 || $response->status() === 403) { + // 401: unknown or revoked key. 403: the key lacks the `chat:write` scope. + // Neither is retryable — ask the omadia operator for a new key. + throw new RuntimeException($response->json('message')); +} + +$body = $response->toPsrResponse()->getBody(); +$buffer = ''; +$answer = ''; + +while (! $body->eof()) { + $buffer .= $body->read(8192); + + // NDJSON: one complete JSON object per line. Never buffer the whole + // response and json_decode it once — it is a stream, not a document. + while (($newline = strpos($buffer, "\n")) !== false) { + $line = substr($buffer, 0, $newline); + $buffer = substr($buffer, $newline + 1); + if (trim($line) === '') { + continue; + } + + $event = json_decode($line, true); + match ($event['type'] ?? null) { + 'text_delta' => $answer .= $event['text'], + // `done.answer` carries the full text, so a caller that doesn't + // need incremental output can ignore text_delta entirely. + 'done' => $answer = $event['answer'], + 'error' => throw new RuntimeException($event['message']), + // Unknown event types are informational — skip, don't fail. + default => null, + }; + } +} +``` + +Retry advice: `429` is the only status worth retrying automatically (back off +until the 60-second window resets). `401`/`403` mean the credential itself is +wrong and retrying will not fix it. A `200` whose stream ends in an `error` +event means the turn failed, not the credential. + +## `POST /api/public/v1/chat` + +### Request + +```json +{ + "message": "What is our current MRR?", + "conversationId": "optional-caller-chosen-thread-id" +} +``` + +| Field | Type | Required | Notes | +|---|---|---|---| +| `message` | string | yes | Non-empty. | +| `conversationId` | string | no | 1–200 chars. Omit it to start a fresh conversation on every call. When set, reusing the same value on later calls continues the same conversation *for that key* — conversation scope is always namespaced per API key, so two different keys can never collide on the same `conversationId`. | + +A body that fails validation returns `400 Bad Request` with an `issues` +array (Zod's validation error shape). This still counts as an authenticated +call — the key must be valid to reach validation at all. + +### Response — NDJSON streaming, no other format in v1 + +The response is **always** a stream, one JSON object per line +(`Content-Type: application/x-ndjson`), regardless of how short the answer +turns out to be. This is the only response shape v1 supports — there is no +folded, single-JSON-body, non-streaming variant, and none is planned as a +follow-up; this is a deliberate v1 design decision, not a gap. Integrators +should read the body as a stream and parse it line by line rather than +buffering the whole response and calling `JSON.parse` once. + +The events on the stream are the same event vocabulary every other omadia +channel (Teams, Telegram, the operator UI) consumes internally. The ones +relevant to a plain chat integration: + +| `type` | Meaning | +|---|---| +| `text_delta` | Incremental chunk of the assistant's answer text. Concatenate these to reconstruct the streamed answer as it's produced. | +| `done` | Terminal event on success. Carries the full `answer` string plus `toolCalls` / `iterations` counters — read `done.answer` if you only want the final text and don't care about incremental deltas. | +| `error` | Terminal event when the turn failed mid-stream (the orchestrator threw, or the orchestrator/verifier yielded an in-band error event without throwing). Carries a `message`. | +| `verifier` | **Informational, safe to ignore.** Only appears when the omadia instance has verifier mode enabled — one extra event **after** `done`, carrying a `summary` of the post-hoc fact-check. Never blocks or retries the turn; the caller already has the answer by the time this arrives. | + +Note: `agent_bound` — an event some other omadia channel routes emit — is +**not** emitted on this route. `CoreApi.handleTurnStream` (what this plugin +calls directly) never yields it; it's synthesized by the kernel's own +`/api/chat/stream` HTTP route handler, which this plugin doesn't go through. +Integrators porting code from that route should not expect it here. + +The full event union carries additional internal event types (tool-call +tracing, heartbeats, token-usage accounting, and similar) that a simple +integration can safely ignore — treat any `type` you don't recognize as +informational and skip it rather than treating it as an error. `done` and +`error` are the terminal events for the turn itself, but note the `verifier` +row above: a `done` or `error` event is not a guarantee that nothing else +will ever appear on the stream afterward. + +A dropped connection on the caller's side does not fail the underlying turn +server-side; the server simply stops writing once it detects the client is +gone. + +## Rate limiting + +Each API key has its own per-minute request budget (`rateLimitPerMinute`, +set by the operator when the key was created — default 60/min). Exceeding it +returns `429 Too Many Requests`: + +```json +{ "error": "rate_limited", "message": "this key is limited to 60 requests/minute" } +``` + +This is a fixed 60-second window, counted per key, in-memory on the server — +back off and retry after the window resets. A rate-limited call is +authenticated (the key was valid) but never reaches the orchestrator. + +**This limiter is in-memory and per-process.** It resets on every restart +and does not share state across multiple replicas/instances of this app — +if the app is ever scaled horizontally, each replica enforces the budget +independently, so a key's effective ceiling becomes `rateLimitPerMinute × +replica count`. This is a known, accepted v1 trade-off (see +`docs/security-architecture.md` § 9), not an oversight. + +## Error summary + +| Status | `error` | When | +|---|---|---| +| `401` | `unauthorized` | Missing/malformed `Authorization` header, or an unknown/revoked key. | +| `403` | `forbidden` | Valid key, but it is not scoped for this route. | +| `400` | `invalid_request` | Body fails schema validation (e.g. empty `message`). | +| `429` | `rate_limited` | Key is over its per-minute budget. | +| `200` + `error` NDJSON event | `error` | Key and request were valid, but the turn itself failed mid-stream. | + +## Minimal curl example + +```bash +curl -N -X POST https:///api/public/v1/chat \ + -H "Authorization: Bearer omk_" \ + -H "Content-Type: application/json" \ + -d '{"message": "What is our current MRR?"}' +``` + +`-N` disables curl's output buffering so you see each NDJSON line as it +arrives rather than only once the stream closes. A successful call prints a +sequence of lines like: + +``` +{"type":"text_delta","text":"Your "} +{"type":"text_delta","text":"current MRR is..."} +{"type":"done","answer":"Your current MRR is...","toolCalls":0,"iterations":1} +``` + +## Layout + +Standard channel-plugin shape: `src/plugin.ts` wires the routes at +`activate()`; `src/chatRouter.ts` is the public `/chat` route, +`src/adminKeysRouter.ts` the operator-only key-management routes. + +The credential itself is **not** implemented here. Minting, hashing (sha256, +constant-time verified), vault-backed storage, scopes, the per-key rate limit, +the usage audit trail, and the `requireApiKey` middleware this route mounts +all live in `@omadia/api-key-auth` +(`middleware/packages/harness-api-key-auth/`, issue #439) so the kernel and +other plugins can reuse the same implementation. See +`docs/security-architecture.md` § 9 for the full security posture (threat +model, storage design, verification details) and +`docs/middleware-agent-handoff.md` for the implementation handoff notes. + +## Tests + +Central suite: `middleware/test/channelApi/` (router, key store, token, +rate limiter, audit log, manifest, plugin wiring, public-path exemption, +the reuse seam against `@omadia/api-key-auth`, and privacy-guard integration +tests). The auth middleware and the scope model are covered separately in +`middleware/test/auth/requireApiKey.test.ts` and +`middleware/test/auth/apiKeyScopes.test.ts`. diff --git a/middleware/packages/harness-channel-api/manifest.yaml b/middleware/packages/harness-channel-api/manifest.yaml new file mode 100644 index 000000000..971929b5c --- /dev/null +++ b/middleware/packages/harness-channel-api/manifest.yaml @@ -0,0 +1,57 @@ +schema_version: "1" + +identity: + id: "@omadia/channel-api" + name: "Public API Channel" + version: "0.1.0" + kind: "channel" + domain: "api" + description: "Exposes omadia's chat flow over a documented public HTTP API (issue #438) so external systems can integrate without a channel or the operator UI. POST /api/public/v1/chat streams an NDJSON turn, authenticated by a per-key vault-backed API key (constant-time verified, per-key rate limit, usage audit log) instead of the operator session cookie. Key lifecycle (create/list/revoke) is managed under /api/public/v1/admin/keys, which stays behind the normal operator session gate." + authors: + - name: "byte5 GmbH" + email: "info@omadia.ai" + url: "https://omadia.ai" + license: "MIT" + categories: + - "infrastructure" + - "channel" + +compat: + core: ">=1.0 <2.0" + node: ">=20" + +lifecycle: + entry: "dist/plugin.js" + +channel: + transport: + # Plain HTTP ingress (not a 3rd-party webhook) — callers authenticate with + # their own API key rather than a provider-signed payload, so + # verify_signature stays false and the router does its own auth. + kind: "webhook" + routes: + - path: "/api/public/v1/chat" + method: "POST" + - path: "/api/public/v1/admin/keys" + method: "GET" + - path: "/api/public/v1/admin/keys" + method: "POST" + - path: "/api/public/v1/admin/keys/:id/revoke" + method: "POST" + verify_signature: false + capabilities: + - "text" + adapters: + - "text" + # Classic channel — no dispatch_service set, so turns route to the shared + # `chatAgent` like every other non-Omadia-UI channel (see IncomingTurn's + # channelType doc). + +permissions: + filesystem: + scratch: false + # Vault-backed API-key + usage-audit storage in this plugin's OWN secret + # namespace (design decision on issue #438: no DB migration for v1). Never + # the raw key — only its sha256 hash (see apiKeyToken.ts). + secrets: + runtime_write: true diff --git a/middleware/packages/harness-channel-api/package.json b/middleware/packages/harness-channel-api/package.json new file mode 100644 index 000000000..d1756ec28 --- /dev/null +++ b/middleware/packages/harness-channel-api/package.json @@ -0,0 +1,24 @@ +{ + "name": "@omadia/channel-api", + "version": "0.1.0", + "private": true, + "type": "module", + "main": "dist/index.js", + "types": "dist/index.d.ts", + "description": "Public API channel for the byte5 Harness (issue #438). Exposes chat (and later other flows) over a documented HTTP API at /api/public/v1, authenticated by per-key vault-backed API keys instead of an operator session cookie. kind: channel; drives turns via CoreApi.handleTurnStream so responses go through the same orchestrator + privacy-guard pipeline every other channel uses.", + "license": "MIT", + "scripts": { + "build": "tsc", + "typecheck": "tsc --noEmit" + }, + "peerDependencies": { + "@omadia/api-key-auth": "^0.1.0", + "@omadia/channel-sdk": "^0.1.0", + "@omadia/plugin-api": "^0.1.0", + "express": "^5.1.0", + "zod": "^4.0.0" + }, + "engines": { + "node": ">=20" + } +} diff --git a/middleware/packages/harness-channel-api/src/adminKeysRouter.ts b/middleware/packages/harness-channel-api/src/adminKeysRouter.ts new file mode 100644 index 000000000..e57918f45 --- /dev/null +++ b/middleware/packages/harness-channel-api/src/adminKeysRouter.ts @@ -0,0 +1,144 @@ +/** + * Issue #438 — POST/GET /api/public/v1/admin/keys, POST .../:id/revoke. + * + * Deliberately NOT exempted in `middleware/src/auth/publicPaths.ts` (only + * `/api/public/v1/chat` is) — key lifecycle is an operator action, meant to + * stay behind the same session an operator uses everywhere else. `core.registerRouter` + * (the kernel API this router is mounted through, in `plugin.ts`) applies + * only an active/inactive gate, never authentication itself — see the + * `RoutesAccessor` doc comment on `PluginContext` ("the kernel does not + * inject middleware around the contributed router") — but that is not the + * whole picture: `middleware/src/index.ts` mounts a broad `app.use('/api', + * requireAuth, ...)` ahead of `pluginRouteRegistry.mountAll(app)` in boot + * order, so every `/api/*` request, including this plugin's, already passes + * through that session gate unless its path is listed in `publicPaths.ts` + * (only `.../chat` is). A previous revision of this file's comment claimed + * the publicPaths omission alone left these routes reachable by any + * anonymous caller; a runtime reproduction mirroring the real mount order + * disproved that — see `docs/security-architecture.md` § 9 for the full + * account and why that coverage, while real, was an *implicit* invariant + * worth replacing with an explicit one. + * + * The middleware below adds that explicit, non-implicit gate: it calls + * `ctx.operatorAuth` (`@omadia/plugin-api`, kernel-published, wraps the + * exact same verification logic `requireAuth` uses for `/api/v1/*`) on every + * request, BEFORE any handler runs, and fails closed — refuses to serve at + * all (`503`) — if the host never wired an `operatorAuth` accessor into this + * plugin's context. Missing/invalid session → `401`, in the same + * `{code, message}` shape `requireAuth` itself returns. That guarantee no + * longer depends on mount order or on this path staying out of + * `publicPaths.ts` — it travels with the router regardless. + * + * Issue #439 added `scopes` to creation and to the listing, on top of — never + * instead of — that operator-session gate. Omitting `scopes` yields + * `LEGACY_DEFAULT_SCOPES` (the exact capability set a key minted before scopes + * existed had), so existing operator tooling that posts `{label}` keeps + * producing working keys. + */ + +import { Router } from 'express'; +import type { NextFunction, Request, Response } from 'express'; +import type { OperatorAuthAccessor } from '@omadia/plugin-api'; +import { z } from 'zod'; +import type { ApiKeyStore } from '@omadia/api-key-auth'; +import { isValidScope } from '@omadia/api-key-auth'; + +const CreateKeyRequestSchema = z.object({ + label: z.string().min(1).max(120).optional(), + rateLimitPerMinute: z.number().int().positive().max(6000).optional(), + // Validated here (400 on a typo) rather than letting the store's + // `assertValidScopes` throw into a 500 — operator input is user input. + // + // `.min(1)`: an explicitly-supplied empty array is rejected, never resolved + // to a default. A zero-capability key is not a useful thing to mint, so `[]` + // is far more likely an operator slip or a buggy client than a deliberate + // request — and the alternatives are both worse. Granting the legacy default + // would hand chat access to someone who asked for none, while minting a key + // with no scopes would produce a credential that silently 403s forever, + // because `normalizeScopes` in `@omadia/api-key-auth` reads a persisted `[]` + // back as corruption and denies everything. Omit the field to accept the + // default. + scopes: z + .array(z.string().refine(isValidScope, 'must be `:` or `*`')) + .min(1, 'scopes must not be empty; omit the field entirely to accept the default') + .optional(), +}); + +export function createAdminKeysRouter( + apiKeys: ApiKeyStore, + operatorAuth: OperatorAuthAccessor | undefined, +): Router { + const router = Router(); + + // Fail-closed operator-session gate, applied to every route below. This + // is the ENTIRE auth story for this router — see the module doc comment + // above for why relying on publicPaths.ts alone was wrong. + router.use((req: Request, res: Response, next: NextFunction) => { + if (!operatorAuth) { + // No kernel-published operatorAuth (e.g. an older host, or a narrow + // test/migration context that never wired one) — refuse to serve + // rather than silently mounting with no auth check at all. + res.status(503).json({ + code: 'operator_auth.unavailable', + message: 'operator auth unavailable', + }); + return; + } + const cookieHeader = req.headers.cookie; + void operatorAuth.hasValidSession(cookieHeader).then( + (valid) => { + if (valid) { + next(); + return; + } + if (!cookieHeader) { + res.status(401).json({ code: 'auth.missing', message: 'no session' }); + return; + } + res + .status(401) + .json({ code: 'auth.invalid', message: 'session invalid or expired' }); + }, + () => { + // hasValidSession is documented to never throw, but a broken + // implementation must not crash the request — treat it as invalid. + res + .status(401) + .json({ code: 'auth.invalid', message: 'session invalid or expired' }); + }, + ); + }); + + router.get('/', async (_req: Request, res: Response) => { + res.json({ keys: await apiKeys.list() }); + }); + + router.post('/', async (req: Request, res: Response) => { + const parsed = CreateKeyRequestSchema.safeParse(req.body ?? {}); + if (!parsed.success) { + res.status(400).json({ error: 'invalid_request', issues: parsed.error.issues }); + return; + } + const created = await apiKeys.create(parsed.data); + // The plaintext token is returned exactly once, here. The operator must + // copy it now — only its hash is ever stored. + res.status(201).json({ key: created.record, token: created.token }); + }); + + router.post('/:id/revoke', async (req: Request, res: Response) => { + const rawId = req.params['id']; + const id = Array.isArray(rawId) ? rawId[0] : rawId; + if (!id) { + res.status(400).json({ error: 'invalid_request', message: 'missing key id' }); + return; + } + const revoked = await apiKeys.revoke(id); + if (!revoked) { + res.status(404).json({ error: 'not_found', id }); + return; + } + res.json({ key: revoked }); + }); + + return router; +} diff --git a/middleware/packages/harness-channel-api/src/chatRouter.ts b/middleware/packages/harness-channel-api/src/chatRouter.ts new file mode 100644 index 000000000..e8108b521 --- /dev/null +++ b/middleware/packages/harness-channel-api/src/chatRouter.ts @@ -0,0 +1,220 @@ +/** + * Issue #438 — POST /api/public/v1/chat. + * + * Public, self-authenticating chat ingress. Authentication, per-key rate + * limiting and the usage-audit entry for rejected calls are all delegated to + * `requireApiKey` from `@omadia/api-key-auth` (issue #439) — this route no + * longer parses bearer headers or verifies hashes itself, so there is exactly + * one API-key auth implementation in the codebase. The route requires the + * `chat:write` scope, which every key (including every key minted before + * scopes existed) carries by default. + * + * Past the guard, the turn is driven via `CoreApi.handleTurnStream` — the + * SAME orchestrator dispatch every other channel uses, so PII masking + * (privacy-guard), memory, and the knowledge graph all apply exactly as they + * do for Teams/Telegram/Omadia UI. No second response-masking path here. + * + * NDJSON framing (one JSON event per line) mirrors `src/routes/chat.ts`'s + * `/chat/stream` — this plugin cannot import that kernel route module + * directly (plugins only depend on `@omadia/channel-sdk` / `@omadia/plugin-api` + * / `@omadia/api-key-auth` / express), so the tiny `writeEvent` helper is + * duplicated rather than imported. + */ + +import { createHash, randomUUID } from 'node:crypto'; + +import { Router } from 'express'; +import type { Request, Response } from 'express'; +import { z } from 'zod'; +import type { CoreApi, IncomingTurn } from '@omadia/channel-sdk'; +import type { ApiKeyStore, AuditLog, RateLimiter } from '@omadia/api-key-auth'; +import { CHAT_WRITE_SCOPE, requireApiKey } from '@omadia/api-key-auth'; + +/** Relative to the router's mount prefix (`/api/public/v1`). */ +export const CHAT_ROUTE = '/chat'; + +const ChatRequestSchema = z.object({ + message: z.string().min(1, 'message must be a non-empty string'), + /** Caller-supplied thread id. Omitted → a fresh conversation per call. */ + conversationId: z.string().min(1).max(200).optional(), +}); + +/** NDJSON framing — see `src/routes/chat.ts`'s `writeEvent` (same shape). */ +function writeEvent(res: Response, event: unknown): void { + res.write(`${JSON.stringify(event)}\n`); +} + +/** + * True for an in-band `{type:'error', ...}` event forwarded from the + * orchestrator stream. These complete the async iterator normally — nothing + * throws — so the caller has to inspect the events themselves to notice the + * turn failed (see the `sawInBandError` tracking below). + */ +function isErrorEvent(event: unknown): boolean { + return ( + typeof event === 'object' && + event !== null && + 'type' in event && + (event as { type: unknown }).type === 'error' + ); +} + +/** + * Derives the internal conversationId from the (fixed, never caller- + * controlled) key id and the caller-supplied conversationId. + * + * This is NOT the same as namespacing via plain string concatenation + * (`${keyId}:${callerConversationId}`), which was the original approach and + * is unsafe: `CoreApi.handleTurnStream` folds this value into a scope string + * that downstream `SessionLogger`'s `sanitizeScope` mangles — punctuation + * runs (any char outside `[a-zA-Z0-9_-]`) collapse to a single `-`, the + * result is lowercased, and it's truncated to 80 chars. Two distinct + * caller-supplied ids can therefore land on the IDENTICAL sanitized scope + * under the same key — e.g. `"case/a"` and `"case?a"` both sanitize to + * `...-case-a`, and two long ids that only differ past the truncation cutoff + * sanitize identically too. Either lets one conversation thread recall + * another thread's memory/graph content under the same key. + * + * A fixed-width (64 hex chars, well under the 80-char cap), collision- + * resistant hash of the same inputs sidesteps sanitizeScope's exact + * transform rules entirely: hex digest output is already lowercase + * alphanumeric, so nothing about it can be mangled or truncated into + * colliding with a different digest. + */ +export function internalConversationId(keyId: string, callerConversationId: string): string { + return createHash('sha256').update(`${keyId}:${callerConversationId}`).digest('hex'); +} + +export interface ApiChatRouterDeps { + /** Channel id this plugin registered under (== `ctx.agentId`). Scopes the + * orchestrator's conversation-id derivation, same as every other channel. */ + channelId: string; + core: Pick; + apiKeys: ApiKeyStore; + rateLimiter: RateLimiter; + auditLog: AuditLog; +} + +export function createApiChatRouter(deps: ApiChatRouterDeps): Router { + const router = Router(); + + router.post( + CHAT_ROUTE, + requireApiKey({ + apiKeys: deps.apiKeys, + rateLimiter: deps.rateLimiter, + auditLog: deps.auditLog, + scope: CHAT_WRITE_SCOPE, + // Pinned rather than derived from `req.path` so the audit trail keeps + // reading `/chat` regardless of where the router gets mounted. + routeLabel: CHAT_ROUTE, + }), + async (req: Request, res: Response) => { + // `requireApiKey` never calls next() without setting this; the guard is + // for the type, not for a reachable runtime state. + const key = req.apiKey; + if (!key) { + res.status(401).json({ error: 'unauthorized', message: 'invalid or revoked API key' }); + return; + } + + const parsed = ChatRequestSchema.safeParse(req.body); + if (!parsed.success) { + key.audit('invalid_request'); + res.status(400).json({ error: 'invalid_request', issues: parsed.error.issues }); + return; + } + + // NDJSON streaming — see the doc comment at the top of this file. + res.status(200); + res.setHeader('Content-Type', 'application/x-ndjson; charset=utf-8'); + res.setHeader('Cache-Control', 'no-cache, no-transform'); + res.setHeader('X-Accel-Buffering', 'no'); + res.flushHeaders(); + + let clientGone = false; + res.on('close', () => { + if (!res.writableEnded) clientGone = true; + }); + const safeWrite = (event: unknown): void => { + if (!clientGone) writeEvent(res, event); + }; + + try { + const turn: IncomingTurn = { + channelId: deps.channelId, + // Namespaced by key identity: CoreApi derives its scope as + // `${channelId}::${conversationId}` (same channelId for every key + // hitting this plugin), and same-scope recall does NOT check + // userRef. An unnamespaced caller-supplied conversationId would let + // two different API keys collide on the exact same core-side scope + // by sending the same conversationId — cross-key context/transcript + // leakage. Hashing `key.keyId` (a per-key UUID, never + // caller-controlled) together with the caller-supplied + // conversationId — rather than plain string concatenation — makes + // both that cross-key collision AND same-key collisions via lossy + // downstream sanitization structurally impossible; see + // `internalConversationId`'s doc comment for why concatenation + // alone isn't enough. + conversationId: internalConversationId( + key.keyId, + parsed.data.conversationId ?? randomUUID(), + ), + // Design decision (issue #438): the key IS its own identity — not a + // delegate for a human end-user. No impersonation surface. + // + // Investigated (post-review): raw `key:` is never resolved to + // a knowledge-graph `omadiaUserId` before dispatch. Confirmed this + // is NOT a plugin-specific regression — it matches the documented, + // universal contract: + // - `ChannelUserRef.id` is typed "channel-native user id (opaque + // to core)" (harness-channel-sdk/src/incoming.ts) — no channel + // is expected to pre-resolve it. + // - `orchestratorDispatcher.ts` passes `input.userRef.id` straight + // through as `userId` with no resolution step, for every channel. + // - `resolveOrCreateChannelIdentity` is only ever called from the + // browser-login flow (src/index.ts, `/api/v1/auth`) to cache an + // `omadiaUserId` in the session JWT — it is not a per-turn, + // per-channel pattern. Even the one channel with that cached id + // available (omadia-ui-channel/canvasConnection.ts) uses the raw + // `session.subject`, not `session.omadiaUserId`, for `userRef.id`. + // - `src/routes/chat.ts`'s `resolveUserId()` already documents the + // same behaviour for Teams and generic HTTP callers: unresolved + // ids are "advisory metadata only" because + // `NeonKnowledgeGraph.ingestRun` throws when no matching + // User-Cluster node exists (by design — it never auto-creates + // one), so the run-trace ingest is dropped while the Session/Turn + // transcript still persists fine via `ingestTurn` (no such check). + // Introducing per-key `resolveOrCreateChannelIdentity` resolution + // here would be a NEW pattern no other channel implements, not an + // alignment with an established one. + userRef: { + kind: 'custom', + id: `key:${key.keyId}`, + ...(key.label ? { displayName: key.label } : {}), + }, + text: parsed.data.message, + }; + // The orchestrator (or, with verifier mode on, the verifier wrapper) + // can yield an in-band `{type:'error', ...}` event on this already-open + // 200 stream WITHOUT throwing — the async iterator completes normally. + // Track whether one was seen so we don't record 'ok' for a turn that + // actually failed (same bug class as issue #403). + let sawInBandError = false; + for await (const event of deps.core.handleTurnStream(turn)) { + if (isErrorEvent(event)) sawInBandError = true; + safeWrite(event); + } + key.audit(sawInBandError ? 'error' : 'ok'); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (!clientGone) writeEvent(res, { type: 'error', message }); + key.audit('error'); + } finally { + res.end(); + } + }, + ); + + return router; +} diff --git a/middleware/packages/harness-channel-api/src/index.ts b/middleware/packages/harness-channel-api/src/index.ts new file mode 100644 index 000000000..5e14a83ce --- /dev/null +++ b/middleware/packages/harness-channel-api/src/index.ts @@ -0,0 +1,17 @@ +// @omadia/channel-api — public barrel. +// +// The API-key primitives (mint/hash/verify, store, rate limiter, audit log, +// `requireApiKey`) moved to `@omadia/api-key-auth` in issue #439 so the +// kernel can consume them without importing a channel plugin. Import them +// from there — this package no longer re-exports them, because a second +// import path is exactly how a "single implementation" quietly stops being +// one. +export { activate, API_PREFIX } from './plugin.js'; + +export { + createApiChatRouter, + CHAT_ROUTE, + type ApiChatRouterDeps, +} from './chatRouter.js'; + +export { createAdminKeysRouter } from './adminKeysRouter.js'; diff --git a/middleware/packages/harness-channel-api/src/plugin.ts b/middleware/packages/harness-channel-api/src/plugin.ts new file mode 100644 index 000000000..575b2ddf5 --- /dev/null +++ b/middleware/packages/harness-channel-api/src/plugin.ts @@ -0,0 +1,84 @@ +/** + * @omadia/channel-api — issue #438, public API channel. + * + * `kind: channel`. Registers ONE router under `/api/public/v1`: + * - `POST /chat` — public, self-authenticating (API key). The only + * path `publicPaths.ts` exempts from the session + * gate for this plugin. + * - `/admin/keys` — key lifecycle (create/list/revoke). NOT + * exempted in `publicPaths.ts`, and (as of the + * issue #438 follow-up) actually enforced: gated + * by `ctx.operatorAuth` inside + * `adminKeysRouter.ts` itself, since + * `core.registerRouter` applies no auth of its + * own — see that file's doc comment. + * + * Follows the same "built-in package, activate(ctx, core)" shape as + * `@omadia/ui-channel` — see that package for the template this mirrors. + * + * Issue #439: the key store, rate limiter, audit log and the bearer-auth + * middleware now live in `@omadia/api-key-auth` so the kernel can reuse them + * too. This plugin only wires them to `ctx.secrets` and mounts the routes. + */ + +import { Router } from 'express'; +import type { ChannelHandle, CoreApi } from '@omadia/channel-sdk'; +import type { PluginContext } from '@omadia/plugin-api'; +import { createApiKeyStore, createAuditLog, createRateLimiter } from '@omadia/api-key-auth'; + +import { createAdminKeysRouter } from './adminKeysRouter.js'; +import { createApiChatRouter } from './chatRouter.js'; + +/** Mount prefix this plugin registers under. `publicPaths.ts` exempts ONLY + * `${API_PREFIX}/chat` from the session gate — keep the two in sync. */ +export const API_PREFIX = '/api/public/v1'; + +export async function activate( + ctx: PluginContext, + core: CoreApi, +): Promise { + ctx.log('activating @omadia/channel-api'); + + if (!ctx.secrets.set || !ctx.secrets.delete) { + // Defensive: the manifest declares permissions.secrets.runtime_write, so + // this should never happen on a correctly-wired core. Degrade to inert + // rather than crash the whole plugin-load pass (mirrors how + // omadia-ui-channel degrades to discovery-only when its WS registry is + // absent). + ctx.log( + '[channel-api] ctx.secrets has no write access — routes NOT mounted (check permissions.secrets.runtime_write in manifest.yaml)', + ); + return { + async close(): Promise {}, + }; + } + + const apiKeys = createApiKeyStore(ctx.secrets); + const auditLog = createAuditLog(ctx.secrets); + const rateLimiter = createRateLimiter(); + + const router = Router(); + router.use( + createApiChatRouter({ + channelId: ctx.agentId, + core, + apiKeys, + rateLimiter, + auditLog, + }), + ); + router.use('/admin/keys', createAdminKeysRouter(apiKeys, ctx.operatorAuth)); + + core.registerRouter(ctx.agentId, API_PREFIX, router); + ctx.log( + `[channel-api] chat route at POST ${API_PREFIX}/chat, key admin at ${API_PREFIX}/admin/keys`, + ); + + return { + async close(): Promise { + ctx.log('deactivating @omadia/channel-api'); + // Routes are torn down by the kernel per channelId (CoreApi contract) — + // nothing else to release (no timers, no sockets). + }, + }; +} diff --git a/middleware/packages/harness-channel-api/tsconfig.json b/middleware/packages/harness-channel-api/tsconfig.json new file mode 100644 index 000000000..4c3e17230 --- /dev/null +++ b/middleware/packages/harness-channel-api/tsconfig.json @@ -0,0 +1,25 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "lib": ["ES2022"], + "outDir": "dist", + "rootDir": "src", + "strict": true, + "noImplicitAny": true, + "strictNullChecks": true, + "noUncheckedIndexedAccess": true, + "noImplicitOverride": true, + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true, + "skipLibCheck": true, + "resolveJsonModule": true, + "declaration": true, + "declarationMap": true, + "sourceMap": true, + "composite": true + }, + "include": ["src/**/*.ts"], + "exclude": ["node_modules", "dist"] +} diff --git a/middleware/packages/plugin-api/src/pluginContext.ts b/middleware/packages/plugin-api/src/pluginContext.ts index cbfb85269..7db19cd32 100644 --- a/middleware/packages/plugin-api/src/pluginContext.ts +++ b/middleware/packages/plugin-api/src/pluginContext.ts @@ -200,6 +200,20 @@ export interface PluginContext { * — a Hub plugin may land on an older core without the broker. */ readonly oauthTokens?: OAuthTokensAccessor; + /** Issue #438 follow-up — kernel-published operator-session verifier. Present + * iff the host wires a session-verification backend into + * `createPluginContext` (production always does; older kernels or narrow + * test/migration contexts may not). Lets a plugin gate its OWN admin-only + * HTTP surface behind the SAME operator session cookie the kernel's own + * `requireAuth` middleware checks, WITHOUT re-implementing (and risking + * drifting from) that verification logic. Per {@link RoutesAccessor}'s doc + * comment the kernel does NOT inject auth middleware around a contributed + * router — a plugin whose admin surface needs operator-only access MUST + * check this itself, and MUST fail closed (refuse to serve, never silently + * mount unauthenticated) when it is undefined. Guard with `if + * (ctx.operatorAuth)`. */ + readonly operatorAuth?: OperatorAuthAccessor; + /** Report an operator-facing action status (e.g. "not connected yet"). The * kernel holds the latest value per plugin and the admin UI renders it as a * badge on the plugin card + a banner on the detail page that clears when @@ -679,6 +693,27 @@ export interface OAuthTokensAccessor { export type OAuthTokenErrorCode = 'not_connected' | 'refresh_failed'; +/** + * Issue #438 follow-up — kernel-published operator-session verifier (see the + * `PluginContext.operatorAuth` doc comment for the full contract). The kernel + * implementation reuses the EXACT SAME verification logic as its own + * `requireAuth` middleware — same cookie name, same signing key, same + * Entra-whitelist rule — so there is exactly one code path that decides + * session validity, never two that can drift apart. Deliberately decoupled + * from Express (this package never imports express types, see + * {@link RoutesAccessor}): the caller hands over the raw `Cookie` header + * string, not a `Request`. + */ +export interface OperatorAuthAccessor { + /** + * Resolves `true` iff the raw `Cookie` request header carries a currently + * valid operator session. Never throws — a missing header, a malformed + * cookie, an expired/invalid session token, or (for Entra-issued sessions) + * an email that fell off the admin whitelist all resolve `false`. + */ + hasValidSession(cookieHeader: string | undefined): Promise; +} + export class OAuthTokenError extends Error { readonly code: OAuthTokenErrorCode; constructor(code: OAuthTokenErrorCode, message: string) { diff --git a/middleware/src/auth/operatorAuthAccessor.ts b/middleware/src/auth/operatorAuthAccessor.ts new file mode 100644 index 000000000..12373e062 --- /dev/null +++ b/middleware/src/auth/operatorAuthAccessor.ts @@ -0,0 +1,35 @@ +import { parse as parseCookieHeader } from 'cookie'; +import type { OperatorAuthAccessor } from '@omadia/plugin-api'; + +import { evaluateSessionToken, SESSION_COOKIE } from './requireAuth.js'; +import type { EmailWhitelist } from './whitelist.js'; + +/** + * Issue #438 follow-up — kernel-side implementation of the plugin-facing + * `ctx.operatorAuth` accessor. Wraps `evaluateSessionToken`, the EXACT SAME + * session-verification logic `requireAuth` runs for every gated + * `/api/v1/*` route, so a plugin that needs an operator-only admin surface + * (e.g. `@omadia/channel-api`'s `/admin/keys`) can reuse it instead of + * re-implementing — and risking drifting from — the kernel's own session + * rules. There is exactly one code path that decides session validity; this + * is a thin adapter from "raw Cookie header" to that path, not a second one. + */ +export function createOperatorAuthAccessor(deps: { + signingKey: Uint8Array; + whitelist: EmailWhitelist; +}): OperatorAuthAccessor { + return { + async hasValidSession(cookieHeader: string | undefined): Promise { + if (!cookieHeader) return false; + let parsed: Record; + try { + parsed = parseCookieHeader(cookieHeader); + } catch { + // Malformed Cookie header — never throw out of this accessor. + return false; + } + const result = await evaluateSessionToken(parsed[SESSION_COOKIE], deps); + return result.ok; + }, + }; +} diff --git a/middleware/src/auth/publicPaths.ts b/middleware/src/auth/publicPaths.ts index e54ca2cb2..903c9a52d 100644 --- a/middleware/src/auth/publicPaths.ts +++ b/middleware/src/auth/publicPaths.ts @@ -46,6 +46,22 @@ export const STATIC_PUBLIC_PATHS: readonly RegExp[] = [ // only a Teams SSO token exists. Plugins exposing sensitive data validate // that token themselves. /^\/p\/[^/]+(?:\/|$|\?)/, + // Issue #438 — the public API channel's chat ingress. A caller presents a + // per-key API key (Authorization: Bearer) instead of a session cookie; the + // `@omadia/channel-api` plugin mounts `requireApiKey` from + // `@omadia/api-key-auth` (issue #439 — constant-time hash compare, per-key + // rate limit, `chat:write` scope) on this route, and that IS its + // authentication. + // Deliberately narrow to `/chat` only: the sibling `/admin/keys` key- + // lifecycle routes under the same `/api/public/v1` prefix are NOT listed + // here, so they stay behind this same session gate like every other admin + // surface (see `src/routes/adminSettings.ts`). + // NOTE for whoever mounts `requireApiKey` next: an API-key-authenticated + // route needs an entry here to be reachable at all, and every entry is a + // new unauthenticated-until-the-handler-says-otherwise surface. Add the + // narrowest regex that covers the one route, never a prefix that also + // catches its siblings. + /^\/api\/public\/v1\/chat(?:\/|$|\?)/, ]; /** diff --git a/middleware/src/auth/requireAuth.ts b/middleware/src/auth/requireAuth.ts index 0dbc41caf..a6ce2330c 100644 --- a/middleware/src/auth/requireAuth.ts +++ b/middleware/src/auth/requireAuth.ts @@ -12,6 +12,50 @@ declare module 'express-serve-static-core' { } } +/** Outcome of {@link evaluateSessionToken} — mirrors the response shape + * `requireAuth` sends on failure (`{code, message}`) so every caller of the + * shared evaluation (the Express middleware below AND the plugin-facing + * `ctx.operatorAuth` accessor) reports failures identically. */ +export type SessionEvaluation = + | { readonly ok: true; readonly claims: SessionClaims } + | { + readonly ok: false; + readonly code: 'auth.missing' | 'auth.invalid' | 'auth.not_whitelisted'; + readonly message: string; + }; + +/** + * The single code path that decides whether a session token is currently + * valid — extracted so `requireAuth` (below) and the kernel's + * `ctx.operatorAuth` accessor (`operatorAuthAccessor.ts`) can never drift + * apart on what "a valid operator session" means. Same rule either caller + * uses: verify the JWT against `signingKey`, then apply the Entra-whitelist + * gate (local-provider sessions skip it — see the doc comment below). + */ +export async function evaluateSessionToken( + token: string | undefined, + deps: { signingKey: Uint8Array; whitelist: EmailWhitelist }, +): Promise { + if (!token) { + return { ok: false, code: 'auth.missing', message: 'no session' }; + } + try { + const claims = await verifySession(token, deps.signingKey); + // Whitelist gate applies only to OIDC-managed identities. Local + // users rely on the users-table status (already checked at login). + if (claims.provider === 'entra' && !deps.whitelist.isAllowed(claims.email)) { + return { + ok: false, + code: 'auth.not_whitelisted', + message: 'email no longer authorised', + }; + } + return { ok: true, claims }; + } catch { + return { ok: false, code: 'auth.invalid', message: 'session invalid or expired' }; + } +} + /** * Gate for /api/v1/* routes (except /api/v1/auth/*). * @@ -60,24 +104,13 @@ export function createRequireAuth(deps: { } const cookies = (req as Request & { cookies?: Record }).cookies; const token = cookies ? cookies[SESSION_COOKIE] : undefined; - if (!token) { - res.status(401).json({ code: 'auth.missing', message: 'no session' }); + const result = await evaluateSessionToken(token, deps); + if (!result.ok) { + const status = result.code === 'auth.not_whitelisted' ? 403 : 401; + res.status(status).json({ code: result.code, message: result.message }); return; } - try { - const claims = await verifySession(token, deps.signingKey); - // Whitelist gate applies only to OIDC-managed identities. Local - // users rely on the users-table status (already checked at login). - if (claims.provider === 'entra' && !deps.whitelist.isAllowed(claims.email)) { - res - .status(403) - .json({ code: 'auth.not_whitelisted', message: 'email no longer authorised' }); - return; - } - req.session = claims; - next(); - } catch { - res.status(401).json({ code: 'auth.invalid', message: 'session invalid or expired' }); - } + req.session = result.claims; + next(); }; } diff --git a/middleware/src/channels/channelRegistry.ts b/middleware/src/channels/channelRegistry.ts index 1b917a5e9..f9968a99b 100644 --- a/middleware/src/channels/channelRegistry.ts +++ b/middleware/src/channels/channelRegistry.ts @@ -11,6 +11,7 @@ import type { JobScheduler } from '../plugins/jobScheduler.js'; import type { PluginCatalog } from '../plugins/manifestLoader.js'; import type { SecretVault } from '../secrets/vault.js'; import type { NativeToolRegistry } from '@omadia/orchestrator'; +import type { OperatorAuthAccessor } from '@omadia/plugin-api'; import type { ChannelHandle, @@ -47,6 +48,12 @@ export interface ChannelRegistryDeps { flowPublicBaseUrl?: string; /** Spec 004 — backing store for `ctx.status`; cleared on deactivate. */ pluginStatusRegistry?: PluginStatusRegistry; + /** Issue #438 follow-up — kernel-published `ctx.operatorAuth`, threaded + * straight into every `createPluginContext`. Optional so narrow test + * contexts can omit it (an admin router relying on it then fails closed). + * This is how `@omadia/channel-api`'s `/admin/keys` router — a channel + * plugin — gets a real operator-session check. */ + operatorAuth?: OperatorAuthAccessor; /** * US4 event-emit catalog. A channel plugin that declares `event_emit` capabilities (e.g. Teams * emitting `teams.message.posted`) has them registered here on activate, so `ctx.events.emit` is @@ -119,6 +126,7 @@ export class DefaultChannelRegistry implements ChannelRegistry { flowSigningKey: this.deps.flowSigningKey, flowPublicBaseUrl: this.deps.flowPublicBaseUrl, pluginStatusRegistry: this.deps.pluginStatusRegistry, + operatorAuth: this.deps.operatorAuth, }); const handle = await impl.activate(ctx, this.deps.coreApi); diff --git a/middleware/src/index.ts b/middleware/src/index.ts index e73812f4b..88547f3fd 100644 --- a/middleware/src/index.ts +++ b/middleware/src/index.ts @@ -163,6 +163,7 @@ import { } from './pairing/mdns.js'; import { publicPaths } from './auth/publicPaths.js'; import { createRequireAuth } from './auth/requireAuth.js'; +import { createOperatorAuthAccessor } from './auth/operatorAuthAccessor.js'; import { assembleDevPlatform, mountDevPlatform } from './devplatform/wireDevPlatform.js'; import { createChatDevJobOrchestratorTools } from './devplatform/chatDevJobToolWiring.js'; import { isPermittedLauncher } from './routes/devPlatformShared.js'; @@ -666,6 +667,24 @@ async function main(): Promise { const flowPublicBaseUrl = config.FLOW_PUBLIC_BASE_URL ?? config.PUBLIC_BASE_URL; + // Admin email whitelist — resolved here (ahead of its original A.1 spot + // below) because it's now ALSO a dependency of `operatorAuth` + // (`ctx.operatorAuth`, issue #438 follow-up), which the plugin runtimes + // constructed further down need at construction time. The `requireAuth` + // Express middleware built at the original A.1 site still uses this same + // instance — nothing there changes. + const emailWhitelist = new EmailWhitelist(config.ADMIN_ALLOWED_EMAILS); + // Issue #438 follow-up — kernel-published `ctx.operatorAuth`. Wraps the + // EXACT SAME session-verification logic `requireAuth` uses (see + // `operatorAuthAccessor.ts`), so a plugin's admin-only HTTP surface (e.g. + // `@omadia/channel-api`'s `/admin/keys`) can check the real operator + // session without re-implementing it. Threaded into every plugin runtime + // below so any plugin — not just channel plugins — can use it. + const operatorAuth = createOperatorAuthAccessor({ + signingKey: sessionSigningKey, + whitelist: emailWhitelist, + }); + const installedRegistry = new FileInstalledRegistry( INSTALLED_REGISTRY_PATH, ); @@ -845,6 +864,7 @@ async function main(): Promise { flowSigningKey: sessionSigningKey, flowPublicBaseUrl, pluginStatusRegistry, + operatorAuth, oauthConnectionTracker, canvasOutputRegistry, eventCatalogRegistry, @@ -918,6 +938,7 @@ async function main(): Promise { flowSigningKey: sessionSigningKey, flowPublicBaseUrl, pluginStatusRegistry, + operatorAuth, oauthConnectionTracker, selfExtendRegistry, extensionStore, @@ -1348,9 +1369,10 @@ async function main(): Promise { }); // ── Admin auth (A.1) ────────────────────────────────────────────────────── - // `sessionSigningKey` is resolved earlier (right after the vault loads) so - // the plugin runtimes can also use it for `ctx.flows` state signing. - const emailWhitelist = new EmailWhitelist(config.ADMIN_ALLOWED_EMAILS); + // `sessionSigningKey` (and `emailWhitelist`) are resolved earlier (right + // after the vault loads) so the plugin runtimes can also use them — + // `sessionSigningKey` for `ctx.flows` state signing, both together for + // `ctx.operatorAuth` (issue #438 follow-up). if (emailWhitelist.isEmpty()) { console.warn( '[middleware] ⚠ ADMIN_ALLOWED_EMAILS is empty — every sign-in will 403 until the secret is set', @@ -4341,6 +4363,7 @@ async function main(): Promise { flowSigningKey: sessionSigningKey, flowPublicBaseUrl, pluginStatusRegistry, + operatorAuth, eventCatalogRegistry, resolver: channelPluginResolver, coreApi: channelCoreApi, diff --git a/middleware/src/platform/pluginContext.ts b/middleware/src/platform/pluginContext.ts index 9e92b3905..a901afcab 100644 --- a/middleware/src/platform/pluginContext.ts +++ b/middleware/src/platform/pluginContext.ts @@ -41,6 +41,7 @@ import { type NotificationsAccessor, type OAuthTokensAccessor, OAuthTokenError, + type OperatorAuthAccessor, type PluginContext, type RoutesAccessor, type PluginActionStatus, @@ -168,6 +169,11 @@ export interface CreatePluginContextOptions { /** Spec 004 — kernel store backing `ctx.status`. Optional: when absent the * accessor is a no-op (test/migration contexts don't surface status). */ pluginStatusRegistry?: PluginStatusRegistry; + /** Issue #438 follow-up — kernel-published `ctx.operatorAuth`. Optional: + * absent in narrow test/migration contexts, in which case `ctx.operatorAuth` + * is `undefined` and any plugin admin-router relying on it MUST fail closed + * (see the `PluginContext.operatorAuth` doc comment). */ + operatorAuth?: OperatorAuthAccessor; logger?: (...args: unknown[]) => void; } @@ -797,6 +803,7 @@ export function createPluginContext( ...(flows ? { flows } : {}), ...(oauthTokens ? { oauthTokens } : {}), ...(events ? { events } : {}), + ...(opts.operatorAuth ? { operatorAuth: opts.operatorAuth } : {}), status, log, }; diff --git a/middleware/src/plugins/dynamicAgentRuntime.ts b/middleware/src/plugins/dynamicAgentRuntime.ts index 0b203f726..3bde55f26 100644 --- a/middleware/src/plugins/dynamicAgentRuntime.ts +++ b/middleware/src/plugins/dynamicAgentRuntime.ts @@ -17,6 +17,7 @@ import { canvasOutputToolIds } from '../platform/canvasOutputRegistry.js'; import { deterministicActionToolIds } from '../platform/deterministicActionRegistry.js'; import { eventEmitIds } from '../platform/eventCatalogRegistry.js'; import { createPluginContext } from '../platform/pluginContext.js'; +import type { OperatorAuthAccessor } from '@omadia/plugin-api'; import type { PluginRouteRegistry } from '../platform/pluginRouteRegistry.js'; import type { NotificationRouter } from '../platform/notificationRouter.js'; import type { PluginStatusRegistry } from '../platform/pluginStatusRegistry.js'; @@ -169,6 +170,10 @@ export interface DynamicAgentRuntimeDeps { flowPublicBaseUrl?: string; /** Spec 004 — backing store for `ctx.status`; cleared on deactivate. */ pluginStatusRegistry?: PluginStatusRegistry; + /** Issue #438 follow-up — kernel-published `ctx.operatorAuth`, threaded + * straight into every `createPluginContext`. Optional so narrow test + * contexts can omit it (an admin router relying on it then fails closed). */ + operatorAuth?: OperatorAuthAccessor; /** Issue #474 (round 5) — automatic OAuth-connection readiness signal, * refreshed from the vault on every activate() and cleared on * deactivate(). Separate from `pluginStatusRegistry` — see @@ -395,6 +400,7 @@ export class DynamicAgentRuntime { flowSigningKey: this.deps.flowSigningKey, flowPublicBaseUrl: this.deps.flowPublicBaseUrl, pluginStatusRegistry: this.deps.pluginStatusRegistry, + operatorAuth: this.deps.operatorAuth, logger: (...args) => console.log(`[${agentId}]`, ...args), }); diff --git a/middleware/src/plugins/toolPluginRuntime.ts b/middleware/src/plugins/toolPluginRuntime.ts index 6e2a8de8e..a808ba8fd 100644 --- a/middleware/src/plugins/toolPluginRuntime.ts +++ b/middleware/src/plugins/toolPluginRuntime.ts @@ -12,7 +12,11 @@ import type { ServiceRegistry } from '../platform/serviceRegistry.js'; import type { SecretVault } from '../secrets/vault.js'; import type { OAuthReadinessTracker } from './oauth/oauthReadinessTracker.js'; import type { NativeToolRegistry } from '@omadia/orchestrator'; -import type { ApprovedExtension, ExtensionTemplate } from '@omadia/plugin-api'; +import type { + ApprovedExtension, + ExtensionTemplate, + OperatorAuthAccessor, +} from '@omadia/plugin-api'; import type { BuiltInPackageStore } from './builtInPackageStore.js'; import type { SelfExtendRegistry } from './selfExtension/selfExtendRegistry.js'; import type { ExtensionStore } from './selfExtension/extensionStore.js'; @@ -88,6 +92,10 @@ export interface ToolPluginRuntimeDeps { flowPublicBaseUrl?: string; /** Spec 004 — backing store for `ctx.status`; cleared on deactivate. */ pluginStatusRegistry?: PluginStatusRegistry; + /** Issue #438 follow-up — kernel-published `ctx.operatorAuth`, threaded + * straight into every `createPluginContext`. Optional so narrow test + * contexts can omit it (an admin router relying on it then fails closed). */ + operatorAuth?: OperatorAuthAccessor; /** Issue #474 (round 5) — automatic OAuth-connection readiness signal, * refreshed from the vault on every activate() and cleared on * deactivate(). Separate from `pluginStatusRegistry` — see @@ -280,6 +288,7 @@ export class ToolPluginRuntime { flowSigningKey: this.deps.flowSigningKey, flowPublicBaseUrl: this.deps.flowPublicBaseUrl, pluginStatusRegistry: this.deps.pluginStatusRegistry, + operatorAuth: this.deps.operatorAuth, logger: (...args) => console.log(`[${agentId}]`, ...args), }); diff --git a/middleware/test/auth/apiKeyScopes.test.ts b/middleware/test/auth/apiKeyScopes.test.ts new file mode 100644 index 000000000..7ea4ccca9 --- /dev/null +++ b/middleware/test/auth/apiKeyScopes.test.ts @@ -0,0 +1,120 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { + assertValidScopes, + CHAT_WRITE_SCOPE, + DENY_ALL_SCOPES, + hasScope, + isValidScope, + LEGACY_DEFAULT_SCOPES, + normalizeScopes, + WILDCARD_SCOPE, +} from '../../packages/harness-api-key-auth/src/apiKeyScopes.js'; + +/** + * Issue #439 — per-key scopes. Two load-bearing properties: + * + * 1. Backward compatibility: a key persisted before scopes existed carries no + * `scopes` field, and must keep working with exactly the capability it used + * to have — not more (a `*` default would be a privilege escalation shipped + * by an upgrade) and not less (an empty default would break live + * integrations). + * 2. Fail-closed on corruption: a `scopes` field that is PRESENT but + * unreadable is not the same situation, and must never resolve to a + * capability grant. `absent` and `malformed` are decided separately. + */ +describe('auth/apiKeyScopes', () => { + it('accepts `:` and the bare wildcard, rejects everything else', () => { + assert.equal(isValidScope('chat:write'), true); + assert.equal(isValidScope('memory:read'), true); + assert.equal(isValidScope('plan-runner:run'), true); + assert.equal(isValidScope(WILDCARD_SCOPE), true); + + assert.equal(isValidScope('chat'), false, 'no action segment'); + assert.equal(isValidScope('chat:'), false); + assert.equal(isValidScope(':write'), false); + assert.equal(isValidScope('Chat:Write'), false, 'uppercase is not a different scope'); + assert.equal(isValidScope('chat:*'), false, 'no prefix wildcards — exact match only'); + assert.equal(isValidScope(''), false); + assert.equal(isValidScope(42), false); + assert.equal(isValidScope(undefined), false); + }); + + it('normalizeScopes gives an ABSENT scopes field the legacy default', () => { + // The only input that means "genuinely minted before #439": the field was + // never written at all. `create()` always persists an explicit array, so + // nothing this store writes can land here. + assert.deepEqual(normalizeScopes(undefined), LEGACY_DEFAULT_SCOPES); + }); + + it('normalizeScopes DENIES a present-but-malformed scopes field instead of granting the default', () => { + // Regression guard for a fail-open: every input below is a `scopes` field + // that EXISTS and cannot be read. Returning `['chat:write']` for any of + // them would hand chat access to a key an operator may have deliberately + // restricted away from chat. + assert.deepEqual(normalizeScopes(null), DENY_ALL_SCOPES, 'null is present, not absent'); + assert.deepEqual( + normalizeScopes('memory:read'), + DENY_ALL_SCOPES, + 'a string instead of an array', + ); + assert.deepEqual(normalizeScopes(42), DENY_ALL_SCOPES, 'a number instead of an array'); + assert.deepEqual( + normalizeScopes({ 0: 'chat:write' }), + DENY_ALL_SCOPES, + 'an object instead of an array', + ); + assert.deepEqual(normalizeScopes([]), DENY_ALL_SCOPES, 'an empty array grants nothing'); + assert.deepEqual( + normalizeScopes(['Chat:Write']), + DENY_ALL_SCOPES, + 'uppercase fails SCOPE_PATTERN — it is not the same scope as chat:write', + ); + assert.deepEqual(normalizeScopes(['nonsense']), DENY_ALL_SCOPES, 'all entries invalid'); + assert.deepEqual(normalizeScopes([null]), DENY_ALL_SCOPES, 'non-string entry'); + }); + + it('a denied scope set fails every capability check closed', () => { + assert.equal(hasScope(normalizeScopes('memory:read'), CHAT_WRITE_SCOPE), false); + assert.equal(hasScope(normalizeScopes(['Chat:Write']), CHAT_WRITE_SCOPE), false); + assert.equal(hasScope(normalizeScopes([]), WILDCARD_SCOPE), false); + }); + + it('normalizeScopes denies a PARTIALLY valid array rather than silently narrowing it', () => { + // Never widen, and do not quietly guess either: half a scope set is a + // record we cannot read faithfully. + assert.deepEqual(normalizeScopes(['chat:write', 'nonsense']), DENY_ALL_SCOPES); + assert.deepEqual(normalizeScopes(['chat:write', 7]), DENY_ALL_SCOPES); + }); + + it('the legacy default is exactly the one capability pre-scopes keys had', () => { + assert.deepEqual(LEGACY_DEFAULT_SCOPES, [CHAT_WRITE_SCOPE]); + assert.equal( + LEGACY_DEFAULT_SCOPES.includes(WILDCARD_SCOPE), + false, + 'defaulting old keys to `*` would widen them on upgrade', + ); + }); + + it('normalizeScopes keeps an all-valid array, de-duplicated', () => { + assert.deepEqual(normalizeScopes(['chat:write', 'memory:read', 'chat:write']), [ + 'chat:write', + 'memory:read', + ]); + assert.deepEqual(normalizeScopes([WILDCARD_SCOPE]), [WILDCARD_SCOPE]); + }); + + it('assertValidScopes throws on a malformed scope instead of silently dropping it', () => { + assert.deepEqual(assertValidScopes(['chat:write', 'chat:write']), ['chat:write']); + assert.throws(() => assertValidScopes(['chat:write', 'oops']), /invalid API-key scope/); + }); + + it('hasScope matches exactly, or via the global wildcard', () => { + assert.equal(hasScope(['chat:write'], 'chat:write'), true); + assert.equal(hasScope(['chat:write'], 'memory:read'), false); + assert.equal(hasScope([WILDCARD_SCOPE], 'anything:goes'), true); + assert.equal(hasScope([], 'chat:write'), false); + assert.equal(hasScope(undefined, 'chat:write'), false); + }); +}); diff --git a/middleware/test/auth/requireApiKey.test.ts b/middleware/test/auth/requireApiKey.test.ts new file mode 100644 index 000000000..99b02ed0d --- /dev/null +++ b/middleware/test/auth/requireApiKey.test.ts @@ -0,0 +1,257 @@ +import { strict as assert } from 'node:assert'; +import { after, before, describe, it } from 'node:test'; +import type { AddressInfo } from 'node:net'; +import type { Server } from 'node:http'; + +import express from 'express'; + +import { createApiKeyStore } from '../../packages/harness-api-key-auth/src/apiKeyStore.js'; +import { createAuditLog } from '../../packages/harness-api-key-auth/src/auditLog.js'; +import { createRateLimiter } from '../../packages/harness-api-key-auth/src/rateLimiter.js'; +import { requireApiKey } from '../../packages/harness-api-key-auth/src/requireApiKey.js'; +import { createFakeSecrets } from '../channelApi/testSecrets.js'; + +/** + * Issue #439 — the reusable half of the story: any route, kernel or plugin, + * can mount `requireApiKey` and be authenticated by a server-to-server bearer + * key instead of the `omadia_session` cookie. Mirrors the router-level + * fixture style of `test/channelApi/chatRouter.test.ts`. + */ +function startGuardedServer(opts: { + scope?: string; + withRateLimiter?: boolean; +}): { + baseUrl: string; + apiKeys: ReturnType; + auditLog: ReturnType; + secrets: ReturnType; + close: () => Promise; +} { + const secrets = createFakeSecrets(); + const apiKeys = createApiKeyStore(secrets); + const auditLog = createAuditLog(secrets); + + const app = express(); + app.use(express.json()); + app.get( + '/guarded', + requireApiKey({ + apiKeys, + auditLog, + ...(opts.withRateLimiter ? { rateLimiter: createRateLimiter() } : {}), + ...(opts.scope ? { scope: opts.scope } : {}), + routeLabel: '/guarded', + }), + (req, res) => { + req.apiKey?.audit('ok'); + res.json({ keyId: req.apiKey?.keyId, scopes: req.apiKey?.scopes }); + }, + ); + const server: Server = app.listen(0); + const addr = server.address() as AddressInfo; + return { + baseUrl: `http://127.0.0.1:${String(addr.port)}/guarded`, + apiKeys, + auditLog, + secrets, + close: () => new Promise((r) => server.close(() => r())), + }; +} + +describe('auth/requireApiKey — authentication', () => { + let harness: ReturnType; + + before(() => { + harness = startGuardedServer({}); + }); + after(async () => { + await harness.close(); + }); + + it('401s with the public-API error shape when no Authorization header is sent', async () => { + const res = await fetch(harness.baseUrl); + assert.equal(res.status, 401); + assert.deepEqual(await res.json(), { + error: 'unauthorized', + message: 'missing Authorization: Bearer header', + }); + }); + + it('401s for a non-Bearer scheme and for an empty bearer value', async () => { + const basic = await fetch(harness.baseUrl, { headers: { authorization: 'Basic abc' } }); + assert.equal(basic.status, 401); + const empty = await fetch(harness.baseUrl, { headers: { authorization: 'Bearer ' } }); + assert.equal(empty.status, 401); + }); + + it('401s for an unknown key', async () => { + const res = await fetch(harness.baseUrl, { + headers: { authorization: 'Bearer omk_not-a-real-key' }, + }); + assert.equal(res.status, 401); + assert.deepEqual(await res.json(), { + error: 'unauthorized', + message: 'invalid or revoked API key', + }); + }); + + it('passes a valid key through and exposes the principal on req.apiKey', async () => { + const created = await harness.apiKeys.create({ label: 'laravel-app' }); + const res = await fetch(harness.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(res.status, 200); + assert.deepEqual(await res.json(), { + keyId: created.record.id, + scopes: ['chat:write'], + }); + }); + + it('401s once the key is revoked, on the very next request', async () => { + const created = await harness.apiKeys.create({ label: 'short-lived' }); + assert.equal( + (await fetch(harness.baseUrl, { headers: { authorization: `Bearer ${created.token}` } })) + .status, + 200, + ); + await harness.apiKeys.revoke(created.record.id); + assert.equal( + (await fetch(harness.baseUrl, { headers: { authorization: `Bearer ${created.token}` } })) + .status, + 401, + ); + }); + + it('does not audit an unauthenticated call — there is no caller identity to attribute', async () => { + const local = startGuardedServer({}); + await fetch(local.baseUrl); + await fetch(local.baseUrl, { headers: { authorization: 'Bearer omk_nope' } }); + assert.equal((await local.auditLog.list()).length, 0); + await local.close(); + }); +}); + +describe('auth/requireApiKey — scopes', () => { + it('403s a key that lacks the required scope, and audits it as forbidden', async () => { + const local = startGuardedServer({ scope: 'memory:read' }); + const created = await local.apiKeys.create({ label: 'chat-only' }); + + const res = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(res.status, 403); + assert.deepEqual(await res.json(), { + error: 'forbidden', + message: "this API key is not scoped for 'memory:read'", + }); + + const entries = await local.auditLog.list(); + assert.equal(entries.length, 1); + assert.equal(entries[0]?.status, 'forbidden'); + assert.equal(entries[0]?.keyId, created.record.id); + await local.close(); + }); + + it('lets a key with the exact scope through', async () => { + const local = startGuardedServer({ scope: 'memory:read' }); + const created = await local.apiKeys.create({ scopes: ['memory:read'] }); + const res = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(res.status, 200); + await local.close(); + }); + + it('lets a wildcard key through any scope gate', async () => { + const local = startGuardedServer({ scope: 'memory:read' }); + const created = await local.apiKeys.create({ scopes: ['*'] }); + const res = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(res.status, 200); + await local.close(); + }); + + it('authenticates without any scope gate when `scope` is omitted', async () => { + const local = startGuardedServer({}); + const created = await local.apiKeys.create({ scopes: ['memory:read'] }); + const res = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(res.status, 200); + await local.close(); + }); + + it('403s a key whose PERSISTED scopes field is malformed, instead of granting it the legacy default', async () => { + // The end-to-end shape of the fail-open this replaced: a vault record + // whose `scopes` is a bare string (or wrong-cased, or partially valid) + // used to hydrate to `['chat:write']`, so a key deliberately restricted + // away from chat authenticated against a `chat:write` route. + for (const corrupt of ['memory:read', ['Chat:Write'], ['chat:write', 'nonsense'], []]) { + const local = startGuardedServer({ scope: 'chat:write' }); + const created = await local.apiKeys.create({ label: 'restricted', scopes: ['memory:read'] }); + const raw = await local.secrets.get(`key:${created.record.id}`); + assert.ok(raw); + await local.secrets.set?.( + `key:${created.record.id}`, + JSON.stringify({ ...(JSON.parse(raw) as Record), scopes: corrupt }), + ); + + const res = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(res.status, 403, `scopes=${JSON.stringify(corrupt)} must not reach the handler`); + assert.deepEqual( + (await local.auditLog.list()).map((e) => e.status), + ['forbidden'], + 'the denial is attributable to the key', + ); + await local.close(); + } + }); +}); + +describe('auth/requireApiKey — rate limiting', () => { + it('429s past the per-key budget and audits it, without invoking the handler', async () => { + const local = startGuardedServer({ withRateLimiter: true }); + const created = await local.apiKeys.create({ rateLimitPerMinute: 1 }); + + const first = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(first.status, 200); + + const second = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(second.status, 429); + assert.deepEqual(await second.json(), { + error: 'rate_limited', + message: 'this key is limited to 1 requests/minute', + }); + + const entries = await local.auditLog.list(); + assert.deepEqual( + entries.map((e) => e.status), + ['ok', 'rate_limited'], + ); + assert.equal(entries[0]?.route, '/guarded'); + assert.equal(entries[0]?.method, 'GET'); + await local.close(); + }); + + it('burns quota before the scope check, so scope probing is not free', async () => { + const local = startGuardedServer({ withRateLimiter: true, scope: 'memory:read' }); + const created = await local.apiKeys.create({ rateLimitPerMinute: 1, scopes: ['chat:write'] }); + + const first = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(first.status, 403, 'no scope → forbidden'); + const second = await fetch(local.baseUrl, { + headers: { authorization: `Bearer ${created.token}` }, + }); + assert.equal(second.status, 429, 'the forbidden call still consumed the budget'); + await local.close(); + }); +}); diff --git a/middleware/test/channelApi/adminKeysRouter.test.ts b/middleware/test/channelApi/adminKeysRouter.test.ts new file mode 100644 index 000000000..30137157e --- /dev/null +++ b/middleware/test/channelApi/adminKeysRouter.test.ts @@ -0,0 +1,320 @@ +import { strict as assert } from 'node:assert'; +import { after, before, describe, it } from 'node:test'; +import type { AddressInfo } from 'node:net'; +import type { Server } from 'node:http'; + +import express from 'express'; + +import { createAdminKeysRouter } from '../../packages/harness-channel-api/src/adminKeysRouter.js'; +import { createApiKeyStore } from '../../packages/harness-api-key-auth/src/apiKeyStore.js'; +import type { OperatorAuthAccessor } from '../../packages/plugin-api/src/index.js'; +import { createOperatorAuthAccessor } from '../../src/auth/operatorAuthAccessor.js'; +import { signSession } from '../../src/auth/sessionJwt.js'; +import { EmailWhitelist } from '../../src/auth/whitelist.js'; +import { createFakeSecrets } from './testSecrets.js'; + +/** Always-valid stub — used by the CRUD tests below, which exercise the + * route handlers themselves, not the auth gate (that has its own describe + * block further down, against the REAL operatorAuth implementation). */ +function alwaysValidOperatorAuth(): OperatorAuthAccessor { + return { async hasValidSession() { return true; } }; +} + +/** + * Router-level coverage for key lifecycle, mounted BEHIND a (stubbed-valid) + * operator-auth gate — mirrors how `plugin.ts` actually wires the router in + * production. The gate's real behaviour (401/503 paths, real session + * verification) is exercised separately below, and via + * `publicPathsExemption.test.ts` for the `publicPaths.ts` side of the story. + */ +describe('channelApi/adminKeysRouter — CRUD (auth stubbed valid)', () => { + let server: Server; + let baseUrl: string; + + before(() => { + const app = express(); + app.use(express.json()); + app.use( + '/admin/keys', + createAdminKeysRouter(createApiKeyStore(createFakeSecrets()), alwaysValidOperatorAuth()), + ); + server = app.listen(0); + const addr = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${String(addr.port)}/admin/keys`; + }); + + after(async () => { + await new Promise((r) => server.close(() => r())); + }); + + it('POST / creates a key, returning the plaintext token once + a hash-free record', async () => { + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ label: 'ci' }), + }); + assert.equal(res.status, 201); + const body = (await res.json()) as { token: string; key: Record }; + assert.ok(body.token.startsWith('omk_')); + assert.equal(body.key['label'], 'ci'); + assert.ok(!('hash' in body.key)); + }); + + it('POST / rejects an invalid body', async () => { + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ rateLimitPerMinute: -1 }), + }); + assert.equal(res.status, 400); + }); + + it('GET / lists created keys without their hash', async () => { + await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ label: 'listed' }), + }); + const res = await fetch(baseUrl); + assert.equal(res.status, 200); + const body = (await res.json()) as { keys: Array> }; + assert.ok(body.keys.some((k) => k['label'] === 'listed')); + assert.ok(body.keys.every((k) => !('hash' in k))); + }); + + it('POST / accepts a scope set, and GET / shows it (issue #439)', async () => { + const created = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ label: 'scoped', scopes: ['chat:write', 'memory:read'] }), + }); + assert.equal(created.status, 201); + const body = (await created.json()) as { key: { id: string; scopes: string[] } }; + assert.deepEqual(body.key.scopes, ['chat:write', 'memory:read']); + + const listed = (await (await fetch(baseUrl)).json()) as { + keys: Array<{ id: string; scopes: string[] }>; + }; + assert.deepEqual( + listed.keys.find((k) => k.id === body.key.id)?.scopes, + ['chat:write', 'memory:read'], + ); + }); + + it('POST / rejects an explicitly empty scope array with 400, never a defaulted key (issue #439)', async () => { + // Regression guard for a create/read divergence: `normalizeScopes` denies + // a persisted `[]`, so creation must not quietly turn it into the legacy + // `chat:write` default. An operator asking for zero capabilities must get + // an error, not a chat-capable key — and not a permanently-403 one either. + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ label: 'readonly', scopes: [] }), + }); + assert.equal(res.status, 400); + const body = (await res.json()) as { error: string }; + assert.equal(body.error, 'invalid_request'); + + // ...and nothing was minted under that label. + const listed = (await (await fetch(baseUrl)).json()) as { + keys: Array<{ label?: string }>; + }; + assert.ok(!listed.keys.some((k) => k.label === 'readonly')); + }); + + it('POST / without scopes still mints a working, chat-capable key (backward compatible)', async () => { + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ label: 'unscoped' }), + }); + assert.equal(res.status, 201); + const body = (await res.json()) as { key: { scopes: string[] } }; + assert.deepEqual(body.key.scopes, ['chat:write']); + }); + + it('POST / 400s on a malformed scope instead of 500-ing out of the store', async () => { + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ scopes: ['nope'] }), + }); + assert.equal(res.status, 400); + }); + + it('POST /:id/revoke revokes an existing key and 404s for an unknown one', async () => { + const created = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({}), + }); + const { key } = (await created.json()) as { key: { id: string } }; + + const revoked = await fetch(`${baseUrl}/${key.id}/revoke`, { method: 'POST' }); + assert.equal(revoked.status, 200); + const revokedBody = (await revoked.json()) as { key: { revokedAt?: number } }; + assert.equal(typeof revokedBody.key.revokedAt, 'number'); + + const missing = await fetch(`${baseUrl}/does-not-exist/revoke`, { method: 'POST' }); + assert.equal(missing.status, 404); + }); +}); + +/** + * The security-critical coverage that was missing before this fixup: a REAL + * end-to-end auth gate, wired exactly like production (`createAdminKeysRouter` + * behind the REAL `createOperatorAuthAccessor`, which itself wraps the exact + * same `evaluateSessionToken` logic `requireAuth` uses for every other + * `/api/v1/*` route). No stubbing of the auth decision anywhere in this block. + */ +describe('channelApi/adminKeysRouter — operator-session auth (real verification)', () => { + const signingKey = new TextEncoder().encode('adminKeysRouter-auth-test-signing-key-32b!!'); + const whitelist = new EmailWhitelist('operator@example.com'); + const operatorAuth = createOperatorAuthAccessor({ signingKey, whitelist }); + + let server: Server; + let baseUrl: string; + + before(() => { + const app = express(); + app.use(express.json()); + app.use( + '/admin/keys', + createAdminKeysRouter(createApiKeyStore(createFakeSecrets()), operatorAuth), + ); + server = app.listen(0); + const addr = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${String(addr.port)}/admin/keys`; + }); + + after(async () => { + await new Promise((r) => server.close(() => r())); + }); + + it('no Cookie header → 401 auth.missing', async () => { + const res = await fetch(baseUrl); + assert.equal(res.status, 401); + const body = (await res.json()) as { code: string }; + assert.equal(body.code, 'auth.missing'); + }); + + it('garbage/invalid cookie value → 401 auth.invalid', async () => { + const res = await fetch(baseUrl, { + headers: { cookie: 'omadia_session=not-a-real-jwt' }, + }); + assert.equal(res.status, 401); + const body = (await res.json()) as { code: string }; + assert.equal(body.code, 'auth.invalid'); + }); + + it('valid session cookie → reaches the handler (200)', async () => { + const token = await signSession( + { + sub: 'operator-1', + email: 'operator@example.com', + display_name: 'Operator', + provider: 'local', + role: 'admin', + }, + signingKey, + ); + const res = await fetch(baseUrl, { + headers: { cookie: `omadia_session=${token}` }, + }); + assert.equal(res.status, 200); + const body = (await res.json()) as { keys: unknown[] }; + assert.ok(Array.isArray(body.keys)); + }); + + it('an Entra-provider session whose email fell off the whitelist → 401 auth.invalid (not_whitelisted collapses to invalid at the boolean accessor)', async () => { + const token = await signSession( + { + sub: 'not-whitelisted-1', + email: 'stranger@example.com', + display_name: 'Stranger', + provider: 'entra', + role: 'admin', + }, + signingKey, + ); + const res = await fetch(baseUrl, { + headers: { cookie: `omadia_session=${token}` }, + }); + assert.equal(res.status, 401); + }); + + it('the #439 scopes surface is BEHIND the gate too — an anonymous POST with scopes mints nothing', async () => { + // Guards the rebase of #439 onto this gate: the scoped create/list path + // must sit on top of the operator-session check, never beside it. A + // caller that can mint `scopes: ['*']` without a session would be the + // worst possible version of this router. + const anonymous = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ label: 'anonymous-wildcard', scopes: ['*'] }), + }); + assert.equal(anonymous.status, 401); + assert.equal(((await anonymous.json()) as { code: string }).code, 'auth.missing'); + + const token = await signSession( + { + sub: 'operator-1', + email: 'operator@example.com', + display_name: 'Operator', + provider: 'local', + role: 'admin', + }, + signingKey, + ); + const listed = (await ( + await fetch(baseUrl, { headers: { cookie: `omadia_session=${token}` } }) + ).json()) as { keys: Array<{ label?: string }> }; + assert.equal( + listed.keys.some((k) => k.label === 'anonymous-wildcard'), + false, + 'the rejected POST must not have created a key', + ); + }); +}); + +/** + * Fail-closed contract (finding #3): a plugin host that never wires + * `ctx.operatorAuth` must NOT fall back to mounting the router with no auth + * check — every route must refuse to serve. + */ +describe('channelApi/adminKeysRouter — fails closed without operatorAuth', () => { + let server: Server; + let baseUrl: string; + + before(() => { + const app = express(); + app.use(express.json()); + app.use( + '/admin/keys', + createAdminKeysRouter(createApiKeyStore(createFakeSecrets()), undefined), + ); + server = app.listen(0); + const addr = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${String(addr.port)}/admin/keys`; + }); + + after(async () => { + await new Promise((r) => server.close(() => r())); + }); + + it('GET / → 503 operator_auth.unavailable, even with no cookie at all', async () => { + const res = await fetch(baseUrl); + assert.equal(res.status, 503); + const body = (await res.json()) as { code: string }; + assert.equal(body.code, 'operator_auth.unavailable'); + }); + + it('POST / → 503 operator_auth.unavailable — never falls through to create a key', async () => { + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ label: 'should-never-be-created' }), + }); + assert.equal(res.status, 503); + }); +}); diff --git a/middleware/test/channelApi/apiKeyAuthReuseSeam.test.ts b/middleware/test/channelApi/apiKeyAuthReuseSeam.test.ts new file mode 100644 index 000000000..dfcd44f20 --- /dev/null +++ b/middleware/test/channelApi/apiKeyAuthReuseSeam.test.ts @@ -0,0 +1,79 @@ +import { strict as assert } from 'node:assert'; +import { readFileSync, readdirSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, it } from 'node:test'; + +/** + * Issue #439 — guards the reuse seam itself, which is the part of this change + * that regressions are cheapest to introduce into and most expensive to + * notice: someone re-adds a "small local helper" that hashes a key, and the + * codebase quietly has two credential implementations again. + * + * Structural assertions on source text (same technique as the constant-time + * assertion in `apiKeyToken.test.ts`), because the property being protected + * is "where does this code live", which no runtime behaviour can express. + */ +const CHANNEL_API_SRC = fileURLToPath( + new URL('../../packages/harness-channel-api/src/', import.meta.url), +); +const KERNEL_SRC = fileURLToPath(new URL('../../src/', import.meta.url)); + +function readChannelApiSources(): { name: string; text: string }[] { + return readdirSync(CHANNEL_API_SRC) + .filter((f) => f.endsWith('.ts')) + .map((name) => ({ name, text: readFileSync(CHANNEL_API_SRC + name, 'utf8') })); +} + +describe('channelApi — API-key auth reuse seam (@omadia/api-key-auth)', () => { + it('the plugin no longer carries its own copy of the credential primitives', () => { + const files = readdirSync(CHANNEL_API_SRC); + for (const moved of ['apiKeyToken.ts', 'apiKeyStore.ts', 'rateLimiter.ts', 'auditLog.ts']) { + assert.equal( + files.includes(moved), + false, + `${moved} must live in @omadia/api-key-auth only — one implementation, not two`, + ); + } + }); + + it('no file in the plugin re-implements minting, hashing, or constant-time compare', () => { + for (const { name, text } of readChannelApiSources()) { + assert.doesNotMatch(text, /node:crypto[\s\S]{0,200}(createHash|timingSafeEqual)/, name); + assert.doesNotMatch(text, /\bsha256Hex\s*\(/, `${name} must not hash key material itself`); + } + }); + + it('the plugin consumes the shared package for auth, storage, and rate limiting', () => { + const byName = new Map(readChannelApiSources().map((f) => [f.name, f.text])); + assert.match(byName.get('plugin.ts') ?? '', /from '@omadia\/api-key-auth'/); + assert.match(byName.get('chatRouter.ts') ?? '', /requireApiKey/); + assert.match(byName.get('adminKeysRouter.ts') ?? '', /from '@omadia\/api-key-auth'/); + + const pkg = JSON.parse( + readFileSync( + fileURLToPath(new URL('../../packages/harness-channel-api/package.json', import.meta.url)), + 'utf8', + ), + ) as { peerDependencies?: Record }; + assert.ok( + pkg.peerDependencies?.['@omadia/api-key-auth'], + 'the dependency must be declared, not merely resolvable via the workspace root', + ); + }); + + it('the kernel never imports a channel plugin — that direction is the layering inversion', () => { + const offenders: string[] = []; + const walk = (dir: string): void => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = `${dir}${entry.name}${entry.isDirectory() ? '/' : ''}`; + if (entry.isDirectory()) { + walk(full); + } else if (entry.name.endsWith('.ts')) { + if (/from '@omadia\/channel-api/.test(readFileSync(full, 'utf8'))) offenders.push(full); + } + } + }; + walk(KERNEL_SRC); + assert.deepEqual(offenders, [], 'middleware/src must not import @omadia/channel-api'); + }); +}); diff --git a/middleware/test/channelApi/apiKeyStore.test.ts b/middleware/test/channelApi/apiKeyStore.test.ts new file mode 100644 index 000000000..c7c9fde0d --- /dev/null +++ b/middleware/test/channelApi/apiKeyStore.test.ts @@ -0,0 +1,169 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { hasScope } from '../../packages/harness-api-key-auth/src/apiKeyScopes.js'; +import { createApiKeyStore } from '../../packages/harness-api-key-auth/src/apiKeyStore.js'; +import { createFakeSecrets } from './testSecrets.js'; + +describe('channelApi/apiKeyStore', () => { + it('create() returns a plaintext token once and a public record without the hash', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({ label: 'ci-bot' }); + assert.ok(created.token.startsWith('omk_')); + assert.equal(created.record.label, 'ci-bot'); + assert.equal(created.record.rateLimitPerMinute, 60, 'defaults to 60/min'); + assert.equal(created.record.revokedAt, undefined); + assert.ok(!('hash' in created.record), 'public view never carries the hash'); + }); + + it('honours a custom rateLimitPerMinute', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({ rateLimitPerMinute: 5 }); + assert.equal(created.record.rateLimitPerMinute, 5); + }); + + it('list() returns every created key, oldest first, without hashes', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const a = await store.create({ label: 'a' }); + const b = await store.create({ label: 'b' }); + const listed = await store.list(); + assert.deepEqual( + listed.map((k) => k.label), + ['a', 'b'], + ); + assert.ok(listed.every((k) => !('hash' in k))); + assert.ok(listed.some((k) => k.id === a.record.id)); + assert.ok(listed.some((k) => k.id === b.record.id)); + }); + + it('verify() resolves a valid, non-revoked key to its record', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({}); + const resolved = await store.verify(created.token); + assert.ok(resolved); + assert.equal(resolved?.id, created.record.id); + }); + + it('verify() rejects an unknown token', async () => { + const store = createApiKeyStore(createFakeSecrets()); + await store.create({}); + const resolved = await store.verify('omk_not-a-real-key'); + assert.equal(resolved, undefined); + }); + + it('revoke() makes the key stop authenticating on the next verify() call', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({}); + assert.ok(await store.verify(created.token), 'valid before revoke'); + + const revoked = await store.revoke(created.record.id); + assert.ok(revoked); + assert.ok(typeof revoked?.revokedAt === 'number'); + + const resolved = await store.verify(created.token); + assert.equal(resolved, undefined, 'revoked key must no longer authenticate'); + }); + + it('revoke() is idempotent and revoke() of an unknown id returns undefined', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({}); + const first = await store.revoke(created.record.id); + const second = await store.revoke(created.record.id); + assert.equal(first?.revokedAt, second?.revokedAt, 'revoking twice does not bump revokedAt'); + assert.equal(await store.revoke('does-not-exist'), undefined); + }); + + it('two keys are independently verifiable and independently revocable', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const a = await store.create({ label: 'a' }); + const b = await store.create({ label: 'b' }); + await store.revoke(a.record.id); + assert.equal(await store.verify(a.token), undefined, 'a is revoked'); + assert.ok(await store.verify(b.token), 'b is untouched'); + }); +}); + +/** Issue #439 — per-key scopes, with the backward-compat contract for keys + * written to the vault before the field existed. */ +describe('channelApi/apiKeyStore — scopes', () => { + it('defaults to the legacy scope set when none is given, and exposes it on create/list', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({ label: 'default-scoped' }); + assert.deepEqual(created.record.scopes, ['chat:write']); + const listed = await store.list(); + assert.deepEqual(listed[0]?.scopes, ['chat:write']); + }); + + it('persists an explicit scope set and returns it on verify()', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({ scopes: ['memory:read', 'chat:write'] }); + assert.deepEqual(created.record.scopes, ['memory:read', 'chat:write']); + const resolved = await store.verify(created.token); + assert.deepEqual(resolved?.scopes, ['memory:read', 'chat:write']); + }); + + it('rejects an explicitly empty scope array at CREATE time rather than resolving it to a default', async () => { + // The read path (`normalizeScopes`) treats a persisted `[]` as corruption + // and denies everything. If creation resolved the same value to the legacy + // default, one field would mean "deny all" coming out and "grant chat" + // going in — and an operator who asked for zero capabilities would be + // handed a chat-capable key. Omitting `scopes` is how you ask for the + // default; `[]` is an error. + const store = createApiKeyStore(createFakeSecrets()); + await assert.rejects(() => store.create({ scopes: [] }), /must not be empty/); + }); + + it('omitting scopes entirely still resolves to the legacy default (unchanged)', async () => { + const store = createApiKeyStore(createFakeSecrets()); + const created = await store.create({ label: 'defaulted' }); + assert.deepEqual(created.record.scopes, ['chat:write']); + assert.deepEqual((await store.verify(created.token))?.scopes, ['chat:write']); + }); + + it('rejects a malformed scope rather than silently dropping it', async () => { + const store = createApiKeyStore(createFakeSecrets()); + await assert.rejects(() => store.create({ scopes: ['not a scope'] }), /invalid API-key scope/); + }); + + it('a key persisted BEFORE scopes existed still authenticates, with the legacy scope set', async () => { + // Exactly the JSON shape issue #438 wrote: no `scopes` field at all. + const secrets = createFakeSecrets(); + const store = createApiKeyStore(secrets); + const legacy = await store.create({ label: 'pre-scopes' }); + const raw = await secrets.get(`key:${legacy.record.id}`); + assert.ok(raw); + const { scopes: _dropped, ...withoutScopes } = JSON.parse(raw) as Record; + await secrets.set?.(`key:${legacy.record.id}`, JSON.stringify(withoutScopes)); + + const resolved = await store.verify(legacy.token); + assert.ok(resolved, 'a pre-scopes key must keep authenticating'); + assert.deepEqual(resolved?.scopes, ['chat:write']); + assert.deepEqual((await store.list())[0]?.scopes, ['chat:write']); + }); + + it('a key whose persisted scopes field is MALFORMED authenticates but is authorized for nothing', async () => { + // The fail-open this replaced: a `scopes` field that is present and + // unreadable used to hydrate to the legacy default, so a key an operator + // had restricted away from chat came back chat-capable. Absent means + // "pre-#439"; malformed means "we cannot tell", and we must not guess in + // the direction of a grant. + for (const corrupt of ['memory:read', ['Chat:Write'], [], ['chat:write', 'nonsense'], null]) { + const secrets = createFakeSecrets(); + const store = createApiKeyStore(secrets); + const key = await store.create({ label: 'restricted', scopes: ['memory:read'] }); + const raw = await secrets.get(`key:${key.record.id}`); + assert.ok(raw); + const record = JSON.parse(raw) as Record; + await secrets.set?.( + `key:${key.record.id}`, + JSON.stringify({ ...record, scopes: corrupt }), + ); + + const resolved = await store.verify(key.token); + assert.ok(resolved, 'the credential itself is still valid — only its authorization is gone'); + assert.deepEqual(resolved?.scopes, [], `scopes=${JSON.stringify(corrupt)} must grant nothing`); + assert.equal(hasScope(resolved?.scopes, 'chat:write'), false); + assert.equal(hasScope(resolved?.scopes, 'memory:read'), false); + } + }); +}); diff --git a/middleware/test/channelApi/apiKeyToken.test.ts b/middleware/test/channelApi/apiKeyToken.test.ts new file mode 100644 index 000000000..507f28fa9 --- /dev/null +++ b/middleware/test/channelApi/apiKeyToken.test.ts @@ -0,0 +1,106 @@ +import { strict as assert } from 'node:assert'; +import { createHash, timingSafeEqual } from 'node:crypto'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, it } from 'node:test'; + +import { + API_KEY_PREFIX, + mintApiKey, + sha256Hex, + verifyApiKey, +} from '../../packages/harness-api-key-auth/src/apiKeyToken.js'; + +/** + * Issue #438 — pure-unit coverage for the API-key token, mirroring + * `test/devplatform/jobToken.test.ts` (the closest existing precedent for a + * hashed, constant-time-verified bearer credential in this codebase). + */ +describe('channelApi/apiKeyToken', () => { + it('mints `omk_` + 32 random bytes base64url, and stores only the sha256 hex', () => { + const { token, hash } = mintApiKey(); + assert.ok(token.startsWith(API_KEY_PREFIX), 'has the omk_ prefix'); + const b64 = token.slice(API_KEY_PREFIX.length); + assert.equal(Buffer.from(b64, 'base64url').length, 32, '32 random bytes'); + assert.match(hash, /^[0-9a-f]{64}$/, 'hash is 64 hex chars (sha256)'); + assert.equal(hash, createHash('sha256').update(token, 'utf8').digest('hex')); + assert.ok(!hash.includes(token), 'the plaintext is not embedded in the hash'); + }); + + it('mints distinct keys', () => { + const a = mintApiKey(); + const b = mintApiKey(); + assert.notEqual(a.token, b.token); + assert.notEqual(a.hash, b.hash); + }); + + it('verifies a key against its own stored hash (round-trip)', () => { + const { token, hash } = mintApiKey(); + assert.equal(verifyApiKey(token, hash), true); + }); + + it('rejects a wrong key of the same length without leaking via a throw', () => { + const { hash } = mintApiKey(); + const other = mintApiKey().token; + assert.equal(verifyApiKey(other, hash), false); + }); + + it('rejects mismatches at the FIRST character and at the LAST character identically', () => { + // Not a timing assertion (that would be flaky) — just confirms the + // observable outcome doesn't depend on where the difference sits, which + // is a prerequisite for (not proof of) constant-time behaviour. The + // "doesn't early-return" guarantee itself comes from delegating to + // `crypto.timingSafeEqual`, asserted structurally below. + const { token, hash } = mintApiKey(); + const mismatchAtStart = 'X' + token.slice(1); + const mismatchAtEnd = token.slice(0, -1) + (token.endsWith('X') ? 'Y' : 'X'); + assert.equal(verifyApiKey(mismatchAtStart, hash), false); + assert.equal(verifyApiKey(mismatchAtEnd, hash), false); + }); + + it('does not throw and returns false when the presented key length differs', () => { + const { hash } = mintApiKey(); + assert.equal(verifyApiKey('x', hash), false); + assert.equal(verifyApiKey('', hash), false); + assert.equal(verifyApiKey(API_KEY_PREFIX + 'A'.repeat(500), hash), false); + }); + + it('returns false for a null/empty/malformed stored hash without throwing', () => { + const { token } = mintApiKey(); + assert.equal(verifyApiKey(token, null), false); + assert.equal(verifyApiKey(token, undefined), false); + assert.equal(verifyApiKey(token, ''), false); + assert.equal(verifyApiKey(token, 'zzzz'), false); + assert.equal(verifyApiKey(token, 'abc'), false); + }); + + it('sha256Hex is stable and matches node crypto', () => { + assert.equal(sha256Hex('omadia'), createHash('sha256').update('omadia', 'utf8').digest('hex')); + }); + + it('the source delegates the actual comparison to crypto.timingSafeEqual, not a naive === / early-exit loop', () => { + // Structural, deterministic check (no wall-clock timing, per the issue's + // own guidance) that the comparison primitive is Node's constant-time + // buffer compare rather than a hand-rolled loop that could short-circuit + // on the first differing byte. + const src = readFileSync( + fileURLToPath(new URL('../../packages/harness-api-key-auth/src/apiKeyToken.ts', import.meta.url)), + 'utf8', + ); + assert.match(src, /timingSafeEqual\(actual, expected\)/, 'verifyApiKey must call timingSafeEqual'); + assert.doesNotMatch( + src, + /actual\s*===\s*expected|expected\s*===\s*actual/, + 'must not fall back to a plain === compare of the hash buffers', + ); + + // Cross-check: for equal-length buffers, our function's result always + // agrees with a direct timingSafeEqual call — i.e. it is not silently + // adding its own early-exit logic ON TOP of the primitive. + const { token, hash } = mintApiKey(); + const wrong = mintApiKey().hash; + const actual = Buffer.from(sha256Hex(token), 'hex'); + assert.equal(verifyApiKey(token, hash), timingSafeEqual(actual, Buffer.from(hash, 'hex'))); + assert.equal(verifyApiKey(token, wrong), timingSafeEqual(actual, Buffer.from(wrong, 'hex'))); + }); +}); diff --git a/middleware/test/channelApi/auditLog.test.ts b/middleware/test/channelApi/auditLog.test.ts new file mode 100644 index 000000000..7c6307631 --- /dev/null +++ b/middleware/test/channelApi/auditLog.test.ts @@ -0,0 +1,46 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { MAX_ENTRIES, createAuditLog } from '../../packages/harness-api-key-auth/src/auditLog.js'; +import { createFakeSecrets } from './testSecrets.js'; + +describe('channelApi/auditLog', () => { + it('record() then list() surfaces the entry (who called what, when)', async () => { + const log = createAuditLog(createFakeSecrets()); + await log.record({ keyId: 'k1', route: '/chat', method: 'POST', at: 1000, status: 'ok' }); + const entries = await log.list(); + assert.equal(entries.length, 1); + assert.deepEqual(entries[0], { + keyId: 'k1', + route: '/chat', + method: 'POST', + at: 1000, + status: 'ok', + }); + }); + + it('concurrent record() calls do not lose entries (serialized writes)', async () => { + const log = createAuditLog(createFakeSecrets()); + const N = 25; + await Promise.all( + Array.from({ length: N }, (_, i) => + log.record({ keyId: `k${i}`, route: '/chat', method: 'POST', at: i, status: 'ok' }), + ), + ); + const entries = await log.list(); + assert.equal(entries.length, N, 'no entry lost to a read-modify-write race'); + const keyIds = new Set(entries.map((e) => e.keyId)); + assert.equal(keyIds.size, N, 'every distinct call is represented'); + }); + + it('caps the log at MAX_ENTRIES, dropping the oldest first', async () => { + const log = createAuditLog(createFakeSecrets()); + for (let i = 0; i < MAX_ENTRIES + 10; i++) { + await log.record({ keyId: 'k1', route: '/chat', method: 'POST', at: i, status: 'ok' }); + } + const entries = await log.list(); + assert.equal(entries.length, MAX_ENTRIES); + assert.equal(entries[0]?.at, 10, 'the oldest 10 entries were dropped'); + assert.equal(entries[entries.length - 1]?.at, MAX_ENTRIES + 9); + }); +}); diff --git a/middleware/test/channelApi/chatRouter.test.ts b/middleware/test/channelApi/chatRouter.test.ts new file mode 100644 index 000000000..0001c352d --- /dev/null +++ b/middleware/test/channelApi/chatRouter.test.ts @@ -0,0 +1,466 @@ +import { strict as assert } from 'node:assert'; +import { after, before, describe, it } from 'node:test'; +import type { AddressInfo } from 'node:net'; +import type { Server } from 'node:http'; + +import express from 'express'; +import type { CoreApi, IncomingTurn } from '@omadia/channel-sdk'; + +import { createApiKeyStore } from '../../packages/harness-api-key-auth/src/apiKeyStore.js'; +import { createAuditLog } from '../../packages/harness-api-key-auth/src/auditLog.js'; +import { createRateLimiter } from '../../packages/harness-api-key-auth/src/rateLimiter.js'; +import { + createApiChatRouter, + internalConversationId, +} from '../../packages/harness-channel-api/src/chatRouter.js'; +import { createFakeSecrets } from './testSecrets.js'; +// Import from source: `graphScopeFor` was added after the last dist build, +// so the built `@omadia/orchestrator` barrel doesn't re-export it yet (same +// rationale as test/sessionLoggerGraphScope.test.ts). +import { graphScopeFor } from '../../packages/harness-orchestrator/src/sessionLogger.js'; + +/** Parses an NDJSON response body (one JSON object per line) into events. */ +function parseNdjson(body: string): unknown[] { + return body + .split('\n') + .filter((line) => line.trim().length > 0) + .map((line) => JSON.parse(line) as unknown); +} + +describe('channelApi/chatRouter — wiring (auth, rate limit, audit, NDJSON framing)', () => { + let server: Server; + let baseUrl: string; + let apiKeys: ReturnType; + let auditLog: ReturnType; + let rateLimiter: ReturnType; + const capturedTurns: IncomingTurn[] = []; + + before(() => { + const secrets = createFakeSecrets(); + apiKeys = createApiKeyStore(secrets); + auditLog = createAuditLog(secrets); + rateLimiter = createRateLimiter(); + + const app = express(); + app.use(express.json()); + app.use( + createApiChatRouter({ + channelId: '@omadia/channel-api', + apiKeys, + auditLog, + rateLimiter, + core: { + async *handleTurnStream(turn: IncomingTurn) { + capturedTurns.push(turn); + yield { type: 'agent_bound', slug: 'general' }; + yield { type: 'text_delta', text: `echo: ${turn.text}` }; + yield { type: 'done', answer: `echo: ${turn.text}`, toolCalls: 0, iterations: 1 }; + }, + }, + }), + ); + server = app.listen(0); + const addr = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${String(addr.port)}/chat`; + }); + + after(async () => { + await new Promise((r) => server.close(() => r())); + }); + + it('401s when no Authorization header is sent', async () => { + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ message: 'hi' }), + }); + assert.equal(res.status, 401); + }); + + it('401s for an unknown API key', async () => { + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: 'Bearer omk_not-a-real-key' }, + body: JSON.stringify({ message: 'hi' }), + }); + assert.equal(res.status, 401); + }); + + it('streams an NDJSON turn end-to-end for a valid key, and audits the call', async () => { + const created = await apiKeys.create({ label: 'streamer' }); + const before = (await auditLog.list()).length; + + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'ping', conversationId: 'conv-1' }), + }); + assert.equal(res.status, 200); + assert.match(res.headers.get('content-type') ?? '', /application\/x-ndjson/); + + const events = parseNdjson(await res.text()); + assert.deepEqual( + events.map((e) => (e as { type: string }).type), + ['agent_bound', 'text_delta', 'done'], + ); + + assert.equal(capturedTurns.length, 1); + assert.equal(capturedTurns[0]?.channelId, '@omadia/channel-api'); + // Namespaced by key identity (cross-key isolation fix) — never the raw + // caller-supplied conversationId on its own, and derived via a hash (not + // plain concatenation) so it can't collide after downstream sanitization + // (see the `internalConversationId` doc comment). + assert.equal( + capturedTurns[0]?.conversationId, + internalConversationId(created.record.id, 'conv-1'), + ); + assert.equal(capturedTurns[0]?.text, 'ping'); + // Design decision (issue #438): the key IS its own identity. + assert.deepEqual(capturedTurns[0]?.userRef, { + kind: 'custom', + id: `key:${created.record.id}`, + displayName: 'streamer', + }); + + const after = await auditLog.list(); + assert.equal(after.length, before + 1, 'exactly one audit row per authenticated call'); + const last = after[after.length - 1]; + assert.equal(last?.keyId, created.record.id); + assert.equal(last?.route, '/chat'); + assert.equal(last?.method, 'POST'); + }); + + it('401s once the key has been revoked — no further calls succeed', async () => { + const created = await apiKeys.create({ label: 'to-revoke' }); + const first = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'hi' }), + }); + assert.equal(first.status, 200); + + await apiKeys.revoke(created.record.id); + + const second = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'hi again' }), + }); + assert.equal(second.status, 401); + }); + + it('429s once a key exceeds its configured rate limit', async () => { + const created = await apiKeys.create({ label: 'limited', rateLimitPerMinute: 1 }); + const first = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'one' }), + }); + assert.equal(first.status, 200); + + const second = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'two' }), + }); + assert.equal(second.status, 429); + }); + + it('400s on an empty message', async () => { + const created = await apiKeys.create({ label: 'validator' }); + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: '' }), + }); + assert.equal(res.status, 400); + }); +}); + +/** Spins up a fresh router + server backed by its own store/log/limiter, for + * tests that need to control the `core.handleTurnStream` behavior per case + * (throwing, capturing turns) without cross-contaminating the shared + * `before()` fixture above. */ +function startTestServer(core: Pick): { + baseUrl: string; + apiKeys: ReturnType; + auditLog: ReturnType; + close: () => Promise; +} { + const secrets = createFakeSecrets(); + const apiKeys = createApiKeyStore(secrets); + const auditLog = createAuditLog(secrets); + const rateLimiter = createRateLimiter(); + + const app = express(); + app.use(express.json()); + app.use(createApiChatRouter({ channelId: '@omadia/channel-api', apiKeys, auditLog, rateLimiter, core })); + const server = app.listen(0); + const addr = server.address() as AddressInfo; + return { + baseUrl: `http://127.0.0.1:${String(addr.port)}/chat`, + apiKeys, + auditLog, + close: () => new Promise((r) => server.close(() => r())), + }; +} + +describe('channelApi/chatRouter — cross-key conversationId isolation (finding #1)', () => { + it('two different keys sending the identical caller-supplied conversationId never collide on the internal conversationId', async () => { + const capturedTurns: IncomingTurn[] = []; + const harness = startTestServer({ + async *handleTurnStream(turn) { + capturedTurns.push(turn); + yield { type: 'done', answer: 'ok', toolCalls: 0, iterations: 1 }; + }, + }); + + const keyA = await harness.apiKeys.create({ label: 'A' }); + const keyB = await harness.apiKeys.create({ label: 'B' }); + + await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${keyA.token}` }, + body: JSON.stringify({ message: 'hi from A', conversationId: 'shared-thread' }), + }); + await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${keyB.token}` }, + body: JSON.stringify({ message: 'hi from B', conversationId: 'shared-thread' }), + }); + + assert.equal(capturedTurns.length, 2); + assert.notEqual( + capturedTurns[0]?.conversationId, + capturedTurns[1]?.conversationId, + 'identical caller-supplied conversationId must still map to distinct internal scopes per key', + ); + assert.equal( + capturedTurns[0]?.conversationId, + internalConversationId(keyA.record.id, 'shared-thread'), + ); + assert.equal( + capturedTurns[1]?.conversationId, + internalConversationId(keyB.record.id, 'shared-thread'), + ); + + await harness.close(); + }); +}); + +describe('channelApi/chatRouter — same-key conversationId collision via lossy sanitizeScope (finding #3)', () => { + it('two caller-supplied conversationIds differing only in punctuation never collide after sanitizeScope', async () => { + const capturedTurns: IncomingTurn[] = []; + const harness = startTestServer({ + async *handleTurnStream(turn) { + capturedTurns.push(turn); + yield { type: 'done', answer: 'ok', toolCalls: 0, iterations: 1 }; + }, + }); + + const key = await harness.apiKeys.create({ label: 'punctuation' }); + + await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${key.token}` }, + body: JSON.stringify({ message: 'hi', conversationId: 'case/a' }), + }); + await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${key.token}` }, + body: JSON.stringify({ message: 'hi', conversationId: 'case?a' }), + }); + + assert.equal(capturedTurns.length, 2); + const idA = capturedTurns[0]?.conversationId ?? ''; + const idB = capturedTurns[1]?.conversationId ?? ''; + // The internal conversationIds themselves must differ... + assert.notEqual(idA, idB); + // ...and, critically, so must the scope SessionLogger actually persists + // under (`graphScopeFor` applies the real `sanitizeScope`, which lower- + // cases and collapses any punctuation run to a single '-' — the exact + // transform that made `"case/a"` and `"case?a"` collide before this fix, + // since plain concatenation left that punctuation exposed). + assert.notEqual(graphScopeFor(undefined, idA), graphScopeFor(undefined, idB)); + + await harness.close(); + }); + + it('two long caller-supplied conversationIds differing only past the 80-char sanitizeScope truncation cutoff never collide', async () => { + const capturedTurns: IncomingTurn[] = []; + const harness = startTestServer({ + async *handleTurnStream(turn) { + capturedTurns.push(turn); + yield { type: 'done', answer: 'ok', toolCalls: 0, iterations: 1 }; + }, + }); + + const key = await harness.apiKeys.create({ label: 'truncation' }); + // Well past 80 chars post-sanitization once namespaced with a key id — + // these two only diverge after the point sanitizeScope would truncate a + // plain-concatenated scope, which is exactly what made them collide + // before this fix. + const longBase = 'x'.repeat(150); + + await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${key.token}` }, + body: JSON.stringify({ message: 'hi', conversationId: `${longBase}-tail-one` }), + }); + await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${key.token}` }, + body: JSON.stringify({ message: 'hi', conversationId: `${longBase}-tail-two` }), + }); + + assert.equal(capturedTurns.length, 2); + const idA = capturedTurns[0]?.conversationId ?? ''; + const idB = capturedTurns[1]?.conversationId ?? ''; + assert.notEqual(idA, idB); + assert.notEqual(graphScopeFor(undefined, idA), graphScopeFor(undefined, idB)); + + await harness.close(); + }); +}); + +describe('channelApi/chatRouter — audit-log accuracy for every authenticated outcome (finding #2)', () => { + it('does NOT audit an unauthenticated call (missing key)', async () => { + const harness = startTestServer({ + async *handleTurnStream() { + yield { type: 'done', answer: 'x', toolCalls: 0, iterations: 1 }; + }, + }); + + const res = await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ message: 'hi' }), + }); + assert.equal(res.status, 401); + assert.equal( + (await harness.auditLog.list()).length, + 0, + 'a call that never authenticated must not produce an audit entry', + ); + await harness.close(); + }); + + it('audits status "rate_limited" for an authenticated call over quota — never "ok"', async () => { + const harness = startTestServer({ + async *handleTurnStream() { + yield { type: 'done', answer: 'x', toolCalls: 0, iterations: 1 }; + }, + }); + const created = await harness.apiKeys.create({ label: 'quota', rateLimitPerMinute: 1 }); + + await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'one' }), + }); + const res = await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'two' }), + }); + assert.equal(res.status, 429); + + const entries = await harness.auditLog.list(); + assert.equal(entries.length, 2, 'both the accepted and the rejected call are audited'); + assert.equal(entries[0]?.status, 'ok'); + assert.equal(entries[1]?.status, 'rate_limited'); + assert.equal(entries[1]?.keyId, created.record.id); + + await harness.close(); + }); + + it('audits status "invalid_request" for a schema-invalid body — never "ok"', async () => { + const harness = startTestServer({ + async *handleTurnStream() { + yield { type: 'done', answer: 'x', toolCalls: 0, iterations: 1 }; + }, + }); + const created = await harness.apiKeys.create({ label: 'validator' }); + + const res = await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: '' }), + }); + assert.equal(res.status, 400); + + const entries = await harness.auditLog.list(); + assert.equal(entries.length, 1); + assert.equal(entries[0]?.status, 'invalid_request'); + + await harness.close(); + }); + + it('audits status "error" — never "ok" — when the orchestrator throws mid-turn', async () => { + const harness = startTestServer({ + async *handleTurnStream() { + await Promise.resolve(); + throw new Error('orchestrator exploded'); + }, + }); + const created = await harness.apiKeys.create({ label: 'crasher' }); + + const res = await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'hi' }), + }); + // Headers are already flushed (200) before dispatch starts — the error + // surfaces as an NDJSON event on the wire, not an HTTP error status. + assert.equal(res.status, 200); + const body = await res.text(); + assert.ok(body.includes('orchestrator exploded')); + + const entries = await harness.auditLog.list(); + assert.equal(entries.length, 1, 'exactly one audit row for this authenticated call'); + assert.equal( + entries[0]?.status, + 'error', + 'a mid-turn throw must be audited as "error", not optimistically as "ok"', + ); + + await harness.close(); + }); + + it('audits status "error" — never "ok" — for an in-band {type:"error"} event with no throw', async () => { + // The real orchestrator (and the verifier-wrapped orchestrator, when + // verifier mode is on) can yield a `{type:'error', message}` event on an + // already-open 200 stream WITHOUT throwing — the async iterator + // completes normally. This is the exact bug class that hit issue #403: + // nothing throws, so a naive "loop completed => audit ok" is wrong. + const harness = startTestServer({ + async *handleTurnStream() { + yield { type: 'text_delta', text: 'partial answer before things went wrong' }; + yield { type: 'error', message: 'downstream tool call failed' }; + }, + }); + const created = await harness.apiKeys.create({ label: 'in-band-error' }); + + const res = await fetch(harness.baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ message: 'hi' }), + }); + assert.equal(res.status, 200); + const events = parseNdjson(await res.text()); + assert.deepEqual( + events.map((e) => (e as { type: string }).type), + ['text_delta', 'error'], + ); + + const entries = await harness.auditLog.list(); + assert.equal(entries.length, 1, 'exactly one audit row for this authenticated call'); + assert.equal( + entries[0]?.status, + 'error', + 'an in-band error event with no throw must be audited as "error", not "ok"', + ); + + await harness.close(); + }); +}); diff --git a/middleware/test/channelApi/chatRouterPrivacyIntegration.test.ts b/middleware/test/channelApi/chatRouterPrivacyIntegration.test.ts new file mode 100644 index 000000000..d6bcdddc5 --- /dev/null +++ b/middleware/test/channelApi/chatRouterPrivacyIntegration.test.ts @@ -0,0 +1,189 @@ +/** + * Issue #438 acceptance: "API chat responses pass through privacy-guard PII + * masking the same way other channels' responses do — test asserts masking + * actually ran (not just that the plugin was called)." + * + * Exercises the REAL orchestrator turn pipeline and the REAL privacy-guard + * service (same construction as `test/orchestrator/promptMaskPipeline.test.ts` + * — the established pattern in this repo for "real orchestrator, fake LLM") + * wired through `createApiChatRouter`'s `core.handleTurnStream` seam, exactly + * as `@omadia/channel-api`'s `plugin.ts` wires the real `CoreApi.handleTurnStream`. + * No second/parallel masking path is built here — the plugin reuses whatever + * `Orchestrator.chatStream` already does for every other channel. + */ + +import { strict as assert } from 'node:assert'; +import { after, before, describe, it } from 'node:test'; +import type { AddressInfo } from 'node:net'; +import type { Server } from 'node:http'; + +import express from 'express'; +import type { IncomingTurn } from '@omadia/channel-sdk'; +import type { + LlmProvider, + LlmRequest, + LlmResponse, + LlmStreamEvent, +} from '@omadia/llm-provider'; +import { NativeToolRegistry, Orchestrator } from '@omadia/orchestrator'; +import { createPrivacyGuardService } from '@omadia/plugin-privacy-guard/dist/index.js'; + +import { createApiKeyStore } from '../../packages/harness-api-key-auth/src/apiKeyStore.js'; +import { createAuditLog } from '../../packages/harness-api-key-auth/src/auditLog.js'; +import { createRateLimiter } from '../../packages/harness-api-key-auth/src/rateLimiter.js'; +import { createApiChatRouter } from '../../packages/harness-channel-api/src/chatRouter.js'; +import { createFakeSecrets } from './testSecrets.js'; + +const providerCapabilities = { + tools: true, + vision: true, + streaming: true, + promptCaching: true, + forcedToolChoice: true, + parallelToolCalls: true, +} as const; + +const RAW_EMAIL = 'anna.schmidt@firma.de'; +const EMAIL_RE = /[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/; + +/** The privacy-guard service with the #361 user-prompt-masking flag forced on. */ +function maskingService(): ReturnType { + return createPrivacyGuardService({ + readConfig: (key: string) => (key === 'mask_user_prompt' ? 'on' : undefined), + }); +} + +function finalResponse(text: string): LlmResponse { + return { + content: [{ type: 'text', text }], + finishReason: 'stop', + providerFinishReason: 'end_turn', + model: 'test', + usage: { inputTokens: 10, outputTokens: 1, cacheReadTokens: 0, cacheWriteTokens: 0 }, + }; +} + +/** Streaming fake: echoes back whatever email-shaped token IT SAW on the + * wire (the masked surrogate, if masking ran — the raw email otherwise). */ +function echoingStreamProvider(requests: string[]): LlmProvider { + return { + id: 'anthropic', + capabilities: providerCapabilities, + complete: (): Promise => { + throw new Error('echoingStreamProvider: complete() not scripted — chatStream uses stream()'); + }, + stream: async function* (req: LlmRequest): AsyncIterable { + const serialized = JSON.stringify(req); + requests.push(serialized); + const email = EMAIL_RE.exec(serialized)?.[0] ?? 'no-email-in-request'; + const text = `Notiert. Ich schreibe an ${email}.`; + yield { type: 'text_delta', text }; + yield { type: 'final', response: finalResponse(text) }; + }, + classifyError: () => ({ retryable: false, kind: 'other' as const }), + } as unknown as LlmProvider; +} + +function parseNdjson(body: string): Array> { + return body + .split('\n') + .filter((line) => line.trim().length > 0) + .map((line) => JSON.parse(line) as Record); +} + +describe('channelApi/chatRouter — real orchestrator + real privacy-guard', () => { + let server: Server; + let baseUrl: string; + let apiKeys: ReturnType; + const mainRequests: string[] = []; + + before(() => { + const secrets = createFakeSecrets(); + apiKeys = createApiKeyStore(secrets); + const auditLog = createAuditLog(secrets); + const rateLimiter = createRateLimiter(); + + const orchestrator = new Orchestrator({ + provider: echoingStreamProvider(mainRequests), + model: 'test', + maxTokens: 1024, + maxToolIterations: 3, + domainTools: [], + nativeToolRegistry: new NativeToolRegistry(), + privacyGuard: () => maskingService(), + } as ConstructorParameters[0]); + + const app = express(); + app.use(express.json()); + app.use( + createApiChatRouter({ + channelId: '@omadia/channel-api', + apiKeys, + auditLog, + rateLimiter, + core: { + handleTurnStream(turn: IncomingTurn) { + return orchestrator.chatStream({ + userMessage: turn.text, + sessionScope: turn.conversationId, + }); + }, + }, + }), + ); + server = app.listen(0); + const addr = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${String(addr.port)}/chat`; + }); + + after(async () => { + await new Promise((r) => server.close(() => r())); + }); + + it('masks PII on the wire to the LLM, and the streamed done event carries a privacy receipt', async () => { + const created = await apiKeys.create({ label: 'privacy-check' }); + + const res = await fetch(baseUrl, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${created.token}` }, + body: JSON.stringify({ + message: `Bitte schreibe an ${RAW_EMAIL} wegen des Vertrags.`, + conversationId: 'privacy-conv-1', + }), + }); + assert.equal(res.status, 200); + const events = parseNdjson(await res.text()); + + // (1) The masking actually ran BEFORE the request left the process: the + // fake LLM provider never saw the raw email on the wire. + assert.equal(mainRequests.length, 1); + assert.ok( + !mainRequests[0]!.includes(RAW_EMAIL), + 'the LLM request must not contain the raw email — masking must have run', + ); + const surrogate = EMAIL_RE.exec(mainRequests[0]!)?.[0]; + assert.ok(surrogate, 'the LLM request must carry a masked email-shaped surrogate instead'); + assert.notEqual(surrogate, RAW_EMAIL); + + // (2) The `done` event on the PUBLIC API stream carries the aggregate + // privacy receipt — the same field every other channel's `/chat/stream` + // exposes (src/routes/chat.ts forwards it unchanged). + const done = events.find((e) => e['type'] === 'done'); + assert.ok(done, 'stream must end with a done event'); + const receipt = done?.['privacyReceipt'] as + | { maskedPromptSpans?: Array<{ type: string }> } + | undefined; + assert.ok(receipt, 'done event must carry a privacyReceipt — proves masking ran, not just that the plugin loaded'); + assert.ok( + (receipt.maskedPromptSpans ?? []).some((s) => s.type === 'email'), + 'receipt must record the masked email span', + ); + + // (3) Restore-on-answer: the user-facing answer carries the REAL value + // back (masking is transport-only, never a lossy transform for the caller). + assert.ok( + typeof done?.['answer'] === 'string' && (done['answer'] as string).includes(RAW_EMAIL), + 'the answer streamed back to the API caller must restore the real email', + ); + }); +}); diff --git a/middleware/test/channelApi/manifest.test.ts b/middleware/test/channelApi/manifest.test.ts new file mode 100644 index 000000000..efefa3b46 --- /dev/null +++ b/middleware/test/channelApi/manifest.test.ts @@ -0,0 +1,45 @@ +import { strict as assert } from 'node:assert'; +import { fileURLToPath } from 'node:url'; +import { describe, it } from 'node:test'; + +import { loadManifestFromPath } from '../../src/plugins/manifestLoader.js'; + +/** + * Issue #438 — the manifest for the new public API channel package. Mirrors + * `test/uiChannelPlugin.test.ts`'s manifest coverage for `@omadia/ui-channel`. + */ +describe('@omadia/channel-api manifest', () => { + it('is a valid schema-v1 channel manifest with a chat webhook route', async () => { + const manifestPath = fileURLToPath( + new URL('../../packages/harness-channel-api/manifest.yaml', import.meta.url), + ); + const entry = await loadManifestFromPath(manifestPath); + assert.ok(entry, 'manifest loads as a valid schema-v1 document'); + assert.equal(entry.plugin.kind, 'channel'); + assert.equal(entry.plugin.id, '@omadia/channel-api'); + + const channel = entry.plugin.channel; + assert.ok(channel, 'channel block present'); + assert.equal(channel.transport.kind, 'webhook'); + assert.ok( + channel.transport.routes.some( + (r) => r.path === '/api/public/v1/chat' && r.method === 'POST', + ), + 'declares the public chat route', + ); + assert.ok(channel.capabilities.includes('text')); + + // No dispatch_service — a classic channel, dispatches to the shared + // chatAgent like Teams/Telegram (see IncomingTurn.channelType doc). + assert.equal(channel.dispatch_service, undefined); + }); + + it('declares permissions.secrets.runtime_write (required — API keys are vault-backed)', async () => { + const manifestPath = fileURLToPath( + new URL('../../packages/harness-channel-api/manifest.yaml', import.meta.url), + ); + const entry = await loadManifestFromPath(manifestPath); + assert.ok(entry); + assert.equal(entry.plugin.permissions_summary.secrets_runtime_write, true); + }); +}); diff --git a/middleware/test/channelApi/plugin.test.ts b/middleware/test/channelApi/plugin.test.ts new file mode 100644 index 000000000..3242a5105 --- /dev/null +++ b/middleware/test/channelApi/plugin.test.ts @@ -0,0 +1,60 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import type { CoreApi } from '../../packages/harness-channel-sdk/src/index.js'; +import type { PluginContext, SecretsAccessor } from '../../packages/plugin-api/src/index.js'; +import { API_PREFIX, activate } from '../../packages/harness-channel-api/src/plugin.js'; +import { createFakeSecrets } from './testSecrets.js'; + +/** Mirrors `test/uiChannelPlugin.test.ts`'s `makeMocks()` for the sibling + * `@omadia/ui-channel` package, adapted to `registerRouter` instead of a + * single `registerRoute`. */ +function makeMocks(secrets: SecretsAccessor) { + const ctx = { + agentId: '@omadia/channel-api', + log: () => {}, + secrets, + } as unknown as PluginContext; + const captured: { channelId?: string; prefix?: string; router?: unknown } = {}; + const core = { + registerRouter: (channelId: string, prefix: string, router: unknown) => { + captured.channelId = channelId; + captured.prefix = prefix; + captured.router = router; + }, + } as unknown as CoreApi; + return { ctx, core, captured }; +} + +describe('@omadia/channel-api activate', () => { + it('mounts one router at /api/public/v1, scoped to its own channelId', async () => { + const { ctx, core, captured } = makeMocks(createFakeSecrets()); + const handle = await activate(ctx, core); + assert.equal(captured.channelId, '@omadia/channel-api'); + assert.equal(captured.prefix, API_PREFIX); + assert.ok(captured.router, 'a router was registered'); + assert.ok(handle.close, 'returns a closeable handle'); + await handle.close(); + }); + + it('degrades to inert (no router mounted) when ctx.secrets has no write access', async () => { + // A read-only accessor — as a plugin gets when the manifest is missing + // permissions.secrets.runtime_write. activate() must not throw; it must + // simply not mount the routes (see the doc comment in plugin.ts). + const readOnlySecrets: SecretsAccessor = { + async get() { + return undefined; + }, + async require(key: string) { + throw new Error(`missing ${key}`); + }, + async keys() { + return []; + }, + }; + const { ctx, core, captured } = makeMocks(readOnlySecrets); + const handle = await activate(ctx, core); + assert.equal(captured.router, undefined, 'no router registered without write access'); + await handle.close(); + }); +}); diff --git a/middleware/test/channelApi/publicPathsExemption.test.ts b/middleware/test/channelApi/publicPathsExemption.test.ts new file mode 100644 index 000000000..b2fc7db9b --- /dev/null +++ b/middleware/test/channelApi/publicPathsExemption.test.ts @@ -0,0 +1,41 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { publicPaths } from '../../src/auth/publicPaths.js'; + +/** + * Issue #438 — the public chat ingress must bypass the `requireAuth` session + * gate (it authenticates itself via API key); the sibling key-lifecycle + * admin routes under the SAME `/api/public/v1` prefix must NOT be exempted + * here. Asserted against the SAME shared array production uses (see the doc + * comment at the top of `publicPaths.ts` for why this file exists as a + * constant). + * + * NOTE: not being exempted here is necessary but NOT sufficient for the + * admin routes' security — `core.registerRouter` (how this plugin actually + * mounts, in `plugin.ts`) never runs `requireAuth` at all, exempted or not + * (see `RoutesAccessor`'s doc comment on `PluginContext`: the kernel injects + * no auth middleware around a plugin-contributed router). The REAL gate for + * `/admin/keys` is `ctx.operatorAuth`, checked explicitly inside + * `adminKeysRouter.ts` itself — see that file's doc comment and + * `adminKeysRouter.test.ts`'s "operator-session auth" block for that + * coverage. This file only proves the (necessary, not sufficient) publicPaths + * half of the story. + */ +describe('publicPaths — @omadia/channel-api exemption', () => { + const paths = publicPaths({ devEndpointsEnabled: false }); + const isPublic = (path: string): boolean => paths.some((re) => re.test(path)); + + it('exempts the public chat route', () => { + assert.equal(isPublic('/api/public/v1/chat'), true); + }); + + it('does NOT exempt the key-admin routes — they stay session-gated', () => { + assert.equal(isPublic('/api/public/v1/admin/keys'), false); + assert.equal(isPublic('/api/public/v1/admin/keys/abc/revoke'), false); + }); + + it('does not exempt an unrelated path that merely starts with the prefix', () => { + assert.equal(isPublic('/api/public/v1/chatty-unrelated'), false); + }); +}); diff --git a/middleware/test/channelApi/rateLimiter.test.ts b/middleware/test/channelApi/rateLimiter.test.ts new file mode 100644 index 000000000..ecc1ad64f --- /dev/null +++ b/middleware/test/channelApi/rateLimiter.test.ts @@ -0,0 +1,22 @@ +import { strict as assert } from 'node:assert'; +import { describe, it } from 'node:test'; + +import { createRateLimiter } from '../../packages/harness-api-key-auth/src/rateLimiter.js'; + +describe('channelApi/rateLimiter', () => { + it('allows up to the configured per-minute capacity, then trips', () => { + const limiter = createRateLimiter(); + const capacity = 3; + for (let i = 0; i < capacity; i++) { + assert.equal(limiter.tryConsume('key-1', capacity), true, `call ${i + 1} within budget`); + } + assert.equal(limiter.tryConsume('key-1', capacity), false, 'call over budget is rejected'); + }); + + it('tracks each key independently', () => { + const limiter = createRateLimiter(); + assert.equal(limiter.tryConsume('key-a', 1), true); + assert.equal(limiter.tryConsume('key-a', 1), false, 'key-a is over budget'); + assert.equal(limiter.tryConsume('key-b', 1), true, 'key-b has its own bucket'); + }); +}); diff --git a/middleware/test/channelApi/testSecrets.ts b/middleware/test/channelApi/testSecrets.ts new file mode 100644 index 000000000..5b66acb22 --- /dev/null +++ b/middleware/test/channelApi/testSecrets.ts @@ -0,0 +1,30 @@ +import type { SecretsAccessor } from '../../packages/plugin-api/src/index.js'; + +/** + * In-memory `SecretsAccessor` for issue #438's channel-api tests — mirrors + * the shape `platform/pluginContext.ts` hands a plugin whose manifest + * declares `permissions.secrets.runtime_write`, without dragging in the real + * `FileSecretVault`/encryption machinery. + */ +export function createFakeSecrets(): SecretsAccessor { + const store = new Map(); + return { + async get(key: string) { + return store.get(key); + }, + async require(key: string) { + const v = store.get(key); + if (v === undefined) throw new Error(`fake secrets: missing key '${key}'`); + return v; + }, + async keys() { + return Array.from(store.keys()); + }, + async set(key: string, value: string) { + store.set(key, value); + }, + async delete(key: string) { + store.delete(key); + }, + }; +}