Skip to content

Commit 0d0cf22

Browse files
committed
fix: address CodeRabbit MAJOR review comments (round 2, PR #20)
- _server: neutralize CR/LF in request path before logging to prevent log forging (CWE-117); ASGI percent-decodes scope["path"] - schema_cast: infer cast direction from the last non-UUID-tail segment so combined anonymous IDs are handled correctly (was reading the UUID-tail segment's None minor and returning "unknown"); add regression test - Makefile: add Makefile to $(PY_ENV_STAMP) prerequisites so tool-set changes (ruff/mypy) invalidate an existing stamp and get reinstalled Signed-off-by: Artfizer <artifizer@gmail.com>
1 parent 44659b5 commit 0d0cf22

4 files changed

Lines changed: 28 additions & 5 deletions

File tree

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ help:
3838
# Create/update the virtual environment and install dev/test dependencies
3939
py-env: $(PY_ENV_STAMP)
4040

41-
$(PY_ENV_STAMP): gts/pyproject.toml .gts-spec/tests/requirements.txt
41+
$(PY_ENV_STAMP): gts/pyproject.toml .gts-spec/tests/requirements.txt Makefile
4242
@echo "Creating/updating Python virtual environment in $(PY_ENV_DIR)..."
4343
$(PYTHON_BOOTSTRAP) -m venv $(PY_ENV_DIR)
4444
$(PYTHON) -m pip install --upgrade pip

‎gts/src/gts/_server.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,14 @@ async def receive():
7070
else:
7171
status_color = Colors.RED
7272

73-
# Log response at INFO level (verbose >= 1)
73+
# Log response at INFO level (verbose >= 1).
74+
# Neutralize CR/LF in the request-derived path to prevent log forging
75+
# (CWE-117); ASGI percent-decodes scope["path"], so it may contain
76+
# newlines that would otherwise inject forged log records.
77+
safe_path = request.url.path.replace("\r", "\\r").replace("\n", "\\n")
7478
logger.info(
7579
f"{Colors.CYAN}{request.method}{Colors.RESET} "
76-
f"{Colors.BLUE}{request.url.path}{Colors.RESET} -> "
80+
f"{Colors.BLUE}{safe_path}{Colors.RESET} -> "
7781
f"{status_color}{response.status_code}{Colors.RESET} "
7882
f"in {Colors.MAGENTA}{dur:.1f}ms{Colors.RESET}"
7983
)

‎gts/src/gts/schema_cast.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,19 @@ def cast(
183183

184184
@staticmethod
185185
def _infer_direction(from_id: str, to_id: str) -> str:
186+
def _last_versioned_segment(gid: GtsID):
187+
# Skip the appended UUID-tail segment (ver_minor is None) so
188+
# combined anonymous IDs still resolve to their versioned segment.
189+
for seg in reversed(gid.gts_id_segments):
190+
if not getattr(seg, "_is_uuid_tail", False):
191+
return seg
192+
return gid.gts_id_segments[-1]
193+
186194
try:
187195
gid_from = GtsID(from_id)
188196
gid_to = GtsID(to_id)
189-
from_minor = gid_from.gts_id_segments[-1].ver_minor
190-
to_minor = gid_to.gts_id_segments[-1].ver_minor
197+
from_minor = _last_versioned_segment(gid_from).ver_minor
198+
to_minor = _last_versioned_segment(gid_to).ver_minor
191199
if from_minor is not None and to_minor is not None:
192200
if to_minor > from_minor:
193201
return "up"

‎tests/test_schema_cast.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,17 @@ def test_none_direction_same_minor(self):
7171
def test_unknown_on_invalid_id(self):
7272
assert GtsEntityCastResult._infer_direction("not-an-id", "also-not") == "unknown"
7373

74+
def test_combined_anonymous_id_uses_versioned_segment(self):
75+
# Regression: the appended UUID-tail segment has ver_minor=None; the
76+
# direction must be inferred from the last versioned segment instead.
77+
assert (
78+
GtsEntityCastResult._infer_direction(
79+
"gts.x.test._.foo.v1.0~123e4567-e89b-12d3-a456-426614174000",
80+
"gts.x.test._.foo.v1.5~",
81+
)
82+
== "up"
83+
)
84+
7485

7586
class TestEffectiveObjectSchema:
7687
def test_non_dict_returns_empty(self):

0 commit comments

Comments
 (0)