fix(pay-slip)[backend]: add file type validation in UploadFile handler#41
fix(pay-slip)[backend]: add file type validation in UploadFile handler#41MohamadNazik wants to merge 1 commit intoLSFLK:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the pay-slip upload functionality by introducing strict file type validation. It ensures that only specified document and image formats can be uploaded, preventing potential issues from invalid file associations and improving data integrity. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully implements file type validation for uploads, addressing issue #30. The changes include updating the OpenAPI specification to reflect the supported formats, centralizing the list of allowed extensions in constants.go, and adding robust validation logic in the UploadFile handler. The validation correctly performs a case-insensitive check on file extensions and returns an appropriate 400 Bad Request for unsupported types. This is a good improvement for the application's security and data integrity.
79f749b to
c483ef6
Compare
- Sniff first 512 bytes for actual content type to prevent extension bypass. - Centralize extension/MIME mapping into a new internal/utils helper. - Ensure immutability for AllowedExtensions using a private slice and public getter. - Improve error message formatting for better user experience. - Refactor Storage layer to accept content type directly from the handler.
c483ef6 to
b0bbf5b
Compare
| ) | ||
|
|
||
| // GetValidatedMimeType checks if the extension is allowed and returns the MIME type. | ||
| func GetValidatedMimeType(filename string) (string, bool) { |
There was a problem hiding this comment.
Extension-based validation can still be spoofed. To make this more robust: use a centralized map for lookups (remove the switch/case) + add content-sniffing support. Here's how you can rewrite this mime_utils.go
// mimeMapping provides a single source of truth for allowed extensions and their canonical MIME types
var mimeMapping = map[string]string{
".pdf": "application/pdf",
".png": "image/png",
".jpg": "image/jpeg",
".jpeg": "image/jpeg",
}
// Check the filename extension and actual content (magic bytes)
// Returns MIME type and a boolean indicating if the file is valid
func ValidateFile(filename string, content []byte) (string, bool) {
ext := strings.ToLower(filepath.Ext(filename))
// Validate Extension against our known allowed list
canonicalMime, supported := mimeMapping[ext]
if !supported {
return "", false
}
// Validate actual content for security
// http.DetectContentType uses the first 512 bytes to determine the MIME type.
detectedMime := http.DetectContentType(content)
// Clean detected MIME (remove charset info if present)
if parts := strings.Split(detectedMime, ";"); len(parts) > 0 {
detectedMime = parts[0]
}
// Make sure the extension matches the actual content type
// This prevents a user from uploading a .exe renamed to .pdf
if detectedMime != canonicalMime {
return "", false
}
return canonicalMime, true
}
// GetMimeByExtension is a helper for cases where only the filename is available
func GetMimeByExtension(filename string) (string, bool) {
ext := strings.ToLower(filepath.Ext(filename))
mime, allowed := mimeMapping[ext]
return mime, allowed
}
Summary
This PR fixes the issue #30 by adding file type validation to the upload endpoint. The API now restricts uploads to supported formats to prevent invalid file associations.
Changes
.pdf,.png,.jpg, and.jpeg(case-insensitive).400 Bad Request.Verification
Closes #30