Skip to content

fix(platform-objects): cite what enforces client_secret's hashed-at-rest claim - #8354

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-8313-client-secret-description-citation
Aug 13, 2026
Merged

os-zhuang merged 1 commit into
mainfrom
claude/issue-8313-client-secret-description-citation

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #8313

What changed

One field description, packages/platform-objects/src/identity/sys-oauth-application.object.ts:304 (sys_oauth_application.client_secret): the old text asserted "hashed/encrypted at rest" with nothing backing it up. The new text cites what actually enforces it, so a declaration-reading survey can confirm the claim from the storage side instead of re-measuring it a third time (this is the cross-lane remainder of #8011, which already verified by real round-trip that the column stores a SHA-256 digest and never receives cleartext).

- description: 'OAuth client secret (hashed/encrypted at rest)'
+ description: 'OAuth client secret — stored as a SHA-256 digest, never plaintext (`@better-auth/oauth-provider`'s `storeClientSecret`, which defaults to hashed whenever the jwt plugin is enabled; wired in plugin-auth's `AuthManager.buildPluginList()`, oidcProvider branch). Shown once at registration.'

sys_scim_provider is untouched — it already cites storeSCIMToken: 'hashed' in its object docstring, which is exactly why the earlier survey scored it near-closed while this field stayed open.

Citation drift note

The card's suggested wording named auth-manager.ts:2473 for the wiring site. That line has drifted — it currently falls inside the phoneNumber plugin block, not the OIDC-provider one. The actual jwt + oauthProvider registration is in AuthManager.buildPluginList()'s if (enabled.oidcProvider) branch, currently around auth-manager.ts:2606-2611. The field description now cites the symbol (AuthManager.buildPluginList(), oidcProvider branch) rather than a line number, so it does not go stale the same way.

Verified directly against the installed @better-auth/oauth-provider@1.7.0-rc.2 dist (not docs): storeClientSecret() (dist/resource-challenge-2qgK1B-a.mjs) has no cleartext branch — every path hashes, encrypts, or throws (Unsupported storeClientSecret type). The default resolves via opts.storeClientSecret ?? (opts.disableJwtPlugin ? "encrypted" : "hashed"), and neither storeClientSecret nor disableJwtPlugin is set anywhere in packages/, with the jwt plugin unconditionally registered alongside oauthProvider — so the default resolves to "hashed", matching the existing regression pin (credential-at-rest-posture.test.ts, #8192 / PR #8305).

Not comment-only in the narrowest sense — a generated i18n bundle needed a hand-edit

The field description is extracted into packages/platform-objects/src/apps/translations/en.objects.generated.ts (the help key under client_secret), so this is not a pure single-file comment change. Important nuance, worth flagging for the next person who assumes "it's just a comment": pnpm check:i18n / check-i18n-bundles.mjs --write runs the extractor in merge mode — it treats every already-committed value (English included) as the translation baseline and preserves it across regeneration; it only reacts to added or dropped keys, never to a changed default/description string. Running --write after this edit reports "in sync" and touches nothing, because client_secret's key already exists with a value. So the generated English bundle would have silently kept the old, uncited text forever unless hand-edited.

Per AGENTS.md's own guardrail table ("Translation values are hand-written and expected to be: … editing a string is fine, while adding or dropping keys is drift"), the fix is a direct hand-edit of the help value in en.objects.generated.ts to match the new source description verbatim — not a generator run. Done here; check:i18n (self-test + full scan, all 9 gated packages) is green afterward. The three translated locales (es-ES, ja-JP, zh-CN) are independently hand-maintained values under the same key and are deliberately left untouched — they still convey the same underlying fact ("hashed"/"encrypted at rest"), and check:i18n never compares a translation's content against the English source, so leaving them unsynced is not drift.

Carried forward for future auditors (not acted on in this PR)

Two ⚠️ notes from #8313's body, recorded here because a later audit applying either mechanically would manufacture a false finding on a clean surface:

  1. [security] verify the unverified "hashed / encrypted at rest" claims on sys_scim_provider.scim_token and sys_oauth_application.client_secret — both are plain Field.text and API-readable #8011's acceptance-bar sentence misfires if read literally. It states "a round-trip that reproduces the stored value from the plaintext proves it is not hashed." Read literally that is backwards — unsalted SHA-256 is reproducible from the plaintext by construction (that's how every hash verifier, including better-auth's own, works) — so the literal sentence would score every correctly-hashed credential in the repo as unhashed. The discriminator actually applied was whether the stored value equals the plaintext or is reversible to it; neither holds here (better-auth's only inverse is recompute-and-compare via constantTimeEqual, never a decrypt).
  2. scim_token's stored digest covers the inner BASE token, not the returned bearer. The bearer handed to the IdP admin is base64url("BASE:providerId:organizationId"), but the stored digest is over BASE alone. An audit that hashes the full returned bearer and compares will get a false negative on a correct implementation.

Scope

Comment-only in behavioural terms — no type, apiMethods, or other file touched. sys_scim_provider is deliberately untouched (rule 4 on the card): it already carries its citation.

Verification

  • pnpm --filter @objectstack/platform-objects build — green.
  • pnpm --filter @objectstack/platform-objects test — 17 test files, 351 tests, all passing.
  • pnpm check:i18n (self-test + full scan) — green, 9/9 gated packages in sync, no undeclared authoring keys.
  • pnpm lint — green.
  • Every check:* step in .github/workflows/lint.yml's ESLint job (43 steps) — all green.
  • node scripts/pm/dispatch-gates.mjs (given the two changed paths) re-derived against the actual diff — surfaces check:cross-package-test-inputs (already run, green) and otherwise confirms this is a packages/platform-objects-scoped change with no other convention-scoped gate mechanically discoverable. check:i18n itself lives in the typecheck job (not named by path-derivation, since the gate walks packages/ dynamically rather than matching a path literal) but was clearly implicated by the diff and run directly, see above.
  • check:i18n-coverage could not be measured locally — it hard-fails with "1 of 12 configs failed to lint" because examples/app-showcase's config fails to load @objectstack/connector-mcp (no build output in this scoped-build worktree), unrelated to this change. Not chased further per "targeted gates locally, the full farm is CI's job" — CI does a full pnpm build first.

Generated by Claude Code

…t_secret's hashed-at-rest claim

The field description asserted "hashed/encrypted at rest" without citing what
enforces it, so a declaration-reading survey could not confirm the claim from
the storage side and had to re-measure it — this is the second time (#7902,
then #8011). #8011 already verified by real round-trip that the column
stores a SHA-256 digest and never receives cleartext; this adds the citation
(@better-auth/oauth-provider's storeClientSecret, wired in plugin-auth's
AuthManager.buildPluginList() oidcProvider branch) so a third re-measurement
is unnecessary.

The card's suggested auth-manager.ts:2473 citation had drifted (that line
now falls inside the phoneNumber block); the description cites the symbol
instead. Also hand-updates the generated English i18n bundle's mirrored
"help" string, since check:i18n's merge-mode extractor preserves existing
values across regeneration and would not have picked up the source change.

Fixes #8313
@vercel

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 13, 2026 8:19am

Request Review

@os-zhuang os-zhuang added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/xs labels Aug 13, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/platform-objects.

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

  • content/docs/plugins/packages.mdx (via @objectstack/platform-objects)
  • content/docs/ui/setup-app.mdx (via @objectstack/platform-objects)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants