-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(copilot): hosted api key validation + credential validation #3000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile OverviewGreptile SummaryThis PR adds credential and API key validation to the workflow editing tool. The changes introduce a new Key changes:
How it works: Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as EditWorkflowClientTool
participant API as /api/copilot/execute-copilot-server-tool
participant Server as EditWorkflowServerTool
participant PreValidate as preValidateCredentialInputs
participant Validator as validateSelectorIds
participant Apply as applyOperationsToWorkflowState
participant PostValidate as validateWorkflowSelectorIds
Client->>API: POST operations, workflowId
API->>Server: execute(params, context)
Note over Server: Parse currentUserWorkflow or fetch from DB
Server->>PreValidate: validate credentials & apiKeys
PreValidate->>PreValidate: Collect oauth-input & apiKey fields from operations
PreValidate->>PreValidate: Filter apiKeys for hosted models
PreValidate->>Validator: validateSelectorIds('oauth-input', credentialIds)
Validator->>Validator: Check credentials belong to user
Validator-->>PreValidate: {valid, invalid, warning}
PreValidate->>PreValidate: Remove invalid credentials from operations
PreValidate-->>Server: {filteredOperations, errors}
Server->>Apply: applyOperationsToWorkflowState(workflowState, filteredOperations)
Apply-->>Server: {modifiedWorkflowState, validationErrors, skippedItems}
Note over Server: Add credential errors to validationErrors
Server->>PostValidate: validateWorkflowSelectorIds(modifiedWorkflowState)
Note over PostValidate: Skips oauth-input (already validated)
PostValidate->>Validator: validateSelectorIds(other selector types)
Validator-->>PostValidate: validation results
PostValidate-->>Server: selector validation errors
Server-->>API: {workflowState, validationErrors, skippedItems}
API-->>Client: {workflowState, inputValidationErrors, skippedItems}
Note over Client: Apply diff & mark complete with errors
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| // Skip oauth-input - credentials are pre-validated before edit application | ||
| // This allows existing collaborator credentials to remain untouched | ||
| if (subBlockConfig.type === 'oauth-input') continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested node credentials bypass ownership validation
High Severity
Credentials in nestedNodes (blocks inside loop/parallel containers) bypass ownership validation entirely. The new preValidateCredentialInputs function only processes op.params.inputs and doesn't iterate over op.params.nestedNodes. Meanwhile, validateWorkflowSelectorIds now skips oauth-input validation because the comment assumes credentials are pre-validated. This creates a gap where nested node credentials are never validated for ownership.
Additional Locations (1)
| } | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested nodes apiKey filtering bypass for hosted models
Medium Severity
The preValidateCredentialInputs function filters apiKey inputs for hosted models but only processes op.params.inputs, not op.params.nestedNodes. When adding blocks inside a loop/parallel with nested nodes that specify both a hosted model and an apiKey, the apiKey won't be filtered. This defeats the stated purpose of the hosted API key validation feature.
| }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit operations skip apiKey check for existing block models
Low Severity
The apiKey validation only checks op.params.inputs.model to determine if the model is hosted. For edit operations on existing blocks where the model is already set (not provided in the operation), an apiKey input won't be filtered even if the block's existing model is a hosted model. The workflow state is available before this function is called but isn't passed in to check existing blocks.
Summary
Brief description of what this PR does and why.
Fixes #(issue)
Type of Change
Testing
How has this been tested? What should reviewers focus on?
Checklist
Screenshots/Videos