feat(campaigns): implement Sandbox Mode for safe campaign testing (#397) - #690
feat(campaigns): implement Sandbox Mode for safe campaign testing (#397)#690SurinderTech wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughCampaigns now record their creator and support Sandbox Mode email redirection. The campaign builder adds sandbox controls, launch confirmation, and Canvas/Mermaid Flowchart views. Gemini personalization gains organization-key selection, request timeouts, and diagnostics. ChangesSandbox Mode
Ancillary updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CampaignBuilder
participant CampaignViewSet
participant send_email_step
participant EmailProvider
CampaignBuilder->>CampaignViewSet: save sandbox_mode and sandbox_email
CampaignViewSet->>send_email_step: provide campaign creator
send_email_step->>send_email_step: resolve sandbox recipient
send_email_step->>EmailProvider: send redirected email
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/campaigns/ai.py (1)
223-228: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not gate personalization on the global API key.
When
GEMINI_API_KEYis empty butlead.organization.gemini_api_keyis configured, this early return executes before the organization key is resolved at Lines 255-258. Tenant-specific personalization is therefore silently disabled. Resolvefinal_api_keyfirst, then log/check that effective key withlogger.debug()instead ofprint().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/campaigns/ai.py` around lines 223 - 228, Update the personalization flow around _get_gemini_api_key so it resolves the effective final_api_key, including lead.organization.gemini_api_key, before applying the missing-key early return. Replace the API-key print with logger.debug() and log whether the effective key exists; gate personalization on final_api_key rather than only the global key.
🧹 Nitpick comments (1)
backend/campaigns/ai.py (1)
247-264: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMigrate this Gemini integration to
google-genai.requirements.txtstill pinsgoogle-generativeai, and this path still usesgenai.configure()/GenerativeModel; update the dependency and call sites together so the SDK and model usage stay aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/campaigns/ai.py` around lines 247 - 264, Replace the google.generativeai integration in this flow with the google-genai SDK, updating requirements.txt and the corresponding import and model initialization around the API-key selection logic. Remove genai.configure() and GenerativeModel usage, and use the google-genai client/model invocation API consistently while preserving the existing key precedence and gemini-2.5-flash model.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/campaigns/ai.py`:
- Around line 261-266: Update the Gemini request in personalize_email,
specifically the model.generate_content call, to pass
google.generativeai.types.RequestOptions with a finite timeout such as 30
seconds. Preserve the existing prompt and model selection while ensuring
send_email_step cannot remain blocked by an unbounded request.
In `@backend/campaigns/tasks.py`:
- Around line 544-577: Update _resolve_sandbox_recipient so the explicit
sandbox_email value from campaign.settings is validated and returned before
checking campaign.created_by.email. Preserve the creator email as the fallback
default, followed by the connected account email and None when no destination is
available.
- Around line 544-577: Extend the CampaignLead lookup that loads clead to
select_related campaign__created_by and
campaign__connected_account__connected_by in addition to the existing relations.
Ensure _resolve_sandbox_recipient can access both campaign.created_by.email and
the fallback connected_by.email without additional database queries.
In `@frontend/campaign-builder.html`:
- Around line 1318-1325: Update the placeholder and helper text in the sandbox
email field identified by setting-sandbox-email and sandbox-email-wrap so they
accurately describe the backend’s _resolve_sandbox_recipient fallback behavior,
specifically that the campaign creator’s email is used before the
connected-account owner. Keep the optional override behavior unchanged.
- Around line 1887-1989: The Mermaid flowchart currently interpolates
unsanitized step text into labels while loose security permits HTML execution.
Update escapeMermaidLabel and the renderFlowchart click handling to prevent
user-controlled labels from becoming HTML, preferably by disabling Mermaid click
callbacks and using a safe encoded label representation while preserving
flowchart rendering and step editing through a non-Mermaid interaction.
In `@frontend/dashboard.html`:
- Around line 513-525: Update new-campaign initialization in
frontend/campaign-builder.html to read SANDBOX_DEFAULT_KEY
(leadorbit_default_sandbox_mode) and apply its saved value to the campaign’s
initial sandbox state. Reuse the existing toggle/state initialization symbols
there, or remove initSandboxBanner and its UI if no consumer can be added.
---
Outside diff comments:
In `@backend/campaigns/ai.py`:
- Around line 223-228: Update the personalization flow around
_get_gemini_api_key so it resolves the effective final_api_key, including
lead.organization.gemini_api_key, before applying the missing-key early return.
Replace the API-key print with logger.debug() and log whether the effective key
exists; gate personalization on final_api_key rather than only the global key.
---
Nitpick comments:
In `@backend/campaigns/ai.py`:
- Around line 247-264: Replace the google.generativeai integration in this flow
with the google-genai SDK, updating requirements.txt and the corresponding
import and model initialization around the API-key selection logic. Remove
genai.configure() and GenerativeModel usage, and use the google-genai
client/model invocation API consistently while preserving the existing key
precedence and gemini-2.5-flash model.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fb2072c9-ce3a-43e6-8909-2b5cc08dbfee
📒 Files selected for processing (8)
.gitignorebackend/campaigns/ai.pybackend/campaigns/migrations/0011 campaign_created.pybackend/campaigns/models.pybackend/campaigns/tasks.pybackend/campaigns/views.pyfrontend/campaign-builder.htmlfrontend/dashboard.html
| // ─── FLOWCHART VIEW (Mermaid) ─────── | ||
| function escapeMermaidLabel(text) { | ||
| return String(text || '') | ||
| .replace(/"/g, '#quot;') | ||
| .replace(/\n/g, ' ') | ||
| .trim() | ||
| .slice(0, 60) || 'Untitled step'; | ||
| } | ||
|
|
||
| function stepFlowchartLabel(step) { | ||
| const info = ICONS[step.type] || { label: step.type }; | ||
| if (step.type === 'EMAIL') { | ||
| return step.subject ? `Email: ${step.subject}` : 'Email (no subject yet)'; | ||
| } | ||
| if (step.type === 'WAIT') { | ||
| return `Wait ${step.delay_value || 1} ${step.delay_unit || 'day(s)'}`; | ||
| } | ||
| if (step.type === 'LINKEDIN') { | ||
| return `LinkedIn: ${(step.action || 'CONNECT').replace('_', ' ')}`; | ||
| } | ||
| if (step.type === 'SMS') { | ||
| return step.body ? `SMS: ${summarizeText(step.body, 30)}` : 'SMS message'; | ||
| } | ||
| if (step.type === 'CALL') { | ||
| return 'Phone call'; | ||
| } | ||
| if (step.type === 'MANUAL') { | ||
| return step.description ? summarizeText(step.description, 30) : 'Manual task'; | ||
| } | ||
| return info.label || step.type; | ||
| } | ||
|
|
||
| function renderFlowchart() { | ||
| const container = document.getElementById('mermaidFlowchart'); | ||
| if (!container || typeof mermaid === 'undefined') return; | ||
|
|
||
| if (!steps.length) { | ||
| container.removeAttribute('data-processed'); | ||
| container.textContent = 'graph TD\n empty("No steps yet \u2014 add one in Canvas view")'; | ||
| try { mermaid.init(undefined, container); } catch (e) { console.warn('Mermaid render failed', e); } | ||
| return; | ||
| } | ||
|
|
||
| const lines = ['graph TD', ' start((Start))']; | ||
|
|
||
| steps.forEach((step, i) => { | ||
| const nodeId = `step${i}`; | ||
| const isCondition = step.type && step.type.startsWith('CONDITION_'); | ||
| const label = escapeMermaidLabel(stepFlowchartLabel(step)); | ||
|
|
||
| lines.push(isCondition ? ` ${nodeId}{"${label}"}` : ` ${nodeId}["${label}"]`); | ||
|
|
||
| if (step.condition_branch && typeof step.condition_parent_index === 'number' && steps[step.condition_parent_index]) { | ||
| const parentId = `step${step.condition_parent_index}`; | ||
| const edgeLabel = step.condition_branch === 'yes' ? 'Yes' : 'No'; | ||
| lines.push(` ${parentId} -->|${edgeLabel}| ${nodeId}`); | ||
| } else if (i === 0) { | ||
| lines.push(` start --> ${nodeId}`); | ||
| } else { | ||
| const prevIsCondition = steps[i - 1].type && steps[i - 1].type.startsWith('CONDITION_'); | ||
| if (!prevIsCondition) { | ||
| lines.push(` step${i - 1} --> ${nodeId}`); | ||
| } | ||
| } | ||
|
|
||
| lines.push(` click ${nodeId} call editStepFromFlowchart("${i}")`); | ||
| }); | ||
|
|
||
| container.removeAttribute('data-processed'); | ||
| container.textContent = lines.join('\n'); | ||
| try { | ||
| mermaid.init(undefined, container); | ||
| } catch (e) { | ||
| console.warn('Mermaid render failed', e); | ||
| } | ||
| } | ||
|
|
||
| window.editStepFromFlowchart = function (indexStr) { | ||
| const index = parseInt(indexStr, 10); | ||
| if (Number.isNaN(index) || !steps[index]) return; | ||
| selectedNodeIndex = index; | ||
| setCanvasView('canvas'); | ||
| renderCanvas(); | ||
| renderEditor(); | ||
| }; | ||
|
|
||
| function setCanvasView(view) { | ||
| canvasView = view; | ||
| const canvasBtn = document.getElementById('canvas-view-btn'); | ||
| const flowchartBtn = document.getElementById('flowchart-view-btn'); | ||
| const canvasEl = document.getElementById('canvas'); | ||
| const flowchartEl = document.getElementById('flowchart-container'); | ||
| if (canvasBtn) canvasBtn.classList.toggle('active', view === 'canvas'); | ||
| if (flowchartBtn) flowchartBtn.classList.toggle('active', view === 'flowchart'); | ||
| if (canvasEl) canvasEl.style.display = view === 'canvas' ? '' : 'none'; | ||
| if (flowchartEl) flowchartEl.style.display = view === 'flowchart' ? 'block' : 'none'; | ||
| if (view === 'flowchart') renderFlowchart(); | ||
| } | ||
|
|
||
| // ─── RENDER CANVAS ────────────────── | ||
| function renderCanvas() { | ||
| window.campaignSteps = steps; | ||
| if (canvasView === 'flowchart') renderFlowchart(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate Mermaid initialization and flowchart rendering in the target file(s)
git ls-files | rg 'frontend/campaign-builder\.html$|frontend/.*campaign-builder.*\.html$'
rg -n "mermaid\.initialize|securityLevel|escapeMermaidLabel|click ${|editStepFromFlowchart|renderFlowchart|flowchart-container" frontend/campaign-builder.html
# Show the relevant sections with line numbers
sed -n '1840,2015p' frontend/campaign-builder.htmlRepository: Kuldeeep18/LeadOrbit
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check Mermaid docs locally if vendored or referenced, and inspect any sanitization helpers
rg -n "DOMPurify|sanitize|securityLevel: 'strict'|securityLevel: 'loose'|antiscript|click .* call" frontend/campaign-builder.html frontend -g '!**/dist/**' -g '!**/build/**'Repository: Kuldeeep18/LeadOrbit
Length of output: 678
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe Mermaid directive/label semantics from installed docs if available in the repo
fd -HI "mermaid" .Repository: Kuldeeep18/LeadOrbit
Length of output: 158
🌐 Web query:
Mermaid securityLevel strict click callbacks html labels loose antiscript documentation
💡 Result:
In Mermaid, the securityLevel configuration controls the trust level of parsed diagrams, specifically governing how HTML labels are handled and whether interactive features like click callbacks are permitted [1][2]. This setting was introduced as a security improvement to mitigate risks associated with rendering untrusted content [1][3]. The available security levels are: 1. strict (default): HTML tags in the text are encoded (rendered as plain text), and click functionality is disabled [1][4]. 2. antiscript: HTML tags in the text are allowed, except for script elements which are removed. Click functionality is enabled [1][5]. 3. loose: HTML tags are allowed, and click functionality is enabled [1][3]. 4. sandbox: Diagrams are rendered within a sandboxed iframe, which prevents JavaScript execution. This mode may hinder interactive features like scripts, popups, and links [1][6]. To configure the security level, you must use the mermaid.initialize method [1][3]: javascript mermaid.initialize({ securityLevel: 'loose' }); Note that when securityLevel is not set to loose, click event callbacks may be ignored [7]. Recent updates (as of late 2025) have introduced console warnings to alert developers when click callbacks are ignored due to restrictive security settings [7][8]. Users taking responsibility for the security of their diagram sources can explicitly set the securityLevel to allow desired interactivity [1][9].
Citations:
- 1: https://github.com/mermaid-js/mermaid/blob/master/docs/config/usage.md
- 2: https://docs.mermaidchart.com/mermaid-oss/config/usage.html?id=usage
- 3: https://github.com/mermaid-js/mermaid/blob/develop/docs/config/usage.md
- 4: https://github.com/mermaid-js/mermaid/blob/9745f325/packages/mermaid/src/schemas/config.schema.yaml
- 5: https://github.com/mermaid-js/mermaid/blob/2a51ae43/docs/config/usage.md
- 6: https://mermaid-js-mermaid.mintlify.app/advanced/rendering-options
- 7: Warn users when click event callbacks are not allowed due to non-loose security level mermaid-js/mermaid#6964
- 8: fixed 6809 mermaid-js/mermaid#6868
- 9: https://github.com/mermaid-js/mermaid/blob/9745f325/packages/mermaid/src/docs/config/usage.md
Stored XSS in the Mermaid flowchart escapeMermaidLabel() only strips quotes/newlines, so step text can still be treated as HTML while securityLevel: 'loose' keeps HTML labels and click callbacks enabled. Keep click-to-edit outside Mermaid or sanitize/encode the label text before rendering.
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 1967-1967: React's useState should not be directly called
Context: setCanvasView('canvas')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/campaign-builder.html` around lines 1887 - 1989, The Mermaid
flowchart currently interpolates unsanitized step text into labels while loose
security permits HTML execution. Update escapeMermaidLabel and the
renderFlowchart click handling to prevent user-controlled labels from becoming
HTML, preferably by disabling Mermaid click callbacks and using a safe encoded
label representation while preserving flowchart rendering and step editing
through a non-Mermaid interaction.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/campaign-builder.html (1)
1889-1897: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winEscape quotes to prevent Mermaid syntax injection (XSS).
While
<and>are now correctly escaped to mitigate direct HTML injection, leaving quotes ("and') unescaped introduces a Mermaid syntax injection vulnerability.By not escaping quotes, an attacker can break out of the Mermaid label format (e.g.,
Node["..."]). WithsecurityLevel: 'loose'still active, a user could input a payload like"] click A call alert(1) %%to inject arbitrary Mermaid directives, leading to XSS when the flowchart renders.As flagged by the static analysis tools, hand-rolled sanitization is prone to bypasses. Please include quotes in your escaping chain.
🔒️ Proposed fix to securely escape quotes
return String(text || '') .replace(/&/g, '&') .replace(/</g, '<') .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, '&`#39`;')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/campaign-builder.html` around lines 1889 - 1897, Update the escaping chain in the text-conversion function around `String(text || '')` to also encode both double-quote and single-quote characters before returning the Mermaid label text. Preserve the existing ampersand, angle-bracket, and replacement order so user-controlled text cannot break Mermaid label syntax.Source: Linters/SAST tools
🧹 Nitpick comments (1)
backend/campaigns/ai.py (1)
231-235: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid potential lazy-loading of
lead.organization.Accessing
lead.organizationhere will trigger a synchronous database query because it hasn't been pre-fetched upstream. While one extra query per background task is generally acceptable, consider addinglead__organizationto theselect_relatedclause where the lead is loaded to avoid unnecessary database hits. As per the provided relevant code snippets,lead__organizationis missing from theselect_relatedcall insend_email_stepinbackend/campaigns/tasks.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/campaigns/ai.py` around lines 231 - 235, Add lead__organization to the select_related clause used when loading the lead in send_email_step, ensuring the organization is prefetched before the AI personalization logic accesses lead.organization in the relevant flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/campaigns/ai.py`:
- Line 263: The email-generation flow around genai.configure must stop mutating
the shared global Gemini configuration per request. Migrate this path to the
official google-genai client and instantiate a request-local client with
final_api_key, then use that client for subsequent model operations so
concurrent tenant requests cannot share credentials.
---
Duplicate comments:
In `@frontend/campaign-builder.html`:
- Around line 1889-1897: Update the escaping chain in the text-conversion
function around `String(text || '')` to also encode both double-quote and
single-quote characters before returning the Mermaid label text. Preserve the
existing ampersand, angle-bracket, and replacement order so user-controlled text
cannot break Mermaid label syntax.
---
Nitpick comments:
In `@backend/campaigns/ai.py`:
- Around line 231-235: Add lead__organization to the select_related clause used
when loading the lead in send_email_step, ensuring the organization is
prefetched before the AI personalization logic accesses lead.organization in the
relevant flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b8b886ae-62cb-4db0-a568-777996441b4e
📒 Files selected for processing (5)
backend/campaigns/ai.pybackend/campaigns/migrations/0011_campaign_created.pybackend/campaigns/tasks.pyfrontend/campaign-builder.htmlfrontend/dashboard.html
💤 Files with no reviewable changes (1)
- frontend/dashboard.html
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/campaigns/tasks.py
| final_api_key = active_key if active_key else api_key | ||
| from google.generativeai.types import RequestOptions | ||
|
|
||
| genai.configure(api_key=final_api_key) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Race condition on global Gemini configuration.
genai.configure(api_key=final_api_key) mutates the global configuration for the Google Generative AI library. If your email workers use threads or coroutines (e.g., ThreadPoolExecutor, gevent) to process campaigns concurrently, tasks will overwrite each other's API keys. This can lead to authentication failures or cross-tenant billing issues where one tenant's request executes using another tenant's key.
Even if workers run sequentially (e.g., Celery prefork), re-configuring the global client on every email send is an anti-pattern that may cause unexpected connection reuse or resource leaks.
If you are using the older google-generativeai SDK, consider migrating to the official google-genai SDK where you can instantiate a localized, thread-safe client per request via client = genai.Client(api_key=final_api_key). Otherwise, ensure your worker concurrency model uses strictly isolated processes rather than threads to prevent key collisions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/campaigns/ai.py` at line 263, The email-generation flow around
genai.configure must stop mutating the shared global Gemini configuration per
request. Migrate this path to the official google-genai client and instantiate a
request-local client with final_api_key, then use that client for subsequent
model operations so concurrent tenant requests cannot share credentials.
1 participant
Pull Request
Closes #397
Summary
This PR implements Sandbox Mode for campaign testing, enabling users to safely test email campaigns without sending emails to actual recipients.
Changes Made
Testing
Related Issue
Closes #397
Summary by CodeRabbit