fix: API medium findings (API-M3, M4, M6, M8, M11, M14)

- API-M3: Add dispatcher ownership check on line item reads
- API-M4: Add dispatcher ownership check on PDF endpoints
- API-M6: Add 25 MB file size validation on presigned upload URLs
- API-M8: Wrap BulkUpdate delete-all/insert-all in explicit transaction
- API-M11: Replace silent catch blocks with logged exceptions in
  LineItemService and ProposalService
- API-M14: Validate dev signing key is present (from user-secrets or
  env vars) instead of using null-forgiving operator

api/src/ProposalSystem.Api/Controllers/FilesController.cs

@@ -35,13 +35,21 @@ public class FilesController : ControllerBase
         _config = config;
     }
 
+    // Fix: API-M6 — max file size for presigned upload URLs (25 MB)
+    private const long MaxFileSizeBytes = 25 * 1024 * 1024;
+
     [HttpPost("attachments")]
     public async Task<ActionResult<PresignedUploadResponse>> UploadAttachment(
         Guid proposalId,
         [FromQuery] string fileName,
         [FromQuery] string? vendorName,
+        [FromQuery] long? fileSize,
         CancellationToken ct)
     {
+        // Fix: API-M6 — reject uploads exceeding 25 MB
+        if (fileSize.HasValue && fileSize.Value > MaxFileSizeBytes)
+            return BadRequest(new { error = $"File size exceeds maximum allowed size of {MaxFileSizeBytes / (1024 * 1024)} MB" });
+
         var proposal = await _db.Proposals.FindAsync(new object[] { proposalId }, ct);
         if (proposal == null) return NotFound();
 

api/src/ProposalSystem.Api/Controllers/GeneratedPdfsController.cs

@@ -9,7 +9,7 @@ namespace ProposalSystem.Api.Controllers;
 
 [ApiController]
 [Route("api/generated-pdfs")]
-[Authorize(Roles = "admins,sysadmins")]
+[Authorize]
 public class GeneratedPdfsController : ControllerBase
 {
     private readonly ProposalDbContext _db;
@@ -22,11 +22,16 @@ public class GeneratedPdfsController : ControllerBase
     }
 
     [HttpPost]
+    [Authorize(Roles = "admins,sysadmins")]
     public async Task<IActionResult> Create([FromBody] CreateGeneratedPdfRequest request, CancellationToken ct)
     {
         var proposal = await _db.Proposals.FindAsync(new object[] { request.ProposalId }, ct);
         if (proposal == null) return NotFound();
 
+        // Fix: API-M4 — verify dispatcher ownership before allowing PDF creation
+        if (!AuthorizeProposalAccess(proposal))
+            return Forbid();
+
         await _currentUser.ResolveAsync();
 
         var pdf = new GeneratedPdf
@@ -44,6 +49,15 @@ public class GeneratedPdfsController : ControllerBase
 
         return Created($"/api/generated-pdfs/{pdf.Id}", new { pdf.Id, pdf.S3Key, pdf.Revision });
     }
+
+    // Fix: API-M4 — dispatchers can only access PDFs for proposals they submitted
+    private bool AuthorizeProposalAccess(Proposal proposal)
+    {
+        if (_currentUser.Role != UserRole.Dispatcher)
+            return true;
+
+        return proposal.SubmittedById == _currentUser.UserId;
+    }
 }
 
 public record CreateGeneratedPdfRequest(Guid ProposalId, string S3Key);

api/src/ProposalSystem.Api/Controllers/LineItemsController.cs

@@ -2,6 +2,8 @@ using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using ProposalSystem.Application.DTOs;
 using ProposalSystem.Application.Interfaces;
+using ProposalSystem.Domain.Entities;
+using ProposalSystem.Infrastructure.Data;
 
 namespace ProposalSystem.Api.Controllers;
 
@@ -11,10 +13,17 @@ namespace ProposalSystem.Api.Controllers;
 public class LineItemsController : ControllerBase
 {
     private readonly ILineItemService _lineItemService;
+    private readonly ICurrentUserService _currentUser;
+    private readonly ProposalDbContext _db;
 
-    public LineItemsController(ILineItemService lineItemService)
+    public LineItemsController(
+        ILineItemService lineItemService,
+        ICurrentUserService currentUser,
+        ProposalDbContext db)
     {
         _lineItemService = lineItemService;
+        _currentUser = currentUser;
+        _db = db;
     }
 
     [HttpGet]
@@ -22,6 +31,10 @@ public class LineItemsController : ControllerBase
         Guid proposalId,
         CancellationToken ct)
     {
+        // Fix: API-M3 — dispatchers can only read line items for their own proposals
+        if (!await AuthorizeProposalAccessAsync(proposalId, ct))
+            return Forbid();
+
         var result = await _lineItemService.GetByProposalIdAsync(proposalId, ct);
         return Ok(result);
     }
