Skip to content

Commit 7811a9f

Browse files
dmealingclaude
andcommitted
fix(python-codegen): PATCH cannot mutate a write-once @autoset column (review gate)
Code-review of the ADR-0045 @autoset stamping (Kotlin clean) found a real integrity hole in the generated Python router: the update handler passed the caller dto straight to repo.update without stripping onCreate @autoset keys, so a caller could overwrite the write-once createdAt via PATCH — a divergence from the Kotlin controller (which excludes @autoset from its patch-settable set) and from the port's own ObjectManager.update (which already does `data.pop(...)`), and one the green api-contract gate doesn't detect (its PATCH body is name-only). Fixes (the caller-facing @autoset surface, mirroring Kotlin): - update handler strips every onCreate @autoset key (`dto.pop(name, None)`) before the repo call — a caller cannot mutate createdAt via the deployed API; - @autoset fields are excluded from _REQUIRED_FIELDS, so a caller-sent present-null on a @required @autoset field is ignored/stamped, not a spurious 400 (Kotlin never null-checks them either). Regression-pinned in test_router_autoset (asserts the strip; render_router callers 26/26). Also two byte-identical simplifier cleanups: Kotlin KotlinTypeMapper.nowExpr uses the bare imported MetaField<*>; the Python awkward conditional-in-spread dissolves by moving the handler comment into _auto_set_stamp_lines (call sites become a plain `*create_autoset`). Deferred (Low, degenerate models): PK / write-through-derived exclusion in _auto_set_split (an @autoset temporal PK or @autoset on a derived field — effectively impossible). Refs #203, #229 (ADR-0045). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeGSV3StPCcJGZNNJ4ZfAb
1 parent c5905b8 commit 7811a9f

3 files changed

Lines changed: 38 additions & 12 deletions

File tree

server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinTypeMapper.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ object KotlinTypeMapper {
663663
* of those `java.time` types exposes a static `now()`. Fully-qualified so no import bookkeeping
664664
* is needed (the four types would otherwise collide on simple names across generated files).
665665
*/
666-
fun nowExpr(field: com.metaobjects.field.MetaField<*>): String {
666+
fun nowExpr(field: MetaField<*>): String {
667667
val tn = kotlinTypeName(field)
668668
val fqn = (tn as? ClassName)?.canonicalName ?: tn.toString()
669669
return "$fqn.now()"

server/python/src/metaobjects/codegen/generators/router_generator.py

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,10 @@ def _required_field_names(entity: MetaObject) -> list[str]:
9696
_scalar_fields, which drops object fields for the sort allowlist). FR-035
9797
PATCH-2 guards present-null on any of these, and a @required jsonb column can
9898
be nulled just like a scalar one."""
99-
return [f.name for f in entity.fields() if is_required(f)]
99+
# #203/ADR-0045: an @autoSet field is server-owned — never a caller-facing PATCH field, so
100+
# it is not present-null-checked (a caller-sent null is ignored/stamped, not a 400). Mirrors
101+
# the Kotlin controller (which excludes @autoSet from its patch-settable set).
102+
return [f.name for f in entity.fields() if is_required(f) and not _is_auto_set(f)]
100103

101104

102105
# --- #203 / ADR-0045: @autoSet stamping in the generated router (above the repo seam) ----------
@@ -108,6 +111,14 @@ def _required_field_names(entity: MetaObject) -> list[str]:
108111
)
109112

110113

114+
def _is_auto_set(field: MetaField) -> bool:
115+
"""True iff *field* is a server-owned ``@autoSet`` column (ADR-0039 resolving read)."""
116+
return field.get_meta_attr(fc.FIELD_ATTR_AUTO_SET) in (
117+
fc.AUTO_SET_ON_CREATE,
118+
fc.AUTO_SET_ON_UPDATE,
119+
)
120+
121+
111122
def _auto_set_split(entity: MetaObject) -> tuple[list[MetaField], list[MetaField]]:
112123
"""The entity's ``@autoSet`` temporal fields split into ``(onCreate, onUpdate)``.
113124
@@ -143,12 +154,13 @@ def _auto_set_stamp_expr(field: MetaField, base_var: str) -> str:
143154
return base_var if is_tz else f"{base_var}.replace(tzinfo=None)"
144155

145156

146-
def _auto_set_stamp_lines(fields: list[MetaField]) -> list[str]:
157+
def _auto_set_stamp_lines(fields: list[MetaField], comment: str) -> list[str]:
147158
"""Router-handler body lines stamping *fields* into ``dto`` from ONE captured base
148-
instant (so all same-op columns are equal). Empty list for no fields."""
159+
instant (so all same-op columns are equal), led by *comment*. Empty for no fields
160+
(so the call site spreads to nothing — a non-@autoSet router stays byte-identical)."""
149161
if not fields:
150162
return []
151-
lines = [" _asnow = _dt.datetime.now(_dt.timezone.utc)"]
163+
lines = [f" # {comment}", " _asnow = _dt.datetime.now(_dt.timezone.utc)"]
152164
lines += [f' dto["{f.name}"] = {_auto_set_stamp_expr(f, "_asnow")}' for f in fields]
153165
return lines
154166

@@ -378,8 +390,7 @@ def _emit_route_handler(
378390
# #203/ADR-0045: the generated router (the API surface) stamps @autoSet columns
379391
# ABOVE the consumer repo seam — onCreate + onUpdate both stamped from one instant
380392
# (a fresh row's createdAt == updatedAt), ignoring any caller-supplied value.
381-
*([" # #203/ADR-0045: stamp @autoSet columns (server-owned; caller ignored)."]
382-
+ list(create_autoset) if create_autoset else []),
393+
*create_autoset,
383394
" return repo.create(dto)",
384395
]
385396
if name == "update":
@@ -406,8 +417,7 @@ def _emit_route_handler(
406417
# #203/ADR-0045: bump onUpdate @autoSet columns (server-owned) on EVERY patch,
407418
# above the repo seam — a server-inserted present key (compatible with the PATCH
408419
# present-key tristate); onCreate columns are never touched here.
409-
*([" # #203/ADR-0045: bump onUpdate @autoSet column(s) (server-owned)."]
410-
+ list(update_autoset) if update_autoset else []),
420+
*update_autoset,
411421
f" saved = repo.update({pk_param}, dto)",
412422
" if saved is None:",
413423
' return JSONResponse(status_code=404, content={"error": "not_found"})',
@@ -755,8 +765,21 @@ def render_router(
755765
# non-@autoSet entity → byte-identical output). insert stamps onCreate+onUpdate from one
756766
# instant; a patch bumps onUpdate only (createdAt stays immutable).
757767
_on_create_auto, _on_update_auto = _auto_set_split(entity)
758-
create_autoset = _auto_set_stamp_lines(_on_create_auto + _on_update_auto)
759-
update_autoset = _auto_set_stamp_lines(_on_update_auto)
768+
create_autoset = _auto_set_stamp_lines(
769+
_on_create_auto + _on_update_auto,
770+
"#203/ADR-0045: stamp @autoSet columns (server-owned; caller ignored).",
771+
)
772+
# #203/ADR-0045 (integrity): onCreate @autoSet columns are write-once — strip any
773+
# caller-supplied value on PATCH before the repo call (a caller cannot mutate createdAt via
774+
# the deployed API). Mirrors ObjectManager.update (`data.pop(...)`) + the Kotlin controller
775+
# (which excludes @autoSet from its patch-settable set).
776+
update_autoset = [
777+
f' dto.pop("{f.name}", None) # onCreate @autoSet is write-once (server-owned)'
778+
for f in _on_create_auto
779+
] + _auto_set_stamp_lines(
780+
_on_update_auto,
781+
"#203/ADR-0045: bump onUpdate @autoSet column(s) (server-owned).",
782+
)
760783
has_autoset = bool(_on_create_auto or _on_update_auto)
761784

762785
parts: list[str] = []

server/python/tests/codegen/test_router_autoset.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ def test_router_update_bumps_only_onupdate() -> None:
7373
update_body = src.split("def update_")[1].split("def delete_")[0]
7474
# the update handler bumps updatedAt; createdAt is never stamped on update (immutable).
7575
assert 'dto["updatedAt"] = _asnow' in update_body
76-
assert 'dto["createdAt"]' not in update_body
76+
assert 'dto["createdAt"] = ' not in update_body
77+
# #203/ADR-0045 integrity: a caller-supplied onCreate @autoSet value is STRIPPED on PATCH
78+
# (write-once — cannot be mutated via the deployed API), so it never reaches repo.update.
79+
assert 'dto.pop("createdAt", None)' in update_body
7780

7881

7982
def test_non_autoset_router_is_byte_identical() -> None:

0 commit comments

Comments
 (0)