gsadmin/sencho
1
0
Fork 0
mirror of https://github.com/Studio-Saelix/sencho.git synced 2026-08-25 17:57:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(backend): sanitize user input before logging to close CRLF injection (#807)

Browse Source 
* refactor(backend): sanitize user input before logging to close CRLF injection

Adds a small sanitizeForLog helper that strips CR, LF, tab, and ASCII
control characters (0x00-0x1F, 0x7F) from a value before it is embedded
in a console.log/warn/error/debug call. Wraps every call site where a
user-controlled value (req.params, req.body, req.query, or a value
derived from them) flows into a log message.

Closes the bulk of the open CodeQL alerts in this family:
- 96 js/log-injection
- 28 js/tainted-format-string

The helper is in backend/src/utils/safeLog.ts. Routes still pre-validate
input at the request boundary; this is the second line of defense and
gives static analyzers a sanitizer they can trace through. JSON
responses, Docker filter labels, and other non-log call sites are
intentionally left unwrapped.

* refactor(backend): printf-style format strings for tainted-log call sites

CodeQL's js/tainted-format-string rule flags template literals in the first
arg of console.X when any interpolated value is user-controlled, regardless
of whether each value is sanitized inline. The canonical mitigation is to
use a static format string and pass values as positional args.

Converts the 28 flagged template literals to printf-style ("%s") format
strings, with sanitizeForLog applied to each positional arg. Also fills in
the log-injection wraps on 9 sites where a user-controlled value was
missed in the first sweep (agents, fleet, gitSources, imageUpdates,
GitSourceService).

No behavior change at runtime. Node's util.format substitutes %s tokens
identically to template-literal interpolation.

* fix(backend): wrap nodeId/snapshotId in fleet restore debug log

