fix(runtime): gate field writes at the assignment, under the acting principal - #8656
Merged
Thamirawaran merged 14 commits intoAug 31, 2026
Merged
Conversation
…rincipal A permission-denied field write used to land on the shared in-process anchor and only be skipped at flush time, so the next flush performed by any principal holding write access serialized and committed it. The denial was a deferral, not a rejection. Field writes now consult check_write_access at the assignment, where the acting principal's context is bound, matching how edge creation and delete are already gated at the operation. A denied assignment records the PermissionDenied diagnostic (PermissionError under JAC_STRICT_PERMISSIONS) and never reaches the identity map that every request in the worker shares. The flush-time check stays as a backstop for mutation paths that do not go through __setattr__. The denial hint now names the scoped Jac.allow_root form first instead of only the deployment-global grant, and spells levels as the AccessLevel enum. Fixes jaseci-labs#8346
…ashes The new field-write gate calls check_write_access from Archetype.__setattr__, which is the first thing in this test to touch access-control machinery. That lazily resolves the context's roots for the first time, and root resolution persists a freshly materialized super-root as a side effect (session.impl.jac _resolve_roots -> mem.put), landing an extra anchor in _dirty that the read barrier then hashes alongside the one under test. A real request always resolves roots during auth, long before its first write, so force that here too and let the setup commit absorb it instead of the counted read barrier.
0.37.0 removed `Archetype.__setattr__` outright: dirty-marking now rides a `_dirty_setattr` hook installed on a generated subclass only when an anchor becomes persistent (`track_writes`). The gate moves to that hook and runs before the mutation, so the mechanism is preserved and a transient archetype pays nothing for it.
No source change. `passes-native`/`test-runtime` failed on a fault that cannot originate in this branch (see the PR comment); this re-triggers the lane so the real signal is visible.
Contributor
Author
|
CI note: the red lanes on this PR are not from this branch. Recording the
1.
A markdown file cannot cause that. Both #8654 and #8664 went 2. Whole-tree
Same 913 files, same "912 passed, 1 failed", different victim each run, and Other people's PRs look green only because their runs predate 0.37 (newest is Happy to file the |
jac check's fork work pool assigns files to workers differently on every run, and a worker serving a file with an incomplete stub prelude reports stdlib types as Self/<Unknown>. One unrelated file fails per run. No source change here.
marsninja
added a commit
that referenced
this pull request
Sep 4, 2026
… is policy instead of a grant the first toucher writes (#8963) ## Summary `jac run` on a project with a static scheduled task that touches `root.shared` logs this once per flush for the whole tick (about 97 lines for a 93-page ingest), and again at every close: ``` INFO - Current root doesn't have field_write access to NodeAnchor Root[1b21b425-...] WARNING - Permission denied: field_write on Root[1b21b425-...] owned by root[<system>]; required WRITE, have CONNECT. ... ``` Three layers combine to produce it, and the last one also launders a refused write into the store: 1. **`get_shared_root()` wrote a grant from a foreign session.** The first principal to touch `root.shared` ran `perm_grant(shared, CONNECT)`, which marks the guest Root dirty in *that* session. Static scheduled tasks run as the internal `__system__` user, an ordinary per-user root (not the super root), so in this app the first toucher is never the owner. 2. **The flush-time backstop re-fires forever.** `Session._flush_locked` refuses the row under the toucher's principal and never records it as flushed, so every later full flush (one per spawn/commit) re-denies and re-logs it. 3. **Close ran under the wrong principal.** The scheduler and the native server middleware reset the request context *before* `ctx.close()`, so the close-time commit ran as the process context (super root) and persisted the row the request had just been refused. That is the deferral-not-denial of #8346 on the ACL path: #8656 gated field writes at the assignment, but `allow_root` / `perm_grant` and friends had no gate at all, so a reader could still grant themself WRITE in memory and have the close write it. ## Fix (runtime) - **ACL writes are gated at the call, under the acting principal.** `allow_root`, `disallow_root`, `allow_group`, `disallow_group`, `perm_grant`, `perm_revoke` now call `check_write_access(anchor, op_hint="acl_write")` before mutating, the same shape as field writes (`__setattr__`), edge writes (`connect`) and `destroy`. A denied ACL write leaves nothing in memory for any later flush to pick up; advisory mode records the diagnostic, strict mode raises `PermissionError`. - **The commons floor is policy, not a stored grant.** `check_access_level` reads a never-granted shared root as `CONNECT` for every principal; an explicit level (`grant(root.shared, level=AccessLevel.READ)`) is respected, and per-root / group entries still apply on top. This is exactly the contract the OSP reference and `test_shared_root.jac` already describe ("the runtime floors the shared root's access at CONNECT"), now implemented without anyone writing a grant, so no session ever dirties the commons. `get_shared_root()` is read-only; the resolved id is cached as a `UUID` in one helper (`JacRuntime.shared_root_id`) used by both callers. Root-typed hops are resolved by the runtime rather than the store's ACL pushdown, which can only see stored grants (same exclusion rule the planner already applies to `__jac_access__` overrides). - **Sessions close under the request principal.** Scheduler `_run` and the native server middleware close the context before resetting the request context (with the reset in a `finally`). The flush-time check in `_flush_locked` stays as the backstop #8656 kept it for (in-place container mutation, which needs mutation attribution); with every graph mutation primitive gated at the call it no longer has a reason to fire on the shared-root path. ## Tests All through the real runtime, no mocks. - `tests/runtimelib/test_permission_diagnostics.jac` (+ fixture): cross-user `allow_root` and `perm_grant` emit exactly one `acl_write` diagnostic at the call and do not land, in memory or in the store (the owner still sees READ, the reader still cannot write the field); strict mode raises; the owner path emits nothing and lands. Three of the four **fail on main** (no diagnostic at the call, no strict raise). - `tests/runtimelib/test_shared_root.jac` (+ fixture): a reader's first touch of a commons that already sits in the store (hash stamped, access enforced, the scheduled-task shape) produces no denial in the response envelope, can attach, and leaves the commons' stored level untouched across a reload. **Fails on main** with three `field_write on Root` denials in one request. A second test pins that a typed hop back onto the commons (`note <-:HasNote:<- [?:Root]`) sees it under the floor. - All 7 pre-existing shared-root tests and 8 pre-existing diagnostics tests pass unchanged; `test_acl_pushdown.jac` and `test_group_membership_cost.jac` pass. Verified end to end on the reporting project: the scheduled docs sync completes with zero permission warnings on a fresh store. ## Not fixed here - In-place container mutation (`node.list.append(...)`) still reaches the flush backstop and still re-fires per flush; that is #8656's declared follow-up and needs mutation attribution in the session. - The floor compares against the shared-root id cached by the first `root.shared` call in the process; a typed hop that lands on the commons before anything in a fresh process has called `root.shared` still resolves through stored grants only. Resolving the id at server boot was tried and reverted: it runs the server's resolver outside a request, and the in-process server's first request then failed to find the guest root it had just committed. - Static scheduled tasks run as the `__system__` user's own root rather than the super root (`get_root_id('__system__')` special-cases the *name*, the scheduler passes the *id*). Orthogonal; left as is. --------- Co-authored-by: marsninja <marsninja@users.noreply.github.com>
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
3 participants
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.
Summary
Fixes #8346.
A caller holding only
AccessLevel.READon a node could mutate its fields. Theruntime detected the violation but only skipped the row at flush time
(
session.impl.jac,_flush_locked), leaving the mutated object in theidentity map every request in the worker shares. The next flush performed by
any principal holding write access serialized it and committed it: the denial
was a deferral, and the unauthorized value was laundered into the store under
someone else's authority.
The deeper cause is a scope mismatch: the gate was evaluated per flush, under
whichever principal happens to be flushing, while the mutation is per
process. Field writes were also the only graph operation gated at
persistence: edge creation and delete both check at the operation
(
runtime.impl.jacconnect/destroy). This PR restores the symmetry:Archetype.__setattr__now consultscheck_write_access(anchor, op_hint="field_write")at the assignment, where the acting principal'scontext is bound. A denied assignment records the
PermissionDenieddiagnostic (surfaced as
warningsin the response envelope), raisesPermissionErrorunderJAC_STRICT_PERMISSIONS, and never reaches theshared identity map, so there is nothing for a later authorized flush to
launder.
Jac.allow_root(node, root_id, AccessLevel.X)before the deployment-globalgrant(node, ...)(the oldtext recommended only the global form, which opens the node to every user
of the deployment), and spells levels as the
AccessLevelenum instead ofthe pre-enum bare names.
The flush-time check is deliberately kept as a backstop for mutation paths
that do not go through
__setattr__(see "Not fixed here").Tests (all real subprocess / real HTTP, no mocks)
scale/tests/server/test_acl_field_write.jac: drives a realjac run --servesubprocess with two registered users over HTTP. Readerwith READ attempts a write; asserts the denial warning is emitted, the
shared anchor is not mutated, and the owner's next flush does not commit
the tampered value. Fails on pre-fix code with
body_now='TAMPERED'(the laundered value), passes with the fix. Apositive-control test asserts owner writes are unaffected.
tests/runtimelib/test_permission_diagnostics.jac: two new tests throughthe real CLI runner: a denied assignment leaves the in-memory archetype
unchanged, and strict mode raises
PermissionErroron a cross-user fieldwrite. All 6 pre-existing tests in the suite pass unchanged (the
diagnostic contract and persist-drop behavior are preserved).
Could this have been less code?
Searched and rejected/reused, in order:
check_write_access/_check_accesswholesale: the gate is onecall at the assignment; diagnostics, strict-mode raise, logging, and hint
all come from the existing machinery. No new permission surface.
Session._evict_uncommitted(evict-at-flush, the issue's fix option 2):rejected after reading
unit_enter/unit_exit. Overlapping requests shareone transaction, and the closing request's context performs the commit for
all of them, so an evict keyed on the flushing principal would drop a
concurrent legitimate writer's pending change (today that change survives
via the hash mismatch and lands on a later flush). Eviction is only safe
with per-mutation principal attribution, which is a larger session redesign.
strict_permissionsdefault flip (fix option 1): rejected as a breakingpolicy change; advisory-by-default is documented behavior (Cross-user writes (edges, mutations, deletes) silently no-op when permissions are insufficient #5788). This PR
makes advisory mean "no exception", not "the write happens anyway"; the
strict escape hatch is unchanged.
not a scale extension point.
Deleted
check_write_accessin
_flush_lockedis retained on purpose as a backstop for non-setattrmutation vectors (in-place container mutation, e.g.
node.items.append(x),which never enters
__setattr__); no test asserted the launderingbehavior; no config key or env var is superseded
(
JAC_STRICT_PERMISSIONSkeeps its meaning at the new, earlier gate).What else could this break
Grepped every changed symbol (
check_write_access,op_hint="field_write",__setattr__,PermissionDenied.message,strict_permissions) for othercall sites; findings:
assigns fields before attaching
__jac__(serializer.impl.jac,setattr(arch, '__jac__', anchor)is last), stub hydration copies__dict__directly, andAnchor-level sets never enterArchetype.__setattr__.fast-path in
check_access_level; fresh anchors short-circuit onhash == 0. Covered by the owner-control tests.in
jac start,jac run --serve, and scale pods; each pod's worker getsthe same per-process protection. Not cluster-specific (kind vs EKS
untouched), store-agnostic (fires before persistence, embedded or external
DB), and replica-count-independent (the poisoned map was per-pod; so is
the gate).
within the request (then flush-denied and laundered); they now see the
original value plus the warning. This is the intended tightening; any app
relying on the temporary in-memory effect was relying on the defect.
different bound root and mutates "its" nodes could previously succeed by
launder; it is now denied with a diagnostic. That corner is already
tracked as the root-binding question in A generator served as SSE executes against a different root: streamed handlers read an empty graph and their writes are lost #8136/RFC: One execution contract, two deployment strategies #8173.
__jac_access__hooks that themselves assign fields on apersistent foreign anchor during policy evaluation would now recurse
into the gate (bounded by Python's recursion limit, so loud, not a hang).
The same class of reentrancy already exists for hooks that create edges
(connect gate). No hook in the tree does either.
Own-anchor assignments hit the owner fast-path (a few comparisons);
granted foreign-anchor assignments may resolve the owning root from the
session, a cost previously paid once per anchor at flush and now paid per
assignment. If an assignment-heavy workload on shared anchors shows up,
a per-request memo is a contained follow-up.
Not fixed here (follow-ups)
node.list.append(...)) bypasses__setattr__and still reaches the flush path, where the skip-not-evictbehavior remains; closing that needs mutation attribution in the session.
ok: true(issue's fix option 3) is a response-contract question left to Jac Scale: a permission-denied field write still mutates the shared in-process anchor, so the next authorized flush writes the unauthorized value to Postgres #8346's
scope note.