create: keep ids server-owned

.github/workflows/ci.yml

@@ -9,6 +9,9 @@ on:
 permissions:
   contents: read
 
+env:
+  NICTOOL_VALIDATE_SPEC: https://github.com/NicTool/validate/archive/45037764d5ef420be6536e66894da02f60527e5e.tar.gz
+
 jobs:
   lint:
     uses: NicTool/.github/.github/workflows/lint.yml@main
@@ -43,6 +46,8 @@ jobs:
           node-version: ${{ matrix.node-version }}
       - run: sh sql/init-mysql.sh
       - run: npm install
+      - name: Install proposed validate source
+        run: npm install --no-save ${{ env.NICTOOL_VALIDATE_SPEC }}
       - run: npm test
 
   test-mac:
@@ -63,6 +68,8 @@ jobs:
           node-version: ${{ matrix.node-version }}
       - run: sh sql/init-mysql.sh
       - run: npm install
+      - name: Install proposed validate source
+        run: npm install --no-save ${{ env.NICTOOL_VALIDATE_SPEC }}
       - run: npm test
 
   test-docker:
@@ -100,4 +107,6 @@ jobs:
           node-version: ${{ matrix.node-version }}
       - run: sh sql/init-mysql.sh
       - run: npm install
+      - name: Install proposed validate source
+        run: npm install --no-save ${{ env.NICTOOL_VALIDATE_SPEC }}
       - run: sh test/run.sh

CHANGELOG.md

@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 ### Unreleased
 
 - fix: reject create conflicts and duplicate permission targets
+- create: allocate ids internally and reject caller-supplied ids
 
 ### [3.0.3] - 2026-07-27
 

docker/Dockerfile

@@ -2,7 +2,11 @@ FROM node:22-trixie-slim
 WORKDIR /app
 COPY package*.json .
 # --omit=dev is safe: tests use node:test (stdlib), devDeps are only eslint/prettier
-RUN npm install --omit=dev
+ARG NICTOOL_VALIDATE_SPEC
+RUN npm install --omit=dev \
+  && if [ -n "$NICTOOL_VALIDATE_SPEC" ]; then \
+    npm install --omit=dev --no-save "$NICTOOL_VALIDATE_SPEC"; \
+  fi
 COPY . .
 EXPOSE 3000
 CMD ["node", "server.js"]

docker/docker-compose.yml

@@ -24,6 +24,8 @@ services:
     build:
       context: ..
       dockerfile: docker/Dockerfile
+      args:
+        NICTOOL_VALIDATE_SPEC: ${NICTOOL_VALIDATE_SPEC:-}
     ports:
       - '${API_PORT:-3000}:3000'
     depends_on:

lib/group/store/file.js

@@ -1,4 +1,4 @@
-import FileStore from '../../store/file.js'
+import FileStore, { nextId } from '../../store/file.js'
 import { idConflict } from '../../store/error.js'
 
 import GroupBase from './base.js'
