Skip to content

Commit f8d50b0

Browse files
fix(strict_provenance): gate update1 inside make() (target + key consistency)
The 2.3 post-release audit found update1 was a fully ungated write path under strict_provenance: inside make(), SideTable.update1({...}) silently mutated any table past the provenance boundary (empirically demonstrated) while the same write via insert is blocked. The asymmetry was unintended: the boundary is 'write only to self (and Parts)', and update1 is a write. update1 now runs the same two checks as insert, before the existence check so a provenance violation raises as such: - assert_write_allowed(self, verb='update1 on') — target must be the make() target or one of its Parts; blocked-update message reads 'update1 on <t> is not permitted inside make() for <target>...'. - assert_row_key_allowed(row, action='update') — key columns overlapping the current make() key must match; mismatch message reads 'updated row's ... Updates must be consistent with the key being populated.' Both gate functions gain wording parameters; the insert-path messages are byte-identical to before. No-op outside make() or with the flag off — update1 behavior outside strict mode is unchanged (pinned by a new test). Tests: blocked update1 on another table (value verified untouched), blocked key-mismatched update1 on self, allowed key-consistent update1 on self, and default-off parity. Full strict suite 12/12; test_update1 4/4. Companion docs update (write-enforcement table + limits) rides in the datajoint-docs post-audit reconciliation PR.
1 parent b50840c commit f8d50b0

3 files changed

Lines changed: 196 additions & 15 deletions

File tree

src/datajoint/provenance.py

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,13 @@ def assert_read_allowed(query_expression) -> None:
125125
)
126126

127127

128-
def assert_write_allowed(target_table) -> None:
128+
def assert_write_allowed(target_table, verb: str = "insert into") -> None:
129129
"""
130-
Verify the *target* of an insert is allowed under the active strict-make context.
130+
Verify the *target* of a write is allowed under the active strict-make context.
131131
132-
Called from ``Table.insert`` after the existing ``_allow_insert`` check and
133-
before any rows are materialized. No-op when no strict-make context is active.
132+
Called from ``Table.insert`` (after the existing ``_allow_insert`` check and
133+
before any rows are materialized) and from ``Table.update1`` (before the
134+
UPDATE is issued). No-op when no strict-make context is active.
134135
135136
Allowed targets:
136137
@@ -140,6 +141,14 @@ def assert_write_allowed(target_table) -> None:
140141
as rows are materialized, so this gate never consumes the caller's ``rows``
141142
iterable — a one-shot generator must survive to reach ``insert``.
142143
144+
Parameters
145+
----------
146+
target_table : Table
147+
The table being written to.
148+
verb : str
149+
Phrase naming the write operation in the error message
150+
(``"insert into"`` or ``"update1 on"``).
151+
143152
Raises ``DataJointError`` if the target is not permitted.
144153
"""
145154
ctx = _active_strict_make.get()
@@ -168,21 +177,29 @@ def assert_write_allowed(target_table) -> None:
168177

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

176185

177-
def assert_row_key_allowed(row) -> None:
186+
def assert_row_key_allowed(row, action: str = "insert") -> None:
178187
"""
179-
Verify a single insert row's key columns match the active ``make()`` key.
188+
Verify a single written row's key columns match the active ``make()`` key.
180189
181-
Called per row from ``Table._insert_rows`` as rows are materialized, so the
190+
Called per row from ``Table._insert_rows`` as rows are materialized (so the
182191
check sees a concrete row without the write gate having to consume the
183-
caller's ``rows`` iterable. No-op when no strict-make context is active or
184-
when ``row`` is not a dict (numpy records / bare sequences carry no field
185-
names to check by — same as the previous behavior).
192+
caller's ``rows`` iterable), and from ``Table.update1`` with the update row.
193+
No-op when no strict-make context is active or when ``row`` is not a dict
194+
(numpy records / bare sequences carry no field names to check by — same as
195+
the previous behavior).
196+
197+
Parameters
198+
----------
199+
row : dict
200+
The row being written.
201+
action : str
202+
``"insert"`` or ``"update"`` — selects the error-message wording.
186203
187204
Raises ``DataJointError`` on a mismatch.
188205
"""
@@ -192,15 +209,16 @@ def assert_row_key_allowed(row) -> None:
192209
if not isinstance(row, dict):
193210
return
194211
_make_target, _allowed_tables, key = ctx
195-
_check_row_key(row, key)
212+
_check_row_key(row, key, action)
196213

197214

198-
def _check_row_key(row: dict, current_key: dict) -> None:
215+
def _check_row_key(row: dict, current_key: dict, action: str = "insert") -> None:
199216
"""Raise if any row attribute overlapping with the current key has a different value."""
217+
past, plural = {"insert": ("inserted", "Inserts"), "update": ("updated", "Updates")}[action]
200218
for k, v in current_key.items():
201219
if k in row and row[k] != v:
202220
raise DataJointError(
203-
f"strict_provenance=True: inserted row's {k!r}={row[k]!r} does not "
204-
f"match the current make() key's {k!r}={v!r}. Inserts must be "
221+
f"strict_provenance=True: {past} row's {k!r}={row[k]!r} does not "
222+
f"match the current make() key's {k!r}={v!r}. {plural} must be "
205223
f"consistent with the key being populated."
206224
)

src/datajoint/table.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,18 @@ def update1(self, row):
551551
raise DataJointError("Attribute `%s` not found." % next(k for k in row if k not in self.heading.names))
552552
except StopIteration:
553553
pass # ok
554+
555+
# Strict-provenance write gate. No-op outside make() or when the config
556+
# flag is off. Same boundary as insert: the target must be the make()
557+
# target or one of its Parts, and the row's key columns must match the
558+
# current key. Placed before the existence check so a provenance
559+
# violation raises as such rather than as "no matching entry".
560+
# See src/datajoint/provenance.py.
561+
from .provenance import assert_row_key_allowed, assert_write_allowed
562+
563+
assert_write_allowed(self, verb="update1 on")
564+
assert_row_key_allowed(row, action="update")
565+
554566
if len(self.restriction):
555567
raise DataJointError("Update cannot be applied to a restricted table.")
556568
key = {k: row[k] for k in self.primary_key}

tests/integration/test_strict_provenance.py

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,3 +320,154 @@ def make(self, key):
320320
# No strict_mode fixture — default-off
321321
DerivedLegacy.populate()
322322
assert (DerivedLegacy & {"subject_id": 1}).fetch1("val") == 0
323+
324+
325+
def test_strict_blocks_update1_on_other_table(prefix, connection_test, strict_mode):
326+
"""update1 is a write: under strict mode, updating a table that is not
327+
self or one of its Parts raises, and the target row is left unmodified.
328+
Regression for the ungated-update1 hole found in the 2.3 post-release audit."""
329+
schema = dj.Schema(f"{prefix}_strict_update1_other", connection=connection_test)
330+
331+
@schema
332+
class Subject(dj.Lookup):
333+
definition = """
334+
subject_id : int32
335+
"""
336+
contents = [(1,)]
337+
338+
@schema
339+
class SideResult(dj.Manual):
340+
definition = """
341+
side_id : int32
342+
---
343+
val : int32
344+
"""
345+
346+
SideResult.insert1({"side_id": 1, "val": 100})
347+
348+
captured: list[Exception] = []
349+
350+
@schema
351+
class Derived(dj.Computed):
352+
definition = """
353+
-> Subject
354+
---
355+
val : int32
356+
"""
357+
358+
def make(self, key):
359+
try:
360+
SideResult.update1({"side_id": 1, "val": 999})
361+
except DataJointError as e:
362+
captured.append(e)
363+
self.insert1({**key, "val": 1})
364+
365+
Derived.populate()
366+
assert len(captured) == 1
367+
assert "update1 on" in str(captured[0])
368+
assert "not permitted" in str(captured[0])
369+
# The side table must be untouched.
370+
assert (SideResult & {"side_id": 1}).fetch1("val") == 100
371+
372+
373+
def test_strict_blocks_update1_with_mismatched_key(prefix, connection_test, strict_mode):
374+
"""update1 on self with key columns that disagree with the current make()
375+
key raises the key-consistency error (before any existence check)."""
376+
schema = dj.Schema(f"{prefix}_strict_update1_key", connection=connection_test)
377+
378+
@schema
379+
class Subject(dj.Lookup):
380+
definition = """
381+
subject_id : int32
382+
"""
383+
contents = [(1,)]
384+
385+
captured: list[Exception] = []
386+
387+
@schema
388+
class Derived(dj.Computed):
389+
definition = """
390+
-> Subject
391+
---
392+
val : int32
393+
"""
394+
395+
def make(self, key):
396+
self.insert1({**key, "val": 1})
397+
try:
398+
# bogus key — must be rejected by the provenance key check,
399+
# not by the "one existing entry" check
400+
self.update1({"subject_id": 999, "val": 2})
401+
except DataJointError as e:
402+
captured.append(e)
403+
404+
Derived.populate()
405+
assert len(captured) == 1
406+
assert "updated row's" in str(captured[0])
407+
assert "does not match the current make() key" in str(captured[0])
408+
409+
410+
def test_strict_update1_on_self_with_matching_key_allowed(prefix, connection_test, strict_mode):
411+
"""update1 on self with a key-consistent row is permitted under strict mode
412+
(corrective update within the provenance boundary)."""
413+
schema = dj.Schema(f"{prefix}_strict_update1_self", connection=connection_test)
414+
415+
@schema
416+
class Subject(dj.Lookup):
417+
definition = """
418+
subject_id : int32
419+
"""
420+
contents = [(1,)]
421+
422+
@schema
423+
class Derived(dj.Computed):
424+
definition = """
425+
-> Subject
426+
---
427+
val : int32
428+
"""
429+
430+
def make(self, key):
431+
self.insert1({**key, "val": 1})
432+
self.update1({**key, "val": 2})
433+
434+
Derived.populate()
435+
assert (Derived & {"subject_id": 1}).fetch1("val") == 2
436+
437+
438+
def test_update1_unchanged_without_strict(prefix, connection_test):
439+
"""With strict_provenance off (default), update1 from inside make() behaves
440+
as before — no gate fires."""
441+
schema = dj.Schema(f"{prefix}_update1_default_off", connection=connection_test)
442+
443+
@schema
444+
class Subject(dj.Lookup):
445+
definition = """
446+
subject_id : int32
447+
"""
448+
contents = [(1,)]
449+
450+
@schema
451+
class SideResult(dj.Manual):
452+
definition = """
453+
side_id : int32
454+
---
455+
val : int32
456+
"""
457+
458+
SideResult.insert1({"side_id": 1, "val": 100})
459+
460+
@schema
461+
class DerivedLegacy(dj.Computed):
462+
definition = """
463+
-> Subject
464+
---
465+
val : int32
466+
"""
467+
468+
def make(self, key):
469+
SideResult.update1({"side_id": 1, "val": 200})
470+
self.insert1({**key, "val": 0})
471+
472+
DerivedLegacy.populate()
473+
assert (SideResult & {"side_id": 1}).fetch1("val") == 200

0 commit comments

Comments
 (0)