Skip to content
Open
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
48 changes: 33 additions & 15 deletions src/datajoint/provenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,13 @@ def assert_read_allowed(query_expression) -> None:
)


def assert_write_allowed(target_table) -> None:
def assert_write_allowed(target_table, verb: str = "insert into") -> None:
"""
Verify the *target* of an insert is allowed under the active strict-make context.
Verify the *target* of a write is allowed under the active strict-make context.

Called from ``Table.insert`` after the existing ``_allow_insert`` check and
before any rows are materialized. No-op when no strict-make context is active.
Called from ``Table.insert`` (after the existing ``_allow_insert`` check and
before any rows are materialized) and from ``Table.update1`` (before the
UPDATE is issued). No-op when no strict-make context is active.

Allowed targets:

Expand All @@ -140,6 +141,14 @@ def assert_write_allowed(target_table) -> None:
as rows are materialized, so this gate never consumes the caller's ``rows``
iterable — a one-shot generator must survive to reach ``insert``.

Parameters
----------
target_table : Table
The table being written to.
verb : str
Phrase naming the write operation in the error message
(``"insert into"`` or ``"update1 on"``).

Raises ``DataJointError`` if the target is not permitted.
"""
ctx = _active_strict_make.get()
Expand Down Expand Up @@ -168,21 +177,29 @@ def assert_write_allowed(target_table) -> None:

if target_name not in target_set:
raise DataJointError(
f"strict_provenance=True: insert into {target_name!r} is not permitted "
f"strict_provenance=True: {verb} {target_name!r} is not permitted "
f"inside make() for {make_target.full_table_name!r}. Only the target "
f"table and its Part tables may be written."
)


def assert_row_key_allowed(row) -> None:
def assert_row_key_allowed(row, action: str = "insert") -> None:
"""
Verify a single insert row's key columns match the active ``make()`` key.
Verify a single written row's key columns match the active ``make()`` key.

Called per row from ``Table._insert_rows`` as rows are materialized, so the
Called per row from ``Table._insert_rows`` as rows are materialized (so the
check sees a concrete row without the write gate having to consume the
caller's ``rows`` iterable. No-op when no strict-make context is active or
when ``row`` is not a dict (numpy records / bare sequences carry no field
names to check by — same as the previous behavior).
caller's ``rows`` iterable), and from ``Table.update1`` with the update row.
No-op when no strict-make context is active or when ``row`` is not a dict
(numpy records / bare sequences carry no field names to check by — same as
the previous behavior).

Parameters
----------
row : dict
The row being written.
action : str
``"insert"`` or ``"update"`` — selects the error-message wording.

Raises ``DataJointError`` on a mismatch.
"""
Expand All @@ -192,15 +209,16 @@ def assert_row_key_allowed(row) -> None:
if not isinstance(row, dict):
return
_make_target, _allowed_tables, key = ctx
_check_row_key(row, key)
_check_row_key(row, key, action)


def _check_row_key(row: dict, current_key: dict) -> None:
def _check_row_key(row: dict, current_key: dict, action: str = "insert") -> None:
"""Raise if any row attribute overlapping with the current key has a different value."""
past, plural = {"insert": ("inserted", "Inserts"), "update": ("updated", "Updates")}[action]
for k, v in current_key.items():
if k in row and row[k] != v:
raise DataJointError(
f"strict_provenance=True: inserted row's {k!r}={row[k]!r} does not "
f"match the current make() key's {k!r}={v!r}. Inserts must be "
f"strict_provenance=True: {past} row's {k!r}={row[k]!r} does not "
f"match the current make() key's {k!r}={v!r}. {plural} must be "
f"consistent with the key being populated."
)
12 changes: 12 additions & 0 deletions src/datajoint/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,18 @@ def update1(self, row):
raise DataJointError("Attribute `%s` not found." % next(k for k in row if k not in self.heading.names))
except StopIteration:
pass # ok

# Strict-provenance write gate. No-op outside make() or when the config
# flag is off. Same boundary as insert: the target must be the make()
# target or one of its Parts, and the row's key columns must match the
# current key. Placed before the existence check so a provenance
# violation raises as such rather than as "no matching entry".
# See src/datajoint/provenance.py.
from .provenance import assert_row_key_allowed, assert_write_allowed

assert_write_allowed(self, verb="update1 on")
assert_row_key_allowed(row, action="update")

if len(self.restriction):
raise DataJointError("Update cannot be applied to a restricted table.")
key = {k: row[k] for k in self.primary_key}
Expand Down
151 changes: 151 additions & 0 deletions tests/integration/test_strict_provenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,154 @@ def make(self, key):
# No strict_mode fixture — default-off
DerivedLegacy.populate()
assert (DerivedLegacy & {"subject_id": 1}).fetch1("val") == 0


def test_strict_blocks_update1_on_other_table(prefix, connection_test, strict_mode):
"""update1 is a write: under strict mode, updating a table that is not
self or one of its Parts raises, and the target row is left unmodified.
Regression for the ungated-update1 hole found in the 2.3 post-release audit."""
schema = dj.Schema(f"{prefix}_strict_update1_other", connection=connection_test)

@schema
class Subject(dj.Lookup):
definition = """
subject_id : int32
"""
contents = [(1,)]

@schema
class SideResult(dj.Manual):
definition = """
side_id : int32
---
val : int32
"""

SideResult.insert1({"side_id": 1, "val": 100})

captured: list[Exception] = []

@schema
class Derived(dj.Computed):
definition = """
-> Subject
---
val : int32
"""

def make(self, key):
try:
SideResult.update1({"side_id": 1, "val": 999})
except DataJointError as e:
captured.append(e)
self.insert1({**key, "val": 1})

Derived.populate()
assert len(captured) == 1
assert "update1 on" in str(captured[0])
assert "not permitted" in str(captured[0])
# The side table must be untouched.
assert (SideResult & {"side_id": 1}).fetch1("val") == 100


def test_strict_blocks_update1_with_mismatched_key(prefix, connection_test, strict_mode):
"""update1 on self with key columns that disagree with the current make()
key raises the key-consistency error (before any existence check)."""
schema = dj.Schema(f"{prefix}_strict_update1_key", connection=connection_test)

@schema
class Subject(dj.Lookup):
definition = """
subject_id : int32
"""
contents = [(1,)]

captured: list[Exception] = []

@schema
class Derived(dj.Computed):
definition = """
-> Subject
---
val : int32
"""

def make(self, key):
self.insert1({**key, "val": 1})
try:
# bogus key — must be rejected by the provenance key check,
# not by the "one existing entry" check
self.update1({"subject_id": 999, "val": 2})
except DataJointError as e:
captured.append(e)

Derived.populate()
assert len(captured) == 1
assert "updated row's" in str(captured[0])
assert "does not match the current make() key" in str(captured[0])


def test_strict_update1_on_self_with_matching_key_allowed(prefix, connection_test, strict_mode):
"""update1 on self with a key-consistent row is permitted under strict mode
(corrective update within the provenance boundary)."""
schema = dj.Schema(f"{prefix}_strict_update1_self", connection=connection_test)

@schema
class Subject(dj.Lookup):
definition = """
subject_id : int32
"""
contents = [(1,)]

@schema
class Derived(dj.Computed):
definition = """
-> Subject
---
val : int32
"""

def make(self, key):
self.insert1({**key, "val": 1})
self.update1({**key, "val": 2})

Derived.populate()
assert (Derived & {"subject_id": 1}).fetch1("val") == 2


def test_update1_unchanged_without_strict(prefix, connection_test):
"""With strict_provenance off (default), update1 from inside make() behaves
as before — no gate fires."""
schema = dj.Schema(f"{prefix}_update1_default_off", connection=connection_test)

@schema
class Subject(dj.Lookup):
definition = """
subject_id : int32
"""
contents = [(1,)]

@schema
class SideResult(dj.Manual):
definition = """
side_id : int32
---
val : int32
"""

SideResult.insert1({"side_id": 1, "val": 100})

@schema
class DerivedLegacy(dj.Computed):
definition = """
-> Subject
---
val : int32
"""

def make(self, key):
SideResult.update1({"side_id": 1, "val": 200})
self.insert1({**key, "val": 0})

DerivedLegacy.populate()
assert (SideResult & {"side_id": 1}).fetch1("val") == 200
Loading