From 2123e2cc4e5be73d9aabf14eb2563d76591ad364 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:00:32 +0800 Subject: [PATCH] fix(host): a role name is the rank table's own key (#1838) `changeRole` validated the requested role with `PROJECT_ROLE_RANKS[wanted] === undefined`, and a plain object literal answers for the names it inherits: `toString`, `__proto__` and `constructor` passed the check, skipped every refusal and fell into the demote path, so an Owner re-roling a Maintainer with `role: "toString"` got `{ ok: true }` and a silent demotion. The rank table's own keys are now the only roles (`Object.hasOwn`). Docs-not-needed: docs/subsystems/core-host.md already states that `changeRole` answers `unknown-role` and leaves the record untouched on a refusal; this change makes the code match that sentence, so no documented boundary moves. --- changelog.d/1838.fixed.md | 1 + core/src/host/project-board.js | 4 +++- core/test/project-roles.test.js | 24 +++++++++++++++++++ .../ds-bot/files/src/host/project-board.js | 4 +++- .../ds-bot/files/test/project-roles.test.js | 24 +++++++++++++++++++ 5 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 changelog.d/1838.fixed.md diff --git a/changelog.d/1838.fixed.md b/changelog.d/1838.fixed.md new file mode 100644 index 000000000..44d6f5d6e --- /dev/null +++ b/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)). diff --git a/core/src/host/project-board.js b/core/src/host/project-board.js index fba1967d3..c562fa5da 100644 --- a/core/src/host/project-board.js +++ b/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) { diff --git a/core/test/project-roles.test.js b/core/test/project-roles.test.js index 470d682ec..44eb2b1bf 100644 --- a/core/test/project-roles.test.js +++ b/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) +}) diff --git a/patches/ds-bot/files/src/host/project-board.js b/patches/ds-bot/files/src/host/project-board.js index fba1967d3..c562fa5da 100644 --- a/patches/ds-bot/files/src/host/project-board.js +++ b/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) { diff --git a/patches/ds-bot/files/test/project-roles.test.js b/patches/ds-bot/files/test/project-roles.test.js index 470d682ec..44eb2b1bf 100644 --- a/patches/ds-bot/files/test/project-roles.test.js +++ b/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) +})