Add email retention sweep and harden untrusted attachment metadata - #46
Merged
Merged
Conversation
Captured emails were kept forever, so the database grew without bound. Add a daily retention job that deletes emails older than EMAIL_RETENTION_DAYS (default 7). Attachment rows cascade with the email but their files on the mounted volume do not, so the job unlinks those first; a missing file is logged and skipped rather than aborting the sweep. Also drop the demo cleanup cron from every 5 minutes to every 30. The demo TTL is 60 minutes, so 30 is still timely, and the old interval kept a connection busy around the clock for a table that is almost always empty. Move ScheduleModule.forRoot() from DemoModule to AppModule. Every cron in the app depended on it being registered there, so disabling the demo module would have silently stopped all scheduled jobs. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Attachment filenames and content types come from the MIME headers of inbound mail, which anyone can send to the public SMTP listener, so they must be treated as untrusted input. Filenames were interpolated straight into the storage path. The timestamp prefix only absorbed the first path segment, so a crafted name could still resolve outside the uploads directory. safeAttachmentPath() now keeps only the basename, rejects null bytes, and verifies the result stays inside uploadsDir before anything is written. The download endpoint reflected the same values into response headers. The content type is now constrained to a well-formed, non-renderable mime type (falling back to application/octet-stream) and the filename is stripped of quotes, backslashes and control characters so it cannot escape the Content-Disposition parameter. Added X-Content-Type-Options: nosniff. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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
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
Two related changes to the API's email handling.
Email retention. Captured emails were kept forever, so storage grew without bound. A daily job now deletes emails older than
EMAIL_RETENTION_DAYS(default 7). Attachment rows cascade with the email, but their files on the mounted volume do not, so the job unlinks those first; a missing file is logged and skipped rather than aborting the sweep.Scheduling cleanup. The demo cleanup cron drops from every 5 minutes to every 30. The demo TTL is 60 minutes, so cleanup stays timely, while the old interval kept a database connection busy around the clock for a table that is almost always empty.
ScheduleModule.forRoot()also moves fromDemoModuletoAppModule— every cron in the app depended on it being registered inside the demo module, so disabling that module would have silently stopped all scheduled jobs.Attachment metadata hardening. Attachment filenames and content types come from the MIME headers of inbound mail, which anyone can send to the public SMTP listener, so they are untrusted input:
safeAttachmentPath()now keeps only the basename, rejects null bytes, and verifies the result stays withinuploadsDirbefore anything is written.application/octet-stream), the filename is stripped of quotes, backslashes and control characters so it cannot escape theContent-Dispositionparameter, andX-Content-Type-Options: nosniffis set.Testing
attachment-path.spec.ts,attachment-headers.spec.ts,email-retention.service.spec.ts)nest buildcleanNotes
Configuration adds one variable, documented in
.env.example:Pre-existing prettier errors in
src/main.tsand atemails.service.ts:189are left untouched to keep the diff focused.🤖 Generated with Claude Code