Feat: 添加应用锁屏功能 - #1609
Conversation
![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: 5be97aa5e0
ℹ️ 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".
| notifyRendererReady: boolean; | ||
| renderChildren: boolean; | ||
| }): boolean { | ||
| return input.notifyRendererReady && input.renderChildren; |
There was a problem hiding this comment.
When Netcatty is launched from an ssh:// link with App Lock enabled, renderChildren stays false on the startup lock screen, so this suppresses rendererReady until the user unlocks. deliverSshDeepLink waits on sendWhenRendererReady with a 15s timeout after the URL has already been shifted from the pending queue, so taking longer than that to enter the password drops the deep link instead of connecting after unlock. Consider keeping the deep link queued/buffered while locked or separating renderer readiness from app-content mounting.
Useful? React with 👍 / 👎.
The overlay helper import was accidentally inserted inside open named import lists, which broke TypeScript parsing. Move it to a top-level import statement in SnippetsManager and SFTP keyboard shortcuts.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01d91f55a7
ℹ️ 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".
| <TooltipProvider delayDuration={300}> | ||
| <AppLockGate | ||
| notifyRendererReady={false} | ||
| forceRenderChildren |
There was a problem hiding this comment.
When a terminal popup is opened while the shared app-lock runtime is locked, this forceRenderChildren path mounts TerminalPopupPage before unlock; after the buffered config arrives it renders <Terminal>, whose mount effects start/reuse the terminal backend and can run the popup startup command with saved credentials behind the lock overlay. Keep only the config listener mounted while locked, or defer rendering/starting the terminal until appLock.locked is false.
Useful? React with 👍 / 👎.
Keep forceRenderChildren so popup config IPC still registers while locked, but do not mark the popup ready (or mount Terminal/startup commands) until allowTerminalStart is true after unlock.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e0988d5f0
ℹ️ 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 ready = Boolean(config && host && vaultInitialized); | ||
| // Defer backend start while app-locked so startup commands/credentials do not | ||
| // run behind the overlay (Codex P2). Config listener still mounts above. | ||
| const ready = Boolean(config && host && vaultInitialized && allowTerminalStart); |
There was a problem hiding this comment.
When App Lock engages after a terminal popup has already started, allowTerminalStart flips to false and this makes ready false, so the render switches from <Terminal> to the spinner. That unmounts Terminal, whose unmount path tears down and closes the backend session, so an idle/background lock can disconnect the popup and then start a fresh session or rerun the startup command after unlock. Track whether the popup terminal has already been mounted and only use the lock state to block the initial start.
Useful? React with 👍 / 👎.
Only defer the initial Terminal start while locked. After a popup session has begun, keep ready true so re-lock does not unmount the live terminal.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e6c323559
ℹ️ 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".
| // Hidden prewarm windows (e.g. Settings) keep the process alive but are | ||
| // not a user session — do not block fresh-session re-lock (Codex P2). | ||
| if (typeof win.isVisible === "function" && !win.isVisible()) { | ||
| return false; |
There was a problem hiding this comment.
When the only remaining app UI is a user-created auxiliary window that is minimized, such as a terminal popup or Settings window, isVisible() is false here so createAndShowMainWindow() treats the next Dock reopen as a fresh session and calls ensureAppLockForFreshSession(). That broadcasts a startup lock into an existing active window even though not all app UI was closed; exclude only known hidden prewarm windows, or count minimized user windows as usable.
Useful? React with 👍 / 👎.
isVisible is false for minimized BrowserWindows, which made Dock reopen re-lock while a minimized main/session window was still open. Treat isMinimized() as a live session, while still ignoring hidden prewarm windows.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79603979ef
ℹ️ 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".
|
|
||
| async function save(nextSettings) { | ||
| const normalized = normalizeAppLockSettings(nextSettings); | ||
| await writeFile(filePath, `${JSON.stringify(normalized, null, 2)}\n`, { mode: 0o600 }); |
There was a problem hiding this comment.
If Netcatty or the machine stops while this writeFile has truncated the existing file but has not finished writing JSON, the next startup catches the parse failure in load() and replaces the state with DEFAULT_APP_LOCK_SETTINGS, silently disabling an enabled lock. Any timeout, password, or system-unlock change rewrites this security-critical file, so preserve the previous valid verifier by writing and syncing a sibling temporary file before atomically renaming it.
Useful? React with 👍 / 👎.
| Menu, | ||
| app, | ||
| isMac, | ||
| isAppLocked: typeof options.isAppLocked === "function" |
There was a problem hiding this comment.
Fresh evidence after the App.tsx command-close guard is that the new isAppLocked callback is consulted only by Reload and DevTools: closeFocusedWindow below still calls closeBrowserWindow directly for non-main auxiliary windows. With a locked terminal popup focused, Cmd/Ctrl+W from the application menu can therefore destroy the popup, and Terminal teardown closes its running backend session behind the overlay; return early from native close roles while locked and gate the Settings close IPC path as well.
Useful? React with 👍 / 👎.
Write app-lock settings via temp file + rename to avoid truncating the verifier on crash. Gate application-menu Close and window:close IPC while locked so popups are not destroyed behind the overlay.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81a62b7ae2
ℹ️ 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 appLockSection = source.slice(appLockSectionStart, nextSectionStart); | ||
|
|
||
| assert.match(source, /appLockSystemUnlockStatus\?\.available \|\| appLockSettings\.systemUnlockEnabled/); | ||
| assert.match(appLockSection, /disabled=\{isSavingAppLockSystemUnlock \|\| !appLockSystemUnlockStatus\.available\}/); |
There was a problem hiding this comment.
This assertion still expects the toggle to be disabled whenever system authentication is unavailable, but the implementation intentionally keeps an already-enabled toggle clickable via !appLockSystemUnlockStatus.available && !appLockSettings.systemUnlockEnabled. The targeted test run therefore always fails at this line; update the expectation to include the enabled-state exception so the test suite can pass.
Useful? React with 👍 / 👎.
| if (typeof rename === "function") { | ||
| const tmpPath = `${filePath}.tmp`; | ||
| await writeFile(tmpPath, payload, { mode: 0o600 }); | ||
| await rename(tmpPath, filePath); |
There was a problem hiding this comment.
When two app-lock mutations overlap—for example, changing the timeout while a system-unlock save is pending—both save() calls write and rename the same .tmp path after independently reading the current snapshot. The first rename can remove that path before the second rename, causing an ENOENT, and stale snapshots can also make the later mutation overwrite unrelated fields; renderer callers commonly swallow these rejections. Serialize mutations and use a unique temporary path per write so concurrent settings changes cannot fail or be lost.
Useful? React with 👍 / 👎.
Queue app-lock save mutations and use unique temp paths so concurrent writes cannot ENOENT or clobber. Update the system-unlock toggle test to match the already-enabled-while-unavailable disable path.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 820587b4f5
ℹ️ 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 nextState = runtimeBridge.lock(reason); | ||
| syncIdleTimer(); | ||
| broadcast("netcatty:appLock:runtimeStateChanged", nextState); |
There was a problem hiding this comment.
Fresh evidence after the View-menu guard is that it only prevents a new toggleDevTools click; setLocked() does not close DevTools that were opened before an idle, background, or manual lock. Detached or docked DevTools remain outside the DOM overlay, so someone arriving after the lock can use the existing console to inspect the mounted application state and invoke privileged renderer bridges without unlocking. Close DevTools for every app window as part of the lock transition.
Useful? React with 👍 / 👎.
| async function saveSettings(nextSettings) { | ||
| const saved = await settingsStore.save(nextSettings); |
There was a problem hiding this comment.
Fresh evidence after the writeChain fix is that only settingsStore.save(nextSettings) is queued; each controller mutation still calls getSettings() and constructs nextSettings before entering that queue. If a timeout or system-unlock change overlaps another pending save, both requests can capture the same old snapshot, and the later queued write can revert the first request's field or restore an old password verifier. Queue the read-modify-write operation itself, or merge each mutation against the latest snapshot inside the serialized section.
Useful? React with 👍 / 👎.
| return { ok: false, error: "unsupported" }; | ||
| } | ||
|
|
||
| const result = await systemAuthBridge.requestUnlock(); |
There was a problem hiding this comment.
When multiple app windows are visible during an idle/manual lock, each AppLockGate receives the shared lock broadcast and independently auto-prompts, but this main-process path has no shared in-flight guard. A main window plus Settings or a terminal popup can therefore launch concurrent Touch ID/Windows Hello requests; later requests may show additional prompts or fail as device-busy and surface errors in the other windows. Reuse one controller-level authentication promise and broadcast its result to all locked renderers.
Useful? React with 👍 / 👎.
…tem unlock Close open DevTools when locking, queue full RMW settings mutations, and share one in-flight system-auth prompt across windows.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 851e772026
ℹ️ 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 saved = await saveSettings({ | ||
| ...current, | ||
| enabled: current.enabled || !current.passwordVerifier, | ||
| passwordVerifier, | ||
| }); |
There was a problem hiding this comment.
Fresh evidence after the mutation-chain fix is that requestPasswordChange() still snapshots current outside mutateSettings() and writes it directly. If the user changes the timeout while password replacement is deriving/verifying a hash, the queued timeout mutation can capture the old verifier and save it after this write, silently restoring the old password even though the password-change request reported success. Put the entire password read-verify-write operation on the same mutation chain.
Useful? React with 👍 / 👎.
| }; | ||
| } | ||
|
|
||
| const nextState = runtimeBridge.unlock(); |
There was a problem hiding this comment.
When Touch ID or Windows Hello is pending and a newer lock transition occurs before the prompt resolves—for example, an idle-locked window starts authentication and is then hidden, changing the lock to background—this unconditional unlock() clears that newer lock. A successful stale prompt can therefore leave the hidden app unlocked when it is reopened; capture the runtime version at prompt start and only unlock if the same lock presentation is still current.
Useful? React with 👍 / 👎.
| unsubscribeAppLockRuntime = appLockController.subscribe((state) => { | ||
| if (state?.locked === false) { | ||
| flushPendingPortForwardToggles(); | ||
| } |
There was a problem hiding this comment.
When App Lock engages on Linux with close-to-tray enabled, this subscription does nothing on the locked transition, so the native tray context menu built by buildTrayMenuTemplate() continues displaying cached host labels, session status, and port-forward endpoints outside the lock overlay. Anyone at the machine can inspect those details without unlocking; rebuild/redact the native tray menu when state.locked becomes true and restore it after unlock.
Useful? React with 👍 / 👎.
Password-change saves go through the settings mutation queue. System unlock re-checks locked state after the OS prompt so a re-lock during the dialog cannot unlock from a stale auth success.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c5b811c27
ℹ️ 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 saved = await saveSettings({ | ||
| enabled: false, | ||
| timeoutMinutes: current.timeoutMinutes, | ||
| passwordVerifier: null, | ||
| }); |
There was a problem hiding this comment.
Fresh evidence after the mutation-chain fixes is that requestDisable() still calls saveSettings() directly. If the user changes the timeout while the disable write is pending—the dropdown remains enabled during isDisablingAppLock—setTimeoutMinutes() can snapshot the still-enabled settings and enqueue its save after the disable, restoring the old verifier and re-enabling App Lock even though disabling reported success. Put the complete disable read/verify/write operation on the same controller mutation queue.
Useful? React with 👍 / 👎.
| if (runtimeBridge.getState().locked !== true) { | ||
| return { ok: false, error: "not-locked" }; | ||
| } |
There was a problem hiding this comment.
Fresh evidence after the stale-result fix is that this check only tests the locked boolean. If authentication began for an idle lock and the app is hidden before the prompt resolves, setLocked("background") leaves locked true while advancing the runtime version and changing the reason, so the old prompt still passes this check and unlocks the newer background lock; the hidden app is then already unlocked when reopened. Capture the runtime version at prompt start and require the same lock presentation before calling unlock().
Useful? React with 👍 / 👎.
While the runtime is locked, tray panel/menu data is redacted to empty sessions and port-forwards so cached host details are not visible.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90b05946d0
ℹ️ 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 onWindowCapture = (event: KeyboardEvent) => { | ||
| // Suppress other window-level keydown handlers registered after us. | ||
| event.stopImmediatePropagation(); |
There was a problem hiding this comment.
Fresh evidence after the earlier key-blocker revisions is this unconditional stopImmediatePropagation() on the window capture listener: stopping propagation at window prevents the event from continuing down to the password input, so typed characters and Enter never reach the lock form. Whenever password unlock is required, users cannot type their password and may be unable to unlock unless system authentication or a mouse-driven paste is available; block underlying shortcuts without stopping the event before its target.
Useful? React with 👍 / 👎.
| locked; defer starting the terminal until unlock (Codex P2). */} | ||
| <LazyTerminalPopupPage | ||
| settings={settings} | ||
| allowTerminalStart={!appLock.locked} |
There was a problem hiding this comment.
Fresh evidence after the popup startup guard is that forceRenderChildren renders this route before the runtime IPC resolves, while the initial app-lock state is initialized: false, locked: false; therefore this expression initially passes true. If a popup is created while the shared runtime is locked, or the app locks while that popup is loading, its buffered config and vault initialization can make TerminalPopupPage start the backend and startup command before the locked state arrives. Require appLock.initialized && !appLock.locked for the initial start.
Useful? React with 👍 / 👎.
Put requestDisable/requestReset on the settings mutation queue so concurrent timeout changes cannot re-enable App Lock after a successful disable. Capture the runtime lock version before system auth and reject the prompt if the lock epoch advanced (idle→background re-lock) while the dialog was open.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! 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". |
2 participants
变更内容
新增 Netcatty 应用锁屏能力：应用可以在主界面渲染前锁定，也可以从顶部标签栏手动锁定，并支持空闲超时、隐藏到后台、重新打开等场景下的锁定/恢复。
本 PR 目前覆盖的是完整 App Lock 链路：
AppLockGate启动 gate，避免启动锁定时主界面先渲染到锁屏背后。systemPreferences使用 Touch ID。给 Reviewer 的说明
这个 PR 的 diff 比“添加锁屏”听起来大，因为它不是单个 UI Overlay，而是跨 renderer gate、Electron 生命周期、持久化设置、IPC、安全认证、Windows native helper 打包和回归测试的一整套功能。
当前 PR 统计：
package.json/package-lock.json约 +505 行。Windows Hello/helper 打包相关内容包含在这个 PR 里，是因为 UI 已经暴露了系统解锁入口，而 Windows 需要 native helper 才能完成该路径。如果希望降低 review 面积，可以把本 PR 收敛为“密码锁屏核心”，再把系统解锁/Windows Hello 拆成后续 PR。
行为细节
安全与锁死风险处理
验证
合并最新
upstream/main后已在本地验证：git diff --name-only --diff-filter=U：无未解决冲突。git diff --cached --check：提交 merge 前通过。npm run lint：通过，0 errors，2 个已有 warnings。npm run generate:capability-tools+ generated JSON diff check：无 drift。npm test：4007 tests，4004 passed，0 failed，3 skipped。npm run build：通过。冲突解决后已推送当前 head commit，并重新触发 CI。
关联 Issue
关联 #672
关联 #955