Skip to content

Instantly share code, notes, and snippets.

@CiprianSpiridon
Last active January 28, 2026 07:59
Show Gist options
  • Select an option

  • Save CiprianSpiridon/cb32ebbb133b54209fc924741dcc62d3 to your computer and use it in GitHub Desktop.

Select an option

Save CiprianSpiridon/cb32ebbb133b54209fc924741dcc62d3 to your computer and use it in GitHub Desktop.
Security + BUG Review

Security & Bug Findings Report

Overview

This comprehensive report documents critical, high, medium, and low severity findings across the ToggleBox codebase, organized by category and severity level.

Total Issues Found: 147


Table of Contents

  1. API & Security
  2. Data Integrity & Validation
  3. Performance Issues
  4. Authentication & Authorization
  5. SDK Issues
  6. Analytics & Stats
  7. Caching & CDN
  8. Database & Adapters
  9. UI/Admin Panel
  10. Code Quality

API & Security

πŸ”΄ CRITICAL: Public Registration Allows Admin Role Escalation

File: packages/auth/src/validators/authSchemas.ts:44-63

Category: Security

Description: Public registration allows setting role to admin. Anyone can self-register as an administrator.

Impact: Complete privilege escalation to admin access.

Fix:

export const registerSchema = z.object({
  name: z.string().min(1).max(100),
  email: z.string().email().min(5).max(255),
  password: z.string().min(8).max(128).regex(/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)/),
  // remove role from public schema
});

// In UserService.register
role: 'viewer'

πŸ”΄ HIGH: Worker Logs Expose Authorization Headers

File: apps/api/src/worker.ts:142-149

Category: Security

Description: The worker logs all request headers, including Authorization and X-API-Key headers.

Impact: Secrets exposed in logs (Cloudflare logs, SIEM, etc.).

Fix:

const headers = Object.fromEntries(request.headers.entries());
delete headers.authorization;
delete headers['x-api-key'];

logger.info('Cloudflare Worker request received', {
  method: request.method,
  url: request.url,
  headers,
});

πŸ”΄ HIGH: Browser Client Logs Sensitive Cookies

File: apps/admin/src/lib/api/browser-client.ts:28-40

Category: Security

Description: When the auth token cookie is missing, the client logs the first 100 characters of document.cookie, which can include auth tokens or other sensitive cookies.

Impact: Token leakage in browser console logs and log collectors; increased account compromise risk.

Fix:

if (!token) {
  console.warn('[browserApiClient] No auth-token cookie found.');
}

πŸ”΄ HIGH: CORS Headers Override Origin Restrictions

File: packages/shared/src/middleware/validation.ts:314-318 (used in apps/api/src/app.ts)

Category: Security

Description: corsHeaders overrides cors() by setting Access-Control-Allow-Origin: * and omits X-API-Key. This effectively disables origin restrictions and breaks API key preflights.

Impact: Unexpected public access; API key calls may fail CORS validation.

Fix:

// app.ts: remove app.use(corsHeaders)
// or update corsHeaders to use config.env.CORS_ORIGIN and include X-API-Key

πŸ”΄ HIGH: Cache Headers Applied to User-Specific Endpoints

File: apps/api/src/app.ts:129-154 + packages/cache/src/middleware/cacheHeaders.ts:29-59

Category: Security / Performance

Description: Cache headers are applied broadly to /api/v1/platforms/**, including user-specific endpoints like flag evaluation and experiment assignment. CDN/browser can cache user-specific results; wrong responses served to other users.

Impact: Data leakage between users; incorrect flag/experiment assignments.

Fix:

app.use(cacheHeaders({
  pathPattern: /^\/api\/v1\/platforms\/[^/]+\/environments\/[^/]+\/(configs|flags|experiments)(\/(list|count|$))?/,
}));
app.use(
  /^\/api\/v1\/platforms\/[^/]+\/environments\/[^/]+\/(flags\/[^/]+\/evaluate|experiments\/[^/]+\/assign)/,
  noCacheHeaders()
);

πŸ”΄ HIGH: Input Sanitization Corrupts JSON Payloads

File: packages/shared/src/middleware/validation.ts:205-275

Category: Bug / Security

Description: sanitizeInput HTML-escapes all string fields, including JSON strings (e.g., config defaultValue). This mutates valid payloads (JSON becomes ") and rejects legitimate values containing javascript: as data.

Impact: Config/flag/experiment payloads corrupted or rejected; JSON configs become invalid at write time.

Fix:

const FIELDS_TO_SANITIZE = new Set(['name', 'description', 'hypothesis']);

const sanitizeObject = (obj: unknown, key?: string): unknown => {
  if (typeof obj === 'string') {
    return key && FIELDS_TO_SANITIZE.has(key) ? sanitizeString(obj) : obj;
  }
  // ... traverse with key context ...
}

🟑 HIGH: Invalid API Key Fallback if JWT Present

File: packages/shared/src/middleware/auth.ts:601-614

Category: Bug / Security

Description: If a request includes both Authorization and X-API-Key, and the JWT is invalid/expired, the middleware returns 401 without attempting API key auth. Valid API keys are rejected.

Impact: Auth failures when clients accidentally include a stale JWT header.

Fix:

export function authenticate(req: AuthRequest, res: Response, next: NextFunction): void {
  const authHeader = req.headers.authorization;
  const apiKeyHeader = req.headers['x-api-key'];

  if (authHeader && authHeader.startsWith('Bearer ')) {
    try {
      return authenticateJWT(req as AuthenticatedRequest, res, next);
    } catch {
      // fall through to API key if provided
    }
  }

  if (apiKeyHeader && typeof apiKeyHeader === 'string') {
    return authenticateAPIKey(req as ApiKeyRequest, res, next);
  }

  res.status(401).json({
    success: false,
    error: 'Authentication required. Provide either Bearer token or X-API-Key header',
    timestamp: new Date().toISOString(),
  });
}

🟑 HIGH: Missing NEXT_PUBLIC_API_URL Fallback to Localhost

File: apps/admin/src/lib/api/browser-client.ts:74-104

Category: Security

Description: If NEXT_PUBLIC_API_URL is not set, the client falls back to http://localhost:3000 and still sends the Authorization header. In production misconfigurations, JWTs could be sent to a local service.

Impact: Token leakage in misconfigured production environments.

Fix:

const baseUrl = process.env.NEXT_PUBLIC_API_URL;
if (!baseUrl) {
  if (process.env.NODE_ENV === 'production') {
    throw new Error('NEXT_PUBLIC_API_URL is required in production');
  }
}
// Only add Authorization for trusted baseUrl

🟑 MEDIUM: Authentication Enabled Hard-Coded

File: apps/api/src/routes/authRoutes.ts:14-17

Category: Bug / Security

Description: authEnabled: true is hard-coded, forcing auth routes even if deployments intend to run with ENABLE_AUTHENTICATION=false.

Impact: Environments expecting "auth disabled" may expose registration/login or fail startup if secrets missing.

Fix:

const authRouter = createAuthRouter({
  dbType: (process.env['DB_TYPE'] || 'dynamodb') as DatabaseType,
  authEnabled: process.env['ENABLE_AUTHENTICATION'] === 'true',
});

🟑 MEDIUM: User Self-Escalation via Profile Update

File: packages/auth/src/routes/userRoutes.ts:84-89

Category: Security

Description: PATCH /users/me uses updateProfileSchema, which includes role. Any user can change their own role to admin.

Impact: Privilege escalation to admin.

Fix:

export const updateProfileSelfSchema = z.object({
  name: z.string().min(1).max(100).optional(),
});

router.patch('/me', authMiddleware.authenticate, validate(updateProfileSelfSchema), userController.updateMe);
// Only admin route allows role changes

🟑 MEDIUM: Last Admin Demotion Not Enforced

File: packages/auth/src/controllers/UserController.ts:595-633

Category: Security

Description: updateUserRole allows demoting the last admin; comment mentions it but no enforcement exists.

Impact: System can end up with no admin users.

Fix:

const admins = await this.userService.listUsers({ role: 'admin' });
if (admins.total <= 1 && role !== 'admin') {
  return res.status(403).json({ success: false, error: 'Cannot remove last admin' });
}

🟑 MEDIUM: Deleted Users Retain API Keys

File: packages/auth/src/services/UserService.ts:299-301

Category: Security / Bug

Description: deleteUser assumes repository handles cascade, but DynamoDB adapter explicitly does not delete API keys/password reset tokens.

Impact: Deleted users retain active API keys, enabling unauthorized access.

Fix:

await apiKeyRepository.deleteByUser(userId);
await passwordResetRepository.deleteByUser(userId);
await userRepository.delete(userId);

🟑 MEDIUM: CSP Header Override Breaks Security Policy

File: packages/shared/src/middleware/validation.ts:349-359

Category: Bug / Security

Description: securityHeaders always sets CSP to default-src 'self', overriding Helmet's CSP config. Legitimate inline styles/scripts may be blocked.

Impact: Inconsistent security policy; legitimate content blocked.

Fix:

if (!res.getHeader('Content-Security-Policy')) {
  res.header('Content-Security-Policy', "default-src 'self'");
}
// Or remove CSP from securityHeaders and rely on helmet config

Data Integrity & Validation

πŸ”΄ CRITICAL: PHP SDK Flag Evaluation Logic Diverges from Server

File: packages/sdk-php/src/ToggleBoxClient.php:465-571

Category: Bug / Logic

Description: PHP SDK flag evaluation has multiple divergences:

  • Disabled flags return serveValue 'A' even when defaultValue is 'B'
  • forceExclude/forceInclude are reversed
  • rolloutEnabled is ignored
  • Targeting mismatches return valueA instead of defaultValue

Impact: Users get wrong values; forced cohorts reversed; analytics incorrect.

Fix: Complete rewrite of evaluateFlag() method to match server logic:

private function evaluateFlag(Flag $flag, FlagContext $context): FlagResult
{
    $getValue = fn(string $which) => $which === 'A' ? $flag->valueA : $flag->valueB;

    // 1) Disabled β†’ defaultValue
    if (!$flag->enabled) {
        $served = $flag->defaultValue ?? 'B';
        return new FlagResult($flag->flagKey, $getValue($served), $served, 'flag_disabled');
    }

    // 2) forceExclude β†’ B
    if ($flag->targeting && in_array($context->userId, $flag->targeting['forceExcludeUsers'] ?? [], true)) {
        return new FlagResult($flag->flagKey, $getValue('B'), 'B', 'force_excluded');
    }

    // 3) forceInclude β†’ A
    if ($flag->targeting && in_array($context->userId, $flag->targeting['forceIncludeUsers'] ?? [], true)) {
        return new FlagResult($flag->flagKey, $getValue('A'), 'A', 'force_included');
    }

    // 4-5) targeting + rollout logic
    // (implement same as server)

    // 6) default fallback
    $served = $flag->defaultValue ?? 'B';
    return new FlagResult($flag->flagKey, $getValue($served), $served, 'default');
}

πŸ”΄ CRITICAL: PHP SDK Missing Flag Properties

File: packages/sdk-php/src/Types/Flag.php:9-35 + packages/sdk-php/src/ToggleBoxClient.php:403-487

Category: Bug

Description: PHP SDK Flag type omits defaultValue and rollout fields. Flag evaluation ignores these properties.

Impact: Users receive wrong flag values; rollout configuration not applied.

Fix:

public function __construct(
    public readonly string $flagKey,
    public readonly string $name,
    public readonly ?string $description,
    public readonly bool $enabled,
    public readonly string $flagType,
    public readonly mixed $valueA,
    public readonly mixed $valueB,
    public readonly ?array $targeting,
    public readonly string $defaultValue,            // add
    public readonly bool $rolloutEnabled,            // add
    public readonly float $rolloutPercentageA,       // add
    public readonly float $rolloutPercentageB,       // add
    public readonly string $createdAt,
    public readonly string $updatedAt,
) {}

public static function fromArray(array $data): self {
    return new self(
        // ...
        defaultValue: $data['defaultValue'] ?? 'B',
        rolloutEnabled: $data['rolloutEnabled'] ?? false,
        rolloutPercentageA: $data['rolloutPercentageA'] ?? 100,
        rolloutPercentageB: $data['rolloutPercentageB'] ?? 0,
        // ...
    );
}

πŸ”΄ CRITICAL: Experiment Creation/Update Missing Refinements

File: packages/experiments/src/schemas.ts:307-339

Category: Bug / Data Integrity

Description: CreateExperimentSchema and UpdateExperimentSchema don't apply same refinements as ExperimentSchema (traffic allocation sum=100, control variation exists, allocation keys exist). Invalid experiments can be created.

Impact: Broken experiments; wrong assignment and analysis.

Fix:

export const CreateExperimentSchema = ExperimentBaseSchema
  .omit({ version: true, isActive: true, status: true, startedAt: true, completedAt: true, results: true, winner: true,
createdAt: true, updatedAt: true })
  .refine(...same checks as ExperimentSchema);

export const UpdateExperimentSchema = ExperimentBaseSchema
  .partial()
  .extend({ createdBy: z.string().email('Invalid email format') })
  .refine((data) => !data.trafficAllocation || Math.abs(data.trafficAllocation.reduce((s, t) => s + t.percentage, 0) - 100) <
0.01, { message: 'Traffic allocation must sum to 100%' })
  .refine((data) => !data.controlVariation || data.variations?.some(v => v.key === data.controlVariation), { message:
'Control variation must exist in variations' })
  .refine((data) => !data.trafficAllocation || !data.variations || data.trafficAllocation.every(t => data.variations!.some(v
=> v.key === t.variationKey)), { message: 'All traffic allocation keys must exist in variations' });

🟑 HIGH: Flag Update Missing Transaction

File: packages/database/src/adapters/dynamodb/DynamoDBNewFlagRepository.ts:143-167

Category: Bug / Data Consistency

Description: Flag update deactivates current version then writes new version without a transaction. If the new Put fails, flag has no active version.

Impact: Flags can disappear or become inconsistent under failure or concurrent updates.

Fix:

const { TransactWriteCommand } = await import('@aws-sdk/lib-dynamodb');
await dynamoDBClient.send(new TransactWriteCommand({
  TransactItems: [
    { Update: { ...deactivate current version... } },
    { Put: { ...new active version... } },
  ],
}));

🟑 HIGH: Mongoose Config Update Missing Transaction

File: packages/database/src/adapters/mongoose/MongooseConfigRepository.ts:104-156

Category: Bug

Description: Update path deactivates current config version before inserting new version without a transaction. If insert fails, parameter has no active version.

Impact: Config reads can fail; configuration downtime.

Fix:

const session = await ConfigParameterModel.startSession();
await session.withTransaction(async () => {
  await ConfigParameterModel.updateOne(..., { isActive: false }).session(session);
  await ConfigParameterModel.create([{ ...newVersion, isActive: true }], { session });
});

🟑 HIGH: D1 Config Update Missing Transaction

File: packages/database/src/adapters/d1/D1ConfigRepository.ts:146-208

Category: Bug

Description: Non-atomic deactivate-then-insert pattern; active version can disappear on failure or race.

Impact: Configuration service downtime.

Fix:

await this.db.batch([
  this.db.prepare(`UPDATE config_parameters SET isActive = 0 ...`),
  this.db.prepare(`INSERT INTO config_parameters (...) VALUES (...)`)
]);
// Or keep old active until new insert succeeds, then flip active in second step

🟑 HIGH: Partial Experiment Update Validation Broken

File: packages/experiments/src/schemas.ts:372-395

Category: Bug / Data Integrity

Description: UpdateExperimentSchema only validates controlVariation and trafficAllocation if variations are also provided. Allows invalid updates.

Impact: Experiments can enter invalid states; assignment breaks.

Fix:

const current = await this.get(platform, environment, experimentKey);
const merged = {
  ...current,
  ...data,
  variations: data.variations ?? current.variations,
  trafficAllocation: data.trafficAllocation ?? current.trafficAllocation,
  controlVariation: data.controlVariation ?? current.controlVariation,
};

// Validate with full ExperimentSchema
ExperimentSchema.parse(merged);

🟑 HIGH: Experiment Winner Validation Missing

File: packages/experiments/src/schemas.ts:411-413 + apps/api/src/controllers/experimentController.ts:278-306

Category: Bug / Data Integrity

Description: winner accepted as any string, never validated against existing variations.

Impact: Completed experiments reference non-existent winners.

Fix:

const current = await this.get(platform, environment, experimentKey);
if (winner && !current.variations.some((v) => v.key === winner)) {
  throw new Error(`Winner variation "${winner}" does not exist for ${experimentKey}`);
}

🟑 MEDIUM: UpdateFlagSchema Missing Rollout Validation

File: packages/flags/src/schemas.ts:182-194

Category: Bug / Data Integrity

Description: UpdateFlagSchema allows rolloutPercentageA and rolloutPercentageB without enforcing they sum to 100.

Impact: Inconsistent rollouts; unexpected targeting results.

Fix:

export const UpdateFlagSchema = z.object({ ... })
  .refine((data) => {
    if (data.rolloutPercentageA !== undefined && data.rolloutPercentageB !== undefined) {
      return data.rolloutPercentageA + data.rolloutPercentageB === 100;
    }
    return true;
  }, { message: 'rolloutPercentageA + rolloutPercentageB must equal 100' });

🟑 MEDIUM: Config DefaultValue Validation Missing Type Check

File: packages/configs/src/schemas.ts:90-114

Category: Bug / Data Integrity

Description: defaultValue validated as string but not against valueType. Invalid numbers/JSON stored.

Impact: Config lookups silently fail or return null.

Fix:

export const CreateConfigParameterSchema = z.object({
  // ...
  valueType: ConfigValueTypeSchema,
  defaultValue: z.string().max(CONFIG_LIMITS.MAX_VALUE_LENGTH),
  // ...
}).superRefine((data, ctx) => {
  const parsed = parseConfigValue(data.defaultValue, data.valueType);
  if (parsed === null) {
    ctx.addIssue({
      code: z.ZodIssueCode.custom,
      message: `defaultValue is not valid for type ${data.valueType}`,
      path: ['defaultValue'],
    });
  }
});

🟑 MEDIUM: ParseConfigValue Returns NaN

File: packages/configs/src/schemas.ts:129-135

Category: Bug / Data Integrity

Description: parseConfigValue returns NaN for invalid numeric strings; leaks to SDKs.

Impact: Runtime errors; incorrect feature behavior.

Fix:

case 'number': {
  const num = parseFloat(value);
  return Number.isFinite(num) ? num : null;
}

🟑 MEDIUM: Division by Zero in Significance Tests

File: packages/stats/src/significance.ts:150-173

Category: Bug

Description: Division by zero when participants is 0 (control or treatment); yields Infinity/NaN.

Impact: Invalid statistical results; UI crashes; misleading conclusions.

Fix:

if (control.participants === 0 || treatment.participants === 0) {
  return {
    pValue: 1,
    isSignificant: false,
    zScore: 0,
    confidenceInterval: [0, 0],
    relativeLift: 0,
    controlConversionRate: 0,
    treatmentConversionRate: 0,
  };
}

🟑 MEDIUM: Concurrent User Updates Can Overwrite Changes

File: packages/auth/src/adapters/dynamodb/userService.ts:170-197

Category: Bug / Concurrency

Description: updateUser does read-modify-write with PutCommand and no conditional check. Concurrent updates overwrite.

Impact: Lost updates (e.g., role change overwritten by profile change).

Fix:

// Use conditional update on updatedAt
ConditionExpression: 'updatedAt = :prev',
ExpressionAttributeValues: { ':prev': existingUser.updatedAt.toISOString(), ... }

πŸ”΄ HIGH: Config Controller Missing Validation

File: apps/api/src/controllers/configController.ts:52-74

Category: Bug / Data Integrity

Description: Controller-local schemas don't validate defaultValue against valueType (unlike @togglebox/configs), and don't enforce MAX_VALUE_LENGTH. Invalid numbers/JSON/boolean strings can be stored.

Impact: Invalid data stored; downstream parsing can throw or silently misbehave.

Fix:

// Use shared schemas to keep validation consistent
import { CreateConfigParameterSchema, UpdateConfigParameterSchema } from '@togglebox/configs';
const bodyData = CreateConfigParameterSchema.parse({
  ...req.body,
  platform,
  environment,
  createdBy,
});

🟑 MEDIUM: Pagination parseInt Not Validated

File: packages/shared/src/utils/pagination.ts:86-118

Category: Bug / Performance

Description: parseInt results not validated; invalid query params produce NaN, which propagates into limit, offset, and page.

Impact: Can cause 500s, broken pagination, or accidental full scans when NaN passed to DB drivers.

Fix:

const toInt = (v: unknown, fallback: number) => {
  const n = typeof v === 'string' ? Number(v) : fallback;
  return Number.isFinite(n) ? n : fallback;
};

const limitNum = toInt(limit, PAGINATION_DEFAULTS.DEFAULT_PER_PAGE);
const offsetNumRaw = toInt(offset, 0);

🟑 MEDIUM: ListActive Always Applies Hard Limit

File:

  • packages/database/src/adapters/prisma/PrismaConfigRepository.ts:213-239
  • packages/database/src/adapters/mongoose/MongooseConfigRepository.ts:259-282
  • packages/database/src/adapters/d1/D1ConfigRepository.ts:266-295

Category: Bug / Performance

Description: When pagination omitted, listActive still applies hard limit ?? 100. Conflicts with getTokenPaginationParams/isPaginationRequested contract ("fetch ALL items if no pagination requested").

Impact: Admin/API callers get incomplete lists without realizing; breaks UI and exports.

Fix:

// Prisma example
const offsetPagination = pagination as OffsetPaginationParams | undefined;
const params = await this.prisma.configParameter.findMany({
  where: { platform, environment, isActive: true },
  orderBy: { parameterKey: 'asc' },
  ...(offsetPagination ? { skip: offsetPagination.offset, take: offsetPagination.limit } : {}),
});

🟑 MEDIUM: Pagination Metadata Always Shows Page 1

File:

  • apps/api/src/controllers/configController.ts:430-447
  • apps/api/src/controllers/configController.ts:737-756
  • apps/api/src/controllers/configController.ts:977-997

Category: Bug

Description: Pagination metadata uses createPaginationMeta(1, pagination.limit, total) (page always 1).

Impact: Incorrect page/hasNext/hasPrev values; frontends mis-render pagination.

Fix:

import { getPaginationParams } from '@togglebox/shared';
const { page, perPage } = getPaginationParams(req);
const meta = createPaginationMeta(page, perPage, result.total);

🟑 MEDIUM: Traffic Allocation Update Missing Validation

File: apps/api/src/controllers/experimentController.ts:803-818

Category: Bug

Description: updateTrafficAllocation only validates each percentage 0–100, but does not enforce sum = 100 or validate variation keys exist.

Impact: Users create allocations that break assignment logic or skew SRM analysis.

Fix:

const TrafficAllocationUpdateSchema = z.object({
  trafficAllocation: z.array(z.object({
    variationKey: z.string(),
    percentage: z.number().min(0).max(100),
  })).min(2),
}).superRefine((data, ctx) => {
  const total = data.trafficAllocation.reduce((s, t) => s + t.percentage, 0);
  if (Math.abs(total - 100) > 0.01) {
    ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'Traffic allocation must sum to 100%' });
  }
});

🟑 MEDIUM: Sample Size Calculation Unvalidated

File: packages/stats/src/significance.ts:240-264

Category: Bug / Performance

Description: calculateRequiredSampleSize does not validate inputs. If minimumDetectableEffect is 0 or baselineConversion is 0, denominator becomes 0 and returns Infinity or NaN.

Impact: UI/API consumers can crash or show nonsensical sample sizes.

Fix:

if (baselineConversion <= 0 || baselineConversion >= 1) {
  throw new Error('baselineConversion must be between 0 and 1');
}
if (minimumDetectableEffect <= 0) {
  throw new Error('minimumDetectableEffect must be > 0');
}

🟑 MEDIUM: Cursor Parsing Not Validated

File:

  • packages/database/src/adapters/mongoose/MongooseFlagRepository.ts:216-224
  • packages/database/src/adapters/mongoose/MongooseExperimentRepository.ts:268-277
  • packages/database/src/adapters/prisma/PrismaFlagRepository.ts:265-272
  • packages/database/src/adapters/prisma/PrismaExperimentRepository.ts:299-307
  • packages/database/src/adapters/d1/D1FlagRepository.ts:308-315
  • packages/database/src/adapters/d1/D1ExperimentRepository.ts:422-430

Category: Bug / Security

Description: Cursor parsing does not validate decoded values; malformed cursors yield NaN and can blow up skip/offset calls.

Impact: Bad cursor can trigger exceptions (DoS vector) or undefined pagination behavior.

Fix:

const parseOffset = (cursor?: string) => {
  if (!cursor) return 0;
  const n = Number(Buffer.from(cursor, 'base64').toString());
  if (!Number.isFinite(n) || n < 0) throw new Error('Invalid cursor');
  return n;
};

🟑 MEDIUM: Token vs Offset Pagination Mixing

File:

  • packages/shared/src/utils/pagination.ts:226-236
  • apps/api/src/controllers/configController.ts:427-447

Category: Bug

Description: getTokenPaginationParams discards page/offset. Config list endpoints use it for SQL/Prisma/D1, so page/perPage always behave like page 1 and metadata wrong.

Impact: Admin pagination broken for non-DynamoDB backends.

Fix:

// Controller: decide pagination mode
const hasToken = typeof req.query.nextToken === 'string';
const tokenPagination = hasToken ? getTokenPaginationParams(req) : undefined;
const offsetPagination = !hasToken ? getPaginationParams(req) : undefined;

const result = await this.db.config.listActive(platform, environment, offsetPagination ?? tokenPagination);
// meta should use offsetPagination.page/perPage when present

🟑 MEDIUM: Query Params Not Validated or Clamped

File: packages/auth/src/controllers/UserController.ts:384-391

Category: Bug / Performance

Description: limit and offset not validated or clamped. Negative/NaN values reach data layer.

Impact: Unbounded queries or errors in repositories.

Fix:

const toPosInt = (v: unknown, fallback: number) => {
  const n = Number(v);
  return Number.isFinite(n) && n >= 0 ? n : fallback;
};
const limit = Math.min(100, Math.max(1, toPosInt(req.query.limit, 20)));
const offset = Math.max(0, toPosInt(req.query.offset, 0));

🟑 MEDIUM: Laravel Cache Flush Wipes Entire Store

File: packages/sdk-laravel/src/Cache/LaravelCacheAdapter.php:36-40

Category: Performance / Code Quality

Description: clear() calls $cache->flush(), which wipes entire cache store, not just ToggleBox keys.

Impact: Evicts unrelated application caches; potential latency spikes and cache stampedes.

Fix:

// Prefer tags when supported
if (method_exists($this->cache, 'tags')) {
    $this->cache->tags([$this->prefix])->flush();
    return;
}
// Otherwise: track keys or no-op with warning

🟒 LOW: Parameter Limit Check Non-Atomic

File: apps/api/src/controllers/configController.ts:183-197

Category: Bug / Concurrency

Description: Parameter limit check is non-atomic (count then create). Parallel requests can exceed MAX_PARAMETERS_PER_ENV.

Impact: Concurrent requests bypass per-environment parameter limits.

Fix:

// Option A: enforce in DB with transaction + constraint
// Option B: re-check in repository or use a transactional "count + insert" in the DB adapter

πŸ”΄ HIGH: Conversion Recording Missing Validation

File: apps/api/src/controllers/statsController.ts:375-402

Category: Bug / Data Integrity

Description: recordConversion accepts any metricId/variationKey without validating they belong to the experiment.

Impact: Invalid conversions can poison stats, break significance analysis, mislead decisioning.

Fix:

const experiment = await this.repos.experiment.get(platform, environment, experimentKey);
if (!experiment) {
  res.status(404).json({ success: false, error: 'Experiment not found' });
  return;
}
const validMetricIds = new Set([
  experiment.primaryMetric?.id,
  ...(experiment.secondaryMetrics?.map(m => m.id) ?? []),
]);
const validVariationKeys = new Set(experiment.variations.map(v => v.key));
if (!validMetricIds.has(metricId) || !validVariationKeys.has(variationKey)) {
  res.status(422).json({
    success: false,
    error: 'Invalid metricId or variationKey for experiment',
    code: 'VALIDATION_FAILED',
  });
  return;
}

🟑 MEDIUM: JWT Validation Blocks API Key Auth

File: packages/auth/src/middleware/auth.ts:335-352

Category: Bug

Description: If stale/invalid JWT is present, authenticate() returns 401 and never attempts API key auth, even if valid X-API-Key provided.

Impact: Legitimate requests rejected when clients send both headers and JWT is expired.

Fix:

if (authHeader && authHeader.startsWith('Bearer ')) {
  try {
    await authenticateJWT(req as AuthenticatedRequest, res, next);
    return;
  } catch {
    // fall through to API key if provided
  }
}
if (apiKeyHeader && typeof apiKeyHeader === 'string') {
  await authenticateAPIKey(req as ApiKeyRequest, res, next);
  return;
}

🟑 MEDIUM: DynamoDB ListUsers Full Table Scan

File: packages/auth/src/adapters/dynamodb/userService.ts:260-287

Category: Performance

Description: listUsers performs full table scan then slices in memory for pagination.

Impact: Poor scalability and high RCUs as user count grows; latency spikes on admin list calls.

Fix:

// Add a GSI: GSI2PK = "USERS", GSI2SK = createdAt
// Then query with Limit + ExclusiveStartKey
const params = {
  TableName: getUsersTableName(),
  IndexName: 'GSI2',
  KeyConditionExpression: 'GSI2PK = :pk',
  ExpressionAttributeValues: { ':pk': 'USERS' },
  Limit: limit,
  ExclusiveStartKey: lastKey,
};

🟑 MEDIUM: DynamoDB CountUsersByRole Full Table Scan

File: packages/auth/src/adapters/dynamodb/userService.ts:302-317

Category: Performance

Description: countUsersByRole uses table scan with filter.

Impact: High RCUs and slow responses on large tables; called for "last admin" checks.

Fix:

// Add a role index: GSI_ROLE_PK = "ROLE#<role>", GSI_ROLE_SK = "USER#<id>"
// Then use Query + Select: 'COUNT'

🟑 MEDIUM: DynamoDB UpdateUser Lost Updates

File: packages/auth/src/adapters/dynamodb/userService.ts:170-195

Category: Bug / Concurrency

Description: updateUser uses read-modify-write with PutCommand and no conditional check. Concurrent updates overwrite.

Impact: Lost updates, especially for role changes or password resets.

Fix:

// Add an optimistic lock (updatedAt or version)
ConditionExpression: 'updatedAt = :prev',
ExpressionAttributeValues: { ':prev': existingUser.updatedAt.toISOString() },

🟑 MEDIUM: Password Reset Token Cleanup Expensive

File: packages/auth/src/adapters/dynamodb/passwordResetService.ts:281-301

Category: Performance

Description: deleteExpiredPasswordResetTokens scans entire table and deletes individually.

Impact: Expensive RCUs and slow cleanup as token volume grows.

Fix:

// Prefer DynamoDB TTL on `expiresAt` to auto-expire tokens.
// If manual cleanup remains, use BatchWrite + segmented scans.

Authentication & Authorization

πŸ”΄ HIGH: UpdateMe Allows Role/Password Escalation

File: packages/auth/src/controllers/UserController.ts:135-147

Category: Security

Description: updateMe passes raw req.body to userService.updateProfile, which accepts role and passwordHash. Lets normal user elevate to admin or set arbitrary password hashes.

Impact: Privilege escalation and account takeover.

Fix:

import { updateProfileSchema } from '../validators/authSchemas';
const data = updateProfileSchema.parse(req.body); // only { name? }
const user = await this.userService.updateProfile(req.user.userId, data);

🟑 MEDIUM: ChangePassword Missing Validation

File: packages/auth/src/controllers/UserController.ts:193-214

Category: Security / Bug

Description: changePassword doesn't validate input; newPassword can be empty/weak or undefined, causing runtime errors or weak credentials.

Impact: Weak password acceptance or unexpected errors.

Fix:

import { changePasswordSchema } from '../validators/authSchemas';
const { currentPassword, newPassword } = changePasswordSchema.parse(req.body);
await this.userService.changePassword(req.user.userId, currentPassword, newPassword);

🟑 MEDIUM: CreateUser Missing Email/Password Validation

File: packages/auth/src/controllers/UserController.ts:279-309

Category: Security / Code Quality

Description: createUser only checks presence and role; does not enforce email format or password strength, despite existing registerSchema.

Impact: Invalid emails and weak passwords can be stored.

Fix:

import { registerSchema } from '../validators/authSchemas';
const { name, email, password, role } = registerSchema.parse(req.body);
const user = await this.userService.register({ name, email, password, role });

🟑 MEDIUM: Password Reset Endpoints Missing Validation

File: packages/auth/src/controllers/PasswordResetController.ts:82-230

Category: Security / Bug

Description: Password reset endpoints do not validate request bodies (email/token/password).

Impact: Invalid payloads reach services; leads to errors, weak resets, or inconsistent behavior.

Fix:

import {
  passwordResetRequestSchema,
  passwordResetVerifySchema,
  passwordResetCompleteSchema,
} from '../validators/authSchemas';
const { email } = passwordResetRequestSchema.parse(req.body);
const { token } = passwordResetVerifySchema.parse(req.body);
const { token, newPassword } = passwordResetCompleteSchema.parse(req.body);

🟑 MEDIUM: Conversion Events Use MetricName Not MetricId

File:

  • packages/stats/src/types.ts:216-233
  • packages/database/src/adapters/prisma/PrismaStatsRepository.ts:643-653

Category: Bug / Data Integrity

Description: Conversion events carry metricName, but recordConversion stores by metricId. If metric's id differs from display name, conversions recorded under wrong metric.

Impact: Experiment metrics become inaccurate or split across IDs, breaking analysis.

Fix:

// Option A: change event schema + SDKs to send metricId
export const ConversionEventSchema = BaseEventSchema.extend({
  type: z.literal('conversion'),
  experimentKey: z.string(),
  metricId: z.string(), // instead of metricName
  variationKey: z.string(),
  userId: z.string(),
  value: z.number().optional(),
});
// Option B: map metricName -> metricId by loading experiment config in processBatch

πŸ”΄ HIGH: GetApiKey Missing Ownership Check

File: packages/auth/src/controllers/ApiKeyController.ts:242-279

Category: Security

Description: getApiKey returns API key metadata for any authenticated user without verifying ownership (comment even notes this).

Impact: Any user can enumerate other users' key IDs and view permissions/metadata.

Fix:

// ApiKeyService
async getApiKeyForUser(id: string, userId: string): Promise<PublicApiKey | null> {
  const apiKey = await this.apiKeyRepository.findById(id);
  if (!apiKey || apiKey.userId !== userId) return null;
  const { keyHash, userId: _, ...publicKey } = apiKey;
  return publicKey;
}
// ApiKeyController.getApiKey
const apiKey = await this.apiKeyService.getApiKeyForUser(id, req.user.userId);
if (!apiKey) {
  res.status(404).json({ success: false, error: 'API key not found' });
  return;
}

🟑 MEDIUM: SRM Calculation Unhandled Error

File: apps/api/src/controllers/experimentController.ts:766-768

Category: Bug

Description: checkSRM called without try/catch. If traffic allocation malformed (sum != 1) or variation counts mismatch, throws and endpoint returns 500.

Impact: Experiment analysis endpoint fails for bad data or partial migrations.

Fix:

let srmResult = null;
try {
  const expectedRatios = experiment.trafficAllocation.map((t) => t.percentage / 100);
  srmResult = checkSRM(variationData, expectedRatios);
} catch (err) {
  logger.warn('SRM calculation failed', { error: err instanceof Error ? err.message : String(err) });
}

🟑 MEDIUM: Daily Experiment Stats Always Empty

File:

  • packages/stats/src/types.ts:125-130
  • packages/database/src/adapters/prisma/PrismaStatsRepository.ts:365-387
  • packages/database/src/adapters/d1/D1StatsRepository.ts:383-392
  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:332-349
  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:520-557

Category: Bug / Data Integrity

Description: Daily experiment stats never populated with conversions; getExperimentStats() always returns dailyData: []. ExperimentStatsDailySchema requires conversions.

Impact: Experiment time-series charts empty or inconsistent across all backends.

Fix:

// Add conversions to daily stats and populate dailyData in getExperimentStats
// Example (Prisma):
await prisma.experimentStatsDaily.upsert({
  where: { platform_environment_experimentKey_variationKey_date: { platform, environment, experimentKey, variationKey, date: today } },
  create: { platform, environment, experimentKey, variationKey, date: today, participants: 1, conversions: 0 },
  update: { participants: { increment: 1 } },
});

// And when recording conversion, also increment daily conversions:
await prisma.experimentStatsDaily.update({
  where: { platform_environment_experimentKey_variationKey_date: { platform, environment, experimentKey, variationKey, date: today } },
  data: { conversions: { increment: 1 } },
});

// Then in getExperimentStats: query daily stats and fill dailyData.

🟑 MEDIUM: Stats Collector Queue Re-queue No Size Check

File: packages/stats/src/collector.ts:221-233

Category: Performance / Bug

Description: On transient send failure, events re-queued via unshift without enforcing maxQueueSize.

Impact: Repeated failures grow queue beyond intended cap, causing memory growth.

Fix:

// After requeue, clamp size
this.queue.unshift(...events);
if (this.queue.length > this.options.maxQueueSize) {
  this.queue.length = this.options.maxQueueSize; // drop oldest overflow
}

🟑 MEDIUM: Cache Invalidation Targets Non-Existent Routes

File:

  • packages/cache/src/providers/CloudFrontCacheProvider.ts:183-204
  • packages/cache/src/providers/CloudflareCacheProvider.ts:220-223

Category: Bug / Performance

Description: generateCachePaths invalidates /versions/:version paths, but API does not expose /platforms/:platform/environments/:environment/versions/:version routes.

Impact: Version-scoped invalidation is a no-op, leaving stale cache entries.

Fix:

// Replace with real endpoints that exist, e.g. configs/flags/experiments
paths.push(`/api/v1/platforms/${platform}/environments/${environment}/configs`);
paths.push(`/api/v1/platforms/${platform}/environments/${environment}/flags`);
paths.push(`/api/v1/platforms/${platform}/environments/${environment}/experiments`);

🟑 MEDIUM: Stats Cache Invalidation Targets Wrong Endpoints

File: packages/cache/src/providers/CloudflareCacheProvider.ts:189-196

Category: Bug / Performance

Description: invalidateStatsCache targets paths like /flags/stats and /experiments/stats, but actual stats endpoints are /flags/:flagKey/stats and /experiments/:experimentKey/stats (under /api/v1/internal).

Impact: Cache invalidation doesn't hit real stats URLs; stale stats served if cached.

Fix:

// Either accept explicit key and invalidate concrete URLs,
// or store a list of recently accessed stats URLs for purge-by-URL.
async invalidateFlagStatsCache(platform: string, environment: string, flagKey: string) {
  return this.invalidateCache([
    `/api/v1/internal/platforms/${platform}/environments/${environment}/flags/${flagKey}/stats`,
  ]);
}

🟒 LOW: Next.js Server Flag Context Missing UserID

File: packages/sdk-nextjs/src/server.ts:147-151

Category: Bug

Description: getFlags().isFlagEnabled() evaluates with context ?? { userId: '' }, violating "userId required" contract and producing deterministic hashes for all users.

Impact: Server renders give inconsistent targeting/rollout behavior vs client.

Fix:

const safeContext = context ?? { userId: 'anonymous' };
const result = evaluateFlag(flag, safeContext);

🟑 MEDIUM: Next.js Server Experiment Double-Counts Exposure

File: packages/sdk-nextjs/src/server.ts:186-196

Category: Bug / Data Integrity

Description: getExperiment calls client.getVariant, which tracks an exposure. In SSR/Server Components, can double-count if client also calls getVariant on hydration.

Impact: Inflated exposure counts and distorted conversion rates.

Fix:

// Avoid exposure tracking on server
const experiment = experiments.find((e) => e.experimentKey === experimentKey) || null
const variant = experiment ? assignVariation(experiment, context) : null

🟑 MEDIUM: Expo Storage Initialization Never Retries

File: packages/sdk-expo/src/storage.ts:36-57

Category: Bug / Reliability

Description: If MMKV initialization fails once, initPromise stays rejected and subsequent calls never retry (even after hot reload or dependency fix).

Impact: Storage can be permanently disabled for the app session.

Fix:

this.initPromise = (async () => {
  try {
    const { MMKV } = await import('react-native-mmkv')
    this.storage = new MMKV({ id: `togglebox-${this.platform}-${this.environment}` })
  } catch (error) {
    this.initPromise = null; // allow retry
    throw error;
  }
})()

🟒 LOW: Expo Provider setState After Unmount

File: packages/sdk-expo/src/provider.tsx:109-150

Category: Bug / React

Description: loadData() is async and may call setState after unmount. Effect cleanup only destroys client; no mounted guard.

Impact: "State update on unmounted component" warnings and potential memory leaks.

Fix:

useEffect(() => {
  let isMounted = true;
  const loadData = async () => {
    // ...
    if (isMounted) setConfig(configData);
    // ...
  };
  loadData();
  return () => {
    isMounted = false;
    client.destroy();
  };
}, [...])

Performance Issues

πŸ”΄ HIGH: Stats Repository Serial Processing

File:

  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:579-623
  • packages/database/src/adapters/prisma/PrismaStatsRepository.ts:512-555
  • packages/database/src/adapters/d1/D1StatsRepository.ts:512-540
  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:469-513

Category: Performance

Description: processBatch() loops with await inside for loop β†’ serial DB operations.

Impact: Batch ingestion becomes O(n) round-trips; degrades heavily under load.

Fix:

await Promise.all(events.map(event => {
  switch (event.type) {
    case 'config_fetch': return this.incrementConfigFetch(...);
    case 'flag_evaluation': return this.incrementFlagEvaluation(...);
    case 'experiment_exposure': return this.recordExperimentExposure(...);
    case 'conversion': return this.recordConversion(...);
  }
}));

πŸ”΄ HIGH: Experiment Results Analysis Too Slow

File: apps/api/src/controllers/experimentController.ts:690-707

Category: Bug / Performance

Description: Results analysis fetches per-variation stats one by one; only compares control vs first treatment.

Impact: Incomplete results for multi-variant experiments; slower analysis.

Fix:

const variationData = await Promise.all(stats.variations.map(async (v) => {
  const metricStats = await this.repos.stats.getExperimentMetricStats(...);
  const conversions = metricStats.reduce((s, m) => s + m.conversions, 0);
  return { variationKey: v.variationKey, participants: v.participants, conversions };
}));

const control = variationData.find(v => v.variationKey === experiment.controlVariation);
const treatments = variationData.filter(v => v.variationKey !== experiment.controlVariation);

const comparisons = control
  ? treatments.map(t => ({
      variationKey: t.variationKey,
      significance: calculateSignificance(control, t, experiment.confidenceLevel),
    }))
  : [];

πŸ”΄ HIGH: N+1 Fan-Out in Admin API Calls

File:

  • apps/admin/src/lib/api/flags.ts:8-40
  • apps/admin/src/lib/api/experiments.ts:8-41
  • apps/admin/src/lib/api/configs.ts:8-41

Category: Performance

Description: getAll*Api() performs N+1+M fan-out (platforms β†’ environments β†’ items) with no pagination or concurrency limits.

Impact: Large tenants heavy load; slow UI; API rate-limits/timeouts.

Fix:

// Option A: single backend endpoint
// GET /api/v1/internal/items?type=flags&limit=100&cursor=...

// Option B: concurrency limit
import pLimit from 'p-limit';
const limit = pLimit(5);
await Promise.all(platforms.map(p => limit(async () => { /* fetch envs/flags */ })));

πŸ”΄ HIGH: Dashboard Stats Heavy Fan-Out

File: apps/admin/src/lib/api/stats.ts:37-158

Category: Performance

Description: Dashboard stats uses heavy client-side fan-out with extensive console logging.

Impact: Slow loads on larger tenants; unnecessary API load; noisy logs.

Fix:

// Single backend endpoint
// GET /api/v1/internal/stats

// Or throttle with concurrency limit
import pLimit from 'p-limit';
const limit = pLimit(5);

🟑 HIGH: Cloudflare Purge Paths Instead of URLs

File: packages/cache/src/providers/CloudflareCacheProvider.ts:66-83

Category: Bug / Performance

Description: invalidateCache sends paths to Cloudflare instead of full URLs. API requires full URLs. Code comments note this but doesn't implement it.

Impact: Cloudflare purge calls fail; stale cache persists.

Fix:

private baseOrigin: string;

constructor(zoneId?: string, apiToken?: string, baseOrigin?: string) {
  this.zoneId = zoneId || '';
  this.apiToken = apiToken || '';
  this.baseOrigin = baseOrigin || '';
  this.enabled = !!(this.zoneId && this.apiToken && this.baseOrigin);
}

const urls = paths.map((path) => {
  const cleanPath = path.replace(/\/\*$/, '');
  return new URL(cleanPath, this.baseOrigin).toString();
});

🟑 HIGH: Cloudflare Stats Cache Invalidation Broken

File: packages/cache/src/providers/CloudflareCacheProvider.ts:169-173

Category: Bug

Description: invalidateStatsCache uses wildcard in middle of path (/*/stats), which Cloudflare doesn't support.

Impact: Stats cache invalidation fails; stale stats across environments.

Fix:

// Option A: purge everything for stats
return this.invalidateCache([
  `/api/v1/platforms/${platform}/environments/${environment}/stats`,
]);

// Option B: enumerate known endpoints
return this.invalidateCache([
  `/api/v1/platforms/${platform}/environments/${environment}/flags/stats`,
  `/api/v1/platforms/${platform}/environments/${environment}/experiments/stats`,
  `/api/v1/platforms/${platform}/environments/${environment}/configs/stats`,
]);

🟑 HIGH: Cloudflare Purge Batch Limit Exceeded

File: packages/cache/src/providers/CloudflareCacheProvider.ts:50-84

Category: Performance / Reliability

Description: Purge-by-URL has 30 URL limit; provider doesn't batch when paths.length > 30.

Impact: Large invalidations fail; stale cache entries remain.

Fix:

const batches = [];
for (let i = 0; i < urls.length; i += 30) batches.push(urls.slice(i, i + 30));

for (const batch of batches) {
  await fetch(`${this.baseUrl}/zones/${this.zoneId}/purge_cache`, {
    method: 'POST',
    headers: { Authorization: `Bearer ${this.apiToken}`, 'Content-Type': 'application/json' },
    body: JSON.stringify({ files: batch }),
  });
}

🟑 HIGH: CloudFront Invalidation Batch Limit

File: packages/cache/src/providers/CloudFrontCacheProvider.ts:47-66

Category: Performance / Reliability

Description: CloudFront invalidations limited to 1000 paths; no batching exists.

Impact: Large invalidations fail; stale data served.

Fix:

const chunks: string[][] = [];
for (let i = 0; i < paths.length; i += 1000) chunks.push(paths.slice(i, i + 1000));

const invalidationIds: string[] = [];
for (const chunk of chunks) {
  const params = {
    DistributionId: this.distributionId,
    InvalidationBatch: {
      Paths: { Quantity: chunk.length, Items: chunk },
      CallerReference: `invalidation-${Date.now()}-${Math.random().toString(36).slice(2)}`
    },
  };
  const result = await this.cloudfront.send(new CreateInvalidationCommand(params));
  if (result.Invalidation?.Id) invalidationIds.push(result.Invalidation.Id);
}
return invalidationIds[0] ?? null;

🟑 MEDIUM: DynamoDB Flag Delete Sequential

File: packages/database/src/adapters/dynamodb/DynamoDBNewFlagRepository.ts:331-340

Category: Performance

Description: Deleting all versions done sequentially.

Impact: Slow deletes for flags with many versions.

Fix:

// Use BatchWriteCommand with chunks of 25

🟑 MEDIUM: DynamoDB Experiment Delete Sequential

File: packages/database/src/adapters/dynamodb/DynamoDBExperimentRepository.ts:351-369

Category: Performance

Description: Deletes experiment versions sequentially.

Impact: Slow deletions; higher latency.

Fix:

// Batch delete with BatchWriteCommand (25 items per batch)

🟑 MEDIUM: DynamoDB Stats Unbounded Concurrent Deletes

File: packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:673-690

Category: Performance

Description: Deleting stats spawns unbounded concurrent DeleteCommands (one per item).

Impact: Throttling; timeouts under large datasets.

Fix:

// Chunk deletes into BatchWriteCommand batches of 25

🟑 MEDIUM: DynamoDB Scan for ListUsers

File: packages/auth/src/adapters/dynamodb/userService.ts:251-286

Category: Performance

Description: listUsers uses full table Scan then in-memory pagination.

Impact: O(n) scans; high latency and cost at scale; DoS susceptible.

Fix:

// Implement paginated Scan with LastEvaluatedKey or maintain a GSI
// Return DynamoDB paging instead of slicing in memory

🟑 MEDIUM: DynamoDB Platform Listing Uses Scan

File: packages/database/src/platformService.ts:79-85, 184-209

Category: Performance / Code Quality

Description: Docs claim GSI (GSI1PK = 'PLATFORM') for listing platforms, but createPlatform doesn't set GSI1PK; listPlatforms uses Scan.

Impact: Slow listing in larger datasets; inconsistent design.

Fix:

// When creating platform, include GSI1PK
Item: { PK: `PLATFORM#${platform.name}`, GSI1PK: 'PLATFORM', ...platformWithId }

// List with Query on GSI1 instead of Scan

🟑 MEDIUM: List Endpoints Unbounded Without Pagination

File:

  • packages/database/src/adapters/prisma/PrismaPlatformRepository.ts:87-108
  • packages/database/src/adapters/prisma/PrismaEnvironmentRepository.ts:115-131
  • packages/database/src/adapters/mongoose/MongoosePlatformRepository.ts:87-101
  • packages/database/src/adapters/d1/D1PlatformRepository.ts:102-118

Category: Performance

Description: If pagination params omitted, these list methods fetch all rows and also do count() query.

Impact: Unbounded memory; slow responses; easy to overload.

Fix:

if (!pagination) {
  const limit = 100; // hard cap
  const items = await this.prisma.platform.findMany({ take: limit, orderBy: { createdAt: 'desc' } });
  return { items, total };
}

🟑 MEDIUM: Polling with No In-Flight Guard

File: packages/sdk-js/src/client.ts:525-536

Category: Performance / Bug

Description: Polling uses setInterval with async refresh() and no in-flight guard. Slow refreshes overlap.

Impact: Concurrent refreshes stampede API; race conditions; memory/network bloat.

Fix:

private isRefreshing = false;

private startPolling(): void {
  if (this.pollingTimer) return;

  this.pollingTimer = setInterval(async () => {
    if (this.isRefreshing) return;
    this.isRefreshing = true;
    try {
      await this.refresh();
    } catch (error) {
      this.emit('error', error);
    } finally {
      this.isRefreshing = false;
    }
  }, this.pollingInterval);
}

🟑 MEDIUM: Stats Flush Race Conditions

File: packages/sdk-js/src/stats.ts:117-148, 170-200

Category: Bug / Performance

Description: StatsReporter has no in-flight guard. setInterval and manual flushes overlap; re-queueing on any error.

Impact: Duplicate sends; out-of-order retries; poison events.

Fix:

private isFlushing = false;

async flush(): Promise<void> {
  if (this.queue.length === 0 || this.isFlushing) return;
  this.isFlushing = true;
  const events = [...this.queue];
  this.queue = [];
  try {
    await this.sendWithRetry(events);
    this.onFlush?.({ eventCount: events.length });
  } catch (error) {
    this.queue = [...events, ...this.queue];
    this.onError?.(error);
  } finally {
    this.isFlushing = false;
  }
}

🟑 MEDIUM: Stats Flush Timeout Missing

File: packages/stats/src/collector.ts:238-258

Category: Performance / Reliability

Description: sendWithRetry uses fetch without timeout. Hung request leaves isFlushing stuck true.

Impact: Stats queue stalls indefinitely; metrics lost.

Fix:

const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), this.options.requestTimeoutMs ?? 10000);

const response = await fetch(url, {
  method: 'POST',
  headers,
  body: JSON.stringify(batch),
  signal: controller.signal,
});

clearTimeout(timeout);

🟑 MEDIUM: Concurrent Stats Processing Unbounded

File: Multiple stats repositories

Category: Performance

Description: processBatch fires unbounded concurrent DB operations (up to 1000).

Impact: Saturated DB connections; timeouts; cascading retries.

Fix:

import pLimit from 'p-limit';

const limit = pLimit(25); // tune per DB
await Promise.all(events.map((event) => limit(() => processEvent(event))));

🟑 MEDIUM: Admin User List Unbounded

File: apps/admin/src/app/(dashboard)/users/page.tsx:15-30

Category: Performance

Description: User list loads all users at once with no pagination.

Impact: Slow page loads; large responses.

Fix:

// Add pagination on API and UI
const { users, nextCursor } = await getUsersApi({ limit: 50, cursor });

Authentication & Authorization

🟑 MEDIUM: JWT Verification Logs Sensitive Data

File: packages/auth/src/utils/jwt.ts:189-214

Category: Security / Code Quality

Description: JWT verification failure logs token preview and decoded claims (unverified).

Impact: Sensitive data (user IDs, claims) leak into logs in production.

Fix:

if (process.env.NODE_ENV === 'development') {
  console.error('[JWT] Verification failed:', errorMessage);
  // optional debug logs
}

SDK Issues

πŸ”΄ HIGH: Laravel Blade Directive XSS Vulnerability

File: packages/sdk-laravel/src/ToggleBoxServiceProvider.php:100-105

Category: Security

Description: Blade directive @variant echoes raw variant values without HTML escaping. Stored XSS possible if experiment values contain HTML/JS.

Impact: Stored XSS if admin input or API returns malicious content.

Fix:

// Before
echo $_togglebox_v?->value ?? '';

// After (escape output)
echo e($_togglebox_v?->value ?? '');

πŸ”΄ CRITICAL: PHP SDK Stats Payload Shape Wrong

File: packages/sdk-php/src/ToggleBoxClient.php:367-376, 579-585

Category: Bug

Description: Stats payload shape doesn't match API (eventType + nested data instead of flattened). API validates against StatsEventSchema.

Impact: Most SDK events rejected; no stats/experiments recorded.

Fix:

private function queueEvent(string $type, array $data): void {
    $this->pendingEvents[] = array_merge(
        ['type' => $type, 'timestamp' => date('c')],
        $data
    );
}

public function flushStats(): void {
    if (empty($this->pendingEvents)) return;
    $path = "/api/v1/platforms/{$this->platform}/environments/{$this->environment}/stats/events";
    $this->http->post($path, ['events' => $this->pendingEvents]);
    $this->pendingEvents = [];
}

πŸ”΄ CRITICAL: PHP SDK Missing Experiment Schedule Support

File: packages/sdk-php/src/Types/Experiment.php:9-48 + packages/sdk-php/src/ToggleBoxClient.php:490-576

Category: Bug

Description: SDK ignores scheduledStartAt/scheduledEndAt (fields not mapped). TS evaluator blocks pre-start/post-end assignments.

Impact: PHP clients serve variants outside intended schedule.

Fix:

// packages/sdk-php/src/Types/Experiment.php
public readonly ?string $scheduledStartAt;
public readonly ?string $scheduledEndAt;
// in fromArray:
scheduledStartAt: $data['scheduledStartAt'] ?? null,
scheduledEndAt: $data['scheduledEndAt'] ?? null,

// packages/sdk-php/src/ToggleBoxClient.php (assignVariation)
$now = new \DateTimeImmutable();
if ($experiment->scheduledStartAt && new \DateTimeImmutable($experiment->scheduledStartAt) > $now) {
    return null;
}
if ($experiment->scheduledEndAt && new \DateTimeImmutable($experiment->scheduledEndAt) < $now) {
    return null;
}

πŸ”΄ HIGH: Conversion Tracking Double-Counts Exposure

File: packages/sdk-js/src/client.ts:295-361 + packages/sdk-php/src/ToggleBoxClient.php:260-286

Category: Bug

Description: trackConversion() calls getVariant(), which always logs exposure. If app already called getVariant() for rendering, every conversion logs a second exposure.

Impact: Exposure stats inflated; SRM and conversion rate calculations inaccurate.

Fix:

// JS SDK
private async getVariantInternal(
  experimentKey: string,
  context: ExperimentContext,
  trackExposure: boolean
): Promise<VariantAssignment | null> {
  // ...fetch experiment & assign...
  if (assignment && trackExposure) {
    this.stats.trackExperimentExposure(experimentKey, assignment.variationKey, context.userId)
  }
  return assignment
}

async getVariant(...) { return this.getVariantInternal(experimentKey, context, true) }

async trackConversion(...) {
  const assignment = await this.getVariantInternal(experimentKey, context, false)
  if (assignment) { /* track conversion */ }
}

