Repository navigation
fix(OMN-16976): scope the unbound tenant write to the attribution it records - #3088
Merged
github-actions[bot] merged 2 commits intoSep 1, 2026
Conversation
…records
OMN-16831 severed attribution from authorization -- correctly. It also, and
unintentionally, severed the write from the isolation MECHANISM: with no
authority bound, `TenantProjectionTableOperation.upsert` issues a bare INSERT
that never sets `app.tenant_id`.
Every tenant-classified relation carries
`WITH CHECK (tenant_id = current_setting('app.tenant_id', true))` under FORCE
ROW LEVEL SECURITY, and the runtime connects as `tenant_projection_writer`
(NOBYPASSRLS). An unset GUC compares against NULL, so Postgres denies the row.
The OMN-16831 fix removed the Python refusal and left a Postgres one in its
place -- which is why the .201 dev lane, running the released fix
(omnibase_infra 0.38.16, omnimarket 0.4.15), still writes nothing.
Live on that lane, 3h window: 40 InsufficientPrivilege on
projection_delegation_inference_response_text -- the one such relation
currently taking traffic -- and zero occurrences of the pre-fix
"cryptographically verified authority" refusal. Same relation, same role, same
statement, proven both ways in-lane: denied with the GUC unset, accepted with
it set from the row's own tenant_id.
The fix: when no authority is bound, the statement is scoped to the tenant the
row itself records. The scope value IS the row's `tenant_id`, so this widens
nothing -- the write is CONFINED to the identity it already declares and
cannot reach another tenant's rows. Reads are symmetric: scoped by the
caller's own filter, and a filter naming no tenant still opens no scope.
Why not the database side: the canonical shape enforced by
`application_database_domain_enforcement` is exactly one permissive ALL policy
for PUBLIC. A second policy admitting the writer role is rejected there as
"extra permissive policy ... widens access" -- as OMN-17298's own
0004_capability_scores_policy_atomic_restatement.sql states. GUC-scoped writes
are the only sanctioned path, so the writer must open the scope.
What stays enforced:
- No capability is minted. The claim is never verified against a dispatch
event and authorizes nothing; `_optional_tenant_context()` still returns
None. A bound authority still wins and is still verified.
- A tenant-less or blank row opens NO scope, so the relation's own policy
denies it. OMN-16831 ruling item 4 (a tenant-less row is the producer's
obligation) is now enforced by the database rather than by a Python raise.
- The scope is transaction-LOCAL (`set_config(..., is_local => true)`),
committed on success and rolled back on failure, so it cannot leak to the
next event on a pooled connection.
- INTERNAL and CATALOG domains supply no scope and open no tenant
transaction, unchanged.
`test_red_control_untrusted_tenant_selection` is deliberately sharpened rather
than deleted: it previously asserted an unverified claim "never reaches
set_config", which conflated opening a session to be ENTITLED to act as a
tenant (a capability) with scoping one statement to the identity that
statement is persisting (the mechanism). It now asserts the invariant that
actually matters -- no authority is minted, and the only scope opened is the
row's own.
RED proven: with the tests kept and only the source reverted to dev tip, 4 of
the 43 fail (StopIteration on the absent set_config, no scope on the read,
DID NOT RAISE on the rollback control).
Gates: 43/43 in the adapter module; 7857 passed in tests/unit/runtime +
tests/unit/topology + tests/ci; mypy --strict clean over 2917 source files;
ruff format + check clean. The 7 wall-clock performance thresholds that failed
under `-n 4` on a loaded host pass 3/3 in isolation with this change applied,
and are independent of this diff.
Ticket: OMN-16976
Contributor
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical findings | No |
blocked |
CRITICAL findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review (OMN-8468/OMN-8524)
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 31, 2026
…ibase_infra#3088 (#7890) * evidence(OMN-16976): author OCC companion for OmniNode-ai/omnibase_infra#3088 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 3ec0c4861eb30c4479eb4f32fbe57f40bc325104. * evidence(OMN-16976): self-bind OCC#7890 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
…riminator
The OMN-15421 domain-adapter proof encoded the half of OMN-16831 that this
ticket contradicts. `_prove_no_authority_records_the_claim` asserted the
unbound write is "still refused, by the DATABASE's own RLS policy
(InsufficientPrivilege)" and called that the correct end state -- while
`TenantProjectionTableOperation.upsert`'s own docstring, from the same change,
says "No authority bound: record exactly what the producer recorded". Both
cannot hold. On a FORCE-RLS relation written by a NOBYPASSRLS role the old
assertion means NO tenant projection row is writable by any production path on
any lane until `bind_projection_tenant_authority` gains its first non-test call
site -- which is the dead projection layer OMN-16976 was filed for.
The proof now asserts the resolved position, and is falsifiable in four
directions: the runtime does not refuse; the row LANDS attributed to exactly
the recorded claim (read back as admin, so the writer's own scope cannot
satisfy it); a tenant-less row is still refused BY THE DATABASE; and no
tenant-less row is ever present afterwards.
It also pins a PostgreSQL fact this change surfaced, found by running the
harness locally rather than in review. A custom GUC that has been set once does
not revert to NULL when its transaction ends -- it reverts to the empty string.
So on a REUSED connection (the runtime's normal case) a policy predicate of
`current_setting('app.tenant_id', true)::uuid` evaluates ''::uuid and raises
InvalidTextRepresentation (22P02) rather than returning NULL and failing the
WITH CHECK (42501). Fail-closed both ways -- the row is refused and nothing is
written under a borrowed identity -- so the proof asserts the refusal and its
SOURCE across both connection states rather than one SQLSTATE. The live
projection_delegation_inference_response_text policy compares as text with no
cast, so it takes the clean 42501 path; the ::uuid relations take the other.
Proven locally against real PostgreSQL 16 in the harness's own container:
"OMN-15421 PostgreSQL 16 rebuilt-container proof: PASS".
Ticket: OMN-16976
github-actions
Bot
deleted the
jonah/omn-16976-projection-writer-scopes-recorded-attribution
branch
September 1, 2026 00:04
This was referenced Sep 24, 2026
Merged
Patel230
added a commit
that referenced
this pull request
Sep 25, 2026
…in it to the ruling tests/integration/db/test_projection_writer_tenant_rls_omn15301.py drives the runtime's own handler_wiring adapter against PostgreSQL 16 with FORCE RLS and NOBYPASSRLS logins. It is in the skip baseline (no initdb on the runners), so nothing saw it go stale: 8 failed on dev, 3 at the OMN-17887 step-3 head. K1: test_missing_authority_is_adjudicated_by_rls_not_by_the_runtime asserted the pre-ruling refusal of every unbound write. Rewritten to the OMN-17422 option (a) ruling (#3088) as test_unbound_write_is_scoped_to_its_recorded_tenant: an unbound row recording A lands as A, B cannot read it, A can, and a row recording no tenant is refused on a fresh connection. K2: test_environment_tenant_is_not_authority wrote a row recording the same tenant the environment named, so it could no longer tell them apart. The row now records B with ONEX_TENANT_ID=A: it lands as B and A cannot read it. K3: fixture relations are built where physical_grant_schema_for_table says the runtime writes (generation_events is logically omninode_internal, physically public), and a new self-check names any relation a future remap strands. Verified on PostgreSQL 16: 16 passed (was 3 failed, 12 passed). RED controls in scratch copies, each against an unmodified copy that passes 16: restoring the pre-#3088 unbound branch fails K1 and K2; promoting ONEX_TENANT_ID into the scope fails K2; building the fixture in the wrong schema fails K3 and names the relation. Test-only; no production code changes.
jonahgabriel
pushed a commit
that referenced
this pull request
Sep 25, 2026
…in it to the ruling (#4112) tests/integration/db/test_projection_writer_tenant_rls_omn15301.py drives the runtime's own handler_wiring adapter against PostgreSQL 16 with FORCE RLS and NOBYPASSRLS logins. It is in the skip baseline (no initdb on the runners), so nothing saw it go stale: 8 failed on dev, 3 at the OMN-17887 step-3 head. K1: test_missing_authority_is_adjudicated_by_rls_not_by_the_runtime asserted the pre-ruling refusal of every unbound write. Rewritten to the OMN-17422 option (a) ruling (#3088) as test_unbound_write_is_scoped_to_its_recorded_tenant: an unbound row recording A lands as A, B cannot read it, A can, and a row recording no tenant is refused on a fresh connection. K2: test_environment_tenant_is_not_authority wrote a row recording the same tenant the environment named, so it could no longer tell them apart. The row now records B with ONEX_TENANT_ID=A: it lands as B and A cannot read it. K3: fixture relations are built where physical_grant_schema_for_table says the runtime writes (generation_events is logically omninode_internal, physically public), and a new self-check names any relation a future remap strands. Verified on PostgreSQL 16: 16 passed (was 3 failed, 12 passed). RED controls in scratch copies, each against an unmodified copy that passes 16: restoring the pre-#3088 unbound branch fails K1 and K2; promoting ONEX_TENANT_ID into the scope fails K2; building the fixture in the wrong schema fails K3 and names the relation. Test-only; no production code changes.
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.
OMN-16976
The OMN-16831 tenant-authority fix is released and is deployed. It still writes no rows. This is the reason, and the fix.
What the ticket's premise got wrong
The description says the fix is "merged but UNRELEASED" and that "every runtime resolves omnibase-infra 0.38.11 from PyPI". Both are stale. Read out of the deployed
omninode-runtimecontainer on the .201 dev lane this session:AC2 is met. The pre-fix refusal string
Tenant projection has no cryptographically verified authorityappears 0 times in a 3h log window. And the projection still writes nothing — 40InsufficientPrivilegein that same window, all onprojection_delegation_inference_response_text, the one tenant-classified relation currently taking traffic.Root cause: the fix removed the Python refusal and left a Postgres one
OMN-16831 severed attribution from authorization, correctly. It also severed the write from the isolation mechanism. With no authority bound — which is every real dispatch —
TenantProjectionTableOperation.upsertcalls_execute_upsert(tenant_context=None), which issues a bare INSERT and never opens a tenant transaction, soapp.tenant_idis never set.Every tenant-classified relation carries
WITH CHECK (tenant_id = current_setting('app.tenant_id', true))under FORCE ROW LEVEL SECURITY, and the runtime connects astenant_projection_writer(NOBYPASSRLS). An unset GUC reads NULL,tenant_id = NULLis false, the row is denied.Proven in-lane, same relation, same role, same statement, for a tenant slug outside the compiled
_LEGACY_TENANT_UUID_MAP(['beta-business-proof','beta-gateway-canary-79afa7263852','omninode']):Why the fix is writer-side, not database-side
FORCE RLS here is the deliberate target state, not drift: 10 relations in
omnidash_analyticscarry it, all tenant-classified, all written bytenant_projection_writer. And the canonical shape enforced byapplication_database_domain_enforcementis exactly one permissive ALL policy for PUBLIC. OMN-17298's own0004_capability_scores_policy_atomic_restatement.sqlstates the consequence outright:So a GUC-scoped write is the only sanctioned path. The writer has to open the scope.
The change
When no authority is bound, the statement is scoped to the tenant the row itself records. The scope value IS the row's
tenant_id, so this widens nothing — the write is confined to the identity it already declares and cannot reach another tenant's rows. Reads are symmetric: scoped by the caller's own filter; a filter naming no tenant opens no scope and stays blind.The read half is not cosmetic.
_preserve_existing_evidencereads before every upsert; under FORCE RLS an unscoped read returns[]even when the tenant's row exists, so the upsert that follows would overwrite accumulatedrecent_responsesinstead of merging it — silently.What stays enforced
_optional_tenant_context()still returnsNone. A bound authority still wins and is still verified against its event.set_config(..., is_local => true)), committed on success and rolled back on failure — it cannot leak to the next event on a pooled connection.The invariant I deliberately sharpened — please read this part
test_red_control_untrusted_tenant_selectionpreviously asserted that an unverified claim "never reachesset_config('app.tenant_id', ...)". That conflated two different things:The test now asserts the invariant that actually matters: no authority is minted, nothing is verified against an event, and the only scope opened is the row's own — asserted positionally against the row's
tenant_id. This is a judgement call made under Operating Rule 2 and it is the one thing in this PR worth arguing with.Red-then-green
Tests kept, source reverted to dev tip → 4 of 43 fail:
StopIterationon the absentset_config, no scope on the read,DID NOT RAISEon the rollback control. With the fix → 43/43.Gates
tests/unit/runtime+tests/unit/topology+tests/cimypy --strict7 wall-clock performance thresholds failed under
-n 4on a loaded host (policy_registry,registry_compute,handler_bootstrap_source— none touched by this diff). Verified independent: they pass 3/3 in isolation with this change applied.Not in this PR
tenant_projection_writerhas zero grants ontenant_registry_mirror, sosync_registry_tenant_uuidraisespermission deniedbefore resolution. Confirmed live again this session. Blocksnode_projection_delegation; does not block the relation this PR unblocks.tenant_registry_mirrorholds 0 rows on the dev lane (OMN-17374 adjacent finding 1).permission denied for table savings_estimates— same grant class.Ticket: OMN-16976
Evidence-Ticket: OMN-16976
Evidence-Source: OCC#7890