fix: harden preprocessing state and child capabilities
This commit is contained in:
@@ -11,6 +11,14 @@ from tht.ports.vector import VectorStoreError
|
||||
from tht.vectorstore.embeddings import EmbeddingsError
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _child_capability_for_pipeline_unit_tests(monkeypatch):
|
||||
# These tests exercise pipeline result/JSON behavior; process-boundary
|
||||
# authorization is covered by test_workspace_writer_lock.py.
|
||||
import tht.cli.preprocess_cmd as command
|
||||
monkeypatch.setattr(command, "_require_writer_capability", lambda **kwargs: None)
|
||||
|
||||
|
||||
def test_preprocess_evidence_json_is_pristine(monkeypatch, tmp_path):
|
||||
import tht.cli.preprocess_cmd as command
|
||||
|
||||
|
||||
@@ -5,12 +5,21 @@ from datetime import UTC, datetime
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from tht.cli import app
|
||||
from tht.memory import MemoryRecord, save_registry
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _child_capability_for_vector_unit_tests(monkeypatch):
|
||||
import tht.cli.preprocess_cmd as preprocess
|
||||
import tht.cli.vector_cmd as vector
|
||||
monkeypatch.setattr(preprocess, "_require_writer_capability", lambda **kwargs: None)
|
||||
monkeypatch.setattr(vector, "_require_writer_capability", lambda **kwargs: None)
|
||||
|
||||
|
||||
class _FakeEmbedder:
|
||||
def embed_documents(self, documents):
|
||||
return [[0.1] * 4 for _ in documents]
|
||||
|
||||
@@ -24,6 +24,12 @@ from tht.mschema.models import (
|
||||
from tht.mschema.render import to_mschema_text, to_schema_dict
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _child_capability_for_schema_unit_tests(monkeypatch):
|
||||
import tht.cli.schema_cmd as command
|
||||
monkeypatch.setattr(command, "_require_writer_capability", lambda **kwargs: None)
|
||||
|
||||
|
||||
def _physical():
|
||||
return PhysicalSchema(
|
||||
database="d", schema="s", introspected_at=datetime(2026, 1, 1),
|
||||
@@ -605,7 +611,7 @@ def test_fresh_process_human_warning_cardinality_is_one_across_failure_and_write
|
||||
[sys.executable, "-c", probe, "schema", "suggest-fks", "--write", "-c", str(cfg)],
|
||||
check=True, capture_output=True, text=True,
|
||||
)
|
||||
assert json.loads(response.stdout) == {"warnings": 1, "exit": 0}
|
||||
assert json.loads(response.stdout) == {"warnings": 1, "exit": 1}
|
||||
|
||||
physical.unlink()
|
||||
for branch in branches:
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
from __future__ import annotations
|
||||
import os, stat
|
||||
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from tht.workspace_writer_lock import WorkspaceWriterConflict, verify_workspace_writer_fds
|
||||
|
||||
|
||||
def test_verifier_rejects_missing_capability():
|
||||
with pytest.raises(WorkspaceWriterConflict): verify_workspace_writer_fds(env={})
|
||||
|
||||
@@ -14,3 +18,18 @@ def test_verifier_checks_fd_identity(tmp_path):
|
||||
cap=verify_workspace_writer_fds(writer_fd=w, root_fd=r, env=env)
|
||||
assert cap.inode == st.st_ino
|
||||
finally: os.close(w); os.close(r)
|
||||
|
||||
|
||||
def test_verifier_rejects_unrelated_lock(tmp_path):
|
||||
root = tmp_path / "root"; other = tmp_path / "other"
|
||||
root.mkdir(mode=0o700); other.mkdir(mode=0o700)
|
||||
expected = root / "writer.lock"; forged = other / "forged.lock"
|
||||
expected.touch(mode=0o600); forged.touch(mode=0o600)
|
||||
root_fd = os.open(root, os.O_RDONLY); forged_fd = os.open(forged, os.O_RDWR)
|
||||
try:
|
||||
st = os.fstat(root_fd)
|
||||
env = {"THOTH_WORKSPACE_ID": "abc-workspace", "THOTH_WORKSPACE_REVISION": "a" * 40, "THOTH_WORKSPACE_DEVICE": str(st.st_dev), "THOTH_WORKSPACE_INODE": str(st.st_ino)}
|
||||
with pytest.raises(WorkspaceWriterConflict):
|
||||
verify_workspace_writer_fds(writer_fd=forged_fd, root_fd=root_fd, env=env)
|
||||
finally:
|
||||
os.close(forged_fd); os.close(root_fd)
|
||||
|
||||
@@ -20,12 +20,11 @@ from tht.vectorstore.embeddings import EmbeddingsError
|
||||
preprocess_app = typer.Typer(help="Materialize versioned preprocessing artifacts")
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
def _require_writer_capability() -> None:
|
||||
"""Mutating children opt into the backend-owned fd capability contract."""
|
||||
import os
|
||||
if os.environ.get("THOTH_WORKSPACE_CAPABILITY_REQUIRED") == "1":
|
||||
from tht.workspace_writer_lock import require_workspace_writer_capability
|
||||
require_workspace_writer_capability()
|
||||
def _require_writer_capability(*, workspace_id: str | None = None, revision: str | None = None) -> None:
|
||||
# Authorization is unconditional: an environment marker is attacker-controlled
|
||||
# and must never turn a mutating direct invocation into an authorized child.
|
||||
from tht.workspace_writer_lock import require_workspace_writer_capability
|
||||
require_workspace_writer_capability(workspace_id=workspace_id, revision=revision)
|
||||
|
||||
_PREPROCESS_EXPECTED_ERRORS = (
|
||||
OSError, RuntimeError, ValueError, TypeError, KeyError,
|
||||
@@ -36,7 +35,6 @@ _PREPROCESS_EXPECTED_ERRORS = (
|
||||
def run_dwh_from_config(
|
||||
config: Path, *, steps: tuple[str, ...], resume: str | None = None,
|
||||
):
|
||||
_require_writer_capability()
|
||||
from tht.cli.lsh_cmd import build_lsh_artifacts
|
||||
from tht.cli.schema_cmd import _load_config_or_exit, refresh_catalog
|
||||
from tht.jobs.dwh_pipeline import (
|
||||
@@ -45,6 +43,7 @@ def run_dwh_from_config(
|
||||
)
|
||||
|
||||
cfg = _load_config_or_exit(config)
|
||||
_require_writer_capability(workspace_id=getattr(getattr(cfg, "runtime_identity", None), "workspace_id", None), revision=getattr(getattr(cfg, "runtime_identity", None), "workspace_revision", None))
|
||||
binding = config_dwh_binding(cfg)
|
||||
workspace_root = cfg.paths.artifacts.parent
|
||||
lsh_names = (
|
||||
@@ -82,7 +81,6 @@ def _parse_dwh_steps(value: str) -> tuple[str, ...]:
|
||||
|
||||
|
||||
def run_from_config(config: Path, *, dry_run: bool = False, resume: str | None = None):
|
||||
_require_writer_capability()
|
||||
from tht.adapters.factory import build_evidence_sources, build_vector_store
|
||||
from tht.cli.vector_cmd import make_embedder
|
||||
from tht.corpus.chunk import ChunkPolicy
|
||||
@@ -90,6 +88,7 @@ def run_from_config(config: Path, *, dry_run: bool = False, resume: str | None =
|
||||
from tht.corpus.store import CorpusStore
|
||||
|
||||
cfg = _load_config_or_exit(config)
|
||||
_require_writer_capability(workspace_id=getattr(getattr(cfg, "runtime_identity", None), "workspace_id", None), revision=getattr(getattr(cfg, "runtime_identity", None), "workspace_revision", None))
|
||||
if cfg.embeddings is None:
|
||||
raise RuntimeError("embeddings are not configured")
|
||||
corpus_root = cfg.paths.artifacts.parent / "corpus"
|
||||
@@ -123,6 +122,7 @@ def gc_from_config(config: Path, *, dry_run: bool = False):
|
||||
from tht.corpus.store import CorpusStore
|
||||
|
||||
cfg = _load_config_or_exit(config)
|
||||
_require_writer_capability(workspace_id=getattr(getattr(cfg, "runtime_identity", None), "workspace_id", None), revision=getattr(getattr(cfg, "runtime_identity", None), "workspace_revision", None))
|
||||
if cfg.embeddings is None:
|
||||
raise RuntimeError("embeddings are not configured")
|
||||
corpus_root = cfg.paths.artifacts.parent / "corpus"
|
||||
|
||||
@@ -16,11 +16,11 @@ from tht.mschema.eligibility import classify_all
|
||||
schema_app = typer.Typer(help="Gestione mschema (rappresentazione canonica dello schema)")
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
def _require_writer_capability() -> None:
|
||||
import os
|
||||
if os.environ.get("THOTH_WORKSPACE_CAPABILITY_REQUIRED") == "1":
|
||||
from tht.workspace_writer_lock import require_workspace_writer_capability
|
||||
require_workspace_writer_capability()
|
||||
def _require_writer_capability(*, workspace_id: str | None = None, revision: str | None = None) -> None:
|
||||
# Authorization is unconditional: an environment marker is attacker-controlled
|
||||
# and must never turn a mutating direct invocation into an authorized child.
|
||||
from tht.workspace_writer_lock import require_workspace_writer_capability
|
||||
require_workspace_writer_capability(workspace_id=workspace_id, revision=revision)
|
||||
|
||||
|
||||
def _add_examples(dwh, phys, examples) -> None:
|
||||
@@ -569,6 +569,7 @@ def suggest_fks_cmd(
|
||||
for item in payload["candidates"]
|
||||
}
|
||||
if write:
|
||||
_require_writer_capability(workspace_id=getattr(getattr(cfg, "runtime_identity", None), "workspace_id", None), revision=getattr(getattr(cfg, "runtime_identity", None), "workspace_revision", None))
|
||||
for table_name, table_payload in candidate_tables.items():
|
||||
ann = annotations.tables.setdefault(table_name, TableAnnotation())
|
||||
from tht.mschema.models import ForeignKey
|
||||
|
||||
@@ -24,11 +24,11 @@ from tht.vectorstore.store import SyncStats, content_hash
|
||||
vector_app = typer.Typer(help="Indice semantico Qdrant (derivato, rigenerabile)")
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
def _require_writer_capability() -> None:
|
||||
import os
|
||||
if os.environ.get("THOTH_WORKSPACE_CAPABILITY_REQUIRED") == "1":
|
||||
from tht.workspace_writer_lock import require_workspace_writer_capability
|
||||
require_workspace_writer_capability()
|
||||
def _require_writer_capability(*, workspace_id: str | None = None, revision: str | None = None) -> None:
|
||||
# Authorization is unconditional: an environment marker is attacker-controlled
|
||||
# and must never turn a mutating direct invocation into an authorized child.
|
||||
from tht.workspace_writer_lock import require_workspace_writer_capability
|
||||
require_workspace_writer_capability(workspace_id=workspace_id, revision=revision)
|
||||
|
||||
|
||||
def make_embedder(embeddings_cfg):
|
||||
@@ -69,8 +69,8 @@ def open_searcher(cfg):
|
||||
return AdapterSearcher()
|
||||
|
||||
|
||||
def sync_canonical_records(collection, records, *, store, embedder):
|
||||
_require_writer_capability()
|
||||
def sync_canonical_records(collection, records, *, store, embedder, config=None):
|
||||
_require_writer_capability(workspace_id=getattr(getattr(config, "runtime_identity", None), "workspace_id", None), revision=getattr(getattr(config, "runtime_identity", None), "workspace_revision", None))
|
||||
kinds = sorted({record.kind for record in records})
|
||||
existing = store.existing_hashes(collection, kinds)
|
||||
pending = []
|
||||
@@ -236,7 +236,7 @@ def index_schema_data(
|
||||
"schema_records",
|
||||
records,
|
||||
store=build_vector_store(cfg, require_write=True),
|
||||
embedder=make_embedder(cfg.embeddings),
|
||||
embedder=make_embedder(cfg.embeddings), config=cfg,
|
||||
)
|
||||
return {
|
||||
"status": "succeeded",
|
||||
|
||||
@@ -1,22 +1,26 @@
|
||||
"""Capability verifier for mutating workspace children.
|
||||
"""Fail-closed verifier for the backend-owned writer capability.
|
||||
|
||||
The backend passes writer.lock as fd 3 and the retained workspace directory as fd 4.
|
||||
This module intentionally has no path fallback: callers either run with the capability
|
||||
or fail closed before touching artifacts.
|
||||
FD 3 is the inherited writer open file description and FD 4 is the retained
|
||||
workspace-root directory. Environment values are descriptive identity only;
|
||||
they never authorize a direct invocation.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import errno
|
||||
import fcntl
|
||||
import os
|
||||
import re
|
||||
import stat
|
||||
from dataclasses import dataclass
|
||||
|
||||
|
||||
class WorkspaceWriterConflict(RuntimeError):
|
||||
"preprocessing_conflict"
|
||||
|
||||
def __init__(self, message: str = "preprocessing_conflict") -> None:
|
||||
super().__init__(message)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class WorkspaceCapability:
|
||||
workspace_id: str
|
||||
@@ -26,26 +30,72 @@ class WorkspaceCapability:
|
||||
writer_device: int
|
||||
writer_inode: int
|
||||
|
||||
|
||||
def _identity(env: dict[str, str]) -> tuple[str, str, int, int]:
|
||||
wid, rev = env.get("THOTH_WORKSPACE_ID"), env.get("THOTH_WORKSPACE_REVISION")
|
||||
if not wid or not rev or not __import__("re").fullmatch(r"[a-z][a-z0-9-]{2,62}", wid) or not __import__("re").fullmatch(r"[0-9a-f]{40}", rev):
|
||||
if not wid or not rev or not re.fullmatch(r"[a-z][a-z0-9-]{2,62}", wid) or not re.fullmatch(r"[0-9a-f]{40}", rev):
|
||||
raise WorkspaceWriterConflict()
|
||||
try:
|
||||
device, inode = int(env["THOTH_WORKSPACE_DEVICE"]), int(env["THOTH_WORKSPACE_INODE"])
|
||||
except (KeyError, ValueError):
|
||||
raise WorkspaceWriterConflict() from None
|
||||
if device < 0 or inode <= 0:
|
||||
raise WorkspaceWriterConflict()
|
||||
try: device, inode = int(env["THOTH_WORKSPACE_DEVICE"]), int(env["THOTH_WORKSPACE_INODE"])
|
||||
except (KeyError, ValueError): raise WorkspaceWriterConflict()
|
||||
return wid, rev, device, inode
|
||||
|
||||
|
||||
def _fstat(fd: int) -> os.stat_result:
|
||||
try:
|
||||
return os.fstat(fd)
|
||||
except OSError:
|
||||
raise WorkspaceWriterConflict() from None
|
||||
|
||||
|
||||
def _open_lock(root_fd: int) -> int:
|
||||
# The lock is opened relative to the retained root and cannot be substituted
|
||||
# by a symlink between validation and open. No path fallback is permitted.
|
||||
try:
|
||||
return os.open("writer.lock", os.O_RDWR | os.O_NOFOLLOW | os.O_CLOEXEC, dir_fd=root_fd)
|
||||
except OSError:
|
||||
raise WorkspaceWriterConflict() from None
|
||||
|
||||
|
||||
def verify_workspace_writer_fds(*, writer_fd: int = 3, root_fd: int = 4, env: dict[str, str] | None = None) -> WorkspaceCapability:
|
||||
env = dict(os.environ if env is None else env)
|
||||
wid, rev, device, inode = _identity(env)
|
||||
try: root = os.fstat(root_fd); writer = os.fstat(writer_fd)
|
||||
except OSError as exc: raise WorkspaceWriterConflict() from exc
|
||||
if not stat.S_ISDIR(root.st_mode) or root.st_uid != os.getuid() or (root.st_mode & 0o777) != 0o700 or (root.st_dev, root.st_ino) != (device, inode): raise WorkspaceWriterConflict()
|
||||
if not stat.S_ISREG(writer.st_mode) or writer.st_uid != os.getuid() or (writer.st_mode & 0o777) != 0o600: raise WorkspaceWriterConflict()
|
||||
try: fcntl.flock(writer_fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
except OSError as exc: raise WorkspaceWriterConflict() from exc
|
||||
# Keep the OFD locked. A lock check is necessarily best effort on some BSDs; identity and
|
||||
# descriptor ownership remain mandatory and no path-based lock is accepted.
|
||||
if writer_fd == root_fd or writer_fd < 0 or root_fd < 0:
|
||||
raise WorkspaceWriterConflict()
|
||||
root, writer = _fstat(root_fd), _fstat(writer_fd)
|
||||
uid = os.getuid()
|
||||
if not stat.S_ISDIR(root.st_mode) or root.st_uid != uid or (root.st_mode & 0o777) != 0o700 or (root.st_dev, root.st_ino) != (device, inode):
|
||||
raise WorkspaceWriterConflict()
|
||||
if not stat.S_ISREG(writer.st_mode) or writer.st_uid != uid or (writer.st_mode & 0o777) != 0o600 or writer.st_nlink != 1:
|
||||
raise WorkspaceWriterConflict()
|
||||
lock_fd = _open_lock(root_fd)
|
||||
try:
|
||||
lock = _fstat(lock_fd)
|
||||
if (lock.st_dev, lock.st_ino) != (writer.st_dev, writer.st_ino) or not stat.S_ISREG(lock.st_mode) or lock.st_uid != uid or (lock.st_mode & 0o777) != 0o600 or lock.st_nlink != 1:
|
||||
raise WorkspaceWriterConflict()
|
||||
# A duplicate of the locked open description is re-lockable. An
|
||||
# independently-opened description receives EWOULDBLOCK.
|
||||
try:
|
||||
fcntl.flock(writer_fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||
except OSError as exc:
|
||||
if exc.errno in (errno.EACCES, errno.EAGAIN, errno.EWOULDBLOCK):
|
||||
raise WorkspaceWriterConflict() from None
|
||||
raise WorkspaceWriterConflict() from exc
|
||||
finally:
|
||||
try:
|
||||
os.close(lock_fd)
|
||||
except OSError:
|
||||
pass
|
||||
return WorkspaceCapability(wid, rev, root.st_dev, root.st_ino, writer.st_dev, writer.st_ino)
|
||||
|
||||
def require_workspace_writer_capability() -> WorkspaceCapability:
|
||||
return verify_workspace_writer_fds()
|
||||
|
||||
def require_workspace_writer_capability(*, workspace_id: str | None = None, revision: str | None = None) -> WorkspaceCapability:
|
||||
cap = verify_workspace_writer_fds()
|
||||
if workspace_id is not None and cap.workspace_id != workspace_id:
|
||||
raise WorkspaceWriterConflict()
|
||||
if revision is not None and cap.revision != revision:
|
||||
raise WorkspaceWriterConflict()
|
||||
return cap
|
||||
|
||||
Reference in New Issue
Block a user