// PHP SDK - similar pattern
private function getVariantInternal(string $experimentKey, ExperimentContext $context, bool $trackExposure): ?VariantAssignment { ... }
public function getVariant(...) { return $this->getVariantInternal(..., true); }
public function trackConversion(...) { $assignment = $this->getVariantInternal(..., false); ... }

🟑 HIGH: PHP SDK Flag Evaluation Ignores Targeting

File: packages/sdk-php/src/Types/Flag.php:13-45 + packages/sdk-php/src/ToggleBoxClient.php:478-495

Category: Bug / Data Integrity

Description: PHP Flag has forceIncludeUsers/forceExcludeUsers as top-level fields, but API sends them under targeting. Evaluation uses top-level, so force include/exclude never apply.

Impact: Forced cohorts ignored.

Fix:

// Remove top-level usage
$excludeUsers = $flag->targeting['forceExcludeUsers'] ?? [];
$includeUsers = $flag->targeting['forceIncludeUsers'] ?? [];

🟑 HIGH: PHP SDK Flag Evaluation Missing Schedule Check

File: packages/sdk-php/src/ToggleBoxClient.php:465-571

Category: Bug

Description: No schedule enforcement in PHP flag evaluation.

Impact: Flags served outside configured schedule window.

Fix:

// Add scheduledStartAt/scheduledEndAt checks

🟑 MEDIUM: PHP Stats Flag Evaluation Wrong Key

File: packages/sdk-php/src/ToggleBoxClient.php:152-169

Category: Bug / Analytics

Description: flag_evaluation events send servedValue but API schema expects value. Also language not included.

Impact: Flag evaluation stats dropped; language breakdown missing.

Fix:

$this->queueEvent('flag_evaluation', [
    'flagKey' => $flagKey,
    'value' => $result->servedValue, // use 'value' key
    'userId' => $context->userId,
    'country' => $context->country,
    'language' => $context->language,
]);

🟑 MEDIUM: PHP SDK Pending Events No Limit/Flush

File: packages/sdk-php/src/ToggleBoxClient.php:39, 395-402, 632-640

Category: Performance / Reliability

Description: pendingEvents has no max size and no periodic flush. Stalled API causes unbounded queue.

Impact: Memory growth; stats never sent.

Fix:

private int $maxQueueSize = 1000;

private function queueEvent(string $eventType, array $data): void
{
    if (count($this->pendingEvents) >= $this->maxQueueSize) {
        array_shift($this->pendingEvents); // drop oldest
    }
    $this->pendingEvents[] = array_merge(
        ['type' => $eventType, 'timestamp' => date('c')],
        $data
    );
}

🟑 MEDIUM: PHP SDK Stats Options Never Used

File: packages/sdk-php/src/Types/ClientOptions.php:9-17 + packages/sdk-php/src/ToggleBoxClient.php:44-73

Category: Code Quality / Bug

Description: StatsOptions exists but never used; batch size, flush interval, max retries ignored.

Impact: Users configure stats behavior that doesn't apply.

Fix:

// ToggleBoxClient: add fields
private int $statsBatchSize;
private int $statsMaxRetries;

// in constructor
$this->statsBatchSize = $options->stats?->batchSize ?? 20;
$this->statsMaxRetries = $options->stats?->maxRetries ?? 3;

// in queueEvent - auto flush when batch full
if (count($this->pendingEvents) >= $this->statsBatchSize) {
    $this->flushStats();
}

// in flushStats - add retry logic
for ($attempt = 0; $attempt < $this->statsMaxRetries; $attempt++) {
    try {
        $this->http->post($path, ['events' => $this->pendingEvents]);
        $this->pendingEvents = [];
        break;
    } catch (ToggleBoxException $e) {
        if ($attempt === $this->statsMaxRetries - 1) { throw $e; }
        usleep(1000 * (2 ** $attempt) * 1000);
    }
}

🟑 MEDIUM: PHP SDK Experiment Targeting Logic Diverges

File: packages/sdk-php/src/ToggleBoxClient.php:586-616, 594-604

Category: Bug / Logic

Description: Experiment targeting logic has multiple divergences from core evaluator:

  • If countries configured and context.country missing, PHP SDK still assigns variation (should exclude)
  • If country has languages and context.language missing, PHP SDK still allows assignment (should exclude)
  • forceIncludeUsers always assigns control variation instead of including user in normal allocation

Impact: Users assigned when should be excluded; traffic allocation biased; experiment results skewed.

Fix:

// If countries defined and no country in context -> exclude
if (!empty($countries) && $context->country === null) {
    return null;
}

foreach ($countries as $countryTarget) {
    if (strtoupper($countryTarget['country']) === strtoupper($context->country)) {
        $languages = $countryTarget['languages'] ?? [];
        if (!empty($languages) && $context->language === null) {
            return null;
        }
        if (!empty($languages)) {
            $matchedLanguage = false;
            foreach ($languages as $langTarget) {
                if (strtolower($langTarget['language']) === strtolower($context->language)) {
                    $matchedLanguage = true;
                    break;
                }
            }
            if (!$matchedLanguage) return null;
        }
        break;
    }
}

// forceIncludeUsers should only include the user, not force control
if (in_array($context->userId, $includeUsers, true)) {
    // continue to normal hash allocation instead of returning control
}

🟑 MEDIUM: PHP SDK Flag Missing Country Not Targeted

File: packages/sdk-php/src/ToggleBoxClient.php:498-548

Category: Bug / Logic

Description: When flag targeting exists and context.country missing, PHP SDK falls through to rollout instead of defaulting/excluding like core evaluator.

Impact: Users without country data get rollout assignments when should receive defaultValue.

Fix:

if ($flag->targeting !== null) {
    $countries = $flag->targeting['countries'] ?? [];
    if (!empty($countries) && $context->country === null) {
        $served = $flag->defaultValue ?? 'B';
        return new FlagResult($flag->flagKey, $getValue($served), $served, 'country_not_targeted');
    }
    // existing country/language checks...
}

🟑 MEDIUM: PHP SDK Conversion Double-Exposure

File: packages/sdk-php/src/ToggleBoxClient.php:248-299

Category: Bug / Analytics

Description: trackConversion() calls getVariant(), which queues an exposure. Every conversion creates a second exposure.

Impact: Exposure stats inflated; conversion rates inaccurate.

Fix:

// Option A: add no-track variant lookup
public function getVariantNoTrack(string $experimentKey, ExperimentContext $context): ?VariantAssignment {
    $experiments = $this->getExperiments();
    foreach ($experiments as $exp) {
        if ($exp->experimentKey === $experimentKey) {
            return $this->assignVariation($exp, $context);
        }
    }
    throw new ToggleBoxException("Experiment \"{$experimentKey}\" not found");
}

public function trackConversion(...): void {
    $assignment = $this->getVariantNoTrack($experimentKey, $context);
    if ($assignment !== null) {
        $this->queueEvent('conversion', [ /* ... */ ]);
    }
}

🟑 MEDIUM: PHP SDK Cache Options Ignored

File: packages/sdk-php/src/Types/ClientOptions.php:9-17 + packages/sdk-php/src/ToggleBoxClient.php:109-199

Category: Performance / Code Quality

Description: CacheOptions->enabled never checked; caching can't be disabled.

Impact: Unexpected caching in short-lived PHP requests/tests.

Fix:

// in ToggleBoxClient getConfig/getFlags/getExperiments
if ($this->options->cache?->enabled !== false) {
  $cached = $this->cache->get($cacheKey);
  if ($cached !== null) return $cached;
}

🟑 MEDIUM: Laravel Locale Parsing Wrong

File: packages/sdk-laravel/src/ToggleBoxManager.php:268-289

Category: Bug

Description: app()->getLocale() can return values like en_US, but targeting expects 2-letter codes.

Impact: Country/language targeting mismatches.

Fix:

$locale = $language ?? app()->getLocale();
$language = strlen($locale) > 2 ? substr($locale, 0, 2) : $locale;

🟒 LOW: Expo SDK Background Refresh Errors Not Surfaced

File: packages/sdk-expo/src/provider.tsx:121-123

Category: Bug / UX

Description: Background refresh errors are only logged, not surfaced via setError. Stale data remains silently when refresh fails.

Impact: Users unaware of sync failures; stale configurations served without indication.

Fix:

client.refresh()
  .then(() => setError(null))
  .catch((err) => setError(err as Error));

🟑 MEDIUM: Expo SDK Unhandled Promise Rejection

File: packages/sdk-expo/src/provider.tsx:82-96

Category: Bug

Description: Async client.on('update') handler awaits storage save without try/catch. If save rejects, triggers unhandled promise rejection.

Impact: App-level crash/console noise; lost updates in React Native.

Fix:

client.on('update', (data) => {
  try {
    const updateData = data as { config: Config; flags: Flag[]; experiments: Experiment[] };
    setConfig(updateData.config);
    setFlags(updateData.flags);
    setExperiments(updateData.experiments);
    if (persistToStorage && storageRef.current) {
      void storageRef.current.save(updateData.config, updateData.flags, updateData.experiments);
    }
  } catch (err) {
    setError(err as Error);
  }
});

🟑 MEDIUM: JS Stats Queue Unbounded

File: packages/sdk-js/src/stats.ts:117-148

Category: Performance

Description: Event queue grows without cap if network is down.

Impact: Memory growth; possible crashes in long-running sessions.

Fix:

if (this.queue.length >= (this.options.maxQueueSize ?? 1000)) {
  this.queue.shift(); // drop oldest
}
this.queue.push(event);

🟑 MEDIUM: Collector Queue Unbounded

File: packages/stats/src/collector.ts:181-214

Category: Performance

Description: StatsCollector has unbounded queue growth on offline clients.

Impact: Memory leak; performance degradation.

Fix:

// Add maxQueueSize to CollectorOptions and enforce it
if (this.queue.length >= (this.options.maxQueueSize ?? 1000)) {
  this.queue.shift();
}
this.queue.push(event);

🟑 MEDIUM: Poison Batch Loop in Stats Collector

File: packages/stats/src/collector.ts:195-213, 246-249

Category: Bug / Performance

Description: Any 4xx response throws and batch re-queued. Creates poison batch loop: invalid payloads retry forever.

Impact: Memory growth; repeated failing calls; client stall.

Fix:

try {
  await this.sendWithRetry(batch);
} catch (error) {
  const isClientError = error instanceof Error && error.message.startsWith('Client error:');
  if (!isClientError) {
    this.queue.unshift(...events); // only requeue for transient errors
  } else {
    console.error('[ToggleBox Stats] Dropping invalid batch:', error);
  }
}

🟑 MEDIUM: JS SDK Flag Evaluation Missing Language

File: packages/sdk-js/src/client.ts:213-218

Category: Bug / Analytics

Description: trackFlagEvaluation drops language parameter.

Impact: Language-specific analysis inaccurate/missing.

Fix:

this.stats.trackFlagEvaluation(
  flagKey,
  result.servedValue,
  context.userId ?? 'anonymous',
  context.country,
  context.language
);

🟑 MEDIUM: JS SDK trackEvent Drops Custom Events

File: packages/sdk-js/src/client.ts:380-395

Category: Bug / Analytics

Description: trackEvent drops custom events unless both experimentKey and variationKey provided; never uses StatsCollector.trackCustomEvent.

Impact: General custom events silently lost.

Fix:

trackEvent(eventName: string, context: ExperimentContext, data?: EventData): void {
  this.stats.trackCustomEvent(eventName, context.userId, data?.properties);

  if (data?.experimentKey && data?.variationKey) {
    this.stats.trackConversion(
      data.experimentKey,
      eventName,
      data.variationKey,
      context.userId
    );
  }
}

🟑 MEDIUM: HttpClient No Request Timeout

File: packages/sdk-js/src/http.ts:45-114

Category: Performance / Reliability

Description: HttpClient has no request timeout; hung request stalls indefinitely.

Impact: UI/server requests hang; retry logic doesn't trigger.

Fix:

private async fetchWithTimeout(url: string, init: RequestInit, timeoutMs = 10000) {
  const controller = new AbortController();
  const id = setTimeout(() => controller.abort(), timeoutMs);
  try {
    return await this.fetchImpl(url, { ...init, signal: controller.signal });
  } finally {
    clearTimeout(id);
  }
}

// use in get/post
const response = await this.fetchWithTimeout(url, { method: 'GET', headers: this.getHeaders() });

🟑 MEDIUM: Next.js Provider Loading State Bug

File: packages/sdk-nextjs/src/provider.tsx:39-85

Category: Bug / React

Description: isLoading only considers initialConfig, so if config provided but flags/experiments not, isLoading false while fetches in flight.

Impact: UI renders as "loaded" with incomplete data; flicker.

Fix:

const initialNeedsFetch =
  !initialConfig || !initialFlags?.length || !initialExperiments?.length;

const [isLoading, setIsLoading] = useState(initialNeedsFetch);

// In useEffect, before fetch:
if (needsFetch) setIsLoading(true);

🟑 MEDIUM: Next.js Server getAnalytics Memory Leak

File: packages/sdk-nextjs/src/server.ts:272-285

Category: Performance / Bug

Description: getAnalytics only destroys client when flushStats() called. If never flushed, client and stats queue remain alive.

Impact: Memory/queue growth in long-lived server processes; unflushed events.

Fix:

export async function getAnalytics(options: ServerOptions): Promise<ServerAnalyticsResult & { close: () => void }> {
  const client = createServerClient(options);

  return {
    trackEvent: (eventName, context, data) => client.trackEvent(eventName, context, data),
    trackConversion: async (experimentKey, context, data) => {
      await client.trackConversion(experimentKey, context, data);
    },
    flushStats: async () => {
      try {
        await client.flushStats();
      } finally {
        client.destroy();
      }
    },
    close: () => client.destroy(),
  };
}

🟒 LOW: PHP SDK Null Access Warning

File: packages/sdk-php/src/ToggleBoxClient.php:333-340

Category: Bug

Description: trackEvent accesses $data['experimentKey'] even when $data is null.

Impact: PHP warnings; potential strict error handling failures.

Fix:

$experimentKey = is_array($data) ? ($data['experimentKey'] ?? null) : null;
$variationKey  = is_array($data) ? ($data['variationKey'] ?? null) : null;
$properties    = is_array($data) ? ($data['properties'] ?? []) : [];

🟒 LOW: Example App Env Var Mismatch

File:

  • apps/example-nextjs/src/app/layout.tsx:8-11
  • apps/example-nextjs/src/app/providers.tsx:6-9
  • apps/example-nextjs/README.md:73-76

Category: Bug / Code Quality

Description: Env var names in Next.js example don't match README. Users following README get undefined API URL/key/platform/env at runtime.

Impact: Production config broken for new users.

Fix:

// apps/example-nextjs/src/app/layout.tsx
const API_URL = process.env.NEXT_PUBLIC_TOGGLEBOX_API_URL || 'http://localhost:3000/api/v1'
const API_KEY = process.env.NEXT_PUBLIC_TOGGLEBOX_API_KEY
const PLATFORM = process.env.NEXT_PUBLIC_TOGGLEBOX_PLATFORM || 'web'
const ENVIRONMENT = process.env.NEXT_PUBLIC_TOGGLEBOX_ENVIRONMENT || 'staging'

// apps/example-nextjs/src/app/providers.tsx
const API_URL = process.env.NEXT_PUBLIC_TOGGLEBOX_API_URL || 'http://localhost:3000/api/v1'
const API_KEY = process.env.NEXT_PUBLIC_TOGGLEBOX_API_KEY
const PLATFORM = process.env.NEXT_PUBLIC_TOGGLEBOX_PLATFORM || 'web'
const ENVIRONMENT = process.env.NEXT_PUBLIC_TOGGLEBOX_ENVIRONMENT || 'staging'

🟒 LOW: Example App Exposes API Keys Client-Side

File:

  • apps/example-nextjs/src/app/providers.tsx:6-40
  • apps/example-nextjs/src/app/layout.tsx:8-28
  • apps/example-expo/app/_layout.tsx:4-20

Category: Security

Description: API keys read from NEXT_PUBLIC_* / EXPO_PUBLIC_*, embedded in client bundles. If privileged, exposed to anyone.

Impact: Unauthorized API access if key is privileged.

Fix:

// Server-only (Next.js server component or server action)
const API_KEY = process.env.TOGGLEBOX_API_KEY

// Client provider: avoid secret keys
<ToggleBoxProvider
  platform={PLATFORM}
  environment={ENVIRONMENT}
  apiUrl={API_URL}
  // apiKey omitted or use strictly public/anonymous key
/>

Analytics & Stats

πŸ”΄ HIGH: DynamoDB Conversion Recording Broken

File: packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:456-503

Category: Bug / Analytics

Description: recordConversion builds an invalid DynamoDB UpdateExpression when value is provided. It appends a second ADD section after SET, which causes the update to fail.

Impact: Any metric with a value (e.g., revenue) fails to record conversions, breaking experiment analytics.

Fix:

// Build a single ADD section
let updateExpression = 'ADD conversions :inc, sampleSize :inc';
const expressionAttributeValues: Record<string, unknown> = {
  ':inc': 1,
  ':now': now,
  ':expKey': experimentKey,
  ':varKey': variationKey,
  ':metricId': metricId,
};
if (value !== undefined) {
  updateExpression += ', sumValue :value';
  expressionAttributeValues[':value'] = value;
}

// Then SET
updateExpression += ' SET lastConversionAt = :now, experimentKey = :expKey, variationKey = :varKey, metricId = :metricId';

🟑 MEDIUM: Experiment Metric Stats Count Wrong

File:

  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:472-486
  • packages/database/src/adapters/d1/D1StatsRepository.ts:509-542

Category: Bug / Analytics

Description: getExperimentMetricStats returns count incorrectly (conversions for Mongoose, 0 for D1).

Impact: Count/average metrics computed from results are wrong or zero, invalidating experiment reports.

Fix:

// Mongoose: include count if stored
count: doc.count ?? 0,

// D1: return actual count column
count: row.count,

// Ensure recordConversion updates count and sampleSize for daily stats (see prior fix pattern)

🟑 MEDIUM: Mongoose Stats Missing Count Field

File:

  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:367-385
  • packages/database/src/adapters/mongoose/schemas.ts:386-393

Category: Bug / Analytics

Description: Mongo stats don't store or increment count for experiment metrics. Schema has no count field; recordConversion doesn't update it.

Impact: Count/average metrics derived from count are incorrect or always zero.

Fix:

// schemas.ts: add count
count: { type: Number, required: false },

// MongooseStatsRepository.recordConversion: increment count
const incFields: Record<string, number> = { conversions: 1, sampleSize: 1, count: 1 };
if (value !== undefined) incFields.sumValue = value;

🟑 MEDIUM: D1 Stats Never Writes Count

File:

  • packages/database/src/adapters/d1/D1StatsRepository.ts:414-439
  • packages/database/src/adapters/d1/D1StatsRepository.ts:520-544

Category: Bug / Analytics

Description: D1 adapter never writes count; getExperimentMetricStats returns count: 0. Count/average metrics wrong.

Impact: Count/average metrics incorrect for D1 deployments.

Fix:

// When updating/inserting experiment_metric_stats + _daily:
SET conversions = conversions + 1, sampleSize = sampleSize + 1, count = count + 1, ...
INSERT ... (conversions, sampleSize, count, sumValue) VALUES (1, 1, 1, ?)

// In getExperimentMetricStats:
count: row.count

🟑 MEDIUM: DynamoDB Stats Missing Count Increment

File:

  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:461-509
  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:564-576

Category: Bug / Analytics

Description: DynamoDB adapter doesn't increment/store count for metrics. getExperimentMetricStats returns count but it's never set.

Impact: Count/average metrics incorrect or zero in DynamoDB deployments.

Fix:

// recordConversion: add count to ADD section
let addExpression = 'ADD conversions :inc, sampleSize :inc, #count :inc';
ExpressionAttributeNames: { '#count': 'count', ... };

let dailyAddExpression = 'ADD conversions :inc, sampleSize :inc, #count :inc';
ExpressionAttributeNames: { '#date': 'date', '#count': 'count' };

🟑 MEDIUM: Custom Events Never Persisted

File:

  • packages/database/src/adapters/prisma/PrismaStatsRepository.ts:604-608
  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:559-561
  • packages/database/src/adapters/d1/D1StatsRepository.ts:602-604
  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:669-671

Category: Bug / Analytics

Description: custom_event is accepted but explicitly not persisted (only logged). No storage implementation.

Impact: trackEvent appears to work, but no custom event analytics can be queried later.

Fix:

// Add storage for custom events (table/collection)
// Then implement recordCustomEvent(...) and call it here
case 'custom_event':
  await this.recordCustomEvent(platform, environment, event.eventName, event.userId, event.properties);
  break;

🟑 MEDIUM: PHP trackEvent Loses Experiment Context

File:

  • packages/sdk-php/src/ToggleBoxClient.php:401-416
  • packages/sdk-php/README.md:106-114
  • packages/stats/src/types.ts:233-240

Category: Bug / Analytics

