Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pmoves/services/gateway/gateway/api/chit.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os, json, base64, hashlib, logging
from pathlib import Path
from typing import Any, Dict, List, Optional, Sequence

from fastapi import APIRouter, HTTPException
Expand Down Expand Up @@ -233,11 +234,13 @@
safe_name = os.path.basename(CHIT_CODEBOOK_PATH or "codebook.jsonl")
if not _SAFE_FILENAME.match(safe_name):
raise HTTPException(status_code=400, detail="Invalid codebook filename")
codebook_dir = os.path.dirname(CHIT_CODEBOOK_PATH or "tests/data/codebook.jsonl") or "."
path = os.path.normpath(os.path.join(codebook_dir, safe_name))
# Ensure resolved path stays within the codebook directory
if not path.startswith(os.path.normpath(codebook_dir)):
codebook_dir = Path(CHIT_CODEBOOK_PATH or "tests/data/codebook.jsonl").parent
if not codebook_dir.exists():
codebook_dir = Path(".")
resolved = (codebook_dir / safe_name).resolve()
Comment thread Dismissed
if not resolved.is_relative_to(codebook_dir.resolve()):
raise HTTPException(status_code=400, detail="Invalid codebook path")
path = str(resolved)
else:
path = CHIT_CODEBOOK_PATH or "tests/data/codebook.jsonl"
items = []
Expand Down
18 changes: 12 additions & 6 deletions pmoves/services/gateway/gateway/api/viz.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
from fastapi import APIRouter, HTTPException, Query
from fastapi.responses import Response, HTMLResponse
from pathlib import Path
from typing import List, Dict, Any, Optional
import json, os, math, re

_SAFE_SHAPE_RE = re.compile(r"^[a-zA-Z0-9._-]+$")
DATA_DIR = Path("data").resolve()

from gateway.api.chit import Constellation, CGP, decode_constellations

Expand Down Expand Up @@ -107,10 +109,12 @@
safe_id = os.path.basename(shape_id)
if not safe_id or safe_id != shape_id or not _SAFE_SHAPE_RE.match(safe_id):
raise HTTPException(status_code=400, detail="invalid shape_id")
path = os.path.join("data", f"{safe_id}.json")
if not os.path.exists(path):
resolved = (DATA_DIR / f"{safe_id}.json").resolve()
Comment thread Dismissed
if not resolved.is_relative_to(DATA_DIR):
raise HTTPException(status_code=400, detail="invalid shape_id")
if not resolved.exists():
Comment thread Dismissed
raise HTTPException(status_code=404, detail="shape not found")
with open(path, "r", encoding="utf-8") as f:
with open(resolved, "r", encoding="utf-8") as f:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI 7 months ago

General approach: Ensure that any path derived from user input is strictly confined to a known safe root directory and cannot escape it via path traversal, absolute paths, symlinks, or unusual DATA_DIR values. This is done by normalizing the combined path, verifying that it resides within the safe directory (using canonical Path operations), and rejecting anything that does not meet the constraints.

Specific fix for this code:

  1. Keep the existing lexical checks on shape_id (basename equality and _SAFE_SHAPE_RE) to reject obvious bad inputs and enforce a simple filename format.
  2. Strengthen the directory containment check by:
    • Constructing the path as DATA_DIR / f"{safe_id}.json".
    • Calling .resolve(strict=False) to normalize the path and follow symlinks.
    • Using resolved.relative_to(DATA_DIR) inside a try/except ValueError to verify that resolved is actually inside DATA_DIR. This is a clearer and more robust idiom than is_relative_to for static analysis tools and works across Python versions.
  3. Keep the existence check and file opening as-is after the containment check succeeds; no change in functionality.

Concrete changes (in pmoves/services/gateway/gateway/api/viz.py):

  • Replace the resolved = ... and if not resolved.is_relative_to(DATA_DIR) block with a resolve(strict=False) call and a try: resolved.relative_to(DATA_DIR) ... except ValueError pattern, while preserving error handling and HTTP status codes.
  • No new imports are needed; pathlib.Path is already imported.

This preserves existing behavior for valid shape_id values but makes the containment logic more explicit and likely to satisfy CodeQL’s expectations.

Suggested changeset 1
pmoves/services/gateway/gateway/api/viz.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/pmoves/services/gateway/gateway/api/viz.py b/pmoves/services/gateway/gateway/api/viz.py
--- a/pmoves/services/gateway/gateway/api/viz.py
+++ b/pmoves/services/gateway/gateway/api/viz.py
@@ -109,8 +109,10 @@
     safe_id = os.path.basename(shape_id)
     if not safe_id or safe_id != shape_id or not _SAFE_SHAPE_RE.match(safe_id):
         raise HTTPException(status_code=400, detail="invalid shape_id")
-    resolved = (DATA_DIR / f"{safe_id}.json").resolve()
-    if not resolved.is_relative_to(DATA_DIR):
+    resolved = (DATA_DIR / f"{safe_id}.json").resolve(strict=False)
+    try:
+        resolved.relative_to(DATA_DIR)
+    except ValueError:
         raise HTTPException(status_code=400, detail="invalid shape_id")
     if not resolved.exists():
         raise HTTPException(status_code=404, detail="shape not found")
EOF
@@ -109,8 +109,10 @@
safe_id = os.path.basename(shape_id)
if not safe_id or safe_id != shape_id or not _SAFE_SHAPE_RE.match(safe_id):
raise HTTPException(status_code=400, detail="invalid shape_id")
resolved = (DATA_DIR / f"{safe_id}.json").resolve()
if not resolved.is_relative_to(DATA_DIR):
resolved = (DATA_DIR / f"{safe_id}.json").resolve(strict=False)
try:
resolved.relative_to(DATA_DIR)
except ValueError:
raise HTTPException(status_code=400, detail="invalid shape_id")
if not resolved.exists():
raise HTTPException(status_code=404, detail="shape not found")
Copilot is powered by AI and may make mistakes. Always verify output.
obj = json.load(f)
try:
cgp = CGP.model_validate(obj)
Expand Down Expand Up @@ -194,10 +198,12 @@
safe_id = os.path.basename(shape_id)
if not safe_id or safe_id != shape_id or not _SAFE_SHAPE_RE.match(safe_id):
raise HTTPException(status_code=400, detail="invalid shape_id")
path = os.path.join("data", f"{safe_id}.json")
if not os.path.exists(path):
resolved = (DATA_DIR / f"{safe_id}.json").resolve()
Comment thread Dismissed
if not resolved.is_relative_to(DATA_DIR):
raise HTTPException(status_code=400, detail="invalid shape_id")
if not resolved.exists():
Comment thread Dismissed
raise HTTPException(status_code=404, detail="shape not found")
obj = json.loads(open(path, "r", encoding="utf-8").read())
obj = json.loads(resolved.read_text(encoding="utf-8"))

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI 7 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

cgp = CGP.model_validate(obj)
out = []
for si, s in enumerate(cgp.super_nodes):
Expand Down
Loading