Task 5011#5030
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughA pull request URL field is added across the CLA manager notification flow: API schemas, handler call, service interface and implementation, email models and templates, URL sanitization, and tests. ChangesPull Request URL Support
Sequence DiagramsequenceDiagram
participant Client
participant Handler as API Handler
participant Service as CLA Manager Service
participant Sanitizer as URL Sanitizer
participant Model as Email Model
participant Template as Email Template
Client->>Handler: POST /invite-company-admin (pullRequestURL)
Handler->>Service: InviteCompanyAdmin(..., pullRequestURL)
Service->>Sanitizer: sanitizePullRequestURL(rawURL)
Sanitizer-->>Service: sanitizedURL or ""
Service->>Model: build EmailToCLAManagerModel / ContributorEmailToOrgAdminModel (PullRequestURL: sanitizedURL)
Model->>Template: Render with params (PullRequestURL)
Template-->>Model: Rendered HTML (conditional "Pull request" link)
Model-->>Service: rendered email
Service-->>Handler: operation result
Handler-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cla-backend-go/v2/cla_manager/emails.go`:
- Around line 31-35: The code is logging the full caller-controlled
pullRequestURL (in sanitizePullRequestURL and other spots), which can leak
secrets/PII; instead parse the rawURL and log only safe metadata: the hostname
(parsedURL.Host), a deterministic short hash (e.g., first 8 chars of
SHA256(rawURL)) and/or the URL with query stripped (scheme+host+path) but never
the full URL with query or tokens. Update the logrus.Fields in
sanitizePullRequestURL (replace "pullRequestURL": rawURL) to include
"pullRequestHost": parsed.Host and "pullRequestURLHash": <short-hash-of-rawURL>
and optionally "pullRequestURLNoQuery": <url-without-query>; apply the same
replacement at the other occurrences noted (the blocks around lines 142-156 and
220-233) so no codepath emits the raw pullRequestURL.
- Around line 21-57: sanitizePullRequestURL currently allows any absolute https
URL; tighten it to only accept known forge hosts and PR/MR/change URL shapes:
after parsing (in sanitizePullRequestURL) normalize parsed.Host (use
parsed.Hostname(), toLower, strip ports) and check it against an explicit
allowlist of trusted domains (e.g., github.com, gitlab.com, gerrit-hosts
configured centrally), then validate the path against provider-specific patterns
(e.g., GitHub "/pull/", GitLab "/-/merge_requests/" or Gerrit change URL shapes)
and return "" (with the existing log.WithFields warnings) if the host or path
pattern does not match; implement the allowlist as a package-level constant or
configurable list and add a small helper like isTrustedForgeHost(host) and
matchesForgePRPath(parsed, provider) to keep sanitizePullRequestURL readable.
In `@cla-backend-go/v2/cla_manager/service.go`:
- Around line 879-887: The log currently includes the raw user-supplied
pullRequestURL in the logrus.Fields map inside InviteCompanyAdmin; sanitize (or
strip sensitive query params/token fragments from) pullRequestURL once at the
start of InviteCompanyAdmin into a new variable (e.g., sanitizedPullRequestURL)
and replace the direct pullRequestURL reference in the f := logrus.Fields{...}
initializer with the sanitized variable (or remove the URL key entirely) so only
the cleaned value is logged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 045b2888-c2bc-4534-a506-4a1ae047f6bb
📒 Files selected for processing (8)
.gitignorecla-backend-go/emails/v2_cla_manager_templates.gocla-backend-go/swagger/cla.v2.yamlcla-backend-go/tests/v2_cla_manager_templates_test.gocla-backend-go/v2/cla_manager/emails.gocla-backend-go/v2/cla_manager/handlers.gocla-backend-go/v2/cla_manager/pull_request_url_test.gocla-backend-go/v2/cla_manager/service.go
There was a problem hiding this comment.
Pull request overview
Adds an optional pull request URL to CLA-manager notification flows so approval/invitation emails can include a direct link back to the originating contribution.
Changes:
- Extends the
InviteCompanyAdminservice/API surface to accept apullRequestURL. - Adds URL sanitization before injecting the link into outgoing CLA Manager / Org Admin emails.
- Updates V2 email templates + tests to conditionally render the “Pull request:” link.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| cla-backend-go/v2/cla_manager/service.go | Threads pullRequestURL through invite + notify flows and passes a sanitized value into outbound email models. |
| cla-backend-go/v2/cla_manager/handlers.go | Passes the new pullRequestURL request field through to the service layer. |
| cla-backend-go/v2/cla_manager/emails.go | Introduces sanitizePullRequestURL and adds PullRequestURL to email model/template params. |
| cla-backend-go/v2/cla_manager/pull_request_url_test.go | Adds unit tests covering accepted/rejected URL cases for sanitization. |
| cla-backend-go/emails/v2_cla_manager_templates.go | Updates V2 CLA manager templates to conditionally include a pull request hyperlink. |
| cla-backend-go/tests/v2_cla_manager_templates_test.go | Verifies templates omit the PR section when empty and render it when provided. |
| cla-backend-go/swagger/cla.v2.yaml | Adds pullRequestURL to the invite-company-admin request body and notify-cla-manager-list schema. |
| .gitignore | Ignores CLAUDE.md. |
No description provided.