alla-tms/docs/reviews/phase-6-security-audit.md

Phase 6 - Security Audit

Executive Summary

The application has a solid baseline in two important areas: server-side authorization is used broadly across route handlers, and many sensitive workflow actions are audited. The main security risks are not widespread missing auth checks, but a smaller set of high-impact gaps in file delivery, request hardening, dependency posture, and abuse protection.

The most serious application-layer issue is direct public exposure of uploaded online-lesson assets. Uploaded video, attachment, and thumbnail files are stored under public/uploads/..., serialized back to clients as direct public URLs, and rendered directly in the UI. This bypasses the guarded download pattern that already exists in other parts of the system.

The second major risk is the import surface. Both spreadsheet import endpoints accept files based only on extension presence, read the entire file into memory, and rely on xlsx, which npm audit currently reports with high-severity advisories. Combined with the absence of rate limiting and explicit file-size controls on import endpoints, this increases the risk of denial-of-service and unsafe spreadsheet parsing.

Threat Surface Inventory

Primary security-sensitive surfaces reviewed:

• Auth.js credential login flow
• Session and active-organization switching
• Server-side authorization helpers
• Permission-management APIs
• Announcement, online-lesson, and certificate file handling
• Training-record and employee spreadsheet imports
• Report export endpoints
• Notification and workflow action endpoints
• Next.js runtime/configuration and proxy perimeter

Authentication Assessment

Strengths

• src/auth.ts:25-33 validates login input and uses bcrypt password verification.
• src/auth.ts:57,81,103,137 records login success and failure events in audit logs.
• Self-service registration is disabled in src/app/api/auth/register/route.ts.
• Session enforcement is centralized through requireSession().

Findings

• src/auth.ts:33 falls back to dev-only-auth-secret-change-me outside production. This is acceptable for local development but risky if a non-production environment is internet-exposed with weak secret hygiene.
• src/auth.ts:36 sets trustHost: true. This is common in proxied deployments, but it raises deployment sensitivity if host/header forwarding is not tightly controlled upstream.
• No brute-force controls, lockout logic, or request throttling were found around credential login.
• Password policy is minimal: sign-in and auth forms enforce only min(8), and src/features/users/schemas/user.ts does not enforce password strength itself.

Assessment

Authentication is functionally correct, but it lacks abuse resistance and stronger operational guardrails.

Authorization Assessment

Strengths

• Broad route coverage uses server-side guards such as requireOrganizationAccess, requirePermission, requireHRD, requireAllPermissions, and requireSuperAdminOrganizationAccess.
• Permission-management endpoints are protected at the route-handler level even though they are not included in the proxy perimeter list.
• Download routes for announcement attachments and training-record certificates enforce organization-aware access checks before reading files from disk.

Findings

• src/proxy.ts:4-18,41-56 only protects a subset of API prefixes at the perimeter. Notably, online-lesson and permission-management APIs are not listed there, even though their handlers do perform server-side checks. This is a defense-in-depth gap rather than a direct bypass.
• src/lib/auth/session.ts and src/app/api/organizations/active/route.ts can auto-create admin memberships for super-admin/HRD organization access. This may be intended, but it is a privileged side effect inside an access path and should be treated carefully.
• Permission-management routes use requireSuperAdminOrganizationAccess(), which currently depends on the same organization-access helper that can create fallback memberships.

Assessment

Authorization is stronger than average for an admin dashboard. The main risk is not broken route protection by default, but privileged side effects embedded inside access-resolution flows.

Input Validation Assessment

Strengths

• Core workflow routes use Zod or equivalent server-side validation.
• Online-lesson URLs are constrained to http/https via schema validation.
• Route params are commonly parsed and rejected when numeric IDs are invalid.

Findings

• src/app/api/training-records/import/route.ts:228-236 and src/app/api/import-employees/route.ts:460-474 only verify that the upload is a File and that the name ends with .xlsx; no explicit file-size or MIME validation is enforced before parsing.
• Import endpoints load the full spreadsheet with await file.arrayBuffer() before further validation.
• Several workflow routes perform large amounts of procedural validation inline inside route handlers, which makes consistency harder to maintain.

Assessment

Input validation is decent for routine CRUD, but weaker on large-file and batch-import surfaces.

Injection Assessment

Strengths

• Most database access uses Drizzle query builders and parameterized expressions.
• No command-execution patterns or child-process usage were found in application runtime paths.
• Storage helpers sanitize path segments and verify resolved paths stay under upload roots.

Findings

• npm audit reports a high-severity advisory for drizzle-orm regarding SQL injection via improperly escaped SQL identifiers in versions <0.45.2.
• src/features/reports/server/report-data.ts:446-555 uses raw SQL composition for reporting. The visible usage appears parameterized, but this area should be considered high-scrutiny because it is the most SQL-heavy path in the app.
• npm audit reports high-severity advisories for xlsx, which is used to parse untrusted spreadsheet uploads.
• CSV export exists in src/app/api/audit-logs/export/route.ts; this phase did not fully verify whether cells beginning with spreadsheet formula prefixes are neutralized.

Assessment

There is no clear app-authored SQL injection bug in the reviewed code, but dependency-level injection risk is real and currently unresolved.

XSS Assessment

Strengths

• Announcement content, review comments, and online-lesson descriptions are rendered as plain text with whitespace-pre-wrap, not injected HTML.
• No user-content Markdown or rich-text renderer was found in the reviewed content workflows.
• The single dangerouslySetInnerHTML usage in src/components/ui/chart.tsx is used to emit generated CSS variables, not user-provided rich text.

Findings

• Online-lesson external video URLs are rendered into outbound links and embed decisions. The code restricts embeddable iframes to recognized YouTube hosts, which is good, but general outbound URLs are still allowed for non-embeddable links.
• No repo-level Content Security Policy was found in next.config.ts or related runtime config.

Assessment

Direct stored-XSS exposure appears low in the reviewed flows. The larger XSS concern is missing browser-side hardening such as CSP.

CSRF & Request Protection Assessment

Strengths

• State-changing operations generally use non-GET methods.
• Auth is session-based and routed through Auth.js rather than custom ad hoc cookies.

Findings

• No explicit CSRF token validation, origin checks, or referer checks were found in state-changing route handlers.
• No centralized request-hardening middleware was found for mutation endpoints.
• Because cookie behavior is library-managed, SameSite/secure cookie posture was not explicitly verified from local code.

Assessment

This is primarily a hardening gap, not a confirmed exploit in the reviewed code. Still, the app currently relies heavily on framework defaults and browser cookie behavior rather than explicit anti-CSRF controls.

File Upload Assessment

High-risk finding

• src/lib/online-lesson-storage.ts:36-86 writes uploaded lesson assets to public/uploads/online-lessons/... and returns direct public URLs.
• src/features/online-lessons/server/online-lesson-data.ts:51,53,55 serializes those direct file URLs to clients unchanged.
• src/features/online-lessons/components/online-lesson-detail-page.tsx:198-200,284-290,328-336 renders those URLs directly in <img>, <video>, and download/open links.

Impact:

• Any user or external party who obtains one of these URLs can fetch the file directly from the public assets path without passing through authorization checks.
• This creates a real broken-access-control and data-exposure risk for uploaded lesson media and attachments.

Additional findings

• src/lib/announcement-storage.ts and src/lib/certificate-storage.ts also store files under public/uploads/... and create direct public paths, but the primary application serializers now prefer guarded API download URLs instead of returning the raw storage path to clients.
• No virus scanning, content scanning, or magic-byte verification was found for uploaded files.
• Online-lesson attachments permit several document/archive formats, including ZIP, but are still served from public paths.

Assessment

File upload handling is the most important application-layer security weakness in the current codebase, especially for online-lesson assets.

Sensitive Data Assessment

Strengths

• Many protected download responses use Cache-Control: private, no-store.
• Permission and organization access is checked server-side before reading many sensitive records.

Findings

• Uploaded lesson assets are publicly reachable by URL, which can expose internal training content and attachments.
• Login failure audit logs in src/auth.ts persist identifiers and, in one case, user email inside audit payloads. This may be acceptable operationally, but it increases PII footprint inside logs.
• User and employee tables render email, employee code, organization, and department information broadly across admin UIs; this is expected for the app, but it increases the need for stronger export and audit controls.

Assessment

The biggest sensitive-data problem is file exposure, not routine JSON overexposure in the reviewed API routes.

Security Headers Assessment

Findings

• No headers() configuration was found in next.config.ts.
• No explicit Content Security Policy, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy, or HSTS configuration was found in reviewed app config.
• Download routes set Cache-Control, but broader site-level browser hardening appears absent.

Assessment

Browser-side security headers are currently under-configured.

Rate Limiting Assessment

Findings

• No rate-limiting or throttling implementation was found for login, imports, exports, uploads, or workflow actions.
• This directly affects:
  • Credential login
  • Spreadsheet import endpoints
  • Report export endpoint
  • File upload endpoints
  • Notification mutation endpoints

Assessment

Rate limiting is a notable gap and materially increases brute-force and denial-of-service risk.

Audit Log Assessment

Strengths

• Authentication success/failure is logged.
• Permission changes have dedicated audit infrastructure.
• Import operations log audit entries.
• Announcement and workflow transitions show extensive audit logging.

Findings

• src/app/api/reports/export/route.ts does not log report exports, even though exports can contain organization-wide training and employee data.
• This creates an audit gap for sensitive data extraction.
• This phase did not verify logout logging or sensitive-record read auditing beyond the reviewed routes.

Assessment

Audit coverage is good for mutations, but weaker for data-export visibility.

Dependency & Configuration Assessment

npm audit results

• Total vulnerabilities: 11
• High: 3
• Moderate: 8

Key findings:

• drizzle-orm
  • High severity advisory reported by npm audit.
• xlsx
  • High severity advisories reported by npm audit.
  • Especially relevant because the app parses untrusted spreadsheet uploads in two endpoints.
• sort-by / object-path
  • High/moderate advisory chain reported by npm audit.
• @opentelemetry/core and related packages
  • Moderate memory-allocation advisory reported by npm audit.

Configuration findings

• src/auth.ts:33 uses a development fallback secret.
• src/auth.ts:36 sets trustHost: true.
• next.config.ts removes console logs in production, which is good, but does not add security headers.

Assessment

Dependency security posture needs attention now, especially because the vulnerable packages are in active code paths rather than unused tooling only.

Business Logic Security Assessment

Strengths

• Review/publish/archive flows check content status and permissions before transition.
• Training-record certificate download checks ownership/scope through the parent training record.

Findings

• Privileged organization switching can materialize new admin memberships as a side effect.
• Report exports are permission-checked, but not audited.
• This phase did not find a clear "publish without approve" bypass in the reviewed content workflows, but the workflow handlers are large enough that they deserve dedicated regression tests.

Assessment

Business-logic security is generally thoughtful, but several privileged flows remain too implicit and too large for easy verification.

Findings by Severity

High

1. Online-lesson uploaded assets are publicly accessible by direct URL.

   • Evidence: online-lesson-storage.ts, online-lesson-data.ts, online-lesson-detail-page.tsx

2. Spreadsheet import endpoints accept unbounded .xlsx uploads and parse them with a dependency that currently has high-severity advisories.

   • Evidence: training-record import route, employee import route, npm audit finding for xlsx

3. Active dependency vulnerabilities include high-severity advisories in drizzle-orm and xlsx.

   • Evidence: npm audit --json

Medium

1. No brute-force or rate-limiting protections were found for login, upload, import, or export endpoints.
2. No repo-level security headers configuration was found.
3. No explicit CSRF/origin validation layer was found for mutation endpoints.
4. Report exports are not audit-logged.
5. Privileged organization access can create admin memberships as a side effect.
6. Login audit logs increase sensitive identifier/email footprint inside logs.

Low

1. Proxy perimeter coverage is incomplete, though server-side route guards still protect the affected APIs.
2. Development fallback secret is acceptable locally, but risky if misused in a shared environment.

Quick Wins

1. Stop returning direct public URLs for online-lesson files; serve them through guarded download/stream routes.
2. Add strict file-size limits and MIME checks to both spreadsheet import endpoints before arrayBuffer() parsing.
3. Add rate limiting for /api/auth/[...nextauth], import, export, and upload routes.
4. Add export audit logging for reports.
5. Add baseline browser security headers in Next.js config or edge middleware.

Structural Improvements

1. Move all sensitive file delivery to authenticated route handlers and eliminate public/uploads for protected assets.
2. Introduce a centralized request-hardening layer for origin checking, abuse controls, and common mutation protections.
3. Separate privileged organization-access resolution from membership-creation side effects.
4. Add dedicated security tests for workflow transitions, file access, and export scope enforcement.
5. Review and remediate vulnerable dependencies in a controlled upgrade track, especially xlsx and drizzle-orm.

Recommended Remediation Order

1. Online-lesson file exposure

   • Highest real-world access-control risk.

2. Import surface hardening

   • Add file-size checks and reduce exposure from vulnerable spreadsheet parsing.

3. Dependency remediation

   • Prioritize packages in active parsing/data-access paths.

4. Rate limiting and request-hardening

   • Best leverage across login, import, export, and uploads.

5. Security headers and audit-gap cleanup

   • Important defense-in-depth after the higher-risk access issues are contained.

Commands Executed and Results

• rg and Select-String across auth, proxy, storage, API routes, and config
  • Used to inspect auth guards, upload paths, security headers, and audit coverage.
• npm audit --json
  • Reported 11 vulnerabilities total: 3 high and 8 moderate.

Files Reviewed

• src/auth.ts
• src/proxy.ts
• src/lib/auth/session.ts
• src/lib/auth/authorization.server.ts
• next.config.ts
• src/lib/announcement-storage.ts
• src/lib/online-lesson-storage.ts
• src/lib/certificate-storage.ts
• src/app/api/auth/register/route.ts
• src/app/api/organizations/active/route.ts
• src/app/api/reports/export/route.ts
• src/app/api/training-records/import/route.ts
• src/app/api/import-employees/route.ts
• src/app/api/announcements/[id]/attachment/route.ts
• src/app/api/training-records/[id]/certificates/route.ts
• src/app/api/training-records/[id]/certificates/[certificateId]/download/route.ts
• src/app/api/user-permissions/route.ts
• src/app/api/permission-templates/route.ts
• src/app/api/permissions/catalog/route.ts
• src/features/announcements/server/announcement-data.ts
• src/features/online-lessons/server/online-lesson-data.ts
• src/features/training-records/server/training-record-data.ts
• src/features/online-lessons/schemas/online-lesson.ts
• src/features/announcements/components/announcement-view-page.tsx
• src/features/online-lessons/components/online-lesson-detail-page.tsx
• src/components/ui/chart.tsx
• src/features/users/schemas/user.ts

Areas Not Verified

• This phase did not perform live penetration testing, browser interception, or runtime CSRF validation against a running deployment.
• Cookie flags were not verified from actual HTTP responses.
• This phase did not exhaustively review every route handler in the repository; it focused on the highest-risk auth, permission, upload, import, and export surfaces.
• Dependency advisories were taken from local npm audit output and were not independently triaged against actual reachable exploit paths in this specific deployment model.