Skip to content

Summit Guardian v1 -- Complete Audit Report

Generated: 2026-02-07

Audited by: 6 specialized AI agents (Claude Opus 4.6)

Files scanned: 207 source files across app/, templates/, static/, tests/, config, and infrastructure


Application Overview

Summit Guardian v1 is a unified booking and operations platform for Highline Adventures, a California-based adventure tourism business. Built with FastAPI + PostgreSQL + Jinja2/HTMX/Alpine.js + Tailwind CSS, it handles property reservations, activity bookings, equipment rentals, point-of-sale, staff task management, waiver/liability compliance, and CCPA data privacy. The application has ~162 routes, 28 database tables, 19 service modules, and 5 user roles (guest, instructor, property_manager, admin, super_admin). It is deployed on an InMotion VPS via systemd + Apache reverse proxy, with GitHub Actions FTP-based CI/CD.

Overall Health Assessment: C+

The application is functionally complete and operational in production. The architecture is well-organized with clean separation of concerns (routes -> services -> models). However, the audit uncovered significant security vulnerabilities (non-functional CSRF, default secret keys, missing auth checks), systemic data integrity risks (float-for-money across 35+ columns, race conditions in booking), and low test coverage (~25% of routes, ~40% of services). The codebase is well-structured for an MVP but requires hardening before handling real customer transactions.


Development Phase 1: Critical Fixes (Do These First)

Issues that are actively causing or could cause security breaches, data loss, double bookings, or financial errors.


Issue 1.1: CSRF Protection is Completely Non-Functional

File(s): app/middleware/csrf.py, app/main.py (missing registration) Category: Security Severity: 🔴 Critical

What's wrong (Simple Explanation): Imagine your house has a fancy alarm system installed, but nobody ever turned it on. That's what happened here -- the code for CSRF protection exists, but it was never actually connected to the application. This means any malicious website could trick a logged-in admin into performing actions (deleting users, cancelling bookings, changing prices) without their knowledge, just by getting them to visit a crafted webpage.

Technical Details: The csrf_protect function in app/middleware/csrf.py is never registered as middleware in app/main.py and is never injected as a Depends() on any route. Additionally:

  • 21 routes accept a csrf_token = Form(...) parameter but never validate it
  • 24 POST routes don't even accept a CSRF token parameter
  • admin/pricing/rules.html has forms with hardcoded empty CSRF values (value="")
  • JSON API calls from calendar.js, checkout.js, and the POS terminal send no CSRF tokens

What happens if you fix this: All state-changing POST requests will require a valid CSRF token. You'll need to:

  1. Register csrf_protect as middleware or inject it via Depends() on every POST route
  2. Add to every form template that's missing it
  3. Include CSRF tokens in JSON fetch() headers
  4. Test every form submission to ensure tokens are properly passed

What happens if you DON'T fix this: An attacker can craft a malicious webpage that, when visited by a logged-in admin, silently performs actions like: cancelling all bookings, deleting users, changing pricing, exporting customer data, or processing fraudulent CCPA deletion requests.

Estimated effort: Medium task (half day) -- the middleware exists, it just needs wiring up + template fixes


Issue 1.2: Secret Keys Are Default Placeholder Values

File(s): .env (lines 4, 13), app/config.py (lines 13, 22) Category: Security Severity: 🔴 Critical

What's wrong (Simple Explanation): The "master keys" to your entire application are set to the example values that ship with the code. It's like having a bank vault where the combination is "1234" -- anyone who reads the public source code template knows the keys and can forge any login session or admin access.

Technical Details:

  • .env line 4: SECRET_KEY=change-me-to-a-secure-random-key (the literal default)
  • .env line 13: JWT_SECRET_KEY=change-me-to-a-separate-secure-key (the literal default)
  • app/config.py also has these as fallback defaults, so even if .env is missing, the app starts with known keys
  • Anyone who has seen .env.example can forge valid JWTs and CSRF tokens
  • config.py does not validate that these have been changed from defaults

What happens if you fix this: Generate two random 64-character hex strings (e.g., python -c "import secrets; print(secrets.token_hex(32))") and set them in the production .env. All existing sessions will be invalidated -- users will need to log in again.

What happens if you DON'T fix this: Anyone can forge valid JWT tokens, impersonate any user (including super_admin), and gain full access to the application and all customer data.

Estimated effort: Quick fix (< 30 min) -- generate keys, update production .env, restart service


Issue 1.3: Deactivated and Deleted Users Can Still Log In

File(s): app/middleware/auth.py (lines 12-31) Category: Security Severity: 🔴 Critical

What's wrong (Simple Explanation): When you fire an employee or a customer requests account deletion, their account is marked as "inactive" or "deleted" in the database. But the login gate never checks these flags -- it's like revoking someone's building pass but never telling the security guard to stop letting them in.

Technical Details:get_current_user() in app/middleware/auth.py loads the user by ID from the JWT token but never checks user.is_active or user.deleted_at. A CCPA-deleted user (whose deleted_at is set and is_active is False) retains full access until their JWT expires.

python
# Current code (line 24-26):
user = await db.get(User, uuid.UUID(user_id))
if not user:
    raise ValueError("User not found")
# Missing: if not user.is_active or user.deleted_at: raise ValueError("Account deactivated")

What happens if you fix this: Any deactivated or deleted user will be logged out on their next request. Existing JWT tokens for these users will be rejected. This is the correct behavior.

What happens if you DON'T fix this: Fired staff members retain admin/staff access. CCPA-deleted users (who legally should have no access) can still log in and access the system, which is a compliance violation.

Estimated effort: Quick fix (< 30 min) -- add 2 lines to get_current_user()


Issue 1.4: Auth Middleware Returns Wrong Type for Non-API Redirects

File(s): app/middleware/auth.py (lines 34-41) Category: Bug Severity: 🔴 Critical

What's wrong (Simple Explanation): When a non-logged-in user visits a protected page, the code tries to redirect them to the login page. But due to a technical mistake, instead of actually redirecting, the "redirect instruction" gets treated as if it were the user object. This means the page crashes with a confusing error instead of sending users to login.

Technical Details:require_auth is used as a FastAPI Depends() dependency. When it returns a RedirectResponse for non-API unauthenticated requests, FastAPI injects the RedirectResponse object as the user parameter to the route handler. The route then tries to access user.id, user.role, etc. on a RedirectResponse object, causing AttributeError.

What happens if you fix this: Unauthenticated users visiting protected pages will be cleanly redirected to /auth/login. Use raise HTTPException(status_code=303, headers={"Location": "/auth/login"}) instead of returning RedirectResponse.

What happens if you DON'T fix this: Non-logged-in users hitting protected HTML pages see a 500 error instead of being redirected to login. API endpoints work correctly (they return 401).

Estimated effort: Quick fix (< 30 min) -- change return RedirectResponse to raise HTTPException


Issue 1.5: Double-Booking Race Condition in Property Availability

File(s): app/services/availability_service.py (lines 190-212) Category: Data Loss / Business Logic Severity: 🔴 Critical

What's wrong (Simple Explanation): Imagine two customers try to book the same cabin for the same dates at the exact same moment. The system checks "is it available?" for both, says "yes" to both, and then books it for both. Now you have two families showing up to the same cabin on the same night. This is called a "race condition."

Technical Details:mark_dates_booked() uses a SELECT-then-UPDATE pattern without row-level locking:

python
# Line 190-198: SELECT reads rows as "available"
result = await db.execute(
    select(AvailabilityCalendar).where(status == "available")
)
rows = result.scalars().all()
# Line 206-209: Then updates each row in Python
for row in rows:
    row.status = status

Between the SELECT and the UPDATE, another concurrent transaction can read the same rows as "available." Both transactions succeed, creating a double booking.

What happens if you fix this: Use SELECT ... FOR UPDATE or a single UPDATE ... WHERE status='available' RETURNING * to make the check-and-set atomic. Only one concurrent booking will succeed; the other will get an error and can retry or be informed the dates are taken.

What happens if you DON'T fix this: Under any level of concurrent traffic, two guests can book the same property for overlapping dates. This creates a customer service nightmare and potential legal liability.

Estimated effort: Small task (1-2 hours) -- rewrite to use atomic UPDATE RETURNING


Issue 1.6: Activity Overbooking Race Condition

File(s): app/services/activity_service.py (lines 357-367, 439-466) Category: Data Loss / Business Logic Severity: 🔴 Critical

What's wrong (Simple Explanation): Same race condition as property booking, but for activities. Two people booking the last slot in a kayak tour at the same time can both succeed, putting you over capacity. Unlike properties (which have the mark_dates_booked overlap guard), activities have NO atomic guard at all.

Technical Details:check_slot_availability() reads the booked count with a SUM query, then create_booking_from_cart() inserts the ActivityBooking. No SELECT FOR UPDATE, no database constraint, and no atomic mechanism prevents two concurrent requests from both passing the availability check.

What happens if you fix this: Add a database-level check constraint or use SELECT FOR UPDATE on the schedule row when checking capacity. One concurrent booking succeeds, the other fails cleanly.

What happens if you DON'T fix this: Activities can be overbooked beyond capacity. For adventure activities (ziplines, rock climbing), this could be a safety issue.

Estimated effort: Small task (1-2 hours) -- add row locking to slot check


Issue 1.7: Float Used for All Financial Calculations (35+ Columns)

File(s): Every model with monetary columns (13 of 16 model files), all service files Category: Data Loss / Financial Severity: 🔴 Critical

What's wrong (Simple Explanation): The app stores and calculates money using "floating point" numbers, which are like a ruler with slightly inaccurate markings. Most of the time it's close enough, but sometimes $10.10 + $10.20 = $20.299999999999997 instead of $20.30. Over thousands of transactions, these tiny errors accumulate into real discrepancies in your financial records.

Technical Details: All 35+ monetary columns use Mapped[float] with Numeric(10,2) in the database. While PostgreSQL stores them precisely as Decimal, Python's type annotation says float, causing:

  • Service-layer arithmetic uses float (e.g., booking_service.py lines 179-183 sum cart totals with float)
  • Pydantic schemas use float for money fields
  • Cart totals are JSON-serialized floats in the cookie
  • Tax calculations start as float, convert to Decimal midway through

Key affected columns: Booking.grand_total, Booking.subtotal, Payment.amount, BookingItem.unit_price, all pricing rule adjustments, all fee amounts, all tax rates.

What happens if you fix this: Change all Mapped[float] to Mapped[Decimal] on monetary columns. Update Pydantic schemas to use Decimal. Update service functions to use Decimal arithmetic. This is a large but mechanical change. Existing database values won't change (they're already stored as Numeric).

What happens if you DON'T fix this: Occasional penny-level discrepancies in booking totals, refund amounts, and tax calculations. Over volume, these compound. Financial reports may not reconcile. Customers may be charged slightly incorrect amounts.

Estimated effort: Large task (full day) -- 35+ model columns, 20+ schema fields, arithmetic in all services


Issue 1.8: Auth Cookies Missing secure Flag

File(s): app/routes/auth.py (lines 77-84), app/services/cart_service.py (lines 77-83) Category: Security Severity: 🔴 Critical

What's wrong (Simple Explanation): The "key cards" (JWT tokens) that prove you're logged in are being sent over both secure (HTTPS) and insecure (HTTP) connections. If anyone is listening on the network (like on public WiFi), they can steal your key card and log in as you.

Technical Details: All response.set_cookie() calls for access_token, refresh_token, and cart cookies are missing secure=True. The production site uses HTTPS (https://app.summitguardian.com), but without the secure flag, cookies will also be sent over any HTTP request (e.g., if someone visits http://app.summitguardian.com or is on a network with SSL stripping).

What happens if you fix this: Add secure=True to all set_cookie() calls. Cookies will only be sent over HTTPS. Users accessing via HTTP will need to be redirected to HTTPS first (which should already happen via Apache config).

What happens if you DON'T fix this: On non-HTTPS connections or networks with SSL stripping, authentication tokens can be intercepted. Combined with the default secret keys (Issue 1.2), this creates a complete authentication compromise chain.

Estimated effort: Quick fix (< 30 min) -- add secure=True to ~4 set_cookie calls


Issue 1.9: Admin Manifest Page Missing Authentication

File(s): app/routes/admin/manifests.py (line 27) Category: Security Severity: 🔴 Critical

What's wrong (Simple Explanation): The manifest index page (which redirects to daily manifests containing guest names, booking details, and property assignments) is publicly accessible with no login required. Anyone who knows the URL can see today's guest roster.

Technical Details:

python
@router.get("/")
async def manifest_index():  # Missing: user=Depends(require_admin)
    return RedirectResponse(url="/admin/manifests/daily", status_code=303)

While the redirected-to pages (/admin/manifests/daily, etc.) do require require_admin, the index route itself has no auth check. This is a minor exposure since it only redirects, but it's inconsistent and could lead to information disclosure if the redirect target's auth is also accidentally removed.

What happens if you fix this: Add user=Depends(require_admin) to the manifest_index function signature.

What happens if you DON'T fix this: Minimal immediate risk since it only redirects. But it indicates an auth gap pattern that should be audited for other routes.

Estimated effort: Quick fix (< 30 min) -- add one dependency parameter


Issue 1.10: Plain FTP Deployment Exposes Source Code

File(s): .github/workflows/deploy.yml (line 23) Category: Security Severity: 🔴 Critical

What's wrong (Simple Explanation): Every time you deploy, your entire application source code is uploaded over the internet without encryption. It's like mailing your business plan on a postcard instead of in a sealed envelope -- anyone along the delivery route can read it.

Technical Details: The GitHub Actions deployment uses protocol: ftp (plain FTP, not FTPS or SFTP). All file contents, including Python source code, templates, and configuration, are transmitted in cleartext. A network observer between GitHub's servers and your VPS can capture the entire application. Note: .env is excluded from deployment, so secrets aren't directly exposed via FTP, but the application logic, database schema, and security mechanisms are.

What happens if you fix this: Switch to FTPS (protocol: ftps) in the GitHub Actions workflow. This encrypts the file transfer. Alternatively, use SSH/rsync if CSF firewall rules can be updated.

What happens if you DON'T fix this: Application source code is transmitted in cleartext on every deploy. An attacker who can observe the network traffic learns the entire application structure, security mechanisms, and can craft targeted attacks.

Estimated effort: Quick fix (< 30 min) -- change protocol: ftp to protocol: ftps in deploy.yml


Development Phase 2: High-Priority Improvements

Issues that significantly degrade reliability, data integrity, or developer productivity but aren't actively causing crashes in the happy path.


Issue 2.1: require_auth Redirect Mechanism is Broken

File(s): app/middleware/auth.py (lines 34-41) Category: Bug Severity: 🟠 High

(Covered in Issue 1.4 above -- this is the dependency injection type mismatch)


Issue 2.2: Grid Optimizer Can Create Double Bookings

File(s): app/services/grid_optimization_service.py (lines 121-167) Category: Bug Severity: 🟠 High

What's wrong (Simple Explanation): The grid optimizer tries to rearrange bookings to fill gaps in FIA (floating inventory) units. But when it moves a 3-night booking to a different unit, it only checks that ONE date is available on the target -- not all 3 nights. So it could move a booking onto a unit that's already partially occupied, creating overlapping stays.

Technical Details: Line 121-126 only checks AvailabilityCalendar.date == gap_start availability on the target unit, not the full date range of the booking being moved.

What happens if you fix this: Check all dates of the moved booking against the target unit's availability before performing the move.

What happens if you DON'T fix this: Running grid optimization with dry_run=False could create double bookings on target units. Since this is an admin-triggered action (not automatic), the risk is limited to when an admin manually runs optimization.

Estimated effort: Small task (1-2 hours)


Issue 2.3: POS Stock Decrement is Not Atomic

File(s): app/services/pos_service.py (lines 225-247) Category: Bug Severity: 🟠 High

What's wrong (Simple Explanation): If two staff members ring up the last water bottle at the same time, the system might sell it twice because it checks stock and deducts stock in two separate steps instead of one atomic operation.

Technical Details: Read-check-modify pattern: item.stock_quantity is read, compared to qty, then decremented. No row locking prevents concurrent overselling. The max(0, ...) prevents negative stock but doesn't prevent the sale from succeeding when it shouldn't.

What happens if you fix this: Use UPDATE pos_items SET stock_quantity = stock_quantity - :qty WHERE id = :id AND stock_quantity >= :qty RETURNING stock_quantity for atomic decrement.

What happens if you DON'T fix this: Concurrent POS sales of the same item can oversell inventory. Low risk given the likely low concurrency of in-person POS transactions, but incorrect inventory counts accumulate over time.

Estimated effort: Small task (1-2 hours)


Issue 2.4: Booking Number Generation Can Produce Duplicates

File(s): app/services/booking_service.py (lines 32-51) Category: Bug Severity: 🟠 High

What's wrong (Simple Explanation): Booking numbers (like "HLA-250207-0001") are generated by reading the highest existing number and adding 1. If two bookings happen at the exact same moment, they both read the same highest number and both generate the same new number.

Technical Details:generate_booking_number() uses MAX(booking_number) without any locking or unique constraint retry logic.

What happens if you fix this: Add a unique constraint on booking_number and retry with an incremented sequence on constraint violation. Alternatively, use a database sequence.

What happens if you DON'T fix this: Under concurrent booking traffic, duplicate booking numbers can be generated. This creates confusion in admin interfaces, manifests, and confirmation emails.

Estimated effort: Small task (1-2 hours)


Issue 2.5: CSP Headers Allow unsafe-eval and unsafe-inline

File(s): app/middleware/security.py (line 24) Category: Security Severity: 🟠 High

What's wrong (Simple Explanation): The Content Security Policy (a browser-level defense against script injection attacks) is configured to allow inline scripts and eval(). This is like having a fence around your yard but leaving the gate wide open. CSP exists specifically to block these attack vectors, but the current configuration permits them.

Technical Details: The CSP header includes 'unsafe-eval' (needed for Alpine.js) and 'unsafe-inline' in the script-src directive. This effectively negates CSP's XSS protection for scripts.

What happens if you fix this: Alpine.js with CSP requires the alpine-csp build which avoids eval(). Replace the standard Alpine.js import with alpinejs/csp. Remove 'unsafe-inline' by moving inline scripts to external files and using nonces.

What happens if you DON'T fix this: If an XSS vulnerability exists anywhere in the application (e.g., unescaped user content), CSP will not block the injected script from executing.

Estimated effort: Medium task (half day) -- switch to Alpine CSP build, extract inline scripts


Issue 2.6: No HSTS Header

File(s): app/middleware/security.py (missing) Category: Security Severity: 🟠 High

What's wrong (Simple Explanation): The first time someone visits your site via HTTP (not HTTPS), their connection isn't encrypted. HSTS tells browsers to "always use HTTPS from now on," but your app never sends this instruction.

Technical Details: No Strict-Transport-Security header is set. The production site runs HTTPS but without HSTS, the first HTTP request is vulnerable to SSL stripping attacks.

What happens if you fix this: Add response.headers["Strict-Transport-Security"] = "max-age=31536000; includeSubDomains" to the security middleware.

What happens if you DON'T fix this: Users who type http://app.summitguardian.com or follow HTTP links are briefly vulnerable to man-in-the-middle attacks before the HTTPS redirect.

Estimated effort: Quick fix (< 30 min) -- add one header


Issue 2.7: Rate Limiter is Per-Process and Leaks Memory

File(s): app/middleware/rate_limit.py (lines 6-31) Category: Bug / Performance Severity: 🟠 High

What's wrong (Simple Explanation): The brute-force protection (which limits login attempts) keeps its data in each server worker's memory separately. If you have 2 workers, an attacker gets 2x the allowed attempts. Also, it never fully cleans up old data, so it slowly uses more and more memory over time.

Technical Details:

  • In-memory dict is per-process. Multi-worker deployments multiply the rate limit by N workers.
  • Behind Apache reverse proxy, request.client.host may always be 127.0.0.1 unless X-Forwarded-For is handled.
  • _attempts dict entries for old IPs are never fully pruned, causing unbounded memory growth.

What happens if you fix this: For MVP, add X-Forwarded-For handling and periodic full cleanup of old entries. For production, consider Redis-backed rate limiting.

What happens if you DON'T fix this: Brute-force protection is significantly weakened. Memory usage grows slowly over time with unique IP addresses.

Estimated effort: Small task (1-2 hours) for the quick fix


Issue 2.8: Instructor Time Calculation Can Crash

File(s): app/services/instructor_service.py (lines 298-301) Category: Bug Severity: 🟠 High

What's wrong (Simple Explanation): If an activity starts at 10 PM and runs for 3 hours, the math tries to create a time of "25:00" which doesn't exist. Python throws an error and the page crashes.

Technical Details:

python
eb_end_minutes = eb_start.hour * 60 + eb_start.minute + eb_duration + eb_buffer
eb_end = time(eb_end_minutes // 60, eb_end_minutes % 60)  # ValueError if >= 24:00

What happens if you fix this: Cap eb_end_minutes at 1439 (23:59) or handle the day-boundary case.

What happens if you DON'T fix this: Any activity with start_time + duration + buffer > 24:00 will crash the instructor availability check, preventing auto-assignment for late-evening activities.

Estimated effort: Quick fix (< 30 min)


Issue 2.9: N+1 Queries in Daily Manifest (50-100+ queries per page)

File(s): app/services/manifest_service.py (lines 40-132) Category: Performance Severity: 🟠 High

What's wrong (Simple Explanation): The daily manifest page (used by staff every morning) makes a separate database query for every single check-in, activity, and equipment return, instead of loading everything at once. On a busy day with 20 check-ins, this means 70+ database round trips just to show one page.

Technical Details: Three N+1 loops:

  1. For each check-in booking: await db.get(Property, item.item_id) inside nested loop
  2. For each activity booking: await db.get(Activity, ab.activity_id) + full booking query
  3. For each equipment return: await db.get(Equipment, ...) + await db.get(User, ...)

What happens if you fix this: Use selectinload on relationships and batch-load with IN queries. Page load drops from 70+ queries to ~5-10.

What happens if you DON'T fix this: Staff experience slow page loads every morning. Database connection pool can be exhausted under load.

Estimated effort: Medium task (half day)


Issue 2.10: Payment Failure Leaves Orphaned Pending Bookings

File(s): app/services/booking_service.py (lines 342-349) Category: Bug Severity: 🟠 High

What's wrong (Simple Explanation): If payment fails during checkout, the booking dates are properly released, but the booking record itself stays in the database marked as "pending." It's never cleaned up or marked as "failed." Over time, orphaned pending bookings accumulate.

Technical Details: On payment failure, release_booked_dates is called and a ValueError is raised, but the Booking record created on line 185 remains with status="pending". The exception causes a transaction rollback in some code paths, but not consistently.

What happens if you fix this: Set booking.status = "payment_failed" before raising the error, or ensure the transaction rollback covers the booking creation.

What happens if you DON'T fix this: Orphaned "pending" bookings accumulate in the database and appear in admin booking lists, causing confusion.

Estimated effort: Quick fix (< 30 min)


Issue 2.11: ALLOWED_HOSTS Defined But Never Enforced

File(s): app/config.py (line 15), app/main.py (missing middleware) Category: Security Severity: 🟠 High

What's wrong (Simple Explanation): The configuration defines which domain names are allowed to access the app, but nobody checks. It's like having a guest list at a party but no bouncer.

Technical Details:ALLOWED_HOSTS is defined in config but TrustedHostMiddleware is never added to the FastAPI app. This allows host header injection attacks.

What happens if you fix this: Add app.add_middleware(TrustedHostMiddleware, allowed_hosts=settings.ALLOWED_HOSTS.split(",")) to main.py.

What happens if you DON'T fix this: Host header injection attacks are possible, potentially leading to cache poisoning or password reset email redirection.

Estimated effort: Quick fix (< 30 min)


Issue 2.12: Seed Script Has Undefined Variable Bug

File(s): scripts/seed_data.py (line 863) Category: Bug Severity: 🟠 High

What's wrong (Simple Explanation): If you run the seed script on a database where the admin user already exists, the script crashes with a NameError because it tries to use a variable (admin) that was only defined inside an if block that didn't execute.

Technical Details:created_by=admin.id on line 863 references admin which is created inside if not existing_admin: on line 43. If the admin exists, admin is undefined.

What happens if you fix this: Assign admin = existing_admin in the else branch.

What happens if you DON'T fix this: Re-running the seed script (which is common during development or after database resets) crashes partway through.

Estimated effort: Quick fix (< 30 min)


Development Phase 3: Mobile & Responsive Fixes


Issue 3.1: Admin Panel Not Mobile-Friendly

File(s): templates/admin/layout.htmlCategory: Responsive Design Severity: 🟡 Medium

What's wrong (Simple Explanation): The admin panel has a fixed 256px sidebar that doesn't collapse on mobile screens. On phones or tablets, the sidebar either pushes the content off-screen or makes it unreadably narrow.

Technical Details: The sidebar uses a fixed w-64 class with no responsive mobile drawer/overlay pattern. A toggle button exists but only uses hidden class toggling, not a proper slide-out drawer.

What happens if you fix this: Implement a responsive sidebar: hidden by default on mobile, revealed via hamburger button with overlay. Use Tailwind's md:block hidden pattern.

What happens if you DON'T fix this: Admin panel is unusable on mobile devices. Admin staff must use desktops for all management tasks.

Estimated effort: Medium task (half day)


Issue 3.2: Weekly/Monthly Manifest Grids Cramped on Mobile

File(s): templates/admin/manifests/weekly.html, templates/admin/manifests/monthly.htmlCategory: Responsive Design Severity: 🟡 Medium

What's wrong (Simple Explanation): The 7-column calendar/grid layout becomes unreadable on small screens with no responsive strategy (scroll, stack, or collapse).

What happens if you fix this: Add overflow-x-auto wrapper and ensure minimum column widths, or switch to a stacked card layout on mobile.

Estimated effort: Small task (1-2 hours)


Issue 3.3: Admin Tables Overflow on Small Screens

File(s): Multiple admin templates (users/list, bookings/list, reports/revenue, waivers/signatures) Category: Responsive Design Severity: 🟡 Medium

What's wrong (Simple Explanation): Wide data tables in admin views extend beyond the screen edge on mobile, with no horizontal scrollbar wrapper.

What happens if you fix this: Wrap each <table> in a <div class="overflow-x-auto"> container.

Estimated effort: Quick fix (< 30 min) -- add wrapper divs to ~6 tables


Issue 3.4: Staff Layout Prevents Pinch-to-Zoom

File(s): templates/staff/layout.html (line 5) Category: Accessibility / Mobile Severity: 🟡 Medium

What's wrong (Simple Explanation): The staff mobile layout has user-scalable=no in the viewport meta tag, which prevents users from zooming in. This is an accessibility issue for users with vision impairments.

What happens if you fix this: Remove maximum-scale=1.0, user-scalable=no from the viewport meta tag.

Estimated effort: Quick fix (< 30 min)


Development Phase 4: Performance Optimizations


Issue 4.1: Tailwind CSS Using Development Play CDN

File(s): templates/base.html, templates/admin/layout.html, templates/staff/layout.htmlCategory: Performance Severity: 🟠 High

What's wrong (Simple Explanation): All three layout files load Tailwind CSS via cdn.tailwindcss.com, which is the development-only "play CDN." This generates CSS on the fly in the browser, adding significant overhead. The Tailwind team explicitly warns against using this in production.

Technical Details: The play CDN downloads the entire Tailwind library (~300KB), then scans the page's HTML and generates CSS client-side. This adds 200-500ms to every page load and prevents proper caching.

What happens if you fix this: Install Tailwind CLI, build a production CSS file, and reference it as a static asset. This produces a small, optimized CSS file (~10-30KB) that loads instantly and caches perfectly.

What happens if you DON'T fix this: Every page load across the entire application (public, admin, staff) incurs an unnecessary 200-500ms penalty and downloads excess CSS.

Estimated effort: Medium task (half day) -- install Tailwind CLI, configure, build, update templates


Issue 4.2: Activity Calendar Makes 100+ Queries Per Month View

File(s): app/services/activity_service.py (lines 686-703) Category: Performance Severity: 🟡 Medium

What's wrong (Simple Explanation): Loading the activity calendar for a month makes 3+ database queries per day x 31 days = 100+ queries. Each day separately checks schedules, blackout dates, and booked counts.

What happens if you fix this: Batch-load all schedules, blackout dates, and booking counts for the entire month in 3-4 queries, then process in Python.

Estimated effort: Medium task (half day)


Issue 4.3: Weekly Manifest Runs 28 Queries

File(s): app/services/manifest_service.py (lines 143-211) Category: Performance Severity: 🟡 Medium

What's wrong (Simple Explanation): The weekly manifest runs 4 separate count queries per day x 7 days = 28 queries minimum. These could be batched into grouped queries.

What happens if you fix this: Use GROUP BY date in a single query per metric instead of per-day queries.

Estimated effort: Small task (1-2 hours)


Issue 4.4: hash_password Blocks the Event Loop

File(s): app/services/auth_service.py (line 30) Category: Performance Severity: 🟡 Medium

What's wrong (Simple Explanation): bcrypt password hashing is intentionally CPU-intensive (takes ~200ms). Running it in an async function blocks the entire event loop, meaning no other requests can be processed during that 200ms.

What happens if you fix this: Use await asyncio.to_thread(hash_password, password) or loop.run_in_executor().

Estimated effort: Quick fix (< 30 min)


Issue 4.5: No Lazy Loading on Images

File(s): All templates with images (property listings, equipment, etc.) Category: Performance Severity: 🟡 Low

What's wrong (Simple Explanation): All images load immediately when the page opens, even ones you can't see until you scroll down. Adding loading="lazy" tells the browser to wait until the image is about to scroll into view.

Estimated effort: Quick fix (< 30 min) -- add loading="lazy" to <img> tags


Development Phase 5: Code Quality & Refactoring


Issue 5.1: datetime.utcnow() Deprecated (50+ Occurrences)

File(s): Every model file, multiple service files Category: Code Quality Severity: 🟡 Medium

What's wrong (Simple Explanation): Python 3.12+ has deprecated datetime.utcnow() because it creates a "naive" datetime (one without timezone info). The replacement is datetime.now(timezone.utc).

What happens if you fix this: Find-and-replace datetime.utcnow with lambda: datetime.now(timezone.utc) in model defaults. Update service files similarly.

Estimated effort: Small task (1-2 hours) -- mechanical find-and-replace


Issue 5.2: Missing Database Indexes on 16+ Foreign Key Columns

File(s): Multiple model files (see Appendix) Category: Performance / Code Quality Severity: 🟡 Medium

What's wrong (Simple Explanation): PostgreSQL does NOT automatically create indexes on foreign key columns. Many commonly-queried columns (like "all bookings for this user" or "all payments for this booking") don't have indexes, meaning the database has to scan every row to find matches.

Technical Details: Key missing indexes:

  • Booking.user_id -- guest's bookings lookup
  • Booking.status -- dashboard filtering
  • Payment.booking_id -- payment lookup
  • Session.token_hash -- AUTH HOT PATH (every authenticated request!)
  • StaffTask.assigned_to, StaffTask.status, StaffTask.due_date
  • EquipmentRental.equipment_id, EquipmentRental.rented_by_user_id
  • WaiverSignature.user_id, WaiverSignature.waiver_template_id

What happens if you fix this: Add index=True to these columns in the model definitions, then generate an Alembic migration.

Estimated effort: Small task (1-2 hours) -- add index=True, generate migration


Issue 5.3: Missing Relationships on 5 Models

File(s): app/models/availability.py, equipment.py, staff_task.py, resource.pyCategory: Code Quality Severity: 🟡 Medium

What's wrong (Simple Explanation): Five models have foreign keys but no SQLAlchemy relationship() definitions, forcing every access to related data to use manual SQL joins instead of clean Python navigation.

Technical Details:

  • AvailabilityCalendar -- no link to Property
  • EquipmentRental -- 3 FKs, 0 relationships
  • StaffTask -- 6 FKs, 0 relationships
  • EquipmentMaintenanceLog -- no relationships
  • ResourceAllocation -- no relationships

Estimated effort: Medium task (half day)


Issue 5.4: Silent Exception Swallowing (11+ Occurrences)

File(s): Multiple admin route files, staff routes Category: Code Quality Severity: 🟡 Medium

What's wrong (Simple Explanation): Many route handlers catch errors with except (ValueError, Exception): pass, meaning if something goes wrong, the user gets silently redirected with no error message and the error is never logged. It's like a "check engine" light that's been taped over.

What happens if you fix this: Log the exception, flash an error message to the user, or return an error template.

Estimated effort: Medium task (half day) -- update 11+ exception handlers


Issue 5.5: Migration Strategy is Non-Standard and Fragile

File(s): migrations/versions/a24af1c1df86_initial_schema_all_tables.pyCategory: Code Quality Severity: 🟡 Medium

What's wrong (Simple Explanation): The initial database migration uses Base.metadata.create_all() instead of individual op.create_table() calls. This means Alembic can't track schema changes, and future alembic --autogenerate commands may produce incorrect or empty migrations. The downgrade also uses drop_all() which destroys ALL tables, not just those from this migration.

What happens if you fix this: Run alembic revision --autogenerate to generate a proper migration reflecting the actual schema. Use standard Alembic operations going forward.

Estimated effort: Medium task (half day)


Issue 5.6: Dead Code and Unused Dependencies

File(s): passenger_wsgi.py, requirements.txtCategory: Code Quality Severity: 🟡 Low

What's wrong (Simple Explanation):

  • passenger_wsgi.py -- written for a Passenger deployment model that's not used. If accidentally loaded, wraps async app in sync WSGI.
  • apscheduler in requirements -- listed but never imported
  • a2wsgi in requirements -- only used by unused passenger_wsgi.py

Estimated effort: Quick fix (< 30 min) -- delete file, remove deps


Issue 5.7: No Dependency Version Pinning

File(s): requirements.txtCategory: Code Quality Severity: 🟡 Medium

What's wrong (Simple Explanation): All dependencies use >= (minimum version) instead of == (exact version). This means running pip install tomorrow might pull a different version than today, potentially breaking the app.

What happens if you fix this: Generate a requirements.lock or use pip freeze > requirements.txt to pin exact versions.

Estimated effort: Quick fix (< 30 min)


Development Phase 6: Accessibility & Compliance


Issue 6.1: Global Focus Outline Removal

File(s): static/css/custom.css (lines 14-16) Category: Accessibility (WCAG 2.4.7) Severity: 🟠 High

What's wrong (Simple Explanation): The CSS globally removes the focus outline on all form inputs (outline: none). This means keyboard users (and users who rely on assistive devices) can't see which element is currently selected. It's like removing the cursor from a text editor.

What happens if you fix this: Remove the global outline: none or replace with a visible custom focus ring (e.g., outline: 2px solid #2563eb).

Estimated effort: Quick fix (< 30 min)


Issue 6.2: Map Pins Not Keyboard Accessible

File(s): templates/components/map.htmlCategory: Accessibility (WCAG 2.1.1) Severity: 🟡 Medium

What's wrong (Simple Explanation): The property map pins only respond to mouse hover (@mouseenter/@mouseleave). Keyboard users can't navigate to or interact with them.

What happens if you fix this: Add tabindex="0", @focus, and @blur handlers to map pins. Add @keydown.enter for pin activation.

Estimated effort: Small task (1-2 hours)


Issue 6.3: Missing ARIA Labels on Interactive Elements

File(s): templates/components/navbar.html, templates/components/calendar.htmlCategory: Accessibility (WCAG 4.1.2) Severity: 🟡 Medium

What's wrong (Simple Explanation): Several interactive buttons (mobile hamburger menu, cart icon, calendar navigation arrows) have no accessible labels. Screen reader users hear "button" with no description of what the button does.

Estimated effort: Quick fix (< 30 min) -- add aria-label attributes


Issue 6.4: Heading Hierarchy Issues

File(s): templates/public/home.htmlCategory: Accessibility (WCAG 1.3.1) Severity: 🟡 Low

What's wrong (Simple Explanation): The home page jumps from <h1> to <h3>, skipping <h2>. Screen readers use heading levels to understand page structure, so skipping levels is confusing.

Estimated effort: Quick fix (< 30 min)


Issue 6.5: CDN Resources Missing Subresource Integrity (SRI)

File(s): All three layout templates Category: Security / Compliance Severity: 🟡 Medium

What's wrong (Simple Explanation): Alpine.js, signature_pad, and html5-qrcode are loaded from CDNs without integrity hashes. If the CDN is compromised, malicious code could be injected into your site. Alpine.js also uses a wildcard version (@3.x.x), meaning the exact script can change at any time.

What happens if you fix this: Pin Alpine.js to a specific version and add integrity="sha384-..." and crossorigin="anonymous" attributes.

Estimated effort: Small task (1-2 hours)


Development Phase 7: Testing & Documentation


Issue 7.1: Core Booking Flow Has ZERO Test Coverage

File(s): tests/ (missing test) Category: Testing Severity: 🔴 Critical

What's wrong (Simple Explanation): The create_booking_from_cart function -- the 330-line function that converts a cart into a confirmed booking with payment, date marking, resource allocation, and keycode generation -- has absolutely no tests. This is the core revenue path of the application. If it breaks, no bookings can be made, and there's no automated way to know.

What happens if you fix this: Write tests covering: accommodation-only cart, mixed cart (accommodation + activity + equipment), payment failure handling, date marking, resource allocation, and keycode generation.

What happens if you DON'T fix this: Any change to the booking flow could silently break the entire checkout process with no automated detection.

Estimated effort: Large task (full day)


Issue 7.2: 64% of Route Modules Have Zero Tests

File(s): 18 of 28 route modules untested Category: Testing Severity: 🟠 High

What's wrong (Simple Explanation): 18 out of 28 route modules have absolutely no tests. All admin CRUD routes (users, properties, pricing, taxes, equipment, activities, instructors, manifests, POS, reports, grid, CCPA, tasks, waivers) are completely untested. All staff task routes are untested.

Estimated effort: Major refactor (multi-day) -- prioritize critical user flows first


Issue 7.3: Tests Accept Server Errors (500) as Passing

File(s): tests/test_booking_flow.py, tests/test_cancellation.pyCategory: Testing Severity: 🟠 High

What's wrong (Simple Explanation): Several tests use assertions like assert response.status_code in (200, 500) -- meaning a server crash counts as a passing test! Other tests silently catch exceptions with except Exception: pass, making them impossible to fail.

What happens if you fix this: Fix the underlying template/rendering issues causing 500s, then assert strict status codes.

Estimated effort: Medium task (half day)


Issue 7.4: No Successful Login Test

File(s): tests/test_auth.pyCategory: Testing Severity: 🟠 High

What's wrong (Simple Explanation): There's no test that verifies a registered user can actually log in. Tests check the login page loads and invalid credentials fail, but never verify that valid credentials succeed with a cookie set.

Estimated effort: Small task (1-2 hours)


Issue 7.5: No Test Coverage Configuration

File(s): pyproject.tomlCategory: Testing / DX Severity: 🟡 Medium

What's wrong (Simple Explanation): No coverage reporting is configured. There's no way to objectively measure test coverage or track it over time.

What happens if you fix this: Add addopts = "--cov=app --cov-report=term-missing" to pyproject.toml.

Estimated effort: Quick fix (< 30 min)


Development Phase 8: Nice-to-Have Enhancements


Issue 8.1: Add __repr__ to All Models

Most models lack __repr__, making debugging and logging harder. When a model instance appears in a traceback or log, it shows <Activity object at 0x...> instead of useful info.

Estimated effort: Small task (1-2 hours)


Issue 8.2: Add Database CHECK Constraints

Add constraints for: prices > 0, ratings in valid ranges, stock >= 0, valid_until > valid_from, min_stay <= max_stay.

Estimated effort: Medium task (half day)


Issue 8.3: Soft Delete Mixin

Only User has soft delete (deleted_at). If other entities need it, a SoftDeleteMixin base class would standardize the approach.

Estimated effort: Small task (1-2 hours)


Issue 8.4: Extract Inline JavaScript to External Files

The POS terminal has a large inline posTerminal() function that should be in an external JS file for better caching and maintainability.

Estimated effort: Small task (1-2 hours)


Issue 8.5: Add loading="lazy" and srcset to Images

Property and equipment images would benefit from responsive sizing and lazy loading for better performance on mobile.

Estimated effort: Small task (1-2 hours)


Appendix A: File-by-File Summary

FileStatusIssuesPhase
app/__init__.pyClean0--
app/main.pyIssues2 (debug default, missing middleware registration)1, 2
app/config.pyIssues4 (default secrets, DEBUG=True, ALLOWED_HOSTS unused)1, 2
app/database.pyClean1 minor (echo=DEBUG)--
app/middleware/__init__.pyClean0--
app/middleware/auth.pyCritical3 (no is_active check, wrong return type, redundant except)1
app/middleware/csrf.pyIssues1 (never registered as middleware)1
app/middleware/rate_limit.pyIssues3 (per-process, memory leak, IP spoofing)2
app/middleware/security.pyIssues4 (unsafe-eval, unsafe-inline, no HSTS, deprecated XSS header)2
app/models/activity.pyIssues11 (float/Decimal, missing indexes, missing cascades)1, 5
app/models/availability.pyIssues8 (no relationships, float/Decimal, missing timestamps)5
app/models/booking.pyCritical11 (float on all money columns, missing indexes, no FK on item_id)1, 5
app/models/equipment.pyIssues10 (no relationships, float/Decimal, missing indexes)5
app/models/instructor.pyMinor2 (float/Decimal)5
app/models/keycode.pyIssues5 (missing indexes, no unique on code)5
app/models/payment.pyCritical6 (float on amount, missing indexes)1, 5
app/models/pos.pyMinor4 (float/Decimal)5
app/models/pricing.pyIssues8 (float/Decimal on all values)5
app/models/property.pyMinor7 (float/Decimal, missing constraints)5
app/models/property_settings.pyMinor35
app/models/resource.pyIssues7 (float/Decimal, missing indexes)5
app/models/staff_task.pyIssues5 (0 relationships despite 6 FKs, missing indexes)5
app/models/user.pyIssues5 (no soft-delete filter, Session.token_hash no index)2, 5
app/models/waiver.pyMinor65
app/schemas/__init__.pyClean0--
app/schemas/activity.pyIssues7 (float for money, missing validators)5
app/schemas/booking.pyMinor35
app/schemas/property.pyIssues5 (unused Decimal import, missing update validators)5
app/schemas/user.pyMinor35
app/schemas/waiver.pyMinor15
app/services/activity_service.pyCritical5 (race condition, N+1 queries)1, 4
app/services/auth_service.pyIssues3 (blocking bcrypt, no session cleanup)4
app/services/availability_service.pyCritical2 (TOCTOU race condition)1
app/services/booking_service.pyCritical7 (float arithmetic, race conditions, orphaned bookings)1, 2
app/services/cart_service.pyIssues4 (float arithmetic, missing secure cookie)1, 5
app/services/ccpa_service.pyIssues3 (no auth check, no session invalidation)2
app/services/dashboard_service.pyClean0--
app/services/equipment_service.pyMinor3 (LIKE injection, good atomic checkout)5
app/services/grid_optimization_service.pyIssues2 (partial date check, N+1)2
app/services/instructor_service.pyIssues4 (time overflow, N+1, None rating)2, 4
app/services/keycode_service.pyMinor25
app/services/manifest_service.pyIssues2 (severe N+1 queries)2, 4
app/services/pos_service.pyIssues4 (non-atomic stock, kwargs injection)2
app/services/pricing_service.pyMinor28
app/services/reporting_service.pyMinor15
app/services/resource_allocation_service.pyIssues1 (race condition)5
app/services/task_service.pyMinor28
app/services/tax_service.pyMinor15
app/services/waiver_service.pyIssues2 (wrong signature association, Ellipsis sentinel)5
app/utils/__init__.pyClean0--
app/utils/security.pyMinor1 (bcrypt not in requirements)5
app/utils/signature.pyMinor2 (no size limit, wrong error message)5
app/utils/qr_generator.pyClean0--
app/providers/**Clean2 minor (random vs secrets, float for money)5, 8
app/routes/auth.pyIssues4 (missing secure cookie, GET logout)1, 5
app/routes/public.pyMinor2 (LIKE injection in search)5
app/routes/booking.pyIssues1 (CSRF accepted but not validated)1
app/routes/account.pyIssues2 (silent exception swallowing)5
app/routes/admin/*.py (16 files)IssuesCSRF never validated, silent exceptions1, 5
app/routes/api/*.py (6 files)IssuesMissing CSRF on mutations, LIKE injection1, 5
app/routes/staff/*.py (2 files)IssuesCSRF never validated, no file size limit1, 5
app/jobs/scheduler.pyIssues4 (wrong created_by FK, LIKE wildcards)2
templates/base.htmlIssues1 (Tailwind play CDN)4
templates/admin/layout.htmlIssues3 (play CDN, not mobile-friendly, no nav element)3, 4
templates/staff/layout.htmlIssues2 (play CDN, user-scalable=no)3, 4
templates/admin/pricing/rules.htmlCritical1 (empty CSRF values)1
templates/components/map.htmlIssues2 (unescaped JS vars, no keyboard access)6
templates/components/navbar.htmlIssues2 (missing aria-labels)6
templates/public/home.htmlMinor1 (heading hierarchy)6
static/css/custom.cssIssues1 (global focus outline removal)6
static/js/calendar.jsIssues2 (no CSRF, console-only errors)1, 5
static/js/checkout.jsIssues2 (no CSRF on waiver sign, alert() for errors)1, 5
static/js/map.jsClean0--
static/js/qr-scanner.jsMinor1 (global mutable state)8
tests/conftest.pyIssues7 (slow teardown, separate sessions, missing role fixtures)7
tests/test_auth.pyIssues8 (no login success test, weak assertions)7
tests/test_booking.pyCritical7 (create_booking_from_cart untested)7
tests/test_booking_flow.pyCritical5 (accepts 500 as passing, no checkout test)7
tests/test_cancellation.pyIssues7 (silent exception swallowing, weak assertions)7
tests/test_cart.pyIssues6 (cart add functions untested)7
tests/test_*.py (remaining)MixedVarious7
.envCritical3 (default secrets, DEBUG=true)1
.env.exampleClean0--
.github/workflows/deploy.ymlIssues2 (plain FTP, extra files deployed)1, 5
alembic.iniIssues1 (hardcoded DB URL)5
migrations/versions/*.pyIssues2 (non-standard create_all, destructive drop_all)5
scripts/seed_data.pyIssues3 (weak passwords, prints creds, undefined var bug)2
scripts/deploy.shMinor3 (non-atomic lock, find precedence, no migration error handling)5
DockerfileNote1 (dev-only, not for production)--
passenger_wsgi.pyDead code1 (unused file for unused deployment model)5
pyproject.tomlMinor2 (blanket warning suppression, no coverage config)7
requirements.txtIssues4 (missing bcrypt, unused deps, no pinning)5

Appendix B: Dependency Audit

PackageRequired VersionPurposeUsed?Notes
fastapi>=0.110.0Web frameworkYesCore
uvicorn[standard]>=0.27.0ASGI serverYesCore
python-multipart>=0.0.9Form parsingYesRequired for Form()
sqlalchemy[asyncio]>=2.0ORMYesCore
alembic>=1.13MigrationsYesCore
asyncpg>=0.29.0Async PostgreSQL driverYesCore
psycopg2-binary>=2.9Sync PostgreSQL driverYesFor Alembic
passlib[bcrypt]>=1.7Password hashingIndirectbcrypt used directly
python-jose[cryptography]>=3.3JWT handlingYesAuth
pydantic[email]>=2.0ValidationYesSchemas
pydantic-settings>=2.0ConfigYesSettings
jinja2>=3.1TemplatesYesCore
qrcode[pil]>=7.4QR generationYesEquipment
pillow>=10.0Image processingYesSignatures
python-dotenv>=1.0.env loadingYesConfig
httpx>=0.27HTTP clientYesTesting
apscheduler>=3.10Job schedulingNoUnused
a2wsgi>=1.10ASGI-WSGI bridgeNoOnly in unused passenger_wsgi.py
bcryptNot listedPassword hashingYesDirect import, transitive dep only

Appendix C: Quick Wins (Top 10 Highest-Impact, Lowest-Effort Fixes)

#FixEffortImpactFiles
1Generate real SECRET_KEY and JWT_SECRET_KEY for production5 minPrevents complete auth bypass.env
2Add secure=True to all set_cookie() calls15 minPrevents cookie theft on HTTPauth.py, cart_service.py
3Add is_active and deleted_at check to get_current_user10 minBlocks deactivated/deleted usersmiddleware/auth.py
4Add require_admin to manifest index route2 minCloses auth gapadmin/manifests.py
5Add HSTS header5 minPrevents SSL downgrademiddleware/security.py
6Add TrustedHostMiddleware5 minPrevents host header injectionmain.py
7Fix empty CSRF values in pricing templates10 minEnables CSRF on pricing operationsadmin/pricing/rules.html
8Remove global focus outline removal5 minRestores keyboard accessibilitycustom.css
9Switch FTP to FTPS in deploy workflow2 minEncrypts deployment trafficdeploy.yml
10Fix seed script undefined variable bug5 minPrevents crash on re-seedingseed_data.py

Total time for all 10 quick wins: ~65 minutesCombined impact: Closes 5 critical security gaps, 1 auth bypass, 1 a11y violation, 1 deployment vulnerability, and 1 crash bug.


End of Audit Report

lock

Enter PIN to continue