Repository navigation
fix(oauth): preserve current source ceilings and read-only userinfo - #192
LIghtJUNction wants to merge 4 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
The dual-locale failure is reproducible on head 16fc693: test_custodial_signup_login_and_device_consent_reuses_vault receives 400 invalid_grant at signup (119 other tests pass in each locale). Source inspection identifies the mismatch: custodial_create deliberately stores the vault signing credential with an empty ceiling, while OAuth binds the long-lived session to that vault key and captures the bootstrap token's temporary ceiling. The new source containment check compares those different roles and rejects legitimate signup. I am preparing a scoped fix that retains the vault's zero request authority, validates custodial grants against the live temporary/custodial policy, and preserves captured-grant containment and the registered-source contraction regressions. No change is being made to #189's feature branch or production. Full final-head validation remains required; this is diagnosis and an implementation plan, not a passing result. Failure evidence: https://github.com/TokenNotIncluded/msg.lmm.best/actions/runs/36687905619/job/109797908140 |
|
Pushed f55374a (tree ec3b9a3) to this PR's existing branch, without changing #189. Custodial sessions now validate captured grants against the live temporary policy after checking their active vault binding; vault signing keys retain an empty request ceiling. Derived credentials must also stay within their captured source grants. Registered-key source contraction checks are unchanged. Added 10 focused source-policy regressions, all passing locally. The real signup/device/refresh test now also asserts the vault key remains grantless and that later custodial policy contraction rejects userinfo without business writes. Ruff/format (597 files), Python 3.15 compile, wheel/sdist and package verification pass. Local PostgreSQL is unavailable, so the original failing HTTP flow, both locale jobs, full contracts and deployment still await final-head CI. No weakened or removed assertions. |
|
Final-head update: f55374a made the original custodial signup/device/refresh test pass, but its locale run exposed four API-key failures. API binding rows contain subject/parent/auth_version only, unlike OAuth families with captured ceilings; assuming the family shape caused internal_error. Pushed c92135b (tree 13f4c2b). API credentials again compare to the current signing-key ceiling, while OAuth credentials retain captured-grant containment and custodial live-policy checks. Added tests with the actual API row shape: both fail on f55374a and pass now; all 12 focused source-boundary tests pass. Existing API create/rotate/recover and contraction assertions are unchanged. Full dual-locale and final-tree CI is running again; this PR remains unmerged. |
|
Validation complete for c92135b, tree 13f4c2b: both C and en_US.utf8 integration jobs pass all 120 tests, including the formerly failing custodial flow and the API create/rotate/recovery/contraction cases. The full eight-shard contracts gate confirms 2,590 distinct nodes; quality, installed deployment rehearsal and all applicable boundary workflows pass. One superseded push integration run was cancelled, not counted as a pass. I downloaded the deployment artifact, verified its SHA256, and confirmed source.json has the same tree as this head. This resolves the regression on this feature branch; #192 remains unmerged into #189, and the existing automated review still points to an older head. Dual-locale evidence: https://github.com/TokenNotIncluded/msg.lmm.best/actions/runs/36692187729 |
Follow-up to #189 targeting its feature branch. A source key can lose authority without changing its ID, revocation timestamp or subject auth_version; existing OAuth/API credentials previously retained their copied ceiling. Userinfo also wrote retained OAuth rate rows on read.
Reuse existing scope_subset/constraints_subset to require captured OAuth source ceilings and current derived credential ceilings to fit the live source key. Source operation/scope contraction now fails closed for access, refresh and API delivery recovery. GET/POST userinfo use read transactions and do not run the durable grant-guess rate counter; current key/family/runtime/quarantine checks remain in place.
Six real PostgreSQL regression vectors plus focused dual-locale CI wiring. Actual red run 36686655096 against 6b7622d: both C and en_US.utf8 produced 6 failed / 114 passed. Four contraction vectors wrongly returned ok, GET userinfo changed authoritative oauth_states; the sixth failure was a missing Content-Type in the initial POST test, corrected to JSON {}. The initial quality artifact had zero lint diagnostics and one function-signature formatting change, now applied. These setup corrections are distinguished from the five confirmed behavior failures.
Fix head 16fc693, tree 40c2dc1336780a92bd34f4014d5c6db309229646. Final dual-locale regressions, full exact-node-ID gate, quality and deployment remain pending. This branch preserves #189 base 6ef1908 and does not overwrite ongoing work or weaken assertions. No production actions.
Refs #76, #78, #83.