Skip to content

fix(objectql): extend the credential-column guard from aggregates to filter and sort keys - #27477

Merged
objectstack-fleet[bot] merged 12 commits into
mainfrom
claude/harden-credential-field-predicates
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 12 commits into
mainfrom
claude/harden-credential-field-predicates

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: #27660

Hardening, ADR-0100 family. No issue is linked: this change was dispatched directly, so there is no closing keyword. BREAKING (an accept-set narrowing), shipped as patch: this PR narrows a query shape that was accepted before, and no metadata an author writes changes. The changeset carries the FROM / TO and one ADR-0087 not-required (no-migration-prescription) marker. It needs one contract-review-tier review before it enters the queue.

维护者速读

  • 改了什么:引擎原本只拒绝对凭据列(password / secret)做分组和 min/max 聚合,现在也拒绝把它们当过滤条件或排序键,统一返回 400 INVALID_FIELD,driver 不会被调用。
  • 为什么改:凭据列读出时一律打码,但过滤和排序会读取存储值,结果可能随这个值变化。这次把已有的聚合守卫补齐到这两个位置。
  • 风险与回滚:系统上下文(isSystem)不受影响,服务端自己的凭据查找照常工作,登录已在 dogfood 中验证。按凭据列过滤或排序的非系统调用会改为收到 400;以凭据列作 matchFields 的 upsert / update 导入,其匹配查找同样被拒,相关行报失败、不写入。回滚只需 revert 本 PR。
  • 先看:packages/objectql/src/credential-evaluation-guard.ts(风险在 isCredentialPresenceCondition 与系统上下文豁免那一行),packages/objectql/src/engine.ts(六个入口的接入点),packages/qa/dogfood/test/encrypted-field-behavior.dogfood.test.ts。
  • 两分钟验证:pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/credential-evaluation-guard.test.ts,预期 30/30 通过。
  • 需要你做的:无,席位落地(待 contract-review-tier 增量复核通过)。

What changes

  • One seam, beside the aggregate guard. rejectCredentialEvaluation (new, packages/objectql/src/credential-evaluation-guard.ts) reads the same exemption-free collector as rejectCredentialAggregation, collectCredentialFields. It answers with the same ADR-0112 envelope: INVALID_FIELD / 400, with field, fields, object and param. It runs on find, findOne, count, aggregate, and predicate update / delete. It runs right after the where array-form lowering and before the search expansion, token resolution, the middleware chain or any driver call.
  • Positions judged: where (every logical branch), a { $field } cross-field comparand, orderBy, each aggregations[i].filter, and each expand entry's own where / orderBy, at every depth, against the target object. A nested-relation condition is judged by the related object's own read, which passes through the same guard.
  • Accepted on a credential column: a has-a-value check only. That is a bare null, $null, $exists, $empty, $eq: null, $ne: null, or $not over one of those. It reveals nothing the masked row does not already show. If keeping this check is a product question, this is the safe default. Every other operator is refused.
  • Who it binds: every caller except a system context. The server's own credential consumers match on such columns as system. The security and sharing middlewares inject their filters after the guard has judged the caller's query.
  • Import match: an import in upsert or update mode whose matchFields names a credential column looks rows up through the same engine read with the import's caller context, so that lookup is now refused too: each row that reaches it is reported failed with the refusal and is not written.
  • Search: the engine's search scan set now withholds credential columns too, as it already withheld internal: true columns. This covers a credential column an object lists in searchableFields.
  • internal: true columns: the filter / sort refusal for these stays where it was decided, at the generic data door (assertNoInternalFieldEvaluated). Some of their server-side consumers do not run as system. The new dogfood case pins that door for one such column, beside the password case.
  • Docs: content/docs/data-modeling/field-types.mdx states the refusal. content/docs/permissions/system-context.mdx gains row 18c and a regenerated register for the guard's one isSystem read.

Tests

  • packages/objectql/src/credential-evaluation-guard.test.ts: a real engine over a recording driver. It covers 13 refused where shapes, orderBy on password and on secret, findOne / count / aggregate / per-aggregation filter / update / delete, an expand entry and a nested-relation condition. Every refusal asserts code, status, httpStatus, field and param, and that the driver was never called. The test also covers eight has-a-value shapes, a system context, non-credential fields and the search scan set.
  • packages/qa/dogfood/test/encrypted-field-behavior.dogfood.test.ts: on the booted showcase REST door, the cases are POST /data/showcase_field_zoo/query with equality, range, $startsWith, $contains, a secret-column predicate and orderBy, plus GET /data/showcase_field_zoo?filter=…. Each answers 400 INVALID_FIELD, and the response never contains the stored value. $null: false and null are served, and so is a control on name. On a scratch object, orderBy and where on a password column and on an internal: true column are refused. The seeded admin's sign-in in each beforeAll covers the login path.
  • Fixture triage: in engine-text-operator-declared-type-door.test.ts and engine-number-comparand-declared-type-door.test.ts, the password / secret cells now run as a system caller. Those doors run first, inside lowerWhereFilterArray; the credential guard runs after them and refuses a non-system caller on the rows they pass. The doors' verdicts do not depend on the caller. federated-injected-column-readers.test.ts gains the guard's referenceTargetOf() row.