Description: PHP trackEvent advertises experimentKey/variationKey, but only queues a custom_event and API schema drops those fields. Experiment-scoped events lost.

Impact: Users think events are tied to experiments but they aren't; experiment event tracking doesn't work.

Fix:

public function trackEvent(string $eventName, ExperimentContext $context, ?array $data = null): void
{
    $experimentKey = is_array($data) ? ($data['experimentKey'] ?? null) : null;
    $variationKey = is_array($data) ? ($data['variationKey'] ?? null) : null;
    $properties = is_array($data) ? ($data['properties'] ?? []) : [];

    // Always send custom_event
    $this->queueEvent('custom_event', [
        'eventName' => $eventName,
        'userId' => $context->userId,
        'properties' => $properties,
    ]);

    // Also track conversion if experiment context provided (parity with JS)
    if ($experimentKey && $variationKey) {
        $this->queueEvent('conversion', [
            'experimentKey' => $experimentKey,
            'metricName' => $eventName,
            'variationKey' => $variationKey,
            'userId' => $context->userId,
        ]);
    }
}

Also update docs to match behavior.


🟑 HIGH: SRM Expected Ratios Ignore Traffic Allocation

File: apps/api/src/controllers/statsController.ts:251-293

Category: Bug / Analytics

Description: SRM expected ratios default to equal split, ignoring experiment's configured trafficAllocation.

Impact: SRM warnings false positives/misses for uneven traffic.

Fix:

const experiment = await this.repos.experiment.get(platform, environment, experimentKey);

if (expectedRatiosParam) {
  expectedRatios = expectedRatiosParam.split(',').map((r) => parseFloat(r.trim()));
} else if (experiment?.trafficAllocation?.length) {
  expectedRatios = experiment.trafficAllocation.map((t) => t.percentage / 100);
} else {
  const equalRatio = 1 / stats.variations.length;
  expectedRatios = stats.variations.map(() => equalRatio);
}

🟑 MEDIUM: Custom Events Never Stored

File:

  • packages/database/src/adapters/prisma/PrismaStatsRepository.ts:524-561
  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:500-515
  • packages/database/src/adapters/d1/D1StatsRepository.ts:521-558
  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:588-625

Category: Bug / Analytics

Description: processBatch never handles custom_event type (defined in schemas). Events accepted but silently dropped.

Impact: trackCustomEvent produces zero analytics.

Fix:

case 'custom_event':
  await this.recordCustomEvent(
    platform,
    environment,
    event.eventName,
    event.userId,
    event.properties
  );
  break;

// Also add recordCustomEvent implementation + storage schema/table

🟑 MEDIUM: Experiment Stats Metric Results Empty

File:

  • packages/database/src/adapters/prisma/PrismaStatsRepository.ts:434-466
  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:394-426
  • packages/database/src/adapters/d1/D1StatsRepository.ts:432-463
  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:469-526
  • apps/api/src/controllers/statsController.ts:255-320

Category: Bug / Analytics

Description: getExperimentStats returns metricResults and dailyData as empty arrays; API doesn't populate them.

Impact: Experiment analytics endpoints never return metric time series or aggregate stats.

Fix:

// apps/api/src/controllers/statsController.ts (after stats fetched)
const experiment = await this.repos.experiment.get(platform, environment, experimentKey);
const metricIds = [
  experiment?.primaryMetric?.id,
  ...(experiment?.secondaryMetrics?.map(m => m.id) ?? []),
].filter(Boolean) as string[];

const metricResults = await Promise.all(
  stats.variations.flatMap(v =>
    metricIds.map(id =>
      this.repos.stats.getExperimentMetricStats(platform, environment, experimentKey, v.variationKey, id)
    )
  )
).then((lists) => lists.flat());

res.json({
  success: true,
  data: { ...stats, metricResults },
  timestamp: new Date().toISOString(),
});

🟑 MEDIUM: Daily Metric Stats Missing sampleSize/count

File:

  • packages/database/src/adapters/mongoose/MongooseStatsRepository.ts:399-427
  • packages/database/src/adapters/d1/D1StatsRepository.ts:441-465
  • packages/database/src/adapters/dynamodb/DynamoDBStatsRepository.ts:483-502

Category: Bug / Analytics

Description: Daily metric stats updates increment conversions but never sampleSize/count. Time-series analytics expect these.

Impact: Averages and variance computed from daily data are incorrect.

Fix (pattern for all adapters):

// Mongoose daily
const dailyIncFields: Record<string, number> = {
  conversions: 1,
  sampleSize: 1,
  count: 1,
};
if (value !== undefined) dailyIncFields.sumValue = value;

// D1 daily UPDATE/INSERT
UPDATE experiment_metric_stats_daily
SET conversions = conversions + 1, sampleSize = sampleSize + 1, count = count + 1, ...

INSERT INTO experiment_metric_stats_daily (..., conversions, sampleSize, count, sumValue) VALUES (..., 1, 1, 1, ?)

// DynamoDB daily UpdateExpression
let dailyUpdateExpression = 'ADD conversions :inc, sampleSize :inc, #count :inc SET #date = :date';
ExpressionAttributeNames: { '#date': 'date', '#count': 'count' };

🟒 LOW: Stats Event Batch Limits Inconsistent

File: packages/stats/src/types.ts:262-268 + apps/api/src/controllers/statsController.ts:12-16

Category: Code Quality / Consistency

Description: StatsEventBatchSchema caps at 100, but API BatchEventsSchema allows 1000.

Impact: Confusing limits across SDKs vs server; client validation may reject valid batches.

Fix:

// Align limits (pick one)
const BatchEventsSchema = z.object({
  events: z.array(StatsEventSchema).min(1).max(100),
});
// or raise StatsEventBatchSchema max to 1000

Database & Adapters

🟑 HIGH: Prisma/Mongoose/D1 JSON.parse Unguarded

File:

  • packages/database/src/adapters/prisma/PrismaFlagRepository.ts:424-426
  • packages/database/src/adapters/d1/D1FlagRepository.ts:451-453
  • packages/database/src/adapters/mongoose/MongooseFlagRepository.ts:348-354

Category: Bug

Description: JSON.parse on stored valueA, valueB, or targeting without guard. Malformed data crashes flag listing/evaluation.

Impact: Any bad row crashes reads and breaks endpoints.

Fix:

const safeParse = <T>(raw: string, fallback: T): T => {
  try { return JSON.parse(raw) as T; } catch { return fallback; }
};

valueA: safeParse(row.valueA, null),
valueB: safeParse(row.valueB, null),
targeting: safeParse(row.targeting, { countries: [], forceIncludeUsers: [], forceExcludeUsers: [] }),

🟑 HIGH: Experiment Repositories Unguarded JSON.parse

File:

  • packages/database/src/adapters/prisma/PrismaExperimentRepository.ts:537-546
  • packages/database/src/adapters/mongoose/MongooseExperimentRepository.ts:469-478
  • packages/database/src/adapters/d1/D1ExperimentRepository.ts:606-615

Category: Bug / Reliability

Description: JSON.parse stored fields (e.g., secondaryMetrics) without guard. Bad row crashes list/detail endpoints.

Impact: Single malformed row breaks reads.

Fix:

function safeParse<T>(raw: string | null | undefined, fallback: T): T {
  if (!raw) return fallback;
  try { return JSON.parse(raw) as T; } catch { return fallback; }
}

// usage
secondaryMetrics: safeParse(row.secondaryMetrics, []),
results: safeParse(row.results, undefined),

🟒 LOW: DynamoDB Pagination Cursor Unvalidated

File:

  • packages/database/src/adapters/dynamodb/DynamoDBExperimentRepository.ts:289-304
  • packages/database/src/adapters/dynamodb/DynamoDBNewFlagRepository.ts:288-289

Category: Bug / Reliability

Description: Pagination cursor decoded with JSON.parse without validation. Invalid cursors throw and return 500.

Impact: Clients can break list endpoints with bad cursor.

Fix:

let exclusiveStartKey: Record<string, unknown> | undefined;
if (cursor) {
  try {
    exclusiveStartKey = JSON.parse(Buffer.from(cursor, 'base64').toString('utf-8'));
  } catch {
    throw new Error('Invalid pagination token');
  }
}

UI/Admin Panel

πŸ”΄ HIGH: Flag Targeting serveValue Always 'A'

File:

  • apps/admin/src/lib/api/flags.ts:109-118
  • apps/admin/src/app/(dashboard)/flags/create/page.tsx:276-303
  • apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/flags/[flagKey]/edit/page.tsx:214-223, 355-388

Category: Bug

Description: Flag targeting serveValue always set to 'A' in admin client; dropped entirely in create/edit payloads. Existing flags with country/language serveValue: 'B' overwritten to 'A' on edit.

Impact: Targeting behavior changes silently after edits; users receive wrong variant.

Fix:

// Add serveValue to UI model and preserve end-to-end
targeting: {
  countries: validCountries.map(c => ({
    country: c.country,
    serveValue: c.serveValue, // 'A' | 'B'
    languages: c.languages?.map(l => ({
      language: l.language,
      serveValue: l.serveValue,
    })),
  })),
  // ...
}

πŸ”΄ HIGH: Admin Client Filter Logic Broken

File: apps/admin/src/app/(dashboard)/configs/page.tsx:100-113

Category: Bug

Description: Filter values don't match logic. "Active" uses 'all', logic checks 'active'. "All Versions" uses 'inactive' but shows only inactive.

Impact: Users cannot view all versions; filtering misleading.

Fix:

type ConfigFilter = 'active' | 'all' | 'inactive';

const filteredParameters = useMemo(() => {
  if (filter === 'active') return activeParameters;
  if (filter === 'inactive') return parameters.filter((p) => !p.isActive);
  return parameters; // all versions
}, [activeParameters, parameters, filter]);

const filterOptions = [
  { value: 'active' as const, label: 'Active', count: activeCount },
  { value: 'all' as const, label: 'All Versions', count: parameters.length },
];

🟑 MEDIUM: Language Code Validation Mismatch

File:

  • apps/admin/src/app/(dashboard)/flags/create/page.tsx:79-104
  • apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/flags/[flagKey]/edit/page.tsx:88-116
  • apps/admin/src/app/(dashboard)/experiments/create/page.tsx:98-115
  • apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/experiments/[experimentKey]/edit/page.tsx:85-113

Category: Bug

Description: UI accepts 2–3 letter language codes, backend requires exactly 2.

Impact: API rejects input (422); confusing save failures.

Fix:

const validPattern = /^[a-z]{2}$/;

🟑 MEDIUM: Rollout Defaults Wrong on Edit

File: apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/flags/[flagKey]/edit/page.tsx:209-212

Category: Bug

Description: Rollout defaults wrong when existing flags lack rollout fields; UI forces 50/50 instead of server defaults 100/0.

Impact: Editing old flags unintentionally changes rollout.

Fix:

setRolloutPercentageA(data.rolloutPercentageA ?? 100);
setRolloutPercentageB(data.rolloutPercentageB ?? 0);

🟑 MEDIUM: UI Claims Default 'A' But Server Default 'B'

File: apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/flags/[flagKey]/edit/page.tsx:701-704

Category: Bug / Code Quality

Description: UI text says "receive Value A by default." Server uses defaultValue (default is B).

Impact: Admin UI misleads; incorrect expectations.

Fix:

// Update copy to reflect defaultValue
<p>When disabled, users receive the configured default value (A or B).</p>

🟑 MEDIUM: Config Delete State Never Reset

File: apps/admin/src/components/configs/config-parameter-history.tsx:82-90

Category: Bug

Description: isDeleting never reset to false after delete. Modal reopened in same session leaves actions disabled.

Impact: Delete/rollback actions permanently disabled until refresh.

Fix:

await deleteConfigParameterApi(...);
setIsDeleting(false);
setIsOpen(false);

🟑 MEDIUM: localStorage No Error Handling

File: apps/admin/src/components/filters/platform-env-filter.tsx:64-74, 115-116, 225-230

Category: Bug

Description: localStorage.getItem/setItem used without try/catch. Safari private mode or restricted storage throws.

Impact: Filter UI crashes for some users.

Fix:

try {
  const stored = localStorage.getItem(STORAGE_KEY);
  // ...
} catch { /* ignore storage errors */ }

try {
  localStorage.setItem(STORAGE_KEY, JSON.stringify({ platform, environment }));
} catch { /* ignore quota/blocked storage */ }

🟒 LOW: List Uses key={index} Instead of Stable ID

File:

  • apps/admin/src/app/(dashboard)/flags/create/page.tsx:546-550
  • apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/flags/[flagKey]/edit/page.tsx:759-766
  • apps/admin/src/app/(dashboard)/experiments/create/page.tsx:970-977
  • apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/experiments/[experimentKey]/edit/page.tsx:669-676

Category: Bug

Description: Dynamic lists use key={index} for country/language rows. Removing row causes inputs to "shift" due to key reuse.

Impact: Users unintentionally edit wrong country/language.

Fix:

type Pair = { id: string; country: string; languages: string };
setCountryLanguagePairs(prev => [...prev, { id: crypto.randomUUID(), country: '', languages: '' }]);
// key={pair.id}

🟒 LOW: Numeric Flag Values Parse to 0 on Error

File:

  • apps/admin/src/app/(dashboard)/flags/create/page.tsx:238-241
  • apps/admin/src/app/(dashboard)/platforms/[platform]/environments/[environment]/flags/[flagKey]/edit/page.tsx:257-262

Category: Bug

Description: Numeric values parsed with Number(val) || 0 / parseFloat(raw) || 0, silently converts invalid to 0.

Impact: Accidental data loss on invalid input.

Fix:

const parsed = Number(val);
if (Number.isNaN(parsed)) {
  setError('Value must be a valid number');
  return;
}
return parsed;

🟒 LOW: useEffect Fetch No Abort Handling

File:

  • apps/admin/src/app/(dashboard)/flags/page.tsx:57-88
  • apps/admin/src/app/(dashboard)/experiments/page.tsx:61-92
  • apps/admin/src/app/(dashboard)/evaluation/page.tsx:108-182

Category: Code Quality / Performance

Description: Data fetches in useEffect have no abort handling. Navigating away sets state on unmounted components.

Impact: React warnings; wasted work on slow networks.

Fix:

useEffect(() => {
  const controller = new AbortController();
  (async () => { await loadFlags({ signal: controller.signal }); })();
  return () => controller.abort();
}, [loadFlags]);

🟒 LOW: Timeout Callbacks No Cleanup

File:

  • apps/admin/src/components/api-keys/create-api-key-dialog.tsx:73
  • apps/admin/src/app/(dashboard)/profile/page.tsx:62,93

Category: Bug

Description: setTimeout callbacks update state without cleanup. Unmount before callback fires causes warnings.

Impact: Console warnings; minor memory leaks.

Fix:

const timerRef = useRef<number | null>(null);

useEffect(() => () => {
  if (timerRef.current) window.clearTimeout(timerRef.current);
}, []);

timerRef.current = window.setTimeout(() => setCopied(false), 2000);

Code Quality

🟑 MEDIUM: Filter Tab Buttons Missing type="button"

File: packages/ui/src/components/filter-tabs.tsx:33-36

Category: Bug / UX

Description: Buttons omit type="button", default to type="submit" inside forms.

Impact: Clicking filter can submit form unexpectedly.

Fix:

<button
  type="button"
  onClick={() => onChange(option.value)}
  className={cn(/* ... */)}
>

🟑 MEDIUM: Alert Close Button Missing type="button"

File: packages/ui/src/components/alert.tsx:57-60

Category: Bug / UX

Description: Close button omits type="button".

Impact: Users inadvertently submit form when dismissing.

Fix:

<button
  type="button"
  onClick={onClose}
  className="absolute right-4 top-4 ..."
>

🟑 MEDIUM: Logger Child Context Missing

File: packages/shared/src/logger.ts:195-198

Category: Bug / Code Quality

Description: LoggerService.child() constructs new instance, replaces logger, but httpLogger created with old logger. Child's HTTP logger won't include new context.

Impact: Request logs miss correlation context; broken tracing.

Fix:

child(context: Record<string, unknown>): LoggerService {
  const childLogger = Object.create(this) as LoggerService;
  childLogger.logger = this.logger.child(context);
  childLogger.httpLogger = pinoHttp({ logger: childLogger.logger, /* same config */ });
  return childLogger;
}

🟒 LOW: Debug Logs in Production Code

File:

  • apps/admin/src/lib/api/stats.ts:40-146

Category: Code Quality / Security

Description: Debug logs include platform/env names and counts in production.

Impact: Noisy logs; potential information disclosure.

Fix:

if (process.env.NODE_ENV === 'development') console.log(...);

Example Apps

🟑 MEDIUM: Health Check Routes Incorrectly Mounted

File:

  • apps/example-nodejs/src/routes/index.ts:11, 12
  • apps/example-nodejs/src/routes/health.routes.ts:9, 16

Category: Bug

Description: /ready mounted to same router as / and /ready, so /ready returns basic health response while readiness runs at /ready/ready. Makes readiness probes report "ok" even when ToggleBox is down.

Impact: Deployment health checks incorrectly pass; orchestrators may route traffic to unhealthy service.

Fix:

// apps/example-nodejs/src/routes/health.routes.ts
const router = Router()

router.get('/health', (req, res) => {
  res.json({ status: 'ok', timestamp: new Date().toISOString() })
})

router.get('/ready', async (req, res) => {
  try {
    const health = await togglebox.checkConnection()
    res.json({ status: 'ready', togglebox: health, timestamp: new Date().toISOString() })
  } catch (error) {
    res.status(503).json({ status: 'not ready', error: error instanceof Error ? error.message : 'Unknown error', timestamp: new Date().toISOString() })
  }
})

export default router

// apps/example-nodejs/src/routes/index.ts
routes.use(healthRoutes)

🟒 LOW: Checkout Validation Accepts Zero/Non-Numeric

File: apps/example-nodejs/src/routes/checkout.routes.ts:92

Category: Bug

Description: if (!orderId || !total) treats total = 0 as missing and accepts non-numeric strings.

Impact: Free orders or zero-priced checkouts fail; malformed input flows into analytics.

Fix:

const { orderId, total } = req.body
const totalValue = Number(total)

if (!orderId || !Number.isFinite(totalValue) || totalValue < 0) {
  res.status(400).json({ error: 'orderId is required and total must be a non-negative number' })
  return
}
// use totalValue below

🟒 LOW: Store Routes Multiple Config Fetches

File:

  • apps/example-nodejs/src/routes/store.routes.ts:16, 19

Category: Performance

Description: getConfig() called, then multiple getConfigValue() calls made. If cache cold/disabled, fans out to multiple API requests per request.

Impact: Unnecessary latency and load on config API.

Fix:

const config = await togglebox.getConfig()
const storeName = (config?.storeName as string) ?? 'My Store'
const currency = (config?.currency as string) ?? 'USD'
const taxRate = (config?.taxRate as number) ?? 0.08
const freeShippingThreshold = (config?.freeShippingThreshold as number) ?? 50

🟒 LOW: Product Routes Flag Checks Sequential

File:

  • apps/example-nodejs/src/routes/products.routes.ts:23, 26

Category: Performance

Description: Feature flag checks are sequential.

Impact: Adds avoidable network latency for every request.

Fix:

const [showReviews, expressShipping] = await Promise.all([
  togglebox.isFlagEnabled('reviews-enabled', context),
  togglebox.isFlagEnabled('express-shipping', context),
])

🟑 MEDIUM: Next.js SSR Examples Inconsistent Env Vars

File:

  • apps/example-nextjs/src/app/examples/quick/ssr-config/page.tsx:3, 4
  • apps/example-nextjs/src/app/examples/full/ssr-hydration/page.tsx:3, 4
  • apps/example-nextjs/README.md:71

Category: Security / Code Quality

Description: Server-side examples read NEXT_PUBLIC_API_KEY and NEXT_PUBLIC_API_URL (public envs), while app layout uses TOGGLEBOX_API_KEY and NEXT_PUBLIC_TOGGLEBOX_*. Inconsistency encourages placing private API key in public env var and makes examples fail unless users guess correct env names.

Impact: Risk of accidental secret exposure in client bundles and misconfiguration.

Fix:

// server components (ssr-config, ssr-hydration)
const API_URL = process.env.TOGGLEBOX_API_URL || 'http://localhost:3000/api/v1'
const API_KEY = process.env.TOGGLEBOX_API_KEY
const PLATFORM = process.env.TOGGLEBOX_PLATFORM || 'web'
const ENVIRONMENT = process.env.TOGGLEBOX_ENVIRONMENT || 'production'

And in .env.local:

TOGGLEBOX_API_URL=http://localhost:3000/api/v1
TOGGLEBOX_PLATFORM=web
TOGGLEBOX_ENVIRONMENT=development
TOGGLEBOX_API_KEY=your-server-api-key
NEXT_PUBLIC_TOGGLEBOX_API_URL=http://localhost:3000/api/v1
NEXT_PUBLIC_TOGGLEBOX_PLATFORM=web
NEXT_PUBLIC_TOGGLEBOX_ENVIRONMENT=development
# Only include NEXT_PUBLIC_TOGGLEBOX_API_KEY if it's explicitly a public/anonymous key

🟒 LOW: Expo README Env Var Mismatch

File:

  • apps/example-expo/README.md:73, 77
  • apps/example-expo/app/_layout.tsx:6

Category: Code Quality

Description: README uses EXPO_PUBLIC_API_* while app uses EXPO_PUBLIC_TOGGLEBOX_*.

Impact: Copy/paste setup doesn't work out of the box.

Fix:

# apps/example-expo/README.md
EXPO_PUBLIC_TOGGLEBOX_API_URL=http://localhost:3000/api/v1
EXPO_PUBLIC_TOGGLEBOX_PLATFORM=mobile
EXPO_PUBLIC_TOGGLEBOX_ENVIRONMENT=development
EXPO_PUBLIC_TOGGLEBOX_API_KEY=your-public-api-key

🟑 MEDIUM: Expo AB Test Duplicate Impressions

File: apps/example-expo/app/examples/full/ab-test-cta.tsx:12

Category: Bug / Performance

Description: Impression tracking fires every time effect reruns (e.g., isLoading toggles), causing duplicate impressions.

Impact: Inflated analytics and skewed A/B test results.

Fix:

const impressionSentRef = useRef(false)

useEffect(() => {
  if (isLoading || impressionSentRef.current) return
  getVariant('pricing-cta', { userId: 'user-123' }).then((v) => {
    setVariant(v)
    if (!impressionSentRef.current) {
      trackEvent('impression', { userId: 'user-123' }, {
        properties: { experimentKey: 'pricing-cta', variationKey: v },
      })
      impressionSentRef.current = true
    }
  })
}, [isLoading, getVariant, trackEvent])

Remaining Pending Issues

πŸ”΄ CRITICAL: Non-Atomic Update Race Condition

File: packages/database/src/adapters/mongoose/MongooseConfigRepository.ts

Category: Data Integrity / Concurrency

Description: Update performs 3 separate database calls without transaction: findOne() β†’ updateOne() (deactivate) β†’ create() (new version). If failure occurs between step 2 and 3, there's no active version (data loss). Race condition under high concurrency.

Impact: Data loss; configuration service downtime under concurrent updates.

Fix:

const session = await mongoose.startSession();
session.startTransaction();
try {
  await Model.updateOne({...}, { session });
  await Model.create([{...}], { session });
  await session.commitTransaction();
} catch (e) {
  await session.abortTransaction();
  throw e;
}

πŸ”΄ HIGH: Missing Zod Schema for createEnvironment

File: apps/api/src/controllers/configController.ts

Category: Bug / Data Integrity

Description: createEnvironment uses manual validation instead of Zod schema. No validation on description length, format, or constraints.

Impact: Invalid environment names/descriptions stored; breaks downstream logic.

Fix:

const CreateEnvironmentSchema = z.object({
  environment: z.string().min(1).max(100).regex(/^[a-z0-9-]+$/),
  description: z.string().max(500).optional(),
});

πŸ”΄ HIGH: Event Listener Cleanup Missing in SDK Providers

File:

  • packages/sdk-nextjs/src/provider.tsx
  • packages/sdk-expo/src/provider.tsx

Category: Bug / Memory Leak

Description: Event listeners not explicitly removed; relies on client.destroy() to clean them up internally. If destroy fails or is skipped, listeners persist.

Impact: Memory leaks and duplicate event handlers on component remount.

Fix:

useEffect(() => {
  const updateHandler = (data) => {
    setConfig(data.config);
    setFlags(data.flags);
  };
  client.on('update', updateHandler);

  return () => {
    client.off('update', updateHandler);  // Explicit cleanup
    client.destroy();
  };
}, [...]);

πŸ”΄ HIGH: Controllers Use Fragile error.message.includes()

File: apps/api/src/controllers/experimentController.ts (10+ occurrences)

Category: Bug / Code Quality

Description: Error handling relies on error message text which is fragile - typos or message changes break error handling. Custom error classes exist but aren't being used.

Impact: Broken error handling if error messages change; inconsistent response codes.

Fix:

} catch (error) {
  if (error instanceof NotFoundError) {
    res.status(404).json({...});
  } else if (error instanceof ValidationError) {
    res.status(400).json({...});
  }
}

πŸ”΄ HIGH: Fire-and-Forget Cache Invalidation

File: apps/api/src/controllers/configController.ts (6 occurrences)

Category: Bug / Data Integrity

Description: Cache invalidation failures logged but swallowed. Client gets success response even though cache may still serve stale data. No visibility into cache invalidation success.

Impact: Stale data served to clients after updates; users see outdated configs/flags/experiments.

Fix (choose one):

// Option A: Await and handle
const invalidated = await this.cacheProvider.invalidateCache(cachePaths)
if (!invalidated) {
  logger.warn('Cache invalidation failed');
  res.status(207).json({ success: true, warning: 'Cache invalidation failed' });
  return;
}

// Option B: Track in response
res.json({
  success: true,
  data: result,
  cacheInvalidated: true,
});

// Option C: Queue for retry
if (!await this.cacheProvider.invalidateCache(cachePaths)) {
  this.retryQueue.push({ type: 'cache', paths: cachePaths });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment