fix: decide the origin of proposal based on the request header#2274
fix: decide the origin of proposal based on the request header#2274JivusAyrus wants to merge 1 commit intomainfrom
Conversation
WalkthroughRemoves the origin field from CreateProposalRequest in the proto schema and generated TypeScript. Updates proposal creation logic to infer ProposalOrigin from the User-Agent header instead of reading it from the request payload, applying this in both main and subgraph proposal paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
controlplane/src/core/bufservices/proposal/createProposal.ts (1)
348-349: Consider extracting origin detection logic and improving maintainability.The User-Agent-based origin detection works correctly but could be more maintainable:
- Hardcoded string: The string
'cosmo-hub'should be extracted to a named constant for maintainability.- Missing context: Add a comment explaining why 'cosmo-hub' in the User-Agent indicates an EXTERNAL origin.
- Code duplication: The AI summary indicates this logic also appears in the "subgraph proposal path." If duplicated, extract this to a helper function like
deriveProposalOriginFromHeaders(headers: Headers): ProposalOrigin.- Debugging support: Consider logging the derived origin for debugging purposes (e.g.,
logger.debug(\Derived proposal origin: ${proposalOrigin}`)`).- Security note: Document that User-Agent headers can be spoofed and this isn't intended as a security boundary, but rather for categorization/telemetry.
Example refactor:
// At the top of the file or in a separate utils file const EXTERNAL_CLIENT_USER_AGENT = 'cosmo-hub'; function deriveProposalOriginFromHeaders(userAgent: string | null): ProposalOrigin { const normalizedUserAgent = userAgent?.toLowerCase() ?? ''; // cosmo-hub indicates requests from the external UI/hub, vs internal CLI/API return normalizedUserAgent.includes(EXTERNAL_CLIENT_USER_AGENT) ? 'EXTERNAL' : 'INTERNAL'; }Then use it:
- const clientHdr = ctx.requestHeader.get('user-agent')?.toLowerCase() ?? ''; - const proposalOrigin: ProposalOrigin = clientHdr.includes('cosmo-hub') ? 'EXTERNAL' : 'INTERNAL'; + const userAgent = ctx.requestHeader.get('user-agent'); + const proposalOrigin = deriveProposalOriginFromHeaders(userAgent); + logger.debug(`Derived proposal origin: ${proposalOrigin} from user-agent: ${userAgent}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**
📒 Files selected for processing (3)
connect/src/wg/cosmo/platform/v1/platform_pb.ts(0 hunks)controlplane/src/core/bufservices/proposal/createProposal.ts(3 hunks)proto/wg/cosmo/platform/v1/platform.proto(1 hunks)
💤 Files with no reviewable changes (1)
- connect/src/wg/cosmo/platform/v1/platform_pb.ts
🧰 Additional context used
🧬 Code graph analysis (1)
controlplane/src/core/bufservices/proposal/createProposal.ts (1)
controlplane/src/db/models.ts (1)
ProposalOrigin(39-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: build-router
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_push_image
- GitHub Check: image_scan (nonroot)
- GitHub Check: integration_test (./telemetry)
- GitHub Check: integration_test (./events)
- GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
- GitHub Check: build_test
- GitHub Check: image_scan
- GitHub Check: build_push_image
- GitHub Check: build_push_image (nonroot)
- GitHub Check: build_test
🔇 Additional comments (1)
controlplane/src/core/bufservices/proposal/createProposal.ts (1)
348-356: LGTM! Server-side origin derivation implemented correctly.The migration from client-provided
req.originto server-derivedproposalOriginbased on the User-Agent header correctly implements the PR objective. The origin value is properly typed and used in the proposal creation.
Summary by CodeRabbit
Checklist