CodeQL flagged the unwrapped numeric args even though they cannot
contain control chars in practice. Apply the sanitizer for taint-flow
recognition.
This commit is contained in:
Anso
2026-04-27 10:47:23 -04:00
committed by GitHub
parent 77f27b4bf9
commit 4e5ba17710
32 changed files with 148 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/services/ComposeService.ts
+8 -7
View File
@@ -12,6 +12,7 @@ import { RegistryService } from './RegistryService';
import { isDebugEnabled } from '../utils/debug';
import { isPathWithinBase, isValidStackName } from '../utils/validation';
import { sanitizeForLog } from '../utils/safeLog';
/**
* ComposeService - local docker compose CLI execution.
@@ -141,7 +142,7 @@ export class ComposeService {
await fsSvc.backupStackFiles(stackName);
sendOutput('=== Backup created for atomic deployment ===\n');
} catch (e) {
console.warn(`Failed to backup stack files for ${stackName}:`, e);
console.warn('Failed to backup stack files for %s:', sanitizeForLog(stackName), e);
}
}
@@ -154,7 +155,7 @@ export class ComposeService {
await dockerController.removeContainers(legacyContainers.map((c: any) => c.Id));
}
} catch (e) {
console.warn(`Failed to clean up legacy containers for ${stackName}:`, e);
console.warn('Failed to clean up legacy containers for %s:', sanitizeForLog(stackName), e);
}
await this.withRegistryAuth(async (env) => {
@@ -196,7 +197,7 @@ export class ComposeService {
}, sendOutput);
sendOutput('=== Rolled back successfully ===\n');
} catch (rollbackError) {
console.error(`Rollback failed for ${stackName}:`, rollbackError);
console.error('Rollback failed for %s:', sanitizeForLog(stackName), rollbackError);
sendOutput('=== Rollback failed - manual intervention may be required ===\n');
}
}
@@ -331,7 +332,7 @@ export class ComposeService {
await fsSvc.backupStackFiles(stackName);
sendOutput('=== Backup created for atomic update ===\n');
} catch (e) {
console.warn(`Failed to backup stack files for ${stackName}:`, e);
console.warn('Failed to backup stack files for %s:', sanitizeForLog(stackName), e);
}
}
@@ -344,7 +345,7 @@ export class ComposeService {
await dockerController.removeContainers(legacyContainers.map((c: any) => c.Id));
}
} catch (e) {
console.warn(`Failed to clean up legacy containers for ${stackName}:`, e);
console.warn('Failed to clean up legacy containers for %s:', sanitizeForLog(stackName), e);
}
await this.withRegistryAuth(async (env) => {
@@ -392,7 +393,7 @@ export class ComposeService {
}, sendOutput);
sendOutput('=== Rolled back successfully ===\n');
} catch (rollbackError) {
console.error(`Rollback failed for ${stackName}:`, rollbackError);
console.error('Rollback failed for %s:', sanitizeForLog(stackName), rollbackError);
sendOutput('=== Rollback failed - manual intervention may be required ===\n');
}
}
@@ -405,7 +406,7 @@ export class ComposeService {
try {
await this.execute('docker', ['compose', 'down', '--volumes', '--remove-orphans'], stackPath, undefined, false);
} catch (error) {
console.warn(`[Teardown] Docker down failed or nothing to clean up for ${stackName}`);
console.warn(`[Teardown] Docker down failed or nothing to clean up for ${sanitizeForLog(stackName)}`);
}
}
backend/src/services/DockerController.ts
+6 -5
View File
@@ -10,6 +10,7 @@ import { NodeRegistry } from './NodeRegistry';
import { CacheService } from './CacheService';
import { isPathWithinBase } from '../utils/validation';
import { isDebugEnabled } from '../utils/debug';
import { sanitizeForLog } from '../utils/safeLog';
const execAsync = promisify(exec);
const COMPOSE_DIR = process.env.COMPOSE_DIR || '/app/compose';
@@ -644,7 +645,7 @@ class DockerController {
} catch (err: unknown) {
const code = (err as NodeJS.ErrnoException)?.code;
if (code !== 'ENOENT' && code !== 'ENOTDIR') {
console.error(`[DockerController] Failed to read ${filePath}:`, err);
console.error('[DockerController] Failed to read %s:', sanitizeForLog(filePath), sanitizeForLog((err as Error)?.message ?? String(err)));
break;
}
}
@@ -790,7 +791,7 @@ class DockerController {
containers = lines.map(line => JSON.parse(line) as ComposeContainer);
} catch (innerError) {
// Log parsing failure with stderr for debugging
console.error(`Docker Compose JSON Parse Error for ${stackName}:`, stderr || (parseError as Error).message);
console.error('Docker Compose JSON Parse Error for %s:', sanitizeForLog(stackName), sanitizeForLog(stderr || (parseError as Error).message));
// Don't return empty - trigger smart fallback below
}
}
@@ -827,7 +828,7 @@ class DockerController {
} catch (error) {
// If command fails (e.g., stack not deployed, invalid YAML, missing env_file)
const execError = error as { stderr?: string; message?: string };
console.error(`Docker Compose Error for ${stackName}:`, execError.stderr || execError.message);
console.error('Docker Compose Error for %s:', sanitizeForLog(stackName), sanitizeForLog(execError.stderr || execError.message || 'unknown'));
// Try smart fallback even on error
return await this.enrichContainers(await this.smartFallback(stackName, stackDir));
@@ -930,7 +931,7 @@ class DockerController {
};
});
} catch (fallbackError) {
console.error(`Smart Fallback failed for ${stackName}:`, fallbackError);
console.error('Smart Fallback failed for %s:', sanitizeForLog(stackName), sanitizeForLog((fallbackError as Error)?.message ?? String(fallbackError)));
return [];
}
}
@@ -1053,7 +1054,7 @@ class DockerController {
await container.remove({ force: true });
results.push({ id, success: true });
} catch (error: any) {
console.error(`Failed to remove container ${id}:`, error.message);
console.error('Failed to remove container %s:', sanitizeForLog(id), sanitizeForLog(error.message));
results.push({ id, success: false, error: error.message });
}
}
backend/src/services/FileSystemService.ts
+2 -1
View File
@@ -4,6 +4,7 @@ import type { Readable } from 'stream';
import { NodeRegistry } from './NodeRegistry';
import { isPathWithinBase, isValidStackName } from '../utils/validation';
import { isBinaryBuffer } from '../utils/binaryDetect';
import { sanitizeForLog } from '../utils/safeLog';
export interface FileEntry {
name: string;
@@ -133,7 +134,7 @@ export class FileSystemService {
const filePath = await this.getComposeFilePath(stackName);
return await fsPromises.readFile(filePath, 'utf-8');
} catch (error) {
console.error('Error reading stack content:', error);
console.error('Error reading stack content:', sanitizeForLog((error as Error)?.message ?? String(error)));
throw new Error(`Failed to read stack: ${stackName}`);
}
}
backend/src/services/GitSourceService.ts
+8 -7
View File
@@ -11,6 +11,7 @@ import { DatabaseService, type StackGitSource, type GitSourceAuthType } from './
import { FileSystemService } from './FileSystemService';
import { ComposeService } from './ComposeService';
import { isDebugEnabled } from '../utils/debug';
import { sanitizeForLog } from '../utils/safeLog';
/**
* GitSourceService - fetch compose files from a Git repository and apply
@@ -380,7 +381,7 @@ export class GitSourceService {
const diag = isDebugEnabled();
if (diag) {
console.log(
`[GitSource:diag] fetch start host=${repoHost(repoUrl)} branch=${branch} compose=${composePath} envSync=${envPath ? 'true' : 'false'} timeoutMs=${timeoutMs}`
`[GitSource:diag] fetch start host=${sanitizeForLog(repoHost(repoUrl))} branch=${sanitizeForLog(branch)} compose=${sanitizeForLog(composePath)} envSync=${envPath ? 'true' : 'false'} timeoutMs=${timeoutMs}`
);
}
@@ -444,7 +445,7 @@ export class GitSourceService {
throw new GitSourceError('GIT_ERROR', scrubCredentials((e as Error).message));
}
if (isLfsPointer(composeContent)) {
console.error(`[GitSource] LFS pointer detected in ${composePath}`);
console.error(`[GitSource] LFS pointer detected in ${sanitizeForLog(composePath)}`);
throw new GitSourceError(
'GIT_ERROR',
`Compose file at ${composePath} is stored in Git LFS, which is not supported. Commit the plain file or replace the LFS pointer before linking this repository.`,
@@ -470,7 +471,7 @@ export class GitSourceService {
}
}
if (envContent !== null && isLfsPointer(envContent)) {
console.error(`[GitSource] LFS pointer detected in ${envPath}`);
console.error(`[GitSource] LFS pointer detected in ${sanitizeForLog(envPath)}`);
throw new GitSourceError(
'GIT_ERROR',
`Env file at ${envPath} is stored in Git LFS, which is not supported. Commit the plain file or replace the LFS pointer before linking this repository.`,
@@ -489,7 +490,7 @@ export class GitSourceService {
if (diag) {
console.log(
`[GitSource:diag] fetch ok host=${repoHost(repoUrl)} branch=${branch} sha=${commitSha.slice(0, 7)} env=${envContent !== null ? 'present' : 'absent'} warnings=${warnings.length} elapsedMs=${Date.now() - startedAt}`
`[GitSource:diag] fetch ok host=${sanitizeForLog(repoHost(repoUrl))} branch=${sanitizeForLog(branch)} sha=${commitSha.slice(0, 7)} env=${envContent !== null ? 'present' : 'absent'} warnings=${warnings.length} elapsedMs=${Date.now() - startedAt}`
);
}
return { composeContent, envContent, commitSha, warnings };
@@ -497,7 +498,7 @@ export class GitSourceService {
if (diag) {
const msg = err instanceof GitSourceError ? `${err.code}: ${err.message}` : (err as Error).message;
console.log(
`[GitSource:diag] fetch fail host=${repoHost(repoUrl)} branch=${branch} elapsedMs=${Date.now() - startedAt} err=${scrubCredentials(msg)}`
`[GitSource:diag] fetch fail host=${sanitizeForLog(repoHost(repoUrl))} branch=${sanitizeForLog(branch)} elapsedMs=${Date.now() - startedAt} err=${sanitizeForLog(scrubCredentials(msg))}`
);
}
throw err;
@@ -742,7 +743,7 @@ export class GitSourceService {
throw new GitSourceError('GIT_ERROR', 'No pending pull to apply. Fetch the source again.');
}
if (src.pending_commit_sha !== commitSha) {
if (diag) console.log(`[GitSource:diag] apply sha mismatch stack=${stackName} expected=${commitSha.slice(0, 7)} pending=${src.pending_commit_sha.slice(0, 7)}`);
if (diag) console.log('[GitSource:diag] apply sha mismatch stack=%s expected=%s pending=%s', sanitizeForLog(stackName), sanitizeForLog(commitSha.slice(0, 7)), sanitizeForLog(src.pending_commit_sha.slice(0, 7)));
throw new GitSourceError('GIT_ERROR', 'Pending commit has changed since this pull was fetched. Please review the latest diff.');
}
@@ -770,7 +771,7 @@ export class GitSourceService {
db.markGitSourceApplied(stackName, commitSha, hash);
const shouldDeploy = opts.deploy ?? src.auto_deploy_on_apply;
if (diag) console.log(`[GitSource:diag] apply wrote stack=${stackName} sha=${commitSha.slice(0, 7)} deploy=${shouldDeploy}`);
if (diag) console.log('[GitSource:diag] apply wrote stack=%s sha=%s deploy=%s', sanitizeForLog(stackName), sanitizeForLog(commitSha.slice(0, 7)), sanitizeForLog(shouldDeploy));
if (shouldDeploy) {
try {
backend/src/services/ImageUpdateService.ts
+2 -1
View File
@@ -9,6 +9,7 @@ import { NotificationService } from './NotificationService';
import { parseImageRef, getRemoteDigest } from './registry-api';
import { isDebugEnabled } from '../utils/debug';
import { getErrorMessage } from '../utils/errors';
import { sanitizeForLog } from '../utils/safeLog';
const BACKFILL_KEY = 'image_update_notifications_backfilled';
@@ -239,7 +240,7 @@ export class ImageUpdateService {
try {
imageUpdateMap.set(imageRef, await this.checkImage(docker, imageRef));
} catch (e) {
console.error(`[ImageUpdateService] Error checking ${imageRef}:`, e);
console.error(`[ImageUpdateService] Error checking ${sanitizeForLog(imageRef)}:`, sanitizeForLog((e as Error)?.message ?? String(e)));
imageUpdateMap.set(imageRef, { hasUpdate: false, error: String(e) });
}
await sleep(ImageUpdateService.INTER_IMAGE_DELAY_MS);
backend/src/services/NotificationService.ts
+2 -1
View File
@@ -3,6 +3,7 @@ import { DatabaseService, NotificationHistory } from './DatabaseService';
import { NodeRegistry } from './NodeRegistry';
import { isDebugEnabled } from '../utils/debug';
import { getErrorMessage } from '../utils/errors';
import { sanitizeForLog } from '../utils/safeLog';
export type NotificationCategory =
| 'deploy_success'
@@ -139,7 +140,7 @@ export class NotificationService {
return true;
});
if (matched.length > 0) {
if (isDebugEnabled()) console.log(`[Notify:diag] Matched ${matched.length} route(s) for stack "${stackName ?? '(none)'}", category="${category}"`);
if (isDebugEnabled()) console.log(`[Notify:diag] Matched ${matched.length} route(s) for stack "${sanitizeForLog(stackName ?? '(none)')}", category="${sanitizeForLog(category)}"`);
await Promise.allSettled(
matched.map(route =>
this.sendToChannel(route.channel_type, route.channel_url, level, message)
backend/src/services/PolicyEnforcement.ts
+2 -1
View File
@@ -13,6 +13,7 @@ import { DatabaseService } from './DatabaseService';
import type { ScanPolicy, VulnSeverity } from './DatabaseService';
import { FleetSyncService } from './FleetSyncService';
import { NotificationService } from './NotificationService';
import { sanitizeForLog } from '../utils/safeLog';
import TrivyService from './TrivyService';
import { isSeverityAtLeast } from '../utils/severity';
import { validateImageRef } from '../utils/image-ref';
@@ -72,7 +73,7 @@ export async function enforcePolicyPreDeploy(
imageRefs = await ComposeService.getInstance(nodeId).listStackImages(stackName);
} catch (err) {
const message = getErrorMessage(err, 'compose parse failed');
console.error(`[Policy] listStackImages failed for ${stackName}:`, message);
console.error('[Policy] listStackImages failed for %s:', sanitizeForLog(stackName), sanitizeForLog(message));
return {
ok: false,
bypassed: false,
backend/src/services/RegistryService.ts
+5 -4
View File
@@ -3,6 +3,7 @@ import http from 'http';
import { CryptoService } from './CryptoService';
import { DatabaseService, type Registry, type RegistryType } from './DatabaseService';
import { isDebugEnabled } from '../utils/debug';
import { sanitizeForLog } from '../utils/safeLog';
// ─── Types ───────────────────────────────────────────────────────────────────
@@ -141,7 +142,7 @@ function httpGet(
nextHeaders = rest;
}
if (isDebugEnabled()) {
console.debug(`[RegistryService][debug] redirect ${status} ${url} -> ${nextUrl.toString()} (auth ${nextHeaders === headers ? 'kept' : 'stripped'})`);
console.debug(`[RegistryService][debug] redirect ${status} ${sanitizeForLog(url)} -> ${sanitizeForLog(nextUrl.toString())} (auth ${nextHeaders === headers ? 'kept' : 'stripped'})`);
}
httpGet(nextUrl.toString(), nextHeaders, timeoutMs, false).then(resolve, reject);
return;
@@ -208,7 +209,7 @@ export class RegistryService {
created_at: now,
updated_at: now,
});
console.info(`[RegistryService] Registry created: id=${id} type=${input.type} name="${input.name}"`);
console.info(`[RegistryService] Registry created: id=${id} type=${sanitizeForLog(input.type)} name="${sanitizeForLog(input.name)}"`);
return id;
}
@@ -282,7 +283,7 @@ export class RegistryService {
return { success: false, error: 'AWS region is required for ECR registries.' };
}
if (isDebugEnabled()) {
console.debug(`[RegistryService][debug] testWithCredentials ECR region=${input.aws_region}`);
console.debug(`[RegistryService][debug] testWithCredentials ECR region=${sanitizeForLog(input.aws_region)}`);
}
await this.fetchEcrToken(input.username, input.secret, input.aws_region);
if (isDebugEnabled()) {
@@ -294,7 +295,7 @@ export class RegistryService {
const probeUrl = toProbeUrl(input.url, input.type);
const basicAuth = Buffer.from(`${input.username}:${input.secret}`).toString('base64');
if (isDebugEnabled()) {
console.debug(`[RegistryService][debug] testWithCredentials probing ${probeUrl}/v2/`);
console.debug(`[RegistryService][debug] testWithCredentials probing ${sanitizeForLog(probeUrl)}/v2/`);
}
const res = await httpGet(`${probeUrl}/v2/`, { Authorization: `Basic ${basicAuth}` });
backend/src/services/SchedulerService.ts
+2 -1
View File
@@ -9,6 +9,7 @@ import { ImageUpdateService } from './ImageUpdateService';
import type { ImageCheckResult } from './ImageUpdateService';
import { isDebugEnabled } from '../utils/debug';
import { getErrorMessage } from '../utils/errors';
import { sanitizeForLog } from '../utils/safeLog';
import { captureLocalNodeFiles, captureRemoteNodeFiles } from '../utils/snapshot-capture';
import { NodeRegistry } from './NodeRegistry';
import { NotificationService } from './NotificationService';
@@ -698,7 +699,7 @@ export class SchedulerService {
} catch (e) {
const msg = getErrorMessage(e, String(e));
checkErrors.push(msg);
console.warn(`[SchedulerService] Failed to check image ${imageRef}:`, e);
console.warn(`[SchedulerService] Failed to check image ${sanitizeForLog(imageRef)}:`, sanitizeForLog((e as Error)?.message ?? String(e)));
}
}
backend/src/services/TrivyService.ts
+2 -1
View File
@@ -13,6 +13,7 @@ import {
import { FileSystemService } from './FileSystemService';
import { RegistryService } from './RegistryService';
import { disableCapability, enableCapability } from './CapabilityRegistry';
import { sanitizeForLog } from '../utils/safeLog';
import TrivyInstaller, { type TrivySource } from './TrivyInstaller';
import { FleetSyncService } from './FleetSyncService';
import { getErrorMessage } from '../utils/errors';
@@ -26,7 +27,7 @@ const SBOM_TIMEOUT_MS = 3 * 60 * 1000;
export const DIGEST_CACHE_TTL_MS = 24 * 60 * 60 * 1000;
function diag(msg: string, ...args: unknown[]): void {
if (isDebugEnabled()) console.log(`[Trivy:diag] ${msg}`, ...args);
if (isDebugEnabled()) console.log(`[Trivy:diag] ${sanitizeForLog(msg)}`, ...args);
}
interface TrivyRawVulnerability {