diff --git a/.changeset/protect-ffi-auth-error-code.md.deferred b/.changeset/protect-ffi-auth-error-code.md.deferred new file mode 100644 index 000000000..b5ec00443 --- /dev/null +++ b/.changeset/protect-ffi-auth-error-code.md.deferred @@ -0,0 +1,32 @@ +--- +'@cipherstash/protect-ffi': minor +--- + +Carry a failure's auth taxonomy across the JavaScript boundary. A thrown error +that originated in `stack-auth` — CipherStash's token service refusing to issue +or renew the service token every ZeroKMS request carries — now sets `authCode` +and `help` alongside the existing `code`, on both bindings and on each item of +`decryptBulkFallible`. `getAuthErrorCode(err)` reads it back, and +`ProtectAuthErrorCode` types it. + +Nothing carried it before. `Error::Auth` and `Error::ZeroKMS` are both +`#[error(transparent)]` with no `#[diagnostic(code(..))]`, so the whole auth +taxonomy arrived as an untyped `Error` with nothing but prose — no code, and no +`help`, since `miette` help is not part of an error's `Display`. That is the +half of each of those errors that says what to do about it. + +The case that made it matter is `USAGE_LIMIT_EXCEEDED`: an organisation over +its billing allowance is neither a credentials problem nor a transient one, and +a caller could not tell it apart from either. A well-behaved retry loop would +hammer a condition only a human with a billing page can clear. + +`authCode` is a separate field from `code`, not more members of it. `code` is +this package's own closed `ProtectErrorCode` set, pinned by `errorCodes.test.ts` +against the `#[diagnostic(code(..))]` attributes in `crates/protect-ffi/src/lib.rs`; +the auth set belongs to `stack-auth` and ships on its own release train, so +`ProtectAuthErrorCode` is deliberately an open union — narrow it with `===`, +don't `switch` exhaustively. + +The code is read off the `AuthError` variant, never off the message, which is +the same rule the `code` contract already states: a rename upstream is a +compile error rather than a silent downgrade to `UNKNOWN`. diff --git a/.changeset/usage-limit-refusal-guidance.md b/.changeset/usage-limit-refusal-guidance.md new file mode 100644 index 000000000..46d950c5a --- /dev/null +++ b/.changeset/usage-limit-refusal-guidance.md @@ -0,0 +1,48 @@ +--- +'@cipherstash/stack': minor +'stash': minor +--- + +Surface a CipherStash billing refusal as something you can act on, instead of a +sentence with no remedy in it. + +When an organisation is over its usage allowance, CipherStash's token service +refuses to issue or renew the service token behind every operation, answering +`402` with `Insufficient balance. Please upgrade your plan.` That reached a +caller as bare prose: it names no dashboard, and nothing in it says that +retrying — or rotating credentials — cannot help. A well-behaved retry loop +would hammer a condition only a human with a billing page can clear. + +**`@cipherstash/stack`.** Every failure that came from the token service now +carries `authCode`, and its `message` carries the remedy — including the +dashboard URL, which the underlying response does not have. It is folded into +`message` rather than parked in a sibling field because `throw new +Error(failure.message)` is how these are surfaced in practice, so guidance +anywhere else is guidance nobody reads at the moment it is needed. + +```typescript +const result = await client.encrypt(value, { column, table }) +if (result.failure?.authCode === 'USAGE_LIMIT_EXCEEDED') { + // Stop retrying. `result.failure.message` names dashboard.cipherstash.com. +} +``` + +`ORG_NOT_PROVISIONED` is the other terminal code, and needs the opposite advice: +the organisation is not registered with the usage system at all, so there is no +plan to upgrade and it goes to support. The remaining codes +(`NOT_AUTHENTICATED`, `WORKSPACE_MISMATCH`, `EXPIRED_TOKEN`, …) are populated +too, and where one of them carries remedy text — `MISSING_WORKSPACE_CRN` naming +`CS_WORKSPACE_CRN`, say — that text now reaches the message instead of being +dropped at the FFI boundary. Both entries are covered — native and +`wasm-inline` — as is +`Encryption()`, which throws rather than returning a `Result` and so attaches +`authCode` to the thrown error. + +Treat the set as open: it belongs to `@cipherstash/auth` and grows on its own +release train, so compare with `===` rather than switching exhaustively. + +**`stash`.** `stash auth login` and `stash env` print the remedy alongside the +diagnosis, and no longer answer a billing refusal with "run `stash auth login` +and try again" — a fresh login cannot mint a credential that is being withheld +on billing grounds. `stash env` reports it as `usage_limit_exceeded` rather than +`session_invalid`. diff --git a/packages/cli/src/commands/auth/__tests__/failure.test.ts b/packages/cli/src/commands/auth/__tests__/failure.test.ts new file mode 100644 index 000000000..383412504 --- /dev/null +++ b/packages/cli/src/commands/auth/__tests__/failure.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest' +import { + authFailureCode, + authFailureHint, + authFailureMessage, +} from '../failure.js' + +const LOGIN_HINT = 'Run `stash auth login` and try again.' + +/** An `AuthFailure` as `@cipherstash/auth` returns it. */ +const failure = (type: string, message: string, help?: string) => ({ + type, + error: new Error(message), + ...(help ? { help } : {}), +}) + +describe('authFailureMessage', () => { + it("appends stack-auth's help to the diagnosis", () => { + // `miette` help is not part of an error's `Display`, so the remedy was + // dropped at every call site: "Not authenticated" with no mention of how + // to authenticate. + expect( + authFailureMessage( + failure( + 'NOT_AUTHENTICATED', + 'Not authenticated', + 'Log in with `stash auth login`.', + ), + ), + ).toBe('Not authenticated. Log in with `stash auth login`.') + }) + + it('leaves a failure without help exactly as it was', () => { + expect(authFailureMessage(failure('INVALID_CLIENT', 'bad client'))).toBe( + 'bad client', + ) + }) + + it('does not double a terminal full stop', () => { + expect( + authFailureMessage(failure('SERVER_ERROR', 'Boom.', 'Try later.')), + ).toBe('Boom. Try later.') + }) +}) + +describe('authFailureHint', () => { + it('sends a usage-limit refusal to the dashboard instead of to login', () => { + // The whole reason this function exists. `LOGIN_HINT` is the right advice + // for a stale session and wrong for a billing refusal — a fresh login + // cannot mint a credential CTS is withholding on billing grounds. + const hint = authFailureHint( + failure('USAGE_LIMIT_EXCEEDED', 'Insufficient balance.'), + LOGIN_HINT, + ) + + expect(hint).toContain('https://dashboard.cipherstash.com') + expect(hint).not.toContain('auth login') + }) + + it('sends an unprovisioned org to support, not to billing', () => { + // A 402 has two causes and they need different remedies: an org over its + // allowance upgrades, an org the usage system has never heard of has + // nothing to buy. + const hint = authFailureHint( + failure('ORG_NOT_PROVISIONED', 'Not provisioned.'), + LOGIN_HINT, + ) + + expect(hint).toContain('support@cipherstash.com') + expect(hint).not.toContain('dashboard.cipherstash.com') + }) + + it('keeps the caller-supplied hint for an ordinary auth failure', () => { + expect( + authFailureHint(failure('EXPIRED_TOKEN', 'Token expired'), LOGIN_HINT), + ).toBe(LOGIN_HINT) + }) + + it('has no hint of its own when the caller supplies none', () => { + expect( + authFailureHint(failure('EXPIRED_TOKEN', 'Token expired')), + ).toBeUndefined() + }) +}) + +describe('authFailureCode', () => { + it('widens the code so a call site can compare it to an unreleased one', () => { + // The pinned `@cipherstash/auth` union does not name USAGE_LIMIT_EXCEEDED + // yet; comparing `failure.type` to it directly is a type error rather than + // a `false`. Routing through here keeps the CLI correct on both sides of + // that dependency bump. + expect( + authFailureCode(failure('USAGE_LIMIT_EXCEEDED', 'Insufficient balance.')), + ).toBe('USAGE_LIMIT_EXCEEDED') + }) +}) diff --git a/packages/cli/src/commands/auth/__tests__/login.test.ts b/packages/cli/src/commands/auth/__tests__/login.test.ts index d18a9906a..097d04ced 100644 --- a/packages/cli/src/commands/auth/__tests__/login.test.ts +++ b/packages/cli/src/commands/auth/__tests__/login.test.ts @@ -274,3 +274,60 @@ describe('login — interactive (non-json) failure handling', () => { expect(clack.log.error).toHaveBeenCalledWith('poll boom') }) }) + +describe('login — a CTS usage-limit refusal', () => { + /** The 402 CTS answers with when the organisation is over its allowance. */ + const usageLimit = () => ({ + failure: { + type: 'USAGE_LIMIT_EXCEEDED', + error: new Error('Insufficient balance. Please upgrade your plan.'), + help: 'The organisation has used its allowance for the current billing period. Upgrade the plan from the CipherStash dashboard, then retry.', + }, + }) + + it('points the user at the dashboard rather than at another login', async () => { + // Logging in again cannot mint a credential CTS is withholding on billing + // grounds, so the default "run `stash auth login`" hint would send the + // user round a loop that has no exit. + authMock.beginDeviceCodeFlow.mockResolvedValueOnce(usageLimit()) + spyExit() + + await expect( + login('us-east-1.aws', undefined, { json: false }), + ).rejects.toThrow('process.exit') + + expect(clack.log.info).toHaveBeenCalledWith( + expect.stringContaining('https://dashboard.cipherstash.com'), + ) + }) + + it("carries stack-auth's remedy into the message", async () => { + authMock.beginDeviceCodeFlow.mockResolvedValueOnce(usageLimit()) + spyExit() + + await expect( + login('us-east-1.aws', undefined, { json: false }), + ).rejects.toThrow('process.exit') + + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('used its allowance'), + ) + }) + + it('gives an agent the code to branch on, without the prose hint', async () => { + // The JSON stream carries `code` separately, so a consumer can stop + // retrying without parsing English. + authMock.beginDeviceCodeFlow.mockResolvedValueOnce(usageLimit()) + spyExit() + const out = captureJsonLines() + + await expect( + login('us-east-1.aws', undefined, { json: true }), + ).rejects.toThrow('process.exit') + + expect(out.lines()[0]).toMatchObject({ + status: 'error', + code: 'USAGE_LIMIT_EXCEEDED', + }) + }) +}) diff --git a/packages/cli/src/commands/auth/failure.ts b/packages/cli/src/commands/auth/failure.ts new file mode 100644 index 000000000..6e0b1292f --- /dev/null +++ b/packages/cli/src/commands/auth/failure.ts @@ -0,0 +1,94 @@ +/** + * Rendering for an `@cipherstash/auth` `AuthFailure` — the shape every CTS + * interaction in this CLI returns on the failure arm. + * + * Two things were being dropped at every call site. + * + * **`help`.** Every `AuthError` in stack-auth carries `miette` help — the + * sentence that says what to actually do — and `miette` help is not part of an + * error's `Display`. `failure.error.message` therefore prints the diagnosis + * without the remedy: "Not authenticated" with no mention of `stash auth + * login`, "Insufficient balance. Please upgrade your plan." with no mention of + * where a plan is upgraded. + * + * **The distinction between a fixable failure and a billing one.** The default + * hint on these paths is "run `stash auth login` and try again", which is + * correct for a stale session and actively misleading for an organisation over + * its usage limit: re-authenticating cannot mint a credential CTS is refusing + * on billing grounds, so the user burns a login round trip and lands back here. + */ + +/** + * The `AuthFailure` fields this module reads. + * + * Structural rather than an import of `AuthFailure` itself, because `type` has + * to be compared against codes the pinned `@cipherstash/auth` does not declare + * yet — `USAGE_LIMIT_EXCEEDED` and `ORG_NOT_PROVISIONED` ship with the CTS + * usage-limit work, and against the closed union those comparisons are a type + * error rather than a `false`. Widening here keeps the CLI correct on both + * sides of that bump instead of gating it on a dependency release. + */ +type RenderableFailure = { + type?: string + error: { message: string } + help?: string +} + +/** + * Hints that supersede the caller's default, keyed by CTS refusal code. + * + * Both of these are terminal for the CLI: nothing the user can type clears + * them, so the hint has to send them somewhere else entirely. + */ +const TERMINAL_HINTS: ReadonlyMap = new Map([ + [ + 'USAGE_LIMIT_EXCEEDED', + 'Your CipherStash organisation has used its allowance for the current billing period. Upgrade the plan at https://dashboard.cipherstash.com and try again — logging in again will not clear this.', + ], + [ + 'ORG_NOT_PROVISIONED', + 'Your CipherStash organisation is not registered with the usage system, so there is no plan to upgrade. Contact support@cipherstash.com — logging in again will not clear this.', + ], +]) + +/** + * The failure's CTS code, widened to a plain string. + * + * The widening is the point — see {@link RenderableFailure}. Comparing + * `failure.type` to `'USAGE_LIMIT_EXCEEDED'` at a call site is a type error + * against the pinned `@cipherstash/auth`, whose union does not name it yet; + * routed through here it is an ordinary string comparison that starts + * returning `true` the day the dependency ships the code. + */ +export function authFailureCode( + failure: RenderableFailure, +): string | undefined { + return failure.type +} + +/** + * What went wrong, plus the remedy stack-auth attached to it. + * + * Falls back to the bare message when the failure carries no help, so nothing + * gains a trailing separator it did not have before. + */ +export function authFailureMessage(failure: RenderableFailure): string { + const { message } = failure.error + if (!failure.help) return message + return message.endsWith('.') + ? `${message} ${failure.help}` + : `${message}. ${failure.help}` +} + +/** + * The hint to show for this failure — the caller's default, unless the failure + * is one no retry can clear. + * + * @param fallback the hint that applies to an ordinary auth failure + */ +export function authFailureHint( + failure: RenderableFailure, + fallback?: string, +): string | undefined { + return (failure.type && TERMINAL_HINTS.get(failure.type)) ?? fallback +} diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index 96f755740..7b174dc59 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -1,9 +1,36 @@ import auth from '@cipherstash/auth' import * as p from '@clack/prompts' import { emitJsonError, emitJsonEvent } from './events.js' +import { authFailureHint, authFailureMessage } from './failure.js' const { beginDeviceCodeFlow, bindClientDevice } = auth +/** + * Report a CTS failure and exit non-zero, on whichever stream this run uses. + * + * One function for all three unwrap sites so the `help` text and the + * terminal-condition hint (see `./failure.js`) cannot be attached to two of + * them and forgotten on the third — which is how they came to be missing from + * all three. + * + * The JSON stream carries `code` separately, so it gets the message alone; a + * consumer branching on `USAGE_LIMIT_EXCEEDED` has what it needs without prose. + */ +function reportAuthFailure( + failure: { type?: string; error: { message: string }; help?: string }, + fallbackCode: string, + json: boolean, +): never { + if (json) { + emitJsonError(failure.type ?? fallbackCode, authFailureMessage(failure)) + } else { + p.log.error(authFailureMessage(failure)) + const hint = authFailureHint(failure) + if (hint) p.log.info(hint) + } + process.exit(1) +} + export interface LoginOptions { /** * Emit newline-delimited JSON events instead of pretty clack output, so an @@ -45,15 +72,7 @@ export async function login( // surface the failure `type` (machine-readable) + message on the JSON stream. const pending = await beginDeviceCodeFlow(region, 'cli') if (pending.failure) { - if (json) { - emitJsonError( - pending.failure.type ?? 'begin_failed', - pending.failure.error.message, - ) - } else { - p.log.error(pending.failure.error.message) - } - process.exit(1) + reportAuthFailure(pending.failure, 'begin_failed', json) } const flow = pending.data @@ -86,15 +105,7 @@ export async function login( const authResult = await flow.pollForToken() if (authResult.failure) { s?.stop('Authorization failed.') - if (json) { - emitJsonError( - authResult.failure.type ?? 'poll_failed', - authResult.failure.error.message, - ) - } else { - p.log.error(authResult.failure.error.message) - } - process.exit(1) + reportAuthFailure(authResult.failure, 'poll_failed', json) } s?.stop('Authenticated!') @@ -124,16 +135,8 @@ export async function bindDevice(opts: BindDeviceOptions = {}) { // `@cipherstash/auth` `0.41` — a failure no longer throws. const result = await bindClientDevice() if (result.failure) { - if (json) { - emitJsonError( - result.failure.type ?? 'bind_failed', - result.failure.error.message, - ) - } else { - s?.stop('Failed to bind your device to the default Keyset!') - p.log.error(result.failure.error.message) - } - process.exit(1) + if (!json) s?.stop('Failed to bind your device to the default Keyset!') + reportAuthFailure(result.failure, 'bind_failed', json) } if (json) { diff --git a/packages/cli/src/commands/env/index.ts b/packages/cli/src/commands/env/index.ts index 4087f9ae1..cea630e1b 100644 --- a/packages/cli/src/commands/env/index.ts +++ b/packages/cli/src/commands/env/index.ts @@ -7,6 +7,11 @@ import { CliExit } from '../../cli/exit.js' import { isInteractive } from '../../config/tty.js' import { messages } from '../../messages.js' import { emitJsonError, emitJsonEvent } from '../auth/events.js' +import { + authFailureCode, + authFailureHint, + authFailureMessage, +} from '../auth/failure.js' import { detectPackageManager, runnerCommand } from '../init/utils.js' const { DeviceSessionStrategy } = auth @@ -336,16 +341,22 @@ async function mintCredentials(keyName: string): Promise { if (strategyResult.failure) { throw new MintError( 'not_logged_in', - `Not logged in: ${strategyResult.failure.error.message}`, - LOGIN_HINT, + `Not logged in: ${authFailureMessage(strategyResult.failure)}`, + authFailureHint(strategyResult.failure, LOGIN_HINT), ) } const tokenResult = await strategyResult.data.getToken() if (tokenResult.failure) { + // The renewal CTS can refuse on billing grounds rather than credential + // ones. `LOGIN_HINT` is wrong advice for that: a fresh login mints nothing + // an organisation over its usage limit is allowed to have, so + // `authFailureHint` sends the user to the dashboard instead. throw new MintError( - 'session_invalid', - `Could not refresh your session: ${tokenResult.failure.error.message}`, - LOGIN_HINT, + authFailureCode(tokenResult.failure) === 'USAGE_LIMIT_EXCEEDED' + ? 'usage_limit_exceeded' + : 'session_invalid', + `Could not refresh your session: ${authFailureMessage(tokenResult.failure)}`, + authFailureHint(tokenResult.failure, LOGIN_HINT), ) } const { token, workspaceId, issuer, services } = tokenResult.data diff --git a/packages/protect-ffi/crates/protect-ffi/src/lib.rs b/packages/protect-ffi/crates/protect-ffi/src/lib.rs index 3cca59cc5..609eb2ddd 100644 --- a/packages/protect-ffi/crates/protect-ffi/src/lib.rs +++ b/packages/protect-ffi/crates/protect-ffi/src/lib.rs @@ -457,16 +457,41 @@ impl From for Error { } } +/// Everything an [`Error`] carries across either JavaScript boundary. +/// +/// Built in one place so the Neon, wasm, and fallible-bulk representations +/// cannot drift — the bug the previous `(String, Option)` tuple was +/// already guarding against, now with four fields instead of two. +pub(crate) struct Diagnostic { + /// The `Display` text. Becomes `err.message`. + pub(crate) message: String, + /// This crate's own `ProtectErrorCode`, from the variant's + /// `#[diagnostic(code(..))]`. Absent becomes `UNKNOWN` on the JS side. + pub(crate) code: Option, + /// The auth taxonomy code, when the failure came from stack-auth. Becomes + /// `err.authCode`. See [`Error::auth_error`]. + pub(crate) auth_code: Option, + /// The auth error's `miette` help text. Becomes `err.help`. + pub(crate) help: Option, +} + impl Error { - /// The human-readable diagnostic message and stable machine-readable code - /// carried across either JavaScript boundary. + /// The message, code, auth code, and help text carried across either + /// JavaScript boundary. /// /// `miette::Diagnostic` uses this error's [`Display`](std::fmt::Display) /// implementation as its primary message; the code comes from the - /// variant's `#[diagnostic(code(..))]`. Keeping the extraction together - /// makes the Neon, wasm, and fallible-bulk representations agree. - pub(crate) fn diagnostic_parts(&self) -> (String, Option) { - (self.to_string(), self.error_code()) + /// variant's `#[diagnostic(code(..))]`. + pub(crate) fn diagnostic_parts(&self) -> Diagnostic { + let auth = self.auth_error(); + Diagnostic { + message: self.to_string(), + code: self.error_code(), + auth_code: auth.map(|e| e.error_code().to_string()), + help: auth + .and_then(miette::Diagnostic::help) + .map(|help| help.to_string()), + } } /// The `ProtectErrorCode` this error crosses the boundary with, if it has @@ -478,6 +503,32 @@ impl Error { pub(crate) fn error_code(&self) -> Option { miette::Diagnostic::code(self).map(|code| code.to_string()) } + + /// The [`AuthError`] underneath, when this failure is one. + /// + /// Auth failures reach JS through two shapes, and neither carries anything + /// of its own: [`Self::Auth`] and [`Self::ZeroKMS`] are both + /// `#[error(transparent)]` with no `#[diagnostic(code(..))]`, so before + /// this the whole auth taxonomy — every code, and the `help` text that + /// tells a caller what to actually DO — arrived as an untyped `Error` with + /// nothing but prose. `USAGE_LIMIT_EXCEEDED` is the case that made it + /// matter: an org over its billing allowance is not a credentials problem + /// and not a transient one, and a caller cannot tell it apart from either + /// without the code. + /// + /// Two shapes rather than one because every ZeroKMS operation resolves its + /// credential first (`cipherstash_client::zerokms::ZeroKMS::get_token`), so + /// a refusal at issuance time arrives wrapped in [`zerokms::Error::Auth`] + /// having never made a ZeroKMS request. Matching the VARIANT rather than + /// the message is the same rule the code contract on [`Error`] states: an + /// upstream rename fails here at compile time. + pub(crate) fn auth_error(&self) -> Option<&AuthError> { + match self { + Self::Auth(err) => Some(err), + Self::ZeroKMS(zerokms::Error::Auth(err)) => Some(err), + _ => None, + } + } } /// JS-backed [`AuthStrategy`] for the Neon build. @@ -939,18 +990,26 @@ enum DecryptResult { /// matches the declared `code?: ProtectErrorCode`. #[serde(skip_serializing_if = "Option::is_none")] code: Option, + /// Absent unless the failure came from stack-auth, for the same + /// reason. See [`Error::auth_error`]. + #[serde(rename = "authCode", skip_serializing_if = "Option::is_none")] + auth_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + help: Option, }, } impl DecryptResult { - /// Builds the failure arm from an [`Error`], keeping message and code in - /// step — the two are read off the same value here rather than one being - /// re-derived from the other later. + /// Builds the failure arm from an [`Error`], keeping every field in step — + /// they are read off the same value here rather than one being re-derived + /// from the other later. fn from_error(err: &Error) -> Self { - let (message, code) = err.diagnostic_parts(); + let diagnostic = err.diagnostic_parts(); Self::Error { - error: message, - code, + error: diagnostic.message, + code: diagnostic.code, + auth_code: diagnostic.auth_code, + help: diagnostic.help, } } } @@ -2017,14 +2076,20 @@ async fn do_encrypt_query_bulk( /// shape `wasm.rs` already uses — and maps once on the way out. #[cfg(not(target_arch = "wasm32"))] fn into_js_error(err: Error) -> impl for<'cx> TryIntoJs<'cx, Value = JsError> { - let (message, code) = err.diagnostic_parts(); + let diagnostic = err.diagnostic_parts(); extract::with(move |cx: &mut Cx| -> JsResult { - let error = cx.error(message)?; + let error = cx.error(diagnostic.message)?; // Left unset rather than set to null when absent, so `'code' in err` // answers the question a caller is actually asking. - if let Some(code) = code { - let code = cx.string(code); - error.set(cx, "code", code)?; + for (key, value) in [ + ("code", diagnostic.code), + ("authCode", diagnostic.auth_code), + ("help", diagnostic.help), + ] { + if let Some(value) = value { + let value = cx.string(value); + error.set(cx, key, value)?; + } } Ok(error) }) @@ -2144,15 +2209,24 @@ async fn decrypt_bulk_fallible( let value = js_plaintext_into_js(cx, data)?; obj.set(cx, "data", value)?; } - DecryptResult::Error { error, code } => { + DecryptResult::Error { + error, + code, + auth_code, + help, + } => { let message = cx.string(error); obj.set(cx, "error", message)?; // Left unset rather than set to null when absent, so // the item matches the declared // `code?: ProtectErrorCode`. - if let Some(code) = code { - let code = cx.string(code); - obj.set(cx, "code", code)?; + for (key, value) in + [("code", code), ("authCode", auth_code), ("help", help)] + { + if let Some(value) = value { + let value = cx.string(value); + obj.set(cx, key, value)?; + } } } } @@ -2461,6 +2535,76 @@ mod tests { } } + /// The auth taxonomy is stack-auth's, not this crate's, so it travels on + /// `authCode` rather than being folded into `ProtectErrorCode` — see + /// [`Error::auth_error`]. These pin that it travels AT ALL: before, both + /// wrappers were `#[error(transparent)]` with no diagnostic, so every auth + /// failure reached JS as bare prose with no code and no help. + mod auth_diagnostics { + use super::*; + use stack_auth::{MissingWorkspaceCrn, NotAuthenticated}; + + #[test] + fn a_direct_auth_error_carries_its_code() { + let err = Error::Auth(AuthError::NotAuthenticated(NotAuthenticated)); + let diagnostic = err.diagnostic_parts(); + + assert_eq!(diagnostic.auth_code.as_deref(), Some("NOT_AUTHENTICATED")); + assert_eq!(diagnostic.message, "Not authenticated"); + } + + #[test] + fn a_zerokms_wrapped_auth_error_carries_its_code() { + // The shape a usage-limit refusal actually takes: every ZeroKMS + // operation resolves its credential first, so a refusal at + // issuance time never becomes a ZeroKMS request. + let err = Error::ZeroKMS(zerokms::Error::Auth(AuthError::NotAuthenticated( + NotAuthenticated, + ))); + + assert_eq!( + err.diagnostic_parts().auth_code.as_deref(), + Some("NOT_AUTHENTICATED") + ); + } + + #[test] + fn the_help_text_survives_the_boundary() { + // The half that matters for a billing refusal: the code says WHICH + // failure, the help says what to do about it. miette help is not + // part of `Display`, so nothing carried it before. + let err = Error::Auth(AuthError::MissingWorkspaceCrn(MissingWorkspaceCrn)); + + assert!( + err.diagnostic_parts() + .help + .is_some_and(|help| help.contains("CS_WORKSPACE_CRN")), + "expected the auth error's miette help to cross the boundary" + ); + } + + #[test] + fn an_auth_error_claims_no_protect_error_code() { + // `code` stays this crate's closed `ProtectErrorCode` set, which + // `errorCodes.test.ts` pins against the `#[diagnostic(code(..))]` + // attributes. An auth code appearing there would break that + // contract in a way only a released stack-auth could repair. + let err = Error::Auth(AuthError::NotAuthenticated(NotAuthenticated)); + + assert_eq!(err.diagnostic_parts().code, None); + } + + #[test] + fn a_non_auth_error_carries_neither() { + let err = Error::InvalidEqlVersion(4); + let diagnostic = err.diagnostic_parts(); + + assert_eq!(diagnostic.auth_code, None); + assert_eq!(diagnostic.help, None); + assert_eq!(diagnostic.code.as_deref(), Some("INVALID_EQL_VERSION")); + } + } + mod truncate_for_error { use super::*; diff --git a/packages/protect-ffi/crates/protect-ffi/src/wasm.rs b/packages/protect-ffi/crates/protect-ffi/src/wasm.rs index 42ef31397..0c410df25 100644 --- a/packages/protect-ffi/crates/protect-ffi/src/wasm.rs +++ b/packages/protect-ffi/crates/protect-ffi/src/wasm.rs @@ -170,7 +170,9 @@ export type { export type { EncryptedV3, EncryptedV3Query } from "../../lib/eql-v3.js"; export { PROTECT_ERROR_CODES, + getAuthErrorCode, isProtectErrorCode, + type ProtectAuthErrorCode, type ProtectErrorCode, } from "./errors.js"; "#; @@ -683,12 +685,19 @@ pub async fn decrypt_bulk_fallible( DecryptResult::Success { data } => { set_prop(&obj, "data", &plaintext_to_js(data)?)?; } - DecryptResult::Error { error, code } => { + DecryptResult::Error { + error, + code, + auth_code, + help, + } => { set_prop(&obj, "error", &JsValue::from_str(error))?; // Left unset rather than set to null when absent, so the item // matches the declared `code?: ProtectErrorCode`. - if let Some(code) = code { - set_prop(&obj, "code", &JsValue::from_str(code))?; + for (key, value) in [("code", code), ("authCode", auth_code), ("help", help)] { + if let Some(value) = value { + set_prop(&obj, key, &JsValue::from_str(value))?; + } } } } @@ -1077,20 +1086,27 @@ fn js_error(msg: &str) -> JsValue { js_sys::Error::new(msg).into() } -/// A JS `Error` carrying this error's `ProtectErrorCode` as `err.code`. +/// A JS `Error` carrying this error's `ProtectErrorCode` as `err.code`, plus +/// `err.authCode` / `err.help` when the failure came from stack-auth. /// /// The code comes off the Rust variant (see [`Error`]'s `code` contract) rather /// than being recovered from the message on the JS side, which is what /// `src/errors.ts` used to do — and could only do for the Neon entry, since /// this build's thrown errors never reached that wrapper (#146). fn error_to_js(e: Error) -> JsValue { - let (message, code) = e.diagnostic_parts(); - let err = js_sys::Error::new(&message); - if let Some(code) = code { - // Infallible in practice: `err` is a fresh, extensible JS object. A - // failure here still yields a correct error, just without the code, - // which beats masking the original failure with a `Reflect` one. - let _ = js_sys::Reflect::set(&err, &JsValue::from_str("code"), &JsValue::from_str(&code)); + let diagnostic = e.diagnostic_parts(); + let err = js_sys::Error::new(&diagnostic.message); + for (key, value) in [ + ("code", diagnostic.code), + ("authCode", diagnostic.auth_code), + ("help", diagnostic.help), + ] { + if let Some(value) = value { + // Infallible in practice: `err` is a fresh, extensible JS object. A + // failure here still yields a correct error, just without the code, + // which beats masking the original failure with a `Reflect` one. + let _ = js_sys::Reflect::set(&err, &JsValue::from_str(key), &JsValue::from_str(&value)); + } } err.into() } diff --git a/packages/protect-ffi/dist/wasm/errors.d.ts b/packages/protect-ffi/dist/wasm/errors.d.ts index d13a29709..13b843af3 100644 --- a/packages/protect-ffi/dist/wasm/errors.d.ts +++ b/packages/protect-ffi/dist/wasm/errors.d.ts @@ -15,6 +15,44 @@ */ export declare const PROTECT_ERROR_CODES: readonly ["INVARIANT_VIOLATION", "UNKNOWN_QUERY_OP", "UNKNOWN_COLUMN", "MISSING_INDEX", "INVALID_QUERY_INPUT", "SHORT_MATCH_NEEDLE", "INVALID_JSON_PATH", "STE_VEC_REQUIRES_JSON_CAST_AS", "MATCH_REQUIRES_TEXT", "UNSUPPORTED_CONFIG_VERSION", "INVALID_EQL_VERSION", "EQL_V3_UNSUPPORTED_COLUMN", "EQL_V3_CONVERSION_FAILED", "INVALID_CIPHERTEXT", "UNKNOWN"]; export type ProtectErrorCode = (typeof PROTECT_ERROR_CODES)[number]; +/** + * The auth taxonomy code on a failure that came from `stack-auth` — CTS + * refused to issue or renew the service token every ZeroKMS request carries. + * + * Deliberately a separate field from {@link ProtectErrorCode} rather than more + * members of it. That set is closed and owned HERE: `errorCodes.test.ts` pins + * it against the `#[diagnostic(code(..))]` attributes in + * `crates/protect-ffi/src/lib.rs`, and every member has one. The auth set is + * owned by `stack-auth` and versioned on its own release train, so folding the + * two together would either break that test or force this package to re-declare + * a taxonomy it does not decide. + * + * So the type is open on purpose — `(string & {})` keeps editor completion for + * the named members without rejecting a code from a newer `stack-auth` than the + * one this build pinned. Narrow with a `===` against a literal; do not + * `switch` exhaustively. + * + * Only the two that carry a caller-actionable remedy are named. The rest of the + * set (`NOT_AUTHENTICATED`, `WORKSPACE_MISMATCH`, `EXPIRED_TOKEN`, …) still + * arrives, and is documented in `@cipherstash/auth`'s `AuthFailure` union. + * + * - `USAGE_LIMIT_EXCEEDED` — the organisation has used its allowance for the + * current billing period. **Not** retryable and **not** a credentials + * problem: nothing clears it until the plan is upgraded. + * - `ORG_NOT_PROVISIONED` — the organisation is not registered with the usage + * system at all. There is no plan to upgrade; it needs support. + * + * Both arrive alongside `help`, the remedy text `stack-auth` wrote for them. + */ +export type ProtectAuthErrorCode = 'USAGE_LIMIT_EXCEEDED' | 'ORG_NOT_PROVISIONED' | (string & {}); +/** + * Read the auth taxonomy code off a thrown FFI error, if it has one. + * + * Present only when the failure came from `stack-auth`; every other failure + * leaves the field unset, so `undefined` means "not an auth failure" rather + * than "an auth failure with no code". + */ +export declare function getAuthErrorCode(error: unknown): ProtectAuthErrorCode | undefined; /** * True when `value` is one of this library's error codes. * diff --git a/packages/protect-ffi/dist/wasm/protect_ffi.d.ts b/packages/protect-ffi/dist/wasm/protect_ffi.d.ts index 5e1053212..de69dbd65 100644 --- a/packages/protect-ffi/dist/wasm/protect_ffi.d.ts +++ b/packages/protect-ffi/dist/wasm/protect_ffi.d.ts @@ -85,7 +85,9 @@ export type { export type { EncryptedV3, EncryptedV3Query } from "../../lib/eql-v3.js"; export { PROTECT_ERROR_CODES, + getAuthErrorCode, isProtectErrorCode, + type ProtectAuthErrorCode, type ProtectErrorCode, } from "./errors.js"; diff --git a/packages/protect-ffi/scripts/inline-wasm.mjs b/packages/protect-ffi/scripts/inline-wasm.mjs index 1f6d4c585..231a8722d 100644 --- a/packages/protect-ffi/scripts/inline-wasm.mjs +++ b/packages/protect-ffi/scripts/inline-wasm.mjs @@ -27,8 +27,13 @@ if (!exportMatch) { ) } const exportList = exportMatch[1].trim() +// Must stay in step with the errors.js re-export block in the +// `typescript_custom_section` in `crates/protect-ffi/src/wasm.rs` — that block +// is what declares these to a consumer, and a name declared there but missing +// here is a type that promises a runtime export the bundle does not have. +// `errorCodes.test.ts` compares the two. const errorHelperExport = - 'export { PROTECT_ERROR_CODES, isProtectErrorCode } from "./errors.js";' + 'export { PROTECT_ERROR_CODES, getAuthErrorCode, isProtectErrorCode } from "./errors.js";' // wasm-bindgen owns the main stub and cannot re-export arbitrary JavaScript // values from a TypeScript custom section. Add the runtime half of the error diff --git a/packages/protect-ffi/src/errorCodes.test.ts b/packages/protect-ffi/src/errorCodes.test.ts index d99a68498..92fea83f5 100644 --- a/packages/protect-ffi/src/errorCodes.test.ts +++ b/packages/protect-ffi/src/errorCodes.test.ts @@ -100,3 +100,49 @@ describe('error codes', () => { } }) }) + +/** + * The wasm bundle declares its error-helper exports in one place and produces + * them in another, and neither knows about the other. + * + * `crates/protect-ffi/src/wasm.rs` carries a `typescript_custom_section` whose + * errors.js re-export block is what a consumer's TypeScript sees. + * The runtime half is appended to wasm-pack's output by + * `scripts/inline-wasm.mjs`, because wasm-bindgen cannot re-export arbitrary + * JavaScript values from a custom section. + * + * A name in the first and not the second is a declared export that does not + * exist at runtime — a `TypeError` in an edge function, with a green build. + */ +describe('wasm error-helper exports', () => { + // Anchored per file rather than one loose pattern over both, for the reason + // CODE_ATTRIBUTE above is anchored: a doc comment that quotes the statement + // it is describing would otherwise be scraped as the statement, and the test + // would compare prose to code. In `wasm.rs` the block starts a line; in + // `inline-wasm.mjs` it is a single-quoted JavaScript string. + const declared = /^export \{([^}]*)\} from "\.\/errors\.js";$/m.exec( + read('crates/protect-ffi/src/wasm.rs'), + ) + const emitted = /'export \{([^}]*)\} from "\.\/errors\.js";'/.exec( + read('scripts/inline-wasm.mjs'), + ) + + /** Value exports only — a `type` specifier has no runtime counterpart. */ + const names = (block: RegExpExecArray | null) => + (block?.[1] ?? '') + .split(',') + .map((name) => name.trim()) + .filter((name) => name.length > 0 && !name.startsWith('type ')) + .sort() + + it('finds both re-export blocks', () => { + // Without this the comparison below passes vacuously — two empty sets are + // equal, and a regex that stopped matching would read as agreement. + expect(names(declared).length).toBeGreaterThan(0) + expect(names(emitted).length).toBeGreaterThan(0) + }) + + it('declares exactly the names the inline bundle re-exports', () => { + expect(names(declared)).toEqual(names(emitted)) + }) +}) diff --git a/packages/protect-ffi/src/errors.test.ts b/packages/protect-ffi/src/errors.test.ts index 3e90233ac..4f6698238 100644 --- a/packages/protect-ffi/src/errors.test.ts +++ b/packages/protect-ffi/src/errors.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { isProtectErrorCode, PROTECT_ERROR_CODES } from './errors.js' +import { + getAuthErrorCode, + isProtectErrorCode, + PROTECT_ERROR_CODES, +} from './errors.js' describe('isProtectErrorCode', () => { it('accepts every declared code', () => { @@ -85,3 +89,55 @@ describe('no message-shape routing', () => { expect(isProtectErrorCode((err as { code?: unknown }).code)).toBe(false) }) }) + +describe('getAuthErrorCode', () => { + /** + * What both bindings throw for a stack-auth failure: the ordinary Error, + * plus `authCode` and the remedy text stack-auth wrote — see + * `Error::auth_error` in `crates/protect-ffi/src/lib.rs`. + */ + const authThrown = (message: string, authCode: string) => + Object.assign(new Error(message), { authCode }) + + it('reads the code off an auth failure', () => { + const err: unknown = authThrown( + 'Insufficient balance. Please upgrade your plan.', + 'USAGE_LIMIT_EXCEEDED', + ) + + expect(getAuthErrorCode(err)).toBe('USAGE_LIMIT_EXCEEDED') + }) + + it('is undefined for a failure that did not come from auth', () => { + // The distinction the field exists to make: absent means "not an auth + // failure", which is why it is unset rather than null. + const err: unknown = Object.assign(new Error('column not found'), { + code: 'UNKNOWN_COLUMN', + }) + + expect(getAuthErrorCode(err)).toBeUndefined() + }) + + it('does not confuse `code` for `authCode`', () => { + // The two taxonomies are separate on purpose — see `ProtectAuthErrorCode`. + const err: unknown = Object.assign(new Error('boom'), { code: 'UNKNOWN' }) + + expect(getAuthErrorCode(err)).toBeUndefined() + }) + + it('survives non-objects and non-string codes', () => { + expect(getAuthErrorCode(undefined)).toBeUndefined() + expect(getAuthErrorCode(null)).toBeUndefined() + expect(getAuthErrorCode('USAGE_LIMIT_EXCEEDED')).toBeUndefined() + expect(getAuthErrorCode({ authCode: 42 })).toBeUndefined() + }) + + it('accepts a code this build has never heard of', () => { + // The set is stack-auth's and moves on its own release train, so a newer + // code than the pinned crate must still reach the caller rather than + // being filtered to undefined. + expect(getAuthErrorCode({ authCode: 'SOME_FUTURE_CODE' })).toBe( + 'SOME_FUTURE_CODE', + ) + }) +}) diff --git a/packages/protect-ffi/src/errors.ts b/packages/protect-ffi/src/errors.ts index d5af3a721..e30358c40 100644 --- a/packages/protect-ffi/src/errors.ts +++ b/packages/protect-ffi/src/errors.ts @@ -35,6 +35,55 @@ export type ProtectErrorCode = (typeof PROTECT_ERROR_CODES)[number] const KNOWN_CODES: ReadonlySet = new Set(PROTECT_ERROR_CODES) +/** + * The auth taxonomy code on a failure that came from `stack-auth` — CTS + * refused to issue or renew the service token every ZeroKMS request carries. + * + * Deliberately a separate field from {@link ProtectErrorCode} rather than more + * members of it. That set is closed and owned HERE: `errorCodes.test.ts` pins + * it against the `#[diagnostic(code(..))]` attributes in + * `crates/protect-ffi/src/lib.rs`, and every member has one. The auth set is + * owned by `stack-auth` and versioned on its own release train, so folding the + * two together would either break that test or force this package to re-declare + * a taxonomy it does not decide. + * + * So the type is open on purpose — `(string & {})` keeps editor completion for + * the named members without rejecting a code from a newer `stack-auth` than the + * one this build pinned. Narrow with a `===` against a literal; do not + * `switch` exhaustively. + * + * Only the two that carry a caller-actionable remedy are named. The rest of the + * set (`NOT_AUTHENTICATED`, `WORKSPACE_MISMATCH`, `EXPIRED_TOKEN`, …) still + * arrives, and is documented in `@cipherstash/auth`'s `AuthFailure` union. + * + * - `USAGE_LIMIT_EXCEEDED` — the organisation has used its allowance for the + * current billing period. **Not** retryable and **not** a credentials + * problem: nothing clears it until the plan is upgraded. + * - `ORG_NOT_PROVISIONED` — the organisation is not registered with the usage + * system at all. There is no plan to upgrade; it needs support. + * + * Both arrive alongside `help`, the remedy text `stack-auth` wrote for them. + */ +export type ProtectAuthErrorCode = + | 'USAGE_LIMIT_EXCEEDED' + | 'ORG_NOT_PROVISIONED' + | (string & {}) + +/** + * Read the auth taxonomy code off a thrown FFI error, if it has one. + * + * Present only when the failure came from `stack-auth`; every other failure + * leaves the field unset, so `undefined` means "not an auth failure" rather + * than "an auth failure with no code". + */ +export function getAuthErrorCode( + error: unknown, +): ProtectAuthErrorCode | undefined { + if (typeof error !== 'object' || error === null) return undefined + const { authCode } = error as { authCode?: unknown } + return typeof authCode === 'string' ? authCode : undefined +} + /** * True when `value` is one of this library's error codes. * diff --git a/packages/protect-ffi/src/index.cts b/packages/protect-ffi/src/index.cts index 0d9b01dd0..38616c972 100644 --- a/packages/protect-ffi/src/index.cts +++ b/packages/protect-ffi/src/index.cts @@ -28,8 +28,10 @@ export * from './eql-v3.js' import type { EncryptedV3Query } from './eql-v3.js' export { + getAuthErrorCode, isProtectErrorCode, PROTECT_ERROR_CODES, + type ProtectAuthErrorCode, type ProtectErrorCode, } from './errors.js' diff --git a/packages/protect-ffi/src/types.ts b/packages/protect-ffi/src/types.ts index 341936f5f..f0fa77ac1 100644 --- a/packages/protect-ffi/src/types.ts +++ b/packages/protect-ffi/src/types.ts @@ -26,11 +26,18 @@ import type { CredentialOpts } from './credentials.js' import type { EncryptedV3 } from './eql-v3.js' -import type { ProtectErrorCode } from './errors.js' +import type { ProtectAuthErrorCode, ProtectErrorCode } from './errors.js' export type DecryptResult = | { data: JsPlaintext } - | { error: string; code?: ProtectErrorCode } + | { + error: string + code?: ProtectErrorCode + /** @see {@link ProtectAuthErrorCode} */ + authCode?: ProtectAuthErrorCode + /** @see {@link ProtectAuthErrorCode} */ + help?: string + } export type EncryptPayload = { plaintext: JsPlaintext diff --git a/packages/stack/__tests__/auth-failure-propagation.test.ts b/packages/stack/__tests__/auth-failure-propagation.test.ts new file mode 100644 index 000000000..74f12e6ce --- /dev/null +++ b/packages/stack/__tests__/auth-failure-propagation.test.ts @@ -0,0 +1,106 @@ +/** + * End-to-end proof that a CTS usage-limit refusal reaches a caller as + * something they can act on, on the two paths they actually meet it. + * + * The failure originates at token issuance: every ZeroKMS operation resolves a + * service token first, so CTS answering `402 USAGE_LIMIT_EXCEEDED` means no + * ZeroKMS request is made at all. protect-ffi surfaces that as a thrown `Error` + * with `authCode` and stack-auth's `help` (see `Error::auth_error` in + * `packages/protect-ffi/crates/protect-ffi/src/lib.rs`); this asserts the SDK + * folds the dashboard remedy into `message` and keeps the code for branching. + * + * Credential-free: protect-ffi is mocked, so there is no CTS round-trip. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +/** What CTS's 402 body says, verbatim — the whole message a caller had before. */ +const CTS_MESSAGE = 'Insufficient balance. Please upgrade your plan.' + +/** The shape protect-ffi throws for a stack-auth failure. */ +const usageLimitRefusal = () => + Object.assign(new Error(CTS_MESSAGE), { + authCode: 'USAGE_LIMIT_EXCEEDED', + help: 'The organisation has used its allowance for the current billing period. Upgrade the plan from the CipherStash dashboard, then retry.', + }) + +vi.mock('@cipherstash/protect-ffi', async (importOriginal) => ({ + // `isProtectErrorCode` is real: `getErrorCode` runs it over the thrown + // error's `code`, and stubbing it would let a wrong answer here pass. + ...(await importOriginal()), + newClient: vi.fn(async () => ({ __mock: 'client' })), + encrypt: vi.fn(async () => { + throw usageLimitRefusal() + }), +})) + +import * as ffi from '@cipherstash/protect-ffi' +import { encryptedTable, types } from '@/encryption/v3' +import { Encryption } from '@/index' + +const users = encryptedTable('users', { + email: types.TextEq('email'), +}) + +beforeEach(() => { + vi.clearAllMocks() +}) + +/** `Encryption()` throws rather than returning a `Result`, so catch to inspect. */ +async function initFailure() { + vi.mocked(ffi.newClient).mockRejectedValueOnce(usageLimitRefusal()) + try { + await Encryption({ schemas: [users] }) + } catch (thrown) { + return thrown as Error & { authCode?: string } + } + throw new Error('expected Encryption() to reject') +} + +describe('a usage-limit refusal at client init', () => { + it('names the dashboard in the thrown message', async () => { + const error = await initFailure() + + // The original text survives — this adds to it, it does not replace it. + expect(error.message).toContain(CTS_MESSAGE) + expect(error.message).toContain('https://dashboard.cipherstash.com') + }) + + it('says the retry a client would otherwise attempt is futile', async () => { + // The reason a code was needed at all: before this the refusal arrived as + // SERVER_ERROR-shaped prose, and a well-behaved retry loop would hammer a + // condition only a human with a billing page can clear. + const error = await initFailure() + + expect(error.message).toContain('cannot succeed') + }) + + it('keeps the code branchable on the thrown error', async () => { + const error = await initFailure() + + expect(error.authCode).toBe('USAGE_LIMIT_EXCEEDED') + }) +}) + +describe('a usage-limit refusal on an operation', () => { + it('is an EncryptionError carrying the same remedy and code', async () => { + const client = await Encryption({ schemas: [users] }) + + const result = await client.encrypt('person@example.com', { + column: users.email, + table: users, + }) + + expect(result.failure?.type).toBe('EncryptionError') + expect(result.failure?.message).toContain(CTS_MESSAGE) + expect(result.failure?.message).toContain( + 'https://dashboard.cipherstash.com', + ) + expect(result.failure?.authCode).toBe('USAGE_LIMIT_EXCEEDED') + }) + + it('leaves `code` to protect-ffi, which claims none for an auth failure', () => { + // The two taxonomies stay separate: `code` is protect-ffi's closed + // `ProtectErrorCode` set, `authCode` is stack-auth's open one. + expect(usageLimitRefusal()).not.toHaveProperty('code') + }) +}) diff --git a/packages/stack/__tests__/auth-failure-remedy.test.ts b/packages/stack/__tests__/auth-failure-remedy.test.ts new file mode 100644 index 000000000..34fdca637 --- /dev/null +++ b/packages/stack/__tests__/auth-failure-remedy.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from 'vitest' +import { + authFailureCode, + messageWithAuthRemedy, +} from '@/encryption/helpers/auth-failure' + +/** + * The shape protect-ffi throws for a stack-auth failure: the ordinary `Error`, + * plus `authCode` and the `miette` help text stack-auth wrote for that code. + * See `Error::auth_error` in `packages/protect-ffi/crates/protect-ffi/src/lib.rs`. + */ +const authError = (message: string, authCode: string, help?: string) => + Object.assign(new Error(message), help ? { authCode, help } : { authCode }) + +describe('authFailureCode', () => { + it('reads the code off a CTS refusal', () => { + expect( + authFailureCode( + authError('Insufficient balance.', 'USAGE_LIMIT_EXCEEDED'), + ), + ).toBe('USAGE_LIMIT_EXCEEDED') + }) + + it('is undefined for a failure that did not come from auth', () => { + const err = Object.assign(new Error('column not found'), { + code: 'UNKNOWN_COLUMN', + }) + + expect(authFailureCode(err)).toBeUndefined() + }) + + it('accepts a code newer than this build knows about', () => { + // The taxonomy belongs to `@cipherstash/auth` and ships on its own release + // train. Validating against a pinned set here would silently drop the next + // code it adds — which is exactly how `USAGE_LIMIT_EXCEEDED` would have + // been lost. + expect(authFailureCode({ authCode: 'SOME_FUTURE_CODE' })).toBe( + 'SOME_FUTURE_CODE', + ) + }) + + it('survives non-objects and non-string codes', () => { + expect(authFailureCode(undefined)).toBeUndefined() + expect(authFailureCode(null)).toBeUndefined() + expect(authFailureCode('USAGE_LIMIT_EXCEEDED')).toBeUndefined() + expect(authFailureCode({ authCode: 42 })).toBeUndefined() + }) +}) + +describe('messageWithAuthRemedy', () => { + it('sends a usage-limit refusal to the dashboard', () => { + // The whole point of the change. CTS answers a 402 with "Insufficient + // balance. Please upgrade your plan." — true, but it names no dashboard, + // and nothing in it says retrying is futile. + const message = messageWithAuthRemedy( + authError( + 'Insufficient balance. Please upgrade your plan.', + 'USAGE_LIMIT_EXCEEDED', + ), + ) + + expect(message).toContain('Insufficient balance. Please upgrade your plan.') + expect(message).toContain('https://dashboard.cipherstash.com') + expect(message).toContain('retrying without upgrading cannot succeed') + }) + + it('does not send an unprovisioned org to the billing page', () => { + // The two 402 causes need different remedies: an org over its allowance + // upgrades, an org the usage system has never heard of has nothing to buy. + // Telling the second to upgrade sends it somewhere that cannot help. + const message = messageWithAuthRemedy( + authError('Organisation is not provisioned.', 'ORG_NOT_PROVISIONED'), + ) + + expect(message).toContain('support@cipherstash.com') + expect(message).not.toContain('dashboard.cipherstash.com') + }) + + it("prefers this package's remedy over stack-auth's help", () => { + // Both say "upgrade your plan"; only one names where. Appending would give + // the reader the same instruction twice in different words. + const message = messageWithAuthRemedy( + authError( + 'Insufficient balance.', + 'USAGE_LIMIT_EXCEEDED', + 'Upgrade the plan from the CipherStash dashboard, then retry.', + ), + ) + + expect(message).not.toContain('then retry.') + expect(message).toContain('https://dashboard.cipherstash.com') + }) + + it("falls back to stack-auth's help for the rest of the taxonomy", () => { + // These carried remedy text all along; `miette` help is not part of an + // error's `Display`, so it was dropped at the FFI boundary and never + // reached anyone. + const message = messageWithAuthRemedy( + authError( + 'Not authenticated', + 'NOT_AUTHENTICATED', + 'Log in with `stash login`, or set `CS_CLIENT_ACCESS_KEY`.', + ), + ) + + expect(message).toBe( + 'Not authenticated. Log in with `stash login`, or set `CS_CLIENT_ACCESS_KEY`.', + ) + }) + + it('leaves a non-auth failure byte-for-byte', () => { + const original = 'column users.email not found in Encrypt config' + + expect( + messageWithAuthRemedy( + Object.assign(new Error(original), { code: 'UNKNOWN_COLUMN' }), + ), + ).toBe(original) + }) + + it('does not double a terminal full stop', () => { + expect( + messageWithAuthRemedy( + authError('Boom.', 'NOT_AUTHENTICATED', 'Try again.'), + ), + ).toBe('Boom. Try again.') + }) + + it('handles a thrown non-Error', () => { + expect(messageWithAuthRemedy('plain string')).toBe('plain string') + }) +}) diff --git a/packages/stack/src/encryption/helpers/auth-failure.ts b/packages/stack/src/encryption/helpers/auth-failure.ts new file mode 100644 index 000000000..c68cfc6ed --- /dev/null +++ b/packages/stack/src/encryption/helpers/auth-failure.ts @@ -0,0 +1,102 @@ +import type { ProtectAuthErrorCode } from '@cipherstash/protect-ffi' + +/** + * The remedies this package owns, keyed by the `stack-auth` code that CTS's + * refusal arrives with. + * + * These exist because the two conditions below are the ones a caller can + * neither retry away nor fix with credentials, and the message alone does not + * say so. CTS answers a usage-limit refusal with a 402 whose body reads + * "Insufficient balance. Please upgrade your plan." — accurate, but it names no + * dashboard, and by the time it has crossed CTS -> stack-auth -> ZeroKMS -> + * protect-ffi it is one sentence with no context on where "upgrade" happens. A + * well-behaved client that treats every token failure as transient will retry + * against a condition only a human with a billing page can clear. + * + * `stack-auth` attaches its own `help` for these (see `UsageLimitExceeded` in + * that crate), which {@link authRemedy} falls back to. What it cannot carry is + * a URL, which is exactly the part a developer reading a stack trace needs, so + * the text here supersedes it rather than appending to it — two sentences + * saying "upgrade your plan" in slightly different words is worse than one. + * + * A `Map`, not a `Record`: {@link ProtectAuthErrorCode} is an open union (the + * set belongs to `stack-auth` and moves on its own release train), so a + * `Record` over it degenerates to "every string is required". + */ +const AUTH_REMEDIES: ReadonlyMap = new Map([ + [ + 'USAGE_LIMIT_EXCEEDED', + 'Your CipherStash organisation has used its allowance for the current billing period. Upgrade the plan at https://dashboard.cipherstash.com and retry — this is a billing condition, so retrying without upgrading cannot succeed, and rotating credentials will not help.', + ], + [ + 'ORG_NOT_PROVISIONED', + 'Your CipherStash organisation is not registered with the usage system, so there is no plan to upgrade. Contact support@cipherstash.com — retrying cannot succeed.', + ], +]) + +/** + * The remedy to append to an auth failure's message, if there is one. + * + * Prefers this package's own text (which names the dashboard) over the `help` + * that `stack-auth` set, and falls back to `help` for every other auth code — + * `NOT_AUTHENTICATED`, `MISSING_WORKSPACE_CRN`, `INVALID_ACCESS_KEY` and the + * rest all carry useful remedy text that was previously dropped at the FFI + * boundary, because `miette` help is not part of an error's `Display`. + */ +function authRemedy(error: unknown, authCode: string | undefined): string { + if (authCode) { + const remedy = AUTH_REMEDIES.get(authCode) + if (remedy) return remedy + } + const { help } = (error ?? {}) as { help?: unknown } + return typeof help === 'string' ? help : '' +} + +/** + * The message a caller sees for a failure, with the auth remedy folded in. + * + * Folded into `message` rather than left as a sibling field on purpose. The + * documented way to surface one of these is `throw new Error(failure.message)` + * — it is what this package's own JSDoc examples do — so guidance parked + * anywhere else is guidance nobody reads at the moment it is needed. + * + * Non-auth failures keep their message byte-for-byte. + */ +export function messageWithAuthRemedy(error: unknown): string { + const message = error instanceof Error ? error.message : String(error) + const remedy = authRemedy(error, authFailureCode(error)) + if (!remedy) return message + return message.endsWith('.') || message.endsWith('!') + ? `${message} ${remedy}` + : `${message}. ${remedy}` +} + +/** + * The `stack-auth` code on a failure, when CTS refused to issue or renew the + * service token every ZeroKMS request carries. + * + * `undefined` for every failure that did not come from auth — protect-ffi + * leaves the field unset rather than null for exactly that reason. + * + * Read STRUCTURALLY, and this module imports from `@cipherstash/protect-ffi` + * type-only, for the same reason `wasm-inline.ts`'s `readErrorCode` does: + * protect-ffi is not in tsup's `noExternal`, so a runtime import here would put + * a bare `@cipherstash/protect-ffi` specifier into `dist/wasm-inline.js` — the + * native NAPI entry, in the one bundle that exists to avoid it. Both entries + * share this helper, so it has to hold to the stricter of the two rules. + * + * Unlike `code`, the value is NOT validated against a known set: the set + * belongs to `@cipherstash/auth` and ships on its own release train, so a code + * newer than the pinned build must still reach the caller. There is no + * `ECONNRESET`-style collision risk to guard against either — `authCode` is not + * a field anything else sets. + * + * @see {@link ProtectAuthErrorCode} for the codes worth branching on. + */ +export function authFailureCode( + error: unknown, +): ProtectAuthErrorCode | undefined { + if (typeof error !== 'object' || error === null) return undefined + const { authCode } = error as { authCode?: unknown } + return typeof authCode === 'string' ? authCode : undefined +} diff --git a/packages/stack/src/encryption/index.ts b/packages/stack/src/encryption/index.ts index 4073fcf13..66d9e6e80 100644 --- a/packages/stack/src/encryption/index.ts +++ b/packages/stack/src/encryption/index.ts @@ -1,6 +1,10 @@ import { type Result, withResult } from '@byteslice/result' import { newClient } from '@cipherstash/protect-ffi' import { validate as uuidValidate } from 'uuid' +import { + authFailureCode, + messageWithAuthRemedy, +} from '@/encryption/helpers/auth-failure' import type { AnyV3Table } from '@/eql/v3' import { buildEncryptConfig } from '@/eql/v3' import { type EncryptionError, EncryptionErrorTypes } from '@/errors' @@ -149,9 +153,13 @@ class NativeEncryptionClient { logger.debug('Successfully initialized the Encryption client.') return this }, + // The first place a caller meets a CTS refusal: `newClient` resolves a + // service token, so an organisation over its usage limit fails here + // rather than on the first encrypt. (error: unknown) => ({ type: EncryptionErrorTypes.ClientInitError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), + authCode: authFailureCode(error), }), ) } @@ -933,7 +941,17 @@ export async function Encryption(config: { }) if (result.failure) { - throw new Error(`[encryption]: ${result.failure.message}`) + // `Encryption()` throws rather than returning a `Result`, so `authCode` has + // to ride on the thrown error or it is lost on the one path a caller is + // most likely to meet a CTS refusal on — a usage-limit failure surfaces at + // init, before any operation exists to return a `{ failure }` from. The + // remedy is already inside `.message` (see `messageWithAuthRemedy`); this + // is what makes it branchable as well as readable. + const error = new Error(`[encryption]: ${result.failure.message}`) + if (result.failure.authCode) { + Object.assign(error, { authCode: result.failure.authCode }) + } + throw error } return createEncryptionClient(result.data, ...schemas) diff --git a/packages/stack/src/encryption/operations/batch-encrypt-query.ts b/packages/stack/src/encryption/operations/batch-encrypt-query.ts index cd0e764e0..a217a6cea 100644 --- a/packages/stack/src/encryption/operations/batch-encrypt-query.ts +++ b/packages/stack/src/encryption/operations/batch-encrypt-query.ts @@ -5,6 +5,10 @@ import { type QueryPayload, } from '@cipherstash/protect-ffi' import { formatEncryptedResult } from '@/encryption/helpers' +import { + authFailureCode, + messageWithAuthRemedy, +} from '@/encryption/helpers/auth-failure' import { getErrorCode } from '@/encryption/helpers/error-code' import { type EncryptionError, EncryptionErrorTypes } from '@/errors' import { @@ -159,8 +163,9 @@ export class BatchEncryptQueryOperation extends EncryptionOperation< log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.EncryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) @@ -227,8 +232,9 @@ export class BatchEncryptQueryOperationWithLockContext extends EncryptionOperati log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.EncryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) diff --git a/packages/stack/src/encryption/operations/bulk-decrypt-models.ts b/packages/stack/src/encryption/operations/bulk-decrypt-models.ts index 4b38615ac..ac6177341 100644 --- a/packages/stack/src/encryption/operations/bulk-decrypt-models.ts +++ b/packages/stack/src/encryption/operations/bulk-decrypt-models.ts @@ -1,4 +1,8 @@ import { type Result, withResult } from '@byteslice/result' +import { + authFailureCode, + messageWithAuthRemedy, +} from '@/encryption/helpers/auth-failure' import { getErrorCode } from '@/encryption/helpers/error-code' import { type EncryptionError, EncryptionErrorTypes } from '@/errors' import { type LockContextInput, resolveLockContext } from '@/identity' @@ -51,8 +55,9 @@ export class BulkDecryptModelsOperation< log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.DecryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) @@ -121,8 +126,9 @@ export class BulkDecryptModelsOperationWithLockContext< log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.DecryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) diff --git a/packages/stack/src/encryption/operations/bulk-decrypt.ts b/packages/stack/src/encryption/operations/bulk-decrypt.ts index def9de4da..0d26573b7 100644 --- a/packages/stack/src/encryption/operations/bulk-decrypt.ts +++ b/packages/stack/src/encryption/operations/bulk-decrypt.ts @@ -4,6 +4,10 @@ import { type DecryptResult, decryptBulkFallible, } from '@cipherstash/protect-ffi' +import { + authFailureCode, + messageWithAuthRemedy, +} from '@/encryption/helpers/auth-failure' import { getErrorCode } from '@/encryption/helpers/error-code' import { type EncryptionError, EncryptionErrorTypes } from '@/errors' import { @@ -111,8 +115,9 @@ export class BulkDecryptOperation extends EncryptionOperation log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.DecryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) @@ -184,8 +189,9 @@ export class BulkDecryptOperationWithLockContext extends EncryptionOperation log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.EncryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) @@ -227,8 +232,9 @@ export class BulkEncryptOperationWithLockContext extends EncryptionOperation { log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.DecryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) @@ -133,8 +138,9 @@ export class DecryptOperationWithLockContext extends EncryptionOperation { log.set({ errorCode: getErrorCode(error) ?? 'unknown' }) return { type: EncryptionErrorTypes.EncryptionError, - message: (error as Error).message, + message: messageWithAuthRemedy(error), code: getErrorCode(error), + authCode: authFailureCode(error), } }, ) @@ -166,8 +171,9 @@ export class EncryptOperationWithLockContext extends EncryptionOperation EncryptionError { return (error: unknown) => ({ type, - message: error instanceof Error ? error.message : String(error), + // Both entries fold the auth remedy into `message` — a WASM caller on + // Workers or Deno hits a usage-limit refusal exactly like a Node one, and + // the whole point of the text is that it reaches whoever reads the log. + message: messageWithAuthRemedy(error), code: readErrorCode(error), + authCode: authFailureCode(error), }) } diff --git a/skills/stash-auth/SKILL.md b/skills/stash-auth/SKILL.md index 84ead6083..b8cae140e 100644 --- a/skills/stash-auth/SKILL.md +++ b/skills/stash-auth/SKILL.md @@ -91,6 +91,47 @@ X is not a member of workspace Y", "No OIDC provider found for issuer: …", organisation is over its usage limit. A 402 is a billing problem, not a credentials problem — don't rotate keys over it. +### The billing refusal, and why it needs its own handling + +A token failure is normally transient (network, an expired token about to be +renewed) or a credentials problem (wrong key, wrong workspace) — both worth +retrying or re-authenticating. A 402 is neither: **nothing the process can do +clears it.** A retry loop that treats every token failure alike will hammer a +condition only a human with a billing page can resolve. + +So it is separately identifiable. A failure that came from CTS carries +`authCode` alongside the usual `type` and `code`, and its `message` already has +the remedy folded in — including the dashboard URL, which is the part the +underlying CTS response does not carry: + +```typescript +const result = await client.encrypt(value, { column, table }) +if (result.failure?.authCode === 'USAGE_LIMIT_EXCEEDED') { + // Stop retrying. `result.failure.message` names dashboard.cipherstash.com. +} +``` + +Two codes mean "stop", and they need different remedies: + +| `authCode` | What it means | What clears it | +|---|---|---| +| `USAGE_LIMIT_EXCEEDED` | The organisation has used its allowance for the current billing period | Upgrade the plan at [dashboard.cipherstash.com](https://dashboard.cipherstash.com) | +| `ORG_NOT_PROVISIONED` | The organisation isn't registered with the usage system at all | Nothing you can buy — contact support@cipherstash.com | + +`authCode` is set for every other CTS failure too (`NOT_AUTHENTICATED`, +`WORKSPACE_MISMATCH`, `EXPIRED_TOKEN`, …), and where one of those carries +remedy text — `MISSING_WORKSPACE_CRN` naming `CS_WORKSPACE_CRN`, say — it now +reaches `message` rather than being dropped. Treat the set as **open** — it +belongs to `@cipherstash/auth` and grows on its own release train, so compare +with `===` rather than switching exhaustively over it. + +`Encryption()` throws rather than returning a `Result`, so at client init the +same code rides on the thrown error: `(err as { authCode?: string }).authCode`. + +On the CLI, `stash env` reports a billing refusal as `usage_limit_exceeded` +(rather than `session_invalid`) and points at the dashboard instead of telling +you to log in again. + ## The strategies From `@cipherstash/auth`, re-exported by `@cipherstash/stack` so no separate diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 18453d1b2..19c38d2f8 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -630,6 +630,15 @@ Things to know: refuses to overwrite an existing file (also before anything is minted). In `--json` mode failures arrive as `{ status: "error", code, message }` on stdout. +- **`usage_limit_exceeded` is not a session problem.** The command renews the + device session before minting anything, and CipherStash refuses that renewal + with a 402 when the organisation is over its billing allowance. That case + reports `usage_limit_exceeded` rather than `session_invalid`, and points at + [dashboard.cipherstash.com](https://dashboard.cipherstash.com) rather than at + `stash auth login` — logging in again cannot mint a credential that is being + withheld on billing grounds. Same for `stash auth login` itself, which fails + with `USAGE_LIMIT_EXCEEDED` on the `--json` stream. See the `stash-auth` + skill for the full taxonomy. - **`--json` + `--write` compose**: the file is written and the JSON confirmation (`{ status: "written", path, … }`) is deliberately secret-free, so captured CI logs never contain the key. diff --git a/skills/stash-encryption/SKILL.md b/skills/stash-encryption/SKILL.md index e3a742752..55f9bd27b 100644 --- a/skills/stash-encryption/SKILL.md +++ b/skills/stash-encryption/SKILL.md @@ -788,6 +788,8 @@ if (result.failure) { `StackError` is a discriminated union of all the error types above, enabling exhaustive `switch` handling. `EncryptionErrorTypes` provides runtime constants for each error type string. Use `getErrorMessage(error: unknown): string` to safely extract a message from any thrown value. +**Don't retry on `authCode`.** A failure whose cause was CipherStash's token service also carries `authCode` — and two of its values mean no retry can ever succeed: `USAGE_LIMIT_EXCEEDED` (the organisation is over its billing allowance) and `ORG_NOT_PROVISIONED`. The `message` already names the remedy, dashboard URL included; `authCode` is there so a retry loop can stop. `type` cannot express this — a billing refusal surfaces as `ClientInitError` or `EncryptionError` like any other failure. See the `stash-auth` skill, which is canonical for the auth failure taxonomy. + ```typescript import { EncryptionErrorTypes, type StackError, getErrorMessage } from "@cipherstash/stack/errors"