Conversation
- Add Telegram bot connection flow with token validation - Create telegram_chats table to store detected channels/groups - Add telegram_automation field to apps schema - Implement webhook handler for incoming Telegram updates - Add scan-chats endpoint to discover bot's chats via polling - Create app-specific automation settings (auto-reply, auto-announce) - Add AI-powered announcement and reply generation - Build UI components for bot connection and chat selection - Add comprehensive integration tests for all endpoints
…a-cloud-v2 into feat/promotion
- Add Discord OAuth flow for bot integration - Create discord_guilds and discord_channels tables to manage server and channel data - Implement Discord automation service for message posting and status management - Add API endpoints for managing Discord connections, channels, and automation settings - Enhance UI components for Discord integration in app settings and promotion dialogs - Include comprehensive tests for Discord-related functionalities
…ation - Extend PromotionConfigSchema to include telegramAutomation and discordAutomation fields - Update validation logic to ensure required configurations are provided for Telegram and Discord channels - Modify PromoteAppDialog to handle results from new automation channels - Implement Discord automation execution logic in AppPromotionService - Add error handling and logging for automation processes
- Update PromotionConfigSchema to include channelId and groupId for Telegram automation - Implement logic to set webhook only for HTTPS URLs, with fallback for local development - Add error handling and logging for Telegram automation setup - Extend app promotion service to execute Telegram automation with new configuration options
…tomation cron - Fix discordAutomation config not initialized when channel toggled - Add input clamping for interval values (min 30/60, max 1440) - Display 'Not configured' for placeholder URLs in review step - Add post preview generation API (/api/v1/apps/[id]/promote/preview) - Add social automation cron job for periodic announcements - Initialize automation configs with valid defaults when selecting channel - Add maxDuration=60 to promote route for long operations - Fix type safety: remove unnecessary ! assertions, fix Record casts - Add Quick Actions section with Post Now buttons - Add CSP and image config for Discord CDN - Add integration tests for promotion preview API Fixes: interval validation, double-click protection, loading states
…loud-v2 into feat/promotion
…eness - Add website scraping to extract real context (title, description, features) - Auto-detect product type (SaaS, AI, Web3, etc.) and industry - Add optional custom prompt field for user-provided instructions - Fix image generation model configuration (use model string directly) - Add better error handling and logging with stream error capture - Add prompt length truncation to prevent model issues - Integrate promotional images into Discord and Telegram posts - Add 25s timeout to Discord sendMessage for reliability - Fix Telegram post route to handle empty request bodies
- Apply dark glassmorphism styling consistent with other modals - Add inner scrollable content area to prevent outer modal scroll - Update channel cards with dark theme colors and hover effects - Style form inputs, buttons, and text for dark theme - Remove auto-reply feature from Telegram automation config - Fix footer styling with proper borders and backgrounds
- Resolve merge conflicts in db/schemas/apps.ts (keep both automation + deployment fields) - Resolve merge conflicts in next.config.ts (merge CSP headers) - Regenerate bun.lock - Miniapp removed in upstream
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
PR #198 Code Review: Social Media Automation FeatureOverviewThis PR introduces comprehensive social media automation across Discord, Telegram, and Twitter platforms, including promotional asset generation with AI. The scope is significant: 78 files changed, 12,183 additions, 721 deletions. ✅ Strengths1. Well-Structured Architecture
2. Security
3. Database Design
4. Error Handling & Logging
🔴 Critical Issues1. SQL Injection Risk in Cron Job (High Priority)File: .where(
sql`${apps.discord_automation}->>'enabled' = 'true'
OR ${apps.telegram_automation}->>'enabled' = 'true'
OR ${apps.twitter_automation}->>'enabled' = 'true'`
)Issue: Using template literals with Fix: Use parameterized queries with proper Drizzle ORM operators: import { or, eq, sql } from 'drizzle-orm';
.where(
or(
sql`${apps.discord_automation}->>'enabled' = ${true}`,
sql`${apps.telegram_automation}->>'enabled' = ${true}`,
sql`${apps.twitter_automation}->>'enabled' = ${true}`
)
)Or better yet, use Drizzle's JSON operators if available. 2. Missing Error Recovery in Cron LoopFile: Issue: The cron job processes apps sequentially without try-catch around individual app processing. If one app throws an unexpected error, it could crash the entire cron job. Fix: Wrap each app's processing in a try-catch: for (const app of appsWithAutomation) {
try {
// Process Discord
const discordResult = await processDiscordAutomation(app);
// ... rest of processing
} catch (error) {
logger.error('[SocialAutomation Cron] App processing failed', {
appId: app.id,
error: error instanceof Error ? error.message : 'Unknown error',
});
// Continue to next app
}
}3. Race Condition in Credit DeductionFile: Issue: Credits are deducted upfront, then asset generation occurs, then failed generations are refunded. If the process crashes between deduction and refund, credits are lost. There's also a race condition if the same endpoint is called concurrently. Fix: Consider using database transactions or implementing idempotency keys for asset generation requests.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| organizationId: string; | ||
| userId: string; | ||
| redirectUrl?: string; | ||
| }; |
There was a problem hiding this comment.
Missing error handling for JSON.parse in OAuth callback
Medium Severity
The JSON.parse(stateJson as string) call on the cached OAuth state is not wrapped in a try-catch block. If the cache returns a corrupted or non-JSON value, this will throw an unhandled exception and crash the callback handler, leaving users stuck with an error page during the Twitter OAuth flow instead of receiving a proper error redirect.
| return NextResponse.json( | ||
| { error: error instanceof Error ? error.message : "Failed to scan for chats" }, | ||
| { status: 500 } | ||
| ); |
There was a problem hiding this comment.
Telegram webhook not restored after scan failure
Medium Severity
The scan operation deletes the Telegram webhook at line 28 before fetching updates, but if any subsequent operation fails (database upserts, webhook re-registration, etc.), the catch block returns an error without restoring the webhook. This leaves the bot unable to receive real-time messages until a successful scan is performed, potentially breaking Telegram automation for affected users.
- Add centralized promotion-pricing.ts with 20% platform markup on AI costs - Fix asset generation cost calculation in promote/assets route - Update promote dialog to show 'Free' for automation setup - Remove SEO and advertising references from promotion UI - Fix Telegram disconnect bug - was calling delete with wrong params - Add promotion pricing integration tests
…plugin - Add credit billing for Discord/Telegram/Twitter AI post generation - Revert agent-loader Twitter plugin injection (not needed for promotion) - Replace @elizaos/plugin-twitter with twitter-api-v2 in package.json - Update preview route to pass organization_id for billing
| user.organization_id, | ||
| id, | ||
| parsed.data.text, | ||
| ); |
There was a problem hiding this comment.
Twitter post endpoint ignores user-specified tweet type
Medium Severity
The request schema validates a type field with enum values "promotional", "engagement", "educational", and "announcement", but parsed.data.type is never passed to postAppTweet. The service defaults to "promotional" when generating tweets without custom text, completely ignoring the user's specified tweet type preference.
| twitterAutomation: app.twitter_automation, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Twitter automation routes missing error handling for service exceptions
Medium Severity
The Twitter automation routes (GET, POST, DELETE handlers and the post route) lack try-catch blocks around service calls, unlike their Discord and Telegram equivalents. When the automation service throws "App not found" or other errors, these routes return HTTP 500 instead of the appropriate status codes (404 for missing app, 400 for client errors). Discord and Telegram routes properly catch these exceptions and return correct status codes. Additionally, request.json() is called without error handling, causing malformed JSON to return 500 instead of 400.
Additional Locations (1)
Comprehensive Code Review - PR #198: Feat/promotionOverviewThis PR introduces significant new functionality for social media automation across Discord, Telegram, and Twitter, plus promotional asset generation. The implementation is comprehensive but has several critical security vulnerabilities and bugs that must be addressed before merging. Recommendation: REQUEST CHANGES - Critical issues need resolution. 🔴 Critical Issues (Must Fix Before Merge)1. SECURITY: Cron Authentication BypassFile: if (CRON_SECRET && authHeader !== `Bearer ${CRON_SECRET}`) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}Issue: The condition allows completely unauthenticated access when Fix: if (!CRON_SECRET || authHeader !== `Bearer ${CRON_SECRET}`) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}2. BUG: Sequential Processing Will TimeoutFile: for (const app of appsWithAutomation) {
const discordResult = await processDiscordAutomation(app);
const telegramResult = await processTelegramAutomation(app);
const twitterResult = await processTwitterAutomation(app);
}Issue: With
Fix: Process apps in parallel: await Promise.allSettled(
appsWithAutomation.map(async (app) => {
const [discordResult, telegramResult, twitterResult] = await Promise.allSettled([
processDiscordAutomation(app),
processTelegramAutomation(app),
processTwitterAutomation(app),
]);
// Handle results...
})
);3. SECURITY: Credit Deduction Without RefundFiles:
Issue: Credits are deducted before AI generation succeeds. If generation fails, users are charged but receive nothing. const deduction = await creditsService.deductCredits({...});
if (!deduction.success) { throw new Error(...); }
const result = await generateText({...}); // If this fails, no refund!Fix: Wrap AI calls in try-catch with refund: const deduction = await creditsService.deductCredits({...});
if (!deduction.success) { throw new Error(...); }
try {
const result = await generateText({...});
return result;
} catch (error) {
// Refund on failure
await creditsService.refundCredits({
organizationId,
amount: cost,
description: "Refund for failed AI generation",
});
throw error;
}4. SECURITY: SSRF Vulnerability in Asset GenerationFile: const response = await fetch(url, {...});
const html = await response.text();Issue: Fetches arbitrary URLs from
Fix: // Validate URL before fetching
const parsedUrl = new URL(url);
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
throw new Error('Invalid protocol');
}
if (parsedUrl.hostname === 'localhost' || parsedUrl.hostname === '127.0.0.1' ||
parsedUrl.hostname.startsWith('169.254.') || parsedUrl.hostname.startsWith('10.') ||
parsedUrl.hostname.startsWith('172.16.') || parsedUrl.hostname.startsWith('192.168.')) {
throw new Error('Cannot fetch from internal IP addresses');
}
// Limit response size
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 10_000);
const response = await fetch(url, {
signal: controller.signal,
size: 5 * 1024 * 1024, // 5MB limit
});
clearTimeout(timeoutId);5. SECURITY: Prompt Injection in Asset GenerationFile: customPrompt: z.string().max(1000).optional(),Issue: User-provided
Fix: Either:
🟠 High Priority Issues6. BUG: Non-Deterministic Interval CalculationFiles:
const targetInterval = minInterval + Math.random() * (maxInterval - minInterval);
return minutesSince >= targetInterval;Issue: Fix: Calculate interval once when scheduling, or use deterministic calculation: function isAnnouncementDue(config: AutomationConfig, type: "announcement" | "post"): boolean {
if (!config.enabled) return false;
const lastTime = type === "announcement" ? config.lastAnnouncementAt : config.lastPostAt;
if (!lastTime) return true;
const lastDate = new Date(lastTime);
const now = new Date();
const minutesSince = (now.getTime() - lastDate.getTime()) / (1000 * 60);
const maxInterval = type === "announcement"
? (config.announceIntervalMax ?? 240)
: (config.postIntervalMax ?? 240);
// Simply check if we've exceeded max interval (deterministic)
return minutesSince >= maxInterval;
}7. BUG: Incorrect Billing LogicFile: const imageCount = parsed.data.includeCopy !== false ? 1 : 0; // WRONG!
const bannerCount = parsed.data.includeAdBanners ? 1 : 0;
const totalImageCost = (imageCount + bannerCount) * PROMO_IMAGE_COST;
const copyCost = parsed.data.includeCopy !== false ? AD_COPY_GENERATION_COST : 0;Issue:
Fix: const imageCount = 1; // Always generate 1 social card
const bannerCount = parsed.data.includeAdBanners ? 1 : 0;
const totalImageCost = (imageCount + bannerCount) * PROMO_IMAGE_COST;
const copyCost = parsed.data.includeCopy !== false ? AD_COPY_GENERATION_COST : 0;
const totalCost = totalImageCost + copyCost;8. BUG: Asset Overwrite Loses Previous DataFile: await appsService.update(id, {
promotional_assets: promotionalAssets, // Completely overwrites!
});Issue: Overwrites all existing Fix: // Fetch current assets
const currentAssets = app.promotional_assets || [];
// Append new assets
const updatedAssets = [
...currentAssets,
...promotionalAssets,
];
await appsService.update(id, {
promotional_assets: updatedAssets,
});9. SECURITY: Missing Rate LimitingAll automation services Issue: No rate limiting on expensive operations:
Fix: Implement rate limiting: import { Ratelimit } from "@upstash/ratelimit";
import { Redis } from "@upstash/redis";
const ratelimit = new Ratelimit({
redis: Redis.fromEnv(),
limiter: Ratelimit.slidingWindow(10, "1 h"), // 10 posts per hour
});
// In postAnnouncement():
const identifier = `discord-post-${organizationId}`;
const { success } = await ratelimit.limit(identifier);
if (!success) {
throw new Error("Rate limit exceeded");
}🟡 Medium Priority Issues10. Performance: Unbounded Cache GrowthFile: private websiteContextCache = new Map<string, WebsiteContext>();Issue: No cache expiration, size limit, or LRU eviction. Memory leak risk. Fix: Implement LRU cache with TTL: import { LRUCache } from "lru-cache";
private websiteContextCache = new LRUCache<string, WebsiteContext>({
max: 500, // Max 500 entries
ttl: 1000 * 60 * 60, // 1 hour TTL
});11. Error Handling: Missing Try-CatchMultiple files - Examples:
Issue: External API calls lack error handling, causing unhandled promise rejections. Fix: Wrap all external calls: try {
const result = await discordAutomationService.sendMessage(...);
} catch (error) {
logger.error("Discord API error", { error });
throw new Error("Failed to send Discord message");
}12. Code Quality: Duplicated Default ConfigsFile: Lines 386-392 and 461-467 have identical default config definitions. DRY violation. Fix: const DEFAULT_TELEGRAM_CONFIG: TelegramAutomationConfig = {
enabled: false,
autoReply: true,
autoAnnounce: false,
announceIntervalMin: 120,
announceIntervalMax: 240,
};
// Use in both places:
const currentConfig = app.telegram_automation || DEFAULT_TELEGRAM_CONFIG;🟢 Low Priority / Nice-to-Have13. Type Safety: Unsafe CastsMultiple instances of const currentConfig = (app.twitter_automation || {}) as TwitterAutomationConfig;Fix: Use Zod for runtime validation or define proper defaults. 14. Documentation: Missing JSDocNo function-level documentation for complex logic. Add JSDoc comments explaining:
15. Testing: No Test CoverageOnly 5 test files for 78 changed files. Need integration tests for:
📊 Test Coverage ObservationsExisting tests (
🎯 RecommendationsBefore Merging:
Post-Merge Improvements:
📝 Detailed File-by-File Notesapp/api/cron/social-automation/route.tsPositives:
Issues:
lib/services/discord-automation/app-automation.tsPositives:
Issues:
lib/services/telegram-automation/app-automation.tsPositives:
Issues:
lib/services/twitter-automation/app-automation.tsPositives:
Issues:
lib/services/app-promotion-assets.tsPositives:
Issues:
🏁 ConclusionThis PR adds valuable functionality but requires significant security and reliability fixes before production deployment. The core architecture is sound, but the implementation has critical flaws that could lead to:
Overall Assessment: 6/10
Please address critical issues #1-5 and high-priority issues #6-9 before requesting re-review. Generated with Claude Code 🤖 |
- Fix cron auth bypass: require CRON_SECRET to be set - Fix non-deterministic interval calculation in cron job - Add credit refunds on AI generation failures (Discord, Telegram, Twitter) - Add SSRF protection with URL validation in asset generation - Fix asset cost calculation (always charge for social card) - Pass channelId/groupId override to Telegram post service - Add interval validation for single-bound cases (all platforms) - Increase preview route timeout to 60s - Add error handling with refunds in asset generation route - Add specific error messages in Twitter post route
Code Review for PR #198: Feat : promotionThis is a substantial PR adding Discord, Telegram, and Twitter automation capabilities for promotional activities. Overall, the implementation is well-structured with good test coverage, but there are several security, performance, and code quality concerns that should be addressed. 🔴 Critical Issues1. SQL Injection Risk in Cron Job
|
| Risk Level | Issue | Impact |
|---|---|---|
| 🔴 Critical | SSRF validation gaps | Server-side request forgery |
| 🔴 Critical | Cron secret bypass | Unauthorized execution |
| 🟡 High | Credit timing | Financial loss |
| 🟡 High | Preview cost | Credit drain |
📊 Overall Assessment
Code Quality: 7/10 - Well-structured, some inconsistencies
Security: 6/10 - Basic measures in place, critical gaps exist
Performance: 6/10 - Sequential processing limits scalability
Test Coverage: 8/10 - Excellent integration tests, missing unit tests
🎬 Recommendations Priority
Before Merge (Blocking):
- Fix cron secret validation (feat: Add versioned /api/v1 endpoints with secure auth, credit tracking, and usage auditing #2)
- Fix SSRF protection gaps (feat : Add generation metadata tracking and auditing across chat, image, and video APIs #3)
- Add rate limiting to preview endpoint (feat: Enhance Dashboard UI with Improved Visualizations and Components #7)
High Priority:
4. Implement parallel processing in cron (#6)
5. Fix credit refund logic (#5)
6. Fix timing race condition (#4)
Medium Priority:
7. Add Redis caching (#12)
8. Add unit tests for timing logic
9. Extract magic numbers (#10)
Great work on this comprehensive feature! The implementation is solid overall, but addressing the critical security issues before merge is essential.
| content, | ||
| type: postTypes[i % postTypes.length], | ||
| timestamp: new Date().toISOString(), | ||
| }); |
There was a problem hiding this comment.
Preview API labels misrepresent Discord/Telegram content types
Medium Severity
For Discord and Telegram previews, postTypes arrays define labels like "promotional", "engagement", "educational" that are included in each preview's type field. However, generateAnnouncement doesn't accept a type parameter - it generates generic announcements. The output misleadingly labels previews with different types even though content generation doesn't vary by type. Unlike Twitter's implementation which correctly passes the type to generateAppTweet, Discord and Telegram previews will show the same style of content with incorrect type labels.
Additional Locations (1)
| const errorUrl = `${returnUrl}${returnUrl.includes("?") ? "&" : "?"}discord=error&message=${encodeURIComponent( | ||
| errorDescription || error | ||
| )}`; | ||
| return NextResponse.redirect(new URL(errorUrl, baseUrl)); |
There was a problem hiding this comment.
Open redirect vulnerability in OAuth callback flows
Medium Severity
The Discord and Twitter OAuth flows accept user-controlled returnUrl/redirectUrl parameters without validating they are relative or same-origin URLs. In Discord's oauth route, returnUrl is taken from query params and embedded in state. In Twitter's connect route, redirectUrl comes from the request body. Both callback handlers then redirect to these URLs. An attacker can craft an OAuth link with a malicious external URL (e.g., https://evil.com), and after the OAuth flow completes, the victim is redirected to the attacker's site. This enables phishing attacks.
Additional Locations (2)
- Fix SSRF protection: block 0.0.0.0, 127.x.x.x range, IPv6 private, multicast, reserved - Fix timing race: use hash-based distribution for post scheduling - Add parallel processing with concurrency limit (5) in cron job - Use Drizzle or() helper for cleaner SQL query - Add error logging in preview endpoint - Extract magic numbers to constants - Return only failures in cron response to reduce payload
Code Review: PR #198 - Social Media Automation FeaturesExecutive SummaryThis PR adds comprehensive Discord, Telegram, and Twitter automation capabilities (13,070 additions across 79 files). While the implementation demonstrates solid architectural patterns and good TypeScript usage, there are critical security vulnerabilities that must be addressed before merging. Overall Assessment:
Recommendation: ⛔ DO NOT MERGE until critical issues are resolved. 🚨 Critical Security Issues (Must Fix Before Merge)1. CRON_SECRET Fail-Open VulnerabilityFile: Issue: The authentication check fails open if const CRON_SECRET = process.env.CRON_SECRET;
if (!CRON_SECRET || authHeader !== `Bearer ${CRON_SECRET}`) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}If Impact: Unauthenticated users can trigger expensive automation operations, potentially draining credits. Fix: if (!CRON_SECRET) {
logger.error("[Cron] CRON_SECRET not configured");
return NextResponse.json({ error: "Server misconfiguration" }, { status: 500 });
}
if (authHeader !== `Bearer ${CRON_SECRET}`) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}2. Telegram Webhook Has No AuthenticationFile: Issue: The webhook endpoint accepts updates from anyone who knows an organization ID. There is zero authentication of incoming requests. export async function POST(request: Request, { params }: RouteParams): Promise<NextResponse> {
const { orgId } = await params;
const botToken = await telegramAutomationService.getBotToken(orgId);
// No verification that request came from Telegram!
const update: Update = await request.json();
await bot.handleUpdate(update);
}Impact: Attackers can:
Fix: Implement Telegram's official webhook security using // Set secret when registering webhook
await telegramAutomationService.setWebhook(url, secretToken);
// Verify in webhook handler
export async function POST(request: Request, { params }: RouteParams) {
const { orgId } = await params;
const secretToken = await telegramAutomationService.getWebhookSecret(orgId);
const receivedToken = request.headers.get("x-telegram-bot-api-secret-token");
if (!secretToken || receivedToken !== secretToken) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}
// ... rest of handler
}🔴 High Priority Issues3. N+1 Query Problem in Discord Guild FetchingFile: Issue: Individual database queries for each guild's channels: const guildsWithCounts = await Promise.all(
guilds.map(async (guild) => {
const channels = await discordChannelsRepository.findByGuild(
organizationId,
guild.guild_id
);
return { ...guild, channelCount: channels.length };
})
);Impact: For 10 guilds, this makes 11 queries (1 for guilds + 10 for channels). Fix: Implement batch fetching: // In repository
async findChannelCountsByOrg(orgId: string): Promise<Map<string, number>> {
const result = await db
.select({ guildId: channels.guild_id, count: count() })
.from(channels)
.where(eq(channels.organization_id, orgId))
.groupBy(channels.guild_id);
return new Map(result.map(r => [r.guildId, r.count]));
}4. No Rate Limiting on Manual Post EndpointsFiles:
Issue: Users can spam posts as fast as their credits allow. Impact:
Recommendation: Add rate limiting: import { Ratelimit } from "@upstash/ratelimit";
const ratelimit = new Ratelimit({
redis: Redis.fromEnv(),
limiter: Ratelimit.slidingWindow(10, "1 h"), // 10 posts per hour
});
const { success } = await ratelimit.limit(`automation:${user.organization_id}`);
if (!success) {
return NextResponse.json({ error: "Rate limit exceeded" }, { status: 429 });
}5. URL Injection VulnerabilityFiles: Multiple automation service files Issue: User-provided URLs ( Example: url: app.website_url || app.app_url, // No validation!Impact: Potential for phishing or XSS if URLs contain malicious content. Fix: import { isValidUrl } from "@/lib/utils/validation";
const appUrl = app.website_url || app.app_url;
if (!appUrl || !isValidUrl(appUrl)) {
throw new Error("Invalid or missing app URL");
}🟡 Medium Priority Issues6. Code Duplication - Interval ValidationFiles:
Issue: Identical interval validation logic duplicated 3 times with constants redefined in each file. Fix: Extract to shared utility: // lib/utils/automation-validation.ts
export const DEFAULT_INTERVAL_MIN = 120;
export const DEFAULT_INTERVAL_MAX = 240;
export function validateIntervals(config: {
intervalMin?: number;
intervalMax?: number;
}) {
const { intervalMin, intervalMax } = config;
if (intervalMin && intervalMax && intervalMin > intervalMax) {
return { error: "Min interval must be less than max interval" };
}
if (intervalMax && intervalMax < DEFAULT_INTERVAL_MIN) {
return { error: `Max interval must be >= ${DEFAULT_INTERVAL_MIN}` };
}
if (intervalMin && intervalMin > DEFAULT_INTERVAL_MAX) {
return { error: `Min interval must be <= ${DEFAULT_INTERVAL_MAX}` };
}
return null;
}7. Inconsistent Error HandlingFile: Issue: Twitter automation GET endpoint lacks try-catch wrapper that Discord and Telegram have. Fix: Add consistent error handling: export async function GET(request: NextRequest, { params }: RouteParams) {
try {
const { user } = await requireAuthOrApiKeyWithOrg(request);
const { id: appId } = await params;
const status = await twitterAppAutomationService.getAutomationStatus(
user.organization_id,
appId
);
return NextResponse.json(status);
} catch (error) {
logger.error("[Twitter Automation] Failed to get status", {
error: error instanceof Error ? error.message : "Unknown error",
});
return NextResponse.json(
{ error: "Failed to get automation status" },
{ status: 500 }
);
}
}8. Concurrent Processing RiskFile: Issue: Up to 15 concurrent external API calls (5 apps × 3 platforms) could trigger rate limits. const MAX_CONCURRENT_POSTS = 5;
const [discordResult, telegramResult, twitterResult] = await Promise.all([
processDiscordAutomation(app),
processTelegramAutomation(app),
processTwitterAutomation(app),
]);Recommendation:
|
| guildId, | ||
| state, | ||
| permissions || undefined | ||
| ); |
There was a problem hiding this comment.
Discord OAuth state not validated enabling CSRF attack
High Severity
The Discord OAuth flow generates a state with a nonce but never stores it server-side for validation. The callback endpoint is unauthenticated and directly uses organizationId from the base64-decoded state without verification. An attacker can craft their own state containing a victim's organizationId, authorize their Discord server, and have it connected to the victim's organization. Compare to the Twitter OAuth implementation which properly stores state in cache and validates it during callback. The nonce provides no protection since there's nothing to validate it against.
Additional Locations (1)
PR Review: Social Media Automation & Promotion FeaturesOverviewThis PR introduces a comprehensive social media automation system for Discord, Telegram, and Twitter, along with enhanced promotional asset generation. The implementation is well-structured and covers multiple platforms with consistent patterns. Stats: 81 files changed, +13,383 additions, -2,307 deletions Code Quality & Best Practices✅ Strengths
|
Code Review - PR #198: Social Media Automation & Promotion FeaturesI've completed a comprehensive review of this large feature PR. Overall, the implementation is solid with good security practices, but I've identified several areas for improvement. ✅ StrengthsSecurity
Code Quality
Performance
|
| Category | Status | Notes |
|---|---|---|
| SSRF Protection | ✅ Excellent | Comprehensive IP filtering |
| Authentication | ✅ Good | Cron secret + org auth |
| Input Validation | ✅ Good | Zod schemas throughout |
| Rate Limiting | Verify API routes have rate limits | |
| Credit Handling | Race condition possible (see #1) | |
| SQL Injection | ✅ Safe | Using Drizzle ORM |
📝 Additional Notes
-
Database Migrations: Verify indexes exist on:
apps.discord_automation->>'enabled'apps.telegram_automation->>'enabled'apps.twitter_automation->>'enabled'
-
Monitoring: Add metrics for:
- Cron job execution time
- Failed automation posts
- Credit refund frequency
-
Documentation: Consider adding:
- Architecture diagram for automation flow
- Pricing calculation examples
- Webhook security documentation
🎯 Recommendation
Status: Approve with Changes
This is a well-implemented feature with good security practices. The critical issues (credit refund race condition, hash distribution) should be fixed before merge, but don't block the overall PR. Medium priority items can be addressed in follow-up PRs.
Before Merge:
- Fix credit refund race condition (Feat: Auth & Chat: Conversation History, Credit Tracking, Error Handling #1)
- Verify fetch timeout exists (feat: Add versioned /api/v1 endpoints with secure auth, credit tracking, and usage auditing #2)
- Consider improving hash distribution (feat : Add generation metadata tracking and auditing across chat, image, and video APIs #3)
- Add SSRF protection tests
Post-Merge (Follow-ups):
- Extract interval validation utility (feat: add credit packs, Stripe checkout , Credit tracking #4)
- Add database indexes for JSONB queries
- Improve error message consistency
- Add JSDoc to public APIs
Great work on this feature! The code quality is generally high and security considerations are well thought out. 🚀
…nstants and improved configuration handling - Introduced automation-constants.ts to centralize default configuration values for Discord, Telegram, and Twitter. - Updated Discord automation service to utilize new constants and improve OAuth configuration checks. - Enhanced Telegram automation service with configuration defaults and announcement scheduling logic. - Improved error handling and caching mechanisms for connection status in both Discord and Telegram services.
…ud-v2 into feat/promotion-improvements
Feat : promotion improvements
Code Review - PR #198: Social Media Automation & Promotion ImprovementsSummaryComprehensive PR adding Discord, Telegram, and Twitter automation features, plus promotional asset generation improvements. Well-structured with good test coverage (966 lines of integration tests). Overall: Approve with minor recommendations Strengths
Critical Issues1. CRON_SECRET Security (app/api/cron/social-automation/route.ts:32)
2. Race Condition in Asset Upload (app/actions/apps.ts:72-77)
High Priority3. File Upload DoS Risk - No rate limiting (add per-org limits) Medium Priority7. Credits deducted before generation - Consider incremental deduction PerformanceGood: Concurrency limiting, batch processing, read replicas, proper maxDuration Test CoverageExcellent: 966 LOC integration tests RecommendationAPPROVE after addressing critical items #1 and #2. Great work on this feature! |
|
|
||
| await appsService.update(id, { | ||
| promotional_assets: promotionalAssets, | ||
| }); |
There was a problem hiding this comment.
Asset generation overwrites existing promotional assets
Medium Severity
When saving generated promotional assets, the code replaces the entire promotional_assets field with only the newly generated assets, discarding any previously existing assets. This is inconsistent with uploadPromotionalAsset in app/actions/apps.ts which explicitly appends new uploads to existing assets. Users who upload custom promotional assets and later generate new ones will lose their uploaded assets. The generation endpoint likely needs to merge new assets with existing ones.
Additional Locations (1)
| }); | ||
|
|
||
| const ASSET_GENERATION_COST = 0.05; | ||
| const COPY_GENERATION_COST = 0.02; |
There was a problem hiding this comment.
Asset generation sizes parameter validated but never used
Low Severity
The GenerateAssetsSchema defines a sizes field that allows users to specify which ad sizes to generate, and the GET endpoint returns availableSizes and recommendedSizes. However, when calling generateAssetBundle, the parsed.data.sizes value is never passed to the service. The service only accepts boolean flags includeSocialCards and includeAdBanners, meaning user-specified sizes are silently ignored and the API always generates a fixed set of assets regardless of what sizes were requested.
Additional Locations (1)
…sitory - Added methods to atomically append and remove promotional assets to/from an app's promotional_assets array, utilizing JSONB operations to prevent race conditions. - Updated uploadPromotionalAsset and deletePromotionalAsset functions to leverage the new repository methods for improved reliability. - Enhanced AppPromote component to extract image dimensions from uploaded files and pass them to the upload function.
PR Review: Social Media Automation FeatureOverviewThis is a substantial PR adding end-to-end social media automation (Discord, Telegram, Twitter) with promotional asset generation. The implementation is generally well-structured with good security considerations and test coverage. ✅ StrengthsSecurity
Code Quality
Testing
🔍 Issues FoundCritical IssuesNone identified - The PR appears production-ready from a security/correctness perspective. Major Issues
Minor Issues
🚀 Performance ConsiderationsGood Practices
Potential Optimizations
📊 Test CoverageWell Tested
Missing Tests
Recommendation: Add tests for: // lib/services/app-promotion-assets.test.ts
test("blocks SSRF attempts", () => {
expect(isValidExternalUrl("http://localhost:3000")).toBe(false);
expect(isValidExternalUrl("http://127.0.0.1")).toBe(false);
expect(isValidExternalUrl("http://169.254.169.254")).toBe(false);
expect(isValidExternalUrl("http://10.0.0.1")).toBe(false);
expect(isValidExternalUrl("http://example.com")).toBe(true);
});🎯 RecommendationsBefore Merge
Post-Merge (Non-blocking)
📝 Code Style NotesCode follows good TypeScript practices:
SummaryThis is high-quality work with excellent attention to security. The atomic operations, SSRF protection, and credit refund logic show careful consideration of edge cases. Recommendation: Approve with minor changes The only blocking issue is the integer validation for width/height. Everything else can be addressed post-merge or is already well-handled. Great job on this feature! 🎉 Review conducted with focus on security, performance, code quality, and test coverage as requested. |
- db/schemas/apps.ts: Keep promotion features (twitter/telegram/discord automation, promotional_assets) and add linked_character_ids from dev - package.json: Use plugin-sql 1.7.1 from dev, keep twitter-api-v2 for promotions - proxy.ts: Add all public paths from both branches - bun.lock: Regenerated lockfile
PR Review: Feat - Promotion (Social Automation & Promotional Assets)OverviewThis PR introduces comprehensive social media automation (Discord, Telegram, Twitter) and enhanced promotional asset generation. The implementation is well-structured with 84 files changed (+13,729/-985 lines). ✅ StrengthsCode Quality
Security
Performance
Test Coverage✓ Good test coverage with 5 new integration tests:
|
| announceIntervalMax: z.number().int().min(60).max(1440).default(120), | ||
| vibeStyle: z.string().max(100).optional(), | ||
| }) | ||
| .optional(), |
There was a problem hiding this comment.
Promote route missing interval range validation
Medium Severity
The PromotionConfigSchema accepts automation configurations with announceIntervalMin/announceIntervalMax (and postIntervalMin/postIntervalMax for Twitter), but unlike the individual automation routes (discord-automation, telegram-automation, twitter-automation), this endpoint doesn't validate that the minimum interval is less than the maximum. This allows submitting invalid configurations like { announceIntervalMin: 200, announceIntervalMax: 100 } that would be rejected by the direct automation endpoints, causing inconsistent API behavior.
| success: false, | ||
| error: error instanceof Error ? error.message : "Failed to upload asset", | ||
| }; | ||
| } |
There was a problem hiding this comment.
Uploaded blob orphaned if database update fails
Low Severity
In uploadPromotionalAsset, if uploadToBlob succeeds but appendPromotionalAsset subsequently fails, the uploaded blob remains in storage with no database reference to it. The catch block returns an error without cleaning up the orphaned blob. Over time, failed uploads accumulate as unreferenced storage objects that cannot be automatically cleaned up, causing gradual storage cost increases.
2 participants
This pull request introduces new APIs for Discord automation and improves the promotional asset generation endpoint. The main changes include adding endpoints to manage and trigger Discord automation for app announcements, updating the promotional asset generation API to support custom prompts and improved credit calculation, and enhancing cost estimation and asset storage.
New Discord Automation APIs:
app/api/cron/social-automation/route.ts) for scheduled social media automation across Discord, Telegram, and Twitter, with secure authorization and logging.GET,POST, andDELETEfor enabling, updating, or disabling automation (app/api/v1/apps/[id]/discord-automation/route.ts) (app/api/v1/apps/[id]/discord-automation/route.tsR1-R161)POSTendpoint to manually trigger a Discord announcement (app/api/v1/apps/[id]/discord-automation/post/route.ts) (app/api/v1/apps/[id]/discord-automation/post/route.tsR1-R79)Promotional Asset Generation Improvements:
customPromptfield, improved credit calculation (using new constants), and now saves generated assets to the app record. Also, refunds credits for failed generations and logs asset creation. (app/api/v1/apps/[id]/promote/assets/route.ts) (app/api/v1/apps/[id]/promote/assets/route.tsL1-R2, app/api/v1/apps/[id]/promote/assets/route.tsR13-R20, app/api/v1/apps/[id]/promote/assets/route.tsR33-L30, app/api/v1/apps/[id]/promote/assets/route.tsL46-R64, app/api/v1/apps/[id]/promote/assets/route.tsL68-R73, app/api/v1/apps/[id]/promote/assets/route.tsL78-R120, app/api/v1/apps/[id]/promote/assets/route.tsL108-R131)app/api/v1/apps/[id]/promote/assets/route.ts) (app/api/v1/apps/[id]/promote/assets/route.tsR156-R172)General Improvements:
These changes provide robust automation controls for Discord announcements and a more flexible, user-friendly promotional asset generation workflow.
Note
Introduces comprehensive social automation and upgrades to promotional assets.
api/cron/social-automationto process scheduled Discord/Telegram/Twitter posts with interval windowing and concurrency limitsGET/POST/DELETEand manualPOSTannouncementGET/POST/DELETEand manualPOSTannouncementPOSTcustomPrompt, uses pricing constants/estimator, deducts credits, refunds on failures, and stores generated assets on the appWritten by Cursor Bugbot for commit dae5c1a. This will update automatically on new commits. Configure here.