Skip to content

feat: add ai_providers table, queries, dbauthz, audit, RBAC#24892

Merged
dannykopping merged 15 commits into
mainfrom
dk/aibridge-providers-db
May 14, 2026
Merged

feat: add ai_providers table, queries, dbauthz, audit, RBAC#24892
dannykopping merged 15 commits into
mainfrom
dk/aibridge-providers-db

Conversation

@dannykopping

@dannykopping dannykopping commented May 1, 2026

Copy link
Copy Markdown
Contributor

Disclaimer: implemented by a Coder Agent using Claude Opus 4.7 and heavily augmented by me.

Part of the implementation of RFC: Common AI Provider Configs.

This PR adds the database foundation for runtime AI Bridge provider management. Provider API keys live in a sibling ai_provider_keys table so a provider can carry zero or many keys (Bedrock providers have none; OpenAI and Anthropic have one or more for failover).

What's in this PR

  • Schema: ai_providers table (type/name/display_name/enabled/deleted/base_url/settings/settings_key_id) and ai_provider_keys table (provider_id FK with ON DELETE CASCADE, api_key/api_key_key_id). New ai_provider_type enum (openai, anthropic). display_name is nullable; callers fall back to name.
  • Queries: GetAIProviderByID, GetAIProviderByName, GetAIProviders (optional enabled filter, soft-deleted rows always excluded), InsertAIProvider, UpdateAIProvider, UpdateAIProviderSettings, DeleteAIProviderByID; plus GetAIProviderKeyByID, GetAIProviderKeysByProviderID, GetAIProviderKeys, InsertAIProviderKey, UpdateEncryptedAIProviderKey, DeleteAIProviderKey.
  • dbauthz: gates every query on ResourceAIProvider.
  • audit: tracks Create/Write/Delete on AIProvider and Create/Delete on AIProviderKey. settings and api_key are marked ActionSecret so they are never leaked in audit diffs. Adds ai_provider/ai_provider_key to resource_type and ai_provider:* scopes to api_key_scope.
  • rbac: adds the ai_provider resource. Owner role gets full access via allPermsExcept default; no other built-in role gets any action on this resource.
  • dbgen: AIProvider and AIProviderKey factories for tests.

dbcrypt-at-rest support for settings and api_key lives in #25326 upstack of this PR.

Design notes

  • Soft-deleted provider names are permanently reserved (the row is preserved for FK and audit history).
  • settings is stored as a single JSON blob (not field-by-field). For Bedrock providers it carries bedrock_region, bedrock_model, bedrock_small_fast_model, and bedrock_access_key_secret. The Bedrock access key itself reuses the existing api_key column on ai_provider_keys.
  • Reserved provider names (providers, proxy) will be enforced in the handler layer.

@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown

Docs preview

📖 View docs preview for docs/reference/api/members.md

Copy link
Copy Markdown
Contributor Author

/coder-agents-review

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First-pass review (Netero). 1 P2, 1 P3.

The database foundation is well-structured: naming conventions are correct, dbauthz wrappers follow the standard pattern, dbcrypt overrides are consistent, and tests are non-vacuous with proper encrypted-row verification. The decryptAIProvider shared helper keeps the dbcrypt layer clean.

One defect needs fixing before the full review panel runs: SoftDeleteAIProviderByID returns an AiProvider struct but has no dbcrypt override, so callers get encrypted ciphertext in api_key and settings. The missing test coverage (P3) is straightforward to add alongside.

This is a first-pass review only. These are mechanical findings from Netero; the full review panel has not yet reviewed this PR. The panel will review after these findings are addressed.

🤖 This review was automatically generated with Coder Agents.

Comment thread enterprise/dbcrypt/dbcrypt.go Outdated
Comment thread enterprise/dbcrypt/dbcrypt_internal_test.go
@dannykopping dannykopping changed the title feat(coderd): add ai_providers table, queries, dbauthz, dbcrypt, audit, RBAC feat: add ai_providers table, queries, dbauthz, dbcrypt, audit, RBAC May 1, 2026
@dannykopping dannykopping force-pushed the dk/aibridge-providers-db branch 4 times, most recently from 80186d4 to 74a413d Compare May 13, 2026 18:44
Comment thread coderd/database/queries/ai_providers.sql Outdated
Comment thread coderd/database/queries/ai_providers.sql Outdated
Comment thread coderd/database/migrations/000494_ai_providers.up.sql Outdated
TRUE,
FALSE,
'https://bedrock-runtime.us-west-2.amazonaws.com/',
'{"bedrock_region":"us-west-2","bedrock_model":"global.anthropic.claude-sonnet-4-5-20250929-v1:0","bedrock_access_key":"fixture-bedrock-access-key","bedrock_access_key_secret":"fixture-bedrock-access-key-secret"}'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This format is not final, but irrelevant for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit [DEREM-5] Fixture filename uses old aibridge_providers naming. Migration is 000494_ai_providers.up.sql. Project convention: fixture filename matches the migration name prefix.

(Netero)

🤖

Comment thread coderd/database/queries/ai_provider_keys.sql Outdated
Comment thread coderd/database/queries/ai_providers.sql Outdated
Comment thread coderd/audit/request.go Outdated
Comment thread coderd/database/dbgen/dbgen.go Outdated
dannykopping and others added 9 commits May 14, 2026 12:49
Adds the database foundation for runtime AI Bridge provider management
described in the 'Common AI Provider Configs' RFC.

- New ai_providers table with type/name/display_name/enabled/deleted,
  base_url, api_key, settings (encrypted blobs via dbcrypt).
- New ai_provider_type enum (openai, anthropic).
- Queries: GetAIProviderByID, GetAIProviderByName,
  GetAIProviderByNameIncludeDeleted, GetAIProviders,
  GetEnabledAIProviders, InsertAIProvider, UpdateAIProvider,
  SoftDeleteAIProviderByID.
- dbauthz: gates all queries on ResourceAibridgeProvider.
- dbcrypt: encrypts api_key and the settings JSON blob at rest with
  separate dbcrypt key references.
- audit: tracks Create/Write/Delete on AiProvider with secret fields
  marked appropriately.
- rbac: introduces aibridge_provider resource (owner-only via
  allPermsExcept default).
- dbgen: AIProvider factory for tests.

Refs RFC: https://www.notion.so/coderhq/RFC-Common-AI-Provider-Configs-34bd579be59280ed958feffb82024797
Adds GetAIProvidersForRotation and UpdateAIProviderEncryptedColumns
queries (and their dbauthz/dbcrypt wrappers) so the Rotate, Decrypt,
and Delete utilities walk the ai_providers table alongside every other
encrypted table.

Previously, key revocation failed with a foreign-key violation when
ai_providers rows still referenced the old key's digest:

    error: rotate ciphers: revoke key: pq: update or delete on table
    "dbcrypt_keys" violates foreign key constraint
    "ai_providers_api_key_key_id_fkey" on table "ai_providers"

Soft-deleted rows are also walked because their FK references persist
until the row itself is hard-deleted.
…erated artifacts

The TestRolePermissions/aibridge_provider-AllActions check failed
because no test case covered the new aibridge_provider resource. Add
an owner-only CRUD case alongside the existing AIBridgeInterception
entries.

Also refreshes the generated artifacts that the previous "make gen"
run missed because of stale build timestamps:

- coderd/apidoc/docs.go and swagger.json
- codersdk/apikey_scopes_gen.go
- docs/admin/security/audit-logs.md
- docs/reference/api/schemas.md
- site/src/api/typesGenerated.ts

These all needed to pick up the aibridge_provider resource and the
AiProvider audit table entry that PR 24892 introduced.
Per the updated RFC, ai_providers no longer carries the api_key column.
Instead a new ai_provider_keys table holds 0..N keys per provider:

