feat(db): add the database schema, migrations and auth storage - #3
Merged
Merged
Conversation
|
vpay had no SQL at all — docs/status.md listed the schema as the first thing that must be true before this can be called an MVP. This adds it, plus the storage the dashboard OIDC provider (ADR-0009) and the merchant API keys will need. Nothing here is reachable from a shipping binary. There is still no connection pool, no repository layer and no query in vpay-server or vpay-worker-bin; the router remains /healthz plus a fallback 404, and authkestra-op is a dev-dependency of the integration test crate only. The schema is real and proven; the auth is not built. Core payment schema (0001-0005): * currencies, providers, payment_intents, charges, ledger, mirroring the Rust types field-for-field. * `one_charge_per_intent` as a plain unique index, exactly as AGENTS.md and docs/flows/payment-lifecycle.md describe. * Two cross-column CHECKs that schemas/vpay.cstack could NOT express (its grammar promotes only single-field validators): `partial_refunds_imply_refunds` and `no_over_refund`. The flow docs were corrected earlier to say these were Rust-only; raw SQL has no such limitation, so they are corrected back — with the distinction that the CHECK bounds committed rows, not that SELECT ... FOR UPDATE exists, because nothing persists refunds yet. * Ledger balancing is deliberately NOT a constraint: it is an aggregate over sibling rows, which a row-level CHECK cannot see. It stays in vpay_ledger::Transaction::validate(). Auth storage (0006-0008): * 0006 transcribes authkestra-op 0.3.4's SqlxOpStore DDL verbatim. That SQL is hardcoded in the crate — schema name, table names and column types are not configurable — so this file must move in lockstep with the `=0.3.4` pin. oauth_device_codes is created despite the PKCE flow never using the device grant, because SqlxOpStore implements DeviceCodeStore unconditionally. * 0007 adds oauth_signing_keys, which vpay must own: authkestra has no signing-key type, store or rotation at any published version. The private key PEM is stored UNENCRYPTED; encryption at rest is not implemented and both the migration and a COMMENT ON COLUMN say so. * 0008 adds merchant_api_keys for Stripe-shaped sk_live_/sk_test_ keys, deliberately not routed through Authkestra: it has no opaque-key primitive, its verify_secret() is argon2 (wrong for a hot path), and client_credentials would add a token exchange that breaks Stripe SDK compatibility. Indexed prefix plus unique SHA-256 digest, livemode, and revoked_at — the instant revocation ADR-0008 says bearer keys lack. Only the digest is stored. Verification, against real postgres:16-alpine containers rather than by reading the SQL, per CLAUDE.md: * 78 passed / 3 skipped, up from 64 / 5. The ignored count fell because two placeholder tests with `unreachable!()` bodies became real. * Every CHECK, unique index and FK claimed above has a test that inserts a violating row and asserts `db_err.constraint()` names that specific constraint — so a test cannot pass by tripping a different one. * SqlxOpStore is driven against our transcribed DDL end to end (find_client, store_code, consume_code, then a second consume_code asserted None). That, not resemblance, is what proves 0006 correct. deny.toml ignores RUSTSEC-2023-0071 (Marvin Attack in `rsa`): no patched release exists and it is an unconditional dependency of authkestra-engine. Accepted deliberately, with the scope and the revisit conditions recorded inline. It now genuinely fires — confirmed via `cargo deny -L info`. `just ci` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stephane-segning
force-pushed
the
claude/dash-auth-authkestra-op
branch
from
August 9, 2026 16:21
f92f45c to
d24593f
Compare
4 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
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.
Summary
vpay had no SQL at all —
docs/status.mdlisted "Database schema + migrations" as the first thing that must be true before this could be called an MVP. This adds it, plus the storage the dashboard OIDC provider and the merchant API keys need.Nothing here is reachable from a shipping binary. There is still no connection pool, no repository layer and no query in
vpay-serverorvpay-worker-bin; the router remains/healthzplus a fallback 404, andauthkestra-opis a dev-dependency of the integration test crate only. The schema is real and proven. The auth is not built.docs/status.mdis written to make that unmissable.Source of truth: a maintainer decision to adopt Authkestra as the centralized auth engine for
/dash/v1, recorded in the new ADR-0009, together with the existing ADR-0008 (OIDC sessions, never an API key), ADR-0003 (a profile selects a config file, never a code path) and ADR-0006. The advisory decision is RUSTSEC-2023-0071.Intent
schemas/vpay.cstackdescribed the intended shape but is excluded from the build graph and generates nothing. Every invariant the flow docs promised at the database level was in fact enforced nowhere, or only in Rust.Scope
Core payment schema —
0001–0005currencies,providers,payment_intents,charges,ledger_transactions/ledger_entries, mirroring the Rust types field-for-field (enums transcribed variant-for-variant fromvpay-core).one_charge_per_intentis a plain unique index, exactly asAGENTS.mddescribes.Two cross-column CHECKs that
schemas/vpay.cstackcould not express — its grammar promotes only single-field validators — are now real:partial_refunds_imply_refundsandno_over_refund. The flow docs were corrected earlier in this effort to say these were Rust-only; raw SQL has no such limitation, so they are corrected back, with one distinction preserved: the CHECK guarantees no committed row is ever over-refunded, but no application-levelSELECT … FOR UPDATEexists, because nothing persists refunds yet.Ledger balancing is deliberately not a constraint.
SUM(debit) = SUM(credit)is an aggregate over sibling rows, which a row-level CHECK cannot see. It stays invpay_ledger::Transaction::validate(). A deferred constraint trigger would have been new, unexercised logic duplicating a tested check — documented as a gap instead of faked.Auth storage —
0006–00080006transcribesauthkestra-op0.3.4'sSqlxOpStoreDDL verbatim. That SQL is hardcoded in the crate — schema name (authkestra.), table names and column types are not configurable — so a transcription that is off by one column type compiles fine and fails at runtime. This file must move in lockstep with the=0.3.4pin, and says so.oauth_device_codesis created despite the PKCE flow never using the device grant, becauseSqlxOpStoreimplementsDeviceCodeStoreunconditionally.0007addsoauth_signing_keys, which vpay must own: authkestra has no signing-key type, store or rotation at any published version (verified by grep acrossauthkestra-op-0.3.4andauthkestra-engine-0.3.4;attestation.rs's own module doc claims otherwise and is stale). Partial unique indexone_active_signing_key, plusactive_key_has_no_expiryandexpiry_after_creation— two CHECKs the only known precedent (~/dev/vsms) lacks, because an accidentally-expired active key locks every staff member out with no fallback IdP.0008addsmerchant_api_keysfor Stripe-shapedsk_live_/sk_test_keys, deliberately not routed through Authkestra: it has no opaque-key primitive, itsverify_secret()is argon2 (correct for low-entropy passwords, wrong for a payment API's hot path), andclient_credentialswould add a token-exchange round trip that breaks Stripe SDK compatibility. Indexedkey_prefix+ unique SHA-256key_digest,livemode, andrevoked_at— the instant revocation ADR-0008 says bearer keys lack. Only the digest is stored.Verification
Against real
postgres:16-alpinecontainers, perCLAUDE.md's "apply it to a real Postgres and prove the constraint fires" — not by reading SQL.cargo nextest run --workspacejust cicargo deny checkThe ignored count fell because two placeholder tests whose bodies were
unreachable!()became real implementations. Every CHECK, unique index and FK claimed above has a test that inserts a violating row and assertsdb_err.constraint()names that specific constraint — so a test cannot pass by tripping a different one.The test that matters most is
sqlx_op_store_round_trips_a_client_and_enforces_single_use_codes: it drives the realSqlxOpStoreagainst our transcribed DDL —find_client(proving the JSONB columns decode),store_code,consume_code, then a secondconsume_codeassertedNone. That, not resemblance, is what proves0006correct.Screenshots / Evidence
Real Postgres rejections, captured from the test run:
Independent transcription check: 30 columns in the crate's
migrate()literal, 30 in0006, none missing.Risk Assessment
Moderate, and concentrated in three places.
aes-gcmsits unused in[workspace.dependencies]. Documented in the migration and aCOMMENT ON COLUMN; it needs a decision before anything issues a token.0006is coupled to a pinned crate version.SqlxOpStore's DDL is string literals; bumpingauthkestra-opwithout re-transcribing produces runtime failures, not compile errors. The round-trip test is the guard.RUSTSEC-2023-0071is now ignored. The Marvin Attack timing side-channel inrsa, no patched release, an unconditional dependency ofauthkestra-engine. Accepted deliberately by the maintainer on 2026-08-09 with scope and revisit conditions recorded inline indeny.toml. It was preemptive when added and now genuinely fires —cargo deny -L inforeportsnote[advisory-ignored]againstrsa v0.9.10. What limits it: the attack targets PKCS#1 v1.5 decryption oracles, vpay's use is JWT signing, and/dash/v1is staff-only and off the merchant payment path.Unresolved and inherited from ADR-0009:
authkestra-ophas no revocation endpoint and no/tokenrate limiting. Mitigation (short TTLs, deny-list) is available, not decided.AI Usage Declaration
AI (Claude Opus 5 via Claude Code) performed this work, delegating implementation to parallel Sonnet sub-agents on disjoint file sets. The orchestrating session independently re-ran every gate and verified the load-bearing claims rather than accepting agent reports — including diffing the transcribed DDL against the crate source column by column, running the
SqlxOpStoreround-trip directly, and confirming the advisory ignore actually matches. Two agent claims were caught and corrected: one reported the local authkestra checkout as ahead of the published crate when it is0.2.4and behind, which would have grounded the schema in stale source; and an earlier "Docker unavailable" note indocs/status.mdwas wrong — Docker works here, only Docker Hub is unreachable.NotImplemented; the 8 declared tokens are unchanged.Reviewer Focus
0007's unencrypted PEM — the most consequential open decision here.0006vs. the crate. Diff it againstauthkestra-op-0.3.4/src/sqlx_store.rsyourself; a wrong column type is invisible until runtime.0008's shape — does prefix + SHA-256 digest +revoked_atmatch how you wantsk_keys issued and rotated?docs/status.md— is anything ✅ that would not fail a test if it broke? Signing keys and merchant API keys were held at 🟡 precisely because only their storage exists.