Skip to content

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

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

fix(authz): protect purpose-bound runtime integrity#65
seonghobae wants to merge 245 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. Policy authority remains an Orgmetra trusted-composition concern; Python value objects validate and detach authorization data but are not unforgeable capabilities against arbitrary code already executing in the service interpreter.

Protected base is develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f; current exact head is b7f2b730b8b231aa28e485c655fd8cdf18007d46. Open · Draft · mechanically mergeable. This long-lived branch is an ordinary descendant carrying inherited mutable-stack deltas; predecessor evidence does not authorize this head.

Current repair

Fresh review of predecessor 1caf8f760e81f1cb6954fdf1d0d13a46dbb6c0b1 found that a retained exact uuid.UUID could expose a caller-controlled .int payload to equality/range semantics before the retained scalar had been proved to be the exact built-in int.

Test-first a260e1c7e59cd0b9eed97f2b3a7e59280f99ccb5 added an exact-UUID forged-payload regression. Its hosted Foundation run was superseded before runner admission, so no hosted RED is claimed for that test-only head.

Ordinary child b7f2b730b8b231aa28e485c655fd8cdf18007d46 reads .int once, validates exact built-in int plus the 128-bit UUID range before sentinel equality, and then carries only the detached scalar. This closes the checked-vs-used retained-scalar path without claiming same-process arbitrary-code isolation.

The repair preserves the existing #65 authorization boundary: exact built-in UUID/text/frozenset validation, construction-time UUID detachment, policy/request revalidation at evaluation, exact coherent/non-subclassable AuthorizationDecision, and durable-consumer decision revalidation retained in branch history.

Current exact-head evidence

Foundation 34289116386 is RED, but RCA isolates the failure outside the #65 authorization package. packages/keyverse-adapter completed 119 passed with 100% coverage. The later Job Analysis API suite failed three retained-datetime/timezone-runtime regressions (3 failed, 196 passed) at the shared HRIS aggregate-kernel ensure_exact_utc_datetime boundary. That is the #63 shared-kernel prerequisite and the durable #244 retained-value invariant; do not copy mutable #63 source into #65.

Latest exact-head workflow read: SAST 34289116322 SUCCESS, Security 34289116372 SUCCESS, Dynamic Code Quality 34289108456 SUCCESS, Required Noema 34289115045 SUCCESS, and merge scheduler 34289114560 SUCCESS. Strix 34289115229 remains in progress and Required OpenCode 34289114784 remains queued.

CodeQL 34289116329 is terminal FAILURE and reproduces the central publication/settlement ordering defect. Actions consumer job 102271601195 enforced failure at 23:14:02Z and Python consumer job 102271601217 at 23:14:03Z; authoritative current-head dispatch job 102273249299 did not begin until 23:17:33Z and then succeeded at 23:17:39Z. The exact consumer canary is handed to canonical .github#2040; do not rerun this unchanged head, poll/sleep, synthesize status, or move the defect into the Orgmetra leaf workflow.

Fresh review enumeration has no qualifying independent APPROVED; known inline review threads are resolved. Keep Draft.

Stack and owner boundaries

Normal owner order remains #63 shared HRIS-kernel protected integration → #64 People mutation protected integration → #65 non-force protected adoption/reconciliation → #141/downstream consumers. #55 owns People reads. #100 alone edits docs/product-technical-gap-baseline.md; its #244 already captures the nested retained-value invariant, so this lane does not create a competing baseline edit.

Central required-workflow defects remain with their canonical owners, including #810 Dependency Review availability, #2040 CodeQL publication/settlement, and model-backed review infrastructure owners where applicable. Do not add direct provider/model/group pins or paid fallback.

No force push, destructive rebase, self/model approval, gate weakening, routine admin bypass, no-op retrigger, mutable sibling source copy, predecessor-verdict transfer, or premature Close.

@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