Skip to content

feat(auth): encrypt the OIDC SSO clientSecret at rest (#8009) - #8223

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-8009-sso-client-secret-encrypt-at-rest
Aug 12, 2026
Merged

feat(auth): encrypt the OIDC SSO clientSecret at rest (#8009)#8223
os-zhuang merged 1 commit into
mainfrom
claude/issue-8009-sso-client-secret-encrypt-at-rest

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8009

The OIDC clientSecret stopped being stored in cleartext. It now lives on the engine's encrypted credential channel, and /sso/callback still gets the plaintext it needs, so federated login is unchanged.

What was measured first, before any design

The dispatch made one thing gating: does Field.secret() actually encrypt, and where does the key come from? Inventing a key-management mechanism would have been a maintainer-floor decision, so this was measured before a line was written.

A complete mechanism already exists, and is production-wired. Nothing new was invented:

PieceWhere
ICryptoProvider contract (encrypt / decrypt / rotateKey / digest)packages/spec/src/contracts/crypto-provider.ts
LocalCryptoProvider — AES-256-GCM keyed off OS_SECRET_KEY, fails loud in production rather than minting an ephemeral keypackages/services/service-settings/
Host injection pointpackages/cli/src/commands/serve.ts (dataEngine.setCryptoProvider)
Encrypt-on-write, fail-closedobjectqlencryptSecretFields
Cipher store, with kms_key_id / version / rotated_at for rotationsys_secret
Privileged read-backengine.resolveSecretField() — added by #7799 for exactly this, and the #7823 shape the ruling named

Measured round trip on sys_sso_provider itself: at rest the column holds secret:sec_1, the generic read path returns the mask, and resolveSecretField returns the plaintext. KMS/Vault providers plug into the same interface, so managed custody needs no change here.

The shape

Ruled option 1 — split only clientSecret, leave the rest of the blob readable so the admin UI can still render endpoints, scopes and mapping.

register / update-provider
-> adapter create / update / updateMany
-> clientSecret LIFTED out of the oidc_config JSON into
sys_sso_provider.oidc_client_secret (type: 'secret')
-> the ENGINE encrypts it, writes a sys_secret ciphertext row,
and leaves an opaque secret: ref on the column
-> oidc_config persists the same blob MINUS clientSecret
/sso/callback and every other better-auth read
-> adapter findOne / findMany
-> plaintext recovered via engine.resolveSecretField() and put
BACK into the blob better-auth receives

Three things this deliberately does not do:

Both write doors are covered. A create-only seam would encrypt at registration and then write cleartext back on the first config edit — the worst shape, because the column would then only look protected. /sso/update-provider was verified live: it returns 200 and the rotated secret is what reads back.

Migration disposition for existing rows

Required by the ruling, so stated plainly.

Rows written before this change keep their cleartext clientSecret inside oidc_config. Two things happen:

  1. They keep working. The read half detects an un-migrated row and hands better-auth the secret from the blob, so no existing federated login breaks on upgrade.
  2. They are swept forward at start.scheduleLegacySsoSecretMigration rewrites every such row through the engine, which lifts the secret into the encrypted column and drops it from the blob. Bounded (SSO providers are env-global admin config, a handful of rows), idempotent, and it re-runs when the host wires the CryptoProvider — because plugins run inside kernel:ready while serve.ts injects the provider only after runtime.start() returns, the same race plugin-webhooks handles for Regression from #7799: for ~60s after every restart, a webhook holding an encrypted signing secret silently drops its subscription — no delivery, no sys_http_delivery row, while it still reads active:true #8022.

What happens to a row that is not migrated: it keeps working and stays cleartext, and it is reported with a warning naming the row and the reason. It is never rewritten into a half-migrated state, and it is never silently left cleartext behind a column that claims to be encrypted. The only way to reach that state is an environment with no CryptoProvider — where new registrations are refused outright anyway.

Verification

Predict-then-mutate ablation. The prediction was written down first, then the seam was broken in two places. Both halves were predicted plain red in different tests — a seam with only one observable direction would be indistinguishable from a test that asserts nothing.

MutationPredictedMeasured
A — write half offat-rest tests red; round trip stays green (plaintext is back in the blob, so the legacy fallback serves it)exactly that — ① red with the plaintext found at rest, ② green. Plus one red not predicted: ④ fail-closed went expected 200 not to be 200, because with no secret field written the engine has nothing to refuse
B — read half offround-trip tests red, clientSecretabsent not wrong; at-rest stays greenexactly that — ②③⑤ red with expected undefined to be '...', ① green

Neither produced "more diagnostics" or an inverted direction; there is no counting gate downstream of this seam and no canonical-first ?? chain that could reroute the verdict.

Same-source contamination. The expected plaintext is a literal chosen by the test and fed in through the public /sso/register endpoint; nothing in the implementation supplies it. The at-rest assertion greps the raw driver row, every column serialized, so moving the secret into some other cleartext column would still fail it — an oidc_config-only check would not. It also asserts the column holds a secret: ref and that the sys_secret ciphertext is not the plaintext. Ablation B's failure message (expected undefined to be 'super-secret-...') is direct evidence the comparison is against the test's own constant, not against anything the seam produced.

Nine cases: no plaintext in any column after register; correct plaintext recovered through better-auth's own adapter the way the callback reads it; the update door; fail-closed with no CryptoProvider; legacy row still logs in, migrates, and the sweep is idempotent; plus partial-update, wrong-object, malformed-blob and column-type unit cases.

pnpm --filter @objectstack/plugin-auth test -> Test Files 46 passed (46) / Tests 1078 passed (1078)
pnpm --filter @objectstack/plugin-auth typecheck -> clean
check:nul-bytes / check:docs-audit-scope / check:test-source-alias
check:query-options-erasure / check:engine-double-contract -> OK
check:type-check-debt (repo root, after a full 70/70 build closure) -> OK, none above ceiling
(plugin-auth TEST_DEBT 131 recorded, 111 measured — the new test adds no type errors)

One thing for review: where the column is declared

The field is declared from plugin-auth's manifest via objectExtensions, not on the object file. Worth a deliberate look, because it is the one judgment call here.

  • sys_sso_provider's definition file lives in packages/platform-objectsdomain:metadata's package — and this lane was told to stop rather than write into it.
  • But the object is registered and owned by plugin-auth (authIdentityObjects, commented "Identity objects owned by plugin-auth"), and the mechanism is domain:identity's. The PM's own re-route reasoned that "the object file is the marking, not the mechanism".
  • objectExtensions is a first-class defineStack key, and was measured to behave identically to an inline declaration in the exact production ownership shape (same package owning and extending): DDL column created, encrypt-on-write fires, mask on read, privileged dereference works.

The honest cost: the spec documents objectExtensions as being for "objects owned by other packages", and a reader of sys-sso-provider.object.ts will not see this column there — which is a mild version of the declaration-does-not-match-reality problem this very card is about. Recommendation: consolidate the declaration onto the object file when the already-queued domain:metadata card opens it to fix the :99 helpText (measured false on this card). That is a move of about five lines with no behaviour change; everything else here is declaration-site-independent.

Not in scope


Generated by Claude Code

`sys_sso_provider.oidc_config` stored the OIDC `clientSecret` byte-for-byte in
cleartext — measured on the real write path in #8009 step 0. That secret
authenticates the platform to the IdP, and the object is readable through the
generic data API (`apiMethods: ['get','list']`).
Per the 2026-08-12 maintainer ruling this is option 1: split ONLY `clientSecret`
out of the blob, into a `Field.secret()`-backed column, leaving endpoints/scopes/
mapping readable for the admin UI. Not option 2 (whole blob encrypted) and not
option 3 (redact on read, which leaves cleartext at rest).
- `sso-client-secret.ts` — the seam between better-auth and its adapter. It does
not encrypt anything itself: it writes cleartext into the `secret`-typed column
exactly once and lets the engine's `ICryptoProvider` path wrap it, inheriting
the engine's fail-closed posture. Same shape as `webhook-secret.ts` (#7799) and
the same privileged accessor (#7823).
- Both write doors covered: adapter `create` (/sso/register), `update`
(/sso/update-provider) and `updateMany`. A create-only seam would write
cleartext back on the first config edit.
- Decrypt-on-read is mandatory, not optional: `/sso/callback` reads the blob
expecting plaintext, so `findOne`/`findMany` re-inject it via
`engine.resolveSecretField()`. Encrypt-only would break every federated login.
- Existing rows are migrated forward at start, with the #8022 crypto-provider
race handled the way plugin-webhooks handles it. An un-migrated row keeps
working and is reported, never silently left looking protected.
The column is declared from plugin-auth's manifest via `objectExtensions`
because the object's definition file lives in `packages/platform-objects`
(`domain:metadata`'s package) while this object is registered and owned by
plugin-auth. Consolidating it onto the object file is a move, not a behaviour
change — see the PR body.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 9:18pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx(via @objectstack/plugin-auth)
  • content/docs/deployment/production-readiness.mdx(via @objectstack/plugin-auth)
  • content/docs/kernel/contracts/cache-service.mdx(via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-auth)
  • content/docs/permissions/authentication.mdx(via @objectstack/plugin-auth)
  • content/docs/permissions/sso.mdx(via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx(via @objectstack/plugin-auth)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-auth)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-auth)
  • content/docs/releases/v9.mdx(via @objectstack/plugin-auth)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 12, 2026 21:39
@os-zhuang
os-zhuang added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit b41f51aAug 12, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8009-sso-client-secret-encrypt-at-rest branch August 12, 2026 21:56
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security] sys_sso_provider.oidc_config carries the OIDC clientSecret in a cleartext JSON textarea, by its own field description

1 participant

@os-zhuang