Skip to content

Harden sign-in, and show what a Bot is doing - #87

Merged
davidmckayv merged 4 commits into
mainfrom
fix/auth-hardening
Aug 21, 2026
Merged

Harden sign-in, and show what a Bot is doing#87
davidmckayv merged 4 commits into
mainfrom
fix/auth-hardening

Conversation

@davidmckayv

Copy link
Copy Markdown
Contributor

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_ENV is unset unless somebody sets it, so the one deployment the check existed to catch, a
container 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=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 had no
test; it has a file now.

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 breaks the first sign-in of
everybody who lands on a replica that has not been replaced yet. 0004 is gone and the rule is
written 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/providers with providers.filter(p => p.userId === session.user.id) and refuses a delete
from 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_config and saml_config
as 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 written
before 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 somebody
else's storage layer.

Sign-in left no trace at all. Nothing recorded that somebody who could edit
INITIAL_ADMIN_EMAILS had granted themselves the 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 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

  • New state that outlives a request? No new tables. sso_providers.user_id becomes
    set null, and accounts.issuer stays nullable.
  • What happens on the second replica? Nothing here is per-process. Activity is per-browser
    and deliberately not shared: it is a view of one person's open tab, not a record.
  • Anything serialised? The SSO config, through the adapter wrapper, in the same envelope
    format the credential vault uses.
  • New listener, port, or schedule? None.

Proof

Driven in a browser on a real deployment with Google configured:

  • Registered an OIDC provider and confirmed SUPER-SECRET-ACME-VALUE-12345 appears in zero rows of
    sso_providers, and that the column holds a version-1 envelope.
  • Deleted the user who registered it. The provider survived with user_id null, still listed, and
    still removable from the screen, which Better Auth's own route would have refused.
  • Signed out and back in through Google. session.signed_in landed with the address on it.
  • Removed the provider through the UI; identity_provider.removed landed naming who did it.
  • Ran a Bot through commands, a file read, a file write, a listing and a deliberate failure, and
    checked both surfaces: Exit code 2 and the stderr in the pane and behind the chevron.

bun test: 606 pass in server, 114 in app, with the two pre-existing
server-side-tools.integration failures that are identical on main. bun run typecheck clean.
bun run format:check clean. drizzle-kit check reports everything's fine and the journal agrees
with the directory.

Docs updated: CHANGELOG.md carries the history, README.md stays about getting started, and
docs/configuration.md, docs/architecture.md, docs/deployment.md and docs/development.md each
take 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.

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.
guidovizoso
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.
@davidmckayv
davidmckayv merged commit 018d493 into main Aug 21, 2026
6 checks passed
@davidmckayv
davidmckayv deleted the fix/auth-hardening branch August 21, 2026 16:23
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.
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