Add audited Admin control plane - #88
Conversation
|
Failed to generate code suggestions for PR |
|
@codex review |
|
Failed to generate code suggestions for PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a3cd447c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62c0650651
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf742a08bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68d5024d12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 209241ef93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 880f9ddba5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e861225fb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff15883975
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96b6627baf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fd5e122cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 206e0964be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02eba1ba43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 009f46e239
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if owner_account_id and row.owner_account_id is None: | ||
| row.owner_account_id = owner_account_id |
There was a problem hiding this comment.
For an anonymous EVM checkout, the reservation's expected_owner_account_id is None, but _bind_payer_account() can resolve the paying wallet to an existing disabled account; this assignment then attaches that account without locking or checking it. If its suspension operation already completed while the reservation was ownerless, provisioning starts afterward and leaves a live VM owned by the disabled account. The fresh evidence beyond the earlier reservation-fence finding is that the new check covers only the reservation's pre-payment owner, not the effective owner_account_id assigned here.
Useful? React with 👍 / 👎.
| .limit(1) | ||
| ) | ||
| ).scalar_one_or_none() | ||
| await self.refunds.record_owed( |
There was a problem hiding this comment.
When an extension fails after x402 settlement, this calls RefundService.record_owed(), whose underlying PaymentLedger.record_event() deliberately swallows persistence failures. A transient ledger/database failure therefore lets this method return normally and the route report that a refund was recorded even though no obligation exists and no recovery job will recreate it; use a required write or another durable retry mechanism for this financial record.
Useful? React with 👍 / 👎.
| operation = match_paid_operation(request.method, request.url.path) | ||
| if operation is None: | ||
| raise HTTPException(status_code=403, detail="admin_bypass_unclassified") | ||
| return operation.admin_operation_class |
There was a problem hiding this comment.
For /v1/tunnel/create, the matched catalog operation has no class override, so this returns its default diagnostic classification. An Admin session without recent password step-up can consequently provision live reverse-tunnel leases at the 120-per-minute diagnostic limit instead of the 10-per-hour real-cost limit, potentially exhausting the daemon's finite public-port pool; classify tunnel creation as real_cost before granting its waiver.
Useful? React with 👍 / 👎.
| extra["amount_received_crypto"] = ( | ||
| str(native_intent.amount_received_crypto) | ||
| if native_intent.amount_received_crypto is not None | ||
| else None |
There was a problem hiding this comment.
When a BTC/XMR domain-plus-VM bundle uses KEEP_VM and domain fulfillment fails, _fail_paid_order() correctly limits amount_usd to domain_amount_usd, but this copies the intent's full bundle payment into amount_received_crypto. The Admin refund response presents that field as the native amount to return, so an operator can refund the entire crypto payment while the customer keeps the provisioned VM; persist the prorated refundable crypto amount or clearly separate total received from the amount owed.
Useful? React with 👍 / 👎.
| config = HyruleConfig() | ||
| engine = create_db_engine(config.database_url) | ||
| try: | ||
| await init_db(engine) |
There was a problem hiding this comment.
If hyrule-admin create is accidentally run before revision 023, this init_db() call invokes Base.metadata.create_all(): against a revision-020 database it commits the new Admin tables but cannot add the new columns to existing tables, after which the bootstrap query fails on accounts.disabled_at. The database is then left partially upgraded and a subsequent alembic upgrade fails when it tries to create those already-created tables; the CLI should verify the Alembic head and fail without mutation rather than running create_all().
Useful? React with 👍 / 👎.
| stop.set() | ||
| await admin_operations_task |
There was a problem hiding this comment.
During worker shutdown, setting stop does not interrupt an Admin operation already inside process_admin_operations(), and this unconditional await blocks until its entire batch finishes. Since each operation can make sequential provider calls for many VMs and an XO exchange alone can wait up to 300 seconds, a deployment or SIGTERM can exceed its termination grace period before providers and the database are closed; cancel or timeout-and-cancel this task while leaving its durable running row for stale recovery.
Useful? React with 👍 / 👎.
| enabled_admin_count = await _enabled_admin_count(session, lock=True) | ||
| target = await _locked_account(session, account_id) |
There was a problem hiding this comment.
A privileged request keeps the detached actor accepted by the dependency but never locks or reloads that account or its session before mutating state. With three Admins, for example, B can enter this endpoint and block on the enabled-Admin lock while A disables B; after A commits and revokes B's sessions, B resumes with the stale actor, observes A and C as the two enabled Admins, and can still disable C. Revalidate the actor's enabled Admin role and live elevated session under the mutation transaction so demotion, disable, and session revocation fence already-started requests.
Useful? React with 👍 / 👎.
| async with _factory(state)() as session: | ||
| row = await session.get(VMRow, vm_id) | ||
| if row is None: | ||
| raise HTTPException(404, "VM not found") |
There was a problem hiding this comment.
The final fenced power-action branch explicitly excludes destroy, so this lookup releases its session before provider and database cleanup. If a VM transfer commits while destroy_vm() is deleting the provider instance, the transfer can return success and assign the attached domain to the recipient, after which destroy marks the VM destroyed and attempts domain detachment using the old owner captured before the transfer; that detachment can fail and leave the recipient's domain attached to a deleted VM. Hold the VM/domain ownership fence through destroy dispatch and persistence, using the same lock order as transfers.
Useful? React with 👍 / 👎.
Main already carries the audited admin console plus expiry retention. Keep that tree so this branch can merge without reverting it. Assisted-by: pi-coding-agent:grok-4.7
1 participant
Operators need authenticated, audited access to account and resource state, durable suspension/resumption work, and an explicit distinction between waived usage and settled revenue. Add the admin control plane and interactive
hyrule-admin createbootstrap, reconciled with current main's checkout ownership, catalog readiness gates, provisioning events and domain-settlement recovery.Browser-only admin sessions require session-bound CSRF protection; real-cost operations require recent password step-up. Disabled accounts are enforced, private routes are excluded from public OpenAPI and return no-store responses, and operations write audit records. API bearer credentials cannot compose with admin privilege. A supplied real x402 signature retains precedence over waivers. Waivers are disabled by default, rate-limited through durable records, and recorded separately from charged revenue; waived failures do not create fictitious refund obligations.
Validation: the reconciled full suite passed 758 tests with 11 warnings. The separate optional PostgreSQL 16.10 test passed the real empty → 020 → 023 → 020 → 023 migration path with fixture accounts and paid/development VM billing backfills. Existing account/VM rows survived the roundtrip. Ruff, full mypy on 110 source files and diff checks passed. The database fixture was network-isolated and removed with its test volume. No production data or services were changed.
Migration 023 currently follows 020; the former admin revision 017 conflicted with the already-deployed provisioning-events migration and is no longer used. Reconcile the parent with pending expiry/guest migrations before merge if they land first, retaining a single head and rerunning migration validation. A downgrade removes admin audit/operation tables and columns; preserve that history and reconcile outstanding operations before considering it. The test proves account/VM row preservation, not audit retention across schema rollback.
Rollout requires green CI, resolved review and the normal infrastructure pinned-SHA promotion workflow, with API/worker quiescence and provisioning preflight from network-operations PR547. Verify the live schema, API/worker health, session/CSRF behavior and an audited read-only admin view. Bootstrap only through the interactive CLI; keep payment waivers disabled until the companion frontend and audit paths are verified. See docs/runbooks/admin-promotion.md.
This is the admin foundation. The operator expiry-extension endpoint, scheduled expiry notices and recoverable disk retention from #110 remain separate outstanding work. Companion frontend: hyrule-web PR45.