Skip to content

Commit 243e105

Browse files
authored
fix(utils): add path traversal validation for ShardedWal groupId (#1232)
1 parent 956fdca commit 243e105

File tree

2 files changed

+112
-3
lines changed

2 files changed

+112
-3
lines changed

packages/utils/src/lib/wal-sharded.ts

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,44 @@ function ensureDirectoryExistsSync(dirPath: string): void {
2828
}
2929
}
3030

31+
/**
32+
* Validates that a groupId is safe to use as a single path segment.
33+
* Rejects path traversal attempts and path separators to prevent writing outside intended directory.
34+
*
35+
* @param groupId - The groupId to validate
36+
* @throws Error if groupId contains unsafe characters or path traversal sequences
37+
*/
38+
function validateGroupId(groupId: string): void {
39+
// Reject empty or whitespace-only groupIds
40+
if (!groupId || groupId.trim().length === 0) {
41+
throw new Error('groupId cannot be empty or whitespace-only');
42+
}
43+
44+
// Reject path separators (both forward and backward slashes)
45+
if (groupId.includes('/') || groupId.includes('\\')) {
46+
throw new Error('groupId cannot contain path separators (/ or \\)');
47+
}
48+
49+
// Reject relative path components
50+
if (groupId === '..' || groupId === '.') {
51+
throw new Error('groupId cannot be "." or ".."');
52+
}
53+
54+
// Reject null bytes which can be used to bypass validation
55+
if (groupId.includes('\0')) {
56+
throw new Error('groupId cannot contain null bytes');
57+
}
58+
59+
// Validate that the resolved path stays within the intended directory
60+
// This catches cases where the path library normalizes to a parent directory
61+
const normalized = path.normalize(groupId);
62+
if (normalized !== groupId || normalized.startsWith('..')) {
63+
throw new Error(
64+
`groupId normalization resulted in unsafe path: ${normalized}`,
65+
);
66+
}
67+
}
68+
3169
// eslint-disable-next-line functional/no-let
3270
let shardCount = 0;
3371

@@ -142,10 +180,10 @@ export class ShardedWal<T extends WalRecord = WalRecord> {
142180
// Determine groupId: use provided, then env var, or generate
143181
// eslint-disable-next-line functional/no-let
144182
let resolvedGroupId: string;
145-
if (groupId) {
146-
// User explicitly provided groupId - use it
183+
if (groupId != null) {
184+
// User explicitly provided groupId - use it (even if empty, validation will catch it)
147185
resolvedGroupId = groupId;
148-
} else if (measureNameEnvVar && process.env[measureNameEnvVar]) {
186+
} else if (measureNameEnvVar && process.env[measureNameEnvVar] != null) {
149187
// Env var is set (by coordinator or previous process) - use it
150188
resolvedGroupId = process.env[measureNameEnvVar];
151189
} else if (measureNameEnvVar) {
@@ -158,6 +196,9 @@ export class ShardedWal<T extends WalRecord = WalRecord> {
158196
resolvedGroupId = getUniqueTimeId();
159197
}
160198

199+
// Validate groupId for path safety before using it
200+
validateGroupId(resolvedGroupId);
201+
161202
this.groupId = resolvedGroupId;
162203

163204
if (dir) {

packages/utils/src/lib/wal-sharded.unit.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const getShardedWal = (overrides?: {
1818
format?: Partial<WalFormat>;
1919
measureNameEnvVar?: string;
2020
autoCoordinator?: boolean;
21+
groupId?: string;
2122
}) => {
2223
const { format, ...rest } = overrides ?? {};
2324
return new ShardedWal({
@@ -39,6 +40,9 @@ describe('ShardedWal', () => {
3940
// Clear coordinator env var for fresh state
4041
// eslint-disable-next-line functional/immutable-data, @typescript-eslint/no-dynamic-delete
4142
delete process.env[PROFILER_SHARDER_ID_ENV_VAR];
43+
// Clear measure name env var to avoid test pollution
44+
// eslint-disable-next-line functional/immutable-data, @typescript-eslint/no-dynamic-delete
45+
delete process.env.CP_PROFILER_MEASURE_NAME;
4246
});
4347

4448
describe('initialization', () => {
@@ -73,6 +77,70 @@ describe('ShardedWal', () => {
7377
});
7478
});
7579

80+
describe('path traversal validation', () => {
81+
it('should reject groupId with forward slashes', () => {
82+
expect(() => getShardedWal({ groupId: '../etc/passwd' })).toThrow(
83+
'groupId cannot contain path separators (/ or \\)',
84+
);
85+
});
86+
87+
it('should reject groupId with backward slashes', () => {
88+
expect(() => getShardedWal({ groupId: '..\\windows\\system32' })).toThrow(
89+
'groupId cannot contain path separators (/ or \\)',
90+
);
91+
});
92+
93+
it('should reject groupId with parent directory reference', () => {
94+
expect(() => getShardedWal({ groupId: '..' })).toThrow(
95+
'groupId cannot be "." or ".."',
96+
);
97+
});
98+
99+
it('should reject groupId with current directory reference', () => {
100+
expect(() => getShardedWal({ groupId: '.' })).toThrow(
101+
'groupId cannot be "." or ".."',
102+
);
103+
});
104+
105+
it('should reject groupId with null bytes', () => {
106+
expect(() => getShardedWal({ groupId: 'test\0malicious' })).toThrow(
107+
'groupId cannot contain null bytes',
108+
);
109+
});
110+
111+
it('should reject empty groupId', () => {
112+
expect(() => getShardedWal({ groupId: '' })).toThrow(
113+
'groupId cannot be empty or whitespace-only',
114+
);
115+
});
116+
117+
it('should reject whitespace-only groupId', () => {
118+
expect(() => getShardedWal({ groupId: ' ' })).toThrow(
119+
'groupId cannot be empty or whitespace-only',
120+
);
121+
});
122+
123+
it('should accept safe alphanumeric groupId', () => {
124+
const sw = getShardedWal({ groupId: 'safe-group-123' });
125+
expect(sw.groupId).toBe('safe-group-123');
126+
});
127+
128+
it('should accept groupId with underscores and hyphens', () => {
129+
const sw = getShardedWal({ groupId: 'test_group-name' });
130+
expect(sw.groupId).toBe('test_group-name');
131+
});
132+
133+
it('should reject groupId from env var with path traversal', () => {
134+
// eslint-disable-next-line functional/immutable-data
135+
process.env.CP_PROFILER_MEASURE_NAME = '../malicious';
136+
expect(() =>
137+
getShardedWal({
138+
measureNameEnvVar: 'CP_PROFILER_MEASURE_NAME',
139+
}),
140+
).toThrow('groupId cannot contain path separators (/ or \\)');
141+
});
142+
});
143+
76144
describe('shard management', () => {
77145
it('should create shard with correct file path', () => {
78146
const sw = getShardedWal({

0 commit comments

Comments
 (0)