Skip to content

fix(auth): clear .olauth on logout, and stop persisting the password by default - #51

Open
Waynting wants to merge 1 commit into
aloth:mainfrom
Waynting:fix/credential-handling
Open

fix(auth): clear .olauth on logout, and stop persisting the password by default#51
Waynting wants to merge 1 commit into
aloth:mainfrom
Waynting:fix/credential-handling

Conversation

@Waynting

@Waynting Waynting commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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. logout left .olauth behind

clearConfig() was config.clear() and nothing else, while .olauth sits
at position 2 in the resolution order — ahead of the global config it had
just cleared. logout now clears both and lists what it actually removed.

Worth noting the blast radius is slightly wider than the issue said:
src/mcp.ts resolves credentials the same way, so a stale .olauth kept
olcli-mcp authenticated too.

Environment variables get reported rather than ignored. A child process
cannot unset its parent's environment, so OVERLEAF_SESSION and
OVERLEAF_EMAIL/OVERLEAF_PASSWORD are named explicitly:

$ olcli logout
Cleared:
  /Users/me/paper/.olauth

Still authenticated in this shell:
  OVERLEAF_SESSION is set
  These outrank anything on disk. Unset them to finish logging out.

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() already
stores the session cookie on that path, as you pointed out, so the change
really is just whether the password follows it.

--no-save-password still parses and still means "do not save" — it is the
default now, so existing scripts keep working rather than hitting an
unknown-option error. Verified against commander: with both declared,
neither flag gives undefined, --save-password gives true,
--no-save-password gives false.

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-password restores the old behaviour.

3. --password is optional; the prompt is the default path

olcli auth --email you@example.com reads the password from the terminal
without 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() already
reads OVERLEAF_EMAIL/OVERLEAF_PASSWORD, and getClient() logs in from
them, so a scripted run never needs auth at all. The flag is therefore
redundant 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:

$ olcli auth --email you@example.com < /dev/null
No terminal available to prompt for a password.
Set OVERLEAF_EMAIL and OVERLEAF_PASSWORD instead — every command reads them,
so a scripted run does not need 'olcli auth' at all.

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:

  • auth printed Password login saved. unconditionally, including under
    --no-save-password, which had just prevented exactly that.
  • check listed "credential sources" while omitting the stored password
    entirely, 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 terminal
wiring around it, same split as diff.ts. That was not tidiness: I drove
the real prompt over a pty with expect and found that filtering only the
ESC of an arrow key leaves the printable [ and A behind, so pressing
Left mid-entry appended [A to 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 build and npm test pass — 58 tests, up from 47.
Lint warnings are unchanged at 62, all pre-existing anys.

New tests avoid the trap that config.ts builds its Conf store against the
real user config path at import time: nothing calls clearConfig() or any
setter, every assertion is scoped to a temp directory or to process.env,
and the two config-backed fields of inspectStoredCredentials() are left
unasserted because they depend on whether the machine is logged in.

Manual verification ran against an isolated HOME, so no real credentials
were touched: logout with nothing stored, with .olauth present, and with
the env vars set; check with and without a stored password; auth with no
TTY; auth --password warning ahead of the login attempt.

…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
Waynting force-pushed the fix/credential-handling branch from a27f0f9 to b577265 Compare September 4, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

logout does not clear .olauth; password stored in plaintext by default

1 participant