@@ -60,37 +60,35 @@ class GroupRepoFile extends GroupBase {
 
   async create(args, options) {
     args = JSON.parse(JSON.stringify(args))
-
-    if (args.id !== undefined) {
-      const existing = [
-        ...(await this.get({ id: args.id })),
-        ...(await this.get({ id: args.id, deleted: true })),
-      ]
-      if (existing.length > 0) return idConflict('group', args.id, options)
-    }
-
     const usable_ns = args.usable_ns ?? []
     delete args.usable_ns
 
-    const gid = args.id
-    args.permissions = {
-      ...JSON.parse(JSON.stringify(defaultPermissions)),
-      id: gid,
-      name: `Group ${args.name} perms`,
-      user: { id: gid, create: false, write: false, delete: false },
-      group: { id: gid, create: false, write: false, delete: false },
-      nameserver: {
-        usable: Array.isArray(usable_ns) ? usable_ns : [],
-        create: false,
-        write: false,
-        delete: false,
-      },
-    }
+    return this.file.mutate('group', (groups, data) => {
+      if (args.id !== undefined && groups.some((group) => group.id === args.id)) {
+        return idConflict('group', args.id, options)
+      }
 
-    const groups = await this._load()
-    groups.push(args)
-    await this._save(groups)
-    return gid
+      if (args.id === undefined) args.id = nextId(groups, data.last_id)
+      data.last_id = Math.max(data.last_id ?? 0, args.id)
+
+      const gid = args.id
+      args.permissions = {
+        ...JSON.parse(JSON.stringify(defaultPermissions)),
+        id: gid,
+        name: `Group ${args.name} perms`,
+        user: { id: gid, create: false, write: false, delete: false },
+        group: { id: gid, create: false, write: false, delete: false },
+        nameserver: {
+          usable: Array.isArray(usable_ns) ? usable_ns : [],
+          create: false,
+          write: false,
+          delete: false,
+        },
+      }
+
+      groups.push(args)
+      return gid
+    })
   }
 
   async get(args_orig) {

lib/nameserver/store/file.js

@@ -1,4 +1,4 @@
-import FileStore from '../../store/file.js'
+import FileStore, { nextId } from '../../store/file.js'
 import { idConflict } from '../../store/error.js'
 
 import NameserverBase from './base.js'
@@ -40,18 +40,18 @@ class NameserverRepoFile extends NameserverBase {
   }
 
   async create(args, options) {
-    if (args.id !== undefined) {
-      const existing = [
-        ...(await this.get({ id: args.id })),
-        ...(await this.get({ id: args.id, deleted: true })),
-      ]
-      if (existing.length > 0) return idConflict('nameserver', args.id, options)
-    }
+    args = JSON.parse(JSON.stringify(args))
 
-    const nameservers = await this._load()
-    nameservers.push(JSON.parse(JSON.stringify(args)))
-    await this._save(nameservers)
-    return args.id
+    return this.file.mutate('nameserver', (nameservers, data) => {
+      if (args.id !== undefined && nameservers.some((nameserver) => nameserver.id === args.id)) {
+        return idConflict('nameserver', args.id, options)
+      }
+
+      if (args.id === undefined) args.id = nextId(nameservers, data.last_id, 0xffff)
+      data.last_id = Math.max(data.last_id ?? 0, args.id)
+      nameservers.push(args)
+      return args.id
+    })
   }
 
   async get(args) {

lib/session/store/file.js

@@ -1,4 +1,4 @@
-import FileStore from '../../store/file.js'
+import FileStore, { nextId } from '../../store/file.js'
 
 // Map legacy nt_* column names to the friendly API names used throughout.
 function normalizeArgs(args) {
@@ -33,15 +33,17 @@ class SessionRepoFile {
   async create(args) {
     args = normalizeArgs(JSON.parse(JSON.stringify(args)))
 
-    const existing = await this.get({ uid: args.uid, session: args.session })
-    if (existing) return existing.id
+    return this.file.mutate('session', (sessions, data) => {
+      const existing = sessions.find(
+        (session) => session.uid === args.uid && session.session === args.session,
+      )
+      if (existing) return existing.id
 
-    const sessions = await this._load()
-    const nextId = sessions.reduce((max, s) => Math.max(max, s.id ?? 0), 0) + 1
-    args.id = nextId
-    sessions.push(args)
-    await this._save(sessions)
-    return nextId
+      args.id = nextId(sessions, data.last_id)
+      data.last_id = args.id
+      sessions.push(args)
+      return args.id
+    })
   }
 
   async get(args) {

lib/store/file.js

@@ -19,11 +19,38 @@ const tomlCodec = {
 }
 
 const codecs = { json: jsonCodec, toml: tomlCodec }
+const mutations = new Map()
+
+async function serialize(file, callback) {
+  const previous = mutations.get(file) ?? Promise.resolve()
+  const current = previous.catch(() => {}).then(callback)
+  mutations.set(file, current)
+
+  try {
+    return await current
+  } finally {
+    if (mutations.get(file) === current) mutations.delete(file)
+  }
+}
 
 export function resolveCodec(type = storeConfig().type) {
   return codecs[type] ?? jsonCodec
 }
 
+export function nextId(rows, lastId = 0, maxId = 0xffffffff) {
+  if (!Number.isInteger(lastId) || lastId < 0) throw new TypeError('file store last id must be unsigned')
+
+  let highest = lastId
+  for (const row of rows) {
+    if (row.id === undefined) continue
+    if (!Number.isInteger(row.id) || row.id < 0) throw new TypeError('file store row id must be unsigned')
+    highest = Math.max(highest, row.id)
+  }
+
+  if (highest >= maxId) throw new RangeError(`file store id space exhausted at ${maxId}`)
+  return highest + 1
+}
+
 /**
  * One file per entity, holding an array of rows under a single top-level key.
  * The codec is chosen by store.type; everything above this layer is identical
@@ -53,20 +80,45 @@ export class FileStore {
     return path.join(base, `${this.basename}.${this.codec.ext}`)
   }
 
-  async load(key) {
+  async _read(file) {
     try {
-      const data = await this.codec.parse(await fs.readFile(this.filePath, 'utf8'))
-      return Array.isArray(data?.[key]) ? data[key] : []
+      const data = await this.codec.parse(await fs.readFile(file, 'utf8'))
+      return data && typeof data === 'object' && !Array.isArray(data) ? data : {}
     } catch (err) {
-      if (err.code === 'ENOENT') return []
+      if (err.code === 'ENOENT') return {}
       throw err
     }
   }
 
+  async _write(file, data) {
+    await fs.mkdir(path.dirname(file), { recursive: true })
+    await fs.writeFile(file, await this.codec.stringify(data))
+  }
+
+  async load(key) {
+    const data = await this._read(this.filePath)
+    return Array.isArray(data[key]) ? data[key] : []
+  }
+
   async save(key, rows) {
     const file = this.filePath
-    await fs.mkdir(path.dirname(file), { recursive: true })
-    await fs.writeFile(file, await this.codec.stringify({ [key]: rows }))
+    return serialize(file, async () => {
+      const data = await this._read(file)
+      data[key] = rows
+      await this._write(file, data)
+    })
+  }
+
+  async mutate(key, callback) {
+    const file = this.filePath
+    return serialize(file, async () => {
+      const data = await this._read(file)
+      const rows = Array.isArray(data[key]) ? data[key] : []
+      const result = await callback(rows, data)
+      data[key] = rows
+      await this._write(file, data)
+      return result
+    })
   }
 }
 

lib/store/file.test.js

@@ -4,14 +4,22 @@ import os from 'node:os'
 import path from 'node:path'
 import { describe, it, before, after, beforeEach } from 'node:test'
 
-import FileStore, { resolveCodec } from './file.js'
+import GroupRepoFile from '../group/store/file.js'
+import FileStore, { nextId, resolveCodec } from './file.js'
 
 const envKeys = ['NICTOOL_DATA_STORE', 'NICTOOL_DATA_STORE_PATH', 'NICTOOL_CONF_DIR']
 
 describe('file store', () => {
   const savedEnv = {}
   let tmp
 
+  it('allocates after the highest existing id', () => {
+    assert.equal(nextId([]), 1)
+    assert.equal(nextId([{ id: 9 }, { id: 2 }, {}]), 10)
+    assert.equal(nextId([{ id: 9 }], 12), 13)
+    assert.throws(() => nextId([{ id: 0xffffffff }]), /id space exhausted/)
+  })
+
   before(() => {
     for (const key of envKeys) {
       savedEnv[key] = process.env[key]
@@ -103,4 +111,21 @@ describe('file store', () => {
     process.env.NICTOOL_DATA_STORE = 'nonsense'
     assert.equal(resolveCodec().ext, 'json')
   })
+
+  for (const type of ['json', 'toml']) {
+    it(`serializes ${type} id allocation and does not reuse a destroyed id`, async () => {
+      process.env.NICTOOL_DATA_STORE = type
+      const store = new GroupRepoFile()
+      const ids = await Promise.all(
+        Array.from({ length: 20 }, (_, index) => store.create({ name: `group-${index}.example` })),
+      )
+
+      assert.deepEqual(
+        [...ids].sort((a, b) => a - b),
+        Array.from({ length: 20 }, (_, index) => index + 1),
+      )
+      await store.destroy({ id: 20 })
+      assert.equal(await store.create({ name: 'after-destroy.example' }), 21)
+    })
+  }
 })

lib/user/store/file.js

@@ -1,4 +1,4 @@
-import FileStore from '../../store/file.js'
+import FileStore, { nextId } from '../../store/file.js'
 import { idConflict } from '../../store/error.js'
 import Config from '../../config.js'
 import Credentials from '../credentials.js'
@@ -132,14 +132,6 @@ class UserRepoFile extends UserBase {
   }
 
   async create(args, options) {
-    if (args.id !== undefined) {
-      const existing = [
-        ...(await this.get({ id: args.id })),
-        ...(await this.get({ id: args.id, deleted: true })),
-      ]
-      if (existing.length > 0) return idConflict('user', args.id, options)
-    }
-
     args = JSON.parse(JSON.stringify(args))
 
     const inherit = args.inherit_group_permissions
@@ -149,19 +141,26 @@ class UserRepoFile extends UserBase {
       Object.assign(args, await Credentials.forStorage(args.password, args.pass_salt))
     }
 
-    if (inherit === false) {
-      args.permissions = {
-        ...JSON.parse(JSON.stringify(defaultPermissions)),
-        id: args.id,
-        user: { id: args.id, create: false, write: false, delete: false },
-        group: { id: args.gid, create: false, write: false, delete: false },
+    return this.file.mutate('user', (users, data) => {
+      if (args.id !== undefined && users.some((user) => user.id === args.id)) {
+        return idConflict('user', args.id, options)
       }
-    }
 
-    const users = await this._load()
-    users.push(args)
-    await this._save(users)
-    return args.id
+      if (args.id === undefined) args.id = nextId(users, data.last_id)
+      data.last_id = Math.max(data.last_id ?? 0, args.id)
+
+      if (inherit === false) {
+        args.permissions = {
+          ...JSON.parse(JSON.stringify(defaultPermissions)),
+          id: args.id,
+          user: { id: args.id, create: false, write: false, delete: false },
+          group: { id: args.gid, create: false, write: false, delete: false },
+        }
+      }
+
+      users.push(args)
+      return args.id
+    })
   }
 
   async put(args) {