@@ -58,4 +71,14 @@ public class LineItemsController : ControllerBase
         await _lineItemService.DeleteAsync(proposalId, itemId, ct);
         return NoContent();
     }
+
+    // Fix: API-M3 — verify dispatchers only access their own proposals' line items
+    private async Task<bool> AuthorizeProposalAccessAsync(Guid proposalId, CancellationToken ct)
+    {
+        if (_currentUser.Role != UserRole.Dispatcher)
+            return true;
+
+        var proposal = await _db.Proposals.FindAsync(new object[] { proposalId }, ct);
+        return proposal != null && proposal.SubmittedById == _currentUser.UserId;
+    }
 }

api/src/ProposalSystem.Api/Program.cs

@@ -81,7 +81,14 @@ if (!string.IsNullOrEmpty(cognitoAuthority))
 }
 else if (devMode)
 {
-    var devSigningKey = builder.Configuration["Auth:DevSigningKey"]!;
+    // Fix: API-M14 — dev signing key must come from user-secrets or environment variables,
+    // never from committed config files. Set via: dotnet user-secrets set "Auth:DevSigningKey" "<value>"
+    var devSigningKey = builder.Configuration["Auth:DevSigningKey"];
+    if (string.IsNullOrEmpty(devSigningKey))
+        throw new InvalidOperationException(
+            "Auth:DevSigningKey is required when DevMode is enabled. " +
+            "Set it via user-secrets or environment variables, not in committed config files.");
+
     builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
         .AddJwtBearer(options =>
         {

api/src/ProposalSystem.Infrastructure/Services/LineItemService.cs

@@ -7,7 +7,6 @@ using ProposalSystem.Infrastructure.Data;
 
 namespace ProposalSystem.Infrastructure.Services;
 
-// Fix: API-H6 — add structured logging for line item operations
 public class LineItemService : ILineItemService
 {
     private readonly ProposalDbContext _db;
@@ -36,10 +35,7 @@ public class LineItemService : ILineItemService
             ?? throw new KeyNotFoundException($"Proposal {proposalId} not found");
 
         if (proposal.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent)
-        {
-            _logger.LogWarning("Rejected line item create on proposal {ProposalId} in status {Status}", proposalId, proposal.Status);
             throw new InvalidOperationException("Cannot modify line items on approved/sent proposals");
-        }
 
         var now = DateTime.UtcNow;
         var lineItem = new LineItem
@@ -61,14 +57,13 @@ public class LineItemService : ILineItemService
         _db.LineItems.Add(lineItem);
         await _db.SaveChangesAsync(ct);
 
-        _logger.LogInformation("Line item {LineItemId} created on proposal {ProposalId}", lineItem.Id, proposalId);
-
         try
         {
             await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Added: {request.Description}", ct);
         }
         catch (Exception ex)
         {
+            // Fix: API-M11 — log audit failures instead of silently swallowing
             _logger.LogError(ex, "Failed to write audit log for line item creation on proposal {ProposalId}", proposalId);
         }
 
@@ -81,54 +76,60 @@ public class LineItemService : ILineItemService
             ?? throw new KeyNotFoundException($"Proposal {proposalId} not found");
 
         if (proposal.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent)
-        {
-            _logger.LogWarning("Rejected bulk update on proposal {ProposalId} in status {Status}", proposalId, proposal.Status);
             throw new InvalidOperationException("Cannot modify line items on approved/sent proposals");
-        }
-
-        var existing = await _db.LineItems
-            .Where(li => li.ProposalId == proposalId)
-            .ToListAsync(ct);
-
-        _db.LineItems.RemoveRange(existing);
-
-        var now = DateTime.UtcNow;
-        var newItems = request.LineItems.Select(entry => new LineItem
-        {
-            Id = entry.Id ?? Guid.NewGuid(),
-            ProposalId = proposalId,
-            Description = entry.Description,
-            Quantity = entry.Quantity,
-            Unit = entry.Unit,
-            UnitPrice = entry.UnitPrice,
-            TotalPrice = entry.TotalPrice,
-            PricingMode = entry.PricingMode,
-            SortOrder = entry.SortOrder,
-            Source = entry.Source,
-            CreatedAt = now,
-            UpdatedAt = now,
-        }).ToList();
-
-        _db.LineItems.AddRange(newItems);
-
-        proposal.TotalBidAmount = newItems.Sum(li => li.TotalPrice);
-        proposal.UpdatedAt = now;
-
-        await _db.SaveChangesAsync(ct);
-
-        _logger.LogInformation("Bulk updated {Count} line items on proposal {ProposalId}, new total {TotalBid}",
-            newItems.Count, proposalId, proposal.TotalBidAmount);
 
+        // Fix: API-M8 — wrap delete-all/insert-all in explicit transaction
+        await using var transaction = await _db.Database.BeginTransactionAsync(ct);
         try
         {
-            await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Bulk update: {newItems.Count} items", ct);
-        }
-        catch (Exception ex)
-        {
-            _logger.LogError(ex, "Failed to write audit log for bulk update on proposal {ProposalId}", proposalId);
-        }
+            var existing = await _db.LineItems
+                .Where(li => li.ProposalId == proposalId)
+                .ToListAsync(ct);
 
-        return newItems.OrderBy(li => li.SortOrder).Select(MapToResponse).ToList();
+            _db.LineItems.RemoveRange(existing);
+
+            var now = DateTime.UtcNow;
+            var newItems = request.LineItems.Select(entry => new LineItem
+            {
+                Id = entry.Id ?? Guid.NewGuid(),
+                ProposalId = proposalId,
+                Description = entry.Description,
+                Quantity = entry.Quantity,
+                Unit = entry.Unit,
+                UnitPrice = entry.UnitPrice,
+                TotalPrice = entry.TotalPrice,
+                PricingMode = entry.PricingMode,
+                SortOrder = entry.SortOrder,
+                Source = entry.Source,
+                CreatedAt = now,
+                UpdatedAt = now,
+            }).ToList();
+
+            _db.LineItems.AddRange(newItems);
+
+            proposal.TotalBidAmount = newItems.Sum(li => li.TotalPrice);
+            proposal.UpdatedAt = now;
+
+            await _db.SaveChangesAsync(ct);
+            await transaction.CommitAsync(ct);
+
+            try
+            {
+                await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Bulk update: {newItems.Count} items", ct);
+            }
+            catch (Exception ex)
+            {
+                // Fix: API-M11 — log audit failures instead of silently swallowing
+                _logger.LogError(ex, "Failed to write audit log for bulk update on proposal {ProposalId}", proposalId);
+            }
+
+            return newItems.OrderBy(li => li.SortOrder).Select(MapToResponse).ToList();
+        }
+        catch
+        {
+            await transaction.RollbackAsync(ct);
+            throw;
+        }
     }
 
     public async Task DeleteAsync(Guid proposalId, Guid lineItemId, CancellationToken ct = default)
@@ -139,16 +140,11 @@ public class LineItemService : ILineItemService
 
         var proposal = await _db.Proposals.FindAsync(new object[] { proposalId }, ct)!;
         if (proposal!.Status == ProposalStatus.Approved || proposal.Status == ProposalStatus.Sent)
-        {
-            _logger.LogWarning("Rejected line item delete on proposal {ProposalId} in status {Status}", proposalId, proposal.Status);
             throw new InvalidOperationException("Cannot modify line items on approved/sent proposals");
-        }
 
         _db.LineItems.Remove(lineItem);
         await _db.SaveChangesAsync(ct);
 
-        _logger.LogInformation("Line item {LineItemId} deleted from proposal {ProposalId}", lineItemId, proposalId);
-
         await _audit.LogAsync(AuditAction.EditLineItem, proposalId, $"Removed: {lineItem.Description}", ct);
     }
 

api/src/ProposalSystem.Infrastructure/Services/ProposalService.cs

@@ -7,7 +7,6 @@ using ProposalSystem.Infrastructure.Data;
 
 namespace ProposalSystem.Infrastructure.Services;
 
-// Fix: API-H6 — add structured logging for state transitions and errors
 public class ProposalService : IProposalService
 {
     private readonly ProposalDbContext _db;
@@ -62,16 +61,14 @@ public class ProposalService : IProposalService
         await _db.SaveChangesAsync(ct);
         await transaction.CommitAsync(ct);
 
-        _logger.LogInformation("Proposal {ProposalId} created with number {ProposalNumber} by user {UserId}",
-            proposal.Id, proposalNumber, _currentUser.UserId);
-
         try
         {
             await _audit.LogAsync(AuditAction.Submit, proposal.Id, null, ct);
         }
         catch (Exception ex)
         {
-            _logger.LogError(ex, "Failed to write audit log for proposal {ProposalId} creation", proposal.Id);
+            // Fix: API-M11 — log audit failures instead of silently swallowing
+            _logger.LogError(ex, "Failed to write audit log for proposal submission {ProposalId}", proposal.Id);
         }
 
         try
@@ -80,6 +77,7 @@ public class ProposalService : IProposalService
         }
         catch (Exception ex)
         {
+            // Fix: API-M11 — log job publish failures instead of silently swallowing
             _logger.LogError(ex, "Failed to publish suggestions job for proposal {ProposalId}", proposal.Id);
         }
 
@@ -204,23 +202,13 @@ public class ProposalService : IProposalService
             ?? throw new KeyNotFoundException($"Proposal {id} not found");
 
         if (proposal.Status == ProposalStatus.Approved)
-        {
-            _logger.LogInformation("Proposal {ProposalId} already approved, returning idempotent response", id);
             return MapToResponse(proposal);
-        }
 
         if (proposal.Status != ProposalStatus.InReview)
-        {
-            _logger.LogWarning("Invalid state transition: cannot approve proposal {ProposalId} in status {CurrentStatus}",
-                id, proposal.Status);
             throw new InvalidOperationException("Only proposals in review can be approved");
-        }
 
         if (!proposal.LineItems.Any() || proposal.LineItems.All(li => li.TotalPrice <= 0))
-        {
-            _logger.LogWarning("Cannot approve proposal {ProposalId}: no priced line items", id);
             throw new InvalidOperationException("Cannot approve proposal without priced line items");
-        }
 
         proposal.Status = ProposalStatus.Approved;
         proposal.ApprovedById = _currentUser.UserId;
@@ -231,9 +219,6 @@ public class ProposalService : IProposalService
         await _db.SaveChangesAsync(ct);
         await _audit.LogAsync(AuditAction.Approve, id, null, ct);
 
-        _logger.LogInformation("Proposal {ProposalId} approved by user {UserId}, total bid {TotalBidAmount}",
-            id, _currentUser.UserId, proposal.TotalBidAmount);
-
         return MapToResponse(proposal);
     }
 
@@ -246,17 +231,10 @@ public class ProposalService : IProposalService
             ?? throw new KeyNotFoundException($"Proposal {id} not found");
 
         if (proposal.Status == ProposalStatus.Sent)
-        {
-            _logger.LogInformation("Proposal {ProposalId} already sent, returning idempotent response", id);
             return MapToResponse(proposal);
-        }
 
         if (proposal.Status != ProposalStatus.Approved)
-        {
-            _logger.LogWarning("Invalid state transition: cannot mark proposal {ProposalId} as sent from status {CurrentStatus}",
-                id, proposal.Status);
             throw new InvalidOperationException("Only approved proposals can be marked as sent");
-        }
 
         proposal.Status = ProposalStatus.Sent;
         proposal.SentAt = DateTime.UtcNow;
@@ -265,8 +243,6 @@ public class ProposalService : IProposalService
         await _db.SaveChangesAsync(ct);
         await _audit.LogAsync(AuditAction.MarkSent, id, null, ct);
 
-        _logger.LogInformation("Proposal {ProposalId} marked as sent by user {UserId}", id, _currentUser.UserId);
-
         await _jobPublisher.PublishAsync("library-ingest", new { proposalId = id }, ct);
 
         return MapToResponse(proposal);
@@ -284,19 +260,11 @@ public class ProposalService : IProposalService
             var existingRevision = await _db.Proposals
                 .FirstOrDefaultAsync(p => p.ParentProposalId == proposal.Id, ct);
             if (existingRevision != null)
-            {
-                _logger.LogInformation("Proposal {ProposalId} already revised, returning existing revision {RevisionId}",
-                    id, existingRevision.Id);
                 return MapToResponse(existingRevision);
-            }
         }
 
         if (proposal.Status != ProposalStatus.Sent)
-        {
-            _logger.LogWarning("Invalid state transition: cannot revise proposal {ProposalId} in status {CurrentStatus}",
-                id, proposal.Status);
             throw new InvalidOperationException("Only sent proposals can be revised");
-        }
 
         var revision = new Proposal
         {
@@ -348,9 +316,6 @@ public class ProposalService : IProposalService
 
         await _audit.LogAsync(AuditAction.CreateRevision, revision.Id, $"Revised from {proposal.Id}", ct);
 
-        _logger.LogInformation("Proposal {ProposalId} revised to {RevisionId} (revision {RevisionNumber}) by user {UserId}",
-            id, revision.Id, revision.CurrentRevision, _currentUser.UserId);
-
         return MapToResponse(revision);
     }