diff --git a/.changeset/stack-diagnostics-subpath.md b/.changeset/stack-diagnostics-subpath.md new file mode 100644 index 000000000..cb949f29d --- /dev/null +++ b/.changeset/stack-diagnostics-subpath.md @@ -0,0 +1,13 @@ +--- +'@cipherstash/stack': minor +--- + +Add a `@cipherstash/stack/diagnostics` subpath, for tooling that needs to prove the protect-ffi native binding is installed. + +It exports one function, `assertNativeBindingAvailable()`. Calling it forces the platform binary to load and throws the loader's own `MODULE_NOT_FOUND` — unwrapped, naming the missing `@cipherstash/protect-ffi--` package — if it is absent. Importing the subpath does not force anything, so the laziness that makes the native load cost nothing for callers that never encrypt is preserved. + +The subpath exists because there is no way to do this from outside: the package's loader is not in its `exports` map, and reading an export never reaches the `@neon-rs/load` proxy. Importing `@cipherstash/stack` itself is not a substitute either — the root entry re-exports the auth strategies, so evaluating it resolves `@cipherstash/auth`'s binding instead. This entry reaches protect-ffi and nothing else. + +It probes by calling `isEncrypted`, which has been published since 0.28.0, rather than protect-ffi's own `assertNativeBindingAvailable` — that export arrived with the lazy native load and is not in any released version, so re-exporting it would build here and fail wherever it shipped (a link-time error under ESM, an `undefined` under CJS). + +Available as both `import` and `require`. diff --git a/.changeset/stash-doctor-probes-native-bindings.md b/.changeset/stash-doctor-probes-native-bindings.md new file mode 100644 index 000000000..ff94dcd6e --- /dev/null +++ b/.changeset/stash-doctor-probes-native-bindings.md @@ -0,0 +1,15 @@ +--- +'stash': patch +--- + +`stash doctor` now detects a missing native binary. Both of its checks had stopped doing so, in different ways, and each reported a green row instead. + +**The encryption engine check never loaded anything.** Since the protect-ffi native load became lazy, importing the package resolves no platform binary — `@neon-rs/load`'s proxy resolves on first use — so the probe passed with nothing installed and the failure surfaced later, at the first encrypt. It now calls `assertNativeBindingAvailable()` through the new `@cipherstash/stack/diagnostics` subpath, which forces the load. + +**It was also reporting the wrong package.** Importing `@cipherstash/stack` reaches `@cipherstash/auth`, whose binding is eager, so the encryption row was really a second auth check: one signal rendered as two rows. The diagnostics subpath does not reach auth, so each row now means what it says. + +**A missing `@cipherstash/auth` binary produced a bare `Fatal error`.** That package's napi loader swallows the resolver's `MODULE_NOT_FOUND` and throws a summary carrying no error `code`, which the CLI's native-binary classifier did not recognise — so every command, not only `doctor`, skipped the recovery guidance and printed a raw message. The classifier now recognises that shape, and prints the missing package with the reinstall steps. + +`stash doctor` exits non-zero when either platform package is missing, and reports an install of `@cipherstash/stack` that predates the diagnostics subpath as unprobeable rather than failing on it. A run that could not complete a check now ends with "stash doctor could not run every check." instead of claiming they all passed — still exit 0, since an unrunnable check is not a diagnosis. + +**A package that is installed but broken is no longer reported as "not installed".** The check for an absent package matched the package name anywhere in the failure message, and the probe's own import path contains it — so a partially installed or partially built `@cipherstash/stack` was reported as one you simply had not installed yet, in green, with nothing to suggest looking further. It now matches on the specifier Node failed to resolve. diff --git a/AGENTS.md b/AGENTS.md index da9f95f20..c040b78bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,7 +72,7 @@ If these variables are missing, tests that require live encryption will fail or ## Repository Layout - `packages/stack`: Main package (`@cipherstash/stack`) containing the encryption client and all integrations - - Subpath exports: `@cipherstash/stack`, `@cipherstash/stack/identity`, `@cipherstash/stack/schema`, `@cipherstash/stack/eql/v3`, `@cipherstash/stack/v3`, `@cipherstash/stack/types`, `@cipherstash/stack/dynamodb`, `@cipherstash/stack/encryption`, `@cipherstash/stack/errors`, `@cipherstash/stack/adapter-kit`, `@cipherstash/stack/wasm-inline` (the Drizzle and Supabase integrations moved to their own packages — see below) + - Subpath exports: `@cipherstash/stack`, `@cipherstash/stack/identity`, `@cipherstash/stack/schema`, `@cipherstash/stack/eql/v3`, `@cipherstash/stack/v3`, `@cipherstash/stack/types`, `@cipherstash/stack/dynamodb`, `@cipherstash/stack/encryption`, `@cipherstash/stack/errors`, `@cipherstash/stack/adapter-kit`, `@cipherstash/stack/wasm-inline`, `@cipherstash/stack/diagnostics` (the Drizzle and Supabase integrations moved to their own packages — see below) - `packages/cli`: The `stash` CLI — auth, init, encryption schema, and database setup (`stash eql install`). Has its own `AGENTS.md`. - `packages/wizard`: AI-powered encryption setup (`@cipherstash/wizard`) - `packages/migrate`: Plaintext-to-encrypted column migration (`@cipherstash/migrate`) — resumable backfill, per-column state diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index fed5d1771..1208997ad 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -7,7 +7,7 @@ This package has **two** Vitest configs (plus a self-skipping live-Postgres mode | Command | Config | Scope | Needs build? | | --- | --- | --- | --- | | `pnpm --filter stash test` | `vitest.config.ts` | Unit tests under `src/__tests__/**` and `src/**/__tests__/**` | **Partly** — needs `@cipherstash/stack` built (see below). Turbo's `^build` supplies it in CI. | -| `pnpm --filter stash test:e2e` | `vitest.integration.config.ts` | E2E tests under `tests/e2e/**.e2e.test.ts` driving the built `dist/bin/stash.js` through a real pty (`node-pty`) | **Yes** — run `pnpm --filter stash build` first, or use the turbo `test:e2e` task which depends on `build`. | +| `pnpm --filter stash test:e2e` | `vitest.integration.config.ts` | E2E tests under `tests/e2e/**.e2e.test.ts` driving the built `dist/bin/stash.js` through a real pty (`node-pty`) | **Yes** — run `pnpm --filter stash build` first, or use the turbo `test:e2e` task which depends on `build`. One test also needs protect-ffi's native binding, which no `build` produces (see below). | The unit config explicitly excludes `tests/e2e/**` so the default `pnpm test` stays fast. @@ -86,6 +86,29 @@ exercise the same code paths. - **Build before E2E.** `dist/bin/stash.js` is the artifact under test. The turbo `test:e2e` task already depends on `build`, but if you invoke the script directly you must build first. +- **`doctor.e2e.test.ts` also needs protect-ffi's native binding, and no + `build` produces one.** `stash doctor` probes the encryption engine by + *calling* through `@cipherstash/stack/diagnostics` — importing it proves + nothing, since the neon load is lazy. `@cipherstash/stack` is a devDependency + of this package, so in the workspace the probe always resolves it and never + takes the "not installed, that's fine" arm; and the workspace-linked + `@cipherstash/protect-ffi-` carries no `index.node` until cargo has + run (protect-ffi's `build` is `tsc`, deliberately cargo-free — see the root + `AGENTS.md`). Without a binding the healthy-install test fails on a red + encryption row and exit 1, and `doctor` offers the recovery it has for an npm + user — reinstall `node_modules` — which does not fix this. **That is a + missing binding, not a broken checkout.** Build one (needs a Rust toolchain), + from `packages/protect-ffi`: + + ```bash + mise run build:debug # or: pnpm --filter @cipherstash/protect-ffi build:native + ``` + + CI never hits this: the `run-tests` job in `tests.yml` runs + `.github/actions/build-ffi-binding` long before the CLI E2E step, and that + action caches on a hash of the Rust inputs, so a JS-only PR pays a restore. + Only the healthy-path test needs a real binding: `doctor-missing-binary` + stages the absence itself, in the spawned CLI, and passes either way. - **macOS spawn-helper exec bit.** pnpm strips the executable bit when unpacking node-pty's prebuilds. The helper auto-fixes this at module load via `ensureSpawnHelperExecutable`. If you see `posix_spawnp failed` after diff --git a/packages/cli/src/__tests__/module-error-classification.test.ts b/packages/cli/src/__tests__/module-error-classification.test.ts new file mode 100644 index 000000000..78ece773d --- /dev/null +++ b/packages/cli/src/__tests__/module-error-classification.test.ts @@ -0,0 +1,160 @@ +import { createRequire } from 'node:module' +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' +import { isPackageMissing, isSubpathUnavailable } from '../module-error.js' +import { isNativeBinaryMissing } from '../native.js' + +// The `module-error.ts` classifiers `stash doctor` sorts a failed probe with. +// Each arm renders a different row and a different exit code, so a probe error +// landing in the wrong one is a wrong diagnosis, not a cosmetic slip — and the +// two here are the ones whose answers are indistinguishable to the user: a +// green "not installed", or advice to upgrade. +// +// Every fixture below is an error NODE raised, never one built by hand with the +// code and message pasted on. A hand-built fixture asserts on itself: it keeps +// passing when Node changes the shape the classifier has to recognise, which is +// exactly how `isNativeBinaryMissing` came to have a `@cipherstash/auth` arm +// that could never fire. +const require = createRequire(import.meta.url) + +function resolutionError(specifier: string): unknown { + try { + require.resolve(specifier) + } catch (err) { + return err + } + throw new Error(`${specifier} resolved; it was expected to fail`) +} + +// `stash` declares `@cipherstash/stack` as an OPTIONAL PEER at `>=1.0.0-rc.0`, +// and the encryption probe imports its `./diagnostics` subpath. So any install +// predating that subpath — a range the CLI itself permits — fails resolution +// with neither a missing package nor a missing binary. Without its own arm that +// error reaches doctor's `else` and is rethrown, surfacing as the launcher's +// bare `Fatal error` for an install that may be perfectly healthy. +describe('isSubpathUnavailable', () => { + // The two probes as `doctor` declares them: one imports a subpath, one does + // not. + const encryption = { pkg: '@cipherstash/stack', subpath: './no-such-subpath' } + const auth = { pkg: '@cipherstash/auth' } + + it('matches an installed package that does not publish the subpath', () => { + // A real package (so resolution gets as far as reading its exports map) + // with a subpath it has never had. + const err = resolutionError('@cipherstash/stack/no-such-subpath') + + expect((err as { code?: string }).code).toBe( + 'ERR_PACKAGE_PATH_NOT_EXPORTED', + ) + expect(isSubpathUnavailable(err, encryption)).toBe(true) + }) + + it('does not match a probe that imports no subpath', () => { + // The advice this arm renders names `@cipherstash/stack` and tells the user + // to upgrade it. Applied to the auth probe — which imports the package root + // — any exports failure raised somewhere inside auth's own dependency graph + // would answer a broken install with an unrelated upgrade and exit 0. + const err = resolutionError('@cipherstash/stack/no-such-subpath') + + expect(isSubpathUnavailable(err, auth)).toBe(false) + }) + + it('does not match a failure on a different subpath of the same package', () => { + // A dependency deeper in the probe's own import graph with an exports + // problem of its own is not "your @cipherstash/stack is too old". + const err = resolutionError('@cipherstash/stack/no-such-subpath') + + expect( + isSubpathUnavailable(err, { + pkg: '@cipherstash/stack', + subpath: './diagnostics', + }), + ).toBe(false) + }) + + it('does not match the errors the other arms own', () => { + // An absent package: Node names the BASE package here, which is why the + // probe classifies against `@cipherstash/stack` while importing the + // subpath, and why this must not be mistaken for a stale install. + const missing = resolutionError('@cipherstash/no-such-package/diagnostics') + expect((missing as { code?: string }).code).toBe('MODULE_NOT_FOUND') + expect(isSubpathUnavailable(missing, encryption)).toBe(false) + + const binary = new Error( + "Cannot find module '@cipherstash/protect-ffi-darwin-arm64'", + ) as Error & { code?: string } + binary.code = 'MODULE_NOT_FOUND' + expect(isSubpathUnavailable(binary, encryption)).toBe(false) + expect(isNativeBinaryMissing(binary)).toBe(true) + }) + + it('ignores non-Error values', () => { + expect(isSubpathUnavailable(undefined, encryption)).toBe(false) + expect( + isSubpathUnavailable( + { code: 'ERR_PACKAGE_PATH_NOT_EXPORTED' }, + encryption, + ), + ).toBe(false) + }) +}) + +// The arm that decides whether a probe failure means "you have not installed +// this yet" — a green row for the optional peer — or something the user has to +// act on. Getting a false positive here is the worst outcome doctor has: it +// tells a user with a broken install that there is nothing to fix. +describe('isPackageMissing', () => { + it('matches the package the probe named', () => { + const err = resolutionError('@cipherstash/no-such-package/diagnostics') + + expect(isPackageMissing(err, '@cipherstash/no-such-package')).toBe(true) + }) + + it('does not match a load failure for a file inside an installed package', () => { + // The probe imports `@cipherstash/stack/diagnostics`, so its failures name + // paths INSIDE the package — `…/node_modules/@cipherstash/stack/dist/ + // diagnostics.js`. A substring test for the package name matches that + // happily and reports a package that is installed but broken (an + // interrupted install, a partially built workspace) as one the user has + // simply not installed yet: a green row, and no reason to look further. + // + // The path the CLI's own resolution produces — `packages/cli/node_modules/ + // @cipherstash/stack/…`, the workspace's stand-in for a user's install. + // Not `require.resolve('@cipherstash/stack/package.json')`: that returns + // the symlink's REAL path (`packages/stack/…`), which drops the scoped + // name the bug turns on. Node raises the error either way; only the path + // handed to it is composed here. + // + // Resolved through CJS to keep Node's own resolver in play rather than + // Vitest's module pipeline. The ESM form of this message differs only by a + // trailing `imported from …`, and both quote the same specifier. + const cliRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + '../..', + ) + const err = resolutionError( + path.join( + cliRoot, + 'node_modules/@cipherstash/stack/dist/no-such-file.js', + ), + ) + + expect((err as { code?: string }).code).toBe('MODULE_NOT_FOUND') + expect((err as Error).message).toContain('@cipherstash/stack') + expect(isPackageMissing(err, '@cipherstash/stack')).toBe(false) + }) + + it('does not match a sibling package that merely shares a prefix', () => { + const err = resolutionError('@cipherstash/no-such-package-extra') + + expect(isPackageMissing(err, '@cipherstash/no-such-package')).toBe(false) + }) + + it('ignores errors that are not module resolution failures', () => { + expect( + isPackageMissing(new Error('@cipherstash/stack'), '@cipherstash/stack'), + ).toBe(false) + expect(isPackageMissing(undefined, '@cipherstash/stack')).toBe(false) + }) +}) diff --git a/packages/cli/src/__tests__/native.test.ts b/packages/cli/src/__tests__/native.test.ts index 544b13f3d..8a98a94bd 100644 --- a/packages/cli/src/__tests__/native.test.ts +++ b/packages/cli/src/__tests__/native.test.ts @@ -16,6 +16,20 @@ function moduleError( return err } +/** + * What napi-rs's generated loader throws when the platform package is absent — + * `@cipherstash/auth`'s shape. Verbatim from `stack-auth-node.js`, whose + * `loadBinding()` swallows each candidate's resolver error (`try { … } catch + * (_) {}`) and ends with this, so no `code` and no `requireStack` survive. + */ +function napiLoadError(pkg: string): Error { + return new Error( + `Failed to load native binding for ${process.platform}-${process.arch}. ` + + `Ensure the optional dependency "${pkg}" is installed, ` + + 'or run "napi build" for local development.', + ) +} + describe('isNativeBinaryMissing', () => { it('matches a missing platform-specific protect-ffi binary', () => { // The real-world failure: npm skipped the optional native dependency. @@ -29,17 +43,38 @@ describe('isNativeBinaryMissing', () => { expect(isNativeBinaryMissing(err)).toBe(true) }) - it('matches the auth native binary on linux/windows targets', () => { - expect( - isNativeBinaryMissing( - moduleError("Cannot find module '@cipherstash/auth-linux-x64-gnu'"), - ), - ).toBe(true) + it('matches the auth native binary across targets', () => { + // Auth is napi-rs, so these are the shape below, not `moduleError` — kept + // as a set because the platform token varies (`-gnu`, `-msvc`, plain). + for (const pkg of [ + '@cipherstash/auth-linux-x64-gnu', + '@cipherstash/auth-win32-x64-msvc', + '@cipherstash/auth-darwin-arm64', + ]) { + expect(isNativeBinaryMissing(napiLoadError(pkg)), pkg).toBe(true) + } + }) + + it('matches the napi loader failure, which carries no code at all', () => { + // The shape that made this whole helper a no-op for `@cipherstash/auth`. + // Its loader requires each candidate inside `try { … } catch (_) {}` and + // throws its own summary, so the resolver's MODULE_NOT_FOUND never + // escapes: no `code`, no `requireStack`, only the message. This assertion + // existed before as a hand-built error carrying `code = + // 'MODULE_NOT_FOUND'` — a shape `@cipherstash/auth` has never thrown, so + // it passed over a path that could not work, and `stash doctor` printed a + // bare `Fatal error` for a missing auth binary instead of the guidance. + const err = napiLoadError('@cipherstash/auth-darwin-arm64') + expect((err as ModuleError).code).toBeUndefined() + expect(isNativeBinaryMissing(err)).toBe(true) + }) + + it('does not match a napi loader failure from someone else', () => { + // Both halves of the message test have to hold: the platform package is + // what makes it ours. expect( - isNativeBinaryMissing( - moduleError("Cannot find module '@cipherstash/auth-win32-x64-msvc'"), - ), - ).toBe(true) + isNativeBinaryMissing(napiLoadError('@other/thing-darwin-arm64')), + ).toBe(false) }) it('matches when only the neon loader appears in the require stack', () => { diff --git a/packages/cli/src/commands/doctor/index.ts b/packages/cli/src/commands/doctor/index.ts index bc5ec8542..008b09b50 100644 --- a/packages/cli/src/commands/doctor/index.ts +++ b/packages/cli/src/commands/doctor/index.ts @@ -1,74 +1,125 @@ import * as p from '@clack/prompts' import { messages } from '../../messages.js' +import { isPackageMissing, isSubpathUnavailable } from '../../module-error.js' import { currentTarget, isNativeBinaryMissing, reportNativeBinaryMissing, } from '../../native.js' -// Native-bearing packages the CLI loads at runtime. Importing each forces its -// @neon-rs/load proxy to resolve the platform binary — the same load that fails -// when npm skips the optional dependency. @cipherstash/stack is the peer that -// pulls protect-ffi; it may legitimately be absent until `stash init`. -const PROBES: { label: string; pkg: string; optional?: boolean }[] = [ +// Native-bearing packages the CLI loads at runtime, and — per package — the +// operation that forces its platform binary to resolve. +// +// Importing a package is not a probe on its own. It was, for protect-ffi, until +// the load became lazy: `@neon-rs/load`'s proxy now resolves the binary on +// first property access inside a wrapper body, so the package imports cleanly +// with nothing installed and fails at the first encrypt instead. Each probe +// therefore names what to CALL, not just what to import. +interface Probe { + label: string + /** + * The package a "not installed" message names, which is not always what the + * probe imports: Node reports the base package for a missing subpath of an + * absent package, so the encryption probe imports `…/diagnostics` and + * classifies against `@cipherstash/stack`. + */ + pkg: string + /** + * The subpath of `pkg` that `force()` imports, when it imports one. Only a + * probe that asks for a subpath can fail for want of it, so this is what + * scopes the too-old-to-probe arm to the probe it has advice for. + */ + subpath?: string + /** May legitimately be absent until `stash init`, so absence is not failure. */ + optional?: boolean + force(): Promise +} + +const PROBES: Probe[] = [ { label: 'Encryption engine (@cipherstash/stack → protect-ffi)', pkg: '@cipherstash/stack', + subpath: './diagnostics', optional: true, + async force() { + // `@cipherstash/stack/diagnostics` exists for this call: it reaches + // protect-ffi WITHOUT reaching `@cipherstash/auth`, which the root entry + // does (it re-exports the auth strategies, and that package's binding is + // eager). Probing the root entry measured auth's binary and reported it + // under this label — two rows, one signal. + const diagnostics = await import('@cipherstash/stack/diagnostics') + diagnostics.assertNativeBindingAvailable() + }, + }, + { + label: 'Auth (@cipherstash/auth)', + pkg: '@cipherstash/auth', + async force() { + // No counterpart call needed. This package's entry is `module.exports = + // { ...native }`, and the spread resolves the binding at module + // evaluation — so here the import IS the probe. + await import('@cipherstash/auth') + }, }, - { label: 'Auth (@cipherstash/auth)', pkg: '@cipherstash/auth' }, ] -function report(ok: boolean, label: string, detail?: string) { +type Outcome = 'ok' | 'warn' | 'fail' + +function report(outcome: Outcome, label: string, detail?: string) { const text = detail ? `${label} — ${detail}` : label - if (ok) p.log.success(text) + if (outcome === 'ok') p.log.success(text) + else if (outcome === 'warn') p.log.warn(text) else p.log.error(text) } -function isPackageMissing(err: unknown, pkg: string): boolean { - if (!(err instanceof Error)) return false - const code = (err as { code?: string }).code - if (code !== 'ERR_MODULE_NOT_FOUND' && code !== 'MODULE_NOT_FOUND') { - return false - } - return err.message.includes(pkg) -} - export async function doctorCommand(): Promise { p.intro(messages.doctor.title) let failed = false + // A check that could not be RUN, as distinct from one that ran and failed. + // Tracked separately so the outro can say so — `failed` would exit 1 on an + // install with nothing known to be wrong with it, and neither flag would + // claim every check passed. + let incomplete = false let nativeError: unknown const nodeMajor = Number(process.versions.node.split('.')[0]) const nodeOk = Number.isFinite(nodeMajor) && nodeMajor >= 22 report( - nodeOk, + nodeOk ? 'ok' : 'fail', `Node.js ${process.versions.node}`, nodeOk ? '' : 'requires >= 22', ) if (!nodeOk) failed = true - report(true, `${messages.doctor.platformLabel} ${currentTarget()}`) + report('ok', `${messages.doctor.platformLabel} ${currentTarget()}`) for (const probe of PROBES) { try { - await import(probe.pkg) - report(true, probe.label) + await probe.force() + report('ok', probe.label) } catch (err) { if (isNativeBinaryMissing(err)) { - report(false, probe.label, 'native binary missing') + report('fail', probe.label, messages.doctor.nativeBinaryMissing) failed = true - nativeError = err + // First one wins. The guidance below is the same whichever package + // reported it; keeping the first keeps the note aligned with the first + // failing row rather than the last. + nativeError ??= err } else if (isPackageMissing(err, probe.pkg)) { // A missing top-level package is a different problem from a missing // native binary; only the latter is what these guards exist for. report( - Boolean(probe.optional), + probe.optional ? 'ok' : 'fail', probe.label, - probe.optional ? 'not installed (run `stash init`)' : 'not installed', + probe.optional + ? messages.doctor.notInstalledOptional + : messages.doctor.notInstalled, ) if (!probe.optional) failed = true + } else if (isSubpathUnavailable(err, probe)) { + report('warn', probe.label, messages.doctor.cannotProbe) + incomplete = true } else { throw err } @@ -80,8 +131,14 @@ export async function doctorCommand(): Promise { } if (failed) { - p.outro('stash doctor found problems.') + p.outro(messages.doctor.problemsFound) process.exit(1) } - p.outro(messages.doctor.allChecksPassed) + // Exit 0 either way — an unrunnable check is not a diagnosis — but only one + // of these two is true. + p.outro( + incomplete + ? messages.doctor.checksIncomplete + : messages.doctor.allChecksPassed, + ) } diff --git a/packages/cli/src/messages.ts b/packages/cli/src/messages.ts index aff38c792..fc9e77251 100644 --- a/packages/cli/src/messages.ts +++ b/packages/cli/src/messages.ts @@ -27,6 +27,26 @@ export const messages = { /** Leader of the platform check line; the `-` is appended. */ platformLabel: 'Platform', allChecksPassed: 'All checks passed.', + /** Row detail when a probe reached the loader and no platform binary was there. */ + nativeBinaryMissing: 'native binary missing', + /** Row detail for an absent optional package — recoverable, not a failure. */ + notInstalledOptional: 'not installed (run `stash init`)', + /** Row detail for an absent required package. */ + notInstalled: 'not installed', + /** + * Row detail when `@cipherstash/stack` is installed but predates the + * `./diagnostics` subpath, so there is no way to force its binding. Not a + * failure: the install may be perfectly healthy, we just can't prove it. + */ + cannotProbe: 'installed, but too old to probe — upgrade @cipherstash/stack', + problemsFound: 'stash doctor found problems.', + /** + * Outro when a check could not be run but nothing failed. Distinct from + * `allChecksPassed` on purpose: a run that skipped a check has not passed + * it, and saying otherwise is the one line of doctor's output a user has no + * way to second-guess. + */ + checksIncomplete: 'stash doctor could not run every check.', }, auth: { /** Same shape as `cli.usagePrefix` — leader only. */ diff --git a/packages/cli/src/module-error.ts b/packages/cli/src/module-error.ts index 4f5f06c98..524ec669c 100644 --- a/packages/cli/src/module-error.ts +++ b/packages/cli/src/module-error.ts @@ -3,7 +3,8 @@ // import) whose message names the unresolved specifier. These turn that raw // error into structured data callers translate into guidance — missing native // binaries in `native.ts`, missing CipherStash packages in -// `config/missing-package.ts`. +// `config/missing-package.ts`, and which of those a failed `stash doctor` probe +// hit in `commands/doctor`. /** A Node module-resolution error. */ export interface ModuleError extends Error { @@ -26,3 +27,59 @@ export function isModuleNotFound(err: unknown): err is ModuleError { export function moduleNotFoundSpecifier(err: ModuleError): string | undefined { return /Cannot find (?:module|package) '([^']+)'/.exec(err.message)?.[1] } + +/** + * True when `pkg` itself is what failed to resolve — it is not installed — as + * opposed to installed and unable to load something. + * + * Matched on the SPECIFIER Node quotes, never on the message text. A substring + * test for the package name is wrong in both directions: an import of + * `@cipherstash/stack/diagnostics` that fails on a missing `dist/` file names + * `…/node_modules/@cipherstash/stack/dist/diagnostics.js` — installed and + * broken, reported as never installed — and `@cipherstash/stack-drizzle` + * contains `@cipherstash/stack`. + */ +export function isPackageMissing(err: unknown, pkg: string): boolean { + if (!isModuleNotFound(err)) return false + const specifier = moduleNotFoundSpecifier(err) + if (specifier === undefined) return false + // For a subpath of an absent package Node quotes the base package under ESM + // but the whole specifier under CJS. Both mean this package. + return specifier === pkg || specifier.startsWith(`${pkg}/`) +} + +/** + * True when `target.pkg` is installed but does not publish `target.subpath` — + * an `@cipherstash/stack` older than `./diagnostics`, say. False when the + * caller asked for no subpath, since then there is none to be missing. + * + * Worth telling apart because it is not a broken install: `stash` declares + * `@cipherstash/stack` as an optional peer with a wide range, so every version + * inside that range predating a subpath lands here, and the user did nothing + * wrong. + * + * Narrow on purpose, because the guidance that follows from it names a package + * and a version. `ERR_PACKAGE_PATH_NOT_EXPORTED` alone does not carry that: an + * exports failure can come from anywhere in an import graph, including a + * dependency of a dependency. So the error must be about this subpath of this + * package, or it is somebody else's problem and must not be answered with an + * upgrade. + */ +export function isSubpathUnavailable( + err: unknown, + target: { pkg: string; subpath?: string }, +): boolean { + if (target.subpath === undefined) return false + if (!(err instanceof Error)) return false + if ((err as { code?: string }).code !== 'ERR_PACKAGE_PATH_NOT_EXPORTED') { + return false + } + // Node names both halves: `Package subpath './diagnostics' is not defined by + // "exports" in /…/@cipherstash/stack/package.json`. Separators normalised + // because win32 is a supported target and that path is built by the OS. + const message = err.message.replaceAll('\\', '/') + return ( + message.includes(`'${target.subpath}'`) && + message.includes(`${target.pkg}/package.json`) + ) +} diff --git a/packages/cli/src/native.ts b/packages/cli/src/native.ts index e79e33e38..011469b80 100644 --- a/packages/cli/src/native.ts +++ b/packages/cli/src/native.ts @@ -32,19 +32,34 @@ export function currentTarget(): string { return `${process.platform}-${process.arch}` } +// napi-rs's generated loader, which is how `@cipherstash/auth` loads (neon is +// only protect-ffi). It requires each candidate inside `try { … } catch (_) {}` +// and then throws this — so the resolver's MODULE_NOT_FOUND never escapes, and +// what does escape is a plain Error with NO `code` and no `requireStack`. It +// names the platform package, which is the only thing left to key on. +const NAPI_LOAD_FAILURE = /Failed to load native binding for / + /** * True when `err` is a failure to load one of our prebuilt native addons (a * missing `@cipherstash/--` optional package), as opposed * to a missing top-level package or any other module error. */ export function isNativeBinaryMissing(err: unknown): err is ModuleError { - if (!isModuleNotFound(err)) return false - const haystack = `${err.message}\n${(err.requireStack ?? []).join('\n')}` - // A platform-suffixed @cipherstash package, or a failure surfaced from the - // neon loader, both mean the optional native binary wasn't installed. - return ( - PLATFORM_PKG.test(haystack) || /[\\/]@neon-rs[\\/]load[\\/]/.test(haystack) - ) + if (isModuleNotFound(err)) { + const haystack = `${err.message}\n${(err.requireStack ?? []).join('\n')}` + // A platform-suffixed @cipherstash package, or a failure surfaced from the + // neon loader, both mean the optional native binary wasn't installed. + return ( + PLATFORM_PKG.test(haystack) || + /[\\/]@neon-rs[\\/]load[\\/]/.test(haystack) + ) + } + + if (!(err instanceof Error)) return false + // The code-less napi shape. Both halves are required: neither is narrow + // enough alone — `Failed to load native binding` pins the thrower, the + // platform package pins it to one of ours. No `requireStack` to consult. + return NAPI_LOAD_FAILURE.test(err.message) && PLATFORM_PKG.test(err.message) } function missingModuleName(err: ModuleError): string | undefined { diff --git a/packages/cli/tests/e2e/doctor-missing-binary.e2e.test.ts b/packages/cli/tests/e2e/doctor-missing-binary.e2e.test.ts new file mode 100644 index 000000000..0f5ae2594 --- /dev/null +++ b/packages/cli/tests/e2e/doctor-missing-binary.e2e.test.ts @@ -0,0 +1,158 @@ +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { messages } from '../../src/messages.js' +import { render } from '../helpers/pty.js' + +// The case `stash doctor` exists for, and the one nothing covered: a platform +// binary that npm skipped (https://github.com/npm/cli/issues/4828). +// +// `doctor.e2e.test.ts` runs the healthy install. That cannot catch a probe that +// has stopped probing — and both of them had. Since the protect-ffi load became +// lazy, importing the package resolves no binary at all, so the encryption row +// went green with nothing installed; and `@cipherstash/auth`'s napi loader +// throws a code-less Error the CLI's classifier did not recognise, so a missing +// auth binary surfaced as a bare `Fatal error` with none of the recovery +// guidance. Both are only visible from outside the process, with a binary +// actually absent. +// +// Absent by patching the resolver in the spawned CLI, not by moving files: the +// suite must not mutate the checkout it runs in, and this works whether or not +// the developer has run `build:native`. Same technique as +// `packages/protect-ffi/src/lintWiring.test.ts`, which re-runs its own suite +// this way. + +interface Target { + /** Package whose platform binary disappears. */ + pkg: '@cipherstash/protect-ffi' | '@cipherstash/auth' + /** The doctor row that must go red. */ + label: string + /** Extra specifiers this package's loader reaches for. */ + extra: string +} + +const TARGETS: Target[] = [ + { + pkg: '@cipherstash/protect-ffi', + label: 'Encryption engine (@cipherstash/stack → protect-ffi)', + // `src/load.cts`'s debug arm — a local cargo build sitting beside `lib/`. + // A contributor who has run `pnpm run debug` has one, and without this the + // fixture would load the binding it means to be missing. + extra: String.raw`|(?:^|[\\/])index\.node$`, + }, + { + pkg: '@cipherstash/auth', + label: 'Auth (@cipherstash/auth)', + // napi's local-build arm, tried before the platform package. + extra: String.raw`|(?:^|[\\/])stack-auth-node\.node$`, + }, +] + +/** + * Writes a CJS preload that makes `pkg`'s platform binary unresolvable, and + * returns the `NODE_OPTIONS` value that loads it. + * + * `Module._load`, not `Module._resolveFilename`: `_load` keys a relative-resolve + * fast path on (parent directory, request) and returns cached modules without + * consulting the resolver, so a redirect installed there can simply never be + * asked. `fs` is patched alongside it because a loader that stats an artifact + * before requiring it would otherwise see a file that `require` then refuses. + */ +function hideBinaryOf(target: Target): string { + const dir = mkdtempSync(join(tmpdir(), 'stash-doctor-missing-')) + const preload = join(dir, 'hide-binding.cjs') + const scope = target.pkg.replace('@cipherstash/', '') + writeFileSync( + preload, + ` +const fs = require('node:fs') +const Module = require('node:module') + +const BINDING = /@cipherstash[\\\\/]${scope}-(?:darwin|linux|win32)-${target.extra}/ + +const load = Module._load +Module._load = function (request, parent, isMain) { + if (BINDING.test(request)) { + const error = new Error("Cannot find module '" + request + "'") + error.code = 'MODULE_NOT_FOUND' + throw error + } + return load.call(this, request, parent, isMain) +} + +const existsSync = fs.existsSync +fs.existsSync = (p) => (BINDING.test(String(p)) ? false : existsSync(p)) + +const statSync = fs.statSync +fs.statSync = (p, ...rest) => { + if (!BINDING.test(String(p))) return statSync(p, ...rest) + const error = new Error('ENOENT: no such file or directory, stat ' + p) + error.code = 'ENOENT' + throw error +} + +Module.syncBuiltinESMExports() +`, + ) + // Quoted: Node splits NODE_OPTIONS on whitespace unless a value is wrapped in + // double quotes, and `preload` sits under a tmpdir this file did not choose. + return [process.env.NODE_OPTIONS, `--require "${preload}"`] + .filter(Boolean) + .join(' ') +} + +const currentTarget = `${process.platform}-${process.arch}` + +describe('stash doctor — a platform binary is missing', () => { + it.each( + TARGETS.map((t) => [t.pkg, t] as const), + )('fails the %s row, names the package and exits non-zero', async (_pkg, target) => { + // Wider than the 100-col default so the note's `Missing package:` line is + // asserted as written rather than as clack happened to wrap it. + const r = render(['doctor'], { + env: { NODE_OPTIONS: hideBinaryOf(target) }, + cols: 140, + }) + const { exitCode } = await r.exit + + expect( + exitCode, + `stash doctor passed with no ${target.pkg} binary installed:\n${r.output}`, + ).toBe(1) + expect(r.output).toContain( + `${target.label} — ${messages.doctor.nativeBinaryMissing}`, + ) + expect(r.output).toContain(messages.doctor.problemsFound) + + // The guidance, not just a red row: the platform package by name is the + // one piece of it a user cannot work out for themselves. + expect(r.output).toContain(`${target.pkg}-${currentTarget}`) + expect(r.output).not.toContain('Fatal error') + }) + + it('fails only the row whose binary is missing', async () => { + // Non-vacuity for the pair above. Both probes went through + // `@cipherstash/stack` before this change — the root entry reaches + // `@cipherstash/auth`, whose binding is eager — so hiding auth's binary + // reddened both rows and hiding protect-ffi's reddened neither. Either way + // the two rows reported one signal, and the assertions above cannot see it. + const protectFfi = TARGETS[0] + if (!protectFfi) throw new Error('TARGETS is empty') + + const r = render(['doctor'], { + env: { NODE_OPTIONS: hideBinaryOf(protectFfi) }, + cols: 140, + }) + await r.exit + + // Counted by splitting, not by `new RegExp(message)`: the needle is copy + // from `messages.ts`, and copy is free to grow a `.`, `(` or `?` — which a + // regex would silently reinterpret rather than fail on. + const failures = + r.output.split(messages.doctor.nativeBinaryMissing).length - 1 + expect(failures, `expected exactly one failing row:\n${r.output}`).toBe(1) + expect(r.output).toContain(protectFfi.label) + expect(r.output).not.toContain('@cipherstash/auth-') + }) +}) diff --git a/packages/cli/tests/e2e/doctor-probe-classification.e2e.test.ts b/packages/cli/tests/e2e/doctor-probe-classification.e2e.test.ts new file mode 100644 index 000000000..1650f7dd8 --- /dev/null +++ b/packages/cli/tests/e2e/doctor-probe-classification.e2e.test.ts @@ -0,0 +1,217 @@ +import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' +import { registerHooks } from 'node:module' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { pathToFileURL } from 'node:url' +import { describe, expect, it } from 'vitest' +import { messages } from '../../src/messages.js' +import { render } from '../helpers/pty.js' + +// `doctor` sorts a probe failure into four arms, and only one of them — the +// missing platform binary — had coverage (`doctor-missing-binary.e2e.test.ts`). +// These are the ones a user reaches by doing nothing wrong: +// +// * `@cipherstash/stack` absent. It is an OPTIONAL PEER, so `npx stash +// doctor` in a project that has not run `stash init` lands here. Must read +// as recoverable, not as a failure. +// * `@cipherstash/stack` installed but older than the `./diagnostics` +// subpath — every version inside the `>=1.0.0-rc.0` peer range that the CLI +// itself permits. Unclassified, this reaches doctor's `else`, is rethrown, +// and surfaces as the launcher's bare `Fatal error`. +// * The same error code from a probe that asks for no subpath, which cannot +// mean "upgrade `@cipherstash/stack`" and must not be answered with it. +// +// Every error here is raised by NODE'S OWN resolver against a package layout +// this file builds — an absent package, an older one — never hand-built with +// the code and message pasted on. A fixture like that keeps passing when Node +// changes either, which is the whole risk being covered. + +/** Mirrors the probe labels in `src/commands/doctor/index.ts`. */ +const ENCRYPTION_LABEL = 'Encryption engine (@cipherstash/stack → protect-ffi)' +const AUTH_LABEL = 'Auth (@cipherstash/auth)' + +interface Unresolve { + /** The bare specifier to divert. */ + specifier: string + /** + * A package to install into the directory the specifier is re-resolved from, + * carrying an `exports` map that does not answer what the probe asks for. + * Omit and the directory stays empty, which is how an ABSENT package is + * staged — no `node_modules` chain to find it in, so Node raises + * `ERR_MODULE_NOT_FOUND` naming the base package. + */ + installed?: { pkg: string; exports: Record } +} + +/** + * Writes a hook module that re-resolves `specifier` against a directory this + * test controls, and returns the `NODE_OPTIONS` value that loads it. + * + * Moving the IMPORTER rather than rewriting the specifier: the specifier the + * CLI asks for has to reach Node's resolver unchanged, because what the + * classifier keys on is which subpath of which package the error names. A + * redirect to some other subpath produces the right error CODE against the + * wrong subpath, which is a shape the real failure never has. + * + * A resolve hook rather than the `Module._load` patch its sibling suite uses: + * the probe is an `await import()` of a real ESM package, so it never reaches + * the CJS loader. `registerHooks` is synchronous and in-process, so the error + * propagates out of the dynamic import exactly as an unhooked failure would. + */ +function unresolve({ specifier, installed }: Unresolve): string { + const dir = mkdtempSync(join(tmpdir(), 'stash-doctor-probe-')) + if (installed) { + const pkgDir = join(dir, 'node_modules', ...installed.pkg.split('/')) + mkdirSync(pkgDir, { recursive: true }) + writeFileSync( + join(pkgDir, 'package.json'), + JSON.stringify({ + name: installed.pkg, + version: '0.0.0', + exports: installed.exports, + }), + ) + } + // The importer is never written — resolution fails before anything is read. + // It only has to sit in this directory, so that what is (or is not) in the + // `node_modules` beside it is what Node resolves against. + const parent = pathToFileURL(join(dir, 'importer.mjs')).href + const hook = join(dir, 'unresolve.mjs') + writeFileSync( + hook, + `import { registerHooks } from 'node:module' + +const SPECIFIER = ${JSON.stringify(specifier)} +const PARENT = ${JSON.stringify(parent)} + +registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier !== SPECIFIER) return nextResolve(specifier, context) + return nextResolve(specifier, { ...context, parentURL: PARENT }) + }, +}) +`, + ) + // Quoted: Node splits NODE_OPTIONS on whitespace unless a value is wrapped in + // double quotes, and the hook sits under a tmpdir this file did not choose. + return [process.env.NODE_OPTIONS, `--import "${pathToFileURL(hook).href}"`] + .filter(Boolean) + .join(' ') +} + +// `module.registerHooks` landed in Node 22.15, and `engines` only asks for +// `>=22`. Skipped rather than failed on an older runtime: the child would die +// in the preload with a TypeError that reads as a broken CLI. CI runs 22 and 24 +// at their latest patch, so this never skips there. +const hooksAvailable = typeof registerHooks === 'function' + +describe.skipIf(!hooksAvailable)('stash doctor — probe classification', () => { + it('reports an absent @cipherstash/stack as recoverable, not a failure', async () => { + // The `npx stash doctor` path in a project that has not run `stash init`. + // Node names the BASE package for a missing subpath of an absent package, + // which is why the probe imports `./diagnostics` but classifies against + // `@cipherstash/stack`. + const r = render(['doctor'], { + env: { + NODE_OPTIONS: unresolve({ + specifier: '@cipherstash/stack/diagnostics', + }), + }, + cols: 140, + }) + const { exitCode } = await r.exit + + expect(exitCode, r.output).toBe(0) + expect(r.output).toContain( + `${ENCRYPTION_LABEL} — ${messages.doctor.notInstalledOptional}`, + ) + expect(r.output).toContain(messages.doctor.allChecksPassed) + expect(r.output).not.toContain('Fatal error') + // The optional package's absence must not be dressed up as a missing + // binary — that would send the user to a reinstall for a package they + // simply have not installed yet. + expect(r.output).not.toContain(messages.doctor.nativeBinaryMissing) + }) + + it('warns, and does not claim every check passed, when a probe cannot run', async () => { + // An `@cipherstash/stack` older than the `./diagnostics` subpath. Nothing + // is known to be wrong with it — the install may be perfectly healthy — so + // this is not a failure and must not exit non-zero. It is also not a pass: + // the check did not run, and an outro saying every check passed would be + // the one line of output that is untrue. + const r = render(['doctor'], { + env: { + NODE_OPTIONS: unresolve({ + specifier: '@cipherstash/stack/diagnostics', + // An `@cipherstash/stack` from before the subpath existed: present, + // resolvable, and its `exports` answers `.` and nothing else. + installed: { + pkg: '@cipherstash/stack', + exports: { '.': './dist/index.js' }, + }, + }), + }, + cols: 140, + }) + const { exitCode } = await r.exit + + expect(exitCode, r.output).toBe(0) + expect(r.output).toContain( + `${ENCRYPTION_LABEL} — ${messages.doctor.cannotProbe}`, + ) + expect(r.output).toContain(messages.doctor.checksIncomplete) + expect(r.output).not.toContain(messages.doctor.allChecksPassed) + // The arm exists to keep this error out of doctor's `else`, where it is + // rethrown and the launcher prints it raw. + expect(r.output).not.toContain('Fatal error') + }) + + it('does not answer an exports failure elsewhere with stack-upgrade advice', async () => { + // The too-old-to-probe arm keys on an error code, and every probe's failure + // is offered to it. `ERR_PACKAGE_PATH_NOT_EXPORTED` can come from anywhere + // in a probe's import graph — a dependency with an exports problem of its + // own — and on the auth probe, which imports no subpath at all, it cannot + // mean "your @cipherstash/stack is too old". Answering a broken install + // with an unrelated upgrade and exiting 0 is a worse outcome than not + // classifying it. + const r = render(['doctor'], { + env: { + NODE_OPTIONS: unresolve({ + specifier: '@cipherstash/auth', + // An auth package whose `exports` does not answer its own root — + // same error code as the case above, from a package the arm has no + // advice for. + installed: { + pkg: '@cipherstash/auth', + exports: { './sub': './sub.js' }, + }, + }), + }, + cols: 140, + }) + const { exitCode } = await r.exit + + expect(r.output).not.toContain(messages.doctor.cannotProbe) + expect(r.output).not.toContain(messages.doctor.allChecksPassed) + expect(exitCode, r.output).toBe(1) + }) + + it('fails on an absent required package', async () => { + // `@cipherstash/auth` is a hard dependency, so absence is a broken install + // — the row must go red and the run must exit non-zero. Non-optional is a + // separate branch from the case above and would otherwise be asserted + // nowhere. + const r = render(['doctor'], { + env: { NODE_OPTIONS: unresolve({ specifier: '@cipherstash/auth' }) }, + cols: 140, + }) + const { exitCode } = await r.exit + + expect(exitCode, r.output).toBe(1) + expect(r.output).toContain( + `${AUTH_LABEL} — ${messages.doctor.notInstalled}`, + ) + expect(r.output).toContain(messages.doctor.problemsFound) + expect(r.output).not.toContain('Fatal error') + }) +}) diff --git a/packages/stack/__tests__/diagnostics-entry.test.ts b/packages/stack/__tests__/diagnostics-entry.test.ts new file mode 100644 index 000000000..b9d5886c7 --- /dev/null +++ b/packages/stack/__tests__/diagnostics-entry.test.ts @@ -0,0 +1,325 @@ +import { execFileSync } from 'node:child_process' +import { + existsSync, + mkdtempSync, + readdirSync, + readFileSync, + realpathSync, + statSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { pathToFileURL } from 'node:url' +import { describe, expect, it } from 'vitest' + +// `@cipherstash/stack/diagnostics` exists so `stash doctor` can prove the +// protect-ffi native binding is installed. Three properties make it work, and +// each one is invisible to a normal unit test: +// +// 1. It must not reach `@cipherstash/auth`. The root entry does — it +// re-exports the auth strategies — and that package's binding is EAGER +// (`module.exports = { ...native }`). A probe that lands on auth reports +// auth's binary under the encryption engine's label, which is the bug +// this entry was added to fix. Bundling is where that regresses: the main +// tsup config emits ESM with `splitting: true`, so a shared chunk is one +// import away. Hence an assertion on the ARTIFACT, not on the source. +// +// 2. Importing it must NOT force the load, and calling it must. That is the +// whole contract `doctor` classifies on, and it is a property of the +// emitted code — esbuild's CJS interop enumerates the required module to +// build its re-exports, and enumerating the wrong object is exactly what +// made the load eager before `b99cbd92`. +// +// 3. The loader's error must arrive unwrapped, so `MODULE_NOT_FOUND` and the +// platform package name survive for the CLI to key on. +// +// Mirrors `wasm-inline-bundle-isolation.test.ts`, which guards the same class +// of accident on the other native-free entry. + +const packageRoot = path.resolve(__dirname, '..') +const distDir = path.join(packageRoot, 'dist') +const srcDir = path.join(packageRoot, 'src') + +function newestMtime(dir: string): number { + let newest = 0 + const stack: string[] = [dir] + while (stack.length > 0) { + const current = stack.pop() as string + const stat = statSync(current) + if (stat.isDirectory()) { + for (const entry of readdirSync(current)) { + stack.push(path.join(current, entry)) + } + } else if (stat.mtimeMs > newest) { + newest = stat.mtimeMs + } + } + return newest +} + +// Same freshness gate as `cjs-require.test.ts`: assert against the CURRENT +// source, never a stale artifact that would pass for the wrong reason. +function distIsFresh(): boolean { + if (!existsSync(distDir)) return false + const tsupConfigMtime = statSync( + path.join(packageRoot, 'tsup.config.ts'), + ).mtimeMs + const distMtime = newestMtime(distDir) + return distMtime >= newestMtime(srcDir) && distMtime >= tsupConfigMtime +} + +if (!distIsFresh()) { + execFileSync('pnpm', ['run', 'build'], { + cwd: packageRoot, + stdio: 'inherit', + }) +} + +const ESM_BUNDLE = path.join(distDir, 'diagnostics.js') +const CJS_BUNDLE = path.join(distDir, 'diagnostics.cjs') + +/** Every bare specifier the bundle imports or requires. */ +function specifiersOf(bundlePath: string): string[] { + const source = readFileSync(bundlePath, 'utf8') + const found = new Set() + for (const [, spec] of source.matchAll( + /(?:\brequire\(\s*|\bfrom\s*|\bimport\s*\(\s*)['"]([^'"]+)['"]/g, + )) { + if (spec) found.add(spec) + } + return [...found] +} + +/** + * A CJS preload that makes every shape of the protect-ffi binding + * unresolvable — the platform packages and the local debug `index.node`, which + * is what `src/load.cts` reaches for. Patching the resolver rather than moving + * files keeps the checkout untouched and works whether or not the developer + * has run `build:native`. + * + * Adapted from `packages/protect-ffi/src/lintWiring.test.ts`, which runs its + * own suite under the same hook. `fs` is patched too: a loader that stats the + * artifact before requiring it would otherwise see a file that `require` then + * refuses to load. + */ +function hideBindingPreload(): string { + const dir = mkdtempSync(path.join(tmpdir(), 'stack-diagnostics-')) + const file = path.join(dir, 'hide-binding.cjs') + writeFileSync( + file, + ` +const fs = require('node:fs') +const Module = require('node:module') + +const BINDING = + /(?:^|[\\\\/])index\\.node$|@cipherstash[\\\\/]protect-ffi-(?:darwin|linux|win32)-/ + +const load = Module._load +Module._load = function (request, parent, isMain) { + if (BINDING.test(request)) { + const error = new Error("Cannot find module '" + request + "'") + error.code = 'MODULE_NOT_FOUND' + throw error + } + return load.call(this, request, parent, isMain) +} + +const existsSync = fs.existsSync +fs.existsSync = (p) => (BINDING.test(String(p)) ? false : existsSync(p)) + +const statSync = fs.statSync +fs.statSync = (p, ...rest) => { + if (!BINDING.test(String(p))) return statSync(p, ...rest) + const error = new Error('ENOENT: no such file or directory, stat ' + p) + error.code = 'ENOENT' + throw error +} + +Module.syncBuiltinESMExports() +`, + ) + return file +} + +/** Thrown by the stand-in's `isEncrypted`, so the call is observable. */ +const PUBLISHED_SURFACE_MARKER = 'reached the published protect-ffi surface' + +/** + * A preload that swaps `@cipherstash/protect-ffi` for a stand-in carrying the + * surface of the version on npm — `isEncrypted`, and no + * `assertNativeBindingAvailable`. + * + * This entry is the one part of the package whose correctness depends on WHICH + * protect-ffi is installed, and `workspace:*` hides that: it resolves to the + * sibling directory here and to whatever version publishing pins there. The + * stand-in's `isEncrypted` throws a marker instead of loading a binding, so the + * assertion is that the call reaches it at all — that the entry probes through + * an export the published package actually has. The tests above cover the other + * half, against the real protect-ffi and a real binding. + */ +function publishedFfiSurfacePreload(): string { + // Realpath'd, and it is load-bearing. `os.tmpdir()` is behind a symlink on + // macOS (`/var` → `/private/var`), and a CJS module reached through one is + // loaded TWICE: the ESM loader keys the module on the symlinked URL it was + // given, while executing a `.cjs` goes through the CJS loader, which + // realpaths. The stand-in's named exports then bind against the instance + // that never ran, so `isEncrypted` arrives `undefined` — a failure that + // looks exactly like the bug under test and is not one. + const dir = realpathSync( + mkdtempSync(path.join(tmpdir(), 'stack-diagnostics-ffi-')), + ) + const standIn = path.join(dir, 'protect-ffi-as-published.cjs') + writeFileSync( + standIn, + `exports.isEncrypted = function isEncrypted() { + throw new Error('${PUBLISHED_SURFACE_MARKER}') +} +exports.encrypt = function encrypt() {} +`, + ) + const hook = path.join(dir, 'substitute-ffi.mjs') + writeFileSync( + hook, + `import { registerHooks } from 'node:module' +import { pathToFileURL } from 'node:url' + +const STAND_IN = pathToFileURL(${JSON.stringify(standIn)}).href + +registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === '@cipherstash/protect-ffi') { + return { url: STAND_IN, shortCircuit: true } + } + return nextResolve(specifier, context) + }, +}) +`, + ) + return `--import "${pathToFileURL(hook).href}"` +} + +/** + * Runs `script` in a child process resolved from this package. A child rather + * than an in-process require: the point is to go through Node's own resolution + * of `@cipherstash/stack/diagnostics` against the published `exports` map, + * which Vitest's aliases would bypass. + */ +function runNode( + script: string, + { esm = false, hideBinding = false, publishedFfi = false } = {}, +): string { + const args = esm ? ['--input-type=module', '-e', script] : ['-e', script] + const preloads = [ + process.env.NODE_OPTIONS, + hideBinding ? `--require "${hideBindingPreload()}"` : undefined, + publishedFfi ? publishedFfiSurfacePreload() : undefined, + ].filter(Boolean) + return execFileSync(process.execPath, args, { + cwd: packageRoot, + encoding: 'utf8', + stdio: 'pipe', + env: + preloads.length > 0 + ? { ...process.env, NODE_OPTIONS: preloads.join(' ') } + : process.env, + }) +} + +describe('@cipherstash/stack/diagnostics', () => { + it('reaches protect-ffi and nothing else', () => { + for (const bundle of [ESM_BUNDLE, CJS_BUNDLE]) { + const specifiers = specifiersOf(bundle) + expect(specifiers, path.basename(bundle)).toContain( + '@cipherstash/protect-ffi', + ) + // Not `.not.toContain('@cipherstash/auth')` — the failure mode is a + // SHARED CHUNK that imports auth, whose own specifier is a relative + // `./chunk-XXXX.js`. Anything beyond the one bare specifier is the + // symptom, whatever its name. + expect(specifiers, path.basename(bundle)).toEqual([ + '@cipherstash/protect-ffi', + ]) + } + }) + + // `@cipherstash/protect-ffi` is a `workspace:*` dependency, so this entry is + // built against the sibling directory and shipped against whatever version + // publishing pins — today 0.31.0, which is on npm WITHOUT + // `assertNativeBindingAvailable`: that export arrived with the lazy load, + // whose changeset is parked as `.deferred` until the publishing cutover, so + // no version carrying it exists. Re-exporting it would ship an entry that + // dies on import under ESM ("Named export … not found", a link-time + // SyntaxError) and hands back `undefined` under CJS — and `stash doctor` + // classifies neither, so it would print a bare `Fatal error` for every user + // on a released install. + it.each([ + ['require', false], + ['import', true], + ])('probes through an export the published protect-ffi has, so %s survives it', (_label, esm) => { + const call = `let raised = 'none' + try { d.assertNativeBindingAvailable() } catch (e) { raised = e.message } + process.stdout.write('imported ' + raised)` + const script = esm + ? `const d = await import('@cipherstash/stack/diagnostics')\n${call}` + : `const d = require('@cipherstash/stack/diagnostics')\n${call}` + + const output = runNode(script, { esm, publishedFfi: true }) + + // Reaching stdout at all means the entry loaded against that surface. + expect(output).toMatch(/^imported /) + // And that the probe ran, rather than resolving to `undefined` and + // throwing a TypeError the CLI would report as an unknown failure. + expect(output).toContain(PUBLISHED_SURFACE_MARKER) + }) + + it('is importable from both module systems', () => { + expect( + runNode( + `const d = require('@cipherstash/stack/diagnostics') + if (typeof d.assertNativeBindingAvailable !== 'function') throw new Error('missing export') + process.stdout.write('ok')`, + ), + ).toBe('ok') + + expect( + runNode( + `const d = await import('@cipherstash/stack/diagnostics') + if (typeof d.assertNativeBindingAvailable !== 'function') throw new Error('missing export') + process.stdout.write('ok')`, + { esm: true }, + ), + ).toBe('ok') + }) + + // The reason the entry exists. If importing it were enough, `doctor` would + // not need this module at all — and if importing it FORCED the load, this + // package would have re-broken the laziness that `b99cbd92` bought. + it.each([ + ['require', false], + ['import', true], + ])('with no binding installed, %s succeeds and only the call throws', (_label, esm) => { + const script = esm + ? `const d = await import('@cipherstash/stack/diagnostics') + let raised = 'none' + try { d.assertNativeBindingAvailable() } catch (e) { raised = e.code + ' ' + e.message.split('\\n')[0] } + process.stdout.write('imported ' + raised)` + : `const d = require('@cipherstash/stack/diagnostics') + let raised = 'none' + try { d.assertNativeBindingAvailable() } catch (e) { raised = e.code + ' ' + e.message.split('\\n')[0] } + process.stdout.write('imported ' + raised)` + + const output = runNode(script, { esm, hideBinding: true }) + + // "imported" at all means the import did not throw — the property under + // test. A child that died during import produces no stdout and fails the + // execFileSync above. + expect(output).toMatch(/^imported /) + // Unwrapped: same `code` and the platform package by name, which is what + // `packages/cli/src/native.ts` classifies on. + expect(output).toContain('MODULE_NOT_FOUND') + expect(output).toMatch( + /Cannot find module '@cipherstash\/protect-ffi-(darwin|linux|win32)-/, + ) + }) +}) diff --git a/packages/stack/__tests__/subpath-types-parity.test.ts b/packages/stack/__tests__/subpath-types-parity.test.ts new file mode 100644 index 000000000..5c548961d --- /dev/null +++ b/packages/stack/__tests__/subpath-types-parity.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from 'vitest' +import packageJson from '../package.json' + +// `exports` and `typesVersions` describe the same subpaths to two different +// resolvers, and only one of them is exercised by anything in this repo. +// +// `exports` carries the types for `moduleResolution: "node16"`/`"bundler"`, +// which is what every consumer here uses. A consumer on classic node10 +// resolution — still the default for `moduleResolution` when `module` is +// `commonjs`, so not a rarity — reads `typesVersions` instead, and a subpath +// missing from it resolves to `any` with no error at the import site. Nothing +// type-checks that: the package builds, publishes, and installs clean. +// +// So the guard is parity, asserted on the manifest. Adding a subpath means +// adding both, and the failure here names the one that was forgotten. +describe('subpath exports', () => { + it('declare types for node10 resolution as well as node16', () => { + const typesVersions = packageJson.typesVersions['*'] as Record< + string, + string[] + > + + // Keyed on what `exports` publishes, so a `typesVersions` entry left behind + // by a REMOVED subpath is not the failure — it is dead weight, not a + // consumer-visible gap, and pinning it here would make this test fail for + // something it is not about. + const declared: Record = {} + const resolvable: Record = {} + for (const [subpath, target] of Object.entries(packageJson.exports)) { + if (subpath === '.' || subpath === './package.json') continue + const key = subpath.slice(2) + const types = (target as { import?: { types?: string } }).import?.types + if (types === undefined) continue + declared[key] = types + resolvable[key] = typesVersions[key]?.[0] ?? '(no typesVersions entry)' + } + + // Compared as whole objects: the diff names every subpath at once, rather + // than failing on the first and hiding the rest. + expect(resolvable).toEqual(declared) + }) +}) diff --git a/packages/stack/package.json b/packages/stack/package.json index 17de66bc0..233c4dd2b 100644 --- a/packages/stack/package.json +++ b/packages/stack/package.json @@ -65,6 +65,9 @@ ], "wasm-inline": [ "./dist/wasm-inline.d.ts" + ], + "diagnostics": [ + "./dist/diagnostics.d.ts" ] } }, @@ -175,6 +178,16 @@ "default": "./dist/wasm-inline.js" } }, + "./diagnostics": { + "import": { + "types": "./dist/diagnostics.d.ts", + "default": "./dist/diagnostics.js" + }, + "require": { + "types": "./dist/diagnostics.d.cts", + "default": "./dist/diagnostics.cjs" + } + }, "./package.json": "./package.json" }, "scripts": { diff --git a/packages/stack/src/diagnostics.ts b/packages/stack/src/diagnostics.ts new file mode 100644 index 000000000..fd1bec92f --- /dev/null +++ b/packages/stack/src/diagnostics.ts @@ -0,0 +1,43 @@ +import { isEncrypted } from '@cipherstash/protect-ffi' + +/** + * Install diagnostics. One export, and it exists so `stash doctor` can prove + * the native binding is present. + * + * **Why a subpath and not the root entry.** Since the protect-ffi native load + * became lazy, importing that package proves nothing: `@neon-rs/load`'s proxy + * resolves the platform binary on first property access, inside a wrapper body, + * so a missing binary imports cleanly and fails later at the first encrypt. + * Nor can a caller force it from outside — `@cipherstash/protect-ffi/lib/load.cjs` + * is not in that package's `exports` (`ERR_PACKAGE_PATH_NOT_EXPORTED`), and + * touching an export never reaches the proxy because the exports are + * protect-ffi's own wrapper functions. CALLING one is what reaches it. + * + * **Why not `@cipherstash/stack` itself.** The root entry re-exports the auth + * strategies, which means evaluating it reads a property off + * `@cipherstash/auth` — a NAPI module whose entry is `module.exports = { + * ...native }`, eager on both counts. So a probe that imports the root entry + * measures AUTH's binary and reports it as the encryption engine's. This module + * imports protect-ffi and nothing else, so the signal belongs to the package + * named in the row. + * + * **Why `isEncrypted` and not protect-ffi's own `assertNativeBindingAvailable`, + * which is this function's twin.** That export does not exist in any version of + * protect-ffi on npm. It arrived with the lazy load, whose changeset is parked + * as `.deferred` until the publishing cutover, so no released version carries + * it — and `@cipherstash/stack` depends on `workspace:*`, which resolves to the + * sibling directory here and to a published version for everyone else. A + * re-export would therefore work in this repo and fail everywhere it shipped: + * under ESM at import, with a link-time SyntaxError, and under CJS as an + * `undefined` that is not a function. `isEncrypted` has been published since + * 0.28.0 and is the call protect-ffi's own assert makes, for the same reason — + * it is pure, synchronous, and validates nothing before reaching the addon, so + * whatever it raises came from the loader. + * + * Kept pure deliberately: no side effects at module scope, and the loader's + * error is not caught, wrapped or re-thrown anywhere on the path — a caller + * classifying `MODULE_NOT_FOUND` sees exactly what the loader raised. + */ +export function assertNativeBindingAvailable(): void { + isEncrypted(null) +} diff --git a/packages/stack/tsup.config.ts b/packages/stack/tsup.config.ts index a96c010c4..c00987893 100644 --- a/packages/stack/tsup.config.ts +++ b/packages/stack/tsup.config.ts @@ -1,9 +1,9 @@ import { defineConfig } from 'tsup' -// Two configs run in parallel inside tsup. They share the same `dist/` -// output dir, so neither uses `clean: true` — a parallel-run race could -// otherwise wipe the other config's output. The pre-tsup `rimraf dist` -// in `package.json`'s build script clears the dir once before either +// Three configs run in parallel inside tsup. They share the same `dist/` +// output dir, so none uses `clean: true` — a parallel-run race could +// otherwise wipe another config's output. The pre-tsup `rimraf dist` +// in `package.json`'s build script clears the dir once before any // starts. export default defineConfig([ // Main entries — dual ESM + CJS bundles. @@ -49,4 +49,25 @@ export default defineConfig([ tsconfig: './tsconfig.json', noExternal: ['evlog', 'uuid', 'zod', '@byteslice/result'], }, + // Diagnostics entry — its own config rather than another `entry` in the + // main one, because what it must NOT reach is the whole point of it. + // `splitting` is on for ESM in the main config, and a shared chunk is + // exactly how `@cipherstash/auth` would arrive here: the probe would then + // fail on auth's binary while reporting the encryption engine, which is the + // bug this entry exists to fix. `splitting: false` makes that structural + // rather than a property of today's module graph. + // + // Nothing is bundled in (no `noExternal`): `@cipherstash/protect-ffi` has to + // stay a bare specifier so the load goes through the installed package's own + // loader, which is the thing under test. + { + entry: { diagnostics: 'src/diagnostics.ts' }, + format: ['cjs', 'esm'], + splitting: false, + sourcemap: true, + dts: { entry: { diagnostics: 'src/diagnostics.ts' } }, + clean: false, + target: 'es2022', + tsconfig: './tsconfig.json', + }, ]) diff --git a/skills/stash-encryption/SKILL.md b/skills/stash-encryption/SKILL.md index fa006decd..e3a742752 100644 --- a/skills/stash-encryption/SKILL.md +++ b/skills/stash-encryption/SKILL.md @@ -195,6 +195,7 @@ The SDK never logs plaintext data. | `@cipherstash/stack/schema` | Low-level encrypt-config types and validation helpers; it is not a schema-authoring DSL | | `@cipherstash/stack/encryption` | The `Encryption` factory and the chainable operation classes its methods return (`EncryptOperation`, `DecryptOperation`, `EncryptQueryOperation`, `BulkEncryptModelsOperation`, …). Import these only to *name* an operation's type; author schemas and build the client from `@cipherstash/stack/v3` | | `@cipherstash/stack/adapter-kit` | The internal seam for the **first-party** adapter packages (`@cipherstash/stack-drizzle`, `@cipherstash/stack-supabase`). Not a general-purpose public API — anything an end user needs has a dedicated subpath above. Do not import it in application code | +| `@cipherstash/stack/diagnostics` | One export, `assertNativeBindingAvailable()`, for **tooling** that needs to prove the protect-ffi native binding is installed — this is what `stash doctor` calls. Importing it forces nothing; calling it forces the platform binary to load and throws the loader's own error, naming the missing `@cipherstash/protect-ffi--` package, if it is absent. Not part of the encryption API and not needed in application code | ## Schema Definition