fix(config): attribute admin system-config writes so they survive restart

.changeset/system-config-admin-writes-authoritative.md

@@ -0,0 +1,20 @@
+---
+'seamless-auth-api': minor
+---
+
+Make admin system-config writes authoritative so they survive a restart.
+
+Env-mapped `system_config` rows are re-seeded from their environment variable on
+every boot for any row whose `updatedBy` is `NULL`. Admin console writes went
+through the access-token path, which never populated `updatedBy` (only the
+service-token path did), so a change made in the console (for example adding an
+OAuth provider or enabling the `oauth` login method) was silently reverted on the
+next restart, contradicting the documented contract.
+
+The whole-config `PATCH /system-config/admin` and the per-provider
+`/system-config/oauth-providers` endpoints now record the acting admin's id in
+`updatedBy` on the access-token path, in addition to the existing service-token
+path, so an admin change is genuinely authoritative and is no longer overwritten
+from env. Boot now also logs a warning when it overwrites a stored value that
+differs from the env-derived one, so the reseed is no longer silent.
+`docs/configuration.md` is updated to describe the real precedence.

docs/configuration.md

@@ -11,8 +11,9 @@ There are two configuration layers:
   OAuth providers, rate limits). These are seeded from environment variables on first boot and
   are the source of truth afterwards.
 
