fix(plugin-reports): forward the caller's execution envelope to the report read (#7204) - #7283
Merged
Merged
Conversation
…eport read (#7204) `executeReport` rebuilt a five-field projection of the caller's `ExecutionContext` (`userId` / `tenantId` / `positions` / `permissions` / `isSystem`) before handing it to the engine read that produces the report, while the method's own comment promised "reports execute with the caller's identity". `accessible_org_ids` was not in that projection, and `buildDriverOptions` reads it by name (ADR-0105 D2 / #3623) to widen the driver's native tenant scope to the caller's membership union under the `group` tenancy posture; absent, drivers fall back to active-org equality. So the identical query returned the union interactively and collapsed to the active org inside a saved or scheduled report -- silently short rows, no error. Measured end-to-end on a real kernel + `@objectstack/driver-sql` sqlite `:memory:`: 3 rows across two member orgs came back as `['a1','a2','b1']` interactively and `['a1','a2']` in the report; the scheduled CSV digest emailed the owner the same two. The read now receives the envelope whole (the #6206 ruling / #6523), minus the `__`-prefixed keys plugin-security stamps for the operation in flight, as a fresh object so a callee's stamp cannot write back into the caller's request context -- the shape plugin-audit (#7141) and service-storage (#7145) landed. `timezone` comes with it, so a read-time formula field resolves the caller's calendar day. The projection's `positions` / `permissions` / `isSystem` defaults are kept byte-for-byte. Direction outside `group` is unchanged and pinned: `isolated`, no posture provider, and a `group` caller with an empty accessible set all still read at active-org equality. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-help
marked this pull request as ready for review
August 10, 2026 05:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7204
executeReportrebuilt a five-field projection of the caller'sExecutionContext(userId/tenantId/positions/permissions/isSystem) before handing it to the engine read that produces the report — while the method's own comment promised "reports execute with the caller's identity". The projection is what broke that promise.Premise: verified on
origin/main@1da1f32a9The card's
report-service.ts:466anchor is theisSystemline of the projection; PR #7206's annotation widening is in the base, so the parameter is already the fullExecutionContextand the whole-envelope forward typechecks with no signature change. The engine consumer is atpackages/objectql/src/engine.ts:2247(accessible_org_idsunder thegroupposture), withhasTztwo lines up at:2228.Repro first — measured, on row sets
Real
ObjectKernel+ real ObjectQL engine +@objectstack/driver-sqlon better-sqlite3:memory:,groupposture, one caller whoseaccessible_org_idsspansorg_a+org_b, three rows split across both:engine.find['a1','a2','b1']['a1','a2','b1']run()['a1','a2']['a1','a2','b1']runAdHoc()['a1','a2']['a1','a2','b1']buildDriverOptionsreadsaccessible_org_idsby name to widen the driver's native tenant scope to the membership union (ADR-0105 D2 / #3623); absent, drivers "fall back to equality: fail toward isolation". So the direction is under-reporting, not a leak — a correctness defect, silent, with nothing in the output saying rows were dropped.timezonewas dropped by the same projection, and it has a read-path consumer too:applyFormulaPlan(engine.ts:777, called from thefindpath at:6007) evaluates read-time formula fields withexecCtx.timezone. Pinned on the row's VALUE — at2026-08-10T20:00:00Zatoday()formula field resolved2026-08-10in the report and2026-08-11interactively for aPacific/Kiritimaticaller. (buildDriverOptions' ownhasTzuse is write-path only in driver-sql — autonumber{YYYYMMDD}fill — so the formula path is where a read observes it.)The change
The read receives the caller's envelope whole — the #6206 ruling / #6523: enforcement adjudicates on the whole
resolveAuthzContextenvelope, never a per-site subset — minus the__-prefixed keys plugin-security stamps for the operation in flight, and as a fresh object. Same shapeplugin-audit(#7141 / PR #7143) andservice-storage(#7145 / PR #7207) landed.On the PM's mechanism hypothesis — whether the in-place-mutation hazard from #7141 applies here: it does, in a different direction, so the
__strip stays. The report's operation object ISreport.object_name, so plugin-security resolving the depth for it is correct. Two hazards remain and the strip covers both:__readScopewhen it resolves permission sets for the new object (if (permissionSets.length > 0),security-plugin.ts:1157). A stale depth would otherwise survive into a question it was never resolved for.sc.__readScope = …mutates whatever object it is handed, so forwarding the caller's own envelope by reference would leave the report's depth on the request context the route goes on using.Also preserved deliberately:
positions/permissions/isSystemdefaults, byte-for-byte, so this change adds fields without changing any that were already there (theisSystem ?? falsefallback the card flagged);assertExportAllowedstill runs against the un-projected caller context, above and untouched — pinned by identity (toBe(caller)).Reverse verification — two directions, and every pin goes red in exactly one
(a) fix removed (
git checkout origin/main -- report-service.ts): 5 red / 3 green.The three green ones are the point:
groupis the only posture the widening applies to, so a change that turned them red would have traded under-reporting for exposure.(b) the naive
context: context— the direction the card asked to be tested rather than assumed. It fixes the row sets and breaks the three preservation pins instead:So no pin in the block is decoration: (a) names the defect, (b) names what the naive fix would have cost.
Tests
packages/plugins/plugin-reports/src/report-group-posture-scope.integration.test.ts(new, 8 cases) carries the end-to-end row-set claims against the real stack — deliberately not a fake engine, because "the key is on the context object" is exactly what this defect looked like from inside the service. The structural half (which keys cross, freshness, defaults, the export-axis input) is 6 new cases inreport-service.test.tsagainst the existing fake engine.@objectstack/objectql+@objectstack/driver-sqlare added as devDependencies ofplugin-reportsfor that harness; runtime deps are unchanged.Local verification
Changeset:
patchfor@objectstack/plugin-reports, naming the before/after.Generated by Claude Code