Skip to content

Commit b98a846

Browse files
dmbutkoDmitry ButkoCopilot
authored
fix: use platform path separator in attachment validation (#149)
isValidAttachmentPath() used hardcoded '/' separator which fails on Windows where paths use '\'. Replace with node:path sep to support both platforms. Also fix test to use join() instead of hardcoded '/'. Co-authored-by: Dmitry Butko <dmitrybutko@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1b1948e commit b98a846

2 files changed

Lines changed: 3 additions & 3 deletions

File tree

src/lib/server/ws/attachment-validation.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('isValidAttachmentPath', () => {
4343

4444
it('rejects paths that match the prefix as a substring but are not inside it', () => {
4545
// e.g. /tmp/copilot-uploads-evil/file.txt should not match /tmp/copilot-uploads/
46-
expect(isValidAttachmentPath(UPLOAD_PREFIX + '-evil/file.txt')).toBe(false);
46+
expect(isValidAttachmentPath(join(UPLOAD_PREFIX + '-evil', 'file.txt'))).toBe(false);
4747
});
4848
});
4949

src/lib/server/ws/attachments.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { resolve } from 'node:path';
1+
import { resolve, sep } from 'node:path';
22
import { logSecurity } from '../security-log.js';
33
import { UPLOAD_DIR_PREFIX } from './constants.js';
44

@@ -12,7 +12,7 @@ export type SdkAttachment =
1212
/** Validate that an attachment path is an absolute path inside the upload directory (prevents arbitrary file reads). */
1313
export function isValidAttachmentPath(filePath: string): boolean {
1414
const resolved = resolve(filePath);
15-
return resolved.startsWith(UPLOAD_DIR_PREFIX + '/');
15+
return resolved.startsWith(UPLOAD_DIR_PREFIX + sep);
1616
}
1717

1818
/** Map client-sent attachments to the SDK format, validating paths and filtering invalid entries. */

0 commit comments

Comments
 (0)