Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions packages/loopover-miner/docs/ams-auth-identity-research.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# AMS auth/identity research — options for a hosted, multi-tenant login layer

Research spike for **#5217**. AMS has no auth/identity layer today — self-host is single-operator, single-machine,
with no login concept at all. A hosted AMS needs one. This document surveys realistic approaches, evaluated
**against the installation-token / GitHub App patterns gittensory already operates** (ORB's token-broker,
`src/orb/broker.ts`), so the eventual auth/identity design issue starts from a baseline consistent with existing
infrastructure rather than an unrelated auth stack. **Research and writeup only — no auth flow, token exchange,
or provider integration is implemented or decided here; the output is a non-binding input to the maintainer-owned
auth design issue. It does not propose any change to `src/orb/broker.ts`'s existing behavior, cited strictly as
precedent.**

## Summary

**Recommendation (non-binding): lead with GitHub OAuth as the identity source, reusing ORB's existing
maintainer-OAuth self-enrollment + installation-token broker, and add a managed identity provider only if/when
AMS must admit non-GitHub tenants (email/SSO).** AMS is already a GitHub-centric system whose authority is the
GitHub *installation*; ORB's broker (`src/orb/broker.ts`, #1255) already proves installation ownership
server-side and mints short-lived installation tokens without the App private key ever leaving the center. Adding
a separate identity provider that duplicates "who owns this installation" would add attack surface for little
gain until a non-GitHub login requirement actually exists. A fully custom JWT/OIDC scheme is the highest-effort,
highest-risk option and is not recommended unless a specific requirement forces it.

Two properties of the existing precedent drive this:

1. **GitHub is already the authority.** ORB's "das-github-mirror" trust model treats the OPERATOR/installation
as the authority: the maintainer-OAuth self-enrollment path (`src/orb/oauth.ts`, referenced from
`broker.ts`) proves the caller is an **admin of the installation's account, server-side, before issuing** —
which is exactly the "who is this tenant and what may they act on" question a hosted auth layer must answer.
2. **The token-mint is already solved and centrally trusted.** The broker binds `installation_id` **at issue
time from the enrollment row, never from the request** (`broker.ts:8`, `:30`), so a stolen secret for
install X can never mint a token for install Y. Any auth option must **reuse** this, not replace it — the
GitHub-token path is orthogonal to tenant *login*.

## Baseline

