From 659ef5a62b9942fccd9cda0de9c75d7de2ee9b42 Mon Sep 17 00:00:00 2001 From: alpha nerd Date: Sun, 19 Jul 2026 12:59:24 +0200 Subject: [PATCH] fix: repair platform resolution in the built bundles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The published package could not be used at all. `require('nomyo-js')` succeeded, but constructing a client threw: Cannot find module './node' Platform selection was done with a runtime require: const NodeSecureMemory = require('./node').NodeSecureMemory; Rollup flattens every module into one file, so './node' and './browser' no longer exist at runtime — and because the calls sit inside function bodies, rollup left them as literal runtime requires rather than resolving them. The failure was therefore deferred to first use: module load and Object.keys() both looked fine, so nothing noticed. The SecureCompletionClient constructor calls createSecureMemory() and createHttpClient(), which made every client unconstructable. Confirmed present at 057ff6c, this branch's merge base, so the npm package has never worked. Platform implementations are now injected by the entry points, which is what src/node.ts and src/browser.ts always claimed to do (they merely re-exported ./index). createSecureMemory/createHttpClient consult a registered factory and throw a directive error if none was registered. No require fallback is kept: leaving one would put an unresolvable relative require back in the bundle, and bundlers resolve requires statically, so webpack/vite would fail on a path that does not exist in dist/. Jest registers the platform via tests/setup.ts instead. This also keeps the Node HTTP client and the optional native addon out of the browser bundle, which previously carried both. Second defect found while verifying: dist/esm/index.mjs contained 13 require() calls (crypto, fs, path, jose, nomyo-native) that the source loads lazily. `require` does not exist in ES module scope, so an ESM consumer crashed with "require is not defined" as soon as one ran — using keyDir for key persistence would have hit it on every Node version. Node 24 masked the crypto case by having a global crypto. The ESM output now carries a createRequire shim. tests/integration/bundle.test.ts covers the artefact that actually ships: both bundles construct a client, expose the API, resolve the platform layer, keep Node-only modules out of the browser build, and the ESM entry is imported and used by a real spawned Node process. Every other suite runs against src/ through ts-jest, where these paths resolve normally — which is precisely why this went unnoticed. Verified end to end by installing the packed tarball into a clean project: CommonJS and ESM both construct a client and run fs-backed key generation on Node 18.19.1 and 24.18.0. Co-Authored-By: Claude Opus 4.8 --- jest.config.js | 1 + rollup.config.mjs | 10 +- src/browser.ts | 16 ++- src/core/http/client.ts | 31 ++++-- src/core/memory/secure.ts | 33 +++++-- src/node.ts | 16 ++- tests/integration/bundle.test.ts | 164 +++++++++++++++++++++++++++++++ tests/setup.ts | 14 +++ 8 files changed, 262 insertions(+), 23 deletions(-) create mode 100644 tests/integration/bundle.test.ts create mode 100644 tests/setup.ts diff --git a/jest.config.js b/jest.config.js index dbf98bf..f8e34a7 100644 --- a/jest.config.js +++ b/jest.config.js @@ -3,6 +3,7 @@ module.exports = { preset: 'ts-jest', testEnvironment: 'node', testMatch: ['**/tests/unit/**/*.test.ts', '**/tests/integration/**/*.test.ts'], + setupFiles: ['/tests/setup.ts'], transform: { '^.+\\.tsx?$': ['ts-jest', { tsconfig: { diff --git a/rollup.config.mjs b/rollup.config.mjs index 1e73602..a55e785 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -30,7 +30,15 @@ if (target === 'node') { }, { file: 'dist/esm/index.mjs', - format: 'es' + format: 'es', + // The source loads several modules lazily via require(): the crypto + // fallback, fs/path for key persistence, the optional native addon, + // and jose. `require` does not exist in ES module scope, so without + // this shim an ESM consumer crashes with "require is not defined" + // the moment one of those paths runs. Node-only output, so + // node:module is safe here. + banner: "import { createRequire as __nomyoCreateRequire } from 'node:module';\n" + + 'const require = __nomyoCreateRequire(import.meta.url);' } ]; config.external = ['crypto', 'https', 'fs', 'path']; diff --git a/src/browser.ts b/src/browser.ts index a79884c..dec271c 100644 --- a/src/browser.ts +++ b/src/browser.ts @@ -1,6 +1,18 @@ /** - * Browser-specific entry point - * Ensures browser-specific implementations are used + * Browser-specific entry point. + * + * Registers the browser platform implementations before anything can ask for + * them, so the bundled build never needs a runtime path lookup. Wiring them in + * here also keeps the Node.js implementations — and their `https`/`fs` imports + * and the optional native addon — out of the browser bundle entirely. */ +import { registerSecureMemory } from './core/memory/secure'; +import { registerHttpClient } from './core/http/client'; +import { BrowserSecureMemory } from './core/memory/browser'; +import { BrowserHttpClient } from './core/http/browser'; + +registerSecureMemory(() => new BrowserSecureMemory()); +registerHttpClient(() => new BrowserHttpClient()); + export * from './index'; diff --git a/src/core/http/client.ts b/src/core/http/client.ts index f2ad6c4..1f8c076 100644 --- a/src/core/http/client.ts +++ b/src/core/http/client.ts @@ -20,17 +20,30 @@ export interface HttpClient { get(url: string, options?: Omit): Promise; } +export type HttpClientFactory = () => HttpClient; + +let httpClientFactory: HttpClientFactory | null = null; + /** - * Create an HTTP client for the current platform + * Register the platform's HttpClient implementation. + * + * Called by the entry points (src/node.ts, src/browser.ts) at load time — see + * registerSecureMemory for why a runtime require cannot survive bundling. + */ +export function registerHttpClient(factory: HttpClientFactory): void { + httpClientFactory = factory; +} + +/** + * Create an HTTP client for the current platform. */ export function createHttpClient(): HttpClient { - if (typeof window !== 'undefined') { - // Browser environment - const BrowserHttpClient = require('./browser').BrowserHttpClient; - return new BrowserHttpClient(); - } else { - // Node.js environment - const NodeHttpClient = require('./node').NodeHttpClient; - return new NodeHttpClient(); + if (httpClientFactory === null) { + throw new Error( + 'No HttpClient implementation registered. Import the package entry ' + + "point ('nomyo-js', or src/node.ts / src/browser.ts) rather than " + + 'deep-importing core modules.' + ); } + return httpClientFactory(); } diff --git a/src/core/memory/secure.ts b/src/core/memory/secure.ts index 458c3d0..ea278f1 100644 --- a/src/core/memory/secure.ts +++ b/src/core/memory/secure.ts @@ -144,17 +144,32 @@ export class SecureByteContext { } } +export type SecureMemoryFactory = () => SecureMemory; + +let secureMemoryFactory: SecureMemoryFactory | null = null; + /** - * Create a secure memory implementation for the current platform + * Register the platform's SecureMemory implementation. + * + * The entry points (src/node.ts, src/browser.ts) call this at load time. That + * matters for the bundled builds: rollup flattens every module into one file, + * so requiring the platform module by relative path at runtime would point at a + * path that no longer exists, and throw the moment platform code was needed. + */ +export function registerSecureMemory(factory: SecureMemoryFactory): void { + secureMemoryFactory = factory; +} + +/** + * Create a secure memory implementation for the current platform. */ export function createSecureMemory(): SecureMemory { - if (typeof window !== 'undefined') { - // Browser environment - const BrowserSecureMemory = require('./browser').BrowserSecureMemory; - return new BrowserSecureMemory(); - } else { - // Node.js environment - const NodeSecureMemory = require('./node').NodeSecureMemory; - return new NodeSecureMemory(); + if (secureMemoryFactory === null) { + throw new Error( + 'No SecureMemory implementation registered. Import the package entry ' + + "point ('nomyo-js', or src/node.ts / src/browser.ts) rather than " + + 'deep-importing core modules.' + ); } + return secureMemoryFactory(); } diff --git a/src/node.ts b/src/node.ts index e907955..5b9fe6d 100644 --- a/src/node.ts +++ b/src/node.ts @@ -1,6 +1,18 @@ /** - * Node.js-specific entry point - * Ensures Node.js-specific implementations are used + * Node.js-specific entry point. + * + * Registers the Node.js platform implementations before anything can ask for + * them. This is what makes the bundled build work: rollup flattens all modules + * into a single file, so the platform layer cannot be discovered at runtime by + * relative path — it has to be wired in here, statically. */ +import { registerSecureMemory } from './core/memory/secure'; +import { registerHttpClient } from './core/http/client'; +import { NodeSecureMemory } from './core/memory/node'; +import { NodeHttpClient } from './core/http/node'; + +registerSecureMemory(() => new NodeSecureMemory()); +registerHttpClient(() => new NodeHttpClient()); + export * from './index'; diff --git a/tests/integration/bundle.test.ts b/tests/integration/bundle.test.ts new file mode 100644 index 0000000..ee16170 --- /dev/null +++ b/tests/integration/bundle.test.ts @@ -0,0 +1,164 @@ +/** + * Smoke tests for the BUILT bundles in dist/. + * + * Every other suite runs against src/ through ts-jest, where the platform + * modules resolve as ordinary relative paths. That masked a bug in which the + * published package loaded fine but threw "Cannot find module './node'" as soon + * as anything touched the platform layer — rollup flattens the modules, so the + * runtime require had nothing to resolve. The client could not be constructed + * at all from the npm package. + * + * These tests exercise the artefact that actually ships. Run `npm run build` + * first; they skip themselves if dist/ is absent. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import { spawnSync } from 'child_process'; + +const DIST = path.join(__dirname, '..', '..', 'dist'); +const NODE_CJS = path.join(DIST, 'node', 'index.js'); +const BROWSER_CJS = path.join(DIST, 'browser', 'index.cjs'); +const ESM_ENTRY = path.join(DIST, 'esm', 'index.mjs'); + +const built = fs.existsSync(NODE_CJS); +const describeIfBuilt = built ? describe : describe.skip; + +if (!built) { + // eslint-disable-next-line no-console + console.warn('dist/ not found — run `npm run build` to exercise the bundle smoke tests'); +} + +interface NomyoModule { + SecureChatCompletion: new (config?: object) => { dispose(): void }; + SecureCompletionClient: new (config: object) => { dispose(): void }; + getMemoryProtectionInfo: () => { method: string; canLock: boolean }; + AttestationPolicy: new (options?: object) => { enforce: boolean }; + SecurityError: new (message: string) => Error; +} + +describeIfBuilt('built Node bundle', () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const nomyo = require(NODE_CJS) as NomyoModule; + + test('exposes the public API', () => { + expect(typeof nomyo.SecureChatCompletion).toBe('function'); + expect(typeof nomyo.SecureCompletionClient).toBe('function'); + expect(typeof nomyo.getMemoryProtectionInfo).toBe('function'); + expect(typeof nomyo.AttestationPolicy).toBe('function'); + }); + + test('constructs a client — the platform layer resolves', () => { + // This is the exact call that threw "Cannot find module './node'" + const client = new nomyo.SecureChatCompletion({}); + expect(client).toBeTruthy(); + client.dispose(); + }); + + test('constructs the low-level client too', () => { + const client = new nomyo.SecureCompletionClient({ + routerUrl: 'https://router.test', + keyRotationInterval: 0, + }); + expect(client).toBeTruthy(); + client.dispose(); + }); + + test('reaches the real secure-memory implementation', () => { + const info = nomyo.getMemoryProtectionInfo(); + expect(['mlock', 'zero-only', 'none']).toContain(info.method); + expect(typeof info.canLock).toBe('boolean'); + }); + + test('carries no unresolved runtime requires', () => { + const source = fs.readFileSync(NODE_CJS, 'utf-8'); + // './node' / './browser' only ever resolved pre-bundling + expect(source).not.toMatch(/require\(['"]\.\/node['"]\)/); + expect(source).not.toMatch(/require\(['"]\.\/browser['"]\)/); + }); + + test('attestation policy survives bundling', () => { + const policy = new nomyo.AttestationPolicy({ enforce: true }); + expect(policy.enforce).toBe(true); + }); +}); + +describeIfBuilt('built browser bundle', () => { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const nomyo = require(BROWSER_CJS) as NomyoModule; + + test('exposes the public API', () => { + expect(typeof nomyo.SecureChatCompletion).toBe('function'); + expect(typeof nomyo.getMemoryProtectionInfo).toBe('function'); + }); + + test('constructs a client without a DOM', () => { + const client = new nomyo.SecureChatCompletion({}); + expect(client).toBeTruthy(); + client.dispose(); + }); + + test('reports browser memory protection', () => { + const info = nomyo.getMemoryProtectionInfo(); + expect(info.canLock).toBe(false); + expect(info.method).toBe('zero-only'); + }); + + test('carries no unresolved runtime requires', () => { + const source = fs.readFileSync(BROWSER_CJS, 'utf-8'); + expect(source).not.toMatch(/require\(['"]\.\/node['"]\)/); + expect(source).not.toMatch(/require\(['"]\.\/browser['"]\)/); + }); + + test('does not drag the Node platform layer into the browser build', () => { + const source = fs.readFileSync(BROWSER_CJS, 'utf-8'); + // The Node HTTP client and the optional native addon must not appear. + // `fs`/`path` legitimately do: KeyManager has Node-only branches guarded + // by `typeof window`, shared by both builds. + for (const nodeOnly of ['https', 'http', 'url', 'nomyo-native', 'node-gyp-build']) { + expect(source).not.toMatch( + new RegExp(`require\\(['"]${nodeOnly}['"]\\)`) + ); + } + }); +}); + +describeIfBuilt('built ESM entry point', () => { + test('is real ESM with an unambiguous extension', () => { + // A .js ES module in a non-"type":"module" package is parsed as + // CommonJS by Node and fails on older versions + expect(fs.existsSync(ESM_ENTRY)).toBe(true); + expect(fs.existsSync(path.join(DIST, 'esm', 'index.js'))).toBe(false); + expect(fs.readFileSync(ESM_ENTRY, 'utf-8')).toMatch(/^export |[\n;]export /); + }); + + test('shims require() for its lazily-loaded modules', () => { + // The source loads crypto/fs/path/jose/the native addon via require(). + // `require` is not defined in ES module scope, so the bundle must + // provide it or those paths throw ReferenceError at runtime. + const source = fs.readFileSync(ESM_ENTRY, 'utf-8'); + if (/[^.\w]require\(/.test(source)) { + expect(source).toMatch(/createRequire/); + } + }); + + test('is importable and usable by a real Node process', () => { + // Not `await import(...)` — ts-jest compiles dynamic import down to + // require() for its CommonJS target, which cannot load .mjs and would + // test the compiler rather than the bundle. Spawn Node instead. + const script = `import(${JSON.stringify(ESM_ENTRY)})` + + '.then(m => {' + + ' const c = new m.SecureChatCompletion({}); c.dispose();' + // Touches the require()-dependent crypto path + + ' if (typeof m.getMemoryProtectionInfo().method !== "string") process.exit(2);' + + '})' + + '.catch(e => { console.error(e.message); process.exit(3); })'; + + const result = spawnSync(process.execPath, ['--input-type=module', '-e', script], { + encoding: 'utf-8', + }); + + expect(result.stderr.trim()).toBe(''); + expect(result.status).toBe(0); + }); +}); diff --git a/tests/setup.ts b/tests/setup.ts new file mode 100644 index 0000000..d6e66dc --- /dev/null +++ b/tests/setup.ts @@ -0,0 +1,14 @@ +/** + * Jest setup: register the Node.js platform implementations. + * + * The unit suites import core modules directly rather than going through the + * package entry point, so nothing would otherwise call registerSecureMemory / + * registerHttpClient. Importing src/node.ts performs that registration exactly + * as the published Node bundle does. + * + * Doing this here rather than falling back to a runtime `require('./node')` + * inside the core modules keeps that require out of the built bundles, where + * the relative path does not exist and bundlers would fail to resolve it. + */ + +import '../src/node';