From ea6dc721cf5e04e761bd4d2e90aa57724e797c3a Mon Sep 17 00:00:00 2001 From: Qwin B <309188780+DevQwinB@users.noreply.github.com> Date: Sun, 30 Aug 2026 18:05:57 +0100 Subject: [PATCH] fix(web): fail closed when CLI linking has no database to write to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wallet links are Wallet rows in Postgres. The read path degrades gracefully with no DATABASE_URL — safeDbProfile and safeDbOperations return null and the caller falls through to a chain read, then to the curated demo profiles — and that is right for reads. It is exactly wrong for writes: the same fall-through makes a link appear to succeed while persisting nothing. The developer believes they are linked, the CLI believes it, and the failure surfaces later from some unrelated command that needed the binding. Add the write-path counterpart next to those helpers in lib/profiles.ts: isDatabaseConfigured, a typed DatabaseRequiredError carrying isConfigurationError, and requireDatabase. Only configuration is checked, not reachability — an unreachable database is a different failure with a different fix, and calling it "not configured" sends an operator to the wrong runbook. POST /api/cli/pair/complete checks the precondition first, before the body and before any signature: if the result cannot be stored, nothing else about the request matters. It answers 503 rather than a 4xx because nothing the caller sent is wrong and nothing they can do to their own account changes it, and carries isConfigurationError so a client can classify it without string-matching prose. Pairing verification itself is #268's; this route returns 501 once the precondition passes rather than pretending to complete a pairing it has not verified. /link reads the same signal server-side, so the page and the API cannot disagree, and disables approval with an explanation before the developer signs something that cannot be stored. docs/CLI.md documents the dependency, what each surface reports, and how an operator fixes it, cross-referenced from ENVIRONMENT.md and #191. --- apps/web/app/api/cli/pair/complete/route.ts | 84 +++++++++++++++++++++ apps/web/app/link/page.tsx | 69 +++++++++++++++++ apps/web/lib/cli-pair-complete.test.ts | 57 ++++++++++++++ apps/web/lib/database-required.test.ts | 59 +++++++++++++++ apps/web/lib/profiles.ts | 59 +++++++++++++++ docs/CLI.md | 65 ++++++++++++++++ docs/ENVIRONMENT.md | 2 +- 7 files changed, 394 insertions(+), 1 deletion(-) create mode 100644 apps/web/app/api/cli/pair/complete/route.ts create mode 100644 apps/web/app/link/page.tsx create mode 100644 apps/web/lib/cli-pair-complete.test.ts create mode 100644 apps/web/lib/database-required.test.ts create mode 100644 docs/CLI.md diff --git a/apps/web/app/api/cli/pair/complete/route.ts b/apps/web/app/api/cli/pair/complete/route.ts new file mode 100644 index 0000000..d2c2bc7 --- /dev/null +++ b/apps/web/app/api/cli/pair/complete/route.ts @@ -0,0 +1,84 @@ +import { + DATABASE_REQUIRED_CODE, + DatabaseRequiredError, + requireDatabase, +} from '../../../../../lib/profiles.ts'; + +export const dynamic = 'force-dynamic'; +export const runtime = 'nodejs'; + +/** + * `POST /api/cli/pair/complete` — the point at which a CLI pairing becomes a + * `Wallet` row. + * + * **This route currently implements the database precondition and nothing + * else.** The pairing verification itself — the challenge, the signature, the + * single-use code — is #268's, and this deliberately does not guess at it: + * the handler returns `501` once the precondition passes, rather than + * pretending to complete a pairing it has not verified. + * + * The precondition is checked **first**, before anything else, and that + * ordering is the point of #277. A link is a row in Postgres. With no + * `DATABASE_URL` there is nowhere to put it, and the read path's graceful + * fall-through (`safeDbProfile`, `safeDbOperations` in `lib/profiles.ts`) + * would make the link appear to succeed while persisting nothing. Failing + * closed here means the developer is told the truth at the moment they act, + * instead of discovering it later from an unrelated command. + * + * See `docs/CLI.md` and #191 (database provisioning). + */ +export async function POST(): Promise { + try { + // Before parsing the body, before touching a signature: if the result + // cannot be stored, nothing else about this request matters. + requireDatabase('CLI wallet linking'); + } catch (err) { + if (err instanceof DatabaseRequiredError) { + return Response.json( + { + error: err.code, + message: err.message, + // The flag a client branches on. Without it the CLI has to + // string-match a message to know this is not the user's fault. + isConfigurationError: true, + docs: 'https://github.com/blockchain-maxis/signet/blob/main/docs/CLI.md#linking-requires-a-database', + }, + { + // 503, not 400 or 500: the service is correctly configured to refuse + // rather than broken, and the condition is not the caller's doing. + status: 503, + // Nothing here changes until an operator provisions a database. + headers: { 'cache-control': 'no-store' }, + }, + ); + } + throw err; + } + + return Response.json( + { + error: 'not_implemented', + message: + 'Pairing verification is not implemented yet (see issue #268). The database ' + + 'precondition for linking is enforced above.', + }, + { status: 501 }, + ); +} + +/** + * `GET /api/cli/pair/complete` — whether linking can succeed at all right now. + * + * Exists so the `/link` page can warn **before** the developer approves, + * rather than after they have signed something that cannot be stored. + */ +export function GET(): Response { + const configured = process.env.DATABASE_URL ? true : false; + return Response.json( + { + available: configured, + ...(configured ? {} : { reason: DATABASE_REQUIRED_CODE }), + }, + { headers: { 'cache-control': 'no-store' } }, + ); +} diff --git a/apps/web/app/link/page.tsx b/apps/web/app/link/page.tsx new file mode 100644 index 0000000..ee56fea --- /dev/null +++ b/apps/web/app/link/page.tsx @@ -0,0 +1,69 @@ +import { isDatabaseConfigured } from '../../lib/profiles.ts'; + +export const dynamic = 'force-dynamic'; + +export const metadata = { + title: 'Link the CLI · Signet', + description: 'Approve a terminal pairing request for your Signet account.', +}; + +/** + * `/link` — the approval page a `signet link` pairing sends the developer to. + * + * The approval UI itself belongs to the pairing work (#258/#268). What this + * page owns today is the precondition: **if linking cannot be persisted, say + * so before the developer approves.** + * + * Checked server-side, on the same signal the API route enforces, so the two + * cannot disagree. Approving a link that will be refused seconds later is the + * worst version of this: the developer has signed something, believes they are + * linked, and finds out otherwise from an unrelated command. + */ +export default function LinkPage() { + const linkingAvailable = isDatabaseConfigured(); + + return ( +
+

