Role names are validated through the prototype chain: role:"toString" silently demotes a Maintainer and answers changed:true

changelog.d/1838.fixed.md

@@ -0,0 +1 @@
+A Project role name is now matched against the rank table's own keys instead of the prototype chain: `toString`, `__proto__` and `constructor` answer `unknown-role` and leave the record untouched, where they used to demote a Maintainer to Member and report success ([#1838](https://github.com/ThinkFlowLab/FlyBot/issues/1838)).

core/src/host/project-board.js

@@ -72,7 +72,9 @@ export function changeRole(project, { actorId, targetId, role } = {}) {
   const target = self ? text(actorId) : text(targetId)
   const wanted = text(role)
   const current = rolesOf(project)
-  if (PROJECT_ROLE_RANKS[wanted] === undefined) return { ok: false, reason: 'unknown-role' }
+  // #1838: a plain object answers for names it inherits, so `toString` used to read as a rank and
+  // fall through to the demote path. Only an own key of the table is a role.
+  if (!Object.hasOwn(PROJECT_ROLE_RANKS, wanted)) return { ok: false, reason: 'unknown-role' }
   if (wanted === 'owner') return { ok: false, reason: 'owner-not-assignable' }
   if (actor === null) return { ok: false, reason: 'not-owner' }
   if (self) {

core/test/project-roles.test.js

@@ -45,3 +45,27 @@ test('the last owner cannot be demoted, by themselves or by anyone else (#1828)'
   assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'owner', role: 'owner' }), { ok: false, reason: 'owner-not-assignable' })
   assert.deepEqual(rolesOf(p), { owner: ['owner'], maintainers: [], members: [] })
 })
+
+// #1838: `PROJECT_ROLE_RANKS` is a plain object, so a name inherited from Object.prototype used to
+// read as a defined rank, skip every refusal, and land on the demote path. A role name is the rank
+// table's own key or it is not a role.
+test('#1838: role "toString" is refused and a Maintainer keeps that rank', () => {
+  const p = project({ roles: { maintainers: ['m'], members: ['n'] } })
+  const before = rolesOf(p)
+  assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'm', role: 'toString' }), { ok: false, reason: 'unknown-role' })
+  assert.deepEqual(rolesOf(p), before)
+})
+
+test('#1838: role "__proto__" is refused and a Maintainer keeps that rank', () => {
+  const p = project({ roles: { maintainers: ['m'], members: ['n'] } })
+  const before = rolesOf(p)
+  assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'm', role: '__proto__' }), { ok: false, reason: 'unknown-role' })
+  assert.deepEqual(rolesOf(p), before)
+})
+
+test('#1838: role "constructor" is refused and a Maintainer keeps that rank', () => {
+  const p = project({ roles: { maintainers: ['m'], members: ['n'] } })
+  const before = rolesOf(p)
+  assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'm', role: 'constructor' }), { ok: false, reason: 'unknown-role' })
+  assert.deepEqual(rolesOf(p), before)
+})

patches/ds-bot/files/src/host/project-board.js

@@ -72,7 +72,9 @@ export function changeRole(project, { actorId, targetId, role } = {}) {
   const target = self ? text(actorId) : text(targetId)
   const wanted = text(role)
   const current = rolesOf(project)
-  if (PROJECT_ROLE_RANKS[wanted] === undefined) return { ok: false, reason: 'unknown-role' }
+  // #1838: a plain object answers for names it inherits, so `toString` used to read as a rank and
+  // fall through to the demote path. Only an own key of the table is a role.
+  if (!Object.hasOwn(PROJECT_ROLE_RANKS, wanted)) return { ok: false, reason: 'unknown-role' }
   if (wanted === 'owner') return { ok: false, reason: 'owner-not-assignable' }
   if (actor === null) return { ok: false, reason: 'not-owner' }
   if (self) {

patches/ds-bot/files/test/project-roles.test.js

@@ -45,3 +45,27 @@ test('the last owner cannot be demoted, by themselves or by anyone else (#1828)'
   assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'owner', role: 'owner' }), { ok: false, reason: 'owner-not-assignable' })
   assert.deepEqual(rolesOf(p), { owner: ['owner'], maintainers: [], members: [] })
 })
+
+// #1838: `PROJECT_ROLE_RANKS` is a plain object, so a name inherited from Object.prototype used to
+// read as a defined rank, skip every refusal, and land on the demote path. A role name is the rank
+// table's own key or it is not a role.
+test('#1838: role "toString" is refused and a Maintainer keeps that rank', () => {
+  const p = project({ roles: { maintainers: ['m'], members: ['n'] } })
+  const before = rolesOf(p)
+  assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'm', role: 'toString' }), { ok: false, reason: 'unknown-role' })
+  assert.deepEqual(rolesOf(p), before)
+})
+
+test('#1838: role "__proto__" is refused and a Maintainer keeps that rank', () => {
+  const p = project({ roles: { maintainers: ['m'], members: ['n'] } })
+  const before = rolesOf(p)
+  assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'm', role: '__proto__' }), { ok: false, reason: 'unknown-role' })
+  assert.deepEqual(rolesOf(p), before)
+})
+
+test('#1838: role "constructor" is refused and a Maintainer keeps that rank', () => {
+  const p = project({ roles: { maintainers: ['m'], members: ['n'] } })
+  const before = rolesOf(p)
+  assert.deepEqual(changeRole(p, { actorId: 'owner', targetId: 'm', role: 'constructor' }), { ok: false, reason: 'unknown-role' })
+  assert.deepEqual(rolesOf(p), before)
+})