Skip to content

Import shared proto types for identity and audit#114

Merged
haasonsaas merged 3 commits intomainfrom
jh/shared-protos-v2
Apr 12, 2026
Merged

Import shared proto types for identity and audit#114
haasonsaas merged 3 commits intomainfrom
jh/shared-protos-v2

Conversation

@haasonsaas
Copy link
Copy Markdown
Contributor

Summary

Replaces Gate-local proto definitions and hand-written structs with canonical types from evalops/proto. Rebased cleanly on main after #111 (ConnectRPC migration).

Identity types (Closes #110)

  • Remove local Organization, OrgMember, APIKey from tenant.proto
  • Import identity.v1.Organization, identity.v1.Member, identity.v1.APIKey from shared proto
  • Update all converter functions and clients (connect_admin.go, http_bridge.go, controlplaneclient)

Audit types (Closes #108)

  • Replace hand-written audit.Entry with auditv1.Event from shared proto
  • Sink interface accepts []*auditv1.Event
  • Logger methods construct proto events with nested Actor/Resource; domain fields in Metadata
  • convertEntries guards nil Timestamp, excludes extracted keys from Details
  • StdoutSink uses protojson and logs marshal errors
  • Added LogResponse zerolog line for consistency
  • Table-driven TestConvertEntries with 4 cases

Note: #109 (buf adoption) was already completed by #111.

Test plan

  • go test ./internal/audit/... ./internal/sync/... ./internal/controlplane/... ./internal/controlplaneclient/... — all pass
  • go build ./... — compiles clean
  • CI pipeline

🤖 Generated with Claude Code

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 12, 2026

PR Summary

Medium Risk
Medium risk because it changes protobuf wire types for tenant/identity APIs and swaps audit logging/storage to auditv1.Event, which could break existing clients and affect audit data mapping if any fields are mis-converted.

Overview
Migrates Gate off local protobuf/struct definitions to canonical shared protos from github.com/evalops/proto.

Identity/tenant APIs now use identity.v1 types (Organization, Member, APIKey) in tenant.proto and generated code, with corresponding updates across control-plane handlers, HTTP bridge conversions, and the control-plane client/tests.

Audit logging is refactored to emit and buffer auditv1.Event (proto) instead of a bespoke audit.Entry, updating the Sink interface, JSON stdout marshaling via protojson, and the connector AuditSink conversion logic (including excluding extracted metadata keys from Details) with expanded unit coverage.

CI/proto generation is updated to pull shared proto imports via symlinks, scope buf generate to proto/gate/v1, and temporarily tolerate buf breaking failures during the migration.

Reviewed by Cursor Bugbot for commit d7f1c6c. Bugbot is set up for automated code reviews on this repo. Configure here.

@socket-security
Copy link
Copy Markdown

socket-security bot commented Apr 12, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedgithub.com/​evalops/​proto@​v0.0.0-20260412001136-61b4bcf4dd1099100100100100

View full report

@haasonsaas haasonsaas force-pushed the jh/shared-protos-v2 branch 3 times, most recently from 836ad5b to a4bdccf Compare April 12, 2026 02:03
@haasonsaas haasonsaas closed this Apr 12, 2026
@haasonsaas haasonsaas reopened this Apr 12, 2026
@haasonsaas haasonsaas force-pushed the jh/shared-protos-v2 branch 3 times, most recently from 8631989 to 15a5493 Compare April 12, 2026 02:31
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but it could not run because the spend limit has been reached. To enable Bugbot Autofix, have a team admin raise the spend limit in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 15a5493. Configure here.

),
),
)
connectHandler := s.newConnectHandler()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ConnectRPC endpoints lose rate limiting and security middleware

High Severity

The connectMuxHandler method previously wrapped the ConnectRPC handler with middleware.RealIP, s.rateLimiter.Middleware(), maxBodySize(10 * 1024 * 1024), and securityHeaders. This commit removes all four layers, so requests to /gate.v1.* ConnectRPC endpoints now bypass rate limiting, request body size limits (10 MB), client IP detection, and security response headers. The REST routes in s.router still have these protections, but all ConnectRPC endpoints are now unprotected. This appears unintentional — the PR is about importing shared proto types, and the old code's comment explicitly noted these middleware were needed for parity with REST routes.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 15a5493. Configure here.

haasonsaas and others added 3 commits April 11, 2026 19:44
Remove local Organization, OrgMember, APIKey message definitions from
tenant.proto and import canonical types from identity/v1. Update all
converter functions, clients, and tests.

Closes #110

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Swap hand-written Entry struct for auditv1.Event from shared proto.
Sink interface accepts []*auditv1.Event. Logger methods construct proto
events with nested Actor/Resource. Metadata field overwrite prevented
by setting dedicated fields last. CI updated with shared proto symlinks
for buf lint and tenant.proto excluded from buf breaking during migration.

Closes #108

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Restore RealIP + rate limiter + body size + security headers on
  ConnectRPC path (lost during rebase from pre-#113 branch)
- Replace authReq with controlplaneclient.ApplyAuth (dedup from #113)
- Replace OutgoingHeaders with ApplyAuth (dedup from #115)
- withAuth delegates to ApplyAuth

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@haasonsaas haasonsaas force-pushed the jh/shared-protos-v2 branch from 15a5493 to d7f1c6c Compare April 12, 2026 02:47
@haasonsaas
Copy link
Copy Markdown
Contributor Author

Addressed all Bugbot feedback in d7f1c6c (rebased on main to pick up #113 and #115):

  1. ConnectRPC bypasses rate limiting/security middleware — Fixed. Restored middleware.RealIP, rateLimiter, maxBodySize, and securityHeaders wrapping on the ConnectRPC handler. This was lost because the branch predated Apply rate limiting to ConnectRPC path, deduplicate auth helpers #113.

  2. Metadata merge can overwrite dedicated audit fields — Not a bug. The code explicitly sets dedicated fields (conn_id, statement, query_type) after mergeMetadata runs, with a comment explaining the ordering: "Set dedicated fields last so protocol metadata cannot overwrite them" (logger.go:85).

  3. CI breaking change detection permanently suppressed — Intentional for this PR. continue-on-error: true is needed because moving Organization/OrgMember/APIKey to shared identity.v1 protos is a breaking wire change. The TODO comment at line 37-39 explains this should be removed after merge.

@haasonsaas haasonsaas merged commit da92474 into main Apr 12, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant