Skip to content

finding: the flow and hook readonly rules still carry the superseded "INSERT is engine-exempt" premise as a scan gap — a non-system create_record / ctx.api.insert of a readonly column is now a silent no-op nothing reports at build time #15394

Description

@claude

Found while implementing #14147 (the create-side readonly strip moving into engine.insert); filed unassigned and out of that PR's scope, because closing it introduces a new error-severity build finding and that is its own change.

What changed under these two rules

packages/lint/src/validate-readonly-flow-writes.ts and packages/lint/src/validate-readonly-hook-writes.ts both scan only update / updateById, and both justify the omission with the same sentence — that INSERT is engine-exempt from the author-declared static-readonly strip, so a create writing a readonly field is not a silent no-op:

//   - Only `update_record`. INSERT is engine-exempt from the readonly strip (a
//     `create_record` may legitimately seed readonly columns; the ingress strip
//     added in #3043 lives in metadata-protocol, which the flow engine bypasses
//     by calling the data engine directly), so a create writing a readonly
//     field is NOT a no-op and is never flagged.

The maintainer ruling of 2026-09-03 (option C, comment 5522734749, overturning their own 2026-07-24 "INSERT (all callers) exempt" row) made that premise false: engine.insert runs the same isSystem-gated stripReadonlyFields the update path runs. The prose in both files has been corrected in #14147's PR; the SCAN GAP has not, because widening it is a behaviour change for every consumer's build.

Why each one is now a real finding

Both surfaces run NON-elevated by default, which is what separates them from the action sibling (an action body is { ...ec, isSystem: true }, so its create genuinely still lands):

Suggested shape

Extend each rule's subject set to the create verb, reporting ONLY the static readonly shape there (readonlyWhen has no create-side strip — engine.ts still says "INSERT stays exempt", so a conditional finding on a create would be false), and keep the existing runAs: 'system' / elevation exemption gating the static branch exactly as it does on update.

⚠️ Whoever takes this should measure the blast radius on the shipped examples first: this adds an error-severity os validate / os build finding, so any in-repo flow or hook that seeds a readonly column on create without elevation reds the build the day it lands. That is the finding working as designed, but it is a landing plan, not a side effect.

Not this

packages/lint/src/validate-readonly-action-writes.ts is already handled in #14147's PR and needs nothing here: an action body is system-elevated by design, so a static-readonly finding on that surface would state something false about a write that lands. Its silence is now recorded as a reasoned refusal (READONLY_ACTION_INSERT_SILENCE) with a pin that the overturned sentence cannot come back as the justification.


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions