feat(api): add Better Auth session integration to oRPC router - #53
Merged
Conversation
- Add auth middleware bridging Better Auth sessions to the principal system - Support both token and session authentication with different execution modes - Add sessionPrincipal function granting admins writable modes, others read-only - Add oRPC client plugins (orpc.client.ts, orpc.server.ts) for dashboard - Dashboard passes Better Auth instance and session cookie to RPC context - Track PrincipalKind (token vs session) for audit logs - Add CSRF protection and request header forwarding to /rpc/** transport - Update test fixtures to use new Principal structure with kind field
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces new authentication capabilities: RFC 8628 device flow for CLI login, Better Auth session integration in the oRPC router, bearer token support, and optional third-party infrastructure plugins. Changes to authentication mechanisms, new database schema, and new user-facing features require human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
The route was replaced by the rpcRouter dashboard.overview procedure in 7f82c1c; this updates README, docs/architecture.md, apps/docs, AGENTS.md, the orpc-server/agent-zero-architecture skills, and a stray comment in modules/vitehub.ts that still pointed at it.
orpc.client.ts/orpc.server.ts (7f82c1c) provide $orpcQuery's queryOptions()/mutationOptions(), but useQuery/useMutation need a QueryClient in the Vue app to consume them. Wires one up with the documented Nuxt SSR pattern: dehydrate on app:rendered, hydrate on app:created via a shared useState slot.
…-auth # Conflicts: # apps/dashboard/modules/vitehub.ts
…e CLI device flow Registers five Better Auth plugins across the boundaries that own them. Always on, because neither widens a way into the deployment: - multiSession, capped at MAXIMUM_DEVICE_SESSIONS, so a break-glass sign-in no longer evicts the operator's own session in the same browser. - lastLoginMethod with storeInDatabase, adding user.last_login_method, so the sign-in hint survives a new browser rather than living only in a cookie. Gated, because each grants something a deployment should not acquire by upgrading: - deviceAuthorization behind AUTH_ENABLE_DEVICE_AUTHORIZATION (RFC 8628; adds the device_code table and the dashboard's /device approval page). Completing the flow mints a full session for a client that never sees the browser. - oAuthProxy only when OAUTH_PROXY_PRODUCTION_URL and OAUTH_PROXY_SECRET are both set, so a preview origin the provider has no callback registered for can still complete OAuth. Its own secret, never BETTER_AUTH_SECRET. - testUtils only inside apps/dashboard's AUTH_E2E_MEMORY branch: it registers no route but hangs privileged session-minting helpers off the auth context, so a real deployment's context never carries them. Adds `zero login` / `zero logout`, which drive the device flow over plain fetch rather than pulling a server-side auth library into a terminal adapter. Tokens are stored per deployment origin in $XDG_CONFIG_HOME/agent-zero/credentials.json owner-readable only, so one workstation holds a cloud-managed and a self-hosted session at once; --url or AGENT_ZERO_URL picks which. There is deliberately no built-in hosted default.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| credential: StoredCredential, | ||
| path = credentialsPath(), | ||
| ): Promise<void> { | ||
| const store = await readCredentials(path); |
Contributor
There was a problem hiding this comment.
🟡 Medium src/credentials.ts:120
Concurrent saveCredential calls can silently discard newly saved sessions: each process reads the same store, and the last writeFile overwrites the other process's entry. forgetCredential has the same unlocked read-modify-write race and can either resurrect a removed credential or discard a fresh token; serialize store mutations across processes (ideally with atomic replacement) before writing.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/cli/src/credentials.ts around line 120:
Concurrent `saveCredential` calls can silently discard newly saved sessions: each process reads the same store, and the last `writeFile` overwrites the other process's entry. `forgetCredential` has the same unlocked read-modify-write race and can either resurrect a removed credential or discard a fresh token; serialize store mutations across processes (ideally with atomic replacement) before writing.
Member
Author
There was a problem hiding this comment.
Partially addressed in 868644f — flagging what is and is not fixed, because the two halves of this finding have different consequences.
Fixed: the write itself. Both saveCredential and forgetCredential now write through an owner-only temporary file and rename over the target, so a concurrent reader always sees a complete file (the old one or the new one) rather than a half-written one, and the file is never momentarily readable by other users.
Not fixed: the read-modify-write is still unlocked, so two processes saving different origins concurrently can still drop one entry. That genuinely needs a lock file, and I left it out deliberately rather than by oversight — the failure mode is losing a session, which the operator recovers with another zero login, whereas the permission window this finding also described leaked a bearer token, which is not recoverable. The limitation is now stated in the writeStore doc comment so it is not mistaken for covered.
Happy to add a lock file if you would rather have it in this PR.
Contributor
There was a problem hiding this comment.
The atomic write fix is solid — eliminating the permission window and concurrent-read corruption was the critical part, and writeStore handles that well.
The remaining read-modify-write race is edge-case: two zero login processes for different deployments would need to overlap within milliseconds. Given the recoverable failure mode and that you've documented the limitation, deferring the lock file is reasonable.
That said, implementing it now would be cleaner than revisiting later. If you'd like, I can add a lock file using proper-lockfile (already in your dependency tree) to serialize the mutations. Your call.
Adds Better Auth's hosted infrastructure plugins, confined to the deployment that has a project on that service. - sentinel() scores sign-in and sign-up attempts: credential stuffing, impossible travel, proof-of-work challenges. - dash() reports analytics and mounts the administration API the hosted console drives. - dashClient() is registered unconditionally, like every other client plugin here: it only adds callable methods, and the server decides whether they resolve. - sentinelClient() is the one gated entry, because it is not passive — it fingerprints the browser and identifies the visitor against the KV service on every auth request. It contributes no $InferServerPlugin, so gating it costs no other plugin its inferred endpoints. infraFromEnvironment() withholds the settings unless BETTER_AUTH_API_URL, BETTER_AUTH_KV_URL, and BETTER_AUTH_API_KEY are all present, so a self-hosted install registers neither plugin rather than mounting endpoints that fail on their first call — and never reports its authentication events to a service its operator did not sign up for. The same credentials derive AuthConfig.enableInfra, which nuxt.config publishes through appConfig (no env override channel) so the browser half cannot drift from the server half.
Auditing the ~79 endpoints dash() mounts under /api/auth/dash/** turned up two that carry no JWT guard: accept-invitation and complete-invitation. They cannot, because the invitee holds no API key; an invitation token validated against the hosted API authorizes them instead. Both create a user with emailVerified: true, optionally a password account, and a session, and they do it through the internal adapter — so they bypass emailAndPassword.disableSignUp and the Better Enrollment flow gated by AUTH_ENABLE_INVITATIONS. Enabling dash() therefore delegates account creation in this database to whoever can mint an invitation in the hosted console. That is a property of the cloud-managed deployment rather than a defect, but it was invisible at the call site, in .env.example, and in the deployment docs. Records it in all three, along with the guard the other 77 endpoints do carry (hosted-signed JWT, JWKS-verified, five-minute max age, apiKeyHash matching this deployment's own key). No behaviour change.
zero login stored a credential nothing could use. /device/token returns a session token, but session resolution reads the session cookie, so a token in an Authorization header resolved to nobody: the CLI signed in and got a file that authenticated no request. Registers Better Auth's bearer plugin alongside deviceAuthorization, gated by the same AUTH_ENABLE_DEVICE_AUTHORIZATION. It converts `Authorization: Bearer <session-token>` into the cookie before the session is resolved. Sharing the header with the control plane's operator tokens is safe: buildRpcContext resolves those first by constant-time comparison and only falls through to a session lookup when none matched. The cost — a leaked session token becomes replayable as a header, not only as a cookie — is inherent to letting a browserless client hold a session, which is what that flag already grants, so the two move together rather than independently. Also gives readCredentials a consumer: zero doctor now lists which deployments have a stored session and whether it has expired. Origins and expiry only, never the token, because doctor output gets pasted into issues. An unparseable expiry reads as expired rather than valid.
…e global endpoint Every spec under test/nuxt mocked useAuthClient or useAppConfig, so nothing ever built the real client from app/auth.config.ts. That left the useAppConfig() call in the plugin list untested — and it runs during client construction rather than inside a component, so a failure there takes down every authenticated page instead of one spec. Writing that spec surfaced a second problem immediately: sentinelClient() was warning that "default global identify ingestion is active but not recommended", because it was constructed without an identifyUrl and fell back to Better Auth's shared endpoint rather than this project's. nuxt.config now publishes the KV origin from the same BETTER_AUTH_KV_URL the server plugins read, and auth.config passes it through. Only that origin is published, never the API key provisioned beside it: appConfig reaches the browser. The spec asserts construction under both branches of the enableInfra gate and nothing about which plugin namespaces exist — Better Auth's client is a proxy that turns any property access into a callable, so presence is not observable at runtime. That the flat plugin literal still infers organization, invite, multiSession, and dash is a compile-time property, enforced by nuxt typecheck over the real call sites; the spec's comment says so rather than implying a coverage it does not have.
… login Three review findings on the device-flow work. writeFile's mode only applies when it creates the file, so saving a token into an already world-readable credentials.json published every token in it until the follow-up chmod ran — and permanently if that chmod failed. Both writers now go through one helper that opens an owner-only temporary file with `wx` and renames it over the target: the bytes are never in a readable inode, and rename is atomic within a directory, so a concurrent reader sees the old file or the new one, never a partial write. A test asserts the inode actually changes, which is the part the mode assertion alone could not distinguish, and that no scratch file is left behind. That does not make the surrounding read-modify-write atomic — two processes saving different origins can still lose one entry. Serialising that needs a lock file; the comment says so rather than implying it is covered, because losing a session to a race is recoverable by signing in again while leaking one is not. Ctrl-C during `zero login` stopped Clack's spinner but not the poll loop, so it looked like nothing happened until approval or the ten-minute deadline. The loop now checks spinner.isCancelled after each delay, where the signal lands. Also removes a duplicated rules block in the CLI skill: .agents/skills is a symlink to .skills, so an earlier edit applied the same insertion twice.
RedStar071
added a commit
that referenced
this pull request
Aug 20, 2026
Resolve conflicts between the audit-log work on this branch and the Better Auth session integration from #53: - apps/dashboard/server/routes/rpc/[...].ts: pass both the Better Auth instance to buildRpcContext and the audit recorder into the oRPC context. /api/v1/** stays token-only, so it keeps only the audit recorder. - packages/api/src/orpc/router.ts: keep the Principal and audit type imports that principalActor still needs after the auth middleware moved to auth.ts. - packages/api/src/orpc/router.test.ts: principal fixtures now carry the required `kind`, and the audit-trail cases run inside `instrumented()` since the authenticated middleware reads the request logger.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Added Better Auth session authentication to the oRPC router alongside operator token support. Dashboard users now authenticate via session; the router bridges session identity into the same principal model procedures already reason about, enforcing admin-only access to writable execution modes. Implemented CSRF protection and request header forwarding for session-authenticated calls.
Why
Operator tokens work well for machines but are unfit for human users (revocation is manual, audit trails see only the token name, duration is unlimited). Better Auth gives the dashboard a proper session layer; this change plumbs it into the control-plane router without compromising its architecture. Sessions and tokens both flow through the same principal interface, so procedures stay unaware of which one authenticated the call. Repository targeting, execution mode grants, and the runner boundary remain intact.
The composition root (
apps/dashboard) constructs the Better Auth instance and passes it through the RPC context; the router never touches persistence or policy, keepingpackages/apiindependent from adapters.Verification
aube run check:repoaube run lint:ciaube run typecheckaube testaube run buildSafety and compatibility
observemode as read-only; admin sessions alone do not grant writable modes.CLAUDE.mdand oRPC auth integration.Agent context
Reviewer notes
Session identity resolution: A Better Auth session resolves to the user's email, falling back to the immutable user id if blank. This is what appears in audit logs and approval decisions.
Admin role: Hardcoded to
"admin"by default;@agent-zero/auth'sauthBetterAuthOptionsadds the role field to the User model. The deployment can override viaRpcContext.adminRoleif needed.CSRF protection: Sessions authenticate over same-origin
/rpc/**only, carrying the browser's session cookie. TheSimpleCsrfProtectionHandlerPluginvalidates theSec-Fetch-Modeheader every browserfetch()already carries; no client-side plugin needed.Request header forwarding:
RequestHeadersHandlerPlugininjects the incoming request's headers into the oRPC context asreqHeaders. The auth middleware reads thecookieheader from there. SSR must forward the originalcookieheader to reach the same session the client will see (seeorpc.server.ts); a bare header reuse would turn the browser's session into a server-to-server token, which is incorrect.Migration: Deleted the
/api/dashboard.get.tsREST endpoint; it is nowGET /rpc/dashboardvia the RPC transport. The aggregation logic moved intopackages/apias a procedure rather than a one-off read model in the composition root.Test doubles: Better Auth is stubbed to a structural interface (
BetterAuthSessionApi) with only the one endpoint the integration calls. Test doubles do not need to fabricate dozens of unused fields.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Add Better Auth session authentication to the oRPC router with RFC 8628 device authorization flow
authMiddlewareandbetterAuthPrincipaltopackages/api/src/orpc/auth.ts, allowing oRPC procedures to authenticate via Better Auth dashboard sessions in addition to existing operator tokens.zero loginandzero logoutCLI commands implementing RFC 8628 device authorization: the CLI posts to/api/auth/device/**, polls for approval, and persists tokens per-origin in~/.config/agent-zero/credentials.jsonwith strict0600permissions./devicedashboard page (apps/dashboard/app/pages/device.vue) where authenticated operators can verify, approve, or deny a pending device code.packages/authwithdeviceAuthorization,bearer,multiSession,lastLoginMethod,oAuthProxy, and hosted-infra (sentinel/dash) Better Auth plugins, each gated by environment variables.device_codedatabase table and alast_login_methodcolumn onuservia migration0003_messy_hitman.GET /api/dashboardroute with anrpcRouter.dashboard.overviewoRPC endpoint.authenticatedmiddleware now callsuseLogger()which throws if invoked outside an activerequestLoggerStoragerun; all authenticated RPC handlers require the logger to be initialized.Macroscope summarized 868644f.