-
-
Notifications
You must be signed in to change notification settings - Fork 7
Security fixes: upgrade Hubot + Slack adapter #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rhamenator
wants to merge
2
commits into
main
Choose a base branch
from
security-fixes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| # Slackbot environment configuration | ||
| # Replace the values below with real tokens from your Slack app configuration. | ||
| HUBOT_SLACK_APP_TOKEN=xapp-1-your-app-token | ||
| HUBOT_SLACK_BOT_TOKEN=xoxb-your-bot-token | ||
| # Optional HTTP listener port | ||
| PORT=8080 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,3 +6,4 @@ deploy/slackbotrc | |
| deploy/slackinrc | ||
| yarn.lock | ||
| TAGS | ||
| .history/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| web: bin/hubot --adapter slack | ||
| web: bin/hubot |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # Security Upgrade Plan | ||
|
|
||
| This document outlines the remediation steps needed to resolve the remaining npm vulnerabilities after PRs #85, #95, and #96. | ||
|
|
||
| ## 1. Replace Deprecated Slack Adapter | ||
|
|
||
| - Swap `hubot-slack` for `@hubot-friends/hubot-slack`. | ||
| - Upgrade Hubot core to `^13.1.4` and adjust startup commands (Procfile, README) to use `hubot -a @hubot-friends/hubot-slack`. | ||
| - Require new env vars `HUBOT_SLACK_APP_TOKEN` and `HUBOT_SLACK_BOT_TOKEN`; document Socket Mode scopes/events. | ||
| - Refactor Slack-specific initializers/tests to remove dependencies on the legacy RTM client (notably `mentioned-rooms-referencer` and `watch-for-disconnected`). | ||
|
|
||
| ## 2. Remove Vulnerable Legacy Helpers | ||
|
|
||
| - Drop unused `google-url` and `expand-url` dependencies to eliminate their transitively vulnerable `request`/`form-data` chain. | ||
| - Update `lib/templates/welcome-email.js` and its specs to stub `shorten-url` instead of making live shortening calls. | ||
|
|
||
| ## 3. Upgrade Firebase SDK | ||
|
|
||
| - Bump `firebase` to `^12.4.0` to pick up patched `@grpc/grpc-js`. | ||
| - Verify Firestore usage (`events-fetcher`) against new APIs and adjust if necessary. | ||
|
|
||
| ## 4. Modernize Tooling | ||
|
|
||
| - Update `standard` to `^17` (and any transitive lint deps). | ||
| - Add Node engine constraint `>=18`. | ||
| - Update CircleCI workflow to run `npm run lint && npm test`. | ||
|
|
||
| ## 5. Verification | ||
|
|
||
| - Blow away lockfile and reinstall: `rm -rf node_modules package-lock.json && npm install`. | ||
| - Run `npm run lint`, `npm test`, and `npm audit --omit=dev`. | ||
| - ✅ `npm run lint` | ||
| - ✅ `npm test` | ||
| - ✅ `npm audit --omit=dev` | ||
| - Perform manual smoke tests in Slack (help command, channel mention notification, reconnect behavior, tweeting). | ||
| - Partial: Hubot boots locally with the new adapter; startup now halts at Slack authentication because placeholder `SLACK_APP_TOKEN` / `SLACK_BOT_TOKEN` were used. Re-run with production Socket Mode tokens to complete verification. | ||
|
|
||
| ## 6. Follow-Up | ||
|
|
||
| - Capture migration instructions in README. | ||
| - Determine if any remaining vulnerabilities stem from third-party Hubot scripts and file follow-up issues if they cannot be patched immediately. | ||
|
|
||
| ## 7. Slack Token Preparation & Sandbox Testing | ||
|
|
||
| 1. Create (or reuse) a Slack workspace dedicated to testing at <https://slack.com/create>. The free plan is sufficient and avoids touching production data while verifying Socket Mode behavior. | ||
| 2. Build a Slack app for that workspace via <https://api.slack.com/apps>: | ||
| - Enable **Socket Mode**. | ||
| - Generate an **App-Level Token** with the `connections:write` scope (this becomes `SLACK_APP_TOKEN`). | ||
| - Under **OAuth & Permissions**, add the required bot scopes (`app_mentions:read`, `channels:history`, `chat:write`, `groups:history`, `im:history`, `mpim:history`, `reactions:read`, `reactions:write`, etc.) and reinstall the app to capture the refreshed Bot User OAuth token (`SLACK_BOT_TOKEN`). | ||
| 3. Store those tokens in a local `.env` (never commit them): | ||
|
|
||
| ```bash | ||
| SLACK_APP_TOKEN=xapp-... | ||
| SLACK_BOT_TOKEN=xoxb-... | ||
| ``` | ||
|
|
||
| 4. Run a local smoke test with the sandbox tokens: `bash bin/hubot --adapter @hubot-friends/hubot-slack`. Confirm the adapter connects (look for "Connected to Slack" in the logs) and exercise key commands (help, channel mention notifications, reconnect behavior, tweeting). | ||
| 5. After approvals, repeat the token creation steps in the production workspace, update deployment secrets (e.g., Heroku config vars), and re-run the smoke test against production. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,5 @@ | ||
| [ | ||
| "hubot-heroku-keepalive", | ||
| "hubot-diagnostics", | ||
| "hubot-help", | ||
| "hubot-maps", | ||
| "hubot-rules", | ||
| "hubot-shipit", | ||
| "hubot-darts", | ||
| "hubot-thank-you", | ||
| "hubot-appearin", | ||
| "hubot-bikeshed", | ||
| "hubot-principles" | ||
| "hubot-rules" | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| module.exports = (organizer) => ({ | ||
| davin: 'Davin', | ||
| atomaka: 'Andrew', | ||
| leo: 'Leo' | ||
| leo: 'Leo', | ||
| 'erik.gillespie': 'Erik' | ||
| })[organizer] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,18 @@ | ||
| module.exports = (group) => ({ | ||
| const aliases = { | ||
| 'Lansing DevOps Meetup': 'Devs & Ops', | ||
| devops: 'Devs & Ops', | ||
| 'Lansing Ruby Meetup Group': 'Rubyists', | ||
| ruby: 'Rubyists', | ||
| 'Lansing Javascript Meetup': 'JSers', | ||
| 'Mobile Monday Lansing': 'Mobile Members' | ||
| })[group.name] | ||
| javascript: 'JSers', | ||
| 'Mobile Monday Lansing': 'Mobile Members', | ||
| mobile: 'Mobile Members' | ||
| } | ||
|
|
||
| const resolveKey = (group) => { | ||
| if (!group) return undefined | ||
| if (typeof group === 'string') return group | ||
| return group.name || group.id || group.slug || group.meetupUrlName | ||
| } | ||
|
|
||
| module.exports = (group) => aliases[resolveKey(group)] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,14 @@ | ||
| // The key matches group.id or event.group in the Firebase data model | ||
| module.exports = group => ({ | ||
| const lookup = { | ||
| 'demo-night': 'erik.gillespie', | ||
| javascript: 'erik.gillespie' | ||
| }[group.name]) | ||
| javascript: 'erik.gillespie', | ||
| 'Lansing Javascript Meetup': 'erik.gillespie' | ||
| } | ||
|
|
||
| const resolveKey = (group) => { | ||
| if (!group) return undefined | ||
| if (typeof group === 'string') return group | ||
| return group.id || group.slug || group.name || group.meetupUrlName | ||
| } | ||
|
|
||
| module.exports = (group) => lookup[resolveKey(group)] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,33 @@ | ||
| const TinyURL = require('tinyurl') | ||
|
|
||
| const ensureHttps = (url) => { | ||
| if (!url) return url | ||
| return url.replace(/^http:\/\//i, 'https://') | ||
| } | ||
|
|
||
| const tryShorten = (longUrl, callback, { retriedWithHttps } = { retriedWithHttps: false }) => { | ||
| TinyURL.shorten(longUrl, (...args) => { | ||
| const [firstArg, secondArg] = args | ||
|
|
||
| const shortUrl = typeof firstArg === 'string' && firstArg | ||
| ? firstArg | ||
| : (typeof secondArg === 'string' && secondArg ? secondArg : null) | ||
|
|
||
| const error = (firstArg instanceof Error ? firstArg : null) || (secondArg instanceof Error ? secondArg : null) | ||
|
|
||
| if (error || !shortUrl || shortUrl === 'Error') { | ||
| if (!retriedWithHttps && /^http:\/\//i.test(longUrl)) { | ||
| return tryShorten(ensureHttps(longUrl), callback, { retriedWithHttps: true }) | ||
| } | ||
|
|
||
| callback(ensureHttps(longUrl)) | ||
| return | ||
| } | ||
|
|
||
| callback(ensureHttps(shortUrl)) | ||
| }) | ||
| } | ||
|
|
||
| module.exports = (longUrl, callback) => { | ||
| TinyURL.shorten(longUrl, (shortUrl) => callback(shortUrl)) | ||
| tryShorten(longUrl, callback) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| // Description: | ||
| // Keep the Heroku dyno awake during configured hours. | ||
| // | ||
| // Dependencies: | ||
| // N/A | ||
| // | ||
| // Configuration: | ||
| // HUBOT_HEROKU_KEEPALIVE_URL or HEROKU_URL - Required base URL to ping. | ||
| // HUBOT_HEROKU_KEEPALIVE_INTERVAL - Minutes between pings (default 5). | ||
| // HUBOT_HEROKU_WAKEUP_TIME - Start of uptime window (HH:mm, default 06:00). | ||
| // HUBOT_HEROKU_SLEEP_TIME - End of uptime window (HH:mm, default 22:00). | ||
| // EXPRESS_USER / EXPRESS_PASSWORD - Optional basic auth credentials. | ||
| // | ||
| // Commands: | ||
| // N/A | ||
| // | ||
| // Notes: | ||
| // Replaces the deprecated hubot-heroku-keepalive external script. | ||
| // | ||
| // Author: | ||
| // Lansing Codes maintainers | ||
|
|
||
| const parseTime = (value, fallback) => { | ||
| const [hours, minutes] = (value || fallback).split(':').map(number => parseInt(number, 10) || 0) | ||
| return { hours: hours % 24, minutes: minutes % 60 } | ||
| } | ||
|
|
||
| const computeOffsets = (wakeTime, sleepTime) => { | ||
| const wakeMinutes = (wakeTime.hours * 60 + wakeTime.minutes) % (24 * 60) | ||
| const sleepMinutes = (sleepTime.hours * 60 + sleepTime.minutes) % (24 * 60) | ||
| const awakeMinutes = (sleepMinutes - wakeMinutes + 24 * 60) % (24 * 60) | ||
|
|
||
| return { wakeMinutes, awakeMinutes } | ||
| } | ||
|
|
||
| module.exports = robot => { | ||
| const url = process.env.HUBOT_HEROKU_KEEPALIVE_URL || process.env.HEROKU_URL | ||
|
|
||
| if (!url) { | ||
| robot.logger.warn('heroku-keepalive skipped: HUBOT_HEROKU_KEEPALIVE_URL/HEROKU_URL not set') | ||
| return | ||
| } | ||
|
|
||
| const normalizedUrl = url.endsWith('/') ? url : `${url}/` | ||
| const intervalMinutes = process.env.HUBOT_HEROKU_KEEPALIVE_INTERVAL | ||
| ? parseFloat(process.env.HUBOT_HEROKU_KEEPALIVE_INTERVAL) | ||
| : 5 | ||
|
|
||
| if (Number.isNaN(intervalMinutes) || intervalMinutes < 0) { | ||
| robot.logger.warn(`Invalid HUBOT_HEROKU_KEEPALIVE_INTERVAL provided: ${process.env.HUBOT_HEROKU_KEEPALIVE_INTERVAL}`) | ||
| return | ||
| } | ||
|
|
||
| if (robot.pingIntervalId) { | ||
| clearInterval(robot.pingIntervalId) | ||
| } | ||
|
|
||
| if (intervalMinutes === 0) { | ||
| robot.logger.info('Heroku keepalive disabled (interval set to 0).') | ||
| return | ||
| } | ||
|
|
||
| const wake = parseTime(process.env.HUBOT_HEROKU_WAKEUP_TIME, '6:00') | ||
| const sleep = parseTime(process.env.HUBOT_HEROKU_SLEEP_TIME, '22:00') | ||
| const { wakeMinutes, awakeMinutes } = computeOffsets(wake, sleep) | ||
|
|
||
| const tick = () => { | ||
| const now = new Date() | ||
| const minutesSinceWake = (now.getHours() * 60 + now.getMinutes() - wakeMinutes + 24 * 60) % (24 * 60) | ||
| if (minutesSinceWake >= awakeMinutes) { | ||
| robot.logger.info('Skipping Heroku keepalive ping (outside wake window).') | ||
| return | ||
| } | ||
|
|
||
| robot.logger.info('Sending Heroku keepalive ping') | ||
|
|
||
| const request = robot.http(`${normalizedUrl}heroku/keepalive`) | ||
| if (process.env.EXPRESS_USER && process.env.EXPRESS_PASSWORD) { | ||
| request.auth(process.env.EXPRESS_USER, process.env.EXPRESS_PASSWORD) | ||
| } | ||
| request.post()((error, res, body) => { | ||
| if (error) { | ||
| robot.logger.error(`Heroku keepalive failed: ${error.message}`) | ||
| robot.emit('error', error) | ||
| return | ||
| } | ||
|
|
||
| robot.logger.info(`Heroku keepalive response: ${res && res.statusCode} ${body || ''}`) | ||
| }) | ||
| } | ||
|
|
||
| if (intervalMinutes > 0) { | ||
| const intervalMs = intervalMinutes * 60 * 1000 | ||
| tick() | ||
| robot.herokuKeepaliveIntervalId = setInterval(tick, intervalMs) | ||
| } | ||
|
|
||
| const handler = (req, res) => { | ||
| res.set('Content-Type', 'text/plain') | ||
| res.send('OK') | ||
| } | ||
|
|
||
| robot.router.post('/heroku/keepalive', handler) | ||
| robot.router.get('/heroku/keepalive', handler) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.