id hardening fix, from coderabbit report
This commit is contained in:
@@ -10,14 +10,15 @@ from wf_core import PersistedRunState
|
||||
|
||||
from ..models import DependencyDiagnostic, WorkflowArtifact, WorkflowDeployment
|
||||
|
||||
RUN_ID_PATTERN = r"^[A-Za-z0-9_.-]+$"
|
||||
RUN_ID_PATTERN = r"^[A-Za-z0-9_][A-Za-z0-9_.-]*$"
|
||||
|
||||
|
||||
def ensure_run_id(run_id: str) -> str:
|
||||
"""Reject ids that cannot safely identify one local run directory."""
|
||||
if not re.fullmatch(RUN_ID_PATTERN, run_id):
|
||||
raise ValueError(
|
||||
"run_id must match [A-Za-z0-9_.-]+; path separators are not allowed"
|
||||
"run_id must start with alphanumeric or underscore and contain only "
|
||||
"[A-Za-z0-9_.-]"
|
||||
)
|
||||
return run_id
|
||||
|
||||
@@ -65,7 +66,7 @@ class WorkflowRunRecord(BaseModel):
|
||||
status: StoredRunStatus
|
||||
resume_readiness: ResumeReadiness
|
||||
environment: PinnedRunEnvironment
|
||||
latest_checkpoint_id: str
|
||||
latest_checkpoint_id: str = Field(pattern=RUN_ID_PATTERN)
|
||||
diagnostics: list[DependencyDiagnostic] = Field(default_factory=list)
|
||||
created_at: datetime
|
||||
updated_at: datetime
|
||||
|
||||
@@ -3,6 +3,7 @@ from __future__ import annotations
|
||||
from datetime import UTC, datetime
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from wf_artifacts import (
|
||||
CheckpointReason,
|
||||
@@ -106,5 +107,13 @@ def test_file_run_store_lists_runs_and_checkpoints_in_order(tmp_path) -> None:
|
||||
def test_file_run_store_rejects_unsafe_run_id(tmp_path) -> None:
|
||||
store = FileRunStore(tmp_path)
|
||||
|
||||
with pytest.raises(ValueError, match="run_id must match"):
|
||||
with pytest.raises(ValueError, match="run_id must start"):
|
||||
store.get_run("../outside")
|
||||
|
||||
with pytest.raises(ValueError, match="run_id must start"):
|
||||
store.get_run(".hidden_run")
|
||||
|
||||
|
||||
def test_workflow_run_record_validates_latest_checkpoint_id() -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
run_record("run_123", "../outside")
|
||||
|
||||
Reference in New Issue
Block a user