fix(plugin-auth)!: an allowed IP range that names no range is refused at save and never matches at the gate

.changeset/allowed-ip-range-entry-rule.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/plugin-auth': patch
+---
+
+fix(plugin-auth)!: an allowed IP range that names no range is refused at save and never matches at the gate
+
+Clause-②: no (narrowing)
+Metadata-protocol: none
+Skills-docs: updated here — `content/docs/permissions/authentication.mdx` (the `allowed_ip_ranges` row, and the first two rules under "Saving, clearing and lockout recovery") and `docs/HARDENING.md` (the Network row); `skills/**` teaches no `allowed_ip_ranges`
+
+<!-- adr-0087: not-required (no-migration-prescription) no metadata an author writes changes and no stored row is rewritten: a value of the `auth.allowed_ip_ranges` setting that is not text, or an entry of it or of the auth plugin's `allowedIpRanges` option that names no range, is refused where it is saved or declared, and matches no address at the sign-in gate -->
+
+**BREAKING** for a deployment whose `auth.allowed_ip_ranges` (ADR-0069 D5), `OS_AUTH_ALLOWED_IP_RANGES` or declared `allowedIpRanges` holds an entry that is not an IP address or a CIDR range, or whose stored `auth.allowed_ip_ranges` value is not text.
+
+The setting holds its entries as text, one per line or comma-separated; a value that is not text (a list, a number, an object) names no list. An entry is an IPv4 or IPv6 address, or an address, `/` and a prefix length written in decimal digits: one or two digits, from 0 to 32, for an IPv4 base (an IPv4-mapped one included), and one to three digits, from 0 to 128, for an IPv6 base. Every such entry keeps its meaning. Any other entry names no range, and one reading of that rule now serves every door that judges or saves the list:
+
+- **The sign-in gate.** An entry that names no range matches no address, its own base address included, so a list holding one admits exactly what its other entries admit. `ipMatchesRange` answers `false` for it.
+- **The settings save.** A value that is not a list of entries written as text is refused with `400 SETTINGS_VALIDATION`, `invalid_value` at `allowed_ip_ranges` and `constraint: { type: 'textarea' }`. A list holding an entry that names no range is refused the same way, the entries in `constraint.invalidEntries`, with a message in the caller's language that names them. Both are judged before the saving address, on every write: the console's `PUT /api/settings/auth` and writes made inside the server alike. Saving the value already in force again is not judged, as before.
+- **The declared list.** An `allowedIpRanges` passed to `AuthPlugin` or `AuthManager`, or set by an `auth:configure` handler, that holds one, or that is not an array, is refused at construction with an error naming each such entry.
+- **A list already stored, or pinned by `OS_AUTH_ALLOWED_IP_RANGES`.** It is applied, the entry matches no address, and the settings binding logs one `error` that names the entry and the fix. A stored value that is not text admits no address while it is in force, and the binding logs one `error` that says so and names the fix.
+- **Who moves.** A deployment with such an entry corrects it to an address, or an address and a prefix length (`203.0.113.0/24`, `2001:db8::/32`), or removes it. A deployment whose stored value is not text saves the list again as text, or clears it. Locked out meanwhile, start the server with `OS_AUTH_ALLOWED_IP_RANGES` set to an empty value, sign in, clear the stored value (`PUT /api/settings/auth` with `allowed_ip_ranges: null`, the one write the override lets through), remove the variable and restart, then save the corrected list.

content/docs/permissions/authentication.mdx

@@ -619,7 +619,7 @@ often before the account-level lock engages.
 
 | Setting | Effect |
 |---|---|
