From 2bbed127ae8cf0f20762ae0b7abb0c4cd6ecad90 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sun, 13 Jul 2025 13:58:19 -0700 Subject: [PATCH 1/2] Pacthes security issue with Slack routes and component APIs --- src/routers/config.js | 6 ++++-- src/routers/slack.js | 25 ++++++++++++++++++------- tests/config.test.js | 2 +- tests/fixtures/db_api.js | 2 +- tests/slack.test.js | 38 +++++++++++++++++++------------------- 5 files changed, 43 insertions(+), 30 deletions(-) diff --git a/src/routers/config.js b/src/routers/config.js index e0634988..1e568724 100644 --- a/src/routers/config.js +++ b/src/routers/config.js @@ -1,5 +1,5 @@ import express from 'express'; -import { check, query } from 'express-validator'; +import { body, check, query } from 'express-validator'; import { relayOptions } from '../models/config.js'; import { auth } from '../middleware/auth.js'; import { ActionTypes, RouterTypes } from '../models/permission.js'; @@ -201,7 +201,9 @@ router.patch('/config/removeComponent/:id', auth, [ }); router.patch('/config/updateComponents/:id', auth, [ - check('id').isMongoId() + check('id').isMongoId(), + body('components').isArray(), + body('components.*').isMongoId() ], validate, async (req, res) => { try { const config = await Services.updateComponent(req.params.id, req.body, req.admin); diff --git a/src/routers/slack.js b/src/routers/slack.js index 5f5488f9..293ea265 100644 --- a/src/routers/slack.js +++ b/src/routers/slack.js @@ -62,7 +62,8 @@ router.post('/slack/v1/installation', slackAuth, [ router.post('/slack/v1/authorize', auth, [ check('domain').isMongoId(), - check('team_id').exists() + check('team_id').exists(), + check('team_id').isAlphanumeric() ], validate, auth, async (req, res) => { try { await Services.authorizeSlackInstallation(req.body, req.admin); @@ -75,6 +76,7 @@ router.post('/slack/v1/authorize', auth, [ router.post('/slack/v1/ticket/clear', auth, [ check('team_id').exists(), + check('team_id').isAlphanumeric(), check('domain_id').isMongoId() ], validate, async (req, res) => { try { @@ -88,6 +90,7 @@ router.post('/slack/v1/ticket/clear', auth, [ router.post('/slack/v1/ticket/validate', slackAuth, [ check('team_id').exists(), + check('team_id').isAlphanumeric(), check('domain_id').isMongoId(), check('ticket_content.environment').exists(), check('ticket_content.group').exists(), @@ -107,6 +110,7 @@ router.post('/slack/v1/ticket/validate', slackAuth, [ router.post('/slack/v1/ticket/create', slackAuth, [ check('team_id').exists(), + check('team_id').isAlphanumeric(), check('domain_id').isMongoId(), check('ticket_content.environment').exists(), check('ticket_content.group').exists(), @@ -127,6 +131,7 @@ router.post('/slack/v1/ticket/create', slackAuth, [ router.post('/slack/v1/ticket/process', slackAuth, [ check('team_id').exists(), + check('team_id').isAlphanumeric(), check('domain_id').isMongoId(), check('ticket_id').isMongoId(), check('approved').isBoolean() @@ -140,7 +145,8 @@ router.post('/slack/v1/ticket/process', slackAuth, [ }); router.get('/slack/v1/findbot', slackAuth, [ - query('team_id').exists() + query('team_id').exists(), + query('team_id').isAlphanumeric() ], validate, async (req, res) => { try { const slack = await Services.getSlack({ @@ -156,13 +162,15 @@ router.get('/slack/v1/findbot', slackAuth, [ }); router.get('/slack/v1/findinstallation', slackAuth, [ - query('team_id').exists() + query('team_id').exists(), + query('team_id').isAlphanumeric() ], validate, async (req, res) => { await findInstallation(req, res); }); router.get('/slack/v1/installation/find', auth, [ - query('team_id').exists() + query('team_id').exists(), + query('team_id').isAlphanumeric() ], validate, async (req, res) => { await findInstallation(req, res, true); }); @@ -191,7 +199,8 @@ router.get('/slack/v1/installation/:domain', auth, [ }); router.get('/slack/v1/domains', slackAuth, [ - query('team_id').exists() + query('team_id').exists(), + query('team_id').isAlphanumeric() ], validate, async (req, res) => { try { const domains = await Services.getDomainsByTeamId(req.query.team_id); @@ -202,13 +211,15 @@ router.get('/slack/v1/domains', slackAuth, [ }); router.delete('/slack/v1/installation', slackAuth, [ - query('team_id').exists() + query('team_id').exists(), + query('team_id').isAlphanumeric() ], validate, async (req, res) => { await deleteInstallation(req, res); }); router.delete('/slack/v1/installation/decline', auth, [ - query('team_id').exists() + query('team_id').exists(), + query('team_id').isAlphanumeric() ], validate, async (req, res) => { await deleteInstallation(req, res); }); diff --git a/tests/config.test.js b/tests/config.test.js index e962e23d..ce1196b5 100644 --- a/tests/config.test.js +++ b/tests/config.test.js @@ -679,7 +679,7 @@ describe('Testing component association', () => { .set('Authorization', `Bearer ${adminMasterAccountToken}`) .send({ components: [ - responseComponent1.body._id, + responseComponent1.body.component._id, new mongoose.Types.ObjectId() ] }).expect(404); diff --git a/tests/fixtures/db_api.js b/tests/fixtures/db_api.js index 3227ed68..5b0bd5a8 100644 --- a/tests/fixtures/db_api.js +++ b/tests/fixtures/db_api.js @@ -226,7 +226,7 @@ export const teamInviteNoTeam = { export const slack = { _id: new mongoose.Types.ObjectId(), - team_id: 'TEAM_ID', + team_id: 'T00NZ', user_id: 'USER_ID', domain: domainId, settings: { diff --git a/tests/slack.test.js b/tests/slack.test.js index 097928cd..7e376173 100644 --- a/tests/slack.test.js +++ b/tests/slack.test.js @@ -189,7 +189,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should authorize installation', async () => { //given - const installation = await buildInstallation('SHOULD_AUTHORIZE_DOMAIN', null); + const installation = await buildInstallation('T11NZ', null); //test const response = await request(app) @@ -257,7 +257,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should NOT authorize installation - Admin is not owner', async () => { //given - const installation = await buildInstallation('SHOULD_NOT_AUTHORIZE_DOMAIN', null); + const installation = await buildInstallation('T01Y', null); //test await request(app) @@ -285,7 +285,7 @@ describe('Slack Installation', () => { .set('Authorization', `Bearer ${adminMasterAccountToken}`) .send({ domain: new mongoose.Types.ObjectId(), - team_id: 'team_id' + team_id: 'T04Y' }).expect(404); }); @@ -301,7 +301,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should find bot', async () => { //given const installation = { ...mock1_slack_installation }; - installation.team_id = 'T_FIND_BOT'; + installation.team_id = 'T14NZ'; installation.bot_payload.app_id = 'TEST_FIND_BOT1'; await Services.createSlackInstallation(installation); @@ -316,7 +316,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should NOT find bot - Not found', async () => { await request(app) - .get('/slack/v1/findbot?enterprise_id=&team_id=NOT_FOUND') + .get('/slack/v1/findbot?enterprise_id=&team_id=T15NZ') .set('Authorization', `Bearer ${generateToken('30s')}`) .send().expect(404); }); @@ -331,7 +331,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should find installation', async () => { //given const installation = { ...mock1_slack_installation }; - installation.team_id = 'T_FIND_INSTALL'; + installation.team_id = 'T13NZ'; installation.installation_payload.app_id = 'TEST_FIND_INSTALLATION1'; await Services.createSlackInstallation(installation); @@ -346,7 +346,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should NOT find installation - Not found', async () => { await request(app) - .get('/slack/v1/findinstallation?enterprise_id=&team_id=NOT_FOUND') + .get('/slack/v1/findinstallation?enterprise_id=&team_id=T12NZ') .set('Authorization', `Bearer ${generateToken('30s')}`) .send().expect(404); }); @@ -361,7 +361,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should find installation (Admin)', async () => { //given const installation = { ...mock1_slack_installation }; - installation.team_id = 'T_FIND_INSTALL_ADMIN'; + installation.team_id = 'T10NZ'; installation.installation_payload.app_id = 'TEST_FIND_INSTALLATION2'; await Services.createSlackInstallation(installation); @@ -377,7 +377,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should delete not authorized installation', async () => { //given const installation = { ...mock1_slack_installation }; - installation.team_id = 'T_DELETE_INSTALL'; + installation.team_id = 'T04NZ'; installation.installation_payload.app_id = 'TEST_DELETE_INSTALLATION1'; await Services.createSlackInstallation(installation); @@ -397,7 +397,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should delete NOT authorized installation', async () => { //given - const installation = await buildInstallation('SHOULD_DELETE_AUTHORIZE_INSTALLATION', null); + const installation = await buildInstallation('T05NZ', null); //test await request(app) @@ -412,7 +412,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should NOT delete installation - Not found', async () => { await request(app) - .delete('/slack/v1/installation?enterprise_id=&team_id=NOT_FOUND') + .delete('/slack/v1/installation?enterprise_id=&team_id=T06NZ') .set('Authorization', `Bearer ${generateToken('30s')}`) .send().expect(404); }); @@ -426,7 +426,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should unlink installation', async () => { //given - const installation = await buildInstallation('SHOULD_UNLINK_INTEGRATION', null); + const installation = await buildInstallation('T02Y', null); await authorizeInstallation(installation, domainId, adminMasterAccountToken); //verify that @@ -449,12 +449,12 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should unlink single installation from Multi Slack Installation', async () => { //given - installation/authorization for Domain 1 - const installation = await buildInstallation('MULTI_DOMAIN_TEAM_ID', null); + const installation = await buildInstallation('T03Y', null); await authorizeInstallation(installation, domainId, adminMasterAccountToken); //given - installation/authorization for Domain 2 const domainId2 = await createDomain('Domain 2'); - const installation2 = await buildInstallation('MULTI_DOMAIN_TEAM_ID', null); + const installation2 = await buildInstallation('T03Y', null); await request(app) .post('/slack/v1/authorize') .set('Authorization', `Bearer ${adminMasterAccountToken}`) @@ -495,7 +495,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should decline installation', async () => { //given - const installation = await buildInstallation('SHOULD_DECLINE_INTEGRATION', null); + const installation = await buildInstallation('T09NZ', null); //test await request(app) @@ -511,7 +511,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should find Domains by Slack Team Id', async () => { //given - const teamId = 'SLACK_INSTALLATION_DOMAINS'; + const teamId = 'T01NZ'; const domainId2 = await createDomain('Domain 2 (findByTeamId)'); await buildInstallation(teamId, domainId2); @@ -532,7 +532,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should find only authorized Domains by Slack Team Id', async () => { //given - const teamId = 'SLACK_INSTALLATION_DOMAINS_NOT_AUTHORIZED'; + const teamId = 'T02NZ'; const domainId = await createDomain('Domain 4 (findByTeamId)'); await buildInstallation(teamId, domainId); // authorized await buildInstallation(teamId); // not authorized @@ -557,7 +557,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should NOT find Domains by Slack Team Id - Team Id not found', async () => { await request(app) - .get('/slack/v1/domains?team_id=NOT_FOUND') + .get('/slack/v1/domains?team_id=T03NZ') .set('Authorization', `Bearer ${generateToken('30s')}`) .send().expect(404); }); @@ -1046,7 +1046,7 @@ describe('Slack Route - Process Ticket', () => { .post('/slack/v1/ticket/process') .set('Authorization', `Bearer ${generateToken('30s')}`) .send({ - team_id: 'NOT_FOUND', + team_id: 'N0000NZ', domain_id: domainId, ticket_id: new mongoose.Types.ObjectId(), approved: true From b7af6ef6091dc6103d2704322d80a22fd3040cfa Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sun, 13 Jul 2025 14:05:10 -0700 Subject: [PATCH 2/2] ci: fixed missing step from sonar workflow --- .github/workflows/sonar.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 55147a5f..5ca0ea6a 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -43,6 +43,9 @@ jobs: with: mongodb-version: 8.0 + - name: Install dependencies + run: npm ci + - name: Lint run: npm run lint