Harden sign-in, and show what a Bot is doing - #87
Merged
Conversation
Found in review of #67, all in the auth work itself rather than in what it replaced. Running with no sign-in was gated on NODE_ENV === "production", which is exactly backwards: NODE_ENV is unset unless somebody sets it, so a container on a VM with a hand-written env file and no identity provider served every visitor as an administrator, silently, because nothing looked wrong from the outside. It now takes an explicit OPENBOT_SINGLE_USER=true and refuses to start without one. .env.example ships that line switched on, so a clone still runs with no configuration at all, and the line is greppable in a way a default never was. The most dangerous boolean in the codebase now has a test file. accounts.issuer no longer takes NOT NULL in the same release that adds the column. A rolling deploy runs the migrations and then serves from old and new replicas at once, and an old replica inserts an account without the column: under NOT NULL the release would have broken the first sign-in of everybody who landed on a replica that had not been replaced yet. The constraint belongs to a later release, once no replica predates the column. Registered identity providers are facts about the deployment rather than about whichever administrator pasted the metadata in. Better Auth answers GET /sso/providers with the ones the person asking registered themselves and refuses a delete from anybody else, so a second administrator saw an empty screen and registered a provider that already existed, and the row cascaded from the registrar's user row, so the person who set sign-in up leaving took the company's sign-in with them. Reads and removals now go through our own admin-gated routes against the whole table, and the foreign key is set null. The client secret for a customer's directory was the one secret here not going through KEY_ENCRYPTION_KEY. The SSO plugin gives no hook, so the seam is the adapter: oidc_config and saml_config are ciphertext at rest, plaintext rows written before this still read, and OAuth access and refresh tokens use Better Auth's own encryptOAuthTokens. Sign-in left no trace at all. Nothing recorded that somebody who could edit INITIAL_ADMIN_EMAILS had granted themselves the administrator role, and revoking a person deletes the sessions that were the only evidence they had ever been here. There are now rows for signing in, for being turned away, and for the configured floor granting the role, and they never block a sign-in when the trail is down. Also: a failed registration showed its error on the page behind the dialog, so the dialog sat there looking as though the button had not worked.
The screen answered half the question. A Bot that spends two minutes in a terminal installing a package shows a blank browser, and the transcript gives it one grey line, `Ran a command rg --version`, with the output nowhere: the model saw it, decided which part mattered, and the person watching had to take its word for it. That is a poor deal on a machine holding somebody's logins. Two surfaces, saying the same thing. The transcript line stays one line and opens to show what the command printed, its exit code, and whether it was cut short or stopped, because a transcript of twenty commands is unreadable if each one dumps a screenful. Beside the screen there is now an Activity tab that fills up while the screen sits still: every command, file read, file write and listing, newest first, with a count on the tab so a Bot working elsewhere is visible without switching to it. This session only, in the browser. The record is the audit trail, which is on the server, survives a reload and is what an investigation reads. This is a window, so it needs no endpoint, no polling and no second copy of command output in the database. A saved file shows its path and size and never its contents, for the same reason the write route declines to echo them: a Bot may be saving something it was told in confidence, and a value repeated into a pane lives in one more place than it should.
The changelog carries the history and the README stays about getting started. Four things moved: running with no sign-in takes a flag rather than a NODE_ENV guess, the issuer constraint is deferred to a later release and the reason is now written down where the next person will hit it, registering an OIDC provider needs its discovery endpoints trusted, and there is a second surface beside the screen showing what a Bot ran.
davidmckayv
requested review from
MikeRyanDev,
guidovizoso and
tylerslaton
as code owners
August 21, 2026 15:54
guidovizoso
previously approved these changes
Aug 21, 2026
Four passages explained a setting by describing what it used to be. That is the changelog's job.
This was referenced Aug 21, 2026
NathanTarbert
added a commit
to NathanTarbert/OpenBot
that referenced
this pull request
Aug 21, 2026
…otKit#91) Two fields moved in core.ts in CopilotKit#87 and meta/0005_snapshot.json was not regenerated, so drizzle-kit generate kept writing a migration nobody asked for and the migrations drift probe kept failing on the dirty tree, on main and on every branch cut from it. The emitted DDL is a no-op against any deployment. accounts.issuer is absent from 0000_schema.sql, added nullable by 0002_sign_in.sql and filled by 0003, and SET NOT NULL was never applied, so DROP NOT NULL drops a constraint that is not there. 0004 already dropped and re-added the sso_providers foreign key with ON DELETE set null, so re-landing it lands it on the constraint it has. The file says all of that at the top, so nobody later goes looking for the schema change it was written for. drizzle-kit check passes and generate now reports no changes, which is what the probe asserts.
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
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.
Two pieces of work. Five findings from a review of #67, and the surface that answers "what is it
doing" when the Bot is not in a browser.
The five
Running with no sign-in was gated on
NODE_ENV === "production", which is exactly backwards.NODE_ENVis unset unless somebody sets it, so the one deployment the check existed to catch, acontainer on a VM with a hand-written env file and no identity provider, was the one it let through:
every visitor on the internet served as an administrator, silently, because nothing looked wrong from
the outside. It now takes an explicit
OPENBOT_SINGLE_USER=trueand refuses to start without one..env.exampleships that line switched on, so a clone still runs with no configuration at all, andthe line is greppable in a way a default never was. The most dangerous boolean in the codebase had no
test; it has a file now.
accounts.issuerno longer takesNOT NULLin the same release that adds the column. A rollingdeploy runs the migrations and then serves from old and new replicas at once, and an old replica
inserts an account without the column: under
NOT NULLthe release breaks the first sign-in ofeverybody who lands on a replica that has not been replaced yet.
0004is gone and the rule iswritten down in
docs/development.md, where the next person will hit it.A registered identity provider belonged to whoever registered it. Better Auth answers
GET /sso/providerswithproviders.filter(p => p.userId === session.user.id)and refuses a deletefrom anybody else, so a second administrator opened the screen, found it empty, and registered a
provider that already existed. The row also cascaded from that person's user row, so deleting the
administrator who set sign-in up deleted the company's sign-in with them. Reads and removals now go
through our own admin-gated routes against the whole table, and the foreign key is
set null.A customer's client secret was in the clear. The SSO plugin writes
oidc_configandsaml_configas plaintext JSON with the OAuth client secret inside: the one secret here not going through
KEY_ENCRYPTION_KEY. The plugin offers no hook, so the seam is the storage adapter. Rows writtenbefore this still read and are re-encrypted on the next write. OAuth access and refresh tokens use
Better Auth's own
encryptOAuthTokens, deliberately theirs rather than hand-rolled inside somebodyelse's storage layer.
Sign-in left no trace at all. Nothing recorded that somebody who could edit
INITIAL_ADMIN_EMAILShad granted themselves the role, and revoking a person deletes the sessionsthat were the only evidence they had ever been here. There are now rows for signing in, for being
turned away, and for the configured floor granting the role, and a trail that is down never blocks a
sign-in.
Watching a Bot work
The screen answered half the question. A Bot that spends two minutes in a terminal shows a blank
browser, and the transcript gives it one grey line per command with the output nowhere: the model
saw it, decided which part mattered, and the person watching had to take its word. That is a poor
deal on a machine holding somebody's logins.
The transcript line now opens to show what the command printed, its exit code, and whether it was cut
short or stopped. Beside the screen there is an Activity tab carrying every command, file read, write
and listing as they happen, with a count on the tab so a Bot working away from the browser is visible
without switching to it. A saved file shows its path and size, never its contents, matching the write
route.
This is a window, held in the browser for the open conversation. The record is the audit trail.
Where it runs
sso_providers.user_idbecomesset null, andaccounts.issuerstays nullable.and deliberately not shared: it is a view of one person's open tab, not a record.
format the credential vault uses.
Proof
Driven in a browser on a real deployment with Google configured:
SUPER-SECRET-ACME-VALUE-12345appears in zero rows ofsso_providers, and that the column holds a version-1 envelope.user_idnull, still listed, andstill removable from the screen, which Better Auth's own route would have refused.
session.signed_inlanded with the address on it.identity_provider.removedlanded naming who did it.checked both surfaces:
Exit code 2and the stderr in the pane and behind the chevron.bun test: 606 pass inserver, 114 inapp, with the two pre-existingserver-side-tools.integrationfailures that are identical onmain.bun run typecheckclean.bun run format:checkclean.drizzle-kit checkreports everything's fine and the journal agreeswith the directory.
Docs updated:
CHANGELOG.mdcarries the history,README.mdstays about getting started, anddocs/configuration.md,docs/architecture.md,docs/deployment.mdanddocs/development.mdeachtake the part that is theirs. One operational note found by driving it: registering an OIDC provider
needs every host in its discovery document in
TRUSTED_ORIGINS, not only the issuer.