thePlug/docs/2026-05-06-codebase-audit.md

thePlug Codebase Audit

Date: 2026-05-06

Checks run:

• cd backend && uv run pytest -q
• cd backend && uv run ruff check .
• cd backend && uv run basedpyright app
• npm --prefix frontend run build

Summary:

• Functional checks currently pass: backend tests are green, Ruff is clean, frontend builds.
• Structural quality is much weaker than the green CI surface suggests.
• The biggest risks are runtime mutation safety around Docker compose rewrites, false confidence in the backend test suite, packaging/type-check fragility, and a very large amount of stateful coordination concentrated in single files.

Findings

Critical

1. Docker port rewrite is not exception-safe and can leave challenge services mutated on disk

• Files:
  • backend/app/integrations/docker/editor.py:14-51
  • backend/app/services/mitmproxy/manager.py:195-236
• Why it matters:
  • change_port() rewrites the compose file in place, then runs docker compose up -d.
  • There is no try/finally around the file mutation path.
  • If subprocess startup, YAML serialization, file writes, or any unexpected exception happens after the compose file has been rewritten, the original file is not guaranteed to be restored.
  • In practice, a failed proxy start/stop/recovery can leave the challenge service permanently remapped until manual cleanup.
• Recommended fix:
  • Make compose rewrite transactional: read once, write temp file, atomically replace, and always restore in finally on failure.
  • Treat backup creation/restoration as part of a single transaction, not best effort.
  • Add integration tests that force failures between rewrite and docker compose up.

2. The backend test suite gives false confidence because most API behavior is mocked at the router seam

• Files:
  • backend/tests/conftest.py:40-69
  • backend/tests/api/test_proxy.py
• Why it matters:
  • The API suite mounts a bespoke FastAPI() app instead of the real application lifecycle.
  • Authentication is bypassed through dependency overrides.
  • Core router behavior is tested by monkeypatching repository/service functions directly on the module under test.
  • This catches serialization and status codes, but misses real dependency wiring, startup/shutdown behavior, Mongo access, WebSocket/auth interplay, and real process/runtime interactions.
• Recommended fix:
  • Split tests into unit, integration, and system/perf layers.
  • Keep monkeypatch for narrow fault injection only.
  • Add real-app integration tests with actual lifespan, real Mongo-backed repositories, and realistic temp runtime directories.

High

3. Log streaming drops data after truncation or rotation

• File:
  • backend/app/api/proxy/router.py:188-213
• Why it matters:
  • On truncation, the stream sets offset = size.
  • If the log file is rotated or recreated with fresh content smaller than the old offset, the new bytes already present in the new file are skipped forever.
  • This is exactly the kind of subtle bug that makes operators distrust live logs.
• Recommended fix:
  • When size < offset, reset to 0 or resend a fresh snapshot and continue from the new file start.
  • Add a regression test that simulates truncation/recreation with immediate new log bytes.

4. The health monitor interval drifts under load

• File:
  • backend/app/services/mitmproxy/health_monitor.py:30-49
• Why it matters:
  • The loop sleeps after the cycle completes.
  • Real interval becomes check duration + HEALTHCHECK_INTERVAL_SECONDS.
  • As proxy count or timeout frequency rises, checks become slower and the dashboard silently becomes stale.
• Recommended fix:
  • Switch to fixed-schedule cadence using monotonic deadlines.
  • Track cycle duration and lag explicitly for observability.

5. Port reassignment in reverse mode has a race window and no reservation mechanism

• Files:
  • backend/app/integrations/docker/ports.py:4-13
  • backend/app/services/mitmproxy/manager.py:195-215
• Why it matters:
  • get_random_port() discovers a free port and returns it, but does not reserve it.
  • Another process can claim the port before Docker rebinds the service.
  • The failure mode is intermittent and hard to reproduce, which makes it a good candidate for production-only flakiness.
• Recommended fix:
  • Move to a reservation strategy or let Docker select the port and then discover the assigned binding.
  • At minimum, retry on bind collision and record the failure reason explicitly.

6. Backend packaging/import structure is fragile and currently fails full static analysis badly

• Files:
  • Broadly across backend/app/**
• Evidence:
  • uv run basedpyright app reports 86 errors and 243 warnings.
• Why it matters:
  • The codebase relies on pythonpath = ["app"] and implicit absolute imports such as from core.config import settings.
  • This is brittle outside the current dev/test invocation shape and blocks meaningful type-checking from being trustworthy.
  • Static type health is effectively unusable right now.
• Recommended fix:
  • Convert backend imports to package-consistent absolute imports from app... or explicit relative imports.
  • Add a real basedpyright target/config to CI once import hygiene is fixed.

7. The frontend control plane is a God object that now owns too many concerns

• File:
  • frontend/src/composables/useControlPlane.ts (1534 LOC)
• Why it matters:
  • One composable currently owns auth, routing, discovery, drafts, editor save flow, templates, logs, health, websocket reconnection, notices, modal state, and error routing.
  • This is a high-risk shape for regressions because unrelated fixes share the same reactive graph and watcher set.
  • State restoration and race-condition fixes will keep getting harder, not easier.
• Recommended fix:
  • Split into focused composables:
    • session/auth
    • proxy workspace state
    • discovery state
    • logs stream state
    • health stream state
    • wizard/settings draft state
  • Keep one thin composition root that wires them together.

8. Full refresh is all-or-nothing, so one failing endpoint blocks unrelated UI surfaces

• File:
  • frontend/src/composables/useControlPlane.ts:648-673
• Why it matters:
  • refreshAll() uses one Promise.all() for proxies, services, options, and templates.
  • If one endpoint fails, none of the others update.
  • A temporary failure in options or templates can block the basic proxy/discovery view.
• Recommended fix:
  • Separate critical and non-critical refreshes.
  • Fetch proxies/services independently from options/templates and degrade per panel.

Medium

9. Template API and UI contract are inconsistent

• Files:
  • backend/app/models/template.py:22-39
  • frontend/src/composables/useControlPlane.ts:1098-1126
  • frontend/src/composables/useControlPlane.ts:483-516
• Why it matters:
  • Backend models allow arbitrary protocol values and default to "custom".
  • The UI only exposes http and tcp lists.
  • A custom template persisted with any other protocol value becomes effectively invisible in the current UI.
• Recommended fix:
  • Validate template protocol/category on the backend to match supported UI semantics.
  • Or explicitly support additional categories in the UI instead of silently hiding them.

10. Template behavior is inconsistent with a predictable “block” contract

• File:
  • backend/app/services/mitmproxy/templates.py
• Why it matters:
  • Built-in “block” templates do not behave consistently:
    • some flow.kill()
    • some return empty 200
    • some return 404
    • some return 413 or 405
  • This may be intentional per challenge type, but as a product surface it is hard to reason about and document.
• Recommended fix:
  • Decide whether built-in templates are “semantic examples” or “operational policies”.
  • If they are operational policies, define a small set of standard outcomes and label deviations explicitly.

11. Synchronous filesystem work happens on the event loop in hot backend paths

• Files:
  • backend/app/services/mitmproxy/manager.py:287-303
  • backend/app/api/proxy/router.py:177-180
  • backend/app/integrations/docker/editor.py:39-51
• Why it matters:
  • read_bytes, write_text, YAML dump/write, and similar disk operations happen directly inside async request/runtime paths.
  • This is acceptable at tiny scale, but it will block the event loop during heavier operator usage or larger logs/files.
• Recommended fix:
  • Move blocking file I/O behind asyncio.to_thread() or a thin async wrapper for runtime/log/config file operations.

12. Health history limit is caller-controlled without a server-side cap

• File:
  • backend/app/api/proxy/router.py:387-397
• Why it matters:
  • limit is accepted directly from query params and forwarded to Mongo.
  • A large limit can create unnecessary memory and serialization pressure.
• Recommended fix:
  • Clamp the value server-side, for example 1..200 or similar.

13. app.state.services is stale mutable state with no real consumer value

• Files:
  • backend/app/main.py:47-56
  • backend/app/api/proxy/router.py:221-230
• Why it matters:
  • Service discovery results are written into app.state.services, but the state is not used as a trusted cache anywhere meaningful.
  • This adds mutable global state without clear ownership and can mislead future maintainers.
• Recommended fix:
  • Remove it or turn it into an intentional cache with invalidation and a real read path.

14. Docker compose editor leaves backups behind and does not manage them as lifecycle artifacts

• File:
  • backend/app/integrations/docker/editor.py:7-44
• Why it matters:
  • Backup files are created every time a port rewrite occurs.
  • They are never cleaned up or versioned intentionally.
  • Over time this pollutes challenge folders and complicates manual debugging.
• Recommended fix:
  • Either manage backups explicitly as runtime artifacts or stop generating long-lived .bak files per rewrite.

Low

15. Static Python highlighting in template preview is intentionally lightweight but very naive

• File:
  • frontend/src/components/TemplateCodePreview.vue:1-40
• Why it matters:
  • Regex-based highlighting on escaped HTML will mis-handle more complex Python constructs.
  • This is mostly cosmetic today, but it limits confidence in template readability as the library grows.
• Recommended fix:
  • Accept it as a lightweight preview or replace it with a read-only CodeMirror/Lezer-based renderer if template complexity grows.

16. Oversized files are now a maintenance signal on their own

• Evidence:
  • frontend/src/composables/useControlPlane.ts: 1534 LOC
  • frontend/src/styles.css: 1798 LOC
  • backend/app/api/proxy/router.py: 549 LOC
  • backend/app/services/mitmproxy/manager.py: 443 LOC
  • backend/tests/api/test_proxy.py: 859 LOC
• Why it matters:
  • Even when behavior is correct, files of this size hide coupling and slow review/debug cycles.
• Recommended fix:
  • Split by responsibility before the next major feature wave.

Roadmap

P0 — Correctness and operator safety

• Make Docker compose port rewrites transactional and exception-safe.
• Fix log stream truncation/rotation handling.
• Add a server-side cap for health history limits.

P1 — Confidence and hardening

• Restructure backend tests into real app/integration layers and reduce router-level monkeypatching.
• Fix import/package hygiene and make basedpyright a meaningful signal.
• Add explicit tests for reverse-mode port reassignment failure paths and orphan recovery.

P2 — Maintainability and resilience

• Split useControlPlane.ts into smaller composables.
• Separate refreshAll() into panel-aware refreshes.
• Remove or formalize app.state.services.
• Move blocking file I/O off the event loop.

P3 — Product quality and performance

• Add external load/perf harness for backend APIs, websockets, and lifecycle operations.
• Normalize built-in template contract and document intended operator semantics.
• Clean up runtime artifact and backup-file lifecycle policy.