fix(serial): auto-login on serial consoles with saved host credentials - #3418
netcatty-bot wants to merge 10 commits into
Conversation
netcatty-bot
added
automation:bot-pr
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 97a8018821
ℹ️ 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".
| const serialUsername = (ctx.host.username ?? "").trim(); | ||
| const serialPassword = sanitizeCredentialValue(ctx.host.password); | ||
| const hasSerialAutoLoginCredentials = Boolean( | ||
| serialUsername || serialPassword !== undefined, |
There was a problem hiding this comment.
When a serial host belongs to a group with default SSH credentials, ctx.host already contains those inherited values: TerminalLayer.tsx:1025-1032 resolves the effective host, while domain/groupConfig.ts:146-152,210-226 applies username and password without excluding serial hosts. Consequently, even though the serial editor's credential fields are blank, connecting to that host now transmits the group's SSH password to any device that prints a login prompt. Read credentials from the raw host or introduce serial-specific credentials so group SSH secrets are not sent implicitly.
Useful? React with 👍 / 👎.
| autoLoginFallbackTimer = setTimeout(() => { | ||
| autoLoginFallbackTimer = undefined; | ||
| if (!disposeAutoLoginComplete) return; | ||
| if ((ctx.bootEpochRef?.current ?? 0) !== bootEpoch) return; | ||
| scheduleStartupAfterAutoLogin(); | ||
| }, SERIAL_AUTO_LOGIN_FALLBACK_MS); |
There was a problem hiding this comment.
The 65-second fallback begins before awaiting startSerialSession, but the main-process 60-second auto-login window does not begin until the port's open callback creates the detector. If opening a busy or slow serial device takes a substantial part of that interval, the startup command can run while auto-login is still active; if opening takes longer than the fallback plus the startup delay, scheduleStartupCommand sees no attached sessionRef, drops the command, and marks it as already run. Arm this timer only after the session has opened and attached.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f861952f3f
ℹ️ 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".
| if ((session.type === 'telnet-native' || session.type === 'serial') && !payload.automated) { | ||
| session.autoLogin?.handleUserInput(); |
There was a problem hiding this comment.
When a serial device emits an ANSI query before its login prompt, xterm sends the corresponding report through this path without automated: true. Although the surrounding input pipeline explicitly recognizes these values with isTerminalReportSequence, this new serial branch treats the report as interactive input, disables auto-login, and emits the cancellation event that also drops a pending startup command. Apply the same terminal-report exclusion here so only genuine user input takes over.
Useful? React with 👍 / 👎.
| const serialUsername = (ctx.host.username ?? "").trim(); | ||
| const serialPassword = sanitizeCredentialValue(ctx.host.password); | ||
| const hasSerialAutoLoginCredentials = Boolean( | ||
| serialUsername || serialPassword !== undefined, | ||
| ); |
There was a problem hiding this comment.
When a synced password cannot be decrypted, sanitizeCredentialValue() returns undefined, but a saved username still makes hasSerialAutoLoginCredentials true. The detector then submits the username without its password, and a configured startup command waits for 65 seconds before potentially being sent into the password prompt. Detect the encrypted placeholder as the Telnet path does and ask the user to re-enter the credential instead of starting a partial auto-login.
Useful? React with 👍 / 👎.
| hostname: selectedPort, | ||
| port: baudRate, | ||
| username: username.trim() || undefined, | ||
| password: password || undefined, |
There was a problem hiding this comment.
For serial accounts whose valid password is empty, saving this form converts "" to undefined. The reused auto-login detector deliberately distinguishes an absent password from a present empty string and can submit the required blank line, so after this conversion it stops responding at the password prompt. Preserve the empty string or provide an explicit control that distinguishes disabling password handling from saving a blank password.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91a69e2e08
ℹ️ 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".
| clearAutoLoginFallbackTimer(); | ||
| scheduleStartupAfterAutoLogin(); |
There was a problem hiding this comment.
When a serial device completes its login exchange before the startSerialSession IPC promise resolves, this callback schedules the startup command while ctx.sessionRef.current is still unset. With a configured startup delay of zero or another sufficiently short value, scheduleStartupCommand marks the command as run, then its timer fails the current-session check and drops it permanently. Record the completion here and schedule only after tryAttachSessionToTerminal succeeds.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1f05be440
ℹ️ 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".
| if ( | ||
| (session.type === 'telnet-native' || session.type === 'serial') | ||
| && !payload.automated | ||
| && !isTerminalReportSequence(data) | ||
| ) { | ||
| session.autoLogin?.handleUserInput(); |
There was a problem hiding this comment.
When serialConfig.lineMode is enabled, createXTermRuntime.ts:1325-1341 buffers printable keystrokes locally and does not call writeToSession until Enter, so this main-process hook never sees the user's initial interactive input. A user can begin typing to take control while the detector remains active and sends saved credentials—or later runs the pending startup command—before the buffered line is submitted. Signal cancellation when line-mode buffering receives its first user input rather than only when data reaches this write path.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 283d15c4d8
ℹ️ 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".
| if (!isSessionWrite) { | ||
| ctx.terminalBackend.notifyUserInput?.(id); |
There was a problem hiding this comment.
When serial line mode is enabled and the device sends an ANSI query such as DA1 or CPR before its login prompt, xterm emits the automatic reply through onData, and this branch calls notifyUserInput because the reply is not CR, LF, or Ctrl+C. That disables auto-login and cancels the pending startup command even though the user never typed. Fresh evidence in this revision is that the dedicated line-mode notification bypasses the isTerminalReportSequence filter added to the normal write path; apply equivalent report classification before notifying.
Useful? React with 👍 / 👎.
| // distinguishes an absent password from a present empty string and | ||
| // submits the required blank line at a Password prompt. Only clear the | ||
| // credential when it was previously set; preserve a saved blank password. | ||
| password: password || (initialData.password === '' ? '' : undefined), |
There was a problem hiding this comment.
For a device whose valid password is an empty line, a user cannot configure that credential from scratch or change a non-empty password to blank: this expression stores undefined unless initialData.password was already '', so the detector will not answer the password prompt. The new conditional preserves imported/pre-existing blank passwords but does not resolve the editable form ambiguity; add an explicit enabled/blank state or otherwise distinguish clearing auto-login from saving an empty password.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07fa516738
ℹ️ 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".
| // submits the required blank line at a Password prompt. An explicit edit | ||
| // saves exactly what the user typed (including a blank line); an | ||
| // untouched field preserves the stored credential as-is. | ||
| password: passwordChanged ? password : (initialData.password ?? undefined), |
There was a problem hiding this comment.
After a serial host has a saved password, clearing this field sets passwordChanged and persists "", which the auto-login detector deliberately treats as a present blank password (typeof options.password === "string"). Even if the username is also cleared, the host therefore continues answering password prompts automatically, with no control in this form that can restore password to undefined; add an explicit auto-login/password-enabled state or another way to remove the credential.
Useful? React with 👍 / 👎.
| autoLoginFallbackTimer = setTimeout(() => { | ||
| autoLoginFallbackTimer = undefined; | ||
| if (!disposeAutoLoginComplete) return; | ||
| if ((ctx.bootEpochRef?.current ?? 0) !== bootEpoch) return; | ||
| scheduleStartupAfterAutoLogin(); |
There was a problem hiding this comment.
When only a username is configured and the device proceeds from Login: to Password:, the detector submits the username but cannot answer the password prompt. This unconditional fallback then sends the configured startup command after 65 seconds, causing that command to be consumed as the password. Restrict the quiet-device fallback to cases where no login interaction occurred, or receive an explicit expired/incomplete result from the detector instead of scheduling blindly.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8abc3801d
ℹ️ 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".
| autoLoginFallbackTimer = setTimeout(() => { | ||
| autoLoginFallbackTimer = undefined; | ||
| if (!disposeAutoLoginComplete) return; | ||
| if ((ctx.bootEpochRef?.current ?? 0) !== bootEpoch) return; | ||
| scheduleStartupAfterAutoLogin(); |
There was a problem hiding this comment.
For a serial device that rejects the saved credentials, reprints Login: or Password: before the 60-second window expires, and then becomes idle, this timer still schedules the startup command at 65 seconds, so that command is consumed as the next credential. createTelnetAutoLogin reports an incomplete exchange only when handleText is called after expiry; it does not cancel on the earlier repeated prompt, and no later bytes means no cancellation event. Fresh evidence beyond the previously reported partial-credential case is the full-credential rejection path in telnetAutoLogin.test.cjs, which explicitly remains silent until another post-timeout chunk arrives; have the fallback consult whether any login exchange occurred or make the detector own an actual expiry timer.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 908e6260e4
ℹ️ 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".
| liveSession?.webContentsId ?? session.webContentsId, | ||
| ); | ||
| contents?.send(channel, { | ||
| sessionId, | ||
| bootEpoch: liveSession?.bootEpoch ?? options.bootEpoch, |
There was a problem hiding this comment.
When a serial reconnect replaces the same sessionId before the previous detector's 60-second timer fires, the displaced detector remains alive and can still invoke onIncomplete. This callback then reads the replacement session and stamps the stale event with its new bootEpoch, so the renderer accepts the cancellation and drops the new session's pending startup command. Capture the original session/epoch and suppress events once sessions.get(sessionId) !== session, while also cancelling the displaced detector's timer.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac4b43cf25
ℹ️ 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".
| if ( | ||
| (session.type === 'telnet-native' || session.type === 'serial') | ||
| && !payload.automated | ||
| && !isTerminalReportSequence(data) | ||
| ) { | ||
| session.autoLogin?.handleUserInput(); |
There was a problem hiding this comment.
When a serial session has a plugin input interceptor, interactive input reaches this cancellation only after terminalDataPipeline.interceptInput() resolves in writeToSessionWithInterception. A slow interceptor therefore leaves auto-login armed after the user has typed, allowing a prompt arriving during that wait to trigger saved-credential transmission. Cancel the detector at user-input ingress, before asynchronous interception, while retaining the terminal-report exclusion.
Useful? React with 👍 / 👎.
| autoLoginFallbackTimer = setTimeout(() => { | ||
| autoLoginFallbackTimer = undefined; | ||
| if (!disposeAutoLoginComplete) return; | ||
| if ((ctx.bootEpochRef?.current ?? 0) !== bootEpoch) return; | ||
| scheduleStartupAfterAutoLogin(); |
There was a problem hiding this comment.
On a slow-booting serial device whose first login prompt appears after the detector's 60-second window but before this 65-second timer, the expired detector ignores the prompt and emits no cancellation because it never sent credentials. This fallback then sends the startup command into that login prompt. Keep observing late output for login/password prompts, or require confirmation that the device remained prompt-free before scheduling the quiet-device fallback.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
netcatty-bot
added
automation:codex-clean
|
Codex reported no major issues. This PR is marked ready for human review/merge. |
2 participants
Summary
Login:/Password:style prompt, Netcatty automatically sends the saved username and password over the port.Why
SSH sessions can auto-fill saved credentials, but serial consoles (very common in embedded debugging) could not — users had to keep snippets/scripts around to type credentials at a login prompt (#3417). The Telnet bridge already has a robust login-prompt detector (
telnetAutoLogin), so serial now reuses it instead of adding a second implementation.Changes
components/SerialHostDetailsPanel.tsx: username/password inputs (eye toggle) on the serial host editor, stored in the host's existing username/password fields.electron/bridges/terminalBridge.cjs:startSerialSessionwirestelnetAutoLoginto the decoded serial data stream when credentials were saved; serial sessions now also cancel auto-login on interactive user input (same as Telnet). Completion/cancel events are emitted on the existing session-scoped auto-login channels.components/terminal/runtime/createTerminalSessionStarters.ts:startSerialpasses saved credentials to the bridge and, when both credentials and a startup command exist, schedules the startup command after auto-login completes (with a fallback timer for devices that never prompt).types/global/netcatty-bridge-session.d.ts: serial start options acceptusername/password.Testing
node --test --import tsx components/terminal/runtime/createTerminalSessionStarters.serial.test.ts(new: credential pass-through, placeholder skip, startup-command deferral until auto-login completes, unchanged behavior without credentials)node --test electron/bridges/terminalBridge.serialAutoLogin.test.cjsplus allterminalBridge*suites (173 pass), telnet auto-login suites, and the fullcomponents/terminal/runtimedir (974 pass)npx tsc --noEmiterror count unchanged vs. baseline (850 before/after); eslint clean on changed fileslogin:/Password:Fixes #3417
🤖 Generated with Claude Code
Automation
@codex review(own/bot PRs only)