security: fix CQL injection and path traversal vulnerabilities#87
security: fix CQL injection and path traversal vulnerabilities#87yaowei1980 wants to merge 1 commit intopchuri:mainfrom
Conversation
- Add escapeCql() function to sanitize CQL query strings - Escapes backslashes, double quotes, wildcards (*, ?), and fuzzy match (~) - Applied to search() and findPageByTitle() methods - Strengthen sanitizeTitle() to prevent path traversal - Remove '..' sequences - Remove leading dots (hidden files) - Replace filesystem reserved characters with underscore - Add sanitizeFilename() function for safe filename handling - Used in uniquePathFor() for attachment downloads and exports Fixes: CQL injection via search queries, path traversal via page titles and attachment filenames All tests passing (159/159)
pchuri
left a comment
There was a problem hiding this comment.
Thanks for the PR! Security hardening for CQL injection and path traversal is a valuable contribution.
A few comments on the changes:
1. .. removal can be bypassed (Critical)
sanitizeFilename() and sanitizeTitle() both use:
.replace(/\.\./g, '')This is a single-pass replacement, so .... becomes .. after one pass — path traversal is still possible.
A safer approach would be using path.basename() to strip directory components entirely:
function sanitizeFilename(filename) {
if (!filename || typeof filename !== 'string') {
return 'unnamed';
}
return path.basename(filename)
.replace(/[\\/:*?"<>|]/g, '_')
.replace(/^\.+/, '')
.trim() || 'unnamed';
}2. sanitizeTitle behavior change — ' ' → '_'
The existing code replaces reserved characters with a space, but this PR changes it to underscore. This could break compatibility with previously exported directories (e.g., re-exporting a page won't match the existing directory name).
Was this intentional? If so, it might be worth noting as a breaking change.
3. escapeCql — is ~ escape necessary?
~ is a CQL operator (fuzzy match), but inside a quoted string literal it may not need escaping depending on the Confluence version. Could you verify this doesn't cause unexpected search behavior?
4. package-lock.json changes
The lock file includes unrelated dev dependency version bumps (ajv, brace-expansion, flatted, minimatch, picomatch). Consider splitting these into a separate commit or PR to keep the security fix focused.
Add escapeCql() function to sanitize CQL query strings
Strengthen sanitizeTitle() to prevent path traversal
Add sanitizeFilename() function for safe filename handling
Fixes: CQL injection via search queries, path traversal via page titles and attachment filenames
All tests passing (159/159)
Pull Request Template
Description
Brief description of what this PR does.
Type of Change
Testing
Checklist
Screenshots (if applicable)
Add screenshots to help explain your changes.
Additional Context
Add any other context about the pull request here.