Acceptance notes

  • Checklist item records-forms.encrypted-field-behavior: every acceptance clause and negative still holds, and none is contradicted. The new filter / sort refusal sits beside acceptance[4]. Adding a clause for it is checklist-authoring work and is not done here.
  • The Console already treats masked columns as not sortable and not filterable (isMaskedColumnKey in ui/packages/components/src/renderers/complex/data-table.tsx), so no UI path offers the refused action.
  • Every door that reaches the engine with a caller context (REST, the MCP bridges, flows, plugin routes) meets this guard. No door needed its own copy.

Verification

Declaration round, at head 5eb62f0
  • Changes in this round: the changeset declaration (Clause-②: no (narrowing), the BREAKING banner with FROM / TO naming the import match consequence, one ADR-0087 not-required (no-migration-prescription) marker) and the comment in the two declared-type door tests. No behaviour change.
  • pnpm check:adr-0087-registration: exit 0. 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition. [BREAKING+clause-②-narrowing] not-required (no-migration-prescription).
  • node scripts/check-changeset-no-major.mjs --event with this body: exit 0. LEVEL AXIS: this PR declares clause-② no (narrowing) and metadata-protocol none ... a non-protocol change at patch.
  • pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 over the two door tests and credential-evaluation-guard.test.ts: Test Files 3 passed (3), Tests 88 passed (88).
  • pnpm --filter @objectstack/objectql typecheck: exit 0, check:test-typecheck: OK, 39 files / 233 errors / 64 pinned signatures held.
  • Full dogfood battery (pnpm --filter @objectstack/dogfood test, closure rebuilt): Test Files 348 passed, 3 skipped (351); Tests 2808 passed, 20 skipped (2828).
  • Derived gate list (dispatch-gates.mjs --commands): 96 derived, 94 exit 0, 2 NOT MEASURED locally (check:dual-build-cjs-loads and check:type-check-debt need a whole-repo build). --ran: 96 accounted for. Both are in Lint & Repo Gates, which is green on this head.

All readings below were taken at head feb0bdeb57, after origin/main was merged in, the dogfood dependency closure was rebuilt and packages/runtime/.objectstack was cleared.

Unit and REST door
  • pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2: Test Files 485 passed (485), Tests 9123 passed (9123).
  • pnpm --filter @objectstack/objectql typecheck: exit 0. check:test-typecheck: OK, with 39 files / 233 errors / 64 pinned signatures held, unchanged.
  • pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2 test/encrypted-field-behavior.dogfood.test.ts: Tests 6 passed (6).
  • pnpm --filter @objectstack/dogfood typecheck: exit 0.
Ablation (one time each, with restore proven)
  • Source-resolved, through node scripts/ablation-replace.mjs: the guard's entry line was made to always return. The mutation landed (anchor 1 → 0, blob 3d38a49b3d6a → 316fd9e3b630). credential-evaluation-guard.test.ts then showed 18 failed / 12 passed of 30. All 18 refusal cases went red. The 12 has-a-value, system, control and search cases stayed green, as they should. Restore: blob equal to HEAD, git diff HEAD empty.
  • Dist-resolved, which the REST door suite consumes: same mutation with a marker, then an @objectstack/objectql rebuild. ablation-dist-preflight found the marker in 4 built files. The dogfood file showed 2 failed / 4 passed. Both new cases failed on the first refused request, which answered 200 with rows. Restore: rebuilt, --absent check clean, and the whole tree clean against HEAD.
Local gate subset
  • node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 94 commands at feb0bdeb57. All 94 were run, each exit code captured before any pipe: 94 × exit 0. --ran reconciliation: 94 derived, 94 run, 0 NOT-MEASURED, 0 UNRUN.
  • On the first pass, at the pre-merge head, two gates read the new test file. check:objectql-double-limit flagged that the recording double read rows before applying the bound. check:query-options-erasure reported the test surface grew 236 → 241. Both were fixed (bound applied after the filter, options typed), and both are exit 0 above.

The required CI families (Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core and the rest) run on this PR's check-runs. They are not re-run here.

Body refreshed by the PM seat 2026-10-29T16:41Z: Skills-docs follow-up filed as #27660.

🤖 Generated with Claude Code

https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk

claude added 10 commits October 29, 2026 05:57
…filter and sort keys

Claude-Session: https://claude.ai/code/session_017xRBcSwDXZrZg7nGjP6P3U
Co-authored-by: Claude <noreply@anthropic.com>
…system; ledger the guard's reference read

