fix(security): No published default signing secrets (GHSA-39j7, GHSA-chm8, GHSA-4r2r, GHSA-3cgp) (#10240)

* fix(security): never sign tokens with a published default secret

GHSA-39j7-x845-4w3c (high) and GHSA-chm8-2ggf-pgjq: when JWT_SECRET,
JWT_REFRESH_TOKEN_SECRET, JWT_VERIFICATION_TOKEN_SECRET or
EXPRESS_SESSION_SECRET was unset, the API fell back to a literal published in
this repository, so anyone could forge tokens and sessions. resolveSecret() now
substitutes a per-process random value instead, and the startup guard still
reports such a secret as unset, so production keeps refusing to boot. The
known-default list is shared by the config resolver, the startup guard and the
desktop apps, and it catches a default published for any key, not only its own.

Desktop apps generated no secrets at all: every install shipped the same baked
DESKTOP_JWT_* values while binding the local API to 0.0.0.0. They now provision
random per-install secrets on first run and rotate a stored published default.

DEMO=true keeps today's behaviour, with a TODO, pending a decision on
demo.gauzy.co's secrets.

Also: the MCP refresh grant re-checks the account on every refresh instead of
trusting a 30-day-old token (GHSA-3cgp-wmrg-4fqg residual), and the misleading
comment claiming the Electron seed-credential exemption is harmless is corrected
(GHSA-4r2r-mv32-3468).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(config): resolve signing secrets lazily so load order cannot split them

Found by a runtime probe against a locally built API: login returned 200 but the
access token it issued was rejected on the next request, because the process was
signing with one secret and verifying with another.

apps/api/src/main.ts calls loadEnv() (which reads .env.local and friends) only
after its imports have run, so @gauzy/config can be evaluated while JWT_SECRET is
still unset. With the published literal as the fallback that was invisible: the
early copy and any later copy both ended up on 'secretKey'. Once an unset secret
became a per-process random value, the early copy generated one while a copy
imported after loadEnv() read the configured value — so tokens signed by one
never verified in the other, and every authenticated request 401'd.

The four secrets are now getters on `environment` / `environment.prod` /
`defaultConfiguration.authOptions`, so each is read at first use, after the env
files are loaded. Adds the regression test for exactly that order.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(security): separate an unavailable account lookup from a dead refresh token

Review pass over PR #10240. The substantive fixes:

- MCP refresh grant (CodeRabbit major, Greptile P1). `getMcpUserInfo` caught
  every error and answered `null`, so a database blip reached
  `refreshAccessToken` as "this user is gone" and the grant handler answered
  `invalid_grant`. Well-behaved OAuth clients discard a refresh token on that,
  so one transient outage signed active users out for good. `getMcpUserInfo`
  now re-throws a failed lookup, `refreshAccessToken` re-throws it as
  `UserLookupUnavailableError` (outside its own catch, so it cannot be
  flattened into `null`), and the grant answers `temporarily_unavailable` /
  503. `null` now means exactly one thing: the account can no longer sign in.
  The refresh token is still never revoked on either path.

- `resolveUser` is now REQUIRED (Greptile P2). It was optional on a public
  method, so a caller could keep the two-argument shape and silently skip the
  account check the parameter exists to perform. A missing resolver now throws
  before anything else (500 server_error), matching the missing-provider path.
  `UserLookupUnavailableError.is()` is used instead of bare `instanceof`: this
  package ships both as source and as a bundle, and a downlevelled
  `extends Error` would break the prototype chain and quietly restore the
  invalid_grant behaviour.

- Nested credentials reached the desktop logs (CodeRabbit major, CWE-532).
  `redactSecretsForLog` only looked at top-level keys, and `apps/desktop` logs
  the whole `DesktopSetupConfig`, so `postgres.dbPassword` and
  `secureProxy.ssl.key` were printed verbatim. It now walks nested objects and
  arrays, with a depth cap and cycle detection.

- The seeded-account warning missed renamed installs (CodeRabbit major).
  `getPublishedSeedAccounts()` read only today's `DEMO_*_EMAIL` values, but the
  database was seeded in the past: an operator who changed the address after
  installing still had `admin@ever.co` with the published password, and the
  check walked past it. Both the configured and the canonical addresses are now
  checked, deduplicated on the (email, password) pair.

- The bounded read could hide a vulnerable account (Greptile P2). One shared
  `IN (...)` query with a global limit of 10 let the rows of whichever address
  came back first use the whole budget. The budget is now per address (5 rows
  each), so every candidate is actually looked at, and rows are re-matched
  against the address they were fetched for.

- The lazy-getter commit (21e0bfd) broke its own regression suite:
  `validate-application-secrets.spec.ts` assigned to `environment.JWT_SECRET`,
  which is now getter-only, and all 8 tests died at "Cannot set property". The
  spec drives `process.env` instead, which is what the getters read, plus a new
  test pinning that a secret set AFTER import still decides the verdict.

Static-analysis cleanups: `config?.secret` next to an unconditional
`config.db` in both server launchers (the inconsistent null check DeepScan
flagged); the duplicated `JWT_VERIFICATION_TOKEN_SECRET` block in
.env.demo.compose; and the Sonar smells (`node:crypto`, assignment inside a
return, optional chaining, useless empty object).

Not applied: CodeRabbit's request to point the three k8s demo manifests and
fly.toml at a secret store. Those are the DEMO=true deployments this PR
deliberately holds; wiring `secretKeyRef` to Secrets that do not exist would
break the demo without changing the effective values, and the manifests already
document how to inject real ones.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(auth): correct a stale comment about the refresh account lookup

The deactivated-user test still said the wired provider answers `null` for a
transient lookup failure. It rejects now, and that case is covered by its own
test, so the comment described behaviour the suite no longer has.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(auth): drop a word cspell rejects from the token-manager comments

The Cspell check failed on "downlevelled" in two comments. Reworded to
"compiled for a pre-ES6 target", which says the same thing in words the
dictionary already has, rather than growing the project word list for prose.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(user): split the published-password check into named steps

The per-address rewrite pushed findAccountsUsingPasswords over SonarCloud's
cognitive-complexity limit (16/15) with three nested loops and an inline ORM
switch. The query and the verification loop are now their own methods, and the
candidates collapse into a Map of address -> passwords instead of being
filtered, de-duplicated and re-filtered inline. Same behaviour: one query per
distinct address, every proposed password tested, first match wins.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(core): report a non-exhaustive seed-password check instead of implying clean

The check reads a bounded number of rows per address, because it runs before
the API listens and every scrypt/bcrypt verification is expensive by design.
With the same seeded address in more tenants than the budget, a vulnerable row
can sit outside the sample — and a boot that found nothing looked exactly like
a boot that checked everything.

findAccountsUsingPasswords now returns `{ matches, inconclusive }`;
`inconclusive` names the addresses whose rows filled the budget without a
match, and boot prints a short note asking for those tenants to be audited
separately. It deliberately does NOT raise INSECURE ACCOUNTS in that case:
a scary warning on every boot of any large multi-tenant install is how a
warning gets ignored.

Raising the budget was the alternative and it does not fix anything — it only
moves the cliff, at the cost of boot time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

apps/desktop/src/index.ts

@@ -30,6 +30,8 @@ import {
 	DesktopThemeListener,
 	DesktopUpdater,
 	DialogErrorHandler,
+	desktopSecretsToEnv,
+	ensureDesktopSecrets,
 	ErrorEventManager,
 	ErrorReport,
 	ErrorReportRepository,
@@ -37,6 +39,7 @@ import {
 	LocalStore,
 	ProtocolRouter,
 	ProviderFactory,
+	redactSecretsForLog,
 	TranslateLoader,
 	TranslateService,
 	TrayIconFactory,
@@ -333,7 +336,7 @@ function setGlobalVariable(setupConfig: {
 }
 
 async function startServer(setupConfig: DesktopSetupConfig, restart = false) {
-	console.log('Starting the Server...', setupConfig);
+	console.log('Starting the Server...', redactSecretsForLog(setupConfig));
 
 	setGlobalVariable(setupConfig);
 
@@ -357,6 +360,13 @@ async function startServer(setupConfig: DesktopSetupConfig, restart = false) {
 		process.env.DB_PASS = setupConfig['postgres']?.dbPassword;
 	}
 
+	if (setupConfig.isLocalServer) {
+		// Per-install random signing/session secrets for the integrated API, generated on first start
+		// and replacing a stored published default on upgrade. Stored with the config below so restarts
+		// keep the same keys (GHSA-39j7-x845-4w3c).
+		setupConfig = { ...setupConfig, secret: ensureDesktopSecrets(setupConfig.secret).secret };
+	}
+
 	try {
 		const config: any = {
 			...setupConfig,
@@ -386,11 +396,8 @@ async function startServer(setupConfig: DesktopSetupConfig, restart = false) {
 		process.env.API_HOST = '0.0.0.0';
 		process.env.API_BASE_URL = `http://127.0.0.1:${setupConfig.port || environment.API_DEFAULT_PORT}`;
 
-		if (!setupConfig.secret || !setupConfig.secret.jwt || !setupConfig.secret.refresh_token) {
-			throw new AppError('MAINSTRSERVER', new Error('JWT secrets are required for local server startup'));
-		}
-		process.env.JWT_SECRET = setupConfig.secret.jwt;
-		process.env.JWT_REFRESH_TOKEN_SECRET = setupConfig.secret.refresh_token;
+		// All four are already provisioned above, so this returns them unchanged.
+		Object.assign(process.env, desktopSecretsToEnv(ensureDesktopSecrets(setupConfig.secret).secret));
 
 		console.log('Setting additional environment variables...', process.env.API_PORT);
 		console.log('Setting additional environment variables...', process.env.API_HOST);

apps/mcp-auth/src/user/oauth-user.service.ts

@@ -77,10 +77,18 @@ export class OAuthUserService {
 
 	/**
 	 * Get user information by user ID for MCP OAuth
-	 * Used by OAuth server to retrieve user details for token claims
+	 * Used by OAuth server to retrieve user details for token claims, and to re-check the account on
+	 * every `refresh_token` grant (GHSA-3cgp-wmrg-4fqg).
+	 *
+	 * `null` therefore means exactly one thing: no user with that id can sign in any more — the row
+	 * is gone, deactivated or archived. A lookup that could not be PERFORMED (database down, timeout)
+	 * is re-thrown rather than flattened into `null`, because the refresh grant answers `null` with
+	 * `invalid_grant`, which tells the client to discard a refresh token that is in fact still valid.
+	 * The callers turn the rejection into a retryable 503/500 instead.
 	 *
 	 * @param userId User ID
-	 * @returns User information or null if not found
+	 * @returns User information, or null when no active user has that id
+	 * @throws The underlying error when the lookup itself fails
 	 */
 	async getMcpUserInfo(userId: string): Promise<IUser | null> {
 		try {
@@ -89,7 +97,7 @@ export class OAuthUserService {
 			return user || null;
 		} catch (error) {
 			this.logger.error('Error retrieving MCP user info', (error as Error)?.stack || (error as Error)?.message);
-			return null;
+			throw error;
 		}
 	}
 }

apps/server-api/src/index.ts

@@ -41,6 +41,8 @@ import {
 	DialogErrorHandler,
 	DialogOpenFile,
 	DialogStopServerExitConfirmation,
+	desktopSecretsToEnv,
+	ensureDesktopSecrets,
 	ErrorEventManager,
 	ErrorReport,
 	ErrorReportRepository,
@@ -376,6 +378,13 @@ const runServer = async () => {
 const getEnvApi = () => {
 	const config = serverConfig.setting;
 	serverConfig.update();
+	// Per-install random signing/session secrets, generated on first start and replacing a stored
+	// published default on upgrade. Persisted before the API starts so restarts keep the same keys
+	// (GHSA-39j7-x845-4w3c).
+	const { secret, changed } = ensureDesktopSecrets(config.secret);
+	if (changed) {
+		serverConfig.setting = { secret };
+	}
 	const addsConfig = LocalStore.getAdditionalConfig();
 	const provider = config.db === 'better-sqlite' ? 'better-sqlite3' : config.db;
 	return {
@@ -393,8 +402,7 @@ const getEnvApi = () => {
 		DEBUG: 'true',
 		API_PORT: String(config.port),
 		...addsConfig,
-		JWT_SECRET: config.secret?.jwt,
-		JWT_REFRESH_TOKEN_SECRET: config.secret?.refresh_token
+		...desktopSecretsToEnv(secret)
 	};
 };
 

apps/server/src/index.ts

@@ -41,6 +41,8 @@ import {
 	DialogErrorHandler,
 	DialogOpenFile,
 	DialogStopServerExitConfirmation,
+	desktopSecretsToEnv,
+	ensureDesktopSecrets,
 	ErrorEventManager,
 	ErrorReport,
 	ErrorReportRepository,
@@ -389,6 +391,13 @@ const initializeAppWindowManager = () => {
 const getEnvApi = () => {
 	const config = serverConfig.setting;
 	serverConfig.update();
+	// Per-install random signing/session secrets, generated on first start and replacing a stored
+	// published default on upgrade. Persisted before the API starts so restarts keep the same keys
+	// (GHSA-39j7-x845-4w3c).
+	const { secret, changed } = ensureDesktopSecrets(config.secret);
+	if (changed) {
+		serverConfig.setting = { secret };
+	}
 	const addsConfig = LocalStore.getAdditionalConfig();
 	const provider = config.db === 'better-sqlite' ? 'better-sqlite3' : config.db;
 	return {
@@ -406,8 +415,7 @@ const getEnvApi = () => {
 		DEBUG: process.env.NODE_ENV !== 'production' ? 'true' : 'false',
 		API_PORT: String(config.port),
 		...addsConfig,
-		JWT_SECRET: config.secret?.jwt,
-		JWT_REFRESH_TOKEN_SECRET: config.secret?.refresh_token
+		...desktopSecretsToEnv(secret)
 	};
 };