Link the Signet CLI

+ + {linkingAvailable ? ( +

+ Approve the pairing request shown in your terminal. The approval flow itself lands with + the CLI pairing work. +

+ ) : ( +
+

+ Linking is unavailable on this deployment +

+

+ Wallet links are stored in a database, and this deployment has no{' '} + DATABASE_URL configured. Approving now would appear to succeed and save + nothing, so approval is disabled. +

+

+ {/* Whose problem this is, stated plainly. Nothing the developer + does to their own account will change it. */} + This is a configuration problem with the Signet deployment, not with your account. + Whoever operates this deployment needs to provision a database — see{' '} + + issue #191 + {' '} + and{' '} + + docs/CLI.md + + . +

+
+ )} +
+ ); +} diff --git a/apps/web/lib/cli-pair-complete.test.ts b/apps/web/lib/cli-pair-complete.test.ts new file mode 100644 index 0000000..47aa70d --- /dev/null +++ b/apps/web/lib/cli-pair-complete.test.ts @@ -0,0 +1,57 @@ +import { test, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; + +import { GET, POST } from '../app/api/cli/pair/complete/route.ts'; + +const original = process.env.DATABASE_URL; + +afterEach(() => { + if (original === undefined) delete process.env.DATABASE_URL; + else process.env.DATABASE_URL = original; +}); + +test('POST fails closed with 503 when no database is configured', async () => { + delete process.env.DATABASE_URL; + + const res = await POST(); + assert.equal(res.status, 503); + + const body = (await res.json()) as Record; + assert.equal(body.error, 'database_required'); + // 503 and this flag together are what let the CLI report a deployment + // problem rather than blaming the user's wallet or signature. + assert.equal(body.isConfigurationError, true); + assert.match(String(body.message), /requires a database/); + assert.match(String(body.docs), /docs\/CLI\.md/); + // Nothing changes until an operator acts, so nothing should be cached. + assert.equal(res.headers.get('cache-control'), 'no-store'); +}); + +test('the refusal is not a 4xx — nothing the caller sent is wrong', async () => { + delete process.env.DATABASE_URL; + const res = await POST(); + assert.ok(res.status >= 500, `expected a server-side status, got ${res.status}`); +}); + +test('POST gets past the precondition once a database is configured', async () => { + process.env.DATABASE_URL = 'postgres://localhost:5432/signet'; + + const res = await POST(); + // 501, not 200: pairing verification is #268's, and this route will not + // pretend to complete a pairing it has not verified. + assert.equal(res.status, 501); + const body = (await res.json()) as Record; + assert.equal(body.error, 'not_implemented'); +}); + +test('GET reports availability so /link can warn before approval', async () => { + delete process.env.DATABASE_URL; + let body = (await GET().json()) as Record; + assert.equal(body.available, false); + assert.equal(body.reason, 'database_required'); + + process.env.DATABASE_URL = 'postgres://localhost:5432/signet'; + body = (await GET().json()) as Record; + assert.equal(body.available, true); + assert.equal(body.reason, undefined); +}); diff --git a/apps/web/lib/database-required.test.ts b/apps/web/lib/database-required.test.ts new file mode 100644 index 0000000..28f8933 --- /dev/null +++ b/apps/web/lib/database-required.test.ts @@ -0,0 +1,59 @@ +import { test, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; + +import { + DATABASE_REQUIRED_CODE, + DatabaseRequiredError, + isDatabaseConfigured, + requireDatabase, + safeDbProfile, +} from './profiles.ts'; + +const original = process.env.DATABASE_URL; + +afterEach(() => { + if (original === undefined) delete process.env.DATABASE_URL; + else process.env.DATABASE_URL = original; +}); + +test('isDatabaseConfigured tracks DATABASE_URL', () => { + delete process.env.DATABASE_URL; + assert.equal(isDatabaseConfigured(), false); + + process.env.DATABASE_URL = 'postgres://localhost:5432/signet'; + assert.equal(isDatabaseConfigured(), true); +}); + +test('requireDatabase throws a typed, classifiable error when unconfigured', () => { + delete process.env.DATABASE_URL; + + assert.throws( + () => requireDatabase('CLI wallet linking'), + (err: unknown) => { + assert.ok(err instanceof DatabaseRequiredError); + assert.equal(err.code, DATABASE_REQUIRED_CODE); + // The flag is what lets a client say "deployment problem" without + // string-matching prose. + assert.equal(err.isConfigurationError, true); + assert.match(err.message, /requires a database/); + // Points at the fix and at #191, so the message is actionable by + // whoever can actually act on it. + assert.match(err.message, /DATABASE_URL/); + assert.match(err.message, /#191/); + return true; + }, + ); +}); + +test('requireDatabase is a no-op once a database is configured', () => { + process.env.DATABASE_URL = 'postgres://localhost:5432/signet'; + assert.doesNotThrow(() => requireDatabase('CLI wallet linking')); +}); + +test('the read path still degrades gracefully — this changes writes only', async () => { + delete process.env.DATABASE_URL; + // The whole point of #277 is that the two paths differ: reads fall through, + // writes fail closed. A regression that made reads throw would break every + // preview deployment. + assert.equal(await safeDbProfile('aquawolf'), null); +}); diff --git a/apps/web/lib/profiles.ts b/apps/web/lib/profiles.ts index 66f4017..a9f6498 100644 --- a/apps/web/lib/profiles.ts +++ b/apps/web/lib/profiles.ts @@ -242,6 +242,65 @@ export async function safeDbProfile(handle: string): Promise { } } +// --- Write path ----------------------------------------------------------- +// +// Everything above degrades: no `DATABASE_URL` means `safeDbProfile` / +// `safeDbOperations` return null and the caller falls through to the chain or +// the curated demo data. That is right for reads — a preview deployment with +// nothing provisioned still renders. +// +// It is exactly wrong for writes. A wallet link is a `Wallet` row; with no +// database there is nowhere to put it, and the same fall-through would make +// the link *appear* to succeed while persisting nothing. A link that silently +// persists nothing is worse than a refusal: the developer believes they are +// linked, the CLI believes it, and the next command that needs the binding +// fails somewhere far away from the cause. +// +// So the write path fails closed, loudly, and says whose problem it is. + +/** Machine-readable code for a write refused because no database is provisioned. */ +export const DATABASE_REQUIRED_CODE = 'database_required'; + +/** + * Whether a database is configured for this deployment. + * + * Only checks configuration, not reachability: an unreachable database is a + * different failure with a different fix, and reporting it as "not configured" + * would send an operator to the wrong runbook. + */ +export function isDatabaseConfigured(): boolean { + return Boolean(process.env.DATABASE_URL); +} + +/** + * Thrown by write paths that cannot proceed without a database. + * + * Carries `isConfigurationError` so a client can classify it without parsing + * prose. Nothing the user did caused this, and nothing they can do fixes it — + * telling them their wallet or signature was bad would send them to debug the + * one thing that is working. + */ +export class DatabaseRequiredError extends Error { + readonly code = DATABASE_REQUIRED_CODE; + + readonly isConfigurationError = true; + + constructor(operation: string) { + super( + `${operation} requires a database. This Signet deployment has no DATABASE_URL ` + + `configured, so there is nowhere to persist the result. This is a deployment ` + + `configuration problem, not a problem with your account — see docs/CLI.md ` + + `and issue #191.`, + ); + this.name = 'DatabaseRequiredError'; + } +} + +/** Throw unless a database is configured. Call before any write. */ +export function requireDatabase(operation: string): void { + if (!isDatabaseConfigured()) throw new DatabaseRequiredError(operation); +} + /** * A Stellar `Address`: either a `G…` account or a `C…` contract StrKey. The * registry binds a handle to an `Address`, so a contract-controlled identity diff --git a/docs/CLI.md b/docs/CLI.md new file mode 100644 index 0000000..b534e2f --- /dev/null +++ b/docs/CLI.md @@ -0,0 +1,65 @@ +# Signet CLI + +`signet link` pairs a terminal with a Signet account: the CLI opens an approval +page, you approve in the browser, and the pairing is recorded against your +account. + +> **Status.** The CLI itself is in progress (#251 and the issues that follow). +> What is documented and enforced today is the one hard infrastructure +> dependency the feature has — see below. + +--- + +## Linking requires a database + +**A Signet deployment with no `DATABASE_URL` configured cannot link a wallet.** +Linking is refused up front rather than failing later. + +### Why + +A wallet link is a `Wallet` row in Postgres. There is no other place it can go. + +Everything on the _read_ path degrades gracefully without a database: +`safeDbProfile` and `safeDbOperations` in +[`apps/web/lib/profiles.ts`](../apps/web/lib/profiles.ts) return `null` and the +caller falls through to a live chain read, then to the curated demo profiles. +A preview deployment with nothing provisioned still renders `/p/{handle}`. + +The write path cannot do that. If linking fell through the same way, the link +would _appear_ to succeed and persist nothing: the CLI would print success, the +developer would believe they were linked, and the failure would surface later +from some unrelated command that needed the binding. **A link that silently +persists nothing is worse than a refusal.** + +See [#191](https://github.com/blockchain-maxis/signet/issues/191) for database +provisioning, and [`ENVIRONMENT.md`](ENVIRONMENT.md) for `DATABASE_URL` itself. + +### What you see + +| Where | With no `DATABASE_URL` | +| ----------------------------- | ---------------------------------------------------------------------------------------- | +| `POST /api/cli/pair/complete` | `503` with `{"error":"database_required","isConfigurationError":true,…}` | +| `GET /api/cli/pair/complete` | `{"available":false,"reason":"database_required"}` | +| `/link` | Approval is disabled, with an explanation, **before** you approve | +| CLI | A deployment configuration error naming `DATABASE_URL` — not a wallet or signature error | + +The status is **`503`, not `4xx`**: nothing about the request was wrong, and +nothing the developer does to their own account will change the outcome. The +`isConfigurationError` flag is there so a client can classify it without +string-matching a message. + +`/link` checks the same signal server-side and refuses _before_ approval, +so nobody signs an approval that cannot be stored. + +### Fixing it + +This is for whoever operates the deployment, not for the developer trying to +link: + +1. Provision Postgres and set `DATABASE_URL` for `apps/web`. +2. Apply migrations — `pnpm db:deploy` (or `pnpm db:migrate` locally). +3. Confirm with `GET /api/health`: `checks.db` should no longer report + `"skipped"`. + +`GET /api/cli/pair/complete` returning `{"available":true}` means linking can +proceed. diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 55b7643..a4e8755 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -18,7 +18,7 @@ Required means “must be set for that surface to do its job in production.” O | Variable | Consumed by | Required / optional | Default | Behaviour when unset | | --- | --- | --- | --- | --- | -| `DATABASE_URL` | web, indexer | **Required** for indexer. **Optional** for web. | _(none)_ | **Web:** profile/activity loaders skip Postgres and use the static manifest (and chain resolve when configured) — demo `/p/*` keeps working. `/api/health` reports `checks.db: "skipped"`. Dashboard account writes that need Prisma return empty. **Indexer:** process refuses to start (`DATABASE_URL is required`). | +| `DATABASE_URL` | web, indexer | **Required** for indexer. **Optional** for web. | _(none)_ | **Web:** profile/activity loaders skip Postgres and use the static manifest (and chain resolve when configured) — demo `/p/*` keeps working. `/api/health` reports `checks.db: "skipped"`. Dashboard account writes that need Prisma return empty. **CLI linking is refused outright** (`503 database_required`) rather than degrading, because a link is a `Wallet` row and a link that persists nothing is worse than a refusal — see [`CLI.md`](CLI.md#linking-requires-a-database) and [#191](https://github.com/blockchain-maxis/signet/issues/191). **Indexer:** process refuses to start (`DATABASE_URL is required`). | | `STELLAR_NETWORK` | _(declared for ops; not read by current TS)_ | Optional | `testnet` in `.env.example` | No runtime effect today. Prefer `NEXT_PUBLIC_STELLAR_NETWORK` (web) and `INDEXER_NETWORK` (indexer). Kept so deploy docs and local `.env` stay aligned. | | `STELLAR_HORIZON_URL` | _(declared for ops; not read by current TS)_ | Optional | `https://horizon-testnet.stellar.org` | No runtime effect today. Indexer reads `INDEXER_HORIZON_URL` instead (same default). | | `SOROBAN_RPC_URL` | web (server) | Optional | Falls through to `NEXT_PUBLIC_SOROBAN_RPC_URL`, then `https://soroban-testnet.stellar.org` | Server-side registry reads (`lib/chain.ts`, directory, profile chain resolve, and the `/api/health` registry check) use the public URL / testnet default. Client claim flow never sees this var (uses `NEXT_PUBLIC_SOROBAN_RPC_URL` only). |