Skip to content

fix(config): validate system_config on runtime read + align cache TTL (#13) #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SAY-5 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(config): validate system_config on runtime read + align cache TTL (#13) #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae10c58
fix(config): validate system_config on runtime read + align cache TTL…
SAY-5 Apr 28, 2026
35edc47
test: address review feedback on #13 PR
SAY-5 Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/config/getSystemConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,38 @@
*/

import { SystemConfig as SysConfigModel } from '../models/systemConfig.js';
import { SystemConfig } from '../schemas/systemConfig.schema.js';
import { SystemConfig, SystemConfigSchema } from '../schemas/systemConfig.schema.js';
import getLogger from '../utils/logger.js';

let cachedConfig: { [k: string]: unknown } | null;
const logger = getLogger('getSystemConfig');

let cachedConfig: SystemConfig | null = null;
let lastLoadedAt = 0;

const CACHE_TTL_MS = 300_000; // 30 seconds
export const CACHE_TTL_MS = 5 * 60 * 1000;

export async function getSystemConfig(): Promise<SystemConfig> {
const now = Date.now();

if (cachedConfig && now - lastLoadedAt < CACHE_TTL_MS) {
return cachedConfig as SystemConfig;
return cachedConfig;
}

const rows = await SysConfigModel.findAll();
const configObject = Object.fromEntries(rows.map((row) => [row.key, row.value]));

const parsed = SystemConfigSchema.safeParse(configObject);
if (!parsed.success) {
logger.error('System config failed schema validation on runtime read', {
issues: parsed.error.issues,
});
throw new Error('System configuration is invalid');
}

cachedConfig = Object.fromEntries(rows.map((row) => [row.key, row.value]));

cachedConfig = parsed.data;
Comment on lines +26 to +36
Copy link
Copy Markdown
Contributor

@Bccorb Bccorb Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I testing my .env file did still let me pass improper config values. For example I passed test to an origins field.

lastLoadedAt = now;

return cachedConfig as SystemConfig;
return cachedConfig;
}

export function invalidateSystemConfigCache() {
Expand Down
96 changes: 85 additions & 11 deletions tests/unit/config/getSystemConfig.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,24 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';

vi.unmock('../../../src/config/getSystemConfig');

function validRows(
overrides: Record<string, unknown> = {},
): Array<{ key: string; value: unknown }> {
const base: Record<string, unknown> = {
app_name: 'TestApp',
default_roles: ['user'],
available_roles: ['user', 'admin'],
access_token_ttl: '15m',
refresh_token_ttl: '7d',
rate_limit: 100,
delay_after: 0,
rpid: 'localhost',
origins: ['https://example.com'],
...overrides,
};
return Object.entries(base).map(([key, value]) => ({ key, value }));
}

describe('getSystemConfig', () => {
beforeEach(() => {
vi.resetModules();
Expand All @@ -11,20 +29,21 @@ describe('getSystemConfig', () => {
it('fetches config from DB when cache empty', async () => {
const { SystemConfig } = await import('../../../src/models/systemConfig');

(SystemConfig.findAll as any).mockResolvedValue([{ key: 'app_name', value: 'TestApp' }]);
(SystemConfig.findAll as any).mockResolvedValue(validRows({ app_name: 'TestApp' }));

const { getSystemConfig } = await import('../../../src/config/getSystemConfig');

const result = await getSystemConfig();

expect(SystemConfig.findAll).toHaveBeenCalled();
expect(result).toEqual({ app_name: 'TestApp' });
expect(result.app_name).toBe('TestApp');
expect(result.default_roles).toEqual(['user']);
});

it('returns cached config when within TTL', async () => {
const { SystemConfig } = await import('../../../src/models/systemConfig');

(SystemConfig.findAll as any).mockResolvedValue([{ key: 'app_name', value: 'TestApp' }]);
(SystemConfig.findAll as any).mockResolvedValue(validRows({ app_name: 'TestApp' }));

const { getSystemConfig } = await import('../../../src/config/getSystemConfig');

Expand All @@ -39,30 +58,30 @@ describe('getSystemConfig', () => {
const { SystemConfig } = await import('../../../src/models/systemConfig');

(SystemConfig.findAll as any)
.mockResolvedValueOnce([{ key: 'app_name', value: 'A' }])
.mockResolvedValueOnce([{ key: 'app_name', value: 'B' }]);
.mockResolvedValueOnce(validRows({ app_name: 'AppA' }))
.mockResolvedValueOnce(validRows({ app_name: 'AppB' }));

const { getSystemConfig } = await import('../../../src/config/getSystemConfig');

const first = await getSystemConfig();

// simulate time passing
vi.spyOn(Date, 'now')
.mockReturnValueOnce(Date.now() + 1)
.mockReturnValueOnce(Date.now() + 400_000); // > TTL
.mockReturnValueOnce(Date.now() + 6 * 60 * 1000);

const second = await getSystemConfig();

expect(second).not.toEqual(first);
expect(second.app_name).toBe('AppB');
expect(first.app_name).toBe('AppA');
expect(SystemConfig.findAll).toHaveBeenCalledTimes(2);
});

it('invalidates cache manually', async () => {
const { SystemConfig } = await import('../../../src/models/systemConfig');

(SystemConfig.findAll as any)
.mockResolvedValueOnce([{ key: 'app_name', value: 'A' }])
.mockResolvedValueOnce([{ key: 'app_name', value: 'B' }]);
.mockResolvedValueOnce(validRows({ app_name: 'AppA' }))
.mockResolvedValueOnce(validRows({ app_name: 'AppB' }));

const { getSystemConfig, invalidateSystemConfigCache } =
await import('../../../src/config/getSystemConfig');
Expand All @@ -73,7 +92,62 @@ describe('getSystemConfig', () => {

const result = await getSystemConfig();

expect(result).toEqual({ app_name: 'B' });
expect(result.app_name).toBe('AppB');
expect(SystemConfig.findAll).toHaveBeenCalledTimes(2);
});

describe('runtime schema validation (#13)', () => {
// Drive the validation gate per-field so a future schema change that
// weakens any single rule fails an obvious test instead of silently
// shipping shape-mismatched config to downstream auth code.
it.each<{ key: string; bad: unknown; reason: string }>([
{ key: 'app_name', bad: 'ab', reason: 'string shorter than min(3)' },
{ key: 'default_roles', bad: ['has space'], reason: 'role contains whitespace' },
{ key: 'default_roles', bad: [], reason: 'empty array' },
{ key: 'available_roles', bad: ['bad/role'], reason: 'role contains slash' },
{ key: 'access_token_ttl', bad: '15', reason: 'missing unit suffix' },
{ key: 'refresh_token_ttl', bad: '7days', reason: 'wrong unit format' },
{ key: 'rate_limit', bad: 0, reason: 'rate_limit must be positive' },
{ key: 'rate_limit', bad: -1, reason: 'rate_limit cannot be negative' },
{ key: 'rate_limit', bad: 1.5, reason: 'rate_limit must be int' },
{ key: 'delay_after', bad: -1, reason: 'delay_after cannot be negative' },
{ key: 'rpid', bad: '', reason: 'empty rpid' },
{ key: 'origins', bad: ['test'], reason: 'origins[i] must be a URL' },
{ key: 'origins', bad: 'https://example.com', reason: 'origins must be an array' },
{ key: 'origins', bad: [], reason: 'origins must have at least one entry' },
])('rejects $key when $reason', async ({ key, bad }) => {
const { SystemConfig } = await import('../../../src/models/systemConfig');
const rows = validRows({ [key]: bad });
(SystemConfig.findAll as any).mockResolvedValue(rows);

const { getSystemConfig } = await import('../../../src/config/getSystemConfig');
await expect(getSystemConfig()).rejects.toThrow('System configuration is invalid');
});

it('rejects when a required field row is missing entirely', async () => {
const { SystemConfig } = await import('../../../src/models/systemConfig');
const tainted = validRows().filter((row) => row.key !== 'default_roles');
(SystemConfig.findAll as any).mockResolvedValue(tainted);

const { getSystemConfig } = await import('../../../src/config/getSystemConfig');
await expect(getSystemConfig()).rejects.toThrow('System configuration is invalid');
});

// After a tainted read the cache must remain empty so the next call
// re-hits the DB. Otherwise the operator's recovery fix (correct row
// value) wouldn't be picked up until process restart.
it('keeps the cache empty after a failed validation so the next call reloads from DB', async () => {
const { SystemConfig } = await import('../../../src/models/systemConfig');
const tainted = validRows().filter((row) => row.key !== 'app_name');
const recovered = validRows({ app_name: 'Recovered' });
(SystemConfig.findAll as any).mockResolvedValueOnce(tainted).mockResolvedValueOnce(recovered);

const { getSystemConfig } = await import('../../../src/config/getSystemConfig');

await expect(getSystemConfig()).rejects.toThrow();
const result = await getSystemConfig();
expect(result.app_name).toBe('Recovered');
expect(SystemConfig.findAll).toHaveBeenCalledTimes(2);
});
});
});