Claude-Session: https://claude.ai/code/session_017xRBcSwDXZrZg7nGjP6P3U
Co-authored-by: Claude <noreply@anthropic.com>
…s at the REST data door

Claude-Session: https://claude.ai/code/session_017xRBcSwDXZrZg7nGjP6P3U
Co-authored-by: Claude <noreply@anthropic.com>
…ding double's find

Claude-Session: https://claude.ai/code/session_017xRBcSwDXZrZg7nGjP6P3U
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 20 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/objectql/src/engine-count-aggregate.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

48 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 57c51aab4d2f63293676e9412daed8a5fc54c1bc.

⛔ 10 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/objectql/src/engine-count-aggregate.ts) — pages documenting those are invisible to this run
  • 1 cross-cutting symbol(s) contributed no route anchor: isSystem (5 routes)
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 78 pages)
  • 8 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 19 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 57c51aab4d2f63293676e9412daed8a5fc54c1bc → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b0a62a912c32f9e97e676d040a907842b4db55aa — the merge of head 5eb62f096eec61828542a2bba3bfa57628843f61 into base 57c51aab4d2f63293676e9412daed8a5fc54c1bc, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b0a62a912c32f9e97e676d040a907842b4db55aa && git checkout b0a62a912c32f9e97e676d040a907842b4db55aa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 57c51aab4d2f63293676e9412daed8a5fc54c1bc 5eb62f096eec61828542a2bba3bfa57628843f61 && git checkout -B drift-repro 57c51aab4d2f63293676e9412daed8a5fc54c1bc && git merge --no-ff 5eb62f096eec61828542a2bba3bfa57628843f61

node scripts/docs-audit/affected-docs.mjs --json 57c51aab4d2f63293676e9412daed8a5fc54c1bc

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 57c51aab4d2f63293676e9412daed8a5fc54c1bc → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 472e55cbb64665dfac1dab6a0b8e2bcfa7a2b9d2
Local-runs: none
Scope: packages/objectql/src/credential-evaluation-guard.ts, packages/objectql/src/engine.ts, packages/objectql/src/engine-count-aggregate.ts, packages/objectql/src/credential-evaluation-guard.test.ts, packages/objectql/src/engine-number-comparand-declared-type-door.test.ts, packages/objectql/src/engine-text-operator-declared-type-door.test.ts, packages/objectql/src/federated-injected-column-readers.test.ts, packages/core/src/security/second-object-read-exposure.pin.test.ts, packages/qa/dogfood/test/encrypted-field-behavior.dogfood.test.ts + packages/objectql/src/secret-fields.ts (collectCredentialFields, collectMaskedReadFields), packages/objectql/src/engine.ts (lowerWhereFilterArray, fillQueryAstDefaults, lowerRelationConditions, expandRelatedRecords, expandSearchOnAst, mergeReadContext), packages/metadata-protocol/src/protocol.ts (findData), packages/metadata-protocol/src/protocol-query-validation.ts (assertNoInternalFieldEvaluated, refuseInternalInFilter, refuseInternalInExpand), packages/plugins/plugin-security/src/predicate-guard.ts, packages/core/src/utils/import-runner.ts (findExisting), packages/spec/src/data/field-value.zod.ts (referenceTargetOf), packages/spec/src/data/filter.zod.ts (FilterConditionSchema, FieldReferenceSchema), packages/drivers/driver-memory/src/memory-driver.ts and packages/drivers/driver-sql/src/sql-driver.ts (array-form refusal at the driver), docs/adr/0100-credential-field-channels.md, scripts/check-adr-0087-registration.mjs, scripts/check-changeset-no-major.mjs, packages/objectql/CHANGELOG.md (the #22646 entry).

Face: narrowing. The diff touches no packages/spec/src/** non-test path and no governed path; the PR body itself states "This PR narrows a query shape that was accepted before" and asks for a contract-review-tier review, and the maintainer dispatched one. The scope floor on this face is every path of the diff outside the prose faces; the changeset and the two content/docs pages are the lane seat's at ACCEPT, read here only for the declaration in ②.

Security-hardening ruling: this record names positions and refusals; it carries no reproduction.

① Derived judgments

The accept set the diff changes, read off the code:

  1. where on a password / secret column, every verb, non-system caller. Previously evaluated; now 400 INVALID_FIELD before the driver. Correct. Order verified in engine.ts: find folds aliases, refuses unknown options, runs lowerWhereFilterArray (array form to object form, the declared-type doors), fills AST defaults, then calls rejectCredentialEvaluation, then expands search, then the middleware chain. findOne, count, aggregate, update, delete call it at the same relative position (after lowerWhereFilterArray, before any token resolution, search expansion, middleware or driver). Every logical branch ($and / $or / $not) and a dotted key's head segment are walked. Matches the claim.
  2. A { $field } comparand naming a credential column. Collected from any comparand position, head segment of the reference. FieldReferenceSchema is a plain string and both the predicate guard and this walker read its first segment, so a cross-object spelling has no reader anywhere; no gap there. Correct.
  3. orderBy on a credential column. For find / findOne the AST's orderBy is already SortNodeSchema-parsed ({ field, order }) when the guard runs; the guard also reads the string spelling and a leading -. The wire-only sort alias is folded by the protocol and refused at a direct engine call. Correct.
  4. aggregations[i].filter. Walked per aggregation, located at aggregations[i].filter. The aggregate half (rejectCredentialAggregation: group-by and operand) is unchanged and still runs after it. Correct.
  5. expand entries' own where / orderBy, every depth, judged against referenceTargetOf(def). Correct, and necessary: expandRelatedRecords re-enters this.find on the target with the caller's context, which would refuse too, but that path catches the sub-read's refusal and retains the bare id, so without the pre-judgment the refusal would be swallowed into a silent drop. A targetless or non-reference field is skipped by both the guard and expandRelatedRecords on the same predicate, so no read happens there. Correct.
  6. Nested-relation condition ({ lookup: { credential: … } }). Deliberately not walked by the guard; lowerRelationConditions reads the related object through this.find with the caller's execution context, which passes through the guard against the related object's own fields; a refusal propagates ("nothing is swallowed"). The unit test pins it. Correct.
  7. Array-form ([field, op, value]) spellings in nested positions (an expand entry's where, a per-aggregation filter, a $and / $or member). The guard reads the object form only, and lowerWhereFilterArray lowers only the top-level where. Judged not a gap: nothing below the top level lowers a tuple (the engine and the protocol call parseFilterAST on the top-level where only; FilterConditionSchema types $and / $or members as records; the memory driver refuses an array that reaches it and reads a $and child's keys, which for a tuple are 0, 1, 2; the SQL driver refuses at its door), so no component evaluates a credential column through that spelling. The walker's shape matches what can execute.
  8. Search scan set. Credential columns are now withheld from the set the search expansion resolves over, including an author-declared searchableFields entry, for every verb that reaches expandSearchOnAst (find / findOne / aggregate / count through expandSearchOnOptions). The guard runs before the expansion, so the expansion's server-authored $or is never judged as the caller's predicate. Correct. See ③-c for what this leaves silently inert.
  9. System-context exemption. The guard returns only on context.isSystem === true. The wire cannot supply it: protocol findData deletes any inbound context unconditionally before setting the server-derived one, and enforceAuth documents that isSystem is never set on inbound HTTP; update / delete read options.context, which the protocol's write doors set from the dispatcher's context. mergeReadContext merges two server-built contexts. Correct. The system-context register row 18c and the regenerated per-symbol table name the one read.
  10. Presence checks (null, $null, $exists, $empty with a boolean operand, $eq: null, $ne: null, $not over any of those). Keeping them is safe, on three readings. (a) Set-or-unset is what every masked read already serves (SECRET_MASK for a set value, null for an unset one, ADR-0100 A and B), so the check reveals nothing beyond the row. (b) A caller withheld the column by field-level security never reaches the row's answer either: plugin-security's assertReadableQueryFields collects a condition's key regardless of operator and refuses 403 on the caller's own AST (it runs after this guard, before RLS injection), unchanged by this PR; so the presence check serves exactly the callers the row already serves. (c) The empty string is unwritable into either type since objectql accepts "" into a secret field: it encrypts the empty string, mints a sys_secret row for it, and every read path then reports "a secret is set" #8559 (EmptyCredentialWriteError), so $empty cannot distinguish a third state. The operand typing is strict (a non-boolean $null, an empty object, a mixed operator set are refused). The default is the safe one, and the product question the dev raised is answered: keep.
  11. collectCredentialFields is exemption-free, so a password-typed column on a managedBy: 'better-auth' object is also refused to a non-system caller although its read is unmasked. Consistent with the aggregate half; ADR-0100 B4 pins that no shipped identity object declares one, and the auth subsystem's own lookups run as system. Accepted as designed.
  12. internal: true columns stay at the generic data door (assertNoInternalFieldEvaluated); the dogfood case pins that door beside the password case. Unchanged, correct.
  13. A consequence the PR body does not name. The import runner's upsert match (findExisting in core/src/utils/import-runner.ts) looks rows up through findData with the import's caller context, which the import doors never elevate; an upsert import whose matchFields names a credential column is therefore now refused inside that lookup. Fail-closed and correct; it belongs in the changeset's FROM / TO (should-fix 2).
  14. Doors. REST POST /data/:object/query and GET /data/:object?filter=… are pinned on the booted showcase; flows, hooks' ctx.api, the MCP bridges and plugin routes reach the same six engine verbs with a caller context. The Console claim holds: isMaskedColumnKey in data-table.tsx already withholds sort and search on masked columns.
  15. Public surface. One new module credential-evaluation-guard.ts; rejectCredentialEvaluation and isCredentialPresenceCondition are module exports but not re-exported from the package entry (no index.ts change in the diff). No published type changes. Census pins updated: the reference-target arbiter census (second-object-read-exposure.pin.test.ts, classified not-a-served-read, the REWORK round-1 fix) and the federated readers ledger (caller-predicate). Both classifications hold against the code: the guard reads declared fields from the registry and no record.

② Semver level

Declared: Clause-②: no, Metadata-protocol: none, @objectstack/objectql: patch. The arm is wrong. A query shape every non-system caller could write and have evaluated on main (a filter or sort key on a credential column) is now refused 400 at six verbs: that is an accept-set narrowing of a served runtime surface, the exact class check-changeset-no-major.mjs describes at its own prompt ("add (narrowing) if it narrows one, a BREAKING change, whatever its level") and the class the PR body itself names in its third sentence. The precedent is the internal: true evaluate door (#22646, packages/objectql/CHANGELOG.md): same positions (filter, sort, per-aggregation filter, nested-relation condition, expand tree, search set), same envelope, shipped as Clause-②: no (narrowing) with a **BREAKING** (an accept-set narrowing) banner and one ADR-0087 marker in the not-required (no-migration-prescription) category. A no without the arm is the auditable false declaration landing-operations names, and it switches off both gates that read the arm (check-adr-0087-registration.mjs signal 4, the no-major gate's grade).

What is right and stays: Metadata-protocol: none (no key, value or metadata type an author writes changes; searchableFields keeps its shape) and the patch level (ruling A: a non-protocol narrowing ships patch and keeps its BREAKING carriers). The Skills-docs line is owed by the arm: skills/objectstack-data/rules/security.md teaches type: 'secret' and says nothing about querying it, and no eval exercises the refusal, so updated here is not claimable from this diff; follow-up owed — skills/objectstack-data/rules/security.md (one sentence: a credential column is never a filter or sort key; only a has-a-value check) or none — REASON is the dev's declaration, read at ACCEPT.

③ Boundary flags

  • (a) Presence checks retained, raised as a product question. Answered in ① item 10: safe, keep. No escalation.
  • (b) internal: true left at the data door "because some server-side consumers do not run as system". Accepted: the two collectors stay independent on purpose, and the dogfood case pins both doors side by side.
  • (c) An author-declared searchableFields naming a credential column is now silently withheld, where the security(data): the data door's filter and group-by positions do not honour a field's internal: true the way its row read does — detail withheld pending maintainer #22646 precedent refuses an explicit $searchFields naming an internal column loudly at the door and ADR-0078 refuses silently inert metadata. Fail-closed on the leak axis, so not a defect of this PR; a spec-side refusal of such an entry (and the door's explicit-searchFields refusal extended to credential types) is a follow-up card (should-fix 1), not a rider here, since it changes what an author can write.
  • (d) Declared-type door tests driven as system for credential cells. The fixture is right (the PASS rows reach the guard and need a system caller), but the comment's ordering claim ("refuses … before this door is reached") is inverted: assertTextOperatorTargetsAreStringCapable and narrowNumberComparands run inside lowerWhereFilterArray, before the guard. Same 400 class either way; wording only (should-fix 3).
  • (e) open_questions: none in the PR body beyond (a). Author is objectstack-fleet[bot]; no login note owed.

④ Design judgment

On the narrowing face, against the published spec on main: the change takes no declared capability away at the consumer and makes the spec's own refusal real. ADR-0100 declares both channels masked on every generic read and names evaluation as the open leak class (its non-goals already called the aggregate position a refusal, shipped as #3171); a position that answers by the stored value contradicts that read-mask promise, and the internal: true door (#22646) settled that such positions are refused, not rewritten. The guard reads the one type-keyed collector the aggregate half reads, so the two halves cannot drift; it sits in the engine (not only at the data door) so every door with a caller context meets it; it answers in the ADR-0112 envelope with param locating the position; and it runs on the caller's own AST, before server-authored filters are injected, which is the ordering contract the predicate guard already states. The accepted shape is the minimal one that keeps the Console's and pickers' "is it set" use working without creating a second rule. Accepted against the scenario (checklist records-forms.encrypted-field-behavior, acceptance[4]) and the code it cites. Ablation evidence in the PR body (source- and dist-resolved, restore proven) is consistent with the test file read here; the record did not re-run it.

Must-fix

  1. Declaration: in the changeset body and the PR body, Clause-②: no (narrowing); keep Metadata-protocol: none and patch; add the **BREAKING** (an accept-set narrowing) banner with a FROM / TO paragraph (the security(data): the data door's filter and group-by positions do not honour a field's internal: true the way its row read does — detail withheld pending maintainer #22646 entry is the template); exactly one ADR-0087 HTML-comment marker in the changeset body, category not-required (no-migration-prescription), stating that no metadata key, spelling, export or stored shape moves and that what narrows is a runtime read door; and a Skills-docs: line in the PR body (② names the admissible values). Then re-run pnpm check:adr-0087-registration and check-changeset-no-major locally.

Should-fix

  1. File a follow-up card: refuse a secret / password entry in searchableFields at the spec door, and extend the data door's explicit-searchFields refusal to credential types, so the declaration is not silently inert (③-c).
  2. Changeset FROM / TO: name the import upsert matchFields consequence (① item 13).
  3. The two declared-type door test comments: state that the doors run first inside lowerWhereFilterArray and the guard answers the rows those doors pass (③-d).

Implemented-by: claude/harden-credential-field-predicates
Reviewed-by: session_017xRBcVy4VXmkQjn2MGeaWk

VERDICT: FAIL

The refusal is complete across every position the PR claims, the system-context exemption is unreachable from the wire, and the presence check is safe to keep; the FAIL is the declaration alone (must-fix 1). A head that carries the corrected declaration and nothing else owes a delta review by this reviewer, not a full round.

The changeset now declares `Clause-②: no (narrowing)` with a BREAKING
banner, a FROM / TO that names the import upsert match consequence, and
one ADR-0087 not-required (no-migration-prescription) marker. The two
declared-type door tests' comments now state the real order: the door
runs first inside lowerWhereFilterArray and the guard answers the rows it
passes. No behaviour change.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk
os-dev-report

{
  "issue": null,
  "status": "done",
  "branch": "claude/harden-credential-field-predicates",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/27477",
  "head": "5eb62f096eec61828542a2bba3bfa57628843f61",
  "session": "session_017xRBcVy4VXmkQjn2MGeaWk (subagent: the parent's id)",
  "premise_still_valid": true,
  "summary": "Declaration-only rework of the contract review (comment 6495005521) on one new commit 5eb62f0 (base 472e55c, no merge of origin/main needed, no force-push, PR left draft). Changeset .changeset/credential-column-filter-sort-guard.md now declares `Clause-②: no (narrowing)` + `Metadata-protocol: none` at `patch`, carries exactly one ADR-0087 marker `not-required (no-migration-prescription)` (no metadata key, spelling, export or stored shape moves; what narrows is a runtime read door), and a `**BREAKING** (an accept-set narrowing)` banner with FROM / TO / Fix modelled on the #22646 entry; FROM / TO names the import consequence (an `upsert` / `update` import whose `matchFields` names a credential column now meets the refusal in `findExisting`'s lookup and that row is reported failed, not written). Should-fix 3: the comment in engine-number-comparand-declared-type-door.test.ts and engine-text-operator-declared-type-door.test.ts now says the door runs first inside lowerWhereFilterArray and the guard answers the rows it passes; no code change. PR BODY NOT WRITTEN BY ME: the os-dev contract forbids a dev PATCH of the PR body (dev writes it once, at pr_create), which conflicts with the dispatch asking for a PR-body change; per that contract the seat writes it. The complete replacement body (lines 1-3 `Clause-②: no (narrowing)` / `Metadata-protocol: none` / `Skills-docs: follow-up owed — skills/objectstack-data/rules/security.md`, a BREAKING sentence, the import-match bullet, the corrected fixture-triage sentence, the 维护者速读 risk line and `需要你做的` updated, plus a declaration-round Verification block) is at /tmp/claude-0/-home-user-objectstack/bdda939c-7424-5594-af38-0e6fe5d7eeed/scratchpad/pr-27477/prbody.new.md. It omits the old session-URL footer; add the footer per platform-readings, and read the body back after writing it. Skills-docs: I picked `follow-up owed`. The skill teaches `type: 'secret'` and says nothing about querying it, so an AI author following it can still write a refused filter.",
  "tests": "At head 5eb62f0: `pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/engine-number-comparand-declared-type-door.test.ts src/engine-text-operator-declared-type-door.test.ts src/credential-evaluation-guard.test.ts` under the lock: VERDICT command-exit 0, Test Files 3 passed (3), Tests 88 passed (88). `pnpm --filter @objectstack/objectql typecheck`: VERDICT command-exit 0; check:test-typecheck OK, 39 files / 233 errors / 64 pinned signatures held. Built `@objectstack/objectql^...` first (VERDICT 0). No ablation: no tested behaviour changed, only a changeset and two test comments.",
  "gates": "Declaration gates: `pnpm check:adr-0087-registration` exit 0 (self-test 705 assertions; `1 declared-breaking changeset(s)` `[BREAKING+clause-②-narrowing] not-required (no-migration-prescription)`). `node scripts/check-changeset-no-major.mjs` exit 0 with no event (level axis NOT APPLICABLE locally). With `--event` built from the draft replacement body, exit 0: `LEVEL AXIS: this PR declares clause-② no (narrowing) and metadata-protocol none ... a non-protocol change at patch`. With the current live body, exit 0 and arm `none declared`. Derived list (`dispatch-gates.mjs --repo objectstack-ai/objectstack --commands`, tree 5eb62f0, 12 commits behind origin/main, stale only in scripts/pm/check-owed-reviews.mjs and record-recognisers.mjs): 96 commands, all run with exit codes captured before any pipe. 94 gave exit 0. Three of those first exited 3 on a missing prerequisite and gave exit 0 on re-run: check-plugin-teardown-shape --self-test after the pinned fixture commit was present, check:lean-entry-closure after the objectql dist was built, and spec check:skill-examples after the client-react closure was built. 2 NOT MEASURED locally (exit 3, they need a whole-repo build): check:dual-build-cjs-loads and check:type-check-debt. `--ran` exit 0: 96 derived, 94 run, 2 NOT-MEASURED, 0 UNRUN. CI check-runs at 5eb62f0, all completed: Lint & Repo Gates success (it carries the 2 NOT MEASURED), TypeScript Type Check success, Test Core success (8/8 shards), Build Core success, Governed Surface Queue Guard success, Check Changeset success, Type Check UI success, Test UI success, Build & E2E UI success, Live E2E UI success, Lint UI skipped. Those check-runs ran against the CURRENT PR body (`Clause-②: no`). Once the seat writes the new body, Check Changeset / no-major re-read the declaration from the edit event.",
  "dogfood_battery": "5eb62f0 · owed by the `(narrowing)` arm · closure `@objectstack/dogfood^...` rebuilt, packages/runtime/.objectstack cleared · lock VERDICT command-exit 0 (held 1618s) · Test Files 348 passed, 3 skipped (351) · Tests 2808 passed, 20 skipped (2828) · no failures. The first attempt's build hit lock exit 99, and its test ran on an unbuilt closure (348 files failed). That run was NOT MEASURED and is not counted. It was re-run from the same slot.",
  "mcp_calls": "0",
  "api_writes": "1 — POST repos/objectstack-ai/objectstack/issues/27477/comments (this os-dev-report, via scripts/pm/post-stamped.mjs). git push of 5eb62f0 is not a REST write. No PR body, label or ready-state write.",
  "deviations": [
    "The harness attribution asked for a `Co-Authored-By` trailer that names the model. The commit carries AGENTS.md's model-free pair (`Co-authored-by: Claude` + `Claude-Session`) instead: the pre-push hook refuses a model identifier, and AGENTS.md takes precedence.",
    "Dispatch vs os-dev contract: the PR-body change was asked of me, but the contract says a dev never PATCHes a PR body. I prepared the replacement body for the seat to write and did not write it myself.",
    "I waited once (no kill) on a PID found with pgrep -f, for my own client-react build after the harness backgrounded it. Discipline is to wait only on a recorded PID."
  ],
  "open_questions": [],
  "out_of_scope_findings": [
    "class: c · reach: named producer: an object's authored `searchableFields` (stored object metadata) listing a `secret` / `password` field. The engine now silently withholds it from the search scan set, while the #22646 precedent refuses an explicit `$searchFields` naming an `internal` column loudly. Evidence: contract review ③-c and should-fix 1. Wanted fix: refuse such an entry at the spec door, and extend the data door's explicit-`searchFields` refusal to credential types. It changes what an author can write, so it gets its own card, not a rider here. Dedupe words: searchableFields credential secret password, searchFields refusal credential, silently inert searchableFields",
    "carrier: seat (Skills-docs follow-up owed) · skills/objectstack-data/rules/security.md: one sentence saying a credential column is never a filter or sort key, only a has-a-value check. The seat files it and rewrites the `Skills-docs:` line to `#N` before ACCEPT.",
    "carrier: seat (PR title) · noted, not filed. The PR title lacks the `!` that the BREAKING declaration implies (suggest `fix(objectql)!: extend the credential-column guard from aggregates to filter and sort keys`). The seat edits titles."
  ]
}

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 5eb62f096eec61828542a2bba3bfa57628843f61
Local-runs: none
Scope: .changeset/credential-column-filter-sort-guard.md, packages/objectql/src/engine-number-comparand-declared-type-door.test.ts, packages/objectql/src/engine-text-operator-declared-type-door.test.ts + the PR body's declaration lines, packages/core/src/utils/import-runner.ts (findExisting, the per-row catch and toFailedResult), packages/objectql/CHANGELOG.md (the #22646 entry), scripts/check-adr-0087-registration.mjs, scripts/check-changeset-no-major.mjs.

Delta review by the same reviewer of the full record at head 472e55c (comment 6495005521, FAIL on the declaration alone). The delta is one commit, 5eb62f0, declaration-only: the changeset body and two test-file comments. No source, test logic, docs page or census pin changed; every ①, ③ and ④ judgment of the full record stands unchanged and is not re-stated. The ADR-0100 hardening ruling holds: positions and refusals only, no reproduction.

① Derived judgments

  1. Nothing in the accept set or public surface moved since 472e55c. The diff touches no code path. The two declared-type door tests change only their header comment; the readAs fixture and every assertion are byte-identical. Correct.
  2. The corrected fixture comment is now true to the code: assertTextOperatorTargetsAreStringCapable and narrowNumberComparands run inside lowerWhereFilterArray, before rejectCredentialEvaluation, and the guard answers the rows those doors pass. Should-fix 3 of the full record is closed.
  3. The changeset's new FROM / TO is accurate against the code. Each FROM bullet names a position the full record verified (filter including $field, per-aggregation filter, expand tree, nested-relation condition; sort; search scan set; the import match lookup), and each TO bullet states the refusal the code gives. The import claim ("each row that reaches the lookup is reported failed with that refusal and is not written") is read true: findExisting calls findData with the import's caller context inside the per-row try, and the row loop's catch records toFailedResult(rowNo, err, objectName) for that row and continues; no write follows for it. Should-fix 2 of the full record is closed.

② Semver level

Declared now, in the changeset body and the PR body alike: Clause-②: no (narrowing), Metadata-protocol: none, @objectstack/objectql: patch; a **BREAKING** (an accept-set narrowing) banner with FROM / TO / Fix modelled on the #22646 entry; exactly one ADR-0087 HTML-comment marker in the changeset, category not-required (no-migration-prescription), stating that no metadata key, spelling, export or stored shape moves and that what narrows is a runtime read door; and a Skills-docs: follow-up owed — skills/objectstack-data/rules/security.md line in the PR body. That is the declaration ② of the full record asked for, and it is consistent with ruling A: a non-protocol narrowing ships patch and keeps its BREAKING carriers. Must-fix 1 is closed. The Skills-docs value is admissible (the skill teaches type: 'secret' and nothing about querying it; no eval exercises the refusal); the seat files the card and rewrites the line to #N before ACCEPT. Check-runs on 5eb62f0: Lint & Repo Gates, TypeScript Type Check, Test Core 8/8, Build Core, Governed Surface Queue Guard, Check Changeset (both the push run and the re-run triggered by the 13:04Z body edit, the one that re-reads the declaration) all green; Lint UI and the opt-in harnesses skipped by design. The os-dev report (comment 6496968203) states the full dogfood battery on 5eb62f0 at 348 files passed, 3 skipped, 0 failed; read as the dev's statement, not re-run here.

③ Boundary flags

  • The os-dev report records that the dev did not write the PR body (the os-dev contract reserves it to the seat); the seat wrote it at 13:04Z and the body read back here carries the three lines. No open flag.
  • The report's out-of-scope rows (searchableFields credential entry as its own card; the Skills-docs follow-up; the PR title lacking the ! the BREAKING banner implies) are the seat's at ACCEPT; the title note is cosmetic and not a contract item.
  • open_questions: none. Author is objectstack-fleet[bot]; no login note owed.

④ Design judgment

Unchanged from the full record: the narrowing takes no declared capability away at the consumer and makes ADR-0100's read-mask promise real at every position that evaluates the stored value, the same disposition #22646 settled for internal: true. Nothing in this delta bears on it.

Must-fix: none.

Should-fix: none beyond what the seat already carries (the searchableFields follow-up card and the Skills-docs card).

Implemented-by: claude/harden-credential-field-predicates
Reviewed-by: session_017xRBcVy4VXmkQjn2MGeaWk

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

ACCEPT → head 5eb62f096eec61828542a2bba3bfa57628843f61 · 2026-10-29T16:44Z

维护者速读

  • 改了什么:对非系统调用方,secret 和 password 类字段不能再用作过滤、排序、展开条件、聚合过滤,也不能出现在 update/delete 的条件里,否则返回 400 INVALID_FIELD;"是否有值"这类检查仍然允许。
  • 为什么改:QA 第三轮发现的安全加固(细节按披露裁决不公开)。
  • 风险与回滚:属于 BREAKING 收窄,以凭据字段做导入匹配的行会失败;回滚即 revert 本 PR。
  • 先看:packages/objectql/src/credential-evaluation-guard.ts。
  • 两分钟验证:pnpm --filter @objectstack/objectql exec vitest run src/credential-evaluation-guard.test.ts。
  • 需要你做的:无。

席位意见:收。增量契约复核 PASS(6497520141),dogfood 全套 348 个文件、0 失败;Skills-docs 后续已立卡 #27660。

席位核对

  1. 形态:draft、目标 main;不建卡,正文无关闭关键词。
  2. 前提实测:复核记录 6495005521 和 6497520141 都读过代码。
  3. 门禁:本 head 两个必需 job 都是 success;正文修改后会重跑 2 项检查,等它们跑完才挂入队列。
  4. 接受的偏差:无。
  5. 范围外去向:searchableFields 里配了凭据字段时会被静默忽略(失败即关闭,不会泄露),交后续卡处理。Skills-docs: #27660。
  6. 落地:不属于受管面,698 行 → pr_ready + automerge_enable。

Generated by Claude Code

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

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant