Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

chore: surface key add/remove/keep counts in audit log #25484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dannykopping merged 2 commits into from
May 20, 2026
Merged

chore: surface key add/remove/keep counts in audit log #25484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21977c3
fix(coderd/ai_providers): surface key add/remove/keep counts in audit…
dannykopping May 19, 2026
286f036
refactor(coderd/ai_providers): unify key-ops audit shape and surface …
dannykopping May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions 61 coderd/ai_providers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"golang.org/x/xerrors"

"cdr.dev/slog/v3"
aibridgeutils "github.com/coder/coder/v2/aibridge/utils"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
Expand Down Expand Up @@ -255,14 +256,19 @@ func (api *API) aiProvidersCreate(rw http.ResponseWriter, r *http.Request) {
// @Success 200 {object} codersdk.AIProvider
// @Router /api/v2/ai/providers/{idOrName} [patch]
func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) {
// keyOpsAudit attaches per-key add/remove/keep counts to the audit
// entry. Keys live in a separate table, so a key-only PATCH would
// otherwise produce an empty diff and hide rotation from the log.
keyOpsAudit := &aiProviderKeyOpsAudit{}
var (
ctx = r.Context()
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.AIProvider](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionWrite,
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionWrite,
AdditionalFields: keyOpsAudit,
})
)
defer commitAudit()
Expand Down Expand Up @@ -347,10 +353,12 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) {
aReq.New = updated

if req.APIKeys != nil {
keys, err = applyAIProviderKeyOps(ctx, tx, updated.ID, *req.APIKeys)
var ops aiProviderKeyOpsAudit
keys, ops, err = applyAIProviderKeyOps(ctx, tx, updated.ID, *req.APIKeys)
if err != nil {
return err
}
*keyOpsAudit = ops
return nil
}

Expand Down Expand Up @@ -547,16 +555,37 @@ func insertAIProviderKeys(ctx context.Context, tx database.Store, providerID uui
return out, nil
}

// aiProviderKeyOpsAudit is serialized into the audit entry's
// additional_fields. Surfacing the per-key ID and masked secret for
// adds and removes gives operators a precise record of which keys
// rotated on a PATCH whose top-level diff would otherwise look empty.
// Kept is a count: a steady-state rotation commonly retains many keys,
// and per-entry detail there is noise.
type aiProviderKeyOpsAudit struct {
Added []aiProviderKeyOp `json:"added"`
Removed []aiProviderKeyOp `json:"removed"`
Kept int `json:"kept"`
}

// aiProviderKeyOp identifies a single key affected by a PATCH. Masked
// is the one-way rendering produced by aibridgeutils.MaskSecret, so
// plaintext never lands in the audit log.
type aiProviderKeyOp struct {
ID uuid.UUID `json:"id"`
Masked string `json:"masked"`
}

// applyAIProviderKeyOps reconciles a provider's keys against the
// supplied mutation list inside a transaction: kept-by-ID rows stay,
// rows whose ID is absent from the list are deleted, and entries
// carrying a plaintext APIKey are inserted as new rows. Caller is
// responsible for prior validation (XOR per entry, no duplicate IDs).
// IDs that do not belong to this provider return errAIProviderKeyUnknown.
func applyAIProviderKeyOps(ctx context.Context, tx database.Store, providerID uuid.UUID, muts []codersdk.AIProviderKeyMutation) ([]database.AIProviderKey, error) {
func applyAIProviderKeyOps(ctx context.Context, tx database.Store, providerID uuid.UUID, muts []codersdk.AIProviderKeyMutation) ([]database.AIProviderKey, aiProviderKeyOpsAudit, error) {
var ops aiProviderKeyOpsAudit
existing, err := tx.GetAIProviderKeysByProviderID(ctx, providerID)
if err != nil {
return nil, xerrors.Errorf("load existing ai provider keys: %w", err)
return nil, ops, xerrors.Errorf("load existing ai provider keys: %w", err)
}
existingByID := make(map[uuid.UUID]struct{}, len(existing))
for _, k := range existing {
Expand All @@ -569,7 +598,7 @@ func applyAIProviderKeyOps(ctx context.Context, tx database.Store, providerID uu
switch {
case m.ID != nil:
if _, ok := existingByID[*m.ID]; !ok {
return nil, xerrors.Errorf("%w: %s", errAIProviderKeyUnknown, *m.ID)
return nil, ops, xerrors.Errorf("%w: %s", errAIProviderKeyUnknown, *m.ID)
}
keep[*m.ID] = struct{}{}
case m.APIKey != nil:
Expand All @@ -582,19 +611,25 @@ func applyAIProviderKeyOps(ctx context.Context, tx database.Store, providerID uu
continue
}
if err := tx.DeleteAIProviderKey(ctx, k.ID); err != nil {
return nil, xerrors.Errorf("delete ai provider key %s: %w", k.ID, err)
return nil, ops, xerrors.Errorf("delete ai provider key %s: %w", k.ID, err)
}
ops.Removed = append(ops.Removed, aiProviderKeyOp{ID: k.ID, Masked: aibridgeutils.MaskSecret(k.APIKey)})
}

if _, err := insertAIProviderKeys(ctx, tx, providerID, inserts); err != nil {
return nil, err
added, err := insertAIProviderKeys(ctx, tx, providerID, inserts)
if err != nil {
return nil, ops, err
}
for _, k := range added {
ops.Added = append(ops.Added, aiProviderKeyOp{ID: k.ID, Masked: aibridgeutils.MaskSecret(k.APIKey)})
}
ops.Kept = len(keep)

out, err := tx.GetAIProviderKeysByProviderID(ctx, providerID)
if err != nil {
return nil, xerrors.Errorf("reload ai provider keys: %w", err)
return nil, ops, xerrors.Errorf("reload ai provider keys: %w", err)
}
return out, nil
return out, ops, nil
}

// errAIProviderKeyUnknown is the sentinel returned by
Expand Down
69 changes: 69 additions & 0 deletions 69 coderd/ai_providers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import (
"github.com/google/uuid"
"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/util/ptr"
Expand Down Expand Up @@ -992,6 +994,73 @@ func TestAIProvidersKeyManagement(t *testing.T) {
require.Contains(t, sdkErr.Validations[0].Detail, "already referenced")
})

t.Run("PATCHKeysSurfacesOpsInAudit", func(t *testing.T) {
t.Parallel()
auditor := audit.NewMock()
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
_ = coderdtest.CreateFirstUser(t, client)
ctx := testutil.Context(t, testutil.WaitLong)

// Without surfacing per-op detail, a PATCH that only rotates
// keys would produce an audit entry whose top-level diff is
// empty: invisible key rotation in the log.
//nolint:gocritic // Owner role is the audience for this endpoint.
provider, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{
Type: codersdk.AIProviderTypeOpenAI,
Name: "keys-audit-ops",
Enabled: true,
BaseURL: "https://api.openai.com/v1",
APIKeys: []string{
"sk-openai-audit-1-ssssssssssssssssssss", //nolint:gosec // test fixture
"sk-openai-audit-2-tttttttttttttttttttt", //nolint:gosec // test fixture
},
})
require.NoError(t, err)
keepID := provider.APIKeys[0].ID

// Keep one, drop one, add one.
mutations := []codersdk.AIProviderKeyMutation{
{ID: &keepID},
{APIKey: ptr.Ref("sk-openai-audit-3-uuuuuuuuuuuuuuuuuuuu")}, //nolint:gosec // test fixture
}
updatedProvider, err := client.UpdateAIProvider(ctx, provider.Name, codersdk.UpdateAIProviderRequest{
APIKeys: &mutations,
})
require.NoError(t, err)

// The newly-inserted row's ID and masked rendering are dynamic;
// pull them from the PATCH response so we can build the expected
// audit payload without re-declaring the audit struct shape.
var added codersdk.AIProviderKey
for _, k := range updatedProvider.APIKeys {
if k.ID != keepID {
added = k
break
}
}
require.NotEqual(t, uuid.Nil, added.ID)
require.NotEmpty(t, added.Masked)
require.NotContains(t, added.Masked, "sk-openai-audit-3-uuuuuuuuuuuuuuuuuuuu")
removed := provider.APIKeys[1]

logs := auditor.AuditLogs()
var updated *database.AuditLog
for i := range logs {
if logs[i].Action == database.AuditActionWrite && logs[i].ResourceType == database.ResourceTypeAIProvider {
updated = &logs[i]
}
}
require.NotNil(t, updated, "expected audit log for AI provider update")

expected, err := json.Marshal(map[string]any{
"added": []map[string]any{{"id": added.ID, "masked": added.Masked}},
"removed": []map[string]any{{"id": removed.ID, "masked": removed.Masked}},
"kept": 1,
})
require.NoError(t, err)
require.JSONEq(t, string(expected), string(updated.AdditionalFields))
})

t.Run("MutationUnknownIDRejected", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
Expand Down
2 changes: 1 addition & 1 deletion 2 coderd/audit/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
case database.AiSeatState:
return database.ResourceTypeAiSeat
case database.AIProvider:
return database.ResourceTypeAiProvider
return database.ResourceTypeAIProvider

@ibetitsmike ibetitsmike May 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

are you sure you are actually consistent with this casing throughout the codebase?

@dannykopping dannykopping May 20, 2026

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.

It's super unfortunate that the other ones were Ai and not corrected to AI.

I've added a follow-up item to correct the others: https://linear.app/codercom/issue/AIGOV-369/change-ai-references-in-coderddatabasemodelsgo-to-ai

case database.AuditableGroupAiBudget:
return database.ResourceTypeGroupAiBudget
case database.Chat:
Expand Down
12 changes: 6 additions & 6 deletions 12 coderd/database/models.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions 2 coderd/database/sqlc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ sql:
ai_provider: AIProvider
ai_provider_key: AIProviderKey
ai_provider_type: AIProviderType
resource_type_ai_provider: ResourceTypeAIProvider
resource_type_ai_provider_key: ResourceTypeAIProviderKey
mcp_server_config: MCPServerConfig
mcp_server_configs: MCPServerConfigs
mcp_server_user_token: MCPServerUserToken
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.