-
Notifications
You must be signed in to change notification settings - Fork 289
security: restrict direct attachment access and upload file types #681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
anonymoususer72041
wants to merge
9
commits into
opencats:master
Choose a base branch
from
anonymoususer72041:security/attachments-access-and-upload
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
security: restrict direct attachment access and upload file types #681
anonymoususer72041
wants to merge
9
commits into
opencats:master
from
anonymoususer72041:security/attachments-access-and-upload
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
87603db to
215e59c
Compare
anonymoususer72041
added a commit
to anonymoususer72041/OpenCATS
that referenced
this pull request
Dec 11, 2025
…ypes opencats#681 commit 215e59c Author: anonymoususer72041 <[email protected]> Date: Mon Dec 8 11:25:49 2025 +0100 Avoid static FileUtility access in attachment upload validation commit 0b489bb Author: anonymoususer72041 <[email protected]> Date: Mon Dec 8 09:15:25 2025 +0100 Add server-side whitelist for allowed attachment upload types commit de5c781 Author: anonymoususer72041 <[email protected]> Date: Mon Dec 8 09:02:18 2025 +0100 Deny direct HTTP access to attachments and require download via UI
This reverts commit 215e59c.
Contributor
Author
- merging Master to pickup the CI/CD pipeline
RussH
requested changes
Jan 26, 2026
Member
RussH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great security upgrade. I've got two small suggestions to make it even more robust:
- Template Hardening: Could we wrap the retrievalURL outputs in the templates with htmlspecialchars(..., ENT_QUOTES, 'UTF-8')? It’s a small extra layer to prevent potential XSS from malformed URLs.
- Webserver Documentation: Since this relies on .htaccess, I’ll follow up with a documentation PR once this lands to help Nginx users replicate this protection.
Happy to merge this once those template tweaks are in.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request strengthens the security of attachment handling in two ways.
First, it hardens the
attachments/directory by denying all direct HTTP access via.htaccess. Attachments are therefore no longer accessible via direct URLs underattachments/and must be served through the application layer (AttachmentsUI), where authentication and authorization checks already exist.Second, it introduces a server-side whitelist of allowed file extensions in
AttachmentCreator::createFromUpload(), so only a predefined set of document and image formats can be uploaded. This reduces the risk of storing and serving unexpected or potentially dangerous file types while still supporting common recruiting-related formats.As part of this change, internal UI references were updated to consistently use the existing retrieval URLs instead of direct
attachments/paths, ensuring that downloads and inline previews (such as candidate photos) continue to work with direct access denied.The unused legacy attachment download precheck assets (
js/attachment.jsandajax/getAttachmentLocal.php) and their template includes were removed.Motivation
Securing access to stored attachments
Before this change, files stored under
attachments/could be accessed via direct URLs for specific extensions, which meant that knowledge of the path alone could be sufficient to retrieve a file without passing through OpenCATS authentication and authorization logic.While the paths are not trivial to guess, this does not align with the principle of defense in depth. By denying all direct HTTP access to
attachments/and relying exclusively onAttachmentsUI::getAttachment()for retrieval, attachment access consistently flows through the existing permission checks and is no longer dependent on any URL obfuscation details or web server behaviors.Restricting allowed upload types
Previously there was no server-side restriction on which file types could be uploaded through the attachment upload flow. Any extension that passed basic size and upload error checks was accepted. This increased the risk of storing file types that are unnecessary for typical recruiting workflows and could introduce security issues, especially if they are rendered inline by the browser.
Introducing a server-side whitelist based on common document and image formats narrows the attack surface and prevents the upload of unexpected or potentially dangerous file types. In particular, excluding
htmlhelps mitigate stored cross-site scripting (XSS) risks where uploaded HTML could otherwise be served under the OpenCATS origin and executed in a user's session.