- **AMS today:** the miner (`packages/loopover-miner`) reads a `GITHUB_TOKEN` from env and acts as a single
operator — no login, no tenant, no session. This is the "before" the hosted auth layer must replace.
- **Existing precedent to build on (not re-invent):**
- `src/orb/broker.ts` (#1255) — central GitHub App token-broker. A self-hosted container exchanges a one-time
enrollment secret (shown once, stored only as a SHA-256 hash) for a short-lived (~1h) GitHub installation
token via `POST /v1/orb/token`; `installation_id` is bound server-side at issue time.
- `src/orb/oauth.ts` — maintainer-OAuth SELF-enrollment: proves the caller is an admin of the installation's
account before issuing, closing the privilege-escalation surface a red-team flagged.
- `src/orb/broker-client.ts` — the self-host side of the exchange; token cache lives with the App-key path in
`src/github/app.ts` (one mint per ~hour per installation).
- `src/auth/security` — `createOpaqueToken` / `hashToken` primitives already used for enrollment secrets.

## What a hosted AMS auth layer must do

1. **Establish tenant identity** — who is signing in.
2. **Map identity → the tenant's GitHub App installation(s)** — the unit AMS actually acts on.
3. **Authorize AMS resource access** — a tenant sees only their own loops/queues/ledgers (the `api_base_url`
composite-key scope, #5563, is where a `tenant_id` would attach; see the storage-abstraction research).
4. **Obtain GitHub tokens to act** — already handled by the broker; the auth layer must feed it, not replace it.
5. **Session management** — issue/refresh/revoke tenant sessions.

## Options

### Option 1 — GitHub OAuth as identity, reusing the ORB broker + self-enrollment — *recommended lead*

- **Identity:** GitHub OAuth login; the tenant *is* their GitHub account/installation, exactly the authority
`src/orb/oauth.ts` self-enrollment already establishes.
- **Broker interaction: REUSE.** Login proves installation admin (as oauth.ts already does), then the existing
broker mints installation tokens unchanged. No second source of truth for installation ownership.
- **Security:** smallest new surface — leans on GitHub's OAuth and the already-hardened server-side
`installation_id` binding; no new long-lived credential store beyond the existing hashed enrollment secrets.
- **Cost/limits:** requires every tenant to have a GitHub identity (true for AMS's current audience). Session
layer (cookies/JWT) still needed on top, but scoped and small.

### Option 2 — Managed identity provider (Auth0 / Clerk / WorkOS) alongside the broker

- **Identity:** a dedicated IdP handles login (email, SSO, social), decoupling tenant identity from GitHub.
- **Broker interaction: ALONGSIDE.** The IdP authenticates the *person*; a linking step still binds that
identity to a GitHub installation, after which the broker mints tokens unchanged. The IdP does **not** replace
the broker — it sits in front of it.
- **Security:** offloads session/credential handling to a hardened vendor (MFA, SSO, breach monitoring) at the
cost of a third-party dependency and an identity↔installation linking table that must be kept authoritative.
- **When it earns its keep:** only once AMS must admit tenants **without** a GitHub identity, or needs
enterprise SSO. Until then it duplicates "who owns this installation" that Option 1 already answers.

### Option 3 — Custom token-based scheme (JWT / OIDC) — *not recommended*

- **Identity:** AMS mints and validates its own JWTs/OIDC tokens for tenant sessions.
- **Broker interaction:** still must call the broker for GitHub tokens (RUN ALONGSIDE), so it adds an auth stack
without removing one.
- **Security:** re-implements key rotation, token revocation, and session hardening that a managed IdP or GitHub
OAuth provides for free — the largest surface to get wrong, and the reason to avoid it absent a hard requirement.

## Interaction with ORB's installation-token exchange (per the deliverable)

| Option | GitHub OAuth (identity) | ORB broker (`broker.ts`) | Net |
|---|---|---|---|
| 1 — GitHub OAuth + self-enrollment | is the identity | **reuse unchanged** | smallest surface; GitHub is authority |
| 2 — Managed IdP alongside | IdP is identity; GitHub linked | **reuse** (fed by a linking step) | adds SSO/non-GitHub login; extra linking table |
| 3 — Custom JWT/OIDC | self-issued | **reuse** (run alongside) | most to build + secure; not recommended |

In every option the broker's server-side `installation_id` binding and short-lived token mint are **reused, not
replaced** — the token path is orthogonal to tenant login, and re-deriving installation ownership elsewhere would
reopen the privilege-escalation surface the self-enrollment design already closed.

## Recommendation (non-binding)

Start from **Option 1** (GitHub OAuth reusing the existing self-enrollment + broker) because GitHub is already
AMS's authority and the ownership-proof + token-mint are already built and hardened. Adopt **Option 2** (a
managed IdP, alongside — never replacing — the broker) only when a concrete non-GitHub-login or enterprise-SSO
requirement lands. Avoid **Option 3** unless a specific constraint rules the others out. This is an input to the
maintainer-owned auth/identity design issue, not the decision.
142 changes: 142 additions & 0 deletions packages/loopover-miner/docs/ams-storage-abstraction-research.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
# AMS storage-abstraction research — replacing the `node:sqlite` local-store for a hosted, tenant-scoped datastore

Research spike for **#5216**. AMS's local stores (`lib/local-store.js` and its siblings) use `node:sqlite`'s
`DatabaseSync` with one file per machine and no tenant concept; a hosted AMS needs a shared, tenant-scoped
datastore. This document compares realistic replacement backends **against AMS's existing read/write patterns**
— not a green-field storage design — so the follow-up (maintainer-owned) storage-abstraction design issue can
reuse an established adapter pattern rather than invent one. **Research and writeup only — no schema, storage
code, or `local-store.js` shape changes here; the recommendation is non-binding.** It intentionally does not
contradict the AMS Cloud Readiness architecture / reuse-boundary spec; where that spec later lands, it governs.

## Summary

**Recommendation (non-binding): adopt ORB's existing `SqliteDriver` adapter seam (`src/selfhost/d1-adapter.ts`
+ `src/selfhost/pg-adapter.ts`) rather than inventing a new one, and lead with managed Postgres for the hosted
tenant datastore, keeping Cloudflare D1 as the edge-native alternative.** ORB already abstracts *D1 or Postgres*
behind one async, D1-shaped interface (`prepare().bind().all()/.first()/.run()/.batch()`), with a
SQLite→Postgres SQL translator (`pg-dialect`), so AMS does not need a bespoke abstraction.

Two properties of AMS's current access patterns dominate the decision:

1. **AMS uses interactive, read-then-conditional-write transactions** — `portfolio-queue.js`'s `batchClaim`
(`BEGIN IMMEDIATE` → read active rows → per-row conditional claim → `COMMIT`, `lib/portfolio-queue.js:334-351`)
and `attempt-log.js:145-165`. **Postgres supports this natively** (a pinned `PoolClient`, real transactions —
`pg-adapter.ts:56` `runOn(client)`); **D1 does not** — it offers only an atomic `batch()` of *predetermined*
statements, so the read result cannot drive the writes inside one transaction. This is the single
backend-specific rework cost, and it is **larger for D1 than for Postgres**.
2. **The `DatabaseSync` API is synchronous; every hosted backend is async.** Converting the stores and their
callers from sync `.prepare().run()/.get()/.all()` to `await` is the dominant, backend-independent migration
cost. ORB's `SqliteDriver` keeps a synchronous `query()` core with async `all()/run()/first()` wrappers
(`d1-adapter.ts:20-26`), which bounds the change to the store layer's own call sites.

A KV / document store is a poor fit and is **not recommended**: AMS's correctness relies on relational,
single-statement atomicity (conditional `UPDATE … RETURNING`, composite-key `ON CONFLICT`) that KV cannot
express without app-level optimistic-concurrency scaffolding.

## What the datastore must support — AMS's actual access patterns

Enumerated from the stores (`lib/local-store.js`, `run-state.js`, `claim-ledger.js`, `portfolio-queue.js`,
`event-ledger.js`, `governor-state.js`), each with the property a replacement must preserve:

1. **Synchronous open + access (`DatabaseSync`).** `openLocalStoreDb` (`local-store.js:45-51`) does
`new DatabaseSync(path)`, `mkdirSync(…, 0o700)` + `chmodSync(…, 0o600)`, `PRAGMA busy_timeout`. Every store
call is synchronous. **A hosted backend is async → the store layer and its callers must become async.**
2. **Race-free single-statement claims.** Claims use `INSERT … ON CONFLICT … DO UPDATE … WHERE status <>
'in_progress'` and atomic `UPDATE … WHERE rowid = (SELECT … ORDER BY priority DESC … LIMIT 1) RETURNING *`
(portfolio-queue dequeue; `claim-ledger.js` `INSERT OR IGNORE` / `ON CONFLICT`). No read-then-write. **Needs
single-statement conditional writes with `RETURNING`.**
3. **Interactive multi-statement transactions.** `batchClaim` and `attempt-log` use `BEGIN IMMEDIATE` → read →
conditional per-row writes → `COMMIT`/`ROLLBACK`. **Needs interactive transactions** (see Summary #1).
4. **Append-only ledgers with a stable total order.** `event-ledger`/`claim-ledger` use
`id INTEGER PRIMARY KEY AUTOINCREMENT` and sequential `seq`-based reads. **Needs a monotonic sequence and
ordered range reads.**
5. **In-place schema migrations.** `schema-version.js` gates migrations on `PRAGMA user_version`; PK reshapes
are done by *table rebuild* ("SQLite cannot ALTER a PRIMARY KEY in place", governor-state / claim-ledger
`_v2`/`_v3` rebuilds). **A shared RDBMS replaces `user_version` with a real migration model and alters keys
in place; the rebuild dance disappears.**
6. **A proto-tenant scope key already exists.** Every store was widened to composite keys prefixed by
`api_base_url` (forge scoping, #5563): `PRIMARY KEY (api_base_url, repo_full_name, identifier)`,
`UNIQUE (api_base_url, repo_full_name, issue_number)`. **This is the natural insertion point for a
`tenant_id` column** — tenant scoping extends an existing composite-key pattern rather than adding a new axis.
7. **Machine-local lease liveness.** Stuck in-flight rows are reclaimed by age via `leased_at` (plus PID-liveness
elsewhere). **A hosted, multi-worker service must reclaim by lease *expiry time*, not by a local PID** —
independent of backend, but it interacts with #3's transaction model.
8. **Single-writer, file-per-machine concurrency.** `PRAGMA busy_timeout` + one file per process is the current
concurrency model. **A shared datastore introduces genuine cross-worker concurrency**, which is exactly why
the atomic-claim (#2) and interactive-transaction (#3) guarantees must be preserved, not weakened.

## Backend options

Each evaluated against the patterns above (not generic pros/cons).

### Option A — Managed Postgres (via ORB's `pg-adapter.ts`) — *recommended lead*

- **Interactive transactions (#3):** native — `pg-adapter.ts` runs a `batch` on a pinned `PoolClient`
(`runOn(client)`, `:56-65`); real `BEGIN … COMMIT` with read-then-write inside. Best fit for `batchClaim`.
- **Single-statement claims (#2):** native `INSERT … ON CONFLICT … RETURNING` and conditional `UPDATE …
RETURNING`. SQLite→Postgres differences (e.g. `RETURNING *`, autoincrement→`GENERATED`/`SERIAL`,
boolean/text affinity) are handled by the existing `translateSql`/`translateDdl` (`pg-dialect`).
- **Append-only order (#4):** `BIGSERIAL`/identity + `ORDER BY` — direct.
- **Migrations (#5):** real DDL migrations; PK reshapes in place — the `_v2/_v3` rebuild dance is retired.
- **Tenant scope (#6):** add `tenant_id` to the existing composite keys; optional row-level security.
- **Cost:** the async refactor (universal) + SQL-dialect coverage (mostly already in `pg-dialect`). Highest
operational weight (a managed PG instance), but the strongest correctness match to AMS's claim/lease semantics.

### Option B — Cloudflare D1 (via ORB's `d1-adapter.ts`) — *edge-native alternative*

- **SQL compatibility:** D1 *is* SQLite, so AMS's SQL and `PRAGMA`-adjacent shapes port with the least
rewriting; ORB's `SqliteDriver`/`Statement` is D1-shaped already.
- **Interactive transactions (#3):** **not supported** — D1 offers atomic `batch()` of predetermined
statements only. `batchClaim`'s read-then-conditional-write must be **reworked** to either a single
conditional `UPDATE … RETURNING` per claim or an optimistic-concurrency loop. This is the main D1-specific cost.
- **Single-statement claims (#2):** supported (`INSERT … ON CONFLICT`, `UPDATE … RETURNING`).
- **Tenant scope (#6):** either a `tenant_id` column or per-tenant database; per-DB size/write limits and the
lack of interactive transactions make it better for smaller/edge tenants than for the busiest.
- **Cost:** async refactor (universal) + rework of the interactive-transaction claim paths (#3). Lowest SQL
translation cost, highest transaction-model cost.

### Option C — KV / document store (Workers KV, DynamoDB-style) — *not recommended*

- **Atomicity (#2, #3):** no relational single-statement conditional writes and no general multi-key
transactions; the ordered-priority dequeue and `ON CONFLICT` claim would need app-level optimistic
concurrency / conditional-put scaffolding, re-implementing what the RDBMS gives for free — and risking the
exact double-claim races the current `INSERT … ON CONFLICT` design was built to prevent.
- **Append-only order (#4):** no server-assigned monotonic sequence; requires an external counter.
- **Verdict:** a semantic mismatch with AMS's relational, atomic-claim core. Not pursued further.

### Fit matrix

| AMS pattern | Postgres | D1 | KV/document |
|---|---|---|---|
| Interactive txn `batchClaim` (#3) | native | **rework (batch-only)** | not viable |
| Single-statement claim + `RETURNING` (#2) | native (via translate) | native | app-level only |
| Append-only monotonic order (#4) | native | native | external counter |
| In-place migrations / PK reshape (#5) | native | SQLite-style | n/a |
| Tenant scope via composite key (#6) | column + optional RLS | column or per-tenant DB | key-prefix only |
| SQL rewrite cost | medium (`pg-dialect`) | **lowest** | n/a |
| Async refactor cost (#1) | required | required | required |

## Migration cost (backend-independent + per-option)

- **Universal:** `DatabaseSync` (sync) → async adapter converts every store method and caller to `await`
(#1). Bounding this to the store layer via the `SqliteDriver` seam is the main lever to keep it tractable.
- **Postgres-specific:** SQL-dialect coverage — largely already implemented in `pg-dialect` (`translateSql`,
`translateDdl`); verify AMS-only constructs (e.g. `PRAGMA busy_timeout`, `RETURNING *`, `AUTOINCREMENT`).
- **D1-specific:** rework the interactive-transaction claim paths (`batchClaim`, `attempt-log`) into D1's
atomic `batch()` or a single conditional statement — the largest single behavioral change.

## Tenant-scoping fit

All three inherit AMS's existing `api_base_url` composite-key scoping (#5563) as the seam: add `tenant_id`
alongside it. Postgres additionally offers row-level security for defense-in-depth; D1 can alternatively give
each tenant its own database (strong isolation, at the cost of cross-tenant queries and per-DB limits). The
lease-liveness reclaim (#7) must move from machine/PID-local to lease-expiry-by-time for any shared backend.

## Consistency with ORB's adapter precedent

ORB already ships the exact seam AMS should reuse: `SqliteDriver` (`d1-adapter.ts:20`) with a `Statement` that
exposes `prepare/bind/all/first/run/raw/batch` and a `pg-adapter.ts` implementing the same surface over a PG
pool with `translateSql`/`translateDdl`. Reusing this means AMS gets **D1-or-Postgres behind one interface**
for free, and the follow-up design issue chooses the deployment target without a second abstraction. The
recommendation here — Postgres-lead, D1-alternative, KV-excluded — is a non-binding input to that
maintainer-owned design issue.