diff --git a/.changeset/30203-allowed-ip-ranges-live.md b/.changeset/30203-allowed-ip-ranges-live.md new file mode 100644 index 00000000000..3773b73aaa0 --- /dev/null +++ b/.changeset/30203-allowed-ip-ranges-live.md @@ -0,0 +1,20 @@ +--- +'@objectstack/plugin-auth': patch +'@objectstack/service-settings': patch +--- + +fix(plugin-auth)!: clearing the allowed IP ranges applies at once, and a console save that leaves out the saving address is refused + +Clause-②: yes (narrowing) — a settings console save (`PUT /api/settings/auth`) of an `allowed_ip_ranges` list none of whose entries matches the saving request's own address is refused, where it was stored before; and `@objectstack/service-settings` widens its published surface: three new exported types (`SettingsWriteOptions`, `SettingsRequestTransport`, `SettingsWritePredicateOptions`), `setMany`'s fourth parameter accepts `SettingsWriteOptions`, `registerWritePredicate` takes an optional fourth `options`, and `SettingsWritePredicateInput` gains an optional `transport`. +Metadata-protocol: none +Skills-docs: none — `skills/**` teaches no `allowed_ip_ranges`; `content/docs/permissions/authentication.mdx` describes the setting and does not yet state the save refusal or the recovery, which ship here in the refusal text and this entry (the page is outside this change's surface and is named for a follow-up). + + + +**BREAKING** for an administrator who saves, through the settings console, an allowed-IP list (ADR-0069 D5) that does not include the address they are connected from. + +- **Clearing applies at once.** The console sends an emptied field as `null`, which resolves with source `default`, and the settings binding used to skip a default-sourced `allowed_ip_ranges`: the last stored list kept refusing sign-in (`403 IP_NOT_ALLOWED`) until a restart while the field showed empty. A cleared or restored-default value now hands the gate back to the deployment's own declaration in the same pass: no list when it declared none, the `allowedIpRanges` the plugin options or an `auth:configure` handler declared otherwise. +- **The save is refused.** A list no entry of which matches the saving request's address is refused with `400 SETTINGS_VALIDATION`, `invalid_value` at `allowed_ip_ranges`, the address in `constraint.clientAddress`, and a message in the caller's language that names it. The address is the one the gate judges at sign-in: the transport's peer address, or the forwarded one when the stack declares `server.trustProxy`. When the server cannot establish the address, every non-empty list is refused, because the gate would refuse that request too. Clearing the list is never refused, and an in-process write (boot, seed, migration) carries no request and is not judged. +- **Recovery when already locked out.** Start the server with `OS_AUTH_ALLOWED_IP_RANGES` set to an empty value. The settings env override wins over the stored list and opens the gate; remove it once the list is fixed. +- **`@objectstack/service-settings`.** `setMany`'s options (`SettingsWriteOptions`) and a write predicate's input gain an optional `transport` (`SettingsRequestTransport`: the transport's peer address and request headers), which the settings routes set on a save so a write predicate can judge the change against the caller's own connection. A predicate sees only the headers it names at registration (`registerWritePredicate(…, { requestHeaders })`, `SettingsWritePredicateOptions`), so a credential header never reaches one that did not ask for it. The transport identifies and authorizes nothing and is never stored. +- **Who moves.** An administrator adds their own address or range to the list, or saves it from an address inside it. diff --git a/packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts b/packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts new file mode 100644 index 00000000000..95dc4b3cc42 --- /dev/null +++ b/packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts @@ -0,0 +1,231 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30203] `auth.allowed_ip_ranges` (ADR-0069 D5) can neither lock out the + * administrator saving it unwarned, nor stay in force after it is cleared. + * + * The composition: the real `AuthPlugin` bound to a real `SettingsService` + * carrying the shipped auth manifest, and the real settings routes in front of + * it, so a save travels the console's own door (`PUT /api/settings/auth`) and + * the clear reaches the running gate through the real change bus. + * + * - Clearing applies at once. The console sends `null` for an emptied field; + * the cascade then answers with source `default`, and the gate returns to + * the deployment's own declaration in the same pass — empty when it + * declared none, its own list when it did. Restoring the namespace's + * defaults does the same. + * - A save whose list leaves out the saving request's address is refused + * (`400 SETTINGS_VALIDATION`, `invalid_value` at the key, the address in + * `constraint.clientAddress`) and nothing is stored. The address is the one + * the gate judges at sign-in: the peer address, or the forwarded one when + * the deployment declares `trustProxy`. A list that includes it saves. + * - The recovery the refusal names: `OS_AUTH_ALLOWED_IP_RANGES` set empty + * opens the gate on the next boot over a stored list. + * + * `@objectstack/service-settings` resolves to its `src/` here (this package's + * `vitest.config.ts`), so the routes and the service are the checkout's. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts'; +import { + SettingsService, + builtinSettingsManifests, + registerSettingsRoutes, +} from '@objectstack/service-settings'; +import { AuthPlugin } from './auth-plugin.js'; +import type { AuthManager } from './auth-manager.js'; + +const OFFICE = '203.0.113.0/24'; +const ADMIN_CTX = { enforced: true, permissions: ['manage_platform_settings'], userId: 'usr_admin' }; +const authManifest = builtinSettingsManifests.find((m) => m.namespace === 'auth')!; + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +interface Rig { + svc: SettingsService; + manager: AuthManager; + save(body: Record, transport?: { headers?: Record; remoteAddress?: string }): + Promise<{ status: number; body: any }>; +} + +async function boot( + opts: { env?: Record; svc?: SettingsService; plugin?: Record } = {}, +): Promise { + const svc = opts.svc ?? new SettingsService({ env: opts.env ?? {} }); + if (!opts.svc) svc.registerManifest(authManifest); + const handlers = new Map Promise>>(); + const ctx: any = { + registerService: vi.fn(), + getService: vi.fn((name: string) => { + if (name === 'manifest') return { register: vi.fn() }; + if (name === 'settings') return svc; + return undefined; + }), + getServices: vi.fn(() => new Map()), + hook: vi.fn((name: string, handler: () => Promise) => { + handlers.set(name, [...(handlers.get(name) ?? []), handler]); + }), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + }; + const plugin = new AuthPlugin({ + secret: 'test-secret-at-least-32-chars-long', + baseUrl: 'http://localhost:3000', + ...(opts.plugin ?? {}), + }); + await plugin.init(ctx); + const manager = ctx.registerService.mock.calls.find(([name]: [string]) => name === 'auth')?.[1] as AuthManager; + await plugin.start(ctx); + for (const h of handlers.get('kernel:ready') ?? []) await h(); + + const http = new MockHttp(); + registerSettingsRoutes(http, svc, { contextFromRequest: () => ({ ...ADMIN_CTX }) }); + const save: Rig['save'] = async (body, transport = {}) => { + const out = { status: 200, body: undefined as any }; + const res: IHttpResponse = { + json: vi.fn((data: any) => { out.body = data; }) as any, + send: vi.fn() as any, + status: vi.fn((code: number) => { out.status = code; return res; }) as any, + header: vi.fn(() => res) as any, + }; + const req: IHttpRequest = { + params: { namespace: 'auth' }, + query: {}, + body, + headers: transport.headers ?? {}, + method: 'PUT', + path: '/api/settings/auth', + ...(transport.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}), + }; + await http.routes.get('PUT /api/settings/:namespace')!(req, res); + return out; + }; + return { svc, manager, save }; +} + +const stored = async (svc: SettingsService) => (await svc.get('auth', 'allowed_ip_ranges', {})).value ?? null; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('#30203 clearing allowed_ip_ranges applies at once', () => { + it('a cleared value opens the gate in the same pass, with no restart', async () => { + const { svc, manager } = await boot(); + await svc.set('auth', 'allowed_ip_ranges', OFFICE, {}); + await vi.waitFor(() => expect(manager.isClientIpAllowed('::1')).toBe(false)); + expect(manager.isClientIpAllowed('203.0.113.7')).toBe(true); + + await svc.set('auth', 'allowed_ip_ranges', null, {}); + expect((await svc.get('auth', 'allowed_ip_ranges', {})).source).toBe('default'); + await vi.waitFor(() => expect(manager.isClientIpAllowed('::1')).toBe(true)); + }); + + it('restoring the namespace defaults opens the gate too', async () => { + const { svc, manager } = await boot(); + await svc.set('auth', 'allowed_ip_ranges', OFFICE, {}); + await vi.waitFor(() => expect(manager.isClientIpAllowed('::1')).toBe(false)); + + await svc.resetNamespace('auth', {}); + await vi.waitFor(() => expect(manager.isClientIpAllowed('::1')).toBe(true)); + }); + + it("a cleared value hands the gate back to the deployment's own declared list, not to none", async () => { + const { svc, manager } = await boot({ plugin: { allowedIpRanges: ['10.0.0.0/8'] } }); + expect(manager.isClientIpAllowed('203.0.113.7')).toBe(false); + await svc.set('auth', 'allowed_ip_ranges', OFFICE, {}); + await vi.waitFor(() => expect(manager.isClientIpAllowed('203.0.113.7')).toBe(true)); + expect(manager.isClientIpAllowed('10.1.2.3')).toBe(false); + + await svc.set('auth', 'allowed_ip_ranges', null, {}); + await vi.waitFor(() => expect(manager.isClientIpAllowed('10.1.2.3')).toBe(true)); + expect(manager.isClientIpAllowed('203.0.113.7')).toBe(false); + }); +}); + +describe('#30203 a console save that excludes the saving address is refused', () => { + it('refuses a list without the peer address, names it, and stores nothing', async () => { + const { svc, save } = await boot(); + const res = await save({ allowed_ip_ranges: OFFICE }, { remoteAddress: '::1' }); + expect(res.status).toBe(400); + expect(res.body.error.code).toBe('SETTINGS_VALIDATION'); + const field = res.body.error.details.fields.find((f: any) => f.field === 'allowed_ip_ranges'); + expect(field.code).toBe('invalid_value'); + expect(field.constraint).toEqual({ clientAddress: '::1' }); + expect(field.message).toContain('::1'); + expect(await stored(svc)).toBeNull(); + }); + + it('saves a list that includes the peer address (preservation)', async () => { + const { svc, manager, save } = await boot(); + const res = await save({ allowed_ip_ranges: `${OFFICE}\n127.0.0.1` }, { remoteAddress: '::ffff:127.0.0.1' }); + expect(res.status).toBe(200); + expect(await stored(svc)).toBe(`${OFFICE}\n127.0.0.1`); + await vi.waitFor(() => expect(manager.isClientIpAllowed('198.51.100.1')).toBe(false)); + expect(manager.isClientIpAllowed('127.0.0.1')).toBe(true); + }); + + it('judges the address the gate judges: the forwarded one under trustProxy, the peer otherwise', async () => { + const forwarded = { headers: { 'x-forwarded-for': '198.51.100.7' }, remoteAddress: '10.0.0.1' }; + + const trusting = await boot({ plugin: { trustProxy: true } }); + const refused = await trusting.save({ allowed_ip_ranges: '10.0.0.0/8' }, forwarded); + expect(refused.status).toBe(400); + expect(refused.body.error.details.fields[0].constraint).toEqual({ clientAddress: '198.51.100.7' }); + expect((await trusting.save({ allowed_ip_ranges: '198.51.100.0/24' }, forwarded)).status).toBe(200); + + const untrusting = await boot(); + const peer = await untrusting.save({ allowed_ip_ranges: '198.51.100.0/24' }, forwarded); + expect(peer.status).toBe(400); + expect(peer.body.error.details.fields[0].constraint).toEqual({ clientAddress: '10.0.0.1' }); + }); + + it('refuses any list when the saving address cannot be established', async () => { + const { svc, save } = await boot(); + const res = await save({ allowed_ip_ranges: OFFICE }, {}); + expect(res.status).toBe(400); + expect(res.body.error.details.fields[0].constraint).toEqual({ clientAddress: null }); + expect(await stored(svc)).toBeNull(); + }); + + it('never refuses a clear, and leaves an in-process write (no request) unjudged', async () => { + const { svc, save } = await boot(); + await svc.set('auth', 'allowed_ip_ranges', OFFICE, {}); + expect(await stored(svc)).toBe(OFFICE); + const res = await save({ allowed_ip_ranges: null }, { remoteAddress: '::1' }); + expect(res.status).toBe(200); + expect(await stored(svc)).toBeNull(); + }); +}); + +describe('#30203 the recovery the refusal names', () => { + it('OS_AUTH_ALLOWED_IP_RANGES set empty opens the gate over a stored list on the next boot', async () => { + const env: Record = {}; + const svc = new SettingsService({ env }); + svc.registerManifest(authManifest); + await svc.set('auth', 'allowed_ip_ranges', OFFICE, {}); + + const locked = await boot({ svc }); + expect(locked.manager.isClientIpAllowed('::1')).toBe(false); + + env.OS_AUTH_ALLOWED_IP_RANGES = ''; + const recovered = await boot({ svc }); + expect(recovered.manager.isClientIpAllowed('::1')).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-auth/src/allowed-ip-ranges.ts b/packages/plugins/plugin-auth/src/allowed-ip-ranges.ts new file mode 100644 index 00000000000..007e9add7ee --- /dev/null +++ b/packages/plugins/plugin-auth/src/allowed-ip-ranges.ts @@ -0,0 +1,159 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADR-0069 D5 — the `auth.allowed_ip_ranges` setting outside the gate itself + * (#30203). + * + * - {@link parseAllowedIpRanges} is THE reading of the setting's textarea, for + * the settings binding (`bindAuthSettings`) and for the save predicate below, + * so the list a save is judged by is the list the gate then enforces. + * - {@link allowedIpRangesWritePredicate} refuses a save, through the settings + * console, of a list that leaves out the address the saving request itself + * comes from. Saved, such a list refuses the administrator's own next + * sign-in, and nothing at the save said so. + * + * The address judged at the save is the one the gate judges at sign-in: the + * auth manager's {@link AuthManager.resolveClientAddress}, from the same two + * facts (the transport's peer address and the request headers) under the same + * `server.trustProxy` declaration, and matched by the same + * {@link ipMatchesRange}. A request whose address the server cannot establish + * is refused at the gate whenever a list is set, so a non-empty list is refused + * at the save for it too. + * + * Only a write that arrives over HTTP is judged. An in-process write (boot, + * seed, a migration) carries no request, so there is no caller connection to + * lock out, and it is let through. + * + * The way back when locked out anyway — a list saved from another address, or + * an address that changed — is the settings env override every auth key has: + * `OS_AUTH_ALLOWED_IP_RANGES` set to an empty value, then a restart. The + * refusal names it. + */ + +import { resolveLocale } from '@objectstack/core'; +import { ipMatchesRange, type AuthManager } from './auth-manager.js'; + +/** The settings env override that pins `auth.allowed_ip_ranges` (`OS_` + namespace + key). */ +export const ALLOWED_IP_RANGES_ENV = 'OS_AUTH_ALLOWED_IP_RANGES'; + +/** + * The setting's textarea as a list: entries separated by newlines or commas, + * trimmed, blanks dropped. Anything that is not a string reads as no list. + */ +export function parseAllowedIpRanges(raw: unknown): string[] { + if (typeof raw !== 'string') return []; + return raw + .split(/[\n,]+/) + .map((r) => r.trim()) + .filter(Boolean); +} + +/** + * The HTTP request a settings write arrived on, as the settings routes hand it + * to a write predicate (`SettingsWriteOptions.transport` in `service-settings`). + */ +interface WriteTransport { + headers?: Record; + remoteAddress?: string; +} + +/** + * What this predicate reads of a settings write predicate's input. Structural, + * for the reason `SettingsReadSurface` is: plugin-auth does not depend on + * service-settings. + */ +export interface AllowedIpRangesWriteInput { + next: unknown; + transport?: WriteTransport; + locale?: string; +} + +/** A refusal, in the settings write predicate's shape. */ +export interface AllowedIpRangesWriteRefusal { + message: string; + constraint: { clientAddress: string | null }; +} + +interface RefusalCopy { + outside(address: string): string; + undetermined: string; + recovery: string; +} + +/** One entry per locale the settings bundle ships. */ +const COPY: Record<'en' | 'zh-CN' | 'ja-JP' | 'es-ES', RefusalCopy> = { + en: { + outside: (address) => + `No entry in this list matches ${address}, the address you are connected from, so saving it would refuse your ` + + 'own next sign-in. Add your address or its range to the list, or save the list from an address it matches.', + undetermined: + 'The server cannot establish the address you are connected from, and while any list is set it refuses every ' + + 'such request, your own next sign-in included. Save the list from a connection whose address the server can ' + + 'establish.', + recovery: ` If you are locked out, start the server with ${ALLOWED_IP_RANGES_ENV} set to an empty value.`, + }, + 'zh-CN': { + outside: (address) => + `这份列表中没有任何一项与你当前连接所用的地址 ${address} 匹配,保存后你自己下一次登录就会被拒绝。` + + '请把你的地址或所在网段加进列表,或在列表能匹配的地址上保存。', + undetermined: + '服务器无法确定你当前连接所用的地址;只要设置了列表,这样的请求都会被拒绝,你自己下一次登录也不例外。' + + '请在服务器能确定地址的连接上保存。', + recovery: `如果已被锁在外面,可将环境变量 ${ALLOWED_IP_RANGES_ENV} 设为空值后重启服务器。`, + }, + 'ja-JP': { + outside: (address) => + `このリストには、現在接続しているアドレス ${address} に一致する項目がないため、保存するとご自身の次回のサインインが拒否されます。` + + 'ご自身のアドレスまたはその範囲をリストに追加するか、リストに一致するアドレスから保存してください。', + undetermined: + 'サーバーは現在の接続のアドレスを確定できません。リストが設定されている間はそのようなリクエストはすべて拒否され、' + + 'ご自身の次回のサインインも同様です。サーバーがアドレスを確定できる接続から保存してください。', + recovery: `締め出された場合は、環境変数 ${ALLOWED_IP_RANGES_ENV} を空の値にしてサーバーを再起動してください。`, + }, + 'es-ES': { + outside: (address) => + `Ninguna entrada de esta lista coincide con ${address}, la dirección desde la que está conectado, así que al ` + + 'guardarla se rechazaría su propio próximo inicio de sesión. Añada su dirección o su rango a la lista, o guarde ' + + 'la lista desde una dirección con la que coincida.', + undetermined: + 'El servidor no puede determinar la dirección desde la que está conectado y, mientras haya una lista, rechaza ' + + 'todas esas solicitudes, incluido su propio próximo inicio de sesión. Guarde la lista desde una conexión cuya ' + + 'dirección el servidor pueda determinar.', + recovery: ` Si queda bloqueado, inicie el servidor con ${ALLOWED_IP_RANGES_ENV} con un valor vacío.`, + }, +}; + +const COPY_LOCALES = Object.keys(COPY); + +function copyFor(locale: string | undefined): RefusalCopy { + const picked = locale ? resolveLocale(locale, COPY_LOCALES) : undefined; + return COPY[(picked ?? 'en') as keyof typeof COPY]; +} + +/** + * `auth.allowed_ip_ranges`'s settings write predicate. `manager` is read per + * write, so the address rule is the one in force when the save happens. + */ +export function allowedIpRangesWritePredicate( + manager: () => Pick | null | undefined, +): (input: unknown) => AllowedIpRangesWriteRefusal | undefined { + return (input) => { + // The settings write predicate's input (`SettingsWritePredicateInput`), read structurally. + const { next, transport, locale } = (input ?? {}) as AllowedIpRangesWriteInput; + if (!transport) return undefined; + const ranges = parseAllowedIpRanges(next); + if (ranges.length === 0) return undefined; + const mgr = manager(); + if (!mgr) return undefined; + const address = mgr.resolveClientAddress({ + ...(transport.headers ? { headers: transport.headers } : {}), + ...(transport.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}), + }); + if (address && ranges.some((range) => ipMatchesRange(address, range))) return undefined; + const copy = copyFor(locale); + return { + message: (address ? copy.outside(address) : copy.undetermined) + copy.recovery, + constraint: { clientAddress: address ?? null }, + }; + }; +} diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 1a10724e0d2..58245fe6d16 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -120,6 +120,8 @@ import { } from './demo-staff.js'; import { cloudManagedRefusal } from './cloud-managed.js'; import { registerCopyInvitationLinkAction } from './invitation-link-action.js'; +import { allowedIpRangesWritePredicate, parseAllowedIpRanges } from './allowed-ip-ranges.js'; +import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address-forwarding.js'; import { runAdminBanUser, runAdminUnbanUser, @@ -173,6 +175,17 @@ interface SettingsReadSurface { value: unknown, ctx?: Record, ): Promise; + /** + * [#30203] Register a rule that may refuse a change to one key, asked before + * the write lands (`SettingsService.registerWritePredicate`). Optional, like + * `subscribe`. + */ + registerWritePredicate?( + namespace: string, + key: string, + predicate: (input: unknown) => unknown, + options?: { requestHeaders?: readonly string[] }, + ): void; } /** [#23367] The auth setting that records when the API-key lifecycle policy took effect. */ @@ -530,6 +543,15 @@ export class AuthPlugin implements Plugin { */ private allowListUndeterminedWarned = false; private allowListUntrustedHeaderWarned = false; + /** + * [#30203] The allow-list the deployment itself declared (plugin options or + * an `auth:configure` handler), taken before any setting is applied: what + * the running list returns to when the stored `allowed_ip_ranges` is cleared + * or restored to its default. + */ + private declaredAllowedIpRanges: string[] | undefined; + /** [#30203] Whether the running allow-list is the one a stored setting gave it. */ + private allowedIpRangesFromSettings = false; private configuredSocialProviders: SocialProviderConfig | undefined; // ADR-0092 D6 — the EFFECTIVE better-auth secondaryStorage (host-supplied or // the kernel-cache adapter wired in init). The identity write guard's @@ -766,6 +788,7 @@ export class AuthPlugin implements Plugin { logger: ctx.logger, }); + this.declaredAllowedIpRanges = authConfig.allowedIpRanges ? [...authConfig.allowedIpRanges] : undefined; // Initialize auth manager with data engine this.authManager = new AuthManager(authConfig); // ADR-0092 D6 — remember the storage better-auth will actually use so the @@ -2289,12 +2312,16 @@ export class AuthPlugin implements Plugin { } // Network gating (ADR-0069 D5) — parse the CIDR/IP textarea into a list. + // [#30203] A cleared or restored-default value resolves with source + // `default`. It hands the gate back to the deployment's own declaration + // at once, rather than leaving the last stored list in force until a + // restart while the console shows the field empty. if (isExplicit('allowed_ip_ranges')) { - const raw = asTrimmedString(values.allowed_ip_ranges) ?? ''; - patch.allowedIpRanges = raw - .split(/[\n,]+/) - .map((r) => r.trim()) - .filter(Boolean); + patch.allowedIpRanges = parseAllowedIpRanges(values.allowed_ip_ranges); + this.allowedIpRangesFromSettings = true; + } else if (this.allowedIpRangesFromSettings) { + patch.allowedIpRanges = this.declaredAllowedIpRanges ? [...this.declaredAllowedIpRanges] : []; + this.allowedIpRangesFromSettings = false; } // Anti-abuse (ADR-0069 D2) — account lockout (custom, per-identity) @@ -2508,6 +2535,26 @@ export class AuthPlugin implements Plugin { } }; + // [#30203] A console save of an allow-list that leaves out the saving + // request's own address is refused, naming the address (`allowed-ip-ranges.ts`). + if (typeof settings.registerWritePredicate === 'function') { + try { + settings.registerWritePredicate( + 'auth', + 'allowed_ip_ranges', + allowedIpRangesWritePredicate(() => this.authManager), + // Only the forwarded-address headers the address rule reads reach it. + { requestHeaders: FORWARDED_CLIENT_ADDRESS_HEADERS }, + ); + } catch (err: any) { + ctx.logger.warn( + '[auth] allowed_ip_ranges: the save-time check was NOT registered ' + + `(${String(err?.message ?? err)}), so the settings console accepts an allow-list that leaves out ` + + "the saving administrator's own address. The allow-list itself is still enforced.", + ); + } + } + await applySettings(); if (typeof settings.subscribe === 'function') { settings.subscribe('auth', () => { diff --git a/packages/services/service-settings/src/index.ts b/packages/services/service-settings/src/index.ts index ba9b6356c56..86c14b16837 100644 --- a/packages/services/service-settings/src/index.ts +++ b/packages/services/service-settings/src/index.ts @@ -59,9 +59,12 @@ export { type SettingsWritePredicateInput, type SettingsWriteRefusal, type SettingsDisplayOptions, + type SettingsWriteOptions, + type SettingsWritePredicateOptions, SETTINGS_LIVE_STATUS_BANNER_ID, type SettingsAuditSink, type SettingsContext, + type SettingsRequestTransport, type SettingsDiagnosticsLogger, type SettingsEngine, type SettingsRow, diff --git a/packages/services/service-settings/src/settings-routes.ts b/packages/services/service-settings/src/settings-routes.ts index 19b90512ee2..88b6ace1807 100644 --- a/packages/services/service-settings/src/settings-routes.ts +++ b/packages/services/service-settings/src/settings-routes.ts @@ -355,8 +355,17 @@ export function registerSettingsRoutes( ns, dropEchoedSecretMasks(body, secretKeys), ctx, - // The language a write predicate's refusal is written in. - { locale: requestLocaleOf(req) }, + // The language a write predicate's refusal is written in, and [#30203] + // the request's own connection, for a predicate that judges the change + // against it (`SettingsWriteOptions.transport`). Each predicate sees + // only the headers it named at registration (`narrowTransport`). + { + locale: requestLocaleOf(req), + transport: { + headers: req.headers, + ...(req.remoteAddress ? { remoteAddress: req.remoteAddress } : {}), + }, + }, ); // The write response carries resolved values too — including cascade // entries the caller never submitted (an upper-scope secret it may not diff --git a/packages/services/service-settings/src/settings-service.ts b/packages/services/service-settings/src/settings-service.ts index 3e0927da0d4..eea4d9db319 100644 --- a/packages/services/service-settings/src/settings-service.ts +++ b/packages/services/service-settings/src/settings-service.ts @@ -23,7 +23,10 @@ import { type SettingsLiveStatus, type SettingsLiveStatusProvider, type SettingsWritePredicate, + type SettingsWritePredicateOptions, + type SettingsRequestTransport, type SettingsDisplayOptions, + type SettingsWriteOptions, SETTINGS_LIVE_STATUS_BANNER_ID, type SettingsAuditSink, type SettingsContext, @@ -893,7 +896,10 @@ export class SettingsService { /** Live-status providers by namespace — see {@link registerLiveStatus}. */ private readonly liveStatusProviders = new Map(); /** Write predicates by namespace, then key — see {@link registerWritePredicate}. */ - private readonly writePredicates = new Map>(); + private readonly writePredicates = new Map< + string, + Map }> + >(); /** Change subscribers, optionally scoped to a namespace. */ private readonly subscribers = new Set<{ ns?: string; @@ -1659,15 +1665,24 @@ export class SettingsService { * Register (or replace) the write predicate of one manifest key — a rule * over engine state that may refuse a CHANGE to the key. See * {@link SettingsWritePredicate}. The last registration wins, the same - * override rule as {@link registerAction}. + * override rule as {@link registerAction}. `options.requestHeaders` names the + * request headers the predicate may read ({@link SettingsWritePredicateOptions}). */ - registerWritePredicate(namespace: string, key: string, predicate: SettingsWritePredicate): void { + registerWritePredicate( + namespace: string, + key: string, + predicate: SettingsWritePredicate, + options: SettingsWritePredicateOptions = {}, + ): void { const reg = this.registry.get(namespace); if (!reg) throw new UnknownNamespaceError(namespace); if (!reg.scopes.has(key)) throw new UnknownKeyError(namespace, key); let byKey = this.writePredicates.get(namespace); if (!byKey) this.writePredicates.set(namespace, (byKey = new Map())); - byKey.set(key, predicate); + byKey.set(key, { + predicate, + requestHeaders: new Set((options.requestHeaders ?? []).map((name) => name.toLowerCase())), + }); } /** @@ -2345,14 +2360,15 @@ export class SettingsService { * the row it would store names no owner, and the user rung answers only its * owner, so no reader would ever see it while the write reported success. * - * `display` picks the language of a write predicate's refusal, and nothing - * else (see {@link refuseByWritePredicates}). + * `display` picks the language of a write predicate's refusal and hands it + * the request the write arrived on, and nothing else (see + * {@link refuseByWritePredicates}). */ async setMany( namespace: string, patch: Record, ctx: SettingsContext = {}, - display: SettingsDisplayOptions = {}, + display: SettingsWriteOptions = {}, ): Promise> { // The pre-bind window: refuse rather than resolve against a store nothing // will ever read. See `assertEngineBound`. @@ -2665,20 +2681,30 @@ export class SettingsService { patch: Record, ctx: SettingsContext, reach: OrganizationReach | undefined, - display: SettingsDisplayOptions, + display: SettingsWriteOptions, ): Promise { const byKey = this.writePredicates.get(namespace); if (!byKey) return; const refusals: FieldError[] = []; for (const [key, submitted] of Object.entries(patch)) { - const predicate = byKey.get(key); - if (!predicate) continue; + const registered = byKey.get(key); + if (!registered) continue; + const { predicate } = registered; const next = submitted ?? null; const current = (await this.get(namespace, key, ctx)).value ?? null; if (stableStringify(next) === stableStringify(current)) continue; const scope = reg.scopes.get(key)!; const organizationId = scope === 'global' || !reach ? undefined : organizationReachRowOwner(reach) ?? null; - const refusal = await predicate({ namespace, key, next, current, organizationId, ctx, locale: display.locale }); + const refusal = await predicate({ + namespace, + key, + next, + current, + organizationId, + ctx, + locale: display.locale, + ...(display.transport ? { transport: narrowTransport(display.transport, registered.requestHeaders) } : {}), + }); if (!refusal) continue; const spec = ((reg.manifest.specifiers ?? []) as Array>) .find((s) => s.key === key); @@ -3796,6 +3822,24 @@ function stableStringify(input: unknown): string { } /** Re-typed env coercer (the canonical one lives in settings-service.types). */ +/** + * [#30203] The request as one write predicate sees it: the peer address, and + * only the headers it named at registration. + */ +function narrowTransport( + transport: SettingsRequestTransport, + names: ReadonlySet, +): SettingsRequestTransport { + const headers: Record = {}; + for (const [name, value] of Object.entries(transport.headers ?? {})) { + if (names.has(name.toLowerCase())) headers[name] = value; + } + return { + headers, + ...(transport.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}), + }; +} + function coerceEnvValue(raw: string, hint: unknown): unknown { if (typeof hint === 'boolean') return raw === 'true' || raw === '1' || raw === 'yes'; if (typeof hint === 'number') { diff --git a/packages/services/service-settings/src/settings-service.types.ts b/packages/services/service-settings/src/settings-service.types.ts index 1d5231f8518..442147e9d82 100644 --- a/packages/services/service-settings/src/settings-service.types.ts +++ b/packages/services/service-settings/src/settings-service.types.ts @@ -99,6 +99,28 @@ export interface SettingsDisplayOptions { locale?: string; } +/** + * [#30203] The HTTP request a write arrived on: its headers and the + * transport's own peer address (`IHttpRequest.headers` / `remoteAddress`). + * It identifies and authorizes nothing (that is {@link SettingsContext}'s), and + * it is never stored or logged. + */ +export interface SettingsRequestTransport { + headers?: Record; + remoteAddress?: string; +} + +/** How a settings WRITE is carried out beyond its context: its prose, and the request it arrived on. */ +export interface SettingsWriteOptions extends SettingsDisplayOptions { + /** + * [#30203] The request the write arrived on. The settings routes set it on a + * save, for a {@link SettingsWritePredicate} that judges the change against + * the caller's own connection (plugin-auth's `allowed_ip_ranges`, which must + * not lock out the address saving it). Absent on an in-process write. + */ + transport?: SettingsRequestTransport; +} + /** Storage row shape used by both the engine and the in-memory store. */ export interface SettingsRow { namespace: string; @@ -357,6 +379,24 @@ export interface SettingsWritePredicateInput { ctx: SettingsContext; /** The language the refusal is written in — {@link SettingsDisplayOptions.locale}. */ locale?: string; + /** + * [#30203] The request the write arrived on ({@link SettingsWriteOptions.transport}), + * narrowed to the headers this predicate declared at registration + * ({@link SettingsWritePredicateOptions.requestHeaders}); absent on an + * in-process write. + */ + transport?: SettingsRequestTransport; +} + +/** [#30203] How a {@link SettingsWritePredicate} is registered. */ +export interface SettingsWritePredicateOptions { + /** + * The request headers the predicate reads, lower-case. Only these reach its + * `transport.headers`, so a credential header (`cookie`, `authorization`, an + * API key) never reaches a predicate that did not name it. None named: the + * predicate sees the peer address and no header. + */ + requestHeaders?: readonly string[]; } /** diff --git a/packages/services/service-settings/src/settings-write-predicate-transport.test.ts b/packages/services/service-settings/src/settings-write-predicate-transport.test.ts new file mode 100644 index 00000000000..ce4e8f28bb9 --- /dev/null +++ b/packages/services/service-settings/src/settings-write-predicate-transport.test.ts @@ -0,0 +1,108 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30203] What of a save's request reaches a write predicate. + * + * The settings routes hand the service the request a save arrived on + * (`SettingsWriteOptions.transport`). A predicate sees the transport's peer + * address and only the headers it named at registration + * (`SettingsWritePredicateOptions.requestHeaders`), so a credential header + * (`cookie`, `authorization`, an API key) never reaches a predicate that did + * not ask for it. An in-process write carries no request at all. + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts'; +import { SettingsService } from './settings-service.js'; +import { registerSettingsRoutes } from './settings-routes.js'; +import { brandingSettingsManifest } from './manifests/branding.manifest.js'; +import type { SettingsWritePredicateInput } from './settings-service.types.js'; + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +const REQUEST_HEADERS = { + cookie: 'better-auth.session_token=secret', + authorization: 'Bearer secret', + 'x-api-key': 'osk_secret', + 'x-forwarded-for': '198.51.100.7', + 'x-real-ip': '198.51.100.8', +}; + +function rig(options?: { requestHeaders?: readonly string[] }) { + const service = new SettingsService({ env: {} }); + service.registerManifest(brandingSettingsManifest); + const seen: SettingsWritePredicateInput[] = []; + service.registerWritePredicate( + 'branding', + 'workspace_name', + (input) => { + seen.push(input); + return undefined; + }, + options, + ); + const http = new MockHttp(); + registerSettingsRoutes(http, service, { + contextFromRequest: () => ({ enforced: true, permissions: ['setup.access', 'setup.write'], userId: 'usr_admin' }), + }); + const save = async (value: string) => { + let status = 200; + const res: IHttpResponse = { + json: vi.fn() as any, + send: vi.fn() as any, + status: vi.fn((code: number) => { status = code; return res; }) as any, + header: vi.fn(() => res) as any, + }; + const req: IHttpRequest = { + params: { namespace: 'branding' }, + query: {}, + body: { workspace_name: value }, + headers: { ...REQUEST_HEADERS }, + method: 'PUT', + path: '/api/settings/branding', + remoteAddress: '10.0.0.1', + }; + await http.routes.get('PUT /api/settings/:namespace')!(req, res); + return status; + }; + return { service, seen, save }; +} + +describe('#30203 a write predicate sees only the request headers it named', () => { + it('a predicate that named the forwarded-address headers gets those and the peer address, no credential', async () => { + const { seen, save } = rig({ requestHeaders: ['X-Forwarded-For', 'x-real-ip'] }); + expect(await save('Acme')).toBe(200); + expect(seen).toHaveLength(1); + expect(seen[0]!.transport).toEqual({ + headers: { 'x-forwarded-for': '198.51.100.7', 'x-real-ip': '198.51.100.8' }, + remoteAddress: '10.0.0.1', + }); + }); + + it('a predicate that named no header gets the peer address and no header', async () => { + const { seen, save } = rig(); + expect(await save('Acme')).toBe(200); + expect(seen[0]!.transport).toEqual({ headers: {}, remoteAddress: '10.0.0.1' }); + }); + + it('an in-process write carries no request', async () => { + const { service, seen } = rig({ requestHeaders: ['x-forwarded-for'] }); + await service.set('branding', 'workspace_name', 'Acme', {}); + expect(seen).toHaveLength(1); + expect(seen[0]!).not.toHaveProperty('transport'); + }); +});