feat(auth): Sign In via BossConsole account in the Share menu

[kshivang]

Adds Sign In as the 4th item in the Share menus, against BossConsole's shared Supabase backend (one user pool) over raw GoTrue REST — no Supabase SDK, reusing the existing ktor client. Magic-link flow: enter email → GoTrue emails a link → BossConsole's redirect function emits bossterm://auth/verify?token_hash=…&type=… → BossTerm verifies and persists the session. Hidden in embedded builds (serverName != "bossterm").

Backend half: risa-labs-inc/BossConsole#789 (closes #787). Until that ships, the email is BOSS-Console-branded and its link opens BossConsole — the client here is complete and testable via a manually-opened deep link.

New compose-ui/.../auth/ package

• BossAccountManager — StateFlow<AccountState>; sendMagicLink (POST /auth/v1/otp, redirect_to=bossterm://auth/verify), handleAuthDeepLink (POST /auth/v1/verify with token_hash + verbatim type), signOut, refresh-on-startup.
• AuthModels / AuthStorage — DTOs + parseAuthDeepLink + error mapping; session in ~/.bossterm/auth.json (atomic write, chmod 600 on POSIX, never logged).
• DeepLinkHandler / DeepLinkSocket — new bossterm:// support: macOS setOpenURIHandler (installed before AWT), Windows/Linux argv + single-instance loopback forwarder (bounded read, constant-time secret compare), -Dbossterm.debug.deeplink dev hook.
• WindowsProtocolRegistrar — HKCU scheme registration on packaged Windows launches.
• SignInWindow — email → sent (60s resend cooldown) → verifying → signed-in.

Wiring

Main.kt (fun main(args) + DeepLinkHandler.install first thing); macOS CFBundleURLTypes; .deb .desktop patch (MimeType + %U); snap x-scheme-handler/bossterm; "Sign In…" at all three Share menu sites; a deferred "Signed in as …" toast.

Security (reviewed)

Anon key is byte-identical to BossConsole's already-public fallback (RLS is the gate); tokens chmod-600 + never logged; deeplink params go into a JSON body (no injection); socket secret is 128-bit, owner-only, constant-time-compared.

Tests / verification

:compose-ui:desktopTest auth suite (deeplink parsing incl. token↔token_hash + verbatim type, error mapping, storage round-trip/corruption, 600-perms assertion) green; full build green.

Manual before release (OS-delivery has no automated coverage): packaged .app cold-start open "bossterm://auth/verify?token_hash=…&type=magiclink"; a Linux .deb .desktop carries the MimeType + %U; Windows HKCU registration. RPM scheme registration is a known gap (jpackage .rpm isn't repacked).

Generated with Claude Code

[claude]

Review: feat(auth): Sign In via BossConsole account

Thorough, well-documented PR — the comments explaining why (verbatim type, token↔token_hash, atomic+chmod-600 storage, constant-time secret compare, the URI host/path parsing gotcha) are excellent and made review easy. The pure-logic test suite covers the right surfaces. A few things worth addressing before release.

Potential bugs

1. Cold-start sign-in toast is silently lost (functional gap).
signInEvents is a MutableSharedFlow(extraBufferCapacity = 1) with replay = 0. When the app is launched by a deep link (Windows/Linux argv path → handle(uri) in install(), or a macOS cold start), handleAuthDeepLink fires immediately in main() — long before TabbedTerminal composes and subscribes to signInEvents. If the network verify completes before the subscriber attaches, tryEmit has no collector and replay=0 means the value is dropped, so the "Signed in as …" toast never appears. The state flow is still correct (StateFlow replays), so it's UX-only — but it's exactly the first-run case. Consider giving signInEvents replay = 1, or driving the toast off a SignedIn state transition instead of a one-shot event.

2. URLDecoder.decode turns + into a space.
In parseAuthDeepLink, values go through URLDecoder.decode(..., UTF_8), which follows application/x-www-form-urlencoded rules — a literal + in the query becomes a space. GoTrue token_hash values are normally hex / base64url (-_), so this is unlikely to bite, but a token containing a raw + would be corrupted into a space and fail verify confusingly. The test only covers the percent-encoded form (%2B). Low risk, but worth a comment or a decoder that preserves + for the token field.

Security (looks solid)

• Anon key public-by-design with env/property override, chmod-600 atomic token storage (nicely tested), deep-link params marshalled into a JSON body (no injection), 128-bit owner-only socket secret with constant-time compare, bounded socket read — all good and clearly reasoned.
• OtpRequest(createUser = true) means any address can self-register via magic link. That is the expected magic-link signup model and also avoids account enumeration, so probably intended — just flagging it is effectively open signup against the shared user pool.
• errorFrom surfaces raw GoTrue body text to the user for non-429/401/403 cases. GoTrue messages are generally benign, but it does pass server-controlled strings straight to the UI; fine as-is, just be aware.

Other notes

• refresh-on-401 gap is already called out in restoreSession's comment — good that it is documented. Just make sure the first feature to actually use the access token honors that contract, since nothing enforces it today.
• DeepLinkSocket.tryForward sets soTimeout (read timeout) but the Socket(...) constructor uses the default connect timeout. Loopback connect to a live ServerSocket is effectively instant, so this is fine; only relevant if the listener is wedged rather than dead.
• Multiple windows: showSignInWindow and the toast collector live per-TabbedTerminal, so with several app windows you can get multiple sign-in windows and a toast per window. Minor, likely acceptable.
• RPM scheme registration is acknowledged as an unhandled gap in the PR body — fine to ship as a known limitation, just make sure it lands in release notes / follow-up.

Tests

Good coverage of the pure layer (parse, error mapping, storage round-trip + corruption + 600-perms). The honest caveat stands: the network flow and all the OS-delivery paths (macOS Apple Event, Windows HKCU, .deb/snap MimeType, single-instance forwarding) have no automated coverage and rest entirely on the manual checklist. Given how platform-specific and hard-to-retest these are, that checklist is the real gate here — worth running it on all three before merge.

Overall: clean, conventions-respecting (ShellCustomizationUtils, no backwards-compat hacks, tokens never logged), and the embedded-build hiding via serverName is the right call. The cold-start toast and the +-decoding note are the only things I would change in code; the rest are confirm-before-release items.

[claude]

Code Review — feat(auth): Sign In via BossConsole account

Thorough, well-documented PR. The pure-logic core (parsing, error mapping, storage) is clean and well-tested, the threading model around the toast/signInEvents is carefully thought through, and the security posture (chmod 600, never-logged tokens, constant-time secret compare, JSON-body params) is solid. A few things worth a look before merge.

🐛 Bugs / behavioral issues

1. DeepLinkSocket: every launch becomes the listener — "last instance wins", and its exit unregisters routing for the others.
DeepLinkHandler.install() calls DeepLinkSocket.startPrimaryListener() on every non-forwarding launch, not just the first. So each normal (non-deep-link) launch overwrites ~/.bossterm/deeplink.port with its own port+secret, and registers portFile.deleteOnExit(). Consequences:

• With instances A then B both running, deep links route to B (last writer), not "the first instance" the doc describes.
• When B exits, deleteOnExit removes the port file even though A is still listening so a subsequent deep link finds no file, falls back to local handling, and spawns a brand-new window instead of routing to A.

Not a crash (graceful degradation), but the doc comment ("the first instance listens") doesn't match the behavior. Consider: only start the listener if no live listener already owns the port file (probe it first), or at least reconcile the comment with "last launch wins."

2. parseAuthInput / paramValue: URLDecoder.decode turns + into a space.
The whole-string decode pass and paramValue's per-value decode both use URLDecoder.decode, which maps + to a space (form-encoding semantics). If a pasted token/token_hash ever contains a literal + (standard base64, not base64url), it gets corrupted. Tokens here are typically hex/base64url so this is unlikely to bite, but it's a latent footgun worth a comment or a guard. The clean OS deep-link path (parseAuthDeepLink, per-param decode) is fine.

⚙️ Minor

• readBoundedLine uses b.toChar() on a raw byte — correct only for ASCII. Fine for hex secret + ASCII URIs, but a multibyte UTF-8 URI would be mangled. A comment noting the ASCII assumption would help.
• signInToast shows in every open window — each TabbedTerminal registers a signInEvents collector, so a sign-in toasts in all windows simultaneously. Likely acceptable; flagging in case single-window was intended.
• WindowsProtocolRegistrar spawns 3 reg add processes on every packaged Windows launch. Harmless but could short-circuit if the key already points at the current exe.
• Stale-listener DoS (loopback-only): a local client that connects but never sends blocks the single-threaded accept() loop for up to soTimeout (2s). Negligible given loopback scope, but the accept loop is serial.

✅ Strengths

• restoreSession offline-handling and the explicit NOTE about needing refresh-on-401 before any authenticated consumer ships is exactly the right call-out.
• Atomic write + ATOMIC_MOVE + POSIX 600, with a test asserting the perms — nicely done.
• The AWT-emoji-corruption avoidance (Java2D glyph instead of an emoji in the menu label) is a good catch and well-commented.
• sendMagicLink returning false on local validation so the UI doesn't start the 60s cooldown is a thoughtful detail.

🧪 Tests

Good coverage of the pure layer (deep-link/paste parsing incl. the legacy token param, verbatim type, error mapping, storage round-trip + corruption + 600-perms). As the PR notes, DeepLinkSocket forwarding and the multi-instance routing (finding #1) are untested — that path is where the surprising behavior lives, so even a single happy-path forward test would be valuable.

Overall LGTM pending a look at finding #1 (or just aligning the doc with reality). Nice work.

Reviewed with Claude Code.

[claude]

Review: feat(auth): Sign In via BossConsole account in the Share menu

Read through the whole diff (auth package, Main wiring, menu sites, packaging, tests). This is a well-engineered, carefully-commented feature — the security hygiene in particular is a cut above. A few things worth addressing before/after merge.

What's good

• Security posture is thoughtful: auth.json is a separate file from settings.json (won't leak into pasted bug reports), atomic write + chmod 600 on POSIX with a test asserting the exact perm set, tokens never logged, anon key documented as public-by-design (RLS is the gate). The loopback forwarder uses a 128-bit owner-only secret, a constant-time compare, and a bounded read against a hostile local writer. This is the right level of paranoia for a local IPC channel.
• The decodeQueryComponent +-preservation fix is correct and genuinely subtle — URLDecoder form-decoding a standard-base64 token_hash would silently corrupt a + to a space. Nicely caught, and the regression tests on both the OS-link and pasted-URL paths lock it in.
• The single-owner deep-link routing fix (probe-before-claim) is a real correctness improvement over "last launch wins", and the doc comment explains the prior bug well.
• Pure logic (parsing, error mapping, storage) is cleanly separated and unit-tested; the token↔token_hash compat and verbatim type pass-through (signup vs magiclink) are exactly the kind of thing that bites in production.

Issues

1. No HTTP request timeout → UI can hang indefinitely (medium).
HttpClient(CIO) { expectSuccess = false } configures no HttpTimeout. verify() sets _state = Verifying then awaits gotrue(...) with no request timeout; if the server accepts the socket but never responds, the dialog stays on "Signing you in…" forever with no way back. Same for sendMagicLink. Recommend installing the HttpTimeout plugin (e.g. request/connect ~15–30s) so a stuck request resolves to your existing mapAuthError(0, …) path.

2. paramValue substring match can grab the wrong param (low, latent).
paramValue(s, "token") does s.indexOf("token="), which also matches inside refresh_token= and access_token=. A pasted URL/fragment containing refresh_token=… before token=… would extract the refresh token as the hash. token_hash is tried first so the common case is safe, and a standard verify URL doesn't carry refresh_token, but it's a real footgun in the deliberately-lenient paste path. Consider anchoring the match to a param boundary (preceding ?, &, #, or start-of-string) rather than a bare substring.

3. Cold-start sign-in toast is silently dropped (minor UX).
signInEvents is a MutableSharedFlow(replay = 0, extraBufferCapacity = 1). If a bossterm://auth/verify link arrives at launch and verify() emits before any TabbedTerminal has started collecting, the event is dropped (no subscriber, no replay) — the user lands signed-in but never sees the confirmation. The state flow still reflects SignedIn, so it's cosmetic, but if the toast matters for cold-start links consider replay = 1 or gating emission on subscriber presence.

Nits

• WindowsProtocolRegistrar.registerIfPackaged() shells out to reg add three times on every packaged Windows launch. Harmless but wasteful — a reg query guard (or doing it once on install) would avoid 3 subprocess spawns per start.
• liveListenerPresent() has a TOCTOU window — two simultaneous cold starts can both claim primary and race on deeplink.port. Already best-effort by design; just noting it's still there after the fix.
• restoreSession()'s offline branch keeps a possibly-expired access token signed-in with no refresh-on-401 anywhere. The comment already flags this clearly; just make sure the first consumer of the token honors that contract.
• GoTrueError.errorCode is declared but never read in text() — dead field.
• RPM scheme registration is a known gap (called out in the PR body) — fine to defer, but worth a tracking issue so it isn't forgotten.

Tests

Good coverage of the pure layer (19 tests: parsing incl. +-literal + percent-encoding, error mapping, storage round-trip/corruption, the 600-perms assertion). The network/OS-delivery half is necessarily manual — the PR body's manual checklist is the right mitigation. Adding a paramValue test for the refresh_token-before-token case (issue #2) would close the one untested logic gap.

Overall: solid, mergeable work. #1 (timeout) is the only one I'd treat as a should-fix; the rest are polish.

🤖 Generated with Claude Code

[claude]

Review: feat(auth): Sign In via BossConsole account in the Share menu

Thorough, well-documented PR — the comments carry the why (RFC 3986 + handling, single-owner routing, AWT emoji-metrics workaround, verbatim type passthrough), the error/auth-state modeling is clean, and the four review-fix commits already closed the highest-value issues (timeouts, anchored param matching, single-owner routing, literal-+ decoding). Findings by severity below.

Bugs / correctness

• paramValue value scan doesn't stop at ? (AuthModels.kt:140) — the end scan breaks on &, hash, whitespace, but not ?. A pasted …token=abc?foo=bar would yield abc?foo=bar as the token. Unlikely in the real confirmation-URL shapes, but cheap to harden (&& s[end] != '?').
• verify() reads global state to recover the email for the toast (BossAccountManager.kt:148): (state.value as? SignedIn)?.let { _signInEvents.tryEmit(it.email) }. scope is Dispatchers.IO (multi-threaded), so a concurrent state transition between adoptSession() and this read could drop the toast or read a stale email. Returning the email from adoptSession and emitting it directly removes the window. Low impact (concurrent verifies are rare), but a latent data race.

Security

• Token bytes briefly world-readable in the temp file (AuthStorage.kt:35-37): order is createTempFile then writeText(token) then restrictToOwner(tmp). Java's createTempFile honors umask (commonly 0644), so the token is on disk group/world-readable until restrictToOwner runs after the write. Restrict the temp file before writing its contents (or create it with an explicit PosixFilePermission set) to close the window. Same-user/local scope so low severity — but easy to make airtight, and the existing 600 test only checks the final file.
• Loopback forwarder trust model (DeepLinkSocket.kt): the 128-bit secret + constant-time compare + bounded read are solid. Worth stating explicitly that any same-user local process can read deeplink.port and inject arbitrary bossterm:// URIs — acceptable (it's the user's own trust boundary, and a forged verify only redeems a token the caller already holds, no escalation), but it's the assumption the design rests on.
• Hardcoded anon key fallback is fine per the Supabase model (RLS is the gate, key is public) and is documented + override-able. No concern.

Robustness / minor

• Blocking file IO on the UI thread: signOut() calls AuthStorage.load() synchronously (BossAccountManager.kt:160) from a Compose onClick. Tiny file, but it's disk IO on the EDT; consider moving the load into the launched coroutine alongside the revoke.
• createTempFile(..., file.parentFile) when parentFile is null (AuthStorage.kt:35) falls back to the system temp dir, which then breaks ATOMIC_MOVE (cross-filesystem). Can't happen for defaultFile(), only an exotic custom File — guard or document.
• Deep-link listener thread / http client are never shut down — both app-lifetime singletons, so fine; just noting there's no teardown path.

Test coverage

Pure-logic coverage is genuinely good — deep-link parsing, token/token_hash compat, the access_token/refresh_token anchoring regressions, literal-+, error mapping, storage round-trip, and the chmod-600 assertion are the right guards. Gaps (acknowledged in the PR body): DeepLinkSocket's single-owner/forwarding logic — the most intricate new code and the source of two prior bugs — has no automated coverage. The probe/claim/stale-cleanup state machine is unit-testable with an injectable port file and would be worth it. WindowsProtocolRegistrar, the .deb/Info.plist patching, and the UI remain manual-only; the RPM-gap call-out is appreciated.

Style

Matches the codebase. Nit: the auth wiring in TabbedTerminal.kt uses fully-qualified inline names rather than imports — consistent with the surrounding share-menu code, so not blocking, but imports would read cleaner.

Overall: solid, security-conscious work with the risky bits clearly flagged. Nothing blocking; the temp-file permission window and the verify() toast race are the two I'd most want addressed before this carries real auth tokens.

[claude]

Code Review: Sign In via BossConsole account (PR #295)

Reviewed the full auth package, deep-link plumbing, menu wiring, packaging changes, and tests. This is a careful, well-documented piece of work — the kdoc on each file explains why, the security model is stated explicitly, and the commit history shows several review findings already chased down (single-owner routing, literal-+ handling, anchored param matching, HTTP timeouts, empty-then-chmod staging). Below are the remaining observations, mostly minor.

Strengths

• Clean separation of pure logic from I/O. parseAuthDeepLink / parseAuthInput / mapAuthError are pure and well-tested, with the network half explicitly carved out as manual-only. Sensible given the constraint.
• Security model is stated, not assumed. The DeepLinkSocket trust-boundary kdoc (loopback + chmod-600 + "same-user is already inside the boundary; a forged verify only redeems a token the caller must already hold") is exactly the right framing. Constant-time secret compare and the bounded read are good hygiene even if loopback-only.
• Token-at-rest handling. chmod-600 before writing token bytes (newOwnerOnlyTempFile), staging in the target dir to keep ATOMIC_MOVE same-filesystem, never logging tokens — all correct, and the empty-then-restrict window is now closed and asserted in a test.
• The +-corruption fix is genuinely subtle and correct. Tracing both the %2B-encoded and literal-+ redirect-URL cases through decodeQueryComponent, the token survives intact in each; the raw pass wins for single-encoded inputs so there's no accidental double-decode. The regression guards lock this in.

Minor issues / suggestions

1. Cold-start sign-in toast can be dropped. signInEvents is a MutableSharedFlow(replay = 0, extraBufferCapacity = 1). On a cold start that carries the deep link (the only-instance argv path), verify() is launched before application {} composes the TabbedTerminal collector. If verification finished before the collector subscribes, the toast is lost (replay=0 → no late delivery). In practice the network round-trip almost always loses the race to composition, and the SignedIn StateFlow still renders correctly, so this is cosmetic — but replay = 1 would guarantee delivery to the first subscriber.

2. WindowsProtocolRegistrar re-registers on every packaged launch. registerIfPackaged() spawns three synchronous reg add … /f processes (with waitFor) on the main thread before application {} on every Windows packaged start. It's idempotent in effect but pays the cost each time and adds startup latency. Consider a reg query guard first, or running it off-thread, since the HKCU entry rarely changes after first run.

3. Committed anon JWT. SupabaseAuthConfig.anonKey ships a hardcoded JWT. This matches Supabase's public-anon-key + RLS model and the kdoc calls it out — flagging only so it's a conscious decision that this repo's visibility is acceptable for that key, and that RLS is truly the gate on every table BossTerm will touch.

4. restoreSession offline path keeps a possibly-expired identity. Well-documented, and the kdoc correctly warns that the first feature to call an authenticated endpoint must handle a 401 by refreshing/signing-out (there's no refresh-on-401 yet). Worth turning that comment into a tracked issue so it isn't forgotten when the first authed call lands.

5. TOCTOU in startPrimaryListener. Two truly-simultaneous cold starts could both pass liveListenerPresent() and both bind+advertise (last writer wins the port file). Documented as best-effort and the window is tiny; fine to leave, just noting it's a known race.

6. HttpClient is never closed. It's an app-lifetime singleton on BossAccountManager, so this is acceptable — mentioning only for completeness.

Test coverage

Strong for what's testable in-process: deep-link parsing (incl. token↔token_hash, verbatim type, literal-+, access/refresh-token decoys, nested-? boundary), error mapping, storage round-trip + corruption + 600-perms (final and staging), and a real-loopback DeepLinkSocket state machine (claim, forward, stale cleanup, no-usurp, wrong-secret, scheme filter). The untested surfaces are the deliberate ones (live GoTrue, OS scheme delivery, RPM registration) and are documented as manual-before-release. The known RPM scheme-registration gap is the one item I'd make sure is tracked so it doesn't silently ship broken for .rpm users.

Verdict

High quality and low risk. Nothing here blocks merge in my view — items 1, 2, and 4 are the most worth a quick follow-up (toast delivery, Windows re-registration cost, and tracking the refresh-on-401 TODO + RPM gap).

Review generated with Claude Code.

[kshivang]

Review follow-up — all findings addressed

Thanks for the thorough pass. Resolved across 37b32637, 5f6db08a, 0844254f, b5c978b1 (branch pushed). Auth test suite is now 31 green (AuthModelsTest 25 + new DeepLinkSocketTest 6).

Bugs / correctness

• paramValue stops at ? — value scan now treats ? as a delimiter too; regression test added.
• verify() toast race — adoptSession() returns the adopted email and verify() emits that directly instead of re-reading the shared state.value (no window for a concurrent transition). restoreSession() still ignores it, so the interactive-verify-only contract holds.

Security

• Temp-file permission window — extracted newOwnerOnlyTempFile(), which chmod-600s the staging file while empty, before any token bytes are written (java.io.File.createTempFile honors umask). New test asserts the staging temp is empty and 600.
• Loopback trust model — documented explicitly on DeepLinkSocket: loopback-only + chmod-600 port file ⇒ secret readable only by this OS user; a same-user process is already inside the trust boundary and a forged verify only redeems a token it must already hold (no escalation).
• Anon key — confirmed role: anon (public by design); no change.

Robustness

• Blocking IO on the UI thread — signOut() flips state synchronously, then does load()/clear() + revoke in the launched coroutine.
• createTempFile null parent / ATOMIC_MOVE — staging dir is now file.absoluteFile.parentFile, so it can't fall back to the cross-device system temp dir.
• No teardown path — added DeepLinkSocket.stop() (closes the listener socket, drops the port file); the daemon listener remains an app-lifetime singleton in production.

Test coverage

• DeepLinkSocket state machine — new DeepLinkSocketTest over real loopback sockets with an injectable port file: claim-on-empty + forward round-trip, no-file forward, stale-file cleanup, no-usurp of a live listener, wrong-secret rejection, non-bossterm:// scheme filter.

Also closed from the earlier reviews

• Cold-start sign-in toast (replay = 0 drop) — completed sign-ins now route through emitSignIn(): emits to a live collector, else stashes for the first window to drain exactly once (avoids the replay = 1 re-toast-on-every-window regression). Tested.
• Dead GoTrueError.error_code field removed; readBoundedLine's ASCII assumption documented.

Intentionally left (with rationale)

• Fully-qualified inline names in TabbedTerminal.kt — kept for consistency with the adjacent share-menu wiring (as noted, non-blocking).
• Multi-window toast, open magic-link signup, WindowsProtocolRegistrar 3× reg add, liveListenerPresent TOCTOU — design choices / harmless as flagged.
• RPM scheme registration — known gap, tracked in the PR body's manual checklist; .deb/Info.plist/Windows-HKCU delivery remain manual-only by nature.

[claude]

PR Review: Sign In via BossConsole account in the Share menu

Thorough, well-documented change. The security reasoning is laid out explicitly in the code, the tricky edge cases (base64 +, access_token boundary anchoring, signup vs magiclink type, cold-start toast) are each handled and tested, and the platform-delivery story is clearly scoped. Below is feedback by category — nothing here is a blocker, but a couple of items are worth addressing before release.

👍 Strengths

• Security posture is genuinely careful: atomic write + chmod 600 for auth.json including the empty-temp window before token bytes land (and a test that asserts it), constant-time secret compare, bounded socket read, single-use-token trust model documented honestly, tokens never logged, anon-key-is-public reasoning correct (RLS is the gate).
• Good failure handling: HTTP timeouts so the UI can't strand on "Signing you in…", offline-at-startup keeps cached identity, corrupted auth.json → silent signed-out, refresh-on-stale with graceful fallback.
• Strong test coverage for the pure/stateful logic: deep-link parsing, error mapping, storage round-trip + perms, and the DeepLinkSocket claim/forward/stale/no-usurp/wrong-secret state machine (the riskiest code) all covered.

🐛 Potential issues

1. DeepLinkSocket writes the secret before restricting perms (low, but inconsistent with your own pattern).
In startPrimaryListener:

portFile.writeText("${server.localPort}\n$secret\n")
restrictToOwner(portFile)

On a fresh deeplink.port (the file is deleteOnExit, so each cold start re-creates it), writeText creates it under the umask (commonly 0644), so the secret bytes are briefly group/world-readable before restrictToOwner runs. This is exactly the window you carefully close for auth.json via newOwnerOnlyTempFile (create empty → restrict → write). Per the documented trust model the secret only keeps other users out — which is precisely the window exposed here. Suggest mirroring the AuthStorage pattern: create empty, restrict, then write.

2. readBoundedLine decodes bytes as Latin-1, not UTF-8 (low).

buf.append(b.toChar())

The URI is sent as UTF-8 (toByteArray(Charsets.UTF_8)), so a non-ASCII byte in a forwarded URI would be corrupted. In practice OS-delivered deep links are ASCII/percent-encoded and the secret is hex, so impact is minimal — but accumulating into a ByteArray and decoding once as UTF-8 would be more correct and costs little.

3. Cold-start verify and restoreSession race on _state (low).
A cold-start deep link triggers BossAccountManager init, which launches restoreSession() on the IO scope, and handleAuthDeepLink launches verify() on the same scope. If a stale stored session exists, both can set _state via adoptSession with nondeterministic ordering (old identity vs. freshly-verified one). Low impact (eventually consistent, and a fresh sign-in usually has no stored session), but worth a comment or sequencing so the interactive verify wins.

⚡ Performance

• WindowsProtocolRegistrar.registerIfPackaged() runs three synchronous reg subprocesses with waitFor() on the startup thread before application {}. On packaged Windows this adds a small but real blocking delay to every launch. Since it's best-effort, consider running it off-thread.
• startPrimaryListener also runs on macOS, creating an unused loopback listener + deeplink.port (macOS delivers via Apple events). Harmless but could be guarded to non-macOS to avoid the stray thread/file.

🎨 Nits

• Heavy fully-qualified references in TabbedTerminal.kt (ai.rever.bossterm.compose.auth.…) — consistent with the existing share.… usage in that file, so fine, just verbose.
• Embedded/tabbed examples keep fun main() and never call DeepLinkHandler.install, so deep links won't work there — expected for examples, just noting it's intentional.

🧪 Test coverage

Good for the pure logic and the socket. Acknowledged gaps that are fine to ship with, given they're called out in the description: the network half (sendMagicLink/verify/refresh) is manual-only, OS-delivery (macOS Apple event, Windows HKCU, .deb/snap MimeType) has no automated coverage, and RPM scheme registration is unhandled. The manual checklist in the PR body covers these — just make sure the RPM gap stays tracked.

Overall a solid, security-conscious implementation. Addressing #1 (secret-file perms window) is the most worthwhile follow-up, since it undercuts the same protection you took pains to get right elsewhere.

Note: I reviewed the diff statically and did not run the build (per CLAUDE.md the app isn't run during review); the PR states the full build and the :compose-ui:desktopTest auth suite are green.

🤖 Generated with Claude Code

[claude]

Review: feat(auth): Sign In via BossConsole account in the Share menu

Reviewed all 16 files. This is a high-quality, unusually well-documented PR — the security reasoning (chmod-600 staging temp, constant-time secret compare, RFC-3986 plus-preservation, boundary-anchored token scanning) is sound and backed by targeted tests. The single-instance forwarding state machine and its regression tests are particularly good. Below are findings, mostly minor; nothing blocks merge.

Bugs / correctness

1. .desktop Exec regex matches every Exec= line, including [Desktop Action] groups. In fixDesktopFileInDebPackage, the (?m)^(Exec=.*?)( %[UufF])?\s*$ replace is global. If jpackage ever emits action sub-launchers (or a future desktop file gains them), each gets %U appended. Today jpackage produces a single Exec=, so this is latent, but anchoring to the first match or to the [Desktop Entry] group would be safer.

2. Cold-start verify failure is invisible until the window is opened. emitSignIn only fires on success; a failed cold-start deep link (an expired link is the common case) lands in AccountState.Error with no window listening. The menu still reads "Sign In…" (the label only reflects SignedIn), so the user gets no signal that their click failed — they must reopen the window to see the error. The toast path already handles success-when-no-window; consider an equivalent for the error case, or document it as accepted.

3. parseAuthDeepLink rejects a trailing slash. uri.path != "/verify" means bossterm://auth/verify/?token_hash=... returns null. Probably fine since the redirect URL is fixed, but a one-line normalization (trimEnd of slash) would harden it against backend drift.

Design / follow-ups (partly acknowledged in comments)

4. No refresh-on-401 anywhere. restoreSession offline branch keeps SignedIn with a possibly-expired access token, and the code comment correctly flags that the first authenticated-endpoint consumer must handle 401-to-refresh. Nothing consumes the token in this PR so it is harmless now, but this is a real trap for the next feature — worth a tracking issue so it is not forgotten.

5. Resend cooldown is per-window and resets on reopen. cooldownUntil/nowMs are remember-scoped to the SignInWindow instance, so closing and reopening clears the 60s lock, and a second window would not share it. The server-side GoTrue 429 is the real gate (and is mapped correctly), so this is cosmetic — but the UI claim of a 60s lock is not actually enforced.

6. DeepLinkSocket listener runs on macOS too. startPrimaryListener is called unconditionally in DeepLinkHandler.install, but macOS delivers via Apple Events, so the loopback listener and deeplink.port file are dead weight there. Harmless, but gating on isMacOS would avoid an unused owner-only file in ~/.bossterm.

Security

• Anon key hardcoded as a fallback: correctly documented as public-by-design (RLS is the gate), byte-identical to BossConsole existing public fallback, with env/property override for rotation. OK
• WindowsProtocolRegistrar uses ProcessBuilder(list) (no shell), so the exe path cannot be shell-injected. OK
• Token storage: chmod-600 on the empty staging temp before bytes are written, atomic move, never logged, separate from settings.json. The restrict-while-empty test is a nice touch. OK
• Loopback-only socket + 128-bit secret + constant-time compare + bounded read; the trust model (same-user processes are already inside the boundary; a forged link only redeems a token the caller already holds) is articulated and correct. OK

Test coverage

Strong for the pure/stateful logic: deep-link parsing (token vs token_hash, verbatim type, literal plus, access_token/refresh_token decoy rejection, nested question-mark), error mapping, storage round-trip + corruption + 600-perms, and the full socket claim/forward/stale/no-usurp/wrong-secret/wrong-scheme matrix. The network half (BossAccountManager send/verify/refresh) is untested-by-design and deferred to manual verification — reasonable given the backend dependency, but restoreSession refresh/offline branching is the riskiest untested logic; a fake gotrue seam would let it be unit-tested later.

Style / conventions

• Fully-qualified inline references in TabbedTerminal.kt match the existing style there — consistent, though imports would read cleaner.
• Platform checks correctly route through ShellCustomizationUtils per CLAUDE.md. OK
• Commit/PR follow the repo no-Co-Authored-By rule. OK

Overall: clean, defensive, and well-tested where it matters. My only address-before-relying-on-it item is #4 (refresh-on-401), and that is a future-consumer concern rather than a defect in this PR.

Generated with Claude Code