-| `allowed_ip_ranges` | Restrict authenticated access to one or more IPv4 CIDR ranges (or exact IPs). The address judged is the one the server establishes, by the same rule as the inbound rate limiter: `X-Forwarded-For` / `X-Real-IP` only when the stack declares `server.trustProxy`, the connection's peer address otherwise. Behind a reverse proxy, declare `server.trustProxy`, or the allow-list judges the proxy's own address. A request whose address cannot be established is refused with `403 IP_NOT_ALLOWED`, like one outside the ranges. `last_login_ip` records the same address. A save that leaves out your own address is refused, and clearing the field applies at once: see [Saving, clearing and lockout recovery](#saving-clearing-and-lockout-recovery). |
+| `allowed_ip_ranges` | Restrict authenticated access to one or more CIDR ranges or exact addresses, IPv4 or IPv6 (`203.0.113.0/24`, `2001:db8::/32`, `192.0.2.7`). An entry that is neither is refused when you save it. The address judged is the one the server establishes, by the same rule as the inbound rate limiter: `X-Forwarded-For` / `X-Real-IP` only when the stack declares `server.trustProxy`, the connection's peer address otherwise. Behind a reverse proxy, declare `server.trustProxy`, or the allow-list judges the proxy's own address. A request whose address cannot be established is refused with `403 IP_NOT_ALLOWED`, like one outside the ranges. `last_login_ip` records the same address. A save that leaves out your own address is refused, and clearing the field applies at once: see [Saving, clearing and lockout recovery](#saving-clearing-and-lockout-recovery). |
 
 > **How enforcement works.** Password expiry (an administrator-forced password
 > change included) and enforced MFA share a single *session-validation gate*.
@@ -654,6 +654,25 @@ often before the account-level lock engages.
 
 These rules apply to `allowed_ip_ranges`.
 
+- **An entry the server cannot read is refused.** Each entry is an IPv4 or IPv6
+  address, or an address followed by `/` and a prefix length written in decimal
+  digits: one or two digits, from 0 to 32, for IPv4, and one to three digits, from
+  0 to 128, for IPv6. Any other entry is refused with `400 SETTINGS_VALIDATION`,
+  whatever address you save from, and nothing is stored. The field error names the
+  entries, also listed in `constraint.invalidEntries`. This check runs before the
+  address check below, on every write, including writes made inside the server. An
+  `allowedIpRanges` list the deployment declares in its own auth configuration is
+  held to the same rule and refused at startup. An entry the server cannot read
+  matches no address at sign-in. If a stored list or `OS_AUTH_ALLOWED_IP_RANGES`
+  already holds one, the server logs it at startup with the entry named, and only
+  the list's other entries admit anyone.
+- **A value that is not text is refused.** The field holds its entries as text, one
+  per line or comma-separated. A value written in another form, such as a JSON list,
+  a number or an object, names no list: through the API or by a write inside the
+  server, it is refused the same way, with `constraint: { type: 'textarea' }`, and
+  nothing is stored. A value of another
+  form stored before this check admits no address at sign-in, and the server logs
+  it at startup. Save the list as text, or clear it.
 - **A save that would lock you out is refused.** When you save the field, the server
   checks the new list against the address your own request comes from. It finds that
   address by the same rule as the gate (`server.trustProxy` decides whether the
@@ -668,7 +687,7 @@ These rules apply to `allowed_ip_ranges`.
   If the server cannot establish your address at all, any non-empty list is refused,
   because the gate would refuse you too. Only a change is checked: saving the list
   already in force again is not refused. Writes made inside the server (boot, seed,
-  migrations) carry no request and are not checked.
+  migrations) carry no request, so no address is checked for them.
 - **Setup shows your address before you save.** Under the field, **Setup →
   Authentication** shows the address the server judges for your request, read from
   `GET /api/v1/auth/admin/client-address` (platform administrators only), which

docs/HARDENING.md

@@ -241,7 +241,7 @@ production deployment, enable at least:
 | Anti-abuse  | `lockout_threshold: 5`, `lockout_duration_minutes: 15`, `rate_limit_max` / `rate_limit_window_seconds` |
 | MFA         | `mfa_required: true` (or per-org `require_mfa`, which needs two-factor enrollment on), `mfa_grace_period_days: 7`; neither saves while `OS_AUTH_TWO_FACTOR=false` |
 | Sessions    | `session_idle_timeout_minutes: 30–60`, `session_absolute_max_hours: 8–12`, `max_concurrent_sessions_per_user` |
-| Network     | `allowed_ip_ranges` (IPv4 CIDR) for tenant- or office-scoped access; locked out, restart with `OS_AUTH_ALLOWED_IP_RANGES=` (empty), sign in, clear the stored list (`PUT /api/settings/auth` with `allowed_ip_ranges: null`), then remove the variable |
+| Network     | `allowed_ip_ranges` (IPv4 or IPv6 CIDR, or exact addresses) for tenant- or office-scoped access; locked out, restart with `OS_AUTH_ALLOWED_IP_RANGES=` (empty), sign in, clear the stored list (`PUT /api/settings/auth` with `allowed_ip_ranges: null`), then remove the variable |
 
 Password expiry and enforced MFA share a *session-validation gate*: a
 non-compliant session can authenticate but is blocked from data access (auth +

packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts

@@ -34,7 +34,7 @@
  * `vitest.config.ts`), so the routes and the service are the checkout's.
  */
 
-import { afterEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, describe, expect, it, vi, type Mock } from 'vitest';
 import { Hono } from 'hono';
 import type { IHttpRequest, IHttpResponse, IHttpServer, RouteHandler } from '@objectstack/spec/contracts';
 import {
@@ -73,6 +73,7 @@ const PLATFORM_ADMIN_SESSION = { user: { id: 'usr_admin', email: 'admin@example.
 interface Rig {
   svc: SettingsService;
   manager: AuthManager;
+  logger: { info: Mock; error: Mock; warn: Mock; debug: Mock };
   save(body: Record<string, unknown>, transport?: Transport): Promise<{ status: number; body: any }>;
   /** `GET /api/v1/auth/admin/client-address` on the plugin's real auth routes. */
   read(transport?: Transport, opts?: { session?: unknown; query?: string }):
@@ -153,7 +154,7 @@ async function boot(
       getApi.mockRestore();
     }
   };
-  return { svc, manager, save, read };
+  return { svc, manager, logger: ctx.logger, save, read };
 }
 
 const stored = async (svc: SettingsService) => (await svc.get('auth', 'allowed_ip_ranges', {})).value ?? null;
@@ -390,6 +391,156 @@ describe('#30398 the address read serves the address the save check judges', ()
   });
 });
 
+describe('an allowed IP range whose prefix is not decimal digits within its family is refused at every save door and never matches at the gate', () => {
+  // An entry is an address, or an address, `/` and a prefix length of one or
+  // two decimal digits from 0 to 32 (IPv4) or one to three from 0 to 128
+  // (IPv6). Each entry here is neither. The saving address, 10.1.2.3, is one
+  // the list's other entry matches, so only the entry itself can refuse it.
+  const UNREADABLE = ['10.0.0.0/', '10.0.0.0/ 24', '10.0.0.0/0x18', '10.0.0.0/33', '10.0.0.0/8/8', '2001:db8::/', '2001:db8::/129', 'office'];
+  const SAVER = { remoteAddress: '10.1.2.3' };
+
+  it('the console save refuses a list holding one, names it, and stores nothing', async () => {
+    const { svc, save } = await boot();
+    for (const entry of UNREADABLE) {
+      for (const list of [entry, `10.1.2.3\n${entry}`]) {
+        const res = await save({ allowed_ip_ranges: list }, SAVER);
+        expect(res.status, JSON.stringify(list)).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({ invalidEntries: [entry] });
+        expect(field.message).toContain(entry);
+        expect(await stored(svc)).toBeNull();
+      }
+    }
+  });
+
+  it('the console save names the unreadable entry first, whatever address it is saved from', async () => {
+    const { save } = await boot();
+    for (const transport of [{}, { remoteAddress: '198.51.100.1' }]) {
+      const res = await save({ allowed_ip_ranges: '203.0.113.0/24, 10.0.0.0/, 2001:db8::/0x20' }, transport);
+      expect(res.status).toBe(400);
+      expect(res.body.error.code).toBe('SETTINGS_VALIDATION');
+      expect(res.body.error.details.fields[0].field).toBe('allowed_ip_ranges');
+      expect(res.body.error.details.fields[0].code).toBe('invalid_value');
+      expect(res.body.error.details.fields[0].constraint).toEqual({ invalidEntries: ['10.0.0.0/', '2001:db8::/0x20'] });
+    }
+  });
+
+  it('an in-process write of a list holding one is refused too', async () => {
+    const { svc } = await boot();
+    const err: any = await svc.set('auth', 'allowed_ip_ranges', '10.0.0.0/', {}).then(() => null, (e) => e);
+    expect(err?.code).toBe('SETTINGS_VALIDATION');
+    expect(err.fields).toEqual([
+      expect.objectContaining({ field: 'allowed_ip_ranges', code: 'invalid_value', constraint: { invalidEntries: ['10.0.0.0/'] } }),
+    ]);
+    expect(await stored(svc)).toBeNull();
+  });
+
+  it("the deployment's own declared allowedIpRanges holding one refuses the boot, naming it", async () => {
+    await expect(boot({ plugin: { allowedIpRanges: ['10.0.0.0/8', '10.0.0.0/'] } })).rejects.toThrow(
+      /allowedIpRanges.*"10\.0\.0\.0\/"/,
+    );
+  });
+
+  it('a list stored before the save check admits no client through the unreadable entry, and the boot names it', async () => {
+    const svc = new SettingsService({ env: {} });
+    svc.registerManifest(authManifest);
+    // Written before the plugin registers its save check: the shape of a list stored by an earlier release.
+    await svc.set('auth', 'allowed_ip_ranges', '10.0.0.0/\n192.0.2.7', {});
+    const { manager, logger } = await boot({ svc });
+    expect(manager.isClientIpAllowed('10.1.2.3')).toBe(false);
+    expect(manager.isClientIpAllowed('8.8.8.8')).toBe(false);
+    expect(manager.isClientIpAllowed('192.0.2.7')).toBe(true);
+    expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('"10.0.0.0/"'));
+  });
+
+  it('OS_AUTH_ALLOWED_IP_RANGES holding one admits no client through it, and the boot names it', async () => {
+    const { manager, logger } = await boot({ env: { OS_AUTH_ALLOWED_IP_RANGES: '10.0.0.0/, 192.0.2.7' } });
+    expect(manager.isClientIpAllowed('10.1.2.3')).toBe(false);
+    expect(manager.isClientIpAllowed('192.0.2.7')).toBe(true);
+    expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('"10.0.0.0/"'));
+  });
+
+  it('a list of well-formed entries saves and gates as before (preservation)', async () => {
+    const list = '10.0.0.0/8\n10.0.0.0/08, 192.0.2.7/32\n2001:db8::/32\n::1/128\n203.0.113.5\n2001:db8::1';
+    const { svc, manager, logger, save } = await boot();
+    const res = await save({ allowed_ip_ranges: list }, SAVER);
+    expect(res.status, JSON.stringify(res.body)).toBe(200);
+    expect(await stored(svc)).toBe(list);
+    await vi.waitFor(() => expect(manager.isClientIpAllowed('198.51.100.1')).toBe(false));
+    expect(manager.isClientIpAllowed('10.200.0.1')).toBe(true);
+    expect(manager.isClientIpAllowed('192.0.2.7')).toBe(true);
+    expect(manager.isClientIpAllowed('192.0.2.8')).toBe(false);
+    expect(manager.isClientIpAllowed('2001:db8:ffff::1')).toBe(true);
+    expect(manager.isClientIpAllowed('::1')).toBe(true);
+    expect(manager.isClientIpAllowed('203.0.113.5')).toBe(true);
+    expect(logger.error).not.toHaveBeenCalledWith(expect.stringContaining('allowed_ip_ranges'));
+  });
+});
+
+describe('an allowed_ip_ranges value that is not text is refused at every save door, and where it is stored the gate admits nobody through it', () => {
+  // The setting is a textarea: its value is text, one entry per line or
+  // comma-separated. Each value here is not text. The saving address,
+  // 10.1.2.3, is one each of them spells, so only the value's form can refuse it.
+  const NOT_TEXT: unknown[] = [['10.1.2.3'], ['10.0.0.0/8', '10.1.2.3'], [], 167838211, true, { ranges: '10.1.2.3' }];
+  const SAVER = { remoteAddress: '10.1.2.3' };
+
+  it('the console save refuses it and stores nothing', async () => {
+    const { svc, save } = await boot();
+    for (const value of NOT_TEXT) {
+      const res = await save({ allowed_ip_ranges: value }, SAVER);
+      expect(res.status, JSON.stringify(value)).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({ type: 'textarea' });
+      expect(field.message).toContain('not text');
+      expect(await stored(svc)).toBeNull();
+    }
+  });
+
+  it('it is refused whatever address it is saved from', async () => {
+    const { save } = await boot();
+    for (const transport of [{}, { remoteAddress: '198.51.100.1' }]) {
+      const res = await save({ allowed_ip_ranges: ['203.0.113.0/24'] }, transport);
+      expect(res.status).toBe(400);
+      expect(res.body.error.code).toBe('SETTINGS_VALIDATION');
+      expect(res.body.error.details.fields[0].code).toBe('invalid_value');
+      expect(res.body.error.details.fields[0].constraint).toEqual({ type: 'textarea' });
+    }
+  });
+
+  it('an in-process write of it is refused too', async () => {
+    const { svc } = await boot();
+    for (const value of NOT_TEXT) {
+      const err: any = await svc.set('auth', 'allowed_ip_ranges', value, {}).then(() => null, (e) => e);
+      expect(err?.code, JSON.stringify(value)).toBe('SETTINGS_VALIDATION');
+      expect(err.fields).toEqual([
+        expect.objectContaining({ field: 'allowed_ip_ranges', code: 'invalid_value', constraint: { type: 'textarea' } }),
+      ]);
+      expect(await stored(svc)).toBeNull();
+    }
+  });
+
+  it('a value stored before the save check admits no client, the boot names it, and saving the list as text replaces it', async () => {
+    for (const value of [['10.1.2.3'], 167838211, { ranges: '10.1.2.3' }]) {
+      const svc = new SettingsService({ env: {} });
+      svc.registerManifest(authManifest);
+      // Written before the plugin registers its save check: the shape of a value stored by an earlier release.
+      await svc.set('auth', 'allowed_ip_ranges', value, {});
+      const { manager, logger, save } = await boot({ svc });
+      expect(manager.isClientIpAllowed('10.1.2.3'), JSON.stringify(value)).toBe(false);
+      expect(manager.isClientIpAllowed('8.8.8.8')).toBe(false);
+      expect(logger.error).toHaveBeenCalledWith(expect.stringMatching(/allowed_ip_ranges.*not text/));
+      const res = await save({ allowed_ip_ranges: '10.1.2.3' }, SAVER);
+      expect(res.status, JSON.stringify(res.body)).toBe(200);
+      await vi.waitFor(() => expect(manager.isClientIpAllowed('10.1.2.3')).toBe(true));
+      expect(manager.isClientIpAllowed('8.8.8.8')).toBe(false);
+    }
+  });
+});
+
 describe('#30385 the env recovery clears the stored list, so lifting the override does not lock the caller out again', () => {
   it('set the override empty, sign in, clear the stored list through the settings door, remove the override: the caller is admitted', async () => {
     const env: Record<string, string | undefined> = {};