Documentation Updates

This commit is contained in:
Eric Gullickson
2025-12-14 12:00:42 -06:00
parent 183d55b0fe
commit 3852ec9f71
11 changed files with 217 additions and 713 deletions

393
docs/AUDIT.md Normal file
View File

@@ -0,0 +1,393 @@
# MotoVaultPro Software Audit Report
**Document Version:** 1.0
**Classification:** Internal/Confidential
**Audit Date:** December 13, 2025
**Application Version:** 1.0.0
**Architecture:** 5-Container Single-Tenant Docker Stack
---
## 1. Executive Summary
### 1.1 High-Level Assessment
**MotoVaultPro** is an Australian automotive vehicle management platform built on a modern 5-container Docker architecture. The application demonstrates solid architectural foundations with proper authentication, modular feature design, and production-ready deployment configuration.
### 1.2 Key Findings Summary
| Category | Rating | Critical | High | Medium | Low |
|----------|--------|----------|------|--------|-----|
| **Security** | 6.5/10 | 1 | 2 | 1 | 2 |
| **Code Quality** | 8.3/10 | 0 | 0 | 2 | 2 |
| **Architecture** | 8.5/10 | 0 | 0 | 1 | 1 |
| **Data Integrity** | 7.5/10 | 0 | 1 | 1 | 0 |
| **Performance** | 8.0/10 | 0 | 0 | 1 | 1 |
| **Compliance** | 7.0/10 | 0 | 1 | 1 | 1 |
### 1.3 Overall Production Readiness
**Verdict: CONDITIONALLY READY**
The application requires remediation of **1 critical** and **2 high-severity** security issues before production deployment:
1. **CRITICAL**: Cryptographically insecure random number generation for document storage keys
2. **HIGH**: Inadequate file upload validation (no magic byte verification)
3. **HIGH**: Google Maps API key exposure to frontend clients
---
## 2. Scope and Methodology
### 2.1 Audit Scope
**In Scope:**
- Source code review (backend/src, frontend/src)
- Docker/container configuration analysis
- Database schema and migration review
- Authentication and authorization mechanisms
- API security assessment
- Infrastructure security review
- Compliance assessment for Australian automotive context
**Out of Scope:**
- External penetration testing
- Third-party dependency deep audit (surface-level only)
- Production infrastructure assessment
- Physical security controls
### 2.2 Methodology
**Standards Applied:**
- OWASP Top 10 2021
- OWASP API Security Top 10 2023
- Australian Privacy Act 1988 considerations
- ACCC consumer data protection guidelines
**Tools and Techniques:**
- Manual code review
- Static analysis via TypeScript compiler (strict mode)
- Configuration file analysis
- Architecture documentation review
- Test coverage analysis
### 2.3 Components Reviewed
| Component | Path | Status |
|-----------|------|--------|
| Backend API | `backend/src/` | Full review |
| Frontend SPA | `frontend/src/` | Full review |
| Docker Config | `docker-compose.yml` | Full review |
| Traefik Config | `config/traefik/` | Full review |
| Database Migrations | `backend/src/features/*/migrations/` | Full review |
### 2.4 Feature Capsules Reviewed
| Feature | Test Coverage | Implementation Status |
|---------|---------------|----------------------|
| vehicles | Full suite | Complete |
| platform | Unit + Integration | Complete |
| documents | Unit + Integration | Complete |
| fuel-logs | Basic tests | Complete |
| maintenance | Basic tests | Complete |
| stations | Basic tests | Partial |
| admin | Unit + Integration | Complete |
---
## 3. Detailed Findings
### 3.a Architecture and Environment
**Rating: 8.5/10**
#### Strengths
- **5-Container Architecture**: Well-designed Docker deployment with Traefik, Frontend, Backend, PostgreSQL, and Redis
- **Feature Capsule Pattern**: Self-contained modules in `backend/src/features/{name}/` enable clear separation
- **K8s-Ready Design**: Health probes (`/health`, `/health/ready`, `/health/live`, `/health/startup`), container orchestration, secret management via mounted files
- **Network Segmentation**: Three Docker networks with appropriate isolation:
- `frontend` (10.96.1.0/24) - Public-facing
- `backend` (10.96.20.0/24) - API services
- `database` (10.96.64.0/24) - Internal only
- **Production-Only Containers**: All services run production builds
#### Concerns
| Severity | Finding | Location |
|----------|---------|----------|
| MEDIUM | Single-tenant limitations (no multi-tenancy support) | Architecture design |
| LOW | Platform module tightly coupled to backend | `backend/src/features/platform/` |
#### Key Files
- `/docker-compose.yml` - Container orchestration
- `/config/traefik/traefik.yml` - Reverse proxy configuration
- `/config/traefik/middleware.yml` - Security middleware
- `/backend/src/features/` - Feature capsule structure
---
### 3.b Code Quality and Maintainability
**Rating: 8.3/10**
#### Strengths
- **Strict TypeScript Configuration**: All strict compiler options enabled
```json
{
"strict": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"exactOptionalPropertyTypes": true,
"noUncheckedIndexedAccess": true
}
```
- **Test Coverage**: 5,872 lines of test code across 15+ test files
- **Structured Logging**: Winston logger with JSON format
- **Input Validation**: Zod schemas for all API inputs
- **Repository Pattern**: Clean data access separation
#### Concerns
| Severity | Finding | Impact |
|----------|---------|--------|
| MEDIUM | 299 uses of `any` type in backend | Reduced type safety |
| MEDIUM | No ESLint configuration in backend | Inconsistent code style |
| LOW | No Prettier configuration | Formatting inconsistency |
| LOW | Missing React Error Boundaries on some pages | Unhandled UI errors |
#### Key Files
- `/backend/tsconfig.json` - TypeScript strict configuration
- `/frontend/eslint.config.js` - Frontend linting (backend missing)
- `/backend/jest.config.js` - Test configuration
---
### 3.c Data Integrity and Functional Accuracy
**Rating: 7.5/10**
#### Strengths
- **Referential Integrity**: Foreign key constraints with `ON DELETE CASCADE`
- **Soft Deletes**: `deleted_at` column on vehicles for audit trail
- **VIN Validation**: Check digit algorithm at application layer
- **Unique Constraints**: `UNIQUE(user_id, vin)` prevents duplicates
- **Migration Tracking**: `_migrations` table prevents re-execution
#### Concerns
| Severity | Finding | Risk |
|----------|---------|------|
| HIGH | No PostgreSQL Row-Level Security (RLS) | Cross-user data exposure if SQL injection occurs |
| MEDIUM | Soft delete orphan risk (cascading deletes are hard deletes) | Data consistency issues |
#### Key Files
- `/backend/src/features/vehicles/migrations/` - Schema definitions
- `/backend/src/features/vehicles/data/vehicles.repository.ts` - Data access
---
### 3.d Security Assessment
**Rating: 6.5/10**
#### Strengths
- **Auth0 OIDC Integration**: Industry-standard authentication via `@fastify/jwt` and `get-jwks`
- **JWT Validation**: JWKS-based public key retrieval with issuer validation
- **HTTPS Enforcement**: Traefik automatic HTTP-to-HTTPS redirect
- **Security Headers**: HSTS, X-Frame-Options, X-Content-Type-Options via Traefik
- **Docker Secrets**: Password management via `/run/secrets/` pattern
- **Rate Limiting**: 50 req/min average, 100 burst via Traefik
- **Admin Guard**: Role-based access control for admin endpoints
#### Critical Finding
**CRITICAL: Insecure Random Number Generation**
| Attribute | Value |
|-----------|-------|
| Location | `backend/src/features/documents/api/documents.controller.ts:321-324` |
| Issue | Document storage keys generated using `Math.random()` |
| Code | `Math.random().toString(36).slice(2) + Date.now().toString(36)` |
| Risk | Predictable file paths could allow unauthorized document access |
| CVSS Score | 8.1 (High) |
| Remediation | Replace with `crypto.randomBytes(32).toString('hex')` |
#### High Severity Findings
**HIGH: Inadequate File Upload Validation**
| Attribute | Value |
|-----------|-------|
| Location | `backend/src/features/documents/api/documents.controller.ts:205-216` |
| Issue | MIME type validation based on Content-Type header only |
| Risk | Malicious files with spoofed Content-Type bypass validation |
| CVSS Score | 7.5 (High) |
| Remediation | Implement magic byte (file signature) validation |
**HIGH: API Key Exposure**
| Attribute | Value |
|-----------|-------|
| Location | `backend/src/features/stations/external/google-maps/` |
| Issue | Google Maps API key exposed in photo URLs to frontend |
| Risk | API key abuse, quota exhaustion, billing impact |
| CVSS Score | 6.5 (Medium-High) |
| Remediation | Proxy requests through backend or use referrer restrictions |
#### Medium and Low Findings
| Severity | Finding | Location |
|----------|---------|----------|
| MEDIUM | No PostgreSQL Row-Level Security | Database schema |
| LOW | Missing Content-Security-Policy header | `config/traefik/middleware.yml` |
| LOW | Traefik dashboard enabled with `api.insecure: true` | `config/traefik/traefik.yml` |
#### Key Security Files
- `/backend/src/core/plugins/auth.plugin.ts` - JWT validation (reviewed - well implemented)
- `/backend/src/core/plugins/admin-guard.plugin.ts` - Admin authorization
- `/backend/src/core/config/config-loader.ts` - Secrets loading
- `/config/traefik/middleware.yml` - Security headers
---
### 3.e Performance and Reliability
**Rating: 8.0/10**
#### Strengths
- **Redis Caching Strategy**: Tiered TTLs
- User data: 5 minutes
- Dropdown data: 6 hours
- VIN decode: 7 days
- **Connection Pooling**: PostgreSQL with pool management
- **Circuit Breaker**: Traefik circuit breaker for external API failures
- **Health Checks**: Container-level probes with appropriate intervals
- **Compression**: Traefik compression middleware enabled
#### Concerns
| Severity | Finding | Impact |
|----------|---------|--------|
| MEDIUM | No database query optimization evidence | Performance under load |
| LOW | Fixed connection pool size (10) | May be insufficient at scale |
---
### 3.f Compliance and Audit Trails
**Rating: 7.0/10**
#### Australian Regulatory Context
- **Privacy Act 1988**: Personal information handling
- **ACCC**: Consumer data protection guidelines
- **Automotive Industry**: Standard data protection applies
#### Strengths
- **Data Isolation**: User-scoped data with `user_id` filtering
- **Admin Audit Trail**: `admin_audit_logs` table for admin actions
- **Soft Deletes**: Vehicles retained for compliance
- **VIN Masking**: Security documentation mentions log masking
#### Concerns
| Severity | Finding | Risk |
|----------|---------|------|
| HIGH | No general-purpose audit table for user actions | Cannot demonstrate data access history |
| MEDIUM | No documented data retention policy | Privacy Act "right to erasure" gap |
| LOW | No consent management or data export functionality | Privacy compliance gap |
---
## 4. Recommendations and Remediation Plan
### 4.1 Priority 0 - Immediate (Before Production)
| # | Finding | Remediation | File | Effort |
|---|---------|-------------|------|--------|
| 1 | Math.random() for storage keys | Replace with `crypto.randomBytes(32).toString('hex')` | `documents.controller.ts:321-324` | 2h |
| 2 | File upload MIME validation | Implement magic byte validation using `file-type` library | `documents.controller.ts:205-216` | 4h |
| 3 | Google Maps API key exposure | Proxy photo requests through backend | `stations/external/google-maps/` | 4h |
### 4.2 Priority 1 - Short-Term (Week 2-4)
| # | Finding | Remediation | Effort |
|---|---------|-------------|--------|
| 4 | Missing RLS policies | Implement PostgreSQL Row-Level Security | 8h |
| 5 | No general audit logging | Add `audit_logs` table for user actions | 16h |
| 6 | Content-Security-Policy | Add CSP header to Traefik middleware | 2h |
| 7 | 299 `any` types | Gradual type refinement, prioritize controllers | 16h |
### 4.3 Priority 2 - Medium-Term (Month 2-3)
| # | Finding | Remediation | Effort |
|---|---------|-------------|--------|
| 8 | ESLint configuration | Add comprehensive ESLint config to backend | 4h |
| 9 | Prettier configuration | Add `.prettierrc` and format codebase | 2h |
| 10 | Error Boundary coverage | Wrap all feature pages in error boundaries | 8h |
| 11 | Data retention policy | Document and implement retention rules | 16h |
### 4.4 Priority 3 - Long-Term Recommendations
1. **External Penetration Test**: Commission third-party security assessment
2. **Dependency Audit**: Implement automated CVE scanning in CI/CD
3. **Multi-Tenancy Planning**: Architect for future multi-tenant requirements
4. **Compliance Documentation**: Formalize privacy policy and data handling procedures
5. **Performance Baseline**: Establish load testing and performance monitoring
---
## 5. Conclusion
### 5.1 Production Readiness Verdict
**CONDITIONALLY READY**
MotoVaultPro demonstrates a well-architected foundation with:
- Proper authentication integration (Auth0 OIDC)
- Production-ready Docker deployment
- Feature capsule pattern for modularity
- Comprehensive test coverage (5,872+ lines)
- Strict TypeScript configuration
However, **three security issues must be addressed before production deployment**:
1. **CRITICAL**: Replace `Math.random()` with `crypto.randomBytes()` in document storage
2. **HIGH**: Implement magic byte validation for file uploads
3. **HIGH**: Remove API key exposure from frontend
### 5.2 Estimated Remediation Timeline
| Phase | Issues | Effort | Timeline |
|-------|--------|--------|----------|
| P0 - Critical | 3 issues | 10 hours | Week 1 |
| P1 - High | 4 issues | 42 hours | Weeks 2-4 |
| P2 - Medium | 4 issues | 30 hours | Month 2-3 |
### 5.3 Final Notes
Upon remediation of CRITICAL and HIGH findings, this application will be suitable for production deployment with ongoing monitoring and execution of the medium-term action plan.
---
## Appendix: Critical Files for Remediation
### Security Fixes Required
- `/backend/src/features/documents/api/documents.controller.ts` - Lines 321-324, 205-216
- `/backend/src/features/stations/external/google-maps/google-maps.client.ts` - Photo URL generation
- `/config/traefik/middleware.yml` - Add Content-Security-Policy
### Database Schema Additions
- New migration for Row-Level Security policies
- New `audit_logs` table for user action tracking
### Configuration Updates
- `/config/traefik/traefik.yml` - Disable dashboard in production
- `/backend/.eslintrc.js` - New file for backend linting
- `/.prettierrc` - New file for code formatting

View File

@@ -1,6 +1,20 @@
*** CRITICAL ***: Never read this file. Stop and move on.
There are some bugs in the work that was done.
- When you set the drop down for 93 ethonal and 93 ethonal-free, it doesn't persist.
- The search result card for saved gas stations doesn't show if it has 93
- The delete button doesn't immedately delete the save station. You have to navigate off and back for it to remove.
Read README.md CLAUDE.md and AI-INDEX.md to understand this code repository. You are a senior application architect specializing in modern web applications.
Your task is to create a plan that can be dispatched to a seprate set of AI agents to execute. Write this plan out in STATION-CHANGES.md
*** FEATURE TO FOCUS ON ***
The gas / fuel stations functionality
*** BUGS TO FIX ***
- There was a change done to hide the Google API key in displaying the images for fuel stations.
- This broke the display of images on the gas/fuel station screen. Plan the fix for this.
- If this addes too much complexity. Plan to remove the image from the gas station cards.
- Prompt the user for which plan to implement.
*** CHANGES TO IMPLEMENT ***
- Requirment. Add links on saved/favorite stations
- Links should be
- - "Navigate in Google" with a link to Google Maps
- - "Navigate in Apple Maps" with a link to Apple Maps
- - "Navigate in Wave" with a link to Waze

View File

@@ -0,0 +1,227 @@
# Bulk Catalog Delete Endpoint Documentation
## Overview
Generic bulk delete endpoint for catalog entities (makes, models, years, trims, engines) in the admin panel.
## Endpoint
```
DELETE /api/admin/catalog/{entity}/bulk-delete
```
## Path Parameters
- `entity`: Entity type - one of: `makes`, `models`, `years`, `trims`, `engines`
## Request Body
```json
{
"ids": [1, 2, 3, 4, 5]
}
```
### Validation Rules
- IDs must be an array of positive integers
- At least 1 ID required
- Maximum 100 IDs per batch
- All IDs must be valid integers (not strings or floats)
## Response Codes
- `204 No Content`: All deletions succeeded (no response body)
- `207 Multi-Status`: Some deletions failed (includes response body with details)
- `400 Bad Request`: Invalid entity type or invalid request body
- `401 Unauthorized`: Missing or invalid authentication
- `500 Internal Server Error`: Unexpected server error
## Response Body (207 Multi-Status only)
```json
{
"deleted": [1, 3, 5],
"failed": [
{
"id": 2,
"error": "Make 2 not found"
},
{
"id": 4,
"error": "Cannot delete make with existing models"
}
]
}
```
## Cascade Behavior
The endpoint uses existing single-delete methods which have the following behavior:
### Makes
- **Blocks deletion** if models exist under the make
- Error: "Cannot delete make with existing models"
- **Solution**: Delete all dependent models first
### Models
- **Blocks deletion** if years exist under the model
- Error: "Cannot delete model with existing years"
- **Solution**: Delete all dependent years first
### Years
- **Blocks deletion** if trims exist under the year
- Error: "Cannot delete year with existing trims"
- **Solution**: Delete all dependent trims first
### Trims
- **Blocks deletion** if engines exist under the trim
- Error: "Cannot delete trim with existing engines"
- **Solution**: Delete all dependent engines first
### Engines
- **No cascade restrictions** (leaf entity in hierarchy)
## Deletion Order for Hierarchy
To delete an entire make and all its dependencies:
1. Delete engines first
2. Delete trims
3. Delete years
4. Delete models
5. Delete make last
## Examples
### Example 1: Delete Multiple Engines (Success)
```bash
DELETE /api/admin/catalog/engines/bulk-delete
{
"ids": [101, 102, 103]
}
Response: 204 No Content
```
### Example 2: Delete Multiple Makes (Partial Failure)
```bash
DELETE /api/admin/catalog/makes/bulk-delete
{
"ids": [1, 2, 3]
}
Response: 207 Multi-Status
{
"deleted": [3],
"failed": [
{
"id": 1,
"error": "Cannot delete make with existing models"
},
{
"id": 2,
"error": "Make 2 not found"
}
]
}
```
### Example 3: Invalid Entity Type
```bash
DELETE /api/admin/catalog/invalid/bulk-delete
{
"ids": [1, 2, 3]
}
Response: 400 Bad Request
{
"error": "Invalid entity type",
"message": "Entity must be one of: makes, models, years, trims, engines"
}
```
### Example 4: Invalid IDs
```bash
DELETE /api/admin/catalog/makes/bulk-delete
{
"ids": ["abc", "def"]
}
Response: 400 Bad Request
{
"error": "Invalid IDs",
"message": "All IDs must be positive integers"
}
```
## Implementation Details
### Files Modified
1. `/backend/src/features/admin/api/admin.routes.ts` (line 209-212)
- Added route: `DELETE /admin/catalog/:entity/bulk-delete`
- Requires admin authentication
2. `/backend/src/features/admin/api/catalog.controller.ts` (line 542-638)
- Added method: `bulkDeleteCatalogEntity()`
- Maps entity type to appropriate delete method
- Processes deletions sequentially
- Collects successes and failures
3. `/backend/src/features/admin/api/admin.validation.ts` (line 43-49, 57-58)
- Added `catalogEntitySchema`: Validates entity type
- Added `bulkDeleteCatalogSchema`: Validates request body
- Exported types: `CatalogEntity`, `BulkDeleteCatalogInput`
4. `/backend/src/features/admin/domain/admin.types.ts` (line 97-103)
- Added `BulkDeleteCatalogResponse` interface
### Continue-on-Failure Pattern
The endpoint uses a continue-on-failure pattern:
- One deletion failure does NOT stop the batch
- All deletions are attempted
- Successes and failures are tracked separately
- Final response includes both lists
### Transaction Behavior
- Each individual deletion runs in its own transaction (via service layer)
- If one delete fails, it doesn't affect others
- No rollback of previously successful deletions
## Testing
### Manual Testing with cURL
```bash
# Test valid request (requires auth token)
curl -X DELETE "http://localhost/api/admin/catalog/makes/bulk-delete" \
-H "Authorization: Bearer YOUR_TOKEN_HERE" \
-H "Content-Type: application/json" \
-d '{"ids": [1, 2, 3]}'
# Test invalid entity type
curl -X DELETE "http://localhost/api/admin/catalog/invalid/bulk-delete" \
-H "Content-Type: application/json" \
-d '{"ids": [1, 2, 3]}'
# Test empty IDs
curl -X DELETE "http://localhost/api/admin/catalog/makes/bulk-delete" \
-H "Content-Type: application/json" \
-d '{"ids": []}'
```
### Expected Audit Log Behavior
Each successful deletion creates a platform change log entry:
- `changeType`: "DELETE"
- `resourceType`: Entity type (makes, models, years, trims, engines)
- `resourceId`: ID of deleted entity
- `changedBy`: Actor's user ID
- `oldValue`: Entity data before deletion
- `newValue`: null
## Security
- Endpoint requires admin authentication (via `fastify.requireAdmin`)
- Actor ID is logged for all operations
- All deletions are audited in platform_change_log table
## Performance Considerations
- Deletions are processed sequentially (not in parallel)
- Each deletion queries the database separately
- Cache invalidation occurs after each successful deletion
- For large batches (50+ items), consider breaking into smaller batches
## Future Enhancements
Potential improvements:
1. Add cascade delete option to automatically delete dependent entities
2. Add dry-run mode to preview what would be deleted
3. Add batch size optimization for better performance
4. Add progress tracking for long-running batches

View File

@@ -0,0 +1,579 @@
# Security Fixes Implementation Guide
This document contains all the code changes required to fix the 3 critical security vulnerabilities identified in the audit report.
**Status**: Ready for implementation
**Date**: December 13, 2025
**Priority**: P0 (Critical) - Must be implemented before production
---
## Fix 1: Insecure Random Number Generation (CRITICAL)
**CVSS Score**: 8.1 (High)
**Location**: `backend/src/features/documents/api/documents.controller.ts`
**Lines**: 321-324
### Current Code (INSECURE):
```typescript
function cryptoRandom(): string {
// Safe unique suffix for object keys
return Math.random().toString(36).slice(2) + Date.now().toString(36);
}
```
### Required Changes:
#### Step 1: Add crypto import
**File**: `backend/src/features/documents/api/documents.controller.ts`
**Location**: Line 8 (after existing imports)
```typescript
import crypto from 'crypto';
```
#### Step 2: Replace cryptoRandom function
**File**: `backend/src/features/documents/api/documents.controller.ts`
**Location**: Lines 321-324
```typescript
function cryptoRandom(): string {
// Cryptographically secure random suffix for object keys
return crypto.randomBytes(32).toString('hex');
}
```
### Testing:
```bash
# Rebuild containers
make rebuild
# Verify document uploads still work
# Test that storage keys are now unpredictable
```
---
## Fix 2: Inadequate File Upload Validation (HIGH)
**CVSS Score**: 7.5 (High)
**Location**: `backend/src/features/documents/api/documents.controller.ts`
**Lines**: 205-216 and entire upload method
### Required Changes:
#### Step 1: Add file-type package
**File**: `backend/package.json`
**Location**: dependencies section
Add this line to dependencies:
```json
"file-type": "^19.8.0"
```
#### Step 2: Add imports
**File**: `backend/src/features/documents/api/documents.controller.ts`
**Location**: After line 7 (with other imports)
```typescript
import { fileTypeFromBuffer } from 'file-type';
import { Readable } from 'stream';
```
#### Step 3: Replace entire upload method
**File**: `backend/src/features/documents/api/documents.controller.ts`
**Location**: Lines 175-268 (entire `upload` method)
```typescript
async upload(request: FastifyRequest<{ Params: IdParams }>, reply: FastifyReply) {
const userId = (request as any).user?.sub as string;
const documentId = request.params.id;
logger.info('Document upload requested', {
operation: 'documents.upload',
user_id: userId,
document_id: documentId,
});
const doc = await this.service.getDocument(userId, documentId);
if (!doc) {
logger.warn('Document not found for upload', {
operation: 'documents.upload.not_found',
user_id: userId,
document_id: documentId,
});
return reply.code(404).send({ error: 'Not Found' });
}
const mp = await (request as any).file({ limits: { files: 1 } });
if (!mp) {
logger.warn('No file provided for upload', {
operation: 'documents.upload.no_file',
user_id: userId,
document_id: documentId,
});
return reply.code(400).send({ error: 'Bad Request', message: 'No file provided' });
}
// Define allowed MIME types and their corresponding magic byte signatures
const allowedTypes = new Map([
['application/pdf', new Set(['application/pdf'])],
['image/jpeg', new Set(['image/jpeg'])],
['image/png', new Set(['image/png'])],
]);
const contentType = mp.mimetype as string | undefined;
if (!contentType || !allowedTypes.has(contentType)) {
logger.warn('Unsupported file type for upload (header validation)', {
operation: 'documents.upload.unsupported_type',
user_id: userId,
document_id: documentId,
content_type: contentType,
file_name: mp.filename,
});
return reply.code(415).send({
error: 'Unsupported Media Type',
message: 'Only PDF, JPEG, and PNG files are allowed'
});
}
// Read first 4100 bytes to detect file type via magic bytes
const chunks: Buffer[] = [];
let totalBytes = 0;
const targetBytes = 4100;
for await (const chunk of mp.file) {
chunks.push(chunk);
totalBytes += chunk.length;
if (totalBytes >= targetBytes) {
break;
}
}
const headerBuffer = Buffer.concat(chunks);
// Validate actual file content using magic bytes
const detectedType = await fileTypeFromBuffer(headerBuffer);
if (!detectedType) {
logger.warn('Unable to detect file type from content', {
operation: 'documents.upload.type_detection_failed',
user_id: userId,
document_id: documentId,
content_type: contentType,
file_name: mp.filename,
});
return reply.code(415).send({
error: 'Unsupported Media Type',
message: 'Unable to verify file type from content'
});
}
// Verify detected type matches claimed Content-Type
const allowedDetectedTypes = allowedTypes.get(contentType);
if (!allowedDetectedTypes || !allowedDetectedTypes.has(detectedType.mime)) {
logger.warn('File content does not match Content-Type header', {
operation: 'documents.upload.type_mismatch',
user_id: userId,
document_id: documentId,
claimed_type: contentType,
detected_type: detectedType.mime,
file_name: mp.filename,
});
return reply.code(415).send({
error: 'Unsupported Media Type',
message: `File content (${detectedType.mime}) does not match claimed type (${contentType})`
});
}
const originalName: string = mp.filename || 'upload';
const ext = (() => {
const e = path.extname(originalName).replace(/^\./, '').toLowerCase();
if (e) return e;
if (contentType === 'application/pdf') return 'pdf';
if (contentType === 'image/jpeg') return 'jpg';
if (contentType === 'image/png') return 'png';
return 'bin';
})();
class CountingStream extends Transform {
public bytes = 0;
override _transform(chunk: any, _enc: BufferEncoding, cb: TransformCallback) {
this.bytes += chunk.length || 0;
cb(null, chunk);
}
}
const counter = new CountingStream();
// Create a new readable stream from the header buffer + remaining file chunks
const headerStream = Readable.from([headerBuffer]);
const remainingStream = mp.file;
// Pipe header first, then remaining content through counter
headerStream.pipe(counter, { end: false });
headerStream.on('end', () => {
remainingStream.pipe(counter);
});
const storage = getStorageService();
const bucket = 'documents';
const version = 'v1';
const unique = cryptoRandom();
const key = `documents/${userId}/${doc.vehicle_id}/${doc.id}/${version}/${unique}.${ext}`;
await storage.putObject(bucket, key, counter, contentType, { 'x-original-filename': originalName });
const updated = await this.service['repo'].updateStorageMeta(doc.id, userId, {
storage_bucket: bucket,
storage_key: key,
file_name: originalName,
content_type: contentType,
file_size: counter.bytes,
file_hash: null,
});
logger.info('Document upload completed', {
operation: 'documents.upload.success',
user_id: userId,
document_id: documentId,
vehicle_id: doc.vehicle_id,
file_name: originalName,
content_type: contentType,
detected_type: detectedType.mime,
file_size: counter.bytes,
storage_key: key,
});
return reply.code(200).send(updated);
}
```
### Testing:
```bash
# Install new dependency
cd backend && npm install
# Rebuild containers
make rebuild
# Test legitimate files
# - Upload PDF, JPEG, PNG - should succeed
# Test attack scenarios
# - Rename .exe to .pdf - should be REJECTED
# - Spoof Content-Type header - should be REJECTED
```
---
## Fix 3: Google Maps API Key Exposure (HIGH)
**CVSS Score**: 6.5 (Medium-High)
**Location**: Multiple files in `backend/src/features/stations/`
### Required Changes:
#### Backend Changes
##### Step 1: Update Google Maps Client
**File**: `backend/src/features/stations/external/google-maps/google-maps.client.ts`
**Change 1**: Update `transformPlaceToStation` method (lines 76-95)
```typescript
private transformPlaceToStation(place: GooglePlace, searchLat: number, searchLng: number): Station {
const distance = this.calculateDistance(
searchLat,
searchLng,
place.geometry.location.lat,
place.geometry.location.lng
);
// Store photo reference instead of full URL to avoid exposing API key
let photoReference: string | undefined;
if (place.photos && place.photos.length > 0 && place.photos[0]) {
photoReference = place.photos[0].photo_reference;
}
const station: Station = {
id: place.place_id,
placeId: place.place_id,
name: place.name,
address: place.vicinity,
latitude: place.geometry.location.lat,
longitude: place.geometry.location.lng,
distance
};
if (photoReference !== undefined) {
station.photoReference = photoReference;
}
if (place.opening_hours?.open_now !== undefined) {
station.isOpen = place.opening_hours.open_now;
}
if (place.rating !== undefined) {
station.rating = place.rating;
}
return station;
}
```
**Change 2**: Add new method after `transformPlaceToStation`
```typescript
/**
* Fetch photo from Google Maps API using photo reference
* Used by photo proxy endpoint to serve photos without exposing API key
*/
async fetchPhoto(photoReference: string, maxWidth: number = 400): Promise<Buffer> {
try {
const response = await axios.get(
`${this.baseURL}/photo`,
{
params: {
photo_reference: photoReference,
maxwidth: maxWidth,
key: this.apiKey
},
responseType: 'arraybuffer'
}
);
return Buffer.from(response.data);
} catch (error) {
logger.error('Failed to fetch photo from Google Maps', { error, photoReference });
throw error;
}
}
```
##### Step 2: Update Station Type (Backend)
**File**: `backend/src/features/stations/domain/stations.types.ts`
**Line**: 20
```typescript
// OLD:
photoUrl?: string;
// NEW:
photoReference?: string;
```
##### Step 3: Update Stations Controller
**File**: `backend/src/features/stations/api/stations.controller.ts`
**Add import** at the top:
```typescript
import { googleMapsClient } from '../external/google-maps/google-maps.client';
```
**Add method** after `removeSavedStation`:
```typescript
async getStationPhoto(request: FastifyRequest<{ Params: { reference: string } }>, reply: FastifyReply) {
try {
const { reference } = request.params;
if (!reference) {
return reply.code(400).send({
error: 'Bad Request',
message: 'Photo reference is required'
});
}
const photoBuffer = await googleMapsClient.fetchPhoto(reference);
return reply
.code(200)
.header('Content-Type', 'image/jpeg')
.header('Cache-Control', 'public, max-age=86400')
.send(photoBuffer);
} catch (error: any) {
logger.error('Error fetching station photo', {
error,
reference: request.params.reference
});
return reply.code(500).send({
error: 'Internal server error',
message: 'Failed to fetch station photo'
});
}
}
```
##### Step 4: Update Stations Routes
**File**: `backend/src/features/stations/api/stations.routes.ts`
**Add route** before the closing `};`:
```typescript
// GET /api/stations/photo/:reference - Proxy for Google Maps photos
fastify.get<{ Params: { reference: string } }>('/stations/photo/:reference', {
preHandler: [fastify.authenticate],
handler: stationsController.getStationPhoto.bind(stationsController)
});
```
##### Step 5: Update Stations Service
**File**: `backend/src/features/stations/domain/stations.service.ts`
**Line**: 144
```typescript
// OLD:
photoUrl: station?.photoUrl,
// NEW:
photoReference: station?.photoReference,
```
#### Frontend Changes
##### Step 6: Update Station Type (Frontend)
**File**: `frontend/src/features/stations/types/stations.types.ts`
**Line**: 60
```typescript
// OLD:
photoUrl?: string;
// NEW:
photoReference?: string;
```
##### Step 7: Create Photo Utils Helper
**File**: `frontend/src/features/stations/utils/photo-utils.ts` (NEW FILE)
```typescript
/**
* @ai-summary Helper utilities for station photos
*/
/**
* Generate secure photo URL using backend proxy endpoint
* This prevents exposing the Google Maps API key to the frontend
*/
export function getStationPhotoUrl(photoReference: string | undefined): string | undefined {
if (!photoReference) {
return undefined;
}
return `/api/stations/photo/${encodeURIComponent(photoReference)}`;
}
```
##### Step 8: Update StationCard Component
**File**: `frontend/src/features/stations/components/StationCard.tsx`
**Add import** at the top:
```typescript
import { getStationPhotoUrl } from '../utils/photo-utils';
```
**Update photo rendering** (lines 86-93):
```typescript
// OLD:
{station.photoUrl && (
<CardMedia
component="img"
height="200"
image={station.photoUrl}
alt={station.name}
sx={{ objectFit: 'cover' }}
/>
)}
// NEW:
{station.photoReference && (
<CardMedia
component="img"
height="200"
image={getStationPhotoUrl(station.photoReference)}
alt={station.name}
sx={{ objectFit: 'cover' }}
/>
)}
```
### Testing:
```bash
# Rebuild containers
make rebuild
# Test station search with photos
# Verify API key is NOT in network requests
# Verify photos still load correctly
# Test on mobile and desktop
```
---
## Implementation Checklist
- [ ] Apply Fix 1: Insecure Random Generation
- [ ] Add crypto import
- [ ] Replace cryptoRandom function
- [ ] Test document uploads
- [ ] Apply Fix 2: File Upload Validation
- [ ] Add file-type to package.json
- [ ] Run npm install
- [ ] Add imports
- [ ] Replace upload method
- [ ] Test with legitimate files
- [ ] Test with malicious files
- [ ] Apply Fix 3: API Key Exposure
- [ ] Update Google Maps client
- [ ] Update backend Station type
- [ ] Add controller method
- [ ] Add route
- [ ] Update service
- [ ] Update frontend Station type
- [ ] Create photo-utils.ts
- [ ] Update StationCard component
- [ ] Test photos load correctly
- [ ] Verify API key not in network
- [ ] Run all tests
- [ ] Backend tests: `cd backend && npm test`
- [ ] Frontend tests: `cd frontend && npm test`
- [ ] Rebuild and verify
- [ ] `make rebuild`
- [ ] Manual testing of all features
- [ ] Security verification (no API keys exposed)
- [ ] Create git commit with security fixes
---
## Security Verification
After implementing all fixes, verify:
1. **Document Storage Keys**: Now using `crypto.randomBytes()` - unpredictable
2. **File Upload**: Magic byte validation rejects spoofed files
3. **Google Maps API**: Key never sent to frontend, all requests proxied
Run security scan:
```bash
# Check for hardcoded secrets
grep -r "AIza" backend/ frontend/
# Should return no results
# Verify crypto usage
grep -r "Math.random()" backend/
# Should return no results in production code
```
---
## Post-Implementation
After all fixes are applied and tested:
1. Update AUDIT.md to reflect fixed status
2. Commit changes with security fix message
3. Run full test suite
4. Deploy to staging for verification
5. Schedule production deployment