Skip to content

fix(objectql): a runAs:'user' hook carries its user's email, so a current_user.email row policy admits it as it admits the user's own request - #24674

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24639-hook-seed-email
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24639-hook-seed-email

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24639
Clause-②: no (a runtime producer change: a runAs: 'user' hook reads what the same user's own request reads. Nothing @objectstack/spec declares moves; no export is added or removed)

What was wrong

ScopedContext[SEED_USER_GRANTS] (packages/objectql/src/engine.ts, added by PR #24518) is the second half of a hook's runAs: 'user' derivation. It seeded positions / permissions and org_user_ids from resolveUserAuthzGrants, and never email, although that same resolution returns grants.email (the user's stored sys_user.email, from the user-row read it already makes). Its early return (if (!needsGrants && !needsPeers) return this;) also returned a context unchanged when only its email was missing.

The RLS compiler binds current_user.email from the context's email and nothing else (plugin-security/src/rls-compiler.ts). So a hook fired from a hand-built { isSystem: true, userId, tenantId } context (the approvals status mirror, a runAs: 'system' flow's writes) read nothing under a select policy assignee == current_user.email, while the same user's request read their row. It failed closed: rows hidden, never leaked.

Measurement (real stack, before and after)

The probe is packages/services/service-automation/src/runas-user-hook-email-owner.integration.test.ts, beside PR #24518's runas-user-hook-org-peers probe and its flow twin runas-user-email-owner. It boots ObjectKernel, ObjectQLPlugin, driver-sql on better-sqlite3 :memory:, the identity objects and the real SecurityPlugin with its shipped default sets plus a fixture set whose only row scope is assignee == current_user.email. A runAs: 'user' afterInsert hook reads the task object through ctx.api, and a middleware records the context that read carries. usr_member and usr_other each hold the set and each have one task assigned to their stored email.

The probe was committed alone first (f7366e4a5c) and run against unchanged source at f89d29e382. The after column is head 841da88b8c.

triggering context before: rows / hook context email after: rows / hook context email
CONTROL: the member's own request (and the GUARD: the same request without its email) member task (GUARD: none) same
the card's shape { isSystem: true, userId, tenantId } none / absent member task / the stored email
the request-resolved envelope, email included (control) member task / verbatim member task / envelope verbatim, nothing resolved
a carried email that differs from the stored one other task / the carried one other task / the carried one, verbatim
positions, permission sets and peer list carried, no email (the early-return case) none / absent member task / the stored email; positions, sets and peer list verbatim

The change

  • ScopedContext[SEED_USER_GRANTS] (engine.ts) adds needsEmail = !ctx.email beside needsGrants / needsPeers, includes it in the early-return test, and seeds email: grants.email only when the context carries none. This is PR fix(objectql): a runAs:'user' hook sees its organization's members in sys_user, as the user's own request does #24518's rule extended to the email, as triage set it:
    • a carried email is kept verbatim (on a request-resolved context, the session's);
    • a context with no userId is still returned untouched;
    • the email comes from the same single resolution, so a context that resolves for its grants or peers pays nothing more. There is no second read and no second resolver.
  • SEED_USER_GRANTS stays a symbol the package entry does not export. No new member, export or type.
  • ⛔ No backfill in plugin-security; rls-compiler.ts is untouched (ruling A′, 6261536443 item 2).

Where the files landed vs the claim's fence. engine.ts (the seed only, plus its import comment) and hook-run-as.ts (the seed's contract text, three docblocks) are in the fence. One file is outside it: hook-wrappers.ts. Its installRunAsApi comment carries the seed's cost contract ("a request-resolved context carries both and resolves nothing"), and its resolver-failure warn said every policy reading current_user.org_user_ids drops out. This change makes both false for a context that lacks only the email, so the comment and the warn's wording were brought in line. No logic there moves. The real-stack probe lives in service-automation for the reason PR #24518 gives: objectql cannot depend on driver-sql, plugin-auth or plugin-security.

Cost. A runAs: 'user' hook whose triggering context lacks the email now pays one resolveUserAuthzGrants per dispatch, as one that lacks either list already did. A request-resolved context carries an email and pays nothing, unless its user stores none (a phone-only sign-up): then every such dispatch resolves and the context still carries no email, which fails closed as before.

Pins

  • hook-run-as.test.ts, new [#24639] block inside the [#24497] one (layer 1, a real ScopedContext over a recording engine that answers the resolver's reads; its sys_user row now carries an email):
    • the hand-built shape carries the stored email, with ONE sys_user read;
    • a carried email stays verbatim while grants and peers are still taken;
    • a context with permissions and a peer list but no email is not returned early: it takes the email alone, the rest verbatim;
    • a context with no userId is returned by reference, nothing read.
  • Moved pins, two:
    • a context that already carries a peer list (a request-resolved one) resolves NOTHING used OPERATOR plus a peer list, which is not a request-resolved context: the request path seeds an email. The fixture now carries one (a fixture that lacked a declaration, re-judged rather than re-spelled), and the pin also asserts the email stays verbatim.
    • the resolver-failure pin matched the warn's sentence current_user.org_user_ids drops out. It now asserts the two variables the warn names (current_user.org_user_ids, current_user.email), not a sentence.
  • The real-stack probe above (5 cases).

Ablation (committed first, mutated through scripts/ablation-replace.mjs)

  • Mutation: delete the one seeding line, ...(needsEmail && grants.email ? { email: grants.email } : {}), (anchor 1 to 0; blob a2c16cc04b to 48e99778a5). objectql rebuilt; ablation-dist-preflight.mjs @objectstack/objectql 'email: grants.email' --absent: marker absent from all 14 built files.
  • Unit: 2 red, 23 green: the hand-built shape and the early-return case (expected undefined to be 'operator@example.test').
  • Integration: 2 red, 3 green: the card's shape (expected [] to deeply equal [ 'member task' ]) and the early-return case (expected undefined to be 'qa-hook-member@example.test'). The two controls and the carried-email pin stay green.
  • Restore: blob equal to HEAD (a2c16cc04b), git diff HEAD empty, whole tree clean, rebuilt, marker present again in 4 built files. 25/25 and 5/5 green.

Tests and gates (head 841da88b8c)

  • pnpm --filter @objectstack/objectql typecheck: exit 0 (check:test-typecheck: OK, debt held at 39 files / 233 errors / 64 signatures).
  • pnpm --filter @objectstack/service-automation typecheck: exit 0 (0 / 0 / 0).
  • objectql, the full local project in three shards (--shard=1/3..3/3): 448 files, 8451 tests passed (2738 + 2809 + 2904).
  • service-automation runas-*, crud-runas and resume-decider-identity: 9 files, 93 tests passed.
  • plugin-approvals status-mirror-envelope and status-mirror-cascade (the named producer): 2 files, 12 tests passed.
  • Derived gates (node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, no paths, 72 commands): all 72 exit 0. check:dual-build-cjs-loads first exited 3 (PREREQUISITE NOT MET, unbuilt packages); after a full turbo run build it exited 0. check:engine-split-ratio ran on a history deepened with git fetch --shallow-since=2026-07-14 origin main.
  • --ran: 72 derived famil(ies) accounted for — 72 run, 0 NOT-MEASURED (a DERIVED zero — all 72 recorded an exit code and none of them is 3).
  • test:repo was not run locally: no test of that project is touched and the derivation names none; Test Core runs it.

Acceptance notes

  • origin/main moved during this run (f89d29e382 to bfc1539fe0, four commits). None touches objectql, the hook path or the resolver; the only overlap by package is a one-line help string in service-automation/src/builtin/try-catch-node.ts. The branch was not merged; this PR's CI merge ref and the queue validate the joint tree.
  • The unreleased PR fix(objectql): a runAs:'user' hook sees its organization's members in sys_user, as the user's own request does #24518 changeset (.changeset/24497-hook-runas-org-peers.md) says a request-resolved context resolves nothing. That stays true for a user with a stored email; this PR's changeset states the email rule, and both ship in the same release. The older changeset is not edited here.

Generated by Claude Code

…he fix)

The real-stack probe and the unit pins for a hook fired from a hand-built
context that names a user and carries no email. Committed alone so the
before column is read against unchanged source.

Claude-Session: https://claude.ai/code/session_01AHeUBbe6S3JRawcpbLejXD
Co-authored-by: Claude <noreply@anthropic.com>
… a current_user.email row policy matches as on the user's own request

ScopedContext[SEED_USER_GRANTS] seeded positions, permission sets and the
peer list from resolveUserAuthzGrants and never the email that same
resolution returns, and its early return also skipped a context that lacked
only the email. A hook fired from a hand-built { isSystem: true, userId,
tenantId } context therefore read nothing under an email-keyed select policy.
The seed now takes grants.email where the context carries none, keeps a
carried email verbatim, and no longer returns early on a context that lacks
only the email.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-handling-server.mdx (via ScopedContext (symbol, a top-level class))
  • content/docs/automation/hook-bodies.mdx (via ScopedContext (symbol, a top-level class))
  • content/docs/permissions/system-context.mdx (via ScopedContext (symbol, a top-level class))
What this run could not see
  • 1 changed file(s) yielded no anchor (packages/objectql/src/hook-run-as.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 54 of 206 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 — 17 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 bfc1539fe00a33bc3215614a8d6c76a05eb8e8aa → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 90e5a36c71a2269f0f311b09b157286951af061c — the merge of head 841da88b8c73a0543f73c075436bcc73866140c0 into base bfc1539fe00a33bc3215614a8d6c76a05eb8e8aa, 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 90e5a36c71a2269f0f311b09b157286951af061c && git checkout 90e5a36c71a2269f0f311b09b157286951af061c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bfc1539fe00a33bc3215614a8d6c76a05eb8e8aa 841da88b8c73a0543f73c075436bcc73866140c0 && git checkout -B drift-repro bfc1539fe00a33bc3215614a8d6c76a05eb8e8aa && git merge --no-ff 841da88b8c73a0543f73c075436bcc73866140c0

node scripts/docs-audit/affected-docs.mjs --json bfc1539fe00a33bc3215614a8d6c76a05eb8e8aa

⚠️ 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 bfc1539fe00a33bc3215614a8d6c76a05eb8e8aa → 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

席位核对 · ACCEPT · domain:engine seat 2 · session_01AHeUBbe6S3JRawcpbLejXD · 2026-10-19T22:49Z

Reviewed against GitHub on head 841da88b8c, not against the report.

  • Form: draft into main. The body's first line is Fixes #24639, and the next line starts Clause-②: no (the claim's value plus a reason in parentheses; clause2-line.mjs reads it as no, no arm). A closing-keyword scan of the whole body finds one hit, Fixes #24639.
  • Scope: 6 files, +343 / −22. check-governed-merges --pr 24674: not governed. Two paths sit outside claim 6278068776's surface, and both are accepted:
  • The fix, read in the diff: needsEmail = !ctx.email joins the early-return test, and email: grants.email is seeded only when it is needed and resolved. A carried email is kept verbatim, and a context with no userId is still returned untouched. SEED_USER_GRANTS stays unexported, and nothing @objectstack/spec declares moves. Ruling A′ item 2 holds: nothing is backfilled in plugin-security.
  • Not an elevation: the hook reads under the email the same user's own request carries (the stored sys_user.email, the request path's fallback), which is what runAs: 'user' promises. Before, it read less, failing closed.
  • Pins: the card's two probe legs, an already-carried email (kept verbatim against a different stored value) and the early-return case, on a real stack (driver-sql, SecurityPlugin, a select policy assignee == current_user.email), plus 4 unit pins. Red before: 2 of 25 and 2 of 5. Ablation of the seed line turned exactly (a) and (d) red, with the mutation proven absent from dist/, and the restore is proven.
  • Changeset (@objectstack/objectql patch): its bullets match the diff (missing email seeded, carried email verbatim, the early-return case, the no-stored-email user still carrying none). Its Clause-②: no line is bare.
  • Cost note accepted: a user with no stored email now pays one resolution per runAs: 'user' dispatch from their own request. It is stated in the PR's Cost paragraph and in the seed's docblock. Carrier: none.
  • Landing: held, not yet armed. main's hourly full battery is red (Dogfood Regression Gate (3/3) at cc95f5e63) and not yet attributed to a lane (see A conditional validation rule's when that reads through a lookup (record.account.status) saves at every door, but checkConditional evaluates it unhydrated, so every write the rule guards is refused as unevaluable #24512, 6278925351), and CI on this head is still converging (7 of 38 in progress, none red). This seat flips the PR to ready and arms auto-merge once both clear.

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/m tests tooling

Projects

None yet

2 participants