fix(auth): clear .olauth on logout, and stop persisting the password by default - #51
Open
Waynting wants to merge 1 commit into
Open
fix(auth): clear .olauth on logout, and stop persisting the password by default#51Waynting wants to merge 1 commit into
Waynting wants to merge 1 commit into
Conversation
…by default Three related problems in credential handling, reported in aloth#50. logout cleared the global config and printed "Credentials cleared", while the .olauth file in the current directory survived. That file is consulted ahead of the global config by both getSessionCookie() and the MCP server, so the user stayed authenticated in that directory. logout now clears both and reports each source it removed. Environment variables cannot be unset by a child process, so OVERLEAF_SESSION and OVERLEAF_EMAIL/OVERLEAF_PASSWORD are reported rather than ignored - staying silent about them would repeat the original mistake somewhere else. The account password was persisted in plaintext unless the user opted out. It is now written only under --save-password. The session cookie stored alongside it is what every later command uses; the password only bought an automatic re-login after that cookie expired, and unlike a cookie it is reusable outside olcli and cannot be revoked without changing it. --no-save-password still parses and still means what it says, so existing scripts keep working. --password is now optional. Omitting it prompts on the terminal without echoing, which keeps the password out of shell history; with no terminal available the error names OVERLEAF_EMAIL/OVERLEAF_PASSWORD, which every command already reads. Keystroke handling is a pure reducer so it can be tested without a pty - driving the real prompt over one is what surfaced the bug it now guards against, where filtering only the ESC of an arrow key left the printable '[' and 'A' behind and appended them to the password. Also: auth said "Password login saved." unconditionally, including under --no-save-password, and check omitted the stored password entirely - the same class of bug as the logout message, in two more places.
Waynting
force-pushed
the
fix/credential-handling
branch
from
September 4, 2026 14:58
a27f0f9 to
b577265
Compare
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
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 #50 (items 1-3; item 4 you already fixed in 0.11.0).
Kept as one PR, as you suggested — the three items turned out to share a
single failure mode, which the diff makes clearer than three separate ones
would have.
1.
logoutleft.olauthbehindclearConfig()wasconfig.clear()and nothing else, while.olauthsitsat position 2 in the resolution order — ahead of the global config it had
just cleared.
logoutnow clears both and lists what it actually removed.Worth noting the blast radius is slightly wider than the issue said:
src/mcp.tsresolves credentials the same way, so a stale.olauthkeptolcli-mcpauthenticated too.Environment variables get reported rather than ignored. A child process
cannot unset its parent's environment, so
OVERLEAF_SESSIONandOVERLEAF_EMAIL/OVERLEAF_PASSWORDare named explicitly:Silently ignoring them would have reproduced the original bug one layer up.
2. The password is no longer stored by default
Written only under
--save-password.persistClientSession()alreadystores the session cookie on that path, as you pointed out, so the change
really is just whether the password follows it.
--no-save-passwordstill parses and still means "do not save" — it is thedefault now, so existing scripts keep working rather than hitting an
unknown-option error. Verified against commander: with both declared,
neither flag gives
undefined,--save-passwordgivestrue,--no-save-passwordgivesfalse.One behaviour change worth calling out in the release notes: an expired
session no longer re-logs in silently. That mattered most to self-hosted
users, which is the population password login exists for. The success
message now says so, and
--save-passwordrestores the old behaviour.3.
--passwordis optional; the prompt is the default patholcli auth --email you@example.comreads the password from the terminalwithout echoing. The flag still works and warns that it is now in shell
history.
On your "does the flag stay or go" question — it stays, but the argument
for keeping it is weaker than it looks.
getPasswordCredentials()alreadyreads
OVERLEAF_EMAIL/OVERLEAF_PASSWORD, andgetClient()logs in fromthem, so a scripted run never needs
authat all. The flag is thereforeredundant for scripting rather than necessary for it. I left it because
removing a documented flag in the same PR that changes two defaults felt
like one change too many — say the word and I will drop it in a follow-up.
That is also what the no-TTY error points at:
The same bug in two more places
Both are the failure mode you named on the issue — a message stating an
outcome that did not happen:
authprintedPassword login saved.unconditionally, including under--no-save-password, which had just prevented exactly that.checklisted "credential sources" while omitting the stored passwordentirely, so "is my password on disk?" could only be answered by opening
the config file. It now reports presence and source, never a value.
On the prompt implementation
No new dependency — a masked read is a raw-mode loop over stdin.
The keystroke handling is a pure reducer (
applyChunk) with the terminalwiring around it, same split as
diff.ts. That was not tidiness: I drovethe real prompt over a pty with
expectand found that filtering only theESCof an arrow key leaves the printable[andAbehind, so pressingLeft mid-entry appended
[Ato the password invisibly. Delete (ESC [ 3 ~)had the same problem. It is now a three-state machine that swallows CSI
sequences whole, including when a terminal splits one across reads.
Extracting it means that logic is covered without a pty; what a pty
confirmed separately is that input is not echoed, Ctrl+C cancels, and
backspace, paste and empty input behave.
Testing
npm run lint,npm run buildandnpm testpass — 58 tests, up from 47.Lint warnings are unchanged at 62, all pre-existing
anys.New tests avoid the trap that
config.tsbuilds itsConfstore against thereal user config path at import time: nothing calls
clearConfig()or anysetter, every assertion is scoped to a temp directory or to
process.env,and the two config-backed fields of
inspectStoredCredentials()are leftunasserted because they depend on whether the machine is logged in.
Manual verification ran against an isolated
HOME, so no real credentialswere touched: logout with nothing stored, with
.olauthpresent, and withthe env vars set;
checkwith and without a stored password;authwith noTTY;
auth --passwordwarning ahead of the login attempt.