feat(core): apply reviewed Organization hierarchy changes - #119
feat(core): apply reviewed Organization hierarchy changes#119seonghobae wants to merge 66 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| CREATE POLICY organization_hierarchy_change_application_scope_policy | ||
| ON organization_hierarchy_change_application_record | ||
| USING (tenant_record_id = current_tenant_record_id()) | ||
| WITH CHECK (tenant_record_id = current_tenant_record_id()); |
There was a problem hiding this comment.
🟥 Caller-controlled tenant context bypasses isolation
Any table-capable role can set orgmetra.tenant_record_id to another tenant. The RLS policy then grants that tenant's row access.
Was this helpful? React with 👍 or 👎 to provide feedback.
| PERFORM pg_catalog.pg_advisory_xact_lock( | ||
| pg_catalog.hashtextextended( | ||
| 'orgmetra_organization_hierarchy:' || p_tenant_record_id::text, | ||
| 0 | ||
| ) | ||
| ); |
There was a problem hiding this comment.
🔍 Serialization depends on exclusive write routing
The advisory lock coordinates only apply_organization_hierarchy_change callers. Deployment privileges must prevent direct hierarchy writes from bypassing its graph serialization.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ALTER TABLE organization_hierarchy_change_application_record ENABLE ROW LEVEL SECURITY; | ||
| ALTER TABLE organization_hierarchy_change_application_record FORCE ROW LEVEL SECURITY; | ||
| CREATE POLICY organization_hierarchy_change_application_scope_policy | ||
| ON organization_hierarchy_change_application_record | ||
| USING (tenant_record_id = current_tenant_record_id()) | ||
| WITH CHECK (tenant_record_id = current_tenant_record_id()); |
There was a problem hiding this comment.
🔍 Evidence table grants need discipline
The migration revokes public function execution, not table insertion. Future grants must keep direct evidence writes unavailable to routine roles.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Parent repair receipt: #96 has now ordinary-merged current protected #119 remains intentionally unchanged and Draft. Its base snapshot |
|
Fresh parent-authority correction for this descendant: #96 is no longer at the body-recorded This #119 head remains |
|
Live parent-authority correction: #96 is no longer |
|
Fresh parent-authority correction for the stacked child; source/commit graph is intentionally unchanged. Parent #96 remains exact
The child remains exact |
|
Parent-authority update after #284. Canonical #96 has advanced ordinarily to This does not move durable authority into the leaf. #119 still owns tenant-qualified persistence uniqueness, authoritative hierarchy re-resolution/mutation, transaction concurrency, authoritative system-recorded audit/outbox time, and immutable persistence. Keep the child graph unchanged while #96 is unintegrated: current #119 |
|
Current parent-authority correction after fresh #96 evidence sweep:
The child graph itself is unchanged: |
|
Fresh parent handoff: #96 has advanced by ordinary forward commits through #285. Current parent exact head is #285 closes a leaf process-local export capability gap: post-issuance Do not restack onto mutable #96 source. Preserve the existing order: #96 normal protected integration → non-force adopt resulting |
|
Parent #96 advanced ordinary-forward to |
|
Fresh parent-authority handoff from #96/#287. The #96 branch advanced ordinary-forward through test-first |
|
Parent authority advanced ordinary-forward for #288. Current #96 head is #288 closes only the process-local public-builder constructor-binding gap: both package-root and implementation-module builder paths retain the governed packet constructor selected at package import even if the later implementation-module class name is reassigned. It does not move durable tenant-qualified uniqueness, authoritative hierarchy re-resolution/concurrency, system-clock provenance, or immutable audit/outbox responsibility out of #119. Keep this child on its existing stale graph while #96 remains unintegrated. Required order is unchanged: normal #96 protected integration -> non-force adoption of resulting protected |
|
Fresh parent handoff: #96 advanced by ordinary forward commit to exact Do not restack this child while #96 remains mutable. Preserve the existing child |
|
Fresh parent handoff: #96 has advanced ordinary-forward to exact Do not restack this child onto mutable #96. Preserve current child |
|
Parent-authority correction only; no child source restack performed. Parent #96 remains exact This does not change #119’s stack rule. Current child |
|
Parent-authority correction for the current stack. Parent #96 remains exact Current exact-parent evidence is: Foundation Required Strix This does not change child ownership or justify mutable-parent restacking. #119 remains exact |
|
Current-parent authority correction without source restack: parent #96 remains exact Fresh exact-parent state is: Foundation This does not change the child graph. #119 remains exact |
|
Fresh authority correction: the opening parent snapshot in this PR body that still describes This does not authorize restacking this child. #119 remains exact |
1 participant
Stack position
Dependency-first descendant of #96 (
feat/organization-hierarchy-change-review). Fresh parent #96 is exactf2c6e70edcf5c62ae0f848c29efd7bd3b8d45b97, open · Draft · mechanically mergeable on protecteddevelop@eb9757f8649aaad026a9865508d9aad50c1a7a4f. The parent owns post-issuance representation/export integrity (#279), one-snapshot checked-versus-used issuance (#280), per-live-object single-use issuance (#281), timestamp provenance separation (#282), closure-private process-local issuance state (#283), captured digest verification (#284), retained-slot/export verifier integrity (#285), captured issuance semantic validation (#286), direct issuance snapshot/canonical-byte/live-reference-guard helper binding (#287), public builder-to-governed-packet authority (#288), transitive validation/canonicalization authority binding (#289), nestedrecorded_attimezone-runtime integrity before post-issuance comparison (#290), and exact built-in datetime representation identity after issuance (#291).#291 is the current parent repair lineage. Exact predecessor
70212e6e00e0e3c0104b2444aadd7ac01880847bretained #290's built-in nested-timezone guard but still used Python aware-datetime equality afterward. Because16:00+09:00 == 07:00+00:00, a low-level replacement by a distinct exact built-in datetime representing the same instant could escape the parent's stated changed-live-state rejection. Test-first4bf2c1411d19ce60099948ec9ce9d97c31af6ab8records the counterexample; no hosted RED is claimed because no PR workflow materialized before that test-only head was superseded. Minimal source repair1d5f423f3ff3c654dc6dfc6d896ab024c92b9045keeps the #290 nested-timezone guard and, for exact built-in datetime only, requires the retained value to be the same immutable object captured at issuance before skipping instant-based equality. Package CHANGELOG60d203d9..., README5e35c3ab..., TRACEABILITY70615f59..., and ADR 0096f2c6e70e...document the same boundary. Current exact-parent Foundation34441833963, Security34441833987, SAST34441834059, and CodeQL34441833995are queued/pending; Foundation job102758308481still has no runner (runner_id=0, no steps), so no current-head GREEN is claimed.Historical predecessor
6573a629...had Foundation34427958099/102717113694and SAST34427958070SUCCESS, plus central coverage/OpenCode/Noema GREEN. Its Security34427958063failed closed at dependency-review support and its CodeQL34427958061reproduced the central producer/consumer ordering defect. Strix34427956795/102719245996subsequently completed SUCCESS after a substantive scan, but that evidence is historical only for the current parent. #290's intermediate70212e6e...also has only superseded queued evidence and does not authorize the current head.This child still records the older parent snapshot in its commit graph: current child head is
eda9ef283996e65cf19ec250ae3aeec118001346, base reffeat/organization-hierarchy-change-reviewat staleb9f8e3d291c4bdcd2f0aa5f9d0378dea09e5e7cd, and GitHub reports open · Draft · conflicting/mergeable=false. The stale parent pointer is a repair finding, not a reason to close or destructively rebase this child. Parent #96 must integrate first; after normal protected integration, non-force adopt the resulting protecteddevelop, reconcile this existing child while preserving its valid delta, renumber provisional migrations 0027–0029 if required by integrated truth, reseal exact changed artifacts, and rerun every applicable exact-current-head gate.Governed application boundary
Migrations 0027–0029 implement a tenant-scoped bitemporal Organization hierarchy application boundary. The mutation re-resolves current/proposed parents, recomputes reviewed unit/hierarchy digests, serializes graph mutation per tenant, rejects stale evidence, self-parenting, current/future-effective cycles and parent continuity gaps, preserves pre-effective business-time truth by correction rather than rewrite, binds exact predecessor/successor/preserved segments, and writes immutable human-confirmed
organization_coreaudit/outbox evidence in the same transaction. FORCE RLS, typed/null-safe review evidence, append-only application evidence, hardened function search paths, and no routine PUBLIC function execution remain part of the boundary.The parent review packet is evidence only. This application boundary must consume the canonical evidence snapshot emitted by the integrated #96 contract and then independently re-resolve authoritative same-tenant HRIS truth at mutation time; it must not assume the parent packet's closure-private in-process tamper, single-use issuance, creation digest, captured verifier/validator/helper/builder/transitive bindings, nested timezone/identity guard, or live-reference state supplies durable uniqueness, authorization or concurrency control. #283–#291 reduce ordinary same-interpreter mutation/execution capability over leaf defense-in-depth state and its construction/verification/sealing paths; they do not move any durable HRIS responsibility from this application/persistence boundary.
Issue #282 adds a clock-provenance acceptance requirement. After normal parent integration, this transaction must generate or attest its authoritative system-recorded audit/outbox timestamp from the system/database clock boundary and persist it separately from packet
recorded_at. Packet time may remain immutable review evidence but must never become transaction-time authority. Preserveeffective_onbusiness time separately from review-evidence time and authoritative application/audit system time.Test-first repairs already preserved on this child
CREATE INDEX CONCURRENTLYretry: RED41328310e77bdf13d62ce39b2c41cb3e566b715d; migration 0028 detects/removes invalid same-name residue concurrently, preserves completed valid work and resumes governed indexes.ecb0b9cd08ae3a17d45e17b5ae0b718e5ddbab2a; repairc2879cd1754287adffadf7059e7a47055e8538ecvalidates owning table, btree method, uniqueness/readiness/validity, exact key order/predicate and exact enabled BEFORE INSERT ROW trigger function.f75e6377bacb54785cf8757a687114035a97ab44, wiring38d9534eb5c6e0c9ff1e185c7b5fe6241f1fb6ee, repaird0d8950dc818a7c6a2ff8aa1eaf8e36d600875f6scopes governed postconditions topublic.d84c5d88142a3726bbb65c6c775c8a154b26dea3, repair996aaef68fbcfb5954d9bb953228c4e37fc3dae8requires the exact preserved predecessor segment.c39d7f667bd0ff985239dc74594a80346fad2a3c, wiring13e98f047bc6f4cc18ab0de556981fac20e2e417, repairdabb5ba15497ddde9788bf98b230239e202df459rejects the extra segment.33392422398/ job99488965687; repair9d9ade3d2fa0d0d4fb9e5ad33ba2ef7813293cd5requires bothCONCURRENTLYandIF NOT EXISTSrather than weakening either property.eda9ef283996e65cf19ec250ae3aeec118001346preserves unrelated sealed manifest evidence.Broader tenant-context trust, exclusive write routing/future grant discipline, production runtime EXECUTE provisioning, provisional migration numbering, deployment-runner ownership and #282 authoritative clock provenance remain distinct controls.
Exact-current-head evidence
Exact current child head remains
eda9ef283996e65cf19ec250ae3aeec118001346. This child is still based on a stale #96 snapshot and conflicting/non-mergeable. Earlier hosted evidence belongs to predecessor heads and does not transfer. Reacquire exact-head evidence only after normal #96 protected integration and non-force child adoption rather than churning this stale descendant now.Do not merge while #96 is unintegrated, this child conflicts with its advanced parent, exact-current-head hosted evidence is absent/non-passing, or unresolved security/deployment/clock-provenance controls remain. Do not self-approve, use routine administrator bypass, weaken a gate, manufacture evidence, or directly modify a dedicated-writer dependency.