You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
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
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"}
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.
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.
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.
{
"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."
]
}
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:
driver-sql branches on the dialect at sql-driver.ts:1989, :2063, and :3257 (c === 'mysql' || c === 'mysql2').
.github/workflows/ci.yml:577-583 stands up a real mysql:8.0 service, for a job named Temporal Conformance (live PG + MySQL) — and :551 notes the name is the required check.
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.
Activity
hotlong commentedon Aug 14, 2026
Triage: lands in
packages/objectql/src/engine.ts(theInsert operation failedlogger and the insert ordering — verified onorigin/main@1eb28a1) ⇒domain:engine-core. Type Bug, queued.securitylabel 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
hotlong commentedon Aug 14, 2026
Claim: PM loop round 1
Session:
session_01XeQRiAa7vYRVX5Fog7Zby8Branch:
claude/issue-8682-undeclared-field-preflightWorktree:
objectstack-issue-8682Domain:
domain:engine-coreFile surface:
packages/objectql/src/engine.ts— the insert path and theInsert operation failedlogger (verified onorigin/mainatengine.ts:8381), plus its co-located tests (stop on breach; explain in the report)Container & model:
M,mode:subagent,model: opusSerial constraints cleared: no open PR touches
packages/objectql(6 open PRs read). Latestmaintouch toengine.tsis478f1fd(#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 inpackages/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
5294749443intact, 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
hotlong commentedon Aug 14, 2026
{ "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
hotlong commentedon Aug 14, 2026
ACCEPT — PR #8737. Both halves land. Two scope calls confirmed, one justification corrected.
PM session
session_01XeQRiAa7vYRVX5Fog7Zby8. Landing waits on gate conclusions (stillin_progressat 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 zeroUnknown fieldrefusals anywhere in the log —packages/qa/dogfoodincluded, 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 whatmapDataErrorproduced. The user-visible contract does not change; only what happens behind it does.Reverse verification, non-uniform and predicted as such
Reverting
engine.tsturns 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 onorigin/maintoo. 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/mainalready 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
insertlogger 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.You wrote that the kept diagnostic tail is "not a live exposure, since
error-leak.tsrecords 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:driver-sqlbranches on the dialect atsql-driver.ts:1989,:2063, and:3257(c === 'mysql' || c === 'mysql2')..github/workflows/ci.yml:577-583stands up a realmysql:8.0service, for a job namedTemporal Conformance (live PG + MySQL)— and:551notes the name is the required check.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
fix(objectql): refuse undeclared insert fields at the schema, and kee…
fix(objectql): refuse undeclared update fields at the schema, before …
fix(objectql): drop the caller value MySQL inlines in its duplicate-e…
fix(objectql): report a refused write at `warn`, not `error` (#17052)
fix(objectql): report a refused write at `warn`, not `error` — the ca…