Skip to content

fix: prevent duplicate newsletter sends from dev/test containers (#440) #441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fatherlinux merged 1 commit into from
May 29, 2026
+24 −0
Merged

fix: prevent duplicate newsletter sends from dev/test containers (#440) #441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ SESSION_SECRET=change-this-to-a-random-string
# Admin email (user with this email gets admin privileges)
ADMIN_EMAIL=scott.mccarty@gmail.com

# Newsletter sends (set to false for dev/test to avoid duplicate Buttondown emails)
NEWSLETTER_SEND_ENABLED=false

# Test environment (set automatically by vitest.config.js)
# NODE_ENV=test
# BYPASS_AUTH=true
19 changes: 19 additions & 0 deletions backend/services/buttondownClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import axios from 'axios';

const BUTTONDOWN_API_BASE = 'https://api.buttondown.email';

function isSendEnabled() {
return process.env.NEWSLETTER_SEND_ENABLED !== 'false';
}
Comment on lines +5 to +7
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The environment variable check is currently case-sensitive. If a user or environment configuration sets NEWSLETTER_SEND_ENABLED=FALSE or False, the check !== 'false' will evaluate to true, and newsletter sends will remain enabled. To prevent accidental sends due to casing differences, we should perform a case-insensitive check.

function isSendEnabled() { const val = process.env.NEWSLETTER_SEND_ENABLED; return !val || val.toLowerCase() !== 'false'; }


let apiKeyCache = null;

async function getApiKey(pool) {
Expand Down Expand Up @@ -82,6 +86,11 @@ async function retryRequest(requestFn, maxRetries = 3) {
}

export async function addSubscriber(email, pool = null) {
if (!isSendEnabled()) {
console.log('Newsletter send disabled (NEWSLETTER_SEND_ENABLED=false), skipping addSubscriber');
return { email, status: 'send_disabled', skipped: true };
}

const apiKey = await getApiKey(pool);
const client = createClient(apiKey);

Expand Down Expand Up @@ -141,6 +150,11 @@ export async function getSubscriberCount(pool = null) {
}

export async function sendEmail(subject, htmlBody, pool = null, { existingEmailId, onDraftCreated } = {}) {
if (!isSendEnabled()) {
console.log('Newsletter send disabled (NEWSLETTER_SEND_ENABLED=false), skipping sendEmail');
return { skipped: true, reason: 'send_disabled' };
}

const apiKey = await getApiKey(pool);

let emailId;
Expand Down Expand Up @@ -205,6 +219,11 @@ export async function sendEmail(subject, htmlBody, pool = null, { existingEmailI
}

export async function sendDraftToRecipients(subject, htmlBody, recipients, pool = null) {
if (!isSendEnabled()) {
console.log('Newsletter send disabled (NEWSLETTER_SEND_ENABLED=false), skipping sendDraftToRecipients');
return { emailId: null, skipped: true, reason: 'send_disabled' };
}

const apiKey = await getApiKey(pool);
const client = createClient(apiKey);

Expand Down
2 changes: 2 additions & 0 deletions run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ ENV_ARGS=""
[ -n "$PGUSER" ] && ENV_ARGS="$ENV_ARGS -e PGUSER=$PGUSER"
[ -n "$PGPASSWORD" ] && ENV_ARGS="$ENV_ARGS -e PGPASSWORD=$PGPASSWORD"
[ -n "$PGDATABASE" ] && ENV_ARGS="$ENV_ARGS -e PGDATABASE=$PGDATABASE"
[ -n "$NEWSLETTER_SEND_ENABLED" ] && ENV_ARGS="$ENV_ARGS -e NEWSLETTER_SEND_ENABLED=$NEWSLETTER_SEND_ENABLED"
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The ENV_ARGS variable is built here but is never actually used or passed to any podman run command in this script (the container instead relies on the mounted /etc/rotv/environment file). Adding NEWSLETTER_SEND_ENABLED to ENV_ARGS here is dead code and has no effect. Additionally, to ensure that the test container also has newsletter sends disabled by default, NEWSLETTER_SEND_ENABLED=false (or NEWSLETTER_SEND_ENABLED=${NEWSLETTER_SEND_ENABLED:-false}) should be added to the ~/.rotv/environment-test file generation block around line 244-255. Otherwise, since isSendEnabled() in buttondownClient.js defaults to true when the environment variable is undefined, the test container will run with newsletter sends enabled.


case "${1:-help}" in
build-base)
Expand Down Expand Up @@ -165,6 +166,7 @@ MCP_ADMIN_TOKEN=$MCP_ADMIN_TOKEN
PGUSER=${PGUSER:-postgres}
PGPASSWORD=${PGPASSWORD:-rotv}
PGDATABASE=${PGDATABASE:-rotv}
NEWSLETTER_SEND_ENABLED=${NEWSLETTER_SEND_ENABLED:-false}
ENVFILE
fi

Expand Down
Loading