sync: update from private repo (63a6e76)

src/bridge/auth.test.ts

@@ -3,7 +3,8 @@ import { mkdtempSync, rmSync } from 'fs';
 import { join } from 'path';
 import { tmpdir } from 'os';
 import { Repository } from '../db/repository.js';
-import { requireAuth, requireAdmin, fetchGiteaOrgsForUser } from './auth.js';
+import { requireAuth, requireAdmin, fetchGiteaOrgsForUser, isProviderConfigured, isProviderActive } from './auth.js';
+import type { AuthConfig } from '../config.js';
 import type { Request, Response, NextFunction } from 'express';
 
 function mockReqRes(overrides: Partial<Request> = {}) {
@@ -172,3 +173,51 @@ describe('fetchGiteaOrgsForUser', () => {
     }
   });
 });
+
+// Audit 2026-06-09: provider completeness + primaryProvider gating. These pin
+// the security-critical fail-closed / restriction semantics behind the new
+// AuthForm (config editable from Settings UI).
+const COMPLETE = { clientId: 'id', clientSecret: 'sec', callbackUrl: 'https://x/cb' };
+const COMPLETE_GITEA = { ...COMPLETE, baseUrl: 'https://gitea' };
+function authCfg(
+  providers: Partial<AuthConfig['providers']>,
+  primaryProvider?: 'google' | 'gitea',
+): AuthConfig {
+  return {
+    sessionSecret: 's', sessionMaxAge: 1, secureCookie: false, adminEmails: [],
+    primaryProvider, providers,
+  } as AuthConfig;
+}
+
+describe('isProviderConfigured', () => {
+  it('true only when all required fields are present', () => {
+    expect(isProviderConfigured(COMPLETE, 'google')).toBe(true);
+    expect(isProviderConfigured(COMPLETE_GITEA, 'gitea')).toBe(true);
+  });
+  it('gitea requires baseUrl', () => {
+    expect(isProviderConfigured(COMPLETE, 'gitea')).toBe(false);
+  });
+  it('false when any field is missing or provider undefined', () => {
+    expect(isProviderConfigured({ ...COMPLETE, clientSecret: '' }, 'google')).toBe(false);
+    expect(isProviderConfigured({ ...COMPLETE, callbackUrl: undefined }, 'google')).toBe(false);
+    expect(isProviderConfigured(undefined, 'google')).toBe(false);
+  });
+});
+
+describe('isProviderActive (primaryProvider restriction)', () => {
+  it('both active when both complete and no primary', () => {
+    const c = authCfg({ google: COMPLETE, gitea: COMPLETE_GITEA });
+    expect(isProviderActive(c, 'google')).toBe(true);
+    expect(isProviderActive(c, 'gitea')).toBe(true);
+  });
+  it('a valid primary restricts to that provider only', () => {
+    const c = authCfg({ google: COMPLETE, gitea: COMPLETE_GITEA }, 'google');
+    expect(isProviderActive(c, 'google')).toBe(true);
+    expect(isProviderActive(c, 'gitea')).toBe(false);
+  });
+  it('an invalid primary (unconfigured provider) is ignored', () => {
+    const c = authCfg({ gitea: COMPLETE_GITEA }, 'google');
+    expect(isProviderActive(c, 'gitea')).toBe(true); // gitea still usable
+    expect(isProviderActive(c, 'google')).toBe(false); // google not configured
+  });
+});

src/bridge/auth.ts

@@ -9,9 +9,10 @@ import passport from 'passport';
 import { Strategy as GoogleStrategy } from 'passport-google-oauth20';
 import { Strategy as OAuth2Strategy } from 'passport-oauth2';
 import type { Database } from 'better-sqlite3';
-import type { AuthConfig } from '../config.js';
+import type { AuthConfig, AuthProviderConfig } from '../config.js';
 import type { Repository } from '../db/repository.js';
 import { logger } from '../logger.js';
