fix(web): fail closed when CLI linking has no database to write to - #349
Draft
DevQwinB wants to merge 1 commit into
Draft
fix(web): fail closed when CLI linking has no database to write to#349DevQwinB wants to merge 1 commit into
DevQwinB wants to merge 1 commit into
Conversation
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 blockchain-maxis#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 blockchain-maxis#191.
|
@DevQwinB Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
✅ Deploy Preview for stellar-signet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@DevQwinB is attempting to deploy a commit to the blockchainmaxis-8449's projects Team on Vercel. A member of the Team first needs to authorize it. |
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #277
The bug
Wallet links are
Walletrows in Postgres. The read path degrades gracefully with noDATABASE_URL—safeDbProfile/safeDbOperationsinlib/profiles.tsreturnnulland the caller falls through to a live chain read, then to the curated demo profiles. That is right for reads; a preview deployment with nothing provisioned still renders/p/{handle}.It is exactly wrong for writes. The same fall-through makes a link appear to succeed and persist nothing: the CLI prints success, the developer believes they are linked, and the failure surfaces later from some unrelated command that needed the binding. A link that silently persists nothing is worse than a refusal.
Change
apps/web/lib/profiles.ts— the write-path counterpartDeliberately in the same file as the read-path fall-through, right below it, so the asymmetry is visible where someone would otherwise copy the wrong pattern. A comment block says which half is which and why.
isDatabaseConfigured()DatabaseRequiredError— carriescode: 'database_required'andisConfigurationError: truerequireDatabase(operation)It checks configuration, not reachability. An unreachable database is a different failure with a different fix, and reporting it as "not configured" sends an operator to the wrong runbook.
POST /api/cli/pair/complete— fails closed, firstThe precondition is checked before the body is parsed and before any signature is looked at. If the result cannot be stored, nothing else about the request matters, and that ordering is the point of the issue.
{ "error": "database_required", "isConfigurationError": true, "message": "CLI wallet linking requires a database. … see docs/CLI.md and issue #191.", "docs": "…/docs/CLI.md#linking-requires-a-database" }503, not a4xx. Nothing the caller sent is wrong; the service is correctly configured to refuse rather than broken; and nothing the developer does to their own account changes it. TheisConfigurationErrorflag is what lets the CLI report a deployment problem without string-matching prose — telling someone their wallet or signature was bad would send them to debug the one thing that is working.cache-control: no-store, since nothing changes until an operator acts.Once the precondition passes the route returns
501, not200: pairing verification is #268's, and this will not pretend to complete a pairing it has not verified.GETon the same route reports{"available":…}so the page can ask before approval./link— says so before you approveReads the same signal server-side, so the page and the API cannot disagree, and disables approval with an explanation. Approving a link that gets refused seconds later is the worst version of this: the developer has already signed something. The notice states plainly that it is a deployment problem, not their account, and links #191 and the docs — because the person who can fix it is not the person reading the page.
Docs
New
docs/CLI.mdwith a#linking-requires-a-databasesection: why the read path degrades and the write path cannot, what each surface reports, and the operator's fix (provision, migrate, check/api/health). Cross-referenced from#191and from theDATABASE_URLrow inENVIRONMENT.md, which previously said only that web "degrades honestly" — true for reads, and now qualified for linking.Verification
isDatabaseConfiguredtracks the var;requireDatabasethrows a typed error whose message namesDATABASE_URLand#191; it is a no-op once configured; the read path still returnsnullrather than throwing (a regression there would break every preview deployment, so it is pinned);POST→ 503 with the right body andno-store; the refusal is>= 500and never a 4xx;POSTreaches 501 once configured;GETflips.node --test "lib/**/*.test.ts"inapps/web→ 215 tests passing, no regressions.pnpm --filter @signet/web typecheckclean,lintclean.prettier --checkclean on every file this PR adds.lib/profiles.tsanddocs/ENVIRONMENT.mdalready failprettier --checkonmain; I left that alone rather than bundling a reformat into this diff.Where this fits
The precondition is the part of #277 that does not depend on how pairing is laid out: it is about
DATABASE_URLand the write path, andlib/profiles.tsis where the issue itself points. The route is the thinnest possible carrier for it and is explicitly marked as #268's seam.Happy to rebase onto whichever pairing PR lands first and move the
requireDatabasecall into its handler — that is a one-line move, and the guard, the page and the docs are unaffected.