fix: auth/CLI hardening v0.8.43 — closes #5, closes #6#7
Merged
Conversation
`import.meta.url === pathToFileURL(process.argv[1]).href` silently returned false when the CLI was invoked through a symlink (npm .bin, Homebrew Cellar, etc.), causing the CLI to exit 0 with no output. Extract a shared `isMainModule()` helper that resolves both sides through `realpathSync` before comparing, with a graceful fallback to the old behavior if realpath fails. Files: cli.js, index.ts, is-main-module.js(+d.ts)
npm's bin linker creates symlinks (not wrappers) on POSIX, so the kernel needs a shebang in `dist/cli.mjs` for direct `./dist/cli.mjs` execution to work. Add `scripts/post-build-shebang.mjs` that prepends `#!/usr/bin/env node` and chmods to 755, wired into the `build` script after tsup. Update the tsup.config.ts comment to document the new pipeline. Refs issue #6
…ape hatch
macOS users reported repeated Keychain permission prompts because four
independent code paths each did an uncached `import("keytar")` call.
- vault.js: Cache the keytar module load (both success and failure) in
`_keytarModuleCache` per-process. Add `isKeychainDisabled()` helper
that respects `PERPLEXITY_DISABLE_KEYCHAIN=1`.
- Export `probeKeychainState()` from vault.js so cli.js and
checks/vault.js can share the cached probe instead of doing their own
inline imports.
- extension/src/auth/vault-passphrase.ts: Check the env var before
spawning the keytar probe child process, and also inside the child
inline code.
Fixes issue #6 bug 3.
The manual and auto login runners used `chromium.launch()` + `browser.newContext()`, which created a fresh ephemeral context every run. Any Cloudflare clearance cookies acquired during the login flow were lost on browser close, forcing the user through CF again on the next login. Switch both runners to `chromium.launchPersistentContext()` using a dedicated `login-browser-data/` directory under the profile (separate from the daemon's `browser-data/` to avoid singleton-lock collisions). Add `loginBrowserData` path to `getProfilePaths()` in profiles.js. Fixes issue #6 bug 4.
getSavedCookies returned `[]` silently in three situations: 1. vault.enc missing for the resolved profile 2. vault.enc exists but the `cookies` key is absent 3. JSON parse failure of the stored cookies Without log lines, users and support can't distinguish scenario 1 (run login first) from scenario 2 (data corruption or wrong profile). Log each empty path with the profile name and the specific cause so that daemon logs and extension output channels show the real reason.
Issue #5 root cause: `PERPLEXITY_HEADLESS_ONLY=1` leaked from the IDE config env block into the daemon process. When the daemon spawned, it inherited `...process.env`, so `PerplexityClient.init()` saw the flag and skipped the headed bootstrap entirely — authenticated Pro features remained unavailable. Changes: - stdio-daemon-proxy.ts: remove `PERPLEXITY_HEADLESS_ONLY` from the auto-generated env block (new configs won't carry it). - launcher.ts spawnDetachedDaemon: delete both `PERPLEXITY_HEADLESS_ONLY` and `PERPLEXITY_NO_DAEMON` from the env before spawning. - runtime.ts spawnBundledDaemon: same stripping. This also fixes pre-existing IDE configs that still have the flag: the spawn-time strip removes it regardless of config age. Refs issue #5
…njection Phase 1 (headed bootstrap) wrote fresh cf_clearance to disk via `launchPersistentContext(browserData)`, but Phase 2 used `chromium.launch()` + `getOrCreateContext()` — a non-persistent context — so the fresh clearance was never loaded. Only stale vault cookies were injected. Fix: Phase 2 now uses `launchPersistentContext(activePaths.browserData)` so it inherits the disk cookies from Phase 1 automatically. To avoid overwriting the fresh cf_clearance with the stale vault copy, inject vault cookies ONLY for cookie names not already present on disk. Remove now-unused `getOrCreateContext` import from client.ts. Refs issue #5
- Add is-main-module.test.js covering symlink resolution (issue #6.1) - Add vault.test.js cases for probeKeychainState caching and PERPLEXITY_DISABLE_KEYCHAIN=1 escape hatch (issue #6.3) - Add config-getSavedCookies.test.js for diagnostic logging (issue #5.3) - Update stdio-daemon-proxy.test.ts to expect empty env (issue #5.1) - Update auto-config.test.ts to expect empty env for OpenCode (issue #5.1) - Fix checks/vault.js to import probeKeychainState from vault.js - Harden tryKeytar() against vitest mock proxies: validate getPassword is a function inside the try-catch so proxy-access errors are caught. Refs issues #5, #6
…llowJs --emitDeclarationOnly The 47-entry-point mcp-server bundle causes tsup's rollup-dts worker to hit ERR_WORKER_OUT_OF_MEMORY on Node 22 / Windows (pre-existing; reproducible on commits before 0.8.43). Split the build so tsup handles the ESM bundle and a post-step `tsc --allowJs --emitDeclarationOnly` generates the .d.ts files. This unblocks the full extension build pipeline. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… optional-chain ctx.browser() getSavedCookies was logging both the real unseal error AND a spurious "'cookies' key is absent" message because the existsSync branch fired regardless of whether the null came from a vault error or a missing key. Added an unsealFailed flag to skip the second diagnostic when the catch path already explained the failure. login-runner.js / manual-login-runner.js: ctx.browser().close() → ctx.browser()?.close() so a null browser() return degrades gracefully instead of throwing a TypeError swallowed by .catch(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee8568f8b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- shared: add DaemonAuthStatus + daemonAuth field on AccountSnapshot - profiles.js: add daemonStatus + loginBrowserData to ProfilePaths (impl + .d.ts) - client.ts: writeDaemonStatus() after every init/reinit/shutdown; fix bare throw→throw err - session.ts: read daemon-status.json into AccountSnapshot - DashboardProvider: FSWatcher on daemon-status.json with 300ms debounce + dispose cleanup; dashboard:refresh touches .reinit when daemon is anonymous with stored login - views.tsx: daemonAuth status dot in hero panel (ok/warn; hidden in stdio mode) - logout.js: clear loginBrowserData on softLogout so next login starts fresh - checks/profiles.js: daemon-status.json auth/tier report + loginBrowserData presence note Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 11, 2026
Closed
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a196df728b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2120
to
+2123
| if (!this.daemonStatusWatcher) { | ||
| if (fs.existsSync(statusFile)) { | ||
| this.startDaemonStatusWatch(); | ||
| } |
There was a problem hiding this comment.
ensureDaemonStatusWatch() only starts a watcher when this.daemonStatusWatcher is null, so once a watcher is attached for profile A it remains active after profile:switch and never retargets profile B's daemon-status.json. In that state, daemon auth changes for the newly active profile will not auto-refresh the dashboard (the exact behavior this watcher was added to provide), and users only see updates after a manual refresh.
Useful? React with 👍 / 👎.
…ofile switch - .gitignore: add packages/mcp-server/scripts/ to allowlist (blanket * rule was excluding the scripts dir, so post-build-shebang.mjs was never tracked) - post-build-shebang.mjs: add the missing script that CI was failing on (Cannot find module: post-build-shebang.mjs across all 8 matrix jobs) - DashboardProvider: track daemonStatusWatchedProfile so ensureDaemonStatusWatch() re-arms when the active profile changes (addresses P2 review comment) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfcd8ff4f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5 — Headed bootstrap reads cookies as anonymous despite successful login
Closes #6 — CLI symlink exit / shebang / keychain prompts / login persistence
What changed
Issue #5 — daemon anonymous mode after successful login
Bug 1 (
fix/#5.1) —PERPLEXITY_HEADLESS_ONLY=1leaked from the launcher's IDE-config env block into the daemon's spawn env via...process.env. The daemon'sPerplexityClient.init()then skipped the headed bootstrap entirely — the only phase that can solve CF challenges. Fixed by strippingPERPLEXITY_HEADLESS_ONLYandPERPLEXITY_NO_DAEMONfrom the spawn env in all three spawn sites (daemon/launcher.ts,extension/src/daemon/runtime.ts,stdio-daemon-proxy.ts).Bug 2 (
fix/#5.2) — Phase 2 (headless search) usedchromium.launch()+ ephemeral context. The freshcf_clearancewritten tobrowser-data/by Phase 1 was never loaded. Phase 2 now useschromium.launchPersistentContext(browserData, ...)— same directory as Phase 1, so the CF clearance is already on disk when Phase 2 starts. Vault cookies are injected selectively (only cookies not already on disk), so a freshly-writtencf_clearanceis never overwritten by a stale vault copy.Diagnostic gap (
fix/#5.3) —getSavedCookies()returned[]silently for four distinct failure modes. Now logs a specific message for each path: novault.enc,cookieskey absent, non-array value, JSON parse failure. AnunsealFailedflag prevents the "key absent" message from firing when the real cause is an unseal error already logged by the catch path.Issue #6 — CLI/auth UX bugs (all four confirmed bugs)
Bug 1 (
fix/#6.1) — CLI and daemon entrypoints silently exited 0 on symlinked invocations (npm global install, Homebrew Cellar,node_modules/.bin/). Theimport.meta.url === pathToFileURL(process.argv[1]).hrefguard fails whenprocess.argv[1]is a symlink. ExtractedisMainModule()helper (src/is-main-module.js) that usesrealpathSyncon both sides, with a defensive fallback to the old comparison whenrealpathSyncrejects.Bug 2 (
fix/#6.2) —dist/cli.mjshad no shebang or executable mode. tsup intentionally strips shebangs (they trip vitest/esbuild during test imports). Newscripts/post-build-shebang.mjspost-build script prepends#!/usr/bin/env nodeandchmod 755s the file. No-op on Windows where npm uses.cmdwrappers.Bug 3 (
fix/#6.3) — Repeated macOS Keychain permission prompts from multiplekeytar.getPassword()calls per session.vault.jsnow caches the keytar module reference in_keytarModuleCache(reset on__resetKeyCache()).probeKeychainState()is exported and shared acrosscli.jsandchecks/vault.js, removing three duplicate inline probe implementations.PERPLEXITY_DISABLE_KEYCHAIN=1escape hatch added for headless/CI environments.Bug 4 (
fix/#6.4) — Login runners (login-runner.js,manual-login-runner.js) used a non-persistent browser context, discarding Google SSO and Cloudflare state between login sessions. Now usechromium.launchPersistentContext(paths.loginBrowserData, ...)with a dedicatedlogin-browser-data/directory (separate from the daemon'sbrowser-data/to avoid Chromium singleton-lock collisions).Build fix
tsup's rollup-dts worker OOMs on this package's 47+ entry points under Node 22+. Build is now split:
tsup --no-dts+tsc --allowJs --emitDeclarationOnly+ post-build shebang script.Review polish (added during code review pass)
getSavedCookies:unsealFailedflag prevents misleading double-log when vault unsealing throwsctx.browser()?.close()(optional chain) so a nullbrowser()return degrades gracefullyTest coverage
test/is-main-module.test.js— exact match, non-match, symlink, missing argv[1]test/vault.test.js—probeKeychainStatecaching +PERPLEXITY_DISABLE_KEYCHAINescape hatchtest/config-getSavedCookies.test.js— "no vault.enc" and "key absent" diagnostic log pathsextension/tests/transports/stdio-daemon-proxy.test.ts— assertPERPLEXITY_HEADLESS_ONLYno longer injectedextension/tests/auto-config.test.ts— same assertion for OpenCode transport130/130 test files pass. Full typecheck clean.
Smoke required before merge
Per
docs/release-process.md: Win11 + macOS 14+ + Ubuntu 22+. Issue #6 bugs 1+2 specifically need the macOS row (Homebrew bin layout is the canonical symlink reproducer). Issue #5 bugs need the daemon-proxy transport path on any OS.