+import { randomBytes } from 'crypto';
 
 /**
  * WebSocket upgrade（生 IncomingMessage）から認証済みユーザーを解決するチェッカー。
@@ -43,6 +44,36 @@ function escapeHtml(s: string): string {
     .replace(/'/g, ''');
 }
 
+/**
+ * A provider is usable only when ALL fields needed to complete an OAuth round
+ * trip are present (Gitea additionally needs base_url). Used to gate auth
+ * activation and login-button visibility so a partial config saved from the
+ * Settings UI can't enable auth in a state where nobody can log in.
+ */
+export function isProviderConfigured(
+  p: AuthProviderConfig | undefined,
+  kind: 'google' | 'gitea',
+): p is AuthProviderConfig {
+  if (!p || !p.clientId || !p.clientSecret || !p.callbackUrl) return false;
+  if (kind === 'gitea' && !p.baseUrl) return false;
+  return true;
+}
+
+/**
+ * Whether a provider should actually be LIVE (strategy + /auth/<kind> route).
+ * A valid `primaryProvider` restricts auth to that single provider, so the
+ * restriction is enforced at the route layer too — not just hidden on the login
+ * page (otherwise the "disabled" provider's route stayed open to direct URLs).
+ * An invalid primary (pointing at an unconfigured provider) is ignored.
+ */
+export function isProviderActive(authConfig: AuthConfig, kind: 'google' | 'gitea'): boolean {
+  if (!isProviderConfigured(authConfig.providers[kind], kind)) return false;
+  const primary = authConfig.primaryProvider;
+  if (primary === 'google' && isProviderConfigured(authConfig.providers.google, 'google')) return kind === 'google';
+  if (primary === 'gitea' && isProviderConfigured(authConfig.providers.gitea, 'gitea')) return kind === 'gitea';
+  return true;
+}
+
 /**
  * auth-login.html をレンダリングする。
  * primary_provider 設定と各プロバイダの configured 状態に応じて
@@ -51,9 +82,15 @@ function escapeHtml(s: string): string {
  */
 function renderLoginPage(authConfig: AuthConfig, branding: LoginBranding = DEFAULT_LOGIN_BRANDING): string {
   const raw = readFileSync(path.join(__authDirname, 'auth-login.html'), 'utf-8');
-  const primary = authConfig.primaryProvider;
-  const googleConfigured = !!authConfig.providers.google?.clientId;
-  const giteaConfigured = !!authConfig.providers.gitea?.clientId;
+  const googleConfigured = isProviderConfigured(authConfig.providers.google, 'google');
+  const giteaConfigured = isProviderConfigured(authConfig.providers.gitea, 'gitea');
+  // Ignore a primaryProvider that points to an unconfigured provider — otherwise
+  // it would hide the only working login button and lock the operator out.
+  const primary =
+    (authConfig.primaryProvider === 'google' && googleConfigured) ||
+    (authConfig.primaryProvider === 'gitea' && giteaConfigured)
+      ? authConfig.primaryProvider
+      : undefined;
 
   // Decide which buttons to show
   let showGoogle: boolean;
@@ -315,7 +352,8 @@ export async function fetchGiteaOrgsForUser(
 
 function registerGoogleStrategy(repo: Repository, authConfig: AuthConfig): void {
   const googleConfig = authConfig.providers.google;
-  if (!googleConfig) return;
+  if (!isProviderConfigured(googleConfig, 'google')) return;
+  if (!isProviderActive(authConfig, 'google')) return;
 
   passport.use(
     new GoogleStrategy(
@@ -331,7 +369,7 @@ function registerGoogleStrategy(repo: Repository, authConfig: AuthConfig): void
 
         await handleOAuthCallback(
           repo,
-          authConfig.adminEmails,
+          authConfig.adminEmails ?? [],
           'google',
           profile.id,
           email,
@@ -346,7 +384,8 @@ function registerGoogleStrategy(repo: Repository, authConfig: AuthConfig): void
 
 function registerGiteaStrategy(repo: Repository, authConfig: AuthConfig): void {
   const giteaConfig = authConfig.providers.gitea;
-  if (!giteaConfig) return;
+  if (!isProviderConfigured(giteaConfig, 'gitea')) return;
+  if (!isProviderActive(authConfig, 'gitea')) return;
 
   const baseUrl = giteaConfig.baseUrl ?? '';
 
@@ -401,7 +440,7 @@ function registerGiteaStrategy(repo: Repository, authConfig: AuthConfig): void {
             name,
             avatarUrl,
           });
-          if (user.status === 'pending' && authConfig.adminEmails.includes(email)) {
+          if (user.status === 'pending' && (authConfig.adminEmails ?? []).includes(email)) {
             repo.updateUser(user.id, { status: 'active', role: 'admin' });
             const updated = repo.getUserById(user.id);
             if (updated) user = updated;
@@ -457,7 +496,7 @@ function createAuthRouter(
   });
 
   // Google OAuth
-  if (authConfig.providers.google) {
+  if (isProviderActive(authConfig, 'google')) {
     router.get('/google', passport.authenticate('google', { scope: ['profile', 'email'] }));
 
     router.get(
@@ -475,7 +514,7 @@ function createAuthRouter(
   }
 
   // Gitea OAuth
-  if (authConfig.providers.gitea) {
+  if (isProviderActive(authConfig, 'gitea')) {
     router.get('/gitea', passport.authenticate('gitea'));
 
     router.get(
@@ -533,9 +572,22 @@ export function setupAuth(
 ): AuthMiddlewares {
   const db = repo.getDb();
 
+  // express-session throws "secret option required" (→ 500 on every request) if
+  // the secret is empty. Auth can be enabled from the Settings UI before a
+  // session_secret is set, so fall back to a random per-process secret with a
+  // warning rather than bricking the server. Sessions reset on restart until a
+  // stable value is configured.
+  let sessionSecret = authConfig.sessionSecret;
+  if (!sessionSecret || sessionSecret.length === 0) {
+    sessionSecret = randomBytes(32).toString('hex');
+    logger.warn(
+      '[auth] auth.session_secret is unset — using a random per-process secret; sessions reset on restart. Set a stable value in Settings → Authentication.',
+    );
+  }
+
   // セッションミドルウェア
   const sessionMiddleware = session({
-    secret: authConfig.sessionSecret,
+    secret: sessionSecret,
     resave: false,
     saveUninitialized: false,
     store: createSqliteSessionStore(db),

src/bridge/server.ts

@@ -22,7 +22,7 @@ import { setSessionManager } from '../engine/tools/browser.js';
 import { setUserFolderToolDeps } from '../engine/tools/user-folder.js';
 import { setSkillToolDeps } from '../engine/tools/skills.js';
 import { setAppDocsDeps } from '../engine/tools/app-docs.js';
-import { setupAuth, requireAuth, requireAdmin } from './auth.js';
+import { setupAuth, requireAuth, requireAdmin, isProviderConfigured } from './auth.js';
 import { canUserSeeTask } from './visibility.js';
 import { mountAdminApi } from './admin-api.js';
 import { createAdminGatewayApi } from './admin-gateway-api.js';
@@ -219,7 +219,33 @@ export function createCoreServer(opts: CoreServerOptions): {
   app.use('/api/local/reflection', express.json());
 
   // === Auth setup ===
-  const authActive = !!(opts.authConfig?.providers);
+  // Auth activates when a provider is COMPLETELY configured (clientId +
+  // clientSecret + callbackUrl, plus baseUrl for Gitea).
+  //
+  // FAIL CLOSED on a partial config: if the operator clearly INTENDED auth
+  // (a provider has a client_id) but it's incomplete (typo'd / missing
+  // secret/callback), we must NOT silently fall back to no-auth — that would
+  // fail OPEN and expose /api/local, /api/config, etc. without authentication.
+  // Refuse to start instead, with a clear message. (A bare `auth` block with no
+  // client_id at all is treated as genuine no-auth mode.)
+  const _authProviders = opts.authConfig?.providers;
+  const authUsable =
+    isProviderConfigured(_authProviders?.google, 'google') ||
+    isProviderConfigured(_authProviders?.gitea, 'gitea');
+  // "Intended" = ANY OAuth field is present on a provider (not just client_id).
+  // Saving only client_secret/callback_url/base_url, or clearing client_id by
+  // mistake, must still fail closed rather than drop to no-auth.
+  const _hasAnyField = (p?: { clientId?: string; clientSecret?: string; callbackUrl?: string; baseUrl?: string }) =>
+    !!(p?.clientId || p?.clientSecret || p?.callbackUrl || p?.baseUrl);
+  const authIntended = _hasAnyField(_authProviders?.google) || _hasAnyField(_authProviders?.gitea);
+  if (authIntended && !authUsable) {
+    throw new Error(
+      '[auth] auth is partially configured: a provider has a client_id but is missing ' +
+        'client_secret / callback_url (Gitea also needs base_url). Refusing to start in an ' +
+        'insecure no-auth state — complete the provider config or remove it from config.yaml.',
+    );
+  }
+  const authActive = authUsable;
   let authenticateUpgrade: import('./auth.js').UpgradeAuthChecker | undefined;
 
   if (authActive) {

src/config-manager.ts

@@ -7,7 +7,15 @@ import { createHash } from 'crypto';
 import { logger } from './logger.js';
 
 const MASKED = '********';
-const SENSITIVE_PATHS = ['tools.xAuthToken', 'tools.xCt0'];
+const SENSITIVE_PATHS = [
+  'tools.xAuthToken',
+  'tools.xCt0',
+  // Auth secrets now editable via AuthForm — mask in GET, restore on save so a
+  // partial edit doesn't expose or clobber them.
+  'auth.sessionSecret',
+  'auth.providers.google.clientSecret',
+  'auth.providers.gitea.clientSecret',
+];
 
 /**
  * Keys stripped from `getConfigForApi` output. The v2 contract (design doc
@@ -59,8 +67,13 @@ export class ConfigManager {
         obj = obj?.[parts[i]];
         if (!obj) break;
       }
-      if (obj && parts[parts.length - 1] in obj) {
-        obj[parts[parts.length - 1]] = MASKED;
+      // Only mask a NON-EMPTY secret. Masking an empty/undefined value would
+      // make an unset secret look configured in the UI, and the mask-restore on
+      // save would then re-write the empty value (codex P2: empty session_secret
+      // would stay empty → random per-restart; empty OAuth secret → login fails).
+      const lastKey = parts[parts.length - 1];
+      if (obj && typeof obj[lastKey] === 'string' && obj[lastKey].length > 0) {
+        obj[lastKey] = MASKED;
       }
     }