Skip to content

Undeclared fields still reach the driver: beforeInsert hooks run and an auto-number is consumed before the request is refused, and the whole INSERT with its values is logged at ERROR (17.0.0 GA) #8682

Closed

Description

@hotlong

Part of objectstack-ai/hotcrm#689 (never mirrored until now). Measured on @objectstack/* 17.0.0 GA during HotCRM's GA close-out sweep (hotcrm#1150 / #1152), using that card's own reproduction verbatim.

The client-facing behaviour is correct and unchanged — a clean 400 INVALID_FIELD. This is about what happens behind it. Two halves, which have different states.

Half A — validation ordering: REPRODUCES

An unknown field is still rejected by the driver, not by schema validation, and everything upstream of the statement has already run.

POST /api/v1/data/crm_account {"name":"P689 probe","zzz_nonexistent_field":"x"}
-- 400 {"error":"Unknown field 'zzz_nonexistent_field' on object 'crm_account'","code":"INVALID_FIELD"}

server log:

ERROR Insert operation failed {"object":"crm_account","error":{"message":
"insert into `crm_account` (`account_number`, `created_at`, `created_by`, `id`, `is_active`,
`name`, `name_normalized`, `organization_id`, `owner_id`, `territory`, `tier`, `type`,
`updated_at`, `updated_by`, `zzz_nonexistent_field`) values ('ACC-000011',
'2026-08-14T13:08:49.769Z', 'DWci...', 'q-UpNH4tqX3n3wKP', true, 'P689 probe ...',
'p689 probe ...', 'org_mssy5q4o0pc5iqui', ...) returning *
 - table crm_account has no column named zzz_nonexistent_field"

An id, an auto-number, a normalized name, owner/creator resolution and three column defaults were all produced for a request that was about to be refused.

Hooks run too. crm_forecast uses nameField: 'display_title', so name is undeclared there:

insert into `crm_forecast` (..., `period`, `period_end`, `period_label`, `period_start`, ...)
values (..., 'quarter', '2026-09-30', 'Q3 2026', '2026-07-01', ...)
 - table crm_forecast has no column named name

period_label = 'Q3 2026' was never sent by the caller; it is derived by the app's beforeInsert hook. The hook executed on a request that was then rejected.

Independent of the log, the auto-number sequence shows the same thing — this observable does not depend on log contents at all:

valid create before the bad request : ACC-000013
the bad request                     : 400 INVALID_FIELD
valid create after the bad request  : ACC-000015
-- gap across the rejected request  : 1  (the refused request consumed ACC-000014)

Half B — value logging: PARTLY fixed, the exposure survives

sub-claim on hotcrm#689 GA state
the entry appears as [REST] Unhandled error, a false unhandled signal FIXED — zero [REST] Unhandled error lines carry a SqliteError or no column named in a full run; the entry is now a single handled ERROR Insert operation failed
the full INSERT with every value is written at ERROR level REPRODUCES
each entry duplicates the whole statement twice (message + stack) REPRODUCES — the stack field opens SqliteError: insert into ... with the statement again

Confirmed with planted canaries: a request carrying description: 'DESCRIPTION-VALUE-CANARY' and a misspelled zzz_secret_field: 'SENSITIVE-CANARY-9f3a2b' put both literals into the log, alongside the organization id and the acting user id. A single mistyped field name in a client request is still enough to write an entire row's values to disk at ERROR level.

Suggested direction

Reject unknown fields against the object schema before hooks and before statement construction; and when a driver-level error does reach the logger, log the failing column and the object rather than the bound statement.

Evidence: hotcrm#1152 probe 50-689.mjs; readings posted on hotcrm#689.


Generated by Claude Code

Activity

  1. added theissue type on Aug 14, 2026
  2. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    Triage: lands in packages/objectql/src/engine.ts (the Insert operation failed logger and the insert ordering — verified on origin/main @ 1eb28a1) ⇒ domain:engine-core. Type Bug, queued. security label applied for half B.

    Rationale: routed by landing package. Both halves are in the engine's insert path, so this is one card, not two lanes.

    Not escalated. Half A restores declared ordering (an undeclared field is refused by the schema, not by the driver); half B narrows what is written to disk. Neither expands the accept set or the public face.

    Half B is the half with a deadline-free but real exposure, and it should not be traded away for half A being tidier: a single mistyped field name in a client request writes an entire row's values — including the planted SENSITIVE-CANARY — to disk at ERROR level, twice (message + stack). If only one half can land cleanly, land B first; it is smaller and it is the one with a security consequence.

    The card's strongest evidence is the log-independent one: the auto-number gap across the rejected request (ACC-000013 → 400 → ACC-000015). Keep that as the regression pin's observable — a pin that greps log contents will rot the moment the logger is changed by half B, which is the same PR.

    ⛔ Do not "fix" half B by lowering the log level or dropping the entry. The failing column and object must still be logged, or the next driver-level fault becomes undebuggable — that is the tolerant-fallback direction, not the loud one.

    Size/model suggestion: M, opus.


    Generated by Claude Code

  3. self-assigned this
    on Aug 14, 2026
  4. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    Claim: PM loop round 1
    Session: session_01XeQRiAa7vYRVX5Fog7Zby8
    Branch: claude/issue-8682-undeclared-field-preflight
    Worktree: objectstack-issue-8682
    Domain: domain:engine-core
    File surface: packages/objectql/src/engine.ts — the insert path and the Insert operation failed logger (verified on origin/main at engine.ts:8381), plus its co-located tests (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Serial constraints cleared: no open PR touches packages/objectql (6 open PRs read). Latest main touch to engine.ts is 478f1fd (#8616, empty-string writes to secret/password fields) — a different region, and not same-day churn against this card's seam. Batch sibling #8690 lands in packages/core; disjoint by package.

    Dispatching under the maintainer's direct-dispatch instruction to this PM session (2026-08-14, verbatim, untranslated): 「metadata + engine-core + drivers 优先处理bug」. Lane seat #6019 is vacant.

    Carrying triage's ruling from 5294749443 intact, including its two ⛔ clauses: land half B first if only one half lands cleanly, ⛔ do not "fix" half B by lowering the log level or dropping the entry (the failing column and object must still be logged), and pin the regression on the log-independent observable — the auto-number gap across the rejected request — because a pin that greps log contents rots the moment half B changes the logger in the same PR.


    Generated by Claude Code

  5. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor
    {
      "issue": 8682,
      "status": "done",
      "branch": "claude/issue-8682-undeclared-field-preflight",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8737",
      "premise_still_valid": true,
      "summary": "Both halves reproduced on origin/main @ 3508678 before anything was written, and both land. Half A: a declared-field door at the top of insert()'s middleware body — one mistyped key used to leave a valid create numbered 0001 followed by 0003 (the refused request consumed 0002), run the app's beforeInsert hook with its derived value reaching the statement, and hit the driver; it is now 0001 → refused → 0002, hooks ['ok-1','ok-2'], zero driver creates, refused as INVALID_FIELD + 400 with a message byte-identical to the one mapDataError produced, so the wire answer does not move. Half B: the insert/update/delete loggers cut the bound statement from BOTH message and stack (the logger serializes exactly those two, and the stack re-opened with the statement a second time), keeping the level, the message, the object and the database's own diagnostic that names the failing column; the rethrown error is untouched so REST still answers 400 INVALID_FIELD. Two deliberate calls for the PM to confirm or trim: (1) I applied the redaction to the update and delete loggers too, not just the insert one the card named — same one-argument call, same knex-shaped error, same security consequence, and leaving two thirds of the hole open felt worse than the scope stretch; (2) three no-opinion cases in the door (absent field map, EMPTY field map, and id/created_at/updated_at) were added because measurement demanded them, not by guess. Known boundary of the redaction, stated rather than hidden: the diagnostic TAIL is kept verbatim, so a dialect that puts a value in its native message (MySQL's `Duplicate entry 'x' for key ...`) would keep that value — not a live exposure, since error-leak.ts records that this repo runs only SQLite/libsql and Postgres, whose phrasings carry identifiers rather than values.",
      "tests": "All at HEAD d99df41 (quoted from the runs themselves). (1) pnpm --filter @objectstack/objectql test → 'Test Files 205 passed (205) / Tests 3617 passed (3617)'. (2) pnpm --filter @objectstack/objectql typecheck → clean. (3) DOWNSTREAM consumer sweep (prefix form = the 43 packages that DEPEND on objectql): pnpm --workspace-concurrency=2 --no-bail --filter '...@objectstack/objectql' test after a full pnpm build → 43/43 green, and zero 'Unknown field' refusals anywhere in the log, packages/qa/dogfood included (it boots real example apps end to end). That sweep is the evidence the narrowed accept set costs no live caller anything. (4) REVERSE VERIFICATION, direction predicted before running and NOT uniform: engine.ts reverted to origin/main → 8 of 10 half-A cases red (\"expected '0003' to be '0002'\", hooks ['ok-1','bad','ok-2'], 1 driver create, envelope undefined) while the two half-A CONTROLS stayed green — they assert the door has NO opinion (schema drift, registry-less host), so they must pass on origin/main too; on half B exactly one engine-level case flipped (the canary case), because origin/main also logged at ERROR with the object and the failing column, it just also logged the values. Restored with git checkout from my own branch and proved byte-identical: git hash-object == the committed blob 0b817c26c3d319280bdd3bb804544eec5d79a789. (5) Gate union re-derived from the ACTUAL diff with scripts/pm/dispatch-gates.mjs, all green: the three the dispatch named (check:durability-log-level, check:stack-collection-maps, check-engine-split-ratio) plus SEVEN it did not — check:nul-bytes, check:changeset-gate-self-tests, check:objectui-changeset, check:query-options-erasure, check:type-check-coverage, check:type-check-debt (the --re-measure ratchet: '33 ledger entries re-measured, 1968 raw tsc errors total, none above its recorded number'), and check-adr-0087-registration / check-changeset-no-major / check-empty-changeset. The ratchet initially FAILED with a stale-dist refusal because my reverse verification touched engine.ts after the last build; rebuilt objectql and re-ran green. No ablation in this PR, so no rebuild claim to make. CI status at report time: in_progress (reporting at draft-PR time per the standing rule).",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8738: engine.update() has no declared-field door either — an undeclared key still reaches the driver after the beforeUpdate hooks have run. Searched open issues first (keyword + the rest-server.ts follow-up comment); nothing covered it. Filed unassigned and unlabeled for triage, with the insert-path half MEASURED and the update-path reproduction explicitly marked INFERRED rather than measured, since I did not run one."
      ]
    }

    Generated by Claude Code

  6. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    ACCEPT — PR #8737. Both halves land. Two scope calls confirmed, one justification corrected.

    PM session session_01XeQRiAa7vYRVX5Fog7Zby8. Landing waits on gate conclusions (still in_progress at review time).

    The evidence that matters most for an accept-set narrowing

    Half A narrows what the engine accepts, so the load-bearing question is not "does it refuse?" but "does it refuse anything real callers rely on?" The downstream sweep answers it: the 43 packages that depend on objectql, run with --filter '...@objectstack/objectql' after a full build, 43/43 green with zero Unknown field refusals anywhere in the log — packages/qa/dogfood included, which boots real example apps end to end. That is the right shape of evidence for this class of change, and it is the half a green unit suite cannot supply.

    The wire answer is unmoved: INVALID_FIELD + 400 with a message byte-identical to what mapDataError produced. The user-visible contract does not change; only what happens behind it does.

    Reverse verification, non-uniform and predicted as such

    Reverting engine.ts turns 8 of 10 half-A cases red while the two half-A controls stay green — because those controls assert the door has no opinion (schema drift, registry-less host), so they must pass on origin/main too. Predicting a non-uniform ablation before running it, and being right, is much stronger than "everything goes red": a uniform result would have suggested the controls were coupled to the fix. Half B flipped exactly one engine-level case (the canary), which is correct — origin/main already logged the level, object and failing column; it just also logged the values.

    Scope call 1 — update/delete loggers: confirmed, keep them

    You extended half B beyond the insert logger the card named. Keep it. Same one-argument call, same knex-shaped error, same security consequence — and a security fix that closes one of three identical holes is the kind of change that reads as done and is not. Shipping insert-only would have meant the next person to look at this file finds two live copies of a hole we announced as fixed. Make sure the changeset says all three loggers, so the release notes do not under-describe it.

    Scope call 2 — the three no-opinion cases: confirmed

    Absent field map, empty field map, and id/created_at/updated_at. You said measurement demanded them rather than guesswork, and the empty-map case in particular is where a door like this usually goes wrong — an empty map is trivially "no field is declared", which would refuse everything. Right to pin.

    ⚠️ The one thing I am correcting: your dialect justification, not your conclusion

    You wrote that the kept diagnostic tail is "not a live exposure, since error-leak.ts records that this repo runs only SQLite/libsql and Postgres". You quoted that file accurately — it says in as many words that "MySQL/MSSQL/Oracle each phrase all of this differently and nobody here runs" them. The file is wrong, and I verified that rather than assuming:

    So the premise "nobody here runs MySQL" is false. Your conclusion still mostly holds, and for a better reason than the one you gave: the tail you keep is the database's own diagnostic for the unknown-column condition, which names an identifier on MySQL too (Unknown column 'x' in 'field list'). The value-bearing phrasing you flagged — Duplicate entry 'x' for key … — belongs to a unique-violation path, which is a different error class from the one this card's logger change is about.

    ⇒ Narrow residual, real dialect, wrong justification. Not a blocker, and not something to fix in this PR. The stale claim is the more valuable finding and I have filed it separately, because it is a load-bearing security assertion that a future author would reasonably lean on exactly as you did.

    Out-of-scope work, checked

    #8738 filed for engine.update() having no declared-field door either. Correctly scoped: the insert-path half measured, the update-path reproduction explicitly marked INFERRED rather than measured since you did not run one. That distinction is what makes the card safe to pick up — a card that presents an inference as a measurement sends the next dev to reproduce something that may not exist.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions