fix(security): harden API endpoints and resolve PR #674 review findings

- Fix X-Forwarded-For header spoofing in requireSensitiveLocalAccess (use socket-level address only)
- Add model ID length validation (max 256 chars) in PUT /models/:provider
- Add provider name validation (alphanumeric, max 64 chars) to prevent path traversal
- Add stale entry eviction for unbounded quotaRateLimits Map (>1000 entries)
- Fix concurrent refresh race in usage aggregator (wait for in-flight before forced refresh)
- Sanitize internal URLs/paths from OAuth failure diagnostics
- Add ValidationError class for denylist violations (distinct from system errors)
- Make CLIProxy sync interval configurable via CCS_CLIPROXY_SYNC_INTERVAL env var
- Improve legacy continuity config error logging (unconditional warn)
- Add CACHE_VERSION history comments
This commit is contained in:
Tam Nhu Tran
2026-03-04 01:30:46 +07:00
parent 2651e17412
commit a4c5bb7421
9 changed files with 76 additions and 22 deletions
+5 -7
View File
@@ -52,13 +52,11 @@ function loadLegacyContinuityInheritanceMap(): Record<string, string> {
return normalized; return normalized;
} catch (error) { } catch (error) {
if (process.env.CCS_DEBUG) { console.error(
console.error( warn(
warn( `Failed to parse legacy continuity mapping at "${configJsonPath}": ${(error as Error).message}`
`Failed to parse legacy continuity mapping at "${configJsonPath}": ${(error as Error).message}` )
) );
);
}
return {}; return {};
} }
} }
+5 -1
View File
@@ -304,8 +304,12 @@ async function handleTokenNotFound(
console.log(''); console.log('');
if (failureReason) { if (failureReason) {
// Sanitize internal URLs/paths from failure reason to avoid leaking infrastructure details
const sanitizedReason = failureReason
.replace(/https?:\/\/(?:localhost|127\.0\.0\.1|0\.0\.0\.0)[^\s]*/gi, '[internal-url]')
.replace(/\/(?:root|home|opt|tmp|var)\/[^\s]*/g, '[path]');
console.log(fail('Authentication completed but token was not persisted')); console.log(fail('Authentication completed but token was not persisted'));
console.log(` ${failureReason}`); console.log(` ${sanitizedReason}`);
console.log(''); console.log('');
console.log('This usually means provider-side authorization was accepted,'); console.log('This usually means provider-side authorization was accepted,');
console.log('but CLIProxy failed a post-auth verification or token save step.'); console.log('but CLIProxy failed a post-auth verification or token save step.');
+14
View File
@@ -155,6 +155,20 @@ export class UserAbortError extends CCSError {
} }
} }
/**
* Input validation errors (model denylist, invalid format, length limits)
* Distinguishes user-input validation failures from system errors
*/
export class ValidationError extends CCSError {
constructor(
message: string,
public readonly field?: string
) {
super(message, ExitCode.GENERAL_ERROR, false);
this.name = 'ValidationError';
}
}
/** /**
* Type guard to check if an error is a CCSError * Type guard to check if an error is a CCSError
*/ */
@@ -77,6 +77,16 @@ function buildQuotaRateLimitKey(req: Request, provider: string): string {
function isQuotaRouteRateLimited(req: Request, provider: string): boolean { function isQuotaRouteRateLimited(req: Request, provider: string): boolean {
const key = buildQuotaRateLimitKey(req, provider); const key = buildQuotaRateLimitKey(req, provider);
const now = Date.now(); const now = Date.now();
// Evict stale entries to prevent unbounded memory growth
if (quotaRateLimits.size > 1000) {
for (const [k, v] of quotaRateLimits) {
if (now - v.windowStart >= QUOTA_RATE_LIMIT_WINDOW_MS * 2) {
quotaRateLimits.delete(k);
}
}
}
const current = quotaRateLimits.get(key); const current = quotaRateLimits.get(key);
if (!current || now - current.windowStart >= QUOTA_RATE_LIMIT_WINDOW_MS) { if (!current || now - current.windowStart >= QUOTA_RATE_LIMIT_WINDOW_MS) {
@@ -584,6 +594,13 @@ router.get('/auth-files/download', async (req: Request, res: Response): Promise<
router.put('/models/:provider', async (req: Request, res: Response): Promise<void> => { router.put('/models/:provider', async (req: Request, res: Response): Promise<void> => {
try { try {
const { provider } = req.params; const { provider } = req.params;
// Validate provider name to prevent path traversal via crafted provider param
if (!provider || provider.length > 64 || !/^[a-zA-Z0-9_-]+$/.test(provider)) {
res.status(400).json({ error: 'Invalid provider name' });
return;
}
const { model } = req.body; const { model } = req.body;
if (!model || typeof model !== 'string') { if (!model || typeof model !== 'string') {
@@ -591,6 +608,11 @@ router.put('/models/:provider', async (req: Request, res: Response): Promise<voi
return; return;
} }
if (model.length > 256) {
res.status(400).json({ error: 'Model ID exceeds maximum length (256 characters)' });
return;
}
const deniedReason = getDeniedModelIdReasonForProvider(model, provider); const deniedReason = getDeniedModelIdReasonForProvider(model, provider);
if (deniedReason) { if (deniedReason) {
res.status(400).json({ error: deniedReason }); res.status(400).json({ error: deniedReason });
+3 -2
View File
@@ -16,6 +16,7 @@ import {
} from '../../cliproxy/model-id-normalizer'; } from '../../cliproxy/model-id-normalizer';
import type { CLIProxyProvider } from '../../cliproxy/types'; import type { CLIProxyProvider } from '../../cliproxy/types';
import type { Config, Settings } from '../../types/config'; import type { Config, Settings } from '../../types/config';
import { ValidationError } from '../../errors/error-types';
/** Model mapping for API profiles */ /** Model mapping for API profiles */
export interface ModelMapping { export interface ModelMapping {
@@ -155,7 +156,7 @@ export function createSettingsFile(
canonicalHaikuModel, canonicalHaikuModel,
]); ]);
if (deniedReason) { if (deniedReason) {
throw new Error(deniedReason); throw new ValidationError(deniedReason, 'model');
} }
const droidProvider = resolveDroidProvider({ const droidProvider = resolveDroidProvider({
provider, provider,
@@ -237,7 +238,7 @@ export function updateSettingsFile(
: settings.env?.ANTHROPIC_DEFAULT_HAIKU_MODEL, : settings.env?.ANTHROPIC_DEFAULT_HAIKU_MODEL,
]); ]);
if (deniedReason) { if (deniedReason) {
throw new Error(deniedReason); throw new ValidationError(deniedReason, 'model');
} }
if (updates.baseUrl) { if (updates.baseUrl) {
+4 -4
View File
@@ -78,10 +78,10 @@ function requireSensitiveLocalAccess(req: Request, res: Response): boolean {
return true; return true;
} }
const forwarded = req.headers['x-forwarded-for']; // Use only socket-level address for security decisions.
const firstForwarded = // X-Forwarded-For is trivially spoofable and must NOT be trusted
typeof forwarded === 'string' ? forwarded.split(',')[0]?.trim() : undefined; // without an explicit trust proxy configuration.
const candidateAddress = firstForwarded || req.socket.remoteAddress || req.ip; const candidateAddress = req.socket.remoteAddress;
if (isLoopbackAddress(candidateAddress)) { if (isLoopbackAddress(candidateAddress)) {
return true; return true;
+10 -1
View File
@@ -378,8 +378,17 @@ async function refreshFromSourceCoalesced(force = false): Promise<{
monthly: MonthlyUsage[]; monthly: MonthlyUsage[];
session: SessionUsage[]; session: SessionUsage[];
}> { }> {
// Wait for any in-flight refresh to finish before starting a forced one
// to prevent concurrent refreshes competing for the same resources
if (force && pendingFullRefresh) {
await pendingFullRefresh.catch(() => {});
}
if (force) { if (force) {
return refreshFromSource(); pendingFullRefresh = refreshFromSource().finally(() => {
pendingFullRefresh = null;
});
return pendingFullRefresh;
} }
if (pendingFullRefresh) { if (pendingFullRefresh) {
+11 -7
View File
@@ -34,6 +34,12 @@ interface CliproxyUsageSnapshot {
const SNAPSHOT_VERSION = 1; const SNAPSHOT_VERSION = 1;
/** Sync interval in ms, configurable via CCS_CLIPROXY_SYNC_INTERVAL env var (default: 5 min) */
const SYNC_INTERVAL_MS = Math.max(
30_000,
parseInt(process.env.CCS_CLIPROXY_SYNC_INTERVAL ?? '300000', 10) || 300_000
);
// Module-level interval ID // Module-level interval ID
let syncIntervalId: ReturnType<typeof setInterval> | null = null; let syncIntervalId: ReturnType<typeof setInterval> | null = null;
@@ -149,17 +155,15 @@ export function startCliproxySync(): void {
return; return;
} }
console.log(info('Starting CLIProxy usage sync (interval: 5 min)')); const intervalMin = Math.round(SYNC_INTERVAL_MS / 60_000);
console.log(info(`Starting CLIProxy usage sync (interval: ${intervalMin} min)`));
// Fire-and-forget initial sync // Fire-and-forget initial sync
void syncCliproxyUsage(); void syncCliproxyUsage();
syncIntervalId = setInterval( syncIntervalId = setInterval(() => {
() => { void syncCliproxyUsage();
void syncCliproxyUsage(); }, SYNC_INTERVAL_MS);
},
5 * 60 * 1000
);
} }
/** /**
+2
View File
@@ -35,6 +35,8 @@ export interface UsageDiskCache {
} }
// Current cache version - increment to invalidate old caches // Current cache version - increment to invalidate old caches
// v1: Initial cache format (daily, monthly, session)
// v2: Added multi-instance aggregation
// v3: Added hourly data to cache // v3: Added hourly data to cache
// v4: Pricing fix for Claude 4.6 models (invalidate stale costs) // v4: Pricing fix for Claude 4.6 models (invalidate stale costs)
const CACHE_VERSION = 4; const CACHE_VERSION = 4;