Skip to content

fix: update proton and migrate to runtime proto validation#1490

Merged
ravisuhag merged 5 commits intomainfrom
chore/update-proton-gen-code
Apr 1, 2026
Merged

fix: update proton and migrate to runtime proto validation#1490
ravisuhag merged 5 commits intomainfrom
chore/update-proton-gen-code

Conversation

@ravisuhag
Copy link
Copy Markdown
Member

@ravisuhag ravisuhag commented Mar 30, 2026

Summary

Update proton to latest main (0b6548e) and migrate proto validation from codegen to runtime.

Background

Proton moved validation annotations from envoyproxy/protoc-gen-validate to buf/validate. The old validate-go codegen plugin does not understand buf.validate annotations — it silently generates empty Validate() methods, making all rules no-ops.

On main today, only 13 out of 253 handlers call .Validate(). The remaining 133 annotated request types were never validated at the handler level. Some of these were caught deeper in the stack by service or repository layers, but not all:

  • Service layer catches some cases — e.g. empty org_id returns ErrInvalidID, invalid UUID fails on resource fetch. Works, but errors are less specific.
  • Nil body causes panicsCreateOrganization, CreateBillingAccount and similar handlers with body: message.required crash if body is nil instead of returning a validation error.
  • Empty collections silently succeedCreateOrganizationInvitation with empty user_ids returns an empty response instead of rejecting (min_items=1).
  • Invalid formats get wrong errorsCheckResourcePermission with invalid@#$% hits the DB and returns 404 instead of 400.

What changed

  • Makefile: Update PROTON_COMMIT to 0b6548e
  • buf.gen.yaml: Remove validate-go codegen plugin, update managed mode for buf/validate
  • proto/v1beta1/: Regenerate .pb.go files, delete 77k lines of .pb.validate.go codegen stubs
  • pkg/server/server.go: Add connectrpc.com/validate.NewInterceptor() to the Connect interceptor chain
  • internal/api/v1beta1connect/: Remove 13 manual .Validate() calls (interceptor handles this now)
  • cmd/: Remove 14 manual .ValidateAll() calls (server validates on receipt)
  • go.mod: Add connectrpc.com/validate; upgrade connectrpc.com/connect, google.golang.org/protobuf

The interceptor validates all incoming requests against buf.validate annotations at runtime before any handler code runs. No per-handler code needed.

Before vs after

main This PR
Handlers with validation 13 manual .Validate() calls All 253 (automatic)
Annotated request types validated 13 / 146 146 / 146
New handler needs validation code Yes No
Mechanism Codegen (protoc-gen-validate) Runtime (connectrpc.com/validate)

Test plan

  • go build ./...
  • go vet ./...
  • CI
  • Send a request with invalid data (e.g. bad UUID) and confirm CodeInvalidArgument

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview, Comment Apr 1, 2026 6:44am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f8d848e2-6363-417b-8349-b2f8bbff3090

📥 Commits

Reviewing files that changed from the base of the PR and between b0ea2cf and 8c4927d.

📒 Files selected for processing (1)
  • pkg/server/server.go
✅ Files skipped from review due to trivial changes (1)
  • pkg/server/server.go

📝 Walkthrough

Summary by CodeRabbit

  • Chores

    • Bumped Go toolchain and upgraded multiple dependencies (Connect, protobuf, gRPC, and others)
    • Updated protobuf generation/validation configuration
  • Refactor

    • Moved request validation out of many client-side flows and consolidated validation on the server via a validation middleware
  • Tests

    • Removed a few test cases that asserted client-side validation errors

Walkthrough

Removed local per-request protobuf validation calls across CLI and handlers, added a centralized connect validation interceptor, updated proto generation commit and buf validation config, and bumped Go toolchain and module dependencies.

Changes

Cohort / File(s) Summary
Proto generation & buf config
Makefile, buf.gen.yaml
Updated PROTON_COMMIT to a new hash; changed buf managed disable entry to buf.build/bufbuild/protovalidate and removed the validate-go plugin generation step.
Go module / toolchain
go.mod
Bumped Go toolchain and upgraded/added multiple modules (connectrpc v1.19.0, connect validate, protovalidate, protobuf, grpc, testify, x/* and various indirect deps).
CLI commands (removed local ValidateAll)
cmd/group.go, cmd/organization.go, cmd/permission.go, cmd/policy.go, cmd/project.go, cmd/role.go, cmd/user.go
Removed reqBody.ValidateAll() calls and their early-return error handling in create/edit commands; parsing and RPC invocation remain.
Connect handlers (removed request.Msg.Validate)
internal/api/v1beta1connect/...
audit_record.go, organization.go, organization_pats.go, session.go, user_pat.go
Deleted per-handler request.Msg.Validate() checks and associated connect.CodeInvalidArgument short-circuits; kept other parsing, explicit checks, and service error mappings.
Server interceptor chain
pkg/server/server.go
Added connectrpc.com/validate interceptor and inserted it into the Connect interceptor chain.
Tests
internal/api/v1beta1connect/audit_record_test.go, internal/api/v1beta1connect/organization_test.go
Removed test cases asserting handler-level validation errors (expecting connect.CodeInvalidArgument on invalid request fields).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • whoAbhishekSah
  • rohilsurana

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ravisuhag ravisuhag force-pushed the chore/update-proton-gen-code branch from d75bea2 to 22d2368 Compare March 31, 2026 00:20
@ravisuhag ravisuhag changed the title chore: update proton to latest and regenerate proto code chore: update proton and migrate to runtime proto validation Mar 31, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
pkg/server/server.go (1)

157-166: Consider interceptor ordering for fail-fast behavior.

The validateInterceptor is placed after authentication and authorization interceptors. This means:

  • Invalid requests from authenticated users still consume auth resources before failing validation
  • However, this prevents leaking validation error details to unauthenticated callers

If early rejection of malformed requests is preferred (fail-fast), consider moving validateInterceptor earlier in the chain (e.g., after UnaryConnectErrorResponseInterceptor). If preventing information disclosure to unauthenticated users is the priority, the current placement is appropriate.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 21822891-ef0f-41ce-b1e2-9c7db0e6fb10

📥 Commits

Reviewing files that changed from the base of the PR and between 711d019 and d75bea2.

⛔ Files ignored due to path filters (4)
  • go.sum is excluded by !**/*.sum
  • proto/v1beta1/admin.pb.validate.go is excluded by !proto/**
  • proto/v1beta1/frontier.pb.validate.go is excluded by !proto/**
  • proto/v1beta1/models.pb.validate.go is excluded by !proto/**
📒 Files selected for processing (15)
  • buf.gen.yaml
  • cmd/group.go
  • cmd/organization.go
  • cmd/permission.go
  • cmd/policy.go
  • cmd/project.go
  • cmd/role.go
  • cmd/user.go
  • go.mod
  • internal/api/v1beta1connect/audit_record.go
  • internal/api/v1beta1connect/organization.go
  • internal/api/v1beta1connect/organization_pats.go
  • internal/api/v1beta1connect/session.go
  • internal/api/v1beta1connect/user_pat.go
  • pkg/server/server.go
💤 Files with no reviewable changes (12)
  • cmd/role.go
  • cmd/organization.go
  • cmd/user.go
  • cmd/permission.go
  • cmd/group.go
  • cmd/project.go
  • cmd/policy.go
  • internal/api/v1beta1connect/user_pat.go
  • internal/api/v1beta1connect/organization_pats.go
  • internal/api/v1beta1connect/organization.go
  • internal/api/v1beta1connect/session.go
  • internal/api/v1beta1connect/audit_record.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • buf.gen.yaml

github.com/stretchr/testify v1.11.1
github.com/stripe/stripe-go/v79 v79.5.0
github.com/ua-parser/uap-go v0.0.0-20250917011043-9c86a9b0f8f0
go.opentelemetry.io/otel/sdk/metric v1.37.0
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for patched otel/sdk versions
echo "=== Checking OpenTelemetry SDK vulnerability advisory ==="
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: GO, package: "go.opentelemetry.io/otel/sdk") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

echo ""
echo "=== Available otel/sdk versions ==="
curl -s "https://proxy.golang.org/go.opentelemetry.io/otel/sdk/@v/list" | tail -10

Repository: raystack/frontier

Length of output: 395


🏁 Script executed:

#!/bin/bash
# Get full list of otel/sdk/metric versions (not just last 10)
echo "=== Full list of go.opentelemetry.io/otel/sdk/metric versions ==="
curl -s "https://proxy.golang.org/go.opentelemetry.io/otel/sdk/metric/@v/list"

echo ""
echo "=== Check if v1.37.0 exists ==="
curl -s -I "https://proxy.golang.org/go.opentelemetry.io/otel/sdk/metric/@v/v1.37.0.info"

Repository: raystack/frontier

Length of output: 1277


🌐 Web query:

GHSA-9h8m-3fm2-qjrq vulnerability OpenTelemetry PATH hijacking

💡 Result:

GHSA-9h8m-3fm2-qjrq (CVE-2026-24051) is a high-severity vulnerability (CVSS 7.0) in the OpenTelemetry Go SDK (go.opentelemetry.io/otel/sdk), affecting versions 1.21.0 to 1.39.0. It enables arbitrary code execution via PATH hijacking on macOS/Darwin systems. The resource detection code in sdk/resource/host_id.go executes the 'ioreg' system command without specifying a full path, relying on the PATH environment variable. A local attacker with low privileges who can modify PATH (e.g., by placing a malicious 'ioreg' executable in a directory prioritized in PATH) can achieve arbitrary code execution in the application's context. Associated with CWE-426 (Untrusted Search Path). Patched in commit d45961b, released in v1.40.0 (February 2026). Upgrade to v1.40.0 or later. As of 2026-03-31, the patch is available. Official sources: GitHub Advisory Database, OpenTelemetry Go security advisory, NVD.

Citations:


🌐 Web query:

CVE OpenTelemetry SDK metric PATH hijacking vulnerability 2024 2025

💡 Result:

No vulnerability matching "OpenTelemetry SDK metric PATH hijacking" was found in searches across 2024-2025 or later. The closest match is CVE-2026-24051 (disclosed Feb 2026), a PATH hijacking vulnerability in the OpenTelemetry Go SDK (go.opentelemetry.io/otel/sdk/resource, versions >=1.21.0, <=1.39.0) affecting macOS/Darwin systems. It allows arbitrary code execution via untrusted PATH when executing the 'ioreg' command in sdk/resource/host_id.go for resource detection (host ID). CVSS 3.1: 7.0 (High). Patched in v1.40.0 via commit d45961bcda453fcbdb6469c22d6e88a1f9970a53. This issue is unrelated to metrics or path traversal in metric handling. Other OpenTelemetry vulnerabilities (e.g., CVE-2023-25151 for high-cardinality metrics DoS in otelhttp, CVE-2024-36129 for DoS in Collector, CVE-2023-43810 for unbound metrics cardinality) do not involve PATH hijacking or path traversal in metrics.

Citations:


🏁 Script executed:

#!/bin/bash
# Check the go.mod file in the repo to see all OpenTelemetry dependencies
echo "=== OpenTelemetry dependencies in go.mod ==="
grep -i "go.opentelemetry" go.mod

echo ""
echo "=== Checking otel/sdk/metric v1.37.0 dependencies ==="
curl -s "https://proxy.golang.org/go.opentelemetry.io/otel/sdk/metric/@v/v1.37.0.mod" | head -20

Repository: raystack/frontier

Length of output: 1244


🌐 Web query:

go.opentelemetry.io/otel/sdk/metric v1.37.0 dependencies PATH hijacking vulnerability

💡 Result:

Yes, go.opentelemetry.io/otel/sdk/metric v1.37.0 is vulnerable to the PATH hijacking vulnerability CVE-2026-24051 (GHSA-9h8m-3fm2-qjrq, GO-2026-4394). This affects the broader OpenTelemetry Go SDK (go.opentelemetry.io/otel/sdk) versions >=v1.21.0 and <v1.40.0, specifically due to untrusted search path usage when executing the 'ioreg' command on macOS/Darwin systems in sdk/resource/host_id.go, allowing local authenticated attackers to achieve arbitrary code execution by modifying the PATH environment variable. The sdk/metric module depends on the vulnerable go.opentelemetry.io/otel/sdk (as shown in pkg.go.dev listings for similar versions like v1.38.0, which lists go.opentelemetry.io/otel/sdk v1.38.0). Version alignment across OpenTelemetry Go modules means v1.37.0 of sdk/metric pulls in the vulnerable sdk v1.37.0. The fix is in v1.40.0 via commit d45961b, which specifies the full path for the ioreg command. CVSS v3.1: 7.0 High (AV:L/AC:H/PR:L/UI:N/S:U/C:H/I:H/A:H). Patched in v1.40.0. Upgrade to v1.40.0 or later. Workarounds: avoid running on macOS/Darwin or ensure PATH cannot be modified by untrusted users.

Citations:


Upgrade go.opentelemetry.io/otel/sdk/metric to v1.40.0 or later.

go.opentelemetry.io/otel/sdk/metric v1.37.0 contains a HIGH severity vulnerability (GHSA-9h8m-3fm2-qjrq / CVE-2026-24051) that enables arbitrary code execution via PATH hijacking on macOS/Darwin systems. The vulnerability exists in the base SDK's resource detection code, which executes the 'ioreg' command without specifying a full path. A patched version (v1.40.0+) is available.

🧰 Tools
🪛 OSV Scanner (2.3.5)

[HIGH] 50-50: go.opentelemetry.io/otel/sdk 1.37.0: OpenTelemetry Go SDK Vulnerable to Arbitrary Code Execution via PATH Hijacking in go.opentelemetry.io/otel/sdk

(GO-2026-4394)


[HIGH] 50-50: go.opentelemetry.io/otel/sdk 1.37.0: OpenTelemetry Go SDK Vulnerable to Arbitrary Code Execution via PATH Hijacking

(GHSA-9h8m-3fm2-qjrq)

google.golang.org/protobuf v1.36.6
golang.org/x/sync v0.17.0
google.golang.org/genproto/googleapis/api v0.0.0-20250922171735-9219d122eba9
google.golang.org/grpc v1.71.0
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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for patched gRPC versions and vulnerability details
echo "=== Checking gRPC vulnerability advisory ==="
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: GO, package: "google.golang.org/grpc") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

echo ""
echo "=== Available gRPC versions from proxy ==="
curl -s "https://proxy.golang.org/google.golang.org/grpc/@v/list" | tail -10

Repository: raystack/frontier

Length of output: 409


🌐 Web query:

GHSA-p77j-4mvh-x3m3 gRPC vulnerability

💡 Result:

GHSA-p77j-4mvh-x3m3 is a critical authorization bypass vulnerability (CVE-2026-33186) in gRPC-Go, the Go implementation of gRPC. Description: The gRPC-Go server improperly validates the HTTP/2 :path pseudo-header, accepting requests without a leading slash (e.g., "Service/Method" instead of "/Service/Method"). While routing works, authorization interceptors like grpc/authz evaluate the raw non-canonical path, bypassing "deny" rules for canonical paths if a fallback "allow" exists. Affected: gRPC-Go versions < 1.79.3, specifically servers using path-based authorization with deny rules and fallback allow. Impact: Attackers can send malformed HTTP/2 requests to bypass auth, gaining unauthorized access (CVSS 3.1: 9.1 Critical, AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:N). Fixed: Upgrade to 1.79.3 or later, which rejects invalid :path headers with codes.Unimplemented. Published: March 17-20, 2026. Official advisory: grpc/grpc-go GitHub security page.

Citations:


🌐 Web query:

google.golang.org/grpc v1.71.0 security vulnerability authorization bypass

💡 Result:

google.golang.org/grpc v1.71.0 is affected by CVE-2026-33186 (authorization bypass), because the advisory lists all versions < 1.79.3 as vulnerable. [1][2]

  • What the bypass is: gRPC-Go accepted HTTP/2 requests whose :path omitted the required leading slash (e.g., Service/Method instead of /Service/Method). The server could still route the call, but path-based authorization (including google.golang.org/grpc/authz) could evaluate the non-canonical path and fail to match “deny” rules written for canonical paths—allowing a bypass when a fallback “allow” exists. [1][2]
  • Who is impacted: gRPC-Go servers using path-based authorization (e.g., grpc/authz / RBAC, or custom interceptors relying on info.FullMethod / grpc.Method(ctx)) with policies that include canonical-path denies but otherwise allow by default. [1][2]
  • Fix: upgrade to v1.79.3+ (patched). The fix rejects :path values that don’t start with /. [2]
  • Workaround (if you can’t upgrade immediately): add an outermost interceptor that rejects requests where info.FullMethod doesn’t start with /, and/or move to a “default deny” policy posture. [2]

Sources: GitHub Security Advisory GHSA-p77j-4mvh-x3m3 / CVE-2026-33186, and NVD entry for CVE-2026-33186. [1][2]


🌐 Web query:

gRPC Go v1.71.1 security patch release notes

💡 Result:

gRPC-Go v1.71.1 (released Apr 1) — “security patch” release notes

In the upstream grpc-go GitHub release entry for v1.71.1, there is no separate “Security” section; it’s published as a bug-fix release with the following items: (github.com)

  • grpc: Fix extra read from the compressor when a compressed message is exactly at the limit (could panic with the built-in gzip compressor). (github.com)
  • xds: Read bootstrap config before creating the first xDS client (instead of at package init time). (github.com)
  • stats/opentelemetry: Use TextMapPropagator and TracerProvider from TraceOptions instead of OpenTelemetry globals. (github.com)
  • client: Fix proxy-related races that could lead to deadlocks/panics. (github.com)
  • client: Fix RPC failures (“no children to pick from”) when using a custom resolver that calls deprecated NewAddress. (github.com)
  • wrr: Fix slow processing of address updates that could lead to issues including RPC failures with many backends. (github.com)

Source

  1. grpc/grpc-go GitHub releases page (v1.71.1 entry). (github.com)

Citations:


Upgrade google.golang.org/grpc to v1.79.3 or later to fix critical authorization bypass vulnerability.

gRPC-Go v1.71.0 is affected by CVE-2026-33186 (GHSA-p77j-4mvh-x3m3), a critical authorization bypass vulnerability (CVSS 9.1). Servers using path-based authorization can be bypassed via malformed HTTP/2 requests with missing leading slash in the :path pseudo-header. The vulnerability is fixed in v1.79.3+; earlier patch versions in the v1.71.x line do not address this issue.

🧰 Tools
🪛 OSV Scanner (2.3.5)

[CRITICAL] 57-57: google.golang.org/grpc 1.71.0: Authorization bypass in gRPC-Go via missing leading slash in :path in google.golang.org/grpc

(GO-2026-4762)


[CRITICAL] 57-57: google.golang.org/grpc 1.71.0: gRPC-Go has an authorization bypass via missing leading slash in :path

(GHSA-p77j-4mvh-x3m3)

Update PROTON_COMMIT to latest main (0b6548e) which migrates from
envoyproxy/protoc-gen-validate to buf/validate annotations.

Replace codegen-based validation with connectrpc.com/validate interceptor
that automatically validates all incoming requests at runtime using
buf.validate annotations from proto descriptors via protovalidate-go.

- Update PROTON_COMMIT and regenerate proto code
- Remove validate-go codegen plugin from buf.gen.yaml
- Delete stale .pb.validate.go files (77k lines)
- Add connectrpc.com/validate interceptor to Connect server
- Remove manual .Validate()/.ValidateAll() calls from handlers and CLI
Tests were calling handlers directly, bypassing the connect interceptor
where validation now runs after migrating to runtime proto validation.
@coveralls
Copy link
Copy Markdown

coveralls commented Mar 31, 2026

Pull Request Test Coverage Report for Build 23835706551

Details

  • 0 of 3 (0.0%) changed or added relevant lines in 1 file are covered.
  • 199 unchanged lines in 15 files lost coverage.
  • Overall coverage decreased (-0.5%) to 41.19%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/server/server.go 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
billing/usage/usage.go 3 0.0%
core/domain/domain.go 3 0.0%
internal/store/spicedb/relation_repository.go 3 0.0%
core/serviceuser/serviceuser.go 6 0.0%
pkg/auditrecord/consts.go 6 0.0%
pkg/mailer/dialer.go 6 0.0%
pkg/crypt/random.go 7 0.0%
core/metaschema/service.go 9 0.0%
billing/plan/plan.go 12 0.0%
pkg/session/metadata.go 12 0.0%
Totals Coverage Status
Change from base Build 23833121201: -0.5%
Covered Lines: 14839
Relevant Lines: 36026

💛 - Coveralls

@ravisuhag ravisuhag changed the title chore: update proton and migrate to runtime proto validation fix: update proton and migrate to runtime proto validation Mar 31, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
go.mod (2)

57-57: ⚠️ Potential issue | 🔴 Critical

Upgrade google.golang.org/grpc from vulnerable v1.71.0.

Line 57 is on a version affected by GHSA-p77j-4mvh-x3m3 / GO-2026-4762 (authorization bypass). Please bump to a patched release (>= v1.79.3).

Suggested diff
-	google.golang.org/grpc v1.71.0
+	google.golang.org/grpc v1.79.3

50-50: ⚠️ Potential issue | 🟠 Major

Bump OpenTelemetry SDK off vulnerable v1.37.0.

Line 50 keeps go.opentelemetry.io/otel/sdk/metric v1.37.0, which is flagged for GHSA-9h8m-3fm2-qjrq / GO-2026-4394. Please upgrade to a patched version (>= v1.40.0) and keep OTEL module versions aligned.

Suggested diff
-	go.opentelemetry.io/otel/sdk/metric v1.37.0
+	go.opentelemetry.io/otel/sdk/metric v1.40.0
pkg/server/server.go (1)

157-165: ⚠️ Potential issue | 🟠 Major

Run validation before authorization in interceptor chain.

validateInterceptor is currently after authZInterceptor (Line 163 → Line 164). If authZ depends on request fields, invalid payload reaches authZ first. Move validation before authZ (typically right after authN/session context setup).

Suggested diff
 	interceptors := connect.WithInterceptors(
 		otelInterceptor,
 		connectinterceptors.UnaryConnectLoggerInterceptor(zapLogger.Desugar(), loggerOpts),
 		connectinterceptors.UnaryConnectErrorResponseInterceptor(),
 		sessionInterceptor,
 		authNInterceptor,
-		authZInterceptor,
 		validateInterceptor,
+		authZInterceptor,
 		auditInterceptor,
 		sessionInterceptor.UnaryConnectResponseInterceptor())

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fcb82b55-58ca-45ec-972b-99f9e1bff530

📥 Commits

Reviewing files that changed from the base of the PR and between 22d2368 and b0ea2cf.

⛔ Files ignored due to path filters (7)
  • go.sum is excluded by !**/*.sum
  • proto/v1beta1/admin.pb.go is excluded by !**/*.pb.go, !proto/**
  • proto/v1beta1/admin.pb.validate.go is excluded by !proto/**
  • proto/v1beta1/frontier.pb.go is excluded by !**/*.pb.go, !proto/**
  • proto/v1beta1/frontier.pb.validate.go is excluded by !proto/**
  • proto/v1beta1/models.pb.go is excluded by !**/*.pb.go, !proto/**
  • proto/v1beta1/models.pb.validate.go is excluded by !proto/**
📒 Files selected for processing (18)
  • Makefile
  • buf.gen.yaml
  • cmd/group.go
  • cmd/organization.go
  • cmd/permission.go
  • cmd/policy.go
  • cmd/project.go
  • cmd/role.go
  • cmd/user.go
  • go.mod
  • internal/api/v1beta1connect/audit_record.go
  • internal/api/v1beta1connect/audit_record_test.go
  • internal/api/v1beta1connect/organization.go
  • internal/api/v1beta1connect/organization_pats.go
  • internal/api/v1beta1connect/organization_test.go
  • internal/api/v1beta1connect/session.go
  • internal/api/v1beta1connect/user_pat.go
  • pkg/server/server.go
💤 Files with no reviewable changes (14)
  • cmd/group.go
  • cmd/organization.go
  • cmd/policy.go
  • cmd/role.go
  • cmd/project.go
  • cmd/permission.go
  • internal/api/v1beta1connect/organization.go
  • cmd/user.go
  • internal/api/v1beta1connect/audit_record.go
  • internal/api/v1beta1connect/user_pat.go
  • internal/api/v1beta1connect/audit_record_test.go
  • internal/api/v1beta1connect/organization_test.go
  • internal/api/v1beta1connect/session.go
  • internal/api/v1beta1connect/organization_pats.go
✅ Files skipped from review due to trivial changes (1)
  • buf.gen.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • Makefile

Reject invalid requests before authorization so malformed input
is caught early and doesn't reach the authz layer.
@ravisuhag ravisuhag merged commit fd403b0 into main Apr 1, 2026
8 checks passed
@ravisuhag ravisuhag deleted the chore/update-proton-gen-code branch April 1, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants