Commit graph

215 commits

Author SHA1 Message Date
Marko Djordjevic
131d2912d5 fix: use chart.applyOptions() for theme changes instead of re-creating chart
Remove resolvedTheme from the chart initialization useEffect dependency
array so theme changes no longer destroy and re-create the entire chart.
Add a separate useEffect that calls chartRef.current.applyOptions() with
the new layout, grid, timeScale and rightPriceScale colors whenever
resolvedTheme changes, avoiding the memory leak and flicker caused by
full chart reconstruction on every theme toggle.

Closes task 8.2.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 15:19:44 +01:00
Marko Djordjevic
0cd7a34c99 fix: replace useState with useRef for preview primitive to prevent memory leak
The preview primitive in SpanAnnotationManager was stored in useState,
causing unnecessary re-renders and potential memory leaks since the
primitive was not cleaned up on unmount. Changed to useRef, updated all
read/write sites, removed from dependency arrays, and added unmount
cleanup effect that detaches the primitive from the series.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 15:18:48 +01:00
Marko Djordjevic
e5b0cc2540 fix: resolve stale closure in SpanAnnotationManager keyboard handler
Use selectedSpanIdRef to avoid capturing stale selectedSpanId in the
keyboard event handler closure. Wrap handleDeleteSpan in useCallback
with stable dependencies (reads selectedSpanIdRef.current instead of
the prop directly). Remove selectedSpanId from the useEffect dependency
array since the ref is used instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 15:16:55 +01:00
Marko Djordjevic
73c88f8211 fix: resolve stale closures in CandleChart click handler
Convert drawingState, selectedLineId, dragState, and annotations to
refs that stay in sync with state via useEffect. The chart click handler
(subscribeClick) captured stale closure values for these variables.
Now reads from refs (e.g. drawingStateRef.current) so the handler
always sees the latest state, and removed the stale state variables
from the useEffect dependency array.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 15:15:24 +01:00
Marko Djordjevic
45a23047dd fix: add AbortController to fetchPredictions and handleFetchBatchPredictions
Prevent race conditions by aborting in-flight requests when a new
request is triggered. Each function now:
- Aborts the previous request via a stored AbortController ref
- Passes signal to all fetch() calls
- Silently discards AbortError in catch blocks

Completes task 7.2.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 15:12:40 +01:00
Marko Djordjevic
838c063b5b mark task 7.1 done: stale closure fix for modelInfo already applied
The modelInfoRef with useEffect sync was already in place in page.tsx
(lines 201-205), and fetchPredictions/generateCacheKey already use
modelInfoRef.current instead of the stale closure value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 15:11:20 +01:00
Marko Djordjevic
300752da3a fix: use curl consistently for healthcheck in Dockerfile and docker-compose.yml
- Replace wget with curl in root Dockerfile healthcheck command (line 50)
- Add curl to apk dependencies in root Dockerfile (line 22)
- Align healthcheck parameters between Dockerfile and docker-compose.yml
- Mark task 6.8 as complete

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:37:52 +01:00
Marko Djordjevic
5896e56faa feat: add sha256 pinning TODO comments to both Dockerfiles
Add TODO comments above each FROM instruction in Dockerfile and
services/ml/Dockerfile instructing how to pin base images to sha256
digests for reproducible builds. Marks task 6.7 as complete.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:37:17 +01:00
Marko Djordjevic
38b270140e fix: remove unnecessary node_modules copy from Dockerfile
Standalone Next.js output includes all required dependencies in the
/app/.next/standalone directory, making the separate node_modules
copy unnecessary. This reduces image size and build complexity.

Closes task 6.6.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:36:35 +01:00
Marko Djordjevic
e146de2e05 security: add SHA256 checksum verification for TA-Lib download in ML Dockerfile
Splits the monolithic TA-Lib build RUN command to insert an ARG for the
expected SHA256 hash and a sha256sum -c verification step immediately after
the wget download, before extraction and build. Marks task 6.5 complete.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:36:06 +01:00
Marko Djordjevic
a6e0697ab5 fix: Change TA-Lib download URL to HTTPS in Dockerfile
Updated the wget command in services/ml/Dockerfile line 10 to use HTTPS instead of HTTP for downloading TA-Lib source. This improves security by ensuring encrypted transport.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:35:15 +01:00
Marko Djordjevic
b0e4a618f0 fix: update .dockerignore with all required entries and mark task 6.3 complete 2026-02-18 11:34:52 +01:00
Marko Djordjevic
1438e474e8 security: add non-root appuser to services/ml/Dockerfile
Create system user appuser with useradd, set ownership of /app,
and switch to non-root user before CMD to reduce container attack surface.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:34:26 +01:00
Marko Djordjevic
34e543ea96 feat: add security headers to next.config.js (task 6.1)
Add async headers() function with X-Frame-Options, X-Content-Type-Options,
Referrer-Policy, Permissions-Policy, and Content-Security-Policy headers
applied to all routes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:33:52 +01:00
Marko Djordjevic
3d8672121e feat: add run_id format validation to GET /training/runs/{run_id} endpoint
- Add new GET endpoint for retrieving a specific training run by run_id
- Validate run_id format with regex pattern ^[a-zA-Z0-9_-]+$ before DB access
- Return HTTP 400 for invalid run_id format, HTTP 404 for non-existent runs
- Ensure DELETE endpoint validation is correctly placed before any DB access
- Both endpoints now provide consistent security validation
- Mark task 5.8 as completed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:33:12 +01:00
Marko Djordjevic
3dc0014328 feat: add training resource limits (500MB size check + 30-min timeout)
- Import concurrent.futures for timeout support
- In _run_training_background: check df.memory_usage(deep=True).sum()
  after loading the labeled dataset; raise ValueError if > 500MB
- Wrap model.fit() in a ThreadPoolExecutor with a 1800s timeout;
  on TimeoutError update DB status to "failed" with message
  "Training timed out after 30 minutes" and return early
- Mark task 5.7 as done in openspec/changes/code-review-fix/tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:31:33 +01:00
Marko Djordjevic
f94d16c6ab fix: implement real health checks in ML service /health endpoint
- Execute SELECT 1 against PostgreSQL via SQLAlchemy session to verify DB connectivity
- HTTP GET to ${MLFLOW_TRACKING_URI}/health (default: http://localhost:5000/health) to verify MLflow
- Return HTTP 503 if any component is unhealthy, HTTP 200 only when both are healthy
- Values: "healthy" / "unhealthy" for database and mlflow fields
- Add `import requests as http_requests` and `sa_text` imports

Closes task 5.6 in openspec/changes/code-review-fix/tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:30:12 +01:00
Marko Djordjevic
d75b05b585 feat: add candle time-sort and duplicate timestamp validation to POST /predict
Sort incoming candles by their time field in ascending chronological order
before processing to ensure correct feature engineering regardless of input
order. Also validate that no duplicate timestamps are present, returning
HTTP 400 with details if duplicates are found.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:28:02 +01:00
Marko Djordjevic
ff952aa083 feat(ml): add date range validation to POST /predict/batch
- Parse start_date and end_date as datetime objects
- Return HTTP 400 if end_date is before start_date
- Return HTTP 400 if date range exceeds 365 days (1 year)

Closes task 5.4 in code-review-fix tasks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:27:12 +01:00
Marko Djordjevic
b9beea1574 fix(ml): add _model_swap_lock to prediction reads for thread-safe model access
In /predict and /predict/batch endpoints, grab the model reference under
_model_swap_lock before running inference. Inference itself runs outside
the lock (using a local variable) to avoid blocking model swaps during
potentially slow computation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:26:33 +01:00
Marko Djordjevic
ff15adc847 feat: add SHA256 model integrity check before joblib.load()
Add verify_model_checksum() that validates model files against a
models/checksums.sha256 manifest before loading. Fails open when
manifest is missing or file not listed (backward compat), raises
HTTP 500 on hash mismatch. Created empty manifest placeholder.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:25:14 +01:00
Marko Djordjevic
b7f9b2e04d security: replace exception details with generic error in ML service HTTP responses
Replace all instances of `detail=str(e)`, `detail=f"...{exc}"`, and similar
patterns that exposed internal exception messages to HTTP clients in
services/ml/app/main.py. All exception details are now logged server-side
only via logger.error(), while clients receive a generic "Internal server error"
message. Fixes 9 handlers across predict, batch predict, pattern detection,
training start, training runs fetch, training run delete, dataset info,
build dataset, and model load endpoints.

Mark task 5.1 as completed in tasks.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:23:41 +01:00
Marko Djordjevic
4c53ef7cae fix: add response.ok checks before .json() in CandleChart.tsx
Added HTTP error checks before calling .json() in the three fetch
functions (fetchCandles, fetchAnnotations, fetchAnnotationTypes)
to prevent silent failures on non-2xx responses.

Marks task 4.12 as complete in tasks.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:21:54 +01:00
Marko Djordjevic
4436cd655f fix: add response.ok checks before .json() in page.tsx fetch calls
Guard all four fetch() calls in src/app/page.tsx against non-2xx HTTP
responses by throwing before attempting to parse the body as JSON.
Affected functions: fetchCharts, fetchAnnotations, fetchSpanAnnotations,
fetchSpanLabelTypes.

Marks task 4.11 as completed in code-review-fix/tasks.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:21:21 +01:00
Marko Djordjevic
b2129ad626 security: add CSV injection protection to all export routes
Add sanitizeCsvCell() helper to both export routes that prefixes cell
values starting with =, +, @, or - with a single quote to prevent CSV
formula injection attacks.

Applied to:
- src/app/api/export/route.ts: timestamp and label_type columns
- src/app/api/span-annotations/export/route.ts: start_time, end_time,
  label, and outcome columns

Closes task 4.10.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:20:36 +01:00
Marko Djordjevic
160f146ab4 mark: task 4.9 complete - integer parameter parsing with isNaN guards 2026-02-18 11:19:34 +01:00
Marko Djordjevic
1678da2d9d fix: wrap chart cascade delete in db.transaction() and add spanAnnotations deletion
- Import spanAnnotations from schema
- Wrap all delete operations in db.transaction() for atomicity
- Delete spanAnnotations first to satisfy FK constraints, then annotations, candles, chart
- Mark task 4.8 as done in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:17:19 +01:00
Marko Djordjevic
103bfa89cb fix: require chartId for bulk delete in annotations route (task 4.7)
Reject DELETE ?all=true without chartId with HTTP 400 to prevent
accidental deletion of annotations across all charts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:16:37 +01:00
Marko Djordjevic
aace19b7f4 fix: replace error.message with generic "Internal server error" in all API catch blocks
Prevents leaking internal error details to clients across 7 route files:
health, candles, annotations, annotations/[id], upload, export, span-annotations/export.
Server-side console.error logging preserved for debugging.

Closes task 4.6.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:16:02 +01:00
Marko Djordjevic
81e3554d82 feat: add Zod schema validation to patterns/detect route
- Import z from zod
- Add CandleSchema validating time, open, high, low, close (number), volume (optional number)
- Add PatternDetectRequestSchema validating candles array and patterns array of non-empty strings
- Use safeParse() and return HTTP 400 with error details on validation failure
- Forward only validated data to the inference service
- Mark task 4.5 as completed in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:14:36 +01:00
Marko Djordjevic
2e02d155af feat: add Zod schema validation to training/start route (task 4.4)
Validates model_type as a non-empty string using .safeParse(); returns
HTTP 400 with error details on invalid input. Marks task 4.4 as done.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:14:00 +01:00
Marko Djordjevic
4cffc223b3 feat: add Zod schema validation to model/load route
Validate run_id in POST /api/model/load using Zod:
- run_id must be a non-empty string matching /^[a-zA-Z0-9_-]+$/
- Returns HTTP 400 with error details if validation fails
- Validated data is forwarded to the inference service

Marks task 4.3 as complete in tasks.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:13:13 +01:00
Marko Djordjevic
5c399037c3 feat: add Zod validation to predict/batch route (task 4.2)
Add BatchPredictRequestSchema with Zod to validate pair, timeframe,
start_date, and end_date fields. Returns HTTP 400 with flattened error
details on invalid input. Forward only validated data to the inference
service.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:12:38 +01:00
Marko Djordjevic
3361236d3f feat: add Zod schema validation to predict API route
- Add CandleSchema validating time, open, high, low, close (number) and optional volume
- Add PredictRequestSchema validating pair (non-empty string), timeframe (non-empty string), candles array
- Use safeParse() and return HTTP 400 with error details on invalid input
- Forward only validated data to the inference service
- Mark task 4.1 as done in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:11:58 +01:00
Marko Djordjevic
c023702644 feat: add API_KEY to .env.example with placeholder and instructions
- Add API_KEY environment variable with placeholder value 'change_me_to_a_strong_random_key'
- Include helpful comment explaining its purpose: authentication between Next.js and ML service
- Provide command for generating strong random value: openssl rand -hex 32
- Mark task 3.4 as completed
2026-02-18 11:06:47 +01:00
Marko Djordjevic
4a3e4a48ba feat: forward X-API-Key header from Next.js proxy routes to ML service
All 12 Next.js API routes that proxy requests to the ML service
(INFERENCE_API_URL / localhost:8001) now include the X-API-Key header
read from process.env.API_KEY. Affected routes:
- /api/predict
- /api/predict/batch
- /api/model/info
- /api/model/load
- /api/training/start
- /api/training/runs
- /api/training/runs/[run_id] (DELETE)
- /api/training/dataset-info
- /api/training/active
- /api/training/build-dataset
- /api/patterns/available
- /api/patterns/detect

Marks task 3.3 as complete in openspec/changes/code-review-fix/tasks.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:06:18 +01:00
Marko Djordjevic
5f569d9134 feat(ml): add API key authentication via FastAPI Depends() on all endpoints except /health
- Import Header, Depends, Security from fastapi
- Add verify_api_key dependency: reads API_KEY env var, checks X-API-Key
  header, raises HTTP 401 if key mismatch; fail-open if env var not set
- Apply Depends(verify_api_key) to all 14 non-health endpoints
- /health endpoint remains unauthenticated for liveness probes
- Mark task 3.2 as complete in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:04:41 +01:00
Marko Djordjevic
577bb2e56e feat: add API key auth middleware for /api/* routes (task 3.1)
- Create src/middleware.ts with Next.js middleware
- Reads API_KEY env var and checks X-API-Key header on all /api/* routes
- Skips auth for /api/health endpoint
- Fails open (with warning) when API_KEY is not configured
- Returns 401 Unauthorized when key is missing or mismatched
- Mark task 3.1 as complete in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:02:51 +01:00
Marko Djordjevic
0cd21887e4 mark: task 2.5 complete (CORS configuration) 2026-02-18 11:02:10 +01:00
Marko Djordjevic
94bc5768d1 feat: add file type validation to upload endpoint
- Validate filename ends with .csv (case-insensitive)
- Validate MIME type is text/* or application/csv or text/csv
- Return HTTP 400 with error message if validation fails
- Mark task 2.4 as complete
2026-02-18 11:01:28 +01:00
Marko Djordjevic
0e239dc3da security: add file size (10MB) and row count (500k) limits to upload route
- Reject uploads larger than 10MB before reading file content
- Reject CSVs with more than 500,000 data rows after parsing
- Checks placed as early as possible in the handler flow
- Mark task 2.3 as done in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:01:02 +01:00
Marko Djordjevic
67dd7aa2f0 security: validate run_id format and add path containment check in ML service
- Add `import re` to services/ml/app/main.py
- In POST /model/load: validate run_id matches ^[a-zA-Z0-9_-]+$ before DB lookup; use Path.resolve() + directory containment check before loading model artifact
- In DELETE /training/runs/{run_id}: validate run_id matches ^[a-zA-Z0-9_-]+$ before any processing; use Path.resolve() + directory containment check before deleting model artifact
- Both endpoints return HTTP 400 with {"detail": "Invalid run_id format"} on invalid input
- Mark task 2.2 as completed in openspec/changes/code-review-fix/tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 11:00:19 +01:00
Marko Djordjevic
870f92d208 feat: add run_id format validation in DELETE training/runs endpoint
Validate that run_id matches /^[a-zA-Z0-9_-]+$ regex before interpolating into the API URL.
Returns HTTP 400 with 'Invalid run_id format' error if validation fails.
This prevents potential URL injection attacks and invalid identifier usage.
2026-02-18 10:58:54 +01:00
Marko Djordjevic
4e5ce321b9 chore: bind ML service port to 127.0.0.1:8001:8001 for localhost-only access
- Changed ML service port binding from 8001:8001 to 127.0.0.1:8001:8001 in docker-compose.yml
- Marks task 1.8 as complete in tasks.md
2026-02-18 10:58:31 +01:00
Marko Djordjevic
c327ba3370 bind: MLflow port to 127.0.0.1:5000:5000 in docker-compose.yml
Changes:
- Updated docker-compose.yml MLflow service port binding from 5000:5000 to 127.0.0.1:5000:5000
  to restrict access to localhost only for security
- Marked task 1.7 as complete in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 10:58:11 +01:00
Marko Djordjevic
9efa1dbbcc fix: Bind PostgreSQL port to 127.0.0.1:5432:5432 for localhost-only access
- Changed PostgreSQL service port binding from 5432:5432 to 127.0.0.1:5432:5432 in docker-compose.yml
- This restricts PostgreSQL to listen only on localhost, improving security by preventing access from other interfaces
- Marked task 1.6 as completed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 10:57:55 +01:00
Marko Djordjevic
e3469ec39f fix: replace hardcoded DB credentials with env var interpolation in docker-compose.yml
All DATABASE_URL values and postgres service env vars now use
\${POSTGRES_USER}, \${POSTGRES_PASSWORD}, \${POSTGRES_DB} interpolation
instead of hardcoded ml_user/ml_password/candle_annotator values.
Also updated pg_isready healthcheck to use the same env vars.

Closes task 1.5.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 10:57:31 +01:00
Marko Djordjevic
9bc82b822c security: remove credential SQL comments and add DATABASE_URL fail-fast check
- Remove hardcoded SQL comments containing 'ml_user' and 'ml_password'
- Remove fallback default credentials in DATABASE_URL construction
- Add fail-fast validation: raise RuntimeError if DATABASE_URL env var is missing or empty
- Mark task 1.4 as complete in code-review-fix/tasks.md
2026-02-18 10:56:49 +01:00
Marko Djordjevic
55ee9c936a fix: replace real credentials in .env.example with placeholders
- Replace ml_password with change_me_to_a_strong_password placeholder
- Replace ml_user with your_db_user placeholder
- Mark task 1.3 as completed in tasks.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-02-18 10:56:23 +01:00
Marko Djordjevic
099f334fe9 chore: mark task 1.2 as completed 2026-02-18 10:55:56 +01:00