fix(column): validate url in config #516
Conversation
WalkthroughEndpoint field in the Column plugin config now requires a valid URL via validator tag change to "required,url". A new test asserts that providing a non-URL endpoint triggers a field validation error during plugin construction. No other logic or public API changes. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer/Test
participant New as ColumnPlugin.New
participant V as Validator
Dev->>New: Provide config { apiKey, endpoint }
New->>V: Validate struct tags (Endpoint: required,url)
alt Endpoint is valid URL
V-->>New: OK
New-->>Dev: Plugin instance
else Endpoint invalid
V-->>New: Validation error
New-->>Dev: Error (field validation)
end
note over V,New: Validation tightened to require URL format
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/connectors/plugins/public/column/config.go (1)
13-13: Optional: Consider enforcing HTTPS if policy requires itIf endpoints should never be plain HTTP in production, you can tighten the validator. One low-friction option (without custom validators) is to require HTTPS via startswith in addition to url:
Apply this diff:
- Endpoint string `json:"endpoint" validate:"required,url"` + Endpoint string `json:"endpoint" validate:"required,url,startswith=https://"`Note: This will break tests using httptest.Server (http). If you want to enforce HTTPS only in production, we can instead add a custom validator or allow http for localhost while requiring https otherwise—happy to propose that if helpful.
internal/connectors/plugins/public/column/plugin_test.go (1)
65-71: Tighten the assertion and drop stdout printing in testAvoid printing the error to stdout and assert the error occurred before inspecting its message. Also check for field and tag to make the test less brittle than matching the generic "Field validation" substring.
Apply this diff:
It("should report errors in config - endpoint not url", func(ctx SpecContext) { config := json.RawMessage(`{"apiKey": "test", "endpoint": "fake"}`) _, err := New(connID, ProviderName, logger, config) - fmt.Println(err.Error()) - Expect(err.Error()).To(ContainSubstring("Field validation")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Endpoint")) + Expect(err.Error()).To(ContainSubstring("'url'")) })If you want to make it even more robust, prefer type-based checks over string matching:
// imports: // "errors" // "github.com/go-playground/validator/v10" var verrs validator.ValidationErrors Expect(errors.As(err, &verrs)).To(BeTrue()) var endpointURLFound bool for _, fe := range verrs { if fe.Field() == "Endpoint" && fe.Tag() == "url" { endpointURLFound = true break } } Expect(endpointURLFound).To(BeTrue(), "expected Endpoint to fail on 'url' tag")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal/connectors/plugins/public/column/config.go(1 hunks)internal/connectors/plugins/public/column/plugin_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: paul-nicolas
PR: formancehq/payments#509
File: internal/connectors/plugins/public/powens/create_user_link.go:31-36
Timestamp: 2025-08-08T13:48:23.427Z
Learning: In formancehq/payments Powens plugin validation functions (e.g., validateCreateUserLinkRequest in internal/connectors/plugins/public/powens/create_user_link.go), avoid duplicating core validations like redirect URL format; the core layer already validates these per maintainer preference (paul-nicolas) in PR #509.
🧬 Code Graph Analysis (1)
internal/connectors/plugins/public/column/plugin_test.go (2)
internal/connectors/plugins/public/column/plugin.go (2)
New(81-102)ProviderName(16-16)internal/connectors/plugins/public/column/client/client.go (1)
New(67-87)
🔇 Additional comments (1)
internal/connectors/plugins/public/column/config.go (1)
13-13: LGTM: Enforcing URL validation for Endpoint is the right moveMaking Endpoint
required,urlaligns the config with how the client uses it and prevents obvious misconfigurations early.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #516 +/- ##
==========================================
- Coverage 67.84% 67.83% -0.01%
==========================================
Files 737 737
Lines 38262 38262
==========================================
- Hits 25957 25954 -3
- Misses 10952 10954 +2
- Partials 1353 1354 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.