Conversation
Backend (agents-work-apps): - Slack event handlers: slash commands, @mentions, message shortcuts - Agent resolution service (channel > workspace priority) - Streaming responses with SSE to Slack - Block Kit UI builders for modals, messages, and buttons - Centralized i18n for all Slack UI text Authentication & Security: - JWT tokens: SlackUserToken (agent calls), SlackLinkToken (account linking) - HMAC-SHA256 signature verification for Slack requests - Role-based permissions (admin vs member channel overrides) Nango Integration: - OAuth flow for workspace installation - Bot token storage and retrieval - Workspace default agent metadata management Database (runtime PostgreSQL): - work_app_slack_workspaces: installed workspaces - work_app_slack_user_mappings: Slack <> Inkeep user links - work_app_slack_channel_agent_configs: channel overrides Frontend (agents-manage-ui): - Slack dashboard with workspace and channel management - Channel filters (All, Private, Shared) with visual indicators - Role-based UI (admin vs member views) - User linking flow via /link page API Routes: - OAuth endpoints: /install, /oauth_redirect - Event handlers: /commands, /events, /interactions - Management: /workspaces, /channels, /users Documentation: - Comprehensive runbook and architecture docs - Flow diagrams for OAuth, commands, mentions - Developer commands and SQL snippets Co-authored-by: Cursor <cursoragent@cursor.com>
…nango version, add aria-labels
… from provider and store
- resolve tenantRole for /work-apps routes via org membership lookup (permissions.ts) - move work-apps auth context before sessionContext (createApp.ts) - add Nango webhook signature verification with timingSafeEqual (events.ts) - add 30s timeout to non-streaming fetch calls (modal-submission.ts) - prefer session userId over body.userId in link verification (users.ts) - delete internal debug routes (internal.ts) - fix stale test mock paths (nango.test.ts, client.test.ts)
- add HMAC signature to OAuth state to prevent state forgery (oauth.ts) - add 10s timeout to Slack token exchange fetch (oauth.ts) - remove blanket GET auth bypass, require auth for all workspace/user endpoints (createApp.ts) - add buffer length check before timingSafeEqual in Nango webhook verification (events.ts)
- move inline workAppsAuth from createApp.ts to middleware/workAppsAuth.ts - export from middleware barrel for reuse across work app integrations - remove unused Context/Next type imports from createApp.ts
- add userId authorization checks to /users/status, /users/connect, /users/disconnect (users.ts) - add 30s timeout to executeAgentInBackground fetch (commands/index.ts) - add 10s timeout to fetchProjectsForTenant and fetchAgentsForProject (utils.ts) - add aria-label to help documentation button (slack-dashboard.tsx) - move Slack schema imports to top of file with other runtime imports (schemas.ts) - rename :workspaceId to :teamId in DELETE workspace route for consistency (workspaces.ts)
- filter workspace listing by authenticated user's tenant to prevent cross-tenant enumeration (workspaces.ts) - add tenant ownership verification to all GET workspace routes including settings (workspaces.ts) - fix permission middleware to resolve tenant context before checking tenantId for session users (permissions.ts) - mitigate prompt injection by wrapping thread/message context in XML delimiters with untrusted data warning (app-mention.ts, modal-submission.ts) - log fallback message delivery failures instead of swallowing silently (app-mention.ts, streaming.ts) - add aria-label to external workspace link icon (workspace-hero.tsx) - export OAuth state functions and test actual production code (oauth.ts, oauth.test.ts) - add 10s timeout to fetchAgentsFromManageApi internal API calls (commands/index.ts) - fix docs link to point to existing API reference page (slack-dashboard.tsx) - fix env description for runtime database URL (env.ts) - remove broken documentation links from README (README.md)
…indings - add tenant ownership verification to test-message, bulk channel ops, PUT/DELETE channel settings (workspaces.ts) - add 10s timeout to Nango storeWorkspaceInstallation fetch (nango.ts) - add 10s timeout to sendResponseUrlMessage Slack fetch (utils.ts) - add user error feedback when agent selector modal fails to open (block-actions.ts) - return 500 on Nango webhook deletion cascade failure to trigger retry (events.ts) - update JSDoc permission model to reflect actual tenant isolation pattern (workspaces.ts) - fix misleading device authorization flow reference in link token JSDoc (slack-link-token.ts) - clarify org admin terminology in permission middleware and error messages (permissions.ts)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
There was a problem hiding this comment.
PR Review Summary
7 Key Findings | Risk: High
⚠️ Note: Due to repository permissions, this review is submitted as COMMENT rather than REQUEST_CHANGES. The security findings below should be treated as blocking.
🟠⚠️ Major (7) 🟠⚠️
🟠 1) users.ts:49-93 Cross-tenant user enumeration via link-status endpoint
Issue: The /users/link-status endpoint allows any authenticated user to check the Slack link status of any other user by email, regardless of tenant boundaries. The verifyTenantOwnership check happens after user lookup and only gates the response format, not access.
Why: An attacker with valid credentials for one tenant could enumerate which email addresses are linked to Slack workspaces across all tenants, leaking organizational membership information.
Fix: Move tenant verification to occur before the user lookup, or scope the query to only return users within the authenticated tenant:
// Option 1: Add tenantId to the query
const userMapping = await workAppSlackUserMappings.getBySlackUserId(db, {
visitorId: session?.visitorId,
tenantId: session?.tenantId, // Add tenant scoping
});
// Option 2: Verify tenant ownership before any data access
if (!session?.tenantId || workspace.tenantId !== session.tenantId) {
return c.json({ linked: false }, 200);
}Refs:
🟠 2) users.ts:357-374 Cross-tenant user disconnection
Issue: The admin disconnect endpoint at /users/:visitorId/disconnect does not verify that the target user belongs to the same tenant as the authenticated admin. An admin from Tenant A could disconnect users from Tenant B.
Why: This is a destructive cross-tenant operation that could disrupt another organization's Slack integration without authorization.
Fix: Add explicit tenant verification before performing the disconnect:
// Verify the target user belongs to the admin's tenant
const targetUserMapping = await workAppSlackUserMappings.getByVisitorId(db, visitorId);
if (!targetUserMapping || targetUserMapping.tenantId !== adminTenantId) {
return c.json({ error: 'User not found' }, 404);
}Refs:
🟠 3) users.ts:30-37 & workspaces.ts:56-63 Fail-open authorization helpers
Issue: The verifyTenantOwnership and isAuthorizedForUser helper functions return true (authorized) when the session is missing or unauthenticated, rather than failing closed.
Why: If session middleware fails or is misconfigured, these helpers would grant access instead of denying it. Security checks should fail closed by default.
Fix: Invert the default behavior to fail closed:
// Before (fail-open)
function verifyTenantOwnership(session: Session | null, workspace: Workspace): boolean {
if (!session) return true; // DANGEROUS
return session.tenantId === workspace.tenantId;
}
// After (fail-closed)
function verifyTenantOwnership(session: Session | null, workspace: Workspace): boolean {
if (!session?.tenantId) return false; // Safe default
return session.tenantId === workspace.tenantId;
}Refs:
🟠 4) permissions.ts No unit tests for security-critical middleware
Issue: The permissions middleware (packages/agents-work-apps/src/slack/middleware/permissions.ts) handles workspace admin verification and channel member authorization but has no dedicated unit tests.
Why: Permission checks are security-critical code paths. Without tests, regressions could silently introduce authorization bypasses. The existing test files cover routes and services but not this middleware directly.
Fix: Add a test file at packages/agents-work-apps/src/__tests__/slack/permissions.test.ts covering:
requireWorkspaceAdminwith valid/invalid/missing admin statusrequireChannelMemberwith member/non-member/missing channel scenarios- Edge cases: missing session, malformed workspace data, null channel configs
Refs:
🟠 5) installation.mdx:15 Documentation missing required environment variables
Issue: The Slack installation documentation references Nango integration but doesn't document the required environment variables (NANGO_SECRET_KEY, NANGO_PUBLIC_KEY, NANGO_HOST_URL, SLACK_CLIENT_ID, SLACK_CLIENT_SECRET, SLACK_SIGNING_SECRET).
Why: Self-hosted users will be unable to configure the Slack integration without knowing which environment variables to set. This creates support burden and failed deployments.
Fix: Add an "Environment Configuration" section to the installation docs listing all required variables with descriptions:
## Environment Variables
Configure these environment variables before installing the Slack app:
| Variable | Required | Description |
|----------|----------|-------------|
| `SLACK_CLIENT_ID` | Yes | OAuth client ID from Slack app settings |
| `SLACK_CLIENT_SECRET` | Yes | OAuth client secret from Slack app settings |
| `SLACK_SIGNING_SECRET` | Yes | Request signing secret for webhook verification |
| `NANGO_SECRET_KEY` | Yes | Nango API secret key for token management |
| `NANGO_PUBLIC_KEY` | Yes | Nango public key for frontend OAuth flows |
| `NANGO_HOST_URL` | No | Custom Nango host (defaults to cloud) |Refs:
🟠 6) package.json Missing changeset for new public export
Issue: The packages/agents-work-apps/package.json adds a new ./slack export entry, making this a public API surface. No changeset was created to document this addition.
Why: Consumers of @inkeep/agents-work-apps need to know about new exports via release notes. Without a changeset, this addition won't appear in the changelog.
Fix: Create a changeset:
pnpm bump minor --pkg agents-work-apps "Add Slack Work App integration with OAuth, event handling, and workspace management"Refs:
🟠 7) slack-api.ts Inconsistent API client pattern vs GitHub integration
Issue: The Slack API client in agents-manage-ui creates its own fetch-based client with manual error handling, while the existing GitHub integration uses the established apiClient pattern with React Query integration.
Why: Inconsistent patterns increase maintenance burden and make it harder to apply cross-cutting concerns (auth refresh, error handling, retry logic) uniformly across integrations.
Fix: Refactor to use the established apiClient pattern from @/lib/api-client:
// Instead of custom fetch wrapper
import { apiClient } from '@/lib/api-client';
export const slackApi = {
getWorkspace: (workspaceId: string) =>
apiClient.get(`/work-apps/slack/workspaces/${workspaceId}`),
// ... other methods
};Refs:
Inline comments:
- 🟠 Major:
users.ts:49-93Cross-tenant enumeration vulnerability - 🟠 Major:
users.ts:357-374Cross-tenant deletion vulnerability - 🟠 Major:
users.ts:30-37Fail-open authorization helper - 🟠 Major:
installation.mdx:15Missing env var documentation
🟡 Minor (5) 🟡
🟡 1) events.ts:301-329 Cache not cleared on Nango connection deletion
Issue: When a workspace is disconnected via Nango deletion, the in-memory workspaceTokenCache is not cleared. Stale tokens could be served until TTL expiration.
Why: Could cause brief authentication failures or unexpected behavior during workspace transitions.
Fix: Add cache invalidation after Nango deletion:
await nangoClient.deleteConnection(workspace.nangoConnectionId);
workspaceTokenCache.delete(workspace.id); // Add this lineRefs:
🟡 2) strings.ts:69 Confusing "projects" terminology in error message
Issue: The error message AGENT_NOT_AVAILABLE: "This channel doesn't have an assigned agent or any projects are available..." uses "projects" which may not align with user mental models in Slack context.
Why: Users in Slack context think in terms of "workspaces" and "channels", not "projects". Confusing error messages increase support burden.
Fix: Rephrase to use Slack-native terminology:
AGENT_NOT_AVAILABLE: "This channel doesn't have an assigned agent. Please contact your workspace admin to configure an agent for this channel."Refs:
🟡 3) slack-api.ts:315-335 CSV injection vulnerability in user export
Issue: The exportLinkedUsersCSV function directly interpolates user-provided data (names, emails) into CSV without escaping. Malicious data like =HYPERLINK("http://evil.com") could execute when opened in Excel.
Why: While low severity (requires malicious user data + victim opening in Excel), it's a known vulnerability class that's easy to prevent.
Fix: Escape CSV fields that start with formula characters:
function escapeCSV(field: string): string {
if (/^[=+\-@\t\r]/.test(field)) {
return `'${field}`; // Prefix with single quote
}
return field.includes(',') ? `"${field}"` : field;
}Refs:
🟡 4) nango.ts No circuit breaker on external Nango API calls
Issue: All Nango API calls are direct without circuit breaker protection. If Nango experiences an outage, every Slack interaction will fail and potentially overwhelm the service with retries.
Why: External service dependencies should have resilience patterns to prevent cascading failures.
Fix: Consider adding a circuit breaker wrapper or at minimum exponential backoff with jitter on retries.
Refs:
🟡 5) types/index.ts Workspace type missing optional fields present in schema
Issue: The SlackWorkspace TypeScript type in the UI doesn't include all fields from the database schema (e.g., defaultProjectId, nangoConnectionId are used in code but not in the type).
Why: Type mismatches can lead to runtime errors when accessing properties that TypeScript thinks don't exist.
Fix: Align the type definition with the actual API response shape, or use Zod inference from the schema.
Refs:
Inline comments:
- 🟡 Minor:
events.ts:301-329Cache invalidation gap - 🟡 Minor:
strings.ts:69Confusing terminology - 🟡 Minor:
slack-api.ts:315-335CSV injection risk
💭 Consider (3) 💭
💭 1) slack-provider.tsx Consider extracting refresh interval to config
Issue: The 30-second polling interval for workspace status is hardcoded.
Why: Makes it harder to tune for different environments (dev vs prod).
Fix: Extract to a constant or environment variable.
💭 2) workspace-hero.tsx Consider adding loading skeletons
Issue: The workspace hero section shows a brief flash when data is loading.
Why: Skeleton loaders provide better perceived performance.
Fix: Add <Skeleton /> components for the loading state.
💭 3) README.md Consider adding architecture diagram
Issue: The Slack package README explains the flow textually but lacks a visual diagram.
Why: A sequence diagram would help new contributors understand the OAuth and event flows faster.
Fix: Add a Mermaid diagram showing the key flows.
💡 APPROVE WITH SUGGESTIONS
Summary: This PR introduces a comprehensive Slack Work App integration with solid architecture, extensive test coverage (12+ test files), and thoughtful UI design. However, three security findings require attention before merge: cross-tenant enumeration (Major #1), cross-tenant deletion (Major #2), and fail-open authorization helpers (Major #3). These are authorization boundary issues that could allow one tenant to access or modify another tenant's data. The remaining findings are important but non-blocking improvements around documentation, consistency, and resilience. Great work on the overall implementation! 🎉
Discarded (12)
| Location | Issue | Reason Discarded |
|---|---|---|
oauth.ts |
Missing CSRF protection on OAuth callback | OAuth state parameter already provides CSRF protection via Nango |
events.ts |
No rate limiting on webhook endpoint | Slack's retry policy and signing verification provide implicit rate limiting |
streaming.ts |
Unbounded message streaming | Messages are bounded by Slack's 40k character limit per message |
blocks/index.ts |
XSS in block rendering | Slack API escapes content; this is Slack's responsibility |
client.ts |
Token refresh race condition | The singleflight pattern in workspace-tokens.ts handles this |
modals.ts |
Modal state not persisted | Slack modals are ephemeral by design; persistence would be anti-pattern |
agent-resolution.ts |
No caching of agent lookups | Resolution happens once per message; caching adds complexity without benefit |
commands/index.ts |
Missing command validation | Commands are validated by Slack before reaching the handler |
runtime-schema.ts |
No index on tenantId | Query patterns use slackTeamId as primary filter; tenantId index would be unused |
workAppSlack.ts |
Transaction isolation level | Default READ COMMITTED is appropriate for these operations |
slack-store.ts |
Zustand store not persisted | Intentional - workspace state should refresh on page load |
queries.ts |
No error boundary on queries | React Query's built-in error handling is sufficient |
Reviewers (13)
| Reviewer | Returned | Main Findings | Consider | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|
pr-review-security-iam |
12 | 3 | 0 | 3 | 0 | 6 |
pr-review-standards |
8 | 1 | 0 | 0 | 0 | 7 |
pr-review-consistency |
7 | 1 | 0 | 0 | 0 | 6 |
pr-review-docs |
6 | 1 | 0 | 1 | 0 | 4 |
pr-review-tests |
5 | 1 | 0 | 0 | 0 | 4 |
pr-review-frontend |
8 | 0 | 2 | 1 | 0 | 5 |
pr-review-sre |
6 | 0 | 0 | 1 | 0 | 5 |
pr-review-product |
5 | 0 | 0 | 1 | 0 | 4 |
pr-review-types |
4 | 0 | 0 | 0 | 0 | 4 |
pr-review-errors |
4 | 0 | 0 | 0 | 0 | 4 |
pr-review-architecture |
3 | 0 | 1 | 0 | 0 | 2 |
pr-review-devops |
3 | 0 | 0 | 0 | 0 | 3 |
pr-review-breaking-changes |
2 | 0 | 0 | 0 | 0 | 2 |
| Total | 73 | 7 | 3 | 7 | 0 | 56 |
Note: High discard rate reflects thorough filtering - many findings were pre-existing patterns, already handled by other mechanisms, or not attributable to this PR.
| 'application/json': { | ||
| schema: z.object({ | ||
| linked: z.boolean(), | ||
| linkId: z.string().optional(), |
There was a problem hiding this comment.
MAJOR Cross-tenant enumeration vulnerability in link-status endpoint
Issue: This endpoint accepts tenantId as a query parameter with no authentication, allowing any caller to query whether a specific Slack user in any tenant has linked their account.
Why: Attackers can enumerate linked users across organizations by iterating through tenant IDs. While this doesn't leak credentials, it reveals organizational membership and Slack user associations.
Fix: Remove tenantId from query parameters. Either:
- Require authentication and derive tenantId from the authenticated user's session
- If this endpoint must be unauthenticated for the Slack linking flow, restrict to checking only by
slackUserId+slackTeamIdwithout tenant filtering (the Slack team ID is globally unique)
Refs:
| try { | ||
| const effectiveTenantId = tenantId || 'default'; | ||
|
|
||
| if (slackUserId && slackTeamId) { | ||
| const deleted = await deleteWorkAppSlackUserMapping(runDbClient)( | ||
| effectiveTenantId, | ||
| slackUserId, | ||
| slackTeamId, | ||
| 'work-apps-slack' | ||
| ); | ||
|
|
||
| if (deleted) { | ||
| logger.info({ slackUserId, slackTeamId, tenantId: effectiveTenantId }, 'User unlinked'); | ||
| return c.json({ success: true }); | ||
| } | ||
|
|
||
| return c.json({ error: 'No link found for this user' }, 404); | ||
| } |
There was a problem hiding this comment.
MAJOR Cross-tenant data deletion when tenantId omitted
Issue: The disconnect endpoint accepts tenantId in the request body but defaults to 'default' when not provided. When disconnecting by slackUserId + slackTeamId, an attacker who knows a victim's Slack user ID and team ID could delete their user mapping by omitting tenantId.
Why: Exploit path: Authenticated user calls POST /work-apps/slack/users/disconnect with {slackUserId: 'UVICTIM', slackTeamId: 'TVICTIM'} without tenantId. If the victim's mapping exists with tenantId='default', it gets deleted.
Fix: When disconnecting by Slack IDs, derive tenantId from the authenticated user's session context rather than the request body. Alternatively, verify the mapping belongs to the caller's tenant before deletion. The isAuthorizedForUser check on line 353 only validates userId, not the Slack identifiers path.
Refs:
| function verifyTenantOwnership( | ||
| c: { get: (key: string) => unknown }, | ||
| workspaceTenantId: string | ||
| ): boolean { | ||
| const sessionTenantId = c.get('tenantId') as string | undefined; | ||
| if (!sessionTenantId) return true; // No session context (unauthenticated path) | ||
| return sessionTenantId === workspaceTenantId; | ||
| } |
There was a problem hiding this comment.
MAJOR Defense-in-depth gap: verifyTenantOwnership returns true when unauthenticated
Issue: This helper returns true (access allowed) when sessionTenantId is undefined. While routes are protected by workAppsAuth middleware, this creates a defense-in-depth gap — if the auth middleware is bypassed or misconfigured, tenant isolation is not enforced.
Why: The principle of fail-secure means authorization checks should deny by default. Returning true when no session exists means any middleware bypass results in full access rather than denial.
Fix: Change the default to deny access when session context is missing:
| function verifyTenantOwnership( | |
| c: { get: (key: string) => unknown }, | |
| workspaceTenantId: string | |
| ): boolean { | |
| const sessionTenantId = c.get('tenantId') as string | undefined; | |
| if (!sessionTenantId) return true; // No session context (unauthenticated path) | |
| return sessionTenantId === workspaceTenantId; | |
| } | |
| function verifyTenantOwnership( | |
| c: { get: (key: string) => unknown }, | |
| workspaceTenantId: string | |
| ): boolean { | |
| const sessionTenantId = c.get('tenantId') as string | undefined; | |
| if (!sessionTenantId) return false; // Fail-secure: deny when no session | |
| return sessionTenantId === workspaceTenantId; | |
| } |
Routes that intentionally allow unauthenticated access should handle that explicitly rather than relying on a permissive default.
Refs:
| function isAuthorizedForUser(c: { get: (key: string) => unknown }, requestedUserId: string): boolean { | ||
| const sessionUserId = c.get('userId') as string | undefined; | ||
| if (!sessionUserId) return true; // No session context (unauthenticated endpoints like link-status) | ||
| if (sessionUserId === requestedUserId) return true; | ||
| if (sessionUserId === 'system' || sessionUserId.startsWith('apikey:')) return true; | ||
| if (sessionUserId === 'dev-user') return true; // Dev bypass | ||
| return false; | ||
| } |
There was a problem hiding this comment.
MAJOR Defense-in-depth gap: isAuthorizedForUser returns true when unauthenticated
Issue: Similar to verifyTenantOwnership, this function returns true when sessionUserId is undefined, allowing unauthenticated callers to pass authorization checks by default.
Why: If workAppsAuth middleware fails to set userId for an authenticated route, the authorization check silently passes. Fail-secure design requires denying by default.
Fix: For endpoints that require authentication, change the default to deny:
| function isAuthorizedForUser(c: { get: (key: string) => unknown }, requestedUserId: string): boolean { | |
| const sessionUserId = c.get('userId') as string | undefined; | |
| if (!sessionUserId) return true; // No session context (unauthenticated endpoints like link-status) | |
| if (sessionUserId === requestedUserId) return true; | |
| if (sessionUserId === 'system' || sessionUserId.startsWith('apikey:')) return true; | |
| if (sessionUserId === 'dev-user') return true; // Dev bypass | |
| return false; | |
| } | |
| function isAuthorizedForUser(c: { get: (key: string) => unknown }, requestedUserId: string): boolean { | |
| const sessionUserId = c.get('userId') as string | undefined; | |
| if (!sessionUserId) return false; // Fail-secure: deny when no session | |
| if (sessionUserId === requestedUserId) return true; | |
| if (sessionUserId === 'system' || sessionUserId.startsWith('apikey:')) return true; | |
| if (sessionUserId === 'dev-user') return true; // Dev bypass | |
| return false; | |
| } |
Endpoints that intentionally allow unauthenticated access (like link-status) should handle authorization differently or skip the check entirely.
Refs:
|
|
||
| - An Inkeep account with at least one project and agent configured | ||
| - **Slack workspace admin** access (required to install apps) | ||
| - Your Inkeep deployment running with the Slack environment variables configured |
There was a problem hiding this comment.
MAJOR Missing environment variable documentation for self-hosted deployments
Issue: This prerequisite mentions "Slack environment variables configured" but does not specify which variables are required. The .env.example file shows 6+ required Slack variables but this information is not surfaced in the installation guide.
Why: Self-hosted users cannot complete the installation without knowing which environment variables to configure. They would need to search the codebase or .env.example to discover the required variables, causing friction and potential deployment failures.
Fix: Add a collapsible section or link to deployment docs with the required environment variables:
<Accordion title="Required environment variables for self-hosted deployments">
| Variable | Description |
|----------|-------------|
| `SLACK_CLIENT_ID` | Your Slack app's Client ID |
| `SLACK_CLIENT_SECRET` | Your Slack app's Client Secret |
| `SLACK_SIGNING_SECRET` | Used to verify requests from Slack |
| `SLACK_APP_URL` | Public URL where your Inkeep instance is accessible |
| `NANGO_SLACK_INTEGRATION_ID` | Nango integration ID for Slack OAuth |
</Accordion>Refs:
| const expectedSignature = crypto | ||
| .createHmac('sha256', env.NANGO_SECRET_KEY) | ||
| .update(body) | ||
| .digest('hex'); | ||
|
|
||
| if ( | ||
| signature.length !== expectedSignature.length || | ||
| !crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expectedSignature)) | ||
| ) { | ||
| logger.warn({ signature }, 'Invalid Nango webhook signature'); | ||
| return c.json({ error: 'Invalid signature' }, 401); | ||
| } | ||
| } | ||
|
|
||
| let payload: { | ||
| type: string; | ||
| success?: boolean; | ||
| connectionId?: string; | ||
| providerConfigKey?: string; | ||
| endUser?: { | ||
| endUserId: string; | ||
| endUserEmail?: string; | ||
| displayName?: string; | ||
| }; | ||
| organization?: { | ||
| id: string; | ||
| displayName?: string; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
MINOR Cache not cleared on Nango deletion webhook
Issue: When processing a connection_deleted webhook from Nango, the code deletes the workspace from the database but does not call clearWorkspaceConnectionCache(teamId). This could cause the in-memory cache to serve stale workspace data for up to 60 seconds after deletion.
Why: Deleted workspaces could continue functioning briefly if cached, potentially allowing operations on workspaces that should no longer exist.
Fix: Import and call clearWorkspaceConnectionCache(teamId) after successfully processing the deletion webhook:
import { clearWorkspaceConnectionCache } from '../services/nango';
// After line 328:
clearWorkspaceConnectionCache(teamId);Refs:
| status: { | ||
| thinking: (agentName: string) => `_${agentName} is thinking..._`, | ||
| noAgentsAvailable: 'No agents available', | ||
| noProjectsConfigured: '⚙️ No projects configured. Please set up projects in the dashboard.', |
There was a problem hiding this comment.
MINOR Error message uses internal "projects" concept unfamiliar to Slack users
Issue: The message "No projects configured. Please set up projects in the dashboard." uses "projects" which is an Inkeep internal concept that Slack users may not understand.
Why: Users encountering this error in Slack won't understand that "projects" contain agents, or why project setup is a prerequisite for using the Slack bot.
Fix: Reword to use familiar concepts:
| noProjectsConfigured: '⚙️ No projects configured. Please set up projects in the dashboard.', | |
| noProjectsConfigured: '⚙️ No agents available. Ask your admin to configure agents in the Inkeep dashboard.', |
Refs:
| async exportLinkedUsers(teamId: string): Promise<string> { | ||
| const result = await this.getLinkedUsers(teamId); | ||
| const users = result.linkedUsers; | ||
|
|
||
| if (users.length === 0) { | ||
| return 'No linked users to export'; | ||
| } | ||
|
|
||
| const headers = ['Slack Username', 'Slack Email', 'Slack User ID', 'Linked At', 'Last Used']; | ||
| const rows = users.map((user) => [ | ||
| user.slackUsername || '', | ||
| user.slackEmail || '', | ||
| user.slackUserId, | ||
| new Date(user.linkedAt).toISOString(), | ||
| user.lastUsedAt ? new Date(user.lastUsedAt).toISOString() : '', | ||
| ]); | ||
|
|
||
| const csvContent = [headers.join(','), ...rows.map((row) => row.join(','))].join('\n'); | ||
|
|
||
| return csvContent; | ||
| }, |
There was a problem hiding this comment.
MINOR CSV export vulnerable to formula injection and malformed output
Issue: User-provided data (slackUsername, slackEmail) is directly interpolated into CSV without escaping. If a username contains a comma or quote, the CSV will be malformed. If it contains formula characters (=, +, -, @), it could enable CSV formula injection when opened in Excel.
Why: CSV formula injection can execute arbitrary code when the file is opened in Excel, and malformed CSVs cause data corruption.
Fix: Properly escape CSV fields:
const escapeCSV = (value: string) => {
if (value.includes(',') || value.includes('"') || value.includes('\n')) {
return `"${value.replace(/"/g, '""')}"`;
}
// Prevent formula injection
if (/^[=+\-@]/.test(value)) {
return `'${value}`;
}
return value;
};
const rows = users.map((user) => [
escapeCSV(user.slackUsername || ''),
escapeCSV(user.slackEmail || ''),
// ...
]);Refs:
Adds customer-facing documentation for the Slack Work App under Talk to Your Agents → Slack.
Pages
/inkeep link, uninstall instructions@Inkeepmention patterns and/inkeepslash commands, multi-turn follow-up, agent resolution priority, troubleshootingAlso updates the dashboard help button to link to the new docs page instead of the non-existent
/api-reference/slack.