-> Because `system_config` is seeded once, editing an already-seeded value in `.env` does not
-> change running behavior. See [Environment vs system_config](#environment-vs-system_config).
+> Once an admin changes an env-mapped value through the admin system-config endpoints, that row
+> becomes authoritative and its environment variable is no longer consulted. Until then, the env
+> var is re-applied on every boot. See [Environment vs system_config](#environment-vs-system_config).
 
 ## Minimal configuration to boot
 
@@ -221,9 +222,23 @@ Validation is enforced by [`systemConfig.schema.ts`](../src/schemas/systemConfig
 
 ## Environment vs `system_config`
 
-`system_config` is seeded **once**. After first boot, the row is authoritative and the mapped
-environment variable is no longer consulted for that key. To change a seeded value you must update
-the `system_config` row (via the admin system-config endpoints), not just `.env`.
+Each env-mapped `system_config` row is seeded from its environment variable on first boot. After
+that, precedence depends on whether the row has been changed through the admin API:
+
+- **Until an admin changes it** (`updatedBy IS NULL`), the mapped environment variable is
+  re-applied on **every** boot. This keeps env-driven config authoritative and stops a seeded
+  default from permanently shadowing a later `.env` change. When a boot overwrites a stored value
+  that differs, it is logged at `warn`.
+- **Once an admin changes it** through the admin system-config endpoints, the row records who made
+  the change (`updatedBy`) and becomes authoritative. The mapped environment variable is no longer
+  consulted for that key, so the admin change survives restarts and later `.env` edits do not
+  affect it.
+
+Both the whole-config `PATCH /system-config/admin` and the per-provider `/system-config/oauth-providers`
+endpoints set `updatedBy`, whether the caller authenticates with a service token or with an admin
+access token. So a change made in the admin console sticks. To make a seeded value change through
+env again after an admin has taken it over, clear that row's `updatedBy` (or delete the row and let
+it re-seed).
 
 Reads are cached in-process, so a `system_config` write should invalidate the cache to take
 effect immediately. See [`getSystemConfig.ts`](../src/config/getSystemConfig.ts).

src/config/bootstrapSystemConfig.ts

@@ -6,10 +6,13 @@
 
 import { SystemConfig } from '../models/systemConfig.js';
 import { SystemConfigSchema } from '../schemas/systemConfig.schema.js';
+import getLogger from '../utils/logger.js';
 import { parseSystemConfigEnvValue } from '../utils/parseEnvConfigs.js';
 import { SYSTEM_CONFIG_DEFAULTS } from './systemConfig.defaults.js';
 import { SYSTEM_CONFIG_ENV_MAP } from './systemConfig.envMap.js';
 
+const logger = getLogger('bootstrapSystemConfig');
+
 export async function bootstrapSystemConfig() {
   const resolvedConfig: Record<string, unknown> = {};
 
@@ -29,6 +32,11 @@ export async function bootstrapSystemConfig() {
         );
 
         if (JSON.stringify(existing.value) !== JSON.stringify(parsed)) {
+          logger.warn(
+            `Overwriting system_config "${key}" from env ${envVar}: the stored value differs and ` +
+              `the row is not marked admin-managed (updatedBy IS NULL). If this value was changed ` +
+              `through the admin console, that change is being reverted here.`,
+          );
           await existing.update({ value: parsed });
         }
 

src/controllers/oauthProviders.ts

@@ -7,6 +7,7 @@
 import { Response } from 'express';
 
 import { getSystemConfig, invalidateSystemConfigCache } from '../config/getSystemConfig.js';
+import { resolveSystemConfigUpdatedBy } from '../lib/systemConfigActor.js';
 import { SystemConfig } from '../models/systemConfig.js';
 import { OAuthProviderConfig, OAuthProviderConfigSchema } from '../schemas/systemConfig.schema.js';
 import { AuthEventService } from '../services/authEventService.js';
@@ -29,7 +30,7 @@ async function persistProviders(
   req: ServiceRequest,
   audit: ProviderAudit,
 ) {
-  const updatedBy = typeof req.clientId === 'function' ? req.clientId() : (req.clientId ?? null);
+  const updatedBy = resolveSystemConfigUpdatedBy(req);
 
   await SystemConfig.sequelize!.transaction(async (tx) => {
     await SystemConfig.upsert(

src/controllers/systemConfig.ts

@@ -7,6 +7,7 @@
 import { Response } from 'express';
 
 import { getSystemConfig, invalidateSystemConfigCache } from '../config/getSystemConfig.js';
+import { resolveSystemConfigUpdatedBy } from '../lib/systemConfigActor.js';
 import { SystemConfig } from '../models/systemConfig.js';
 import { User } from '../models/users.js';
 import { createPatchSystemConfigSchema } from '../schemas/systemConfig.patch.schema.js';
@@ -73,7 +74,7 @@ export async function updateSystemConfig(req: ServiceRequest, res: Response) {
 
   const existingMap = Object.fromEntries(existingRows.map((row) => [row.key, row.value]));
 
-  const updatedBy = typeof req.clientId === 'function' ? req.clientId() : (req.clientId ?? null);
+  const updatedBy = resolveSystemConfigUpdatedBy(req);
 
   await SystemConfig.sequelize!.transaction(async (tx) => {
     for (const [key, value] of Object.entries(updates)) {

src/lib/systemConfigActor.ts

@@ -0,0 +1,19 @@
+/*
+ * Copyright © 2026 Fells Code, LLC
+ * Licensed under the GNU Affero General Public License v3.0
+ * See LICENSE file in the project root for full license information
+ */
+
+import { AuthenticatedRequest, ServiceRequest } from '../types/types.js';
+
+// Who a system_config write is attributed to. Service-token callers carry
+// req.clientId; admin console callers authenticate with an access token and
+// carry req.user instead. Either one sets updatedBy, which marks the row as
+// admin-managed so bootstrap stops re-seeding it from env on the next boot.
+export function resolveSystemConfigUpdatedBy(req: ServiceRequest): string | null {
+  const clientId = typeof req.clientId === 'function' ? req.clientId() : req.clientId;
+  if (clientId) return clientId;
+
+  const user = (req as Partial<Pick<AuthenticatedRequest, 'user'>>).user;
+  return user?.id ?? null;
+}

tests/integration/systemConfig/oauthProviders.spec.ts

@@ -80,7 +80,7 @@ describe('POST /system-config/oauth-providers', () => {
     expect(res.status).toBe(201);
     expect(res.body.provider.id).toBe('github');
     expect(SystemConfig.upsert).toHaveBeenCalledWith(
-      expect.objectContaining({ key: 'oauth_providers' }),
+      expect.objectContaining({ key: 'oauth_providers', updatedBy: 'user-1' }),
       expect.anything(),
     );
     expect(invalidateSystemConfigCache).toHaveBeenCalled();

tests/integration/systemConfig/systemConfig.spec.ts

@@ -75,6 +75,10 @@ describe('PATCH /system-config/admin', () => {
     expect(res.status).toBe(200);
     expect(res.body.success).toBe(true);
     expect(invalidateSystemConfigCache).toHaveBeenCalled();
+    expect(SystemConfig.upsert).toHaveBeenCalledWith(
+      expect.objectContaining({ updatedBy: 'user-1' }),
+      expect.anything(),
+    );
   });
 
   it('accepts scoped role names', async () => {

tests/unit/config/bootstrapSystemConfig.spec.ts

@@ -24,6 +24,17 @@ vi.mock('../../../src/schemas/systemConfig.schema', () => ({
   },
 }));
 
+const { loggerWarn } = vi.hoisted(() => ({ loggerWarn: vi.fn() }));
+
+vi.mock('../../../src/utils/logger', () => ({
+  default: () => ({
+    warn: loggerWarn,
+    info: vi.fn(),
+    error: vi.fn(),
+    debug: vi.fn(),
+  }),
+}));
+
 function resetEnv() {
   delete process.env.APP_NAME;
   delete process.env.RATE_LIMIT;
@@ -101,6 +112,10 @@ describe('bootstrapSystemConfig', () => {
     await bootstrapSystemConfig();
 
     expect(update).toHaveBeenCalledWith({ value: 'parsed' });
+    expect(loggerWarn).toHaveBeenCalledTimes(2);
+    const warnedMessages = loggerWarn.mock.calls.map((call) => call[0]).join('\n');
+    expect(warnedMessages).toContain('app_name');
+    expect(warnedMessages).toContain('rate_limit');
   });
 
   it('does not rewrite an env-backed row whose value already matches', async () => {
@@ -120,6 +135,7 @@ describe('bootstrapSystemConfig', () => {
     await bootstrapSystemConfig();
 
     expect(update).not.toHaveBeenCalled();
+    expect(loggerWarn).not.toHaveBeenCalled();
   });
 
   it('preserves a row that was changed via the admin API (updatedBy set)', async () => {