Skip to content

feat(mac): skip OAuth on localhost — adopt bootstrap PAT#777

Closed
buremba wants to merge 3 commits into
mainfrom
feat/no-auth-mode
Closed

feat(mac): skip OAuth on localhost — adopt bootstrap PAT#777
buremba wants to merge 3 commits into
mainfrom
feat/no-auth-mode

Conversation

@buremba
Copy link
Copy Markdown
Member

@buremba buremba commented May 16, 2026

Summary

When the menu bar app starts `lobu run` on this Mac, skip the OAuth device flow entirely and adopt the bootstrap PAT that the server already writes on first boot. Click "Start" once → the server spawns, the PAT appears, the popover transitions to signed-in. No browser, no code, no approval click.

Why this works without server changes: `ensureBootstrapPat()` in `packages/server/src/start-local.ts` already creates a default user, a personal org, an owner membership, and a long-lived PAT (mode `0600`) on first boot of an empty `LOBU_DATA_DIR`. Until now the Mac app ignored that and ran OAuth anyway. This PR has it read `~/lobu/data/bootstrap-pat.txt` after `startLocalLobu()` and lift the bootstrap identity directly.

Changes

  • `LocalLobuRunner.bootstrapPATPath` — static URL to the file.
  • `AppState.adoptBootstrapCredentialsIfAvailable(baseURL:)` — synthesises `OAuthCredentials` with the bootstrap user/org details.
  • `AppState.waitForBootstrapPAT(timeout:)` — polls every 250 ms for up to 10 s (the HTTP listener comes up before `ensureBootstrapPat` writes the file).
  • `MenuBarContent.connectButtonTitle` — "Start" / "Connect" for managed-runner URLs instead of "Start & sign in."
  • Updated `docs/plans/personal-mode-auth.md` to reflect what actually shipped (Phase A only). Phase B server-side hardening (CSRF middleware, loopback bind enforcement, `LOBU_NO_AUTH=1` env) deferred — not load-bearing for the personal-use threat model.

Test plan

  • Fresh install: open menu bar → connection card shows `http://localhost:8787\` → button reads "Start."
  • Click Start → runner spawns → after ~1–2 s, popover transitions to the signed-in view with "Local Developer" / "Local Dev" org. No browser opens, no OAuth code displayed.
  • Quit Lobu, relaunch → already signed in (credentials persisted).
  • Edit URL to `https://app.lobu.ai\` → button reads "Sign in" → click → normal OAuth flow.
  • Edit URL to `http://localhost:9999\` → button reads "Sign in" (NOT "Start") → click → OAuth attempt against :9999 (runner not started).
  • If `bootstrap-pat.txt` somehow disappears mid-session → next `connect()` falls through to OAuth instead of hanging.

Defers

  • Server-side hardening (CSRF middleware, loopback bind enforcement, `LOBU_NO_AUTH=1` env). The PAT is `0600` and the server binds to `127.0.0.1` — fine for personal use. Worth revisiting if we ever change the deployment shape.

Summary by CodeRabbit

  • New Features

    • Local connection flow can adopt a runner-provided bootstrap token to sign in automatically (falls back to normal OAuth if adoption fails).
    • Connect button labels adjusted: shows "Start" when the local runner is not running and "Connect" when it is.
  • Documentation

    • Authentication plan rewritten to a phased approach: Phase A documents the local bootstrap-token flow; Phase B hardening items deferred.

Review Change Stack

The embedded server's ensureBootstrapPat() already creates a default user,
personal org, and a long-lived PAT (mode 0600) on first boot of an empty
LOBU_DATA_DIR. Until now the Mac app ignored that and ran the full OAuth
device flow against the local server anyway — browser opens, code shown,
user clicks approve. Pure ceremony when the server is on the same Mac.

This change has the Mac app read that PAT after startLocalLobu() and lift
the bootstrap identity directly: synthesised OAuthCredentials with the
bootstrap-user details + the "dev" org, no OAuth call, no browser.

- LocalLobuRunner.bootstrapPATPath: static URL pointing at
  ~/lobu/data/bootstrap-pat.txt. Kept in lock-step with the file path
  in packages/server/src/start-local.ts.
- AppState.connect() — when matchesManagedRunner(url) is true, calls
  adoptBootstrapCredentialsIfAvailable() after the runner is up. Falls
  through to OAuth if the file isn't there (we're never strictly worse
  than today's behavior).
- AppState.waitForBootstrapPAT(timeout:) — polls every 250 ms for up to
  10 s. The HTTP listener comes up before ensureBootstrapPat() writes
  the file, so "reachable" doesn't mean "PAT exists yet."
- MenuBarContent.connectButtonTitle — flips to "Start" / "Connect" for
  managed-runner URLs instead of "Start & sign in," since there's
  literally no sign-in step anymore.
- docs/plans/personal-mode-auth.md updated to reflect what shipped
  (Phase A only). Phase B server hardening (CSRF middleware, loopback
  bind enforcement, LOBU_NO_AUTH=1 env) deferred — not load-bearing for
  the personal-use threat model since the PAT is 0600 and the server
  binds to 127.0.0.1.

User experience: click "Start" once. Server spawns, PAT appears,
popover transitions to signed-in. No browser, no code, no approval.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2d102337-dd38-4551-84a6-1cf1b22420cf

📥 Commits

Reviewing files that changed from the base of the PR and between 6130b18 and 61a2cbb.

📒 Files selected for processing (1)
  • apps/mac/Lobu/AppState.swift

📝 Walkthrough

Walkthrough

The PR implements a bootstrap PAT flow for local-only startup: the macOS app detects managed-runner URLs, waits for a runner-written bootstrap PAT, validates and synthesizes OAuth credentials, persists them, starts auto-polling, and falls back to the normal OAuth device flow on failure.

Changes

Bootstrap PAT Flow for Local Server

Layer / File(s) Summary
Bootstrap PAT file path infrastructure
apps/mac/Lobu/LocalLobuRunner.swift
Introduces LocalLobuRunner.bootstrapPATPath static property that computes the filesystem URL to bootstrap-pat.txt written by lobu run.
AppState connect flow with bootstrap support
apps/mac/Lobu/AppState.swift
Refactors connect() to derive serverMode from managed-runner matching. For managed-runner URLs, attempts bootstrap credential adoption: waits for the bootstrap PAT file, verifies it via a WorkerClient call, synthesizes and persists OAuthCredentials, updates baseURL/status, starts auto-polling, and returns early on success; falls back to persisting baseURL and the normal OAuth device signIn() flow on failure or timeout.
UI label updates for bootstrap flow
apps/mac/Lobu/MenuBarContent.swift
Updates connectButtonTitle to display "Start" (local Lobu not running) or "Connect" (local Lobu running) for managed-runner URLs, replacing the prior "Start & sign in" label.
Architecture documentation: Phase A shipped, Phase B deferred
docs/plans/personal-mode-auth.md
Rewrites the plan to describe Phase A (shipped bootstrap-PAT local-only approach) and explicitly defers Phase B server hardening; updates security boundary and open questions to match the implemented approach.

Sequence Diagram

sequenceDiagram
  participant App as AppState.connect
  participant Matcher as matchesManagedRunner
  participant Bootstrap as adoptBootstrapCredentialsIfAvailable
  participant PATFile as LocalLobuRunner.bootstrapPATPath
  participant Worker as WorkerClient
  participant Keychain as CredentialStore
  participant Poll as AutoPoller
  participant OAuthFallback as signIn (device)

  App->>Matcher: check URL == managed runner
  alt managed-runner URL
    App->>Bootstrap: attempt bootstrap adoption
    Bootstrap->>PATFile: waitForBootstrapPAT()
    alt PAT found
      Bootstrap->>Worker: verify token (getUnreadCount)
      Bootstrap->>Keychain: persist synthesized OAuthCredentials
      Bootstrap->>Poll: start auto-polling
      Bootstrap-->>App: return success
    else failure or timeout
      Bootstrap-->>App: adoption failed
      App->>OAuthFallback: fallback to device auth
    end
  else non-managed URL
    App->>OAuthFallback: standard device auth
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • lobu-ai/lobu#774: Modifies macOS connection path and managed-runner detection; this PR extends that flow by adding bootstrap-PAT credential adoption.

Poem

🐰 I found a token in the dirt,
The runner wrote it, cleaned my shirt,
The app peeks in, reads what it needs,
A tiny PAT fulfills the deeds,
Start, connect — the rabbit leads.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: skipping OAuth and adopting the bootstrap PAT for localhost connections in the macOS app.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering the summary, detailed changes, test plan, and deferrals, matching the repository template requirements.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/no-auth-mode

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7236294965

ℹ️ 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".

// synthesising credentials is the entire "sign-in". If the file
// isn't there yet (race) or unreadable, fall back to OAuth so we're
// never strictly worse than before.
if autoStart, await adoptBootstrapCredentialsIfAvailable(baseURL: urlString) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid adopting a PAT for an already-running server

When port 8787 is already serving a Lobu process that this app did not spawn, LocalLobuRunner.start() returns early before setting LOBU_DATA_DIR, but this branch still reads ~/lobu/data/bootstrap-pat.txt and treats it as valid for that server. If the user's manually-started localhost:8787 uses ~/.lobu/data or any other data dir while an old ~/lobu/data/bootstrap-pat.txt exists, the app persists a bearer token from the wrong database and skips OAuth, so subsequent API/sync calls get 401 until credentials are cleared. Only adopt the bootstrap PAT when it is known to belong to the server on that port, or validate it against the server before returning.

Useful? React with 👍 / 👎.

…ng it

Pi's review of PR #777 flagged two blocking issues with the initial adopt
path:

1. The PAT was sent to whatever process happened to win port :8787.
   If something else was squatting (different user, stale process,
   misconfigured proxy), we'd silently leak the bootstrap PAT to it.

2. File existence didn't prove the PAT still worked. A reset DB or a
   revoked PAT row would let the UI transition to "signed in" and then
   break later in confusing ways.

Both fixed by calling the server's /userinfo endpoint with the PAT
before saving credentials. If the call fails (wrong server, stale PAT,
no bootstrap), we fall through to OAuth — same defensive fallback as
when the PAT file is missing.

Also fixed:
- Identity (name, email, org) now comes from the server's userinfo
  response, not hardcoded constants in the Mac app. If the server's
  bootstrap constants drift, the menu bar still shows the right info.
- Keychain save failure now returns false instead of silently
  continuing with in-memory creds that won't survive a relaunch.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
docs/plans/personal-mode-auth.md (1)

69-69: ⚡ Quick win

Simplify the reset-path claim or mark it as unverified behavior.

The parenthetical explanation is difficult to follow and reads implementation-coupled. Prefer a shorter statement like “PAT recreation after deletion/corruption should be verified by test” unless this behavior is already covered by an automated test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans/personal-mode-auth.md` at line 69, Update the sentence about the
reset path to a concise, implementation-agnostic note: replace the parenthetical
explanation referencing `if (existsSync(patFilePath)) return` and the user-count
guard with a single line such as “PAT recreation after deletion/corruption
should be verified by test” (or mark the claim as unverified) and add a TODO
referencing the planned “Reset Lobu” footer action if you want to follow up with
a manual check; ensure the text no longer ties behavior to the `patFilePath`
existence check or internal guard logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/mac/Lobu/AppState.swift`:
- Around line 449-452: The code currently swallows errors from
credentialStore.save(creds) and proceeds to set the in-memory credentials
variable, which falsely signals successful bootstrap; change the flow so that if
credentialStore.save throws you do not assign credentials or treat the operation
as successful—propagate or return the error (or call the failure completion)
instead of setting credentials = creds; specifically update the block around
credentialStore.save(creds) to only set the credentials variable after a
successful save and ensure the save error is surfaced to callers.

In `@docs/plans/personal-mode-auth.md`:
- Around line 22-30: The doc mixes two sources for the bootstrap PAT path;
update the text to either (A) explicitly state that the managed runner sets
LOBU_DATA_DIR to ~/lobu/data so LocalLobuRunner.bootstrapPATPath is always
~/lobu/data/bootstrap-pat.txt, or (B) reword to show the Mac app derives
LocalLobuRunner.bootstrapPATPath from the same LOBU_DATA_DIR env/config value
(and not a hardcoded ~/lobu/data), and ensure AppState.connect()’s behavior
(calling adoptBootstrapCredentialsIfAvailable(baseURL:)) references that derived
path contract. Choose one approach and make the contract explicit in the doc.

---

Nitpick comments:
In `@docs/plans/personal-mode-auth.md`:
- Line 69: Update the sentence about the reset path to a concise,
implementation-agnostic note: replace the parenthetical explanation referencing
`if (existsSync(patFilePath)) return` and the user-count guard with a single
line such as “PAT recreation after deletion/corruption should be verified by
test” (or mark the claim as unverified) and add a TODO referencing the planned
“Reset Lobu” footer action if you want to follow up with a manual check; ensure
the text no longer ties behavior to the `patFilePath` existence check or
internal guard logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f175655e-476e-4681-9cb3-de19722a2f09

📥 Commits

Reviewing files that changed from the base of the PR and between e98e1ea and 7236294.

📒 Files selected for processing (4)
  • apps/mac/Lobu/AppState.swift
  • apps/mac/Lobu/LocalLobuRunner.swift
  • apps/mac/Lobu/MenuBarContent.swift
  • docs/plans/personal-mode-auth.md

Comment thread apps/mac/Lobu/AppState.swift Outdated
Comment on lines +22 to +30
- A long-lived PAT, written to `<LOBU_DATA_DIR>/bootstrap-pat.txt` with mode `0600`.
- A Better Auth credential for the web SPA so "Open Lobu" works too.

Avatar / real email: not in v1. Documented in §Open questions.
Phase A in the Mac app simply **reads that file after `startLocalLobu()` returns** and synthesises an `OAuthCredentials` from it. No OAuth device flow, no browser, no code shown.

---

## Threat model
### Implementation (apps/mac/Lobu)

| Threat | Mitigation |
|---|---|
| Network attacker on LAN reaches `:<port>` | Loopback bind enforced at server (refuses non-loopback in personal mode; post-listen assertion verifies). |
| Browser tab on the same Mac CSRFs the API | Strict CORS (no foreign-origin preflight passes for `X-Lobu-Client`/`Authorization`) + Origin + Sec-Fetch-Site + Host + Content-Type checks. |
| Other process on the same Mac (same user) reads Keychain or attaches to server memory | Out of scope — same-user adversary already owns the data. |
| Other macOS user on a shared Mac hits localhost | Per-user data dir + per-user port = each user runs their own server on their own port. A sibling user can hit `127.0.0.1:<other-user-port>` but doesn't have the Keychain secret, so all sensitive endpoints 401. CSRF stack still applies. |
| Tunnel (Tailscale Funnel / ngrok / cloudflared) exposes localhost to internet | Loopback bind doesn't prevent tunnels by itself, but: (a) advisory startup warning, (b) `Host` allowlist rejects requests with non-localhost `Host` headers (most tunnels rewrite this), (c) menu bar UI flags the warning. Not a hard guarantee; documented. |
| User configures `HOST=0.0.0.0` thinking it'll just work | Server in personal mode refuses to start with non-loopback bind. Marker enforces single-mode-per-data-dir. |
| Bootstrap token leaks (browser sync, screen-share, history) | One-time use enforced by atomic `Map.delete()`. 10-second TTL. Stripped from URL via `history.replaceState()` immediately. Stored as hash, not plaintext, so server memory dump doesn't reveal usable tokens. |
| Bootstrap token replay between server restarts | Token store is in-memory only. Server restart invalidates all tokens. |
| Long-lived Keychain secret leaks | Same boundary as user's filesystem. Mitigation: revoke + regenerate is one click ("Reset Lobu" — see §Reset). |
| Cross-tab credential confusion (a tab from Cloud thinks it's local) | Cookie scoped to `Path=/`, Better Auth issues distinct session per origin. Web SPA loaded from `app.lobu.ai` can't read a localhost cookie. |

---
- `LocalLobuRunner.bootstrapPATPath` — static URL pointing at `~/lobu/data/bootstrap-pat.txt`.
- `AppState.connect()` — when `matchesManagedRunner(url)` is true, calls `adoptBootstrapCredentialsIfAvailable(baseURL:)` after the runner is up. Falls through to OAuth if the file isn't there (defensive: we're never strictly worse than today's behavior).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify bootstrap PAT path contract (LOBU_DATA_DIR vs hardcoded ~/lobu/data).

This section currently mixes two sources of truth for the PAT location. Please state explicitly that the managed runner fixes LOBU_DATA_DIR=~/lobu/data (if true), or reword to say the Mac app path is derived from the same data-dir setting.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans/personal-mode-auth.md` around lines 22 - 30, The doc mixes two
sources for the bootstrap PAT path; update the text to either (A) explicitly
state that the managed runner sets LOBU_DATA_DIR to ~/lobu/data so
LocalLobuRunner.bootstrapPATPath is always ~/lobu/data/bootstrap-pat.txt, or (B)
reword to show the Mac app derives LocalLobuRunner.bootstrapPATPath from the
same LOBU_DATA_DIR env/config value (and not a hardcoded ~/lobu/data), and
ensure AppState.connect()’s behavior (calling
adoptBootstrapCredentialsIfAvailable(baseURL:)) references that derived path
contract. Choose one approach and make the contract explicit in the doc.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Pi's previous review caught that the /userinfo discovery flow doesn't
actually verify PATs — that endpoint only validates OAuth tokens, not
the personal_access_tokens table the bootstrap PAT lives in. So the
prior verification 403'd every time and silently fell back to OAuth,
defeating the whole point of the zero-click sign-in.

Switch to a real org-scoped REST call (getUnreadCount, cheapest
auth-required endpoint that does accept PATs). Construct the WorkerClient
ourselves against the URL we already control — never follow a
server-supplied URL that could redirect the PAT off-origin.

Identity goes back to synthesised constants in lock-step with
packages/server/src/start-local.ts (BOOTSTRAP_USER_*, BOOTSTRAP_ORG_*).
There's no PAT-accepting /userinfo endpoint to query, and these are the
documented contract the server creates — if either side drifts the
constants in lock-step both files break together.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/mac/Lobu/AppState.swift (1)

402-410: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don't treat non-root localhost URLs as the managed runner.

This branch now forces .local based on matchesManagedRunner(url), but that helper only checks scheme/host/port. Inputs like http://localhost:8787/foo or http://localhost:8787?x=1 will still flip serverMode to .local and enter the bootstrap path, which breaks the “exact one our runner manages” guarantee and can auto-spawn the runner on later launches for the wrong URL.

Suggested fix
-        let autoStart = AppState.matchesManagedRunner(url)
+        let isCanonicalManagedRunnerURL =
+            AppState.matchesManagedRunner(url)
+            && (url.path.isEmpty || url.path == "/")
+            && url.query == nil
+            && url.fragment == nil
-        if autoStart && !localLobuStatus.isRunning {
+        if isCanonicalManagedRunnerURL && !localLobuStatus.isRunning {
             await startLocalLobu()
             guard localLobuStatus.isRunning else { return }
         }
@@
-        serverMode = autoStart ? .local : .remote
+        serverMode = isCanonicalManagedRunnerURL ? .local : .remote
@@
-        if autoStart, await adoptBootstrapCredentialsIfAvailable(baseURL: urlString) {
+        if isCanonicalManagedRunnerURL, await adoptBootstrapCredentialsIfAvailable(baseURL: urlString) {
             startAutoPollIfSignedIn()
             return
         }
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/mac/Lobu/AppState.swift`:
- Around line 465-482: The bootstrap path creates OAuthCredentials with the raw
baseURL which can include a trailing slash; normalize baseURL the same way
signIn() does before persisting: trim any trailing slash (and optionally
whitespace) from baseURL and use that normalized value when constructing
OAuthCredentials, when calling credentialStore.save(creds), and when calling
setBaseURL(...)/persisting to UserDefaults so bootstrap sessions match signIn()
behavior and avoid malformed or mismatched URLs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 15ad3dd2-fa38-4a88-a780-113f2b156e15

📥 Commits

Reviewing files that changed from the base of the PR and between 7236294 and 6130b18.

📒 Files selected for processing (1)
  • apps/mac/Lobu/AppState.swift

Comment on lines +465 to +482
let creds = OAuthCredentials(
baseURL: baseURL,
clientID: "menubar-local",
clientSecret: nil,
accessToken: pat,
refreshToken: nil,
expiresAt: nil,
userInfo: info
)
do {
try credentialStore.save(creds)
} catch {
NSLog("[Lobu] bootstrap adopt: keychain save failed: \(error.localizedDescription)")
return false
}
credentials = creds
setBaseURL(baseURL)
setStatus("")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize the bootstrap baseURL before saving it.

signIn() trims the trailing slash before persisting credentials, but this path stores the raw baseURL in both OAuthCredentials and UserDefaults. Entering http://localhost:8787/ will therefore persist a different base URL only for bootstrap sessions, which is an easy way to get //... request URLs or equality mismatches later.

Suggested fix
-        let creds = OAuthCredentials(
-            baseURL: baseURL,
+        let normalizedBaseURL = baseURL.trimmedTrailingSlash()
+        let creds = OAuthCredentials(
+            baseURL: normalizedBaseURL,
             clientID: "menubar-local",
             clientSecret: nil,
             accessToken: pat,
             refreshToken: nil,
             expiresAt: nil,
             userInfo: info
         )
@@
         credentials = creds
-        setBaseURL(baseURL)
+        setBaseURL(normalizedBaseURL)
         setStatus("")
         return true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let creds = OAuthCredentials(
baseURL: baseURL,
clientID: "menubar-local",
clientSecret: nil,
accessToken: pat,
refreshToken: nil,
expiresAt: nil,
userInfo: info
)
do {
try credentialStore.save(creds)
} catch {
NSLog("[Lobu] bootstrap adopt: keychain save failed: \(error.localizedDescription)")
return false
}
credentials = creds
setBaseURL(baseURL)
setStatus("")
let normalizedBaseURL = baseURL.trimmedTrailingSlash()
let creds = OAuthCredentials(
baseURL: normalizedBaseURL,
clientID: "menubar-local",
clientSecret: nil,
accessToken: pat,
refreshToken: nil,
expiresAt: nil,
userInfo: info
)
do {
try credentialStore.save(creds)
} catch {
NSLog("[Lobu] bootstrap adopt: keychain save failed: \(error.localizedDescription)")
return false
}
credentials = creds
setBaseURL(normalizedBaseURL)
setStatus("")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mac/Lobu/AppState.swift` around lines 465 - 482, The bootstrap path
creates OAuthCredentials with the raw baseURL which can include a trailing
slash; normalize baseURL the same way signIn() does before persisting: trim any
trailing slash (and optionally whitespace) from baseURL and use that normalized
value when constructing OAuthCredentials, when calling
credentialStore.save(creds), and when calling setBaseURL(...)/persisting to
UserDefaults so bootstrap sessions match signIn() behavior and avoid malformed
or mismatched URLs.

@buremba
Copy link
Copy Markdown
Member Author

buremba commented May 16, 2026

Closing in favor of a cleaner Phase B implementation — server-side LOBU_NO_AUTH=1 + CSRF middleware + loopback-bind enforcement, instead of the lift-the-bootstrap-PAT pattern this PR used. Pi's iterative review surfaced enough corners in the lift-token frame that the no-token frame is actually less code overall. New PR incoming.

@buremba buremba closed this May 16, 2026
buremba added a commit that referenced this pull request May 16, 2026
…779)

* feat(server,mac): no-auth mode for embedded server (LOBU_NO_AUTH=1)

Replaces the closed PR #777 ("lift the bootstrap PAT") with the cleaner
Phase B from docs/plans/personal-mode-auth.md: server short-circuits
auth when LOBU_NO_AUTH=1, attributes every request to the local user
ensureBootstrapPat() seeded. The macOS menu bar spawns the runner with
that env set, then sets credentials directly — no PAT to read, no
verification call, no ownership tracking.

Server (packages/server):
- multi-tenant.ts: getNoAuthUser() loads the bootstrap-user + their
  personal org once and caches; resolveAuth() short-circuits with
  owner-role attribution when LOBU_NO_AUTH=1. URL-supplied org slug
  must match the local user's org (single-org by definition).
- start-local.ts: post-listen bind assertion refuses to serve on
  anything other than 127.0.0.1 / ::1 when LOBU_NO_AUTH=1. Surfaces a
  hard error early instead of silently exposing the local user's data.

Mac app (apps/mac/Lobu):
- LocalLobuRunner sets LOBU_NO_AUTH=1 in the spawn env alongside
  LOBU_DATA_DIR.
- AppState.connect() — when targeting the managed runner, calls
  adoptLocalCredentials() with synthesised OAuthCredentials (dummy
  bearer; server ignores it). No PAT file, no userinfo verification,
  no spawnedThisSession check — none of that is needed when the
  server itself bypasses auth.
- MenuBarContent button reads "Start" / "Connect" for managed-runner
  URLs.

User experience: click Start once. Server spawns with no-auth env,
popover transitions to signed-in within ~1 s. No browser, no code, no
approval. The dummy bearer the menu bar sends is never validated; it
exists only so the existing WorkerClient/Authorization scaffolding
doesn't have to learn a "no header" mode.

Defers (still in docs/plans/personal-mode-auth.md):
- CSRF middleware on mutating routes — browser-tab exfiltration risk
  remains until we ship Origin / Sec-Fetch-Site / Host / Content-Type
  checks. Today no-auth mode trusts that the loopback bind is the
  only attack surface.
- Per-user data dir / port for shared macOS user accounts.

* fix(no-auth): address all 8 pi blockers on PR #779

Server (packages/server):
- HOST default of 0.0.0.0 conflicted with the no-auth bind guard. The Mac
  runner now explicitly sets HOST=127.0.0.1 in the spawn env (#1), and
  server.ts also gets the same loopback bind guard so an accidental
  LOBU_NO_AUTH=1 in production refuses to start instead of silently
  bypassing auth on the public bind (#5).

- ensureBootstrapPat now runs BEFORE httpServer.listen so the very first
  request can't 503 due to the seeding race (#3). The early-return now
  trusts the DB row, not the bootstrap-pat.txt file: a wiped LOBU_DATA_DIR
  with a leftover file used to leave no-auth permanently 503; now we
  re-mint the user/org/PAT (#4). Production safety guard still skips when
  OTHER (non-bootstrap) users exist.

- getNoAuthUser pins to the BOOTSTRAP_USER_ID + BOOTSTRAP_ORG_ID pair
  directly, not "first owner/admin membership LIMIT 1" — eliminates the
  nondeterminism if bootstrap-user ever gets cross-org memberships (#7).

- isLoopbackHost moved to packages/server/src/utils/loopback.ts so both
  start-local and server share it. Handles the full IPv4 loopback /8,
  ::1, [::1], and IPv4-mapped IPv6 loopback (::ffff:127.x.y.z) (#8).

- New CSRF middleware in index.ts. Only fires when LOBU_NO_AUTH=1. On
  mutating methods (POST/PUT/PATCH/DELETE) requires:
    * Host header is a loopback alias (defeats DNS rebinding).
    * Origin or Sec-Fetch-Site says same-origin/none, OR a custom
      X-Lobu-Client header is present (native clients omit Origin).
    * Content-Type, if set, must be application/json — defeats CSRF
      simple-request form posts that browsers allow without preflight.
  This is the only protection between the no-auth bypass and any
  malicious site the user visits in their browser, so it MUST be on
  whenever no-auth is on (#6).

Mac app (apps/mac/Lobu):
- LocalLobuRunner: pass HOST=127.0.0.1 alongside LOBU_NO_AUTH=1, and
  restore spawnedThisSession tracking so adoptLocalCredentials can refuse
  adoption when start() adopted a pre-existing server instead of
  spawning one. A malicious squatter or someone else's lobu run would
  otherwise receive our synthesised credentials (#2).
- WorkerClient sends X-Lobu-Client: menubar on every request so the new
  CSRF middleware accepts native client traffic that legitimately omits
  Origin (#6).

* fix(no-auth): close pi round-2 gaps — startup-leak, partial-state, CSRF holes

Pi's verification of the previous fixup commit caught three remaining
issues:

1. AppState's init re-spawns the runner and starts polling using the
   persisted no-auth credentials WITHOUT checking spawnedThisSession.
   If a squatter happened to win :8787 on startup, the polling client
   would send our synthesised "noauth" bearer + X-Lobu-Client to it.
   Now: after startLocalLobu in init, if we didn't actually spawn the
   process, clear the persisted creds and stop. The user will see the
   connection card again and can sign in via OAuth.

2. ensureBootstrapPat checked only the user row's existence before the
   early-return. Partial state (user exists, org or member rows missing)
   would still wedge getNoAuthUser forever. Now we check all three rows
   together — any missing one triggers a re-mint.

3. CSRF middleware: tightened in three ways.
   - Missing Content-Type on a mutation is now rejected (was previously
     a bypass — `if (ct && ...)` skipped when ct was empty).
   - WorkerClient.markNotificationRead now sends Content-Type:
     application/json even with an empty body to satisfy the tightened
     check.
   - OAuthClient.postRawJSON and ChromeBridgeHost.mintChildToken now
     send X-Lobu-Client: menubar so they aren't rejected for missing
     Origin in no-auth mode.
   - Host header validation reuses the shared isLoopbackHost util
     (stripping port + brackets first) so the alias set is consistent
     with the bind-time enforcement.
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.

2 participants