Skip to content

Commit a7c7455

Browse files
parth0025claude
andauthored
fix: address safe CodeRabbit findings from the #240 promotion review (#249)
* fix: address safe CodeRabbit findings from the promotion (#240) review Six low-risk, verified fixes: - Webhooks/dispatcher.js: guard against an undefined pending entry in the debounce callback (entry.doc could throw). - gitlabOAuth/controller.js: add a 15s timeout to the token-exchange axios call (was unbounded). - Admin/common/controller.js: drop an erroneous JSON.parse in the missing-file branch (makeDefaultBrandSettings resolves an object) - fixes a first-run 404 on getBrandSettingsData. - Stickies/helpers/stickyRules.js: strict boolean for isPinned (the string 'false' no longer coerces to true). - StickiesPanel.vue: clear the pending debounced save in remove() so it cannot fire a stale PUT after delete. - README.md: 'Priority support & SLAs' -> 'response-time targets' to match SUPPORT.md. Verified: node --check (backend), sticky-rules jest 16/16, StickiesPanel SFC compile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: scope updateMember role-promotion guard to PAT requests Companion to #248: the owner-only role-promotion check in updateMember now runs only for PAT/MCP requests (req.apiToken), so the web app's role management behaves exactly as before. Same 2026-06-15 MCP-isolation hardening as permissionGuard.js. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent cc4ce92 commit a7c7455

7 files changed

Lines changed: 18 additions & 8 deletions

File tree

β€ŽModules/Admin/common/controller.jsβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ exports.getBrandSettingsData = (req, res) => {
106106
if (!fs.existsSync(filePath)) {
107107
exports.makeDefaultBrandSettings()
108108
.then((data) => {
109-
res.status(200).json(withDemo(JSON.parse(data)));
109+
// makeDefaultBrandSettings resolves an OBJECT (not a JSON string),
110+
// so it must not be JSON.parse'd β€” that threw on first run.
111+
res.status(200).json(withDemo(data));
110112
})
111113
.catch((error) => {
112114
res.status(404).send(error);

β€ŽModules/Stickies/helpers/stickyRules.jsβ€Ž

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ const validateStickyPayload = (payload = {}, { partial = false } = {}) => {
4343
}
4444

4545
if (payload.isPinned !== undefined) {
46-
out.isPinned = Boolean(payload.isPinned);
46+
// Strict boolean β€” the string "false" must not coerce to true.
47+
out.isPinned = payload.isPinned === true || payload.isPinned === 'true' || payload.isPinned === 1 || payload.isPinned === '1';
4748
}
4849

4950
if (payload.sortIndex !== undefined) {

β€ŽModules/Webhooks/dispatcher.jsβ€Ž

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ function onTaskEvent(type) {
144144
timer: setTimeout(() => {
145145
const entry = pending.get(key);
146146
pending.delete(key);
147+
if (!entry) return; // key already replaced/cleaned up β€” nothing to flush
147148
flush(companyId, event, entry.doc).catch((error) => {
148149
logger.error(`${LOG_PREFIX} flush failed: ${error.message}`);
149150
});

β€ŽModules/gitlabOAuth/controller.jsβ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ exports.getAccessToken = async (req, res) => {
2828
code,
2929
grant_type: "authorization_code",
3030
redirect_uri: redirectUri,
31-
}, { headers: { Accept: "application/json" } });
31+
}, { headers: { Accept: "application/json" }, timeout: 15000 });
3232

3333
const accessToken = tokenResponse.data.access_token;
3434

β€ŽModules/settings/Members/controller.jsβ€Ž

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,11 @@ exports.updateMember = async (req, res) => {
219219
});
220220
}
221221

222-
// Defense in depth (route is already owner/admin-gated): granting
223-
// owner/admin (roleType 1/2) is OWNER-only β€” prevents an admin from
224-
// promoting anyone (incl. self) to owner. Members never reach here.
225-
if (data && data.roleType !== undefined) {
222+
// Defense in depth: granting owner/admin (roleType 1/2) is OWNER-only.
223+
// SCOPED TO PAT/MCP REQUESTS ONLY (req.apiToken) so the web app's role
224+
// management behaves exactly as before β€” MCP changes must never affect
225+
// existing frontend/backend behavior (2026-06-15).
226+
if (req.apiToken && data && data.roleType !== undefined) {
226227
const { getRoleType, ROLE_OWNER, ROLE_ADMIN } = require('../../../Config/permissionGuard');
227228
const callerRole = await getRoleType(req.headers['companyid'] || '', req.uid);
228229
const targetRole = Number(data.roleType);

β€ŽREADME.mdβ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ AlianHub is built and maintained by [Aliansoftware](https://aliansoftware.net).
413413

414414
- πŸš€ **Managed hosting & deployment** β€” on your infrastructure or ours
415415
- πŸ› οΈ **Custom features & integrations** β€” tailored to your workflow
416-
- 🀝 **Priority support & SLAs** β€” [support@aliansoftware.net](mailto:support@aliansoftware.net)
416+
- 🀝 **Priority support & response-time targets** β€” [support@aliansoftware.net](mailto:support@aliansoftware.net)
417417

418418
---
419419

β€Žfrontend/src/components/molecules/Stickies/StickiesPanel.vueβ€Ž

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,11 @@ function onReorder() {
259259
260260
function remove(note) {
261261
openMenuId.value = null;
262+
// Cancel any pending debounced save so it can't fire a stale PUT after delete.
263+
if (saveTimers[note._id]) {
264+
clearTimeout(saveTimers[note._id]);
265+
delete saveTimers[note._id];
266+
}
262267
apiRequest('delete', `/api/v2/stickies/${note._id}?uid=${encodeURIComponent(userId.value)}`)
263268
.then((response) => {
264269
if (response.data?.status) {

0 commit comments

Comments
Β (0)