-
Notifications
You must be signed in to change notification settings - Fork 18
Open
Description
Description
The GCP Pub/Sub destination provider doesn't validate the contents of serviceAccountJson
during creation or validate config during updates. This allows invalid data to be stored.
2 tests are currently skipped in the existing test suite due to missing validation.
Current Behavior
- Invalid
serviceAccountJson
(e.g.,"not-valid-json"
) is accepted during creation - Invalid config values are accepted during updates
- No validation error is returned
Evidence
Skipped Tests: spec-sdk-tests/tests/destinations/gcp-pubsub.test.ts
-
Line 218:
should reject creation with invalid serviceAccountJson
(it.skip)// TODO: Re-enable this test once the backend validates the contents of the serviceAccountJson. it.skip('should reject creation with invalid serviceAccountJson', async () => { await client.createDestination({ type: 'gcp_pubsub', topics: '*', config: { projectId: 'test-project', topic: 'test-topic', }, credentials: { serviceAccountJson: 'not-valid-json', // Should be rejected }, }); });
-
Line 520:
should reject update with invalid config
(it.skip)// TODO: Re-enable this test once the backend validates the config on update. it.skip('should reject update with invalid config', async () => { await client.updateDestination(destinationId, { config: { // Missing required fields - should be rejected }, }); });
Expected Behavior
-
serviceAccountJson validation:
- Must be valid JSON
- Must contain required GCP service account fields
- Return 400/422 error for invalid data
-
Config update validation:
- Required fields must be present (even in PATCH)
- Invalid values should be rejected
- Return 400/422 error for invalid data
Impact
- Invalid GCP Pub/Sub destinations can be created
- Configuration errors aren't caught until runtime
- Poor user experience
Proposed Solution
Add validation in the GCP Pub/Sub provider:
// Validate serviceAccountJson is valid JSON
func validateServiceAccountJson(json string) error {
var sa map[string]interface{}
if err := json.Unmarshal([]byte(json), &sa); err != nil {
return fmt.Errorf("invalid JSON: %w", err)
}
// Validate required fields
required := []string{"type", "project_id", "private_key_id", "private_key", "client_email"}
for _, field := range required {
if _, ok := sa[field]; !ok {
return fmt.Errorf("missing required field: %s", field)
}
}
return nil
}
Additional Context
- These tests were already skipped in the existing test suite
- This is a pre-existing limitation, not a new issue
- All other destination types have proper validation
- See detailed analysis in
spec-sdk-tests/TEST_STATUS.md
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Status
Backlog