feat(ai-gateway): add project usage governance - #543
Open
dviejokfs wants to merge 14 commits into
Open
Conversation
📓 Changelog previewThis is what your commits will add to the generated ## [Unreleased]
### Added
- **ai-gateway:** Reapply project usage governance on current main
- **cli:** Add AI gateway governance commands
- **web:** Add AI gateway governance tab for cost limits
### Documentation
- **skills:** Document AI gateway governance CLI commands
- **skills:** Pin CLI version and regenerate command references
### Fixed
- **ai-gateway:** Wire project/environment filters into usage summary endpoints
- **web:** Scope AI gateway governance spend display to the selected scope
- **ai-gateway:** Fix review findings in governance policy enforcement
- **web:** Match governance permission errors by problem type, not title
- **ai-gateway:** Correct 10 governance bugs found in PR #543 review
- **ai-gateway:** Stop leaking governance config in error responses
### Miscellaneous
- **web:** Regenerate SDK client for AI gateway governance routes
- **web:** Regenerate SDK client for scoped usage summary params
- **web:** Regenerate SDK client after rebase onto main |
dviejokfs
force-pushed
the
feat/ai-gateway-project-governance
branch
from
August 16, 2026 21:20
e961cd8 to
5edf624
Compare
dviejokfs
added a commit
that referenced
this pull request
Aug 16, 2026
Addresses a review-pr pass on PR #543: - migration: ai_gateway_rate_events was missing the request_id column that check_rates_and_record inserts into, which would break RPM enforcement on every fresh deployment (500 on first rate-limited scope). No prior test exercised the INSERT, so nothing caught it. - security: governance policies are operator-only by design (they set limits for a whole scope, not just the caller's own usage), but AiGatewayWrite alone is held by Role::User too. Add an explicit admin/platform-admin check to all three handlers so a regular user can no longer tamper with instance-wide or another project's limits. - correctness: a negative RPM/budget value returned 500 instead of 400 since InvalidGovernanceConfig mapped to INTERNAL_SERVER_ERROR. - add integration tests for RPM rejection, budget rejection, and expired-reservation-to-conservative-debit conversion -- the three enforcement paths that had no test coverage (the RPM test is what would have caught the request_id bug above). - add unit tests for the CLI's governance helper functions and for UsageFilter's project_id/environment_id parameter binding. - tidy the rate-event cleanup window (comment said one minute, query used one hour).
dviejokfs
added a commit
that referenced
this pull request
Aug 17, 2026
Blocking fixes: - Remove spurious `::uuid` cast in DELETE on ai_gateway_cost_reservations (request_id is varchar(64), not uuid; cast caused runtime type error) - Skip releasing reservations that were promoted to conservative debit (DELETE now filters `is_conservative_debit = FALSE` to preserve promoted rows) - Pass `Some(0)` output tokens for embedding requests instead of `None` so budget projection can proceed using input-only cost without triggering `BudgetRequiresMaxTokens` Major fixes: - Strip spend/limit amounts from MonthlyBudgetExceeded HTTP response to prevent leaking operator budget config to untrusted deployment-token callers; emit them as structured tracing fields instead - Upgrade ai_gateway_cost_reservations primary key from single-column (request_id) to composite (request_id, scope) so multi-scope budgets don't hit a PK violation on the second reservation insert - Add three composite indexes on ai_usage_logs new attribution columns (billing_period, project_id), (billing_period, environment_id), (billing_period, deployment_token_id) to support budget roll-up queries - Add comprehensive handler-level auth tests verifying that non-admin callers (even with AiGatewayWrite) are rejected by require_operator, and that callers without AiGatewayWrite are rejected by permission_guard! Minor fixes: - Add `.if_not_exists()` to all three pre-existing create_index calls in the governance migration to make the migration idempotent - Add CHECK constraint `reserved_microcents >= 0` on the reservations table to enforce the invariant at the database level - Add regression test documenting that stream_options.include_usage is unconditionally forced to true (overwriting caller-supplied false)
Reapplies the AI gateway cost-governance feature (scoped model allowlists, RPM limits, monthly cost budgets enforced via PostgreSQL advisory locks and atomic cost reservations) against the current temps-ai-gateway crate, which was substantially refactored since the feature was originally built.
Adds `temps ai governance list/set/unset` for the plugin-only AI gateway governance routes (model allowlists, RPM limits, monthly spend caps per scope). Hand-written against the raw client per CLAUDE.md's plugin-route pattern since these routes live in temps-ai-gateway and must stay out of the committed openapi.json.
Adds the ai governance list/set/unset subcommands to the temps-cli skill's command reference and routing table, and points to them from the temps-best-practices skill's observability guidance.
Adds a Governance tab (instance/project/environment scope selector, model-allowlist tri-state picker, RPM and monthly-budget fields, and a spend-vs-budget gauge) so operators can configure the AI gateway cost governance API that previously had no UI. Wired into the AI Gateway sidebar/command palette alongside Usage/Activity/Setup, always visible and gated on a clear "insufficient permissions" state rather than a blank panel.
…endpoints /ai/usage/summary and /ai/usage/by-provider accepted project_id and environment_id on UsageFilter but never populated them from query params, so scope-filtered spend was silently unscoped. Needed by the new governance UI to show spend for a project/environment scope rather than always falling back to instance-wide totals.
…cope Now that /ai/usage/summary accepts project_id/environment_id, pass the governance tab's selected scope through instead of always showing instance-wide spend with a "not available" caveat.
Addresses a review-pr pass on PR #543: - migration: ai_gateway_rate_events was missing the request_id column that check_rates_and_record inserts into, which would break RPM enforcement on every fresh deployment (500 on first rate-limited scope). No prior test exercised the INSERT, so nothing caught it. - security: governance policies are operator-only by design (they set limits for a whole scope, not just the caller's own usage), but AiGatewayWrite alone is held by Role::User too. Add an explicit admin/platform-admin check to all three handlers so a regular user can no longer tamper with instance-wide or another project's limits. - correctness: a negative RPM/budget value returned 500 instead of 400 since InvalidGovernanceConfig mapped to INTERNAL_SERVER_ERROR. - add integration tests for RPM rejection, budget rejection, and expired-reservation-to-conservative-debit conversion -- the three enforcement paths that had no test coverage (the RPM test is what would have caught the request_id bug above). - add unit tests for the CLI's governance helper functions and for UsageFilter's project_id/environment_id parameter binding. - tidy the rate-event cleanup window (comment said one minute, query used one hour).
isInsufficientPermissionsError only matched permission_guard!'s "Insufficient Permissions" title, missing the operator-only gate's distinct "Operator Privileges Required" title (governance.rs's require_operator). Both share the same RFC 7807 problem `type` URI, so match on that instead — the realistic non-admin caller now sees the onboarding "insufficient access" state instead of a generic error.
Blocking fixes: - Remove spurious `::uuid` cast in DELETE on ai_gateway_cost_reservations (request_id is varchar(64), not uuid; cast caused runtime type error) - Skip releasing reservations that were promoted to conservative debit (DELETE now filters `is_conservative_debit = FALSE` to preserve promoted rows) - Pass `Some(0)` output tokens for embedding requests instead of `None` so budget projection can proceed using input-only cost without triggering `BudgetRequiresMaxTokens` Major fixes: - Strip spend/limit amounts from MonthlyBudgetExceeded HTTP response to prevent leaking operator budget config to untrusted deployment-token callers; emit them as structured tracing fields instead - Upgrade ai_gateway_cost_reservations primary key from single-column (request_id) to composite (request_id, scope) so multi-scope budgets don't hit a PK violation on the second reservation insert - Add three composite indexes on ai_usage_logs new attribution columns (billing_period, project_id), (billing_period, environment_id), (billing_period, deployment_token_id) to support budget roll-up queries - Add comprehensive handler-level auth tests verifying that non-admin callers (even with AiGatewayWrite) are rejected by require_operator, and that callers without AiGatewayWrite are rejected by permission_guard! Minor fixes: - Add `.if_not_exists()` to all three pre-existing create_index calls in the governance migration to make the migration idempotent - Add CHECK constraint `reserved_microcents >= 0` on the reservations table to enforce the invariant at the database level - Add regression test documenting that stream_options.include_usage is unconditionally forced to true (overwriting caller-supplied false)
Rebasing feat/ai-gateway-project-governance onto main picked up several unrelated endpoints (image retention, deployment media, project health summary fields) that had already regenerated the same generated-client files, causing merge conflicts. Per CLAUDE.md, generated-client conflicts are never hand-merged: resolved by taking the rebase-target side, then regenerating in full against a server built off the merged source. tsc --noEmit is clean.
dviejokfs
force-pushed
the
feat/ai-gateway-project-governance
branch
from
August 17, 2026 16:20
76158dd to
4632af5
Compare
skills/temps/references/commands/ai.md is generated from skills/temps-cli/references/COMMANDS.md; the governance docs added by this PR updated the source but never regenerated the derived file, failing the CI drift check (skills/temps/scripts/generate_command_references.py diff gate). Also pins the example's @temps-sdk/cli reference to @0.1.33, required by validate_skill.py's PINNED_CLI check.
Two disclosure gaps a dedicated security-auditor pass found in the same class as the already-fixed MonthlyBudgetExceeded leak: - RateLimitExceeded's HTTP body included the exact operator-configured limit_per_minute, letting a deployment-token caller learn the RPM cap by triggering it. Stripped from the response; logged server-side via warn!. The already-computed retry_after_seconds, previously dropped silently, is now surfaced as a Retry-After header instead. - InvalidGovernanceConfig's Display embeds the offending field name and value from a corrupted/invalid stored config row, and was forwarded verbatim via error.to_string() into a 500 response. Replaced with a generic message; details logged server-side via error!. Also files (not fixed here, tracked as #705): the AI gateway usage endpoints (/ai/usage/summary, /ai/usage/by-provider) have no ownership scoping at all, predating this PR — any AiGatewayRead holder (the default for Role::User) can already read any project's usage. Out of scope for this PR per the same triage pattern as the existing proxy-logs IDOR #403.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bunx @temps-sdk/cli ai governance list/set/unsetfor the same, hand-written against the plugin route per this repo's CLI-parity conventions (governance is served by thetemps-ai-gatewayplugin, so it's intentionally excluded from the CLI's committed OpenAPI spec)ai governancedocumented in thetemps-cliskill's command reference and routing table, with a pointer fromtemps-best-practicesPolicies apply only from trusted deployment-token attribution. BYOK requests still obey allowlists and RPM limits but do not consume operator-funded monthly budgets. Internal control-plane AI calls remain outside this deployment-token HTTP boundary. Governance policies are operator-only: setting/viewing limits for any scope (including a project you own) requires an instance administrator, since a policy affects every request in that scope, not just the caller's own usage.
Closes #376
Note on this branch's history
This branch was originally built against an older commit and has since been reapplied against current
main—temps-ai-gateway'sgateway.rs,pricing.rs,providers.rs, andusage.rswere substantially refactored in the interim, so the governance logic was hand-reintegrated rather than merged, with a fresh migration timestamped after main's newer migrations. The original 2-commit history is preserved for reference onbackup/ai-gateway-project-governance-origin the working tree that produced this push.Review pass and fixes applied
A full
review-prpass (Rust standards, security-auditor, migration safety, frontend/SDK, test coverage, evidence) found two blocking issues, since fixed:ai_gateway_rate_eventswas missing therequest_idcolumn thatcheck_rates_and_recordinserts into — RPM enforcement would 500 on every fresh deployment as soon as any scope had an RPM limit configured. No prior test exercised that INSERT, so it went uncaught. Fixed the migration and added an integration test (rpm_limit_rejects_after_threshold) that exercises the exact path.AiGatewayWrite, whichRole::Useralso holds for their own projects' day-to-day AI usage — meaning a regular user could tamper with instance-wide limits or another project's governance policy by naming its scope. Added an explicit operator-only check (admin/platform-admin) tolist/upsert/delete, matching the "operator-only" design intent stated above.Also fixed: a negative RPM/budget value returned
500instead of the documented400(status-code mapping bug), a stale rate-event cleanup window (comment said 1 minute, query used 1 hour), and closed three test-coverage gaps that had zero assertions on the actual enforcement behavior (RPM rejection, budget rejection, expired-reservation-to-conservative-debit conversion) plus CLI helper unit tests and aproject_id/environment_idfilter-binding test.Out of scope, flagged as a separate follow-up: the security pass also found BYOK's
x-provider-base-urlheader has no SSRF validation (private/internal IP ranges aren't rejected) — this predates this PR (present onmainbefore the reapply) and isn't part of this diff; worth a dedicated fix.Evidence
AI governance lifecycle and concurrency (real PostgreSQL/TimescaleDB)
cargo test -p temps-ai-gateway --test governance_integration -- --nocapture(includes the 3 new tests for RPM rejection, budget rejection, and expired-reservation conversion)
Gateway behavior and regression coverage
cargo test --lib -p temps-ai-gatewayMigration applied / rollback (real TimescaleDB)
cargo test -p temps-migrations --test migration_tests test_migration_down -- --exact --nocaptureRepository gates
Frontend
Live-verified in-browser against a local dev instance: logged in, opened
/ai-gateway/governance, saved RPM=42 + $12.50 monthly budget for the instance scope (PUT /api/ai/governance/instance-> 200), reloaded and confirmed persistence, removed limits (DELETE-> 204) and confirmed revert to unlimited, exercised the project/environment scope selector and the model-allowlist tri-state multi-select. No console errors. Scoped spend display verified against theproject_id/environment_idfilters newly wired into/ai/usage/summary.CLI
Live-verified against a local dev instance:
ai governance set instance --rpm 100 --monthly-budget 50.00->ai governance list(table shows RPM 100 / $50.00) ->ai governance unset instance --yes->ai governance list(empty again). Also verified--models <list>,--models none(block-all), and--jsonoutput.Rust review, security re-review, migration/test review: APPROVE (post-fix, this session).
Second review-pr pass and fixes (this session)
A fresh
/review-prpass (Rust standards, security-auditor, migration safety, frontend/SDK, test coverage, evidence — all run independently, with findings verified against the actual diff before being trusted) found 3 blocking, 4 major, and 3 minor issues that survived the first review-pr pass above. All are fixed inc17c43722and76158dd44:Blocking:
usage_service.rs's reservation-cleanup DELETE cast its bound param to::uuidagainst avarchar(64)column — Postgres has no implicit varchar↔uuid comparison, so this raised a runtime error on every successful governed request, rolling back the accompanying usage-log insert. Cast removed.release_cost_reservationdeleted a reservation row unconditionally byrequest_id, with no guard against rows already promoted to a durable "conservative debit" by the expired-reservation cleanup job — a slow request that later failed upstream could silently erase spend the mechanism was built to make durable. Added anAND is_conservative_debit = FALSEguard.max_output_tokens = Noneinto the budget-projection check, which unconditionally requires aSomevalue once any budget scope applies — the instant an operator configured any budget covering embedding traffic, every non-BYOK embedding request 400'd. Now passesSome(0)(embeddings have no output tokens by definition), letting projection proceed on input cost alone.Major:
MonthlyBudgetExceeded's HTTP-facing message included exactspent_microcents/limit_microcents, leaking the operator's configured budget and running spend to any deployment-token caller probing near the ceiling. Amounts stripped from the response; moved to a structuredwarn!log instead.ai_gateway_cost_reservationshad a single-column PK onrequest_id, but the code inserts one row per applicable budget-limited scope sharing the samerequest_id— any deployment with two simultaneously active budget scopes (e.g. instance + project) hit a PK violation on the second insert. Migration changed to a composite(request_id, scope)primary key.ai_usage_logs's newproject_id/environment_id/deployment_token_idcolumns despite them being filtered in the budget roll-up query — added(billing_period, *)composite indexes matching the query shape.require_operator) and theAiGatewayWritepermission gate had no HTTP-layer test proving they actually reject a non-admin caller — added handler-level tests for all three governance endpoints, plus a frontend fix so the UI's "insufficient permissions" onboarding state recognizesrequire_operator's distinct problem title (previously only matchedpermission_guard!'s title; both now matched via the shared RFC 7807 problemtypeURI).Minor: added
.if_not_exists()to the migration's index creations, added aCHECK (reserved_microcents >= 0)constraint, and added a regression test locking in thestream_options.include_usageoverwrite-caller-value behavior (intentional, to prevent billing bypass).Re-verified evidence after the fix (this session, real Docker/Postgres/TimescaleDB, not asserted):
cargo test --lib -p temps-ai-gateway$(command -v cargo) clippy -p temps-ai-gateway -p temps-migrations --lib --tests -- -D warningscargo test -p temps-ai-gateway --test governance_integration -- --nocapturecargo test -p temps-migrations --test migration_tests test_migration_down -- --exact --nocaptureKnown follow-ups, not blocking: no test exercises the PostgreSQL advisory-lock claim under real concurrency (two simultaneous
check_requestcalls against the same scope) — the cross-node correctness invariant is currently proven only by code inspection. Separately,lock_scopesalways includes the"instance"scope in its lock set, so any instance-wide RPM/budget policy serializes all governed traffic platform-wide on one advisory lock for the full check-and-insert transaction — worth a load test before recommending instance-wide governance as a default. Both flagged for a fast-follow, not this PR.Rebase onto main + SDK regen (this session, commit
4632af563)mainhad moved 11 commits since this branch's base, causing real merge conflicts (not stale-status noise): a migration registration conflict incrates/temps-migrations/src/migration/mod.rs(main addedm20260816_000001_add_image_retention_hours, sharing this PR's date/sequence stamp — different table, no schema collision, just an ordering conflict) and generated-SDK conflicts inweb/src/api/client/{sdk.gen.ts,index.ts,@tanstack/react-query.gen.ts}(main added unrelated endpoints — image retention, deployment media, project health summary — that regenerated the same files).Resolved per this repo's rule for generated-client conflicts (never hand-merge): rebased, registered the governance migration last in
Migrator::migrations()with a comment explaining the shared-stamp ordering, took one side to clear the generated-file conflicts, then regenerated the SDK in full against atemps serveinstance built off the merged source (minted a temporary local admin API key for the auth-gated/api/api-docs/openapi.jsonendpoint, regenerated, immediately revoked the key).Re-verified after the rebase (real Docker/Postgres/TimescaleDB, this session):
cargo check --lib # workspace-wide, not just the two changed cratescargo test --lib -p temps-ai-gateway -p temps-migrations$(command -v cargo) clippy -p temps-ai-gateway -p temps-migrations --lib --tests -- -D warningscargo test -p temps-ai-gateway --test governance_integration -- --nocapturecargo test -p temps-migrations --test migration_tests test_migration_down -- --exact --nocapturePR mergeability confirmed clean post-rebase (
gh pr view 543 --json mergeable→MERGEABLE, wasCONFLICTINGbefore this rebase).Dedicated security-auditor pass + fixes (this session)
A dedicated, independent security review (beyond the standard review-pr gate) found two more disclosure issues in the same class as the already-fixed budget-amount leak, both fixed in
f0daf9dc9:RateLimitExceeded— the 429 response body included the exact operator-configuredlimit_per_minute, letting any deployment-token caller learn the RPM cap by deliberately triggering it. Stripped from the response; the value is now only logged server-side. The already-computedretry_after_seconds(previously silently dropped) is now surfaced properly as aRetry-Afterheader.InvalidGovernanceConfig— itsDisplayembeds the offending field name and value from a corrupted/invalid stored governance config row (reachable via DB corruption or a direct manual write, since the write path itself validates non-negativity), and was forwarded verbatim into a 500 response body. Replaced with a generic message; details logged server-side.Both fixes have new regression tests (
test_rate_limit_exceeded_response_does_not_leak_configured_limit,test_invalid_governance_config_response_does_not_leak_field_or_value) asserting the response body does not contain the sensitive values.Also confirmed still correctly fixed (re-verified independently, not just re-asserted): the operator-only gate on all three governance handlers, the embeddings budget-DoS fix, and that the BYOK
x-provider-base-urlSSRF gap remains genuinely untouched by this diff.Filed as a separate tracked issue, not fixed in this PR: gotempsh/temps#705 —
/ai/usage/summaryand/ai/usage/by-providerhave no ownership scoping at all, predating this PR (anyAiGatewayReadholder — the default forRole::User— can already read any project's usage; this PR only added aproject_id/environment_idfilter on top of an already-unscoped query, making it easier to target but not introducing the underlying gap). Triaged the same way this repo already handles the analogous proxy-logs IDOR (#403): tracked separately rather than scope-creeping this PR.Re-verified after these fixes (real Docker/Postgres, this session):
cargo test --lib -p temps-ai-gateway$(command -v cargo) clippy -p temps-ai-gateway --lib --tests -- -D warningscargo test -p temps-ai-gateway --test governance_integration -- --nocaptureCI fix: skill docs generation drift (this session)
The initial push after the rebase failed CI's
Scan skills/tempsjob — not a malicious-content flag, but a generated-file drift check:skills/temps/references/commands/ai.mdis generated fromskills/temps-cli/references/COMMANDS.mdviaskills/temps/scripts/generate_command_references.py, and this PR's earlier docs commit updated the source but never regenerated the derived file. Also caught by the same job: the new command's example used an unpinned@temps-sdk/clireference, failingvalidate_skill.py's pinned-version check. Both fixed in1ba0fdec8— pinned the example to@temps-sdk/cli@0.1.33and regeneratedai.md; local drift check and the skill's own unit tests now pass clean.