feat(auth): use OS keyring in bundle distributions#1197
Conversation
Bundle binaries (bun --compile) silently fell back to plaintext file storage because @napi-rs/keyring's createRequire-based platform loader isn't followed by --compile, so no native .node was embedded. Embed exactly one platform subpackage per bundle instead: - build-cli-bundles.ts keeps a placeholder keyring import external in the fat-JS step so the literal import() survives, then rewrites it per target to @napi-rs/keyring-<platform> so --compile resolves and embeds that one .node. The rewrite/write runs once per subpackage, not per target (baseline variants share their sibling's subpackage). - credentials.ts imports that placeholder in bundle mode (useCLIMetadata().installMethod === 'bundle') and the @napi-rs/keyring wrapper otherwise; all fallback/migration behavior is unchanged. - package.json pins pnpm.supportedArchitectures so every target's native subpackage is installed at build time (affects this repo's installs only, never npm consumers or the shipped bundle). - login.ts drops the now-obsolete "install via npm for keyring" hint. Closes #1170 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| function keyringSubpackage(os: string, arch: string, musl: boolean): string { | ||
| switch (os) { | ||
| case 'linux': | ||
| return `@napi-rs/keyring-linux-${arch}-${musl ? 'musl' : 'gnu'}`; | ||
| case 'darwin': | ||
| return `@napi-rs/keyring-darwin-${arch}`; | ||
| case 'windows': | ||
| return `@napi-rs/keyring-win32-${arch}-msvc`; | ||
| default: | ||
| throw new Error(`No @napi-rs/keyring subpackage known for ${os}-${arch}`); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This is the main hack, --compile (bun) skipped napi-rs due to its dynamic require shenanigans -> we provide literal for specific bundle which is shipped as external subpackage (per arch).
Move the keyring cross-platform target config out of package.json's pnpm field into pnpm-workspace.yaml, alongside the other pnpm settings (allowBuilds, overrides, nodeLinker). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Testing bundles in progress, but feel free to review. |
|
I did review using In build-cli-bundles.ts, the keyring rewrite was inserted after the Windows ARM64 special case that overrides arch = 'arm64'. CI has a windows-11-arm matrix job, so this path is live. Those bundles are still compiled with target: 'bun-windows-x64' — the output is an x64 PE that runs under x64 emulation on ARM64 Windows. The rewrite, however, computes keyringSubpackage('windows', 'arm64', …) → @napi-rs/keyring-win32-arm64-msvc, a plain ARM64 DLL. An x64 process (even emulated on ARM64 Windows) can only load x64 or ARM64EC DLLs, so LoadLibrary will fail, the dynamic import throws, and the code falls back to file storage — recreating the exact plaintext-fallback bug this PR fixes, for Windows ARM users only. Fix: capture the target's declared arch before the override (or move the rewrite above the override block) so these bundles embed win32-x64-msvc, which loads fine under emulation. It degrades gracefully today, so this isn't a blocker, but it's a two-line fix and otherwise the issue will be invisible until someone debugs it again. |
|
Ocne we merge my PR for unified CLI bundles the arm64 Windows remark won't matter anyways, so lets deal with that first |
Closes #1170.
Problem
The OS-keyring storage from #1148 works for
npm install -g apify-clibut not for the bundle distributions (install-cli.sh/install-cli.ps1). Those bundles are built with Bun's--compile. The@napi-rs/keyringwrapper resolves its native.nodeat runtime viacreateRequire(__filename)('@napi-rs/keyring-<platform>'), and--compiledoes not follow that indirection — so no native module was embedded and bundle users silently fell back to plaintext file storage (secretsBackend: "file", token in cleartext) even with a working keyring present.Approach — embed one platform subpackage per bundle
The key constraint (per Bun's docs: "the
.nodefile must be directly required or it won't bundle correctly") is that the native module must be referenced by a literal specifier that survives to the--compilestep. Verified empirically: only a literal dynamicimport('@napi-rs/keyring-<platform>')embeds and runs; static named/default imports either fail to bundle or crash with__require is not definedin ESM output, andcreateRequireisn't bundled at all.scripts/build-cli-bundles.ts— a placeholder keyring specifier is keptexternalin the fat-JS step so the literalimport()survives, then rewritten per target to@napi-rs/keyring-<platform>before--compileresolves and embeds that one.node. The rewrite/write runs once per subpackage rather than per target (baseline variants share their sibling's subpackage).src/lib/credentials.ts— imports that placeholder in bundle mode (useCLIMetadata().installMethod === 'bundle', the codebase's standard bundle signal) and the@napi-rs/keyringwrapper otherwise. All fallback/migration behavior is unchanged.package.json—pnpm.supportedArchitecturesso every target's native subpackage is installed at build time. Scope: affects onlypnpm installinside this repo (contributors/CI); npm consumers and the shipped bundle are unaffected (npm ignores thepnpmfield; bundle users get a single embedded.node).src/lib/typings/keyring-native-subpackage.d.ts— ambient declaration so tsc/oxlint accept the placeholder specifier.src/commands/auth/login.ts— drops the now-obsolete "install via npm for keyring" hint.Verification
All four acceptance criteria verified end-to-end by compiling the real
credentials.tsinto abun-linux-arm64bundle and running it against a real D-Bus + gnome-keyring Secret Service:auth.jsonmigrates →secretsBackend: "keyring",tokenremoved, proxy password stripped.com.apify.clifrom a separate process (secret-tool search).secretsBackend: "file", no crash.lint,format,build,tscall green.test:local: 304 passed; the only 2 failures are pre-existing Python actor-run fixtures (no Python runtime in CI sandbox), unrelated to this change.Note for reviewers
Criteria 1 & 2 were validated in a hand-rolled keyring environment; a sanity check on a real macOS/Windows/Linux desktop with
pnpm run build-bundlesis worth doing before release.🤖 Generated with Claude Code