Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 44 additions & 3 deletions packages/utils/src/lib/wal-sharded.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,44 @@ function ensureDirectoryExistsSync(dirPath: string): void {
}
}

/**
* Validates that a groupId is safe to use as a single path segment.
* Rejects path traversal attempts and path separators to prevent writing outside intended directory.
*
* @param groupId - The groupId to validate
* @throws Error if groupId contains unsafe characters or path traversal sequences
*/
function validateGroupId(groupId: string): void {
// Reject empty or whitespace-only groupIds
if (!groupId || groupId.trim().length === 0) {
throw new Error('groupId cannot be empty or whitespace-only');
}

// Reject path separators (both forward and backward slashes)
if (groupId.includes('/') || groupId.includes('\\')) {
throw new Error('groupId cannot contain path separators (/ or \\)');
}

// Reject relative path components
if (groupId === '..' || groupId === '.') {
throw new Error('groupId cannot be "." or ".."');
}

// Reject null bytes which can be used to bypass validation
if (groupId.includes('\0')) {
throw new Error('groupId cannot contain null bytes');
}

// Validate that the resolved path stays within the intended directory
// This catches cases where the path library normalizes to a parent directory
const normalized = path.normalize(groupId);
if (normalized !== groupId || normalized.startsWith('..')) {
throw new Error(
`groupId normalization resulted in unsafe path: ${normalized}`,
);
}
}

// eslint-disable-next-line functional/no-let
let shardCount = 0;

Expand Down Expand Up @@ -142,10 +180,10 @@ export class ShardedWal<T extends WalRecord = WalRecord> {
// Determine groupId: use provided, then env var, or generate
// eslint-disable-next-line functional/no-let
let resolvedGroupId: string;
if (groupId) {
// User explicitly provided groupId - use it
if (groupId != null) {
// User explicitly provided groupId - use it (even if empty, validation will catch it)
resolvedGroupId = groupId;
} else if (measureNameEnvVar && process.env[measureNameEnvVar]) {
} else if (measureNameEnvVar && process.env[measureNameEnvVar] != null) {
// Env var is set (by coordinator or previous process) - use it
resolvedGroupId = process.env[measureNameEnvVar];
} else if (measureNameEnvVar) {
Expand All @@ -158,6 +196,9 @@ export class ShardedWal<T extends WalRecord = WalRecord> {
resolvedGroupId = getUniqueTimeId();
}

// Validate groupId for path safety before using it
validateGroupId(resolvedGroupId);

this.groupId = resolvedGroupId;

if (dir) {
Expand Down
68 changes: 68 additions & 0 deletions packages/utils/src/lib/wal-sharded.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const getShardedWal = (overrides?: {
format?: Partial<WalFormat>;
measureNameEnvVar?: string;
autoCoordinator?: boolean;
groupId?: string;
}) => {
const { format, ...rest } = overrides ?? {};
return new ShardedWal({
Expand All @@ -39,6 +40,9 @@ describe('ShardedWal', () => {
// Clear coordinator env var for fresh state
// eslint-disable-next-line functional/immutable-data, @typescript-eslint/no-dynamic-delete
delete process.env[PROFILER_SHARDER_ID_ENV_VAR];
// Clear measure name env var to avoid test pollution
// eslint-disable-next-line functional/immutable-data, @typescript-eslint/no-dynamic-delete
delete process.env.CP_PROFILER_MEASURE_NAME;
});

describe('initialization', () => {
Expand Down Expand Up @@ -73,6 +77,70 @@ describe('ShardedWal', () => {
});
});

describe('path traversal validation', () => {
it('should reject groupId with forward slashes', () => {
expect(() => getShardedWal({ groupId: '../etc/passwd' })).toThrow(
'groupId cannot contain path separators (/ or \\)',
);
});

it('should reject groupId with backward slashes', () => {
expect(() => getShardedWal({ groupId: '..\\windows\\system32' })).toThrow(
'groupId cannot contain path separators (/ or \\)',
);
});

it('should reject groupId with parent directory reference', () => {
expect(() => getShardedWal({ groupId: '..' })).toThrow(
'groupId cannot be "." or ".."',
);
});

it('should reject groupId with current directory reference', () => {
expect(() => getShardedWal({ groupId: '.' })).toThrow(
'groupId cannot be "." or ".."',
);
});

it('should reject groupId with null bytes', () => {
expect(() => getShardedWal({ groupId: 'test\0malicious' })).toThrow(
'groupId cannot contain null bytes',
);
});

it('should reject empty groupId', () => {
expect(() => getShardedWal({ groupId: '' })).toThrow(
'groupId cannot be empty or whitespace-only',
);
});

it('should reject whitespace-only groupId', () => {
expect(() => getShardedWal({ groupId: ' ' })).toThrow(
'groupId cannot be empty or whitespace-only',
);
});

it('should accept safe alphanumeric groupId', () => {
const sw = getShardedWal({ groupId: 'safe-group-123' });
expect(sw.groupId).toBe('safe-group-123');
});

it('should accept groupId with underscores and hyphens', () => {
const sw = getShardedWal({ groupId: 'test_group-name' });
expect(sw.groupId).toBe('test_group-name');
});

it('should reject groupId from env var with path traversal', () => {
// eslint-disable-next-line functional/immutable-data
process.env.CP_PROFILER_MEASURE_NAME = '../malicious';
expect(() =>
getShardedWal({
measureNameEnvVar: 'CP_PROFILER_MEASURE_NAME',
}),
).toThrow('groupId cannot contain path separators (/ or \\)');
});
});

describe('shard management', () => {
it('should create shard with correct file path', () => {
const sw = getShardedWal({
Expand Down
Loading