fix: enforce ownership checks for attachments and correspondence

JobTrackerApi/Controllers/AttachmentsController.cs

@@ -3,6 +3,7 @@ using Microsoft.EntityFrameworkCore;
 using JobTrackerApi.Data;
 using JobTrackerApi.Models;
 using JobTrackerApi.Services;
+using System.Security.Cryptography;
 
 namespace JobTrackerApi.Controllers
 {
@@ -10,6 +11,13 @@ namespace JobTrackerApi.Controllers
     [Route("api/attachments")]
     public class AttachmentsController : ControllerBase
     {
+        private const long MaxFileSizeBytes = 10 * 1024 * 1024; // 10 MB per file keeps local storage use predictable.
+
+        private static readonly HashSet<string> AllowedExtensions = new(StringComparer.OrdinalIgnoreCase)
+        {
+            ".pdf", ".doc", ".docx", ".txt", ".rtf", ".png", ".jpg", ".jpeg", ".webp"
+        };
+
         private readonly AppPaths _paths;
         private readonly JobTrackerContext _db;
 
@@ -21,6 +29,23 @@ namespace JobTrackerApi.Controllers
 
         public sealed record AttachmentDto(int Id, string FileName, DateTime UploadDate, string FileType, long FileSize);
 
+        // Child entities are accessed by raw integer ids in a few endpoints below.
+        // Always resolve them through the parent JobApplication query so the global job-level
+        // ownership filter is still enforced for multi-user environments.
+        private Task<Attachment?> FindOwnedAttachmentAsync(int attachmentId, CancellationToken cancellationToken)
+        {
+            return _db.Attachments
+                .Include(a => a.JobApplication)
+                .FirstOrDefaultAsync(a => a.Id == attachmentId, cancellationToken);
+        }
+
+        private static string BuildStoredFileName(string originalName)
+        {
+            var ext = Path.GetExtension(originalName);
+            var suffix = Convert.ToHexString(RandomNumberGenerator.GetBytes(6)).ToLowerInvariant();
+            return $"{DateTime.UtcNow:yyyyMMddHHmmssfff}-{suffix}{ext}";
+        }
+
         [HttpGet("{jobId:int}")]
         public async Task<ActionResult<List<AttachmentDto>>> ListForJob([FromRoute] int jobId, CancellationToken cancellationToken)
         {
@@ -40,7 +65,7 @@ namespace JobTrackerApi.Controllers
         [HttpGet("download/{id:int}")]
         public async Task<IActionResult> Download([FromRoute] int id, CancellationToken cancellationToken)
         {
-            var att = await _db.Attachments.AsNoTracking().FirstOrDefaultAsync(a => a.Id == id, cancellationToken);
+            var att = await FindOwnedAttachmentAsync(id, cancellationToken);
             if (att is null) return NotFound();
 
             if (string.IsNullOrWhiteSpace(att.FilePath) || !System.IO.File.Exists(att.FilePath))
@@ -56,18 +81,22 @@ namespace JobTrackerApi.Controllers
         [HttpPatch("{id:int}")]
         public async Task<IActionResult> Rename([FromRoute] int id, [FromBody] RenameAttachmentRequest request, CancellationToken cancellationToken)
         {
-            var att = await _db.Attachments.FirstOrDefaultAsync(a => a.Id == id, cancellationToken);
+            var att = await FindOwnedAttachmentAsync(id, cancellationToken);
             if (att is null) return NotFound();
 
             var name = Path.GetFileName((request.FileName ?? "").Trim());
             if (name.Length == 0) return BadRequest("FileName is required.");
 
+            var ext = Path.GetExtension(name);
+            if (!AllowedExtensions.Contains(ext))
+                return BadRequest("That file type is not allowed.");
+
             var folder = Path.GetDirectoryName(att.FilePath) ?? _paths.AttachmentsRoot;
-            var newPath = Path.Combine(folder, name);
+            var newPath = Path.Combine(folder, BuildStoredFileName(name));
 
             if (System.IO.File.Exists(att.FilePath) && !string.Equals(att.FilePath, newPath, StringComparison.OrdinalIgnoreCase))
             {
-                System.IO.File.Move(att.FilePath, newPath, overwrite: true);
+                System.IO.File.Move(att.FilePath, newPath, overwrite: false);
             }
 
             att.FileName = name;
@@ -80,7 +109,7 @@ namespace JobTrackerApi.Controllers
         [HttpDelete("{id:int}")]
         public async Task<IActionResult> Delete([FromRoute] int id, CancellationToken cancellationToken)
         {
-            var att = await _db.Attachments.FirstOrDefaultAsync(a => a.Id == id, cancellationToken);
+            var att = await FindOwnedAttachmentAsync(id, cancellationToken);
             if (att is null) return NotFound();
 
             var path = att.FilePath;
@@ -115,16 +144,25 @@ namespace JobTrackerApi.Controllers
             foreach (var file in files)
             {
                 if (file.Length == 0) continue;
+                if (file.Length > MaxFileSizeBytes)
+                    return BadRequest($"{file.FileName} exceeds the 10 MB upload limit.");
 
-                var safeName = Path.GetFileName(file.FileName);
-                var path = Path.Combine(folder, safeName);
-                await using var stream = new FileStream(path, FileMode.Create);
+                var displayName = Path.GetFileName(file.FileName);
+                var ext = Path.GetExtension(displayName);
+                if (!AllowedExtensions.Contains(ext))
+                    return BadRequest($"{displayName} is not an allowed file type.");
+
+                // Store uploads under unique generated filenames so re-uploads never overwrite
+                // earlier files with the same visible name.
+                var storedName = BuildStoredFileName(displayName);
+                var path = Path.Combine(folder, storedName);
+                await using var stream = new FileStream(path, FileMode.CreateNew, FileAccess.Write, FileShare.None);
                 await file.CopyToAsync(stream, cancellationToken);
 
                 _db.Attachments.Add(new Attachment
                 {
                     JobApplicationId = jobId,
-                    FileName = safeName,
+                    FileName = displayName,
                     FilePath = path,
                     UploadDate = DateTime.Now,
                     FileType = string.IsNullOrWhiteSpace(file.ContentType) ? "application/octet-stream" : file.ContentType,

JobTrackerApi/Controllers/CorrespondenceController.cs

@@ -16,6 +16,15 @@ namespace JobTrackerApi.Controllers
             _db = db;
         }
 
+        // Resolve correspondence through its parent job so the DbContext's user-scoped
+        // job filter still protects raw id endpoints in multi-user deployments.
+        private Task<Correspondence?> FindOwnedMessageAsync(int correspondenceId, CancellationToken cancellationToken)
+        {
+            return _db.Correspondences
+                .Include(c => c.JobApplication)
+                .FirstOrDefaultAsync(c => c.Id == correspondenceId, cancellationToken);
+        }
+
         // GET all messages for a job
         [HttpGet("{jobId:int}")]
         public async Task<ActionResult<List<Correspondence>>> GetForJob([FromRoute] int jobId, CancellationToken cancellationToken)
@@ -72,7 +81,7 @@ namespace JobTrackerApi.Controllers
         [HttpDelete("{id:int}")]
         public async Task<IActionResult> Delete([FromRoute] int id, CancellationToken cancellationToken)
         {
-            var message = await _db.Correspondences.FirstOrDefaultAsync(c => c.Id == id, cancellationToken);
+            var message = await FindOwnedMessageAsync(id, cancellationToken);
             if (message is null) return NotFound();
 
             _db.Correspondences.Remove(message);