- Bedrock providers have 0 keys; their AWS credentials live in
  ai_providers.settings (bedrock_access_key + bedrock_access_key_secret).
- OpenAI/Anthropic providers have 1..N keys to support future failover
  and rotation.

Changes:

- Migration 000483: drop api_key/api_key_key_id from ai_providers; add
  ai_provider_keys(id, provider_id FK ON DELETE CASCADE, api_key,
  api_key_key_id, timestamps).
- Queries split: aibridgeproviders.sql for the provider row,
  aibridgeproviderkeys.sql for the keys.
- dbauthz: new key queries gated on ResourceAibridgeProvider.
- dbcrypt: provider rotation walks only settings now; new key rotation
  walks ai_provider_keys including those whose provider is soft-deleted.
- audit: new AiProviderKey row tracks Create/Delete with api_key marked
  ActionSecret.
- dbgen: new AIProviderKey factory.
- Tests: TestAIProviderKeys (round-trip), TestRotateAIProviderKeys
  (rotation including soft-deleted parent provider), and the existing
  AIProvider tests rewritten without the api_key field.
- make gen refreshes all derived artifacts.
Renames SoftDeleteAIProviderByID -> DeleteAIProviderByID,
UpdateAIProviderEncryptedColumns -> UpdateAIProviderSettings,
GetAIProviderKeysForRotation -> GetAIProviderKeys, and
UpdateAIProviderKeyEncryptedColumns -> UpdateEncryptedAIProviderKey.
Drops GetAIProviderByNameIncludeDeleted, GetEnabledAIProviders, and
GetAIProvidersForRotation; GetAIProviders now accepts optional
enabled/deleted filters that the dbcrypt rotation walks with both
flags NULL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renames the new RBAC resource so it can be used by all AI-related
features, not just AI Bridge. Sets Name: "AIProvider" on the policy
entry so the generated Resource* constants use AI; adds sqlc.yaml
renames so the database struct types follow suit. Generator-derived
ScopeAi*/APIKeyScopeAi* constants are left as-is.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ring

Re-running make gen revealed three issues left by earlier commits on this
branch. The apidoc, swagger, audit-logs, RBAC, and codersdk type artifacts
were stale relative to the @Router/@basepath, RBAC, and codersdk changes
that landed in 74a413d and the commits leading up to it. The AIProvider
rename missed the AuditActionMap keys, which are looked up case-sensitively
by the audit doc generator and so rendered the AI rows with empty actions.
dbgen.AIProviderKey did not set the CreatedAt sql.NullTime field that
InsertAIProviderKeyParams gained, tripping exhaustruct.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renames query files, migration files, and Go symbols to drop the
"aibridge" prefix for the generic AI provider concept. Adds a missing
dbcrypt override on DeleteAIProviderByID so soft-deleted rows return
decrypted settings, matching sibling methods.

Env vars (CODER_AIBRIDGE_PROVIDER_<N>_<KEY>) and aibridgeproxyd symbols
are intentionally left unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sqlc orders queries.sql.go by source filename; renaming
aibridgeproviders.sql / aibridgeproviderkeys.sql to ai_providers.sql /
ai_provider_keys.sql shifts those query blocks ahead of aibridge.sql in
the generated output. Pure reorder: 1673 lines moved, no content
changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dannykopping dannykopping force-pushed the dk/aibridge-providers-db branch from 1a61871 to 692e9c3 Compare May 14, 2026 10:57
Comment thread coderd/database/queries/ai_providers.sql Outdated

Copy link
Copy Markdown
Contributor Author

/coder-agents-review

@dannykopping dannykopping changed the title feat: add ai_providers table, queries, dbauthz, dbcrypt, audit, RBAC feat: add ai_providers table, queries, dbauthz, audit, RBAC May 14, 2026
@dannykopping dannykopping marked this pull request as ready for review May 14, 2026 11:41
@dannykopping dannykopping requested a review from Emyrk as a code owner May 14, 2026 11:41
@dannykopping dannykopping requested a review from johnstcn May 14, 2026 11:42

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second-pass review (Netero). R1 findings addressed. 1 P1, 1 P3, 1 Nit, 1 Note new.

DEREM-1 (P2) and DEREM-2 (P3) are resolved: SoftDeleteAIProviderByID was converted to :exec (no return value), and the untested methods were removed entirely. Clean fixes.

The restructuring between rounds exposed a significant gap: the PR title and description claim dbcrypt wiring, but zero lines in enterprise/dbcrypt/ were changed. The schema has FK constraints from ai_providers.settings_key_id and ai_provider_keys.api_key_key_id to dbcrypt_keys(active_key_digest), so key rotation will fail on revocation once encryption is enabled.

This is a first-pass review only. These are mechanical findings from Netero; the full review panel has not yet reviewed this PR. The panel will review after these findings are addressed.

🤖 This review was automatically generated with Coder Agents.

Comment thread coderd/database/dbauthz/dbauthz.go
Comment thread codersdk/audit.go Outdated
Comment thread coderd/database/dbauthz/dbauthz.go

@johnstcn johnstcn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing the dbcrypt methods. I'd also recommend some additional dbcrypt tests specifically for those queries.

Comment thread coderd/rbac/roles_test.go Outdated
Comment thread coderd/database/queries/ai_provider_keys.sql Outdated
Comment thread coderd/database/migrations/000495_ai_providers.up.sql Outdated
@dannykopping

dannykopping commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

Missing the dbcrypt methods. I'd also recommend some additional dbcrypt tests specifically for those queries.

@johnstcn I mentioned in the description; the dbcrypt changes have been split out into a separate PR upstack

- Remove GetAIProviderByNameIncludeDeleted; soft-deleted lookups can be
  added back where actually needed.
- Replace the 3-way sqlc.narg-based deleted/enabled filters on
  GetAIProviders with plain bool include_deleted/include_disabled
  flags that default to false, so the common path of "live + enabled
  rows only" needs no params.
- Convert DeleteAIProviderByID and DeleteAIProviderKey to :exec since
  callers do not need the affected row back.
- Require explicit created_at/updated_at on InsertAIProviderKey instead
  of falling back to NOW() inside the query.
The column is now nullable text with no default; callers fall back to
name when display_name is NULL. The dbgen test factory mirrors this by
seeding display_name from the provider name when none is supplied.
The previous 8-character prefix was lossy. Surface the full provider
UUID so audit consumers can correlate the key back to its provider
without guessing.
…ncryptedAIProviderSettings

Mirrors UpdateEncryptedAIProviderKey on ai_provider_keys; the name now
makes clear that the query targets the encrypted settings columns and
is intended for the dbcrypt rotation utility.
Signed-off-by: Danny Kopping <danny@coder.com>
@dannykopping dannykopping force-pushed the dk/aibridge-providers-db branch from 22dabfa to 62b664c Compare May 14, 2026 13:20
- Allow soft-deleted provider names to be reused by replacing the
  global UNIQUE constraint on ai_providers.name with a partial unique
  index that only applies to non-deleted rows.
- Fix FriendlyString to return "ai provider" / "ai provider key"
  instead of "ai bridge provider".
- Rename fixture file 000495_aibridge_providers.up.sql to
  000495_ai_providers.up.sql to match its migration.
- Update roles_test.go comment to say AI providers are
  deployment-wide, not workspace-wide.
- Update GetAIProviderKeysByProviderID comment to say the oldest
  key (not the first key) is used.
- Trim verbose GetAIProviders comment.
- Update dbauthz test for GetAIProviders to use the new
  GetAIProvidersParams struct.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dannykopping dannykopping requested a review from johnstcn May 14, 2026 13:40
@dannykopping dannykopping merged commit 841b777 into main May 14, 2026
47 of 49 checks passed
@dannykopping dannykopping deleted the dk/aibridge-providers-db branch May 14, 2026 14:10
@github-actions github-actions Bot locked and limited conversation to collaborators May 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants