Repository navigation
fix(config): keep credential storage independent of working directories - #158
Merged
Merged
Conversation
Exercise credential storage across directories and reject relative user-directory settings through CLI commands. Cover missing HOME through the public config API without touching the actual user configuration.
Use the OS home directory when HOME is absent and share absolute-path validation across profile storage and shell completion. Preserve directory precedence and the existing layout without importing project-local credentials. Apply POSIX credential mode checks only on platforms where they describe access permissions. State the Windows ACL limitation explicitly and exercise source and npm bundle storage behavior in cross-platform CI.
leonkenneth
reviewed
Oct 19, 2026
| const home = readEnv("HOME"); | ||
| if (home) return requireAbsoluteDirectory(home, "HOME"); | ||
| try { | ||
| return requireAbsoluteDirectory(homedir(), "OS home directory"); |
Contributor
There was a problem hiding this comment.
We could probably default to this instead of the env var?
| } | ||
|
|
||
| function requireSafeCredentialsFile(): void { | ||
| function requireSafeCredentialsFile(platform: NodeJS.Platform): void { |
Contributor
There was a problem hiding this comment.
I think if we want to support Windows we should use a dedicated secret-store abstraction (https://github.com/Brooooooklyn/keyring-node or the like) to actually use Windows' credential manager
Member
Author
There was a problem hiding this comment.
Agreed—native credential storage is the direction we should take. The permission-check change here only unblocks the existing file backend. It doesn’t add native protection.
I'll leave #151 open for now.
robinvrd
approved these changes
Oct 19, 2026
Use os.homedir directly so Windows follows USERPROFILE instead of a separate HOME override. Retain absolute-path validation and explicit config overrides. Exercise completion installation in isolated CLI processes because Bun caches its home-directory lookup. Cover Windows HOME and USERPROFILE precedence through the public CLI.
albert20260301
suggested changes
Oct 19, 2026
albert20260301
left a comment
albert20260301
left a comment
Contributor
There was a problem hiding this comment.
The required Windows User directories job is failing at Verify user-directory storage through CLI commands, so this cannot merge yet. Please fix the Windows regression and rerun the cross-platform storage gate; Linux and macOS passing is not sufficient for this credential-path remediation.
Pass credential input directly to Bun.spawn instead of manually closing a pipe before the child consumes it. Keep the storage regressions focused on completed CLI invocations across platforms.
…ies-review # Conflicts: # CHANGELOG.md
Windows CI reproduces early child exit while credential stdin is supplied through a pipe, including Bun.spawn Blob input. Use a temporary input file so directory tests exercise completed public CLI writes without depending on pipe lifetime.
Windows CI reproduces successful process exit with no saved credentials while awaiting Bun.stdin.text, for both pipe and file-backed input. Read file descriptor zero synchronously so configure completes before the process can exit. Cover API keys and lakehouse passwords through public CLI subprocesses while preserving directory and secret-storage assertions.
albert20260301
approved these changes
Oct 19, 2026
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
4 participants
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.
When
HOMEis unset, the CLI can save profiles and credentials under the current project instead of the user's home directory. This fixes directory resolution through a shared API used by configuration storage and shell-completion installation.Use
os.homedir()directly for platform-native home resolution, preserve the existing config-directory precedence and layout, and reject relative or Windows current-drive-dependent directory settings before accessing storage. Credentials accidentally written inside projects are not imported automatically; affected users should sign in again and revoke any exposed credentials.Windows CI also exposed credential configuration exiting before its asynchronous stdin read completed, with no credentials saved. Read credential stdin through the standard file-descriptor API before continuing; regressions cover both API keys and lakehouse passwords, with pipe and file input.
The Windows file backend stops interpreting Unix mode bits as ACL permissions. This is an interim compatibility correction: the file fallback remains plaintext, Windows ACLs are not validated, and native credential-manager integration is a separate workstream. #151 remains open for that work.
Verification passed on commit
62534ec: canonical repository/integration checks, public CLI regressions against source and npm bundles on Linux/macOS/Windows, minimum supported Bun runtime, and compiled-binary builds/smoke tests for all release targets. The regression coverage includes directory precedence, invalid paths, credential reads across working directories, completion installation, and WindowsUSERPROFILEprecedence.Fixes #150.
Related to #151.