Skip to content

fix(authz): protect purpose-bound runtime integrity - #65

Draft
seonghobae wants to merge 243 commits into
developfrom
fix/purpose-bound-authorization-runtime-integrity
Draft

fix(authz): protect purpose-bound runtime integrity#65
seonghobae wants to merge 243 commits into
developfrom
fix/purpose-bound-authorization-runtime-integrity

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Scope

Canonical Orgmetra purpose-bound authorization/runtime-integrity lane. Keyverse remains identity/credential/scope authority; Orgmetra remains HR policy/evidence authority. #65 does not copy mutable #63/#64/#141/Assignment source.

Current exact head remains 1caf8f760e81f1cb6954fdf1d0d13a46dbb6c0b1 on protected develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f, open · Draft · mechanically mergeable. Its retained #182#210 authorization hardening and package-root validate_authorization_decision(...) flow remain valid delta.

Current hosted RED and owner RCA

Foundation 33932302769, repository-quality job 101213236190, is a real exact-head RED. Checkout, runner, Foundation validation and dependency hygiene passed; all 118 keyverse-adapter tests passed at 100% statement/branch coverage. The first failing consumer was the older shared HRIS-kernel audit timestamp boundary. That defect is owned and repaired by #63. #65 must not copy mutable #63 source; preserve this RED until #63 becomes protected truth, then non-force adopt and reacquire exact-head acceptance.

Owner prerequisites

#63 is exact 42ef99ae272d78aeeeea7aac6cfd533b800a40a4 on the same protected base, open · Draft · mechanically mergeable. Exact-head Foundation 34089818143, Security 34089818168, and SAST 34089818051 are terminal SUCCESS; CodeQL 34089818084 is terminal FAILURE in the central exact-head verdict-handoff path rather than a leaf source verdict. Qualifying independent approval is still absent. Do not transfer those partial gates or bypass normal integration.

#64 People mutation/hire owner is now exact 818950960c4fe0a4e7b026bd5499fe9cca1002be, open · Draft · mechanically mergeable on protected develop. It retains #229#233, #246, packaging/runtime-floor repairs, #262 exact operational-UUID boundaries, #264 immutable validated PostgreSQL capability binding, #266#269 complete command/result alias detachment, and #270 pre-executable replay-verification evidence binding.

#271 was invalidated before integration. Assignment already serializes valid same-Employment writers on the current converted-worker row and same-Position writers on the Position root. Ordinary successor 19c6c7e... removed the redundant Employment lock and source-shape regression; #65 must not inherit or reintroduce it.

#272 owns executable PostgreSQL behavior evidence. The owner lane uses two independent libpq-backed writers under READ COMMITTED, requires pg_stat_activity/pg_blocking_pids(...) to expose the real conflict, and distinguishes deterministic synthetic integration fixtures from provenance-backed commercial/scientific evidence. The cleanup chain includes barrier release, server/client cleanup, deadline-bounded joins, verified backend termination on expiry, and fail-closed thread quiescence.

Exact predecessor 52257a66f68cf0893f2b351cbc0392b3e52f5bb4 repaired the termination receipt itself to require exactly one captured PID + internal application_name match and successful pg_terminate_backend, represented as exact 1|true. Foundation 34127782377 subsequently produced a real hosted RED: the forced-timeout regression expired the first cleanup join for both writers although only writer B's connection cleanup was deliberately held. Writer A was allowed to quiesce naturally after barrier release and could disappear before the termination lookup, so the new exact receipt correctly rejected that manufactured zero-match termination requirement.

Ordinary successor 818950960c4fe0a4e7b026bd5499fe9cca1002be repairs only that regression seam. The timeout probe now identifies the deliberately held writer from its factory and forces first-join expiry only for that writer; the released writer receives the real bounded cleanup join. The held writer remains live so the real PID + internal application_name termination query and exact receipt are still exercised. Production People SQL, transaction semantics, row-lock ordering, RLS, audit/outbox, idempotency, and #65 authorization semantics are unchanged.

#64 Foundation 34131641452, job 101772911159, is now terminal SUCCESS on exact 818950.... Exact checkout and the full People API test tree passed under the pinned Foundation PostgreSQL image, including #272's normal two-session interleavings and forced post-lock cleanup/timeout regression. Security 34131642055, SAST 34131642172, and CodeQL 34131641597 remain non-terminal, and qualifying independent approval is absent. Treat the Foundation result as current owner mechanism evidence only; do not transfer it to #65 or infer the remaining gates.

The real/right-cleared commercial/scientific evidence tier remains separate. #65 must never amplify the synthetic concurrency fixture into buyer-realistic or scientific evidence without provenance-backed permitted data.

#64 and #65 overlap on hire.py, mutations.py, postgres_hire.py, and postgres_mutations.py. After #63 and #64 integrate normally, #65 must non-force adopt protected owner state while preserving #229#233, #246, packaging/runtime-floor regressions, #262/#264/#266#270, #272's measured PostgreSQL interleaving + evidence-classification + deterministic cleanup + bounded-wait + verified-owned-backend + exact one-session termination-receipt contract, and the requirement that a forced timeout seam target only a writer intentionally kept live. Preserve inherited acceptance regressions and #65's own detached/revalidated AuthorizationDecision semantics. Do not retarget onto mutable #64 or cherry-pick its source.

#141 follows #65 and must later non-force adopt protected owner truth while retaining only its employing-legal-Organization feature delta. Assignment #163/#165 remain downstream consumers. The independent Workforce Validation lane may progress from protected develop while these prerequisites run.

docs/product-technical-gap-baseline.md remains single-writer-owned by #100. Durable invariants may flow there through owner handoff; volatile SHAs/run IDs stay in PR/Issue metadata. #271's invalidated lock claim is not a baseline invariant, and #272 synthetic fixtures are integration evidence only.

Canonical order remains #63 normal protected integration → #64 remaining exact-head required workflows/governance → #65 non-force protected adoption/reconciliation + fresh exact-head acceptance → #141 → Assignment descendants. No self-approval, administrator bypass, gate weakening, force-push, destructive rebase, mutable-owner source copy, no-op retrigger, or predecessor-evidence transfer.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 21, 2026 09:38
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent Please review the current unchanged head against protected develop. Local exact-head verification: all owned package suites pass at 100% statement/branch coverage.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact head c35d434adfdee94fa838819bba51bd2449ce940d. Focus on AuthorizationDecision construction-time runtime integrity, allow/deny field invariants, reference namespace binding, compatibility with existing direct decision constructors, and the new adversarial decision-evidence regressions. Do not transfer findings or approval from predecessor heads.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Exact revision =='
git rev-parse HEAD
git status --short
printf '%s\n' '== Merge base with develop =='
git merge-base HEAD develop
printf '%s\n' '== Changed files =='
git diff --name-status develop...HEAD
printf '%s\n' '== Python structure candidates =='
fd -e py . | xargs -r ast-grep outline --items all --type class,function --match 'AuthorizationDecision|Authorization|Decision' 2>/dev/null || true
printf '%s\n' '== Focused diff =='
git diff --find-renames --unified=80 develop...HEAD -- '*.py'

Length of output: 44422


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Authorization module, complete numbered source =='
cat -n packages/keyverse-adapter/src/orgmetra_keyverse_adapter/authorization.py

printf '%s\n' '== Direct constructors and consumer references =='
rg -n -C 8 --glob '*.py' \
  '(AuthorizationDecision\s*\(|\bAuthorizationDecision\b)' \
  packages services

printf '%s\n' '== Existing adapter authorization tests =='
cat -n packages/keyverse-adapter/tests/test_authorization.py

printf '%s\n' '== Persistence-boundary authorization validation =='
sed -n '150,230p' services/people-api/src/orgmetra_people_api/postgres_hire.py
sed -n '1,260p' services/people-api/src/orgmetra_people_api/postgres_mutations.py

printf '%s\n' '== Decision construction test map =='
rg -n -C 5 --glob 'test_*.py' \
  '(AuthorizationDecision\s*\(|_decision\s*\(|allowed=False|authorized_fields)' \
  packages/keyverse-adapter/tests services/people-api/tests services/job-analysis-api/tests

Length of output: 169233


@seonghobae I found one runtime-integrity bypass on exact head c35d434adfdee94fa838819bba51bd2449ce940d.

High: Prevent AuthorizationDecision subclass validation bypass

AuthorizationDecision is still subclassable. A subclass can override __post_init__ and omit super().__post_init__(). The generated dataclass initializer dispatches to that override. The subclass can then contain malformed or executable evidence.

The People persistence boundaries accept the subclass because they use isinstance(authorization, AuthorizationDecision).

  • packages/keyverse-adapter/src/orgmetra_keyverse_adapter/authorization.py, Line 188
  • services/people-api/src/orgmetra_people_api/postgres_hire.py, Line 179
  • services/people-api/src/orgmetra_people_api/postgres_mutations.py, Line 325

This bypass can reintroduce the forged allowed, UUID, string, field-set, and reference values that this change intends to reject.

Make AuthorizationDecision non-subclassable, for example with an __init_subclass__ method that raises TypeError. Add an adversarial regression that defines a subclass which skips __post_init__, then verify that class definition or construction fails. This preserves existing direct construction of the exact AuthorizationDecision class.

I did not transfer predecessor findings or approval. I completed static review only. The sandbox cannot execute the repository test suites.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact head edbd07337162f85ac7ab6b74e570b49e53a6d296, focusing on AuthorizationDecision verdict/reason polarity, preservation of extensible denial codes and non-authoritative bounded next_action, and compatibility with direct People persistence/replay fixtures. Do not transfer predecessor findings or GREEN evidence.

seonghobae commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Owner-path update after the #268 source repair: canonical #64 is now exact 6c8be6d44e28fe77bfe52a2e0946df4e0cbe3b83. #268 adds test-first retained nested-UUID mutation coverage for Employment, Position and Assignment across executable purpose-bound authorization, plus a direct PostgreSQL Position connection-factory callback regression; the production repair reconstructs every validated generic-command UUID from its once-read exact integer payload, so existing dataclasses.replace(...) boundaries detach nested identity authority as well as the outer command. Current #64 exact-head Foundation/Security/SAST/CodeQL runs are still queued and no qualifying APPROVED review exists, so this is not protected truth yet. #65 must not copy the mutable source; after #64 integrates normally, non-force adopt protected #64 including #262/#264/#266/#267/#268, then reconcile #65's own detached/revalidated AuthorizationDecision semantics and reacquire exact-head gates/review.

Copy link
Copy Markdown
Contributor Author

Owner-order update: canonical #64 has advanced by ordinary forward commits to 41829e0971a79012a2cd3723b8c355baa5acac3c on the same protected develop@eb9757f... base. In addition to #267/#268 nested command UUID detachment, #269 now requires accepted People mutation results themselves to be detached from adapter-retained outer result aliases and nested UUID payloads before validation/use/return. #65 must not copy mutable #64 source. After #64 normally integrates, non-force adopt protected truth and preserve #229#233, #246, packaging/runtime-floor repairs, #262, #264, #266, #267, #268, #269 plus #65's own detached/revalidated AuthorizationDecision semantics. Current #64 workflows for 41829e... are newly queued/pending; no predecessor GREEN transfer.

seonghobae commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Fresh canonical #64 owner handoff: #270 adds a separate post-executable replay-verification invariant after #246/#269. The replay check must not reuse either a semantic command object or an AuthorizationDecision object after those same objects have crossed an executable mutation-port boundary; otherwise the port can mutate checked evidence and manufacture a digest that the application verifies against the changed state. Current #64 keeps a second detached command copy for the port and reduces the authorized command + decision to an inert expected replay digest before persistence. After #64 normally integrates, #65 should non-force adopt protected truth and preserve #270 together with #229#233/#246/#262/#264/#266#269 and its own broader detached/revalidated AuthorizationDecision semantics. Do not copy current mutable #64 source or transfer predecessor checks.

Copy link
Copy Markdown
Contributor Author

Owner-path update after #271 revalidation: #64 ordinary-forward head 19c6c7e... removed the invalidated Employment-root lock experiment. New #272 is the canonical #64 acceptance gap for real PostgreSQL two-session Assignment concurrency evidence; #65 must inherit that protected test contract after #64 normal integration, but must not copy mutable #64 source or reintroduce #271. #65's own detached/revalidated AuthorizationDecision semantics remain unchanged.

Copy link
Copy Markdown
Contributor Author

#272 downstream handoff correction: when #65 eventually non-force adopts protected #64, preserve the PostgreSQL concurrency behavioral contract and evidence-tier boundary, but do not copy the current mutable test harness. Fresh review of #64 exact 1ea5cb1d... found that _exercise_conflict(...) lacks finally cleanup around DB lock observation; an assertion failure can leave writer A blocked at its pre-COMMIT barrier while the ephemeral PostgreSQL container unwinds. #272 now owns deterministic release/rollback + thread-join cleanup before teardown.

Also preserve the corrected evidence classification: deterministic synthetic structural rows may establish PostgreSQL mechanism/integration behavior, but they are not real/right-cleared buyer/scientific acceptance data. Do not inherit the superseded #271 Employment lock.

Copy link
Copy Markdown
Contributor Author

#64 owner handoff — fresh exact source is now 558afdf482af7e6e69ea0694e9f731365f1300be on protected develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f direct base. Intervening 6cf31e... repaired #272 failure cleanup by releasing the pre-COMMIT barrier and joining every started writer from a finally. 558afdf... adds the missing executable preservation regression: observe the real PostgreSQL lock first, force an assertion failure immediately afterward, then require zero concurrency-writer sessions in pg_stat_activity while the container is still alive. Do not copy this mutable harness/source. After #64 is normally integrated, non-force adopt only the protected behavioral contract: real DB-visible conflict serialization, failure-path writer cleanup before teardown, synthetic-integration-vs-real/right-cleared evidence separation, plus #64's retained #229#233/#246/#262/#264/#266#270 invariants. #65 remains owner of detached/revalidated AuthorizationDecision semantics.

Copy link
Copy Markdown
Contributor Author

#64 / #272 owner-path handoff update: current #64 exact head b71263918cc59b9ca4177abf1770f1c87b619440 extends the Assignment PostgreSQL failure-cleanup contract. A bounded Thread.join(timeout=30) is not sufficient evidence of quiescence: if the writer is still alive and its captured libpq connection remains open, #64 now targets only the verified owned backend (captured PID + internal writer application_name) with pg_terminate_backend, then performs a second bounded join and fails closed if the thread remains live. Do not copy the mutable harness/source into #65. After #64 is normally integrated into protected develop, non-force adopt the behavioral contract together with #272's DB-visible serialization and synthetic-vs-real evidence boundary, then preserve #65's own detached/revalidated AuthorizationDecision semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant