store: reject create conflicts

CHANGELOG.md

@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Unreleased
 
+- fix: reject create conflicts and duplicate permission targets
+
 ### [3.0.3] - 2026-07-27
 
 - many updates for data stores and NS backends

lib/group/store/base.js

@@ -6,7 +6,7 @@
  *
  * Repo contract:
  *   get(args)              → object[]
- *   create(args)           → number  (groupId)
+ *   create(args, options)  → number  (groupId)
  *   put(args)              → boolean
  *   delete(args)           → boolean
  *   destroy(args)          → boolean
@@ -29,7 +29,7 @@ class GroupBase {
     throw new Error('get() not implemented by this repo')
   }
 
-  async create(_args) {
+  async create(_args, _options) {
     throw new Error('create() not implemented by this repo')
   }
 

lib/group/store/file.js

@@ -1,4 +1,5 @@
 import FileStore from '../../store/file.js'
+import { idConflict } from '../../store/error.js'
 
 import GroupBase from './base.js'
 
@@ -57,12 +58,15 @@ class GroupRepoFile extends GroupBase {
     return [rootGid, ...this._collectSubgroupIds(groups, rootGid)]
   }
 
-  async create(args) {
+  async create(args, options) {
     args = JSON.parse(JSON.stringify(args))
 
-    if (args.id) {
-      const existing = await this.get({ id: args.id })
-      if (existing.length === 1) return existing[0].id
+    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 ?? []

lib/group/store/mysql.js

@@ -1,6 +1,7 @@
 import Mysql from '../../mysql.js'
 import GroupBase from './base.js'
 import Permission from '../../permission/index.js'
+import { idConflict } from '../../store/error.js'
 import { mapToDbColumn } from '../../util.js'
 
 const groupDbMap = { id: 'nt_group_id', parent_gid: 'parent_group_id' }
@@ -12,10 +13,10 @@ class Group extends GroupBase {
     this.mysql = Mysql
   }
 
-  async create(args) {
-    if (args.id) {
-      const g = await this.get({ id: args.id })
-      if (g.length === 1) return g[0].id
+  async create(args, options) {
+    if (args.id !== undefined) {
+      const g = [...(await this.get({ id: args.id })), ...(await this.get({ id: args.id, deleted: true }))]
+      if (g.length > 0) return idConflict('group', args.id, options)
     }
 
     const usable_ns = args.usable_ns

lib/group/test/index.js

@@ -17,7 +17,7 @@ after(async () => {
 
 describe('group', function () {
   before(async () => {
-    await Group.create(testCase)
+    await Group.create(testCase, { ifExists: 'return' })
   })
 
   it('gets group by id', async () => {

lib/nameserver/store/base.js

@@ -6,7 +6,7 @@
  *
  * Repo contract:
  *   get(args)              → object[]
- *   create(args)           → number  (zoneId)
+ *   create(args, options)  → number  (zoneId)
  *   put(args)              → boolean
  *   delete(args)           → boolean
  *   destroy(args)          → boolean
@@ -24,7 +24,7 @@ class NameserverBase {
     throw new Error('get() not implemented by this repo')
   }
 
-  async create(_args) {
+  async create(_args, _options) {
     throw new Error('create() not implemented by this repo')
   }
 

lib/nameserver/store/file.js

@@ -1,4 +1,5 @@
 import FileStore from '../../store/file.js'
+import { idConflict } from '../../store/error.js'
 
 import NameserverBase from './base.js'
 
@@ -38,10 +39,13 @@ class NameserverRepoFile extends NameserverBase {
     return r
   }
 
-  async create(args) {
-    if (args.id) {
-      const existing = await this.get({ id: args.id })
-      if (existing.length === 1) return existing[0].id
+  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)
     }
 
     const nameservers = await this._load()

lib/nameserver/store/mysql.js

@@ -1,5 +1,6 @@
 import Mysql from '../../mysql.js'
 import NameserverBase from './base.js'
+import { idConflict } from '../../store/error.js'
 import { mapToDbColumn } from '../../util.js'
 
 const nsDbMap = { id: 'nt_nameserver_id', gid: 'nt_group_id' }
@@ -15,10 +16,10 @@ class Nameserver extends NameserverBase {
     this.mysql = Mysql
   }
 
-  async create(args) {
-    if (args.id) {
-      const g = await this.get({ id: args.id })
-      if (g.length === 1) return g[0].id
+  async create(args, options) {
+    if (args.id !== undefined) {
+      const g = [...(await this.get({ id: args.id })), ...(await this.get({ id: args.id, deleted: true }))]
+      if (g.length > 0) return idConflict('nameserver', args.id, options)
     }
 
     args = await resolveType(args)

lib/permission/store/base.js

@@ -5,7 +5,7 @@
  * repository classes must extend this class and implement the repo contract.
  *
  * Repo contract:
- *   create(args)       → id
+ *   create(args, options) → id
  *   get(args)          → object | undefined
  *   getGroup(args)     → object | undefined
  *   put(args)          → boolean
@@ -17,7 +17,7 @@ class PermissionBase {
     this.debug = args?.debug ?? false
   }
 
-  async create(_args) {
+  async create(_args, _options) {
     throw new Error('create() not implemented by this store')
   }
 

lib/permission/store/file.js

@@ -1,4 +1,5 @@
 import FileStore from '../../store/file.js'
+import { idConflict } from '../../store/error.js'
 
 import PermissionBase from './base.js'
 
@@ -73,28 +74,52 @@ class PermissionRepoFile extends PermissionBase {
   // CRUD
   // ---------------------------------------------------------------------------
 
-  async create(args) {
+  async create(args, options) {
     args = JSON.parse(JSON.stringify(args))
     const uid = args.uid ?? args.user?.id
     const gid = args.gid ?? args.group?.id
     delete args.uid
     delete args.gid
 
+    if (args.id !== undefined) {
+      const [users, groups, permissions] = await Promise.all([
+        this._loadUsers(),
+        this._loadGroups(),
+        this._loadStandalone(),
+      ])
+      const existing = [
+        ...users.map((user) => ({ permission: user.permissions, uid: user.id, gid: user.gid })),
+        ...groups.map((group) => ({ permission: group.permissions, gid: group.id })),
+        ...permissions.map((permission) => ({
+          permission,
+          uid: permission.uid ?? permission.user?.id,
+          gid: permission.gid ?? permission.group?.id,
+        })),
+      ].find((entry) => entry.permission?.id === args.id)
+      if (existing) {
+        const sameTarget =
+          uid !== undefined
+            ? existing.uid === uid
+            : gid !== undefined
+              ? existing.uid === undefined && existing.gid === gid
+              : true
+        return idConflict('permission', args.id, sameTarget ? options : undefined)
+      }
+    }
+
     if (uid !== undefined) {
       const users = await this._loadUsers()
       const idx = users.findIndex((u) => u.id === uid)
 
       if (idx !== -1) {
-        // Store inline in user.toml using the actual permission data from args
-        if (!users[idx].permissions) {
-          const perm = JSON.parse(JSON.stringify(args))
-          perm.id = uid
-          if (!perm.user) perm.user = {}
-          perm.user.id = uid
-          if (!perm.group) perm.group = {}
-          perm.group.id = gid ?? users[idx].gid
-          users[idx].permissions = perm
-        }
+        if (users[idx].permissions) return idConflict('permission', users[idx].permissions.id, options)
+        const perm = JSON.parse(JSON.stringify(args))
+        perm.id = args.id ?? uid
+        if (!perm.user) perm.user = {}
+        perm.user.id = uid
+        if (!perm.group) perm.group = {}
+        perm.group.id = gid ?? users[idx].gid
+        users[idx].permissions = perm
         await this._saveUsers(users)
         return users[idx].permissions.id
       }
@@ -107,14 +132,12 @@ class PermissionRepoFile extends PermissionBase {
       const idx = groups.findIndex((g) => g.id === gid)
 
       if (idx !== -1) {
-        // Store inline in group.toml
-        if (!groups[idx].permissions) {
-          const perm = JSON.parse(JSON.stringify(args))
-          perm.id = gid
-          if (!perm.group) perm.group = {}
-          perm.group.id = gid
-          groups[idx].permissions = perm
-        }
+        if (groups[idx].permissions) return idConflict('permission', groups[idx].permissions.id, options)
+        const perm = JSON.parse(JSON.stringify(args))
+        perm.id = args.id ?? gid
+        if (!perm.group) perm.group = {}
+        perm.group.id = gid
+        groups[idx].permissions = perm
         await this._saveGroups(groups)
         return groups[idx].permissions.id
       }
@@ -127,13 +150,14 @@ class PermissionRepoFile extends PermissionBase {
     if (permId === undefined) return undefined
 
     const perms = await this._loadStandalone()
-    if (!perms.find((p) => p.id === permId)) {
-      const perm = { ...args, id: permId }
-      if (uid !== undefined) perm.uid = uid
-      if (gid !== undefined) perm.gid = gid
-      perms.push(perm)
-      await this._saveStandalone(perms)
+    if (perms.some((permission) => permission.id === permId)) {
+      return idConflict('permission', permId, options)
     }
+    const perm = { ...args, id: permId }
+    if (uid !== undefined) perm.uid = uid
+    if (gid !== undefined) perm.gid = gid
+    perms.push(perm)
+    await this._saveStandalone(perms)
     return permId
   }
 

lib/permission/store/mysql.js

@@ -1,4 +1,5 @@
 import Mysql from '../../mysql.js'
+import { idConflict } from '../../store/error.js'
 import { mapToDbColumn } from '../../util.js'
 import PermissionBase from './base.js'
 
@@ -16,22 +17,38 @@ class PermissionRepoMySQL extends PermissionBase {
     this.mysql = Mysql
   }
 
-  async create(args) {
-    if (args.id) {
-      const p = await this.get({ id: args.id })
-      if (p) return p.id
+  async create(args, options) {
+    args = objectToDb(args)
+
+    if (args.id !== undefined) {
+      const p = (await this.get({ id: args.id })) ?? (await this.get({ id: args.id, deleted: true }))
+      if (p) {
+        // The fixture opt-out covers re-creating the same permission, not another
+        // target that happens to hold this id.
+        const sameTarget =
+          args.uid !== undefined
+            ? p.user?.id === args.uid
+            : args.gid !== undefined
+              ? p.user?.id === undefined && p.group?.id === args.gid
+              : true
+        return idConflict('permission', args.id, sameTarget ? options : undefined)
+      }
     }
 
-    // Deduplicate group-level permission rows (uid IS NULL) to prevent accumulation
-    if (args.gid !== undefined && args.uid === undefined) {
-      const rows = await Mysql.execute(
-        `SELECT nt_perm_id FROM nt_perm WHERE nt_group_id = ? AND nt_user_id IS NULL LIMIT 1`,
+    // nt_perm carries at most one row per user and one per group; a second makes
+    // every later read of that permission ambiguous.
+    let rows = []
+    if (args.uid !== undefined) {
+      rows = await Mysql.execute('SELECT nt_perm_id FROM nt_perm WHERE nt_user_id = ? LIMIT 1', [args.uid])
+    } else if (args.gid !== undefined) {
+      rows = await Mysql.execute(
+        'SELECT nt_perm_id FROM nt_perm WHERE nt_group_id = ? AND nt_user_id IS NULL LIMIT 1',
         [args.gid],
       )
-      if (rows.length > 0) return rows[0].nt_perm_id
     }
+    if (rows.length > 0) return idConflict('permission', rows[0].nt_perm_id, options)
 
-    return await Mysql.execute(...Mysql.insert(`nt_perm`, mapToDbColumn(objectToDb(args), permDbMap)))
+    return await Mysql.execute(...Mysql.insert(`nt_perm`, mapToDbColumn(args, permDbMap)))
   }
 
   async get(args) {

lib/permission/test/index.js

@@ -10,8 +10,8 @@ import userTestCase from '../../user/test/user.json' with { type: 'json' }
 import permTestCase from './permission.json' with { type: 'json' }
 
 before(async () => {
-  await Group.create(groupTestCase)
-  await User.create(userTestCase)
+  await Group.create(groupTestCase, { ifExists: 'return' })
+  await User.create(userTestCase, { ifExists: 'return' })
 })
 
 after(async () => {
@@ -20,7 +20,7 @@ after(async () => {
 
 describe('permission', function () {
   it('creates a permission', async () => {
-    assert.ok(await Permission.create(permTestCase))
+    assert.ok(await Permission.create(permTestCase, { ifExists: 'return' }))
   })
 
   it('get: by id', async () => {

lib/permission/test/permission.json

@@ -1,5 +1,5 @@
 {
-  "id": 4096,
+  "id": 63096,
   "inherit": true,
   "name": "Test Permission",
   "self_write": false,

lib/session/test/index.js

@@ -13,7 +13,7 @@ const sessionUser = {
 }
 
 before(async () => {
-  await User.create(sessionUser)
+  await User.create(sessionUser, { ifExists: 'return' })
 })
 
 after(async () => {