VibecoderMcSwaggins commited on
Commit
cfeab47
·
1 Parent(s): c3e7865

fix: apply CodeRabbit feedback (security + lint)

Browse files

Addresses 2 CodeRabbit review comments:

1. CRITICAL - Path traversal guard in files.py:
- Changed str(resolved).startswith(str(base)) to resolved.is_relative_to(base_dir)
- The startswith check could be bypassed with prefix collisions
- Example: /tmp/stroke-results-evil/file.txt would pass startswith
but correctly fails is_relative_to()

2. Minor - React lint error in CaseSelector.tsx:
- Moved setIsLoading(false) from .finally() into fetchCases callback
- Fixes react-hooks/set-state-in-effect lint rule violation
- setState should not be called synchronously within effect body

Tests: 69/69 frontend tests pass

frontend/src/components/CaseSelector.tsx CHANGED
@@ -30,6 +30,7 @@ export function CaseSelector({
30
  setCases(data.cases);
31
  setIsWakingUp(false);
32
  setRetryCount(0);
 
33
  return; // Success
34
  } catch (err) {
35
  if (err instanceof Error && err.name === "AbortError") return;
@@ -63,6 +64,7 @@ export function CaseSelector({
63
  : "Unknown error";
64
  setError(`Failed to load cases: ${message}`);
65
  setIsWakingUp(false);
 
66
  return;
67
  }
68
  }
@@ -70,13 +72,7 @@ export function CaseSelector({
70
 
71
  useEffect(() => {
72
  const abortController = new AbortController();
73
-
74
- fetchCases(abortController.signal).finally(() => {
75
- if (!abortController.signal.aborted) {
76
- setIsLoading(false);
77
- }
78
- });
79
-
80
  return () => abortController.abort();
81
  }, [fetchCases]);
82
 
 
30
  setCases(data.cases);
31
  setIsWakingUp(false);
32
  setRetryCount(0);
33
+ setIsLoading(false);
34
  return; // Success
35
  } catch (err) {
36
  if (err instanceof Error && err.name === "AbortError") return;
 
64
  : "Unknown error";
65
  setError(`Failed to load cases: ${message}`);
66
  setIsWakingUp(false);
67
+ setIsLoading(false);
68
  return;
69
  }
70
  }
 
72
 
73
  useEffect(() => {
74
  const abortController = new AbortController();
75
+ fetchCases(abortController.signal);
 
 
 
 
 
 
76
  return () => abortController.abort();
77
  }, [fetchCases]);
78
 
src/stroke_deepisles_demo/api/files.py CHANGED
@@ -48,9 +48,12 @@ async def get_result_file(job_id: str, case_id: str, filename: str) -> FileRespo
48
  file_path = RESULTS_DIR / job_id / case_id / filename
49
 
50
  # Security: Ensure path doesn't escape RESULTS_DIR (path traversal protection)
 
 
51
  try:
 
52
  resolved = file_path.resolve()
53
- if not str(resolved).startswith(str(RESULTS_DIR.resolve())):
54
  logger.warning("Path traversal attempt blocked: %s", filename)
55
  raise HTTPException(status_code=404, detail="File not found")
56
  except (OSError, ValueError):
 
48
  file_path = RESULTS_DIR / job_id / case_id / filename
49
 
50
  # Security: Ensure path doesn't escape RESULTS_DIR (path traversal protection)
51
+ # Using is_relative_to() instead of startswith() to prevent prefix-collision bypass
52
+ # e.g., /tmp/stroke-results-evil/file.txt would pass startswith but fail is_relative_to
53
  try:
54
+ base_dir = RESULTS_DIR.resolve()
55
  resolved = file_path.resolve()
56
+ if not resolved.is_relative_to(base_dir):
57
  logger.warning("Path traversal attempt blocked: %s", filename)
58
  raise HTTPException(status_code=404, detail="File not found")
59
  except (OSError, ValueError):